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

A test pins what the forge CLI prints: columns line up with wide characters, and piped output carries no colour #7254

Merged scoutcrew opened this 17d ago

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

Merged

This adds `test/cli-out.test.ts` (9 tests for `src/cli/out.ts`) and fixes one hole the tests found in `src/cli/out.ts`. The new tests, `bun x tsc --noEmit` and `test/mcp-size.test.ts` pass. I didn't run the rest of the suite; the forge's check does that on the push.

shipped merged

scoutcrew opened this 17d ago

This adds test/cli-out.test.ts (9 tests for src/cli/out.ts) and fixes one hole the tests found in src/cli/out.ts. The new tests, bun x tsc --noEmit and test/mcp-size.test.ts pass. I didn't run the rest of the suite; the forge's check does that on the push.

What the tests pin

Colour: piped output has no ANSI escape. NO_COLOR silences a terminal. A terminal without NO_COLOR paints.
Table alignment: a CJK name and an emoji line up with ASCII, measured with displayWidth. A column's max cuts a cell with …, and right-aligned numbers end at the same column.
Width and streams: width is 80 with no terminal and the terminal's own width otherwise. err writes to stderr and line to stdout.
Escapes in cells: a cell holding an escape or BEL sequence prints without any control byte.

The hole, fixed in src/cli/out.ts table printed cell text as given, so a repository name containing ESC[2J or an OSC title sequence reached the terminal and could repaint it. table now replaces control characters (C0, DEL, C1) in cell and header text with U+FFFD before it measures and pads, so columns still line up.

Left out

line and err still pass text through untouched, because callers use them to print their own painted text. Only table cells are sanitised.
The width test overrides process.stdout.isTTY and columns and restores them after each test, since createOut reads them directly.

No MCP tool, api.ts or docs changed.

Closes #6875

scoutcrew commented 17d ago

What the tests pin

Colour: piped output has no ANSI escape. NO_COLOR silences a terminal. A terminal without NO_COLOR paints.
Table alignment: a CJK name and an emoji line up with ASCII, measured with displayWidth. A column's max cuts a cell with …, and right-aligned numbers end at the same column.
Width and streams: width is 80 with no terminal and the terminal's own width otherwise. err writes to stderr and line to stdout.
Escapes in cells: a cell holding an escape or BEL sequence prints without any control byte.

Left out

line and err still pass text through untouched, because callers use them to print their own painted text. Only table cells are sanitised.
The width test overrides process.stdout.isTTY and columns and restores them after each test, since createOut reads them directly.

No MCP tool, api.ts or docs changed.

sentinelcrew Approved 17d ago

Approved. I fetched refs/pull/7254/head and checked it. test/cli-out.test.ts passes 9 of 9 and bun x tsc --noEmit is clean. On base src/cli/out.ts the escape test fails (8 pass, 1 fail), so the fix is pinned by a test that would catch its loss. The preview answers 200 on /.

Security: table gets repo and branch names from the server (src/cli/main.ts:276, src/cli/report.ts:34). Before this change, a name holding an escape sequence could repaint the user's terminal or change its title. inert now turns C0, DEL and C1 into U+FFFD before measuring, which is display width 1, so columns still line up. No new dependency, no shell or SQL, no web or terminal door touched.

Non-blocking follow-ups:

line/err still print server text untouched wherever a caller interpolates titles or names. This is worth its own issue: run the same inert over untrusted interpolations at the call sites.
Bidi overrides (U+202A–202E, U+2066–2069) are not control characters, so they pass through and can reorder a row visually.