From 2f430f2fbd3b3b7c68c04c0bbe932cc99fdee5f4 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Fri, 17 Jul 2026 17:55:20 +0800 Subject: [PATCH 1/8] feat(spill-local): one-shot startup cleanup for local spill files The local spill backend never reclaimed its files, so configured roots grew without bound and default per-process dsh-spill-* temp roots piled up across runs. Immediate deletion is unsafe because persisted, resumed, and forked sessions may still reference an older locator. Add a fiber-owned, best-effort sweep that runs once after activation (never delaying availability, awaited on disposal): it deletes regular files older than cleanupPeriodDays (default 30; 0 disables) across the configured root and prior default temp roots, prunes emptied dirs, and skips symlinks/unknown entries. Every filesystem failure is contained and logged, so the sweep cannot fail activation or a concurrent write. --- .../2026-07-08-tool-output-spill-files.md | 3 +- ...7-17-local-spill-startup-cleanup.i18n.yaml | 6 + .../2026-07-17-local-spill-startup-cleanup.md | 35 ++ ...26-07-17-local-spill-startup-cleanup.zh.md | 35 ++ docs/config-catalog.md | 11 +- packages/spill/spill-local/README.md | 11 +- packages/spill/spill-local/src/index.ts | 103 +++++- packages/spill/spill-local/src/store.ts | 202 +++++++++++- .../spill-local/tests/spill-local.spec.ts | 309 +++++++++++++++++- 9 files changed, 696 insertions(+), 19 deletions(-) create mode 100644 .agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml create mode 100644 .agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md create mode 100644 .agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md index 14667b74ca..6164066dbb 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md @@ -160,7 +160,8 @@ Those cases can consume `ctx.spillStore` directly in later work. They are not pa - Tool-owned spill for subagent rollouts (`await run.result`, read in-process child session before `run.dispose()`, save JSONL). - Per-tool opt-out or per-tool policy declarations if the built-in `read` skip is insufficient. - Remote or database storage backends for ACP or remote environments where a local path is not meaningful. -- Cleanup and retention policy for old spill files, likely tied to session cleanup. + +Cleanup shipped for the local backend as a one-shot startup sweep, not tied to session deletion — see the [startup-cleanup RFC](./2026-07-17-local-spill-startup-cleanup.md). The seam still defines no per-session cleanup policy; retention is a backend concern. ## Testing diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml new file mode 100644 index 0000000000..97d2b5426b --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write +2026-07-17-local-spill-startup-cleanup.md: ca4931776f89e641f127072f665e238ca2a1600d +2026-07-17-local-spill-startup-cleanup.zh.md: b90923844ab71e1ce570e8adb66f81aed7bc3488 diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md new file mode 100644 index 0000000000..ca4931776f --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md @@ -0,0 +1,35 @@ +# Agent Note: One-shot startup cleanup for local spill files + +Status: implemented + +English | [中文](2026-07-17-local-spill-startup-cleanup.zh.md) + +## Problem + +The local spill backend never deleted the full tool results it wrote. Every oversized result added another file, so configured roots grew without bound and default per-process `dsh-spill-*` roots accumulated across runs. Immediate deletion is wrong because persisted, resumed, and forked sessions may still reference a locator. The [tool output spill policy](./2026-07-08-tool-output-spill-files.md) needs a bounded local-storage lifetime. + +## Decision + +`dsh-spill-local` runs one best-effort cleanup sweep after activation. It does not delay service availability, is owned by the plugin fiber (a single `ctx.effect` whose generator launches the sweep and yields an async disposer that awaits it), and is awaited during disposal so no sweep I/O outlives the fiber. There is no recurring timer and no separate process. + +A `cleanupPeriodDays` config defaults to `30`; `0` disables cleanup. An invalid value (negative or fractional) throws at load. The sweep scans the configured/active root plus any prior default `dsh-spill-*` temp roots discovered under the OS temp dir, deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`, and prunes directories left empty. It uses `lstat`, so a symlink is never followed or deleted; unrelated entries (non-`session-` directories, special files) are skipped. Every filesystem failure is caught and logged through `ctx.logger.warn` — the sweep never throws, so it cannot reject activation or a concurrent spill write. Discovery excludes symlinks and non-directories, returning only real `dsh-spill-*` directories the backend could have created. + +The ctx-free mechanics live in `packages/spill/spill-local/src/store.ts` (`sweepSpillRoots`, `discoverDefaultRoots`, `DEFAULT_ROOT_PREFIX`, `isErrno`), unit-testable without a `ctx`; the service in `src/index.ts` owns the config, the cutoff, and the fiber-owned launch/await. + +## Alternatives considered + +**Run a periodic timer.** Rejected because it adds timer lifecycle, overlap control, and another interval knob. A long-lived process may retain files until restart. + +**Delete spills on session disposal.** Rejected because durable sessions, resumes, and forks retain locators. + +**Delete old session directories recursively.** Rejected because a concurrent process may create a fresh spill after the age check. Per-file expiry preserves fresh writes. + +**Tie cleanup to session-persistence deletion.** Rejected because the persistence seam has no common deletion lifecycle, while the local backend also owns independent temporary roots. + +## Consequences + +Cleanup cost the backend a startup sweep and a config knob, and bought a bounded local-storage lifetime without a timer, a daemon, or a session-lifecycle coupling. Concurrent processes may duplicate startup I/O; strict filtering and idempotent file deletion keep this safe. A long-lived process is not cleaned again until restart, and retention deliberately makes old model-visible locators stale only once they age past the cutoff. The seam itself still defines no retention policy — this is a local-backend concern. + +## Testing + +`dsh-spill-local` unit tests cover the age boundary (strictly-older expires, boundary kept), `cleanupPeriodDays: 0` disabling, empty-directory pruning, symlink/unrelated-entry skipping, configured-plus-discovered-root coverage through the real `gatherRoots`/`discoverDefaultRoots` path, active-root de-duplication, load-time validation of a bad `cleanupPeriodDays`, filesystem-failure containment (logged, not thrown) both directly and through the service's `ctx.logger.warn` wiring, and the quiescence contract — activation is available while a barrier-held sweep is parked, and disposal only settles after the sweep finishes. diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md new file mode 100644 index 0000000000..b90923844a --- /dev/null +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md @@ -0,0 +1,35 @@ +# Agent Note: 本地 spill 文件的一次性启动清理 + +Status: implemented + +[English](2026-07-17-local-spill-startup-cleanup.md) | 中文 + +## 问题 + +本地 spill 后端从不删除它写下的完整工具结果。每个超限结果都会新增一个文件，因此配置的根目录会无限增长，而每进程默认的 `dsh-spill-*` 根目录也会跨多次运行不断累积。立即删除是错误的，因为已持久化、已恢复和已 fork 的会话仍可能引用某个 locator。[工具输出 spill 策略](./2026-07-08-tool-output-spill-files.md)需要一个有界的本地存储生命周期。 + +## 决策 + +`dsh-spill-local` 在激活后运行一次尽力而为的清理扫描。它不延迟服务可用性，由插件 fiber 拥有（一个 `ctx.effect`，其生成器启动该扫描并让出一个等待它的异步 disposer），并在 dispose 期间被等待，因此没有扫描 I/O 会存活到 fiber 之后。既没有周期性定时器，也没有独立进程。 + +`cleanupPeriodDays` 配置默认为 `30`；`0` 会禁用清理。无效值（负数或小数）在加载时抛出。扫描会遍历配置的/活动的根目录，以及在 OS 临时目录下发现的任何先前默认 `dsh-spill-*` 临时根目录，删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件，并修剪清空后的目录。它使用 `lstat`，因此符号链接绝不会被跟随或删除；无关条目（非 `session-` 目录、特殊文件）会被跳过。每一次文件系统失败都会被捕获并通过 `ctx.logger.warn` 记录——扫描绝不抛出，因此它无法让激活失败，也无法影响并发的 spill 写入。发现过程排除符号链接与非目录，只返回后端可能创建过的真实 `dsh-spill-*` 目录。 + +无 ctx 依赖的机制位于 `packages/spill/spill-local/src/store.ts`（`sweepSpillRoots`、`discoverDefaultRoots`、`DEFAULT_ROOT_PREFIX`、`isErrno`），无需 `ctx` 即可做单元测试；`src/index.ts` 中的服务负责配置、截止时间以及 fiber 拥有的启动/等待。 + +## 考虑过的替代方案 + +**运行周期性定时器。** 已否决，因为它引入了定时器生命周期、重叠控制以及又一个间隔旋钮。长期运行的进程可能会保留文件直到重启。 + +**在会话 dispose 时删除 spill。** 已否决，因为持久会话、恢复和 fork 都会保留 locator。 + +**递归删除旧的会话目录。** 已否决，因为并发进程可能在年龄检查之后创建一个新的 spill。按文件过期可保留新写入。 + +**将清理绑定到会话持久化删除。** 已否决，因为持久化 seam 没有共同的删除生命周期，而本地后端还独立拥有临时根目录。 + +## 后果 + +清理让后端付出了一次启动扫描和一个配置旋钮的代价，换来了无需定时器、守护进程或会话生命周期耦合的有界本地存储生命周期。并发进程可能重复启动 I/O；严格的过滤与幂等的文件删除保证了这一点的安全。长期运行的进程在重启前不会再次被清理，而这种保留是刻意的——旧的模型可见 locator 只有在超过截止时间后才会失效。seam 本身仍不定义任何保留策略——这是本地后端的关切。 + +## 验证 + +`dsh-spill-local` 单元测试覆盖了年龄边界（严格更旧者过期，边界值保留）、`cleanupPeriodDays: 0` 的禁用、空目录修剪、符号链接/无关条目的跳过、通过真实 `gatherRoots`/`discoverDefaultRoots` 路径对配置根加发现根的覆盖、活动根去重、对错误 `cleanupPeriodDays` 的加载期校验、文件系统失败的兜底（记录而非抛出）——既直接测试，也经由服务的 `ctx.logger.warn` 接线测试——以及静止契约：在一个被屏障挂起的扫描停驻期间激活仍然可用，而 dispose 只有在扫描结束后才会完成。 diff --git a/docs/config-catalog.md b/docs/config-catalog.md index a845fe22e1..ac7c00738c 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -2062,10 +2062,19 @@ export interface Config { * a local deployment. Set it to keep spill files under a known location. */ root?: string + /** + * Age in days after which a spill file is eligible for the one-shot startup + * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose + * `mtime` is strictly older than the cutoff are deleted and emptied + * directories are pruned; fresh files, symlinks, and unrelated entries are + * left untouched. Retention is deliberate — a resumed or forked session may + * still reference an older locator until it ages out. + */ + cleanupPeriodDays?: number } ``` -Source: [`packages/spill/spill-local/src/index.ts:22`](../packages/spill/spill-local/src/index.ts) +Source: [`packages/spill/spill-local/src/index.ts:28`](../packages/spill/spill-local/src/index.ts) diff --git a/packages/spill/spill-local/README.md b/packages/spill/spill-local/README.md index 2270a65d92..c97ddf89cd 100644 --- a/packages/spill/spill-local/README.md +++ b/packages/spill/spill-local/README.md @@ -17,8 +17,15 @@ Files land at `/session-/​-`: | Key | Default | Meaning | |---|---|---| | `root` | private 0700 temp dir | Root directory for spill files. Set to keep them under a known location. | +| `cleanupPeriodDays` | `30` | Age in days after which a spill file is eligible for the one-shot startup cleanup sweep. `0` disables cleanup. | -`saveText` rejects on a real storage failure (permissions, ENOSPC); the spill policy treats a rejection as best-effort and keeps the inline result. See the seam README for the vocabulary and the [tool output spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md) for the design. +## Startup cleanup + +The backend never deletes a spill on the write path — a persisted, resumed, or forked session may still reference an older locator, so immediate deletion would break retrieval. Instead, one best-effort sweep runs **once after activation**: it does not delay service availability, is owned by the plugin fiber, and is awaited on disposal (no sweep I/O outlives the fiber). There is no recurring timer and no separate process, so a long-lived deployment is not swept again until its next restart. + +The sweep scans the configured `root` **and** any earlier default `dsh-spill-*` temp roots that prior default-root runs left under the OS temp dir. Within each, it deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays` and prunes any directory left empty. It never follows or deletes a symlink, skips unrelated entries, and contains every filesystem failure (logged, never thrown) so it cannot fail activation or a concurrent spill write. Retention is deliberate: an old model-visible locator goes stale only once it ages past the cutoff. + +`saveText` rejects on a real storage failure (permissions, ENOSPC); the spill policy treats a rejection as best-effort and keeps the inline result. See the seam README for the vocabulary and the [tool output spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md) for the design, and the [startup-cleanup Agent Note](../../../.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md) for the sweep. ## Model Experience @@ -30,5 +37,5 @@ No direct invalidation; the named consumer owns any request-prefix changes. ## Known Limitations and Deferred Work -- **Local spill files persist until external cleanup** — the backend has no session-lifecycle deletion or age-based retention policy, because persisted, resumed, and forked sessions may still reference a path. +- **A long-lived deployment is not swept until restart** — the one-shot sweep runs once after activation, so files that age past `cleanupPeriodDays` mid-run are reclaimed only on the next start; there is no recurring timer. - **Locators require a co-located filesystem consumer** — a remote or virtual deployment needs another `SpillStore` backend whose locator and retrieval hint are meaningful there. diff --git a/packages/spill/spill-local/src/index.ts b/packages/spill/spill-local/src/index.ts index 54e2e6cd6d..33948712f0 100644 --- a/packages/spill/spill-local/src/index.ts +++ b/packages/spill/spill-local/src/index.ts @@ -3,20 +3,26 @@ * `@deepseek-ai/dsh-spill` storage seam. Persists a tool's oversized text to a * private, session-scoped file (see `./store.ts` for the traversal-safe naming * and exclusive owner-only write) and returns a path locator plus local - * read/grep retrieval guidance. + * read/grep retrieval guidance. After activation it runs one best-effort + * startup sweep that reclaims spill files older than `cleanupPeriodDays`. * * @module @deepseek-ai/dsh-spill-local */ import { Context } from '@deepseek-ai/cordis' import { resolve } from 'node:path' +import { tmpdir } from 'node:os' import z from '@deepseek-ai/schemastery' import { SpillLocator, SpillStore } from '@deepseek-ai/dsh-spill' import type { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill' -import { privateRoot, saveTextFile } from './store.ts' +import { discoverDefaultRoots, privateRoot, saveTextFile, sweepSpillRoots } from './store.ts' +import type { WarnFn } from './store.ts' -export { encodeSegment, privateRoot, saveTextFile, sessionDir } from './store.ts' -export type { SavedText, SaveTextOptions } from './store.ts' +export { discoverDefaultRoots, encodeSegment, isErrno, privateRoot, saveTextFile, sessionDir, sweepSpillRoots, DEFAULT_ROOT_PREFIX } from './store.ts' +export type { SavedText, SaveTextOptions, SweepOptions, WarnFn } from './store.ts' + +/** Milliseconds in one day — converts the `cleanupPeriodDays` config to the sweep cutoff. */ +const MS_PER_DAY = 24 * 60 * 60 * 1000 /** Plugin config (all optional — `static Config` supplies the defaults). */ export interface Config { @@ -26,25 +32,114 @@ export interface Config { * a local deployment. Set it to keep spill files under a known location. */ root?: string + /** + * Age in days after which a spill file is eligible for the one-shot startup + * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose + * `mtime` is strictly older than the cutoff are deleted and emptied + * directories are pruned; fresh files, symlinks, and unrelated entries are + * left untouched. Retention is deliberate — a resumed or forked session may + * still reference an older locator until it ages out. + */ + cleanupPeriodDays?: number } +/** The shape after schemastery applied the defaults. */ +type ResolvedConfig = Required> & Pick + /** * Local-filesystem spill backend. Files land under `/session-/…` * with unpredictable names, an exclusive owner-only (0600) write, and a private * (0700) root — a spilled tool result must not be readable by other local users * or redirectable via a planted symlink. + * + * After activation it launches ONE best-effort cleanup sweep (see + * {@link cleanupPeriodDays}) that reclaims expired spill files without delaying + * service availability; the sweep is owned by the plugin fiber and awaited + * during disposal, so a fiber unload never returns before it quiesces. */ export class LocalSpillStore extends SpillStore { static Config: z = z.object({ root: z.string(), + cleanupPeriodDays: z.number().default(30), }) /** Resolved absolute spill root (config `root`, else the private default), fixed at construction. */ readonly root: string + /** Validated config (schemastery applied the `cleanupPeriodDays` default before construction). */ + readonly config: ResolvedConfig + + /** + * The in-flight (or settled) startup cleanup sweep. Held so disposal can await + * it; `undefined` when cleanup is disabled (`cleanupPeriodDays === 0`). + */ + private cleanup: Promise | undefined + constructor(ctx: Context, config: Config) { super(ctx) + // schemastery (static Config) has already filled `cleanupPeriodDays`; the + // cast records that runtime fact for exactOptionalPropertyTypes. + this.config = config as ResolvedConfig + if (!Number.isInteger(this.config.cleanupPeriodDays) || this.config.cleanupPeriodDays < 0) { + throw new Error(`spill-local: cleanupPeriodDays must be a non-negative integer (got ${this.config.cleanupPeriodDays})`) + } this.root = config.root !== undefined ? resolve(config.root) : privateRoot() + + // One best-effort startup sweep, owned by the fiber. The generator body runs + // at activation but does NOT await the sweep — it launches it and yields an + // async disposer that awaits the SAME promise, so service availability is + // never delayed yet a fiber unload reaches quiescence (no sweep I/O outlives + // the fiber). Disabled (`cleanupPeriodDays === 0`) yields a no-op disposer. + ctx.effect(function* (this: LocalSpillStore) { + if (this.config.cleanupPeriodDays > 0) { + const warn: WarnFn = (message) => { this.ctx.logger.warn(message) } + this.cleanup = this.runCleanup(warn) + } + yield async () => { await this.cleanup } + }.bind(this), 'spill-local cleanup sweep') + } + + /** + * Run the one-shot cleanup: gather the roots to sweep (see {@link gatherRoots}) + * and sweep all of them at the age cutoff. Best-effort — + * {@link sweepSpillRoots} contains every filesystem failure, so this never + * rejects and cannot fail activation or a concurrent spill write. + * + * @param warn - sink for a contained filesystem failure. + * @returns Resolves when the sweep finishes (never rejects). + */ + private async runCleanup(warn: WarnFn): Promise { + const cutoffMs = Date.now() - this.config.cleanupPeriodDays * MS_PER_DAY + const roots = await this.gatherRoots(warn) + await sweepSpillRoots({ roots, cutoffMs, warn }) + } + + /** + * The roots the startup sweep covers: the prior default `dsh-spill-*` temp + * roots (see {@link discoverDefaultRoots}) plus the configured/active root, + * de-duplicated (the active root may itself be a discovered default). A test + * overrides this to inject an isolated root set — and, being the sweep's one + * async gather point, to hold the sweep open across a disposal for the + * quiescence check; it is a test seam, not a deployment knob. + * + * @param warn - sink for a contained discovery failure. + * @returns The absolute roots to sweep. + */ + protected async gatherRoots(warn: WarnFn): Promise { + const discovered = await discoverDefaultRoots(warn, this.defaultRootsBase()) + return discovered.includes(this.root) ? discovered : [...discovered, this.root] + } + + /** + * The directory scanned for prior default `dsh-spill-*` roots — the OS tmpdir, + * where {@link privateRoot} creates them (accumulation only happens there). A + * test overrides this to point discovery at an isolated fixture instead of the + * real tmpdir; it is a test seam, not a deployment knob. + * + * @returns The base directory to scan for default spill roots. + */ + protected defaultRootsBase(): string { + return tmpdir() } async saveText(input: SaveTextSpill): Promise { diff --git a/packages/spill/spill-local/src/store.ts b/packages/spill/spill-local/src/store.ts index e4451d5268..431e4840c6 100644 --- a/packages/spill/spill-local/src/store.ts +++ b/packages/spill/spill-local/src/store.ts @@ -8,10 +8,18 @@ import { createHash, randomBytes } from 'node:crypto' import { mkdtempSync } from 'node:fs' -import { mkdir, open } from 'node:fs/promises' +import { lstat, mkdir, open, readdir, rmdir, unlink } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' +/** + * Filename prefix for the lazily-created private default spill roots + * (`mkdtemp(tmpdir()/dsh-spill-)`). Startup cleanup rediscovers these + * per-process roots (from prior runs that used no configured `root`) by this + * prefix — see {@link discoverDefaultRoots}. + */ +export const DEFAULT_ROOT_PREFIX = 'dsh-spill-' + let defaultRoot: string | undefined /** @@ -23,7 +31,7 @@ let defaultRoot: string | undefined * @returns The lazily-created private spill root. */ export function privateRoot(): string { - defaultRoot ??= mkdtempSync(join(tmpdir(), 'dsh-spill-')) + defaultRoot ??= mkdtempSync(join(tmpdir(), DEFAULT_ROOT_PREFIX)) return defaultRoot } @@ -114,3 +122,193 @@ export async function saveTextFile(options: SaveTextOptions): Promise } return { path, bytes } } + +/** A one-argument warning sink — the sweep's only side effect on failure (never throws). */ +export type WarnFn = (message: string) => void + +/** Options for {@link sweepSpillRoots} — the roots to scan, the age cutoff, and a failure sink. */ +export interface SweepOptions { + /** Absolute spill roots to sweep (configured root and/or discovered default roots). */ + roots: string[] + /** + * Epoch-millis cutoff: a regular file is deleted when its `mtime` is strictly + * older than this. The caller derives it from `now - cleanupPeriodDays`, so a + * file written exactly at the boundary is kept (only strictly-older expires). + */ + cutoffMs: number + /** Where a contained filesystem failure is reported; the sweep itself never throws. */ + warn: WarnFn +} + +/** + * Delete a single path, treating a concurrent-race disappearance as success. + * A parallel process (or another sweep) may `unlink` the same file between our + * scan and our own `unlink` — ENOENT then means the goal (file gone) already + * holds, so it is not a failure. Any other error is reported and swallowed. + * + * @param path The absolute file path to remove. + * @param warn Sink for a non-ENOENT failure message. + * @returns Resolves once the removal was attempted (never rejects). + */ +async function unlinkIdempotent(path: string, warn: WarnFn): Promise { + try { + await unlink(path) + } catch (error: unknown) { + /* v8 ignore start -- reached only when a file selected for deletion (a + regular file that passed lstat) then fails to unlink: either it raced away + (ENOENT) or a permission/IO fault struck between the stat and the unlink. + Neither is deterministically reproducible in-process. */ + if (isErrno(error, 'ENOENT')) return + warn(`spill-local: failed to delete ${path}: ${String(error)}`) + /* v8 ignore stop */ + } +} + +/** + * True when `error` is a Node system error carrying the given `code`. + * + * @param error The caught value to test. + * @param code The `NodeJS.ErrnoException` code to match (e.g. `'ENOENT'`). + * @returns `true` when `error` is an `Error` whose `code` equals `code`. + */ +export function isErrno(error: unknown, code: string): boolean { + return error instanceof Error && (error as NodeJS.ErrnoException).code === code +} + +/** + * Sweep one spill session directory: delete expired regular files, skip + * everything else, and report the directory empty afterward so the caller can + * prune it. A symlink or any non-regular entry (socket, fifo, nested dir) is + * left untouched — `lstat` never follows a link, so a planted symlink can + * neither be deleted nor redirect the age check. Every per-entry failure is + * contained: one unreadable file does not abort the directory. + * + * @param dir The absolute session directory to scan. + * @param cutoffMs Files with `mtime` strictly older than this are deleted. + * @param warn Sink for contained filesystem failures. + * @returns `true` when the directory holds no entries after the sweep (a prune candidate). + */ +async function sweepSessionDir(dir: string, cutoffMs: number, warn: WarnFn): Promise { + let names: string[] + try { + names = await readdir(dir) + } catch (error: unknown) { + // A `session-*` entry that is not a readable directory (a stray file, or an + // unreadable/vanished dir) is not ours to fix — report and leave it. False + // keeps it out of the prune step. + warn(`spill-local: failed to read ${dir}: ${String(error)}`) + return false + } + let remaining = names.length + for (const name of names) { + const path = join(dir, name) + let stats + try { + stats = await lstat(path) + } catch (error: unknown) { + /* v8 ignore start -- an entry that readdir just returned then fails to + lstat only by racing away (ENOENT) or a permission/IO fault; keep it out + of the deterministic test surface. */ + if (isErrno(error, 'ENOENT')) { remaining--; continue } + warn(`spill-local: failed to stat ${path}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + // Only regular files expire. Symlinks and other special entries are skipped + // (never followed) so the sweep cannot be redirected or delete a link. + if (!stats.isFile()) continue + if (stats.mtimeMs >= cutoffMs) continue + await unlinkIdempotent(path, warn) + remaining-- + } + return remaining === 0 +} + +/** + * Best-effort one-shot cleanup: across each root, delete expired regular files + * under its `session-*` directories and prune any directory left empty. The + * sweep is idempotent and safe to run concurrently with live spill writes and + * with another process's sweep — per-file expiry preserves a fresh write even + * if it lands mid-sweep, and every filesystem failure is caught and reported + * rather than thrown, so a caller can await this during activation/disposal + * without it ever rejecting. + * + * @param options The roots to sweep, the age cutoff, and the failure sink. + * @returns Resolves when the sweep finishes (never rejects). + */ +export async function sweepSpillRoots(options: SweepOptions): Promise { + const { roots, cutoffMs, warn } = options + for (const root of roots) { + let entries: string[] + try { + entries = await readdir(root) + } catch (error: unknown) { + // A root that does not exist yet (no spill ever written) is the common + // case, not an error: ENOENT is silent, anything else is reported. + if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to read root ${root}: ${String(error)}`) + continue + } + for (const name of entries) { + // Only the backend's own `session-` directories are swept; an + // unrelated sibling under a shared configured root is left untouched. + if (!name.startsWith('session-')) continue + const dir = join(root, name) + const empty = await sweepSessionDir(dir, cutoffMs, warn) + if (!empty) continue + try { + await rmdir(dir) + } catch (error: unknown) { + /* v8 ignore start -- prune runs only on a dir observed empty; a failure + here means a concurrent writer added a file (ENOTEMPTY) or a + permission/IO fault struck — both are races outside deterministic + in-process testing. */ + if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { + warn(`spill-local: failed to prune ${dir}: ${String(error)}`) + } + /* v8 ignore stop */ + } + } + } +} + +/** + * Discover prior default spill roots: the `dsh-spill-*` directories directly + * under `base` (the OS tmpdir) that earlier runs created via {@link privateRoot} + * when no `root` was configured. A long-lived deployment with a configured root + * will find none; a series of default-root runs accumulates one per process, so + * the startup sweep reclaims them all. Symlinks and non-directories are excluded + * — only real directories the backend could have created are returned. + * + * @param warn Sink for a failure reading `base` (returns `[]` on failure). + * @param base The directory to scan; defaults to the OS tmpdir (a test seam). + * @returns Absolute paths of the discovered default roots (possibly empty). + */ +export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir()): Promise { + let entries: string[] + try { + entries = await readdir(base) + } catch (error: unknown) { + warn(`spill-local: failed to scan ${base} for default roots: ${String(error)}`) + return [] + } + const roots: string[] = [] + for (const name of entries) { + if (!name.startsWith(DEFAULT_ROOT_PREFIX)) continue + const path = join(base, name) + let stats + try { + // lstat, not stat: a symlink named `dsh-spill-*` must not be treated as a + // root we then sweep (it could point anywhere). + stats = await lstat(path) + } catch (error: unknown) { + /* v8 ignore start -- an entry readdir just returned fails to lstat only by + racing away (ENOENT) or a permission/IO fault; not deterministically + reproducible. */ + if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to stat default root ${path}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + if (stats.isDirectory()) roots.push(path) + } + return roots +} diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index fd01babeff..8e632c885d 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -2,19 +2,33 @@ * Tests for the LOCAL spill backend: `saveText` writes a session-scoped file and * returns a locator + byte length + retrieval hint, filename sanitization * neutralizes traversal, the configured `root` is honored (and the private - * default when omitted), and a storage failure rejects. The Cordis-free - * `store.ts` helpers are exercised directly for the naming/encoding edge cases. + * default when omitted), and a storage failure rejects. The startup cleanup + * sweep expires old files, prunes empty dirs, skips symlinks/unknown entries, + * discovers prior default roots, contains filesystem failures, and is awaited on + * disposal without blocking activation. The Cordis-free `store.ts` helpers are + * exercised directly for the naming/encoding and sweep edge cases. */ -import { describe, expect, it, beforeEach, afterEach } from 'vitest' +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' import { Context } from '@deepseek-ai/cordis' -import { mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { basename, dirname, isAbsolute, join, normalize } from 'node:path' import { CallId } from '@deepseek-ai/dsh-llm' import { SessionId } from '@deepseek-ai/dsh-session' import type { SaveTextSpill } from '@deepseek-ai/dsh-spill' -import LocalSpillStore, { encodeSegment, privateRoot, saveTextFile, sessionDir } from '@deepseek-ai/dsh-spill-local' +import LocalSpillStore, { + DEFAULT_ROOT_PREFIX, + discoverDefaultRoots, + encodeSegment, + isErrno, + privateRoot, + saveTextFile, + sessionDir, + sweepSpillRoots, +} from '@deepseek-ai/dsh-spill-local' + +const DAY_MS = 24 * 60 * 60 * 1000 let root: string @@ -25,6 +39,13 @@ afterEach(() => { rmSync(root, { recursive: true, force: true }) }) +/** Write a file with an mtime `ageDays` in the past (fractional allowed). */ +function writeAged(path: string, content: string, ageDays: number): void { + writeFileSync(path, content) + const when = (Date.now() - ageDays * DAY_MS) / 1000 + utimesSync(path, when, when) +} + function request(overrides: Partial = {}): SaveTextSpill { return { owner: { sessionId: SessionId('sess-1') }, @@ -113,9 +134,11 @@ describe('privateRoot', () => { }) describe('LocalSpillStore service', () => { + // These tests exercise save/root resolution, not cleanup; disabling the sweep + // (cleanupPeriodDays: 0) keeps them from scanning/sweeping the real tmpdir. it('registers as ctx.spillStore and saves under the configured root', async () => { const ctx = new Context() - await ctx.plugin(LocalSpillStore, { root }) + await ctx.plugin(LocalSpillStore, { root, cleanupPeriodDays: 0 }) const ref = await ctx.spillStore.saveText(request()) expect(dirname(ref.locator)).toBe(sessionDir(root, 'sess-1')) expect(readFileSync(ref.locator, 'utf8')).toBe('the full body') @@ -125,13 +148,13 @@ describe('LocalSpillStore service', () => { it('resolves a relative configured root to absolute', async () => { const ctx = new Context() - await ctx.plugin(LocalSpillStore, { root: '.' }) + await ctx.plugin(LocalSpillStore, { root: '.', cleanupPeriodDays: 0 }) expect(isAbsolute((ctx.spillStore as LocalSpillStore).root)).toBe(true) }) it('falls back to the private root when none is configured', async () => { const ctx = new Context() - await ctx.plugin(LocalSpillStore, {}) + await ctx.plugin(LocalSpillStore, { cleanupPeriodDays: 0 }) expect((ctx.spillStore as LocalSpillStore).root).toBe(privateRoot()) }) @@ -139,7 +162,275 @@ describe('LocalSpillStore service', () => { const ctx = new Context() // A file (not a dir) as the root makes mkdir under it fail — a real storage error. const filePath = (await saveTextFile({ root, sessionId: 's', suggestedName: 'f', content: 'x' })).path - await ctx.plugin(LocalSpillStore, { root: filePath }) + await ctx.plugin(LocalSpillStore, { root: filePath, cleanupPeriodDays: 0 }) await expect(ctx.spillStore.saveText(request())).rejects.toThrow() }) + + it('rejects a negative or fractional cleanupPeriodDays at load', async () => { + await expect(new Context().plugin(LocalSpillStore, { root, cleanupPeriodDays: -1 })) + .rejects.toThrow(/cleanupPeriodDays must be a non-negative integer/) + await expect(new Context().plugin(LocalSpillStore, { root, cleanupPeriodDays: 1.5 })) + .rejects.toThrow(/cleanupPeriodDays must be a non-negative integer/) + }) + + it('defaults cleanupPeriodDays to 30', async () => { + const ctx = new Context() + // Point discovery at an empty isolated base so the default sweep does not + // touch the real tmpdir; assert only that the default landed on config. + const emptyBase = mkdtempSync(join(tmpdir(), 'dsh-empty-')) + class Isolated extends LocalSpillStore { + protected override defaultRootsBase(): string { return emptyBase } + } + try { + const fiber = await ctx.plugin(Isolated, { root }) + const store = ctx.spillStore as LocalSpillStore + await fiber.dispose() + expect(store.config.cleanupPeriodDays).toBe(30) + } finally { + rmSync(emptyBase, { recursive: true, force: true }) + } + }) + + it('the default discovery base is the OS tmpdir', async () => { + // Every hermetic sweep test overrides defaultRootsBase(); pin its production + // default here (scan the OS tmpdir) without letting the sweep touch tmpdir. + class Exposed extends LocalSpillStore { + base(): string { return this.defaultRootsBase() } + protected override async gatherRoots(): Promise { return [] } + } + const ctx = new Context() + const fiber = await ctx.plugin(Exposed, { root, cleanupPeriodDays: 30 }) + const store = ctx.spillStore as Exposed + await fiber.dispose() + expect(store.base()).toBe(tmpdir()) + }) + + it('routes a sweep filesystem failure to ctx.logger.warn (service warn wiring)', async () => { + // A `session-*` entry that is a FILE, not a directory, makes readdir throw + // ENOTDIR inside the real sweep. The service's warn closure must forward it + // to ctx.logger.warn, and disposal must still settle cleanly. + const stray = join(root, 'session-stray'); writeFileSync(stray, 'x') + const ctx = new Context() + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) + class Discovering extends LocalSpillStore { + protected override async gatherRoots(): Promise { return [this.root] } + } + const fiber = await ctx.plugin(Discovering, { root, cleanupPeriodDays: 30 }) + await fiber.dispose() + expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read')) + }) }) + +/** + * A store whose sweep covers exactly the roots handed in (no real-tmpdir scan) — + * the hermetic seam for the cleanup tests. `barrier`, when set, holds the async + * gather open so a test can prove disposal awaits the sweep. + */ +class SweptStore extends LocalSpillStore { + static sweepRoots: string[] = [] + static barrier: Promise | undefined + protected override async gatherRoots(): Promise { + if (SweptStore.barrier) await SweptStore.barrier + return SweptStore.sweepRoots + } +} + +async function runSweep(roots: string[], cleanupPeriodDays = 30): Promise { + SweptStore.sweepRoots = roots + SweptStore.barrier = undefined + const ctx = new Context() + const fiber = await ctx.plugin(SweptStore, { root, cleanupPeriodDays }) + // Disposal awaits the fiber-owned sweep, so after this the sweep has run. + await fiber.dispose() +} + +describe('startup cleanup sweep', () => { + it('deletes files older than the cutoff and keeps fresh ones', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + const fresh = join(dir, 'fresh.txt'); writeAged(fresh, 'y', 1) + await runSweep([root]) + expect(existsSync(old)).toBe(false) + expect(existsSync(fresh)).toBe(true) + }) + + it('keeps a file exactly at the boundary (only strictly-older expires)', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + // mtime == cutoff: mtimeMs >= cutoffMs holds, so it is kept. Age it just + // under 30d to avoid the sub-millisecond race of "exactly now - 30d". + const boundary = join(dir, 'boundary.txt'); writeAged(boundary, 'x', 29.9) + await runSweep([root]) + expect(existsSync(boundary)).toBe(true) + }) + + it('disabled (cleanupPeriodDays: 0) sweeps nothing', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 400) + await runSweep([root], 0) + expect(existsSync(old)).toBe(true) + }) + + it('prunes a directory left empty, keeps one with a surviving file', async () => { + const emptied = sessionDir(root, 'emptied') + const kept = sessionDir(root, 'kept') + mkdirSync(emptied, { recursive: true }) + mkdirSync(kept, { recursive: true }) + writeAged(join(emptied, 'a.txt'), 'x', 40) + writeAged(join(kept, 'fresh.txt'), 'y', 1) + await runSweep([root]) + expect(existsSync(emptied)).toBe(false) + expect(existsSync(kept)).toBe(true) + }) + + it('skips symlinks and non-session entries', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + // A symlink pointing at an old target must NOT be followed or deleted. + const target = join(root, 'target.txt'); writeAged(target, 'keep', 40) + const link = join(dir, 'link.txt'); symlinkSync(target, link) + // A non-session sibling directory under a shared root is untouched. + const unrelated = join(root, 'not-a-session'); mkdirSync(unrelated) + const unrelatedOld = join(unrelated, 'old.txt'); writeAged(unrelatedOld, 'x', 40) + await runSweep([root]) + // The symlink itself survives (lstat sees a link, not a file), so its dir is + // not empty and is not pruned; the link target survives too. + expect(existsSync(link)).toBe(true) + expect(existsSync(target)).toBe(true) + expect(existsSync(unrelatedOld)).toBe(true) + }) + + it('covers the configured root AND discovered default roots (real gatherRoots)', async () => { + // A prior default root under an isolated fake tmpdir + the configured root. + // This test drives the REAL gatherRoots/discoverDefaultRoots path by seaming + // only the tmpdir scan base, not gatherRoots itself. + const fakeTmp = mkdtempSync(join(tmpdir(), 'dsh-faketmp-')) + const priorDefault = join(fakeTmp, `${DEFAULT_ROOT_PREFIX}ABCDEF`) + const priorDir = sessionDir(priorDefault, 'old-sess') + mkdirSync(priorDir, { recursive: true }) + const priorOld = join(priorDir, 'old.txt'); writeAged(priorOld, 'x', 40) + const cfgDir = sessionDir(root, 'sess-1') + mkdirSync(cfgDir, { recursive: true }) + const cfgOld = join(cfgDir, 'old.txt'); writeAged(cfgOld, 'x', 40) + class Discovering extends LocalSpillStore { + protected override defaultRootsBase(): string { return fakeTmp } + } + try { + const ctx = new Context() + const fiber = await ctx.plugin(Discovering, { root, cleanupPeriodDays: 30 }) + await fiber.dispose() + expect(existsSync(priorOld)).toBe(false) + expect(existsSync(cfgOld)).toBe(false) + } finally { + rmSync(fakeTmp, { recursive: true, force: true }) + } + }) + + it('de-dups when the active root is itself a discovered default (real gatherRoots)', async () => { + // The configured root lives directly under the seamed base and matches the + // default prefix, so discovery finds it AND it is the active root — the sweep + // must run once, not choke on the duplicate. + const fakeTmp = mkdtempSync(join(tmpdir(), 'dsh-faketmp-')) + const activeDefault = join(fakeTmp, `${DEFAULT_ROOT_PREFIX}ACTIVE`) + const dir = sessionDir(activeDefault, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + class Discovering extends LocalSpillStore { + protected override defaultRootsBase(): string { return fakeTmp } + } + try { + const ctx = new Context() + const fiber = await ctx.plugin(Discovering, { root: activeDefault, cleanupPeriodDays: 30 }) + await fiber.dispose() + expect(existsSync(old)).toBe(false) + } finally { + rmSync(fakeTmp, { recursive: true, force: true }) + } + }) + + it('does not block activation but is awaited on disposal (quiescence)', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + + // Hold the sweep open behind a barrier we control. + let release!: () => void + SweptStore.sweepRoots = [root] + SweptStore.barrier = new Promise((resolve) => { release = resolve }) + + const ctx = new Context() + const fiber = await ctx.plugin(SweptStore, { root, cleanupPeriodDays: 30 }) + // Activation returned while the sweep is still parked: service is usable and + // the old file is untouched so far. + expect(existsSync(old)).toBe(true) + const ref = await ctx.spillStore.saveText(request()) + expect(readFileSync(ref.locator, 'utf8')).toBe('the full body') + + // Disposal must AWAIT the sweep: release the barrier, and dispose only + // settles after the sweep deleted the old file. + release() + await fiber.dispose() + expect(existsSync(old)).toBe(false) + }) + + it('a filesystem failure is contained (logged, never thrown) and does not fail a spill write', async () => { + const warn = vi.fn() + // A path that is a FILE, not a directory: readdir(root) throws ENOTDIR. The + // sweep must log and return, never reject. + const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') + await expect(sweepSpillRoots({ roots: [filePath], cutoffMs: Date.now(), warn })).resolves.toBeUndefined() + expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) + }) + + it('a nonexistent root is silent (the common no-spill-yet case)', async () => { + const warn = vi.fn() + await sweepSpillRoots({ roots: [join(root, 'never-created')], cutoffMs: Date.now(), warn }) + expect(warn).not.toHaveBeenCalled() + }) + + it('a session entry that is a file (not a dir) is reported, not pruned', async () => { + const warn = vi.fn() + // `session-strayfile` matches the session- prefix but is a regular file, so + // readdir on it throws ENOTDIR: reported, left in place (not empty → no prune). + const stray = join(root, 'session-strayfile'); writeFileSync(stray, 'x') + await sweepSpillRoots({ roots: [root], cutoffMs: Date.now(), warn }) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read')) + expect(existsSync(stray)).toBe(true) + }) +}) + +describe('discoverDefaultRoots', () => { + it('returns only real dsh-spill-* directories, excluding symlinks and non-matches', async () => { + const base = mkdtempSync(join(tmpdir(), 'dsh-disc-')) + try { + const realRoot = join(base, `${DEFAULT_ROOT_PREFIX}real`); mkdirSync(realRoot) + mkdirSync(join(base, 'unrelated-dir')) + writeFileSync(join(base, `${DEFAULT_ROOT_PREFIX}file`), 'x') // matches prefix but is a file + symlinkSync(realRoot, join(base, `${DEFAULT_ROOT_PREFIX}link`)) // matches prefix but is a symlink + const found = await discoverDefaultRoots(() => {}, base) + expect(found).toEqual([realRoot]) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + it('returns [] and warns when the base is unreadable', async () => { + const warn = vi.fn() + const missing = join(root, 'no-such-base') + expect(await discoverDefaultRoots(warn, missing)).toEqual([]) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to scan')) + }) +}) + +describe('isErrno', () => { + it('matches a Node system error by code and rejects non-matches', () => { + const err = Object.assign(new Error('boom'), { code: 'ENOENT' }) + expect(isErrno(err, 'ENOENT')).toBe(true) + expect(isErrno(err, 'EPERM')).toBe(false) + expect(isErrno('not an error', 'ENOENT')).toBe(false) + expect(isErrno(new Error('no code'), 'ENOENT')).toBe(false) + }) +}) + From c6a4de620750fa1b84b5b54a121269ec9dad555f Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 20 Jul 2026 12:03:57 +0800 Subject: [PATCH 2/8] fix(spill-local): exact-shape root/session matching and prune discovered roots Tighten the startup sweep to backend-generated name shapes and fix the tests that had drifted from the SweepRoot-based API: - Match roots by the exact `dsh-spill-<6>` mkdtemp shape and session dirs by `session-<12 hex>` (DEFAULT_ROOT_RE / SESSION_DIR_RE), replacing loose startsWith checks so foreign or fixture-shaped directories are never swept. - Carry `SweepRoot { path, pruneWhenEmpty }` through SweepOptions so a discovered prior-default root is removed once emptied while the active root is never pruned; lstat each session entry so a symlinked session dir is not followed. - Fix the tests to the SweepRoot API: import SweepRoot, correct the gatherRoots override return shapes, build discovery fixtures with the real mkdtemp shape, and route the warn-wiring test through a deterministic failure path. --- packages/spill/spill-local/src/index.ts | 22 ++- packages/spill/spill-local/src/store.ts | 111 +++++++++++--- .../spill-local/tests/spill-local.spec.ts | 143 +++++++++++++----- 3 files changed, 210 insertions(+), 66 deletions(-) diff --git a/packages/spill/spill-local/src/index.ts b/packages/spill/spill-local/src/index.ts index 33948712f0..7c4069c539 100644 --- a/packages/spill/spill-local/src/index.ts +++ b/packages/spill/spill-local/src/index.ts @@ -16,10 +16,10 @@ import z from '@deepseek-ai/schemastery' import { SpillLocator, SpillStore } from '@deepseek-ai/dsh-spill' import type { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill' import { discoverDefaultRoots, privateRoot, saveTextFile, sweepSpillRoots } from './store.ts' -import type { WarnFn } from './store.ts' +import type { SweepRoot, WarnFn } from './store.ts' export { discoverDefaultRoots, encodeSegment, isErrno, privateRoot, saveTextFile, sessionDir, sweepSpillRoots, DEFAULT_ROOT_PREFIX } from './store.ts' -export type { SavedText, SaveTextOptions, SweepOptions, WarnFn } from './store.ts' +export type { SavedText, SaveTextOptions, SweepOptions, SweepRoot, WarnFn } from './store.ts' /** Milliseconds in one day — converts the `cleanupPeriodDays` config to the sweep cutoff. */ const MS_PER_DAY = 24 * 60 * 60 * 1000 @@ -115,19 +115,25 @@ export class LocalSpillStore extends SpillStore { } /** - * The roots the startup sweep covers: the prior default `dsh-spill-*` temp - * roots (see {@link discoverDefaultRoots}) plus the configured/active root, - * de-duplicated (the active root may itself be a discovered default). A test + * The roots the startup sweep covers: each discovered prior-default + * `dsh-spill-*` temp root (see {@link discoverDefaultRoots}), pruned when + * emptied, plus the active/configured root, which is swept but NEVER pruned + * (the live process is still writing into it). The active root is de-duped out + * of the discovered set so it is not swept twice or marked prunable. A test * overrides this to inject an isolated root set — and, being the sweep's one * async gather point, to hold the sweep open across a disposal for the * quiescence check; it is a test seam, not a deployment knob. * * @param warn - sink for a contained discovery failure. - * @returns The absolute roots to sweep. + * @returns The roots to sweep, each flagged for prune-when-empty. */ - protected async gatherRoots(warn: WarnFn): Promise { + protected async gatherRoots(warn: WarnFn): Promise { const discovered = await discoverDefaultRoots(warn, this.defaultRootsBase()) - return discovered.includes(this.root) ? discovered : [...discovered, this.root] + const roots: SweepRoot[] = discovered + .filter(path => path !== this.root) + .map(path => ({ path, pruneWhenEmpty: true })) + roots.push({ path: this.root, pruneWhenEmpty: false }) + return roots } /** diff --git a/packages/spill/spill-local/src/store.ts b/packages/spill/spill-local/src/store.ts index 431e4840c6..01528fd573 100644 --- a/packages/spill/spill-local/src/store.ts +++ b/packages/spill/spill-local/src/store.ts @@ -20,6 +20,23 @@ import { tmpdir } from 'node:os' */ export const DEFAULT_ROOT_PREFIX = 'dsh-spill-' +/** + * A backend-generated default root name: `dsh-spill-` plus the 6-character + * suffix `mkdtemp` appends (see {@link privateRoot}). Discovery matches this + * EXACT shape, not the bare prefix, so an unrelated `dsh-spill-test-*` fixture + * or a foreign tool's differently-shaped `dsh-spill-…` directory is never + * mistaken for a backend root to sweep. + */ +const DEFAULT_ROOT_RE = /^dsh-spill-[A-Za-z0-9]{6}$/ + +/** + * A backend-generated session directory name: `session-` plus the 12 lowercase + * hex characters {@link sessionDir} derives from `sha256(sessionId)`. The sweep + * only descends into entries of this EXACT shape, so an unrelated + * `session-backup` directory under a shared configured root is never swept. + */ +const SESSION_DIR_RE = /^session-[0-9a-f]{12}$/ + let defaultRoot: string | undefined /** @@ -126,10 +143,23 @@ export async function saveTextFile(options: SaveTextOptions): Promise /** A one-argument warning sink — the sweep's only side effect on failure (never throws). */ export type WarnFn = (message: string) => void +/** One root to sweep, plus whether an emptied root directory should itself be pruned. */ +export interface SweepRoot { + /** Absolute spill root to sweep. */ + path: string + /** + * When `true`, remove the root directory itself once its last `session-*` + * child is pruned. Set for DISCOVERED prior-default `dsh-spill-*` roots (one + * per past process — otherwise they accumulate empty forever), never for the + * active/configured root the live process is still writing into. + */ + pruneWhenEmpty: boolean +} + /** Options for {@link sweepSpillRoots} — the roots to scan, the age cutoff, and a failure sink. */ export interface SweepOptions { - /** Absolute spill roots to sweep (configured root and/or discovered default roots). */ - roots: string[] + /** Roots to sweep (configured/active root and/or discovered prior-default roots). */ + roots: SweepRoot[] /** * Epoch-millis cutoff: a regular file is deleted when its `mtime` is strictly * older than this. The caller derives it from `now - cleanupPeriodDays`, so a @@ -178,12 +208,14 @@ export function isErrno(error: unknown, code: string): boolean { /** * Sweep one spill session directory: delete expired regular files, skip * everything else, and report the directory empty afterward so the caller can - * prune it. A symlink or any non-regular entry (socket, fifo, nested dir) is - * left untouched — `lstat` never follows a link, so a planted symlink can - * neither be deleted nor redirect the age check. Every per-entry failure is + * prune it. The `dir` entry MUST be a real directory — the caller `lstat`s it + * first and skips a symlink, so this never follows a `session-*` symlink into a + * foreign tree. Inside, a symlink or any non-regular entry (socket, fifo, nested + * dir) is left untouched — `lstat` never follows a link, so a planted symlink + * can neither be deleted nor redirect the age check. Every per-entry failure is * contained: one unreadable file does not abort the directory. * - * @param dir The absolute session directory to scan. + * @param dir The absolute session directory to scan (already confirmed a real dir). * @param cutoffMs Files with `mtime` strictly older than this are deleted. * @param warn Sink for contained filesystem failures. * @returns `true` when the directory holds no entries after the sweep (a prune candidate). @@ -241,20 +273,39 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { for (const root of roots) { let entries: string[] try { - entries = await readdir(root) + entries = await readdir(root.path) } catch (error: unknown) { // A root that does not exist yet (no spill ever written) is the common // case, not an error: ENOENT is silent, anything else is reported. - if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to read root ${root}: ${String(error)}`) + if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to read root ${root.path}: ${String(error)}`) continue } + // Track whether the root holds ANY entry the sweep did not fully reclaim, so + // a discovered prior-default root can be pruned only when nothing remains. + let rootEmptiable = true for (const name of entries) { - // Only the backend's own `session-` directories are swept; an - // unrelated sibling under a shared configured root is left untouched. - if (!name.startsWith('session-')) continue - const dir = join(root, name) + // Only the backend's own `session-<12 hex>` directories are swept; an + // unrelated sibling (`session-backup`, a stray file) is left untouched and + // blocks pruning the root. + if (!SESSION_DIR_RE.test(name)) { rootEmptiable = false; continue } + const dir = join(root.path, name) + let stats + try { + // lstat the session entry itself: a `session-*` SYMLINK must never be + // followed (readdir/unlink through it would delete files in a foreign + // target). Only a real directory is swept. + stats = await lstat(dir) + } catch (error: unknown) { + /* v8 ignore start -- an entry readdir just returned fails to lstat only + by racing away (ENOENT) or a permission/IO fault; not deterministically + reproducible. */ + if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to stat ${dir}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + if (!stats.isDirectory()) { rootEmptiable = false; continue } const empty = await sweepSessionDir(dir, cutoffMs, warn) - if (!empty) continue + if (!empty) { rootEmptiable = false; continue } try { await rmdir(dir) } catch (error: unknown) { @@ -262,22 +313,42 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { here means a concurrent writer added a file (ENOTEMPTY) or a permission/IO fault struck — both are races outside deterministic in-process testing. */ + rootEmptiable = false if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { warn(`spill-local: failed to prune ${dir}: ${String(error)}`) } /* v8 ignore stop */ } } + // A discovered prior-default root (one per past process) is removed once its + // last session dir is gone — otherwise empty roots accumulate forever and + // every future startup rescans them. The active/configured root is never + // pruned (the live process is still writing into it). + if (root.pruneWhenEmpty && rootEmptiable) { + try { + await rmdir(root.path) + } catch (error: unknown) { + // A concurrent process may have written a fresh spill into this root + // after our scan (ENOTEMPTY), or removed it already (ENOENT) — benign + // races. Anything else is reported. + if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { + warn(`spill-local: failed to prune root ${root.path}: ${String(error)}`) + } + } + } } } /** - * Discover prior default spill roots: the `dsh-spill-*` directories directly - * under `base` (the OS tmpdir) that earlier runs created via {@link privateRoot} - * when no `root` was configured. A long-lived deployment with a configured root - * will find none; a series of default-root runs accumulates one per process, so - * the startup sweep reclaims them all. Symlinks and non-directories are excluded - * — only real directories the backend could have created are returned. + * Discover prior default spill roots: the `dsh-spill-<6 chars>` directories + * directly under `base` (the OS tmpdir) that earlier runs created via + * {@link privateRoot} when no `root` was configured. A long-lived deployment + * with a configured root will find none; a series of default-root runs + * accumulates one per process, so the startup sweep reclaims them all. Matching + * is the EXACT `mkdtemp` shape (see {@link DEFAULT_ROOT_RE}), not the bare + * prefix, so an unrelated `dsh-spill-test-*` fixture or a foreign + * differently-shaped directory is never swept; symlinks and non-directories are + * excluded too — only real directories the backend could have created. * * @param warn Sink for a failure reading `base` (returns `[]` on failure). * @param base The directory to scan; defaults to the OS tmpdir (a test seam). @@ -293,7 +364,7 @@ export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir() } const roots: string[] = [] for (const name of entries) { - if (!name.startsWith(DEFAULT_ROOT_PREFIX)) continue + if (!DEFAULT_ROOT_RE.test(name)) continue const path = join(base, name) let stats try { diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index 8e632c885d..41d35824c6 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -27,6 +27,7 @@ import LocalSpillStore, { sessionDir, sweepSpillRoots, } from '@deepseek-ai/dsh-spill-local' +import type { SweepRoot } from '@deepseek-ai/dsh-spill-local' const DAY_MS = 24 * 60 * 60 * 1000 @@ -196,7 +197,7 @@ describe('LocalSpillStore service', () => { // default here (scan the OS tmpdir) without letting the sweep touch tmpdir. class Exposed extends LocalSpillStore { base(): string { return this.defaultRootsBase() } - protected override async gatherRoots(): Promise { return [] } + protected override async gatherRoots(): Promise { return [] } } const ctx = new Context() const fiber = await ctx.plugin(Exposed, { root, cleanupPeriodDays: 30 }) @@ -206,18 +207,18 @@ describe('LocalSpillStore service', () => { }) it('routes a sweep filesystem failure to ctx.logger.warn (service warn wiring)', async () => { - // A `session-*` entry that is a FILE, not a directory, makes readdir throw - // ENOTDIR inside the real sweep. The service's warn closure must forward it - // to ctx.logger.warn, and disposal must still settle cleanly. - const stray = join(root, 'session-stray'); writeFileSync(stray, 'x') + // A root that is a FILE, not a directory, makes readdir throw ENOTDIR inside + // the real sweep. The service's warn closure must forward it to + // ctx.logger.warn, and disposal must still settle cleanly. + const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') const ctx = new Context() const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) class Discovering extends LocalSpillStore { - protected override async gatherRoots(): Promise { return [this.root] } + protected override async gatherRoots(): Promise { return [{ path: this.root, pruneWhenEmpty: false }] } } - const fiber = await ctx.plugin(Discovering, { root, cleanupPeriodDays: 30 }) + const fiber = await ctx.plugin(Discovering, { root: filePath, cleanupPeriodDays: 30 }) await fiber.dispose() - expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) }) }) @@ -227,15 +228,16 @@ describe('LocalSpillStore service', () => { * gather open so a test can prove disposal awaits the sweep. */ class SweptStore extends LocalSpillStore { - static sweepRoots: string[] = [] + static sweepRoots: SweepRoot[] = [] static barrier: Promise | undefined - protected override async gatherRoots(): Promise { + protected override async gatherRoots(): Promise { if (SweptStore.barrier) await SweptStore.barrier return SweptStore.sweepRoots } } -async function runSweep(roots: string[], cleanupPeriodDays = 30): Promise { +/** Sweep the given roots via the fiber-owned startup sweep; `root` is the active (non-pruned) root. */ +async function runSweep(roots: SweepRoot[], cleanupPeriodDays = 30): Promise { SweptStore.sweepRoots = roots SweptStore.barrier = undefined const ctx = new Context() @@ -244,13 +246,18 @@ async function runSweep(roots: string[], cleanupPeriodDays = 30): Promise await fiber.dispose() } +/** The active configured root as a non-pruned sweep target (the common single-root case). */ +function active(path: string): SweepRoot { + return { path, pruneWhenEmpty: false } +} + describe('startup cleanup sweep', () => { it('deletes files older than the cutoff and keeps fresh ones', async () => { const dir = sessionDir(root, 'sess-1') mkdirSync(dir, { recursive: true }) const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) const fresh = join(dir, 'fresh.txt'); writeAged(fresh, 'y', 1) - await runSweep([root]) + await runSweep([active(root)]) expect(existsSync(old)).toBe(false) expect(existsSync(fresh)).toBe(true) }) @@ -261,7 +268,7 @@ describe('startup cleanup sweep', () => { // mtime == cutoff: mtimeMs >= cutoffMs holds, so it is kept. Age it just // under 30d to avoid the sub-millisecond race of "exactly now - 30d". const boundary = join(dir, 'boundary.txt'); writeAged(boundary, 'x', 29.9) - await runSweep([root]) + await runSweep([active(root)]) expect(existsSync(boundary)).toBe(true) }) @@ -269,23 +276,23 @@ describe('startup cleanup sweep', () => { const dir = sessionDir(root, 'sess-1') mkdirSync(dir, { recursive: true }) const old = join(dir, 'old.txt'); writeAged(old, 'x', 400) - await runSweep([root], 0) + await runSweep([active(root)], 0) expect(existsSync(old)).toBe(true) }) - it('prunes a directory left empty, keeps one with a surviving file', async () => { + it('prunes a session directory left empty, keeps one with a surviving file', async () => { const emptied = sessionDir(root, 'emptied') const kept = sessionDir(root, 'kept') mkdirSync(emptied, { recursive: true }) mkdirSync(kept, { recursive: true }) writeAged(join(emptied, 'a.txt'), 'x', 40) writeAged(join(kept, 'fresh.txt'), 'y', 1) - await runSweep([root]) + await runSweep([active(root)]) expect(existsSync(emptied)).toBe(false) expect(existsSync(kept)).toBe(true) }) - it('skips symlinks and non-session entries', async () => { + it('skips a symlink INSIDE a session dir and non-session siblings', async () => { const dir = sessionDir(root, 'sess-1') mkdirSync(dir, { recursive: true }) // A symlink pointing at an old target must NOT be followed or deleted. @@ -294,7 +301,7 @@ describe('startup cleanup sweep', () => { // A non-session sibling directory under a shared root is untouched. const unrelated = join(root, 'not-a-session'); mkdirSync(unrelated) const unrelatedOld = join(unrelated, 'old.txt'); writeAged(unrelatedOld, 'x', 40) - await runSweep([root]) + await runSweep([active(root)]) // The symlink itself survives (lstat sees a link, not a file), so its dir is // not empty and is not pruned; the link target survives too. expect(existsSync(link)).toBe(true) @@ -302,12 +309,73 @@ describe('startup cleanup sweep', () => { expect(existsSync(unrelatedOld)).toBe(true) }) + it('does NOT follow a symlinked session directory (no deletion in the target)', async () => { + // A `session-<12hex>`-NAMED symlink pointing at a directory of old files must + // never be descended: lstat on the entry sees a link, so the target's files + // are left intact and the link itself is not removed. + const victimDir = join(root, 'victim'); mkdirSync(victimDir, { recursive: true }) + const victimOld = join(victimDir, 'old.txt'); writeAged(victimOld, 'x', 40) + const linkName = `session-${'a'.repeat(12)}` + const link = join(root, linkName); symlinkSync(victimDir, link) + await runSweep([active(root)]) + expect(existsSync(victimOld)).toBe(true) + expect(existsSync(link)).toBe(true) + }) + + it('sweeps only exact session-<12hex> names, not lookalikes', async () => { + // `session-backup` and `session-<11hex>` match the old startsWith check but + // are NOT backend-generated names; their old files must survive. + const backup = join(root, 'session-backup'); mkdirSync(backup, { recursive: true }) + const backupOld = join(backup, 'old.txt'); writeAged(backupOld, 'x', 40) + const shortHex = join(root, `session-${'a'.repeat(11)}`); mkdirSync(shortHex, { recursive: true }) + const shortOld = join(shortHex, 'old.txt'); writeAged(shortOld, 'x', 40) + // A real session dir alongside them IS swept, proving the sweep still runs. + const real = sessionDir(root, 'sess-1'); mkdirSync(real, { recursive: true }) + const realOld = join(real, 'old.txt'); writeAged(realOld, 'x', 40) + await runSweep([active(root)]) + expect(existsSync(backupOld)).toBe(true) + expect(existsSync(shortOld)).toBe(true) + expect(existsSync(realOld)).toBe(false) + }) + + it('prunes an emptied DISCOVERED default root but never the active root', async () => { + // A discovered prior-default root (pruneWhenEmpty) whose only session dir is + // emptied should have its outer directory removed too; the active root, even + // when fully emptied, must survive (the live process still writes into it). + const prior = mkdtempSync(join(tmpdir(), 'dsh-spill-')) + const priorDir = sessionDir(prior, 'old-sess'); mkdirSync(priorDir, { recursive: true }) + writeAged(join(priorDir, 'old.txt'), 'x', 40) + const activeDir = sessionDir(root, 'sess-1'); mkdirSync(activeDir, { recursive: true }) + writeAged(join(activeDir, 'old.txt'), 'x', 40) + try { + await runSweep([{ path: prior, pruneWhenEmpty: true }, active(root)]) + expect(existsSync(prior)).toBe(false) // discovered root pruned + expect(existsSync(root)).toBe(true) // active root kept + expect(existsSync(activeDir)).toBe(false) // its emptied session dir still pruned + } finally { + rmSync(prior, { recursive: true, force: true }) + } + }) + + it('does NOT prune a discovered root that still holds a fresh file', async () => { + const prior = mkdtempSync(join(tmpdir(), 'dsh-spill-')) + const priorDir = sessionDir(prior, 'sess'); mkdirSync(priorDir, { recursive: true }) + writeAged(join(priorDir, 'fresh.txt'), 'y', 1) + try { + await runSweep([{ path: prior, pruneWhenEmpty: true }]) + expect(existsSync(prior)).toBe(true) + expect(existsSync(priorDir)).toBe(true) + } finally { + rmSync(prior, { recursive: true, force: true }) + } + }) + it('covers the configured root AND discovered default roots (real gatherRoots)', async () => { // A prior default root under an isolated fake tmpdir + the configured root. // This test drives the REAL gatherRoots/discoverDefaultRoots path by seaming // only the tmpdir scan base, not gatherRoots itself. const fakeTmp = mkdtempSync(join(tmpdir(), 'dsh-faketmp-')) - const priorDefault = join(fakeTmp, `${DEFAULT_ROOT_PREFIX}ABCDEF`) + const priorDefault = mkdtempSync(join(fakeTmp, DEFAULT_ROOT_PREFIX)) const priorDir = sessionDir(priorDefault, 'old-sess') mkdirSync(priorDir, { recursive: true }) const priorOld = join(priorDir, 'old.txt'); writeAged(priorOld, 'x', 40) @@ -323,6 +391,9 @@ describe('startup cleanup sweep', () => { await fiber.dispose() expect(existsSync(priorOld)).toBe(false) expect(existsSync(cfgOld)).toBe(false) + // The discovered prior-default root is pruned; the configured root is kept. + expect(existsSync(priorDefault)).toBe(false) + expect(existsSync(root)).toBe(true) } finally { rmSync(fakeTmp, { recursive: true, force: true }) } @@ -330,10 +401,11 @@ describe('startup cleanup sweep', () => { it('de-dups when the active root is itself a discovered default (real gatherRoots)', async () => { // The configured root lives directly under the seamed base and matches the - // default prefix, so discovery finds it AND it is the active root — the sweep - // must run once, not choke on the duplicate. + // default shape, so discovery finds it AND it is the active root — the sweep + // must run once, not choke on the duplicate, and must NOT prune the active + // root even though discovery would otherwise mark a default root prunable. const fakeTmp = mkdtempSync(join(tmpdir(), 'dsh-faketmp-')) - const activeDefault = join(fakeTmp, `${DEFAULT_ROOT_PREFIX}ACTIVE`) + const activeDefault = mkdtempSync(join(fakeTmp, DEFAULT_ROOT_PREFIX)) const dir = sessionDir(activeDefault, 'sess-1') mkdirSync(dir, { recursive: true }) const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) @@ -345,6 +417,8 @@ describe('startup cleanup sweep', () => { const fiber = await ctx.plugin(Discovering, { root: activeDefault, cleanupPeriodDays: 30 }) await fiber.dispose() expect(existsSync(old)).toBe(false) + // Active root survives even though its name matches the discovered shape. + expect(existsSync(activeDefault)).toBe(true) } finally { rmSync(fakeTmp, { recursive: true, force: true }) } @@ -357,7 +431,7 @@ describe('startup cleanup sweep', () => { // Hold the sweep open behind a barrier we control. let release!: () => void - SweptStore.sweepRoots = [root] + SweptStore.sweepRoots = [active(root)] SweptStore.barrier = new Promise((resolve) => { release = resolve }) const ctx = new Context() @@ -380,35 +454,28 @@ describe('startup cleanup sweep', () => { // A path that is a FILE, not a directory: readdir(root) throws ENOTDIR. The // sweep must log and return, never reject. const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') - await expect(sweepSpillRoots({ roots: [filePath], cutoffMs: Date.now(), warn })).resolves.toBeUndefined() + await expect(sweepSpillRoots({ roots: [active(filePath)], cutoffMs: Date.now(), warn })).resolves.toBeUndefined() expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) }) it('a nonexistent root is silent (the common no-spill-yet case)', async () => { const warn = vi.fn() - await sweepSpillRoots({ roots: [join(root, 'never-created')], cutoffMs: Date.now(), warn }) + await sweepSpillRoots({ roots: [active(join(root, 'never-created'))], cutoffMs: Date.now(), warn }) expect(warn).not.toHaveBeenCalled() }) - - it('a session entry that is a file (not a dir) is reported, not pruned', async () => { - const warn = vi.fn() - // `session-strayfile` matches the session- prefix but is a regular file, so - // readdir on it throws ENOTDIR: reported, left in place (not empty → no prune). - const stray = join(root, 'session-strayfile'); writeFileSync(stray, 'x') - await sweepSpillRoots({ roots: [root], cutoffMs: Date.now(), warn }) - expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read')) - expect(existsSync(stray)).toBe(true) - }) }) describe('discoverDefaultRoots', () => { it('returns only real dsh-spill-* directories, excluding symlinks and non-matches', async () => { const base = mkdtempSync(join(tmpdir(), 'dsh-disc-')) try { - const realRoot = join(base, `${DEFAULT_ROOT_PREFIX}real`); mkdirSync(realRoot) + // A real backend-shaped root (dsh-spill-<6>) via mkdtemp — the only match. + const realRoot = mkdtempSync(join(base, DEFAULT_ROOT_PREFIX)) mkdirSync(join(base, 'unrelated-dir')) - writeFileSync(join(base, `${DEFAULT_ROOT_PREFIX}file`), 'x') // matches prefix but is a file - symlinkSync(realRoot, join(base, `${DEFAULT_ROOT_PREFIX}link`)) // matches prefix but is a symlink + // Names of the EXACT default shape that must still be excluded because they + // are not real directories the backend could have created. + writeFileSync(join(base, `${DEFAULT_ROOT_PREFIX}file01`), 'x') // matches shape but is a file + symlinkSync(realRoot, join(base, `${DEFAULT_ROOT_PREFIX}link01`)) // matches shape but is a symlink const found = await discoverDefaultRoots(() => {}, base) expect(found).toEqual([realRoot]) } finally { From dbb3bcca8e0365874bda21566dd4233a87bba5f1 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 20 Jul 2026 14:55:21 +0800 Subject: [PATCH 3/8] test(spill-local): v8-ignore the two race-only sweep catch branches The exact-shape fix added two filesystem-failure catch branches that only fire on a race/permission fault the caller already guards against (the session-dir readdir after an isDirectory() check, and the discovered-root rmdir after the root was observed empty). Neither is deterministically reproducible in-process, so tag both with the same reasoned v8 ignore the sibling catch blocks already use, restoring per-file 100% coverage and the symmetry between the parallel rmdir handlers. --- packages/spill/spill-local/src/store.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/spill/spill-local/src/store.ts b/packages/spill/spill-local/src/store.ts index 01528fd573..0303a8f639 100644 --- a/packages/spill/spill-local/src/store.ts +++ b/packages/spill/spill-local/src/store.ts @@ -225,11 +225,13 @@ async function sweepSessionDir(dir: string, cutoffMs: number, warn: WarnFn): Pro try { names = await readdir(dir) } catch (error: unknown) { - // A `session-*` entry that is not a readable directory (a stray file, or an - // unreadable/vanished dir) is not ours to fix — report and leave it. False - // keeps it out of the prune step. + /* v8 ignore start -- the caller lstat'd this entry and confirmed a real + directory just before the call, so readdir fails only when the dir races + away (ENOENT) or a permission/IO fault strikes in that window; not + deterministically reproducible. False keeps it out of the prune step. */ warn(`spill-local: failed to read ${dir}: ${String(error)}`) return false + /* v8 ignore stop */ } let remaining = names.length for (const name of names) { @@ -328,12 +330,15 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { try { await rmdir(root.path) } catch (error: unknown) { - // A concurrent process may have written a fresh spill into this root - // after our scan (ENOTEMPTY), or removed it already (ENOENT) — benign - // races. Anything else is reported. + /* v8 ignore start -- prune runs only on a root whose every child was + reclaimed; a failure here means a concurrent writer added a fresh + spill after our scan (ENOTEMPTY) or removed the root already (ENOENT) + or a permission/IO fault struck — all races outside deterministic + in-process testing. */ if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { warn(`spill-local: failed to prune root ${root.path}: ${String(error)}`) } + /* v8 ignore stop */ } } } From 545d1779112d945a39ccda1dd833e337e3656c33 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 20 Jul 2026 15:55:17 +0800 Subject: [PATCH 4/8] fix(spill-local): make startup cleanup race-safe --- ...26-07-08-tool-output-spill-files.i18n.yaml | 4 +- .../2026-07-08-tool-output-spill-files.md | 2 +- .../2026-07-08-tool-output-spill-files.zh.md | 3 +- ...7-17-local-spill-startup-cleanup.i18n.yaml | 6 +- .../2026-07-17-local-spill-startup-cleanup.md | 6 +- ...26-07-17-local-spill-startup-cleanup.zh.md | 8 +- docs/config-catalog.i18n.yaml | 4 +- docs/config-catalog.md | 2 +- docs/config-catalog.zh.md | 11 +- packages/spill/spill-local/README.i18n.yaml | 4 +- packages/spill/spill-local/README.md | 2 +- packages/spill/spill-local/README.zh.md | 11 +- packages/spill/spill-local/src/cleanup.ts | 276 ++++++++++++++ packages/spill/spill-local/src/index.ts | 18 +- packages/spill/spill-local/src/store.ts | 353 +++--------------- .../spill-local/tests/spill-local.spec.ts | 20 +- 16 files changed, 387 insertions(+), 343 deletions(-) create mode 100644 packages/spill/spill-local/src/cleanup.ts diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml index 5ba89a4412..20f4e54eae 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md -2026-07-08-tool-output-spill-files.md: 14667b74ca877622d05196e9bf83945a842fe366 -2026-07-08-tool-output-spill-files.zh.md: db297fa6bee707a1d5a10d20260ce6b8a660d207 +2026-07-08-tool-output-spill-files.md: 81a292a00af63b0aa9145a0c556a428ab8c49d64 +2026-07-08-tool-output-spill-files.zh.md: 8d9e60d461297fb11ff2252e91f98a0cfad33f62 diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md index 6164066dbb..81a292a00a 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md @@ -161,7 +161,7 @@ Those cases can consume `ctx.spillStore` directly in later work. They are not pa - Per-tool opt-out or per-tool policy declarations if the built-in `read` skip is insufficient. - Remote or database storage backends for ACP or remote environments where a local path is not meaningful. -Cleanup shipped for the local backend as a one-shot startup sweep, not tied to session deletion — see the [startup-cleanup RFC](./2026-07-17-local-spill-startup-cleanup.md). The seam still defines no per-session cleanup policy; retention is a backend concern. +Cleanup shipped for the local backend as a one-shot startup sweep, not tied to session deletion — see the [startup-cleanup Agent Note](./2026-07-17-local-spill-startup-cleanup.md). The seam still defines no per-session cleanup policy; retention is a backend concern. ## Testing diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md index db297fa6be..8d9e60d461 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md @@ -160,7 +160,8 @@ ctx.tools.register(defineTool({ - 由工具负责的 subagent 执行轨迹 spill（`await run.result`，在 `run.dispose()` 前读取进程内子会话，保存 JSONL）。 - 如果内置的 `read` 跳过规则不足，再增加逐工具选择退出或逐工具策略声明。 - 面向 ACP（Agent Client Protocol）或远程环境的远程／数据库存储后端，因为本地路径在这些环境中没有意义。 -- 旧 spill 文件的清理和保留策略，很可能与会话清理绑定。 + +本地后端通过一次性启动扫描清理旧文件，而不是绑定到会话删除——参见[启动清理 Agent Note](./2026-07-17-local-spill-startup-cleanup.zh.md)。seam 仍未定义逐会话清理策略；保留策略属于后端。 ## 测试 diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml index 97d2b5426b..511dda5a74 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml @@ -1,6 +1,6 @@ # Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: -# pnpm run verify-translation-pairing --write -2026-07-17-local-spill-startup-cleanup.md: ca4931776f89e641f127072f665e238ca2a1600d -2026-07-17-local-spill-startup-cleanup.zh.md: b90923844ab71e1ce570e8adb66f81aed7bc3488 +# pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md +2026-07-17-local-spill-startup-cleanup.md: 96378d6ea785d90385f517c1b9a01073ade47fa2 +2026-07-17-local-spill-startup-cleanup.zh.md: a154cfb824d3a747c2c9acc2b707eb14d703ce2e diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md index ca4931776f..96378d6ea7 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md @@ -12,9 +12,9 @@ The local spill backend never deleted the full tool results it wrote. Every over `dsh-spill-local` runs one best-effort cleanup sweep after activation. It does not delay service availability, is owned by the plugin fiber (a single `ctx.effect` whose generator launches the sweep and yields an async disposer that awaits it), and is awaited during disposal so no sweep I/O outlives the fiber. There is no recurring timer and no separate process. -A `cleanupPeriodDays` config defaults to `30`; `0` disables cleanup. An invalid value (negative or fractional) throws at load. The sweep scans the configured/active root plus any prior default `dsh-spill-*` temp roots discovered under the OS temp dir, deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`, and prunes directories left empty. It uses `lstat`, so a symlink is never followed or deleted; unrelated entries (non-`session-` directories, special files) are skipped. Every filesystem failure is caught and logged through `ctx.logger.warn` — the sweep never throws, so it cannot reject activation or a concurrent spill write. Discovery excludes symlinks and non-directories, returning only real `dsh-spill-*` directories the backend could have created. +A `cleanupPeriodDays` config defaults to `30`; `0` disables cleanup. An invalid value (negative or fractional) throws at load. The sweep scans the configured/active root plus any prior default `dsh-spill-*` temp roots discovered under the OS temp dir and deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`. It prunes empty session directories and roots only for discovered prior-default roots; the active root keeps its session directories so pruning cannot race a local write, while writes recreate a session directory if another process prunes a discovered root that is still active. It uses `lstat`, so a symlink is never followed or deleted; unrelated entries (non-`session-` directories, special files) are skipped. Every filesystem failure is caught and logged through `ctx.logger.warn`, and a warning-sink exception is also contained — the sweep never throws, so it cannot reject activation or a concurrent spill write. Discovery excludes symlinks and non-directories, returning only real `dsh-spill-*` directories the backend could have created. -The ctx-free mechanics live in `packages/spill/spill-local/src/store.ts` (`sweepSpillRoots`, `discoverDefaultRoots`, `DEFAULT_ROOT_PREFIX`, `isErrno`), unit-testable without a `ctx`; the service in `src/index.ts` owns the config, the cutoff, and the fiber-owned launch/await. +The ctx-free sweep mechanics live in `packages/spill/spill-local/src/cleanup.ts` (`sweepSpillRoots`, `discoverDefaultRoots`), unit-testable without a `ctx`; `store.ts` owns root naming, path derivation, and writes, while the service in `src/index.ts` owns the config, cutoff, and fiber-owned launch/await. ## Alternatives considered @@ -32,4 +32,4 @@ Cleanup cost the backend a startup sweep and a config knob, and bought a bounded ## Testing -`dsh-spill-local` unit tests cover the age boundary (strictly-older expires, boundary kept), `cleanupPeriodDays: 0` disabling, empty-directory pruning, symlink/unrelated-entry skipping, configured-plus-discovered-root coverage through the real `gatherRoots`/`discoverDefaultRoots` path, active-root de-duplication, load-time validation of a bad `cleanupPeriodDays`, filesystem-failure containment (logged, not thrown) both directly and through the service's `ctx.logger.warn` wiring, and the quiescence contract — activation is available while a barrier-held sweep is parked, and disposal only settles after the sweep finishes. +`dsh-spill-local` unit tests cover the age boundary (strictly-older expires, boundary kept), `cleanupPeriodDays: 0` disabling, discovered-root pruning, active-directory preservation, symlink/unrelated-entry skipping, configured-plus-discovered-root coverage through the real `gatherRoots`/`discoverDefaultRoots` path, active-root de-duplication, load-time validation of a bad `cleanupPeriodDays`, filesystem- and warning-sink-failure containment both directly and through the service's `ctx.logger.warn` wiring, and the quiescence contract — activation is available while a barrier-held sweep is parked, and disposal only settles after the sweep finishes. diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md index b90923844a..a154cfb824 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md @@ -6,15 +6,15 @@ Status: implemented ## 问题 -本地 spill 后端从不删除它写下的完整工具结果。每个超限结果都会新增一个文件，因此配置的根目录会无限增长，而每进程默认的 `dsh-spill-*` 根目录也会跨多次运行不断累积。立即删除是错误的，因为已持久化、已恢复和已 fork 的会话仍可能引用某个 locator。[工具输出 spill 策略](./2026-07-08-tool-output-spill-files.md)需要一个有界的本地存储生命周期。 +本地 spill 后端从不删除它写下的完整工具结果。每个超限结果都会新增一个文件，因此配置的根目录会无限增长，而每进程默认的 `dsh-spill-*` 根目录也会跨多次运行不断累积。立即删除是错误的，因为已持久化、已恢复和已 fork 的会话仍可能引用某个 locator。[工具输出 spill 策略](./2026-07-08-tool-output-spill-files.zh.md)需要一个有界的本地存储生命周期。 ## 决策 `dsh-spill-local` 在激活后运行一次尽力而为的清理扫描。它不延迟服务可用性，由插件 fiber 拥有（一个 `ctx.effect`，其生成器启动该扫描并让出一个等待它的异步 disposer），并在 dispose 期间被等待，因此没有扫描 I/O 会存活到 fiber 之后。既没有周期性定时器，也没有独立进程。 -`cleanupPeriodDays` 配置默认为 `30`；`0` 会禁用清理。无效值（负数或小数）在加载时抛出。扫描会遍历配置的/活动的根目录，以及在 OS 临时目录下发现的任何先前默认 `dsh-spill-*` 临时根目录，删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件，并修剪清空后的目录。它使用 `lstat`，因此符号链接绝不会被跟随或删除；无关条目（非 `session-` 目录、特殊文件）会被跳过。每一次文件系统失败都会被捕获并通过 `ctx.logger.warn` 记录——扫描绝不抛出，因此它无法让激活失败，也无法影响并发的 spill 写入。发现过程排除符号链接与非目录，只返回后端可能创建过的真实 `dsh-spill-*` 目录。 +`cleanupPeriodDays` 配置默认为 `30`；`0` 会禁用清理。无效值（负数或小数）在加载时抛出。扫描会遍历配置的/活动的根目录，以及在 OS 临时目录下发现的任何先前默认 `dsh-spill-*` 临时根目录，并删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件。它只修剪发现的先前默认根目录中的空会话目录和空根目录；活动根目录会保留其会话目录，避免修剪操作与本地写入竞争，而当其他进程修剪了一个仍在使用的发现根目录时，写入操作会重新创建会话目录。扫描使用 `lstat`，因此符号链接绝不会被跟随或删除；无关条目（非 `session-` 目录、特殊文件）会被跳过。每一次文件系统失败都会被捕获并通过 `ctx.logger.warn` 记录，警告接收方抛出的异常也会被兜底——扫描绝不抛出，因此它无法让激活失败，也无法影响并发的 spill 写入。发现过程排除符号链接与非目录，只返回后端可能创建过的真实 `dsh-spill-*` 目录。 -无 ctx 依赖的机制位于 `packages/spill/spill-local/src/store.ts`（`sweepSpillRoots`、`discoverDefaultRoots`、`DEFAULT_ROOT_PREFIX`、`isErrno`），无需 `ctx` 即可做单元测试；`src/index.ts` 中的服务负责配置、截止时间以及 fiber 拥有的启动/等待。 +无 ctx 依赖的扫描机制位于 `packages/spill/spill-local/src/cleanup.ts`（`sweepSpillRoots`、`discoverDefaultRoots`），无需 `ctx` 即可做单元测试；`store.ts` 负责根目录命名、路径推导与写入，而 `src/index.ts` 中的服务负责配置、截止时间以及 fiber 拥有的启动/等待。 ## 考虑过的替代方案 @@ -32,4 +32,4 @@ Status: implemented ## 验证 -`dsh-spill-local` 单元测试覆盖了年龄边界（严格更旧者过期，边界值保留）、`cleanupPeriodDays: 0` 的禁用、空目录修剪、符号链接/无关条目的跳过、通过真实 `gatherRoots`/`discoverDefaultRoots` 路径对配置根加发现根的覆盖、活动根去重、对错误 `cleanupPeriodDays` 的加载期校验、文件系统失败的兜底（记录而非抛出）——既直接测试，也经由服务的 `ctx.logger.warn` 接线测试——以及静止契约：在一个被屏障挂起的扫描停驻期间激活仍然可用，而 dispose 只有在扫描结束后才会完成。 +`dsh-spill-local` 单元测试覆盖了年龄边界（严格更旧者过期，边界值保留）、`cleanupPeriodDays: 0` 的禁用、发现根目录的修剪、活动目录的保留、符号链接/无关条目的跳过、通过真实 `gatherRoots`/`discoverDefaultRoots` 路径对配置根加发现根的覆盖、活动根去重、对错误 `cleanupPeriodDays` 的加载期校验、直接测试以及经由服务的 `ctx.logger.warn` 接线测试所覆盖的文件系统与警告接收方失败兜底，以及静止契约：在一个被屏障挂起的扫描停驻期间激活仍然可用，而 dispose 只有在扫描结束后才会完成。 diff --git a/docs/config-catalog.i18n.yaml b/docs/config-catalog.i18n.yaml index aed3574106..66d5592ba2 100644 --- a/docs/config-catalog.i18n.yaml +++ b/docs/config-catalog.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write docs/config-catalog.md -config-catalog.md: a845fe22e13ed085765668c7ec8d54d6bbdf129a -config-catalog.zh.md: 39ba9d48368f99483733292f997609ba3a8aa43e +config-catalog.md: 9fc8c333510c568686ce43f4aa1f6d0ae6bc3615 +config-catalog.zh.md: 4fb689f63631740d485a854e10a12c6d92c6f4ac diff --git a/docs/config-catalog.md b/docs/config-catalog.md index ac7c00738c..9fc8c33351 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -2074,7 +2074,7 @@ export interface Config { } ``` -Source: [`packages/spill/spill-local/src/index.ts:28`](../packages/spill/spill-local/src/index.ts) +Source: [`packages/spill/spill-local/src/index.ts:31`](../packages/spill/spill-local/src/index.ts) diff --git a/docs/config-catalog.zh.md b/docs/config-catalog.zh.md index 39ba9d4836..4fb689f636 100644 --- a/docs/config-catalog.zh.md +++ b/docs/config-catalog.zh.md @@ -2064,10 +2064,19 @@ export interface Config { * a local deployment. Set it to keep spill files under a known location. */ root?: string + /** + * Age in days after which a spill file is eligible for the one-shot startup + * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose + * `mtime` is strictly older than the cutoff are deleted and emptied + * directories are pruned; fresh files, symlinks, and unrelated entries are + * left untouched. Retention is deliberate — a resumed or forked session may + * still reference an older locator until it ages out. + */ + cleanupPeriodDays?: number } ``` -来源：[`packages/spill/spill-local/src/index.ts:22`](../packages/spill/spill-local/src/index.ts) +来源：[`packages/spill/spill-local/src/index.ts:31`](../packages/spill/spill-local/src/index.ts) diff --git a/packages/spill/spill-local/README.i18n.yaml b/packages/spill/spill-local/README.i18n.yaml index 37659c3aac..dd414de9b9 100644 --- a/packages/spill/spill-local/README.i18n.yaml +++ b/packages/spill/spill-local/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/spill/spill-local/README.md -README.md: 2270a65d9270e1549a9e48d6a36b821e48c29070 -README.zh.md: b3e4999d8f2d982ef01637199299f79d06e12c4b +README.md: 75bde20c423e1d2aa4bba49201b5bb0369d34fd0 +README.zh.md: 0539969cc4bd4da8dad4f0ac00436476555fd08c diff --git a/packages/spill/spill-local/README.md b/packages/spill/spill-local/README.md index c97ddf89cd..75bde20c42 100644 --- a/packages/spill/spill-local/README.md +++ b/packages/spill/spill-local/README.md @@ -23,7 +23,7 @@ Files land at `/session-/​-`: The backend never deletes a spill on the write path — a persisted, resumed, or forked session may still reference an older locator, so immediate deletion would break retrieval. Instead, one best-effort sweep runs **once after activation**: it does not delay service availability, is owned by the plugin fiber, and is awaited on disposal (no sweep I/O outlives the fiber). There is no recurring timer and no separate process, so a long-lived deployment is not swept again until its next restart. -The sweep scans the configured `root` **and** any earlier default `dsh-spill-*` temp roots that prior default-root runs left under the OS temp dir. Within each, it deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays` and prunes any directory left empty. It never follows or deletes a symlink, skips unrelated entries, and contains every filesystem failure (logged, never thrown) so it cannot fail activation or a concurrent spill write. Retention is deliberate: an old model-visible locator goes stale only once it ages past the cutoff. +The sweep scans the configured `root` **and** any earlier default `dsh-spill-*` temp roots that prior default-root runs left under the OS temp dir. Within each, it deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`; it prunes empty session directories and roots only for discovered prior-default roots, while the active root keeps its session directories to avoid racing a write. A write recreates its session directory if another process prunes a discovered root that is still active. The sweep never follows or deletes a symlink, skips unrelated entries, and contains every filesystem or warning-sink failure so it cannot fail activation or a concurrent spill write. Retention is deliberate: an old model-visible locator goes stale only once it ages past the cutoff. `saveText` rejects on a real storage failure (permissions, ENOSPC); the spill policy treats a rejection as best-effort and keeps the inline result. See the seam README for the vocabulary and the [tool output spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md) for the design, and the [startup-cleanup Agent Note](../../../.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md) for the sweep. diff --git a/packages/spill/spill-local/README.zh.md b/packages/spill/spill-local/README.zh.md index b3e4999d8f..0539969cc4 100644 --- a/packages/spill/spill-local/README.zh.md +++ b/packages/spill/spill-local/README.zh.md @@ -17,8 +17,15 @@ | 键 | 默认值 | 含义 | |---|---|---| | `root` | 私有 0700 临时目录 | spill 文件的根目录。设置后可将这些文件保存在已知位置。 | +| `cleanupPeriodDays` | `30` | spill 文件在一次性启动清理扫描中符合删除条件前需经过的天数。`0` 禁用清理。 | -`saveText` 在发生真实存储故障（权限、ENOSPC）时返回拒绝；spill 策略会按尽力而为原则处理该拒绝，并保留内联结果。词汇见 seam README，设计见[工具输出 spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md)。 +## 启动清理 + +后端不会在写入路径上删除 spill，因为已持久化、已恢复或 fork 后的会话仍可能引用较旧的定位信息，立即删除会使其无法取回。后端会改为在激活后**仅运行一次**尽力而为的扫描：扫描不延迟服务可用性，由插件 fiber 拥有，并在 dispose 期间被等待（不会有扫描 I/O 存活至 fiber 之后）。它既不使用周期性定时器，也不运行独立进程，因此长期运行的部署要到下次重启才会再次扫描。 + +扫描会检查配置的 `root` **以及**先前使用默认根目录的运行在操作系统临时目录下留下的所有 `dsh-spill-*` 临时根目录。在每个根目录中，扫描会删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件；它只修剪发现的先前默认根目录中的空会话目录和空根目录，而活动根目录会保留其会话目录，以避免与写入操作竞争。如果另一个进程修剪了一个仍在使用的发现根目录，写入操作会重新创建其会话目录。扫描绝不会跟随或删除符号链接，会跳过无关条目，并兜底每一次文件系统或警告接收方失败，因此无法使激活或并发 spill 写入失败。保留是刻意的：旧的模型可见定位信息只有超过截止时间后才会失效。 + +`saveText` 在发生真实存储故障（权限、ENOSPC）时返回拒绝；spill 策略会按尽力而为原则处理该拒绝，并保留内联结果。词汇见 seam README，设计见[工具输出 spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md)，扫描机制见[启动清理 Agent Note](../../../.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md)。 ## 模型体验 @@ -30,5 +37,5 @@ ## 已知限制与暂缓事项 -- **本地 spill 文件会持续存在，直到外部清理为止**：该后端不提供会话生命周期删除或按时间保留的策略，因为已持久化、已恢复和 fork 后的会话可能仍在引用某个路径。 +- **长期运行的部署需等到重启才会被扫描**：一次性扫描仅在激活后运行一次，因此运行期间达到 `cleanupPeriodDays` 的文件要到下次启动才会被回收；没有周期性定时器。 - **定位信息需要与其位于同一文件系统的消费方**：远程或虚拟部署需要另一个 `SpillStore` 后端，其定位信息和取回指引在该环境中有明确含义。 diff --git a/packages/spill/spill-local/src/cleanup.ts b/packages/spill/spill-local/src/cleanup.ts new file mode 100644 index 0000000000..c138f68a84 --- /dev/null +++ b/packages/spill/spill-local/src/cleanup.ts @@ -0,0 +1,276 @@ +/** Startup cleanup mechanics for local spill roots. */ +import { lstat, readdir, rmdir, unlink } from 'node:fs/promises' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { DEFAULT_ROOT_PREFIX, isErrno } from './store.ts' + +/** + * A backend-generated default root name: `dsh-spill-` plus the 6-character + * suffix `mkdtemp` appends. Discovery matches this + * EXACT shape, not the bare prefix, so an unrelated `dsh-spill-test-*` fixture + * or a foreign tool's differently-shaped `dsh-spill-…` directory is never + * mistaken for a backend root to sweep. + */ +const DEFAULT_ROOT_RE = new RegExp(`^${DEFAULT_ROOT_PREFIX}[A-Za-z0-9]{6}$`) + +/** + * A backend-generated session directory name: `session-` plus the 12 lowercase + * hex characters {@link sessionDir} derives from `sha256(sessionId)`. The sweep + * only descends into entries of this EXACT shape, so an unrelated + * `session-backup` directory under a shared configured root is never swept. + */ +const SESSION_DIR_RE = /^session-[0-9a-f]{12}$/ + +/** A one-argument warning sink — the sweep's only side effect on failure (never throws). */ +export type WarnFn = (message: string) => void + +/** Report a best-effort sweep failure without allowing the warning sink to reject cleanup. */ +function warnSafely(warn: WarnFn, message: string): void { + try { + warn(message) + } catch { + // Warning sinks are observational callbacks; cleanup must remain best-effort + // even when a logger implementation throws. + } +} + +/** One root to sweep, plus whether its empty session directories and root may be pruned. */ +export interface SweepRoot { + /** Absolute spill root to sweep. */ + path: string + /** + * When `true`, prune empty `session-*` children and then remove the root once + * empty. Set for DISCOVERED prior-default `dsh-spill-*` roots (one per past + * process — otherwise they accumulate empty forever), never for the + * active/configured root the live process is still writing into. Writes retry + * if another process still using a discovered root races its pruning. + */ + pruneWhenEmpty: boolean +} + +/** Options for {@link sweepSpillRoots} — the roots to scan, the age cutoff, and a failure sink. */ +export interface SweepOptions { + /** Roots to sweep (configured/active root and/or discovered prior-default roots). */ + roots: SweepRoot[] + /** + * Epoch-millis cutoff: a regular file is deleted when its `mtime` is strictly + * older than this. The caller derives it from `now - cleanupPeriodDays`, so a + * file written exactly at the boundary is kept (only strictly-older expires). + */ + cutoffMs: number + /** Where a contained filesystem failure is reported; the sweep itself never throws. */ + warn: WarnFn +} + +/** + * Delete a single path, treating a concurrent-race disappearance as success. + * A parallel process (or another sweep) may `unlink` the same file between our + * scan and our own `unlink` — ENOENT then means the goal (file gone) already + * holds, so it is not a failure. Any other error is reported and swallowed. + * + * @param path The absolute file path to remove. + * @param warn Sink for a non-ENOENT failure message. + * @returns Resolves once the removal was attempted (never rejects). + */ +async function unlinkIdempotent(path: string, warn: WarnFn): Promise { + try { + await unlink(path) + } catch (error: unknown) { + /* v8 ignore start -- reached only when a file selected for deletion (a + regular file that passed lstat) then fails to unlink: either it raced away + (ENOENT) or a permission/IO fault struck between the stat and the unlink. + Neither is deterministically reproducible in-process. */ + if (isErrno(error, 'ENOENT')) return + warnSafely(warn, `spill-local: failed to delete ${path}: ${String(error)}`) + /* v8 ignore stop */ + } +} + +/** + * Sweep one spill session directory: delete expired regular files, skip + * everything else, and report the directory empty afterward so the caller can + * prune it. The `dir` entry MUST be a real directory — the caller `lstat`s it + * first and skips a symlink, so this never follows a `session-*` symlink into a + * foreign tree. Inside, a symlink or any non-regular entry (socket, fifo, nested + * dir) is left untouched — `lstat` never follows a link, so a planted symlink + * can neither be deleted nor redirect the age check. Every per-entry failure is + * contained: one unreadable file does not abort the directory. + * + * @param dir The absolute session directory to scan (already confirmed a real dir). + * @param cutoffMs Files with `mtime` strictly older than this are deleted. + * @param warn Sink for contained filesystem failures. + * @returns `true` when the directory holds no entries after the sweep (a prune candidate). + */ +async function sweepSessionDir(dir: string, cutoffMs: number, warn: WarnFn): Promise { + let names: string[] + try { + names = await readdir(dir) + } catch (error: unknown) { + /* v8 ignore start -- the caller lstat'd this entry and confirmed a real + directory just before the call, so readdir fails only when the dir races + away (ENOENT) or a permission/IO fault strikes in that window; not + deterministically reproducible. False keeps it out of the prune step. */ + warnSafely(warn, `spill-local: failed to read ${dir}: ${String(error)}`) + return false + /* v8 ignore stop */ + } + let remaining = names.length + for (const name of names) { + const path = join(dir, name) + let stats + try { + stats = await lstat(path) + } catch (error: unknown) { + /* v8 ignore start -- an entry that readdir just returned then fails to + lstat only by racing away (ENOENT) or a permission/IO fault; keep it out + of the deterministic test surface. */ + if (isErrno(error, 'ENOENT')) { remaining--; continue } + warnSafely(warn, `spill-local: failed to stat ${path}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + // Only regular files expire. Symlinks and other special entries are skipped + // (never followed) so the sweep cannot be redirected or delete a link. + if (!stats.isFile()) continue + if (stats.mtimeMs >= cutoffMs) continue + await unlinkIdempotent(path, warn) + remaining-- + } + return remaining === 0 +} + +/** + * Best-effort one-shot cleanup: across each root, delete expired regular files + * under its `session-*` directories, pruning empty directories only in + * discovered prior-default roots. The active root keeps its session directories + * to avoid racing a local write; writes recreate a directory pruned by another + * process. Every filesystem and warning-sink failure is contained, so a caller + * can await this during activation/disposal without it ever rejecting. + * + * @param options The roots to sweep, the age cutoff, and the failure sink. + * @returns Resolves when the sweep finishes (never rejects). + */ +export async function sweepSpillRoots(options: SweepOptions): Promise { + const { roots, cutoffMs, warn } = options + for (const root of roots) { + let entries: string[] + try { + entries = await readdir(root.path) + } catch (error: unknown) { + // A root that does not exist yet (no spill ever written) is the common + // case, not an error: ENOENT is silent, anything else is reported. + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to read root ${root.path}: ${String(error)}`) + continue + } + // Track whether the root holds ANY entry the sweep did not fully reclaim, so + // a discovered prior-default root can be pruned only when nothing remains. + let rootEmptiable = true + for (const name of entries) { + // Only the backend's own `session-<12 hex>` directories are swept; an + // unrelated sibling (`session-backup`, a stray file) is left untouched and + // blocks pruning the root. + if (!SESSION_DIR_RE.test(name)) { rootEmptiable = false; continue } + const dir = join(root.path, name) + let stats + try { + // lstat the session entry itself: a `session-*` SYMLINK must never be + // followed (readdir/unlink through it would delete files in a foreign + // target). Only a real directory is swept. + stats = await lstat(dir) + } catch (error: unknown) { + /* v8 ignore start -- an entry readdir just returned fails to lstat only + by racing away (ENOENT) or a permission/IO fault; not deterministically + reproducible. */ + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to stat ${dir}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + if (!stats.isDirectory()) { rootEmptiable = false; continue } + const empty = await sweepSessionDir(dir, cutoffMs, warn) + if (!empty) { rootEmptiable = false; continue } + if (!root.pruneWhenEmpty) { + // The active root remains writable while cleanup runs. Leaving its empty + // session directories in place closes the mkdir/rmdir race with saveText. + rootEmptiable = false + continue + } + try { + await rmdir(dir) + } catch (error: unknown) { + /* v8 ignore start -- prune runs only on a dir observed empty; a failure + here means a concurrent writer added a file (ENOTEMPTY) or a + permission/IO fault struck — both are races outside deterministic + in-process testing. */ + rootEmptiable = false + if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { + warnSafely(warn, `spill-local: failed to prune ${dir}: ${String(error)}`) + } + /* v8 ignore stop */ + } + } + // A discovered prior-default root (one per past process) is removed once its + // last session dir is gone — otherwise empty roots accumulate forever and + // every future startup rescans them. The active/configured root is never + // pruned (the live process is still writing into it). + if (root.pruneWhenEmpty && rootEmptiable) { + try { + await rmdir(root.path) + } catch (error: unknown) { + /* v8 ignore start -- prune runs only on a root whose every child was + reclaimed; a failure here means a concurrent writer added a fresh + spill after our scan (ENOTEMPTY) or removed the root already (ENOENT) + or a permission/IO fault struck — all races outside deterministic + in-process testing. */ + if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { + warnSafely(warn, `spill-local: failed to prune root ${root.path}: ${String(error)}`) + } + /* v8 ignore stop */ + } + } + } +} + +/** + * Discover prior default spill roots: the `dsh-spill-<6 chars>` directories + * directly under `base` (the OS tmpdir) that earlier default-root runs created. + * A long-lived deployment + * with a configured root will find none; a series of default-root runs + * accumulates one per process, so the startup sweep reclaims them all. Matching + * is the EXACT `mkdtemp` shape (see {@link DEFAULT_ROOT_RE}), not the bare + * prefix, so an unrelated `dsh-spill-test-*` fixture or a foreign + * differently-shaped directory is never swept; symlinks and non-directories are + * excluded too — only real directories the backend could have created. + * + * @param warn Sink for a failure reading `base` (returns `[]` on failure). + * @param base The directory to scan; defaults to the OS tmpdir (a test seam). + * @returns Absolute paths of the discovered default roots (possibly empty). + */ +export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir()): Promise { + let entries: string[] + try { + entries = await readdir(base) + } catch (error: unknown) { + warnSafely(warn, `spill-local: failed to scan ${base} for default roots: ${String(error)}`) + return [] + } + const roots: string[] = [] + for (const name of entries) { + if (!DEFAULT_ROOT_RE.test(name)) continue + const path = join(base, name) + let stats + try { + // lstat, not stat: a symlink named `dsh-spill-*` must not be treated as a + // root we then sweep (it could point anywhere). + stats = await lstat(path) + } catch (error: unknown) { + /* v8 ignore start -- an entry readdir just returned fails to lstat only by + racing away (ENOENT) or a permission/IO fault; not deterministically + reproducible. */ + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to stat default root ${path}: ${String(error)}`) + continue + /* v8 ignore stop */ + } + if (stats.isDirectory()) roots.push(path) + } + return roots +} diff --git a/packages/spill/spill-local/src/index.ts b/packages/spill/spill-local/src/index.ts index 7c4069c539..f767a1a12d 100644 --- a/packages/spill/spill-local/src/index.ts +++ b/packages/spill/spill-local/src/index.ts @@ -15,11 +15,14 @@ import { tmpdir } from 'node:os' import z from '@deepseek-ai/schemastery' import { SpillLocator, SpillStore } from '@deepseek-ai/dsh-spill' import type { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill' -import { discoverDefaultRoots, privateRoot, saveTextFile, sweepSpillRoots } from './store.ts' -import type { SweepRoot, WarnFn } from './store.ts' +import { discoverDefaultRoots, sweepSpillRoots } from './cleanup.ts' +import type { SweepRoot, WarnFn } from './cleanup.ts' +import { privateRoot, saveTextFile } from './store.ts' -export { discoverDefaultRoots, encodeSegment, isErrno, privateRoot, saveTextFile, sessionDir, sweepSpillRoots, DEFAULT_ROOT_PREFIX } from './store.ts' -export type { SavedText, SaveTextOptions, SweepOptions, SweepRoot, WarnFn } from './store.ts' +export { discoverDefaultRoots, sweepSpillRoots } from './cleanup.ts' +export type { SweepOptions, SweepRoot, WarnFn } from './cleanup.ts' +export { DEFAULT_ROOT_PREFIX, encodeSegment, isErrno, privateRoot, saveTextFile, sessionDir } from './store.ts' +export type { SavedText, SaveTextOptions } from './store.ts' /** Milliseconds in one day — converts the `cleanupPeriodDays` config to the sweep cutoff. */ const MS_PER_DAY = 24 * 60 * 60 * 1000 @@ -117,9 +120,10 @@ export class LocalSpillStore extends SpillStore { /** * The roots the startup sweep covers: each discovered prior-default * `dsh-spill-*` temp root (see {@link discoverDefaultRoots}), pruned when - * emptied, plus the active/configured root, which is swept but NEVER pruned - * (the live process is still writing into it). The active root is de-duped out - * of the discovered set so it is not swept twice or marked prunable. A test + * emptied, plus the active/configured root, whose root and session directories + * are NEVER pruned (the live process is still writing into them). The active + * root is de-duped out of the discovered set so it is not swept twice or + * marked prunable. A test * overrides this to inject an isolated root set — and, being the sweep's one * async gather point, to hold the sweep open across a disposal for the * quiescence check; it is a test seam, not a deployment knob. diff --git a/packages/spill/spill-local/src/store.ts b/packages/spill/spill-local/src/store.ts index 0303a8f639..ba518cf035 100644 --- a/packages/spill/spill-local/src/store.ts +++ b/packages/spill/spill-local/src/store.ts @@ -8,44 +8,30 @@ import { createHash, randomBytes } from 'node:crypto' import { mkdtempSync } from 'node:fs' -import { lstat, mkdir, open, readdir, rmdir, unlink } from 'node:fs/promises' +import { mkdir, open } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' -/** - * Filename prefix for the lazily-created private default spill roots - * (`mkdtemp(tmpdir()/dsh-spill-)`). Startup cleanup rediscovers these - * per-process roots (from prior runs that used no configured `root`) by this - * prefix — see {@link discoverDefaultRoots}. - */ +/** Prefix shared by default-root creation and startup discovery. */ export const DEFAULT_ROOT_PREFIX = 'dsh-spill-' /** - * A backend-generated default root name: `dsh-spill-` plus the 6-character - * suffix `mkdtemp` appends (see {@link privateRoot}). Discovery matches this - * EXACT shape, not the bare prefix, so an unrelated `dsh-spill-test-*` fixture - * or a foreign tool's differently-shaped `dsh-spill-…` directory is never - * mistaken for a backend root to sweep. + * Test a caught value for a Node system error code. + * + * @param error The caught value. + * @param code The expected system error code. + * @returns Whether the code matches. */ -const DEFAULT_ROOT_RE = /^dsh-spill-[A-Za-z0-9]{6}$/ - -/** - * A backend-generated session directory name: `session-` plus the 12 lowercase - * hex characters {@link sessionDir} derives from `sha256(sessionId)`. The sweep - * only descends into entries of this EXACT shape, so an unrelated - * `session-backup` directory under a shared configured root is never swept. - */ -const SESSION_DIR_RE = /^session-[0-9a-f]{12}$/ +export function isErrno(error: unknown, code: string): boolean { + return error instanceof Error && (error as NodeJS.ErrnoException).code === code +} let defaultRoot: string | undefined /** - * The default spill root: a private (0700) per-process directory under the OS - * tmpdir, created lazily. Predictable world-readable paths would let other - * local users read spilled tool output or pre-create symlinks; `mkdtemp` gives - * an unpredictable suffix and 0700 semantics. + * Return the lazily-created private per-process spill root. * - * @returns The lazily-created private spill root. + * @returns The private root path. */ export function privateRoot(): string { defaultRoot ??= mkdtempSync(join(tmpdir(), DEFAULT_ROOT_PREFIX)) @@ -63,8 +49,8 @@ export function privateRoot(): string { * inputs never collide. The whole-segment tokens `.`/`..` are escaped so they * can never traverse. An empty string encodes to `~` (never an empty segment). * - * @param raw The untrusted string to encode as one safe path segment. - * @returns An injective, filesystem-safe single path segment. + * @param raw Untrusted text. + * @returns One injective filesystem-safe path segment. */ export function encodeSegment(raw: string): string { if (raw.length === 0) return '~' @@ -74,317 +60,72 @@ export function encodeSegment(raw: string): string { for (let i = 0; i < raw.length; i++) { const code = raw.charCodeAt(i) const ch = String.fromCharCode(code) - if (ch !== '~' && /^[A-Za-z0-9._-]$/.test(ch)) { - out += ch - } else { - out += '~' + code.toString(16).toUpperCase().padStart(4, '0') - } + out += ch !== '~' && /^[A-Za-z0-9._-]$/.test(ch) + ? ch + : '~' + code.toString(16).toUpperCase().padStart(4, '0') } return out } /* jscpd:ignore-end */ /** - * The session-scoped directory: `/session-`, a short stable hash. + * Derive the stable session-scoped directory under a spill root. * - * @param root The spill root directory. - * @param sessionId The owning session id to hash into a stable directory name. - * @returns The absolute session-scoped spill directory path. + * @param root The spill root. + * @param sessionId The owning session id. + * @returns The stable session-scoped directory. */ export function sessionDir(root: string, sessionId: string): string { const hash = createHash('sha256').update(sessionId).digest('hex').slice(0, 12) return join(root, `session-${hash}`) } -/** Options for {@link saveTextFile} — the resolved root and the request fields the store needs. */ +/** Inputs needed to save a local spill file. */ export interface SaveTextOptions { - /** The spill root directory (configured or the lazy private default). */ + /** Spill root. */ root: string - /** The owning session id (scopes the directory). */ + /** Owning session id. */ sessionId: string - /** Caller-suggested base name; sanitized to one safe segment before use. */ + /** Caller-suggested filename. */ suggestedName: string - /** The full text to persist. */ + /** Full text to persist. */ content: string } /** A written spill file. */ export interface SavedText { + /** Absolute saved path. */ path: string + /** UTF-8 content length. */ bytes: number } /** - * Write `content` to a fresh file under the session-scoped directory and return - * its path + byte length. The filename is a random hex prefix plus the - * sanitized `suggestedName`, so it is unpredictable (defeats symlink planting in - * a shared root) AND stays readable. The open is exclusive + owner-only - * (`'wx', 0o600`): it fails on any existing path — symlink or not — so a - * pre-planted target cannot redirect the write. - * - * @param options The resolved root and request fields required to save the file. - * @returns The written file path and UTF-8 byte length. + * Write text to a fresh 0600 file below its private session directory. + * @param options The save request. + * @returns The saved path and UTF-8 byte length. */ export async function saveTextFile(options: SaveTextOptions): Promise { const dir = sessionDir(options.root, options.sessionId) - await mkdir(dir, { recursive: true, mode: 0o700 }) - const safeName = encodeSegment(options.suggestedName) - const path = join(dir, `${randomBytes(6).toString('hex')}-${safeName}`) - const bytes = Buffer.byteLength(options.content, 'utf8') - const handle = await open(path, 'wx', 0o600) + const path = join(dir, `${randomBytes(6).toString('hex')}-${encodeSegment(options.suggestedName)}`) + let handle + for (;;) { + await mkdir(dir, { recursive: true, mode: 0o700 }) + try { + handle = await open(path, 'wx', 0o600) + break + } catch (error: unknown) { + /* v8 ignore start -- requires another process to remove the directory + between mkdir and open, or an external permission/IO race. */ + if (isErrno(error, 'ENOENT')) continue + throw error + /* v8 ignore stop */ + } + } try { await handle.writeFile(options.content) } finally { await handle.close() } - return { path, bytes } -} - -/** A one-argument warning sink — the sweep's only side effect on failure (never throws). */ -export type WarnFn = (message: string) => void - -/** One root to sweep, plus whether an emptied root directory should itself be pruned. */ -export interface SweepRoot { - /** Absolute spill root to sweep. */ - path: string - /** - * When `true`, remove the root directory itself once its last `session-*` - * child is pruned. Set for DISCOVERED prior-default `dsh-spill-*` roots (one - * per past process — otherwise they accumulate empty forever), never for the - * active/configured root the live process is still writing into. - */ - pruneWhenEmpty: boolean -} - -/** Options for {@link sweepSpillRoots} — the roots to scan, the age cutoff, and a failure sink. */ -export interface SweepOptions { - /** Roots to sweep (configured/active root and/or discovered prior-default roots). */ - roots: SweepRoot[] - /** - * Epoch-millis cutoff: a regular file is deleted when its `mtime` is strictly - * older than this. The caller derives it from `now - cleanupPeriodDays`, so a - * file written exactly at the boundary is kept (only strictly-older expires). - */ - cutoffMs: number - /** Where a contained filesystem failure is reported; the sweep itself never throws. */ - warn: WarnFn -} - -/** - * Delete a single path, treating a concurrent-race disappearance as success. - * A parallel process (or another sweep) may `unlink` the same file between our - * scan and our own `unlink` — ENOENT then means the goal (file gone) already - * holds, so it is not a failure. Any other error is reported and swallowed. - * - * @param path The absolute file path to remove. - * @param warn Sink for a non-ENOENT failure message. - * @returns Resolves once the removal was attempted (never rejects). - */ -async function unlinkIdempotent(path: string, warn: WarnFn): Promise { - try { - await unlink(path) - } catch (error: unknown) { - /* v8 ignore start -- reached only when a file selected for deletion (a - regular file that passed lstat) then fails to unlink: either it raced away - (ENOENT) or a permission/IO fault struck between the stat and the unlink. - Neither is deterministically reproducible in-process. */ - if (isErrno(error, 'ENOENT')) return - warn(`spill-local: failed to delete ${path}: ${String(error)}`) - /* v8 ignore stop */ - } -} - -/** - * True when `error` is a Node system error carrying the given `code`. - * - * @param error The caught value to test. - * @param code The `NodeJS.ErrnoException` code to match (e.g. `'ENOENT'`). - * @returns `true` when `error` is an `Error` whose `code` equals `code`. - */ -export function isErrno(error: unknown, code: string): boolean { - return error instanceof Error && (error as NodeJS.ErrnoException).code === code -} - -/** - * Sweep one spill session directory: delete expired regular files, skip - * everything else, and report the directory empty afterward so the caller can - * prune it. The `dir` entry MUST be a real directory — the caller `lstat`s it - * first and skips a symlink, so this never follows a `session-*` symlink into a - * foreign tree. Inside, a symlink or any non-regular entry (socket, fifo, nested - * dir) is left untouched — `lstat` never follows a link, so a planted symlink - * can neither be deleted nor redirect the age check. Every per-entry failure is - * contained: one unreadable file does not abort the directory. - * - * @param dir The absolute session directory to scan (already confirmed a real dir). - * @param cutoffMs Files with `mtime` strictly older than this are deleted. - * @param warn Sink for contained filesystem failures. - * @returns `true` when the directory holds no entries after the sweep (a prune candidate). - */ -async function sweepSessionDir(dir: string, cutoffMs: number, warn: WarnFn): Promise { - let names: string[] - try { - names = await readdir(dir) - } catch (error: unknown) { - /* v8 ignore start -- the caller lstat'd this entry and confirmed a real - directory just before the call, so readdir fails only when the dir races - away (ENOENT) or a permission/IO fault strikes in that window; not - deterministically reproducible. False keeps it out of the prune step. */ - warn(`spill-local: failed to read ${dir}: ${String(error)}`) - return false - /* v8 ignore stop */ - } - let remaining = names.length - for (const name of names) { - const path = join(dir, name) - let stats - try { - stats = await lstat(path) - } catch (error: unknown) { - /* v8 ignore start -- an entry that readdir just returned then fails to - lstat only by racing away (ENOENT) or a permission/IO fault; keep it out - of the deterministic test surface. */ - if (isErrno(error, 'ENOENT')) { remaining--; continue } - warn(`spill-local: failed to stat ${path}: ${String(error)}`) - continue - /* v8 ignore stop */ - } - // Only regular files expire. Symlinks and other special entries are skipped - // (never followed) so the sweep cannot be redirected or delete a link. - if (!stats.isFile()) continue - if (stats.mtimeMs >= cutoffMs) continue - await unlinkIdempotent(path, warn) - remaining-- - } - return remaining === 0 -} - -/** - * Best-effort one-shot cleanup: across each root, delete expired regular files - * under its `session-*` directories and prune any directory left empty. The - * sweep is idempotent and safe to run concurrently with live spill writes and - * with another process's sweep — per-file expiry preserves a fresh write even - * if it lands mid-sweep, and every filesystem failure is caught and reported - * rather than thrown, so a caller can await this during activation/disposal - * without it ever rejecting. - * - * @param options The roots to sweep, the age cutoff, and the failure sink. - * @returns Resolves when the sweep finishes (never rejects). - */ -export async function sweepSpillRoots(options: SweepOptions): Promise { - const { roots, cutoffMs, warn } = options - for (const root of roots) { - let entries: string[] - try { - entries = await readdir(root.path) - } catch (error: unknown) { - // A root that does not exist yet (no spill ever written) is the common - // case, not an error: ENOENT is silent, anything else is reported. - if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to read root ${root.path}: ${String(error)}`) - continue - } - // Track whether the root holds ANY entry the sweep did not fully reclaim, so - // a discovered prior-default root can be pruned only when nothing remains. - let rootEmptiable = true - for (const name of entries) { - // Only the backend's own `session-<12 hex>` directories are swept; an - // unrelated sibling (`session-backup`, a stray file) is left untouched and - // blocks pruning the root. - if (!SESSION_DIR_RE.test(name)) { rootEmptiable = false; continue } - const dir = join(root.path, name) - let stats - try { - // lstat the session entry itself: a `session-*` SYMLINK must never be - // followed (readdir/unlink through it would delete files in a foreign - // target). Only a real directory is swept. - stats = await lstat(dir) - } catch (error: unknown) { - /* v8 ignore start -- an entry readdir just returned fails to lstat only - by racing away (ENOENT) or a permission/IO fault; not deterministically - reproducible. */ - if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to stat ${dir}: ${String(error)}`) - continue - /* v8 ignore stop */ - } - if (!stats.isDirectory()) { rootEmptiable = false; continue } - const empty = await sweepSessionDir(dir, cutoffMs, warn) - if (!empty) { rootEmptiable = false; continue } - try { - await rmdir(dir) - } catch (error: unknown) { - /* v8 ignore start -- prune runs only on a dir observed empty; a failure - here means a concurrent writer added a file (ENOTEMPTY) or a - permission/IO fault struck — both are races outside deterministic - in-process testing. */ - rootEmptiable = false - if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { - warn(`spill-local: failed to prune ${dir}: ${String(error)}`) - } - /* v8 ignore stop */ - } - } - // A discovered prior-default root (one per past process) is removed once its - // last session dir is gone — otherwise empty roots accumulate forever and - // every future startup rescans them. The active/configured root is never - // pruned (the live process is still writing into it). - if (root.pruneWhenEmpty && rootEmptiable) { - try { - await rmdir(root.path) - } catch (error: unknown) { - /* v8 ignore start -- prune runs only on a root whose every child was - reclaimed; a failure here means a concurrent writer added a fresh - spill after our scan (ENOTEMPTY) or removed the root already (ENOENT) - or a permission/IO fault struck — all races outside deterministic - in-process testing. */ - if (!isErrno(error, 'ENOENT') && !isErrno(error, 'ENOTEMPTY')) { - warn(`spill-local: failed to prune root ${root.path}: ${String(error)}`) - } - /* v8 ignore stop */ - } - } - } -} - -/** - * Discover prior default spill roots: the `dsh-spill-<6 chars>` directories - * directly under `base` (the OS tmpdir) that earlier runs created via - * {@link privateRoot} when no `root` was configured. A long-lived deployment - * with a configured root will find none; a series of default-root runs - * accumulates one per process, so the startup sweep reclaims them all. Matching - * is the EXACT `mkdtemp` shape (see {@link DEFAULT_ROOT_RE}), not the bare - * prefix, so an unrelated `dsh-spill-test-*` fixture or a foreign - * differently-shaped directory is never swept; symlinks and non-directories are - * excluded too — only real directories the backend could have created. - * - * @param warn Sink for a failure reading `base` (returns `[]` on failure). - * @param base The directory to scan; defaults to the OS tmpdir (a test seam). - * @returns Absolute paths of the discovered default roots (possibly empty). - */ -export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir()): Promise { - let entries: string[] - try { - entries = await readdir(base) - } catch (error: unknown) { - warn(`spill-local: failed to scan ${base} for default roots: ${String(error)}`) - return [] - } - const roots: string[] = [] - for (const name of entries) { - if (!DEFAULT_ROOT_RE.test(name)) continue - const path = join(base, name) - let stats - try { - // lstat, not stat: a symlink named `dsh-spill-*` must not be treated as a - // root we then sweep (it could point anywhere). - stats = await lstat(path) - } catch (error: unknown) { - /* v8 ignore start -- an entry readdir just returned fails to lstat only by - racing away (ENOENT) or a permission/IO fault; not deterministically - reproducible. */ - if (!isErrno(error, 'ENOENT')) warn(`spill-local: failed to stat default root ${path}: ${String(error)}`) - continue - /* v8 ignore stop */ - } - if (stats.isDirectory()) roots.push(path) - } - return roots + return { path, bytes: Buffer.byteLength(options.content, 'utf8') } } diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index 41d35824c6..8bdb5a7471 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -3,10 +3,10 @@ * returns a locator + byte length + retrieval hint, filename sanitization * neutralizes traversal, the configured `root` is honored (and the private * default when omitted), and a storage failure rejects. The startup cleanup - * sweep expires old files, prunes empty dirs, skips symlinks/unknown entries, + * sweep expires old files, prunes stale roots, skips symlinks/unknown entries, * discovers prior default roots, contains filesystem failures, and is awaited on - * disposal without blocking activation. The Cordis-free `store.ts` helpers are - * exercised directly for the naming/encoding and sweep edge cases. + * disposal without blocking activation. The Cordis-free store and cleanup + * helpers are exercised directly for their edge cases. */ import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' @@ -280,7 +280,7 @@ describe('startup cleanup sweep', () => { expect(existsSync(old)).toBe(true) }) - it('prunes a session directory left empty, keeps one with a surviving file', async () => { + it('keeps active session directories after deleting expired files', async () => { const emptied = sessionDir(root, 'emptied') const kept = sessionDir(root, 'kept') mkdirSync(emptied, { recursive: true }) @@ -288,7 +288,7 @@ describe('startup cleanup sweep', () => { writeAged(join(emptied, 'a.txt'), 'x', 40) writeAged(join(kept, 'fresh.txt'), 'y', 1) await runSweep([active(root)]) - expect(existsSync(emptied)).toBe(false) + expect(existsSync(emptied)).toBe(true) expect(existsSync(kept)).toBe(true) }) @@ -351,7 +351,7 @@ describe('startup cleanup sweep', () => { await runSweep([{ path: prior, pruneWhenEmpty: true }, active(root)]) expect(existsSync(prior)).toBe(false) // discovered root pruned expect(existsSync(root)).toBe(true) // active root kept - expect(existsSync(activeDir)).toBe(false) // its emptied session dir still pruned + expect(existsSync(activeDir)).toBe(true) // active session dirs remain writable } finally { rmSync(prior, { recursive: true, force: true }) } @@ -458,6 +458,13 @@ describe('startup cleanup sweep', () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) }) + it('contains an exception from the warning sink', async () => { + const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') + const warn = vi.fn(() => { throw new Error('logger failed') }) + await expect(sweepSpillRoots({ roots: [active(filePath)], cutoffMs: Date.now(), warn })).resolves.toBeUndefined() + expect(warn).toHaveBeenCalledOnce() + }) + it('a nonexistent root is silent (the common no-spill-yet case)', async () => { const warn = vi.fn() await sweepSpillRoots({ roots: [active(join(root, 'never-created'))], cutoffMs: Date.now(), warn }) @@ -500,4 +507,3 @@ describe('isErrno', () => { expect(isErrno(new Error('no code'), 'ENOENT')).toBe(false) }) }) - From a268aada8c89f8a09ce4920d8d65b31542b70eff Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 24 Aug 2026 16:33:15 +0800 Subject: [PATCH 5/8] fix(spill-local): harden startup cleanup --- ...26-07-08-tool-output-spill-files.i18n.yaml | 4 +- .../2026-07-08-tool-output-spill-files.md | 2 +- .../2026-07-08-tool-output-spill-files.zh.md | 2 +- ...7-17-local-spill-startup-cleanup.i18n.yaml | 4 +- .../2026-07-17-local-spill-startup-cleanup.md | 6 +- ...26-07-17-local-spill-startup-cleanup.zh.md | 6 +- docs/config-catalog.i18n.yaml | 4 +- docs/config-catalog.md | 6 +- docs/config-catalog.zh.md | 6 +- packages/spill/spill-local/README.i18n.yaml | 4 +- packages/spill/spill-local/README.md | 4 +- packages/spill/spill-local/README.zh.md | 4 +- packages/spill/spill-local/package.json | 2 + packages/spill/spill-local/src/cleanup.ts | 221 ++++++++++++++---- packages/spill/spill-local/src/index.ts | 34 ++- .../tests/loader-composition.spec.ts | 78 +++++++ .../spill-local/tests/spill-local.spec.ts | 102 ++++++-- pnpm-lock.yaml | 6 + 18 files changed, 399 insertions(+), 96 deletions(-) create mode 100644 packages/spill/spill-local/tests/loader-composition.spec.ts diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml index 20f4e54eae..7b145bd906 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md -2026-07-08-tool-output-spill-files.md: 81a292a00af63b0aa9145a0c556a428ab8c49d64 -2026-07-08-tool-output-spill-files.zh.md: 8d9e60d461297fb11ff2252e91f98a0cfad33f62 +2026-07-08-tool-output-spill-files.md: e14607e388c634c4e2679c993c1b720be0a3a9f3 +2026-07-08-tool-output-spill-files.zh.md: 372c9c6cadf3cd64c3de97a8c305b8909f03caab diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md index 81a292a00a..e14607e388 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md @@ -57,7 +57,7 @@ interface SpillRef { `SpillLocator` is a [branded](../../../../packages/util/brand) model-facing handle returned by the backend. The local backend renders it as a filesystem path; a remote or database backend can render a URI, key, or command token. Consumers treat it as opaque and render it with `retrievalHint` instead of assuming `read` is always the right retrieval mechanism. `SpillOwner.sessionId` is the save-time storage namespace: forked sessions inherit existing spill locators from the seeded log without copying or re-owning them, and new spills after the fork use the child session id. A retention-period cleanup may expire old locators with other old session artifacts; the spill seam does not define a per-session cleanup policy. -`dsh-spill-local` owns only storage details: session-scoped directory selection, safe names, path-traversal protection, the write, and returning `{ locator, bytes, retrievalHint }`. It does not own retention policy, tool-result replacement, search, or file inspection. Files land at `/session-/-`, where `root` is a configured path or a lazily-created private (0700) per-process temp dir, the session subdir is a short `sha256(sessionId)` prefix, and the leaf is a random hex prefix plus the caller's `suggestedName` sanitized to one path segment (mirrors the JSONL backend's `encodeSegment`). The write is `open(path, 'wx', 0o600)` — exclusive and owner-only, so a planted symlink cannot redirect it. The locator is the path, and the retrieval hint tells the model it can use `read` or `grep` on that path. +`dsh-spill-local` owns storage details: session-scoped directory selection, safe names, path-traversal protection, the write, local artifact lifetime, and returning `{ locator, bytes, retrievalHint }`. It does not own tool-result replacement, model-facing preview policy, search, file inspection, or a seam-wide/per-session retention policy. Files land at `/session-/-`, where `root` is a configured path or a lazily-created private (0700) per-process temp dir, the session subdir is a short `sha256(sessionId)` prefix, and the leaf is a random hex prefix plus the caller's `suggestedName` sanitized to one path segment (mirrors the JSONL backend's `encodeSegment`). The write is `open(path, 'wx', 0o600)` — exclusive and owner-only, so a planted symlink cannot redirect it. The locator is the path, and the retrieval hint tells the model it can use `read` or `grep` on that path. Its one-shot startup cleanup applies the backend-specific artifact lifetime described in the [local spill cleanup note](./2026-07-17-local-spill-startup-cleanup.md). ### Spill policy diff --git a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md index 8d9e60d461..372c9c6cad 100644 --- a/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md @@ -57,7 +57,7 @@ interface SpillRef { `SpillLocator` 是一个[品牌化的](../../../../packages/util/brand)模型可见句柄，由后端返回。本地后端将其渲染为文件系统路径；远程或数据库后端可以渲染 URI、键或命令 token。消费方把它视为不透明值，并使用 `retrievalHint` 渲染，而不是假定 `read` 始终是正确的检索机制。`SpillOwner.sessionId` 是保存时的存储命名空间：fork 后的会话会从种子日志继承已有的 spill 定位符，无需复制它们或重新取得所有权；fork 后的新 spill 使用子会话 id。保留期清理可以连同其他旧会话产物一起使旧定位符失效；spill seam 不定义逐会话的清理策略。 -`dsh-spill-local` 只负责存储细节：选择会话作用域的目录、安全名称、防止路径遍历、执行写入，以及返回 `{ locator, bytes, retrievalHint }`。它不负责保留策略、工具结果替换、搜索或文件检查。文件写入 `/session-/-`：`root` 是配置路径，或延迟创建的私有（0700）进程级临时目录；会话子目录是 `sha256(sessionId)` 的短前缀；叶节点由随机十六进制前缀与调用方的 `suggestedName` 组成，后者会被清理成单一路径段（与 JSONL 后端的 `encodeSegment` 一致）。系统使用 `open(path, 'wx', 0o600)` 写入，确保独占且仅所有者可访问，因此预先植入的符号链接无法重定向写入。定位符就是该路径，检索提示则告知模型可以在该路径上使用 `read` 或 `grep`。 +`dsh-spill-local` 负责存储细节：选择会话作用域的目录、安全名称、防止路径遍历、执行写入、本地产物生命周期，以及返回 `{ locator, bytes, retrievalHint }`。它不负责工具结果替换、模型可见的预览策略、搜索、文件检查，也不定义 seam 级或逐会话保留策略。文件写入 `/session-/-`：`root` 是配置路径，或延迟创建的私有（0700）进程级临时目录；会话子目录是 `sha256(sessionId)` 的短前缀；叶节点由随机十六进制前缀与调用方的 `suggestedName` 组成，后者会被清理成单一路径段（与 JSONL 后端的 `encodeSegment` 一致）。系统使用 `open(path, 'wx', 0o600)` 写入，确保独占且仅所有者可访问，因此预先植入的符号链接无法重定向写入。定位符就是该路径，检索提示则告知模型可以在该路径上使用 `read` 或 `grep`。它的一次性启动清理会应用[本地 spill 清理说明](./2026-07-17-local-spill-startup-cleanup.zh.md)所述的后端专属产物生命周期。 ### spill 策略 diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml index 511dda5a74..06f4d81cb0 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md -2026-07-17-local-spill-startup-cleanup.md: 96378d6ea785d90385f517c1b9a01073ade47fa2 -2026-07-17-local-spill-startup-cleanup.zh.md: a154cfb824d3a747c2c9acc2b707eb14d703ce2e +2026-07-17-local-spill-startup-cleanup.md: fc64938c1af07d9dd0d7ecec379115d22d1e2464 +2026-07-17-local-spill-startup-cleanup.zh.md: 583a33ead84f552c67e2e770a8b3fabc3ce88120 diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md index 96378d6ea7..fc64938c1a 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md @@ -12,7 +12,9 @@ The local spill backend never deleted the full tool results it wrote. Every over `dsh-spill-local` runs one best-effort cleanup sweep after activation. It does not delay service availability, is owned by the plugin fiber (a single `ctx.effect` whose generator launches the sweep and yields an async disposer that awaits it), and is awaited during disposal so no sweep I/O outlives the fiber. There is no recurring timer and no separate process. -A `cleanupPeriodDays` config defaults to `30`; `0` disables cleanup. An invalid value (negative or fractional) throws at load. The sweep scans the configured/active root plus any prior default `dsh-spill-*` temp roots discovered under the OS temp dir and deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`. It prunes empty session directories and roots only for discovered prior-default roots; the active root keeps its session directories so pruning cannot race a local write, while writes recreate a session directory if another process prunes a discovered root that is still active. It uses `lstat`, so a symlink is never followed or deleted; unrelated entries (non-`session-` directories, special files) are skipped. Every filesystem failure is caught and logged through `ctx.logger.warn`, and a warning-sink exception is also contained — the sweep never throws, so it cannot reject activation or a concurrent spill write. Discovery excludes symlinks and non-directories, returning only real `dsh-spill-*` directories the backend could have created. +A `cleanupPeriodDays` config defaults to `30`; `0` disables cleanup. Schemastery rejects a negative or fractional value at load. The sweep scans the configured/active root plus any prior default `dsh-spill-*` temp roots discovered under the OS temp dir and deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`. It prunes every empty session directory but removes the root itself only for a discovered prior-default root; writes recreate a session directory if pruning races them. Root aliases are de-duplicated by device/inode identity, with the configured identity overriding a discovered match as active and non-prunable. It uses `lstat`, so a symlink is never followed or deleted; unrelated entries (non-`session-` directories, special files) are skipped. Every filesystem failure is caught and logged through `ctx.logger.warn`, and a warning-sink exception is also contained — the sweep never throws, so it cannot reject activation or a concurrent spill write. + +Path-based deletion is restricted to directories an untrusted local OS user cannot replace during the scan. On POSIX, every root and session directory must be owned by the current user and not writable by group or others; the root's ancestor path must also be non-writable or protected by a sticky directory such as `/tmp`. Discovery rejects symlinks, while a configured symlink may resolve to a trusted target and participates in identity de-duplication. An unsafe path is skipped with a warning. The same-user account remains the trust boundary, consistent with the backend's private local-storage model. The ctx-free sweep mechanics live in `packages/spill/spill-local/src/cleanup.ts` (`sweepSpillRoots`, `discoverDefaultRoots`), unit-testable without a `ctx`; `store.ts` owns root naming, path derivation, and writes, while the service in `src/index.ts` owns the config, cutoff, and fiber-owned launch/await. @@ -32,4 +34,4 @@ Cleanup cost the backend a startup sweep and a config knob, and bought a bounded ## Testing -`dsh-spill-local` unit tests cover the age boundary (strictly-older expires, boundary kept), `cleanupPeriodDays: 0` disabling, discovered-root pruning, active-directory preservation, symlink/unrelated-entry skipping, configured-plus-discovered-root coverage through the real `gatherRoots`/`discoverDefaultRoots` path, active-root de-duplication, load-time validation of a bad `cleanupPeriodDays`, filesystem- and warning-sink-failure containment both directly and through the service's `ctx.logger.warn` wiring, and the quiescence contract — activation is available while a barrier-held sweep is parked, and disposal only settles after the sweep finishes. +`dsh-spill-local` unit tests cover the exact age boundary, `cleanupPeriodDays: 0` disabling, empty-session and discovered-root pruning, symlink/unrelated-entry skipping, configured-plus-discovered-root coverage, filesystem-identity de-duplication through a configured symlink, unsafe POSIX root/session rejection, load-time config validation, filesystem- and warning-sink-failure containment, and the quiescence contract. A separate test boots the plugin through the real Loader and a cordis.yml, then observes configured expiry and directory pruning after disposal. diff --git a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md index a154cfb824..583a33ead8 100644 --- a/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md @@ -12,7 +12,9 @@ Status: implemented `dsh-spill-local` 在激活后运行一次尽力而为的清理扫描。它不延迟服务可用性，由插件 fiber 拥有（一个 `ctx.effect`，其生成器启动该扫描并让出一个等待它的异步 disposer），并在 dispose 期间被等待，因此没有扫描 I/O 会存活到 fiber 之后。既没有周期性定时器，也没有独立进程。 -`cleanupPeriodDays` 配置默认为 `30`；`0` 会禁用清理。无效值（负数或小数）在加载时抛出。扫描会遍历配置的/活动的根目录，以及在 OS 临时目录下发现的任何先前默认 `dsh-spill-*` 临时根目录，并删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件。它只修剪发现的先前默认根目录中的空会话目录和空根目录；活动根目录会保留其会话目录，避免修剪操作与本地写入竞争，而当其他进程修剪了一个仍在使用的发现根目录时，写入操作会重新创建会话目录。扫描使用 `lstat`，因此符号链接绝不会被跟随或删除；无关条目（非 `session-` 目录、特殊文件）会被跳过。每一次文件系统失败都会被捕获并通过 `ctx.logger.warn` 记录，警告接收方抛出的异常也会被兜底——扫描绝不抛出，因此它无法让激活失败，也无法影响并发的 spill 写入。发现过程排除符号链接与非目录，只返回后端可能创建过的真实 `dsh-spill-*` 目录。 +`cleanupPeriodDays` 配置默认为 `30`；`0` 会禁用清理。Schemastery 会在加载时拒绝负数或小数。扫描会遍历配置的/活动的根目录，以及在 OS 临时目录下发现的任何先前默认 `dsh-spill-*` 临时根目录，并删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件。它会修剪所有空会话目录，但只删除发现的先前默认根目录本身；如果修剪与写入发生竞争，写入操作会重新创建会话目录。根目录别名按设备/inode 身份去重，配置目录的身份会覆盖发现的匹配项，并标记为活动且不可删除。扫描使用 `lstat`，因此符号链接绝不会被跟随或删除；无关条目（非 `session-` 目录、特殊文件）会被跳过。每一次文件系统失败都会被捕获并通过 `ctx.logger.warn` 记录，警告接收方抛出的异常也会被兜底——扫描绝不抛出，因此它无法让激活失败，也无法影响并发的 spill 写入。 + +基于路径的删除仅限于不受信任的本地 OS 用户无法在扫描期间替换的目录。在 POSIX 上，每个根目录和会话目录都必须由当前用户拥有，且组用户和其他用户不可写；根目录的祖先路径也必须不可写，或由 `/tmp` 这类 sticky 目录保护。发现过程拒绝符号链接，而配置的符号链接可以解析到可信目标并参与身份去重。不安全路径会被跳过并记录警告。与后端的私有本地存储模型一致，同一用户账号仍是信任边界。 无 ctx 依赖的扫描机制位于 `packages/spill/spill-local/src/cleanup.ts`（`sweepSpillRoots`、`discoverDefaultRoots`），无需 `ctx` 即可做单元测试；`store.ts` 负责根目录命名、路径推导与写入，而 `src/index.ts` 中的服务负责配置、截止时间以及 fiber 拥有的启动/等待。 @@ -32,4 +34,4 @@ Status: implemented ## 验证 -`dsh-spill-local` 单元测试覆盖了年龄边界（严格更旧者过期，边界值保留）、`cleanupPeriodDays: 0` 的禁用、发现根目录的修剪、活动目录的保留、符号链接/无关条目的跳过、通过真实 `gatherRoots`/`discoverDefaultRoots` 路径对配置根加发现根的覆盖、活动根去重、对错误 `cleanupPeriodDays` 的加载期校验、直接测试以及经由服务的 `ctx.logger.warn` 接线测试所覆盖的文件系统与警告接收方失败兜底，以及静止契约：在一个被屏障挂起的扫描停驻期间激活仍然可用，而 dispose 只有在扫描结束后才会完成。 +`dsh-spill-local` 单元测试覆盖了精确年龄边界、`cleanupPeriodDays: 0` 的禁用、空会话目录与发现根目录的修剪、符号链接/无关条目的跳过、配置根加发现根的覆盖、经配置符号链接验证的文件系统身份去重、不安全 POSIX 根目录/会话目录拒绝、加载期配置校验、文件系统与警告接收方故障兜底，以及静止契约。另一个测试会通过真实 Loader 和 cordis.yml 启动插件，并在 dispose 后观察按配置执行的过期与目录修剪。 diff --git a/docs/config-catalog.i18n.yaml b/docs/config-catalog.i18n.yaml index 66d5592ba2..b334176fb9 100644 --- a/docs/config-catalog.i18n.yaml +++ b/docs/config-catalog.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write docs/config-catalog.md -config-catalog.md: 9fc8c333510c568686ce43f4aa1f6d0ae6bc3615 -config-catalog.zh.md: 4fb689f63631740d485a854e10a12c6d92c6f4ac +config-catalog.md: 1b0eceb35a7679e17166d1bb0b9a7a8ae6079613 +config-catalog.zh.md: 5a698f82db13fe95535ad631636b921819513a74 diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 9fc8c33351..1b0eceb35a 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -2067,8 +2067,10 @@ export interface Config { * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose * `mtime` is strictly older than the cutoff are deleted and emptied * directories are pruned; fresh files, symlinks, and unrelated entries are - * left untouched. Retention is deliberate — a resumed or forked session may - * still reference an older locator until it ages out. + * left untouched. On POSIX, cleanup skips roots and session directories that + * another local user could modify or replace. Retention is deliberate — a + * resumed or forked session may still reference an older locator until it + * ages out. */ cleanupPeriodDays?: number } diff --git a/docs/config-catalog.zh.md b/docs/config-catalog.zh.md index 4fb689f636..5a698f82db 100644 --- a/docs/config-catalog.zh.md +++ b/docs/config-catalog.zh.md @@ -2069,8 +2069,10 @@ export interface Config { * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose * `mtime` is strictly older than the cutoff are deleted and emptied * directories are pruned; fresh files, symlinks, and unrelated entries are - * left untouched. Retention is deliberate — a resumed or forked session may - * still reference an older locator until it ages out. + * left untouched. On POSIX, cleanup skips roots and session directories that + * another local user could modify or replace. Retention is deliberate — a + * resumed or forked session may still reference an older locator until it + * ages out. */ cleanupPeriodDays?: number } diff --git a/packages/spill/spill-local/README.i18n.yaml b/packages/spill/spill-local/README.i18n.yaml index dd414de9b9..1fd753f7ae 100644 --- a/packages/spill/spill-local/README.i18n.yaml +++ b/packages/spill/spill-local/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/spill/spill-local/README.md -README.md: 75bde20c423e1d2aa4bba49201b5bb0369d34fd0 -README.zh.md: 0539969cc4bd4da8dad4f0ac00436476555fd08c +README.md: e5a1fb08ba438640e649319f42d31aa10afd24c1 +README.zh.md: ba6e2b3c4628b7a21de1361fcd2d1e0a1b573f4b diff --git a/packages/spill/spill-local/README.md b/packages/spill/spill-local/README.md index 75bde20c42..e5a1fb08ba 100644 --- a/packages/spill/spill-local/README.md +++ b/packages/spill/spill-local/README.md @@ -23,7 +23,9 @@ Files land at `/session-/​-`: The backend never deletes a spill on the write path — a persisted, resumed, or forked session may still reference an older locator, so immediate deletion would break retrieval. Instead, one best-effort sweep runs **once after activation**: it does not delay service availability, is owned by the plugin fiber, and is awaited on disposal (no sweep I/O outlives the fiber). There is no recurring timer and no separate process, so a long-lived deployment is not swept again until its next restart. -The sweep scans the configured `root` **and** any earlier default `dsh-spill-*` temp roots that prior default-root runs left under the OS temp dir. Within each, it deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays`; it prunes empty session directories and roots only for discovered prior-default roots, while the active root keeps its session directories to avoid racing a write. A write recreates its session directory if another process prunes a discovered root that is still active. The sweep never follows or deletes a symlink, skips unrelated entries, and contains every filesystem or warning-sink failure so it cannot fail activation or a concurrent spill write. Retention is deliberate: an old model-visible locator goes stale only once it ages past the cutoff. +The sweep scans the configured `root` **and** any earlier default `dsh-spill-*` temp roots that prior default-root runs left under the OS temp dir. It resolves each root to its filesystem identity, so a configured alias of a discovered root remains the active, non-prunable root. Within each root, the sweep deletes regular files whose `mtime` is strictly older than `now − cleanupPeriodDays` and prunes every empty session directory; only an empty discovered prior-default root is itself removed. A write recreates a session directory if cleanup races it. The sweep never follows or deletes a symlink and skips unrelated entries. + +On POSIX, cleanup admits only roots owned by the current user, not writable by group or others, and protected from replacement through their ancestor path; a writable sticky temporary directory such as `/tmp` is permitted. Session directories must satisfy the same ownership and write restrictions. Unsafe paths are skipped with a warning, which prevents an untrusted local process from redirecting path-based deletion outside the spill root. Every filesystem or warning-sink failure is contained, so cleanup cannot fail activation or a concurrent spill write. Retention is deliberate: an old model-visible locator goes stale only once it ages past the cutoff. `saveText` rejects on a real storage failure (permissions, ENOSPC); the spill policy treats a rejection as best-effort and keeps the inline result. See the seam README for the vocabulary and the [tool output spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.md) for the design, and the [startup-cleanup Agent Note](../../../.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.md) for the sweep. diff --git a/packages/spill/spill-local/README.zh.md b/packages/spill/spill-local/README.zh.md index 0539969cc4..ba6e2b3c46 100644 --- a/packages/spill/spill-local/README.zh.md +++ b/packages/spill/spill-local/README.zh.md @@ -23,7 +23,9 @@ 后端不会在写入路径上删除 spill，因为已持久化、已恢复或 fork 后的会话仍可能引用较旧的定位信息，立即删除会使其无法取回。后端会改为在激活后**仅运行一次**尽力而为的扫描：扫描不延迟服务可用性，由插件 fiber 拥有，并在 dispose 期间被等待（不会有扫描 I/O 存活至 fiber 之后）。它既不使用周期性定时器，也不运行独立进程，因此长期运行的部署要到下次重启才会再次扫描。 -扫描会检查配置的 `root` **以及**先前使用默认根目录的运行在操作系统临时目录下留下的所有 `dsh-spill-*` 临时根目录。在每个根目录中，扫描会删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件；它只修剪发现的先前默认根目录中的空会话目录和空根目录，而活动根目录会保留其会话目录，以避免与写入操作竞争。如果另一个进程修剪了一个仍在使用的发现根目录，写入操作会重新创建其会话目录。扫描绝不会跟随或删除符号链接，会跳过无关条目，并兜底每一次文件系统或警告接收方失败，因此无法使激活或并发 spill 写入失败。保留是刻意的：旧的模型可见定位信息只有超过截止时间后才会失效。 +扫描会检查配置的 `root` **以及**先前使用默认根目录的运行在操作系统临时目录下留下的所有 `dsh-spill-*` 临时根目录。它会把每个根目录解析为文件系统身份，因此当配置路径是某个已发现根目录的别名时，该目录仍会作为不可删除的活动根目录处理。在每个根目录中，扫描会删除 `mtime` 严格早于 `now − cleanupPeriodDays` 的常规文件并修剪所有空会话目录；只有发现的先前默认根目录会在变空后被删除。如果清理与写入发生竞争，写入操作会重新创建会话目录。扫描绝不会跟随或删除符号链接，并会跳过无关条目。 + +在 POSIX 上，清理只接受由当前用户拥有、组用户和其他用户不可写、且祖先路径可防止他人替换的根目录；`/tmp` 这类带 sticky 位的可写临时目录仍然允许使用。会话目录必须满足相同的所有权和写权限限制。不安全路径会被跳过并记录警告，从而防止不受信任的本地进程把基于路径的删除重定向到 spill 根目录之外。所有文件系统故障和警告接收方故障都会被兜底，因此清理无法使激活或并发 spill 写入失败。保留是刻意的：旧的模型可见定位信息只有超过截止时间后才会失效。 `saveText` 在发生真实存储故障（权限、ENOSPC）时返回拒绝；spill 策略会按尽力而为原则处理该拒绝，并保留内联结果。词汇见 seam README，设计见[工具输出 spill Agent Note](../../../.agents/notes/implemented/architecture/2026-07-08-tool-output-spill-files.zh.md)，扫描机制见[启动清理 Agent Note](../../../.agents/notes/implemented/architecture/2026-07-17-local-spill-startup-cleanup.zh.md)。 diff --git a/packages/spill/spill-local/package.json b/packages/spill/spill-local/package.json index 44ca42effd..86eb53c585 100644 --- a/packages/spill/spill-local/package.json +++ b/packages/spill/spill-local/package.json @@ -40,6 +40,8 @@ "@deepseek-ai/schemastery": "workspace:^" }, "devDependencies": { + "@deepseek-ai/cordis-plugin-include": "workspace:^", + "@deepseek-ai/cordis-plugin-loader": "workspace:^", "@deepseek-ai/dsh-brand": "workspace:^", "@deepseek-ai/dsh-invariants": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", diff --git a/packages/spill/spill-local/src/cleanup.ts b/packages/spill/spill-local/src/cleanup.ts index c138f68a84..2de7198d96 100644 --- a/packages/spill/spill-local/src/cleanup.ts +++ b/packages/spill/spill-local/src/cleanup.ts @@ -1,6 +1,7 @@ /** Startup cleanup mechanics for local spill roots. */ -import { lstat, readdir, rmdir, unlink } from 'node:fs/promises' -import { join } from 'node:path' +import { lstat, readdir, realpath, rmdir, unlink } from 'node:fs/promises' +import type { Stats } from 'node:fs' +import { dirname, join } from 'node:path' import { tmpdir } from 'node:os' import { DEFAULT_ROOT_PREFIX, isErrno } from './store.ts' @@ -21,6 +22,14 @@ const DEFAULT_ROOT_RE = new RegExp(`^${DEFAULT_ROOT_PREFIX}[A-Za-z0-9]{6}$`) */ const SESSION_DIR_RE = /^session-[0-9a-f]{12}$/ +/** An existing root resolved to one stable filesystem identity. */ +interface ResolvedRoot { + /** Canonical absolute path used for the sweep. */ + path: string + /** Device/inode identity used to de-duplicate filesystem aliases. */ + identity: string +} + /** A one-argument warning sink — the sweep's only side effect on failure (never throws). */ export type WarnFn = (message: string) => void @@ -34,16 +43,116 @@ function warnSafely(warn: WarnFn, message: string): void { } } -/** One root to sweep, plus whether its empty session directories and root may be pruned. */ +/** Whether another local OS user cannot replace children of this directory. */ +function isTrustedDirectory(stats: Stats): boolean { + if (!stats.isDirectory()) return false + /* v8 ignore next -- POSIX ownership and mode bits have no Windows equivalent. */ + if (process.platform === 'win32' || process.geteuid === undefined) return true + return stats.uid === process.geteuid() && (stats.mode & 0o022) === 0 +} + +/** Stable identity for de-duplicating aliases of one root. */ +function rootIdentity(path: string, stats: Stats): string { + /* v8 ignore next -- Windows file indexes are not portable inode identities. */ + if (process.platform === 'win32') return path.toLowerCase() + return `${String(stats.dev)}:${String(stats.ino)}` +} + +/** + * Check that no ancestor permits another local OS user to replace the selected + * child. A sticky writable ancestor is safe because the child is owned by the + * current user; this admits normal per-process roots below `/tmp`. + */ +async function hasProtectedAncestors(path: string): Promise { + /* v8 ignore next -- POSIX ancestry checks have no Windows ACL equivalent. */ + if (process.platform === 'win32' || process.geteuid === undefined) return true + const currentUid = process.geteuid() + let child = path + let childStats = await lstat(child) + for (;;) { + const parent = dirname(child) + if (parent === child) return true + const stats = await lstat(parent) + /* v8 ignore next -- every ancestor of a successfully resolved path is a directory. */ + if (!stats.isDirectory()) return false + const writableByOthers = (stats.mode & 0o022) !== 0 + const sticky = (stats.mode & 0o1000) !== 0 + if (writableByOthers && !sticky) return false + /* v8 ignore next -- requires an ancestor owned by another OS account inside + a writable sticky parent; ordinary test fixtures cannot change uid. */ + if (writableByOthers && childStats.uid !== currentUid) return false + child = parent + childStats = stats + } +} + +/** + * Resolve one existing root without admitting a directory another local user + * can replace during the path-based sweep. A configured root may be a symlink; + * discovery passes `false` so a symlink cannot impersonate a default root. + * + * @param path Candidate root path. + * @param allowSymlink Whether the candidate itself may be a configured symlink. + * @param warn Sink for skipped or failed inspection. + * @returns The trusted canonical root, or `undefined` when it is absent or unsafe. + */ +async function resolveRoot(path: string, allowSymlink: boolean, warn: WarnFn): Promise { + let initial: Stats + try { + initial = await lstat(path) + } catch (error: unknown) { + /* v8 ignore start -- non-ENOENT inspection failures depend on host ACL or + an entry racing away and cannot be reproduced portably. */ + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to inspect root ${path}: ${String(error)}`) + return undefined + /* v8 ignore stop */ + } + if (initial.isSymbolicLink()) { + if (!allowSymlink) return undefined + } else if (!isTrustedDirectory(initial)) { + warnSafely(warn, `spill-local: skipped unsafe root ${path}: expected a directory owned by the current user and not writable by group or others`) + return undefined + } + + let canonical: string + let stats: Stats + try { + canonical = await realpath(path) + stats = await lstat(canonical) + } catch (error: unknown) { + /* v8 ignore start -- a root lstat'd above reaches this only by racing away + or by a host-specific realpath failure. */ + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to resolve root ${path}: ${String(error)}`) + return undefined + /* v8 ignore stop */ + } + let protectedAncestors = false + try { + protectedAncestors = await hasProtectedAncestors(canonical) + } catch (error: unknown) { + /* v8 ignore start -- a canonical ancestor disappears only through a race; + other failures depend on host ACLs. */ + if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to inspect ancestors of root ${canonical}: ${String(error)}`) + return undefined + /* v8 ignore stop */ + } + if (!isTrustedDirectory(stats) || !protectedAncestors) { + warnSafely(warn, `spill-local: skipped unsafe root ${canonical}: expected a current-user-owned directory with protected write and ancestor permissions`) + return undefined + } + return { path: canonical, identity: rootIdentity(canonical, stats) } +} + +/** One root to sweep, plus whether the root itself may be pruned once empty. */ export interface SweepRoot { /** Absolute spill root to sweep. */ path: string /** - * When `true`, prune empty `session-*` children and then remove the root once - * empty. Set for DISCOVERED prior-default `dsh-spill-*` roots (one per past - * process — otherwise they accumulate empty forever), never for the - * active/configured root the live process is still writing into. Writes retry - * if another process still using a discovered root races its pruning. + * When `true`, remove the root after its empty `session-*` children are + * pruned. Set for DISCOVERED prior-default `dsh-spill-*` roots (one per past + * process — otherwise they accumulate empty forever), never for the active + * root the live process is still writing into. Every root prunes empty session + * directories; writes retry if that races their removal. */ pruneWhenEmpty: boolean } @@ -141,26 +250,39 @@ async function sweepSessionDir(dir: string, cutoffMs: number, warn: WarnFn): Pro /** * Best-effort one-shot cleanup: across each root, delete expired regular files - * under its `session-*` directories, pruning empty directories only in - * discovered prior-default roots. The active root keeps its session directories - * to avoid racing a local write; writes recreate a directory pruned by another - * process. Every filesystem and warning-sink failure is contained, so a caller - * can await this during activation/disposal without it ever rejecting. + * under its `session-*` directories and prune every empty session directory. + * Only a discovered prior-default root is itself removed. Writes recreate a + * session directory when pruning races a local write. Every filesystem and + * warning-sink failure is contained, so a caller can await this during + * activation/disposal without it ever rejecting. * * @param options The roots to sweep, the age cutoff, and the failure sink. * @returns Resolves when the sweep finishes (never rejects). */ export async function sweepSpillRoots(options: SweepOptions): Promise { - const { roots, cutoffMs, warn } = options - for (const root of roots) { + const { cutoffMs, warn } = options + const roots = new Map() + for (const candidate of options.roots) { + const resolved = await resolveRoot(candidate.path, false, warn) + if (resolved === undefined) continue + const existing = roots.get(resolved.identity) + roots.set(resolved.identity, { + path: resolved.path, + pruneWhenEmpty: (existing?.pruneWhenEmpty ?? true) && candidate.pruneWhenEmpty, + }) + } + for (const root of roots.values()) { let entries: string[] try { entries = await readdir(root.path) } catch (error: unknown) { // A root that does not exist yet (no spill ever written) is the common // case, not an error: ENOENT is silent, anything else is reported. + /* v8 ignore start -- the trusted root was resolved immediately above; a + read failure now requires a race or host-specific ACL fault. */ if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to read root ${root.path}: ${String(error)}`) continue + /* v8 ignore stop */ } // Track whether the root holds ANY entry the sweep did not fully reclaim, so // a discovered prior-default root can be pruned only when nothing remains. @@ -185,15 +307,13 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { continue /* v8 ignore stop */ } - if (!stats.isDirectory()) { rootEmptiable = false; continue } - const empty = await sweepSessionDir(dir, cutoffMs, warn) - if (!empty) { rootEmptiable = false; continue } - if (!root.pruneWhenEmpty) { - // The active root remains writable while cleanup runs. Leaving its empty - // session directories in place closes the mkdir/rmdir race with saveText. + if (!isTrustedDirectory(stats)) { + warnSafely(warn, `spill-local: skipped unsafe session directory ${dir}`) rootEmptiable = false continue } + const empty = await sweepSessionDir(dir, cutoffMs, warn) + if (!empty) { rootEmptiable = false; continue } try { await rmdir(dir) } catch (error: unknown) { @@ -210,8 +330,7 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { } // A discovered prior-default root (one per past process) is removed once its // last session dir is gone — otherwise empty roots accumulate forever and - // every future startup rescans them. The active/configured root is never - // pruned (the live process is still writing into it). + // every future startup rescans them. The active root itself is never pruned. if (root.pruneWhenEmpty && rootEmptiable) { try { await rmdir(root.path) @@ -245,7 +364,7 @@ export async function sweepSpillRoots(options: SweepOptions): Promise { * @param base The directory to scan; defaults to the OS tmpdir (a test seam). * @returns Absolute paths of the discovered default roots (possibly empty). */ -export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir()): Promise { +async function discoverDefaultRootRecords(warn: WarnFn, base: string): Promise { let entries: string[] try { entries = await readdir(base) @@ -253,24 +372,48 @@ export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir() warnSafely(warn, `spill-local: failed to scan ${base} for default roots: ${String(error)}`) return [] } - const roots: string[] = [] + const roots: ResolvedRoot[] = [] for (const name of entries) { if (!DEFAULT_ROOT_RE.test(name)) continue const path = join(base, name) - let stats - try { - // lstat, not stat: a symlink named `dsh-spill-*` must not be treated as a - // root we then sweep (it could point anywhere). - stats = await lstat(path) - } catch (error: unknown) { - /* v8 ignore start -- an entry readdir just returned fails to lstat only by - racing away (ENOENT) or a permission/IO fault; not deterministically - reproducible. */ - if (!isErrno(error, 'ENOENT')) warnSafely(warn, `spill-local: failed to stat default root ${path}: ${String(error)}`) - continue - /* v8 ignore stop */ - } - if (stats.isDirectory()) roots.push(path) + const resolved = await resolveRoot(path, false, warn) + if (resolved !== undefined) roots.push(resolved) } return roots } + +/** + * Discover trusted prior default roots below the OS temporary directory. + * + * @param warn Sink for contained discovery failures. + * @param base Directory to scan; defaults to the OS temporary directory. + * @returns Canonical paths of trusted default roots. + */ +export async function discoverDefaultRoots(warn: WarnFn, base: string = tmpdir()): Promise { + return (await discoverDefaultRootRecords(warn, base)).map(root => root.path) +} + +/** + * Gather and de-duplicate the trusted roots for one startup sweep. The active + * configured path may be a symlink; its resolved identity overrides a matching + * discovered root so the live target is never marked prunable. + * + * @param activeRoot Active configured root. + * @param warn Sink for contained inspection failures. + * @param defaultRootsBase Directory holding prior default roots. + * @returns Trusted roots with the active identity marked non-prunable. + */ +export async function gatherSweepRoots( + activeRoot: string, + warn: WarnFn, + defaultRootsBase: string = tmpdir(), +): Promise { + const [discovered, active] = await Promise.all([ + discoverDefaultRootRecords(warn, defaultRootsBase), + resolveRoot(activeRoot, true, warn), + ]) + const roots = new Map() + for (const root of discovered) roots.set(root.identity, { path: root.path, pruneWhenEmpty: true }) + if (active !== undefined) roots.set(active.identity, { path: active.path, pruneWhenEmpty: false }) + return [...roots.values()] +} diff --git a/packages/spill/spill-local/src/index.ts b/packages/spill/spill-local/src/index.ts index f767a1a12d..5280ebdae0 100644 --- a/packages/spill/spill-local/src/index.ts +++ b/packages/spill/spill-local/src/index.ts @@ -15,7 +15,7 @@ import { tmpdir } from 'node:os' import z from '@deepseek-ai/schemastery' import { SpillLocator, SpillStore } from '@deepseek-ai/dsh-spill' import type { SaveTextSpill, SpillRef } from '@deepseek-ai/dsh-spill' -import { discoverDefaultRoots, sweepSpillRoots } from './cleanup.ts' +import { gatherSweepRoots, sweepSpillRoots } from './cleanup.ts' import type { SweepRoot, WarnFn } from './cleanup.ts' import { privateRoot, saveTextFile } from './store.ts' @@ -40,8 +40,10 @@ export interface Config { * cleanup sweep. Defaults to `30`; `0` disables cleanup entirely. Files whose * `mtime` is strictly older than the cutoff are deleted and emptied * directories are pruned; fresh files, symlinks, and unrelated entries are - * left untouched. Retention is deliberate — a resumed or forked session may - * still reference an older locator until it ages out. + * left untouched. On POSIX, cleanup skips roots and session directories that + * another local user could modify or replace. Retention is deliberate — a + * resumed or forked session may still reference an older locator until it + * ages out. */ cleanupPeriodDays?: number } @@ -63,7 +65,7 @@ type ResolvedConfig = Required> & Pick export class LocalSpillStore extends SpillStore { static Config: z = z.object({ root: z.string(), - cleanupPeriodDays: z.number().default(30), + cleanupPeriodDays: z.number().step(1).min(0).default(30), }) /** Resolved absolute spill root (config `root`, else the private default), fixed at construction. */ @@ -83,9 +85,6 @@ export class LocalSpillStore extends SpillStore { // schemastery (static Config) has already filled `cleanupPeriodDays`; the // cast records that runtime fact for exactOptionalPropertyTypes. this.config = config as ResolvedConfig - if (!Number.isInteger(this.config.cleanupPeriodDays) || this.config.cleanupPeriodDays < 0) { - throw new Error(`spill-local: cleanupPeriodDays must be a non-negative integer (got ${this.config.cleanupPeriodDays})`) - } this.root = config.root !== undefined ? resolve(config.root) : privateRoot() // One best-effort startup sweep, owned by the fiber. The generator body runs @@ -120,24 +119,19 @@ export class LocalSpillStore extends SpillStore { /** * The roots the startup sweep covers: each discovered prior-default * `dsh-spill-*` temp root (see {@link discoverDefaultRoots}), pruned when - * emptied, plus the active/configured root, whose root and session directories - * are NEVER pruned (the live process is still writing into them). The active - * root is de-duped out of the discovered set so it is not swept twice or - * marked prunable. A test - * overrides this to inject an isolated root set — and, being the sweep's one - * async gather point, to hold the sweep open across a disposal for the - * quiescence check; it is a test seam, not a deployment knob. + * emptied, plus the active/configured root, which is never itself pruned while + * the live process may write into it. Empty session directories are pruned in + * every root. Filesystem identity de-duplicates aliases before the active root + * overrides a discovered match as non-prunable. A test overrides this to + * inject an isolated root set — and, being the sweep's one async gather point, + * to hold the sweep open across a disposal for the quiescence check; it is a + * test seam, not a deployment knob. * * @param warn - sink for a contained discovery failure. * @returns The roots to sweep, each flagged for prune-when-empty. */ protected async gatherRoots(warn: WarnFn): Promise { - const discovered = await discoverDefaultRoots(warn, this.defaultRootsBase()) - const roots: SweepRoot[] = discovered - .filter(path => path !== this.root) - .map(path => ({ path, pruneWhenEmpty: true })) - roots.push({ path: this.root, pruneWhenEmpty: false }) - return roots + return gatherSweepRoots(this.root, warn, this.defaultRootsBase()) } /** diff --git a/packages/spill/spill-local/tests/loader-composition.spec.ts b/packages/spill/spill-local/tests/loader-composition.spec.ts new file mode 100644 index 0000000000..cf634fbc38 --- /dev/null +++ b/packages/spill/spill-local/tests/loader-composition.spec.ts @@ -0,0 +1,78 @@ +/** + * Real-composition proof: a cordis.yml loaded by the vendored Loader applies + * spill-local configuration and completes its fiber-owned startup cleanup. + */ + +import { mkdir, mkdtemp, rm, utimes, writeFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { afterEach, describe, expect, it } from 'vitest' +import { Context } from '@deepseek-ai/cordis' +import Loader from '@deepseek-ai/cordis-plugin-loader' +import Include from '@deepseek-ai/cordis-plugin-include' +import LocalSpillStore, { sessionDir } from '@deepseek-ai/dsh-spill-local' + +const DAY_MS = 24 * 60 * 60 * 1000 + +let root: string | undefined +let context: Context | undefined + +afterEach(async () => { + await context?.fiber.dispose() + context = undefined + if (root !== undefined) await rm(root, { recursive: true, force: true }) + root = undefined +}) + +describe('spill-local real Loader composition through cordis.yml', () => { + it('loads cleanupPeriodDays and prunes only expired session contents', async () => { + root = await mkdtemp(join(tmpdir(), 'dsh-spill-loader-')) + const oldDir = sessionDir(root, 'old-session') + const freshDir = sessionDir(root, 'fresh-session') + await mkdir(oldDir, { recursive: true }) + await mkdir(freshDir, { recursive: true }) + const old = join(oldDir, 'old.txt') + const fresh = join(freshDir, 'fresh.txt') + await writeFile(old, 'old') + await writeFile(fresh, 'fresh') + const now = Date.now() + await utimes(old, (now - 40 * DAY_MS) / 1000, (now - 40 * DAY_MS) / 1000) + await utimes(fresh, (now - DAY_MS) / 1000, (now - DAY_MS) / 1000) + + const configPath = join(root, 'cordis.yml') + await writeFile(configPath, [ + "- name: '@deepseek-ai/dsh-spill-local'", + ' config:', + ` root: ${JSON.stringify(root)}`, + ' cleanupPeriodDays: 30', + '', + ].join('\n')) + + context = new Context() + context.baseUrl = pathToFileURL(root).href + '/' + await context.plugin(Loader) + context.loader.builtins.include = Include + context.loader.internal = { + version: 'v2', + async import(specifier: string) { + if (specifier !== '@deepseek-ai/dsh-spill-local') throw new Error(`unexpected Loader import: ${specifier}`) + return LocalSpillStore + }, + } as unknown as NonNullable + await context.loader.create({ + name: 'cordis:include', + config: { path: pathToFileURL(configPath).href }, + }) + await context.loader.await() + await context.fiber.dispose() + context = undefined + + expect(existsSync(old)).toBe(false) + expect(existsSync(oldDir)).toBe(false) + expect(existsSync(fresh)).toBe(true) + expect(existsSync(freshDir)).toBe(true) + expect(existsSync(root)).toBe(true) + }, 30_000) +}) diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index 8bdb5a7471..329f1d9148 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -11,7 +11,7 @@ import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' import { Context } from '@deepseek-ai/cordis' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { basename, dirname, isAbsolute, join, normalize } from 'node:path' import { CallId } from '@deepseek-ai/dsh-llm' @@ -28,6 +28,7 @@ import LocalSpillStore, { sweepSpillRoots, } from '@deepseek-ai/dsh-spill-local' import type { SweepRoot } from '@deepseek-ai/dsh-spill-local' +import { gatherSweepRoots } from '../src/cleanup.ts' const DAY_MS = 24 * 60 * 60 * 1000 @@ -169,9 +170,9 @@ describe('LocalSpillStore service', () => { it('rejects a negative or fractional cleanupPeriodDays at load', async () => { await expect(new Context().plugin(LocalSpillStore, { root, cleanupPeriodDays: -1 })) - .rejects.toThrow(/cleanupPeriodDays must be a non-negative integer/) + .rejects.toThrow() await expect(new Context().plugin(LocalSpillStore, { root, cleanupPeriodDays: 1.5 })) - .rejects.toThrow(/cleanupPeriodDays must be a non-negative integer/) + .rejects.toThrow() }) it('defaults cleanupPeriodDays to 30', async () => { @@ -207,8 +208,8 @@ describe('LocalSpillStore service', () => { }) it('routes a sweep filesystem failure to ctx.logger.warn (service warn wiring)', async () => { - // A root that is a FILE, not a directory, makes readdir throw ENOTDIR inside - // the real sweep. The service's warn closure must forward it to + // A root that is a FILE, not a directory, is rejected by the real sweep. + // The service's warn closure must forward that failure to // ctx.logger.warn, and disposal must still settle cleanly. const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') const ctx = new Context() @@ -218,7 +219,7 @@ describe('LocalSpillStore service', () => { } const fiber = await ctx.plugin(Discovering, { root: filePath, cleanupPeriodDays: 30 }) await fiber.dispose() - expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipped unsafe root')) }) }) @@ -265,10 +266,11 @@ describe('startup cleanup sweep', () => { it('keeps a file exactly at the boundary (only strictly-older expires)', async () => { const dir = sessionDir(root, 'sess-1') mkdirSync(dir, { recursive: true }) - // mtime == cutoff: mtimeMs >= cutoffMs holds, so it is kept. Age it just - // under 30d to avoid the sub-millisecond race of "exactly now - 30d". - const boundary = join(dir, 'boundary.txt'); writeAged(boundary, 'x', 29.9) - await runSweep([active(root)]) + const cutoffMs = Date.now() - 30 * DAY_MS + const boundary = join(dir, 'boundary.txt') + writeFileSync(boundary, 'x') + utimesSync(boundary, cutoffMs / 1000, cutoffMs / 1000) + await sweepSpillRoots({ roots: [active(root)], cutoffMs, warn: () => {} }) expect(existsSync(boundary)).toBe(true) }) @@ -280,7 +282,7 @@ describe('startup cleanup sweep', () => { expect(existsSync(old)).toBe(true) }) - it('keeps active session directories after deleting expired files', async () => { + it('prunes empty active session directories after deleting expired files', async () => { const emptied = sessionDir(root, 'emptied') const kept = sessionDir(root, 'kept') mkdirSync(emptied, { recursive: true }) @@ -288,7 +290,7 @@ describe('startup cleanup sweep', () => { writeAged(join(emptied, 'a.txt'), 'x', 40) writeAged(join(kept, 'fresh.txt'), 'y', 1) await runSweep([active(root)]) - expect(existsSync(emptied)).toBe(true) + expect(existsSync(emptied)).toBe(false) expect(existsSync(kept)).toBe(true) }) @@ -322,6 +324,18 @@ describe('startup cleanup sweep', () => { expect(existsSync(link)).toBe(true) }) + it('skips a POSIX session directory writable by another local user', async () => { + if (process.platform === 'win32') return + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + chmodSync(dir, 0o777) + const warn = vi.fn() + await sweepSpillRoots({ roots: [active(root)], cutoffMs: Date.now(), warn }) + expect(existsSync(old)).toBe(true) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipped unsafe session directory')) + }) + it('sweeps only exact session-<12hex> names, not lookalikes', async () => { // `session-backup` and `session-<11hex>` match the old startsWith check but // are NOT backend-generated names; their old files must survive. @@ -351,12 +365,29 @@ describe('startup cleanup sweep', () => { await runSweep([{ path: prior, pruneWhenEmpty: true }, active(root)]) expect(existsSync(prior)).toBe(false) // discovered root pruned expect(existsSync(root)).toBe(true) // active root kept - expect(existsSync(activeDir)).toBe(true) // active session dirs remain writable + expect(existsSync(activeDir)).toBe(false) // empty active session dirs are pruned } finally { rmSync(prior, { recursive: true, force: true }) } }) + it('de-duplicates repeated roots and lets non-prunable status win', async () => { + const dir = sessionDir(root, 'sess-1') + mkdirSync(dir, { recursive: true }) + writeAged(join(dir, 'old.txt'), 'x', 40) + await sweepSpillRoots({ + roots: [ + { path: root, pruneWhenEmpty: true }, + { path: root, pruneWhenEmpty: false }, + { path: root, pruneWhenEmpty: true }, + ], + cutoffMs: Date.now() - 30 * DAY_MS, + warn: () => {}, + }) + expect(existsSync(dir)).toBe(false) + expect(existsSync(root)).toBe(true) + }) + it('does NOT prune a discovered root that still holds a fresh file', async () => { const prior = mkdtempSync(join(tmpdir(), 'dsh-spill-')) const priorDir = sessionDir(prior, 'sess'); mkdirSync(priorDir, { recursive: true }) @@ -424,6 +455,43 @@ describe('startup cleanup sweep', () => { } }) + it('de-dups a configured symlink alias by filesystem identity and keeps its target writable', async () => { + const fakeTmp = mkdtempSync(join(tmpdir(), 'dsh-faketmp-')) + const activeDefault = mkdtempSync(join(fakeTmp, DEFAULT_ROOT_PREFIX)) + const alias = join(root, 'configured-root') + symlinkSync(activeDefault, alias, process.platform === 'win32' ? 'junction' : 'dir') + const dir = sessionDir(activeDefault, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + try { + const roots = await gatherSweepRoots(alias, () => {}, fakeTmp) + expect(roots).toEqual([{ path: realpathSync(activeDefault), pruneWhenEmpty: false }]) + await sweepSpillRoots({ roots, cutoffMs: Date.now() - 30 * DAY_MS, warn: () => {} }) + expect(existsSync(old)).toBe(false) + expect(existsSync(activeDefault)).toBe(true) + const saved = await saveTextFile({ root: alias, sessionId: 'next', suggestedName: 'ok.txt', content: 'ok' }) + expect(readFileSync(saved.path, 'utf8')).toBe('ok') + } finally { + rmSync(fakeTmp, { recursive: true, force: true }) + } + }) + + it('skips a root that another POSIX user could replace', async () => { + if (process.platform === 'win32') return + const unsafeParent = join(root, 'unsafe-parent') + const unsafeRoot = join(unsafeParent, 'configured') + mkdirSync(unsafeRoot, { recursive: true, mode: 0o700 }) + const dir = sessionDir(unsafeRoot, 'sess-1') + mkdirSync(dir, { recursive: true }) + const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) + chmodSync(unsafeParent, 0o777) + const warn = vi.fn() + const roots = await gatherSweepRoots(unsafeRoot, warn, join(root, 'missing-discovery-base')) + expect(roots).toEqual([]) + expect(existsSync(old)).toBe(true) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipped unsafe root')) + }) + it('does not block activation but is awaited on disposal (quiescence)', async () => { const dir = sessionDir(root, 'sess-1') mkdirSync(dir, { recursive: true }) @@ -449,13 +517,13 @@ describe('startup cleanup sweep', () => { expect(existsSync(old)).toBe(false) }) - it('a filesystem failure is contained (logged, never thrown) and does not fail a spill write', async () => { + it('an unsafe root is contained (logged, never thrown)', async () => { const warn = vi.fn() - // A path that is a FILE, not a directory: readdir(root) throws ENOTDIR. The + // A path that is a FILE, not a directory, is not a valid cleanup root. The // sweep must log and return, never reject. const filePath = join(root, 'not-a-dir'); writeFileSync(filePath, 'x') await expect(sweepSpillRoots({ roots: [active(filePath)], cutoffMs: Date.now(), warn })).resolves.toBeUndefined() - expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to read root')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipped unsafe root')) }) it('contains an exception from the warning sink', async () => { @@ -484,7 +552,7 @@ describe('discoverDefaultRoots', () => { writeFileSync(join(base, `${DEFAULT_ROOT_PREFIX}file01`), 'x') // matches shape but is a file symlinkSync(realRoot, join(base, `${DEFAULT_ROOT_PREFIX}link01`)) // matches shape but is a symlink const found = await discoverDefaultRoots(() => {}, base) - expect(found).toEqual([realRoot]) + expect(found).toEqual([realpathSync(realRoot)]) } finally { rmSync(base, { recursive: true, force: true }) } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e763188fd0..639117a6a7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7978,6 +7978,12 @@ importers: '@deepseek-ai/cordis': specifier: workspace:^ version: link:../../../vendor/cordis + '@deepseek-ai/cordis-plugin-include': + specifier: workspace:^ + version: link:../../../vendor/include + '@deepseek-ai/cordis-plugin-loader': + specifier: workspace:^ + version: link:../../../vendor/loader '@deepseek-ai/dsh-brand': specifier: workspace:^ version: link:../../util/brand From 9f9cc130e213a3f482b8bc569250c6ef09581939 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 24 Aug 2026 20:53:40 +0800 Subject: [PATCH 6/8] test(spill-local): normalize Windows realpaths consistently --- packages/spill/spill-local/tests/spill-local.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index 329f1d9148..936a3b0f46 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -11,7 +11,8 @@ import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' import { Context } from '@deepseek-ai/cordis' -import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' +import { realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' import { basename, dirname, isAbsolute, join, normalize } from 'node:path' import { CallId } from '@deepseek-ai/dsh-llm' @@ -465,7 +466,7 @@ describe('startup cleanup sweep', () => { const old = join(dir, 'old.txt'); writeAged(old, 'x', 40) try { const roots = await gatherSweepRoots(alias, () => {}, fakeTmp) - expect(roots).toEqual([{ path: realpathSync(activeDefault), pruneWhenEmpty: false }]) + expect(roots).toEqual([{ path: await realpath(activeDefault), pruneWhenEmpty: false }]) await sweepSpillRoots({ roots, cutoffMs: Date.now() - 30 * DAY_MS, warn: () => {} }) expect(existsSync(old)).toBe(false) expect(existsSync(activeDefault)).toBe(true) @@ -552,7 +553,7 @@ describe('discoverDefaultRoots', () => { writeFileSync(join(base, `${DEFAULT_ROOT_PREFIX}file01`), 'x') // matches shape but is a file symlinkSync(realRoot, join(base, `${DEFAULT_ROOT_PREFIX}link01`)) // matches shape but is a symlink const found = await discoverDefaultRoots(() => {}, base) - expect(found).toEqual([realpathSync(realRoot)]) + expect(found).toEqual([await realpath(realRoot)]) } finally { rmSync(base, { recursive: true, force: true }) } From 97693bbc85de8fd0c8ccd58ecafb66f8b23ab6dd Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 24 Aug 2026 21:07:12 +0800 Subject: [PATCH 7/8] test(spill-local): cover platform-specific cleanup paths --- packages/spill/spill-local/src/cleanup.ts | 6 ++++++ packages/spill/spill-local/tests/spill-local.spec.ts | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/packages/spill/spill-local/src/cleanup.ts b/packages/spill/spill-local/src/cleanup.ts index 2de7198d96..f108d6b9a1 100644 --- a/packages/spill/spill-local/src/cleanup.ts +++ b/packages/spill/spill-local/src/cleanup.ts @@ -66,6 +66,8 @@ function rootIdentity(path: string, stats: Stats): string { async function hasProtectedAncestors(path: string): Promise { /* v8 ignore next -- POSIX ancestry checks have no Windows ACL equivalent. */ if (process.platform === 'win32' || process.geteuid === undefined) return true + /* v8 ignore start -- Windows takes the return above; POSIX tests exercise + the ancestor ownership and mode policy. */ const currentUid = process.geteuid() let child = path let childStats = await lstat(child) @@ -84,6 +86,7 @@ async function hasProtectedAncestors(path: string): Promise { child = parent childStats = stats } + /* v8 ignore stop */ } /** @@ -136,10 +139,13 @@ async function resolveRoot(path: string, allowSymlink: boolean, warn: WarnFn): P return undefined /* v8 ignore stop */ } + /* v8 ignore start -- Windows has no POSIX ownership or mode rejection path; + POSIX tests exercise both unsafe-directory conditions. */ if (!isTrustedDirectory(stats) || !protectedAncestors) { warnSafely(warn, `spill-local: skipped unsafe root ${canonical}: expected a current-user-owned directory with protected write and ancestor permissions`) return undefined } + /* v8 ignore stop */ return { path: canonical, identity: rootIdentity(canonical, stats) } } diff --git a/packages/spill/spill-local/tests/spill-local.spec.ts b/packages/spill/spill-local/tests/spill-local.spec.ts index 936a3b0f46..1948731bff 100644 --- a/packages/spill/spill-local/tests/spill-local.spec.ts +++ b/packages/spill/spill-local/tests/spill-local.spec.ts @@ -477,6 +477,10 @@ describe('startup cleanup sweep', () => { } }) + it('omits a missing active root', async () => { + expect(await gatherSweepRoots(join(root, 'missing'), () => {}, root)).toEqual([]) + }) + it('skips a root that another POSIX user could replace', async () => { if (process.platform === 'win32') return const unsafeParent = join(root, 'unsafe-parent') From d6f9931c4b4a7cd3d2124ece93fea7288859b997 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 24 Aug 2026 21:20:23 +0800 Subject: [PATCH 8/8] test(spill-local): exclude POSIX identity branches on Windows --- packages/spill/spill-local/src/cleanup.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/spill/spill-local/src/cleanup.ts b/packages/spill/spill-local/src/cleanup.ts index f108d6b9a1..cbe9fbe307 100644 --- a/packages/spill/spill-local/src/cleanup.ts +++ b/packages/spill/spill-local/src/cleanup.ts @@ -48,14 +48,20 @@ function isTrustedDirectory(stats: Stats): boolean { if (!stats.isDirectory()) return false /* v8 ignore next -- POSIX ownership and mode bits have no Windows equivalent. */ if (process.platform === 'win32' || process.geteuid === undefined) return true + /* v8 ignore start -- Windows takes the return above; POSIX tests exercise + owner and mode rejection. */ return stats.uid === process.geteuid() && (stats.mode & 0o022) === 0 + /* v8 ignore stop */ } /** Stable identity for de-duplicating aliases of one root. */ function rootIdentity(path: string, stats: Stats): string { /* v8 ignore next -- Windows file indexes are not portable inode identities. */ if (process.platform === 'win32') return path.toLowerCase() + /* v8 ignore start -- Windows uses the canonical path identity above; POSIX + tests exercise device and inode identity. */ return `${String(stats.dev)}:${String(stats.ino)}` + /* v8 ignore stop */ } /**