Enforce repair contract at every checkpoint

[Jhacarreiro]

Summary

Update the repair checkpoint contract after maintainer review on #340.

This revision moves the contract from manually injected unsupported fields into the canonical fix artifact contract:

fix_artifact.repair_contract = {
  must_touch: string[],
  match: "any" | "all",
  scope: "every_checkpoint"
}

The checkpoint gate now activates only when fix_artifact.repair_contract is present. It continues to bypass uncertain/incomplete likely_files so stale planning hints do not fail-close otherwise valid repairs.

What changed from the closed #340 approach

- added repair_contract to schema/repair/codex-result.schema.json
- added prompt/output guidance for repair_contract
- added runtime validation in execute-fix validation and review-results
- added deterministic producer support where the edit surface is concrete and bounded
- replaced inline executor parsing with src/repair/repair-checkpoint-contract.ts
- changed executor status parsing to git status --porcelain=v1 -z --untracked-files=all
- added functional tests for any/all, bypass behavior, shape validation, paths with spaces, and rename destinations

This directly addresses the two maintainer blockers from #340:

1. The contract is now part of the canonical schema/output contract instead of unsupported must_touch fields.
2. The parser now consumes porcelain v1 z output, avoiding quoted path parsing problems from plain porcelain text.

Scope

schema/repair/codex-result.schema.json
src/repair/repair-checkpoint-contract.ts
src/repair/execute-fix-artifact.ts
src/repair/execute-fix-validation.ts
src/repair/review-results.ts
src/repair/lib.ts
src/repair/deterministic-automerge-result.ts
src/repair/commit-finding-intake.ts
test/repair/repair-checkpoint-contract.test.ts
test/repair/execute-fix-artifact-source.test.ts

Semantics

repair_contract omitted:
  no checkpoint contract is enforced.

repair_contract.match = "any":
  each checkpoint may commit if at least one must_touch path changed.

repair_contract.match = "all":
  each checkpoint may commit only if every must_touch path changed.

repair_contract.scope = "every_checkpoint":
  the invariant is checked before every checkpoint commit path:
  - initial
  - review-fix
  - base-sync
  - finalize

Validation

corepack pnpm run format:check
corepack pnpm run build:repair
node --test \
  test/repair/repair-checkpoint-contract.test.ts \
  test/repair/execute-fix-artifact-source.test.ts \
  test/repair/deterministic-automerge-result.test.ts \
  test/repair/commit-finding-report.test.ts
corepack pnpm run lint:repair
corepack pnpm run test:repair

Observed result:

format:check: passed
build:repair: passed
focused tests: 19/19 passed
lint:repair: 0 warnings, 0 errors
test:repair: passed

Local repair-worker dry-run proof

I ran the real dist/repair/execute-fix-artifact.js executor locally in --dry-run mode with CODEX_BIN set to a deterministic fake Codex binary. This exercises the executor, subprocess boundary, git checkout, checkpoint helper and dry-run report path without pushing or opening PRs.

The fixture uses the canonical field now accepted by the schema:

{
  "repair_contract": {
    "must_touch": ["docs/repair-contract-proof.md"],
    "match": "any",
    "scope": "every_checkpoint"
  }
}

1. Allowed checkpoint when repair_contract.must_touch is changed

Setup:

fix_artifact.repair_contract.must_touch = ["docs/repair-contract-proof.md"]
fake worker edit = docs/repair-contract-proof.md

Observed result:

exit: 0
report_status: planned
action: open_fix_pr planned
checkpoint commit: 9cdf11e6b647 test: local repair contract e2e proof
git_status after run: clean

2. Rejected checkpoint when only an off-contract file is changed

Setup:

fix_artifact.repair_contract.must_touch = ["docs/repair-contract-proof.md"]
fake worker edit = docs/off-contract-proof.md

Observed result:

exit: 1
head remained at base commit: c88270b81889
working tree: ?? docs/off-contract-proof.md

Error:

Error: repair checkpoint contract rejected initial: no required repair_contract.must_touch file changed; match=any; must_touch=docs/repair-contract-proof.md; changed_files=docs/off-contract-proof.md

Relation to closed PRs

• Reworks the closed Enforce repair contract at every checkpoint #340 approach in-place on Enforce repair contract at every checkpoint #341.
• Does not reopen Enforce repair contract before checkpointing #336 or Enforce repair contract at every checkpoint #340.
• Excludes the conflict-prompt changes that already landed via fix(repair): harden rebase conflict retries #332.
• The fork-only Jhacarreiro/clawsweeper#2 was closed as local hygiene; the useful parser idea is represented here through porcelain v1 z parsing.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 10:38 AM ET / 14:38 UTC.

Summary
The branch adds optional fix_artifact.repair_contract schema, prompt, validation, executor enforcement with NUL-delimited git status/diff parsing, and focused repair tests.

Reproducibility: not applicable. as an issue reproduction; this is a PR adding optional repair-contract enforcement. The changed behavior is source-verifiable and the PR body includes terminal dry-run output for accepted and rejected checkpoint cases.

Review metrics: 2 noteworthy metrics.

• Patch surface: 8 files changed, +387/-6. The diff touches the live repair executor, schema, validators, prompt text, and focused repair tests.
• Checkpoint coverage: 4 checkpoint commit paths plus deterministic rebase. The branch intentionally changes when repair automation may commit or accept checkpointed branch state.

Root-cause cluster
Relationship: canonical
Canonical: #341
Summary: This PR is the current canonical repair-contract proposal after earlier unmerged repair-contract branches were closed or narrowed; the merged rebase-conflict PR is adjacent, not a replacement.

Members:

• superseded: Enforce repair contract at every checkpoint #340 - Closed unmerged predecessor with the same central goal; its maintainer review required a canonical schema field and structured porcelain parsing, which this PR attempts.
• superseded: Enforce repair contract before checkpointing #336 - Earlier closed repair-contract proposal guarded a narrower checkpoint path and mixed in work that maintainers later separated.
• adjacent_distinct: fix(repair): harden rebase conflict retries #332 - Merged adjacent rebase-conflict hardening that this PR explicitly excludes rather than replaces.
• partial_overlap: Harden repair executor gates #305 - Earlier broad repair executor hardening overlapped the checkpoint-gate idea but was closed unmerged and is not a safe replacement for this focused proposal.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• none.

Risk before merge

• [P1] Merging this makes fix_artifact.repair_contract a canonical worker-facing artifact field, so maintainers need to accept that contract boundary before external repair workers rely on it.
• [P1] A too-narrow must_touch set can reject review-fix, base-sync, finalize, or deterministic-rebase checkpoints that would otherwise leave a useful repair branch.

Maintainer options:

1. Accept The Canonical Checkpoint Contract
   Merge after required checks if maintainers want repair_contract to be the worker-facing boundary and accept fail-closed checkpoint behavior when it is emitted.
2. Narrow Activation Before Merge
   Keep the schema and validator but further constrain prompt or runtime activation until maintainers are satisfied with later-checkpoint behavior in live repair jobs.
3. Pause The Contract Surface
   Pause or close this PR if the permanent repair artifact contract is not ready to be accepted in core.

Next step before merge

• [P1] Maintainer review is needed to accept the canonical repair_contract artifact field and fail-closed every-checkpoint policy; no narrow automated repair remains.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes local repair executor logic, schema validation, and tests without new dependencies, workflows, permissions, or secret handling.

Review details

Best possible solution:

Land the contract only after maintainers accept repair_contract as the long-term artifact boundary for fail-closed repair checkpoints.

Do we have a high-confidence way to reproduce the issue?

Not applicable as an issue reproduction; this is a PR adding optional repair-contract enforcement. The changed behavior is source-verifiable and the PR body includes terminal dry-run output for accepted and rejected checkpoint cases.

Is this the best way to solve the issue?

Mostly yes technically: the patch addresses the predecessor blockers by defining the canonical field and using structured NUL-delimited parsing. Product acceptance of the new artifact field and fail-closed policy remains the open maintainer decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 323f7bdf7236.

Label changes

Label justifications:

• P2: This is normal-priority repair-lane automation hardening with bounded but real merge impact.
• merge-risk: 🚨 automation: The PR changes repair-worker checkpoint acceptance behavior that CI alone does not settle for live repair jobs.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal-style after-change dry-run output from the real repair executor for both allowed and rejected contract checkpoints.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal-style after-change dry-run output from the real repair executor for both allowed and rejected contract checkpoints.

Evidence reviewed

What I checked:

• Repository policy read and applied: AGENTS.md was read fully; its repair-lane guidance for narrow, evidence-backed, automation-safe changes applies because this PR changes repair executor behavior. (AGENTS.md:1, 323f7bdf7236)
• Current main does not already implement the contract: Current main has no repair_contract search hits and still uses unguarded checkpoint commits, so the central change is not already implemented on main. (src/repair/execute-fix-artifact.ts:2206, 323f7bdf7236)
• PR defines the canonical artifact field: The PR head adds fix_artifact.repair_contract with required must_touch, match, and scope fields under the repair result schema. (schema/repair/codex-result.schema.json:360, 486a5e0fc8a5)
• PR centralizes checkpoint enforcement: The PR routes checkpoint commits through commitRepairCheckpointIfNeeded and enforces the contract before committing or accepting checkpoint state. (src/repair/execute-fix-artifact.ts:3338, 486a5e0fc8a5)
• PR adds structured parsing and validation: The new helper validates contract shape, parses porcelain v1 NUL-delimited status, parses NUL-delimited name-only diffs, and checks changed paths against must_touch. (src/repair/repair-checkpoint-contract.ts:58, 486a5e0fc8a5)
• Focused coverage exists: The PR adds tests for explicit activation, schema shape, space-preserving porcelain parsing, rename destinations, name-only parsing, and any/all enforcement semantics. (test/repair/repair-checkpoint-contract.test.ts:12, 486a5e0fc8a5)

Likely related people:

• steipete: Merged adjacent rebase-conflict repair work and provided the concrete predecessor review blockers that this PR addresses. (role: recent repair-lane feature owner and predecessor reviewer; confidence: high; commits: 39239e718f9d, dc824915bb6c; files: src/repair/execute-fix-artifact.ts, src/repair/fix-prompt-builder.ts, src/repair/rebase-conflict-policy.ts)
• vincentkoc: Recently touched the repair executor and source tests on current main, and authored the last two commits on this PR head affecting checkpoint contract coverage. (role: recent area contributor; confidence: high; commits: ab37a53656df, babd6a435afb, 265aef5ebe55; files: src/repair/execute-fix-artifact.ts, test/repair/execute-fix-artifact-source.test.ts, test/repair/repair-checkpoint-contract.test.ts)
• brokemac79: Git blame ties the existing checkpoint helper and deterministic fast-rebase repair path to this author before the PR layers contract enforcement on top. (role: introduced current checkpoint and fast-rebase repair structure; confidence: medium; commits: bb22ac97a417; files: src/repair/execute-fix-artifact.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[Jhacarreiro]

Updated this PR after the maintainer deep review on #340.

Changes now address the two blockers called out there:

- no unsupported top-level must_touch / must_touch_files fields;
- canonical fix_artifact.repair_contract added to schema, prompt/output contract, validators and deterministic producers;
- executor now enforces fix_artifact.repair_contract only when present;
- parser now uses git status --porcelain=v1 -z --untracked-files=all;
- functional tests cover quoted/space paths, rename destinations, any/all semantics and likely_files bypass;
- local dry-run e2e proof rerun with repair_contract, allow -> planned, block -> rejected before checkpoint.

Validation run locally:

corepack pnpm run format:check
corepack pnpm run build:repair
node --test test/repair/repair-checkpoint-contract.test.ts test/repair/execute-fix-artifact-source.test.ts test/repair/deterministic-automerge-result.test.ts test/repair/commit-finding-report.test.ts
corepack pnpm run lint:repair
corepack pnpm run test:repair

All passed.

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

[Jhacarreiro]

v3 update pushed

I updated this PR to address the maintainer blockers from #340 instead of reopening or duplicating that closed PR.

What changed:

- `must_touch` / `must_touch_files` are no longer standalone unsupported fields.
- The contract is now canonical: `fix_artifact.repair_contract` in `schema/repair/codex-result.schema.json`.
- The prompt/output contract documents when to emit it and when to omit it.
- Runtime validation now checks the contract shape in execute-fix validation and review-results.
- Deterministic producers attach the contract only when the expected edit surface is concrete and bounded.
- The executor now reads `git status --porcelain=v1 -z --untracked-files=all`.
- Contract parsing/enforcement lives in `src/repair/repair-checkpoint-contract.ts` with functional tests.

Validation run locally:

corepack pnpm run format:check
corepack pnpm run build:repair
node --test test/repair/repair-checkpoint-contract.test.ts test/repair/execute-fix-artifact-source.test.ts test/repair/deterministic-automerge-result.test.ts test/repair/commit-finding-report.test.ts
corepack pnpm run lint:repair
corepack pnpm run test:repair

Result:

format:check: passed
build:repair: passed
focused tests: 19/19 passed
lint:repair: 0 warnings, 0 errors
test:repair: passed

Local dry-run executor proof with canonical fix_artifact.repair_contract:

allow:
  exit 0
  report_status planned
  action open_fix_pr planned
  checkpoint commit 9cdf11e6b647
  git_status clean

block:
  exit 1
  head remained c88270b81889
  working tree ?? docs/off-contract-proof.md
  error: repair checkpoint contract rejected initial: no required repair_contract.must_touch file changed; match=any; must_touch=docs/repair-contract-proof.md; changed_files=docs/off-contract-proof.md

[Jhacarreiro]

@clawsweeper re-review

Please review the current head da96beca9e35f42da4d7da482bd622e6ba4909e7 and updated body/comment. This revision addresses the #340 maintainer blockers by moving the checkpoint contract into canonical fix_artifact.repair_contract, schema, prompt/output contract, validators, deterministic producers, and porcelain v1 z parsing.

[Jhacarreiro]

Small implementation-shape note for review: if maintainers prefer a narrower contract activation policy, I can make that adjustment in this PR without changing the overall design.

The intentionally small knobs are:

- keep `repair_contract` schema/prompt/validator/executor semantics as the canonical boundary;
- make deterministic producers more conservative, or remove producer emission entirely if maintainers prefer model/planner-authored contracts only;
- make `match` / `scope` defaults explicit in runtime while preserving the schema contract, if that better matches existing artifact conventions.

One extra architectural motivation for this shape: keeping repair_contract inside the canonical fix_artifact contract gives external repair workers/connectors a stable boundary without adding another execution path inside ClawSweeper.

That keeps the core provider-neutral: ClawSweeper owns schema, validation, git/checkpoint safety, and PR mechanics, while any external worker that can emit the canonical artifact can interoperate through the same contract. This should make the repair lane easier to extend safely without growing backend-specific logic in the core.

Those would be narrow follow-ups on the current head, not a new PR and not a redesign of the checkpoint gate.