feat(agent-plugins): Agent Plugins 1.0.0 support — client pipeline, CAO-as-plugin packages, MCP delivery (#573) - #584
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #584 +/- ##
=======================================
Coverage ? 91.76%
=======================================
Files ? 194
Lines ? 25555
Branches ? 0
=======================================
Hits ? 23450
Misses ? 2105
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
haofeif
left a comment
haofeif
left a comment
There was a problem hiding this comment.
Requesting changes for the reproducible issues below.
Non-blocking documentation note: docs/issues/ is a recent historical-design-record convention, introduced by #345 and explicitly reused by #568; it is not runtime or end-user documentation. Retaining durable decisions and the OpenCode verification evidence here is useful, but this PR adds 2,288 lines under docs/issues/573-agent-plugins/, including an unchecked 374-line implementation task plan and duplicated requirements. I recommend keeping the final design/provenance and dropping or relocating the planning artifacts before merge.
| # utils/opencode_config.translate_mcp_server_config already derive every | ||
| # provider's native MCP form, so this single merge reaches all of them | ||
| # with no per-provider code. | ||
| profile.mcpServers, plugin_mcp = merge_plugin_mcp_servers( |
There was a problem hiding this comment.
[P2] Deliver plugin MCP servers on runtime-loaded provider paths
This merge only mutates the local parsed profile. No MCP-bearing artifact is written for Claude Code, Codex, Kimi, Antigravity, or Cursor, and _write_context_file persists the original raw profile; those providers later call load_agent_profile() again at launch, so these merged entries are gone. Copilot likewise builds its runtime MCP config with only cao-mcp-server. I reproduced an installed stdio plugin where install_agent("worker", "claude_code") succeeds but the eventual Claude command contains neither --mcp-config nor the plugin server. Merge/remap on the actual launch profile (and extend Copilot's runtime config), or persist configuration that those providers consume; the all-provider tests need to inspect real launch commands rather than treating collect_plugin_mcp_servers() as delivery.
|
|
||
| for skill_name in sorted(set(previous) - set(current)): | ||
| path = target_dir / skill_name | ||
| if _remove_quiet(path): |
There was a problem hiding this comment.
[P2] Preserve a user skill that replaced a copied projection
previous records only the old projection claim, not the current path's ownership. In supported copy mode, cao skills add <folder> --force successfully replaces the copied plugin skill with a user-owned directory, but a later cao plugin remove reaches this loop and recursively deletes that new directory because its name is still in previous. I reproduced the sequence and the user skill is gone after removal; any intervening projection rebuild also overwrites it with the plugin copy. Verify a marker/hash before deleting copied projections or have the skills command transfer ownership so the documented "user-added skill always wins" rule remains true after installation order changes.
| return True | ||
| if existing.get("enabled") is False and entry_within_roots(existing, plugin_store_roots): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
[P2] Recognize changed CAO entries during OpenCode updates
A force update that changes a plugin server's command, args, or environment makes the candidate differ from the enabled entry CAO wrote previously. This function then returns false, so _materialize_opencode_mcp misclassifies CAO's own v1 entry as user-owned, leaves the stale v1 command in opencode.json, and removes the agent's tool grant because the v2 entry was dropped. I reproduced exactly that result; the same path breaks a lexicographic winner transition between plugins. Persist exact ownership (record or marker) so enabled CAO entries can be updated without weakening the guard for genuinely user-authored entries.
| _ALL_TRANSPORTS = frozenset({"stdio", "streamable-http", "sse"}) | ||
| PROVIDER_TRANSPORTS: Dict[str, frozenset] = { | ||
| "opencode_cli": _STDIO_ONLY, | ||
| } |
There was a problem hiding this comment.
[P2] Reject HTTP transports for providers that cannot serialize them
The default-all assumption is not true for every provider. Codex's serializer emits only command, args, and env overrides and never emits url; Antigravity similarly always writes a local command/args entry. Consequently map_mcp_config(..., provider="codex") accepts a streamable-http server, but the generated Codex command contains the server name with no URL or command, so it cannot start. Add the actual provider capability entries (at least Codex and Antigravity as stdio-only) or implement their HTTP translations before reporting these transports as delivered.
| from cli_agent_orchestrator.agent_plugins.installer import PluginInstallError, install | ||
|
|
||
| try: | ||
| outcome = install(_plugin_source(body), force=body.force) |
There was a problem hiding this comment.
[P2] Move plugin resolution off the FastAPI event loop
This is an async def handler but it directly runs the synchronous resolve/validate/publish/refresh pipeline. A git source can block in subprocess.run for up to 300 seconds, and a large local source blocks in copytree; during that time the server loop cannot service health/session requests or run status and inbox tasks. The validate call at line 2222 and uninstall path have the same problem. Run these operations in a worker thread (or make the handlers synchronous so FastAPI does so) rather than freezing the orchestrator during a normal plugin operation.
| if backup is not None: | ||
| _rmtree_quiet(backup) | ||
|
|
||
| self.write_record(record) |
There was a problem hiding this comment.
[P2] Retain the old root until the install record commits
On a force update the finally above deletes the backup before this record write. If the state write fails (for example, the state volume is full or loses permission), publish() raises but the new package bytes have already replaced the old root, the old backup is gone, and the old record remains. I reproduced a v1 record paired with v2 bytes after an injected write_record failure, contradicting the failed-install isolation guarantee. Keep the backup through the metadata commit and restore the prior root/record on failure (and remove a new root when an initial record write fails).
|
|
||
| record_path = self._record_path(validated) | ||
| if record_path.exists(): | ||
| record_path.unlink() |
There was a problem hiding this comment.
[P2] Keep the record when deleting the plugin root fails
_rmtree_quiet(root) swallows OSError, but this code still marks the plugin removed and unlinks its record. With a simulated busy/permission failure, unpublish() returns true while the full plugin root remains and get() returns none; cao plugin remove therefore reports success and leaves an untracked installation that blocks a later non-force add. This is especially plausible on Windows while an MCP executable is still open. Make root deletion report failure and only remove the record after the root is actually gone; apply the same truthful result handling to purge_data.
|
Hmmm CI tests are taking extremely long to run ... i dont think its my change as I see the same happening on other PRs, but making a note to create an issue for it later. |
fanhongy
left a comment
fanhongy
left a comment
There was a problem hiding this comment.
Summary
Request changes. The PR exposes surfaces that its own unresolved M1 gate says must not ship, and it has two reproducible state-integrity paths that can delete a user skill or corrupt plugin records. Three additional failure paths leave operations partially committed or report failure while continuing in the background.
Findings
P1: The unresolved M1 gate does not prevent the public surfaces from executing
src/cli_agent_orchestrator/cli/main.py:55 registers cao plugin unconditionally. hidden=True only removes it from help; cao plugin list --json succeeds at this head. The API is likewise live and discoverable through the routes registered at src/cli_agent_orchestrator/api/main.py:2180. This contradicts Requirement 16.5 and the AC6 traceability row, which explicitly gate the CLI/API/web surfaces until maintainers resolve M1. Merging this ships the unresolved command name and mutating API despite the claimed closed gate. Either resolve M1 and update the specification/docs in this PR, or conditionally omit command and route registration until the decision is made; hiding navigation is not an execution gate.
P1: A failed ownership transfer allows plugin removal to delete the user's replacement skill
src/cli_agent_orchestrator/agent_plugins/projection.py:123 catches every record-write failure and returns None, which is indistinguishable from "no plugin owns this skill." The force-install caller then deletes the projected entry and installs the user's copy at src/cli_agent_orchestrator/cli/commands/skills.py:45. Because the plugin record still claims the name, a later plugin uninstall sweeps that path and recursively deletes the user's skill. This reproduces when .state is read-only while SKILLS_DIR remains writable. Propagate a distinct failure and abort before replacing the projection, or make ownership transfer and replacement one atomic operation.
P1: Projection write-back can overwrite newer store state
src/cli_agent_orchestrator/agent_plugins/projection.py:628 rewrites a complete PluginRecord captured earlier at line 178, outside the lock used by publish() and unpublish(). Concurrent API requests can therefore commit an update/removal between the snapshot and this write. A stale rebuild then reverts force-update metadata or recreates a removed record; an isolated reproduction left get("demo") returning version 1.0 while the plugin root no longer existed. Serialize projection record updates with store mutations, or use a compare-and-swap that updates only projected_skill_names after verifying the current record identity and root.
P2: Plugin-data creation failure leaves an install committed after reporting failure
src/cli_agent_orchestrator/agent_plugins/installer.py:147 creates PLUGIN_DATA after store.publish() has committed the root and record. If the data directory is unwritable, install() raises, but the plugin remains installed without projection; a retry is then rejected as already installed. The isolated reproduction produced record_exists=True, root_exists=True, and skill_projected=False. Preflight/create the data directory before publication with cleanup on failure, or roll back the publication when this mandatory step fails.
P2: Purge failure removes the plugin record before returning an error
src/cli_agent_orchestrator/agent_plugins/store.py:368 deletes the install record before attempting data deletion at line 373. If the data tree is busy or undeletable, unpublish(..., purge_data=True) raises after the root and record are already gone, while its error incorrectly says the installation remains tracked. The command cannot be retried because the plugin now appears absent, and the requested data remains stranded. Make root, record, and data removal transactional, retaining or restoring tracked state when purge fails.
P2: The web client times out while the server can still complete the install
web/src/api.ts:390 aborts install and validation requests after 120 seconds, but the resolver permits a git subprocess to run for 300 seconds and asyncio.to_thread continues after client disconnect. A clone taking 121-300 seconds therefore reports failure in the panel while the backend can still publish the plugin, so retrying can produce an unexpected "already installed" result. Use a timeout longer than the server's maximum operation, or expose cancellable/job-based status so an aborted request cannot silently finish.
Validation
- Verified checkout HEAD:
747509672171d863ea19434495b02b834469a063. - Read and hash-verified all acquisition artifacts;
diff.patchcontains 55,282 lines. - Focused tests under isolated CAO/HOME paths:
61 passed, 1 skipped. - Reproduced stale-record resurrection, user-skill deletion after failed claim release, partial install after
PLUGIN_DATAfailure, and partial purge after data deletion failure. - Confirmed
cao plugin list --jsonexecutes and all four/pluginsroutes are registered. git diff --checkreports four trailing-whitespace additions; no P3 finding was raised for those.- The first non-isolated focused run saw one environment-dependent API assertion fail because it discovered live local CAO sessions; the isolated rerun passed.
88a10b3 to
46f6e51
Compare
…AO-as-plugin packages, MCP delivery (awslabs#573) Adopts the [Agent Plugins 1.0.0](https://agent-plugins.org/specification) packaging standard in both directions: CAO installs agent plugins, and CAO ships as one. ## Client pipeline `cao plugin add|list|validate|remove` plus `GET/POST /plugins`, `POST /plugins/validate` and `DELETE /plugins/{name}`, over a manifest-first pipeline in `src/cli_agent_orchestrator/agent_plugins/`: `resolver` (local path or git clone, containment-checked), `validation` (both closed schemas, vendored and byte-pinned — no schema retrieval at load time), `store` (transactional publish under a cross-process lock), `projection` (plugin skills into the existing skill store, deterministic collision rule, user-installed skills always win), `mcp_mapping` + `mcp_delivery` (per-provider MCP config with `${PLUGIN_ROOT}` / `${PLUGIN_DATA}` expansion), and `provenance`. Failure isolation follows the conformance floor: an invalid skill is skipped with a finding while its siblings load, an invalid `mcp.json` disables MCP for that plugin only, and a missing `skills/` directory is a valid absence. ## CAO as an agent plugin `agent-plugin/cao` (operator: session-management skill + the `cao-ops` stdio MCP server) and `agent-plugin/cao-contributor` (authoring skills), both generated from the canonical `skills/` tree by `scripts/build_agent_plugin.py` and byte-diff guarded by `make check-agent-plugin`, so the committed packages cannot drift from their sources. Each carries a Claude Code compatibility overlay (`.claude-plugin/plugin.json` and a `.mcp.json` byte-identical to `mcp.json`), measured against Claude Code 2.1.226. ## Ship gate — Requirement 16.5 The whole management surface is default-off and **refuses to execute**: one shared predicate, `agent_plugins.gate.agent_plugins_surface_enabled()`, reads `CAO_AGENT_PLUGINS_ENABLED` (`1`/`true`/`yes`, matching the `CAO_AGUI_ENABLED` and `CAO_EAGER_INBOX_DELIVERY` precedents), so the CLI group and the four HTTP routes cannot disagree about whether the surface is live. Without it the routes 404 and every CLI verb errors out naming the variable. The web tab is additionally off at build time (`PLUGINS_TAB_ENABLED`) and the four TUI rows are `Policy::Hidden` and filtered out of the command palette. Environment-only by design: this is a release gate, not a user preference. Maintainer decisions M1–M4 stay open — nothing here resolves the verb, the route paths, or the extension namespace. ## Review round 1 (haofeif) — seven P2 findings * store not transactional: `publish` now holds the aside-backup through the record commit and rolls back on failure; `unpublish` reports deletion failure and unlinks the record only after the bytes are confirmed gone, as does `purge_data`; * blocking the event loop: resolve/validate/publish/uninstall run via `asyncio.to_thread`; * an untrue transport table: Codex and Antigravity audited stdio-only, and `DEFAULT_TRANSPORTS` made explicit for all ten providers; * a user skill destroyed on rebuild: `cao skills add --force` transfers ownership; * OpenCode ownership misclassified after a command change: ownership is decided by whether the command resolves inside the CAO-managed plugin store; * MCP servers computed but not delivered at launch: one shared apply function on both the install and launch paths, since five providers re-read the profile from disk at launch; * a publish race: `flock` on the state directory plus a swap-time `force` re-check. Two existing tests that encoded the defective expectations were inverted with their reasoning recorded. ## Review round 2 (fanhongy) — three P1, three P2 * **P1, the M1 gate did not prevent execution.** `hidden=True` suppresses help text while Click's dispatch is untouched, and the routes' only dependency was `require_any_scope(...)` — authorization, a no-op when auth is disabled, not a feature gate. Fixed by the execution gate described above, and the prose that documented the advertisement-only stance is rewritten in all five places it appeared. * **P1, a failed ownership transfer deleted the user's skill.** `release_projection_claim` returned `None` both for "no plugin held this name" and for "the record write failed", so a force install proceeded to replace the projection while the record still claimed the name, and the next sweep removed the user's real directory by name alone. Two layers: `ProjectionClaimError` makes the release tri-state and `_install_skill_folder` aborts before unlinking anything; and `_sweep` phase one now requires `_is_managed_projection` — the realpath-containment test phase two already had — reporting a `projection.sweep_skipped_unmanaged` finding instead of deleting silently. Residual stated in the code: a copy-mode projection left by an earlier copy-mode rebuild is not swept by a later symlink-mode rebuild, which over-preserves rather than losing data. * **P1, the projection write-back raced the store lock.** `rebuild_projection` snapshotted the installed set, did slow filesystem work, then rewrote a complete record from that stale snapshot with no lock, reverting a concurrent publish or resurrecting a removed record. `InstalledPluginStore.update_projected_names` is now a compare-and-set under the lock that re-reads fresh, skips a vanished record, and patches only `projected_skill_names`; `release_projected_name` is its sibling. Deliberately narrow — locking the whole rebuild would serialize installs behind slow filesystem work. * **P2, `PLUGIN_DATA` created after the commit** left root + record installed with no data directory when the data volume failed, and the retry was refused as already-installed. Created before `store.publish` now, so a failure aborts before any commit. * **P2, purge deleted the record before the data**, so a purge failure raised an error promising the install "remains tracked and can be retried" when the record was gone. The purge moved above the unlink; the retry re-runs because root deletion treats absence as success. * **P2, the web client aborted at 120s** against a 300s server git budget, on an `asyncio.to_thread` call that cannot be cancelled. Both cloning operations now wait 330s, with the comment deriving the number from `resolver.GIT_TIMEOUT_S` by name. Also clears the four trailing-whitespace additions `git diff --check` reported (fixed in the canonical `skills/` sources and both mirrors regenerated, not by hand-editing generated trees), and teaches the dog-food recorder to open the ship gate it now depends on. ## Verification `test/agent_plugins/` carries 814 passing tests, including the conformance corpus, the property suites, per-provider delivery equivalence against each provider's real launch artifact, and fault injection at every commit point. Every review fix has a test that fails without it, mutation-verified by reverting the fix and confirming the source restores byte-identical. The full suite's failures are exactly the pre-existing AG-UI (58) and OpenTelemetry (3) set with the same per-file grouping as `main`. `make check-agent-plugin`, `make check-agent-plugins-schemas`, the docs vocabulary guard, the link validator, `black`/`isort`, `npm run build` in `web/`, and the end-to-end dog-food recorder all pass. Closes awslabs#573 Two further gaps, found by an independent read-only audit of this commit rather than by the review, are closed here too: * the ship gate was applied as each handler's *first statement*, which is not first in the request: FastAPI validates the body and solves the scope dependency before the handler runs, so a malformed payload answered 422 and an unauthorized caller 401/403 — each disclosing that the gated route exists. The gate is now a route-level dependency, verified to answer 404 ahead of both, so a disabled surface is indistinguishable from an unregistered path; * the F2 containment guard covered the sweep's delete-by-name path but not ``_place``, which removes whatever occupies the target before writing — the same ``shutil.rmtree`` on a real directory. Materialization now refuses a real directory at a name the previous projection did not own, reporting ``projection.target_not_ours``; a name the projection did own stays replaceable, which keeps plugin upgrades and copy-to-symlink migration working. Co-authored-by: plauzy <4451274+plauzy@users.noreply.github.com> Co-authored-by: Kiro Agent <244629292+kiro-agent@users.noreply.github.com>
46f6e51 to
a3cc8a7
Compare
|
Moving this change back to Draft while I run a few manual analyses and audits - all findings are addrssed to be clear, but want to ensure the quality bar is highest it can be. Will re-open and reply in-line to all comments verifying once done and ready for new review. |
4 participants
Agent Plugins 1.0.0 — client pipeline, CAO-as-plugin packages, MCP delivery
Implements Agent Plugins 1.0.0 support — the client install pipeline, two published CAO packages, and MCP-server delivery into agent profiles — as proposed in #573. Agent Plugins is the open, vendor-neutral packaging standard for Agent Skills + MCP servers, governed by a TSC drawing maintainers from AWS, Cursor, Microsoft, OpenAI and Vercel. Publishing to it means every compatible client becomes a distribution channel for CAO instead of another integration guide to maintain.
Every operator-facing surface is gated closed pending the naming decision (M1), and the gate now refuses execution rather than only hiding navigation — the correction review 2 asked for. One shared predicate reads
CAO_AGENT_PLUGINS_ENABLED(default off), so thecao plugingroup errors out and all four/plugins*routes return 404 without it; the four TUI rows arePolicy::Hiddenand filtered out of the command palette, and the Web UI panel is behindPLUGINS_TAB_ENABLED = false. Each gate has a test asserting the closed state, and the default-off tests deliberately do not use the fixture that opens it, so with no decision taken the CLI, API, TUI and Web UI are all inert for end users. The existing Python event-plugin system (docs/plugins.md, decision D7) is untouched — its diff is empty.Single signed commit on top of
main, with both review rounds folded in — haofeif's seven P2 findings and fanhongy's three P1 / three P2 findings, each with a test that fails without its fix.What ships here
./-rooted containment, report-and-continue failure boundariesSKILLS_DIRas symlinks, reaching every provider incl. Kiro CLI + OpenCode${PLUGIN_ROOT}/${PLUGIN_DATA}expansion, then merged into the agent profile and materialized into each provider's native configcao plugin add | list | validate | remove(behind the M1 gate)agent-plugin/cao(4 skills +cao-ops) andagent-plugin/cao-contributor(skills-only, a live §11.2 conformance instance)make check-agent-plugins-schemas+make check-agent-pluginon every PRAcceptance criteria cross-reference (#573)
plugin.json+mcp.jsonvalidate against the canonical 1.0.0 schemas in CI on every PRmake check-agent-plugins-schemas+make check-agent-pluginin CI. Schemas are vendored with recordedsha256s and validation is offline by construction — the registry refuses every retrieval, asserted by blocking socket creation process-wide and then validating a real plugincaoplugin installs and works in ≥2 compatible clients:cao-opscallable,cao-session-managementdiscoveredskill://+mcpServersin the emitted agent JSON) and Claude Code 2.1.226 (a client outside the spec's listed set). In Claude Code: strictclaude plugin validatepasses, a marketplace install discovers all four skills and thecao-opsserver, and its tools were exercised over MCP stdio against the publishedcli-agent-orchestrator==2.4.1pin — a structured error naming operation and cause with nocao-serverrunning, real data (list_sessions → {"success":true,...}) with one upagent-plugin/cao-contributorships nomcp.json, exercising the skills-only conformance floor (§11.2)cao plugin addinstalls the canonical example: skill delivered; invalid sibling skipped with a report; missingskills/tolerated; fatalplugin.jsonviolation rejects before any component loadstest_conformance_corpus.py— 22 cases, exact-match, plus a guard asserting every corpus path is something git actually stores (an empty-by-design fixture directory once vanished between machines; that class of bug is now impossible)cursor-cli.md,opencode-cli.md) are permanent, justified exemptions where the bare noun names a third party's own plugin concept that our qualifiers would misdescribecao-server, no credentials in package data,./-rooted containment on installGET /pluginsmoved onto the read floor (it discloses local paths plus live terminal IDs, session and skill names), andtest_scope_coverage.pypins today's ungated GETs as data so a new ungated GET fails the buildDemo recordings (shift-left — CI-gated proof-of-work)
The GIF above is generated by the build, not hand-recorded.
Agent Plugins dog-food (shift-left recording)runs the asserting example — CAO installing its owncaopackage through its own plugin pipeline — and exports the GIF only when the run exits0and prints itsPASSmarker. A regressed pipeline cannot produce a green recording.The gate was proven rather than asserted: neutralizing the OpenCode disable call made the recorder exit
1with the GIF'ssha256and mtime unchanged — never produced — after which the source was restored byte-identical and the run went green again.cao plugin validate ./agent-plugin/caomcp_presentwithcao-opscao plugin add ./agent-plugin/caolsoutputcao install … --provider kiro_cliskill://globs andcao-opswithPLUGIN_ROOT/PLUGIN_DATAexpanded to real paths, no internal marker leaked,@cao-opsgrantedcao install … --provider opencode_cliopencode.jsongets a real boolean"enabled": true; installing over a user's own same-named entry preserves it and emits a findingcao plugin remove caoRuns offline under a scratch
HOME/CAO_HOME_DIR/CAO_AGENTS_DIR— no provider binary, network or secrets, no real config read or written, and no real home path rendered (CONTRIBUTING.md§ Recording test fixtures safely).Try it in a client
Full per-client walkthrough:
docs/agent-plugins.md§ Trying it in a client. From a clone:cao plugin validate ./agent-plugin/cao # loadable? skills named? mcp present? cao plugin add ./agent-plugin/caoKiro CLI — native, both routes.
cao install <profile> --provider kiro_cliwritesskill://globs and thecao-opsserver directly into the agent JSON. Because Kiro powers install Agent Plugins natively, the same directory is also installable as a power with no CAO-specific packaging.Claude Code —
claude plugin validate ./agent-plugin/caopasses, then install through its marketplace flow. The useful check is the two-state one: with nocao-serverrunning acao-opscall returns a structured error naming the operation and cause; start one and the same call returns real data. That proves the tool is wired to a local CAO rather than merely registered.Antigravity CLI / Gemini-family clients — Antigravity is a CAO provider and receives skills by runtime prompt injection plus the
load_skilltool, socao install <profile> --provider antigravity_cliis all that is needed; no client-side plugin install. For any other client that accepts an MCP server configuration,agent-plugin/cao/mcp.jsonis the portable declaration to copy — the samemcpServersshape the standard defines, pinned to a published version, needing no CAO-specific translation.Verification status, stated plainly: the Kiro and Claude Code paths were exercised end to end against live clients. The Gemini-family note describes the portable declaration those clients consume; it is not a claim that each has been tested.
The Claude Code finding went upstream, not into a fork
Verifying AC2 against a real foreign client turned up a portability gap worth more than a local workaround: Claude Code 2.1.226 discovers the standard
skills/layout unchanged, but reads identity only from.claude-plugin/plugin.jsonand MCP servers only from a root.mcp.json— its strict validator rejects the root-manifest layout that the marketplace path tolerates.CAO's side is a generated, drift-guarded overlay that conformant clients cannot see (§6.1's fixed component locations exclude dot-prefixed entries). The spec-pure alternative — a
com.anthropic.claude-code/extension directory per §8.2 — was rejected because Claude Code does not read extension directories, which would make the overlay decorative.Both findings are being written up for the ecosystem rather than kept as CAO trivia — non-normative, implementer-facing notes for
agentplugins/agent-plugins-spec(client manifest-location convergence, and glob-vs-symlink semantics for delegated discovery). Nothing in this PR depends on them.The second finding is the kind of bug only a real client surfaces: a client that delegates skill discovery to another program's glob inherits that program's
**-over-symlink behaviour.glob.glob(recursive=True)descends into symlinked directories;pathlib.Path.globdoes not. Projected plugin skills are symlinks, so a pathlib-shaped reading inside a client would hide every plugin skill — invisibly, and untestable from CAO's side. CAO emits a single-level glob as well, which is immune, and the deviation is recorded with a tripwire test that fails ifpathlib's behaviour ever changes.How this was built
Worth stating because it is the reason the evidence above exists, and it maps onto the frontier-team practices rather than aspiring to them.
Intent was explicit before code.
docs/issues/573-agent-plugins/carries requirements with numbered acceptance criteria, a design with the rejected alternatives written down, and a sequenced task breakdown — following this repo's existingdocs/issues/convention. Tests cite the criteria they satisfy (**Validates: Requirement 13.2, 18.9**), so traceability is checkable rather than claimed. One requirement was amended mid-flight when the design turned out to presuppose a delivery obligation it never stated; the amendment note says so instead of quietly renumbering.Correctness was derived from the requirements. Property-based (Hypothesis) suites cover validation totality, containment, install/remove algebra, delivery equivalence and the schema pin — asserting properties the requirements imply, not examples someone thought of. Where a duplicate of an existing property was skipped, the module docstring names the test that already covers it, so the decision is auditable.
Testing shifted left, and the guardrails have teeth. The recorder gates on a real run.
make check-agent-pluginfails on package drift. The schema pin fails on tampering. Docs guards assert the warning is in the first screenful and the prerequisites are facts rather than guesses. A vocabulary guard keeps "event plugins" and "agent plugins" apart, with the two permanent exemptions listed as data. And new behaviour is mutation-verified: each fix was confirmed to fail when the thing it asserts is removed — stringify the disabled boolean, drop the collision guard, drop the reconcile, force the ownership predicateTrue, and each breaks its own test. One extra mutation was run purely to prove a test was not a tautology.Agents were fed, not babysat. The work ran as parallel agents against written task files with an independent reviewer auditing the result, and the two hardest findings in this PR came from that adversarial loop rather than from CI: a review comment identified the OpenCode removal gap, and verifying that turned up a worse sibling — a silent overwrite of a user's hand-written config at install time.
The claims are bounded. Where something could not be verified it says so: AC5 is gated, the Gemini-family path is documented rather than tested, and the OpenCode behaviour is pinned to the version it was measured against.
The decisions worth reviewing
Skills are projected into
SKILLS_DIR, not registered as a search directory. The highest-consequence choice here. Appending plugin roots to_skill_search_dirs()is a one-function change that cannot cover Kiro CLI or OpenCode: Kiro gets onlyskill://globs rooted atSKILLS_DIR, OpenCode reads through a single symlink to it, so a skill stored anywhere else is invisible to both — including CAO's own default provider. Asserted inTestTheRejectedAlternativeWouldNotHaveWorked.MCP servers are delivered by re-mapping, not by persisting the mapping. The expansions are absolute paths in
args/env/cwd; a persisted record would serve paths that no longer exist onceCAO_HOME_DIRmoves, a--forcereinstall would keep serving the old record, and an edited tree would disagree with its record silently. Re-mapping makes the delivered set a pure function of what is on disk now — the propertyprojection.pygets by rebuilding rather than patching. Review 1 found the mapping was applied on the install path only, while five providers re-read the profile from disk at launch and discarded it; the merge now happens on the read seam that every launch passes through, so the property holds where it is actually consumed. Cost is one JSON read plus one offline validation per plugin.Collisions resolve by a rule, never by history.
installed_atis deliberately not the key — it would elect different winners for A-then-B and B-then-A from the same final installed set.OpenCode's shared config needed two guards the other providers do not. Kiro's and Copilot's per-agent files are rewritten wholesale, so a withdrawn server simply is not written. OpenCode's
opencode.jsonis edited in place and has no delete. Left alone that meant (a) a removed plugin's server survived withenabled: truepointing at a just-deletedPLUGIN_ROOT— a failingposix_spawn/ENOENT on every launch — and (b) an install silently overwrote a user's hand-written entry, because the "profile always wins" rule is scoped to the agent profile'smcpServersand never sees the shared file. Removal now disables in place; install refuses to overwrite an entry it cannot prove it placed. That disable rests on OpenCode not spawningenabled: false, verified rather than assumed — two strictenabled === falsegates in the shipped 1.18.15 bundle both return before the only spawn path, confirmed empirically with a firing positive control. Because the gate is strict the helper writes a real JSON boolean;"false"or0would still spawn, and a unit assertion pins that. Ownership was originally a documented heuristic, claimed to fail only toward over-preservation. Review 1 disproved that: a force update that changed the command made CAO misclassify its own entry as user-owned, keeping the stale command and dropping the tool grant. Ownership is now decided by whether the entry's command resolves inside the CAO-managed plugin store, so CAO's own entries stay updatable while a genuinely user-authored entry is still never overwritten.Testing
Remaining Python failures are pre-existing (
test/services/agui, the AG-UI tests intest/api, OpenTelemetry) — none in any area this change touches, established by comparing failing-test-name sets againstmainin both directions rather than comparing counts.black/isortclean on the CI scope;mypy src/matches its pre-existing baseline with no new errors in touched files;validate_markdown_links.pyexits 0;cargo fmt/clippy -D warningsclean;make check-agent-pluginandmake check-agent-plugins-schemasgreen.Still open, by design
agy plugin installreads identity and skills from the untouched portable layout but takes plugin MCP servers only from a rootmcp_config.json; the bridge is one generated byte-identical file, measured against 1.1.11.cursor-cli.md,opencode-cli.md), justified in the guard's own data rather than silently skipped.mcp.jsonIncrement 1 does not ship). Happy to split into two commits for review — commit re-ordering, not a rewrite.I confirm this contribution is made under the terms of the project's Apache-2.0 license.