fix review findings: own-property and plain-JSON discipline in the schema subset

Three Codex findings on json-schema.ts, one discipline:

- required-declared and every value check now use Object.hasOwn — 'in' let
  inherited names (toString) satisfy required, dodge additionalProperties:
  false, and validate a declared property against the value's prototype
  member instead of a carried one
- isObjectLike now means PLAIN JSON object (proto chain of at most one link,
  realm-agnostic): a Date annotation or a Map-as-properties no longer passes
  structurally and serializes lossily — they fail loud as subset violations
- startInProcessRun asserts BEFORE the defensive structuredClone, so a
  hostile schema fails as OutputSchemaError, never a raw DataCloneError

Also the type-equiv catalog gap: tools.md gains the structured-output subset
vocabulary (4 blocks) with matching manifest entries. The driver index also
drops the runtime internals from its public re-export (runs acquire it
internally; no external consumer remains — see the following commit).
This commit is contained in:
Tianyi Cui
2026-07-07 21:06:14 +08:00
parent 8c8189844f
commit d1b52a063b
5 changed files with 495 additions and 95 deletions

View File

@@ -91,9 +91,20 @@ const ANNOTATION_KEYWORDS = new Set(['description', 'title', 'default', 'example
const SCHEMA_TYPES: readonly StructuredSchemaType[] = ['object', 'array', 'string', 'number', 'integer', 'boolean', 'null']
/** Whether a value is a non-null, non-array object (structural, realm-agnostic). */
/**
* Whether a value is a PLAIN JSON object — non-null, non-array, and with a
* prototype chain of at most one link (`null`-proto, or any realm's
* `Object.prototype`, whose own prototype is `null`). Realm-agnostic on
* purpose: a schema materialized in another realm carries THAT realm's
* `Object.prototype`, which an identity check would wrongly reject. Exotic
* hosts (`Date`, `Map`, class instances) have longer chains and are rejected —
* they would serialize lossily (`Date` → string, `Map` → `{}`) instead of
* failing loud.
*/
function isObjectLike(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value)
if (typeof value !== 'object' || value === null || Array.isArray(value)) return false
const proto: unknown = Object.getPrototypeOf(value)
return proto === null || Object.getPrototypeOf(proto) === null
}
/** Whether a value is a supported scalar (`enum`/`const` member): string, finite number, boolean, or null. */
@@ -116,6 +127,9 @@ function isJsonData(value: unknown, seen: Set<object>): boolean {
seen.add(value)
try {
if (Array.isArray(value)) return value.every(entry => isJsonData(entry, seen))
// A non-plain object (Date, Map, class instance) is NOT JSON data even when
// it has no enumerable values — it would serialize lossily, not loudly.
if (!isObjectLike(value)) return false
return Object.values(value).every(entry => isJsonData(entry, seen))
} finally {
seen.delete(value)
@@ -194,8 +208,11 @@ function checkSchemaNode(node: unknown, path: string, violations: string[], seen
violations.push(`${path}.required must be an array of strings`)
} else {
const declared = isObjectLike(properties) ? properties : {}
for (const key of required) {
if (!(key in declared)) violations.push(`${path}.required names "${key}" which is not in properties`)
// The guard above proved every entry is a string.
for (const key of required as string[]) {
// Own-property check: `in` would let inherited names (`toString`)
// satisfy the declared-in-properties contract via the prototype.
if (!Object.hasOwn(declared, key)) violations.push(`${path}.required names "${key}" which is not in properties`)
}
}
}
@@ -256,16 +273,20 @@ function checkValue(node: StructuredSchemaNode, value: unknown, path: string): s
if (!isObjectLike(value)) return [`"${path}" must be an object`]
const violations: string[] = []
const properties = node.properties ?? {}
// Own-property discipline throughout: JSON carries own enumerable
// properties only, so an inherited `toString` must not satisfy
// `required`, dodge `additionalProperties: false`, or be validated as if
// the value carried it.
for (const key of node.required ?? []) {
if (value[key] === undefined) violations.push(`missing required property "${path}.${key}"`)
if (!Object.hasOwn(value, key) || value[key] === undefined) violations.push(`missing required property "${path}.${key}"`)
}
for (const [key, child] of Object.entries(properties)) {
if (value[key] === undefined) continue
if (!Object.hasOwn(value, key) || value[key] === undefined) continue
violations.push(...checkValue(child, value[key], `${path}.${key}`))
}
if (node.additionalProperties === false) {
for (const key of Object.keys(value)) {
if (!(key in properties)) violations.push(`"${path}.${key}" is not a declared property (additionalProperties: false)`)
if (!Object.hasOwn(properties, key)) violations.push(`"${path}.${key}" is not a declared property (additionalProperties: false)`)
}
}
return violations

View File

@@ -154,6 +154,30 @@ describe('assertSupportedOutputSchema', () => {
const leaf = { type: 'string' }
asserted({ type: 'object', properties: { a: leaf, b: leaf } })
})
it('required cannot be satisfied by INHERITED names — `toString` is not a declared property', () => {
// `'toString' in {}` is true via Object.prototype; the declared-property
// contract must be an own-property check.
expect(violationsOf({ type: 'object', properties: {}, required: ['toString'] }))
.toEqual(['schema.required names "toString" which is not in properties'])
})
it('rejects exotic host objects where the subset expects plain JSON structure', () => {
// A Map as `properties` has no own enumerable entries: structurally it
// would read as "no properties" and serialize to {} — lossy, not loud.
expect(violationsOf({ type: 'object', properties: new Map() }))
.toEqual(['schema.properties must be an object of schemas'])
// A Date node is not a schema object even though Object.values(date) is [].
expect(violationsOf({ type: 'object', properties: { at: new Date(0) } }))
.toEqual(['schema.properties.at must be a schema object'])
})
it('rejects exotic annotation payloads that would serialize lossily', () => {
expect(violationsOf({ type: 'object', default: new Date(0) }))
.toEqual(['schema.default annotation must be JSON data'])
expect(violationsOf({ type: 'object', examples: [new Map()] }))
.toEqual(['schema.examples annotation must be JSON data'])
})
})
describe('validateStructuredValue', () => {
@@ -223,6 +247,32 @@ describe('validateStructuredValue', () => {
expect(validateStructuredValue(schema, { file: undefined })).toEqual(['missing required property "value.file"'])
})
it('inherited properties satisfy nothing: required, additionalProperties, and recursion are own-property only', () => {
// required: ['toString'] must NOT be satisfied by Object.prototype.toString.
expect(validateStructuredValue(
asserted({ type: 'object', properties: { toString: { type: 'string' } }, required: ['toString'] }),
{},
)).toEqual(['missing required property "value.toString"'])
// additionalProperties: false must flag an OWN `toString` key even though
// `'toString' in properties` is true via the prototype.
expect(validateStructuredValue(
asserted({ type: 'object', additionalProperties: false }),
{ toString: 1 },
)).toEqual(['"value.toString" is not a declared property (additionalProperties: false)'])
// A declared property the value does NOT carry must not be validated
// against the value's INHERITED member (constructor is a function on
// every plain object's prototype, not a carried property).
expect(validateStructuredValue(
asserted({ type: 'object', properties: { constructor: { type: 'string' } } }),
{},
)).toEqual([])
})
it('a non-plain object value is not an object in the JSON sense', () => {
expect(validateStructuredValue(asserted({ type: 'object' }), new Date(0)))
.toEqual(['"value" must be an object'])
})
it('collects multiple violations across branches in one pass', () => {
expect(validateStructuredValue(schema, { line: 'x', severity: 'mid' })).toEqual([
'missing required property "value.file"',

View File

@@ -25,11 +25,12 @@ import {
type StructuredAcquisition,
} from './structured.ts'
// The runtime itself (acquire/attach/release) is package-internal: runs
// acquire it inside startInProcessRun, and no other package drives it. Only
// the model-facing vocabulary is public.
export {
acquireStructuredRuntime,
STRUCTURED_OUTPUT_TOOL,
STRUCTURED_OUTPUT_INSTRUCTION,
type StructuredAcquisition,
} from './structured.ts'
declare module '@deepseek-ai/dsh-agent' {
@@ -110,15 +111,18 @@ export function startInProcessRun(
if (request.maxDepth !== undefined && childDepth > request.maxDepth) {
throw new SubagentDepthError(childDepth, request.maxDepth)
}
// Snapshot, then assert, the schema subset BEFORE any child exists (the
// Assert, then snapshot, the schema subset BEFORE any child exists (the
// service has already capability-gated; this rejects a schema outside the
// enforced subset loud). The snapshot is load-bearing: the caller keeps its
// reference, so validating and attaching the ORIGINAL would let a
// post-start() mutation drift the enforced schema away from the asserted
// one — the clone pins assertion, the model-visible parameters, and
// validateStructuredValue to the same isolation-immutable value.
// enforced subset loud). Assertion comes FIRST so a hostile value fails as
// OutputSchemaError, never as structuredClone's raw DataCloneError — the
// asserted subset is plain JSON data, which always clones. The snapshot is
// load-bearing: the caller keeps its reference, so attaching the ORIGINAL
// would let a post-start() mutation drift the enforced schema away from the
// asserted one — the clone (taken synchronously with the assertion, no
// interleaving possible) pins assertion, the model-visible parameters, and
// validateStructuredValue to one isolation-immutable value.
if (request.outputSchema !== undefined) assertSupportedOutputSchema(request.outputSchema)
const schema = request.outputSchema === undefined ? undefined : structuredClone(request.outputSchema)
if (schema !== undefined) assertSupportedOutputSchema(schema)
const childId = AgentId(randomUUID())
// The child's OWN events begin after the seed (fork seeds the parent's