fix(skills): login and plugin install skills through the shared installer

[dg-coreylweathers]

Stacked on #124 (d1cf0f0) until it merges.

Commits, each passing ruff, mypy and the core, skills, login and plugin tests on its own:

1. refactor(skills): share one fetch, preflight and install loop from core
2. fix(skills): install skills from login and plugin through the shared installer
3. refactor(core): delete the install() shim and the code only it used

Greg's review (2026-10-07)

• B1, core floors. Not changed in this PR. The fix needs a floor above deepctl-core 0.2.18, which is both the workspace version and the latest on PyPI (it does not have install_for or warn_install_failure). check_dependency_floors.py rule 2 fails any floor above the workspace version, so the raise has to happen in the release PR after release-please bumps core. See Release checklist below.
• S1, quiet and stdout. The "AI assistant skills updated" success line is removed. A successful refresh now prints nothing, and failures still warn on stderr through print_warning, which respects --quiet. New CLI test test_successful_refresh_keeps_skills_off_stdout_and_quiet_silent runs plugin install through CliRunner in non-agentic mode, with and without quiet: nothing about skills on stdout or stderr, and under quiet both are empty.
• S2, README. The sentence is now your wording: "A skills installation failure prints a warning on stderr and does not change those commands' exit codes. Ctrl-C still cancels the command."
• S3, CRLF. output.py is CRLF on main (as are 7 other .py files), and ruff format rewrites mixed endings back to CRLF, so converting only the added lines fails the format check. A new one-line .gitattributes rule sets whitespace=cr-at-eol for that file. git diff --check origin/goal/bg-2-skills-folder-install...HEAD is now clean when run from this branch's checkout, which carries the new .gitattributes, and no file is reformatted.

What

dg login's skills step and the skills refresh after dg plugin install, update and remove now install through install_for, a new shared helper in deepctl_core.skill_generator. It is the same fetch-once, preflight-every-tool, record-as-each-tool-lands loop dg skills install and update use. SkillsCommand._install now calls it too, so there is one copy of the loop.

The SkillGenerator.install() compatibility shim from #124 is deleted, along with save_skills_state and the 0.3.x code only the shim used.

Closes

Greg's earlier B3 on #111 (review of 2026-09-22):

• One shared install path. Login, the plugin refresh and dg skills all go through install_for.
• Full-target preflight. Every tool is checked before anything is written, so one conflict means nothing is written for any tool.
• No folder without a record. Login and plugin write nothing themselves. Every folder and record write is feat(skills): install and remove only skill folders deepctl owns #124's install_tool (record before swap, then settle).
• Two-tool failure test. When the second tool fails, the first stays recorded.
• Failures are reported, not swallowed. except Exception: pass is gone from both commands.

Behavior

Otherwise, behavior matches dg skills install (login) and dg skills update (plugin refresh): same records, same per-tool stop on the first failure, same ownership refusals (E1, E22). The differences:

• Warnings go to stderr. A failure prints a plain warning, through a new print_warning(..., stderr=True) so stdout stays clean outside agentic mode too. If staging was left behind, an E12 line comes first.
• The warning names the retry command. Rerunning login or a plugin command does not retry the skills step, so the warning ends with run 'dg skills install' to try again (login) or run 'dg skills update' to try again (plugin). It replaces the core message's "then run the command again" in every sentence, so a warning that lists several folders names the retry command for each one. dg skills messages are unchanged. For example:
  ⚠ AI assistant skills were not updated: deepctl cannot prove it installed /home/you/.cursor/skills/api, so it will not replace anything there; move or rename what is there, then run 'dg skills update' to try again.
• Exit codes don't change. A skills installation failure never changes the login or plugin exit code. Ctrl-C still cancels the command and exits 2, as on main.
• Success is silent. A successful plugin refresh prints nothing.
• Hint-only tools are silent in the refresh. Amazon Q and Aider are skipped without a message during the plugin refresh, so they don't add a warning to every plugin command. Login still prints E15 for them on stderr if you select them.
• Login and plugin share feat(skills): install and remove only skill folders deepctl owns #124's lock. They install through install_tool, which holds skills.json.lock for each tool's whole install, so they wait up to 30 seconds for another dg skills, dg login or dg plugin command, then warn (E27) without changing the exit code. The README's lock sentence now names all three.
• The lock warning says "stopped". feat(skills): install and remove only skill folders deepctl owns #124's E27 ends "so this one waited 30 seconds and changed nothing". Login and the plugin refresh may already have installed earlier tools when they hit the lock, so their warning says "waited 30 seconds and stopped" instead. dg skills keeps feat(skills): install and remove only skill folders deepctl owns #124's wording.
• A record-save failure after install has no "not updated" prefix. When the skills are in place but skills.json could not be saved (E9b, which starts "The skills for ... are installed, but"), the warning prints E9b and the retry command without "AI assistant skills were not updated" or "Skills setup did not finish" in front.
• 0.3.x records stay unchanged. The refresh never rewrites installed_skills. Amazon Q's and Aider's entries, and every folder tool's 0.3.x paths, stay byte-identical.

What else users see:

• The login prompt is unchanged. Same text, list and default.
• Results are one line per tool: ✓ Claude Code → /home/you/.claude/skills (14 skills). Main printed one line per path, which would be 84 lines for six tools.
• No summary after a failure. "Skills installed!" prints only when the whole run finishes, as with dg skills install.
• A corrupt skills.json now warns once per command and is never rewritten. On main this was silent.
• The plugin refresh on an upgraded 0.3.x HOME installs folders for every recorded folder tool, as dg skills update does.
• Test hygiene. Plugin and login tests now point skills.json at a throwaway path. Before this, test_handle_install_git_url_detection ran the real refresh against the developer's HOME.

Done when

• One PR from goal/bg-3-login-plugin-installer, based on feat(skills): install and remove only skill folders deepctl owns #124's branch as a stacked draft.
• Source diff is 135 added lines of 200 (701 deleted). Measured by summing the added column of git diff --numstat origin/goal/bg-2-skills-folder-install...HEAD -- . ':(exclude)*/tests/*' ':(exclude)tests/*'.
• B3, same records:
  • test_login_writes_the_records_dg_skills_install_would and test_refresh_writes_the_records_dg_skills_update_would compare normalized skills.json against a real SkillsCommand run in a second HOME.
  • The upgrade check repeats this against origin/main's HOME.
• B3, second tool fails: test_install_for_second_tool_failure_keeps_first_recorded (core), test_login_second_tool_failure_keeps_first_recorded_warns_once_exit_zero (through LoginCommand.handle) and test_refresh_second_tool_failure_keeps_first_recorded_warns_once_exit_zero (through click's CliRunner). The first tool stays recorded as installed.
• Warning on stderr, stdout clean, success exit:
  • The same login and plugin tests, in agentic and non-agentic mode (login's result status stays success, the plugin command exits 0).
  • TestWarningOnStderr covers print_warning(stderr=True).
• Shim gone:
  • test_generator_has_no_install_shim checks it.
  • git grep -nE "gen\.install\(|\.install\(commands|save_skills_state" packages/*/src src prints nothing.
  • The broader git grep "\.install(" packages/*/src prints only deepctl-cmd-plugin/.../command.py:761 strategy.install(options). That is the pip/uv plugin install strategy, unrelated to skills.
• Upgrade check passes against a HOME written by origin/main's real dg skills install --all, in Docker as uid 0 and uid 1000 (details below).
• Docker gate green: ruff format, ruff check, mypy and the full pytest suite on 3.12 (1970 passed, 9 skipped); core tests as uid 1000 (716 passed, 2 skipped) and on 3.10 (715 passed, 3 skipped). Per commit: ruff, mypy and core, skills, login and plugin tests pass (874, 924 and 895 passed).
• No commit has a Claude trailer.

Mutation testing. 29 mutants of the new guards and messages, all killed:

• install_for's hint-only filter, preflight and ref dedup;
• each stderr=True;
• the auto_update, empty-plan and leftover branches;
• the warning calls and the recorded-ref lookup;
• the plugin refresh's except Exception narrowed to SkillInstallError (fetch failure and invalid recorded ref);
• login ignoring DEEPCTL_SKILLS_REF, and the per-tool line without escape();
• the retry text dropped, the "run the command again" tail kept, and each command's retry command swapped;
• the retry rewritten in only the last sentence (removesuffix), or only the first;
• the refresh skipping 0.3.x-only tools;
• the E27 "stopped" rewrite dropped, and the prefix kept on E9b.

Upgrade check

Input: the origin/main HOME archive (8 tools, 0.3.x files, user mine.md), plus a seeded ~/.claude/skills/my-skill, .api.tmp-123 and .api.old-1-2. Results were the same as uid 0 and as uid 1000, on this branch on top of #124's earlier head 4aaf3ac; since then #124 changed only remove's handling of copies left by an interrupted run, which this HOME does not trigger.

1. Login on the upgraded HOME (real pty, typing all): no prompt, because installed_skills is non-empty. skills.json is byte-identical and all 16 pre-existing files are sha-identical.
2. Plugin refresh with an npx-style ~/.cursor/skills/api:
   • exactly one E1 warning on stderr, nothing on stdout, exit 0;
   • nothing written in any root, and skills.json is byte-identical.
3. Plugin refresh, real fetch of the pin:
   • 6 tools × 14 folders, all recorded installed, with v03 set;
   • installed_skills is equal to the archive's, including Amazon Q's and Aider's entries and every 0.3.x paths;
   • stderr empty, every pre-existing file sha-identical, no staging left.
4. A second refresh: same result.
5. dg skills status, list and remove --all: remove notes the 0.3.x files for Claude Code and Amazon Q Developer, and every pre-existing file is sha-identical.
6. Login after remove (real pty, all):
   • installs 14 skills for each of the 6 folder tools;
   • E15 for Amazon Q and Aider on stderr only;
   • records equal a dg skills install --all run on a copy of the same HOME (normalized).

0.x API removal

This PR removes these public deepctl_core.skill_generator names, which no command uses after this change:

• SkillGenerator.install
• save_skills_state
• CommandMetadata
• collect_command_metadata
• skills_need_update
• render_developer_guide
• render_skill_content
• private _commands_hash and _safe_get_arguments

They are removed in a separate commit. The removal will be noted in the release PR's release notes, with no breaking-change ! marker (deepctl-core is 0.x).

Release checklist

For the release PR that ships this stack (B1):

1. After release-please bumps deepctl-core, raise the deepctl-core floor in deepctl-cmd-login, deepctl-cmd-plugin and deepctl-cmd-skills to that new core version. They call install_for and warn_install_failure, which no published core has. check_dependency_floors.py --fix only updates root floors, so this is by hand.
2. Publish deepctl-core before, or together with, the command packages.
3. Before publishing, install each of the three command packages' wheels in a clean venv with deepctl-core pinned to its declared minimum, and import the command module.

Concurrency check

A race harness using #124's real lock races a plugin refresh, dg skills update, login and dg skills install against dg skills remove for the same tool, with remove starting mid-swap and before the first swap: 8 runs on this branch, in Linux Docker as uid 1000. In every run remove waited on the lock until the install finished, 0 orphaned folders, 0 staging left, and the next dg skills install --all and plugin refresh succeeded with no warning.

Not in this PR

• Re-checking the recorded tools and ref under the lock:
  • A refresh computes its plan (tools and ref) before the fetch, as dg skills update does today. So it can bring back a tool removed after the refresh read skills.json and before it reaches that tool (during the download, while earlier tools install, or while it waits for the lock), or overwrite a --ref chosen in that window.
  • A follow-up PR re-checks the recorded tools and ref under the lock before installing, so removed tools stay removed. Its check inside install_tool, under the lock, closes this window.
  • Neither loses data on disk: install_tool re-proves ownership from disk under the lock.
• 0.3.x leftovers: a follow-up PR cleans them up.
• Deferred: keep-going past a failed tool, and retired-skill pruning.

🤖 Generated with Claude Code

[GregHolmes]

Requesting changes based on one release-compatibility blocker and three follow-ups. This is a content assessment only: the PR remains draft and stacked on #124.

[B1] packages/deepctl-cmd-login/pyproject.toml:24 and sibling command packages allow incompatible core releases

Summary: Login, plugin, and skills now call install_for and warn_install_failure, introduced here in deepctl_core.skill_generator, but each package still permits deepctl-core>=0.1.10. A resolver can install a published older core with these newly released command packages, then fail after login or a plugin operation has completed; the failure handler can also fail because the warning helper is absent.

Expected: Every independently published command package must require the first released deepctl-core version that implements every core API it calls.

Observed: deepctl-cmd-login, deepctl-cmd-plugin, and deepctl-cmd-skills retain their pre-feature deepctl-core>=0.1.10 floors while now depending on APIs first added in this stack.

Recommended fix: In the release PR, raise all three leaf-package floors to the new core release version, publish core before or alongside the command packages, and add an isolated wheel-resolution test using each declared minimum. Do not publish the command packages until that release artifact is complete.

Verified in Docker: Ruff, mypy, and the full test suite passed (1,890 passed, 8 skipped).

[GregHolmes]

[S1] Refresh status bypasses quiet mode and writes to stdout

Summary: The new plugin-refresh success message uses a module-level Rich Console directly. That bypasses the output configuration, so dg --quiet plugin install, update, or remove still prints an unrelated skills-refresh message; it also adds status text to stdout.

Expected: A best-effort skills refresh must produce no non-essential output under --quiet; when it is shown, its status output must respect the command's stdout/stderr contract.

Observed: console.print("[dim]AI assistant skills updated[/dim]") always runs after a successful refresh. The new tests assert that output exists but do not cover quiet mode or its output channel.

Recommended fix: Remove the success notification, or send it through a quiet-aware status helper on stderr. Add CLI-level non-agentic tests for successful refresh with --quiet and for stdout/stderr separation.

[dg-coreylweathers]

Fixed in 1d34a22. The success line is removed, so a successful refresh prints nothing; failures still go through the quiet-aware print_warning on stderr. New CLI test test_successful_refresh_keeps_skills_off_stdout_and_quiet_silent runs plugin install non-agentic with and without quiet and asserts nothing about skills on stdout or stderr, and both empty under quiet.

[GregHolmes]

[S2] The README makes cancellation look non-fatal

Summary: The README says a problem during the skills step never changes login or plugin exit codes. But the handlers catch Exception, not KeyboardInterrupt; an interrupt during the shared installer propagates and exits 2, as the PR description itself states.

Expected: The README must distinguish recoverable skills-install failures from user cancellation or process interruption.

Observed: The code intentionally allows Ctrl-C to escape the skills handlers, while the README describes every problem as exit-code-neutral.

Recommended fix: Replace the sentence with: A skills installation failure prints a warning on stderr and does not change those commands' exit codes. Ctrl-C still cancels the command.

[dg-coreylweathers]

Fixed in 1d34a22 with your wording: "A skills installation failure prints a warning on stderr and does not change those commands' exit codes. Ctrl-C still cancels the command."

[GregHolmes]

[S3] Added CRLF lines fail whitespace validation

Summary: The added print_warning lines use CRLF endings. git diff --check origin/goal/bg-2-skills-folder-install...HEAD reports every added line as trailing whitespace, even though Ruff accepts the file.

Expected: The PR diff must pass Git's whitespace validation so review and CI tooling can treat whitespace failures as meaningful.

Observed: git diff --check reports trailing whitespace on lines 372, 373, and 378-380 of this file.

Recommended fix: Normalize the touched file to the repository's expected line endings, then rerun git diff --check.

[dg-coreylweathers]

output.py is CRLF on main (as are 7 other files), and ruff format rewrites mixed endings back to CRLF, so converting only the added lines can't pass the format check. ec9a7e4 adds a two-line .gitattributes setting whitespace=cr-at-eol for that one file, so git diff --check origin/goal/bg-2-skills-folder-install...HEAD is clean when run from this branch's checkout. File contents are unchanged.

[dg-coreylweathers]

On B1 (the deepctl-core floors): same constraint as #124. Rule 2 of scripts/check_dependency_floors.py fails any floor above the workspace core (0.2.18, also the latest on PyPI), so the floors can't be raised here. The PR body's Release checklist covers it: publish core first, raise deepctl-cmd-login, deepctl-cmd-plugin and deepctl-cmd-skills to that core release in the release PR, and add a wheel-resolution test at each declared minimum.

[GregHolmes]

Content approved. Corey's dependency-floor note is the correct release sequencing, not a blocker to this stacked PR: the compatible core version cannot exist until the stack lands. Before publishing, the release PR must raise the deepctl-core minimum for deepctl-cmd-login, deepctl-cmd-plugin, and deepctl-cmd-skills, then verify each wheel imports with exactly its declared minimum core.