mosaicstack/stack
1
0
Fork 0
Code Issues 25 Pull Requests 4 Actions Packages Projects Releases 14 Wiki Activity

fix(launch): include Pi native skill roots in 'all' mode; dedup 'discover' force-loads #556

Merged
jason.woltje merged 1 commits from fix/pi-skill-args-all-discover into main 2026-06-19 19:58:10 +00:00
Conversation 0 Commits 1 Files Changed 2 +184 -15
jason.woltje commented 2026-06-19 19:23:07 +00:00
Owner
Copy Link

Summary

Fast-follow for the two code-review findings on #555 (buildPiSkillArgs).

Finding 1 — all mode dropped Pi's native skill roots. mosaic launches Pi with --no-skills, which suppresses Pi's own native auto-discovery. The all catalog therefore has to re-enumerate the native roots (~/.pi/agent/skills/ and <cwd>/.pi/skills/) explicitly, or any skill living only under those roots is silently dropped. discoverPiSkills now scans them. This also fixes a latent bug in the enumerator: it skipped symlinked entries (!entry.isDirectory()), but synced fleet skills land as symlinks — so they were being dropped on every launch.

Finding 2 — discover mode double-loaded. discover keeps Pi's native auto-discovery ON (no --no-skills), but it force-loaded the fleet skills unconditionally, registering anything Pi already finds natively twice. It now filters force-loads against the native-root realpath set.

Approach

Realpath-based dedup throughout, so a skill reached via a symlink and via its canonical path collapse to one --skill:

  • skillRealPath — realpathSync with literal-path fallback (broken/missing links degrade gracefully).
  • piNativeSkillRoots(cwd) — the two roots Pi auto-discovers.
  • enumerateSkillDirs(roots) — scans roots, accepts dirs and symlinks bearing a SKILL.md, dedups by realpath. Exported for test coverage.
  • piNativeSkillRealPaths(cwd) — realpaths Pi will auto-discover, for the discover-mode filter.
  • mergeSkillArgs — now dedups by realpath.
  • buildPiSkillArgs — gains an injectable 5th param (nativeSkillRealPaths) so the discover-mode filter is deterministic under test.

No launcher/passthrough behavior changes; none mode is unchanged.

Tests

  • discover-mode: drops a forced skill present in the native set; keeps one that isn't; collapses an intra-forced-set duplicate.
  • enumerateSkillDirs real-FS coverage: symlinked-skill acceptance (the regression), cross-root realpath dedup, and SKILL.md/missing-root gating.
  • 308 pass (was 304); pnpm typecheck 41/41, pnpm lint 23/23, prettier --check clean.

Review

Independent review (author ≠ reviewer) completed before open: APPROVE-WITH-NITS; all three SHOULD-FIX nits (discover intra-set dedup consistency + the two missing-coverage items) remediated in this branch.

🤖 Generated with Claude Code

## Summary Fast-follow for the two code-review findings on #555 (`buildPiSkillArgs`). **Finding 1 — `all` mode dropped Pi's native skill roots.** `mosaic` launches Pi with `--no-skills`, which suppresses Pi's own native auto-discovery. The `all` catalog therefore has to re-enumerate the native roots (`~/.pi/agent/skills/` and `<cwd>/.pi/skills/`) explicitly, or any skill living *only* under those roots is silently dropped. `discoverPiSkills` now scans them. This also fixes a latent bug in the enumerator: it skipped symlinked entries (`!entry.isDirectory()`), but synced fleet skills land as symlinks — so they were being dropped on every launch. **Finding 2 — `discover` mode double-loaded.** `discover` keeps Pi's native auto-discovery ON (no `--no-skills`), but it force-loaded the fleet skills unconditionally, registering anything Pi already finds natively twice. It now filters force-loads against the native-root realpath set. ## Approach Realpath-based dedup throughout, so a skill reached via a symlink and via its canonical path collapse to one `--skill`: - `skillRealPath` — `realpathSync` with literal-path fallback (broken/missing links degrade gracefully). - `piNativeSkillRoots(cwd)` — the two roots Pi auto-discovers. - `enumerateSkillDirs(roots)` — scans roots, accepts dirs **and** symlinks bearing a `SKILL.md`, dedups by realpath. Exported for test coverage. - `piNativeSkillRealPaths(cwd)` — realpaths Pi will auto-discover, for the discover-mode filter. - `mergeSkillArgs` — now dedups by realpath. - `buildPiSkillArgs` — gains an injectable 5th param (`nativeSkillRealPaths`) so the discover-mode filter is deterministic under test. No launcher/passthrough behavior changes; `none` mode is unchanged. ## Tests - discover-mode: drops a forced skill present in the native set; keeps one that isn't; collapses an intra-forced-set duplicate. - `enumerateSkillDirs` real-FS coverage: symlinked-skill acceptance (the regression), cross-root realpath dedup, and `SKILL.md`/missing-root gating. - **308 pass** (was 304); `pnpm typecheck` 41/41, `pnpm lint` 23/23, `prettier --check` clean. ## Review Independent review (author ≠ reviewer) completed before open: APPROVE-WITH-NITS; all three SHOULD-FIX nits (discover intra-set dedup consistency + the two missing-coverage items) remediated in this branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jason.woltje added 1 commit 2026-06-19 19:23:08 +00:00
fix(launch): include Pi native skill roots in 'all' mode; dedup 'discover' force-loads
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
Details
ci/woodpecker/pr/ci Pipeline was successful
Details
fa0d2f64de 
Fast-follow for the two code-review findings on #555.

Finding 1 — `all` mode dropped Pi's native skill roots. `mosaic` passes
`--no-skills`, which suppresses Pi's own auto-discovery, so the `all`
catalog must re-enumerate the native roots (`~/.pi/agent/skills/` and
`<cwd>/.pi/skills/`) explicitly or skills living only there vanish.
`discoverPiSkills` now scans those roots too. Also fixes a latent bug:
the old enumerator skipped symlinked entries (`!isDirectory()`), but
synced fleet skills land as symlinks — they were being dropped.

Finding 2 — `discover` mode (which keeps native discovery ON) force-loaded
fleet skills unconditionally, double-registering any skill Pi already finds
natively. It now filters force-loads against the native-root realpath set.

Implementation: realpath-based dedup throughout. New `skillRealPath`,
`piNativeSkillRoots`, `enumerateSkillDirs` (accepts dirs + symlinks, dedup
by realpath), `piNativeSkillRealPaths`. `mergeSkillArgs` dedups by realpath.
`buildPiSkillArgs` gains an injectable 5th param for deterministic tests.

Tests: discover-mode native-filter + intra-set dedup cases, plus real-FS
coverage of `enumerateSkillDirs` (symlink acceptance, cross-root realpath
dedup, SKILL.md gating). 308 pass; typecheck/lint/prettier green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QoYiBeKNh3BiYtAJS5Z587
jason.woltje merged commit 87f561c1f8 into main 2026-06-19 19:58:10 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#556
Delete Branch "fix/pi-skill-args-all-discover"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?