Repository navigation
Move the turn engine into @openbot/toolkit, and split the largest server files - #784
Conversation
…interrupt's metadata
…used before it starts
… run-turn stops importing copilot.ts
…rc/toolkit, apart from the computer tools
…src/toolkit, apart from the plugin store
…ead of reading the lifecycle and controls globals
…ead of the audit store
…data key into src/toolkit
…nentCatalog port, and keep AuditSink on the server
…story helpers into src/toolkit
…de server/src/toolkit
…e(), with the lock held by a lease
…lse's, not on any failed renewal
…t when its lock is renewed as somebody else's
…erive the sweep's abandoned-run cutoff from it
…tever a hop asks for
…ead its Anthropic fallback from the shared spec
…everal call sites share
…e adapter into composio-vendor.ts
…roup, mounted in their original order
…approvals/resume-sweep.ts
…approvals/person-response-sweep.ts
…of index.ts into http-server.ts
… agents/handoff-wiring.ts
…er out of index.ts into agents/coordination-wiring.ts
…s actions and four broker method groups
…the thread lock, and keep the core free of CopilotKit
…, with the server's run hooks passed in
…ex.ts into approvals/wiring.ts
…f index.ts into responsibilities/wiring.ts
…, where no cycle prevents it
# Conflicts: # server/src/index.ts # server/tests/passwords-sign-in.test.ts
davidmckayv
left a comment
There was a problem hiding this comment.
Requesting changes for one blocker: Desktop can't install a release that contains this. The refactor itself checks out.
What I verified on this branch
-
The moves are verbatim. A scripted syntax-tree comparison against
main:- all 123 plugin-store units and all 61 Composio units match;
- the 21 routes match on method, path, middleware and order, and their 32 handlers match;
http-server.tsmatches main'sBun.serveblock apart from two re-wrapped lines;- 99 of the 132 modified TS files change nothing but import paths.
-
index.tsstartup is unchanged. The 8 top-level awaits keep their order, and none of the three reassigned variables is passed into a wiring module by value. -
settle()matches main. It reproduces main's order of throws, error objects and side effects, apart from the intended lease change. -
The tests catch regressions. I re-broke each fix: the lease stopping on any failure, the handoff ignoring lock loss, admission after the lock, and two outcomes in
settleswapped. The tests caught every one. -
The merge from
mainlost nothing. #776, #751, #775 and #780 are intact. -
Same dependencies. The lockfile resolves the same package@version set as
main; three packages only swapped which version is hoisted. -
Both Dockerfiles build and start, and every toolkit and shared module resolves inside them. CI skipped
imageandcomponent dockerfiles, which run only with thefull-cilabel, so I built both by hand. -
It works live. I booted it on a local stack and drove it in the browser:
- a chat turn;
- a two-Bot group turn through the toolkit runner;
- the computer stream websocket, both frames and input;
- live updates over
/api/channels/events; - demonstration recording capture.
All of it worked, which covers the two paths the description lists as unverified. I didn't exercise an approval resume.
Blocker
- Desktop install fails.
- Desktop copies a fixed list of folders from the release tarball (
desktop/src-tauri/src/deployment.rs:175-187), andtoolkitisn't on it. - It then runs
bun install --frozen-lockfileat the root, which now stops witherror: Workspace not found "toolkit". Even past that, the server's@openbot/toolkit/*imports wouldn't resolve. - I reproduced it from the copied tree; main's tree installs fine. CI misses it because the startup jobs use the full checkout.
- Fix: add
"toolkit"toALSO_COPIED. Also addsharedandtoolkittoPACKAGESindesktop/src-tauri/src/preparation.rs:14, so a dependency change in either triggers a reinstall.
- Desktop copies a fixed list of folders from the release tarball (
Should fix
- The lease fails open, with no time limit (inline on
toolkit/src/lease.ts). - The toolkit core can import CopilotKit code through its own package name (inline on
biome.json).
Nits
runtimeModelis now computed beforenormalizeModelBaseUrls()runs (inline).- Setting the headless lock lifetime back to 20s fails no test.
routine-run-turn.test.ts:766andthread-lock.test.ts:157compare against the constant itself, so a literal120would pin it.
…rprint shared and toolkit's dependencies
…ugh the package's own name
…eads them, as on main
…ifetime, not only on a conflict
davidmckayv
left a comment
There was a problem hiding this comment.
Thanks, all five check out on 135cbe2.
- Desktop:
toolkitis copied into a deployment,sharedandtoolkitare inPACKAGES, and the fixture lays out both folders. - Lease: this is what I suggested. A renewal that lands late extends the lock only from when it was asked for, which is the right cautious choice. With the lifetime check removed, two lease tests and the hung-renewal handoff test fail.
- Lint: the self-reference probe is now flagged.
- Model URLs:
normalizeModelBaseUrls()runs right beforeruntimeModelagain. - Lock lifetime: 120 is pinned as a literal in both tests.
The lease, settle, thread-lock, handoff and turn-runner suites pass locally: 34 toolkit tests and 74 server tests.
Optional follow-up, not blocking: a lapse is only noticed on a tick, so a turn can run up to one interval past the lifetime: 15s for a turn, 30s for a handoff. Comparing against lifetimeMs - everyMs would stop it before the lock lapses rather than after.
Approving. app (macos), app (windows) and the Python harness job were still running when I looked.
What this changes
A refactor of the server toward packages: the code that runs a Bot's turn becomes its own workspace,
@openbot/toolkit;shared/becomes@openbot/shared; and the largest files are split. Three behaviour fixes to how turns hold their conversation's lock ride along, each in its own commit. They're described below and in the changelog.Draft, because it's big (42 commits on
main, 196 files) and is meant to be reviewed commit by commit, or split into the stacked PRs listed under Plan.Benefits
The turn engine is safer to change
toolkit/, outside the server.toolkit/src/copilotkit/, and a second rule keeps the toolkit core free of CopilotKit.main, loadingrun-turn.tspulled in 49 server modules, including the database schema and all ofcopilot.ts.settle(evidence) → TurnOutcome(completed | failed | lockLost | timedOut | suspended | interrupted | empty), andconclude()is the only place an outcome becomes a reply or an error.TurnAdmission,ComponentCatalog,ThreadLock) instead of read from module globals.Behaviour fixes
keepLease) stops a turn only when Intelligence answers 409THREAD_LOCK_RENEWAL_FAILED, meaning the lock is now someone else's. A stopped turn is retried from the start, so a blip could make a Bot repeat a side effect.No more 5,000-line files
mainplugins/store.tsplugin-store/(model, shared context, five method groups)plugins/composio-adapter.tscomposio-vendor.tsandcomposio-client/index.tsplugins/routes.tsroute-groups/(six groups)copilot.tstoolkit/src/copilotkit/agents.ts)Each split was checked mechanically:
@composio/coreindex.tsis less fragile. It had 29 places that read a name declared further down. That worked only because the read happened later, at run time, and an extraction could turn any of them into a startup crash. They're down to 12. The ones left are real cycles (for exampleactorAgentResolver↔copilotRuntime), a hoisted function, or would mean moving 100–200 lines of startup for one reference.Clearer contracts
@openbot/sharedhas explicitexports. Nothing imports it through../../../sharedany more.TURN_LIMITS) and the shared OAuth and delivery timeouts are named. The routines sweep calculates its "abandoned run" cutoff from the turn timeout instead of keeping a hidden copy.More tests: 46 new: the uncovered waiting-on-interrupt branch, the handoff lock adapter, admission, the lease rule,
settle,turnMessages/turnInput, and the provider-switch model fallbacks.How to read it
The commits follow the phases, each ending where the next starts:
feb91d1117481b1ffcbb9db0shared/→@openbot/sharedc33f0d3c…684325dcTurnAdmissiona9f5aee8…1b0329baserver/src/toolkit/, add the boundary rulec90311efsettle(), the lease, pureturnMessages/turnInput654f3174toolkit/becomes the@openbot/toolkitworkspace9f378bf0afb21822a1ceb39f…dc3d7523722e2b09…876a2aeeindex.tsreordering, CopilotKit agents into the toolkitd6e9f0baefd8ef8amain, changelogMost commits are mechanical moves. Where code moved, it moved verbatim: the store and the Composio client were split with the TypeScript compiler's syntax tree, which keeps each node's exact text and comments. The commits that change behaviour are
9f378bf0,afb21822anddc3d7523.Plan
Next, in this refactor
copilot.ts. Moving them in behind the ports already sketched (PromptContext,ModelConfig,ToolProvider) would let the toolkit take "this Bot, this person, this message" and return a finished turn.index.ts(2505),plugin-store/connections.ts(2495),app.ts(2005),plugin-store/servers.ts(1907) andcopilot.ts's agent building.After that: Slack, Teams and any other platform, through the Channels SDK
Reading OpenTag and
@copilotkit/channels0.11.0 for this work turned up two things:The OpenTag side of the pairing isn't public. OpenBot's pairing expects an
openbot.sendercontext entry and/openbot/deliverand/openbot/membershipendpoints. No public CopilotKit repository implements them, OpenTag included; upstream OpenTag sends a free-text sender label and has neither route. Theconfirm_writeapproval card does match.The runtime OpenBot already runs can host Channels.
@copilotkit/runtime1.77.0 acceptschannelson the same Intelligence runtime OpenBot mounts. OpenBot could therefore declare its own Channel:identifyUserwould be the existing binding lookup, plus same-domain email auto-linking.agentwould be the person's Bot.onInterrupt, and charts throughchannels-ui.That would remove the OpenTag relay, the shared secret and most of
delivery/opentag*.ts. It would also bring the SDK's Discord, Telegram and WhatsApp adapters. The open gap is messages a Bot starts on its own: the SDK says "Proactive delivery to subscribed conversations is not yet wired".The next step is a spike: one Channel inside
mountCopilotRuntime, and checking whether a linked person can chat, approve an action from a card and get a chart back, with no OpenTag in between.Where it runs
main. Turns still serialise on Intelligence's thread lock. The only change is when a turn gives up: on a 409 renewal (the lock is now another replica's or the person's), not on a network error. A handoff that loses its lock fails into the existing work-queue retry.SET … NX). Nothing new.http-server.ts.Boundary and audit
agent.handoff_failed), and a capability refusal is still final.Changelog
Unreleasedhas "Bot turns hold their conversation more carefully", covering the three behaviour fixes and the lock lifetime. The rest moves code without changing behaviour.Proof
Run on this branch after merging
main:Tests ran against a dedicated pgvector database, migrated to
0056. The shared local Postgres runs out of connections under the full suite.Other checks along the way:
index.tsmove, the server booted against the test database and served/healthand/api/mewith no "before initialization" errors. Handoff delivery was enabled, so its startup ran.settleorder each failed the new tests, as intended.@copilotkit/runtimeand./copilotkit/*were rejected, a copilotkit-folder import of server code was rejected, and the allowed imports passed.@openbot/toolkitand@openbot/sharedresolve inside it with their own dependencies.Not verified: the computer stream and channel websocket paths through the moved
http-server.ts. No test reaches them throughindex.ts, and the boot check doesn't open a socket.