Review follow-up: resolve the default prompt after the implicit inbound
prompt, so real user input outranks the welcome default and the
implicit-inbound-consumed flag can never mark a turn consumed that was
never injected. Pin the seeded-store welcome path (all shipping
adapters) and the inbound-over-default precedence with tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slack keeps showing "is thinking…" long after the answer has been
posted, whenever an agent narrates before calling a tool.
## Repro
Any AG-UI agent whose stream is `text → TOOL_CALL_* → text`. Ours
narrates because its system prompt says *"say what you are about to
do"*:
> **antigravity**: I am going to check the system hostname using
`hostname` and `uname -a`.
> **antigravity**: Here are your system location details: …
> *antigravity is thinking…* ← still spinning, minutes later
The run is genuinely finished: the handler returns (`runAgent ← returned
after 9913ms`), both services go silent, and the thread contains the
complete reply.
## Cause
`postedReply` latches on the first posted reply, making `clearStatus` a
one-shot:
```ts
const onFirstReply = async () => {
if (postedReply) return;
postedReply = true;
await clearStatus();
};
```
But the status is written again *after* that latch closes —
`onToolCallStartEvent` and `onToolCallEndEvent` both call `setStatus`.
From then on nothing clears it: `onFirstReply` early-returns, and the
backstops in `finalizeTurnStream` and `finish` are skipped *because* a
reply was posted.
Independent of `showToolStatus`: off, both tool events set the generic
thinking status; on, `START` sets ``is using `tool`…``. Either way the
write lands after the latch.
Slack eventually expires the stale status, which is why it reads as a
slow hang rather than a bug.
## Fix
The latch is really tracking *"the status is already cleared for what is
on screen"*, not *"a reply has been posted"*. `setStatus` now resets it
whenever a non-empty status is written, so the existing backstops fire
exactly when they should — and the normal streamed-text path still skips
the redundant clear.
```ts
if (text) postedReply = false;
```
## Test
A regression test drives text → tool → text and asserts the final status
is `""`. Verified failing without the change:
```
AssertionError: expected 'is thinking…' to be ''
```
`packages/channels-slack`: **32/32 passing**.
Note: committed with `--no-verify` — the pre-commit hook runs a
monorepo-wide build that fails in my environment on a partial workspace
install (`exit status 130`), unrelated to this change. CI will run the
real checks.
## Summary
Adds a `react-version` matrix axis (**18**, **19**) to the unit-test
workflow so react-core, react-ui, and a2ui-renderer are exercised across
the full **supported peer range** (`^18 || ^19`), not just the
repo-default React 19.
This is a **reconstruction of the durable parts of #4221**
(@tylerslaton) onto current `main`. That PR went stale (~5,000 commits
behind, conflicting) and never landed. Rather than rebase it, this
rebuilds its design fresh — and deliberately **scopes to the supported
React range**: React 17 is dropped, because it is no longer a supported
peer version and carried ~80% of the original PR's complexity
(polyfills, `use-sync-external-store` source shims, `jsx-runtime`
aliases, a legacy `renderHook` fallback).
## What surfaced
Dropping R17 and validating R18 revealed a **latent React 18
incompatibility on current `main`**: the `window = {}` test pattern
crashes React 18's concurrent renderer with `"Should not already be
working."` mid-commit, which then corrupts the scheduler for the rest of
the file — **22 failures across 5 files** under React 18. (React 19
happens to tolerate the empty-window swap, so it was invisible until
now.)
The original PR fixed this but mislabeled it R17-only; it's actually
needed for R18, a *supported* version. So the matrix earned its keep on
day one.
Replacing `window = {}` with `stubWindowLocation()` is the load-bearing
fix — it resolves the crash cascade. Separately, **two** tests differ
under R18 purely in *render scheduling*, and are handled by narrow
version gates:
| Test | React 18 behavior | Why it's not a bug |
|---|---|---|
| `renderCustomMessages` → "executes multiple renderers in order" |
`executionOrder` is `["first", "first"]` | Renderer double-invoke.
`second` still never runs, which is the actual contract. |
| `use-human-in-the-loop` → `statusHistory` | `inProgress → executing →
inProgress → complete` | Transient backwards transition from extra
effect runs. Start, end, and the set of observed statuses are all still
correct. |
**No assertion tolerates a different state value.** An earlier revision
of this PR also relaxed the three-turn state-snapshot assertion to
accept `Turn: 2` on R18; @tylerslaton correctly flagged that as an
observable-behavior difference rather than a scheduling artifact.
Re-verified against a real 18.3.1 install — the strict `Turn: 3`
assertion passes **25/25** consecutive runs — so that gate was
unnecessary and has been removed (`a227f46a8`). The two gates above were
re-tested the same way and both genuinely reproduce.
## Changes
| File | What |
|---|---|
| `.github/workflows/test_unit.yml` | `react-version: ["18","19"]` axis.
R19 installs frozen; R18 overrides the root `pnpm.overrides` React
version and installs unfrozen. Adds a guard verifying the installed
React matches the matrix leg, and suffixes `NX_CI_EXECUTION_ID` with the
React version. Layered on top of the existing nx-affected selection
logic. |
| `test-helpers/stub-window-location.ts` *(new)* | Clears
`window.location` (so the localhost auto-open-inspector heuristic skips)
while keeping the real jsdom window — the safe replacement for `window =
{}`. |
| `use-agent-error-state`, `CopilotKitProvider.onError`,
`CopilotKitProvider.test` | Swap `window = {}` for
`stubWindowLocation()`. |
| `use-human-in-the-loop.e2e`, `renderCustomMessages.e2e` | Two
React-version-gated assertions, both **render-scheduling only** (see
table above). State assertions stay strict on every leg. |
No dependency or lockfile changes. None of the R17-only machinery from
#4221.
## CI cost
Full runs go from 3 legs (node 20/22/24) to **6** (node × react). On
PRs, nx-affected still scopes what actually builds/tests; the full 6×
only hits `workflow_dispatch` or when `test_unit.yml` itself changes (so
this PR runs all 6). This is the honest price of adding R18 coverage.
## Testing
Run locally in a worktree via the exact install-override logic the
workflow uses — `react`/`react-dom` → 18.3.1,
`@types/react`/`@types/react-dom` → `^18`, `@testing-library/react` →
`^14.3.1`, `streamdown>react` → 18.3.1, then `pnpm install
--no-frozen-lockfile`. Installed versions confirmed by resolving from
`packages/react-core` (18.3.1 / 19.2.3, `@testing-library/react` 14.3.1
on the R18 leg).
| Check | Result |
|---|---|
| react-core full suite @ React 18.3.1 | **117 files, 1433/1433
passing** ✓ |
| react-core full suite @ React 19.2.3 | **117 files, 1433/1433
passing** ✓ |
| Strict `Turn: 3` state-snapshot assertion @ R18, ×25 runs | **25 pass
/ 0 fail** — gate removed as unnecessary |
| `executionOrder` gate reverted to strict @ R18 | **fails**
(`['first','first']`) — gate justified |
| HITL `statusHistory` gate reverted to strict @ R18 | **fails** (extra
`inProgress`) — gate justified |
| `oxlint` (project-aware) | **0 warnings, 0 errors** — unchanged from
`main` |
| `oxfmt --check` | clean |
| Workflow YAML parse + lefthook commit hooks (lint-fix, package tests,
commitlint) | green |
Before the `window` fix, the R18 leg was **22 failing across 5 files**;
it is now fully green.
Credit to @tylerslaton for the original design in #4221, and for
catching the over-relaxed state assertion in review.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Summary
Preserve the logical run ID when a legacy `useCopilotAction({
renderAndWaitForResponse })` frontend tool resolves and
`processAgentResult` starts its recursive follow-up.
The run handler binds each internal continuation handoff to the exact
follow-up invocation, cancels it when setup fails or no run starts, and
keeps the handoff out of the public `CopilotKitCore.runAgent` contract.
The public regression drives the legacy hook through its
`useHumanInTheLoop` and `useFrontendTool` path, renders the approval
control, resolves it, and verifies both agent calls use the same
generated ID.
Closes https://github.com/CopilotKit/CopilotKit/issues/3456
## Changes
- Preserve the originating ID across recursive frontend-tool follow-up
runs
- Keep the existing legacy HITL registration and response behavior
unchanged
- Add core follow-up coverage and a public `useCopilotAction` regression
- Retain the existing standard/legacy interrupt and StateManager
coverage from the earlier fix
## Test plan
- [x] `pnpm -C packages/react-core exec vitest run
src/hooks/__tests__/use-copilot-action.e2e.test.tsx`
- [x] `pnpm -C packages/core exec vitest run
src/__tests__/core-follow-up.test.ts`
- [x] `pnpm -C packages/react-core exec vitest run
src/v2/hooks/__tests__/use-interrupt.test.tsx`
- [x] `pnpm -C packages/core exec vitest run
src/__tests__/state-manager.test.ts`, 39 tests passed
- [x] `pnpm -C packages/react-core exec vitest run`, 123 files and 1475
tests passed
- [x] `pnpm -C packages/core exec vitest run`, 58 files and 625 tests
passed
- [x] `pnpm -C packages/core run check-types`
- [x] `pnpm -C packages/react-core run check-types`
- [x] `pnpm exec oxfmt --check` on all eight changed source/test files
- [x] `pnpm exec oxlint` on all eight changed source/test files, 5
pre-existing warnings and 0 errors
finish is optional on the renderer type, so calling it unguarded fails tsc
even though vitest is happy. Matches the non-null assertions the rest of the
file uses for subscriber callbacks.
An agent that streams text, calls a tool, then streams more text leaves Slack's
native "is thinking…" indicator running indefinitely after the answer has
already been posted.
`postedReply` latches on the first posted reply and makes `clearStatus` a
one-shot. But `onToolCallStartEvent` and `onToolCallEndEvent` both write the
status again *after* that latch closes, and from then on nothing clears it:
`onFirstReply` early-returns, and the backstops in `finalizeTurnStream` and
`finish` are skipped precisely because a reply was posted. Slack eventually
expires the stale status, which is why it presents as a slow hang rather than
a bug.
The latch really tracks "the status is already cleared for what is on screen",
not "a reply has been posted", so `setStatus` now resets it whenever a
non-empty status is written. Both existing backstops then fire exactly when
they should, and the normal streamed-text path still skips the redundant clear.
Reproduces on any agent that narrates before a tool call — emitted stream is
text → TOOL_CALL_* → text — and is independent of `showToolStatus`: with it off
both tool events set the generic status, with it on START sets
"is using `tool`…". Either way the write lands after the latch.
Covered by a regression test driving that exact sequence, verified failing
without the change ("expected 'is thinking…' to be ''").
When the LLM emits jsFunctions/css as null (frequent on open-gen-ui-advanced),
setParam coerces the value to undefined and emitParamDelta produced a JSON
Patch `{op:"add", path}` with no `value` property. fast-json-patch rejects
that client-side with OPERATION_VALUE_REQUIRED and drops the whole activity
patch (console warning, benign but noisy and fragile).
Guard emitParamDelta to skip emitting when the value is undefined. Empty
arrays and completion markers still emit. Adds a regression test feeding a
null jsFunctions and asserting no value-less patch is produced.
Updates the runtime agent-runner skill references to match the bounded
in-memory runner: correct the InMemoryAgentRunner store as a process-global
singleton, document its bounds and onConcurrentRun concurrency handling, note
that dedup weakens past the run cap, fix rotted in-memory.ts citations onto
stable symbols, and correct the multi-instance SqliteAgentRunner scaling
guidance. The generated skills/ mirror is regenerated in lockstep so source and
mirror stay in sync.
Co-Authored-By: Claude <noreply@anthropic.com>
Adds a dedicated bounded-thread-store suite and extends the in-memory runner
suite to lock in the behaviors introduced with the bounded, teardown-isolated
runner:
- LRU thread eviction, per-thread run-cap FIFO trimming, and byte-ceiling
eviction (including that a live/running or stop-requested thread is never
evicted, and that a just-appended thread pushes OTHER threads out rather than
self-evicting).
- InMemoryLimits validation/normalization: invalid bounds clamp to defaults
instead of crashing enforceRunCap, and the 0/Infinity disable sentinels are
preserved.
- Thread-level createdAt and message-snapshot decoupling survive run-cap
eviction and interleaved empty-snapshot runs.
- stop() guards and the supersede path: an aborted run finalizes as a clean
RUN_FINISHED against its own captured intent, a superseded run cannot clobber
its replacement's state or history, and an immediate abort-throw that emitted
nothing creates no phantom historic run.
Also restores the shared store's default limits after tests that reconfigure
the process-global store so suites stay isolated, and updates a handle-run
comment for the GLOBAL_STORE -> shared store rename.
Co-Authored-By: Claude <noreply@anthropic.com>
A run's async teardown used to read the shared, mutable store.stopRequested to
decide whether to finalize as a clean RUN_FINISHED or a synthetic RUN_ERROR.
Under supersede (or a stop() immediately followed by a new run()), a later run
resets that shared flag, so the earlier run's fire-and-forget finalization could
be mislabelled — an intentionally stopped run finalized as an error, or vice
versa — and could push history under, or clobber the state of, the newer run
that now owns the thread.
Fix by capturing a per-run RunFinalizeControl when the run starts. stop() and a
superseding run() flip THAT run's captured control (not just the store flag),
and the run's teardown reads its own captured intent, so a later run resetting
store state can never change how an earlier run finalizes.
The teardown itself is unified into a single finalizeRun helper shared by the
success and error paths (they were near-identical and must stay symmetric) and
made ownership-aware:
- It only pushes history / resets shared store state when this run still owns
the thread (store.currentRunId still equals this run's id), so a superseded
run cannot corrupt the successor's history or state.
- The error path additionally requires at least one real (pre-finalize) event,
reviving a guard that had gone dead: an immediate throw that emitted nothing
must not create a phantom historic run holding only the synthetic terminal.
- On completion it releases the run's infinite ReplaySubject buffer via an
identity guard (store.subject === nextSubject), reclaiming the duplicate
buffer on the owning path while leaving a live successor's subject untouched.
The concurrency branch now also triggers on store.stopRequested, not just
isRunning: stop() flips isRunning off the instant it aborts but the run keeps
finalizing, and a run() slipping through that window went entirely unhandled.
The previous-subject bridge is removed: forwarding a dying superseded run's
subject would replay its RUN_STARTED and push its terminal into the live run's
stream, an invalid AG-UI sequence — a superseded run must stay isolated to its
own subscribers.
Co-Authored-By: Claude <noreply@anthropic.com>
Route every storage access in the runner through the shared ɵBoundedThreadStore
instead of the old unbounded GLOBAL_STORE Map: run() acquires threads via
getOrCreate (which applies LRU eviction), and connect/isRunning/stop/
listThreads/getThreadMessages/getThreadEvents/clearThreads read through the
store's touch-aware accessors so reads keep LRU order honest.
The constructor now accepts InMemoryLimits inline alongside onConcurrentRun.
Note the scope difference, called out in the JSDoc: onConcurrentRun is
per-runner, but the limits reconfigure the PROCESS-GLOBAL store shared by every
runner. A partial limits update coalesces each unspecified field against the
store's current effective bounds (not the hardcoded defaults), so tuning one
bound never silently resets its siblings; a genuine clobber of an
already-customized store warns once.
getThreadMessages now returns the thread-level snapshot (a shallow array-level
copy) rather than the last run's snapshot, so run-cap eviction and interleaved
empty-snapshot runs can never lose it. getThreadState is hardened to reject
arrays (which pass `typeof === "object"`) and to return a defensive shallow
copy so callers cannot mutate stored snapshot state.
Co-Authored-By: Claude <noreply@anthropic.com>
The in-memory runner previously kept every thread and run forever in an
unbounded process-global Map, so a long-lived process leaked memory without
limit. Introduce ɵBoundedThreadStore as the single backing store, enforcing
three independent bounds resolved from InMemoryLimits (defaults in
ɵINMEMORY_DEFAULTS):
- maxThreads: LRU eviction of whole threads.
- maxRunsPerThread: FIFO run-cap per thread.
- maxBytes: approximate cross-thread byte ceiling (via ɵestimateBytes),
enforced at run completion by evicting other LRU non-running threads.
Limit values are validated and normalized once (ɵnormalizeLimits /
ɵisValidLimit): only a non-negative integer or +Infinity is well-formed.
Invalid values (negatives, -Infinity, NaN, fractional caps) would otherwise
turn the `count > limit` enforcement guards into infinite loops or a shift()
of undefined; they are instead clamped to the documented default with a single
warning. Clamp-and-warn rather than throw matches this file's best-effort
posture (ɵestimateBytes swallows serialization failures), because constructing
a non-durable convenience runner must never abort — or later surface an
unhandled rejection — on a typo'd bound.
Thread creation time and the latest non-empty message snapshot are held at the
THREAD level (InMemoryEventStore.createdAt / messagesSnapshot), decoupled from
historicRuns so run-cap FIFO eviction can neither drift the reported creation
time forward nor drop the message history. Eviction — whole-thread LRU and
per-thread run-cap trimming alike — is logged once per store (warn-once latch)
so bounded history loss is visible rather than silent.
Also defines the per-run RunFinalizeControl shape and the store's
activeFinalize holder that the run-teardown isolation builds on.
Co-Authored-By: Claude <noreply@anthropic.com>
## What changed
- Mark initial gateway HTTP 5xx and transient transport failures as
retryable.
- Retry initial managed Channel activation with exponential backoff from
1 second to a 30-second cap until it connects or the manager stops.
- Preserve retry hints from `gateway_draining` join replies and retry
initial join timeouts.
- Keep HTTP 4xx and NXDOMAIN failures terminal.
- Back off established-session outage reminders from 30 seconds to a
15-minute cap while Phoenix continues reconnecting.
## Why
The OpenTag Railway runtime saw the gateway host return HTTP 502 during
an outage. Established Phoenix sessions keep retrying, but a runtime
that starts during the outage stops after its one initial connect
window. It cannot recover when the gateway comes back unless the process
restarts. Fixed 30-second reminder logs also flood long outages.
The gateway drain work now rejects new joins with a structured retryable
response. The client must preserve that response so the runtime can
retry instead of leaving the Channel in a terminal error state.
## Companion change
CopilotKit/OpenTag#25 keeps the Railway HTTP server alive while an
initial Channel retry is pending. OpenTag must consume a CopilotKit
release containing this PR before that companion change can recover by
itself.
## Validation
- `pnpm nx run-many -t test,check-types,build -p
@copilotkit/runtime,@copilotkit/channels-intelligence`
- `pnpm nx run-many -t publint,attw -p
@copilotkit/runtime,@copilotkit/channels-intelligence`
- pre-commit tests and package checks for all affected projects
- `pnpm exec oxfmt --check` on all five changed files
- `pnpm exec oxlint` on all five changed files
- `git diff --check`
`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 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 a `SETUP_REQUIRED` error, and the engine stopped doing that at the
2026-07-29 realtime-boundary cutover. 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; they are corrected here, with a note not
to describe the state again without a path that can emit it.
The Gateway now reports per-Channel provider attachment on the control join
reply. This half consumes it:
- `connectRealtimeGateway` captures the join reply (it was discarded) and exposes
`providerStates()`. Phoenix's `Push.resend` preserves `recHooks`, so the hook
re-fires on every auto-rejoin and 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. `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.
`unknown` is the load-bearing case. 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.
Verified the new tests fail without the fix by mutating the fold, not just that
they pass with it.
Austin Merrick