mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
main
638 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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> |
||
|
Maxim
|
24c6c5dafe |
fix(reskinnable-demo): guard the page-level HITL approval renders
|
||
|
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> |
||
|
Maxim
|
db9b9205b0 | Merge branch 'main' into feat/reskinnable-demo-people-skin | ||
|
Maxim
|
3d4de1cedd |
feat(reskinnable-demo): add the people skin (Rowan), a demo-complete People Ops desk
Rowan is a People Operations command center and the second skin built against
the full nine-beat bar in `.claude/skills/reskin/demo-beats.md` (banking was the
first). Pages: Roster (index), Compensation, Requests, Onboarding.
REST-backed like banking and logistics: `/api/people/v1/*` serves one `ledger`
snapshot read plus the write paths, a generated `offer-letter` PDF, and a
presenter-gated `dev/reset`. Components read the ledger through the skin's own
`usePeopleLedger()` context, so `useData` is omitted. That context is mounted in
`RuntimeProviders` rather than `Providers`, which lets the single fetch also feed
`useRuntimeProperties`.
The signature element is the band ladder: one rail per level, each normalized to
its OWN band, so "halfway up L3" and "halfway up L7" line up at the same height
and become comparable. Anyone outside their band is drawn outside the rail, in
the negative colour, always labelled.
Beats, all walked in a browser against a live Intelligence stack:
1 face showCompBands renders the ladder + a two-sentence answer
2 rich thread gen-UI replays intact on reopen after a hard reload
3a drive the app setBaseSalary — the figure is typed into a chat card and
goes straight to REST; it appears nowhere in the transcript
3b sees screen route readable + per-page on-screen readables on all four
pages; Roster and Requests give different, correct answers
3c levers HITL confirm naming the levers, then
?status=pending&sort=aging_desc&top=10 with the Status, Sort
and Show controls visibly tinted, "TOP 10 OF 11"
3d multimodal an offer-letter PDF rides the pill, and the filed packet
survives deleting the thread and reloading
4 memory seeded preference recalled AND named in the component's
`note` slot
5 stored skill one vague sentence fires three visible writes in order, no
confirmations, amid four distractor tools
6 teach a skill 422 OUT_OF_BAND (symptom only) -> decline -> record the
demonstration -> save -> apply unaided to a DIFFERENT person
in a fresh thread
Notes for reviewers:
- The beat-6 gate is deliberately discriminating. Decoy exception codes file and
finalize successfully and still do not lift it, and an unknown code is refused
without enumerating the catalogue — so "the agent filed an exception" is not
the same as "the agent cleared the gate". Two out-of-band comp requests are
seeded so the case taught on stage and the unaided replay are different people.
- Seed dates are relative offsets materialized at store init, not absolute ISO
strings, so request aging and the generated offer letter stay coherent years
from now and a Reset genuinely re-freshens the queue.
- Memories are seeded and saved at `user` scope, not `project`. Verified against
the running stack: a project-scoped row is returned for EVERY user id in the
instance, so with several skins sharing one backend it is not a per-skin
boundary. For the same reason this skin's `forgetAllMemories` skips
project-scoped rows rather than deleting data it does not own, and `dev/reset`
reports the skipped count.
- `temperature` is not set. gpt-5.4 rejects it and the value is discarded, so
carrying it alongside a comment claiming determinism would be misleading.
- Beat 2 additionally requires the thread-list identity fix sent separately; the
skin merges and runs fine without it, it just cannot demo thread reopen.
Docs updated for the fifth skin per the app's standing skill-staleness rule:
CLAUDE.md (skin list, substrate split, beat matrix), README.md,
docs/teach-mode/README.md (teach-mode is now per-skin, not banking-only), and
`.claude/skills/reskin/{SKILL,demo-beats,templates}.md` — including six
"only banking does this" claims that are no longer true.
Verified: pnpm build, pnpm lint, pnpm test:unit (335/335) on this base.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Maxim
|
d7ab8ce2a8 |
fix(reskinnable-demo): resolve thread-list identity from the query-string agentId
`agentIdFromUrl` only read the target agent from the URL PATH (`/agent/:agentId/run`). Thread routes carry it in the QUERY STRING instead (`/threads?agentId=<id>`), so every thread-list request looked agentId-less and fell through to `defaultSkinId`'s `identifyUser` — banking's. The result was a split identity for every non-default skin: runs created threads under the skin's own end-user id (the run path resolves correctly), while the list asked for banking's id and got an empty array back. The thread rail read "No conversations yet" forever and reopening a conversation after a reload was impossible. Nothing errored, which is what made it hard to see — and it reads to a viewer as "this product doesn't persist threads", the opposite of what the demo exists to show. Banking was immune only because it IS `defaultSkinId`. Verified against a local Intelligence stack; thread counts returned by `GET /api/copilotkit/threads?agentId=<id>` before → after: people 0 → 11 airline 0 → 1 logistics 0 → 3 banking 7 → 7 (unchanged; it was already resolving correctly) Skins with no `identifyUser` (airline) still fall through to `genericIdentity()` via the existing guard, so this widens correct resolution without introducing a new failure mode. Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
Ran Shem Tov
|
e920d86cb0 | docs: add Claude Managed Agents cookbook | ||
|
Ran Shem Tov
|
6862508eb2 |
Merge remote-tracking branch 'origin/main' into codex/crewai-full-d6
# Conflicts: # showcase/harness/Dockerfile # showcase/scripts/fail-baseline.json |
||
|
Maxim
|
e79376b11b | Merge branch 'main' into feat/reskinnable-demo-lock-skin | ||
|
Maxim
|
d6f285baec |
docs(reskinnable-demo): require a skill-staleness check on every code change
The reskin 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. There is no mechanism that
notices — only a person who thought to look.
This adds one standing question to every change to existing code: does it make
anything in `.claude/skills/reskin/` wrong, incomplete or misleading? Answered in
the PR body or commit message; "checked, no skill impact" is a fine answer. The
unanswered question is the failure, not a considered no.
Grounded in three real misses from the LOCK_SKIN root-serving change in this same
PR, all caught late and none by tooling:
- templates.md handed every new skin the two patterns that change had just removed
(a hardcoded `/${skin.id}/…` href, a fixed `pathname.split("/").slice(2)`). Both
fail silently under a lock — the page renders, the URL is just wrong.
- SKILL.md's verification steps pointed at `pnpm test:unit` and a drift test the
same PR deleted. Caught by a reviewer, not by a gate.
- The skill's authoring half was updated and its verification half was not; the gap
survived until it was asked about directly.
Includes a trigger table (contract change, required/forbidden call, a gate a skin
must pass, registration/routing/boundary, beat mechanism, brand or id, deleted or
renamed referenced file) so it is a lookup rather than a judgement call, and a
~2-minute grep check.
Skill-staleness check for THIS change: no impact. It is a process rule for people
editing the app, not guidance for people authoring a skin; no contract, gate,
command or path the skill references is altered.
Co-Authored-By: Claude <noreply@anthropic.com>
|
||
|
Mark
|
0c10d8c882 |
docs(pydantic-ai): remove duplicate quickstart, fix dead links and commands
Mechanical repairs found while auditing the pydantic-ai docs. Each was verified against the tree; nothing here is a content rewrite. - Delete `quickstart/pydantic-ai.mdx` + its `meta.json`. `seo-redirects.ts` already routes `/pydantic-ai/quickstart/pydantic-ai` -> `/pydantic-ai/quickstart` (rule F6), and adk got the same treatment (F7). pydantic-ai was the only framework still carrying a `quickstart/` subdirectory alongside the canonical `quickstart.mdx`. - `human-in-the-loop/agent.mdx`: link to the canonical quickstart directly instead of the redirected legacy path, and point the starter link at `examples/integrations/pydantic-ai` — `examples/coagents-starter-pydantic-ai` does not exist. - `docs-links.json`: `subagents.shell_docs_path` was `/multi-agent/subagents`, which has no page. The real page is `/multi-agent-flows`, which the entry's own `og_docs_url` already pointed at. - `headless-simple/chat.tsx`: the console tag said `langgraph-python` inside the pydantic-ai package. This sits in an `@region` block, so it is pulled into docs as a snippet. 11 other integrations carry the same copy-paste; they are left for the fleet sweep. - `examples/showcases/pydantic-ai-todos/README.md`: `uv run src/main.py` -> `uv run main.py` (there is no `src/main.py` in that tree), and the stated Python floor now matches `agent/pyproject.toml` (`>=3.13`). - `examples/canvas/pydantic-ai/README.md`: Python 3.8+ was unrunnable — `agent/agent.py` uses PEP 604 unions. Aligned to the sibling tree that pins the same `pydantic-ai-slim==2.22.0`. |
||
|
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>
|
||
|
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>
|