docs(specs): assign the stale-key re-run to the S17 write path

Dzmitry (Dima) Meshcharakourequested to merge
dmeshcharakou/s04a-spec-retry-owner into main

What

S04-A said the caller re-acquires the namespace key and retries when the ns_key_id FK refuses a credential write stamped from a retired key. It named no caller, and no caller does it. This assigns the re-run to the owner that can perform it, and records the obligation on that owner's own spec.

Ownership is split rather than moved wholesale: the S17 management write path owns the rollback and the re-run, while the key-cache eviction hook that makes a re-run recover rather than repeat the refusal belongs to internal/crypto and to S04-B's rotation and retirement tooling.

Raised in review on !2553 (merged), where the sentinel this recovery is built on has no consumer outside tests.

Why the datastore write could not be the owner

The rejection aborts the transaction the datastore credential-write helper runs in, so that helper cannot re-acquire a key and re-issue the write inside the same call. It logs the rejection and returns its format's stale-key sentinel.

The management write path specified in S17 composes credential writes into one transaction with the url write, which makes it that transaction's owner rather than a layer outside it. Its recovery is therefore a rollback and a re-run of the whole operation, not a re-issue of the one write: the url write, the last_health_status and last_health_checked_at resets and, on create, the kind-agnostic and per-format child inserts all re-run with it.

The re-run is also not immediate. Step 1 resolves the key through KeyManager.GetKey, which serves a live cache entry without touching the store, so a key another pod retired is re-served until that entry's TTL lapses and a prompt re-run is refused identically. Closing that needs an eviction hook on the key manager, internal/crypto's to expose and S04-B's rotation and retirement tooling to call.

What changed

Two files. Line numbers are at 85f21cd8e.

docs/specs/S04-a-column-level-encryption.md, one commit per review finding across both rounds:

Passage Change
:381 FK-rejection paragraph: names the S17 write path, why the helper cannot re-run, and that the re-run waits on the key cache
:1454 Write path: the helper does not retry and returns its format's stale-key sentinel, named rather than described
:1463 Recovery is S17's rollback and re-run, with what re-runs alongside it, and the key-cache window that refuses a prompt re-run
:1491 The construction claim names remote.CredentialManager, the one undefined component name the rename pass left behind
:1499 The constraint name keys the mapping; the retry site keys on the sentinel and never sees a constraint name
:1511 The warn line is booked where the stamped key id is in hand, not where the rejection is classified
:1815 Crypto-shredding passage: the same owner and key-cache corrections
:2242 Error Cases row: splits the two actors and carries the same corrections in its one cell

docs/specs/S17-rest-management-api.md, recording the obligation on the receiving side and keeping the section it lands in self-consistent:

Passage Change
:50 References gains the S04-A entry the list did not carry
:85 Dependencies gains an S04-A row: the integration point, and the state measured at f9cd4b12f - the encrypted-credential columns and their validated check constraints on all three *_remote_repositories tables, each format's Err<Format>RemoteStaleNamespaceKey declared in internal/datastore, and no package outside internal/datastore consulting one
:1008 Non-hosted repository CRUD (Phase 6): the refusal, the sentinel the datastore helper returns, the rollback and re-run this surface owes, the writes that re-run with it, the eviction enqueue the rollback does not undo, and the key-cache window that refuses a prompt re-run
:1013 Points at the refusal's own Error Cases row rather than at the table's catch-all
:1020 The eviction enqueue precedes the commit, matching the acceptance criterion at :1526 and the backend rule at :1022
:1624 Error Cases gains the Create and Update row for the refusal
:1670 Security Considerations: the same enqueue ordering, stated through the mechanism that earns the claim

The accepted failure window is written as a condition and its event, and it closes on two events: the S17 Phase 6 endpoints are what make the refusal reach a user, and the eviction hook is what makes a re-run recover rather than repeat the refusal.

The refusal now carries its own Error Cases row on Create and Update. That row invents no code: it answers the same 500 and internal_server_error the table's unexpected-server-failure catch-all answers, so what changed is that the refusal is itemized rather than folded into the catch-all. docs/dev/writing-discipline.md is why it earns a row, under ## A contract table listing one status of a class lists the whole class: the table already itemizes the Connection test's 500, so listing none of that class for Create and Update read as a deliberate selection rather than a uniform absence. The gap sits in the row as a condition and its event.

Accepted cost

Two costs, and they are different kinds. Each applies to a different pair of open MRs, and neither pair carries the other pair's cost.

Spec text that moved under work in flight. Two S17 Phase 6 step MRs are open against the Phase 6 plan: !2665 (merged) (37/42) and !2666 (merged) (38/42), the markers read off those MRs' own titles and the denominator re-counted from that plan's Status table at 85f21cd8e, which carries 42 rows. They implement steps 37 and 38, virtual create and delete for npm and container, which the same Non-hosted repository CRUD (Phase 6) section this MR edits specifies. The paragraphs this MR adds to that section are remote-credential text, which a virtual create or delete does not execute, so what those two get is a section that moved under them mid-flight rather than behavior their code now has to serve. That was accepted over a separate S17 amendment MR or a hard gate, so the obligation and the spec that has to serve it land together. Neither carries any overlap cost: git diff --name-only origin/main...origin/mr/2665 returns 22 files at d5bb0662d and the same command for origin/mr/2666 returns 26 at 079197162, and no path in either list is under docs/specs/.

File overlap on the S17 spec. The two open MRs that do edit docs/specs/S17-rest-management-api.md are !2652 (merged) (1150/spec-s17-parents-count-detail-only), whose diff against origin/main is that one file, and !2660 (merged) (1150/spec-tag-index-claims), whose diff is that file plus docs/specs/S12-container-oci-hosted.md, which this MR does not touch. Both come back exit 0 from git merge-tree --write-tree --name-only --no-messages HEAD <mr head>, re-derived at 85f21cd8e against 04aedb1c2 and 1f9dd80f5, so no merge order is required. ## Merge order below carries the section-by-section reason. Neither of these two is a Phase 6 step MR, so neither carries the moved-under cost above.

What this does not do

  • No plan change. The obligation belongs to the S17 management write path, not to a new S04-A step, so the per-format columns plan keeps its 11 Status rows and the 4/11 markers on !2553 (merged) and !2554 (merged) stay correct, re-counted at f9cd4b12f.
  • No work item. Recording the obligation in S17 is the first of the two options the review offered; the fallback, a work item cited from S04-A, is not used.
  • No second copy of the store-side rules. internal/datastore/maven_remote_repositories_errors.md landed with !2553 (merged) and already rules that nothing retries in the store, gives the key-cache mechanism, and names the eviction hook as the real blocker. This MR cites that document rather than restating it.

Merge order

None required, re-derived at 85f21cd8e: 76 open MRs, 0 unchecked heads. Four of them edit one of the two files:

All four come back exit 0 from git merge-tree --write-tree --name-only --no-messages HEAD <mr head>.

Two heads needed a forced refspec to fetch at all, !2630 (merged) and !2622 (merged), both having been force-pushed since the previous scan. A plain refs/merge-requests/<iid>/head fetch skips a non-fast-forward head in silence, so the scan reports them as unchecked rather than clean; + in front of the refspec is what makes it apply. !2622 (merged) edits neither file.

On the S04-A side, both edit the identifier-limit section and the check-constraint discussion under it, and nothing else: !2630 (merged) spans main :417 to :509, !2662 (merged) a single hunk at main :470 to :491. This MR's S04-A passages are the FK-rejection paragraph, the write path, the accepted-failure-window paragraph, the crypto-shredding passage and the Error Cases row, so the two sets share no section. !2662 (merged)'s earlier hunk in the crypto-shredding purge order, named here in a previous revision, is gone as of its 7c4bef5f0 head.

On the S17 side, !2660 (merged) is a single hunk at main :748, in the Phase 3 tag-index discussion. !2652 (merged)'s hunks span the Phase 3 manifest and list sections, the Acceptance Criteria block (main :1582 to :1592) and the Resolutions block (main :1837 to :1950). Neither touches ## Error Cases (main :1596 to :1647) or ## Security Considerations (main :1647 to :1692), which is where this MR's :1624 and :1670 land, so the two new S17 hunks sit in sections no other open MR edits.

!2642 (merged) edits the sibling S04-B spec, not either file. !2542 (merged) and !2543 (merged), named here in an earlier revision, have both merged.

Checks

markdownlint-cli2, vale and lychee run individually against both files, all three passing on both at 85f21cd8e. lychee runs with --include-fragments, so the cross-spec anchors resolve in both directions: S04-A's #non-hosted-repository-crud-phase-6, #remote-repository-settings-phase-6 and #dependencies into S17, and S17's #write-path into S04-A alongside its own #error-cases and #non-hosted-repository-crud-phase-6.

Related to #1212 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports