fix(cli): duck-type Commander check for cross-realm module instances#75
Open
pradeepmouli wants to merge 1 commit into
Open
fix(cli): duck-type Commander check for cross-realm module instances#75pradeepmouli wants to merge 1 commit into
pradeepmouli wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
pradeepmouli
force-pushed
the
fix/cli-cross-realm-commander
branch
from
c51d648 to
6f52151
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes @skillit/cli’s program loading for cases where a consumer’s Commander Command instance comes from a different module realm (e.g., separate monorepo node_modules) by switching from instanceof checks to a structural (duck-typed) check. It also updates the bundled CLI documentation skill and bootstrap routing reference to explain the <Command>Options interface pattern, and adds a changeset for the patch release.
Changes:
- Replace
instanceof Commandchecks with a cross-realm-safe duck-type guard inprogram-loader.ts. - Document the
<PascalCommandName>Optionsinterface pattern for routing-tag writeback in bundled docs and bootstrap references. - Add a changeset to publish the fix as a patch for
@skillit/cli.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/cli/src/program-loader.ts | Switches commander program detection to a structural check to support cross-realm module instances. |
| packages/cli/skills/skillit-cli-docs/SKILL.md | Documents the option-interface pattern for routing tags in CLI skills guidance. |
| packages/client/skills/skillit-bootstrap/references/surface-routing.md | Adds more explicit per-command option-interface guidance for routing/enrichment. |
| .changeset/fix-cli-cross-realm-commander.md | Declares a patch release and summarizes the cross-realm commander fix. |
Comment on lines
+29
to
+32
| * @useWhen - Server advertises callHierarchyProvider capability | ||
| * @avoidWhen - Server doesn't support call hierarchy; lsproxy will error | ||
| * @never - NEVER invoke without verifying server supports this capability. Fix: check --help output or use `lsproxy call` to probe the server first | ||
| */ |
Comment on lines
+75
to
+76
| /** @useWhen - <scenario> @avoidWhen - <alt> @never - NEVER … Fix: … */ | ||
| interface CallHierarchyOptions {} |
Comment on lines
+19
to
+27
| /** | ||
| * Duck-type check for a Commander {@link Command}. | ||
| * | ||
| * `instanceof Command` fails when the consumer's `commander` package is a | ||
| * different module instance than the one loaded here (e.g. separate monorepos | ||
| * with separate `node_modules`). Structural checks are cross-realm safe. | ||
| */ | ||
| function isCommanderCommand(value: unknown): value is Command { | ||
| if (typeof value !== 'object' || value === null) return false; |
Comment on lines
+26
to
+33
| function isCommanderCommand(value: unknown): value is Command { | ||
| if (typeof value !== 'object' || value === null) return false; | ||
| const v = value as Record<string, unknown>; | ||
| return ( | ||
| typeof v['name'] === 'function' && | ||
| Array.isArray(v['commands']) && | ||
| typeof v['parseAsync'] === 'function' | ||
| ); |
Replace `instanceof Command` with structural duck-typing on `name`, `commands`, and `parseAsync`. Fixes program-loader when the consumer's commander package is a different module instance (separate monorepos with separate node_modules). Also updates skillit-cli-docs and surface-routing to document the per-command <Command>Options interface pattern for routing tag writeback. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pradeepmouli
force-pushed
the
fix/cli-cross-realm-commander
branch
from
6f52151 to
df9d69e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
instanceof Commandwith structural duck-typing (name,commands,parseAsync) inprogram-loader.tsskillit gen --source cliwhen the consumer'scommanderis a different module instance (e.g. separate monoreponode_modules)skillit-cli-docsSKILL.md to document the<Command>Optionsinterface pattern for routing tag writebacksurface-routing.mdwith per-command interface guidanceTest plan
pnpm testpasses (1132 tests)pnpm run type-checkpassesskillit gen --source cli --program <cross-realm-commander-binary>resolves correctly🤖 Generated with Claude Code