Repository navigation
fix: reconcile interrupted save intent as unknown - #284
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50d720ad10
ℹ️ 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".
| latest = workflow_store.transition(thread_id, owner, expected_run=run_id, | ||
| expected_version=version, statuses={"saving"}, | ||
| changes={"status": "unknown", "failure_reason": "save_result_unknown"}, | ||
| kind="save_unknown") |
There was a problem hiding this comment.
When /cancel runs during either awaited recovery lookup, it does not acquire thread_lock; WorkflowStore.cancel() can therefore change this attempted save from saving to unknown before this transition executes. The statuses={"saving"} check then raises Conflict, which is handled as a generic 502 instead of returning the canonical unknown state with cancelRequested=true. Handle the conflict by re-reading and returning the current state, as the analysis path already does for cancellation races.
AGENTS.md reference: services/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
1 participant
Recovering a save intent interrupted before Java dispatch returned
savingafter Java reported no record, instead of the requiredunknown. Recovery withretry:falsenow persistsunknownwithout another business write; a later explicit retry retains the original key and confirmation guards.The restart regression reproduces the original failure, verifies repeated recovery makes zero writes, and then verifies an explicit retry saves once. The API reference describes this behavior.
Validation on remote-dev: regression failed before the fix; all 49 Agent service tests and 58 security/store tests passed afterward. Independent standards and spec reviews approved the diff.
git diff --checkpassed.Exact
f26a219bbfull verification exposed three repository-corpus failures after the API reference changed. Follow-up50d720ad10153bcb83d90c6d79a2259cccf28c34refreshes its source hash/chunk identity and shifts the excerpt range by two lines while preserving the original excerpt digest. All 67 corpus tests pass; independent range/digest review approved. The supported full Agent wrapper on a clean, asserted exact-head remote checkout passed 1719 tests, with one optional Qdrant dependency skip, exit 0 (187.88 seconds). CI 38156569344 is still running; its Agent job passed. The failed predecessor run remains recorded.Original genuine U03 evidence remains INCOMPLETE (
u03_restart_status_mismatch) on candidate 3730b74. This fix does not relabel that record or establish fresh U02/U03/U04 acceptance. No automatic paid rerun or holdout consumption occurred.