Skip to content

spec(hooks): disambiguate Not decision-eligible and align subagentStop as Audit-only - #341

Open
drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:spec/decision-eligibility-wording-146
Open

drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:spec/decision-eligibility-wording-146

Conversation

@drk-ram

@drk-ram drk-ram commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Removes the overloaded "Not decision-eligible" phrase from docs/spec/instrument/hooks.md and specification/v0.1.0/hooks/subagent-stop.json, labels postCompact explicitly as ALLOW / MODIFY (DENY not permitted), aligns subagent-stop.json with hooks.md and conformance.md as Audit-only (ALLOW only) while documenting where subagent output redaction attaches, and adds a guard test in tests/test_hook_taxonomy.py.

Which issue does this implement

Fixes #146

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

Specification changes

  • I opened a Discussion before this PR
  • Schema changes validate against the JSON Schema spec
  • I described the impact on downstream implementers below

Breaking for implementers? No wire shape changes. Resolves the terminology conflict between subagent-stop.json, postCompact, turnEnd, and hooks.md.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

Documents explicitly on subagentStop that because it is audit-only on the parent chain, output redaction for subagents must be enforced inside the subagent's own session (toolCallResult, agentResponse) or when the parent acts on the returned content.

@github-actions

Copy link
Copy Markdown

This pull request's description says it closes #146, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

@rocklambros rocklambros left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raghuvar, "Not decision-eligible" is now gone from hooks.md and from every hook schema, and the postCompact line says what post-compact.json says. Your schema edit changes only description text, so the wire shape stays the same (I compared the file). The merged tree on integration 19b1197 passes every suite.

@bar-capsule @afogel, this PR needs your answer on issue #146 first, and I won't repeat that question here. The PR also adds something that question doesn't cover, so I have one more.

hooks.md:258 says redaction of subagent output "MUST be enforced". Is that meant to be a new MUST? I'm asking for three reasons.

  • Every other redaction line in hooks.md is a MAY (lines 106, 132, 156, 232), and MODIFY is only SHOULD-support in conformance.md:98. The sentence also doesn't say who must do it (the Guardian, the client or the deployment).
  • In PR #22, the adapters can't do this at the hooks the sentence names. Claude Code treats agentResponse and toolCallResult as observation-only (adapters/claude-code/mapping.md:45). Cursor does the same for agentResponse (adapters/cursor/mapping.md:74), and it can rewrite tool output for MCP tools only (adapters/cursor/mapping.md:55). A subagent that ends as failed, cancelled or timeout can also still carry a summary (the schema allows it), and its agentResponse may never have fired.
  • The other path, "when the parent subsequently acts", names no hook. By then the text is already in the parent's context, and a MODIFY on a later step changes only that step.

Issue #146 asks for the consequence to be recorded, and I don't think that takes a MUST. If you agree, this text would record it. It's your decision.

`subagentStop` gives the parent no gate on the returned content. A deployment that redacts subagent output does it at the subagent's own `toolCallResult` or `agentResponse`, before the output reaches the parent.

Once you've answered both, Raghuvar can push, and one of you can approve and merge. That merge closes issue #146.

Raghuvar, here is what's waiting and what isn't.

These wait on other people, so there's nothing for you to do on them:

  • Bar's or Ariel's answer on issue #146, and their answer on the MUST above.
  • PR #338 and PR #340. All three PRs add a test at the end of tests/test_hook_taxonomy.py, so this PR lands after those two.

These don't wait:

  • Removing the phrase and your postCompact and turnEnd labels are right under either answer on issue #146. Only the subagentStop text depends on it.
  • Asks 1 to 3 below are yours, and you can make them now. Please push only once, after both items above are done, so you only rebase once.

If the answer is Audit-only, asks 1 to 3 don't change. If it's the postCompact rule, the subagentStop parts of asks 1 to 3 change, and this PR also needs the §6.5 change that issue #146 describes.

  1. docs/topics/faq.md:46 still lists postCompact as audit-only, but your hooks.md line says MODIFY is allowed there. The two pages disagree, which is the same problem as issue #146. The FAQ landed the day after you opened this PR, so you couldn't have seen it. Replace the sentence that starts "The other five" with this:

    The other five fire after the fact. At `postCompact` the Guardian can still rewrite the summary with `modify`. The other four are audit-only records: `subagentStop`, `skillUnload`, `turnEnd`, and `sessionEnd`.
    
  2. hooks.md now says "ALLOW only" for subagentStop and turnEnd, but those two schemas rule out only deny and modify. §6 has five dispositions, so the schemas leave ask and defer open. Please change both:

    • subagent-stop.json:5, replace returns 'allow' (cannot return 'deny' or 'modify'). with returns 'allow'. No other disposition is legal.
    • turn-end.json:5, replace so the Guardian writes a ContextEntry but cannot return 'deny' or 'modify'. with so the Guardian writes a ContextEntry and returns 'allow'. No other disposition is legal.

    Please leave skillUnload and sessionEnd as they are. Issue #148 covers those.

  3. Your test at tests/test_hook_taxonomy.py:330 never compares subagent-stop.json with hooks.md, although its name says it does. I broke your text 12 ways, and it caught 5. It passed when I changed subagentStop in hooks.md to "ALLOW / MODIFY", when I changed the schema to "MAY return 'modify'", and when I put "not decision-eligible" back in lower case. Replace it with this test, which also checks asks 1 and 2. It failed on all 8 ways I broke the new text. End the file with one newline.

    def decision_line(hook):
        page = HOOKS_PAGE.read_text(encoding="utf-8")
        section = page.split(f"\n## {hook}\n")[1].split("\n---")[0]
        return next(line for line in section.splitlines() if line.startswith("**Decision:**"))
    
    
    def test_post_event_hooks_carry_one_label_on_every_surface():
        """Issue #146: each rule has one label, and every surface says the same thing."""
        hooks_text = HOOKS_PAGE.read_text(encoding="utf-8")
        schema_texts = [path.read_text(encoding="utf-8") for path in HOOK_SCHEMAS.glob("*.json")]
        for text in [hooks_text, *schema_texts]:
            assert "not decision-eligible" not in text.lower()
    
        assert "| ALLOW / MODIFY (no DENY" in hooks_text
        assert decision_line("postCompact").startswith("**Decision:** ALLOW / MODIFY (DENY not permitted)")
        for hook, schema in (("subagentStop", "subagent-stop.json"), ("turnEnd", "turn-end.json")):
            assert decision_line(hook).startswith("**Decision:** Audit only (ALLOW only)")
            description = json.loads((HOOK_SCHEMAS / schema).read_text(encoding="utf-8"))["description"]
            assert "Audit-only" in description
            assert "No other disposition is legal" in description
    
        faq = (DOCS / "topics" / "faq.md").read_text(encoding="utf-8")
        assert "`postCompact`" not in faq.split("audit-only records:")[1].split(".")[0]
  4. Please rebase after PR #338 and PR #340 land. Git will show a conflict in tests/test_hook_taxonomy.py. The conflict is only about position, so keep every test. I tried this on a scratch copy with all three PRs: 927 tests collected, 926 passed, 1 skipped (the usual one). Asks 1 to 3 pass there too.

    Test: uv run pytest tests/test_hook_taxonomy.py passes, and the PR #338, PR #340 and PR #341 tests are all in the file.

Issue #146 has no assignee yet, because GitHub still won't let me assign you. It's the same step as on your other PRs, so please leave a comment on the issue. After you push, I'll run all of this again.

…p as Audit-only

Stop overloading 'Not decision-eligible' for both MODIFY-eligible
(postCompact) and audit-only (subagentStop, turnEnd) hooks in hooks.md,
docs/topics/faq.md, hooks/subagent-stop.json, and hooks/turn-end.json;
label postCompact explicitly as ALLOW / MODIFY (DENY not permitted);
and state 'No other disposition is legal' on subagent-stop.json and
turn-end.json.

Fixes GenAI-Security-Project#146

Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>
@drk-ram

drk-ram commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

@rocklambros Rebased onto integration (1b8f970) after PR #338 merged, applied asks 1 to 3, and updated the subagentStop sentence to your non-MUST wording in b3f6e95 (pytest tests/test_hook_taxonomy.py passes).

@drk-ram
drk-ram force-pushed the spec/decision-eligibility-wording-146 branch from 9fbd829 to b3f6e95 Compare October 31, 2026 23:39

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[Bug] "Not decision-eligible" is overloaded, and subagentStop is labelled inconsistently between its schema and hooks.md

2 participants