chore: runtime tests in CI + override two vulnerable dev transitives - #64
Merged
Conversation
Runtime tests. CI was structural only - it greps built files and never executes a tool call - so the connection-layer and protocol work merged in #63 had no automated coverage. Two suites now run in the MCP job: test-protocol.js drives the real server over stdio and SSE: cold tools/list returns fallback schemas, a fake extension registers over WebSocket, exactly one well-formed tools/list_changed arrives on both transports, re-list returns the extension's schemas, an unchanged re-register stays silent, and a disconnect notifies again. test-connection.js loads the real background.js under stubbed extension globals (simulating Firefox MV2, the heartbeat path) and drives it against the real server: a replaced socket's close event must not clear the live connection's heartbeat, inflate the attempt counter, or arm a reconnect; a failed connect must increment attempts once, not twice; and the reconnect must be one-shot rather than fixed-rate. Both were validated against the pre-fix code at aefab6a, where the connection suite fails 5 of its assertions - so these detect the regressions they describe rather than passing vacuously. They live in opendia-mcp because they need the server and the ws client, which are that package's dependencies; test-connection.js reads the extension source by path and does not import from that package. Neither ships to npm - the package's files list is server.js and README.md. npm test is now these suites plus the syntax check, replacing a stub that always exited 1. Dependency overrides. Two high-severity Dependabot alerts, both development scope and both reached only through web-ext: brace-expansion 1.1.15 -> 1.1.17 (via multimatch -> minimatch), GHSA-3jxr-9vmj-r5cp shell-quote 1.8.4 -> 1.10.0 (via fx-runner), GHSA-395f-4hp3-45gv Both are denial-of-service by input complexity in build tooling. Nothing reaches a user: web-ext is a devDependency and node_modules is not part of the extension bundle. Overrides because web-ext pins the vulnerable ranges transitively. web-ext lint still reports 0 errors and the same 3 pre-existing warnings.
…ilure CI failed with 'server never came up on 45554 (port busy?)' while both suites passed locally on Node 20 and 26. The harness could not say why: it swallowed the server's stderr, polled only the port it had requested, and never noticed if the process had exited. Both suites now share test-helpers.js, which waits for the server's own 'HTTP/SSE server running' banner, parses the actual ports out of 'Ports resolved: WebSocket=..., HTTP=...', detects an early exit, and includes the captured stderr in the thrown error. The server shifts to the next free port on a conflict, so assuming the requested port was a latent bug in the test rather than a property of the server. Re-verified against the pre-fix code at aefab6a: the connection suite still fails 5 assertions there, so the refactor did not weaken detection.
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.
Follow-up to #63.
Runtime tests
CI was structural only — it greps built files and never executes a tool call — so everything merged in #63 had no automated coverage. Two suites now run in the MCP job:
test-protocol.jsdrives the real server over stdio and SSE: coldtools/listreturns fallback schemas → a fake extension registers over WebSocket → exactly one well-formedtools/list_changedarrives on both transports → re-list returns the extension's schemas → an unchanged re-register stays silent → disconnect notifies again.test-connection.jsloads the realbackground.jsunder stubbed extension globals (simulating Firefox MV2, the heartbeat path) and drives it against the real server: a replaced socket's close event must not clear the live connection's heartbeat, inflate the attempt counter, or arm a reconnect; a failed connect must increment attempts once, not twice; the reconnect must be one-shot rather than fixed-rate.These were validated against the pre-fix code at
aefab6a, where the connection suite fails 5 of its assertions. So they detect the regressions they describe rather than passing vacuously — which was the whole problem with the structure tests before #63.They live in
opendia-mcpbecause they need the server and thewsclient, which are that package's dependencies;test-connection.jsreads the extension source by path and does not import from that package. Neither ships to npm (the package'sfileslist isserver.js+README.md).npm testis now these suites plus the syntax check, replacing a stub that always exited 1.Dependency overrides
Two high-severity Dependabot alerts, both development scope, both reached only through
web-ext:brace-expansion(viamultimatch→minimatch)shell-quote(viafx-runner)Both are denial-of-service by input complexity in build tooling. Nothing reaches a user:
web-extis a devDependency andnode_modulesis not part of the extension bundle. Overrides rather than a plain bump becauseweb-extpins the vulnerable ranges transitively.Verification
npm ciin sync, extension build + validate + structure tests pass,web-ext lintstill 0 errors and the same 3 pre-existing warnings, and both new suites pass.