From d520453b071edd1a6ac72c3f4187e2850b9b8e12 Mon Sep 17 00:00:00 2001 From: Ogden-2688 Date: Tue, 27 Oct 2026 16:32:58 -0400 Subject: [PATCH 1/3] Test what a tool says when a write's answer is lost A fake GitHub keeps a request and then closes the socket without answering, as a connection that drops after GitHub stored a write does: - a comment and an issue GitHub received should read as unknown, not as failed, and the comment's text should say a rollback won't undo it; - GitHub's own 5xx on a write should read as unknown too; - a read lost the same way, and a write GitHub refused with a 4xx, still read as failed; - a lost GraphQL mutation may have taken effect, and a lost GraphQL query didn't; - a write tool's own lookup whose answer was lost (upsert_sticky_comment's listComments) is a failed read, and nothing is written; - a token mint whose answer was lost is a failure of the tool's request, which never went out, for a read and for a comment alike. --- test/lost-answer.test.ts | 212 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 test/lost-answer.test.ts diff --git a/test/lost-answer.test.ts b/test/lost-answer.test.ts new file mode 100644 index 0000000..f229511 --- /dev/null +++ b/test/lost-answer.test.ts @@ -0,0 +1,212 @@ +import { strict as assert } from 'node:assert'; +import { generateKeyPairSync } from 'node:crypto'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { createServer, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, test } from 'node:test'; +import { GitHubAdapter, GitHubApiError } from '../src/github-adapter.js'; +import { GithubState } from '../src/state.js'; +import { commentsToolModule } from '../src/tools-comments.js'; +import { issuesToolModule } from '../src/tools-issues-pr.js'; +import { readToolModule } from '../src/tools-read.js'; +import { readConfig, silentLogger, type RepoRef, type ThreadRef, type ToolContext, type ToolResult } from '../src/types.js'; + +const REPO: RepoRef = { owner: 'anima-research', name: 'rayforce' }; +const PULL: ThreadRef = { repo: REPO, number: 12, kind: 'pull' }; +const R = '/repos/anima-research/rayforce'; +const INSTALLATION_ID = 42; + +const KEY_FILE = join(mkdtempSync(join(tmpdir(), 'github-mcpl-key-')), 'app.pem'); +writeFileSync( + KEY_FILE, + generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }).privateKey, + { mode: 0o600 }, +); + +type Answer = { status: number; body?: unknown } | 'lost'; + +// 'lost' keeps the whole request and closes the socket without answering, as +// a connection that drops after GitHub stored a write does. +class FakeGitHub { + readonly received: { method: string; path: string; body: string }[] = []; + private readonly answers = new Map([ + [`POST /app/installations/${INSTALLATION_ID}/access_tokens`, { status: 201, body: { token: 'ghs_test', expires_at: new Date(Date.now() + 3_600_000).toISOString() } }], + [`GET ${R}/installation`, { status: 200, body: { id: INSTALLATION_ID } }], + ]); + private server: Server | null = null; + baseUrl = ''; + + on(route: string, answer: Answer): this { + this.answers.set(route, answer); + return this; + } + + async start(): Promise { + this.server = createServer((req, res) => { + let body = ''; + req.on('data', (chunk: Buffer) => (body += chunk.toString('utf8'))); + req.on('end', () => { + const path = new URL(req.url ?? '/', 'http://x').pathname; + this.received.push({ method: req.method ?? '', path, body }); + const answer = this.answers.get(`${req.method} ${path}`) ?? { status: 404, body: { message: 'Not Found' } }; + if (answer === 'lost') { + req.socket.destroy(); + return; + } + res.writeHead(answer.status, { 'content-type': 'application/json' }); + res.end(answer.body === undefined ? '' : JSON.stringify(answer.body)); + }); + }); + await new Promise((resolve) => this.server!.listen(0, '127.0.0.1', resolve)); + this.baseUrl = `http://127.0.0.1:${(this.server.address() as AddressInfo).port}`; + } + + async stop(): Promise { + await new Promise((resolve) => this.server?.close(() => resolve()) ?? resolve()); + } +} + +const fakes: FakeGitHub[] = []; +after(async () => { + await Promise.all(fakes.map((f) => f.stop())); +}); + +async function rig(routes: Record): Promise<{ fake: FakeGitHub; adapter: GitHubAdapter; ctx: ToolContext }> { + const fake = new FakeGitHub(); + for (const [route, answer] of Object.entries(routes)) fake.on(route, answer); + await fake.start(); + fakes.push(fake); + const config = readConfig({ + GITHUB_APP_ID: '123456', + GITHUB_APP_PRIVATE_KEY_FILE: KEY_FILE, + GITHUB_APP_INSTALLATION_ID: String(INSTALLATION_ID), + GITHUB_REPOS: 'anima-research/rayforce', + GITHUB_BOT_LOGIN: 'rayforce-clone[bot]', + GITHUB_PR_BASE_ALLOW: 'dev', + }); + const logger = silentLogger(); + const adapter = new GitHubAdapter(config, { baseUrl: fake.baseUrl, logger, sleep: async () => {} }); + const ctx: ToolContext = { + api: adapter, + state: GithubState.empty(null, logger), + botLogin: 'rayforce-clone[bot]', + repos: [REPO], + prBaseAllow: ['dev'], + logger, + }; + return { fake, adapter, ctx }; +} + +function text(result: ToolResult): string { + return result.content.map((c) => ('text' in c ? c.text : '')).join('\n'); +} + +test('a comment GitHub stored whose answer was lost reads as unknown, not as failed', { timeout: 10_000 }, async () => { + const { fake, ctx } = await rig({ [`POST ${R}/issues/7/comments`]: 'lost' }); + + const result = await commentsToolModule.call('comment', { thread: 7, body: 'hello' }, ctx); + + assert.ok(fake.received.some((r) => r.method === 'POST' && r.path === `${R}/issues/7/comments` && r.body.includes('hello')), 'GitHub got the comment'); + assert.equal(result.isError, true); + const said = text(result); + assert.match(said, /whether it took effect is unknown/); + assert.match(said, /rollback has no record of it/); + assert.doesNotMatch(said, /failed/); +}); + +test("GitHub's own 5xx on a write reads as unknown too", { timeout: 10_000 }, async () => { + const { ctx } = await rig({ [`POST ${R}/issues/7/comments`]: { status: 502, body: { message: 'Server Error' } } }); + + const result = await commentsToolModule.call('comment', { thread: 7, body: 'hello' }, ctx); + + assert.equal(result.isError, true); + assert.match(text(result), /GitHub answered createComment with 502 \(Server Error\), so whether it took effect is unknown/); +}); + +test('an issue GitHub created whose answer was lost reads as unknown, not as failed', { timeout: 10_000 }, async () => { + const { fake, ctx } = await rig({ [`POST ${R}/issues`]: 'lost' }); + + const result = await issuesToolModule.call('create_issue', { title: 'A bug' }, ctx); + + assert.ok(fake.received.some((r) => r.method === 'POST' && r.path === `${R}/issues`), 'GitHub got the issue'); + assert.equal(result.isError, true); + assert.match(text(result), /^create_issue: no answer came back from GitHub for createIssue \(.+\), so whether it took effect is unknown/); + assert.doesNotMatch(text(result), /failed|rollback/); +}); + +test('a read whose answer was lost still reads as failed', { timeout: 10_000 }, async () => { + const { ctx } = await rig({ [`GET ${R}/issues/7`]: 'lost' }); + + const result = await readToolModule.call('get_issue', { number: 7 }, ctx); + + assert.equal(result.isError, true); + assert.match(text(result), /^get_issue failed: /); + assert.doesNotMatch(text(result), /unknown/); +}); + +test("a write tool's own lookup whose answer was lost is a failed read, and nothing is written", { timeout: 10_000 }, async () => { + const { fake, ctx } = await rig({ [`GET ${R}/issues/7/comments`]: 'lost' }); + + const failure = await commentsToolModule.call('upsert_sticky_comment', { thread: 7, marker: 'ci-status', body: 'green' }, ctx).then( + () => assert.fail('upsert_sticky_comment should have thrown'), + (err: unknown) => err, + ); + + assert.ok(failure instanceof GitHubApiError); + assert.equal(failure.operation, 'listComments'); + assert.notEqual(failure.possiblyDone, true); + assert.ok(!fake.received.some((r) => r.method !== 'GET' && r.path.startsWith(R)), 'nothing was written'); +}); + +test("a token mint whose answer was lost is a failure of the tool's request, which never went out", { timeout: 10_000 }, async () => { + const { fake, ctx } = await rig({ [`POST /app/installations/${INSTALLATION_ID}/access_tokens`]: 'lost' }); + + const read = await readToolModule.call('get_issue', { number: 7 }, ctx); + assert.equal(read.isError, true); + assert.match(text(read), /^get_issue failed: /); + assert.doesNotMatch(text(read), /unknown/); + + const write = await commentsToolModule.call('comment', { thread: 7, body: 'hello' }, ctx).then( + () => assert.fail('comment should have thrown'), + (err: unknown) => err, + ); + assert.ok(write instanceof GitHubApiError); + assert.equal(write.operation, 'mintInstallationToken'); + assert.notEqual(write.possiblyDone, true); + assert.ok(!fake.received.some((r) => r.path.startsWith(R)), 'no request for the repository went out'); +}); + +test('a write GitHub refused with a 4xx still reads as failed', { timeout: 10_000 }, async () => { + const { ctx } = await rig({ [`POST ${R}/issues`]: { status: 422, body: { message: 'Validation Failed' } } }); + + const result = await issuesToolModule.call('create_issue', { title: 'A bug' }, ctx); + + assert.equal(result.isError, true); + assert.match(text(result), /^create_issue failed: GitHub error 422: /); +}); + +test('a GraphQL mutation whose answer was lost may have taken effect; a GraphQL query may not', { timeout: 10_000 }, async () => { + const { adapter } = await rig({ 'POST /graphql': 'lost' }); + + const mutation = await adapter.resolveReviewThread('PRRT_x').then( + () => assert.fail('resolveReviewThread should have thrown'), + (err: unknown) => err, + ); + assert.ok(mutation instanceof GitHubApiError); + assert.equal(mutation.possiblyDone, true); + assert.equal(mutation.answered, false); + + const query = await adapter.listReviewThreads(PULL).then( + () => assert.fail('listReviewThreads should have thrown'), + (err: unknown) => err, + ); + assert.ok(query instanceof GitHubApiError); + assert.equal(query.possiblyDone, false); +}); + From fe29c403c14f61e4982f0ef409af319e187468f5 Mon Sep 17 00:00:00 2001 From: Ogden-2688 Date: Tue, 27 Oct 2026 16:32:59 -0400 Subject: [PATCH 2/3] Say a write's outcome is unknown when its answer is lost A write whose request went out but whose answer never came back, or that GitHub answered with a 5xx, may have taken effect. Until now the tools said it failed, and an agent reading that may write again. - github-adapter: normalizeFailure also keeps whether the failed request only read (a GET or HEAD, or a GraphQL document that begins with query or {) and whether GitHub answered at all. GitHubApiError carries them as possiblyDone (status 500 or more, and not a read) and answered. A failure whose request can't be judged counts as a possible write. A failure to mint a token or find the installation comes before the request is sent, so it is never possibly done. - tool-support: describeFailure says, for such a failure, that whether it took effect is unknown, and to look on GitHub before trying again. - tools-comments: the comment tools, which threw on any failure, return that text for this case as an error result, and add that rollback has no record of the write if it took effect: it was never booked. - server and tool-support: three comments that said an error result means nothing happened now say only what holds. - A changelog fragment. channels/publish is unchanged: an error answer to a publish doesn't claim the message wasn't posted, and agent-framework, for one, reads it as delivery uncertain. --- changelog.d/lost-answer.fixed.md | 7 +++++ src/github-adapter.ts | 53 ++++++++++++++++++++++++++------ src/server.ts | 9 +++--- src/tool-support.ts | 19 ++++++++++-- src/tools-comments.ts | 40 +++++++++++++++--------- 5 files changed, 96 insertions(+), 32 deletions(-) create mode 100644 changelog.d/lost-answer.fixed.md diff --git a/changelog.d/lost-answer.fixed.md b/changelog.d/lost-answer.fixed.md new file mode 100644 index 0000000..b37d646 --- /dev/null +++ b/changelog.d/lost-answer.fixed.md @@ -0,0 +1,7 @@ +- A write tool that got no answer from GitHub, or a 5xx, no longer says it + failed: it says whether the write took effect is unknown, and to look on + GitHub before trying again, since a retry may do it twice. The comment + tools, which answered such a failure with a JSON-RPC error, now answer with + an error result saying so, and add that rollback has no record of the write + if it took effect. A read, and a write GitHub refused with a 4xx, still say + they failed. diff --git a/src/github-adapter.ts b/src/github-adapter.ts index 00d6419..36321af 100644 --- a/src/github-adapter.ts +++ b/src/github-adapter.ts @@ -192,6 +192,8 @@ export class GitHubApiError extends Error { githubMessage: string, /** The GitHubApi method that failed, e.g. `getIssue`. Never a URL. */ readonly operation: string, + readonly possiblyDone = false, + readonly answered = true, ) { const safe = redactSecrets(githubMessage); super(`GitHub ${status} on ${operation}: ${safe}`); @@ -230,6 +232,8 @@ interface Failure { status: number; message: string; headers: Record; + read: boolean; + answered: boolean; } // ───────────────────────────────────────────────────────────────────────────── @@ -835,12 +839,15 @@ export class GitHubAdapter implements GitHubApi { try { return await fn(); } catch (raw) { - // Auth runs inside the transport, so a failure to mint a token or to - // find the installation surfaces here wrapped in octokit's generic - // "the fetch threw" error. Unwrap it and let the real status and the - // real operation out instead of a 500 from `getIssue`. + // Auth runs inside the transport, before the request is sent, so a + // failure to mint a token or to find the installation surfaces here + // wrapped in octokit's generic "the fetch threw" error. Unwrap it and + // let the real status and the real operation out instead of a 500 + // from `getIssue`, as a failure of a request that never went out. const authFailure = authFailureIn(raw); - if (authFailure) throw authFailure; + if (authFailure) { + throw new GitHubApiError(authFailure.status, authFailure.githubMessage, authFailure.operation); + } const failure = normalizeFailure(raw); @@ -864,7 +871,13 @@ export class GitHubAdapter implements GitHubApi { continue; } - throw new GitHubApiError(failure.status, this.redact(failure.message), operation); + throw new GitHubApiError( + failure.status, + this.redact(failure.message), + operation, + failure.status >= 500 && !failure.read, + failure.answered, + ); } } } @@ -1713,8 +1726,9 @@ export function summarizeChecks( } /** - * Turn whatever octokit threw into a status, GitHub's message and the headers - * the retry decision needs. Nothing else of the error is kept: octokit's + * Turn whatever octokit threw into a status, GitHub's message, the headers + * the retry decision needs, whether the request only read, and whether GitHub + * answered at all. Nothing else of the error is kept: octokit's * `RequestError` carries the request it made, including its headers, and that * object must not travel any further than this function. */ @@ -1722,10 +1736,13 @@ function normalizeFailure(raw: unknown): Failure { const err = raw as { status?: number; message?: string; + request?: { method?: string; url?: string; body?: unknown }; response?: { headers?: Record; data?: unknown; status?: number }; errors?: Array<{ message?: string }>; } | null; + const read = isReadRequest(err?.request?.method, err?.request?.url, err?.request?.body); + const answered = err?.response !== undefined; const headers: Record = {}; for (const [k, v] of Object.entries(err?.response?.headers ?? {})) { if (typeof v === 'string' || typeof v === 'number') headers[k.toLowerCase()] = String(v); @@ -1735,10 +1752,26 @@ function normalizeFailure(raw: unknown): Failure { // a GraphqlResponseError, which has no status of its own. if (Array.isArray(err?.errors) && err.errors.length > 0 && err?.status === undefined) { const message = err.errors.map((e) => e?.message ?? '').filter(Boolean).join('; '); - return { status: err.response?.status ?? 200, message: message || 'GraphQL error', headers }; + return { status: err.response?.status ?? 200, message: message || 'GraphQL error', headers, read, answered }; } const data = err?.response?.data as { message?: string } | undefined; const message = data?.message ?? err?.message ?? 'unknown error'; - return { status: err?.status ?? err?.response?.status ?? 0, message, headers }; + return { status: err?.status ?? err?.response?.status ?? 0, message, headers, read, answered }; +} + +function isReadRequest(method: string | undefined, url: string | undefined, body: unknown): boolean { + const verb = method?.toUpperCase(); + if (verb === 'GET' || verb === 'HEAD') return true; + if (verb !== 'POST' || !url?.endsWith('/graphql')) return false; + let document = body; + if (typeof document === 'string') { + try { + document = JSON.parse(document); + } catch { + return false; + } + } + const query = (document as { query?: unknown } | null)?.query; + return typeof query === 'string' && /^(?:\s|#[^\n]*)*(?:query\b|\{)/.test(query); } diff --git a/src/server.ts b/src/server.ts index 9a6f4f5..a34d9cb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -982,16 +982,17 @@ export class GitHubMcplServer { }; const result = await module.call(name, args, ctx); - // A refused or failed call touched nothing: it may have read a thread on - // the way to refusing, and that is not a reason to announce a channel or - // to claim the bot has written anywhere. + // An error result claims nothing: the call may have read a thread on the + // way to refusing, or written where GitHub's answer was lost, and neither + // is a reason to announce a channel or to claim the bot has written + // anywhere. if (result.isError === true) return result; await this.afterToolCall(name, touches, changes); // §8.2: a rollback-capable feature set checkpoints after the write, and // the result names the checkpoint and its parent. A failed call is not - // checkpointed — there is nothing to undo. + // checkpointed: nothing it may have done is booked to undo. if (module.featureSet !== COMMENTS_FEATURE_SET) return result; try { return { ...result, state: await this.checkpoint(COMMENTS_FEATURE_SET) }; diff --git a/src/tool-support.ts b/src/tool-support.ts index ddc4f0b..fc0abbe 100644 --- a/src/tool-support.ts +++ b/src/tool-support.ts @@ -28,6 +28,8 @@ import { type ToolContext, type ToolResult, } from './types.js'; +import { GitHubApiError } from './github-adapter.js'; + // ───────────────────────────────────────────────────────────────────────────── // Bounds // ───────────────────────────────────────────────────────────────────────────── @@ -142,6 +144,8 @@ export function describeFailure(toolName: string, err: unknown): string { const tool = field(toolName); if (err instanceof ToolRefusal) return `${tool} refused: ${err.message}`; if (err instanceof ToolArgumentError) return `${tool}: ${err.message}`; + const uncertain = uncertainWrite(err); + if (uncertain) return `${tool}: ${uncertain}`; const message = err instanceof Error ? err.message : String(err); const status = statusOf(err); return status === undefined @@ -149,10 +153,19 @@ export function describeFailure(toolName: string, err: unknown): string { : `${tool} failed: GitHub error ${status}: ${bounded(message, LIMITS.commentChars)}`; } +export function uncertainWrite(err: unknown): string | null { + if (!(err instanceof GitHubApiError) || !err.possiblyDone) return null; + const operation = field(err.operation); + const detail = bounded(err.githubMessage, LIMITS.commentChars); + const what = err.answered + ? `GitHub answered ${operation} with ${err.status} (${detail})` + : `no answer came back from GitHub for ${operation} (${detail})`; + return `${what}, so whether it took effect is unknown. Look on GitHub before trying again: a retry may do it twice.`; +} + /** - * Run one tool body. Every refusal, malformed call and GitHub error comes back - * as an error *result* the model can read (slack-mcpl's convention), never as - * a thrown protocol error: the call was permitted, it just did not happen. + * A refusal, a malformed call or a GitHub error comes back as an error result + * the model can read, not as a JSON-RPC error. */ export async function runTool( tool: string, diff --git a/src/tools-comments.ts b/src/tools-comments.ts index de9ee53..3ffe192 100644 --- a/src/tools-comments.ts +++ b/src/tools-comments.ts @@ -35,7 +35,7 @@ import { type StateRollbackResult, } from '@animalabs/mcpl-core'; import { McplRpcError } from './errors.js'; -import { THREAD_PROPERTY, parseThread, type Parsed } from './tool-support.js'; +import { THREAD_PROPERTY, field, parseThread, uncertainWrite, type Parsed } from './tool-support.js'; import type { GithubStateStore, ThreadLocator } from './state.js'; import { COMMENTS_FEATURE_SET, @@ -357,24 +357,34 @@ export const COMMENT_TOOLS: ToolDefinition[] = [ export const commentsToolModule: ToolModule = { featureSet: COMMENTS_FEATURE_SET, tools: COMMENT_TOOLS, - call(name, args, ctx) { - switch (name) { - case TOOL_COMMENT: - return comment(args, ctx); - case TOOL_REPLY_IN_THREAD: - return replyInThread(args, ctx); - case TOOL_EDIT_OWN_COMMENT: - return editOwnComment(args, ctx); - case TOOL_DELETE_OWN_COMMENT: - return deleteOwnComment(args, ctx); - case TOOL_UPSERT_STICKY_COMMENT: - return upsertStickyComment(args, ctx); - default: - return Promise.resolve(refuse(`unknown tool: ${name}`)); + async call(name, args, ctx) { + try { + return await callComment(name, args, ctx); + } catch (err) { + const uncertain = uncertainWrite(err); + if (!uncertain) throw err; + return refuse(`${field(name)}: ${uncertain} If it did, rollback has no record of it.`); } }, }; +function callComment(name: string, args: Record, ctx: ToolContext): Promise { + switch (name) { + case TOOL_COMMENT: + return comment(args, ctx); + case TOOL_REPLY_IN_THREAD: + return replyInThread(args, ctx); + case TOOL_EDIT_OWN_COMMENT: + return editOwnComment(args, ctx); + case TOOL_DELETE_OWN_COMMENT: + return deleteOwnComment(args, ctx); + case TOOL_UPSERT_STICKY_COMMENT: + return upsertStickyComment(args, ctx); + default: + return Promise.resolve(refuse(`unknown tool: ${name}`)); + } +} + async function comment(args: Record, ctx: ToolContext): Promise { const thread = parseThreadArg(args.thread, ctx); if (!thread.ok) return refuse(thread.error); From 72616ecced0bbc77dc95337f2074bbe78d5c7617 Mon Sep 17 00:00:00 2001 From: Ogden-2688 Date: Tue, 27 Oct 2026 17:23:08 -0400 Subject: [PATCH 3/3] Log an uncertain comment write, and read the adapter's error by its fields - tools-comments: the comment tools log a write whose outcome is unknown, at warn level as runTool does for every other tool. Before, the error they threw reached the dispatcher's log. - tool-support: uncertainWrite reads possiblyDone, answered, status, operation and githubMessage off the error, as statusOf reads status, instead of importing the adapter's error class. The tool layer talks to the adapter only through types.ts again. --- src/tool-support.ts | 22 ++++++++++++++-------- src/tools-comments.ts | 4 +++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/tool-support.ts b/src/tool-support.ts index fc0abbe..4a13964 100644 --- a/src/tool-support.ts +++ b/src/tool-support.ts @@ -28,8 +28,6 @@ import { type ToolContext, type ToolResult, } from './types.js'; -import { GitHubApiError } from './github-adapter.js'; - // ───────────────────────────────────────────────────────────────────────────── // Bounds // ───────────────────────────────────────────────────────────────────────────── @@ -154,12 +152,20 @@ export function describeFailure(toolName: string, err: unknown): string { } export function uncertainWrite(err: unknown): string | null { - if (!(err instanceof GitHubApiError) || !err.possiblyDone) return null; - const operation = field(err.operation); - const detail = bounded(err.githubMessage, LIMITS.commentChars); - const what = err.answered - ? `GitHub answered ${operation} with ${err.status} (${detail})` - : `no answer came back from GitHub for ${operation} (${detail})`; + const e = err as { + possiblyDone?: unknown; + answered?: unknown; + status?: unknown; + operation?: unknown; + githubMessage?: unknown; + } | null; + if (e?.possiblyDone !== true) return null; + const operation = field(String(e.operation)); + const detail = bounded(String(e.githubMessage), LIMITS.commentChars); + const what = + e.answered === false + ? `no answer came back from GitHub for ${operation} (${detail})` + : `GitHub answered ${operation} with ${String(e.status)} (${detail})`; return `${what}, so whether it took effect is unknown. Look on GitHub before trying again: a retry may do it twice.`; } diff --git a/src/tools-comments.ts b/src/tools-comments.ts index 3ffe192..7813556 100644 --- a/src/tools-comments.ts +++ b/src/tools-comments.ts @@ -363,7 +363,9 @@ export const commentsToolModule: ToolModule = { } catch (err) { const uncertain = uncertainWrite(err); if (!uncertain) throw err; - return refuse(`${field(name)}: ${uncertain} If it did, rollback has no record of it.`); + const text = `${field(name)}: ${uncertain} If it did, rollback has no record of it.`; + ctx.logger.warn('tool call failed', { tool: field(name), detail: text }); + return refuse(text); } }, };