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.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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 }'),
|
||||
|
||||
@@ -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<string, string>()
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user