From ebae282e4dc5a27077bf9ad3510948e3ffc9e931 Mon Sep 17 00:00:00 2001
From: JounQin <admin@1stg.me>
Date: Sat, 3 Oct 2026 17:28:18 +0000
Subject: [PATCH 1/3] feat: change a review comment's body through the broker

A review comment can be read with `pr --threads` and answered with
`--reply-to`, but not corrected. `pr --edit <id>` now replaces its body
with PATCH /repos/{owner}/{repo}/pulls/comments/{id}, on the app's
installation token narrowed to `pull_requests: write`.

The comment id is the REST id `--threads` already prints as a comment's
databaseId, and the repository stays in the path so the allowlist binds
the call. The CLI requires `--number`, a positive id and a non-empty
body; the broker validates the same and audits the action. GitHub
accepts an edit only from the comment's author, and the comments this
command posts are the app's, so those are the ones it can change.
---
 README.md                    | 10 ++++++++-
 src/broker.ts                | 12 ++++++++---
 src/commands/pr.ts           | 17 ++++++++++++---
 src/providers/github-app.ts  | 23 ++++++++++++++++++++
 src/types.ts                 |  3 ++-
 test/unit/broker.test.ts     | 41 ++++++++++++++++++++++++++++++++++++
 test/unit/commands.test.ts   | 12 +++++++++++
 test/unit/github-app.test.ts | 32 ++++++++++++++++++++++++++++
 8 files changed, 142 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index c22469c..0a932bc 100644
--- a/README.md
+++ b/README.md
@@ -238,13 +238,21 @@ git-credential-broker pr --host github.com --repo owner/repo \
   --head feature --base main --title "a title" --body-file pr.md
   # say it about one line rather than about the pull request
   # --number 7 --comment --file Dockerfile --line 4 --side right --body "why this marker exists"
+  # read its review threads, answer one, or change a comment you wrote
+  # --number 7 --threads
+  # --number 7 --reply-to 123456 --body "good point"
+  # --number 7 --edit 123456 --body "corrected: it is the other branch"
   # from your fork into an upstream: a remote name instead of a repository typed by hand
   git-credential-broker pr --repo code-vaults/repo --head origin:feature --base upstream:main --title "a title" \
     --body-file pr.md
 ```
 
 Both the app and the installation need `pull_requests: write`; without it the error names the
-permission instead of failing at GitHub. `--draft` opens it as a draft.
+permission instead of failing at GitHub. `--draft` opens it as a draft. `--threads` prints the ids of
+a pull request's review threads and their comments; `--reply-to` adds a reply inside one, and
+`--edit` replaces the body of one of those review comments. GitHub lets only an author change a
+comment, and the comments this command posts are the app's, so `--edit` changes those rather than a
+person's.
 
 ### Opening one as yourself
 
diff --git a/src/broker.ts b/src/broker.ts
index 1ef50e4..af668f0 100644
--- a/src/broker.ts
+++ b/src/broker.ts
@@ -281,6 +281,7 @@ export function createRequestHandler(
         action !== 'comment' &&
         action !== 'threads' &&
         action !== 'reply' &&
+        action !== 'edit' &&
         action !== 'resolve'
       ) {
         return deny(context, CODES.BAD_REQUEST, `bad action ${JSON.stringify(action)}`, 'unknown pull request action');
@@ -341,11 +342,16 @@ export function createRequestHandler(
       if (action === 'update' && title === undefined && body === undefined && base === undefined) {
         return deny(context, CODES.BAD_REQUEST, 'empty update', 'an update has to change a title, a body or a base');
       }
-      if ((action === 'comment' || action === 'reply') && (typeof body !== 'string' || body.trim() === '')) {
+      if ((action === 'comment' || action === 'reply' || action === 'edit') && (typeof body !== 'string' || body.trim() === '')) {
         return deny(context, CODES.BAD_REQUEST, 'empty comment', 'a comment has to say something');
       }
-      if (action === 'reply' && (typeof commentId !== 'number' || !Number.isInteger(commentId) || commentId <= 0)) {
-        return deny(context, CODES.BAD_REQUEST, 'bad commentId', 'a reply needs the id of the comment it answers');
+      if ((action === 'reply' || action === 'edit') && (typeof commentId !== 'number' || !Number.isInteger(commentId) || commentId <= 0)) {
+        return deny(
+          context,
+          CODES.BAD_REQUEST,
+          'bad commentId',
+          action === 'reply' ? 'a reply needs the id of the comment it answers' : 'an edit needs the id of the review comment it changes',
+        );
       }
       if (action === 'resolve' && (typeof threadId !== 'string' || threadId === '')) {
         return deny(context, CODES.BAD_REQUEST, 'bad threadId', 'resolving needs the id of the thread, which --threads prints');
diff --git a/src/commands/pr.ts b/src/commands/pr.ts
index cc10901..4cfbafb 100644
--- a/src/commands/pr.ts
+++ b/src/commands/pr.ts
@@ -53,6 +53,8 @@ Acting on one
   --side <left|right>  Which side of the diff --line counts on (default: right)
   --threads            List its review threads: their ids, whether they are resolved, and the comments
   --reply-to <id>      Reply inside the thread that comment id belongs to, with --body/--body-file
+  --edit <id>          Replace the body of that review comment, with --body/--body-file
+                       (an id --threads prints; GitHub accepts it only from the comment's author)
   --resolve <thread>   Mark that review thread resolved (the id --threads prints)
   --close              Close it
   --merge              Merge it (branch protection still applies)
@@ -76,6 +78,7 @@ Common
 function actionOf(args: { has(name: string): boolean; value(name: string): string | undefined }) {
   if (args.value('resolve') !== undefined) return 'resolve' as const;
   if (args.value('reply-to') !== undefined) return 'reply' as const;
+  if (args.value('edit') !== undefined) return 'edit' as const;
   if (args.has('threads')) return 'threads' as const;
   if (args.has('comment')) return 'comment' as const;
   if (args.has('status')) return 'status' as const;
@@ -155,14 +158,15 @@ export async function runPr(argv: readonly string[]): Promise<number> {
     if (!title) fail('--title is required to open a pull request');
   } else if (action !== 'resolve' && number === undefined) {
     const verb = action === 'status' ? 'inspect' : action;
-    fail(`--number is required to ${verb} a pull request`);
+    const target = action === 'edit' ? 'the review comment on a pull request' : 'a pull request';
+    fail(`--number is required to ${verb} ${target}`);
   } else if (action !== 'resolve' && (!Number.isInteger(Number(number)) || Number(number) <= 0)) {
     fail(`--number must be a positive integer, got ${JSON.stringify(number)}`);
   }
   if (action === 'update' && title === undefined && body === undefined && base === undefined) {
     fail('an update has to change something: pass --title, --body/--body-file or --base');
   }
-  if ((action === 'comment' || action === 'reply') && (body === undefined || body.trim() === '')) {
+  if ((action === 'comment' || action === 'reply' || action === 'edit') && (body === undefined || body.trim() === '')) {
     fail('a comment has to say something: pass --body or --body-file');
   }
 
@@ -185,6 +189,10 @@ export async function runPr(argv: readonly string[]): Promise<number> {
   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)}`);
   }
+  const editId = args.value('edit');
+  if (action === 'edit' && (editId === undefined || !Number.isInteger(Number(editId)) || Number(editId) <= 0)) {
+    fail(`--edit needs the id of the review comment being changed, got ${JSON.stringify(editId)}`);
+  }
 
   if (viaHost) {
     if (head === undefined || title === undefined) fail('--via-host needs --head and --title');
@@ -245,6 +253,7 @@ export async function runPr(argv: readonly string[]): Promise<number> {
         action,
         ...(number === undefined ? {} : { number: Number(number) }),
         ...(action === 'reply' ? { commentId: Number(replyTo) } : {}),
+        ...(action === 'edit' ? { commentId: Number(editId) } : {}),
         ...(action === 'resolve' ? { threadId: args.value('resolve') } : {}),
         ...(head === undefined ? {} : { head }),
         ...(base === undefined ? {} : { base }),
@@ -284,7 +293,9 @@ export async function runPr(argv: readonly string[]): Promise<number> {
             ? 'commented on'
             : action === 'reply'
               ? 'replied in'
-              : 'updated';
+              : action === 'edit'
+                ? 'edited a review comment on'
+                : 'updated';
   const state = response.prState === undefined ? '' : ` (${response.prState})`;
   say(`${done} #${response.prNumber ?? '?'}${state}: ${response.prUrl ?? '(no url)'}`);
   return 0;
diff --git a/src/providers/github-app.ts b/src/providers/github-app.ts
index 043d5c2..8508251 100644
--- a/src/providers/github-app.ts
+++ b/src/providers/github-app.ts
@@ -799,6 +799,29 @@ function describeCause(error: unknown): string {
         return { number: request.number ?? 0, url: '' };
       }
 
+      if (request.action === 'edit') {
+        // A review comment is addressed by its own id, so unlike a reply the pull request number is not
+        // part of the route — but the repository is, which is what keeps the allowlist binding it. The
+        // comment id is the REST id, the one `--threads` prints as a comment's databaseId.
+        const { text: edited } = await callWithToken(
+          'PATCH',
+          `${api}/repos/${request.owner}/${request.repo}/pulls/comments/${String(request.commentId ?? 0)}`,
+          token,
+          { body: request.body ?? '' },
+        );
+        let read: unknown = null;
+        try {
+          read = JSON.parse(edited);
+        } catch {
+          read = null;
+        }
+        const html = (read ?? {}) as { html_url?: unknown };
+        return {
+          number: request.number ?? 0,
+          url: typeof html.html_url === 'string' ? html.html_url : '',
+        };
+      }
+
       if (request.action === 'threads') {
         // GraphQL, because review threads have no REST list. The repository and the number are
         // named here, so the allowlist still decides what can be reached.
diff --git a/src/types.ts b/src/types.ts
index 068562c..81dddb0 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -72,6 +72,7 @@ export type PullRequestAction =
   | 'comment'
   | 'threads'
   | 'reply'
+  | 'edit'
   | 'resolve';
 
 /** How a merge should be recorded. */
@@ -85,7 +86,7 @@ export interface PullRequestRequest {
   readonly action: PullRequestAction;
   /** The pull request number. Not needed to open one, required for the rest. */
   readonly number?: number;
-  /** The review comment being answered, for `action: "reply"`. */
+  /** The review comment being answered or changed, for `action: "reply"` or `"edit"`. */
   readonly commentId?: number;
   /** The review thread being resolved, for `action: "resolve"`. */
   readonly threadId?: string;
diff --git a/test/unit/broker.test.ts b/test/unit/broker.test.ts
index 3a037cb..9151dcb 100644
--- a/test/unit/broker.test.ts
+++ b/test/unit/broker.test.ts
@@ -409,6 +409,47 @@ describe('broker request handling', () => {
     assert.equal(wrongAction.code, 'bad-request', 'and an anchor only belongs on a comment');
   });
 
+  it('carries an edit of a review comment, and refuses one with nothing to say', async () => {
+    const seen: Array<Record<string, unknown>> = [];
+    const provider: Provider = {
+      name: 'with-edits',
+      getCredential: () => Promise.reject(new Error('not used here')),
+      pullRequest: (request) => {
+        seen.push(request as unknown as Record<string, unknown>);
+        return Promise.resolve({ number: request.number ?? 0, url: 'u' });
+      },
+    };
+    const { handle, events } = makeHarness(['acme/widget'], provider);
+    const good = await handle({
+      ...REQUEST,
+      op: 'pull-request',
+      action: 'edit',
+      number: 7,
+      commentId: 99,
+      body: 'a corrected note',
+    });
+    assert.equal(good.ok, true);
+    assert.deepEqual(
+      [seen.at(-1)?.['action'], seen.at(-1)?.['commentId'], seen.at(-1)?.['body']],
+      ['edit', 99, 'a corrected note'],
+      'the action, the comment id and the new body all reach the provider',
+    );
+    assert.equal(events.at(-1)?.['action'], 'edit', 'and the audit records the action');
+
+    const empty = await handle({
+      ...REQUEST,
+      op: 'pull-request',
+      action: 'edit',
+      number: 7,
+      commentId: 99,
+      body: '   ',
+    });
+    assert.equal(empty.code, 'bad-request', 'an edit has to say something');
+
+    const noId = await handle({ ...REQUEST, op: 'pull-request', action: 'edit', number: 7, body: 'a note' });
+    assert.equal(noId.code, 'bad-request', 'and it has to name the comment it changes');
+  });
+
 describe('prepareSocketPath', () => {
   const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-sock-'));
   after(() => fs.rmSync(dir, { recursive: true, force: true }));
diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts
index c749406..6f8f125 100644
--- a/test/unit/commands.test.ts
+++ b/test/unit/commands.test.ts
@@ -762,4 +762,16 @@ describe('the anchor flags, at the CLI boundary', () => {
     assert.equal(updated.status, 1);
     assert.match(updated.stderr, /only goes with --merge/);
   });
+
+  it('refuses an edit with nothing to say, or with no comment to change', () => {
+    const noBody = cli([...base, '--edit', '99']);
+    assert.equal(noBody.status, 1, 'an edit with no body is refused');
+    assert.match(noBody.stderr, /a comment has to say something/);
+
+    for (const id of ['0', 'x', '-1']) {
+      const bad = cli([...base, '--edit', id, '--body', 'a corrected note']);
+      assert.equal(bad.status, 1, `--edit ${id} is refused`);
+      assert.match(bad.stderr, /--edit needs the id of the review comment/);
+    }
+  });
 });
diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts
index 3102415..68c7e3e 100644
--- a/test/unit/github-app.test.ts
+++ b/test/unit/github-app.test.ts
@@ -204,6 +204,15 @@ function createStubFetch(options: {
       );
     }
     if (url.endsWith('/reviews')) return Promise.resolve(jsonResponse({}));
+    if (/\/pulls\/comments\/\d+$/.test(url)) {
+      return Promise.resolve(
+        jsonResponse({
+          id: 99,
+          html_url: 'https://github.com/acme/widget/pull/7#discussion_r99',
+          body: 'a corrected note',
+        }),
+      );
+    }
     if (url.endsWith('/pulls')) {
       return Promise.resolve(
         jsonResponse({ number: 42, html_url: 'https://github.com/acme/widget/pull/42' }, 201),
@@ -737,6 +746,29 @@ describe('createGithubAppProvider', () => {
     assert.deepEqual(review?.body, { body: 'a general remark', event: 'COMMENT' }, 'nothing new in the payload');
   });
 
+  it('changes a review comment by its id, and reports the comment it changed', async () => {
+    const stub = createStubFetch({ installations: [{ id: 42, account: { login: 'acme' } }], nowMs });
+    const changed = await providerFor(stub).pullRequest!({
+      action: 'edit',
+      host: 'github.com',
+      owner: 'acme',
+      repo: 'widget',
+      number: 7,
+      commentId: 99,
+      body: 'a corrected note',
+    });
+
+    const patch = stub.calls.find((call) => call.method === 'PATCH' && call.url.includes('/pulls/comments/'));
+    assert.ok(patch, 'a review comment is changed at the review comments endpoint');
+    // No pull request number in the route: the comment id already names the thing, and the repository
+    // in the path is what keeps the allowlist binding it.
+    assert.equal(patch?.url, 'https://api.github.com/repos/acme/widget/pulls/comments/99');
+    assert.deepEqual(patch?.body, { body: 'a corrected note' });
+    assert.equal(patch?.authorization, 'Bearer ghs_stub_token_value', 'and it is the app that changes it');
+    assert.equal(changed.number, 7);
+    assert.equal(changed.url, 'https://github.com/acme/widget/pull/7#discussion_r99');
+  });
+
 
   it("resolves a thread with a token per owner, which is a person acting too", async () => {
     const dir = mkdtempSync(join(tmpdir(), 'resolve-owner-'));

From 3ecc2886c08a226b4bdbd2b0b72db11f3d1abcdf Mon Sep 17 00:00:00 2001
From: JounQin <admin@1stg.me>
Date: Sat, 3 Oct 2026 17:39:50 +0000
Subject: [PATCH 2/3] fix: the review round's two findings, and the class the
 first one named

A bare `--edit` (and `--reply-to`, `--resolve`) is parsed as a boolean
flag with no value, so selecting the action from `value` alone let it
fall through to `update` and patch the pull request body with the text
meant for a comment. The action is now chosen from the flag's presence,
and a missing `--resolve` id is refused like the other two.

The review comment route carries no pull request number, so a comment id
belonging to another pull request in the same repository was edited while
the CLI reported the number it was given. The provider now reads the
comment first and refuses when it belongs to a different pull request,
before any mutation happens.
---
 src/commands/pr.ts           | 15 +++++++--
 src/providers/github-app.ts  | 62 +++++++++++++++++++++++++++++-------
 test/unit/commands.test.ts   | 15 +++++++++
 test/unit/github-app.test.ts | 38 ++++++++++++++++++++--
 4 files changed, 112 insertions(+), 18 deletions(-)

diff --git a/src/commands/pr.ts b/src/commands/pr.ts
index 4cfbafb..ba80d31 100644
--- a/src/commands/pr.ts
+++ b/src/commands/pr.ts
@@ -76,9 +76,14 @@ Common
  * @returns `close`, `merge`, `update` or `open`.
  */
 function actionOf(args: { has(name: string): boolean; value(name: string): string | undefined }) {
-  if (args.value('resolve') !== undefined) return 'resolve' as const;
-  if (args.value('reply-to') !== undefined) return 'reply' as const;
-  if (args.value('edit') !== undefined) return 'edit' as const;
+  // `--resolve`, `--reply-to` and `--edit` take a value, and a bare one is parsed as a boolean flag with
+  // no value at all. Selecting the action from the flag's *presence* is what turns that into the refusal
+  // below — reading only `value` let a bare `--edit` fall through to `update`, which patched the pull
+  // request itself with the body meant for the comment.
+  const present = (name: string): boolean => args.has(name) || args.value(name) !== undefined;
+  if (present('resolve')) return 'resolve' as const;
+  if (present('reply-to')) return 'reply' as const;
+  if (present('edit')) return 'edit' as const;
   if (args.has('threads')) return 'threads' as const;
   if (args.has('comment')) return 'comment' as const;
   if (args.has('status')) return 'status' as const;
@@ -193,6 +198,10 @@ export async function runPr(argv: readonly string[]): Promise<number> {
   if (action === 'edit' && (editId === undefined || !Number.isInteger(Number(editId)) || Number(editId) <= 0)) {
     fail(`--edit needs the id of the review comment being changed, got ${JSON.stringify(editId)}`);
   }
+  const resolveId = args.value('resolve');
+  if (action === 'resolve' && (resolveId === undefined || resolveId.trim() === '')) {
+    fail(`--resolve needs the id of the thread being resolved, got ${JSON.stringify(resolveId)}`);
+  }
 
   if (viaHost) {
     if (head === undefined || title === undefined) fail('--via-host needs --head and --title');
diff --git a/src/providers/github-app.ts b/src/providers/github-app.ts
index 8508251..336dbef 100644
--- a/src/providers/github-app.ts
+++ b/src/providers/github-app.ts
@@ -531,6 +531,24 @@ function describeCause(error: unknown): string {
   return typeof cause === 'string' && cause !== '' ? ` (${cause})` : '';
 }
 
+/**
+ * The pull request number in the URL GitHub returns for a pull request.
+ *
+ * A review comment carries the pull request it belongs to as `pull_request_url`, which is how a comment
+ * id — one for the whole repository, not one per pull request — is tied back to the pull request named
+ * on the command line.
+ *
+ * @param value - an `.../pulls/{number}` URL, as GitHub returns it.
+ * @returns the number, or undefined when the value names none.
+ */
+function pullRequestNumber(value: unknown): number | undefined {
+  if (typeof value !== 'string') return undefined;
+  const match = /\/pulls\/(\d+)\/?$/.exec(value);
+  if (match?.[1] === undefined) return undefined;
+  const number = Number(match[1]);
+  return Number.isInteger(number) && number > 0 ? number : undefined;
+}
+
   return {
     name: 'github-app',
 
@@ -800,24 +818,44 @@ function describeCause(error: unknown): string {
       }
 
       if (request.action === 'edit') {
-        // A review comment is addressed by its own id, so unlike a reply the pull request number is not
-        // part of the route — but the repository is, which is what keeps the allowlist binding it. The
-        // comment id is the REST id, the one `--threads` prints as a comment's databaseId.
-        const { text: edited } = await callWithToken(
-          'PATCH',
-          `${api}/repos/${request.owner}/${request.repo}/pulls/comments/${String(request.commentId ?? 0)}`,
-          token,
-          { body: request.body ?? '' },
-        );
+        // A review comment is addressed by its own id, so unlike a reply the route carries no pull request
+        // number — and that id is one for the whole repository, not one per pull request. Read the comment
+        // first and refuse when it belongs to a different pull request than the one named: a mutation is
+        // the wrong place to discover that `--number` was wrong, and the reply would otherwise claim a
+        // target it never touched. The repository in the path is still what the allowlist binds.
+        const endpoint = `${api}/repos/${request.owner}/${request.repo}/pulls/comments/${String(request.commentId ?? 0)}`;
+        const { text: current } = await callWithToken('GET', endpoint, token);
         let read: unknown = null;
         try {
-          read = JSON.parse(edited);
+          read = JSON.parse(current);
         } catch {
           read = null;
         }
-        const html = (read ?? {}) as { html_url?: unknown };
+        const found = (read ?? {}) as { pull_request_url?: unknown };
+        const belongsTo = pullRequestNumber(found.pull_request_url);
+        // A plain Error on purpose: which pull request a comment is on is the host log's business, not
+        // the container's, and `--number` is the caller's own claim about it.
+        if (belongsTo === undefined) {
+          throw new Error(
+            `refusing to edit comment ${String(request.commentId)}: could not read which pull request it belongs to`,
+          );
+        }
+        if (request.number !== undefined && belongsTo !== request.number) {
+          throw new Error(
+            `refusing to edit comment ${String(request.commentId)}: it is a review comment of ` +
+              `${request.owner}/${request.repo}#${belongsTo}, not #${request.number}`,
+          );
+        }
+        const { text: edited } = await callWithToken('PATCH', endpoint, token, { body: request.body ?? '' });
+        let changed: unknown = null;
+        try {
+          changed = JSON.parse(edited);
+        } catch {
+          changed = null;
+        }
+        const html = (changed ?? {}) as { html_url?: unknown };
         return {
-          number: request.number ?? 0,
+          number: belongsTo,
           url: typeof html.html_url === 'string' ? html.html_url : '',
         };
       }
diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts
index 6f8f125..cabf276 100644
--- a/test/unit/commands.test.ts
+++ b/test/unit/commands.test.ts
@@ -774,4 +774,19 @@ describe('the anchor flags, at the CLI boundary', () => {
       assert.match(bad.stderr, /--edit needs the id of the review comment/);
     }
   });
+
+  it('refuses a value flag given with no value, instead of updating the pull request', () => {
+    // A bare --edit / --reply-to / --resolve is parsed as a boolean flag with no value, so the action
+    // has to be selected from the flag's presence: reading only its value let the command fall through
+    // to `update`, which patched the body meant for the comment onto the pull request itself.
+    for (const [flag, message] of [
+      ['--edit', /--edit needs the id of the review comment/],
+      ['--reply-to', /--reply-to needs the id of the comment/],
+      ['--resolve', /--resolve needs the id of the thread/],
+    ] as const) {
+      const refused = cli([...base, flag, '--body', 'a corrected note']);
+      assert.equal(refused.status, 1, `a bare ${flag} is refused`);
+      assert.match(refused.stderr, message);
+    }
+  });
 });
diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts
index 68c7e3e..7c02dae 100644
--- a/test/unit/github-app.test.ts
+++ b/test/unit/github-app.test.ts
@@ -138,6 +138,8 @@ function createStubFetch(options: {
   installations: unknown;
   /** What the GraphQL scope check is told the thread belongs to. */
   threadRepo?: string;
+  /** Which pull request the stubbed review comment belongs to. */
+  commentPr?: number;
   nowMs: number;
   token?: string;
   tokenStatus?: number;
@@ -155,6 +157,7 @@ function createStubFetch(options: {
     installationPermissions = appPermissions,
     logText = 'stub log\n',
     jobMissing = false,
+    commentPr = 7,
   } = options;
   const calls: RecordedCall[] = [];
   const fetchImpl: FetchLike = (url, init) => {
@@ -208,7 +211,8 @@ function createStubFetch(options: {
       return Promise.resolve(
         jsonResponse({
           id: 99,
-          html_url: 'https://github.com/acme/widget/pull/7#discussion_r99',
+          html_url: `https://github.com/acme/widget/pull/${commentPr}#discussion_r99`,
+          pull_request_url: `https://api.github.com/repos/acme/widget/pulls/${commentPr}`,
           body: 'a corrected note',
         }),
       );
@@ -760,15 +764,43 @@ describe('createGithubAppProvider', () => {
 
     const patch = stub.calls.find((call) => call.method === 'PATCH' && call.url.includes('/pulls/comments/'));
     assert.ok(patch, 'a review comment is changed at the review comments endpoint');
-    // No pull request number in the route: the comment id already names the thing, and the repository
-    // in the path is what keeps the allowlist binding it.
+    // The comment id names the thing and is one for the whole repository, so the route carries no pull
+    // request number; the comment is read first to tie it back to the number the caller named.
     assert.equal(patch?.url, 'https://api.github.com/repos/acme/widget/pulls/comments/99');
     assert.deepEqual(patch?.body, { body: 'a corrected note' });
     assert.equal(patch?.authorization, 'Bearer ghs_stub_token_value', 'and it is the app that changes it');
+    const read = stub.calls.find((call) => call.method === 'GET' && call.url.endsWith('/pulls/comments/99'));
+    assert.ok(read, 'the comment is read before it is changed');
     assert.equal(changed.number, 7);
     assert.equal(changed.url, 'https://github.com/acme/widget/pull/7#discussion_r99');
   });
 
+  it('refuses to edit a comment that belongs to another pull request, and changes nothing', async () => {
+    const stub = createStubFetch({
+      installations: [{ id: 42, account: { login: 'acme' } }],
+      nowMs,
+      commentPr: 8,
+    });
+    await assert.rejects(
+      providerFor(stub).pullRequest!({
+        action: 'edit',
+        host: 'github.com',
+        owner: 'acme',
+        repo: 'widget',
+        number: 7,
+        commentId: 99,
+        body: 'a corrected note',
+      }),
+      /#8, not #7/,
+      'the mismatch is refused by name',
+    );
+    assert.equal(
+      stub.calls.some((call) => call.method === 'PATCH'),
+      false,
+      'and the comment on the other pull request is left alone',
+    );
+  });
+
 
   it("resolves a thread with a token per owner, which is a person acting too", async () => {
     const dir = mkdtempSync(join(tmpdir(), 'resolve-owner-'));

From 3632c00f214099ea964058473e60fd9d65273a31 Mon Sep 17 00:00:00 2001
From: JounQin <admin@1stg.me>
Date: Sat, 3 Oct 2026 17:45:14 +0000
Subject: [PATCH 3/3] fix: no number an edit cannot honour, and the comment it
 changes in the audit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`--edit` names the comment by id, and the route cannot honour a pull
request number, so the number is no longer required or sent — the number
reported is the one the comment itself carries in the answer, not the
caller's claim. The pull-request audit entry now records `comment_id`,
the one thing an edit changes that a number does not name (a reply
carries one too).

The CLI test helper pins HOME and clears the socket variables, so a case
that gets past validation dies at the socket rather than at whatever the
machine happens to have configured.
---
 README.md                    |  8 +++----
 src/broker.ts                |  5 ++++-
 src/commands/pr.ts           | 14 ++++++------
 src/providers/github-app.ts  | 35 ++++++------------------------
 src/types.ts                 |  2 +-
 test/unit/broker.test.ts     | 20 +++++++++--------
 test/unit/commands.test.ts   | 20 +++++++++++++++++
 test/unit/github-app.test.ts | 42 +++++++++++++++++-------------------
 8 files changed, 74 insertions(+), 72 deletions(-)

diff --git a/README.md b/README.md
index 0a932bc..95ab39e 100644
--- a/README.md
+++ b/README.md
@@ -241,7 +241,7 @@ git-credential-broker pr --host github.com --repo owner/repo \
   # read its review threads, answer one, or change a comment you wrote
   # --number 7 --threads
   # --number 7 --reply-to 123456 --body "good point"
-  # --number 7 --edit 123456 --body "corrected: it is the other branch"
+  # --edit 123456 --body "corrected: it is the other branch"
   # from your fork into an upstream: a remote name instead of a repository typed by hand
   git-credential-broker pr --repo code-vaults/repo --head origin:feature --base upstream:main --title "a title" \
     --body-file pr.md
@@ -250,9 +250,9 @@ git-credential-broker pr --host github.com --repo owner/repo \
 Both the app and the installation need `pull_requests: write`; without it the error names the
 permission instead of failing at GitHub. `--draft` opens it as a draft. `--threads` prints the ids of
 a pull request's review threads and their comments; `--reply-to` adds a reply inside one, and
-`--edit` replaces the body of one of those review comments. GitHub lets only an author change a
-comment, and the comments this command posts are the app's, so `--edit` changes those rather than a
-person's.
+`--edit` replaces the body of one of those review comments. The comment id names the comment — and its
+pull request — so `--edit` takes no `--number`. GitHub lets only an author change a comment, and the
+comments this command posts are the app's, so `--edit` changes those rather than a person's.
 
 ### Opening one as yourself
 
diff --git a/src/broker.ts b/src/broker.ts
index af668f0..d2b20dc 100644
--- a/src/broker.ts
+++ b/src/broker.ts
@@ -356,7 +356,7 @@ export function createRequestHandler(
       if (action === 'resolve' && (typeof threadId !== 'string' || threadId === '')) {
         return deny(context, CODES.BAD_REQUEST, 'bad threadId', 'resolving needs the id of the thread, which --threads prints');
       }
-      if (action !== 'open' && action !== 'resolve' && (typeof number !== 'number' || !Number.isInteger(number) || number <= 0)) {
+      if (action !== 'open' && action !== 'resolve' && action !== 'edit' && (typeof number !== 'number' || !Number.isInteger(number) || number <= 0)) {
         return deny(context, CODES.BAD_REQUEST, `bad number ${JSON.stringify(number)}`, 'this action needs a pull request number');
       }
       if (action === 'merge' && method !== 'merge' && method !== 'squash' && method !== 'rebase') {
@@ -395,6 +395,9 @@ export function createRequestHandler(
           ...context,
           action,
           ...(typeof number === 'number' ? { number } : {}),
+          // The comment an edit changes is the one thing the entry would otherwise not say, and the
+          // number alone does not name it. Recorded for a reply too, which also carries an id.
+          ...(typeof commentId === 'number' ? { comment_id: commentId } : {}),
           ...(typeof head === 'string' ? { head } : {}),
           ...(typeof base === 'string' ? { base } : {}),
           pr_number: result.number,
diff --git a/src/commands/pr.ts b/src/commands/pr.ts
index ba80d31..25ef934 100644
--- a/src/commands/pr.ts
+++ b/src/commands/pr.ts
@@ -54,7 +54,7 @@ Acting on one
   --threads            List its review threads: their ids, whether they are resolved, and the comments
   --reply-to <id>      Reply inside the thread that comment id belongs to, with --body/--body-file
   --edit <id>          Replace the body of that review comment, with --body/--body-file
-                       (an id --threads prints; GitHub accepts it only from the comment's author)
+                       (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)
@@ -161,11 +161,13 @@ export async function runPr(argv: readonly string[]): Promise<number> {
   if (action === 'open') {
     if (!head) fail('--head is required to open a pull request');
     if (!title) fail('--title is required to open a pull request');
-  } else if (action !== 'resolve' && number === undefined) {
+  } else if (action !== 'resolve' && action !== 'edit' && number === undefined) {
     const verb = action === 'status' ? 'inspect' : action;
-    const target = action === 'edit' ? 'the review comment on a pull request' : 'a pull request';
-    fail(`--number is required to ${verb} ${target}`);
-  } else if (action !== 'resolve' && (!Number.isInteger(Number(number)) || Number(number) <= 0)) {
+    fail(`--number is required to ${verb} a pull request`);
+  }
+  // `--resolve` and `--edit` name their target by id and need no pull request number, but one given
+  // anyway still has to be a number.
+  if (number !== undefined && (!Number.isInteger(Number(number)) || Number(number) <= 0)) {
     fail(`--number must be a positive integer, got ${JSON.stringify(number)}`);
   }
   if (action === 'update' && title === undefined && body === undefined && base === undefined) {
@@ -260,7 +262,7 @@ export async function runPr(argv: readonly string[]): Promise<number> {
         host,
         path: repo,
         action,
-        ...(number === undefined ? {} : { number: Number(number) }),
+        ...(number === undefined || action === 'edit' ? {} : { number: Number(number) }),
         ...(action === 'reply' ? { commentId: Number(replyTo) } : {}),
         ...(action === 'edit' ? { commentId: Number(editId) } : {}),
         ...(action === 'resolve' ? { threadId: args.value('resolve') } : {}),
diff --git a/src/providers/github-app.ts b/src/providers/github-app.ts
index 336dbef..937610c 100644
--- a/src/providers/github-app.ts
+++ b/src/providers/github-app.ts
@@ -818,34 +818,11 @@ function pullRequestNumber(value: unknown): number | undefined {
       }
 
       if (request.action === 'edit') {
-        // A review comment is addressed by its own id, so unlike a reply the route carries no pull request
-        // number — and that id is one for the whole repository, not one per pull request. Read the comment
-        // first and refuse when it belongs to a different pull request than the one named: a mutation is
-        // the wrong place to discover that `--number` was wrong, and the reply would otherwise claim a
-        // target it never touched. The repository in the path is still what the allowlist binds.
+        // A review comment is addressed by its own id, and that id is one for the whole repository, not
+        // one per pull request. The route therefore carries no pull request number and the CLI asks for
+        // none: the number to report is the one the comment itself names, read back from the answer
+        // rather than echoed from the request. The repository in the path is what the allowlist binds.
         const endpoint = `${api}/repos/${request.owner}/${request.repo}/pulls/comments/${String(request.commentId ?? 0)}`;
-        const { text: current } = await callWithToken('GET', endpoint, token);
-        let read: unknown = null;
-        try {
-          read = JSON.parse(current);
-        } catch {
-          read = null;
-        }
-        const found = (read ?? {}) as { pull_request_url?: unknown };
-        const belongsTo = pullRequestNumber(found.pull_request_url);
-        // A plain Error on purpose: which pull request a comment is on is the host log's business, not
-        // the container's, and `--number` is the caller's own claim about it.
-        if (belongsTo === undefined) {
-          throw new Error(
-            `refusing to edit comment ${String(request.commentId)}: could not read which pull request it belongs to`,
-          );
-        }
-        if (request.number !== undefined && belongsTo !== request.number) {
-          throw new Error(
-            `refusing to edit comment ${String(request.commentId)}: it is a review comment of ` +
-              `${request.owner}/${request.repo}#${belongsTo}, not #${request.number}`,
-          );
-        }
         const { text: edited } = await callWithToken('PATCH', endpoint, token, { body: request.body ?? '' });
         let changed: unknown = null;
         try {
@@ -853,9 +830,9 @@ function pullRequestNumber(value: unknown): number | undefined {
         } catch {
           changed = null;
         }
-        const html = (changed ?? {}) as { html_url?: unknown };
+        const html = (changed ?? {}) as { html_url?: unknown; pull_request_url?: unknown };
         return {
-          number: belongsTo,
+          number: pullRequestNumber(html.pull_request_url) ?? request.number ?? 0,
           url: typeof html.html_url === 'string' ? html.html_url : '',
         };
       }
diff --git a/src/types.ts b/src/types.ts
index 81dddb0..f8b69f4 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -84,7 +84,7 @@ export interface PullRequestRequest {
   readonly owner: string;
   readonly repo: string;
   readonly action: PullRequestAction;
-  /** The pull request number. Not needed to open one, required for the rest. */
+  /** The pull request number. Not needed to open one, to resolve a thread, or to edit a comment. */
   readonly number?: number;
   /** The review comment being answered or changed, for `action: "reply"` or `"edit"`. */
   readonly commentId?: number;
diff --git a/test/unit/broker.test.ts b/test/unit/broker.test.ts
index 9151dcb..aeec56b 100644
--- a/test/unit/broker.test.ts
+++ b/test/unit/broker.test.ts
@@ -409,14 +409,15 @@ describe('broker request handling', () => {
     assert.equal(wrongAction.code, 'bad-request', 'and an anchor only belongs on a comment');
   });
 
-  it('carries an edit of a review comment, and refuses one with nothing to say', async () => {
+  it('carries an edit by comment id alone, and audits which comment it changed', async () => {
     const seen: Array<Record<string, unknown>> = [];
     const provider: Provider = {
       name: 'with-edits',
       getCredential: () => Promise.reject(new Error('not used here')),
       pullRequest: (request) => {
         seen.push(request as unknown as Record<string, unknown>);
-        return Promise.resolve({ number: request.number ?? 0, url: 'u' });
+        // The provider reads the pull request back from the comment, so the broker need not be told it.
+        return Promise.resolve({ number: 8, url: 'https://github.com/acme/widget/pull/8#discussion_r99' });
       },
     };
     const { handle, events } = makeHarness(['acme/widget'], provider);
@@ -424,29 +425,30 @@ describe('broker request handling', () => {
       ...REQUEST,
       op: 'pull-request',
       action: 'edit',
-      number: 7,
       commentId: 99,
       body: 'a corrected note',
     });
     assert.equal(good.ok, true);
+    assert.equal(good.prNumber, 8);
     assert.deepEqual(
-      [seen.at(-1)?.['action'], seen.at(-1)?.['commentId'], seen.at(-1)?.['body']],
-      ['edit', 99, 'a corrected note'],
-      'the action, the comment id and the new body all reach the provider',
+      [seen.at(-1)?.['action'], seen.at(-1)?.['commentId'], seen.at(-1)?.['body'], seen.at(-1)?.['number']],
+      ['edit', 99, 'a corrected note', undefined],
+      'the action, the comment id and the new body reach the provider, with no number required',
     );
-    assert.equal(events.at(-1)?.['action'], 'edit', 'and the audit records the action');
+    assert.equal(events.at(-1)?.['action'], 'edit', 'the audit records the action');
+    assert.equal(events.at(-1)?.['comment_id'], 99, 'and the comment it changed, which a number would not name');
+    assert.equal(events.at(-1)?.['pr_number'], 8, 'with the number the comment itself reports');
 
     const empty = await handle({
       ...REQUEST,
       op: 'pull-request',
       action: 'edit',
-      number: 7,
       commentId: 99,
       body: '   ',
     });
     assert.equal(empty.code, 'bad-request', 'an edit has to say something');
 
-    const noId = await handle({ ...REQUEST, op: 'pull-request', action: 'edit', number: 7, body: 'a note' });
+    const noId = await handle({ ...REQUEST, op: 'pull-request', action: 'edit', body: 'a note' });
     assert.equal(noId.code, 'bad-request', 'and it has to name the comment it changes');
   });
 
diff --git a/test/unit/commands.test.ts b/test/unit/commands.test.ts
index cabf276..aaad021 100644
--- a/test/unit/commands.test.ts
+++ b/test/unit/commands.test.ts
@@ -714,12 +714,19 @@ describe('the anchor flags, at the CLI boundary', () => {
    * Every case here is refused by `fail`, which exits, so it cannot be a call into the module — and that is
    * the point of testing it here: the checks are the CLI's, and the unit tests under them never see them.
    */
+  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 } => {
     try {
       execFileSync('node', ['src/cli/helper.ts', 'pr', ...args], {
         cwd: path.resolve('.'),
         encoding: 'utf8',
         stdio: ['ignore', 'pipe', 'pipe'],
+        // A test-owned HOME and no socket in the environment: the fallback must not depend on the machine
+        // that runs the suite, and a case that gets past validation has to die at the socket rather than
+        // reach a broker someone happens to have running.
+        env: { ...process.env, HOME: home, GIT_BROKER_SOCKET: '', GIT_BROKER_SOCKET_FILE: '' },
       });
       return { status: 0, stderr: '' };
     } catch (error) {
@@ -789,4 +796,17 @@ describe('the anchor flags, at the CLI boundary', () => {
       assert.match(refused.stderr, message);
     }
   });
+
+  it('needs no --number to edit, which names the comment instead', () => {
+    // The route is /pulls/comments/{id}: it carries no pull request number, so requiring one would be a
+    // flag the command could not honour. This case gets past validation and dies at the socket, which is
+    // exactly the proof that the number check let it through.
+    const noNumber = cli([
+      '--host', 'github.com', '--repo', 'acme/widget',
+      '--edit', '99', '--body', 'a corrected note',
+    ]);
+    assert.equal(noNumber.status, 1);
+    assert.match(noNumber.stderr, /no broker socket/, 'it reached socket resolution');
+    assert.doesNotMatch(noNumber.stderr, /--number is required/, 'without being asked for a number');
+  });
 });
diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts
index 7c02dae..8479936 100644
--- a/test/unit/github-app.test.ts
+++ b/test/unit/github-app.test.ts
@@ -765,40 +765,38 @@ describe('createGithubAppProvider', () => {
     const patch = stub.calls.find((call) => call.method === 'PATCH' && call.url.includes('/pulls/comments/'));
     assert.ok(patch, 'a review comment is changed at the review comments endpoint');
     // The comment id names the thing and is one for the whole repository, so the route carries no pull
-    // request number; the comment is read first to tie it back to the number the caller named.
+    // request number and nothing is read first: the number to report comes back in the answer.
     assert.equal(patch?.url, 'https://api.github.com/repos/acme/widget/pulls/comments/99');
     assert.deepEqual(patch?.body, { body: 'a corrected note' });
     assert.equal(patch?.authorization, 'Bearer ghs_stub_token_value', 'and it is the app that changes it');
-    const read = stub.calls.find((call) => call.method === 'GET' && call.url.endsWith('/pulls/comments/99'));
-    assert.ok(read, 'the comment is read before it is changed');
+    assert.equal(
+      stub.calls.some((call) => call.method === 'GET' && call.url.includes('/pulls/comments/')),
+      false,
+      'no round trip to read the comment first',
+    );
     assert.equal(changed.number, 7);
     assert.equal(changed.url, 'https://github.com/acme/widget/pull/7#discussion_r99');
   });
 
-  it('refuses to edit a comment that belongs to another pull request, and changes nothing', async () => {
+  it("reports the comment's own pull request, not the number the caller passed", async () => {
+    // The route cannot honour a pull request number, so the provider must not echo one: the review
+    // comment object names the pull request it belongs to, and that is what the CLI reports.
     const stub = createStubFetch({
       installations: [{ id: 42, account: { login: 'acme' } }],
       nowMs,
       commentPr: 8,
     });
-    await assert.rejects(
-      providerFor(stub).pullRequest!({
-        action: 'edit',
-        host: 'github.com',
-        owner: 'acme',
-        repo: 'widget',
-        number: 7,
-        commentId: 99,
-        body: 'a corrected note',
-      }),
-      /#8, not #7/,
-      'the mismatch is refused by name',
-    );
-    assert.equal(
-      stub.calls.some((call) => call.method === 'PATCH'),
-      false,
-      'and the comment on the other pull request is left alone',
-    );
+    const changed = await providerFor(stub).pullRequest!({
+      action: 'edit',
+      host: 'github.com',
+      owner: 'acme',
+      repo: 'widget',
+      number: 7,
+      commentId: 99,
+      body: 'a corrected note',
+    });
+    assert.equal(changed.number, 8, 'the comment names #8, so #8 is what is reported');
+    assert.equal(changed.url, 'https://github.com/acme/widget/pull/8#discussion_r99');
   });