Skip to main content
← Back to list
01Issue
FeatureClosedSwamp CLI
AssigneesNone

#255 Plan v4 step 9 literal test untestable under current catalog PK semantics

Opened by stack72 · 5/5/2026

Context

W2 (swamp-club#231 / PR systeminit/swamp#1310) plan v4 step 9 specifies:

Order-independence regression test for the upgrade pattern. Assert `saveAll([v2, tombstoneAll(v1)])` succeeds — not just the canonical `saveAll([tombstoneAll(v1), v2])`. I-Repo-1 evaluates only on post-save state, so order should not matter.

This is structurally untestable for the literal within-extension form under the current catalog schema. Reason:

The catalog primary key is `source_path`. v1 and v2 of the SAME extension at the same relative path share the SAME row. Submitting `[v2, tombstoneAll(v1)]` to `saveAll` runs:

  1. `applyDiffForExtension(v2)` → UPSERT v2 onto the row at ``.
  2. `applyDiffForExtension(tombstoneAll(v1))` → DELETE BY `` because the v1 source is now Tombstoned and `removeBySourcePath` is called.

Result: the row v2 just wrote gets DELETEd. The aggregate disappears. Not because order-independence is broken at the I-Repo-1 layer, but because the diff-save's per-source-path semantics make "reorder of operations on the same path" a no-op concept.

What W2 shipped instead

The bulk-upgrade order-independence test (`extension_repository_test.ts:295`) covers two distinct extensions with distinct rows. It exercises the meaningful generalization of the order-independence claim — the one that catches real regressions where saveAll iterates in a way that lets one extension's mid-loop state leak into another's diff. The architecture-agent review accepted this as "strictly subsumes" the within-extension form.

Inline comment in the test documents this so a future reader doesn't try to re-add the literal test.

Decision needed

This issue tracks the open question for a future architecture pass:

Should the catalog PK be `(source_path, extension_version)` rather than just `source_path`?

If yes:

  • The literal step 9 form becomes testable.
  • The atomic-upgrade pattern's semantics become more transparent (v1 and v2 are distinct rows that coexist briefly during `saveAll` and v1's tombstone DELETEs its own row, leaving v2's untouched).
  • Migration: every existing row needs `extension_version` populated before the unique constraint flips. W1b's migration already populates this column, so the path is clear.

If no:

  • The current PK is fine; document the within-extension order-independence is a no-op concept.
  • W2's bulk-upgrade test is the correct generalization.

Why not blocking W2

The architecture-agent labeled the literal test "Optional" in the W2 review. The contract that matters (I-Repo-1 evaluates the post-save state, not order-of-args) is verified by the bulk-upgrade test. This issue is for the architectural question, not the verification gap.

Recommendation

Defer the PK change until W3 / W4 catalog rework lands. At that point this question can be decided alongside the unified-loader (KindAdapter) work.

02Bog Flow
OPENTRIAGEDIN PROGRESSCLOSED

Closed

5/5/2026, 7:06:02 PM

No activity in this phase yet.

03Sludge Pulse

Sign in to post a ripple.