[devswha]

The next feature slice out of server/index.ts and shared/protocol.ts, after the integrations pilot (#1686): the plugin action routes and their types move into their own files. Behaviour, paths, status codes and messages stay the same. The plan, with the order and the risks, is .omo/plans/feature-slices-plan.md in the maintainer's workspace.

The plan lists the agent catalog before plugins. It comes after this one because the open fix #1597 changes the same /api/agents branch, and the refactor brief says to fix first, then move.

This PR moves responsibility, so it is not move-only.

Two commits: the check first, then the move

1. test(server): pin the plugin routes before they move adds server/plugin-routes.test.ts. The existing route coverage is server/plugins.contract.test.ts, which needs a live isolated herdr and a disposable plugin registry, and route-envelope.contract.test.ts for the gates. plugin-actions.test.ts covers the helpers but not the routes. So the new test runs the routes in the unit lane without herdr: a real createServer({ port: 0, hostname: "127.0.0.1", stateDir: mkdtempSync(…), … }), with pluginList, pluginActionList, pluginLogList, pluginActionInvoke and, in one case, sessionSnapshot stubbed through spyOn(herdr, …), as server/http.test.ts does for paneFind. It pins:

   • GET /api/plugins/actions:
     • each plugin with only this platform's actions (an action for another platform is dropped; one with no platforms is kept);
     • description null when missing, contexts [] when missing, and no command line in the answer;
     • a POST is 400 method_not_allowed.
   • GET /api/plugin/action:
     • a missing plugin_id or log_id is a 400;
     • a known log answers its result (failed run: exit code and its own words);
     • the log list is read with herdr's limit of 200;
     • an unknown log is 404 plugin_log_not_found;
     • herdr's own error passes through (502).
   • POST /api/plugin/action, validation before herdr is asked:
     • invalid_json, missing_plugin_id, missing_action_id and invalid_pane_id are 400s;
     • a PUT is 400 method_not_allowed;
     • in all of these, nothing is invoked.
   • POST /api/plugin/action, runs:
     • without a pane, herdr is invoked with no context;
     • with a pane, herdr gets exactly pluginPaneContext(snapshot, pane);
     • with a pane that is not there, the answer is 404 pane_not_found and nothing is invoked.

   On unchanged code it gives 4 pass, 0 fail.

2. refactor(server): move the plugin routes and types into their own slice

   • Moved into the new server/plugin-routes.ts:
     • the three branches, unchanged, as handlePluginRequest(request, url, pathname, options): Promise<Response | null>, the nullable route pattern of workspace-routes.ts, pane-routes.ts and integrations-routes.ts;
     • the three things only those branches used: PLUGIN_PLATFORM, PLUGIN_ACTION_WAIT_MS and livePaneIds.
   • Moved into the new shared/plugins.ts: the six plugin action types (PluginActionContext, PluginAction, PluginActions, PluginActionsResponse, PluginActionRequest, PluginActionResult), verbatim with their doc comments. The file holds types only, with no imports and no values.
   • Stays in index.ts: one call at the same place in the dispatch order, after the workspace routes and before the pane routes. createServer still passes its pluginActionWaitMs option through options. The access, origin, watch and timeout gates, the /api/machines/local alias allowlist and the machine proxy path list are untouched. The now-unused imports HerdrError, readJsonObject, the four plugin RPCs, PluginInvocationContext, PluginActionsResponse and the plugin-actions.ts helpers leave index.ts.
   • Stays in protocol.ts: a type re-export of the six names, so every importer compiles unchanged: server/plugin-actions.ts, server/plugins.contract.test.ts, src/lib/api.ts, machineContext.tsx, pluginRun.ts, paletteSearch.ts, CommandPalette.tsx, SettingsDialog.tsx and site/demo/transport.ts. This PR does not touch them.

Checks

• After the move: HERDR_TEST_MODE=unit bun test server/plugin-routes.test.ts server/plugin-actions.test.ts server/integrations-routes.test.ts src/lib/pluginRun.test.ts src/lib/paletteSearch.test.ts gives 46 pass, 0 fail. The pinning test is unchanged between the two commits.
• --color-moved on the move commit: git diff --color-moved=plain --color-moved-ws=allow-indentation-change HEAD~1 HEAD -- server shared shows 265 lines as moved: the three branches (one indent level shallower in their new file), the three constants, both import lines that moved with them, and the six types. The 21 other lines are:
  • in index.ts:
    • the shortened protocol import;
    • the http.ts import without readJsonObject;
    • the six dropped herdr/client.ts names;
    • the new plugin-routes.ts import;
    • the 2-line call.
  • in the new route file:
    • its 3-line header;
    • the types import and the herdr/client.ts import;
    • the function's doc comment and signature;
    • return null.
  • in protocol.ts: the re-export line.
• Client imports: bun run typecheck passes on both tsconfigs. No file in src/ or site/ imports shared/plugins.ts. shared/plugins.ts has no import and no value export.
• Sizes: server/index.ts goes from 854 to 778 lines and shared/protocol.ts from 1,503 to 1,452. The new modules are 87 and 52 lines.
• Risks checked:
  • Import cycles: none new. The route imports herdr/client.ts, http.ts and plugin-actions.ts, none of which reaches index.ts.
  • Initialization order: unchanged. The moved constants are pure, and PLUGIN_PLATFORM reads process.platform as before. The handler constructs nothing.
• bun run check fast: passes (unit 6135 pass, 0 fail, 12 skip in 436 files, 8 at a time, 57.7s).
• CI's integration lane runs server/plugins.contract.test.ts and route-envelope.contract.test.ts against an isolated live herdr.

Overlapping open PRs

None of the open PRs that touch server/index.ts or shared/protocol.ts (#1597, #1598, #1652, #1442, #1427) changes the moved lines or the plugin types, so any merge order works.

No changelog fragment: users see no change.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: refactor/slice-plugins
• File: .coderabbit.yaml
• Commit: a02cc2e
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7756c4b0-3af4-4f01-ba09-69bdc5376aa0

📥 Commits

Reviewing files that changed from the base of the PR and between 410b51a and a02cc2e.

📒 Files selected for processing: 5

• server/index.ts
• server/plugin-routes.test.ts
• server/plugin-routes.ts
• shared/plugins.ts
• shared/protocol.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

• New Features
  • Added plugin action listings, with actions filtered for the server’s platform.
  • Added the ability to check an action’s status and view its output.
  • Added support for running plugin actions, optionally targeting a pane, and receiving the result.
  • Added validation for action requests and pane selection.

Plugin route handling moves from server/index.ts to server/plugin-routes.ts. Shared plugin action types move to shared/plugins.ts and remain re-exported by shared/protocol.ts. New tests cover route behavior and validation.

Changes: Plugin routes

Layer / File(s)	Summary
Shared plugin action types shared/plugins.ts, shared/protocol.ts	Plugin action context, metadata, request, response, and result types are defined in shared/plugins.ts. shared/protocol.ts re-exports them.
Route handling and server delegation server/plugin-routes.ts, server/index.ts	The handler serves action listings filtered by platform, action log lookups, and action invocations with optional pane context. The server delegates requests to the handler.
Route behavior tests server/plugin-routes.test.ts	Tests cover platform filtering, log lookup results and errors, request validation, invocation with and without pane context, and missing panes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: haeminway1

Merge Risk: ⚪ Minimal · up to a02cc

The plugin routes appear to retain their existing behavior. No issue identified here needs resolution before merge.

Pre-merge checks: 2 · 3

Failed checks: 3 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #1597 remains unmet. The whole-PR diff changes plugin routing and plugin types only. server/shell-agent.ts still implements shellAgentExecutable with `Bun.which(kind, { PATH: process.env["PA…	Implement #1597 before keeping it directly linked. Add the shared ShellAgentLocator with the required bounded, cached Unix login-shell probe, refresh and shutdown handling, and use it for both /api/agents and shell-agent starts in `serv…
Out of Scope Changes check		The PR changes are outside issue #1597's scope. The added server/plugin-routes.test.ts, server/plugin-routes.ts, shared/plugins.ts, and the related removals and re-export in server/index.ts an…	If this PR is intended to implement #1597, remove the unrelated plugin route and plugin type changes and submit them separately. If the plugin refactor is the intended work, remove the direct link to #1597 or link the correct plugin-refacto…
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 2 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: moving plugin routes and types into separate files.
Description check		The description explains the change, states that behavior remains unchanged, and provides detailed validation results, including HTTP route and contract-test coverage. It uses a "Checks" heading inste…

Full details: Description check

Explanation

The description explains the change, states that behavior remains unchanged, and provides detailed validation results, including HTTP route and contract-test coverage. It uses a "Checks" heading instead of the template's "Validation" heading, but the required information is present.

Full details: Linked Issues check

Explanation

Issue #1597 remains unmet. The whole-PR diff changes plugin routing and plugin types only. server/shell-agent.ts still implements shellAgentExecutable with Bun.which(kind, { PATH: process.env["PATH"] ?? "" }); it has no bounded cached login-shell probe or shared ShellAgentLocator. server/remote-entry.ts still only adjusts PATH and calls createServer; it does not pass or use a locator. The required server and SSH bridge behavior, refresh and shutdown lifecycle, probe limits, and shared offer/start lookup are therefore not implemented by the reviewed head.

Resolution

Implement #1597 before keeping it directly linked. Add the shared ShellAgentLocator with the required bounded, cached Unix login-shell probe, refresh and shutdown handling, and use it for both /api/agents and shell-agent starts in server/index.ts and server/remote-entry.ts. Add the issue's automated coverage for offer/start agreement, limits, concurrency, Windows behavior, and shutdown.

Full details: Out of Scope Changes check

Explanation

The PR changes are outside issue #1597's scope. The added server/plugin-routes.test.ts, server/plugin-routes.ts, shared/plugins.ts, and the related removals and re-export in server/index.ts and shared/protocol.ts implement a plugin route and type refactor. They do not support the agent PATH discovery, shell-agent lifecycle, or remote-entry requirements in #1597.

Resolution

If this PR is intended to implement #1597, remove the unrelated plugin route and plugin type changes and submit them separately. If the plugin refactor is the intended work, remove the direct link to #1597 or link the correct plugin-refactor issue.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Merge-pass review. CodeRabbit had no actionable comments; a code-review agent also reviewed it: APPROVE, nothing to fix.

• Behaviour is identical.
  • The handler matches the old inline block (server/index.ts:590-656 on main) with whitespace ignored.
  • The six types match the old protocol.ts block.
  • The move commit's --color-moved diff shows only moved lines, apart from imports, the call, the new file's header and signature, and the export type line.
• No route or gate changed.
  • The call keeps its place: after the workspace routes, before the pane routes.
  • Every gate runs before it: the bridge path list, observe-only refusal, the 60 s timeout, the machine proxy and the /api/machines/local list. None of them is in the diff.
  • Method checks, validation codes, status codes and JSON shapes are unchanged.
• No state moved and no import cycle. startup-order.test.ts passes.
• The pinning test catches most changes. Of 12 mutations, 9 fail the test.
  • Ignoring the wait option fails the real-herdr plugins.contract.test.ts.
  • The two left are harmless: the call order between two handlers of disjoint paths, and the wait default.
• The re-export is type-only.
• Checks: bun run typecheck passes. The unit tests give 67 pass. plugins.contract.test.ts gives 12 pass on a disposable herdr session. bun run check fast passes.

Note for the next slice: the comment at server/index.ts:189 still names PLUGIN_ACTION_WAIT_MS, which now lives in server/plugin-routes.ts.