mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
feat(acp): support /cd command in ACP sessions (#5903)
* feat(acp): support /cd command in ACP sessions Implements the server-side /cd command for ACP multi-session architecture: - Infrastructure: ext-method constant, bridge types, CdWhilePromptActiveError - Core: skipProcessChdir option in Config.relocateWorkingDirectory() - ACP child handler: trust check, fs validation, sandbox guard - Bridge: promptQueue-serialized changeSessionCwd with event publishing - HTTP route: POST /session/:id/cd with error-to-status mapping - directory_not_found → 400 - directory_not_trusted → 403 - restrictive_sandbox → 403 - session_busy (CdWhilePromptActiveError) → 409 SDK client wrappers and capability registration are deferred to a follow-up PR once the server-side implementation is validated end-to-end. Closes #5677 * qwen: address PR review feedback (#5903) - [Critical] Thread skipProcessChdir through prepareSessionArtifactMigration to prevent corrupting shared daemon process.cwd() on rollback in ACP mode - [Suggestion] Raise cd timeout to Math.max(initTimeoutMs, 30_000) for large-project fs ops and memory refresh - [Suggestion] Move errorKind-to-HTTP mapping (restrictive_sandbox, directory_not_found, directory_not_trusted) into sendBridgeErrorImpl; cd route catch block now matches /branch and /fork pattern * fix(acp): address review feedback on /cd implementation (#5903) Critical fixes: - Do not overwrite entry.workspaceCwd — session stays listed under the daemon's bound workspace; logical cwd lives in agent Config only - Queue tail tied to raw extMethod settlement, not timeout — subsequent prompts cannot run against unknown cwd state after caller timeout - Add realpath equality check for skipProcessChdir path to guard against TOCTOU symlink swaps between trust check and state update - Skip artifact migration for ACP /cd (skipArtifactMigration option) — storage stays at bound workspace so branch/load/lifecycle remain consistent Suggestions addressed: - Add response shape validation for extMethod result - Rename CdWhilePromptActiveError HTTP code from 'session_busy' to 'cd_while_prompt_active' (avoids collision with SessionBusyError) - Add null-byte path validation in ACP sessionCd handler - Add operator logging on successful cwd change (warnings included) * chore: revert unrelated cua-driver formatting, add to .prettierignore Reverts 23 cua-driver files that picked up Prettier formatting noise during merge conflict resolution. Adds packages/cua-driver/ to .prettierignore since it's a vendored package whose content should not be auto-formatted by local hooks. --------- Co-authored-by: qwen-code-dev-bot <[email protected]>
This commit is contained in:
1 parent
8b5cb01a33
commit
fa8b2ad87a
10 files changed
+351
-20
No files matched your search
@@ -22,4 +22,5 @@ junit.xml
|
||||
Thumbs.db
|
||||
packages/vscode-ide-companion/schemas/settings.schema.json
|
||||
packages/cli/src/services/insight/templates/insightTemplate.ts
|
||||
packages/cua-driver/
|
||||
packages/desktop/
|
||||
@@ -48,6 +48,7 @@ import {
|
||||
} from './status.js';
|
||||
import {
|
||||
BranchWhilePromptActiveError,
|
||||
CdWhilePromptActiveError,
|
||||
SessionNotFoundError,
|
||||
RestoreInProgressError,
|
||||
InvalidSessionScopeError,
|
||||
@@ -81,6 +82,8 @@ import type {
|
||||
AcpSessionBridge,
|
||||
MidTurnQueueEntry,
|
||||
BridgeDaemonStatusSnapshot,
|
||||
ChangeSessionCwdRequest,
|
||||
ChangeSessionCwdResult,
|
||||
} from './bridgeTypes.js';
|
||||
import type { BridgeOptions, BridgeTelemetry } from './bridgeOptions.js';
|
||||
import { MCP_RESTART_SERVER_DEADLINE_MS } from './mcpTimeouts.js';
|
||||
@@ -3474,6 +3477,95 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
|
||||
return branchResult;
|
||||
},
|
||||
|
||||
async changeSessionCwd(
|
||||
sessionId: string,
|
||||
req: ChangeSessionCwdRequest,
|
||||
context?: BridgeClientRequestContext,
|
||||
): Promise<ChangeSessionCwdResult> {
|
||||
if (shuttingDown) throw new Error('AcpSessionBridge is shutting down');
|
||||
|
||||
const entry = byId.get(sessionId);
|
||||
if (!entry) throw new SessionNotFoundError(sessionId);
|
||||
|
||||
const originatorClientId = resolveTrustedClientId(
|
||||
entry,
|
||||
context?.clientId,
|
||||
);
|
||||
|
||||
// Chain onto promptQueue and update tail — ensures:
|
||||
// 1. cd waits for any in-flight prompt to complete
|
||||
// 2. Subsequent prompts wait for cd to complete (prevents stale config.cwd)
|
||||
const cdPromise = entry.promptQueue.then(async () => {
|
||||
if (entry.promptActive) {
|
||||
throw new CdWhilePromptActiveError(sessionId);
|
||||
}
|
||||
|
||||
const ci = await ensureChannel();
|
||||
const raw = await ci.connection.extMethod(
|
||||
SERVE_CONTROL_EXT_METHODS.sessionCd,
|
||||
{
|
||||
sessionId,
|
||||
path: req.path,
|
||||
},
|
||||
);
|
||||
const extResult = raw as {
|
||||
previousCwd: string;
|
||||
newCwd: string;
|
||||
warnings: string[];
|
||||
};
|
||||
if (
|
||||
typeof extResult?.previousCwd !== 'string' ||
|
||||
typeof extResult?.newCwd !== 'string' ||
|
||||
!Array.isArray(extResult?.warnings)
|
||||
) {
|
||||
throw new Error(
|
||||
`changeSessionCwd: unexpected response shape from agent: ${JSON.stringify(raw)}`,
|
||||
);
|
||||
}
|
||||
|
||||
// State update inside the queue lambda — always executes when
|
||||
// the extMethod settles, regardless of caller timeout.
|
||||
if (extResult.previousCwd !== extResult.newCwd) {
|
||||
entry.events.publish({
|
||||
type: 'session_cwd_changed',
|
||||
data: {
|
||||
sessionId,
|
||||
previousCwd: extResult.previousCwd,
|
||||
newCwd: extResult.newCwd,
|
||||
},
|
||||
...(originatorClientId ? { originatorClientId } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
return extResult;
|
||||
});
|
||||
|
||||
// Queue tail tied to the raw extMethod settlement — subsequent
|
||||
// operations wait for the actual cd to finish, not the timeout.
|
||||
entry.promptQueue = cdPromise.then(
|
||||
() => undefined,
|
||||
() => undefined,
|
||||
);
|
||||
|
||||
// Timeout is caller-facing only: surfaces a deadline exceeded error
|
||||
// to the HTTP client without advancing the queue prematurely.
|
||||
const result = await withTimeout(
|
||||
cdPromise,
|
||||
Math.max(initTimeoutMs, 30_000),
|
||||
'changeSessionCwd',
|
||||
);
|
||||
|
||||
writeStderrLine(
|
||||
`qwen serve: session ${sessionId} cwd changed: ` +
|
||||
`${result.previousCwd} -> ${result.newCwd}` +
|
||||
(result.warnings.length > 0
|
||||
? ` (warnings: ${result.warnings.join('; ')})`
|
||||
: ''),
|
||||
);
|
||||
|
||||
return { sessionId, ...result };
|
||||
},
|
||||
|
||||
async closeSession(sessionId, context, closeOpts) {
|
||||
return closeSessionImpl(sessionId, context, closeOpts);
|
||||
},
|
||||
|
||||
@@ -501,3 +501,14 @@ export class BranchWhilePromptActiveError extends Error {
|
||||
this.sessionId = sessionId;
|
||||
}
|
||||
}
|
||||
|
||||
export class CdWhilePromptActiveError extends Error {
|
||||
readonly sessionId: string;
|
||||
constructor(sessionId: string) {
|
||||
super(
|
||||
`Cannot change directory for session ${sessionId}: a prompt is currently active`,
|
||||
);
|
||||
this.name = 'CdWhilePromptActiveError';
|
||||
this.sessionId = sessionId;
|
||||
}
|
||||
}
|
||||
@@ -124,6 +124,17 @@ export interface BridgeForkAgentResult {
|
||||
launched: boolean;
|
||||
}
|
||||
|
||||
export interface ChangeSessionCwdRequest {
|
||||
path: string;
|
||||
}
|
||||
|
||||
export interface ChangeSessionCwdResult {
|
||||
sessionId: string;
|
||||
previousCwd: string;
|
||||
newCwd: string;
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
/** Sparse summary used by `GET /workspace/:id/sessions`. */
|
||||
export interface BridgeSessionSummary {
|
||||
sessionId: string;
|
||||
@@ -302,6 +313,21 @@ export interface AcpSessionBridge {
|
||||
context?: BridgeClientRequestContext,
|
||||
): Promise<BridgeBranchedSession>;
|
||||
|
||||
/**
|
||||
* Change the working directory of a live session. The session must be
|
||||
* idle (no active prompt). Chains onto `entry.promptQueue` and updates
|
||||
* the tail to prevent concurrent mutations.
|
||||
*
|
||||
* Throws `CdWhilePromptActiveError` when a prompt is running,
|
||||
* `SessionNotFoundError` for unknown ids, and `InvalidClientIdError`
|
||||
* when the caller's client id is not bound to the session.
|
||||
*/
|
||||
changeSessionCwd(
|
||||
sessionId: string,
|
||||
req: ChangeSessionCwdRequest,
|
||||
context?: BridgeClientRequestContext,
|
||||
): Promise<ChangeSessionCwdResult>;
|
||||
|
||||
/**
|
||||
* Forward a prompt to the agent. Concurrent prompts against the same
|
||||
* session FIFO-serialize through a per-session queue.
|
||||
|
||||
@@ -144,6 +144,7 @@ export const SERVE_CONTROL_EXT_METHODS = {
|
||||
workspaceMcpRuntimeRemove: 'qwen/control/workspace/mcp/runtime-remove',
|
||||
workspaceReload: 'qwen/control/workspace/reload',
|
||||
workspaceExtensionsRefresh: 'qwen/control/workspace/extensions/refresh',
|
||||
sessionCd: 'qwen/control/session/cd',
|
||||
} as const;
|
||||
|
||||
export type ServeStatus =
|
||||
|
||||
@@ -174,7 +174,11 @@ import {
|
||||
getCurrentLanguage,
|
||||
SUPPORTED_LANGUAGES,
|
||||
} from '../i18n/index.js';
|
||||
import { isWorkspaceTrusted } from '../config/trustedFolders.js';
|
||||
import {
|
||||
isWorkspaceTrusted,
|
||||
isFolderTrustEnabled,
|
||||
loadTrustedFolders,
|
||||
} from '../config/trustedFolders.js';
|
||||
import {
|
||||
ACP_PREFLIGHT_KINDS,
|
||||
STATUS_SCHEMA_VERSION,
|
||||
@@ -5697,6 +5701,109 @@ class QwenAgent implements Agent {
|
||||
await this.closeStoredSession(sessionId);
|
||||
return { sessionId, closed: true };
|
||||
}
|
||||
case SERVE_CONTROL_EXT_METHODS.sessionCd: {
|
||||
const sessionId = params['sessionId'];
|
||||
const targetPath = params['path'];
|
||||
if (typeof sessionId !== 'string' || sessionId.length === 0) {
|
||||
throw RequestError.invalidParams(
|
||||
undefined,
|
||||
'Invalid or missing sessionId',
|
||||
);
|
||||
}
|
||||
if (
|
||||
typeof targetPath !== 'string' ||
|
||||
targetPath.length === 0 ||
|
||||
!path.isAbsolute(targetPath) ||
|
||||
targetPath.includes('\0')
|
||||
) {
|
||||
throw RequestError.invalidParams(
|
||||
undefined,
|
||||
'Invalid or missing path (must be an absolute path)',
|
||||
);
|
||||
}
|
||||
|
||||
const session = this.sessionOrThrow(sessionId);
|
||||
const config = session.getConfig();
|
||||
|
||||
// Restrictive sandbox check
|
||||
if (config.isRestrictiveSandbox()) {
|
||||
throw new RequestError(-32003, 'Restrictive sandbox mode active', {
|
||||
errorKind: 'restrictive_sandbox',
|
||||
});
|
||||
}
|
||||
|
||||
// Verify directory exists
|
||||
let stats;
|
||||
try {
|
||||
stats = await fs.stat(targetPath);
|
||||
} catch {
|
||||
throw new RequestError(-32002, `Directory not found: ${targetPath}`, {
|
||||
errorKind: 'directory_not_found',
|
||||
path: targetPath,
|
||||
});
|
||||
}
|
||||
if (!stats.isDirectory()) {
|
||||
throw new RequestError(-32002, `Not a directory: ${targetPath}`, {
|
||||
errorKind: 'directory_not_found',
|
||||
path: targetPath,
|
||||
});
|
||||
}
|
||||
|
||||
// Canonicalize path
|
||||
const canonicalPath = await fs.realpath(targetPath);
|
||||
|
||||
// Noop check
|
||||
const previousCwd = config.getTargetDir();
|
||||
if (canonicalPath === previousCwd) {
|
||||
return { previousCwd, newCwd: canonicalPath, warnings: [] };
|
||||
}
|
||||
|
||||
// Trust check
|
||||
if (isFolderTrustEnabled(this.settings.merged)) {
|
||||
const trustedFolders = loadTrustedFolders();
|
||||
if (trustedFolders.isPathTrusted(canonicalPath) !== true) {
|
||||
throw new RequestError(
|
||||
-32001,
|
||||
`Directory not trusted: ${canonicalPath}`,
|
||||
{ errorKind: 'directory_not_trusted', path: canonicalPath },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Relocate working directory (skip process.chdir and artifact
|
||||
// migration for ACP — storage stays at the bound workspace so
|
||||
// branch/load/lifecycle paths remain consistent).
|
||||
const warnings: string[] = [];
|
||||
const relocation = await config.relocateWorkingDirectory(
|
||||
canonicalPath,
|
||||
canonicalPath,
|
||||
{ skipProcessChdir: true, skipArtifactMigration: true },
|
||||
);
|
||||
if (relocation.memoryRefreshError) {
|
||||
warnings.push(
|
||||
`Memory refresh failed: ${
|
||||
relocation.memoryRefreshError instanceof Error
|
||||
? relocation.memoryRefreshError.message
|
||||
: String(relocation.memoryRefreshError)
|
||||
}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Update model context
|
||||
try {
|
||||
await config
|
||||
.getGeminiClient()
|
||||
?.addWorkingDirectoryChangedContext(previousCwd, canonicalPath);
|
||||
} catch (error) {
|
||||
warnings.push(
|
||||
`Model context refresh failed: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`,
|
||||
);
|
||||
}
|
||||
|
||||
return { previousCwd, newCwd: canonicalPath, warnings };
|
||||
}
|
||||
case SERVE_CONTROL_EXT_METHODS.sessionApprovalMode: {
|
||||
const sessionId = params['sessionId'];
|
||||
const mode = params['mode'];
|
||||
|
||||
@@ -81,6 +81,7 @@ export type {
|
||||
|
||||
export {
|
||||
BranchWhilePromptActiveError,
|
||||
CdWhilePromptActiveError,
|
||||
SessionNotFoundError,
|
||||
RestoreInProgressError,
|
||||
InvalidSessionScopeError,
|
||||
|
||||
@@ -308,6 +308,39 @@ export function registerSessionRoutes(
|
||||
}
|
||||
});
|
||||
|
||||
app.post('/session/:id/cd', mutate(), async (req, res) => {
|
||||
const sessionId = requireSessionId(req, res);
|
||||
if (sessionId === null) return;
|
||||
const body = safeBody(req);
|
||||
const targetPath = body['path'];
|
||||
if (
|
||||
typeof targetPath !== 'string' ||
|
||||
targetPath.length === 0 ||
|
||||
!path.isAbsolute(targetPath)
|
||||
) {
|
||||
res.status(400).json({
|
||||
error: '`path` is required and must be an absolute path',
|
||||
code: 'invalid_path',
|
||||
});
|
||||
return;
|
||||
}
|
||||
const clientId = parseClientIdHeader(req, res);
|
||||
if (clientId === null) return;
|
||||
try {
|
||||
const result = await bridge.changeSessionCwd(
|
||||
sessionId,
|
||||
{ path: targetPath },
|
||||
clientId !== undefined ? { clientId } : undefined,
|
||||
);
|
||||
res.status(200).json(result);
|
||||
} catch (err) {
|
||||
sendBridgeError(res, err, {
|
||||
route: 'POST /session/:id/cd',
|
||||
sessionId,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
app.get('/session/:id/status', (req, res) => {
|
||||
const sessionId = requireSessionId(req, res);
|
||||
if (sessionId === null) return;
|
||||
|
||||
@@ -15,6 +15,7 @@ import { writeStderrLine } from '../../utils/stdioHelpers.js';
|
||||
import {
|
||||
BranchWhilePromptActiveError,
|
||||
CancelSentinelCollisionError,
|
||||
CdWhilePromptActiveError,
|
||||
InvalidClientIdError,
|
||||
InvalidPermissionOptionError,
|
||||
InvalidRewindTargetError,
|
||||
@@ -218,6 +219,14 @@ export function sendBridgeError(
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (err instanceof CdWhilePromptActiveError) {
|
||||
res.status(409).json({
|
||||
error: err.message,
|
||||
code: 'cd_while_prompt_active',
|
||||
sessionId: err.sessionId,
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (err instanceof TrustGateError) {
|
||||
// Trust-folder rejection. 403 because the workspace's trust posture
|
||||
// forbids the privileged mode.
|
||||
@@ -427,6 +436,31 @@ export function sendBridgeError(
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (kind === 'restrictive_sandbox') {
|
||||
res.status(403).json({
|
||||
error: errorMessage(err),
|
||||
code: 'restrictive_sandbox',
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (kind === 'directory_not_found') {
|
||||
const d = data as { path?: string };
|
||||
res.status(400).json({
|
||||
error: errorMessage(err),
|
||||
code: 'directory_not_found',
|
||||
path: d.path,
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (kind === 'directory_not_trusted') {
|
||||
const d = data as { path?: string };
|
||||
res.status(403).json({
|
||||
error: errorMessage(err),
|
||||
code: 'directory_not_trusted',
|
||||
path: d.path,
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
// 5xx is the kind of error operators need to see in their daemon log
|
||||
|
||||
@@ -3293,6 +3293,7 @@ export class Config {
|
||||
oldStorage: Storage,
|
||||
newStorage: Storage,
|
||||
oldDir: string,
|
||||
opts?: { skipProcessChdir?: boolean },
|
||||
): Promise<void> {
|
||||
this.chatRecordingService?.finalize();
|
||||
await this.chatRecordingService?.flush();
|
||||
@@ -3300,13 +3301,15 @@ export class Config {
|
||||
try {
|
||||
this.moveCurrentSessionArtifacts(oldStorage, newStorage);
|
||||
} catch (error) {
|
||||
try {
|
||||
process.chdir(oldDir);
|
||||
} catch (rollbackError) {
|
||||
this.debugLogger.warn(
|
||||
'Failed to roll back working directory after session artifact migration failed',
|
||||
rollbackError,
|
||||
);
|
||||
if (!opts?.skipProcessChdir) {
|
||||
try {
|
||||
process.chdir(oldDir);
|
||||
} catch (rollbackError) {
|
||||
this.debugLogger.warn(
|
||||
'Failed to roll back working directory after session artifact migration failed',
|
||||
rollbackError,
|
||||
);
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
@@ -3315,8 +3318,11 @@ export class Config {
|
||||
async relocateWorkingDirectory(
|
||||
newDir: string,
|
||||
expectedCanonicalDir?: string,
|
||||
opts?: { skipProcessChdir?: boolean; skipArtifactMigration?: boolean },
|
||||
): Promise<{ memoryRefreshError?: unknown }> {
|
||||
const oldDir = fs.realpathSync(process.cwd());
|
||||
const oldDir = opts?.skipProcessChdir
|
||||
? this.cwd
|
||||
: fs.realpathSync(process.cwd());
|
||||
const targetPath = path.resolve(newDir);
|
||||
const expected = expectedCanonicalDir ?? fs.realpathSync(targetPath);
|
||||
if (!fs.statSync(targetPath).isDirectory()) {
|
||||
@@ -3327,23 +3333,42 @@ export class Config {
|
||||
this.explicitIncludeDirectories,
|
||||
);
|
||||
|
||||
process.chdir(targetPath);
|
||||
const actualCwd = fs.realpathSync(process.cwd());
|
||||
if (actualCwd !== expected) {
|
||||
process.chdir(oldDir);
|
||||
throw new Error(
|
||||
`Changed directory to ${actualCwd}, expected ${expected}.`,
|
||||
);
|
||||
if (!opts?.skipProcessChdir) {
|
||||
process.chdir(targetPath);
|
||||
const actualCwd = fs.realpathSync(process.cwd());
|
||||
if (actualCwd !== expected) {
|
||||
process.chdir(oldDir);
|
||||
throw new Error(
|
||||
`Changed directory to ${actualCwd}, expected ${expected}.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// ACP path: validate realpath matches expected without calling
|
||||
// process.chdir — guards against TOCTOU swaps between the trust
|
||||
// check and the config state update.
|
||||
const actualCanonical = fs.realpathSync(targetPath);
|
||||
if (actualCanonical !== expected) {
|
||||
throw new Error(
|
||||
`Realpath mismatch: resolved ${actualCanonical}, expected ${expected}.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const oldStorage = this.storage;
|
||||
const newStorage = new Storage(expected);
|
||||
await this.prepareSessionArtifactMigration(oldStorage, newStorage, oldDir);
|
||||
if (!opts?.skipArtifactMigration) {
|
||||
const newStorage = new Storage(expected);
|
||||
await this.prepareSessionArtifactMigration(
|
||||
oldStorage,
|
||||
newStorage,
|
||||
oldDir,
|
||||
opts,
|
||||
);
|
||||
this.storage = newStorage;
|
||||
this.chatRecordingService?.resetStoragePaths();
|
||||
}
|
||||
|
||||
this.targetDir = expected;
|
||||
this.cwd = expected;
|
||||
this.storage = newStorage;
|
||||
this.chatRecordingService?.resetStoragePaths();
|
||||
await this.refreshCurrentRuntimeStatus(expected);
|
||||
this.workspaceContext.applyRootDirectories(workspaceDirectories);
|
||||
this.fileDiscoveryService = null;
|
||||
|
||||
Reference in new issue
Block a user