fix(grok): clipboard, native chat, hooks, sessions, ConPTY KKP#7944
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Grok support across native chat session resolution, transcript decoding, hook normalization, renderer availability, pane resolution, image paste, and startup handling. It updates Grok hook matchers and installation paths, forces terminal identity variables for PTY environments, filters orphaned terminal query replies, preserves Kitty keyboard support for Grok on Windows ConPTY, and updates OSC 52 clipboard defaults, documentation, settings text, tests, and translations. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/native-chat/session-file-resolver.ts (1)
129-147: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider targeted directory listing instead of full tree walk.
resolveGrokSessionFileuseswalkSessionFilesto traverse the entire sessions tree and filter bybasename(dirname(path)) === sessionId. Given the known 2-level structure (sessionsDir/encodeURIComponent(cwd)/sessionId/chat_history.jsonl), listing the top-level directories and checking forjoin(dir, sessionId, 'chat_history.jsonl')directly would avoid reading every.jsonlfile across all sessions — a meaningful improvement for users with many working directories.⚡ Proposed targeted lookup
async function resolveGrokSessionFile( sessionId: string, sessionsDir: string ): Promise<string | null> { - // Why: Grok nests chat_history.jsonl under encodeURIComponent(cwd)/sessionId/ - // (same layout agent-hook-listener getGrokChatHistoryPath uses). Walk the - // sessions tree for that exact leaf name under a directory named sessionId. + // Why: Grok nests chat_history.jsonl under encodeURIComponent(cwd)/sessionId/ + // (same layout agent-hook-listener getGrokChatHistoryPath uses). List the + // top-level cwd directories and probe for the exact leaf path. if (!existsSync(sessionsDir)) { return null } - const targetName = 'chat_history.jsonl' - const files = await walkSessionFiles(sessionsDir, 'grok', [], { - extensions: new Set(['.jsonl']), - filePredicate: (path) => - basename(path) === targetName && basename(dirname(path)) === sessionId - }) - return files[0] ?? null + const targetName = 'chat_history.jsonl' + const entries = await readdir(sessionsDir, { withFileTypes: true }) + for (const entry of entries) { + if (!entry.isDirectory()) continue + const candidate = join(sessionsDir, entry.name, sessionId, targetName) + if (existsSync(candidate)) { + return candidate + } + } + return null }src/main/native-chat/transcript-line-decoders.grok.test.ts (1)
1-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
backend_tool_call,tool_call, andtool_resultrecord types.The suite covers user, assistant (tool-call-only), reasoning, and system branches, but the
backend_tool_call/tool_callpath (lines 119–132 in the decoder) andtool_resultpath (lines 134–149) — which include multi-fallback name extraction andisErrornormalization — are untested.🧪 Suggested additional test cases
it('decodes backend_tool_call records', () => { const line = JSON.stringify({ type: 'backend_tool_call', kind: { tool_type: 'grep', pattern: 'foo' }, id: 'btc-1' }) expect(decodeGrokTranscriptLine(line, 'fb-5')).toEqual({ id: 'btc-1', role: 'assistant', blocks: [{ type: 'tool-call', name: 'grep', input: { tool_type: 'grep', pattern: 'foo' } }], timestamp: null, source: 'transcript' }) }) it('decodes tool_result records with error flag', () => { const line = JSON.stringify({ type: 'tool_result', content: 'Command failed', is_error: true, id: 'tr-1' }) expect(decodeGrokTranscriptLine(line, 'fb-6')).toEqual({ id: 'tr-1', role: 'tool', blocks: [{ type: 'tool-result', output: 'Command failed', isError: true }], timestamp: null, source: 'transcript' }) }) it('decodes assistant with both text content and tool_calls', () => { const line = JSON.stringify({ type: 'assistant', content: [{ type: 'text', text: 'Running grep now' }], tool_calls: [{ id: 'c2', name: 'grep', arguments: '{"pattern":"bar"}' }], id: 'asst-2' }) expect(decodeGrokTranscriptLine(line, 'fb-7')).toEqual({ id: 'asst-2', role: 'assistant', blocks: [ { type: 'text', text: 'Running grep now' }, { type: 'tool-call', name: 'grep', input: { pattern: 'bar' } } ], timestamp: null, source: 'transcript' }) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7dabc9bb-2fe6-446a-a69d-433e7ad1310e
📒 Files selected for processing (20)
src/main/codex-accounts/runtime-home-service.test.tssrc/main/codex-accounts/service.test.tssrc/main/native-chat/session-file-resolver.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/native-chat/transcript-line-decoders.grok.test.tssrc/main/native-chat/transcript-line-decoders.tssrc/main/native-chat/transcript-reader.tssrc/main/native-chat/transcript-watch.tssrc/renderer/src/components/native-chat/NativeChatView.test.tsxsrc/renderer/src/components/native-chat/native-chat-availability.test.tssrc/renderer/src/components/native-chat/native-chat-availability.tssrc/renderer/src/components/native-chat/native-chat-image-paste.test.tssrc/renderer/src/components/native-chat/native-chat-image-paste.tssrc/renderer/src/components/native-chat/native-chat-pane-resolution.test.tssrc/renderer/src/components/native-chat/use-native-chat-toggle-shortcut.tssrc/renderer/src/components/settings/TerminalInteractionSection.tsxsrc/renderer/src/lib/native-chat-supported-agent.tssrc/renderer/src/lib/tui-agent-startup.test.tssrc/shared/constants.tssrc/shared/tui-agent-config.ts
bbingz
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/shared/grok-session-paths.ts (1)
53-71: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd sessionId validation to prevent path traversal in
buildGrokChatHistoryPathCandidates.
sessionIdis used directly injoin(args.sessionsDir, encoded, sessionId, GROK_CHAT_HISTORY_FILE)without checking for path traversal characters. While current callers (e.g.,getGrokChatHistoryPathinagent-hook-listener.ts) validate viaisSafeGrokSessionId, this is a public exported function — a future caller that skips validation could traverse outsidesessionsDirwith a craftedsessionIdlike../../etc.🛡️ Proposed defense-in-depth validation
export function buildGrokChatHistoryPathCandidates(args: { sessionId: string cwd?: string | null sessionsDir: string }): string[] { const sessionId = args.sessionId.trim() - if (!sessionId) { + if (!sessionId || /[/\\]/.test(sessionId) || sessionId.includes('..')) { return [] }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e7a3a876-5c96-4b74-9be3-8e6e4454ee4d
📒 Files selected for processing (20)
src/main/agent-hooks/remote-hook-service-installers.test.tssrc/main/grok/hook-service.test.tssrc/main/grok/hook-service.tssrc/main/native-chat/session-file-resolver.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/providers/ssh-pty-provider.test.tssrc/main/providers/ssh-pty-provider.tssrc/relay/pty-handler.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tssrc/renderer/src/lib/pane-manager/pane-lifecycle.test.tssrc/renderer/src/lib/pane-manager/terminal-keyboard-protocol.test.tssrc/renderer/src/lib/pane-manager/terminal-keyboard-protocol.tssrc/shared/agent-hook-listener.test.tssrc/shared/agent-hook-listener.tssrc/shared/grok-session-paths.test.tssrc/shared/grok-session-paths.tssrc/shared/terminal-query-reply.test.tssrc/shared/terminal-query-reply.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/native-chat/session-file-resolver.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/native-chat/transcript-line-decoders-claude.ts (1)
52-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
parseTimestampto a shared module.This identical
parseTimestampwrapper is duplicated intranscript-line-decoders-codex.ts(lines 136-139) andtranscript-line-decoders-grok.ts(lines 179-182). Sincetranscript-record-blocks.tsalready serves as the shared utility module for these decoders (exportingclaudeContentBlocksandtoolResultOutput), movingparseTimestampthere would eliminate the triplication and provide a single point of change for timestamp null-handling logic.♻️ Proposed refactor
In
src/main/native-chat/transcript-record-blocks.ts, add:+export function parseTimestamp(value: unknown): number | null { + const parsed = timestampMs(value) + return Number.isFinite(parsed) ? parsed : null +}Then in each decoder file (
transcript-line-decoders-claude.ts,transcript-line-decoders-codex.ts,transcript-line-decoders-grok.ts), replace the localparseTimestampwith an import:-import { timestampMs } from '../ai-vault/session-scanner-values' +import { timestampMs } from '../ai-vault/session-scanner-values' +import { parseTimestamp } from './transcript-record-blocks'And remove the local
parseTimestampdefinition from each file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f83e5077-8100-4234-a194-2c94e53e9457
📒 Files selected for processing (8)
src/main/grok/hook-service.test.tssrc/main/grok/hook-service.tssrc/main/native-chat/transcript-line-decoders-claude.tssrc/main/native-chat/transcript-line-decoders-codex.tssrc/main/native-chat/transcript-line-decoders-grok.tssrc/main/native-chat/transcript-line-decoders.tssrc/shared/constants.tssrc/shared/types.ts
✅ Files skipped from review due to trivial changes (1)
- src/shared/types.ts
bbingz
force-pushed
the
fix/grok-clipboard-compat
branch
2 times, most recently
from
604ef40 to
0eb05c7
Compare
Grok CLI already supports argv prompts, OSC 52 copy, and image paste chips. Orca was blocking those paths: stdin-after-start keystroke injection, OSC 52 writes default-off, image-attachment denylist, and native-chat allowlist. - Launch Grok with positional argv prompts - Default OSC 52 TUI clipboard writes on (still user-toggleable) - Treat Grok as image-attachment capable - Parse ~/.grok/.../chat_history.jsonl for native chat OSC 52 clipboard *query* remains ignored by design (host clipboard exfil risk); xAI docs only require OSC 52 write for remote copy.
Update terminalAllowOsc52Clipboard type docs for the true default, and refresh locale strings so settings UI mentions Grok alongside other TUIs.
Grok tool-event matchers are real regexes; bare `*` failed as match-all. Install `.*` for Pre/Post tool hooks, add StopFailure for API-error ends, recognize Grok-native tool input keys, and map ask_user_question PreToolUse to waiting with interactivePrompt (Kimi-style live card path).
Centralize Grok session path helpers so hooks and native-chat honor GROK_HOME and find chat_history.jsonl by session id when the cwd group is slug-encoded (encoded name > 255 bytes) instead of only encodeURIComponent(cwd).
Local Windows ConPTY withholds KKP so CSI-u-blind CLIs (e.g. Antigravity) keep Enter/nav working (stablyai#2434). Grok needs KKP for Ctrl+Enter interject and modified-Enter newline chords; blanking the advertisement for Orca-launched Grok left those actions broken. - Prefer KKP when tuiAgent is grok despite ConPTY withhold - Wire launchAgent from tab/startup into keyboard protocol options
… hooks - Keep terminalAllowOsc52Clipboard default false (clipboard exfil risk) - Split transcript-line-decoders under max-lines without suppressions - Install local Grok hooks under resolveGrokHomeDir() / GROK_HOME
bbingz
force-pushed
the
fix/grok-clipboard-compat
branch
from
0eb05c7 to
b6abb2f
Compare
bbingz
changed the title
Jinwoo-H
left a comment
Jinwoo-H
left a comment
There was a problem hiding this comment.
Thanks — I reviewed and extended this end to end.
- Verified real Grok 0.2.93 launch, hook attribution, Native Chat transcript loading, image paste/read, and exact responses.
- Hardened transcript discovery/filtering and ordering, hook lifecycle, custom GROK_HOME handling, and prompt argument separation.
- Verified OSC 52 remains default-off and works when explicitly enabled.
- Exercised a real Linux SSH relay, including custom/default TERM, environment deletion, framed XTVERSION replies, custom remote Grok hooks, and hook round-trip.
- Fixed the final node-pty boundary that was overwriting custom remote TERM values.
- Added regression coverage for the races and compatibility cases found during review.
#8174) launch-agent-in-new-tab.ts crossed the 300-line oxlint max-lines limit (303 counted lines) after #5510 and #7944 both grew it; verify only runs on PRs, so the over-limit state landed on main via a merge race and now fails lint for every open PR. Move the web-runtime host launch branch (stale-local-tab pruning plus createWebRuntimeSessionTerminal call) into launch-agent-web-host-tab.ts. No behavior change; the i18n key is kept so locale catalogs are untouched. Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local>
2 participants
Summary
Grok Build compatibility fixes that remain unique after reconciling with current
mainand #7869:.*Grok tool hook matchers, addStopFailure, tool previews, and waiting state forask_user_question.chat_history.jsonlunderGROK_HOMEand long-cwd slug layouts.Reconciliation with main
GROK_HOMEstack.GROK_HOMEpath lookup is centralized and reused by main's existing read-only Grok auth lookup, AI Vault discovery, hooks, and native-chat session resolution.Screenshots
No visual change.
Testing
pnpm lintpnpm typecheckpnpm buildoxlint,pnpm check:max-lines-ratchet, andgit diff --checkAI Review Report
The branch was rebased onto current main and reviewed file-by-file against #7869 and #7841. The review separated semantic overlap from files that merely share Grok types or locale catalogs.
It found that usage code had already been removed, but the remote XTVERSION/TERM commit exactly duplicated #7841 and was dropped. It also found three separate
GROK_HOMEimplementations across usage, AI Vault, and this PR; these now use one shared resolver. The latest TypeScript configuration exposed an overly strict environment-map type during reconciliation, which was corrected and revalidated.Cross-platform review covered macOS/Linux local behavior, Windows ConPTY KKP policy, SSH-safe path handling, and host-vs-guest environment boundaries. The #7841 remote PTY behavior is no longer duplicated here.
Security Audit
GROK_HOMEinto a guest.Notes
This PR intentionally no longer contains remote TERM/XTVERSION behavior; see #7841. Managed Grok accounts and owned-home isolation remain out of scope per maintainer direction.