reply skills: install, list, update and remove skill packs across AI assistants - #13
Merged
Merged
Conversation
Agent scratch (ledgers, task briefs, review packages) lives under .superpowers/ in the working tree; it must never reach a commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
Fix glob_first_real to correctly expand single wildcard segment by stripping trailing separators instead of calling path.dirname, which was walking up an extra level. Add three new tests that exercise the real glob_first implementation against actual filesystem trees with hash-named subdirectories, ensuring Codex off-PATH binary resolution works correctly on Windows.
- Fix Important 1: read_journal now distinguishes ENOENT (no journal) from other read errors (RuntimeError), following file-store.ts pattern - Fix Important 2: write_journal now uses atomic temp-file + rename to prevent data loss on crash, following file-store.ts pattern - Minor: drop dead EMPTY export not in brief's Produces list - Minor: hoist duplicate corrupt-journal error message to constant - Test: add coverage for shape-validation and non-ENOENT read errors
- Add update_scope to Host_cli to distinguish per-pack vs marketplace-wide updates - Deduplicate Codex's marketplace-wide upgrade (run once, not per pack) - Filter installed plugins by marketplace to prevent silently skipping our packs - Track both failed and blocked packs for transitive dependency chains - Report failed host status when plugin list fails, not success - Re-read listing after updates to report actual versions - Update hint to name blocked packs, only emit when something was actually blocked - Add comprehensive tests for all four fixes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
Reviewer found the journal had no scope in its key, so a user-scope install and a --project install of the same pack on the same host could delete each other's files or silently no-op; the copy/journal loop could reject instead of returning a failed Host_outcome; deletion had no containment against a tampered journal entry; a same-named user skill could be silently overwritten and later deleted; several flat hosts sharing .agents/skills under --project could delete each other's install; and skills_target could crash on a host with no directory for the requested scope. Fixes all six, plus two related one-liners in clone_repo (check git rev-parse's exit code, clean up its temp dir on either failure path). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
adapter-native.ts already refuses to install a dependent (reply-adapter, agentic-runtime) once its dependency (ai-sdr-core) has failed, via failed_names/blocked_names. The flat adapter had no equivalent: a pack that failed on a name collision just continued to the next pack, so a dependent could still be copied and journaled with its dependency missing. Mirrors the native adapter's tracking and hint, and makes the host-level status reflect a collision failure instead of staying 'ok'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
A failed copy journaled the partial file list under the target version, so `pending` (and `list`) read it as installed and `install` silently did nothing on retry — the exact repair the failure hint tells the user to run. Journal_entry gains `complete: boolean`; a version match alone no longer counts as installed, and an incomplete entry is always re-attempted and never reported `current`. Also folds in two related fixes flagged by review: the copy-error abort path used outcomes.length as a stand-in for "something landed" (wrong when every outcome was itself a failure) and dropped the blocked-packs hint; and owns_dir/protected_files compared paths case-sensitively while is_within did not, which could misread a differently-cased path (routine on Windows) as a foreign collision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
…specificity - Label detail lines with pack name (e.g. 'reply-adapter: installation incomplete...') so multiple failed packs on one host are unambiguous - Replace detail-test substring assertion with line-array check: asserts detail appears as its own indented line with pack name - Add test with two failed packs having different details, asserting both appear with their correct pack labels on distinct lines Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
…l commits Wrap each per-host adapter call in the orchestrator so an escaping throw (e.g. a journal write outside run_flat's own try/catch) becomes that host's failed outcome instead of aborting every other host's run. Stop deriving source.commit by scanning the journal after the fact: a flat host now stamps its own Host_outcome.commit with the commit it actually cloned this run, and the orchestrator reports it only when every flat host that cloned agrees — never a stale entry from an earlier run, a sibling host's commit, or a commit from a run whose clone failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
reply skills list is read-only, but reused the "upgraded" pack action to mean "a newer version exists" — which rendered as "<pack> updated" and falsely implied the read-only listing had changed something. human_lines now renders that case as "update available" only when the report action is list, leaving install/update wording unchanged, and the README no longer claims a '*' marker that nothing in the code ever prints.
For install/update/remove, summary.installed still counts hosts by outcome status, unchanged. For list — a read-only query — a host answering "ok" says nothing about whether it actually has any pack; summarize now counts a list host as installed only when it reports at least one pack, so `skills list --json` on a clean machine reports summary.installed: 0 instead of a false-positive count of queryable hosts. skipped/failed keep their existing host-oriented meaning.
The smoke script tests the installer against Claude Code, Codex, and flat-directory hosts with proper sandbox isolation. Revealed and fixed a bug in adapter-native where Claude Code's updated plugin list format (direct array with id field) was not being parsed correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
The initial smoke script had a critical isolation flaw: flat-directory hosts (cursor, gemini-cli, github-copilot) were writing to the real home despite HOME/USERPROFILE redirection. This was caught in testing before deployment. Fixes: 1. Strengthened pre-flight assertion to verify sandbox is clean 2. Added post-run assertion that verifies the real home remains untouched by checking that no reply skill directories were created there 3. If either assertion fails, script aborts non-zero without installing Isolation mechanisms: - Native hosts (Claude Code, Codex): CLAUDE_CONFIG_DIR, CODEX_HOME - Flat hosts (Cursor, Gemini, Copilot): HOME/USERPROFILE redirect to sandbox - Detection: script creates marker directories in sandbox so hosts are found there The post-run assertion makes this smoke test safe: if isolation ever breaks, the assertion catches it and proves the failure via the assertion exit code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
The initial post-run assertion was vacuous: it checked for pack names
('ai-sdr-core', 'reply-adapter') but what lands on disk are skill
directories ('approval-boundaries', 'audience-building', etc.). The
check could not fire even on the actual contamination it missed.
Replaced with snapshot-based comparison:
- Before any CLI invocation, snapshot all real flat-host directories:
~/.copilot/skills, ~/.cursor/skills, ~/.gemini/skills,
~/.codeium/windsurf/skills, ~/.agents/skills, and the reply config dir
- After the smoke test, snapshot again and assert identical
- Any filesystem change (new entry, removed file, directory created/deleted)
fails the script non-zero and names the exact delta
This design requires no knowledge of what gets installed. It catches any
isolation failure: if HOME/USERPROFILE redirection breaks, the CLI writes
to the real home, changing its snapshot.
The assertion also triggers on read-only commands that unexpectedly write,
catching unexpected side effects.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
…exit The snapshot-based post-run assertion (previous commit) was itself unreachable in two of its three exit paths: process.exit() inside a try block skips the try's finally entirely, so a sandbox-isolation pre-flight failure or a "no assistant detected" early exit both bypassed the post-run comparison and the sandbox cleanup. The one path that did reach the comparison then called process.exit(1) again, still skipping cleanup. Restructured so every functional assertion (pre-flight isolation, host detection, resolve order, idempotency, selective install, dependency guard) lives inside a nested try/catch that reports failures via process.exitCode instead of aborting, and an unexpected thrown error is caught and reported the same way. Control always falls through to the post-run snapshot comparison and then to the outer finally, so a failing smoke run still proves (or disproves) that the real home was touched, and the sandbox is always removed. Verified by temporarily writing into the real ~/.copilot/skills between the two snapshots: the assertion reported the exact delta and exited non-zero, then the change was reverted and the probe removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
snapshot_state() watched each flat host's skills leaf directory (~/.cursor/skills, ~/.copilot/skills, etc.) but not the host's root config directory (~/.cursor, ~/.copilot, etc.) that detect_hosts actually keys presence on. An empty root created with no skills subdirectory inside it was invisible to the comparison: both snapshots recorded DOES_NOT_EXIST for the leaf, and the assertion reported "untouched" even though the next `reply skills install` on that machine would now detect and write into that host. Added each host's root directory to the snapshot set alongside its leaf. Verified with two probes between the two snapshots, each reverted after: - leaf case: an entry created inside real ~/.copilot/skills is reported as "ADDED to <path>: zz-smoke-probe" - root case: a bare ~/.cursor created with nothing inside it (the case this commit exists to catch) is reported as "CREATED: <path>" Both exited non-zero. Re-ran clean afterward to confirm a passing run still reports real home untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
…n the id-shape parse
Three Important findings from task 10's review, all fixed:
1. The pre-flight assertion never proved HOME/USERPROFILE redirection was in
effect. It only checked that the sandboxed journal reported no packs, but
journal reads are gated by REPLY_CONFIG_DIR, which is set unconditionally
regardless of whether HOME/USERPROFILE redirection works — so a fully
broken redirect would still read "clean". Added a direct proof before
anything mutating runs: spawn a child with the exact same env used for the
CLI and assert its own os.homedir() resolves inside the sandbox. Kept the
journal-emptiness check as a secondary sanity check, re-commented to state
what it actually establishes.
2. Marker directories are created unconditionally for every flat host
(Cursor, Gemini CLI, GitHub Copilot), so they always report "detected"
regardless of the real machine, making the brief's "no assistant at all"
exit-0 path dead code, and making CONTRIBUTING.md's claim that the script
"installs into the assistants actually present on your machine" false for
those three. Documented the real behavior in both the script header and
CONTRIBUTING.md: flat hosts are always simulated inside the sandbox
(deliberately — it's the only way that install path gets exercised at
all), native hosts (Claude Code, Codex) are only genuinely exercised when
really installed. Replaced the dead "no assistant" check with a reachable,
meaningful one keyed on native-host status specifically (a native host
reporting 'skipped' means its marker existed but the real binary could not
be resolved from the real PATH — i.e. not really installed here).
3. The adapter-native.ts fix for Claude Code's new direct-array-with-id
`plugin list --json` shape shipped with no regression test, so a future
refactor could silently reintroduce the exact bug that shipped broken
until a real-host run caught it. Added tests using the real shape,
captured from `claude plugin list --json` (Claude Code 2.1.220) on this
machine: a positive case (our marketplace via the id suffix maps to a
version) and two negative cases proving marketplace filtering still
applies in the new shape — the real foreign-marketplace row seen on this
machine (elastic-agent-skills), and a same-pack-name/foreign-marketplace
row exercised through the full run_native install flow, mirroring the
existing test for the old {plugins:[...]} envelope.
Verified: npm run build, npm test (395 passed, 3 skipped, up from 392 — the
3 new adapter-native tests), and a clean npm run smoke:hosts showing both new
pre-flight lines and the native-host advisory. Real home unchanged throughout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
Install blocks a dependent whose dependency failed, transitively; remove
only reversed the order. A failed `plugin uninstall reply-adapter`
followed by a successful `ai-sdr-core` removal left the host holding an
adapter with no core — the one state this feature exists to prevent.
Both adapters now carry the transposed guard: a pack is never removed
once a pack that depends on it failed or was itself blocked, the block
propagates down the chain, and the kept packs are named in the host
hint. In the flat adapter the guard is only meaningful because
delete_files no longer swallows its errors: `fs.rmSync(file, {force})`
does not clear the read-only attribute on Windows and throws EPERM, so
a pack was reported `removed` with the file still on disk. Both a
refused path (outside the skills directory) and a failed delete now
report the pack `failed` and keep its journal entry for a retry.
Two more findings in the same code paths:
- A project-scope journal entry now records the repository it belongs
to. The key is host -> scope -> pack, which cannot tell two checkouts
apart, so `remove --project` from a second repository deleted nothing
(containment correctly refused every path) and still forgot the entry
and reported `removed`. An entry from another project is now invisible
to this run: not read, not replaced, not forgotten. User scope is
unchanged.
- An update that did not move the version reports `current`, never
`upgraded X -> X`. The Codex marketplace path exits 0 whether or not
anything moved and used to hardcode `upgraded`; the flat adapter
re-copies from a fresh clone, which the commit records, not the
version. Both now agree with the Claude Code per-pack path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
…peration Two contract-level slips in the shared handler. `reply skills list` is a data command, but every human-mode line went to stderr, so `skills list 2>/dev/null` printed nothing and `skills list > installed.txt` produced an empty file — the opposite of the repo's output contract. Its table now goes to stdout; install, update and remove print progress, which is status, and stay on stderr. The test that pinned the old behaviour asserted it as intentional, so it is replaced by one assertion per side of the split. The exit-1 error was install-worded for all four operations: a failed `remove` said "No assistant received the skills." and pointed at `skills install --dry-run`. Title, code and hint now come from the operation that actually ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
`verified: false` was declared on four of six hosts, asserted by one
registry test, and read by nothing: not the README, not the report, not
`--json`. A Cursor or Windsurf user got a green tick for a directory we
have never confirmed the assistant reads from.
The orchestrator now stamps `Host_outcome.verified` from the registry
for every host it reports, including one requested with --agent but not
installed. This is a `--json` contract addition: each host object gains
a boolean `verified`. The human report appends "(paths not yet
verified)" to the line for such a host, and only where a claim about
packs is actually being made. The README's Skills section gains a host
coverage table saying which assistants are confirmed and what "not yet"
means, linking REPLY-51268.
Also here, because both are orchestrator-level:
- An end-to-end `update` test through run_skills with one native and
one flat host, asserting both answer "already at the target version"
with `current` and that the "start a new session" advice does not
fire. `update` had no coverage through the orchestrator at all.
- A test whose title claimed the opposite of its body ("routes a native
host through the flat adapter under --project", asserting that Claude
Code stays native) now describes what it asserts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
… same version I3 made an unchanged version report `current` in every adapter, which is right, but the reporter decided "start a new session" from the action labels alone — so two cases that really did rewrite the user's files went silent: repairing an incomplete install, and an `update` that pulls a new commit on the same ref at an unchanged 0.1.0. Both used to print the advice through the wrong `upgraded 0.1.0 -> 0.1.0` label this wave removed. A regression introduced by 36ae3a7. "The version moved" and "the bytes moved" are two facts, and only the first survived. They are separated rather than recombined: the flat adapter already holds the clone commit next to `previous.commit` at the point it journals a copy, so a `current` outcome now carries `refreshed: true` when the commit differs or the previous entry was incomplete. The reporter's advice keys off that as well as the actions. This is a `--json` contract addition — `Pack_outcome` gains an optional boolean `refreshed`. No new `Pack_action` value: `current` still means "the version did not move", which is the rule as prescribed. The flag appears only on a `current` pack, since that is the only label that drops the information. A native host never sets it (it cannot see below its own plugin CLI) and neither does a dry run, which does not clone and so cannot know whether the ref moved. Tests: a same-version re-copy from a new commit reports `current` with `refreshed`, and the report still advises a new session, asserted both at the adapter and end-to-end through run_skills; the same commit re-copied does not set it; a dry run never guesses; and the repair case carries it too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
The differently-cased-journaled-path test asserted that the two spellings name one file. That is true on Windows and false on Linux, so CI was red on ubuntu and green on windows. The production code is correct on both: owns_dir and is_within share path.relative, whose case sensitivity deliberately follows the platform's filesystem. Split the test to assert the real behaviour on each — already ours where case is ignored, a foreign file that must not be overwritten where case distinguishes paths. The POSIX expectation is taken from the actual CI output, not inferred. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QmPs4sPmBd8zBKKGiFYxM4
vigubikReply
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
reply skillscommand group that installs Reply's three skill packs —ai-sdr-core,reply-adapter,agentic-runtime— into whichever AI assistants are present on the machine, resolving the dependency graph itself.No argument means all three packs;
core,adapterandruntimeare accepted as short names. Selective installs pull dependencies automatically, soinstall adapteralso installsai-sdr-core.The invariant everything is built around
No host may end up with
reply-adapteroragentic-runtimepresent whileai-sdr-coreis missing. Packs are always ordered dependency-first; if a dependency fails to install, its dependents are not attempted on that host, transitively. The same rule runs in reverse on removal — a pack that fails to uninstall protects the packs it depends on — and removing a dependency that something else still needs is refused with a hint.Two host classes, one interface
Native hosts (Claude Code, Codex) are driven through their own plugin CLI rather than by copying files, so Claude Code's marketplace update channel keeps working and Codex installs natively. Idempotency comes from asking the host what it has, never from inspecting the filesystem. Neither host resolves dependencies for us, so the installer does it before invoking either.
Flat hosts (Cursor, Gemini CLI, GitHub Copilot, Windsurf, and Codex under
--project) get a shallow clone copied into their skills directory, with an installer-side journal recording exactly what was written — the minimum needed for an idempotent update and for a removal that deletes our files and nothing else.Claude Code and Codex are verified against real installs (2.1.220 and codex-cli 0.146.0-alpha.3.1). The other four ship marked
verified: false, and that is surfaced in the README and in--jsonrather than left implicit.Safety
This writes into and deletes from a user's home directory, so the guarantees are explicit and tested:
Output contract
Data to stdout, status and errors to stderr.
--jsonemits one object carryingrequestedalongsideresolved, so an agent sees the dependency pull without parsing prose, plus per-host and per-pack outcomes. Exit0when at least one host succeeded,1when nothing installed anywhere — and the report still prints in that case, because otherwise the per-host reason is lost.Verification
421 offline tests (up from 273), all green, plus
tsc. Nothing in the suite touches the network or a real assistant.npm run smoke:hostsis separate and opt-in: it exercises the real commands against really installed assistants inside a throwawayHOME, proves the redirect is in effect before it mutates anything, and snapshots the real home before and after to prove it changed nothing. Both of those assertions were themselves tested by deliberately breaking them. It is not part ofnpm testbecause it needs the assistants installed and clones from GitHub.Review notes
fix:commits are review findings, not churn.--jsongains two additive fields on this branch:verifiedper host, andrefreshedon a pack that was re-copied at an unchanged version.mainand--pinwaits on that.