From 604c882d57b11b481759c7c368a386119a8ca5c1 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 12:43:45 +0000 Subject: [PATCH 01/16] feat: route plain pr comments through gh Use the catalogued pr:comment action for an unanchored comment; anchored comments keep the review-comment request. --- packages/client/src/commands/pr.ts | 10 +++++++++ test/unit/gh-command.test.ts | 35 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index f2016abd..c7c0b671 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -256,6 +256,16 @@ export async function runPr(argv: readonly string[]): Promise { return 0; } + // A plain conversation comment is gh's own path; an anchored one has no stable gh CLI and stays + // on the review-comment request below. + if (action === 'comment' && filePath === undefined) { + const ghArgs = ['pr', 'comment', String(number), '--body', body as string]; + if (host !== undefined) ghArgs.push('--host', host); + ghArgs.push('--repo', repo); + if (socketPath !== undefined) ghArgs.push('--socket', socketPath); + return (await import('./gh.ts')).runGh(ghArgs); + } + if (socketPath === undefined) { fail('no broker socket: pass --socket, set GIT_BROKER_SOCKET, or run `gcb setup`'); } diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 691f44da..f7569e7d 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -240,6 +240,41 @@ describe('gcb gh', () => { } }); + it('routes a plain pr --comment through pr:comment', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-pr-comment-')); + 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 result = await runClient([ + 'pr', '--comment', '--number', '7', '--body', 'hello', + '--repo', 'acme/widget', '--host', 'github.com', '--socket', socketPath, + ]); + assert.equal(result.status, 0); + assert.deepEqual(actions, ['pr:comment']); + } 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 dc68cb66ca0a2213bdeeac7f92e8a8dd9d147fc1 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 12:47:53 +0000 Subject: [PATCH 02/16] refactor: point plain pr comments at gh Remove the duplicate conversation-comment path instead of aliasing it; anchored comments remain the only --comment use. --- packages/client/src/commands/pr.ts | 19 +++++++--------- test/unit/commands.test.ts | 6 +++++ test/unit/gh-command.test.ts | 35 ------------------------------ 3 files changed, 14 insertions(+), 46 deletions(-) diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index c7c0b671..cfc3b16c 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -53,7 +53,8 @@ Opening (the default) Acting on one --number The pull request number [required] --status Report its state, mergeability and the runs for its commit - --comment Post the body as a comment on it (needs only --number and --body/--body-file) + --comment Post an anchored review comment (also needs --file and --line); + for a plain comment use: gcb gh pr comment --body ... --file Anchor that comment to a line of this file, repository-relative --line The line to anchor to, in the new file unless --side says otherwise --side Which side of the diff --line counts on (default: right) @@ -200,6 +201,12 @@ export async function runPr(argv: readonly string[]): Promise { fail(`--side must be left or right, got ${JSON.stringify(args.value('side'))}`); } } + if (action === 'comment' && filePath === undefined) { + fail( + 'plain comments moved to: gcb gh pr comment --body ...; ' + + '--comment here only posts an anchored comment with --file and --line', + ); + } const replyTo = args.value('reply-to'); if (action === 'reply' && (replyTo === undefined || !Number.isInteger(Number(replyTo)) || Number(replyTo) <= 0)) { fail(`--reply-to needs the id of the comment being answered, got ${JSON.stringify(replyTo)}`); @@ -256,16 +263,6 @@ export async function runPr(argv: readonly string[]): Promise { return 0; } - // A plain conversation comment is gh's own path; an anchored one has no stable gh CLI and stays - // on the review-comment request below. - if (action === 'comment' && filePath === undefined) { - const ghArgs = ['pr', 'comment', String(number), '--body', body as string]; - if (host !== undefined) ghArgs.push('--host', host); - ghArgs.push('--repo', repo); - if (socketPath !== undefined) ghArgs.push('--socket', socketPath); - return (await import('./gh.ts')).runGh(ghArgs); - } - if (socketPath === undefined) { fail('no broker socket: pass --socket, set GIT_BROKER_SOCKET, or run `gcb setup`'); } diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index c6adda73..1cedee45 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1166,6 +1166,12 @@ describe('the anchor flags, at the CLI boundary', () => { assert.match(result.stdout, /Usage: gcb gh/); }); + it('points a plain --comment at the gh action', () => { + const moved = cli([...base, '--comment', '--body', 'x']); + assert.equal(moved.status, 1); + assert.match(moved.stderr, /gcb gh pr comment/); + }); + it('points --close at the gh action', () => { const moved = cli([...base, '--close']); assert.equal(moved.status, 1); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index f7569e7d..691f44da 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -240,41 +240,6 @@ describe('gcb gh', () => { } }); - it('routes a plain pr --comment through pr:comment', async () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-pr-comment-')); - 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 result = await runClient([ - 'pr', '--comment', '--number', '7', '--body', 'hello', - '--repo', 'acme/widget', '--host', 'github.com', '--socket', socketPath, - ]); - assert.equal(result.status, 0); - assert.deepEqual(actions, ['pr:comment']); - } 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 e4da24c87c4265e72b0b80f4e6bff13e9ed98535 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 12:51:26 +0000 Subject: [PATCH 03/16] fix: do not treat a gh flag value as help Consume a known value flag's next token before parsing help, so --body --help posts the body; add a regression test. --- packages/client/src/commands/gh.ts | 34 +++++++++++++++++++++++++++--- test/unit/gh-command.test.ts | 34 ++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 56a35414..040761cf 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -66,11 +66,15 @@ ${ACTION_LINES}`; * @returns the process exit code: 0 success, 1 refusal. */ export async function runGh(argv: readonly string[]): Promise { - if (argv.length === 0 || argv.includes('--help') || argv.includes('-h')) { + if (argv.length === 0) { say(GH_USAGE); - return argv.length === 0 ? 1 : 0; + return 1; + } + const [domain, command, ...rawRest] = argv; + if (domain === '--help' || domain === '-h' || command === '--help' || command === '-h') { + say(GH_USAGE); + return 0; } - const [domain, command, ...rest] = argv; if (domain === undefined || command === undefined) { // 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. @@ -82,11 +86,35 @@ export async function runGh(argv: readonly string[]): Promise { try { action = getGhAction(actionName); } catch { + if (rawRest.includes('--help') || rawRest.includes('-h')) { + say(GH_USAGE); + return 0; + } fail(`unknown gh action ${JSON.stringify(actionName)}`); } + // A known value-taking flag consumes the next token even when it looks like an option, so a body + // that is literally `--help` is data, not a help request. The parser then sees `--body=--help`. + const valueFlags = new Set(action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name)); + const rest: string[] = []; + for (let index = 0; index < rawRest.length; index += 1) { + const argument = rawRest[index]; + const name = argument?.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; + const next = rawRest[index + 1]; + if (name !== undefined && valueFlags.has(name) && next !== undefined) { + rest.push(`${argument}=${next}`); + index += 1; + } else if (argument !== undefined) { + rest.push(argument); + } + } + const booleans = action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name); const args = parseArgs(rest, { booleans }); + if (args.has('help')) { + say(GH_USAGE); + return 0; + } const host = args.value('host') ?? 'github.com'; let repo = args.value('repo') ?? args.value('R'); if (repo === undefined) { diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 691f44da..c1402b21 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -240,6 +240,40 @@ describe('gcb gh', () => { } }); + it('does not mistake a flag value for a help request', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-body-help-')); + 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() }; + let seen: unknown; + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async (request) => { + seen = request.values; + 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 result = await runGhCli([ + 'pr', 'comment', '7', '--body', '--help', '--repo', 'acme/widget', '--socket', socketPath, + ]); + assert.equal(result.status, 0); + assert.deepEqual(seen, { number: '7', body: '--help' }); + } 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 d829d48846e8d9849f83f77f94a767cc7601c537 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 21:01:47 +0800 Subject: [PATCH 04/16] fix: answer help everywhere and name the comment replacement first Declare help a boolean in the gh parser, so a --help that follows a positional still prints the catalog; a value flag's next token is merged before parsing and stays data. Refuse a plain --comment before --repo, as --close already does, so the migration message is what the caller sees. --- packages/client/src/commands/gh.ts | 7 ++++++- packages/client/src/commands/pr.ts | 14 ++++++++------ test/unit/commands.test.ts | 4 ++++ test/unit/gh-command.test.ts | 9 +++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 040761cf..3dc2f743 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -109,7 +109,12 @@ export async function runGh(argv: readonly string[]): Promise { } } - const booleans = action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name); + // `help` is declared a boolean so it is a help request wherever it sits: a value flag's next token + // was already merged above, so `--body --help` reaches the parser as data. + const booleans = [ + ...action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name), + 'help', + ]; const args = parseArgs(rest, { booleans }); if (args.has('help')) { say(GH_USAGE); diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index cfc3b16c..15fc25bb 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -116,6 +116,14 @@ export async function runPr(argv: readonly string[]): Promise { if (args.has('close') || args.value('close') !== undefined) { fail('--close moved to: gcb gh pr close --repo owner/name'); } + // Before --repo too, for the same reason: the message names the command that does it now. An + // anchored comment (--file and --line) is still this command's to post. + if (args.has('comment') && args.value('file') === undefined && args.value('line') === undefined) { + fail( + 'plain comments moved to: gcb gh pr comment --body ...; ' + + '--comment here only posts an anchored comment with --file and --line', + ); + } const socketPath = resolveSocketPath(args.value('socket'), process.env); const host = args.value('host'); @@ -201,12 +209,6 @@ export async function runPr(argv: readonly string[]): Promise { fail(`--side must be left or right, got ${JSON.stringify(args.value('side'))}`); } } - if (action === 'comment' && filePath === undefined) { - fail( - 'plain comments moved to: gcb gh pr comment --body ...; ' + - '--comment here only posts an anchored comment with --file and --line', - ); - } const replyTo = args.value('reply-to'); if (action === 'reply' && (replyTo === undefined || !Number.isInteger(Number(replyTo)) || Number(replyTo) <= 0)) { fail(`--reply-to needs the id of the comment being answered, got ${JSON.stringify(replyTo)}`); diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index 1cedee45..0f19c234 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1170,6 +1170,10 @@ describe('the anchor flags, at the CLI boundary', () => { const moved = cli([...base, '--comment', '--body', 'x']); assert.equal(moved.status, 1); assert.match(moved.stderr, /gcb gh pr comment/); + + const bare = cli(['--comment']); + assert.equal(bare.status, 1); + assert.match(bare.stderr, /gcb gh pr comment/, 'before --repo, so the message names the replacement'); }); it('points --close at the gh action', () => { diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index c1402b21..5dfcc1cb 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -274,6 +274,15 @@ describe('gcb gh', () => { } }); + it('treats --help as help wherever it sits', async () => { + // No broker is involved: each case has to answer before the socket is looked for. + for (const argv of [['--help'], ['pr', 'view', '--help', '7'], ['pr', 'view', '7', '--help']]) { + const result = await runGhCli(argv); + assert.equal(result.status, 0, argv.join(' ')); + assert.match(result.stdout, /Usage: gcb gh/); + } + }); + 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 760964df8c8762ca4ed28f7016184b7d35032db0 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 13:08:26 +0000 Subject: [PATCH 05/16] refactor: drop the plain-comment migration hint Keep the early refusal, but explain the anchor requirement instead of naming the old command. --- packages/client/src/commands/pr.ts | 5 +---- test/unit/commands.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index 15fc25bb..babb56e5 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -119,10 +119,7 @@ export async function runPr(argv: readonly string[]): Promise { // Before --repo too, for the same reason: the message names the command that does it now. An // anchored comment (--file and --line) is still this command's to post. if (args.has('comment') && args.value('file') === undefined && args.value('line') === undefined) { - fail( - 'plain comments moved to: gcb gh pr comment --body ...; ' + - '--comment here only posts an anchored comment with --file and --line', - ); + fail('--comment only posts an anchored review comment; pass --file and --line'); } const socketPath = resolveSocketPath(args.value('socket'), process.env); diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index 0f19c234..f53c44a9 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1169,11 +1169,11 @@ describe('the anchor flags, at the CLI boundary', () => { it('points a plain --comment at the gh action', () => { const moved = cli([...base, '--comment', '--body', 'x']); assert.equal(moved.status, 1); - assert.match(moved.stderr, /gcb gh pr comment/); + assert.match(moved.stderr, /anchored review comment/); const bare = cli(['--comment']); assert.equal(bare.status, 1); - assert.match(bare.stderr, /gcb gh pr comment/, 'before --repo, so the message names the replacement'); + assert.match(bare.stderr, /anchored review comment/, 'before --repo too'); }); it('points --close at the gh action', () => { From 4656d99251399d0481a49c6c9301d6f386698850 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 21:14:47 +0800 Subject: [PATCH 06/16] docs: match the comment refusal with its message The ordering comment and the test name still described the migration hint this branch dropped; both now describe the anchor requirement. --- packages/client/src/commands/pr.ts | 4 ++-- test/unit/commands.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/client/src/commands/pr.ts b/packages/client/src/commands/pr.ts index babb56e5..729409bf 100644 --- a/packages/client/src/commands/pr.ts +++ b/packages/client/src/commands/pr.ts @@ -116,8 +116,8 @@ export async function runPr(argv: readonly string[]): Promise { if (args.has('close') || args.value('close') !== undefined) { fail('--close moved to: gcb gh pr close --repo owner/name'); } - // Before --repo too, for the same reason: the message names the command that does it now. An - // anchored comment (--file and --line) is still this command's to post. + // Before --repo too: a bare `gcb pr --comment` should hear what the flag needs, not that --repo + // is missing. An anchored comment (--file and --line) is still this command's to post. if (args.has('comment') && args.value('file') === undefined && args.value('line') === undefined) { fail('--comment only posts an anchored review comment; pass --file and --line'); } diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts index f53c44a9..de423f67 100644 --- a/test/unit/commands.test.ts +++ b/test/unit/commands.test.ts @@ -1166,7 +1166,7 @@ describe('the anchor flags, at the CLI boundary', () => { assert.match(result.stdout, /Usage: gcb gh/); }); - it('points a plain --comment at the gh action', () => { + it('refuses a plain --comment and names the anchor requirement', () => { const moved = cli([...base, '--comment', '--body', 'x']); assert.equal(moved.status, 1); assert.match(moved.stderr, /anchored review comment/); From 341aa88e55cd35ef9bc0b235193e088e49b7e9f9 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 14:18:08 +0000 Subject: [PATCH 07/16] fix: reject a known flag as a missing value Do not merge a following known flag into a value flag; keep --body --help as data and fail --title --draft as a missing title. --- packages/client/src/commands/gh.ts | 9 ++++++--- test/unit/gh-command.test.ts | 11 +++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 3dc2f743..15b9ff83 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -93,15 +93,18 @@ export async function runGh(argv: readonly string[]): Promise { fail(`unknown gh action ${JSON.stringify(actionName)}`); } - // A known value-taking flag consumes the next token even when it looks like an option, so a body - // that is literally `--help` is data, not a help request. The parser then sees `--body=--help`. + // A known value-taking flag consumes the next token only when that token is a value. A following + // flag is left alone (so a missing value fails as missing), except a literal --help/-h: a body that + // is exactly one of those is data, not a help request. The parser then sees --body=--help. const valueFlags = new Set(action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name)); const rest: string[] = []; for (let index = 0; index < rawRest.length; index += 1) { const argument = rawRest[index]; const name = argument?.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; const next = rawRest[index + 1]; - if (name !== undefined && valueFlags.has(name) && next !== undefined) { + const nextIsFlag = next !== undefined && /^--?[A-Za-z]/.test(next); + const literalHelp = next === '--help' || next === '-h'; + if (name !== undefined && valueFlags.has(name) && next !== undefined && (!nextIsFlag || literalHelp)) { rest.push(`${argument}=${next}`); index += 1; } else if (argument !== undefined) { diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 5dfcc1cb..eeb9c1d8 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -323,3 +323,14 @@ describe('bare run', () => { assert.match(result.stdout, /Usage: gcb gh/); }); }); + +describe('gh option values', () => { + it('refuses a value flag followed by another flag', async () => { + const result = await runGhCli([ + 'pr', 'create', '--head', 'feat', '--title', '--draft', '--body', 'b', + '--repo', 'acme/widget', '--socket', '/nonexistent', + ]); + assert.equal(result.status, 1); + assert.match(result.stderr, /needs --title/); + }); +}); From 8332c1e0ade6c08b3bb6539c910b16254d2a6da7 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 15:29:11 +0000 Subject: [PATCH 08/16] fix: refuse a missing optional flag value A value flag followed by a known flag now fails, including optional flags such as --reviewer before --draft; single-dash operands and literal --help/-h remain values. --- packages/client/src/commands/gh.ts | 23 +++++++++++++++++++++++ test/unit/gh-command.test.ts | 11 ++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 15b9ff83..174704d3 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -97,6 +97,29 @@ export async function runGh(argv: readonly string[]): Promise { // flag is left alone (so a missing value fails as missing), except a literal --help/-h: a body that // is exactly one of those is data, not a help request. The parser then sees --body=--help. const valueFlags = new Set(action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name)); + + const knownFlags = new Set([ + ...action.flags.map((flag) => flag.name), + 'repo', + 'R', + 'host', + 'socket', + 'help', + ]); + for (let index = 0; index < rawRest.length; index += 1) { + const argument = rawRest[index]; + if (argument === undefined || !argument.startsWith('--') || argument.includes('=')) continue; + const name = argument.slice(2); + if (!valueFlags.has(name)) continue; + const next = rawRest[index + 1]; + if (next === undefined) fail(`--${name} needs a value`); + if (next === '--help' || next === '-h') continue; + if (next.startsWith('--')) { + const nextName = next.slice(2).split('=')[0]; + if (nextName !== undefined && knownFlags.has(nextName)) fail(`--${name} needs a value`); + } + } + const rest: string[] = []; for (let index = 0; index < rawRest.length; index += 1) { const argument = rawRest[index]; diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index eeb9c1d8..05941dac 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -325,12 +325,21 @@ describe('bare run', () => { }); describe('gh option values', () => { + it('refuses an optional value flag followed by another flag', async () => { + const result = await runGhCli([ + 'pr', 'create', '--head', 'feat', '--title', 't', '--body', 'b', '--reviewer', '--draft', + '--repo', 'acme/widget', '--socket', '/nonexistent', + ]); + assert.equal(result.status, 1); + assert.match(result.stderr, /--reviewer needs a value/); + }); + it('refuses a value flag followed by another flag', async () => { const result = await runGhCli([ 'pr', 'create', '--head', 'feat', '--title', '--draft', '--body', 'b', '--repo', 'acme/widget', '--socket', '/nonexistent', ]); assert.equal(result.status, 1); - assert.match(result.stderr, /needs --title/); + assert.match(result.stderr, /--title needs a value/); }); }); From 3ff2725ce0d87ecf3b3dff7dc10ca9efc2290911 Mon Sep 17 00:00:00 2001 From: JounQin Date: Wed, 28 Oct 2026 00:41:26 +0800 Subject: [PATCH 09/16] refactor: derive the known flags from the allowed set The missing-value check kept a second copy of the flag list, which could drift from the one the unknown-option check uses. Build one set and add help to it. --- packages/client/src/commands/gh.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 174704d3..f509a100 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -94,18 +94,19 @@ export async function runGh(argv: readonly string[]): Promise { } // A known value-taking flag consumes the next token only when that token is a value. A following - // flag is left alone (so a missing value fails as missing), except a literal --help/-h: a body that - // is exactly one of those is data, not a help request. The parser then sees --body=--help. + // known flag is not taken as a value (so a missing value fails as missing), except a literal + // --help/-h: a body that is exactly one of those is data, not a help request. The parser then + // sees --body=--help. const valueFlags = new Set(action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name)); - const knownFlags = new Set([ + const allowed = new Set([ ...action.flags.map((flag) => flag.name), 'repo', 'R', 'host', 'socket', - 'help', ]); + const knownFlags = new Set([...allowed, 'help']); for (let index = 0; index < rawRest.length; index += 1) { const argument = rawRest[index]; if (argument === undefined || !argument.startsWith('--') || argument.includes('=')) continue; @@ -159,13 +160,6 @@ export async function runGh(argv: readonly string[]): Promise { const socketPath = resolveSocketPath(args.value('socket'), process.env); if (socketPath === undefined) fail('no broker socket is configured'); - const allowed = new Set([ - ...action.flags.map((flag) => flag.name), - 'repo', - 'R', - 'host', - 'socket', - ]); for (const argument of rest) { const match = /^--([^=]+)/.exec(argument); if (match?.[1] !== undefined && !allowed.has(match[1])) { From e5762d920ca0b3be588552ae50b098c1b0f5770b Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 16:50:59 +0000 Subject: [PATCH 10/16] fix: let help win after a value flag A literal --help stays a body value, but for other value flags it is an unconsumed help request rather than a missing value. --- packages/client/src/commands/gh.ts | 8 +++++++- test/unit/gh-command.test.ts | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index f509a100..18a0bdd6 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -114,7 +114,13 @@ export async function runGh(argv: readonly string[]): Promise { if (!valueFlags.has(name)) continue; const next = rawRest[index + 1]; if (next === undefined) fail(`--${name} needs a value`); - if (next === '--help' || next === '-h') continue; + if (next === '--help' || next === '-h') { + // A body is free text and may literally be --help; every other value flag leaves it as an + // unconsumed request so the help wins over the missing value. + if (name === 'body') continue; + say(GH_USAGE); + return 0; + } if (next.startsWith('--')) { const nextName = next.slice(2).split('=')[0]; if (nextName !== undefined && knownFlags.has(nextName)) fail(`--${name} needs a value`); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 05941dac..7654a7cd 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -343,3 +343,14 @@ describe('gh option values', () => { assert.match(result.stderr, /--title needs a value/); }); }); + +describe('help after a value flag', () => { + it('prints help instead of a missing-value error', async () => { + const result = await runGhCli([ + 'pr', 'create', '--title', '--help', '--head', 'feat', '--body', 'b', + '--repo', 'acme/widget', '--socket', '/nonexistent', + ]); + assert.equal(result.status, 0); + assert.match(result.stdout, /Usage: gcb gh/); + }); +}); From 49e82a73860bf373faf2808fae846bf7ea37bcc4 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 18:37:40 +0000 Subject: [PATCH 11/16] fix: preserve the pr transport code and help ordering The pr shorthand now returns 3 when the broker is unreachable, and help is resolved before missing-value validation so a later --help wins. --- packages/client/src/commands/gh.ts | 46 +++++++++++++++--------------- packages/client/src/index.ts | 2 +- test/unit/gh-command.test.ts | 13 ++++++++- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 18a0bdd6..9bc7332e 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -63,9 +63,12 @@ ${ACTION_LINES}`; * Run the `gh` command. * * @param argv - arguments after `gh`. - * @returns the process exit code: 0 success, 1 refusal. + * @returns the process exit code: 0 success, 1 refusal, 3 broker unreachable. */ -export async function runGh(argv: readonly string[]): Promise { +export async function runGh( + argv: readonly string[], + options: { readonly unreachableExitCode?: number } = {}, +): Promise { if (argv.length === 0) { say(GH_USAGE); return 1; @@ -107,25 +110,6 @@ export async function runGh(argv: readonly string[]): Promise { 'socket', ]); const knownFlags = new Set([...allowed, 'help']); - for (let index = 0; index < rawRest.length; index += 1) { - const argument = rawRest[index]; - if (argument === undefined || !argument.startsWith('--') || argument.includes('=')) continue; - const name = argument.slice(2); - if (!valueFlags.has(name)) continue; - const next = rawRest[index + 1]; - if (next === undefined) fail(`--${name} needs a value`); - if (next === '--help' || next === '-h') { - // A body is free text and may literally be --help; every other value flag leaves it as an - // unconsumed request so the help wins over the missing value. - if (name === 'body') continue; - say(GH_USAGE); - return 0; - } - if (next.startsWith('--')) { - const nextName = next.slice(2).split('=')[0]; - if (nextName !== undefined && knownFlags.has(nextName)) fail(`--${name} needs a value`); - } - } const rest: string[] = []; for (let index = 0; index < rawRest.length; index += 1) { @@ -133,7 +117,7 @@ export async function runGh(argv: readonly string[]): Promise { const name = argument?.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; const next = rawRest[index + 1]; const nextIsFlag = next !== undefined && /^--?[A-Za-z]/.test(next); - const literalHelp = next === '--help' || next === '-h'; + const literalHelp = (next === '--help' || next === '-h') && name === 'body'; if (name !== undefined && valueFlags.has(name) && next !== undefined && (!nextIsFlag || literalHelp)) { rest.push(`${argument}=${next}`); index += 1; @@ -153,6 +137,21 @@ export async function runGh(argv: readonly string[]): Promise { say(GH_USAGE); return 0; } + + for (let index = 0; index < rawRest.length; index += 1) { + const argument = rawRest[index]; + if (argument === undefined || !argument.startsWith('--') || argument.includes('=')) continue; + const name = argument.slice(2); + if (!valueFlags.has(name)) continue; + const next = rawRest[index + 1]; + if (next === undefined) fail(`--${name} needs a value`); + if (name === 'body' && (next === '--help' || next === '-h')) continue; + if (next.startsWith('--')) { + const nextName = next.slice(2).split('=')[0]; + if (nextName !== undefined && knownFlags.has(nextName)) fail(`--${name} needs a value`); + } + } + const host = args.value('host') ?? 'github.com'; let repo = args.value('repo') ?? args.value('R'); if (repo === undefined) { @@ -207,7 +206,8 @@ export async function runGh(argv: readonly string[]): Promise { action.timeoutSeconds * 1000 + 30_000, ); } catch (error) { - fail((error as Error).message); + process.stderr.write(`unreachable: ${(error as Error).message}\n`); + return options.unreachableExitCode ?? 1; } if (response.ok !== true) { if (response.warning) process.stderr.write(`warning: ${response.warning}\n`); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index e82db86a..9a896ef0 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -64,7 +64,7 @@ async function run(argv: readonly string[]): Promise { 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]); + return (await import('./commands/gh.ts')).runGh(['pr', ...rest], { unreachableExitCode: 3 }); } return (await import('./commands/pr.ts')).runPr(rest); } diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 7654a7cd..b69dd538 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -276,13 +276,24 @@ describe('gcb gh', () => { it('treats --help as help wherever it sits', async () => { // No broker is involved: each case has to answer before the socket is looked for. - for (const argv of [['--help'], ['pr', 'view', '--help', '7'], ['pr', 'view', '7', '--help']]) { + for (const argv of [ + ['--help'], + ['pr', 'view', '--help', '7'], + ['pr', 'view', '7', '--help'], + ['pr', 'create', '--title', '--head', '--help'], + ]) { const result = await runGhCli(argv); assert.equal(result.status, 0, argv.join(' ')); assert.match(result.stdout, /Usage: gcb gh/); } }); + it('keeps the pr unreachable exit code', async () => { + const result = await runClient(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', '/nonexistent']); + assert.equal(result.status, 3); + assert.match(result.stderr, /unreachable/); + }); + 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 42c68aaaa02c19d65f244a37d5fb8cbb0ee4a68b Mon Sep 17 00:00:00 2001 From: JounQin Date: Wed, 28 Oct 2026 02:42:50 +0800 Subject: [PATCH 12/16] fix: keep -h working after a value flag Resolving help after parseArgs lost the -h spelling: parseArgs normalises only the token it reads at flag position, so a value flag's next -h was consumed as that value while --help still won. Normalise -h in the merge pass too; a body's -h is merged before that line. Extend the help and literal-body tests. --- packages/client/src/commands/gh.ts | 8 ++++++-- test/unit/gh-command.test.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 9bc7332e..f419797b 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -63,7 +63,8 @@ ${ACTION_LINES}`; * Run the `gh` command. * * @param argv - arguments after `gh`. - * @returns the process exit code: 0 success, 1 refusal, 3 broker unreachable. + * @returns the process exit code: 0 success, 1 refusal, and the requested unreachable code + * (1 by default; `pr` passes 3 for runPr's contract). */ export async function runGh( argv: readonly string[], @@ -113,7 +114,10 @@ export async function runGh( const rest: string[] = []; for (let index = 0; index < rawRest.length; index += 1) { - const argument = rawRest[index]; + // `-h` is help in flag position, and parseArgs normalises only the token it reads at that + // position: normalise it here too, or a value flag's next `-h` is taken as its value. A body's + // `-h` was merged into `--body=-h` above and never reaches this line. + const argument = rawRest[index]?.replace(/^-h$/, '--help'); const name = argument?.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; const next = rawRest[index + 1]; const nextIsFlag = next !== undefined && /^--?[A-Za-z]/.test(next); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index b69dd538..f38a9ba9 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -268,6 +268,12 @@ describe('gcb gh', () => { ]); assert.equal(result.status, 0); assert.deepEqual(seen, { number: '7', body: '--help' }); + + const shortHelp = await runGhCli([ + 'pr', 'comment', '7', '--body', '-h', '--repo', 'acme/widget', '--socket', socketPath, + ]); + assert.equal(shortHelp.status, 0); + assert.deepEqual(seen, { number: '7', body: '-h' }, 'a body of literally -h is data too'); } finally { await broker.close(); fs.rmSync(root, { recursive: true, force: true }); @@ -280,7 +286,9 @@ describe('gcb gh', () => { ['--help'], ['pr', 'view', '--help', '7'], ['pr', 'view', '7', '--help'], + ['pr', 'view', '7', '-h'], ['pr', 'create', '--title', '--head', '--help'], + ['pr', 'create', '--title', '-h', '--head', 'feat', '--body', 'b'], ]) { const result = await runGhCli(argv); assert.equal(result.status, 0, argv.join(' ')); From 150a8c963f17194ace12ce1b056020791f50790c Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 18:57:59 +0000 Subject: [PATCH 13/16] refactor: scan gh options in one pass Fold value merging, help discovery and missing-value checks into one loop, keep unknown flags unknown, and treat common repo/host/socket flags as value-taking. --- packages/client/src/commands/gh.ts | 84 ++++++++++++++++++------------ test/unit/gh-command.test.ts | 11 ++++ 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index f419797b..594a5b3e 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -101,7 +101,13 @@ export async function runGh( // known flag is not taken as a value (so a missing value fails as missing), except a literal // --help/-h: a body that is exactly one of those is data, not a help request. The parser then // sees --body=--help. - const valueFlags = new Set(action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name)); + const valueFlags = new Set([ + ...action.flags.filter((flag) => flag.type !== 'boolean').map((flag) => flag.name), + 'repo', + 'R', + 'host', + 'socket', + ]); const allowed = new Set([ ...action.flags.map((flag) => flag.name), @@ -113,48 +119,62 @@ export async function runGh( const knownFlags = new Set([...allowed, 'help']); const rest: string[] = []; + let missingValue: string | undefined; + let help = false; for (let index = 0; index < rawRest.length; index += 1) { - // `-h` is help in flag position, and parseArgs normalises only the token it reads at that - // position: normalise it here too, or a value flag's next `-h` is taken as its value. A body's - // `-h` was merged into `--body=-h` above and never reaches this line. - const argument = rawRest[index]?.replace(/^-h$/, '--help'); - const name = argument?.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; + const raw = rawRest[index]; + const argument = raw?.replace(/^-h$/, '--help'); + if (argument === undefined) continue; + if (argument === '--help') { + help = true; + rest.push(argument); + continue; + } + const name = argument.startsWith('--') && !argument.includes('=') ? argument.slice(2) : undefined; + if (name === undefined || !valueFlags.has(name)) { + rest.push(argument); + continue; + } const next = rawRest[index + 1]; - const nextIsFlag = next !== undefined && /^--?[A-Za-z]/.test(next); - const literalHelp = (next === '--help' || next === '-h') && name === 'body'; - if (name !== undefined && valueFlags.has(name) && next !== undefined && (!nextIsFlag || literalHelp)) { - rest.push(`${argument}=${next}`); - index += 1; - } else if (argument !== undefined) { + if (next === undefined) { + missingValue ??= `--${name} needs a value`; + rest.push(argument); + continue; + } + if (next === '--help' || next === '-h') { + if (name === 'body') { + rest.push(`${argument}=${next}`); + index += 1; + } else { + help = true; + rest.push(argument); + } + continue; + } + if (next.startsWith('--')) { + const nextName = next.slice(2).split('=')[0]; + if (nextName !== undefined && knownFlags.has(nextName)) { + missingValue ??= `--${name} needs a value`; + } + // Leave both tokens alone: a known flag is a missing value, an unknown one must reach the + // unknown-option check rather than becoming a value. rest.push(argument); + continue; } + rest.push(`${argument}=${next}`); + index += 1; + } + if (help) { + say(GH_USAGE); + return 0; } + if (missingValue !== undefined) fail(missingValue); - // `help` is declared a boolean so it is a help request wherever it sits: a value flag's next token - // was already merged above, so `--body --help` reaches the parser as data. const booleans = [ ...action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name), 'help', ]; const args = parseArgs(rest, { booleans }); - if (args.has('help')) { - say(GH_USAGE); - return 0; - } - - for (let index = 0; index < rawRest.length; index += 1) { - const argument = rawRest[index]; - if (argument === undefined || !argument.startsWith('--') || argument.includes('=')) continue; - const name = argument.slice(2); - if (!valueFlags.has(name)) continue; - const next = rawRest[index + 1]; - if (next === undefined) fail(`--${name} needs a value`); - if (name === 'body' && (next === '--help' || next === '-h')) continue; - if (next.startsWith('--')) { - const nextName = next.slice(2).split('=')[0]; - if (nextName !== undefined && knownFlags.has(nextName)) fail(`--${name} needs a value`); - } - } const host = args.value('host') ?? 'github.com'; let repo = args.value('repo') ?? args.value('R'); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index f38a9ba9..74c3810d 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -373,3 +373,14 @@ describe('help after a value flag', () => { assert.match(result.stdout, /Usage: gcb gh/); }); }); + +describe('unknown gh options', () => { + it('does not let an unknown flag become a value', async () => { + const result = await runGhCli([ + 'pr', 'comment', '7', '--body', 'x', '--bogus', 'y', + '--repo', 'acme/widget', '--socket', '/nonexistent', + ]); + assert.equal(result.status, 1); + assert.match(result.stderr, /unknown option --bogus/); + }); +}); From b8388e8da8bc1b5ea0b16302793916feb0cdf66c Mon Sep 17 00:00:00 2001 From: JounQin Date: Wed, 28 Oct 2026 03:58:35 +0800 Subject: [PATCH 14/16] fix: handle short gh flags and drop the unused JSON flag --- packages/client/src/commands/gh.ts | 13 ++++++------- test/unit/gh-command.test.ts | 8 ++++++++ tsconfig.build.base.json | 1 - tsconfig.json | 1 - 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 594a5b3e..65694016 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -123,7 +123,9 @@ export async function runGh( let help = false; for (let index = 0; index < rawRest.length; index += 1) { const raw = rawRest[index]; - const argument = raw?.replace(/^-h$/, '--help'); + // Match parseArgs: -h is help everywhere; any other single-dash letter is its long flag. A + // single-dash operand belongs to a preceding value flag and was consumed before this point. + const argument = raw?.replace(/^-h$/, '--help').replace(/^-(?=[A-Za-z])/, '--'); if (argument === undefined) continue; if (argument === '--help') { help = true; @@ -137,7 +139,7 @@ export async function runGh( } const next = rawRest[index + 1]; if (next === undefined) { - missingValue ??= `--${name} needs a value`; + missingValue ??= `${raw} needs a value`; rest.push(argument); continue; } @@ -154,7 +156,7 @@ export async function runGh( if (next.startsWith('--')) { const nextName = next.slice(2).split('=')[0]; if (nextName !== undefined && knownFlags.has(nextName)) { - missingValue ??= `--${name} needs a value`; + missingValue ??= `${raw} needs a value`; } // Leave both tokens alone: a known flag is a missing value, an unknown one must reach the // unknown-option check rather than becoming a value. @@ -170,10 +172,7 @@ export async function runGh( } if (missingValue !== undefined) fail(missingValue); - const booleans = [ - ...action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name), - 'help', - ]; + const booleans = action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name); const args = parseArgs(rest, { booleans }); const host = args.value('host') ?? 'github.com'; diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 74c3810d..1cf729e2 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -384,3 +384,11 @@ describe('unknown gh options', () => { assert.match(result.stderr, /unknown option --bogus/); }); }); + +describe('short gh flags', () => { + it('reports a missing -R value using the typed spelling', async () => { + const result = await runGhCli(['pr', 'view', '7', '-R']); + assert.equal(result.status, 1); + assert.match(result.stderr, /-R needs a value/); + }); +}); diff --git a/tsconfig.build.base.json b/tsconfig.build.base.json index 5f56957c..36222007 100644 --- a/tsconfig.build.base.json +++ b/tsconfig.build.base.json @@ -4,7 +4,6 @@ "lib": ["ES2023"], "module": "nodenext", "moduleResolution": "nodenext", - "resolveJsonModule": true, "types": ["node"], "strict": true, diff --git a/tsconfig.json b/tsconfig.json index c284a8b4..312c1237 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,6 @@ "lib": ["ES2023"], "module": "nodenext", "moduleResolution": "nodenext", - "resolveJsonModule": true, "types": ["node"], "strict": true, From 0077b231ec162a42aef89c2ceccdc7188f94454a Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 20:19:02 +0000 Subject: [PATCH 15/16] fix: honour the -- terminator in gh options Stop help detection and unknown-option scanning at --, so a later --help is a positional. --- packages/client/src/commands/gh.ts | 5 +++++ test/unit/gh-command.test.ts | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 65694016..31c8dc7f 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -127,6 +127,10 @@ export async function runGh( // single-dash operand belongs to a preceding value flag and was consumed before this point. const argument = raw?.replace(/^-h$/, '--help').replace(/^-(?=[A-Za-z])/, '--'); if (argument === undefined) continue; + if (argument === '--') { + rest.push(...rawRest.slice(index)); + break; + } if (argument === '--help') { help = true; rest.push(argument); @@ -189,6 +193,7 @@ export async function runGh( if (socketPath === undefined) fail('no broker socket is configured'); for (const argument of rest) { + if (argument === '--') break; const match = /^--([^=]+)/.exec(argument); if (match?.[1] !== undefined && !allowed.has(match[1])) { fail(`unknown option --${match[1]} for gcb gh ${domain} ${command}`); diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 1cf729e2..0bf60fd0 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -392,3 +392,12 @@ describe('short gh flags', () => { assert.match(result.stderr, /-R needs a value/); }); }); + +describe('the -- terminator', () => { + it('does not treat the token after -- as help', async () => { + const result = await runGhCli(['pr', 'view', '7', '--', '--help']); + assert.equal(result.status, 1); + assert.doesNotMatch(result.stdout, /Usage: gcb gh/); + assert.match(result.stderr, /too many positional arguments/); + }); +}); From 54a92f8d4ae796c9a7c394947acc9cbef58781d3 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 20:28:25 +0000 Subject: [PATCH 16/16] test: make the -- terminator case independent of the machine Pass the repo and socket explicitly so the extra positional is the error, not a missing local checkout or socket. --- test/unit/gh-command.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 0bf60fd0..11b59a6f 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -395,7 +395,10 @@ describe('short gh flags', () => { describe('the -- terminator', () => { it('does not treat the token after -- as help', async () => { - const result = await runGhCli(['pr', 'view', '7', '--', '--help']); + const result = await runGhCli([ + 'pr', 'view', '7', '--repo', 'acme/widget', '--socket', '/nonexistent', + '--', '--help', + ]); assert.equal(result.status, 1); assert.doesNotMatch(result.stdout, /Usage: gcb gh/); assert.match(result.stderr, /too many positional arguments/);