From 71f36d5720efc296d166dd4ac896b14204b1196d Mon Sep 17 00:00:00 2001 From: Raghav Chari Date: Wed, 1 Jul 2026 12:22:18 -0400 Subject: [PATCH 1/2] test(#25): assert the real OPENCODE_CONFIG_CONTENT injection + merge (close boot_smoke false-green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit boot_smoke.mjs is the only server-booting test and boots WITHOUT OPENCODE_CONFIG_CONTENT, so it stayed green even if the instructions/permission injection were removed entirely (false-green surfaced in #12 review). The real fix belongs where the REAL buildOpencodeConfigContent can be imported (a .mjs can't import the TS builder without re-deriving it → drift). So harden the existing real-binary test in opencode_config.test.ts: it already runs `opencode debug config` (resolves the merged config, ≈ GET /config) with the real builder; now it also - writes a user global config with a distinctive `model` + `permission` key, - asserts the injected `instructions` (AGENTS.md path) is present — the exact regression the false-green missed, - asserts the global `model` survives the deep-merge (Q129 provider selection), - keeps the #22 permission deep-merge assertions (user key survives + ours added). Verified it's a real catcher: dropping `instructions` from the builder reds this test (and the unit test) at the `instructions` assertion. boot_smoke.mjs: documented its scope (binary /event liveness only; the injection + merge coverage lives in opencode_config.test.ts) so the split is explicit. 80 extension tests green; test:smoke PASS. Closes #25. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/extension/test/boot_smoke.mjs | 8 ++++ .../extension/test/opencode_config.test.ts | 39 ++++++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/extension/test/boot_smoke.mjs b/packages/extension/test/boot_smoke.mjs index e295fbbd..d5d461d2 100644 --- a/packages/extension/test/boot_smoke.mjs +++ b/packages/extension/test/boot_smoke.mjs @@ -3,6 +3,14 @@ // GET /event as an SSE stream (HTTP 200, text/event-stream) against a // synthesized project, with no LLM creds. Exit 0 = pass. // +// SCOPE (see #25): this is the binary-liveness gate only — it deliberately does +// NOT set OPENCODE_CONFIG_CONTENT, so it can't (and doesn't claim to) catch a +// regression in the instructions/permission injection or the config merge. That +// injection + merge is asserted against the REAL binary + the REAL +// buildOpencodeConfigContent in test/opencode_config.test.ts ("opencode config +// injection + merge"), which can import the TS builder (this .mjs can't, so +// re-deriving the config here would just risk drift). +// // Boot + probe logic lives in scripts/opencode_probe.mjs (shared with the // healthcheck, which derives BOTH the /event gate and the provider signal from a // single boot); this script asserts the /event gate and exits. diff --git a/packages/extension/test/opencode_config.test.ts b/packages/extension/test/opencode_config.test.ts index 1d572012..9620d58d 100644 --- a/packages/extension/test/opencode_config.test.ts +++ b/packages/extension/test/opencode_config.test.ts @@ -66,25 +66,44 @@ describe('buildOpencodeConfigContent', () => { }) }) -// Integration: confirms opencode 1.17.3 DEEP-merges the injected `permission` -// object over the user's global config rather than shallow-replacing it (which -// would wipe the user's other permission keys). Skipped when the vendored binary -// isn't present (e.g. minimal CI before `fetch:opencode`). +// Integration (#25): boots the REAL opencode binary (`opencode debug config` +// resolves + dumps the merged config, equivalent to GET /config) with the REAL +// buildOpencodeConfigContent as OPENCODE_CONFIG_CONTENT, and asserts the whole +// injection + merge the extension relies on at spawn: +// - the injected `instructions` (the AGENTS.md path) is present — this is the +// regression the old boot_smoke false-green missed: boot_smoke.mjs boots +// WITHOUT OPENCODE_CONFIG_CONTENT, so it stayed green even if the instruction +// injection were removed. This test reds instead. +// - the user's global `model` survives the deep-merge (opencode picks the +// provider from it — the merge must not clobber it); +// - the user's global `permission` keys survive AND our injected permission key +// is added (deep-merge, not shallow-replace — folds in the #22 check). +// Uses the real builder (no transcribed copy → no drift; boot_smoke.mjs can't +// import the TS builder, which is why this lives here). Skipped when the vendored +// binary isn't present (e.g. minimal CI before `fetch:opencode`). const OC_BIN = join(__dirname, '..', 'vendor', 'opencode', `${process.platform}-${process.arch}`, 'opencode') -describe.skipIf(!existsSync(OC_BIN))('opencode permission merge (1.17.3)', () => { - it('injected permission ADDS keys — the user\'s global permission keys survive', () => { +describe.skipIf(!existsSync(OC_BIN))('opencode config injection + merge (1.17.3)', () => { + it('injects instructions/permission AND preserves the user global model + permission', () => { const home = mkdtempSync(join(tmpdir(), 'ochome-')) mkdirSync(join(home, '.config', 'opencode'), { recursive: true }) + // A user global config with a distinctive model + permission key — both must + // survive the deep-merge under OPENCODE_CONFIG_CONTENT. writeFileSync(join(home, '.config', 'opencode', 'opencode.json'), - JSON.stringify({ permission: { doom_loop: 'deny' } })) // a distinctive user-set key + JSON.stringify({ model: 'anthropic/claude-sonnet-4-6', permission: { doom_loop: 'deny' } })) + const agentsPath = join(home, 'AGENTS.md') // the exact file our `instructions` must point at + writeFileSync(agentsPath, '# amico\n') const out = execFileSync(OC_BIN, ['debug', 'config'], { encoding: 'utf8', env: { ...process.env, HOME: home, XDG_CONFIG_HOME: join(home, '.config'), - OPENCODE_CONFIG_CONTENT: buildOpencodeConfigContent('/abs/AGENTS.md', '/ext/templates/solve_template.jl') }, + OPENCODE_CONFIG_CONTENT: buildOpencodeConfigContent(agentsPath, '/ext/templates/solve_template.jl') }, }) const cfg = JSON.parse(out) - expect(cfg.permission.doom_loop).toBe('deny') // user's key SURVIVED the deep-merge - expect(typeof cfg.permission.external_directory).toBe('object') // our injected key is present too + // our injection landed (the false-green boot_smoke couldn't catch): + expect(cfg.instructions).toContain(agentsPath) // the AGENTS.md instruction injection + expect(typeof cfg.permission.external_directory).toBe('object') // our injected permission key + // the user's global config SURVIVED the deep-merge: + expect(cfg.model).toBe('anthropic/claude-sonnet-4-6') // provider/model preserved (Q129 needs this) + expect(cfg.permission.doom_loop).toBe('deny') // user permission key preserved (#22) }) }) From e89a7e53052386bd0170684cc362438af0130d86 Mon Sep 17 00:00:00 2001 From: Raghav Chari Date: Wed, 1 Jul 2026 12:24:31 -0400 Subject: [PATCH 2/2] ci(#25): fetch opencode in the fast job so the inject/merge test runs (not skips) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The opencode config injection+merge integration test is skipIf(!OC_BIN), and the fast job never fetched the vendored binary before vitest — so it self-skipped in CI. That was the false-green ONE level up: even with the test added, a real injection/merge regression would pass CI because the test didn't run. Fetch opencode before `pnpm -r run test` so it executes and gates. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index daa8826c..e7fd4465 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,12 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm -r run build - run: pnpm -r run typecheck - - run: pnpm -r run test # amico-run suite (incl. S31 grep rule) + extension unit suite + @amicode/schema conformance + # Fetch the vendored opencode BEFORE vitest so the OC_BIN-gated integration + # test (opencode config injection + merge) actually RUNS in CI instead of + # self-skipping — the skip was the #25 CI-level false-green (an injection or + # config-merge regression would pass CI because the only test for it skipped). + - run: pnpm --filter amicode-v2 fetch:opencode + - run: pnpm -r run test # amico-run suite (incl. S31 grep rule) + extension unit suite (incl. the opencode inject/merge integration) + @amicode/schema conformance - name: amico-validate — shipped configs conform + linked bin works (0.1c gate) run: | # Exercise the LINKED bin via a dependent (the bin links into amico-run /