fix: preserve publish submissions on registration conflict

[jcfischer]

Summary

• stop retrying the state-changing version registration POST after server failures
• treat visible in-flight/review submissions as successful publish handoffs instead of immutable duplicate failures
• probe /versions/:version/submission on 409 before reporting a true duplicate-version conflict
• format non-public submission states like validating and audit as uploaded submissions in CI output

Fixes #199.

Verification

• rtk bun test test/unit/publish.test.ts
• rtk bun test test/commands/publish.test.ts
• rtk bunx tsc --noEmit
• rtk bun run lint
• rtk bun test

[jcfischer]

Code Review — fix: preserve publish submissions on registration conflict

Lenses applied: CodeQuality, Security, Architecture

Summary

Clean, well-scoped fix for a real operational problem (issue #199 — Soma 0.8.2/0.8.3 publish failures). Three behavioral changes, each with regression coverage:

1. Remove retry loop on state-changing POST — correct. The for (attempt < 2) retry was unsafe because the server may have already created rows before returning 5xx. Single-attempt + extract-submission-from-error-body is the right call.

2. Probe /versions/:version/submission on 409 — correct. Distinguishes "version truly published and immutable" from "version row exists but submission is still in-flight/review." The fallback chain (inline submission → probe → immutable error) is sound.

3. normalizeSubmission + VISIBLE_SUBMISSION_STATUSES — good extraction. Centralizes the wire-format variance (nested .submission, flat submission_id/submission_status, or direct id/status) into one normalizer. The status allowlist is explicit and correct for the use case.

Findings

No blockers, no majors.

Nits (0 actionable, 1 observational):

• The SubmissionBody type intersection (& SubmissionWire) means a body with just { id, status } at the top level is also accepted. This is intentional (the probe endpoint returns flat { id, status }), but worth a one-line comment on the type explaining why the intersection exists — future readers may wonder if it's accidental. Not blocking.

Verification

• bun test test/unit/publish.test.ts — 47 pass ✅
• bun test test/commands/publish.test.ts — 16 pass ✅
• bunx tsc --noEmit — clean ✅
• All four acceptance criteria from issue #199 covered by the new tests

Test coverage assessment

The three new unit tests directly map to the issue's acceptance criteria:

• 502 + inline submission → no retry, extract submission ✅
• 409 + visible submission via probe → success ✅
• 409 + no visible submission → immutable error preserved ✅

The two new command-level tests verify end-to-end publish() + formatPublish() for the validating status path and the true-conflict path. Good layering.

Verdict: approve (posted as comment — can't self-approve). Ship it.

[jcfischer]

CodeQuality + Security Review

Lenses applied: CodeQuality, Security, Architecture

Summary

Clean, well-motivated fix that addresses all four acceptance criteria from #199. The behavioral changes are correct and the test coverage is thorough.

What this PR does right

1. Retry removal — State-changing POST retries were the root cause. Removing the retry loop is the correct fix; the server may have already committed rows before returning 5xx.

2. normalizeSubmission() extraction — Centralizes three wire shapes (nested .submission, flat submission_* prefixed, and flat bare id/status) into one normalizer. Reduces duplication vs the old inline destructuring.

3. Inline submission check before status dispatch — A 502 that created a submission now returns success: true with the submission data instead of retrying into a 409. The VISIBLE_SUBMISSION_STATUSES guard prevents random error-body fields from triggering false positives.

4. 409 submission probe — GET /versions/:version/submission on 409 distinguishes "version exists with an active submission" (success) from "version exists and is published/immutable" (error). encodeURIComponent(payload.version) in the URL is good defensive practice.

5. formatPublish generalization — Now handles all submission statuses, not just pending_review. CI output will correctly show "submission validating" etc.

Test coverage

• ✅ 409 + visible submission → success (unit + integration)
• ✅ 409 + no visible submission → immutable error preserved (unit + integration)
• ✅ 502 with inline submission → success, no retry (unit)
• ✅ 500 → no retry, failure (unit)
• ✅ Existing rejected-submission test unchanged and passing

Observations (non-blocking)

• The inline-submission check runs for ALL non-2xx responses before status-specific handlers (400, 403, 401). In theory a 403 response with submission_status: "validating" would be treated as success. In practice the server won't include those fields on auth errors, and the VISIBLE_SUBMISSION_STATUSES filter makes accidental matches very unlikely. Acceptable as-is.

• rejected in VISIBLE_SUBMISSION_STATUSES means registerVersion returns success: true for rejected submissions. This is correct — the command-level publish() converts rejected submissions to user-facing failures. The existing "rejected publish submission fails" test confirms this path.

Verdict: approve. blockers=0 majors=0 nits=0. (Can't formally approve own PR — posting as comment.)