ACP agent, faster delegation, and gateway port recovery - #3
Merged
Merged
Conversation
Highlighting agent output now copies it straight to the clipboard the way a terminal does, and a floating pill (or ⌘L / Ctrl+L) quotes it into the composer as visible "> " lines. Copying goes through the main process: the renderer's async clipboard API needs focus plus a permission grant, and neither holds while a drag-select is still in flight. Scoped to the transcript — selecting in the composer, a project field, or a modal means editing, not quoting. Toggle with /copy on|off, persisted in prefs.json. A textarea can't style its own contents, so the composer paints transparent text over a <pre> mirror that re-renders the same string with quoted lines dimmed and paste tokens accented. Every path that changes input.value goes through grow(), so the mirror re-renders in one place instead of a dozen call sites. Two traps worth knowing: <pre> parsing swallows a leading newline, so every line is wrapped in a span or a blank first line shifts the mirror off by one; and --sel has to stay translucent, or the textarea's selection would paint over the text it is meant to tint. Pasting or quoting more than 6 lines / 800 chars collapses to "[Pasted text #1 +322 lines]". The body is held aside and spliced back in on the way to the model. The token is one unit, not 27 characters: a single Backspace or Delete touching it removes the whole block along with the trailing space, so the keypress right after pasting undoes the paste. Editing a token apart drops its body — what gets sent is always what you can see. After submitting, the block opens under the prompt line, capped and scrollable like a tool card, with a click to collapse it again; the bodies ride on the transcript event, so they survive a restart. Two fixes fell out along the way: the pill dismissed itself whenever the transcript auto-scrolled (it follows the selection now), and a message carrying only attachments and no typed text refused to send. Verified by test/selection.js, which drives the real renderer through OMNIWORK_UI_TEST — 27 assertions covering the pill, quote insertion, both collapse paths, atomic delete, and suppression inside editable fields — then reads the system clipboard to prove copy-on-select actually wrote to it. Mirror alignment is asserted as zero scrollHeight drift across six layout traps (leading/trailing blank line, unbreakable 400-char run, heavy wrapping, quotes, tokens). Live end-to-end: quoted 322 lines with a probe buried at line 200, asked what line 200 was, got it back; restarted the app and the block came back from disk with all 322 lines intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test killed Electron with SIGKILL, which cannot be trapped — so the shutdown path in main.js never ran, and the gateway (spawned detached, in its own process group) survived as an orphan reparented to init. It kept holding port 20128 while spinning at 100% CPU without serving requests, so the next app launch adopted a sidecar that never answers and sat on "engine error" indefinitely. main.js already handles SIGTERM/SIGINT/SIGHUP for exactly this reason; the test was the one path bypassing it. Shutdown is now SIGTERM with an 8s SIGKILL fallback, and the run asserts nothing is still listening on its port afterwards, so a regression fails the test instead of quietly leaking a process. The run also shared the real profile: it drove the app against the default port and userData dir, so it fought with any copy of OmniWork the developer had open and rewrote their prefs.json via /copy off|on. It now gets its own --user-data-dir and OMNIWORK_GATEWAY_PORT. Worth knowing: #watchdog only arms after the gateway reaches ready, so a boot that fails outright is not retried — "engine error" needs a relaunch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Delegating to OmniWork from Claude Code took far longer than the work justified. Three causes, none of them the model. The agent tried a streaming request first and fell back to a non-streaming one whenever the stream came back empty — which free `auto` routing does often, since it lands on whichever provider is available. Nobody reads the token stream during delegation, so that fallback bought nothing and cost a second full request on every affected step, up to 40 steps per task. Headless callers now make exactly one request per step, and parallel subagents never stream either: only their tool labels and final summary are ever read. The gateway also booted lazily on the *first* delegate call, so the caller paid the entire OmniRoute cold start — tens of seconds, or an engine download on lite builds — before any work began. It now starts in the background at `initialize`, overlapping with the host reading the tool list and deciding what to do. Finally, delegation reported nothing until it was completely finished, so a slow call and a hung one looked identical from the outside. Delegate calls now emit `notifications/progress` per step and per finished subagent, and a stalled turn is cut off at OMNIWORK_DELEGATE_TIMEOUT_MS (10 min) and returns its partial work rather than hanging until the client gives up. Gateway, skills, memory, and project wiring move to electron/headless.js so the ACP server can share them, and both headless servers now honour OMNIWORK_BASE_URL / OMNIWORK_API_KEY to run against an OpenAI-compatible endpoint other than the bundled gateway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The MCP server makes OmniWork a *tool* your agent calls. This makes it an
*agent* another harness runs: OpenClaw, acpx, Zed, Neovim — anything that
speaks the Agent Client Protocol. The harness owns the UI, the approval
prompts, and the transcript; OmniWork does the work on free models.
~/.acpx/config.json
{ "agents": { "omniwork": { "argv": ["npx", "-y", "omniwork-acp"] } } }
The agent loop already emitted everything the protocol wants, so most of
this is translation. Text and reasoning stream as message chunks; every tool
call reports pending -> in_progress -> completed with its ACP kind and
source locations; write_file and edit_file arrive as native diffs rather
than a byte count, computed before the write lands because that is the only
moment the old content still exists. Gated calls become
session/request_permission, where "allow always" flips the session to auto
mode instead of silently prompting again on the next call. The Agent Deck
surfaces as N live parallel tool calls rather than one opaque block, and
installed skills are published as slash commands.
Approval modes (ask, edits, auto, plan) are exposed as ACP session modes.
`ask` is the default: the whole point of the protocol is that the client
owns the permission boundary, so it should get the chance to prompt rather
than have us run commands unattended in someone's editor.
OMNIWORK_ACP_MODE overrides it.
Hand-rolled JSON-RPC rather than the official SDK, which is ESM-only while
this app is CommonJS — and the stdio loop was already written for the MCP
server. Unlike that one this connection is bidirectional, since the agent
sends requests to the client, so outbound ids get a pending map.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
If a previous run left an OmniRoute process bound to port 20128 but no longer answering, every subsequent boot failed — slowly, then reported only "Gateway exited (code 1)". Delegation on top of that looks identical to delegation being merely slow, which is how it went unnoticed. Two bugs. The health-check loop called fetch with no per-request timeout, so a process that accepts the connection and never replies parked each poll for undici's 300s default; the loop's own 90s deadline is only evaluated between iterations, so it never fired. And a child that died instantly on EADDRINUSE was indistinguishable from one still starting up, so the loop kept politely asking a port owned by somebody else how it was getting on. Health checks are now bounded per request, a dead child ends the wait immediately instead of burning the full budget, and EADDRINUSE is handled as its own case: re-probe patiently first, since the holder is often a healthy gateway that was simply slower than the 2s startup probe, and adopt it if so. If it really is wedged, start on an OS-assigned free port instead. The port therefore becomes per-instance; everything already read baseUrl and dashboardUrl off the Gateway object, so only discovery still assumes the well-known port, which is correct. It deliberately does not kill the holder. That process may not belong to OmniWork at all, and taking a port by force isn't a decision to make on the user's behalf. test/sidecar.js covers the probe directly — closed, wedged, healthy, 401, and 500 — without booting OmniRoute, so the regression stays cheap to check. Verified end to end against the real failure: a wedged listener pinned to 20128 is now detected immediately, re-probed, and recovered onto a fallback port, healthy in 30s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README gets a section on driving OmniWork from OpenClaw, acpx, or Zed, with the config for both, a table of what the harness actually gets, and the new environment variables. CHANGELOG records the ACP server, the delegation speedups, and the gateway port recovery. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three conflicts. One was textual, two were not. electron/sidecar.js was the real one. 0.11.1 stopped piping the gateway's stdout/stderr to us — deliberately, because the engine is built to outlive the process that started it, and a detached child writing to a pipe whose reader has exited takes EPIPE and dies. My EADDRINUSE recovery detected the busy port by scanning that exact output, which no longer exists. Resolved by taking main's file wholesale — its _stopped shutdown latch, #killProc split, and /dev/null stdio all preserved — and re-implementing the detection on top: ask the OS whether we can bind the port before spawning, rather than reading why the child died. That is strictly better anyway. It is deterministic instead of string-matching, it runs before we spawn a child that was always going to die, and it composes with main's retry loop without touching it, so the DB-quarantine path and every _stopped guard stay exactly as main wrote them. Re-verified end to end against a wedged listener pinned to 20128: detected, re-probed, recovered onto a fallback port, healthy in 25s. CHANGELOG.md: main released 0.9.1 through 0.11.1 while this branch was open, so the Unreleased section is rebuilt on main's structure with only the four entries that are genuinely still unreleased. `npm run connect` and `npm run setup` were dropped from it — main shipped both in 0.10.0. This also repairs a heading that an earlier edit on this branch had swallowed, which had left 0.9.1's entire body orphaned under Unreleased. README.md: keep main's feature table and add the three new rows to it. The branch's "Web-aware" row is dropped — main replaced it with "Browsing built in", which describes the same capability more accurately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Six commits, four of them new work. The two carried over — copy-on-select and the selection test fix — were sitting on a branch whose PR had already merged, so they never made it into one.
Speak ACP
The MCP server makes OmniWork a tool your agent calls. This makes it an agent another harness runs — OpenClaw, acpx, Zed, Neovim. The harness owns the UI, approval prompts, and transcript; OmniWork does the work on free models.
The agent loop already emitted everything the protocol wants, so most of this is translation. Tool calls carry their ACP kind and source locations,
write_file/edit_filearrive as native diffs rather than a byte count, gated calls becomesession/request_permission(where "allow always" flips the session to auto mode instead of prompting again), the Agent Deck surfaces as N live parallel tool calls, and installed skills publish as slash commands.Modes default to
ask. The point of ACP is that the client owns the permission boundary, so it should get the chance to prompt rather than have us run commands unattended in someone's editor.Hand-rolled JSON-RPC rather than the official SDK, which is ESM-only while this app is CommonJS.
Delegation was paying for work nobody was watching
Delegating from Claude Code took far longer than the work justified. Three causes, none of them the model:
autorouting does often. Nobody reads the token stream during delegation, so that cost a second full request per step, up to 40 steps per task. Headless callers now make one request per step; subagents never stream.initialize.notifications/progressper step, and a stalled turn is cut off and returns partial work.A wedged gateway no longer takes the app down
Found while testing the above. If a previous run left OmniRoute bound to 20128 but not answering, every later boot failed — slowly, reporting only
Gateway exited (code 1).Two bugs: the health loop called
fetchwith no per-request timeout, so a process that accepts and never replies parked each poll for undici's 300s default and the loop's own 90s deadline never fired; and a child dying instantly onEADDRINUSElooked the same as one still booting.Health checks are now bounded, a dead child ends the wait immediately, and
EADDRINUSEgets its own path: re-probe patiently (the holder is often a healthy gateway that was just slower than the 2s startup probe) and adopt it, or start on a free port if it's genuinely wedged. It deliberately does not kill the holder — that process may not be OmniWork's.Verification
Driven end to end by the real
acpxclient: handshake,session/new, permission request, native diff render, streaming,end_turn— and the file it was asked to write contained what it should.The port fix was reproduced against the actual failure: a wedged listener pinned to 20128, detected immediately, recovered onto a fallback port, healthy in 30s.
test/acp.js(19 checks) andtest/sidecar.js(9 checks) are new and fast — the sidecar one covers closed/wedged/healthy/401/500 without booting OmniRoute.sidecar,acp,mcp,projects,compact,skills,modes,approvals,selection,schedulerall pass.Not re-run:
smoke,features,persist. They boot a real gateway and take minutes. They failed mid-session only because a wedged gateway was holding 20128 — exactly the case this PR fixes — but I have not confirmed them green, so I'm not claiming it.🤖 Generated with Claude Code