diff --git a/base-action/src/setup-claude-code-settings.ts b/base-action/src/setup-claude-code-settings.ts index 0fe68414f..8bde38a0b 100644 --- a/base-action/src/setup-claude-code-settings.ts +++ b/base-action/src/setup-claude-code-settings.ts @@ -1,6 +1,7 @@ import { $ } from "bun"; import { homedir } from "os"; import { readFile } from "fs/promises"; +import { parseJsonWithLocation } from "./validate-json"; export async function setupClaudeCodeSettings( settingsInput?: string, @@ -10,59 +11,94 @@ export async function setupClaudeCodeSettings( const settingsPath = `${home}/.claude/settings.json`; console.log(`Setting up Claude settings at: ${settingsPath}`); - // Ensure .claude directory exists console.log(`Creating .claude directory...`); await $`mkdir -p ${home}/.claude`.quiet(); - let settings: Record = {}; - try { - const existingSettings = await $`cat ${settingsPath}`.quiet().text(); - if (existingSettings.trim()) { - settings = JSON.parse(existingSettings); - console.log( - `Found existing settings:`, - JSON.stringify(settings, null, 2), - ); - } else { - console.log(`Settings file exists but is empty`); - } - } catch (e) { - console.log(`No existing settings file found, creating new one`); - } + let settings = await loadExistingSettings(settingsPath); - // Handle settings input (either file path or JSON string) if (settingsInput && settingsInput.trim()) { - console.log(`Processing settings input...`); - let inputSettings: Record = {}; - - try { - // First try to parse as JSON - inputSettings = JSON.parse(settingsInput); - console.log(`Parsed settings input as JSON`); - } catch (e) { - // If not JSON, treat as file path - console.log( - `Settings input is not JSON, treating as file path: ${settingsInput}`, - ); - try { - const fileContent = await readFile(settingsInput, "utf-8"); - inputSettings = JSON.parse(fileContent); - console.log(`Successfully read and parsed settings from file`); - } catch (fileError) { - console.error(`Failed to read or parse settings file: ${fileError}`); - throw new Error(`Failed to process settings input: ${fileError}`); - } - } - - // Merge input settings with existing settings + const inputSettings = await loadInputSettings(settingsInput); settings = { ...settings, ...inputSettings }; console.log(`Merged settings with input settings`); } - // Always set enableAllProjectMcpServers to true settings.enableAllProjectMcpServers = true; console.log(`Updated settings with enableAllProjectMcpServers: true`); await $`echo ${JSON.stringify(settings, null, 2)} > ${settingsPath}`.quiet(); console.log(`Settings saved successfully`); } + +async function loadExistingSettings( + settingsPath: string, +): Promise> { + let existingContent: string; + try { + existingContent = await readFile(settingsPath, "utf-8"); + } catch (error) { + if (isFileNotFoundError(error)) { + console.log(`No existing settings file found, creating new one`); + return {}; + } + throw error; + } + + if (!existingContent.trim()) { + console.log(`Settings file exists but is empty`); + return {}; + } + + const settings = parseJsonWithLocation>( + existingContent, + settingsPath, + ); + console.log(`Found existing settings:`, JSON.stringify(settings, null, 2)); + return settings; +} + +async function loadInputSettings( + settingsInput: string, +): Promise> { + console.log(`Processing settings input...`); + + if (looksLikeJsonObject(settingsInput)) { + const parsed = parseJsonWithLocation>( + settingsInput, + "settings input", + ); + console.log(`Parsed settings input as JSON`); + return parsed; + } + + console.log( + `Settings input is not JSON, treating as file path: ${settingsInput}`, + ); + let fileContent: string; + try { + fileContent = await readFile(settingsInput, "utf-8"); + } catch (fileError) { + throw new Error( + `Failed to read settings file at "${settingsInput}": ${fileError}`, + ); + } + + const parsed = parseJsonWithLocation>( + fileContent, + settingsInput, + ); + console.log(`Successfully read and parsed settings from file`); + return parsed; +} + +function looksLikeJsonObject(value: string): boolean { + return value.trim().startsWith("{"); +} + +function isFileNotFoundError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code: unknown }).code === "ENOENT" + ); +} diff --git a/base-action/src/validate-json.ts b/base-action/src/validate-json.ts new file mode 100755 index 000000000..8fa5bbebc --- /dev/null +++ b/base-action/src/validate-json.ts @@ -0,0 +1,67 @@ +export class JsonParseError extends Error { + constructor( + public readonly source: string, + public readonly underlyingMessage: string, + public readonly line: number | null, + public readonly column: number | null, + ) { + super(buildErrorMessage(source, underlyingMessage, line, column)); + this.name = "JsonParseError"; + } +} + +export function parseJsonWithLocation( + text: string, + source: string, +): T { + try { + return JSON.parse(text) as T; + } catch (error) { + const underlyingMessage = + error instanceof Error ? error.message : String(error); + const position = extractPosition(underlyingMessage); + const location = position === null ? null : toLineColumn(text, position); + throw new JsonParseError( + source, + underlyingMessage, + location?.line ?? null, + location?.column ?? null, + ); + } +} + +function buildErrorMessage( + source: string, + underlyingMessage: string, + line: number | null, + column: number | null, +): string { + const prefix = `Invalid JSON in ${source}: ${underlyingMessage}`; + if (line === null || column === null) { + return prefix; + } + return `${prefix} (line ${line}, column ${column})`; +} + +function extractPosition(message: string): number | null { + const match = message.match(/position\s+(\d+)/i); + if (match === null) { + return null; + } + const parsed = Number(match[1]); + return Number.isFinite(parsed) ? parsed : null; +} + +function toLineColumn( + text: string, + position: number, +): { line: number; column: number } { + const safePosition = Math.min(Math.max(position, 0), text.length); + const before = text.slice(0, safePosition); + const lineBreaks = before.split("\n"); + const currentLine = lineBreaks[lineBreaks.length - 1] ?? ""; + return { + line: lineBreaks.length, + column: currentLine.length + 1, + }; +} diff --git a/base-action/test/validate-json.test.ts b/base-action/test/validate-json.test.ts new file mode 100755 index 000000000..0c0b688ab --- /dev/null +++ b/base-action/test/validate-json.test.ts @@ -0,0 +1,71 @@ +#!/usr/bin/env bun + +import { describe, test, expect } from "bun:test"; +import { JsonParseError, parseJsonWithLocation } from "../src/validate-json"; + +describe("parseJsonWithLocation", () => { + test("returns parsed value for valid JSON", () => { + const result = parseJsonWithLocation<{ name: string }>( + '{"name":"claude"}', + "test source", + ); + expect(result).toEqual({ name: "claude" }); + }); + + test("preserves the parsed type via generic", () => { + const result = parseJsonWithLocation("[1,2,3]", "test source"); + expect(result).toEqual([1, 2, 3]); + }); + + test("throws JsonParseError for malformed JSON", () => { + expect(() => parseJsonWithLocation("{invalid", "test source")).toThrow( + JsonParseError, + ); + }); + + test("error message includes the source identifier", () => { + try { + parseJsonWithLocation("{invalid", "/tmp/settings.json"); + throw new Error("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(JsonParseError); + expect((error as JsonParseError).message).toContain("/tmp/settings.json"); + } + }); + + test("error message includes line and column when parser reports position", () => { + const malformed = '{\n "name": "claude",\n "broken":\n}'; + try { + parseJsonWithLocation(malformed, "test source"); + throw new Error("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(JsonParseError); + const parseError = error as JsonParseError; + if (parseError.line !== null) { + expect(parseError.line).toBeGreaterThan(0); + expect(parseError.column).toBeGreaterThan(0); + expect(parseError.message).toMatch(/line \d+, column \d+/); + } + } + }); + + test("does not include source content in error message", () => { + const secretLaden = '{"ANTHROPIC_API_KEY":"sk-ant-secret123",}'; + try { + parseJsonWithLocation(secretLaden, "settings"); + throw new Error("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(JsonParseError); + expect((error as JsonParseError).message).not.toContain( + "sk-ant-secret123", + ); + } + }); + + test("falls back gracefully when underlying error has no position", () => { + const error = new JsonParseError("source", "weird message", null, null); + expect(error.message).toBe("Invalid JSON in source: weird message"); + expect(error.line).toBeNull(); + expect(error.column).toBeNull(); + }); +}); diff --git a/src/entrypoints/collect-inputs.ts b/src/entrypoints/collect-inputs.ts index 079565c7b..a2d75e779 100644 --- a/src/entrypoints/collect-inputs.ts +++ b/src/entrypoints/collect-inputs.ts @@ -1,3 +1,9 @@ +import * as core from "@actions/core"; +import { + JsonParseError, + parseJsonWithLocation, +} from "../../base-action/src/validate-json"; + export function collectActionInputsPresence(): string { const inputDefaults: Record = { trigger_phrase: "@claude", @@ -36,9 +42,13 @@ export function collectActionInputsPresence(): string { let allInputs: Record; try { - allInputs = JSON.parse(allInputsJson); + allInputs = parseJsonWithLocation>( + allInputsJson, + "ALL_INPUTS environment variable", + ); } catch (e) { - console.error("Failed to parse ALL_INPUTS JSON:", e); + const message = e instanceof JsonParseError ? e.message : String(e); + core.warning(`Failed to parse ALL_INPUTS JSON: ${message}`); return JSON.stringify({}); }