* fix(#1936): reconstruct OpenCode review from JSON events; diagnosable empty-output stub On a large review prompt, OpenCode's default `build` agent runs a few read tool calls then ends its turn with zero output tokens (reason:"stop", output:0), so `opencode run --format default` emits empty stdout. The reviewer block redirected stderr to /dev/null and wrote a generic "failed or returned empty output" stub — so the phase silently lost its second independent reviewer with no diagnostic and no timeout. Rewrite the OpenCode reviewer block to invoke `--format json` as the primary call and reconstruct the review from the assistant `text` parts (jq). Capture stderr to a `.err` sidecar (mirrors the Codex block). When the agent emits no text, surface the stop reason, output-token count, and stderr so the failure is diagnosable. Gate the stub on the extracted CONTENT, not the output file size — an empty jq extraction still prints a lone newline that a `[ -s file ]` check would treat as populated. Document the wall-clock timeout as a Bash-tool param (macOS lacks GNU timeout; opencode has no native timeout flag). review.md was already at the DEFAULT size-tier ceiling (40956/40960), so the fix cannot fit without reclassifying it into the LARGE tier (it is a multi-reviewer orchestration file that outgrew "focused single-purpose"; 43.4 KB sits well under the LARGE high-water mark). Recapture the 16 golden-install fixtures — the diff is exactly one review.md hash per runtime. Regression block folded into review-default-reviewers-workflow.test.cjs (new bug-NNNN test files are not accepted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1936): add changeset * test(#1936): property-test the OpenCode review jq reconstruction Address the re-review's one actionable finding: the jq JSON-event → text reconstruction had no fast-check property test. Add tests/opencode-review-reconstruction.property.test.cjs. It extracts the two shipped jq programs (OPENCODE_REVIEW, OPENCODE_DIAG) verbatim from gsd-core/workflows/review.md and runs the real jq — not a reimplementation — so the shipped logic is what gets tested. Properties: the reconstructed review equals the newline-join of every assistant text part (order preserved); a stream with no text part reconstructs to empty (drives the #1936 stub); null/absent text parts are dropped, never rendered as "null". Plus example-based coverage of the diagnostic edges the reviewer cited: missing .tokens.output and no step_finish degrade to "?"; non-JSON stdout makes jq fail rather than masquerade as a review. Verified the invariant has teeth (a comma-join jq fails the property). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#1936): skip jq reconstruction property test when jq is absent The property test shells out to `jq`, which GitHub's windows-latest runners do not ship (macOS/Linux runners do). `execFileSync('jq')` therefore ENOENT-failed the whole file on `test (windows-latest, *)`. Probe `jq --version` at load and skip the suite when jq is not on PATH — the reconstruction logic is platform-independent, so the assertions still run in full on every jq-present runner (mirrors how golden-install-parity skips on win32). Verified: jq present → 7 pass; jq removed from PATH → 7 skipped, 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#1936): skip jq reconstruction property test on Windows, not just when jq is absent The prior guard skipped only when `jq` was absent from PATH — but the windows-latest runners DO ship jq, so the suite still ran there and failed with `jq: parse error: Invalid numeric literal` (confirmed from the CI job log). Root cause is Node's child_process argument quoting mangling the jq program (it embeds double quotes) on Windows, not the shipped review.md logic — the macOS/Linux legs pass. Gate the suite on `process.platform === 'win32'` (still also skipping when jq is absent), mirroring golden-install-parity's win32 skip. Logic is platform-independent and fully asserted on every macOS/Linux CI leg. Verified: macOS → 7 pass; simulated win32 → skips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
168 lines
7.6 KiB
JavaScript
168 lines
7.6 KiB
JavaScript
// allow-test-rule: source-text-is-the-product (see #1936)
|
|
// The OpenCode reviewer reconstructs its review from opencode's --format json
|
|
// event stream using two embedded jq programs in gsd-core/workflows/review.md.
|
|
// Those programs ARE the runtime contract; this test extracts them verbatim from
|
|
// the workflow and exercises the real jq (not a reimplementation) so the shipped
|
|
// reconstruction logic is what gets property-tested.
|
|
'use strict';
|
|
|
|
const { describe, test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const { execFileSync } = require('node:child_process');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
const fc = require('./helpers/fast-check-setup.cjs');
|
|
|
|
const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
|
const workflow = fs.readFileSync(reviewPath, 'utf-8');
|
|
|
|
// Extract the two shipped jq programs verbatim. If review.md changes their shape,
|
|
// these throw and the test fails loudly (intended coupling — #1936).
|
|
function extractJqProgram(varName) {
|
|
const re = new RegExp(`${varName}=\\$\\(jq -rs '([^']*)'`);
|
|
const m = workflow.match(re);
|
|
assert.ok(m, `review.md must define ${varName} via jq -rs '<program>' (#1936)`);
|
|
return m[1];
|
|
}
|
|
const TEXT_PROGRAM = extractJqProgram('OPENCODE_REVIEW'); // review reconstruction
|
|
const DIAG_PROGRAM = extractJqProgram('OPENCODE_DIAG'); // empty-output diagnostic
|
|
|
|
// This suite shells out to `jq`. On Windows, Node's child_process argument quoting
|
|
// mangles the jq program (it embeds quotes) — jq then raises a parse error — and
|
|
// jq isn't guaranteed on the host regardless. The reconstruction logic is
|
|
// platform-independent (the review workflow runs jq in its Unix-y runtime), so gate
|
|
// the suite to jq-present non-Windows hosts, mirroring golden-install-parity's win32
|
|
// skip. The assertions run in full on every macOS/Linux CI leg.
|
|
let jqAvailable = false;
|
|
try { execFileSync('jq', ['--version'], { stdio: 'ignore' }); jqAvailable = true; } catch { /* no jq on PATH */ }
|
|
const skipReason = process.platform === 'win32'
|
|
? 'jq invocation is not portable under Node child_process arg-quoting on Windows; logic is platform-independent and asserted on macOS/Linux'
|
|
: (jqAvailable ? false : 'jq not on PATH');
|
|
const opts = { skip: skipReason };
|
|
|
|
// Run a shipped jq program against a stream of events serialized exactly as
|
|
// opencode emits them: one JSON value per line (jq -s slurps them into an array).
|
|
// jq -r appends a single trailing newline to the (single) string result; strip it
|
|
// to recover the value the workflow's `$(…)` capture would see.
|
|
function runJq(program, events) {
|
|
const jsonl = events.map((e) => JSON.stringify(e)).join('\n');
|
|
const out = execFileSync('jq', ['-rs', program], { input: jsonl, encoding: 'utf8' });
|
|
return out.endsWith('\n') ? out.slice(0, -1) : out;
|
|
}
|
|
|
|
// Text values safe to round-trip through JSON → jq (utf8) → string. Excludes lone
|
|
// surrogates (which don't survive utf8) but keeps the interesting cases: newlines,
|
|
// quotes, backslashes, braces, unicode.
|
|
const safeText = fc
|
|
.string({ minLength: 0, maxLength: 40 })
|
|
.filter((s) => Buffer.from(s, 'utf8').toString('utf8') === s);
|
|
|
|
// A `text` event whose `.part.text` is a string, or null/absent (dropped by `// empty`).
|
|
const textEvent = fc.record({
|
|
type: fc.constant('text'),
|
|
part: fc.oneof(
|
|
fc.record({ text: safeText }),
|
|
fc.record({ text: fc.constant(null) }), // null → jq `// empty` drops it
|
|
fc.record({}), // absent → jq `// empty` drops it
|
|
),
|
|
});
|
|
const stepFinishEvent = fc.record({
|
|
type: fc.constant('step_finish'),
|
|
part: fc.record({
|
|
reason: fc.constantFrom('stop', 'length', 'tool_calls'),
|
|
tokens: fc.record({ output: fc.integer({ min: 0, max: 100000 }) }),
|
|
}),
|
|
});
|
|
const nonTextEvent = fc.oneof(
|
|
stepFinishEvent,
|
|
fc.record({ type: fc.constant('tool_use'), part: fc.record({ tool: safeText }) }),
|
|
fc.record({ type: fc.constant('step_start'), part: fc.record({}) }),
|
|
);
|
|
// Weight text events higher so streams routinely mix real review text with noise,
|
|
// but also generate text-free streams (the #1936 zero-output case).
|
|
const eventStream = fc.array(fc.oneof(textEvent, textEvent, nonTextEvent), {
|
|
minLength: 1,
|
|
maxLength: 30,
|
|
});
|
|
|
|
describe('#1936 OpenCode review reconstruction — jq properties', () => {
|
|
test('review == the newline-join of every assistant text part (order preserved)', opts, () => {
|
|
fc.assert(
|
|
fc.property(eventStream, (events) => {
|
|
const expected = events
|
|
.filter((e) => e.type === 'text' && e.part && typeof e.part.text === 'string')
|
|
.map((e) => e.part.text)
|
|
.join('\n');
|
|
assert.equal(runJq(TEXT_PROGRAM, events), expected);
|
|
}),
|
|
);
|
|
});
|
|
|
|
test('a stream with no assistant text part reconstructs to empty (drives the #1936 stub)', opts, () => {
|
|
fc.assert(
|
|
fc.property(fc.array(nonTextEvent, { minLength: 1, maxLength: 20 }), (events) => {
|
|
// This is the exact failure the bug describes: the agent runs tool calls
|
|
// and ends with step_finish, emitting no text. Reconstruction must be empty
|
|
// so the content-gate (`[ -n "$OPENCODE_REVIEW" ]`) falls through to the stub.
|
|
assert.equal(runJq(TEXT_PROGRAM, events), '');
|
|
}),
|
|
);
|
|
});
|
|
|
|
test('text parts that are null/absent are dropped, never rendered as "null"', opts, () => {
|
|
fc.assert(
|
|
fc.property(
|
|
fc.array(
|
|
fc.oneof(
|
|
fc.record({ type: fc.constant('text'), part: fc.record({ text: fc.constant(null) }) }),
|
|
fc.record({ type: fc.constant('text'), part: fc.record({}) }),
|
|
),
|
|
{ minLength: 1, maxLength: 10 },
|
|
),
|
|
(events) => {
|
|
const out = runJq(TEXT_PROGRAM, events);
|
|
assert.equal(out, '');
|
|
assert.doesNotMatch(out, /null/);
|
|
},
|
|
),
|
|
);
|
|
});
|
|
|
|
// Diagnostic path (empty-output stub). The finding calls out `missing .tokens.output`
|
|
// and no-step_finish as real edges — pin them with examples against the shipped jq.
|
|
describe('diagnostic reconstruction (stop reason + output tokens)', () => {
|
|
test('reports reason and output tokens from the LAST step_finish', opts, () => {
|
|
const events = [
|
|
{ type: 'step_finish', part: { reason: 'tool_calls', tokens: { output: 5 } } },
|
|
{ type: 'tool_use', part: {} },
|
|
{ type: 'step_finish', part: { reason: 'stop', tokens: { output: 0 } } },
|
|
];
|
|
assert.equal(runJq(DIAG_PROGRAM, events), 'stop reason=stop, output tokens=0');
|
|
});
|
|
|
|
test('missing .tokens.output degrades to "?" rather than null/garbage', opts, () => {
|
|
const events = [{ type: 'step_finish', part: { reason: 'stop', tokens: {} } }];
|
|
assert.equal(runJq(DIAG_PROGRAM, events), 'stop reason=stop, output tokens=?');
|
|
});
|
|
|
|
test('no step_finish at all degrades both fields to "?"', opts, () => {
|
|
const events = [{ type: 'tool_use', part: { tool: 'read' } }];
|
|
assert.equal(runJq(DIAG_PROGRAM, events), 'stop reason=?, output tokens=?');
|
|
});
|
|
});
|
|
|
|
// The primary reconstruction runs before any content gate; on non-JSON stdout
|
|
// (e.g. an opencode crash that printed a plain-text error) jq must fail rather
|
|
// than emit that text as a "review" — the workflow's `2>/dev/null` + empty
|
|
// capture then routes to the diagnostic stub.
|
|
test('non-JSON stdout does not masquerade as a reconstructed review', opts, () => {
|
|
let threw = false;
|
|
try {
|
|
execFileSync('jq', ['-rs', TEXT_PROGRAM], { input: 'auth token expired\n', encoding: 'utf8' });
|
|
} catch {
|
|
threw = true;
|
|
}
|
|
assert.ok(threw, 'jq must reject non-JSON input so it cannot be captured as a review');
|
|
});
|
|
});
|