feat(workload-router): skill provider adapter + post-run cleanup - #14
Conversation
Replace hard-coded prpm install logic with a SkillProvider adapter interface so skill sources beyond prpm can be wired in without touching materializeSkills. Adds a skill.sh provider (github URL + skill fragment form) alongside the existing prpm provider, and makes each SkillInstall carry a cleanupPaths list so buildInstallArtifacts can append rm -rf after a successful install — leaving the provider lockfile in place so repeat persona runs still benefit from cached resolution. Adds two personas that exercise both providers: skill-finder (skill.sh find-skills) under intent skill-discovery, and prpm-self-improver (@prpm/self-improving) under intent prpm-self-improvement. generate-personas.mjs now skips unmapped draft personas with a warning so unfinished drafts no longer block the build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 681c02d9c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cleanupPart = `rm -rf ${install.cleanupPaths.map(shellEscape).join(' ')}`; | ||
| return `${installPart} && ${cleanupPart}`; |
There was a problem hiding this comment.
This appends rm -rf directly to each install command, but that command string is executed in the install step before the agent step (usePersona wires installCommandString into the deterministic install step). As a result, skill directories are deleted before the persona run can consume them, so personas can execute without their declared skills; it also breaks Mode B/install-only flows where install.commandString is supposed to stage skills for later installSkills: false runs. Cleanup needs to run after agent execution, not inline with the pre-run install step.
Useful? React with 👍 / 👎.
…y-discovery Merge the separate skill-discovery and prpm-self-improvement intents into a single capability-discovery intent backed by one persona (capability-discoverer) that carries both the skill.sh/find-skills and @prpm/self-improving skills. Same underlying job — find an existing skill/agent/hook for the project — just with the right ecosystem picked per candidate. This exercises multi-skill install planning: a single persona install chain now runs two provider installs back-to-back, each followed by its own rm -rf cleanup, with neither lockfile touched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const installCommandString = plan.installs | ||
| .map((install) => { | ||
| const installPart = commandToShellString(install.installCommand); | ||
| if (install.cleanupPaths.length === 0) { | ||
| return installPart; | ||
| } | ||
| const cleanupPart = `rm -rf ${install.cleanupPaths.map(shellEscape).join(' ')}`; | ||
| return `${installPart} && ${cleanupPart}`; | ||
| }) | ||
| .join(' && '); |
There was a problem hiding this comment.
🔴 Skill cleanup (rm -rf) runs during install step, deleting files before the agent step can read them
The buildInstallArtifacts function chains rm -rf cleanup immediately after each skill install command. This cleanup runs as part of the install step, which executes BEFORE the agent step. The agent step then tries to use the skill files — but they've already been deleted.
The comments on cleanupPaths say these paths are "safe to rm -rf once the persona run has read what it needs from them" (packages/workload-router/src/index.ts:136), and the code comment at line 372 says "after the persona run finishes" — but the implementation does cleanup during install, not after.
Evidence that agents need skill files on disk
The README states: "Once installed, Claude Code auto-discovers skills from .claude/skills/; for other harnesses, read the manifest off disk and inject it into the agent's task body." (README.md:201)
The existing workflow workflows/finish-npm-provenance-persona.ts:121 has a step verifying the manifest is on disk BEFORE the agent runs:
'test -f .claude/skills/npm-trusted-publishing/SKILL.md && echo "OK" || (echo "SKILL MANIFEST MISSING" >&2; exit 1)'
For example, usePersona('npm-provenance').sendMessage(...) generates an install step like:
npx -y prpm install @prpm/npm-trusted-publishing --as opencode && rm -rf .skills/npm-trusted-publishing
This installs the skill, immediately deletes it, and then the agent step runs and cannot find it. The same applies to the new capability-discovery persona where both skill.sh and prpm skills are cleaned up before the agent step begins.
Prompt for agents
The buildInstallArtifacts function at packages/workload-router/src/index.ts:591-624 chains rm -rf cleanup immediately after each install command in the shell string. This cleanup runs during the install step (a deterministic workflow step), which completes BEFORE the agent step starts.
The problem is that agents need skill files to be on disk when they run: Claude Code auto-discovers skills from .claude/skills/, and other harnesses read SKILL.md manifests from disk.
The cleanup should happen AFTER the agent step completes, not during the install step. Possible approaches:
1. Remove the rm -rf from buildInstallArtifacts entirely and add a separate post-agent cleanup step in the sendMessage() workflow builder (around line 1398-1409 in index.ts). This step would depend on the agent step and run rm -rf on the cleanupPaths.
2. Expose cleanupPaths on PersonaInstallContext so callers can handle cleanup themselves after the persona run.
3. Keep cleanupPaths as metadata on SkillInstall but do not embed them in the install command string. Instead, add a new method or workflow step that runs cleanup after execution.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
✅ Resolved: This bug reported that buildInstallArtifacts chained rm -rf cleanup into the install step, deleting skill files before the agent could read them. The latest commit (acc638d) explicitly fixes this: buildInstallArtifacts no longer includes cleanup, and a separate buildCleanupArtifacts function was added. The cleanup now runs as a post-agent workflow step (dependsOn: [stepName]) at lines 1436-1446. The bug is resolved.
Adds a GitHub Actions workflow that runs `pnpm run lint`, `pnpm run typecheck`, and `pnpm run test` against every pull request targeting main and every push to main. Pinned to the same Node version the publish workflow uses so PR checks and release builds match. Uses concurrency cancel-in-progress on PRs so fast follow-up pushes don't keep stale runs queued. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The install step ran its command string BEFORE the agent step, so inlining `&& rm -rf <cleanupPaths>` into that string deleted the skill manifests the agent needed to read. This broke both Mode A (sendMessage owns the install) and Mode B (caller pre-stages install.commandString in a Dockerfile/CI step and then runs with installSkills: false — they were getting an empty skills directory). Changes: - buildInstallArtifacts no longer chains cleanup; it emits a pure install command string again. - New buildCleanupArtifacts helper produces a single `rm -rf` line covering every cleanupPaths entry across all installs in the plan (`:` for empty plans to keep the shape uniform). - PersonaInstallContext now carries cleanupCommand and cleanupCommandString so Mode B callers can run post-agent cleanup themselves. - sendMessage adds a dedicated `<stepName>-cleanup-skills` deterministic step that dependsOn the agent step and runs cleanupCommandString. It uses failOnError: false so a cleanup hiccup does not mask agent success, and is skipped entirely when the plan has no cleanupPaths. - Regression test writes a real SKILL.md into the declared cleanupPath, has the fake agent assert the file exists during its run, then asserts the directory is gone after sendMessage settles. 29/29 green. Flagged by devin-ai-integration and chatgpt-codex-connector on PR #14. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
SkillProvideradapter interface so new skill sources can be wired in without touchingmaterializeSkills. Adds askill.shprovider (https://github.com/<org>/<repo>#<skill>source form) alongside the existing prpm provider.SkillInstallnow carries acleanupPathslist;buildInstallArtifactsappendsrm -rf <paths>after a successful install so persona runs leave only the provider lockfile behind. The lockfile is deliberately excluded from cleanup so repeat runs keep cached resolution.skill-finder(intentskill-discovery, usesskill.sh/find-skills) andprpm-self-improver(intentprpm-self-improvement, uses@prpm/self-improving).scripts/generate-personas.mjsnow skips unmapped draft personas with a warning instead of throwing, unblocking the existing untracked drafts.Why cleanup matters
When a user runs a workload persona via
usePersona(...).sendMessage(), they don't want leftover.claude/skills/*,.agents/skills/*, etc. in their workspace. The provider's lockfile is the source of truth; everything else is ephemeral scratch that can be wiped and re-resolved from the lock on the next run.skill.sh artifact layout (verified by live
npx -y skills add ... -yrun)Cleanup wipes all of:
.agents/skills/<name>(universal content dir).claude/skills/<name>,.factory/skills/<name>,.kiro/skills/<name>,skills/<name>(harness symlinks)Preserved:
skills-lock.json.Test plan
npm run generate:personassucceeds and emits the two new persona exportsnpm run typecheckcleannpm test— 25/25 passing, including new cases:materializeSkills emits a skill.sh install for a github#skill sourceprpm installs carry a harness-scoped cleanup path (not the lockfile)usePersona install command appends rm -rf cleanup after the install stepresolves skill-discovery persona with the skill.sh find-skills skill attachedresolves prpm-self-improvement persona with the @prpm/self-improving skill attachedsendMessage()and confirm the workspace is clean afterwards except for the lockfile🤖 Generated with Claude Code