<!--
Thank you for sending the PR! We appreciate you spending the time to
work on these changes.
Help us understand your motivation by explaining why you decided to make
this change.
**Please PLEASE reach out to us first before starting any significant
work on new or existing features.**
By the time you've gotten here, you're looking at creating a pull
request so hopefully we're not too late.
We love community contributions! That said, we want to make sure we're
all on the same page before you start.
Investing a lot of time and effort just to find out it doesn't align
with the upstream project feels awful, and we don't want that to happen.
It also helps to make sure the work you're planning isn't already in
progress.
As described in our contributing guide, please file an issue first:
https://github.com/ag-ui-protocol/ag-ui/issues
Or, reach out to us on Discord: https://discord.com/invite/6dffbvGU3D
You can learn more about contributing to copilotkit here:
https://github.com/copilotkit/copilotkit/blob/master/CONTRIBUTING.md
Happy contributing!
-->
## What does this PR do?
Fixes three `examples/v1/*` README files whose "Clone the repository"
step `cd`s into a directory that no longer exists (leftover from when
examples were reorganized under `examples/v1/`). Following the README as
written fails at the first step with `cd: no such file or directory`.
- `examples/v1/chat-with-your-data/README.md`: `cd
CopilotKit/examples/copilot-chat-with-your-data` → `cd
CopilotKit/examples/v1/chat-with-your-data`
- `examples/v1/form-filling/README.md`: `cd
CopilotKit/examples/copilot-form-filling` → `cd
CopilotKit/examples/v1/form-filling`
- `examples/v1/state-machine/README.md`: `cd
CopilotKit/examples/copilot-state-machine` → `cd
CopilotKit/examples/v1/state-machine`
This matches the already-correct format in
`examples/v1/travel/README.md`.
Docs-only change, no code/behavior affected.
## Related PRs and Issues
- N/A
## Checklist
- [X] I have read the [Contribution
Guide](https://github.com/copilotkit/copilotkit/blob/master/CONTRIBUTING.md)
- [ ] If the PR changes or adds functionality, I have updated the
relevant documentation
- [X] "Allow edits by maintainers" is checked (lets us help iterate on
your PR directly — faster turnaround for everyone)
A 13.9k-line new file trips the fork-PR supply-chain heuristic
(security_fork-pr-alert flags any added file over 5000 lines), and the
job cannot post its explanation because fork tokens are read-only.
Several showcases ship no lockfile; this one is not a pnpm workspace
member, so nothing depends on it.
grok-4.6 runs xAI's X Search server-side, then composes the answer out of
real React components through five CopilotKit frontend tools. Every post
rendered is a real post the model found.
Registers next.config.ts in the build-config allowlist and adds the row to
the examples index.
Bring the app's own documentation back in line with what the skin now does,
and record in the authoring skill the failure that a live run exposed.
CLAUDE.md had drifted in five separate places, each phrased differently
enough that keyword searches kept missing one: the seed catalog size, the
count of SKIPPED beat-map rows (twice), the beat-matrix cell for
stored-procedure replay, the intro paragraph's list of skipped beats, and a
claim that the seed file seeds "no procedure at all". An exhaustive audit of
every bookstore claim in the file — 36 of them — is what finally closed it.
Also documents a repo-level trap in § Commands: pnpm lint is ESLint only,
while lefthook's pre-commit additionally runs oxlint --fix and oxfmt --write
over staged files and re-stages the result. The two disagree (oxlint enforces
prefer-top-level type imports; ESLint does not), so a contributor can satisfy
the documented gate and still be silently rewritten at commit time. This
already misled a reviewer into filing a finding asking for the exact thing
the hook auto-reverts.
The reskin skill gains an empty-recall requirement for the stored-procedure
beat, in both templates.md and demo-beats.md, which previously documented
only the happy path. A skin author following either would ship the gap this
skin shipped: with the memory empty, the agent reported the miss correctly
and then offered to learn the procedure — beat 6's moment, arriving as an
improvised fallback. Both now require saying so and stopping, with no
guessing and no teach-offer, and cite the worked example.
Note the reference skin has the same gap: banking's beat-5 clause has no
empty-recall branch, and its only such instruction affirmatively calls
offerWorkflowRecording. Correctly scoped to its teach path, but it leaves a
pattern pointing the wrong way for beat 5. Left for a separate change.
The demo half of beat 5: a procedure the agent already knows, an instruction
to recall rather than improvise it, and a pill so the presenter never types.
- intelligence/seed-memories.ts: a kind "operational", scope "user" memory
naming addToCart -> swapEdition -> applyPromoCode -> setDeliveryBy in
order, and explicitly excluding the three distractors. The procedure is
SEEDED, not taught — it is recalled. Scope is "user" and never "project",
which would return the memory for every user of a shared instance.
- agent.ts: clause 7 calls recall_memory FIRST, runs all four steps in order
without confirmation, and states that finding the club is not running the
procedure — reporting the pick, code or date and stopping is the failure
mode, not a partial success. It scopes openCheckout out, so the run ends
with a filled but unpaid cart, and refuses the teach-offer: this is a
recall, not a teaching moment.
- The empty-recall branch exists because a live run went off-script the
moment the store was empty. With nothing recalled the model said so
correctly and then offered to LEARN the procedure — which the clause
already forbade, and which is beat 6's moment. It now says plainly that
nothing was found and stops, without guessing the pick, edition, code or
date from the catalog or cart: an invented answer that looks right is worse
than an honest failure, because on stage the two are indistinguishable.
The four tool names are frozen string literals shared by the prompt and the
seed, and no test reads either, so renaming one breaks the beat with a green
suite. A drift guard is the next commit's concern, not this one's.
Price the cart through the three-argument cartTotals and show what the club
run actually did to it.
The discount is rendered as up to TWO rows, not one. discountCents is a
single scalar, so a single row labelled with the club would render
club-plus-credit under the club's name and silently misattribute the credit —
and that combined case is reachable exactly when the applyStoreCredit
distractor misfires, the most scrutinised second of the demo. splitCartDiscount
recomputes the club-only discount and takes the credit as the remainder, so
both parts are attributed honestly and clubPart + creditPart === discountCents
holds for every case, including a credit that exceeds the subtotal (the club
keeps its full percentage; credit takes only the applied remainder).
Also adds the delivery-by badge, wishlist and reminder counts, and the same
figures on the page readable so "what's on my screen" agrees with what the
agent says. card_last4 remains the only card datum that leaves the checkout.
Everything the saved book-club procedure needs in order to run: the club
constant and its computed next-meeting date, the edition pair the swap moves
between, discount-aware pricing, the six store writes, and the twelve
registered frontend tools.
- data/club.ts: BOOKSTORE_CLUB (pick, promo code, 15%, meeting weekday),
nextMeetingDate/nextMeetingISO (UTC-only by design) and localCalendarDay,
which re-anchors the caller's LOCAL calendar day onto UTC midnight. Without
it a presenter west of UTC demoing on a Thursday evening gets next
Thursday: at 2026-12-31T23:00-08:00 the naive path skips a full week.
- data/seed.ts: a 25th book, the club pick's paperback, sharing workId
"trust" with the hardcover so swapEdition has a real work to move within.
- data/query.ts: cartTotals gains an optional pricing object and returns
subtotalCents/discountCents alongside totalCents, which stays the
POST-discount amount charged. Inputs are sanitised so
0 <= discountCents <= subtotalCents holds for any input, including a
non-finite credit or discountPercent.
- data/use-data.ts: promoCode, deliverBy, storeCreditCents, wishlist and
reminders persist under one extras key with a field-by-field validator;
six writes returning WriteResult; placeOrder prices through cartTotals and
consumes all three sticky fields. swapEdition merges into an existing
target line rather than duplicating a bookId, and setDeliveryBy's
past-check reads the local calendar day so the club's own date is never
refused.
- tools.tsx: the club readable (the only agent-reachable source of the promo
code), the three procedure writes, the three distractors that genuinely
work, and discount-aware pricing in both the cart readable and
openCheckout's render so the total the agent speaks matches the cart page,
the checkout form and the order record.
Every registration uses [] deps and reaches the store through dataRef:
useFrontendTool keys its effect on JSON.stringify(deps), so a callback in a
dep array stringifies to a constant and pins the pre-hydration store.
Reskin skill: checked, no impact — skin-internal data, store and tool
wiring; no Skin contract field, registration, routing or gate changed.
Answers the standing question in CLAUDE.md — this work found the skill wrong, so
the fixes ship with it.
Rule 1 on tool deps told authors to 'pass the data each closure reads' without
noting that useFrontendTool keys its effect on JSON.stringify(deps). A Map, a Set
or a function stringifies to a constant, so the registration is inert and the
closure never refreshes — the skill's own words for the bug it warns about
described the fix it recommended.
The useData template taught a bare useState(SEED) and said nothing about a
storage-mirrored variant, so an author needing one writes a hydration effect and
trips react-hooks/set-state-in-effect immediately.
Roster prose across CLAUDE.md, README.md, .env.example and the skill now covers
seven skins. Most count claims were rephrased without a numeral rather than
renumbered, so the next skin cannot re-falsify them — skin-roster-docs.test.ts
is what caught them, and its roster fixtures are updated to match.
resolvePage uses a Map, never a plain object: segments[0] is untrusted URL
input, and an object lookup walks the prototype chain, so /bookstore/constructor
would resolve a Function where a ComponentType is declared and crash React
instead of 404ing. skin.test.tsx pins that with the prototype-chain keys.
An unknown book slug resolves the detail page and renders a not-found body
rather than 404ing — the agent hands out these links, and 404ing a renamed book
would break a deep link.
Registration is four files, not two: both registries plus skins-config (whose
test asserts skinIds and skinIdentities match the live registry, and which
LOCK_SKIN is validated against) and eslint.config.mjs, where the id joins
LINTED_SKIN_IDS — the array the URL-contract selectors interpolate, so without
it lint is blind to this skin.
The prompt is where the beats are enforced: recall memory before recommending
and name the recalled preference in the note, never ask for or repeat card
digits, never emit a markdown table where a gen-UI component exists.
No temperature is set. gpt-5.4 rejects the parameter and logs that it is
unsupported on every run, so pinning it alongside a comment claiming
determinism would assert a guarantee the model discards.
Tool registrations read live store data through a ref and close with empty deps
where a dep cannot re-register them: useFrontendTool keys its effect on
JSON.stringify(deps), so a Map or a function stringifies to a constant and the
closure keeps its first values forever. openCheckout additionally must not
re-register mid-call — placeOrder mutates the cart, and a teardown would lose
respond() and fail the thread.
Every render keys off the recorded result rather than status: a reopened thread
replays with a stored result and no status transition, so a status-keyed render
looks correct live and blanks on reload.
The route readable in the layout plus one readable per page is what makes the
screen-awareness beat work: asking on two pages must give two different correct
answers. All four payloads are deliberately disjoint.
The active segment comes from useSkinSegments, not a pathname slice — the shell
hook strips a leading skin id rather than a fixed offset, so it stays correct
under a LOCK_SKIN deploy where the segment is absent entirely.
The presenter reset is a full-page assign, not a router.push: it clears storage
with removeItem, bypassing the store, and the store has no storage listener, so
only a document load re-reads it. A client navigation would leave the cart
visibly full right after a successful reset.
The cart page has no checkout button by design — checkout is the agent's beat.
Covers are typographic and generated rather than sourced images: 24 scans would
be a licensing problem, would not reskin with the theme, and would read as stock
photography in a demo whose argument is that the UI belongs to the product.
checkout-card carries the security boundary — onSubmit receives only the last
four digits, the other digits are cleared from state at that boundary, and both
sensitive inputs are type=password because this card appears on a projector. Its
receipt mode re-derives from a replayed result so a reopened thread shows a
receipt rather than a blank form.
filter-bar keeps the ebook lever even though no seed book has that format: the
agent can set format=ebook via browseWithFilters, and a missing lever would make
an agent-applied filter invisible, which is the one thing the component exists
to prevent.
Reads through useSyncExternalStore rather than useState plus a hydration
effect: the effect form fails react-hooks/set-state-in-effect, and a useState
lazy initialiser that reads storage makes the server and client markup
disagree. layout-preferences.tsx is the shell's sanctioned pattern.
getSnapshot caches the parsed value and getServerSnapshot returns a frozen
module-level constant — cart and orders are arrays, and a fresh array per call
fails Object.is and infinite-loops during hydration.
The storage mirror exists for the durable-thread beat: its proof is a hard
reload, and a useState-only cart empties at exactly that moment.
Memory is scoped per shopper, so the same suggestion pill answers differently
for Maya (one seeded taste preference) and Guest (none). That contrast is the
demo's headline claim, so identifyUser never derives a scope from userRole —
both shoppers share the role and a role-derived scope would merge them.
forgetAllMemories deliberately skips scope:'project' rows: project scope is
global to the Intelligence backend instance rather than partitioned per
product, all skins share one instance locally, and banking seeds a
project-scoped procedure memory a bookstore reset must not destroy.
The reset route maps raw shopper ids through resolveBookstoreUserId before
clearing. Passing the raw ids would clear scopes nothing writes to and no-op
while reporting success.
Theme values are space-separated HSL channels, not hex: globals.css wraps every
token in hsl(), so a hex value yields invalid CSS and the whole skin silently
falls back to the gray :root defaults.
href.ts and nav-target.ts route every URL through useSkinHref. A hardcoded
/bookstore/... path puts the tenant segment back in the address bar on a
LOCK_SKIN deploy, and concatenating onto the builder's base emits the
protocol-relative //book/x because that base is '/' under a lock.
The seed test encodes the demo's falsifiability rule: the literary and
translated shelves must carry hardcovers and over-$20 titles, or the
recalled 'paperback only, under $20' preference has no visible effect.
cartTotals returns { itemCount, totalCents } — no tax and no shipping, so a
separate subtotal would duplicate the total and could drift.
0a9b99aae3 added the durable `resolved` guard to ApprovalButtons and wired the
three call sites in tools.tsx that need it. The five useHumanInTheLoop renders
living on the pages were missed: removeMember, changeMemberRole and
changeMemberTeam in pages/team.tsx, addNewCard and assignPolicyToCard in
pages/cards.tsx.
All five guard only on status "inProgress" and then fall through to
ApprovalButtons, whose local `responded` state dies with the component. These
cards are remounted when the run syncs, which resurrects live Approve/Deny on
an action the user already took; a second click fires a duplicate write against
an already-settled call.
They now pass the same durable signal as the three wired renders. The other
three in tools.tsx still need nothing — they return a terminal card when
complete, so they never reach the buttons.
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>
Alem Tuzlak