feat(skills): synchronize managed skills at startup#111
Conversation
Keep supported agent hosts aligned before command execution so bundled skills and locally cached registry skills are available without an explicit install step. The startup path stays silent, avoids network fetches, and preserves unmanaged skill targets. Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary by CodeRabbit
WalkthroughThis PR implements automatic startup synchronization of managed AI agent skills across all available host directories. The changes introduce a new Sequence DiagramsequenceDiagram
participant CLI as CLI Bootstrap
participant Sync as Startup Sync Handler
participant Hosts as Available Hosts
participant Bundled as Bundled Skills
participant Registry as Registry Skills
participant FS as Filesystem
CLI->>Sync: synchronizeManagedSkillsForAvailableHosts()
Sync->>Hosts: Resolve available host directories
Hosts-->>Sync: [Codex, Claude Code, OpenClaw, ...]
loop For each host directory
Sync->>Bundled: Check bundled skill state
Bundled->>FS: Read managed/unmanaged/missing status
FS-->>Bundled: Current state
alt Managed but outdated (non-development)
Bundled->>FS: Publish bundled skill installation
FS-->>Bundled: Success/Warning
else Skip (development version or unmanaged)
Bundled-->>Sync: Skipped
end
end
loop For each host directory
Sync->>Registry: Enumerate canonical registry skills
Registry->>FS: Read canonical skill metadata
FS-->>Registry: Skill configuration
alt Managed and version mismatch
Registry->>FS: Publish registry skill
FS-->>Registry: Success/Warning
else Skip (up-to-date or unmanaged)
Registry-->>Sync: Skipped
end
end
Sync-->>CLI: Synchronization complete
CLI->>CLI: Execute requested command
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/application/commands/skills/list.cli.test.ts (1)
75-87:⚠️ Potential issue | 🟡 MinorThis setup no longer proves
skills listtriggered the sync.Line 81 now runs the same startup synchronization as every other CLI command, so
skills install oocan createoo-find-skillsbefore Line 85 executes. That means this test still passes even if the bootstrap sync insideskills listregresses. Seed onlyooon disk, or assertoo-find-skillsis absent before theskills listinvocation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/application/commands/skills/list.cli.test.ts` around lines 75 - 87, The test incorrectly relies on calling sandbox.run(["skills","install","oo"]) which performs the same startup sync as skills list; instead seed only the on-disk bundled package and ensure no pre-synced marker exists before invoking skills list. Use resolveOpenClawHomeDirectory(sandbox.env) to locate the OpenClaw home, create the minimal on-disk bundle for "oo" (e.g., a directory or manifest) with mkdir(..., { recursive: true }) rather than calling sandbox.run install, or alternatively after the install call explicitly assert that the "oo-find-skills" sync artifact is absent before calling sandbox.run(["skills","list"]) so the test actually verifies that skills list triggers the sync.
🧹 Nitpick comments (1)
src/application/commands/skills/index.cli.test.ts (1)
58-88: Extract a shared startup-test helper.These new cases all repeat the same
sandbox.run(["--help"])/ log-read / baseline assertions. A small local helper would keep the startup contract checks aligned and make later behavior changes much easier to update consistently.Based on learnings: Extract repeated setup (mocks, stubs, setup objects) into local factory functions in test files; avoid copy-pasting test setup.
Also applies to: 94-196, 198-248, 250-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/application/commands/skills/index.cli.test.ts` around lines 58 - 88, Multiple tests duplicate the same CLI startup flow (creating sandbox, running sandbox.run(["--help"]), reading logs via readLatestLogContent, and asserting baseline expectations). Extract a local helper (e.g., runCliStartupCheck or createStartupSandbox) that accepts any per-test overrides and performs the shared steps: createCliSandbox(), mkdir(codexHomeDirectory,...), run(["--help"], { version }), readLatestLogContent(sandbox), and the baseline assertions (result.exitCode === 0, result.stderr === "", result.stdout contains "Options:", etc.), then replace the repeated blocks in the tests (the test "auto-installs bundled skills during cli startup" and the other ranges 94-196, 198-248, 250-272) to call that helper and only assert test-specific outcomes (e.g., existence of skillDirectoryPath, specific log messages). Ensure the helper exposes returned values (sandbox, result, content, codexHomeDirectory) so tests can make additional assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/application/commands/skills/list.cli.test.ts`:
- Around line 75-87: The test incorrectly relies on calling
sandbox.run(["skills","install","oo"]) which performs the same startup sync as
skills list; instead seed only the on-disk bundled package and ensure no
pre-synced marker exists before invoking skills list. Use
resolveOpenClawHomeDirectory(sandbox.env) to locate the OpenClaw home, create
the minimal on-disk bundle for "oo" (e.g., a directory or manifest) with
mkdir(..., { recursive: true }) rather than calling sandbox.run install, or
alternatively after the install call explicitly assert that the "oo-find-skills"
sync artifact is absent before calling sandbox.run(["skills","list"]) so the
test actually verifies that skills list triggers the sync.
---
Nitpick comments:
In `@src/application/commands/skills/index.cli.test.ts`:
- Around line 58-88: Multiple tests duplicate the same CLI startup flow
(creating sandbox, running sandbox.run(["--help"]), reading logs via
readLatestLogContent, and asserting baseline expectations). Extract a local
helper (e.g., runCliStartupCheck or createStartupSandbox) that accepts any
per-test overrides and performs the shared steps: createCliSandbox(),
mkdir(codexHomeDirectory,...), run(["--help"], { version }),
readLatestLogContent(sandbox), and the baseline assertions (result.exitCode ===
0, result.stderr === "", result.stdout contains "Options:", etc.), then replace
the repeated blocks in the tests (the test "auto-installs bundled skills during
cli startup" and the other ranges 94-196, 198-248, 250-272) to call that helper
and only assert test-specific outcomes (e.g., existence of skillDirectoryPath,
specific log messages). Ensure the helper exposes returned values (sandbox,
result, content, codexHomeDirectory) so tests can make additional assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 532f85e8-f839-4bd5-85c9-284c43101479
📒 Files selected for processing (8)
CONTRIBUTING.mddocs/commands.mddocs/commands.zh-CN.mdsrc/application/bootstrap/run-cli.tssrc/application/commands/skills/auto-sync.tssrc/application/commands/skills/index.cli.test.tssrc/application/commands/skills/list.cli.test.tssrc/application/commands/skills/shared.ts
| async function synchronizeRegistrySkills( | ||
| hosts: readonly ManagedSkillHost[], | ||
| context: SkillSyncContext, | ||
| ): Promise<void> { | ||
| const skills = await listCanonicalRegistrySkills(context); | ||
|
|
||
| await Promise.all( | ||
| skills.flatMap(skill => | ||
| resolveManagedSkillHostInstallations(hosts, skill.name).map( | ||
| installation => | ||
| synchronizeRegistrySkill(installation, skill, context), | ||
| ), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Cap registry startup sync concurrency.
Lines 237-244 and 368-410 both fan out with unbounded Promise.all. Because this runs before every CLI invocation, a large local registry cache can turn even oo --help into hundreds of concurrent fs reads/writes here, which is enough to slow startup badly or hit file-descriptor pressure. Please batch or otherwise bound this work instead of launching one task per skill/host all at once.
Also applies to: 344-413
1 participant
Keep supported agent hosts aligned before command execution so bundled skills and locally cached registry skills are available without an explicit install step. The startup path stays silent, avoids network fetches, and preserves unmanaged skill targets.