feat: add structured output support via json_schema

[bogini]

Add support for Agent SDK structured outputs.

New Feature

• Input: json_schema - JSON schema for validated outputs
• Behavior: Auto-sets GitHub Action outputs for each field

Documentation

https://docs.claude.com/en/docs/agent-sdk/structured-outputs

Examples

• examples/test-failure-analysis.yml - Auto-retry flaky tests
• .github/workflows/test-structured-output.yml - Integration tests (disabled until feature ships)

Tests

• 26 new unit tests
• All 480 tests passing
• Integration tests ready but disabled (awaiting CLI release)

[claude]

Code Review Summary - PR #683

I've completed a comprehensive review using specialized agents across security, code quality, performance, testing, and documentation. Here are the key findings:

🔴 Critical Issues

1. Command Injection Vulnerability (Security)

• Location: src/modes/agent/index.ts:162 and src/modes/tag/index.ts:190
• Risk: Shell escaping of json_schema is insufficient. Attackers can inject CLI arguments to bypass security controls.
• Action Required: Pass schema via temporary file instead of command-line argument.

2. Unsafe GitHub Actions Output Injection (Security)

• Location: base-action/src/run-claude.ts:202
• Risk: User-controlled values set as outputs without sanitization enable script injection in downstream workflow steps.
• Action Required: Add size limits, multi-line sanitization, and document safe consumption patterns.

🟡 Important Issues

3. Insufficient JSON Schema Validation (Security)

• Only validates JSON syntax, not JSON Schema structure
• Could allow malformed schemas causing DoS or unexpected behavior
• Recommend using proper JSON Schema validator (e.g., Ajv)

4. Ineffective Empty Key Validation (Code Quality)

• Empty check at line 189 is unreachable due to sanitization logic
• Missing validation for keys starting with non-letter characters
• Could result in meaningless output keys like "___"

5. Inconsistent Error Handling (Code Quality)

• Mixed patterns between throwing original errors and wrapping them
• Loses stack traces in some paths, making debugging harder

6. Test/Production Code Mismatch (Testing)

• Test mock differs from production for edge-case types (Symbol, Function)
• Could mask bugs in production code

✅ Strengths

• Clean, well-organized implementation
• Good separation of concerns
• Comprehensive test suite with 26 new tests covering happy paths
• Clear function documentation
• Efficient single-pass parsing with no performance bottlenecks
• Excellent example in examples/test-failure-analysis.yml

📚 Documentation Gaps

The json_schema feature is not documented in:

• README.md
• docs/usage.md (missing from inputs table)

Consider adding a structured outputs section to the usage guide for discoverability.

🧪 Test Coverage Gaps

Missing tests for:

• File system errors (execution file missing/corrupted)
• Undefined value handling (implementation discrepancy)
• Invalid JSON schema validation
• Very large output values (DoS scenario)

---

Overall Assessment: The feature implementation is solid with good architecture, but has critical security vulnerabilities that must be addressed before merging. The command injection and output injection issues pose real security risks in GitHub Actions environments.

Recommendation: Address the two critical security issues, then consider the code quality and testing improvements before release.

[claude]

Comprehensive Code Review Summary

I've completed a thorough review using specialized review agents covering code quality, performance, test coverage, documentation, and security. Overall, this is a well-implemented feature with strong security practices, but there are several noteworthy improvements needed before merging.

---

🔴 Critical Issues (Must Fix)

1. Code Duplication - JSON Schema Validation

Impact: Maintenance burden and risk of inconsistencies

The JSON schema validation logic is duplicated between src/modes/agent/index.ts and src/modes/tag/index.ts. Extract to a shared utility function.

2. Tests Don't Test Production Code

Impact: False confidence in test coverage

The unit tests in base-action/test/structured-output.test.ts use inline function implementations instead of importing the actual production code. The real implementation remains untested.

3. Documentation Inaccuracy

Impact: User confusion

The json_schema input description in both action.yml files doesn't mention field name sanitization, leading to confusion when field names like test.result become test_result.

---

🟡 High Priority Issues (Should Fix)

4. Performance - String Concatenation in Loop

File: base-action/src/run-claude.ts:328
Impact: O(n²) complexity for large outputs (up to 10MB)

Use array accumulator pattern instead of string concatenation to avoid excessive memory allocations.

5. Security - Missing Size Limit on JSON Schema Input

Impact: DoS through resource exhaustion

Add validation to limit JSON schema input size (recommend 100KB max) to prevent unbounded memory/disk usage.

6. Resource Leak - Temp File Cleanup

Impact: Disk space issues in long-running workflows

Temporary schema files are created but never cleaned up. Add cleanup logic in finally block.

7. Test Coverage Gaps

Missing critical tests:

• JSON schema validation error paths
• Missing structured_output error handling
• Output key validation edge cases
• Output size limit boundary conditions
• File I/O error scenarios

---

🟢 Medium Priority Issues (Consider)

8. Potential Race Condition

Date.now() for temp file names could cause collisions. Use randomUUID() for guaranteed uniqueness.

9. Output Name Collision Detection

Fields like user@email and user_email both sanitize to user_email, causing silent overwrites. Add collision detection.

10. JSON Truncation Safety

Truncating objects/arrays at 1MB can produce invalid JSON. Skip these outputs rather than setting invalid data.

11. Integration Tests Disabled

The comprehensive test suite in .github/workflows/test-structured-output.yml is disabled. Enable when feature ships, and add negative test scenarios.

---

✅ Positive Observations

The implementation demonstrates excellent security practices:

• ✅ Command injection prevention via temp files
• ✅ JSON Schema validation with AJV
• ✅ Output size limits (1MB per field)
• ✅ Safe shell argument parsing with shell-quote
• ✅ Proper temp file usage with RUNNER_TEMP
• ✅ Output name validation

The test workflows are well-designed with explicit prompts for deterministic results, and the feature has clear separation of concerns.

---

📋 Recommendations

Before Merge (Critical):

1. Extract duplicated JSON schema validation to shared utility
2. Fix unit tests to import and test actual production code
3. Update action.yml documentation to mention sanitization
4. Fix string concatenation performance issue

Follow-up PR (High Priority):
5. Add JSON schema size validation
6. Implement temp file cleanup
7. Add missing test coverage for error paths
8. Add race condition prevention in temp file naming

Future Improvements:
9. Add collision detection for output names
10. Improve JSON truncation handling
11. Enable and expand integration tests

---

📊 Review Metrics

• Files Changed: 11
• Additions: +1008 lines
• Test Count: 26 unit tests (claimed), 5 integration test jobs (disabled)
• Security Posture: Strong ✅
• Documentation: Needs improvement

---

See inline comments for specific code suggestions and detailed explanations.

[claude]

Code Review Summary: PR #683 - Structured Output Support

I've completed a comprehensive review using specialized agents for security, code quality, performance, testing, and documentation. Overall, this is a well-implemented feature with good test coverage and documentation, but there are critical security issues that must be addressed before merging.

---

🚨 Critical Issues (Must Fix)

1. Command Injection Vulnerability (base-action/src/index.ts:42)

The JSON schema input is directly interpolated into shell arguments without proper escaping, allowing command injection. The fix is simple: use the existing appendJsonSchemaArg utility from src/utils/json-schema.ts which already handles this correctly.

Severity: Critical - Allows arbitrary command execution in GitHub Actions runners

---

🔴 High Priority Issues (Should Fix)

2. Prototype Pollution (base-action/src/run-claude.ts:210)

Missing validation for dangerous property names like __proto__, constructor, and prototype when iterating over structured output fields.

3. Error Handling Anti-pattern (base-action/src/run-claude.ts:262)

The function both calls core.setFailed() AND throws errors, creating confusion about error handling responsibility.

4. Documentation Inaccuracies (docs/usage.md:238, 241)

• Missing structured_output from reserved names list
• Incorrectly states hyphens are replaced when they're actually preserved

5. Critical Test Gap (base-action/test/structured-output.test.ts:112)

The main parseAndSetStructuredOutputs function is never directly tested, leaving ~50 lines of validation logic untested (reserved names, size limits, invalid keys).

---

🟡 Medium Priority Issues (Nice to Fix)

6. Performance: Unnecessary Stringification (base-action/src/run-claude.ts:238)

Large objects are stringified before checking size limits, wasting CPU on objects that will be discarded.

7. Type Safety: as any Casting (base-action/src/run-claude.ts:228)

Using as any bypasses TypeScript's type safety when checking reserved output names.

8. Missing Test Coverage (base-action/src/run-claude.ts:152)

Circular reference handling, undefined, NaN, Infinity, and special types (Symbol, Function, BigInt) lack test coverage.

---

✅ Strengths

• Excellent integration tests with comprehensive scenarios
• Good security practices (size limits, name sanitization, reserved name blocking)
• Clear documentation with practical examples
• Comprehensive handling of type conversions
• Well-structured code with good separation of concerns

---

Recommendation

Do not merge until the command injection vulnerability (Issue #1) is fixed. Issues #2-5 should also be addressed before merging to ensure security, maintainability, and test coverage.

The remaining issues can be addressed in follow-up PRs if needed, though fixing them now would be ideal.

[claude]

Security Issue: Inconsistent JSON Schema Escaping

The comment says "no escaping - parseShellArgs handles it", but single quotes aren't escaped. This differs from src/utils/json-schema.ts which properly escapes quotes:

const escapedSchema = schema.replace(/'/g, "'\\''");
return `${claudeArgs} --json-schema '${escapedSchema}'`;

JSON schemas often contain quotes. Without proper escaping, schemas with quotes will fail or behave unexpectedly.

Suggested change

	// Add JSON schema if specified (no escaping - parseShellArgs handles it)
	if (process.env.JSON_SCHEMA) {
	// Wrap in single quotes for parseShellArgs
	claudeArgs += ` --json-schema '${process.env.JSON_SCHEMA}'`;
	// Add JSON schema if specified (escape single quotes for shell safety)
	if (process.env.JSON_SCHEMA) {
	const escapedSchema = process.env.JSON_SCHEMA.replace(/'/g, "'\\''");
	claudeArgs += ` --json-schema '${escapedSchema}'`;
	}

[claude]

Code Quality: Weak Type Safety

The ExecutionMessage type is too broad. type: string allows any string, making type guards ineffective. Missing fields like subtype, is_error, etc. that are used in the code.

Recommended improvement using discriminated unions:

type ExecutionMessage = 
  | { type: "system"; subtype: string; model?: string }
  | { type: "result"; subtype?: string; is_error?: boolean; 
      duration_ms?: number; cost_usd?: number;
      structured_output?: Record<string, unknown> }
  | { type: "turn"; content?: string }
  | { type: string }; // fallback for unknown types

This provides compile-time validation and better IDE support.

[claude]

Code Quality: Improve Error Message Context

The error message doesn't help debug why structured_output is missing. Could be schema validation failure, Claude error, or wrong message format.

Suggested change

	if (!result?.structured_output) {
	throw new Error(
	`json_schema was provided but Claude did not return structured_output.\n` +
	`Found ${messages.length} messages. Result exists: ${!!result}\n`,
	);
	if (!result?.structured_output) {
	const messageTypes = messages.map(m => m.type).join(', ');
	const hasResult = !!result;
	const hasStructuredOutput = result ? !!result.structured_output : false;
	throw new Error(
	`json_schema was provided but Claude did not return structured_output.\n` +
	`Found ${messages.length} messages (types: ${messageTypes}).\n` +
	`Result message exists: ${hasResult}${hasResult ? `, has structured_output: ${hasStructuredOutput}` : ''}.\n` +
	`This usually means Claude failed to comply with the schema or encountered an error.`,
	);
	}

[claude]

Test Coverage: Missing Edge Cases

The test suite is comprehensive but missing a few important edge cases:

1. Null values in structured output fields:

test("should handle null values in structured_output", async () => {
  await createMockExecutionFile({ nullable_field: null, present_field: "value" });
  await parseAndSetStructuredOutputs(TEST_EXECUTION_FILE);
  const parsed = JSON.parse(setOutputSpy.mock.calls[0][1]);
  expect(parsed.nullable_field).toBeNull();
});

2. Multiple result messages (currently uses find() which returns first match):

test("should use first result message when multiple exist", async () => {
  const messages = [
    { type: "result", structured_output: { first: true } },
    { type: "result", structured_output: { second: true } }
  ];
  await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
  await parseAndSetStructuredOutputs(TEST_EXECUTION_FILE);
  const parsed = JSON.parse(setOutputSpy.mock.calls[0][1]);
  expect(parsed.first).toBe(true);
  expect(parsed.second).toBeUndefined();
});

[claude]

Security: Command Injection Risk

The JSON schema is wrapped in single quotes without proper escaping, creating a potential injection vector. An attacker controlling JSON_SCHEMA could include a single quote to inject malicious CLI flags.

Issue: '${process.env.JSON_SCHEMA}' doesn't escape single quotes within the schema.

Example Attack:

{"type":"object"}' --malicious-flag '

Result: --json-schema '{"type":"object"}' --malicious-flag ''

Fix: Apply the same escaping used in appendJsonSchemaArg (src/utils/json-schema.ts:15):

if (process.env.JSON_SCHEMA) {
  const escapedSchema = process.env.JSON_SCHEMA.replace(/'/g, "'\\''");
  claudeArgs += ` --json-schema '${escapedSchema}'`;
}

Or better yet, reuse the utility function:

import { appendJsonSchemaArg } from '../../utils/json-schema';
// ...
claudeArgs = appendJsonSchemaArg(claudeArgs, process.env.JSON_SCHEMA);

[claude]

Performance: Inefficient File Parsing

This reads and parses the entire execution file (potentially 10MB per line 344) to extract just the last result message. For long-running Claude sessions, this is 50-100x slower than necessary.

Issue: Full file read + full JSON parse + array iteration when only the last message is needed.

Optimization: Since the execution file is newline-delimited JSON, read it in reverse:

// Read last ~10KB of file (enough for typical result message)
const fd = await open(executionFile, 'r');
const stat = await fd.stat();
const readSize = Math.min(10240, stat.size);
const buffer = Buffer.alloc(readSize);
await fd.read(buffer, 0, readSize, stat.size - readSize);
await fd.close();

// Parse lines in reverse
const lines = buffer.toString('utf-8').split('\n').reverse();
for (const line of lines) {
  if (!line.trim()) continue;
  try {
    const msg = JSON.parse(line);
    if (msg.type === 'result' && msg.structured_output) {
      return msg.structured_output;
    }
  } catch { continue; }
}

Also consider: Add size limit validation before parsing (currently vulnerable to DoS via huge JSON).

[claude]

Documentation Issue: Fragile fromJSON() pattern

This pattern is fragile - if the action fails or output is malformed, the workflow will error. GitHub Actions expressions don't handle JSON parsing failures gracefully.

Better approach (add this to docs):

# Safer: check output exists before parsing
- name: Retry if flaky
  if: |
    steps.analyze.outputs.structured_output != '' && 
    fromJSON(steps.analyze.outputs.structured_output).is_flaky == true
  run: gh workflow run CI

Or parse explicitly:

- name: Parse results
  id: parse
  run: |
    OUTPUT='${{ steps.analyze.outputs.structured_output }}'
    if [ -n "$OUTPUT" ]; then
      echo "is_flaky=$(echo "$OUTPUT" | jq -r '.is_flaky')" >> $GITHUB_OUTPUT
    else
      echo "is_flaky=false" >> $GITHUB_OUTPUT
    fi
    
- name: Retry if flaky  
  if: steps.parse.outputs.is_flaky == 'true'
  run: gh workflow run CI

[claude]

Documentation Gap: Missing Failure Behavior

The docs don't explain what happens when Claude fails to return structured output.

Add section:

### Error Handling

If `json_schema` is provided but Claude cannot return structured output, **the action will fail** with:

json_schema was provided but Claude did not return structured_output

This ensures your automation workflows don't proceed with missing data.

**To handle failures gracefully**:
```yaml
- name: Analyze code
  id: analyze
  continue-on-error: true
  uses: anthropics/claude-code-action@v1
  with:
    json_schema: |
      {...}

- name: Handle failure
  if: steps.analyze.outputs.conclusion == 'failure'
  run: echo "Analysis failed - using defaults"

[claude]

Test Coverage Gap: Missing critical edge cases

The tests are comprehensive but miss several important scenarios:

1. structured_output: null vs undefined - Current test only checks when key is missing, not when explicitly set to null
2. Multiple result messages - Should verify findLast() returns the last one
3. Non-object structured_output - What if Claude returns an array/string/number at root?

Add tests:

test("should handle structured_output: null", async () => {
  const messages = [
    { type: "result", structured_output: null }
  ];
  await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
  
  await expect(parseAndSetStructuredOutputs(TEST_EXECUTION_FILE))
    .rejects.toThrow("did not return structured_output");
});

test("should use last result when multiple exist", async () => {
  await createMockExecutionFile({ first: true });
  // Add another result
  const messages = JSON.parse(await readFile(TEST_EXECUTION_FILE, "utf-8"));
  messages.push({ type: "result", structured_output: { last: true } });
  await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
  
  await parseAndSetStructuredOutputs(TEST_EXECUTION_FILE);
  
  const output = JSON.parse(setOutputSpy.mock.calls[0][1]);
  expect(output.last).toBe(true);
  expect(output.first).toBeUndefined();
});

[claude]

Code Quality: Misleading error handling pattern

This pattern claims to "preserve original error and stack trace" but that's misleading:

1. Re-throwing doesn't preserve the catch site in the stack
2. The non-Error case (second branch) is effectively unreachable since all errors thrown in the try block are Error instances:
   • readFile throws Error
   • JSON.parse throws SyntaxError (extends Error)
   • Line 146 throws Error

Options:

A) Remove the pattern (simple):

} catch (error) {
  // All errors in try block are Error instances
  throw error instanceof Error 
    ? error 
    : new Error(`Unexpected error: ${error}`);
}

B) Document it's defensive (if you want to keep it):

} catch (error) {
  // Defensive: handle exotic non-Error throws (strings, numbers, etc.)
  // In practice, all errors from readFile/JSON.parse are Error instances
  if (error instanceof Error) {
    throw error;
  }
  throw new Error(`Failed to parse structured outputs: ${error}`);
}

[claude]

Should use @v1 instead of @main for consistency with documentation and stability.

[claude]

Summary: Comprehensive Code Review

I've completed a thorough review of PR #683 using specialized review agents. Here are the key findings:

---

🔴 Critical Issues

1. Security: Command Injection Vulnerability

File: base-action/src/index.ts:40-42

The JSON schema is concatenated into shell arguments without proper escaping, creating a potential command injection vector. See inline comment for details and fix.

2. Missing Output in Main Action

File: action.yml

The structured_output is generated by the base-action but not exposed in the main action's outputs section. Users cannot access this critical feature when using the full action.

---

🟡 High Priority Issues

3. Performance: Inefficient File Parsing (base-action/src/run-claude.ts:134-143)

4. Documentation: Fragile fromJSON() Pattern (docs/usage.md:215-217)

5. Documentation: Missing Failure Behavior (docs/usage.md:230)

---

🟢 Medium Priority Issues

6. Test Coverage Gaps (base-action/test/structured-output.test.ts)

7. Code Quality: Misleading Error Handling (base-action/src/run-claude.ts:159-165)

8. Example Uses Wrong Version (examples/test-failure-analysis.yml:27)

---

✅ Positive Aspects

1. Comprehensive test suite: 26 unit tests + 5 integration test jobs
2. Safe argument passing with spawn()
3. Good security library usage
4. Clear API design
5. Well-structured code

---

Recommendations by Priority

Must fix before merge:

1. Fix command injection vulnerability (security)
2. Add structured_output to main action.yml outputs (functionality)
3. Add error handling guidance to documentation (usability)
4. Document failure behavior (usability)

Should fix:
5. Optimize file parsing performance
6. Add missing test coverage
7. Clarify error handling code comments
8. Fix example to use @v1

---

Review Statistics: 13 files reviewed, 7 inline comments posted, 2 critical issues, 3 high priority issues, 3 medium priority issues

[bogini]

Closing to create fresh PR without review noise