mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-03 20:13:02 +00:00
fix(shared-manager): publish adopted settings by replacement
Adoption moved the canonical settings.json aside with rename() and left
the path empty until publication, roughly 100 ms later. Claude Code or a
second `ccs` starting inside that window found no file and seeded an
empty placeholder; publication then failed with EEXIST because link() is
no-replace, and the rollback published a backup and unlinked the claim,
destroying the only remaining copy of the user's settings. Recovering
meant digging through sidecar files by hand.
Publish by replacement instead: write a temp file next to the canonical
inode and rename() it over the target, so the path always holds a regular
file and no placeholder can be seeded. A compare-and-swap guard on
(ino, mtime, size) runs immediately before the rename and refuses to
publish when the canonical inode changed since it was read, so a writer
that got there first is still never clobbered. The pre-image backup is
published before the replacement, keeping the old content recoverable if
publication is interrupted.
Drops the canonical claim entirely along with restoreCanonicalClaim, and
folds the two identical sidecar publishers into one helper.
recoverOrphanedCanonicalClaim stays, since claims written by older
versions may still be on disk.
New tests cover both writers seen in the incident: Claude Code seeding
`{}` with a trailing newline, and a second `ccs` seeding the 2-byte
variant from shared-dir-linker. Four tests that pinned the claim-based
design were rewritten, among them `preserves a canonical write that
lands during no-replace publication`, whose intent is now enforced by
the CAS guard instead of by an EEXIST from a no-replace link.
Built [OnSteroids](https://onsteroids.ai)
This commit is contained in:
1 parent
a53980782f
commit
00a4dceb94
4 files changed
+304
-117
No files matched your search
@@ -1,15 +1,16 @@
|
|||||||
{
|
{
|
||||||
"scope": "src/**/*.{ts,tsx,js,jsx,mjs,cjs}",
|
"scope": "src/**/*.{ts,tsx,js,jsx,mjs,cjs}",
|
||||||
"syncFs": {
|
"syncFs": {
|
||||||
"totalOccurrences": 2519,
|
"totalOccurrences": 2516,
|
||||||
"filesAffected": 263,
|
"filesAffected": 263,
|
||||||
"hotpathOccurrences": 1193,
|
"hotpathOccurrences": 1190,
|
||||||
"hotpathFilesAffected": 156,
|
"hotpathFilesAffected": 156,
|
||||||
"topHotpathFiles": [
|
"topHotpathFiles": [
|
||||||
{
|
{
|
||||||
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
||||||
"count": 39,
|
"count": 36,
|
||||||
"calls": [
|
"calls": [
|
||||||
|
"chmodSync",
|
||||||
"closeSync",
|
"closeSync",
|
||||||
"fsyncSync",
|
"fsyncSync",
|
||||||
"linkSync",
|
"linkSync",
|
||||||
@@ -245,9 +246,24 @@
|
|||||||
"markers": []
|
"markers": []
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
"file": "src/cliproxy/executor/__tests__/variant-port-integration.test.js",
|
||||||
"count": 39,
|
"count": 36,
|
||||||
"calls": [
|
"calls": [
|
||||||
|
"existsSync",
|
||||||
|
"mkdirSync",
|
||||||
|
"readdirSync",
|
||||||
|
"readFileSync",
|
||||||
|
"rmSync",
|
||||||
|
"unlinkSync",
|
||||||
|
"writeFileSync"
|
||||||
|
],
|
||||||
|
"markers": []
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"file": "src/management/shared-manager/diverged-file-adopter.ts",
|
||||||
|
"count": 36,
|
||||||
|
"calls": [
|
||||||
|
"chmodSync",
|
||||||
"closeSync",
|
"closeSync",
|
||||||
"fsyncSync",
|
"fsyncSync",
|
||||||
"linkSync",
|
"linkSync",
|
||||||
@@ -263,20 +279,6 @@
|
|||||||
],
|
],
|
||||||
"markers": []
|
"markers": []
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"file": "src/cliproxy/executor/__tests__/variant-port-integration.test.js",
|
|
||||||
"count": 36,
|
|
||||||
"calls": [
|
|
||||||
"existsSync",
|
|
||||||
"mkdirSync",
|
|
||||||
"readdirSync",
|
|
||||||
"readFileSync",
|
|
||||||
"rmSync",
|
|
||||||
"unlinkSync",
|
|
||||||
"writeFileSync"
|
|
||||||
],
|
|
||||||
"markers": []
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"file": "src/cliproxy/executor/__tests__/composite-variant-service.test.ts",
|
"file": "src/cliproxy/executor/__tests__/composite-variant-service.test.ts",
|
||||||
"count": 33,
|
"count": 33,
|
||||||
@@ -720,7 +722,7 @@
|
|||||||
]
|
]
|
||||||
},
|
},
|
||||||
"largeFiles": {
|
"largeFiles": {
|
||||||
"countOver400": 92,
|
"countOver400": 93,
|
||||||
"countOver600": 42,
|
"countOver600": 42,
|
||||||
"topOver400": [
|
"topOver400": [
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -6,9 +6,9 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
|||||||
|
|
||||||
| Metric | Value |
|
| Metric | Value |
|
||||||
|---|---:|
|
|---|---:|
|
||||||
| Sync fs occurrences (all) | 2519 |
|
| Sync fs occurrences (all) | 2516 |
|
||||||
| Sync fs files affected (all) | 263 |
|
| Sync fs files affected (all) | 263 |
|
||||||
| Sync fs occurrences (runtime hotpaths) | 1193 |
|
| Sync fs occurrences (runtime hotpaths) | 1190 |
|
||||||
| Sync fs files affected (runtime hotpaths) | 156 |
|
| Sync fs files affected (runtime hotpaths) | 156 |
|
||||||
| Legacy shim markers | 465 |
|
| Legacy shim markers | 465 |
|
||||||
| Legacy shim files affected | 176 |
|
| Legacy shim files affected | 176 |
|
||||||
@@ -17,7 +17,7 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
|||||||
|
|
||||||
| File | Sync Calls | API Names |
|
| 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/management/shared-manager/diverged-file-adopter.ts` | 36 | chmodSync, 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/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/utils/image-analysis/mcp-installer.ts` | 30 | chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync |
|
||||||
| `src/utils/claude-symlink-manager.ts` | 27 | copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, unlinkSync |
|
| `src/utils/claude-symlink-manager.ts` | 27 | copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, readlinkSync, renameSync, rmSync, statSync, symlinkSync, unlinkSync |
|
||||||
@@ -62,7 +62,7 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}`
|
|||||||
| hotpath console.error/warn files | 81 |
|
| hotpath console.error/warn files | 81 |
|
||||||
| files with createLogger | 65/765 |
|
| 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) |
|
| 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 |
|
| files > 600 LOC | 42 |
|
||||||
|
|
||||||
### Top Hotpath console.error/warn Files
|
### Top Hotpath console.error/warn Files
|
||||||
|
|||||||
@@ -137,7 +137,42 @@ function validateManagedJson(filePath: string, content: Buffer): boolean {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function atomicWriteFile(targetPath: string, content: Buffer, mode: number): void {
|
/**
|
||||||
|
* 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 readCanonicalIdentity(writePath: string): CanonicalIdentity | null {
|
||||||
|
const stats = getLstatSync(writePath);
|
||||||
|
if (!stats?.isFile()) return null;
|
||||||
|
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
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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 = `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
const tempPath = `${targetPath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
||||||
let descriptor: number | null = null;
|
let descriptor: number | null = null;
|
||||||
try {
|
try {
|
||||||
@@ -162,16 +197,17 @@ function atomicWriteFile(targetPath: string, content: Buffer, mode: number): voi
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function publishBackupNoReplace(sourcePath: string, canonicalPath: string): string {
|
/**
|
||||||
const basePath = `${canonicalPath}.bak-ccs-adopt`;
|
* Publish a sidecar next to a managed file, never replacing an existing one.
|
||||||
const content = fs.readFileSync(sourcePath);
|
* Numbered suffixes keep every concurrent writer's artifact recoverable.
|
||||||
const mode = fs.statSync(sourcePath).mode & 0o777;
|
*/
|
||||||
|
function publishSidecarNoReplace(basePath: string, content: Buffer, mode: number): string {
|
||||||
let sequence = 0;
|
let sequence = 0;
|
||||||
while (true) {
|
while (true) {
|
||||||
const backupPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
const sidecarPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
||||||
try {
|
try {
|
||||||
atomicWriteFile(backupPath, content, mode);
|
createFileNoReplace(sidecarPath, content, mode);
|
||||||
return backupPath;
|
return sidecarPath;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
||||||
sequence++;
|
sequence++;
|
||||||
@@ -179,43 +215,74 @@ function publishBackupNoReplace(sourcePath: string, canonicalPath: string): stri
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function publishAdoptedRecoveryNoReplace(sourcePath: string, divergedPath: string): string {
|
/**
|
||||||
const basePath = `${divergedPath}.ccs-adopted-recovery`;
|
* Publish adopted content onto the canonical path by replacement.
|
||||||
const content = fs.readFileSync(sourcePath);
|
*
|
||||||
const mode = fs.statSync(sourcePath).mode & 0o777;
|
* The canonical path is never emptied: a fully written temp file is renamed
|
||||||
let sequence = 0;
|
* over it, so no window exists in which Claude Code or a second `ccs` can
|
||||||
while (true) {
|
* observe the path as missing and seed an empty placeholder there.
|
||||||
const recoveryPath = sequence === 0 ? basePath : `${basePath}-${sequence}`;
|
*
|
||||||
try {
|
* A compare-and-swap guard runs as late as possible - after the temp file is
|
||||||
atomicWriteFile(recoveryPath, content, mode);
|
* durable, immediately before the rename. When the canonical inode changed
|
||||||
return recoveryPath;
|
* since it was read, publication is refused rather than clobbering a writer
|
||||||
} catch (err) {
|
* that got there first; the adopted bytes stay in the sidecars the caller
|
||||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
* published. A pure chmod is not a content change, so the mode the inode
|
||||||
sequence++;
|
* carries at publication time wins.
|
||||||
}
|
*/
|
||||||
}
|
function publishCanonicalContent(
|
||||||
}
|
writePath: string,
|
||||||
|
content: Buffer,
|
||||||
function restoreCanonicalClaim(claimPath: string, writePath: string, canonicalPath: string): void {
|
mode: number,
|
||||||
|
expected: CanonicalIdentity | null
|
||||||
|
): void {
|
||||||
|
const tempPath = `${writePath}.ccs-write-${process.pid}-${Date.now()}-${adoptionClaimSequence++}`;
|
||||||
|
let descriptor: number | null = null;
|
||||||
try {
|
try {
|
||||||
fs.linkSync(claimPath, writePath);
|
descriptor = fs.openSync(tempPath, 'wx', mode);
|
||||||
fs.unlinkSync(claimPath);
|
fs.fchmodSync(descriptor, mode);
|
||||||
|
fs.writeFileSync(descriptor, content);
|
||||||
|
fs.fsyncSync(descriptor);
|
||||||
|
fs.closeSync(descriptor);
|
||||||
|
descriptor = null;
|
||||||
|
|
||||||
|
const currentStats = getLstatSync(writePath);
|
||||||
|
const current = currentStats?.isFile()
|
||||||
|
? { ino: currentStats.ino, mtimeMs: currentStats.mtimeMs, size: currentStats.size }
|
||||||
|
: null;
|
||||||
|
if (!canonicalIdentityMatches(expected, current)) {
|
||||||
|
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) {
|
} catch (err) {
|
||||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
|
if (descriptor !== null) {
|
||||||
publishBackupNoReplace(claimPath, canonicalPath);
|
fs.closeSync(descriptor);
|
||||||
fs.unlinkSync(claimPath);
|
}
|
||||||
|
try {
|
||||||
|
fs.unlinkSync(tempPath);
|
||||||
|
} catch {
|
||||||
|
// The temp file may not have been created or may already have been renamed.
|
||||||
|
}
|
||||||
|
throw err;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function getCanonicalFile(canonicalPath: string): {
|
function getCanonicalFile(canonicalPath: string): {
|
||||||
content: Buffer | null;
|
content: Buffer | null;
|
||||||
|
identity: CanonicalIdentity | null;
|
||||||
mode: number;
|
mode: number;
|
||||||
mtimeMs: number | null;
|
mtimeMs: number | null;
|
||||||
writePath: string;
|
writePath: string;
|
||||||
} {
|
} {
|
||||||
const canonicalLstat = getLstatSync(canonicalPath);
|
const canonicalLstat = getLstatSync(canonicalPath);
|
||||||
if (!canonicalLstat) {
|
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;
|
let writePath = canonicalPath;
|
||||||
@@ -230,8 +297,12 @@ function getCanonicalFile(canonicalPath: string): {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Identity first: a write landing between the two reads leaves us holding
|
||||||
|
// newer bytes than the identity describes, and publication fails closed.
|
||||||
|
const identity = readCanonicalIdentity(writePath);
|
||||||
return {
|
return {
|
||||||
content: fs.readFileSync(canonicalPath),
|
content: fs.readFileSync(writePath),
|
||||||
|
identity,
|
||||||
mode: canonicalStats.mode & 0o777,
|
mode: canonicalStats.mode & 0o777,
|
||||||
mtimeMs: canonicalStats.mtimeMs,
|
mtimeMs: canonicalStats.mtimeMs,
|
||||||
writePath,
|
writePath,
|
||||||
@@ -260,8 +331,6 @@ export function adoptDivergedFileContent(
|
|||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
|
|
||||||
let canonicalClaimPath: string | null = null;
|
|
||||||
let canonicalWritePath: string | null = null;
|
|
||||||
let divergencePreserved = false;
|
let divergencePreserved = false;
|
||||||
try {
|
try {
|
||||||
const claimedStats = fs.lstatSync(claimPath);
|
const claimedStats = fs.lstatSync(claimPath);
|
||||||
@@ -288,24 +357,12 @@ export function adoptDivergedFileContent(
|
|||||||
}
|
}
|
||||||
|
|
||||||
const canonical = getCanonicalFile(canonicalPath);
|
const canonical = getCanonicalFile(canonicalPath);
|
||||||
let current = canonical.content;
|
if (canonical.content) {
|
||||||
let publishMode = canonical.mode;
|
if (diverged.equals(canonical.content)) {
|
||||||
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;
|
|
||||||
fs.unlinkSync(claimPath);
|
fs.unlinkSync(claimPath);
|
||||||
return 'claimed';
|
return 'claimed';
|
||||||
}
|
}
|
||||||
if (claimedStats.mtimeMs <= currentStats.mtimeMs) {
|
if (claimedStats.mtimeMs <= (canonical.mtimeMs ?? 0)) {
|
||||||
restoreCanonicalClaim(canonicalClaimPath, canonical.writePath, canonicalPath);
|
|
||||||
canonicalClaimPath = null;
|
|
||||||
preserveClaim(
|
preserveClaim(
|
||||||
claimPath,
|
claimPath,
|
||||||
divergedPath,
|
divergedPath,
|
||||||
@@ -313,14 +370,17 @@ export function adoptDivergedFileContent(
|
|||||||
);
|
);
|
||||||
return 'claimed';
|
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) {
|
publishCanonicalContent(canonical.writePath, diverged, canonical.mode, canonical.identity);
|
||||||
publishBackupNoReplace(canonicalClaimPath, canonicalPath);
|
|
||||||
}
|
|
||||||
publishAdoptedRecoveryNoReplace(claimPath, divergedPath);
|
|
||||||
|
|
||||||
atomicWriteFile(canonical.writePath, diverged, publishMode);
|
|
||||||
if (!fs.readFileSync(canonicalPath).equals(diverged)) {
|
if (!fs.readFileSync(canonicalPath).equals(diverged)) {
|
||||||
throw Object.assign(
|
throw Object.assign(
|
||||||
new TypeError(`Canonical adoption postcondition failed: ${canonicalPath}`),
|
new TypeError(`Canonical adoption postcondition failed: ${canonicalPath}`),
|
||||||
@@ -329,10 +389,6 @@ export function adoptDivergedFileContent(
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (canonicalClaimPath) {
|
|
||||||
fs.unlinkSync(canonicalClaimPath);
|
|
||||||
canonicalClaimPath = null;
|
|
||||||
}
|
|
||||||
if (getLstatSync(divergedPath)) {
|
if (getLstatSync(divergedPath)) {
|
||||||
preserveClaim(claimPath, divergedPath, `Concurrent replacement detected at ${divergedPath}`);
|
preserveClaim(claimPath, divergedPath, `Concurrent replacement detected at ${divergedPath}`);
|
||||||
divergencePreserved = true;
|
divergencePreserved = true;
|
||||||
@@ -346,10 +402,6 @@ export function adoptDivergedFileContent(
|
|||||||
);
|
);
|
||||||
return 'claimed';
|
return 'claimed';
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (canonicalClaimPath && canonicalWritePath) {
|
|
||||||
restoreCanonicalClaim(canonicalClaimPath, canonicalWritePath, canonicalPath);
|
|
||||||
canonicalClaimPath = null;
|
|
||||||
}
|
|
||||||
if (divergencePreserved) {
|
if (divergencePreserved) {
|
||||||
throw err;
|
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 manager = new SharedManager();
|
||||||
const instancePath = instanceDir('canonical-race');
|
const instancePath = instanceDir('canonical-race');
|
||||||
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
||||||
@@ -555,24 +566,132 @@ describe('SharedManager', () => {
|
|||||||
writeJson(divergedPath, { generation: 1 });
|
writeJson(divergedPath, { generation: 1 });
|
||||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||||
|
|
||||||
const originalLinkSync = fs.linkSync;
|
// Land the competing write while the publication temp file is being
|
||||||
const linkSpy = spyOn(fs, 'linkSync').mockImplementation(((
|
// prepared, i.e. after the canonical bytes were read but before the
|
||||||
existingPath: fs.PathLike,
|
// compare-and-swap guard re-checks the inode.
|
||||||
newPath: fs.PathLike
|
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 });
|
writeJson(canonicalPath, { generation: 99 });
|
||||||
}
|
}
|
||||||
return originalLinkSync(existingPath, newPath);
|
return originalOpenSync(openPath, flags, mode);
|
||||||
}) as typeof fs.linkSync);
|
}) as typeof fs.openSync);
|
||||||
|
|
||||||
expect(() => manager.linkSharedDirectories(instancePath)).toThrow();
|
expect(() => manager.linkSharedDirectories(instancePath)).toThrow(
|
||||||
linkSpy.mockRestore();
|
'Canonical file changed during adoption'
|
||||||
|
);
|
||||||
|
openSpy.mockRestore();
|
||||||
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
||||||
expect(readJson(divergedPath)).toEqual({ generation: 1 });
|
expect(readJson(divergedPath)).toEqual({ generation: 1 });
|
||||||
expect(readJson(`${canonicalPath}.bak-ccs-adopt`)).toEqual({ generation: 0 });
|
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();
|
||||||
|
}
|
||||||
|
|
||||||
|
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', () => {
|
it('keeps adopted bytes recoverable when canonical changes after verification', () => {
|
||||||
const manager = new SharedManager();
|
const manager = new SharedManager();
|
||||||
const instancePath = instanceDir('late-canonical-writer');
|
const instancePath = instanceDir('late-canonical-writer');
|
||||||
@@ -584,10 +703,12 @@ describe('SharedManager', () => {
|
|||||||
writeJson(divergedPath, { generation: 1 });
|
writeJson(divergedPath, { generation: 1 });
|
||||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
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;
|
const originalUnlinkSync = fs.unlinkSync;
|
||||||
let injected = false;
|
let injected = false;
|
||||||
const unlinkSpy = spyOn(fs, 'unlinkSync').mockImplementation(((targetPath: fs.PathLike) => {
|
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;
|
injected = true;
|
||||||
writeJson(canonicalPath, { generation: 99 });
|
writeJson(canonicalPath, { generation: 99 });
|
||||||
}
|
}
|
||||||
@@ -596,6 +717,7 @@ describe('SharedManager', () => {
|
|||||||
|
|
||||||
manager.linkSharedDirectories(instancePath);
|
manager.linkSharedDirectories(instancePath);
|
||||||
unlinkSpy.mockRestore();
|
unlinkSpy.mockRestore();
|
||||||
|
expect(injected).toBe(true);
|
||||||
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
expect(readJson(canonicalPath)).toEqual({ generation: 99 });
|
||||||
expect(readJson(`${divergedPath}.ccs-adopted-recovery`)).toEqual({ generation: 1 });
|
expect(readJson(`${divergedPath}.ccs-adopted-recovery`)).toEqual({ generation: 1 });
|
||||||
});
|
});
|
||||||
@@ -611,20 +733,30 @@ describe('SharedManager', () => {
|
|||||||
writeJson(divergedPath, { generation: 1 });
|
writeJson(divergedPath, { generation: 1 });
|
||||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
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;
|
let injected = false;
|
||||||
const unlinkSpy = spyOn(fs, 'unlinkSync').mockImplementation(((targetPath: fs.PathLike) => {
|
const renameSpy = spyOn(fs, 'renameSync').mockImplementation(((
|
||||||
if (!injected && String(targetPath).includes('.ccs-canonical-claim-')) {
|
oldPath: fs.PathLike,
|
||||||
|
newPath: fs.PathLike
|
||||||
|
) => {
|
||||||
|
originalRenameSync(oldPath, newPath);
|
||||||
|
if (
|
||||||
|
!injected &&
|
||||||
|
String(newPath) === canonicalPath &&
|
||||||
|
String(oldPath).startsWith(`${canonicalPath}.ccs-write-`)
|
||||||
|
) {
|
||||||
injected = true;
|
injected = true;
|
||||||
writeJson(divergedPath, { generation: 2 });
|
writeJson(divergedPath, { generation: 2 });
|
||||||
}
|
}
|
||||||
return originalUnlinkSync(targetPath);
|
}) as typeof fs.renameSync);
|
||||||
}) as typeof fs.unlinkSync);
|
|
||||||
|
|
||||||
expect(() => manager.linkSharedDirectories(instancePath)).toThrow(
|
expect(() => manager.linkSharedDirectories(instancePath)).toThrow(
|
||||||
'Concurrent replacement detected'
|
'Concurrent replacement detected'
|
||||||
);
|
);
|
||||||
unlinkSpy.mockRestore();
|
renameSpy.mockRestore();
|
||||||
|
expect(injected).toBe(true);
|
||||||
expect(readJson(divergedPath)).toEqual({ generation: 2 });
|
expect(readJson(divergedPath)).toEqual({ generation: 2 });
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -694,7 +826,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 manager = new SharedManager();
|
||||||
const instancePath = instanceDir('concurrent-mode');
|
const instancePath = instanceDir('concurrent-mode');
|
||||||
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
const canonicalPath = path.join(claudeDir(), 'settings.json');
|
||||||
@@ -706,23 +838,24 @@ describe('SharedManager', () => {
|
|||||||
writeJson(divergedPath, { generation: 1 });
|
writeJson(divergedPath, { generation: 1 });
|
||||||
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
setMtime(divergedPath, fs.statSync(canonicalPath).mtimeMs + 2_000);
|
||||||
|
|
||||||
const originalRenameSync = fs.renameSync;
|
// A chmod between reading the canonical file and publishing it leaves
|
||||||
const renameSpy = spyOn(fs, 'renameSync').mockImplementation(((
|
// the content untouched, so publication proceeds with the newer mode.
|
||||||
oldPath: fs.PathLike,
|
const originalOpenSync = fs.openSync;
|
||||||
newPath: fs.PathLike
|
const openSpy = spyOn(fs, 'openSync').mockImplementation(((
|
||||||
|
openPath: fs.PathLike,
|
||||||
|
flags: number | string,
|
||||||
|
mode?: fs.Mode
|
||||||
) => {
|
) => {
|
||||||
if (
|
if (String(openPath).startsWith(`${canonicalPath}.ccs-write-`)) {
|
||||||
String(oldPath) === canonicalPath &&
|
|
||||||
String(newPath).includes('.ccs-canonical-claim-')
|
|
||||||
) {
|
|
||||||
fs.chmodSync(canonicalPath, 0o664);
|
fs.chmodSync(canonicalPath, 0o664);
|
||||||
}
|
}
|
||||||
return originalRenameSync(oldPath, newPath);
|
return originalOpenSync(openPath, flags, mode);
|
||||||
}) as typeof fs.renameSync);
|
}) as typeof fs.openSync);
|
||||||
|
|
||||||
manager.linkSharedDirectories(instancePath);
|
manager.linkSharedDirectories(instancePath);
|
||||||
renameSpy.mockRestore();
|
openSpy.mockRestore();
|
||||||
expect(fs.statSync(canonicalPath).mode & 0o777).toBe(0o664);
|
expect(fs.statSync(canonicalPath).mode & 0o777).toBe(0o664);
|
||||||
|
expect(readJson(canonicalPath)).toEqual({ generation: 1 });
|
||||||
});
|
});
|
||||||
|
|
||||||
it('recovers an interrupted canonical claim before provisioning defaults', () => {
|
it('recovers an interrupted canonical claim before provisioning defaults', () => {
|
||||||
|
|||||||
Reference in new issue
Block a user