mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
worktree-banking-codex-harness
14820 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maxim
|
da69dea5c5 |
fix(reskinnable-demo): stop the beat-6 rule block disabling the LOCK_SKIN guards
Flat-config `rules` options are REPLACED, not merged. The beat-6 block added in
|
||
|
Austin Merrick
|
6001aa48de |
docs: clarify managed organization onboarding (#6447)
## What changed - replace the managed “Create a free account” wording with the hosted organization admission flow - state that Clerk's automatic Free subscription does not count as an explicit Developer choice - document Clerk-native Self-Service Agreement consent for new accounts and no re-consent for existing accounts - explain that old organizations continue while new hosted organizations choose Developer or a paid plan - make the browser return to the CLI or hosted destination explicit before that destination performs project selection - keep customer-run self-hosted setup outside Clerk and this admission gate ## Why Managed onboarding docs must describe the same flow users see in the hosted app and CLI without changing starters, runtime packages, or self-hosted setup. Plan: [New-Organization Admission, Clerk Consent, and Monthly Seat Pricing](https://app.notion.com/p/3b83aa381852818a868acee473e1a07c?pvs=204) ## Clean replacement This PR targets `main` directly and supersedes #6441. It does not depend on #6188 and changes only two managed-onboarding pages, one architecture CTA, and their standalone contract test. ## Validation - managed onboarding contract test: passed - TypeScript: passed - lint: passed with inherited warnings - production docs build: passed; 222 pages rendered - formatting and `git diff --check`: passed The repository-wide test command still has four unrelated baseline failures: three Git LFS PNG pointer checks and one Channels dark-image expectation. This PR does not change those files or checks. |
||
|
Maxim
|
f39a6fb0b4 |
fix(reskinnable-demo): withhold logistics' escalation vocabulary from the agent
Beat 6 claims the agent learns an unknown procedure by watching once. An
agent holding the unlock vocabulary already knows it, clears the gate
unaided, and there is nothing to teach.
Logistics published the catalogue FOUR ways, not the three the plan named:
a useAgentContext readable described as "valid escalation codes", a
z.enum(ESCALATION_CODES) on fileEscalation's schema, that tool's own
description pointing at "the catalogue in your context", and an agent.ts
RULES line listing the codes among what is "provided". Closing three of
four moves the leak rather than closing it, so all four are closed: the
code parameter is now a free z.string() whose describe() states the
withholding, and the prompt names escalation codes as the one thing NOT in
context. The route already refused an uncatalogued code without
enumerating the valid set (audited, unchanged).
The justifying/decoy split and checkAuthority are unchanged, and
escalation-codes.test.ts already covered the predicates, so no test was
added. The guard is instead a new AST no-restricted-syntax rule,
withheldGateVocabulary in eslint.config.mjs, scoped to
src/skins/logistics/tools.tsx: every symptom here is invisible (the app
compiles, type-checks, lints and demos with the readable restored), and the
schema leak was line-wrapped, so a source-text guard would silently never
have matched. It was verified RED (6 errors, including the wrapped :268
enum) before the removals and green after.
docs/teach-mode/verify-logistics-gate.sh proves the gate over pure REST
with no agent: refusal names the symptom and no code, a decoy records and
approves without unlocking, an uncatalogued code is refused without
enumerating, a justifying code lifts the gate. It diverges from the plan's
draft on four audited facts — mitigate requires plannerId and answers 403
OVER_AUTHORITY (not 422 ABOVE_APPROVAL_AUTHORITY); "absorb" always costs
$0 and can never be over authority, so the script discovers the bounded
planner, the exception shipment and the over-authority KIND (expedite)
from the live API; and the refusal legitimately names the generic recovery
path ("file an escalation"), so the leak assertion is that it names no
CODE.
Skill staleness (CLAUDE.md standing rule): YES, the reskin skill was wrong
here and is updated in this commit. SKILL.md and templates.md told every
author to enumerate closed-set parameters with z.enum "so the vocabulary
reaches the model" with no carve-out — the exact advice that produced this
defect; all three sites now carry the beat-6 exception. failure-modes.md
gains § 10 (the five leak channels, why the guard is a lint rule, and the
requirement to append your skin to its files glob), renumbering the old
§ 10 to § 11. demo-beats.md's beat-6 part 3 and SKILL.md's beat-6 walk step
now name the channels and the REST proof.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
1c50e3f977 |
docs(reskinnable-demo): write the attach snippets in the form the hook keeps
The two `@/shell/attach` call-site snippets used an inline `type AttachmentDocument` in the value import. The commit hook's `oxlint --fix` (consistent-type-imports, `stage_fixed: true`) rewrites that form — it did so to all three skin wrappers inside the previous commit — so a skin author copying the snippet verbatim would watch their file change under them. Both snippets now show the separated `import type` the hook leaves alone, and say why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
fce12788c5 |
refactor(reskinnable-demo): stage all three attachments through the shell chain
Each skin's file reduces to a document URL, a filename and a message; the
detection, the bounded waits and the abort-on-failure rule come from
@/shell/attach.
Banking's and people's chains covered only the first rows of the detection
table, so both silently gain the causes they lacked — including the two that
let a prompt go out WITHOUT the file: no wait for the attachment chip, and no
wait for the chip to print the filename while the file is still base64-encoding.
FIXES BANKING'S AND PEOPLE'S UNGATED SEND. Both had their send half in
skin.tsx, not in their attach file, in this shape:
const staged = await stageOfferLetterAttachment();
if (staged) await wait(500); // `staged` gated ONLY the wait
setTextareaValue(textarea, PACKET_MESSAGE);
sendButton?.click(); // ...and sent REGARDLESS
So a failed stage still sent the prompt, the model invented the document's
contents, and the tool filed a plausible-looking artifact off them — the beat
proving the opposite of its claim with nobody in the room able to tell. Four
defects in those ~20 lines, all now handled by the shell: the ungated send, a
fixed 500 ms sleep racing an async encode, a silent `if (!textarea) return`,
and a `click()` that never checked the STOP state (mid-run that click CANCELS
the run instead of sending). None of that behaviour is preserved.
Commerce's `launchBeat3d` wrapper is deleted as redundant, not unfashionable:
both shell entry points are wholly inside their own `try` and report cause
"unexpected" before resolving `false`, so neither can reject.
Commerce's attachment test drops the cause enumeration (26 tests -> 6; the
fifteen causes are driven once in src/shell/attach/stage-attachment.test.ts)
and keeps only what is still commerce's: that the wrapper passes the right URL,
filename and message — asserted against hardcoded literals, since a test that
imports the constant it checks cannot notice the constant changing — and that a
failure reports and does not send. It imports composer selectors from
@/shell/attach/stage-attachment, because the barrel deliberately omits them.
Verified in a REAL browser (fresh dev server, forced-new thread), which the
chain had never had: on /banking, /people and /commerce the pill stages a chip
that PRINTS the filename, the chip is consumed by the send, and the artifact is
filed citing document content — banking's report carries the invoice's five
Meridian line items, people's packet the offer letter's 15 Aug start date and
week-one schedule, commerce's plan the sheet's Net 45 / 30% deposit terms and
the first-time Alder Crewneck quote. With the document route forced to 500 all
three abort: composer never driven, `[attach:http-error]` logged, alert raised,
artifact count unmoved.
Skill staleness (CLAUDE.md standing rule): YES, this made
.claude/skills/reskin/ wrong, and all of it is fixed here. demo-beats.md § 3d
now says the chain is shell-owned and shows the ~10-line call site instead of
instructing the author to build it; its dangling `stageInvoiceAttachment`
reference and its "copy commerce, not banking" caveat are gone, and the
"copy from" table row points at @/shell/attach. failure-modes.md § 4 is
repointed at src/shell/attach/stage-attachment.ts and records that the taxonomy
is now complete (all fifteen causes emitted AND driven, which was not true of
the file it described). templates.md's build-it-by-hand comment block becomes
the call site, and its `launchBeat3d(...)` lines become bare `void`s. Three
facts new to the skill: the `[attach:<cause>]` log tag is load-bearing (17 cases
across 10 blocks parse it by regex), `Beat3dTimings` is injectable so a test
never sleeps a production budget, and NOTHING_SENT_LEDE is overridden in
exactly two places. Stale comments naming `stagePriceSheetAttachment` /
`sendRestockRequestWithPriceSheet` in the price-sheet route and its test are
updated too — one of them claimed the prompt is sent regardless, which is the
behaviour this commit removes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Ran Shem Tov
|
766ac3afd3 | test(showcase): refresh unavailable backend fixture | ||
|
Ran Shem Tov
|
4151807154 | test(showcase): update partner baseline count | ||
|
Murat Sari
|
cbf79ef52a | fix: align Angular 20 support and resolve packed smoke paths | ||
|
Ran Shem Tov
|
79f56d9f8e | fix(showcase): finalize CrewAI D6 on official bridge | ||
|
Austin Merrick
|
18cd9534b9 | fix(docs): link managed onboarding CTAs | ||
|
Maxim
|
310c2f1410 |
test(reskinnable-demo): make the attachment cause taxonomy exhaustive
`observed.size === 15` only counted causes some test DROVE, so a sixteenth union member that no code ever constructs never entered the map, the size stayed 15, and the assertion passed. That is precisely the rot this module was extracted to fix — commerce declared fifteen causes and constructed eight, and nothing could see the difference. `Record<AttachmentFailureCause, true>` is exhaustive by construction, so the expected count now comes FROM THE UNION: a new member is a tsc error until it is listed, and once listed it raises the count until a test drives it. Verified both directions by mutation. Also: the paperclip kept its own lede on the failures it anticipated and then fell back to "nothing was sent" in its catch, contradicting itself depending on which failure it hit — and that catch was the only one of the module's two `unexpected` emissions with no test. Both fixed. `staging-threw` joins the send-path table, which covered seven of the eight ways staging can fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
bd0eb9cf60 |
feat(reskinnable-demo): add the shell attachment staging chain
Every step of staging a file into CopilotKit's composer is a request made of framework code we do not own, so an unobserved step is an assumption. This centralizes all fifteen detection causes, the bounded condition waits, and the rule that any failure ABORTS the send. That last rule is what makes beat 3d honest rather than merely defensive: a chain that lets the prompt go out without the file makes the model invent the document's contents, the tool files the artifact anyway, and the result reads plausibly — so the beat proves the opposite of its claim and nobody in the room can tell. Moved from commerce's `attach-price-sheet.ts` with only three values lifted into parameters — the document's url, its filename, and the pill's message. `reportAttachmentFailure` now takes the cause and tags the log line with it, which is what lets the send path's failures be pinned by tests at all: those entry points return a bare boolean, so the log is the only place the cause is observable. Commerce keeps its own copy until task 7 migrates the skins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
703d649a03 |
refactor(reskinnable-demo): build both PDFs on the shell primitive
Commerce's builder loses its byte layout to @/shell/documents and keeps its
content. People's gains the ASCII fold it never had — a live defect, not a
hypothetical: the seed carries Ines Vidal, Sasha Bergstrom and Montreal
(accented in the data) and the offer-letter route reaches all three, which
produced mojibake AND a /Length that disagreed with the byte count.
Commerce's duplicated byte-level assertions move to the shell's test, where
they cover every skin once.
toAscii now transliterates instead of blanking. The fold Task 4 moved verbatim
covered punctuation only, so every accented LETTER became "?" — the price sheet
printed "?MILE & FILS" and "Cr?me Br?l?e Tee", and commerce's suite asserted
those strings, pinning the defect as though it were the spec. It now
NFD-normalizes and drops combining marks, keeping a visible "?" only where
there is no ASCII base (CJK, currency symbols): a dropped character is a silent
corruption, a "?" is a legible one. The class is \p{M}, NOT the \p{Diacritic}
the plan proposed — Diacritic also covers ASCII "^" and "`" (deleted out of
ordinary prose) and U+00B7 MIDDLE DOT, which the fold map deliberately turns
into "-". This deliberately breaks the byte-identity Task 4 established; that
claim existed to prove the move was faithful, and it did.
Two audited differences change the offer letter's rendered page, both accepted
because one shared primitive is the point of the extraction:
- Margin. People used MARGIN = 64, the shell is fixed at commerce's 58, so
the letter's text now starts 6pt further left and 6pt higher.
- Body type. People defaulted to 11pt with 4pt extra leading; the shell
defaults to 10.5pt with 3.5pt. Unlisted in the plan, found while migrating.
Parameterizing either would hand every caller a knob nobody asked for.
The letter's page dictionary now declares Courier /F3 and /F4 without
referencing them. Left alone: declared-but-unreferenced is inert, whereas
referenced-but-undeclared renders blank.
Three of commerce's eleven layout cases were deleted, not five: the xref,
/Length and font-resolution cases are pure mechanism and the shell's test now
covers them for every skin. "Draws every columnar line in a monospaced face"
and "keeps every mono line inside the drawable width" STAYED — the shell proves
that a `mono` line comes out in Courier, but only commerce's own test can prove
that commerce marks its rows `mono` and picks column widths that fit. Both
would stay green in the shell while the sheet rendered ragged.
Skill impact (the standing CLAUDE.md question): yes, and fixed here. Six
locations in .claude/skills/reskin/ told a new author to copy commerce's
builder and reimplement both traps — demo-beats.md's copy-from table, its
"Writing a PDF by hand?" block (all four paragraphs, including the one naming
people as the unfixed counter-example) and its alignment paragraph, plus
failure-modes.md's decorative-assertions entry. They now point at
@/shell/documents, describe the traps as centralized, and say which half of
the alignment trap a skin still owns.
Verified: npx tsc --noEmit clean; pnpm lint clean; pnpm vitest run 102 files /
1153 tests (was 101/1151). offer-letter-pdf.test.ts red before the migration
(3 failures, including /Length 1740 vs 1736) and green after. Both documents
rendered and inspected: all four PDFs pure ASCII with correct /Length and xref,
the letter reads "Ines Vidal"/"Sasha Bergstrom"/"Montreal", and the price sheet
for a non-accented vendor is byte-identical to its pre-migration output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
ab00e284d2 |
test(reskinnable-demo): make the shell PDF font and xref guards able to fail
Both cases passed unconditionally. The font test scanned the WHOLE document for /Fn, so it harvested the page's own /Font dictionary and its size check was always 4, then asserted the presence of the substring it had just matched with a \d+ that accepted any object number. It now reads the referenced faces from the content stream alone and follows stream -> /Font dictionary -> emitted font object, checking the BaseFont at the end of the chain. The xref case checked only that startxref landed on the literal "xref", never that an entry pointed at the object it claims. It now walks every entry and checks /Size, per commerce's expectCorrectXref. Both verified by mutation: FIRST_FONT_OBJECT 5 -> 6 (every glyph blank, whole suite otherwise green) and one recorded offset +1 now fail, and only these two cases fail. Also: assert drawableWidth is 496 rather than positive, cover the middot fold, assert %PDF-1.4 with its trailing newline unmasked by trimEnd, and drop a font-set assertion implied by the per-line check above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
478e200a88 |
feat(reskinnable-demo): add the shell PDF document primitive
Extracts commerce's price-sheet builder into a content-agnostic primitive: the toAscii fold, the Helvetica + Courier font dictionary, the /Length and xref arithmetic, and the mono page-fit bound. Callers supply a flat list of lines; a section is a heading line carrying a gap. Two traps this centralizes, both of which produce a VALID PDF that is wrong on screen, and neither of which anything type-checks: a UTF-8 content stream under base-14 WinAnsi fonts (mojibake plus a desynced /Length), and padEnd alignment drawn in a proportional face. A verbatim move: commerce's builder is untouched and keeps its own copy until the migration lands. Byte-identity was verified by running commerce's own content half over both implementations, ASCII and non-ASCII, and comparing the emitted bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
190be901d5 |
refactor(reskinnable-demo): migrate people and commerce onto the shell teach module
Both gain the visible-duration floor, the feed de-dupe and the automatic fresh-window reset they were missing; both lose their manual reset() and their per-skin @keyframes, which the shell's token-driven .recording-vignette replaces. Six copies of the ref-counted state machine are now one. Skill impact: checked. `.claude/skills/reskin/demo-beats.md` item 4 already points at `@/shell/teach` (Task 2), and neither SKILL.md nor templates.md teaches a per-skin recording context or per-skin vignette keyframes, so nothing there went stale. One live doc DID: `docs/teach-mode/README.md`'s role table still named the banking copy Task 2 deleted — repointed at `src/shell/teach/`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
7a45003d37 |
refactor(reskinnable-demo): migrate banking onto the shell teach module
Deletes banking's recording context, feed and vignette in favour of @/shell/teach. noteDemonstratedCode collapses into logStep's second argument, so the feed line and the code that lifted the gate are recorded by one call at the moment the operator files it. Behaviour is unchanged; banking already had the union's stricter half. Two additions beyond a straight import swap, both to keep coverage flat: - banking's recording-context.test.tsx held three cases, and only two were genuinely re-covered by the shell module's own test. The third was a PendingApprovalsChat integration test proving that component's handleApprove brackets in the right order (logStep before beginRecording silently drops the line — it has been wrong once). Moved to components/wow/pending-approvals-chat.test.tsx rather than deleted. - pinned the invariant the API collapse now depends on: the outer bracket held open from "start recording" until "I'm done" is what keeps the derived code alive across the nested bracket the filing opens. The old ref-based noteDemonstratedCode did not need this; getDemonstratedCode does, and nothing tested it. Reskin-skill review (per CLAUDE.md): NOT clean — demo-beats.md item 4 pointed a new skin's author at banking's three deleted files as the pattern to copy. Rewritten to send them to @/shell/teach instead, with the outer-bracket rule and the silent failure modes spelled out. Also refreshed the six HITL-chain line anchors, which were already ~8 lines stale before this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
5c86863aec |
fix(reskinnable-demo): make the recording feed readonly so it type-checks
Object.freeze([]) is readonly never[], and the `as RecordedStep[]` assertion on the frozen no-provider fallback was not a legal widening — TS2352. It went unnoticed because this app has no typecheck script, so `pnpm lint` passing said nothing about it; `npx tsc --noEmit` (~4s) is the cheap authoritative gate. Fixed by changing the contract rather than satisfying the cast: RecordingValue.steps is now `readonly RecordedStep[]`, so the frozen fallback needs no assertion at all. That is also the honest type — consumers only read the feed (.length, .map), and every out-of-provider consumer shares ONE frozen array, so a mutable type invited splicing a singleton out from under all the others. No consumer needed a mutable array: nothing outside the module imports @/shell/teach yet, and none of the three per-skin copies Tasks 2/3 migrate onto this contract mutates its feed in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
a218e326f3 |
fix(reskinnable-demo): pin the recording module's untested properties
Review of the shell-owned recording module found three of its four load-bearing properties had no failing assertion behind them, and one undisclosed change. - getDemonstratedCode is back to the intended [...x].reverse(). It had shipped as x.toReversed() because lefthook's lint-fix runs `oxlint --fix` with stage_fixed and unicorn/no-array-reverse autofixed it into the commit after the last local test run. toReversed() is ES2023 against an ES2017 target with lib:esnext, so it type-checks clean and silently raises the browser floor to Chrome 110 / Safari 16.4. Suppressed with oxlint-disable-next-line, which is the only spelling that holds: the eslint- form fails `pnpm lint` because no unicorn plugin is loaded. - The ref-count test advances past the MIN_VISIBLE_MS hold on both sides of the inner bracket. Previously the 1200ms floor alone kept the flag true, so the assertion passed with ref-counting deleted. Note ref-counting is guarded in two places (the early return and the timer callback's count re-check); only removing both fails the test. - Added a two-coded-step case so the reversal is pinned. The other code tests all pass under a forward find(), which is precisely the demo path: file a decoy, get refused, file the real code. - Added the first coverage of RecordingVignette's data-recording attribute, the entire contract with the .recording-vignette CSS. Also hoists the no-provider fallback to a frozen module constant so consumers outside a provider get stable function identities rather than a fresh literal per render, and documents that a beginRecording() during the hold inherits the existing feed on purpose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
2ddea0b403 |
feat(reskinnable-demo): add the shell-owned teach-mode recording module
Three skins had shipped three copies of the recording context and they had diverged: banking alone had the minimum-visible-duration floor, the feed de-dupe and the fresh-window reset; commerce and people alone derived the demonstrated code from what the operator actually filed. This is the union. Every failure mode here is silent — useRecording returns inert no-ops outside a provider and logStep returns early when idle — so a broken copy does not throw, the feed is simply empty and the glow never appears. The vignette CSS moves from banking/theme.css to globals.css. It reads the shared --brand-violet / --brand-indigo tokens, which all six skins define, so the glow now reskins without a per-skin copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Lukas Moschitz
|
9303fabcdd |
fix(channels-slack): let an image use a file already in the workspace
Slack's image block takes either an external `image_url` or a `slack_file` pointing at a file that already exists in the workspace. The required-field check demanded `image_url` unconditionally, so the `slack_file` form could not be built at all — an image sourced from Slack itself was unreachable through the catalog. An image now needs alt text plus *either* source. Passing neither is still an error: the check moved, it did not disappear, and the test covers both halves. |
||
|
Lukas Moschitz
|
078116ee8d |
fix(channels-slack): stop tagging untyped composition objects with a type
Slack's option object is `{text, value}` — it has no `type` field, and neither do
confirm, option_group, conversation_filter, dispatch_action_config, slack_file,
trigger or workflow. The codec tagged every catalog entry with its manifest type
regardless, so each of those carried an unknown field and Slack refused the
entire message containing it.
That took out every select, multi-select, checkbox, radio group, overflow menu
and confirmation dialog authored through `Slack.Object.*` — the whole interactive
surface. Measured against a real workspace: 1 of 26 block elements was delivered
before this change, 23 after.
It went unnoticed because a refused payload produces no error. There is no log
line and no exception; the message simply never arrives, which looks exactly like
a bot that had nothing to say.
The existing catalog test asserted the very assumption that was wrong — that
every entry serializes its discriminator — so it was green while the product was
broken. It now asserts the corrected rule and guards against a silent relapse.
|
||
|
Lukas Moschitz
|
81e7fefb18 |
fix(channels-slack): correct the container slot and drop the unpostable file block
Two catalog errors, both surfaced by delivering every documented block into a real Slack workspace rather than reading the reference again. container serialized its children into `blocks`; Slack reads `child_blocks`, so every container an app sent was refused — silently, because a refused payload produces no error anywhere, just a message that never arrives. file leaves the manifest. Slack states you cannot add it to app surfaces directly; it only appears when *reading* messages that contain remote files, and the same sentence appears verbatim in `@slack/types`' own doc comment. Keeping it in an authorable catalog offered a component that could never succeed. Sending a file remains thread.postFile(). alert stays out for the same class of reason (modals only), and both exclusions now carry their citation so "missing" and "deliberately not authorable" stay distinguishable. |
||
|
Maxim
|
54c32a1945 |
fix(reskinnable-demo): guard the page-level HITL approval renders (#6453)
## What
`0a9b99aae3` added the durable `resolved` prop to `ApprovalButtons` and
wired the three call sites in `tools.tsx` that need it. The five
`useHumanInTheLoop` renders that live on the pages were missed:
| File | Actions |
|---|---|
| `pages/team.tsx` | `removeMember`, `changeMemberRole`,
`changeMemberTeam` |
| `pages/cards.tsx` | `addNewCard`, `assignPolicyToCard` |
All five guarded only on `status === "inProgress"` and then fell through
to `ApprovalButtons`, whose local `responded` state dies with the
component. Each now destructures `result` and passes the same signal the
three wired renders use:
```tsx
resolved={status === "complete" || !!result}
```
## Why it matters
Reload a thread containing an already-answered `removeMember` and the
local state is gone, so live Approve/Deny buttons come back on a settled
call. A second click fires a duplicate write. This is reachable without
the multi-step chain that motivated the original fix.
The other three renders in `tools.tsx` still need nothing — they return
a terminal card when complete, so they never reach the buttons.
## Correctness
The render props are a discriminated union: `result` is `undefined`
while `Executing` and a `string` once `Complete`, so the buttons stay
live in exactly the one state where the user should act.
`ToolCallStatus` is a string enum, so the `status === "complete"`
comparison deserved a check rather than an assumption. Verified against
the repo's tsc 5.9.2 in isolation: it compiles clean, and a negative
control (`status === "definitelyNotAMember"`) errors with TS2367, so the
check isn't vacuous.
`|| !!result` is redundant given the union, but kept for consistency
with the three existing call sites.
## Verification
- `nx run reskinnable-demo:lint` — clean
- `nx run reskinnable-demo:test:unit` — 1130 tests across 99 files pass
- lefthook pre-commit + commitlint — passed, no `--no-verify`
Two things I did not do, stated plainly:
- **Not exercised in a browser.** The original three were verified live;
these five are structurally identical renders extended by inspection.
- **`nx run reskinnable-demo:build` fails locally**, on `Module not
found: Can't resolve '@copilotkit/shared'` from
`packages/web-inspector/dist`. It fails identically on clean `main`, so
it is a stale local workspace link and not a regression from this change
— but it does mean CI is the first real type-check of this diff.
## Out of scope
Only the banking skin's `ApprovalButtons` sites were audited. The other
skins (commerce, keel, logistics, people, airline) were not checked for
their own approval UIs carrying the same local-state-only pattern.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||
|
Maxim
|
24c6c5dafe |
fix(reskinnable-demo): guard the page-level HITL approval renders
|
||
|
Maxim
|
ea587ec828 |
feat(reskinnable-demo): add the commerce skin (Bellwether) (#6442)
Adds a sixth skin to the reskinnable demo: **`commerce` / "Bellwether"**, a storefront operations console for a DTC retail brand. REST-backed over `/api/commerce/v1/*`, and demo-complete against all nine demo beats plus the presenter-reset requirement — which puts it alongside `banking` and `people` rather than the earlier wiring-only skins. Its teachable gate is approving a markdown that would trade **below the category margin floor** (422 `BELOW_MARGIN_FLOOR`), unlocked by a margin waiver filed under a justifying code. Two below-floor markdowns are seeded, so the case taught on stage and the unaided replay are different products. ## The signature visual: the margin ladder One rail per category, each anchored to that category's own margin floor, so *"how far from the line I may not cross"* is comparable across categories at a glance. The decisions inside it are all recorded in the commit bodies, because all of them were wrong first: - Every floor line sits at **one** rail height, so the rails read as a set — the per-category floor is encoded by dot position, not by moving the line. - A dot that overflows its rail is **never hidden**. Hiding it lent the dot to the *next* rail, silently misattributing a below-floor product to a compliant category. - An **empty** ladder renders as "nothing plotted", not as an all-clear. - A category with **no margin floor on file** renders as visibly *unchecked* — not green, not red, and not a bare figure in neutral ink, which reads as "checked, fine" on the exact question the ladder exists to answer. ## Also the reference for a four-lever view (beat 3c) Status, exception class, sort and top-N all arrive from the query string and **all four controls tint**. The queue count is computed against the filters actually applied rather than the unfiltered set, and an unusable top-N lever is ignored instead of collapsing the view to one row. ## Review This branch went through an unusually long review: **95 findings** fixed in the first cycle, then a **24-agent unbiased confirmation round** (all seven slot types, verbatim prompt) which returned ~233 more. Those resolved into **11 recurring defect classes**, closed by class sweeps rather than per-instance fixes. **Every sweep found more instances than the finding that triggered it** — which is the argument for the approach, as data rather than opinion: | Class | Named | Found | | ----- | ----- | ----- | | Secret redaction covering the URL but not the key | 2 secrets | **5** (the API key appeared verbatim 5× in one response body) | | Beat 3d attachment reporting success it never verified | 6 | 6 | | Silent input coercion on the refund path | 2 | **9 of 22 probed inputs became issuable figures** | | Gen-UI renderers dereferencing streaming args | 4 | **9**, from enumerating all 19 renders | | Guards whose premise was wrong | 4 | **8** | | Unchecked-floor claims | 5 | **9**, from 33 consumers enumerated | | Write controls that could fire twice or latch | 4 | 6, across three pages | | Doc fixes that had falsified their neighbours | 4 | 4 + 21 verified sound | Four post-convergence audits (2 → 2 → 1 → 0) then caught a class no diff-scoped reviewer could: **claims this PR made about code outside the diff.** That is how the two most valuable findings in the whole review surfaced — see below. Tests: **372 → 1130**. Full evidence, per-finding, in the review ledger. ## Two live defects in *other* skins, found by this review Neither is in scope here; both are on the follow-up list and worth prioritising: 1. **`people`'s offer letter generates a structurally corrupt PDF today.** Same `/Length`-from-string-length byte math commerce's price sheet had, but with **no ASCII fold at all** — live-reachable for the seeded employees `Inés Vidal`, `Sasha Bergström` and the city `Montréal`. Commerce's fix here is the template. (Commerce's own guard was found to work only by accident: with `toAscii` neutered, **8 pre-existing tests stayed green on a structurally corrupt document**. The invariant is now pinned.) 2. **`banking` and `people` leak more secrets than commerce did** — an unredacted `memoryError` plus `apiUrl` echoed into a presenter-facing reset body, where commerce leaked one API key. `src/lib/redact-secrets.ts` added here is skin-agnostic and adoptable as-is. Also: `banking/attach-invoice.ts` and `people/attach-offer-letter.ts` carry beat 3d's identical unverified-attachment defect — **wiring `onUploadFailed` in `chat-panel.tsx` would close it for all three skins at once.** ## Shipping with a known, documented parked list This is deliberate and recorded, not an oversight. The parked items are in-subject and real but not demo-blocking — a conditional cross-skin memory deletion (needs a pinned `INTELLIGENCE_USER_ID`), a latent ladder-rail inversion (unreachable at current seed floors), two prototype-chain lookups needing a hostile URL, 500-vs-400 on malformed request bodies, an unguarded presenter Reset, and ~20 test-hygiene items that belong in a test-hygiene PR of their own. ## Skill-staleness rule `CLAUDE.md` requires every change to existing code to answer whether it leaves `.claude/skills/reskin/` wrong, incomplete or misleading. **Answered in full in the final commit body: yes, in many places, and that commit is the fix.** The skill gained a new `failure-modes.md` encoding the cross-cutting lessons — chiefly that **a demo skin's characteristic bug is not a crash but a confident falsehood**, and that a skin publishes facts to three audiences (the screen, an agent readable, generated UI) with three different obligations. Existing files were corrected throughout: a demo-complete skin scores out of nine beats not six, the required pill count is now derived from the beat map, the Reset link no longer teaches an href the app's own lint bans, and the `theme.css` scaffold is written so prettier cannot mangle its selector. ## History 109 commits reorganised into 12 by area of concern. The tree was proven identical to the pre-rewrite state three ways — empty diff, identical root tree object, and nothing staged after the move — and all 109 original SHAs are cited across the 12 bodies, so the per-fix reasoning stays reachable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Maxim
|
0c3a4962ac | Merge branch 'main' into feat/reskinnable-demo-commerce-skin | ||
|
Maxim
|
d718d9d875 |
docs(reskinnable-demo): update the reskin skill and add failure-modes.md
THE REPO'S STANDING QUESTION, answered here as CLAUDE.md requires of every change to
existing code: does this change make anything in `.claude/skills/reskin/` wrong,
incomplete or misleading for the next person authoring a skin?
YES -- in a lot of places, and this commit is the fix. The skill is the only
instruction a new skin's author reads and it goes stale SILENTLY: nothing
type-checks it, no test imports it, and a skin built from a stale template still
compiles, lints and renders.
NEW: `failure-modes.md`. Building commerce surfaced the same class of defect over
and over, in code that compiled and looked right, so the lessons are now written
down as a checklist rather than left implicit in one skin's diff. It is about the
ways a skin LIES: publishing a verdict it never checked, claiming a write that never
happened, reporting success it has not earned, narrating a partial failure as a
complete one, and counting rows it silently truncated.
CORRECTED throughout `SKILL.md`, `demo-beats.md` and `templates.md`:
- A demo-complete skin scores out of NINE beats, not six, and the required pill
count is DERIVED from the beat map rather than stated as a magic number.
- The URL-contract section stopped calling this a four-skin demo, and now tells a
new skin to register in `skinIds` AND `skinIdentities`, not just the registry.
- All three skins that seed memories are credited; `keel` is credited in the
`useData` contract row; the presenter-reset beat names the skins that ship it.
- The template's Reset link taught a hardcoded `/${skin.id}/…` href -- exactly the
pattern the app's own lint bans and that breaks silently under a LOCK_SKIN deploy.
- The multi-page route template taught the record lookup the skill forbids elsewhere.
- The layout template coupled sidebar width to an inset the shell no longer applies.
- The `theme.css` scaffold is written so prettier cannot mangle the `.theme-<id>`
selector when an author runs the formatter over it.
Subsumes: ae2a8dde6e 933c7f1f38 4644354f65 628f27a1ca d329643c84 08e6692cbf
777014d288 a6667cfbdf 8dc393addf f205ee0c04 89a58d7bd1 8147676dd9 d72db4a20b
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
d9dcf69893 |
docs(reskinnable-demo): update the app docs for a six-skin roster
`CLAUDE.md`, `README.md`, `.env.example` and `docs/teach-mode/README.md`. The roster went from four skins to six, and almost every count derived from it was wrong. Corrected here: - The demo-beat matrix, which now lists `commerce` and reports gen-UI counts that match the registrations actually made. - The `useData` contract row, which names BOTH in-memory skins rather than one. - The list of skins that identify their user, and the list that ship `intelligence/seed-memories.ts` -- `commerce` was missing from both. - `.env.example`, which capped `LOCK_SKIN` at four legal values. - The claim that `people` re-scopes memory per operator, stated more strongly in CLAUDE.md than the code supports. Two decisions about HOW the docs were fixed, because they cost the most time: - CROSS-SKIN CLAIMS ARE SCOPED TO THE SKINS ACTUALLY CHECKED. Several sentences asserted a property of "every skin" on the evidence of one or two. They now name the skins verified. - EACH FIX WAS CHECKED AGAINST THE SENTENCE BESIDE IT. Correcting one count repeatedly left its neighbour false, because the counts are stated redundantly in adjacent prose. That pattern is what motivated the roster test in the shell. Subsumes: 7f5813d7ea 1600591918 8071d05812 f78f6f7cd4 ca2145228d ad76437e1c 4308c02135 3fab172fb1 0ca1564406 f0afa4acce Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
0cc2a00218 |
feat(reskinnable-demo): register commerce and repair the LOCK_SKIN lint guard
Registration is deliberately duplicated across three places -- the client
`registry.ts`, the server `agent-registry.ts` (as `{ createAgent, identifyUser }`),
and `skins-config.ts`'s `skinIds` -- because the server registry must never pull
client-only modules and the config must be importable from an RSC and the proxy.
This commit adds commerce to all three, plus `eslint.config.mjs` and
`src/lib/locked-skin.ts`.
Decisions:
- `LINTED_SKIN_IDS` IN `eslint.config.mjs` HAD ROTTED. It is a hand-copy of
`skinIds` -- an ESLint flat config is loaded by Node and cannot import a `.ts`
module -- and it still named four skins two releases after `people` and `commerce`
shipped, so the LOCK_SKIN skin-prefix guard was blind to both. `skins-config.test.ts`
now lints a synthetic prefixed link for EVERY registered skin through the real
selectors, so the copy cannot silently rot again.
- `skin-roster-docs.test.ts` is new and FAILS THE BUILD when a doc miscounts the
skin roster. The "four skins" claim was wrong in several places at once; a test is
the only thing that keeps prose counts honest, since nothing else type-checks them.
- The registries' own comments were lying: they credited `logistics` with durable
memory it does not have, promised `commerce` a memory switch it does not ship, and
said Rowan re-scopes memory in a way it does not.
Reviewer note: `skin-roster-docs.test.ts` was largely rewritten by `3f994daf0f`,
which is cited in the theme commit.
Subsumes: 00514f678d 229fcdee66 951a8090c8 bc3a15f677 7ea21b37c0 724e518ca8
0f18a24d74
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
989016e01c |
feat(reskinnable-demo): assemble the commerce Skin contract object
`skin.tsx` implements the frozen `Skin` contract, with `identity.ts` (brand, logo, favicon), `suggestions.ts` (one pill per beat, in demo order, with the skin's beat map written out at the top of the file) and `providers.tsx` (the teach-mode recording stack). Decisions: - IT OMITS `useData`. The ledger is read through the skin's own `useCommerceLedger()` context, mounted in `RuntimeProviders` rather than `Providers`, so the single ledger fetch also feeds `useRuntimeProperties`. That is the same shape banking, logistics and people use. - Beyond the required fields it sets `Providers`, `CanvasSurface`, `sandboxFunctions`, `toolLabels`, `chatHeaderActions`, `onSuggestionSelect`, `RuntimeProviders` and `useRuntimeProperties` -- the full optional surface, which is what a demo-complete skin needs. - `resolvePage` 404s a segment named after something on `Object.prototype` instead of resolving an inherited property to a page component. Subsumes: eb6c516d3f Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
1f7a0eae65 |
feat(reskinnable-demo): add Bellwether's theme block and its contrast guard
`theme.css` is a single `.theme-commerce` block that RE-VALUES the shell's shared design tokens -- it invents no token names, which is what keeps a reskin a pure value swap. `theme.test.ts` is the guard that the values are actually usable. Decisions: - Bellwether's dark-mode buttons were unreadable: the button foreground was resolved against the wrong background, so the pair that shipped had contrast far under the bar the token values were chosen to hit. - THE GUARD NOW MEASURES WHAT IT NAMES. Several assertions were checking a different token pair from the one in their own description -- passing tests that proved nothing about the thing they claimed. Two further "coverage" claims promised checks that were never run at all. Reviewer note: `3f994daf0f` and `e2f3d61489` also touched `src/shell/skin-roster-docs.test.ts` and `docs/teach-mode/README.md`; those files land in the shell and docs commits respectively, but the SHAs are cited only here. Subsumes: 511e173d81 3f994daf0f e2f3d61489 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
4598666200 |
feat(reskinnable-demo): scope commerce's memory and add its presenter reset
The long-term-memory and stored-procedure beats are not emergent behaviour; they need a per-user memory scope and a reset that puts the demo back to a known state. This area is that machinery: `intelligence/user-id.ts` (the server-safe `identifyUser`), `intelligence/seed-memories.ts`, `intelligence/forget-memories.ts`, the gated `dev/reset` route, the skin `layout.tsx` that hosts the Reset control, and a shared `src/lib/redact-secrets.ts`. Decisions: - RESET BUCKETS ARE DERIVED FROM `resolveUserId`, not hand-listed. A hand-listed set drifts from the identity the runs actually use, and the failure mode is a reset that reports success while wiping a bucket nobody writes to. - THE IDENTITY MAP REFUSES INHERITED KEYS. A role named after something on `Object.prototype` resolved to a function and produced a nonsense scope. - THE RESET NEVER REPORTS SUCCESS IT HAS NOT EARNED. It fails when the memory WIPE did not finish, does not claim memories it never seeded, and when it throws it reports what it had measured up to that point rather than a bare error. - A PARTIAL RESET NO LONGER LEAVES TWO NARRATORS DISAGREEING. The route's summary and the on-screen confirmation are now driven from one result, so a half-completed reset cannot be described as complete by one of them. - THE RESPONSE BODY CARRIES NEITHER THE INTELLIGENCE BACKEND URL NOR THE API KEY. `src/lib/redact-secrets.ts` is the shared scrubber, and it scrubs the KEY, not just the URL -- scrubbing only the URL left the credential in the body it was embedded in. Subsumes: 085ca5484e c3f6012ddb 4dd9bcd1c9 4b5b70e3e7 6f2ca38f88 fec06be733 c4d1ef71af 691c6e84e7 d1d6570deb Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
82c1c8e1fd |
feat(reskinnable-demo): add commerce's agent, a2ui trade brief and OGUI sandbox
The server half of the skin plus the two generated-surface paths. - `agent.ts` -- the server-safe `BuiltInAgent` factory. No "use client", no JSX, no React, so the server-only `agentRegistry` can import it. `id === skin id === agent id` is the only link between the skin and its agent. - `catalog/` -- the a2ui catalog and its renderers; `canvas-surface.tsx` and `report-data.tsx` render the server tool `render_trade_brief` full-region on the shared canvas; `build-brief-ops.ts` assembles the brief. - `design-skill.ts` and `sandbox-functions.ts` -- the OGUI brief and the functions exposed inside sandboxed iframes. Decisions: - THE PROMPT CARRIES A CLAUSE-TO-BEAT MAP. Most beats are enforced by prompt text, which makes an unlabelled prompt unreviewable: nobody can tell which clause is load-bearing for which beat, so nobody dares delete a line. The map is the fix. - THE SANDBOX REFUSES WHAT IT CANNOT SERVE. An empty snapshot returned on a failed read is indistinguishable from a genuinely empty ledger, and generated UI then renders a confident "no exceptions" screen. It now refuses instead. - Sandbox views SHARE THE APP'S SET PREDICATES rather than re-implementing them, so generated UI and the app agree on what "on hold" or "below floor" means. - The brief de-duplicates its a2ui selections; the same order appearing twice made the brief's counts wrong. - Beat 5's writes stay inside the view beat 3c built -- the agent may only write to rows the presenter's filters actually selected. Subsumes: ff8708651e 61a2404528 4067feadba e5efcac1a1 904ba72dcd Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
7050d655b3 |
feat(reskinnable-demo): add commerce's price sheet and beat 3d ingest
Beat 3d is "multimodal in, durable artifact out". For commerce the document is a vendor PRICE SHEET and the artifact is a filed restock plan. `data/price-sheet-pdf.ts` generates the PDF, `data/price-sheet-styles.ts` holds its typography, `/api/commerce/v1/price-sheet` serves it, `attach-price-sheet.ts` stages it onto the real composer so the pill's prompt rides with the attachment, and `/api/commerce/v1/plans` files the plan the ingest produces. Decisions: - COLUMNS ARE SET IN A MONOSPACED FACE. The sheet is a price table; proportional digits made the columns unreadable and, worse, made the demo's "read this document" claim look like a claim about a picture rather than about data. - The PDF's byte math assumes ASCII content. That was an unstated invariant holding up the whole layout; it is now pinned by a test rather than left to hold by luck. - EACH VENDOR IS QUOTED ONLY ITS OWN STYLES. The sheet leaked a second vendor's pricing into the first vendor's document -- correct-looking output, wrong data. - The cost narrative is DERIVED FROM the rows actually printed, so the prose under the table cannot describe a table that is not there. - The route fails LOUDLY. It no longer serves a zero-byte or partial PDF on an internal fault, and an empty `vendor` param is a refusal about the param rather than a 404 that reads as "no such vendor". - The beat-3d pill refuses to send its prompt when no price sheet is attached, and PROVES the attachment landed before claiming a send. Sending the prompt alone produced a confident answer about a document the agent never received. - A wrong restock plan is refused rather than filed. Subsumes: 6204113e8f 5bf498e850 ac0d1ff654 acec790481 cac652ff14 2d4ec6b76a 990c20b398 4dd3aff698 4c6690ea06 5e88b0640c Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
94f28eb4f3 |
feat(reskinnable-demo): add commerce's frontend tools, HITL and gen-UI cards
`tools.tsx` registers the skin's frontend tools, HITL interrupts, gen-UI cards and agent-context readables. Around it: `settle.ts` (the shared write-settlement narrator every card renders through), `refund.ts`, `teach-mode-directives.ts`, `order-queue-levers.ts`, `category-argument.ts` and `margin-summary.ts`. This is where beats 1, 3a, 3c, 8 and 9 are actually enforced on the client. Decisions: - HITL INTERRUPTS ARE ALWAYS SETTLED. Every path out of an interrupt resolves it, including the refusal and error paths -- an unsettled interrupt left the run wedged with a card that could never be dismissed. - A WRITE NEVER THROWS OUT OF ITS HANDLER. Failures come back as a refusal line the card renders, so a rejected write is visible instead of surfacing as an unhandled rejection in the console and a card stuck mid-render. - CARDS NEVER READ ARGS THAT HAVE NOT ARRIVED. Tool args stream; a card reading a half-streamed argument rendered a confidently wrong value. Cards now render a pending state until the argument they need is present. - The save-procedure card reports a DECLINE as a decline, not as a write. Claiming a write that never happened is the single worst thing a demo surface can do. - The recorded step count is reported FROM the recording rather than re-derived from the transcript, so the two cannot disagree. - Queue levers agree with the view: the lever chips describe the filters actually in force, not the ones the tool was asked for. - `category-argument.ts` refuses a category the model invented, so the margin ladder can never plot one that does not exist in the ledger. - `margin-summary.ts` caps its rows and SAYS what it left out. A silently truncated list is worse than a declared one, because the model cannot tell the difference. - The refund receipt's staleness wording was reconciled with the guarantee `settle` actually makes, and a blank record needle is refused instead of writing to row 0. Subsumes: 61d117bada 3eefc19766 f909cd2eee 5311f05d60 11779cd3f4 30d2413f4e dd370ef392 8814e78b20 f9cd20903a e3022a4228 b385b37764 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
bebc2d960d |
feat(reskinnable-demo): add commerce's REST API routes
`src/app/api/commerce/v1/*` -- one `ledger` snapshot read plus every write path the skin and its agent use: order patch, notes and notify; promotion approve and decline; return decision and refund; margin-waiver create and finalize. Together with the data layer they are what makes commerce REST-backed rather than in-memory, which is the point of shipping it beside the in-memory skins. Decisions: - A refund amount arriving as a string, `NaN` or a non-finite number is REFUSED at the boundary rather than coerced. `Number(body.amount)` turning `"12.oo"` into a refund is the failure this closes. - An UNREADABLE request body is told apart from a store fault: a malformed JSON body is the caller's error (4xx), a store failure is ours (5xx). Collapsing both into one status made a real backend fault look like a client typo. Subsumes: 3ed3b75581 f96c62870b Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
7be4079d74 |
feat(reskinnable-demo): add commerce's data layer and seeded ledger
The substrate under the commerce skin: `data/store.ts` (the in-process ledger the API routes mutate), `data/seed.ts` (the fixture the demo starts from), `data/types.ts`, `data/derive.ts` (every read-model the pages, tools, sandbox and a2ui brief share), `data/http.ts`, `data/find-record.ts`, `data/waiver-codes.ts`, `data/ledger-context.tsx` (the client-side `useCommerceLedger()` the skin uses in place of the contract's `useData`), and their tests. Decisions worth knowing, most of which are corrections to a first cut that looked right and was not: - ORDERS HAVE A REAL STATE MACHINE. Transitions are enumerated and illegal ones are refused at the store, not merely discouraged in the prompt. Beat-5 writes are validated against a CLOSED set for the same reason. - A MISSING MARGIN FLOOR IS `null`, NEVER `false`. A category can have no floor on file -- an unvalidated `/ledger` cast, a provider that mounts on a failed first fetch, a sandbox snapshot with no floors. In that state "is this below the floor?" has no answer, and `false` is the worst one available: it is indistinguishable from "checked, and it is fine" on the question the teachable gate is about. This is what `derive.FloorStatus` encodes. - THE UNLOCK MUST BE EARNED BY THE RIGHT PAPERWORK. A decoy waiver -- right shape, wrong subject -- no longer steals credit for another product's unlock, and a waiver filed AFTER the decision is refused rather than backdating it. The waiver must carry a real justification under a code from `waiver-codes.ts`. - PROTOTYPE KEYS. A record id named after something on `Object.prototype` no longer turns a failed write into a 200, and a blank needle is refused instead of resolving to row 0. - Promotion windows are compared DAY to DAY, not instant to day, so a promotion does not flip active state on a timezone boundary. - Refund rules: a refund on an UNDECIDED return is refused, and the refund guidance the UI shows agrees with the rule the store enforces (they disagreed). - Seed integrity: orders are numbered forward in time, `ret-2204` agrees with the order it returns, and there are enough exception orders that a `top=10` lever genuinely truncates -- otherwise beat 3c's top-N lever proves nothing on stage. - `ledger-context` refreshes are CANCELLABLE and honest: a caller can tell "done" apart from "done, but the screen is stale". Subsumes: a6d2104e2d bdf72fad7a c835259d20 b813f3cab0 55b5929b84 25c4f91b7e 2cf840a057 535cc82e96 b316977017 c1babd02bf 5855c9c6ab eb44916681 ed291d778f c3e9b27d46 6fb2f831d0 c2e0202a28 879c0c666a d0fddceb6e 1d71954555 b9a0516a6e Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
b115e58de4 |
feat(reskinnable-demo): add the commerce (Bellwether) skin
Adds a sixth skin to the reskinnable demo: `commerce` / "Bellwether", a storefront operations console for a DTC retail brand. It is REST-backed (over `/api/commerce/v1/*`) and demo-complete against all nine demo beats plus the presenter-reset requirement, which puts it alongside `banking` and `people` rather than the earlier wiring-only skins. Its teachable gate is approving a markdown that would trade BELOW the category margin floor (422 `BELOW_MARGIN_FLOOR`), unlocked by a margin waiver filed under a justifying code. This commit carries the bulk of the new skin: the four pages -- orders (index), catalog, promotions, returns -- and the components they are built from. The rest of the skin lands in the commits that follow, grouped by area of concern. SIGNATURE VISUAL -- the margin ladder. One rail per category, each anchored to that category's own margin floor, so "how far from the line I may not cross" is comparable across categories at a glance. The decisions inside it, all of which were wrong first: - Every floor line sits at ONE rail height, so the rails read as a set. The per-category floor is encoded by dot position, not by moving the line. - A dot that overflows its rail is never hidden. Hiding it lent the dot to the NEXT rail, silently misattributing a below-floor product to a compliant category. - Each below-floor label points at the dot it actually names. - An EMPTY ladder renders as "nothing plotted", not as an all-clear. Absence of data and absence of violations are different statements. - A category with NO margin floor on file renders as visibly UNCHECKED -- not green, not red, and not a bare figure in neutral ink, which reads as "checked, fine" on the exact question the ladder exists to answer. NAVIGATION -- orders is the reference for a four-lever view (beat 3c): status, exception class, sort and top-N all arrive from the query string and all four controls tint. The queue count is computed against the filters actually applied rather than the unfiltered set, and an unusable top-N lever is ignored instead of collapsing the view to a single row. WRITES -- the pages share one in-flight guard, extracted out of the orders page into `components/use-in-flight.ts`, so a control cannot fire twice or latch. The order queue guards per ROW, not per page, so one row's write does not freeze the others. Refusals returned by the write routes are surfaced on screen instead of being swallowed, and each page's on-screen readable describes the screen it is actually on rather than a generic skin summary. Note for reviewers: `1ad9711d98` created files across every area below; it is cited here once so each original SHA appears in exactly one body. Subsumes: 1ad9711d98 d52b4373ac 92159da17b 737e788fc3 441ce536de 6e8a02c029 6c7bd27cb6 4173278545 7a4c927da9 9a53d5712f 127190449d fefb43c16c 02638a398d c3a07fcd06 e793b652a6 094b4a6e4b 9663656e99 ed6969683e Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Sam Julien
|
6540848745 | chore: refresh agent artifacts for 1.67.1 | ||
|
Sam Julien
|
1853a24d00 | fix(skills): align public guidance with current APIs | ||
|
Sam Julien
|
73d1df29a2 | feat(release): add a generated public API manifest | ||
|
Sam Julien
|
00d800dc2b | fix(docs): enforce the production canonical host | ||
|
Tyler Slaton
|
bee39139bd |
chore: release monorepo v1.67.1 (#6449)
## Release monorepo v1.67.1 **Scope:** `monorepo` | **Bump:** `patch` --- ### How this release process works 1. **This PR was created automatically** by the "release / create-pr" workflow. It bumped the `monorepo` packages to `1.67.1` 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 `monorepo` packages to npm at version `1.67.1` - Creates git tag `monorepo/v1.67.1` - 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.v1.67.1 |
||
|
Maxim
|
b4cfcf6f98 |
fix(react-core): stop the purity gate crashing opaquely on the declared Node floor
`assert-headless-purity.mjs` resolved its dist directory with
`import.meta.dirname`, which landed in Node 20.11 and is `undefined` below it.
The root package.json declares `engines: { "node": ">=18" }`, so a contributor
or runner on Node 18 hit this hard-fail CI gate as:
TypeError [ERR_INVALID_ARG_TYPE]: The "paths[0]" argument must be of type
string. Received undefined
at Object.resolve (node:path:1115:7)
at .../scripts/assert-headless-purity.mjs:71:19
— a stack trace into node internals, at module load, that names neither the
gate nor the real problem. Reproduced against a real Node 18.20.8.
Switch to `path.dirname(fileURLToPath(import.meta.url))`, which both sibling
scripts in this CI job already use (react-core's measure-copilotchat.mjs and
react-native's measure-headless.mjs), so all three read the same and none of
them carries a hidden runtime floor its own package does not declare.
Verified under real Node 18.20.8: the script now walks all four entries (650 /
646 / 649 / 645 modules) and exits 0, and still exits 1 with the full
`links the heavy render stack` report when a forbidden dep is injected into a
dist entry. The metafile-driven graph walk, the loud failure on unresolvable
edges and all 17 negative tests are untouched (`test:scripts`: 19 pass).
Skill-staleness check (reskinnable-demo CLAUDE.md rule): not applicable — this
touches packages/react-core, nothing under .claude/skills/reskin/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
4b25cf34b8 |
test(react-native): stop the entry guard racing its own module load
The #4893 entry-surface guard timed out nondeterministically at full test parallelism ("Test timed out in 5000ms" on `await import("../headless")`), which four independent agents each worked around with --testTimeout or --maxWorkers=2. A flaky hard gate is a gate people learn to ignore. Measured, not guessed. The import is a one-time module-graph load whose VARIANCE — not its mean — broke the default budget: ~0.7-1.1s for this file alone and ~0.9-1.8s inside the full 22-file suite (n=8 each), but 4568ms on the run straight after a cold `nx build`, i.e. 91% of the 5000ms budget spent on an otherwise idle machine. The cost is resolve/transform plus cold-page- cache I/O over the ~283 KB of workspace dist that vitest.config.mjs inlines via `server.deps.inline: [/@copilotkit/]` (core ~218 KB, react-core v2/headless ~55 KB, v2/context, shared); bare-Node `import()` of the equivalent prebuilt dist is 461ms, so evaluation is not the expensive part. Four separate tests each awaited that same import, so all four raced one cost against one budget — and when the first lost the race the other three inherited its in-flight import and timed out with it, which is why the observed signature was three simultaneous failures rather than one. They now share a single explicitly-budgeted `beforeAll`, so the cost lives in exactly one place and each test reports ~0ms. The hook is nested rather than top-level on purpose: its failure domain must cover only the tests that need the module, or an import failure would take down the fs-only graph tests too — the same blast-radius problem as blind spot #4, just relocated into a hook. No assertion is weakened: comment stripping, import()/require() extraction, the exact resolved-graph pin and the revived existence test are untouched, and the guard still fails on a real violation (injecting a lazy `import("@copilotkit/react-core/v2")` into src/streaming-fetch.ts trips 3 assertions; reverted). Verification: 5 consecutive full-suite runs at DEFAULT parallelism, no --maxWorkers or --testTimeout override, 271/271 passing in 3.97-5.87s wall each; plus 3 concurrent full suites (30 workers on 10 cores) all green, and one pass at load average 235. `check-types` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
e7f3d7644d |
fix(react-core): make the #4893 purity gate scan the graph it claimed to scan
`scripts/assert-headless-purity.mjs` is a hard-fail CI gate, and it did not do what its header said. It read four built entry files and asked `code.includes(dep)`. That is weaker than the claim in both directions, and every item below was reproduced against a real build before this rewrite: 1. It never followed an edge out of those four files. Re-exporting one hook from the fat `@copilotkit/react-core/v2` entry — which links shiki, mermaid, cytoscape, katex and streamdown — left `dist/v2/headless.mjs` importing that entry by name, and the gate printed "clean" for all four files, exit 0. Same for a heavy dep reached through `@copilotkit/core`, which is external to this build: the entry says only `from "@copilotkit/core"` and there is nothing to grep. A split-out relative chunk escaped identically. 2. The header claimed the check "follows into node_modules". It followed nothing — not node_modules, not a relative sibling chunk. 3. `code.includes(dep)` is unanchored, so it matched comments and strings. Not hypothetical in either direction: the built artifact is comment-PRESERVING (233 lines of block comments survive in dist/v2/headless.mjs), and the five banned tokens sit in `src/v2/headless.ts`'s own banner. They are absent from dist only because that module is a re-export shell whose banner attaches to no retained code — moving the same sentence into a module that ships code hard-failed CI on all five tokens while linking none of them. The gate now drives esbuild with `metafile: true` over each built entry and matches on the RESOLVED graph, so it follows relative chunk edges and into node_modules for real, resolves `exports` maps, subpaths and pnpm symlinks, and cannot be fooled or tripped by a comment. Matching is anchored at the package name (`@shikijs/langs` and `cytoscape-fcose` count; `shikimori` does not) and also covers a forbidden dep left external, which resolves to no graph input at all. Unresolvable edges FAIL LOUDLY instead of reading as clean, as does a graph that does not contain its own entry. One edge shape survives a bundler: `import(name)` with a non-literal argument, which esbuild leaves alone without even warning. For that the gate reads text — the only place it does — over the graph's first-party files, using the `stripComments` helper ported from the sibling RN guard so a documented counter-example cannot trip it. Adds `scripts/__tests__/assert-headless-purity.test.mjs` (17 tests, wired into `test:scripts` next to measure-copilotchat's), because a hard-fail gate with no coverage of its own failure mode is how this shipped. Proven after the fix: both false negatives now exit 1, a clean build exits 0, and a banned token that appears only in a comment exits 0. esbuild is already this package's devDependency and already runs in the same CI job, so the gate needs no workflow change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
60d3ef1071 |
fix(react-native): externalize react-dom in the headless size measurement
The esbuild `external` list omitted `react-dom`, unlike react-core's measure-copilotchat.mjs, so a stray web-oriented edge could be absorbed into the figure the PR's bundle claim rests on. Checked empirically before changing anything: `react-dom` is NOT reachable from @copilotkit/react-native/headless today. A metafile run shows 0 of the 653 input modules are react-dom, and no module references it even pre-resolution. The reported figure is therefore UNCHANGED — 94941 B gzip (92.7 kB) before and after, byte for byte. The headline "92.8 kB -> 92.7 kB, flat" claim is unaffected and stays comparable with previously reported numbers. The guard is still worth having. Simulating a stray edge measures the inflation it prevents: +56.3 kB gzip via react-dom/client, +57.3 kB via react-dom/server (not the ~130 kB estimated in review — that is closer to the raw magnitude; react-dom-client.production.js is 536 kB raw). The subtler case is a bare `react-dom` edge at +1.4 kB, small enough to read as noise while still being a real regression. No subpath entries: esbuild prefix-matches package paths, so `react-dom` already covers react-dom/client and react-dom/server (verified on the pinned 0.27.3; esbuild CHANGELOG 0.5.14 and 0.14.13). Listing them would imply they were required. Hoisted the list to an exported HEADLESS_EXTERNAL with per-entry rationale, mirroring the sibling's DEFAULT_EXTERNAL, and made `external` an overridable option so the new test can A/B it rather than assert on a literal. Both new tests fail if react-dom is removed from the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maxim
|
4315adb1e7 |
test(react-native): make the tool-result assertions read the result
Two tests claimed to prove a tool RESULT reaches its renderer and neither
read it. "reports complete and passes the result through" rendered through a
registrar printing only status and args, so replacing the correlated tool
message's content with a constant left it green; its in-place-mutation twin
had the same hole. The integration test's only result assertion built its
tool message as `{ content: "ok" }` behind an `as never`, so it carried no
toolCallId — the id production correlates a result to a call by.
Both now render status, args AND result together, and a new negative case
gives a tool call a result belonging to a DIFFERENT call. That last one is
the only detector for a lookup that ignores the map key: every fixture in
the file matched on id, so an unkeyed "hand out any tool result we have"
lookup passed the whole suite unchanged.
Fixtures move to src/__mocks__/tool-fixtures.ts. toolMessage() takes
toolCallId as a required positional argument, so no fixture can omit the
correlation, and assistantToolCall() returns a typed AssistantMessage —
which retires the `as never`, an `as unknown as Message`, and three `any`s
in the touched files. A properly-typed ToolMessage typechecks at that call
site unchanged; the cast was convenience, not a type-system limit.
Test-only: CopilotChat.tsx is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maxim
|
d144757d8a |
test(react-native): guard the entry surface's export kinds against type-only stripping
`export type { X }` strips X's runtime binding. Five runtime values shipped from
`src/headless.ts` inside `export type` blocks — the `ToolCallStatus`,
`UseAgentUpdate`, `CopilotKitCoreErrorCode` and
`CopilotKitCoreRuntimeConnectionStatus` enums, and the `AbstractAgent` class —
while the reference docs told consumers to import and branch on them. Nothing in
the repo could see it: the package built, typechecked, linted and passed its
suite, because nothing here consumed its own entry the way a consumer does.
react-core's `headless-type-exports.test-d.ts` cannot cover this. Export kind is
a property of the re-exporting module, and that guard reads react-core's entry —
react-core's own `UseAgentUpdate` was already a correct value export while RN's
was wrong. The guard has to live on the RN side and read RN's own entries.
Adds `src/__tests__/headless-value-exports.test.ts`, in three layers:
- §1 asserts each of the five is a present runtime binding of the expected
`typeof`, with its enum members nameable, on BOTH `@copilotkit/react-native`
and `@copilotkit/react-native/headless`. A stripped export is an absent module
binding, so a runtime test is the direct instrument and cannot be faked by a
cast or an expect-error.
- §2 needs no symbol list: it parses both entry sources, and for every symbol
re-exported type-only it imports the module that symbol came from and fails if
that module has a runtime binding for it. A future contributor who adds a new
enum re-export inside an `export type { … }` block is caught without anyone
updating §1, and the failure names the symbol, the source module and the fix.
A floor on the parsed specifier count keeps a rotted parser from passing
vacuously.
- §3 type-checks the consumer-visible symptom (enum-member comparison on a
render-prop `status`, `extends AbstractAgent`, `instanceof`). The file lives
under `src/`, so `check-types` compiles it and a regression also fails there
with TS1362 naming the symbol. Its bodies are lazy on purpose: a module-scope
`extends` would crash collection and hide §1/§2's guided messages.
Proven by transiently restoring the defect three ways — the `AbstractAgent`
class, `ToolCallStatus` moved into an `export type` block, and `UseAgentUpdate`
via the inline `type ` prefix. Each produced 4 failing tests plus TS1362;
`src/headless.ts` is byte-identical to before.
RN suite 23 files / 276 tests (baseline 22 / 261, so +15 and no change
elsewhere); `nx run @copilotkit/react-native:check-types` clean; oxfmt and
oxlint clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|