mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-11 03:13:12 +00:00
Merge pull request #1718 from sgaluza/fix/adopt-settings-publish-by-replacement
fix(shared-manager): publish adopted settings by replacement
This commit is contained in:
4 files changed
+346
-149
No files matched your search
@@ -1,30 +1,11 @@
|
||||
{
|
||||
"scope": "src/**/*.{ts,tsx,js,jsx,mjs,cjs}",
|
||||
"syncFs": {
|
||||
"totalOccurrences": 2519,
|
||||
"totalOccurrences": 2509,
|
||||
"filesAffected": 263,
|
||||
"hotpathOccurrences": 1193,
|
||||
"hotpathOccurrences": 1183,
|
||||
"hotpathFilesAffected": 156,
|
||||
"topHotpathFiles": [
|
||||
{
|
||||
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
||||
"count": 39,
|
||||
"calls": [
|
||||
"closeSync",
|
||||
"fsyncSync",
|
||||
"linkSync",
|
||||
"lstatSync",
|
||||
"openSync",
|
||||
"readdirSync",
|
||||
"readFileSync",
|
||||
"readlinkSync",
|
||||
"renameSync",
|
||||
"statSync",
|
||||
"unlinkSync",
|
||||
"writeFileSync"
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/utils/browser/mcp-installer.ts",
|
||||
"count": 32,
|
||||
@@ -57,6 +38,26 @@
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
||||
"count": 29,
|
||||
"calls": [
|
||||
"chmodSync",
|
||||
"closeSync",
|
||||
"fsyncSync",
|
||||
"linkSync",
|
||||
"lstatSync",
|
||||
"openSync",
|
||||
"readdirSync",
|
||||
"readFileSync",
|
||||
"readlinkSync",
|
||||
"renameSync",
|
||||
"statSync",
|
||||
"unlinkSync",
|
||||
"writeFileSync"
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/utils/claude-symlink-manager.ts",
|
||||
"count": 27,
|
||||
@@ -244,25 +245,6 @@
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
||||
"count": 39,
|
||||
"calls": [
|
||||
"closeSync",
|
||||
"fsyncSync",
|
||||
"linkSync",
|
||||
"lstatSync",
|
||||
"openSync",
|
||||
"readdirSync",
|
||||
"readFileSync",
|
||||
"readlinkSync",
|
||||
"renameSync",
|
||||
"statSync",
|
||||
"unlinkSync",
|
||||
"writeFileSync"
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/cliproxy/executor/__tests__/variant-port-integration.test.js",
|
||||
"count": 36,
|
||||
@@ -302,6 +284,22 @@
|
||||
"writeFileSync"
|
||||
],
|
||||
"markers": []
|
||||
},
|
||||
{
|
||||
"file": "src/utils/browser/mcp-installer.ts",
|
||||
"count": 32,
|
||||
"calls": [
|
||||
"chmodSync",
|
||||
"copyFileSync",
|
||||
"existsSync",
|
||||
"mkdirSync",
|
||||
"readFileSync",
|
||||
"renameSync",
|
||||
"statSync",
|
||||
"unlinkSync",
|
||||
"writeFileSync"
|
||||
],
|
||||
"markers": []
|
||||
}
|
||||
]
|
||||
},
|
||||
@@ -720,7 +718,7 @@
|
||||
]
|
||||
},
|
||||
"largeFiles": {
|
||||
"countOver400": 92,
|
||||
"countOver400": 93,
|
||||
"countOver600": 42,
|
||||
"topOver400": [
|
||||
{
|
||||
|
||||
@@ -6,9 +6,9 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
||||
|
||||
| Metric | Value |
|
||||
|---|---:|
|
||||
| Sync fs occurrences (all) | 2519 |
|
||||
| Sync fs occurrences (all) | 2509 |
|
||||
| Sync fs files affected (all) | 263 |
|
||||
| Sync fs occurrences (runtime hotpaths) | 1193 |
|
||||
| Sync fs occurrences (runtime hotpaths) | 1183 |
|
||||
| Sync fs files affected (runtime hotpaths) | 156 |
|
||||
| Legacy shim markers | 465 |
|
||||
| Legacy shim files affected | 176 |
|
||||
@@ -17,9 +17,9 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
||||
|
||||
| File | Sync Calls | API Names |
|
||||
|---|---:|---|
|
||||
| `src/management/shared-manager/diverged-file-adopter.ts` | 39 | closeSync, fsyncSync, linkSync, lstatSync, openSync, readdirSync, readFileSync, readlinkSync, renameSync, statSync, unlinkSync, writeFileSync |
|
||||
| `src/utils/browser/mcp-installer.ts` | 32 | chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync |
|
||||
| `src/utils/image-analysis/mcp-installer.ts` | 30 | chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync |
|
||||
| `src/management/shared-manager/diverged-file-adopter.ts` | 29 | chmodSync, closeSync, fsyncSync, linkSync, lstatSync, openSync, readdirSync, readFileSync, readlinkSync, renameSync, statSync, unlinkSync, writeFileSync |
|
||||
| `src/utils/claude-symlink-manager.ts` | 27 | copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, unlinkSync |
|
||||
| `src/cliproxy/config/env-builder.ts` | 25 | existsSync, mkdirSync, readFileSync, writeFileSync |
|
||||
| `src/management/shared-manager/migrations.ts` | 25 | copyFileSync, cpSync, existsSync, lstatSync, mkdirSync, readdirSync, symlinkSync, unlinkSync, writeFileSync |
|
||||
@@ -62,7 +62,7 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
||||
| hotpath console.error/warn files | 81 |
|
||||
| files with createLogger | 65/765 |
|
||||
| subdomains with zero createLogger | 15 (api, bin, channels, cliproxy, cliproxy/accounts, cliproxy/ai-providers, cliproxy/binary, cliproxy/config, cliproxy/management, cliproxy/sync, cliproxy/types, config, dispatcher, shared, types) |
|
||||
| files > 400 LOC | 92 |
|
||||
| files > 400 LOC | 93 |
|
||||
| files > 600 LOC | 42 |
|
||||
|
||||
### Top Hotpath console.error/warn Files
|
||||
|
||||
@@ -137,41 +137,92 @@ function validateManagedJson(filePath: string, content: Buffer): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
function atomicWriteFile(targetPath: string, content: Buffer, mode: number): void {
|
||||
const tempPath = `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
||||
/**
|
||||
* Identity of the canonical inode as it was read. Publication compares it
|
||||
* again immediately before replacing the file, so a concurrent writer is
|
||||
* detected instead of silently overwritten.
|
||||
*/
|
||||
interface CanonicalIdentity {
|
||||
ino: number;
|
||||
mtimeMs: number;
|
||||
size: number;
|
||||
}
|
||||
|
||||
function canonicalIdentityOf(stats: fs.Stats): CanonicalIdentity {
|
||||
return { ino: stats.ino, mtimeMs: stats.mtimeMs, size: stats.size };
|
||||
}
|
||||
|
||||
function canonicalIdentityMatches(
|
||||
expected: CanonicalIdentity | null,
|
||||
current: CanonicalIdentity | null
|
||||
): boolean {
|
||||
if (!expected || !current) return expected === current;
|
||||
return (
|
||||
expected.ino === current.ino &&
|
||||
expected.mtimeMs === current.mtimeMs &&
|
||||
expected.size === current.size
|
||||
);
|
||||
}
|
||||
|
||||
function tempWritePath(targetPath: string): string {
|
||||
return `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Write content into a fresh temp file and make it durable, so whatever
|
||||
* publishes it under its final name publishes complete bytes.
|
||||
*/
|
||||
function writeDurableTempFile(tempPath: string, content: Buffer, mode: number): void {
|
||||
let descriptor: number | null = null;
|
||||
try {
|
||||
descriptor = fs.openSync(tempPath, 'wx', mode);
|
||||
fs.fchmodSync(descriptor, mode);
|
||||
fs.writeFileSync(descriptor, content);
|
||||
fs.fsyncSync(descriptor);
|
||||
fs.closeSync(descriptor);
|
||||
descriptor = null;
|
||||
fs.linkSync(tempPath, targetPath);
|
||||
fs.unlinkSync(tempPath);
|
||||
} catch (err) {
|
||||
} finally {
|
||||
if (descriptor !== null) {
|
||||
fs.closeSync(descriptor);
|
||||
}
|
||||
try {
|
||||
fs.unlinkSync(tempPath);
|
||||
} catch {
|
||||
// The temp file may not have been created or may already have been renamed.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function discardTempFile(tempPath: string): void {
|
||||
try {
|
||||
fs.unlinkSync(tempPath);
|
||||
} catch {
|
||||
// The temp file may not have been created or may already have been renamed.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a file only if the path is free, writing the content atomically.
|
||||
*
|
||||
* link() is an atomic no-replace operation, so concurrent CCS processes
|
||||
* cannot overwrite each other's sidecar artifacts.
|
||||
*/
|
||||
function createFileNoReplace(targetPath: string, content: Buffer, mode: number): void {
|
||||
const tempPath = tempWritePath(targetPath);
|
||||
try {
|
||||
writeDurableTempFile(tempPath, content, mode);
|
||||
fs.linkSync(tempPath, targetPath);
|
||||
fs.unlinkSync(tempPath);
|
||||
} catch (err) {
|
||||
discardTempFile(tempPath);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
function publishBackupNoReplace(sourcePath: string, canonicalPath: string): string {
|
||||
const basePath = `${canonicalPath}.bak-ccs-adopt`;
|
||||
const content = fs.readFileSync(sourcePath);
|
||||
const mode = fs.statSync(sourcePath).mode & 0o777;
|
||||
/**
|
||||
* Publish a sidecar next to a managed file, never replacing an existing one.
|
||||
* Numbered suffixes keep every concurrent writer's artifact recoverable.
|
||||
*/
|
||||
function publishSidecarNoReplace(basePath: string, content: Buffer, mode: number): string {
|
||||
let sequence = 0;
|
||||
while (true) {
|
||||
const backupPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
||||
const sidecarPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
||||
try {
|
||||
atomicWriteFile(backupPath, content, mode);
|
||||
return backupPath;
|
||||
createFileNoReplace(sidecarPath, content, mode);
|
||||
return sidecarPath;
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
||||
sequence++;
|
||||
@@ -179,43 +230,69 @@ function publishBackupNoReplace(sourcePath: string, canonicalPath: string): stri
|
||||
}
|
||||
}
|
||||
|
||||
function publishAdoptedRecoveryNoReplace(sourcePath: string, divergedPath: string): string {
|
||||
const basePath = `${divergedPath}.ccs-adopted-recovery`;
|
||||
const content = fs.readFileSync(sourcePath);
|
||||
const mode = fs.statSync(sourcePath).mode & 0o777;
|
||||
let sequence = 0;
|
||||
while (true) {
|
||||
const recoveryPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
||||
try {
|
||||
atomicWriteFile(recoveryPath, content, mode);
|
||||
return recoveryPath;
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
||||
sequence++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function restoreCanonicalClaim(claimPath: string, writePath: string, canonicalPath: string): void {
|
||||
/**
|
||||
* Publish adopted content onto the canonical path by replacement.
|
||||
*
|
||||
* The canonical path is never emptied: a fully written temp file is renamed
|
||||
* over it, so no window exists in which Claude Code or a second `ccs` can
|
||||
* observe the path as missing and seed an empty placeholder there.
|
||||
*
|
||||
* A compare-and-swap guard runs as late as possible - after the temp file is
|
||||
* durable, immediately before the rename. When the canonical inode changed
|
||||
* since it was read, publication is refused rather than clobbering a writer
|
||||
* that got there first; the adopted bytes stay in the sidecars the caller
|
||||
* published. A pure chmod is not a content change, so the mode the inode
|
||||
* carries at publication time wins.
|
||||
*
|
||||
* The guard is read-then-act, not atomic: POSIX offers no compare-and-swap
|
||||
* rename, so a writer landing between the check and the rename is still
|
||||
* overwritten. That is a narrowing, not a guarantee - the window shrinks from
|
||||
* the ~100 ms the old claim-and-republish path left open to two adjacent
|
||||
* syscalls, and the pre-image sidecar the caller published keeps even that
|
||||
* outcome recoverable. Do not build stricter guarantees on top of it.
|
||||
*/
|
||||
function publishCanonicalContent(
|
||||
writePath: string,
|
||||
content: Buffer,
|
||||
mode: number,
|
||||
expected: CanonicalIdentity | null
|
||||
): void {
|
||||
const tempPath = tempWritePath(writePath);
|
||||
try {
|
||||
fs.linkSync(claimPath, writePath);
|
||||
fs.unlinkSync(claimPath);
|
||||
writeDurableTempFile(tempPath, content, mode);
|
||||
|
||||
const currentStats = getLstatSync(writePath);
|
||||
const current = currentStats?.isFile() ? canonicalIdentityOf(currentStats) : null;
|
||||
if (!canonicalIdentityMatches(expected, current)) {
|
||||
// EEXIST is this file's marker for "a race was detected", the same code
|
||||
// the reappeared-path and concurrent-replacement guards raise. It does
|
||||
// not mean a no-replace link() or open('wx') hit an existing path.
|
||||
throw Object.assign(new TypeError(`Canonical file changed during adoption: ${writePath}`), {
|
||||
code: 'EEXIST',
|
||||
});
|
||||
}
|
||||
|
||||
const publishMode = currentStats ? currentStats.mode & 0o777 : mode;
|
||||
if (publishMode !== mode) {
|
||||
fs.chmodSync(tempPath, publishMode);
|
||||
}
|
||||
fs.renameSync(tempPath, writePath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
||||
publishBackupNoReplace(claimPath, canonicalPath);
|
||||
fs.unlinkSync(claimPath);
|
||||
discardTempFile(tempPath);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
function getCanonicalFile(canonicalPath: string): {
|
||||
content: Buffer | null;
|
||||
identity: CanonicalIdentity | null;
|
||||
mode: number;
|
||||
mtimeMs: number | null;
|
||||
writePath: string;
|
||||
} {
|
||||
const canonicalLstat = getLstatSync(canonicalPath);
|
||||
if (!canonicalLstat) {
|
||||
return { content: null, mode: 0o600, mtimeMs: null, writePath: canonicalPath };
|
||||
return { content: null, identity: null, mode: 0o600, mtimeMs: null, writePath: canonicalPath };
|
||||
}
|
||||
|
||||
let writePath = canonicalPath;
|
||||
@@ -230,8 +307,12 @@ function getCanonicalFile(canonicalPath: string): {
|
||||
});
|
||||
}
|
||||
|
||||
// The identity describes the inode as of the stat above, taken before the
|
||||
// content read: a write landing in between leaves us holding newer bytes
|
||||
// than the identity describes, and publication fails closed.
|
||||
return {
|
||||
content: fs.readFileSync(canonicalPath),
|
||||
content: fs.readFileSync(writePath),
|
||||
identity: canonicalIdentityOf(canonicalStats),
|
||||
mode: canonicalStats.mode & 0o777,
|
||||
mtimeMs: canonicalStats.mtimeMs,
|
||||
writePath,
|
||||
@@ -260,8 +341,6 @@ export function adoptDivergedFileContent(
|
||||
throw err;
|
||||
}
|
||||
|
||||
let canonicalClaimPath: string | null = null;
|
||||
let canonicalWritePath: string | null = null;
|
||||
let divergencePreserved = false;
|
||||
try {
|
||||
const claimedStats = fs.lstatSync(claimPath);
|
||||
@@ -288,24 +367,12 @@ export function adoptDivergedFileContent(
|
||||
}
|
||||
|
||||
const canonical = getCanonicalFile(canonicalPath);
|
||||
let current = canonical.content;
|
||||
let publishMode = canonical.mode;
|
||||
if (current) {
|
||||
canonicalWritePath = canonical.writePath;
|
||||
canonicalClaimPath = `${canonical.writePath}.ccs-canonical-claim-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
||||
fs.renameSync(canonical.writePath, canonicalClaimPath);
|
||||
const currentStats = fs.statSync(canonicalClaimPath);
|
||||
publishMode = currentStats.mode & 0o777;
|
||||
current = fs.readFileSync(canonicalClaimPath);
|
||||
if (diverged.equals(current)) {
|
||||
restoreCanonicalClaim(canonicalClaimPath, canonical.writePath, canonicalPath);
|
||||
canonicalClaimPath = null;
|
||||
if (canonical.content) {
|
||||
if (diverged.equals(canonical.content)) {
|
||||
fs.unlinkSync(claimPath);
|
||||
return 'claimed';
|
||||
}
|
||||
if (claimedStats.mtimeMs <= currentStats.mtimeMs) {
|
||||
restoreCanonicalClaim(canonicalClaimPath, canonical.writePath, canonicalPath);
|
||||
canonicalClaimPath = null;
|
||||
if (claimedStats.mtimeMs <= (canonical.mtimeMs ?? 0)) {
|
||||
preserveClaim(
|
||||
claimPath,
|
||||
divergedPath,
|
||||
@@ -313,14 +380,17 @@ export function adoptDivergedFileContent(
|
||||
);
|
||||
return 'claimed';
|
||||
}
|
||||
// Publish the pre-image before the canonical file is replaced, so an
|
||||
// interruption mid-publication still leaves the old content recoverable.
|
||||
publishSidecarNoReplace(`${canonicalPath}.bak-ccs-adopt`, canonical.content, canonical.mode);
|
||||
}
|
||||
publishSidecarNoReplace(
|
||||
`${divergedPath}.ccs-adopted-recovery`,
|
||||
diverged,
|
||||
claimedStats.mode & 0o777
|
||||
);
|
||||
|
||||
if (canonicalClaimPath) {
|
||||
publishBackupNoReplace(canonicalClaimPath, canonicalPath);
|
||||
}
|
||||
publishAdoptedRecoveryNoReplace(claimPath, divergedPath);
|
||||
|
||||
atomicWriteFile(canonical.writePath, diverged, publishMode);
|
||||
publishCanonicalContent(canonical.writePath, diverged, canonical.mode, canonical.identity);
|
||||
if (!fs.readFileSync(canonicalPath).equals(diverged)) {
|
||||
throw Object.assign(
|
||||
new TypeError(`Canonical adoption postcondition failed: ${canonicalPath}`),
|
||||
@@ -329,10 +399,6 @@ export function adoptDivergedFileContent(
|
||||
}
|
||||
);
|
||||
}
|
||||
if (canonicalClaimPath) {
|
||||
fs.unlinkSync(canonicalClaimPath);
|
||||
canonicalClaimPath = null;
|
||||
}
|
||||
if (getLstatSync(divergedPath)) {
|
||||
preserveClaim(claimPath, divergedPath, `Concurrent replacement detected at ${divergedPath}`);
|
||||
divergencePreserved = true;
|
||||
@@ -346,10 +412,6 @@ export function adoptDivergedFileContent(
|
||||
);
|
||||
return 'claimed';
|
||||
} catch (err) {
|
||||
if (canonicalClaimPath && canonicalWritePath) {
|
||||
restoreCanonicalClaim(canonicalClaimPath, canonicalWritePath, canonicalPath);
|
||||
canonicalClaimPath = null;
|
||||
}
|
||||
if (divergencePreserved) {
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -544,7 +544,18 @@ describe('SharedManager', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves a canonical write that lands during no-replace publication', () => {
|
||||
/**
|
||||
* Replaces 'preserves a canonical write that lands during no-replace
|
||||
* publication', which locked in the outcome of the CCS-4 incident.
|
||||
*
|
||||
* The intent it encoded - never clobber a writer that got to the canonical
|
||||
* file first - is kept, but it is now enforced by the compare-and-swap
|
||||
* guard instead of by an EEXIST from a no-replace link. The difference
|
||||
* that matters: the canonical path is no longer emptied first, so only a
|
||||
* genuinely concurrent write can land here, and it is preserved without
|
||||
* costing the user the settings that were already there.
|
||||
*/
|
||||
it('refuses to publish when the canonical file changes before publication', () => {
|
||||
const manager = new SharedManager();
|
||||
const instancePath = instanceDir('canonical-race');
|
||||
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
||||
@@ -555,24 +566,136 @@ describe('SharedManager', () => {
|
||||
writeJson(divergedPath, { generation: 1 });
|
||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||
|
||||
const originalLinkSync = fs.linkSync;
|
||||
const linkSpy = spyOn(fs, 'linkSync').mockImplementation(((
|
||||
existingPath: fs.PathLike,
|
||||
newPath: fs.PathLike
|
||||
// Land the competing write while the publication temp file is being
|
||||
// prepared, i.e. after the canonical bytes were read but before the
|
||||
// compare-and-swap guard re-checks the inode.
|
||||
const originalOpenSync = fs.openSync;
|
||||
const openSpy = spyOn(fs, 'openSync').mockImplementation(((
|
||||
openPath: fs.PathLike,
|
||||
flags: number | string,
|
||||
mode?: fs.Mode
|
||||
) => {
|
||||
if (String(newPath) === canonicalPath && String(existingPath).includes('.ccs-write-')) {
|
||||
if (String(openPath).startsWith(`${canonicalPath}.ccs-write-`)) {
|
||||
writeJson(canonicalPath, { generation: 99 });
|
||||
}
|
||||
return originalLinkSync(existingPath, newPath);
|
||||
}) as typeof fs.linkSync);
|
||||
return originalOpenSync(openPath, flags, mode);
|
||||
}) as typeof fs.openSync);
|
||||
|
||||
expect(() => manager.linkSharedDirectories(instancePath)).toThrow();
|
||||
linkSpy.mockRestore();
|
||||
expect(() => manager.linkSharedDirectories(instancePath)).toThrow(
|
||||
'Canonical file changed during adoption'
|
||||
);
|
||||
openSpy.mockRestore();
|
||||
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
||||
expect(readJson(divergedPath)).toEqual({ generation: 1 });
|
||||
expect(readJson(`${canonicalPath}.bak-ccs-adopt`)).toEqual({ generation: 0 });
|
||||
expect(readJson(`${divergedPath}.ccs-adopted-recovery`)).toEqual({ generation: 1 });
|
||||
});
|
||||
|
||||
/**
|
||||
* Model a foreign writer that shares ownership of the canonical
|
||||
* settings.json: the instant the path is left without a file, it lands an
|
||||
* empty-settings placeholder there. Claude Code does exactly this on
|
||||
* startup, and a second concurrent `ccs` does the same through
|
||||
* shared-dir-linker.ts:127.
|
||||
*
|
||||
* The writer reacts to the path becoming empty rather than to one specific
|
||||
* call site, so it keeps modelling the race no matter which fs primitive
|
||||
* the adopter uses to move the canonical file out of the way.
|
||||
*/
|
||||
function installForeignCanonicalWriter(
|
||||
canonicalPath: string,
|
||||
placeholder: string
|
||||
): { placeholderWrites: () => number; restore: () => void } {
|
||||
let placeholderWrites = 0;
|
||||
const claimEmptyCanonicalPath = (): void => {
|
||||
if (fs.existsSync(canonicalPath)) return;
|
||||
fs.writeFileSync(canonicalPath, placeholder, 'utf8');
|
||||
placeholderWrites++;
|
||||
};
|
||||
|
||||
const originalRenameSync = fs.renameSync;
|
||||
const renameSpy = spyOn(fs, 'renameSync').mockImplementation(((
|
||||
oldPath: fs.PathLike,
|
||||
newPath: fs.PathLike
|
||||
) => {
|
||||
originalRenameSync(oldPath, newPath);
|
||||
claimEmptyCanonicalPath();
|
||||
}) as typeof fs.renameSync);
|
||||
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
const unlinkSpy = spyOn(fs, 'unlinkSync').mockImplementation(((targetPath: fs.PathLike) => {
|
||||
originalUnlinkSync(targetPath);
|
||||
claimEmptyCanonicalPath();
|
||||
}) as typeof fs.unlinkSync);
|
||||
|
||||
return {
|
||||
placeholderWrites: () => placeholderWrites,
|
||||
restore: () => {
|
||||
renameSpy.mockRestore();
|
||||
unlinkSpy.mockRestore();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Reproduce the 2026-08-20 incident: adoption moves the canonical
|
||||
* settings.json aside, a foreign writer fills the empty path, and the user
|
||||
* ends up with neither the adopted nor the previous settings on the live
|
||||
* path.
|
||||
*
|
||||
* The opposite outcome used to be pinned by 'preserves a canonical write
|
||||
* that lands during no-replace publication'; that test was rewritten as
|
||||
* 'refuses to publish when the canonical file changes before publication'
|
||||
* once publication stopped emptying the canonical path.
|
||||
*/
|
||||
const foreignWriterCases: ReadonlyArray<{ writer: string; placeholder: string }> = [
|
||||
// Claude Code starts, finds no settings file and writes empty settings.
|
||||
{ writer: 'Claude Code', placeholder: '{}\n' },
|
||||
// A second concurrent `ccs` provisions the same placeholder without the
|
||||
// trailing newline (shared-dir-linker.ts:127).
|
||||
{ writer: 'a concurrent ccs run', placeholder: JSON.stringify({}, null, 2) },
|
||||
];
|
||||
|
||||
for (const { writer, placeholder } of foreignWriterCases) {
|
||||
it(`keeps live settings when ${writer} fills the canonical path during adoption`, () => {
|
||||
const manager = new SharedManager();
|
||||
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
||||
const sharedSettingsPath = path.join(ccsDir(), 'shared', 'settings.json');
|
||||
const previousSettings = {
|
||||
model: 'opus',
|
||||
permissions: { allow: ['Bash(git status:*)'] },
|
||||
};
|
||||
const divergedSettings = {
|
||||
model: 'opus',
|
||||
permissions: { allow: ['Bash(git status:*)', 'Bash(git diff:*)'] },
|
||||
};
|
||||
|
||||
fs.mkdirSync(claudeDir(), { recursive: true });
|
||||
fs.mkdirSync(path.join(ccsDir(), 'shared'), { recursive: true });
|
||||
writeJson(canonicalPath, previousSettings);
|
||||
writeJson(sharedSettingsPath, divergedSettings);
|
||||
setMtime(sharedSettingsPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||
|
||||
const foreignWriter = installForeignCanonicalWriter(canonicalPath, placeholder);
|
||||
try {
|
||||
manager.ensureSharedDirectories();
|
||||
} catch {
|
||||
// Losing the race may abort reconciliation; the user's live settings
|
||||
// must survive either way.
|
||||
} finally {
|
||||
foreignWriter.restore();
|
||||
}
|
||||
|
||||
// The writer only fires when the canonical path is observed empty, so
|
||||
// zero writes is the invariant itself: adoption never left the path
|
||||
// without a regular file.
|
||||
expect(foreignWriter.placeholderWrites()).toBe(0);
|
||||
expect(fs.existsSync(canonicalPath)).toBe(true);
|
||||
expect(fs.readFileSync(canonicalPath, 'utf8')).not.toBe(placeholder);
|
||||
expect([divergedSettings, previousSettings]).toContainEqual(readJson(canonicalPath));
|
||||
});
|
||||
}
|
||||
|
||||
it('keeps adopted bytes recoverable when canonical changes after verification', () => {
|
||||
const manager = new SharedManager();
|
||||
const instancePath = instanceDir('late-canonical-writer');
|
||||
@@ -584,10 +707,12 @@ describe('SharedManager', () => {
|
||||
writeJson(divergedPath, { generation: 1 });
|
||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||
|
||||
// The diverged claim is dropped only after publication was verified, so
|
||||
// a write injected there lands strictly after adoption completed.
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
let injected = false;
|
||||
const unlinkSpy = spyOn(fs, 'unlinkSync').mockImplementation(((targetPath: fs.PathLike) => {
|
||||
if (!injected && String(targetPath).includes('.ccs-canonical-claim-')) {
|
||||
if (!injected && String(targetPath).includes('.ccs-adopt-claim-')) {
|
||||
injected = true;
|
||||
writeJson(canonicalPath, { generation: 99 });
|
||||
}
|
||||
@@ -596,6 +721,7 @@ describe('SharedManager', () => {
|
||||
|
||||
manager.linkSharedDirectories(instancePath);
|
||||
unlinkSpy.mockRestore();
|
||||
expect(injected).toBe(true);
|
||||
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
||||
expect(readJson(`${divergedPath}.ccs-adopted-recovery`)).toEqual({ generation: 1 });
|
||||
});
|
||||
@@ -611,20 +737,30 @@ describe('SharedManager', () => {
|
||||
writeJson(divergedPath, { generation: 1 });
|
||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
// Recreate the managed source right after the canonical file was
|
||||
// replaced, while the adopter is still cleaning up.
|
||||
const originalRenameSync = fs.renameSync;
|
||||
let injected = false;
|
||||
const unlinkSpy = spyOn(fs, 'unlinkSync').mockImplementation(((targetPath: fs.PathLike) => {
|
||||
if (!injected && String(targetPath).includes('.ccs-canonical-claim-')) {
|
||||
const renameSpy = spyOn(fs, 'renameSync').mockImplementation(((
|
||||
oldPath: fs.PathLike,
|
||||
newPath: fs.PathLike
|
||||
) => {
|
||||
originalRenameSync(oldPath, newPath);
|
||||
if (
|
||||
!injected &&
|
||||
String(newPath) === canonicalPath &&
|
||||
String(oldPath).startsWith(`${canonicalPath}.ccs-write-`)
|
||||
) {
|
||||
injected = true;
|
||||
writeJson(divergedPath, { generation: 2 });
|
||||
}
|
||||
return originalUnlinkSync(targetPath);
|
||||
}) as typeof fs.unlinkSync);
|
||||
}) as typeof fs.renameSync);
|
||||
|
||||
expect(() => manager.linkSharedDirectories(instancePath)).toThrow(
|
||||
'Concurrent replacement detected'
|
||||
);
|
||||
unlinkSpy.mockRestore();
|
||||
renameSpy.mockRestore();
|
||||
expect(injected).toBe(true);
|
||||
expect(readJson(divergedPath)).toEqual({ generation: 2 });
|
||||
});
|
||||
|
||||
@@ -694,7 +830,7 @@ describe('SharedManager', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('uses the mode of the canonical inode actually claimed for publication', () => {
|
||||
it('publishes with the mode the canonical inode carries at publication time', () => {
|
||||
const manager = new SharedManager();
|
||||
const instancePath = instanceDir('concurrent-mode');
|
||||
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
||||
@@ -706,23 +842,24 @@ describe('SharedManager', () => {
|
||||
writeJson(divergedPath, { generation: 1 });
|
||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||
|
||||
const originalRenameSync = fs.renameSync;
|
||||
const renameSpy = spyOn(fs, 'renameSync').mockImplementation(((
|
||||
oldPath: fs.PathLike,
|
||||
newPath: fs.PathLike
|
||||
// A chmod between reading the canonical file and publishing it leaves
|
||||
// the content untouched, so publication proceeds with the newer mode.
|
||||
const originalOpenSync = fs.openSync;
|
||||
const openSpy = spyOn(fs, 'openSync').mockImplementation(((
|
||||
openPath: fs.PathLike,
|
||||
flags: number | string,
|
||||
mode?: fs.Mode
|
||||
) => {
|
||||
if (
|
||||
String(oldPath) === canonicalPath &&
|
||||
String(newPath).includes('.ccs-canonical-claim-')
|
||||
) {
|
||||
if (String(openPath).startsWith(`${canonicalPath}.ccs-write-`)) {
|
||||
fs.chmodSync(canonicalPath, 0o664);
|
||||
}
|
||||
return originalRenameSync(oldPath, newPath);
|
||||
}) as typeof fs.renameSync);
|
||||
return originalOpenSync(openPath, flags, mode);
|
||||
}) as typeof fs.openSync);
|
||||
|
||||
manager.linkSharedDirectories(instancePath);
|
||||
renameSpy.mockRestore();
|
||||
openSpy.mockRestore();
|
||||
expect(fs.statSync(canonicalPath).mode & 0o777).toBe(0o664);
|
||||
expect(readJson(canonicalPath)).toEqual({ generation: 1 });
|
||||
});
|
||||
|
||||
it('recovers an interrupted canonical claim before provisioning defaults', () => {
|
||||
|
||||
Reference in new issue
Block a user