From 2728e3bd5b9d83546596d85a225ed9d8635a921b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:21:23 +0800 Subject: [PATCH] Close remaining keyof-join holes the review verification found - The owning top-level interface SessionEventMap must now be the SINGLE EXPORTED declaration in @deepseek-ai/dsh-session: a non-exported local interface (even inside the owning package) and a second exported copy are hard errors, so a same-named helper can no longer be catalogued as the on-disk vocabulary. - Any SessionEventMap declaration carrying an extends clause is a hard error: inherited keys join keyof SessionEventMap but have no catalog row, so heritage is a silent-skip path the gate must reject. Three new spec cases; RFC and module doc updated to match. --- .../2026-07-04-persistence-log-catalog.md | 2 +- .../tests/gen-persistence-catalog.spec.ts | 23 +++++++++++ scripts/gen-persistence-catalog.ts | 38 ++++++++++++++----- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/docs/rfc/implemented/process/2026-07-04-persistence-log-catalog.md b/docs/rfc/implemented/process/2026-07-04-persistence-log-catalog.md index b205538979..b03c79e84a 100644 --- a/docs/rfc/implemented/process/2026-07-04-persistence-log-catalog.md +++ b/docs/rfc/implemented/process/2026-07-04-persistence-log-catalog.md @@ -17,7 +17,7 @@ Specific choices: - **JSDoc completeness, enforced.** Every member must carry description prose — the JSDoc becomes the catalog entry, the same forcing function the cordis catalog applies to bus events. An `@mode` tag on a member is a hard error: dispatch modes belong to cordis bus events, and a log event has none — the tag would misread as "this fires on the bus with mode X". Violations aggregate into one error listing every offender. - **The surface badge is derived, not hand-listed.** `SurfaceEventType` — the subset that produces LLM messages and may carry `surfaceOp` — is parsed from its union declaration in the owning package; a union member naming no declared event is a hard error (a stale union member would otherwise silently badge nothing). Everything else renders **log-only**. - **A dedicated fence.** Payload blocks use a ` ```ts persistence-catalog ` info string that `doc-typecheck` recognizes and skips, excluded from the opt-out ratio — the same treatment as `ts cordis-catalog` (a bare payload fragment is not standalone-compilable). -- **Repo scope.** The catalog enumerates the packages in this repo, matching the siblings' packages-only scope; a downstream plugin can merge further event types, which are outside the catalog by construction. The walk defends its own assumptions: a top-level `interface SessionEventMap` outside `@deepseek-ai/dsh-session` is a hard error (an unrelated same-named local interface cannot be catalogued as the on-disk vocabulary), a member that is not a property signature with an explicit payload type is a hard error (a method-form member would join `keyof SessionEventMap` yet slip past a silent walk), and a duplicate member across declarations is a hard error. +- **Repo scope.** The catalog enumerates the packages in this repo, matching the siblings' packages-only scope; a downstream plugin can merge further event types, which are outside the catalog by construction. The walk defends its own assumptions with hard errors: the owning top-level `interface SessionEventMap` must be the single exported declaration in `@deepseek-ai/dsh-session` (an unrelated, local, or duplicate same-named interface cannot be catalogued as the on-disk vocabulary), no declaration may carry `extends` (inherited keys would join `keyof SessionEventMap` without a catalog row), every member must be a property signature with an explicit payload type (a method-form member would join `keyof` yet slip past a silent walk), and a duplicate member across declarations fails. This supersedes the hand-copies: the session.md `hook/*` table, the compact README's event table, the hook-protocol README's payload bullets, and the session README's name-list now link the catalog instead of restating payloads (the surrounding semantics prose stays where it was). The two stray `@mode emit` tags on the hook-protocol merge members are removed — the new gate rejects them as the category error they were. diff --git a/packages/core/session/tests/gen-persistence-catalog.spec.ts b/packages/core/session/tests/gen-persistence-catalog.spec.ts index b5bec1b92e..bc4ab4fbb1 100644 --- a/packages/core/session/tests/gen-persistence-catalog.spec.ts +++ b/packages/core/session/tests/gen-persistence-catalog.spec.ts @@ -78,6 +78,29 @@ describe('gen-persistence-catalog collectLogEvents', () => { }))).toThrow(/top-level interface SessionEventMap .* is outside @deepseek-ai\/dsh-session \(package @deepseek-ai\/dsh-alien\)/) }) + it('hard-errors on a non-exported top-level interface even in the owning package', () => { + expect(() => collectLogEvents(make({ + 'packages/core/fix/package.json': OWNER_MANIFEST, + 'packages/core/fix/src/helper.ts': + 'interface SessionEventMap {\n /** A local helper, not the vocabulary. */\n \'fix/local\': { turn: number }\n}\nexport const use: SessionEventMap | null = null\n', + }))).toThrow(/is not exported; the owning vocabulary is the single exported declaration/) + }) + + it('hard-errors when the owning interface is exported from two files', () => { + expect(() => collectLogEvents(make({ + 'packages/core/fix/package.json': OWNER_MANIFEST, + 'packages/core/fix/src/a.ts': 'export interface SessionEventMap {\n /** First home. */\n \'fix/a\': { turn: number }\n}\n', + 'packages/core/fix/src/b.ts': 'export interface SessionEventMap {\n /** Second home. */\n \'fix/b\': { turn: number }\n}\n', + }))).toThrow(/is already declared at packages\/core\/fix\/src\/a\.ts:1; the owning vocabulary has exactly one home/) + }) + + it('hard-errors on an extends clause (inherited keys would escape the catalog)', () => { + expect(() => collectLogEvents(make({ + 'packages/group/fix/src/types.ts': + 'interface Extra { \'fix/hidden\': { turn: number } }\ndeclare module \'@deepseek-ai/dsh-session\' {\n interface SessionEventMap extends Extra {\n /** Declared directly. */\n \'fix/direct\': { turn: number }\n }\n}\n', + }))).toThrow(/uses extends; inherited keys would join keyof SessionEventMap without a catalog row/) + }) + it('extracts a member declaration-merged via the session module', () => { const events = collectLogEvents(make({ 'packages/group/fix/src/types.ts': merge(' /** Merged provenance. */\n \'fix/merged\': { id: string }'), diff --git a/scripts/gen-persistence-catalog.ts b/scripts/gen-persistence-catalog.ts index 4c58c558d2..6d1988beb7 100644 --- a/scripts/gen-persistence-catalog.ts +++ b/scripts/gen-persistence-catalog.ts @@ -26,9 +26,10 @@ * error — dispatch modes belong to cordis bus events, and a log event has none * (see docs/rfc/implemented/process/2026-07-04-persistence-log-catalog.md). * Structural holes are hard errors for the same reason: a member that is not a - * property signature with an explicit payload type, a top-level - * `interface SessionEventMap` outside the owning package, and a duplicate - * declaration of one event would each let something join (or impersonate) + * property signature with an explicit payload type, an `extends` clause on a + * declaration, a top-level `interface SessionEventMap` that is not the single + * exported declaration in the owning package, and a duplicate declaration of + * one event would each let something join (or impersonate) * `keyof SessionEventMap` without a truthful catalog row. Violations aggregate * into ONE error listing every offender. * @@ -245,29 +246,48 @@ function packageNameFor(rel: string, scanRoot: string): string | null { * on any completeness violation: a member without description prose, an * `@mode` tag (a category error — log events have no dispatch mode), a member * that is not a property signature with an explicit payload type, a - * non-literal member name, a top-level declaration outside the owning package, - * or the same event declared twice. + * non-literal member name, an `extends` clause (inherited keys would join + * `keyof SessionEventMap` without a catalog row), a top-level declaration that + * is not the single exported one in the owning package, or the same event + * declared twice. * `scanRoot` defaults to the repo root; tests pass a fixture dir. */ export function collectLogEvents(scanRoot: string = root): LogEventEntry[] { const entries: LogEventEntry[] = [] const violations: string[] = [] const seen = new Map() + let owningDecl: string | null = null for (const rel of globSync('packages/*/*/src/**/*.ts', { cwd: scanRoot }).sort()) { const abs = resolve(scanRoot, rel) const text = readFileSync(abs, 'utf8') if (!text.includes('SessionEventMap')) continue const sf = ts.createSourceFile(abs, text, ts.ScriptTarget.Latest, true) for (const { decl, topLevel } of sessionEventMapDecls(sf)) { + const declSrc = pointer(rel, sf, decl) if (topLevel) { - // The top-level form is the OWNING vocabulary; anywhere else, a - // same-named local interface is a different type entirely and must not - // be catalogued as on-disk events. + // The top-level form is the OWNING vocabulary, and it has exactly one + // home: the single EXPORTED declaration in the owning package. A + // same-named interface anywhere else — another package, a non-exported + // local, a second exported copy — is a different type that must not be + // catalogued as on-disk events. const pkg = packageNameFor(rel, scanRoot) if (pkg !== SESSION_MODULE) { - violations.push(`top-level interface SessionEventMap (${pointer(rel, sf, decl)}) is outside ${SESSION_MODULE} (package ${pkg ?? 'unknown'}). Rename the interface, or contribute events via declare module '${SESSION_MODULE}'.`) + violations.push(`top-level interface SessionEventMap (${declSrc}) is outside ${SESSION_MODULE} (package ${pkg ?? 'unknown'}). Rename the interface, or contribute events via declare module '${SESSION_MODULE}'.`) continue } + const exported = decl.modifiers?.some(m => m.kind === ts.SyntaxKind.ExportKeyword) ?? false + if (!exported) { + violations.push(`top-level interface SessionEventMap (${declSrc}) is not exported; the owning vocabulary is the single exported declaration — rename a local helper interface.`) + continue + } + if (owningDecl) { + violations.push(`top-level interface SessionEventMap (${declSrc}) is already declared at ${owningDecl}; the owning vocabulary has exactly one home.`) + continue + } + owningDecl = declSrc + } + if (decl.heritageClauses?.length) { + violations.push(`SessionEventMap declaration (${declSrc}) uses extends; inherited keys would join keyof SessionEventMap without a catalog row — declare event members directly.`) } for (const member of decl.members) { const src = pointer(rel, sf, member)