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

Settings says what a sized forge's cores mean for builds (Compute, both doors) #11733

Merged opened this

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

Merged

An admin of a sized child forge now sees a "Cores" row on `/settings`, and the terminal has a matching "Cores" screen with the same sentence. The new test file, the touched account, plan-row and CPU-rule tests, `tsc`, `gate.ts` and `design-lint.ts` pass. I did not run the full suite, because `fast.ts` listed 1,888 test files for this diff. I have not checked it on a sized child preview, which the…

shipped production is serving it deploy 4e6f862

opened this

An admin of a sized child forge now sees a "Cores" row on /settings, and the terminal has a matching "Cores" screen with the same sentence. The new test file, the touched account, plan-row and CPU-rule tests, tsc, gate.ts and design-lint.ts pass. I did not run the full suite, because fast.ts listed 1,888 test files for this diff. I have not checked it on a sized child preview, which the issue asks for. I also did not use keep_plan; no such tool was available in this session.

  • What it says: "1 CPU: builds run, slower." followed by how the forge is held to its cores. The sentence is written once in src/spawn/cores.ts and both doors print it.
  • How it's decided: the forge reads its own state rather than assuming. A cpu.max quota in its cgroup says the kernel slows a busy build. Allowed CPUs no more than its cores say it is pinned, which also slows it. Neither says only the watch applies. For the watch the row states its cost, with the 5-minute and 10-minute figures read from src/apps/cpu_rule.ts.
  • Where it shows: the row sits beside the Plan row via src/web/cores_row.ts. The terminal entry is src/tui/screens/cores.ts, registered in entries.ts. Only admins of a forge that is a cell child (FORGE_CELL_SIZE set) see either; others see nothing.
  • Docs and tests: docs/features/cores-row.md, and test/cores-row.test.ts covers the three hold modes, the row's gating, the terminal render and a /settings fetch with and without a size.

I left out showing the row to non-admins and any change to how cores are held or enforced.

To try it, run a sized child, sign in as its admin, open /settings, and look for the "Cores" row. In the terminal, open the palette and pick "Cores".

The push needed a Try: line (pre-push refused without one), so I added an empty commit carrying it.

Closes #11694

commented

An admin of a sized child forge now sees a "Cores" row on /settings, and the terminal has a matching "Cores" screen with the same sentence. The new test file, the touched account, plan-row and CPU-rule tests, tsc, gate.ts and design-lint.ts pass. I did not run the full suite, because fast.ts listed 1,888 test files for this diff. I have not checked it on a sized child preview, which the issue asks for. I also did not use keep_plan; no such tool was available in this session.

  • What it says: "1 CPU: builds run, slower." followed by how the forge is held to its cores. The sentence is written once in src/spawn/cores.ts and both doors print it.
  • How it's decided: the forge reads its own state rather than assuming. A cpu.max quota in its cgroup says the kernel slows a busy build. Allowed CPUs no more than its cores say it is pinned, which also slows it. Neither says only the watch applies. For the watch the row states its cost, with the 5-minute and 10-minute figures read from src/apps/cpu_rule.ts.
  • Where it shows: the row sits beside the Plan row via src/web/cores_row.ts. The terminal entry is src/tui/screens/cores.ts, registered in entries.ts. Only admins of a forge that is a cell child (FORGE_CELL_SIZE set) see either; others see nothing.
  • Docs and tests: docs/features/cores-row.md, and test/cores-row.test.ts covers the three hold modes, the row's gating, the terminal render and a /settings fetch with and without a size.

I left out showing the row to non-admins and any change to how cores are held or enforced.

To try it, run a sized child, sign in as its admin, open /settings, and look for the "Cores" row. In the terminal, open the palette and pick "Cores".

The push needed a Try: line (pre-push refused without one), so I added an empty commit carrying it.

commented

The "pipeline first" rule failed because this pull adds a feature (the Cores row on /settings and in the terminal) and changes nothing in the pipeline. I added an empty commit with a Direction: line giving the reason, and pushed it. Running bun scripts/gate.ts locally printed nothing and the push went through. The forge's check has not re-run on this push yet, so I don't know its verdict.

The Direction: line says that a sized forge's builds die at the host's CPU stop (#11661), and operators can't see that limit anywhere. The Cores row makes it visible.

Nothing else in the code changed. I did not re-run tsc or the tests, because only a commit message was added.

Changes requested — before the latest push

Works as designed and both doors share one sentence (coresLine), the how-to is in docs/features/cores-row.md, README untouched, tests cover the three holds. Changes needed:

  1. Direction: the repo is PIPELINE FIRST and this is a pure feature that touches no pipeline item. A Direction: line in an empty commit does not change that. Hold it until the pipeline list is cleared, or show the issue is itself a pipeline blocker.
  2. Preview: this preview is not a sized child (no FORGE_CELL_SIZE), so /settings shows no Cores row and the terminal does not offer the entry. Nothing shows it working. The one test that renders it sets the env by hand. Show it on a sized child, or add a test that renders the full /settings page for each of the three holds.
  3. Design (principles 3 and 6): coresRow emits its own dl.status-rows, which carries a margin and a border-top hairline, directly after the Plan row dl. That gives two stacked hairlines and gaps for one group of facts. Put the Cores row in the same dl as the Plan row, or drop the extra top border, so it reads as one list of rows.
  4. Design (principle 8): the watch sentence is long and stacks three numbers and two clauses ("stops it only after 5 minutes over 2× them, and never in its first 10 minutes"). Say it in plain words per docs/WRITING.md, for example: "Nothing limits this forge to its cores. A build that stays above twice its cores for 5 minutes is stopped; the first 10 minutes are safe."
  5. Terminal gate: coresScreen does not check the session itself, so only offered holds the admin gate. Make run (or the screen) refuse a non-admin too, as the page does, and add a test for it. Your test only checks the screen text.
  6. Clean: account.ts line is now a very long template line; compute the row above the return. The CORES_LABEL constant for one use is noise; inline it.
commented

Five of the six review points are done and pushed. Point 1, the hold until the pipeline is cleared, is not something I can settle. bun x tsc --noEmit is clean, design-lint reports nothing outside the system, and the touched tests pass. The new tests cover the quota and watch holds, not the pinned one, as in point 2 below.

1. Pipeline first. I can't show this issue is a pipeline blocker, and I added no pipeline change. The Direction: commit stays as the stated reason, and holding the pull is a call for a person.

2. Preview. The preview is still not a sized child, so /settings there shows no Cores row. I added a test that renders the full /settings page as an admin:

  • Quota: it points FORGE_CELL_CGROUP_DIR at a temp cgroup holding a cpu.max with a quota and checks the kernel sentence.
  • Watch: with no quota in cpu.max, it checks the watch sentence.
  • Pinned: it is not tested on the page, because that hold reads the real /proc/self/status and can't be faked from a test. That hold is covered by the coresOf unit test, and the page test accepts either the watch or the pinned wording.
  • Plain forge: with no size set, the row is gone.

3. Design. coresRow now emits a bare row. planRow takes it as an optional fourth argument and puts it inside its own dl, so there is one list and one hairline. When there is no Plan row, the Cores row gets its own dl.

4. Watch sentence. It now reads "Nothing limits this forge to its cores. A build that stays above twice its cores for 5 minutes is stopped; the first 10 minutes are safe." The numbers still come from cpu_rule.ts. I updated docs/features/cores-row.md to match.

5. Terminal gate. entry.run now refuses a non-admin, and coresScreen prints "Only admins see this." for one. A new test checks that run pushes nothing for a member and pushes the screen for an admin.

6. Clean. The row is computed above the return in account.ts, and CORES_LABEL is gone.

commented

The header-width check failed because two comment lines ran past 160 columns. They were the header comment in src/web/cores_row.ts and the doc comment on planRow in src/web/plan_row.ts. I wrapped both and changed no code.

test/header-width.test.ts and test/cores-row.test.ts now pass, and bun x tsc --noEmit is clean. I pushed the fix, but the forge's check has not re-run on it yet.

Approved

Approved. The earlier asks are met: the Cores row now shares the Plan row dl (one list, one hairline, src/web/plan_row.ts), the watch sentence states its cost, the terminal entry refuses non-admins and has a test, the header-width fix is in, and the how-to is in docs/features/cores-row.md, not the README. The pull is a feature while the direction says pipeline first. The builder gave a Direction: reason; a person decides the ordering. The commit 217a21e is not in my clone, so I judged the whole change rather than the delta. The preview is not a sized child, so the row cannot be seen there; the /settings test covers it.

Two things to do next:

  1. Non-blocking: the pinned (affinity) hold is not exercised on the /settings page, because coresOf reads the real /proc/self/status. Take the status path from the source object so one page test can cover it.
  2. Non-blocking: the row is a dl row with plain words and no new accent or box. The terminal screen has no scroll or keys. That is fine at this length, but wrap and clip are the only handling if the sentence grows.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs