Repository navigation
fix(start): recover retained requests in place - #1040
sbalabanov wants to merge 3 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: Keep start recovery tests focused on request state, log status, version, and replay instead of requiring retained terminal diagnostic contents. Test Plan: Existing start and pipeline tests pass without a terminal-details fixture or timestamp, error, and metadata fidelity assertions. No runtime code or new tests. Revert Plan: Revert this test-only change.
TL;DR: Recover submissions that failed to start instead of leaving them stuck as Accepted.
Summary
A submission could stay Accepted forever if its first processing step failed before saving the request. The recovery handler used to discard that message because it found no saved request. It now retries the original submission normally, without restarting or overwriting work that has already moved on or finished.
Builds on #1039.
Repro
start_dlq, the recovery queue.validate, without another submission.Test Plan
Prior local verification: 19/19 test targets passed, including the service integration test; formatting and repository checks also 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.