Skip to content

fix(scripts): require --actor, the way --files is required - #125

Merged
os-elon merged 3 commits into
mainfrom
claude/issue-116-ownership-actor-required
Aug 19, 2026
Merged

fix(scripts): require --actor, the way --files is required#125
os-elon merged 3 commits into
mainfrom
claude/issue-116-ownership-actor-required

Conversation

@os-elon

@os-elonos-elon commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Fixes#116

What changed

check-translation-ownership.mjs documented --actor LOGIN as part of its invocation but
defaulted the value to the empty string and judged the PR anyway. An empty actor can never
equal a non-empty TRANSLATION_BOT_LOGIN, so isBot was false and a translation-account PR
invoked without the flag was rejected as hand-written — a confident verdict reached with the
discriminator the whole check rests on absent, and nothing saying so.

--actor is now required unconditionally, exactly the way --files is: same UsageError
class from #117, same usage block, no stack, exit 1. Declared, therefore enforced — one usage
convention for the whole file.

This is the governing ruling on the card: the triage adjudication (option 1), issue comment
5331667901. It answers a malformed invocation and nothing else. What the check means for
a well-formed call while TRANSLATION_BOT_LOGIN is unset — the inert report-and-pass path — is
the subject of #68 and is not addressed here; rows 01-03 and 09-14 of the matrix below pin it
byte for byte.

The transcript below is verbatim except for one thing: GitHub strips an angle bracket followed
by a letter from stored bodies (measured on an earlier revision of this description, which lost
every placeholder inside this very code fence), so each placeholder here carries a space the
script does not print.

✗ --actor < login> is required
usage: node .github/scripts/check-translation-ownership.mjs --actor < login> --files < path>
--actor < login> PR author login (workflows pass github.event.pull_request.user.login)
--files < path> file listing one changed path per line (git diff --name-only)

Argument validation runs before the list file is read, so a call that is wrong in two ways
reports the missing --actor rather than the unreadable path.

Declared second change: the unguarded ENOENT

Authorised on dispatch as a rider, same species and named in the card's tail: a --files path
that does not exist died with a raw seven-line ENOENT stack, which reads as "this script is
broken" rather than "I called it wrong". It is now the same usage failure as omitting --files.
The catch is narrowed to error.code === 'ENOENT' — every other way the read can fail (a
directory, a permission error) still rethrows and keeps its stack — and the UsageError doc
comment was corrected to match, since it previously promised a stack for "an unreadable list
file", which now means a file that exists but cannot be read.

Invocation matrix, before and after

25 invocation shapes recorded against 69204ab and against this branch, then compared block by
block (env spelling, argv, exit status, stdout, stderr). 13 byte-identical, 12 changed — and
every changed row is either the defect, the intended requirement, or the rider.

Unchanged — the invariant set, including every shape the real caller produces:

#envargvbeforeafter
01-03unsetactor + files (human clean / human artifacts / bot mixed)00, identical
09transbothuman, clean00, identical
10transbothuman, artifacts11, identical
11transbotbot, artifacts only00, identical
12transbotbot, mixed11, identical
13transbotbot, login case-flipped00, identical
14transbot--actor=…--files=… equals form00, identical
19-21unset / transbotno --files at all11, identical (usage block is unchanged from #117)
25transbotrelative --files (the translations.yml shape)11, identical

Intended change — a missing or empty actor is now a usage failure everywhere:

#envargvbeforeafter
04-05unsetno --actor, clean / artifacts0, inert pass1, usage error
06unsetempty --actor, artifacts0, inert pass1, usage error
07-08empty / all-blankno --actor, clean0, inert pass1, usage error
15transbotno --actor, artifacts1, wrong reason1, usage error
16transbotno --actor, clean0, silent1, usage error
17transbotempty --actor, clean0, silent1, usage error
18transbotblank --actor, artifacts1, wrong reason1, usage error

Rider:

#envargvbeforeafter
22-23unset / transbot--files nonexistent1, ENOENT stack1, usage error
24transbotno --actorand nonexistent --files1, ENOENT stack1, missing-actor usage error

Rows 04-08 are the behaviour change the ruling asks for: an invocation that omits the
documented discriminator now says so instead of returning a verdict. The one thing that had to
survive did: the only real caller, .github/workflows/translations.yml:48, always passes
--actor, and every shape it produces (rows 09-14, plus row 25 for its relative --files
path) is byte-identical before and after. Rows 07-08 also record that TRANSLATION_BOT_LOGIN
set to the empty string or to whitespace still behaves as unset, unchanged by this PR.

Gates

All on Node 22.22.2, matching CI, at commit 1778d4a:

  • pnpm install --frozen-lockfile — 0
  • pnpm turbo run type-check --continue --force — 0
  • pnpm turbo run build --force — 0
  • pnpm turbo run test --force — 0 (includes the self-test runner)
  • node .github/scripts/check-translations.mjs — 0
  • node .github/scripts/check-translation-output.mjs --self-test — 0
  • node .github/scripts/check-translation-output.mjs --files on this PR's changed list — 0
  • the ownership script itself on this PR's own changed list, both paths — 0 and 0
  • the workflow's own invocation shape reproduced verbatim (relative --files, actor passed) — 0

No changeset: this repo does not use them.


Generated by Claude Code

check-translation-ownership.mjs documented `--actor <login>` but defaulted it
to the empty string and judged the PR anyway. An empty actor can never equal a
non-empty bot login, so a translation-account PR invoked without the flag was
rejected as hand-written: a confident verdict reached with the discriminator
the check rests on absent.
The argument is now required exactly while TRANSLATION_BOT_LOGIN is set, which
is the only condition under which it is read. With the variable unset the check
stays inert and a missing --actor still exits 0, unchanged.
Declared second change, same species: a --files path that does not exist is
answered with the usage message instead of a raw ENOENT stack. Every other read
failure still keeps its stack.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Yaqu7kmKZM3tRPd9Y4xivo
…ation
Rework of the first commit on this branch, which implemented the conditional
form. The governing ruling on the card is the triage adjudication: `--actor` is
required exactly the way `--files` is, one usage-error convention for the file.
The actor is the discriminator the check is built on and the header documents
it as part of the invocation, yet it defaulted to the empty string and the PR
was judged anyway — an empty actor can never equal a non-empty bot login, so a
translation-account PR invoked without the flag came back rejected as
hand-written. Declared, therefore enforced.
This answers a malformed invocation, not the meaning of the inert path: a
well-formed call with TRANSLATION_BOT_LOGIN unset still reports and passes.
Declared second change, unchanged from the first round: a --files path that
does not exist is answered with the usage message instead of a raw ENOENT
stack, narrowed to ENOENT so every other read failure keeps its stack.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Yaqu7kmKZM3tRPd9Y4xivo
@os-elonos-elon changed the title fix(scripts): require --actor when TRANSLATION_BOT_LOGIN is setfix(scripts): require --actor, the way --files is requiredAug 19, 2026
@os-elon
os-elon marked this pull request as ready for review August 19, 2026 00:26
@os-elon
os-elon merged commit 599037c into mainAug 19, 2026
2 checks passed
Sign up for freeto 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.

check-translation-ownership.mjs accepts a missing --actor silently and then judges the PR anyway

2 participants

@os-elon@claude