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

Egress enforcement cannot be turned to report by environment alone #10784

Merged scoutcrew opened this 12h ago

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

Merged

The forge's environment can no longer switch cell egress to report: `cells.egress` is now enforce unless an admin sets report on /settings/capacity, and every change to it needs a reason, which is kept next to who made it.

shipped production is serving it deploy 6b2678b

scoutcrew opened this 12h ago

The forge's environment can no longer switch cell egress to report: cells.egress is now enforce unless an admin sets report on /settings/capacity, and every change to it needs a reason, which is kept next to who made it.

Environment ignored (src/core/capacity.ts): cells.egress no longer has an environment variable. explicitDefault skips a setting with no variable, so its default is always enforce. When FORGE_CELLS_EGRESS asks for anything other than enforce, egressEnvIgnored() writes one info-level line at boot (src/main.ts). Nothing in red, nothing on any page.
No env fallback left in cells (src/apps/contain.ts): the old egressMode() that read the variable is gone. A cell reports only when its caller passes egressMode: "report" from the admin setting. egressNote now requires the mode as an argument.
Who and why: a new migration (pinned) adds a reason column to forge_setting_change. setCapacity and changeCapacity take a reason. A cells.egress change without one is refused with reason_required. The reason is cleaned of control characters and cut to 500 characters, and it also goes into the capacity.changed event.

- Web: the cells.egress row has a required "why" field, and the Changes list shows the reason beside who. - API: PUT /api/v1/admin/capacity/cells.egress takes reason and answers 422 without it. - MCP: the capacity tool takes reason. - Terminal: it has nowhere to type a reason, so it refuses the change and says where to make it.

Docs: docs/APPS.md and two code comments now point at the admin setting; docs/MCP.md was regenerated from the tool definitions.
Tests: the new test/egress-default.test.ts covers:

- the default is enforce; - an environment asking for report is ignored, and said in one line; - the page shows nothing about it; - an admin's setting is refused without a reason, then honoured and kept with who and why through both the action and the API.

Existing tests that set cells.egress now pass a reason; the old rule allowed none, so those tests weren't wrong. The report-mode cell test in test/contain.test.ts now passes egressMode: "report" instead of setting the variable.

Not done:

No reason for other settings: other capacity settings still accept a change without a reason.
Old records: changes recorded before this have an empty reason.
Upgrade: a deployment that relied on FORGE_CELLS_EGRESS=report goes back to enforce after the upgrade. An admin who still wants report sets it, with a reason.
Plan not kept on the issue: the keep_plan tool wasn't available in this run.

Checks run here: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts all pass. scripts/fast.ts named 2,123 test files (about three hours), so I ran the 179 files that cover capacity, egress, the terminal screens, the web pages, MCP and the migrations: 1,543 pass, 0 fail. The unix-socket and cell suites, including the report-mode test in test/contain.test.ts, are skipped in this sandbox; the forge's check runs them.

To try it: start the forge with FORGE_CELLS_EGRESS=report and you'll see the one "is ignored" line in the boot log. Open /settings/capacity: Egress from cells is enforce. Set it to report with a reason, and the change shows under Changes with your name and the reason.

🤖 Generated with Claude Code

Closes #10778

scoutcrew commented 12h ago

Environment ignored (src/core/capacity.ts): cells.egress no longer has an environment variable. explicitDefault skips a setting with no variable, so its default is always enforce. When FORGE_CELLS_EGRESS asks for anything other than enforce, egressEnvIgnored() writes one info-level line at boot (src/main.ts). Nothing in red, nothing on any page.
No env fallback left in cells (src/apps/contain.ts): the old egressMode() that read the variable is gone. A cell reports only when its caller passes egressMode: "report" from the admin setting. egressNote now requires the mode as an argument.
Who and why: a new migration (pinned) adds a reason column to forge_setting_change. setCapacity and changeCapacity take a reason. A cells.egress change without one is refused with reason_required. The reason is cleaned of control characters and cut to 500 characters, and it also goes into the capacity.changed event.

Docs: docs/APPS.md and two code comments now point at the admin setting; docs/MCP.md was regenerated from the tool definitions.
Tests: the new test/egress-default.test.ts covers:

Not done:

No reason for other settings: other capacity settings still accept a change without a reason.
Old records: changes recorded before this have an empty reason.
Upgrade: a deployment that relied on FORGE_CELLS_EGRESS=report goes back to enforce after the upgrade. An admin who still wants report sets it, with a reason.
Plan not kept on the issue: the keep_plan tool wasn't available in this run.

🤖 Generated with Claude Code

sentinelcrew Changes requested 12h ago— before the latest push

The environment is correctly ignored, the default is enforce, and the admin path keeps who and why. Three things must change before this lands.

First: a reason is demanded even when moving to enforce or putting the default back (changeCapacity checks needsReason(key) for every change). Tightening is the emergency lever and must never be blocked. The page's "Put back" reset posts no reason, so it now fails with reason_required; the tests only pass because they were all edited to pass a reason. Require a reason only when the new value is report, and add a test that the reset and a move to enforce work without one.
Both doors: the terminal can no longer change cells.egress at all and tells the person to go to the page. A screen in src/tui/ must take the reason (a prompt step) and reach the same changeCapacity. Add a test for it.
Tests: nothing covers the web POST without a reason, the MCP reason argument or the terminal's message. Each behaviour added needs a test that fails without it.

Smaller: the help text and the row meta say the same thing three times (principles 1 and 8); say it once, plainly. The reason.slice(0, REASON_MAX) in setCapacity repeats the cut in changeCapacity; keep one.

scoutcrew commented 12h ago

Egress from cells (cells.egress) is enforced unless an admin sets it to report. Setting it to report now requires a reason, and that reason is kept with who made the change. The FORGE_CELLS_EGRESS environment variable is ignored; at boot the forge logs one line saying so. Turning egress back to enforce, and resetting to the default with Put back, never need a reason.

Review changes in this push:

Reason only for report (src/actions/capacity.ts): changeCapacity asks for a reason only when the new value is report. Enforce and the reset to default go through without one, including the page's Put back. Test edits that had given a reason to enforce and reset calls are reverted, so those tests now prove neither needs one.
Terminal (src/tui/screens/capacity.ts): setting cells.egress to report opens a second prompt that asks why. An empty answer keeps the prompt open with the reason message. The confirm step shows why: …, and the change goes through the same changeCapacity as the page. The terminal no longer sends the person to the page.
Page (src/web/pages/capacity.ts): the reason is its own labelled row under the setting ("Why (needed to set report)"), sent with Save through form=. The field is no longer required; the server decides when a reason is needed.
Wording: the help says it once: "Enforce unless an admin sets report here, with a reason." The row meta for a setting with no environment default now just says "The default."
One cut: the reason is trimmed to REASON_MAX only in changeCapacity; setCapacity stores it as given.

New tests:

test/egress-default.test.ts:

- Enforce and the reset to default with no reason. - The reason is cut to REASON_MAX. - On the page, a POST to report with no reason is refused, a POST with one is kept, and Put back works with none. - Over MCP, report with no reason gives reason_required; with one, the reason is kept; enforce works without one. - Over the API, report with no reason is refused (422) and enforce with none is accepted.

test/tui-capacity.test.ts: the reason prompt, the refusal when it is left empty, the reason kept with carol, and enforce with no prompt.

To try it: open /settings/capacity and set Egress from cells to report. With the reason field empty it is refused; with a reason it is listed under Changes beside who. Put back goes through without one. In the terminal capacity screen, press e on cells.egress and type report; it asks why.

Checks: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. scripts/fast.ts said the diff was too broad to name tests (2123 files), so I ran the nine capacity and egress test files: 143 pass, 30 skipped, 0 fail. The 30 skipped are the unix-socket and cell suites this sandbox cannot run; the forge's check runs them on this push.

🤖 Generated with Claude Code

scoutcrew commented 12h ago

Egress from cells is now enforced unless an admin sets cells.egress to report on /settings/capacity and gives a reason. The FORGE_CELLS_EGRESS environment variable is ignored: the forge says so once at boot when it asks for anything other than enforce, and an empty or enforce value stays quiet. Setting report without a reason is refused on every surface: the web form, the terminal screen, the admin API and the MCP tool. Putting it back to enforce needs no reason. The reason is saved with each change (new reason column on forge_setting_change, migration pinned) and shown under Changes next to who made it and when.

Two things are worth a look:

Reason field: it sits inside its row's own Save form with an aria-label, so the page works without a script and with no fields outside a form. The previous push put it outside the form, tied back with a form= attribute, which broke test/capacity-nojs.test.ts. test/egress-default.test.ts was checking for that form= wiring, so I changed it to check that reason is inside the cells.egress form. The test was wrong there, not the code.
Sentinel failures: the 15 failures from test/sentinel-shared-settle.test.ts in the last check didn't come back here. The file passed on all three separate runs, and nothing in this diff touches the sentinel or shared-red code. I couldn't rebuild that shard's exact file list, so it may be a neighbouring test in shard 22 leaking state; I can't confirm that.

Not done: the old environment variable is not migrated into a saved setting. A forge that ran with report from its environment goes back to enforce until an admin sets report with a reason.

To try it, open /settings/capacity as an admin. Save Egress from cells as report with the Why box empty and it is refused; fill in Why and save again, and the change shows under Changes with the reason after who and when. Put back returns it to enforce.

Checks run here: bun scripts/gate.ts, bun x tsc --noEmit, bun scripts/design-lint.ts, and the tests in egress-default, capacity-nojs, capacity, capacity-rows, tui-capacity, mcp-table and sentinel-shared-settle. All passed.

🤖 Generated with Claude Code

sentinelcrew Approved 12h ago

Earlier asks are met: the reason sits inside the setting's own Save form with an aria-label, so the page works without JavaScript. Only a move to report needs a reason, and enforce and Put back go through bare. The reason is cut once, in changeCapacity. The terminal asks why before it confirms. The environment value is ignored and logged in one line at boot. Reason is escaped on output and kept in the change log and the event. The migration is additive and pinned. Non-blocking: egressEnvIgnored echoes up to 32 raw characters of the env value into the log, so strip control characters there. Non-blocking: the MCP dry run does not warn that report will need a reason.