feat(backends): completion contracts can declare pushed-changes alternatives (spec 025, plan 1/3)

[zbigniewsobiecki]

Spec 025 plan 1 of 3 — docs/plans/025-pr-comment-completion-and-codex-resume/1-completion-alternatives.md.done (parent spec docs/specs/025-…; the spec and all three plans are committed here). Independent of #1553 / #1554.

What changed

• Finish-hook flag hooks.finish.scm.pushedChangesAlternatives: [pr-response], valid only next to requiresPushedChanges: true (load-time refinement) — no agent can inherit the alternative implicitly.
• Evidence model (src/backends/completion.ts): PR-response sidecar (url + kind: top-level | inline-reply; anything else existing is malformed) and repository state (git status --porcelain + rev-parse HEAD in repoDir vs initialHeadSha, 10 s timeout, any failure ⇒ unavailable).
• Evaluator evaluatePushedChanges: push branch first, then each declared alternative; first satisfied wins; otherwise one rejection reason per branch. Fail-closed on missing/malformed/contradictory evidence.
• Failure + prompt: profiles without alternatives keep the exact legacy error and prompt (pinned by test). With alternatives: COMPLETION_ERROR_NO_PUSH_OR_RESPONSE, rejections, and a continuation prompt that names an already-posted response and forbids re-posting it.
• Logging: decideContinuation logs pushedChangesOutcome on success, rejections on continuation and on a new exhaustion WARN; postProcessResult consumes the same evaluation (duplicate push check removed) and logs the same.
• Dormant: no built-in agent declares an alternative; the sidecar producers land in plan 2, the PR-comment opt-in and end-to-end proof in plan 3.

Also: tests/helpers/tempGitRepo.ts (real-git helper), and an out-of-plan test(cli) commit giving the three cascade-tools spawn tests a per-test budget above their 30 s spawn budget (they timed out at vitest's 5 s default under load; house rule: flaky = failing).

Verification

• npm run lint, npm run typecheck, and the four unit projects: green — 624 files / 11275 tests (run with --maxWorkers=4; an unrelated test run pushed the host past load 90 during this session).
• Fresh-context z:local-review gate, round 1: 0 must-fix; 3 should-fix applied (git timeout, DRY'd CLI budget, helper index), suggestions applied where cheap (single evidence read in applyCompletionEvidence, pure repo-state checks, constant-based test pins). Deferred items are listed in the plan's Progress section.
• Done-gate script passes; every per-plan AC is checked in the .done file.

Commits

docs(spec) → chore(plan): lock → 6× feat(...) (one per TDD task) → test(cli) → docs(changelog) → refactor(backends) (review findings) → chore(plan): done.

Next: /z:implement docs/plans/025-pr-comment-completion-and-codex-resume/2-pr-response-evidence.md (after this merges — plans are serial).

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 99.01478% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/backends/adapter.ts	83.33%	1 Missing ⚠️
src/backends/completion.ts	99.30%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

APPROVE — a clean, well-decomposed, fully-dormant infrastructure change. It teaches the shared completion contract to express pushed-changes alternatives without altering any agent's observable behaviour, and it's backed by exhaustive tests. I verified the affected unit suites (198 tests), typecheck, and lint all pass locally, and confirmed the flag is dormant (no built-in YAML declares pushedChangesAlternatives).

What I verified

Correctness / logic

• evaluatePushedChanges checks pushed-changes first, then declared alternatives in order; first satisfied wins; otherwise one rejection per branch. OUTCOME_CHECKS is a Record<PushedChangesOutcome, …>, so adding an alternative without a check is a compile error — good extension guard.
• rejectPRResponse ordering (malformed → no response → no repo-state → dirty → HEAD moved) is sensible and fail-closed. readRepoState treats any git failure/timeout as undefined (unavailable ⇒ rejected), and untracked files count as dirty via --porcelain.
• Backward-compat is genuinely preserved: COMPLETION_ERROR_NO_PUSH and CONTINUATION_PROMPT_NO_PUSH are byte-identical to the previous inline literal, and resolvePushedChangesError returns the legacy error whenever no alternative is declared (pinned by the "keeps the legacy no-push error and prompt" test).
• The post-run gate guard (pushedChangesSidecarPath !== undefined in adapter.ts) preserves the exact skip semantics of the old hasAuthoritativePushedChanges === false guard, so non-native-tool runs behave as before.

Blast radius

• postProcessResult has a single caller (adapter.ts), correctly migrated from hasAuthoritativePushedChanges to pushedChangesEvaluation + pushedChangesError. No dangling references to the removed option.
• applyCompletionEvidence now reads only prSidecarPath — safe, since it only consumes prUrl/prCommand, and it avoids a per-turn git shell-out (the full evidence read, incl. repo state, happens once per turn in decideContinuation).
• The superRefine on FinishHooksSchema (now a ZodEffects) has no external consumers — ScmFinishSchema stays a plain object, so FinishHookFlags' z.infer is unaffected. DefinitionPatchSchema.partial() still typechecks.

Security: readRepoState uses execFileSync('git', [...], { cwd }) with fixed args and a 10 s timeout — no shell, no interpolation of initialHeadSha.

Non-blocking notes

• Forward risk for plan 2 (not this PR): the comment-only outcome hinges on headSha === initialHeadSha, where initialHeadSha will be input.headSha/CASCADE_INITIAL_HEAD_SHA and headSha comes from git rev-parse HEAD (full 40-char). Plan 2/3's e2e tests should confirm input.headSha is a full SHA, or the pr-response branch would silently always reject as "HEAD moved." Design decision 5 already anticipates this; flagging only so the plan-2 reviewer keeps it in view.
• Legacy push-requiring agents now also get a rejections field on their WARN logs plus a new turns-exhausted WARN — both are additive observability wins, not behaviour changes.

LGTM.

🕵️ claude-code · claude-opus-4-8 · run details

[aaight]

CI Failures Resolved

Fixes Applied

• lint-and-test → "Audit production dependencies": npm audit --omit=dev --audit-level=high failed on a high-severity js-yaml@4.3.1 advisory (GHSA-2883-xcg3-v3hh — maxTotalMergeKeys does not limit CPU use for empty merge sources). Bumped the direct production dependency to js-yaml@^4.3.2 and refreshed package-lock.json (all transitive copies dedupe to 4.3.2).

Verification (local, matching CI)

• npm audit --omit=dev --audit-level=high → exit 0 (only unrelated moderate advisories remain, below the gate).
• npm ci → clean install from the updated lockfile.
• npm run typecheck → passes.
• Full unit suite via pre-push hook → 11265 passed / 33 skipped.

Only package.json and package-lock.json changed. Pushed to feat/025-1-completion-alternatives.

🕵️ claude-code · claude-opus-4-8 · run details

[nhopeatall]

Summary

APPROVE — a well-scoped, fail-closed, and provably dormant refactor that centralizes the pushed-changes verdict into one shared evaluator. Verified locally: typecheck and lint clean (no warnings in changed files), and 198 affected unit tests pass (completion 34, postProcess 18, continuationLoop 17, schema 44, profiles 23, adapter 62).

What I verified

• Dormancy holds. No built-in *.yaml declares pushedChangesAlternatives, and secretOrchestrator.ts populates only pushedChangesAlternatives (undefined everywhere) — not repoDir/initialHeadSha/prResponseSidecarPath. So readRepoState never shells out to git, readPRResponseSidecar reads nothing, and evaluatePushedChanges only ever evaluates the pushed-changes branch at runtime. Existing agents are observably unchanged.
• Legacy path preserved. For profiles without alternatives, getCompletionFailure returns the exact COMPLETION_ERROR_NO_PUSH + CONTINUATION_PROMPT_NO_PUSH (pinned by test); the new rejections field is purely additive to log payloads.
• Schema refinement is safe. The superRefine is attached to FinishHooksSchema (now a ZodEffects, only ever consumed via .optional()), so ScmFinishSchema stays a plain ZodObject and z.infer → FinishHookFlags still resolves. Type-only import of PushedChangesAlternative into completion.ts introduces no runtime cycle.
• applyCompletionEvidence narrowing is correct. It only ever consumes prUrl/prCommand, so reading just prSidecarPath is output-identical while avoiding a second git shell-out per turn.
• Both gates share one evaluator. decideContinuation and postProcessResult both route through evaluatePushedChanges, and the adapter's guard (pushedChangesSidecarPath !== undefined) mirrors the previous hasAuthoritativePushedChanges guard, so the post-run verdict can't drift from the loop.
• Record<PushedChangesOutcome, OutcomeCheck> forces a compile error if a future alternative is added without a check — good type-safety.

Optional (non-blocking, already tracked)

• On the success path, decideContinuation calls evaluatePushedChanges a second time (after getCompletionFailure already computed it) purely to log pushedChangesOutcome. It's a pure function over already-read evidence, so the cost is nil — the plan's Progress section already lists returning the evaluation from getCompletionFailure as a deferred cosmetic cleanup. A satisfied push run also logs the outcome twice (loop INFO + postProcess INFO); harmless. Neither needs action here.

The bundled js-yaml ^4.3.1 → ^4.3.2 bump (audit fix) has a consistent lock-file entry and is low risk.

🕵️ claude-code · claude-opus-4-8 · run details