From 995d75387561678c9cb64032ba17efb9f6edb498 Mon Sep 17 00:00:00 2001 From: Nicholas023 <498594526@qq.com> Date: Sat, 14 Nov 2026 05:19:45 +0800 Subject: [PATCH] fix(ptc): teach only the tool surface a scope publishes A scope without the loader was still taught to call `tools.ops_load_sdk`, so every such session spent a round trip on a call that cannot succeed and failed with "is not a function". The guidance and the `bypass` notice now render from the surface the scope actually publishes: - a loader-less scope gets a direct-call step in place of the load step, its table row, and any fenced example that teaches loading, and says so when nothing is published at all; - the file-work line names only the file APIs that are loadable or resident, never the shell it just accused, and a read-only policy drops the mutating APIs from that list; - the surface is resolved per agent at delivery time, so a narrow pool or a read-only session is never told to load what it cannot reach. The loader-present path is byte-identical for a full pool. Two limits are documented in the reference: the shipped lesson itself is substituted only when the loader is absent, and the lists follow the configured pool and the read-only policy without the per-agent ownership narrowing. Closes #922 --- docs/CHANGELOG.md | 2 + docs/configuration-reference.md | 12 +- ...15\347\275\256\345\217\202\350\200\203.md" | 8 +- src/ptc/batching-guidance.ts | 274 +++++++++++++++++- src/publication/ptc.ts | 26 +- test/batching-guidance.test.mjs | 98 ++++++- test/batching-trigger.test.mjs | 95 +++++- test/resident-switch.test.mjs | 38 ++- 8 files changed, 524 insertions(+), 29 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9b03729e..28ca1c33 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,8 @@ this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +- `batchingGuidance` no longer teaches `ops_load_sdk` in a scope that does not publish it. The guidance text and the `bypass` notice follow the published surface: with `dynamicTools: false` the load step and its table row are replaced by a direct-call section listing the published APIs, and a scope that publishes nothing says so. Every line of the notice names only what the scope can reach: a pool that cannot load `ops_edit_file` is no longer told to load it, and a resident name that is not a file API (such as the shell) is never offered as the way to do file work. A read-only session drops the mutating APIs from that list too. Previously every loader-less session spent a round trip calling `tools.ops_load_sdk` and failed with "is not a function". +- `batchingGuidance` 不再在不发布加载器的作用域里教模型调用 `ops_load_sdk`：引导文本与 `bypass` 提醒改为跟随实际发布面，`dynamicTools: false` 时"先加载再调用"一步及其表格行被替换为列出现有 API 的直调一节，一个都不发布时明确说明。提醒里只点名该作用域真正可达的 API：可加载池里没有的（如 `ops_edit_file`）不再被告知去加载，常驻集合里的非文件 API（如 shell）也不会被当作做文件工作的手段；只读策略下还会去掉会改文件的 API（教学文本本身仍只在无加载器时替换，窄池不会改写随包的轨迹示例）。此前每个无加载器的会话都会白花一次往返并报 "is not a function"。 - Native tool construction and the publication refusal queue no longer attach listeners or lifetime effects to the composition root. The evidence observer and result admission subscribe on the caller's context with global dispatch, and the refusal queue falls back to the caller lifetime when the root refuses an effect. dsh-tui 0.20 refuses `root.events.on` and `root.effect` from a plugin activation, and the native-tools row is one, so every `dsh-tui --resume` aborted with "cannot resume session" before the agent mounted. - 原生工具构造与发布拒绝队列不再向组合根挂监听或生命周期 effect：证据观察者与 result admission 改为在调用方上下文订阅并声明 global 分发，拒绝队列在 root 拒绝 effect 时回落到调用方生命周期。dsh-tui 0.20 会拒绝插件激活期内的 `root.events.on` 与 `root.effect`，而原生工具行正是插件激活，导致每次 `dsh-tui --resume` 都在 agent 挂载前报 "cannot resume session"。 - The Bash tool description now states the parse contract of `stdout`/`stderr` where the model reads it: `text` may be a partial fragment, `truncated` marks one that must not be parsed as complete structured output, and `spillPath` is optional and not a completeness guarantee. The typed output schema is unchanged: repeating the contract there would cost every PTC request without saying anything the tool text does not. Command semantics, exit codes, result values, limits, spill files, permissions and rendering are unchanged. diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 97fd05ae..350374c1 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -92,7 +92,8 @@ external English Markdown file: - **Once at session start**: one plugin-context message joins the session's first accepted non-empty user-message batch (step-tail, source `dsh-ops:step-tail` / `batching-guidance`). It enters logged history, so the model's first request already sees the whole procedure: the first-line `// plan:` comment, the - dependency split, the load-before-call step and four copyable trajectories. + dependency split, the load-before-call step (only while dynamic loading publishes the loader) and four + copyable trajectories. - **On a thin program**: when a **root** `run_code` program breaks one of three latched categories, a short notice is appended to that call's own result as `additionalContexts`. The tool result keeps its value, its content and its existing contexts; the program is never blocked or rewritten. @@ -109,6 +110,15 @@ external English Markdown file: - Everything is plugin-context history and the tool schema is untouched, so the system prompt and its cache survive. The lesson counts as spent only once it appears as a durable `user/message`; a batch withdrawn before admission (cancellation, rejection) leaves the next accepted batch free to teach it again. +- The text follows the published surface. While dynamic loading publishes `ops_load_sdk`, the lesson ships + byte for byte, load step included. With `dynamicTools: false` the scope publishes no loader, so that step and + its table row are replaced by a direct-call section that lists the APIs this scope actually publishes; a + scope that publishes nothing says so and points at the official tools. The program-shape notices carry the + same surface, so `bypass` names neither a loader nor an API the scope cannot reach: its file-work line lists + only the file APIs that are loadable (or resident) here, and a shell tool that bypassed them is never offered + as the replacement. Two limits are deliberate: the shipped lesson itself is substituted only when the loader is + absent (a narrow pool narrows the notices, not the trajectories), and the lists follow the configured pool and the + read-only policy without the per-agent ownership narrowing. `batchingTelemetry` (same row, YAML-only, default `false`) emits one structured log line per delivered notice, for example `dsh-ops batching: turn=3 category=plan plan=1 bypass=0 shape=2 total=3`. It is diff --git "a/docs/\351\205\215\347\275\256\345\217\202\350\200\203.md" "b/docs/\351\205\215\347\275\256\345\217\202\350\200\203.md" index aaf4561e..fcd96172 100644 --- "a/docs/\351\205\215\347\275\256\345\217\202\350\200\203.md" +++ "b/docs/\351\205\215\347\275\256\345\217\202\350\200\203.md" @@ -65,7 +65,7 @@ OPS 沿用单行插件页：bundle 只插入一个 `dsh-ops` 行。内置预设 - **会话首轮一次**：在会话第一个被接受的非空用户消息批次里追加一条插件上下文（step-tail，来源 `dsh-ops:step-tail` / `batching-guidance`），它进入会话历史，模型第一条请求即可看到完整范式：首行 - `// plan:` 指令、按依赖拆分、先加载再调用，以及四条可照抄的轨迹。 + `// plan:` 指令、按依赖拆分、先加载再调用（仅动态加载发布加载器时），以及四条可照抄的轨迹。 - **薄程序触发**：**根** `run_code` 程序命中三类锁中的一类时，在该次调用的结果上以 `additionalContexts` 追加一条短提醒。工具结果的值、正文与既有上下文都不变，程序既不阻断也不改写。 - 三类分别是 `bypass`（用 shell 或 `node:fs` 做文件工作）、`plan`（第一物理行没有 `// plan:`）与 `shape` @@ -77,6 +77,12 @@ OPS 沿用单行插件页：bundle 只插入一个 `dsh-ops` 行。内置预设 提醒文案会说明这一点，不宣称实测的 dispatch 数；被取消或从未派发的调用不参与分析。 - 全部都是插件上下文历史，tool schema 不变，系统提示词与其缓存因此不受影响。教学只有在真正出现持久 `user/message` 时才算兑现；因取消或拒绝而撤回的批次不会消耗它，下一个被接受的批次仍会教学。 +- 文案跟随实际发布面。动态加载发布 `ops_load_sdk` 时，教学文本逐字节照发，含"先加载再调用"这一步；`dynamicTools: + false` 时该作用域没有加载器，这一步与其表格行会被替换成"直接调用"一节，并列出本作用域实际发布的 API；一个 + OPS API 都不发布时则明确说明并指向官方工具。程序形状提醒同样带这一信息，`bypass` 既不指向不可达的加载器， + 也不点名本作用域拿不到的 API：文件工作那句只列可加载（或常驻）中真实存在的文件类 API，shell 这类绕过的工具 + 不会被当作替代手段。两处是刻意取舍：随包教学文本本身只在无加载器时替换（窄池只收窄提醒，不改写轨迹 + 示例）；列表来自配置池与只读策略，不含按 agent 的归属收窄。 `batchingTelemetry`（同一行、YAML-only、默认 `false`）每投递一条提醒打印一行结构化日志，例如 `dsh-ops batching: turn=3 category=plan plan=1 bypass=0 shape=2 total=3`。它只做观测：不产生上下文、 diff --git a/src/ptc/batching-guidance.ts b/src/ptc/batching-guidance.ts index ed237e6d..e1b6b877 100644 --- a/src/ptc/batching-guidance.ts +++ b/src/ptc/batching-guidance.ts @@ -15,7 +15,7 @@ import { literalShellFileWork } from './shell-file-work.js' /** Identity of the guidance in the host message source. */ export const BATCHING_GUIDANCE_ID = 'batching-guidance' /** Revision of the guidance text; bump it when the wording or the file format changes. */ -export const BATCHING_GUIDANCE_REVISION = '4' +export const BATCHING_GUIDANCE_REVISION = '5' /** The shipped guidance file, resolved from this module's own location (src or lib). */ const DEFAULT_GUIDANCE_FILE = new URL('../../prompts/batching-guidance.md', import.meta.url) @@ -23,6 +23,8 @@ const DEFAULT_GUIDANCE_FILE = new URL('../../prompts/batching-guidance.md', impo /** Loaded text by absolute location; a failed load caches '' instead of retrying. */ const loaded = new Map() let warned = false +/** Reported once: the shipped file lost the anchor the loader-less rendering rewrites. */ +let warnedAnchor = false /** Report an unreadable guidance file once, never once per agent or call. */ function warnUnavailable(location: string, error: unknown): void { @@ -60,6 +62,248 @@ export function loadBatchingGuidance(filePath?: string | URL): string { } } +/** + * What one PTC scope actually publishes, in public tool names. + * + * The guidance file teaches progressive disclosure, and its loader lesson assumes + * `ops_load_sdk` is reachable. The loader is only published while dynamic loading is + * on, so the same text must not reach a scope that has none: it would spend a round + * trip in that scope on a call that cannot succeed. + */ +export interface GuidanceSurface { + /** Whether the scope publishes `ops_load_sdk`. */ + readonly loader: boolean + /** Public names the scope publishes without loading. */ + readonly resident: readonly string[] + /** Public names the loader can add, empty when the scope publishes no loader. */ + readonly loadable: readonly string[] +} + +/** The file-work APIs the guidance names, in the order it names them. */ +const FILE_APIS: readonly string[] = Object.freeze([ + 'ops_glob', 'ops_grep', 'ops_read_files', 'ops_inspect_local_file', 'ops_edit_file', +]) + +/** + * One sentence naming the file-work APIs the scope really publishes. + * + * The shipped wording is kept when every name it mentions is available, so the + * common loader scope reads exactly as before; a narrower scope gets the subset and + * a scope with none of them is pointed at the official tools instead. + * @param names - public names available for file work in this scope. + * @returns the sentence for the bypass notice. + */ +function fileWorkHint(names: readonly string[]): string { + const usable = FILE_APIS.filter(name => names.includes(name)) + if (usable.length === 0) return 'No OPS file API is available here: do file work with the official tools.' + if (usable.length === FILE_APIS.length) { + return 'List with `ops_glob`, search with `ops_grep`, read with `ops_read_files` or `ops_inspect_local_file`, edit with `ops_edit_file`.' + } + return 'Do file work with the APIs this scope publishes: `' + usable.join('`, `') + '`.' +} + +/** + * Drop every section whose heading names loading. + * + * The fallback path runs when the shipped file no longer carries the anchor: a + * renamed heading still leaves the old lesson in place, and pairing it with the + * replacement would put two contradicting steps in one document. + * @param rows - guidance lines. + * @returns the lines without loader-named sections. + */ +function dropLoadSections(rows: readonly string[]): string[] { + const kept: string[] = [] + let dropping = false + let fence: string | null = null + for (const row of rows) { + if (fence !== null) { + if (row.trim().startsWith(fence)) fence = null + if (!dropping) kept.push(row) + continue + } + const opening = /^\s*(```+|~~~+)/.exec(row) + if (opening !== null) { + fence = opening[1]! + if (!dropping) kept.push(row) + continue + } + // A heading inside a fence is example text, and only a word that means loading may + // drop a section: "payload", "download" and "workload" are ordinary prose. + if (row.startsWith('## ')) dropping = LOAD_HEADING.test(row) + if (!dropping) kept.push(row) + } + return kept +} + +/** + * Remove every trace of the load step from the lines that stay in the document. + * + * A fenced block is dropped as a unit when it teaches loading: deleting the call and + * keeping the lines that consumed the loaded names would ship a half example that a + * model can copy. + * @param rows - guidance lines outside the replaced section. + * @returns the lines without the load step. + */ +function stripLoader(rows: readonly string[]): string[] { + const kept: string[] = [] + let fence: string | null = null + let block: string[] = [] + for (const row of rows) { + if (fence === null) { + const opening = /^\s*(```+|~~~+)/.exec(row) + if (opening !== null) { + fence = opening[1]! + block = [row] + continue + } + if (row.startsWith(LOAD_TABLE_ROW_PREFIX)) continue + if (row.includes('ops_load_sdk')) continue + kept.push(row) + continue + } + block.push(row) + if (row.trim().startsWith(fence)) { + if (!block.some(line => line.includes('ops_load_sdk'))) kept.push(...block) + fence = null + block = [] + } + } + // An unterminated fence keeps its lines: dropping the tail of a drifted file would + // hide more than the load step. + kept.push(...block) + return kept +} + +/** The heading that starts the loader lesson in the guidance file. */ +export const LOAD_SECTION_HEADING = '## Step 3 - load before you call' +/** A heading that names loading, by word: "payload" is not a load step. */ +const LOAD_HEADING = /\bload(?:ing|er)?\b/i + +/** The one table row that teaches the loader. */ +export const LOAD_TABLE_ROW_PREFIX = '| ops_read_files before it is loaded' + +/** + * The section that replaces the loader lesson when the scope publishes no loader. + * @param surface - what the scope publishes. + * @returns the replacement lines, heading included. + */ +function directSteps(surface: GuidanceSurface): string[] { + if (surface.resident.length === 0) { + return [ + '## Step 3 - what this scope publishes', + '', + 'This scope publishes no OPS API and no loader. Work with the official tools;', + 'an `ops_*` name outside the SDK is not available here, and retrying will not', + 'make one appear.', + ] + } + return [ + '## Step 3 - call what is published', + '', + 'This scope publishes no loader: every API already in the SDK is callable directly,', + 'and a name outside it is not available. Calling what you already have is the whole', + 'step, with no load to spend a program on:', + '', + '`' + surface.resident.join('`, `') + '`', + ] +} + +/** + * The index of the heading that ends the section starting at `start`. + * @param lines - the guidance lines. + * @param start - the section heading index. + * @returns the next heading index, or the line count when the section is last. + */ +/** + * The index of the load-step heading, ignoring anything inside a fence. + * + * A fenced example that quotes the heading is text, not structure: taking it for the + * anchor would cut the document in the wrong place. + * @param lines - the guidance lines. + * @returns the heading index, or -1 when the file lost it. + */ +function anchorIndex(lines: readonly string[]): number { + let fence: string | null = null + for (let index = 0; index < lines.length; index += 1) { + const row = lines[index]! + if (fence !== null) { + if (row.trim().startsWith(fence)) fence = null + continue + } + const opening = /^\s*(```+|~~~+)/.exec(row) + if (opening !== null) { + fence = opening[1]! + continue + } + if (row.trim() === LOAD_SECTION_HEADING) return index + } + return -1 +} + +function nextHeading(lines: readonly string[], from: number): number { + let fence: string | null = null + for (let index = from; index < lines.length; index += 1) { + const row = lines[index]! + if (fence !== null) { + if (row.trim().startsWith(fence)) fence = null + continue + } + const opening = /^\s*(```+|~~~+)/.exec(row) + if (opening !== null) { + fence = opening[1]! + continue + } + if (row.startsWith('## ')) return index + } + return lines.length +} + +/** + * The index of the heading that ends the section starting at `start`. + * @param lines - the guidance lines. + * @param start - the section heading index. + * @returns the next heading index outside any fence, or the line count. + */ +function sectionEnd(lines: readonly string[], start: number): number { + return nextHeading(lines, start + 1) +} + +/** + * Render the guidance for one published surface. + * + * A scope with the loader receives the file byte for byte, so the default path is + * unchanged. A loader-less scope receives the same lesson with the load step + * replaced: the prose that tells the model to load cannot be true there. + * @param text - the loaded guidance file. + * @param surface - what the scope publishes. + * @returns the model-facing guidance text. + */ +export function renderBatchingGuidance(text: string, surface: GuidanceSurface): string { + if (text === '' || surface.loader) return text + const lines = text.split('\n') + const start = anchorIndex(lines) + // Lines that teach loading, and the table row that points at it, never reach a + // scope without a loader. The replacement is inserted after this filter, so the + // filter can never eat the section it was built to produce. + if (start < 0) { + // A drifted file is a plugin defect, not a model-facing surprise: say so once, + // then still keep the lesson free of an unreachable call. + if (!warnedAnchor) { + warnedAnchor = true + console.warn('dsh-ops: the batching guidance lost its "' + LOAD_SECTION_HEADING + + '" anchor; the load step is replaced before the first section instead.') + } + const body = dropLoadSections(stripLoader(lines)) + const at = nextHeading(body, 0) + return [...body.slice(0, at), ...directSteps(surface), '', ...body.slice(at)].join('\n') + } + // Insert the replacement exactly where the load step stood, so the document keeps + // its heading order and the shared sections keep their positions. + const before = stripLoader(lines.slice(0, start)) + const after = stripLoader(lines.slice(sectionEnd(lines, start))) + return [...before, ...directSteps(surface), '', ...after].join('\n') +} + // ---- program shape: what one run_code program says about its step ---- /** A program shorter than this is treated as single-step unless it is structured. */ @@ -351,7 +595,7 @@ export interface ProgramLatch { * @param shape - the program shape the notice describes. * @returns the model-facing text, without the plugin-context envelope. */ -export function noticeText(category: BatchingCategory, shape: ProgramShape): string { +export function noticeText(category: BatchingCategory, shape: ProgramShape, surface: GuidanceSurface): string { if (category === 'plan') { return [ 'Your last program did not open with a plan line.', @@ -360,10 +604,27 @@ export function noticeText(category: BatchingCategory, shape: ProgramShape): str ].join('\n') } if (category === 'bypass') { + // Every line names only what this scope publishes: a hardcoded tool list would + // contradict the sentence that explains the scope two lines below it. + // Resident APIs need no load, so they count as reachable beside the pool. + const available = surface.loader + ? [...new Set([...surface.resident, ...surface.loadable])] + : [...surface.resident] + const fileUsable = FILE_APIS.filter(name => available.includes(name)).length + const coversFiles = fileUsable === FILE_APIS.length return [ 'Your last program did file work through the shell or Node IO, which bypasses path policy and the audit trail.', - 'List with `ops_glob`, search with `ops_grep`, read with `ops_read_files` or `ops_inspect_local_file`, edit with `ops_edit_file`.', - 'Use the APIs in the current SDK; load only missing names with `tools.ops_load_sdk` for the next program.', + // A pool that cannot load `ops_edit_file` must not be told to load it either. + fileWorkHint(available), + surface.loader + ? coversFiles + ? 'Use the APIs in the current SDK; load only missing names with `tools.ops_load_sdk` for the next program.' + : fileUsable === 0 + ? 'The loadable pool holds no file API either, so loading adds nothing here.' + : 'Load only names this scope publishes with `tools.ops_load_sdk`; an API outside that pool is not reachable.' + : fileUsable === 0 + ? 'There is no loader here, and retrying a name will not publish one.' + : 'There is no loader here: call those APIs directly in the next program.', ].join('\n') } return [ @@ -382,7 +643,7 @@ export function noticeText(category: BatchingCategory, shape: ProgramShape): str * by the notice text, while a missed correction lasts the rest of the turn. * @returns the latch; state is keyed by agent identity and never serialized. */ -export function createProgramLatch(): ProgramLatch { +export function createProgramLatch(surfaceOf: (agent: Agent) => GuidanceSurface): ProgramLatch { const states = new WeakMap; sent: Record }>() const fresh = (turn: number) => ({ turn, armed: { plan: true, bypass: true, shape: true }, sent: { plan: 0, bypass: 0, shape: 0 } }) const stateFor = (agent: Agent) => { @@ -408,7 +669,8 @@ export function createProgramLatch(): ProgramLatch { if (category === undefined) return undefined state.armed[category] = false state.sent[category] += 1 - return { category, turn: state.turn, text: noticeText(category, shape) } + // Read-only policy is per agent, so the surface is resolved at delivery time. + return { category, turn: state.turn, text: noticeText(category, shape, surfaceOf(agent)) } }, isArmed(agent, category) { return stateFor(agent).armed[category] }, stats(agent) { diff --git a/src/publication/ptc.ts b/src/publication/ptc.ts index f9167c8e..206ac340 100644 --- a/src/publication/ptc.ts +++ b/src/publication/ptc.ts @@ -13,7 +13,8 @@ import { renderSdkDirectory } from '../projections/ptc/directory.js' import { PTC_RESIDENT_TOOLS } from '../ptc/config.js' import type { ApiName } from '../ptc/config.js' import { ownPtcScopes } from '../ptc/ownership.js' -import { BATCHING_GUIDANCE_ID, BATCHING_GUIDANCE_REVISION, createProgramLatch, loadBatchingGuidance } from '../ptc/batching-guidance.js' +import { BATCHING_GUIDANCE_ID, BATCHING_GUIDANCE_REVISION, createProgramLatch, loadBatchingGuidance, renderBatchingGuidance } from '../ptc/batching-guidance.js' +import type { GuidanceSurface } from '../ptc/batching-guidance.js' import { InvocationLifetime, InvocationNotStarted } from '../lifecycle/invocations.js' import { monitoredInvoke } from '../execution/monitored.js' import { record } from '../execution/boundary.js' @@ -80,13 +81,30 @@ export function mountPtc(root: Context, rawConfig: unknown, gate: PtcGate = { al const selectable: ApiName[] = gate.dynamic === false ? [] : [...config.allowedTools, 'result' as ApiName] const pool = new Set([...resident, ...selectable]) const owns = (agent: Agent): boolean => !stopped && gate.allows(agent) && Boolean(ownership?.owns(agent)) + // One predicate for "may this agent call it": the guidance surface and the tool + // registration must never disagree about a read-only policy. + const reachable = (mode: string | undefined, api: ApiName): boolean => + mode !== 'read-only' || !capability(api)?.mutates const permitted = (agent: Agent, api: ApiName | 'load_sdk'): boolean => { if (!owns(agent) || !ctx.get('ptcRuntime') || !ctx.get('subprocess')) return false const policy = ctx.get('sandboxPolicy')?.resolve({ session: agent.session }) return policy !== undefined && ['read-only', 'workspace-write', 'danger-full-access'].includes(policy.mode) - && (api === 'load_sdk' || api === 'result' || !capability(api)?.mutates || policy.mode !== 'read-only') + && (api === 'load_sdk' || api === 'result' || reachable(policy.mode, api)) } const loadable = [...new Set([...config.allowedTools, 'result'])] + // What this scope publishes decides which guidance text is truthful, and the answer + // is per agent: a read-only policy removes the mutating calls even when the row names + // them. Ownership and runtime availability are checked separately, so teaching never + // depends on mount order. + const surfaceFor = (agent: Agent): GuidanceSurface => { + const mode = ctx.get('sandboxPolicy')?.resolve({ session: agent.session })?.mode + return { + loader: gate.dynamic !== false, + resident: [...resident].filter(api => reachable(mode, api)).map(api => publicToolName(api)), + // Loadable names answer to the same gate the loader itself uses. + loadable: allowedApis(agent).map(api => publicToolName(api)), + } + } const allowedApis = (agent: Agent): ApiName[] => gate.dynamic === false ? [] : loadable.filter(api => permitted(agent, api)) const selected = (entry: Entry, api: ApiName | 'load_sdk'): boolean => api === 'load_sdk' ? gate.dynamic !== false : resident.has(api) || entry.selected.has(api) @@ -99,7 +117,7 @@ export function mountPtc(root: Context, rawConfig: unknown, gate: PtcGate = { al // the triggering tool result, so no system-prompt bytes are spent up front. // One latch for the program-shape trigger. It observes the root run_code program, // which is the only place the whole step is visible. - const shapeLatch = createProgramLatch() + const shapeLatch = createProgramLatch(surfaceFor) const guidanceMessage = async (turn: number, text: string): Promise => { const createUserMessage = await loadUserMessage() return createUserMessage({ @@ -190,7 +208,7 @@ export function mountPtc(root: Context, rawConfig: unknown, gate: PtcGate = { al revision: BATCHING_GUIDANCE_REVISION, provide() { if (entry.taught) return undefined - const text = loadBatchingGuidance() + const text = renderBatchingGuidance(loadBatchingGuidance(), surfaceFor(agent)) return text.length > 0 ? text : undefined }, })) diff --git a/test/batching-guidance.test.mjs b/test/batching-guidance.test.mjs index b562b738..ca8f0488 100644 --- a/test/batching-guidance.test.mjs +++ b/test/batching-guidance.test.mjs @@ -7,7 +7,7 @@ import fs from 'node:fs' import path from 'node:path' import { assert, test, report, PACKAGE_ROOT } from './lib/harness.mjs' -import { BATCHING_GUIDANCE_ID, BATCHING_GUIDANCE_REVISION, loadBatchingGuidance } from '../lib/ptc/batching-guidance.js' +import { BATCHING_GUIDANCE_ID, BATCHING_GUIDANCE_REVISION, LOAD_SECTION_HEADING, LOAD_TABLE_ROW_PREFIX, loadBatchingGuidance, renderBatchingGuidance } from '../lib/ptc/batching-guidance.js' /** The editable guidance file shipped at the package root, outside the build output. */ const GUIDANCE_FILE = path.join(PACKAGE_ROOT, 'prompts', 'batching-guidance.md') @@ -20,7 +20,7 @@ await test('the shipped guidance is English Markdown with its host-source identi } assert.equal(BATCHING_GUIDANCE_ID, 'batching-guidance') // A wording or format change is a product change and must bump the revision. - assert.equal(BATCHING_GUIDANCE_REVISION, '4') + assert.equal(BATCHING_GUIDANCE_REVISION, '5') }) await test('the guidance loader returns exactly the shipped Markdown file', () => { @@ -106,4 +106,98 @@ await test('the teacher trajectory pins the first-line plan and a runnable load assert.ok(text.length > 2500 && text.length < 9000, 'guidance stays bounded: ' + text.length) }) +await test('a scope with the loader receives the shipped file unchanged', () => { + // The default path must not pay for a branch it never needs: byte identity is the contract. + const text = loadBatchingGuidance() + assert.equal(renderBatchingGuidance(text, { loader: true, resident: [] }), text) + // An unreadable file stays empty, so delivery is skipped instead of failing a tool result. + assert.equal(renderBatchingGuidance('', { loader: false, resident: ['ops_glob'], loadable: [] }), '') +}) + +await test('a loader-less scope is never taught to load', () => { + const rendered = renderBatchingGuidance(loadBatchingGuidance(), { loader: false, resident: ['ops_glob', 'ops_grep'], loadable: [] }) + // The defect this guards: the lesson named a call that cannot succeed in this scope. + assert.equal(rendered.includes('ops_load_sdk'), false, 'the unreachable loader is gone') + assert.equal(rendered.includes(LOAD_SECTION_HEADING), false, 'the load section is replaced') + assert.ok(rendered.includes('## Step 3 - call what is published'), 'the step that remains is named') + assert.ok(rendered.includes('ops_glob') && rendered.includes('ops_grep'), 'the resident names are listed instead') + // Everything shared with the loader path survives verbatim: the lesson is not truncated. + for (const kept of ['# Batch tool calls', '## Step 1 - split by dependency', '## Step 2 - reconcile', '## Common mistakes', '// plan:']) { + assert.ok(rendered.includes(kept), kept) + } +}) + +await test('a scope that publishes nothing says so', () => { + const rendered = renderBatchingGuidance(loadBatchingGuidance(), { loader: false, resident: [] }) + assert.equal(rendered.includes('ops_load_sdk'), false) + assert.ok(rendered.includes('publishes no OPS API and no loader'), 'the empty surface is stated') + assert.equal(rendered.includes('## Step 3 - call what is published'), false, 'no resident list is invented') +}) + +await test('the replacement survives a resident list that names the loader', () => { + // The list is generated after the filter, so the filter can never eat it. + const rendered = renderBatchingGuidance(loadBatchingGuidance(), { loader: false, resident: ['ops_load_sdk', 'ops_glob'], loadable: [] }) + assert.ok(rendered.includes('ops_load_sdk') && rendered.includes('ops_glob'), 'the list is rendered whole') +}) + +await test('a drifted file keeps its structure when the anchor is gone', () => { + const drifted = '# Batch tool calls\n\nintro\n## Step 1 - split by dependency\nbody mentions ops_load_sdk once\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.ok(out.startsWith('# Batch tool calls'), 'the H1 stays first') + assert.ok(out.indexOf('call what is published') < out.indexOf('## Step 1'), 'the step precedes the first section') + assert.equal(out.includes('ops_load_sdk'), false, 'no unreachable mention survives') +}) + +await test('a renamed load section does not survive beside its replacement', () => { + // A drifted file keeps the old lesson under a different heading; two contradicting + // Step 3 sections would teach both loading and direct calls. + const drifted = '# Batch tool calls\n\nintro\n## Step 3 - loading tools\nload prose here\n## Step 1 - split\nbody\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.equal((out.match(/## Step 3/g) ?? []).length, 1, 'exactly one Step 3 remains') + assert.equal(out.includes('load prose here'), false, 'the old load section is dropped') + assert.ok(out.startsWith('# Batch tool calls'), 'the H1 stays first') +}) + +await test('a fenced example that teaches loading is dropped whole', () => { + // Deleting one line inside a fence would leave a half example that calls a name the + // scope cannot reach, so the whole block goes with it. + const drifted = '# Batch tool calls\n\nintro\n```js\nconst sdk = await tools.ops_load_sdk({ tools: ["edit_file"] })\nawait sdk.ops_edit_file({ path: "a" })\n```\n## Step 1 - split\nbody\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.equal(out.includes('sdk.ops_edit_file'), false, 'the dangling call does not survive') + assert.equal(out.includes('ops_load_sdk'), false, 'the load call does not survive') + assert.equal((out.match(/```/g) ?? []).length, 0, 'no half-open fence is left behind') +}) + +await test('only a heading that means loading loses its section', () => { + // A substring match would swallow "Payload" and drop a valid section with it. + const drifted = '# Batch tool calls\n## Payload budget\nkeep me\n## Step 3 - loading tools\nload prose\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.ok(out.includes('## Payload budget'), 'the ordinary section survives') + assert.ok(out.includes('keep me'), 'its body survives too') + assert.equal(out.includes('load prose'), false, 'the load section still goes') +}) + +await test('a heading inside a fence is example text, not a boundary', () => { + // The fenced line must not cut the load section short, or half of it survives. + const drifted = '# Batch tool calls\n## Step 3 - load before you call\nprose one\n```\n## not a heading\n```\nprose two\n## Trajectory 1 - next\nbody\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.equal(out.includes('prose two'), false, 'the whole section is replaced') + assert.ok(out.includes('## Trajectory 1 - next'), 'the next real section is kept') +}) + +await test('a fenced copy of the heading is not the anchor', () => { + // Taking the quoted line for the anchor would slice the document inside the fence. + const drifted = '# Batch tool calls\n```\n## Step 3 - load before you call\n```\nreal prose\n' + const out = renderBatchingGuidance(drifted, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.ok(out.includes('real prose'), 'the real body survives the fallback') + assert.ok(out.startsWith('# Batch tool calls'), 'the H1 stays first') +}) + +await test('the shipped file still carries the anchors the renderer rewrites', () => { + // The loader-less rendering edits prose by anchor: moving either one must fail here, not at runtime. + const lines = loadBatchingGuidance().split('\n') + assert.ok(lines.some(line => line.trim() === LOAD_SECTION_HEADING), 'the load heading is present') + assert.ok(lines.some(line => line.startsWith(LOAD_TABLE_ROW_PREFIX)), 'the load row is present') +}) + report('batching-guidance') diff --git a/test/batching-trigger.test.mjs b/test/batching-trigger.test.mjs index ce60e224..14df97a6 100644 --- a/test/batching-trigger.test.mjs +++ b/test/batching-trigger.test.mjs @@ -11,6 +11,9 @@ import { assert, test, report } from './lib/harness.mjs' import { analyzeProgram, parsePlan, violationsOf, createProgramLatch, noticeText, CATEGORY_PRIORITY, PLAN_DIRECTIVE, THIN_CODE_CHARS } from '../lib/ptc/batching-guidance.js' +/** A scope that publishes the loader and keeps the pool loadable. */ +const LOADER = { loader: true, resident: [], loadable: ['ops_glob', 'ops_grep', 'ops_read_files', 'ops_inspect_local_file', 'ops_edit_file'] } + await test('the plan comment is read from the first physical line only', () => { assert.deepEqual(parsePlan('// plan: read the config; read the test'), ['read the config', 'read the test']) // The full-width semicolon separates entries as well; both are typed on a CJK layout. @@ -89,7 +92,7 @@ await test('stdin filters do not accuse command output of bypassing file APIs', assert.equal(analyzeProgram('await tools.ops_bash({ description: "grep file", command: "git diff" })').bypassesFileTools, false) assert.equal(analyzeProgram('const example = "tools.ops_bash({ command: \'cat file\' })"; return example').dispatchSites, 0) assert.equal(analyzeProgram('// tools.ops_bash({ command: "cat file" })').bypassesFileTools, false) - assert.doesNotMatch(noticeText('bypass', shape('cat file')), /Only .* resident|load the others/) + assert.doesNotMatch(noticeText('bypass', shape('cat file'), LOADER), /Only .* resident|load the others/) }) await test('a program with no dispatch site earns no category', () => { @@ -99,7 +102,7 @@ await test('a program with no dispatch site earns no category', () => { assert.deepEqual(violationsOf(analyzeProgram(code)), { plan: false, bypass: false, shape: false }) } // The latch stays silent, so the model is never accused of a shape it did not take. - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} assert.equal(latch.observe(agent, 'return 1'), undefined) assert.equal(latch.observe(agent, '// plan: read nothing\n// a note\nreturn 1'), undefined) assert.equal(latch.stats(agent).total, 0, 'no category was spent') @@ -136,7 +139,7 @@ const THIN = 'const r = await tools.ops_read_files({ files: [{ path: "a" }] })\n const GOOD = '// plan: list the sources; read the manifest\nconst [a, b] = await Promise.all([tools.ops_glob({ pattern: ["src/**"] }), tools.ops_read_files({ files: [{ path: "package.json" }] })])\nreturn { a, b }' await test('one category delivers once per turn and stays silent until it is re-armed', () => { - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} assert.equal(latch.observe(agent, THIN).category, 'plan', 'the first program earns the plan notice') assert.equal(latch.isArmed(agent, 'plan'), false) // THIN breaks two categories, so the second one speaks before the turn falls silent. @@ -145,7 +148,7 @@ await test('one category delivers once per turn and stays silent until it is re- }) await test('a compliant program re-arms its category, so a regression speaks again', () => { - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} assert.equal(latch.observe(agent, THIN).category, 'plan') assert.equal(latch.observe(agent, THIN).category, 'shape', 'the other broken category still speaks') assert.equal(latch.observe(agent, THIN), undefined, 'both categories are spent') @@ -157,7 +160,7 @@ await test('a compliant program re-arms its category, so a regression speaks aga }) await test('priority orders the categories and the others stay armed', () => { - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} assert.deepEqual([...CATEGORY_PRIORITY], ['bypass', 'plan', 'shape']) assert.equal(latch.observe(agent, BYPASS).category, 'bypass', 'a bypass outranks the other two') assert.equal(latch.isArmed(agent, 'plan'), true, 'the plan notice was not spent') @@ -166,7 +169,7 @@ await test('priority orders the categories and the others stay armed', () => { }) await test('a new turn re-arms every category and a repeated turn does not', () => { - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} latch.noteTurn(agent, 1) assert.equal(latch.observe(agent, THIN).category, 'plan') latch.noteTurn(agent, 1) @@ -176,7 +179,7 @@ await test('a new turn re-arms every category and a repeated turn does not', () }) await test('state never leaks between agents and forget drops it', () => { - const latch = createProgramLatch(), first = {}, second = {} + const latch = createProgramLatch(() => LOADER), first = {}, second = {} assert.equal(latch.observe(first, THIN).category, 'plan') assert.equal(latch.observe(second, THIN).category, 'plan', 'the second agent is independent') latch.forget(first) @@ -184,7 +187,7 @@ await test('state never leaks between agents and forget drops it', () => { }) await test('the latch counts deliveries per turn for the optional telemetry line', () => { - const latch = createProgramLatch(), agent = {} + const latch = createProgramLatch(() => LOADER), agent = {} latch.noteTurn(agent, 4) assert.deepEqual(latch.stats(agent), { turn: 4, plan: 0, bypass: 0, shape: 0, total: 0 }) latch.observe(agent, THIN) @@ -197,11 +200,77 @@ await test('the latch counts deliveries per turn for the optional telemetry line await test('the notices name the fix and never claim a runtime dispatch count', () => { const shape = analyzeProgram(BYPASS) - assert.match(noticeText('plan', shape), /first line/, 'the plan notice names the first line') - assert.doesNotMatch(noticeText('plan', shape), /Promise\.all/, 'the plan notice stays about the comment') - assert.match(noticeText('bypass', shape), /ops_glob/) - assert.match(noticeText('bypass', shape), /ops_load_sdk/) - assert.match(noticeText('shape', shape), /program text alone/, 'the shape notice states its own limit') + assert.match(noticeText('plan', shape, LOADER), /first line/, 'the plan notice names the first line') + assert.doesNotMatch(noticeText('plan', shape, LOADER), /Promise\.all/, 'the plan notice stays about the comment') + assert.match(noticeText('bypass', shape, LOADER), /ops_glob/) + assert.match(noticeText('bypass', shape, LOADER), /ops_load_sdk/) + assert.match(noticeText('shape', shape, LOADER), /program text alone/, 'the shape notice states its own limit') +}) + +await test('the bypass notice names only the APIs its scope publishes', () => { + const shape = analyzeProgram(BYPASS) + // With a loader the notice keeps teaching the load step. + assert.match(noticeText('bypass', shape, LOADER), /ops_load_sdk/) + // Without one it must not name a call that cannot succeed, and every other line + // must stay inside the published surface too. + const direct = noticeText('bypass', shape, { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.equal(direct.includes('ops_load_sdk'), false, 'the unreachable loader is gone') + assert.match(direct, /ops_glob/, 'the published API is named') + for (const absent of ['ops_read_files', 'ops_grep', 'ops_edit_file']) { + assert.equal(direct.includes(absent), false, 'an unpublished API is not named: ' + absent) + } + // A scope that publishes nothing points at the official tools only. + const none = noticeText('bypass', shape, { loader: false, resident: [], loadable: [] }) + assert.match(none, /official tools/) + assert.equal(/ops_[a-z_]+/.test(none), false, 'no OPS name survives an empty surface') +}) + +await test('a narrow pool is not told to load an API it cannot reach', () => { + const shape = analyzeProgram(BYPASS) + // The pool holds stat only: naming edit_file would cost the same failed round trip. + const narrow = noticeText('bypass', shape, { loader: true, resident: [], loadable: ['ops_stat'] }) + for (const absent of ['ops_edit_file', 'ops_read_files', 'ops_glob']) { + assert.equal(narrow.includes(absent), false, 'an unloadable API is not named: ' + absent) + } + assert.match(narrow, /official tools/, 'an empty file-api intersection points at the official tools') + // The shipped wording survives while the whole list is loadable. + assert.match(noticeText('bypass', shape, LOADER), /List with `ops_glob`/) + // A narrow pool must not invite a load with the old catch-all sentence. + assert.doesNotMatch(narrow, /load only missing names/, 'the load sentence follows the pool too') +}) + +await test('a resident file API is named even when the pool cannot load it', () => { + const shape = analyzeProgram(BYPASS) + // read_files is resident and callable without loading; the hint must not claim that + // no file API exists just because the pool holds stat alone. + const mixed = noticeText('bypass', shape, { loader: true, resident: ['ops_read_files'], loadable: ['ops_stat'] }) + assert.match(mixed, /ops_read_files/, 'the resident file API is named') + assert.doesNotMatch(mixed, /No OPS file API is available/, 'the fallback does not fire while one is reachable') +}) + +await test('the file-work line never names the shell it just accused', () => { + const shape = analyzeProgram(BYPASS) + const mixed = noticeText('bypass', shape, { loader: false, resident: ['ops_bash', 'ops_glob'], loadable: [] }) + assert.match(mixed, /ops_glob/, 'the file API is named') + assert.equal(mixed.includes('ops_bash'), false, 'the bypass vector is never offered as the fix') +}) + +await test('the latch resolves the surface per agent', () => { + const first = {}, second = {} + const latch = createProgramLatch(agent => agent === first + ? LOADER + : { loader: false, resident: ['ops_glob'], loadable: [] }) + assert.match(latch.observe(first, BYPASS).text, /ops_load_sdk/, 'the loader scope keeps its lesson') + assert.equal(latch.observe(second, BYPASS).text.includes('ops_load_sdk'), false, 'the other agent gets its own surface') +}) + +await test('a scope with no file API does not refer to them', () => { + const shape = analyzeProgram(BYPASS) + // A resident non-file API leaves the hint pointing at the official tools; the next + // line must not then invite a call to "those APIs". + const statOnly = noticeText('bypass', shape, { loader: false, resident: ['ops_stat'], loadable: [] }) + assert.match(statOnly, /official tools/, 'the official tools are named') + assert.doesNotMatch(statOnly, /call those APIs/, 'the pronoun has no referent here') }) report('batching-trigger') diff --git a/test/resident-switch.test.mjs b/test/resident-switch.test.mjs index 68945394..1d4c64bd 100644 --- a/test/resident-switch.test.mjs +++ b/test/resident-switch.test.mjs @@ -203,6 +203,22 @@ await test('ops-ptc batchingGuidance teaches until the lesson is committed to hi } finally { await shutdown(ctx) } })) +await test('a loader-less ops-ptc row teaches only what it publishes', () => withTempDir('batching-no-loader', async cwd => { + const { ctx, agent } = await setup(cwd, 'ops-ptc', { residentTools: true, dynamicTools: false, + batchingGuidance: true, alwaysLoaded: ['stat'], allowedTools: ['stat'] }) + try { + await waitFor(() => names(ctx, agent).includes(publicToolName('stat'))) + // End to end through the provider: the scope publishes no loader, so the lesson + // must not name one. This is the assertion that fails when the surface is dropped. + const taught = stepTailMessages(await drivePreStep(agent, 1)) + assert.equal(taught.length, 1, JSON.stringify(taught)) + const text = JSON.stringify(taught) + assert.equal(text.includes('ops_load_sdk'), false, 'the unreachable loader is gone') + assert.match(text, /call what is published/, 'the direct-call step replaces it') + assert.match(text, /ops_stat/, 'the published API is listed instead') + } finally { await shutdown(ctx) } +})) + await test('ops-ptc without batchingGuidance teaches nothing on the first turn', () => withTempDir('batching-teach-off', async cwd => { const { ctx, agent } = await setup(cwd, 'ops-ptc', { residentTools: true, dynamicTools: false, batchingGuidance: false, alwaysLoaded: ['stat'], allowedTools: ['stat'] }) @@ -230,12 +246,30 @@ await test('the root run_code trigger answers on its own tool result', () => wit assert.equal(thin.isError, false, JSON.stringify(thin)) assert.match(text(thin), /did not open with a plan line/, 'the plan notice rides the root result') assert.doesNotMatch(text(thin), /Promise\.all/, 'the plan notice stays about the comment') - // A shell listing is a bypass; the notice names the API that replaces it. + // A shell listing is a bypass; resident file APIs are reachable without loading, + // so the notice still names them even though this pool could not load them. const bypass = await run('const r = await tools.ops_bash({ description: "list", command: "ls -la" })\nreturn r.stdout.text') - assert.match(text(bypass), /ops_glob/, 'the bypass notice names the listing tool') + assert.match(text(bypass), /ops_inspect_local_file/, 'the notice names the resident file API') + assert.doesNotMatch(text(bypass), /ops_bash/, 'the shell it just accused is not offered as the fix') // The trigger appends context and never rewrites the tool result itself. assert.equal(thin.value.result, 'completed', JSON.stringify(thin.value)) } finally { await shutdown(ctx) } })) +await test('a read-only agent is never told to edit files through the notice', () => withTempDir('batching-readonly', async cwd => { + const { ctx, agent } = await setup(cwd, 'ops-ptc', { residentTools: true, dynamicTools: true, + batchingGuidance: true, alwaysLoaded: ['stat'], allowedTools: ['stat', 'edit_file'] }) + try { + await waitFor(() => names(ctx, agent).includes(publicToolName('stat'))) + // The policy is resolved per delivery, so a read-only session narrows the surface. + agent.session.mode = 'read-only' + const result = await executeCall(ctx, { agent, toolSet: agent.prepared.toolSet, name: 'run_code', + arguments: { description: 'shape fixture', code: 'const r = await tools.ops_bash({ description: "list", command: "ls -la" })\nreturn r.stdout.text' }, + callId: 'readonly-' + ++sequence, signal: new AbortController().signal }) + const text = JSON.stringify(result.additionalContexts ?? []) + assert.doesNotMatch(text, /ops_edit_file/, 'a read-only agent is not taught to mutate') + assert.match(text, /ops_inspect_local_file/, 'the reachable file API stays named') + } finally { await shutdown(ctx) } +})) + report('resident-switch')