Skip to content

fix: the review's remaining findings, in the gate and in the fence test - #20

Merged
T-Crypt merged 2 commits into
oh-my-pocketfrom
fix/review-findings
Oct 13, 2026
Merged

T-Crypt merged 2 commits into
oh-my-pocketfrom
fix/review-findings

Conversation

@T-Crypt

@T-Crypt T-Crypt commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Two of the findings from the review of this flight, both small and neither able to redden a push run on its own.

scripts/check-web.mjs

  • A tsc configuration error exits 2, which the gate accepted as "could not run": it reported web/ clean having checked nothing. An error TS line that is not under web/ now fails the gate.
  • A web-clean.txt line with a trailing space or a CRLF ending never matched, so a listed file with an error passed. Each line is trimmed.
  • No maxBuffer on the findings: past about a megabyte the output truncated, status was null, and the step went red without naming a clean file. The bound is 64 MiB and the exit is written through process.exitCode so the report flushes.

Proved against a scratch project: noinputs and badoption exit 0 before and 1 after; trailing and crlf exit 0 before and 1 after, naming web/themes.js; a 1.4 MB report is read whole. The real npm run check:web on web/ is unchanged: exit 0, 3,253 findings, byte-identical output.

test/fence.test.ts

Four groups INVISIBLE_CLASS names were unpinned, each deletable without a test failing: \\uFE00-\\uFE0F, \\u115F\\u1160, \\u17B4\\u17B5, \\u180B-\\u180F. One representative each, in the same 81-character form the file already uses. Removing any of the four single fragments now fails the file, naming the new representative. src/server/fence.ts is unchanged.

Proof

  • npm test: 640 tests, 640 pass, 0 fail, 0 skipped.
  • node --test test/fence.test.ts: 7 pass.
  • npm run check, npm run lint, npm run format:check: exit 0.

@T-Crypt
T-Crypt merged commit 6e25471 into oh-my-pocket Oct 13, 2026
@T-Crypt
T-Crypt deleted the fix/review-findings branch October 14, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant