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>
`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>
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>
`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>
`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>
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>
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>
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>
`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>
`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>
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>
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>
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>
`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>
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>
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`.
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>
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>
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>
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>
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>
`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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Covers the two things a reader would otherwise assume wrongly: it does NOT pin
dark/light (separate axis), and it does NOT hide the inspector — a locked deploy
still shows it, which is the intended FDE configuration.
Says what the gate actually governs: the UI and routing expose only that skin.
It is a presentation/deploy gate, NOT a security boundary — all four agents stay
registered server-side, so another skin's agent endpoint remains reachable under
a lock.
Also corrects the stale "floating selector at the bottom-left" description; the
switcher is a dropdown at the top of the assistant column.
Co-Authored-By: Claude <noreply@anthropic.com>
The inspector's agentId-less /memories and /info requests stay keyed to
defaultSkinId even under a lock, so on a deploy locked to a non-default skin they
resolve a different scope than the running agent. Deliberate: the default
resolver is the one whose scope is seeded, so switching to the locked skin's
resolver would read empty on any skin without seed data. Only banking ships real
durable memory and it is also the default, so the two align in the configuration
that matters.
Co-Authored-By: Claude <noreply@anthropic.com>
The suite visits /airline and asserts all four switcher options, so a developer
with LOCK_SKIN set locally would watch it fail for reasons that look nothing like
the cause.
The pin only covers a server Playwright STARTS. reuseExistingServer means a warm
local run adopts an already-running pnpm dev and skips the whole env block, so
the comment documents both shapes: a non-banking lock 404s the hardcoded
/banking readiness probe and aborts at webServer startup, while a banking lock
gets through and fails the /airline assertions instead. In CI reuseExistingServer
is false, so the pin always applies.
Co-Authored-By: Claude <noreply@anthropic.com>
The dropdown is not rendered at all — no trigger, no chevron, no options in the
DOM. A disabled dropdown was rejected: it implies a choice that does not exist
and reads as a bug rather than as a single-tenant product. The badge is a div
with cursor:auto, no handler and tabIndex -1, so there is no dead control to
click or tab onto.
The identity block is defined once and rendered into either a button or a plain
div, so the two modes cannot drift apart. Swap-sides, hide, the skin-selector
testid the layout e2e keys off, and useSkinThemeReconcile's root all stay put.
Co-Authored-By: Claude <noreply@anthropic.com>
The tab read "CopilotKit Reskinnable Demo" beside the locked skin's own favicon,
leaking both "CopilotKit" and "demo" on the most visible surface a prospect sees.
generateMetadata brands the title AND the description from the locked skin, so
crawlers and link unfurlers see a coherent product page. A client effect cannot
do this: Next applies route metadata after hydration, so SSR always shipped the
demo strings.
force-dynamic here too. The root layout reads LOCK_SKIN and
PRESENTER_RESET_ENABLED per request and threads both into client gates;
correctness otherwise rested on the implicit invariant that every descendant
route happens to be dynamic. Cost is one dynamically-rendered /_not-found.
Unlocked metadata is byte-identical in both fields.
Co-Authored-By: Claude <noreply@anthropic.com>
Enforcement is one extra notFound() condition in SkinLayout. No middleware is
needed: notFound() throws during render, so a disowned skin never mounts a
provider, a thread or an agent registration. A redirect WOULD have needed
request-time middleware, and a route matcher there risks intercepting
/api/copilotkit's SSE stream.
Under a lock, /airline is as absent as /nope — uniform 404 semantics. / now
redirects to the locked skin, without which a locked deploy's front door would
land on defaultSkinId and 404.
force-dynamic on / is not optional: reading process.env is not a dynamic API, so
next build otherwise prerenders / and bakes the build-time skin into the
redirect. A deploy built unset then run locked sent / to a 404 front door.
Co-Authored-By: Claude <noreply@anthropic.com>
Same shape as the presenter-reset gate: the server env is read in the root
layout and passed down through a small context. The context default is null
(unlocked) so any subtree without the provider — including SelectorCard's bare
unit tests — behaves exactly as before.
isSkinLockedOut is a named predicate rather than an inline comparison because
inverting it would 404 every skin on an UNLOCKED deploy. Extracted, it gets
exhaustive mutation-sensitive tests without rendering SkinLayout and mounting
CopilotKitProvider.
Co-Authored-By: Claude <noreply@anthropic.com>
A per-deploy SERVER env, deliberately non-NEXT_PUBLIC_ like
PRESENTER_RESET_ENABLED, so one build serves both a locked single-tenant host
and the unlocked four-skin demo.
Throws on an unrecognised id rather than falling back to unlocked: silently
accepting a typo would 404 every skin AND send / to a 404 too, leaving the whole
app dark with nothing pointing at the cause.
Co-Authored-By: Claude <noreply@anthropic.com>
LOCK_SKIN must be validated, and a locked deploy's brand and tagline resolved,
from server components. Those cannot import registry.ts — it pulls in four
client skin modules. skins-config.ts is the import-free home for that, so the
data is duplicated there and fenced by a drift guard asserting it matches the
registry, which is what stops the copy rotting.
Co-Authored-By: Claude <noreply@anthropic.com>
Ports the banking demo's #6401 fix, which was never carried over to this app.
ApprovalButtons collapsed only on local `responded` state, which dies with the
component. These cards do get remounted when the run syncs, which resurrected
live Approve/Deny buttons on an action the user had already taken; clicking
them again fires a duplicate write against an already-settled call.
Adds a durable `resolved` prop, OR-ed with the local state so a click still
collapses without waiting for the round trip. It is passed from the tool call
itself at the three HITL renders that do not already early-return on status
"complete". The other three (offerWorkflowRecording,
awaitDashboardDemonstration, saveLearnedWorkflow) render their own terminal
card when complete, so they never reach the buttons and need nothing — which
is why banking also has exactly three call sites.
Verified against the banking skin in the browser: before, approving a policy
exception left a second card carrying live Approve/Deny; after, that card
reads "Response submitted." `pnpm lint` and `pnpm build` both exit 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Approve/Deny buttons could reappear on a card the user had already answered.
The clearest case is the teach arc: after "Open policy exception" is approved
and the chain moves on, that first card came back showing live buttons while
the later cards read "Response submitted."
"Response submitted." was purely local useState, so nothing tied it to the tool
call. Any remount lost it, and the earliest card in a multi-step chain has its
subtree replaced when the run syncs. Clicking the resurrected buttons would
fire a duplicate write against an already-settled call.
ApprovalButtons now takes a `resolved` prop carrying the durable signal from
the tool call itself, OR-ed with the local state: local collapses the buttons
immediately on click, `resolved` survives remounts and thread reloads. This is
the same result-over-status rule already applied to the PIN and charges cards.
Wires the 3 call sites that had no guard. The 3 that already collapse on
`result` are unchanged — there TypeScript correctly rejects the status
comparison as unreachable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Maxim