From ee4ea413ef0171f8880e6cac7d52f291e4e06287 Mon Sep 17 00:00:00 2001 From: Brian Majewski Date: Mon, 8 Jun 2026 09:58:23 -0700 Subject: [PATCH] =?UTF-8?q?docs:=20additive-v2=20implementation=20plan=20+?= =?UTF-8?q?=20spec=20=C2=A73=20fix=20(no=20marker=20auto-stamp)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-06-08-additive-v2-dual-write.md | 587 ++++++++++++++++++ ...026-06-08-additive-v2-dual-write-design.md | 31 +- 2 files changed, 609 insertions(+), 9 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-08-additive-v2-dual-write.md diff --git a/docs/superpowers/plans/2026-06-08-additive-v2-dual-write.md b/docs/superpowers/plans/2026-06-08-additive-v2-dual-write.md new file mode 100644 index 0000000..4b21c0c --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-additive-v2-dual-write.md @@ -0,0 +1,587 @@ +# Additive-v2 Dual-Write Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make store-format v2 *additive* — upgraded `secrets` clients read either blob suffix and dual-write existing `properties` externals — so the destructive `migrate --finalize` becomes optional GC and the cross-machine coordination gate disappears. + +**Architecture:** Replace the marker-driven single-suffix helper `_external_blob_suffix(type)` with two helpers: a read-resolver that tries `.properties.age` then falls back to `.gradle-properties.age`, and a write-targets helper that writes the v2 suffix always plus the v1 suffix *only when a v1 twin already exists* (dual-write existing externals; brand-new externals are v2-only — the intended forcing function). Reads and writes no longer depend on the `.secrets-format` marker, which keeps its meaning (born-v2 / finalized). Because fresh pushes now write the v2 suffix on any store, the migrate/finalize test fixtures (which relied on push producing a v1 blob) are updated to fabricate an old-client v1 blob. + +**Tech Stack:** Single bash 3.2 script (`secrets`); `age`, `git`, `jq`. Tests: `bats-core` (`test/migrate.bats`, `test/manifest.bats`). Every standalone `[[ ]]` test assertion ends with `|| false` (bash 3.2 ERR-trap gotcha). + +**Spec:** `docs/superpowers/specs/2026-06-08-additive-v2-dual-write-design.md`. **Ticket:** EGB-712. + +--- + +## Background facts (verified against branch `brian/egb-712-...`, post-EGB-710) + +- `_external_blob_suffix()` — `secrets:548-555`. Returns `properties` for `gradle-properties` on a v2 store (`_store_format == 2`), else the type verbatim; `file` always returns `file`. Blob path = `external/..age`. +- Callers of `_external_blob_suffix`: push file write `secrets:655`, push properties write `secrets:684`, pull read `secrets:713`, verify read `secrets:2056`. All four are replaced; then the helper is deleted. +- `_secrets_files_slug(path)` (`secrets:512`) derives the machine-independent slug. dotenv and `file` blobs are byte-identical in v1/v2 (only the `properties` suffix diverges). +- `_store_format()` (`secrets:529`) and the marker stay as-is — set only by `init` (born-v2) and `migrate --finalize`. Push must NOT stamp it (see spec §3). +- Existing test that codifies OLD write behavior: `test/migrate.bats:52` "push on a v1 store still writes .gradle-properties.age (back-compat)" — rewritten in Task 2. +- Tests that fabricate-or-rely-on a v1 properties blob from `make_v1_store; push` and break once push writes v2-only (repaired in Task 3): the migrate copy-forward/idempotent tests, the `--dry-run` rename test, the dotenv+file untouched test, all four finalize tests, the EGB-710 manifest-free / undeclared-twin tests, and the two `--status` tests that need an actual v1 blob. (dotenv-only and already-v2 tests are unaffected.) +- Baseline before this plan: `bats test/` = 242 passing. + +## File structure + +- Modify: `secrets` — delete `_external_blob_suffix` (548-555); add `_resolve_external_blob_read` + `_external_blob_write_targets` in its place; rewire push (651-688), pull (712-713), verify (2052-2068); extend `_migrate_status`; docs in `cmd_help`. +- Modify: `test/migrate.bats` — add `m_fake_v1_blob` helper; add read-fallback + write-rule + status-coverage tests; repair the v1-blob-dependent fixtures. +- Modify: `CLAUDE.md`, `README.md`, `VERSION` (→ `0.7.0.0`), `CHANGELOG.md`. + +--- + +## Task 1: Read-resolver — reads try both suffixes + +**Files:** `secrets` (replace `_external_blob_suffix` with the read-resolver; rewire pull + verify reads), `test/migrate.bats`. + +- [ ] **Step 1: Write the failing test** — a v2 store whose properties blob exists ONLY in the v1 suffix must still pull. + +Add to `test/migrate.bats` (after the format-marker tests, ~line 60): + +```bash +@test "pull reads a v1-suffix properties blob on a v2 store (read-fallback)" { + init_with_remote # born-v2 store (marker=2) + m_gradle_src $'beaconClerkPkTest=pk_test_v1\n' + create_project_dir rffallback + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push rffallback >/dev/null 2>&1 # writes .properties.age on a v2 store + # Simulate an external that exists only in the v1 suffix (an old client wrote it): + local v2blob; v2blob=$(ls "$SECRETS_DIR/rffallback/external/"*.properties.age) + mv "$v2blob" "${v2blob%.properties.age}.gradle-properties.age" + rm -f "$HOME/.gradle/gradle.properties" + "$SECRETS_BIN" pull rffallback >/dev/null 2>&1 + run grep -q 'beaconClerkPkTest=pk_test_v1' "$HOME/.gradle/gradle.properties" + [ "$status" -eq 0 ] +} +``` + +- [ ] **Step 2: Run it, confirm it fails** + +Run: `bats test/migrate.bats -f "read-fallback"` +Expected: FAIL — old pull (`secrets:713`) uses `_external_blob_suffix gradle-properties` = `properties` on a v2 store, looks only for `.properties.age` (which we renamed away), warns "no encrypted data", restores nothing → the grep fails. + +- [ ] **Step 3: Replace `_external_blob_suffix` with the read-resolver.** Replace `secrets:542-555` (the comment block + `_external_blob_suffix()` through its closing `}`) with: + +```bash +# Resolve the on-disk path of an external blob for READING. Tries the v2 suffix +# (.properties.age) first, then falls back to the v1 (.gradle-properties.age) for +# `properties` externals, so an upgraded client finds the blob whichever format +# wrote it (additive v2 — EGB-712). `file` externals share one suffix in both +# formats. Echoes the path of the blob that exists; if neither exists, echoes the +# canonical v2 path so the caller's "no blob" message reads sensibly. Read-only. +_resolve_external_blob_read() { + local project="$1" slug="$2" mtype="$3" + local base="$SECRETS_DIR/$project/external/$slug" + case "$mtype" in + file) + echo "$base.file.age" ;; + properties|gradle-properties) + if [ -f "$base.properties.age" ]; then + echo "$base.properties.age" + elif [ -f "$base.gradle-properties.age" ]; then + echo "$base.gradle-properties.age" + else + echo "$base.properties.age" + fi ;; + *) + echo "$base.$mtype.age" ;; + esac +} +``` + +(The write-targets helper is added in Task 2 — leave a gap; do not reintroduce `_external_blob_suffix`.) + +- [ ] **Step 4: Rewire the pull read.** At `secrets:712-713`, replace: + +```bash + local slug; slug=$(_secrets_files_slug "$mpath") + local blob="$SECRETS_DIR/$project/external/$slug.$(_external_blob_suffix "$mtype").age" +``` + +with: + +```bash + local slug; slug=$(_secrets_files_slug "$mpath") + local blob; blob=$(_resolve_external_blob_read "$project" "$slug" "$mtype") +``` + +- [ ] **Step 5: Rewire the verify read.** At `secrets:2055-2065`, replace: + +```bash + slug=$(_secrets_files_slug "$epath") + erel="external/$slug.$(_external_blob_suffix "$etype").age" + # Account for BOTH the v1 and v2 suffix forms in the orphan set. During the + # migration window (after copy-forward, before --finalize) the v2 twin + # coexists with the v1 blob; neither should read as an orphan whichever + # format the store currently reports. (file's two forms are identical.) + expected="${expected}external/$slug.$etype.age"$'\n' + [ "$etype" = "gradle-properties" ] && expected="${expected}external/$slug.properties.age"$'\n' + eblob="$pdir/$erel" + if [ ! -f "$eblob" ]; then + echo "FINDING: external '$epath' ($etype) is declared but has no blob in the store ($project/$erel missing). Run 'secrets push'." >&2 +``` + +with: + +```bash + slug=$(_secrets_files_slug "$epath") + # Account for BOTH suffix forms in the orphan set — a dual-written `properties` + # external (additive v2 — EGB-712) legitimately has both blobs on disk; neither + # is an orphan. (file's two forms are identical.) + expected="${expected}external/$slug.$etype.age"$'\n' + [ "$etype" = "gradle-properties" ] && expected="${expected}external/$slug.properties.age"$'\n' + eblob=$(_resolve_external_blob_read "$project" "$slug" "$etype") + erel="${eblob#"$pdir"/}" + if [ ! -f "$eblob" ]; then + echo "FINDING: external '$epath' ($etype) is declared but has no blob in the store ($project/$erel missing). Run 'secrets push'." >&2 +``` + +(Note: `_external_blob_suffix` still has two remaining callers in push — push isn't rewired until Task 2, so the script still parses and runs. Those calls keep working because the function is only deleted in Task 2 Step 6, after push is rewired.) + +**IMPORTANT:** do NOT delete `_external_blob_suffix` yet — push (`secrets:655`, `secrets:684`) still calls it until Task 2. Deleting it now breaks push. + +- [ ] **Step 6: Run the read-fallback test + full migrate suite** + +Run: `bats test/migrate.bats` +Expected: the new "read-fallback" test PASSES; all other migrate tests still PASS (push unchanged; pull/verify now use the resolver, which is equivalent to the old behavior whenever the suffix matches the store format). + +- [ ] **Step 7: Commit** + +```bash +git add secrets test/migrate.bats +git commit -m "feat: read-resolver tries both external suffixes (additive v2, EGB-712)" +``` + +--- + +## Task 2: Write-targets — twin rule (dual-write existing, v2-only for new) + +**Files:** `secrets` (add `_external_blob_write_targets`; rewire push file + properties writes; delete `_external_blob_suffix`), `test/migrate.bats`. + +- [ ] **Step 1: Write the failing tests.** Add to `test/migrate.bats`: + +```bash +@test "push writes the v2 suffix for a fresh external even on a v1 store" { + make_v1_store + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + create_project_dir freshv1 + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push freshv1 >/dev/null 2>&1 + run bash -c "ls $SECRETS_DIR/freshv1/external/*.properties.age" + [ "$status" -eq 0 ] # v2 suffix regardless of marker + run bash -c "ls $SECRETS_DIR/freshv1/external/*.gradle-properties.age 2>/dev/null" + [ "$status" -ne 0 ] # no v1 twin for a brand-new external +} + +@test "push dual-writes the v1 twin so old clients stay fresh" { + make_v1_store + m_gradle_src $'beaconClerkPkTest=pk_test_old\n' + create_project_dir dualwrite + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push dualwrite >/dev/null 2>&1 # v2-only (fresh) + m_fake_v1_twin dualwrite # simulate a pre-existing v1 twin + m_gradle_src $'beaconClerkPkTest=pk_test_new\n' + "$SECRETS_BIN" push dualwrite >/dev/null 2>&1 # twin exists -> dual-write both + # Prove the v1 twin was refreshed: drop the v2 blob, pull, expect the NEW value. + rm -f "$SECRETS_DIR/dualwrite/external/"*.properties.age + rm -f "$HOME/.gradle/gradle.properties" + "$SECRETS_BIN" pull dualwrite >/dev/null 2>&1 + run grep -q 'beaconClerkPkTest=pk_test_new' "$HOME/.gradle/gradle.properties" + [ "$status" -eq 0 ] +} +``` + +- [ ] **Step 2: Add the `m_fake_v1_twin` test helper.** In `test/migrate.bats`, next to `make_v1_store` (~line 12), add: + +```bash +# Simulate an old (v1) client's properties blob: copy the pushed v2 +# .properties.age to its v1 .gradle-properties.age twin. (Current clients never +# write the v1 suffix for a fresh external, so tests fabricate it.) Use `cp` to +# KEEP the v2 blob (dual present); see m_make_v1_only to leave only the v1 blob. +m_fake_v1_twin() { + local proj="$1" v2 + v2=$(ls "$SECRETS_DIR/$proj/external/"*.properties.age) + cp "$v2" "${v2%.properties.age}.gradle-properties.age" +} +# Like m_fake_v1_twin but renames (leaves ONLY the v1 blob) — for old-client-only +# / copy-forward fixtures. +m_make_v1_only() { + local proj="$1" v2 + v2=$(ls "$SECRETS_DIR/$proj/external/"*.properties.age) + mv "$v2" "${v2%.properties.age}.gradle-properties.age" +} +``` + +- [ ] **Step 3: Run the new tests, confirm they fail** + +Run: `bats test/migrate.bats -f "fresh external even on a v1 store"` +Expected: FAIL — on a v1 store, the old push (`_external_blob_suffix gradle-properties` = `gradle-properties`) writes `.gradle-properties.age`, so the `*.properties.age` assertion fails. +Run: `bats test/migrate.bats -f "dual-writes the v1 twin"` +Expected: FAIL — old push writes a single suffix; the second push won't refresh the v1 twin. + +- [ ] **Step 4: Add the write-targets helper.** Immediately after `_resolve_external_blob_read` (added in Task 1), insert: + +```bash +# The on-disk path(s) to WRITE for an external blob, one per line. For a +# `properties` external this is the v2 suffix (.properties.age) ALWAYS, plus the +# v1 suffix (.gradle-properties.age) WHEN a v1 twin already exists in the store +# (dual-write keeps old clients fresh; a brand-new external is v2-only — the +# intended forcing function, additive v2 / EGB-712). `file` externals have a +# single suffix in both formats. Independent of the store marker. +_external_blob_write_targets() { + local project="$1" slug="$2" mtype="$3" + local base="$SECRETS_DIR/$project/external/$slug" + case "$mtype" in + file) + echo "$base.file.age" ;; + properties|gradle-properties) + echo "$base.properties.age" + [ -f "$base.gradle-properties.age" ] && echo "$base.gradle-properties.age" ;; + *) + echo "$base.$mtype.age" ;; + esac +} +``` + +- [ ] **Step 5: Rewire the push writes.** At `secrets:651-658` (the `file` branch), replace: + +```bash + if [ "$mtype" = "file" ]; then + # EGB-652: whole-file sync — encrypt the file verbatim (binary-safe). + mkdir -p "$SECRETS_DIR/$project/external" + local fslug; fslug=$(_secrets_files_slug "$mpath") + age -r "$pubkey" -o "$SECRETS_DIR/$project/external/$fslug.$(_external_blob_suffix file).age" "$expanded" + info "Encrypted file $mpath" + pushed=$((pushed + 1)) + continue + fi +``` + +with: + +```bash + if [ "$mtype" = "file" ]; then + # EGB-652: whole-file sync — encrypt the file verbatim (binary-safe). + mkdir -p "$SECRETS_DIR/$project/external" + local fslug; fslug=$(_secrets_files_slug "$mpath") + local wt + while IFS= read -r wt; do + [ -n "$wt" ] || continue + age -r "$pubkey" -o "$wt" "$expanded" + done < <(_external_blob_write_targets "$project" "$fslug" file) + info "Encrypted file $mpath" + pushed=$((pushed + 1)) + continue + fi +``` + +Then at `secrets:682-684` (the properties branch), replace: + +```bash + mkdir -p "$SECRETS_DIR/$project/external" + local slug; slug=$(_secrets_files_slug "$mpath") + age -r "$pubkey" -o "$SECRETS_DIR/$project/external/$slug.$(_external_blob_suffix "$mtype").age" "$tmp" +``` + +with: + +```bash + mkdir -p "$SECRETS_DIR/$project/external" + local slug; slug=$(_secrets_files_slug "$mpath") + local wt + while IFS= read -r wt; do + [ -n "$wt" ] || continue + age -r "$pubkey" -o "$wt" "$tmp" + done < <(_external_blob_write_targets "$project" "$slug" "$mtype") +``` + +- [ ] **Step 6: Delete the now-unused `_external_blob_suffix`.** Confirm zero remaining callers first: + +Run: `grep -n "_external_blob_suffix" secrets` +Expected: no matches (all four call sites rewired). If any remain, rewire them before deleting. Then delete the `_resolve_external_blob_read`-replaced... — it's already gone (replaced in Task 1). Verify the function is absent: `grep -c "_external_blob_suffix()" secrets` → `0`. + +- [ ] **Step 7: Run the two new write tests** + +Run: `bats test/migrate.bats -f "fresh external even on a v1 store"` then `-f "dual-writes the v1 twin"` +Expected: both PASS. + +- [ ] **Step 8: Rewrite the obsolete back-compat test.** Replace the `@test "push on a v1 store still writes .gradle-properties.age (back-compat)"` block (`test/migrate.bats:52-60`) with: + +```bash +@test "push on a v1 store writes the v2 suffix for a fresh external (additive v2)" { + make_v1_store + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + create_project_dir v1push + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push v1push >/dev/null 2>&1 + run bash -c "ls $SECRETS_DIR/v1push/external/*.properties.age" + [ "$status" -eq 0 ] +} +``` + +- [ ] **Step 9: Run the full migrate suite — expect the v1-blob-dependent fixtures to FAIL.** This is expected; Task 3 repairs them. + +Run: `bats test/migrate.bats` +Expected: the read-fallback + two write tests + rewritten back-compat test PASS; several copy-forward/finalize/status tests now FAIL (push no longer writes a `.gradle-properties.age` for them to migrate). Note which fail — Task 3 fixes exactly those. + +- [ ] **Step 10: Commit** (suite intentionally not yet fully green — Task 3 follows immediately) + +```bash +git add secrets test/migrate.bats +git commit -m "feat: twin-rule write targets — dual-write existing, v2-only for new (additive v2, EGB-712)" +``` + +--- + +## Task 3: Repair migrate/finalize/status fixtures + +**Files:** `test/migrate.bats`. No production code changes — this re-greens the suite by fabricating the old-client v1 blobs that push no longer writes. + +The rule for each repair: after the `"$SECRETS_BIN" push ` line, insert a fabrication call: +- Use **`m_make_v1_only `** (rename → only the v1 blob exists) for tests asserting a `*.gradle-properties.age` blob exists / is copy-forwarded (mirrors the pre-EGB-712 state where push produced a v1 blob). +- Use **`m_fake_v1_twin `** (keep both) only where a test needs both suffixes present. + +- [ ] **Step 1: Repair the copy-forward / dry-run / idempotent tests.** In each of these tests, insert `m_make_v1_only ` immediately after the `push ` line: + - `"migrate --dry-run reports the rename and writes nothing"` (proj `dryproj`) + - `"migrate copy-forward creates the v2 twin and keeps the v1 blob (byte-identical)"` (proj `cfproj`) + - `"migrate copy-forward is idempotent"` (proj `idemproj`) + - `"migrate leaves dotenv and file blobs untouched"` (proj `mixproj`) — note this one ALSO pushes a `file` external; `m_make_v1_only` only touches `*.properties.age`, leaving the `.file.age` blob alone (correct). + +Worked example — the copy-forward test becomes: + +```bash +@test "migrate copy-forward creates the v2 twin and keeps the v1 blob (byte-identical)" { + make_v1_store + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + create_project_dir cfproj + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push cfproj >/dev/null 2>&1 + m_make_v1_only cfproj + local old; old=$(ls "$SECRETS_DIR/cfproj/external/"*.gradle-properties.age) + run "$SECRETS_BIN" migrate + [ "$status" -eq 0 ] + local new; new=$(ls "$SECRETS_DIR/cfproj/external/"*.properties.age) + [ -f "$old" ] + [ -f "$new" ] + cmp -s "$old" "$new" +} +``` + +- [ ] **Step 2: Repair the finalize tests.** Insert `m_make_v1_only ` after the `push ` line in: + - `"finalize refuses when verify --all is not green"` (proj `failverify`) — then the existing `migrate` step creates the twin; corrupting `*.properties.age` still trips verify. + - `"finalize refuses an un-twinned v1 blob (project not migrated)"` (proj `untwinned`) — leaves a lone v1 blob, no twin: exactly the un-twinned state the test wants. + - `"finalize green path drops v1, keeps v2, stamps the marker"` (proj `finproj`). + - `"finalize cuts a recovery tag before deleting v1 blobs"` (proj — read it from the test). + +- [ ] **Step 3: Repair the EGB-710 manifest-free / undeclared-twin tests.** Insert `m_make_v1_only ` after the `push ` line in: + - `"migrate copy-forwards a v1 properties blob with no .secrets.json (manifest-free)"` (proj `nomanifestblob`) — insert BEFORE the `rm -f .secrets.json` line. + - `"migrate twins a store blob even when the manifest no longer declares it"` (proj `staleblob`) — insert before the `.secrets.json` rewrite. + +- [ ] **Step 4: Repair the `--status` tests that need a real v1 blob.** + - `"migrate --status flags a project that needs migrating"` (proj `needsmig`) — insert `m_make_v1_only needsmig` after push, so a lone un-twinned v1 blob exists → NEEDS MIGRATE. + - `"migrate --status reports finalize-ready once every blob is twinned"` (proj `readymig`) — insert `m_make_v1_only readymig` after push and BEFORE the `migrate` step (migrate then creates the twin → finalize-ready). + +- [ ] **Step 5: Run the full migrate suite** + +Run: `bats test/migrate.bats` +Expected: ALL pass. If any copy-forward test still reports "nothing to migrate", its `m_make_v1_only` call is missing or misplaced (must come after push, before migrate). + +- [ ] **Step 6: Run the WHOLE suite** (manifest.bats exercises externals end-to-end and must still be green) + +Run: `bats test/` +Expected: all pass. If a `manifest.bats` external test fails, check it isn't asserting a specific suffix that additive-v2 changed (a fresh push now writes `.properties.age`); update such an assertion the same way (assert `.properties.age`, or use the resolver-agnostic round-trip via pull). + +- [ ] **Step 7: Commit** + +```bash +git add test/migrate.bats +git commit -m "test: fabricate old-client v1 blobs in migrate/finalize/status fixtures (additive v2, EGB-712)" +``` + +--- + +## Task 4: `migrate --status` — dual-write coverage line + +**Files:** `secrets` (`_migrate_status`, `secrets:2190-2227`), `test/migrate.bats`. + +Adds visibility into which `properties` externals are v2-only (old clients can't read them — the forcing function) vs dual-written (old clients still served). + +- [ ] **Step 1: Write the failing test.** Add to `test/migrate.bats`: + +```bash +@test "migrate --status counts v2-only externals (old clients not served)" { + make_v1_store + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + create_project_dir v2onlyext + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + "$SECRETS_BIN" push v2onlyext >/dev/null 2>&1 # v2-only (fresh, no v1 twin) + run "$SECRETS_BIN" migrate --status + [ "$status" -eq 0 ] # no v1 blobs -> finalize-ready + [[ "$output" == *"v2-only"* ]] || false # surfaced as v2-only coverage +} +``` + +- [ ] **Step 2: Run it, confirm it fails** + +Run: `bats test/migrate.bats -f "counts v2-only externals"` +Expected: FAIL — `_migrate_status` currently only counts `*.gradle-properties.age`; it never mentions `v2-only`. + +- [ ] **Step 3: Extend `_migrate_status`.** In `_migrate_status` (`secrets:2190`), inside the `for dir` loop, after the existing `while ... done < <(find "$dir" -type f -name '*.gradle-properties.age' ...)` block and before the per-project classification, add a v2-only count, then surface it in the per-project line. Concretely, replace the classification block: + +```bash + if [ "$v1" -eq 0 ]; then + echo " $project: v2-ready (no v1 properties blobs)" + elif [ "$untwinned" -eq 0 ]; then + echo " $project: migrated ($v1 v1 blob(s), all twinned)" + else + echo " $project: NEEDS MIGRATE ($untwinned of $v1 v1 blob(s) un-twinned) — cd into the project and run 'secrets migrate'" + any_untwinned=1 + fi +``` + +with: + +```bash + # v2-only externals: a .properties.age with no .gradle-properties.age twin — + # old (v1) clients cannot read these (the additive-v2 forcing function). + local v2only=0 pf + while IFS= read -r pf; do + [ -f "$pf" ] || continue + [ -f "${pf%.properties.age}.gradle-properties.age" ] || v2only=$((v2only + 1)) + done < <(find "$dir" -type f -name '*.properties.age' 2>/dev/null) + local v2note="" + [ "$v2only" -gt 0 ] && v2note=" [$v2only v2-only — old clients not served]" + if [ "$v1" -eq 0 ]; then + echo " $project: v2-ready (no v1 properties blobs)$v2note" + elif [ "$untwinned" -eq 0 ]; then + echo " $project: migrated ($v1 v1 blob(s), all twinned)$v2note" + else + echo " $project: NEEDS MIGRATE ($untwinned of $v1 v1 blob(s) un-twinned) — cd into the project and run 'secrets migrate'$v2note" + any_untwinned=1 + fi +``` + +(`local` inside the loop is bash-3.2-fine — it re-declares per iteration.) + +- [ ] **Step 4: Run the new test + status suite** + +Run: `bats test/migrate.bats -f "status"` +Expected: all status tests PASS, including the new v2-only one. + +- [ ] **Step 5: Run the full suite** + +Run: `bats test/` +Expected: all pass. + +- [ ] **Step 6: Commit** + +```bash +git add secrets test/migrate.bats +git commit -m "feat: migrate --status surfaces v2-only externals (coverage, EGB-712)" +``` + +--- + +## Task 5: Docs, help, version, changelog + +**Files:** `secrets` (`cmd_help`), `CLAUDE.md`, `README.md`, `VERSION`, `CHANGELOG.md`. + +- [ ] **Step 1: `cmd_help` — reframe finalize as optional.** In `cmd_help()`, replace the finalize line: + +``` + secrets migrate --finalize Drop v1 blobs and mark the store v2 (after verify) +``` + +with: + +``` + secrets migrate --finalize Optional GC: drop v1 blobs and mark the store pure v2 +``` + +- [ ] **Step 2: `CLAUDE.md` — additive-v2 paragraph.** In the "Store format" bullet (line ~65), after the migration-chain sentence, add (new sentence, same bullet): + +``` + Additive v2 (EGB-712): upgraded clients read either blob suffix (`_resolve_external_blob_read` tries `.properties.age` then `.gradle-properties.age`) and dual-write a `properties` external only when a v1 twin already exists (`_external_blob_write_targets`) — so existing externals keep old clients fresh, brand-new externals are v2-only (a gentle forcing function), and `migrate --finalize` is now OPTIONAL GC rather than a required, coordination-gated flag-day. dotenv and `file` blobs are identical across formats, so they always propagate to old clients. +``` + +- [ ] **Step 3: `README.md` — propagation table.** Add, near the migrate rows in the command table or in a short "Upgrading / store format" subsection, the propagation-by-secret-type summary (verbatim from spec §6): + +```markdown +**Do teammates on an older `secrets` get new secrets?** + +| Secret type | Old client gets it? | +|---|---| +| `.env` / `.env.*` / `.dev.vars` | **Yes, always** (blob name identical across formats) | +| whole-file external | **Yes, always** | +| `properties` external that already existed | **Yes** (dual-written) | +| brand-new `properties` external | **No — must upgrade `secrets`** (the forcing function) | + +"Upgrade your secrets" = `git pull` the tool clone (binary ≥ 0.6.0.0) and/or `secrets migrate` the store. A read-only teammate only needs the tool `git pull`. +``` + +- [ ] **Step 4: `VERSION`** — set to `0.7.0.0`. + +- [ ] **Step 5: `CHANGELOG.md`** — insert above the top entry: + +```markdown +## [0.7.0.0] - 2026-06-08 + +### Changed + +- **Additive store-format v2 (EGB-712)** — upgraded `secrets` clients now read + either external blob suffix (`.properties.age` or the legacy + `.gradle-properties.age`) and **dual-write** a `properties` external whenever a + v1 twin already exists in the store. Existing externals keep working for + teammates on an older `secrets`; only a brand-new `properties` external is + written v2-only (a gentle "upgrade to see it" forcing function). dotenv and + whole-`file` externals are unchanged across formats and always propagate. +- **`secrets migrate --finalize` is now optional GC**, not a required milestone. + Because clients dual-write and read-fall-back, no teammate is ever cut off by + *not* finalizing; finalize only reclaims the duplicate v1 blobs, and stays + deferrable indefinitely. Its safety gates are unchanged. + +### Added + +- **`secrets migrate --status`** now reports `v2-only` externals per project + (the ones an un-upgraded client cannot read), so you can see the forcing + function's footprint at a glance. +``` + +- [ ] **Step 6: Run the full suite** + +Run: `bats test/` +Expected: all pass (docs don't affect tests). Confirm the count is baseline 242 + net new tests from Tasks 1/2/4 (read-fallback, two write tests, v2-only status) minus the rewritten back-compat test (replaced, not added) = **246**. + +- [ ] **Step 7: Sanity — help renders** + +Run: `./secrets help 2>&1 | grep -- "--finalize"` +Expected: shows the reframed "Optional GC" line. (Help only prints; touches no store.) + +- [ ] **Step 8: Commit** + +```bash +git add secrets CLAUDE.md README.md VERSION CHANGELOG.md +git commit -m "docs: additive-v2 propagation + optional-GC finalize; bump 0.7.0.0 (EGB-712)" +``` + +--- + +## Self-review against the spec + +- **§1 read resolution** → Task 1 (`_resolve_external_blob_read`, wired into pull + verify). Test: read-fallback. +- **§2 write rule / twin rule** → Task 2 (`_external_blob_write_targets`, push wiring). Tests: fresh-→v2-only, existing-twin-→dual-write. +- **§3 marker NOT stamped** → no production change (push never touches the marker); guarded implicitly by Task 3 keeping the `make_v1_store; push; migrate` flow working (store stays markerless after push). The plan deliberately does not add marker-stamping. +- **§4 finalize = optional GC** → unchanged logic; reframed in Task 5 docs/help. Existing finalize tests stay green (Task 3 keeps them green). +- **§5 migrate / --status coverage** → Task 4 (v2-only line); `migrate` copy-forward unchanged (EGB-710). +- **§6 propagation table** → Task 5 README/CLAUDE.md. +- **§7 caveat** → documented in CHANGELOG/CLAUDE.md framing; the read-fallback test exercises the v1-only read path. +- **§8 back-compat matrix** → covered across Task 1 (reads), Task 2 (writes), Task 3 (old-client v1 blobs simulated). +- **Safety invariants** → no path-rail changes; bash 3.2 (no associative arrays; `while read` + `find`); recursive walks unchanged; finalize gating untouched. + +**Type/name consistency:** `_resolve_external_blob_read(project, slug, mtype)` and `_external_blob_write_targets(project, slug, mtype)` use the same arg order everywhere; test helpers `m_fake_v1_twin` (keep both) and `m_make_v1_only` (rename to v1-only) are used consistently per their documented semantics. + +**Placeholder scan:** none — every step carries verbatim code or an exact enumerated edit with the precise insertion point. + +## Operator-local follow-up (not part of this plan) + +Per `.ship-policy.json`, before any PR ask the operator to run `./test/run-security.sh` and complete the SIGNOFF. EGB-712 also needs its one stale AC bullet ("Marker auto-stamps…") corrected to match the §3 decision (no auto-stamp) — a one-line Linear edit. diff --git a/docs/superpowers/specs/2026-06-08-additive-v2-dual-write-design.md b/docs/superpowers/specs/2026-06-08-additive-v2-dual-write-design.md index 7827e44..cda4e6d 100644 --- a/docs/superpowers/specs/2026-06-08-additive-v2-dual-write-design.md +++ b/docs/superpowers/specs/2026-06-08-additive-v2-dual-write-design.md @@ -65,12 +65,24 @@ No stored state is required: the presence/absence of the v1 twin **is** the signal. Mental model: *"keep alive what old clients already knew; new things are v2-only."* -### 3. The marker — auto-stamped, informational only +### 3. The marker — unchanged meaning, NOT auto-stamped -The first push by an upgraded client stamps `.secrets-format = 2`. Because writes -are now driven by the twin rule (not the marker), the marker no longer decides -blob location — it becomes purely "a v2-aware client has touched this store." -`secrets which` continues to report `format: vN`. +Additive-v2 read/write behavior does **not** depend on the `.secrets-format` +marker at all (reads try both suffixes; writes follow the twin rule). So the +marker is left exactly as it is today: set only by `init` (born-v2, a fresh pure-v2 +store) or `migrate --finalize` (a store GC'd to pure v2). A transitional store +that upgraded clients are dual-writing stays **markerless (v1)** — which is +*accurate*: it has not been finalized to pure v2, and v1 twins still exist. + +Push deliberately does **not** auto-stamp the marker. (An earlier draft proposed +auto-stamping on first push; that was dropped during planning because it would +make a v1 store read as v2 the moment anyone pushed — turning every `migrate` +into a no-op and contradicting the "v1 until finalized" model the whole +migration relies on. The marker's only purposes — `secrets which` display and +gating `migrate`/`finalize` — are better served by it continuing to mean +"finalized/pure v2.") + +`secrets which` continues to report `format: vN` from the marker. ### 4. `finalize` → optional GC, never required @@ -145,8 +157,8 @@ blobs — YAGNI for v1). path(s) to write: for `properties`, both suffixes when a v1 twin already exists, else v2-only; single path for `file`. - **`push_external_files`** — write to every path from - `_external_blob_write_targets` (was a single `age -o`); auto-stamp the marker - on first push by an upgraded client. + `_external_blob_write_targets` (was a single `age -o`). Push does **not** stamp + the marker (see §3). - **`pull_external_files` / `cmd_verify`** — resolve blobs via `_resolve_external_blob_read` (was the single-suffix lookup). - **`_migrate_finalize`** — unchanged logic; doc/help reframed as optional GC. @@ -174,8 +186,9 @@ blobs — YAGNI for v1). 3. Twin rule — new external → v2-only: push a brand-new `properties` external; assert **only** `.properties.age` is written (no v1 twin created) — the forcing function. -4. Marker auto-stamp: first push by an upgraded client on a markerless store - stamps `.secrets-format = 2`. +4. Marker NOT stamped on push: a `make_v1_store` + push leaves `.secrets-format` + absent (store stays v1/transitional); the existing `make_v1_store; push; + migrate` flow is unaffected. 5. dotenv/`file` unaffected: a dotenv and a `file` external round-trip identically regardless of store marker. 6. `migrate --status` coverage: reports which externals are dual-written vs