Repository navigation
Improve saved package export usability - #383
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds package export surface schemas and import-specifier helper; expands the MCP package_get capability to include ready-to-import export entries; updates module-graph to parse the root package.json and resolve root kody: exports directly; and updates tests and search formatting to use the new helper. ChangesPackage detail + import-specifier + module-graph
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-383.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7e8c173. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/package-runtime/module-graph.node.test.ts (1)
697-785: ⚡ Quick winLGTM — the cycle-resolution regression test is well-constructed.
The test correctly establishes the transitive cycle (
personal-history → journaling → personal-history) with no published artifacts, and would fail if the root-package bypass inensurePackageProxywere absent (becauseresolvePackageExportPathwould throw looking for./state-ensurein journaling's manifest).One optional improvement: add an explicit assertion that
getSavedPackageByNamewas called exactly once (for journaling only) to make the bypass guarantee explicit, rather than leaving it as a side-effect check.🧪 Suggested assertion
}).resolves.toEqual({ ... }) + + // Explicit bypass guarantee: root package must not be resolved through the saved-package DB path. + expect(mockModule.getSavedPackageByName).toHaveBeenCalledTimes(1) + expect(mockModule.getSavedPackageByName).toHaveBeenCalledWith( + {}, + { userId: 'user-1', name: '@kentcdodds/journaling' }, + ) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/package-runtime/module-graph.node.test.ts` around lines 697 - 785, Add an explicit assertion that mockModule.getSavedPackageByName was invoked exactly once for the transitive package to make the bypass guarantee explicit: in the test that calls buildKodyModuleBundle (the test named "buildKodyModuleBundle resolves transitive imports back to the root package source during rebuilds"), after awaiting the resolved result, assert mockModule.getSavedPackageByName was called once with '@kentcdodds/journaling' (referencing the mockModule.getSavedPackageByName mock and the buildKodyModuleBundle call) so the test fails if the root-package bypass is not used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/package-runtime/module-graph.node.test.ts`:
- Around line 697-785: Add an explicit assertion that
mockModule.getSavedPackageByName was invoked exactly once for the transitive
package to make the bypass guarantee explicit: in the test that calls
buildKodyModuleBundle (the test named "buildKodyModuleBundle resolves transitive
imports back to the root package source during rebuilds"), after awaiting the
resolved result, assert mockModule.getSavedPackageByName was called once with
'@kentcdodds/journaling' (referencing the mockModule.getSavedPackageByName mock
and the buildKodyModuleBundle call) so the test fails if the root-package bypass
is not used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 692e9c4e-2190-4862-ba8c-4d23c5c474bb
📒 Files selected for processing (5)
packages/worker/src/mcp/capabilities/packages/get-package.node.test.tspackages/worker/src/mcp/capabilities/packages/get-package.tspackages/worker/src/mcp/capabilities/packages/shared.tspackages/worker/src/package-runtime/module-graph.node.test.tspackages/worker/src/package-runtime/module-graph.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/capabilities/packages/get-package.ts`:
- Around line 51-61: The code calls projection.exports.map(...) which will throw
if projection.exports is null or undefined; change the mapping to use a
null-coalescing fallback like (projection.exports ?? []).map(...) so an empty
array is used when no exports are present, e.g., update the exports assignment
in get-package.ts to exports: (projection.exports ?? []).map(...) while keeping
the existing mapping body that uses buildPackageImportSpecifier and saved.name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db95df8a-dc5f-47de-a2a4-f321b1f99a1d
📒 Files selected for processing (3)
packages/worker/src/mcp/capabilities/packages/get-package.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/package-registry/package-import-specifier.ts
✅ Files skipped from review due to trivial changes (2)
- packages/worker/src/mcp/tools/search-format.ts
- packages/worker/src/package-registry/package-import-specifier.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
package_get, including ready-to-usekody:import specifiers and runtime/type targets.package_getexport details.Validation
npx vitest run "packages/worker/src/package-runtime/module-graph.node.test.ts" "packages/worker/src/mcp/capabilities/packages/get-package.node.test.ts" "packages/worker/src/package-runtime/published-bundle-artifacts.node.test.ts" "packages/worker/src/package-registry/service.node.test.ts"npx vitest run "packages/worker/src/mcp/capabilities/packages/get-package.node.test.ts" "packages/worker/src/mcp/tools/search.node.test.ts"npm run typechecknpm run lintnpm run format:checkReview loop
Operational follow-up after deploy
personal-history,journaling,discord-personal-history,discord-general-chat, anddiscord-gateway.Summary by CodeRabbit
New Features
Improvements
Tests