Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions packages/acp-bridge/src/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
RestoreInProgressError,
SessionShellClientRequiredError,
SessionShellDisabledError,
SessionBusyError,
SessionNotFoundError,
WorkspaceMismatchError,
} from './bridgeErrors.js';
Expand Down Expand Up @@ -2946,6 +2947,70 @@ describe('createAcpSessionBridge', () => {
await bridge.shutdown();
});

it('publishes session_branched only on the new session stream', async () => {
const factory: ChannelFactory = async () =>
makeChannel({
extMethodImpl: async (method) => {
if (method !== 'qwen/control/session/branch') return {};
return { newSessionId: 'branch-1', title: 'Branch 1' };
},
resumeSessionImpl: () => ({}),
}).channel;
const bridge = makeBridge({ channelFactory: factory });
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });
const sourceAbort = new AbortController();
const sourceIter = bridge
.subscribeEvents(session.sessionId, { signal: sourceAbort.signal })
[Symbol.asyncIterator]();

const branch = await bridge.branchSession(session.sessionId, {
name: 'Branch 1',
});

const sourceEvent = await Promise.race([
sourceIter.next(),
new Promise<'timeout'>((resolve) => setTimeout(resolve, 25, 'timeout')),
]);
expect(sourceEvent).toBe('timeout');
sourceAbort.abort();

const sourceReplayAbort = new AbortController();
const sourceReplayIter = bridge
.subscribeEvents(session.sessionId, {
lastEventId: 0,
signal: sourceReplayAbort.signal,
})
[Symbol.asyncIterator]();
const sourceReplayEvent = await Promise.race([
sourceReplayIter.next(),
new Promise<'timeout'>((resolve) => setTimeout(resolve, 25, 'timeout')),
]);
expect(sourceReplayEvent).toMatchObject({
value: { type: 'replay_complete' },
});
const sourceReplayNext = await Promise.race([
sourceReplayIter.next(),
new Promise<'timeout'>((resolve) => setTimeout(resolve, 25, 'timeout')),
]);
expect(sourceReplayNext).toBe('timeout');
sourceReplayAbort.abort();

const branchedIter = bridge
.subscribeEvents(branch.sessionId, { lastEventId: 0 })
[Symbol.asyncIterator]();
const replayed = await branchedIter.next();
expect(replayed.value).toMatchObject({
type: 'session_branched',
data: {
sourceSessionId: session.sessionId,
newSessionId: branch.sessionId,
displayName: 'Branch 1',
},
});

await bridge.shutdown();
});

it('a failed prompt does not poison the queue for subsequent prompts', async () => {
let promptCount = 0;
const handles: ChannelHandle[] = [];
Expand Down Expand Up @@ -2985,6 +3050,37 @@ describe('createAcpSessionBridge', () => {
await bridge.shutdown();
});

it('rejects launchSessionForkAgent while a prompt is active', async () => {
let releasePrompt: (() => void) | undefined;
const handle = makeChannel({
promptImpl: async () =>
new Promise<PromptResponse>((resolve) => {
releasePrompt = () => resolve({ stopReason: 'end_turn' });
}),
});
const bridge = makeBridge({ channelFactory: async () => handle.channel });
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });

const active = bridge.sendPrompt(session.sessionId, {
sessionId: session.sessionId,
prompt: [{ type: 'text', text: 'active' }],
});
await vi.waitFor(() => expect(releasePrompt).toBeDefined());

await expect(
bridge.launchSessionForkAgent(session.sessionId, 'review this'),
).rejects.toBeInstanceOf(SessionBusyError);
expect(
handle.agent.extMethodCalls.some(
(call) => call.method === 'qwen/control/session/fork_agent',
),
).toBe(false);

releasePrompt!();
await expect(active).resolves.toEqual({ stopReason: 'end_turn' });
await bridge.shutdown();
});

it('throws SessionNotFoundError for unknown session ids', async () => {
const bridge = makeBridge({
channelFactory: async () => {
Expand Down
84 changes: 80 additions & 4 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3394,14 +3394,14 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {

let restored;
try {
restored = await restoreSession('resume', {
restored = await restoreSession('load', {
sessionId: result.newSessionId,
workspaceCwd: boundWorkspace,
clientId: context?.clientId,
});
} catch (restoreErr) {
writeStderrLine(
`qwen serve: branchSession resume failed for ${result.newSessionId}, attempting cleanup...`,
`qwen serve: branchSession load failed for ${result.newSessionId}, attempting cleanup...`,
);
try {
await ci.connection.extMethod(
Expand Down Expand Up @@ -3429,8 +3429,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
data: eventData,
...(originatorClientId ? { originatorClientId } : {}),
};
entry.events.publish(branchEnvelope);
broadcastWorkspaceEvent(branchEnvelope, sessionId);
// The branch announcement belongs to the new session only. Publishing
// it on the source session would persist in that session's replay ring.
newEntry?.events.publish(branchEnvelope);

return {
...restored,
Expand Down Expand Up @@ -4350,6 +4351,81 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
};
},

async launchSessionForkAgent(sessionId, directive, context) {
const entry = byId.get(sessionId);
if (!entry) throw new SessionNotFoundError(sessionId);
const info = channelInfoForEntry(entry);
if (!info || info.isDying) throw new SessionNotFoundError(sessionId);
resolveTrustedClientId(entry, context?.clientId);

const trimmed = directive.trim();
if (!trimmed) {
throw new Error('Fork directive is required');
}
if (entry.pendingPromptCount > 0 || entry.promptActive) {
Comment thread
ytahdn marked this conversation as resolved.
throw new SessionBusyError(
sessionId,
'Cannot fork while a response or tool call is in progress',
);
}
return entry.promptQueue.then(async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] launchSessionForkAgent chains onto entry.promptQueue.then(...) but never writes back entry.promptQueue = result.then(...), unlike sendPrompt (line 3065) and branchSession (line 3445) which both do entry.promptQueue = result.then(() => undefined, () => undefined).

This means a sendPrompt arriving while the fork's extMethod is in-flight won't serialize behind it — both can execute concurrently against the same agent channel. Today the fork's server-side handler is a quick fire-and-forget subagent launch so this has no practical impact, but if the fork ever gains state-mutating side effects or a longer-running RPC, the gap widens into a real race.

Suggested change
return entry.promptQueue.then(async () => {
const forkResult = entry.promptQueue.then(async () => {

…and after the closing }); of the .then(), add:

      entry.promptQueue = forkResult.then(() => undefined, () => undefined);
      return forkResult;

— qwen3.7-max via Qwen Code /review

if (entry.pendingPromptCount > 0 || entry.promptActive) {
throw new SessionBusyError(
sessionId,
'Cannot fork while a response or tool call is in progress',
);
}

opts.onDiagnosticLine?.(
`qwen serve: launchSessionForkAgent requested for session=${sessionId}`,
'info',
);

let response: {
description?: string;
launched?: boolean;
};
try {
response = (await Promise.race([
withTimeout(
entry.connection.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionForkAgent,
{
sessionId,
directive: trimmed,
},
),
initTimeoutMs,
SERVE_CONTROL_EXT_METHODS.sessionForkAgent,
),
getTransportClosedReject(entry),
])) as {
description?: string;
launched?: boolean;
};
} catch (error) {
opts.onDiagnosticLine?.(
`qwen serve: launchSessionForkAgent failed for session=${sessionId}: ${
error instanceof Error ? error.message : String(error)
}`,
'warn',
);
throw error;
}

const result = {
sessionId: entry.sessionId,
description: response.description ?? trimmed.slice(0, 60),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The fallback description trimmed.slice(0, 60) doesn't collapse internal whitespace or append an ellipsis, unlike the agent-side collapseForkDirective(directive, 60) which does both. If this fallback ever triggers, the web-shell toast would show uncollapsed whitespace (e.g., "review \t this\nbranch") with a hard cutoff instead of "review this branch…".

Consider importing or replicating the collapse helper:

Suggested change
description: response.description ?? trimmed.slice(0, 60),
description: response.description ?? collapseForkDirective(trimmed, 60),

— qwen3.7-max via Qwen Code /review

launched: response.launched === true,
};
opts.onDiagnosticLine?.(
`qwen serve: launchSessionForkAgent completed for session=${sessionId} launched=${result.launched}`,
'info',
);
return result;
});
},

async executeShellCommand(
sessionId,
command,
Expand Down
17 changes: 17 additions & 0 deletions packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ export interface BridgeBranchedSession extends BridgeRestoredSession {
forkedFrom: { sessionId: string; displayName: string };
}

export interface BridgeForkAgentResult {
sessionId: string;
description: string;
launched: boolean;
}

/** Sparse summary used by `GET /workspace/:id/sessions`. */
export interface BridgeSessionSummary {
sessionId: string;
Expand Down Expand Up @@ -572,6 +578,17 @@ export interface AcpSessionBridge {
context?: BridgeClientRequestContext,
): Promise<{ sessionId: string; answer: string | null }>;

/**
* Launch a background fork agent that inherits the live session's current
* conversation context. This is CLI `/fork`, not ACP `session/fork`
* (which maps to `/branch`).
*/
launchSessionForkAgent(
sessionId: string,
directive: string,
context?: BridgeClientRequestContext,
): Promise<BridgeForkAgentResult>;

/**
* Queue a mid-turn user message for the running turn. The ACP child drains
* it between tool batches via the `craft/drainMidTurnQueue` ext-method so
Expand Down
1 change: 1 addition & 0 deletions packages/acp-bridge/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export const SERVE_CONTROL_EXT_METHODS = {
sessionClose: 'qwen/control/session/close',
sessionApprovalMode: 'qwen/control/session/approval_mode',
sessionBranch: 'qwen/control/session/branch',
sessionForkAgent: 'qwen/control/session/fork_agent',
sessionRecap: 'qwen/control/session/recap',
sessionBtw: 'qwen/control/session/btw',
sessionShellHistory: 'qwen/control/session/shell_history',
Expand Down
81 changes: 81 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ vi.mock('@qwen-code/qwen-code-core', () => ({
USE_GEMINI: 'gemini',
USE_VERTEX_AI: 'vertex-ai',
},
ToolNames: {
AGENT: 'agent',
},
FORK_SUBAGENT_TYPE: 'fork',
ALL_PROVIDERS: [
{
id: 'deepseek',
Expand Down Expand Up @@ -2103,6 +2107,83 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
await agentPromise;
});

it('launches fork agents with neutral history text', async () => {
const sessionId = '11111111-1111-1111-1111-111111111111';
const innerConfig = await setupSessionMocks(sessionId);
const addHistory = vi.fn();
const execute = vi.fn().mockResolvedValue({ llmContent: 'ok' });
const build = vi.fn().mockReturnValue({ execute });
const directive = `review this\nbranch ${'x'.repeat(220)}`;
const collapsed = `review this branch ${'x'.repeat(220)}`;

Object.assign(innerConfig, {
getGeminiClient: vi.fn().mockReturnValue({
isInitialized: vi.fn().mockReturnValue(true),
initialize: vi.fn().mockResolvedValue(undefined),
waitForMcpReady: vi.fn().mockResolvedValue(undefined),
getHistoryShallow: vi
.fn()
.mockReturnValue([{ role: 'user', parts: [{ text: 'before' }] }]),
addHistory,
}),
getToolRegistry: vi.fn().mockReturnValue({
getTool: vi.fn((name: string) =>
name === 'agent' ? { build } : undefined,
),
}),
});

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());

const agent = capturedAgentFactory!({
get closed() {
return mockConnectionState.promise;
},
}) as AgentLike;

await agent.newSession({ cwd: '/tmp', mcpServers: [] });
await expect(
agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionForkAgent, {
sessionId,
directive,
}),
).resolves.toEqual({
sessionId,
description: `${collapsed.slice(0, 57)}…`,
launched: true,
});

expect(build).toHaveBeenCalledWith({
description: `${collapsed.slice(0, 57)}…`,
prompt: directive.trim(),
subagent_type: 'fork',
run_in_background: true,
});
expect(execute).toHaveBeenCalledTimes(1);
expect(addHistory).toHaveBeenCalledWith({
role: 'user',
parts: [
{
text: `User launched a background fork via /fork. Directive (truncated): ${collapsed.slice(
0,
197,
)}…`,
},
],
});
expect(addHistory.mock.calls[0]?.[0]?.parts[0]?.text).not.toContain(
'[system]',
);

mockConnectionState.resolve();
await agentPromise;
});

it('allows cancelling paused agent tasks', async () => {
const sessionId = '11111111-1111-1111-1111-111111111111';
const innerConfig = await setupSessionMocks(sessionId);
Expand Down
Loading
Loading