feat(#2099): drive GitHub Copilot through the EoS descriptor + multi-event hook bus (ADR-1239)
Fold Copilot's residual runtime-literal branches onto descriptor-driven hostBehaviors. Several issue premises were inaccurate (verified via research) and deliberately NOT followed: writesSharedSettings/"legacy exclusion list" (per-runtime descriptor data, copilot's false is correct); RUNTIME_CONTENT_DISPATCH.copilot + installSurface==='copilot-instructions' (already descriptor-driven); extendedHookEvents (closed Claude/Gemini enum — hooks extended in code instead); reapply ternary (already folded, kimi #2095). Real folds (all byte-parity — golden byte-identical for every runtime): - src/install-engine.cts + src/surface.cts: the two `.agent.md` filename cutovers (_copyStaged + _syncGsdDir) unified onto hostBehaviors.agentFileExtension via a new exported agentFileExtensionFor() accessor (kills the two-mechanism divergence). - src/runtime-artifact-conversion.cts: applyAgentPathRewrites' copilot skip → hostBehaviors.noPathRewrite:true (antigravity #2096 precedent). - bin/install.js uninstall: the two isCopilot cleanup branches → installSurface=== 'copilot-instructions' gate (symmetric with install-time). - bin/install.js: `!isCopilot` in the two skipSharedHooksInstall checks → hostBehaviors.skipSharedHooksInstall:true (copilot has no shared gsd-*.js hooks). - bin/install.js: three dead legacy inline-agent-loop isCopilot refs removed (copilot ∈ _DESCRIPTOR_AGENTS_RUNTIMES → unreachable; byte-parity proven by clean golden + real reachable-runtime install diffs). isCopilot dropped from 4 destructures. Zero live `runtime==='copilot'`/`isCopilot` branches remain in bin/install.js, install-engine.cts, surface.cts, or runtime-artifact-conversion.cts (AC2 guard scans all four). UPGRADE 1 (multi-event hook bus): buildCopilotHookConfig() now emits preToolUse/ postToolUse/userPromptSubmitted/sessionEnd advisory handlers alongside sessionStart (static inline bash/powershell — deterministic, golden-trackable). Only copilot.json's gsd-session.json hash changes. UPGRADE 2 (background dispatch): surfaced via the negotiated contract only — dispatch.background:true exceeds the declarative-cli baseline and survives negotiation with no downgrade warning. NO .agent.md frontmatter field (copilot has none). MCP companion out of scope (AC4 names only 2 upgrades). Tests: declarative-reference-copilot (adapter/axes/fail-closed + AC2 4-file source-grep guard) + copilot-upgrades (live 5-event hook wiring; dispatch.background negotiation). Matrix EoS note + how-to; changeset (Changed). capability-registry regenerated. Incidental flaky-test RE-ARCHITECTURE (no-defer, maintainer-directed): tests/opencode-review-reconstruction.property.test.cjs spawned ~600 synchronous execFileSync('jq') subprocesses (numRuns:200 × 3 fast-check properties, one jq per generated stream); a single jq freezing on a contended macos-22 CI runner hung the whole unit-test chunk to its 600s kill (this PR's CI). --test-force-exit can't interrupt a synchronous execFileSync, so the cure is to stop spawning per case, not just time-bound it. Re-architected to run the SHIPPED jq program over the whole fast-check corpus in ONE jq process: each generated stream is one compact-JSON array per line in a temp file, `jq -c <PROGRAM>` (no -s) applies PROGRAM to each array (`.` == the array, exactly what production's `jq -rs <file>` sees after slurping) and emits one result per line — empirically byte-identical to the per-stream form across embedded-newline/empty/quote/ unicode/null-drop cases, and file-input (like production) so there's no stdin pipe to deadlock on large I/O. ~600 spawns → 6; coverage unchanged (200-case corpus per property, deterministic seeds) plus explicit boundary/diagnostic example batches. Still property- tests the real shipped jq (no JS reimplementation). Per-call jq timeout retained as a belt-and-suspenders bound. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
149
tests/copilot-upgrades.test.cjs
Normal file
149
tests/copilot-upgrades.test.cjs
Normal file
@@ -0,0 +1,149 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Copilot capability UPGRADES — ADR-1239 / #2099 (EoS/copilot).
|
||||
*
|
||||
* Drives the user-reachable surface (spawned `bin/install.js` via
|
||||
* `runMinimalInstall`) plus a direct negotiation-contract check to prove the
|
||||
* two real upgrades Copilot contributes as part of the EoS migration:
|
||||
*
|
||||
* UPGRADE 1 — multi-event hook bus: buildCopilotHookConfig() previously
|
||||
* emitted only `sessionStart`. This PR wires four additional events —
|
||||
* `preToolUse`, `postToolUse`, `userPromptSubmitted`, `sessionEnd` — each a
|
||||
* static, deterministic advisory command (no node-runner invocation), so a
|
||||
* live install's hooks/gsd-session.json registers all five events.
|
||||
*
|
||||
* UPGRADE 2 — dispatch.background: Copilot's capability.json already
|
||||
* declares `dispatch.background: true`, which legitimately EXCEEDS the
|
||||
* `declarative-cli` profile baseline (`false`, per
|
||||
* `PROFILE_BASELINES['declarative-cli']` in src/host-integration.cts).
|
||||
* NO agent-file/frontmatter change is involved — Copilot's .agent.md
|
||||
* frontmatter has no background-dispatch field (fields are
|
||||
* description/infer/mcp-servers/model/name/tools). This is surfaced
|
||||
* purely through the negotiated contract: a documented `true` value must
|
||||
* survive negotiation without a downgrade warning.
|
||||
*/
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const { runMinimalInstall } = require('./helpers/install-shared.cjs');
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
const {
|
||||
profileOf,
|
||||
negotiateHostCapabilities,
|
||||
PROFILE_BASELINES,
|
||||
} = require('../gsd-core/bin/lib/host-integration.cjs');
|
||||
|
||||
const COPILOT_CAP = JSON.parse(
|
||||
fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'copilot', 'capability.json'), 'utf8'),
|
||||
);
|
||||
const COPILOT_AXES = COPILOT_CAP.runtime.hostIntegration;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// UPGRADE 1: multi-event hook bus — all 4 newly-wired events' live-install
|
||||
// coverage, on top of the pre-existing sessionStart.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const NEWLY_WIRED_EVENTS = ['preToolUse', 'postToolUse', 'userPromptSubmitted', 'sessionEnd'];
|
||||
const ALL_EXPECTED_EVENTS = ['sessionStart', ...NEWLY_WIRED_EVENTS];
|
||||
|
||||
test('copilot --global: hooks/gsd-session.json wires sessionStart plus all 4 newly-added events (UPGRADE 1)', (t) => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'copilot', scope: 'global' });
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const hookPath = path.join(configDir, 'hooks', 'gsd-session.json');
|
||||
assert.ok(fs.existsSync(hookPath), `${hookPath} must exist`);
|
||||
const parsed = JSON.parse(fs.readFileSync(hookPath, 'utf8'));
|
||||
|
||||
assert.strictEqual(parsed.version, 1, 'hook config version must be 1');
|
||||
assert.ok(parsed.hooks && typeof parsed.hooks === 'object', 'has hooks object');
|
||||
|
||||
for (const eventName of ALL_EXPECTED_EVENTS) {
|
||||
const eventHooks = parsed.hooks[eventName];
|
||||
assert.ok(Array.isArray(eventHooks) && eventHooks.length > 0,
|
||||
`hooks.${eventName} must exist and be non-empty`);
|
||||
const entry = eventHooks[0];
|
||||
assert.strictEqual(entry.type, 'command', `${eventName} entry type must be 'command'`);
|
||||
assert.ok(typeof entry.bash === 'string' && entry.bash.length > 0,
|
||||
`${eventName} entry must have a non-empty inline bash body`);
|
||||
assert.ok(typeof entry.powershell === 'string' && entry.powershell.length > 0,
|
||||
`${eventName} entry must have a non-empty inline powershell body`);
|
||||
assert.strictEqual(entry.timeoutSec, 10, `${eventName} entry must use timeoutSec 10`);
|
||||
}
|
||||
});
|
||||
|
||||
test('each newly-wired event emits a distinct static advisory (no shared boilerplate, no node-runner invocation)', (t) => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'copilot', scope: 'global' });
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const hookPath = path.join(configDir, 'hooks', 'gsd-session.json');
|
||||
const parsed = JSON.parse(fs.readFileSync(hookPath, 'utf8'));
|
||||
|
||||
const bashBodies = new Set();
|
||||
for (const eventName of ALL_EXPECTED_EVENTS) {
|
||||
const entry = parsed.hooks[eventName][0];
|
||||
assert.ok(entry.bash.includes('"additionalContext"'),
|
||||
`${eventName} bash body must emit the additionalContext JSON envelope`);
|
||||
assert.ok(!/hooks\/gsd-[\w-]+\.(js|cjs|sh)/.test(entry.bash),
|
||||
`${eventName} bash body must not reference an external hook script (cannot dangle)`);
|
||||
bashBodies.add(entry.bash);
|
||||
}
|
||||
assert.strictEqual(bashBodies.size, ALL_EXPECTED_EVENTS.length,
|
||||
'each event must emit its own distinct advisory command, not a copy-pasted duplicate');
|
||||
});
|
||||
|
||||
test('a runtime whose hook config omits the new events does NOT get them (descriptor-gated, not a global default)', (t) => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'claude', scope: 'global' });
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
// Claude uses settings.json, not hooks/gsd-session.json — the Copilot
|
||||
// multi-event bus additions are self-contained to buildCopilotHookConfig
|
||||
// and must not leak into another runtime's hook surface.
|
||||
const copilotHookPath = path.join(configDir, 'hooks', 'gsd-session.json');
|
||||
assert.ok(!fs.existsSync(copilotHookPath),
|
||||
'claude must not have a Copilot-style hooks/gsd-session.json file');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// UPGRADE 2: dispatch.background negotiation — documented true survives,
|
||||
// exceeding the declarative-cli profile baseline. No agent-file/frontmatter
|
||||
// change; negotiated-contract only (Copilot .agent.md has no background field).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("copilot classifies as the 'declarative-cli' profile, whose baseline dispatch.background is false", () => {
|
||||
assert.equal(profileOf(COPILOT_AXES), 'declarative-cli');
|
||||
assert.equal(PROFILE_BASELINES['declarative-cli'].dispatch.background, false,
|
||||
'sanity: the declarative-cli baseline is false — copilot legitimately exceeds it');
|
||||
});
|
||||
|
||||
test('negotiateHostCapabilities surfaces copilot\'s documented dispatch.background:true with no downgrade warning (UPGRADE 2)', () => {
|
||||
assert.equal(COPILOT_AXES.dispatch.background, true,
|
||||
'sanity: the descriptor declares dispatch.background: true (documented, not undocumented)');
|
||||
|
||||
const { effective, warnings } = negotiateHostCapabilities(COPILOT_AXES);
|
||||
|
||||
assert.equal(effective.dispatch.background, true,
|
||||
'a documented true value must survive negotiation, exceeding the declarative-cli baseline of false');
|
||||
assert.ok(
|
||||
!warnings.some((w) => w.includes('dispatch.background')),
|
||||
`no warning may be raised for the documented dispatch.background axis, got: ${JSON.stringify(warnings)}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('copilot .agent.md frontmatter has no background-dispatch field (negotiated contract only, sanity)', (t) => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'copilot', scope: 'global' });
|
||||
t.after(() => cleanup(root));
|
||||
|
||||
const agentsDir = path.join(configDir, 'agents');
|
||||
const agentFiles = fs.readdirSync(agentsDir).filter((f) => f.endsWith('.agent.md'));
|
||||
assert.ok(agentFiles.length > 0, 'at least one .agent.md must be installed');
|
||||
|
||||
const sample = fs.readFileSync(path.join(agentsDir, agentFiles[0]), 'utf8');
|
||||
const frontmatterMatch = sample.match(/^---\r?\n([\s\S]*?)\r?\n---/);
|
||||
assert.ok(frontmatterMatch, 'agent file must have a frontmatter block');
|
||||
assert.ok(!/^background:/m.test(frontmatterMatch[1]),
|
||||
'UPGRADE 2 must not add a background: frontmatter field — Copilot .agent.md has no such field');
|
||||
});
|
||||
164
tests/declarative-reference-copilot.test.cjs
Normal file
164
tests/declarative-reference-copilot.test.cjs
Normal file
@@ -0,0 +1,164 @@
|
||||
// allow-test-rule: structural-regression-guard — AC2: assert no `runtime === 'copilot'` string-equality branch, no live `isCopilot` read remains in bin/install.js, src/install-engine.cts, src/surface.cts, or src/runtime-artifact-conversion.cts — a source-text property, so source-grep is the faithful check (#2099, expanded #2103)
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Declarative reference host — GitHub Copilot (#2099 / ADR-1239 EoS).
|
||||
*
|
||||
* Copilot already installs through the descriptor-driven artifactLayout
|
||||
* (skills/agents, each with a named `converter`), and its capability.json
|
||||
* already declares `hostIntegration` + `dispatch` axes. Issue #2099's
|
||||
* literal premises about a "legacy exclusion list" / writesSharedSettings
|
||||
* mismatch and a hardcoded RUNTIME_CONTENT_DISPATCH branch were WRONG (see
|
||||
* the deep-research writeup on the issue) — this migration instead folds
|
||||
* the real residual `isCopilot` branches:
|
||||
* 1. src/install-engine.cts's `.agent.md` destination-suffix rename —
|
||||
* folded onto `hostBehaviors.agentFileExtension`.
|
||||
* 2. bin/install.js uninstall's two Copilot side-effect branches
|
||||
* (repo-root AGENTS.md cleanup + copilot-instructions.md/hook
|
||||
* cleanup) — folded onto `resolveInstallPlan(runtime).installSurface
|
||||
* === 'copilot-instructions'` (unique to copilot, so byte-parity).
|
||||
* 3. bin/install.js's two `skipSharedHooksInstall` checks — folded onto
|
||||
* `hostBehaviors.skipSharedHooksInstall:true` (copilot's golden has
|
||||
* only hooks/gsd-session.json, no shared gsd-*.js scripts).
|
||||
* 4. bin/install.js's dead legacy agent-converter dispatch arm in the
|
||||
* inline agent-copy loop — unreachable because copilot is a member of
|
||||
* `_DESCRIPTOR_AGENTS_RUNTIMES` (installRuntimeArtifacts already wrote
|
||||
* the agents before that loop runs) — deleted outright.
|
||||
*
|
||||
* This test is the reference-host dogfood mirroring
|
||||
* tests/declarative-reference-codebuddy.test.cjs: it (1) classifies
|
||||
* Copilot's profile via profileOf, (2) confirms the public declarative
|
||||
* adapter classifies it as declarative, (3) round-trips a real install
|
||||
* proving a gsd agent/skill surface is emitted, (4) proves negotiation
|
||||
* fails CLOSED on a corrupted descriptor, (5) proves the validator accepts
|
||||
* the descriptor, and (6) source-greps the folded modules for the retired
|
||||
* `isCopilot` branches (AC2).
|
||||
*
|
||||
* UPGRADE 1 (multi-event hook bus) + UPGRADE 2 (dispatch.background
|
||||
* negotiation) live-install coverage is in tests/copilot-upgrades.test.cjs
|
||||
* — not duplicated here.
|
||||
*/
|
||||
|
||||
const { test, before } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
|
||||
const {
|
||||
profileOf,
|
||||
negotiateHostCapabilities,
|
||||
PROFILE_BASELINES,
|
||||
UNDOCUMENTED,
|
||||
} = require('../gsd-core/bin/lib/host-integration.cjs');
|
||||
const { validateCapability } = require('../gsd-core/bin/lib/capability-validator.cjs');
|
||||
const { createDeclarativeAdapter } = require('../gsd-core/bin/lib/adapter-declarative.cjs');
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
const { walk, runMinimalInstall, BUILD_SCRIPT } = require('./helpers/install-shared.cjs');
|
||||
|
||||
const DESC = path.join(__dirname, '..', 'capabilities', 'copilot', 'capability.json');
|
||||
const COPILOT_CAP = JSON.parse(fs.readFileSync(DESC, 'utf8'));
|
||||
const COPILOT_AXES = COPILOT_CAP.runtime.hostIntegration;
|
||||
|
||||
// hooks/dist is gitignored and built (mirrors golden-install-parity harness).
|
||||
before(() => {
|
||||
execFileSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf-8', stdio: 'pipe' });
|
||||
});
|
||||
|
||||
test('Copilot classifies as the declarative-cli reference profile (profileOf)', () => {
|
||||
const desc = JSON.parse(fs.readFileSync(DESC, 'utf8'));
|
||||
const axes = desc.runtime.hostIntegration;
|
||||
assert.ok(axes && axes.embeddingMode, 'copilot descriptor declares hostIntegration axes');
|
||||
assert.equal(profileOf(axes), 'declarative-cli',
|
||||
'Copilot is a Declarative-CLI host');
|
||||
});
|
||||
|
||||
test('the public declarative adapter classifies Copilot as a declarative host', () => {
|
||||
const adapter = createDeclarativeAdapter({ runtime: 'copilot' });
|
||||
assert.equal(adapter.kind, 'declarative');
|
||||
assert.equal(adapter.runtime, 'copilot');
|
||||
assert.equal(typeof adapter.install, 'function');
|
||||
assert.equal(typeof adapter.uninstall, 'function');
|
||||
});
|
||||
|
||||
test('a real Copilot install emits a gsd agent/skill surface (invocable)', () => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'copilot', scope: 'global' });
|
||||
try {
|
||||
const files = walk(configDir);
|
||||
assert.ok(files.length > 0, 'install must emit artifacts');
|
||||
const gsdSurface = files.filter((f) => /gsd/i.test(path.relative(configDir, f)));
|
||||
assert.ok(gsdSurface.length > 0,
|
||||
'install must emit a gsd agent/skill surface (declarative reference)');
|
||||
const agentsDir = path.join(configDir, 'agents');
|
||||
assert.ok(fs.existsSync(agentsDir), 'agents/ directory must exist');
|
||||
const agentFiles = fs.readdirSync(agentsDir)
|
||||
.filter((f) => f.startsWith('gsd-') && f.endsWith('.agent.md'));
|
||||
assert.ok(agentFiles.length > 0, 'agents/ must contain gsd-*.agent.md files');
|
||||
const skillsDir = path.join(configDir, 'skills');
|
||||
assert.ok(fs.existsSync(skillsDir), 'skills/ directory must exist');
|
||||
} finally {
|
||||
cleanup(root);
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #2099 EoS/copilot — fail-closed negotiation + validator acceptance +
|
||||
// the folded descriptor (mirrors codebuddy/antigravity/qwen reference tests).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('negotiateHostCapabilities never throws for copilot, even fully corrupted', () => {
|
||||
assert.doesNotThrow(() => negotiateHostCapabilities({}));
|
||||
assert.doesNotThrow(() => negotiateHostCapabilities({ ...COPILOT_AXES, embeddingMode: UNDOCUMENTED }));
|
||||
assert.doesNotThrow(() => negotiateHostCapabilities({ ...COPILOT_AXES, embeddingMode: 'future-unknown' }));
|
||||
assert.doesNotThrow(() => negotiateHostCapabilities({ ...COPILOT_AXES, dispatch: 'corrupted-not-an-object' }));
|
||||
assert.doesNotThrow(() => negotiateHostCapabilities({ ...COPILOT_AXES, dispatch: { ...COPILOT_AXES.dispatch, maxDepth: 'not-a-number' } }));
|
||||
});
|
||||
|
||||
test('a partial/empty copilot descriptor degrades to the safe floor, not the declarative-cli baseline', () => {
|
||||
const result = negotiateHostCapabilities({});
|
||||
assert.equal(result.effective.embeddingMode, 'declarative', 'omitted embeddingMode degrades closed');
|
||||
assert.equal(result.effective.hookBus, 'none');
|
||||
assert.notDeepEqual(result.effective, PROFILE_BASELINES['declarative-cli']);
|
||||
assert.ok(result.warnings.length > 0);
|
||||
});
|
||||
|
||||
test('capabilities/copilot/capability.json validates — no errors', () => {
|
||||
const errors = validateCapability(COPILOT_CAP, 'copilot');
|
||||
assert.deepEqual(errors, [], `validateCapability must return no errors, got: ${JSON.stringify(errors)}`);
|
||||
});
|
||||
|
||||
// -- AC2: the hardcoded branches are retired across all folded modules ------
|
||||
|
||||
test('no `runtime === "copilot"` string-equality branch (nor live `isCopilot` read) remains in bin/install.js, src/install-engine.cts, src/surface.cts, or src/runtime-artifact-conversion.cts (AC2)', () => {
|
||||
const strip = (src) => src
|
||||
.replace(/\/\*[\s\S]*?\*\//g, '')
|
||||
.replace(/\/\/[^\r\n]*/g, '')
|
||||
.replace(/`[^`]*`/g, '');
|
||||
const repoRoot = path.join(__dirname, '..');
|
||||
const files = [
|
||||
path.join(repoRoot, 'bin', 'install.js'),
|
||||
path.join(repoRoot, 'src', 'install-engine.cts'),
|
||||
path.join(repoRoot, 'src', 'surface.cts'),
|
||||
path.join(repoRoot, 'src', 'runtime-artifact-conversion.cts'),
|
||||
];
|
||||
for (const file of files) {
|
||||
const src = fs.readFileSync(file, 'utf8');
|
||||
const stripped = strip(src);
|
||||
|
||||
// NIT-1: catch both quote styles (single- and double-quoted 'copilot').
|
||||
const eqOffenders = stripped.match(/runtime\s*[!=]==\s*["']copilot["']/g) || [];
|
||||
assert.deepEqual(eqOffenders, [],
|
||||
`AC2: no hardcoded runtime==='copilot' branch may remain in ${path.relative(repoRoot, file)}; found: ${eqOffenders.join(', ')}`);
|
||||
|
||||
// Excludes legit enumeration sites: --copilot CLI flag parsing, the
|
||||
// '7':'copilot' numbered-menu map, allRuntimes/_DESCRIPTOR_AGENTS_RUNTIMES
|
||||
// set literals, config-dir lists, the copilot conversion FUNCTION names
|
||||
// (convertClaudeCommandToCopilotSkill / convertClaudeAgentToCopilotAgent /
|
||||
// convertCopilotToolName), and installSurface==='copilot-instructions'
|
||||
// (a descriptor-field string, not a runtime literal) — none of those
|
||||
// contain the token `isCopilot`, so a literal-word match is precise here.
|
||||
const isCopilotHits = stripped.match(/\bisCopilot\b/g) || [];
|
||||
assert.deepEqual(isCopilotHits, [],
|
||||
`AC2: no live isCopilot read may remain in ${path.relative(repoRoot, file)}; found ${isCopilotHits.length} occurrence(s)`);
|
||||
}
|
||||
});
|
||||
@@ -312,7 +312,7 @@
|
||||
"gsd-core/workflows/validate-phase.md": "2f705775a4b76d42",
|
||||
"gsd-core/workflows/verify-phase.md": "5c72780e34214e27",
|
||||
"gsd-core/workflows/verify-work.md": "c966971a3cbd1d71",
|
||||
"hooks/gsd-session.json": "0a462834f2a28fee",
|
||||
"hooks/gsd-session.json": "3382597f61e3a559",
|
||||
"scripts/changeset/README.md": "86ff89331dfd94b2",
|
||||
"scripts/changeset/cli.cjs": "68f92a344b199271",
|
||||
"scripts/changeset/github-release-notes.cjs": "795677f0c009b132",
|
||||
|
||||
@@ -4,14 +4,30 @@
|
||||
// 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.
|
||||
//
|
||||
// ARCHITECTURE (#2099): the shipped jq program is run over a WHOLE fast-check corpus
|
||||
// in ONE jq process, not once per generated case. Each generated event stream is
|
||||
// written as one compact-JSON array per line to a temp file, then `jq -c <PROGRAM>`
|
||||
// (no `-s`) applies PROGRAM to each array — `.` is that array, exactly what
|
||||
// production's `jq -rs` sees after slurping opencode's one-value-per-line stream —
|
||||
// emitting one compact-JSON result per line. This is empirically identical to the
|
||||
// per-stream `-rs` form (verified across embedded-newline/empty/quote/unicode/
|
||||
// null-drop cases) AND reads from a file like production (`jq -rs '…' <file>`), so
|
||||
// there is no stdin pipe to deadlock on large I/O. The prior design spawned ~600
|
||||
// synchronous jq subprocesses (numRuns × 3 properties); a single one freezing on a
|
||||
// contended CI runner hung the whole unit-test chunk to its 600s kill (macOS CI,
|
||||
// #2099). `node --test`'s --test-force-exit cannot interrupt a synchronous
|
||||
// execFileSync, so the cure is to stop spawning per case — not just to time-bound it.
|
||||
'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 os = require('node:os');
|
||||
const path = require('node:path');
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
|
||||
const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
||||
const workflow = fs.readFileSync(reviewPath, 'utf-8');
|
||||
@@ -34,20 +50,48 @@ const DIAG_PROGRAM = extractJqProgram('OPENCODE_DIAG'); // empty-output diagn
|
||||
// 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 */ }
|
||||
try { execFileSync('jq', ['--version'], { stdio: 'ignore', timeout: 10000, killSignal: 'SIGKILL' }); 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;
|
||||
// Bound each jq subprocess so a frozen spawn on a contended runner fails fast +
|
||||
// diagnosably (ETIMEDOUT) rather than hanging the chunk. jq over this corpus
|
||||
// completes in ~10ms, so 30s is an enormous margin that never trips on a healthy run.
|
||||
const JQ_EXEC_OPTS = { encoding: 'utf8', timeout: 30000, killSignal: 'SIGKILL', maxBuffer: 64 * 1024 * 1024 };
|
||||
|
||||
// Run a shipped jq program over an array of event streams in a SINGLE jq process.
|
||||
// Returns one result per input stream (order preserved), decoded from jq's compact
|
||||
// (`-c`) JSON output back to the raw string production's `-r` would have captured.
|
||||
// Both shipped programs yield exactly one value per stream (join(...) / last|"...");
|
||||
// the length assertion pins that invariant so a future program change that broke it
|
||||
// (0 or >1 outputs) fails loudly instead of silently misaligning results.
|
||||
function runJqBatch(program, streams) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-jq-batch-'));
|
||||
try {
|
||||
const file = path.join(dir, 'streams.jsonl');
|
||||
fs.writeFileSync(file, streams.map((events) => JSON.stringify(events)).join('\n') + '\n');
|
||||
const out = execFileSync('jq', ['-c', program, file], JQ_EXEC_OPTS);
|
||||
const lines = out.split('\n').filter((line) => line.length > 0);
|
||||
assert.equal(
|
||||
lines.length,
|
||||
streams.length,
|
||||
`jq must emit exactly one result per stream (got ${lines.length} for ${streams.length})`,
|
||||
);
|
||||
return lines.map((line) => JSON.parse(line));
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
}
|
||||
|
||||
// The intended reconstruction, computed independently of jq. jq is the unit under
|
||||
// test; this JS is the spec it must match on every generated case.
|
||||
function expectedReview(events) {
|
||||
return events
|
||||
.filter((e) => e.type === 'text' && e.part && typeof e.part.text === 'string')
|
||||
.map((e) => e.part.text)
|
||||
.join('\n');
|
||||
}
|
||||
|
||||
// Text values safe to round-trip through JSON → jq (utf8) → string. Excludes lone
|
||||
@@ -85,69 +129,82 @@ const eventStream = fc.array(fc.oneof(textEvent, textEvent, nonTextEvent), {
|
||||
maxLength: 30,
|
||||
});
|
||||
|
||||
// Corpus size per property. Matches the prior fast-check-setup numRuns:200 so
|
||||
// coverage is unchanged; distinct seeds give each property an independent corpus,
|
||||
// and fixed seeds keep the corpus deterministic across CI runs/OS legs.
|
||||
const CORPUS = 200;
|
||||
|
||||
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('review == the newline-join of every assistant text part, over a generated corpus (order preserved)', opts, () => {
|
||||
const streams = fc.sample(eventStream, { numRuns: CORPUS, seed: 42 });
|
||||
const actual = runJqBatch(TEXT_PROGRAM, streams); // one jq process for the whole corpus
|
||||
streams.forEach((events, i) => {
|
||||
assert.equal(
|
||||
actual[i],
|
||||
expectedReview(events),
|
||||
`case ${i}: shipped jq review must equal the spec for events=${JSON.stringify(events)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
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('a stream with no assistant text part reconstructs to empty (drives the #1936 stub), over a corpus', opts, () => {
|
||||
// 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.
|
||||
const streams = fc.sample(fc.array(nonTextEvent, { minLength: 1, maxLength: 20 }), { numRuns: CORPUS, seed: 43 });
|
||||
const actual = runJqBatch(TEXT_PROGRAM, streams);
|
||||
streams.forEach((events, i) => {
|
||||
assert.equal(actual[i], '', `case ${i}: text-free stream must reconstruct to '' for ${JSON.stringify(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/);
|
||||
},
|
||||
test('text parts that are null/absent are dropped, never rendered as "null", over a corpus', opts, () => {
|
||||
const nullish = 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 },
|
||||
);
|
||||
const streams = fc.sample(nullish, { numRuns: CORPUS, seed: 44 });
|
||||
const actual = runJqBatch(TEXT_PROGRAM, streams);
|
||||
streams.forEach((events, i) => {
|
||||
assert.equal(actual[i], '', `case ${i}: null/absent text must drop to ''`);
|
||||
assert.doesNotMatch(actual[i], /null/, `case ${i}: must never render "null"`);
|
||||
});
|
||||
});
|
||||
|
||||
// Explicit boundary + happy examples (deterministic, not sampled) — all in one
|
||||
// batched jq spawn. Pins the exact contract the corpus only covers probabilistically.
|
||||
test('boundary + happy example streams reconstruct exactly (batched)', opts, () => {
|
||||
const cases = [
|
||||
{ events: [{ type: 'text', part: { text: 'only' } }], expect: 'only' }, // single text part
|
||||
{ events: [{ type: 'text', part: { text: '' } }], expect: '' }, // empty-string text is kept
|
||||
{ events: [{ type: 'text', part: { text: 'a' } }, { type: 'tool_use', part: { tool: 'r' } }, { type: 'text', part: { text: 'b' } }], expect: 'a\nb' }, // text interleaved with noise
|
||||
{ events: [{ type: 'text', part: { text: 'x\ny' } }], expect: 'x\ny' }, // embedded newline preserved
|
||||
{ events: [{ type: 'tool_use', part: { tool: 'read' } }, { type: 'step_finish', part: { reason: 'stop', tokens: { output: 3 } } }], expect: '' }, // no text at all
|
||||
{ events: Array.from({ length: 30 }, (_v, i) => ({ type: 'text', part: { text: `p${i}` } })), expect: Array.from({ length: 30 }, (_v, i) => `p${i}`).join('\n') }, // max-size all-text stream
|
||||
];
|
||||
const actual = runJqBatch(TEXT_PROGRAM, cases.map((c) => c.events));
|
||||
cases.forEach((c, i) => assert.equal(actual[i], c.expect, `example ${i}: ${JSON.stringify(c.events)}`));
|
||||
});
|
||||
|
||||
// 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.
|
||||
// and no-step_finish as real edges — pin them with examples against the shipped jq,
|
||||
// all in one batched spawn.
|
||||
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 } } },
|
||||
test('reports reason/tokens from the LAST step_finish and degrades missing fields to "?"', opts, () => {
|
||||
const cases = [
|
||||
{ events: [
|
||||
{ type: 'step_finish', part: { reason: 'tool_calls', tokens: { output: 5 } } },
|
||||
{ type: 'tool_use', part: {} },
|
||||
{ type: 'step_finish', part: { reason: 'stop', tokens: { output: 0 } } },
|
||||
], expect: 'stop reason=stop, output tokens=0' }, // LAST step_finish wins
|
||||
{ events: [{ type: 'step_finish', part: { reason: 'stop', tokens: {} } }], expect: 'stop reason=stop, output tokens=?' }, // missing .tokens.output → "?"
|
||||
{ events: [{ type: 'tool_use', part: { tool: 'read' } }], expect: 'stop reason=?, output tokens=?' }, // no step_finish at all → both "?"
|
||||
];
|
||||
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=?');
|
||||
const actual = runJqBatch(DIAG_PROGRAM, cases.map((c) => c.events));
|
||||
cases.forEach((c, i) => assert.equal(actual[i], c.expect, `diag ${i}: ${JSON.stringify(c.events)}`));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -158,7 +215,7 @@ describe('#1936 OpenCode review reconstruction — jq properties', () => {
|
||||
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' });
|
||||
execFileSync('jq', ['-rs', TEXT_PROGRAM], { ...JQ_EXEC_OPTS, input: 'auth token expired\n' });
|
||||
} catch {
|
||||
threw = true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user