feat(app-sdk): MessageRenderContext carries an optional session-open seam (plumbing for #8254)

[chenmingwei23]

What is the problem?

MessageRenderContext (the exported app-sdk render context, messageRenderers.tsx:46) had no way to resolve a /chat?sid=<slot-key> session link. Its MarkdownRenderer call sites passed content alone, so resolveSessionChip refused for want of onSessionOpen/sessions and the link fell through to an external new-tab link. #8254 asks - as a CONTRIBUTING-routed RFC question, since the interface is exported - whether the context should carry an optional session-open callback, the way the dashboard's own rows already do.

This PR ships the answer as plumbing only: the seam is added, no dashboard host is wired to it, and the user-visible new-tab behaviour is unchanged. #8254 stays open for the host-wiring half (see the last section for why that half is not mechanical).

Why this issue matters to the user

Nothing changes for the user in this PR. The value is to the next contributor: the exported context now can carry the session-open capability with onFileOpen's exact shape, so a host that has a correct in-place switch to offer can supply it without touching the store-free registry. Absent a host, every row behaves exactly as today.

How our fix solves it

The context already carries a host-supplied optional callback - onFileOpen? at messageRenderers.tsx:55 - and that is the shape the session case needs. #3299's constraint is "no store import", not "no host callbacks".

• MessageRenderContext and ChatMessageListProps gain optional onSessionOpen / sessions / activeSession, mirroring onFileOpen? verbatim. The registry stays store-free.
• The three default rows that render markdown - user, assistant, note/inject - forward the trio to MarkdownRenderer, whose resolveSessionChip switches in place when they are present, drops a link naming the active slot (the A /chat?sid= link in a user message opens a new tab instead of switching session #8253 negative control), and falls through to an external link when they are absent (every host today).
• MarkdownRenderer and AssistantMessage already accept these props end-to-end, so the forwarding is the whole wiring on the registry side.

Scope is honest: this is the seam, not a behaviour change. There are zero host consumers in this PR by design.

What tests we did

• ChatMessageList.test.tsx gains a session-navigation callback threading (#8254) block: the user, assistant, and note/inject rows each receive the trio when a host supplies it, plus a negative control proving the rows fall back to no session props when the host wires nothing.
• vitest run on ChatMessageList.test.tsx, messageRenderers.test.ts, transcriptRenderers.test.tsx, and ChatPane.switchLabelOptimistic.test.tsx (proving ChatPane is untouched) - all passing.
• tsc -b zero errors in the changed files; eslint clean. No Python touched. Diff is 3 files: messageRenderers.tsx, ChatMessageList.tsx, ChatMessageList.test.tsx.
• No security surface: an in-app slot switch has no credential or network boundary.

Any other suggestions on the work

Why no host is wired here, and why that half is not mechanical. All three ChatMessageList hosts render a fixed slot, none renders chat.activeSlot: SessionGridView binds each pane to leaf.slot, MembersPage binds its pane to a page-local slot, and ChatEmbed is store-free. switchSlot(key) mutates the global chat.activeSlot and has no router side-effect, so dispatching it from any of these panes would change global state with no visible effect in the pane the user clicked in - a callback that looks wired but silently does nothing, which is worse than the honest external link. A correct in-place wiring is host-shaped (rebind the fixed pane to the target slot, or drive that host's own navigation) and also needs the offline guard the main page documents (an offline switchSlot rejects and clears the target slot's messages). That belongs in a focused follow-up per host; shipping the seam first keeps this PR small and never fires the callback into a no-op.

• Open PR feat(chat): Quote / Ask on selected text in ChatPane (Members + split) #8947 adds sibling optional fields to the same struct at the same anchor; whichever lands second takes a trivial rebase.

Refs #8254

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of e968b5e36ae718feb7071249a7038f0e1300dce2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Verified the load-bearing claims: MarkdownRenderer already has the SessionActions context and resolveSessionChip guard (refuses without handler+roster, inert on active key), and AssistantMessage already threads the trio — the registry was genuinely the only missing link. The shape is copied verbatim from the dashboard's already-exercised ChatPage → MarkdownRenderer path, so the exported seam isn't an unvalidated guess, and per-pane identity is captured in the host's closure, so (key: string) => void suffices for the host-shaped rebind the follow-up needs. The unused-seam risk is neutralized by resolveSessionChip falling through to today's external link, and the negative-control test pins that.

Design-Verdict: PASS

Additive optional seam mirroring a proven shape (onFileOpen), validated by an existing MarkdownRenderer consumer, inert until a host wires it.

[DESIGN-REVIEWED] e968b5e

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of e968b5e36ae718feb7071249a7038f0e1300dce2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

No production call site passes the new props — ChatPane, ChatEmbed, and SideChat all omit them, so the forwarded values are always undefined and every session link keeps today's external-tab fallback. This PR adds no user-visible control, label, state, or pixel: it is type/plumbing only, which lens 12 exempts from screenshot coverage, so the absent blind read leaves no evidence gap.

UX-Verdict: PASS

Plumbing-only seam with zero wired hosts — nothing a user sees, says, or clicks changes in this PR.

[UX-REVIEWED] e968b5e

[github-actions]

First Principles Review (Fable 5) — 🔴 BLOCK

Premise-level review of e968b5e36ae718feb7071249a7038f0e1300dce2 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All the evidence is in. The new trio has zero suppliers anywhere in-tree (all three ChatMessageList call sites — ChatPane.tsx:725, ChatEmbed.tsx:262, SideChat.tsx:598 — pass none of it), the dashboard's own rows already thread the same trio via transcriptRenderers.tsx, and the app-sdk barrel itself documents why surface must not be published before a consumer exercises it.

First-Principles-Verdict: BLOCK

The seam ships with zero consumers by the author's own count, onto a published SDK contract the repo itself says must not be frozen unexercised.

What this change ships

Intent: let a /chat?sid= link in an app-sdk-rendered row switch sessions in place instead of opening a new tab — this PR is an ADDITION (declared "plumbing only"), not a FIX.

1. ChatMessageListProps gains optional onSessionOpen/sessions/activeSession — zero consumers
2. Exported MessageRenderContext publishes the same trio to app authors — zero consumers, one-way-door
3. User, assistant, and note/inject rows forward the trio to MarkdownRenderer — inert; rides on items 1–2
4. Nothing a user can observe changes — stated and confirmed by the diff

Nothing is undeclared; the framing is honest. Honesty does not supply a consumer.

Blockers

Zero-consumer surface on a published contract. The description states it: "There are zero host consumers in this PR by design" and "Nothing changes for the user in this PR." Counted: grep <ChatMessageList finds 3 non-test call sites (ChatPane.tsx:725, ChatEmbed.tsx:262, SideChat.tsx:598); none passes any of the trio, and nothing else constructs a MessageRenderContext. The surface is one-way: both types export through app-sdk/index.ts to apps, and that same barrel records the repo's own rule for exactly this case (index.ts:531-536, on useComposerDraft): exporting "would publish it through the vendor stub … and freeze the contract before its richest consumer … has exercised it. Publishing later is additive; un-publishing is a break." The author's own last section proves no exercising consumer can exist yet — every current host needs a "host-shaped" wiring, so the global (key: string) => void shape is validated by nobody, while the dashboard rows already have this capability through transcriptRenderers.tsx:163-165,296-298. And #8254 is quoted as an open RFC ("asks … whether the context should carry"); merging the seam decides it by default, its only support being "mirroring onFileOpen? verbatim" — symmetry, not a constraint. Subtraction: drop the seam entirely and land the trio inside the first per-host wiring PR #8254 already anticipates — the fields are optional, so adding them then is, in the repo's own words, additive.

[FIRST-PRINCIPLES-REVIEWED] e968b5e

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of e968b5e36ae718feb7071249a7038f0e1300dce2 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e968b5e

False positive or not applicable? A repository writer can comment:
/ai-review override gpt e968b5e36ae718feb7071249a7038f0e1300dce2: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed e968b5e36ae718feb7071249a7038f0e1300dce2 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] e968b5e

Verdict parsed from the review's SHA-scoped output markers for commit e968b5e36ae718feb7071249a7038f0e1300dce2.

False positive or not applicable? A repository writer can comment:
/ai-review override fable e968b5e36ae718feb7071249a7038f0e1300dce2: <one-sentence reason>

[chenmingwei23]

Addressed in b21ea28932051b978f1dee5a77f2b66d2bae6801.

GPT F1 (blocking) - pane link mutates global session state. The concern is real and I fixed it by construction, but I read the mechanism more narrowly than the prescription. A session chip is a "go to that session" affordance, and switching the active session is its intended effect - ChatPage's own rows do exactly dispatch(switchSlot(key)). The genuine hazard is the one Opus named precisely: the switch was ungated on connection, so an offline click would hit switchSlot.rejected, which clears the target slot's messages to [] and drops the reader onto the WelcomeView. That is the data-loss shape.

Fix, mirroring ChatPage exactly:

• ChatPane now reads useConnected(). selectSessionTab early-returns while offline (if (!connected || key === activeSlot) return), and the roster is passed as connected ? sessionRoster : undefined to both createTranscriptRenderers and <ChatMessageList>. With no roster, resolveSessionChip refuses, so an offline link stays external and no click can reach switchSlot in the first place - belt and suspenders.

I did not adopt the "host-owned rebind / per-pane navigation" prescription: rebinding a fixed-slot pane to a different slot on a link click is a larger behavioural change than the issue asks for, and it would diverge from the established ChatPage semantics that this PR is deliberately matching. Switching the active session in place is the shipped chip behaviour; gating it on connection is what makes it safe.

Opus (advisory) - same finding. Resolved by the same change: connected gate + connected ? sessionTitles : undefined.

First Principles (advisory) - two accuracy corrections, both fixed in the PR body:

1. The "exactly one real constructor" measurement undercounted - there is a second MessageRenderContext constructor at ChatPage.tsx:7377. Harmless to the change (the fields are optional and ChatPage overrides its own markdown rows), but the body now says two.
2. SideChat is a store-connected host (it imports the store selectors), not an embed host. The body no longer files it under the store-free constraint; it is recorded as a wireable host deliberately left on today's behaviour in this PR.

New test ChatPane.sessionLinkGate.test.tsx asserts the pane supplies the roster while connected and withholds it while disconnected.

[chenmingwei23]

Addressed in 3eb09f543e702f0869c2dcb81eece9bf6b0ba0a0 by removing the host wiring, not by either prescribed remedy.

The finding is correct on the merits. In a fixed-slot pane, dispatch(switchSlot(key)) changes the global chat.activeSlot but the pane keeps rendering its own slotKey, and switchSlot has no router/navigation side-effect - so an inline session-link click in a split-view pane (SessionGridView binds each pane to leaf.slot) or a member-DM pane (MembersPage binds the pane to a page-local slot, not chat.activeSlot) mutates global state with no visible effect in that pane. A callback that looks wired but silently does nothing in exactly the multi-pane cases that motivate a callback is worse than an honest external link. I under-weighted this in my prior reply; it stands.

I judged the two remedies and took neither:

• Routing through onOpenFull (which in SessionGridView is onCollapse) would tear down the user's entire split layout to full-screen on a single inline chip click - a large, surprising side-effect disproportionate to clicking a link.
• Navigating the embedded host to /chat reintroduces the leave-this-surface behaviour the in-place switch was meant to avoid, and there is no clean per-click host-navigation seam for the grid.

The correct in-place wiring is genuinely host-shaped (rebind the fixed pane to the target slot, or drive that host's own navigation), and it differs per host. That is a separate decision, so I removed the ChatPane wiring entirely rather than ship a host where the callback fires into a no-op.

What this PR now is: the interface addition #8254 actually asks about - MessageRenderContext / ChatMessageListProps gain the optional onSessionOpen / sessions / activeSession, and the three default markdown rows forward them, so a host that supplies them gets in-place switching and a host that omits them (every host today) is byte-for-byte unchanged. No host is wired, so the no-op case cannot arise. ChatPane is back to its main state; the connection-gate test that covered the removed wiring is removed with it.

Final diff: 3 files - messageRenderers.tsx, ChatMessageList.tsx, ChatMessageList.test.tsx.

[chenmingwei23]

First Principles is right, and I am conceding on the merits rather than pushing back.

The decisive evidence is the repo's own rule, which I verified at app-sdk/index.ts:507-514: MessageRenderContext is exported through this barrel, and the block immediately below it documents the policy for exactly this case (written for useComposerDraft) - do not publish SDK surface / freeze the contract "before its richest consumer ... has exercised it," because "Publishing later is additive; un-publishing is a break."

That rule binds here because I have also confirmed no host can exercise the seam correctly yet. In split mode ChatPage renders SessionGridView instead of the single-pane transcript, and the grid binds each pane to a fixed leaf.slot and never reads chat.activeSlot; switchSlot has no router side-effect. So a (key: string) => void that dispatches switchSlot from any current ChatMessageList host changes global state with no visible result - the shape is validated by nobody. Publishing it now would freeze an unexercised contract and pre-decide open RFC #8254 by default.

So the subtraction you prescribe is the correct one: drop the exported trio and land it inside the first per-host wiring PR that #8254 anticipates - additive at that point, and shaped by a consumer that actually works. I am taking this back to the pipeline to either close this PR or reduce it to nothing shippable-alone; I will not force the seam past a documented published-surface rule.

[chenmingwei23]

Standing down and closing this PR. First Principles' BLOCK is upheld on the merits, and the wiring turns out to be genuinely host-shaped, so the seam should not be published unexercised.

Evidence that no single onSessionOpen: (key: string) => void serves any current host correctly:

• SessionGridView pane (ChatPane.tsx:725) - the correct in-place switch is grid.fillLeaf(leaf.id, { kind: 'session', slot }) (the same call PlaceholderPane.onPick uses at SessionGridView.tsx:158). It needs the leaf.id (which pane), a grid concept ChatPane does not hold and ChatMessageList cannot see. dispatch(switchSlot(key)) is a no-op here because split mode renders the grid, which never reads chat.activeSlot. The trio's key-only shape cannot express fillLeaf(leaf.id, slot).
• MembersPage pane - binds to a page-local member selection, not chat.activeSlot; the correct switch is "select that member," a MembersPage concept, again not a bare slot-key callback.
• ChatEmbed.tsx:262 - fully store-free (zero store/dispatch/navigate references); no roster and no session-switch concept, so it cannot supply the trio at all.
• SideChat.tsx:598 - a side-question buffer, not a session navigator; a session link there is out of concept.

The dashboard's own rows already have this capability via transcriptRenderers.tsx precisely because ChatPage is the one surface where switchSlot is visible - and ChatPage does not use ChatMessageList. So the (key)=>void shape is validated by none of its actual hosts, and each host that might want it needs a different signature.

Per app-sdk/index.ts's own published-surface rule (do not freeze an SDK contract before its richest consumer has exercised it; publishing later is additive, un-publishing is a break), the seam should land alongside its first real consumer - a host-specific wiring PR that introduces the correctly-shaped callback (likely a grid-leaf rebind, not a bare key). The fields are optional, so adding them then is additive.

#8254 stays open for that first per-host wiring PR. Closing this one; nothing here is shippable alone.