-
Notifications
You must be signed in to change notification settings - Fork 1
Pipeline Design 242
Code confirmed. The plan's analysis is accurate. Here's the ADR:
_extract_text_from_json() in scripts/sw-loop.sh:561-608 parses Claude's --output-format json responses to extract plain text for the build loop log. The function handles four cases: empty file, JSON array, JSON-looking-but-no-jq, and plain text.
The bug: Case 2 (line 579) only checks for first_char == "[" (arrays). When Claude outputs a JSON object ({"type":"result","result":"..."}), it falls through to Case 3 (line 599), which prints "JSON output but jq not available" — even when jq is installed. Case 3 conflates two distinct scenarios: (a) JSON object with jq available, and (b) any JSON without jq available.
Constraint: Bash 3.2 compatibility required (macOS default). No associative arrays, no readarray, no ${var,,}.
Extend Case 2 with an elif branch for JSON objects (Option A from the plan).
The fix adds a parallel extraction path for first_char == "{" that:
- Tries
jq -r '.result // empty'on the object directly (not.[-1].resultas with arrays) - Falls back to
.content // emptyif.resultis absent - Warns with an accurate message if neither field exists
- Leaves Case 3 to fire only when
command -v jqgenuinely fails
This keeps the array path (Case 2) untouched, so there is zero risk of regression to the primary code path. Case 3's condition ([[ "$first_char" == "[" || "$first_char" == "{" ]]) remains correct — it's now only reachable when jq is not installed.
Data flow after fix:
JSON object input → first_char == "{" → jq available?
→ yes: extract .result → found? → write to log_file ✓
→ not found: try .content → found? → write ✓
→ not found: warn "no .result field" ✓
→ no: Case 3 → warn "jq not available" (now accurate) ✓
-
Option B: Fix only the warning message in Case 3 — Pros: 1-2 line change, trivial blast radius. Cons: Doesn't extract
.resultfrom JSON objects — raw JSON gets dumped to the log file, which downstream consumers (progress reporting, convergence detection) can't use. Fixes the symptom, not the limitation. -
Merge Cases 2 and 3 into a unified JSON handler — Pros: DRYer code, single jq availability check. Cons: Restructures the entire function for a one-branch addition; higher review burden and regression risk for no functional benefit.
- Files to create: None
-
Files to modify:
-
scripts/sw-loop.sh— Addelifbranch at line ~596 (between current Case 2 and Case 3) -
scripts/sw-loop-test.sh— Add test case for JSON object extraction (after Test 19, ~line 299)
-
-
Dependencies: None (jq is already a soft dependency;
command -v jqguards all paths) -
Risk areas:
- The
sed -n '/^_extract_text_from_json()/,/^}/p'pattern in tests extracts the function by matching^}— the newelifbranch must not introduce a bare}at column 0, or the test extraction will truncate early. This is safe because all braces inside the function are indented. - The
.result // emptyjq filter must use// empty(not// "") to correctly distinguish between null/missing and empty string, matching the existing array path.
- The
-
_extract_text_from_jsonextracts.resultfrom a JSON object ({"type":"result","result":"text"}) into the log file -
_extract_text_from_jsonextracts.contentfrom a JSON object when.resultis absent -
_extract_text_from_jsonwarns accurately ("no .result field") for objects with neither.resultnor.content - JSON array extraction (
.[-1].result) remains unchanged — existing Test 17-19 and Test 21 pass - The "jq not available" warning fires only when
command -v jqfails (no false positives) - Plain text pass-through (Case 4) still works
- Empty file handling (Case 1) still works
- Binary input handling still doesn't crash
- No Bash 3.2 incompatibilities (
shellcheckclean, no new syntax) - All
sw-loop-test.shtests pass including the new JSON object test