jason.woltje
158205c5cc
Unclaimed files left in the canonical checkout. Inventory: canonical-cleanup-inventory.md (commit-as-evidence groups). detect-secrets scan clean on 2026-11-02: hits are sha256 pins, git object ids, test sentinels already in tracked tests and unit-file lines.
158 lines
8.6 KiB
Markdown
158 lines
8.6 KiB
Markdown
# Row 186: the activate plan's backup line says what the run will do
|
|
|
|
Brief: `docs/plans/2026-10-20_plan-backup-line.md` (blob 9610b31d0c19, f5c72a5c3). Gitea #1684, P19 task 404.
|
|
These are Filbert's N1 and N2 on row 185 (#1683 comment 29404). Owner Darkwing, reviewer Filbert, gate owner Sage.
|
|
|
|
## Files
|
|
|
|
| File | What |
|
|
|---|---|
|
|
| `build.patch` | the change, `git diff 1d7fd75e2` over the three paths below |
|
|
| `candidate-manifest.sha256` | one line per changed path |
|
|
|
|
- Base: 1d7fd75e2. None of the three paths changed between the brief's b724b7b0b and 1d7fd75e2, so the
|
|
brief's line numbers hold.
|
|
- Changed: `scripts/stack-release.sh`, `packages/cli/tests/stack-release.test.mjs` and `docs/TOOLS.md`. Nothing
|
|
outside the brief's list.
|
|
|
|
## Design
|
|
|
|
### The plan (change 1)
|
|
|
|
`plan` now sets a local `early` in the migration block, in the two branches where `run_steps` (:846-847) rolls
|
|
back before `backup_store` (:849):
|
|
|
|
- in chain without `--migrate`: `[ "$MIGRATE" = 1 ] || early=1`, next to the `needs: --migrate` line that
|
|
already says the run would roll back with `migration-needs-flag`;
|
|
- outside the chain: `early=1`, next to the `store-newer` line.
|
|
|
|
The backup block then picks one of four:
|
|
|
|
| Case | Line |
|
|
|---|---|
|
|
| rollback | none (unchanged) |
|
|
| activate, no store file | `backup: none; there is no store file` (unchanged) |
|
|
| activate, `early` set | `backup: none; the run rolls back before the backup` (new) |
|
|
| activate, otherwise | `backup: the run backs up the stopped store to <dataRoot>/backups/stack/<activation>/ before any migration` (unchanged) |
|
|
|
|
The no-store test now comes before `early`. With no store file `plan_store` falls back to the pointer's schema,
|
|
and the run skips the whole store block (:839), so "there is no store file" is the right reason even when the
|
|
pointer's schema is outside the target's chain.
|
|
|
|
Row 185's BUILD.md said I did not add a third form for these two cases, since the migration lines already said
|
|
the run would roll back. Filbert's N1 is right that the plan then states two things that can't both happen. This
|
|
row adds that form.
|
|
|
|
### The S_CHANGED reading (change 2)
|
|
|
|
When the bus stops during the held-store read, `plan_store` sets `S_CHANGED` and the store schema prints as
|
|
`unknown ... run --plan again`. The migration block prints nothing and leaves `early` empty, so the plan keeps
|
|
today's backup line (T29 pins it).
|
|
|
|
That is the safe reading. The plan can't tell which of three things the run will do with the stopped store: back
|
|
it up (same schema, or in chain with `--migrate`), or roll back with `store-newer` or `migration-needs-flag`. If
|
|
the backup line turns out wrong, the run rolled back before it touched the store, so the store is as it was and
|
|
nothing needed a backup. If a "none" line turns out wrong, the run backed up and went on to install, perhaps
|
|
migrate, and the operator holds a plan saying there is no backup to restore from. The first mistake costs
|
|
nothing. The second hides the one directory a `--restore` needs. The store schema line already tells the
|
|
operator to plan again before running.
|
|
|
|
### A reading outside the brief: the store schema from the pointer
|
|
|
|
`plan_store` also falls back to the pointer's schema when the bus is stopped and the store is unclean (a `-wal`
|
|
or `-shm` beside it) or unreadable. The run then rolls back with `store-unclean` or `schema-unreadable`
|
|
(:840-841), both before the backup, but the plan judges `early` by the pointer's schema and can print the backup
|
|
line. The brief names only `store-newer` and `migration-needs-flag`, and the plan's store schema line already
|
|
says the store was not read and why, so I left this alone. It's a bounded follow-up if Filbert or Sage want it:
|
|
a `backup: none; the run rolls back with store-unclean` form for a stopped, unclean store.
|
|
|
|
### What does not change (change 3)
|
|
|
|
`run_steps`, `backup_store`, every refusal code, exit code and flag. The diff to `scripts/stack-release.sh` is
|
|
inside `plan` only.
|
|
|
|
### docs/TOOLS.md (change 4)
|
|
|
|
"Pinned stack update" step 3 now lists the backup among what `--plan` names, and says the `backup:` line gives
|
|
the directory or why the run takes none: no store file, or a rollback with `store-newer` or
|
|
`migration-needs-flag` first.
|
|
|
|
## Tests
|
|
|
|
`packages/cli/tests/stack-release.test.mjs`:
|
|
|
|
- `planNoBackup`, the new line, sits beside `planBackup`.
|
|
- T26: the not-given case now expects `planNoBackup` where it expected `planBackup`. The `--migrate` case and the
|
|
same-schema check are unchanged.
|
|
- T52 (new), six subtests on a fresh stand-in each. Each plans, checks the plan's only `backup` line and its
|
|
migration or needs line, checks nothing was written (`fp`, the data root tree, no starts or stops, no log
|
|
line, no `backups`), then runs the same activate and checks `backups/stack` exists, and the log line names
|
|
the activation as its backup, exactly when the plan printed the backup line:
|
|
|
|
| Subtest | Store | Target | Flags | Plan | Run exit |
|
|
|---|---|---|---|---|---|
|
|
| same schema | 3g | C1 (3g) | none | backup line | 0 |
|
|
| same schema with --migrate | 3g | C1 | `--migrate` | backup line | 0 |
|
|
| in chain with --migrate | 3g | C (3h) | `--migrate` | backup line | 0 |
|
|
| in chain without --migrate | 3g | C | none | none, rolls back | 1 |
|
|
| outside the chain | 3z | C | none | none, rolls back | 1 |
|
|
| outside the chain with --migrate | 3z | C | `--migrate` | none, rolls back | 1 |
|
|
|
|
Brief tests: 1 is T50 (unchanged) and T52's same-schema cases. 2 is T26's `--migrate` case and T52. 3 is T26's
|
|
not-given case and T52. 4 is T52's two outside-the-chain cases. 5 is T51 (unchanged). 6 is T50, T51 and T52's
|
|
write checks; T26 runs only `--plan`.
|
|
|
|
No test reads `docs/TOOLS.md`, and no test pins a hash of any of the three files (grep of `packages/*/tests`,
|
|
`scripts/test-*.sh` and every non-doc file for the paths and for 64-hex pins).
|
|
|
|
## Results
|
|
|
|
Run from a detached worktree of 1d7fd75e2 with the change applied, `TMPDIR` a scratch directory outside `/tmp`,
|
|
stamps from `date -u`. Nothing touched `~/.mosaic-dev` or a live unit. Every case uses the test's stand-in
|
|
`systemctl`, units, stores and data root.
|
|
|
|
- `node --test packages/cli/tests/stack-release.test.mjs`, whole file: 132 tests, 132 pass, 0 fail
|
|
(08:45:15Z to 08:49:31Z).
|
|
- Base control: the base `scripts/stack-release.sh` under the new tests fails T26 and T52's three rollback
|
|
subtests (in chain without `--migrate`, outside the chain, outside the chain with `--migrate`). So the new
|
|
expectations are not met by today's plan.
|
|
|
|
Mutants: each is one exact replacement in the candidate's `scripts/stack-release.sh`, run against the plan tests
|
|
(`^(T15|T23|T26|T27|T29|T50|T51|T52) `, 18 tests), then put back and checked by sha256.
|
|
|
|
| Mutant (brief) | Edit | Result | Failing tests |
|
|
|---|---|---|---|
|
|
| m1 the plan ignores `--migrate` when it picks the backup line | drop `[ "$MIGRATE" = 1 ] \|\| early=1` | killed, 3 fail | T26, T52 in chain without `--migrate` |
|
|
| m2 the plan prints the backup line for a store outside the chain | drop `early=1` in the `store-newer` branch | killed, 3 fail | T52 outside the chain, both subtests |
|
|
| m3 the plan prints the `none; the run rolls back` line for a same-schema activate | add `early=1` in the same-schema branch | killed, 5 fail | T26, T50, T52 same schema, both subtests |
|
|
|
|
(The fail counts include the T52 parent with its failing subtests.)
|
|
|
|
Candidate (`candidate-manifest.sha256`, sha256 449e91219333…):
|
|
|
|
```
|
|
6afc6bda76c450550ff6e0119b25dfafc20af219f7c512bc7d696c9d98b882fa docs/TOOLS.md
|
|
1d1e6e2260f0ee34ee5e140156238fc952afc6540034c88dff61a09b349ec3b7 packages/cli/tests/stack-release.test.mjs
|
|
983037ee7bb970de82a0448761c675904910de9dc256bfb19438349014668879 scripts/stack-release.sh
|
|
```
|
|
|
|
`build.patch` (sha256 75efa836…) applies with `git apply --check` on 1d7fd75e2.
|
|
|
|
## For Sage's gate
|
|
|
|
The cli suite and every `scripts/test-*.sh` from a git worktree of the candidate, per the brief. I ran only
|
|
`stack-release.test.mjs`. It is the one test that reads any of the three files.
|
|
|
|
## Review
|
|
|
|
Filbert approved round 1 on digest 449e91219333 (#1684 comment 29419, rev 2198, 08:59:42Z). Their run of
|
|
stack-release.test.mjs passed 132/132. Their M7 (an `S_CHANGED` read counts as early) was killed by T29, which
|
|
pins the safe reading above. Two non-blocking findings stay as follow-ups and are not in this candidate:
|
|
|
|
- N1: their M6 (the `early` test placed ahead of the no-store test) survives. No T52 case has a missing store
|
|
with a pointer schema outside the target's chain. The order in the candidate is right.
|
|
- N2: the unclean-store reading above, as a bounded follow-up. Use the `store-unclean` form only when the plan
|
|
saw the stopped store unclean, since a held read that failed may read fine once the run stops the bus.
|
|
|
|
Both fit one small follow-up row: the N2 form plus a T52 case for each.
|