Repository navigation
feat(skills): remove the skills a run installed when it ends - #1489
Conversation
runProgram now notes the wizard-marked skills in .claude/skills before each run and deletes the ones the run added when it ends, whether it succeeds, fails or is interrupted. Skills the user wrote, or kept from an earlier run, stay. An empty skills folder is removed too. This covers headless and CI runs, which never showed the keep-skills screen and so left their skills behind. Self-driving drops its own postRun removal, since every program now gets it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every program flow ended on a screen asking whether to keep the installed skills. The runner now removes them when the run ends, so the screen and its step in each flow are gone. Flows end on the exit screen. After a failed run, choosing "continue" ended the run when keep-skills finished. The host and the abort presenter now wait for the exit screen instead. The e2e harness reports `flowComplete` in place of `skillsComplete` (read_state and e2e-result.json), and profiles lose their `skills` setting and the `keep_skills` action. The `skills complete` and `screen keep-skills` events stop firing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Approved by hand after checking the automated review.
| // The run neutralized its directory's settings; put them back whatever happened. | ||
| if (settings.backedUp) restoreClaudeSettings(runDir); | ||
| unregisterSkills(); | ||
| removeSkills(); |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
[correctness] LOW
removeSkills() runs unguarded in the finally. removeRunSkills guards each rmSync but not the readdirSync in listWizardSkills, so an unreadable skills folder (e.g. EACCES) would throw from the finally and replace the run's real result or error. Wrapping the call in a try/catch that logs would match the helper's best-effort intent.
An unreadable .claude/skills folder made listWizardSkills throw, which could fail a run before its agent started or replace its result from the finally. Listing now returns null on a read error, and removeRunSkills deletes nothing when either listing is null. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/wizard-ci basic-integration/next-js |
🧙 Wizard CI ResultsTrigger ID:
Configuration
|
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
The run's snapshot held only wizard-marked folders, so an unmarked folder that a same-named install marked during the run was deleted, and a symlinked .claude/skills could point that deletion outside the project. The snapshot now holds every entry, the listing returns null when the skills folder resolves outside the project, and symlinked entries are skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rmdir removes only an empty folder, so the cleanup can never delete contents added between its emptiness check and the removal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
runProgram now notes the wizard-marked skills in
.claude/skillsbefore each run and deletes the ones the run added when it ends.Skills the user wrote, or kept from an earlier run, stay. An empty skills folder is removed too.
This covers headless and CI runs, which never showed the keep-skills screen and so left their skills behind. Self-driving drops its own postRun removal, since every program now gets the default behavior.