Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 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
14 changes: 14 additions & 0 deletions packages/acp-bridge/src/sessionAttachments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,20 @@ describe('SessionAttachmentStore', () => {
'application/octet-stream',
'kept.bin',
);
const originalOpen = fs.open.bind(fs);
const originalStat = fs.stat.bind(fs);
const open = vi.spyOn(fs, 'open').mockImplementation(async (...args) => {
const handle = await originalOpen(...args);
if (String(args[0]) === root) {
const handleStat = handle.stat.bind(handle);
handle.stat = (async (...statArgs) => {
const result = await handleStat(...statArgs);
Object.defineProperty(result, 'ino', { value: 1 });
return result;
}) as typeof handle.stat;
}
return handle;
});
const stat = vi.spyOn(fs, 'stat').mockImplementation(async (...args) => {
const result = await originalStat(...args);
if (String(args[0]) === root) {
Expand All @@ -1065,6 +1078,7 @@ describe('SessionAttachmentStore', () => {
fs.stat(path.join(root, 'session-session-a', 'kept.bin')),
).resolves.toBeDefined();
} finally {
open.mockRestore();
stat.mockRestore();
rename.mockRestore();
await fs.rm(root, { recursive: true, force: true });
Expand Down
74 changes: 72 additions & 2 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,18 @@ import {
afterAll,
type MockInstance,
} from 'vitest';
import type { Stats } from 'node:fs';
import * as fs from 'node:fs/promises';
import * as os from 'node:os';
import * as path from 'node:path';

// Spy-mode interception so the unverifiable-inode degradation test can
// pose the managed-directory stats; everything else delegates to the real
// implementation.
vi.mock('node:fs/promises', { spy: true });

const realFsPromises =
await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises');
import { NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE } from '@qwen-code/acp-bridge/bridgeErrors';
import { ACP_EVENT_LOOP_STALL_RESTART_MS } from '@qwen-code/channel-base';
import { getConversationDirectoryName } from '../utils/conversation-directory-identity.js';
Expand Down Expand Up @@ -4466,18 +4475,20 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
const canonicalChild = await fs.realpath(path.join(canonicalRoot, name));
const rootStats = await fs.lstat(canonicalRoot);
const childStats = await fs.lstat(canonicalChild);
const normalizedInode = (inode: number) =>
Number.isSafeInteger(inode) && inode > 0 ? inode : 0;
return {
canonicalSessionId: sessionId,
root: {
canonicalPath: canonicalRoot,
device: rootStats.dev,
inode: rootStats.ino,
inode: normalizedInode(rootStats.ino),
},
child: {
name,
canonicalPath: canonicalChild,
device: childStats.dev,
inode: childStats.ino,
inode: normalizedInode(childStats.ino),
},
};
}
Expand Down Expand Up @@ -5379,6 +5390,65 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
});
});

it.each([0, Number.MAX_SAFE_INTEGER + 1])(
'degrades instead of rejecting an unverifiable standalone inode %s',
async (inode) => {
// Mirrors the conversation-directory-identity degradation test: on a
// volume whose file IDs are not verifiable (FAT-style ino 0, or a
// Windows ID past the safe-integer range) the expectation builds
// with inode 0 and sessionCd must resolve — a predicate that
// unconditionally claims verifiability fails the parity check and
// reintroduces the 'working directory identity is compromised'
// rejection this PR removes.
await withEmptyTrustedFolders(async (directory) => {
const settings = makeSessionSettings({ mcpServers: {} });
const { agent, agentPromise, sessionId, innerConfig } =
await bootRelocatableSession(settings, 'expected-capability');
const root = path.join(directory, 'Conversations');
const target = path.join(root, getConversationDirectoryName(sessionId));
await fs.mkdir(target, { recursive: true, mode: 0o700 });
const realLstat = realFsPromises.lstat;
vi.mocked(fs.lstat).mockImplementation((async (p: string) => {
const stats = await realLstat(p);
return {
...stats,
ino: inode,
isDirectory: () => stats.isDirectory(),
isSymbolicLink: () => stats.isSymbolicLink(),
} as Stats;
}) as unknown as typeof fs.lstat);
try {
const expectation = await managedConversationExpectation(
root,
sessionId,
);
expect(expectation.root.inode).toBe(0);
expect(expectation.child.inode).toBe(0);
innerConfig.getSessionSourceType.mockReturnValue('standalone');
innerConfig.getTargetDir.mockReturnValue(
expectation.child.canonicalPath,
);

await expect(
agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionCd, {
sessionId,
path: expectation.child.canonicalPath,
allowedRoots: [expectation.root.canonicalPath],
managedRelocation: 'live-conversation',
conversationDirectoryExpectation: expectation,
}),
).resolves.toMatchObject({
newCwd: expectation.child.canonicalPath,
});
} finally {
vi.mocked(fs.lstat).mockRestore();
mockConnectionState.resolve();
await agentPromise;
}
});
},
);

it('rejects standalone relocation while child-local background work is active', async () => {
await withEmptyTrustedFolders(async (directory) => {
const settings = makeSessionSettings({ mcpServers: {} });
Expand Down
65 changes: 42 additions & 23 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ import { createLoadedSettingsAdapter } from '../config/loadedSettingsAdapter.js'
import { isCompatibleLiveSessionSource } from '../runtime/live-session-source.js';
import {
getConversationDirectoryName,
hasVerifiableInode,
isSameConversationPath,
} from '../utils/conversation-directory-identity.js';
import type { ApprovalModeValue } from './session/types.js';
Expand Down Expand Up @@ -3198,6 +3199,28 @@ function isOwnerOnlyDirectory(stats: Stats): boolean {
return (stats.mode & 0o077) === 0;
}

function hasExpectedManagedDirectoryIdentity(
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
stats: Stats,
expected: { device: number; inode: number },
): boolean {
const inodeVerifiable = hasVerifiableInode(stats.ino);
return (
stats.dev === expected.device &&
inodeVerifiable === (expected.inode !== 0) &&
(!inodeVerifiable || stats.ino === expected.inode)
);
}

function isSameManagedDirectoryIdentity(left: Stats, right: Stats): boolean {
const leftVerifiable = hasVerifiableInode(left.ino);
const rightVerifiable = hasVerifiableInode(right.ino);
return (
left.dev === right.dev &&
leftVerifiable === rightVerifiable &&
(!leftVerifiable || left.ino === right.ino)
);
}

function hasOnlyKeys(
value: Record<string, unknown>,
keys: readonly string[],
Expand Down Expand Up @@ -3315,10 +3338,8 @@ async function assertManagedConversationDirectoryIdentity(
!isOwnerOnlyDirectory(rootBefore) ||
!isOwnerOnlyDirectory(rootAfter) ||
!isSameConversationPath(canonicalRoot, expectation.root.canonicalPath) ||
rootBefore.dev !== expectation.root.device ||
rootBefore.ino !== expectation.root.inode ||
rootAfter.dev !== expectation.root.device ||
rootAfter.ino !== expectation.root.inode
!hasExpectedManagedDirectoryIdentity(rootBefore, expectation.root) ||
!hasExpectedManagedDirectoryIdentity(rootAfter, expectation.root)
) {
throw managedConversationDirectoryError(false);
}
Expand All @@ -3339,10 +3360,8 @@ async function assertManagedConversationDirectoryIdentity(
!isOwnerOnlyDirectory(childBefore) ||
!isOwnerOnlyDirectory(childAfter) ||
!isSameConversationPath(canonicalChild, expectation.child.canonicalPath) ||
childBefore.dev !== expectation.child.device ||
childBefore.ino !== expectation.child.inode ||
childAfter.dev !== expectation.child.device ||
childAfter.ino !== expectation.child.inode
!hasExpectedManagedDirectoryIdentity(childBefore, expectation.child) ||
!hasExpectedManagedDirectoryIdentity(childAfter, expectation.child)
) {
throw managedConversationDirectoryError(false);
}
Expand Down Expand Up @@ -10003,16 +10022,16 @@ class QwenAgent implements Agent {
if (
!isOwnerOnlyDirectory(rootBefore) ||
!isOwnerOnlyDirectory(rootAfter) ||
rootBefore.dev !== rootAfter.dev ||
rootBefore.ino !== rootAfter.ino ||
!isSameManagedDirectoryIdentity(rootBefore, rootAfter) ||
(conversationDirectoryExpectation !== undefined &&
(!isSameConversationPath(
canonicalRoot,
conversationDirectoryExpectation.root.canonicalPath,
) ||
rootAfter.dev !==
conversationDirectoryExpectation.root.device ||
rootAfter.ino !== conversationDirectoryExpectation.root.inode))
!hasExpectedManagedDirectoryIdentity(
rootAfter,
conversationDirectoryExpectation.root,
)))
) {
if (conversationDirectoryExpectation !== undefined) {
throw managedConversationDirectoryError(false);
Expand Down Expand Up @@ -10045,17 +10064,16 @@ class QwenAgent implements Agent {
if (
!isOwnerOnlyDirectory(targetBefore) ||
!isOwnerOnlyDirectory(targetAfter) ||
targetBefore.dev !== targetAfter.dev ||
targetBefore.ino !== targetAfter.ino ||
!isSameManagedDirectoryIdentity(targetBefore, targetAfter) ||
(conversationDirectoryExpectation !== undefined &&
(!isSameConversationPath(
canonicalPath,
conversationDirectoryExpectation.child.canonicalPath,
) ||
targetAfter.dev !==
conversationDirectoryExpectation.child.device ||
targetAfter.ino !==
conversationDirectoryExpectation.child.inode)) ||
!hasExpectedManagedDirectoryIdentity(
targetAfter,
conversationDirectoryExpectation.child,
))) ||
relativeTarget.length === 0 ||
relativeTarget.startsWith('..') ||
path.isAbsolute(relativeTarget) ||
Expand Down Expand Up @@ -10086,11 +10104,12 @@ class QwenAgent implements Agent {
}
if (
!isOwnerOnlyDirectory(rootFinal) ||
rootFinal.dev !== rootAfter.dev ||
rootFinal.ino !== rootAfter.ino ||
!isSameManagedDirectoryIdentity(rootFinal, rootAfter) ||
(conversationDirectoryExpectation !== undefined &&
(rootFinal.dev !== conversationDirectoryExpectation.root.device ||
rootFinal.ino !== conversationDirectoryExpectation.root.inode))
!hasExpectedManagedDirectoryIdentity(
rootFinal,
conversationDirectoryExpectation.root,
))
) {
if (conversationDirectoryExpectation !== undefined) {
throw managedConversationDirectoryError(false);
Expand Down
17 changes: 4 additions & 13 deletions packages/cli/src/commands/review/capture-local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
invisibleTrackedPaths,
movedSince,
hashWorktreeFiles,
isPathProvablyAbsent,
readLocalCache,
revisionIdentities,
stateIdOf,
Expand Down Expand Up @@ -186,19 +187,9 @@ function vanishedStillOnDisk(
const onDisk: string[] = [];
for (const p of Object.keys(cachedFiles)) {
if (Object.hasOwn(currentHashes, p)) continue;
try {
lstatSync(join(repoRoot, p));
} catch (err) {
// Only ENOENT proves deletion. Any other failure — EACCES under an
// unreadable ancestor, ELOOP, EIO — is a path that may well still
// hold the cached bytes, and folding it into "gone" let the round end
// at a DECIDED scope-emptied stop over bytes no round captured
// (R19-3). Unmeasurable is uncertifiable: it stays in the on-disk
// set, whose downstream is a refusal at the cost of a full round.
if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
continue; // genuinely gone — the symmetric difference owns it
}
}
// Only ENOENT-proven absence may skip the re-check (R19-3): every
// unmeasurable shape the helper refuses stays on the on-disk list.
if (isPathProvablyAbsent(repoRoot, p)) continue;
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
onDisk.push(p);
}
if (onDisk.length === 0) return [];
Expand Down
Loading
Loading