[sbalabanov]

TLDR: Keep start-payload identity decoding private to the dedicated start DLQ without changing deserialization.

Summary

Move the start-only identity helper into the start DLQ controller's file and make it private. It continues to use UnmarshalLandRequest, preserving complete canonical wire validation and the existing identity checks. Update the existing test reference and the Start DLQ wiring example.

Based on #1040, which introduces the dedicated start DLQ controller.

Repro

Run ./tool/bazel test //submitqueue/orchestrator/controller/dlq:go_default_test --test_filter='TestDLQStartController_(ReconcilesOnlyError|RejectsMalformedPayload)' --test_output=all. Complete Start payloads retain their existing reconciliation behavior; malformed known fields and invalid identities are still rejected.

Test Plan

• Existing Start DLQ reconciliation, malformed-payload, and retry tests cover the unchanged helper behavior; all five focused DLQ, message-contract, pipeline, primary-start, and gateway controller targets pass.
• The DLQ build target, make gazelle, make fmt, make lint, and make check-gazelle pass.

Revert Plan

Revert this PR.

Issue Links

None.

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[sbalabanov]

If DecodeLandRequestID is only used by "start" dlq, should it be placed there? Can put it on top of a PR implementing a separate start dlq.
Also, the goal was to make deserialization as safe as possible, so if LandRequest slightly changes, ID and queue could still be safely deserialized. Present me with options; if switching away from deserialization helper only brings us subtle performance/memory saving benefits, it does not worth the additional complexity of the code.

[sbalabanov]

In reply to your review: yes, this is start-specific. On main only the start-DLQ registration uses it; the separate start-DLQ controller becomes its sole production caller, so the helper can be private in that controller's file.

Both the existing helper and this PR deserialize the complete generated Start message with the same canonical protojson options. Unknown additions are already tolerated. LandRequestFromStart is currently just a conversion, with no validation/error return. Skipping it avoids that coupling and an unused URI copy/strategy mapping, but does not improve recovery from invalid known wire fields; both versions reject change:false.

Options:

1. Keep the existing deserialization helper (recommended minimal option). Do not carry this standalone conversion cleanup. When the separate start DLQ is in place, move/unexport the start-only helper there. This preserves current full-wire validation with the least code.
2. Keep direct full-Start decoding, local to the separate start DLQ. This can sit on top of that controller change, but its benefit is only avoiding the unused domain conversion and its coupling, not stronger compatibility. I would not keep it just for small memory/work savings.
3. Make start-DLQ recovery explicitly identity-only. Read the whole JSON document, but allow unusable non-identity land fields while requiring an unambiguous ID/queue and retaining tenant checks. That would recover change:false, unlike either current implementation. The tradeoff is intentionally relaxing Start-field validation in the DLQ only; it needs explicit compatibility/identity-safety rules and tests, not merely replacing the helper call. Primary start processing would retain full validation.

For the resilience goal you described, option 3 is the meaningful behavior change; otherwise I recommend option 1. No code, stacking, or PR-state changes made while waiting for your choice.

[addressed by agent]

[sbalabanov]

In reply to your review: yes, this is start-specific. On main only the start-DLQ registration uses it; the separate start-DLQ controller becomes its sole production caller, so the helper can be private in that controller's file.

Both the existing helper and this PR deserialize the complete generated Start message with the same canonical protojson options. Unknown additions are already tolerated. LandRequestFromStart is currently just a conversion, with no validation/error return. Skipping it avoids that coupling and an unused URI copy/strategy mapping, but does not improve recovery from invalid known wire fields; both versions reject change:false.

Options:

1. Keep the existing deserialization helper (recommended minimal option). Do not carry this standalone conversion cleanup. When the separate start DLQ is in place, move/unexport the start-only helper there. This preserves current full-wire validation with the least code.
2. Keep direct full-Start decoding, local to the separate start DLQ. This can sit on top of that controller change, but its benefit is only avoiding the unused domain conversion and its coupling, not stronger compatibility. I would not keep it just for small memory/work savings.
3. Make start-DLQ recovery explicitly identity-only. Read the whole JSON document, but allow unusable non-identity land fields while requiring an unambiguous ID/queue and retaining tenant checks. That would recover change:false, unlike either current implementation. The tradeoff is intentionally relaxing Start-field validation in the DLQ only; it needs explicit compatibility/identity-safety rules and tests, not merely replacing the helper call. Primary start processing would retain full validation.

For the resilience goal you described, option 3 is the meaningful behavior change; otherwise I recommend option 1. No code, stacking, or PR-state changes made while waiting for your choice.

[addressed by agent]

In that light, keep using UnmarshalLandRequest(). However, refactor the whole helper function to be private and move to a new start dlq controller introduced in another PR. Base this PR on top of that another PR.

[sbalabanov]

In reply to your comment: moved the identity helper into dlq/start.go as private decodeLandRequestID in 2a08ae39.

It still calls UnmarshalLandRequest(), with deserialization and controller identity checks unchanged. This PR now targets the dedicated start-DLQ PR #1040's branch. Existing Start DLQ reconciliation, malformed-payload, and retry coverage, the focused test targets, and build/lint checks pass.

[addressed by agent]