fix: self-contained injected functions, aligned catalog normalization, clean drift reports - #9
Merged
Merged
Conversation
…lization
Two runtime bugs in the extension's main-world functions, neither of
which the unit tests could see.
chrome.scripting.executeScript({func}) serializes the function and
loses its execution context, so a module-scope helper is simply not
defined in the page. PR #7 extracted readInputSchema out of
probeWebMcp, which made every injection throw ReferenceError -- and
probeWebMcp's own try/catch turned that into {supported: false} on
every page. Silent, total failure of discovery, with a green test
suite, because importing a function in a test keeps module scope alive.
probeWebMcp, invokeWebMcp, and cancelWebMcp are now self-contained; the
duplication between them is forced by that boundary, and is documented
as such rather than looking like an oversight.
invokeWebMcp's normalization also did not match the one that produces
the value it compares against. The service worker builds
expectedCatalog as normalizeTools(probeWebMcp output) -- which sorts by
name, bounds names to 128 characters, and truncates descriptions to
1000 -- while invokeWebMcp did none of those. Any page whose tools were
not already in alphabetical order would have failed every invocation
with stale_catalog. This predates the merge; it was in the original
invocation path.
test/injection.test.js pins both: it rebuilds each function from source
the way Chrome does, and asserts the two normalization paths agree
across unsorted, over-long, over-cap, and snake_case catalogs. Verified
by reintroducing each bug and confirming the suite goes red.
stableStringify moves from service_worker.js to scanning.js so the
comparison basis is importable by tests instead of private to a module
that cannot be imported outside a browser.
Also fixes the drift watcher capturing Mix dependency-compile output
into the issue body: the checker now writes the report with --output
rather than having CI tee it off stdout.
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.
Followups from #6 and #7. Two of these are real runtime bugs that the test suites could not see.
The injected functions were not self-contained
chrome.scripting.executeScript({func})serializes the function, losing its execution context. A module-scope helper is therefore not defined in the page.#7 extracted
readInputSchemaout ofprobeWebMcp. Injected, that threwReferenceError— andprobeWebMcp's owntry/catchswallowed it into{supported: false, tools: []}. Discovery would have silently reported no WebMCP support on every page, with a fully green test suite, because importing a function in a test keeps module scope alive.Reproduced before fixing:
All three injected functions are now self-contained. The duplication between
probeWebMcpandinvokeWebMcpis forced by that boundary and is documented as such.The two normalizations disagreed
invokeWebMcpcompares its catalog againstexpectedCatalog, which the service worker builds asnormalizeTools(probeWebMcp output).normalizeToolssorts by name, bounds names to 128 characters, and truncates descriptions to 1000.invokeWebMcpdid none of those.Any page whose tools were not already in alphabetical order would have failed every invocation with
stale_catalog. This predates the merge — it was in the original invocation path in #6.A test that can actually see both
test/injection.test.jsrebuilds each function from source the way Chrome does, then asserts the two normalization paths agree across unsorted, over-long-name, over-long-description, over-cap, object-schema, and snake_case catalogs.Verified by reintroducing each bug and confirming the suite goes red — removing the sort fails
out of order; reintroducing a module-scope helper fails 3 tests.Smaller fixes
Compiling ...spliced in ahead of the report. The checker now takes--outputand writes the report directly.stableStringifymoved fromservice_worker.jstoscanning.js, so the comparison basis is importable by tests rather than private to a module that cannot be imported outside a browser.executeToolis documented as unspecified where it is used. It is not in the WebMCP IDL orwebmcp-types; the upstream README still reads "TODO: Spec and describe themodelContext.getTools()andmodelContext.executeTool()APIs", and webmachinelearning/webmcp#51 has been open since 2025-11-03.getTools()was specced in #223; its sibling was not. The existing feature-detect-and-report-unavailable behavior is unchanged and is what §21 of the design spec requires.src/globals.d.tsdeclares Webby's own main-world global, sowebmcp-typesstays the only externally-owned contract asserted.Verification
getToolsorinputSchemain the definitions still fails the check, now in both functions.credo --strictclean, format clean.