fix(maven): gate metadata erasure on the empty version set alone (S20-A Maven metadata purge plan: 4/5)

Sylvia Shenrequested to merge
worktree-1167-4 into main

Summary

Serializes Maven package-metadata erasure with concurrent version creation, and gates it on the empty live-version-set check alone rather than on whether the previously stored maven-metadata.xml parses.

Before this change, reconcileOnce parsed the stored metadata document before checking the live version set, so an unparseable, oversize, or plugins-shaped document under an emptied package was skipped instead of erased, and the erasure itself ran directly on the connection pool with no lock, racing a concurrent FindOrCreateVersion.

eraseEmptiedPackageMetadata now runs inside datastore.RunInTx: ReVerifyRepositoryAlive (FOR SHARE), then ReVerifyPackageAlive (FOR UPDATE, the lock FindOrCreateVersion also takes FOR SHARE), then the guarded SoftDeletePackageMetadataFile update on the transaction handle. Both re-verify calls now also reject a wrong-but-live repositoryID argument and a bare connection-pool handle, mirroring the guards writeRewrittenMetadata and its sibling re-verify seams already have.

Spec and plan

What changed

  • internal/format/maven/reconciler.go: reordered reconcileOnce so the live-version-set check gates the erasure arm ahead of readStoredMetadata; added eraseEmptiedPackageMetadata, which runs the erasure under the package lock and rejects a repository/package parentage mismatch. Both arms now share reverifyPackageChain for the parentage guard, the repository FOR SHARE re-check, and the package FOR UPDATE re-check, so the lock order has one home. The parentage-mismatch logger.ErrorContext that carried package_repository_id and argument_repository_id is removed from both arms: the sentinel is returned rather than logged, per the go-style handle-each-error-once rule. A mismatch therefore no longer logs those two IDs.
  • internal/datastore/maven_files.go: SoftDeletePackageMetadataFile now takes the caller's transaction handle and rejects a bare connection-pool handle at runtime, matching ReVerifyPackageAlive/ReVerifyVersionAlive.
  • Tests: two new integration test files covering the erasure's lock composition (concurrent publish ordering, rollback, dead-repository/dead-package aborts) and the HTTP-level metadata/sidecar reads before and after erasure; new unit and integration coverage for the parentage guard, the connection-pool guard, and the erasure arm's stale-retry path through Reconciler.Reconcile.
  • docs/testing/e2e/maven.md: adds the e2e.maven.lifecycle.delete-last-version scenario and describes the erasure arm alongside the existing rewrite-arm description.
  • Documentation: docs/user/** is intentionally out of this MR. The product documentation for a plan is authored by the end-of-plan docs-author pass, which runs once per plan after its steps merge (docs/dev/agentic-development.md); Step 4's file list owns only docs/testing/e2e/maven.md.

Size

1,111 reviewable LOC, over the 500 LOC ceiling. Not split, because the pieces are interdependent: the datastore signature change, the reconciler reordering, and their tests prove one correctness property together (metadata erasure is safe under a concurrent publish), and splitting them would let one land without the lock discipline the other half proves.

By file group:

  • Production code: 166 LOC (wire_maven.go, maven_files.go, reconciler.go)
  • Tests: 935 LOC (two new integration test files, plus unit/integration coverage for the parentage guard, the pool-rejection guard, and the stale-retry path)
  • Docs: 10 LOC (docs/testing/e2e/maven.md)

Test plan

  • go test ./internal/datastore/... ./internal/format/maven/... (plain unit tests)
  • go test -tags=integration ./internal/datastore/... ./internal/format/maven/... (integration suite, real Postgres)
  • golangci-lint run and the --build-tags=integration variant on both packages
  • scripts/ci/check-comment-caps.sh --base main

Related to #1167 (closed)

Edited by Sylvia Shen

Merge request reports