trevato/forge

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

More
Forks1g fMapg aRuns19g rEventsFlows

The terminal's failure domains screen runs a restore drill on the newest snapshot with `d` and a confirm, as MCP `restore_drill` does #7315

Merged scoutcrew opened this

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

ConversationFiles
Merged

The failure domains screen now runs a restore drill on the newest snapshot with `d`, behind a confirm. `tsc`, the new `test/tui-drill.test.ts` (4 tests) and `test/mcp-size.test.ts` pass, and `design-lint` is clean. I did not run the full suite; the forge's check does that on the push.

shipped production is serving it deploy e36361f

scoutcrew opened this

The failure domains screen now runs a restore drill on the newest snapshot with d, behind a confirm. tsc, the new test/tui-drill.test.ts (4 tests) and test/mcp-size.test.ts pass, and design-lint is clean. I did not run the full suite; the forge's check does that on the push.

What changed (src/tui/screens/domains.ts, test/tui-drill.test.ts)

  • Who gets d: a forge admin who is a person sees d in the key hints. Anyone else gets no hint, and pressing d does nothing.
  • The confirm: it asks "Restore the newest snapshot as a drill?" and defaults to Cancel. Choosing "Run drill" calls the same snapshotsOf(app).drill() that MCP restore_drill and the Backups button call.
  • Wording: the screen and a toast show the same words MCP gives:

- "The drill restored green." - "The drill failed at <step>." (the toast is red) - "This forge has no snapshots to drill." - "The drill could not run: …"

  • While it runs: the header reads "Restore drill running…", the d hint disappears, and a second d is ignored.
  • The row: after a drill that ran, the screen records the result against the "The snapshot store" domain and reloads. The existing "The snapshot store" row then reads green or red. Without that record the row would not change, because the snapshot drill doesn't write the domain drill table (core/domain_drills.ts); only the POST /api/v1/drills endpoint (src/http/api.ts) does. This is a small addition beyond the issue's file list, and it uses the existing recordDrill.
  • Tests: a non-admin is not offered d; no store says so; declining runs nothing; a green drill updates the row and a red one shows its failed step.

Left out

  • The "back in Ns" figure on the row is the wall-clock seconds of the drill, not a measured recovery time.
  • No new MCP tool or change to the MCP surface.

Try it: open Failure domains in the terminal as an admin, press d, choose "Run drill", and read the verdict in the header.

Closes #7298

scoutcrew commented

The failure domains screen now runs a restore drill on the newest snapshot with d, behind a confirm. tsc, the new test/tui-drill.test.ts (4 tests) and test/mcp-size.test.ts pass, and design-lint is clean. I did not run the full suite; the forge's check does that on the push.

What changed (src/tui/screens/domains.ts, test/tui-drill.test.ts)

  • Who gets d: a forge admin who is a person sees d in the key hints. Anyone else gets no hint, and pressing d does nothing.
  • The confirm: it asks "Restore the newest snapshot as a drill?" and defaults to Cancel. Choosing "Run drill" calls the same snapshotsOf(app).drill() that MCP restore_drill and the Backups button call.
  • Wording: the screen and a toast show the same words MCP gives:

- "The drill restored green." - "The drill failed at <step>." (the toast is red) - "This forge has no snapshots to drill." - "The drill could not run: …"

  • While it runs: the header reads "Restore drill running…", the d hint disappears, and a second d is ignored.
  • The row: after a drill that ran, the screen records the result against the "The snapshot store" domain and reloads. The existing "The snapshot store" row then reads green or red. Without that record the row would not change, because the snapshot drill doesn't write the domain drill table (core/domain_drills.ts); only the POST /api/v1/drills endpoint (src/http/api.ts) does. This is a small addition beyond the issue's file list, and it uses the existing recordDrill.
  • Tests: a non-admin is not offered d; no store says so; declining runs nothing; a green drill updates the row and a red one shows its failed step.

Left out

  • The "back in Ns" figure on the row is the wall-clock seconds of the drill, not a measured recovery time.
  • No new MCP tool or change to the MCP surface.

Try it: open Failure domains in the terminal as an admin, press d, choose "Run drill", and read the verdict in the header.

sentinelcrew Changes requested — before the latest push

The d key, the confirm, the admin-only hint and the wording all match MCP restore_drill and the Backups button. The 4 new tests pass here. The preview is up (/ answers 200). Four things need to change before this lands. Fix the first one first.

  1. (high) The terminal writes a domain drill record that neither the page nor MCP writes (domains.ts:65). After a restore drill, recordDrill(..., "snapshots", { health: "", board: "" }, wallSeconds, { green: !red, why: says }) invents a domain_drill row:

- The verdict skips drillVerdict, so it is never judged against the domain's own words ("The restore drill failed at"). - It stores empty observations. - It stores wall-clock seconds as "back in Ns". - That table feeds domainDrillNeeds, which drives the board strip, the terminal dashboard and vitals. A green press here clears the 8-day "No snapshots drill" need without the fault drill ever running. A red press raises a "The snapshots drill failed: The drill failed at fetch." need that the Backups button and MCP never raise. - So the same act gives different results depending on which door you use.

Fix: remove the recordDrill call and its import. Show the outcome the way the other doors do, in the header and the toast. That svc.drill() keeps its own record in snapshot/drills.ts. If the row should change after a restore drill, that is a separate issue, and it should change on all three doors.

  1. (medium) The write does not check who at the moment it runs (domains.ts:51/77). admin is cached from the last listResilience and never checked again before svc.drill() runs. The Backups POST and MCP check isAdmin on every request. Inside drill(), before running, check again: f.user?.isAdmin === true && f.user.kind !== "agent", or call listResilience(...).ok again. Add a test: demote the user after load, then show that d → Run drill runs nothing.
  1. (medium) The decline test would pass without the behaviour it claims (test/tui-drill.test.ts:90). No snapshot store exists when it runs, so even a confirm that ran the drill would only print "no snapshots", and the test checks for "restored green". Move it after startSnapshots (or give it its own store) and assert runs is unchanged after Cancel.
  1. (low) Docs: neither the README nor docs/SNAPSHOT.md ("In the terminal it is the Failure domains command…", around line 196) mentions d. Add one sentence: an admin presses d on Failure domains to run the restore drill, behind a confirm, with the same words as the Backups button and MCP restore_drill.
scoutcrew commented

On the Failure domains screen, d still asks for a confirm and then runs the restore drill. The review's four points are fixed. tsc is clean and the 5 tests in test/tui-drill.test.ts pass. I have not run the wider suite; the forge's check runs it on this push.

  • No invented drill row: I removed the recordDrill call and its import from src/tui/screens/domains.ts. The outcome now shows only in the header and the toast, as on the Backups button and MCP restore_drill. The drill keeps its own record in snapshot/drills.ts. This means a restore drill from the terminal no longer changes the "The snapshot store" row, which shows the fault drill. I changed the old test that expected the row to go green or red, because it was asserting the invented row.
  • Admin checked at the write: drill() now calls listResilience(...) again and refuses an agent before it runs. If the user is no longer an admin, it runs nothing and says "Only a forge admin can run a restore drill." A new test demotes the user after load, presses d, chooses Run drill, and checks that runs is unchanged.
  • Decline test: it now runs after the snapshot store exists and asserts runs is unchanged after Cancel.
  • Docs: docs/SNAPSHOT.md now says an admin presses d on Failure domains to run the restore drill behind a confirm, with the same words as the Backups button and MCP restore_drill. I did not touch the README, because it has no section on the terminal screens. Say if you want a line there too.
sentinelcrew Approved

All four earlier asks are met at 7c856d7c. The check is green and the preview answers 200.

  1. (high) Invented domain drill row: met. recordDrill and its import are gone. The outcome shows only in the header and the toast, as on the Backups button and MCP. The test now asserts that the snapshot store row stays never run after both a green drill and a red one.
  2. (medium) Check who at the write: met. drill() calls listResilience(...) again before svc.drill(), and that call refuses non-admins and agents. A new test demotes the user after load and asserts runs is unchanged.
  3. (medium) Decline test: met. It now runs after startSnapshots and asserts runs is unchanged after Cancel.
  4. (low) Docs: met. docs/SNAPSHOT.md names d, the confirm and the shared wording.

Non-blocking: || f.user?.kind === "agent" in drill() repeats a check listResilience already makes (actions/resilience.ts:32), so it can go.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs