Repository navigation
fix: retain cancellations received before start - #1042
sbalabanov wants to merge 2 commits into
Conversation
Summary: Independent start and cancel topics can deliver cancellation before the Request exists. Retain that intent rather than treating absence as failure. Builds on #1040's retained start recovery. Changes: - Hold the original missing-request cancellation without expiry or failure budget. - Replay final cancel deliveries with the full reason and ordinary cancellation. - Remove the unused generic cancel decoder and extend existing regressions. Test Plan: All six focused unit/shared/integration targets passed. The full orchestrator suite proves primary and retained-final cancellations survive four committed postponements beyond a three-attempt primary budget, then cancel with the original reason after start. Consumer integration also passed an uncached run. Repository format/lint/tidy/Gazelle/mock checks passed; cancel line coverage is 184/201. aifx verify had no configuration and ran no checks. Revert Plan: Revert this commit without a migration. Preserve held/final cancellation deliveries before rollback; the previous handler can drop missing-request intent.
|
[independent-review] Should fix — public description boundary (100% confidence). The Test Plan names an organization-internal optional validation wrapper. Please delete only that sentence/bullet, preserving the reproducible repository-native checks and their truthful results. The public-repository rule in Independent metadata review checked all seven draft titles/bodies and actual check conclusions; skipped code CI is not passing. This comment covers metadata only, not the separate ongoing code review. Independently rechecked against PR #1042 at |
|
[agent-response SQC-012] Addressed this feedback by deleting only the optional validation sentence from the Test Plan. The repository-native checks/results, remaining description, and title are unchanged. No code or tests changed; head remains |
|
[independent-review] Should fix — public commit-message boundary (95% confidence). The Test Plan in commit This requires rewriting already-published history: prepare the exact message-only amendment, but do not force-push until the user explicitly confirms the old/new hashes and branch. The complete code review read every assigned file and reran the actual orchestrator integration suite; no reachable code defect or further safe complexity reduction was established. Generated with the repository's |
|
[agent-response SQC-012] Prepared a commit-message-only cleanup for this feedback. The repository tree is byte-identical and the native verification evidence is preserved. The published branch remains unchanged at |
Summary: Keep checking the cancellation reason on the first emitted log, without requiring it to be retained in the Request record. All cancel-before-start, waiting, and final-state checks remain unchanged. Test Plan: The cancel controller and full orchestrator integration targets pass uncached. Formatting, lint, module, BUILD, and mock checks pass. A conflict-free tree simulation with the reduced replay contract has no integration references to retained terminal details and preserves the first-publication reason check. Revert Plan: Revert this test-only commit if terminal-detail retention is restored.
TL;DR: Keep cancellations that arrive before start from being lost.
Summary
A cancellation can arrive before
startsaves the submission's Request record. Keep it until that record exists, without timing out or using up failure retries, then process it with the original reason. This also recovers cancellations already waiting in the final recovery queue.If merging has already started (
Landing), cancellation is still best-effort: the change may still land. This does not fix races while a request is being added to a batch or change how other failures are handled.Builds on #1040.
Repro
startsaves its Request; keep start delayed past the usual retry limit.The existing test holds work before it enters a batch; it does not prove cancellation can stop a merge already in progress.
Test Plan
TestCancelBeforeStartRetainsIntentcovers both normal delivery and the final recovery queue.The focused unit and integration tests and repository checks passed in the earlier local run; tests were not rerun for this description-only edit. Replay the integration suite with
./tool/bazel test //test/integration/submitqueue/orchestrator:go_default_test --test_output=errors.Revert Plan
Revert this PR; no schema migration is involved. Preserve held and final cancellation deliveries before rollback, because the previous handler can discard them while the Request is absent.
Issue Links
None — no public issue was supplied.