fix(daemon): warn on non-atomic pid-publish degrade, correct reap comment, run claim-reap contract on Windows CI

This commit is contained in:
Brennan Benson
2026-08-28 08:02:57 -07:00
parent 3e46ca9ec7
commit 40ce87bb2c
5 changed files with 31 additions and 3 deletions
+1
View File
@@ -707,6 +707,7 @@ jobs:
src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts
src/shared/secure-file-fsync-flags.test.ts
src/main/daemon/daemon-pid-publish-disk-contract.test.ts
src/main/daemon/daemon-pid-publish-claim-reap.test.ts
src/main/ipc/pty-codex-account-attribution.test.ts
src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts
+1
View File
@@ -196,6 +196,7 @@ const WINDOWS_PACKAGE_TESTS = [
'src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts',
'src/shared/secure-file-fsync-flags.test.ts',
'src/main/daemon/daemon-pid-publish-disk-contract.test.ts',
'src/main/daemon/daemon-pid-publish-claim-reap.test.ts',
'src/main/ipc/pty-codex-account-attribution.test.ts',
'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts'
]
@@ -55,15 +55,19 @@ function listPublishScratch(): string[] {
return readdirSync(dir).filter((name) => name !== 'daemon-v1.pid')
}
let warnSpy: ReturnType<typeof vi.spyOn>
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'daemon-pid-atomic-'))
pidPath = join(dir, 'daemon-v1.pid')
fsFaults.tornWriteBytes = null
fsFaults.linkError = null
fsFaults.events = []
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
})
afterEach(() => {
warnSpy.mockRestore()
rmSync(dir, { recursive: true, force: true })
})
@@ -100,6 +104,9 @@ describe('publishDaemonPidFile atomicity', () => {
expect(parseDaemonPidFile(readFileSync(pidPath, 'utf8'))).toMatchObject(RECORD)
expect(listPublishScratch()).toEqual([])
// The atomic path is the normal one; warning on it would train operators to ignore
// the degrade warning below.
expect(warnSpy).not.toHaveBeenCalled()
})
it('flushes the record to disk before it becomes visible at the canonical path', () => {
@@ -132,6 +139,11 @@ describe('publishDaemonPidFile atomicity', () => {
expect(parseDaemonPidFile(readFileSync(pidPath, 'utf8'))).toMatchObject(RECORD)
expect(listPublishScratch()).toEqual([])
// The degrade must be operator-visible: on a volume without hard links every publish
// silently loses torn-write protection otherwise.
expect(warnSpy).toHaveBeenCalledTimes(1)
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('ENOTSUP'))
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('non-atomic'))
})
it('keeps exclusive-create semantics on the degraded path too', () => {
@@ -13,8 +13,10 @@ const PUBLISH_CLAIM_PATTERN = /^daemon-v\d+\.pid\.publish-(\d+)-[0-9a-f-]+$/
/**
* Why proven death and not age: a claim is only scratch — its content was never canonical —
* but deleting a LIVE publisher's claim between its write and its link would fail that
* publish. Only ESRCH proves the owner is gone; EPERM means it exists under another user,
* but deleting a LIVE publisher's claim between its write and its link knocks that publish
* off the atomic path: the link fails non-EEXIST and degrades to the exclusive direct
* write, which succeeds but reopens the torn-write window this discipline exists to close.
* Only ESRCH proves the owner is gone; EPERM means it exists under another user,
* and pid recycling merely preserves junk a little longer, which is the safe direction.
*/
function claimOwnerIsProvenDead(pid: number): boolean {
+13 -1
View File
@@ -160,6 +160,12 @@ function isExistingFileError(error: unknown): boolean {
return typeof error === 'object' && error !== null && 'code' in error && error.code === 'EEXIST'
}
function describeErrnoCode(error: unknown): string {
return typeof error === 'object' && error !== null && 'code' in error
? String(error.code)
: 'no errno'
}
/**
* Scratch for the atomic publish below. Distinct from the swap/hold claims: those can hold
* the ONLY copy of a live record and must be restored, while publish scratch holds a record
@@ -190,7 +196,13 @@ export function publishDaemonPidFile(pidPath: string, pidFile: DaemonPidFile): v
}
// Why: hard links can be unsupported on exotic volumes. The exclusive direct write
// is exactly the pre-atomic behavior, so those filesystems keep a working daemon
// instead of trading a torn-write window for no pid record at all.
// instead of trading a torn-write window for no pid record at all. Warn because the
// degrade is otherwise invisible: an operator on such a volume (network mount,
// container overlay) should be able to tell the atomic path is not in use.
console.warn(
`[daemon] pid-record hard-link publish failed (${describeErrnoCode(error)}); ` +
'degrading to the non-atomic exclusive write — torn-write protection is off for this record'
)
writeFileSync(pidPath, serializeDaemonPidFile(pidFile), { mode: 0o600, flag: 'wx' })
}
} finally {