0.3.4: keep the verdict channel clean, read Copilot CLI's camelCase, keep PowerShell exit codes - #161
Merged
Merged
Conversation
A handler that printed (or ran a child process that printed) put text ahead of the JSON verdict on stdout. Claude Code and Gemini CLI then fail to parse the hook output and treat it as a non-blocking error, so a deny became an allow (witnessed live). While the handler runs, sys.stdout is redirected to sys.stderr and fd 1 is pointed at fd 2 (devnull when there is no stderr), so only the verdict reaches the real stdout. The bundled single-file runtime (runtime.py.tmpl) gets the same helper; bundler hoists contextlib, io and os for it. Tests: tests/test_handler_stdout.py (in-process run/handle, a real hook process whose handler and child print, and the bundled runtime). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
…level form
Copilot CLI's native camelCase hooks send {sessionId, timestamp, cwd,
toolName, toolArgs} with no event name. parse() read only tool_input,
so command/path/content arrived as None and every guard allowed the
call; detect() did not claim the payload at all.
- parse toolArgs (an object per the docs, JSON text in earlier builds)
when tool_input is absent; sessionId and toolResult.textResultForLlm
fill session_id/output; a toolResult marks an unnamed payload as
postToolUse
- claims() accepts an unnamed payload carrying toolArgs
- camelCase preToolUse answers with the documented top-level
{permissionDecision, permissionDecisionReason}; it has no input
rewrite, so a transform blocks with the reason saying so
Source: https://docs.github.com/en/copilot/reference/hooks-configuration
(preToolUse input/output, read 2026-09-27). Vendor record recounted;
the fields claim now cites vendor-docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Claude Code on Windows runs shell commands through its PowerShell tool (on by default on Windows; the docs say to match `Bash|PowerShell`), so a guard keyed on the recorded shell tools never saw those calls. Its hook input carries the command in tool_input.command, the same field the existing chain reads, so parsing needed no change. claude_code tools.shell is now [Bash, PowerShell]; the tools claim cites the vendor docs (PowerShell not yet seen live). No other code assumed a single shell tool name (the probe's reference agent only synthesises Bash calls). Sources: https://code.claude.com/docs/en/tools-reference , https://code.claude.com/docs/en/hooks (read 2026-09-27). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
… as the write tool Codex loads project-local hooks only once the project .codex/ layer is trusted, and records trust per hook hash, so a new or changed hook is skipped until the user trusts it with /hooks. needs_trust was false, and nothing printed it anyway: `agentseam install` reported a gate as wired that Codex would not run. - codex_cli needs_trust: true, with a new optional trust_hint vendor fact (schema + evidence record, pairing pinned in tests/test_vendor_config.py) - `agentseam install` and `doctor` print "not live until trusted: ..." for any agent whose NEEDS_TRUST is set (codex_cli, grok) - codex_cli tools.write: [apply_patch]. PreToolUse fires for it and the patch rides in tool_input.command, which the existing chain already parses into event.command (fixture in the documented shape) Source: https://learn.chatgpt.com/docs/hooks ("Project-local hooks", tool coverage), read 2026-09-27. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
…nine The review asked to drop gemini_cli's timestamp reject (Gemini's documented base input carries timestamp) and have detect() decline when Gemini and Tabnine both claim. Not applied as asked: Tabnine CLI sends the identical shape, and an ambiguous detect() allows silently, which would turn every unpinned Gemini deny into an allow. Resolving to Tabnine is fail-safe: allow/deny are byte-identical on the wire, and ask/transform (which only Gemini honours) degrade to deny. The reject stays as a documented tie-break; transcript_path was never rejected. - gemini_cli claims gain a notes entry saying exactly that (recounted) - tests: a documented-shape BeforeTool payload parses and gets its ask when gemini_cli is named, and auto-detection is never weaker than gemini_cli for deny/allow/escalate/transform needs_trust is left false: the docs say changed project hooks are fingerprinted and "you will be warned before it executes", not that they do not run. Source: https://geminicli.com/docs/hooks/ and /docs/hooks/reference/ (read 2026-09-27). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
devin's accept_names listed PermissionRequest as a name Claude Code
never sends; Claude Code does send it (a tool event whose deny goes
through hookSpecificOutput.decision), so a Claude Code PermissionRequest
was handed to Devin's top-level {"decision": "block"} dialect.
PermissionRequest now takes the marker path like every other Devin
event: prompt_id, which Devin documents as absent only before the first
user prompt, so a permission request always carries it. The fixture is
updated to that documented shape; a payload without it is ambiguous and
nobody guesses. Schema descriptions that cited the old example updated.
Sources: https://code.claude.com/docs/en/hooks ,
https://docs.devin.ai/cli/extensibility/hooks/overview (read 2026-09-27).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
…es, refuse to clobber - a tool-name matcher is written only at pre_tool/post_tool/tool_failure. Elsewhere vendors ignore it or read it as something else (Claude Code's SessionStart matcher is the session source), so `--matcher Bash` silently stopped stop/session hooks from firing. Golden hook_config fixtures regenerated - uninstall (and the strip before a reinstall) drops the event lists and objects its own removal emptied; a user's own empty value stays - merging into an existing config refuses with ConfigUnreadableError when the user's value at an event is not a list (or not an object), instead of overwriting it; the CLI reports that as a skip - the JSON config is written in the file's own key order rather than re-sorted Tests: tests/test_install_merge.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
…es not model
- drive_real: a shell exit of 127 (POSIX) or 9009 (cmd.exe) means the
driver's CLI was not found; that used to reach the classifier as zero
hook invocations and read "the hook never fired -- config path or
format is wrong", blaming the config for a missing install. It now
raises DriverNotFoundError ("driver binary not found ...")
- run_trial with the reference driver refuses any agent outside
reference_agent.MODELLED_AGENTS (claude_code): it reads only Claude
Code's settings shape, so every other agent scored "never fired"
Tests: tests/test_experiment_driver.py.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
`command -v python3` finds the Microsoft Store's python3 stub on Windows, which exits non-zero; under `set -e` the hook then failed every commit that touched src/. The hook now takes the first of python3, python, py that runs a Python 3.9+ (`-c` version check), and still warns and exits 0 when none does. Test: tests/test_git_hooks.py builds a PATH whose python3 is a failing stub and a real python behind it, and commits through the hook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
…vscode_copilot row
The row's notes described only the VS Code dialects. They now say the
CLI's camelCase preToolUse carries no event name ({sessionId,
timestamp, cwd, toolName, toolArgs}), is claimed by toolArgs, and is
answered with a top-level permissionDecision -- as vendor docs, not a
live witness. No grade changes. Generated example page refreshed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Version bumped in pyproject.toml, agentseam.__version__ and CITATION.cff; social-preview.svg regenerated (the PNG is left for a machine with the Geist font installed, as CI checks the SVG only). CHANGELOG entry for the review fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
`pwsh -Command` exits 1 whenever the last native command failed, whatever its code, so a hook's exit 2 (block) reached Codex as 1, a non-blocking error (openai/codex#48183). powershell_command() now ends with `; exit $LASTEXITCODE`, idempotently. Golden fixtures and generated examples re-derived. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
`exit $LASTEXITCODE` exits 0 when no native command ran (python not on PATH): $LASTEXITCODE is $null, so a missing interpreter became an allow where `& cmd` alone had exited 1. The suffix now exits 2 then, and an entry carrying the earlier bare suffix is upgraded, not suffixed twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
A handler writing to sys.__stdout__ or a stream cached at import filled a buffer redirect_stdout never touched; it flushed at exit, after the verdict, and the host could no longer parse the JSON. Flush every stdout stream before restoring fd 1. With fd 2 closed, dup(1) returned 2 and the diversion pointed stdout back at itself; divert to devnull then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
A camelCase payload with toolName but no toolArgs was claimed by no adapter, so it was allowed as unrecognized before any handler ran. toolArgs JSON text with leading whitespace is decoded too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
This fixes several paths where a deny, or a call that was never judged, reached the agent as an allow. It releases as 0.3.4. The main fixes:
sys.stdoutand fd 1 are redirected to stderr, so aprint()ahead of the JSON verdict no longer turns a deny into a non-blocking error (witnessed live).toolName,toolArgs) is read, and a camelCasepreToolUseis answered with a top-levelpermissionDecision.PowerShelltool is a shell tool.needs_trust/trust_hintare recorded, andapply_patchis its write tool.install()writes the matcher only at tool events and never clobbers a user's non-list event value.PermissionRequestby name.Review round, this session: an adversarial review focused on fail-open paths found these, now fixed:
; exit $LASTEXITCODEsuffix exited 0 when the interpreter was not on PATH ($LASTEXITCODEis$null), which is an allow. Before the suffix,&alone exited 1. The suffix now exits 2 when no native command ran, and an entry carrying the bare suffix is upgraded. Confirmed in pwsh 7.4.590cb63sys.__stdout__, or a reference cached at import) stayed buffered and was flushed at exit, after the verdict, so the JSON no longer parsed. Every stdout stream is now flushed while fd 1 still points at stderr.25d01fadup(1)returned 2, and the diversion pointed stdout back at itself. It now goes to devnull.25d01fatoolNamebut notoolArgswas claimed by no adapter, so it was allowed as "unrecognized". It is now claimed and judged.toolArgsJSON text with leading whitespace is decoded too.6578e11Each fix has a test that fails without it.
Claim check
MATRIXrows carry averifiedrecordpreToolUse) and Windows: PreToolUse hook exit code 2 does not block; hook commands run underpwsh -Command, which reports exit 1 openai/codex#48183 (PowerShell exit code)Checks
pytest -qpasses (1888 passed, 4 skipped)ruff check .andruff format --check .pass;examples/generate.py --checkis up to dateNotes for the reviewer
Follow-ups not addressed here:
toolArgsthat are present but not an object reach the handler as empty; nothing flags them as unreadable.apply_patchwrite carries its path only insideevent.command, so a path-based policy must readcommand.PermissionRequestwithoutprompt_idis undetected. The only evidence that Devin sendsprompt_idis the test fixture.tools/literal_duplication.pynever reads its allowlist, so it exits 1 onmaintoo. CI does not run it.chock's adoption PR re-uses the PowerShell suffix for Copilot entries and needs the same
$nullguard.🤖 Generated with Claude Code
https://claude.ai/code/session_016DapRw9Ce9z6jRd11uiuyu
Generated by Claude Code