Repository navigation
feat: discover and validate Agent Plugins manifests - #29321
jscottmiller wants to merge 1 commit into
Conversation
3578f4d to
992390e
Compare
3ebe295 to
950ecc4
Compare
950ecc4 to
6d46ac9
Compare
6d46ac9 to
a41a621
Compare
a41a621 to
28fa2a2
Compare
28fa2a2 to
2699387
Compare
69ca917 to
8f41fda
Compare
8ef3c91 to
30f213e
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
Round 7. Reviewers verified the round 5 fixes: orphaned plugin skills dropped after the caps, hasLinkInside at a drive root, the lock-free role map, the shared MaxContextSourceBytes, the new watcher and junction tests, the once-only Info notice, and the wording fixes. The symlinked plugin.json watch gap is tracked in CODAGT-1354. New: 1 P2, 6 P4, 2 Nit.
The P2 is the same class as earlier whole-push rejections: two rows with one Source, which coderd rejects as a whole and the agent resends forever. Its trigger is narrow (a declared scan root that is a skill directory holding an Agent Plugins manifest). The Windows junction test runs only in CI and was not run here.
Notes:
agent/agentcontext/watcher.go:292: the commit body says the event loop cannot deadlock againstSyncon Windows, butschedule()still takesw.mufor each relevant event (the pre-existing case listed out of scope in round 5). The code comment is accurate; the squash merge uses the PR description, so only the commit body overstates it.agent/agentcontext/resolve.go:786:withoutOrphanedPluginResourcesis the sixth agent-side copy of a coderd whole-push validation rule. Per-resource rejection in coderd and not resending a rejected push, both listed out of scope earlier, would remove the need for these copies.
🤖 This review was automatically generated with Coder Agents.
30f213e to
8d14dc9
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
Round 8. Reviewers verified the round 7 fixes: the notice logging again when PluginsSupported changes, WouldResolveOn reading the roles Sync publishes, the single role map, plugin fields in both snapshot converters, Debug logs for invalid plugins, and the wording fixes. The CRF-126 fix is partial and is re-raised on its thread at P3: pushes are now accepted, but the post-discovery pass can reject or drop a valid plugin. New: 3 P4, 2 Nit.
Notes:
agent/agentcontext/resolve.go:301: in the skill-root-first order, the plugin row and itsskills/*are dropped with no log of their own, and no test covers a plugin skill in that layout. The CRF-126 re-raise fix covers this.agent/agentcontext/resolve.go:301:withoutDuplicateSourcesis the seventh agent-side copy of a coderd whole-push rule. Per-resource rejection in coderd would remove the need for these copies.
Out of scope: coderd/agentapi/context.go:329 rejects the whole push when two rows of different kinds share a Source; the ID already includes the kind.
🤖 This review was automatically generated with Coder Agents.
8d14dc9 to
5ba3985
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
Round 9. Reviewers verified the round 8 fixes: deduplication on Source inside appendResource with the plugin name claimed only after its row is kept, the Debug skip log, the notice constants, the watched rename, and the description sentences. New: 6 P4, 2 Nit. Four of them follow from the Source rule: a skipped plugin's nested containers now load, an invalid plugin row is dropped without an error row, a symlinked root still gives two rows for one directory, and the new test does not check that a skipped plugin loads no skills.
Out of scope (needs a ticket or explicit acceptance by a human):
agent/agentcontext/mcp.go:68:KindMCPServerrows use the server name asSourceand skip the walk'sSourcededup, so a.mcp.jsonkey equal to a filesystem resource path makes coderd reject every push.
agent/agentcontext/resolve.go:869
P4 [CRF-149] The resourceID doc says sources never collide across kinds because each kind owns a distinct file-name pattern; this PR adds a skill and a plugin that share one directory Source. (Melody, Mafuuu)
The walk comment added this round (resolve.go:281) says the opposite, and a reader who trusts the resourceID doc can switch the walk back to ID dedup and bring back the whole-push rejection. Say that Source can repeat across kinds and that the walk deduplicates on Source.
🤖
agent/agentcontext/resolve.go:621
Nit [CRF-150] The readFileResource comment says symlinked instruction files share an ID and collapse through the walk's ID-based dedup; the walk now deduplicates on Source. (Mafuuu, Knov, Razor, Melody)
The collapse still happens because the files share Source. Change it to "share a Source and collapse to a single resource via the walk's Source-based dedup".
🤖
🤖 This review was automatically generated with Coder Agents.
5ba3985 to
0b4472b
Compare
d6a9e6b to
f42c800
Compare
f42c800 to
1b47e38
Compare
40032a6 to
a02e876
Compare
a02e876 to
f6b5a68
Compare
f6b5a68 to
0e53c51
Compare
ibetitsmike
left a comment
There was a problem hiding this comment.
Reviewed 3c89c18 against its base 831ef99 (#29320). This was a multi-agent review: 10 reviewers (correctness, security, concurrency, tests, and design, each on two models) worked at the exact head with throwaway probe tests and mutation runs, and 2 adjudicators re-ran the probe for every P1 to P3 finding and checked them against the 114 resolved bot threads and the later layers through #30577. Build, vet, go test -race of the touched packages, and golangci-lint were clean at this head.
Overall: go with fixes. Static containment held across every symlink layout the security reviewers tried, the manifest typing is thorough, and the one-row-per-Source rules are careful. The problems are in the experiment contract, the push fallback, and edge paths.
What I'd fix before merge:
- P1: the agent no longer knows whether the
agent-pluginsexperiment is on, so default deployments get plugin behavior, including a skills regression against main (resolve.go). - P2: one transient coderd error turns plugin data off for the rest of the connection (
drpc.go). Dialing the plugin API minor and deleting the fallback is the cleaner fix, but fix the 1000-row and NUL comments first, because the latch currently absorbs them.
The P3 comments cover the 1000-row push cap, NUL in manifest metadata, the raw $schema in errors, watches after a directory replace, the spec 7.1 skill checks, silent skips, two Source dedup interactions, the docs, a FIFO SKILL.md hang (it predates this PR for plain skills), and 13 mutants that survive the package tests. The Obs and Nit comments are optional.
Outside this diff:
- #29326 starts plugin MCP servers with the experiment off (see the P1 comment), and also for plugins under declared sources, although the docs say MCP servers load only from the working directory.
- #29320: main has moved to Agent API v2.15, and main's v2.13 is a different change (
app_nameandconnection_method). The stack's v2.13PluginBodyneeds a new minor on rebase, with the agent dial bumped to it, which also enables the structural fix for the P2.
Not covered: nobody ran Windows or macOS, so the Windows notes rest on standard-library and fsnotify code traces. Later layers were checked for fixes and consumers, not re-reviewed.
Xum wrote this review on behalf of Mike.
| } | ||
| r.discoverTopLevelFiles(root, out, seenID) | ||
| var pluginSkillsContainer string | ||
| result := r.discoverPlugin(root.Path, root, out, seenSource, pluginNames, pluginAtScanRoot) |
There was a problem hiding this comment.
P1: the agent no longer knows whether the agent-plugins experiment is on, so default deployments get plugin behavior.
The PluginsSupported gate (WithPluginsEnabled, SetPluginsEnabled) was still in 5ba3985, the last commit the bot reviewed, and no layer through #30577 has an agent-side experiment signal now. The description says coderd drops plugin data when the experiment is off, but that filter only drops rows. It can't undo what the agent changes locally:
- Regression against main. A scan root with an Agent Plugins
plugin.jsonnow owns itsskills/directory (lines 333-353). With a valid manifest those skills carryPluginNameand coderd drops them; with a rejected manifest they aren't loaded at all. Through the real coderd validator with the experiment off, the root stores 1 skill row withoutplugin.jsonand 0 with it, so a workspace that develops an Agent Plugin loses its skills in chats. - Cap crowd-out.
capRankputs plugin rows ahead of skills for the 500-row cap. 500 plugins plus 1 plain skill leave the plain skillexcluded, and coderd then drops the plugins too. - Watches. Every
plugins/<x>and.agents/plugins/<x>directory gets an inotify watch, manifest or not (watcher.go:391-395; 300 empty directories gave 302 watches). CRF-13 was fixed by the gate that's gone now, andplugins/is a common directory name in unrelated repositories. - feat: run plugin MCP servers discovered by agentcontext #29326 starts plugin MCP servers.
mcpConfigSourcesstarts themcp.jsonservers of every OK plugin in the local snapshot with no experiment check, so plugin stdio commands run on deployments that never enabled the experiment. As far as I traced, their tools never reach chats (coderd drops the rows, and chatd builds workspace MCP tools from pinned rows), so the processes run without being usable. A working-directory.mcp.jsonalready starts servers, so this is an admin-control problem rather than a new way to run code. - The missing-directory notice always recommends
~/.coder/.agents/pluginsagain, which undoes the CRF-5 fix, and the push fallback's "could not be loaded" notice also reaches experiment-off deployments.
That leaves the ticket's "Experiment-gated" criterion unmet.
Fix: restore an "agent plugins enabled" bit on the manifest (#29320) and gate discoverPlugin, the plugin containers, plugin watches, the notice hint, and the #29326 plugin MCP sources on it.
Test: with plugins disabled, a root holding a valid plugin.json and skills/deploy/SKILL.md resolves to one plain deploy skill and no KindPlugin row, plugins/a/ without a manifest isn't watched, and #29326 starts no plugin MCP source.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed. #29320 restores the bit as Manifest.agent_plugins_enabled, which coderd sets when the experiment is enabled (8b7f074). This is the kind of wire change you were wary of in r4254922682, but no agent-side mitigation could keep #29326 from starting plugin MCP servers on experiment-off deployments. The agent calls SetPluginsEnabled from every manifest (9e9c004). With it off, the resolver reads no plugin.json, so a root's skills/ holds plain skills and no plugin rows compete for the cap, plugin directories aren't watched, the missing-directory notice drops the ~/.coder/.agents/plugins hint, and #29326 adds no plugin MCP sources (20bca8d). The push fallback and its notice are gone (see the P2 thread). Tests: TestPlugin_DisabledEmitsNothingPluginRelated, TestWatcher_PluginDirsWatchedOnlyWhenEnabled, TestAgent_MissingDirectoryNotice, and TestAgent_PluginMCPServersNotLoadedWithPluginsDisabled. (Coder Agents generated)
| resp, err := p.send(ctx, req) | ||
| // TODO(jscottmiller): remove this retry and the pluginsRejected latch | ||
| // once Agent Plugins have been stable in production for a few releases. | ||
| if err == nil || errors.Is(err, ErrPushUnimplemented) || | ||
| errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || | ||
| ctx.Err() != nil || !slices.ContainsFunc(req.Resources, isPluginData) { | ||
| return resp, err | ||
| } | ||
| resp, retryErr := p.send(ctx, withoutPluginData(req)) | ||
| if retryErr != nil { | ||
| return nil, err | ||
| } | ||
| p.mu.Lock() | ||
| p.pluginsRejected = true | ||
| p.mu.Unlock() | ||
| p.Logger.Warn(ctx, "coderd rejected Agent Plugins data; pushing context without plugins on this connection", | ||
| slog.Error(err)) | ||
| return resp, nil |
There was a problem hiding this comment.
P2: one transient coderd error turns plugin data off for the rest of the connection.
This retries without plugin data on every error except Unimplemented and context errors, and sets pluginsRejected as soon as the stripped retry succeeds. coderd returns plain errors for database failures inside ReadModifyUpdate. The first attempt rolls back, so the immediate stripped resend succeeds, and the pusher lives as long as the connection (agent.go), which is often days.
A fake client whose call 1 fails with get latest snapshot: driver: bad connection shows it: calls 2 to 5 go out without plugin data and with the snapshot error "Agent Plugins could not be loaded. Check the workspace agent logs for details." That includes call 5, whose request had no plugin data in the first place. coderd's next accepted push deletes the stored plugin rows, and the Warn log carries the database error, which reads like a plugin problem.
Preferred fix: the fallback only exists for an agent that is newer than its coderd. The stack still dials ConnectRPC212WithRole while PluginBody is Agent API v2.13, and main bumps the dial with each minor (it dials ConnectRPC215WithRole now). Dialing the plugin minor lets you delete the mutex, the latch, withoutPluginData, the notice, the Logger wiring, and their tests (about 290 lines), which also fits the downtime-upgrade support model.
Order matters: today the latch also absorbs the cases in the 1000-row and NUL comments by dropping only plugins. Without the latch those inputs fail every push and stall all context sync, as on main for plain skills (CODAGT-1313), so fix them first or in the same change.
Minimal fix if the fallback stays: latch only when the full request fails again on a second try while the stripped one succeeds, append the notice only to requests that carried plugin data, and correct the type doc at lines 25-29.
Test: a fake client that fails only the first call; the second push still carries plugin data and an empty SnapshotError. The DeadlineExceeded and ctx.Err() guards on lines 72-73 are untested too: deleting either one keeps the package green.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed with the preferred option. #29320 bumps the Agent API to v2.16 and the agent now dials ConnectRPC216WithRole (8b7f074), so an older coderd refuses the connection, as it does for every minor. 9e9c004 deletes the mutex, the latch, withoutPluginData, the notice, the Logger wiring and their tests. The 1000-row and NUL fixes landed in the same change (see those threads). (Coder Agents generated)
| @@ -677,11 +774,12 @@ func (r *Resolver) emitSkillsFromContainer(container string, root ScanRoot, out | |||
| // append additional resources (e.g. MCP server tool lists) can | |||
| // apply the same byte cap to the appended slice. | |||
| func (r *Resolver) applyCaps(resources []Resource) ([]Resource, uint64) { | |||
There was a problem hiding this comment.
P3: rows past the 500 cap stay as excluded stubs, so more than 1000 plugin directories make coderd reject the push, and the fallback then turns every plugin off.
applyCaps marks rows past MaxResources StatusExcluded but keeps them (lines 789-792), and coderd rejects any push with more than maxContextResourcesPerPush = 1000 rows (context.go). 1001 .agents/plugins/pNNNN/plugin.json files, even {}, give resources=1001. The stripped resend then latches every plugin off, including the user's own ~/.coder/.agents/plugins.
The stub pattern predates this PR for plain skills (on main the whole push fails). This PR adds a new input family and the plugin-wide consequence. It needs 1000 or more directories, hence P3.
Fix: drop rows past a hard bound below 1000 and set a snapshot error that names the count.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 9e9c004: the resolver drops excluded rows past workspacesdk.MaxContextResources (1000, the constant coderd's push cap now uses) and sets SnapshotError to "snapshot has N resources; excluded resources past the 1000-resource push cap were dropped". Covered by TestResolver_PushCapDropsExcludedRows. (Coder Agents generated)
| m.Version = strutil.Truncate(codersdk.SanitizePromptText(m.Version), MaxPluginVersionRunes) | ||
| m.Description = strutil.Truncate(codersdk.SanitizePromptText(m.Description), MaxPluginDescriptionRunes) |
There was a problem hiding this comment.
P3: a JSON-escaped NUL in description or version passes validation, PostgreSQL rejects the JSONB body, and the fallback then turns every plugin off.
SanitizePromptText keeps U+0000, and the plugin is marked OK. The resource body column is jsonb (migration 000522), and PostgreSQL 16 rejects the write with "unsupported Unicode escape sequence" because U+0000 cannot be converted to text (reproduced against a throwaway database). One repository manifest therefore disables all plugins, from every root, for the connection.
Fix: strip U+0000 (or reject the plugin with a clear error) before marking it OK.
Test: a manifest whose description holds the escaped NUL produces a storable body or an invalid row, and a sibling plugin still loads. Related gap: removing SanitizePromptText from the version on line 273 keeps the package green, because the "capped and sanitized" row only puts a zero-width character in the description. A zero-width character in the version would pin it.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 9e9c004: pluginText strips U+0000 from the version and description before the plugin is marked OK. New row NUL removed from version and description in TestPlugin_ManifestValidation, and the capped and sanitized row now also puts a zero-width character in the version. Skill frontmatter gets the same strip (TestResolver_SkillFrontmatterNULRemoved). (Coder Agents generated)
| return pluginManifest{}, false, &foreignManifestError{msg: fmt.Sprintf("plugin.json has no $schema; Agent Plugins manifests declare %q", PluginSchemaV1)} | ||
| } | ||
| if json.Unmarshal(raw, &schema) != nil || !strings.HasPrefix(schema, pluginSchemaPrefix) { | ||
| return pluginManifest{}, false, &foreignManifestError{msg: fmt.Sprintf("plugin.json $schema %s is not an Agent Plugins schema; Agent Plugins manifests declare %q", string(raw), PluginSchemaV1)} |
There was a problem hiding this comment.
P3: the foreign-$schema error embeds the raw JSON bytes, so invalid UTF-8 breaks the local snapshot RPCs and control characters reach the CLI.
string(raw) copies the raw $schema value into Resource.Error. The coderd push repairs UTF-8 (drpc.go:145, so the CRF-65 fix holds there), but the agent socket's contextSnapshotToProto copies Error verbatim into a proto3 string (agent/agentsocket/service.go:327).
A .agents/plugins/bad/plugin.json whose $schema string holds the raw bytes 0xFF 0xFE gives a non-UTF-8 Error, and proto.Marshal of the socket resource fails with string field contains invalid UTF-8. GetContextSnapshot and ResyncContext both use that converter, so coder exp chat context show and the in-workspace coder exp chat context refresh fail for the whole snapshot. CR, LF, U+202E, and C1 bytes also pass through to the CLI table; every other manifest error here uses %q. Only those two commands consume it, hence P3.
Fix (one line): format the value with %q, or say "is not a string". strings.ToValidUTF8 in contextSnapshotToProto is cheap defense in depth.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 9e9c004: a non-string $schema now reads "plugin.json $schema is not a string", and a string value is quoted with %q. contextSnapshotToProto also runs every string field through strings.ToValidUTF8. Covered by the $schema with invalid UTF-8 reported as valid UTF-8 row in TestPlugin_ManifestValidation and by TestDRPCAgentSocketService_Context/InvalidUTF8Repaired. (Coder Agents generated)
| // Windows junction, or cannot be read. Lstat follows links in all but | ||
| // the last component, so each one is checked. Components above base are | ||
| // trusted; for a path outside base only its last component is checked. | ||
| func hasLinkInside(base, path string) bool { |
There was a problem hiding this comment.
| // pluginShadowed means the plugin is valid but another resource | ||
| // already holds its Source, so nothing was emitted. Its directory | ||
| // is still treated as a plugin root. | ||
| pluginShadowed |
There was a problem hiding this comment.
Nit: pluginShadowed behaves exactly like pluginValid at its only consumer (resolve.go:338).
Returning pluginValid instead and deleting this state keeps every test green.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Removed in 9e9c004: discoverPlugin returns pluginValid, and the docs no longer mention a shadowed state. (Coder Agents generated)
| // MaxPluginVersionRunes caps the manifest version carried on a | ||
| // plugin resource. | ||
| MaxPluginVersionRunes = 64 | ||
| // MaxPluginDescriptionRunes caps the manifest description carried | ||
| // on a plugin resource. | ||
| MaxPluginDescriptionRunes = 1024 |
There was a problem hiding this comment.
Nit: MaxPluginVersionRunes and MaxPluginDescriptionRunes are used only by this package and its tests through #30577.
The reason given in CRF-41 for keeping them exported applies only to PluginSchemaV1, which coderd/x/chatd tests use, so these two could be unexported and re-exported through export_test.go.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Done in 9e9c004: both are unexported and re-exported for tests through export_test.go. PluginSchemaV1 stays exported. (Coder Agents generated)
| // pluginRejected means a plugin resource with a non-OK status | ||
| // was emitted and none of the plugin's components were loaded. |
There was a problem hiding this comment.
Nit: this doc, and the discoverPlugin doc on lines 314-315, promise a non-OK plugin row, but the diagnostic can instead be a warning on an OK skill row (resolve.go:416-418).
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 9e9c004: the pluginRejected and discoverPlugin docs now say a rejected plugin is reported as a non-OK plugin row, or as a warning on an OK resource that holds the same Source. (Coder Agents generated)
| // - Agent Plugins (agent-plugins.org, spec 1.0.0). A plugin is a | ||
| // directory whose plugin.json carries the agent-plugins $schema; |
There was a problem hiding this comment.
Nit: "a directory whose plugin.json carries the agent-plugins $schema" isn't true for .agents/plugins/, where a manifest without $schema is reported rather than ignored.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 9e9c004: doc.go now says a plugin.json without the Agent Plugins $schema is ignored at a scan root and in plugins/, and reported in .agents/plugins/. (Coder Agents generated)
The agentcontext resolver recognizes Agent Plugins (agent-plugins.org, spec 1.0.0) directories and emits them as KindPlugin resources with their skills attributed to the plugin. Discovery: a plugin is an immediate child of plugins/ or .agents/plugins/ under a scan root, or a scan root that itself holds a plugin.json. A plugin.json at a scan root, or in plugins/ (a name other tools also use), whose $schema is absent or not an https://agent-plugins.org/schemas/ URL may belong to another tool and is ignored after a debug log; at a scan root an unreadable or malformed file is ignored too. Every other problem is reported as an invalid plugin with the reason, and in .agents/plugins/ every plugin.json is taken as an Agent Plugins manifest. Rejected and duplicate manifests are logged at Debug through Resolver.Logger; their rows carry the error. A scan root that validates as a plugin keeps its instruction files, .mcp.json and other skill containers, but its skills/ belongs to the plugin and its plugin containers are not scanned. Symlinked plugin directories and plugin containers are skipped, as is a plugin or plugin skill directory whose path is longer than workspacesdk.MaxContextSourceBytes (1024), coderd's Source cap, or whose name is not valid UTF-8. The plugin root is canonicalized once and is the containment boundary for plugin.json, the skills/ container, each skill directory, and every SKILL.md: following spec section 4.1, each may be a symlink that resolves inside the plugin root. An escaping skills/ disables the skills component with a warning on the plugin resource, and an escaping skill directory or SKILL.md is reported as an invalid skill. Canonicalization falls back to the cleaned path when EvalSymlinks fails, which it does on Windows substituted drives, but only for an existing directory with no link between the scan root and the path. Validation follows spec section 5 by hand: name grammar (via workspacesdk.ValidatePluginName), optional string fields, keywords as strings, closed author object, extensions as object. Unknown top-level fields and a non-object extensions value are recorded in Resource.Error while the status stays OK; every other violation, an unsupported schema version, invalid JSON, or an oversize manifest rejects the plugin and loads no components. Version and description are sanitized, stripped of U+0000 (which coderd's jsonb storage rejects), and capped at 64 and 1024 runes before they reach the resource; the spec sets no limit on either, so the caps are Coder's. Skills are deduplicated by (PluginName, Name) so a plugin skill and a plain skill of the same name both survive. A plugin name already claimed by another plugin root makes later same-named plugins StatusInvalid with no components. After the caps, skills and MCP servers whose plugin is not StatusOK (for example one the count cap excluded) are dropped. Resources are deduplicated on Source as they are found, across kinds, for the same reason: a skill directory that is also a scan-root plugin keeps the earlier scan root's row unless that row is invalid and the later one valid; skill directories are compared after resolving symlinks. A valid plugin whose row is not kept claims no name, emits no skills, and is still treated as a plugin root, so its plugin containers are not scanned. A rejected plugin that loses to an OK skill adds its error to the skill row as a warning. Resource gains PluginName, set on resources that belong to a plugin, and PluginVersion. The DRPC pusher maps KindPlugin to PluginBody and nests each resource with a PluginName in the OK plugin of that name; a resource without one is left out of the push, though the aggregate hash still covers it. Every resource error has invalid UTF-8 replaced and is truncated on a UTF-8 boundary to workspacesdk.MaxContextErrorBytes (4096). The resolver drops the excluded rows past workspacesdk.MaxContextResources (1000), coderd's push cap, and reports the count in SnapshotError. coderd's agentapi now uses these constants and workspacesdk.MaxContextSourceBytes in place of its own. Manager.SetPluginsEnabled stores the flag and re-resolves on change; both resolve paths pass it to Resolver.ResolveContext through ResolveOptions, and Resolve keeps plugin discovery off. The agent sets the flag from Manifest.AgentPluginsEnabled on every manifest. With it off, the resolver reads no plugin.json, a scan root's skills/ holds plain skills, and no plugin directories are watched. Watcher syncs are serialized and each reads the current scan roots and options, so an older root set never replaces a newer one. With plugins enabled, the fsnotify watcher registers the plugin containers under each scan root (or the existing parent of a missing .agents/plugins), each immediate plugin directory, its skills/ container, and the skill directories the resolver would read, and treats plugin.json events as relevant. Skills are watched only for a directory holding a plugin.json, and in a plugin directory only plugin.json and skills events re-resolve. A scan root that holds a plugin.json has its skills/ watched the same way. Sync publishes each directory's watch role through an atomic pointer, and the event loop reads it without the watcher lock, so filtering an event never waits on a Sync that is blocked in fsnotify Add or Remove on Windows. The built-in ~/.claude/plugins/cache scan root is removed: Claude's .claude-plugin manifests are not read. When the manifest has no directory the agent logs, at Info and again only when Manifest.AgentPluginsEnabled changes, a notice listing the working-directory entries it does not scan (instruction files, .mcp.json, skill containers, and with plugins on plugin.json and the plugin containers) and, with plugins on, pointing at ~/.coder/.agents/plugins, since templates without coder_agent.dir otherwise give no signal. The agent socket's ContextResource and the HTTP API's SnapshotResource gain plugin_name and plugin_version, and `coder exp chat context show` prints a plugin column. Discovery edge cases: - In .agents/plugins/, a symlinked plugin directory and a plugin.json that cannot be stat'ed are reported as plugin rows instead of skipped. - Plugin skills follow the Agent Skills limits (spec 7.1): a missing or over-1024-character description, or a name over 64, is invalid. - A skill hidden by an earlier same-named skill claims no Source, so a plugin at its directory still loads. - A non-OK plugin skill is keyed on its own Source, so a plain skill its symlink points at no longer hides it. - U+0000 is removed from skill names and descriptions, and a byte-order-marked plugin.json is rejected with an error that says so. - The foreign $schema error quotes the value, and the agent socket's snapshot converter makes every string valid UTF-8. The watcher re-adds a watched directory that was removed or renamed since the last Sync, so a replaced plugin directory is watched again. The debounce timer has its own lock, so the event loop no longer takes the lock Sync holds across fsnotify calls. The extending-agents docs gain an Agent Plugins (experimental) section covering the experiment, the plugin locations, and the $schema rule for each, and say that declared sources contribute plugins too. The platform controls experiments list links to the section.
tl;dr: The workspace agent discovers and validates Agent Plugins directories and reports each plugin, with its skills nested in it, as context resources.
The
agentcontextresolver treats each immediate child ofplugins/or.agents/plugins/under a scan root, or a scan root that is itself a plugin, as a plugin candidate. Aplugin.jsonis validated against spec section 5 in code (the published name regex is not RE2-compatible), and plugin skills must also meet the Agent Skills limits that spec section 7.1 applies. A manifest whose$schemais absent or not anhttps://agent-plugins.org/schemas/URL is ignored at a scan root or inplugins/, since another tool may own it. In.agents/plugins/every problem, including a symlinked plugin directory or aplugin.jsonthat can't be stat'ed, produces an invalid row with the reason. All plugin paths are contained to the canonical plugin root (in-root symlinks are allowed per spec section 4.1). Skills are deduplicated on(plugin, name), so a plugin skill and a plain skill with the same name are both sent; chatd tells them apart only from #29323 on. The built-in~/.claude/plugins/cachescan root is removed.Discovery runs only while
Manifest.AgentPluginsEnabledis set. With it off, the resolver reads noplugin.json, a scan root'sskills/holds plain skills, and no plugin directories are watched. The DRPC pusher nests each plugin's skills in itsPluginBody. Resource errors are made valid UTF-8 and truncated toworkspacesdk.MaxContextErrorBytes, the resolver drops excluded rows pastworkspacesdk.MaxContextResources(1000) and reports the count in the snapshot error, and U+0000, which coderd's jsonb storage rejects, is stripped from manifest text and skill frontmatter. When the template sets no agentdir, the agent logs a notice listing the working-directory context it skips, and logs it again whenever the plugin setting changes.Because coderd rejects a push that repeats a
Source, the resolver keeps one resource perSourceacross kinds: the earliest scan root's row wins, an OK row replaces an invalid one, and a skill hidden by an earlier same-named skill claims none. The watcher registers plugin containers, plugin directories, and their skills, watches a replaced plugin directory again on the next sync, and keeps its event loop off the lockSyncholds across fsnotify calls. The agent socket, the HTTP snapshot API, andcoder exp chat context showgain the plugin name and version, and the extending-agents docs gain an Agent Plugins (experimental) section.PR map
scott/agent-plugins/1-api..a7a0398e1fagent/agentcontext/plugin.go(+669/-0, new): plugin discovery, manifest validation, plugin skills with the spec 7.1 limits, path containment, and rows for symlinked plugin directories in.agents/plugins/.agent/agentcontext/resolve.go(+283/-64): plugin containers,ResolveOptions.PluginsEnabled,resourceIndex(one resource perSource; a name-shadowed skill claims none), the 1000-row cap, and U+0000 stripping in skill frontmatter.agent/agentcontext/watcher.go(+186/-54, concurrency): watches plugin containers and plugin skill directories, re-adds a replaced directory on the nextSync, and gives the debounce timer its own lock.agent/agentcontext/drpc.go(+69/-14):PluginBodywith nested skills, and error truncation with UTF-8 repair.agent/agentcontext/manager.go(+61/-15, concurrency):SetPluginsEnabledand serialized watcher syncs.agent/agent.go(+31/-0): sets the plugin flag from the manifest and logs the missing-directory notice.codersdk/workspacesdk/workspacesdk.go(+12, API):MaxContextResources,MaxContextErrorBytes, andMaxContextSourceBytes, whichcoderd/agentapi/context.go(+12/-15) now uses.agent/agentsocket/,agent/agentcontext/api.go,cli/exp_chat.go(+71/-53, API): plugin name and version on snapshot resources; the socket converter makes strings valid UTF-8.This PR was generated by Coder Agents on behalf of @jscottmiller.