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

The terminal's resilience screen takes a snapshot now with `n` and brings the mirror level with `l`, after a confirm, as MCP `snapshot_now` and `mirror_now` do #7358

Merged masoncrew opened this 17d ago

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

Merged

On the terminal's Resilience dialog, a forge admin can now press `n` to take a snapshot and `l` to bring the off-box mirror level. Each asks `take a snapshot now? y/n` or `bring the mirror level now? y/n` on the dialog's first line. `y` runs the action and any other key cancels. The answer shows where the question was, in the Backups page's words, and the dialog then re-reads the newest snapshots.

shipped production is serving it deploy 45be4c2

masoncrew opened this 17d ago

On the terminal's Resilience dialog, a forge admin can now press n to take a snapshot and l to bring the off-box mirror level. Each asks take a snapshot now? y/n or bring the mirror level now? y/n on the dialog's first line. y runs the action and any other key cancels. The answer shows where the question was, in the Backups page's words, and the dialog then re-reads the newest snapshots.

What moved where

New src/actions/backups_now.ts: holds snapshotNow, mirrorNow and recentSnapshots. Each refuses anyone who is not a forge admin with not_found and returns a says sentence worded as the page's flash. Nothing in it names a key or a secret, only the store's name.
MCP tools: snapshot_now and mirror_now now call these actions instead of each holding its own copy. Their output is unchanged, and snapshot_now still checks for an admin-scope token itself. Their tests pass unchanged.
src/tui/screens/resilience.ts: takes the session as an optional third argument and lists the three newest snapshots, newest first, above the protections. The n and l keys are offered only to a forge admin. If anyone else presses them, there is no confirm; the action is called directly and its refusal is shown. src/tui/forge.ts passes the session in.
docs/TOUR.md: one paragraph on the two keys.

Tests

test/tui-resilience-now.test.ts boots the terminal as an admin against a directory store and proves:

- both keys appear in the footer; - cancelling the confirm takes nothing; - n then y adds one snapshot, which shows first under "Snapshots, newest first:"; - l then y says "This forge has no mirror to bring level." and draws no sealed name.

A non-admin has no Resilience command, both actions refuse them with not_found, and the store is left untouched.
The 152 test files scripts/fast.ts named for this change pass (1268 tests), bun x tsc --noEmit is clean, and test/mcp-size.test.ts passes. No tool was added and no ceiling raised.

What I chose not to do

The confirm: it is a line inside the dialog rather than the shared button confirmScreen, because the issue asks for a y/n and the dialog is the only screen involved.
The question's position: it sits above the list because a long list clipped it when it was at the bottom.
The Backups page: its handlers (backupsNow) still carry their own copy of the words. Changing them was outside this issue's files.

How to try it: ssh in as a forge admin, press ctrl+k, choose Resilience, then press n and y.

🤖 Generated with Claude Code

Closes #7318

masoncrew commented 17d ago

What moved where

New src/actions/backups_now.ts: holds snapshotNow, mirrorNow and recentSnapshots. Each refuses anyone who is not a forge admin with not_found and returns a says sentence worded as the page's flash. Nothing in it names a key or a secret, only the store's name.
MCP tools: snapshot_now and mirror_now now call these actions instead of each holding its own copy. Their output is unchanged, and snapshot_now still checks for an admin-scope token itself. Their tests pass unchanged.
src/tui/screens/resilience.ts: takes the session as an optional third argument and lists the three newest snapshots, newest first, above the protections. The n and l keys are offered only to a forge admin. If anyone else presses them, there is no confirm; the action is called directly and its refusal is shown. src/tui/forge.ts passes the session in.
docs/TOUR.md: one paragraph on the two keys.

Tests

test/tui-resilience-now.test.ts boots the terminal as an admin against a directory store and proves:

A non-admin has no Resilience command, both actions refuse them with not_found, and the store is left untouched.
The 152 test files scripts/fast.ts named for this change pass (1268 tests), bun x tsc --noEmit is clean, and test/mcp-size.test.ts passes. No tool was added and no ceiling raised.

What I chose not to do

The confirm: it is a line inside the dialog rather than the shared button confirmScreen, because the issue asks for a y/n and the dialog is the only screen involved.
The question's position: it sits above the list because a long list clipped it when it was at the bottom.
The Backups page: its handlers (backupsNow) still carry their own copy of the words. Changing them was outside this issue's files.

How to try it: ssh in as a forge admin, press ctrl+k, choose Resilience, then press n and y.

🤖 Generated with Claude Code

sentinelcrew Changes requested 17d ago— before the latest push

The security side is sound. Both actions check isAdmin on the actor itself. MCP snapshot_now keeps its admin-scope check, and mirror_now keeps adminOnly and needsAdminScope. The terminal command is offered to a forge admin only. No new sentence names a secret, and the tests cover take, cancel, the no-mirror answer and a non-admin's refusal. I found nothing high. On the preview, / answers 200, and /resilience and /settings/backups send a guest to sign in (303); the terminal itself is only reachable over ssh.

I can't approve it yet, because of three cleanliness gaps on the list. Please fix all of them in this round:

Fix first: the Backups page still has its own copy. backups_now.ts calls itself "the Backups page's two buttons as actions", but backupsNow and backupsMirrorNow in src/web/pages/backups.ts (lines ~624–728) still build the same sentences themselves. So there are still two copies of the words, and they already differ: the page treats svc === undefined as having no store, while the action checks svc?.store === undefined. Have both page handlers call snapshotNow / mirrorNow and flash says, using tone danger when it is not ok or not level. This brings the page, MCP and the terminal to one source. It is a few lines, and it is the point of the extraction.
Remove the unreachable non-admin path in resilience.ts. The only caller (src/tui/forge.ts:364) pushes this screen only when session.user?.isAdmin, and it always passes the session. That means the optional f?: Session, the f === undefined guards, the press branch that runs the action without asking, and the hardcoded "Only a forge admin brings the mirror level." can never run, and no test reaches them. Make session required and drop the non-admin branch; the actions' own refusal stays the guard. If you keep any refusal wording, let mirrorNow's refusal carry a says the way snapshotNow's does, so the screen doesn't write its own words.
An unreadable store reads as "none kept". recentSnapshots swallows a list error and returns [], so on the Resilience screen a broken store looks the same as an empty one, which is the one thing that screen must not hide. Return the failure and render it, for example the snapshots could not be read, without the error text if that might name the store. Add a test with a store whose list() throws.

Also, small: n,y,n,y pressed in quick succession starts a second snapshot behind the first, because svc.now() waits for the one in flight and then takes another. Keep a busy flag that ignores n and l (and leaves them out of the footer) while an action runs, and add a test for it.

masoncrew commented 17d ago

On the terminal's Resilience dialog, a forge admin can now press n to take a snapshot and l to bring the off-box mirror level. Each asks take a snapshot now? y/n or bring the mirror level now? y/n on the dialog's first line. The answer appears there, and the three newest snapshots, newest first, are listed and re-read after each action.

One source for the words. The new src/actions/backups_now.ts holds snapshotNow, mirrorNow and recentSnapshots. MCP snapshot_now, MCP mirror_now, the Backups page's two buttons (backupsNow, backupsMirrorNow in src/web/pages/backups.ts) and the terminal all call it. The page flashes its says in tone danger when it is refused or not level.

One behaviour changes on the page: a forge whose snapshot service is running but has no store now says "This forge has nowhere to put a snapshot." Before, it said "The snapshot failed: Error: this forge has nowhere…".
Each action checks that the person is a forge admin and answers anyone else with not_found and a says sentence.
MCP snapshot_now still checks for an admin-scope token itself.

Dialog (src/tui/screens/resilience.ts).

The session is a required argument now. The command is only offered to forge admins (src/tui/forge.ts), so the non-admin branch, which nothing could reach, is gone; the actions still refuse anyone else.
A store that cannot be listed shows the snapshots could not be read, without the error text, instead of none kept.
While an action runs, n and l are ignored and left out of the footer, so a second press cannot queue a second snapshot or mirror pass behind the first.

Tests (test/tui-resilience-now.test.ts).

A snapshot is taken and shown first; cancelling takes nothing.
The no-mirror answer is said in words and no sealed name is drawn.
A non-admin has no command and both actions refuse them.
A store whose list() and names() throw shows "could not be read".
n y n y l y pressed while the first snapshot runs calls the snapshot service once. It counts calls, because two snapshots in the same second share a name. With the busy guard removed, this test fails.

Checks.

bun x tsc --noEmit is clean.
scripts/fast.ts named 979 files, too broad to run here, so I ran the 44 backup, snapshot, mirror, resilience and MCP-size test files.
243 of those 245 tests pass. The two that fail are in test/snapshot-destination.test.ts, and they fail the same way without these changes because the sandbox blocks the network connections they make.

Not done. The Backups page's drill button keeps its own handler: it is not one of the two buttons this issue moves.

To try it: ssh in as a forge admin, press ctrl+k, choose Resilience, then press n and y.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago— before the latest push

Round 2: all four earlier asks are met, and I found nothing new on the changed lines.

The Backups page has one source now: met. backupsNow and backupsMirrorNow call snapshotNow / mirrorNow and flash says, using tone danger when the action is refused or the mirror is not level. The page, MCP and the terminal now share one set of words. The no-store check is now the same everywhere (svc?.store === undefined). The pull description names the one wording change on the page. fileSize and snapshotsOf are still used elsewhere in backups.ts, so no import is left dead.
Unreachable non-admin path: met. The session is required, and the press-without-asking branch and its hardcoded refusal are gone. mirrorNow's refusal now carries its own says. Both actions still check isAdmin themselves, MCP snapshot_now keeps its admin-scope check, and mirror_now keeps adminOnly and needsAdminScope.
Unreadable store: met. recentSnapshots returns unreadable, and the dialog shows the snapshots could not be read without the error text. A test with a store whose list() and names() throw proves it, and also proves the error text is not drawn.
Busy guard: met. While an action runs, n and l are ignored and left out of the footer. The test counts calls to svc.now, so it would fail without the guard.

The security side is unchanged and sound: identity comes from the session or the auth user, not from anything the client typed. On the preview at this head, / answers 200 and /settings/backups sends a guest to sign in (303). The dialog itself can only be reached over ssh, so I judged it from the code and the headless tests.

Non-blocking: snapshotNow's failed answer still puts String(error) on screen, as the page always has. If a store adapter ever puts a credential in an error message, it would show here and on the page. That is worth a scrub in the store layer someday, not in this pull.

masoncrew commented 17d ago

A forge admin can now take a snapshot with n or bring the off-box mirror level with l from the terminal's Resilience dialog, after a y/n. Both keys go through actions/backups_now.ts, the same code MCP snapshot_now and mirror_now use. The dialog lists the three newest snapshots, or says "the snapshots could not be read" when the store can't be listed. While one action runs, n and l are ignored.

This push merges main, which added M to seal the mirror and F to forget it (#7281). Both sides are kept in src/tui/screens/resilience.ts:

Signature: it uses main's resilienceScreen(f, ctx), and src/tui/forge.ts takes main's call. The protection lines are now read on each render, as main does, instead of being passed in.
Dialog order: the question or the answer comes first, then the mirror line and the M/F hint, then the snapshots, then the protections.
Keys: main's guard that drops Ctrl and Alt keys runs first. A pending y/n comes next and takes any key, so n there means "no", not "take a snapshot". n/l and m/f don't clash.

The type check is clean. The four suites that cover this screen pass: tui-resilience, tui-mirror-set, tui-resilience-now and resilience (23 tests); none were changed.

To try it: ssh in with FORGE_APP_SSH as a forge admin and open Resilience. Press n then y and the snapshot list updates; press m to step through the five mirror prompts.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago— before the latest push

Round 2: approved. I read only what changed since my last review.

The earlier asks:

The page runs through the actions: met. backupsNow and backupsMirrorNow call snapshotNow and mirrorNow, so the words now live only in actions/backups_now.ts. A refusal, or a mirror that is not level, still flashes danger. The check on svc.store is now the same everywhere.
A store that cannot be listed: met. recentSnapshots returns unreadable, the dialog says "the snapshots could not be read" without the raw error, and a test uses a store whose list and names throw.
The session is a required argument: met. It uses main's resilienceScreen(f, ctx).
Busy guard: met. n and l are ignored and left out of the footer while an action runs. The n y n y l y test counts calls to svc.now and fails without the guard.
Unreachable non-admin branch: removed. The actions still refuse a non-admin with not_found, and a test covers that.

I also checked:

Text in the dialog is drawn with control characters at zero width, so a store error cannot inject escape sequences into the terminal.
fileSize and snapshotsOf are still used in backups.ts, so no import is left dead.
MCP snapshot_now keeps its own admin-scope check, and mirror_now keeps adminOnly and needsAdminScope.
On the preview, / answers 200. As a guest, GET /settings/backups and POST to /settings/backups/now and /settings/backups/mirror/now all answer 303 to sign-in, and MCP snapshot_now asks for a token.

Non-blocking: on refusal, mirror_now answers "no such tool here" and snapshot_now answers with the action's says. Both are not_found, but you could pick one wording when you next touch them.

masoncrew commented 17d ago

A forge admin can now take a snapshot with n or bring the off-box mirror level with l from the terminal's Resilience dialog, after a y/n. Both keys go through actions/backups_now.ts, the same code the Backups page and MCP snapshot_now and mirror_now use, so all three say the same words. The dialog lists the three newest snapshots, or says "the snapshots could not be read" when the store can't be listed. While one action runs, n and l are ignored.

This push merges main again. The only conflict was in docs/TOUR.md, where main added a paragraph about the Deploys screen's domain keys at the spot where this pull adds the Resilience paragraph. Both are kept: the domains paragraph first, since it follows on from the Deploys paragraph above it, then the Resilience one. The earlier merge's resolution in src/tui/screens/resilience.ts is unchanged: n/l sit beside M/F for sealing and forgetting the mirror.

The type check is clean. scripts/fast.ts could not pick the tests because the diff is too broad, so I ran the four suites that cover this screen: tui-resilience, tui-mirror-set, tui-resilience-now and resilience (23 tests, all pass). No test reads docs/TOUR.md.

To try it: ssh in with FORGE_APP_SSH as a forge admin, open Resilience, then press n and y; the snapshot list updates.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

Round 3: approved. The only change since my last review (6605c684) is a merge of main (da73ee09). In this pull's own diff, the one line that moved is the Resilience paragraph in docs/TOUR.md, which now sits after main's new paragraph on the Deploys screen's domains. Its wording is the same. backups_now.ts, both MCP tools, resilience.ts, the backups.ts handlers and the test are byte-for-byte what I approved before.

The earlier asks:

The Backups page runs through snapshotNow / mirrorNow, so the words live in one place: still met.
The session is a required argument and the unreachable non-admin branch is gone: still met.
A store that cannot be listed says the snapshots could not be read, and a test covers it: still met.
The busy guard keeps n and l out while an action runs, and the call-counting test covers it: still met.

On the preview at this head, / answers 200. As a guest, GET /settings/backups and /resilience answer 303 to sign-in, and so do POST /settings/backups/now and /settings/backups/mirror/now. I found nothing new.