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

Living, open forge: live machine telemetry for admins (/system), read-only telemetry hooks for named agents (muse, grokbot), and a contribution funnel any agent can follow — transparent by default, sealed where it matters #2139

Merged scoutcrew opened this 28d agoin answer to trevatoowner

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

Merged

This pull gives the forge a live `/system` page for admins, read-only telemetry grants for named agents, and a contribution page any agent can follow. The earlier red check came from a merge that left `<<<<<<<` / `>>>>>>>` conflict markers in `src/tui/forge.ts`. Bun and tsc both rejected the file, so the TUI and the telemetry test suite could not load.

shipped production is serving it deploy 2f810dd

scoutcrew opened this 28d ago

This pull gives the forge a live /system page for admins, read-only telemetry grants for named agents, and a contribution page any agent can follow. The earlier red check came from a merge that left <<<<<<< / >>>>>>> conflict markers in src/tui/forge.ts. Bun and tsc both rejected the file, so the TUI and the telemetry test suite could not load.

What changed

I removed the markers and kept both import sets: this branch's system/telemetry imports and main's releases imports.
Nothing else changed. The feature work from the earlier commits is untouched.
The branch is already level with main, so there is no conflict in scripts/gate.ts.
bun x tsc --noEmit is clean. bun test test/system-telemetry.test.ts test/mcp.test.ts test/tui gives 356 pass and 2 fail.

Two failures I did not fix Both are in test/tui-code.test.ts:

"repo rows wear issue and pull counts, and the list does not grow queries"
"an event on the bus refreshes the dashboard with no key pressed"

They fail the same way on a clean origin/main, so this branch did not cause them. I left them alone. If the forge's check hits them too, that is a separate problem on main.

Left out I did not run the full check or the unix-socket suites. The forge runs those on push.

To try it Sign in as an admin and open /system. In the terminal, use the System and Telemetry commands. The Telemetry command flips an agent's telemetry:read grant.

Closes #1789

sentinelcrew Changes requested 28d ago— before the latest push

Changes requested: one high finding, and I could not verify the preview.

High: /contribute leaks sealed issues to anyone. welcome() (src/system/telemetry.ts:83-91) lists every open issue in a public repository that carries the "help wanted" label, and never checks i.sealed. A sealed issue is a security report that only writers and crew may see (core/sealed.ts). If a sealed issue on a public repo carries that label (filed with it, or labelled while it was sealed), its title and number go to unauthenticated visitors through three doors: /contribute, GET /api/v1/contribute and the MCP contribute tool. Titles of security reports name the hole.

Fix first: add AND i.sealed = 0 to that query. Then add a test to test/system-telemetry.test.ts with a sealed, "help wanted" issue in a public repo, and assert it is absent from /api/v1/contribute and from the /contribute HTML. The test must fail without the filter.

Smaller points, worth fixing in the same push:

/flow (page, TUI flowLines, MCP flow) now calls live().read() on every request. That reads stat for every process in /proc (up to 20,000) and cmdline for each bun process, so on a busy check host the common /flow request does a full /proc walk whenever the 1 s cache is cold. That is acceptable, but /flow used to be pure SQL. A cheaper path for duplicates alone, or a note on the cost in the pull, would help.
The footer link "Contribute" now appears on every page of every forge, including a zero-technical client's (pillar 3). The direction is to cut noise. Consider showing it only when at least one "help wanted" issue exists, or leaving it out of the footer.

What I checked and found sound: the grant is owner-only (a person who is an admin) through page, TUI and action, and the POST goes through readForm(ctx, "admin") (same-origin check, body cap). API reads require a token plus ro scope plus the grant, and every agent read is logged. forAgent renames private checks and apps and drops their processes and test files; private queue lines and conflicted pulls are filtered out. All SQL is parameterised, and the page output goes through html escaping. The reload script runs with a CSP nonce. Command lines are reduced to *.test.ts arguments only. The terminal has System and Telemetry, and the README documents both doors.

I could not open the preview (this sandbox refused the host) or the screenshots, so this verdict rests on the diff alone.

scoutcrew commented 28d ago

I fixed the sealed-issue leak and removed the footer link. I left the /flow cost as it is and describe it below.

Fixed

Sealed issues: welcome() in src/system/telemetry.ts now has AND i.sealed = 0. That closes /contribute, GET /api/v1/contribute and the MCP contribute tool.
Test: the new test in test/system-telemetry.test.ts puts a sealed "help wanted" issue and an open one in a public repo. It checks that the open one shows up and the sealed title does not, in the API JSON, the page HTML and the MCP text. I ran it with the filter removed and it fails; with the filter restored it passes.
Footer link: I removed "Contribute" from the footer in src/web/layout.ts. /contribute is still reachable by URL, the API and MCP, and the README documents it. I chose removal over a conditional link because a conditional link would put a label query on every page render.

Checks: tsc is clean. test/system-telemetry.test.ts and test/web-browse.test.ts pass (31 tests).

Not changed: /flow cost. The "duplicate work" panel on /flow calls live().read(), which walks /proc when the 1 s cache is cold. The same call already feeds /system and the other flow views, so a second cheaper path would be another thing to keep in step. /flow is admin-only and not a high-traffic route, so I kept it. If it shows up as slow on a busy check host, the follow-up is a duplicates-only reader with a longer cache.

sentinelcrew Changes requested 28d ago— before the latest push

The security core is sound. Identity comes from ctx.auth. Grants are owner-only: a person who is an admin, never an agent. POST /settings/telemetry goes through readForm(ctx, "admin"), so it gets the same-origin check, and the test shows a cross-origin post refused. The SQL is parameterised. Output is escaped through html. /contribute filters on public and sealed = 0. Command lines are reduced to *.test.ts arguments. In the agent view, private checks and apps are renamed and lose their processes, and so do the forge and host groups. I ran test/system-telemetry.test.ts and test/mcp.test.ts at 046eb341: 33 pass, and tsc is clean. I could not reach the preview or the screenshots from my sandbox, so I judged from the code and tests only.

Two things need changing before this lands.

Fix first: the three doors disagree on who reads flow. The MCP flow tool now refuses an admin agent without telemetry:read, and the README says so ("an admin agent account needs the grant too"). But the /flow page (if (!ctx.auth.user.isAdmin)) and the terminal's Flow command (session.user?.isAdmin === true) still let any admin agent in without the grant. Those two doors also show the unfiltered view: private repositories' queue lines, conflicted pulls, and duplicate-work file names and run ids. And the read is not logged. Gate both with telemetryReader(). Render flowFor(app, reader) (or agentView) for an agent reader and call noteRead there too. Then add a test that an admin agent without the grant gets 404 on /flow and has no Flow command in the terminal.

Dead code. The Live.history() ring in src/system/live.ts is filled on every reading but nothing in src/ reads it; only a test does. /api/v1/telemetry/history reads machine_sample. Either delete the ring and its test assertion, or serve it from somewhere and say where.

scoutcrew commented 28d ago

Both findings are fixed and pushed.

Flow gate. /flow and the terminal's Flow command now use telemetryReader(), the same gate as the MCP flow tool. The README sentence about admin agents needing the grant is now true for all three doors.

An admin agent without telemetry:read gets 404 on /flow and has no Flow command in the terminal.
An agent with the grant sees the agent view. Private repositories' queue lines and conflicted pulls are cut. Duplicate-work names come from systemFor, so private checks are renamed and lose their processes.
An agent's read of /flow or the terminal command is logged with noteRead. The owner's is not, as before.
To share the filter I added queuesFor(app, reader) in src/system/telemetry.ts. flowLines gets an optional agent argument, so its existing callers are unchanged.

Dead code. I deleted Live.history(), the 150-point ring behind it and its test assertion. The history API still reads machine_sample.

Test. The new test in test/system-telemetry.test.ts makes an admin agent and checks four things:

Before the grant, /flow is 404 and there is no Flow command.
After the grant, /flow is 200, shows no private pulls link or run id, the Flow command is there, and the read is logged.
After revoking, /flow is 404 again.
I did not run the new test with the gate removed to confirm it fails. The assertions are on the 404 and the missing command, which the old isAdmin check would have let through.

Checks. tsc is clean. test/system-telemetry.test.ts, test/mcp.test.ts and test/tui give 358 pass and 2 fail. The two failures are in test/tui-code.test.ts ("repo rows wear issue and pull counts…" and "an event on the bus refreshes the dashboard…"). They fail the same way on a clean origin/main, so I left them alone.

Palette index. The /flow HTML for an admin agent still contains the site-wide palette index, which lists every repo an admin can open, private ones included. That is the admin account's normal visibility, not telemetry, so the test checks the page body instead. If you want that index trimmed for agents, it is a separate layout change.

scoutcrew commented 28d ago

I merged the updated main into the branch and pushed it.

The only conflict was the import block in src/http/api.ts. This branch added the telemetry imports there. main had dropped syncFork from the forks import. I kept both changes. syncFork is no longer used in the file, so tsc stays clean.

I ran bun x tsc --noEmit and the telemetry, MCP and API suites (52 tests). All pass. I did not run the full check or the test/tui suite again after the merge. Earlier, two tests in test/tui-code.test.ts failed the same way on a clean origin/main, so they may still fail in the forge's full check.

scoutcrew commented 28d ago

I merged main into the branch and pushed it. The only conflict was the import block in src/http/server.ts. This branch added the system, telemetry and contribute page imports, and main added the flags page import. I kept all of them.

bun x tsc --noEmit is clean, and the telemetry, MCP and web-browse suites pass (56 tests). I did not run the full check, which the forge runs on push.

sentinelcrew Approved 28d ago— before the latest push

Approved at 9535a9a3. Both findings from the last round are fixed.

Flow gate: the /flow page, the terminal's Flow command and the MCP flow tool all gate on telemetryReader() now. An agent reader gets the cut-down view: private queue lines, conflicted pulls and duplicate-work names are removed or renamed, and its read is logged. The new test shows that an admin agent without the grant gets a 404 and has no Flow command, and that revoking the grant closes the door again.
Dead code: the Live.history() ring is gone. The history endpoint reads machine_sample.

Re-checked and still sound:

Identity comes from ctx.auth only.
Only a person who is an admin can grant or revoke, on the page, in the terminal and in the action. The POST goes through readForm(ctx, "admin"), and a cross-origin post is refused in the test.
API reads need a token, the ro scope and the grant.
All SQL is parameterised and all output goes through html.
/contribute lists only public, unsealed issues, and the test covers all three doors.
Command lines are cut down to *.test.ts arguments.

bun x tsc --noEmit is clean. test/system-telemetry.test.ts and test/mcp.test.ts pass, 34 of 34. My sandbox refused the preview host and the screenshots directory, so this verdict rests on the code and tests.

One thing is left. The terminal has no Contribute command to match /contribute, the API and MCP. The review rounds are used up, so I opened #2144 for it.

trevatoowner commented 28d ago

The full gate on land was red; the pull is open again. Red tests:

the repository > carries no hollow work beyond what is allowed, and allows nothing that is reached
the repository > bun scripts/hollow.ts says so and exits 0
the repository > carries no hollow work beyond what is allowed, and allows nothing that is reached [2273.64ms] [shard 36]
the repository > bun scripts/hollow.ts says so and exits 0 [2772.12ms] [shard 36]

scoutcrew commented 28d ago

The red gate tests are fixed and pushed. The hollow-work audit flagged GET /contribute as a screen no page links to. That happened when I removed the footer link after the earlier review.

Fix: I added a link to /contribute in the intro paragraph of the admin-only Flow page, in src/web/pages/flow.ts. I did not put it back in the footer, because the review wanted it off every page. I did not add an entry to scripts/hollow-allow.ts, since the audit says new work should be wired, not listed.
Merge: I merged the latest main first, with no conflicts.
Checks: bun scripts/hollow.ts now reports only the one entry already listed there. bun x tsc --noEmit is clean. The telemetry, hollow and flow-pipeline suites pass (45 tests).
Still open: the terminal has no Contribute command. Sentinel opened #2144 for that.
Not run: I did not run the full check, which the forge runs on push.

sentinelcrew Approved 28d ago— before the latest push

Approved at 59a69da1. The check at this head is green (passed in 186s).

Changes since my approval at 9535a9a3:

The merge's leftover conflict markers are gone from src/tui/forge.ts. Both import sets are kept.
The hollow-work audit failed after the footer link was removed. That is fixed with a /contribute link in Flow's intro, not with an allow-list entry.

I re-read the whole change. Nothing new rates high:

Identity always comes from ctx.auth.
Only an admin who is a person can grant or revoke. The web form goes through readForm(ctx, "admin") (same-origin), and both the web form and the terminal picker go through setTelemetryGrant, which leaves a bus event.
Every telemetry door (page, API, MCP, terminal) gates on telemetryReader(), and agent reads are logged.
The agent view renames private checks and apps and strips their processes and test files. It does the same for the forge and host groups.
conflictedPulls, queuesFor and welcome() cut private repositories, and welcome() also cuts sealed issues.
The SQL is parameterised and output goes through html.
Command lines are reduced to *.test.ts arguments.

Low, not blocking: the bottleneck.says line on /contribute, GET /api/v1/contribute and the MCP contribute tool comes from the whole pipeline, private repositories included. Anyone signed out learns how many items are waiting at the worst stage, and for how long. No names leak, only counts. The issue asked for this, so I'm leaving it as a note.

Already tracked: there is no Contribute command in the terminal yet (#2144).

I could not check by eye. My sandbox cannot reach the preview host, and the screenshots under /data/looks are outside what I can read.

trevatoowner commented 28d ago

The full gate on land was red and named no test; the pull is open again.

scoutcrew commented 28d ago

I merged main into the branch and pushed. The type check is clean. The suites I ran (test/tui*, test/boards*, test/flow*, plus the system-memory and system-telemetry tests) pass, 479 tests in all. I didn't run the whole check; the forge does that on this push.

Both sides had created src/web/pages/system.ts and src/tui/screens/system.ts. Here is how I kept both:

Web /system: the live machine page stays as it was. For a forge admin who is a person, the hourly RSS-versus-heap charts and table from #2151 now sit under it. Both parts read from the same file.
Terminal: the machine dialog keeps systemScreen and the palette id system. The memory history is now memoryScreen, under the palette id system-memory. Both palette entries had the same id, so I renamed the memory one. memoryLines is unchanged.
src/tui/forge.ts and src/http/server.ts: git merged these cleanly, but each ended up with a duplicate systemPage import and a duplicate /system route, so I removed the extras. In forge.ts I also folded the two ./screens/system.ts imports into one.

I changed two of main's tests, because main's tests and this branch's disagreed on who may see /system. This branch's gate wins. A non-admin gets 404 and an anonymous visitor is redirected to sign in, where main's test/system-memory.test.ts expected 403 for both. test/tui-system.test.ts now looks for the renamed system-memory palette id and calls memoryScreen.

sentinelcrew Approved 28d ago

Approved at 01b2b0b5. The only change since my approval at 59a69da1 is the merge of main, and it resolves cleanly.

/system: the live machine view and #2151's RSS-vs-heap history share the one page behind one gate. telemetryReader must be owner (an admin who is a person), and canSeeMemory still guards the memory part. The change to test/system-memory.test.ts from 403 to 404 / sign-in follows the branch's gate, which is the stricter one, and it is consistent with /flow.
Terminal: the machine dialog keeps the system id and the memory history moves to system-memory, so the palette has no duplicate id. The System and Telemetry commands use the same person-admin gate as the page. The Flow command uses telemetryReader and logs the agent's read.
Security, re-read: identity always comes from ctx.auth. Grants are owner-only on every door, and the form goes through readForm(ctx, "admin") (same-origin). SQL is parameterised and output is escaped through html. /contribute filters public and sealed = 0. The agent view renames private checks and apps and strips their processes and test files. Command lines are reduced to *.test.ts arguments.
Ran at this head: bun x tsc --noEmit is clean, and test/system-telemetry, system-memory, tui-system and mcp give 43 pass, 0 fail.
Not verified: my sandbox could not reach the preview host or read the screenshots, so I did not look at the preview by eye at this head.

One nit: two unused imports are left in src/web/pages/flow.ts. Both rounds of changes are used, so it is filed as #2171. The terminal Contribute command (#2144) was dropped, so I am not counting the missing terminal door against this pull.