Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
dc92622
feat(core): add atomicWriteFileSync + forceMode option
doudouOUC May 19, 2026
0231fe4
refactor(core): migrate credential writes to atomicWriteFile (#4095 T…
doudouOUC May 19, 2026
015fa38
refactor(core): migrate memory state writes to atomicWriteFile (#4095…
doudouOUC May 19, 2026
2a6ded1
refactor: migrate config + logger + state writes to atomic helpers (#…
doudouOUC May 19, 2026
b7badc7
fix(core): flush JSONL appends to disk (#4095 Tier 3b, closes #3681)
doudouOUC May 19, 2026
8869ac3
refactor(core): migrate extension config + LSP edit to atomic write
doudouOUC May 19, 2026
d7f47ba
chore: cosmetic cleanups from PR review
doudouOUC May 19, 2026
0ed40df
fix(core): address Codex review findings on Phase 2 PR
doudouOUC May 19, 2026
12868d9
fix(lsp): refuse LSP edits to chmod 0444 files (Codex round 2)
doudouOUC May 19, 2026
2ac4e7a
fix(core): drop withTimeout around atomic credential write (Codex rou…
doudouOUC May 19, 2026
7b1a7ae
Merge remote-tracking branch 'origin/main' into worktree-ethereal-bub…
doudouOUC May 19, 2026
741265d
test(core): add rename-retry + EXDEV-fallback coverage (#4333 review)
doudouOUC May 19, 2026
f817330
fix(test): update telemetry sdk.test.ts appendFile assertions for flu…
doudouOUC May 19, 2026
dd549ae
test: cover LSP error branches + sync EXDEV cleanup + JSONL writes (#…
doudouOUC May 20, 2026
f6c4d2f
fix(core,test): wrap atomic write errors + guard root user + cover sa…
doudouOUC May 20, 2026
7a562eb
fix(core,lsp): annotate sync errors correctly + cover EXDEV fallback …
doudouOUC May 20, 2026
1e7fc3e
test(core): cover EXDEV-fallback-write-failure annotation path (#4333…
doudouOUC May 20, 2026
17222d7
fix(core): annotate guard, drop debug fsync, add noFollow for creds (…
doudouOUC May 20, 2026
21853b8
fix(core): EXDEV fallback honors noFollow + fix test correctness (#43…
doudouOUC May 20, 2026
7c47664
fix(core): close TOCTOU window on noFollow EXDEV fallback + cover ENO…
doudouOUC May 20, 2026
0aada50
test(core): skip new noFollow EXDEV-fallback tests on Windows (NTFS p…
doudouOUC May 21, 2026
0435199
fix(core): narrow fchmod catch + verify mechanism on noFollow EXDEV
doudouOUC May 21, 2026
c66537b
fix(core): clean up O_EXCL orphan on noFollow EXDEV fchmod failure + …
doudouOUC May 21, 2026
c951958
fix(core): annotate symlink errors + cover ENOTSUP/EACCES + log orpha…
doudouOUC May 21, 2026
72f2ffa
fix(core): widen atomicWriteJSON options + tighten trustedHooks perms…
doudouOUC May 21, 2026
ef6c08a
fix(core): force JS slow path for appendFile flush + correct debugLog…
doudouOUC May 21, 2026
b52eeaf
fix(core): narrow tryChmod catch + writeFileSync(fd) + cover credenti…
doudouOUC May 21, 2026
65aa871
fix(core): route orphan unlink through seam + cover tryChmod + truste…
doudouOUC May 22, 2026
1504692
fix(core): add noFollow to trustedHooks/trustedFolders + route tmp cl…
doudouOUC May 23, 2026
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
21 changes: 16 additions & 5 deletions packages/cli/src/config/trustedFolders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
*/

import * as osActual from 'node:os';
import { FatalConfigError, ideContextStore } from '@qwen-code/qwen-code-core';
import {
atomicWriteFileSync,
FatalConfigError,
ideContextStore,
} from '@qwen-code/qwen-code-core';
import {
describe,
it,
Expand Down Expand Up @@ -50,17 +54,24 @@ vi.mock('strip-json-comments', () => ({
default: vi.fn((content) => content),
}));

vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
return {
...actual,
atomicWriteFileSync: vi.fn(),
};
});

describe('Trusted Folders Loading', () => {
let mockFsExistsSync: Mocked<typeof fs.existsSync>;
let mockStripJsonComments: Mocked<typeof stripJsonComments>;
let mockFsWriteFileSync: Mocked<typeof fs.writeFileSync>;

beforeEach(() => {
resetTrustedFoldersForTesting();
vi.resetAllMocks();
mockFsExistsSync = vi.mocked(fs.existsSync);
mockStripJsonComments = vi.mocked(stripJsonComments);
mockFsWriteFileSync = vi.mocked(fs.writeFileSync);
vi.mocked(osActual.homedir).mockReturnValue('/mock/home/user');
(mockStripJsonComments as unknown as Mock).mockImplementation(
(jsonString: string) => jsonString,
Expand Down Expand Up @@ -190,10 +201,10 @@ describe('Trusted Folders Loading', () => {
expect(loadedFolders.user.config['/new/path']).toBe(
TrustLevel.TRUST_FOLDER,
);
expect(mockFsWriteFileSync).toHaveBeenCalledWith(
expect(atomicWriteFileSync).toHaveBeenCalledWith(
getTrustedFoldersPath(),
JSON.stringify({ '/new/path': TrustLevel.TRUST_FOLDER }, null, 2),
{ encoding: 'utf-8', mode: 0o600 },
{ encoding: 'utf-8', mode: 0o600, forceMode: true },
);
});
});
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/config/trustedFolders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import {
atomicWriteFileSync,
FatalConfigError,
getErrorMessage,
isWithinRoot,
Expand Down Expand Up @@ -179,10 +180,10 @@ export function saveTrustedFolders(
fs.mkdirSync(dirPath, { recursive: true });
}

fs.writeFileSync(
atomicWriteFileSync(
trustedFoldersFile.path,
JSON.stringify(trustedFoldersFile.config, null, 2),
{ encoding: 'utf-8', mode: 0o600 },
{ encoding: 'utf-8', mode: 0o600, forceMode: true },
);
} catch (error) {
writeStderrLine('Error saving trusted folders file.');
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/services/tips/tipHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import * as fs from 'node:fs';
import * as path from 'node:path';
import { Storage } from '@qwen-code/qwen-code-core';
import { atomicWriteFileSync, Storage } from '@qwen-code/qwen-code-core';

interface TipHistoryEntry {
totalShown: number;
Expand Down Expand Up @@ -114,8 +114,9 @@ export class TipHistory {
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
}
fs.writeFileSync(this.filePath, JSON.stringify(this.data, null, 2), {
atomicWriteFileSync(this.filePath, JSON.stringify(this.data, null, 2), {
mode: 0o600,
forceMode: true,
});
} catch {
// Silently ignore write errors — tips are non-critical
Expand Down
32 changes: 28 additions & 4 deletions packages/core/src/core/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from './logger.js';
import { Storage } from '../config/storage.js';
import { getProjectHash } from '../utils/paths.js';
import { atomicWriteFile } from '../utils/atomicFileWrite.js';
import { promises as fs, existsSync } from 'node:fs';
import path from 'node:path';
import type { Content } from '@google/genai';
Expand Down Expand Up @@ -73,6 +74,26 @@ vi.mock('../utils/session.js', () => ({
sessionId: 'test-session-id',
}));

// Re-export the real atomicWriteFile so tests can override individual
// calls (e.g. .mockRejectedValueOnce) while preserving normal behavior.
// The default implementation is re-attached in `beforeEach` because the
// suite calls `vi.resetAllMocks()` which strips vi.fn(impl) back to no-op.
vi.mock('../utils/atomicFileWrite.js', async () => {
const actual = await vi.importActual<
typeof import('../utils/atomicFileWrite.js')
>('../utils/atomicFileWrite.js');
return {
...actual,
atomicWriteFile: vi.fn(actual.atomicWriteFile),
};
});

const realAtomicWriteFile = (
await vi.importActual<typeof import('../utils/atomicFileWrite.js')>(
'../utils/atomicFileWrite.js',
)
).atomicWriteFile;

vi.mock('../utils/debugLogger.js', async (importOriginal) => {
const original =
await importOriginal<typeof import('../utils/debugLogger.js')>();
Expand All @@ -93,6 +114,9 @@ describe('Logger', () => {

beforeEach(async () => {
vi.resetAllMocks();
// resetAllMocks blanks the vi.fn(actual) delegation — re-attach so the
// logger's initialize/append paths still hit the real disk.
vi.mocked(atomicWriteFile).mockImplementation(realAtomicWriteFile);
vi.useFakeTimers();
vi.setSystemTime(new Date('2025-01-01T12:00:00.000Z'));
originalHome = process.env['HOME'];
Expand Down Expand Up @@ -440,7 +464,7 @@ describe('Logger', () => {
});

it('should not throw, not increment messageId, and log error if writing to file fails', async () => {
vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(new Error('Disk full'));
vi.mocked(atomicWriteFile).mockRejectedValueOnce(new Error('Disk full'));
const initialMessageId = logger['messageId'];
const initialLogCount = logger['logs'].length;

Expand Down Expand Up @@ -892,7 +916,7 @@ describe('Logger', () => {
await logger.logMessage(MessageSenderType.USER, 'kept');
vi.advanceTimersByTime(1000);

vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(new Error('Disk full'));
vi.mocked(atomicWriteFile).mockRejectedValueOnce(new Error('Disk full'));
await logger.logMessage(MessageSenderType.USER, 'failed write');

expect(logger['lastLoggedUserEntry']).toBeNull();
Expand Down Expand Up @@ -938,7 +962,7 @@ describe('Logger', () => {
'cancelled prompt',
]);

vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(new Error('Disk full'));
vi.mocked(atomicWriteFile).mockRejectedValueOnce(new Error('Disk full'));
const removed = await logger.removeLastUserMessage();
expect(removed).toBe(false);

Expand Down Expand Up @@ -987,7 +1011,7 @@ describe('Logger', () => {
const trackedAfterUser = logger['lastLoggedUserEntry'];
expect(trackedAfterUser).not.toBeNull();

vi.spyOn(fs, 'writeFile').mockRejectedValueOnce(new Error('Disk full'));
vi.mocked(atomicWriteFile).mockRejectedValueOnce(new Error('Disk full'));
await logger.logMessage(MessageSenderType.MODEL_SWITCH, 'qwen→qwen-max');

// Tracker is unchanged — the non-USER failure didn't shift which
Expand Down
15 changes: 9 additions & 6 deletions packages/core/src/core/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import path from 'node:path';
import { promises as fs } from 'node:fs';
import type { Content } from '@google/genai';
import type { Storage } from '../config/storage.js';
import { atomicWriteFile } from '../utils/atomicFileWrite.js';
import { createDebugLogger, type DebugLogger } from '../utils/debugLogger.js';

const LOG_FILE_NAME = 'logs.json';
Expand Down Expand Up @@ -187,7 +188,7 @@ export class Logger {
}
this.logs = await this._readLogFile();
if (!fileExisted && this.logs.length === 0) {
await fs.writeFile(this.logFilePath, '[]', 'utf-8');
await atomicWriteFile(this.logFilePath, '[]', { encoding: 'utf-8' });
}
const sessionLogs = this.logs.filter(
(entry) => entry.sessionId === this.sessionId,
Expand Down Expand Up @@ -258,10 +259,10 @@ export class Logger {
currentLogsOnDisk.push(entryToAppend);

try {
await fs.writeFile(
await atomicWriteFile(
this.logFilePath,
JSON.stringify(currentLogsOnDisk, null, 2),
'utf-8',
{ encoding: 'utf-8' },
);
this.logs = currentLogsOnDisk;
return entryToAppend; // Return the successfully appended entry
Expand Down Expand Up @@ -461,10 +462,10 @@ export class Logger {
currentLogsOnDisk.splice(idx, 1);

try {
await fs.writeFile(
await atomicWriteFile(
logFilePath,
JSON.stringify(currentLogsOnDisk, null, 2),
'utf-8',
{ encoding: 'utf-8' },
);
this.logs = currentLogsOnDisk;
// Roll back this instance's nextMessageId so a subsequent log doesn't
Expand Down Expand Up @@ -539,7 +540,9 @@ export class Logger {
// Always save with the new encoded path.
const path = this._checkpointPath(tag);
try {
await fs.writeFile(path, JSON.stringify(conversation, null, 2), 'utf-8');
await atomicWriteFile(path, JSON.stringify(conversation, null, 2), {
encoding: 'utf-8',
});
} catch (error) {
this.debugLogger.error('Error writing to checkpoint file:', error);
}
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/extension/extensionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';

import {
atomicWriteFile,
atomicWriteFileSync,
} from '../utils/atomicFileWrite.js';
import { getErrorMessage } from '../utils/errors.js';
import {
EXTENSIONS_CONFIG_FILENAME,
Expand Down Expand Up @@ -530,7 +534,7 @@ export class ExtensionManager {

private writeEnablementConfig(config: AllExtensionsEnablementConfig): void {
fs.mkdirSync(this.configDir, { recursive: true });
fs.writeFileSync(this.configFilePath, JSON.stringify(config, null, 2));
atomicWriteFileSync(this.configFilePath, JSON.stringify(config, null, 2));
}

/**
Expand Down Expand Up @@ -1070,7 +1074,7 @@ export class ExtensionManager {
destinationPath,
INSTALL_METADATA_FILENAME,
);
await fs.promises.writeFile(metadataPath, metadataString);
await atomicWriteFile(metadataPath, metadataString);

extension = await this.loadExtension({ extensionDir: destinationPath });
if (!extension) {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/hooks/trustedHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
type HookDefinition,
type HookEventName,
} from './types.js';
import { atomicWriteFileSync } from '../utils/atomicFileWrite.js';
import { createDebugLogger } from '../utils/debugLogger.js';

const debugLogger = createDebugLogger('TRUSTED_HOOKS');
Expand Down Expand Up @@ -50,7 +51,7 @@ export class TrustedHooksManager {
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
}
fs.writeFileSync(
atomicWriteFileSync(
Comment thread
doudouOUC marked this conversation as resolved.
Comment thread
doudouOUC marked this conversation as resolved.
this.configPath,
JSON.stringify(this.trustedHooks, null, 2),
);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ export * from './followup/index.js';
// Utilities
// ============================================================================

export * from './utils/atomicFileWrite.js';
export * from './utils/browser.js';
export * from './utils/bundlePaths.js';
export * from './utils/configResolver.js';
Expand Down
28 changes: 23 additions & 5 deletions packages/core/src/lsp/NativeLspService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type {
import * as path from 'path';
import { fileURLToPath, pathToFileURL } from 'url';
import * as fs from 'node:fs';
import { atomicWriteFileSync } from '../utils/atomicFileWrite.js';
import { createDebugLogger } from '../utils/debugLogger.js';
import { globSync } from 'glob';

Expand Down Expand Up @@ -1306,12 +1307,17 @@ export class NativeLspService {
throw new Error(`Refusing to apply edits outside workspace: ${filePath}`);
}

// Read the current file content
// Read the current file content. Only treat ENOENT as "new file"; any
// other read failure (EACCES on a read-protected file, EISDIR, etc.)
// must propagate — otherwise the atomic rename below would silently
// replace the unreadable target with edits applied to an empty buffer.
let content: string;
try {
content = fs.readFileSync(filePath, 'utf-8');
} catch {
// File doesn't exist, treat as empty
} catch (err) {
if ((err as NodeJS.ErrnoException)?.code !== 'ENOENT') {
Comment thread
doudouOUC marked this conversation as resolved.
throw err;
}
content = '';
}

Expand Down Expand Up @@ -1347,8 +1353,20 @@ export class NativeLspService {
lines.splice(startLine, endLine - startLine + 1, ...newLines);
}

// Write back to file
fs.writeFileSync(filePath, lines.join('\n'), 'utf-8');
// Honor file-level write permissions. Atomic rename (tmp + rename)
// would otherwise bypass a chmod 0444 lock because rename only needs
// parent-directory write access. ENOENT is fine — LSP may be creating
// the file via edits.
try {
fs.accessSync(filePath, fs.constants.W_OK);
} catch (err) {
if ((err as NodeJS.ErrnoException)?.code !== 'ENOENT') {
throw err;
}
}

// Atomic write so a crash mid-edit can't leave the user file half-written.
atomicWriteFileSync(filePath, lines.join('\n'), { encoding: 'utf-8' });
Comment thread
doudouOUC marked this conversation as resolved.
Outdated
}

/**
Expand Down
Loading