fix(orchestrator): fix score reconciliation bugs

[albertywu]

Summary

Fix score reconciliation so redelivery converges from the batch's durable state instead of repeating or regressing work.

Bugs

• The score controller treated every non-cancelling, non-terminal batch as needing to be scored. A stale score delivery could therefore move a batch that had already advanced to Speculating or Merging back to Scored.
• If the Scored state was persisted but publishing request logs or the speculate message failed, redelivery recalculated the score and wrote another Scored version instead of replaying fanout from the persisted checkpoint.
• storage.ErrVersionMismatch was not classified as retryable. An optimistic-lock race could therefore be rejected and sent to the DLQ instead of being redelivered to reconcile from fresh state.

Fix

• Reconcile explicitly from the current batch state:
  • Created: calculate the score and conditionally transition to Scored.
  • Scored: preserve the persisted score and replay request logs plus the speculate message.
  • Speculating or Merging: acknowledge the stale score delivery without regressing state.
  • Cancelling or terminal: acknowledge because another controller owns recovery.
• Return storage.ErrVersionMismatch to the consumer instead of retrying inside the controller.
• Add a SubmitQueue domain classifier that maps storage.ErrVersionMismatch to InfraRetryable, allowing consumer redelivery to reload and reconcile the latest state.
• Add regression coverage for downstream-state protection, version-conflict redelivery, and replay of fanout for an already-Scored batch.

Test Plan

✅ make fmt
✅ make lint
✅ make check-tidy
✅ make check-gazelle
✅ make build
✅ make test

Issues

Stack

1. docs(orchestrator): document controller correctness #398
2. @ fix(orchestrator): fix score reconciliation bugs #399

[behinddwalls]

The retry loop shouldn't be here. The consumer framework is already the retry loop: a nack re-enters Process, re-reads the batch, and re-dispatches on state — same convergence as the continue, via the broker.

The actual gap is classification: storage.ErrVersionMismatch is unclassified, so it defaults to non-retryable and dead-letters. Fix that once instead of patching it per controller:

• Drop the loop. On ErrVersionMismatch, bump version_conflicts and return the wrapped error.
• Add a domain classifier mapping storage.ErrVersionMismatch → InfraRetryable, composed into the pipeline consumers' ClassifierProcessor in the orchestrator wiring (domain-side — platform/errs can't see the sentinel).

That covers every CAS-writing controller (score, speculate, merge, cancel — the cancel DLQ race is this same bug). Controllers stay thin state machines; retryability stays in classifiers, which is where this repo puts it.

Keep the new state branches — that's the real fix. With the loop gone, add a direct test for redelivery of an already-Scored batch; it's currently only covered via the reload path.

[sbalabanov]

ErrVersionMismatch is likely always retryable?
In which case it could be created as infra_retryable right at the place where it is declared

[albertywu]

@behinddwalls @sbalabanov Addressed feedback in 5904769:

• Removed the controller-local retry loop. Process now performs one reconciliation pass, increments version_conflicts on storage.ErrVersionMismatch, and returns the wrapped error.
• Added a SubmitQueue domain classifier mapping storage.ErrVersionMismatch to InfraRetryable, and composed it into the orchestrator's primary consumer. Consumer redelivery now reloads durable state and re-enters the state machine.
• Kept the explicit state branches. An already-Scored batch reuses its persisted score and replays logs plus speculate; Speculating and Merging are acknowledged without regressing state.
• Added direct tests for version-conflict return and redelivery of an already-Scored batch.

I kept ErrVersionMismatch as a plain storage sentinel rather than constructing a classified error at its declaration. This follows the existing extension error contract: storage reports domain errors, while the consumer's classifier owns workflow retry policy. The resulting behavior is still that version mismatches are retryable for SubmitQueue pipeline consumers.

[sbalabanov]

If Runway and Stovepipe use the same optimistic concurrency pattern with database checkpoints, then we have to

• either duplicate such a classifier across all services
• or come up with a platform alternative - but it should not have access to storage-specific sentinels
• or go with "classify at declaration" path

which one to prefer?

[albertywu]

If Runway and Stovepipe use the same optimistic concurrency pattern with database checkpoints, then we have to either duplicate such a classifier across all services
or come up with a platform alternative - but it should not have access to storage-specific sentinels
or go with "classify at declaration" path
which one to prefer?

For classifier placement, I prefer the SubmitQueue domain classifier for now:

• Runway does not currently use optimistic-concurrency storage.
• Stovepipe handles its version conflicts through local reload and reconciliation loops rather than consumer-level redelivery.
• A stale CAS is not retryable by repeating the same storage operation. The retryable operation is the higher-level workflow that reloads current state and reconciles. Classifying at the storage declaration would attach that workflow policy to the extension and conflict with the current plain-extension-error contract.

If another domain later needs the same consumer-level semantics, I think the appropriate extraction is a platform-neutral optimistic-conflict marker plus generic classification. That avoids importing domain storage sentinels into platform code without making the storage extension directly own transport retry policy.