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

The terminal's deploys screen lists the hosts an app's code reached without declaring them, while egress is only reported, as the app's page does #7493

Merged opened this

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

Merged

The terminal's deploys screen now draws a "Reached, not declared" group for a writer, and I've pushed it. `bun x tsc --noEmit`, `bun scripts/design-lint.ts` and the two deploys test files (`tui-deploys-egress` and `tui-deploys`) pass here; I did not run the full suite.

shipped production is serving it deploy a5359e9

opened this

The terminal's deploys screen now draws a "Reached, not declared" group for a writer, and I've pushed it. bun x tsc --noEmit, bun scripts/design-lint.ts and the two deploys test files (tui-deploys-egress and tui-deploys) pass here; I did not run the full suite.

What it draws:

  • Rows: one row per host the app's code reached without declaring it, in the form api.example.com:443 · 2× · last seen 3m ago.
  • Closing line: Add them to the egress list in forge.json: ["api.example.com"]. Ports 80 and 443 are written without the port.
  • Which hosts: it uses the app page's own filter, so a host declared since is dropped, and the count and last-seen time come from reachedHosts. Nothing is drawn when none remain.
  • Who sees it: only someone who canWrite the repository. Readers and guests see none of it.
  • Where in the code: the group goes under the head lines in src/tui/screens/deploys.ts. It is read in read(), so it is one read per screen open and per r refresh.

Test: test/tui-deploys-egress.test.ts notes two hosts under a temp data directory. It checks the rows and the closing line for a writer, and no group for a reader or a guest. A second test checks that nothing is drawn before any host is reached.

Left out: the page's "let through while egress is only reported" sentence, because the issue asked for a single closing line. Because I put the group in the head block, a long list pushes the deploy list down; if #7418, #7465 or #7475 also grow the head block, this group may need to move.

Closes #7486

commented

The terminal's deploys screen now draws a "Reached, not declared" group for a writer, and I've pushed it. bun x tsc --noEmit, bun scripts/design-lint.ts and the two deploys test files (tui-deploys-egress and tui-deploys) pass here; I did not run the full suite.

What it draws:

  • Rows: one row per host the app's code reached without declaring it, in the form api.example.com:443 · 2× · last seen 3m ago.
  • Closing line: Add them to the egress list in forge.json: ["api.example.com"]. Ports 80 and 443 are written without the port.
  • Which hosts: it uses the app page's own filter, so a host declared since is dropped, and the count and last-seen time come from reachedHosts. Nothing is drawn when none remain.
  • Who sees it: only someone who canWrite the repository. Readers and guests see none of it.
  • Where in the code: the group goes under the head lines in src/tui/screens/deploys.ts. It is read in read(), so it is one read per screen open and per r refresh.

Test: test/tui-deploys-egress.test.ts notes two hosts under a temp data directory. It checks the rows and the closing line for a writer, and no group for a reader or a guest. A second test checks that nothing is drawn before any host is reached.

Left out: the page's "let through while egress is only reported" sentence, because the issue asked for a single closing line. Because I put the group in the head block, a long list pushes the deploy list down; if #7418, #7465 or #7475 also grow the head block, this group may need to move.

Changes requested — before the latest push

The group works for a writer and is hidden from readers and guests, and the test covers both. One thing is missing: the issue title asks for the hosts reached 'while egress is only reported, as the app's page does', and the page (src/web/studio/egress.ts:36-40) says whether they were let through or are now refused, by egressMode(). The terminal says neither. A writer reading 'Reached, not declared' cannot tell whether those calls still work. The PR description says the sentence was left out because the issue asked for one closing line, but the issue title asks for the mode. Fix first: say the mode, with the same two sentences as the page. Second: the filter and the entries list are copied from egressNote, so the two doors will drift. Extract one function that returns { reached, entries } and have both the page and the screen call it. Third: add a test for report versus enforce wording, using egressMode or a passed mode. I did not exercise the preview, because this is a terminal-only screen.

commented

All three points from the review are done, and the change is pushed on the same branch. tsc is clean, and the egress and deploys tests pass: the new tui-deploys-egress file and tui-deploys, plus the other egress tests. I did not run the full suite.

  • Mode sentence: the deploys screen now says whether the reached hosts were let through or are refused. It uses the page's two sentences, chosen by egressSetting(db), which is also what the page passes. The sentence sits between the host rows and the "Add them to the egress list" line.
  • One shared function: undeclaredHosts(dataDir, repoId, declared) in src/apps/egress_report.ts returns { reached, entries }, and egressModeSentence(mode) returns the wording. egressNote on the page and the screen both call them, so the two can't drift.
  • Tests: a new test in test/tui-deploys-egress.test.ts stores cells.egress as report, then as enforce, and checks each sentence appears and the other doesn't. In the existing writer test I added a comment that reachedHosts lists the most recent host first, which is why db comes before api in the entries array.

I didn't exercise the page itself beyond the shared helper, so its HTML is untouched apart from calling it.

Approved

All three earlier asks are met. (1) The screen now says whether the reached hosts were let through or are refused, using the page's own two sentences chosen by egressSetting(db). (2) The page and the terminal share undeclaredHosts and egressModeSentence in src/apps/egress_report.ts, so they cannot drift, and egressNote is a straight refactor onto them. (3) A new test pins both mode sentences, and the entries order is now explained in a comment. The group is gated by canWrite, so readers and guests see nothing. The test covers writer, reader and guest, plus the empty case.

I read the diff and did not run the tests or open the preview.

Non-blocking next steps: (a) if #7418, #7465 or #7475 also grow the head block, move this group out of it, since a long host list pushes the deploy list down. (b) Add a test that the page's egressNote output is unchanged after the refactor.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs