fix(hooks): decouple secretary-spawn carve-out to a members-only join witness (#1023)

PR #1021's config-less inbox-witness fallback in _team_has_secretary was also consumed by the bootstrap_gate secretary-spawn carve-out (binding 5). The inbox created by TaskUpdate(owner=secretary) BEFORE the Agent spawn made the witness True prematurely, so the carve-out DENIED the canonical secretary spawn — re-deadlocking bootstrap on every fresh CLI session.

Adds a members[]-only JOIN witness (_secretary_in_members) in bootstrap_gate with a broad except for the get_claude_config_dir/Path.home RuntimeError seam; the local marker_writer import is removed and _team_has_secretary keeps its inbox DISPATCH witness for the marker writer. A witness-read error now fires the carve-out (allow) — safe, since it only ever permits the canonical secretary spawn (bindings 1-3 gate all else). Non-vacuous both-modes regression (real task-assignment inbox), symbol-name docstring cleanup, PATCH 4.4.40.

Closes #1023.

Author: michael-wojcik

.claude-plugin/marketplace.json

@@ -12,7 +12,7 @@
       "name": "PACT",
       "source": "./pact-plugin",
       "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents",
-      "version": "4.4.39",
+      "version": "4.4.40",
       "author": {
         "name": "Synaptic-Labs-AI"
       },

README.md

@@ -605,7 +605,7 @@ When installed as a plugin, PACT lives in your plugin cache:
 │   └── cache/
 │       └── pact-plugin/
 │           └── PACT/
-│               └── 4.4.39/      # Plugin version
+│               └── 4.4.40/      # Plugin version
 │                   ├── agents/
 │                   ├── commands/
 │                   ├── skills/

pact-plugin/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "PACT",
-  "version": "4.4.39",
+  "version": "4.4.40",
   "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents",
   "author": {
     "name": "Synaptic-Labs-AI",

pact-plugin/README.md

@@ -1,6 +1,6 @@
 # PACT — Orchestration Harness for Claude Code
 
-> **Version**: 4.4.39
+> **Version**: 4.4.40
 
 Turn a single Claude Code session into a managed team of specialist AI agents that prepare, design, build, and test your code systematically.
 

pact-plugin/hooks/bootstrap_gate.py

@@ -361,10 +361,10 @@ def _degraded_decision(stage: str, error: BaseException, tool_name: "str | None"
 # carve-out below; any drift silently re-introduces the bootstrap-deadlock
 # these constants are here to prevent.
 #
-# _SECRETARY_NAME mirrors bootstrap_marker_writer._SECRETARY_NAME (the
-# producer-side constant at marker_writer.py:103) and the literal at
-# commands/bootstrap.md Step 2. Cross-file atomic edits required across
-# this file, bootstrap_marker_writer.py, AND commands/bootstrap.md.
+# _SECRETARY_NAME mirrors the producer-side bootstrap_marker_writer._SECRETARY_NAME
+# constant and the literal at commands/bootstrap.md Step 2. Cross-file atomic
+# edits required across this file, bootstrap_marker_writer.py, AND
+# commands/bootstrap.md.
 #
 # _SECRETARY_AGENT_TYPE is the canonical agentType from
 # commands/bootstrap.md Step 2 — no producer-side mirror in
@@ -386,6 +386,47 @@ def _degraded_decision(stage: str, error: BaseException, tool_name: "str | None"
 )
 
 
+def _secretary_in_members(team_name: str) -> bool:
+    """Members[]-only JOIN witness: has the secretary actually joined the team
+    roster? Sole consumer is _is_canonical_secretary_spawn binding 5.
+
+    Reads the team config members[] via the shared pact_context._iter_members
+    helper (already imported at this module's top level) and returns True iff a
+    member's ``name`` equals _SECRETARY_NAME. This is a JOIN witness — distinct
+    from bootstrap_marker_writer._team_has_secretary, which is a DISPATCH
+    witness (members[] OR the secretary's inbox file). The two predicates
+    answer DIFFERENT questions on purpose (#1023): the marker writer needs
+    "was the secretary DISPATCHED?" (the inbox is created by
+    TaskUpdate(owner=secretary) BEFORE the spawn, so a dispatch witness is
+    correct there); the carve-out needs "has the secretary JOINED members[]?"
+    (only an actual Agent() spawn-return populates members[]). Reading the
+    inbox arm here is what re-deadlocked the canonical secretary spawn in
+    #1021 — the inbox predates the spawn, so binding 5 (`not True`) denied the
+    very spawn the carve-out exists to permit.
+
+    NEVER raises (totality, #989). The body is wrapped in a BROAD
+    ``except Exception: return False`` rather than a typed tuple because the
+    members[] read transitively calls pact_context.get_claude_config_dir() ->
+    Path.home(), which raises RuntimeError when HOME is unresolvable. That
+    RuntimeError is composed BEFORE _iter_members' own typed try-block, so it
+    escapes _iter_members uncaught, and it is ALSO absent from binding 5's
+    outer typed except — without a broad wrap here it would propagate to
+    main()'s degraded-DENY path and re-deadlock the secretary spawn (the wrong
+    fail direction). A False return makes the carve-out FIRE (the safe
+    direction — it only ever permits the canonical secretary spawn, never a
+    non-secretary tool, which bindings 1/2/3 already exclude). This mirrors the
+    bare-except seam precedent at shared.pact_context._resolve_aligned_team_name,
+    which uses a broad except for the same Path.home RuntimeError reason.
+    """
+    try:
+        return any(
+            member.get("name") == _SECRETARY_NAME
+            for member in pact_context._iter_members(team_name)
+        )
+    except Exception:  # noqa: BLE001 — broad by design: Path.home() RuntimeError seam (see docstring)
+        return False
+
+
 def _is_canonical_secretary_spawn(input_data: dict) -> bool:
     """Audit anchor: canonical secretary spawn carve-out for #789.
 
@@ -404,30 +445,62 @@ def _is_canonical_secretary_spawn(input_data: dict) -> bool:
          (the orchestrator may still pass a stale arg the platform discards).
          The carve-out stays tight via bindings 2/3 (exact subagent_type +
          name literals) and binding 5 (one-shot, gated on the REAL team dir).
-      5. NOT _team_has_secretary(get_team_name()) — one-shot semantic; flips
-         to False the moment the spawned secretary lands in members[]. Reads
-         the REAL session team dir (expected_team), which the empty-team
-         fail-closed below guarantees is a non-empty path segment.
+      5. NOT _secretary_in_members(get_team_name()) — members[]-only JOIN
+         witness (#1023). Reads the REAL session team dir (expected_team),
+         which the empty-team fail-closed below guarantees is a non-empty path
+         segment. Uses the gate-local _secretary_in_members helper (a JOIN
+         witness), NOT bootstrap_marker_writer._team_has_secretary (a DISPATCH
+         witness that also accepts the secretary's inbox file). See the
+         join-vs-dispatch note below.
 
     Binding (1) is a hardcoded literal. Bindings (2) and (3) compare against
     module constants, not tool_input-derived values. Binding (5) is a disk
-    read of the team config members[]; True after first successful dispatch,
-    so the carve-out fires at most once per session. With binding 4 dropped,
-    the carve-out reads no tool_input-derived team value — the
-    secretary-presence check resolves against the SSOT team dir only.
+    read of the team config members[]. With binding 4 dropped, the carve-out
+    reads no tool_input-derived team value — the secretary-presence check
+    resolves against the SSOT team dir only.
+
+    JOIN witness vs DISPATCH witness (#1023): binding 5 calls the gate-local
+    _secretary_in_members (members[]-ONLY) and deliberately does NOT call
+    bootstrap_marker_writer._team_has_secretary. The two answer DIFFERENT
+    questions. The marker writer needs "was the secretary DISPATCHED?" and so
+    accepts the inbox file as a fallback witness — correct for it, because the
+    inbox is the config-less Desktop signal (#1019). But the inbox is created
+    by TaskUpdate(owner=secretary) (bootstrap Step 2) BEFORE the Agent spawn
+    (Step 3), so reading the inbox here made binding 5 (`not True`) DENY the
+    very spawn the carve-out exists to permit — the #1021 regression. The
+    carve-out needs "has the secretary JOINED members[]?", which only an actual
+    Agent() spawn-return populates. Hence the split: members[]-only here, inbox
+    fallback left to the marker writer alone.
+
+    ONE-SHOT GUARANTEE (D3 decision-record, #1023) — the carve-out's one-shot
+    semantic MIGRATES from binding-5-self-closure to MARKER-PRESENCE. Under CLI
+    the old members[] check flipped True once the secretary joined, self-closing
+    the carve-out. But under config-less Desktop members[] is STRUCTURALLY empty
+    (no config.json), so binding 5 can no longer self-close — _secretary_in_members
+    is always False there and the carve-out always fires pre-marker. The durable
+    one-shot is now MARKER-PRESENCE: the is_marker_set fast-path in
+    _check_tool_allowed returns None (allow-all) BEFORE the carve-out
+    (this _is_canonical_secretary_spawn check) is ever reached in
+    _check_tool_allowed, so once the marker is
+    written the carve-out is moot. Documented so no future reader restores a
+    binding-5 one-shot and re-deadlocks Desktop. The Desktop always-fire window
+    is contained by bindings 1/2/3 (exact Agent + pact-secretary + secretary,
+    all module constants) plus the marker fast-path that closes it.
 
     On ANY disk-read exception, returns False — caller falls through to
     the existing _BLOCKED_TOOLS deny path so the user sees the canonical
     _DENY_REASON ("PACT bootstrap required...") rather than the
     load-failure variant. Mirrors is_marker_set's silent-on-exception
-    style.
-
-    SACROSANCT — local-import discipline: _team_has_secretary is imported
-    LOCALLY (function-call time, not module-load time) to break the
-    reciprocal cycle with bootstrap_marker_writer, which imports
-    is_marker_set from this module at its own top-level. Reciprocal
-    top-level import here would deadlock module load and route every
-    tool call through the fail-closed deny path.
+    style. (_secretary_in_members has its own broad-except totality guard for
+    the Path.home() RuntimeError seam — see its docstring.)
+
+    (The former SACROSANCT local-import discipline note is now OBSOLETE: binding
+    5 no longer imports _team_has_secretary from bootstrap_marker_writer, so the
+    reciprocal-cycle local-import is gone. The members[] read goes through the
+    top-level-imported shared.pact_context, which bootstrap_marker_writer also
+    imports at top level — no cycle. The one remaining cross-module edge is
+    unchanged: bootstrap_marker_writer imports is_marker_set from THIS module at
+    its own top level.)
     """
     try:
         if input_data.get("tool_name") != "Agent":
@@ -445,13 +518,14 @@ def _is_canonical_secretary_spawn(input_data: dict) -> bool:
         expected_team = pact_context.get_team_name()
         if not expected_team:
             return False
-        # Local-import: reciprocal-cycle prevention. bootstrap_marker_writer
-        # imports is_marker_set from this module at its OWN top-level; a
-        # reciprocal top-level import here would deadlock module load and
-        # silently route every tool call through the fail-closed deny path.
-        # See SACROSANCT block in this docstring.
-        from bootstrap_marker_writer import _team_has_secretary
-        return not _team_has_secretary(expected_team)
+        # Binding 5: members[]-only JOIN witness (#1023). _secretary_in_members
+        # is a gate-local helper reading shared.pact_context._iter_members (the
+        # top-level-imported module) — NOT a cross-module import of
+        # bootstrap_marker_writer._team_has_secretary (the DISPATCH witness,
+        # which also accepts the inbox file and so re-deadlocked the spawn in
+        # #1021). The former reciprocal-cycle local-import is gone. See the
+        # join-vs-dispatch + D3 one-shot notes in this docstring.
+        return not _secretary_in_members(expected_team)
     except (OSError, ValueError, KeyError, TypeError, AttributeError, ImportError):
         return False
 

pact-plugin/hooks/merge_guard_post.py

@@ -449,9 +449,9 @@ def _retire_token_for_command(
             # Cycle-1 fail-OPEN-on-either-empty AND-short-circuit WAS
             # itself the attack surface — populated current_session +
             # empty token_session let attacker-written tokens through.
-            # See test_merge_guard.py:5068 (test_no_session_id_accepts_any_token)
-            # for the preserved invariant; :5086 inversion is the SEC-S1
-            # fix landing.
+            # See test_no_session_id_accepts_any_token (in test_merge_guard.py)
+            # for the preserved invariant; its SEC-S1 inversion counterpart is
+            # the fix landing.
             if not token_session or current_session != token_session:
                 continue
         try:

pact-plugin/hooks/merge_guard_pre.py

@@ -802,9 +802,9 @@ def find_valid_token(token_dir: Path | None = None) -> tuple[dict, str] | tuple[
                 # Cycle-1 fail-OPEN-on-either-empty AND-short-circuit WAS
                 # itself the attack surface — populated current_session +
                 # empty token_session let attacker-written tokens through.
-                # See test_merge_guard.py:5068 (test_no_session_id_accepts_any_token)
-                # for the preserved invariant; :5086 inversion is the SEC-S1
-                # fix landing.
+                # See test_no_session_id_accepts_any_token (in test_merge_guard.py)
+                # for the preserved invariant; its SEC-S1 inversion counterpart
+                # is the fix landing.
                 if not token_session or current_session != token_session:
                     continue
 

pact-plugin/hooks/shared/hook_infra_classifier.py

@@ -161,15 +161,20 @@
         "tool_response", "variety_scorer",
     }),
     "bootstrap_gate": frozenset({
-        "claude_md_manager", "constants", "marker_schema", "pact_context",
-        "paths", "pin_caps", "session_journal", "session_registry",
-        "session_resume", "session_state", "staleness",
-    }),  # claude_md_manager / session_resume / staleness / pin_caps reached
-         # here via bootstrap_gate -> bootstrap_marker_writer -> session_resume
-         # (update_session_info) + claude_md_manager (resolve_project_claude_md_path),
-         # and session_resume -> staleness -> pin_caps. Added when #989's
-         # write-back self-heal pulled session_resume + claude_md_manager into
-         # bootstrap_marker_writer's imports.
+        "constants", "marker_schema", "pact_context",
+        "paths", "session_journal", "session_registry",
+        "session_state",
+    }),  # #1023 SHRANK this closure: the carve-out's binding 5 no longer
+         # imports bootstrap_marker_writer (it reads the gate-local
+         # _secretary_in_members JOIN witness via the already-top-level
+         # pact_context._iter_members), so bootstrap_gate no longer reaches
+         # bootstrap_marker_writer's transitive closure. The former extras
+         # claude_md_manager / session_resume / staleness / pin_caps were
+         # reachable ONLY through that deleted edge (bootstrap_gate ->
+         # bootstrap_marker_writer -> session_resume (update_session_info) +
+         # claude_md_manager (resolve_project_claude_md_path); session_resume ->
+         # staleness -> pin_caps) and are now gone from this closure.
+         # bootstrap_marker_writer's OWN closure (below) is unchanged.
     "bootstrap_marker_writer": frozenset({
         "claude_md_manager", "constants", "marker_schema", "pact_context",
         "paths", "pin_caps", "session_journal", "session_registry",

pact-plugin/hooks/shared/intentional_wait.py

@@ -92,8 +92,8 @@
 #
 # Auditor signal-tasks are NOT in this set — they self-complete via
 # `metadata.completion_type == "signal"` + `metadata.type in {"blocker",
-# "algedonic"}` (the inline-literal pattern at task_utils.py:276 /
-# session_resume.py:522). Two distinct exemption surfaces:
+# "algedonic"}` (the same inline-literal pattern used in task_utils and
+# session_resume). Two distinct exemption surfaces:
 # - SELF_COMPLETE_EXEMPT_AGENT_TYPES: by team-config agentType lookup.
 # - signal-task pattern: by task metadata, applies to any agent.
 SELF_COMPLETE_EXEMPT_AGENT_TYPES: frozenset = frozenset({
@@ -352,8 +352,8 @@ def is_self_complete_exempt(
        surface short-circuits to False (fail-closed).
     2. By signal-task metadata: task.metadata.completion_type == "signal" AND
        task.metadata.type in {"blocker", "algedonic"}. Mirrors the inline
-       literal at agent_handoff_emitter.py / task_utils.py:276 /
-       session_resume.py:522. Independent of `team_name`.
+       literal in agent_handoff_emitter / task_utils / session_resume.
+       Independent of `team_name`.
 
     Pure function; never raises on plain dicts (PACT task representation
     via json.loads); defaults to False on missing or malformed fields

pact-plugin/hooks/shared/session_state.py

@@ -142,7 +142,7 @@ def is_safe_path_component(value: str) -> bool:
 
     Defense-in-depth against path-traversal via tampered session
     context (see security review Finding 2). The upstream allowlist
-    lives at `session_init.py:401` — `re.sub(r"[^a-f0-9-]", "",
+    lives in session_init's team-name generation — `re.sub(r"[^a-f0-9-]", "",
     session_id[:8])` — which already filters path separators, `..`,
     nulls, and controls at team-name generation time. This guard is a
     second line of defense at the I/O boundary.