From a48dc40fa3c1c418cc518b728f674cb014f1b74c Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 14:48:54 +0000 Subject: [PATCH 01/10] feat: tell the caller when a token revoke fails Return a warning through the action result and socket response, print it in gcb gh, and mark the token lifecycle item complete. --- ROADMAP.md | 5 ++--- packages/cli/src/types.ts | 4 ++++ packages/client/src/commands/gh.ts | 1 + packages/host/src/broker.ts | 1 + packages/host/src/providers/github-app.ts | 8 +++++-- test/unit/broker.test.ts | 26 +++++++++++++++++++++++ test/unit/github-app.test.ts | 1 + 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 977abe4c..e175ae96 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -242,12 +242,11 @@ host_broker: - per-action total timeout, `SIGTERM` then `SIGKILL`, kill the process group; - no automatic retry. A timed-out write reports that it may have completed. (Current executor sends SIGKILL only; its timeout starts after preflight/token mint.) -- [ ] Token lifecycle: +- [x] Token lifecycle: - mint per action with only that action's permissions; - never cache; - `finally`-revoke on success, failure, and timeout; - - if revoke fails, log host-side and report that the token expires on its own. (The host log is - now wired; the caller is not yet told.) + - if revoke fails, log host-side and report that the token expires on its own. - [ ] Single-source action catalog (table below): one declarative table drives the permission preflight, token mint, command table and docs; configured actions extend it under the Phase 1 rules. A test rejects drift between catalog, preflight, mint and docs, and rejects any built-in diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index 11ef8828..c662e4a8 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -38,6 +38,8 @@ export interface ActionRunResult { readonly error?: string; /** SHA-256 prefix of the action token, for the audit record only. */ readonly tokenFingerprint?: string; + /** Set when the action succeeded but its token could not be revoked. */ + readonly warning?: string; } /** One credential, as handed to git by the helper. */ @@ -279,6 +281,8 @@ export interface WireResponse { readonly stderr?: string; readonly exitCode?: number | null; readonly timedOut?: boolean; + /** A non-fatal action warning, for example a failed token revoke. */ + readonly warning?: string; } /** Where audit records go. */ diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index ffa3f5f2..5883c78d 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -127,6 +127,7 @@ export async function runGh(argv: readonly string[]): Promise { } if (response.stdout) process.stdout.write(response.stdout); if (response.stderr) process.stderr.write(response.stderr); + if (response.warning) process.stderr.write(`warning: ${response.warning}\n`); if (response.truncated === true) { process.stderr.write('warning: the action output was truncated at the broker\x27s limit\n'); } diff --git a/packages/host/src/broker.ts b/packages/host/src/broker.ts index b5ed3b74..89cc30a6 100644 --- a/packages/host/src/broker.ts +++ b/packages/host/src/broker.ts @@ -319,6 +319,7 @@ export function createRequestHandler( exitCode: result.code, truncated: result.truncated, timedOut: result.timedOut, + ...(result.warning === undefined ? {} : { warning: result.warning }), }; } catch (error) { const reason = String((error as Error).message ?? error).slice(0, 300); diff --git a/packages/host/src/providers/github-app.ts b/packages/host/src/providers/github-app.ts index b39011e9..7e66bd9e 100644 --- a/packages/host/src/providers/github-app.ts +++ b/packages/host/src/providers/github-app.ts @@ -578,8 +578,10 @@ function pullRequestNumber(value: unknown): number | undefined { if (typeof token !== 'string' || !token) { throw new ProviderConfigError('GitHub returned no installation token for the action'); } + let result!: Awaited>; + let warning: string | undefined; try { - const result = await runGhAction({ + result = await runGhAction({ action: request.action, values: request.values, repo: request.full, @@ -588,7 +590,6 @@ function pullRequestNumber(value: unknown): number | undefined { ghPath, env: process.env, }); - return { ...result, tokenFingerprint: tokenFingerprint(token) }; } finally { try { const response = await fetchImpl(`${api}/installation/token`, { @@ -608,11 +609,14 @@ function pullRequestNumber(value: unknown): number | undefined { // The status is enough; a body that cannot be read is still not a successful revoke. } log(`could not revoke the action token: GitHub answered ${response.status}${detail ? ` ${detail}` : ''}`); + warning = 'the action token could not be revoked; it expires on its own'; } } catch (error) { log(`could not revoke the action token: ${String((error as Error).message ?? error)}`); + warning = 'the action token could not be revoked; it expires on its own'; } } + return { ...result, tokenFingerprint: tokenFingerprint(token), ...(warning === undefined ? {} : { warning }) }; }, async getCredential(request): Promise { diff --git a/test/unit/broker.test.ts b/test/unit/broker.test.ts index 377510ef..ad3a9ddd 100644 --- a/test/unit/broker.test.ts +++ b/test/unit/broker.test.ts @@ -577,6 +577,32 @@ describe('broker actions', () => { assert.equal(record?.['token_fingerprint'], 'action-fingerprint'); }); + it('passes a provider warning to the caller', async () => { + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async () => ({ + stdout: 'ok', + stderr: '', + code: 0, + signal: null, + truncated: false, + timedOut: false, + warning: 'the action token could not be revoked; it expires on its own', + }), + }; + const harness = makeHarness(undefined, provider); + const response = await harness.handle({ + op: 'action', + host: HOST, + path: 'acme/widget', + ghAction: 'pr:view', + values: { number: 7 }, + }); + assert.equal(response.ok, true); + assert.match(String(response.warning), /expires on its own/); + }); + it('refuses an undeclared flag before the provider', async () => { let called = false; const provider: Provider = { diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index 299ea83e..386a1e47 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -319,6 +319,7 @@ describe('createGithubAppProvider', () => { assert.equal(result?.code, 0); assert.ok(logs.some((line) => line.includes('could not revoke the action token'))); assert.ok(logs.some((line) => line.includes('500'))); + assert.match(result?.warning ?? '', /expires on its own/); } finally { rmSync(dir, { recursive: true, force: true }); } From aad2a8a2d8c085923d74500f7452c5d781f87e9d Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 14:57:00 +0000 Subject: [PATCH 02/10] test: cover the revoke warning in the client Use ActionRunResult for the provider result, share the warning literal, and assert gcb gh prints it. --- packages/host/src/providers/github-app.ts | 9 +++--- test/unit/gh-command.test.ts | 38 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/host/src/providers/github-app.ts b/packages/host/src/providers/github-app.ts index 7e66bd9e..bdb09198 100644 --- a/packages/host/src/providers/github-app.ts +++ b/packages/host/src/providers/github-app.ts @@ -19,7 +19,7 @@ */ import { userTokenPath } from '../config.ts'; import { createSign } from 'node:crypto'; -import { getGhAction, ProviderConfigError, tokenFingerprint } from 'gcb-cli'; +import { getGhAction, ProviderConfigError, tokenFingerprint, type ActionRunResult } from 'gcb-cli'; import { runGhAction } from '../gh.ts'; import { existsSync, readFileSync, writeFileSync } from 'node:fs'; @@ -190,6 +190,7 @@ export function createGithubAppProvider(options: GithubAppProviderOptions): Prov * a token minted for git should not also be able to open a pull request. */ const PULL_REQUEST_PERMISSIONS: Record = { pull_requests: 'write', contents: 'read' }; + const REVOKE_WARNING = 'the action token could not be revoked; it expires on its own'; /** owner login (lowercased) -> installation */ const installations = new Map(); @@ -578,7 +579,7 @@ function pullRequestNumber(value: unknown): number | undefined { if (typeof token !== 'string' || !token) { throw new ProviderConfigError('GitHub returned no installation token for the action'); } - let result!: Awaited>; + let result: ActionRunResult; let warning: string | undefined; try { result = await runGhAction({ @@ -609,11 +610,11 @@ function pullRequestNumber(value: unknown): number | undefined { // The status is enough; a body that cannot be read is still not a successful revoke. } log(`could not revoke the action token: GitHub answered ${response.status}${detail ? ` ${detail}` : ''}`); - warning = 'the action token could not be revoked; it expires on its own'; + warning = REVOKE_WARNING; } } catch (error) { log(`could not revoke the action token: ${String((error as Error).message ?? error)}`); - warning = 'the action token could not be revoked; it expires on its own'; + warning = REVOKE_WARNING; } } return { ...result, tokenFingerprint: tokenFingerprint(token), ...(warning === undefined ? {} : { warning }) }; diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index db3f76c8..a8239291 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -70,6 +70,44 @@ describe('gcb gh', () => { } }); + it('prints a provider warning to stderr', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-warning-')); + const socketPath = path.join(root, 'broker.sock'); + const block: StaticHostConfig = { provider: 'static', allow: ['acme/widget'] }; + const config: BrokerConfig = { + socketPath, + socketMode: 0o600, + auditPath: null, + tokenCacheSkewSeconds: 300, + hosts: { 'github.com': block }, + }; + const audit: AuditSink = { path: null, record: () => {}, close: () => Promise.resolve() }; + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async () => ({ + stdout: 'viewed\n', + stderr: '', + code: 0, + signal: null, + truncated: false, + timedOut: false, + warning: 'the action token could not be revoked; it expires on its own', + }), + }; + const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); + try { + const result = await capture(() => + runGh(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]), + ); + assert.equal(result.code, 0); + assert.match(result.stderr, /expires on its own/); + } finally { + await broker.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + it('sends a typed action to the broker and prints its stdout', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-command-')); const socketPath = path.join(root, 'broker.sock'); From 0f2e3012773797a0fa5f6bbc0914c52a1b92520e Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 15:04:52 +0000 Subject: [PATCH 03/10] test: cover the revoke rejection and count the client tests Run gcb gh as a child process so the reporter keeps all three tests, and let the fetch stub reject the revoke DELETE to exercise the catch branch. --- test/unit/gh-command.test.ts | 47 +++++++++++------------------------- test/unit/github-app.test.ts | 35 ++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index a8239291..728f8ec7 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -4,30 +4,17 @@ import os from 'node:os'; import path from 'node:path'; import { describe, it } from 'node:test'; -import { runGh } from '../../packages/client/src/commands/gh.ts'; import { startBroker } from '../../packages/host/src/broker.ts'; +import { tryRunAsync, type RunResult } from '../lib/run.ts'; import type { AuditSink, BrokerConfig, Provider, StaticHostConfig } from '../../packages/cli/src/types.ts'; -/** Capture stdout for one command. */ -async function capture(run: () => Promise): Promise<{ code: number; stdout: string; stderr: string }> { - let stdout = ''; - let stderr = ''; - const originalOut = process.stdout.write; - const originalErr = process.stderr.write; - process.stdout.write = ((chunk: unknown) => { - stdout += String(chunk); - return true; - }) as typeof process.stdout.write; - process.stderr.write = ((chunk: unknown) => { - stderr += String(chunk); - return true; - }) as typeof process.stderr.write; - try { - return { code: await run(), stdout, stderr }; - } finally { - process.stdout.write = originalOut; - process.stderr.write = originalErr; - } +/** Run the installed client as a child, so its output never replaces the test reporter. */ +function runGhCli(args: readonly string[]): Promise { + return tryRunAsync( + process.execPath, + ['--conditions=development', 'packages/client/bin/gcb.js', 'gh', ...args], + { env: process.env, timeoutMs: 5000 }, + ); } describe('gcb gh', () => { @@ -57,10 +44,8 @@ describe('gcb gh', () => { }; const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); try { - const result = await capture(() => - runGh(['pr', 'create', '--head', 'feat/x', '--title', 'x', '--body', 'b', '--repo', 'acme/widget', '--socket', socketPath]), - ); - assert.equal(result.code, 1); + const result = await runGhCli(['pr', 'create', '--head', 'feat/x', '--title', 'x', '--body', 'b', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(result.status, 1); assert.ok(result.stdout.endsWith('partial output\n')); assert.match(result.stderr, /output was truncated/); assert.match(result.stderr, /may have completed/); @@ -97,10 +82,8 @@ describe('gcb gh', () => { }; const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); try { - const result = await capture(() => - runGh(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]), - ); - assert.equal(result.code, 0); + const result = await runGhCli(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(result.status, 0); assert.match(result.stderr, /expires on its own/); } finally { await broker.close(); @@ -131,10 +114,8 @@ describe('gcb gh', () => { }; const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); try { - const result = await capture(() => - runGh(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]), - ); - assert.equal(result.code, 0); + const result = await runGhCli(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(result.status, 0); assert.ok(result.stdout.endsWith('the viewed PR\n')); } finally { await broker.close(); diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index 386a1e47..a3a08f0a 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -146,6 +146,7 @@ function createStubFetch(options: { token?: string; tokenStatus?: number; revokeStatus?: number; + revokeReject?: boolean; appPermissions?: Record; installationPermissions?: Record; logText?: string; @@ -157,6 +158,7 @@ function createStubFetch(options: { token = 'ghs_stub_token_value', tokenStatus = 201, revokeStatus = 204, + revokeReject = false, appPermissions = { contents: 'write', pull_requests: 'write' }, installationPermissions = appPermissions, logText = 'stub log\n', @@ -240,7 +242,7 @@ function createStubFetch(options: { return Promise.resolve(jsonResponse({ permissions: installationPermissions })); } if (url.endsWith('/installation/token')) { - return Promise.resolve(jsonResponse({}, revokeStatus)); + return revokeReject ? Promise.reject(new Error('network down')) : Promise.resolve(jsonResponse({}, revokeStatus)); } if (url.includes('/access_tokens')) { if (tokenStatus !== 201) return Promise.resolve(jsonResponse({ message: 'Bad credentials' }, tokenStatus)); @@ -325,6 +327,37 @@ describe('createGithubAppProvider', () => { } }); + it('reports a rejected action-token revoke', async () => { + const dir = mkdtempSync(join(tmpdir(), 'gcb-action-revoke-reject-')); + const gh = join(dir, 'gh'); + writeFileSync(gh, '#!/bin/sh\nexit 0\n'); + chmodSync(gh, 0o755); + const permissions = { pull_requests: 'read', contents: 'read', checks: 'read', statuses: 'read' }; + const stub = createStubFetch({ + installations: [{ id: 42, account: { login: 'Acme' } }], + nowMs, + appPermissions: permissions, + installationPermissions: permissions, + revokeReject: true, + }); + const logs: string[] = []; + const provider = providerFor(stub, {}, gh, (message) => logs.push(message)); + try { + const result = await provider.runAction?.({ + host: 'github.com', + owner: 'acme', + repo: 'widget', + full: 'acme/widget', + action: 'pr:view', + values: { number: 7 }, + }); + assert.match(result?.warning ?? '', /expires on its own/); + assert.ok(logs.some((line) => line.includes('network down'))); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('runs an action with an action token and revokes it', async () => { const dir = mkdtempSync(join(tmpdir(), 'gcb-action-')); const gh = join(dir, 'gh'); From 312d7216d0f7462fbe66009cd6d2c467ce28593d Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 15:13:22 +0000 Subject: [PATCH 04/10] test: resolve the client bin from the test file Do not depend on the cwd used to invoke node --test. --- test/unit/gh-command.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 728f8ec7..8ccbe418 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -2,17 +2,21 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { describe, it } from 'node:test'; import { startBroker } from '../../packages/host/src/broker.ts'; import { tryRunAsync, type RunResult } from '../lib/run.ts'; import type { AuditSink, BrokerConfig, Provider, StaticHostConfig } from '../../packages/cli/src/types.ts'; +/** Run the client from source, next to this file rather than from the cwd. */ +const CLIENT_BIN = fileURLToPath(new URL('../../packages/client/bin/gcb.js', import.meta.url)); + /** Run the installed client as a child, so its output never replaces the test reporter. */ function runGhCli(args: readonly string[]): Promise { return tryRunAsync( process.execPath, - ['--conditions=development', 'packages/client/bin/gcb.js', 'gh', ...args], + ['--conditions=development', CLIENT_BIN, 'gh', ...args], { env: process.env, timeoutMs: 5000 }, ); } From a5024209b875da15c68bc82a32c4f3e8ba46c6e3 Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 15:41:32 +0000 Subject: [PATCH 05/10] fix: keep the revoke warning on a failed start A provider error can carry a warning; return it with the refusal and print it before gcb gh exits. --- packages/client/src/commands/gh.ts | 1 + packages/host/src/broker.ts | 3 ++- test/unit/broker.test.ts | 27 +++++++++++++++++++++ test/unit/gh-command.test.ts | 38 ++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index 5883c78d..f8c20563 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -123,6 +123,7 @@ export async function runGh(argv: readonly string[]): Promise { fail((error as Error).message); } if (response.ok !== true) { + if (response.warning) process.stderr.write(`warning: ${response.warning}\n`); fail(`refused [${response.code ?? 'unknown'}]: ${response.reason ?? 'no reason'}`); } if (response.stdout) process.stdout.write(response.stdout); diff --git a/packages/host/src/broker.ts b/packages/host/src/broker.ts index 89cc30a6..2c2bc240 100644 --- a/packages/host/src/broker.ts +++ b/packages/host/src/broker.ts @@ -296,7 +296,8 @@ export function createRequestHandler( values: actionValues, }); if (result.error !== undefined) { - return denyAction(CODES.PROVIDER_ERROR, result.error, 'the host could not start gh'); + const refusal = denyAction(CODES.PROVIDER_ERROR, result.error, 'the host could not start gh'); + return result.warning === undefined ? refusal : { ...refusal, warning: result.warning }; } audit.record({ event: 'action', diff --git a/test/unit/broker.test.ts b/test/unit/broker.test.ts index ad3a9ddd..ab023738 100644 --- a/test/unit/broker.test.ts +++ b/test/unit/broker.test.ts @@ -577,6 +577,33 @@ describe('broker actions', () => { assert.equal(record?.['token_fingerprint'], 'action-fingerprint'); }); + it('passes a warning with a failed start', async () => { + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async () => ({ + stdout: '', + stderr: '', + code: null, + signal: null, + truncated: false, + timedOut: false, + error: 'spawn failed', + warning: 'the action token could not be revoked; it expires on its own', + }), + }; + const harness = makeHarness(undefined, provider); + const response = await harness.handle({ + op: 'action', + host: HOST, + path: 'acme/widget', + ghAction: 'pr:view', + values: { number: 7 }, + }); + assert.equal(response.ok, false); + assert.match(String(response.warning), /expires on its own/); + }); + it('passes a provider warning to the caller', async () => { const provider: Provider = { name: 'static', diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index 8ccbe418..e9f020b9 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -59,6 +59,44 @@ describe('gcb gh', () => { } }); + it('prints a warning on a refused action', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-refused-warning-')); + const socketPath = path.join(root, 'broker.sock'); + const block: StaticHostConfig = { provider: 'static', allow: ['acme/widget'] }; + const config: BrokerConfig = { + socketPath, + socketMode: 0o600, + auditPath: null, + tokenCacheSkewSeconds: 300, + hosts: { 'github.com': block }, + }; + const audit: AuditSink = { path: null, record: () => {}, close: () => Promise.resolve() }; + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async () => ({ + stdout: '', + stderr: '', + code: null, + signal: null, + truncated: false, + timedOut: false, + error: 'spawn failed', + warning: 'the action token could not be revoked; it expires on its own', + }), + }; + const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); + try { + const result = await runGhCli(['pr', 'view', '7', '--repo', 'acme/widget', '--socket', socketPath]); + assert.equal(result.status, 1); + assert.match(result.stderr, /expires on its own/); + assert.match(result.stderr, /refused/); + } finally { + await broker.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + it('prints a provider warning to stderr', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-warning-')); const socketPath = path.join(root, 'broker.sock'); From 010a5c21a9ab20c83e863ed52ebe7b0abeebeeac Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 16:15:46 +0000 Subject: [PATCH 06/10] docs: describe the warning on both action results The warning is also returned when the action failed to start, not only after success. --- packages/cli/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index c662e4a8..58156e9f 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -38,7 +38,7 @@ export interface ActionRunResult { readonly error?: string; /** SHA-256 prefix of the action token, for the audit record only. */ readonly tokenFingerprint?: string; - /** Set when the action succeeded but its token could not be revoked. */ + /** Set when the token could not be revoked and so expires on its own. */ readonly warning?: string; } From 89be815070ddcb2960eba0e172602f09e4c605c8 Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 16:47:28 +0000 Subject: [PATCH 07/10] fix: keep the revoke warning when the executor throws Turn a runGhAction setup exception into an ActionRunResult before the revoke finally, so the existing refusal path can carry the warning. --- packages/host/src/providers/github-app.ts | 10 ++++++++++ test/unit/github-app.test.ts | 24 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/packages/host/src/providers/github-app.ts b/packages/host/src/providers/github-app.ts index bdb09198..a5056e43 100644 --- a/packages/host/src/providers/github-app.ts +++ b/packages/host/src/providers/github-app.ts @@ -591,6 +591,16 @@ function pullRequestNumber(value: unknown): number | undefined { ghPath, env: process.env, }); + } catch (error) { + result = { + stdout: '', + stderr: '', + code: null, + signal: null, + truncated: false, + timedOut: false, + error: error instanceof Error ? error.message : String(error), + }; } finally { try { const response = await fetchImpl(`${api}/installation/token`, { diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index a3a08f0a..a73d6ed6 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -358,6 +358,30 @@ describe('createGithubAppProvider', () => { } }); + it('returns setup failures and a failed-revoke warning together', async () => { + const permissions = { pull_requests: 'read', contents: 'read', checks: 'read', statuses: 'read' }; + const stub = createStubFetch({ + installations: [{ id: 42, account: { login: 'Acme' } }], + nowMs, + appPermissions: permissions, + installationPermissions: permissions, + revokeStatus: 500, + }); + const logs: string[] = []; + const provider = providerFor(stub, {}, undefined, (message) => logs.push(message)); + const result = await provider.runAction?.({ + host: 'github.com', + owner: 'acme', + repo: 'widget', + full: 'acme/widget', + action: 'pr:view', + values: {}, + }); + assert.ok(result?.error); + assert.match(result?.warning ?? '', /expires on its own/); + assert.ok(logs.some((line) => line.includes('could not revoke the action token'))); + }); + it('runs an action with an action token and revokes it', async () => { const dir = mkdtempSync(join(tmpdir(), 'gcb-action-')); const gh = join(dir, 'gh'); From 96ddcb4a45e758bf9b89d54d0821353e0b1718fb Mon Sep 17 00:00:00 2001 From: JounQin Date: Mon, 26 Oct 2026 16:55:18 +0000 Subject: [PATCH 08/10] refactor: share the no-run action result Keep the setup-failure shape from drifting as the result grows. --- packages/host/src/providers/github-app.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/host/src/providers/github-app.ts b/packages/host/src/providers/github-app.ts index a5056e43..b485cd4b 100644 --- a/packages/host/src/providers/github-app.ts +++ b/packages/host/src/providers/github-app.ts @@ -190,6 +190,8 @@ export function createGithubAppProvider(options: GithubAppProviderOptions): Prov * a token minted for git should not also be able to open a pull request. */ const PULL_REQUEST_PERMISSIONS: Record = { pull_requests: 'write', contents: 'read' }; + /** The result of an action that never reached the executor. */ + const NO_RUN = { stdout: '', stderr: '', code: null, signal: null, truncated: false, timedOut: false }; const REVOKE_WARNING = 'the action token could not be revoked; it expires on its own'; /** owner login (lowercased) -> installation */ @@ -592,15 +594,7 @@ function pullRequestNumber(value: unknown): number | undefined { env: process.env, }); } catch (error) { - result = { - stdout: '', - stderr: '', - code: null, - signal: null, - truncated: false, - timedOut: false, - error: error instanceof Error ? error.message : String(error), - }; + result = { ...NO_RUN, error: error instanceof Error ? error.message : String(error) }; } finally { try { const response = await fetchImpl(`${api}/installation/token`, { From 5cfa1b033dc0e0a303538f2eb3ce011573b536fc Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 04:30:23 +0000 Subject: [PATCH 09/10] test: fail the combined revoke case during setup Use a NUL gh path so spawn throws before any process starts, while the action values remain valid. --- test/unit/github-app.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index a73d6ed6..efd11df5 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -368,14 +368,14 @@ describe('createGithubAppProvider', () => { revokeStatus: 500, }); const logs: string[] = []; - const provider = providerFor(stub, {}, undefined, (message) => logs.push(message)); + const provider = providerFor(stub, {}, '\0', (message) => logs.push(message)); const result = await provider.runAction?.({ host: 'github.com', owner: 'acme', repo: 'widget', full: 'acme/widget', action: 'pr:view', - values: {}, + values: { number: 7 }, }); assert.ok(result?.error); assert.match(result?.warning ?? '', /expires on its own/); From a2bf4612153d4f113905958868cce83ab1fd6d27 Mon Sep 17 00:00:00 2001 From: JounQin Date: Tue, 27 Oct 2026 04:36:40 +0000 Subject: [PATCH 10/10] test: explain the NUL setup failure Spell out why the NUL is the throw path rather than the returned-error path. --- test/unit/github-app.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index efd11df5..511aa8c1 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -368,6 +368,7 @@ describe('createGithubAppProvider', () => { revokeStatus: 500, }); const logs: string[] = []; + // A NUL makes spawn throw synchronously; an ordinary bad path would be returned as result.error. const provider = providerFor(stub, {}, '\0', (message) => logs.push(message)); const result = await provider.runAction?.({ host: 'github.com',