Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): answer a missing --files with a usage message, not a stack trace - #117
Merged
Merged
Conversation
…ack trace `check-translation-ownership.mjs` run without `--files` threw an uncaught Error: Node printed the source frame plus a seven-line stack, and the reader's first conclusion was "this script is broken" rather than "I called it wrong". It misled exactly that way once already — a dispatch prompt listed the bare command as a check to run and the stack read as a regression until it was diffed against main. Argument validation now throws a UsageError and the entry point catches that class only: message plus the usage line on stderr, no stack, still exit 1. A misinvocation stays a failure — exiting 0 would turn it into a silent pass. Every other throw is rethrown untouched, so a genuine internal fault (unparseable i18n.ts, unreadable list file, a bug here) keeps the stack it needs. No change to what the gate accepts, rejects or reports when it is invoked correctly: all eight correct-invocation shapes, on both the inert and the enforcing path, are byte-identical before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yaqu7kmKZM3tRPd9Y4xivo
os-elon
marked this pull request as ready for review
August 18, 2026 16:57
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#113
Run without
--files, this gate threw an uncaughtError: Node printed the source frame plus a seven-line stack and its own version banner. The message was right and the exit code was already 1 — the shape was wrong. A stack trace says "this script is broken" when the truth is "you called it wrong", and it misled exactly that way once: a dispatch prompt listed the bare command as a check to run clean, and the trace read as a regression until it was diffed againstmain.Argument validation now throws a
UsageError, and the entry point catches that class only — message and usage line on stderr, no stack, still exit 1. Every other throw is rethrown untouched, so an unparseablei18n.ts, an unreadable list file or a bug in the check keeps the stack a real fault needs.Missing required argument — before and after
Before:
After:
(The two placeholders are angle-bracketed in the real source and the real output; GitHub's body sanitizer eats that token, which is also why the card's own quote of line 65 is missing it.)
Same result for the other three shapes of the same mistake: no arguments at all,
--actoralone,--fileswith no value, and--files=with an empty value.It still fails. Exiting 0 on a missing required argument would turn a misinvocation into a silent pass, which is the species of dishonest green this repo removed from the
teststep in #112.Correct invocations are byte-identical
Twelve invocations were recorded on
mainand again on this branch and diffed: four inert-path shapes (TRANSLATION_BOT_LOGINunset), four enforcing-path shapes with the variable set (human clean, human with artifacts rejected, bot artifacts-only accepted, bot mixed rejected), the--files=spelling, and three shapes with--actorabsent. Zero diff lines across all of them — the only changes in the transcript are the four usage-error shapes above.A genuine internal failure keeps its stack, verified with a
--filespath that does not exist: samenode:fs:440source frame, sameENOENTproperties, same version banner, only the line numbers moved as the file grew.Deliberately not in this PR
--actoris left alone. It is documented as required but implemented as optional ((value('actor') ?? '').trim()), so omitting it does not crash — it silently classifies the run as a human PR. That is a different defect from this card's, and requiring it would flip an invocation that exits 0 today into exit 1 through the inert path that The ownership check is inert on main until TRANSLATION_BOT_LOGIN is set #68 owns. Measured and filed ascheck-translation-ownership.mjsaccepts a missing--actorsilently and then judges the PR anyway #116 with options.tools/ci-scripts/run-self-tests.mjsand itsSELF_TESTEDlist are untouched — the runner's own comment names this script as deliberately not run there. Its guard was exercised anyway:turbo run testexecuted for real on this edit (.github/scripts/**is an input to that task) and passed.TRANSLATION_BOT_LOGINinertness, or the workflow..github/workflows/translations.ymlpasses both flags, so the one real caller never reaches the changed path.Gates, at b48bc8c
pnpm install --frozen-lockfilepnpm turbo run type-check --continuepnpm turbo run buildpnpm turbo run test0 cached, 1 total, real execution, 20 self-test casesnode .github/scripts/check-translations.mjsnode .github/scripts/check-translation-ownership.mjs --actor os-elon --files changed.txtnode .github/scripts/check-translation-output.mjs --files changed.txt