fix(shared): html_render pages show for Grok threads#35434

merged2

main

factory/issue-23714-a17031

View on GitHubTwo-way sync enabledPR last imported 4h ago

Grok CLI reports MCP tool results as {type:'MCP', output:{OkayOutput:''}}. readResult in packages/shared/src/toolOutput.ts only went into structuredContent/content, so compactDynamicToolOutput found no htmlRender and htmlRenderFromToolItem returned undefined for Grok's html_render items. The fix: readResult now also descends into output.OkayOutput, with the existing depth, node and byte limits. Every consumer of htmlRenderFromToolItem (the web session-logic, mobile threadActivity, server attachmentStore, guest share and thread search) now builds the html-render entry for Grok. In the ACP adapter, the completed html_render branch now gets a reportedPage and claims the published page by attachmentId, so the wrapper-input mismatch the issue describes no longer matters on the success path. No adapter change was needed. There is no new regex or parser: the change is a property read on an already-parsed object, and the JSON.parse of the OkayOutput string runs under the existing MAX_PARSED_BYTES (16 KiB) / MAX_ENVELOPE_DEPTH (4) / MAX_ENVELOPE_NODES (128) limits, so adversarial input is linear in at most 16 KiB. I reproduced the bug with the branch's failing htmlRender.test.ts row (it returned undefined) before the fix. I did not rerun a live Grok turn against the fix.

Fixes pingdotgg/t3code#23714

Reproduction on main

On origin/main f03db4fbef, started a dev server with T3CODE_DEV_SEED=0 (fresh database), added a scratch git project with t3 project add, enabled a grok provider instance (driver grok) in the worktree's own .t3 settings, and ran node apps/server/src/bin.ts thread start --provider grok --model grok-4.7 --wait <project> "Show me a pie chart ... Use the t3-code html_preview tool, then the t3-code html_render tool ..." with Grok CLI 1.0.77. The turn completed: html_preview and html_render both succeeded, and the page was written to /.t3/userdata/attachments/-...-html.html. The stored turn item (type dynamic_tool, status completed, toolName t3-code.html_render) has input {tool_name:'t3-code__html_render', tool_input:{...}, variant:'UseTool'} and output {"type":"MCP","tool_name":"html_render","server_name":"t3-code","output":{"OkayOutput":"{"htmlRender":{"attachmentId":...},"message":"Shown to the reader above your reply..."}"}}. This is the reporter's envelope; the ACP adapter's takeHtmlRender fallback did not rewrite it. Opening the thread in the web client showed only the agent's reply 'The pie uses the made-up totals...' with no iframe or card above it. Setup note: on this VM, Grok 1.0.77 advertises only the 'grok.com' auth method while T3 requires the advertised 'xai.api_key' (raw ACP showed authenticate with xai.api_key works anyway). To open a session I disabled that check in AcpSessionRuntime.ts in my local checkout only; nothing on the html_render path was changed. Failing test: one row added to the htmlRenderFromToolItem table in packages/shared/src/htmlRender.test.ts; it returns undefined on main.

Evidence

Replayed the reported Grok MCP output.OkayOutput envelope with an identical synthetic chart attachment and completed reply on both commits. Opened the thread and waited for message syncing to finish. Environment: Chromium, 1280×800, light theme, fresh databases, seeding disabled.

The scenario failed on base and passed on head. Visual inspection confirmed no chart on base and the chart inline above the reply on head.

Before	After
Before: reply without chart	After: chart above reply
f03db4fbef	552bf9625d
Before recording	After recording

Scenario source

Scope: this proves rendering of the reported envelope through replay; it does not rerun live Grok authentication or execute html_preview/html_render themselves.

AI-assisted; verified with the tests and tools listed above.

Not shown:

Live Grok authentication and actual html_preview/html_render execution were not rerun. The scenario supplied the synthetic attachment and replayed the reported completed tool-result envelope through the real app.

Tests

vp test run src/htmlRender.test.ts (packages/shared): 1 failed before the fix, 16/16 passed after; vp test run src/htmlRender.test.ts src/toolOutput.test.ts: 28/28 passed; vp run check:changed: format, lint, knip, typecheck (35 packages) and tests (3 files, 30 tests) all passed.

Not in this change

The ACP adapter's takeHtmlRender fallback (used when no reference is reported) still passes Grok's {tool_name, tool_input, variant} wrapper as the page input; unwrapping tool_input there would only matter if Grok ever drops the result, and it isn't needed for this bug.
Grok's error variant of the envelope (presumably output.ErrorOutput) is not recognised as a failure by readResult; I didn't observe it in this report.

AI-assisted; verified with the tests and tools listed above.

Checks

27 successful · 15 skipped

All checks passed

Added 2 commits4h ago

fix(shared): html_render pages show for Grok threads552bf96
test(shared): html_render reference inside Grok's OkayOutput envelope699b8e1

MmacroscopeappBotapproved these changes4h ago

MmacroscopeappBot4h ago

Approvability

Verdict: Approved at 552bf96

Macroscope's review found this PR approvable — Grok-wrapped tool results now use the existing parsing path so HTML pages can appear in threads. Existing parsing limits, reference validation and tool-name checks remain in place.

^{You can add or adjust custom eligibility rules. Learn more.}

Ggithub-actionsBot4h ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.5 KiB	21.5 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	1	1	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	1	1	0 (0.0%)	8	✅

Other scenarios

All 64 scenario measurements match main.

All 64 scenario measurements

Provider	Scenario	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	`short turn resume`	`WebSocket decoded`	10.5 KiB	10.5 KiB	0 B (0.0%)	12.7 KiB	✅
Codex	`short turn resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`long turn running, cold open`	`HTTP decoded`	281.7 KiB	281.7 KiB	0 B (0.0%)	302.7 KiB	✅
Codex	`long turn running, cold open`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`long turn running, history page`	`HTTP decoded`	92.4 KiB	92.4 KiB	0 B (0.0%)	107.4 KiB	✅
Codex	`long turn running, history page`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`long turn finished, cold open`	`HTTP decoded`	381.5 KiB	381.5 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	`long turn finished, cold open`	`rows`	151	151	0 (0.0%)	151	✅
Codex	`long turn finished, socket cold open`	`WebSocket decoded`	381.6 KiB	381.6 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	`long turn finished, socket cold open`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`long turn finished, socket cold open`	`rows`	151	151	0 (0.0%)	151	✅
Codex	`long turn finished, work page`	`HTTP decoded`	186.2 KiB	186.2 KiB	0 B (0.0%)	200.2 KiB	✅
Codex	`long turn finished, work page`	`rows`	150	150	0 (0.0%)	150	✅
Codex	`idle thread resume`	`WebSocket decoded`	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	`idle thread resume`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Codex	`fleet shell snapshot`	`HTTP decoded`	41.6 KiB	41.6 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	`fleet shell snapshot`	`rows`	27	27	0 (0.0%)	27	✅
Codex	`fleet shell subscribe`	`WebSocket decoded`	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	`fleet shell subscribe`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Codex	`fleet shell resume check`	`HTTP decoded`	18 B	18 B	0 B (0.0%)	100 B	✅
Codex	`fleet shell resume check`	`rows`	0	0	0	0	✅
Codex	`stale shell reload`	`HTTP decoded`	41.6 KiB	41.6 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	`stale shell reload`	`rows`	27	27	0 (0.0%)	27	✅
Codex	`stale shell resume`	`WebSocket decoded`	140 B	140 B	0 B (0.0%)	400 B	✅
Codex	`stale shell resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`HTTP decoded`	72.4 KiB	72.4 KiB	0 B (0.0%)	83.0 KiB	✅
Codex	`stale todo list cold open`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`stale todo list cold open`	`nodes`	2	2	0 (0.0%)	4	✅
Codex	`stale todo list cold open`	`subagents`	2	2	0 (0.0%)	4	✅
Codex	`stale todo list cold open`	`plans`	1	1	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`providerTurns`	1	1	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`checkpoints`	1	1	0 (0.0%)	2	✅
Claude	`short turn resume`	`WebSocket decoded`	10.6 KiB	10.6 KiB	0 B (0.0%)	12.7 KiB	✅
Claude	`short turn resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`long turn running, cold open`	`HTTP decoded`	283.3 KiB	283.3 KiB	0 B (0.0%)	302.7 KiB	✅
Claude	`long turn running, cold open`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`long turn running, history page`	`HTTP decoded`	93.4 KiB	93.4 KiB	0 B (0.0%)	107.4 KiB	✅
Claude	`long turn running, history page`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`long turn finished, cold open`	`HTTP decoded`	384.2 KiB	384.2 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	`long turn finished, cold open`	`rows`	151	151	0 (0.0%)	151	✅
Claude	`long turn finished, socket cold open`	`WebSocket decoded`	384.3 KiB	384.3 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	`long turn finished, socket cold open`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`long turn finished, socket cold open`	`rows`	151	151	0 (0.0%)	151	✅
Claude	`long turn finished, work page`	`HTTP decoded`	188.1 KiB	188.1 KiB	0 B (0.0%)	200.2 KiB	✅
Claude	`long turn finished, work page`	`rows`	150	150	0 (0.0%)	150	✅
Claude	`idle thread resume`	`WebSocket decoded`	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	`idle thread resume`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Claude	`fleet shell snapshot`	`HTTP decoded`	42.2 KiB	42.2 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	`fleet shell snapshot`	`rows`	27	27	0 (0.0%)	27	✅
Claude	`fleet shell subscribe`	`WebSocket decoded`	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	`fleet shell subscribe`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Claude	`fleet shell resume check`	`HTTP decoded`	18 B	18 B	0 B (0.0%)	100 B	✅
Claude	`fleet shell resume check`	`rows`	0	0	0	0	✅
Claude	`stale shell reload`	`HTTP decoded`	42.2 KiB	42.2 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	`stale shell reload`	`rows`	27	27	0 (0.0%)	27	✅
Claude	`stale shell resume`	`WebSocket decoded`	140 B	140 B	0 B (0.0%)	400 B	✅
Claude	`stale shell resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`HTTP decoded`	73.4 KiB	73.4 KiB	0 B (0.0%)	83.0 KiB	✅
Claude	`stale todo list cold open`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`stale todo list cold open`	`nodes`	2	2	0 (0.0%)	4	✅
Claude	`stale todo list cold open`	`subagents`	2	2	0 (0.0%)	4	✅
Claude	`stale todo list cold open`	`plans`	1	1	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`providerTurns`	1	1	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`checkpoints`	1	1	0 (0.0%)	2	✅

Baseline: 0cd6a73 · PR result: 552bf96 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot4h ago

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

readResult now reads Grok CLI MCP results from output.OkayOutput when structuredContent and content are absent. A test checks that an html_render result in this envelope resolves to the expected reference.

Changes: Grok MCP result parsing

Layer / File(s)	Summary
Read the MCP result fallback `packages/shared/src/toolOutput.ts`, `packages/shared/src/htmlRender.test.ts`	`readResult` falls back to `output.OkayOutput` when `structuredContent` and `content` are absent. The test checks that the nested `html_render` result resolves to the expected reference.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: maria-rcks

Merge Risk: External image Minimal · up to 552bf

This change lets completed Grok html_render results display inline by reading Grok's MCP result envelope. No actionable merge-blocking risk was found. The fallback for results with no page reference and Grok's error envelope are still not handled, but the PR states these are out of scope.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title "fix(shared): html_render pages show for Grok threads" names the main change: Grok CLI MCP results now yield html_render pages. The raw summary confirms readResult in packages/shared/src/too…
Linked Issues check	Passed	The only coding requirement from `#23714` is that a completed Grok html_render result, which stores the htmlRender reference inside output.OkayOutput, resolves to an html-render entry so the page can sh…
Out of Scope Changes check	Passed	The PR changes two files. toolOutput.ts adds the OkayOutput read path that implements `#23714`. htmlRender.test.ts adds a regression row for that envelope. Both changes connect directly to the linked is…
Description check	Passed	The description explains the Grok CLI MCP envelope issue, the `readResult` fix, and the affected `html_render` consumers. It reports tests, screenshots, recordings, and checks not performed. The descr…

Full details: Title check

Explanation

The title "fix(shared): html_render pages show for Grok threads" names the main change: Grok CLI MCP results now yield html_render pages. The raw summary confirms readResult in packages/shared/src/toolOutput.ts reads the Grok wrapper (output.OkayOutput), and the htmlRender test covers the Grok case. It follows the conventional commit form the template recommends.

Full details: Linked Issues check

Explanation

The only coding requirement from #23714 is that a completed Grok html_render result, which stores the htmlRender reference inside output.OkayOutput, resolves to an html-render entry so the page can show inline. The diff in packages/shared/src/toolOutput.ts makes readResult read value.output.OkayOutput when structuredContent and content are absent. The result then passes through the existing reader, which keeps the depth, node, and byte limits. packages/shared/src/htmlRender.test.ts adds a Grok envelope case for htmlRenderFromToolItem. The PR description states that the web, mobile, server, and guest consumers call this shared function, so they receive the fix. The issue's fallback mismatch (wrapper input passed to takeHtmlRender) was marked UNCLEAR by the reporter and is not a stated requirement. The PR states it does not change that fallback. Limits: the PR reports no live Grok run after the fix, and I did not verify the consumer call sites. The PR description says the ACP adapter now gets a reportedPage. The diff for packages/provider-acp/src/server/adapter.ts is empty, so that statement does not match this PR. It does not change the verdict, because the success path depends on the shared reader.

Full details: Out of Scope Changes check

Explanation

The PR changes two files. toolOutput.ts adds the OkayOutput read path that implements #23714. htmlRender.test.ts adds a regression row for that envelope. Both changes connect directly to the linked issue. The adapter.ts diff is empty. The PR description's claim of an adapter change conflicts with the diff, but no unrelated code change was found. The PR also does not claim to fix the error envelope or the fallback wrapper, and it lists both as out of scope.

Full details: Description check

Explanation

The description explains the Grok CLI MCP envelope issue, the readResult fix, and the affected html_render consumers. It reports tests, screenshots, recordings, and checks not performed. The description is substantively complete, though it is lengthy and includes a duplicated AI-assisted note.

Finishing Touches

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: factory/issue-23714-a17031
File: .coderabbit.config.ts
Commit: 552bf96
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: `39bfb126-831e-4c69-9e28-7e2b477a1912`

External image Commits

Reviewing files that changed from the base of the PR and between 0cd6a73433c3b9c1e328560d591fd297e72d7ef9 and 552bf9625db09987c13db4b6992f8cfc5a34ac27.

External image Files selected for processing: 2

`packages/shared/src/htmlRender.test.ts`
`packages/shared/src/toolOutput.ts`

Limit details: You’ve used all 10 included reviews currently available.