Repository navigation
Conversation
…current main This maintenance mirror preserves exact contributor source/test blob hashes from muhammad-luay/opencode-goal@6e988e5, against Goal main including #286 and #287, without importing fork CI workflows. This is a *test branch*, not a PR merge or published release. Co-authored-by: muhammad-luay <muhammad-luay@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…Code 2 (#275 #276) Contributed by @muhammad-luay. Keep V1/2 state writers on fresh snapshots and retry stale generation only, with bounded 1..8 attempts. Guard same-session Goal/revision replacement; re-evaluate changed verification evidence, progress fingerprints, Todo, declared files and requirements before completion; ensure async file evidence is refreshed on collision. Fully tested in owner-repository verification mirror #288 against latest main, all 11 workflows green. Published package and reporter replay remain tracked in #275. Co-authored-by: Muhammad Luay <muhammadluay45@gmail.com>
|
CI mirror completed successfully: 18 fork source/test blobs from @muhammad-luay's #276 were SHA-identical, and all 11 owner CI workflows passed on latest main ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 396e74e7c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const owned = textFromParts(output.parts) | ||
| const next = replaceGoalConstraints(current, parsed.constraints) | ||
| await store.save(next) | ||
| const result = await mutateGoal(store, event.sessionID, (current) => replaceGoalConstraints(current, parsed.constraints)) |
There was a problem hiding this comment.
Guard constraint retries against revision changes
When another process edits the Goal after this mutator's first load but before its save, mutateGoal retries against the new revision and applies the older command's constraint flags to it. This silently produces a Goal whose newer objective/requirements are paired with constraints from the superseded /goal create or /goal edit; capture the post-command Goal ID/revision on the first attempt and reject subsequent attempts when either changes.
Useful? React with 👍 / 👎.
| await mutateGoal(store, sessionID, (latest) => { | ||
| if (latest.id !== current.id || latest.revision !== current.revision || latest.status !== "active") return null | ||
| return pauseGoal(latest, restrictedAgentStopReason(currentAgent)) | ||
| }) |
There was a problem hiding this comment.
Recheck the execution agent before pausing recovery
If a concurrent writer changes the execution agent from a restricted agent to Build after currentAgent is read, the retry still pauses the fresh active snapshot because the guard checks only ID, revision, and status. Execution-context updates do not increment the Goal revision, so startup recovery can incorrectly pause a Goal that is now eligible to run; make the restricted-agent decision from latest.execution?.agent and reconsider the outer recovery path when it changed.
Useful? React with 👍 / 👎.
| @@ -615,11 +635,11 @@ export async function mutateGoal( | |||
| return { goal: next, wrote: true } | |||
There was a problem hiding this comment.
Report semantic no-ops as unwritten
When a mutator returns a state that differs only by updatedAt, GoalStore.save() deliberately skips the write and preserves the generation, but this helper still returns wrote: true. Callers use that flag to authorize external side effects: for example, two recovery coordinators can both observe a due retry, the second can re-mark an already-dispatched nextRetryAt: 0 state as a semantic no-op, and wakeHostLimitRetry will nevertheless schedule a second autonomous continuation. Derive wrote from whether persistence actually advanced rather than merely whether save() returned.
Useful? React with 👍 / 👎.
| if (!observed) continue | ||
| observedGoal = observed.goal |
There was a problem hiding this comment.
Stop stale terminal events after a rejected mutation
If the Goal is edited between coordinatorGoal() and this mutation, the ID/revision guard returns null, but mutateGoal represents that as a truthy { goal, wrote: false }, so this check does not stop the old execution-terminal event. The handler then treats the newer revision as successfully observed, marks the old execution successful, and can prepare or dispatch continuation for the new revision; verify that observed.goal still matches the captured Goal before continuing. The later unitResult truthiness check has the same stale-revision fall-through.
Useful? React with 👍 / 👎.
Verification-only mirror of the original contribution
Original contributor: @muhammad-luay, PR #276. Thank you for the substantial V1 and V2 concurrency work. Its 11 fork-origin Actions checks are still
action_requiredrather than passed. To get reproducible owner-repository CI evidence, this draft PR reproduces exactly the 18 changed source/test blobs from fork commit6e988e5as a single test commit on top of current Goalmainde15109(includes verified closure-proof #286 and native OpenTUI peer #287). Each source blob SHA was checked identical. No workflows, package scripts or secrets were copied from the fork.Questions this CI must settle
#286goal completion proof and#287OpenTUI 0.5/0.7 strict compatibility are preserved, never reverted.Safety and credit
Do not merge this draft automatically. If all gates pass, review and reconcile with original contributor PR #276 to preserve attribution and avoid duplicate deployment. If any fails, add deterministic failing fixture and work with the contributor on a targeted fix. This owner-side mirror solely enables trustworthy CI evidence, not a publish or claim that #275 is fully resolved. Maintainer reviewer notes are in #276; original PR remains open.
Co-authored-by: muhammad-luay muhammad-luay@users.noreply.github.com