Repository navigation
feat(native-chat): teach chat agents to show inline visuals in their own folder - #26099
Conversation
…s visuals folder
A shared grammar for the ::orca-visual{file="..." title="..."} reply line,
the per-chat visuals folder location on the owning host, and the
agentSession.readVisual runtime method that reads one visual with lexical and
canonical containment, a 512 KiB bounded read and UTF-8 refusal.
Native-chat assistant replies render a ::orca-visual{...} line as the chat's
HTML visual in an opaque, scripts-only sandboxed frame: CSP first, the host
frame navigation guard registered before content runs, live theme without a
reload, fitted height, links opened in the viewer's browser only from a real
gesture, lazy mount, and one muted line when the visual cannot be shown.
Open in sidebar shows the same frame in the right sidebar, widened while it
is open and restored after.
…own folder Native-chat Claude and Codex sessions now get a per-chat visuals folder on the host that runs them, write access to exactly that folder, its path in ORCA_CHAT_VISUALS_DIR, and an Orca skill that teaches the ::orca-visual line. - Skill ships as an unpacked plugin folder in desktop and headless builds. - Claude: --plugin-dir via SDK plugins, behind the CLI version probe (now one shared probe for every version-gated flag); folder added to additionalDirectories beside the user's own. - Codex: skills/extraRoots/set and the folder appended to the user's own writable roots, between initialize and the thread open, under a 2 s budget; unsupported, failed or hung setup opens the chat without visuals. - A host sweep removes folders no chat record maps to, and folders whose local workspace is provably removed; anything unproven is kept.
…er' into brennanb2025/inline-visuals-skill
…streaming hold Registers agentSession.readVisual from the methods index so the structured method file stays under its line budget, replaces reflective reads with checked narrowing, moves the pure height governor to src/shared for mobile, and holds a half-written directive tail while the turn works (structured text rows carry no running state).
Re-checks after the open that the chat's visuals folder is still the real directory at Orca's path, reports unexpected filesystem faults by code without host paths, and lets one click in a visual open at most one page.
… review fixes One shared helper drops visual lines (outside fenced code) from reply text where it becomes plain text: the structured status summary that feeds the sidebar row, dashboard, notifications, phone rows and handoffs, and AI Vault reply previews. Review fixes: height also counts a pinned body's overflow, only the live frontier row holds a half-written visual line, the runaway-height stop needs the same step repeated, and any host refusal evicts the cached revision.
- Visuals sweep: a workspace counts as removed only when no profile's catalog holds it (chats and visuals are shared by every profile, catalogs are not); a worktree in a known project is removed only when git no longer records it; any unreadable profile decides nothing; one catalog snapshot per run; a symlinked visuals root is never walked. - The other-profile catalog reader moves out of window/ and also returns project ids. - A folder Orca itself inherited is stripped at the spawn layer for both agents, not only from the launch overlay. - Claude version probe: a probe that gave no version is never remembered; the plugin check waits up to the probe's kill time so a slow first probe no longer costs a chat its skill. - Skill: kept out of Claude's / menu, filename and theme guidance matched to the renderer, refused writes are not retried. - Opt-in real Codex test for skill discovery and the writable root.
…arts The first chat after Orca starts usually finds the version known, so the plugin and thinking-display checks answer at once instead of probing a cold binary while the chat waits.
…nal-paths helper Main removed the per-chat journal paths and the journal database's state directory; the visuals folder keeps the same sha256 layout on its own and the read method uses the profile state directory the chat host is opened in.
…er' into brennanb2025/inline-visuals-skill # Conflicts: # src/main/claude/claude-structured-launch-resolution.ts # src/main/native-chat/native-chat-visuals-folder.ts # src/main/runtime/agent-session-record-store-file.ts # src/main/runtime/agent-session-record-store.ts # src/main/runtime/structured-agent-runtime-registrations.ts
…al lines Reply previews in Agent Session History drop visual lines per text part before lines are folded; the frame adds a body's overflow only when the body really overflows; fence tracking follows CommonMark closers and openers; the copy button copies a reply without visual lines; a coded read fault keeps its cause.
- Read git worktree records written relative to their own folder (git 2.48+), so a worktree on an unmounted drive in such a repo is still kept. - Skip the running profile by its own storage folder, not the profile index a switch rewrites first; a profile never written to counts as empty, so the workspace rule is not switched off by a profile that was never opened. - Ask for readable thinking again once the plugin check has waited for the version, so a slow first probe no longer drops it for the chat's life. - Launch flag decisions move to their own module; tests use a typed record fixture instead of casts.
…er' into brennanb2025/inline-visuals-skill
- Resolve a relative git worktree record against its folder's real path, so a project added through a link still matches and its worktree is kept. - A profile with only backups of its data file is a lost file, not a fresh profile: it still stops the workspace rule. - The readable-thinking re-check reads what is known and never starts a second version probe.
…er' into brennanb2025/inline-visuals-skill
… visuals folder pair at once
…er' into brennanb2025/inline-visuals-skill
…on agent-session surface
…er' into brennanb2025/inline-visuals-skill
Review summaryReview rounds (Claude reviewers, independent of the author)Round 1, two reviewers in parallel (correctness/design; deletion and grant safety) on the feature commit:
Round 2, confirmation: all round-1 fixes confirmed. Four P3s fixed:
Round 3, confirmation: no P1/P2. Three P3s fixed:
Live QAIsolated hidden rig built from the branch, with a stand-in Claude by absolute path (never a real agent) and hooks off. The UI was driven by a separate QA agent (Codex) through Chrome DevTools; no desktop input was used.
Real CLIs (opt-in, real login for Claude, isolated home for Codex)
The real Local gate (on 01b0441; the later merges only brought base-PR fixes)
CI (classified against main)After #26103 was squash-merged, this branch still carried its original commits, so the diff against main included the rendering work. The PR now targets main and contains only its own 46 changed files: the skill, folder delivery, grants, and cleanup. Coordinator-approved final main merge: merged Final CI: 35 passed, 17 skipped, 0 failed, 0 pending. All ten unit-test shards, their verification and selection checks, static analysis/typechecks, relay integration, cross-version compatibility, both packages, all SSH host checks, and native smoke checks passed. GitHub reports the PR as mergeable and clean. Completed checks. Issues from this head, classified against merged main:
Local validation on this pushed head passed:
The PR remains ready and unmerged, with auto-merge absent. No app, rig, or agent CLI was launched for this update; the earlier live QA and real-CLI results above are historical evidence. Earlier reds, by owner:
e2e: skipped by the workflow for this head; not a passing rendered-UI test. Not verified
|
|
CI after marking ready (same head, d392323): all 26 checks that ran passed.
e2e was skipped again, by design: the E2E routing ( |
Removing visual lines now closes only the gap each removal leaves, instead of collapsing blank lines across the whole reply and trimming its indentation; the visuals folder is checked parent first again so a broken path answers the same way every time.
…er' into brennanb2025/inline-visuals-skill
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change adds a packaged native chat visuals skill and delivers private, session-specific visual folders to Claude and Codex launches. Claude now uses shared version-gated CLI flag detection for thinking display and plugin loading. Codex receives skill-root and writable-root thread configuration. The runtime schedules cleanup of unused visual folders using held sessions, profile catalogs, filesystem checks, and Git worktree records. Packaging and integration tests cover the new behavior. Priority: ➖ Normal Merge Risk: 🔵 Low · up to The change is mergeable with awareness that running the affected tests on Windows may require symlink permissions. The inspected PR Windows checks do not run those tests. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8613dcde-3a9d-49eb-8d73-f511f03508f5
⛔ Files ignored due to path filters (1)
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (94)
config/electron-builder.config.cjsconfig/scripts/build-orcad.mjsresources/native-chat-visuals/.claude-plugin/plugin.jsonresources/native-chat-visuals/skills/orca-chat-visuals/SKILL.mdsrc/main/ai-vault/session-scanner-accumulator.tssrc/main/ai-vault/session-scanner-text-normalization.tssrc/main/ai-vault/session-scanner-values.test.tssrc/main/claude/claude-child-process-environment.tssrc/main/claude/claude-cli-flag-prewarm.test.tssrc/main/claude/claude-cli-flag-prewarm.tssrc/main/claude/claude-cli-flag-support.test.tssrc/main/claude/claude-cli-flag-support.tssrc/main/claude/claude-structured-launch-flags.tssrc/main/claude/claude-structured-launch-resolution.test.tssrc/main/claude/claude-structured-launch-resolution.tssrc/main/claude/claude-structured-launch-visuals.test.tssrc/main/claude/claude-structured-real-cli-visuals.test.tssrc/main/claude/claude-thinking-display-support.test.tssrc/main/claude/claude-thinking-display-support.tssrc/main/codex/codex-structured-child-environment.tssrc/main/codex/codex-structured-launch-resolution.tssrc/main/codex/codex-structured-real-cli-visuals.test.tssrc/main/codex/codex-structured-session-acquire.tssrc/main/codex/codex-structured-session-state.tssrc/main/codex/codex-structured-thread-open.tssrc/main/codex/codex-structured-visuals.test.tssrc/main/codex/codex-structured-visuals.tssrc/main/native-chat/agent-session-record-test-fixture.tssrc/main/native-chat/native-chat-visual-file-read.test.tssrc/main/native-chat/native-chat-visual-file-read.tssrc/main/native-chat/native-chat-visuals-delivery.test.tssrc/main/native-chat/native-chat-visuals-delivery.tssrc/main/native-chat/native-chat-visuals-folder.tssrc/main/native-chat/native-chat-visuals-skill-location.tssrc/main/native-chat/native-chat-visuals-sweep.test.tssrc/main/native-chat/native-chat-visuals-sweep.tssrc/main/orca-profiles/other-profile-workspace-catalog.test.tssrc/main/orca-profiles/other-profile-workspace-catalog.tssrc/main/runtime/agent-session-record-store.tssrc/main/runtime/agent-session-store-state.tssrc/main/runtime/claude-structured-session-integration.test.tssrc/main/runtime/native-chat-visuals-workspace-verdict.test.tssrc/main/runtime/native-chat-visuals-workspace-verdict.tssrc/main/runtime/orca-runtime-get-worktree-ps.tssrc/main/runtime/rpc/methods/index.tssrc/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.tssrc/main/runtime/rpc/methods/structured-agent-session-visual.test.tssrc/main/runtime/rpc/methods/structured-agent-session-visual.tssrc/main/runtime/structured-agent-runtime-registrations.tssrc/main/runtime/structured-agent-session-runtime-teardown.tssrc/main/runtime/structured-agent-session-runtime.tssrc/main/runtime/structured-claude-runtime-adapter.tssrc/main/runtime/structured-claude-thinking-display-refusal.test.tssrc/main/window/history-gc-worktree-ids.tssrc/main/window/host-frame-navigation-guard.test.tssrc/main/window/host-frame-navigation-guard.tssrc/main/window/main-window-webview-security.test.tssrc/main/window/main-window-webview-security.tssrc/renderer/src/components/native-chat/NativeChatInlineVisual.tsxsrc/renderer/src/components/native-chat/NativeChatMarkdown.tsxsrc/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsxsrc/renderer/src/components/native-chat/NativeChatMessageRow.test.tsxsrc/renderer/src/components/native-chat/NativeChatMessageRow.tsxsrc/renderer/src/components/native-chat/NativeChatTranscriptChrome.tsxsrc/renderer/src/components/native-chat/NativeChatView.tsxsrc/renderer/src/components/native-chat/NativeChatVisualFrame.test.tsxsrc/renderer/src/components/native-chat/NativeChatVisualFrame.tsxsrc/renderer/src/components/native-chat/NativeChatVisualPanel.tsxsrc/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsxsrc/renderer/src/components/native-chat/native-chat-visual-markdown-syntax.test.tsxsrc/renderer/src/components/native-chat/native-chat-visual-markdown-syntax.tssrc/renderer/src/components/native-chat/native-chat-visual-owner.tsxsrc/renderer/src/components/native-chat/native-chat-visual-read-client.test.tssrc/renderer/src/components/native-chat/native-chat-visual-read-client.tssrc/renderer/src/components/native-chat/use-native-chat-visual-document.tssrc/renderer/src/components/native-chat/use-native-chat-visual-theme.tssrc/renderer/src/components/right-sidebar/index.tsxsrc/renderer/src/components/right-sidebar/right-sidebar-panel-content.tsxsrc/renderer/src/components/right-sidebar/right-sidebar-width.tssrc/renderer/src/components/sidebar/CommentMarkdown.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/store/slices/editor/actions/right-sidebar-state.tssrc/renderer/src/store/slices/editor/actions/right-sidebar-visual-state.test.tssrc/shared/native-chat-visual-directive.test.tssrc/shared/native-chat-visual-directive.tssrc/shared/native-chat-visual-height-governor.test.tssrc/shared/native-chat-visual-height-governor.tssrc/shared/native-chat-visual-shell.test.tssrc/shared/native-chat-visual-shell.tssrc/shared/orcad-artifacts.tssrc/shared/rpc-contract/agent-session-visual-params.tssrc/shared/structured-agent-session-latest-request.test.tssrc/shared/structured-agent-session-latest-request.tstests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
💤 Files with no reviewable changes (2)
- src/main/claude/claude-thinking-display-support.test.ts
- src/main/claude/claude-thinking-display-support.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| it('never touches entries it did not mint, symlinks included', async () => { | ||
| const state = stateDirectory() | ||
| const root = nativeChatVisualsRootFor(state) | ||
| mkdirSync(join(root, 'notes'), { recursive: true }) | ||
| writeFileSync(join(root, 'f'.repeat(32)), 'a file, not a folder') | ||
| const target = mkdtempSync(join(tmpdir(), 'orca-visuals-target-')) | ||
| scratch.push(target) | ||
| writeFileSync(join(target, 'keep.txt'), 'keep') | ||
| symlinkSync(target, join(root, 'e'.repeat(32))) | ||
| await sweepNativeChatVisualsFolders(deps(state, [])) | ||
| expect(existsSync(join(root, 'notes'))).toBe(true) | ||
| expect(existsSync(join(root, 'f'.repeat(32)))).toBe(true) | ||
| expect(existsSync(join(root, 'e'.repeat(32)))).toBe(true) | ||
| expect(existsSync(join(target, 'keep.txt'))).toBe(true) | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Skip the symlink tests on Windows. Three new tests call symlinkSync. On Windows CI runners without symlink permission, that call fails with EPERM.
src/main/native-chat/native-chat-visuals-sweep.test.ts#L113-L127: changeit(toit.skipIf(process.platform === 'win32')(.src/main/native-chat/native-chat-visuals-sweep.test.ts#L171-L179: changeit(toit.skipIf(process.platform === 'win32')(.src/main/runtime/native-chat-visuals-workspace-verdict.test.ts#L212-L230: changeit(toit.skipIf(process.platform === 'win32')(.
Based on learnings: tests that create filesystem symlinks "should be guarded with it.skipIf(process.platform === 'win32')".
📍 Affects 2 files
src/main/native-chat/native-chat-visuals-sweep.test.ts#L113-L127(this comment)src/main/native-chat/native-chat-visuals-sweep.test.ts#L171-L179src/main/runtime/native-chat-visuals-workspace-verdict.test.ts#L212-L230
Source: Learnings
ELI5
The base PR (branch
brennanb2025/inline-visuals-render) lets Orca's native chat show an HTML chart or diagram inside a reply when the reply contains a::orca-visual{file="…"}line. But nothing told the agent it could do that, where to put the HTML file, or let it write there. This PR closes that gap. Every native chat now gets its own private folder for visuals. The agent may write into exactly that folder, it is told where the folder is, and it gets a short Orca skill explaining when and how to make a visual. The folder is cleaned up once its chat is gone.What Changed
Before: a Claude or Codex native chat didn't know inline visuals existed. Even if it guessed the syntax, it had nowhere sanctioned to put the HTML file: writing into the user's repo would dirty git status, and writing anywhere else needed approval or failed.
After, what you see: ask a native-chat Claude or Codex for something easier to see than read (say "compare p95 latency by region") and it can write one self-contained HTML page and end its reply with a visual line. The base PR renders it in the reply and in the right sidebar. Nothing is written into your project. In plan or read-only mode, or when any piece below is unavailable, the agent answers with a Markdown table, a Mermaid block or prose instead, as the skill tells it to.
Mechanism:
<Orca data>/native-chat-visuals/<hash of the chat id>/(the base PR's resolver) and is created at launch, readable only by the user. If creating it fails, the chat still starts, without visuals.--add-dirthe user configured.sandbox_workspace_write.writable_roots. Codex replaces that list instead of merging it, so Orca first reads the user's own effective roots (config/readfor that working directory) and keeps them. If they can't be read, Orca overrides nothing; a write there then asks for approval like any other outside path. Under full access nothing is added, because everything is writable already.ORCA_CHAT_VISUALS_DIRand never into the skill text. A value Orca itself inherited (say, Orca started from inside a chat) is stripped from both agents' environments, so a chat can never be pointed at another chat's folder.SKILL.md, shipped as an unpacked plugin folder (resources/native-chat-visuals/, Claude plugin layout). It ships in the desktop app's resources and in the headless server (orcad) build and install manifest. It is resolved on the host that starts the agent, and a packaged app never loads it from a checkout.pluginsentry (--plugin-dir) when the installed CLI accepts that flag. The flag first appears in Claude Code 2.0.25; I read the published packages, and 2.0.24's option parser has no such option. The skill is markeduser-invocable: false, so it stays out of the/command menu while the model still sees it.--thinking-displaynow serves every version-gated flag (one probe per binary and folder). Only a confirmed version is remembered. A probe that times out or fails is forgotten, so the next launch asks again instead of latching "unsupported" at boot. The probe also starts when native chat starts: in the home folder, which loads the binary from a cold disk, and in the folders of open Claude chats pinned to one (capped at 4). The answer is kept per folder, because a version manager's shim can pick a different CLI per project. So a chat in a folder not yet asked still runs its own probe, but against a warm binary (tens of milliseconds). Readable thinking keeps its 1.5 s budget, and is asked again once the longer skill wait has learned the version, so a slow first probe no longer drops it.skills/extraRoots/setwith the skill root, sent afterinitializeand before the thread opens, on every start, resume or respawn. Each chat has its own app-server, so the process-wide list is exactly Orca's roots. Both Codex requests run in parallel under a 2 s budget instead of the 30 s request default. An unsupported method (older Codex answers "unknown variant", newer "method not found"), an error or a hang leaves the chat working without the skill.Why
.gitignoreor git-exclude edits, and the folder can be cleaned up with the chat.Worst-case delay before a chat starts
claude --versiontakes that long or hangs. The chat then starts without the skill, nothing is remembered, and the next launch asks again. The user's prompt is never held longer than that, plus at most 0.25 s for the thinking re-check.Differences from the common pattern
Linked Issue
N/A (maintainer). Part of the inline chat visuals work, stacked on
brennanb2025/inline-visuals-render.Visual Proof
Cold-start run on the final head (ab3e2d3): a freshly started, isolated, hidden Orca built from this branch. Its Claude is a stand-in program, not a real agent, that writes one chart into the folder named by
ORCA_CHAT_VISUALS_DIRand ends its reply with the visual line. The UI was driven by a separate QA agent through Chrome DevTools on the hidden window.First chat after the cold start: the folder, the grant and
--plugin-dirwere all handed over, and the chart renders inline. No raw::orca-visualtext appears anywhere on the page, the sidebar included.A second chat in the same app:
Open in sidebar, from an earlier run:
Testing
Platforms actually run: macOS only. Windows/Linux are covered by the cross-platform code paths and unit tests, not run live.
Unit and integration tests: new tests for the folder, env and skill delivery, the shared Claude version check (no latching of failures, per-call budget, prewarm), the Codex setup (unsupported, failed, hung, full access, user roots kept), the sweep (orphans, cross-profile, unmounted drive via git records, symlinks, failures) and the other-profile catalog. 193 explicit test files that import the changed modules: 192 pass, 1 file skipped as on the base branch.
Real CLIs, opt-in: run with
ORCA_REAL_CLAUDE_CLI_TEST=1/ORCA_REAL_CODEX_CLI_TEST=1:claude-structured-real-cliand-foldsuites pass.orca-chat-visualsfrom its skill catalog. The skill appears in neither the CLI's skill list nor its command list, so it never reaches the/menu.~/.codexand~/.claudesettings hashes were unchanged across the runs.Live app, cold start: hidden isolated rig built from this branch, with a stand-in Claude and a separate QA agent driving the UI through Chrome DevTools.
--plugin-dir, and its chart rendered. So did a second chat.Not verified:
thread/resume. A fresh thread has nothing to resume, so this would need a real model turn; it is covered by unit tests against the request shape.I manually tested these changes locally
Automated tests added/updated
Review
See the review-summary comment.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation. (The skill text is original.)Notes
node:path; path comparison uses the shared normalizer; the env-var strip handles Windows key casing; the folder is owner-only on POSIX.mkdirper launch. The Claude version check is shared with the existing one and prewarmed. Codex setup is two local requests in parallel (4 ms against Codex 0.159.0). The sweep is bounded and runs rarely.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Author: @BrennanKB5