From 36da195a5c7d9c4d78759989f64fb6bb99b753e3 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 09:08:04 +0900 Subject: [PATCH 1/7] fix(telegram): reap abandoned publication staging temps The notification self-heal reaper never claimed `.tmp` staging files, so every failed publication left one unreachable file in the agent notifications directory permanently. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination. If the staging write or the rename fails, or the process dies between the two, that temp is never published and never read again. No prefix in NOTIFICATION_LEAK_ARTIFACT_PREFIXES matched it, so the reaper walked past it. This accumulates across the roots registry, daemon state, callback aliases, seen-update ids, and the topic registry snapshot, and is most visible where a rename-blocking condition persists (Windows EPERM from an antivirus or indexer handle, EACCES, EIO, ENOSPC). Reaping is shape-matched and still bounded by the existing five-minute mtime grace window, so a temp an in-flight publication is still staging is never removed. Fixing it in the reaper rather than the writer also reclaims temps orphaned by a crash, which no writer-side unwind can reach, and leaves the protected `writeJsonAtomic` lifecycle declaration byte-identical so no DAEMON_GENERATION bump is required. --- packages/coding-agent/CHANGELOG.md | 1 + .../src/sdk/bus/telegram-daemon.ts | 24 +++- ...-telegram-daemon-staging-temp-leak.test.ts | 115 ++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9ad3f1869a..55772afc9c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ - Runtime settings reconciliation now validates every `web_search.fallback` entry against the declared provider enum instead of accepting unsupported or non-string array items (#3601). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now pin `CI_DEV_CHANGED_PATHS` hermetically in their setup/teardown. Their temp checkpoints live inside the enclosing git work tree, so the CI planner's changed paths (which include computer control surface paths on branches that touch them) previously leaked into the computed change set and falsely triggered the mandatory computer red-team suite (`COMPUTER_REDTEAM_CASE_MISSING: … must include kill-switch-bypass`). The production kill-switch-bypass gate is unchanged; only the test fixtures now isolate their own contract from the host branch's diff (#3533). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now relocate temp dirs to `os.tmpdir()` (outside the enclosing git work tree) and pin `CI_DEV_CHANGED_PATHS` to a non-computer test path. The prior in-repo temp dirs caused `computeCheckpointChangeSet` to return `captureIncomplete=true` under parallel shard load (git command timeouts), which unconditionally triggered the mandatory computer red-team suite even when no computer surface was touched. The production kill-switch-bypass gate is unchanged; the `.tmp-*` gitignore entry prevents in-repo test artifacts from polluting untracked-file inventory (#3533). +- The Telegram notification self-heal reaper now reclaims abandoned publication staging files in the agent `notifications/` directory. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination; if the staging write or the rename fails, or the process dies between the two, that temp is never published and never read again. No prefix in the reaper's leak-artifact list claimed `.tmp`, so one unreachable file accumulated per failed attempt — permanently, across the roots registry, daemon state, callback aliases, seen-update ids, and the topic registry snapshot. This is most visible where a rename-blocking condition persists (a Windows `EPERM` from an antivirus or indexer holding a handle, `EACCES`, `EIO`, `ENOSPC`). Reaping is shape-matched and still bounded by the existing five-minute mtime grace window, so a temp that an in-flight publication is still staging is never removed, and reclaiming it here also recovers temps orphaned by a crash, which no writer-side unwind can reach. ### Fixed diff --git a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts index dde4527e79..fc3221973d 100644 --- a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts +++ b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts @@ -1298,6 +1298,28 @@ export function isPermanentMissingPathError(error: unknown): boolean { return code === "ENOENT" || code === "ENOTDIR"; } +/** + * Shape of an abandoned `writeJsonAtomic` staging file: + * `....tmp`. + * + * Publication stages a sibling temp and then renames it over the destination. + * If the staging write or the rename fails, or the process dies between the + * two, that temp is never published and never read again — no prefix in + * {@link NOTIFICATION_LEAK_ARTIFACT_PREFIXES} claimed it, so it accumulated in + * the agent notifications directory permanently, once per failed attempt. + * + * Reaping it here (rather than only unwinding in the writer) also reclaims + * temps orphaned by a crash or power loss, which no writer-side cleanup can + * reach. The caller's mtime grace window is what makes this safe: a temp that + * an in-flight publication is still staging is far younger than the grace, so + * only genuinely abandoned files are removed. + */ +const NOTIFICATION_STAGING_TEMP_PATTERN = /^.+\.\d+\.\d+\.[0-9a-z]+\.tmp$/; + +/** True when `name` is an abandoned publication staging temp. */ +export function isNotificationStagingTempName(name: string): boolean { + return NOTIFICATION_STAGING_TEMP_PATTERN.test(name); +} export function isNotificationLeakArtifactName(name: string): boolean { return NOTIFICATION_LEAK_ARTIFACT_PREFIXES.some(prefix => name.startsWith(prefix)); } @@ -1433,7 +1455,7 @@ export async function reapStaleNotificationArtifacts(input: { throw error; } for (const name of names) { - if (!isNotificationLeakArtifactName(name)) continue; + if (!isNotificationLeakArtifactName(name) && !isNotificationStagingTempName(name)) continue; const file = path.join(paths.dir, name); try { const stat = fsImpl.stat ? await fsImpl.stat(file) : undefined; diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts new file mode 100644 index 0000000000..3b26d5e232 --- /dev/null +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { Settings } from "../src/config/settings"; +import { + daemonPaths, + NOTIFICATION_LEAK_ARTIFACT_GRACE_MS, + reapStaleNotificationArtifacts, + registerNotificationRoot, + type TelegramDaemonFs, +} from "../src/sdk/bus/telegram-daemon"; + +function isolatedSettings(agentDir: string): Settings { + const isolated = Settings.isolated({ + "notifications.enabled": true, + "notifications.telegram.botToken": "123456:secret-token", + "notifications.telegram.chatId": "42", + }) as Settings; + return new Proxy(isolated, { + get(target, prop) { + if (prop === "getAgentDir") return () => agentDir; + const value = Reflect.get(target, prop, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }) as Settings; +} + +/** Wrap the real fs so publishing `roots` fails the way a locked/denied rename does. */ +function renameFailingFs(rootsPath: string): TelegramDaemonFs { + const base = fs.promises as unknown as TelegramDaemonFs; + return { + ...base, + rename: async (oldPath: string, newPath: string): Promise => { + if (path.resolve(newPath) === path.resolve(rootsPath)) { + const error = new Error("EPERM: operation not permitted, rename") as NodeJS.ErrnoException; + error.code = "EPERM"; + throw error; + } + await base.rename(oldPath, newPath); + }, + }; +} + +function stagingTempFiles(dir: string): string[] { + return fs.readdirSync(dir).filter(name => name.endsWith(".tmp")); +} + +/** Drive a publication failure so the writer abandons one staging temp on disk. */ +async function leakOneStagingTemp(agentDir: string, sessionId: string): Promise { + const paths = daemonPaths(agentDir); + await expect( + registerNotificationRoot({ + settings: isolatedSettings(agentDir), + cwd: agentDir, + sessionId, + fs: renameFailingFs(paths.roots), + }), + ).rejects.toThrow(/EPERM/); +} + +test("the notification reaper reclaims staging temps abandoned by a failed publication", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + for (let attempt = 0; attempt < 3; attempt++) await leakOneStagingTemp(agentDir, `session-${attempt}`); + // Precondition: publication really did abandon its staged temps. + expect(stagingTempFiles(paths.dir)).toHaveLength(3); + + // Advance the reaper's clock past the grace window rather than zeroing the + // window: a temp written in the same millisecond can carry a fractional mtime + // slightly ahead of an integer `Date.now()`, which reads as negative age and + // is treated as still-staging. + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + now: () => Date.now() + NOTIFICATION_LEAK_ARTIFACT_GRACE_MS + 60_000, + }); + + expect(stagingTempFiles(paths.dir)).toEqual([]); + expect(result.removed).toHaveLength(3); +}); + +test("the notification reaper leaves a staging temp younger than the grace window alone", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + await leakOneStagingTemp(agentDir, "session-fresh"); + const [fresh] = stagingTempFiles(paths.dir); + expect(fresh).toBeString(); + + // A concurrent publication that is still staging its temp must never have it + // reaped out from under the pending rename. + const result = await reapStaleNotificationArtifacts({ settings: isolatedSettings(agentDir) }); + + expect(stagingTempFiles(paths.dir)).toEqual([fresh!]); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); +}); + +test("the notification reaper never removes a published notification file", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + fs.mkdirSync(paths.dir, { recursive: true }); + // Published names carry no `.tmp` suffix, and a `.json.1.2.abc` shaped name is + // not a staging temp either; neither may be reaped. + fs.writeFileSync(paths.roots, '{"version":1,"roots":[]}\n'); + const decoy = path.join(paths.dir, "telegram-daemon.roots.json.1.2.abc"); + fs.writeFileSync(decoy, "{}\n"); + + const result = await reapStaleNotificationArtifacts({ settings: isolatedSettings(agentDir), graceMs: 0 }); + + expect(fs.existsSync(paths.roots)).toBe(true); + expect(fs.existsSync(decoy)).toBe(true); + expect(result.removed).toEqual([]); +}); From e1856dcdd6eedae9932cbdbeb37a41bd59db4d69 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 18:03:20 +0900 Subject: [PATCH 2/7] fix(telegram): fence staging-temp reaping to dead publishers and exact identity The staging-temp reaper decided purely on filename shape plus a path-following mtime, so a live-but-blocked publisher whose temp aged past the grace window could have its in-flight file deleted, and the readdir/stat -> unlink gap allowed an ABA replacement or a symlink to be destroyed by pathname. Reaping now requires a proven-dead publisher and an identity-bound delete: - The publisher pid is parsed from the writer name shape the writer already produces, and only a probe result of `dead` proceeds. `alive` and any indeterminate or throwing probe retain the file and count as skipped. - Identity is captured no-follow via the existing `readEndpointFile` seam, multi-link files are rejected, and removal goes through `exactUnlink`, which verifies dev+ino+size+mtimeNs+sha256 before unlinking. The grace decision uses the captured `mtimeNs`, so the same inode is bound from decision through deletion. - Missing seams retain rather than fall back to an unfenced unlink, and the staging quarantine name is added to the leak-artifact prefixes so a retained quarantine self-heals instead of becoming a new leak. No protected declaration changes, so no DAEMON_GENERATION bump is required. --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/sdk/bus/telegram-daemon.ts | 154 ++++++++++++-- ...-telegram-daemon-staging-temp-leak.test.ts | 192 +++++++++++++++++- 3 files changed, 330 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 55772afc9c..a0277faf44 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,7 +13,7 @@ - Runtime settings reconciliation now validates every `web_search.fallback` entry against the declared provider enum instead of accepting unsupported or non-string array items (#3601). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now pin `CI_DEV_CHANGED_PATHS` hermetically in their setup/teardown. Their temp checkpoints live inside the enclosing git work tree, so the CI planner's changed paths (which include computer control surface paths on branches that touch them) previously leaked into the computed change set and falsely triggered the mandatory computer red-team suite (`COMPUTER_REDTEAM_CASE_MISSING: … must include kill-switch-bypass`). The production kill-switch-bypass gate is unchanged; only the test fixtures now isolate their own contract from the host branch's diff (#3533). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now relocate temp dirs to `os.tmpdir()` (outside the enclosing git work tree) and pin `CI_DEV_CHANGED_PATHS` to a non-computer test path. The prior in-repo temp dirs caused `computeCheckpointChangeSet` to return `captureIncomplete=true` under parallel shard load (git command timeouts), which unconditionally triggered the mandatory computer red-team suite even when no computer surface was touched. The production kill-switch-bypass gate is unchanged; the `.tmp-*` gitignore entry prevents in-repo test artifacts from polluting untracked-file inventory (#3533). -- The Telegram notification self-heal reaper now reclaims abandoned publication staging files in the agent `notifications/` directory. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination; if the staging write or the rename fails, or the process dies between the two, that temp is never published and never read again. No prefix in the reaper's leak-artifact list claimed `.tmp`, so one unreachable file accumulated per failed attempt — permanently, across the roots registry, daemon state, callback aliases, seen-update ids, and the topic registry snapshot. This is most visible where a rename-blocking condition persists (a Windows `EPERM` from an antivirus or indexer holding a handle, `EACCES`, `EIO`, `ENOSPC`). Reaping is shape-matched and still bounded by the existing five-minute mtime grace window, so a temp that an in-flight publication is still staging is never removed, and reclaiming it here also recovers temps orphaned by a crash, which no writer-side unwind can reach. +- The Telegram notification self-heal reaper now reclaims abandoned publication staging files in the agent `notifications/` directory. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination; if the staging write or the rename fails, or the process dies between the two, that temp is never published and never read again. No prefix in the reaper's leak-artifact list claimed `.tmp`, so one unreachable file accumulated per failed attempt — permanently, across the roots registry, daemon state, callback aliases, seen-update ids, and the topic registry snapshot. This is most visible where a rename-blocking condition persists (a Windows `EPERM` from an antivirus or indexer holding a handle, `EACCES`, `EIO`, `ENOSPC`). Reclaiming it here also recovers temps orphaned by a crash, which no writer-side unwind can reach. Removal is fenced rather than age-only: the reaper parses the publisher PID out of the temp's own name and removes it only when that publisher is *provably dead*, so a live or slow publication keeps its staged temp however old it is, and an indeterminate liveness probe or an unparseable claim retains the file. A proven-dead temp is still bounded by the existing five-minute mtime grace window, and the deletion itself is bound to a no-follow identity capture (`dev`+`ino`+`size`+`mtime`+content digest, single-link regular files only) executed through the exact-unlink native, so a symlink is never followed and a temp replaced between capture and delete is refused instead of destroying the successor. ### Fixed diff --git a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts index fc3221973d..7c31bed4e6 100644 --- a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts +++ b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts @@ -160,11 +160,17 @@ export interface TelegramDaemonFs { size?: number; dev?: number; ino?: number; + /** Hard-link count; required to prove a staging temp has no second name. */ + nlink?: number; ctimeMs?: number; isDirectory?: () => boolean; }>; readEndpointFile?(path: string): Promise; - exactUnlink?(path: string, identity: NotificationEndpointFileIdentity): Promise; + exactUnlink?( + path: string, + identity: NotificationEndpointFileIdentity, + quarantineName?: string, + ): Promise; } export interface SpawnResult { @@ -228,8 +234,12 @@ function negotiateToolActivityCapability( const nodeFs: TelegramDaemonFs = { ...(fs.promises as unknown as TelegramDaemonFs), readEndpointFile: readNotificationEndpointFile, - exactUnlink: async (file, identity) => - exactUnlinkNotificationFile(file, identity, `.gjc-delete-daemon-transition-${crypto.randomUUID()}.json`), + exactUnlink: async (file, identity, quarantineName) => + exactUnlinkNotificationFile( + file, + identity, + quarantineName ?? `.gjc-delete-daemon-transition-${crypto.randomUUID()}.json`, + ), }; /** @@ -935,8 +945,9 @@ async function exactUnlinkAcceptedWithRetainedEvidence( fsImpl: TelegramDaemonFs, file: string, identity: NotificationEndpointFileIdentity, + quarantineName?: string, ): Promise { - const removed = await fsImpl.exactUnlink!(file, identity); + const removed = await fsImpl.exactUnlink!(file, identity, quarantineName); if (removed.ok) return true; return ( removed.code === "cleanup_pending" && @@ -1287,6 +1298,7 @@ export const NOTIFICATION_LEAK_ARTIFACT_PREFIXES = [ ".gjc-delete-daemon-transition-", ".gjc-exact-unlink-placeholder-", ".gjc-delete-notification-endpoint-", + ".gjc-delete-notification-staging-temp-", ] as const; /** Grace window before a leak artifact is reaped (covers in-flight unlinks). */ @@ -1310,15 +1322,67 @@ export function isPermanentMissingPathError(error: unknown): boolean { * * Reaping it here (rather than only unwinding in the writer) also reclaims * temps orphaned by a crash or power loss, which no writer-side cleanup can - * reach. The caller's mtime grace window is what makes this safe: a temp that - * an in-flight publication is still staging is far younger than the grace, so - * only genuinely abandoned files are removed. + * reach. Age alone is not proof of abandonment, though: a slow or blocked + * publisher (a stalled network write, a rename fenced by an antivirus handle) + * can hold a live staged temp well past any grace window. The staged name + * therefore carries its publisher's PID, and the reaper only removes a temp + * whose publisher is provably dead — see {@link parseNotificationStagingTemp}. */ -const NOTIFICATION_STAGING_TEMP_PATTERN = /^.+\.\d+\.\d+\.[0-9a-z]+\.tmp$/; +const NOTIFICATION_STAGING_TEMP_PATTERN = /^(?.+)\.(?\d+)\.(?\d+)\.[0-9a-z]+\.tmp$/; -/** True when `name` is an abandoned publication staging temp. */ -export function isNotificationStagingTempName(name: string): boolean { - return NOTIFICATION_STAGING_TEMP_PATTERN.test(name); +/** A publication staging claim recovered from an abandoned temp's name. */ +export interface NotificationStagingTempClaim { + /** Published sibling this temp was staged for. */ + destination: string; + /** PID of the process that staged it. */ + pid: number; + /** Wall-clock ms the publisher recorded when it staged the temp. */ + stagedAtMs: number; +} + +/** + * Recover the publication claim encoded in a staging temp's name, or + * `undefined` when `name` is not a staging temp or its PID/timestamp fields are + * not usable integers. An unparseable claim is never reaped. + */ +export function parseNotificationStagingTemp(name: string): NotificationStagingTempClaim | undefined { + const groups = NOTIFICATION_STAGING_TEMP_PATTERN.exec(name)?.groups; + if (!groups) return undefined; + const pid = Number(groups.pid); + const stagedAtMs = Number(groups.stagedAt); + if (!validDaemonPid(pid) || !Number.isSafeInteger(stagedAtMs) || stagedAtMs < 0) return undefined; + return { destination: groups.destination as string, pid, stagedAtMs }; +} + +/** Liveness verdict for a staging temp's publisher; `unknown` fails closed. */ +type NotificationPublisherLiveness = "alive" | "dead" | "unknown"; + +/** + * Classify a staging temp publisher against the daemon's liveness seam. A + * throwing probe (an unreadable process table, a denied query) is + * indeterminate, not dead, so the temp is retained. + */ +function classifyNotificationStagingPublisher( + claim: NotificationStagingTempClaim, + pidAlive: (pid: number) => boolean, +): NotificationPublisherLiveness { + try { + return pidAlive(claim.pid) ? "alive" : "dead"; + } catch { + return "unknown"; + } +} + +/** + * True when `file` is a regular file reachable under exactly one name. A + * multi-link file shares its inode with another pathname, so unlinking this one + * would not reclaim the data and may be another owner's live hardlink. Fails + * closed when the `stat` seam cannot report a link count. + */ +async function isSingleLinkRegularFile(fsImpl: TelegramDaemonFs, file: string): Promise { + if (!fsImpl.stat) return false; + const stat = await fsImpl.stat(file); + return stat.nlink === 1; } export function isNotificationLeakArtifactName(name: string): boolean { return NOTIFICATION_LEAK_ARTIFACT_PREFIXES.some(prefix => name.startsWith(prefix)); @@ -1430,21 +1494,68 @@ export async function pruneMissingNotificationRoots(input: { return { pruned, remaining }; } +/** + * Remove one abandoned publication staging temp under a liveness fence and an + * identity-bound delete. Returns whether the temp was removed; `false` means it + * was deliberately retained (live/indeterminate publisher, not a single-link + * regular file, still inside the grace window, or an identity change between + * capture and delete). Filesystem faults propagate to the caller's best-effort + * handler. + */ +async function reapAbandonedNotificationStagingTemp(input: { + fs: TelegramDaemonFs; + file: string; + claim: NotificationStagingTempClaim; + now: number; + graceMs: number; + pidAlive: (pid: number) => boolean; +}): Promise { + // A live or blocked publisher can hold a staged temp far past any grace + // window; only a provably dead publisher's claim is abandoned. `unknown` + // (throwing probe) fails closed. + if (classifyNotificationStagingPublisher(input.claim, input.pidAlive) !== "dead") return false; + const readEndpointFile = input.fs.readEndpointFile; + // Both seams are optional; without them there is no no-follow capture and no + // identity-bound delete, so retain rather than unlink unfenced. + if (!readEndpointFile || !input.fs.exactUnlink) return false; + // No-follow capture: rejects symlinks, directories, and anything that changes + // while it is read. A reparse point or dangling link is therefore retained. + const endpoint = await readEndpointFile(input.file); + if (!(await isSingleLinkRegularFile(input.fs, input.file))) return false; + // Age from the captured (no-follow) mtime rather than a second path-following + // stat, so the grace decision and the delete bind the same inode. Integer ns + // truncation only ever ages the file, never rejuvenates it. + const age = input.now - Number(endpoint.identity.mtimeNs / 1_000_000n); + if (age < input.graceMs) return false; + // The native verifies dev+ino+size+mtimeNs+sha256 before unlinking, so a + // readdir/capture -> replacement ABA cannot delete the fresh generation. + return await exactUnlinkAcceptedWithRetainedEvidence( + input.fs, + input.file, + endpoint.identity, + `.gjc-delete-notification-staging-temp-${crypto.randomUUID()}.json`, + ); +} + /** * Reap retained exact-unlink / ownership-transition quarantine files older than - * the grace window from the notifications directory. + * the grace window from the notifications directory, plus publication staging + * temps whose publisher is provably dead. */ export async function reapStaleNotificationArtifacts(input: { settings: Settings; fs?: TelegramDaemonFs; now?: () => number; graceMs?: number; + /** Liveness seam used to prove a staging temp's publisher is dead. */ + pidAlive?: (pid: number) => boolean; }): Promise<{ removed: string[]; skipped: number }> { const fsImpl = input.fs ?? nodeFs; const paths = daemonPaths(input.settings.getAgentDir()); await ensureDir(fsImpl, paths.dir); const now = input.now?.() ?? Date.now(); const graceMs = input.graceMs ?? NOTIFICATION_LEAK_ARTIFACT_GRACE_MS; + const pidAlive = input.pidAlive ?? defaultPidAlive; const removed: string[] = []; let skipped = 0; let names: string[]; @@ -1455,9 +1566,23 @@ export async function reapStaleNotificationArtifacts(input: { throw error; } for (const name of names) { - if (!isNotificationLeakArtifactName(name) && !isNotificationStagingTempName(name)) continue; + const stagingTemp = NOTIFICATION_STAGING_TEMP_PATTERN.test(name); + if (!isNotificationLeakArtifactName(name) && !stagingTemp) continue; const file = path.join(paths.dir, name); try { + if (stagingTemp) { + const claim = parseNotificationStagingTemp(name); + // A staging-temp shape whose PID/timestamp will not parse carries no + // provable claim, so it is retained. + if (!claim) { + skipped += 1; + continue; + } + if (await reapAbandonedNotificationStagingTemp({ fs: fsImpl, file, claim, now, graceMs, pidAlive })) + removed.push(file); + else skipped += 1; + continue; + } const stat = fsImpl.stat ? await fsImpl.stat(file) : undefined; const age = stat ? now - stat.mtimeMs : Number.POSITIVE_INFINITY; if (Number.isFinite(age) && age < graceMs) { @@ -1484,6 +1609,8 @@ export async function healTelegramDaemonNotificationState(input: { fs?: TelegramDaemonFs; now?: () => number; graceMs?: number; + /** Liveness seam used to prove a staging temp's publisher is dead. */ + pidAlive?: (pid: number) => boolean; }): Promise<{ prunedRoots: string[]; removedArtifacts: string[] }> { const prune = await pruneMissingNotificationRoots(input); const reap = await reapStaleNotificationArtifacts(input); @@ -5285,6 +5412,7 @@ export class TelegramNotificationDaemon { settings: this.opts.settings, fs: this.fsImpl, now: this.opts.now, + pidAlive: this.opts.pidAlive, }); } catch (error) { logger.warn(`notifications: leak-artifact reap failed: ${sanitizeDiagnostic(String(error))}`); diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts index 3b26d5e232..16d4e55d5b 100644 --- a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { Settings } from "../src/config/settings"; +import { exactUnlinkNotificationFile, readNotificationEndpointFile } from "../src/sdk/bus/notification-service"; import { daemonPaths, NOTIFICATION_LEAK_ARTIFACT_GRACE_MS, @@ -42,10 +43,32 @@ function renameFailingFs(rootsPath: string): TelegramDaemonFs { }; } +/** + * The production seam set: real filesystem plus the no-follow capture and + * identity-bound delete the reaper fences its removals with. + */ +function identityFencedFs(): TelegramDaemonFs { + return { + ...(fs.promises as unknown as TelegramDaemonFs), + readEndpointFile: readNotificationEndpointFile, + exactUnlink: async (file, identity, quarantineName) => + exactUnlinkNotificationFile( + file, + identity, + quarantineName ?? ".gjc-delete-notification-staging-temp-test.json", + ), + }; +} + function stagingTempFiles(dir: string): string[] { return fs.readdirSync(dir).filter(name => name.endsWith(".tmp")); } +/** Reaper clock far enough ahead that the mtime grace window cannot retain a temp. */ +function pastGraceWindow(): number { + return Date.now() + NOTIFICATION_LEAK_ARTIFACT_GRACE_MS + 60_000; +} + /** Drive a publication failure so the writer abandons one staging temp on disk. */ async function leakOneStagingTemp(agentDir: string, sessionId: string): Promise { const paths = daemonPaths(agentDir); @@ -59,6 +82,12 @@ async function leakOneStagingTemp(agentDir: string, sessionId: string): Promise< ).rejects.toThrow(/EPERM/); } +function agentDirWithNotifications(): string { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + fs.mkdirSync(daemonPaths(agentDir).dir, { recursive: true }); + return agentDir; +} + test("the notification reaper reclaims staging temps abandoned by a failed publication", async () => { const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); const paths = daemonPaths(agentDir); @@ -67,13 +96,17 @@ test("the notification reaper reclaims staging temps abandoned by a failed publi // Precondition: publication really did abandon its staged temps. expect(stagingTempFiles(paths.dir)).toHaveLength(3); + // The temps name this very test process as publisher, so abandonment is only + // provable once liveness reports that publisher dead. + // // Advance the reaper's clock past the grace window rather than zeroing the // window: a temp written in the same millisecond can carry a fractional mtime // slightly ahead of an integer `Date.now()`, which reads as negative age and // is treated as still-staging. const result = await reapStaleNotificationArtifacts({ settings: isolatedSettings(agentDir), - now: () => Date.now() + NOTIFICATION_LEAK_ARTIFACT_GRACE_MS + 60_000, + now: pastGraceWindow, + pidAlive: () => false, }); expect(stagingTempFiles(paths.dir)).toEqual([]); @@ -89,8 +122,12 @@ test("the notification reaper leaves a staging temp younger than the grace windo expect(fresh).toBeString(); // A concurrent publication that is still staging its temp must never have it - // reaped out from under the pending rename. - const result = await reapStaleNotificationArtifacts({ settings: isolatedSettings(agentDir) }); + // reaped out from under the pending rename. Liveness is forced dead so the + // retention proves the grace window, not the liveness fence. + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + pidAlive: () => false, + }); expect(stagingTempFiles(paths.dir)).toEqual([fresh!]); expect(result.removed).toEqual([]); @@ -107,9 +144,156 @@ test("the notification reaper never removes a published notification file", asyn const decoy = path.join(paths.dir, "telegram-daemon.roots.json.1.2.abc"); fs.writeFileSync(decoy, "{}\n"); - const result = await reapStaleNotificationArtifacts({ settings: isolatedSettings(agentDir), graceMs: 0 }); + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + graceMs: 0, + pidAlive: () => false, + }); expect(fs.existsSync(paths.roots)).toBe(true); expect(fs.existsSync(decoy)).toBe(true); expect(result.removed).toEqual([]); }); + +test("a staging temp whose publisher is still alive is never reaped, however old it is", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + await leakOneStagingTemp(agentDir, "session-live"); + const [staged] = stagingTempFiles(paths.dir); + expect(staged).toBeString(); + // The abandoned temp names this test process; the default liveness probe sees + // it running, which is exactly the live-publisher case age alone misreads. + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + now: pastGraceWindow, + graceMs: 0, + }); + + expect(stagingTempFiles(paths.dir)).toEqual([staged!]); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); +}); + +test("a staging temp is retained when publisher liveness is indeterminate", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + await leakOneStagingTemp(agentDir, "session-unknown"); + const [staged] = stagingTempFiles(paths.dir); + expect(staged).toBeString(); + + // A probe that cannot answer (permission denied, unsupported platform) must + // fail closed rather than degrade to age-only deletion. + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => { + throw new Error("EPERM: liveness probe denied"); + }, + }); + + expect(stagingTempFiles(paths.dir)).toEqual([staged!]); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); +}); + +test("a staging temp with an unparseable publisher claim is retained", async () => { + const agentDir = agentDirWithNotifications(); + const paths = daemonPaths(agentDir); + // Staging-temp shape, but pid 0 is not a valid publisher, so no claim can be + // proven dead and the temp must survive. + const malformed = path.join(paths.dir, "telegram-daemon.roots.json.0.1700000000000.abc123.tmp"); + fs.writeFileSync(malformed, "{}\n"); + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + + expect(fs.existsSync(malformed)).toBe(true); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); +}); + +test("a dead publisher's staging temp past the grace window is reaped through the identity fence", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + await leakOneStagingTemp(agentDir, "session-dead"); + const [staged] = stagingTempFiles(paths.dir); + expect(staged).toBeString(); + const file = path.join(paths.dir, staged!); + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: identityFencedFs(), + now: pastGraceWindow, + pidAlive: () => false, + }); + + expect(fs.existsSync(file)).toBe(false); + expect(result.removed).toEqual([file]); +}); + +test("a staging temp replaced between identity capture and delete is not removed", async () => { + const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); + const paths = daemonPaths(agentDir); + + await leakOneStagingTemp(agentDir, "session-aba"); + const [staged] = stagingTempFiles(paths.dir); + expect(staged).toBeString(); + const file = path.join(paths.dir, staged!); + + const base = identityFencedFs(); + // ABA: the name is rewritten after the reaper captured its identity, so the + // delete must bind the captured inode contents and refuse the successor. + const racingFs: TelegramDaemonFs = { + ...base, + readEndpointFile: async target => { + const endpoint = await base.readEndpointFile!(target); + if (target === file) fs.writeFileSync(file, "successor-staged-after-capture\n"); + return endpoint; + }, + }; + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: racingFs, + now: pastGraceWindow, + pidAlive: () => false, + }); + + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); + expect(fs.existsSync(file)).toBe(true); + expect(fs.readFileSync(file, "utf8")).toBe("successor-staged-after-capture\n"); +}); + +test("a symlink shaped like a staging temp is never followed or deleted", async () => { + const agentDir = agentDirWithNotifications(); + const paths = daemonPaths(agentDir); + const victim = path.join(agentDir, "victim.json"); + fs.writeFileSync(victim, '{"keep":true}\n'); + const link = path.join(paths.dir, `telegram-daemon.roots.json.${process.pid + 1}.1700000000000.abc123.tmp`); + fs.symlinkSync(victim, link); + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: identityFencedFs(), + now: pastGraceWindow, + pidAlive: () => false, + }); + + // The no-follow capture rejects the link, so neither the link nor its target + // is unlinked. + expect(fs.existsSync(victim)).toBe(true); + expect(fs.readFileSync(victim, "utf8")).toBe('{"keep":true}\n'); + expect(fs.lstatSync(link).isSymbolicLink()).toBe(true); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); +}); From 9b7c29d676863b89b0e15599ea089733c0472126 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 18:51:48 +0900 Subject: [PATCH 3/7] fix(telegram): reap retained staging quarantine exactly --- .../src/sdk/bus/telegram-daemon.ts | 30 ++++++++++++++----- ...-telegram-daemon-staging-temp-leak.test.ts | 28 +++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts index 7c31bed4e6..9e748838b8 100644 --- a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts +++ b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts @@ -1537,6 +1537,26 @@ async function reapAbandonedNotificationStagingTemp(input: { ); } +/** Reap one retained delete quarantine without following or unlinking a successor. */ +async function reapNotificationLeakArtifact(input: { + fs: TelegramDaemonFs; + file: string; + now: number; + graceMs: number; +}): Promise { + if (!input.fs.readEndpointFile || !input.fs.exactUnlink) return false; + const endpoint = await input.fs.readEndpointFile(input.file); + if (!(await isSingleLinkRegularFile(input.fs, input.file))) return false; + const age = input.now - Number(endpoint.identity.mtimeNs / 1_000_000n); + if (age < input.graceMs) return false; + return await exactUnlinkAcceptedWithRetainedEvidence( + input.fs, + input.file, + endpoint.identity, + `.gjc-exact-unlink-placeholder-${crypto.randomUUID()}.json`, + ); +} + /** * Reap retained exact-unlink / ownership-transition quarantine files older than * the grace window from the notifications directory, plus publication staging @@ -1583,14 +1603,8 @@ export async function reapStaleNotificationArtifacts(input: { else skipped += 1; continue; } - const stat = fsImpl.stat ? await fsImpl.stat(file) : undefined; - const age = stat ? now - stat.mtimeMs : Number.POSITIVE_INFINITY; - if (Number.isFinite(age) && age < graceMs) { - skipped += 1; - continue; - } - await fsImpl.unlink(file); - removed.push(file); + if (await reapNotificationLeakArtifact({ fs: fsImpl, file, now, graceMs })) removed.push(file); + else skipped += 1; } catch (error) { if (isPermanentMissingPathError(error)) continue; // Best-effort: a busy file must not fail daemon ownership. diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts index 16d4e55d5b..4cffb5a334 100644 --- a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -273,6 +273,34 @@ test("a staging temp replaced between identity capture and delete is not removed expect(fs.existsSync(file)).toBe(true); expect(fs.readFileSync(file, "utf8")).toBe("successor-staged-after-capture\n"); }); +test("a retained staging quarantine replacement is not removed by the generic reaper", async () => { + const agentDir = agentDirWithNotifications(); + const paths = daemonPaths(agentDir); + const file = path.join(paths.dir, ".gjc-delete-notification-staging-temp-retained.json"); + fs.writeFileSync(file, "retained-original\n"); + + const base = identityFencedFs(); + const racingFs: TelegramDaemonFs = { + ...base, + readEndpointFile: async target => { + const endpoint = await base.readEndpointFile!(target); + if (target === file) fs.writeFileSync(file, "retained-successor\n"); + return endpoint; + }, + }; + + const result = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: racingFs, + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); + expect(fs.readFileSync(file, "utf8")).toBe("retained-successor\n"); +}); test("a symlink shaped like a staging temp is never followed or deleted", async () => { const agentDir = agentDirWithNotifications(); From bcb2a1e570b4246c51719665c8459b5a68e04f1a Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 20:40:35 +0900 Subject: [PATCH 4/7] test(telegram): prove retained quarantine self-heal --- ...-telegram-daemon-staging-temp-leak.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts index 4cffb5a334..703dae0a42 100644 --- a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -302,6 +302,50 @@ test("a retained staging quarantine replacement is not removed by the generic re expect(fs.readFileSync(file, "utf8")).toBe("retained-successor\n"); }); +test("a retained quarantine cleanup_pending successor stays visible and self-heals on the next scan", async () => { + const agentDir = agentDirWithNotifications(); + const paths = daemonPaths(agentDir); + const file = path.join(paths.dir, ".gjc-delete-notification-staging-temp-retained.json"); + fs.writeFileSync(file, "retained-original\n"); + + const base = identityFencedFs(); + let detachedPath: string | undefined; + const retainingFs: TelegramDaemonFs = { + ...base, + exactUnlink: async (target, _identity, quarantineName) => { + if (!quarantineName) throw new Error("expected a retained quarantine name"); + detachedPath = path.join(path.dirname(target), quarantineName); + fs.renameSync(target, detachedPath); + return { ok: false, code: "cleanup_pending", detachedPath }; + }, + }; + + const retained = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: retainingFs, + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + + expect(retained.removed).toEqual([file]); + expect(fs.existsSync(file)).toBe(false); + expect(detachedPath).toBeString(); + expect(path.basename(detachedPath!)).toStartWith(".gjc-exact-unlink-placeholder-"); + expect(fs.readFileSync(detachedPath!, "utf8")).toBe("retained-original\n"); + + const healed = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: base, + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + + expect(healed.removed).toEqual([detachedPath!]); + expect(fs.existsSync(detachedPath!)).toBe(false); +}); + test("a symlink shaped like a staging temp is never followed or deleted", async () => { const agentDir = agentDirWithNotifications(); const paths = daemonPaths(agentDir); From 3a0f481f22df0c46f51937a9da77e5507ab91065 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 20:47:56 +0900 Subject: [PATCH 5/7] fix(telegram): report retained cleanup truthfully --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/sdk/bus/telegram-daemon.ts | 58 ++++++++++++++----- ...-telegram-daemon-staging-temp-leak.test.ts | 17 ++++-- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a0277faf44..17ec7e71a1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,7 +13,7 @@ - Runtime settings reconciliation now validates every `web_search.fallback` entry against the declared provider enum instead of accepting unsupported or non-string array items (#3601). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now pin `CI_DEV_CHANGED_PATHS` hermetically in their setup/teardown. Their temp checkpoints live inside the enclosing git work tree, so the CI planner's changed paths (which include computer control surface paths on branches that touch them) previously leaked into the computed change set and falsely triggered the mandatory computer red-team suite (`COMPUTER_REDTEAM_CASE_MISSING: … must include kill-switch-bypass`). The production kill-switch-bypass gate is unchanged; only the test fixtures now isolate their own contract from the host branch's diff (#3533). - Ultragoal critic-gate, dogfood, review, durable-completion, and runtime test suites now relocate temp dirs to `os.tmpdir()` (outside the enclosing git work tree) and pin `CI_DEV_CHANGED_PATHS` to a non-computer test path. The prior in-repo temp dirs caused `computeCheckpointChangeSet` to return `captureIncomplete=true` under parallel shard load (git command timeouts), which unconditionally triggered the mandatory computer red-team suite even when no computer surface was touched. The production kill-switch-bypass gate is unchanged; the `.tmp-*` gitignore entry prevents in-repo test artifacts from polluting untracked-file inventory (#3533). -- The Telegram notification self-heal reaper now reclaims abandoned publication staging files in the agent `notifications/` directory. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination; if the staging write or the rename fails, or the process dies between the two, that temp is never published and never read again. No prefix in the reaper's leak-artifact list claimed `.tmp`, so one unreachable file accumulated per failed attempt — permanently, across the roots registry, daemon state, callback aliases, seen-update ids, and the topic registry snapshot. This is most visible where a rename-blocking condition persists (a Windows `EPERM` from an antivirus or indexer holding a handle, `EACCES`, `EIO`, `ENOSPC`). Reclaiming it here also recovers temps orphaned by a crash, which no writer-side unwind can reach. Removal is fenced rather than age-only: the reaper parses the publisher PID out of the temp's own name and removes it only when that publisher is *provably dead*, so a live or slow publication keeps its staged temp however old it is, and an indeterminate liveness probe or an unparseable claim retains the file. A proven-dead temp is still bounded by the existing five-minute mtime grace window, and the deletion itself is bound to a no-follow identity capture (`dev`+`ino`+`size`+`mtime`+content digest, single-link regular files only) executed through the exact-unlink native, so a symlink is never followed and a temp replaced between capture and delete is refused instead of destroying the successor. +- The Telegram notification self-heal reaper now handles abandoned publication staging files in the agent `notifications/` directory. `writeJsonAtomic` stages a sibling `....tmp` and renames it over the destination; a failed staging write or rename, or a process death between those steps, can leave an unreachable file. The reaper parses the publisher PID from the temp name and acts only when that publisher is provably dead and the existing five-minute mtime grace window has elapsed. Deletion is bound to a no-follow identity capture (`dev`+`ino`+`size`+`mtime`+content digest, single-link regular files only) through the exact-unlink native, so symlinks and same-name replacements are retained. A terminal native removal is reported as reclaimed; a typed `cleanup_pending` result is instead reported as skipped, leaves the bytes visible under a recognized exact-unlink placeholder, and later scans preserve that placeholder without pathname churn rather than claiming false removal. ### Fixed diff --git a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts index 9e748838b8..7c3c551e07 100644 --- a/packages/coding-agent/src/sdk/bus/telegram-daemon.ts +++ b/packages/coding-agent/src/sdk/bus/telegram-daemon.ts @@ -957,6 +957,27 @@ async function exactUnlinkAcceptedWithRetainedEvidence( ); } +type NotificationArtifactReapOutcome = "removed" | "retained" | "unchanged"; + +/** Distinguish terminal removal from a safely detached artifact that still needs authority-preserving cleanup. */ +async function reapNotificationArtifactExactly( + fsImpl: TelegramDaemonFs, + file: string, + identity: NotificationEndpointFileIdentity, + quarantineName: string, +): Promise { + const removed = await fsImpl.exactUnlink!(file, identity, quarantineName); + if (removed.ok) return "removed"; + if ( + removed.code === "cleanup_pending" && + typeof removed.detachedPath === "string" && + removed.detachedPath.length > 0 && + (await fsImpl.readEndpointFile!(file).catch(() => undefined)) === undefined + ) + return "retained"; + return "unchanged"; +} + async function unlinkOwnershipLockExactly( fsImpl: TelegramDaemonFs, file: string, @@ -1509,27 +1530,27 @@ async function reapAbandonedNotificationStagingTemp(input: { now: number; graceMs: number; pidAlive: (pid: number) => boolean; -}): Promise { +}): Promise { // A live or blocked publisher can hold a staged temp far past any grace // window; only a provably dead publisher's claim is abandoned. `unknown` // (throwing probe) fails closed. - if (classifyNotificationStagingPublisher(input.claim, input.pidAlive) !== "dead") return false; + if (classifyNotificationStagingPublisher(input.claim, input.pidAlive) !== "dead") return "unchanged"; const readEndpointFile = input.fs.readEndpointFile; // Both seams are optional; without them there is no no-follow capture and no // identity-bound delete, so retain rather than unlink unfenced. - if (!readEndpointFile || !input.fs.exactUnlink) return false; + if (!readEndpointFile || !input.fs.exactUnlink) return "unchanged"; // No-follow capture: rejects symlinks, directories, and anything that changes // while it is read. A reparse point or dangling link is therefore retained. const endpoint = await readEndpointFile(input.file); - if (!(await isSingleLinkRegularFile(input.fs, input.file))) return false; + if (!(await isSingleLinkRegularFile(input.fs, input.file))) return "unchanged"; // Age from the captured (no-follow) mtime rather than a second path-following // stat, so the grace decision and the delete bind the same inode. Integer ns // truncation only ever ages the file, never rejuvenates it. const age = input.now - Number(endpoint.identity.mtimeNs / 1_000_000n); - if (age < input.graceMs) return false; + if (age < input.graceMs) return "unchanged"; // The native verifies dev+ino+size+mtimeNs+sha256 before unlinking, so a // readdir/capture -> replacement ABA cannot delete the fresh generation. - return await exactUnlinkAcceptedWithRetainedEvidence( + return await reapNotificationArtifactExactly( input.fs, input.file, endpoint.identity, @@ -1543,13 +1564,14 @@ async function reapNotificationLeakArtifact(input: { file: string; now: number; graceMs: number; -}): Promise { - if (!input.fs.readEndpointFile || !input.fs.exactUnlink) return false; +}): Promise { + if (path.basename(input.file).startsWith(".gjc-exact-unlink-placeholder-")) return "retained"; + if (!input.fs.readEndpointFile || !input.fs.exactUnlink) return "unchanged"; const endpoint = await input.fs.readEndpointFile(input.file); - if (!(await isSingleLinkRegularFile(input.fs, input.file))) return false; + if (!(await isSingleLinkRegularFile(input.fs, input.file))) return "unchanged"; const age = input.now - Number(endpoint.identity.mtimeNs / 1_000_000n); - if (age < input.graceMs) return false; - return await exactUnlinkAcceptedWithRetainedEvidence( + if (age < input.graceMs) return "unchanged"; + return await reapNotificationArtifactExactly( input.fs, input.file, endpoint.identity, @@ -1598,12 +1620,20 @@ export async function reapStaleNotificationArtifacts(input: { skipped += 1; continue; } - if (await reapAbandonedNotificationStagingTemp({ fs: fsImpl, file, claim, now, graceMs, pidAlive })) - removed.push(file); + const outcome = await reapAbandonedNotificationStagingTemp({ + fs: fsImpl, + file, + claim, + now, + graceMs, + pidAlive, + }); + if (outcome === "removed") removed.push(file); else skipped += 1; continue; } - if (await reapNotificationLeakArtifact({ fs: fsImpl, file, now, graceMs })) removed.push(file); + const outcome = await reapNotificationLeakArtifact({ fs: fsImpl, file, now, graceMs }); + if (outcome === "removed") removed.push(file); else skipped += 1; } catch (error) { if (isPermanentMissingPathError(error)) continue; diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts index 703dae0a42..d70a14124a 100644 --- a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -302,7 +302,7 @@ test("a retained staging quarantine replacement is not removed by the generic re expect(fs.readFileSync(file, "utf8")).toBe("retained-successor\n"); }); -test("a retained quarantine cleanup_pending successor stays visible and self-heals on the next scan", async () => { +test("a retained cleanup_pending successor stays surfaced without pathname churn", async () => { const agentDir = agentDirWithNotifications(); const paths = daemonPaths(agentDir); const file = path.join(paths.dir, ".gjc-delete-notification-staging-temp-retained.json"); @@ -328,7 +328,8 @@ test("a retained quarantine cleanup_pending successor stays visible and self-hea pidAlive: () => false, }); - expect(retained.removed).toEqual([file]); + expect(retained.removed).toEqual([]); + expect(retained.skipped).toBeGreaterThan(0); expect(fs.existsSync(file)).toBe(false); expect(detachedPath).toBeString(); expect(path.basename(detachedPath!)).toStartWith(".gjc-exact-unlink-placeholder-"); @@ -342,8 +343,16 @@ test("a retained quarantine cleanup_pending successor stays visible and self-hea pidAlive: () => false, }); - expect(healed.removed).toEqual([detachedPath!]); - expect(fs.existsSync(detachedPath!)).toBe(false); + expect(healed.removed).toEqual([]); + expect(healed.skipped).toBeGreaterThan(0); + expect(fs.existsSync(detachedPath!)).toBe(true); + expect(fs.readFileSync(detachedPath!, "utf8")).toBe("retained-original\n"); + expect( + fs + .readdirSync(paths.dir) + .filter(name => name.startsWith(".gjc-exact-unlink-placeholder-")) + .map(name => path.join(paths.dir, name)), + ).toEqual([detachedPath!]); }); test("a symlink shaped like a staging temp is never followed or deleted", async () => { From 281cb54c4580c8b51fe82663c61bd9f04d26fc61 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 20:56:36 +0900 Subject: [PATCH 6/7] test(telegram): assert source-native retained outcomes --- ...-telegram-daemon-staging-temp-leak.test.ts | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts index d70a14124a..84f0d49445 100644 --- a/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts +++ b/packages/coding-agent/test/notifications-telegram-daemon-staging-temp-leak.test.ts @@ -88,7 +88,7 @@ function agentDirWithNotifications(): string { return agentDir; } -test("the notification reaper reclaims staging temps abandoned by a failed publication", async () => { +test("the notification reaper detaches dead-publisher staging temps without claiming terminal removal", async () => { const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); const paths = daemonPaths(agentDir); @@ -110,7 +110,11 @@ test("the notification reaper reclaims staging temps abandoned by a failed publi }); expect(stagingTempFiles(paths.dir)).toEqual([]); - expect(result.removed).toHaveLength(3); + expect(result.removed).toEqual([]); + expect(result.skipped).toBe(3); + expect( + fs.readdirSync(paths.dir).filter(name => name.startsWith(".gjc-delete-notification-staging-temp-")), + ).toHaveLength(3); }); test("the notification reaper leaves a staging temp younger than the grace window alone", async () => { @@ -220,7 +224,7 @@ test("a staging temp with an unparseable publisher claim is retained", async () expect(result.skipped).toBeGreaterThan(0); }); -test("a dead publisher's staging temp past the grace window is reaped through the identity fence", async () => { +test("a dead publisher's staging temp stays retained under stable exact authority on POSIX", async () => { const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "gjc-telegram-staging-leak-")); const paths = daemonPaths(agentDir); @@ -237,7 +241,50 @@ test("a dead publisher's staging temp past the grace window is reaped through th }); expect(fs.existsSync(file)).toBe(false); - expect(result.removed).toEqual([file]); + expect(result.removed).toEqual([]); + expect(result.skipped).toBeGreaterThan(0); + const [detachedName] = fs + .readdirSync(paths.dir) + .filter(name => name.startsWith(".gjc-delete-notification-staging-temp-")); + expect(detachedName).toBeString(); + const detachedPath = path.join(paths.dir, detachedName!); + const retainedBytes = fs.readFileSync(detachedPath); + + const normalized = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: identityFencedFs(), + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + expect(normalized.removed).toEqual([]); + expect(normalized.skipped).toBeGreaterThan(0); + expect(fs.existsSync(detachedPath)).toBe(false); + const placeholderNames = fs + .readdirSync(paths.dir) + .filter(name => name.startsWith(".gjc-exact-unlink-placeholder-")) + .sort(); + const payloadPlaceholderName = placeholderNames.find(name => name.endsWith(".json")); + expect(payloadPlaceholderName).toBeString(); + const payloadPlaceholderPath = path.join(paths.dir, payloadPlaceholderName!); + expect(fs.readFileSync(payloadPlaceholderPath)).toEqual(retainedBytes); + + const stable = await reapStaleNotificationArtifacts({ + settings: isolatedSettings(agentDir), + fs: identityFencedFs(), + now: pastGraceWindow, + graceMs: 0, + pidAlive: () => false, + }); + expect(stable.removed).toEqual([]); + expect(stable.skipped).toBeGreaterThan(0); + expect( + fs + .readdirSync(paths.dir) + .filter(name => name.startsWith(".gjc-exact-unlink-placeholder-")) + .sort(), + ).toEqual(placeholderNames); + expect(fs.readFileSync(payloadPlaceholderPath)).toEqual(retainedBytes); }); test("a staging temp replaced between identity capture and delete is not removed", async () => { From 1e31638c6ef21057cfaacca6c346ba3d48cf6262 Mon Sep 17 00:00:00 2001 From: twoimo Date: Fri, 31 Jul 2026 21:14:40 +0900 Subject: [PATCH 7/7] ci: retry timed-out Telegram generation guard