mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
codex/channel-components-v2
14751 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maxim
|
43df6e5afd | Merge remote-tracking branch 'origin/main' into feat/reskinnable-demo-lock-skin | ||
|
Maxim
|
964f7c784c |
docs(reskinnable-demo): name demo-beats.md in the README's reskin-skill pointer
The README described the reskin skill as "(SKILL.md + templates.md)". The skill
has THREE canonical files — demo-beats.md is the read-first one, and both
SKILL.md and CLAUDE.md say so ("Write the beat map before you write code"). A
reader following the README alone never learns it exists, and a skin authored
without mapping its beats first has to be rebuilt, because the beats decide the
tools, pages and pills.
Surfaced by the post-convergence promotion audit, which proposed it as
PROMOTE_TO_A on the grounds that this PR introduced demo-beats.md and thereby
made the README claim newly wrong. That premise is FALSE and was refuted before
acting: demo-beats.md is absent from this PR's diff (only SKILL.md and
templates.md are modified) and already exists at the merge-base, and README:75-76
falls between this PR's hunks. The omission predates this branch.
Fixed anyway rather than escalated: the gap is real, the correction is one
sentence, and Procedure 3 sanctions "or fix it" as a resolution. Recorded as a
refuted-premise doc fix, NOT a promotion-driven reopen — the loop stays
converged.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
8955701ff2 |
fix(reskinnable-demo): scope nav-target lint selectors to navigation objects
The NAV_TARGET_ANCESTORS selectors matched by method name only (.push/.replace/.assign on any object), so String.prototype.replace, Object.assign, and Array.prototype.push with slash-containing templates false-positived as broken in-skin navigation. Pin each call form to its object (router.push/replace, location.assign, window.location.assign); leave the JSX href and location.href assignment ancestors unchanged. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
0549bbcd61 |
fix(reskinnable-demo): scope the //-concat lint guard to navigation targets
The interpolationThenSlash selector fired on the bare AST shape "interpolation
then a quasi opening with /", which is identical to an ordinary date
`${month}/${day}` or ratio `${used}/${total} used`. Any future skin component
formatting a date or fraction would have been blocked with a link error that
makes no sense for that code (verified by probe).
Narrow the selector to fire only when the template is an actual navigation
target: router.push/replace, location.assign, location.href, or a JSX href
attribute (ESLint ancestry). Literal-prefix guards (literalSkinPrefix,
templateLeadingPrefix) are unchanged — they never false-positived and cover the
prefix shapes regardless of use site.
Residual limitation documented plainly in the config, SKILL.md, and CLAUDE.md: a
URL assembled into a variable first and then passed to router.push(u) is not
caught by an ancestry-scoped selector.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
384f5f0c23 |
docs(reskinnable-demo): name keel's real brand in the skin lists
The README and CLAUDE.md skin bullets put "Harbor Point Health" in the brand slot for keel, but keel's brand is "Keel" (Harbor Point Health is the tagline's healthcare org). The three sibling bullets quote their real brands (Northwind Finance, Meridian, Aeronova); keel now matches, with Harbor Point Health kept as the org descriptor. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Maxim
|
9148fa2e54 |
fix(reskinnable-demo): enforce LOCK_SKIN URL contract via ESLint AST, retire the regex scanner
The URL-contract drift guard scanned skin source as raw text with regexes — a
re-implementation of a fragment of a JS parser that produced a mandatory review
finding three rounds running, each a different hole (missed spellings, a header
out of sync with its detectors, an unescaped `$` var name spliced into `new
RegExp`, and comment-stripping that both false-tripped on a trailing example
path and over-stripped inside strings).
Replace it with `no-restricted-syntax` selectors in eslint.config.mjs, scoped to
`src/skins/**`:
- (i) literal skin-id prefix — `"/banking/cards"`, `` `/keel/runs/${id}` ``
- (ii) interpolation immediately followed by `/` — `` `${base}/charges` `` (the
`//` that shipped); scoped OFF for the REST/data layer (`actions.ts`,
`intelligence/**`) whose `` `${apiBase}/…` `` targets a server URL the
lock never rewrites
- (iii) leading-slash interpolation — `` `/${skin.id}/…` ``
Each selector names useSkinHref / the skin's own helper and points at
src/shell/skin-path.ts. The AST rule ignores comments/prose and is immune to a
`$` in a variable name. Skin tests are exempt (they assert unlocked, prefixed
hrefs by design).
Delete src/shell/skin-path.drift.test.ts — one mechanism, not two. Point the
reskin skill (verification step 7 + URL-contract section) and CLAUDE.md at
`pnpm lint` and the ESLint rule instead of `pnpm test:unit` and the drift test.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
8311d4d412 |
fix(reskinnable-demo): drop the URL prefix for the LOCKED skin, not any lock
`useSkinHref(skinId)` computed its base as `locked ? "" : `/${skinId}``,
testing whether ANY skin is locked rather than whether the CALLER's skin is
the locked one. Under `LOCK_SKIN=banking`, `useSkinHref("airline")("trips")`
returned `/trips` — a banking URL — silently discarding the `skinId` argument
and pointing the caller at the wrong app. Correct only by an invariant held
OUTSIDE the function (the locked deploy 404s every non-locked skin before it
mounts, and the one cross-skin link bypasses this hook).
Make it correct by construction: `locked === skinId ? "" : `/${skinId}``.
The prefix is dropped only for the skin that is actually locked.
Call-site enumeration (Procedure 2 step 8) — every `useSkinHref(` /
`useKeelHref(` caller and why the change is behaviour-preserving for it. In
every case the caller passes its OWN skin id, and a skin's layout/pages/tools
only render when that skin is active; under a lock the only skin that mounts
IS the locked one, so `skinId === locked` there and `locked === skinId`
reduces to the old `locked` truthiness. Equivalent everywhere:
src/skins/keel/href.ts:25 useSkinHref(KEEL_ID="keel") — wrapped by
useKeelHref(); consumed by keel/tools.tsx, layout.tsx, run-timeline,
approval-card, playbook-card, pages/{knowledge,desk,document,playbooks,
runs}. All render only under the keel skin ⇒ passes "keel"; under a lock
that lock is "keel". Unchanged.
src/skins/banking/tools.tsx:111 useSkinHref(skin.id="banking"). Banking-
only render. Unchanged.
src/skins/banking/layout.tsx:126 useSkinHref(skin.id="banking"). Banking-
only render. Unchanged.
src/skins/airline/layout.tsx:30 useSkinHref(skin.id="airline"). Airline-
only render. Unchanged.
src/skins/logistics/layout.tsx:25 useSkinHref(skin.id="logistics").
Logistics-only render. Unchanged.
Non-callers, for completeness:
src/shell/layout/selector-card.tsx the sole cross-skin link; deliberately
bypasses this hook and builds `/${skin.id}` directly (line 126). Never
exercised the buggy branch — unaffected.
src/skins/banking/nav-target.test.tsx:14,19 probes with skinId="banking"
under lock null or "banking"; `locked === "banking"` matches old `locked`.
Unchanged.
Test: added a covering case in skin-path.test.tsx asserting that under
`LOCK_SKIN=banking`, `useSkinHref("airline")("trips")` still returns the
PREFIXED `/airline/trips`. Verified red against the old one-line impl
(returned `/trips`), green after. Doc comment restated: the prefix is dropped
for the locked skin specifically, not "under a lock" for any skin.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Mark
|
dc3681b106 | Merge branch 'main' into chore/showcase-pydantic-ai-v2 | ||
|
Ben Taylor
|
291cd32832 |
chore: stop changeset files from reappearing in PRs (#6406)
## What does this PR do? Community PRs keep arriving with `.changeset/*.md` files even though the repo migrated off Changesets to conventional-commit-driven releases. `.changeset/` has now been deleted from `main` twice (`5afa55f067` on 2026-06-16, `1e5ba689e0` on 2026-07-29) and **five open PRs carry changeset files today** (#6287, #6289, #6290, #6292, #6346). Three mechanisms keep feeding it: 1. **Stale forks.** `rodboev/CopilotKit`'s default branch still contains 10 of the pre-cleanup `.changeset/*.md` debris files. Three of the five open PRs come from that fork — the contributor's agent opens the repo, sees a directory full of changesets, and adds one more. (No `config.json`, and `@changesets/cli` isn't installed anywhere, so these are hand-written by agents, not CLI output.) 2. **Merging stale PRs re-seeds `main`.** The two files Tyler removed in `1e5ba689e0` arrived via 2026-06-10-authored branches (#2910, #5360) merged on 2026-07-25 — they sat on `main` for four days, and anyone who forked in that window inherited the directory. His hunch in that commit message was right. 3. **Convention inference, uncontradicted.** #6346 is from a branch in this repo, where `.changeset/` does *not* exist, and it still has one. The repo reads as a Changesets repo: pnpm workspace monorepo, per-package `CHANGELOG.md` in Changesets' exact `### Patch Changes` output format, `chore: release monorepo vX.Y.Z` release PRs. Nothing in `CONTRIBUTING.md`, the PR template, `AGENTS.md`, `CLAUDE.md`, or `.claude/docs/` said otherwise, so the guess was well-supported. This PR closes all three off: - **`CONTRIBUTING.md`** — new "Changelogs and releases — do not add a changeset" section: we did use Changesets, `scripts/release/` now builds changelogs from commit subjects, `.changeset/*.md` is inert, write a good conventional commit subject instead, and leave versions/changelogs to maintainers. Includes a note to rebase old forks. - **`AGENTS.md` / `CLAUDE.md`** — the same rule as an Essentials bullet. This is the highest-leverage change: the contributors doing this are coding agents, and agents load these files automatically while mostly not reading `CONTRIBUTING.md`. - **`static / check binaries`** — fail the PR on added `.changeset/*` files, so this stops depending on review catching it (which is what failed in July and restarted the loop). Added to the existing forbidden-files gate rather than a new workflow: it already runs on every PR to `main`, is fork-safe (`contents: read`, no secrets), and has exactly this `git diff --name-only origin/BASE...HEAD` + `VIOLATIONS` shape. Filters on `--diff-filter=AM` so a PR that *deletes* stale changesets still passes. - **`.oxfmtrc.json`** — drop the ignore entry for `.github/actions/changesets-action/src/run.ts`, a path that hasn't existed for a long time. It was the last grep-visible "we use changesets" signal in a root config file. ## Related PRs and Issues - Follows up `1e5ba689e0` ("fix: remove all changesets"), whose commit message asked for exactly this: a durable record of the decision that future agents can find. - Open PRs that would be caught by the new gate: #6287, #6289, #6290, #6292, #6346. ## Testing Docs + CI-config change, so verification focused on the guard. `actionlint` was run on the workflow, then the step body was extracted with `yq` and executed against real branches. **Lint / parse:** ``` $ actionlint .github/workflows/static_check-binaries.yml actionlint: clean $ python3 -c "import json; json.load(open('.oxfmtrc.json'))" # oxfmtrc still valid JSON oxfmtrc JSON OK ``` **True positive** — real head of #6292, via `yq '.jobs.check-binaries.steps[1].run'` piped to bash with `BASE_REF=main`: ``` ::error::Changeset files detected in PR: .changeset/enable-mcp-apps-tool-filters.md This repo no longer uses Changesets — releases are driven by conventional commit subjects (see scripts/release/). Nothing reads .changeset/*.md. Delete these files and describe the change in your commit subject instead. See the 'Changelogs and releases' section of CONTRIBUTING.md. This PR contains files that should not be committed (see the errors above). Please remove them and update your .gitignore if needed. exit=1 ``` **True negative** — same script on this branch, which has five changed files and no changesets: ``` $ git diff --name-only origin/main...HEAD .github/workflows/static_check-binaries.yml .oxfmtrc.json AGENTS.md CLAUDE.md CONTRIBUTING.md $ BASE_REF=main bash step.sh No binary artifacts or oversized files detected. exit=0 ``` **Delete-safety** — a commit that *removes* changesets must not be punished. Using the real cleanup commit (`8806f668d1...1e5ba689e0`): ``` unfiltered: with --diff-filter=AM (what the gate uses): .changeset/coalesce-...md (empty) .changeset/fix-parallel-...md ``` Not verified locally: the gate firing in real GitHub Actions — that needs this PR's own CI run (the `static / check binaries` check on this PR exercises the true-negative path). 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Ben Taylor
|
c6d59c529f |
feat(telemetry): emit telemetry-registry fragments for runtime + docs surfaces (#5891)
## What Adds the CopilotKit side of the [telemetry event registry](https://github.com/CopilotKit/oss-path-to-production/blob/main/docs/telemetry-registry-publish-roadmap.md): tooling + CI that generate this repo's registry **fragments** and open path-limited PRs into `CopilotKit/oss-path-to-production`, where the reconciler folds them into `telemetry-events.json`. Two surfaces, two mechanisms (per the surface-owns-its-extractor design): | Surface | Events | Extraction | Trigger | |---|---|---|---| | **runtime** | 5 `oss.runtime.*` | **bespoke catalog** — reads the `AnalyticsEvents` type map (names + properties), scans `capture()` sites for `call_sites`; **fails loud if the v1/v2 catalogs diverge** | stable **monorepo** release (`on: release`, tag `vX.Y.Z`) | | **docs** (`showcase/shell-docs`) | 11 | **callee mode** — inline `posthog.capture("name", {…})` literals; drops `$`-reserved events | push to `main` touching `showcase/shell-docs/src/**` (excluding `src/content`) | ## Key properties - **Content-gated.** The emitter leaves the target fragment byte-for-byte untouched when the extracted event set is unchanged, so a PR opens **only when telemetry actually changes** — no per-release / per-commit churn. - **Reconciled canonical in every PR.** Both workflows run the registry's `pnpm reconcile` and commit `telemetry-events.json` alongside the fragment, matching the registry's shipped emitters — a fragment-only PR fails its `telemetry-reconcile` staleness gate. - **Least-privilege cross-repo token.** No explicit `owner` (defaults to the app installation's org) + bare `repositories: oss-path-to-production` + `contents`/`pull-requests` write only; mint gated on a job-level env var (GitHub rejects `secrets.*` in `if:`). - **zizmor clean** at CI's `--min-severity low` (one `cache-poisoning` suppression, justified in `.github/zizmor.yml`: the workflow configures no cache and publishes a PR, not build artifacts). ## Files - `scripts/telemetry/extract.ts` — pure extraction (callee scan + catalog reader), deterministic output. - `scripts/telemetry/emit-fragment.ts` — CLI: `--surface runtime|docs --out <path>`, assembles + content-gates the fragment. - `scripts/__tests__/telemetry-fragment.test.ts` — 13 unit tests (fixtures) + a loose real-catalog drift smoke test. - `.github/workflows/telemetry-{runtime,docs}-fragment.yml` — the two CI jobs. ## Testing Rebased onto `main` (`55aaad21a6`) and revalidated end-to-end on 2026-08-05 — the branch had fallen 1345 commits behind. **Unit / static** - `vitest run scripts/__tests__/telemetry-fragment.test.ts` → **13/13 passed**. - `tsc --noEmit --strict --esModuleInterop` over both scripts → **clean** (`scripts/` has no tsconfig, so this is the ad-hoc invocation). - `oxlint scripts/telemetry` → **0 warnings, 0 errors**; `oxfmt --check` → **all files correctly formatted**. - `zizmor --min-severity low --config .github/zizmor.yml .github/workflows` (CI's exact invocation) → **No findings to report** (32 ignored, 233 suppressed). **Runtime surface — mechanism proven against the live rebased tree** ``` $ tsx scripts/telemetry/emit-fragment.ts --surface runtime --out /tmp/CopilotKit.runtime.json runtime: wrote 5 events → /tmp/CopilotKit.runtime.json (released_in runtime@1.66.2) ``` Diffed event-for-event against the registry's committed `CopilotKit.runtime.json`: **semantically identical** (same 5 events, same `call_sites`, same `properties_seen`) — the only difference is ordering, since the emitter sorts alphabetically and the hand-seeded fragment is in catalog-declaration order. Confirmed the reorder is a no-op at the canonical level (see below), so the first automated run opens one reordering PR with an empty `telemetry-events.json` diff and is quiet thereafter. Also confirmed the catalog is still complete on current `main`: the only `oss.*` event literals anywhere under `packages/runtime/src` + `packages/shared/src` are the 5 catalog entries (43/22/12/9/9 occurrences), so no untyped event is being silently dropped. Both v1 and v2 catalogs remain byte-identical, so the divergence guard passes. **Docs surface** ``` $ tsx scripts/telemetry/emit-fragment.ts --surface docs --out /tmp/CopilotKit.docs.json docs: wrote 11 events → /tmp/CopilotKit.docs.json (released_in shell-docs@5855496103) ``` 11 events (up from 7 when this PR was authored — the docs site grew): `cli_command_copied`, `docs_conversion_clicked`, `docs_conversion_copied`, `docs.framework_selected`, `docs.frontend_selected`, `docs.journey_continued`, `hero_command_copied`, `markdown_copied`, `open_in_llm_clicked`, `talk_to_us_clicked`, `try_for_free_clicked`. `$pageview` correctly dropped. **End-to-end against the real registry** Dropped both emitted fragments into a clean `oss-path-to-production@main` worktree and ran its own `pnpm reconcile`: - Both fragments **validate against `fragment.schema.json`** (ajv, via the reconciler's loader). - Reconcile succeeded; `telemetry-events.json` grew by 216 lines with 11 new `"surface": "docs"` observations. - **Zero `oss.runtime.*` entries changed** — confirming the runtime fragment's reordering has no canonical effect. ## Fixed during revalidation - **`add-paths` bug in the docs workflow (would have failed on first run).** It ran `pnpm reconcile` but listed only the fragment in `add-paths`, so its PR would have landed a fresh fragment beside a stale `telemetry-events.json` and tripped the registry's `telemetry-reconcile` staleness gate — the exact failure the runtime workflow was already fixed for. Verified against the registry's shipped emitters: every automated fragment PR there (`website.corp` #232/#220, Intelligence surfaces #228) carries `telemetry-events.json` alongside its fragment. - **Stale action pins.** Refreshed to the SHAs `main` now uses everywhere: `actions/checkout` v7, `actions/setup-node` v7.0.0, `pnpm/action-setup` v6.0.10. - **Over-broad docs trigger.** Narrowed from `showcase/shell-docs/**` to the code under `src/**`, excluding `src/content/**` — 1012 MDX + 140 JSON prose files with zero `.ts`/`.tsx`, none of which can hold a `posthog.capture` call site. Prose edits no longer fire a full monorepo install. - **zizmor justification accuracy.** `setup-node` v7 adds a `package-manager-cache` input defaulting to `true`; per its `action.yml` it engages only when `package.json` declares **npm**, and this repo declares pnpm — so the workflow is still cacheless and the suppression still holds. Noted inline. ## Prerequisite — now satisfied The registry App secrets (`TELEMETRY_REGISTRY_APP_ID`, `TELEMETRY_REGISTRY_APP_PRIVATE_KEY`) are configured on this repo (added 2026-07-09), and `app/copilotkit-telemetry-bot` is demonstrably installed on `oss-path-to-production` — it has been opening fragment PRs there from other surfaces (#232, #228, #220). No further setup needed. ## Not in this PR - The registry-side seed of the **docs** surface. The docs fragment first appears via this workflow's initial run, which now also carries the reconciled canonical, so it lands green. - The **web-inspector** surface, hand-seeded in the registry since this PR was authored, remains manual. Automating it is a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Tyler Slaton
|
00969e323e |
chore: release channels v0.8.0 (#6419)
## Release channels v0.8.0 **Scope:** `channels` | **Bump:** `minor` --- ### How this release process works 1. **This PR was created automatically** by the "release / create-pr" workflow. It bumped the `channels` packages to `0.8.0` and generated AI-enhanced release notes. 2. **CI runs on this PR** — the full test suite (unit tests, lint, type checks, build) must pass before merging. This is the review gate. 3. **Review the release notes** in `release-notes.md` in this PR. If a Notion draft was created, you can edit the release notes there before merging. 4. **When this PR is merged**, the `release / publish` workflow automatically: - Builds all packages - Publishes the `channels` packages to npm at version `0.8.0` - Creates git tag `channels/v0.8.0` - Creates a GitHub Release with the final release notes ### Before merging - [ ] CI is green (tests, lint, types, build) - [ ] Version bumps look correct - [ ] Release notes are accurate (edit in Notion if a draft was created) --- > **Do not merge until CI is fully green.** The full test suite runs automatically on this PR.channels/v0.8.0 |
||
|
tylerslaton
|
289ae4a539 | chore: release channels v0.8.0 | ||
|
renovate[bot]
|
df6be1876c |
chore(deps): update dorny/paths-filter action to v4.0.3 (#6393)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [dorny/paths-filter](https://redirect.github.com/dorny/paths-filter) | action | patch | `v4.0.2` → `v4.0.3` | --- ### Release Notes <details> <summary>dorny/paths-filter (dorny/paths-filter)</summary> ### [`v4.0.3`](https://redirect.github.com/dorny/paths-filter/blob/HEAD/CHANGELOG.md#v403) [Compare Source](https://redirect.github.com/dorny/paths-filter/compare/v4.0.2...v4.0.3) - [Document safe handling of file list outputs in workflows](https://redirect.github.com/dorny/paths-filter/pull/326) - [Escape multi-line filenames in list-files shell and csv output](https://redirect.github.com/advisories/GHSA-7hc6-8hq5-9q2m) - [Add 'some-with-excludes' predicate quantifier](https://redirect.github.com/dorny/paths-filter/pull/322) - [Add contents permission to PR example](https://redirect.github.com/dorny/paths-filter/pull/248) - [Scope base-ignored warning to API path](https://redirect.github.com/dorny/paths-filter/pull/319) - [Update outputs in readme to account for the 'every' predicate-quantifier](https://redirect.github.com/dorny/paths-filter/pull/247) </details> --- ### Configuration 📅 **Schedule**: (in timezone America/Los_Angeles) - Branch creation - "before 9am every weekday" - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/CopilotKit/CopilotKit). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xMi4wIiwidXBkYXRlZEluVmVyIjoiNDQuMTIuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> |
||
|
Maxim
|
703283c84a |
test(reskinnable-demo): decouple LOCK_SKIN nav guard from admin-gated /team
The vacuity precondition in locked-skin.spec.ts required the banking nav to render /, /dashboard, /charges AND /team. But /team is admin-gated in the banking layout (rendered only when currentUser.role === MemberRole.Admin), and the default user is team[0] from the seed (Alex Morgan, Admin). That silently coupled the LOCK_SKIN prefix guard to seed order and the default user's role — a reorder or role flip would fail the suite on an assertion unrelated to LOCK_SKIN. Require only the role-independent targets (/, /dashboard, /charges) as the vacuity guard, and document why /team must not be re-added. The /team route stays covered role-independently by the cold deep-page load test. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
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 |