fix(spec): resolve multi-line and .js imports in skill-reference resolver (#3139)#3155
Merged
os-zhuang merged 1 commit intoJul 18, 2026
Merged
Conversation
…lver (#3139) `extractLocalImports` in build-skill-references.ts under-reported a skill's transitive schema deps because of two blind spots in the import scan: - The regex `/^import\s+.*\s+from\s+.../` used `.`, which does not cross newlines, so any multi-line named import (`import {\n … \n} from './x'`) was silently dropped and never queued into the closure. - `.js`-suffixed ESM specifiers were mangled: the resolver appended `.ts` unconditionally, turning `./types.js` into a non-existent `./types.js.ts` that then failed the `fs.existsSync` check and was discarded. Replace the pattern with a multi-line-tolerant scan that excludes `;`, quotes, and `(` between `import` and `from` — this keeps the non-greedy span from bridging across statement boundaries, side-effect imports, or a dynamic `import(...)`. Normalize `.js` specifiers to `.ts` before resolving. Regenerated the indexes with the fix in place: the platform skill now surfaces `system/tenant.zod.ts`, reached through the previously-invisible `.js` import in `kernel/context.zod.ts`. The `check:skill-refs` gate added in #3138 passes with the regenerated output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YTjm38Bo2xUv1UUPWu4mC5
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 102 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 18, 2026 02:28
os-zhuang
deleted the
claude/build-skill-references-multiline-imports-y4xv6o
branch
July 18, 2026 02:28
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
2 participants
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.
Fixes #3139.
Problem
extractLocalImportsinpackages/spec/scripts/build-skill-references.tsbuilds each skill's transitive Zod-schema closure by scanningimport … fromstatements. It had two blind spots that silently under-reported dependencies:/^import\s+.*\s+from\s+…/gmuses., which does not match newlines, so a named import that wraps across lines was never seen:.jsESM specifiers — the resolver appended.tsunconditionally, so./types.jsbecame a non-existent./types.js.ts, failedfs.existsSync, and was dropped.Both failure modes drop the dep with no warning — the file simply never enters the queue, and
skills/*/references/_index.md(shipped to third parties vianpx skills add) ends up advertising an incomplete schema graph.Fix
/^import\b[^;'"()]*?\bfrom\s*['"](\.[^'"]+)['"]/gm. Excluding;, quotes, and(betweenimportandfromkeeps the non-greedy span from bridging across a statement boundary, a side-effect import (import './x'), or a dynamicimport(...)into the wrong specifier..jsspecifiers to.tsbefore resolving (./types.js→./types.ts), instead of blindly appending.ts.The regex was validated against single-line, multi-line,
type-only, namespace, default+named,.js, side-effect, dynamic-import, commented-out, and bare-module ('zod') cases — only the genuine relative imports are captured.Regenerated output
Ran
gen:skill-refswith the fix in place. Exactly one new dependency surfaces across all skills: the platform skill now listssystem/tenant.zod.ts, reached through the previously-invisible.jsimport inkernel/context.zod.ts(import { TenantQuotaSchema } from '../system/tenant.zod.js') — confirming real reachability from aSKILL_MAPclosure, which the issue flagged as worth checking. The other multi-line imports the issue enumerated are real but not reachable from anySKILL_MAPentry, so they produce no drift.The
check:skill-refsgate added in #3138 passes (8 generated files in sync with packages/spec).Notes
Per AGENTS.md, no changeset is included — this is a pure bug fix (the sibling #3138 in the same area likewise carried none).
🤖 Generated with Claude Code
https://claude.ai/code/session_01YTjm38Bo2xUv1UUPWu4mC5
Generated by Claude Code