diff --git a/src/main/agent-hooks/managed-agent-hook-registry.ts b/src/main/agent-hooks/managed-agent-hook-registry.ts index e09d556527f..3cdfeab98a3 100644 --- a/src/main/agent-hooks/managed-agent-hook-registry.ts +++ b/src/main/agent-hooks/managed-agent-hook-registry.ts @@ -93,7 +93,8 @@ export const MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS: readonly ManagedAgentHookScri ['kimi', () => kimiHookService.refreshManagedScripts()], ['muse', () => museHookService.refreshManagedScripts()], ['zcode', () => zcodeHookService.refreshManagedScripts()], - ['dsh', () => dshHookService.refreshManagedScripts()] + ['dsh', () => dshHookService.refreshManagedScripts()], + ['jcode', () => jcodeHookService.refreshManagedScripts()] ] export const MANAGED_AGENT_HOOK_REMOVERS: readonly ManagedAgentHookRemover[] = [ diff --git a/src/main/agent-hooks/managed-hook-command-contract.test.ts b/src/main/agent-hooks/managed-hook-command-contract.test.ts index c38c30ac77b..051e47259d7 100644 --- a/src/main/agent-hooks/managed-hook-command-contract.test.ts +++ b/src/main/agent-hooks/managed-hook-command-contract.test.ts @@ -24,6 +24,7 @@ import { getGrokManagedCommand } from '../grok/grok-hook-script' import { getMuseManagedCommand, getMuseRemoteManagedCommand } from '../muse/hook-settings' import { getDshManagedCommand, getDshRemoteManagedCommand } from '../dsh/hook-settings' import { getZCodeManagedCommand, getZCodeRemoteManagedCommand } from '../zcode/hook-settings' +import { getJcodeManagedCommand, getJcodeRemoteManagedCommand } from '../jcode/hook-settings' import { wrapPosixHookCommand, wrapWindowsCmdHookCommand, @@ -217,6 +218,15 @@ const buildersByAgent = new Map([ local: (path) => [getZCodeManagedCommand(path)], remote: (path) => [getZCodeRemoteManagedCommand(path)] } + ], + [ + // Why bare: jcode parses the hook command line shell-style but executes it + // directly, so a `sh -c`/`if [ -f … ]` wrapper would be run as the program name. + 'jcode', + { + local: (path) => [getJcodeManagedCommand(path)], + remote: (path) => [getJcodeRemoteManagedCommand(path)] + } ] ]) diff --git a/src/main/agent-hooks/server-retired-pane-new-turn.test.ts b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts index c6c27a734c8..aec2999e7ec 100644 --- a/src/main/agent-hooks/server-retired-pane-new-turn.test.ts +++ b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts @@ -48,7 +48,7 @@ const NEW_TURN_EVENT: Record = { muse: 'UserPromptSubmit', zcode: 'SessionStart', dsh: 'SessionStart', - jcode: 'session_start' + jcode: 'turn_start' } function reviveRetiredPane(source: unknown, hookEventName: string): boolean { diff --git a/src/main/jcode/hook-config.test.ts b/src/main/jcode/hook-config.test.ts index ea0453643da..c51093b941d 100644 --- a/src/main/jcode/hook-config.test.ts +++ b/src/main/jcode/hook-config.test.ts @@ -84,6 +84,41 @@ session_start = "~/bin/mine" expect(result.content).toContain('session_start = "~/bin/mine"') }) + it('keeps every table declared after [hooks] when removing managed entries', () => { + // Why: `remote` rebuilds the file from the lines it keeps, so an early exit at + // the next table header silently truncated the rest of a user's config. + const source = `[hooks] +turn_end = ${tomlQuoteString(MANAGED_COMMAND)} +pre_tool_timeout_ms = 5000 + +[terminal] +preferred = "ghostty" + +[ui] +theme = "dark" +` + const result = removeJcodeManagedHooks(source, 'jcode-hook.sh') + expect(result.changed).toBe(true) + expect(result.content).not.toContain(MANAGED_COMMAND) + expect(result.content).toContain('pre_tool_timeout_ms = 5000') + expect(result.content).toContain('[terminal]') + expect(result.content).toContain('preferred = "ghostty"') + expect(result.content).toContain('[ui]') + expect(result.content).toContain('theme = "dark"') + }) + + it('does not touch a managed-looking command outside the [hooks] table', () => { + const source = `[terminal] +spawn_hook = ${tomlQuoteString(MANAGED_COMMAND)} + +[hooks] +turn_end = ${tomlQuoteString(MANAGED_COMMAND)} +` + const result = removeJcodeManagedHooks(source, 'jcode-hook.sh') + expect(result.content).toContain(`spawn_hook = ${tomlQuoteString(MANAGED_COMMAND)}`) + expect(result.content).not.toContain(`turn_end = ${tomlQuoteString(MANAGED_COMMAND)}`) + }) + it('keeps CRLF line endings when editing a Windows-owned config', () => { const source = '[hooks]\r\nturn_end = "~/bin/mine"\r\n' const result = applyJcodeManagedHooks(source, EVENTS, MANAGED_COMMAND, 'jcode-hook.sh') diff --git a/src/main/jcode/hook-config.ts b/src/main/jcode/hook-config.ts index 974a72f4c40..b5bea534c8c 100644 --- a/src/main/jcode/hook-config.ts +++ b/src/main/jcode/hook-config.ts @@ -178,12 +178,10 @@ export function removeJcodeManagedHooks( } const header = getTomlTableHeader(line) if (header) { - if (inHooksTable) { - break - } - if (parseTomlTablePath(header)?.join('.') === 'hooks') { - inHooksTable = true - } + // Why: leaving the table stops the removal, but the rest of the file must + // still be copied out — `kept` is the whole result, so breaking here once + // truncated every table declared after [hooks]. + inHooksTable = parseTomlTablePath(header)?.join('.') === 'hooks' kept.push(line) state = updateTomlLineScanState(state, line) continue diff --git a/src/main/jcode/hook-gate-script.test.ts b/src/main/jcode/hook-gate-script.test.ts new file mode 100644 index 00000000000..31e539f5e0e --- /dev/null +++ b/src/main/jcode/hook-gate-script.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it, vi } from 'vitest' +import { execFileSync } from 'node:child_process' +import { createServer, type Server } from 'node:net' +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' + +const { homedirMock } = vi.hoisted(() => ({ homedirMock: vi.fn<() => string>() })) +vi.mock('os', async () => { + const actual = (await vi.importActual('os')) as Record + return { ...actual, homedir: homedirMock } +}) + +import { JcodeHookService } from './hook-service' +import { getJcodeManagedScriptPath } from './hook-settings' + +/** Installs the managed hook into a throwaway home and returns the script path. */ +function installManagedScript(): { scriptPath: string; cleanup: () => void } { + const homeDir = mkdtempSync(join(tmpdir(), 'orca-jcode-gate-')) + homedirMock.mockReturnValue(homeDir) + vi.stubEnv('JCODE_HOME', join(homeDir, '.jcode')) + new JcodeHookService().install() + const scriptPath = getJcodeManagedScriptPath() + return { + scriptPath, + cleanup: () => { + vi.unstubAllEnvs() + rmSync(homeDir, { recursive: true, force: true }) + } + } +} + +describe.runIf(process.platform !== 'win32')('jcode managed hook as jcode runs it', () => { + it('returns immediately on pre_tool even when the hook server never answers', async () => { + const { scriptPath, cleanup } = installManagedScript() + // A server that accepts the connection and then never replies: curl holds it + // open until its own --max-time 1.5, which is what a synchronous gate would + // hand straight to the agent on every single tool call. + const blackHole: Server = createServer(() => {}) + await new Promise((resolve) => blackHole.listen(0, '127.0.0.1', resolve)) + const address = blackHole.address() + const port = typeof address === 'object' && address ? address.port : 0 + const endpointDir = mkdtempSync(join(tmpdir(), 'orca-jcode-endpoint-')) + try { + const endpoint = join(endpointDir, 'endpoint.sh') + writeFileSync( + endpoint, + `ORCA_AGENT_HOOK_PORT=${port}\nORCA_AGENT_HOOK_TOKEN=t\nexport ORCA_AGENT_HOOK_PORT ORCA_AGENT_HOOK_TOKEN\n` + ) + + // A tool input far larger than a 64 KB pipe buffer: jcode write_all()s this + // to the gate's stdin and awaits it, so a gate that never reads stdin stalls. + const bigToolInput = JSON.stringify({ content: 'x'.repeat(512 * 1024) }) + const startedAt = Date.now() + execFileSync('/bin/sh', [scriptPath], { + input: bigToolInput, + env: { + ...process.env, + ORCA_AGENT_HOOK_ENDPOINT: endpoint, + ORCA_PANE_KEY: 'tab-1:leaf-1', + JCODE_HOOK_EVENT: 'pre_tool', + JCODE_HOOK_SESSION_ID: 'session_gate_1', + JCODE_HOOK_PAYLOAD: JSON.stringify({ event: 'pre_tool', tool_name: 'write' }) + }, + // Why: the assertion below is the real gate; this only stops a regression + // from hanging the suite instead of failing it. + timeout: 20_000, + // stdio is the point of the test: jcode reads stderr to EOF, so an + // inherited pipe in a backgrounded child would hold the gate open for as + // long as the POST ran, detached or not. + stdio: ['pipe', 'pipe', 'pipe'] + }) + const elapsed = Date.now() - startedAt + + // Comfortably under curl's 1.5s ceiling: a synchronous POST would sit on + // that ceiling for every tool call, and jcode's own budget is only 5s. + expect(elapsed).toBeLessThan(1_000) + } finally { + rmSync(endpointDir, { recursive: true, force: true }) + await new Promise((resolve) => blackHole.close(() => resolve())) + cleanup() + } + }) + + it('drains the gate stdin before exiting on a missing Orca environment', () => { + const { scriptPath, cleanup } = installManagedScript() + try { + const startedAt = Date.now() + execFileSync('/bin/sh', [scriptPath], { + input: JSON.stringify({ content: 'y'.repeat(512 * 1024) }), + // No ORCA_PANE_KEY: the script exits early, but only after taking stdin. + env: { ...process.env, JCODE_HOOK_EVENT: 'pre_tool', ORCA_PANE_KEY: '' }, + timeout: 20_000, + stdio: ['pipe', 'pipe', 'pipe'] + }) + expect(Date.now() - startedAt).toBeLessThan(2_000) + } finally { + cleanup() + } + }) + + it('posts synchronously for observer events, which jcode never waits on', () => { + const { scriptPath, cleanup } = installManagedScript() + try { + 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. + expect(script.trimEnd().endsWith('exit 0')).toBe(true) + expect(script).toContain('\norca_post_jcode_event\n') + } finally { + cleanup() + } + }) +}) + +describe.runIf(process.platform !== 'win32')('managed script shape', () => { + it('writes an executable script jcode can exec directly', () => { + const { scriptPath, cleanup } = installManagedScript() + try { + mkdirSync(dirname(scriptPath), { recursive: true }) + chmodSync(scriptPath, 0o755) + const script = readFileSync(scriptPath, 'utf8') + // Why: jcode parses the command shell-style but executes it directly, so the + // file itself must carry the interpreter. + expect(script.startsWith('#!/bin/sh\n')).toBe(true) + } finally { + cleanup() + } + }) +}) diff --git a/src/main/jcode/hook-service.test.ts b/src/main/jcode/hook-service.test.ts index 1e0d37fd35c..a6f99e910d8 100644 --- a/src/main/jcode/hook-service.test.ts +++ b/src/main/jcode/hook-service.test.ts @@ -13,7 +13,11 @@ vi.mock('os', async () => { }) import { JcodeHookService } from './hook-service' -import { getJcodeConfigPath, getJcodeManagedScriptPath } from './hook-settings' +import { + getJcodeConfigPath, + getJcodeManagedScriptPath, + JCODE_HOOK_EVENTS +} from './hook-settings' import { tomlQuoteString } from './hook-config' describe('JcodeHookService', () => { @@ -50,7 +54,7 @@ describe('JcodeHookService', () => { expect(status.managedHooksPresent).toBe(true) const config = readFileSync(getJcodeConfigPath(), 'utf8') - for (const event of ['turn_end', 'session_start', 'session_end', 'post_tool']) { + for (const event of JCODE_HOOK_EVENTS) { // Why: tomlQuoteString doubles backslashes, so the serialized value (not // the raw path) is what appears in the config. expect(config).toContain(`${event} = ${tomlQuoteString(getJcodeManagedScriptPath())}`) diff --git a/src/main/jcode/hook-service.ts b/src/main/jcode/hook-service.ts index e9694afe1c3..c33f4fd249c 100644 --- a/src/main/jcode/hook-service.ts +++ b/src/main/jcode/hook-service.ts @@ -6,6 +6,7 @@ import { buildWindowsAgentHookPostCommand, writeManagedScript } from '../agent-hooks/installer-utils' +import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' import { readTextFileRemote, writeManagedScriptRemote, @@ -13,7 +14,9 @@ import { } from '../agent-hooks/installer-utils-remote' import { buildWindowsHookEnvironmentGuardLines, - buildWindowsHookStdinDrainEpilogue + buildWindowsHookStdinDrainEpilogue, + POSIX_HOOK_STDIN_DRAIN_COMMAND, + WINDOWS_HOOK_STDIN_DRAIN_COMMAND } from '../agent-hooks/hook-stdin-contract' import { applyJcodeManagedHooks, @@ -37,6 +40,10 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // Why: endpoint file holds the live port/token; a PTY that outlives an Orca restart carries stale env, so `call` it to refresh (else PTY env). 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', ...buildWindowsHookEnvironmentGuardLines(), + // Why: pre_tool is jcode's gate — it writes the tool input to our stdin and + // waits for us. Drain it first so a tool input larger than the pipe buffer + // can never stall the agent mid-write. + `if "%JCODE_HOOK_EVENT%"=="pre_tool" ${WINDOWS_HOOK_STDIN_DRAIN_COMMAND}`, buildWindowsAgentHookPostCommand('jcode'), 'exit /b 0', ...buildWindowsHookStdinDrainEpilogue(), @@ -51,6 +58,12 @@ 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. + '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', @@ -58,20 +71,30 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // 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. - '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', + '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', + '}', + // 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). + 'if [ "$JCODE_HOOK_EVENT" = pre_tool ]; then', + ' orca_post_jcode_event >/dev/null 2>&1 &', + ' exit 0', + 'fi', + 'orca_post_jcode_event', 'exit 0', '' ].join('\n') @@ -154,6 +177,12 @@ export class JcodeHookService { return this.getStatus() } + // Why: jcode invokes the script path recorded in its own config.toml, so an Orca + // upgrade that changes the script body must rewrite the file the user already has. + async refreshManagedScripts(): Promise { + await refreshManagedScriptIfPresent(getJcodeManagedScriptPath(), getManagedScript()) + } + async installRemote(sftp: SFTPWrapper, remoteHome: string): Promise { // Why: remote-Windows is out of scope for v1 (same as Devin); assume POSIX. const remoteConfigPath = getJcodeRemoteConfigPath(remoteHome) diff --git a/src/main/jcode/hook-settings.ts b/src/main/jcode/hook-settings.ts index 30aea26cf7f..e6dcc70bbaf 100644 --- a/src/main/jcode/hook-settings.ts +++ b/src/main/jcode/hook-settings.ts @@ -6,11 +6,27 @@ import { join } from 'node:path' const JCODE_SCRIPT_BASE = 'jcode-hook' -// Why: only observer hooks. pre_tool is a synchronous gate that would add -// startup latency to every tool call without gating anything for Orca. -export const JCODE_HOOK_EVENTS = ['turn_end', 'session_start', 'session_end', 'post_tool'] as const +// The lifecycle points Orca subscribes to, in the order jcode fires them. +// `pre_tool` is jcode's synchronous gate, but the managed script backgrounds its +// POST and exits 0 immediately, so Orca observes the tool without ever holding +// up a tool call. Without it a long `bash` would show no tool at all until it +// finished, and `request_permission` (the only jcode tool a human answers) +// would only be seen after the answer. +export const JCODE_HOOK_EVENTS = [ + 'session_start', + 'turn_start', + 'pre_tool', + 'post_tool', + 'turn_end', + 'session_end' +] as const export type JcodeHookEvent = (typeof JCODE_HOOK_EVENTS)[number] +/** jcode waits for this one; the managed script must never block on it. */ +export function isJcodeGateHookEvent(event: JcodeHookEvent): boolean { + return event === 'pre_tool' +} + export function getJcodeConfigPath(env: NodeJS.ProcessEnv = process.env): string { const explicit = env.JCODE_HOME?.trim() return explicit ? join(explicit, 'config.toml') : join(homedir(), '.jcode', 'config.toml') diff --git a/src/shared/agent-hook-listener-jcode.test.ts b/src/shared/agent-hook-listener-jcode.test.ts index e861c0aca5e..b2aa8abaa9e 100644 --- a/src/shared/agent-hook-listener-jcode.test.ts +++ b/src/shared/agent-hook-listener-jcode.test.ts @@ -6,7 +6,19 @@ import { import { normalizeHookPayload } from './agent-hook-listener' import { PANE_KEY } from './agent-hook-listener-test-harness' -describe('shared agent-hook-listener', () => { +// Payloads below are jcode 0.87.1's own `JCODE_HOOK_PAYLOAD` objects, captured by +// pointing every `[hooks]` entry at a logging script; see +// docs/reference/jcode-hook-events.md. +function ingest(state: HookListenerState, payload: Record) { + return normalizeHookPayload( + state, + 'jcode', + { paneKey: PANE_KEY, payload: { hook_event_name: payload.event, ...payload } }, + 'production' + ) +} + +describe('shared agent-hook-listener: jcode', () => { let state: HookListenerState beforeEach(() => { @@ -17,66 +29,114 @@ describe('shared agent-hook-listener', () => { vi.unstubAllEnvs() }) - it('maps jcode post_tool to working with the tool name', () => { - const event = normalizeHookPayload( - state, - 'jcode', - { - paneKey: PANE_KEY, - payload: { - hook_event_name: 'post_tool', - event: 'post_tool', - session_id: 'session_jc_1', - tool_name: 'read_file' - } - }, - 'production' - ) + it('maps turn_start to working before any tool has run', () => { + const event = ingest(state, { + event: 'turn_start', + session_id: 'session_jc_1', + model: 'claude-haiku-4-5', + source: 'chat' + }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'working', - toolName: 'read_file' + model: 'claude-haiku-4-5' + }) + expect(event?.payload?.toolName).toBeUndefined() + }) + + it('reports the live tool from pre_tool, before the tool has finished', () => { + const event = ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_1', + tool_name: 'read', + tool_input: '{"file_path":"sample.txt","intent":"Read sample.txt to get its contents"}' + }) + expect(event?.payload).toMatchObject({ + agentType: 'jcode', + state: 'working', + toolName: 'read', + toolInput: 'sample.txt' }) }) - it('maps jcode user-input tools to waiting', () => { - const event = normalizeHookPayload( - state, - 'jcode', - { - paneKey: PANE_KEY, - payload: { - hook_event_name: 'post_tool', - event: 'post_tool', - session_id: 'session_jc_2', - tool_name: 'ask_user' - } - }, - 'production' - ) + it('falls back to the tool intent when no tool-specific key matches', () => { + const event = ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_1', + tool_name: 'swarm', + tool_input: '{"action":"status","intent":"Check on the workers"}' + }) + expect(event?.payload).toMatchObject({ toolName: 'swarm', toolInput: 'Check on the workers' }) + }) + + it('keeps the pre_tool input visible when post_tool reports completion', () => { + ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_1', + tool_name: 'bash', + tool_input: '{"command":"pnpm test","intent":"Run the suite"}' + }) + const event = ingest(state, { + event: 'post_tool', + session_id: 'session_jc_1', + tool_name: 'bash', + status: 'ok', + duration_ms: '9', + output_bytes: '137' + }) + expect(event?.payload).toMatchObject({ + state: 'working', + toolName: 'bash', + toolInput: 'pnpm test' + }) + }) + + it('maps a pending request_permission to waiting with the full question', () => { + const event = ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_2', + tool_name: 'request_permission', + tool_input: '{"action":"delete the staging bucket","reason":"Why this needs approval"}' + }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'waiting', - toolName: 'ask_user' + toolName: 'request_permission' + }) + expect(JSON.parse(event?.payload?.interactivePrompt ?? '{}')).toMatchObject({ + action: 'delete the staging bucket' }) }) - it('maps jcode turn_end to done with the last assistant message', () => { - const event = normalizeHookPayload( - state, - 'jcode', - { - paneKey: PANE_KEY, - payload: { - hook_event_name: 'turn_end', - event: 'turn_end', - session_id: 'session_jc_3', - status: 'ok', - last_assistant_message: 'Done.' - } - }, - 'production' - ) + it('does not re-open a question on post_tool, which fires after the answer', () => { + const event = ingest(state, { + event: 'post_tool', + session_id: 'session_jc_2', + tool_name: 'request_permission', + status: 'ok' + }) + expect(event?.payload).toMatchObject({ state: 'working' }) + }) + + it('leaves unrelated tool names working even when they read like a question', () => { + const event = ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_2', + tool_name: 'conversation_search', + tool_input: '{"query":"what did we confirm about the ask flow"}' + }) + expect(event?.payload).toMatchObject({ state: 'working', toolName: 'conversation_search' }) + }) + + it('maps turn_end to done with the last assistant text', () => { + const event = ingest(state, { + event: 'turn_end', + session_id: 'session_jc_3', + status: 'ok', + duration_ms: '6868', + model: 'claude-haiku-4-5', + last_assistant_text: 'Done.' + }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'done', @@ -84,42 +144,56 @@ describe('shared agent-hook-listener', () => { }) }) - it('treats jcode session_start as identity-only (no status row)', () => { - const event = normalizeHookPayload( - state, - 'jcode', - { - paneKey: PANE_KEY, - payload: { - hook_event_name: 'session_start', - event: 'session_start', - session_id: 'session_jc_4', - source: 'create' - } - }, - 'production' - ) + it('surfaces the turn error instead of a stale reply when a turn fails', () => { + const event = ingest(state, { + event: 'turn_end', + session_id: 'session_jc_3', + status: 'error', + model: 'claude-haiku-4-5', + error: 'Anthropic API error (503 Service Unavailable)' + }) + expect(event?.payload).toMatchObject({ + state: 'done', + lastAssistantMessage: 'Anthropic API error (503 Service Unavailable)' + }) + }) + + it('clears the previous turn tool when a new turn starts', () => { + ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_4', + tool_name: 'bash', + tool_input: '{"command":"pnpm lint"}' + }) + const event = ingest(state, { + event: 'turn_start', + session_id: 'session_jc_4', + model: 'claude-haiku-4-5', + source: 'chat' + }) + expect(event?.payload?.toolName).toBeUndefined() + expect(event?.payload?.toolInput).toBeUndefined() + }) + + it('treats session_start as identity-only (no status row)', () => { + const event = ingest(state, { + event: 'session_start', + session_id: 'session_jc_5', + model: 'claude-haiku-4-5', + source: 'create' + }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'done' }) - expect(event?.providerSession).toEqual({ key: 'session_id', id: 'session_jc_4' }) + expect(event?.providerSession).toEqual({ key: 'session_id', id: 'session_jc_5' }) }) it('does not count a direct jcode prompt without journal evidence as explicit', () => { - // Why: regression — a post_tool/turn_end prompt without a usable session id - // has no journal backing, so it must not set hasExplicitPrompt. - const event = normalizeHookPayload( - state, - 'jcode', - { - paneKey: PANE_KEY, - payload: { - hook_event_name: 'post_tool', - event: 'post_tool', - tool_name: 'read_file', - prompt: 'fix the bug' - } - }, - 'production' - ) + // Why: regression — a prompt field on a hook event has no journal backing, so + // it must not set hasExplicitPrompt. + const event = ingest(state, { + event: 'post_tool', + tool_name: 'read', + prompt: 'fix the bug' + }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'working' }) expect(event?.hasExplicitPrompt).toBeFalsy() }) diff --git a/src/shared/agent-hook-listener/provider-event-routing.ts b/src/shared/agent-hook-listener/provider-event-routing.ts index 87c4a55a7c2..71de20b1928 100644 --- a/src/shared/agent-hook-listener/provider-event-routing.ts +++ b/src/shared/agent-hook-listener/provider-event-routing.ts @@ -83,8 +83,10 @@ export function isNewTurnEvent(source: AgentHookSource, eventName: unknown): boo // Why: SessionStart is handled by an early return in normalizeDevinEvent, so UserPromptSubmit is Devin's real new-turn boundary here. return eventName === 'UserPromptSubmit' case 'jcode': - // Why: jcode hooks carry no UserPromptSubmit; a fresh/attached session_start is the only durable turn boundary. - return eventName === 'session_start' + // Why: jcode has no UserPromptSubmit, but turn_start fires once per submitted + // prompt before the model generates — its real turn boundary. session_start + // returns early in normalizeJcodeEvent and clears the cache itself. + return eventName === 'turn_start' } } @@ -113,7 +115,10 @@ export function hasExplicitUserPrompt( } if ( source === 'jcode' && - (eventName === 'post_tool' || eventName === 'turn_end') && + (eventName === 'turn_start' || + eventName === 'pre_tool' || + eventName === 'post_tool' || + eventName === 'turn_end') && hasTranscriptPromptEvidence && resolvedPromptText.trim().length > 0 ) { diff --git a/src/shared/agent-hook-listener/providers/jcode-events.ts b/src/shared/agent-hook-listener/providers/jcode-events.ts index ed0f650ebad..89dc65143a1 100644 --- a/src/shared/agent-hook-listener/providers/jcode-events.ts +++ b/src/shared/agent-hook-listener/providers/jcode-events.ts @@ -6,18 +6,11 @@ import { clearPaneTurnCacheState, type HookListenerState } from '../listener-sta import { resolvePrompt, resolveToolState } from '../prompt-fields' import { extractToolFields, isNewTurnEvent } from '../provider-event-routing' import { readString } from '../tool-input-preview' +import { isJcodeUserInputTool } from './jcode-tool-fields' -// Why: jcode's permission/ask surface is tool-driven; token-normalize the name -// (like Kimi's AskUserQuestion check) so renamed or spaced variants still count. -export function isJcodeUserInputTool(toolName: string | undefined): boolean { - const normalized = toolName?.replaceAll(/[^a-z0-9]/gi, '').toLowerCase() ?? '' - return ( - normalized.includes('ask') || - normalized.includes('question') || - normalized.includes('permission') || - normalized.includes('approval') || - normalized.includes('confirm') - ) +/** 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 } export function normalizeJcodeEvent( @@ -35,14 +28,16 @@ export function normalizeJcodeEvent( } const toolName = readString(hookPayload, 'tool_name') - const stateName = - eventName === 'post_tool' && isJcodeUserInputTool(toolName) - ? 'waiting' - : eventName === 'post_tool' - ? 'working' - : eventName === 'turn_end' || eventName === 'session_end' - ? 'done' - : null + // 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 if (!stateName) { return null @@ -61,8 +56,12 @@ export function normalizeJcodeEvent( resetOnNewTurn: isNewTurnEvent('jcode', eventName) }), 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. + model: readString(hookPayload, 'model'), toolName: snapshot.toolName, toolInput: snapshot.toolInput, - lastAssistantMessage: snapshot.lastAssistantMessage + interactivePrompt: snapshot.interactivePrompt, + lastAssistantMessage: readJcodeErrorText(hookPayload) ?? 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 fb8cf1f5308..9220a9741b7 100644 --- a/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts +++ b/src/shared/agent-hook-listener/providers/jcode-tool-fields.ts @@ -1,27 +1,92 @@ import type { ToolSnapshot } from '../listener-event' -import { hasOwnField, readString, toolUpdate } from '../tool-input-preview' +import { + deriveFallbackToolInputPreview, + deriveToolInputPreview, + hasOwnField, + readString, + 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']) + +export function isJcodeUserInputTool(toolName: string | undefined): boolean { + return toolName !== undefined && JCODE_USER_INPUT_TOOLS.has(toolName) +} + +/** jcode's `tool_input` field is the tool's argument JSON as a string. */ +function parseJcodeToolInput(hookPayload: Record): unknown { + const raw = readString(hookPayload, 'tool_input') + if (raw === undefined) { + return undefined + } + try { + return JSON.parse(raw) + } catch { + return raw + } +} + +// Why: every jcode tool schema carries an `intent` string the model fills in with +// what it is doing, which reads better than a bare path when the tool-specific +// key (file_path, command, …) is missing. +function readJcodeIntent(toolInput: unknown): string | undefined { + if (typeof toolInput !== 'object' || toolInput === null) { + return undefined + } + const intent = (toolInput as Record).intent + return typeof intent === 'string' && intent.trim().length > 0 ? intent : undefined +} -// Why: jcode's post_tool hook reports the tool name but no tool input (only -// the pre_tool gate hook receives the input JSON, and Orca does not gate). export function extractJcodeToolFields( eventName: unknown, hookPayload: Record ): ToolSnapshot { + if (eventName === 'pre_tool') { + const toolName = readString(hookPayload, 'tool_name') + const toolInput = parseJcodeToolInput(hookPayload) + const preview = + deriveToolInputPreview(toolName, toolInput) ?? + readJcodeIntent(toolInput) ?? + deriveFallbackToolInputPreview(toolInput) + return toolUpdate( + { + toolName, + toolInput: preview, + // Why: the question card renders the untruncated tool input; only the + // ask-the-user tool gets one, and resolveToolState never inherits it, so + // a resolved question cannot linger on the row. + interactivePrompt: + isJcodeUserInputTool(toolName) && toolInput !== undefined + ? JSON.stringify(toolInput) + : undefined + }, + { hasToolInputField: hasOwnField(hookPayload, 'tool_input') } + ) + } if (eventName === 'post_tool') { const toolName = readString(hookPayload, 'tool_name') - return toolUpdate( - { toolName, toolInput: undefined }, - // Why: hasToolInputField with an undefined input clears any stale tool - // input from the previous turn instead of inheriting it. - { hasToolInputField: hasOwnField(hookPayload, 'tool_name') } - ) + // Why: post_tool reports no input. Keeping `hasToolInputField` false lets the + // matching pre_tool preview survive the tool's completion instead of blanking. + return toolUpdate({ toolName, toolInput: undefined }, { hasToolInputField: false }) + } + if (eventName === 'turn_start') { + // Why: a new turn starts with no tool; clearing both fields stops the previous + // turn's last tool from being shown as this turn's live work. + return toolUpdate({ toolName: undefined, toolInput: undefined }, { hasToolInputField: true }) } if (eventName === 'turn_end') { const message = - readString(hookPayload, 'last_assistant_message') ?? - readString(hookPayload, 'last_assistant_text') - if (message) { - return { lastAssistantMessage: message } + readString(hookPayload, 'last_assistant_text') ?? + readString(hookPayload, 'last_assistant_message') + return { + ...(message ? { lastAssistantMessage: message } : { clearLastAssistantMessage: true }), + ...toolUpdate({ toolName: undefined, toolInput: undefined }, { hasToolInputField: true }) } } return {} diff --git a/src/shared/commit-message-agent-spec.test.ts b/src/shared/commit-message-agent-spec.test.ts index 4edbc9f1d9b..3199195a1e6 100644 --- a/src/shared/commit-message-agent-spec.test.ts +++ b/src/shared/commit-message-agent-spec.test.ts @@ -258,15 +258,15 @@ describe('buildArgs (Jcode)', () => { const spec = getCommitMessageAgentSpec('jcode')! it('builds a jcode run argv with the model and prompt', () => { - const args = spec.buildArgs({ prompt: 'name this branch', model: 'claude-sonnet-4' }) + const args = spec.buildArgs({ prompt: 'name this branch', model: 'claude-haiku-4-5' }) expect(args).toEqual([ '--no-update', '--quiet', '--no-selfdev', + '--model', + 'claude-haiku-4-5', 'run', '--json', - '--model', - 'claude-sonnet-4', 'name this branch' ]) }) @@ -283,6 +283,31 @@ describe('buildArgs (Jcode)', () => { ]) }) + it('keeps every jcode flag ahead of the subcommand', () => { + // Why: --no-update/--quiet/--no-selfdev are jcode global options; clap only + // accepts them before `run`, and --model rides the same position so the argv + // has one shape rather than two. + const args = spec.buildArgs({ prompt: 'name this branch', model: 'claude-haiku-4-5' }) + const runIndex = args.indexOf('run') + expect(runIndex).toBeGreaterThan(0) + expect(args.slice(0, runIndex).every((arg) => arg.startsWith('--') || arg !== 'run')).toBe(true) + expect(args.slice(runIndex)).toEqual(['run', '--json', 'name this branch']) + }) + + it('discovers models from `jcode model list`', () => { + expect(spec.modelSource).toBe('dynamic') + expect(spec.modelDiscovery?.binary).toBe('jcode') + expect(spec.modelDiscovery?.args).toEqual(['--no-update', '--quiet', 'model', 'list']) + // Real `jcode model list` output: one bare id per line. + expect( + spec.modelDiscovery?.parse('claude-opus-5-5\nclaude-haiku-4-5\ngemini-2.5-pro\n') + ).toEqual([ + { id: 'claude-opus-5-5', label: 'Claude Opus 5 5' }, + { id: 'claude-haiku-4-5', label: 'Claude Haiku 4 5' }, + { id: 'gemini-2.5-pro', label: 'Gemini 2.5 Pro' } + ]) + }) + it('defaults the model to the jcode config default', () => { expect(spec.defaultModelId).toBe('default') }) diff --git a/src/shared/commit-message-agent-spec.ts b/src/shared/commit-message-agent-spec.ts index 6927f0a9ca7..2d788739149 100644 --- a/src/shared/commit-message-agent-spec.ts +++ b/src/shared/commit-message-agent-spec.ts @@ -125,7 +125,8 @@ export const COMMIT_MESSAGE_AGENT_SPECS: Partial CommitMessageModel[] parseAntigravityModels: (stdout: string) => CommitMessageModel[] + parseLineModels: (stdout: string) => CommitMessageModel[] } export function buildSecondaryCommitMessageAgentSpecs({ BASIC_THINKING_LEVELS, OPENAI_THINKING_LEVELS, parseCursorModels, - parseAntigravityModels + parseAntigravityModels, + parseLineModels }: SecondaryAgentSpecDeps): Partial> { return { amp: { @@ -280,30 +282,35 @@ export function buildSecondaryCommitMessageAgentSpecs({ id: 'jcode', label: 'Jcode', binary: 'jcode', - // Why: jcode run takes the message as a positional argv argument and has - // no stdin prompt mode; Source Control AI prompts ride argv (fine for - // branch naming and small diffs, argv-capped on Windows). + // Why: `jcode run` takes the message as a positional argv argument and has no + // stdin prompt mode, so Source Control AI prompts ride argv (fine for branch + // naming and small diffs, argv-capped on Windows). promptDelivery: 'argv', buildArgs: ({ prompt, model }) => [ - // Why: the documented wrapper pattern is `jcode --quiet --no-update - // --no-selfdev run "…"` — global flags before the subcommand. + // Why: these are jcode global options, so they must precede the subcommand; + // clap rejects them after `run`. '--no-update', '--quiet', '--no-selfdev', + ...(model && model !== 'default' ? ['--model', model] : []), 'run', '--json', - // Why: bare `default` (or an empty model) lets jcode use the model - // configured in its own config.toml, so no provider is hardcoded here. - ...(model && model !== 'default' ? ['--model', model] : []), prompt ], - modelSource: 'static', - models: [ - { id: 'default', label: 'Config default' }, - { id: 'claude-sonnet-4', label: 'Claude Sonnet 4' }, - { id: 'claude-opus-4-6', label: 'Claude Opus 4.6' }, - { id: 'gpt-5.4', label: 'GPT-5.4' } - ], + singletonOptions: [['--model']], + modelSource: 'dynamic', + // Why: `jcode model list` prints one bare model id per line, which is exactly + // what parseLineModels reads. Discovering beats a hardcoded list because + // jcode's catalog spans every provider the user has authenticated. + modelDiscovery: { + binary: 'jcode', + args: ['--no-update', '--quiet', 'model', 'list'], + parse: parseLineModels + }, + // Why: `default` is not a jcode model id — it is the sentinel that omits + // --model so jcode uses the model from its own config.toml, rather than Orca + // pinning a provider the user may not be logged in to. + models: [{ id: 'default', label: 'Config default' }], defaultModelId: 'default' } }