Repository navigation
[safety] Nothing decides whether an agent may merge to main — three unattended merges already happened #314
Copy link
Copy link
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
Observed
Run 73ffc073 on the FWS Repo Coder, from a single delegated objective, unattended:
- Uncommitted work — committed to branch
feat/uncommitted-work, PR Agent work as first-class Tickets — granular, with reasoning + per-ticket conversation #150 opened and merged to main.- Issue CODER-007 — (Stretch) Real-time status push instead of polling (Durable Object / relay) #83 — … PR Existing instances stuck on a non-tool-capable model — #100 fix only covers NEW agents, and there's no way to change an instance's model #151 opened and merged.
- Issue CODER-006 — (Optional) Aggregate live session-status endpoint (one call for all sessions) #82 — … PR Voice: unmute command word — dictation-based wake-to-record with configurable keyword and start-recording signal #152 opened and merged. Issue closed.
- All open bug issues exhausted —
gh issue list --label bug --state openreturns[].
Three merges to main of a real repository, plus an issue closed, with no human in the loop. The
objective did say "merge each before starting the next" — so the agent did what it was told. That
is the point: nothing decided whether it was allowed to.
The gap
There is no notion of merge authority anywhere:
- The engine runs
claude --dangerously-skip-permissions/codex exec --sandbox danger-full-access,
sogh pr mergeis simply available. delegate_goalis write-scoped and consent-gated ([connectors] Connector consent + write-scope safety + admin visibility #90) at the connector level — but consent is
"may this agent use the GitHub connector", not "may it merge to a protected branch".- [coordination] Autonomous work budget — bound spend across every unattended entry point #184 bounds spend across a delegation tree. Nothing bounds irreversibility.
- The supervisor view does not report that a merge happened. I only found these by reading run
detail after the fact.
Spend is recoverable. A merge to main is not, and neither is a closed issue.
What this is not
Not an argument that autonomous merge is wrong. For a solo operator dogfooding their own repos it
may be exactly what is wanted, and forcing a review step on every trivial fix would make the Coder
much less useful. The problem is that it is neither chosen nor visible.
Proposal
- A declared merge policy per repo, defaulting to the safe end:
none(commit only) ·pr(open, never merge) ·merge(current behaviour).
Repo-level rather than agent-level, because the answer differs per repository — a dogfood repo
and a customer repo are not the same risk. - Surface it where work is reported. A run that merged should say so in its outcome and on the
board card. Today "PR Agent work as first-class Tickets — granular, with reasoning + per-ticket conversation #150 opened and merged" is buried in a rundetailstring nobody reads
unless they go looking. - Consider the same treatment for other irreversible acts the engine can reach: force-push,
branch deletion, closing issues,gh release. Merge is the one that has actually happened, so
it is the one worth doing first — the rest can follow the same declaration.
Verification
- With policy
pr, a delegated goal instructing "merge it" opens the PR and reports plainly that
merging is not permitted, rather than silently complying or silently refusing. - A run that merges records it in its outcome and on the board.
- Default for an existing repo is the current behaviour or
pr— decided deliberately, since
changing it silently is its own surprise.
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working
Activity
serge-ivo commentedon Aug 6, 2026
Two things found while reading the code, before anyone builds this
1. This cannot be a hard control — say so, or someone will ship it believing it is
The engine runs
claude --dangerously-skip-permissions/codex exec --sandbox danger-full-access.gh pr mergeis reachable from that shell, always. No string in a prompt changes that.So a merge policy is declared intent injected into the Pilot's instructions, plus detection after
the fact — it makes the boundary explicit and per-repo instead of unstated, and it is the layer a
real control would later enforce. It is not enforcement itself.
Worth writing into the code and the UI copy, because the failure mode of overclaiming here is
already on the board: #246 shipped a dry-run guard that inspected only
click, while the prompttold the model "Submit clicks are blocked anyway" — a guarantee that was true of one action type
and believed for all of them.
Real enforcement, if it is ever wanted, is a different ticket: a wrapped git credential, a
protected branch on the GitHub side, or a review gate the runner cannot bypass. Those are controls.
This is a norm, and the two should not be confused in the copy.
Corollary: detection is the half that actually holds. A run that merged must say so in its
outcome and on the board card regardless of policy — that part does not depend on the model
cooperating, and it is what turns "three merges happened and nobody knew" into something visible.
2. The clause must give the model a correct action, not a prohibition
A bare "do not merge" against an objective that says "commit, push, PR, and merge each one before
starting the next" is a contradiction, and #254 is the recorded consequence of handing this
model a contradiction: it obeyed the capitalised prohibition, denied work it had really done, and
told the user to do it by hand.
So the clause should read as a rule about what to do when the objective asks for more than the
policy allows. Roughly:
That last sentence matters: without it a policy refusal looks like
outcome: failedto thesupervisor, and the Lead will report a successful run as a failure.
Grounding for whoever picks this up
systemPrompt(goal)inlib/coding-loop.ts:172;CodingGoal(
:16) is the carrier — it already threadsspecialInstructionsanddryRunthe same way.coding_reposalready has aninstructions TEXT NOT NULL DEFAULT ''column (migration0031)as precedent for per-repo prompt text, so a
merge_policycolumn follows an established shape.PUT /:instanceId/coding/repos/:repoIdroute currently accepts onlynameandurls;it is the natural place to set the policy.
On the default
prrather thanmerge, and note this does change behaviour for repos whose agents mergetoday — which is the point: the current behaviour was never chosen, it was the absence of a
decision. A default of
mergepreserves the surprise and makes the setting decorative for everyonewho never finds it.
The costs are not symmetric: being wrong toward
prmeans a PR someone has to click; being wrongtoward
mergemeans an unreviewed commit onmain.serge-ivo commentedon Aug 6, 2026
Shipped in
da7f381.Where the gate sits
workers/api/src/lib/coding-authority.tsis the decision half; #294 was already the detection half. Policy per repo:merge(may merge to the trunk) ·pr(may open a PR, never merge or push tomain) ·none(commit only).Resolution is repo override → agent setting → platform constant, each one value to change:
PUT /v1/instances/{id}/coding/repos/{repoId}{"mergePolicy":"pr"};""clears it back to inherit. An unrecognised value is refused, not ignored — believing you tightened a policy you did not is worse than an error.merge_policysettingsSchemafield (migration0091), added to every agent declaring thecodingsurface. It renders in the console's existing "Agent settings" card, so there was no UI to build.DEFAULT_MERGE_POLICYincoding-authority.ts.Three layers, and only two are enforcement:
coding-loop.tssystemPrompt) — the policy, worded as overriding the objective, since "merge each before starting the next" is what a human typed. Advisory.runCodingLoop) — the Pilot's outgoing instruction is checked before it reaches the Engine. Deterministic, cloud-side, and the layer that would have stopped73ffc073. The refusal is fed back through the step log the loop already shows the brain, so it adapts; capped at 3 refusals so a heuristic false positive costs a stopped run with a clear reason rather than a silent spin; and announced in your chat thread, because silently refusing is the other failure mode this issue names.coding-session.tscapture + closing drain) — an act the policy forbids halts the run, writesact.unauthorizedto the trace atlevel: error, and raises aneeds_humanboard card. Checked in the closing drain too, because a coding run routinely ends with the merge — the incident's own shape.Reporting (item 2 of the proposal): the delegated run's
detail, its board card and the session timeline all carry the policy line whenever one is in force, with any breach named ahead of the ordinaryActs:summary — buried after "and 3 more" it would be the same invisibility this issue is about.What it does NOT do — please read this part
--dangerously-skip-permissionson your machine, sogh pr mergestays physically available to it. Layer 3 catches the merge after it happened and stops the run before the second. It bounds three merges to one; it cannot make the first not happen. Denying the command at launch (--disallowedTools) lives in the runner and is follow-up work.codexengine does not report what it ran, so this run's actual commands could not be checked."prdoes not forbidpush.force. Force-pushing a feature branch is ordinary in a PR workflow and a policy that broke rebasing would be switched off within a day. A force-push aimed at the trunk classifies aspush.force, notpush.trunk, so it is a known narrow hole.gh releasefollow the same declaration when someone wants them —nonealready coverspush.force/branch.delete, and issue-closing has no act kind yet.The default is yours to choose
I shipped
merge— today's behaviour, unchanged byte for byte. Every arm of the gate is a literal no-op under it: no prompt text, no screening, no forbidden acts, anddescribeAuthorityreturns null so no run claims protection it does not have. There is a test whose only job is to assert exactly that.That was deliberate, not a preference for permissiveness. Flipping the default to
prwould change how your Coder behaves on repos you never touched — work you rely on would start failing without you asking for it, which is the same unannounced-change problem this ticket is about, pointed the other way.My recommendation: set
pras the agent-wide setting, and leavemergeon the dogfood repos you actually want merging unattended. That inverts the risk correctly — the repos where autonomous merge is fine are the ones you can name, and everything you add later is safe by default. One field on the Coder's settings does it.CI status — stated plainly
No CI run verified this. GitHub Actions is in a declared major outage and there were 24 unverified commits on
mainwhen I started. Verified locally against the rebased tree:pnpm typecheck— clean (11 projects)npx vitest run— 293 files / 4498 tests passingnpx biome check— 14 errors / 283 warnings, identical to the pre-change baseline; 0 on files I touchedpnpm docs:drift— no drift (API coverage 282/282)node scripts/check-file-size.mjs— OK;routes/coding.tspin raised 1769 → 1774 on purpose, reason recorded in the PINS comment and the commit0091applied to a scratch SQLite fixture: appends to existing schemas, leaves non-coding agents alone, survives a legacyconfig = ''row, idempotent on re-runserge-ivo commentedon Aug 6, 2026
Closing: the mechanism is in and the setting is exposed. The remaining items (per-repo UI, runner-level command denial, the same declaration for force-push/branch-delete/issue-close/release) are follow-ups rather than parts of this, and are listed in the comment above.
fix(sql): a value spliced into a SQL literal is the only splice that …
fix(supervision,github): answer from the store that holds the answer (#…
refactor(api): the Coder control plane, split where a reader has to s…
test(api): drive all 39 coding routes as a stranger, and pin who pays (…
fix(coder): the Coding tab's Loop stops being the loop, and starts on…
feat(coding): a Pulls panel, and a GitHub cache whose key is a tenanc…
fix(coding): a repo's address is stored once, and it can be corrected (…
fix(budget): every autonomous coding entry point opens a pool, and MC…
fix(coding): a repeated instruction is bounded in code and the Pilot …
fix(coding): the Pilot is told when the engine refused a turn, and st…
fix(coding): an instruction that speaks for the owner is stamped wher…
feat(github): the commit-close sweep runs only where the agent may pu…
feat(coding): a repo that has no pull requests can refuse to be given…