Skip to content

fix(daemon): bind Omnigent sessions to a runner host_id - #20

Merged
jasonkneen merged 2 commits into
mainfrom
polly/daemon-host-binding
Jul 15, 2026
Merged

jasonkneen merged 2 commits into
mainfrom
polly/daemon-host-binding

Conversation

@jasonkneen

@jasonkneen jasonkneen commented Jun 15, 2026 •

Copy link
Copy Markdown
Owner

Problem

When the desktop/daemon routes an Omnigent chat turn, every turn fails with backend error runner_unavailable / "No runner bound for session".

Root cause: the daemon created the Omnigent session without a host_id. With no host_id, the Omnigent backend never binds/launches a runner for the session, so turns can't execute. The daemon create-session payload was { agent_id, title, workspace } — host_id appeared nowhere. The CLI provider already does this correctly; this PR mirrors that proven pattern into the daemon.

Fix

  1. Resolve a host_id before create (chat-jobs.mjs resolveOmnigentHostId): use settings.omnigent.hostId when configured; otherwise GET /v1/hosts from the same base URL and auto-pick the first host with status online (falling back to the first host if none report status).
  2. Include host_id in the POST /v1/sessions body alongside { agent_id, title, workspace }.
  3. Optional hostId setting added to the daemon omnigent settings resolver (omnigent-settings.mjs), mirroring agentId, with env override CODESURF_OMNIGENT_HOST_ID. Default blank → auto-pick.
  4. Fail closed on zero hosts: if /v1/hosts returns no hosts, throw a clear error explaining no runner host is registered — rather than silently creating a session that will fail with runner_unavailable.

Strictly daemon-side and minimal; no changes outside the create-session / host-resolution path + the settings schema.

Implementation notes

  • Pure, SDK-free helpers added to omnigent-provider.mjs (parseOmnigentHosts, chooseOmnigentHost, buildOmnigentSessionBody) so the create-session body is unit-testable without importing the Claude SDK that chat-jobs.mjs pulls in — mirrors the existing buildCodexExecArgs pattern in the same file.
  • host_id resolution happens only inside the if (!sessionId) branch (session creation). Resumed turns already have a runner bound.

Gates

  • npm test (packages/codesurf-daemon): 14/14 pass — includes new tests proving host_id is in the create-session payload and that an online host is auto-picked from /v1/hosts.
  • npm run typecheck (repo): 16 pre-existing errors on baseline origin/main, none in the files touched here. All changed files are .mjs (not TS-compiled), so this introduces zero new type errors.

Assumptions

  • Env var name CODESURF_OMNIGENT_HOST_ID mirrors the existing CODESURF_OMNIGENT_AGENT_ID.
  • Host-pick tie-break follows the CLI: first online host, else first host. Host id read from host_id (preferred) or id.

Summary by CodeRabbit

  • New Features

    • Added Omnigent hostId support via CODESURF_OMNIGENT_HOST_ID and settings (to control which host sessions are created on).
    • Session creation now uses the resolved host selection to bind work to the intended Omnigent host.
  • Bug Fixes

    • Improved fail-safe behavior when no usable Omnigent hosts are available, with clearer error reporting instead of proceeding with an invalid host selection.

The daemon's Omnigent create-session payload omitted host_id, so the
backend never bound/launched a runner for the session and every turn
failed with runner_unavailable ("No runner bound for session").

Mirror the CLI provider: resolve a host_id before POST /v1/sessions.
settings.omnigent.hostId wins; otherwise GET /v1/hosts and auto-pick the
first online host (first host if none report status). Fail closed with a
clear error when zero hosts are registered, instead of creating a session
guaranteed to fail at turn time.

- omnigent-provider.mjs: pure parseOmnigentHosts / chooseOmnigentHost /
  buildOmnigentSessionBody helpers (SDK-free, unit-testable)
- chat-jobs.mjs: resolveOmnigentHostId + include host_id in the body
- omnigent-settings.mjs: optional hostId field (+ CODESURF_OMNIGENT_HOST_ID)
- tests: host parse/pick + host_id-in-payload + settings coverage
Copilot AI review requested due to automatic review settings June 15, 2026 22:54
@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9444dff6-c87a-4bbe-95eb-d3e37e1e66b2

📥 Commits

Reviewing files that changed from the base of the PR and between 058dcd6 and c972b83.

📒 Files selected for processing (2)
  • packages/codesurf-daemon/bin/omnigent-provider.mjs
  • packages/codesurf-daemon/test/omnigent.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/codesurf-daemon/test/omnigent.test.mjs
  • packages/codesurf-daemon/bin/omnigent-provider.mjs

📝 Walkthrough

Walkthrough

The PR adds three pure helper exports to omnigent-provider.mjs (parseOmnigentHosts, chooseOmnigentHost, buildOmnigentSessionBody), extends resolveOmnigentSettings to surface a hostId field, and wires a new resolveOmnigentHostId function into runOmnigentJob so a runner host is explicitly resolved from /v1/hosts before each session POST. Tests are expanded to cover all new helpers and the updated settings resolution.

Changes

Omnigent Host Resolution

Layer / File(s) Summary
Provider wire helpers: parse, choose, build
packages/codesurf-daemon/bin/omnigent-provider.mjs
Adds parseOmnigentHosts (normalizes array, { hosts }, and { data } backend shapes into { id, name, status } rows), chooseOmnigentHost (selects first online host or first host), and buildOmnigentSessionBody (constructs /v1/sessions POST body with optional host_id and workspace).
hostId propagation in resolveOmnigentSettings
packages/codesurf-daemon/bin/omnigent-settings.mjs
resolveOmnigentSettings now reads CODESURF_OMNIGENT_HOST_ID (or cfg.hostId) and returns hostId alongside the existing settings fields; inline example comment updated to document the new field.
Host resolution wired into runOmnigentJob
packages/codesurf-daemon/bin/chat-jobs.mjs
Imports parseOmnigentHosts; adds resolveOmnigentHostId which calls GET /v1/hosts, parses the response, selects a host, and throws when none are available; runOmnigentJob now calls resolveOmnigentHostId then buildOmnigentSessionBody instead of building the session body inline.
Tests for new helpers and updated settings
packages/codesurf-daemon/test/omnigent.test.mjs
Extends resolveOmnigentSettings assertions to cover hostId and boolean env parsing; adds tests for parseOmnigentHosts (multiple shapes, id preference, filtering), chooseOmnigentHost (online preference, fallback, null on empty), and buildOmnigentSessionBody (host_id inclusion/omission).

Sequence Diagram(s)

sequenceDiagram
  participant runOmnigentJob
  participant resolveOmnigentHostId
  participant OmnigentAPI
  participant buildOmnigentSessionBody

  runOmnigentJob->>resolveOmnigentHostId: settings, baseUrl, apiKey, signal
  resolveOmnigentHostId->>OmnigentAPI: GET /v1/hosts
  OmnigentAPI-->>resolveOmnigentHostId: hosts payload
  resolveOmnigentHostId->>resolveOmnigentHostId: parseOmnigentHosts → chooseOmnigentHost
  resolveOmnigentHostId-->>runOmnigentJob: hostId (throws if none)
  runOmnigentJob->>buildOmnigentSessionBody: agentId, hostId, title, workspace
  buildOmnigentSessionBody-->>runOmnigentJob: POST body with host_id
  runOmnigentJob->>OmnigentAPI: POST /v1/sessions
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • jasonkneen/codesurf#18: Directly touches the same chat-jobs.mjs and omnigent-provider.mjs session-creation flow that this PR extends with host resolution.

Poem

🐇 Hop, hop, which host shall we choose?
The first one online — no time to snooze!
We parse the payload, we pick the id,
We build the body, then off we bid.
No more inline guessing, we fail closed and bright —
A rabbit who resolves things always gets it right! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: binding Omnigent sessions to a runner host_id, which is the core fix addressing the missing host_id parameter in session creation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch polly/daemon-host-binding

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Omnigent session creation in the daemon by ensuring sessions are bound to a runner host_id, preventing runner_unavailable errors (“No runner bound for session”) on every routed turn.

Changes:

  • Add runner host resolution (settings.omnigent.hostId override, otherwise auto-pick from GET /v1/hosts, fail closed on zero hosts).
  • Include host_id in the daemon’s POST /v1/sessions payload via a pure helper (buildOmnigentSessionBody).
  • Extend daemon Omnigent settings resolution + tests to cover hostId and host parsing/selection logic.

Reviewed changes

Copilot reviewed 1 out of 4 changed files in this pull request and generated no comments.

File Description
packages/codesurf-daemon/test/omnigent.test.mjs Adds unit tests covering host parsing/selection and asserting host_id inclusion in the create-session body.
packages/codesurf-daemon/bin/omnigent-settings.mjs Adds optional hostId setting resolution with env override CODESURF_OMNIGENT_HOST_ID.
packages/codesurf-daemon/bin/omnigent-provider.mjs Introduces pure helpers parseOmnigentHosts, chooseOmnigentHost, and buildOmnigentSessionBody for testable session/host logic.
packages/codesurf-daemon/bin/chat-jobs.mjs Resolves host_id during session creation and uses the new helper to build the create-session request payload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Backends/CLIs report ONLINE/Online/online; chooseOmnigentHost compared
status === 'online' case-sensitively, so an uppercase ONLINE host was
missed and a stale first (offline) host was wrongly picked. Normalize to
lowercase before comparing. (cross-review blocking fix on PR #20)
@jasonkneen
jasonkneen merged commit ef9046e into main Jul 15, 2026
1 of 2 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.

2 participants