feat(skills): verify GitHub Copilot as a host - #23
Conversation
The last host carrying paths from documentation alone. Verified against Copilot CLI 1.0.80 at ~/.copilot/skills, and at .agents/skills with --project, in both directions: the installer's own report, and `copilot skill list --json` naming every skill it loaded and the directory it came from. A session invoked the reply-mcp skill and answered from the pack. Per-pack install and remove were exercised the same way at both scopes — 9 skills, 14, 18 and back — each count read from the host rather than from the files on disk. The registry keeps .agents/skills for project scope, which the host reads alongside .github/skills and .claude/skills, so one install still serves the other flat hosts. Two orchestrate tests moved off `verified` and onto needs_new_session. With every host verified, a report asserting `verified` alone can no longer fail — a stamp hardcoded to true would pass it — while needs_new_session is mixed and still catches a dropped or invented flag.
ArtemKosolap
left a comment
There was a problem hiding this comment.
The guard fired exactly as intended, and the answer to it is the right one. I checked both halves of the reasoning rather than taking it:
- Stamping the flag —
needs_new_session: truehardcoded where the report reads it — fails both rewritten tests. The mixed flag does the work now. - A wrong
verifiedis still caught, bymarks only the hosts we actually verifiedin the registry test. Setting Antigravity toverified: falsefails it and nothing else. - Clean run on this head: 555 passed, 5 skipped, 0 failed.
The verification write-up is the strongest of the four hosts: counts read back from copilot skill list --json rather than from the filesystem, per-pack sequences at both scopes, an isolated COPILOT_HOME so user scope could not mask project scope, and a session that actually called a pack. Scoping the row to the CLI and stating the VS Code behaviour separately is the honest split.
Two notes.
One thing the retired assertion took with it
The old test also proved the (paths not yet verified) note reaches human output from a real registry. That could not survive an empty unverified list, and dropping it was right — but it means the chain is now covered in two halves that never meet: the registry-to-report flags here, and the note's rendering in report.test.ts from literal fixtures. I confirmed the gap by planting verified: false on Antigravity: only the registry test fails, nothing notices that a real unverified host would produce no warning.
Nothing to do today, since the assertion needs a host to point at. Worth a line in the comment where the guard used to be, so whoever adds the next unverified host restores it rather than rediscovering the hole.
The Claude Code crossover claim reads wider than it is
"…and ~/.claude/skills for user scope, so a Claude Code install shows up in it as well." In this README, "a Claude Code install" reads as one this command made — and that install never lands in either .claude/skills or ~/.claude/skills. orchestrate.ts:129-133 says it plainly: a native host falls back to the flat adapter only when its plugin mechanism cannot express project scope, and Claude Code can, so both scopes go through claude plugin install.
The crossover is real, just for a different set of files: skills put there by hand, or by the npx skills channel, which the packs repository documents as writing .claude/skills/ for a project-scoped Claude Code install. Worth naming which, because the two sets are managed by different tools and only one of them is reply skills remove's business.
Related, and worth a moment: project_skills_dir: '.claude/skills' on the Claude Code entry is never written by this tool for the same reason. If it is only there to satisfy the registry shape, a word saying so would stop the next reader concluding that --project copies there.
The overlap note said a Claude Code install shows up in GitHub Copilot. Not one made by this command: a native host falls back to the flat adapter only when its plugin mechanism cannot express project scope, and Claude Code's can, so both scopes go through its plugin CLI and nothing is written to .claude/skills. Copilot does read that directory, so the crossover is real when something else fills it — a skill copied by hand, or the npx skills channel. Named as such now. Also separates the two products' user directories: ~/.claude/skills is a VS Code default, not one the CLI reads.
ArtemKosolap
left a comment
There was a problem hiding this comment.
Both corrections are right, and the second one is yours rather than mine — I had not spotted that ~/.claude/skills is a VS Code default rather than something the copilot CLI reads. Separating the two products' directories is a better fix than the one I asked for, because the sentence I flagged was only half the confusion.
The replacement says the useful thing plainly: Copilot does read those two repository directories, this command never writes to them, so the crossover appears only when something else fills them. That is exactly the distinction a user needs before assuming remove reaches across.
One thing from the review is still open, and it is the smaller of the two: a line where the guard used to live, so whoever adds the next unverified host restores the end-to-end assertion that the (paths not yet verified) note reaches the report. Nothing to assert today — it needs a host to point at — which is precisely why it will be invisible when the moment comes. A sentence in the comment block above in_session is enough.
Everything else I checked holds: 555 passed, 5 skipped on this head, the stamped-flag plant fails both rewritten tests, and a wrong verified is still caught by the registry test.
ArtemKosolap
left a comment
There was a problem hiding this comment.
Approving. The verification behind this one is the most thorough of the four hosts — counts read back from the host rather than the filesystem, both scopes with an isolated COPILOT_HOME, and a session that actually called a pack — and the two corrections since landed cleanly, including the one you found yourself about ~/.claude/skills belonging to VS Code rather than the CLI.
Checked on this head: 555 passed, 5 skipped; stamping needs_new_session fails both rewritten tests; a wrong verified still fails the registry test. The pivot from verified to needs_new_session as the mixed flag is the right answer to the guard firing.
The one open item is a line in the comment block above in_session, so the next unverified host brings back the end-to-end assertion that the note reaches the report. Not worth another round — fold it in whenever you next touch the file, or leave it and I will raise it again when that host arrives.
3e997c6 to
00113a2
Compare
Stacked on #22 — retargets to
mainwhen that merges. Closes out the last of the four hosts REPLY-51268 asked us to verify, soverified: falsenow describes no host in the registry.What was verified
Copilot CLI 1.0.80 (
npm i -g @github/copilot,/opt/homebrew/bin/copilot). Every count below is read from the host, not from the files on disk —copilot skill list --jsonreports each skill it loaded, its source scope and its path, without spending a model call.install --agent github-copilotsource: personal-copilot,~/.copilot/skillsinstall --agent github-copilot --projectsource: project,.agents/skillsinstall ai-sdr-coreinstall reply-adapterai-sdr-core already current; reply-adapter installedinstall agentic-runtimeremove agentic-runtimeremove reply-adapterupdate/listProject scope was checked with an isolated
COPILOT_HOMEso user-scope skills of the same names could not mask it, and the per-pack sequence was run at both scopes.A session used a pack. Asked which API key Reply MCP uses, the CLI called
skill(reply-mcp)and answered from it — personal scoped key or OAuth, team and org keys not supported,REPLY_MCP_TOKEN, never in a URL. Files in the right folder prove nothing; this is the part that does.Paths
~/.copilot/skillscomes from the host's own resolver, which also honoursCOPILOT_HOME:For project scope it reads
.github/skills,.agents/skillsand.claude/skills, preferring.github/skillsfor skills it writes itself. The registry keeps.agents/skills, so a project install still serves Cursor, Codex and Antigravity from one copy — and a Claude Code install shows up here too, which the README now says.Takes effect on a new session, or
/skills reloadin the open one.Not claimed here
The VS Code agent host reads the same
~/.copilot/skills— it is one of the defaults ofchat.agentSkillsLocations, andchat.useAgentSkillsdefaults on — but it scans once per window and does not watch the directory, so an install made while it is open needsDeveloper: Reload Window. That behaviour is documented in the README; a chat turn using a pack there is not yet verified, and the row claims the CLI only.Tests
Two orchestrate tests selected "the unverified host" from the registry and had nowhere to point once this flag flipped — the guard left for that moment fired rather than the assertion passing on nothing. They now assert
needs_new_sessionalongsideverified: with every host verified, a report assertingverifiedalone cannot fail, since a stamp hardcoded totruewould satisfy it, whileneeds_new_sessionis mixed and still catches a dropped or invented flag.