Feature/chat view#432
Open
Cheggin wants to merge 33 commits into
Open
Conversation
Adds summarizeBashCommand and summarizeBrowserHarnessJs — pure-regex pattern
matchers that map common shell commands and CDP calls to plain-English labels
("Read AGENTS.md", "Visited https://x.com", "Connected to browser") for the
chat-view tool pills.
Patterns cover the cat/sed/head/tail/ls/find/grep family, git, package
managers, mkdir/mv/cp/rm, curl/wget, cd-prefix stripping, and the
browser-harness-js JS payload (Page.navigate / captureScreenshot / Input.*
/ DOM.* / connectToAssignedTarget). Only maps when the binary or method
alone unambiguously determines the action; anything ambiguous falls through
to null so the caller keeps the raw "Ran command" UI.
Includes a corpus test that replays every distinct bash call from the local
sessions.db through the summarizer to measure coverage on real agent output.
Adds ToolGroup, a wrapper that batches consecutive tool calls in an agent
turn into a single inline summary. While any tool in the group is in-flight
the header shows the current tool's active label ("Reading file…",
"Visiting x.com…") with a spinner; once complete it settles into a deduped
prose summary ("Connected to browser, read file, and ran 1 command"). Click
to expand and see each tool inline as a textual outline tied to the summary
by a left rule.
Unrecognized bash entries are pooled into "ran N command(s)" rather than
showing raw shell. Specific identifiers (URLs, filenames, branches) stay in
the value slot when meaningful; generic nouns (browser, page, tests) fold
into the bold label so the chip never reads as "Bold verb — muted noun".
When a session is opened in grid mode (typically because the user clicked Switch to browser from the chat view), there was no in-app affordance to get back to the chat. Adds an optional onOpenChat handler on AgentPaneProps and renders a "Back to chat" pill in the action row when it's provided. Button is conditional on the prop, so AgentPane consumers that don't pass it (none today besides HubApp's grid mode) are unaffected.
zustand + immer back the new sessions and UI stores; prism-react-renderer syntax-highlights bash, json, yaml in the chat tool-call expanded panels.
Sessions store is normalized (byId + order) with immer mutations so per-session updates stay referentially stable. The bridge owns the event-stream subscription: listAll() on mount for hydrate, then per-event patches via session-output (mirroring the logs pane's path) plus session-updated for non-output fields. hasBrowser is owned exclusively by sessionBrowserAttached / sessionBrowserGone — never overwritten by session-updated payloads which carry it as undefined. UI store is intentionally narrow: only chatSessionId persists, viewMode stays in HubApp local state to avoid dual-persistence drift.
Adds SessionScreencast manager that polls Page.captureScreenshot via webContents.debugger every 1s and forwards JPEG bytes to the renderer as session-preview-frame(id, base64). captureScreenshot is used instead of Page.startScreencast so frames flow regardless of whether the view is currently attached to the window — the renderer-level paint doesn't need an on-screen compositor surface. Adds BrowserPool.setOnCreate listener, wired in main to emit sessions:browser-attached when a new view is registered. The renderer flips hasBrowser to true the moment a browser is acquired mid-session, without waiting for the next listAll snapshot. Mirror of the existing sessions:browser-gone path. Debugger is left attached across stop/start cycles to avoid CDP in-flight command loss under React StrictMode setup/cleanup churn.
ChatPane renders a header (back / title / status / cost), a transcript scrolling at viewport width with content centered in a 780px column, and a composer at the bottom wired to electronAPI.sessions.resume. ChatTranscript subscribes via useShallow to a narrow slice of the sessions store (output + createdAt + status). Scroll-pinning rule: stick to bottom when within 32px of bottom, release on scroll-up, force pin on a new user_input turn. Renders a terminal-spinner + live elapsed counter while status is running. ChatTurn batches consecutive tool_call entries into a ToolGroup so a long agent turn renders as one collapsed chip rather than a stack of pills. UserBubble clamps long messages with a Show more affordance and a copy button. groupIntoTurns is a pure function (entries → Turn[]) covered by tests in a follow-up commit. chat.css scopes typography, normalized rounding, and theme tokens (uses --color-bg-elevated to differentiate from the sidebar's sunken surface).
…eview
ToolBlock renders a single tool_call as a small inline pill with the
friendly verb-tense label (Running command / Ran command) and a
primary-parameter summary in mono. Expanded, it shows a Command block
and an Output block. The Output block detects markdown (rendered via
the existing Markdown component), JSON (pretty-printed + prism), or
yaml-style (used when JSON has any multi-line string value, so embedded
\n in fields like bodyText render as real line breaks rather than
literal escapes). Generic tools render args+result as pretty JSON.
CodeBlock is the building block — bordered header bar (label + copy),
body with prism-react-renderer (oneDark/github theme, autodetected from
--color-bg-base brightness). New asMarkdown mode swaps the body for
the Markdown component with chat-scoped font-size overrides.
toolLabels mirrors browser_use_cloud's tool-labels.ts — TOOL_TYPES /
TOOL_LABELS / getToolDisplayValue + parseBashResult that unwraps codex
{stdout, stderr, aggregated_output, exit_code} envelopes (regex-based
fallback for the case where codex slices preview to 2000 chars and
truncates the JSON tail).
TerminalSpinner is a JS-driven braille frame cycler ⠋⠙⠹⠸…, paired
with Elapsed which ticks an / counter every second.
BrowserPreview subscribes to hasBrowser from the sessions store, starts
the screencast when true via sessions.previewStart, listens for
session-preview-frame events, and renders the latest base64 JPEG as a
200×125 thumbnail above the composer. Click triggers a 220ms zoom-out
animation then onExpand — the parent switches to grid mode.
Extends ViewMode with chat. Mounts useSessionsBridge once at the root. Dashboard submit, dashboard session selection, and sidebar row clicks all enter chat for the chosen session via enterChat(id), which writes chatSessionId to uiStore and flips viewMode. Browser views are now hidden everywhere except grid (was: everywhere except settings) so they don't bleed through the chat UI. ChatPane receives onExit (sets viewMode to dashboard) and onSwitchToBrowser (sets viewMode to grid for the same session).
Covers empty input, single-turn grouping, multi-turn user_input boundaries, the leading null-user turn for entries that arrive before the first user_input, and consecutive user_inputs producing empty-agent-entry turns.
Hook the existing ToastProvider up to user-visible save/sync actions so changes get explicit confirmation instead of dying silently. Restyle the toast itself to match the app's frosted-glass surfaces — layered shadow, backdrop blur, and per-variant tinted borders and glows.
Adds a Linkify component that scans plain-text chat content (thinking blocks, error messages, notify chips, file_output text) for paths shaped like `outputs/<id>/<file>.<ext>` and turns them into clickable links. Click invokes `sessions:reveal-output` (already exposed via preload as `electronAPI.sessions.revealOutput`), which uses Electron's `shell.showItemInFolder` — Finder on macOS, Explorer on Windows, the desktop environment's file manager on Linux. No renderer-side platform branching required. Detection is intentionally narrow: only paths that resolve inside the harness outputs root, which is the only thing the IPC handler will reveal. Other paths fall through as plain text so clicks are never misleading. Inline styles keep the component self-contained. The renderer integration (wrapping ChatTurn's plain-text branches with <Linkify>) lives in the working tree alongside other in-progress edits to that file; it'll land in the next commit that takes ChatTurn.
# Conflicts: # app/src/renderer/hub/HubApp.tsx
- Replace plain text engine label with provider logo (claude-code, codex, browsercode) - Add SUBSCRIPTION/KEY badge and cost chip mirroring AgentPane - Status pill border now picks up the status color (running/idle/stuck/paused/stopped) - Remove redundant back button
- Add BU_OUTPUTS_DIR env so the harness can write screenshots into the session's watched outputs dir (<userData>/harness/outputs/<sid>/). - Log every file detected by the outputs watcher with abs path, bytes, and mime so users can locate artifacts. - Register a chatfile:// protocol scoped to the harness outputs root, so the renderer can <img src=chatfile://...> safely. Path is canonicalized via realpathSync and rejected if it escapes the root. - Allow chatfile: in the hub CSP img-src. - AGENTS.md: explain when to save (judgment-based — task confirmation, unexpected findings, stuck states, mid-progress checkpoints) vs. keep in memory (selectors / state inspection).
- StreamingProse + useTypewriter: smooth out chunky provider streams (claude-code, codex, browsercode) by revealing chars at a steady rate with adaptive catch-up. Same component spans the thinking->done swap (stable key) so the cursor doesn't reset. - stableMarkdown: temporarily close unbalanced code fences / inline ticks while typing so markdown rendering stays locally stable instead of re-rewriting as new chars arrive. - Skip the typewriter entirely on re-entry to settled turns — drove the 'everything re-streams' bug when reopening a finished chat. - Hoist image file_outputs into the trailing done block as floated insets (magazine layout). Defer rendering until done lands so the image appears in its final position, not as a standalone block that later jumps. - 'Working' indicator now lives at the top of the live agent turn and never hides — old logic toggled it off whenever the last entry was an in-flight tool_call, which made it flicker on/off as tool calls resolved. - Narrow transcript column to 640px, stable scrollbar gutter. - Strip tool pill chrome (no border/bg/icon) so solo tools and grouped tools read as the same plain-text outline. - Bump assistant prose weight and size for readability.
Adds the text-selection-to-quote flow, ported from browser_use_cloud/.../quote-selection-button.tsx: - useTextSelection hook listens to selectionchange + mouseup, scoped to a containerRef so only selections inside the transcript trigger the floating button (composer textarea and sidebar are excluded). - QuoteSelectionButton is a fixed-position floating action anchored to the selection's bounding rect (centered above, falls back below when clipping the viewport top, clamped horizontally). - parseUserMessage / formatUserMessageWithQuote handle the wire format: consecutive '> ' lines, blank line separator, then the user message. Round-trips through sessions.resume unchanged. - TaskInput gained an optional topSlot prop. ChatPane uses it to render the dismissible quote preview chip *inside* the input's bordered box so it visually extends the textarea rather than floating above. - UserBubble parses the quote prefix and renders it in a styled blockquote block above the actual user message text. Long-message clamp now bases on the message body so the quote doesn't inflate the line count. Composer layout fix: - chat-preview-rail moved inside chat-composer so they share a stacking container. composer is display:flex column with gap, so the preview thumbnail naturally sits above the TaskInput and shifts up as the composer grows (quote chip, multi-line input). Previously the rail was absolutely positioned at a fixed bottom:86px and got obscured by the composer once it grew taller than that. - chat-preview-rail gains padding: 0 var(--space-3) so its left edge aligns with the visible TaskInput box (which is inset from .task-input by --space-3 on each side per hub.css).
Typewriter was overwriting shownLenRef with stale React state on every render, slamming the reveal back to ~3 chars per commit. Refs are now written only by the raf loop; shrink/resume handled in effects. Rerun clears session.output in main but emits via session-updated, which the bridge strips output from. Detect rerun via createdAt bump and reset output alongside the patch.
…o feature/chat-view
Add the product blueprint, product soul, and user-transcript handoff docs so the branch keeps the direction artifacts separate from implementation work. Constraint: User explicitly asked to keep the PRODUCT_ markdown files while restarting the code changes Confidence: high Scope-risk: narrow Tested: Documentation-only commit; no runtime tests required Not-tested: Rendered documentation preview
The chat edit affordance needs a renderer-safe API that can rerun an existing session with updated prompt text. Route editAndRerun through the existing rerun path and persist the prompt override before clearing the transcript. Constraint: Keep this to the first-message rerun path; follow-up truncation/resume was not restored from the stash Rejected: Reapply the larger edit-and-resume stash path | it pulled in broader session-history changes outside the requested UI restore Confidence: high Scope-risk: moderate Tested: yarn vitest run tests/unit/sessions/SessionManager.persistence.test.ts Tested: yarn typecheck Not-tested: Manual Electron edit-rerun flow
Recover the stash's chat-view UI work while deliberately leaving the committed typewriter and Markdown streaming path intact. New sessions now open in chat, the center view toggle is gone, the composer locks to the session engine, terminal quoting can seed a dashboard prompt, latest-turn scrolling gets stable spacing and timestamps, browser previews wait for real navigations, and chat rows regain edit, action, and file-card polish. Constraint: Do not reintroduce the depth-trail or Streamdown streaming experiment Rejected: Checkout the stash's Markdown.tsx and depth-trail ChatTurn/css hunks | user said the old committed streaming felt better Confidence: high Scope-risk: moderate Tested: yarn typecheck Tested: yarn eslint src/renderer/components/empty/ErrorBoundary.tsx src/renderer/hub/Dashboard.tsx src/renderer/hub/HubApp.tsx src/renderer/hub/TaskInput.tsx src/renderer/hub/chat/BrowserPreview.tsx src/renderer/hub/chat/ChatPane.tsx src/renderer/hub/chat/ChatTranscript.tsx src/renderer/hub/chat/ChatTurn.tsx src/renderer/hub/chat/QuoteSelectionButton.tsx src/renderer/hub/chat/ToolGroup.tsx src/renderer/hub/state/sessionsStore.ts src/renderer/hub/state/uiStore.ts src/renderer/hub/types.ts Not-tested: Manual Electron chat UX pass Co-authored-by: OmX <omx@oh-my-codex.dev>
Replace the long denylist with an allowlist for .vite output so packaged app contents follow the Electron Forge Vite build contract and do not depend on maintaining individual source, test, and doc excludes. Constraint: Production externals are restored by packageAfterPrune and app-update.yml is copied as a resource Rejected: Maintain a growing ignore regex denylist | it is easy to miss new dev-only files Confidence: medium Scope-risk: moderate Tested: yarn typecheck Not-tested: Full Electron Forge package build Co-authored-by: OmX <omx@oh-my-codex.dev>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
12 issues found across 52 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/chat/chat.css">
<violation number="1" location="app/src/renderer/hub/chat/chat.css:865">
P2: Scope the new markdown table selectors to chat (for example under `.chat-pane`) to avoid global style leakage into other hub markdown views.</violation>
</file>
<file name="app/src/renderer/hub/chat/CodeBlock.tsx">
<violation number="1" location="app/src/renderer/hub/chat/CodeBlock.tsx:85">
P2: Code block syntax theme can become stale after a theme switch because it isn’t driven by reactive theme state.</violation>
</file>
<file name="app/src/main/index.ts">
<violation number="1" location="app/src/main/index.ts:1308">
P1: Edited rerun prompts are ignored at execution time because `runEngine` uses a stale `session.prompt` snapshot captured before `rerunSession` applies `promptOverride`.</violation>
</file>
<file name="app/src/renderer/hub/state/useSessionsBridge.ts">
<violation number="1" location="app/src/renderer/hub/state/useSessionsBridge.ts:32">
P1: Avoid full-store hydrate here; it can overwrite live IPC updates that arrived before `listAll()` resolves.</violation>
</file>
<file name="app/forge.config.ts">
<violation number="1" location="app/forge.config.ts:76">
P1: The new `ignore` function filters out `/package.json`, but later packaging hooks read `buildPath/package.json` unconditionally, which can break `electron-forge package/make` with a missing-file error.</violation>
</file>
<file name="app/src/renderer/hub/chat/QuoteSelectionButton.tsx">
<violation number="1" location="app/src/renderer/hub/chat/QuoteSelectionButton.tsx:48">
P2: Clamp `maxLeft` to at least the horizontal margin; otherwise very long labels or narrow viewports can push the floating button off-screen.</violation>
</file>
<file name="app/src/renderer/hub/TaskInput.tsx">
<violation number="1" location="app/src/renderer/hub/TaskInput.tsx:250">
P2: `topSlot` is rendered in the wrong position relative to attachment chips. Move it below the chips block to match the intended layout and prop contract.</violation>
</file>
<file name="app/src/main/sessions/SessionScreencast.ts">
<violation number="1" location="app/src/main/sessions/SessionScreencast.ts:30">
P2: `maxWidth`/`maxHeight` are exposed and logged but never used in screenshot capture, so the new config is non-functional.</violation>
</file>
<file name="app/src/renderer/hub/HubApp.tsx">
<violation number="1" location="app/src/renderer/hub/HubApp.tsx:78">
P2: Closing the keybindings overlay can wrongly re-show BrowserViews in chat/dashboard, conflicting with the new grid-only visibility behavior.</violation>
</file>
<file name="app/src/renderer/hub/chat/ChatPane.tsx">
<violation number="1" location="app/src/renderer/hub/chat/ChatPane.tsx:147">
P2: Unlike `onEditMessage`, the `onSubmit` handler silently swallows errors (console-only). If `sessions.resume` fails, the user gets no visual feedback that their follow-up message wasn't sent. Consider showing a toast on failure for consistency.</violation>
</file>
<file name="app/src/renderer/hub/chat/toolLabels.ts">
<violation number="1" location="app/src/renderer/hub/chat/toolLabels.ts:526">
P2: `parseBashResult` does not recover `exit_code`/`status` from truncated JSON, so failed bash runs can be misclassified as non-errors when parsing falls back.</violation>
</file>
<file name="app/src/renderer/hub/chat/ToolBlock.tsx">
<violation number="1" location="app/src/renderer/hub/chat/ToolBlock.tsx:155">
P2: The expanded bash "Command" view can show a truncated command because it prefers `displayValue`, which is capped to 80 chars.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
| // packageAfterPrune hook below, and app-update.yml is copied as a resource. | ||
| ignore: (file) => { | ||
| if (!file) return false; | ||
| return !file.startsWith('/.vite'); |
There was a problem hiding this comment.
P1: The new ignore function filters out /package.json, but later packaging hooks read buildPath/package.json unconditionally, which can break electron-forge package/make with a missing-file error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/forge.config.ts, line 76:
<comment>The new `ignore` function filters out `/package.json`, but later packaging hooks read `buildPath/package.json` unconditionally, which can break `electron-forge package/make` with a missing-file error.</comment>
<file context>
@@ -68,32 +68,13 @@ const config: ForgeConfig = {
+ // packageAfterPrune hook below, and app-update.yml is copied as a resource.
+ ignore: (file) => {
+ if (!file) return false;
+ return !file.startsWith('/.vite');
+ },
</file context>
Suggested change
| return !file.startsWith('/.vite'); | |
| return !(file === '/package.json' || file.startsWith('/.vite')); |
Several issue-review findings shared the same surface: initial chat hydration could overwrite early IPC output, follow-up failures were console-only, markdown tables leaked outside chat, and tool/screencast details could be stale or truncated. This keeps hydration merge-safe, restores BrowserView visibility based on the current hub mode, and fixes the targeted chat affordances without staging unrelated local edits. Constraint: Worktree contains unrelated unstaged changes, including mixed hunks in chat.css and TaskInput.tsx. Rejected: Stage the full working tree | would include unrelated user/local changes. Rejected: Replace hydrate semantics globally | tests still need hydrate([]) as a reset path. Confidence: high Scope-risk: moderate Directive: Keep chat store initial listAll handling merge-safe with live IPC events. Tested: yarn typecheck Tested: yarn vitest run tests/unit/hub/summarizeBashCommand.spec.ts tests/unit/hub/sessionsStore.spec.ts tests/unit/sessions/SessionManager.persistence.test.ts Tested: yarn eslint src/main/sessions/SessionScreencast.ts src/renderer/hub/HubApp.tsx src/renderer/hub/TaskInput.tsx src/renderer/hub/chat/ChatPane.tsx src/renderer/hub/chat/CodeBlock.tsx src/renderer/hub/chat/QuoteSelectionButton.tsx src/renderer/hub/chat/ToolBlock.tsx src/renderer/hub/chat/toolLabels.ts src/renderer/hub/state/sessionsStore.ts src/renderer/hub/state/useSessionsBridge.ts tests/unit/hub/summarizeBashCommand.spec.ts tests/unit/hub/sessionsStore.spec.ts Tested: git diff --cached --check Not-tested: electron-forge package/make Co-authored-by: OmX <omx@oh-my-codex.dev>
The durable session event log now owns the chat kickoff. New sessions persist the initial user input as the first session_events row, follow-ups append later user_input events without mutating sessions.prompt, and reruns restart from the first logged kickoff unless an edited kickoff is explicitly provided. The renderer also stops synthesizing a first bubble from stale sessions.prompt when event-log entries exist, which prevents older rows with corrupted prompt metadata from displaying the latest follow-up as the original request. Constraint: sessions.prompt can be stale after resumed follow-ups and remains only a legacy metadata/display fallback. Rejected: Remove the sessions.prompt column | too broad for this patch because legacy rows and display surfaces still need compatibility fallback. Confidence: high Scope-risk: moderate Directive: Treat session_events.user_input[0] as the canonical kickoff for starts, reruns, attachment replay, and transcript rendering. Tested: npm run test -- tests/unit/sessions/SessionManager.persistence.test.ts tests/unit/hub/ChatTranscript.spec.tsx tests/unit/sessions/session-schemas.test.ts Tested: npx eslint src/main/sessions/SessionManager.ts src/main/sessions/SessionDb.ts src/main/index.ts src/shared/session-schemas.ts src/renderer/hub/types.ts src/renderer/hub/chat/ChatTranscript.tsx src/renderer/hub/chat/groupIntoTurns.ts tests/unit/sessions/SessionManager.persistence.test.ts tests/unit/hub/ChatTranscript.spec.tsx (0 errors; 8 existing warnings in src/main/index.ts) Tested: git diff --cached --check Not-tested: Manual Electron rerun flow in this commit step Co-authored-by: OmX <omx@oh-my-codex.dev>
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/sessions/SessionManager.ts">
<violation number="1" location="app/src/main/sessions/SessionManager.ts:436">
P2: `getTermReplay` no longer prepends the initial prompt for sessions with existing non-`user_input` events, so legacy sessions can lose the kickoff line during terminal replay.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic
Comment on lines
+436
to
+438
| if (events.length === 0 && session.prompt) { | ||
| events.push({ type: 'user_input', text: session.prompt }); | ||
| } |
There was a problem hiding this comment.
P2: getTermReplay no longer prepends the initial prompt for sessions with existing non-user_input events, so legacy sessions can lose the kickoff line during terminal replay.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/sessions/SessionManager.ts, line 436:
<comment>`getTermReplay` no longer prepends the initial prompt for sessions with existing non-`user_input` events, so legacy sessions can lose the kickoff line during terminal replay.</comment>
<file context>
@@ -375,8 +432,10 @@ export class SessionManager extends EventEmitter {
const events: HlEvent[] = [];
- if (session.prompt) events.push({ type: 'user_input', text: session.prompt });
events.push(...session.output);
+ if (events.length === 0 && session.prompt) {
+ events.push({ type: 'user_input', text: session.prompt });
+ }
</file context>
Suggested change
| if (events.length === 0 && session.prompt) { | |
| events.push({ type: 'user_input', text: session.prompt }); | |
| } | |
| if (!events.some((event): event is UserInputEvent => event.type === 'user_input') && session.prompt) { | |
| events.unshift({ type: 'user_input', text: session.prompt }); | |
| } |
Tip: Review your code locally with the cubic CLI to iterate faster.
The chat preview now polls CDP screenshots from a parked browser view instead of embedding a live WebContentsView in the chat pane. Parking at a one-pixel window intersection keeps Chromium compositing while preserving the existing preview tile UI. The renderer consumes streamed JPEG frames, while BrowserPool tracks parked state and restores the last visible bounds when the user returns to grid view. Constraint: Fully detached WebContentsView instances can stop producing compositor-backed screenshots Rejected: Embed the live WebContentsView in the chat pane | it covered chat and made preview layout unstable Confidence: high Scope-risk: moderate Directive: Do not remove preview parking without verifying first frame before click and post-navigation frame changes Tested: yarn test tests/unit/sessions/SessionScreencast.test.ts tests/unit/sessions/BrowserPool.test.ts; yarn typecheck; yarn lint; git diff --check; live session 91914fe4-dd23-414a-9b7b-fe6358f11660 Not-tested: Full yarn test has unrelated Toast import and better-sqlite3 ABI failures Co-authored-by: OmX <omx@oh-my-codex.dev>
The PRODUCT_ docs are personal product-direction drafts for local agent context. They are removed from Git tracking with git rm --cached and ignored via app/docs/PRODUCT_*.md, preserving the local files. Constraint: User requested the PRODUCT_ files remain on the local filesystem Rejected: Delete the PRODUCT_ files from the working tree | user explicitly asked to keep local copies Confidence: high Scope-risk: narrow Tested: test -f app/docs/PRODUCT_BLUEPRINT.md app/docs/PRODUCT_SOUL.md app/docs/PRODUCT_SOUL_USER_TRANSCRIPT.md Tested: git status --ignored app/docs/PRODUCT_BLUEPRINT.md app/docs/PRODUCT_SOUL.md app/docs/PRODUCT_SOUL_USER_TRANSCRIPT.md .gitignore Not-tested: No runtime tests needed for ignore-only tracking change Co-authored-by: OmX <omx@oh-my-codex.dev>
Skill usage rows now resolve their harness skill files through one shared path/id conversion layer, expose a bounded read-skill IPC endpoint, and render expandable chat cards with descriptions and Finder reveal actions instead of opaque chips. The engine post-processor reuses the same resolver as the renderer IPC path, and the stock agent-skill validator now requires frontmatter descriptions so cards do not display arbitrary markdown as summaries. Constraint: Skill files live in three different harness layouts: user skills, domain skills, and interaction skills. Rejected: Keep string-only skill chips | they hide useful skill context and cannot reveal the underlying file. Confidence: high Scope-risk: moderate Directive: Keep skill path/id conversion centralized in harness.ts when adding new skill domains. Tested: yarn typecheck Tested: yarn lint (0 errors, existing warnings only) Tested: git diff --cached --check Not-tested: Manual expanded skill-card click path in Electron Co-authored-by: OmX <omx@oh-my-codex.dev>
The chat busy indicator now uses a persisted spinner-verb store with presets, custom verb entry, and cycle interval controls in Settings. The same pass speeds up streaming prose catch-up and moves the locked engine explanation onto the shared tooltip surface. Constraint: Avoid new dependencies and keep the control in the existing Settings appearance section. Rejected: Hard-code another single busy label | long-running sessions still feel stale and users cannot tune the tone. Confidence: high Scope-risk: narrow Directive: Keep spinner presets data-only so new options flow through Settings without bespoke UI branches. Tested: yarn typecheck Tested: yarn lint (0 errors, existing warnings only) Tested: git diff --cached --check Not-tested: Manual Settings persistence pass in Electron Co-authored-by: OmX <omx@oh-my-codex.dev>
Vitest now resolves the same @ -> src alias that renderer Vite builds use, so hub component specs can import shared renderer modules in CI. The affected specs also wrap their render trees in ToastProvider to match the production hub root now that ConnectionsPane and SettingsPane use the toast hook. Constraint: Renderer code already relies on the @ alias via Vite and tsconfig paths. Rejected: Replace renderer imports with relative paths | it would diverge from app code and leave Vitest unlike the renderer build. Confidence: high Scope-risk: narrow Directive: Keep Vitest aliases aligned with renderer Vite aliases when adding app-wide import prefixes. Tested: yarn vitest run tests/unit/hub/ConnectionsPane.spec.tsx tests/unit/hub/SettingsPane.spec.tsx Tested: yarn typecheck Tested: yarn lint (0 errors, existing warnings only) Not-tested: Full yarn test blocked locally by better-sqlite3 NODE_MODULE_VERSION 145 vs Node-required 127 ABI mismatch after all non-SQLite suites passed Co-authored-by: OmX <omx@oh-my-codex.dev>
Collaborator
Author
|
@cubic please review everything |
There was a problem hiding this comment.
4 issues found across 62 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/sessions/SessionScreencast.ts">
<violation number="1" location="app/src/main/sessions/SessionScreencast.ts:91">
P1: Release preview parking when debugger attach fails; otherwise failed startup can leave the BrowserView parked.</violation>
</file>
<file name="app/src/renderer/hub/chat/Linkify.tsx">
<violation number="1" location="app/src/renderer/hub/chat/Linkify.tsx:35">
P1: Guard the promise chain when using optional IPC calls; the current unconditional `.catch` can throw if `revealOutput` is unavailable.</violation>
</file>
<file name="app/src/renderer/hub/hub.css">
<violation number="1" location="app/src/renderer/hub/hub.css:2069">
P2: `word-break: break-all` makes output paths fracture at arbitrary characters, reducing readability and copyability. Use normal word breaking with `overflow-wrap` for safer wrapping.</violation>
</file>
<file name="app/src/renderer/hub/state/useSessionsBridge.ts">
<violation number="1" location="app/src/renderer/hub/state/useSessionsBridge.ts:92">
P2: Buffer `sessionBrowserAttached/sessionBrowserGone` until hydration completes. Dropping pre-hydration events can leave `hasBrowser` permanently stale for a session.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Preview capture and renderer hydration both had small failure windows where the UI could retain stale state after a startup race. Release preview parking when debugger attach fails, queue browser-state IPC until listAll hydration has seeded the store, and keep optional output-path IPC handling defensive. Constraint: Browser state is seeded by sessions:list-all and then reconciled by dedicated IPC events Rejected: Let session-updated carry hasBrowser | that channel intentionally excludes hasBrowser because it is computed from BrowserPool state Confidence: high Scope-risk: narrow Directive: Keep hasBrowser ownership on browser-attached/browser-gone events unless the session snapshot contract is changed Tested: npm test -- --run tests/unit/sessions/SessionScreencast.test.ts tests/unit/hub/useSessionsBridge.spec.tsx tests/unit/hub/Linkify.spec.tsx Tested: npm run typecheck Tested: npm run lint Tested: npm test Not-tested: Visual browser pass for CSS wrapping Co-authored-by: OmX <omx@oh-my-codex.dev>
Collaborator
Author
|
@cubic review |
There was a problem hiding this comment.
7 issues found across 64 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/forge.config.ts">
<violation number="1" location="app/forge.config.ts:76">
P0: The ignore filter assumes relative paths, but Electron Packager passes absolute paths; this will ignore nearly all files and break packaged builds.</violation>
</file>
<file name="app/src/main/sessions/SessionScreencast.ts">
<violation number="1" location="app/src/main/sessions/SessionScreencast.ts:198">
P2: Do not skip `releasePreviewParking` when the window is destroyed; this prevents clearing parked/attached state in `BrowserPool` during preview cleanup.</violation>
</file>
<file name="app/src/main/hl/harness.ts">
<violation number="1" location="app/src/main/hl/harness.ts:77">
P2: Reject `.`/`..` and absolute topic segments when converting skill IDs to paths; otherwise `domainTopic` can resolve outside its skill namespace.</violation>
</file>
<file name="app/src/renderer/hub/chat/ChatTurn.tsx">
<violation number="1" location="app/src/renderer/hub/chat/ChatTurn.tsx:98">
P2: Editing a quoted user message drops the quote context because only the message body is sent back.</violation>
<violation number="2" location="app/src/renderer/hub/chat/ChatTurn.tsx:699">
P1: Image outputs are hidden when a turn has no `done` entry, because they are skipped as hoisted but never rendered.</violation>
</file>
<file name="app/src/renderer/hub/state/sessionsStore.ts">
<violation number="1" location="app/src/renderer/hub/state/sessionsStore.ts:73">
P1: `upsertSession` can clobber newer in-memory output with stale `sessions.get` data. Preserve existing non-empty `prev.output` (and its timestamps) when merging to avoid losing live chat events.</violation>
</file>
<file name="app/src/renderer/hub/state/useSessionsBridge.ts">
<violation number="1" location="app/src/renderer/hub/state/useSessionsBridge.ts:38">
P2: Queued startup `session-output` events can be lost for sessions not yet in the store because unknown-session outputs are discarded instead of buffered until `session-updated` inserts the session.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Reported review issues covered several independent failure modes: package file filtering, preview parking cleanup, skill path parsing, chat rendering, and live session reconciliation. This patch keeps Forge Vite output through both relative and absolute ignore inputs, clears BrowserPool preview state even after the preview window is gone, rejects unsafe skill topic segments, preserves quoted edit context, renders image outputs before done events, and buffers session output until the session exists. Constraint: Electron Forge Vite expects packaged apps to retain .vite output while excluding source files. Constraint: Native modules are rebuilt for Electron during packaging, while Vitest runs against the host Node ABI. Rejected: Drop unknown session output until session-updated | loses startup events in event-order races. Rejected: Skip releasePreviewParking when BrowserWindow is destroyed | leaves BrowserPool attached/parked flags stale. Confidence: high Scope-risk: moderate Directive: Keep session output and outputTimestamps as one invariant when merging snapshots with live IPC. Tested: npm run typecheck Tested: npm run lint (0 errors, existing warnings) Tested: npm run test (484 passed, 6 skipped) Tested: npm run package on Node v22.22.2; app.asar contains /.vite/build/main.js and excludes /src/main/index.ts and /forge.config.ts Not-tested: Signed/notarized release make artifacts Co-authored-by: OmX <omx@oh-my-codex.dev>
Collaborator
Author
|
@cubic review |
There was a problem hiding this comment.
6 issues found across 67 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/hl/harness.ts">
<violation number="1" location="app/src/main/hl/harness.ts:110">
P2: Quote sanitization only removes trailing quotes, so quoted skill IDs fail to resolve.</violation>
</file>
<file name="app/src/renderer/hub/chat/ChatTranscript.tsx">
<violation number="1" location="app/src/renderer/hub/chat/ChatTranscript.tsx:245">
P2: `activityKey` omits `sessionId`, so elapsed-time state can leak across session switches and show the wrong “Working” timer.</violation>
</file>
<file name="app/src/renderer/hub/chat/ToolGroup.tsx">
<violation number="1" location="app/src/renderer/hub/chat/ToolGroup.tsx:26">
P2: Bash group summaries are generated from a truncated command string, which can misclassify recognizable commands and degrade tool-group labels.</violation>
</file>
<file name="app/src/renderer/hub/chat/toolLabels.ts">
<violation number="1" location="app/src/renderer/hub/chat/toolLabels.ts:130">
P2: Normalize label lookup through the canonical tool type so alias tool names get friendly status labels instead of falling back to raw title-cased names.</violation>
</file>
<file name="app/src/main/sessions/SessionScreencast.ts">
<violation number="1" location="app/src/main/sessions/SessionScreencast.ts:96">
P1: `start()` has a race condition for concurrent calls on the same session, which can create orphaned duplicate capture timers. Re-check `this.previews` after async setup (before creating the interval) and abort the duplicate start.</violation>
</file>
<file name="app/src/main/protocols/chatfile.ts">
<violation number="1" location="app/src/main/protocols/chatfile.ts:43">
P2: `root` is not canonicalized but requested paths are resolved via `realpathSync`. If any component of the userData/harness/outputs path is a symlink, all legitimate requests will silently get 403. Consider resolving `root` through `realpathSync` as well (lazily, since the directory may not exist at registration time).</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
| } | ||
| } | ||
|
|
||
| const options = { ...DEFAULT_OPTIONS, ...opts }; |
There was a problem hiding this comment.
P1: start() has a race condition for concurrent calls on the same session, which can create orphaned duplicate capture timers. Re-check this.previews after async setup (before creating the interval) and abort the duplicate start.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/sessions/SessionScreencast.ts, line 96:
<comment>`start()` has a race condition for concurrent calls on the same session, which can create orphaned duplicate capture timers. Re-check `this.previews` after async setup (before creating the interval) and abort the duplicate start.</comment>
<file context>
@@ -0,0 +1,210 @@
+ }
+ }
+
+ const options = { ...DEFAULT_OPTIONS, ...opts };
+ const preview: ActivePreview = {
+ wc,
</file context>
| } | ||
|
|
||
| export function skillIdToPath(skillId: string, rootDir: string = harnessDir()): string | null { | ||
| const cleaned = skillId.trim().replace(/['"]+$/g, '').replace(/\\/g, '/'); |
There was a problem hiding this comment.
P2: Quote sanitization only removes trailing quotes, so quoted skill IDs fail to resolve.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/hl/harness.ts, line 110:
<comment>Quote sanitization only removes trailing quotes, so quoted skill IDs fail to resolve.</comment>
<file context>
@@ -65,6 +65,57 @@ export function browserHarnessJsDir(): string { return path.join(harnessDir(), '
+}
+
+export function skillIdToPath(skillId: string, rootDir: string = harnessDir()): string | null {
+ const cleaned = skillId.trim().replace(/['"]+$/g, '').replace(/\\/g, '/');
+ if (!cleaned || cleaned.startsWith('--') || cleaned.startsWith('/') || path.isAbsolute(skillId)) return null;
+ const parts = cleaned.split('/');
</file context>
Suggested change
| const cleaned = skillId.trim().replace(/['"]+$/g, '').replace(/\\/g, '/'); | |
| const cleaned = skillId.trim().replace(/^['"]+|['"]+$/g, '').replace(/\\/g, '/'); |
| // is a new agent entry appearing OR a tool_call's result landing. We do NOT | ||
| // key on streamed content length — otherwise every token would reset the | ||
| // marker and the timer would stick at 0s during long streams. | ||
| let activityKey = `turn:${lastTurn?.id ?? ''}|user:${lastTurn?.userEntry?.id ?? ''}`; |
There was a problem hiding this comment.
P2: activityKey omits sessionId, so elapsed-time state can leak across session switches and show the wrong “Working” timer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/renderer/hub/chat/ChatTranscript.tsx, line 245:
<comment>`activityKey` omits `sessionId`, so elapsed-time state can leak across session switches and show the wrong “Working” timer.</comment>
<file context>
@@ -0,0 +1,284 @@
+ // is a new agent entry appearing OR a tool_call's result landing. We do NOT
+ // key on streamed content length — otherwise every token would reset the
+ // marker and the timer would stick at 0s during long streams.
+ let activityKey = `turn:${lastTurn?.id ?? ''}|user:${lastTurn?.userEntry?.id ?? ''}`;
+ if (lastTurn && lastTurn.agentEntries.length > 0) {
+ const last = lastTurn.agentEntries[lastTurn.agentEntries.length - 1];
</file context>
| // `{"preview":"/bin/zsh …","command":"/bin/zsh …"}`. Run it through | ||
| // getToolDisplayValue first so summarizeBashCommand sees the raw shell | ||
| // command, not the JSON wrapper. | ||
| const display = getToolDisplayValue(entry.tool, entry.content || ''); |
There was a problem hiding this comment.
P2: Bash group summaries are generated from a truncated command string, which can misclassify recognizable commands and degrade tool-group labels.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/renderer/hub/chat/ToolGroup.tsx, line 26:
<comment>Bash group summaries are generated from a truncated command string, which can misclassify recognizable commands and degrade tool-group labels.</comment>
<file context>
@@ -0,0 +1,140 @@
+ // `{"preview":"/bin/zsh …","command":"/bin/zsh …"}`. Run it through
+ // getToolDisplayValue first so summarizeBashCommand sees the raw shell
+ // command, not the JSON wrapper.
+ const display = getToolDisplayValue(entry.tool, entry.content || '');
+ if (getToolType(entry.tool) === 'bash') {
+ const s = summarizeBashCommand(display || entry.content || '');
</file context>
|
|
||
| export function getToolLabel(toolName: string | undefined, status: ToolStatus = 'pending'): string { | ||
| if (!toolName) return 'Unknown action'; | ||
| const labels = TOOL_LABELS[toolName.toLowerCase()]; |
There was a problem hiding this comment.
P2: Normalize label lookup through the canonical tool type so alias tool names get friendly status labels instead of falling back to raw title-cased names.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/renderer/hub/chat/toolLabels.ts, line 130:
<comment>Normalize label lookup through the canonical tool type so alias tool names get friendly status labels instead of falling back to raw title-cased names.</comment>
<file context>
@@ -0,0 +1,554 @@
+
+export function getToolLabel(toolName: string | undefined, status: ToolStatus = 'pending'): string {
+ if (!toolName) return 'Unknown action';
+ const labels = TOOL_LABELS[toolName.toLowerCase()];
+ if (!labels) {
+ return toolName
</file context>
| } | ||
|
|
||
| export function registerChatfileHandler(): void { | ||
| const root = path.resolve(path.join(harnessDir(), 'outputs')) + path.sep; |
There was a problem hiding this comment.
P2: root is not canonicalized but requested paths are resolved via realpathSync. If any component of the userData/harness/outputs path is a symlink, all legitimate requests will silently get 403. Consider resolving root through realpathSync as well (lazily, since the directory may not exist at registration time).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/protocols/chatfile.ts, line 43:
<comment>`root` is not canonicalized but requested paths are resolved via `realpathSync`. If any component of the userData/harness/outputs path is a symlink, all legitimate requests will silently get 403. Consider resolving `root` through `realpathSync` as well (lazily, since the directory may not exist at registration time).</comment>
<file context>
@@ -0,0 +1,74 @@
+}
+
+export function registerChatfileHandler(): void {
+ const root = path.resolve(path.join(harnessDir(), 'outputs')) + path.sep;
+
+ protocol.handle(CHATFILE_SCHEME, async (req) => {
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Introduces a chat‑first view that groups tool calls, shows live browser previews, and safely inlines screenshots. Adds skill cards in chat, configurable “Working…” verbs, and keeps session UI state accurate across startup and packaging.
New Features
chatfile://fromBU_OUTPUTS_DIR; CSP updated for images; clickable harness‑output paths.zustand-backed session/UI stores with an IPC bridge; edit‑and‑rerun kickoff; configurable spinner verbs with presets in Settings.Bug Fixes
output/timestamps aligned during merge..viteoutput (both relative and absolute paths supported); add unit tests to prevent source files from leaking into builds.listAllhydration completes; release preview parking when CDP debugger attach fails; harden output‑path reveal IPC; preserve quoted edit context in chat.Written for commit f38dac6. Summary will update on new commits.