Surface line and column for malformed settings JSON

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<string, unknown> = {};
-  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<string, unknown> = {};
-
-    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<Record<string, unknown>> {
+  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<Record<string, unknown>>(
+    existingContent,
+    settingsPath,
+  );
+  console.log(`Found existing settings:`, JSON.stringify(settings, null, 2));
+  return settings;
+}
+
+async function loadInputSettings(
+  settingsInput: string,
+): Promise<Record<string, unknown>> {
+  console.log(`Processing settings input...`);
+
+  if (looksLikeJsonObject(settingsInput)) {
+    const parsed = parseJsonWithLocation<Record<string, unknown>>(
+      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<Record<string, unknown>>(
+    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"
+  );
+}

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<T = unknown>(
+  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,
+  };
+}

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<number[]>("[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();
+  });
+});

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<string, string> = {
     trigger_phrase: "@claude",
@@ -36,9 +42,13 @@ export function collectActionInputsPresence(): string {
 
   let allInputs: Record<string, string>;
   try {
-    allInputs = JSON.parse(allInputsJson);
+    allInputs = parseJsonWithLocation<Record<string, string>>(
+      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({});
   }