From 8cd2cc12368482d570958d0d01ef8ae4d9492c82 Mon Sep 17 00:00:00 2001 From: inigo Date: Tue, 18 Nov 2025 14:01:58 -0800 Subject: [PATCH] refactor: remove individual field outputs, keep only structured_output JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since GitHub Actions composite actions cannot expose dynamic outputs, individual field outputs were not accessible anyway and only added complexity and collision risk. Simplified by: - Removing individual core.setOutput() calls for each field - Removing RESERVED_OUTPUTS check (no longer needed) - Removing sanitizeOutputName, convertToString, MAX_OUTPUT_SIZE helpers - Removing related unit tests for removed functionality Users access all fields via single structured_output JSON string: fromJSON(steps..outputs.structured_output).field_name Or with jq: echo '${{ steps..outputs.structured_output }}' | jq '.field_name' All tests pass (462 tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- base-action/src/run-claude.ts | 104 --------------- base-action/test/structured-output.test.ts | 144 +-------------------- 2 files changed, 1 insertion(+), 247 deletions(-) diff --git a/base-action/src/run-claude.ts b/base-action/src/run-claude.ts index 13df52a..7c0a8ff 100644 --- a/base-action/src/run-claude.ts +++ b/base-action/src/run-claude.ts @@ -12,9 +12,6 @@ const PIPE_PATH = `${process.env.RUNNER_TEMP}/claude_prompt_pipe`; const EXECUTION_FILE = `${process.env.RUNNER_TEMP}/claude-execution-output.json`; const BASE_ARGS = ["--verbose", "--output-format", "stream-json"]; -// GitHub Actions output limits -const MAX_OUTPUT_SIZE = 1024 * 1024; // 1MB per output field - type ExecutionMessage = { type: string; structured_output?: Record; @@ -130,48 +127,6 @@ export function prepareRunConfig( }; } -/** - * Sanitizes output field names to meet GitHub Actions output naming requirements - * GitHub outputs must be alphanumeric, hyphen, or underscore only - */ -export function sanitizeOutputName(name: string): string { - return name.replace(/[^a-zA-Z0-9_-]/g, "_"); -} - -// Reserved output names that cannot be used by structured outputs -const RESERVED_OUTPUTS = [ - "conclusion", - "execution_file", - "structured_output", -] as const; - -/** - * Converts values to string format for GitHub Actions outputs - * GitHub outputs must always be strings - */ -export function convertToString(value: unknown): string { - switch (typeof value) { - case "string": - return value; - case "boolean": - case "number": - return String(value); - case "object": - if (value === null) return ""; - // Handle circular references - try { - return JSON.stringify(value); - } catch (e) { - return "[Circular or non-serializable object]"; - } - case "undefined": - return ""; - default: - // Handle Symbol, Function, etc. - return String(value); - } -} - /** * Parses structured_output from execution file and sets GitHub Action outputs * Only runs if json_schema was explicitly provided by the user @@ -202,65 +157,6 @@ async function parseAndSetStructuredOutputs( core.info( `Set structured_output with ${Object.keys(result.structured_output).length} field(s)`, ); - - // Also set individual field outputs for direct usage (when not using composite action) - const entries = Object.entries(result.structured_output); - core.info(`Setting ${entries.length} individual structured output(s)`); - - for (const [key, value] of entries) { - // Validate key before sanitization - if (!key || key.trim() === "") { - core.warning("Skipping empty output key"); - continue; - } - - const sanitizedKey = sanitizeOutputName(key); - - // Ensure key starts with letter or underscore (GitHub Actions convention) - if (!/^[a-zA-Z_]/.test(sanitizedKey)) { - core.warning( - `Skipping invalid output key "${key}" (sanitized: "${sanitizedKey}")`, - ); - continue; - } - - // Prevent shadowing reserved action outputs - if (RESERVED_OUTPUTS.includes(sanitizedKey as any)) { - core.warning( - `Skipping reserved output key "${key}" - would shadow action output "${sanitizedKey}"`, - ); - continue; - } - - const stringValue = convertToString(value); - - // Enforce GitHub Actions output size limit (1MB) - if (stringValue.length > MAX_OUTPUT_SIZE) { - // Don't truncate objects/arrays - would create invalid JSON - if (typeof value === "object" && value !== null) { - core.warning( - `Output "${sanitizedKey}" object/array exceeds 1MB (${stringValue.length} bytes). Skipping - reduce data size.`, - ); - continue; - } - // For primitives, truncation is safe - core.warning( - `Output "${sanitizedKey}" exceeds 1MB (${stringValue.length} bytes), truncating`, - ); - const truncated = stringValue.substring(0, MAX_OUTPUT_SIZE); - core.setOutput(sanitizedKey, truncated); - core.info(`✓ ${sanitizedKey}=[TRUNCATED ${stringValue.length} bytes]`); - } else { - // Truncate long values in logs for readability - const displayValue = - stringValue.length > 100 - ? `${stringValue.slice(0, 97)}...` - : stringValue; - - core.setOutput(sanitizedKey, stringValue); - core.info(`✓ ${sanitizedKey}=${displayValue}`); - } - } } catch (error) { if (error instanceof Error) { core.setFailed(error.message); diff --git a/base-action/test/structured-output.test.ts b/base-action/test/structured-output.test.ts index 3cf2240..d00fb41 100644 --- a/base-action/test/structured-output.test.ts +++ b/base-action/test/structured-output.test.ts @@ -4,7 +4,6 @@ import { describe, test, expect, afterEach } from "bun:test"; import { writeFile, unlink } from "fs/promises"; import { tmpdir } from "os"; import { join } from "path"; -import { sanitizeOutputName, convertToString } from "../src/run-claude"; // Import the type for testing type ExecutionMessage = { @@ -37,7 +36,7 @@ async function createMockExecutionFile( await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages)); } -describe("Structured Output - Pure Functions", () => { +describe("Structured Output Parsing", () => { afterEach(async () => { try { await unlink(TEST_EXECUTION_FILE); @@ -46,84 +45,7 @@ describe("Structured Output - Pure Functions", () => { } }); - describe("sanitizeOutputName", () => { - test("should keep valid characters", () => { - expect(sanitizeOutputName("valid_name-123")).toBe("valid_name-123"); - }); - - test("should replace invalid characters with underscores", () => { - expect(sanitizeOutputName("invalid@name!")).toBe("invalid_name_"); - expect(sanitizeOutputName("has spaces")).toBe("has_spaces"); - expect(sanitizeOutputName("has.dots")).toBe("has_dots"); - }); - - test("should handle special characters", () => { - expect(sanitizeOutputName("$field%name&")).toBe("_field_name_"); - expect(sanitizeOutputName("field[0]")).toBe("field_0_"); - }); - }); - - describe("convertToString", () => { - test("should keep strings as-is", () => { - expect(convertToString("hello")).toBe("hello"); - expect(convertToString("")).toBe(""); - }); - - test("should convert booleans to strings", () => { - expect(convertToString(true)).toBe("true"); - expect(convertToString(false)).toBe("false"); - }); - - test("should convert numbers to strings", () => { - expect(convertToString(42)).toBe("42"); - expect(convertToString(3.14)).toBe("3.14"); - expect(convertToString(0)).toBe("0"); - }); - - test("should convert null to empty string", () => { - expect(convertToString(null)).toBe(""); - }); - - test("should JSON stringify objects", () => { - expect(convertToString({ foo: "bar" })).toBe('{"foo":"bar"}'); - }); - - test("should JSON stringify arrays", () => { - expect(convertToString([1, 2, 3])).toBe("[1,2,3]"); - expect(convertToString(["a", "b"])).toBe('["a","b"]'); - }); - - test("should handle nested structures", () => { - const nested = { items: [{ id: 1, name: "test" }] }; - expect(convertToString(nested)).toBe( - '{"items":[{"id":1,"name":"test"}]}', - ); - }); - }); - describe("parseAndSetStructuredOutputs integration", () => { - test("should parse and set simple structured outputs", async () => { - await createMockExecutionFile({ - is_antonly: true, - confidence: 0.95, - risk: "low", - }); - - // In a real test, we'd import and call parseAndSetStructuredOutputs - // For now, we simulate the behavior - const content = await Bun.file(TEST_EXECUTION_FILE).text(); - const messages = JSON.parse(content) as ExecutionMessage[]; - const result = messages.find( - (m) => m.type === "result" && m.structured_output, - ); - - expect(result?.structured_output).toEqual({ - is_antonly: true, - confidence: 0.95, - risk: "low", - }); - }); - test("should handle array outputs", async () => { await createMockExecutionFile({ affected_areas: ["auth", "database", "api"], @@ -214,35 +136,6 @@ describe("Structured Output - Pure Functions", () => { }); }); - describe("output naming with prefix", () => { - test("should apply prefix correctly", () => { - const prefix = "CLAUDE_"; - const key = "is_antonly"; - const sanitizedKey = key.replace(/[^a-zA-Z0-9_-]/g, "_"); - const outputName = prefix + sanitizedKey; - - expect(outputName).toBe("CLAUDE_is_antonly"); - }); - - test("should handle empty prefix", () => { - const prefix = ""; - const key = "result"; - const sanitizedKey = key.replace(/[^a-zA-Z0-9_-]/g, "_"); - const outputName = prefix + sanitizedKey; - - expect(outputName).toBe("result"); - }); - - test("should sanitize and prefix invalid keys", () => { - const prefix = "OUT_"; - const key = "invalid@key!"; - const sanitizedKey = key.replace(/[^a-zA-Z0-9_-]/g, "_"); - const outputName = prefix + sanitizedKey; - - expect(outputName).toBe("OUT_invalid_key_"); - }); - }); - describe("error scenarios", () => { test("should handle malformed JSON", async () => { await writeFile(TEST_EXECUTION_FILE, "invalid json {"); @@ -287,39 +180,4 @@ describe("Structured Output - Pure Functions", () => { expect(result).toBeUndefined(); }); }); - - describe("value truncation in logs", () => { - test("should truncate long string values for display", () => { - const longValue = "a".repeat(150); - const displayValue = - longValue.length > 100 ? `${longValue.slice(0, 97)}...` : longValue; - - expect(displayValue).toBe("a".repeat(97) + "..."); - expect(displayValue.length).toBe(100); - }); - - test("should not truncate short values", () => { - const shortValue = "short"; - const displayValue = - shortValue.length > 100 ? `${shortValue.slice(0, 97)}...` : shortValue; - - expect(displayValue).toBe("short"); - }); - - test("should truncate exactly 100 character values", () => { - const value = "a".repeat(100); - const displayValue = - value.length > 100 ? `${value.slice(0, 97)}...` : value; - - expect(displayValue).toBe(value); - }); - - test("should truncate 101 character values", () => { - const value = "a".repeat(101); - const displayValue = - value.length > 100 ? `${value.slice(0, 97)}...` : value; - - expect(displayValue).toBe("a".repeat(97) + "..."); - }); - }); });