fix(web): address review findings on producer-declared context forms
- `catalogHistory` validated its durable read. `agent.session.events` is a JSONL/SQLite seed on resume or fork, and seed validation guarantees only a source object with a non-empty `kind`; a `skill-catalog` record with missing or wrongly shaped `entries` threw inside the step listener, failing every later turn of that session. It is now skipped as an unrecognizable record, the posture the replaced content digest had, with a regression test over six malformed shapes. - The headless keyless smoke still filtered catalogs by the old plugin source, so the `built-bin-smoke` gate would not have found the catalog message. - Entries record the published description unescaped. The pseudo-XML escaping belongs to the `<available_skills>` frame and is applied when rendering it, so a description containing `<` no longer reaches the card as `<`. `escapeText` is injective, so republish semantics and the model-facing text are unchanged. - Adjacent text blocks join with no separator, matching how provider adapters flatten them; the body no longer shows a line break the model never saw. - Provenance fields are bounded like the text: an unknown producer may record an arbitrarily large value. - Both readers are all-or-nothing, and the row's form marker reports what rendered rather than what was declared, so a partly unreadable record cannot present a confident but incomplete account. - The catalog body consumes `update` as a replacement notice; the digest canonicalizes per entry as JSON, since every separator character is itself legal in a description. - `core.md` documents the form axis with a `ContextForm` type-equiv block, and both projections assert the wiring they duplicate.
This commit is contained in:
@@ -284,9 +284,14 @@ function renderCatalogUpdate(entries: SkillCatalogSource['entries']): UserMessag
|
||||
})
|
||||
}
|
||||
|
||||
/** Model-facing catalog lines, projected from the same entries the source records. */
|
||||
/**
|
||||
* Model-facing catalog lines, projected from the same entries the source records.
|
||||
* The pseudo-XML escaping belongs to this frame, not to the published fact, so it
|
||||
* is applied here and never stored. Names are `isSkillName`-validated and carry
|
||||
* no escapable character.
|
||||
*/
|
||||
function renderCatalogEntries(entries: SkillCatalogSource['entries']): string[] {
|
||||
return entries.map(entry => `- \`${entry.name}\`: ${entry.description}`)
|
||||
return entries.map(entry => `- \`${entry.name}\`: ${escapeText(entry.description)}`)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -295,12 +300,38 @@ function renderCatalogEntries(entries: SkillCatalogSource['entries']): string[]
|
||||
* written for the model and must not decide whether a republish is needed.
|
||||
*/
|
||||
function digestCatalogEntries(entries: SkillCatalogSource['entries']): string {
|
||||
const canonical = entries.map(entry => `${entry.name}\u0000${entry.description}`).join('\n')
|
||||
// JSON per entry rather than a separator character: every separator is itself
|
||||
// a legal description character, so only quoting makes the boundary exact.
|
||||
const canonical = entries.map(entry => JSON.stringify([entry.name, entry.description])).join('\n')
|
||||
return createHash('sha256')
|
||||
.update(canonical)
|
||||
.digest('hex')
|
||||
}
|
||||
|
||||
/**
|
||||
* Entries of one durable catalog message, or undefined when the record is not a
|
||||
* usable catalog.
|
||||
*
|
||||
* `agent.session.events` may be a resumed, forked, or externally written seed,
|
||||
* and seed validation only guarantees a source object with a non-empty `kind`;
|
||||
* no per-kind field is checked there. An unreadable record is therefore treated
|
||||
* as "not this plugin's catalog" — the posture the replaced content digest had —
|
||||
* rather than throwing inside the step listener, which would fail every
|
||||
* subsequent turn of that session.
|
||||
*/
|
||||
function readCatalogEntries(source: unknown): SkillCatalogSource['entries'] | undefined {
|
||||
const entries = (source as { entries?: unknown }).entries
|
||||
if (!Array.isArray(entries)) return undefined
|
||||
const readable: { name: string; description: string }[] = []
|
||||
for (const entry of entries as readonly unknown[]) {
|
||||
if (typeof entry !== 'object' || entry === null) return undefined
|
||||
const { name, description } = entry as { name?: unknown; description?: unknown }
|
||||
if (typeof name !== 'string' || name === '' || typeof description !== 'string') return undefined
|
||||
readable.push({ name, description })
|
||||
}
|
||||
return readable
|
||||
}
|
||||
|
||||
function catalogHistory(agent: Agent): { visibleDigest?: string; published: boolean } {
|
||||
const visible = new Set(agent.session.surface.nodes)
|
||||
const events = agent.session.events
|
||||
@@ -310,19 +341,19 @@ function catalogHistory(agent: Agent): { visibleDigest?: string; published: bool
|
||||
// oxlint-disable-next-line typescript/no-non-null-assertion
|
||||
const event = events[index]!
|
||||
if (event.type !== 'user/message' || event.data.source.kind !== 'skill-catalog') continue
|
||||
const digest = digestCatalogEntries(event.data.source.entries)
|
||||
const entries = readCatalogEntries(event.data.source)
|
||||
if (entries === undefined) continue
|
||||
const digest = digestCatalogEntries(entries)
|
||||
published = true
|
||||
if (visible.has(event.seq)) return { visibleDigest: digest, published }
|
||||
}
|
||||
return { published }
|
||||
}
|
||||
|
||||
/** Normalized, length-bounded description exactly as the catalog publishes it (unescaped). */
|
||||
function catalogDescription(value: string, maxLength: number): string {
|
||||
const normalized = value.replaceAll(/\s+/g, ' ').trim()
|
||||
const truncated = normalized.length <= maxLength
|
||||
? normalized
|
||||
: `${normalized.slice(0, maxLength - 3)}...`
|
||||
return escapeText(truncated)
|
||||
return normalized.length <= maxLength ? normalized : `${normalized.slice(0, maxLength - 3)}...`
|
||||
}
|
||||
|
||||
function assertPositiveInteger(name: string, value: number, minimum = 1): void {
|
||||
|
||||
@@ -97,6 +97,14 @@ function catalogMessages(session: Session): Extract<SessionEvent, { type: 'user/
|
||||
&& event.data.source.kind === 'skill-catalog')
|
||||
}
|
||||
|
||||
function readableCatalog(event: Extract<SessionEvent, { type: 'user/message' }>): boolean {
|
||||
const entries = (event.data.source as { entries?: unknown }).entries
|
||||
return Array.isArray(entries)
|
||||
&& entries.every(entry => typeof entry === 'object' && entry !== null
|
||||
&& typeof (entry as { name?: unknown }).name === 'string'
|
||||
&& typeof (entry as { description?: unknown }).description === 'string')
|
||||
}
|
||||
|
||||
function catalogContent(entries: string[]): Message['content'] {
|
||||
return [{
|
||||
type: 'text',
|
||||
@@ -218,7 +226,7 @@ describe('dsh-tool-skill', () => {
|
||||
kind: 'skill-catalog',
|
||||
form: 'catalog',
|
||||
entries: [
|
||||
{ name: 'a-skill', description: 'Use {{placeholder}} <safely> & carefully.' },
|
||||
{ name: 'a-skill', description: 'Use {{placeholder}} <safely> & carefully.' },
|
||||
{ name: 'model-only-skill', description: 'Model-only skill.' },
|
||||
{ name: 'z-skill', description: 'Long description Long description Long descript...' },
|
||||
],
|
||||
@@ -411,6 +419,48 @@ describe('dsh-tool-skill', () => {
|
||||
expect(catalogMessages(session)).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('treats a malformed durable catalog as unrecognizable instead of failing the step', async () => {
|
||||
// Seeds reach `agent.session.events` from JSONL/SQLite on resume or fork,
|
||||
// and seed validation only guarantees a source object with a non-empty
|
||||
// `kind`. A catalog whose entries are missing or wrongly shaped must be
|
||||
// skipped like any foreign record; throwing here would fail every later
|
||||
// step of that session at the latest possible point.
|
||||
const home = await tempDir('tool-catalog-malformed')
|
||||
const ctx = await setup(home)
|
||||
ctx.skills.register({
|
||||
name: 'live-skill',
|
||||
description: 'Live skill',
|
||||
source: 'runtime',
|
||||
content: 'Live body.',
|
||||
})
|
||||
const session = Session.create(SessionId('catalog-malformed'))
|
||||
const agent = sessionAgent(session)
|
||||
openMessageTurn(session)
|
||||
for (const source of [
|
||||
{ kind: 'skill-catalog', form: 'catalog' },
|
||||
{ kind: 'skill-catalog', form: 'catalog', entries: null },
|
||||
{ kind: 'skill-catalog', form: 'catalog', entries: 'not-an-array' },
|
||||
{ kind: 'skill-catalog', form: 'catalog', entries: [null] },
|
||||
{ kind: 'skill-catalog', form: 'catalog', entries: [{ name: 'x' }] },
|
||||
{ kind: 'skill-catalog', form: 'catalog', entries: [{ description: 'no name' }] },
|
||||
]) {
|
||||
session.append('user/message', createUserMessage({
|
||||
content: [{ type: 'text', text: 'unreadable catalog' }],
|
||||
source: source as never,
|
||||
}), { surfaceOp: 'append' })
|
||||
}
|
||||
|
||||
await expect(fireStep(ctx, agent, 1, 1)).resolves.toBeUndefined()
|
||||
|
||||
// None of the six counted as published, so the live catalog lands as a
|
||||
// first publication rather than a replacement.
|
||||
const published = catalogMessages(session).filter(event => readableCatalog(event))
|
||||
expect(published).toHaveLength(1)
|
||||
expect(published[0]?.data.source).toMatchObject({ kind: 'skill-catalog', form: 'catalog' })
|
||||
expect(published[0]?.data.source).not.toHaveProperty('update')
|
||||
expect(JSON.stringify(published[0]?.data.content)).toContain('live-skill')
|
||||
})
|
||||
|
||||
it('re-establishes the current catalog after compaction hides its durable message', async () => {
|
||||
const home = await tempDir('tool-catalog-compaction')
|
||||
const ctx = await setup(home)
|
||||
|
||||
Reference in New Issue
Block a user