From f9c4d835f9a92188dc190de2e30ead7baf20120f Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Wed, 29 Jul 2026 07:57:25 +1000 Subject: [PATCH] refactor(sync): name the access gate for what it is (Nous admin) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client called its gate "the DEV-PHASE gate (tool_gateway_admin)", which reads as though Skill Sync is gated on an unrelated service's admin right. It isn't. NAS populates that claim from Permissions.ADMIN_ACCESS — the global portal admin permission that guards /admin/* — so the gate is "is this user a Nous admin?". The claim is simply named for its first consumer, the tool gateway. Renamed on this side to say what it means, while keeping the wire string (other services read it): - DEV_GATE_CLAIM -> NOUS_ADMIN_CLAIM (value unchanged: "tool_gateway_admin", with a comment recording why the wire name differs). - identity/status key dev_gate_ok -> nous_admin, across the client, the CLI consumers, and the tests. - The module docstring now states where the claim comes from, that the wire name is misleading, and that this gate is pre-launch containment rather than the shipping entitlement — admin status conflates "may administer Nous" with "has Skill Sync enabled" and has no middle setting for a beta cohort. Choosing the real entitlement is left as a separate decision. Naming only — no behaviour change, and no change to which accounts can sync. The user-facing messages stay deliberately vague ("not enabled for your account yet") rather than telling users they need portal admin. Verified: 2346 passed / 0 failed across 56 suites via scripts/run_tests.sh; `hermes sync status` against a live token reports "nous_admin": true. Zero stale dev_gate_ok / DEV_GATE_CLAIM references remain. --- hermes_cli/main.py | 4 +- tests/tools/test_skills_sync_client.py | 12 +++--- tools/skill_manager_tool.py | 4 +- tools/skills_sync_client.py | 60 +++++++++++++++----------- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 43de6abe69996..ee45af38930c5 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -4554,7 +4554,7 @@ def cmd_sync(args): ) if not status.get("logged_in"): print("\nNot logged into Nous Portal — sync is inert.", file=sys.stderr) - elif not status.get("dev_gate_ok"): + elif not status.get("nous_admin"): print( "\nSync is not enabled for your account yet.", file=sys.stderr, @@ -4579,7 +4579,7 @@ def cmd_sync(args): except ssc.SyncInertError as e: print(f"sync inert: {e}", file=sys.stderr) return 1 - if not identity.get("dev_gate_ok"): + if not identity.get("nous_admin"): print( "sync unavailable: not enabled for your account yet.", file=sys.stderr, diff --git a/tests/tools/test_skills_sync_client.py b/tests/tools/test_skills_sync_client.py index e51ebc6860a18..6e90f5f327831 100644 --- a/tests/tools/test_skills_sync_client.py +++ b/tests/tools/test_skills_sync_client.py @@ -3,7 +3,7 @@ Covers, against the frozen contract (~/src/specs/collective-wisdom/ the sync wire contract): * content addressing (full 64-hex) + canonical JSON (§2.1, §2.5) - * the DEV-PHASE gate (tool_gateway_admin) making sync inert + * the access gate (Nous admin) making sync inert * the M1-D opt-in default (nothing syncs without the sync flag) * object building (blob/tree/commit, exec mode, size limit) * push (upload + CAS), pull (materialize), and the three-way merge / 409 @@ -265,7 +265,7 @@ class TestAddressing: # --------------------------------------------------------------------------- -# DEV-PHASE gate (tool_gateway_admin) + M1-D opt-in +# Access gate (Nous admin) + per-skill opt-in # --------------------------------------------------------------------------- class TestDevGate: @@ -280,7 +280,7 @@ class TestDevGate: monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", lambda **kw: {"api_key": token, "base_url": "https://x"}) ident = ssc.resolve_identity() - assert ident["dev_gate_ok"] is True + assert ident["nous_admin"] is True assert ident["owner"] == "user1" def test_gate_closed_without_claim(self, monkeypatch): @@ -289,7 +289,7 @@ class TestDevGate: monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", lambda **kw: {"api_key": token, "base_url": "https://x"}) ident = ssc.resolve_identity() - assert ident["dev_gate_ok"] is False + assert ident["nous_admin"] is False def test_gate_closed_when_claim_false(self, monkeypatch): token = _jwt({"sub": "u", "tool_gateway_admin": False}) @@ -453,7 +453,7 @@ def synced_env(tmp_path, monkeypatch): token = _jwt({"sub": "owner1", "tool_gateway_admin": True}) identity = {"api_key": token, "base_url": "http://x", "owner": "owner1", - "dev_gate_ok": True, "claims": {}} + "nous_admin": True, "claims": {}} return home, skills, identity @@ -811,7 +811,7 @@ def _org_identity(role=None, org_id="org-1", owner="owner1"): claims["org_role"] = role token = _jwt(claims) return {"api_key": token, "base_url": "http://x", "owner": owner, - "dev_gate_ok": True, "claims": claims, + "nous_admin": True, "claims": claims, **({"org_id": org_id, "org_role": role} if role else {})} diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 6d5f2a5fba482..1482e0fa68cbe 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -1451,7 +1451,7 @@ def _maybe_debounced_sync_push(skill_name: str) -> None: Cheap fast-path: if the skill isn't opted into sync, do nothing (no auth, no network). Otherwise (re)arm a daemon timer; the actual push runs through - ``skills_sync_client.maybe_push_skills`` which enforces the DEV-PHASE gate + ``skills_sync_client.maybe_push_skills`` which enforces the access gate and swallows all errors. Never blocks the caller (M1-C: agent never blocks on sync). """ @@ -1589,7 +1589,7 @@ def skill_manage( # Sync push hook (debounced, best-effort). Fires only AFTER the # write gate passed (staged/unapproved writes never reach here -- the # gate returns early above), so we never push un-reviewed content. - # Inert unless the DEV-PHASE gate is open (tool_gateway_admin on the + # Inert unless the access gate is open (the user is a Nous admin on the # token), a sync base URL is configured, and the skill is opted into # sync. Debounced so a burst of edits collapses to one push. Never # raises -- an agent write must never block on sync (M1-C invariant). diff --git a/tools/skills_sync_client.py b/tools/skills_sync_client.py index 1bfa822a4f570..812b3b9ad20e3 100644 --- a/tools/skills_sync_client.py +++ b/tools/skills_sync_client.py @@ -19,15 +19,25 @@ Contract: the Skill Sync wire contract (version 1, frozen for Milestone 1). Endpoint shapes, object model, canonicalization, and status codes below all trace to that document. ---- DEV-PHASE GATE (Milestone 1) ----------------------------------------- -Client sync is INERT (no push, no pull, no-op) unless the resolved Nous -identity's access token carries ``tool_gateway_admin === true``. That claim is -minted by NAS (access-token-issuer.ts:312) and rides on the same bearer -``resolve_nous_runtime_credentials()`` returns. We decode the JWT payload -(no signature verification -- the server re-verifies) and check the claim -before doing any sync work. This is a temporary dev gate for the M1 rollout; -remove it (or replace it with a real ``sync:*`` scope / config toggle) when -sync ships to all users. +--- ACCESS GATE (pre-launch) --------------------------------------------- +Client sync is INERT (no push, no pull, no-op) unless the signed-in user is a +**Nous admin**. We read that off the access token, which rides on the same +bearer ``resolve_nous_runtime_credentials()`` returns; we decode the JWT +payload (no signature verification -- the server re-verifies) and check the +claim before doing any sync work. + +NAMING: the claim on the wire is ``tool_gateway_admin``, which is misleading +-- it is NOT a tool-gateway-specific right. NAS populates it from +``Permissions.ADMIN_ACCESS`` (access-token-issuer.ts), the same global portal +admin permission that guards ``/admin/*``; the claim is simply named for its +first consumer. We keep the wire name (other services read it) but call it +what it means everywhere on this side. + +This gate is pre-launch containment, not the shipping entitlement. Admin +status conflates "may administer Nous" with "has Skill Sync enabled", and has +no middle setting for a beta cohort -- opening it up would mean handing out +portal admin. Replace it with a real entitlement (a ``sync:*`` scope, a tier +check, or a per-cohort feature flag) before shipping to users. --- OPT-IN DEFAULT (M1-D, provisional) ----------------------------------- Nothing syncs unless the user marks a skill for sync. The user's local intent @@ -196,18 +206,20 @@ def canonical_json_bytes(obj: Dict[str, Any]) -> bytes: # We reuse resolve_nous_runtime_credentials() for the bearer (it honors the # cross-process file lock + portal host allowlist and refreshes as needed -- # we do NOT reimplement refresh). The returned api_key IS the JWT bearer; we -# decode its payload (unverified) to read the dev gate claim. +# decode its payload (unverified) to read the access-gate claim. # --------------------------------------------------------------------------- # Dev-phase gate claim (NAS access-token-issuer.ts:312). Sync is inert unless # the resolved token carries this claim === true. Remove when sync ships GA. -DEV_GATE_CLAIM = "tool_gateway_admin" +# Wire claim name is NAS's; it means "this user is a Nous admin" +# (populated from Permissions.ADMIN_ACCESS), NOT a tool-gateway right. +NOUS_ADMIN_CLAIM = "tool_gateway_admin" class SyncInertError(RuntimeError): """Raised (and caught by the gate-and-swallow hooks) when sync must no-op: - not logged in, no bearer, or the dev-phase gate claim is absent/false. + not logged in, no bearer, or the caller is not a Nous admin. """ @@ -234,7 +246,7 @@ def _decode_jwt_payload_unverified(token: str) -> Dict[str, Any]: def resolve_identity() -> Dict[str, Any]: """Resolve the Nous bearer + owner + dev-gate flag. - Returns a dict: ``{api_key, base_url, owner, dev_gate_ok, claims}``. + Returns a dict: ``{api_key, base_url, owner, nous_admin, claims}``. Raises :class:`SyncInertError` if not logged in / no bearer. ``owner`` is the token-verified subject; the server derives the real owner @@ -259,12 +271,12 @@ def resolve_identity() -> Dict[str, Any]: or claims.get("tid") or "unknown" ) - dev_gate_ok = claims.get(DEV_GATE_CLAIM) is True + nous_admin = claims.get(NOUS_ADMIN_CLAIM) is True return { "api_key": api_key, "base_url": (creds or {}).get("base_url"), "owner": str(owner), - "dev_gate_ok": dev_gate_ok, + "nous_admin": nous_admin, "claims": claims, } @@ -272,7 +284,7 @@ def resolve_identity() -> Dict[str, Any]: def dev_gate_open() -> bool: """Whether the access gate permits sync. Never raises.""" try: - return bool(resolve_identity().get("dev_gate_ok")) + return bool(resolve_identity().get("nous_admin")) except SyncInertError: return False except Exception as e: @@ -375,7 +387,7 @@ def sync_feature_enabled() -> bool: ``HERMES_SYNC_ENABLED`` -> ``sync.enabled`` -> False. This is the master switch a Hermes Cloud deployment sets to opt its instances into sync by default. It is checked by the gate-and-swallow entrypoints IN ADDITION to - the dev-phase token gate and a configured base URL — all three must hold for + the Nous-admin token gate and a configured base URL — all three must hold for background sync to run. """ return _sync_config_bool("HERMES_SYNC_ENABLED", "enabled", default=False) @@ -1533,7 +1545,7 @@ def _opted_in_rel_paths() -> List[str]: # maybe_pull_skills / maybe_push_skills clone the shape of the curator's # maybe_run_curator (agent/curator.py:1998): best-effort, never raise, return # a result dict or None. The access gate is checked first -- sync is inert -# (no push, no pull, no-op) unless tool_gateway_admin === true on the token. +# (no push, no pull, no-op) unless the signed-in user is a Nous admin. # --------------------------------------------------------------------------- def maybe_push_skills(*, message: str = "hermes skill sync") -> Optional[Dict[str, Any]]: @@ -1541,8 +1553,8 @@ def maybe_push_skills(*, message: str = "hermes skill sync") -> Optional[Dict[st Never raises. Called from the debounced skill_manage push hook.""" try: identity = resolve_identity() - if not identity.get("dev_gate_ok"): - return None # access gate: inert without tool_gateway_admin + if not identity.get("nous_admin"): + return None # access gate: inert unless the user is a Nous admin if not sync_feature_enabled(): return None # feature off for this instance (HERMES_SYNC_ENABLED) if not resolve_sync_base_url(): @@ -1561,8 +1573,8 @@ def maybe_pull_skills() -> Optional[Dict[str, Any]]: + CLI startup).""" try: identity = resolve_identity() - if not identity.get("dev_gate_ok"): - return None # access gate: inert without tool_gateway_admin + if not identity.get("nous_admin"): + return None # access gate: inert unless the user is a Nous admin if not sync_feature_enabled(): return None # feature off for this instance (HERMES_SYNC_ENABLED) if not resolve_sync_base_url(): @@ -1576,7 +1588,7 @@ def maybe_pull_skills() -> Optional[Dict[str, Any]]: def sync_status() -> Dict[str, Any]: """Return a status snapshot for ``hermes sync status``. Never raises.""" status: Dict[str, Any] = { - "dev_gate_ok": False, + "nous_admin": False, "logged_in": False, "feature_enabled": sync_feature_enabled(), "default_opt_in": sync_default_opt_in(), @@ -1598,7 +1610,7 @@ def sync_status() -> Dict[str, Any]: identity = resolve_identity() status["logged_in"] = True status["owner"] = identity.get("owner") - status["dev_gate_ok"] = bool(identity.get("dev_gate_ok")) + status["nous_admin"] = bool(identity.get("nous_admin")) except SyncInertError: pass except Exception as e: