Repository navigation
fix: mark successfully applied proposals to prevent replay - #241
Conversation
[Medium impact] The numbered crash-lock finding is fixed, and no new blocking findings were accepted. Reviews (3) · Last reviewed commit: "no-mistakes(ci): Removed proposal lockin..." · Reviewed by Greptile |
|
Speaking as Kun's firstmate: thanks @WTF-Am-ID for taking on #35. I reviewed the diff (no workflow, network, or exec changes) and approved fork CI. The rollback handling and the 8 of 8 live scenarios are thorough. This is waiting on you for one fix. Greptile's P1 on VISION per-rule:
Contract-class: restore. The |
…ing now verifies the reviewed proposal under a lock shared by saves and clears. Reproduced the overwrite on 8efafaa, then verified fresh bytes survive and old edits roll back. All 34 authorized focused tests pass. No scratch files remain in the worktree
…ore-stamp and rollback. Replaced the lock-specific test with an interrupted-save regression, reproduced before the fix. All 34 tests pass using the authorized focused command, including stale/cleared proposal, rollback, retry, replay, dry-run, and rejection coverage. No worktree scratch files remain
|
Speaking as Kun's firstmate: thanks @WTF-Am-ID, the stale-overwrite guard is in. VISION per-rule:
Contract-class: restore. The |
|
Speaking as Kun's firstmate: this is merged. Thank you @WTF-Am-ID — really appreciate you taking the time on this. |
Intent
ok same instructions i gave you before pick high value issues from scan, in all the repos you scanning and use codex only to close them
Context: this task is scan pick P4: upstream issue #35 in kunchenguid/backpass. Successful normal apply never stamps appliedAt/appliedBy, so the proposal can reopen for review. Triage contract class: restore. Scan notes for this pick:
Issue: #35
Successful normal apply never stamps appliedAt/appliedBy, so the proposal can reopen for review.
An existing replay-prevention guard has no corresponding successful state transition.
Size S.
Contract class: restore.
Maintainer triage.
Evidence: Current remote
src/commands/apply.js:99-101checks appliedAt, but the file contains no successful appliedAt/appliedBy assignment. Latest maintainer restamp confirms only bootstrap stamps.Competing work: No exact target among the 16 open proposals. #214 changes target-specific proposal storage but its diff retains only the existing appliedAt guard and adds no success stamp. The earlier attempted #36 was closed without merge, according to the complete issue thread.
Closure test: Apply an accepted synthetic proposal to disposable files. On complete non-dry-run success persist appliedAt and appliedBy=apply, then verify a second apply stops at the existing replay guard before review or writes. Dry-run, rejected edits and any write failure must leave it unstamped and retryable. Check the actual proposal.json and resulting file bytes.
The owner approved fixing the scan's high-value picks upstream through PRs, written and reviewed by Codex only.
What Changed
appliedAtandappliedBy: "apply"after successful applies with accepted edits, so the existing replay guard blocks subsequent review and writes.Risk Assessment
✅ Low: The change is bounded, preserves replay and dry-run behavior, and includes proposal persistence failures in existing rollback handling.
Testing
Review used only Codex models by the owner's temporary choice.
Focused apply tests and all live CLI scenarios passed. The base revision reproduced the missing stamp. Terminal review, plain/JSON output, rollback, and retries were verified against disposable repositories, with CLI transcripts and persisted bytes retained as evidence. Scratch files were removed. No full suite, lint, formatting, or static analysis ran.
Evidence: Live CLI transcripts
~/.no-mistakes-upstream-codex/evidence/01M615473HKPTENEG3ND2C7DQG/live-apply.json)Evidence: Interactive terminal review
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
src/commands/apply.js:159- F1: Proposal persistence is outside the writer's rollback boundary. With an existing readable proposal and a non-writable .backpass directory, an accepted apply commits the memory and skills, then writeProposal throws while creating proposal.json.tmp. The proposal stays unstamped, but restoring permissions does not make it retryable: the writer rejects its original memory hash. This contradicts the required criterion "any write failure must leave it unstamped and retryable." The new call at src/commands/apply.js:159 affects browser, terminal, and --decisions paths, including --json. The failure test at test/apply-cli.test.js:777 fails before this step and misses this case. Include stamp persistence within the existing guarded commit/rollback boundary. The acceptance contradiction requires owner review.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
TMPDIR="$PWD/.test-phase/tmp" node --import ./test/helpers/hermetic-config.js --test test/apply-cli.test.js test/user-apply.test.jsgit archive bc7bc6458cd2a023606d826a195decd3d72b7016 | tar -x -C .test-phase/baseline-sourcefollowed by real baseline CLI apply reproduced the missing stamp.TMPDIR="$PWD/.test-phase/tmp" node .test-phase/live-apply.mjsdrove plain and JSON success, replay, dry-run, rejection, write failures, rollback, and retries.TMPDIR="$PWD/.test-phase/tmp" node .test-phase/live-apply.mjs setup-terminalandpython3 .test-phase/terminal.pydrove real terminal acceptance in a 40×120 PTY.TMPDIR="$PWD/.test-phase/tmp" node .test-phase/live-apply.mjs verify-terminalchecked persisted bytes, stamps, and replay refusal.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.