feat(plugin): universal plugin system — OMP and Claude Code compatibility - #105
Open
giveen wants to merge 28 commits into
Open
feat(plugin): universal plugin system — OMP and Claude Code compatibility#105giveen wants to merge 28 commits into
giveen wants to merge 28 commits into
Conversation
…nToolResult pipeline, quickstart docs New surfaces: - marketplace.go: MarketplaceClient resolving bare names via JSON registry (LATE_PLUGIN_REGISTRY), falls back to plain npm on 404. - hooks.go: CallOnToolResultHooks now returns ([]byte, error) — sequential mutation pipeline; empty stdout = pass-through, valid JSON = replace result, literal "blocked" = veto. - installer.go: Install() dispatcher (URL/path/npm/marketplace), Update() and UpdateAll() with atomic git temp-clone+rename and npm @latest. - command.go: handlePluginInstall routes through Install(); handlePluginUpdate routes through Update/UpdateAll; removed dead isGitURL/isLocalPath helpers. - update_test.go: 6 tests covering npm happy-path, local-source refusal, unknown-name error, exec propagation, bulk update, marketplace fallback. - marketplace_test.go: stub server + resolve tests for npm/git/404 entries. - manifest.go: InstalledPlugin.Source string field tracks original install arg. Quickstart docs: - quickstart.md + quickstart.zh-CN.md: new Plugins section covering discovery paths, install from npm/git/local/marketplace, enable/disable/remove/update commands, and surface table (skills/MCP/commands/themes/hooks/tools). - plugin-sdk.md: documented marketplace install, update command, tool-result mutation semantics.
…oop via BuildToolResultMiddlewares - hooks.go: new BuildToolResultMiddlewares() returns post-execution middleware that calls CallOnToolResultHooks after tool success, skips hooks on error. - main.go: appends the new middlewares to the root agent's middleware chain right after the existing BuildHookMiddlewares() line. - hooks_themes_test.go: two tests — one verifies result mutation after a successful tool call, one verifies hooks are skipped on tool errors.
- manager.go: drop unused 'context' import (replaced by package-level seams) - command.go: drop unused 'os' and 'os/exec' imports (removed inline updatePlugin that used exec.Command; new Update/UpdateAll APIs own it) - command.go: change `for i, a := range args` to `for _, a := range args` since the loop index was unused - hooks.go: add 'sort' to imports (sort.Slice is used at L150) - hooks.go: wrap runHook's string return in []byte before assigning to json.RawMessage (RawMessage is []byte, not string) - watcher.go: use = instead of := for struct field reassignment - watcher.go: remove redundant `_ = os.Stat(...) if _, err := os.Stat(...)` duplicate - update.go: rename `func (m *Model) PluginCommands() []string` to `func (m *Model) ListedPluginCommands() []string` to fix field/method shadowing — inside the method body, `m.PluginCommands` was resolving to the method itself, breaking `len(m.PluginCommands)` callers in view.go and other call sites - plugin_test.go: update Empty test to read field directly, SetAndGet test to call the renamed ListedPluginCommands() method
Last commit was too aggressive — removed the os import and renamed the loop index from i to _ in parseProjectFlag, but both are actually used: - `os` is required for os.Stderr, os.Stdout and os.UserHomeDir calls throughout the file (HandlePluginCommand, printPluginUsage, handlePluginList, handlePluginLink and the install/remove/enable paths) - The loop index `i` is consumed by the inner `for j, r := range args` to omit the --project/--local flag from the returned args. Renaming to _ broke that comparison. Restoring both to their pre-fix state.
- main.go: import late/internal/pathutil and call pathutil.LateSkillsDir()
at the plugin-skills registration site. The common.LateSkillsDir export
was moved upstream.
- main.go: stop using tool.ScriptTool as the registration shape for
plugin-declared inline tools. Upstream repurposed that struct's fields
(SkillName/ScriptName/ScriptPath) for skill dispatch only, so plugin
tools no longer fit. New local pluginInlineTool type at the top of
cmd/late/main.go implements the common.Tool interface (Name,
Description, Parameters, Execute) and bridges to plugin.InlineTool by
synthesizing a client.ToolCall at Execute time from the registered
name + the executor's json.RawMessage args.
- main.go: registration call updated to pluginInlineTool{ Name, Description,
Parameters, Runner }.
- model.MessageHook = pluginManager.HookedMessage unchanged — signatures
already match.
Three changes to make Late's plugin system actually installable and
compatible with omp and Claude Code ecosystems:
1. Universal manifest loading (manifest.go)
- Recognize 'omp' field in package.json (Oh My Pi plugins)
- Recognize .claude-plugin/plugin.json + auto-detect skills/,
commands/, .mcp.json, hooks/hooks.json (Claude Code format)
- Both are translated into LateManifest at load time so all
surface registration works identically
2. Install fixes (installer.go, manager.go)
- Symlink discovery: os.ReadDir.IsDir() returns false for
symlinks, so linked plugins were invisible to the watcher
- Scoped npm packages (@scope/name): relative symlink target
was computed from the wrong parent directory
- @-prefixed scope dirs are now recursed into to find plugins
- Sandbox subprocesses via setCmdSysProcAttr (hooks.go)
3. Go 1.26 compat (sandbox_linux.go, sandbox_other.go)
- NoNewPrivs removed from SysProcAttr (not present in Go 1.26)
- CLONE_NEWPID retained for PID namespace isolation
Claude Code .mcp.json supports two shapes:
{'mcpServers': {'name': {...}}} — wrapped (omp/Late convention)
{'name': {'type':'sse','url':...}} — flat map (Claude Code convention)
The asana, github, linear, and other official plugins use the flat
format and were silently skipped. Now try both.
Verified with two real official Claude Code plugins:
- asana -> surfaces: 1 MCP server(s)
- discord -> surfaces: 1 skill(s), 1 MCP server(s)
Upstream changed AvailableCommands from []string to []CommandDef.
Our plugin branch added /clear, /themes commands and ThemeEntry struct.
Kept both: use CommandDef format with all commands from both sides.
Also fixed downstream code that still treated entries as strings:
- builtinSet: c → c.Name
- plugin command append: wrap string in CommandDef{}
giveen
marked this pull request as ready for review
July 27, 2026 03:50
Contributor
Author
|
@mlhher we had talked about a plugin system, so here you go, its compatible with OMP and Claude Code plugins plus gives us room to make our own plugins as well. |
Owner
|
Lol @giveen what a champ. This will take some time to go over everything and test everything so please bear with me. |
Contributor
Author
|
No worries, I was on vacation and this was my work every morning with my coffee at the beach. I've only done preliminary tests with it, but more in depth testing should be done. Im going to be installing and uninstall lots of plugins this week to see what screw ups I made. |
LateCommands.UnmarshalJSON now accepts mixed-shape arrays via []json.RawMessage element dispatch. resolveArgs tightened to ./ and ../ prefixes only. MCPServerConfig.Dir (json:"-") threads plugin.Path into the MCP stdio transport.
HandlePluginCommand short-circuits -h/--help/help at top level AND per subcommand via isHelpToken + hasHelpFlag. Fixes install --help falling through to npm install docs and link --help treating --help as a filesystem path.
RegisterPluginSkills now loads skills from either /skills/SKILL.md at root OR one /skills/<skillname>/SKILL.md per subdirectory, matching the Claude Code / Agent Skills convention.
setCmdSysProcAttr sets SysProcAttr=nil on Linux. Unprivileged PID namespaces trigger EPERM on many kernels and abort plugin execution; security trade-off documented in the function comment.
writeBarePlugin/mkPlugin now emit late:{} package.json so LoadPlugin accepts the fixture. parseProjectFlag flipped expectation verified. Cross-package ResolveRenderTheme tests dropped from hooks_themes_test.go. update_test.go rec/recorded() rename reconciled.
themes_test.go uses bubbletea v2 KeyPressMsg{Code, Text} shape. plugin_test.go rewritten for field-level access to Model.PluginCommands. New theme_test.go restores ResolveRenderTheme coverage previously in hooks_themes_test.go.
Two related uninstall-path fixes discovered during plugin testing: 1) removeFromDir now prunes the empty <pluginsdir>/@scope/ parent in addition to the existing <pluginsdir>/node_modules/@scope/ path, so link-installed scoped plugins no longer leave empty @scope orphan directories after `late plugin remove`. 2) handlePluginRemove now calls pm.Discover() and pm.RegisterPluginSkills("") after a successful RemovePlugin, self-cleaning stale ~/.config/late/skills/<plugin>:<skill> symlinks instead of waiting for the next watcher tick or TUI bootstrap. Both calls are best-effort warnings, not fatal, so a post-remove failure does not undo the on-disk removal. Tests (all in project_test.go): - TestRemovePlugin_ScopedLink_CleansEmptyScopeParent (bug verifier) - TestRemovePlugin_ScopedLink_KeepsNonEmptyScopeParent (non-empty preserve) - TestRemovePlugin_Project_ScopedLink_CleansEmptyScopeParent (project mirror) - TestHandlePluginRemove_PurgesStaleSkillSymlink (sandboxed via XDG_CONFIG_HOME) - TestHandlePluginRemove_PreservesSiblingSkillSymlink (over-pruning guard)
Contributor
Author
|
@mlhher okay I "think" thats everything. |
Repository owner
deleted a comment from
Sergio87Felix
Jul 30, 2026
Owner
|
@giveen Due to the size I will go at this in multiple passes. For now these are things that I did notice. Please take a look at them.
|
Names were only checked for emptiness before being joined into filesystem paths. A manifest with '..' components (e.g. ../../x) could make InstallFromLocal create symlinks outside the store and make RemovePlugin remove arbitrary directories. Validate names at every load point (native, omp, claude-code loaders and LoadPluginMeta), allowing plain and npm-scoped (@scope/pkg) names but rejecting empty, '.' or '..' path components and backslashes.
# Conflicts: # cmd/late/main.go # internal/tui/state.go # internal/tui/update.go
isPluginCmd compared the full input string against registered command
names, so '/lint file.go' never matched '/lint' and handlers received no
arguments. Match on the first whitespace-separated field instead, keeping
lookalike prefixes ('/lint2') unmatched.
pluginInlineTool.RequiresConfirmation always returned false, so the TUI confirmation middleware skipped the prompt and arbitrary plugin scripts ran without approval. The docs (plugin-sdk.md, plugin-example.md) promise plugin tools respect user confirmation, matching the precedent of skill scripts (tool.ScriptTool) and MCP tools. Return true so the normal confirmation flow applies.
HandleCommand held pm.mu.RLock and then called All(), which takes the same RLock again. When a writer queues between the two read locks, Go's RWMutex blocks the second RLock forever (readers yield to waiting writers), deadlocking the manager. Extract the sorted-copy body into allLocked() (caller must already hold the lock); All() and HandleCommand both use it, so HandleCommand takes exactly one RLock. Audit of every other lock-holder found no further nested acquisitions. Also fix writeExecutableShell to create the script parent dir — three tests (HandleCommand dispatcher/duplicate, GetInlineTools) failed with 'no such file or directory' because scripts/ never existed. Add TestHandleCommand_ConcurrentWithWriters: bounded stress test racing handler calls against writer traffic under a watchdog, so a regression fails the test instead of hanging the suite.
runHook rejected any stdin payload over 256 bytes, but tool arguments, tool results, full user messages, and inline-tool args routinely exceed that, so hooks and handlers silently failed on normal input. The cap is a sanity bound (the caller already holds the payload in memory and the 15s hook timeout bounds consumption), not a functional limit — bump to 16 MiB and rename hookCommandMax -> hookStdinMax to say what it bounds. Add TestRunHook_LargePayloadPassesThrough: a 64 KiB payload round-trips through a hook script intact (256x the old cap).
…strings
CallOnToolResultHooks wrapped the tool result in json.RawMessage, which
embeds bytes verbatim — json.Marshal failed on any non-JSON result
(plain command output, file contents), the error was discarded, and the
hook received empty stdin. The documented contract (plugin-sdk.md,
manifest.go) is {"tool": ..., "result": "..."} with result as a
string. Marshal as string(result), which always produces valid JSON with
proper escaping, and surface marshal errors instead of silently sending
nil payloads.
Add TestCallOnToolResultHooks_PlainTextResultPayload: a plain-text
result with quotes and backslashes round-trips through a capture hook as
valid JSON. The existing middleware test only used a JSON-shaped result,
which is why the bug went unnoticed.
These three hooks were defined and documented but never called by production code: onInput duplicated onMessageSend's sequential transform contract, and the turn hooks have no lifecycle moment wired to them. Plugins declaring them silently never fired. Remove the functions (CallOnInputHooks, CallOnTurnStartHooks, CallOnTurnEndHooks), the snapshotHooks cases, the manifest fields, and the documentation (plugin-sdk.md, plugin-example.md, quickstart). The manifest parser now only exposes hooks that actually run: onToolCall, onToolResult, onSessionStart, onMessageSend. Restoring the removed hooks later is a matter of re-adding the fields and wiring the calls.
TestInstallFromGit_CleansUpOnCloneFailure cloned a nonexistent GitHub URL, hitting live DNS/HTTPS and potentially prompting for credentials under make test. Replace it with a fake 'git' on PATH that creates the clone target directory and exits 1: the same partial-clone cleanup path runs, with zero network, and the fake actually exercises the stated contract (a half-populated dir must be removed) — the live-network failure never created the dir in the first place.
- resolveHookPath/resolveThemePath: reject absolute paths outright.
filepath.Join silently flattens a leading slash ('/etc/passwd' became
pluginDir/etc/passwd), so absolute manifest paths passed the
containment check. TestResolveHookPath_RejectsTraversal and
TestResolveThemePath_RejectsTraversal now pass; this was a real
contract gap, not a test bug.
- updateNpm: compute the relative symlink from linkDir's parent, not
targetDir. For scoped sources (@scope/pkg) the link is nested one
level deeper; the old rel produced a broken symlink and update
reload failed. Mirrors InstallFromNpm. Real production bug.
- Update local-source refusal now says 'local dev symlink' (test
contract expects the word 'local').
- InstallFromNpm routes through the runCommandOutput seam (like
updateNpm): npm output is captured and surfaced on failure instead of
streamed, and the exec is interceptable in tests.
- npm update test fixtures: scoped sources (@late/<name>) install under
node_modules/@late/<name>, matching real npm layout; the old fixtures
put them at node_modules/<name>.
- theme lookup test fixtures: theme JSON files must live inside the
plugin directory (p.Path), not its parent — the declared path
resolves against the plugin root.
- marketplace-fallback test: '@late/scoped-pkg' was mis-classified as a
direct npm package (contains '/'), so the marketplace branch never
ran. Use a bare name, which is what actually exercises the
404-fall-through path.
Full suite green: go test -race ./... (15 packages).
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.
Description of Changes
What
The Late plugin system now supports installing plugins from three ecosystems — not just plugins written specifically for Late:
package.json→"late"fieldpackage.json→"omp"field.claude-plugin/plugin.jsonskills/,commands/,.mcp.json(flat + wrapped),hooks/hooks.jsonWhy
Late should be able to install plugins from any ecosystem without requiring authors to publish specifically for Late. This makes the entire OMP and Claude Code plugin catalogs available to Late users.
How
internal/plugin/manifest.go— AddedOmpManifestandClaudePluginManifeststructs.LoadPluginnow tries three format loaders in order: native Late → OMP → Claude Code. Both OMP and Claude Code formats are translated intoLateManifestso all surface registration (skills, MCP, commands, hooks) works identically..mcp.jsondetection handles both{"mcpServers": {...}}(wrapped) and{"name": {...}}(flat Claude Code convention) formats.internal/plugin/installer.go— Fixed relative symlink path for scoped npm packages (@scope/name).internal/plugin/manager.go—discoverFromDirnow accepts symlinks (Goos.ReadDir().IsDir()returns false for symlinks) and recurses into@-prefixed scope directories.internal/plugin/hooks.go— Hook subprocesses get sandboxing viasetCmdSysProcAttr.internal/plugin/sandbox_linux.go— Go 1.26 compat:NoNewPrivsremoved fromsyscall.SysProcAttr.Verified With Real Plugins
OMP plugin from npm:
Claude Code official plugins:
All standard plugin lifecycle commands verified:
install,link,list,disable,enable,remove,update.Open Items
hooks/hooks.jsonauto-detection is stubbed but not yet wired into the hook execution pipeline (the hook event model differs between systems).registry.late.devis a placeholder that falls through to npm).CLA