From 7e3d5332dc305f06289ef103c3b34cc8ed77ac8a Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 21 Aug 2026 17:48:45 +0800 Subject: [PATCH] fix(session): address migration review feedback --- docs/subsystems/persistence.i18n.yaml | 4 +- docs/subsystems/persistence.md | 26 +++++++- docs/subsystems/persistence.zh.md | 26 +++++++- packages/core/session/src/types.ts | 6 +- .../README.i18n.yaml | 4 +- .../session-persistence-jsonl/README.md | 2 +- .../session-persistence-jsonl/README.zh.md | 2 +- .../session-persistence-jsonl/src/index.ts | 16 +++-- .../tests/jsonl.spec.ts | 17 +++++ .../session-persistence/src/format-decoder.ts | 15 +++-- .../tests/format-decoder.spec.ts | 63 +++++++++++++++++++ scripts/type-equiv.manifest.json | 5 ++ 12 files changed, 162 insertions(+), 24 deletions(-) diff --git a/docs/subsystems/persistence.i18n.yaml b/docs/subsystems/persistence.i18n.yaml index 886f483490..57476a2fb6 100644 --- a/docs/subsystems/persistence.i18n.yaml +++ b/docs/subsystems/persistence.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/subsystems/persistence.md -persistence.md: 1480780b343e2d55544e441362abde58ffdffb2b -persistence.zh.md: 67bd8900fbcd0d006adb80da13fa8dbb2b6dd3e0 +persistence.md: c08ac3e37a34678b3731a251f727e1648e00211e +persistence.zh.md: 91d8b2549cb78f5fb4d00be63b4c7e787b910fad diff --git a/docs/subsystems/persistence.md b/docs/subsystems/persistence.md index 1480780b34..c08ac3e37a 100644 --- a/docs/subsystems/persistence.md +++ b/docs/subsystems/persistence.md @@ -51,8 +51,8 @@ Source: [`packages/core/session/src/types.ts`](../../packages/core/session/src/t interface SessionHeader { /** * On-disk format version, stamped from {@link SESSION_FORMAT_VERSION} when the - * session is created. A persistence backend rejects any other version on load - * (no migration — see the constant). + * session is created. Persistence refuses newer versions and older versions + * without a complete registered migration path. */ readonly version: number /** The session's id (mirrors the {@link Session}'s id). */ @@ -91,7 +91,27 @@ interface SessionHeader { ## Format refusal — logs a build cannot faithfully read -A backend refuses a log it cannot faithfully interpret with `SessionFormatUnsupportedError`, distinct from `SessionPersistenceCorruptionError` because nothing is damaged. A header `version` ahead of `SESSION_FORMAT_VERSION` names the direction ("written by a newer harness — upgrade the harness to open it"); one behind it states that this build ships no upgrade path. After legacy-shape normalization, an event type outside this build's generated vocabulary (`KNOWN_SESSION_EVENT_TYPES`, emitted by `gen-persistence-catalog`) refuses the same way unless the event's envelope carries `ignorable: true` — silently skipping an unrecognized required event could change how the rest of the log must be read. The message appends the raw log path when the backend keeps one artifact per session, so the refused text stays reachable. The JSONL backend refuses a foreign version straight from the raw header line, before validating today's header shape or decoding any event row — a structurally different future format still reports the upgrade direction, never "corrupt"; SQLite gates whole-file structure through its own `SCHEMA_VERSION` pragma first. Design rationale and the deferred upgrader chain live in the [session-log-version-mechanism note](../../.agents/notes/implemented/architecture/2026-08-10-session-log-version-mechanism.md). +A backend refuses a log it cannot faithfully interpret with `SessionFormatUnsupportedError`, distinct from `SessionPersistenceCorruptionError` because nothing is damaged. A header `version` ahead of `SESSION_FORMAT_VERSION` names the direction ("written by a newer harness — upgrade the harness to open it"); one behind it requires a complete registered adjacent-version migration path or names the missing step. After legacy-shape normalization, an event type outside this build's generated vocabulary (`KNOWN_SESSION_EVENT_TYPES`, emitted by `gen-persistence-catalog`) refuses the same way unless the event's envelope carries `ignorable: true` — silently skipping an unrecognized required event could change how the rest of the log must be read. The message appends the raw log path when the backend keeps one artifact per session, so the refused text stays reachable. The JSONL backend refuses a foreign version straight from the raw header line, before validating today's header shape or decoding any event row — a structurally different future format still reports the upgrade direction, never "corrupt"; SQLite gates whole-file structure through its own `SCHEMA_VERSION` pragma first. Design rationale lives in the [session-log-version-mechanism note](../../.agents/notes/implemented/architecture/2026-08-10-session-log-version-mechanism.md). + +## `SessionFormatMigration` — adjacent static format upgrades + +Each migration class declares one adjacent `from`/`to` pair and creates fresh state for one decode attempt. The decoder snapshots every header and event output as detached lossless JSON before the next migration receives it, preserves event sequence numbers, and calls optional EOF validation only after the complete event stream is consumed. The [package README](../../packages/session/session-persistence/README.md) owns the registration and version-bump procedure. + +```ts type-equiv +/** Static identity and constructor for one adjacent-version migration. */ +interface SessionFormatMigration { + /** Input Session format version. */ + readonly from: number + /** Output Session format version; must equal `from + 1`. */ + readonly to: number + /** + * Create fresh state for one header decode and its optional complete event + * stream. Instances are never shared across sessions or decode attempts. + * @returns a single-use migration instance. + */ + new(): SessionFormatMigrationInstance +} +``` ## `CreateSessionOptions` — seeding and metadata diff --git a/docs/subsystems/persistence.zh.md b/docs/subsystems/persistence.zh.md index 67bd8900fb..91d8b2549c 100644 --- a/docs/subsystems/persistence.zh.md +++ b/docs/subsystems/persistence.zh.md @@ -51,8 +51,8 @@ interface SessionLocation { interface SessionHeader { /** * On-disk format version, stamped from {@link SESSION_FORMAT_VERSION} when the - * session is created. A persistence backend rejects any other version on load - * (no migration — see the constant). + * session is created. Persistence refuses newer versions and older versions + * without a complete registered migration path. */ readonly version: number /** The session's id (mirrors the {@link Session}'s id). */ @@ -91,7 +91,27 @@ interface SessionHeader { ## 格式拒绝：本构建无法可靠读取的日志 -后端用 `SessionFormatUnsupportedError` 拒绝无法可靠解读的日志，它与 `SessionPersistenceCorruptionError` 区分，因为数据没有损坏。header 的 `version` 比 `SESSION_FORMAT_VERSION` 新时，消息说明方向（"由更新的 harness 写入，请升级 harness 后打开"）；比它旧时说明本构建没有升级路径。经过 legacy 形状归一化后，本构建生成词汇表（`KNOWN_SESSION_EVENT_TYPES`，由 `gen-persistence-catalog` 生成）之外的事件类型同样被拒绝，除非该事件的信封带 `ignorable: true`：静默跳过一个不认识的必需事件可能改变日志其余部分的解读方式。后端为每个会话保留独立文件时，消息附上原始日志路径，被拒绝的文本仍然可读。JSONL 后端直接从原始 header 行拒绝外来版本，先于当前 header 形状校验和任何事件行解码，因此结构完全不同的未来格式仍会报告升级方向，绝不会报"损坏"；SQLite 则先由自己的 `SCHEMA_VERSION` pragma 把关整个文件的结构。设计理由与推迟建设的升级器链见 [session-log 版本机制 Agent Note](../../.agents/notes/implemented/architecture/2026-08-10-session-log-version-mechanism.zh.md)。 +后端用 `SessionFormatUnsupportedError` 拒绝无法可靠解读的日志，它与 `SessionPersistenceCorruptionError` 区分，因为数据没有损坏。header 的 `version` 比 `SESSION_FORMAT_VERSION` 新时，消息说明方向（"由更新的 harness 写入，请升级 harness 后打开"）；比它旧时则要求一条完整注册的相邻版本迁移路径，否则会指出缺失步骤。经过 legacy 形状归一化后，本构建生成词汇表（`KNOWN_SESSION_EVENT_TYPES`，由 `gen-persistence-catalog` 生成）之外的事件类型同样被拒绝，除非该事件的信封带 `ignorable: true`：静默跳过一个不认识的必需事件可能改变日志其余部分的解读方式。后端为每个会话保留独立文件时，消息附上原始日志路径，被拒绝的文本仍然可读。JSONL 后端直接从原始 header 行拒绝外来版本，先于当前 header 形状校验和任何事件行解码，因此结构完全不同的未来格式仍会报告升级方向，绝不会报"损坏"；SQLite 则先由自己的 `SCHEMA_VERSION` pragma 把关整个文件的结构。设计理由见 [session-log 版本机制 Agent Note](../../.agents/notes/implemented/architecture/2026-08-10-session-log-version-mechanism.zh.md)。 + +## `SessionFormatMigration`：相邻静态格式升级 + +每个迁移 class 声明一组相邻的 `from`／`to`，并为一次解码创建全新状态。decoder 会将每次 header 和事件输出快照为分离的无损 JSON，再交给下一项迁移，同时保留事件 seq；只有完整消费事件流后，才会调用可选的 EOF 验证。[包 README](../../packages/session/session-persistence/README.zh.md)负责说明注册与版本递增步骤。 + +```ts type-equiv +/** Static identity and constructor for one adjacent-version migration. */ +interface SessionFormatMigration { + /** Input Session format version. */ + readonly from: number + /** Output Session format version; must equal `from + 1`. */ + readonly to: number + /** + * Create fresh state for one header decode and its optional complete event + * stream. Instances are never shared across sessions or decode attempts. + * @returns a single-use migration instance. + */ + new(): SessionFormatMigrationInstance +} +``` ## `CreateSessionOptions`：seed 与元数据 diff --git a/packages/core/session/src/types.ts b/packages/core/session/src/types.ts index 31ce28a01b..3c12f5ee07 100644 --- a/packages/core/session/src/types.ts +++ b/packages/core/session/src/types.ts @@ -35,7 +35,7 @@ export function SessionId(id: string): SessionId { * and enforced by every persistence backend on load. The single source of truth for the * version — write sites and the load-time check all read it. * While the harness is unreleased it is pinned at `0`: no compatibility is - * implied, incompatible logs are rejected, and no migration is provided. + * implied; older logs load only through a complete adjacent migration path. * * The version is a single monotonic integer with no major/minor split. Whether * a bump is needed is decided by what the WRITER emits, never by what a newer @@ -61,8 +61,8 @@ export const SESSION_FORMAT_VERSION = 0 export interface SessionHeader { /** * On-disk format version, stamped from {@link SESSION_FORMAT_VERSION} when the - * session is created. A persistence backend rejects any other version on load - * (no migration — see the constant). + * session is created. Persistence refuses newer versions and older versions + * without a complete registered migration path. */ readonly version: number /** The session's id (mirrors the {@link Session}'s id). */ diff --git a/packages/session/session-persistence-jsonl/README.i18n.yaml b/packages/session/session-persistence-jsonl/README.i18n.yaml index b4401a1dc3..3501fa53df 100644 --- a/packages/session/session-persistence-jsonl/README.i18n.yaml +++ b/packages/session/session-persistence-jsonl/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/session/session-persistence-jsonl/README.md -README.md: 00f893c134133207a1e9a12397c996c7c6c0c76c -README.zh.md: 800eaf6a8d758a18ac3bbecb87644b4f95838119 +README.md: 73e503d8f6741e84ee50be8cc4cbfbf2c07babc8 +README.zh.md: 9e778cb155f9f1d964bc9592057ae39bb0afb8cb diff --git a/packages/session/session-persistence-jsonl/README.md b/packages/session/session-persistence-jsonl/README.md index 00f893c134..73e503d8f6 100644 --- a/packages/session/session-persistence-jsonl/README.md +++ b/packages/session/session-persistence-jsonl/README.md @@ -35,7 +35,7 @@ The JSONL durable session-persistence backend — a concrete `SessionPersistence The default artifact is a standard concatenation of independent [Zstandard frames](../../../.agents/notes/implemented/architecture/2026-07-19-zstandard-jsonl-session-logs.md): one checksummed frame containing only the header line, followed by one checksummed frame per durable append batch. The backend uses Node's built-in Zstandard API with its default compression level and exposes no level knob. Listing reads and validates only the header frame. `compression: 'none'` keeps the same logical lines in the original raw representation. -A root belongs to one encoding. Startup discovery and targeted lookup reject the opposite suffix with an error naming the incompatible artifact and instructing the caller to select the matching mode or a separate root. Flat `/.jsonl*` artifacts are also rejected instead of ignored. Session format steps can replace a logical log within its configured encoding; there is no compression migration, mixed-root fallback, or dual write. +A root belongs to one encoding. Startup discovery and targeted lookup reject the opposite suffix with an error naming the incompatible artifact and instructing the caller to select the matching mode or a separate root. Flat `/.jsonl*` artifacts are also rejected instead of ignored. Session format migrations can replace a logical log within its configured encoding; there is no compression migration, mixed-root fallback, or dual write. ## Durability and crash semantics diff --git a/packages/session/session-persistence-jsonl/README.zh.md b/packages/session/session-persistence-jsonl/README.zh.md index 800eaf6a8d..9e778cb155 100644 --- a/packages/session/session-persistence-jsonl/README.zh.md +++ b/packages/session/session-persistence-jsonl/README.zh.md @@ -35,7 +35,7 @@ JSONL 持久会话存储后端：`SessionPersistence` 的一个具体实现（`d 默认产物是独立 [Zstandard frame](../../../.agents/notes/implemented/architecture/2026-07-19-zstandard-jsonl-session-logs.zh.md) 的标准拼接：一个仅包含 header 行的带 checksum frame，后跟每个持久 append 批次一个带 checksum frame。后端使用 Node 内置 Zstandard API 和默认压缩级别，不提供级别开关。列表只读取并验证 header frame。`compression: 'none'` 在原始表示中保留相同逻辑行。 -一个根只属于一种编码。启动发现和定向查找会拒绝相反 suffix，错误会命名不兼容产物，并指示调用方选择匹配 mode 或独立根。平铺 `/.jsonl*` 产物也会被拒绝，而不是忽略。Session 格式步骤可以在已配置编码内替换逻辑日志；不提供压缩迁移、混合根回退或双写。 +一个根只属于一种编码。启动发现和定向查找会拒绝相反 suffix，错误会命名不兼容产物，并指示调用方选择匹配 mode 或独立根。平铺 `/.jsonl*` 产物也会被拒绝，而不是忽略。Session 格式迁移可以在已配置编码内替换逻辑日志；不提供压缩迁移、混合根回退或双写。 ## 持久性与崩溃语义 diff --git a/packages/session/session-persistence-jsonl/src/index.ts b/packages/session/session-persistence-jsonl/src/index.ts index d4d8e5c19f..a6d222cf5c 100644 --- a/packages/session/session-persistence-jsonl/src/index.ts +++ b/packages/session/session-persistence-jsonl/src/index.ts @@ -39,9 +39,9 @@ export type { JsonlCompression } from './format.ts' const DEFAULT_PACK_CHUNKS = true const DEFAULT_COMPRESSION: JsonlCompression = 'zstd' /** - * Internal scheduling constant, not deployment configuration: balance - * frame-boundary event-loop yields against `setImmediate` overhead. One frame - * remains an indivisible synchronous decode. + * Internal scheduling constants, not deployment configuration: decode yields + * balance frame latency against `setImmediate` overhead; replacement batches + * bound memory and frame granularity without changing durable behavior. */ const ZSTD_DECODE_YIELD_INTERVAL_MS = 500 const REPLACEMENT_BATCH_SIZE = 128 @@ -605,8 +605,14 @@ export class JsonlSessionPersistence extends SessionPersistence implements Persi if (first === undefined) continue // empty/half-written file const rawMeta = parseStoredHeaderMeta(first) if (rawMeta === undefined) continue // not a session header - const identity = this.storedIdentity(rawMeta, path) - const meta = decodeStoredSessionHeader(rawMeta, identity.id, { kind: 'jsonl', path }) + const rawId = typeof rawMeta === 'object' && rawMeta !== null + ? (rawMeta as Record)['id'] + : undefined + const expectedId = typeof rawId === 'string' + ? SessionId(rawId) + : SessionId('') + const meta = decodeStoredSessionHeader(rawMeta, expectedId, { kind: 'jsonl', path }) + this.storedIdentity(rawMeta, path) await this.assertStoredIdentity(path, rawMeta, undefined, signal) signal?.throwIfAborted() if (ids.has(meta.id)) { diff --git a/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts b/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts index ff88640e34..2a10b14f2f 100644 --- a/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts +++ b/packages/session/session-persistence-jsonl/tests/jsonl.spec.ts @@ -1524,6 +1524,23 @@ describe('JsonlSessionPersistence: edge cases', () => { expect(await ctx.sessionPersistence.list()).toEqual([]) }) + it('listing refuses a future format before validating current identity fields', async () => { + const id = SessionId('future-list') + const path = rawLogPath(root, '/work', id) + await mkdir(dirname(path), { recursive: true }) + await writeFile(path, `${JSON.stringify({ type: 'session', version: 42, id: 123 })}\n`) + + for (const list of [ + () => ctx.sessionPersistence.list(), + () => ctx.sessionPersistence.listSnapshots(), + ]) { + const failure = await list().then(() => undefined, (error: unknown) => error as Error) + expect(failure?.name).toBe('SessionFormatUnsupportedError') + expect(failure?.message).toContain('session "123" uses log format v42') + expect(failure?.message).toMatch(/written by a newer harness.*upgrade the harness/) + } + }) + it('keeps the transcript in an extensible session-owned directory', async () => { const m = meta('owned-directory', '/project') await ctx.sessionPersistence.create(m) diff --git a/packages/session/session-persistence/src/format-decoder.ts b/packages/session/session-persistence/src/format-decoder.ts index 19eb383a03..398b487860 100644 --- a/packages/session/session-persistence/src/format-decoder.ts +++ b/packages/session/session-persistence/src/format-decoder.ts @@ -33,8 +33,9 @@ interface SessionFormatMigrationInstance { */ header(meta: unknown): unknown /** - * Transform exactly one event while retaining its sequence number. Instance - * fields may accumulate facts from the header and earlier events. + * Transform exactly one event into detached lossless JSON while retaining + * its sequence number. Instance fields may accumulate facts from the header + * and earlier events. * @param event - detached input event in durable sequence order. * @returns exactly one detached event for the same sequence number. */ @@ -137,7 +138,10 @@ export interface DecodedSession { readonly revision: SessionPersistenceRevision /** Validated current-format events at or past the requested sequence. */ readonly events: AsyncIterable - /** Completion metadata from the physical read supplying the events. */ + /** + * Completion metadata from the physical read supplying the events. Settles + * only after the events iterable is fully consumed or fails. + */ readonly completed: Promise> } @@ -363,7 +367,10 @@ async function* transformEvents( const sourceSeq = asStoredRecord(value)?.['seq'] let output: unknown try { - output = instance.event(value) + output = snapshotJsonValue(instance.event(value)) + if (output === undefined) { + throw new Error('migration returned an event that is not losslessly JSON-serializable') + } } catch (error: unknown) { throw new Error( `session "${id}" event migration v${Migration.from} -> v${Migration.to} failed at seq ${String(sourceSeq)}`, diff --git a/packages/session/session-persistence/tests/format-decoder.spec.ts b/packages/session/session-persistence/tests/format-decoder.spec.ts index 5ba23fab17..5b80ac15c6 100644 --- a/packages/session/session-persistence/tests/format-decoder.spec.ts +++ b/packages/session/session-persistence/tests/format-decoder.spec.ts @@ -316,6 +316,69 @@ describe('versioned Session format decoder', { concurrent: false }, () => { expect(events[0]?.data).toMatchObject({ migrationPath: [0, 1] }) }) + it('detaches each migration output before the next migration mutates its input', async () => { + const retained: Array> = [] + const first = defineMigration(0, () => ({ + header: meta => ({ ...(meta as Record), version: 1 }), + event(value) { + const event = value as SessionEvent + const output = { + ...event, + data: { ...(event.data as Record), first: true }, + } + retained.push(output.data) + return output + }, + })) + const second = defineMigration(1, () => ({ + header: meta => ({ ...(meta as Record), version: 2 }), + event(value) { + const event = value as SessionEvent + const data = event.data as Record + data['second'] = true + return event + }, + })) + const { decodeStoredSession } = await configuredDecoder(2, [first, second]) + + const decoded = decodeStoredSession(storedSource(0, eventLog()).source, id) + const events = await collectEvents(decoded.events) + await decoded.completed + + expect(events.every(event => (event.data as Record)['second'] === true)).toBe(true) + expect(retained.every(data => data['second'] === undefined)).toBe(true) + }) + + it('rejects a non-JSON event output before a later migration can repair it', async () => { + const first = defineMigration(0, () => ({ + header: meta => ({ ...(meta as Record), version: 1 }), + event(value) { + const event = value as SessionEvent + return { + ...event, + data: { ...(event.data as Record), transient: undefined }, + } + }, + })) + const second = defineMigration(1, () => ({ + header: meta => ({ ...(meta as Record), version: 2 }), + event(value) { + const event = value as SessionEvent + const data = event.data as Record + delete data['transient'] + return event + }, + })) + const { decodeStoredSession } = await configuredDecoder(2, [first, second]) + + const failure = await decodedFailure( + decodeStoredSession(storedSource(0, eventLog()).source, id), + ) + + expect(failure.message).toMatch(/event migration v0 -> v1 failed at seq 0/) + expect((failure.cause as Error).message).toMatch(/not losslessly JSON-serializable/) + }) + it('plans by version even when registry entries are declared out of order', async () => { const calls: string[] = [] const { decodeStoredSession } = await configuredDecoder( diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index 8b580750bd..2b29010478 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -545,6 +545,11 @@ "symbol": "SessionLocation", "source": "packages/session/session-persistence/src/index.ts" }, + { + "doc": "docs/subsystems/persistence.md", + "symbol": "SessionFormatMigration", + "source": "packages/session/session-persistence/src/format-decoder.ts" + }, { "doc": "docs/subsystems/persistence.md", "symbol": "SessionRawArtifact",