Skip to content

fix(cli): resolve local skill sources against the persona JSON directory#226

Merged
khaliqgant merged 2 commits into
mainfrom
fix/local-skill-source-resolution
Jun 11, 2026
Merged

fix(cli): resolve local skill sources against the persona JSON directory#226
khaliqgant merged 2 commits into
mainfrom
fix/local-skill-source-resolution

Conversation

@khaliqgant

Copy link
Copy Markdown
Member

Problem

Personas that declare local skills with relative paths (e.g. ./skills/persona-relayfile-mount.md, as persona-linear-dispatcher and persona-repo-router do) break when spawned from anywhere other than the persona package root. The loader passes the source through verbatim and it's only resolved at install time against process.cwd(), so the generated cp looks for the file in the launch directory:

cp: /Users/.../pear/skills/persona-relayfile-mount.md: No such file or directory

Because all skill installs run as one &&-chained shell command, this single failing cp has two compounding effects:

  1. The harness spawn aborts (runInstall exits non-zero), surfacing downstream as pear's 120s "Timed out waiting for Workforce persona … to prepare its harness".
  2. The skill cache marker is never written, so every subsequent spawn is a cache miss and re-downloads all prpm skills from scratch — the "installing skills takes forever on every spawn" symptom.

Fix

Resolve local skill sources at persona load time in local-personas.ts, mirroring the existing sidecar (claudeMd/agentsMd) handling:

  • Resolve against the declaring JSON file's directory (__sourceDir) first.
  • Fall back to the package root above it (persona-pack layout keeps skills/ next to personas/).
  • Fall back to cwd (preserves the installer's rewritten __assets/... convention).
  • A file found nowhere produces a load warning and drops that skill instead of aborting the launch — matching missing-sidecar semantics, and keeping a broken pointer from invalidating the skill cache for all the other skills.

Resolution happens where skills enter a resolved spec (standaloneSpecFromOverride / mergeOverride), which is unambiguous because skills replace wholesale per cascade layer; inherited skills were already resolved when the base layer was built.

Testing

  • 3 new tests: resolution next to the JSON, package-root fallback, missing-file → warn + drop (remote skills survive).
  • node --test dist/local-personas.test.js: 44/44 pass.
  • Note: this test file fails locally (pre-existing, unrelated) if you have a real ~/.agentworkforce/workforce/config.json, because the suite isn't isolated from the developer's persona-dir config — run with AGENT_WORKFORCE_HOME pointed at an empty dir.

🤖 Generated with Claude Code

Relative local skill sources (./skills/foo.md) were passed through the
loader verbatim and only resolved at install time against process.cwd().
For personas installed into a project (or loaded from a configured
persona dir), that path points at the launch directory, not the persona
package — the generated `cp` fails, and because skill installs run as a
single `&&` chain, the failure aborts the harness spawn AND prevents the
skill cache marker from being written, forcing every subsequent spawn to
re-download all prpm skills from scratch.

Resolve local skill sources at load time, mirroring sidecar handling:
against the declaring JSON's directory, falling back to the package root
above it (persona-pack layout keeps skills/ next to personas/) and cwd
(the installer's rewritten __assets convention). Missing files now warn
and drop the skill instead of aborting the launch, matching the
missing-sidecar semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for resolving relative local skill sources (non-URL paths ending in .md) against the persona JSON directory, with fallbacks for persona-pack layouts and cwd-relative paths. It also ensures that missing local skill files produce warnings and are gracefully dropped instead of throwing an error. The feedback suggests adding a defensive check in resolveLocalSkillSources to verify that each skill element is a valid object before accessing its properties, preventing potential runtime TypeErrors from malformed user JSON.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +945 to +946
return skills.flatMap((skill) => {
const source = skill.source;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since skills is parsed from user-provided JSON files, its elements are not fully validated to be objects at parse time. If any element in the skills array is null, undefined, or not an object, accessing skill.source will throw a runtime TypeError. Adding a defensive check for skill being a valid object prevents potential crashes.

  return skills.flatMap((skill) => {
    if (!skill || typeof skill !== 'object') {
      return [];
    }
    const source = skill.source;

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b71f2ab6-ec48-4a0e-88f7-23b30e32dc98

📥 Commits

Reviewing files that changed from the base of the PR and between 889309f and f981233.

📒 Files selected for processing (2)
  • packages/cli/src/local-personas.test.ts
  • packages/cli/src/local-personas.ts

📝 Walkthrough

Walkthrough

Personas now resolve local skill file sources during spec creation: relative .md skill paths are resolved against the declaring persona JSON directory with fallback candidates (including package/installer layouts and loader cwd). Missing local skill files no longer throw; they are removed from spec.skills and a persona-scoped warning is emitted.

Changes

Local skill source resolution in persona overrides

Layer / File(s) Summary
Type import and core resolution helper
packages/cli/src/local-personas.ts
PersonaSkill type imported and resolveLocalSkillSources added to detect non-URL .md sources, generate candidate absolute paths using override.__sourceDir and fallbacks, validate with statSync, return resolved paths, and record warnings while dropping missing skills.
Thread cwd and warnings through resolution pipeline
packages/cli/src/local-personas.ts
standaloneSpecFromOverride, resolveInLayer, findInLowerLayers, and mergeOverride signatures are extended to accept cwd and sidecarWarnings; calls are updated to thread cwd so local resolution uses a consistent base.
Integration with persona spec creation
packages/cli/src/local-personas.ts
When an override provides override.skills, mergeOverride now replaces them by calling resolveLocalSkillSources(…, override.__sourceDir, override.id, sidecarWarnings, cwd) so override-local skill sources are resolved per-layer.
Test coverage for skill resolution
packages/cli/src/local-personas.test.ts
Four new tests validate resolution of ./skills/<file> relative to persona JSON dir, fallback to package-level adjacent skills/, fallback to loader cwd for asset layouts, and behavior when referenced local skill files are missing (skill dropped, warning emitted).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I sniff the paths where skills may hide,
From JSON burrows to packages wide,
I hop through fallbacks, statSync in paw,
If one is missing, I warn—no flaw.
A gentle drop, the rest stand tall.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: resolving local skill sources against the persona JSON directory, which is the core fix implemented in the changeset.
Description check ✅ Passed The description comprehensively explains the problem, the fix approach, and testing details, all directly related to the changeset's local skill source resolution implementation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/local-skill-source-resolution

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/cli/src/local-personas.ts`:
- Around line 939-976: resolveLocalSkillSources currently falls back to
process.cwd() when resolving relative skill paths, ignoring the loader-provided
cwd; change its signature to accept a cwd parameter (e.g., cwd: string |
undefined) and replace resolvePath(process.cwd(), source) with resolvePath(cwd
?? process.cwd(), source). Update callers (e.g., loadLocalPersonas and any other
callers of resolveLocalSkillSources) to pass the loader's options.cwd so
resolution uses the supplied cwd context.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2dd9ada2-71f2-4d1f-841a-1b46376e8e3d

📥 Commits

Reviewing files that changed from the base of the PR and between baad3ab and 889309f.

📒 Files selected for processing (2)
  • packages/cli/src/local-personas.test.ts
  • packages/cli/src/local-personas.ts

Comment thread packages/cli/src/local-personas.ts
@agent-relay-code

Copy link
Copy Markdown
Contributor

Implemented a scoped fix for the PR.

Changed:

Addressed comments

  • No bot/reviewer comments were present in .workforce/context.json or other .workforce artifacts, so there were no external review threads to validate.

Advisory Notes

  • None.

Validation run locally:

  • corepack pnpm install --frozen-lockfile
  • corepack pnpm -r run build
  • corepack pnpm --filter @agentworkforce/cli test
  • corepack pnpm -r run lint
  • corepack pnpm -r run typecheck
  • corepack pnpm run typecheck:examples
  • corepack pnpm -r run test

All passed. The root pnpm run check script could not run directly because this container lacks a global pnpm shim and Corepack cannot write to /usr/bin, so I ran the equivalent CI subcommands with corepack pnpm.

khaliqgant added a commit to AgentWorkforce/pear that referenced this pull request Jun 11, 2026
…dy; repair installed persona skill paths (#223)

* feat(factory): software factory pipeline — ready-for-agent → scope → team spawn → implement

Adds the complete software factory pipeline so issues marked `ready-for-agent`
flow automatically through scoping, team spawning, and implementation:

- writeRemoteFile IPC: renderer/agents can now create/update Linear issues
- ready-for-agent 4th band in Attention Inbox (expanded by default)
- spawnTeamForIssue: spawns codex-impl + claude-review pairs with task prompts
- issue-scoping module: detectRepo, suggestTeamSize, labelIssueWithRepo
- board-steward persona: proactive agent that watches and drives the pipeline

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: apply pr-reviewer fixes for #222

* chore: apply pr-reviewer fixes for #222

* chore: add linear-dispatcher + repo-router personas to refresh script

Install @agentworkforce/persona-linear-dispatcher and
@agentworkforce/persona-repo-router, and append both to the
personas:refresh npm script so all personas stay current via one command.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: remove board-steward persona

board-steward overlaps with the linear-dispatcher + repo-router pair —
both poll ready-for-agent Linear issues and spawn impl/review teams, so
running them together double-dispatches. Keep linear-dispatcher (board
watcher) + repo-router (per-issue routing) as the single dispatch path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(broker): fail fast when a persona worker exits before harness-ready; repair installed persona skill paths

A workforce CLI that dies during setup (e.g. a failed skill install
calls process.exit) previously burned the entire 120s
PERSONA_HARNESS_READY_TIMEOUT_MS before surfacing an error. The
harness-ready wait now rejects as soon as the worker is gone: it watches
agent_exit/agent_exited/agent_released events and runs a 2s listAgents
liveness poll as a catch-all for exits that slip past the event stream
(e.g. between spawn and subscription).

Also repairs the installed linear-dispatcher and repo-router personas,
which carried raw `./skills/*.md` sources from an installer version
without skill-asset support. The cp for those paths resolved against the
project root, failed, aborted every spawn, and kept the workforce skill
cache marker from being written — forcing a full prpm re-download on
each attempt. Skill files now live under __assets/<id>/skills/ with
sources rewritten to the convention the fixed installer emits
(AgentWorkforce/workforce#226). Do not re-run personas:refresh until a
CLI with that fix is published and the agentworkforce dep is bumped —
npx prefers the local 3.0.52 binary, which would regenerate the broken
JSONs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com>
@khaliqgant khaliqgant merged commit b680963 into main Jun 11, 2026
3 checks passed
@khaliqgant khaliqgant deleted the fix/local-skill-source-resolution branch June 11, 2026 10:29
@agent-relay-code

Copy link
Copy Markdown
Contributor

Implemented a scoped fix for the remaining valid review finding.

Changed:

Addressed comments

  • gemini-code-assist[bot] inline comment: malformed skills entries could crash on skill.source; fixed in packages/cli/src/local-personas.ts:449 and covered in packages/cli/src/local-personas.test.ts:829.
  • gemini-code-assist[bot] review summary: same malformed-skill validation concern; fixed in packages/cli/src/local-personas.ts:449.
  • coderabbitai[bot] inline comment: resolveLocalSkillSources ignored loader-provided cwd; stale, already fixed in current checkout at packages/cli/src/local-personas.ts:978 and threaded from packages/cli/src/local-personas.ts:1182.
  • coderabbitai[bot] review summary: same cwd fallback concern; stale, already fixed in current checkout at packages/cli/src/local-personas.ts:978.
  • coderabbitai[bot] walkthrough/pre-merge comment: docstring coverage warning and generated finishing-touch suggestions; no code change because this is advisory/generated and not a demonstrated breakage in this PR’s changed behavior.
  • agent-relay-code[bot] comment: previous run said no bot comments were present; stale after fetching current PR comments, no code change needed.

Advisory Notes

  • CodeRabbit’s docstring coverage warning is not tied to the local skill resolution behavior and would be a broader policy cleanup, so I left it out of this PR.

Validation:

  • corepack pnpm install --frozen-lockfile
  • corepack pnpm -r run build
  • corepack pnpm -r run lint
  • corepack pnpm -r run typecheck
  • corepack pnpm run typecheck:examples
  • corepack pnpm -r run test

All passed. The root corepack pnpm run lint wrapper cannot run in this container because its script shells out to a missing global pnpm shim, so I ran the equivalent CI subcommands directly with corepack pnpm.

GitHub currently reports PR #226 as closed and merged, so I am not marking it READY.

khaliqgant added a commit to AgentWorkforce/pear that referenced this pull request Jun 11, 2026
The 4.0.2 release carries the workforce-side fixes this branch depends
on: the persona loader resolves local skill sources against the persona
JSON's directory (AgentWorkforce/workforce#226), and the installer
copies local skill assets into __assets/ with rewritten sources.

- bump agentworkforce + @agentworkforce/deploy ^3.0.51 -> ^4.0.2
- update the AGENTWORKFORCE_CLI_VERSION npx fallback pin to match
- re-run personas:refresh with the 4.0.2 installer; it reproduced the
  hand-repaired linear-dispatcher/repo-router JSONs byte-for-byte and
  picked up the published slack-comms permissions field

Smoke-tested: `agentworkforce show` now resolves local skill sources to
absolute existing paths; typecheck and 120/120 tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
khaliqgant added a commit to AgentWorkforce/pear that referenced this pull request Jun 11, 2026
…224)

* chore: bump agentworkforce to ^4.0.2 and refresh installed personas

The 4.0.2 release carries the workforce-side fixes this branch depends
on: the persona loader resolves local skill sources against the persona
JSON's directory (AgentWorkforce/workforce#226), and the installer
copies local skill assets into __assets/ with rewritten sources.

- bump agentworkforce + @agentworkforce/deploy ^3.0.51 -> ^4.0.2
- update the AGENTWORKFORCE_CLI_VERSION npx fallback pin to match
- re-run personas:refresh with the 4.0.2 installer; it reproduced the
  hand-repaired linear-dispatcher/repo-router JSONs byte-for-byte and
  picked up the published slack-comms permissions field

Smoke-tested: `agentworkforce show` now resolves local skill sources to
absolute existing paths; typecheck and 120/120 tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: apply pr-reviewer fixes for #224

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant