Money import Undo can silently remove later Budget edits (#1140) #1177

Open
opened 2026-10-06 04:48:46 +00:00 by kayg · 2 comments
Owner

Blocking finding from the independent #1140 review of job/wizard-1140 at 02050bea9.

MoneyImportSession.undoImport reads listBudgets, resolves the imported stable Budget ID, then calls the shared Files trash action on the entire Budget folder. The import receipt has only created, replacedTrash and completedAt. There is no publication source revision and no compare-and-swap check.

A User can import, edit the new Budget, then select Undo import. The same path trashes the edited Budget and restores the older one, without a refusal message or a merge. Recovery from Trash remains possible, but Undo silently removes later edits from Money.

Required: a server-owned conditional inverse with a durable publication source set. Check all source file names and exact bytes immediately before Undo. Refuse after later edits with plain text, or implement the merge specified in #1140. Refusal must keep both Budgets and receipts unchanged. Test a rename, a transaction edit, a new month, an unknown preserved line and a fresh Installation after restart. Do not solve this with a browser-only check: other writers can race it.

Blocks SAFE TO MERGE for #1140. No existing test expectation was changed.

Blocking finding from the independent #1140 review of `job/wizard-1140` at `02050bea9`. `MoneyImportSession.undoImport` reads `listBudgets`, resolves the imported stable Budget ID, then calls the shared Files `trash` action on the entire Budget folder. The import receipt has only `created`, `replacedTrash` and `completedAt`. There is no publication source revision and no compare-and-swap check. A User can import, edit the new Budget, then select Undo import. The same path trashes the edited Budget and restores the older one, without a refusal message or a merge. Recovery from Trash remains possible, but Undo silently removes later edits from Money. Required: a server-owned conditional inverse with a durable publication source set. Check all source file names and exact bytes immediately before Undo. Refuse after later edits with plain text, or implement the merge specified in #1140. Refusal must keep both Budgets and receipts unchanged. Test a rename, a transaction edit, a new month, an unknown preserved line and a fresh Installation after restart. Do not solve this with a browser-only check: other writers can race it. Blocks SAFE TO MERGE for #1140. No existing test expectation was changed.
Author
Owner

Implemented #1177 in b825bae753ad72d0f9054ae8615376c0acffcb8e. Undo is a User-bound server mutation. It compares exact published file names and BLAKE3 hashes under the Root writer lock before committing any inverse intent. Folder rename, transaction edit, new month, unknown line, unknown file and filename changes refuse with the required plain text and leave files and receipt unchanged. The browser never falls back to Files Trash/restore.

#1178 focused recovery evidence also covers abrupt child process exit at phases 1–6, cancellation before publication, byte-exact predecessor restoration and harmless consumed-receipt replay after manual restoration.

test result: ok. 9 passed; 0 failed; 1 ignored; 0 measured; 107 filtered out; finished in 34.43s
Test Files  2 passed (2)
     Tests  37 passed (37)
test result: ok. 96 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 19.51s

The ignored process entry is invoked explicitly by the passing parent test. Other filesystem integration targets passed. Money and filesystem Clippy passed. Final combined gates and visual evidence remain in progress. Legacy receipts without trusted publication hashes cannot enable Undo by hashing edited files retroactively.

Implemented #1177 in `b825bae753ad72d0f9054ae8615376c0acffcb8e`. Undo is a User-bound server mutation. It compares exact published file names and BLAKE3 hashes under the Root writer lock before committing any inverse intent. Folder rename, transaction edit, new month, unknown line, unknown file and filename changes refuse with the required plain text and leave files and receipt unchanged. The browser never falls back to Files Trash/restore. #1178 focused recovery evidence also covers abrupt child process exit at phases 1–6, cancellation before publication, byte-exact predecessor restoration and harmless consumed-receipt replay after manual restoration. ``` test result: ok. 9 passed; 0 failed; 1 ignored; 0 measured; 107 filtered out; finished in 34.43s Test Files 2 passed (2) Tests 37 passed (37) test result: ok. 96 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 19.51s ``` The ignored process entry is invoked explicitly by the passing parent test. Other filesystem integration targets passed. Money and filesystem Clippy passed. Final combined gates and visual evidence remain in progress. Legacy receipts without trusted publication hashes cannot enable Undo by hashing edited files retroactively.
Author
Owner

All four review findings are implemented in ten atomic commits. The branch is not fully green: the server suite has an unresolved startup deadline failure, tracked in #1192. Do not mark this report READY FOR MERGE.

Branch: job/wizard-1140. Head: abd4812a0a6a59a48e68f1aabeae2aafed31cbe0. The one origin/dev merge check returned Already up to date. No push, deploy, promotion or merge into dev was performed.

Built

  • #1177: an owner-bound server Undo endpoint compares exact published names and BLAKE3 hashes under the shared writer lock. Any edit or rename refuses with the owner's exact plain-text copy. Refusal changes no file or receipt. A matching import goes to Trash and its predecessor is restored in one journal. The receipt is single-use, including restart, replay and late parser completion.
  • #1178: the server owns replacement staging, durable intent, old-budget Trash, publication and finalization. FULL commits precede source mutations. Restart and real process-exit tests cover all four publication phases and both inverse phases. Cancellation restores the original exact bytes before publication. Admin queue cancellation checks both pending cancellation state and leased cancellation flags; abandonment cannot produce a false creation receipt.
  • #1183: Admin Money job lists/details expose content-free operational fields. They hide names, amounts, item counts, raw errors, results and receipts. Retry/cancel remains available. The production route test uses an Admin and a second User. DESIGN §48 records the isolation exception.
  • #1184: both production upgrade fixtures now include Notifications 0007. Original receipt timestamps/checksums, retained data and repeat-upgrade assertions remain.
  • Contracts, generated API types, classifications and exact performance proofs are refreshed. No guard was weakened; scoped exceptions fell from 22177 to 22149. The existing benchmark now covers publication, replacement and both inverses.

UX gaps closed

Edited imports retain their Budget and Undo receipt after refusal. Unchanged Undo works with touch and keyboard. Open Budget navigates. Replacement uses the server operation. Session changes fence writes. Admin views carry no Budget content. Six macOS-emulated production cases cover 390, 820 and 1440 px, light and dark; 36 screenshots are attached. Action alignment crops were checked. Visual quality review remains with the orchestrator.

UX gaps left / known gaps

Legacy receipts without trusted published hashes refuse Undo. There is no separate one-click redo toast after Undo; Files never restores the replacement from the browser. The full server gate remains failed at the unchanged 15-second startup deadline. An isolated fresh-process retry also failed; a load-only cause is not established. #1192 has both outputs. The focused Admin regression, all five upgrade tests and the remaining server integration checks pass.

Decisions

Use the existing FULL authority pool for the small, non-rebuildable import intents and inverse receipts; parsing/progress stays NORMAL. DESIGN §2 records this narrow exception. Legacy receipts fail closed; do not manufacture trusted hashes from possibly edited files. Reuse the existing queue and filesystem journal with deterministic private Trash names, including rename-once replay when restoration reuses the source path.

Gate output (verbatim excerpts)

cargo fmt --check passed with exit 0 and no output. git diff --check passed.

Per-crate Clippy (--all-targets -- -D warnings), in order: calternal-fs, calternal-plugin-money, calternal-server:

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 3m 31s
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 39.36s
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 41.59s

cargo test -p calternal-fs:

test result: ok. 96 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 19.51s
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.55s
test result: ok. 48 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.29s
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

cargo test -p calternal-plugin-money -- --test-threads=4:

test result: ok. 119 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 156.47s
test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 8.90s

cd apps/web && bun run check; bun run test --maxWorkers=2:

perf-lint: PASS; 0 violations; 22149 scoped exceptions
svelte-check found 0 errors and 4 warnings in 3 files
 Test Files  271 passed (271)
      Tests  1883 passed (1883)

cargo test -p calternal-server -- --test-threads=4 — failed:

HTTP startup waited for an upgrade backfill: Elapsed(())
test result: FAILED. 255 passed; 1 failed; 11 ignored; 0 measured; 0 filtered out; finished in 102.92s

Unchanged isolated startup retry — failed:

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 266 filtered out; finished in 97.22s

Focused wire::tests::admin_money_jobs_are_content_free -- --ignored --exact --test-threads=1:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 266 filtered out; finished in 20.85s

cargo test -p calternal-server --test perf_guards --test private_index_permissions -- --test-threads=4:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 45.79s
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.85s

python3 -m unittest discover -s tests/adversarial -p test_xuser_classification.py:

Ran 16 tests in 0.421s

OK

For the merge round

  • Resolve #1192 and run cargo test -p calternal-server -- --test-threads=4; prove HTTP startup does not wait for a locked upgrade backfill. Keep the current expectation.
  • Run tests/adversarial/run.sh; prove the combined authorization, cross-User and hostile-input matrices. This job ran focused real-route regressions and offline classifications, not the full matrices.
  • After the combined production build, run CALTERNAL_E2E_ASSET_OVERRIDE=1 bun apps/web/e2e/money-wizard-1140.mjs --acceptance with CALTERNAL_SERVER_BIN set to that build; prove the full format/restart/replacement acceptance round.
  • Staging/o2 and real macOS interoperability remain merge-round work under the verification policy. The requested second independent numbers review remains outstanding.
  • This is a correctness job. No performance run was made under the current policy. When measured on the perf VM, use flock /root/perf.lock bash -c 'uptime; bun bench/money-wizard-1140.mjs' with the combined shared release binary in CALTERNAL_SERVER_BIN; record p50/p95, CPU/RSS and the burst beside docs/perf/baseline.json.

Screenshot attachments

Files

  • apps/web/e2e/money-wizard-1140.mjs
  • apps/web/src/lib/money/api.test.ts
  • apps/web/src/lib/money/api.ts
  • apps/web/src/lib/money/import-session.svelte.test.ts
  • apps/web/src/lib/money/import-session.svelte.ts
  • bench/money-wizard-1140.mjs
  • contracts/action-policy.json
  • contracts/actions.json
  • contracts/openapi.json
  • contracts/perf/adoption-1058.json
  • contracts/perf/exceptions.json
  • contracts/perf/ratchet.json
  • contracts/perf/registry.json
  • crates/calternal-fs/src/journal.rs
  • crates/calternal-fs/src/trash.rs
  • crates/calternal-server/src/upgrade_tests.rs
  • crates/calternal-server/src/wire.rs
  • crates/plugins/money/src/durable_import.rs
  • crates/plugins/money/src/durable_import_tests.rs
  • crates/plugins/money/src/routes.rs
  • crates/plugins/money/src/tests.rs
  • docs/DESIGN.md
  • packages/api-client/src/generated.ts
  • tests/adversarial/test_xuser_classification.py
  • tests/adversarial/xuser_matrix.py

Cleanup

Cargo clean completed:

Removed 18614 files, 13.6GiB total

Web build output, .svelte-kit and the job's generated test temporary directory were removed. Review artifacts remain ignored and attached to #1140. The worktree is clean.

All four review findings are implemented in ten atomic commits. The branch is **not fully green**: the server suite has an unresolved startup deadline failure, tracked in [#1192](https://git.kayg.org/kayg/calternal/issues/1192). Do not mark this report READY FOR MERGE. Branch: `job/wizard-1140`. Head: `abd4812a0a6a59a48e68f1aabeae2aafed31cbe0`. The one origin/dev merge check returned `Already up to date.` No push, deploy, promotion or merge into dev was performed. **Built** - #1177: an owner-bound server Undo endpoint compares exact published names and BLAKE3 hashes under the shared writer lock. Any edit or rename refuses with the owner's exact plain-text copy. Refusal changes no file or receipt. A matching import goes to Trash and its predecessor is restored in one journal. The receipt is single-use, including restart, replay and late parser completion. - #1178: the server owns replacement staging, durable intent, old-budget Trash, publication and finalization. FULL commits precede source mutations. Restart and real process-exit tests cover all four publication phases and both inverse phases. Cancellation restores the original exact bytes before publication. Admin queue cancellation checks both pending cancellation state and leased cancellation flags; abandonment cannot produce a false creation receipt. - #1183: Admin Money job lists/details expose content-free operational fields. They hide names, amounts, item counts, raw errors, results and receipts. Retry/cancel remains available. The production route test uses an Admin and a second User. DESIGN §48 records the isolation exception. - #1184: both production upgrade fixtures now include Notifications 0007. Original receipt timestamps/checksums, retained data and repeat-upgrade assertions remain. - Contracts, generated API types, classifications and exact performance proofs are refreshed. No guard was weakened; scoped exceptions fell from 22177 to 22149. The existing benchmark now covers publication, replacement and both inverses. **UX gaps closed** Edited imports retain their Budget and Undo receipt after refusal. Unchanged Undo works with touch and keyboard. Open Budget navigates. Replacement uses the server operation. Session changes fence writes. Admin views carry no Budget content. Six macOS-emulated production cases cover 390, 820 and 1440 px, light and dark; 36 screenshots are attached. Action alignment crops were checked. Visual quality review remains with the orchestrator. **UX gaps left / known gaps** Legacy receipts without trusted published hashes refuse Undo. There is no separate one-click redo toast after Undo; Files never restores the replacement from the browser. The full server gate remains failed at the unchanged 15-second startup deadline. An isolated fresh-process retry also failed; a load-only cause is not established. #1192 has both outputs. The focused Admin regression, all five upgrade tests and the remaining server integration checks pass. **Decisions** Use the existing FULL authority pool for the small, non-rebuildable import intents and inverse receipts; parsing/progress stays NORMAL. DESIGN §2 records this narrow exception. Legacy receipts fail closed; do not manufacture trusted hashes from possibly edited files. Reuse the existing queue and filesystem journal with deterministic private Trash names, including rename-once replay when restoration reuses the source path. **Gate output (verbatim excerpts)** `cargo fmt --check` passed with exit 0 and no output. `git diff --check` passed. Per-crate Clippy (`--all-targets -- -D warnings`), in order: calternal-fs, calternal-plugin-money, calternal-server: ``` Finished `dev` profile [unoptimized + debuginfo] target(s) in 3m 31s Finished `dev` profile [unoptimized + debuginfo] target(s) in 39.36s Finished `dev` profile [unoptimized + debuginfo] target(s) in 41.59s ``` `cargo test -p calternal-fs`: ``` test result: ok. 96 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 19.51s test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.55s test result: ok. 48 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.29s test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` `cargo test -p calternal-plugin-money -- --test-threads=4`: ``` test result: ok. 119 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 156.47s test result: ok. 27 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 8.90s ``` `cd apps/web && bun run check`; `bun run test --maxWorkers=2`: ``` perf-lint: PASS; 0 violations; 22149 scoped exceptions svelte-check found 0 errors and 4 warnings in 3 files Test Files 271 passed (271) Tests 1883 passed (1883) ``` `cargo test -p calternal-server -- --test-threads=4` — failed: ``` HTTP startup waited for an upgrade backfill: Elapsed(()) test result: FAILED. 255 passed; 1 failed; 11 ignored; 0 measured; 0 filtered out; finished in 102.92s ``` Unchanged isolated startup retry — failed: ``` test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 266 filtered out; finished in 97.22s ``` Focused `wire::tests::admin_money_jobs_are_content_free -- --ignored --exact --test-threads=1`: ``` test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 266 filtered out; finished in 20.85s ``` `cargo test -p calternal-server --test perf_guards --test private_index_permissions -- --test-threads=4`: ``` test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 45.79s test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.85s ``` `python3 -m unittest discover -s tests/adversarial -p test_xuser_classification.py`: ``` Ran 16 tests in 0.421s OK ``` **For the merge round** - Resolve #1192 and run `cargo test -p calternal-server -- --test-threads=4`; prove HTTP startup does not wait for a locked upgrade backfill. Keep the current expectation. - Run `tests/adversarial/run.sh`; prove the combined authorization, cross-User and hostile-input matrices. This job ran focused real-route regressions and offline classifications, not the full matrices. - After the combined production build, run `CALTERNAL_E2E_ASSET_OVERRIDE=1 bun apps/web/e2e/money-wizard-1140.mjs --acceptance` with `CALTERNAL_SERVER_BIN` set to that build; prove the full format/restart/replacement acceptance round. - Staging/o2 and real macOS interoperability remain merge-round work under the verification policy. The requested second independent numbers review remains outstanding. - This is a correctness job. No performance run was made under the current policy. When measured on the perf VM, use `flock /root/perf.lock bash -c 'uptime; bun bench/money-wizard-1140.mjs'` with the combined shared release binary in `CALTERNAL_SERVER_BIN`; record p50/p95, CPU/RSS and the burst beside `docs/perf/baseline.json`. **Screenshot attachments** - [inverse-light-390.zip](https://git.kayg.org/attachments/a4c8756e-074e-43c6-9acc-c73895de3c11) - [inverse-light-820.zip](https://git.kayg.org/attachments/ff6328bc-300f-4a11-83e5-0cf1202dba20) - [inverse-light-1440.zip](https://git.kayg.org/attachments/101b5cff-9b7f-4413-bb82-b8307a8015b8) - [inverse-dark-390.zip](https://git.kayg.org/attachments/656922c3-178e-483a-9b03-480519eae66e) - [inverse-dark-820.zip](https://git.kayg.org/attachments/df3ca33d-8c5e-466c-995c-993fa7279985) - [inverse-dark-1440.zip](https://git.kayg.org/attachments/d921a64b-afe4-4a8f-b3c4-336de453dd13) **Files** - `apps/web/e2e/money-wizard-1140.mjs` - `apps/web/src/lib/money/api.test.ts` - `apps/web/src/lib/money/api.ts` - `apps/web/src/lib/money/import-session.svelte.test.ts` - `apps/web/src/lib/money/import-session.svelte.ts` - `bench/money-wizard-1140.mjs` - `contracts/action-policy.json` - `contracts/actions.json` - `contracts/openapi.json` - `contracts/perf/adoption-1058.json` - `contracts/perf/exceptions.json` - `contracts/perf/ratchet.json` - `contracts/perf/registry.json` - `crates/calternal-fs/src/journal.rs` - `crates/calternal-fs/src/trash.rs` - `crates/calternal-server/src/upgrade_tests.rs` - `crates/calternal-server/src/wire.rs` - `crates/plugins/money/src/durable_import.rs` - `crates/plugins/money/src/durable_import_tests.rs` - `crates/plugins/money/src/routes.rs` - `crates/plugins/money/src/tests.rs` - `docs/DESIGN.md` - `packages/api-client/src/generated.ts` - `tests/adversarial/test_xuser_classification.py` - `tests/adversarial/xuser_matrix.py` **Cleanup** Cargo clean completed: ``` Removed 18614 files, 13.6GiB total ``` Web build output, .svelte-kit and the job's generated test temporary directory were removed. Review artifacts remain ignored and attached to #1140. The worktree is clean.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#1177
No description provided.