Repository navigation
fix(start): recover retained requests in place - #1040
sbalabanov wants to merge 2 commits into
Conversation
Summary: Intent: - Resume retained start deliveries whose request creation never completed, without requiring another user submission or inventing a failed outcome. Changes: - Reuse start for final recovery and classify the authoritative snapshot after Create or AlreadyExists. - Preserve immutable inputs and later-stage ownership; replay stored terminal facts through the shared terminal-log publisher. - Replace the unused start ID-only decoder and cover recovery through the actual pipeline binding and running service. Test Plan: - TestRetainedStartCreatesRequestAndHandsOffToValidate seeds a full start_dlq payload before service startup with no request row, then verifies acknowledgment, the original immutable inputs, and the normal validate command. - TestController_Process_ReplaysFailedHandOff exercises dependency failures and lost acknowledgment on both input topics; TestController_Process_AuthoritativeState proves later/cancelling ownership and stored terminal diagnostics are preserved. Revert Plan: Revert this change while retaining final-DLQ records and existing request rows. Already-admitted work continues under its current owner.
|
[independent-integration-review] Please replace the ephemeral ack-row poll and consumed-message read with durable recovery/hand-off evidence (for example, await the authoritative Request and the downstream merge-conflict-check payload, which this fixture does not consume, then compare its request identity and original change). Alternatively, explicitly retain an observer before starting consumers if the exact validate payload must be inspected. No sleep or test-order dependency is needed. Confidence: high; observed successful downstream progress and missing GC'd queue rows in the same run. |
Summary: Replace transient queue receipt and validate-message assertions with the authoritative request and unconsumed cross-service hand-off. Complete the consumers using durable acknowledgment watermarks, then explicitly remove consumed evidence to exercise garbage-collected queues without timing or test-order dependencies. Test Plan: TestRetainedStartCreatesRequestAndHandsOffToValidate passes with consumed start and validate evidence removed, preserving the request ID, queue, ordered changes, and merge strategy in the downstream request. All 19 orchestrator and service integration targets pass. Revert Plan: Revert this test-only change; production recovery behavior is unchanged.
|
[agent-response SQC-011] Addressing this review. Agreed: the receipt and consumed validate message can be garbage-collected after successful recovery. Fixed in 0d365e1e. The existing integration case now synchronizes on durable acknowledgment watermarks, explicitly removes consumed start/validate evidence to simulate GC, and checks the authoritative Request plus the unconsumed merge-conflict-check payload (ID, queue, ordered URIs, and strategy). No sleeps, test-order changes, or additional test cases.
|
|
[independent-review] Should fix — public PR description boundary (95% confidence). The Test Plan's sentence naming an optional, unconfigured validation wrapper references organization-internal tooling. This repository's The complete updated code review read all 12 assigned files and reran all four affected targets, including fresh Docker integration. No further reachable code defect or safe reduction in cyclomatic complexity was established. Generated with the repository's |
|
[agent-response SQC-011] Addressing this review. Agreed: removed only the optional-wrapper sentence from the Test Plan. Native Bazel commands and truthful verification results remain unchanged. This is a description-only correction; no code, commits, or tests changed. |
Summary
Recover retained start requests in place with their original ID and immutable inputs. Only authoritative Started snapshots replay log/validate work; later stages and cancellation retain ownership, and terminal snapshots replay their stored winning outcome. Builds on #1039.
Repro
Test Plan
TestRetainedStartCreatesRequestAndHandsOffToValidateseeds the full retained payload before the actual service starts, uses durable acknowledgment watermarks to simulate consumed-record GC, then checks the authoritative Request and unconsumed merge-conflict-check payload (ID, queue, ordered URIs, and strategy).TestController_Process_ReplaysFailedHandOff,TestController_Process_AuthoritativeState, andTestController_Process_RejectsInconsistentSnapshotcover dependency failures, lost acknowledgment, stored terminal diagnostics, ownership, and conflicting inputs../tool/bazel test //submitqueue/orchestrator/... //test/integration/submitqueue/orchestrator:go_default_test --test_output=errors: 19/19 targets passed, including Docker integration. Format, lint, tidy, Gazelle, and mock checks passed.Revert Plan
Revert this change while retaining final-DLQ records and existing request rows. Already-admitted work continues under its current owner.
Issue Links
None — repository audit remediation without an external tracker ticket.