feat: unify person routes for reviews and edits

.changeset/host-review.md

@@ -0,0 +1,13 @@
+---
+'gcb-client': minor
+'gcb-core': minor
+'gcb-host': minor
+---
+
+Let a person submit a pull request review through the broker: bare `--as-user` uses the broker's
+authorized person through the app broker, while `--as-user <login>` writes a typed host request,
+checked against the broker allowlist, that runs `gh pr review` without a shell. `--approve`,
+`--request-changes` and `--review-comment` are the review events. `pr:edit` also supports the
+same person identities, so a pull request's title, body or base can be changed through the broker.
+The catalog also gains `issue:edit` and `issue:comment`, with the same app/person identity
+handling.

AGENTS.md

@@ -210,10 +210,11 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p
   person) by default, or `pr create --as-user <login>` for a specific host account; the latter writes
   a request under `<git dir>/gcb/pr-requests` (shared with the host, untracked) for `gcb host`.
   A plain `gcb pr create` (app identity) is the exception, only when the operator explicitly asks for
-  an App-authored PR; automated reviewers may skip those. The flag-only `gcb pr --as-user` form is
-  for reply, edit, comment and resolve, not for opening. Bare `issue create --as-user` uses the
-  broker's person action token; `issue create --as-user <login>` uses the host broker. It runs no
-  shell, its program is configuration, and it cannot push, merge, close or read.
+  an App-authored PR; automated reviewers may skip those. The flag-only `gcb pr --as-user` form also
+  opens with `--head/--title`, and covers review, reply, PR edit, review-comment edit/comment and
+  resolve. Bare `issue create/edit/comment --as-user` uses the broker's person action token; the login
+  form uses the host broker. It runs no shell, its program is configuration, and it cannot push,
+  merge, close or read.
 
   The host broker is started once per machine, not once per repository: it discovers the checkouts under
   its `--root`s (default `$HOME`) within `--depth` levels of each (4 by default), so anything added
@@ -238,9 +239,9 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p
 
   Authorization is not a token to place: `authorize` runs the device flow once from a browser
   somewhere else and leaves a refresh token beside the key, which the broker renews by itself. The user
-  token is used where the person's identity is required or preferred: authoring a pull request,
-  resolving a review thread (GraphQL refuses that for an app), replying, editing or commenting on a
-  review, and creating an issue through the action path. It needs the client id
+  token is used where the person's identity is required or preferred: authoring or editing a pull
+  request, submitting a review, resolving a review thread (GraphQL refuses that for an app), replying,
+  editing or commenting on a review, and creating an issue through the action path. It needs the client id
   in the configuration and **Enable Device Flow** in the app settings, and no client secret at all. The
   refresh token is rotated on every exchange and written back; not writing it back would work once.
 
@@ -257,15 +258,16 @@ sources must stay erasable (`erasableSyntaxOnly` is on: no enums, no parameter p
   listing the installations and the repositories each one reaches; asking for a credential for a
   repository the app *is* installed on is the control that needs no such command.
 
-  **Which actions are left is the operational half.** The host broker creates pull requests/issues
-  and replies to review comments (with `--as-user <login>`), while bare `--as-user` through the
-  broker also handles reply, resolve, edit and comment. Every other action still goes through the
-  installation token.
-  On a repository the app is not installed on, a pull request can be opened and its review comments
-  replied to, edited or resolved with bare `--as-user`, but the pull request itself cannot be edited,
-  closed or merged, and neither the container nor the host broker has a route to its metadata. That is not a gap to close with a flag: acting as a
-  person, from a checkout on the host that holds the person's own credentials or an allowlisted explicit
-  repository, is the design.
+  **Which actions are left is the operational half.** The host broker creates pull requests/issues,
+  submits reviews, replies to review comments, edits pull requests and edits or comments on issues
+  (with `--as-user <login>`),
+  while bare `--as-user` through the broker also handles review, reply, resolve, edit and comment. Every other action still
+  goes through the installation token.
+  On a repository the app is not installed on, a pull request can be opened, edited, reviewed, and its
+  review comments replied to, edited or resolved with bare `--as-user`; close and merge still have no
+  route, and neither the container nor the host broker can do them. Acting as a person, from a
+  checkout on the host that holds the person's own credentials or an allowlisted explicit repository,
+  is the design.
 - **The configuration is found by convention and its paths belong to the broker, not to you.**
   Every command resolves `--config`, else `$GIT_BROKER_CONFIG`, else `./broker.config.yml`, and takes
   the deployment directory from the file's own location. The paths *inside* it are resolved by

README.md

@@ -135,10 +135,10 @@ described below is still what runs the lifecycle `gh` cannot express (opening as
 threads); `gcb pr --close` is gone and refuses with a pointer at `gcb gh pr close`.
 
 For that flag-only form, creation uses the app's installation token by default; `--as-user` without
-a value uses the broker's authorized user token for open, reply, resolve, edit and comment, and
-`--as-user <login>`
-uses the host broker for creation. Resolving a review thread defaults to the user identity because the pull-request token this broker
-mints is narrowed to `pull_requests: write` and `contents: read`, while `resolveReviewThread` needs
+a value uses the broker's authorized user token for open, review, reply, resolve, edit and comment,
+and `--as-user <login>` uses the host broker for creation, review, reply and edit. Resolving a review
+thread defaults to the user identity because the pull-request token this broker mints is narrowed to
+`pull_requests: write` and `contents: read`, while `resolveReviewThread` needs
 `contents: write` — a grant the broker gives only to the credential git pushes with, never to a
 pull-request token.
 
@@ -382,7 +382,7 @@ comment id names the comment — and its pull request — so `--edit` takes no `
 only an author change a comment, so without `--as-user` the app can change only the comments it
 authored; with bare `--as-user` it changes the authorized person's.
 
-### Opening a pull request or issue as yourself
+### Person actions: pull requests, issues and reviews
 
 There are three ways to have a pull request or issue authored by a person, and they stay alternatives: the **host
 host broker** below needs no long-lived secret anywhere and costs a process you keep running; **`userTokens`**
@@ -398,8 +398,9 @@ host, beside the same checkout, and ask for it with `--as-user <login>` from the
 ```sh
 # on the host, once — it serves every checkout below $HOME
 gcb host          # --root <dir> to narrow it, repeatable
-# for repo fork, review replies or an explicit --repo, also pass --config <broker.config.yml>
-# so its allowlist applies
+# for repo fork, reviews, review replies, pull request edit, issue edit/comment or an explicit
+# --repo, also pass --config <broker.config.yml> so its allowlist applies; reviews always carry a
+# repo, even from origin
 # or, at boot and for good: examples/gcb-host-boot.sh (see below)
 ```
 
@@ -413,12 +414,26 @@ gcb pr --as-user octocat --repo owner/repo --head feature --base main \
 
 # reply as that host account
 gcb pr --as-user octocat --number 94 --reply-to 123456 --body "good point"
+
+# submit a review or edit a pull request as the broker's authorized person, or a host account
+gcb pr --as-user --number 94 --approve
+gcb pr --as-user octocat --number 94 --approve
+gcb pr --as-user --number 94 --title "new title"
+gcb pr --as-user octocat --number 94 --body-file pr.md
+gcb pr --as-user octocat --number 94 --request-changes --body "please fix"
+gcb pr --as-user octocat --number 94 --review-comment --body "a note"
+
+# edit or comment on an issue as a person
+gcb issue edit 94 --as-user --title "new title"
+gcb issue comment 94 --as-user --body "a note"
 ```
 
-The host broker creates pull requests and issues, replies to review comments, and forks with the gh
-account named by the request; it does not edit, close or merge. For `repo fork`, review replies and
-any request that names `--repo` explicitly, it checks the repository against the broker config's
-allowlist (pass `--config`). An explicit `--repo` needs no checkout of that repository; without it,
+The host broker creates pull requests and issues, submits reviews, replies to review comments, edits
+pull requests and issues, and forks with the gh account named by the request; it does not close or
+merge. For `repo fork`, review replies, reviews, pull request edit, issue edit/comment and any
+request that names `--repo` explicitly — every review always carries its repository, even when
+derived from origin —
+it checks the repository against the broker config's allowlist (pass `--config`). An explicit `--repo` needs no checkout of that repository; without it,
 the checkout decides.
 `--as-user <login>`
 selects `~/.config/gh-<login>` on the host, matching the login exactly including case; create that
@@ -438,9 +453,9 @@ picks up. The bound exists because the walk repeats every `--interval` seconds;
 deeper layouts. To start it once and keep it,
 `examples/gcb-host.service` is a systemd user unit; on a system without user services,
 `examples/gcb-host-boot.sh` is the same thing for a DSM boot-up task — absolute paths, one
-instance at a time, and its output in a log file. Without `--as-user`, `pr` and `issue create` use
+instance at a time, and its output in a log file. Without `--as-user`, `pr` and `issue` actions use
 the broker route as the app; bare `--as-user` selects the broker's authorized person for a pull
-request or issue; `--as-user <login>` writes a request for this host broker. With a login and no host broker
+request or issue action; `--as-user <login>` writes a request for this host broker. With a login and no host broker
 running they fail; they do not fall back to the broker route.
 
 ```sh
@@ -498,8 +513,8 @@ whatever device has a browser. The refresh token is stored beside the private ke
 broker looks for it, and the broker renews it from then on — nothing else needs running.
 
 The user token is used where the person identity is required or preferred: authoring a pull request,
-resolving a review thread (GraphQL refuses that for an app), replying to or editing a review comment,
-and creating an issue through the broker action path.
+submitting a review, resolving a review thread (GraphQL refuses that for an app), replying to or
+editing a review comment, and creating an issue through the broker action path.
 
 The file needs to be **writable by the broker**, unlike the key: GitHub rotates the refresh token on
 every exchange, so mount it read-write (`./user.refresh:/etc/git-cred-broker/user.refresh:rw` in the
@@ -752,6 +767,8 @@ actions. Grant them on the App, then approve any changed permissions on each ins
 | `pr review` | `pull_requests: write` |
 | `pr review --id` (`pr:review-by-id`) | `pull_requests: read` |
 | `issue create` | `issues: write` |
+| `issue edit` | `issues: write` |
+| `issue comment` | `issues: write` |
 | `run view` | `actions: read` |
 | `run list` | `actions: read` |
 

ROADMAP.md

@@ -366,8 +366,10 @@ Account selection:
       - `gh auth switch` is global mutable state and races concurrent requests;
       - a request names the account; the host broker refuses an unknown account and lists the configured
         ones.
-- [ ] Extend host broker operations beyond `pr create`:
+- [x] Extend host broker operations beyond `pr create`:
       - `pr review` (approve / request changes / comment);
+      - `pr edit` (title / body / base);
+      - `issue edit` / `issue comment`;
       - `issue create`;
       - `pr comment` if the per-user identity matters.
       Each operation is a typed request with a host-side command table, canonical argv, repo

packages/client/src/commands/gh.ts

@@ -4,7 +4,7 @@
  * The action catalog defines the command, flags, positionals and permissions. The client only
  * parses names and values; the broker rebuilds the canonical argv and runs gh with its own token.
  */
-import { GH_ACTIONS, getGhAction, parseArgs, say, fail, type ActionValues } from 'gcb-core';
+import { GH_ACTIONS, buildGhArgv, getGhAction, parseArgs, say, fail, type ActionValues } from 'gcb-core';
 
 import { requestOverSocket, resolveSocketPath } from '../helper.ts';
 import { remoteRepo } from './remotes.ts';
@@ -196,8 +196,6 @@ export async function runGh(
     }
     repo = found.repo;
   }
-  const socketPath = resolveSocketPath(args.value('socket'), process.env);
-  if (socketPath === undefined) fail('no broker socket is configured');
 
   for (const argument of rest) {
     if (argument === '--') break;
@@ -242,6 +240,15 @@ export async function runGh(
     else values[flag.name] = entries[0] as string;
   }
 
+  try {
+    buildGhArgv(actionName, values as ActionValues, repo);
+  } catch (error) {
+    fail((error as Error).message);
+  }
+
+  const socketPath = resolveSocketPath(args.value('socket'), process.env);
+  if (socketPath === undefined) fail('no broker socket is configured');
+
   let response;
   try {
     response = await requestOverSocket(

packages/client/src/commands/host-submit.ts

@@ -13,14 +13,15 @@ import {
   findGitDir,
   readOpener,
   removeResult,
+  requestProblems,
   waitForResult,
   writeRequest,
 } from 'gcb-core';
 import type { HostOpenResult, HostRequest } from 'gcb-core';
 import { fail, say } from 'gcb-core';
 
 /**
- * Explain why an opener request produced no answer.
+ * Explain why a host-broker request produced no answer.
  *
  * @param dir - the channel directory.
  * @param id - the request id.
@@ -45,25 +46,33 @@ export function noAnswerMessage(dir: string, id: string, noun: string, timeoutMs
   );
 }
 
+/** How each request kind is named, and the capability an older host broker may lack. */
+type RequestKind = NonNullable<HostRequest['kind']>;
+const KINDS: Readonly<Record<RequestKind, { noun: string; needed?: string }>> = {
+  'pull-request': { noun: 'pull request' },
+  issue: { noun: 'issue', needed: 'issue' },
+  fork: { noun: 'fork', needed: 'fork' },
+  reply: { noun: 'reply', needed: 'reply' },
+  edit: { noun: 'edit', needed: 'edit' },
+  'issue-edit': { noun: 'issue edit', needed: 'issue-edit' },
+  'issue-comment': { noun: 'issue comment', needed: 'issue-comment' },
+  review: { noun: 'review', needed: 'review' },
+};
+
 /**
- * Write, wait for and report one opener request.
+ * Write, wait for and report one host-broker request.
  *
  * @param request - the validated request.
  * @param success - formats the success line from the answer.
  * @returns 0 after the answer is reported.
  */
-export async function submitToOpener(
+export async function submitToHostBroker(
   request: HostRequest,
   success: (answer: HostOpenResult) => string,
 ): Promise<number> {
-  const noun =
-    request.kind === 'fork'
-      ? 'fork'
-      : request.kind === 'issue'
-        ? 'issue'
-        : request.kind === 'reply'
-          ? 'reply'
-          : 'pull request';
+  const problems = requestProblems(request);
+  if (problems.length > 0) fail(problems.join('; '));
+  const { noun, needed } = KINDS[request.kind ?? 'pull-request'];
   const gitDir = findGitDir(process.cwd());
   if (gitDir === undefined) fail('--as-user needs a git repository: run it from the checkout');
   const dir = channelDir(gitDir);
@@ -72,17 +81,15 @@ export async function submitToOpener(
     fail(
       `no host broker is watching ${dir}\n` +
         '  start one on the host: gcb host --repo <that checkout>\n' +
-        '  or drop --as-user to open it as the app instead',
+        '  or drop --as-user to do it as the app instead',
     );
   }
-  const needed =
-    request.kind === 'issue' ? 'issue' : request.kind === 'fork' ? 'fork' : request.kind === 'reply' ? 'reply' : undefined;
   if (needed !== undefined && !(Array.isArray(opener.kinds) && opener.kinds.includes(needed))) {
     fail(`the host broker does not advertise ${needed} support; restart it from this gcb-host version`);
   }
 
-  // writeRequest refuses what the opener would refuse — a title past its limit, a body past its limit,
-  // a head equal to its base. It throws, and a stack trace tells the caller none of that.
+  // requestProblems above already checked the contract. Keep the catch as defence in case the two
+  // validators ever diverge; a stack trace would tell the caller none of it.
   let id: string;
   try {
     id = writeRequest(dir, request);
@@ -99,7 +106,7 @@ export async function submitToOpener(
   if (answer === undefined) answer = await waitForResult(dir, id, 300, 50);
   if (answer === undefined) fail(noAnswerMessage(dir, id, noun, timeoutMs));
   removeResult(dir, id);
-  if (answer.error !== undefined) fail(`the host broker could not open it: ${answer.error}`);
+  if (answer.error !== undefined) fail(`the host broker could not do it: ${answer.error}`);
   say(success(answer));
   return 0;
 }