Skip to content

0.3.4: keep the verdict channel clean, read Copilot CLI's camelCase, keep PowerShell exit codes - #161

Merged
jothimani-rajendran merged 16 commits into
mainfrom
fix/review-2026-09-27
Sep 27, 2026
Merged

jothimani-rajendran merged 16 commits into
mainfrom
fix/review-2026-09-27

Conversation

@jothimani-rajendran

Copy link
Copy Markdown
Collaborator

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:

  • While a handler runs, sys.stdout and fd 1 are redirected to stderr, so a print() ahead of the JSON verdict no longer turns a deny into a non-blocking error (witnessed live).
  • Copilot CLI's native camelCase payload (toolName, toolArgs) is read, and a camelCase preToolUse is answered with a top-level permissionDecision.
  • Claude Code's PowerShell tool is a shell tool.
  • Codex hooks: needs_trust/trust_hint are recorded, and apply_patch is its write tool.
  • A PowerShell hook keeps its exit code.
  • install() writes the matcher only at tool events and never clobbers a user's non-list event value.
  • The repo's pre-commit hook picks a Python that actually runs.
  • Devin no longer claims PermissionRequest by name.
  • The probe refuses a missing agent binary.

Review round, this session: an adversarial review focused on fail-open paths found these, now fixed:

Severity Finding Commit
High The new ; exit $LASTEXITCODE suffix exited 0 when the interpreter was not on PATH ($LASTEXITCODE is $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. 590cb63
High Text a handler wrote to a stream it held on to (sys.__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. 25d01fa
Medium With fd 2 closed, dup(1) returned 2, and the diversion pointed stdout back at itself. It now goes to devnull. 25d01fa
Medium A Copilot CLI payload with toolName but no toolArgs was claimed by no adapter, so it was allowed as "unrecognized". It is now claimed and judged. toolArgs JSON text with leading whitespace is decoded too. 6578e11

Each fix has a test that fails without it.

Claim check

Checks

  • pytest -q passes (1888 passed, 4 skipped)
  • ruff check . and ruff format --check . pass; examples/generate.py --check is up to date
  • Runtime path is still stdlib-only
  • Commits are signed off

Notes for the reviewer

Follow-ups not addressed here:

  • toolArgs that are present but not an object reach the handler as empty; nothing flags them as unreadable.
  • A Codex apply_patch write carries its path only inside event.command, so a path-based policy must read command.
  • A Devin PermissionRequest without prompt_id is undetected. The only evidence that Devin sends prompt_id is the test fixture.
  • A forked, non-exec'd child that stays alive delays the verdict. This predates the branch.
  • tools/literal_duplication.py never reads its allowlist, so it exits 1 on main too. CI does not run it.

chock's adoption PR re-uses the PowerShell suffix for Copilot entries and needs the same $null guard.

🤖 Generated with Claude Code

https://claude.ai/code/session_016DapRw9Ce9z6jRd11uiuyu


Generated by Claude Code

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>
@jothimani-rajendran
jothimani-rajendran marked this pull request as ready for review September 27, 2026 18:34
@jothimani-rajendran
jothimani-rajendran merged commit d3e083e into main Sep 27, 2026
19 checks passed
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.

2 participants