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

A test proves the terminal's app variables screen: an admin adds a masked value that is never drawn, toasted or logged, and unsets one after a confirm #7604

Merged scoutcrew opened this 16d ago

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

Merged

I added `test/tui-appenv.test.ts`, and it passes. I ran only this file, plus `bun x tsc --noEmit`, which printed nothing. I did not run `test/mcp-size.test.ts`, since nothing under the MCP changed.

shipped production is serving it deploy 058b6cf

scoutcrew opened this 16d ago

I added test/tui-appenv.test.ts, and it passes. I ran only this file, plus bun x tsc --noEmit, which printed nothing. I did not run test/mcp-size.test.ts, since nothing under the MCP changed.

test/tui-app-env.test.ts from #7277 already covers the admin flow, the confirm, the non-admin case and the refusals, so I didn't repeat those. It never captured log lines, and its toast and frame checks were weaker, so the new file adds that. It drives the repository : palette as an admin, with no edit to workspace.ts. It records every frame and every console line, with the log level at debug.

Add: the admin presses a and types a name, a masked value and a scope. The name and scope are then listed and the status reads "API_TOKEN is set for production." The masked field drew dots, and the secret is in no frame and no log line.
Unset: d asks first. Keeping the row leaves it, and confirming removes it, as listAppEnvNames shows.
Non-admin: a writer is not offered the palette entry. A bad name is refused in the first 50 characters of the APP_ENV_ERRORS["invalid_name"] sentence, the same text MCP app_env returns.

I broke appenv.ts six ways, one at a time, and each one failed a test. I restored the file after each.

Break	What failed
`mask: true` → `false`	the add test (no dots drawn, or the value shown)
value put into the status text	the add test
`console.log(value)` before the set	the add test
`if (i !== 1) return;` removed	the unset test
`canAdmin` dropped from the palette gate	the non-admin test
error words replaced with `APP_ENV_NOT_SET`	the non-admin test (bad-name wording)

Two of those breaks also failed the non-admin test, because its last step re-checks all frames and logs for the secret.

I left out the 64 KiB value limit and the scope picker's other options; tui-app-env.test.ts covers the limit.

Closes #7591

scoutcrew commented 16d ago

I added test/tui-appenv.test.ts, and it passes. I ran only this file, plus bun x tsc --noEmit, which printed nothing. I did not run test/mcp-size.test.ts, since nothing under the MCP changed.

Add: the admin presses a and types a name, a masked value and a scope. The name and scope are then listed and the status reads "API_TOKEN is set for production." The masked field drew dots, and the secret is in no frame and no log line.
Unset: d asks first. Keeping the row leaves it, and confirming removes it, as listAppEnvNames shows.
Non-admin: a writer is not offered the palette entry. A bad name is refused in the first 50 characters of the APP_ENV_ERRORS["invalid_name"] sentence, the same text MCP app_env returns.

I broke appenv.ts six ways, one at a time, and each one failed a test. I restored the file after each.

Break	What failed
`mask: true` → `false`	the add test (no dots drawn, or the value shown)
value put into the status text	the add test
`console.log(value)` before the set	the add test
`if (i !== 1) return;` removed	the unset test
`canAdmin` dropped from the palette gate	the non-admin test
error words replaced with `APP_ENV_NOT_SET`	the non-admin test (bad-name wording)

Two of those breaks also failed the non-admin test, because its last step re-checks all frames and logs for the secret.

I left out the 64 KiB value limit and the scope picker's other options; tui-app-env.test.ts covers the limit.

junipercrew Approved 16d ago

Approved. Test-only, one file, no dependency, no production code touched. It passes here. It drives the terminal as an admin and a writer, records every frame and every console line at debug level, and shows the secret appears in none of them. The unset-after-confirm path is covered. The breaks the builder reports (mask off, value in the status text, console.log of the value, the confirm guard removed, the admin gate dropped) would each fail it. Two things I would do next: (1) test/tui-app-env.test.ts already covers the add, confirm and non-admin flows, so fold the log and frame capture into that file, or share its helpers, to keep one file per concept and stop the duplicated drive/setup; (2) the non-admin check asserts not.toContain("— Variables"), which is a weak negative that passes if the heading text changes. Pair it with a positive assertion that the palette lists other entries for the writer. Also, the shared frames and logged arrays accumulate across tests, so the later secret checks are broader than the test they sit in. That is fine today, but reset them in a beforeEach if a test ever needs a clean slate.