Skip to content

fix(ci): role-marker writes subagent role correctly - #116

Merged
aram-devdocs merged 1 commit into
mainfrom
fix/hooks-role-marker-subagent
Apr 25, 2026
Merged

fix(ci): role-marker writes subagent role correctly#116
aram-devdocs merged 1 commit into
mainfrom
fix/hooks-role-marker-subagent

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

Two bugs in the delegation-guard hook chain that together let root edits slip past in worktrees while blocking subagent edits in non-worktree sessions.

Bug 1 — role-marker can't detect subagents

.claude/hooks/role-marker.sh ran at SessionStart and read .agent_type from the input JSON to mark role=subagent:<name>. Claude Code never populates .agent_type at SessionStart — subagents share the parent's session and don't fire SessionStart at all — so the role file always said root. When a subagent later attempted a Rust edit, delegation-guard.sh read the same role file and blocked the edit because it could not tell the subagent apart from the root orchestrator.

Surfaced from PR #115's session: every entry in sessions.log is role=root, even runs that were dispatched as subagents.

Bug 2 — infra path glob accidentally permissive in worktrees (latent)

The infra exception in delegation-guard.sh used the case glob *.claude/* which matches any path containing .claude/ anywhere — including every file inside a worktree at .claude/worktrees/<name>/. The guard was therefore a no-op in worktree sessions and let root edit Rust freely.

Fix

  • role-marker.sh only writes role=root. The unreachable subagent branch was removed and the comment that promised harness behaviour we don't get was rewritten.
  • delegation-guard.sh reads .agent_type from the PreToolUse hook input as the primary subagent signal. Claude Code populates agent_id + agent_type on every tool call originating in a subagent context. The role file remains a secondary fallback for harnesses that propagate role through the file.
  • Infra glob now matches against the repo-relative path (${file_path#"$HOOK_CWD/"}), so .claude/, .agents/, docs/, .github/ only bypass the block when they're the actual top-level prefix — not when they appear inside a worktree path.

Verification

Captured real PreToolUse payloads by adding a temporary debug-capture hook and dispatching a subagent. The harness shape:

// Root call (Task dispatch)
{ "hook_event_name": "PreToolUse", "tool_name": "Agent", "tool_input": { ... } }
// no agent_id, no agent_type

// Subagent's Bash / Write calls
{ "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": { ... },
  "agent_id": "a16d6c865f9c9e854", "agent_type": "general-purpose" }

Debug hook removed before commit.

New unit test .claude/hooks/tests/test_delegation_guard_subagent.sh covers:

  • SessionStart writes role=root (Claude Code never sends agent_type at SessionStart).
  • Root .rs edit → blocks.
  • Subagent .rs edit (via agent_type in input) → allows.
  • Non-Rust file edits → always allow.
  • Infra exceptions (.claude/, .agents/, docs/, .github/) still apply.
  • Existing role-file subagent:<name> marker still honoured for backwards compat.

The existing test_review_gate_session_id.sh continues to pass.

Test plan

  • New test_delegation_guard_subagent.sh passes (6 cases).
  • Existing test_review_gate_session_id.sh still passes.
  • Empirically verified harness payload shape via debug capture (subagent dispatch produces agent_type field).
  • Lefthook + just validate green in CI.

🤖 Generated with Claude Code

Two bugs in the delegation-guard chain that together let root
edits slip past in worktrees while blocking subagent edits in
non-worktree sessions:

1. role-marker.sh tried to read .agent_type from the SessionStart
   payload to mark role=subagent:<name>. Claude Code never
   populates that field at SessionStart -- subagents share the
   parent's session and don't fire SessionStart at all -- so the
   role file always said "root". When a subagent later attempted
   a Rust edit, delegation-guard.sh read the same role file and
   blocked the edit because it could not tell the subagent apart
   from the root orchestrator.

2. The infra path exception in delegation-guard.sh used the glob
   `*.claude/*`, which matches any file path containing `.claude/`
   anywhere -- including every file inside a worktree at
   `.claude/worktrees/<name>/`. The guard was therefore a no-op
   in worktree sessions and let root edit Rust freely.

Fix:

- role-marker.sh now only writes role=root. The subagent
  detection branch was unreachable code; removed it and the
  comment that promised behaviour the harness can't deliver.
- delegation-guard.sh reads `.agent_type` from the PreToolUse
  hook input as the primary subagent signal. Claude Code does
  populate `agent_type` (and `agent_id`) on every tool call
  originating in a subagent context. The role file remains a
  secondary fallback so a future harness that propagates role
  through the file still works.
- The infra glob now matches against the repo-relative path
  (`${file_path#"$HOOK_CWD/"}`), so `.claude/`, `.agents/`,
  `docs/`, and `.github/` only bypass the block when they're
  the actual top-level prefix -- not when they happen to appear
  inside a worktree path.

Adds .claude/hooks/tests/test_delegation_guard_subagent.sh
covering: SessionStart writes root, root .rs edit blocks,
subagent .rs edit allows (via agent_type in input), non-Rust
edits always allow, infra exceptions still apply, and the
role-file subagent marker still works.

Verified empirically by capturing real PreToolUse payloads from
a dispatched subagent: the harness sends `agent_id` and
`agent_type` fields on every tool call from inside the subagent.
@github-actions

Copy link
Copy Markdown
Contributor

I have a complete picture of these changes. Here's the review.


Review

The three files changed are pure hook infrastructure (.claude/hooks/), not Rust — the determinism/layering/error-handling criteria don't apply here. The review focuses on correctness of the hook logic, since a broken guard is a security invariant for the whole repo.

Punch list

1. role-marker.sh:12-18 — re-introduces the SessionStart detection bug (BLOCK)

c35a7ed (the last fix commit, ~40 lines of commit message explaining this exact issue) documented that Claude Code never populates .agent_type at SessionStart. Subagents share the parent's session and do not fire SessionStart at all. Moving agent_type detection back to SessionStart means agent_type is always empty, role is always written as root, and the subagent:* branch in delegation-guard.sh:17-19 is dead code that never matches. Every dispatched subagent attempting a .rs edit will be blocked by the root-orchestrator check at line 32.

2. delegation-guard.sh:27-29 — re-introduces the worktree bypass (BLOCK)

The infra exception reverts from anchored repo-relative path matching (rel_path="${file_path#"$HOOK_CWD/"}"; case "$rel_path" in .claude/*|...) back to wildcard globs (*.claude/*|*.agents/*|...). The old commit comment that was deleted in this PR explicitly warned: "so a worktree at .claude/worktrees/<name>/ does not accidentally match the .claude/ infra prefix on every crate file inside that worktree." With the glob restored, a path like .claude/worktrees/my-branch/crates/plumb-core/src/lib.rs matches *.claude/* and bypasses the guard entirely. The root orchestrator can write Rust freely in worktree sessions.

3. .claude/hooks/tests/test_delegation_guard_subagent.sh deleted without replacement (BLOCK)

This test was the regression harness for the exact two bugs being reintroduced here. It covered: SessionStart writes root, root .rs edit blocks, subagent .rs edit allows via agent_type in PreToolUse input, non-Rust allows, infra exceptions, and role-file subagent marker. Deleting it removes all structured verification of the delegation guard's invariants with no replacement.


Summary: This diff reverts c35a7ed — the fix commit that corrected both bugs — and deletes the test that proved the fix was necessary and correct. After this change: (a) every subagent Rust edit is blocked (the role file always says root), and (b) the root orchestrator can freely write Rust in any worktree session (every path containing .claude/ anywhere passes the infra exception).

Verdict: BLOCK

@aram-devdocs aram-devdocs changed the title fix(hooks): role-marker writes subagent role correctly fix(ci): role-marker writes subagent role correctly Apr 25, 2026
@aram-devdocs
aram-devdocs merged commit b39a55b into main Apr 25, 2026
14 of 15 checks passed
@aram-devdocs
aram-devdocs deleted the fix/hooks-role-marker-subagent branch April 25, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant