Skip to content

feat: Add --fresh for clean skill output rebuilds - #850

Open
gigerIT wants to merge 7 commits into
laravel:mainfrom
gigerIT:main
Open

feat: Add --fresh for clean skill output rebuilds#850
gigerIT wants to merge 7 commits into
laravel:mainfrom
gigerIT:main

Conversation

@gigerIT

@gigerIT gigerIT commented Jun 12, 2026

Copy link
Copy Markdown

Adds a --fresh flag to boost:update and boost:install that deletes each selected agent’s generated skills directory before running the normal skill sync.

Why

We have run into this a few times in real projects: generated skill folders can end up with dangling symlinks, stale files, or broken copied directories after switching between operating systems, resolving merge conflicts, or committing generated agent output by mistake.

The fix was always straightforward: delete the generated skills folder manually, then rerun boost:update or boost:install.

This flag makes that cleanup an explicit Boost workflow instead of a manual recovery step.

Behavior

When --fresh is passed, Boost removes the generated skills directory for each selected agent before syncing skills again.

Default behavior is unchanged unless --fresh is passed.

.ai/skills remains untouched.

Use case

This is useful when generated agent directories contain files that are no longer part of the current Boost sync result. Without deleting the generated directory first, those stale files can remain present and continue influencing the agent.

Example:

php artisan boost:update --fresh

or:

php artisan boost:install --fresh

Safety

The source .ai/skills directory is preserved.

Running boost:update or boost:install without --fresh behaves exactly as before.

Tests

Tests cover:

  • boost:update --fresh
  • boost:install --fresh
  • .ai/skills is preserved
  • only selected agents are cleaned
  • normal behavior is unchanged without --fresh

gigerIT and others added 4 commits June 12, 2026 10:35
- Introduced a new `--fresh` option in `InstallCommand` and `UpdateCommand` to clean up output folders by deleteing each of agent's generated skills directories before installation or update.
- Updated `SkillWriter` to handle the fresh sync logic, ensuring untracked entries are removed and dangling symlinks are cleaned up.
- Added tests to verify the functionality of the fresh sync feature, ensuring it behaves correctly under various scenarios.
Use one symlink deletion path for both top-level and nested skill entries. Windows can require rmdir() for directory symlinks, including dangling links whose targets no longer exist, so nested cleanup must not gate the fallback on is_dir().

Add a regression test for nested dangling symlinks and make the test cleanup helper use the same unlink/rmdir fallback.
@pushpak1300

Copy link
Copy Markdown
Member

AI generated review

  1. --fresh deletes the entire agent skills directory, wiping user-authored skills — SkillWriter.php:117 [CONFIRMED ×2 independently]
    deleteDirectory($targetRoot) nukes the whole .claude/skills (or .cursor/skills, …) — a directory users also populate with their own hand-written skills. writeAll only rebuilds Boost's skills, so anything else is gone forever. This directly contradicts the guarantee the normal-sync path is tested to uphold (SkillWriterTest.php:523 keeps untracked skills; the new fresh test at :1112 proves an untracked orphaned-skill dir is deleted). The flag help says "generated skills directory" but the code deletes the whole shared directory. Fix: scope deletion to Boost-tracked skill names (the previouslyTrackedSkills set already exists), or gate --fresh behind a confirmation prompt.

  2. --fresh on boost:update forces skills ON for guidelines-only projects — UpdateCommand.php:37 [CONFIRMED]
    || $this->option('fresh') forces $hasSkills = true even when skills were never configured and no .ai/skills exists. A user who runs Boost only for guidelines does boost:update --fresh and it (a) deletes their .claude/skills, then (b) installs default skills they never opted into. Compounds finding Update README.md #1 by triggering it on projects that never used skills. Fix: don't let --fresh synthesize skill installation where none was configured.

  3. Guard checks exact path equality, not containment — SkillWriter.php:116 [from inline pass, unverified by workflow]
    ! pathsMatch($targetRoot, $canonicalRoot) protects .ai/skills only on exact match. A configured skills_path that is an ancestor of the canonical root (e.g. .ai) makes fresh delete .ai/skills itself; custom-skill writes then read from the deleted canonical source and fail. Contrived (requires config override) but a real gap — reject any targquals thecanonical root.

  4. Empty-collecphp:37[PLAUSIBLE] — the verifier tempered this: SkillComposer::skills() is normally non-empty (bundled laravel/pest skills), so "writes nothing back" only holds for a project with no recognized packages and no user skills. The data loss holds regardless; the "writes nothing" detail is the uncommon case.

gigerIT added 2 commits August 6, 2026 09:24
# Conflicts:
#	src/Console/UpdateCommand.php
Require confirmation unless --force is supplied and reject targets that overlap the canonical custom skills directory.
@gigerIT

gigerIT commented Aug 6, 2026

Copy link
Copy Markdown
Author

Addressed in e96179c

  1. Full-directory deletion and user-authored skills

    The full rebuild remains intentional because limiting deletion to previously tracked skills would not remove the stale files, broken directories, and dangling symlinks that --fresh is intended to recover from.

    To make that destructive behavior explicit and safe, both boost:install --fresh and boost:update --fresh now use Laravel’s ConfirmableTrait. They warn that the selected agents’ complete skills directories will be rebuilt and that custom skills must be stored in .ai/skills. --force provides Laravel’s standard non-interactive/no-questions path.

    For boost:update, confirmation occurs before package discovery, so declining also leaves boost.json unchanged.

  2. --fresh enabling skills on guidelines-only projects

    Removed --fresh from the $hasSkills calculation. It now only changes how an already-enabled skill installation runs; it cannot enable skills by itself. A guidelines-only project therefore continues updating guidelines without installing or deleting skills.

  3. Canonical path containment

    Fresh rebuilds now reject any target that overlaps .ai/skills, including:

    • the canonical directory itself;
    • an ancestor such as .ai;
    • a descendant such as .ai/skills/generated-output.

    Paths are normalized before comparison, and the overlap is rejected before any deletion occurs.

  4. Empty skill collections

    This case is now explicitly covered. After an approved or forced fresh rebuild, an empty composed skill collection leaves the generated target empty/absent. That is intentional fresh-rebuild behavior rather than an accidental partial sync.

    The confirmation requirement and canonical-path overlap guard ensure this cleanup cannot silently delete the .ai/skills source directory.

Focused coverage now includes confirmation and --force, guidelines-only updates, cancellation without discovery/config mutation, exact/ancestor/descendant canonical-path overlap, and empty fresh rebuilds. The focused suite passes with 77 tests and 252 assertions.

# Conflicts:
#	src/Console/UpdateCommand.php
@pushpak1300

pushpak1300 commented Aug 7, 2026

Copy link
Copy Markdown
Member

AI-Review: (reviewed at 73b8cdb; each finding validated against the code, method noted)

1. --fresh deletes the whole shared skills directory, wiping user-authored skills — SkillWriter.php:125 [CONFIRMED]
deleteDirectory($targetRoot) removes all of .claude/skills (or .agents/skills, …), but writeAll() only restores Boost's skills — anything else is gone. These are shared directories, not Boost output: .claude/skills is Claude Code's own documented location for hand-written skills, and .agents/skills is used by both Codex (Codex.php:86) and OpenCode (OpenCode.php:101). Proven in this repo — .claude/skills holds hand-written boost-testing and migrate-boost-guidelines, .ai/skills doesn't exist, and .claude is gitignored, so boost:install --fresh destroys them unrecoverably. SkillWriterTest.php:1112 passes [] for tracked skills and asserts the orphan is deleted, pinning the data loss as intended behavior. Fix: report untracked entries instead of deleting them.

2. A correctly-scoped --fresh would be a no-op — SkillWriter.php:124 [CONFIRMED]
sync() already fully replaces everything Boost owns on every run: copyDirectory() deletes each target first (:222), createSymlink() handles pre-existing and dangling links (:293), deleteDirectory() handles a file-where-a-dir-belongs (:175), and removeStale() handles dropped skills (:132). Scope the delete to array_unique([...$previouslyTrackedSkills, ...$skills->keys()->all()]) and it deletes nothing that write() wouldn't delete moments later. The flag only does something because it's unscoped — i.e. its entire value is removing files Boost can't prove it wrote. Fix: see #1; Boost has no provenance marker distinguishing its own residue from a user's skill, so it shouldn't guess.

3. Overlap guard is swallowed and the command still exits 0 — SkillWriter.php:121 [CONFIRMED]
sync() runs per agent inside installFeature(), which wraps each call in try/catch (InstallCommand.php:660-671). The RuntimeException becomes a red ✗ for one agent; the loop continues and the remaining agents are still wiped and rebuilt, and handle() returns self::SUCCESS. A skills_path overlapping .ai/skills therefore yields a warning line, mutated agents, and exit code 0. Fix: validate every target before the first delete, and fail non-zero.

4. --force collides with an existing meaning in this codebase — InstallCommand.php:56 [CONFIRMED]
boost:skill:add --force is documented as "Overwrite existing skills" (AddSkillCommand.php:40). Here the same flag means "skip the fresh-rebuild confirmation", and on its own it does nothing — grep confirms ConfirmableTrait is its only consumer. Someone will put boost:install --force in CI expecting the other meaning. ConfirmableTrait's contract is also "confirm in production"; $callback = true repurposes it into "always confirm". Fix: rename to --fresh-force and call Laravel\Prompts\confirm() directly.

5. Failed deletes are silent — SkillWriter.php:125 [CONFIRMED]
The deleteDirectory() return value is discarded. On a permissions error or Windows file lock the delete fails, sync proceeds, and the command reports success — the exact broken state --fresh exists to repair. Fix: check it and fail.

6. deleteSymlink() is a genuine Windows fix — please split it out — SkillWriter.php:201 [CONFIRMED, verified by running]
The old nested-link branch was if (! @unlink($linkPath) && is_dir($linkPath)) { @rmdir($linkPath); }. is_dir() follows the link, so on a dangling directory symlink it returns false — verified directly (is_link: true, is_dir: false, file_exists: false). On Windows, where unlink fails on directory symlinks, the rmdir fallback was unreachable and the link leaked. deleteSymlink() always attempts rmdir, which fixes it. This needs none of --fresh and would merge on its own with the two dangling-symlink tests.


Suggested alternative to --fresh — report, don't delete. Untracked entries are a discovery problem; surfacing them solves the visible half with no data loss, and no --force, ConfirmableTrait, or path-overlap guard:

/** @return array<int, string> Entries in the skills dir Boost did not write. */
public function untracked(Collection $skills, array $previouslyTrackedSkills = []): array
{
    $root = base_path($this->agent->skillsPath());

    if (! is_dir($root)) {
        return [];
    }

    $known = array_merge($skills->keys()->all(), $previouslyTrackedSkills);

    return array_values(array_diff(scandir($root) ?: [], $known, ['.', '..']));
}
  3 entries in .claude/skills are not managed by Boost and were left alone:
    boost-testing, migrate-boost-guidelines, old-flux-skill

  Remove any stale ones manually, or move custom skills to .ai/skills to have Boost track them.

To be clear on what I'm not disputing: the underlying annoyance is real and manually rm -rf-ing a generated directory to recover is a bad experience worth fixing. The disagreement is only over whether the fix should delete files Boost can't prove it wrote — and #6 is good work that deserves to land either way.

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.

2 participants