feat(externagent): state each provider's episode lockdown, and refuse one that cannot be stripped - #532
Merged
Merged
Conversation
one that cannot be stripped Both bundled adapters lock their CLI's native surface down correctly, and both do it inside the argv their Command builds, where nothing outside the adapter can read the posture and nothing checks it is still there. A third adapter that omits the lockdown produces episodes that look governed in the record. Adapter now states a Lockdown: how native writes and native commands are taken away, and whether the operator's own configuration reaches the child. It names the mechanism (denied by the CLI's own controls, or contained by the boundary) rather than the flag, since the binary, the flag spelling and the tool names are the parts that do not transfer between providers. The runner reads it before it binds or spawns anything and refuses a provider it does not strip, naming every class that failed. The zero value is a refusal, so an omitted declaration cannot pass for a lockdown. The two adapters reach different verdicts from the same contract: claude denies its effectors outright, codex has no flag for its shell or patch tools and declares containment. Tests check each declaration against the argv that adapter actually builds, so the statement cannot drift from what it describes. An episode also no longer runs on the operator's codex config. Seeding config.toml handed the child the MCP servers, model providers and settings of whoever launched the run; only the credential is seeded now, and everything an episode needs is passed on the command line. Signed-off-by: Ion Alpha <contact@ionalpha.io>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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
externagent.Adapternow states aLockdown: how the CLI's native writes and native commands are taken away for an episode, and whether the operator's own configuration reaches the child. The runner reads it before it binds or spawns anything and refuses a provider it does not strip, naming every class that failed. The zero value is a refusal, so an adapter that declares nothing is treated exactly like one that cannot strip its surface.An episode also stops running on the operator's codex configuration: only the credential is seeded into the per-episode home now, not
config.toml.Why
Both bundled adapters lock their CLI down correctly, and both do it inside the argv their
Commandbuilds. Nothing outside the adapter can read the posture, and nothing checks that it is still there, so the rule is a property of who wrote the adapter rather than of the port. A third adapter that omits it produces episodes that look governed in the record.The lockdown is also the part of an integration least likely to transfer. The binary, the flag spelling and the tool names are all provider-specific, so a rule written as "pass
--disallowedTools Edit Write" holds on exactly one CLI and silently holds on none of the others. The contract names the mechanism instead: denied by the CLI's own controls, or contained by the boundary. The two bundled adapters reach different verdicts from it, which is the point: claude denies its effectors outright, and codex, which has no flag for its shell or patch tools, declares containment.config.tomlis where a codex user keeps their MCP servers, model providers and settings. Seeding it into the episode home let whoever launched the run steer the harness, and gave the child servers this run neither hosts nor governs. Everything an episode needs is passed on the command line, so the child's configuration is now a function of the run.How to verify
go test ./externagent/... ./cmd/flynn/...The new tests are the contract: every bundled adapter must declare a stripped lockdown, each declaration is checked against the argv that adapter actually builds (claude's denial list and permission mode, codex's read-only sandbox and denied approval path), and the runner refuses both an unstrippable provider and an undeclared one without spawning anything.
Notes for reviewers
Adding a method to
externagent.Adapteris an incompatible change on the domain surface, which./dev/apidiffreports rather than gates. It is deliberate: an optional interface would make the omission silent, which is the failure this closes.Dropping
config.tomlfrom the seed changes behavior for anyone whose codex config sets something the episode relies on. A model comes from the run and the rmcp client is set on the command line, so the case left is a non-default model provider, which an episode should not inherit from the host anyway.The three
cmd/flynnexternal-agent tests fail on a machine that hascodex.cmdinstalled without the vendored binary beside it. That reproduces onmainand is unrelated to this change.