Repository navigation
Add active contract checks for dynamic package invocation - #435
Conversation
📝 WalkthroughWalkthroughThe PR extends package runtime invocations with a pre-invocation checking capability. ChangesPre-invocation checking for package invocations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
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-435.kentcdodds.workers.dev Worker: Mocks:
|
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
4ff9c2b to
841211c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/use/packages.md`:
- Around line 158-164: The docs paragraph implies a top-level warnings field but
in code warnings live under the returned contract; update the text to explicitly
state that package checks return warnings on check.contract.warnings (e.g.,
"warnings are available on check.contract.warnings") and adjust the surrounding
sentence so consumers don't expect a top-level warnings property when using
packages.check or destructuring the result.
In `@packages/worker/src/package-invocations/service.ts`:
- Around line 389-404: The preflight validation in checkPackageInvokeForRuntime
hard-codes the caller operation name to "packages.check" when calling
parsePackageInvokeInput, which causes validation errors to be labeled
incorrectly for invokeChecked callers; change the function signature of
checkPackageInvokeForRuntime to accept an operationName string (e.g.,
"packages.invokeChecked" or the caller-provided name) and pass that
operationName into parsePackageInvokeInput instead of the hard-coded
"packages.check"; also update the other parsePackageInvokeInput call-site(s) in
this file (the similar block around the later invocation) to thread the
appropriate operation name through so validation errors retain the correct
operation label.
🪄 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: c6564635-356b-44a9-9cfd-655ed0d5f492
📒 Files selected for processing (12)
docs/contributing/packages-and-manifests.mddocs/use/execute.mddocs/use/packages.mdpackages/worker/src/mcp/run-codemode-registry.node.test.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/mcp/server-instructions.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-invocations/service.tspackages/worker/src/package-runtime/module-graph.workers.test.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/repo/checks.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
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 ebc556b. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |

Summary
mainpackages.checkandpackages.invokeCheckedruntime helpers pluscodemode.package_invoke_check/codemode.package_invoke_checkedpackages.invoke, and checked dynamic invocationcheck.contract.warnings, and preservinginvokeCheckedoperation labels in preflight validation errorsTesting
npx vitest run packages/worker/src/package-invocations/service.node.test.ts(20 passed)npx vitest run packages/worker/src/package-invocations/service.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts packages/worker/src/package-runtime/module-graph.workers.test.ts packages/worker/src/repo/checks.node.test.ts(67 passed)Summary by CodeRabbit
New Features
packages.check()method to validate package contracts before invocationpackages.invokeChecked()method for safer dynamic package calls with built-in validationDocumentation
Tests