chore: merge dev into main

CLAUDE.md

@@ -167,6 +167,8 @@ GitHub's changed-file API is used for file enumeration and change counts, but co
 
 When review output misses something, check the `PR context prepared` log entry for `included` / `skipped` / `skipReasons`, `patchSources`, `totalDiffTokens`, `perFileTokenCap`, and `localGitMismatches` to confirm whether the file was visible to the agent and whether GitHub's API patch differed from the local patch. Also check context offload logs if the diff context was written under `.cascade/context/`.
 
+CI check status is **informational, not fatal** (MNG-1750): the `fetchPRContextStep` boot step wraps only `getCheckSuiteStatus` in a try/catch. If the reviewer PAT lacks the **Actions: Read** permission the Actions API throws 403, and the `GetPRChecks` context injection degrades to an explicit "CI check status UNAVAILABLE" message (with the permission hint, deliberately distinct from "No CI checks configured") plus a `WARN CI check status unavailable` log — instead of killing the agent boot with a `BootFailureError`. PR details (`getPR`) and the diff (`getPRDiff`) stay fatal — a review without the PR itself is meaningless.
+
 **cascade-tools shell-safety contract** — MNG-1059. cascade-tools commands that accept markdown/multiline payloads (`--body`, `--text`, `--description`, `--details`, `--comments`) declare a `--*-file <path>` companion via `cli.fileInputAlternatives`. Agents are instructed in the system prompt to prefer the file form when content contains backticks, code fences, `$(...)`, or newlines — shells expand those tokens even inside single quotes once they layer through `bash -c`, and newlines break argv parsing. The shared CLI factory at `src/gadgets/shared/cli/params.ts:rejectMultipleStdinConsumers` enforces the single-stdin-consumer invariant: only one `--*-file -` per command. Passing two stdin consumers (e.g. `--body-file - --comments-file -`) returns a structured `flag-parse` envelope with `error.flag: "body-file,comments-file"` and a hint to write one payload to a temp file — *before* any `readFileSync(0, ...)` call. The native-tool system prompt also renders a "cascade-tools shell-safety rules" section with safe heredoc / temp-file patterns. Prompt example rendering suppresses inline `--body '...'` examples for shell-sensitive content (backticks / `$(...)` / newlines) when a file-input companion exists, redirecting agents at the safer `--*-file <path>` form.
 
 ## Engines

src/agents/definitions/contextSteps.ts

@@ -5,7 +5,10 @@
  * These are the building blocks composed by the YAML contextPipeline arrays.
  */
 
-import { formatCheckStatus } from '../../gadgets/github/core/getPRChecks.js';
+import {
+	formatCheckStatus,
+	formatCheckStatusUnavailable,
+} from '../../gadgets/github/core/getPRChecks.js';
 import { ListDirectory } from '../../gadgets/ListDirectory.js';
 import {
 	readStructuredWorkItemDetails,
@@ -164,10 +167,31 @@ export async function fetchPRContextStep(params: FetchContextParams): Promise<Co
 
 	const prDetails = await githubClient.getPR(owner, repo, prNumber);
 	const prDiff = await githubClient.getPRDiff(owner, repo, prNumber);
-	const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, prDetails.headSha);
+
+	// CI check status is informational, not fatal (MNG-1750). A reviewer PAT
+	// without the "Actions: Read" permission throws 403 here; degrade gracefully
+	// so the review still boots instead of dying with a BootFailureError. The
+	// PR details + diff above stay fatal — a review without the PR is meaningless.
+	let checkStatusFormatted: string;
+	let checkStatusDescription = 'Pre-fetched CI check status';
+	try {
+		const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, prDetails.headSha);
+		checkStatusFormatted = formatCheckStatus(prNumber, checkStatus);
+	} catch (error) {
+		// Log/inject only `error.message`, never the raw Octokit RequestError
+		// object — it can carry the Authorization header.
+		const message = error instanceof Error ? error.message : String(error);
+		params.logWriter('WARN', 'CI check status unavailable', {
+			owner,
+			repo,
+			prNumber,
+			error: message,
+		});
+		checkStatusFormatted = formatCheckStatusUnavailable(prNumber, message);
+		checkStatusDescription = 'CI check status unavailable';
+	}
 
 	const prDetailsFormatted = formatPRDetails(prDetails);
-	const checkStatusFormatted = formatCheckStatus(prNumber, checkStatus);
 
 	injections.push({
 		toolName: 'GetPRDetails',
@@ -180,7 +204,7 @@ export async function fetchPRContextStep(params: FetchContextParams): Promise<Co
 		toolName: 'GetPRChecks',
 		params: { comment: 'Pre-fetching CI check status for review', owner, repo, prNumber },
 		result: checkStatusFormatted,
-		description: 'Pre-fetched CI check status',
+		description: checkStatusDescription,
 	});
 
 	// Total changed files (now complete — `getPRDiff` paginates beyond the first 100).

src/gadgets/github/core/getPRChecks.ts

@@ -26,6 +26,34 @@ export function formatCheckStatus(prNumber: number, checkStatus: CheckSuiteStatu
 	return lines.join('\n');
 }
 
+/**
+ * Format an operator/agent-facing message for the case where CI check status
+ * could NOT be fetched (MNG-1750). This is deliberately distinct from
+ * {@link formatCheckStatus}'s `No CI checks configured` string: the agent must
+ * be able to tell "CASCADE tried to read CI status and failed" apart from
+ * "there is nothing to check".
+ *
+ * Takes a plain `errorMessage` string (never a raw Octokit `RequestError`
+ * object) — callers pass only `error.message` to avoid leaking the
+ * `Authorization` header carried on the request object.
+ */
+export function formatCheckStatusUnavailable(prNumber: number, errorMessage: string): string {
+	return [
+		`PR #${prNumber}: CI check status UNAVAILABLE (could not be fetched)`,
+		'',
+		`Upstream error: ${errorMessage}`,
+		'',
+		'Probable cause: the reviewer credential is a fine-grained PAT lacking the',
+		'"Actions: Read" repository permission (GitHub returns 403 "Resource not',
+		'accessible by personal access token" for the Actions API).',
+		'',
+		'IMPORTANT: This is NOT the same as "No CI checks configured" — CASCADE',
+		'attempted to read CI status and failed. Do NOT assume checks are green or',
+		'red. You may retry with the GetPRChecks gadget later, or explicitly note in',
+		'your review that CI status could not be verified.',
+	].join('\n');
+}
+
 function getStatusIcon(status: string, conclusion: string | null): string {
 	if (status !== 'completed') {
 		return status === 'in_progress' ? '⏳' : '⏸';

tests/unit/agents/definitions/contextSteps.test.ts

@@ -897,4 +897,80 @@ describe('fetchPRContextStep — compact diffs + SKIPPED FILES contract', () =>
 			expect.objectContaining({ baseBranch: 'parent-feature' }),
 		);
 	});
+
+	// MNG-1750: a reviewer PAT lacking the "Actions: Read" permission makes
+	// getCheckSuiteStatus throw 403. That must degrade to an informational
+	// injection instead of killing the whole agent boot (BootFailureError).
+	describe('MNG-1750 — graceful CI check-status degradation', () => {
+		beforeEach(() => {
+			mockGetPRDiff.mockResolvedValue([
+				{
+					filename: 'src/a.ts',
+					status: 'modified',
+					additions: 1,
+					deletions: 0,
+					changes: 1,
+					patch: '@@ -1 +1 @@\n+x',
+				},
+			]);
+		});
+
+		it('proceeds when getCheckSuiteStatus throws 403 and injects an UNAVAILABLE signal', async () => {
+			const err = Object.assign(new Error('Resource not accessible by personal access token'), {
+				status: 403,
+			});
+			mockGetCheckSuiteStatus.mockRejectedValue(err);
+
+			const injections = await fetchPRContextStep(makePRParams()); // must NOT throw
+
+			const checks = injections.find((i) => i.toolName === 'GetPRChecks');
+			expect(checks).toBeDefined();
+			expect(checks?.result as string).toContain('UNAVAILABLE');
+			expect(checks?.result as string).toContain(
+				'Resource not accessible by personal access token',
+			);
+			expect(checks?.result as string).toContain('Actions: Read');
+			expect(checks?.description).toBe('CI check status unavailable');
+		});
+
+		it('logs a WARN carrying the upstream error message', async () => {
+			mockGetCheckSuiteStatus.mockRejectedValue(
+				new Error('Resource not accessible by personal access token'),
+			);
+
+			const params = makePRParams();
+			await fetchPRContextStep(params);
+
+			expect(params.logWriter).toHaveBeenCalledWith(
+				'WARN',
+				'CI check status unavailable',
+				expect.objectContaining({ error: 'Resource not accessible by personal access token' }),
+			);
+		});
+
+		it('leaves the GetPRChecks injection unchanged on the success path', async () => {
+			mockGetCheckSuiteStatus.mockResolvedValue({
+				totalCount: 1,
+				checkRuns: [{ name: 'build', status: 'completed', conclusion: 'success' }],
+				allPassing: true,
+			});
+
+			const injections = await fetchPRContextStep(makePRParams());
+
+			const checks = injections.find((i) => i.toolName === 'GetPRChecks');
+			expect(checks?.result as string).toContain('PR #1092 Check Status: 1/1');
+			expect(checks?.description).toBe('Pre-fetched CI check status');
+		});
+
+		it('still throws when getPR fails (PR details stay fatal)', async () => {
+			mockGetPR.mockRejectedValueOnce(new Error('PR not found'));
+			await expect(fetchPRContextStep(makePRParams())).rejects.toThrow('PR not found');
+		});
+
+		it('still throws when getPRDiff fails (diff stays fatal)', async () => {
+			mockGetPRDiff.mockReset();
+			mockGetPRDiff.mockRejectedValueOnce(new Error('diff unavailable'));
+			await expect(fetchPRContextStep(makePRParams())).rejects.toThrow('diff unavailable');
+		});
+	});
 });

tests/unit/gadgets/github/core/getPRChecks.test.ts

@@ -9,6 +9,7 @@ vi.mock('../../../../../src/github/client.js', () => ({
 
 import {
 	formatCheckStatus,
+	formatCheckStatusUnavailable,
 	getPRChecks,
 } from '../../../../../src/gadgets/github/core/getPRChecks.js';
 import { githubClient } from '../../../../../src/github/client.js';
@@ -120,6 +121,45 @@ describe('formatCheckStatus', () => {
 	});
 });
 
+describe('formatCheckStatusUnavailable', () => {
+	it('includes the upstream error message', () => {
+		const result = formatCheckStatusUnavailable(
+			7,
+			'Resource not accessible by personal access token',
+		);
+		expect(result).toContain('Resource not accessible by personal access token');
+	});
+
+	it('includes the "Actions: Read" permission hint', () => {
+		const result = formatCheckStatusUnavailable(7, 'boom');
+		expect(result).toContain('Actions: Read');
+	});
+
+	it('references the PR number', () => {
+		const result = formatCheckStatusUnavailable(1234, 'boom');
+		expect(result).toContain('PR #1234');
+	});
+
+	it('is visibly distinct from the "No CI checks configured" message', () => {
+		const unavailable = formatCheckStatusUnavailable(7, 'boom');
+		const noChecks = formatCheckStatus(7, { totalCount: 0, allPassing: true, checkRuns: [] });
+		// The "unavailable" text carries the UNAVAILABLE marker; the "no checks"
+		// text does not. (The unavailable text intentionally *mentions* the
+		// "No CI checks configured" phrase to contrast against it, so the two are
+		// distinguished by the UNAVAILABLE marker and by not being equal.)
+		expect(unavailable).toContain('UNAVAILABLE');
+		expect(noChecks).not.toContain('UNAVAILABLE');
+		expect(unavailable).not.toBe(noChecks);
+	});
+
+	it('does not stringify error objects (takes a plain string only)', () => {
+		// The caller passes error.message, never the whole RequestError. This
+		// helper's signature enforces that: a string in, no [object Object].
+		const result = formatCheckStatusUnavailable(1, 'plain message');
+		expect(result).not.toContain('[object Object]');
+	});
+});
+
 describe('getPRChecks', () => {
 	beforeEach(() => {
 		vi.resetAllMocks();