From 19a56ec542f30c07f4bb5ec419a91de80c107948 Mon Sep 17 00:00:00 2001 From: Hypatia May Date: Thu, 16 Jul 2026 13:23:20 +0800 Subject: [PATCH] fix(token-meter): reject stale config (round 2) --- docs/cordis-catalog/services.md | 2 +- .../2026-06-18-compaction-capability-seam.md | 2 +- packages/compact/compact-basic/README.md | 2 +- packages/compact/compact-basic/src/config.ts | 23 +++++ .../compact-basic/tests/compact-basic.spec.ts | 2 + .../tests/loader-composition.spec.ts | 91 +++++++++++++------ packages/llm/token-meter/README.md | 2 +- packages/llm/token-meter/src/index.ts | 15 +++ .../llm/token-meter/tests/token-meter.spec.ts | 5 + 9 files changed, 110 insertions(+), 34 deletions(-) diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 5805ae130d..d57f0ed9c5 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -273,7 +273,7 @@ estimateMessage(message: Message): number Types: [Message](../core-data-structures/core.md) -Source: [`packages/llm/token-meter/src/index.ts:92`](../../packages/llm/token-meter/src/index.ts) +Source: [`packages/llm/token-meter/src/index.ts:107`](../../packages/llm/token-meter/src/index.ts) ## `ctx.tools` — `ToolRegistry` diff --git a/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md b/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md index 77f836e8e3..341e5f2866 100644 --- a/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md +++ b/docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md @@ -28,7 +28,7 @@ This is not a coupling smell — it is the contract's domain. The "only cordis" ### Abstract `compactIfNeeded` / `compactRegion`, algorithm in the backend -An earlier draft put the full algorithm (the retention walk, token-summing, text extraction) as concrete methods on the interface. That recouples the contract to one strategy: a backend that wants a different retention policy or event sequence would have to fight inherited concrete code. Making both core methods abstract puts every *how* decision in the backend and keeps the interface a statement of *what*. Token measurement is not a compaction hook at all; the standalone service lets multiple consumers share one model/session replay fold. +An earlier draft put the full algorithm (the retention walk, token-summing, text extraction) as concrete methods on the interface. That recouples the contract to one strategy: a backend that wants a different retention policy or event sequence would have to fight inherited concrete code. Making both core methods abstract puts every *how* decision in the backend and keeps the interface a statement of *what*. Token measurement is not a compaction hook at all; the singleton service lets multiple consumers share one per-session replay fold. `compactIfNeeded(agent, fullSystemPrompt, sessionPrefix, signal)` takes required pressure inputs and cancellation. The session comes from the agent. `compactRegion(session, start, end, agent, signal?)` keeps an optional signal for manual callers and requires `session === agent.session`; implementations reject mismatch before model resolution, lock acquisition, summarization, or log mutation. The pre-step integration resolves a provisional model from the latest logged request header, then `AgentOptions.model`; a model-less router-only first step skips pressure because `agent/request` can route later. The default summarizer resolves its model from explicit config, the latest logged routed model, then agent options. diff --git a/packages/compact/compact-basic/README.md b/packages/compact/compact-basic/README.md index b166636a5f..d537848bd2 100644 --- a/packages/compact/compact-basic/README.md +++ b/packages/compact/compact-basic/README.md @@ -20,7 +20,7 @@ This backend owns the compaction policy: ## Config (`BasicCompactConfig`) -Every setting is optional. The pressure and retention policy applies to the token meter's single context window. +Every setting is optional. The pressure and retention policy applies to the token meter's single context window. Unrecognized top-level keys are rejected. | Key | Required | Meaning | |---|---|---| diff --git a/packages/compact/compact-basic/src/config.ts b/packages/compact/compact-basic/src/config.ts index da2cd7bea7..377bd21523 100644 --- a/packages/compact/compact-basic/src/config.ts +++ b/packages/compact/compact-basic/src/config.ts @@ -14,6 +14,28 @@ const DEFAULT_THRESHOLD_RATIO = 0.8 /** Default verbatim-tail fraction of the token meter's context window. */ const DEFAULT_RETAIN_RATIO = 0.16 +/** Complete public configuration key set. */ +const BASIC_COMPACT_CONFIG_KEYS: ReadonlySet = new Set([ + 'thresholdRatio', + 'retainTokens', + 'summarizationModel', + 'maxTokens', + 'compactionRetries', + 'auto', +]) + +/** Reject stale or misspelled keys before defaults can hide them. */ +function validateConfigKeys(config: BasicCompactConfig): void { + for (const key of Object.keys(config)) { + if (!BASIC_COMPACT_CONFIG_KEYS.has(key)) { + throw new Error( + `BasicCompactConfig: unknown key "${key}" ` + + '(allowed: thresholdRatio, retainTokens, summarizationModel, maxTokens, compactionRetries, auto)', + ) + } + } +} + /** * Resolve defaults and validate the service-wide compaction policy. * @param config - raw compact-basic configuration. @@ -24,6 +46,7 @@ export function resolveConfig( config: BasicCompactConfig = {}, tokenMeter: TokenMeterService, ): ResolvedConfig { + validateConfigKeys(config) const thresholdRatio = config.thresholdRatio ?? DEFAULT_THRESHOLD_RATIO const retainTokens = config.retainTokens ?? Math.floor(tokenMeter.contextWindow * DEFAULT_RETAIN_RATIO) diff --git a/packages/compact/compact-basic/tests/compact-basic.spec.ts b/packages/compact/compact-basic/tests/compact-basic.spec.ts index 86471d2630..ffc60ef024 100644 --- a/packages/compact/compact-basic/tests/compact-basic.spec.ts +++ b/packages/compact/compact-basic/tests/compact-basic.spec.ts @@ -163,6 +163,8 @@ describe('compact configuration and defaults', () => { [{ thresholdRatio: 1.1 }, /number in \(0, 1\]/], [{ retainTokens: -1 }, /non-negative integer/], [{ thresholdRatio: 0.5, retainTokens: 500 }, /less than threshold/], + [{ models: { [MODEL]: { retainTokens: 10 } } }, /BasicCompactConfig: unknown key "models"/], + [{ thresholdRato: 0.5 }, /BasicCompactConfig: unknown key "thresholdRato"/], ] as Array<[unknown, RegExp]> for (const [config, pattern] of bad) { diff --git a/packages/compact/compact-basic/tests/loader-composition.spec.ts b/packages/compact/compact-basic/tests/loader-composition.spec.ts index 26ff37a8e6..b13e8f8b67 100644 --- a/packages/compact/compact-basic/tests/loader-composition.spec.ts +++ b/packages/compact/compact-basic/tests/loader-composition.spec.ts @@ -20,44 +20,75 @@ afterEach(async () => { root = undefined }) +async function loadYaml(lines: readonly string[]): Promise { + root = await mkdtemp(join(tmpdir(), 'dsh-token-meter-loader-')) + const configPath = join(root, 'cordis.yml') + await writeFile(configPath, [...lines, ''].join('\n')) + + context = new Context() + context.baseUrl = pathToFileURL(root).href + '/' + await context.plugin(Loader) + context.loader.builtins.include = Include + const modules = new Map([ + ['@deepseek-ai/dsh-llm', LlmService], + ['@deepseek-ai/dsh-token-meter', TokenMeterService], + ['@deepseek-ai/dsh-compact-basic', BasicCompactService], + ]) + context.loader.internal = { + version: 'v2', + async import(specifier: string) { + if (!modules.has(specifier)) throw new Error(`unexpected Loader import: ${specifier}`) + return modules.get(specifier) + }, + } as unknown as NonNullable + await context.loader.create({ + name: 'cordis:include', + config: { path: pathToFileURL(configPath).href }, + }) + await context.loader.await() + return context +} + describe('real Loader composition', () => { - it('loads the zero-config token-meter then compact-basic YAML pair', async () => { - root = await mkdtemp(join(tmpdir(), 'dsh-token-meter-loader-')) - const configPath = join(root, 'cordis.yml') - await writeFile(configPath, [ + it('loads the flat token-meter and compact-basic YAML shape', async () => { + const loaded = await loadYaml([ "- name: '@deepseek-ai/dsh-llm'", "- name: '@deepseek-ai/dsh-token-meter'", + ' config:', + ' contextWindow: 4096', "- name: '@deepseek-ai/dsh-compact-basic'", - '', - ].join('\n')) - - context = new Context() - context.baseUrl = pathToFileURL(root).href + '/' - await context.plugin(Loader) - context.loader.builtins.include = Include - const modules = new Map([ - ['@deepseek-ai/dsh-llm', LlmService], - ['@deepseek-ai/dsh-token-meter', TokenMeterService], - ['@deepseek-ai/dsh-compact-basic', BasicCompactService], + ' config:', + ' thresholdRatio: 0.5', + ' retainTokens: 512', + ' auto: false', ]) - context.loader.internal = { - version: 'v2', - async import(specifier: string) { - if (!modules.has(specifier)) throw new Error(`unexpected Loader import: ${specifier}`) - return modules.get(specifier) - }, - } as unknown as NonNullable - await context.loader.create({ - name: 'cordis:include', - config: { path: pathToFileURL(configPath).href }, - }) - await context.loader.await() - const unloaded = [...context.loader.entries()] + const unloaded = [...loaded.loader.entries()] .filter(entry => entry.fiber === undefined && !entry.disabled) .map(entry => entry.options.name) expect(unloaded).toEqual([]) - expect(context.tokenMeter.contextWindow).toBe(128_000) - expect(context.get('compact')).toBeInstanceOf(BasicCompactService) + expect(loaded.tokenMeter.contextWindow).toBe(4096) + expect(loaded.get('compact')).toBeInstanceOf(BasicCompactService) + expect((loaded.compact as BasicCompactService).config).toMatchObject({ + thresholdRatio: 0.5, + retainTokens: 512, + auto: false, + }) + }) + + it('rejects stale token-meter config after Schemastery normalization', async () => { + context = new Context() + await expect(context.plugin(TokenMeterService, { + models: { legacy: { contextWindow: 4096 } }, + } as never)).rejects.toThrow(/TokenMeterConfig: unknown key "models"/) + }) + + it('rejects stale compact-basic config after Schemastery normalization', async () => { + context = new Context() + await context.plugin(LlmService) + await context.plugin(TokenMeterService) + await expect(context.plugin(BasicCompactService, { + models: { legacy: { thresholdRatio: 0.5 } }, + } as never)).rejects.toThrow(/BasicCompactConfig: unknown key "models"/) }) }) diff --git a/packages/llm/token-meter/README.md b/packages/llm/token-meter/README.md index 85f14aed35..3beaff1506 100644 --- a/packages/llm/token-meter/README.md +++ b/packages/llm/token-meter/README.md @@ -8,7 +8,7 @@ Replay-aware token measurement through the singleton `ctx.tokenMeter` service. I |---|---:|---| | `contextWindow` | `128000` | Positive integer service-wide context capacity. | -The estimator intentionally uses one fixed heuristic: four characters per token plus structural overhead for roles, blocks, and request-envelope fields. `contextWindow` is the only deployment setting. Direct construction validates it; Loader mounts first apply the package's Schemastery shape validation. +The estimator intentionally uses one fixed heuristic: four characters per token plus structural overhead for roles, blocks, and request-envelope fields. `contextWindow` is the only deployment setting. Direct construction validates it; Loader mounts first apply the package's Schemastery shape validation. Unrecognized top-level keys are rejected. ## Measurement contract diff --git a/packages/llm/token-meter/src/index.ts b/packages/llm/token-meter/src/index.ts index 0b5b3b062d..aa88831033 100644 --- a/packages/llm/token-meter/src/index.ts +++ b/packages/llm/token-meter/src/index.ts @@ -23,6 +23,9 @@ export type * from './types.ts' /** Default service-wide provider context capacity. */ const DEFAULT_CONTEXT_WINDOW = 128_000 +/** Complete public configuration key set. */ +const TOKEN_METER_CONFIG_KEYS: ReadonlySet = new Set(['contextWindow']) + /** Fixed text-density estimate used until exact tokenization is needed. */ const CHARS_PER_TOKEN = 4 @@ -69,8 +72,20 @@ function optionalHeaderEquals( return headerEquals(left, right) } +/** Reject stale or misspelled keys before defaults can hide them. */ +function validateConfigKeys(config: TokenMeterConfig): void { + for (const key of Object.keys(config)) { + if (!TOKEN_METER_CONFIG_KEYS.has(key)) { + throw new Error( + `TokenMeterConfig: unknown key "${key}" (allowed: contextWindow)`, + ) + } + } +} + /** Resolve and validate the one service-wide context capacity. */ function resolveContextWindow(config: TokenMeterConfig): number { + validateConfigKeys(config) const contextWindow = config.contextWindow === undefined ? DEFAULT_CONTEXT_WINDOW : config.contextWindow diff --git a/packages/llm/token-meter/tests/token-meter.spec.ts b/packages/llm/token-meter/tests/token-meter.spec.ts index a91bd392e1..648eef01f5 100644 --- a/packages/llm/token-meter/tests/token-meter.spec.ts +++ b/packages/llm/token-meter/tests/token-meter.spec.ts @@ -81,6 +81,11 @@ describe('TokenMeterService configuration and registration', () => { expect(meter({ contextWindow: 32_000 }).contextWindow).toBe(32_000) }) + it.each(['models', 'contextWidow'])('rejects unknown top-level config key %s', (key) => { + expect(() => meter({ [key]: {} })) + .toThrow(`TokenMeterConfig: unknown key "${key}"`) + }) + it.each([ { contextWindow: 0 }, { contextWindow: -1 },