test: assert the real OPENCODE_CONFIG_CONTENT injection + merge (close boot_smoke false-green) - #55
Merged
Conversation
…(close boot_smoke false-green) 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) <noreply@anthropic.com>
… (not skips) 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) <noreply@anthropic.com>
jack-champagne
approved these changes
Jul 2, 2026
jack-champagne
left a comment
jack-champagne
left a comment
Member
There was a problem hiding this comment.
Reviewed — right fix, and cleaner than re-deriving the config in the .mjs. Closes the false-green at both levels: (1) the injection assertion (instructions land + permission merge) now exists in opencode_config.test.ts against the real buildOpencodeConfigContent (no drift), and (2) CI fetches opencode before the suite so the OC_BIN-gated integration test actually runs instead of self-skipping — the deeper false-green. Folds in the #22 permission-merge check and adds global-model preservation (Q129). debug-config vs live GET /config is a fine call (same resolution path). Verified green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #25.
boot_smoke.mjsis the only server-booting test and it boots withoutOPENCODE_CONFIG_CONTENT, so it stayed green even if the instructions/permission injection were removed entirely — a false-green surfaced in #12 review.Where the fix lives
The real fix must use the real
buildOpencodeConfigContent(a transcribed copy would just drift — the thing #25 warns about). A.mjsboot script can't import the TS builder, so the assertion belongs inopencode_config.test.ts, which already runs the real binary viaopencode debug config(resolves the merged config ≈GET /config) with the real builder.Change
test/opencode_config.test.ts— hardened the real-binary merge test to a full injection+merge assertion. It now writes a user global config with a distinctivemodel+permissionkey and asserts:instructions(the AGENTS.md path) is present — the exact regression the false-green missed;modelsurvives the deep-merge (opencode's provider selection, Q129);permissionkey survives and our injectedexternal_directoryis added (the fix: chat→solve QA — permission-hang blocker + multi-qubit/regime guidance #22 deep-merge check, folded in).test/boot_smoke.mjs— documented its scope: binary/eventliveness only; the injection + merge coverage lives inopencode_config.test.ts.Validation
test:smokePASS.instructionsfrom the builder reds this test at theinstructionsassertion (and the unit test) — proving it now catches the regression the false-green allowed.🤖 Generated with Claude Code