Repository navigation
Add the 0002-static-first-invocation package codemod - #1056
Conversation
Registers the narrow-phase migration for the static-first two-rule
model in the new codemod framework: mechanical AST rewrite of
packages.invokeChecked member calls to packages.invoke (identical call
shape; invoke is always contract-checked and key-less calls run the
lean path), with conservative needsManual findings for packages.check
and literal dynamic import("kody:@...") usage. Detection reuses the
deprecated-invocation-usage collector so codemod findings and the
publish-check deprecation warnings stay in lockstep. Its fleet-wide
scan is the phase-3 gate for #1048.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds codemod ChangesStatic-first invocation migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PackageFiles
participant CodemodRegistry
participant StaticFirstInvocationCodemod
participant ASTParser
PackageFiles->>CodemodRegistry: select registered codemod
CodemodRegistry->>StaticFirstInvocationCodemod: detect deprecated usage
StaticFirstInvocationCodemod->>ASTParser: parse code files
ASTParser-->>StaticFirstInvocationCodemod: member-expression ranges
StaticFirstInvocationCodemod->>PackageFiles: rewrite invokeChecked to invoke
StaticFirstInvocationCodemod-->>PackageFiles: return changed paths and needsManual findings
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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 using default effort 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 08db34b. Configure here.
|
🔎 Preview deployed: https://kody-pr-1056.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/worker/src/package-codemods/codemods/0002-static-first-invocation.ts (2)
149-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLoop is a filter+map.
- for (const usage of usages) { - // `packages.check` return values and dynamic-import namespaces have no - // mechanical one-to-one rewrite; report them instead of guessing. - if (usage.kind !== 'packages.invokeChecked') { - needsManual.push({ - path: usage.filePath, - message: findingMessageForUsage(usage), - }) - } - } + // `packages.check` return values and dynamic-import namespaces have no + // mechanical one-to-one rewrite; report them instead of guessing. + for (const usage of usages.filter( + (candidate) => candidate.kind !== 'packages.invokeChecked', + )) { + needsManual.push({ + path: usage.filePath, + message: findingMessageForUsage(usage), + }) + }🤖 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-codemods/codemods/0002-static-first-invocation.ts` around lines 149 - 158, Update the loop over usages to use a filter-and-map style transformation: retain only usages whose kind is not packages.invokeChecked, then produce needsManual entries with each usage’s filePath and findingMessageForUsage(usage). Preserve the existing exclusion and output fields.
212-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
satisfies PackageCodemodfor consistency with codemod 0001.
ambientStorageToPackageStorageCodemodusessatisfies, which retains the literalidtype; the annotation here widens it tostring.🤖 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-codemods/codemods/0002-static-first-invocation.ts` around lines 212 - 218, Update the staticFirstInvocationCodemod declaration to use the satisfies operator with PackageCodemod instead of a direct type annotation, preserving the literal type of staticFirstInvocationCodemodId while retaining the existing detect and transform implementations.
🤖 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/package-codemods/codemods/0002-static-first-invocation.node.test.ts`:
- Around line 83-124: The test must cover malformed source containing
packages.invokeChecked and assert that detection and transformation report a
parse-failure needsManual finding without changing the file. Update
collectDeprecatedInvocationUsage so parse errors are propagated or represented
as findings instead of being swallowed as no usage, allowing the rewrite flow to
emit the required parse-failure message.
In
`@packages/worker/src/package-codemods/codemods/0002-static-first-invocation.ts`:
- Around line 160-192: Update the post-rewrite verification loop in
0002-static-first-invocation.ts to iterate over invokeCheckedPaths, while
avoiding duplicate needsManual entries for paths already recorded with a
parse-failure finding; preserve the existing scannable-file, declaration-file,
and remaining-usage checks. In docs/contributing/package-codemods.md lines
140-142, retain the broad verification guarantee now that all candidates are
covered.
---
Nitpick comments:
In
`@packages/worker/src/package-codemods/codemods/0002-static-first-invocation.ts`:
- Around line 149-158: Update the loop over usages to use a filter-and-map style
transformation: retain only usages whose kind is not packages.invokeChecked,
then produce needsManual entries with each usage’s filePath and
findingMessageForUsage(usage). Preserve the existing exclusion and output
fields.
- Around line 212-218: Update the staticFirstInvocationCodemod declaration to
use the satisfies operator with PackageCodemod instead of a direct type
annotation, preserving the literal type of staticFirstInvocationCodemodId while
retaining the existing detect and transform implementations.
🪄 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: 47873b17-acee-43c1-85c3-c5d4d74a3891
📒 Files selected for processing (4)
docs/contributing/package-codemods.mdpackages/worker/src/package-codemods/codemods/0002-static-first-invocation.node.test.tspackages/worker/src/package-codemods/codemods/0002-static-first-invocation.tspackages/worker/src/package-codemods/registry.ts
| test('0002 reports packages.check and literal dynamic kody imports as needsManual', () => { | ||
| const files = { | ||
| 'package.json': manifest(), | ||
| 'index.ts': [ | ||
| "import { packages } from 'kody:runtime'", | ||
| '', | ||
| 'export default async function run() {', | ||
| "\tconst contract = await packages.check({ kodyId: 'github', exportName: './request' })", | ||
| "\tconst dynamicModule = await import('kody:@kentcdodds/github/request')", | ||
| '\treturn { contract, dynamicType: typeof dynamicModule.default }', | ||
| '}', | ||
| '', | ||
| ].join('\n'), | ||
| } | ||
|
|
||
| const findings = staticFirstInvocationCodemod.detect(files) | ||
| expect(findings).toEqual([ | ||
| { | ||
| path: 'index.ts', | ||
| message: expect.stringContaining('literal dynamic `import("kody:@...")`'), | ||
| }, | ||
| { | ||
| path: 'index.ts', | ||
| message: expect.stringContaining('`packages.check`'), | ||
| }, | ||
| ]) | ||
|
|
||
| const result = staticFirstInvocationCodemod.transform(files) | ||
| expect(result.changed).toBe(false) | ||
| expect(result.changedPaths).toEqual([]) | ||
| expect(result.files).toEqual(files) | ||
| expect(result.needsManual).toEqual([ | ||
| { | ||
| path: 'index.ts', | ||
| message: expect.stringContaining('literal dynamic `import("kody:@...")`'), | ||
| }, | ||
| { | ||
| path: 'index.ts', | ||
| message: expect.stringContaining('`packages.check`'), | ||
| }, | ||
| ]) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cover—and fix—the required parse-failure manual finding.
The PR requires parse failures to yield needsManual, but this suite omits that case. More importantly, the supplied implementation catches parse errors in collectDeprecatedInvocationUsage() and returns no usage, so malformed files never enter the rewrite loop or receive the parse-failure finding. Add a malformed packages.invokeChecked fixture expecting needsManual; it will expose the required implementation fix.
🤖 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-codemods/codemods/0002-static-first-invocation.node.test.ts`
around lines 83 - 124, The test must cover malformed source containing
packages.invokeChecked and assert that detection and transformation report a
parse-failure needsManual finding without changing the file. Update
collectDeprecatedInvocationUsage so parse errors are propagated or represented
as findings instead of being swallowed as no usage, allowing the rewrite flow to
emit the required parse-failure message.
| for (const path of [...invokeCheckedPaths].sort((left, right) => | ||
| left.localeCompare(right), | ||
| )) { | ||
| const source = nextFiles[path] | ||
| if (typeof source !== 'string') continue | ||
| const rewritten = rewriteInvokeChecked(source) | ||
| if (rewritten == null) { | ||
| needsManual.push({ path, message: manualParseFailureMessage }) | ||
| continue | ||
| } | ||
| if (rewritten !== source) { | ||
| nextFiles[path] = rewritten | ||
| changedPaths.push(path) | ||
| } | ||
| } | ||
|
|
||
| // Defensive verification: the rewrite must fully clear detectable | ||
| // `packages.invokeChecked` member expressions, or the file needs a human. | ||
| for (const path of changedPaths) { | ||
| const source = nextFiles[path] | ||
| if (!scannableModuleFilePattern.test(path)) continue | ||
| if (isTypeDeclarationFilePath(path)) continue | ||
| const remaining = collectDeprecatedInvocationUsage({ | ||
| [path]: source ?? '', | ||
| }).filter((usage) => usage.kind === 'packages.invokeChecked') | ||
| if (remaining.length > 0) { | ||
| needsManual.push({ | ||
| path, | ||
| message: | ||
| '`packages.invokeChecked` member expressions remain after the rewrite; migrate manually.', | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Post-rewrite verification does not cover every invokeChecked candidate, so the documented guarantee overstates behavior. Verification iterates changedPaths only; a candidate file whose AST scan yields no ranges is neither rewritten nor flagged, leaving deprecated usage unreported.
packages/worker/src/package-codemods/codemods/0002-static-first-invocation.ts#L160-L192: run the verification loop overinvokeCheckedPathsrather thanchangedPaths, de-duplicating against the parse-failure finding already pushed for that path.docs/contributing/package-codemods.md#L140-L142: once the loop covers all candidates the wording holds; otherwise narrow it to say verification applies only to files the codemod actually rewrote.
📍 Affects 2 files
packages/worker/src/package-codemods/codemods/0002-static-first-invocation.ts#L160-L192(this comment)docs/contributing/package-codemods.md#L140-L142
🤖 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-codemods/codemods/0002-static-first-invocation.ts`
around lines 160 - 192, Update the post-rewrite verification loop in
0002-static-first-invocation.ts to iterate over invokeCheckedPaths, while
avoiding duplicate needsManual entries for paths already recorded with a
parse-failure finding; preserve the existing scannable-file, declaration-file,
and remaining-usage checks. In docs/contributing/package-codemods.md lines
140-142, retain the broad verification guarantee now that all candidates are
covered.
…ite candidates collectDeprecatedInvocationUsage returns nothing for files it cannot parse, so unparseable files referencing the deprecated surface silently scanned clean. A textual pre-scan now surfaces them as needsManual in both detect and transform, and the post-rewrite verification covers every invokeChecked candidate rather than only changed files. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
3 participants
Summary
Registers the static-first migration in the new package-codemod framework (#1049) as
0002-static-first-invocation— the narrow-phase enforcement vehicle for #1048.Safety-first design (per Kent's direction: avoid changing semantics)
The only automated transform is the API rename:
packages.invokeChecked(...)member calls (includingpackages?.invokeChecked) →packages.invoke(...)via AST range replacement. Semantics-preserving by construction: since feat(invoke): key-less packages.invoke lean path + widen-phase deprecation shims #1046, the runtime aliasesinvokeCheckedtoinvoke(runtime-tool-factories.tsreturnsinvokeChecked: invoke) — same call shape, same mandatory contract check, same key-less behavior. The rewrite changes which name appears in source and nothing else; it future-proofs the package for the narrow phase when the alias is removed.Everything that would change semantics is a conservative
needsManualfinding (per the codemod doctrine), naming the replacement:packages.check(...)— its contract return value has no mechanical equivalent (invokechecks internally); the call site needs human restructuring.import("kody:@...")— namespace semantics andkody.dependenciesmanifest changes need a human; static-import conversion also changes bundling/pinning, realm, and storage binding, so it is never automated.A post-rewrite verification confirms no detectable
invokeCheckedmember expressions remain. Detection reusespackage-runtime/deprecated-invocation-usage.ts(the publish-check collector from #1046), so codemoddetectfindings and the non-fatal publish lint warnings stay in lockstep by construction. Documented indocs/contributing/package-codemods.mdalongside0001.Role in the program
Kent's own 10 affected packages were already migrated manually (pre-framework); this codemod's fleet-wide
scanis the proof mechanism for the #1048 gate ("legacy surface absent from all production packages") and the migration path for community forks and post-launch users before the shims are removed.Testing
invokeCheckedidentifiers and non-code files untouched;packages.check/ dynamic-importneedsManualfindings; mixed mechanical+manual package.package-codemodssuite green (14 tests), typecheck green, format/lint clean.Program report
scanviaadmin_package_codemod_scanand attach the zero-usage report to Narrow phase: eliminate the deprecated dynamic invocation surface (invokeChecked, packages.check, dynamic kody:@ imports) #1048.Summary by CodeRabbit
New Features
packages.invokeChecked(...)calls topackages.invoke(...), including optional chaining.Documentation