feat(types,app-shell): carry the tool approval envelope through hydration - #9229
Conversation
…tion
Three of `ChatToolInvocation`'s ten declared `state` values —
`approval-requested`, `approval-responded`, `output-denied` — are states the AI
SDK's own tool-part union cannot express without an
`{ id, approved?, reason?, isAutomatic?, signature? }` envelope. The contract
declared the states and not the envelope, and the Console's hydration mapper
built each invocation from six fields with neither the envelope nor
`pendingActionId` among them. A rehydrated pending approval therefore arrived
carrying a state that says "a human must decide" and nothing a decision could be
made with: `useHitlInChat` keys its index on `pendingActionId` and skips any
invocation without one.
Contract first: `ChatToolInvocation` gains the optional `approval` envelope with
its Zod mirror, and the runtime `ChatbotEnhanced.ChatToolInvocation` mirrors it.
A compile-time pin holds the two declarations to the SAME type in both
directions, because the member crosses the render adapter as an untouched spread
where a divergence would be invisible.
`hydratedMessagesToChatMessages` then lifts both halves, which arrive from
different places. The SDK envelope is persisted ON THE PART and is narrowed to
its declared shape rather than cast — an `approval` with no usable `id` is
refused, not passed through. `pendingActionId` is never a part key; in
rehydrated history it exists only inside the tool RESULT, so it is derived with
`detectPendingApproval`, the same parse the live mapper uses, now exported so
one envelope has one reader instead of two dialects.
Nothing is narrowed. The envelope stays optional, and a pin says so, so that the
`state`-union narrowing this sequences in front of cannot arrive early under
this change's name.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…po's written precedent PM ruling on the bump the seat flagged rather than assumed. The lane's runtime test — does existing stored data render differently — answers no, but that test is about stored data, and what lands here is published SURFACE: two capabilities a consumer can newly rely on, `ChatToolInvocation.approval` and the `detectPendingApproval` export. `.changeset/8214-chatbot-anypart-state-widen.md` settles that case in this repo in those words. The sequencing argument for `patch` — don't spend objectui#8426's `minor` + `**BREAKING**` carrier early — does not hold, for two reasons measured here. The level was never the signal: this repo ships a breaking change AS `minor`, so what marks objectui#8426's half is the `**BREAKING**` carrier, untouched by this declaration. And `.changeset/config.json` puts all 41 packages in ONE `fixed` group, so the released level is the maximum across every pending changeset regardless of this file. `patch` therefore bought no smaller release and no preserved signal — only a changelog line that under-describes what shipped. Prose and frontmatter only; no source file is touched. Ref: objectui#8442 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ Co-authored-by: Claude <noreply@anthropic.com>
PM ruling on both open questions — and my scope split was incomplete, which is the first thing to say
⭐ The split held where it was tested, and the seat tested it honestly in both directions — the additive half compiles, tests and becomes observable with no narrowing pulled in, and the report says so with the ablation to back it. But the seat also found the place where the split was simply silent, and that is mine. Q1 — the additive
|
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract review — head
|
2 participants
Part of #8442 — the ADDITIVE half of the objectui#8426 chain, under the director-seat ruling of decision batch #86 (option A, contract-first). ⛔ Deliberately not a closing keyword: the chain's sequencing is not settled here.
What this carries
ChatToolInvocationin@object-ui/typesgains an optionalapprovalenvelope —{ id, approved?, reason?, isAutomatic?, signature? }— with its hand-written Zod mirror, andChatbotEnhanced.ChatToolInvocationmirrors it.hydratedMessagesToChatMessagesinpackages/app-shell/src/console/ai/AiChatPage.tsxstops droppingapprovalandpendingActionId.Three of the ten declared
statevalues —approval-requested,approval-responded,output-denied— are states the AI SDK's own tool-part union cannot express without that envelope. The contract declared the states and not the envelope; the hydration mapper carried the states through and dropped the data that makes them actionable.⭐ What was MEASURED, not inherited from the card body
The card cited three line addresses from an older commit. All three were re-located by symbol on this branch's base (
b775500af) and both behavioural claims execute:mapMessages.extractToolInvocationsliftspendingActionId(cited:687)mapMessages.ts:694on the basependingActionId: pending?.pendingActionIdin the returned invocationuseHitlInChatindexestoolCallId -> pendingActionIdand skips invocations without one (cited:154-166)useHitlInChat.ts:157-166on the baseidMapmemo guards onif (tool.pendingActionId && tool.toolCallId), anddecide()answers an unindexed call with'No pending-action id found for this tool call.'output-deniedpass-through (cited:44/:49):45/:50Two further readings that change how the halves had to be lifted, and that the card did not have:
pendingActionIdis not a part key and never was. In rehydrated history it exists only INSIDE the tool result envelope. "Stop dropping it" therefore cannot be a pass-through: it has to be derived, and the only honest derivation is the one the live path already uses. HencedetectPendingApprovalis exported (it was the one detector of nine that was not) rather than a second envelope reader being written next to it — AGENTS.md Commandment #0.1.approvalIS a part key, so it is lifted from the part — and narrowed to the declared shape rather than cast, becauseHydratedUIMessagePartis an open record and whatever the server wrote is reachable and unverified. Anapprovalwith no usableidcannot be replied on, so it is refused rather than half-carried.The card said the operator-facing outcome was NOT verified end to end, and asked for that not to become an assumption. Measured here, at the source:
toUIMessages' merge step rewrites a part'sstatetooutput-availablewhenever a tool result is merged onto it. So on the ModelMessage sub-path (assistant call row + separate tool-result row) the state never reaches this mapper asapproval-requestedat all.useHitlInChatkeys purely onpendingActionId, so this change does restore the index entry and the envelope on every sub-path; but the awaiting-approval CARD is gated onstate === 'approval-requested', so on that one sub-path it still does not render. That reading is pinned as a reading in the new test, so the card that changes it turns the line red rather than finding a stale sentence. ⛔ It is out of this PR's scope — it lives in the hydration pipeline, not in this mapper — and is reported to the PM rather than repaired here.⛔ What was deliberately NOT pulled in
Every narrowing in the ruling belongs to objectui#8426 and none of it is here: the authoring
stateunion shedding the three runtime-only approval states; theUseObjectChatOptions.initialMessagesnarrowing that shipsminor+**BREAKING**; theas anydeletion at theuseChatcall; the parts-builder discriminated arms; the deadtoolNameexcess property.The PM's scope split HELD. The additive half compiles, tests, type-checks and becomes observable with no narrowing pulled in — build 29/29 successful, type-check 32/32 successful. A pin states that the envelope is optional, precisely so a later tidy-up cannot ship objectui#8426's break under this change's name.
Bump call —
patch, and the reason is sequencing, not diff sizeThe lane's test (does existing stored data render differently) answers no: the member is optional, every value that parsed before still parses, and nothing changes for data carrying no envelope. The counter-reading is real and is named in the changeset rather than hidden: two published capabilities DO land (the type member, and the
detectPendingApprovalexport), and this repo's own recent precedent bumpedminorfor "a capability a consumer can newly rely on". It still loses — the ruling reserves theminor+**BREAKING**carrier for the NARROWING half, and spending it here would blur the signal the chain sequences on. ⭐ Flagged for the reviewer to overrule if that reading is wrong.Verification
turbo run build --filter=@object-ui/app-shell...)Tasks: 29 successful, 29 totalTasks: 32 successful, 32 totalzod-mirror-parity)Test Files 6 passed (6)·Tests 123 passed (123)check-changeset-presence.mjs✅ 8 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s)check-control-bytes.mjs✅ check-control-bytes: OK (scanned 7408 tracked text file(s); skipped 85 binary)check-new-cross-file-line-citations.mjsVERDICT new-cross-file-line-citations: 0 new citation(s)check-governed-queue-guard.mjs --test✅ NOT GOVERNED — 10 path(s) checked against 5 governed surface(s); none matched.check-changeset-fixed/-no-major/-claims/-overwrite✅markdown-test-inputs.mjs --audit47 candidate test files, all adjudicated; 41 declared entries, all present222 passed (222)/3969 passed· app-shell685 passed (685)/6634 passed/1 skipped@object-ui/console(the one consumer that calls the changed mapper)Every exit code captured by redirect-then-capture, never through a pipe. All numbers above are from the final commit
58e734cce.@object-ui/consoletype-check exited 2 withTS2307/TS2882across five plugins. That is a MISSING PREREQUISITE, not a red gate — those packages had nodistbecause the earlier build only covered app-shell's closure. Re-run afterturbo run build --filter=@object-ui/console^...(Tasks: 34 successful), it reads 0 errors, exit 0. The first result is not recorded as a failed measurement.Ablation — four legs, each with an on-disk mutation proof and a green control
⛔ No permanent test artefact: every leg mutates, measures, restores, and proves the restore by comparing
git hash-objectagainst theHEADblob (not by reading an exit code), with atrap ... EXIT INT TERMon absolute paths.git diff HEADwas empty after every leg.approvalarm from the Zod mirror (marker grep 1 -> 0,0 14numstat)Tests 3 failed/35 passed; the three that fail are the retention, minimal-envelope and refusal pinstsczod-mirror-parity.test.ts: Type '"complex.zod.ts#ChatToolInvocationSchema"' is not assignable to type 'never'0 2numstat)Tests 5 failed/8 passed; the 8 survivors are the pre-existing pins, untouchedapprovalfromChatbotEnhanced.ChatToolInvocation(marker 1 -> 0,0 18numstat)tscTS2344: Type 'false' does not satisfy the constraint 'true'at the two-sidedEqualpintsc⭐ A vs A2 is the load-bearing pair, and it corrects an assumption worth writing down. Under leg A the derived
zod-mirror-parity.test.tspassed. Its pins are compile-time assertions and vitest erases them — its own header says so — sopnpm testproves nothing about mirror drift on this pair andtype-checkis the gate of record. Had only leg A been run, "the parity test covers it" would have been recorded as measured when it was NOT MEASURED. A2 is what makes the claim true.Cards filed, not folded in
Two findings in this area, each measured, neither in this card's scope and neither repaired here:
sanitizeChatMessagesForCache(the localStorage cache WRITE side) rebuilds tool parts without either key, so once this lands the server path and the cache-fallback path disagree. A fix invents a serializer and owes a round-trip test, so it is not a bounded in-place repair.toUIMessages' merge step rewritesstatetooutput-availablefor every merged result, which is the other half of this card's consequence 2. Both were deduped through one targetedsearch_issuescall (REST/search/*is refused for this session), with objectui#8442 itself returned as the positive control.The eslint run is a narrowed one, and here is why the narrowing excluded nothing. (1) The repository-wide run is
turbo run lint, per package. (2) The narrowed run linted exactly the 9 changed source files, counted from--format jsonoutput, not estimated. (3)eslint.config.jsenables no type-aware linting — noprojectService, noparserOptions.project, notsconfigRootDir, norecommendedTypeChecked— so no rule in this configuration can read across files, and this diff cannot move the verdict on any file it does not touch. All 91 warnings are pre-existing classes; the 4 that land on added lines arereact-refresh/only-export-components, which already fires on all 39 named-export sites in that barrel, including the eight sibling detectors exported beside the new one. Zero errors anywhere.Review notes
needs:contract-reviewis on both carriers. Clause-② was declaredyesat dispatch on the mechanical boundary test (a published authoring type gains a member); the default-tier review is the review of record per the maintainer ruling quoted on the card.packages/specis untouched.packages/components/src/ui/**is untouched.https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ.PATCHto a pull-request body downgrades a session-URL attribution footer to the bare form and appends a second footer unconditionally (this body was measured gaining exactly 58 bytes on its first edit), so the session reference only survives in prose. ⛔ The footer below is the platform's own and is deliberately not re-posted by hand.Generated by Claude Code