Skip to content

Pin the invariant that a public-review stage never dispatches through the CoS runner #6105

Description

@atomantic

Problem

spawnAgentForTask forces every public-review stage down the direct-CLI path with one line:

// server/services/agentLifecycle.js
const dispatchUseRunner = publicReview ? false : useRunner;

Nothing pins that line. It is now more load-bearing than when it was written, because the two halves of a vendor's no-tool posture are enforced in different places:

  • argv is declared on the vendor row and resolved inside buildVendorSpawnConfig, so it survives any dispatch path.
  • OpenCode's posture lives entirely in OPENCODE_CONFIG_CONTENT, applied only by buildCliChildEnv from the safetyProfile it is handed. The CoS-runner payload (composeProviderEnv at spawnAgentViaRunner) does not carry safetyProfile.

So if a public-review stage ever reached the runner, the stage would still look enforced — right agent flag, right provider, gate passes — while running tool-enabled against contributor-authored PR text. Silent, and green in CI.

Why this wasn't fixed inline

agentLifecycle.postureGate.test.js is the right observing surface, but its mock set deliberately stops the spawn at prepareAgentWorkspace (outcome: 'blocked'). Reaching the dispatch requires standing up materializePublicReviewInput, readPublicReviewInputSnapshot, buildAgentPrompt, createAgentRun, and a writable agent directory — a scaffold larger than the change that surfaced this.

Note that the sibling test spawns a public-review stage on a TUI provider headless, never as a PTY session has the same limitation: its negatives pass because the flow never reaches a spawner at all.

Work

  1. Extend the mock set in server/services/agentLifecycle.postureGate.test.js so a public-review spawn reaches the dispatch (temp AGENTS_DIR, resolved snapshot/prompt/run mocks).
  2. Add a case: a public-review stage carrying useRunner: true must call spawnDirectly and never spawnAgentViaRunner.
  3. While there, make the existing TUI test non-vacuous the same way — assert spawnDirectly was called, not only that the TUI spawner was not.
  4. Decided, not deferred: do NOT thread safetyProfile into the runner payload as a belt. Public-review stages are direct-only by design (the runner is a shared process that may inherit ambient tool configuration — see the comment above dispatchUseRunner); adding the thread would make routing them through the runner look supported. Pin the invariant instead.

Acceptance

  • The new test fails when dispatchUseRunner is changed to plain useRunner, and passes on main.
  • No production change.

Activity

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

Metadata

Metadata

Assignees

Labels

effort:mediumEffort: mediummodel:mediumModel size: mediumplanTracked by /do:replanplanner:opus-5Plan authored by the opus-5 modeltestsTest suite / test infrastructure

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions