golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#1936): reconstruct OpenCode review from JSON events (#1992)

Browse Source 
* 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>
This commit is contained in:
Behruz Nassre Esfahani
2026-07-05 11:07:07 -07:00
committed by GitHub
parent 8de2ff9121
commit 23254ca5a7
21 changed files with 312 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/bold-deer-zip.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 1992
---
**OpenCode reviewer no longer silently yields an empty review on large prompts** — `/gsd-review --opencode` now invokes `opencode run --format json` and reconstructs the review from the assistant text parts, so a large-prompt run where the default `build` agent ends its turn with zero output tokens no longer produces an empty stub. When the agent genuinely emits no text, the stub now reports the stop reason, output-token count, and captured stderr instead of a generic message. (#1936)

44
gsd-core/workflows/review.md
View File

@@ -302,14 +302,50 @@ coderabbit review --prompt-only 2>/dev/null > /tmp/gsd-review-coderabbit-{phase}
```
**OpenCode (via GitHub Copilot):**
OpenCode's default `build` agent is an agentic coder, not a prompt→completion API.
On a large review prompt it may run a few `read` tool calls and then end its turn
with **zero output tokens** (`reason:"stop"`, `output:0`), so `--format default`
yields empty stdout and the second reviewer is silently lost (#1936). Invoke with
`--format json` and reconstruct the review from the assistant `text` parts; if the
agent emitted none, surface the stop `reason`, output-token count, and captured
stderr so the failure is diagnosable instead of a generic empty stub. Runs are also
nondeterministic in length, so bound this Bash tool call with a wall-clock timeout —
set `timeout: 660000` on the call (same mechanism the CodeRabbit block documents).
That bound is hard: if it fires mid-`opencode run` the tool kills the command and
the jq reconstruction below never runs, so the reviewing agent simply proceeds
without an OpenCode result. The completing zero-output case — the actual #1936 bug —
is fully handled below; the timeout only backstops the rarer nondeterministic hang. A
reviewer instance with `"agent": "review"` (see
`gsd-core/references/reviewer-instances.md`) sidesteps the default `build` agent and
is the durable fix when this recurs.
```bash
# stderr → sidecar (never /dev/null) so a real error is diagnosable — mirrors the
# Codex block. --format json is the primary invocation (not a fallback): the review
# text lives in assistant `text` parts, which the default formatter drops when the
# agent stops with no final message (#1936).
if [ -n "$OPENCODE_MODEL" ] && [ "$OPENCODE_MODEL" != "null" ]; then
cat /tmp/gsd-review-prompt-{phase}.md | opencode run --model "$OPENCODE_MODEL" - 2>/dev/null > /tmp/gsd-review-opencode-{phase}.md
set -- --model "$OPENCODE_MODEL"
else
cat /tmp/gsd-review-prompt-{phase}.md | opencode run - 2>/dev/null > /tmp/gsd-review-opencode-{phase}.md
set --
fi
if [ ! -s /tmp/gsd-review-opencode-{phase}.md ]; then
echo "OpenCode review failed or returned empty output." > /tmp/gsd-review-opencode-{phase}.md
cat /tmp/gsd-review-prompt-{phase}.md | opencode run "$@" --format json - 2>/tmp/gsd-review-opencode-{phase}.err > /tmp/gsd-review-opencode-{phase}.json
# Reconstruct the review from the assistant text parts. Capture into a variable and
# test its CONTENT (not the output file's size): an empty extraction still prints a
# trailing newline, which would fool a `[ -s file ]` check into skipping the stub.
OPENCODE_REVIEW=$(jq -rs '[.[] | select(.type=="text") | .part.text // empty] | join("\n")' /tmp/gsd-review-opencode-{phase}.json 2>/dev/null)
if [ -n "$OPENCODE_REVIEW" ]; then
printf '%s\n' "$OPENCODE_REVIEW" > /tmp/gsd-review-opencode-{phase}.md
else
# No assistant text (agent emitted no final message, or stdout was not valid JSON events).
{
echo "OpenCode review returned no assistant text (#1936: agent ended its turn with no final message)."
OPENCODE_DIAG=$(jq -rs '[.[] | select(.type=="step_finish")] | last | "stop reason=\(.part.reason // "?"), output tokens=\(.part.tokens.output // "?")"' /tmp/gsd-review-opencode-{phase}.json 2>/dev/null)
[ -n "$OPENCODE_DIAG" ] && echo "Diagnostic: $OPENCODE_DIAG"
echo "stderr:"
cat /tmp/gsd-review-opencode-{phase}.err
} > /tmp/gsd-review-opencode-{phase}.md
fi
```

2
tests/fixtures/golden-install-parity/antigravity.json vendored
View File

@@ -278,7 +278,7 @@
"gsd-core/workflows/remove-phase.md": "6b9947fba1a97f46",
"gsd-core/workflows/remove-workspace.md": "95f05defffe6754e",
"gsd-core/workflows/resume-project.md": "cadf390bae95fc76",
"gsd-core/workflows/review.md": "aa08449a6a180944",
"gsd-core/workflows/review.md": "42efd80befc112de",
"gsd-core/workflows/scan.md": "f2754f3e3ea528ff",
"gsd-core/workflows/secure-phase.md": "78705b7f09612464",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/augment.json vendored
View File

@@ -348,7 +348,7 @@
"gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9",
"gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc",
"gsd-core/workflows/resume-project.md": "b18b51fd15cbce95",
"gsd-core/workflows/review.md": "fbfa6eebcad41aa0",
"gsd-core/workflows/review.md": "b59974ca892b7404",
"gsd-core/workflows/scan.md": "54ff1ff60041d065",
"gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/claude.json vendored
View File

@@ -277,7 +277,7 @@
"gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09",
"gsd-core/workflows/remove-workspace.md": "0e73844f0f41cbc3",
"gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35",
"gsd-core/workflows/review.md": "a1d3ebce6222ff72",
"gsd-core/workflows/review.md": "fec86657cbd9e92f",
"gsd-core/workflows/scan.md": "8e1bbf2eed1752ca",
"gsd-core/workflows/secure-phase.md": "b2b9100ff79d6017",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/cline.json vendored
View File

@@ -281,7 +281,7 @@
"gsd-core/workflows/remove-phase.md": "61b68a4af414e3c2",
"gsd-core/workflows/remove-workspace.md": "411bfff942216185",
"gsd-core/workflows/resume-project.md": "f8f71b5a98374b85",
"gsd-core/workflows/review.md": "f7d5b0f64631adea",
"gsd-core/workflows/review.md": "a09b4d6ece11ddb6",
"gsd-core/workflows/scan.md": "db0c48c3963f9a8b",
"gsd-core/workflows/secure-phase.md": "c51b86e369574195",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/codebuddy.json vendored
View File

@@ -348,7 +348,7 @@
"gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9",
"gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc",
"gsd-core/workflows/resume-project.md": "b18b51fd15cbce95",
"gsd-core/workflows/review.md": "fbfa6eebcad41aa0",
"gsd-core/workflows/review.md": "b59974ca892b7404",
"gsd-core/workflows/scan.md": "54ff1ff60041d065",
"gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/codex.json vendored
View File

@@ -313,7 +313,7 @@
"gsd-core/workflows/remove-phase.md": "75c8ca0dbd1ce404",
"gsd-core/workflows/remove-workspace.md": "a9d1dcda7755b6c9",
"gsd-core/workflows/resume-project.md": "94ac2cac4c562557",
"gsd-core/workflows/review.md": "bd3873bea54d6143",
"gsd-core/workflows/review.md": "0ed332a3ab1426e6",
"gsd-core/workflows/scan.md": "29f3b65f5d9de885",
"gsd-core/workflows/secure-phase.md": "858b5e1152b569ed",
"gsd-core/workflows/session-report.md": "dd8fa011c9394075",

2
tests/fixtures/golden-install-parity/copilot.json vendored
View File

@@ -279,7 +279,7 @@
"gsd-core/workflows/remove-phase.md": "e94ddfe4eabc0e08",
"gsd-core/workflows/remove-workspace.md": "f28be7f32249f0d4",
"gsd-core/workflows/resume-project.md": "4ebcb4acd3c29302",
"gsd-core/workflows/review.md": "60b8ca1c543ccdfa",
"gsd-core/workflows/review.md": "2b673bcc3a00e2f5",
"gsd-core/workflows/scan.md": "28a2847b8d04156e",
"gsd-core/workflows/secure-phase.md": "bae509fa254fe435",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/cursor.json vendored
View File

@@ -348,7 +348,7 @@
"gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09",
"gsd-core/workflows/remove-workspace.md": "dadbb14d9033ea3f",
"gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35",
"gsd-core/workflows/review.md": "141589a39b8c7e18",
"gsd-core/workflows/review.md": "3002073a6f3b5f83",
"gsd-core/workflows/scan.md": "8e1bbf2eed1752ca",
"gsd-core/workflows/secure-phase.md": "7bd4c335484fd121",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/hermes.json vendored
View File

@@ -278,7 +278,7 @@
"gsd-core/workflows/remove-phase.md": "f27cecd8c78008ae",
"gsd-core/workflows/remove-workspace.md": "977155e18b9df623",
"gsd-core/workflows/resume-project.md": "849f3d49d366ff13",
"gsd-core/workflows/review.md": "06cd9992e289b241",
"gsd-core/workflows/review.md": "aeedade386e820f5",
"gsd-core/workflows/scan.md": "cbde2b8b2b5fa5dd",
"gsd-core/workflows/secure-phase.md": "5fc8fbd5e217e48d",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/kilo.json vendored
View File

@@ -348,7 +348,7 @@
"gsd-core/workflows/remove-phase.md": "f76e1c2a4dd31a09",
"gsd-core/workflows/remove-workspace.md": "7bd5d1b63ef091a8",
"gsd-core/workflows/resume-project.md": "3dcaa7abe1800d35",
"gsd-core/workflows/review.md": "9ed1fda4d8b657f1",
"gsd-core/workflows/review.md": "2af34d173c11f74e",
"gsd-core/workflows/scan.md": "8e1bbf2eed1752ca",
"gsd-core/workflows/secure-phase.md": "87fdcb37a8610e58",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/kimi.json vendored
View File

@@ -314,7 +314,7 @@
"gsd-core/workflows/remove-phase.md": "d030f80ca0df4fa9",
"gsd-core/workflows/remove-workspace.md": "b8817a3a5907f5bc",
"gsd-core/workflows/resume-project.md": "b18b51fd15cbce95",
"gsd-core/workflows/review.md": "fbfa6eebcad41aa0",
"gsd-core/workflows/review.md": "b59974ca892b7404",
"gsd-core/workflows/scan.md": "54ff1ff60041d065",
"gsd-core/workflows/secure-phase.md": "1d1c66ad9ea01bd2",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/opencode.json vendored
View File

@@ -348,7 +348,7 @@
"gsd-core/workflows/remove-phase.md": "863c58e99e9d630d",
"gsd-core/workflows/remove-workspace.md": "4ed03c52e5c7bd4d",
"gsd-core/workflows/resume-project.md": "15153ce5a4c38674",
"gsd-core/workflows/review.md": "4076abd5ff01195d",
"gsd-core/workflows/review.md": "7cd3922bff5f4a48",
"gsd-core/workflows/scan.md": "4634caefa32a7307",
"gsd-core/workflows/secure-phase.md": "9616682fe23cf042",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/qwen.json vendored
View File

@@ -278,7 +278,7 @@
"gsd-core/workflows/remove-phase.md": "17c46fdcef27df2d",
"gsd-core/workflows/remove-workspace.md": "1d34788a9b2272d2",
"gsd-core/workflows/resume-project.md": "bba3250afbea8f09",
"gsd-core/workflows/review.md": "8ed9a9e7d150978d",
"gsd-core/workflows/review.md": "75d82be469799e0f",
"gsd-core/workflows/scan.md": "514d3eba81565193",
"gsd-core/workflows/secure-phase.md": "a7e8edb0e5f48256",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/trae.json vendored
View File

@@ -278,7 +278,7 @@
"gsd-core/workflows/remove-phase.md": "5a2521695edd486b",
"gsd-core/workflows/remove-workspace.md": "ed8e5e30c33f1bfd",
"gsd-core/workflows/resume-project.md": "ee9dcc4e3dd3e32c",
"gsd-core/workflows/review.md": "a2dab55fa3acfca0",
"gsd-core/workflows/review.md": "c9fd33007ace85d3",
"gsd-core/workflows/scan.md": "afc48f3339293b30",
"gsd-core/workflows/secure-phase.md": "882886f04d3b7e82",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

2
tests/fixtures/golden-install-parity/windsurf.json vendored
View File

@@ -278,7 +278,7 @@
"gsd-core/workflows/remove-phase.md": "24559d23e008c9c3",
"gsd-core/workflows/remove-workspace.md": "e958c1e932aef492",
"gsd-core/workflows/resume-project.md": "c5731b8aabed70f9",
"gsd-core/workflows/review.md": "dc8366ef168183db",
"gsd-core/workflows/review.md": "7ace79b5e67c9be3",
"gsd-core/workflows/scan.md": "8aa95448b1ba4a4a",
"gsd-core/workflows/secure-phase.md": "550152cd82c25c00",
"gsd-core/workflows/session-report.md": "2e5b1205324ddefa",

167
tests/opencode-review-reconstruction.property.test.cjs Normal file
View File

@@ -0,0 +1,167 @@
// 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');
});
});

83
tests/review-default-reviewers-workflow.test.cjs
View File

@@ -355,3 +355,86 @@ describe('#1698 regression: codex review is captured via --output-last-message,
});
});
}
// ────────────────────────────────────────────────────────────────────────
// #1936: OpenCode reviewer must not silently yield an empty review
// ────────────────────────────────────────────────────────────────────────
{
const { describe: __foldDescribe } = require('node:test');
__foldDescribe('#1936: OpenCode reviewer empty-output hardening', () => {
'use strict';
// allow-test-rule: source-text-is-the-product (see #1936)
// review.md is a workflow file whose embedded bash IS the runtime contract; the
// `opencode run` invocation on a large agentic prompt cannot be run in CI, so we
// assert on its content.
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
const read = () => fs.readFileSync(reviewPath, 'utf-8');
// Isolate the base OpenCode reviewer block (heading -> next reviewer heading) so
// assertions about its stderr handling don't accidentally match sibling reviewers
// (gemini/claude/coderabbit/qwen legitimately use /dev/null).
function openCodeBlock() {
const c = read();
const start = c.indexOf('**OpenCode (via GitHub Copilot):**');
assert.notStrictEqual(start, -1, 'review.md must contain the base OpenCode reviewer block');
const rest = c.slice(start + 1);
const nextHeading = rest.search(/\n\*\*[A-Z][^\n]*:\*\*/);
return nextHeading === -1 ? c.slice(start) : c.slice(start, start + 1 + nextHeading);
}
describe('bug #1936: OpenCode reviewer must not silently yield an empty review', () => {
test('captures opencode stderr to a sidecar, never /dev/null', () => {
const block = openCodeBlock();
assert.match(block, /opencode run [^\n]*2>\/tmp\/gsd-review-opencode-\{phase\}\.err/,
'the opencode invocation must send stderr to a .err sidecar so failures are diagnosable');
assert.doesNotMatch(block, /opencode run [^\n]*2>\/dev\/null/,
'the opencode invocation must not discard stderr to /dev/null (#1936)');
});
test('requests structured JSON output and reconstructs review from assistant text parts', () => {
const block = openCodeBlock();
assert.match(block, /opencode run [^\n]*--format json/,
'must invoke opencode with --format json so assistant text parts are recoverable');
assert.match(block, /select\(\.type=="text"\)\s*\|\s*\.part\.text/,
'must extract the assistant text parts via `.part.text` from the JSON event stream');
});
test('gates the empty-review stub on extracted CONTENT, not output-file size', () => {
// An empty jq extraction still writes a trailing newline, so a `[ -s file ]`
// check would treat a content-less review as populated and skip the stub. The
// block must test the captured text variable instead.
const block = openCodeBlock();
assert.match(block, /OPENCODE_REVIEW=\$\(jq/, 'must capture the extraction into a variable');
assert.match(block, /\[ -n "\$OPENCODE_REVIEW" \]/,
'must branch on the content of $OPENCODE_REVIEW, not on the size of the .md file');
assert.doesNotMatch(block, /\[ ! -s \/tmp\/gsd-review-opencode-\{phase\}\.md \]/,
'must not gate the stub on `[ ! -s ...opencode...md ]` (a lone newline defeats it)');
});
test('empty-output stub is diagnosable: references #1936, stop reason/tokens, and stderr', () => {
const block = openCodeBlock();
assert.match(block, /#1936/, 'the empty-output stub must reference the issue');
assert.match(block, /step_finish[\s\S]*\.part\.reason[\s\S]*\.part\.tokens\.output/,
'the stub must surface the stop reason and output-token count from the final step_finish');
assert.match(block, /cat \/tmp\/gsd-review-opencode-\{phase\}\.err/,
'the stub must append the captured stderr');
});
test('does not regress the Codex reviewer block (still captures stderr to .err)', () => {
// #1936 changes only the OpenCode block; the Codex block's existing
// stderr-to-sidecar contract must remain intact.
assert.match(read(), /codex exec [^\n]*2>\/tmp\/gsd-review-codex-\{phase\}\.err/,
'the Codex reviewer block must be left unchanged');
});
});
});
}

2
tests/workflow-size-baseline.json
View File

@@ -63,7 +63,7 @@
"remove-phase.md": 8469,
"remove-workspace.md": 7507,
"resume-project.md": 17226,
"review.md": 40956,
"review.md": 43406,
"scan.md": 7688,
"secure-phase.md": 13476,
"session-report.md": 4044,

1
tests/workflow-size-budget.test.cjs
View File

@@ -124,6 +124,7 @@ const LARGE_WORKFLOWS = new Set([
'update', // 20766
'quick', // 45710
'code-review', // 28726
'review', // multi-reviewer orchestration; outgrew DEFAULT (was at the 40960 ceiling) when the OpenCode reviewer gained JSON reconstruction + a diagnosable empty-output stub (#1936)
]);
// Single source of truth for BOTH enumeration and measurement (#1074; finishes