mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
feat/rn-streaming-tool-render
14474 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maxim
|
c1d3a0a22c |
test(reskinnable-demo): state skin-path guard detector (ii)'s name gate honestly
The header claimed every detector matches the SHAPE of the defect, but detector (ii) (builderResultConcat) is name-gated to the two sanctioned builder-result names skinHref/keelHref — so a renamed builder slips the // bug through. That is intrinsic, not a bug: a lexical guard cannot tell `const base = skinHref()` from `const base = apiUrl.replace(...)` (banking/intelligence, legitimately concatenated) without the callee name. Keep the name gate (deliberate precision/recall trade-off — those two are the only href builders the reskin skill teaches) and correct the header to state the actual guarantee and its known blind spot. Encode the blind spot in an executable test so a renamed builder staying uncaught is a reviewed decision, not a silent regression. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
5ff990a891 |
docs(reskinnable-demo): correct README skin count from two to four
The intro said the app 'ships two of them' and listed only banking and
airline, contradicting line 56 ('banking, airline, logistics, keel'),
CLAUDE.md, and src/shell/registry.ts. Corrected the count to four, added
logistics and keel to the list with their substrates, rewrote the
substrate-agnostic paragraph to name all four honestly (banking + logistics
REST-backed, airline + keel in-memory; keel the only one with parameterized
routes), and fixed 'the richer of the two' to 'the richest of the four'.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
3368370b6c |
fix(reskinnable-demo): make the URL drift guard cover the invariant, not a spelling list
The URL-contract drift guard enumerated known spellings of a mistake
(literal ids and the exact `${skin.id}`/`${skinId}` interpolations) and
so reported green while blind to the shape that actually shipped:
`router.push(`${base}/charges`)` with `base = skinHref()`, which returns
`/` under a LOCK_SKIN deploy and ships `//charges`. A guard that lists
spellings cannot cover the space.
Rewrite the guard to match the SHAPE of the defect via three detectors
over one invariant (no in-skin link may carry a skin prefix or yield `//`):
- (i) interpolated id at the START of a quoted path, ANY holder whose
expression ends in id/Id (`/${id}`, `/${s.id}`, `/${activeSkin.id}`),
not just the literal `skin.id`/`skinId`;
- (ii) concatenation onto a value BOUND from a builder call
(`const base = skinHref()` → `${base}/x`, `${base}${x}`). Gating on
the builder BINDING is what spares the legitimate REST bases
`const base = apiUrl.replace(...)` (banking/intelligence) and
`const BASE = "/api/logistics/v1"` (logistics/actions), and keel's
inline `${keelHref(...)}#${id}` deep links (not bound vars);
- (iii) literal skin prefix (kept).
Correct the docstring/behaviour mismatch: the check matches ANY skin id,
which is STRICTER than "its OWN prefix". Kept the stricter rule (a comment
explains why: cross-skin nav is the shell switcher's job, out of scope by
living outside src/skins/; inside a skin any sibling prefix is just as
broken under a lock) rather than narrowing to the owning id.
Fix the live bug the hardened guard exposed in banking/tools.tsx: two
`base = skinHref()` concatenations (`${base}/charges` and
`${base}${page}`) now route through skinHref(), which strips leading
slashes and re-joins cleanly under both lock states.
The self-test now asserts every previously-MISSED shape is caught and the
two REST-base forms are not; the guard was also proven to fire end-to-end
by injecting a real literal-prefix and a real `${base}/x` violation into
skin sources (each failed naming its file), then reverting.
Call-Site Enumeration (Procedure 2 step 8): swept all `src/skins/**` for
in-skin link construction. Builder-result vars: `base` (banking/tools.tsx,
banking/layout.tsx), `href` (airline/keel/logistics layout.tsx). Only
banking/tools.tsx concatenated onto one (2 sites, both fixed);
banking/layout.tsx and the `href` vars use the value bare. No literal-id or
start-interpolation offenders exist. Legitimate non-lock bases confirmed
untouched: banking/intelligence `${base}/api/memories`, logistics/actions
`${BASE}/...`, and banking/actions `/api/banking/v1/.../${id}/...`.
pnpm lint, pnpm test:unit (330 tests), and pnpm build all pass.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
a719246bb1 |
fix(reskinnable-demo): stop banking tools emitting protocol-relative // hrefs under LOCK_SKIN
BankingTools composed navigation URLs by concatenating onto the no-arg
result of the skin href builder (`const base = skinHref()`). `useSkinHref`
returns "/" — not "" — for the skin index under a lock (the empty string
is not a usable href), so on a LOCK_SKIN deploy:
- `${base}${page.toLowerCase()}` for page "/team" -> "//team"
- `${base}/charges` (and the ?qs variant) -> "//charges"
Both are protocol-relative URLs: the browser reads "//team" as
"https://team/" and navigates off-site. Unlocked they were correct
(/banking/team, /banking/charges) which is why this never surfaced there.
Fix: route both through the `skinHref(path)` builder, which strips a
leading slash and yields /banking/team|/team and /banking/charges|/charges
with no "//". The two compositions are extracted into a pure module
(src/skins/banking/nav-target.ts: navTarget, chargesTarget) so they can be
unit-tested without rendering the whole tools tree, which needs the full
CopilotKit/auth/recording provider stack. Query-string behaviour at the
charges site and the "/"+"/cards" -> skin-index special case are preserved.
Red-green verified: reverting the helpers to the `${base}...` concat form
turns the two locked-deploy tests RED (asserting "//team"/"//charges"),
restoring them GREEN.
Call-site enumeration (Procedure 2 step 8):
- Local `base` in BankingTools (removed): had two code users — the
navigateToPageAndPerform target and the showCharges push. Both now call
the helpers; grep shows no remaining code reference (only comments).
Assumption removed cleanly.
- navTarget / chargesTarget (added): referenced only from tools.tsx
(navigateToPageAndPerform, showCharges) and nav-target.test.tsx. New
symbols, no external assumptions.
- SkinHref type (added): local to nav-target.ts; mirrors useSkinHref's
public return type `(path?: string) => string`. Holds.
- useSkinHref / skinHref (unchanged signature): still called as
skinHref(page.toLowerCase()) and skinHref("charges"); all other call
sites across skins (keel keelHref(path), airline/logistics
skinHref(route.segment), banking/layout.tsx base-as-index-href) are
unaffected — none concatenated onto the no-arg result, so their
assumptions still hold.
- The other `${base}` matches in banking/intelligence/{seed,forget}-memories.ts
are an unrelated API base URL, not the skin href builder.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
fcdf512a91 |
docs(reskinnable-demo): correct the false LOCK_SKIN 'defence in depth' claim on /
Under a lock, src/proxy.ts rewrites / to /<locked> in place before src/app/
page.tsx renders, so the page is unreachable on a locked deploy (verified: a
locked server answers GET / with 200 and no redirect). The old comments framed
its lockedSkinId() read as 'defence in depth' and claimed that without it a
locked / would 404 — both premised on the page running under a lock, which it
never does.
Behaviour is already correct and unchanged: redirect(`/${lockedSkinId() ??
defaultSkinId}`). The read is a proxy-INDEPENDENT backup (were / to reach this
page with the proxy absent, it targets the locked skin's real route /<locked>,
which renders — not defaultSkinId which would 404, and not / which would loop).
It is not the double-prefix trap: /<locked> is only re-rewritten to
/<locked>/<locked> when the proxy is present, and then this page never runs.
Rewrite the page.tsx header, the CLAUDE.md routing bullet, and the page.test.ts
locked-case comment to state this precisely. Left the isSkinLockedOut 'defence
in depth' bullet: verified accurate — it correctly 404s a non-locked skin if the
layout is ever reached directly.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
a5791f072a |
test(reskinnable-demo): make locked-skin guards positive, not vacuous
The headline "no in-app link carries the skin prefix" test asserted an empty match set (`a[href^="/banking"]` -> []), which passes green if the nav never renders at all. Add a positive precondition — nonzero in-app anchors and the known banking nav targets (/, /dashboard, /charges, /team) present — before asserting the prefix is absent, and also assert no rendered href is protocol-relative (`//host`), the other way the skin-href builder breaks. Make the metadata description assertion a strict null-safe exact match on the locked skin's tagline, mirroring toHaveTitle, instead of a negative-only `.not.toContain`. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
1da2001fec |
fix(reskinnable-demo): parameterize the unlocked webServer probe and correct its comments
The unlocked webServer hardcoded its readiness probe as http://localhost:3000/banking, restating both the port and the default skin id while the baseURL hardcoded the port separately. A divergence in the port or defaultSkinId would silently mismatch the probe and fail before any spec runs. Parameterize the unlocked side like the locked side: derive the port from UNLOCKED_PORT (also passed as PORT to the dev server) and the skin from the real defaultSkinId (imported from the deliberately import-free skins-config). Also correct the webServer rationale: Playwright starts webServer entries in parallel, so aimock has no ordering guarantee over the dev servers. The invariant holds because the runtime reads OPENAI_BASE_URL per request, not at boot. Update the count to three servers, and extend the reuseExistingServer warning to cover the locked port too. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
151ce3505b |
fix(reskinnable-demo): anchor proxy matcher to segment boundaries and exclude _next dev endpoints
The LOCK_SKIN proxy matcher had two boundary defects:
1. It excluded only `_next/static` and `_next/image`, not `_next` generally.
Extension-less framework paths therefore got rewritten under a lock:
`/_next/webpack-hmr`, `/_next/dev/on-demand-entries-ping`, and
`/__nextjs_original-stack-frame` all MATCHED and would rewrite to
`/<locked>/_next/...`. That breaks HMR and the error overlay — and
next.config.mjs states this demo is PRESENTED from `next dev`, so that is
the feature's real usage, not an edge case.
2. `api` and `_next` were PREFIX matches, not SEGMENT matches. A future
top-level route like `/apiary` or `/api-keys` would silently skip the
rewrite and 404 only on locked deploys.
Fix: anchor `api` and `_next` to a segment boundary (`(?:/|$)`) and exclude
`_next` plus the `__nextjs`-prefixed dev endpoints wholesale. Dotted paths
(public assets, favicon.ico) stay excluded. The `api` exclusion is NOT
weakened — `/api/copilotkit` carries the agent SSE stream and must never
enter the proxy; bare `/api` and all `/api/*` remain excluded.
Also corrected the matcher comment: it overstated the old pattern's coverage
("Next's own asset routes") and cited keel run ids as kebab-case `r-1` when
the real ids are `RUN-1041`-style (src/skins/keel/data/seed.ts). The dot-free
property the comment relies on still holds — doc ids are kebab-case
(`phi-access-contractor`), run ids are `RUN-1041` — so the conclusion stands;
only the stated evidence is fixed.
Tests: added boundary near-miss cases to src/proxy.test.ts — `/_next/webpack-hmr`,
`/_next/dev/on-demand-entries-ping`, `/__nextjs_original-stack-frame` (excluded)
and `/apiary`, `/api-keys` (matched — app routes, not the API) plus bare `/api`
(excluded). Red-green verified: the five differentiating cases FAIL against the
old matcher and pass after the fix.
Call-site enumeration (Procedure 2 step 8): `config.matcher` and `proxy` are
exported from src/proxy.ts. Next.js loads this file by convention (Next 16's
rename of middleware.ts) and reads `config.matcher` to decide which paths
invoke `proxy` — a framework consumer, not app code. The only in-repo importer
is src/proxy.test.ts (imports both `config` and `proxy`). No other module
references either symbol, so the behavior change is contained to the framework
routing hook and its test.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
494dc4f773 |
test(reskinnable-demo): cover the LOCK_SKIN deploy shape
LOCK_SKIN's headline behaviour had ZERO automated coverage. Every existing spec
pins the gate off (`LOCK_SKIN: ""` in the webServer env), so the locked shape was
verified only by hand. That gap matters more now that the lock rewrites the whole
URL space rather than just picking a redirect target: the client and server
halves (useSkinHref and proxy.ts) must agree, and if they do not the feature
half-works SILENTLY — pages still resolve, the tenant prefix just reappears in
the address bar. Nothing fails; the demo stops being what it claims to be.
Two guards, cheapest first.
`src/shell/skin-path.drift.test.ts` — a lexical guard that no file under
src/skins/** hardcodes its own route prefix. Deliberately static, not a render
test: the violation type-checks, lints, renders AND navigates correctly, so
there is nothing for a behavioural test to catch short of reading the href. The
shell's skin SWITCHER is the one legitimate hardcoded prefix (it targets a
DIFFERENT skin and only renders unlocked) and sits outside src/skins/, so it is
out of scope by construction rather than by exemption list. Verified the guard
actually fires by reintroducing airline's old `/${skin.id}/${route.segment}` and
confirming it failed naming that file.
`e2e/locked-skin.spec.ts` + a `locked` Playwright project — the real check, in a
browser against a genuinely locked server. The lock is a boot-time server env, so
the two deploy shapes are two processes; hence a second webServer rather than a
fixture. 12 tests: served at `/` with no redirect, branded metadata, static
badge, NO link carrying the prefix, click-through keeping the URL clean, deep
page cold-loading, the other skins 404ing, and the SSE + public-asset paths
staying un-rewritten.
Supporting config, each item load-bearing:
- `next.config.mjs` gains an env-driven `distDir`. Two `next dev` processes
corrupt each other's output through a shared `.next`.
- `eslint.config.mjs` ignores `.next-locked/**`. ESLint does not read
.gitignore, so without it one e2e run made `pnpm lint` report 23,706 problems
in generated output.
- `tsconfig.json` pre-lists the `.next-locked` type globs so the locked server
has nothing to append to a tracked file.
- The unlocked project repeats `ogui-routing.spec.ts` in its OWN testIgnore. A
project-level testIgnore REPLACES the config-level one rather than adding to
it, so introducing projects silently re-admitted those 7 specs — caught by
checking the per-project test counts against the pre-change baseline, not by
the run passing.
Suite goes 17 -> 29 tests: the same 17 unlocked (baseline preserved exactly) plus
12 locked. Unit tests 326 -> 330. `pnpm lint` clean, `pnpm build` clean.
Also verified airline and logistics locked in a browser — both were changed by
the parent commit (href construction AND active-state derivation) and neither had
been exercised. Nav is prefix-free and aria-current tracks correctly in both.
KNOWN CHURN, documented at the env block: Next rewrites the tracked
`next-env.d.ts` to reference whichever dist dir booted last, so a full e2e run
leaves it pointing at `.next-locked`. Discard that hunk before committing; any
build restores it.
Reskin skill: verification gains the two steps that would have caught a new
skin's violation — run `pnpm test:unit` for the drift guard, then run the skin
under `LOCK_SKIN=<id>` and open `/`.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
853156e7f8 |
docs(reskinnable-demo): teach the reskin skill the URL contract
The layout template handed every new skin the two patterns the LOCK_SKIN
root-serving change just removed: a hardcoded `/${skin.id}/${segment}` href and
a `pathname.split("/").slice(2)` segment derivation.
Both fail SILENTLY on a locked deploy, which is what makes them worth a skill
edit rather than just a fixed template. The hardcoded href still resolves — it
merely puts `/banking` back in the address bar on the first nav click, undoing
the single-tenant illusion the lock exists to create. The fixed slice eats the
first real segment when there is no prefix to skip, so every locked page reports
itself as the index and the wrong nav entry lights up.
Template now uses `useSkinHref` / `useSkinSegments`, and compares segments rather
than `pathname === href` for the active entry. SKILL.md gains a "URL contract"
section stating the rule, the two failure modes, the per-skin wrapper pattern
(`src/skins/keel/href.ts`), and the one legitimate exception — a link to a
DIFFERENT skin, which must keep the prefix and only ever renders unlocked.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
c554f4e5da |
feat(reskinnable-demo): serve the locked skin AT the root, not under its prefix
LOCK_SKIN made `/` REDIRECT to `/<id>`, so a single-tenant deploy still showed
the substrate's tenant segment in the address bar — on the front door and on
every link after it. A customer opening the Meridian deploy landed on
`/logistics`. The lock removed the OTHER skins; it never removed the prefix.
Now the prefix leaves the URL space entirely: `LOCK_SKIN=banking` serves the
cards view at `/`, the dashboard at `/dashboard`, the team page at `/team`.
Nothing redirects. Unlocked behaviour is unchanged in every respect.
Two halves, and they must agree:
- `src/proxy.ts` rewrites the prefix-free space onto the `/[skin]` route tree
(`/cards` -> `/banking/cards`). `proxy.ts` (Next 16's rename of
`middleware.ts`) and NOT a `next.config` rewrite, because `rewrites()` is
serialised into routes-manifest.json at BUILD time and would bake the lock
into the artifact — forfeiting the one-build-serves-both-hosts invariant this
feature was built around. Proxy files always run on the Node server, so
LOCK_SKIN stays a per-request read.
- `useSkinHref` (`src/shell/skin-path.ts`) makes every in-skin link prefix-free
under a lock. Without it the rewrite alone is useless: the first nav click
would put `/banking` straight back in the address bar.
Because the rewrite TARGET keeps the `[skin]` segment, `params` is untouched —
keel's `useParams<{ skin, rest }>` pages needed no change. That is why this is a
proxy rewrite rather than a collapse of `[skin]/[[...rest]]` into a root
catch-all, which would have broken them.
`useSkinSegments` replaces three copies of `pathname.split("/").slice(2)`. It
strips a LEADING skin id instead of slicing a fixed offset, so it is correct
whether or not the pathname carries the prefix — the fixed slice ate the first
real segment on every locked page, highlighting the wrong nav entry.
The SSE stream is safe by construction: the matcher excludes `api`, so
`/api/copilotkit` never enters the proxy. That was the stated reason the
original change avoided a request-time hook; the documented matcher answers it.
Under a lock the locked skin's OWN prefix (`/banking`) now 404s, consistent with
the existing "a disowned skin is as absent as /nope" semantics.
Verified on ONE build artifact served three ways (locked banking, locked keel,
unlocked), in a real browser rather than only in tests — SSR alone cannot see
these hrefs, since the skin tree is entirely client-rendered:
locked banking / -> cards view, title "Northwind Finance", nav hrefs
/, /dashboard, /charges, /team; click -> URL stays
/dashboard with aria-current on the right entry;
/banking, /airline, /nope -> 404 page
locked keel /knowledge/phi-access-policy -> doc reader renders all six
sections (useParams resolved through the rewrite);
zero /keel-prefixed hrefs in the DOM
unlocked / -> 307 /banking; hrefs prefixed; switcher present;
all four skins 200
`pnpm lint` clean · `pnpm test:unit` 53 files / 326 tests (+25) · `pnpm build`
clean, zero static routes, proxy registered.
Known, pre-existing: an unknown path under a lock renders the 404 PAGE but
returns HTTP 200. This is not caused by the rewrite — on the unlocked build
`/banking/nope` is already 200, because `notFound()` raised from the client PAGE
component (resolvePage -> null) cannot change a status Next has already
committed, whereas `notFound()` from the layout can. The lock only makes the
page-level path the one unknown URLs take.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
renovate[bot]
|
732987da9f | chore(deps): update dorny/paths-filter action to v4.0.3 | ||
|
Murat Sari
|
53cf7e6575 |
fix(angular): height measurement cleanup (#6080)
Fixes two layout bugs in the Angular chat view: 1. **Floating-input height stuck at `0`.** Measurement ran once in `ngAfterViewInit`, but on the welcome screen the input overlay isn't in the DOM yet — retries expired and the height never recovered. After the first message, the list rendered under the floating input and the scroll-to-bottom button overlapped it. 2. **Scroll wrapper sized to the viewport** (`h-[calc(100vh-9rem)]`), which overshoots when the chat is embedded in a smaller panel. |
||
|
Murat Sari
|
df8599c8a3 | Merge branch 'CopilotKit:main' into feat/height-measurement-cleanup | ||
|
Ran Shemtov
|
9b768a0b98 |
feat(showcase): finalize MAF Python - D6 green on agent-framework 1.0 latest (#5985)
## Finalize MAF Python: D6 green on official agent-framework 1.0 latest Brings the `ms-agent-python` showcase integration to a clean, reproducible D6 state on the officially published latest `agent-framework` packages, with feature parity to `langgraph-python` on everything buildable today. ### Dependencies (exact pins, official latest) - `agent-framework-ag-ui==1.0.1` - `agent-framework-openai==1.12.0` - `agent-framework-core==1.13.0` No beta/rc floors, no ranges. Removed two unused `langchain-*` deps. All framework deps are exact pins; `validate-pins` ratchet baseline moves down 31 to 27. The only remaining ms-agent-python pin FAIL is the shared-frontend `openai ^5.9.0`, identical across every integration (pre-existing baseline). ### D6 result: all green on the published mock Verified with `showcase test ms-agent-python --d6 --direct --rebuild` against the actual published `ghcr.io/copilotkit/aimock:latest` (**v1.38.0**), freshly pulled: 37 distinct cells executed, 37 conversations completed, zero failures, aggregate `d6:ms-agent-python green (104.2s)`. `tool-rendering-reasoning-chain` (previously the only red on the published mock) is now green: it needed `reasoning.encrypted_content` echoed back on the second Responses request (upstream microsoft/agent-framework#7233), which the published mock did not synthesize until [aimock#342](https://github.com/CopilotKit/aimock/pull/342), shipped in aimock **v1.38.0**. Fixed upstream, not worked around. `multimodal` is un-quarantined and now matches langgraph. It had been wrongly marked unsupported based on a local-only failure: the `sample.png`/`sample.pdf` demo assets are Git LFS pointers, and without git-lfs on PATH the attachment send fails before the run starts (`runStartCount=0`). langgraph-python multimodal fails locally for the identical reason yet declares the feature supported. Verified the MAF agent works (D6 cell green with the real assets, 2 turns, assertions passed); both production deploys serve the real 10KB PNG. `not_supported_features` now equals langgraph exactly: `[gen-ui-interrupt, interrupt-headless]` (both a shared `@copilotkit/react-core/v2` resume-path bug, quarantined in langgraph too). ### Cells fixed on this branch - `tool-rendering-custom-catchall` (18-entry fixture + MESSAGES_SNAPSHOT-drop subclass so narration renders last) - `shared-state-streaming` (seed `/document` after RUN_STARTED + `chunkSize` fixtures so replay emits per-token deltas) - `tool-rendering-reasoning-chain` (un-quarantined; green on aimock v1.38.0) - `frontend-tools-async` (removed a stray broad fixture that shadowed/looped) - `open-gen-ui` + `open-gen-ui-advanced` (removed six stray fixtures colliding in the shared gen-ui fixture file) - `multimodal` (un-quarantined; parity with langgraph) ### Deferred to upstream (not worked around) - **a2ui-recovery**: langgraph ships a bespoke A2UI validate-and-retry recovery demo. MAF Python's A2UI is going native via [microsoft/agent-framework#7423](https://github.com/microsoft/agent-framework/pull/7423), which delivers progressive streaming, error recovery, and the sub-agent design built into `agent-framework-ag-ui`, and even includes the same two bridge fixes hand-rolled here (unanswered-tool-call stripping + A2UI MESSAGES_SNAPSHOT suppression). Building a bespoke recovery demo now would be throwaway. When #7423 merges and releases, the showcase A2UI migrates to the native path and the recovery demo lands with it. ### Validators - `generate-registry`: OK - `validate-pins`: 27 fails, hash matches ratcheted baseline - `validate-parity`: PASS - `validate-fixture-tool-surface`: clean ### Notes - `useCoAgent` is deprecated; all demos use `useAgent` from `@copilotkit/react-core/v2`. - Kept in draft pending review. No blocking external gates: aimock#342 shipped in v1.38.0. |
||
|
Ran Shemtov
|
464550ef68 |
fix(runtime): skip value-less activity patch for null open-gen-ui params (#6396)
## Problem
In `open-generative-ui-middleware.ts`, when the LLM emits `jsFunctions`
(or `css`) as `null`/empty, `setParam` sets the param to `undefined`,
then `emitParamDelta` produces a JSON Patch op `{op:"add",
path:"/jsFunctions"}` with **no `value` property**.
`fast-json-patch` rejects this client-side with
`OPERATION_VALUE_REQUIRED` and drops the whole activity patch:
```
Failed to apply activity patch ... Operation `value` property is not present ... path /jsFunctions
```
Benign today (the dropped delta carried nothing) but noisy in the
console and fragile.
**Repro:** `open-gen-ui-advanced` on a live LLM (any framework); the
model frequently emits an empty `jsFunctions`.
## Fix
Guard `emitParamDelta` to skip emitting when `value === undefined`. This
covers all three callers (`jsFunctions`, `css`, and the delayed
`initialHeight` delta). Empty arrays (`[]`) and completion markers
(`jsFunctionsComplete: true`) still emit as before.
## Test
Adds a regression test feeding a `null` jsFunctions value and asserting:
- no emitted patch op is missing its `value` property
- the value-less `/jsFunctions` delta is skipped entirely
- the `/jsFunctionsComplete` marker still fires
`nx test runtime` → 22 passed. `nx check-types runtime` clean.
## Release note
This is in the published `@copilotkit/runtime` source; a runtime release
is needed before showcase consumes it.
|
||
|
Murat Sari
|
89a5c4503c | Revert change in unrelated area | ||
|
Murat Sari
|
4d5e3da712 | feat(chat): implement input height measurement and adjust scroll view styles | ||
|
Alem Tuzlak
|
19c0c109f8 |
fix(showcase): let the Mastra MCP Apps agent self-correct a rejected diagram (#6398)
## Problem
PNI-118: the Mastra **MCP Apps** cell intermittently renders an **empty
iframe** (an empty box or a thin band) on a live endpoint, while passing
under aimock. Reproduced on `showcase-mastra-staging`.
## Root cause
Not the renderer, not a delivery race, not a version pin.
Excalidraw's `create_view` takes `elements` as a **stringified** JSON
array:
```json
{"elements": {"type": "string", "description": "JSON array string of Excalidraw elements. Must be valid JSON ..."}}
```
So the model has to hand-escape nested JSON, and it appends a stray `}`
just past the closing `]`:
```
tail: ...,"width":800,"height":600}]}
^ stray brace
```
The MCP server rejects the call, returns `isError: true`, and there is
no diagram to draw, so the iframe paints empty.
The agent's raw tool-call args arrive over SSE as **valid** JSON, so
streaming and arg assembly are healthy. The stray brace sits inside the
`elements` string value, written by the model.
## Fix: let the agent self-correct
Swapping models only moved the failure rate around (gpt-4o-mini ~63% of
diagrams failed, gpt-5.4 ~30%), so this stops depending on one-shot
accuracy.
The server's error already names the exact fault and already comes back
as a tool result, and the agent had no step cap, so a retry was
mechanically possible all along. **What blocked it was our own prompt:**
`"Call create_view ONCE"` and `"do NOT iterate, do NOT make multiple
calls. Ship on the first shot."`
Now the prompt tells the model to read the error and try again, capped
at **2 corrections (3 calls total)**, with `stopWhen: stepCountIs(6)`
bounding the loop if it never converges. This mirrors the
validate-then-retry recovery pattern already used for A2UI on the other
integrations.
The agent also moves to `gpt-5.4` (owner preference for the 5.x line).
## Validation
Against the **real Excalidraw MCP server**, using the agent's prompt
extracted verbatim from this file and the real tool schema, over the
Responses API (the path the AI SDK actually uses):
| scenario | result |
| --- | --- |
| normal runs | **12/12 succeeded**, all on the first call |
| attempt 1 force-corrupted with the real-world stray `}` | **10/10
recovered on the second call** (`err > ok` every trial) |
Model comparison that motivated moving away from one-shot (create_view
against the real server):
| model | OK | isError |
| --- | --- | --- |
| gpt-4o-mini (before) | 3 | 5 |
| gpt-5.4 (no retry) | 7 | 3 |
| gpt-4.1 | 8 | 0 |
| gpt-5.5 | 10 | 0 |
Prompt hardening alone was measured and does **not** fix it (gpt-4o-mini
8/12 to 7/12 invalid; gpt-5.4 still 2/16).
Also verified in the running app (local dev server, real key): valid
JSON, `isError: false`, diagram rendered.
**Not yet verified in-app:** the recovery path itself. No natural
failure occurred during the in-app runs, so the retry is proven at the
API level rather than through the Mastra agent loop.
## Why aimock never caught this
- aimock replaces the LLM and the fixture hard-codes the `create_view`
tool call, so no model-generated JSON is involved. This bug cannot occur
under replay.
- The shared d5/d6 probe (`harness/src/probes/scripts/d5-mcp-apps.ts`)
asserts only that an iframe element exists
(`[data-testid="mcp-app-iframe"] || iframe[sandbox]`) - "the page
renders an iframe shell". It never asserts a diagram rendered, and a
**rejected payload still mounts an iframe**. So the probe is blind to
this class of failure by construction.
The fixture is unaffected by this change:
`showcase/aimock/d6/mastra/mcp-apps.json` contains no model or `gpt`
reference.
## Follow-ups (not fixed here)
1. **The mcp-apps fixture payload is itself schema-invalid.** Its
`elements` is a JSON **array**, but `create_view` requires a **string**.
Sent verbatim to the real server it returns `-32602 Invalid arguments
for tool create_view: expected "string"`; stringified, the same payload
succeeds. No test overrides `MCP_SERVER_URL`, so aimock runs still call
the real server.
2. **`MCPAppsActivityRenderer` ignores `isError`** and still renders an
empty sandbox iframe, which is why a rejected tool call looks like a
silent blank box. Lives in the published `@copilotkit/react-core`, so it
would need a release.
3. **The shared probe could assert the diagram actually drew**, not just
that an iframe exists. That is one shared probe across every integration
(iron rule 1), so it is broader than this ticket.
|
||
|
Ran Shemtov
|
adb2848db7 | Merge branch 'main' into claude/jolly-boyd-38b55c | ||
|
Ran Shemtov
|
a7006cd1ed | Merge branch 'main' into claude/elated-snyder-01a8ce | ||
|
David McKay
|
2d05114160 |
fix(core): stop HITL continuations reusing the originating run id on the wire (#6411)
Linear: [CPK-7786](https://linear.app/copilotkit/issue/CPK-7786/hitl-continuation-reuses-the-originating-run-id-on-the-wire-breaking) Regression fix. **Nothing from #6296 is reverted** — its goal is kept and moved one layer up. ## What broke #6296 (@rodboev) preserved the logical run id across a HITL resolve by pinning the originating id on the follow-up's agent invocation, correctly fixing #3456 (external tracing saw one logical run split into two halves). Pinning it **on the wire**, though, made the transport treat the follow-up as a resumption of a run it had already finished: - it re-delivered that run's already-applied half, duplicating every tool call on the message — each duplicate carrying **empty arguments**, because a start event has none and the `TOOL_CALL_ARGS` deltas that follow are addressed to the first copy; and - the follow-up's own tool call never reached client state, so its card never rendered. In `reskinnable-demo`'s banking skin that killed teach mode: the agent called `awaitDashboardDemonstration`, the server emitted `TOOL_CALL_START` for it, and the live "Recording your workflow" card never appeared — no REC indicator, no step feed, no "I'm done", so a demonstration could not be finished or saved. ## Evidence - Intelligence event log shows **two `RUN_STARTED`/`RUN_FINISHED` cycles under one run id**, with `TOOL_CALL_START [awaitDashboardDemonstration]` in the second — the server does emit it. - Instrumented `useRenderToolCall`: **never invoked** for that tool, despite its renderer being registered. - Client message state after the click: `assistant tc:["recall_memory","recall_memory"]`, `assistant tc:["offerWorkflowRecording","offerWorkflowRecording"]` — prior cycle re-applied, new call absent. - Bisect: reverting #6296's four source files makes the card appear; restoring them breaks it again. No later commit touches those files. - Ruled out: React StrictMode (disabled in that app), model/prompt/tool-availability differences (byte-identical between the working and broken apps), and the view-layer dedupe from #6407. ## The fix `markNextRunAsContinuation` already accepted an `expectedRunId` that was never used. It now records it, and the state manager re-stamps the continuation's events onto that id. So: - **logical identity is preserved** — state/message association and external tracing still see one run, which is #6296's whole point; - **the wire identifies the invocation honestly** — the follow-up no longer claims to be a run that already finished, so nothing is re-delivered and the continuation's own tool call lands. ## Test changes — please review this part closely `core-follow-up`'s run-id test asserted the *mechanism* (both invocations carry the same wire id), which this deliberately changes. It now asserts the *goal*: the originating id is pinned on the first invocation, and the follow-up leaves the id to the transport. Its sibling assertion — the thread still knows exactly one run — was already there and passes untouched. A new `StateManager` test covers the re-stamp directly, verified **red before green** by dropping the `expectedRunId` lookup (it fails, along with one of #6296's own tests). @rodboev — flagging you directly since this touches your change from today. If the wire-level pinning was load-bearing for something I have not seen, say so and I will rework it. ## Verification - `@copilotkit/core` 58 files and `@copilotkit/react-core` 123 files pass. - In-browser against a live Intelligence stack: before, the recording card never rendered; after, it renders with its REC indicator and I'm done / Cancel controls. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Maxim
|
1972599063 |
docs(reskinnable-demo): document the LOCK_SKIN single-tenant gate
Covers the two things a reader would otherwise assume wrongly: it does NOT pin dark/light (separate axis), and it does NOT hide the inspector — a locked deploy still shows it, which is the intended FDE configuration. Says what the gate actually governs: the UI and routing expose only that skin. It is a presentation/deploy gate, NOT a security boundary — all four agents stay registered server-side, so another skin's agent endpoint remains reachable under a lock. Also corrects the stale "floating selector at the bottom-left" description; the switcher is a dropdown at the top of the assistant column. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
David McKay
|
6188404f3a |
test(react-core): assert the legacy HITL follow-up goal, not the wire id
The sibling of the core-follow-up assertion, missed in the previous commit: it required the follow-up invocation to repeat the originating run id on the wire, which is exactly what this change stops doing. It now asserts the follow-up happened and left the id to the transport. Logical identity is covered where it now lives — StateManager's re-stamp test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
3b548cb1c7 |
docs(reskinnable-demo): note why inspector identity ignores LOCK_SKIN
The inspector's agentId-less /memories and /info requests stay keyed to defaultSkinId even under a lock, so on a deploy locked to a non-default skin they resolve a different scope than the running agent. Deliberate: the default resolver is the one whose scope is seeded, so switching to the locked skin's resolver would read empty on any skin without seed data. Only banking ships real durable memory and it is also the default, so the two align in the configuration that matters. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
779bd501d0 |
test(reskinnable-demo): pin LOCK_SKIN off for the e2e dev server
The suite visits /airline and asserts all four switcher options, so a developer with LOCK_SKIN set locally would watch it fail for reasons that look nothing like the cause. The pin only covers a server Playwright STARTS. reuseExistingServer means a warm local run adopts an already-running pnpm dev and skips the whole env block, so the comment documents both shapes: a non-banking lock 404s the hardcoded /banking readiness probe and aborts at webServer startup, while a banking lock gets through and fails the /airline assertions instead. In CI reuseExistingServer is false, so the pin always applies. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
8b5de10b11 |
feat(reskinnable-demo): collapse the switcher to a brand badge when locked
The dropdown is not rendered at all — no trigger, no chevron, no options in the DOM. A disabled dropdown was rejected: it implies a choice that does not exist and reads as a bug rather than as a single-tenant product. The badge is a div with cursor:auto, no handler and tabIndex -1, so there is no dead control to click or tab onto. The identity block is defined once and rendered into either a button or a plain div, so the two modes cannot drift apart. Swap-sides, hide, the skin-selector testid the layout e2e keys off, and useSkinThemeReconcile's root all stay put. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
635900c73f |
feat(reskinnable-demo): brand the locked deploy's SSR title and description
The tab read "CopilotKit Reskinnable Demo" beside the locked skin's own favicon, leaking both "CopilotKit" and "demo" on the most visible surface a prospect sees. generateMetadata brands the title AND the description from the locked skin, so crawlers and link unfurlers see a coherent product page. A client effect cannot do this: Next applies route metadata after hydration, so SSR always shipped the demo strings. force-dynamic here too. The root layout reads LOCK_SKIN and PRESENTER_RESET_ENABLED per request and threads both into client gates; correctness otherwise rested on the implicit invariant that every descendant route happens to be dynamic. Cost is one dynamically-rendered /_not-found. Unlocked metadata is byte-identical in both fields. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
f113cf69d6 |
feat(reskinnable-demo): 404 every skin but the locked one
Enforcement is one extra notFound() condition in SkinLayout. No middleware is needed: notFound() throws during render, so a disowned skin never mounts a provider, a thread or an agent registration. A redirect WOULD have needed request-time middleware, and a route matcher there risks intercepting /api/copilotkit's SSE stream. Under a lock, /airline is as absent as /nope — uniform 404 semantics. / now redirects to the locked skin, without which a locked deploy's front door would land on defaultSkinId and 404. force-dynamic on / is not optional: reading process.env is not a dynamic API, so next build otherwise prerenders / and bakes the build-time skin into the redirect. A deploy built unset then run locked sent / to a 404 front door. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
8a7250bccc |
feat(reskinnable-demo): thread the LOCK_SKIN gate to client chrome
Same shape as the presenter-reset gate: the server env is read in the root layout and passed down through a small context. The context default is null (unlocked) so any subtree without the provider — including SelectorCard's bare unit tests — behaves exactly as before. isSkinLockedOut is a named predicate rather than an inline comparison because inverting it would 404 every skin on an UNLOCKED deploy. Extracted, it gets exhaustive mutation-sensitive tests without rendering SkinLayout and mounting CopilotKitProvider. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
35b267dc9e |
feat(reskinnable-demo): add lockedSkinId(), the LOCK_SKIN reader
A per-deploy SERVER env, deliberately non-NEXT_PUBLIC_ like PRESENTER_RESET_ENABLED, so one build serves both a locked single-tenant host and the unlocked four-skin demo. Throws on an unrecognised id rather than falling back to unlocked: silently accepting a typo would 404 every skin AND send / to a 404 too, leaving the whole app dark with nothing pointing at the cause. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
ca5c84bb76 |
feat(reskinnable-demo): expose skin ids and identities as import-free config
LOCK_SKIN must be validated, and a locked deploy's brand and tagline resolved, from server components. Those cannot import registry.ts — it pulls in four client skin modules. skins-config.ts is the import-free home for that, so the data is duplicated there and fenced by a drift guard asserting it matches the registry, which is what stops the copy rotting. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Tyler Slaton
|
ac8b3bb6d0 |
fix(channels): make managed Slack DM replies reliable (#6368)
## Summary - Start managed Slack DM status in the Slack thread. The server uses `messageTs` as the native status and streaming anchor. - Use legacy message create and replace only when native stream start fails. After native output opens, append and stop errors fail the run. - Keep a separate provider reference for each legacy long-message chunk. ## Test plan - Fresh `@copilotkit/channels-slack` package tests: 383 passed. - Fresh `@copilotkit/channels-intelligence` package tests: 192 passed. - Type checks passed for both packages. - Builds passed for both packages. - Formatter check passed. - `git diff --check` passed. No live Slack testing was run. |
||
|
David McKay
|
696c44244b |
fix(core): stop HITL continuations reusing the originating run id on the wire
#6296 preserved the logical run id across a HITL resolve by pinning the originating id on the follow-up's agent invocation. That fixed #3456 (external tracing saw one logical run split into two halves), but pinning it on the WIRE made the transport treat the follow-up as a resumption of a run it had already finished. It re-delivered that run's already-applied half — duplicating every tool call on the message, each duplicate carrying empty arguments, since a start event has none and the TOOL_CALL_ARGS deltas that follow are addressed to the first copy — and the follow-up's own tool call never reached client state, so its card never rendered. In the reskinnable-demo banking skin that broke teach mode outright: the agent called awaitDashboardDemonstration, the server emitted TOOL_CALL_START for it, and the live "Recording your workflow" card never appeared, leaving no way to finish or save the demonstration. #6296's goal is kept, moved one layer up. The continuation is registered against the originating id (markNextRunAsContinuation already took an expectedRunId parameter, previously unused) and the state manager re-stamps the continuation's events onto it. State/message association and external tracing still see ONE logical run; the wire is simply allowed to identify the invocation honestly. Nothing from #6296 is reverted. core-follow-up's run-id test asserted the mechanism (both invocations carry the same wire id), which this deliberately changes, so it now asserts the goal: the originating id is pinned on the first invocation and the follow-up leaves it to the transport. Its sibling assertion — the thread still knows exactly one run — was already there and still passes untouched. A new StateManager test covers the re-stamp directly; verified red before green by dropping the expectedRunId lookup. Verified in the browser against a live Intelligence stack: before, the recording card never rendered; after, it renders with its REC indicator and I'm done / Cancel controls. `@copilotkit/core` 58 files and `@copilotkit/react-core` 123 files pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tyler Slaton
|
0cf899095b |
fix(channels): take the legacy path when a dropped stream start settles as applied
The gateway settles a direct-message slack.stream.start provider failure as applied with capabilityError and no provider reference. Parse the start result inside the fallback try so that shape reaches the legacy create instead of hard-failing the stream body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Alem Tuzlak
|
85d30dd327 | fix(channels): keep managed Slack DM replies visible | ||
|
Benjamin Taylor
|
a3c5f079ed |
fix(react-ui): let sidebar children fill the viewport height
Children of `CopilotSidebar` cannot use `height: 100%`. Both wrappers the sidebar puts around your app -- `.copilotKitSidebarContentWrapper` and `.copilotKitModalChildrenWrapper` (which had no CSS rule at all) -- are auto-height blocks, so a percentage height on a child has no definite containing block and collapses to content height. Add an opt-in `fullHeightChildren` prop that gives the content wrapper a one-viewport height and lets the children wrapper fill it. It is opt-in because the content wrapper wraps the entire consumer app, and giving every react-ui sidebar user a flex column with a fixed height would reflow apps that never asked for it. The height is a viewport unit, not `100%`: `100%` only resolves when every ancestor (html/body/#root) also declares a height, which react-ui neither sets nor can guarantee, so it would silently no-op in a stock Next.js app. `min-height: 0` on the children wrapper clears the flex-item `min-height: auto` floor so tall content scrolls inside the child instead of stretching the wrapper past the viewport. Fixes #261 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Ashish Shaw <77574570+ashish4143@users.noreply.github.com> |
||
|
Ben Taylor
|
ca9a481efd |
fix(web-inspector): show step lifecycle events (#6323)
## What does this PR do? The Web Inspector did not record `STEP_STARTED` or `STEP_FINISHED` from live agent events. This PR: - records both step lifecycle events - adds both events to the Inspector filter - adds a regression test ## Related PRs and Issues - Fixes #6324 ## Testing - `pnpm nx test @copilotkit/web-inspector --skip-nx-cache` - `pnpm nx run @copilotkit/web-inspector:check-types --skip-nx-cache` - `pnpm nx run @copilotkit/web-inspector:build --skip-nx-cache` - repository pre-commit checks ## Checklist - [x] I have read the [Contribution Guide](https://github.com/copilotkit/copilotkit/blob/master/CONTRIBUTING.md) - [x] No documentation update is needed because this fixes existing behavior without changing the public API - [x] "Allow edits by maintainers" is checked |
||
|
David McKay
|
d4d10409f2 |
fix(react-core): collapse duplicate tool-call ids in the message view (#6407)
Follow-up to #6404. That PR stopped the *duplicate write* from a resurrected approval card; this one removes the duplicate card itself. ## Symptom Approving a HITL card in the banking skin left a **second, blank copy** of the same card in the transcript — "Open policy exception" with no transaction id and no code — plus a React warning: ``` Encountered two children with the same key, call_QHa801k5PL32WtPKsPItO1pI ``` ## Root cause `@ag-ui/client`'s `TOOL_CALL_START` handler appends to the parent assistant message's `toolCalls` with no check for an existing entry with that id: ```js message.toolCalls ??= []; message.toolCalls.push({ id, type: "function", function: { name, arguments: "" } }); ``` So when a start event is applied twice — which the HITL flow triggers when the run syncs after `respond()` — the message carries the same call twice. The second copy has **empty `arguments`**, because a start event carries none; the args arrive afterwards as `TOOL_CALL_ARGS` deltas addressed to the first copy. That empty copy is what rendered as the blank card, and `CopilotChatToolCallsView` uses the call id as its render key, hence the warning. Evidence gathered while diagnosing: - Instrumented the message view: the offending assistant message ends up with `toolCalls: ["call_X","call_X"]`, and the React warning fires on the same id in the same tick. - Queried a local Intelligence stack: the server emits **exactly one** `TOOL_CALL_START` for that id (one `START`, one `END`, one `RESULT`). So this is client-side state, not a stream defect. - React StrictMode is **not** involved — the demo sets `reactStrictMode: false`. ## Fix Extends the existing `deduplicateMessages()` — which already collapses duplicate *message* ids arriving from streaming re-delivery — to also collapse duplicate *call* ids within a message, preferring whichever copy actually carries arguments. Applied outside the merge branch too, because the duplicate also lands on a message that was never itself duplicated (observed `rawMsgs=5 dedupMsgs=5` with the duplicate still present). Returns the original array untouched when there is nothing to collapse, so memoized consumers don't re-render needlessly. ## Scope / known limitation This fixes what renders. The underlying agent state still holds the duplicate entry, so a fully correct fix also wants an idempotency guard in the AG-UI start handler (upstream). Filed separately — flagging here so the remaining gap is explicit rather than implied-fixed. The precise reason a single start event gets applied twice is also still open; the fix is deliberately robust to re-application whatever the trigger. ## Verification - 5 regression tests added, **verified red before green** (neutered the fix → 3 failed with "Expected 1, Received 2"; restored → pass). - Full `@copilotkit/react-core` suite: **1480 passed / 123 files**. - `test`, `publint`, `attw` green for react-core and its dependents. - In-browser against a live Intelligence stack: before, approving left a blank second card + 3 key warnings; after, **one card and zero warnings**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Ben Taylor
|
9a86e21d48 |
feat(runtime): split Channel status into transport and provider legs (refs OSS-739) (#6360)
**Half 2 of 2 for OSS-739.** Gateway half ships first: CopilotKit/Intelligence#746. ## Why `status().overall === "online"` proved only that the runtime reached the Gateway with a valid project API key. It said nothing about whether a Slack/Teams app was bound to the Channel, so **a Channel with no provider at all reported `online`** — and every version of our Channels onboarding guidance used that value to certify end-to-end success. `setup_required` had **no producer**. The manager set it only when the activation engine threw `SETUP_REQUIRED`, and the engine stopped doing that at the 2026-07-29 realtime-boundary cutover (`8f166577ce`). In published `@copilotkit/channels-intelligence@0.7.0` the string survives in exactly one file — a shipped *test*. The 15 doc comments describing the state outlived the mechanism, which is why nobody noticed for a week. ## Change - `connectRealtimeGateway` captures the control join reply (it was **discarded**) and exposes `providerStates()`. Phoenix's `Push.resend` preserves `recHooks`, so the hook re-fires on every auto-rejoin — a Channel provisioned while the runtime was disconnected is picked up with no extra plumbing. - The launcher and the manager's handle view delegate it as a **getter**, not a captured snapshot, for that same reason. - `status()` gains `detail`, reporting `transport` and `provider` separately so a caller can assert the leg it cares about: ```ts status() → { overall: "setup_required", channels: { support: "setup_required" }, detail: { support: { status: "setup_required", transport: "online", provider: "not_attached" } }, } ``` `channels` keeps its shape — turning its values into objects would break the CLI's `channels-report` and the starter channel-host — but its values are now the fold of the two legs, which is what makes `overall` honest. - The stale `setup_required` doc comments are corrected, with a note not to describe the state again without a path that can emit it. ## Back-compat: `unknown` is load-bearing An older Gateway, a Gateway whose lookup failed, a handle without the seam, a Channel the Gateway did not mention, an unrecognised state, and a throwing getter **all** yield `unknown`, which keeps the transport-derived status — exactly today's behaviour. Only a *positively reported* absence downgrades a Channel, so no existing deployment turns amber on upgrade. The **41 pre-existing channel-manager tests pass unchanged**, which is that guarantee. ## Testing - `channel-manager-provider-leg.test.ts` — 14, incl. the regression test that never existed ("reports setup_required for a joined Channel with no provider attached") and one case per degradation path - `realtime-gateway-provider-states.test.ts` — 8 parser cases - `realtime-gateway.test.ts` — +2 proving the wiring end-to-end through real Phoenix framing, not just the parser - Full suites: runtime **1874/1874**, channels-intelligence **192/192**; `check-types` and `build` clean for both packages **I mutation-tested the fold** — neutralising it fails 5 tests including the linchpin — so these assert behaviour rather than passing vacuously. Worth noting: two type errors (`ChannelsHandle` in `runtime.ts`, a session mock) were invisible to vitest, which transpiles without typechecking. The pre-commit build gate caught them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
David McKay
|
eee580f6da |
fix(react-core): collapse duplicate tool-call ids in the message view
AG-UI's TOOL_CALL_START handler appends to the parent assistant message's `toolCalls` without checking whether an entry with that id is already present, so whenever a start event is applied twice — which the human-in-the-loop flow triggers when the run syncs after `respond()` — the message carries the same call twice. The second copy has EMPTY arguments, because a start event carries none; the args arrive afterwards as TOOL_CALL_ARGS deltas addressed to the first. Rendering both produced a phantom duplicate card in the transcript (in the banking skin: a second "Open policy exception" with no transaction id or code) plus a React "Encountered two children with the same key" warning, since the call id is the render key in CopilotChatToolCallsView. Verified against a local Intelligence stack that the server emits exactly ONE TOOL_CALL_START for the affected id, so this is client-side state, not a stream defect. React StrictMode is not involved (the demo disables it). Extends the existing deduplicateMessages() — which already collapses duplicate message ids from streaming re-delivery — to also collapse duplicate call ids within a message, preferring whichever copy actually carries arguments. Applied outside the merge branch too, because the duplicate also lands on a message that was never itself duplicated. Returns the original array when there is nothing to collapse, so memoized consumers do not re-render needlessly. Does not change the underlying agent state, which still holds the duplicate; that needs an idempotency guard in the AG-UI start handler. 5 regression tests, verified red before green. Full react-core suite: 1480 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ben Taylor
|
743351cb3f |
fix(runtime): initialize arrays before /- append in AGUISendStateDelta (#6293)
## Summary `AGUISendStateDelta` can emit an array append against a state where the target array has not been initialized. The emitted patch then fails during event compaction with `OPERATION_PATH_CANNOT_ADD`. This change keeps the authoritative state represented in emitted events, initializes a missing array immediately before its first `/-` append, and applies the same contract across the generic, AI SDK, and TanStack state-delta paths. Existing arrays and valid deltas retain their current contents and operation order. Closes https://github.com/CopilotKit/CopilotKit/issues/5998 ## Changes - Emit the input state before the first delta when no earlier state event represents it - Initialize a missing array before normalized classic, AI SDK, and TanStack state deltas append through `/-`; custom raw-event mode remains outside this normalizer - Preserve populated arrays and already-valid patch operations - Preserve caller-owned state when structured cloning falls back - Cover missing-array reconstruction, sibling converters, failure guards, and existing-array preservation - Follow the current `release.config.json` and Nx release convention; no Changeset file is added. ## Test plan - [x] `pnpm -C packages/runtime exec vitest run src/agent/__tests__/state-tools.test.ts src/agent/__tests__/converter-aisdk.test.ts src/agent/__tests__/converter-tanstack.test.ts` - [x] `pnpm -C packages/runtime exec vitest run` - [x] `pnpm exec nx run @copilotkit/runtime:check-types` - [x] `pnpm exec oxfmt --check` on changed runtime files - [x] `pnpm exec oxlint` on changed runtime files, zero errors with two pre-existing no-shadow warnings - [x] Verify no `.changeset` file is added because current main uses Nx release - [x] Verify the final diff contains only the private helper, runtime implementation, sibling converters, and focused tests |
||
|
Ben Taylor
|
a3d0d2bfab |
fix(runtime): reject unenforceable mcpApps tool policy instead of silently ignoring it (#6292)
## Summary `mcpApps.servers` entries that carry `includeTools` or `excludeTools` are currently accepted even though the pinned `@ag-ui/mcp-apps-middleware` package has no option for them. The runtime then ignores the keys, so tools an operator intended to restrict remain available. This change rejects that configuration instead of allowing a silent no-op. ## What CopilotKit owns - `mcpApps.servers` configuration and `agentId` scoping. - Projection of selected servers into `MCPAppsMiddleware`. - Reporting unsupported configuration before middleware construction. Discovery, model-emitted tool execution, frontend-proxied execution, server identity, and tool provenance belong to `@ag-ui/mcp-apps-middleware`. ## Changes - Extract the server projection into `resolveMcpAppsServers`, which scans all configured entries for defined policy keys, filters by `agentId`, strips only `agentId`, and forwards other fields unchanged. - Return a configuration error naming the unsupported key, server, pinned middleware version, owning package, and issue when a policy key is supplied. - Add tests for agent scoping, field forwarding, malformed and empty values, undefined spread values, constructor avoidance, and the existing HTTP error path. - Document the ownership boundary and add a runtime changeset. ## Why the filter stays external The pinned package is version `0.0.3`. It owns the private server maps, UI-tool discovery, model-emitted execution, and frontend proxy execution. A CopilotKit middleware could observe only one of those paths and would have to duplicate private server identity and tool provenance. The complete `includeTools` and `excludeTools` implementation belongs in the external package, where one predicate can cover discovery and both execution paths. ## Current behavior Plain JavaScript or JSON configuration can supply `excludeTools: ["delete_account"]` without a TypeScript excess-property check. The runtime currently accepts the configuration, constructs `MCPAppsMiddleware`, and leaves the tool available. The new behavior returns an HTTP 500 through the existing runtime error path, names the unsupported key and dependency, and does not construct the middleware. ## Follow-up The counterpart change in `@ag-ui/mcp-apps-middleware` should add the fields to the per-server configuration, preserve absent versus empty include lists, resolve server identity through its existing maps, and apply one predicate after UI-resource discovery and before model-emitted and proxied tool execution. Once that version is released, CopilotKit can remove the rejection and pass the fields through unchanged. ## Related issue Refs #5930. The cross-repository ownership split follows the proposal in https://github.com/CopilotKit/CopilotKit/issues/5930#issuecomment-5128722524. This PR does not close the issue. ## Test plan - [x] `pnpm -C packages/runtime exec vitest run src/v2/runtime/__tests__/mcp-apps-servers.test.ts src/v2/runtime/__tests__/mcp-apps-middleware-integration.test.ts` passed, 2 files and 20 tests - [x] `pnpm -C packages/runtime exec vitest run` passed, 129 files and 1,836 tests - [x] `pnpm exec nx run @copilotkit/runtime:check-types` passed - [x] `pnpm exec oxlint` and `pnpm exec oxfmt --check` passed on changed TypeScript files - [x] `pnpm check:plugin-skills` passed - [ ] `CI green for static / quality and test / unit on Node 20, 22, and 24` |
||
|
Benjamin Taylor
|
9280e71346 |
refactor(channels): trim redundant provider-leg tests and fix a wrong comment
Self-review of the previous commit. Corrects a factual error I introduced: the `SETUP_REQUIRED` note claimed such a Channel "has no transport at all (the launcher never returned a handle)". False for the `hasDirectAdapter` branch, which starts the developer-owned transport and assigns a synthetic handle — so there IS a running transport there. Dropped the wrong reasoning and shortened the note to the part that holds: the misattribution is cosmetic on a path with no producer, and a future producer should report through the `providerStates` seam. Removes three tests that did not earn their place: - "calls the seam ON the session" — redundant. `ProviderStateGateway.providerStates` reads `this`, so the two remaining tests already fail if the launcher ever used a detached reference. It died on the same mutation as the first test, for the same reason. - "omits providerStates for a session without the seam" — survived the mutation that removes the forward, so it guarded nothing. - the channel-level-error rejoin case — same `Push.resend` hook as the transport drop, so it re-proved one mechanism at ~1s extra wall-clock. Kept the drop case: it asserts a genuinely fresh socket, which is the "provisioned while the runtime was disconnected" story the design claim is about. - "keeps the last reported states while a rejoin has not yet succeeded" — pinned behaviour with no observable consequence, since the transport leg dominates the fold while offline. Also trims the drift-guard comment: why a guard was NOT added belongs in the PR discussion, not permanently in source. Re-mutation-tested after trimming: removing the forward kills both remaining seam tests; making `providerStates` a snapshot kills the rejoin test while the other 41 gateway tests pass. Verified: channels-intelligence 195/195, runtime 1874/1874, build + oxfmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ben Taylor
|
a0daff1eab |
docs(showcase): align A2UI docs with v0.9 helpers (#6288)
## Summary The A2UI integration docs still use helper names and wire keys from before the Python SDK's v0.9 API. The examples now match the current SDK, while the A2A page clearly labels its cloned starter's v0.8 compatibility contract. ## Changes - Update the generic, DeepAgents, and LangGraph A2UI pages to current helper names, wire keys, and operation order. - Reconcile the A2A page to the cloned starter's v0.8 payload and defer its v0.9 migration. - Remove unsupported `action_handlers=` and `dataContextPath` claims from the advanced examples. - Use the exported `createA2UIMessageRenderer` `onAction` interceptor in the React guides. ## Out of scope Migrating `examples/integrations/a2a-a2ui/` from its v0.8 renderer and operation list requires source and example changes outside this documentation-only target. ## Related PRs and Issues Addresses the v0.9 documentation portion of #4821. The corrected names follow `sdk-python/copilotkit/a2ui.py`; the A2A page follows the cloned starter's current v0.8 contract. Closed partial work is tracked in https://github.com/CopilotKit/CopilotKit/pull/5854. ## Test plan - [x] Documentation search passed. The v0.9 pages contain no stale Python helper names, current wire names are present, and the A2A compatibility page is labeled. - [x] Diff validation passed. Only the eight named MDX files changed. - [ ] Shell-docs typecheck, lint, and formatting were unavailable because this worktree has no installed `node_modules`; CI will run them on the PR. - [ ] CI green (`static / quality`, `test / unit` on Node 20/22/24). |
||
|
Benjamin Taylor
|
c71d9ac58b |
fix(channels): forward the provider seam through the exported launcher helper
Addresses review on #6360. `startChannelsWithGatewayControl` is public so callers can compose over a session they manage themselves, but it forwarded only `onClose` and `onStateChange` — not `providerStates`. A handle without the provider seam makes `ChannelManager.providerLeg` fall back to `unknown`, which keeps the transport-derived status and reports `online` for a Channel with no provider bound: the exact false green OSS-739 removes, still reachable through a public export. The guard is widened too, so a session exposing only `providerStates` is no longer dropped on the fall-through path. Tests the reconnect claim that makes `providerStates` a getter rather than a snapshot. Nothing exercised it: the gateway tests covered only the initial join reply, and the manager-side rejoin test proves the manager re-reads on each `status()` call, not that the session's value ever changes. The fake socket's join reply can now vary per join, so a drop -> rejoin carrying a different `channels` map asserts the refresh over real Phoenix framing — via both rejoin paths (channel-level error on a live socket, and a full transport drop onto a fresh socket), plus the case where a rejoin has not yet succeeded and the last known states must persist. Mutation-tested both: removing the forward kills 3 of 4 seam tests, and making `providerStates` a captured snapshot kills both rejoin tests while all 3 pre-existing provider-state tests still pass — which is the gap itself. Docs corrected against their real mechanisms: - `attached`/`unhealthy`/`not_attached` now state the gateway's actual rule (adapter `status == "active"` is part of the predicate; a configured adapter in `error` is `unhealthy` with no failed health check), plus the best-of adapter fold that keeps a Slack-only Channel `attached`. - `ready()` no longer promises it rejects on `error`. It awaits activation, so it can resolve while `status().overall === "error"` from an `unhealthy` provider. Says that instead. - Notes the legacy `SETUP_REQUIRED` path reports a provider condition on the transport leg (dead, cosmetic, left rather than guessed at), and why the provider-state set is duplicated across the duck-typed package seam. Verified: channels-intelligence 199/199, runtime 1874/1874, both builds clean, oxfmt/oxlint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ran Shem Tov
|
ab94c1315e |
fix(showcase): let the Mastra MCP Apps agent self-correct a rejected diagram
Switching models only moved the failure rate around, it never removed it, so
stop relying on the model getting hand-escaped JSON right on the first try.
`create_view` takes `elements` as a stringified JSON array. When the model
appends a stray `}` past the closing `]`, the MCP server rejects the call and
names the exact fault ("Invalid JSON in elements: Unexpected non-whitespace
character after JSON at position N"). That error already comes back as a tool
result, and the agent had no step cap, so a retry was mechanically possible
all along. What blocked it was our own prompt: "Call create_view ONCE" and
"do NOT iterate, do NOT make multiple calls. Ship on the first shot."
The prompt now tells the model to read the error and try again, capped at 2
corrections (3 calls total), with stopWhen: stepCountIs(6) bounding the loop
if it never converges. This mirrors the validate-then-retry recovery pattern
already used for A2UI on the other integrations.
Validated against the real Excalidraw MCP server, using the agent's prompt
extracted verbatim from this file and the real tool schema:
normal runs 12/12 succeeded, all on the first call
attempt 1 force-corrupted with
the real-world stray `}` 10/10 recovered on the second call
Also verified in the running app (local dev server, real key): valid JSON,
isError false, diagram rendered.
Not yet verified in-app: the recovery path itself. No natural failure occurred
during the in-app runs, so the retry is proven at the API level rather than
through the Mastra agent loop.
|
||
|
Ran Shemtov
|
701b03ab96 | Merge branch 'main' into claude/jolly-boyd-38b55c | ||
|
Benjamin Taylor
|
b77ebb435f |
chore: stop changeset files from reappearing in PRs
The repo migrated off @changesets/* to conventional-commit-driven releases (scripts/release/ reads commit subjects from git log <lastTag>..HEAD), but .changeset/ has been removed twice already ( |
||
|
Ran Shemtov
|
8f24b0373b | Merge branch 'main' into claude/framework-d6-integration-validate-7be45e |