[Bug] A single corrupted session log prevents the whole workspace from booting (loader entry "workspace" fails to apply) #1473
ConradLu2740
started this conversation in
General
Replies: 4 comments
|
Verified — this is a real resilience gap, and it's directly connected to the session-log corruption family already reported today (#1333, #1452: seq-gap corruption from concurrent writers; #1449: emoji-truncated tool result poisoning a session). Your report is the third distinct way a single bad log becomes a total outage instead of a per-session one. Source confirmation: Source confirmation (master)
Why this matters more than it looksThe three corruption paths reported today share a theme: the loader treats one bad artifact as a fatal boot error, while the data is usually recoverable:
In all three, the blast radius is "whole workspace/process dead" when the actual damage is one session. The durability posture ( Suggested fix direction
This deserves a regression test: a workspace containing one artifact whose first frame decodes to 2 lines must still boot, with that session reported as unavailable. Want me to draft the |
0 replies
|
单个损坏会话日志拖垮整个 workspace boot——启动容错问题（一个坏文件全盘失败）。 临时：找到 |
0 replies
|
Thanks @argszero for the source confirmation and for connecting this to #1333/#1452/#1449 — the "recoverable data, whole-process blast radius" framing matches exactly what I hit. Yes — here's the skip-and-warn patch, ready to apply/cherry-pick. Since upstream doesn't take external PRs right now, I'm attaching it here per the community template. The patch
git apply 1473-skip-and-warn.patch # or: git cherry-pick <commit> once committedWhat changed
Verification
Notes / known boundaries
Happy to adjust the shape (e.g., return a Patch (full, for direct copy-paste)diff --git a/packages/session/session-persistence-jsonl/src/index.ts b/packages/session/session-persistence-jsonl/src/index.ts
index 5113746..5722ab2 100644
--- a/packages/session/session-persistence-jsonl/src/index.ts
+++ b/packages/session/session-persistence-jsonl/src/index.ts
@@ -9,7 +9,7 @@
import { Context } from '@deepseek-ai/cordis'
import z from '@deepseek-ai/schemastery'
import { readdirSync } from 'node:fs'
-import { open, mkdir, readFile, readdir, realpath, link, rm, stat, truncate } from 'node:fs/promises'
+import { open, mkdir, readFile, readdir, realpath, link, rename, rm, stat, truncate } from 'node:fs/promises'
import { dirname, join, resolve } from 'node:path'
import { performance } from 'node:perf_hooks'
import { scheduler } from 'node:timers/promises'
@@ -50,6 +50,25 @@ function assertZstdHeaderFrame(plaintext: Buffer): void {
}
}
+/**
+ * Whether an error raised on the listing path means one session artifact is
+ * corrupt (skippable) rather than a configuration or format conflict that must
+ * stay loud.
+ *
+ * Scope note: this classifies the JSONL backend's own un-wrapped errors thrown
+ * while `listArtifacts` reads/validates one artifact's header frame — message
+ * prefixes `corrupt Zstandard session log` / `corrupt session log`. Errors
+ * wrapped by the coordinator (e.g. {@link SessionPersistenceCorruptionError})
+ * or thrown on other paths are intentionally NOT matched here; callers that
+ * need broader corruption detection should test `instanceof` themselves.
+ */
+export function isCorruptSessionLogError(error: unknown): boolean {
+ return error instanceof Error && (
+ error.message.startsWith('corrupt Zstandard session log')
+ || error.message.startsWith('corrupt session log')
+ )
+}
+
/** Loader schema for the JSONL artifact's physical encoding. */
export const JsonlCompressionSchema: z<JsonlCompression> = z.union([
z.const('zstd'),
@@ -76,6 +95,13 @@ export interface Config {
packChunks?: boolean
/** Physical encoding; defaults to checksummed Zstandard frames. */
compression?: JsonlCompression
+ /**
+ * When a session artifact fails header validation during listing, rename it
+ * to a `.corrupt-<ts>` sibling so it stops being enumerated, instead of only
+ * skipping it with a warning. Off by default: listing stays best-effort and
+ * never mutates user files unless asked. The renamed path is logged either way.
+ */
+ quarantineCorrupt?: boolean
/** Maximum cold Session preparations retained for history-to-resume reuse. */
preparedSessionCacheSize?: number
/** Fixed live-event coalescing window; not a backend completion deadline. */
@@ -127,6 +153,7 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi
root: z.string().required(),
packChunks: z.boolean().default(DEFAULT_PACK_CHUNKS),
compression: JsonlCompressionSchema,
+ quarantineCorrupt: z.boolean().default(false),
preparedSessionCacheSize: z.number().step(1).min(1).default(DEFAULT_PREPARED_SESSION_CACHE_SIZE),
writeBatchMaxDelayMs: z.number().step(1).min(1).max(MAX_WRITE_BATCH_DELAY_MS)
.default(DEFAULT_WRITE_BATCH_MAX_DELAY_MS),
@@ -142,6 +169,7 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi
private root: string
private packChunks: boolean
private compression: JsonlCompression
+ private quarantineCorrupt: boolean
private coordinator: PersistenceCoordinator<JsonlTornMarker>
private rootEncodingCheck: Promise<void> | undefined
@@ -156,6 +184,7 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi
?? DEFAULT_WRITE_BATCH_MAX_DELAY_MS
this.packChunks = config.packChunks ?? DEFAULT_PACK_CHUNKS
this.compression = config.compression ?? DEFAULT_COMPRESSION
+ this.quarantineCorrupt = config.quarantineCorrupt ?? false
this.assertUsableRoot()
this.coordinator = new PersistenceCoordinator<JsonlTornMarker>(this.ctx, this, {
preparedSessionCacheSize,
@@ -469,6 +498,35 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi
return snapshots
}
+ /**
+ * Handle one artifact that failed header read/validation: warn (always) and,
+ * when configured, quarantine the corrupt file so it stops being enumerated.
+ * @returns whether the artifact should be skipped (corruption, not a loud error).
+ */
+ private async quarantineCorruptArtifact(path: string, error: unknown): Promise<boolean> {
+ if (!isCorruptSessionLogError(error)) return false
+ if (this.quarantineCorrupt) {
+ try {
+ const target = `${path}.corrupt-${Date.now()}`
+ await rename(path, target)
+ this.ctx.logger.warn(
+ `${this.name}: corrupt session artifact "${path}" quarantined to "${target}": ${String(error)}`,
+ )
+ } catch (renameError) {
+ // Renaming must never take down startup; the artifact stays skipped with a warning.
+ // A concurrent listing may have already renamed it (ENOENT) — that's expected, not noise.
+ if (!isENOENT(renameError)) {
+ this.ctx.logger.warn(
+ `${this.name}: corrupt session artifact "${path}" could not be quarantined: ${String(renameError)}`,
+ )
+ }
+ }
+ } else {
+ this.ctx.logger.warn(`${this.name}: skipping corrupt session artifact "${path}": ${String(error)}`)
+ }
+ return true
+ }
+
private async listArtifacts(signal?: AbortSignal): Promise<Array<{ header: SessionHeader; path: string }>> {
signal?.throwIfAborted()
await this.ensureRootEncoding()
@@ -488,14 +546,27 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi
signal?.throwIfAborted()
if (!pathExists) continue
// Read only headers so listing scales with session count, not log size.
- const first = this.compression === 'zstd'
- ? await this.readFirstZstdLine(path, signal)
- : await this.readFirstLine(path, signal)
+ let first: string | undefined
+ try {
+ first = this.compression === 'zstd'
+ ? await this.readFirstZstdLine(path, signal)
+ : await this.readFirstLine(path, signal)
+ } catch (error) {
+ signal?.throwIfAborted()
+ if (await this.quarantineCorruptArtifact(path, error)) continue
+ throw error
+ }
signal?.throwIfAborted()
if (first === undefined) continue // empty/half-written file
const meta = parseHeaderMeta(first)
if (meta === undefined) continue // not a session header
- await this.assertStoredIdentity(path, meta, undefined, signal)
+ try {
+ await this.assertStoredIdentity(path, meta, undefined, signal)
+ } catch (error) {
+ signal?.throwIfAborted()
+ if (await this.quarantineCorruptArtifact(path, error)) continue
+ throw error
+ }
signal?.throwIfAborted()
if (ids.has(meta.id)) {
throw new Error(`duplicate JSONL session id "${meta.id}" appears in multiple project directories`)
diff --git a/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts b/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts
index 70ae6d1..3b7bab8 100644
--- a/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts
+++ b/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts
@@ -1302,13 +1302,18 @@ describe('JsonlSessionPersistence: edge cases', () => {
.toThrow(/retired policy baseline fields/)
})
- it('list rejects a header whose cwd does not identify its physical log', async () => {
+ it('list skips a header whose cwd does not identify its physical log (per-artifact corruption)', async () => {
const m = meta('misplaced', '/stored')
await ctx.sessionPersistence.create(m)
await ctx.sessionPersistence.append(m.id, oneTurnLog())
await rewriteHeader(rawLogPath(root, m.cwd, m.id), (header) => { header.cwd = '/elsewhere' })
- await expect(ctx.sessionPersistence.list()).rejects.toThrow(/and cwd identify/)
+ // The header/identity mismatch is per-artifact corruption: listing skips it
+ // (with a warning) so the rest of the workspace still boots.
+ const warn = vi.spyOn(ctx.logger, 'warn')
+ expect(await ctx.sessionPersistence.list()).toEqual([])
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipping corrupt session artifact'))
+ warn.mockRestore()
})
it('accepts an alternate project path only when it identifies the same physical log', async () => {
@@ -1328,14 +1333,17 @@ describe('JsonlSessionPersistence: edge cases', () => {
expect((await ctx.sessionPersistence.list()).map(header => header.id)).toContain(m.id)
})
- it('list rejects a session header whose id cannot name a storage path', async () => {
+ it('list skips a session header whose id cannot name a storage path (per-artifact corruption)', async () => {
const dir = join(projectDir(root, undefined), 'invalid-id')
await mkdir(dir, { recursive: true })
await writeFile(join(dir, 'session.jsonl'), JSON.stringify({
type: 'session', version: 0, id: '', createdAt: 1, delegationDepth: 0,
}) + '\n')
- await expect(ctx.sessionPersistence.list()).rejects.toThrow(/header id cannot name a storage path/)
+ const warn = vi.spyOn(ctx.logger, 'warn')
+ expect(await ctx.sessionPersistence.list()).toEqual([])
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipping corrupt session artifact'))
+ warn.mockRestore()
})
it('load and list reject one id materialized in multiple project directories', async () => {
diff --git a/packages/session/session-persistence-jsonl/tests/zstd.spec.ts b/packages/session/session-persistence-jsonl/tests/zstd.spec.ts
index b9cced0..525ea1f 100644
--- a/packages/session/session-persistence-jsonl/tests/zstd.spec.ts
+++ b/packages/session/session-persistence-jsonl/tests/zstd.spec.ts
@@ -649,7 +649,7 @@ describe('JsonlSessionPersistence: default Zstandard encoding', () => {
expect((await ctx.sessionPersistence.load(header.id)).events).toEqual([...oneTurnLog(), ...secondTurn])
})
- it('skips empty, incomplete, and non-header compressed artifacts while rejecting malformed header frames', async () => {
+ it('skips empty, incomplete, and non-header compressed artifacts while rejecting malformed header frames on targeted reads', async () => {
const root = await freshRoot()
for (const [id, content] of [
['empty', Buffer.alloc(0)],
@@ -663,6 +663,9 @@ describe('JsonlSessionPersistence: default Zstandard encoding', () => {
const ctx = await mount(root)
expect(await ctx.sessionPersistence.list()).toEqual([])
+ // A structurally complete frame decoding to 0 or 2+ lines is corrupt: listing
+ // now skips it best-effort (with a warning) instead of taking the whole
+ // workspace down, while a targeted load still fails loudly.
const twoLinesId = SessionId('two-lines')
await mkdir(sessionDir(root, undefined, twoLinesId), { recursive: true })
await writeFile(logPath(root, undefined, twoLinesId, 'zstd'), await compressZstdFrame([
@@ -670,11 +673,67 @@ describe('JsonlSessionPersistence: default Zstandard encoding', () => {
JSON.stringify({ type: 'turn/start' }),
'',
].join('\n')))
- await expect(ctx.sessionPersistence.list()).rejects.toThrow(/first frame is not exactly one header line/)
+ const warn = vi.spyOn(ctx.logger, 'warn')
+ expect(await ctx.sessionPersistence.list()).toEqual([])
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipping corrupt session artifact'))
+ warn.mockRestore()
await expect(ctx.sessionPersistence.load(SessionId('two-lines')))
.rejects.toThrow(/first frame is not exactly one header line/)
})
+ it('keeps listing healthy sessions when a sibling artifact is corrupt (workspace boot resilience)', async () => {
+ const root = await freshRoot()
+ // One healthy session that must survive the corrupt sibling below.
+ const healthy = meta('healthy-boot', '/boot')
+ const healthyCtx = await mount(root)
+ await healthyCtx.sessionPersistence.create(healthy)
+ await healthyCtx.sessionPersistence.append(healthy.id, oneTurnLog())
+
+ // One corrupt artifact: structurally complete first frame decoding to 2 lines.
+ const bad = meta('corrupt-boot', '/boot')
+ await mkdir(sessionDir(root, bad.cwd, bad.id), { recursive: true })
+ await writeFile(logPath(root, bad.cwd, bad.id, 'zstd'), await compressZstdFrame([
+ JSON.stringify(toHeaderLine(bad)),
+ JSON.stringify({ type: 'turn/start' }),
+ '',
+ ].join('\n')))
+
+ const warn = vi.spyOn(healthyCtx.logger, 'warn')
+ const listed = (await healthyCtx.sessionPersistence.list()).map(h => h.id)
+ expect(listed).toEqual([healthy.id])
+ expect(listed).not.toContain(bad.id)
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipping corrupt session artifact'))
+ warn.mockRestore()
+ })
+
+ it('quarantines a corrupt artifact to a .corrupt-<ts> sibling when configured', async () => {
+ const root = await freshRoot()
+ const healthy = meta('healthy-quarantine', '/q')
+ const ctx = new Context()
+ contexts.push(ctx)
+ await ctx.plugin(SessionStore)
+ await ctx.plugin(JsonlSessionPersistence, { root, quarantineCorrupt: true })
+ await ctx.sessionPersistence.create(healthy)
+ await ctx.sessionPersistence.append(healthy.id, oneTurnLog())
+
+ const bad = meta('corrupt-quarantine', '/q')
+ await mkdir(sessionDir(root, bad.cwd, bad.id), { recursive: true })
+ const badPath = logPath(root, bad.cwd, bad.id, 'zstd')
+ await writeFile(badPath, await compressZstdFrame([
+ JSON.stringify(toHeaderLine(bad)),
+ JSON.stringify({ type: 'turn/start' }),
+ '',
+ ].join('\n')))
+
+ const warn = vi.spyOn(ctx.logger, 'warn')
+ const listed = (await ctx.sessionPersistence.list()).map(h => h.id)
+ expect(listed).toEqual([healthy.id])
+ // The corrupt artifact was renamed away and logged.
+ await expect(stat(badPath)).rejects.toThrow()
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('quarantined to'))
+ warn.mockRestore()
+ })
+
it('rejects missing, empty, and checksum-corrupt header frames on targeted reads', async () => {
const root = await freshRoot()
for (const id of ['partial-only', 'empty-header', 'bad-checksum']) {
@@ -691,7 +750,11 @@ describe('JsonlSessionPersistence: default Zstandard encoding', () => {
.rejects.toThrow(/empty or header-less Zstandard session log/)
await expect(ctx.sessionPersistence.load(SessionId('empty-header')))
.rejects.toThrow(/first frame is not exactly one header line/)
- await expect(ctx.sessionPersistence.list()).rejects.toThrow(/header frame failed validation/)
+ // Listing is best-effort: the checksum-corrupt artifact is skipped with a warning.
+ const warn = vi.spyOn(ctx.logger, 'warn')
+ expect(await ctx.sessionPersistence.list()).toEqual([])
+ expect(warn).toHaveBeenCalledWith(expect.stringContaining('skipping corrupt session artifact'))
+ warn.mockRestore()
})
}) |
0 replies
|
I hit the same failure today with the npm release Additional forensic evidenceThe affected file was not a broken Zstd stream:
The bad file's frame descriptor/content-size metadata was also consistent with an external one-shot compression of the full plaintext stream. By contrast, the repository's I could not find the exact initiating command in shell history or recorded tool calls, so “external one-shot recompression” is a forensic inference rather than direct attribution. The important distinction is that this is structural corruption, not JSON or Zstd payload corruption. Tested lossless recoveryI recovered the session without dropping any records by stopping DSH, preserving the original compressed file, splitting the decompressed stream before line 2, and recompressing the two parts as concatenated Zstd frames: log=/absolute/path/to/session.jsonl.zstd
tmp_dir="$(mktemp -d "$(dirname "$log")/.dsh-repair.XXXXXX")"
cp -p "$log" "${log}.pre-reframe.bak"
original_sha="$(zstd -q -d -c "$log" | shasum -a 256 | awk '{print $1}')"
zstd -q -d -f "$log" -o "$tmp_dir/session.jsonl"
csplit -s -f "$tmp_dir/part-" -n 2 "$tmp_dir/session.jsonl" 2
test "$(wc -l < "$tmp_dir/part-00" | tr -d ' ')" = 1
zstd -q -f "$tmp_dir/part-00" "$tmp_dir/part-01" \
-o "$tmp_dir/session.repaired.jsonl.zstd"
zstd --test "$tmp_dir/session.repaired.jsonl.zstd"
zstd --list -v "$tmp_dir/session.repaired.jsonl.zstd"
repaired_sha="$(zstd -q -d -c "$tmp_dir/session.repaired.jsonl.zstd" | shasum -a 256 | awk '{print $1}')"
test "$original_sha" = "$repaired_sha"
# Only after all checks pass, replace the inactive log atomically on the same filesystem.
mv "$tmp_dir/session.repaired.jsonl.zstd" "$log"The repaired file had two frames, its decompressed SHA-256 matched the original exactly, DSH then started successfully, and the WebUI returned HTTP 200. The Suggested upstream handlingThe skip-and-warn patch already proposed in this discussion is useful for workspace availability, but it does not recover the affected session. Since the dedicated first frame is intentionally used for metadata-only reads, I suggest keeping that invariant and adding an offline repair path, for example
This also appears to be the same structural failure reported in #1043 and #1047. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
A single corrupted session log under
~/.dsh/sessions/<workspace>/is enough to makedsh webfail to boot entirely. During startup the workspace plugin (@deepseek-ai/dsh-workspace) callssessionPersistence.list(), which reads the first zstd frame of every session. When one frame is malformed,assertZstdHeaderFramethrows, the loader entryworkspacefails to apply, and the whole plugin tree fails to load — the process exits instead of starting.One bad file = total outage, not just "that session's history fails to load".
Reproduction
Minimal repro (one-line-per-frame zstd, Node ≥ 22.15 / 24
node:zlib):node repro-corrupt-session.mjs --mode corrupt-header dsh web # exits with the error belowThe script builds a minimal valid session (header + 7 events, each compressed as its own checksummed zstd frame) and then packs two JSONL lines into the first frame (header + first event), so the first frame is no longer "exactly one header line".
The corrupt artifact is 817 bytes / 7 frames; frame 0 (199 bytes) decodes to 2 JSONL lines instead of 1.
Actual result
Call chain:
dsh-workspace [cordis.init]→sessionPersistence.list()(dsh-session-persistence-jsonl:listArtifacts→readFirstZstdLine→assertZstdHeaderFrame) → throw →Fiber._reload→Entry._start/Entry._initfails → root loaderawait()throws → process exits.Expected
A corrupted session should be skipped or quarantined (or at most fail that one session), so the rest of the workspace still boots. Note the asymmetry that makes one bad file a full outage:
readFirstZstdLinealready skips a torn (incomplete) first frame:scanZstdFramesreturns no complete frame →readFirstZstdLinereturnsundefined→listArtifactscontinues. (Verified: truncating the first frame mid-header letsdsh webboot fine.)assertZstdHeaderFrame, which throws and aborts the entirelist().Root cause (source, 0.1.0-rc.6)
dsh-session-persistence-jsonl/lib/index.jsassertZstdHeaderFrame(≈742): throws whenplaintext.length === 0 || plaintext.indexOf(10) !== plaintext.length - 1.readFirstZstdLine(≈1279): reads the first frame and calls the assertion; the throw propagates throughlistArtifacts(≈1078) andlist(≈1038).dsh-workspace/lib/index.js[cordis.init](≈324): callssessionPersistence.list()during startup; the throw fails the whole loader entry.Environment
0.1.0-rc.6(npm@deepseek-ai/dsh, HEAD47f9438)dsh-session-persistence-jsonl(zstd)Related
corrupt session log: seq gap— those fail one session's history load; this report is about a single malformed header taking down the whole workspace boot.Found while developing
@proactive-agent/dshplugins (pa-dsh).All reactions