[devswha]

The first feature slice out of server/index.ts and shared/protocol.ts: GET /api/integrations and its two types move into their own files. Behaviour, path, status codes and messages stay the same. The plan, with the ownership of every route and protocol type, the order of the next slices, and this slice's risks, is .omo/plans/feature-slices-plan.md in the maintainer's workspace.

This PR moves responsibility, so it is not move-only and takes the normal review path.

Why integrations first

It is the smallest family still written inline in index.ts (9 lines). It has two protocol types, no state, no dependency on the server core, and a live contract test that already checks its answer (server/api.contract.test.ts, "lists herdr's integrations as integration.list reports them, and refuses anything but GET"). The next candidates, in plan order, are the agent catalog, plugins, background tasks, plans and files.

Two commits: the check first, then the move

1. test(server): pin GET /api/integrations before it moves adds server/integrations-routes.test.ts. It runs in the unit lane without herdr: it starts a real createServer({ port: 0, hostname: "127.0.0.1", stateDir: mkdtempSync(…), … }) and stubs integrationList with spyOn(herdr, "integrationList"), the same seam server/http.test.ts uses for paneFind. It pins:

   • GET answers 200 with exactly { integrations: <what integration.list returned> }, including a state this build does not know;
   • a HerdrError("connect_failed") answers 502 with that code and message, and any other error answers 500 internal_error without its text;
   • a POST answers 400 method_not_allowed / use GET without asking herdr.

   On unchanged code it gives 3 pass, 0 fail.

2. refactor(server): move the integrations route and types into their own slice

   • Moved: the route branch (with its "read only" comment), unchanged, into handleIntegrationsRequest(request, pathname): Promise<Response | null> in the new server/integrations-routes.ts. This is the nullable route pattern of workspace-routes.ts and pane-routes.ts: no state, no dependency bag, and it imports integrationList and the HTTP helpers directly.
   • Moved: AgentIntegration and IntegrationsResponse, with their doc comment, verbatim into the new shared/integrations.ts. That 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 /api/agents and before /api/fs/*. Every access, origin, watch and timeout gate before it, the /api/machines/local alias allowlist and the machine proxy path list are untouched. The now-unused integrationList and IntegrationsResponse imports are dropped.
   • Stays in protocol.ts: export type { AgentIntegration, IntegrationsResponse } from "./integrations.ts"; at the old place, as protocol.ts already does for the machine types. All seven existing importers (server/herdr/client.ts, server/api.contract.test.ts, src/lib/api.ts, SettingsDialog.tsx, src/lib/integrations.ts and its test, site/demo/transport.ts) compile unchanged, so this PR does not touch those shared files.

Checks

• The pinning test, after the move: HERDR_TEST_MODE=unit bun test server/integrations-routes.test.ts server/http.test.ts src/lib/integrations.test.ts gives 41 pass, 0 fail; the pinning test itself 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 71 lines as moved: the handler body (one indent level shallower in its new file) and both interfaces. The 15 other lines are:
  • in index.ts: the new import, the shortened protocol type import, the dropped integrationList import, and the 2-line call;
  • in the new route file: its 2-line header, 3 imports, the function's doc comment and signature, and return null;
  • in protocol.ts: the type re-export.
• Client imports: bun run typecheck passes on both tsconfigs, so every importer still compiles through the re-export. No file in src/ or site/ imports shared/integrations.ts, so the client keeps its type-only rule. shared/integrations.ts has no import and no value export.
• Sizes: server/index.ts goes from 862 to 854 lines and shared/protocol.ts from 1,522 to 1,503. The new modules are 22 and 20 lines.
• Risks checked:
  • Import cycles: none new. The route imports only herdr/client.ts and http.ts, and neither reaches index.ts.
  • Initialization order: unchanged. The handler has no module state and constructs nothing, so server/startup-order.test.ts still applies as is.
  • Environment reads: the protocol value exports are untouched, so no environment read moves.
• bun run check fast: passes (unit 6131 pass, 0 fail, 12 skip in 435 files, 8 at a time, 57.7s).
• CI's integration lane runs the existing api.contract.test.ts case against an isolated live herdr.

Overlapping open PRs

Open PRs that touch these files: #1597 (/api/agents branch in index.ts, ending about 8 lines above the integrations branch), #1598 (adds a session-resume path to the lists that mention /api/integrations; it does not change them otherwise), and #1652, #1442 and #1427 (other parts of protocol.ts). None of them changes the moved lines, so any order works. Whichever merges second rebases; if #1597 merges first and its context overlaps, the rebase keeps both changes as they are.

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-integrations
• File: .coderabbit.yaml
• Commit: 25981dd
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 80b15528-1e30-4a46-973f-4f0e3cb13889

📥 Commits

Reviewing files that changed from the base of the PR and between 4f156f2 and 25981dd.

📒 Files selected for processing: 5

• server/index.ts
• server/integrations-routes.test.ts
• server/integrations-routes.ts
• shared/integrations.ts
• shared/protocol.ts

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

📝 Summary

Summary by CodeRabbit

• New Features
  • The integrations API now provides a consistent list of integrations, including their availability and status.
• Bug Fixes
  • Requests using unsupported methods receive a clear error response. Integration lookup failures return appropriate error responses, with unexpected errors kept generic.

The integrations endpoint now delegates to a dedicated route handler. Its response interfaces are defined in shared/integrations.ts and re-exported from shared/protocol.ts. Tests cover listing, error responses, and rejected methods.

Changes: Integrations API

Layer / File(s)	Summary
Integrations response contract shared/integrations.ts, shared/protocol.ts	Defines AgentIntegration and IntegrationsResponse in a shared module. The protocol module re-exports both interfaces.
Route handling and server wiring server/integrations-routes.ts, server/index.ts, server/integrations-routes.test.ts	The route handler serves GET requests and converts listing errors to error responses. The server delegates the endpoint to the handler. Tests cover successful listing, error responses, and POST rejection.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: haeminway1

Merge Risk: ⚪ Minimal · up to 25981

No actionable behavior change or merge-blocking risk is established; this is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
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: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: moving the integrations route and types into separate modules.
Description check		The description explains the change, resulting behavior, risks, affected files, tests, contract coverage, and changelog decision. It uses "## Checks" instead of the template's "## Validation" heading,…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the change, resulting behavior, risks, affected files, tests, contract coverage, and changelog decision. It uses "## Checks" instead of the template's "## Validation" heading, but it provides the required validation information and is otherwise complete.

✨ 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. This PR moves an HTTP route and protocol types, so besides CodeRabbit a code-review agent reviewed it: APPROVE, no findings to fix.

Behaviour is identical.

• With indentation ignored, the old branch (server/index.ts:543-552 on main) and handleIntegrationsRequest (server/integrations-routes.ts:11-20) are the same lines.
• shared/integrations.ts is a byte-for-byte copy of the two types and their comment.
• --color-moved marks 71 lines as moved.
• The call keeps its place: after /api/agents and every gate, before /api/fs/*.
• These lists are untouched: the /api/machines/local alias allowlist, the bridge path list and the machine proxy list.

The pinning test can fail. It passes 3/3 on commit 1 alone. Each of seven mutations fails it:

• a weaker method check;
• a renamed error code;
• a flattened error mapping;
• status 201;
• a filtered list;
• asking herdr before the method check;
• removing the call from index.ts.

No cycle, no order change, no contract change.

• Nothing the route file imports leads back to server/index.ts, and startup-order.test.ts passes.
• The protocol re-export is export type, so src/ and site/demo see no runtime or bundle change.
• JSON shapes and type names are unchanged, and bun run typecheck passes on both configs.

Tests: HERDR_TEST_MODE=unit bun test server/integrations-routes.test.ts server/http.test.ts src/lib/integrations.test.ts gives 41 pass. The new test follows AGENTS.md: temporary stateDir, port: 0, no real herdr, no sleeps.