Skip to content

fix(external-cli): record where safety values come from - #3199

Merged
nicobailon merged 10 commits into
nicobailon:mainfrom
drye:fix/external-cli-declared-safety
Nov 5, 2026
Merged

nicobailon merged 10 commits into
nicobailon:mainfrom
drye:fix/external-cli-declared-safety

Conversation

@drye

@drye drye commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

Every code-owned external CLI run (codex-exec*, claude-code*, cursor-agent*) records a safety block. resolveExternalCliRunnerStatus fills it in from the adapter id alone. It is the access the adapter's own argv asks for, not something read from the launched process. Status (subagent({ action: "status" })) and workflow receipts still present it as fact.

runner.command can point at a wrapper script that changes argv before it starts the CLI. Profiles cannot add argv, so a wrapper is how people grant a child something different. When one is used, the record is wrong and says nothing about the wrapper:

  • A claude-code-writer agent whose wrapper removes the editors and adds Bash, WebFetch, WebSearch and an MCP server still records tools=Read,Write,Edit,Glob,Grep and mcp=empty-strict.
  • A wrapper that adds Bash and keeps acceptEdits also records no Bash.
  • The block is identical before and after changing what the wrapper grants, so the run record can't tell you which grant a run had.
  • workflow-receipt.ts requires the profile literals (tools !== "Read,Write,Edit,Glob,Grep" throws), so an accurate record would be rejected.

How to reproduce

  1. Write an agent with runner: { type: external-cli, adapter: claude-code-writer, command: <script> }, where the script runs claude with --tools Read,Glob,Grep,Bash in place of the adapter's --tools value.
  2. Run it and call subagent({ action: "status", id }).
  3. Status prints tools=Read,Write,Edit,Glob,Grep, mcp=empty-strict, and the child could run Bash. The workflow receipt entry has the same values.

What this changes

  • Runner status and receipt metadata now carry safetySource: { measured: false, command, declaredFields } next to safety. command is the launch command as configured, so a wrapper shows up in the record. declaredFields names the fields that did not come from the adapter profile.
  • A new runner.declaredSafety field, accepted in agent frontmatter and runtime agent definitions, lets the agent author state what the wrapper grants. Its values replace the profile values for those fields only. Validation is strict: it needs a code-owned adapter and at least one field. Keys must be fields that adapter already records (network: open is rejected with the list of allowed keys). String fields need a non-empty trimmed string of up to 512 characters, and boolean fields need a boolean. Fields left out keep the profile values.
  • The receipt check still applies. Every field not in declaredFields must equal the adapter profile, so a receipt that changes tools without listing it in declaredFields is rejected, as is one with measured: true or an unknown declared field. Unwrapped runs get the same checks as before. The check confirms that a record is consistent and says where its values came from. It cannot reject a record that is consistent but wrong, such as a wrapper run with no declaration, because it never sees the launched process.
  • A run placed on a Herdr saved machine launches the adapter's own argv in a remote pane and never runs runner.command. An agent with declaredSafety is therefore refused before launch when a saved machine is requested, and the pane path throws if one reaches it.
  • Status prints declared values as Safety (declared tools, mcp): ... instead of Safety: ..., and adds a Safety source: line naming the current adapter profile (status rebuilds undeclared fields from today's profile), the command and "not measured from the launched process". The existing Safety: line format is unchanged for runs with no declared fields.
  • The six per-adapter profile literals move into one EXTERNAL_CLI_ADAPTER_SAFETY table used by both the resolver and the receipt parser. That replaces five near-identical branches in workflow-receipt.ts. The Claude Code legacy receipt branch and the legacy grok-build branch are kept as they were, and neither accepts safetySource.

Compatibility

New runs always write safetySource. On read, a missing safetySource is treated as "all fields from the profile", which is what those records meant. There is no legacy branch for this: it is one optional field, and requiring it would make existing receipts unreadable for resume and inspection. Status and receipt consumers that read safety get the same keys as before. Only a declared run has different values, and that run is flagged by safetySource.declaredFields.

Out of scope

  • Measuring the grant. Nothing here inspects the launched process or the wrapper. declaredSafety is the agent author's statement, and the docs say so. Detecting a wrapper automatically, for example by comparing the command to the expected binary, would be a guess.
  • declaredSafety does not change the launch argv. User profiles still cannot add argv.
  • agent-management create/update does not accept declaredSafety, just as it does not accept capabilities today. Declared agents are written as frontmatter files or runtime definitions.
  • The generic external-cli runner (no adapter) records no safety and cannot declare one.

Docs

docs/agents.md had no description of the safety record. This adds a new "Safety record and wrapper commands" section covering what safety is, safetySource, declaredSafety with an example and its validation rules, how status labels it, and the receipt rule. docs/tool-reference.md gets a short paragraph in "External CLI agent profiles" that links to it. One CHANGELOG.md entry under Fixed.

Tests

  • npm run typecheck: clean.
  • npm test: 4327 tests, 4310 pass, 0 fail, 17 skipped by the suite. test/unit/host-peer-runtime-imports.test.ts has been reported as failing on a clean main. It passed in my runs on this branch and I made no changes for it.
  • New tests in test/unit/claude-code-adapter.test.ts:
    • The profile record carries the command and measured: false.
    • Declared fields merge over the writer profile and survive a status round-trip and a receipt write and read.
    • Receipts are rejected for undeclared drift, for declaredFields emptied while values still differ from the profile, an unknown declared field, an empty declared value, measured: true, and missing safety. An empty declaredFields with profile values stays valid, since every unwrapped run writes it.
    • Persisted status with an invalid declared field is treated as invalid runner metadata.
    • Frontmatter accepts declared fields and rejects an unknown field, the wrong value kind, an empty object, and a generic runner.
  • New test in test/unit/herdr-machine.test.ts: saved-machine launch gating rejects an agent with declaredSafety. It fails with the gating change reverted.
  • New test in test/unit/run-status.test.ts: the status output labels a profile run as an unmeasured profile with its command, and labels a declared run's fields without printing a bare Safety: line.
  • One existing fixture in claude-code-adapter.test.ts that builds a legacy Claude receipt now also removes safetySource, because real legacy receipts never had it.

🤖 Generated with Claude Code

drye and others added 2 commits November 5, 2026 19:23
…ers declare theirs

The safety block on a code-owned external CLI run is the adapter's argv
profile, but status and workflow receipts presented it as fact. When
runner.command is a wrapper that rewrites argv, the record was wrong in
both directions: a claude-code-writer run whose wrapper dropped the
editors and added Bash, WebFetch, WebSearch and an MCP server still
recorded tools=Read,Write,Edit,Glob,Grep and mcp=empty-strict, and the
record did not name the command that launched it. The receipt parser
required those literals, so an accurate record could not be written.

- Runner status and receipt metadata carry
  safetySource: { measured: false, command, declaredFields }.
- runner.declaredSafety (agent frontmatter and runtime definitions)
  overrides profile fields for that run. It needs a code-owned adapter,
  accepts only fields that adapter already records, and checks each value
  kind against the profile.
- Receipts accept declared values but still require every undeclared
  field to equal the profile. Receipts without safetySource are read as
  the profile, as before.
- Status labels declared values and prints a Safety source line naming
  the launch command.
- The per-adapter profiles move to one EXTERNAL_CLI_ADAPTER_SAFETY table
  that both the resolver and the receipt parser use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aredSafety

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] No new code issues were found; required CI on the exact head must be confirmed before merging.

Summary

The PR marks external CLI safety values as an unmeasured adapter profile and records the configured command for local runs.

  • External CLI safety records now show where profile values come from.

Reviews (6) · Last reviewed commit: "test(external-cli): check a placed run's..." · Reviewed by Greptile

Comment thread CHANGELOG.md Resolved
drye and others added 5 commits November 5, 2026 19:32
A run placed on a Herdr saved machine launches through the internal pane
adapter, which builds the adapter's own argv and never runs
runner.command. Recording a declared wrapper grant and that command for
such a run made two false statements. Launch gating now refuses an agent
with runner.declaredSafety when a saved machine is requested, and the
pane path throws if one reaches it.

Status now says "current <adapter> adapter profile", because it rebuilds
undeclared fields from today's profile, not from the persisted record.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…o fix/external-cli-declared-safety

# Conflicts:
#	CHANGELOG.md
Keep safetySource { measured: false, command } beside the adapter profile
safety block and the shared EXTERNAL_CLI_ADAPTER_SAFETY table. Drop the
runner.declaredSafety field: it replaced code-owned profile values with
unverified author text that nothing checks or acts on.

A run on a Herdr saved machine records no command, because the remote pane
runs the adapter's own binary, never runner.command. Status labels the block
as the adapter profile instead of adding a second line repeating the command.
@nicobailon

Copy link
Copy Markdown
Owner

Thanks for this. The safety record really did present the adapter profile as fact, with no sign that a wrapper was in play. I pushed two commits that keep that fix and drop runner.declaredSafety:

  • Kept: safetySource: { measured: false, command } beside safety in status and receipts, the EXTERNAL_CLI_ADAPTER_SAFETY table, and the table-driven receipt check. Status now labels the line Safety (adapter profile, not measured):. The Runner: line above it already prints the command.
  • Dropped: declaredSafety. Nothing reads safety to decide anything; it is only displayed and validated. A declared value would swap one unverified value for another through a new public field and receipt rules. With the command recorded, the wrapper itself is where the real grant can be checked.
  • Fixed: a run on a Herdr saved machine recorded runner.command, which the remote pane never runs (it runs the adapter's own binary). Those runs now record safetySource: { measured: false } with no command.

16 files (+305 −72) → 9 files (+127 −62). Your commits and the changelog credit stay; it will be squash-merged.

… status

The status-step runner of a Herdr saved-machine run was built without the
machine, so status.json (and receipts that fall back to it) named the local
runner.command, which the remote pane never runs. Pass the step's machine
through. Also name safetySource in the generic-adapter receipt error and
keep the status safety label in one constant.
…ing ssh

The test ran a real Herdr launch against an unreachable host. The launch
failure rejects runSubagent before the runner closes its control inbox, and
on Linux that inbox is a native fs.watch handle, so the test file never
exited and the CI shard hit its time limit. A stop requested before the
runner starts ends the run before the placed launch instead.
@nicobailon
nicobailon merged commit 3caee85 into nicobailon:main Nov 5, 2026
19 checks passed
@nicobailon nicobailon changed the title fix(external-cli): record where safety values come from and let wrappers declare theirs fix(external-cli): record where safety values come from Nov 5, 2026
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