fix: address review threads on the legacy-removal PRs - #850
Conversation
…llability `scanEntryExportsSource` now counts a top-level `module.exports = <expr>` as a default export, so AB4730 no longer rejects a `.cjs` stdio MCP entry that the generated lifecycle shell can run: the shell imports the entry namespace and the bundler exposes `module.exports` as its `default`. `exports.foo` and `module.exports.foo` stay named. The SQLite state driver's generic table check now compares each column's NOT NULL flag from `PRAGMA table_info` against the CREATE TABLE statements, so a journal whose `result_state` is nullable fails at open with the existing typed `corrupt` error instead of on the first NULL row. Docs record both, plus the resource/app contract fixture shape, in en and zh.
🦋 Changeset detectedLatest commit: 0947fef The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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. |
![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: 9fddcf9d26
ℹ️ 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".
commit: |
1 participant
Summary
Address the Codex review threads left on the merged legacy-removal PRs #837, #838, #839, and #840.
AB4730rejected a.cjsstdio MCP entry written asmodule.exports = () => server. The generated lifecycle shell imports the entry as a namespace and readsdefault, which the bundler maps tomodule.exports, so that entry runs.scanEntryExportsSourcenow counts a top-levelmodule.exports = <expr>as the default export.exports.foo,module.exports.foo, and nested assignments stay named.result_statecolumn exists but is nullable opened and failed later on a NULL row. The check now compares each column'sPRAGMA table_infoNOT NULL flag too, and a mismatched table fails at open with the existingcorrupterror.reference/runtime-environment.mdx(en, zh) says pre-current SQLite stores are not read or migrated and how to recover.guide/development/testing.mdx(en, zh) says resource and explicit App fixtures must be{ kind: 'resource' }.guide/authoring/mcp.mdx(en, zh) anddocs/entry-conventions.mdmentionmodule.exportsfactories.'complete'Flight message was an internal dev worker protocol, and no page describes it or cross-version epoch restore. A stale epoch fails withCompiled route worker failed.until the next successful rebuild replaces it, which the release note states.Verification
On this branch merged with
origin/main(7b99b1f2a5, after the Effect RC re-pin):pnpm build: passpnpm typecheck: passpnpm lint: passpnpm test:unit: pass (309 files, 4,446 tests)pnpm exec rstest --config rstest.integration.config.ts mcp state-sqlite state-packaging package-conventions: pass (15 files, 134 tests), including the new "accepts a CommonJS stdio entry whose server factory is module.exports, and runs it", which builds the entry and lists its tools over real stdiopnpm docs:site:build: passReview
Independent review (GPT-5.6, change-risk pass) and the Codex bot both found the same issue in the first commit.
module.exports =inside the sharedscanEntryExportsSourcealso changed bin, script, and rendered-script judgments, and misread a file with its ownmodulebinding. The shared scanner is unchanged now. A separateassignsModuleExportsSourceis consulted only byAB4730, and it returns false when the file declares or importsmodule. Tests cover.mtsand import bindings.{ kind: 'resource' }. The contract checkskindonly, so they now say the fixture must setkind: 'resource'.