feat(skills): remove the deepctl 0.3.x skill files deepctl can prove it wrote

[dg-coreylweathers]

Summary

Older deepctl releases (0.2.16 through 0.3.2) wrote Deepgram skill files straight into each AI tool's config folders. The current dg skills installs skill folders instead, so a user who upgrades ends up with both: the old files and the new folders.

This PR removes those old files, but only where deepctl can prove it wrote them. It runs right after a tool's new skill folders have landed, from dg skills install, dg skills update, dg login, and dg plugin install, update and remove (their skills refresh). Anything deepctl can't prove is kept, with one warning.

Fixes finding B8 from #111: that version deleted any legacy file whose frontmatter said name: api, which a user's own file could match. Frontmatter is never used as proof here.

Stacked on #125 (base goal/bg-3-login-plugin-installer).

Review response (Greg, head b527195)

• B1 (a save between the re-read and os.replace was overwritten): fixed. The shared-file cleanup no longer uses os.replace. It writes the cut copy to a .deepctl-v03-<uuid>.tmp file, moves the file aside to .deepctl-v03-<name> with the no-replace rename, re-proves the aside, and publishes the cut copy with the no-replace rename too. If anything was saved at the path after the move, the publish is refused: the save stays, the earlier file (section included) stays in .deepctl-v03-<name>, and E37 says to compare them before deleting it (E4's "move it back by hand" would have put the older file over the save, so it is not used here). Regression test: an editor saves atomically right after the re-proof (test_a_save_between_the_proof_and_the_publish_is_kept); it fails on b527195.
• Interruptions in the new window: the path is absent for the time of one rename, a re-read and one rename. Ctrl-C right after the move-aside rename returns is caught (the rename is inside the put-back try, and the put-back does nothing if nothing moved). A process killed after the move (SIGKILL, SIGTERM, SIGHUP, power loss) leaves the file in .deepctl-v03-<name>; the aside name is the same on every run, so the next install or update puts it back with the no-replace rename (E39) and carries on. If both the file and its aside exist, deepctl changes neither and names both on each run (E38, or E42 if the aside isn't a regular file, so it isn't called an earlier version). Tests cover Ctrl-C, SIGKILL, SIGTERM and SIGHUP at every step, for every tool.
• B2 (cleanup followed linked parent folders): fixed. Every folder from below the home folder down to the file must be a real folder. On POSIX each one is opened with O_NOFOLLOW from the one above, and every stat, read, temp, rename (renameat2 / renameatx_np with the folder fd), unlink and rmdir is relative to the last folder's fd, so a folder swapped for a link after the check is never followed. Windows has no folder fds, so it checks every folder for a link or junction again right before each change. A linked ~/.claude, ~/.claude/commands, ~/.claude/commands/deepgram, ~/.cursor[/rules], ~/.cline[/rules], ~/.codex, ~/.gemini or ~/.opencode leaves the target byte-identical, with one E40 per file naming the link (for GEMINI.md, instructions.md and agents.md, E40 says to remove only the section's marker lines and what is between them, as E34 does). A link that appears later in the run (on Windows, which re-walks before each change) gives E35 (E35b if the file couldn't be put back) instead, and the file stays tracked: it is kept out of the prune of records whose files look gone, since a link can hide the file. If the file was already moved aside, deepctl still tries to put it back, and E4 names the aside if it can't. The home folder itself may be a link (/home/x -> /data/x). Tests: a link at every folder level; a folder swapped for a link after the walk, and between a folder's check and its open; HOME a link; the Windows branch with a junction swapped in mid-run.
• S1 (README overstated the plugin trigger): fixed. It now names dg plugin install, update or remove.
• Also fixed: a failed delete of the aside puts the file back (E35 stays true; test_e35_on_unlink_failure_install_succeeds is unchanged); mypy passes for linux, darwin and win32; _rename_excl keeps renamex_np when no folder fd is passed, so _place and feat(skills): install and remove only skill folders deepctl owns #124's tests are unchanged and relative paths work on macOS.

What gets removed, and the proof for each

Tool	Path	Proof
Claude Code	~/.claude/commands/deepgram/{api,docs,setup-mcp,starters}.md	The bytes are exactly one allowlisted deepgram/skills version of that skill
Cursor, Cline	~/.cursor/rules/deepctl.mdc, ~/.cline/rules/deepctl.md	The bytes are allowlisted versions joined with \n\n---\n\n in deepctl's order (api, docs, setup-mcp, starters), each skill at most once, any of them missing (a failed fetch in 0.3.x dropped it)
Codex, Gemini CLI, OpenCode	~/.codex/instructions.md, ~/.gemini/GEMINI.md, ~/.opencode/agents.md	Exactly one complete section from the <!-- BEGIN deepctl CLI Reference (auto-generated by deepctl) --> line to the <!-- END deepctl CLI Reference --> line, each marker on a line of its own. Only that section, and the one blank line 0.3.x put before it, are removed. The rest of the file is kept byte for byte. A file that held only the section is deleted

Standalone files must use one line-ending style (all LF, or all CRLF from a Windows UTF-8 install). Mixed endings never prove.

~/.claude/commands/deepgram is removed only if it ends up empty and is not a link.

Why an allowlist of upstream history

0.2.16 to 0.3.2 did not generate these files. At install time they downloaded skills/<name>/SKILL.md from deepgram/skills main, a moving branch, and wrote it verbatim. So the content depends on when the user installed, not on the deepctl version. The allowlist is every SKILL.md blob main served after 0.2.16 was published: 29 blobs. packages/deepctl-core/tests/unit/fixtures/legacy_v03/allowlist.tsv traces each one to its upstream commit, push time, replacement time and the releases that could have fetched it. Its live_releases column lists the releases that were current on PyPI while main served the blob (any 0.2.16 to 0.3.2 install could fetch it); the TSV header and regen_allowlist.py say the same. regen_allowlist.py in the same folder rebuilds it from a clone of deepgram/skills plus gh api .../activity (it fails on any force-push) and PyPI upload times. One blob was excluded: setup-mcp 3356 bytes was replaced 36 minutes before 0.2.16 reached PyPI, and earlier releases fetched skills/mcp instead.

Allowlist upkeep: main keeps moving while releases are held, so rerun regen_allowlist.py right before this is marked ready and again before the release that ships it.

What stays, with one warning

Edited files, links (dangling included), files in a folder reached through a link (such as a dotfiles ~/.claude), folders at a file path, files over 16 MiB, files with mixed line endings, files from a deepgram/skills version newer than the allowlist, sections that are incomplete, repeated, nested, out of order or not on lines of their own, and shared files with another hard link, another owner, no write permission (os.access(path, os.W_OK) false, or no write bit in the mode, which os.access ignores for root) or an immutable flag (UF_IMMUTABLE/SF_IMMUTABLE, read from st_flags where it exists). The warning prints once, and only if 0.3.x's record in skills.json lists the file. deepctl then stops tracking it, so later runs are silent. Under --quiet the warning can't be seen, so the file stays tracked and the next run without --quiet prints it once.

In a shared file, everything between the two marker lines is removed with no check of its body, including anything the user changed there. The README says so.

Files whose replacing folder didn't install (for example a --ref bundle without api) are kept silently and stay tracked. A shared file with no markers at all is left alone silently. Amazon Q Developer and Aider files are kept: those tools have no skill folders, so nothing replaces them.

I/O failures (permission denied, disk full, a file in use on Windows) stay tracked, so the next install or update retries. E35 prints only for a path in 0.3.x's record, so a user who never ran 0.3.x doesn't get a repeating warning about, say, an unreadable ~/.codex.

Messages (verbatim)

On success, on stderr. Only files deleted outright count as removed files; a shared file whose section was cut gets its own line:

Removed deepctl 0.3.x files for {display}: {paths}.

Removed the deepctl 0.3.x section from {path}; the rest of the file is unchanged.

New warnings, on stderr:

• E33: deepctl can't prove it wrote {path} ({why}), so it left it in place and no longer tracks it; if it's an old deepctl 0.3.x copy you don't need, delete it.
• E34: deepctl can't safely remove its 0.3.x section from {path} ({why}), so it left the file as it is and won't warn about it again; a later install or update removes the section once it can, or remove only the lines from '' yourself and keep the rest of the file.
• E35: Could not remove deepctl 0.3.x content from {path}: {reason}; {path} is unchanged and still recorded, so the next install or update tries again.
• E35b, the same failure after E4, E37 or E41, or when the next run can't put back an interrupted run's aside: Could not remove deepctl 0.3.x content from {path}: {reason}; it is still recorded, so the next install or update tries again.
• E36: The skills for {display} are installed, but deepctl could not finish removing its 0.3.x files: {reason}; the next install or update tries again.
• E37: {dest} was saved while deepctl was removing its 0.3.x content, so deepctl kept your save; the earlier version is in {aside}. Compare them before you delete {aside}.
• E38: {aside}, left by an earlier deepctl run, holds an earlier version of {dest}, so deepctl changed neither; compare them, keep what you want in {dest}, then delete {aside}.
• E39: {dest} was missing, so deepctl put it back from {aside}, where an interrupted deepctl run had moved it.
• E40: {why}, so deepctl left {path} as it is and won't warn about it again; if it holds deepctl 0.3.x content you don't need, remove that content yourself.
• E41: {aside} is no longer the file deepctl moved there (a link or folder is there now), so deepctl did not put it back and {dest} is missing; restore {dest} from a backup if you need it, then delete {aside}.
• E42: {aside} is not a file deepctl moved there, so deepctl changed neither it nor {dest}; delete {aside} if you don't need it.
• E40 for GEMINI.md, instructions.md and agents.md: {why}, so deepctl left {path} as it is and won't warn about it again; if it holds deepctl 0.3.x content you don't need, remove only the lines from '' yourself and keep the rest of the file.

The {why} values are: "it differs from every deepgram/skills version deepctl 0.3.x copied", "it is a link", "it is not a file", "it is larger than 16 MiB", "it changed while deepctl read it" (it changed between deepctl's check and its read), "it mixes line endings", "its deepctl section is incomplete, repeated or not on lines of its own", "it has other hard links or another user owns it", "it is read-only or locked", "it changed while deepctl was removing it", "it changed while deepctl was editing it", and "deepctl's temporary copy of it was replaced" (the temp was no longer the file deepctl wrote right before the publish; the file itself didn't change). E40's {why} is "{folder} is a link, which deepctl doesn't follow". E40 is used only when the first walk to the file finds the link, before anything moved.

E37 and E38 print whether or not 0.3.x recorded the file, and a file that gets E37 gets no E34; the aside itself shows deepctl moved it. If a moved-aside file can't be put back because a file was saved at its name meanwhile, E37 says to compare them (moving the aside back by hand would replace the save). For any other put-back failure, the existing E4 is reused: "{dest} changed while deepctl was replacing or removing it, so what was there is now in {aside}; move it back by hand if you need it." If the aside is no longer a regular file (a link or folder was swapped in), it is not put back and E41 says so instead. After E4, E37 or E41 for a failed re-proof, no E33 or E34 follows for that file, and it is no longer tracked, since E4, E37 or E41 already named the aside. After E4, E37 or E41 for an I/O error, and when the next run can't put back a file an interrupted run moved aside (E39 fails), E35b follows (it doesn't say the file is unchanged) and the file stays tracked; when the file is back in place or was never moved, E35 says it is unchanged. When a file sits next to an aside that isn't a regular file (for example after E41, once the user restores the file), E42 prints on each run instead of E38.

dg skills status, when 0.3.x files are still recorded:

Files from deepctl 0.3.x are recorded for {tools}; 'dg skills install' or 'dg skills update' removes the ones deepctl can prove it wrote once the skill folders are installed.

dg skills remove, for a folder tool whose recorded 0.3.x files are still on disk (checked with lstat), for a tool whose recorded GEMINI.md, instructions.md or agents.md is still on disk, for a hint-only tool, and for Aider. Once every recorded 0.3.x file is gone, it says nothing about 0.3.x files. A shared file is never called a deepctl file or offered for deletion, since the user's own text is in it. remove drops the tool's records, so the line doesn't claim the files are still recorded. Keeping the record was considered and rejected: dg skills update and the dg plugin refresh install every tool in installed_skills, so a kept record would bring back the folders the user just removed.

For {display}, its deepctl 0.3.x files were kept: {paths}; delete any you don't need, or 'dg skills install' removes the ones deepctl can prove it wrote.

For {display}, the deepctl 0.3.x section in {path}, if any, was kept; 'dg skills install' removes it when it can do so safely; if it is still there afterwards, remove the lines between its marker lines yourself.

{display} has no skill folders, so nothing was removed and its deepctl 0.3.x file is kept; delete it yourself if you don't need it.

Aider has no skill folders, so nothing was removed and its deepctl 0.3.x file is kept; delete it and its entry under 'read:' in ~/.aider.conf.yml yourself if you don't need it.

A folder tool with no folder records prints "{display} has no skill folders recorded, so nothing was removed; " followed by the same its deepctl 0.3.x files were kept... or the deepctl 0.3.x section in... text as above, or "{display} has no skill folders recorded, so nothing was removed." when no recorded 0.3.x file is left on disk.

Commits

1. feat(skills): prove and remove deepctl 0.3.x skill files after the new folders land: the standalone files, the records, the dg skills status and remove text, and their tests.
2. feat(skills): remove the deepctl section from GEMINI.md, instructions.md and agents.md: the shared files, the remove text for a kept section, and their tests.
3. docs(skills): explain the 0.3.x upgrade cleanup: the README.

Safety

• Every delete cites its proof. A file is renamed aside (same folder, .deepctl-v03-<name>, a no-replace rename), re-read and compared with the proven bytes, then unlinked. If the re-read differs, or the unlink fails, it is put back, never over a file that appeared meanwhile, and only if the aside is still a regular file. If a file was saved at the name meanwhile, E37 says to compare the two; if the put-back fails otherwise, E4 names the aside; if the aside is no longer a regular file, E41 says the file is missing. None is followed by a line saying the file was left in place.
• Shared files are never replaced over a newer save. The cut copy goes to a .deepctl-v03-<uuid>.tmp file in the same folder (O_EXCL, O_BINARY), fsynced, with the original's mode and times, set through the open file on POSIX so a link swapped in for the temp is never followed. Right before the publish, deepctl checks that the temp is still the regular file it wrote (same type, mode, inode and device); if not, the file is put back and kept (E34, "deepctl's temporary copy of it was replaced"). That check stops a link or another file swapped in for the temp before it; the Residual bullet covers what it can't stop. The original is moved aside and re-proven, then the cut copy is published with the no-replace rename. A save that landed at the path in the meantime makes the publish fail, and E37 names both copies. The temp is deleted whenever it wasn't published. An owner check (POSIX), a hard link check and a write-permission and immutable-flag check run first. File flags and xattrs are not copied to the cut copy (shutil.copystat is gone, since it can't take a folder fd); immutable files were already refused.
• No links between home and the file. Every folder below the home folder is checked; on POSIX the folders are opened O_NOFOLLOW down an openat chain and every operation is relative to the final folder's fd; Windows checks each folder again right before each change (a junction swapped in during the microseconds between that check and the change is the remaining window there); a link found by that re-check gives E35, not E40, and the file stays tracked (even though the link may hide it from the record prune). A moved-aside file is still put back, or E4 names the aside, including when the re-check fails while deepctl is checking the temp before the publish. The home folder itself is trusted, as it is for skills.json and every skills root.
• Nothing runs if the install failed. The hook sits after install_tool's raise error, inside its skills.json lock.
• The cleanup can't fail an install that landed. Any error becomes E36 on stderr. Only Ctrl-C propagates, after a moved-aside file is put back or, once the cut copy is published, left for the next run to name (E38).
• Everything prints on stderr, so dg plugin ... -o json stdout is unaffected. print_info gains stderr=, mirroring print_warning.
• skills.json is written only when something changed, through _update_state. The installed_skills.<tool> key is kept (login and the startup check key on it); its paths fall back to the folder paths. skill_folders.<tool>.v03 is popped once no legacy path is left; dg skills remove mentions 0.3.x files only while a recorded one is still on disk.
• Names come from a fixed table, joined with Path.home().joinpath(*parts). Nothing is globbed or swept by name.
• Interrupted runs: the aside name is the same on every run, so after a kill (SIGKILL, SIGTERM, SIGHUP, power loss) between the move and the publish or unlink, the next run puts the file back with the no-replace rename (E39) and carries on (if that put-back fails, E35b says it is still recorded, not that the file is unchanged, and the next run tries again); if both the file and the aside exist, it changes neither and names both (E38, or E42 if the aside isn't a regular file). A kill while the cut copy is being written leaves a .deepctl-v03-<uuid>.tmp (never the user's only copy); the README says to delete it. Nothing is swept by name.
• Residual: a writer that already holds the file open and writes between the re-proof and the unlink of the aside loses that write (microseconds; editors and these tools don't hold the file open). A Ctrl-C or kill in the instant after the state write and before the warnings print can drop a one-time E33/E34/E40 for a kept file; this follows feat(skills): install and remove only skill folders deepctl owns #124's untrack-then-report pattern, and the file is kept. On Windows the temp's mode is set by path while it is open, and its times by path after it closes, before the move (there is no fd utime there); a link swapped in for the temp in that gap would have its target's times set, and the temp check then refuses the publish. A process running as the same user that races deepctl's random temp name and fixed aside name inside the microseconds between the temp check and the publish, or between the publish and the aside's unlink (Linux can reuse an inode number for a recreated temp), can get its own file published at the path and the original deleted; such a process can already overwrite GEMINI.md directly, so this is not a privilege boundary. On Windows, if a folder becomes a link after the move, the moved-aside file stays in the folder that moved; that run's E4 names it (at its old path), but later runs print E40 for the link and don't name the aside.
• Windows: no literal separators; the rename refuses an existing destination; there is no owner check (st_nlink is still checked); links and junctions count as links.

Design decisions

1. Allowlist source. The spec asked for hashes of each 0.3.x tag's generator output. The tags generate nothing (their render path is dead code); they copied deepgram/skills main at install time. The allowlist is the 29 upstream blobs instead, each traced to a commit, a push time and the releases that fetched it.
2. Amazon Q and Aider files are kept. Those tools are hint-only (no skill folders), so nothing lands to trigger cleanup, and Aider's read: entry would dangle. The remove text says the file is kept.
3. "Byte for byte" for shared files is exact for user text ending in one newline. 0.3.x had already normalized CRLF and trailing newlines when it wrote the file, and the one blank line it added before its section is removed. Other shapes are unit tests with documented results.
4. An unprovable file is warned about once, then no longer tracked, so later runs are silent.
5. Shared files are marker-only: one complete, line-anchored section is removed with no check of its body. A file with no markers is a silent no-op.
6. Source budget is 380 lines (added + deleted). Corey accepted up to about 380. See Budget.

Tests

• packages/deepctl-core/tests/unit/test_legacy_v03.py (235 tests, 91 before Greg's review): the allowlist matches the TSV trace (29 rows, each with a release); the fixtures regenerate from a verbatim quote of v0.3.2's writer (fixtures/legacy_v03/v032_writer.py) to the captured hashes (joined c6302c78…, section-only b6158ef6…); the B8 regression (a user's api.md with name: api frontmatter, a one-byte edit, a same-size edit, mixed endings, an empty file, another skill's text, trailing separator) survives byte for byte with one warning and a silent second run; joined files in LF and CRLF with partial fetches; reordered or repeated joins kept; seven shared-file shapes; eight malformed-section shapes kept; links, dangling links, folders, oversized files; hard links and foreign owner; mode, mtime and CRLF bytes kept on a cut; a chmod-000 parent gives E35 and stays recorded, and is silent when 0.3.x didn't record the file; a --quiet run keeps an unprovable file tracked and the next run warns once; a read-only shared file, one whose os.access says writable (as for root), and on macOS a uchg one, is kept with one E34, the second run is silent and no temp file is left; a refused publish (EACCES) puts the file back with E35 and leaves no temp file or open fd; a full disk while writing the temp leaves the file; a cut section prints the section line, not the files line, even if the file disappears right after the cut; a moved-home record is pruned; the stale v03 flag is popped; a failed install (swap failure, failure after three folders landed, ownership refusal, settle write failure, lock timeout) leaves every 0.3.x file byte and inode identical; E35 on unlink and on publish; E36 when the hook fails; the re-proof failure puts the file back; E4 when the put-back fails; Ctrl-C during the move; the aside and temp prefixes (the aside name is the same on every run); no skills.json write when nothing changed; stderr only outside agentic mode; the Windows branch.
• test_skills_legacy_v03.py (dg skills remove names a kept 0.3.x file and says nothing about 0.3.x files once it's gone; a kept GEMINI.md section is called a section, not a deepctl file), test_plugin_legacy_v03.py (dg plugin remove foo -o json: the cleanup stays off stdout; in agentic mode stdout is empty), test_login_legacy_v03.py (the login skills step cleans up on stderr).
• New for Greg's review (TestLinkedFolders, TestInterrupted, TestSharedRaces): a link at every folder level for all six tools (target byte-identical, one warning naming the link, silent second run, no fd left open); HOME itself a link; a folder swapped for a link after the walk, and between a folder's check and its open; a file where a folder should be; the Windows branch with a linked folder and with a folder swapped for a link mid-run; _rename_excl with relative names at the cwd and at a folder fd; Ctrl-C right before the move, after the move, after the re-proof, after the publish and after the aside's unlink, for every tool (nothing lost, and no E4 once the cut copy is published; the next run finishes or names what it kept); SIGKILL, SIGTERM and SIGHUP at the same steps in a forked child (the next run puts the file back with E39, or names both copies with E38); a temp swapped for a link to an outside file right before its chmod and right before the publish (the outside file's mode and time unchanged, GEMINI.md never a link, the file put back); an editor's atomic save right after the re-proof (E37, the save and the earlier file both kept, E4 and E34 not printed, E38 on the next run); a failed aside delete after a publish keeps the cut (E38 next run); a section-only file whose aside can't be deleted is put back with E35; a put-back that meets a newly saved file never replaces it; an aside that is a folder or a link is never put back, at the start of a run or after a failed re-proof (E41, with no E33 after it); a temp swapped for a link gives E34 with "deepctl's temporary copy of it was replaced", not "it changed"; a linked folder in front of GEMINI.md, instructions.md or agents.md gets E40 with E34's marker-line ending, and in front of the other files E40's "remove that content yourself"; on the Windows branch, a link swapped in mid-run (after the move, after the re-proof, right before the publish, and a dotfiles-style link to where the folder went) gives E4 and E35b with no "left in place" line, never touches the link's target, and keeps the file tracked. Also: an aside swapped for a link before a failed delete gives E41 then E35b; after E41, a file the user restores gets E42 on each run, never E38; E35 says the file is unchanged after a refused delete (EACCES), a full disk (ENOSPC) and a failed publish whose temp vanished; on the Windows branch, Ctrl-C right after the publish, with a link swapped in at the same moment, still raises Ctrl-C (not an OSError from the temp check) and E4 names the aside; a put-back refused because a file now sits at the name gives E37, not E4; when the next run can't put back an interrupted run's aside (permission denied, or a file saved there), the warning is E35b and never says the file is unchanged; a file that changes between deepctl's check and its read is reported as changed, not as too large. Assertions on a real OS refusal compare with that exception's own text, since Windows words it differently.
• Ten existing tests mocked removed internals (_place, _v03_drop, _v03_rewrite, tempfile.mkstemp, shutil.copystat, os.replace) and now patch _rename_excl, _read_regular, _v03_mv and os.open with the same on-disk assertions. test_e35_on_unlink_failure_install_succeeds is unchanged. The replace-failure test no longer gives the temp a read-only mode and uchg flag, because nothing copies flags to the temp any more.
• Four existing tests that asserted 0.3.x files and records are always kept now seed a file whose folder doesn't land, so they still cover the kept case.
• Mutation testing for Greg's review, rerun on the final head: 38 mutants, all killed: the move-aside outside the put-back try, the aside unlink outside it, no restore on the next run, os.replace for the publish, no O_NOFOLLOW, no link check, E4 in place of E37, no E38, a put-back that doesn't check the aside exists, no Windows re-check, the re-proof skipped, no chmod or no utime on the temp, E37 for any publish error, a non-file aside put back, no temp cleanup, E34 after E37, a put-back after the publish, the link case warned as E33/E34, the link case left tracked, the temp's mode and times set by path, no temp re-check before the publish, an aside of any kind put back, a link found mid-run reported as E40, the swapped-temp reason replaced by "it changed", E40's marker-line ending dropped for shared files or used for every file, E35 paths pruned from the record, a put-back after a failed publish that waits on the temp check, E4 in place of E41, E33 printed after E4 or E41, E35 printed after E4 or E41 (or E35b without them), E38 for an aside that isn't a file, the temp check before the put-back on Windows allowed to raise over Ctrl-C, E4 in place of E37 when a save blocks the put-back, a failed E39 put-back reported as E35, and a file that changed before the read reported as too large.
• Earlier mutation testing: 23 targeted mutants of the matcher, the marker parser, the installed-folder gate, the hook's place after the error check and the record prune. All 23 are killed. Seven more cover the write-permission check, the whole lock check, the temp's flag reset, the section line, the quiet gate and the E35 record gate; 6 of those are killed. Removing only the st_flags half survives on macOS, because os.access already reports a uchg file as not writable there; the st_flags check is kept as a second guard.
• Upgrade check (Docker): a HOME written by origin/main's real dg skills install --all (shared files seeded with LF user text ending in one newline), then this branch's dg skills update. Every 0.3.x file of the six folder tools is gone (three "Removed deepctl 0.3.x files" lines and three section lines on stderr), each shared file is byte-identical to its seed, all 14 folders exist with markers for each of the six tools, the Amazon Q and Aider files and records are unchanged, skills.json has no v03 and no legacy paths, update's stdout has no cleanup line, and dg skills remove --all mentions 0.3.x only for Amazon Q and Aider (Aider's line names its entry under read:). Variants: two older upstream eras (316940a and 25bb74c) written through the quoted v0.3.2 writer, including a partial fetch, are fully removed; an api.md with one byte changed is kept, warned about once, untracked, and the second update is silent; an edited api.md that stays tracked under --quiet is named by dg skills remove --cli claude and left on disk.
• Data-safety review matrix (macOS, and Docker Linux as uid 1000 and root; rerun on the final head in Docker): a probe injects an editor's atomic replace, an in-place O_TRUNC write and an in-place create at each of the 21 editor-write steps of the shared-file cleanup, swaps the temp for a link to a file outside the folder right before its chmod and its utime, checks E38 and E39 recovery, and SIGKILLs the run at each of 23 steps for Claude, Cursor and Gemini (69/69 pass). 129 of 135 cases pass, as uid 1000 and as root. Both temp-swap cases pass (the outside file's mode and time are unchanged; they failed on 79fbd8f). Every atomic-replace and in-place-create step passes. The 6 that fail are in-place O_TRUNC writes at steps 14 to 19; they are probe no-ops: the probe opens GEMINI.md by path while the file sits in the aside, so it writes nothing to the user's file. The held-open-writer residual above was not exercised by this probe. Older probe cases that hooked removed internals (such as shutil.copystat) were retired; their behaviors are unit tests now.
• Upgrade check rerun on e19664e: 68 of 68 rows pass, with no FAIL (the dg skills remove row now expects the current wording, "removes the ones deepctl can prove it wrote"). Three more scenarios pass (10 checks): a dotfiles-linked ~/.claude keeps all four 0.3.x files byte-identical with four E40 warnings, each naming the link, and a silent second update; a linked HOME cuts the GEMINI.md section; an aside left by a killed run is put back (E39) and cut.
• Merge check with fix(skills): keep a newer skills ref and a removed tool when a refresh overlaps #126 (b0b1595): git merge-tree is clean in both orders and gives the same tree. On the merged tree of the final head, ruff, mypy (linux, darwin, win32) and the core, skills, login and plugin tests pass (1154 passed, 6 skipped). Earlier, on the merged tree of eeb36f4, the full gate was green (2089 passed), the upgrade check passes, the E-codes don't collide (fix(skills): keep a newer skills ref and a removed tool when a refresh overlaps #126 uses E30 to E32, this uses E33 to E42; rechecked on the final merged tree), and a dg skills update and a plugin refresh over two tools with 0.3.x files install both tools with no E30, E31 or E32 skip.

Gate

Docker ghcr.io/astral-sh/uv:python3.12-bookworm-slim: ruff format --check, ruff check, mypy and the full pytest suite as root on the final head (2213 passed, 12 skipped); on e19664e, the core tests as uid 1000 (939 passed, 3 skipped) and on Python 3.10 (936 passed, 6 skipped). Each commit passes ruff format --check, ruff check, mypy --platform linux, darwin and win32, and the core, skills, login and plugin tests on its own (C1: 1020 passed; C2 and C3: 1135 passed). On Linux, test_legacy_v03.py also passes with the text of every real OS error rewritten, so no assertion depends on POSIX error wording. On the macOS host (temp homes only), the same suites passed for e19664e's C1 (1016 passed) and head (1128 passed). git diff --check against the base is clean. Fixtures are marked -text in .gitattributes so Windows checkouts keep their exact bytes.

Budget

Source lines (git diff --numstat against the base, excluding tests): 380 (added + deleted), within the about 380 Corey accepted; 365 added only. Per commit: C1 298, C2 132, C3 8 (C2 also rewrites C1 lines).

File	Lines (added + deleted)
packages/deepctl-core/src/deepctl_core/skill_generator.py	350 (40 of them the allowlist and its comment, one blob per line)
packages/deepctl-cmd-skills/src/deepctl_cmd_skills/command.py	17
packages/deepctl-core/src/deepctl_core/output.py	4
README.md	8
.gitattributes	1

🤖 Generated with Claude Code

[GregHolmes]

Review: deepgram/cli #130, feat(skills): remove the deepctl 0.3.x skill files deepctl can prove it wrote

Classification: mixed (code-led)
Verdict: request-changes

Intent

After new skill folders install successfully, remove only the legacy 0.2.16-0.3.2 skill files or shared-file sections that deepctl can prove it wrote. Preserve all user content and make cleanup non-fatal to a successful folder install.

Blocking

[B1] packages/deepctl-core/src/deepctl_core/skill_generator.py:523-524 Shared-file cleanup can overwrite a concurrent user save

Summary: _v03_rewrite builds a section-stripped temp file, then checks that the live file still equals the bytes it originally read. It publishes with unconditional os.replace, however. Editors commonly save by atomically replacing the file, so a user save between the equality check and os.replace is silently overwritten with the stale temp file.

Expected: A cleanup must never replace a user edit that appears at any point after the legacy content was proven.

Observed: A user who saves GEMINI.md, instructions.md, or agents.md immediately after _read_regular(path, ...) == data passes can have that new file replaced by the cleanup's old, section-stripped temp file.

Recommended fix: Use the existing no-replace protocol: atomically move the proven source to a private aside with _place, re-prove the aside, then publish the temp with _place only while the destination remains absent. If a new destination appears, retain the aside and warn rather than overwriting the user's file. Add a regression test that atomically replaces the source between the re-proof and publish steps.

[B2] packages/deepctl-core/src/deepctl_core/skill_generator.py:542-549 Cleanup follows symlinked parent directories

Summary: The cleanup rejects a symlink at the legacy file itself, but it never verifies the path components between Path.home() and that file. If, for example, ~/.gemini is a symlink, GEMINI.md can be regular while _v03_rewrite creates a temp and replaces a file in the symlink target. The same path traversal affects standalone-file deletion.

Expected: Legacy cleanup must operate only on non-symlink path components under the user's home directory; a symlinked parent must leave the legacy content untouched.

Observed: os.lstat(path) and _read_regular(path, ...) validate only the final file. tempfile.mkstemp(dir=path.parent), os.unlink, and _place follow a symlinked parent and can alter a file outside the configured tool directory.

Recommended fix: Before any read, temp creation, rename, unlink, or state pruning, reject a symlink in every component from the home directory through the legacy file's parent. Use opened directory file descriptors with no-follow semantics for the mutation path as well, so a parent-directory swap cannot bypass the validation. Add tests for a symlinked .gemini/.cursor parent that prove the target remains unchanged.

Should-fix

[S1] README.md:324 Plugin cleanup trigger is overstated

Summary: The upgrade guide says a dg plugin command installs folders and triggers cleanup. Only successful dg plugin install, dg plugin update, and dg plugin remove call _maybe_update_skills; commands such as dg plugin list do not.

Expected: The upgrade guide must name only plugin subcommands that can trigger legacy cleanup.

Observed: A developer can run another plugin command expecting it to clean legacy files, but it never enters the skills refresh path.

Recommended fix: Replace "a dg plugin command" with "dg plugin install, dg plugin update, or dg plugin remove".

Nits

• None.

Verified

• Exact blob and joined-file matching avoids the original B8 false-positive class: frontmatter alone is not treated as proof.
• Cleanup is invoked only after install_tool finishes without an error and remains inside the skills state lock (packages/deepctl-core/src/deepctl_core/skill_generator.py:943-946).
• Targeted cleanup tests pass: 97 passed, 3 skipped.
• Docker full suite passes: 2069 passed, 12 skipped; mypy reports no issues.
• git diff --check passes.

Needs human

• None.

Developer-facing messaging

• Until B1 and B2 are fixed, this should not ship as a safety-preserving upgrade cleanup: developers can lose a concurrent shared-file edit or have content modified through a symlinked tool configuration directory.