refactor: remove individual field outputs, keep only structured_output JSON

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.<id>.outputs.structured_output).field_name

Or with jq:
  echo '${{ steps.<id>.outputs.structured_output }}' | jq '.field_name'

All tests pass (462 tests).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
inigo
2025-11-18 14:01:58 -08:00
parent dcee434ef2
commit 8cd2cc1236
2 changed files with 1 additions and 247 deletions

View File

@@ -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 EXECUTION_FILE = `${process.env.RUNNER_TEMP}/claude-execution-output.json`;
const BASE_ARGS = ["--verbose", "--output-format", "stream-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 ExecutionMessage = {
type: string; type: string;
structured_output?: Record<string, unknown>; structured_output?: Record<string, unknown>;
@@ -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 * Parses structured_output from execution file and sets GitHub Action outputs
* Only runs if json_schema was explicitly provided by the user * Only runs if json_schema was explicitly provided by the user
@@ -202,65 +157,6 @@ async function parseAndSetStructuredOutputs(
core.info( core.info(
`Set structured_output with ${Object.keys(result.structured_output).length} field(s)`, `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) { } catch (error) {
if (error instanceof Error) { if (error instanceof Error) {
core.setFailed(error.message); core.setFailed(error.message);

View File

@@ -4,7 +4,6 @@ import { describe, test, expect, afterEach } from "bun:test";
import { writeFile, unlink } from "fs/promises"; import { writeFile, unlink } from "fs/promises";
import { tmpdir } from "os"; import { tmpdir } from "os";
import { join } from "path"; import { join } from "path";
import { sanitizeOutputName, convertToString } from "../src/run-claude";
// Import the type for testing // Import the type for testing
type ExecutionMessage = { type ExecutionMessage = {
@@ -37,7 +36,7 @@ async function createMockExecutionFile(
await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages)); await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
} }
describe("Structured Output - Pure Functions", () => { describe("Structured Output Parsing", () => {
afterEach(async () => { afterEach(async () => {
try { try {
await unlink(TEST_EXECUTION_FILE); 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", () => { 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 () => { test("should handle array outputs", async () => {
await createMockExecutionFile({ await createMockExecutionFile({
affected_areas: ["auth", "database", "api"], 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", () => { describe("error scenarios", () => {
test("should handle malformed JSON", async () => { test("should handle malformed JSON", async () => {
await writeFile(TEST_EXECUTION_FILE, "invalid json {"); await writeFile(TEST_EXECUTION_FILE, "invalid json {");
@@ -287,39 +180,4 @@ describe("Structured Output - Pure Functions", () => {
expect(result).toBeUndefined(); 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) + "...");
});
});
}); });