Repository navigation
test: keep CI tests off the npm registry and close code scanning alerts - #719
Merged
Merged
Conversation
- sanitize-markdown: treat data: and vbscript: links as dangerous, not only javascript: (js/incomplete-url-scheme-check) - prompt: build the custom read override output inside the generated tool instead of splicing a JSON-stringified value into code (js/bad-code-sanitization) - MCP HTTP fixture: log the handler error and return a generic 500 body instead of the error text (js/stack-trace-exposure)
…d asks Close the remaining gaps in the 1.0.64 changes, each checked against a mutation of the guarded code: - core runner and opencode host stop admitting organizer calls once reported usage fills the per-session reservation ceiling - core runner skips reasoning too large for one organizer call without a model call - opencode host aborts an interrupted organizer call and settles it before the next turn (the fixture keeps the response alive, so only the abort can end it) - MCP tools: the forced dialog shows the hook-rewritten arguments, approval runs them once without a second dialog, rejection never calls the server
Every .opencode directory installs @opencode-ai/plugin in the background and custom tools wait for that install. Under bun the bundled SDK next to the executable is absent, so each install ran arborist against the public npm registry (about 1.8s per test in CI). The nine prompt.test.ts context-folding tests that load .opencode/tools timed out together in the post-merge run of #717 and in earlier PR runs, while the interleaved tests without custom tools passed. Point OPENCODE_PLUGIN_SDK_PATH at a local stub in the test preload, as the preload already does for the global config directory. A full local run made no registry installs for test directories (33 in the probed CI run before) and the folding tests drop from about 25s to 10s.
…pt test Removes the two lint warnings the new tests added (4796, as on main).
LeXwDeX
marked this pull request as ready for review
October 8, 2026 10:49
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c7dc337d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The protocol check covered data: and vbscript:, but the fixture only produced a javascript: link, so those cases could not fail. Add one link for each; allowing either scheme in DOMPurify now fails the test.
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
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.
Issue for this PR
Closes #718
Why
Unit Tests (linux)failed onmainafter #717 merged, which blocks the 1.0.64 release. The same nineprompt.test.tscontext-folding cases had timed out in earlier PR runs of #713 and #717 and passed in others on identical trees. All nine load a custom tool from.opencode/tools; the folding tests interleaved with them that load no custom tool passed in the same runs.Custom tools wait for the background
@opencode-ai/plugininstall of their.opencodedirectory. Under bun the bundled SDK next to the executable is absent, so that install ran arborist against the public npm registry. Probes in two dispatched CI runs measured about 1.8 s per folding test and 33 registry installs for test directories per run. The rest of the suite already avoids this by mocking the wait or pre-seeding the SDK, and the preload already seeds the global config directory.The same work closes the three open code scanning alerts (all in test code) and the coverage gaps found in the 1.0.64 changes.
Type of change
What changed
OPENCODE_PLUGIN_SDK_PATHpoints at a local SDK stub, so no test install of the plugin SDK reaches npm. Tests that need the real SDK still place it in their ownnode_modules, which the install uses first.data:andvbscript:links (docs: trim README to English + Simplified Chinese only #83); the custom read override builds its output inside the generated tool instead of splicing a stringified value into code (Merge pull request #81 from LeXwDeX/release/sync-main-into-dev #82); the MCP HTTP fixture returns a generic 500 body and logs the error (sync: fast-forward dev to main (reconverge after #80) #81).What does this PR do?
Test and fixture changes only; no runtime source changes. It removes the npm registry from the unit test path, resolves the open code scanning alerts and adds the missing regression tests.
Evidence
Local, on this head:
bun run lint: 4796 warnings (main: 4796; cap 4850)bun run typecheck: 31/31bun run test:dag-core: all critical behavior and coverage floors passedscript/release-notes.tsforgraphagent-v1.0.64How did you verify your code works?
Each new test was checked against a mutation of the code it guards: disabling the session ceiling (core runner and opencode host), removing the interrupt abort (opencode host; the fixture keeps the response alive so only the abort can end it), not passing the forced-ask confirmation and showing the original input (MCP). Every mutation made the corresponding test fail; the source was restored unchanged. The probe-only diagnostic branch
test/folding-timeout-probeis not part of this PR.Screenshots / recordings
Not applicable: no UI change.
Checklist
🤖 Generated with Claude Code