fix(review): scope fast-path action IDs by run

[rianjs]

Summary

• scope approval-override and partial-repair planned-action IDs by their owning run
• preserve legacy constant-ID rows without a migration or purge
• add regressions for both collision paths and assert legacy rows remain unchanged

Root cause

Both fast paths allocated a fresh run but inserted process-wide constant action IDs into planned_actions, where action_id is the table-wide primary key. The first persisted approval override or repair action therefore blocked every later execution of that path in the same ledger.

Empirical validation

• read-only inspection found approval-override-submit-review already posted for PR fix: make review workbenches location-independent and resume-safe #507
• the active schema confirmed planned_actions.action_id is the primary key
• PR feat(review): layer repo-local and shared reviewer selection #517 classified a valid author override as approve=true, then failed the insert with SQLite constraint 1555
• no new PR feat(review): layer repo-local and shared reviewer selection #517 run survived after the failed insert, matching cleanup behavior
• before the production change, focused approval-override and repair tests both reproduced the exact unique-constraint failure

Test plan

• focused approval-override and repair regressions pass
• go test ./internal/gateio ./internal/ledger
• go test ./...: 2,758 tests passed across 66 packages
• golangci-lint run: 0 issues
• build passed

The independent GPT-5.6 Sol problem/plan review converged with no blockers or majors. Its compatibility finding was incorporated by asserting the seeded legacy rows remain unchanged.

Note: make check currently stops on pre-existing merged-main tidy/fmt drift from #517 (github.com/gobwas/glob remains marked indirect, plus three unrelated files require go fmt). Those unrelated changes are intentionally excluded from this PR; the remaining format, lint, test, and build gates passed.

Closes #518

[rianjs]

Architect review

No findings. The change is minimal and ticket-scoped, preserves legacy rows, covers both collision paths, and uses the existing marker rendering flow. Focused tests and the full suite pass (2,758 tests across 66 packages).

[rianjs]

TDD assessment

• Major: The tests cover collision with legacy constant IDs, but not uniqueness across two post-fix executions. An incorrect implementation using one new fixed ID could pass these tests. Add a regression that runs each fast path twice with different run IDs and verifies both actions are retained with distinct marker IDs.

The legacy-row compatibility checks and both fast paths are otherwise covered without over-mocking.

[rianjs]

Post-TDD architect review

No blockers, majors, or minors.

Nit: TestSpecialSubmitReviewActionIDsAreRunScoped may appear duplicative of the integration regressions. It is retained because those regressions cover one legacy constant plus one new execution, while this test covers two post-fix executions and prevents replacing the old constants with different fixed IDs.

Full suite: 2,761 tests across 66 packages.

[rianjs-bot]

Automated PR Review

Reviewed commit: 349c45a1d6f4
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	0

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_constrained	internal/gateio/gateio.go, internal/gateio/gateio_test.go	unavailable	Focused Go test command could not complete because the local compiler invocation mishandled the workspace path containing spaces; review was based on the diff and existing test seams.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 1m 07s | unavailable | gpt-5.6-terra | cr 0.10.260

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	1m 07s wall · 44s compute
Cost	unavailable
Tokens	131.7k in / 1.4k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	15.5k	153	10.5k	unavailable	unavailable	5s
go:implementation-tests	gpt-5.6-terra	84.2k	1.1k	54.0k	unavailable	unavailable	36s
orchestrator-rollup	gpt-5.6-terra	32.1k	189	25.6k	unavailable	unavailable	2s

[rianjs]

Final architect pass

No blockers, majors, minors, or post-review architectural drift at HEAD 349c45a.

The prior nit about the two-execution regression remains acknowledged; the test is retained because it covers a distinct post-fix-to-post-fix collision invariant identified by the independent TDD assessment.