Support explicit Flow run abandonment - #211
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The patch introduces a deterministic resolver/apply loop, a cross-worktree Flow-identity lookup, and an unreachable valid abandonment entry. Model-level verification is recommended before merge. Questions: can every terminal workspace state reach the new bind prerequisite, and can the new abandonment selection priority enter any additional zero-progress cycle?
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.99
The patch introduces a resolver/apply disagreement and breaks terminal replay for the new synthetic abandonment run identity. Model-level verification is recommended before merge: verify that every newly selectable abandonment path either reaches its marked state or retains a replayable recovery path.
| if state.Terminal != model.TerminalEstablished { | ||
| return fmt.Errorf("a different delivery requires the prior delivery to be terminal") |
There was a problem hiding this comment.
[P1] Reject active delivery replacement during resolution
Invariant: a prescribed transition must be accepted by apply for the identical state and context. With active nonterminal delivery A, request objective B for delivery B; the supervisor still prescribes objective.bind, but this new reducer check deterministically rejects it. State remains unchanged, so resolving again selects the same transition, producing a zero-progress loop through the standard CLI/RPC path. Add an engine-level test asserting that untargeted and targeted resolution refuse replacement before issuing a prescription unless A is terminal.
Confidence: 0.99
| func (s State) ActiveObjective() (model.Objective, bool) { | ||
| return s.Objective, s.Objective.ID != "" && s.Terminal == model.TerminalNonterminal |
There was a problem hiding this comment.
[P2] Preserve abandonment run identity after terminal commit
Invariant: a committed Flow transition must remain replayable under its original run identity after reaching its marked terminal. Abandonment uses a synthetic run ID derived from active-run:<original-run>, but after plan.abandon commits, ActiveObjective returns false. A retry after a lost response therefore resolves the plan normally and derives a different run ID from its file digest, failing with FLOW_RUN_MISMATCH before idempotency lookup or marked-state observation. The durable abandonment succeeds, but the client cannot recover its receipt through the Flow path. Add an end-to-end test that discards the first successful abandonment response and retries the exact request, expecting the committed receipt or marked result.
Confidence: 0.98
Summary
Verification
go test ./...npm run test:flow-sdk