fix(autopilot): enforce the in-session merge-auth preflight in code - #330
Merged
Conversation
The merge-authorization requirement lived only as prose in SKILL.md, so an
unattended/scheduled run (no live user turn to grant it) would deliver a whole
ticket and then SILENTLY wedge at the first merge — the observed failure.
Add a run-start preflight that requires an EXPLICIT authorization rather than
trying to detect the harness auto-mode classifier (which is not observable from
code). `scripts/autopilot/preflight.mjs` `mergeAuthPreflight({authorized,config})`
is a pure decision that returns the effective merge mode: `auto-merge` only when
an explicit in-session grant is held AND config doesn't disable it; otherwise
`pr-only` with the exact human-readable notice to surface. `startRun` records the
chosen mergeMode + reason into run.json (auditable, resume-safe), and `runMerge`
gates on it — `pr-only` carries autoMergeEnabled:false semantics, so the merge
path parks the ticket awaiting-human instead of attempting a merge that stalls.
The auto-merge path is unchanged when authorized. The mode threads through the
`autopilot_merge` MCP tool so the live merge honors it by construction.
SKILL.md's preflight section now references the code preflight as the mechanism
(it previously noted a helper "would be additive").
Tests: AC-316.1 (authorized -> auto-merge; absent/omitted/non-true -> pr-only;
config opt-out wins; startRun records + resume refreshes the non-file-backed
grant) and AC-316.2 (pr-only parks and never attempts a merge; auto-merge
proceeds to the bar and squash-merges on green; mode omitted preserves prior
behavior). Existing #315 pass-through test updated for the new `mode` arg.
Closes #316
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SATRHKa6mDHDuirhP6QuwL
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #316 (p1 bug, epic #183)
Problem
The autopilot merge-authorization requirement was prose in
SKILL.mdonly. An unattended/scheduled run has no live user turn to grant it, so a run would deliver a whole ticket and then silently wedge at the first merge — the observed failure. The harness auto-mode classifier is not observable from code, so the fix is not to detect it: it is a run-start preflight that requires an EXPLICIT authorization to be recorded, and degrades to PR-only (awaiting-human) when it is absent — instead of proceeding into deliveries that stall.Change (thin: a preflight decision + ledger record + wiring)
plugin/scripts/autopilot/preflight.mjs— puremergeAuthPreflight({ authorized, config })returns the effective merge mode:auto-mergeonly when an explicit in-session grant is held AND config doesn't disable it; otherwisepr-onlywith the exact human-readable notice the orchestrator surfaces.authorizedistrueonly for a genuine live user grant (a value in run.json / config / allowlist / narration passesfalse). Thin CLI prints the decision.ledger.mjsstartRun(cwd, opts)— recordsmergeMode+mergeReasonintorun.jsonat run start (auditable, resume-safe). On resume it keeps the original start time but re-runs the (non-file-backed) preflight. No-optstartRun(cwd)is unchanged (back-compat).merge.mjsrunMerge(..., { mode })—pr-onlycarriesautoMergeEnabled:falsesemantics: the merge path parks the ticket awaiting-human rather than attempting a merge that stalls. The auto-merge path is unchanged when authorized.autopilot_mergeMCP tool — threadsmodethrough so the live merge honors the recorded decision by construction.SKILL.md— the preflight section now references the code preflight as the mechanism (previously "a helper would be additive"); driver-scripts list documentspreflight.mjs.Acceptance criteria
mergeAuthPreflightreturnspr-onlywhen no grant (and when config opts out);startRunrecords the decision intorun.json; preflight CLI exercised both paths (exit 3 pr-only / exit 0 auto-merge).#316block intests/autopilot/engine.test.mjs— AC-316.1 (grant → auto-merge; absent/omitted/non-true→ pr-only; config opt-out wins; startRun records + resume refreshes) and AC-316.2 (runMergemodepr-onlyparks and never callspr merge/pr view;auto-mergesquash-merges on green; mode omitted preserves prior behavior).Verification
pnpm verify— 603/603 green (was 603 pre-change; +7 new autopilot: enforce the in-session merge-auth preflight in code #316 tests, 1 existing autopilot: gate the live merge through the tested merge bar #315 pass-through test updated for the newmodearg, net counts steady across added/updated).claude plugin validate ./plugin --strict— passes locally.🤖 Generated with Claude Code