fix(subagent): make the structured re-assert placement-preserving
Re-auditing the review-fix commits surfaced a regression the REPLACE re-assert (825cbab3) introduced: unconditionally rebuilding both arrays as filter(...)+append moved structured_output to the END of the model-visible tool list on every untampered assembly (overriding the registry's toolOrder/lexicographic contract) and moved the instruction section to the absolute array end — renderPrompt reads ARRAY order, so any section above order 190 would render before the trailing instruction, violating the sections-sorted-ascending contract. The presence-check version it replaced touched neither array when the entries were intact. The re-assert keeps its REPLACE content semantics but is now placement-preserving: the tool is replaced IN PLACE (duplicates collapse, append only when stripped); the section is re-inserted at its ascending-order position (the first entry above 190 — exactly where the registry's stable sort put it, so the untampered path reaches the model byte-identical). Pinned by two regression tests that fail against the filter+append form: untampered placement (tool before a lexicographically later tool, instruction before an order-200 section) and tamper recovery (stripped section re-enters its band; an added duplicate collapses to one right-schema entry).
This commit is contained in:
@@ -21,8 +21,11 @@
|
||||
* always carries its capture tool and the trailing instruction section. The
|
||||
* registry already contributes both; this outermost wrapper preserves the
|
||||
* guarantee against a (global) listener that strips or replaces the
|
||||
* assembly. The loop logs the rendered assembly as the request header, so
|
||||
* the demand is reconstructable log state, never a wire-only mutation.
|
||||
* assembly — placement-preserving, so an untampered assembly reaches the
|
||||
* model byte-identical (tools replaced in place, the section re-inserted at
|
||||
* its ascending-order position). The loop logs the rendered assembly as the
|
||||
* request header, so the demand is reconstructable log state, never a
|
||||
* wire-only mutation.
|
||||
* - `agent/turn-continuation` (prepend, scoped): stop the child's turn once
|
||||
* its output is captured — the loop's default "had tool calls ⇒ continue"
|
||||
* would buy a wasted extra model step per structured child.
|
||||
@@ -138,15 +141,39 @@ export function attachStructuredRuntime(childCtx: Context, schema: StructuredOut
|
||||
// REPLACE, not merely ensure-present: a downstream listener may have
|
||||
// mutated or injected a same-named entry with the WRONG schema/text, and
|
||||
// the model-visible demand must be exactly this run's own — the same
|
||||
// schema validateStructuredValue enforces.
|
||||
final.tools = [
|
||||
...final.tools.filter(tool => tool.name !== STRUCTURED_OUTPUT_TOOL),
|
||||
{ ...schemaEntry, parameters: structuredClone(schemaEntry.parameters) },
|
||||
]
|
||||
final.sections = [
|
||||
...final.sections.filter(section => section.name !== `tool:${STRUCTURED_OUTPUT_TOOL}`),
|
||||
{ name: `tool:${STRUCTURED_OUTPUT_TOOL}`, order: 190, text: STRUCTURED_OUTPUT_INSTRUCTION },
|
||||
]
|
||||
// schema validateStructuredValue enforces. Placement-preserving on both
|
||||
// arrays: the untampered path must reach the model byte-identical to the
|
||||
// registry's output (tool order is the `toolOrder`/lexicographic
|
||||
// contract, section order is the ascending contract `renderPrompt`
|
||||
// trusts), so this never reorders what it only re-asserts.
|
||||
const freshTool: ToolSchema = { ...schemaEntry, parameters: structuredClone(schemaEntry.parameters) }
|
||||
// Tools: replace the first same-named entry IN PLACE (its position is the
|
||||
// chain's product; a tool's list position carries no semantic band to
|
||||
// restore), drop any duplicates, append only when stripped entirely.
|
||||
const tools: ToolSchema[] = []
|
||||
let toolReplaced = false
|
||||
for (const tool of final.tools) {
|
||||
if (tool.name !== STRUCTURED_OUTPUT_TOOL) {
|
||||
tools.push(tool)
|
||||
} else if (!toolReplaced) {
|
||||
tools.push(freshTool)
|
||||
toolReplaced = true
|
||||
}
|
||||
}
|
||||
if (!toolReplaced) tools.push(freshTool)
|
||||
final.tools = tools
|
||||
// Sections: remove every same-named entry and re-insert at the
|
||||
// ascending-correct position (the first entry above order 190) — sections
|
||||
// DO carry an order contract, and the renderer reads array order, so a
|
||||
// stripped-or-moved instruction is restored to its band, not appended
|
||||
// after unrelated higher-order sections. On the untampered path this
|
||||
// lands exactly where the registry's stable sort put it (last of the 190
|
||||
// band — the scoped section registers after every load-time 190).
|
||||
const sectionName = `tool:${STRUCTURED_OUTPUT_TOOL}`
|
||||
const sections = final.sections.filter(section => section.name !== sectionName)
|
||||
const insertAt = sections.findIndex(section => section.order > 190)
|
||||
sections.splice(insertAt === -1 ? sections.length : insertAt, 0, { name: sectionName, order: 190, text: STRUCTURED_OUTPUT_INSTRUCTION })
|
||||
final.sections = sections
|
||||
return final
|
||||
}, { prepend: true })
|
||||
|
||||
|
||||
Reference in New Issue
Block a user