From c6e62ff9710df49cc9eff5e75d71837d26a37245 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:12:28 +0800 Subject: [PATCH] fix(persistence): retain explicitly forbidden JSON fields A reader can reject a property name while its type declares that property as optional never. Dropping it from the extracted schema makes a future optional JSON value look additive, although the existing reader refuses it. The type history must retain that explicit prohibition. Support the argument-free @persistenceReserved marker only on optional properties whose values are never or undefined. Keep those properties as optional primitive-never nodes, so later JSON values change an existing field and require a version bump. Reject invalid placement, arguments, repeated tags, required or JSON-valued declarations, and mapped uses that violate the reservation. Keep every unmarked omission rule unchanged. The classification regression failed before this change with an additive optional-property result and now requires a type-change bump. All fifteen focused reservation cases and all 210 tooling tests pass, including inherited fields, misuse, and unmarked preservation. Host typecheck, scoped lint, and pairing pass. Regeneration leaves the unannotated baseline catalog and inventory byte-identical; all 61 roots still match their existing history. No runtime property is activated in this tooling commit. --- ...-09-17-persistence-schema-review.i18n.yaml | 4 +- .../2026-09-17-persistence-schema-review.md | 4 ++ ...2026-09-17-persistence-schema-review.zh.md | 4 ++ docs/persistence-changes/README.i18n.yaml | 4 +- docs/persistence-changes/README.md | 2 + docs/persistence-changes/README.zh.md | 2 + scripts/persistence-schema.spec.ts | 52 +++++++++++++++++++ scripts/persistence-schema.ts | 36 ++++++++++++- 8 files changed, 102 insertions(+), 6 deletions(-) diff --git a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.i18n.yaml b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.i18n.yaml index 6e8c8c5809..45223d0f08 100644 --- a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.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/process/2026-09-17-persistence-schema-review.md -2026-09-17-persistence-schema-review.md: c4b5f87e0bc37a7f1bba8999317b045f4a367847 -2026-09-17-persistence-schema-review.zh.md: 8707d304eb8e3653a97c73b5876b6977fea3ac83 +2026-09-17-persistence-schema-review.md: 47308a5584b368c99ee849b6d550538ba1856a09 +2026-09-17-persistence-schema-review.zh.md: eaa3c17991b23612753634297993dd3b951259ab diff --git a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.md b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.md index c4b5f87e0b..47308a5584 100644 --- a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.md +++ b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.md @@ -14,6 +14,8 @@ Inventory format 2 records supported compatibility metadata with the affected pr The extractor accepts explicit source bindings only at the core-owned source declaration and requires a literal user or developer role at each use. Producer annotations qualify complete wire-kind groups. The classifier permits a qualified new attribution kind only when both saved schemas carry matching promises. Existing kinds remain subject to structural comparison; removals, retroactive qualification, reserved identities, and unrelated breaking changes retain strict classification. Runtime owners establish and test the promised preservation behavior before activating a binding. +An optional property with no JSON value is normally omitted from the extracted graph. `@persistenceReserved` explicitly retains such a property as optional `never` when a reader forbids its name. A later JSON-valued definition then changes an existing type instead of appearing as an additive optional field. The marker accepts only one argument-free annotation on an optional `never` or `undefined` property and rejects a mapped use that makes the property required or gives it a JSON value. + Current inventories retain authored names and source locations as descriptive metadata. Structural digests identify types and provide catalog anchors. Catalogs group source alternatives by literal `kind` and retain every `form` alternative, required field, index signature, and link to its complete definition. Bounded array labels describe their elements instead of inheriting the first event that reached the array. Historical references retain their frozen rendering. The read-only `persistence-review` command accepts explicit before and after inventories. It matches unchanged schemas by digest and changed alternatives only when literal discriminators identify them uniquely. Ambiguous alternatives remain separate additions and removals. Shared structural evidence lists every affected root, while a separate section preserves the authoritative classifier diagnostics. The [history mechanism](2026-09-11-persistence-type-history.md) continues to own acknowledgement and version requirements. @@ -28,6 +30,8 @@ The read-only `persistence-review` command accepts explicit before and after inv **Replace compatibility checks with a readable diff.** Review explanations group evidence for a human reader. They cannot relax the classifier or replace the in-tree acknowledgement history. +**Retain every optional-never property.** Unmarked optional-never properties already normalize away in recorded schemas. Changing that rule would alter unrelated fingerprints and historical normalization; explicit reservations retain only the reader-owned prohibition. + ## Consequences Declared compatibility promises participate in fingerprints and remain available when comparing historical snapshots. The tooling does not infer or enforce runtime preservation; the owner of an activated binding supplies that behavior. Ordinary comments, labels, and source locations remain outside structural identity. diff --git a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.zh.md b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.zh.md index 8707d304eb..eaa3c17991 100644 --- a/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.zh.md +++ b/.agents/notes/implemented/process/2026-09-17-persistence-schema-review.zh.md @@ -14,6 +14,8 @@ Inventory format 2 在受影响属性上记录受支持的兼容性元数据。 提取器仅接受核心拥有的 source 声明上的显式绑定，并要求每个使用位置具有 user 或 developer 字面量角色。生产者标记对完整 wire-kind 分组赋予资格。仅当双方保存的 schema 携带匹配承诺时，分类器才允许新增符合条件的归属 kind。已有 kind 仍须进行结构比较；删除、追认资格、保留标识和无关破坏性变更继续采用严格分类。运行时所有者在激活绑定前实现并验证承诺的保留行为。 +没有 JSON 值的可选属性通常从提取图中省略。当读取器禁止某个属性名时，`@persistenceReserved` 将该属性显式保留为可选 `never`。后续允许 JSON 值的定义因此属于已有类型变更，而非新增可选字段。标记仅接受可选 `never` 或 `undefined` 属性上的单个无参数注解，并拒绝将属性变为必选或允许 JSON 值的映射类型使用。 + 当前 inventory 将人工声明名称和源码位置保留为描述性元数据。结构摘要标识类型并提供目录锚点。目录按字面量 `kind` 对消息源分支分组，并保留每个 `form` 分支、必需字段、索引签名及指向完整定义的链接。有界数组标签描述元素，不继承最先到达该数组的事件名称。历史参考保留冻结的渲染方式。 只读 `persistence-review` 命令接受显式的变更前后 inventory。它按摘要匹配未变 schema，仅当字面量判别字段唯一标识分支时才匹配变化分支。有歧义的分支保留为独立新增与删除。共享结构证据列出每个受影响根，独立章节保留权威分类器诊断。[历史机制](2026-09-11-persistence-type-history.zh.md)继续负责确认和版本要求。 @@ -28,6 +30,8 @@ Inventory format 2 在受影响属性上记录受支持的兼容性元数据。 **用可读 diff 取代兼容性检查。** 审阅说明为读者归并证据，不能放宽分类器，也不能取代目录内的确认历史。 +**保留每个 optional-never 属性。** 未标记的 optional-never 属性在已记录 schema 中已经被规范化省略。改变该规则会影响无关指纹和历史规范化；显式保留仅记录读取器拥有的禁止规则。 + ## 影响 声明的兼容性承诺参与指纹，并在比较历史快照时保持可用。工具不会推断或强制运行时保留行为；激活绑定的所有者提供该行为。普通注释、标签和源码位置仍处于结构身份之外。 diff --git a/docs/persistence-changes/README.i18n.yaml b/docs/persistence-changes/README.i18n.yaml index 49fba696f5..4439e3a78d 100644 --- a/docs/persistence-changes/README.i18n.yaml +++ b/docs/persistence-changes/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 docs/persistence-changes/README.md -README.md: b4d8fdf4cc264e2d9f63188e461ecaa5599547dc -README.zh.md: 0ab14a9ccb315f57359eaf897d8366d39c96e458 +README.md: abe1c8454b88af243a5a18a46d37744ce91506d2 +README.zh.md: 96b10059a1f2a1dfd35103cf951fe9980ddd8ab0 diff --git a/docs/persistence-changes/README.md b/docs/persistence-changes/README.md index b4d8fdf4cc..abe1c8454b 100644 --- a/docs/persistence-changes/README.md +++ b/docs/persistence-changes/README.md @@ -57,6 +57,8 @@ The extractor accepts an explicit `@persistenceSource` binding for a core-owned A same-version explanation states why old records can omit the addition and why older readers can ignore it without changing replay. For required-to-optional changes, it explains how readers handle an absent value. The checker validates the type classification; reviewers assess the explanation. A version-bump record includes the increasing header version in the same transition and follows the [Session-format procedure](../cookbook/adding-a-session-format-version.md). +When a reader rejects a JSON property by name, declare the property as optional `never` and mark it with argument-free `@persistenceReserved`. The extractor retains the forbidden field, so allowing a JSON value later requires an existing-field type change. Required or JSON-valued properties cannot carry this marker. Unmarked optional `never` and `undefined` properties retain their existing omission behavior. + ## History and limitations diff --git a/docs/persistence-changes/README.zh.md b/docs/persistence-changes/README.zh.md index 0ab14a9ccb..96b10059a1 100644 --- a/docs/persistence-changes/README.zh.md +++ b/docs/persistence-changes/README.zh.md @@ -57,6 +57,8 @@ description: "审阅和维护已记录的会话持久化类型变更、对应 sc 同版本说明须解释旧记录为何可以缺少新增内容，以及旧读取器为何可以忽略它而不改变回放。对于必选改可选的变更，说明须解释读取器如何处理缺失值。检查器验证类型分类；审阅者判断说明是否成立。升版本记录在同一转换中包含递增的头部版本，并遵循[会话格式流程](../cookbook/adding-a-session-format-version.zh.md)。 +当读取器按属性名拒绝 JSON 字段时，将该属性声明为可选 `never`，并添加无参数的 `@persistenceReserved` 标记。提取器保留这一禁止字段，因此后续允许 JSON 值属于已有字段类型变更。必选属性或允许 JSON 值的属性不能携带该标记。未标记的可选 `never` 和 `undefined` 属性保留现有省略行为。 + ## 历史与限制 diff --git a/scripts/persistence-schema.spec.ts b/scripts/persistence-schema.spec.ts index f6517faa3e..0f8a222617 100644 --- a/scripts/persistence-schema.spec.ts +++ b/scripts/persistence-schema.spec.ts @@ -334,6 +334,58 @@ interface SessionEventMap { }) }) +describe('explicitly reserved JSON properties', () => { + function extractedEvent(source: string) { + const model = extractPersistenceSchema(fixture(source)) + const root = model.roots.find(root => root.event === 'test/record') + if (root === undefined) throw new Error('missing reserved-property fixture event') + return root + } + + it('classifies a value added to a reserved field as a changed type', () => { + const before = extractedEvent('export interface Payload { id: string;\n/** @persistenceReserved */\ntool?: never }') + const after = extractedEvent('export interface Payload { id: string; tool?: string }') + expect(classifyPersistenceChange(before, after)).toEqual([expect.objectContaining({ + path: 'event:test/record.data.tool', kind: 'type-changed', requiresVersionBump: true, + })]) + }) + + it.each(['never', 'undefined', 'never | undefined'])('retains a marked optional %s property as never', (value) => { + const root = extractedEvent(`interface Reserved {\n/** @persistenceReserved */\ntool?: ${value}\n}\nexport interface Payload extends Reserved { id: string }`) + const reserved = root.schema.nodes.flatMap(node => node.kind === 'object' + ? node.properties.filter(property => property.name === 'tool').map(property => ({ optional: property.optional, value: root.schema.nodes[property.type] })) + : []) + expect(reserved).toEqual([{ optional: true, value: { kind: 'primitive', type: 'never' } }]) + }) + + it('keeps unmarked optional never and undefined fields erased', () => { + const before = extractedEvent('export interface Payload { id: string; tool?: never; missing?: undefined }') + const after = extractedEvent('export interface Payload { id: string }') + expect(before).toEqual(after) + }) + + it.each([ + 'Required', + '{ [Key in keyof Reserved]?: string }', + ])('rejects a reserved property rewritten by %s', (mapped) => { + expect(() => extractedEvent(`interface Reserved {\n/** @persistenceReserved */\ntool?: never\n}\nexport type Payload = ${mapped} & {id: string}`)) + .toThrow('@persistenceReserved must remain an optional never or undefined property') + }) + + it.each([ + ['required field', 'export interface Payload { id: string;\n/** @persistenceReserved */\ntool: never }'], + ['string field', 'export interface Payload { id: string;\n/** @persistenceReserved */\ntool?: string }'], + ['nullable field', 'export interface Payload { id: string;\n/** @persistenceReserved */\ntool?: null }'], + ['unknown field', 'export interface Payload { id: string;\n/** @persistenceReserved */\ntool?: unknown }'], + ['tag arguments', 'export interface Payload { id: string;\n/** @persistenceReserved value */\ntool?: never }'], + ['duplicate tag', 'export interface Payload { id: string;\n/** @persistenceReserved\n * @persistenceReserved */\ntool?: never }'], + ['non-property tag', '/** @persistenceReserved */\nexport interface Payload { id: string }'], + ['unreachable misuse', 'interface Unused {\n/** @persistenceReserved */\ntool?: string }\nexport interface Payload { id: string }'], + ])('rejects %s', (_name, source) => { + expect(() => extractedEvent(source)).toThrow('@persistenceReserved requires one argument-free marker on an optional never or undefined property') + }) +}) + function sourceFixture(producers: string, role: 'user' | 'developer' = 'user'): string { const root = fixture(`export type Payload = import('../../../llm/llm/src/message.js').${role === 'user' ? 'UserMessage' : 'DeveloperMessage'}`) put(root, 'packages/llm/llm/src/message.ts', ` diff --git a/scripts/persistence-schema.ts b/scripts/persistence-schema.ts index 7293196d7e..ecdf22e1c6 100644 --- a/scripts/persistence-schema.ts +++ b/scripts/persistence-schema.ts @@ -205,6 +205,33 @@ function validateReachableDeclarations( return definitions } +function isNeverOrUndefined(type: ts.Type): boolean { + return (type.isUnion() ? type.types : [type]) + .every(member => (member.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Never)) !== 0) +} + +function reservedPropertyAnnotations(root: string, program: ts.Program): ReadonlySet { + const checker = program.getTypeChecker() + const reserved = new Set() + for (const file of program.getSourceFiles()) { + const path = slash(relative(root, file.fileName)) + if (!path.startsWith('packages/') || path.includes('/node_modules/')) continue + const visit = (node: ts.Node): void => { + const tags = ts.getJSDocTags(node).filter(tag => tag.tagName.text === 'persistenceReserved') + if (tags.length > 0) { + if (tags.length !== 1 || tags[0]?.comment !== undefined || !ts.isPropertySignature(node) + || node.questionToken === undefined || !isNeverOrUndefined(checker.getTypeAtLocation(node))) { + throw new PersistenceSchemaError(`persistence schema: ${path}: @persistenceReserved requires one argument-free marker on an optional never or undefined property`) + } + reserved.add(node) + } + ts.forEachChild(node, visit) + } + visit(file) + } + return reserved +} + class SchemaExtractor { readonly nodes: SchemaNode[] = [] private readonly cache = new Map() @@ -212,6 +239,7 @@ class SchemaExtractor { private readonly checker: ts.TypeChecker private readonly sourcePolicies: ReadonlyMap + private readonly reservedProperties: ReadonlySet constructor( private readonly root: string, @@ -220,6 +248,7 @@ class SchemaExtractor { ) { this.checker = program.getTypeChecker() this.sourcePolicies = sourceCompatibilityAnnotations(root, program) + this.reservedProperties = reservedPropertyAnnotations(root, program) } convert(type: ts.Type, site: ts.Node): number { @@ -297,8 +326,11 @@ class SchemaExtractor { const declaration = property.valueDeclaration ?? property.declarations?.[0] ?? site const propertyType = this.checker.getTypeOfSymbolAtLocation(property, declaration) const optional = (property.flags & ts.SymbolFlags.Optional) !== 0 - if (optional && (propertyType.isUnion() ? propertyType.types : [propertyType]) - .every(member => (member.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Never)) !== 0)) continue + const reserved = property.declarations?.some(declaration => this.reservedProperties.has(declaration)) === true + if (reserved && (!optional || !isNeverOrUndefined(propertyType))) { + this.fail(type, declaration, '@persistenceReserved must remain an optional never or undefined property') + } + if (optional && isNeverOrUndefined(propertyType) && !reserved) continue const child = this.valueType(propertyType, declaration, optional) const policies = this.propertyPolicies(type, property, declaration) if (policies.length > 1) this.fail(type, declaration, 'ambiguous source compatibility binding')