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)

  1. 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-process RebuildPackumentCache service plus an enqueueRebuild goroutine helper (same shape as Step 24's bufferedUpdate); Step 20 inlines the cascade in the parent tx; Steps 16/17/19/21 swap EnqueueTx calls for post-commit enqueueRebuild. Net: zero npm steps gate on S27.
  2. 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-flow anchor → #packument-generation; S12 OQ-16 → S22; Step 11 HTTP-cache-headers spec-line-996 ref replaced with #api-contracts anchor; AC 14/15 unit-half / integration-half labeling per the AC 23/39/40/47 convention.
  3. docs(plans): add AC -> Step Map matrix to npm-local plan: add the Acceptance Criterion → Step Map matrix between Testing Strategy and Open Questions as the authoritative per-AC coverage contract. Mirrors the matrix @mkhalifa3 landed in !214 (merged).
  4. docs(plans): apply second-pass review on npm-local plan: reverse the prior-commit drop of blob_storage_attachments per @vespian_gl's clarification - npm_files reverts 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.
  5. docs(plans): fold in review-pass cleanup on npm-local plan: trim Step 9 acceptance bullets to match Step 24's collapsed helper; add disttags.go Modify to Step 12; widen Step 22 observability deps to cover packument GET + publish commit; fix stale step numbers in Testing Strategy after renumber.
  6. docs(plans): refresh npm-local index entry after revision: docs/plans/README.md index entry no longer claims the plan ships foundation tables inline.
  7. 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 the npm.publish_concurrency semaphore, strip residual "no OnCommit" / "no atomicity gap" / "in this revision" historical sentences.
  8. docs(plans): drop foundation Steps 1-2 and renumber to 1-25: namespaces, repository_collections, repositories, and repository_collection_repositories come from MR !237 (merged); the npm plan declares MR !237 (merged) as a hard external prereq for the schema-touching steps.
  9. 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_attachments stays per S11 spec. npm_files FKs into blob_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 touches blob_storage_blobs directly - the table is owned by S06 and reached through the BlobStore API.
  • 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 standalone pg_jsonschema + npm_versions.package_json CHECK migration). Matches the per-table-per-migration pattern @mkhalifa3 landed in !214 (merged).
  • No proto reserved fields on day-1 schema. The Step 3 "Field-reservation contract" paragraph and the Naming Conventions "Proto field reservation" subsection are dropped. When streampub_buffer_size is eventually removed, the removal MR can decide on reserved based on the deployment state at that point.
  • Single disttags.go. Step 10 ships GET, Step 21 modifies the same file to add PUT/DELETE.
  • bufferedUpdate collapsed to inline goroutine. Step 24 ships a thin helper with recover() and Prometheus metrics; no app.Component, no semaphore, no LRU rate-limit ledger, no shutdown drain. Throwaway until the cross-format buffered-write subsystem ships.
  • npm.publish_concurrency removed. Per-format CPU admission is a platform concern. A separate platform-level request-admission issue rides as a follow-up.
  • Acceptance Criterion → Step Map matrix 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 RebuildPackumentCache service plus an enqueueRebuild goroutine helper; Step 20 inlines the whole-package cascade in the parent tx. Call sites carry TODO(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-online or after URL-keyed cache staleness) and handles 304 correctly via make-fetch-happen's revalidate() 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.)
Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading