feat(npm): meter the publish window between blob and row commits

What this delivers

This merge request adds one Prometheus counter, gitlab_artifact_registry_npm_publish_post_blob_commit_failures_total. It counts npm publishes whose tarball blob committed to content-addressed storage and whose npm rows then did not. That publish referenced nothing it stored. A later publish of the same tarball deduplicates against the same blob row and attaches it, which is the scenario #910 reports, so the clause is scoped to the publish rather than stated as an absolute.

This merge request meters that window. It does not fix the defect #910 reports, and that is the point of it.

The issue lists remedies and records that nothing measures how often the window is hit. This change meters the window first. The remedy decision follows later, from the data the counter produces, and this merge request takes no position on which remedy to choose.

The under-counted namespace_statistics.deduplicated_size_bytes the issue reports stays under-counted. One GAP verdict in docs/dev/storage-accounting.md belongs to this window, and it still reads GAP. It is the deduplicated_size_bytes cell of the "publish that fails after committing the tarball blob" row. That file carries 17 GAP verdict cells in its emit-coverage grids, and the other 16 sit outside this window. This command counts them, at this branch's head:

awk -F'|' 'NF==7' docs/dev/storage-accounting.md | grep -o '`GAP`' | wc -l

A grep -c GAP on the same file also returns 17, and that agreement is a coincidence. That number counts lines, 3 of which belong to this window, and the 17 verdict cells sit on 9 lines. No accounting behavior changes and no emit site changes.

Two sites increment the counter, both in internal/format/npm:

Site	Where	What it counts
A flag-and-defer chokepoint	PublishHandler.commitPublish, publish_commit.go	Every exit from the commit stage that leaves the npm rows uncommitted, a panic and a runtime.Goexit included
One direct .Inc()	PublishHandler.writeCoherenceError, publish_stream.go	The coherence 422, which returns before the commit stage opens

The window opens at the upload-session commit in PublishHandler.stagePublish. It closes when the PublishCommitter.CommitPublish seam reports the rows written.

No plan file under docs/plans/ covers this change. It is one issue's work, driven from bug issue #910. In that issue the operator chose the metering-only remedy from four worked options. The type is feat because the change adds one metric series.

How to read the counter, and what it cannot tell you

• It counts entries into the window, not bytes. A non-zero rate says that publishes end inside the window. It does not say how many blobs leaked, and it says nothing about how much storage they hold.
• It is a lower bound on entries into the window. The increment is deferred inside PublishHandler.commitPublish, so a panic there still counts. A process death records nothing: an OOM kill, a SIGKILL, or an os.Exit never runs the defer. Two in-process panic sites also record nothing, and both sit after the blob commit and before control enters commitPublish. They are the coherenceCheck call, and writeCoherenceError's own logging ahead of the two increments it makes.
• A blob can be stranded without the window opening at all. storage.Session.Commit moves the object to its content-addressed path before it writes the blob row. A commit that then fails on the transaction leaves an object standing with no blob_storage_blobs row. The publish answers 500, the window never opens, and this counter does not move. No job in the tree reclaims that object. ADR-025's gc:reconcile-scan reads blob rows and never enumerates storage, so an object with no blob_storage_blobs row is outside it. ADR-025 assigns that class to the deferred data-reconciliation service of ADR-011.
• It carries no namespace label. The cardinality rules in internal/metrics/cardinality.go refuse namespace_id and namespace_slug outright.
• It cannot see what decides whether an entry cost anything. It cannot see whether the publish was retried, whether that retry deduplicated, or whether a reconciliation pass already corrected the namespace.
• Each entry is separately an upper bound on wasted storage. The blob is content-addressed, so an entry costs nothing when the refused tarball byte-matched a blob the same namespace already held.
• The baseline is not zero in ordinary operation. Several arms enter the window with nothing defective behind them, and the list is open rather than closed. They are the coherence 422s; a per-version object that npm.max_envelope_package_json_size admits and npm.max_package_json_size then refuses, a client-reachable 422 raised after the tarball has committed; a row transaction that fails on a canceled request context, which a client hangup mid-commit reaches and which a drain that outruns server.shutdown.timeout reaches at every rollout whose drain does not finish; the commit-time 409; and the in-transaction version-count and tag-count re-checks. Subtracting the per-code series removes the coherence route and nothing else, so neither the raw series nor that difference has a zero baseline. Alert on a rate change rather than on any non-zero value.

internal/format/npm/metrics.md carries the full reading and the complete list of exits that record nothing, and docs/dev/observability.md carries the one-row summary.

The subtraction against the coherence series

..._npm_publish_coherence_failures_total breaks the coherence route down per code. The window series minus that one approximates the arms that carried no metering before, the commit conflicts and the transaction faults among them.

Read that difference as an approximation rather than as an identity, and read it with its excursions named. It holds while every entry into the window reaches one of the two increment sites. One exit breaks it in a fixed direction: a panic at the coherenceCheck call loses the window increment while the per-code series has nothing to lose, so the difference then undercounts the arms it names. A process death runs no deferred increment either, and it also drops every count the process holds that no scrape has taken, so it moves the difference in no fixed direction, and that loss is bounded neither to commitPublish nor to this window. One exit leaves the difference alone: a panic inside writeCoherenceError ahead of the two increments it makes. The coherence route increments both series, so a panic ahead of them loses both and the difference stays correct.

Evidence from a running service

The counter was driven against a local service at 61c698531, and it moved 0 to 1 to 2. Later rewrites replaced that commit, so it is no longer on the branch, but the project still holds it at /-/commit/61c698531. Each row is one request, with a scrape of /-/metrics before it and after it.

#	Request	HTTP	Error code	Window counter	Coherence counter
1	first publish of window-demo@1.0.0, matching dist.shasum	201	—	+0	+0
2	republish of the identical envelope	409	version_exists	+0	+0
3	publish of window-tagdemo@1.0.0 under a semver-shaped dist-tag	422	tag_name_invalid	+0	+0
4	publish of window-coh@1.0.0, tarball package.json names a different package	422	manifest_coherence_failed	+1	{coherence_code="manifest_coherence_failed"} +1
5	publish of window-sha@1.0.0, dist.shasum declared as 40 zeros	422	shasum_mismatch	+1	{coherence_code="shasum_mismatch"} +1

git diff 61c698531..HEAD -- internal/format/npm/metrics.go internal/format/npm/publish_commit.go internal/format/npm/publish_stream.go reports executable differences, and none of them is this branch's. Each one is a collector that reached this branch from main after 61c698531, and all of them sit in internal/format/npm/metrics.go:

• virtualHealthMemoMisses.
• The hosted inline-build set: packumentInlineBuildDeadlineKills, distTagsInlineBuildDeadlineKills, inlineBuildDeadlineArmFailures, packumentInlineBuildDocumentBytes and distTagsInlineBuildDocumentBytes, with their kill_outcome constants and their bucket family.

This branch's own diff touches none of those lines, and none of them is a metered path. Every remaining difference across the three files is a comment or the Help string literal. In publish_commit.go the two rowsCommitted lines differ in their trailing comment alone, and publish_stream.go carries comment changes only. The scrape above quotes the first sentence of the Help, and that sentence is the same at both commits.

Both increments matched real orphaned blobs. After the five requests the database held only the successful publish, and window-coh and window-sha left no npm_packages row at all. The service logged the orphan at each increment with its blob digest, and both objects were present in object storage. Two increments, two orphans, zero rows.

Row 5 is the pair the subtraction rests on: the whole-window count and its per-code breakdown both move by 1 on one request.

Row 2 is worth a second look. A republish returns 409 version_exists from the fail-fast pre-check. That pre-check rejects inside the attachment gate, and the gate returns before it calls openSession. No session is opened and nothing is staged, so the counter correctly does not move. The commit-time 409 is the concurrent-race loser, and one client cannot reach it.

One counted arm cannot be reached by a single client: the commit-time version conflict needs a race between two concurrent publishes of one coordinate. The transaction fault is reachable from a client, because a hangup during the row transaction cancels the request context; the tests drive it through an injected database error instead. The tests below cover both.

Coverage

The rows come from the coverage table the test author wrote when the tests landed. Three rows are added for tests that arrived after that table was written: TestCommitPublish_CountsAPanicLeavingTheWindow, TestStagePublish_LosesTheEntryWhenWriteCoherenceErrorPanics and TestCommitPublish_LeavesTheCounterAloneWhenTheTrackerPanicsPastTheRows. The grouping is this description's own, because one route enters the window and is still not counted.

Counted: failures inside the window

Arm	Where	Test
Filtered version object over the package.json ceiling	publish_commit.go commitPublish	TestCommitPublish_CountsEveryRefusalBeforeTheRowTransaction/the_filtered_version_object_is_over_the_package.json_ceiling
Missing Resolution at the commit stage	publish_commit.go commitPublish	TestCommitPublish_CountsEveryRefusalBeforeTheRowTransaction/the_request_carries_no_resolution
filterPackageJSON cannot parse the version object	publish_commit.go commitPublish	TestCommitPublish_CountsEveryRefusalBeforeTheRowTransaction/the_version_object_does_not_parse_for_filtering
npmrules.ParseScope refuses the package name	publish_commit.go commitPublish	TestCommitPublish_CountsEveryRefusalBeforeTheRowTransaction/the_scope_parser_refuses_the_package_name
Server-computed digest is not 32 bytes	publish_commit.go commitPublish	TestCommitPublish_CountsEveryRefusalBeforeTheRowTransaction/the_server-computed_digest_is_not_32_bytes
409 version_exists	publish_commit.go writeCommitError	TestStagePublish_CountsRowsThatFailAfterTheBlobCommits/the_version_already_exists
422 version_count_exceeded (in-transaction re-check)	publish_commit.go writeCommitError	TestStagePublish_CountsRowsThatFailAfterTheBlobCommits/the_transaction_re-check_finds_the_version_cap_reached
422 tag_count_exceeded (in-transaction re-check)	publish_commit.go writeCommitError	TestStagePublish_CountsRowsThatFailAfterTheBlobCommits/the_transaction_re-check_finds_the_tag_cap_reached
500 fall-through on an unrecognized commit error	publish_commit.go writeCommitError	TestStagePublish_CountsRowsThatFailAfterTheBlobCommits/the_transaction_faults_on_something_unrecognized
422 coherence refusal	publish_stream.go writeCoherenceError	TestStagePublish_CountsACoherenceRefusalAfterTheBlobCommits
A runtime.Goexit leaving the chokepoint	publish_commit.go commitPublish	TestCommitPublish_CountsAGoexitLeavingTheWindow
A panic leaving the chokepoint, which still propagates	publish_commit.go commitPublish	TestCommitPublish_CountsAPanicLeavingTheWindow

Not counted: routes that never enter the window

Route	Where	Test
Pre-commit package_json_too_large from the tarball inspector	publish_stream.go writeStageError	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_tarball_package.json_is_over_the_ceiling
Pre-check version_exists	publish_precheck.go runPrecheck	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_pre-check_finds_the_version_already_published
Pre-check version_count_exceeded, active rows at the cap	publish_precheck.go versionCountVerdict	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_pre-check_finds_the_active_rows_at_the_version_cap
Pre-check version_count_exceeded, tombstoned predecessors	publish_precheck.go versionCountVerdict	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_pre-check_reaches_the_version_cap_through_tombstoned_rows
Pre-check tag_count_exceeded	publish_precheck.go tagCountVerdict	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_pre-check_finds_a_new_dist-tag_at_the_tag_cap
The upload session's own commit fails	publish_stream.go writeStorageError	TestStagePublish_LeavesTheCounterAloneBeforeTheBlobCommits/the_upload_session's_own_commit_fails

The last row carries a caveat, and this counter cannot reach it. storage.Session.Commit moves the object to its content-addressed path before it writes the blob row. A commit that fails on the transaction after that move leaves an object standing with no blob_storage_blobs row. The window still does not open, because the commit never reported success. The row is therefore right about the window and says nothing about what was stranded. No job in the tree reclaims that object: ADR-025's gc:reconcile-scan reads blob rows and never enumerates storage, so a row-less object is outside it. ADR-025 assigns that class to the deferred data-reconciliation service of ADR-011.

Not counted: routes inside the window that are not failures

Route	Where	Test
A publish whose rows committed	publish_commit.go commitPublish	TestStagePublish_LeavesTheCounterAloneOnTheSuccessPaths/the_rows_commit
The interim 200 with no committer wired	publish_stream.go stagePublish	TestStagePublish_LeavesTheCounterAloneOnTheSuccessPaths/no_committer_wired_answers_the_interim_200
A committed publish whose usage tracker then panics	publish_commit.go commitPublish	TestCommitPublish_LeavesTheCounterAloneWhenTheTrackerPanicsPastTheRows

Not counted: an entry inside the window that the counter loses

Route	Where	Test
A panic above both increments, raised while the log line is built	publish_stream.go writeCoherenceError	TestStagePublish_LosesTheEntryWhenWriteCoherenceErrorPanics

This is the exit the section ### The subtraction against the coherence series names as the one that leaves the difference alone. The blob committed, so the window was entered. Both increments sit below the log line, so both series lose the entry together.

Registration

Constraint	Test
The series is exposed on the prefixed scrape path, and the expected-name list and npmCollectors stay the same length	TestRegisterMetrics_ExposesAllOperationalVectors

e2e scenario catalogs

No e2e scenario is added or affected. The catalogs in docs/testing/ record user-level journeys and the outcomes a real package manager gets; this MR adds one Prometheus counter and changes no status code, response body, route, or client-visible behaviour. A search of docs/testing/ for metric, _total, prometheus, observab, Grafana and dashboard finds nothing, so no scenario asserts a metric series. Driving the npm publish journeys against a local service on this branch returned the outcomes the catalog already states: 422 manifest_coherence_failed with no version created for e2e.npm.publish.reject-invalid-manifest, and the version-exists refusal with the earlier tarball intact for e2e.npm.publish.republish-same-version.

Reviewable size

The diff is past the 500 reviewable-LOC guideline, at 824 insertions and 32 deletions across 8 files. The figures come from git diff --numstat 64a7702f7...91d2e9493, between this branch's base 64a7702f70d424dd5bbcd3ed7ccdd7f6cd732599 and its head 91d2e9493bf7f201fa1bba825130a473e41b9512.

Group	Files	Insertions	Deletions
New test suite	internal/format/npm/publish_post_blob_commit_metric_internal_test.go	746	0
Production Go	internal/format/npm/metrics.go (15), publish_commit.go (14), publish_stream.go (4)	33	7
Documentation	internal/format/npm/metrics.md (39), docs/dev/storage-accounting.md (4), docs/dev/observability.md (1)	44	25
Test wiring	internal/format/npm/metrics_test.go	1	0

This is a justification rather than a split, because a split does not help here. The one new test file carries 746 of the 824 insertions, which is 90.5% of the diff. The production change is 33 insertions across three Go files. A split separates that 33-line counter from the coverage that pins which arms it counts and which arms it must not. That pairing is the substance of the review.

Merge order

One other branch conflicts with this one. An earlier revision of this section named two. !2123 was the second, it merged on 2026-09-02 as d2273d070, and this branch is now rebased onto a main that carries that commit, so its conflict is resolved and gone.

Branch prozlach/issue-939-npm-packument-bytes-no-counter still conflicts, on docs/dev/storage-accounting.md alone. It deletes the #939 row, which is the line directly above the #910 row this branch edits (lines 1429 and 1430 at this head). That branch has no merge request in any state, so nothing in GitLab reports this conflict. Its tip moved several times during this work, and the conflict holds at every tip measured, the origin tip 3c76e6226 of 2026-09-02 included. A reviewer cannot discover this conflict from the merge request page, which is why it is named here. Measured with git merge-tree --write-tree --name-only HEAD 3c76e6226 at head 7b6ce23fa on 2026-09-02. Whichever change lands second resolves it, and it is not a defect in this branch.

Two comments in packument_cache.go left uncorrected, on purpose

Both sit in internal/format/npm/packument_cache.go, and this branch changes nothing in that file. git diff origin/main HEAD -- internal/format/npm/packument_cache.go produces no output.

The storage sentence that is not namespace-qualified

Three copies of the storage sentence this branch touches now name the namespace: internal/format/npm/metrics.md:170, internal/format/npm/metrics.md:610, and docs/dev/observability.md:61. A fourth copy is not namespace-qualified. It sits at internal/format/npm/packument_cache.go:1292, and it reads "byte-match a blob already stored".

That copy is left as it is, deliberately. Correcting the line pulls its whole doc comment into the comment-cap check. That check charges 38 lines against a cap of 1 for an unexported block. Leaving it is permitted, and disclosing it is the condition on leaving it.

The gc:reconcile-scan attribution this branch corrected elsewhere

Three sites claimed that ADR-025's gc:reconcile-scan covers the post-move Session.Commit orphan, and this branch corrected all three. They are the publish-window paragraph and the packument-rebuild paragraph of internal/format/npm/metrics.md (lines 587 and 192), and the publish-window cell of docs/dev/observability.md (line 61).

A fourth copy survives, at internal/format/npm/packument_cache.go:1315-1319. It says the same shape is "storage-layer-owned and reclaimed by ADR 025's gc:reconcile-scan once written". A reviewer who greps the tree for gc:reconcile-scan finds that line, so it is disclosed here rather than left to read as an oversight.

That copy is out of scope for this merge request. Lines 1288 to 1290 of the same file record that about 70 comments across this repository defer orphans to ADR-011 reconciliation. Issue #498 is the home for a repository-wide sweep of them.

Why Related to and not Closes

The validation pass recorded Closes-eligible: no, for this reason:

this merge request meters the window between the tarball's storage commit and the npm row commit with one Prometheus counter and changes no accounting behaviour, so the under-counted namespace_statistics.deduplicated_size_bytes the issue reports stays under-counted and its GAP verdict in docs/dev/storage-accounting.md still reads GAP, and all three Closes-eligible conditions fail: the verdict is WARN, 3 of 5 items have evidence, and the acceptance was derived by this run rather than stated by the issue author.

The issue stays open, and it stays open by design.

Related to #910

This is a bot message 🤖 — /smurfit