feat(kit): add skill loader foundation#366
Conversation
|
Warning Review limit reached
More reviews will be available in 31 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds a complete skill loading subsystem to ChangesSkill Loader Subsystem
Sequence Diagram(s)sequenceDiagram
participant Caller
participant loadSkill
participant createSkillLoadJob
participant loadGithubSkillFiles
participant fetchGithubWithRetry
participant GitHubContentsAPI
participant createSkillDefinition
Caller->>loadSkill: loadSkill({ source: 'github', repo, path })
loadSkill->>createSkillLoadJob: createSkillLoadJob(load)
createSkillLoadJob-->>Caller: SkillLoadJob (with cancel())
Note over createSkillLoadJob,loadGithubSkillFiles: job awaited
createSkillLoadJob->>loadGithubSkillFiles: loadGithubSkillFiles(options, context)
loadGithubSkillFiles->>fetchGithubWithRetry: fetchGithubJson(contentsURL)
fetchGithubWithRetry->>GitHubContentsAPI: fetch(url, { signal })
GitHubContentsAPI-->>fetchGithubWithRetry: 429 → retry with backoff
GitHubContentsAPI-->>fetchGithubWithRetry: 200 OK → directory listing
fetchGithubWithRetry-->>loadGithubSkillFiles: JSON entries
loadGithubSkillFiles->>GitHubContentsAPI: fetchGithubBytes(download_url) per file
GitHubContentsAPI-->>loadGithubSkillFiles: Uint8Array
loadGithubSkillFiles-->>createSkillLoadJob: LoadableSkillFile[]
createSkillLoadJob->>createSkillDefinition: createSkillDefinition(files, options)
createSkillDefinition-->>createSkillLoadJob: SkillLoadResult
createSkillLoadJob-->>Caller: SkillDefinition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 build completed successfully!
|
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@fa9522c pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@fa9522c pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@fa9522c commit: fa9522c |
@coderabbitai review now |
|
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/kit/src/skills/test/loaderDefinition.test.ts (1)
124-223: ⚡ Quick winAdd a regression test for non-canonical
entryFileoption values.Line 124 onward covers entry-file error cases well, but there’s no assertion for
entryFileinputs like./SKILL.md(or backslash style) resolving correctly after path normalization. A focused case here would prevent regressions.🤖 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/kit/src/skills/test/loaderDefinition.test.ts` around lines 124 - 223, Add a new test case after the existing entry file error tests (after the "throws when the entry file has no instructions" test) that verifies the createSkillDefinition function correctly handles non-canonical entryFile option values such as ./SKILL.md or paths with backslashes. The test should pass these non-canonical paths in the options object and assert that they resolve correctly to the same SKILL.md file, confirming that path normalization works as expected and prevents regressions when users provide paths with different separators or relative path prefixes.
🤖 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/kit/scripts/download-skill-fixtures.mjs`:
- Around line 32-59: The fetchJson and fetchBytes functions lack timeout and
retry mechanisms, causing test suite hangs on transient GitHub failures or
stalled connections. Implement a retry wrapper that wraps both functions to
retry on transient errors (429 and 5xx status codes) with exponential backoff,
and add an AbortController-based timeout to the fetch calls in both fetchJson
and fetchBytes (with a reasonable timeout like 30 seconds) to prevent indefinite
hangs. Apply consistent retry and timeout logic to both functions to ensure
robust fixture downloading in the pretest phase.
In `@packages/kit/src/skills/loader/browser.ts`:
- Around line 13-17: Replace the unbounded parallel file loading in the
Promise.all() call that processes all files from options.fileList
simultaneously. Instead of mapping all files to loadBrowserFile calls at once,
implement bounded concurrency control to limit the number of concurrent file
reads (typically using a concurrency library or a queue-based approach). This
will prevent memory spikes from reading all files in parallel and ensure proper
resource management for large file lists, while the actual implementation loads
files in controlled batches rather than all at once through the current
Promise.all() with array map pattern.
In `@packages/kit/src/skills/loader/definition.ts`:
- Around line 15-18: The entryFile variable is not being normalized before being
compared against the normalized file paths in the find operation. Apply the same
normalization logic to options.entryFile that is used in the normalizeFiles
function call, then assign the result to entryFile so that the path comparison
on line 17 correctly matches normalized file paths regardless of input format
(with or without ./ prefixes or backslash variants).
In `@packages/kit/src/skills/loader/github.ts`:
- Around line 126-147: The for loop condition in the GitHub fetch retry logic
uses a less-than-or-equal-to comparison operator (`<=`) which causes one extra
iteration compared to the configured retry limit. Change the loop condition from
`retryCount <= maxGithubFetchRetries` to `retryCount < maxGithubFetchRetries` so
that with a maxGithubFetchRetries value of 5, exactly 5 total attempts are made
instead of 6.
In `@packages/kit/src/skills/test/loaderNode.test.ts`:
- Around line 64-80: The test "loads weather skill from GitHub over the network"
is making actual network calls to GitHub, which makes it unreliable for CI/CD.
Mark this test as skipped by default using either it.skip or a conditional check
for an environment variable/flag that allows it to run only when explicitly
enabled, similar to how integration tests are typically gated. This ensures the
unit test suite runs deterministically without external dependencies while
allowing developers to opt-in to the network-dependent test when needed.
---
Nitpick comments:
In `@packages/kit/src/skills/test/loaderDefinition.test.ts`:
- Around line 124-223: Add a new test case after the existing entry file error
tests (after the "throws when the entry file has no instructions" test) that
verifies the createSkillDefinition function correctly handles non-canonical
entryFile option values such as ./SKILL.md or paths with backslashes. The test
should pass these non-canonical paths in the options object and assert that they
resolve correctly to the same SKILL.md file, confirming that path normalization
works as expected and prevents regressions when users provide paths with
different separators or relative path prefixes.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 192db42e-6270-45c1-977c-9bcc6b1d38ce
📒 Files selected for processing (17)
packages/kit/package.jsonpackages/kit/scripts/download-skill-fixtures.mjspackages/kit/src/node.tspackages/kit/src/skills/loader/browser.tspackages/kit/src/skills/loader/definition.tspackages/kit/src/skills/loader/fs.tspackages/kit/src/skills/loader/github.tspackages/kit/src/skills/loader/index.tspackages/kit/src/skills/loader/node.tspackages/kit/src/skills/loader/type.tspackages/kit/src/skills/loader/utils.tspackages/kit/src/skills/test/.gitignorepackages/kit/src/skills/test/loaderBrowser.test.tspackages/kit/src/skills/test/loaderDefinition.test.tspackages/kit/src/skills/test/loaderNode.test.tspackages/kit/src/skills/types/index.tspackages/kit/src/skills/utils.ts
|
✅ Action performedReview finished.
|
… entry file paths
1 participant
PR 描述
新增 skill 基础定义与 loader 能力，为后续 storage 和 skillPlugin 提供统一的
SkillDefinition输入。主要改动：
SkillDefinition、SkillResourceDescriptor、SkillCandidate等基础类型。fileList/directoryHandle加载 skill。loadSkill/loadSkillWithDetails，默认返回SkillDefinition，details API 返回 warnings。SkillLoadJob。@opentiny/tiny-robot-kit/node子入口。验证：
pnpm -F @opentiny/tiny-robot-kit test -- --run src/skills/test/loaderBrowser.test.ts src/skills/test/loaderDefinition.test.ts src/skills/test/loaderNode.test.ts pnpm buildSummary by CodeRabbit
Release Notes
New Features
./nodepackage export for Node environment skill loading.Chores