docs(plans): add npm local implementation plan
Summary
Revise the npm-local plan in response to MR !186 (merged) review. The change is
dominated by stripping plan-meta narrative, dropping out-of-territory
content (S04 foundation tables, S08 auth internals, S09 permission
helpers, platform-level CPU and buffered-update concerns), and
reshaping the schema-touching steps to consume MR !237 (merged) (foundation
schema migrations) and work item #92 (closed) (blob_storage_attachments
foundation).
LOC dropped from 2,580 to 1,717 (-33%; after the AC → Step Map matrix and the S27 → goroutines refactor added in the latest revisions). Step count dropped from 27 to 25.
Commits in this revision (top of branch first)
docs(plans): switch npm async work to goroutines (drop S27 gate): per @jdrpereira's review, apply the dev model's "async deferral use goroutines until the background jobs foundation lands" rule and the !214 (merged) (S10 Maven) precedent. Step 12 flips from "jobsriver job kind" to in-processRebuildPackumentCacheservice plus anenqueueRebuildgoroutine helper (same shape as Step 24'sbufferedUpdate); Step 20 inlines the cascade in the parent tx; Steps 16/17/19/21 swapEnqueueTxcalls for post-commitenqueueRebuild. Net: zero npm steps gate on S27.docs(plans): apply @jdrpereira's review on npm-local plan: spec-step labels in Approach and Step 14/15/16 match the spec's own publish-flow numbering; S08 plan reference points to the merged path;#install-flowanchor →#packument-generation; S12 OQ-16 → S22; Step 11 HTTP-cache-headers spec-line-996 ref replaced with#api-contractsanchor; AC 14/15 unit-half / integration-half labeling per the AC 23/39/40/47 convention.docs(plans): add AC -> Step Map matrix to npm-local plan: add theAcceptance Criterion → Step Mapmatrix between Testing Strategy and Open Questions as the authoritative per-AC coverage contract. Mirrors the matrix @mkhalifa3 landed in !214 (merged).docs(plans): apply second-pass review on npm-local plan: reverse the prior-commit drop ofblob_storage_attachmentsper @vespian_gl's clarification -npm_filesreverts to the spec's composite(blob_storage_attachment_id, blob_sha256)→blob_storage_attachments(id, sha256)FK, with work item #92 (closed) added as a hard external prereq; split Steps 1 and 2 into per-table migration files (matches the Maven precedent set in !214 (merged)); drop the Step 3 "Field-reservation contract" paragraph and the Naming Conventions "Proto field reservation" subsection since the app isn't deployed yet.docs(plans): fold in review-pass cleanup on npm-local plan: trim Step 9 acceptance bullets to match Step 24's collapsed helper; adddisttags.goModify to Step 12; widen Step 22 observability deps to cover packument GET + publish commit; fix stale step numbers in Testing Strategy after renumber.docs(plans): refresh npm-local index entry after revision:docs/plans/README.mdindex entry no longer claims the plan ships foundation tables inline.docs(plans): apply remaining step-level review feedback: drop permission helpers and principal-on-context (Step 8), narrow Step 9's S06 prereq to the interface only, collapse Step 24 (bufferedUpdate) to a thin goroutine, consolidate dist-tags filenames, drop thenpm.publish_concurrencysemaphore, strip residual "no OnCommit" / "no atomicity gap" / "in this revision" historical sentences.docs(plans): drop foundation Steps 1-2 and renumber to 1-25:namespaces,repository_collections,repositories, andrepository_collection_repositoriescome from MR !237 (merged); the npm plan declares MR !237 (merged) as a hard external prereq for the schema-touching steps.docs(plans): strip plan-meta narrative from npm-local plan: delete the Research-Findings / Context / Approach prose that paraphrased the spec or restated ADR/foundation-plan state.
Operator decisions captured in this revision
blob_storage_attachmentsstays per S11 spec.npm_filesFKs intoblob_storage_attachments(id, sha256)via the composite(blob_storage_attachment_id, blob_sha256). Work item #92 (closed) (hswimelar) is the cross-format foundation MR and is now a hard external prereq for Step 2. npm code never touchesblob_storage_blobsdirectly - the table is owned by S06 and reached through theBlobStoreAPI.- Per-table migration files in Steps 1 and 2. Step 1 ships three
migrations (
npm_repositories,npm_packages,npm_versions), Step 2 ships four (npm_files,npm_tags,npm_metadata_files, plus a standalonepg_jsonschema+npm_versions.package_jsonCHECK migration). Matches the per-table-per-migration pattern @mkhalifa3 landed in !214 (merged). - No proto
reservedfields on day-1 schema. The Step 3 "Field-reservation contract" paragraph and the Naming Conventions "Proto field reservation" subsection are dropped. Whenstreampub_buffer_sizeis eventually removed, the removal MR can decide onreservedbased on the deployment state at that point. - Single
disttags.go. Step 10 ships GET, Step 21 modifies the same file to add PUT/DELETE. bufferedUpdatecollapsed to inline goroutine. Step 24 ships a thin helper withrecover()and Prometheus metrics; noapp.Component, no semaphore, no LRU rate-limit ledger, no shutdown drain. Throwaway until the cross-format buffered-write subsystem ships.npm.publish_concurrencyremoved. Per-format CPU admission is a platform concern. A separate platform-level request-admission issue rides as a follow-up.Acceptance Criterion → Step Mapmatrix added. All 65 spec ACs are now traceable to the covering step(s) in a single authoritative table between Testing Strategy and Open Questions. Mirrors the contract @mkhalifa3 landed in !214 (merged).- Async work runs in-process until S27 lands. Per the dev
model's "async deferral use goroutines until the background
jobs foundation lands" rule and the !214 (merged) (S10 Maven) precedent,
zero npm steps gate on S27. Step 12 ships an in-process
RebuildPackumentCacheservice plus anenqueueRebuildgoroutine helper; Step 20 inlines the whole-package cascade in the parent tx. Call sites carryTODO(s27-cache-rebuild)so the future S27 swap is grep-discoverable.
304 client-compatibility investigation (Step 9)
The conditional GET path on tarballs (If-None-Match /
If-Modified-Since → 304) was robot-investigated against npm CLI,
Yarn v1, Yarn berry, and pnpm. Findings:
- npm CLI sends conditional headers (only when
--prefer-onlineor after URL-keyed cache staleness) and handles 304 correctly viamake-fetch-happen'srevalidate()path. - Yarn v1, Yarn berry, pnpm never send conditional headers on
tarball GETs (they reserve that for packument metadata). pnpm
treats any non-200 as a fatal
FetchError, so it would break on 304 - but since it never sends the conditional headers in the first place, the server never returns 304 to pnpm. - Decision: Step 9's conditional-GET implementation is safe to keep (no client breaks). The npm CLI is the only client that exercises it, and it does so correctly. Findings posted to the thread on the corresponding review note.
Self-review
- Branch rebased onto current
main; all commits pass pre-commit hooks (markdownlint, vale, lychee). - Every cross-plan step reference (S06 Step N, S27 Step N) re-verified against the source-of-truth plan files.
- Mermaid DAG renumbered; all edges connect existing nodes.
- Status table rewritten with new numbering (1-25).
- Open Questions section is empty.
- Reply to each of the file-anchored review threads with the action taken; resolve. (In progress in a follow-up.)