Repository navigation
Native chat: every agent takes a message while it starts, and Stop ends the session for every agent #26561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Native chat: every agent takes a message while it starts, and Stop ends the session for every agent #26561
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
1a07fc6
Every agent opens its chat at launch, and one Stop rule (rebuilt on m…
brennanb2025 ad5d2cc
Keep an agent's own sign-in words on a refused start; let a picker re…
brennanb2025 a6c3a0f
Take the model-catalog start hold back out
brennanb2025 58f3e6f
Merge origin/main: a chat's learned default is reported at its proven…
brennanb2025 d7acf9a
Merge remote-tracking branch 'origin/main' into brennanb2025/agent-pu…
brennanb2025 2cade43
Merge remote-tracking branch 'origin/main' into brennanb2025/agent-pu…
brennanb2025 20832cb
Merge remote-tracking branch 'origin/main' into brennanb2025/agent-pu…
brennanb2025 2a7fa4e
fix(native-chat): a Codex or Pi start has only the host's start deadline
brennanb2025 4ad891d
Merge remote-tracking branch 'origin/main' into brennanb2025/agent-pu…
brennanb2025 5c66c69
fix(native-chat): settle a signed-out Pi's first message once; keep h…
brennanb2025 05ccab6
fix(native-chat): keep a chat's picks shown while its next runtime st…
brennanb2025 fc15e0a
Merge remote-tracking branch 'origin/main' into brennanb2025/agent-pu…
brennanb2025 70febbb
Prove Codex workload exit and keep failed-start option reads passive
brennanb2025 e7fb001
Report unobserved Codex workload exit as unverifiable
brennanb2025 18ec576
Keep connection mocks in tests and extract the scripted server fixture
brennanb2025 34e5388
Fix the queued Stop test clock in CI
brennanb2025 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { mkdtemp, rm } from 'node:fs/promises' | ||
| import { tmpdir } from 'node:os' | ||
| import { join } from 'node:path' | ||
| import { afterEach, describe, expect, it, vi } from 'vitest' | ||
| import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' | ||
| import { | ||
| HOST_TEST_SESSION, | ||
| hostTestAttachParams, | ||
| hostTestMessage | ||
| } from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' | ||
| import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' | ||
| import { | ||
| ensureStructuredAgentSessionHost, | ||
| stopStructuredAgentSessionRuntime | ||
| } from '../runtime/structured-agent-session-runtime' | ||
| import { acpScriptedChild } from './acp-scripted-child.test-fixture' | ||
|
|
||
| let stateDirectory: string | undefined | ||
| let operation = 0 | ||
| const operationId = () => `${Date.now()}-${(++operation).toString(16).padStart(32, '0')}` | ||
| const caller = { callerKey: 'scripted-acp-runtime' } | ||
|
|
||
| afterEach(async () => { | ||
| await stopStructuredAgentSessionRuntime() | ||
| if (stateDirectory) { | ||
| await rm(stateDirectory, { recursive: true, force: true }) | ||
| } | ||
| stateDirectory = undefined | ||
| }) | ||
|
|
||
| describe.each(['grok', 'opencode'] as const)('%s runtime with a scripted ACP child', (agent) => { | ||
| it('attaches and accepts a send while startup is held, then delivers once the protocol session opens', async () => { | ||
| stateDirectory = await mkdtemp(join(tmpdir(), 'orca-scripted-acp-')) | ||
| const script = acpScriptedChild(agent)() | ||
| const host = await ensureStructuredAgentSessionHost({ | ||
| logger: createStructuredAgentSessionLogger(), | ||
| stateDirectory, | ||
| hostId: 'local', | ||
| claimKeyId: 'key-1', | ||
| resolveWorkspacePath: async () => stateDirectory ?? '', | ||
| resolveLaunchArgs: () => [], | ||
| resolveClaudeAuthPolicy: () => ({ account: 'managed' }), | ||
| ...script.deps | ||
| }) | ||
| const attach = hostTestAttachParams(null, { | ||
| provider: agent, | ||
| agent, | ||
| providerHandle: undefined, | ||
| location: { | ||
| executionHostId: 'local', | ||
| wslDistro: null, | ||
| workspaceId: 'folder-1', | ||
| workspaceKind: 'folder' | ||
| }, | ||
| accountHome: | ||
| agent === 'grok' | ||
| ? { variable: 'GROK_HOME', path: stateDirectory } | ||
| : { kind: 'opencode', locator: { kind: 'unmanaged' } } | ||
| }) | ||
| attach.envelope.clientOperationId = operationId() | ||
| const attached = await host.attach(caller, attach) | ||
| expect(attached).toMatchObject({ ok: true }) | ||
| if (!attached.ok) { | ||
| throw new Error(JSON.stringify(attached.refusal)) | ||
| } | ||
| const body = hostTestMessage('message during startup') | ||
| const sent = await host.send(caller, { | ||
| body, | ||
| envelope: { | ||
| sessionId: HOST_TEST_SESSION, | ||
| clientOperationId: operationId(), | ||
| expectedRuntimeFence: attached.value.fence, | ||
| payloadFingerprint: computeAgentSessionPayloadFingerprint({ | ||
| method: 'agentSession.send', | ||
| sessionId: HOST_TEST_SESSION, | ||
| fields: { body } | ||
| }) | ||
| } | ||
| }) | ||
| expect(sent).toMatchObject({ ok: true }) | ||
| expect(script.prompts()).toEqual([]) | ||
| await vi.waitFor(() => expect(script.spawns()).toBe(1)) | ||
| script.releaseHandshake() | ||
| await vi.waitFor(() => expect(script.prompts()).toEqual(['message during startup'])) | ||
| await host.close(HOST_TEST_SESSION, 'user-close') | ||
| expect(script.closes()).toBe(1) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| import { z } from 'zod' | ||
| import type { | ||
| ScriptedAgentChild, | ||
| ScriptedAgentChildFactory | ||
| } from '../runtime/structured-agent-scripted-child.test-fixture' | ||
| import { FakeAcpChild, GROK_CONFIG_OPTIONS } from './acp-structured-adapter.test-support' | ||
| import type { FakeFrame } from './acp-scripted-agent.test-support' | ||
| import type { AcpLaunchSpec } from './acp-launch-specs' | ||
|
|
||
| const promptParams = z.object({ | ||
| sessionId: z.string(), | ||
| prompt: z.array(z.object({ type: z.string(), text: z.string().optional() })), | ||
| _meta: z.object({ promptId: z.string() }).optional() | ||
| }) | ||
| const loadParams = z.object({ sessionId: z.string() }) | ||
| const configParams = z.object({ configId: z.string(), value: z.string() }) | ||
|
|
||
| export function acpScriptedChild(agent: AcpLaunchSpec['agent']): ScriptedAgentChildFactory { | ||
| return () => { | ||
| let current: FakeAcpChild | undefined | ||
| let handshake: (() => void) | undefined | ||
| let released = false | ||
| let opened = 0 | ||
| let reopened = 0 | ||
| let closed = 0 | ||
| const texts: string[] = [] | ||
| const fixture: ScriptedAgentChild = { | ||
| holdHandshakes: true, | ||
| completeTurns: true, | ||
| deps: { | ||
| resolveEnvironment: async () => ({ PATH: '' }), | ||
| acpLaunchCommand: { | ||
| resolveCommand: () => 'scripted-acp-agent', | ||
| probeVersion: async () => true | ||
| }, | ||
| readProcessStartTime: async () => 1_700_000_000_000 + opened, | ||
| openAcpConnection: (launch, options) => { | ||
| opened += 1 | ||
| handshake = undefined | ||
| released = false | ||
| const child = new FakeAcpChild(launch, options) | ||
| current = child | ||
| let sessionId = `${agent}-scripted-session` | ||
| let running: FakeFrame | undefined | ||
| let configOptions = GROK_CONFIG_OPTIONS.map((option) => ({ ...option })) | ||
| const script = child.agent | ||
| script.on('initialize', (frame) => | ||
| script.reply(frame, { | ||
| protocolVersion: 1, | ||
| agentCapabilities: { loadSession: true } | ||
| }) | ||
| ) | ||
| const start = (frame: FakeFrame, resumes: boolean): void => { | ||
| if (resumes) { | ||
| reopened += 1 | ||
| sessionId = loadParams.parse(frame.params).sessionId | ||
| } | ||
| const answer = () => { | ||
| if (child.closed || child.exited) { | ||
| return | ||
| } | ||
| script.reply(frame, { ...(!resumes ? { sessionId } : {}), configOptions }) | ||
| } | ||
| if (fixture.holdHandshakes && !released) { | ||
| handshake = answer | ||
| } else { | ||
| answer() | ||
| } | ||
| } | ||
| script.on('session/new', (frame) => start(frame, false)) | ||
| script.on('session/load', (frame) => start(frame, true)) | ||
| script.on('session/set_config_option', (frame) => { | ||
| const { configId, value } = configParams.parse(frame.params) | ||
| configOptions = configOptions.map((option) => | ||
| option.id === configId ? { ...option, currentValue: value } : option | ||
| ) | ||
| script.reply(frame, { configOptions }) | ||
| }) | ||
| script.on('session/set_model', (frame) => script.reply(frame, {})) | ||
| // Grok's subagent-stop probe: the handler refuses the missing id, so stops are offered. | ||
| script.on('_x.ai/subagent/cancel', (frame) => | ||
| script.fail( | ||
| frame, | ||
| -32602, | ||
| 'Invalid params', | ||
| 'invalid params: missing field `subagentId`' | ||
| ) | ||
| ) | ||
| // Its background-task stop probe, answered the same way. | ||
| script.on('_x.ai/task/kill', (frame) => | ||
| script.fail( | ||
| frame, | ||
| -32602, | ||
| 'Invalid params', | ||
| 'invalid params: missing field `sessionId`' | ||
| ) | ||
| ) | ||
| script.on('session/prompt', (frame) => { | ||
| const params = promptParams.parse(frame.params) | ||
| texts.push(params.prompt.map((block) => block.text ?? '').join('')) | ||
| running = frame | ||
| script.notify('session/update', { | ||
| sessionId, | ||
| update: { | ||
| sessionUpdate: 'agent_message_chunk', | ||
| content: { type: 'text', text: 'Scripted response' } | ||
| }, | ||
| ...(params._meta ? { _meta: params._meta } : {}) | ||
| }) | ||
| if (fixture.completeTurns) { | ||
| script.reply(frame, { stopReason: 'end_turn' }) | ||
| running = undefined | ||
| } | ||
| }) | ||
| script.on('session/cancel', () => { | ||
| if (running) { | ||
| script.reply(running, { stopReason: 'cancelled' }) | ||
| } | ||
| running = undefined | ||
| }) | ||
| const prove = child.proveClose | ||
| child.proveClose = async () => { | ||
| if (!child.exited) { | ||
| closed += 1 | ||
| } | ||
| return prove() | ||
| } | ||
| return child | ||
| } | ||
| }, | ||
| releaseHandshake: () => { | ||
| released = true | ||
| handshake?.() | ||
| handshake = undefined | ||
| }, | ||
| failHandshake: (message) => { | ||
| if (!current) { | ||
| throw new Error('no scripted ACP child') | ||
| } | ||
| current.stderr = message | ||
| current.exit() | ||
| handshake = undefined | ||
| }, | ||
| prompts: () => texts, | ||
| spawns: () => opened, | ||
| resumes: () => reopened, | ||
| closes: () => closed | ||
| } | ||
| return fixture | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import type { ProviderProcessExit } from '../provider-process/managed-provider-process' | ||
| import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' | ||
| import type { AcpAgentConnectionOptions } from './acp-agent-connection' | ||
| import { AcpConnectionClosedError } from './acp-errors' | ||
| import { AcpSessionRuntime } from './acp-session-runtime' | ||
| import type { AcpStructuredConnection } from './acp-structured-connection' | ||
| import { AcpScriptedAgent } from './acp-scripted-agent.test-support' | ||
| export const PID = 4242 | ||
|
|
||
| /** A scripted agent behind the connection surface: the real protocol runtime over in-memory stdio, | ||
| * with the process lifecycle `AcpAgentConnection` gives it (stdout EOF is not exit; the exit closes | ||
| * the protocol with the agent's last words; a protocol failure while it runs is `onClose`). */ | ||
| export class FakeAcpChild extends AcpSessionRuntime implements AcpStructuredConnection { | ||
| readonly agent: AcpScriptedAgent | ||
| readonly pid: number | undefined = PID | ||
| readonly spawned = Promise.resolve() | ||
| stderr = '' | ||
| processTreeUnproven = false | ||
| private exitListeners: ((exit: ProviderProcessExit) => void)[] = [] | ||
| private gone = false | ||
| private closing = false | ||
| private lostWith: Error | undefined | ||
| closes = 0 | ||
|
|
||
| constructor( | ||
| readonly launch: ProviderProcessLaunch, | ||
| private readonly connectionOptions: AcpAgentConnectionOptions, | ||
| agent = new AcpScriptedAgent() | ||
| ) { | ||
| const lifecycle: { child: FakeAcpChild | null } = { child: null } | ||
| super(agent.stdout, agent.stdin, { | ||
| ...connectionOptions, | ||
| peer: { ...connectionOptions.peer, closeOnInputEnd: false }, | ||
| onClose: (error) => lifecycle.child?.lost(error) | ||
| }) | ||
| lifecycle.child = this | ||
| this.agent = agent | ||
| } | ||
|
|
||
| get exited() { | ||
| return this.gone | ||
| } | ||
| onExit(listener: (exit: ProviderProcessExit) => void): void { | ||
| if (this.gone) { | ||
| listener(FAKE_EXIT) | ||
| } else { | ||
| this.exitListeners.push(listener) | ||
| } | ||
| } | ||
| stderrTail(): string { | ||
| return this.stderr | ||
| } | ||
| pauseReading(): void { | ||
| this.agent.stdout.pause() | ||
| } | ||
| resumeReading(): void { | ||
| this.agent.stdout.resume() | ||
| } | ||
| /** What a close proves once the protocol is closed; replace it to leave the exit unproven. */ | ||
| proveClose = async (): Promise<boolean> => { | ||
| this.exit() | ||
| return true | ||
| } | ||
| override close(error?: Error): Promise<boolean> { | ||
| this.closes += 1 | ||
| this.closing ||= !this.gone | ||
| this.drainNotifications(error) | ||
| return this.proveClose().finally(() => super.close(error)) | ||
| } | ||
| /** The agent process ends on its own (or Orca's close landed). */ | ||
| exit(): void { | ||
| if (this.gone) { | ||
| return | ||
| } | ||
| this.gone = true | ||
| const error = | ||
| this.lostWith ?? new AcpConnectionClosedError(this.stderr || `${this.launch.command} exited`) | ||
| super.close(error) | ||
| this.connectionOptions.onExit?.(error, { expected: this.closing, exit: FAKE_EXIT }) | ||
| for (const listener of this.exitListeners.splice(0)) { | ||
| listener(FAKE_EXIT) | ||
| } | ||
| } | ||
| private lost(error: Error): void { | ||
| this.lostWith ??= error | ||
| if (this.closing || this.gone) { | ||
| return | ||
| } | ||
| this.connectionOptions.onClose?.(error) | ||
| } | ||
| } | ||
|
|
||
| const FAKE_EXIT: ProviderProcessExit = { code: 0, signal: null, processless: false } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: stablyai/orca
Length of output: 3515
🏁 Script executed:
Repository: stablyai/orca
Length of output: 42058
🏁 Script executed:
Repository: stablyai/orca
Length of output: 5345
🏁 Script executed:
Repository: stablyai/orca
Length of output: 13392
Add the new ACP tests to the SQLite runtime include.
The startup-attempt registrations test was deleted, so it does not need to be restored. However, the three new ACP tests are absent from the SQLite runtime lists. Their rigs use real record-store or journal fixtures. Without these entries, the tests can run under the wrong Vitest runtime.
🐛 Suggested fix