Skip to content

refactor(backend): extract remote-server shared module from opencode + kilo - #172

Merged
claudiusthebot merged 8 commits into
mainfrom
feat/backend-abstraction
May 15, 2026
Merged

refactor(backend): extract remote-server shared module from opencode + kilo#172
claudiusthebot merged 8 commits into
mainfrom
feat/backend-abstraction

Conversation

@claudiusthebot

Copy link
Copy Markdown
Collaborator

Summary

The OpenCode and Kilo backend server.ts files were 90%+ byte-for-byte duplicates β€” same MCP registration logic, same chat-visibility rotation, same session-permission ruleset, same provider catalog walk. Every fix had to land in both copies; exactly the recipe for the drift this PR closes.

Extracts common machinery into src/backend/remote-server/:

  • client.ts β€” narrow RemoteAgentClient interface that both OpencodeClient and KiloClient structurally satisfy.
  • state.ts β€” per-backend mutable RemoteServerState<TClient> container (config, gateway port, frontend label, MCP cache, provider cache).
  • lifecycle.ts β€” ensureRemoteServer / stopRemoteServer (lazy spawn + reuse-existing-server via /global/health).
  • mcp.ts β€” chat & plugin MCP registration, tool overrides, disconnect. Owns the chat-visibility rotation (disconnect rival chat servers before registering the current one β€” upstream's permission rules only block execution, not visibility, so this dance is the only way to hide cross-chat tools from the model's catalog).
  • sessions.ts β€” ensureRemoteSession with Talon's standard permission ruleset (allow this chat, deny others, auto-allow built-ins).
  • providers.ts β€” resolveProviderID with injected backend hooks for bucket priority + provider guessing.

Both backend server modules become thin wrappers:

File Before After Delta
kilo/server.ts 745 357 βˆ’388
opencode/server.ts 475 224 βˆ’251
backend/remote-server/* (new) β€” 962 +962
remote-server-mcp.test.ts (new) β€” 372 +372

Net: 856 deleted, 1554 inserted; new code is mostly docstrings + the unit tests. Actual logic deduplicated: ~600 lines.

What's preserved

  • All existing exports on kilo/server.ts and opencode/server.ts β€” the handler / heartbeat / test code that already imports KILO_BASE_URL, parseStoredKiloModelSelection, getRegisteredMcpServerNames, errMsg, the OPENCODE_-prefixed deprecated aliases, etc. all continue to work.
  • All behaviour: chat-visibility rotation, heartbeat sentinel exemption, plugin server caching, idempotent mcp.add short-circuit, session expiry β†’ recreate flow, provider catalog scoring heuristic.
  • Existing kilo-server.test.ts and the integration/{kilo,opencode}-real-bootstrap.test.ts integration tests pass unchanged.

New tests

src/__tests__/remote-server-mcp.test.ts β€” 20 unit tests with a recording mock client, covering:

  • Chat MCP server name derivation (Telegram numeric, supergroup negatives, Discord snowflakes, empty string).
  • isTalonToolID matching for both _ and - variants.
  • Visibility rotation: switching chatA β†’ chatB disconnects chatA first.
  • Heartbeat sentinel (talon-tools-heartbeat) is exempt from rotation.
  • Plugin servers (mempalace, brave-search) stay connected across chat switches.
  • Local cache short-circuits redundant mcp.add calls.
  • ensureChatMcpServer returns the server name even when the upstream add throws.
  • buildToolOverrides produces the correct enable/disable map for multi-chat catalogs.
  • Returns undefined when no chat tools matched, only rival chats present, or tool.ids throws.
  • disconnectChatMcpServer removes the cache entry on success, doesn't throw on upstream failure.

Full suite: 2228 passing (was 2208), 12 skipped (live tier, workflow_dispatch-only), 0 failing.

Upstream dissection notes

For the design check, shallow-cloned sst/opencode and Kilo-Org/kilocode to ~/.talon/workspace/builds/upstream-dissect/:

  • Kilocode is a literal fork of opencode at packages/opencode/src level (diff -q confirms identical subdirectories). That's why their MCP wiring is identical β€” they're the same code with a few feature deltas.
  • Upstream's packages/opencode/src/session/llm.ts:439 (resolveTools()) confirms the model's visible tool catalog comes from Object.keys(input.tools) (global registered MCP tools) filtered by input.user.tools?.[k] !== false (per-prompt overrides). Permission rules are evaluated separately at execution time. This is why Talon's chat-visibility hack (disconnect rival chat MCP servers) is necessary β€” it's the only way to keep tools out of the model's catalog, not just out of its allowed-to-execute set. The new mcp.ts documents this explicitly.

Why now

The June 1 Claude Max cancellation gives ~17 days runway. The OpenCode and Kilo backends are Talon's main fallback path when Claude goes offline; they need to be solid and easy to evolve. Reducing the duplicated surface from two parallel ~500-line copies to one tested shared module makes the next round of work tractable without a fork tax.

Deferred to follow-up PRs

  • OpenCode β†’ SSE/promptAsync β€” currently sync prompt hangs when upstream stalls. Kilo's SSE pattern in events.ts is the model to follow.
  • Unified delivery routing β€” Kilo's 4-route delivery decision (tool / text-part / synthetic-error / empty) vs OpenCode's text-part-only vs Claude SDK's delivery-tool-delegation is its own design conversation.
  • Backend conformance test matrix β€” same scenarios run against all three backends through the stub-client integration tier.
  • Cross-backend system prompt β€” currently each backend has its own suffix.

Test plan

  • npm run lint β€” 0 errors, 15 pre-existing warnings (unchanged)
  • npx tsc --noEmit β€” clean
  • npm test β€” 2228 passing, 12 skipped (live-tier), 0 failing (was 2208 β€” +20 new unit tests)
  • npm run format:check β€” clean
  • Manual: run prod against this branch and verify chat switching still disconnects the rival chat MCP server (Pandario β†’ Dylan DM β†’ Pandario should show the disconnect / re-register log lines)
  • CI matrix on push

πŸ€– Generated with Claude Code

…+ kilo

The opencode and kilo backend `server.ts` files were 90%+ byte-for-byte
duplicates β€” same MCP registration logic, same chat-visibility rotation
dance, same session-permission ruleset, same provider catalog walk.
Every fix or behaviour tweak had to land in both copies, which is
exactly the recipe for the kind of drift Dylan flagged.

Extract the common machinery into `src/backend/remote-server/`:

  - `client.ts` β€” narrow `RemoteAgentClient` interface that both
    `OpencodeClient` and `KiloClient` structurally satisfy.
  - `state.ts` β€” per-backend mutable `RemoteServerState<TClient>`
    container (config, gateway port resolver, frontend label, MCP
    cache, provider cache).
  - `lifecycle.ts` β€” `ensureRemoteServer` / `stopRemoteServer` (lazy
    spawn + reuse-existing-server probe via `/global/health`).
  - `mcp.ts` β€” `ensureChatMcpServer`, `ensurePluginMcpServers`,
    `buildToolOverrides`, `disconnectChatMcpServer`, plus name
    derivation. Owns the chat-visibility rotation (disconnect rival
    chat servers before registering the current one).
  - `sessions.ts` β€” `ensureRemoteSession` with Talon's standard
    permission ruleset (allow this chat, deny others, auto-allow
    built-ins).
  - `providers.ts` β€” `resolveProviderID` with injected backend hooks
    for bucket priority and provider guessing.
  - `index.ts` β€” barrel re-export.

Both backend server modules now thin-wrapper-style:

  - kilo/server.ts: 745 β†’ 357 LOC
  - opencode/server.ts: 475 β†’ 224 LOC
  - net: -646 LOC duplicated, +962 LOC shared (mostly docstrings)

All existing public exports preserved. The legacy OPENCODE_-prefixed
aliases on the kilo module stay in place so bootstrap.ts and the older
tests don't break. The OpencodeClient and KiloClient types are kept on
the concrete backend modules β€” only the shared helpers operate against
the narrow structural interface.

20 new unit tests in `remote-server-mcp.test.ts` covering the chat
visibility rotation (heartbeat sentinel exempt, plugin servers not
rotated, cache short-circuit on redundant adds, build-overrides
enable/disable map shape, graceful upstream-failure handling). Full
suite: 2228 passing (was 2208), 12 skipped live-tier, 0 failing.

Why this matters: the June 1 Claude Max cancellation gives ~17 days
runway. The OpenCode and Kilo backends are Talon's main fallback path
when Claude goes offline; they need to be solid and easy to evolve.
Reducing the duplicated surface from two parallel ~500-line copies to
one tested shared module makes the next round of backend work (proper
SSE streaming on OpenCode, unified delivery routing across all three
backends, backend-conformance test suite) tractable without a fork tax.

What's deferred to follow-up PRs (intentional scope cap):

  - OpenCode β†’ SSE/promptAsync (currently sync `prompt` hangs when
    upstream stalls; Kilo's SSE pattern in `events.ts` is the model).
  - Unified delivery routing (Kilo has a 4-route delivery decision;
    OpenCode trusts text-part; Claude SDK delegates to delivery tools
    β€” that's its own design conversation).
  - Backend conformance test matrix running the same scenarios against
    all three backends through a stub-client integration tier.

Refs upstream dissection: `~/.talon/workspace/builds/upstream-dissect/`
holds shallow clones of opencode + kilocode upstream β€” kilocode is a
literal fork of opencode at `packages/opencode/src` level, which is
why their MCP wiring is identical. Upstream's `resolveTools()` in
`session/llm.ts:439` confirmed Talon's chat-visibility hack (disconnect
rival chat MCP servers) is the correct workaround for upstream's
"all registered tools visible to all sessions" model.
Rewrites the three files in `docker/kilo-test/` as straight reference
documentation. Drops the "this PR adds parity" framing that aged out
the moment PR #169 landed, removes the smoke-checklist that no longer
maps to current behaviour, trims narrative comments in the compose file
and Dockerfile down to what an operator actually needs at the keyboard.

Net: 281 β†’ 137 LOC across the three files. Same functionality, no
language that reads like a dev journal entry.
The README hadn't kept pace with what landed across PRs #96, #160, #161,
#165, #169, #170, and #172:

- Kilo and OpenCode backends were missing or misrepresented (the badge
  still said "Claude Agent SDK", the backend config row listed only
  claude/opencode, the architecture tree didn't mention kilo,
  remote-server, or shared).
- Discord frontend (PR #160) was absent from every list.
- Triggers (PR #96) were absent from the features table.
- Test count was stale at "1300+" β€” the suite is now 2200+ across the
  unit / SDK-stub / MCP-functional / integration tiers.
- Prerequisites assumed a single backend (Claude CLI on PATH).

Changes:

- New "Backends" section explaining the three options + their transport
  shape + shared remote-server infrastructure.
- Backends badge replaces the Claude Agent SDK badge.
- Features table: dedicated "Pluggable backend" row, new "Triggers"
  row, MCP tools row mentions triggers.
- Architecture tree refreshed: backend/registry.ts, backend/shared/,
  backend/remote-server/, kilo/, plus discord/ under frontend.
- Backend-specific prerequisites called out under Quick Start.
- Dependency rule paragraph mentions the QueryBackend interface.
- Config table: backend accepts claude/kilo/opencode, frontend accepts
  discord, model description is backend-agnostic.
- Development: test count updated to 2200+ across the tier matrix,
  added `npm run format`.
…cripts

Repository root had accumulated artefacts that don't belong on the
top-level surface. Tidied up:

  - Removed `sdk-test.ts` β€” a one-off probe for debugging a Claude
    Agent SDK hang earlier in development, kept in the tree without
    purpose since.
  - Moved `talon.service` β†’ `packaging/systemd/talon.service`. The
    systemd unit is a distribution artefact, not a development file;
    it lives with future packaging output (Debian/Homebrew formula,
    etc.) instead of cluttering the repo root.
  - Updated the systemd unit's install instructions to reflect the new
    path and to drop the stale "Claude-powered Telegram bot" framing
    (Talon is multi-frontend, multi-backend now).
  - Added `packaging/README.md` as the placeholder index for the new
    directory.
  - Refreshed `SECURITY.md`'s supported-versions table β€” said `1.0.x`
    when the current version is 1.11.x. Switched to a continuous-
    release statement pointing at CHANGELOG.md instead of carrying a
    fixed version row that ages every release.
  - Updated the README Production section to point at the new systemd
    unit location.

Repository root tree after this commit:

    bin/          Talon CLI entrypoint (talon.js)
    docker/       Auxiliary Docker harnesses (kilo-test, ...)
    packaging/    Distribution artefacts (systemd unit, future packages)
    prompts/      Markdown prompt templates loaded at runtime
    src/          Source tree (backend, core, frontend, plugins, storage, util, __tests__)
    Dockerfile, docker-compose.yml    Primary production image
    package.json, package-lock.json   Node/npm
    tsconfig.json, vitest.config.ts   TypeScript + test config
    release-please-*.json             Release tooling
    CHANGELOG.md, LICENSE, README.md, SECURITY.md
    Dotfiles (.gitignore, .dockerignore, .editorconfig)

No source-code changes. Tests / typecheck / lint unchanged: 2228
passing, 12 skipped, 0 failing.
Brings the OpenCode backend up to streaming parity with the Kilo backend.
Previously the OpenCode handler used the sync `oc.session.prompt()` call
which holds the HTTP connection open until the upstream model finishes β€”
when the upstream stalls (common with free providers and unreliable
network), the POST hangs and Talon's `await` blocks forever. With this
PR the OpenCode handler now mirrors Kilo's SSE-driven design:

  1. Subscribe to `oc.global.event()` BEFORE issuing the prompt so no
     early events are lost.
  2. Fire `oc.session.promptAsync()`. The HTTP POST returns immediately
     with a messageID; OpenCode runs the model task in the background.
  3. Await the SSE close event (`session.turn.close` / `session.idle` /
     `session.error`). The await is on event iteration we control, never
     on a long-running HTTP call we can't interrupt.
  4. Read `oc.session.messages()` for the authoritative final parts list.
  5. Drain it through the shared `finalizePartsIntoState`.

`session.abort()` now fires when a terminator tool (end_turn / send /
react) closes the turn, short-circuiting the model's wrap-up round-trip
the same way Kilo does. Saves ~2-3s of phantom typing per turn.

Refactoring:

- New `backend/remote-server/events.ts` β€” the SSE event processor that
  was previously in `kilo/events.ts`. The wire format is identical
  between OpenCode and Kilo (Kilo is a fork of OpenCode), so the
  per-event logic is shared. Backend-specific extraction
  (`extractPartsSummary`) is injected so each backend can supply its
  own synthetic-error detection.
- `kilo/events.ts` is now a thin shim that binds the Kilo-specific
  `extractPartsSummary` and labels logs `[Kilo]`.
- OpenCode handler imports the shared event processor directly and
  labels its logs `[OpenCode]`.
- `rejectPendingQuestions` exported from `opencode/sessions.ts` so the
  new question-rejection watchdog can use it.

Behaviour change for OpenCode:

- Per-turn `disconnectChatMcpServer` removed from the handler's
  `finally` block. The chat MCP server now stays connected across
  turns of the same chat (saves ~800ms per message). Chat switches
  still rotate via `ensureChatMcpServer`'s disconnect-rivals dance,
  preserving cross-chat visibility isolation.
- Integration test `per-turn MCP teardown leaves no chat servers
  connected after the turn` rewritten as `chat-switch disconnects the
  previous chat's MCP server`, mirroring the kilo isolation test.

Stats:
- `opencode/handler.ts`: 241 β†’ 552 LOC (added SSE driver, question
  watchdog, delivery routing, synthetic-error handling, abort flow)
- `kilo/events.ts`: 370 β†’ 56 LOC (now a configurator shim)
- New `backend/remote-server/events.ts`: 363 LOC of documented shared
  helpers.
- Net: ~+360 LOC in source; ~330 LOC of duplicated behaviour
  collapsed into one tested module.

Tests: 2228 passing, 12 skipped (live-tier), 0 failing β€” including
both `kilo-events.test.ts` (29) and the rewritten OpenCode isolation
test.
Both backends had ~90 lines of byte-for-byte identical end-of-turn
delivery routing logic (tool / synthetic-error / text-part / empty).
Extract into `backend/shared/delivery.ts` so the decision tree lives in
one place.

Routes:

  - `tool` β€” a delivery tool (`end_turn` / `send` / `react`) already
    bridged the message to the platform. Don't re-emit.
  - `synthetic-error` β€” upstream emitted a `synthetic: true` text part
    (e.g. "model hit output limit"). Surface as `⚠️ <Backend>: <msg>`
    instead of shipping verbatim.
  - `text-part` β€” model emitted plain assistant text. Ship via
    `onTextBlock`.
  - `empty` β€” no tool, no text, no synthetic-error. Surface a concise
    notice ("no reply β€” model returned no output" or "...called tools
    but didn't produce output text").

The shared router takes `backendLabel` so each backend keeps its own
prefix in the user-visible synthetic-error message and its own metric
namespace (`kilo.synthetic_error` vs `opencode.synthetic_error`).

Stats:
- `kilo/handler.ts`: -85 LOC (delivery block + unused
  `formatSyntheticPreview` removed)
- `opencode/handler.ts`: -80 LOC (same)
- New `backend/shared/delivery.ts`: 175 LOC of documented helper.
- New `shared-delivery.test.ts`: 12 unit tests covering each route +
  preference order + error-tolerance + decision-only mode.

Tests: 2240 passing, 12 skipped (live-tier).
…backend conformance suite

Two related changes:

1. **Remove deprecation aliases.** `kilo/server.ts` and `kilo/sessions.ts`
   carried `@deprecated` re-exports of `OPENCODE_HOSTNAME`, `OPENCODE_PORT`,
   `OPENCODE_BASE_URL`, `OPENCODE_SYSTEM_PROMPT_SUFFIX`, `initOpenCodeAgent`,
   `stopOpenCodeServer`, `parseStoredOpenCodeModelSelection`,
   `OPENCODE_SESSION_MESSAGE_LIMIT`, `OpenCodeAssistantInfo`,
   `OpenCodeSessionSnapshot`, `summarizeOpenCodeAssistantMessages`,
   `getOpenCodeTurnSummary`, `getOpenCodeSessionSnapshot`. All migrated;
   call sites updated:
   - `kilo-server.test.ts` β†’ uses `initKiloAgent`/`stopKiloServer`/
     `parseStoredKiloModelSelection`.
   - `kilo-summary.test.ts` β†’ uses `summarizeKiloAssistantMessages`.
   - `kilo-models.test.ts` and `kilo-model-provider.test.ts` mocks β†’
     `initKiloAgent`/`stopKiloServer`.
   - `kilo/index.ts` β†’ drops the alias re-exports.

   The internal `OpenCode*` type names in `kilo/models.ts` (catalog,
   provider entry, model resolution) remain β€” they describe the
   upstream wire shape (Kilo is a fork) and renaming them would obscure
   that, not clarify.

2. **Add `backend-conformance.test.ts`.** New file exercising the
   shared infrastructure with both backends side-by-side:

   - `processStreamEvent` produces identical state mutations regardless
     of `backendLabel` (Kilo vs OpenCode).
   - Out-of-session event filtering behaves identically.
   - `state.eventCounts` accumulates symmetrically.
   - `finalizePartsIntoState` extracts the same text + tools through
     either backend's `extractPartsSummary` callback.
   - Documents the one known asymmetry: kilo extractor recognises the
     `synthetic: true` flag; opencode extractor does not (yet). When
     OpenCode's upstream gains the synthetic marker, the kilo extractor
     change can be ported and the test will be updated.
   - `routeDelivery` returns the same route shape for equivalent state
     across backends, with the backend label correctly threaded through
     the synthetic-error message prefix.
   - MCP chat-server rotation behaves identically (chat-switch
     disconnects, heartbeat sentinel preserved) for both backend states.

   9 new tests, all green.

Tests: 2261 total (2249 pass + 12 live-tier skipped). +21 since the
phase started (12 shared-delivery + 9 conformance).
CI tsc (likely a stricter version than local) rejected
`createStreamState({ deliveredTextNorms: ["x"] } as never)` since
createStreamState takes no arguments. The Object.assign on the next
line was already what mutated the state β€” the argument was a no-op
leftover from earlier sketching.
@claudiusthebot
claudiusthebot merged commit 0c97e2f into main May 15, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant