fix(webmcp): repair post-merge registration and fallback contracts - #52
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 189468c77b
ℹ️ 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".
| test('committed fallback snapshot is exact, usable on a clean checkout, and retains source provenance', async () => { | ||
| const snapshot = await readFallback(); | ||
| assert.equal(snapshot.source.repository, 'The-Interdependency/skill-lib'); | ||
| assert.equal(snapshot.source.commit, '260671303733a45c8f8d5563e41d8854e09856e6'); |
There was a problem hiding this comment.
In the pull-request and Pages workflows, npm run check runs the build before npm test; the build's prevalidate hook runs refresh:data, and a successful refresh:skills overwrites this snapshot with the current skill-lib HEAD. As soon as that HEAD differs from 260671..., readFallback() correctly returns the refreshed commit but this assertion fails, blocking checks and deployment despite the documented contract that online refreshes replace the bootstrap with the newest verified snapshot. Test the bootstrap through an isolated fixture or allow valid refreshed provenance here.
Useful? React with 👍 / 👎.
| const actualBlob = gitBlobSha1(text); | ||
| if (actualBlob !== BOOTSTRAP_SNAPSHOT_BLOB) { |
There was a problem hiding this comment.
On a clean checkout with core.autocrlf=true, such as a typical Windows checkout, Git can rewrite this JSON snapshot from LF to CRLF because the repository has no .gitattributes rule fixing its line endings. gitBlobSha1(text) then hashes the converted working-tree bytes, so this comparison rejects the otherwise exact committed bootstrap and OFFLINE=1 fails in the clean-checkout scenario this change promises to support. Enforce LF or binary treatment for the snapshot, or verify canonicalized bytes.
Useful? React with 👍 / 👎.
| if (globalThis.__interdependencyWebMcpRegistered) { | ||
| return { registered: true, reused: true, registry: status }; |
There was a problem hiding this comment.
When this exported function is invoked again after the registry asset has changed, it fetches the new projection and calls updateSource(status) before reaching this reuse branch, but the already-registered tools are not replaced and their execute closures still reference the first registry. The page and returned registry therefore advertise the new commit while tool calls continue serving the old one; a failed refetch can also reject this reuse path even though the tools remain registered. Check the registration marker before reloading, or retain and return the status associated with the original registrations.
Useful? React with 👍 / 👎.
1 participant
Post-merge repair for #51
Codex review landed after #51 merged and identified one P1 plus two P2 defects in the WebMCP surface. This PR repairs only those defects on current
main@653e832a2dcf79bb24a1feb180c0d7acef22bdb2.Repairs
navigator.modelContextrather thandocument.modelContext;The-Interdependency/skill-lib@260671303733a45c8f8d5563e41d8854e09856e6:skills.json(Git blob7f71adeadac07a751b953c39e38dd78be599976f);Scope
Four files only: provider registration, registry collector, one committed bootstrap snapshot, and its tests. No write-capable WebMCP tool or authority transfer is introduced.
Validation
Repository CI is authoritative. The focused tests are part of
npm test; the full Pull Request workflow must pass before merge.hmmm
WebMCP remains browser/platform-dependent. Browsers without the API intentionally expose the truthful human-readable registry status without tool registration.