From 7ef0bf416196dafadd0dc3bae90f0ba700528c9a Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 08:34:25 +0000 Subject: [PATCH 1/8] feat: route pr subcommands and run through gh Keep the legacy pr lifecycle for flag-only invocations, but send pr view/list/diff/create/comment/review and the new run command to the catalogued gh actions. --- packages/cli/src/index.ts | 2 +- packages/client/src/index.ts | 15 ++++++++++--- test/unit/gh-command.test.ts | 42 ++++++++++++++++++++++++++++++++++-- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index bdfdd9bc..0ceef907 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -20,7 +20,7 @@ import { platformRefusal } from './platform.ts'; import { packageVersion } from './version.ts'; /** Commands implemented by the container-side package. */ -export const CLIENT_COMMANDS = ['setup', 'probe', 'logs', 'pr', 'gh'] as const; +export const CLIENT_COMMANDS = ['setup', 'probe', 'logs', 'pr', 'run', 'gh'] as const; /** Commands implemented by the host-side package. */ export const HOST_COMMANDS = [ diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index ca8741dc..d2aaf3a1 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -15,7 +15,9 @@ Commands setup Configure this environment's git to use the broker probe Ask the broker what it would do for a repository, without pushing logs Read a workflow job's log through the broker - pr Use the broker's pull-request operations + pr Use the broker's pull-request operations; pr view/list/diff/create/comment/review + resolve to the gh actions + run Run a workflow run action (run view, run list) gh Run a catalogued GitHub action through the broker As a git credential helper @@ -37,8 +39,15 @@ async function run(argv: readonly string[]): Promise { return (await import('./commands/probe.ts')).runProbe(rest); case 'logs': return (await import('./commands/logs.ts')).runLogs(rest); - case 'pr': + case 'pr': { + const sub = rest[0]; + if (sub !== undefined && ['view', 'list', 'diff', 'create', 'comment', 'review'].includes(sub)) { + return (await import('./commands/gh.ts')).runGh(['pr', ...rest]); + } return (await import('./commands/pr.ts')).runPr(rest); + } + case 'run': + return (await import('./commands/gh.ts')).runGh(['run', ...rest]); case 'gh': return (await import('./commands/gh.ts')).runGh(rest); default: @@ -49,7 +58,7 @@ async function run(argv: readonly string[]): Promise { export const clientBackend: CommandBackend = { role: 'client', - commands: ['setup', 'probe', 'logs', 'pr', 'gh'], + commands: ['setup', 'probe', 'logs', 'pr', 'run', 'gh'], help: CLIENT_HELP, run, }; diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 651068b7..7911964a 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -14,14 +14,18 @@ import type { AuditSink, BrokerConfig, Provider, StaticHostConfig } from '../../ const CLIENT_BIN = fileURLToPath(new URL('../../packages/client/bin/gcb.js', import.meta.url)); /** Run the installed client as a child, so its output never replaces the test reporter. */ -function runGhCli(args: readonly string[], cwd?: string): Promise { +function runClient(args: readonly string[], cwd?: string): Promise { return tryRunAsync( process.execPath, - ['--conditions=development', CLIENT_BIN, 'gh', ...args], + ['--conditions=development', CLIENT_BIN, ...args], { cwd, env: process.env, timeoutMs: 5000 }, ); } +function runGhCli(args: readonly string[], cwd?: string): Promise { + return runClient(['gh', ...args], cwd); +} + describe('gcb gh', () => { it('reports truncation and timed-out writes', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-output-')); @@ -200,6 +204,40 @@ describe('gcb gh', () => { } }); + it('routes pr subcommands and run through the gh action', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-shorthand-')); + const socketPath = path.join(root, 'broker.sock'); + const block: StaticHostConfig = { provider: 'static', allow: ['acme/widget'] }; + const config: BrokerConfig = { + socketPath, + socketMode: 0o600, + auditPath: null, + tokenCacheSkewSeconds: 300, + hosts: { 'github.com': block }, + }; + const audit: AuditSink = { path: null, record: () => {}, close: () => Promise.resolve() }; + const actions: string[] = []; + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async (request) => { + actions.push(request.action); + return { stdout: '', stderr: '', code: 0, signal: null, truncated: false, timedOut: false }; + }, + }; + const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); + try { + const pr = await runClient(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(pr.status, 0); + const run = await runClient(['run', 'list', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(run.status, 0); + assert.deepEqual(actions, ['pr:view', 'run:list']); + } finally { + await broker.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + it('sends a typed action to the broker and prints its stdout', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-command-')); const socketPath = path.join(root, 'broker.sock'); From 5a08d62b90068bf7c9518857c5d79d51086f1f25 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 08:44:35 +0000 Subject: [PATCH 2/8] feat: move pr close to the gh action catalog Delete the pull-request close implementation and point --close at gcb gh pr close; keep the legacy pr command for actions gh cannot express. --- ROADMAP.md | 1 + packages/cli/src/actions.ts | 12 ++++++++++++ packages/cli/src/types.ts | 1 - packages/client/src/commands/pr.ts | 9 +++------ packages/host/src/broker.ts | 1 - packages/host/src/providers/github-app.ts | 6 +----- test/unit/actions.test.ts | 2 ++ test/unit/broker.test.ts | 16 ++++------------ test/unit/commands.test.ts | 8 +++++++- 9 files changed, 30 insertions(+), 26 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index e175ae96..21e17c91 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -261,6 +261,7 @@ host_broker: | `pr:list` | `gh pr list` | `pull_requests: read` | | `pr:diff` | `gh pr diff` | `pull_requests: read`, `contents: read` | | `pr:create` | `gh pr create` | `pull_requests: write`, `contents: read` | + | `pr:close` | `gh pr close` | `pull_requests: write` | | `pr:comment` | `gh pr comment` | `pull_requests: write` | | `pr:review` | `gh pr review` | `pull_requests: write` | | `issue:create` | `gh issue create` | `issues: write` | diff --git a/packages/cli/src/actions.ts b/packages/cli/src/actions.ts index 6abf354d..6660ed25 100644 --- a/packages/cli/src/actions.ts +++ b/packages/cli/src/actions.ts @@ -152,6 +152,17 @@ const PR_COMMENT: GhAction = { hostUser: true, }; +// GitHub: PATCH /repos/{owner}/{repo}/pulls/{pull_number} +const PR_CLOSE: GhAction = { + name: 'pr:close', + command: ['pr', 'close'], + positionals: [{ name: 'number', type: 'number', required: true }], + flags: [], + permissions: { pull_requests: 'write' }, + timeoutSeconds: 60, + hostUser: true, +}; + // GitHub: POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews const PR_REVIEW: GhAction = { name: 'pr:review', @@ -226,6 +237,7 @@ export const GH_ACTIONS: Readonly> = Object.freeze({ 'pr:list': PR_LIST, 'pr:diff': PR_DIFF, 'pr:create': PR_CREATE, + 'pr:close': PR_CLOSE, 'pr:comment': PR_COMMENT, 'pr:review': PR_REVIEW, 'issue:create': ISSUE_CREATE, diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index 58156e9f..82aca14a 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -100,7 +100,6 @@ export interface JobLogRequest { /** What can be done to a pull request. */ export type PullRequestAction = | 'open' - | 'close' | 'merge' | 'update' | 'status' diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index 328013c4..485012a3 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -56,7 +56,6 @@ Acting on one --edit Replace the body of that review comment, with --body/--body-file (an id --threads prints; no --number is needed) --resolve Mark that review thread resolved (the id --threads prints) - --close Close it --merge Merge it (branch protection still applies) --method merge, squash (default) or rebase With --number and none of the above, the title, body or base is updated. @@ -87,7 +86,6 @@ function actionOf(args: { has(name: string): boolean; value(name: string): strin if (args.has('threads')) return 'threads' as const; if (args.has('comment')) return 'comment' as const; if (args.has('status')) return 'status' as const; - if (args.has('close')) return 'close' as const; if (args.has('merge')) return 'merge' as const; if (args.value('number') !== undefined) return 'update' as const; return 'open' as const; @@ -101,7 +99,7 @@ function actionOf(args: { has(name: string): boolean; value(name: string): strin */ export async function runPr(argv: readonly string[]): Promise { const args = parseArgs(argv, { - booleans: ['help', 'draft', 'close', 'merge', 'status', 'comment', 'threads', 'via-host'], + booleans: ['help', 'draft', 'merge', 'status', 'comment', 'threads', 'via-host'], }); if (args.has('help') || args.has('h')) { say(PR_USAGE); @@ -111,6 +109,7 @@ export async function runPr(argv: readonly string[]): Promise { const socketPath = resolveSocketPath(args.value('socket'), process.env); const host = args.value('host'); const repo = args.value('repo') ?? fail('--repo is required (e.g. owner/name)'); + if (args.has('close')) fail('--close moved to: gcb gh pr close --repo owner/name'); const action = actionOf(args); if (args.has('via-host') && action !== 'open') { fail('--via-host only creates a pull request, so it does not go with ' + JSON.stringify(action)); @@ -296,9 +295,7 @@ export async function runPr(argv: readonly string[]): Promise { const done = action === 'open' ? 'opened' - : action === 'close' - ? 'closed' - : action === 'merge' + : action === 'merge' ? 'merged' : action === 'comment' ? 'commented on' diff --git a/packages/host/src/broker.ts b/packages/host/src/broker.ts index 2c2bc240..bd03b8bf 100644 --- a/packages/host/src/broker.ts +++ b/packages/host/src/broker.ts @@ -382,7 +382,6 @@ export function createRequestHandler( const action = message['action'] ?? 'open'; if ( action !== 'open' && - action !== 'close' && action !== 'merge' && action !== 'update' && action !== 'status' && diff --git a/packages/host/src/providers/github-app.ts b/packages/host/src/providers/github-app.ts index b485cd4b..f68305d8 100644 --- a/packages/host/src/providers/github-app.ts +++ b/packages/host/src/providers/github-app.ts @@ -1036,11 +1036,7 @@ function pullRequestNumber(value: unknown): number | undefined { let method = 'POST'; let url = collection; let body: Record = { title: request.title, body: request.body, head: request.head, base: request.base, draft: request.draft === true }; - if (request.action === 'close') { - method = 'PATCH'; - url = `${collection}/${request.number}`; - body = { state: 'closed' }; - } else if (request.action === 'update') { + if (request.action === 'update') { method = 'PATCH'; url = `${collection}/${request.number}`; body = { title: request.title, body: request.body, base: request.base }; diff --git a/test/unit/actions.test.ts b/test/unit/actions.test.ts index 123e3c61..c2367ad2 100644 --- a/test/unit/actions.test.ts +++ b/test/unit/actions.test.ts @@ -17,6 +17,7 @@ describe('gh action catalog', () => { 'pr:list', 'pr:diff', 'pr:create', + 'pr:close', 'pr:comment', 'pr:review', 'issue:create', @@ -25,6 +26,7 @@ describe('gh action catalog', () => { ]); assert.equal('pr:merge' in GH_ACTIONS, false); assert.equal(getGhAction('pr:create').command.join(' '), 'pr create'); + assert.equal(getGhAction('pr:close').command.join(' '), 'pr close'); }); it('keeps creation token permissions narrow', () => { diff --git a/test/unit/broker.test.ts b/test/unit/broker.test.ts index ab023738..6afba1ff 100644 --- a/test/unit/broker.test.ts +++ b/test/unit/broker.test.ts @@ -280,24 +280,16 @@ describe('broker request handling', () => { assert.match(unsupported.reason ?? '', /cannot manage pull requests/); }); - it('closes a pull request, and refuses a merge method it does not know', async () => { + it('refuses a merge method it does not know', async () => { const seen: string[] = []; const managed: Provider = { name: 'with-prs', getCredential: () => Promise.reject(new Error('not used here')), pullRequest: (request) => { seen.push(request.action); - return Promise.resolve({ number: request.number ?? 0, url: 'u', state: 'closed' }); + return Promise.resolve({ number: request.number ?? 0, url: 'u', state: 'open' }); }, }; - const { handle, events } = makeHarness(['acme/widget'], managed); - const response = await handle({ ...REQUEST, op: 'pull-request', action: 'close', number: 7 }); - - assert.equal(response.ok, true); - assert.deepEqual(seen, ['close']); - assert.equal(response.prState, 'closed'); - assert.equal(events.at(-1)?.['action'], 'close'); - const badMethod = await makeHarness(['acme/widget'], managed).handle({ ...REQUEST, op: 'pull-request', @@ -306,7 +298,7 @@ describe('broker request handling', () => { method: 'fast-forward', }); assert.equal(badMethod.code, CODES.BAD_REQUEST); - assert.deepEqual(seen, ['close'], 'and the provider was never asked to do it'); + assert.deepEqual(seen, [], 'the provider was never asked to do it'); }); it('reports a pull request without changing it', async () => { @@ -407,7 +399,7 @@ describe('broker request handling', () => { assert.equal(response.code, 'bad-request', label + ' has to be refused by the broker'); } - const wrongAction = await handle({ ...comments, action: 'close' }); + const wrongAction = await handle({ ...comments, action: 'merge' }); assert.equal(wrongAction.code, 'bad-request', 'and an anchor only belongs on a comment'); }); diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index 6b9121f2..d407b611 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1143,11 +1143,17 @@ describe('the anchor flags, at the CLI boundary', () => { }); it('refuses an anchor on an action that is not a comment', () => { - const closed = cli([...base, '--close', '--file', 'Dockerfile', '--line', '4', '--body', 'x']); + const closed = cli([...base, '--merge', '--file', 'Dockerfile', '--line', '4', '--body', 'x']); assert.equal(closed.status, 1); assert.match(closed.stderr, /only go with --comment/); }); + it('points --close at the gh action', () => { + const moved = cli([...base, '--close']); + assert.equal(moved.status, 1); + assert.match(moved.stderr, /gcb gh pr close/); + }); + it('refuses a side that is neither, and a line that is not a line', () => { const side = cli([...base, '--comment', '--file', 'Dockerfile', '--line', '4', '--side', 'middle', '--body', 'x']); assert.match(side.stderr, /--side must be left or right/); From 8787a6d740ba58b20d2e60f54e2a879f769293c6 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 16:50:58 +0800 Subject: [PATCH 3/8] fix: route every catalogued pr subcommand, and document the shorthand The dispatch list in the client was a second copy of the catalog, and it left `pr:close` out, so `gcb pr close 7` fell through to the flag-only lifecycle and reported a missing --host instead of closing anything. Ask the catalog instead: a `pr ` becomes the action when `pr:` exists, and everything else keeps its flags. `--close` now refuses before `--repo` is required, so the message naming `gcb gh pr close` is what the caller sees for a bare `gcb pr --close`; the usage text, the module comment and `actionOf`'s doc no longer claim that the flag-only form closes. `gcb pr` and `gcb run` with no subcommand print the catalog help, which is what `gcb gh` already does, rather than an error that names a different command. Docs: README documents the shorthand and the moved close; the ROADMAP line that said the shorthand is not wired now says what is; AGENTS.md's lifecycle rule no longer says `--close`. --- AGENTS.md | 4 +++- README.md | 5 +++++ ROADMAP.md | 5 +++-- packages/client/src/commands/gh.ts | 5 ++++- packages/client/src/commands/pr.ts | 15 +++++++++++---- packages/client/src/index.ts | 9 ++++++--- test/unit/commands.test.ts | 4 ++++ test/unit/gh-command.test.ts | 4 +++- 8 files changed, 39 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f9fe5f3b..e12f0bdb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,7 +191,9 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p usually one a later push replaced: read the newest run for the commit, not the first. - **Open a pull request; the agent opens it, and can close or update it.** `gcb pr` covers most of the lifecycle: `--head`/`--base`/`--title` to open, - `--number` with `--close`, or `--number` with neither (which updates the title, body or base). + `--number` with none of the acting flags (which updates the title, body or base). Closing is the + catalogued `pr:close` action: `gcb pr close ` (or `gcb gh pr close`), and `--close` + refuses with a pointer there. The broker performs each action without the token leaving it, and prints the number, the URL and the state. diff --git a/README.md b/README.md index 8e675d6a..6dadde78 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,11 @@ gcb gh run view 123456 --log --repo owner/name `-R` is the short form of `--repo`, as in `gh` itself. +`gcb pr ` and `gcb run ` are shorthands for the same actions when the +catalog has one — `gcb pr view 12`, `gcb pr close 12`, `gcb run list`. The flag-only `gcb pr` form +described below is still what runs the lifecycle `gh` cannot express (opening as a person, review +threads); `gcb pr --close` is gone and refuses with a pointer at `gcb gh pr close`. + Inside a checkout, `--repo` can be omitted only when `origin` names a usable GitHub repository whose host matches `--host`; otherwise pass `--repo`, or the matching `--host`. diff --git a/ROADMAP.md b/ROADMAP.md index 21e17c91..f691d75c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -204,8 +204,9 @@ host_broker: - built-ins first (`setup`, `init`, `stage`, `compose`, `probe`, `diagnose`, `authorize`, `app`, `host`, `config`, `version`, `help`); - `gcb gh …` explicit; - - `gcb pr view …`, `gcb run view …` etc. resolve to the `gh` subcommand. (Explicit - `gcb gh pr view` is wired; the shorthand is not.) + - `gcb pr view …`, `gcb run view …` etc. resolve to the `gh` subcommand. (Wired: any `pr` + subcommand the catalog has, plus `run`; the flag-only `gcb pr` stays for the lifecycle `gh` + cannot express, and `--close` refuses with a pointer at `gcb gh pr close`.) - [ ] Remove the built-in `pr` and `logs` commands. `gcb run view --job --log` replaces `logs`. - [x] Build the command table. Every entry has: - canonical subcommand (no aliases, no abbreviations); diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index d5bc76e2..818c9a8a 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -48,6 +48,7 @@ function repoFromCheckout(cwd: string): { repo: string; host: string } | undefin export const GH_USAGE = `Usage: gcb gh [options] Run a catalogued GitHub action through the broker. +gcb pr and gcb run are shorthands for the same actions. Common -R, --repo Repository the action belongs to (default: origin remote) @@ -71,7 +72,9 @@ export async function runGh(argv: readonly string[]): Promise { } const [domain, command, ...rest] = argv; if (domain === undefined || command === undefined) { - fail('gcb gh needs a command, for example: gcb gh pr view 12 --repo owner/name'); + // `gcb pr` and `gcb run` arrive here too, so the answer is the catalog rather than one example. + say(GH_USAGE); + return 1; } const actionName = `${domain}:${command}`; let action; diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index 485012a3..98d4b16d 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -1,5 +1,8 @@ /** - * `pr` — open, close, merge or update one pull request through the broker. + * `pr` — open, merge or update one pull request through the broker. + * + * `pr ` runs as the catalogued gh action when the catalog has one (`pr:view`, + * `pr:close`, …); this file is the flag-only lifecycle for the actions gh cannot express. * * The credential never leaves the broker. The broker mints a token narrowed to `pull_requests: write` * plus read access to the branches it names, performs the action itself, and answers with the number, @@ -29,9 +32,12 @@ const PR_TIMEOUT_MS = 60_000; /** Usage text for `pr`. */ export const PR_USAGE = `Usage: gcb pr [options] -Open, close, merge, update or inspect a pull request through the broker, without the credential +Open, merge, update or inspect a pull request through the broker, without the credential ever reaching this side. +A subcommand the gh action catalog has (pr view, pr close, …) runs as that action instead; +gcb gh --help lists them. The flags below are the flag-only lifecycle gh cannot express. + Opening (the default) --head The branch holding the change [required] May be :, e.g. origin:feat/x, to name a fork @@ -72,7 +78,7 @@ Common * Read the action out of the flags. * * @param args - the parsed arguments. - * @returns `close`, `merge`, `update` or `open`. + * @returns `merge`, `update` or `open`. */ function actionOf(args: { has(name: string): boolean; value(name: string): string | undefined }) { // `--resolve`, `--reply-to` and `--edit` take a value, and a bare one is parsed as a boolean flag with @@ -105,11 +111,12 @@ export async function runPr(argv: readonly string[]): Promise { say(PR_USAGE); return 0; } + // Before --repo: the point of the message is to name the command that does it now. + if (args.has('close')) fail('--close moved to: gcb gh pr close --repo owner/name'); const socketPath = resolveSocketPath(args.value('socket'), process.env); const host = args.value('host'); const repo = args.value('repo') ?? fail('--repo is required (e.g. owner/name)'); - if (args.has('close')) fail('--close moved to: gcb gh pr close --repo owner/name'); const action = actionOf(args); if (args.has('via-host') && action !== 'open') { fail('--via-host only creates a pull request, so it does not go with ' + JSON.stringify(action)); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index d2aaf3a1..9870d7f5 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3,6 +3,7 @@ * * The host commands are not imported here; installing `gcb-client` never pulls `yaml`. */ +import { GH_ACTIONS } from 'gcb-cli'; import type { CommandBackend } from 'gcb-cli'; export { runHelper } from './helper.ts'; @@ -15,8 +16,8 @@ Commands setup Configure this environment's git to use the broker probe Ask the broker what it would do for a repository, without pushing logs Read a workflow job's log through the broker - pr Use the broker's pull-request operations; pr view/list/diff/create/comment/review - resolve to the gh actions + pr Use the broker's pull-request lifecycle; a subcommand the gh action catalog has + (pr view, pr close, …) runs as that action instead run Run a workflow run action (run view, run list) gh Run a catalogued GitHub action through the broker @@ -40,8 +41,10 @@ async function run(argv: readonly string[]): Promise { case 'logs': return (await import('./commands/logs.ts')).runLogs(rest); case 'pr': { + // The catalog is the one source for an action, so the shorthand asks it rather than keeping a + // second list: a subcommand the catalog does not have stays with the flag-only lifecycle. const sub = rest[0]; - if (sub !== undefined && ['view', 'list', 'diff', 'create', 'comment', 'review'].includes(sub)) { + if (sub !== undefined && Object.hasOwn(GH_ACTIONS, `pr:${sub}`)) { return (await import('./commands/gh.ts')).runGh(['pr', ...rest]); } return (await import('./commands/pr.ts')).runPr(rest); diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index d407b611..3eb035ef 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1152,6 +1152,10 @@ describe('the anchor flags, at the CLI boundary', () => { const moved = cli([...base, '--close']); assert.equal(moved.status, 1); assert.match(moved.stderr, /gcb gh pr close/); + + const bare = cli(['--close']); + assert.equal(bare.status, 1); + assert.match(bare.stderr, /gcb gh pr close/, 'before --repo, so the message names the replacement'); }); it('refuses a side that is neither, and a line that is not a line', () => { diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 7911964a..a5c91c6d 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -229,9 +229,11 @@ describe('gcb gh', () => { try { const pr = await runClient(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]); assert.equal(pr.status, 0); + const close = await runClient(['pr', 'close', '7', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(close.status, 0); const run = await runClient(['run', 'list', '--repo', 'acme/widget', '--socket', socketPath]); assert.equal(run.status, 0); - assert.deepEqual(actions, ['pr:view', 'run:list']); + assert.deepEqual(actions, ['pr:view', 'pr:close', 'run:list']); } finally { await broker.close(); fs.rmSync(root, { recursive: true, force: true }); From 6ee6c16cd060ce82cf348b87487eb310cd220e4c Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 09:09:39 +0000 Subject: [PATCH 4/8] feat: generate the client shorthand help from the catalog The client help now lists the pr/run subcommands from GH_ACTIONS instead of a hand-maintained second list. --- packages/client/src/index.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 9870d7f5..a0110a3d 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -8,6 +8,15 @@ import type { CommandBackend } from 'gcb-cli'; export { runHelper } from './helper.ts'; +const PR_SUBCOMMANDS = Object.values(GH_ACTIONS) + .filter((action) => action.command[0] === 'pr') + .map((action) => action.command[1]) + .join('|'); +const RUN_SUBCOMMANDS = Object.values(GH_ACTIONS) + .filter((action) => action.command[0] === 'run') + .map((action) => action.command[1]) + .join('|'); + export const CLIENT_HELP = `gcb-client — container-side git credential helper and broker client. Usage: gcb [options] @@ -16,9 +25,9 @@ Commands setup Configure this environment's git to use the broker probe Ask the broker what it would do for a repository, without pushing logs Read a workflow job's log through the broker - pr Use the broker's pull-request lifecycle; a subcommand the gh action catalog has - (pr view, pr close, …) runs as that action instead - run Run a workflow run action (run view, run list) + pr Use the broker's pull-request lifecycle; these run as gh actions: + ${PR_SUBCOMMANDS} + run Run a workflow run action: ${RUN_SUBCOMMANDS} gh Run a catalogued GitHub action through the broker As a git credential helper From ab7fc58bb6089d38c9bfd6b9909002f80cfb92b7 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 09:36:19 +0000 Subject: [PATCH 5/8] fix: reject valued --close and help bare pr Check --close whether parsed as a flag or a value, show the catalog help for bare gcb pr, and document pr:close as the GraphQL mutation. --- packages/cli/src/actions.ts | 2 +- packages/client/src/commands/pr.ts | 4 +++- packages/client/src/index.ts | 4 ++++ test/unit/commands.test.ts | 24 +++++++++++++++++++++--- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/actions.ts b/packages/cli/src/actions.ts index 6660ed25..bc8b9491 100644 --- a/packages/cli/src/actions.ts +++ b/packages/cli/src/actions.ts @@ -152,7 +152,7 @@ const PR_COMMENT: GhAction = { hostUser: true, }; -// GitHub: PATCH /repos/{owner}/{repo}/pulls/{pull_number} +// GitHub GraphQL: closePullRequest const PR_CLOSE: GhAction = { name: 'pr:close', command: ['pr', 'close'], diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index 98d4b16d..f2016abd 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -112,7 +112,9 @@ export async function runPr(argv: readonly string[]): Promise { return 0; } // Before --repo: the point of the message is to name the command that does it now. - if (args.has('close')) fail('--close moved to: gcb gh pr close --repo owner/name'); + if (args.has('close') || args.value('close') !== undefined) { + fail('--close moved to: gcb gh pr close --repo owner/name'); + } const socketPath = resolveSocketPath(args.value('socket'), process.env); const host = args.value('host'); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index a0110a3d..0e808049 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -52,6 +52,10 @@ async function run(argv: readonly string[]): Promise { case 'pr': { // The catalog is the one source for an action, so the shorthand asks it rather than keeping a // second list: a subcommand the catalog does not have stays with the flag-only lifecycle. + if (rest.length === 0) { + await (await import('./commands/gh.ts')).runGh(['pr', '--help']); + return 1; + } const sub = rest[0]; if (sub !== undefined && Object.hasOwn(GH_ACTIONS, `pr:${sub}`)) { return (await import('./commands/gh.ts')).runGh(['pr', ...rest]); diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index 3eb035ef..c6adda73 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1112,7 +1112,7 @@ describe('the anchor flags, at the CLI boundary', () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-pr-home-')); after(() => fs.rmSync(home, { recursive: true, force: true })); - const cli = (args: readonly string[]): { status: number; stderr: string } => { + const cli = (args: readonly string[]): { status: number; stdout: string; stderr: string } => { try { execFileSync('node', ['--conditions=development', 'packages/client/bin/gcb.js', 'pr', ...args], { cwd: path.resolve('.'), @@ -1123,10 +1123,14 @@ describe('the anchor flags, at the CLI boundary', () => { // reach a broker someone happens to have running. env: { ...process.env, HOME: home, GIT_BROKER_SOCKET: '', GIT_BROKER_SOCKET_FILE: '' }, }); - return { status: 0, stderr: '' }; + return { status: 0, stdout: '', stderr: '' }; } catch (error) { const failed = error as { status?: number | null; stderr?: string }; - return { status: failed.status ?? -1, stderr: String(failed.stderr ?? '') }; + return { + status: failed.status ?? -1, + stdout: String((failed as { stdout?: string }).stdout ?? ''), + stderr: String(failed.stderr ?? ''), + }; } }; @@ -1148,6 +1152,20 @@ describe('the anchor flags, at the CLI boundary', () => { assert.match(closed.stderr, /only go with --comment/); }); + it('refuses --close with a value too', () => { + for (const argv of [['--close', '7'], ['--close=7']]) { + const moved = cli([...base, ...argv]); + assert.equal(moved.status, 1); + assert.match(moved.stderr, /gcb gh pr close/); + } + }); + + it('shows help for bare gcb pr', () => { + const result = cli([]); + assert.equal(result.status, 1); + assert.match(result.stdout, /Usage: gcb gh/); + }); + it('points --close at the gh action', () => { const moved = cli([...base, '--close']); assert.equal(moved.status, 1); From c180dbc586ec540cd012fbec9b85c89f7d1c0f0f Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 18:23:02 +0800 Subject: [PATCH 6/8] refactor: one helper derives both shorthand help lists `pr` and `run` each carried the same five lines. One function, called from the help template, says it once and stays lazy about nothing: the values are interpolated into CLIENT_HELP at module load, as before. --- packages/client/src/index.ts | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 0e808049..a4581862 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -8,14 +8,21 @@ import type { CommandBackend } from 'gcb-cli'; export { runHelper } from './helper.ts'; -const PR_SUBCOMMANDS = Object.values(GH_ACTIONS) - .filter((action) => action.command[0] === 'pr') - .map((action) => action.command[1]) - .join('|'); -const RUN_SUBCOMMANDS = Object.values(GH_ACTIONS) - .filter((action) => action.command[0] === 'run') - .map((action) => action.command[1]) - .join('|'); +/** + * The catalogued subcommands of one domain, for the help text. + * + * The dispatch below reads the same table, so this cannot list a subcommand that does not run and a + * new action shows up in the help without a second edit. + * + * @param domain - the command before the colon, e.g. `pr`. + * @returns the subcommands, joined for one usage line. + */ +function cataloguedSubcommands(domain: string): string { + return Object.values(GH_ACTIONS) + .filter((action) => action.command[0] === domain) + .map((action) => action.command[1]) + .join('|'); +} export const CLIENT_HELP = `gcb-client — container-side git credential helper and broker client. @@ -26,8 +33,8 @@ Commands probe Ask the broker what it would do for a repository, without pushing logs Read a workflow job's log through the broker pr Use the broker's pull-request lifecycle; these run as gh actions: - ${PR_SUBCOMMANDS} - run Run a workflow run action: ${RUN_SUBCOMMANDS} + ${cataloguedSubcommands('pr')} + run Run a workflow run action: ${cataloguedSubcommands('run')} gh Run a catalogued GitHub action through the broker As a git credential helper From 4bec34f65b33405a0a074cf5cde75903f7bc3ad8 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 11:00:32 +0000 Subject: [PATCH 7/8] fix: show the catalog help for bare gcb run Match gcb pr's no-subcommand behavior. --- packages/client/src/index.ts | 4 ++++ test/unit/gh-command.test.ts | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index a4581862..f9498b8d 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -70,6 +70,10 @@ async function run(argv: readonly string[]): Promise { return (await import('./commands/pr.ts')).runPr(rest); } case 'run': + if (rest.length === 0) { + await (await import('./commands/gh.ts')).runGh(['run', '--help']); + return 1; + } return (await import('./commands/gh.ts')).runGh(['run', ...rest]); case 'gh': return (await import('./commands/gh.ts')).runGh(rest); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index a5c91c6d..691f44da 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -272,3 +272,11 @@ describe('gcb gh', () => { } }); }); + +describe('bare run', () => { + it('prints the catalog help', async () => { + const result = await runClient(['run']); + assert.equal(result.status, 1); + assert.match(result.stdout, /Usage: gcb gh/); + }); +}); From 88c457054c2b49fc646fed14973a58e7995a161d Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 19:01:58 +0800 Subject: [PATCH 8/8] refactor: drop the branch that bare run did not need MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runGh` already answers a domain with no command by printing the catalog and returning 1, so `run` with no arguments reached that answer on its own before the branch was added — the branch ran the same code under `--help` and threw away its exit code to return the 1 it would have got anyway. The test stays: it is the regression cover for that shared path. Bare `pr` is the one that needs a branch, because the flag-only path would take it, and it now says the same thing in one line instead of a second copy of the print-and-return. --- packages/client/src/commands/gh.ts | 3 ++- packages/client/src/index.ts | 11 +++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 818c9a8a..56a35414 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -72,7 +72,8 @@ export async function runGh(argv: readonly string[]): Promise { } const [domain, command, ...rest] = argv; if (domain === undefined || command === undefined) { - // `gcb pr` and `gcb run` arrive here too, so the answer is the catalog rather than one example. + // A caller with a domain and no command (`gcb run`, `gcb gh pr`) gets the catalog rather than one + // example: the answer is every action, and the example would only be right for one of them. say(GH_USAGE); return 1; } diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index f9498b8d..e82db86a 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -59,10 +59,9 @@ async function run(argv: readonly string[]): Promise { case 'pr': { // The catalog is the one source for an action, so the shorthand asks it rather than keeping a // second list: a subcommand the catalog does not have stays with the flag-only lifecycle. - if (rest.length === 0) { - await (await import('./commands/gh.ts')).runGh(['pr', '--help']); - return 1; - } + // Bare `pr` asks the catalog too, where `run` needs no branch: the flag-only path would only + // tell it that --repo is required. + if (rest.length === 0) return (await import('./commands/gh.ts')).runGh(['pr']); const sub = rest[0]; if (sub !== undefined && Object.hasOwn(GH_ACTIONS, `pr:${sub}`)) { return (await import('./commands/gh.ts')).runGh(['pr', ...rest]); @@ -70,10 +69,6 @@ async function run(argv: readonly string[]): Promise { return (await import('./commands/pr.ts')).runPr(rest); } case 'run': - if (rest.length === 0) { - await (await import('./commands/gh.ts')).runGh(['run', '--help']); - return 1; - } return (await import('./commands/gh.ts')).runGh(['run', ...rest]); case 'gh': return (await import('./commands/gh.ts')).runGh(rest);