From 0a45ad5ec47dcefd60b2244da48fb8aee3cebb62 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 23 Sep 2026 13:31:31 -0700 Subject: [PATCH] refactor(jcode): fold three reverse-scan copies into one, reuse the shared hook POST The jcode journal reader, the Claude transcript reader and the Command Code transcript reader each carried their own copy of the same reverse chunked line scan; they now share one tested helper. jcode's managed hook script drops its hand-rolled curl for buildPosixAgentHookPostCommand, which also gains it the raw-JSON transport and the --noproxy guard the bespoke copy was missing. Co-authored-by: czzczz --- src/main/jcode/daemon-prewarm.test.ts | 9 +- src/main/jcode/daemon-prewarm.ts | 72 +++++-------- src/main/jcode/hook-gate-script.test.ts | 6 +- src/main/jcode/hook-service.test.ts | 2 +- src/main/jcode/hook-service.ts | 41 +++---- .../pane-launch-agent-candidate.ts | 9 +- src/shared/agent-hook-listener-jcode.test.ts | 16 +++ src/shared/agent-hook-listener.ts | 6 +- .../command-code-transcript.ts | 102 +++--------------- .../providers/jcode-events.ts | 41 +++---- .../providers/jcode-tool-fields.ts | 17 ++- .../reverse-file-region-scan.test.ts | 91 ++++++++++++++++ .../reverse-file-region-scan.ts | 88 +++++++++++++++ .../agent-hook-listener/transcript-reader.ts | 75 ++----------- src/shared/jcode-session-files.ts | 75 ++----------- src/shared/jcode-terminal-title.test.ts | 12 +-- src/shared/jcode-terminal-title.ts | 30 +++--- 17 files changed, 335 insertions(+), 357 deletions(-) create mode 100644 src/shared/agent-hook-listener/reverse-file-region-scan.test.ts create mode 100644 src/shared/agent-hook-listener/reverse-file-region-scan.ts diff --git a/src/main/jcode/daemon-prewarm.test.ts b/src/main/jcode/daemon-prewarm.test.ts index 539c485bb04..da5f36025ef 100644 --- a/src/main/jcode/daemon-prewarm.test.ts +++ b/src/main/jcode/daemon-prewarm.test.ts @@ -3,11 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { spawnProcessMock } = vi.hoisted(() => ({ spawnProcessMock: vi.fn() })) vi.mock('../../shared/child-process/run-process', () => ({ spawnProcess: spawnProcessMock })) -import { - prewarmJcodeDaemon, - resetJcodeDaemonPrewarmForTests, - shouldPrewarmJcodeDaemon -} from './daemon-prewarm' +import { prewarmJcodeDaemon, resetJcodeDaemonPrewarmForTests } from './daemon-prewarm' function stubChild() { return { unref: vi.fn(), on: vi.fn() } @@ -59,12 +55,13 @@ describe('jcode daemon pre-warm', () => { it('stays out of the way on Windows, which has no runtime dir', () => { expect( - shouldPrewarmJcodeDaemon({ + prewarmJcodeDaemon({ launchAgent: 'jcode', runtimeDir: 'C:/tmp/orca-jcode/abc', platform: 'win32' }) ).toBe(false) + expect(spawnProcessMock).not.toHaveBeenCalled() }) it('reports failure instead of throwing when the binary is missing', () => { diff --git a/src/main/jcode/daemon-prewarm.ts b/src/main/jcode/daemon-prewarm.ts index b7c7e67ff86..fd40454cd59 100644 --- a/src/main/jcode/daemon-prewarm.ts +++ b/src/main/jcode/daemon-prewarm.ts @@ -9,33 +9,27 @@ import { spawnProcess } from '../../shared/child-process/run-process' import { getTuiAgentLaunchCommand, TUI_AGENT_CONFIG } from '../../shared/tui-agent-config' -/** Runtime dirs already pre-warmed in this Orca process; the daemon outlives one pane. */ -const prewarmedRuntimeDirs = new Set() +export type JcodeDaemonPrewarm = { + launchAgent?: string + runtimeDir?: string + cwd?: string + env?: Record + platform?: NodeJS.Platform +} + +/** Runtime dirs already warmed in this Orca process; the daemon outlives one pane. */ +const warmed = new Set() export function resetJcodeDaemonPrewarmForTests(): void { - prewarmedRuntimeDirs.clear() + warmed.clear() } -/** The runtime dir to warm, or null when this pane is not a local jcode launch. */ -export function resolveJcodePrewarmRuntimeDir(args: { - launchAgent?: string - runtimeDir?: string - platform?: NodeJS.Platform -}): string | null { - // Why non-Windows only: the runtime dir is a unix-socket directory, and Orca - // only stamps it off Windows (see shouldInjectJcodeRuntimeDir). - if (args.launchAgent !== 'jcode' || (args.platform ?? process.platform) === 'win32') { - return null - } - return args.runtimeDir !== undefined && args.runtimeDir.length > 0 ? args.runtimeDir : null -} - -export function shouldPrewarmJcodeDaemon(args: { - launchAgent?: string - runtimeDir?: string - platform?: NodeJS.Platform -}): boolean { - return resolveJcodePrewarmRuntimeDir(args) !== null +/** The runtime dir to warm, or null when this pane is not a local jcode launch. + * Why non-Windows only: the runtime dir is a unix-socket directory, and Orca only + * stamps it off Windows (see shouldInjectJcodeRuntimeDir). */ +function prewarmTarget({ launchAgent, runtimeDir, platform }: JcodeDaemonPrewarm): string | null { + const unsupported = launchAgent !== 'jcode' || (platform ?? process.platform) === 'win32' + return unsupported || !runtimeDir ? null : runtimeDir } /** @@ -45,39 +39,31 @@ export function shouldPrewarmJcodeDaemon(args: { * listening, so a pre-warm that fails costs nothing beyond the cold start Orca * already had. Never throws, and never blocks the spawn path. */ -export function prewarmJcodeDaemon(args: { - launchAgent?: string - runtimeDir?: string - cwd?: string - env?: Record - platform?: NodeJS.Platform -}): boolean { - const runtimeDir = resolveJcodePrewarmRuntimeDir(args) - if (runtimeDir === null || prewarmedRuntimeDirs.has(runtimeDir)) { +export function prewarmJcodeDaemon(args: JcodeDaemonPrewarm): boolean { + const runtimeDir = prewarmTarget(args) + if (runtimeDir === null || warmed.has(runtimeDir)) { return false } - prewarmedRuntimeDirs.add(runtimeDir) + warmed.add(runtimeDir) + // A missing binary or a spawn refusal just means no head start; forget the dir so + // the next pane on it can try again. + const giveUp = (): boolean => (warmed.delete(runtimeDir), false) try { const child = spawnProcess({ program: getTuiAgentLaunchCommand(TUI_AGENT_CONFIG.jcode, args.platform ?? process.platform), - // Why --no-update: an update check on the pre-warm path would delay the very - // socket the client is about to wait on. + // Why --no-update: an update check here would delay the very socket the client + // is about to wait on. Why stdio ignore + unref: the daemon is jcode's to own + // and must outlive this spawn, so Orca keeps no handle on it. args: ['--no-update', 'serve'], cwd: args.cwd, env: { ...args.env, JCODE_RUNTIME_DIR: runtimeDir }, detached: true, stdio: 'ignore' }) - // Why unref: the daemon is jcode's to own and must outlive this spawn; keeping a - // handle would tie Orca's event loop to it. child.unref() - child.on('error', () => { - // A missing binary or a spawn refusal just means no head start. - prewarmedRuntimeDirs.delete(runtimeDir) - }) + child.on('error', giveUp) return true } catch { - prewarmedRuntimeDirs.delete(runtimeDir) - return false + return giveUp() } } diff --git a/src/main/jcode/hook-gate-script.test.ts b/src/main/jcode/hook-gate-script.test.ts index 78865e8b061..a9273368306 100644 --- a/src/main/jcode/hook-gate-script.test.ts +++ b/src/main/jcode/hook-gate-script.test.ts @@ -105,10 +105,10 @@ describe.runIf(process.platform !== 'win32')('jcode managed hook as jcode runs i const script = readFileSync(scriptPath, 'utf8') const gateBranch = script.slice(script.indexOf('if [ "$JCODE_HOOK_EVENT" = pre_tool ]')) expect(gateBranch).toContain('orca_post_jcode_event >/dev/null 2>&1 &') - // The observer path keeps the plain call, so a slow POST cannot be lost to - // a script that exited first. + // The observer path keeps the foreground call, so a slow POST cannot be lost + // to a script that exited first. + expect(gateBranch).toContain('orca_post_jcode_event >/dev/null 2>&1 || :') expect(script.trimEnd().endsWith('exit 0')).toBe(true) - expect(script).toContain('\norca_post_jcode_event\n') } finally { cleanup() } diff --git a/src/main/jcode/hook-service.test.ts b/src/main/jcode/hook-service.test.ts index 86c9f90e52c..abcfe7518d2 100644 --- a/src/main/jcode/hook-service.test.ts +++ b/src/main/jcode/hook-service.test.ts @@ -60,7 +60,7 @@ describe('JcodeHookService', () => { expect(script).toContain('payload@-') // Why: the payload is jcode's own JCODE_HOOK_PAYLOAD, forwarded verbatim. expect(script).toContain('$JCODE_HOOK_PAYLOAD') - expect(script).toContain('hook_event_name=${JCODE_HOOK_EVENT}') + expect(script).toContain('payload="$JCODE_HOOK_PAYLOAD"') }) it('preserves unrelated config tables when installing hooks', () => { diff --git a/src/main/jcode/hook-service.ts b/src/main/jcode/hook-service.ts index c33f4fd249c..6dd244ac1e5 100644 --- a/src/main/jcode/hook-service.ts +++ b/src/main/jcode/hook-service.ts @@ -7,6 +7,7 @@ import { writeManagedScript } from '../agent-hooks/installer-utils' import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' +import { buildPosixAgentHookPostCommand } from '../agent-hooks/hook-post-command' import { readTextFileRemote, writeManagedScriptRemote, @@ -58,43 +59,31 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', 'fi', - // Why: pre_tool is jcode's gate. It writes the tool input to our stdin and - // waits for us, so drain stdin before any exit — a tool input larger than - // the pipe buffer would otherwise stall the agent mid-write. + // Why before the env guard: pre_tool is jcode's gate — it writes the tool input to + // our stdin and waits for us, so stdin must be drained before ANY exit path or a + // tool input larger than the pipe buffer stalls the agent mid-write. 'if [ "$JCODE_HOOK_EVENT" = pre_tool ]; then', ` ${POSIX_HOOK_STDIN_DRAIN_COMMAND}`, 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - // Why: jcode already supplies JCODE_HOOK_PAYLOAD as a JSON object (capped - // at 16 KB), so Orca forwards it verbatim instead of hand-building JSON in - // shell (unsafe for arbitrary text). The event name is also posted as a - // top-level form field for old payloads that omit it. + // Why the env var rather than a stdin capture: jcode hands the hook its payload as + // a ready JSON object (capped at 16 KB), so Orca forwards it verbatim instead of + // hand-building JSON in shell, which is unsafe for arbitrary text. + 'payload="$JCODE_HOOK_PAYLOAD"', 'orca_post_jcode_event() {', - ' printf \'%s\' "$JCODE_HOOK_PAYLOAD" | curl -sS -X POST "http://127.0.0.1:${ORCA_AGENT_HOOK_PORT}/hook/jcode" \\', - ' --connect-timeout 0.5 --max-time 1.5 \\', - ' -H "Content-Type: application/x-www-form-urlencoded" \\', - ' -H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}" \\', - ' --data-urlencode "paneKey=${ORCA_PANE_KEY}" \\', - ' --data-urlencode "tabId=${ORCA_TAB_ID}" \\', - ' --data-urlencode "launchToken=${ORCA_AGENT_LAUNCH_TOKEN}" \\', - ' --data-urlencode "worktreeId=${ORCA_WORKTREE_ID}" \\', - ' --data-urlencode "env=${ORCA_AGENT_HOOK_ENV}" \\', - ' --data-urlencode "version=${ORCA_AGENT_HOOK_VERSION}" \\', - ' --data-urlencode "hook_event_name=${JCODE_HOOK_EVENT}" \\', - ' --data-urlencode "session_id=${JCODE_HOOK_SESSION_ID}" \\', - ' --data-urlencode "cwd=${JCODE_HOOK_CWD}" \\', - ' --data-urlencode "payload@-" >/dev/null 2>&1 || true', + ...buildPosixAgentHookPostCommand('jcode').map((line) => ` ${line}`), '}', - // Why: jcode reads this gate's stderr to EOF before releasing the tool call, so - // the POST runs detached with both pipes closed. Orca observes the tool live and - // adds no latency; the gate always allows (Orca never blocks a jcode tool). + // Why detached on the gate: jcode reads this script's stderr to EOF before it + // releases the tool call, so an inherited pipe would hold the tool open for as long + // as the POST ran. Orca observes the tool live and adds no latency; the gate always + // allows, because Orca never blocks a jcode tool. 'if [ "$JCODE_HOOK_EVENT" = pre_tool ]; then', ' orca_post_jcode_event >/dev/null 2>&1 &', - ' exit 0', + 'else', + ' orca_post_jcode_event >/dev/null 2>&1 || :', 'fi', - 'orca_post_jcode_event', 'exit 0', '' ].join('\n') diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-launch-agent-candidate.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-launch-agent-candidate.ts index a8525470c46..de997902a46 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-launch-agent-candidate.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-launch-agent-candidate.ts @@ -27,15 +27,12 @@ export function resolvePaneLaunchAgentCandidate( state: PaneLaunchAgentStoreSlice, pane: PaneLaunchAgentPaneSlice ): string | undefined { - const tab = (state.tabsByWorktree[pane.worktreeId] ?? []).find( - (candidate) => candidate.id === pane.tabId - ) - const registeredLaunchAgent = state.agentLaunchConfigByPaneKey[pane.paneKey]?.identity?.agentType + const registered = state.agentLaunchConfigByPaneKey[pane.paneKey]?.identity?.agentType return ( - tab?.launchAgent ?? + state.tabsByWorktree[pane.worktreeId]?.find((tab) => tab.id === pane.tabId)?.launchAgent ?? pane.startup?.launchAgent ?? pane.startup?.initialAgentStatus?.agent ?? - (isTuiAgent(registeredLaunchAgent) ? registeredLaunchAgent : undefined) + (isTuiAgent(registered) ? registered : undefined) ) } diff --git a/src/shared/agent-hook-listener-jcode.test.ts b/src/shared/agent-hook-listener-jcode.test.ts index 3d35729fa61..bd79d7ef955 100644 --- a/src/shared/agent-hook-listener-jcode.test.ts +++ b/src/shared/agent-hook-listener-jcode.test.ts @@ -211,6 +211,22 @@ describe('shared agent-hook-listener: jcode', () => { expect(event?.providerSession).toEqual({ key: 'session_id', id: 'session_jc_5' }) }) + it('reads the lifecycle point from jcode\u2019s own `event` key', () => { + // Why this matters: the managed script posts JCODE_HOOK_PAYLOAD verbatim through + // the shared hook transport, so nothing re-states the event as a form field — + // jcode names it `event`, and the listener has to accept that. + const event = normalizeHookPayload( + state, + 'jcode', + { + paneKey: PANE_KEY, + payload: { event: 'turn_start', session_id: 'session_jc_7', model: 'claude-haiku-4-5' } + }, + 'production' + ) + expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'working' }) + }) + it('does not count a direct jcode prompt without journal evidence as explicit', () => { // Why: regression — a prompt field on a hook event has no journal backing, so // it must not set hasExplicitPrompt. diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 6246ef42d2f..b8dba7cc3ec 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -60,7 +60,11 @@ export function normalizeHookPayload( const eventName = readFirstString(record, ['hook_event_name', 'hookEventName', 'hook_type', 'hookType']) ?? hookPayloadRecord.hook_event_name ?? - hookPayloadRecord.hookEventName + hookPayloadRecord.hookEventName ?? + // Why jcode only: its payload names the lifecycle point `event`, and it is posted + // verbatim through the shared transport rather than re-stated as a form field. + // Scoped so another provider's unrelated `event` key cannot become an event name. + (source === 'jcode' ? hookPayloadRecord.event : undefined) // Codex child hooks expose the child's session_id on the parent's pane. const providerSession = source === 'codex' && readString(hookPayloadRecord, 'agent_id') diff --git a/src/shared/agent-hook-listener/command-code-transcript.ts b/src/shared/agent-hook-listener/command-code-transcript.ts index b3ec2a1d497..e2fccf259a9 100644 --- a/src/shared/agent-hook-listener/command-code-transcript.ts +++ b/src/shared/agent-hook-listener/command-code-transcript.ts @@ -1,10 +1,9 @@ import { createHash } from 'node:crypto' -import { closeSync, openSync, readSync, statSync } from 'node:fs' import { parseAgentHookJson } from './request-body' +import { scanFileRegionsBackward } from './reverse-file-region-scan' import { extractAssistantContentText } from './transcript-entry-text' import { - EMPTY_TRANSCRIPT_REGION, readLastTextFromTranscriptOnce, TRANSCRIPT_CHUNK_BYTES, TRANSCRIPT_MAX_SCAN_BYTES @@ -59,91 +58,24 @@ export function readLastCommandCodeUserPromptEntryFromTranscript( if (typeof transcriptPath !== 'string' || transcriptPath.length === 0) { return undefined } - try { - const stats = statSync(transcriptPath) - const size = stats.size - if (size <= 0) { - return undefined - } - const fd = openSync(transcriptPath, 'r') - try { - // Why scan backward: the answer is the LAST user line, so walking up from - // EOF returns on the first hit instead of parsing every line of a - // multi-megabyte transcript on every hook event. - // Why a chunk list: carry holds a partial line, and re-concatenating it per - // block made one oversized line (a big tool result) cost O(line^2). - let carryChunks: Buffer[] = [] - let bytesRead = 0 - let scanEnd = size - while (scanEnd > 0 && bytesRead < TRANSCRIPT_MAX_SCAN_BYTES) { - const chunkSize = Math.min( - scanEnd, - TRANSCRIPT_CHUNK_BYTES, - TRANSCRIPT_MAX_SCAN_BYTES - bytesRead - ) - const position = scanEnd - chunkSize - const buffer = Buffer.alloc(chunkSize) - let filled = 0 - while (filled < chunkSize) { - const n = readSync(fd, buffer, filled, chunkSize - filled, position + filled) - if (n === 0) { - break + return scanFileRegionsBackward( + transcriptPath, + { chunkBytes: TRANSCRIPT_CHUNK_BYTES, maxScanBytes: TRANSCRIPT_MAX_SCAN_BYTES }, + (region, regionPosition) => { + const found = findLastCommandCodePromptInRegion(region) + return found + ? { + text: found.prompt, + interactionKey: [ + 'command-code-transcript', + hashInteractionKeyPart(transcriptPath), + String(regionPosition + found.byteOffset), + hashInteractionKeyPart(found.prompt) + ].join('-') } - filled += n - } - // Why bail on a short read: the file shrank under us, so the bytes above - // this block no longer line up and any stitched offset would be wrong. - if (filled < chunkSize) { - break - } - bytesRead += filled - scanEnd = position - // Why search only the new block: carry is always the run before a newline, - // so it holds none of its own. - const firstNewline = buffer.indexOf(0x0a) - // Why only at a true file start: a scan that stops on the size cap must - // discard its leading partial line, exactly as the capped read did. - const atStart = position === 0 - let completeRegion: Buffer - let regionPosition: number - if (atStart) { - completeRegion = - carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) - regionPosition = position - carryChunks = [] - } else if (firstNewline === -1) { - completeRegion = EMPTY_TRANSCRIPT_REGION - regionPosition = position - carryChunks.unshift(buffer) - } else { - const afterNewline = buffer.subarray(firstNewline + 1) - completeRegion = - carryChunks.length === 0 ? afterNewline : Buffer.concat([afterNewline, ...carryChunks]) - regionPosition = position + firstNewline + 1 - carryChunks = [buffer.subarray(0, firstNewline)] - } - if (completeRegion.length > 0) { - const found = findLastCommandCodePromptInRegion(completeRegion) - if (found) { - return { - text: found.prompt, - interactionKey: [ - 'command-code-transcript', - hashInteractionKeyPart(transcriptPath), - String(regionPosition + found.byteOffset), - hashInteractionKeyPart(found.prompt) - ].join('-') - } - } - } - } - return undefined - } finally { - closeSync(fd) + : undefined } - } catch { - return undefined - } + ) } export function extractCommandCodeAssistantTextFromLine(line: string): string | undefined { diff --git a/src/shared/agent-hook-listener/providers/jcode-events.ts b/src/shared/agent-hook-listener/providers/jcode-events.ts index 89dc65143a1..de0d37f7414 100644 --- a/src/shared/agent-hook-listener/providers/jcode-events.ts +++ b/src/shared/agent-hook-listener/providers/jcode-events.ts @@ -1,5 +1,6 @@ import { normalizeAgentStatusPayload, + type AgentStatusState, type ParsedAgentStatusPayload } from '../../agent-status-types' import { clearPaneTurnCacheState, type HookListenerState } from '../listener-state' @@ -8,9 +9,14 @@ import { extractToolFields, isNewTurnEvent } from '../provider-event-routing' import { readString } from '../tool-input-preview' import { isJcodeUserInputTool } from './jcode-tool-fields' -/** Text jcode reports for a failed turn or tool, preferred over a stale reply. */ -function readJcodeErrorText(hookPayload: Record): string | undefined { - return hookPayload.status === 'error' ? readString(hookPayload, 'error') : undefined +// jcode's six lifecycle points, mapped the way docs/reference/jcode-hook-events.md +// records them. `session_start` is absent on purpose: it returns early below. +const JCODE_EVENT_STATES: Record = { + turn_start: 'working', + pre_tool: 'working', + post_tool: 'working', + turn_end: 'done', + session_end: 'done' } export function normalizeJcodeEvent( @@ -27,34 +33,29 @@ export function normalizeJcodeEvent( return null } - const toolName = readString(hookPayload, 'tool_name') - // Why: only the pre_tool gate can report a pending question — post_tool fires - // after the human already answered it. - const isPendingUserInput = eventName === 'pre_tool' && isJcodeUserInputTool(toolName) - const stateName = isPendingUserInput - ? 'waiting' - : eventName === 'turn_start' || eventName === 'pre_tool' || eventName === 'post_tool' - ? 'working' - : eventName === 'turn_end' || eventName === 'session_end' - ? 'done' - : null - + // Why the gate only: post_tool for the same tool fires after the human already + // answered, so it must not re-open the question. + const stateName = + eventName === 'pre_tool' && isJcodeUserInputTool(readString(hookPayload, 'tool_name')) + ? 'waiting' + : JCODE_EVENT_STATES[String(eventName)] if (!stateName) { return null } + const resetOnNewTurn = isNewTurnEvent('jcode', eventName) const snapshot = resolveToolState( state, paneKey, extractToolFields('jcode', eventName, hookPayload), - { resetOnNewTurn: isNewTurnEvent('jcode', eventName) } + { resetOnNewTurn } ) + // Why the error text first: a failed turn's own message beats the reply it never replaced. + const errorText = hookPayload.status === 'error' ? readString(hookPayload, 'error') : undefined return normalizeAgentStatusPayload({ state: stateName, - prompt: resolvePrompt(state, paneKey, promptText, { - resetOnNewTurn: isNewTurnEvent('jcode', eventName) - }), + prompt: resolvePrompt(state, paneKey, promptText, { resetOnNewTurn }), agentType: 'jcode', // Why: jcode stamps the live model on session_start/turn_start/turn_end, so // the row keeps naming the right model after an in-session `/model` switch. @@ -62,6 +63,6 @@ export function normalizeJcodeEvent( toolName: snapshot.toolName, toolInput: snapshot.toolInput, interactivePrompt: snapshot.interactivePrompt, - lastAssistantMessage: readJcodeErrorText(hookPayload) ?? snapshot.lastAssistantMessage + lastAssistantMessage: errorText ?? snapshot.lastAssistantMessage }) } diff --git a/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts b/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts index 5c4392a83ab..35d48f4650d 100644 --- a/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts +++ b/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts @@ -7,16 +7,15 @@ import { toolUpdate } from '../tool-input-preview' -// Why: jcode has no per-tool approval prompt — its safety model denies or asks -// the model to reflect, both inside the tool. The one tool a *human* answers is -// ambient mode's `request_permission` (crates/jcode-app-core/src/tool/ambient.rs), -// resolved out of band with `jcode permissions`. Matching by exact name keeps a -// future rename visible instead of silently widening to unrelated tools; the -// aliases are the names jcode has shipped for the same surface. -const JCODE_USER_INPUT_TOOLS = new Set(['request_permission', 'ask_user', 'ask_question']) - +/** True for the one jcode tool a *human* answers. + * + * jcode has no per-tool approval prompt — its safety model denies or asks the model + * to reflect, both inside the tool. `request_permission` + * (crates/jcode-app-core/src/tool/ambient.rs) is the only surface that waits on a + * person, resolved out of band with `jcode permissions`. Matched by exact name so a + * rename fails loudly here rather than silently widening to unrelated tools. */ export function isJcodeUserInputTool(toolName: string | undefined): boolean { - return toolName !== undefined && JCODE_USER_INPUT_TOOLS.has(toolName) + return toolName === 'request_permission' } /** jcode's `tool_input` field is the tool's argument JSON as a string. */ diff --git a/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts b/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts new file mode 100644 index 00000000000..8e929a83e76 --- /dev/null +++ b/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { scanFileRegionsBackward } from './reverse-file-region-scan' + +const roots: string[] = [] +afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } +}) + +function writeFixture(contents: string): string { + const root = mkdtempSync(join(tmpdir(), 'orca-reverse-scan-')) + roots.push(root) + const file = join(root, 'transcript.jsonl') + writeFileSync(file, contents) + return file +} + +/** Every whole line the scan handed out, in the order the caller saw them. */ +function collectLines(file: string, chunkBytes: number, maxScanBytes = 1024 * 1024): string[] { + const seen: string[] = [] + scanFileRegionsBackward(file, { chunkBytes, maxScanBytes }, (region) => { + seen.push(...region.toString('utf8').split('\n').filter(Boolean)) + return undefined + }) + return seen +} + +describe('scanFileRegionsBackward', () => { + it('yields every whole line exactly once, whatever the chunk size', () => { + const lines = Array.from({ length: 40 }, (_, i) => `line-${i}`) + const file = writeFixture(`${lines.join('\n')}\n`) + for (const chunkBytes of [8, 16, 64, 4096]) { + // Why sorted: the scan walks backwards, so the caller sees later lines first. + expect([...collectLines(file, chunkBytes)].sort()).toEqual([...lines].sort()) + } + }) + + it('stops at the first defined result without reading the rest of the file', () => { + const file = writeFixture(`${Array.from({ length: 200 }, (_, i) => `line-${i}`).join('\n')}\n`) + let regions = 0 + const found = scanFileRegionsBackward( + file, + { chunkBytes: 32, maxScanBytes: 1024 * 1024 }, + () => { + regions += 1 + return 'first' + } + ) + expect(found).toBe('first') + expect(regions).toBe(1) + }) + + it('reports the byte offset each region starts at', () => { + const file = writeFixture('alpha\nbravo\ncharlie\n') + const offsets = scanFileRegionsBackward( + file, + { chunkBytes: 4096, maxScanBytes: 4096 }, + (region, regionPosition) => ({ text: region.toString('utf8'), regionPosition }) + ) + expect(offsets).toEqual({ text: 'alpha\nbravo\ncharlie\n', regionPosition: 0 }) + }) + + it('drops the leading partial line when it stops on the byte budget', () => { + // Why: a scan capped mid-file has not seen the start of its topmost line, so + // emitting it would hand the caller a truncated record. + const file = writeFixture(`${'x'.repeat(500)}\nkeep-me\n`) + expect(collectLines(file, 64, 128)).toEqual(['keep-me']) + }) + + it('reads a file with no trailing newline', () => { + expect(collectLines(writeFixture('alpha\nbravo'), 4)).toContain('bravo') + }) + + it('treats an empty, missing, or unreadable file as nothing found', () => { + expect(collectLines(writeFixture(''), 64)).toEqual([]) + expect( + scanFileRegionsBackward( + '/nonexistent/transcript.jsonl', + { + chunkBytes: 64, + maxScanBytes: 64 + }, + () => 'x' + ) + ).toBeUndefined() + }) +}) diff --git a/src/shared/agent-hook-listener/reverse-file-region-scan.ts b/src/shared/agent-hook-listener/reverse-file-region-scan.ts new file mode 100644 index 00000000000..7f34fae3834 --- /dev/null +++ b/src/shared/agent-hook-listener/reverse-file-region-scan.ts @@ -0,0 +1,88 @@ +import { closeSync, openSync, readSync, statSync } from 'node:fs' + +/** + * Walk a file backwards in chunks, handing each caller a region of whole lines. + * + * Why backward: every caller wants the LAST entry that matches, so walking up from + * EOF returns on the first hit instead of parsing every line of a multi-megabyte + * transcript on every hook event. + * + * Why a chunk list rather than a growing buffer: `carry` holds the partial line that + * straddles a block boundary, and re-concatenating it per block made one oversized + * line (a big tool result or pasted prompt) cost O(line²). + * + * `visit` receives only whole lines, and the byte offset that region starts at. + * The scan stops at the first defined result, at the byte budget, or at the file + * start — whichever comes first. Any error reads as "nothing found". + */ +export function scanFileRegionsBackward( + filePath: string, + limits: { chunkBytes: number; maxScanBytes: number }, + visit: (region: Buffer, regionPosition: number) => T | undefined +): T | undefined { + try { + const size = statSync(filePath).size + if (size <= 0) { + return undefined + } + const fd = openSync(filePath, 'r') + try { + let carryChunks: Buffer[] = [] + let bytesRead = 0 + let scanEnd = size + while (scanEnd > 0 && bytesRead < limits.maxScanBytes) { + const chunkSize = Math.min(scanEnd, limits.chunkBytes, limits.maxScanBytes - bytesRead) + const position = scanEnd - chunkSize + const buffer = Buffer.alloc(chunkSize) + let filled = 0 + while (filled < chunkSize) { + const read = readSync(fd, buffer, filled, chunkSize - filled, position + filled) + if (read === 0) { + break + } + filled += read + } + // Why bail on a short read: the file shrank under us, so the bytes above this + // block no longer line up with what the earlier ones assumed. + if (filled < chunkSize) { + return undefined + } + bytesRead += filled + scanEnd = position + // Why search only the new block: carry is always the run before a newline, so + // it holds none of its own. + const firstNewline = buffer.indexOf(0x0a) + let region: Buffer + let regionPosition = position + if (position === 0) { + // Only at a true file start is the leading partial line a whole line. A scan + // that stops on the byte cap must discard it, as a capped read would. + region = carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) + carryChunks = [] + } else if (firstNewline === -1) { + region = EMPTY_REGION + carryChunks.unshift(buffer) + } else { + const afterNewline = buffer.subarray(firstNewline + 1) + region = + carryChunks.length === 0 ? afterNewline : Buffer.concat([afterNewline, ...carryChunks]) + regionPosition = position + firstNewline + 1 + carryChunks = [buffer.subarray(0, firstNewline)] + } + if (region.length > 0) { + const found = visit(region, regionPosition) + if (found !== undefined) { + return found + } + } + } + return undefined + } finally { + closeSync(fd) + } + } catch { + return undefined + } +} + +const EMPTY_REGION = Buffer.alloc(0) diff --git a/src/shared/agent-hook-listener/transcript-reader.ts b/src/shared/agent-hook-listener/transcript-reader.ts index 89df71da2df..49e6c85e64c 100644 --- a/src/shared/agent-hook-listener/transcript-reader.ts +++ b/src/shared/agent-hook-listener/transcript-reader.ts @@ -1,6 +1,5 @@ -import { closeSync, openSync, readSync, statSync } from 'node:fs' - import { extractAssistantTextFromLine } from './transcript-entry-text' +import { scanFileRegionsBackward } from './reverse-file-region-scan' export const TRANSCRIPT_CHUNK_BYTES = 64 * 1024 export const TRANSCRIPT_MAX_SCAN_BYTES = 4 * 1024 * 1024 @@ -13,73 +12,11 @@ export function readLastTextFromTranscriptOnce( transcriptPath: string, extractLineText: (line: string) => string | undefined ): string | undefined { - try { - const stats = statSync(transcriptPath) - const size = stats.size - if (size <= 0) { - return undefined - } - const fd = openSync(transcriptPath, 'r') - try { - // Why a chunk list: carry holds a partial line, and re-joining it per block - // made one oversized line (a big tool result or pasted prompt) cost O(line^2). - let carryChunks: Buffer[] = [] - let bytesRead = 0 - let scanEnd = size - while (scanEnd > 0 && bytesRead < TRANSCRIPT_MAX_SCAN_BYTES) { - const chunkSize = Math.min(scanEnd, TRANSCRIPT_CHUNK_BYTES) - const position = scanEnd - chunkSize - const buffer = Buffer.alloc(chunkSize) - let filled = 0 - while (filled < chunkSize) { - const n = readSync(fd, buffer, filled, chunkSize - filled, position + filled) - if (n === 0) { - break - } - filled += n - } - // Why bail on a short read: the file shrank under us, so the bytes above - // this block no longer line up with what the earlier ones assumed. - if (filled < chunkSize) { - break - } - bytesRead += filled - scanEnd = position - // Why search only the new block: carry is always the run before a newline, - // so it holds none of its own. - const firstNewline = buffer.indexOf(0x0a) - const atStart = position === 0 - let completeRegion: Buffer - if (atStart) { - completeRegion = - carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) - carryChunks = [] - } else if (firstNewline === -1) { - completeRegion = EMPTY_TRANSCRIPT_REGION - carryChunks.unshift(buffer) - } else { - const afterNewline = buffer.subarray(firstNewline + 1) - completeRegion = - carryChunks.length === 0 ? afterNewline : Buffer.concat([afterNewline, ...carryChunks]) - carryChunks = [buffer.subarray(0, firstNewline)] - } - if (completeRegion.length > 0) { - const extracted = findLastExtractedTranscriptLineText( - completeRegion.toString('utf8'), - extractLineText - ) - if (extracted !== undefined) { - return extracted - } - } - } - return undefined - } finally { - closeSync(fd) - } - } catch { - return undefined - } + return scanFileRegionsBackward( + transcriptPath, + { chunkBytes: TRANSCRIPT_CHUNK_BYTES, maxScanBytes: TRANSCRIPT_MAX_SCAN_BYTES }, + (region) => findLastExtractedTranscriptLineText(region.toString('utf8'), extractLineText) + ) } export function findLastExtractedTranscriptLineText( diff --git a/src/shared/jcode-session-files.ts b/src/shared/jcode-session-files.ts index 73c5eeb04a9..c31278a8327 100644 --- a/src/shared/jcode-session-files.ts +++ b/src/shared/jcode-session-files.ts @@ -4,15 +4,15 @@ // consolidated `session_*.json` document. Bounded like the Grok/Command Code // transcript readers so hook events stay cheap on multi-megabyte sessions. import { createHash } from 'node:crypto' -import { closeSync, openSync, readFileSync, readSync, statSync } from 'node:fs' +import { readFileSync, statSync } from 'node:fs' import { homedir } from 'node:os' import { join } from 'node:path' +import { scanFileRegionsBackward } from './agent-hook-listener/reverse-file-region-scan' const JCODE_SESSION_ID_MAX_LENGTH = 512 const JCODE_SESSION_SCAN_BYTES = 4 * 1024 * 1024 const JCODE_JOURNAL_CHUNK_BYTES = 64 * 1024 const JCODE_JSON_DOC_MAX_PARSE_BYTES = 8 * 1024 * 1024 -const EMPTY_REGION = Buffer.alloc(0) function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null @@ -136,68 +136,15 @@ function readLastUserMessageFromJournal( journalPath: string, sessionId: string ): JcodeUserPromptEvidence | null { - let stats: ReturnType - try { - stats = statSync(journalPath) - } catch { - return null - } - if (stats.size <= 0) { - return null - } - try { - const fd = openSync(journalPath, 'r') - try { - // Why backward scan: the last user message is near EOF; the first hit - // returns instead of parsing a multi-megabyte log from the top. - let carryChunks: Buffer[] = [] - let bytesRead = 0 - let scanEnd = stats.size - while (scanEnd > 0 && bytesRead < JCODE_SESSION_SCAN_BYTES) { - const chunkSize = Math.min( - scanEnd, - JCODE_JOURNAL_CHUNK_BYTES, - JCODE_SESSION_SCAN_BYTES - bytesRead - ) - const position = scanEnd - chunkSize - const buffer = Buffer.alloc(chunkSize) - const filled = readSync(fd, buffer, 0, chunkSize, position) - if (filled < chunkSize) { - break - } - bytesRead += filled - scanEnd = position - const firstNewline = buffer.indexOf(0x0a) - const atStart = position === 0 - let completeRegion: Buffer - if (atStart) { - completeRegion = - carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) - carryChunks = [] - } else if (firstNewline === -1) { - completeRegion = EMPTY_REGION - carryChunks.unshift(buffer) - } else { - const afterNewline = buffer.subarray(firstNewline + 1) - completeRegion = - carryChunks.length === 0 ? afterNewline : Buffer.concat([afterNewline, ...carryChunks]) - carryChunks = [buffer.subarray(0, firstNewline)] - } - if (completeRegion.length > 0) { - const lines = completeRegion.toString('utf8').split('\n') - const found = readLastUserMessageFromJournalLines(lines, sessionId) - if (found) { - return found - } - } - } - return null - } finally { - closeSync(fd) - } - } catch { - return null - } + return ( + scanFileRegionsBackward( + journalPath, + { chunkBytes: JCODE_JOURNAL_CHUNK_BYTES, maxScanBytes: JCODE_SESSION_SCAN_BYTES }, + (region) => + readLastUserMessageFromJournalLines(region.toString('utf8').split('\n'), sessionId) ?? + undefined + ) ?? null + ) } function readLastUserMessageFromJson( diff --git a/src/shared/jcode-terminal-title.test.ts b/src/shared/jcode-terminal-title.test.ts index a6edcd29b66..8847ff7a2e3 100644 --- a/src/shared/jcode-terminal-title.test.ts +++ b/src/shared/jcode-terminal-title.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { isJcodeIdentityTerminalTitle, stripJcodeTitleMetrics } from './jcode-terminal-title' +import { isJcodeIdentityTerminalTitle, stripJcodeTitleStatus } from './jcode-terminal-title' // Every title below was captured from a real jcode 0.87.1 TUI session; see // docs/reference/jcode-hook-events.md. @@ -19,11 +19,11 @@ const CAPTURED_TITLES = [ describe('jcode terminal titles', () => { it('strips the live diff and duration segments', () => { - expect(stripJcodeTitleMetrics('jcode Puppy · +3 -0 · last ~23s')).toBe('jcode Puppy') - expect(stripJcodeTitleMetrics('🌐 jcode Puppy · +3 -0 · last ~23s')).toBe('jcode Puppy') - expect(stripJcodeTitleMetrics('jcode Snake · work ~6s')).toBe('jcode Snake') - expect(stripJcodeTitleMetrics('jcode Snake · last ~1m02s')).toBe('jcode Snake') - expect(stripJcodeTitleMetrics('jcode Snake · work ~2h05m')).toBe('jcode Snake') + expect(stripJcodeTitleStatus('jcode Puppy · +3 -0 · last ~23s')).toBe('jcode Puppy') + expect(stripJcodeTitleStatus('🌐 jcode Puppy · +3 -0 · last ~23s')).toBe('jcode Puppy') + expect(stripJcodeTitleStatus('jcode Snake · work ~6s')).toBe('jcode Snake') + expect(stripJcodeTitleStatus('jcode Snake · last ~1m02s')).toBe('jcode Snake') + expect(stripJcodeTitleStatus('jcode Snake · work ~2h05m')).toBe('jcode Snake') }) it.each(CAPTURED_TITLES)('treats %j as identity, not a conversation name', (title) => { diff --git a/src/shared/jcode-terminal-title.ts b/src/shared/jcode-terminal-title.ts index cff1536d00f..7ebf95c506a 100644 --- a/src/shared/jcode-terminal-title.ts +++ b/src/shared/jcode-terminal-title.ts @@ -1,27 +1,21 @@ -// jcode paints ` jcode [ · +N -M][ · work|last ~]` about -// once a second (crates/jcode-tui/src/tui/app/terminal_title.rs). Everything after -// the first ` · ` is live metrics, and the head is jcode's identity plus the -// codename it generates per session ("Puppy", "Tigress") — a label, never a +// jcode paints ` jcode [ · +N -M][ · work|last ~]` about once +// a second (crates/jcode-tui/src/tui/app/terminal_title.rs). The tail is live status, +// the emoji is picked per session and swapped mid-turn, and the head is jcode's own +// name plus the codename it generates ("Puppy", "Tigress") — a label, never a // conversation name. Captured titles are in docs/reference/jcode-hook-events.md. -const JCODE_TITLE_METRICS_RE = /\s+·\s+(?:\+\d+\s+-\d+|(?:work|last)\s+~\S+)(?=\s+·\s+|$)/gu -// jcode picks a per-session emoji (🐍 Snake, 🐕 Puppy) and swaps it for a status one -// mid-turn, so it identifies nothing stable. It is not in the shared decoration -// stripper because that list is a fixed set of agent status glyphs, not open-ended. -const JCODE_TITLE_LEADING_EMOJI_RE = /^(?:\p{Extended_Pictographic}|\uFE0F|\u200D)+\s*/u +const JCODE_TITLE_STATUS_RE = + /^[\p{Extended_Pictographic}\u{FE0F}\u{200D}]+\s*|\s+·\s+(?:\+\d+\s+-\d+|(?:work|last)\s+~\S+)(?=\s+·\s+|$)/gu -/** The jcode title with its leading emoji and live diff/duration segments removed. */ -export function stripJcodeTitleMetrics(title: string): string { - return title.replace(JCODE_TITLE_LEADING_EMOJI_RE, '').replace(JCODE_TITLE_METRICS_RE, '').trim() +/** The jcode title with its per-session emoji and live diff/duration segments removed. */ +export function stripJcodeTitleStatus(title: string): string { + return title.replace(JCODE_TITLE_STATUS_RE, '').trim() } -// `jcode` alone, `jcode Puppy`, and `jcode/creek Puppy` (the self-dev variant) are -// all identity; anything the user could recognise as their own work has more to it. +// `jcode`, `jcode Puppy`, and `jcode/creek Puppy` (the self-dev variant) are all +// identity; anything the user could recognise as their own work has more to it. const JCODE_IDENTITY_TITLE_RE = /^jcode(?:\/[^\s·]+)?(?:\s+[^\s·]+)?$/iu /** True when a jcode title says only which jcode session this is, not what it is doing. */ export function isJcodeIdentityTerminalTitle(title: string | null | undefined): boolean { - if (!title) { - return false - } - return JCODE_IDENTITY_TITLE_RE.test(stripJcodeTitleMetrics(title)) + return Boolean(title) && JCODE_IDENTITY_TITLE_RE.test(stripJcodeTitleStatus(title ?? '')) }