feat: route pr shorthand and close through gh

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 <number>` (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.
 

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 <subcommand>` and `gcb run <subcommand>` 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`.
 

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 <id> --log` replaces `logs`.
 - [x] Build the command table. Every entry has:
       - canonical subcommand (no aliases, no abbreviations);
@@ -261,6 +262,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` |

packages/cli/src/actions.ts

@@ -152,6 +152,17 @@ const PR_COMMENT: GhAction = {
   hostUser: true,
 };
 
+// GitHub GraphQL: closePullRequest
+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<Record<string, GhAction>> = 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,

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 = [

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'

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 <domain> <command> [options]
 
 Run a catalogued GitHub action through the broker.
+gcb pr <subcommand> and gcb run <subcommand> are shorthands for the same actions.
 
 Common
   -R, --repo <owner/name>  Repository the action belongs to (default: origin remote)
@@ -71,7 +72,10 @@ export async function runGh(argv: readonly string[]): Promise<number> {
   }
   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');
+    // 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;
   }
   const actionName = `${domain}:${command}`;
   let action;

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 <subcommand>` 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 <branch>      The branch holding the change                 [required]
                        May be <remote>:<branch>, e.g. origin:feat/x, to name a fork
@@ -56,7 +62,6 @@ Acting on one
   --edit <id>          Replace the body of that review comment, with --body/--body-file
                        (an id --threads prints; no --number is needed)
   --resolve <thread>   Mark that review thread resolved (the id --threads prints)
-  --close              Close it
   --merge              Merge it (branch protection still applies)
   --method <m>         merge, squash (default) or rebase
   With --number and none of the above, the title, body or base is updated.
@@ -73,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
@@ -87,7 +92,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,12 +105,16 @@ function actionOf(args: { has(name: string): boolean; value(name: string): strin
  */
 export async function runPr(argv: readonly string[]): Promise<number> {
   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);
     return 0;
   }
+  // Before --repo: the point of the message is to name the command that does it now.
+  if (args.has('close') || args.value('close') !== undefined) {
+    fail('--close moved to: gcb gh pr close <number> --repo owner/name');
+  }
 
   const socketPath = resolveSocketPath(args.value('socket'), process.env);
   const host = args.value('host');
@@ -296,9 +304,7 @@ export async function runPr(argv: readonly string[]): Promise<number> {
   const done =
     action === 'open'
       ? 'opened'
-      : action === 'close'
-        ? 'closed'
-        : action === 'merge'
+      : action === 'merge'
           ? 'merged'
           : action === 'comment'
             ? 'commented on'

packages/client/src/index.ts

@@ -3,10 +3,27 @@
  *
  * 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';
 
+/**
+ * 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.
 
 Usage: gcb <command> [options]
@@ -15,7 +32,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 lifecycle; these run as gh actions:
+             ${cataloguedSubcommands('pr')}
+  run        Run a workflow run action: ${cataloguedSubcommands('run')}
   gh         Run a catalogued GitHub action through the broker
 
 As a git credential helper
@@ -37,8 +56,20 @@ async function run(argv: readonly string[]): Promise<number> {
       return (await import('./commands/probe.ts')).runProbe(rest);
     case 'logs':
       return (await import('./commands/logs.ts')).runLogs(rest);
-    case 'pr':
+    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.
+      // 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]);
+      }
       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 +80,7 @@ async function run(argv: readonly string[]): Promise<number> {
 
 export const clientBackend: CommandBackend = {
   role: 'client',
-  commands: ['setup', 'probe', 'logs', 'pr', 'gh'],
+  commands: ['setup', 'probe', 'logs', 'pr', 'run', 'gh'],
   help: CLIENT_HELP,
   run,
 };

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' &&

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<string, unknown> = { 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 };

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', () => {

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');
   });
 

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 ?? ''),
+      };
     }
   };
 
@@ -1143,11 +1147,35 @@ 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('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);
+    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', () => {
     const side = cli([...base, '--comment', '--file', 'Dockerfile', '--line', '4', '--side', 'middle', '--body', 'x']);
     assert.match(side.stderr, /--side must be left or right/);