Repository navigation
refactor(server): the server answers /api/copilotkit/info itself; @copilotkit/runtime leaves - #230
Conversation
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2bc582b. Configure here.
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
Refactors the server to answer /api/copilotkit/info with its own Hono route instead of @copilotkit/runtime, removing the runtime dependency. The new route returns a byte-identical response for the app's client, closes all other paths with 404, and refuses non-GET methods on /info with 405. Tests cover the full request surface including the real handler behind the real auth guard.
✅ No issues worth flagging.
solar-pro4 (openrouter) · 6 files · +352/-582 · 278.9s · CodeSolar
Comment /codesolar on this PR to re-run the review.
The CopilotKit runtime was mounted to answer one question, GET /api/copilotkit/info, for the app's client; every other route it served was already refused in its hook. The route is a Hono route in copilot.ts now and answers what runtime 1.67.1 answered, byte for byte, from the same per-request roster, behind the same guard. What differs is on requests the app never makes: - info is the exact path. The runtime's router read a route off a path's last segments, so /x/info and /info/ were info too. - No response carries Access-Control-Allow-Origin: * and a preflight is not answered 204. The app is same-origin and asks with its cookie, which a wildcard cannot be used with. - Another method on info is still 405 with Allow: GET, without the runtime's sentence for a body; a path that is not info is the server's own 404 whatever its method. - A roster that cannot be read is thrown to the app's error handler, which logs it and answers its own 500. The runtime logged nothing and put the error's message in the body. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
Nothing imports it since the info route became the server's own. 171 package names leave bun.lock (986 to 815) and no name@version is added: the MCP SDK's express and @hono/node-server, nested while the runtime pinned older ones, move up. The dependency list loses its line and the seven licence exceptions only its tree needed. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
… 4683) Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
…s seventeen Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
2bc582b to
317ae36
Compare
There was a problem hiding this comment.
🔆 CodeSolar review ✅ Looks good
🔄 2nd review of this pull request · 11 new commit(s) since the last one
The server now answers GET /api/copilotkit/info with its own Hono route instead of @copilotkit/runtime. The route returns the per-request agent roster byte-for-byte as the runtime did, and every other path under the base returns 404. Tests cover the full response shape, per-request roster resolution, method/spelling restrictions, absence of CORS headers, and the real handler behind the real auth guard. The mountCopilotRuntime signature changed: stallGuard is now required and timeZone has no default.
✅ No issues worth flagging.
solar-pro4 (openrouter) · 6 files · +352/-582 · 299.8s · CodeSolar
Comment /codesolar on this PR to re-run the review.
… the variable It set COPILOTKIT_TELEMETRY_DISABLED as the first import of main.ts, for @copilotkit/runtime, which read it and left the server in #230. Checked before removing: the bundles of the server's three entries (main.ts, converter.ts, scripts/migrate.ts) name the variable once, in the line that wrote it; the 177 packages the server image installs name neither it nor DO_NOT_TRACK outside bun's own type declarations. In the whole workspace the one package that reads it is @copilotkit/shared, which is the app's and never loaded by the server. DO_NOT_TRACK is still set by the server for the two children it starts (converter-process.ts, workbench/runner.ts); those are untouched. With it go its import, its entry in config-discipline.test.ts and its row in docs/configuration.md. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
…it has now undici: nothing in bun.lock asks for it since @copilotkit/runtime left (#230) — `bun why undici` finds no package — so the override named nothing. tests/workspace.test.ts keeps the lockfile's half of its check: a 5.x that comes back still fails there. uuid: recorded as being for the runtime's ^10. Three packages still ask for it (@ag-ui/client and @copilotkit/shared ^11.1.0, mermaid ^11.1.0 || …), each in a range that admits 11.1.0, the version GHSA-w5hq-g745-h8pq is about, so the override stays as the floor and docs/laf/deploying.md says so. The lockfile resolves to one uuid@11.1.1 with or without it (tried in a copy of the manifests, from the existing lock and from none). The comment on qs is corrected the same way: it is under the MCP SDK's Express alone now. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
… now Since #230 the route under /api/copilotkit is the server's own. Four comments in copilot.ts, two in app.ts and two in main.ts still described CopilotKit's runtime as the thing mounted, served or guarded. Corrected to what is true; where a comment records what went wrong with the runtime (the unguarded mount, the eventsource import that broke the tests), the history is kept and dated. No code changes. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
server/src/index.ts loaded `eventsource` before main.ts, and the test
suite preloaded it (bunfig.toml, server/scripts/test-preload.ts), to settle
a boot race: a CommonJS `require("eventsource")` inside
@copilotkit/runtime's tree against an ESM import of the same module. The
runtime left in #230.
The graph: following every static import, require and literal dynamic
import with Bun's resolver, through node_modules, from server/src/index.ts,
main.ts, converter.ts, server/scripts/*, scripts/*, evals/* and all 654
test files, reaches `eventsource` from none (a probe importing the SDK's
client/sse.js does reach it, so the walk can see it). The bundles of the
three server entries hold no eventsource module.
The experiment the old comment describes, in the image server/Dockerfile
builds (oven/bun:1.3.14, two CPUs), thirty cold boots at once, five
rounds, each boot counted at its `boot` log line:
tree before #230, entry without the preload, from source 72 of 150 died (all that TypeError)
main, with the preload, from source / as the bundle 0 of 150 / 0 of 150
this branch, without it, from source / as the bundle 0 of 300 / 0 of 300
Cold start, one boot at a time, spawn to the boot line, median of 5 in
each of three containers: bundle 175 / 195 / 172 ms on main and 175 / 173 /
169 ms here; from source 293 / 295 / 292 ms and 294 / 289 / 324 ms.
`eventsource` stays in bun.lock as the MCP SDK's own dependency; it is
no longer one of ours, so its line leaves tests/dependencies.test.ts and
the Dependabot ignore that kept ours on the SDK's major leaves too.
index.ts keeps the history.
Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
…d a runtime on the server `@copilotkit/runtime` left the server in #230 and `@copilotkit/react-core` left the app in #234; these still said, in the present tense, that they were there. What happened is kept as history, and what is said of today is true: - CLAUDE.md: the Tech line names AG-UI alone, and the React Compiler section keeps the bug with CopilotKit's message array as a past one. Shorter than it was (25,865 → 25,856 characters). - The app: the provider on the conversation, compose and signed-in routes is the tools' own; the registry, not CopilotKit, draws a renderer through a memo and hands a decision card no `respond`; the session watch wraps no runtime calls; the transcript is AG-UI's messages. - The server: who is asking is resolved for the screen socket alone, and its refusals no longer call the request a CopilotKit run; the thread-id note is in the past tense; the log test's header names the AG-UI client. - `shared/tools/standard-schema.ts` (comment only): the conversion to JSON Schema is the app's own `tool-schema.ts`. The tool list's bytes did not move (tool-list-bytes.test.tsx, 7 pass). - `shared/whereabouts.ts` (comment only): the device rides with each message. - docs/laf/design-tokens.md §9: the `.dark` block that beats inheritance on a nested element is `prompt-area/tailwind.css`'s. The vendored `app/src/shadcn-tailwind.css` has none. Measured again in a browser: under a light root, a `.dark` element has `--sand-bg-elevated: #1b1c20` and `--popover: oklch(0.205 0 0)`. - docs/laf/deploying.md: only `@ag-ui/client` still asks for a `uuid` range the override is a floor under; the other two are gone from the lock. - docs/development.md: the smoke journey's first step, as it now asks. Co-authored-by: LAF Agent <274876363+laf-agent@users.noreply.github.com>
What this changes
server/no longer depends on@copilotkit/runtime. The one thing it did here — answerGET /api/copilotkit/infofor the app's CopilotKit client, with every other route it knew already refused in its hook — is a Hono route of our own inserver/src/copilot.ts(mountCopilotRuntime, same name, same signature, same mount inmain.ts/app.ts, samerequireUserin front). The answer is what runtime 1.67.1 gave, byte for byte, from the same per-request roster (createRequestAgents), so@ag-ui/client, the forwardedProps middleware, the tools fingerprint and the agents map are untouched.@copilotkit/react-core(app) and@copilotkit/aimock(dev) are untouched.Why it is worth it: the runtime was a router, a runner, a GraphQL server, five model SDKs and a channels package loaded at every boot to answer one question.
ecd5eb23/health, median (range)ps rss), median (range)topMEM, compressed pages included), five of each idling side by side, median (range)bun.lock(against main12c1c37e)name@versionpairsThe time and memory rows were measured before the rebase, branch point
ecd5eb23against the branch; nothing that landed on main since touches what the server loads at boot. On the idle row:ps rssafter ten minutes is not a measure on macOS — idle pages are compressed and leave RSS, by an amount that depended on what else the machine was doing (main's five read 95 MiB and this branch's 162–170 MiB in the same minute, with 190 M and 50 M compressed). The footprint, which counts compressed pages, is the row above.bun.lock: the diff is deletions, plus the MCP SDK'sexpress5 and@hono/node-server2.1 moving up from under@modelcontextprotocol/sdk/…now that nothing pins older ones beside them. Noname@versionappears that was not in the lock before (compared set against set).Before / after, request by request
A script booted the server against a database of its own and recorded status, every header and the body for 59 requests under
/api/copilotkit— each method × pathcopilot.test.tsexercises, plus OPTIONS, HEAD, TRACE, trailing slash, deeper paths, the bare base path, the single-route envelope, a malformed body, a foreignOrigin, and three neighbours outside the base — on main and on this branch, same database state (one live Bot, one deleted).Access-Control-Allow-Origin: *to every response and this route does not. Among them the fourGET /infoanswers (557 bytes, equal byte for byte,Content-Type: application/json), the 19 closed doors of the test's list and a malformed-body run (404 {"code":"laf:not_found"},application/json;charset=utf-8), and the three neighbours.GET ${runtimeUrl}/info(and, only if that fails,POSTto the base, which was and is a 404); nothing else inapp/,desktop/,agent-bot/orscripts/names the path./info/,/x/info,//infolaf:not_found/nothing,/INFO, the envelope…){"error":"Not found"}{"code":"laf:not_found"}, the server's own/info(POST, PUT, PATCH, DELETE, HEAD, TRACE)Allow: GET,{"error":"Method not allowed"}Allow: GET, no bodyAccess-Control-Allow-Methods/-Headers: */info, 404 elsewhereAllowlaf:not_foundDecided by reading, not kept by accident:
/infowithcredentials: "include", which a browser refuses to pair with*— so no page on another origin could have been reading this with a session.error-codes.test.ts("no refusal anywhere in the server is a sentence") failed on them the moment they were ours. Status andAlloware kept.{"error":"Failed to retrieve runtime information","message":<the error's own message>}sent to the page. It is thrown toapp.onErrornow:route_failedin the log,500 {"code":"laf:internal"}to the page. Still a 500 to the client, which reads only the status.Where it runs
Boundary and audit
/infois now logged (route_failed) where it was silent.Korean
The gate
2bc582b0(rebased on main12c1c37e), after a cleanbun install:typecheckexit 0 ·biome lintno errors (2 warnings and 1 info, all three in files this does not touch) ·format:checkclean ·test:ci(2 workers) 9,812 tests: server 4,724 · app 2,663 · agent-computer 650 · root 1,775.server/tests/copilot.test.ts(42 → 59 in that file), in its own commit.tests/dependencies.test.tsloses the runtime's line and the seven licence exceptions only its tree needed (@ag-ui/*@0.0.54×4,@ag-ui/mcp-apps-middleware,@ag-ui/mcp-middleware,limiter) — the test named each of them as stale once the package was gone.The existing tests
The three cases of "the runtime's doors" keep their assertions: info is 200 with the roster and
mode: "sse"and contacts no Bot; the nineteen closed routes are404 {"code":"laf:not_found"}and contact no Bot;POST /api/copilotkitis 404 andPOST /infois 405. Two titles changed a word ("the runtime knows" → "a runtime knows"), and the file no longer importstelemetry-offfirst — that was for the runtime it built.The seventeen, and how each was seen red
Each line below is one edit to
server/src/copilot.ts(the last one toapp.ts),bun test tests/copilot.test.ts, restore.telemetryDisabled: true→false;classNamespelled as a literal; athreadEndpointsvalue flipped; the version constant changed (four separate runs)Allowdropped; a sentence put back as the 405's body (two runs)Allow)Access-Control-Allow-Origin: *put backapp.use("/api/copilotkit/*", requireUser)removedProof
Measured, on the server at
:3461and the app at:3462against a database made for this and dropped after, headless Chrome through Playwright:GET /api/copilotkit/info→ 200, 401 bytes, the same body main's server gave for that roster.showChecklistcard drew in the transcript and in 홈's 만든 것, three items.Those four turns ran on the first cut of the route; the cut that followed changed only the 405's body and the failure path. On the final commit the conversation was reloaded in the browser (
/info200, noAccess-Control-Allow-Origin) and the 59-request dump above was taken.Model spend, read from the trail of that database: $0.0046 (11 Bot calls, and the decision model's calls at each boot of the server).
Left, on purpose
server/src/index.tsandserver/scripts/test-preload.tsstill loadeventsourcefirst, andeventsourceis still a dependency. Their stated reason was a CommonJS module in the runtime's tree; bundlingserver/src/main.tson this branch (bun build --target=bun) finds noeventsourcein the graph at all (50 packages,eventsource-parserthe only near name). Whether the preload can go needs the concurrent-boot run its comment describes, on Linux — not done here.server/src/telemetry-off.tssetsCOPILOTKIT_TELEMETRY_DISABLED, which nothing in the server reads any more; it, its import inmain.ts, its entry inconfig-discipline.test.tsand the row indocs/configuration.mdare outside this change's files.package.jsonoverride ofuuidwas recorded (docs/laf/deploying.md) as being for the runtime's^10; not re-examined./infostill saysthreadEndpoints.list/inspect: true, as the runtime did, though those routes are closed. Kept for the byte; the client makes no request from it./infoitself goes when the app's side stops asking for it.🤖 Generated with Claude Code