mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
Ben Taylor
8a8e52d9e9
Closes OSS-646. Split out of OSS-641 as the unambiguous half. This PR does **not** change when activation happens — whether the long-running wrappers should auto-connect stays open on OSS-641. ## Why `createCopilotRuntimeHandler` builds the `ChannelManager` but opens no connection; activation is lazy, triggered by the first `channels.ready()`. That is deliberate (`fbf35ac59`, OSS-473) — Cloudflare/Next isolates freeze and recycle per request, so cold starts would mint conflicting listeners. Two things were left inconsistent with it: 1. `endpoints/node.ts` still documented the pre-`fbf35ac59` world — "the same `ChannelsControl` surface the underlying fetch handler **activates at creation time**" — and labelled the one required call as `// Optional:`. That's the TSDoc developers and coding agents see in-editor, and it contradicted every channel-package README. Same failure class as OSS-634. 2. `68349bc1f` gave the fetch handler a branded overload so `handler.channels.ready()` type-checks without `?.`, but the node wrapper never got it — so every call site, including our own example and all nine showcase docs pages, was written defensively. ### A live consequence, found en route `examples/slack/app/managed.ts` never called `ready()`. It built the runtime, mounted the listener, logged `[channel] started managed Channel "…"`, and only ever called `stop()` — so since activation went lazy it has connected nothing while reporting success. It was written against exactly the creation-time model the TSDoc described. Fixed here, with a regression assertion. ## What changed - **Types** — `createCopilotNodeListener` gets the branded overload pair mirroring `createCopilotRuntimeHandler`: a runtime with at least one declared Channel yields non-optional `.channels`; `activateChannels: false` and channel-less runtimes keep the optional shape. Adds `NodeCopilotListenerWithChannels`; both listener types are now exported from `@copilotkit/runtime/v2/node`. - **Docs** — node/express/hono TSDoc corrected: creation opens no connection, `ready()` is what activates, and it is required on a long-running host. Same stale claim fixed in the three example comments and `examples/slack/README.md` that repeated it. - **Call sites** — `?.` dropped from `examples/slack`, `examples/teams`, both READMEs, and the nine `showcase/shell-docs` channel pages. ## Deliberate scope choices, called out - **Express/Hono keep an optional `.channels`.** Only their TSDoc is corrected here. Their own type docs name Node as the lifecycle-owning surface and attach `.channels` best-effort, so the branded overload is Node-only for now; `endpoints-channels.test.ts` still uses `!` for those two. Say the word if the overload should extend to them. - **The non-optional shape requires a literal `channels` tuple** (`readonly [Channel, ...Channel[]]`). A runtime built from a dynamically-assembled `Channel[]` is unbranded and still needs `?.`. Now stated in the node TSDoc. - **`examples/slack/app/managed.ts` now exits nonzero if activation fails**, where before it stayed up serving HTTP with nothing connected. Intentional — fail loud, and it matches `index.ts`. Note that `ready()` resolves for `setup_required`, so a declared-but-unprovisioned channel still logs as started. - **Signal handlers are registered before awaiting activation** in `managed.ts`, so a Ctrl-C inside the 30s activation window still tears the Channel down instead of hitting Node's default handler. ## Verification - **Type contract, red → green:** the new `KeyIsRequired<typeof listener, "channels">` assertion in `handler-channels-types.test.ts` failed to compile before the overload (`error TS2344: Type 'false' does not satisfy the constraint 'true'`) and passes after. - **Example bug, red → green:** stashing only `managed.ts` fails the new guard with `expected "vi.fn()" to be called once, but got 0 times`. - **Strict-null proof:** `slack-example` and `teams-example` both `tsc --noEmit` clean under `strict: true` with the `?.` removed. This matters because the runtime package compiles with `strict: false`, so its own type test can only probe the optionality modifier structurally. - Runtime channel suites 54/54; slack example 63/63. - **Coverage limit:** the `managed.ts` guard is mocked — it proves the example *calls* `ready()` with a bound, not that a Channel connects. Nothing in CI exercises a real gateway connect for these examples. 🤖 Generated with [Claude Code](https://claude.com/claude-code)