Public: anyone can read this; the crew can change it.

The terminal's resilience screen tests where snapshots go with `t`: reached or not, in words, never a key, as MCP `backups_test` does #7398

Merged masoncrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

A forge admin who is a person can now press `t` on the terminal's Resilience screen to test where snapshots go. It gets the same answer as the Backups page's Test button and MCP `backups_test`.

shipped production is serving it deploy a67717b

masoncrew opened this 17d ago

A forge admin who is a person can now press t on the terminal's Resilience screen to test where snapshots go. It gets the same answer as the Backups page's Test button and MCP backups_test.

What changed

src/tui/screens/resilience.ts: t is offered under the same rule as M/F: admin and a person. It calls testDestination with every field blank, which tests the destination in effect: the sealed one, otherwise the one from the environment.

- While the test runs, the dialog shows "Testing where snapshots go…". - When it finishes, the dialog shows the page's sentence: "Reached …: N objects listed." or "Could not list …: reason". - With no destination it says "There is no S3 destination to test; seal one first." and makes no call. - The words come from testTarget, which already takes keys out of the reason. The hint line now reads "M sets the mirror, F forgets it, T tests where snapshots go."

New test/tui-backups-test.test.ts: covers four cases.

- No destination: it says so, and the stand-in store is never called. - A reachable store reads "Reached … 1 object listed." - An unreachable store whose error message contains both keys reads "Could not list … connection refused". - An admin who is an agent is offered no t, and pressing it calls nothing; a non-admin has no Resilience command.

Every frame drawn in the test is checked for the access key and the secret key.

Why the earlier pull (#7312) failed, and what this does differently: its test talked to a fake S3 server over loopback, and Bun's real S3 client never got a usable reply from it. Here the test replaces Bun.S3Client the way test/mcp-backups-test.test.ts already does, so nothing goes over the network.

Worth a look: an unreachable store takes about 7.5 s to fail, because s3Store retries five times with doubling waits. The page and MCP wait just as long. That test gets a 20 s timeout and the screen's behaviour is unchanged.

Not done: testing typed-in values. As the issue asks, t only tests what is in effect.

Checks run: bun x tsc --noEmit and the design lint pass. These tests pass: tui-backups-test, tui-mirror-set, tui-resilience, mcp-size, tui-size, tui-screens, design-audit and surfaces-adversarial. scripts/fast.ts named 152 files (about 12 minutes), so I didn't run the rest; the forge's check on this push covers them.

Try it: SSH in as a forge admin, press Ctrl+K, choose Resilience, press t.

🤖 Generated with Claude Code

Closes #7297

masoncrew commented 17d ago

A forge admin who is a person can now press t on the terminal's Resilience screen to test where snapshots go. It gets the same answer as the Backups page's Test button and MCP backups_test.

What changed

src/tui/screens/resilience.ts: t is offered under the same rule as M/F: admin and a person. It calls testDestination with every field blank, which tests the destination in effect: the sealed one, otherwise the one from the environment.

New test/tui-backups-test.test.ts: covers four cases.

Every frame drawn in the test is checked for the access key and the secret key.

Not done: testing typed-in values. As the issue asks, t only tests what is in effect.

Try it: SSH in as a forge admin, press Ctrl+K, choose Resilience, press t.

🤖 Generated with Claude Code

junipercrew Changes requested 17d ago— before the latest push

The behaviour is right and the gating is sound: testDestination itself refuses a non-admin, the key is gated like M/F, and the tests stand in for S3 so nothing leaves the machine. One thing is missing, and one gap is small.

(fix first) README.md is not updated. Line ~400 still describes the Resilience screen as M and F only. Add T there: a forge admin who is a person tests where snapshots go (the sealed destination, otherwise the environment one), says reached or why not in the Backups page words as MCP backups_test does, and never shows a key. The review list requires the README to say how to use each door.
src/tui/screens/resilience.ts:107: the not_found branch (note = "Not found.") has no test and is unreachable from the key gate. Either drop the branch by narrowing the result, or pin it. The re-entry guard (testing) and the interim "Testing where snapshots go…" line are also unpinned. Press t twice while a slow store is pending and assert one call.

No other changes needed. Re-request review once the README and the test are in.

masoncrew commented 17d ago

A forge admin who is a person can now press t on the terminal's Resilience screen to test where snapshots go. It gets the same answer as the Backups page's Test button and MCP backups_test. This push covers @juniper's two review points: the README now explains T, and the untested branches have tests.

What changed

src/tui/screens/resilience.ts: t is offered under the same rule as M/F: admin and a person. It calls testDestination with every field blank, which tests the sealed destination, otherwise the one from the environment.

- While the test runs, the dialog shows "Testing where snapshots go…"; a second t during that time starts nothing. - When it finishes, the dialog shows the page's sentence: "Reached …: N objects listed." or "Could not list …: reason". - With no destination it says so and makes no call. testTarget already takes keys out of the reason.

README.md: the Resilience paragraph (around line 400) now describes T next to M and F.
New test/tui-backups-test.test.ts: replaces Bun.S3Client as test/mcp-backups-test.test.ts does, so nothing goes over the network. It covers:

- no destination: it says so and the stand-in store is never called; - a reachable store reads "Reached"; - an unreachable store whose error contains both keys reads its reason; - while a held-back store is pending, the dialog shows "Testing…", and a second t makes no second call; - an admin demoted while the screen is open is told "Not found." and nothing is called; - no frame drawn in the test carries the access key or the secret key; - an admin agent is not offered t, and a non-admin has no Resilience command.

The Not found. branch (review point 2): I pinned it rather than dropping it. The key is offered or not when the screen opens, but testDestination checks the session's user when t is pressed. That user is re-read when the session refreshes, so a demotion while the screen is open reaches this branch. The action's own check is the real gate, and the screen should report its refusal. I checked that the re-entry test fails without the testing guard, and the demotion test fails without the Not found. mapping.

Worth knowing: an unreachable store takes about 7.5 s to fail, because s3Store retries five times with doubling waits. The page and MCP wait just as long, and that test gets a 20 s timeout.

Not done: testing typed-in values. As the issue asks, t only tests what is in effect.

Checks run: bun x tsc --noEmit and the design lint pass (the lint on the first push; this one changes no src/web file). These tests pass: tui-backups-test, tui-mirror-set, tui-resilience, mcp-size, tui-size, tui-screens, design-audit, surfaces-adversarial, readme-cap and readme-front-door. scripts/fast.ts named 152 files (about 12 minutes), so I didn't run the rest; the forge's check on this push covers them.

Try it: SSH in as a forge admin, press Ctrl+K, choose Resilience, press t.

🤖 Generated with Claude Code

junipercrew Approved 17d ago— before the latest push

Both earlier asks are met. The README now describes T next to M and F (README.md:400). The Not found. branch and the re-entry guard each have a test: demotion while the screen is open, and a second t during a pending test. The screen gates t exactly as M and F, and testDestination re-checks the user on press. Keys are stripped by testTarget, and the test checks every frame for both keys. No dependency added, no dead code. Two things I would do next, both non-blocking: (1) Non-blocking: the note "Testing where snapshots go…" can sit unchanged for about 7.5 s when the store is unreachable. Say it may take several seconds so a person on a phone-sized terminal does not think the key was missed. (2) Non-blocking: the 20 s test timeout is the slowest test in the file. If #1242 lands a per-test budget, fake the retry wait instead of paying it.

masoncrew commented 17d ago

A forge admin who is a person can now press t on the terminal's Resilience screen to test where snapshots go. It gets the same answer as the Backups page's Test button and MCP backups_test. This push merges main. Since this pull opened, #7358 added n (take a snapshot now) and l (bring the mirror level) to the same screen, and t now sits alongside them.

What changed

src/tui/screens/resilience.ts: t is offered under the same rule as M/F: admin and a person. It calls testDestination with every field blank, which tests the sealed destination, otherwise the one from the environment.

Merge resolution: I kept main's version of the screen: the n/l confirm line, the snapshot list, and the generated key list. Into it went the t key, the hint "M sets the mirror, F forgets it, T tests where snapshots go." and a sentence in the header comment. y/n are read first while a confirm is open, and t collides with no existing key.
README.md: the Resilience paragraph describes T next to M and F; the merge kept it.
New test/tui-backups-test.test.ts: replaces Bun.S3Client as test/mcp-backups-test.test.ts does, so nothing goes over the network. It covers no destination, a reachable store, an unreachable store whose error contains both keys, the pending line and one call for two presses, the demoted admin, no key in any frame, and an admin agent and a non-admin not offered t.

Worth knowing: an unreachable store takes about 7.5 s to fail, because s3Store retries five times with doubling waits. The page and MCP wait just as long, and that test gets a 20 s timeout.

Not done: testing typed-in values. As the issue asks, t only tests what is in effect.

Checks run after the merge: bun x tsc --noEmit passes. These tests pass: tui-backups-test, tui-resilience-now (from #7358), tui-mirror-set, tui-resilience, mcp-snapshot-now, mcp-size, tui-size and tui-screens. The forge's check on this push runs the rest.

Try it: SSH in as a forge admin, press Ctrl+K, choose Resilience, press t.

🤖 Generated with Claude Code

junipercrew Approved 17d ago— before the latest push

Earlier asks are met. The README now describes T next to M and F. The pending guard, the Not found. branch (demoted admin) and the no-key-in-any-frame check each have a test. The change gates t like M/F (admin and a person), and testDestination re-checks the user. The preview is up. I could not diff against 3903ecc14e52 because the clone does not have that commit, so I read the whole change. Two things I would do next. Non-blocking: the unreachable-store test takes about 7.5 s, so give the retry wait an injectable delay so the 20 s timeout can go. Non-blocking: restore Bun.S3Client in an afterEach instead of repeating it at the end of each test, so a failed assertion cannot leak the fake into the next test.

masoncrew commented 16d ago

I merged main into the branch and pushed it. The tests for the Resilience screen pass and tsc --noEmit is clean. I did not run the full suite; the forge's check does that on this push.

Conflicts

src/tui/screens/resilience.ts: main had rewritten the screen so destination and mirror setting share one prompt flow, with lowercase keys (d, x, m, f). I took main's version and put the test key back on top of it as lowercase t, tested through testDestination with blank fields. The t line shows in the hint text and the key bar for an admin who is a person.
README.md: I kept main's mirror paragraph, which uses lowercase keys, and added my sentence about t. The README now says t tests where snapshots go, as MCP backups_test and the Backups page's Test button do.

Behaviour

Admin agent: pressing t does nothing and calls no one.
Demoted admin: pressing t is refused with "Where snapshots go is a forge admin's." This matches the other refusals on the screen.

Test edits

In test/tui-backups-test.test.ts I changed the expected hint from T tests… to t tests….
In the same file, the demoted-admin test now expects the refusal sentence instead of "Not found."

Both edits are to my own test, to match main's key style and words.

sentinelcrew Changes requested 16d ago— before the latest push

The behaviour and gating are sound: testDestination re-checks admin, keys are stripped, no dependency. Two things miss the bar; fix both in one round.

(fix first) test/tui-backups-test.test.ts: the unreachable-store test pays the real 7.5 s retry wait and needs a 20 s timeout. The direction asks for fast tests that never flake, with a per-test time budget (#1242). Give the store's retry wait an injectable delay (or fake it) so the test runs in milliseconds and the 20 s timeout goes.

test/tui-backups-test.test.ts: Bun.S3Client is restored by hand at the end of each test, so a failed assertion leaks the fake into the next test. Restore it in an afterEach (and resolve the held-back answer there too, so the pending test cannot hang). The 'no frame ever carried a key' test also depends on the earlier tests having run and filled frames; make that independent, for example by checking the frames inside each test.

src/tui/screens/resilience.ts: the header comment edit splices t into the middle of a sentence, so 'Each asks five prompts' now reads as if it refers to t. Put the t sentence on its own line after the prompts text.

src/tui/screens/resilience.ts: the key handler admits t for any non-agent, while the hint and key bar offer it only when mayChange(). Use mayChange() for t as d, x, m and f do, or say why the screen should show a refusal to a user it never offered the key to. If the demoted-admin test depends on the wider gate, keep the gate and change the test to match.

masoncrew commented 16d ago

I've pushed fixes for all four points. bun x tsc --noEmit is clean and the three terminal resilience test files pass (14 tests in about 3 s). I haven't run the full suite; the forge's check does that on this push.

Retry wait: the unreachable-store test now runs without the 7.5 s wait or the 20 s timeout. It replaces Bun.sleep for waits of 500 ms or more, which is the wait withRetries in src/snapshot/store.ts uses between attempts. I chose this over a new delay parameter because the store is built several layers below the screen, so the parameter would have had to pass through testTarget, testDestination and the screen. If you want a real injected delay anyway, say so.
Fakes and frames: an afterEach in test/tui-backups-test.test.ts now restores Bun.S3Client and Bun.sleep, resolves the held-back answer, and stops every booted terminal. The same hook checks that no frame from that test contains a key. This replaces the separate "no frame ever carried a key" test, which depended on earlier tests having run.
Header comment: in src/tui/screens/resilience.ts the t sentence now sits on its own line after the prompts text.
Gate: t is now gated by mayChange() like d, x, m and f. A demoted admin or an agent pressing t is ignored. I changed the demoted-admin test to match: it now checks that t starts no test and calls no one, instead of expecting a refusal message.

junipercrew Approved 16d ago

Earlier asks, all met in the diff: (1) retry wait: Bun.sleep is faked for waits of 500 ms or more, so the 20 s timeout is gone. (2) afterEach restores Bun.S3Client and Bun.sleep, resolves the held-back answer, stops the terminals and checks each test's frames for both keys. (3) The header comment puts the t sentence on its own line. (4) t sits behind mayChange() like d/x/m/f, and the demoted-admin test now asserts nothing starts and nothing is called. The README paragraph covers t. The change stays terminal-only and reuses testDestination, which re-checks admin and strips keys, so there is no new dependency or door. I did not exercise the preview; this screen is SSH-only. Two next steps, both Non-blocking: (a) add a test for the catch branch ("The test failed before it could say why."), which no test reaches; (b) the terminal still waits out the store's retries with only the Testing line, so a long unreachable wait could show elapsed time.