[muhammad-luay]

Refs #275 (expanded V2 coverage; #280 already landed the narrow V1 fix). Do not auto-close #275: release consumer replay remains outstanding.

State

Rebased onto main c70c127 so the branch includes #280 (V1 foreground guard) and #283 (pinned OpenCode 2.1.4 lifecycle). The duplicate mutateGoal helper is reconciled onto the reviewed #280 implementation. #280's latest.id/latest.revision steering guard and its foreground regression test are preserved, not reverted.

Problem

Multiple writers can persist the same Goal in one plugin instance. Core writes are serialized by plugin.ts's per-session queue, but wrapper writers each construct their own GoalStore and are not on that queue. A wrapper commit between a core load and save correctly triggers the store's stale_write guard, but several call sites do not retry.

Fix

• mutateGoal(store, sessionID, mutator, { maxAttempts }) in src/persistence/store.ts now validates maxAttempts as an integer in 1..8 (default 6), retries only stale_write, and propagates lock/integrity/live-replacement failures unchanged. Mutators are documented and tested as re-entrant; only a successful save commits their effect.
• Route unguarded V1 writers through it: core chat.message (owned + steering), chat.params, message.updated accounting, patch/tool progress, Goal tools, agent boundary, model resume, startup recovery, infrastructure recovery, budget/contract UX and todo binding.
• Route V2 writers through it: model resume, successful tool progress, dispatch/limit/compaction failure recovery, owned execution completion, unit observation, continuation preparation and model-limit telemetry.
• V1 steering keeps the fix(goal): protect V1 foreground chat from stale concurrent Goal writes (#275) #280 guard: the earlier goal.id/goal.revision is captured before retry and a changed revision or same-session replacement is rejected.
• Completion evidence is now bound to the exact verified snapshot (completionSnapshotIdentity): Goal id/revision/status, requirements, evidence, checks, revision turn baseline, progressRevision, progress fingerprints and the current Todo plan. Notes/timestamps/non-turn usage do not force a replay.
• V2 completion re-runs host checks + file contracts + semantic verification (bounded at 3 attempts) when a relevant snapshot changes under the same id/revision, and re-reads declared target files before finalizing. V1 completion fails closed on the same mismatch. Pause-on-verifier-outage no longer merges a stale evaluation.
• Async evidence capture is re-entrant: on a stale collision opencode_goal_evidence_file re-reads the target file and the persisted evidence reflects the fresh read.

Tests (added)

• V1 cross-revision and new-Goal-in-same-session steering reject the old execution context.
• V1 completion rejects progress changed during semantic verification.
• V2 completion re-verifies when progress changes during verification or a competing writer commits before the final save.
• V2 completion fails closed when requirements change during verification, when unfinished Todo work is added, and when a declared target file changes during verification.
• V2 file evidence re-reads after a file change on a stale collision.
• mutateGoal rejects invalid retry budgets before loading, propagates lock/integrity/live-replacement errors without retrying, and re-applies a re-entrant mutator without duplicating updates.

Local evidence

• npm run check: pass.
• npm test: 505 tests, 502 pass, 3 skipped, 0 fail (Node 22 and Node 26/macOS).
• npm run eval: 182/182, weighted score 100%.
• git diff --check: clean.
• npm run package:smoke fails locally on this macOS host; the identical failure reproduces on unmodified main (native-entry isolation), so it is pre-existing and unrelated to this branch.

CI

The 11 upstream checks on head eab0f93 are currently action_required (fork workflow approval), not success. A maintainer needs to approve the workflow runs before the Linux/Windows CI, native 2.1.4 stable host and Goal/Loop lifecycle jobs can execute. I have not claimed CI green.

[ByBrawe]

Thanks for the careful root-cause report, the shared fresh-snapshot retry approach, and tests for BOTH V1 chat and V2 work tools. The underlying stale_write symptom is real (#275). I reviewed all 15 changed paths. Before merging this broad patch, please address and test the following:

1. New Goal/revision guard for foreground V1 steering: in src/opencode/plugin.ts the new steering mutateGoal(store, event.sessionID, latest => ...) currently checks only latest.status === "active". If the original Goal was edited/replaced (same session) between the earlier load and a retry, this hook can attach the previous turn's execution agent/model to a different Goal. Capture the original goal.id/revision (and owner where appropriate) before retry, reject a changed snapshot. Add deterministic cross-revision and new-Goal-same-session tests. Our separately green narrow V1 fix #280 contains that guard.
2. Completion after a competing writer: in src/opencode2/work-tools.ts, host checks and independent semantic evaluation are computed from snapshot before mutateGoal, but the final retry can load a newer Goal with the same Goal ID/revision and a different progressRevision/evidence/Todo. Please prove stale verification cannot close freshly changed work: bind the verifier/check evidence to the exact intended progress snapshot, or fail closed and re-run verification on a changed relevant revision. Regression: mutate requirements/progress/target artifacts while verification or the store save is pending.
3. Retry semantics/side effects: keep callbacks re-entrant/idempotent. Validate maxAttempts as a positive bounded integer, and test non-stale lock/integrity/owner errors still propagate. For async evidence capture that can re-run on a stale collision, explicitly test whether re-reading files after a change is safe.
4. CI/rebase: all eleven checks on fork PR #276 currently read action_required, not success. The existing reviewed narrower #280 has eight green checks and a competing mutateGoal helper; reconcile on a fresh main base and ensure main's V1 fix is not reverted. Run Linux/Windows + native 2.1.4 stable + Goal/Loop joint lifecycle before merging this broader V2 migration.

This is a request for targeted changes and verified evidence, not a rejection of the contribution. #275 stays open until the cross-writer V2 and published consumer paths are proven.

[ByBrawe]

Follow-up to the source review: the narrower V1 chat-only collision fix has now merged in #280 as 2671f4e, after 8 successful CI workflows. This does not replace your broader V1+V2 wrapper coverage: PR #276 is still welcome after rebasing onto latest main, reconciling duplicate mutateGoal, adding changed-Goal/progress proof race tests requested in review, and obtaining actual CI success (currently action_required). I am keeping parent issue #275 open to track that expanded coverage and release consumer replay.

[muhammad-luay]

@ByBrawe Addressed the review on head cb62b59:

• Rebased onto main c70c127; fix(goal): protect V1 foreground chat from stale concurrent Goal writes (#275) #280 and P1: OpenCode 2.1.x + dev SDK/event/compaction contract and real native Goal matrix #283 are included, the duplicate mutateGoal is reconciled onto fix(goal): protect V1 foreground chat from stale concurrent Goal writes (#275) #280's helper, and fix(goal): protect V1 foreground chat from stale concurrent Goal writes (#275) #280's V1 guard/test are preserved.
• V1 steering rejects a changed goal.id/revision or a new Goal in the same session; cross-revision and new-Goal tests added.
• V2 completion is bound to the exact verified snapshot (requirements, evidence, checks, turn baseline, progressRevision, fingerprints, Todo). Relevant changes trigger bounded re-verification; declared target files are re-read before finalizing; V1 completion fails closed on mismatch.
• Retry budgets are validated as integers in 1..8; lock/integrity/live-replacement errors still propagate; async file evidence is proven to re-read after a stale collision; re-entrant mutation tests added.

Local: npm run check pass, npm test 505 tests / 0 fail (Node 22 and 26), npm run eval 182/182. package:smoke fails identically on unmodified main on this host (pre-existing native-entry isolation failure).

All 11 workflows on cb62b59 are still action_required; I cannot approve fork workflow runs. Please approve the runs so Linux/Windows, native 2.1.4 stable, and Goal/Loop lifecycle can execute before merge.

[muhammad-luay]

Follow-up: head is now eab0f93 (added a lock-loss propagation case to the retry tests). All 11 workflows on eab0f93 are also action_required, still awaiting maintainer approval. Local npm test remains 505 tests / 0 fail.

[ByBrawe]

Maintainer status update (2026-10-27): PR #286 is now merged as 3473741, fixing source-proven false completions, mixed N-turn+deliverable audits and host-observed file-deletion evidence. Please rebase #276 on this new main and preserve these verifier guards and deletion/absence evidence when implementing V2 fresh-snapshot retries; a retry must not reintroduce an old proven status or complete against a changed artifact. The reviewer-requested identity/revision, progress snapshot, retry idempotence and lock-loss test improvements from your updated head eab0f93 are appreciated. Fork workflows remain action_required and cannot be counted as green; maintainer approval of contributor GitHub Actions is still required. Do not merge until actual Windows/Linux/native V2 and strict package CI run and pass. Thanks again for the thoughtful work!

[muhammad-luay]

Rebased onto new main 3473741; head is now 6e988e5 (the three PR commits replayed cleanly, no conflicts).

Preserved the #286 verifier guards rather than reintroducing pre-#286 behavior: runtime-counter-only proof rejection, mixed turn+deliverable audits, target-matched file/command evidence, and symlink-safe host-observed deletion/absence with post-verifier recheck all remain in src/verification/process.ts, src/verification/verifier-evidence.ts, and src/verification/semantic-targets.ts. The fresh-snapshot retry binds verification to the completion snapshot and fails closed on changed progress/artifacts.

Local Node 22.22.3: npm run check pass; npm test 514 tests, 511 pass / 0 fail / 3 skipped (includes main's new verifier guard tests); npm run eval 182/182, gate PASS.

All 11 workflows on 6e988e5 are again action_required. Please approve the fork runs so Windows/Linux, native 2.1.4, restart and Loop coexistence lanes can actually execute.

[ByBrawe]

Thank you @muhammad-luay for rebasing and expanding #276 to cover same-session Goal replacement, progress/Todo changes during verification, file mutation replay, and bounded stale-write retries. I verified the exact blob hashes of all 18 changed source/test files from your latest 6e988e5 commit and ported them onto the current maintainer main (including recently merged Goal #286 proof gating and #287 strict OpenTUI peers) in draft CI mirror #288. This is only to run owner-repository Windows/Linux and real OpenCode 2.1.4 checks; the original contribution and your authorship are preserved. The mirror has spawned 11 actual CI workflows, not action_required. I will review any failures with you here before anything merges. Thanks again for the thorough tests and thoughtful fixes!

[ByBrawe]

Thank you @muhammad-luay for addressing the previous requested changes. The V1 same-session ID/revision steering guard, bounded/validated shared retry helper, V2 re-verification against progress/Todo/requirements/evidence snapshots, asynchronous file-evidence refresh and deterministic cross-writer tests are now present. I verified all 18 changed source/test Git blob hashes against your exact PR head 6e988e5, replayed them on owner main (including #286 completion proof and #287 strict OpenTUI peers) in draft test PR #288, and obtained 11/11 genuinely successful maintainer GitHub Actions workflows, including Windows/Linux package Release Readiness, Current OpenCode 2 stable, experimental host, real Goal/Loop coexistence, restart, Todo, and native sidebar. This replaces the unavailable fork workflow approval evidence; it is not claiming the fork's action_required runs passed. The original issue #275 remains open for published npm consumer and affected-reporter replay. Thank you for the excellent thorough engineering contribution!