feat(npm): version publish writes (S11 Step 5)

📦 What

Step 5 of the npm local plan, sliced into four stacked ~500-LOC MRs. Slice 3/4 — the version publish-side write methods.

Store file	Write methods
npm_versions.go	InsertNpmVersion (ON CONFLICT DO NOTHING RETURNING; duplicate → ErrNpmVersionExists), UpdateNpmVersionPackageJSON

Both validate package_json via schemas.ValidateNpmPackageJSON (slice 1/4) before the write, so a denylisted document is rejected before any row is touched.

✅ Testing

internal/datastore/npm_write_integration_test.go (integration, testcontainers PostgreSQL): concurrent InsertNpmVersion (parallel goroutines → exactly one success + one conflict via the partial unique index), duplicate-version conflict, soft-delete re-insert, UpdateNpmVersionPackageJSON not-found / soft-deleted (active-only filter), and a guard that a denylisted package.json is rejected before any write. Adds internal/datastore/npm_versions_test.go (unit): jsonbStringExpr panics on empty input, and argument-guard tables for InsertNpmVersion (nil ctx / db, zero namespace) and UpdateNpmVersionPackageJSON (same, plus non-positive version id).

Size: ~716 LOC, ~64% test.

🔍 Reviewer note

AppSec flagged that the jsonbStringExpr helper was not self-defending - it relied on ValidateNpmPackageJSON running first at both call sites. Addressed in this MR: the helper now panics on empty input (an empty ::jsonb cast is invalid SQL, and package_json is NOT NULL so empty is never a valid value), enforcing the precondition itself rather than per-caller. A unit test covers the panic. Duo's two nits (ambiguous version=%d label → version_id=%d; missing NpmVersionID <= 0 guard) are also addressed.

🔗 Stack

Step 5 was split into four stacked MRs (each targets the previous; GitLab auto-retargets to main as they merge):

1. !499 (merged) — package_json schema validator — merged
2. !503 (merged) — package & file writes + shared test harness — merged
3. !504 (merged) — version writes
4. !505 (merged) — dist-tag writes

The metadata-cache writes that originally shared slice 4 were carved out: the Step 4b reorg removed the npm_metadata_files read layer they build on (deferred to a later step), so they follow it there.

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method	Plan node	Index	Rows (plan / actual)	Cost	Time	Buffers (hit / read)	Partitions
npm_versions.InsertNpmVersion	Insert	unique_npm_versions_ns_id_pkg_id_version (conflict arbiter)	1 / 1	0.01	1.137ms	98 / 5	1
npm_versions.UpdateNpmVersionPackageJSON	Update (Seq Scan)	n/a (Seq Scan at seed scale; see notes)	1 / 1	1.75	1.019ms	20 / 2	1

npm_versions.InsertNpmVersion

Summary: Plan matches intent: a single-row Insert with conflict resolution NOTHING arbitrated by the partial unique index unique_npm_versions_ns_id_pkg_id_version, which is exactly the (namespace_id, npm_package_id, version) WHERE soft_deleted_at IS NULL index the method's ON CONFLICT ... WHERE soft_deleted_at IS NULL targets. One tuple inserted, zero conflicting, routed to a single partition. The two FK-validation triggers (namespace, package) fire as expected for a new row. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1

Rendered SQL:

INSERT INTO public.npm_versions (namespace_id, npm_package_id, version, package_json)
VALUES ($1::uuid, $2, $3::text, $4::text::jsonb)
ON CONFLICT (namespace_id, npm_package_id, version) WHERE soft_deleted_at IS NULL DO NOTHING
RETURNING npm_versions.id AS "npm_versions.id";

Bound args: [<seeded namespaces.id>, <seeded npm_packages.id>, '1.0.0', '{"name":"review-prep-pkg","version":"1.0.0"}'::jsonb]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Insert on npm_versions  (cost=0.00..0.01 rows=1 width=216) (actual time=0.233..0.233 rows=1 loops=1)
   Conflict Resolution: NOTHING
   Conflict Arbiter Indexes: unique_npm_versions_ns_id_pkg_id_version
   Tuples Inserted: 1
   Conflicting Tuples: 0
   Buffers: shared hit=98 read=5 dirtied=11 written=6
   ->  Result  (cost=0.00..0.01 rows=1 width=216) (actual time=0.021..0.021 rows=1 loops=1)
         Buffers: shared hit=12
 Planning:
   Buffers: shared hit=126 read=1
 Planning Time: 0.264 ms
 Trigger for constraint fk_npm_versions_namespace_id_namespaces on npm_versions_p13: time=0.155 calls=1
 Trigger for constraint fk_npm_versions_npm_package_id_npm_packages on npm_versions_p13: time=0.476 calls=1
 Execution Time: 1.137 ms

Timings: planning 0.264ms, execution 1.137ms, total 1.401ms.

npm_versions.UpdateNpmVersionPackageJSON

Summary: Plan matches intent: a single-row Update on the partition holding the target, filtered by (namespace_id, id) AND soft_deleted_at IS NULL, touching exactly one row out of the 50 seeded (49 removed by filter). The planner chose a Seq Scan because the single partition holds only 50 rows at seed scale; the predicate is fully index-supportable — forcing enable_seqscan=off switches it to an Index Scan using npm_versions_pXX_pkey (the (id, namespace_id) primary key) with Index Cond: ((id = ...) AND (namespace_id = ...)), so production cardinality picks the PK index automatically. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1, npm_versions=50

Rendered SQL:

UPDATE public.npm_versions
SET package_json = $1::text::jsonb
WHERE ((npm_versions.namespace_id = $2::uuid) AND (npm_versions.id = $3)) AND (npm_versions.soft_deleted_at IS NULL);

Bound args: ['{"name":"review-prep-pkg","version":"1.0.0","deprecated":"use 2.x"}'::jsonb, <seeded namespaces.id>, <target npm_versions.id>]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Update on npm_versions  (cost=0.00..1.75 rows=0 width=0) (actual time=0.071..0.071 rows=0 loops=1)
   Update on npm_versions_p23 npm_versions_1
   Buffers: shared hit=20 read=2
   ->  Seq Scan on npm_versions_p23 npm_versions_1  (cost=0.00..1.75 rows=1 width=42) (actual time=0.004..0.004 rows=1 loops=1)
         Filter: ((soft_deleted_at IS NULL) AND (namespace_id = '63f7efca-2198-458e-a1c2-59fcae69cb18'::uuid) AND (id = '51'::bigint))
         Rows Removed by Filter: 49
         Buffers: shared hit=1
 Planning:
   Buffers: shared hit=210 read=1
 Planning Time: 0.494 ms
 Trigger for constraint fk_npm_versions_namespace_id_namespaces on npm_versions_p23: time=0.015 calls=1
 Trigger for constraint fk_npm_versions_npm_package_id_npm_packages on npm_versions_p23: time=0.840 calls=1
 Execution Time: 1.019 ms

Timings: planning 0.494ms, execution 1.019ms, total 1.513ms.

Query notes:

• No anomalies. InsertNpmVersion resolves its ON CONFLICT arbiter to the partial unique index unique_npm_versions_ns_id_pkg_id_version (the race-free version guard), single-partition on namespace_id (ADR 022). UpdateNpmVersionPackageJSON plans a Seq Scan only because the write-target table is seeded to 50 rows per the skill's methodology; the (namespace_id, id) predicate is index-supported and the planner switches to the (id, namespace_id) primary key at production cardinality (verified with enable_seqscan=off).

Related to #122 (closed)