From c0330a8b9fddac9007ca1ded25627a339003c7c1 Mon Sep 17 00:00:00 2001 From: elkaix Date: Wed, 9 Sep 2026 19:13:07 -0400 Subject: [PATCH 01/12] fix: keep MCP structured results and warn on malformed model entries Preserve an MCP tool's structured payload alongside its text and media blocks, dropping the structured copy only when a text block already carries the same complete JSON value, and escape angle brackets in the serialized extras instead of stripping literal closing tags. Warn at config load when a [models] entry has no model field, with a hint when an unquoted dotted alias parsed as a nested table. Config sections now contribute load-time diagnostics through a hook. Scrub every ambient product env var in the engine and gateway test setups and give the SDK, client and extension projects an explicit integration-test timeout. --- apps/vscode/vitest.projects.ts | 1 + docs/customization/mcp.md | 2 + .../agent-core-v2/src/agent/mcp/output.ts | 30 +++-- .../agent-core-v2/src/app/config/config.ts | 4 + .../src/app/config/configService.ts | 11 +- .../src/app/kosongConfig/configSection.ts | 50 +++++++++ .../test/agent/mcp/output.test.ts | 104 ++++++++++++++++-- .../test/app/config/config.test.ts | 92 ++++++++++++++++ packages/agent-core-v2/test/setup.ts | 2 +- packages/agent-gateway/test/setup.ts | 3 +- packages/klient/vitest.config.ts | 1 + packages/node-sdk/vitest.config.ts | 1 + 12 files changed, 282 insertions(+), 19 deletions(-) diff --git a/apps/vscode/vitest.projects.ts b/apps/vscode/vitest.projects.ts index b0f3f673f..84ee9603d 100644 --- a/apps/vscode/vitest.projects.ts +++ b/apps/vscode/vitest.projects.ts @@ -17,6 +17,7 @@ export const vscodeProjects = [ include: ['test/**/*.test.ts'], exclude: ['test/webview/**'], environment: 'node', + testTimeout: 15_000, }, }, { diff --git a/docs/customization/mcp.md b/docs/customization/mcp.md index 392746006..2b0c37a7a 100644 --- a/docs/customization/mcp.md +++ b/docs/customization/mcp.md @@ -2,6 +2,8 @@ [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) is an open protocol that lets models safely call tools exposed by external processes or services — for example, reading GitHub issues, querying databases, or operating the local file system. Pythinker Code CLI acts as an MCP client to connect these external tools and exposes them to the Agent alongside built-in tools (`Read`, `Bash`, `Grep`, etc.) with no behavioral difference. +MCP tool results can include text (`content`) and structured data (`structuredContent`). Pythinker Code CLI makes both available to the agent and omits the structured copy only when it can confirm that a text block already contains the same complete JSON value. Text summaries and media do not replace structured records. + ## Connection Methods Pythinker Code CLI supports three MCP server connection methods: diff --git a/packages/agent-core-v2/src/agent/mcp/output.ts b/packages/agent-core-v2/src/agent/mcp/output.ts index 3d68783cf..a2bc65e46 100644 --- a/packages/agent-core-v2/src/agent/mcp/output.ts +++ b/packages/agent-core-v2/src/agent/mcp/output.ts @@ -1,3 +1,5 @@ +import { isDeepStrictEqual } from 'node:util'; + import type { ContentPart } from '#/kosong/contract/message'; import type { ITelemetryService } from '#/app/telemetry/telemetry'; import type { ExecutableToolResult } from '#/tool/toolContract'; @@ -115,13 +117,18 @@ export async function mcpResultToExecutableOutput( } const wrapped = wrapMediaOnly(converted, qualifiedToolName); - const hasUsableContent = converted.some((part) => - part.type === 'text' - ? part.text.trim().length > 0 && !part.text.startsWith('[MCP content dropped:') - : true, - ); + const hasStructuredCopy = + result.structuredContent !== undefined && + converted.some((part) => { + if (part.type !== 'text') return false; + try { + return isDeepStrictEqual(parseComparableJson(part.text), result.structuredContent); + } catch { + return false; + } + }); const structuredExtras: Record = {}; - if (result.structuredContent !== undefined && !hasUsableContent) { + if (result.structuredContent !== undefined && !hasStructuredCopy) { structuredExtras['structuredContent'] = result.structuredContent; } if (result._meta !== undefined) { @@ -168,9 +175,18 @@ export async function mcpResultToExecutableOutput( return result.isError ? { ...base, isError: true } : base; } +function parseComparableJson(text: string): unknown { + return JSON.parse(text, (_key: string, value: unknown, context?: { source?: string }) => { + if (typeof value === 'number' && context?.source !== JSON.stringify(value)) { + throw new Error('JSON number cannot be compared without normalization'); + } + return value; + }); +} + function serializeStructuredExtras(extras: Record): string | undefined { try { - return JSON.stringify(extras).replaceAll('', ''); + return JSON.stringify(extras).replaceAll('<', '\\u003c'); } catch { return undefined; } diff --git a/packages/agent-core-v2/src/app/config/config.ts b/packages/agent-core-v2/src/app/config/config.ts index f668acebc..626bf7e4c 100644 --- a/packages/agent-core-v2/src/app/config/config.ts +++ b/packages/agent-core-v2/src/app/config/config.ts @@ -24,6 +24,8 @@ export interface ConfigKeyDeprecation { readonly message?: string; } +export type ConfigCollectDiagnostics = (rawSection: unknown) => readonly ConfigDiagnostic[]; + export type EnvBindings = EnvBinding | { [K in keyof T]?: EnvBinding | EnvBindings }; export type AnyEnvBindings = EnvBinding | { readonly [key: string]: EnvBinding | AnyEnvBindings }; @@ -93,6 +95,7 @@ export interface ConfigSection { readonly fromToml?: ConfigFromToml; readonly toToml?: ConfigToToml; readonly deprecations?: readonly ConfigKeyDeprecation[]; + readonly collectDiagnostics?: ConfigCollectDiagnostics; } export interface RegisterSectionOptions { @@ -104,6 +107,7 @@ export interface RegisterSectionOptions { readonly fromToml?: ConfigFromToml; readonly toToml?: ConfigToToml; readonly deprecations?: readonly ConfigKeyDeprecation[]; + readonly collectDiagnostics?: ConfigCollectDiagnostics; } export interface ConfigEffectiveOverlay { diff --git a/packages/agent-core-v2/src/app/config/configService.ts b/packages/agent-core-v2/src/app/config/configService.ts index 0cf755d38..c5f6ce977 100644 --- a/packages/agent-core-v2/src/app/config/configService.ts +++ b/packages/agent-core-v2/src/app/config/configService.ts @@ -145,7 +145,8 @@ function isSameSection( existing.fromToml === options.fromToml && existing.toToml === options.toToml && deepEqual(existing.defaultValue, options.defaultValue) && - deepEqual(existing.deprecations, options.deprecations) + deepEqual(existing.deprecations, options.deprecations) && + existing.collectDiagnostics === options.collectDiagnostics ); } @@ -243,6 +244,7 @@ export class ConfigRegistry extends Disposable implements IConfigRegistry { fromToml: options.fromToml, toToml: options.toToml, deprecations: options.deprecations, + collectDiagnostics: options.collectDiagnostics, }); this._onDidRegisterSection.fire({ domain }); } @@ -564,6 +566,13 @@ export class ConfigService extends Disposable implements IConfigService { for (const diagnostic of collectKeyDeprecations(nextRawSnake, this.registry.listSections())) { this.pushDiagnostic(diagnostic); } + for (const section of this.registry.listSections()) { + if (section.collectDiagnostics === undefined) continue; + const rawSection = nextRawSnake[camelToSnake(section.domain)]; + for (const diagnostic of section.collectDiagnostics(rawSection)) { + this.pushDiagnostic(diagnostic); + } + } if (source !== 'load' && JSON.stringify(nextRawSnake) === JSON.stringify(this.rawSnake)) { const scratch = { ...this.validated }; this.applySectionEnvBindings(scratch, true); diff --git a/packages/agent-core-v2/src/app/kosongConfig/configSection.ts b/packages/agent-core-v2/src/app/kosongConfig/configSection.ts index 9c256a257..ede868e7c 100644 --- a/packages/agent-core-v2/src/app/kosongConfig/configSection.ts +++ b/packages/agent-core-v2/src/app/kosongConfig/configSection.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; import { + type ConfigDiagnostic, type ConfigStripEnv, envBindings, } from '#/app/config/config'; @@ -200,6 +201,54 @@ type _AssertModelsSection = AssertExact< Equal, ModelsSection> >; +const MODEL_OBJECT_FIELDS = new Set( + Object.entries(ModelRecordSchema.shape) + .filter(([, field]) => unwrapWrapperSchema(field as z.ZodTypeAny) instanceof z.ZodObject) + .map(([key]) => camelToSnake(key)), +); + +function unwrapWrapperSchema(schema: z.ZodTypeAny): z.ZodTypeAny { + let current = schema; + while ( + current instanceof z.ZodOptional || + current instanceof z.ZodNullable || + current instanceof z.ZodDefault + ) { + current = current.unwrap() as z.ZodTypeAny; + } + return current; +} + +function collectMalformedModelEntries(rawModels: unknown): ConfigDiagnostic[] { + if (!isPlainObject(rawModels)) return []; + const diagnostics: ConfigDiagnostic[] = []; + for (const [alias, entry] of Object.entries(rawModels)) { + if (!isPlainObject(entry)) continue; + if (entry['model'] !== undefined || entry['name'] !== undefined) continue; + diagnostics.push({ + domain: MODELS_SECTION, + severity: 'warning', + message: malformedModelMessage(alias, entry), + }); + } + return diagnostics; +} + +function malformedModelMessage(alias: string, entry: Record): string { + const base = `[models] entry '${alias}' is missing the 'model' field and cannot be used as a model`; + const dottedAlias = dottedAliasSuffix(alias, entry); + if (dottedAlias === undefined) return `${base}.`; + return `${base}; if the alias contains dots, quote the table name (e.g. [models."${dottedAlias}"]).`; +} + +function dottedAliasSuffix(alias: string, entry: Record): string | undefined { + for (const [key, value] of Object.entries(entry)) { + if (MODEL_OBJECT_FIELDS.has(key) || !isPlainObject(value)) continue; + return dottedAliasSuffix(`${alias}.${key}`, value) ?? `${alias}.${key}`; + } + return undefined; +} + export const modelsFromToml = (rawSnake: unknown): unknown => { if (!isPlainObject(rawSnake)) return rawSnake; const out: Record = {}; @@ -260,6 +309,7 @@ registerConfigSection(MODELS_SECTION, ModelsSectionSchema, { defaultValue: {}, fromToml: modelsFromToml, toToml: modelsToToml, + collectDiagnostics: collectMalformedModelEntries, }); registerConfigSection(LAST_USED_MODEL_SECTION, z.string().optional()); diff --git a/packages/agent-core-v2/test/agent/mcp/output.test.ts b/packages/agent-core-v2/test/agent/mcp/output.test.ts index 77f764311..d64dd2500 100644 --- a/packages/agent-core-v2/test/agent/mcp/output.test.ts +++ b/packages/agent-core-v2/test/agent/mcp/output.test.ts @@ -17,6 +17,16 @@ function isPromiseLike(value: ToolExecution | Promise): value is return typeof (value as Promise).then === 'function'; } +function parseResultExtras(output: string | ContentPart[]): Record { + const text = + typeof output === 'string' + ? output + : output.map((part) => (part.type === 'text' ? part.text : '')).join('\n'); + const json = /\n([\s\S]*?)\n<\/mcp-result-extras>/.exec(text)?.[1]; + if (json === undefined) throw new Error('Expected model-visible MCP result extras'); + return JSON.parse(json) as Record; +} + function assertValidMcpBlock(block: T): T { const parsed = ContentBlockSchema.safeParse(block); if (!parsed.success) { @@ -292,7 +302,7 @@ describe('mcpResultToExecutableOutput', () => { expect(out).toEqual({ output: 'oops', isError: true }); }); - test('prefers usable content over structuredContent while still surfacing _meta', async () => { + test('preserves structured records alongside a prose summary and _meta', async () => { const out = await mcpResultToExecutableOutput( { content: [{ type: 'text', text: 'ok' }], @@ -305,12 +315,85 @@ describe('mcpResultToExecutableOutput', () => { const parts = out.output as ContentPart[]; const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); expect(joined).toContain(''); - expect(joined).not.toContain('"structuredContent"'); - expect(joined).toContain('"_meta":{"bar":2}'); + expect(parseResultExtras(out.output)).toEqual({ + structuredContent: { foo: 1 }, + _meta: { bar: 2 }, + }); expect(out.isError).toBeUndefined(); }); - test('prefers media content over structuredContent', async () => { + test('drops the structured copy only when a text block repeats it verbatim', async () => { + const out = await mcpResultToExecutableOutput( + { + content: [{ type: 'text', text: '{"foo":1}' }], + isError: false, + structuredContent: { foo: 1 }, + }, + 'mcp__s__t', + ); + expect(out.output).toBe('{"foo":1}'); + }); + + test('preserves both values when parsing the text would round a number', async () => { + const text = '{"id":9007199254740993}'; + const out = await mcpResultToExecutableOutput( + { content: [{ type: 'text', text }], isError: false, structuredContent: { id: 9007199254740992 } }, + 'mcp__s__t', + ); + + expect(out.output).toContainEqual({ type: 'text', text }); + expect(parseResultExtras(out.output)['structuredContent']).toEqual({ id: 9007199254740992 }); + }); + + test.each([ + { difference: 'value', text: '{"count":1}', structuredContent: { count: 2 } }, + { difference: 'type', text: '{"id":"1"}', structuredContent: { id: 1 } }, + { difference: 'array order', text: '{"ids":[2,1]}', structuredContent: { ids: [1, 2] } }, + { difference: 'extra field', text: '{"id":1}', structuredContent: { id: 1, name: 'Example' } }, + { difference: 'null', text: '{"value":"none"}', structuredContent: { value: null } }, + { difference: 'non-JSON wrapper', text: '```json\n{"id":1}\n```', structuredContent: { id: 1 } }, + ])('keeps text and structured data with a $difference difference', async ({ text, structuredContent }) => { + const out = await mcpResultToExecutableOutput( + { content: [{ type: 'text', text }], isError: false, structuredContent }, + 'mcp__s__t', + ); + + expect(out.output).toContainEqual({ type: 'text', text }); + expect(parseResultExtras(out.output)['structuredContent']).toEqual(structuredContent); + }); + + test('keeps explanatory blocks when another text block contains the complete JSON', async () => { + const content = [ + { type: 'text', text: 'Found 1 row.' }, + { type: 'text', text: '{"rows":[1]}' }, + { type: 'text', text: 'More rows are available.' }, + ] as const; + const out = await mcpResultToExecutableOutput( + { content: [...content], isError: false, structuredContent: { rows: [1] } }, + 'mcp__s__t', + ); + + expect(out.output).toEqual([...content]); + }); + + test('keeps structured error details and the tool error status', async () => { + const out = await mcpResultToExecutableOutput( + { + content: [{ type: 'text', text: 'Request failed.' }], + isError: true, + structuredContent: { code: 'EXAMPLE_ERROR', retryable: false }, + }, + 'mcp__s__t', + ); + + expect(out.isError).toBe(true); + expect(parseResultExtras(out.output)['structuredContent']).toEqual({ + code: 'EXAMPLE_ERROR', + retryable: false, + }); + }); + + test('keeps media and its structured data together', async () => { const out = await mcpResultToExecutableOutput( { content: [{ type: 'image', data: 'AAA', mimeType: 'image/png' }], @@ -321,8 +404,9 @@ describe('mcpResultToExecutableOutput', () => { ); const parts = out.output as ContentPart[]; expect(parts[0]).toEqual({ type: 'text', text: '' }); - expect(parts.at(-1)).toEqual({ type: 'text', text: '' }); - expect(parts.some((part) => part.type === 'text' && part.text.includes('structuredContent'))).toBe(false); + expect(parts).toContainEqual({ type: 'text', text: '' }); + expect(parts.some((part) => part.type === 'image_url')).toBe(true); + expect(parseResultExtras(out.output)['structuredContent']).toEqual({ foo: 1 }); }); test('uses structuredContent when content has only whitespace', async () => { @@ -362,18 +446,22 @@ describe('mcpResultToExecutableOutput', () => { expect(joined).toContain('"structuredContent":{"answer":42}'); }); - test('strips literal closing tags inside the structured payload', async () => { + test('escapes literal closing tags without changing structured values', async () => { const out = await mcpResultToExecutableOutput( { content: [{ type: 'text', text: 'ok' }], isError: false, + structuredContent: { text: 'ab' }, _meta: { evil: 'ab' }, }, 'mcp__s__t', ); const parts = out.output as ContentPart[]; const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); - expect(joined).toContain('"evil":"ab"'); + expect(parseResultExtras(out.output)).toEqual({ + structuredContent: { text: 'ab' }, + _meta: { evil: 'ab' }, + }); expect(joined.split('')).toHaveLength(2); }); diff --git a/packages/agent-core-v2/test/app/config/config.test.ts b/packages/agent-core-v2/test/app/config/config.test.ts index 54e807e72..8236b20b4 100644 --- a/packages/agent-core-v2/test/app/config/config.test.ts +++ b/packages/agent-core-v2/test/app/config/config.test.ts @@ -1354,6 +1354,98 @@ describe('config deprecations', () => { }); }); +describe('malformed models config entries', () => { + async function createConfig(toml: string) { + const disposables = new DisposableStore(); + const ix = disposables.add(new TestInstantiationService()); + const storage = new InMemoryStorageService(); + await storage.write('', 'config.toml', new TextEncoder().encode(toml)); + ix.stub(ILogService, stubLog()); + ix.stub(IBootstrapService, stubBootstrap('/tmp/pythinker-cfg', {})); + ix.stub(IFileSystemStorageService, storage); + ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); + ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); + ix.set(IConfigService, new SyncDescriptor(ConfigService)); + const config = ix.get(IConfigService); + await config.ready; + return { config, disposables, storage }; + } + + it('warns at load time when a dotted alias parses as a nested table', async () => { + const { config, disposables } = await createConfig( + '[models.acme-m1.5-code]\nmodel = "acme-m1.5-code"\nmax_context_size = 262144\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'acme-m1' is missing the 'model' field and cannot be used as a model; " + + 'if the alias contains dots, quote the table name (e.g. [models."acme-m1.5-code"]).', + }); + + disposables.dispose(); + }); + + it('stays silent for quoted dotted aliases and entries with a wire-facing name', async () => { + const { config, disposables } = await createConfig( + '[models."acme-m1.5-code"]\nmodel = "acme-m1.5-code"\n\n[models.renamed]\nname = "wire-name"\n', + ); + + expect(config.diagnostics()).toEqual([]); + + disposables.dispose(); + }); + + it('warns without the dotted-alias hint when the entry has no nested table', async () => { + const { config, disposables } = await createConfig( + '[models.partial]\nmax_context_size = 262144\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'partial' is missing the 'model' field and cannot be used as a model.", + }); + + disposables.dispose(); + }); + + it('does not mistake schema object fields for a dotted alias', async () => { + const { config, disposables } = await createConfig( + '[models.partial]\noverrides = { max_output_size = 8192 }\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'partial' is missing the 'model' field and cannot be used as a model.", + }); + + disposables.dispose(); + }); + + it('clears the warning on reload once the entry is fixed', async () => { + const { config, disposables, storage } = await createConfig( + '[models.acme-m1.5-code]\nmodel = "acme-m1.5-code"\n', + ); + expect(config.diagnostics()).toHaveLength(1); + + await storage.write( + '', + 'config.toml', + new TextEncoder().encode('[models."acme-m1.5-code"]\nmodel = "acme-m1.5-code"\n'), + ); + await config.reload(); + + expect(config.diagnostics()).toEqual([]); + + disposables.dispose(); + }); +}); + describe('task config section', () => { it('re-applies the keepAliveOnExit env binding on every get()', async () => { const env: Record = {}; diff --git a/packages/agent-core-v2/test/setup.ts b/packages/agent-core-v2/test/setup.ts index 3cdd7e8f8..3670bff15 100644 --- a/packages/agent-core-v2/test/setup.ts +++ b/packages/agent-core-v2/test/setup.ts @@ -1,5 +1,5 @@ for (const key of Object.keys(process.env)) { - if (key.startsWith('PYTHINKER_CODE_EXPERIMENTAL_')) { + if (key.startsWith('PYTHINKER_CODE_')) { delete process.env[key]; } } diff --git a/packages/agent-gateway/test/setup.ts b/packages/agent-gateway/test/setup.ts index e93131d5b..4aa3ef892 100644 --- a/packages/agent-gateway/test/setup.ts +++ b/packages/agent-gateway/test/setup.ts @@ -1,6 +1,5 @@ -delete process.env['PYTHINKER_CODE_EXPERIMENTAL_FLAG']; for (const key of Object.keys(process.env)) { - if (key.startsWith('PYTHINKER_CODE_EXPERIMENTAL_')) { + if (key.startsWith('PYTHINKER_CODE_')) { delete process.env[key]; } } diff --git a/packages/klient/vitest.config.ts b/packages/klient/vitest.config.ts index b5a71f285..dfc575c87 100644 --- a/packages/klient/vitest.config.ts +++ b/packages/klient/vitest.config.ts @@ -4,6 +4,7 @@ export default defineConfig({ test: { name: 'klient', include: ['test/**/*.test.ts'], + testTimeout: 15_000, reporters: ['default', './test/e2e/legacy/report/vitest-reporter.ts'], }, }); diff --git a/packages/node-sdk/vitest.config.ts b/packages/node-sdk/vitest.config.ts index 38a3f587a..7ca8fdbf1 100644 --- a/packages/node-sdk/vitest.config.ts +++ b/packages/node-sdk/vitest.config.ts @@ -17,5 +17,6 @@ export default defineConfig({ PYTHINKER_LOG_LEVEL: 'off', }, include: ['test/**/*.test.ts'], + testTimeout: 15_000, }, }); From 2d4d27ab0529abae8d8768152d810c54cccae809 Mon Sep 17 00:00:00 2001 From: elkaix Date: Wed, 9 Sep 2026 19:16:39 -0400 Subject: [PATCH 02/12] feat: allow read-only tools in side questions and clarify subagent models Side questions started with /btw can now call Read, Grep and Glob so answers can rest on current file contents; every write or execute tool stays rejected. The subagent model list now always names what "primary" is bound to, drops the duplicate main-model marker, states that pool entries do not inherit the caller's thinking level, and the dynamic-workflow tool renders the pool as a one-line summary instead of repeating the block. --- .../src/agent/tools/agent/agent.ts | 2 +- .../agent-core-v2/src/features/btw/btw.ts | 17 +++--- .../src/features/btw/btwService.ts | 13 ++++- .../agent-dynamic_workflow.ts | 2 +- .../agentDynamicWorkflowTool.ts | 8 +-- .../src/session/subagent/configSection.ts | 48 ++++++++++------- .../test/features/btw/btw.test.ts | 54 +++++++++++++------ .../dynamic_workflow/dynamic_workflow.test.ts | 7 ++- packages/agent-core-v2/test/tool/tool.test.ts | 27 +++++----- 9 files changed, 109 insertions(+), 69 deletions(-) diff --git a/packages/agent-core-v2/src/agent/tools/agent/agent.ts b/packages/agent-core-v2/src/agent/tools/agent/agent.ts index 76fc5d132..77da6c2ae 100644 --- a/packages/agent-core-v2/src/agent/tools/agent/agent.ts +++ b/packages/agent-core-v2/src/agent/tools/agent/agent.ts @@ -57,7 +57,7 @@ export const SubagentToolInputSchema = z.preprocess( .string() .optional() .describe( - 'Which model to run the subagent on: one of the aliases listed under "Available models" in this tool description, or "primary" for the main model you are running on (for hard, quality-sensitive tasks). When omitted, the configured default model is used. Ignored when resuming — resumed subagents keep their own model.', + 'Which model to run the subagent on: one of the aliases listed under "Available models" in this tool description, or "primary" for your current model and thinking level. When omitted, the configured default model is used. Ignored when resuming — resumed subagents keep their own model.', ), }), ); diff --git a/packages/agent-core-v2/src/features/btw/btw.ts b/packages/agent-core-v2/src/features/btw/btw.ts index 00a09e751..7fa91fc3f 100644 --- a/packages/agent-core-v2/src/features/btw/btw.ts +++ b/packages/agent-core-v2/src/features/btw/btw.ts @@ -1,19 +1,22 @@ import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation'; +export const BTW_READONLY_TOOLS = new Set(['Read', 'Grep', 'Glob']); + export const TOOL_CALL_DISABLED_MESSAGE = - 'Tool calls are disabled for side questions. Answer with text only.'; + 'Only the read-only tools Read, Grep, and Glob are available for side questions. Other tool calls are disabled.'; export const SIDE_QUESTION_SYSTEM_REMINDER = ` -This is a side-channel conversation with the user. You should answer user questions directly based on what you already know. +This is a side-channel conversation with the user. You should answer user questions directly. IMPORTANT: - You are a separate, lightweight instance. - The main agent continues independently; do not reference being interrupted. -- Do not call any tools. All tool calls are disabled and will be rejected. - Even though tool definitions are visible in this request, they exist only - for technical reasons (prompt cache). You must not use them. -- Respond only with text based on what you already know from the conversation - and this side-channel conversation. +- You may use the read-only tools Read, Grep, and Glob to inspect files when + the answer depends on current file contents. All other tools are disabled + and will be rejected, even though their definitions are visible in this + request (they exist only for technical reasons — prompt cache). +- Prefer answering from what you already know from the conversation and this + side-channel conversation; reach for the read-only tools only when needed. - Follow-up turns may happen in this side-channel conversation. - If you do not know the answer, say so directly. `.trim(); diff --git a/packages/agent-core-v2/src/features/btw/btwService.ts b/packages/agent-core-v2/src/features/btw/btwService.ts index d91901d1a..4e17ec957 100644 --- a/packages/agent-core-v2/src/features/btw/btwService.ts +++ b/packages/agent-core-v2/src/features/btw/btwService.ts @@ -6,7 +6,12 @@ import { IAgentScopeContext } from '#/agent/scopeContext/scopeContext'; import { ErrorCodes, Error2 } from '#/errors'; import { IAgentLifecycleService, MAIN_AGENT_ID } from '#/session/agentLifecycle/agentLifecycle'; -import { ISessionBtwService, SIDE_QUESTION_SYSTEM_REMINDER, TOOL_CALL_DISABLED_MESSAGE } from './btw'; +import { + BTW_READONLY_TOOLS, + ISessionBtwService, + SIDE_QUESTION_SYSTEM_REMINDER, + TOOL_CALL_DISABLED_MESSAGE, +} from './btw'; export class SessionBtwService implements ISessionBtwService { declare readonly _serviceBrand: undefined; @@ -32,7 +37,11 @@ export class SessionBtwService implements ISessionBtwService { child.accessor .get(IAgentToolExecutorService) ?.onBeforeExecuteTool((event) => { - event.veto(denyToolExecution(reason)); + if (!BTW_READONLY_TOOLS.has(event.toolCall.name)) { + if (!BTW_READONLY_TOOLS.has(event.toolCall.name)) { + event.veto(denyToolExecution(reason)); + } + } }); return childContext.agentId; } diff --git a/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agent-dynamic_workflow.ts b/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agent-dynamic_workflow.ts index 20ea8ee5d..17065af4a 100644 --- a/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agent-dynamic_workflow.ts +++ b/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agent-dynamic_workflow.ts @@ -77,7 +77,7 @@ export const AgentDynamicWorkflowToolInputSchema = z .string() .optional() .describe( - 'Which model to run the item-spawned subagents on: one of the aliases listed under "Available models" in this tool description, or "primary" for the main model you are running on (for hard, quality-sensitive tasks). When omitted, the configured default model is used. Resumed subagents always keep their own model.', + 'Which model to run the item-spawned subagents on: one of the aliases listed under "Available models" in this tool description, or "primary" for your current model and thinking level. When omitted, the configured default model is used. Resumed subagents always keep their own model.', ), }) .strict() diff --git a/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agentDynamicWorkflowTool.ts b/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agentDynamicWorkflowTool.ts index fa9375aba..e4b36acb2 100644 --- a/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agentDynamicWorkflowTool.ts +++ b/packages/agent-core-v2/src/features/dynamic_workflow/tools/agent-dynamic_workflow/agentDynamicWorkflowTool.ts @@ -23,7 +23,7 @@ import { } from '#/session/subagent/spawn'; import { SUBAGENT_FORK_FLAG_ID } from '#/session/subagent/flag'; import { - buildSubagentModelDescriptions, + buildSubagentModelSummary, exposesSubagentModelChoice, stripSubagentForkParameter, stripSubagentModelParameter, @@ -107,11 +107,7 @@ export class AgentDynamicWorkflowTool implements IAgentDynamicWorkflowTool { if (this.flags.enabled(SUBAGENT_FORK_FLAG_ID)) { description += `\n\n${AGENT_DYNAMIC_WORKFLOW_FORK_DESCRIPTION}`; } - const modelLines = buildSubagentModelDescriptions( - this.config, - this.flags, - this.profile.data().modelAlias, - ); + const modelLines = buildSubagentModelSummary(this.config, this.flags); return modelLines === undefined ? description : `${description}\n\n${modelLines}`; } diff --git a/packages/agent-core-v2/src/session/subagent/configSection.ts b/packages/agent-core-v2/src/session/subagent/configSection.ts index 636c5280b..461080a20 100644 --- a/packages/agent-core-v2/src/session/subagent/configSection.ts +++ b/packages/agent-core-v2/src/session/subagent/configSection.ts @@ -221,29 +221,39 @@ export function buildSubagentModelDescriptions( const pool = resolveSubagentModelPool(config)!; const lines = ['Available models (pass via model):']; const defaultModel = pool.defaultModel; - const markersFor = (alias: string): string => { - const markers: string[] = []; - if (alias === defaultModel) markers.push('[default]'); - if (alias === callerModelAlias) markers.push('[main model]'); - return markers.length === 0 ? '' : ` ${markers.join(' ')}`; - }; - if (defaultModel !== undefined && Object.hasOwn(pool.models, defaultModel)) { - lines.push( - formatPoolLine(`${defaultModel}${markersFor(defaultModel)}`, pool.models[defaultModel]!), - ); - } - for (const [alias, description] of Object.entries(pool.models)) { - if (alias === defaultModel) continue; - lines.push(formatPoolLine(`${alias}${markersFor(alias)}`, description)); + for (const alias of orderedPoolAliases(pool)) { + const marker = alias === defaultModel ? ' [default]' : ''; + lines.push(formatPoolLine(`${alias}${marker}`, pool.models[alias]!)); } - const callerInPool = - callerModelAlias !== undefined && Object.hasOwn(pool.models, callerModelAlias); - lines.push( - `- ${PRIMARY_SUBAGENT_MODEL_CHOICE}${callerInPool ? ` (${callerModelAlias})` : ''}: the main model you are running on, bound with your current thinking level; use it for hard, quality-sensitive subagent tasks`, - ); + const primaryLabel = + callerModelAlias === undefined + ? PRIMARY_SUBAGENT_MODEL_CHOICE + : `${PRIMARY_SUBAGENT_MODEL_CHOICE} (= ${callerModelAlias})`; + lines.push(`- ${primaryLabel}: your current model and thinking level`); + lines.push("Pool entries don't inherit your thinking level."); return lines.join('\n'); } +export function buildSubagentModelSummary( + config: IConfigService, + flags: IFlagService, +): string | undefined { + if (!exposesSubagentModelChoice(config, flags)) return undefined; + const pool = resolveSubagentModelPool(config)!; + const labels = orderedPoolAliases(pool).map((alias) => + alias === pool.defaultModel ? `${alias} [default]` : alias, + ); + labels.push(`${PRIMARY_SUBAGENT_MODEL_CHOICE} (your current model and thinking level)`); + return `Available models (pass via model): ${labels.join(', ')}.`; +} + +function orderedPoolAliases(pool: SubagentModelPool): string[] { + const aliases = Object.keys(pool.models); + const defaultModel = pool.defaultModel; + if (defaultModel === undefined || !Object.hasOwn(pool.models, defaultModel)) return aliases; + return [defaultModel, ...aliases.filter((alias) => alias !== defaultModel)]; +} + function formatPoolLine(label: string, description: string): string { return description === '' ? `- ${label}` : `- ${label}: ${description}`; } diff --git a/packages/agent-core-v2/test/features/btw/btw.test.ts b/packages/agent-core-v2/test/features/btw/btw.test.ts index 10551e824..748c3d3e2 100644 --- a/packages/agent-core-v2/test/features/btw/btw.test.ts +++ b/packages/agent-core-v2/test/features/btw/btw.test.ts @@ -7,6 +7,7 @@ import { TestInstantiationService } from '#/_base/di/test'; import { IAgentToolApprovalService } from '#/agent/toolApproval/toolApproval'; import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor'; import { + BTW_READONLY_TOOLS, ISessionBtwService, SIDE_QUESTION_SYSTEM_REMINDER, TOOL_CALL_DISABLED_MESSAGE, @@ -85,26 +86,47 @@ describe('SessionBtwService', () => { }); }); - it('vetoes every tool call on the child through the btw deny listener', async () => { + it('vetoes non-read-only tool calls on the child through the btw deny listener', async () => { const svc = ix.get(ISessionBtwService); await svc.start(); - const toolCall: ToolCall = { type: 'function', id: 'call_1', name: 'Bash', arguments: '{}' }; - const decision = await executorEvents.fireBeforeExecute({ - turnId: 0, - signal: new AbortController().signal, - toolCall, - toolCalls: [toolCall], - args: {}, - execution: { approvalRule: 'Bash', execute: async () => ({ output: '' }) }, - }); + for (const name of ['Bash', 'Write', 'Edit']) { + const toolCall: ToolCall = { type: 'function', id: `call_${name}`, name, arguments: '{}' }; + const decision = await executorEvents.fireBeforeExecute({ + turnId: 0, + signal: new AbortController().signal, + toolCall, + toolCalls: [toolCall], + args: {}, + execution: { approvalRule: name, execute: async () => ({ output: '' }) }, + }); - expect(decision).toEqual({ - veto: { - output: `${TOOL_CALL_DISABLED_MESSAGE} [worker guidance]`, - isError: true, - }, - }); + expect(decision).toEqual({ + veto: { + output: `${TOOL_CALL_DISABLED_MESSAGE} [worker guidance]`, + isError: true, + }, + }); + } expect(formatDenyMessage).toHaveBeenCalledWith(TOOL_CALL_DISABLED_MESSAGE); }); + + it('allows read-only tool calls (Read, Grep, Glob) on the child', async () => { + const svc = ix.get(ISessionBtwService); + await svc.start(); + + for (const name of BTW_READONLY_TOOLS) { + const toolCall: ToolCall = { type: 'function', id: `call_${name}`, name, arguments: '{}' }; + const decision = await executorEvents.fireBeforeExecute({ + turnId: 0, + signal: new AbortController().signal, + toolCall, + toolCalls: [toolCall], + args: {}, + execution: { approvalRule: name, execute: async () => ({ output: '' }) }, + }); + + expect(decision).toBeUndefined(); + } + }); }); diff --git a/packages/agent-core-v2/test/features/dynamic_workflow/dynamic_workflow.test.ts b/packages/agent-core-v2/test/features/dynamic_workflow/dynamic_workflow.test.ts index e862ef18d..c42a98d6c 100644 --- a/packages/agent-core-v2/test/features/dynamic_workflow/dynamic_workflow.test.ts +++ b/packages/agent-core-v2/test/features/dynamic_workflow/dynamic_workflow.test.ts @@ -1418,10 +1418,9 @@ describe('AgentDynamicWorkflowTool', () => { const host = mockDynamicWorkflowHost(); const configured = new AgentDynamicWorkflowTool(host.dynamicWorkflowService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockDynamicWorkflowMode(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), realSubagents(stubDynamicWorkflowCatalog(), stubConfig({ defaultModel: 'provider/fast', models: { 'provider/fast': 'fast and cheap', 'main-model': 'the main model' } }), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); - expect(configured.description).toContain('Available models'); - expect(configured.description).toContain('- provider/fast [default]: fast and cheap'); - expect(configured.description).toContain('- main-model [main model]: the main model'); - expect(configured.description).toContain('- primary (main-model)'); + expect(configured.description).toContain( + 'Available models (pass via model): provider/fast [default], main-model, primary (your current model and thinking level).', + ); const unconfigured = new AgentDynamicWorkflowTool(host.dynamicWorkflowService, makeAgentScopeContext({ agentId: host.callerAgentId, agentScope: '' }), mockDynamicWorkflowMode(), stubConfig(), stubFlag(true), realSubagents(stubDynamicWorkflowCatalog(), stubConfig(), stubFlag(true), stubCallerProfile({ modelAlias: 'main-model' })), stubCallerProfile({ modelAlias: 'main-model' })); diff --git a/packages/agent-core-v2/test/tool/tool.test.ts b/packages/agent-core-v2/test/tool/tool.test.ts index 7fe43f6e4..eef4ff3ad 100644 --- a/packages/agent-core-v2/test/tool/tool.test.ts +++ b/packages/agent-core-v2/test/tool/tool.test.ts @@ -1080,13 +1080,15 @@ describe('Agent tool description', () => { expect(description).toContain('Available models'); const defaultIndex = description.indexOf('- provider/fast [default]: fast and cheap'); const smartIndex = description.indexOf('- provider/smart: hard tasks'); - const primaryIndex = description.indexOf('- primary:'); + const primaryIndex = description.indexOf( + '- primary (= mock-model): your current model and thinking level\n', + ); expect(defaultIndex).toBeGreaterThanOrEqual(0); expect(smartIndex).toBeGreaterThan(defaultIndex); expect(primaryIndex).toBeGreaterThan(smartIndex); }); - it('lists the caller-in-pool alias with a [main model] marker and renders empty descriptions bare', () => { + it('renders the caller-in-pool alias as a plain entry and renders empty descriptions bare', () => { ctx = createTestAgent(secondaryModelFlags(), { initialConfig: { secondaryModel: { @@ -1104,12 +1106,12 @@ describe('Agent tool description', () => { const description = agentDescription(); expect(description).toContain('- provider/fast [default]: fast and cheap'); - expect(description).toContain('- mock-model [main model]: the main model, great at hard things'); + expect(description).toContain('- mock-model: the main model, great at hard things'); expect(description).toContain('- provider/smart\n'); - expect(description).toContain('- primary (mock-model)'); + expect(description).toContain('- primary (= mock-model): your current model and thinking level\n'); }); - it('marks the caller-as-default alias with both [default] and [main model]', () => { + it('marks the default alias with [default]', () => { ctx = createTestAgent(secondaryModelFlags(), { initialConfig: { secondaryModel: { @@ -1126,12 +1128,12 @@ describe('Agent tool description', () => { const description = agentDescription(); const defaultIndex = description.indexOf( - '- mock-model [default] [main model]: the main model, great at hard things', + '- mock-model [default]: the main model, great at hard things', ); const fastIndex = description.indexOf('- provider/fast: fast and cheap'); expect(defaultIndex).toBeGreaterThanOrEqual(0); expect(fastIndex).toBeGreaterThan(defaultIndex); - expect(description).toContain('- primary (mock-model)'); + expect(description).toContain('- primary (= mock-model): your current model and thinking level\n'); }); function agentParameters(): Record { @@ -1198,7 +1200,7 @@ describe('Agent tool description', () => { const description = agentDescription(); expect(description).toContain('- provider/fast [default]\n'); - expect(description).toContain('- primary:'); + expect(description).toContain('- primary (= mock-model): your current model and thinking level\n'); }); it('hides the model parameter and the pool description when force is set', () => { @@ -3093,7 +3095,7 @@ describe('AgentDynamicWorkflow tool description', () => { expect(agentDynamicWorkflowDescription()).not.toContain('Available models'); }); - it('renders the configured pool with the default marker and a generic primary line', () => { + it('renders the configured pool as a compact one-line summary', () => { ctx = createTestAgent(secondaryModelFlags(), { initialConfig: { secondaryModel: { @@ -3106,10 +3108,9 @@ describe('AgentDynamicWorkflow tool description', () => { const description = agentDynamicWorkflowDescription(); - expect(description).toContain('Available models'); - expect(description).toContain('- provider/fast [default]: fast and cheap'); - expect(description).toContain('- provider/smart: hard tasks'); - expect(description).toContain('- primary:'); + expect(description).toContain( + 'Available models (pass via model): provider/fast [default], provider/smart, primary (your current model and thinking level).', + ); }); function agentDynamicWorkflowParameters(): Record { From f12fdabac70fa9a5ab0b88b84e171474ba779858 Mon Sep 17 00:00:00 2001 From: elkaix Date: Wed, 9 Sep 2026 19:21:52 -0400 Subject: [PATCH 03/12] feat: page through Glob matches beyond the first hundred Glob now takes offset and head_limit, defaults to 100 matches, and accepts head_limit=0 to lift the match-count limit. Every page stays within the output character limit, ends on a complete path, reports the range it covers, and gives the next offset when more matches remain. The tool card counts only paths, and marks a page that is not the last. --- .../messages/tool-renderers/chip.ts | 20 ++- .../messages/tool-renderers/chip.test.ts | 35 +++++ docs/reference/tools.md | 4 +- .../src/agent/tools/os/glob/glob.md | 6 +- .../src/agent/tools/os/glob/glob.ts | 18 ++- .../src/agent/tools/os/glob/globTool.ts | 95 +++++++++---- .../fullCompaction/fullCompaction.test.ts | 7 +- .../test/agent/loop/loop.test.ts | 4 +- .../os/backends/node-local/tools/glob.test.ts | 133 ++++++++++++++++-- packages/agent-core-v2/test/tool/tool.test.ts | 10 +- 10 files changed, 275 insertions(+), 57 deletions(-) diff --git a/apps/pythinker-code/src/tui/components/messages/tool-renderers/chip.ts b/apps/pythinker-code/src/tui/components/messages/tool-renderers/chip.ts index 37536c140..4ab07b1b6 100644 --- a/apps/pythinker-code/src/tui/components/messages/tool-renderers/chip.ts +++ b/apps/pythinker-code/src/tui/components/messages/tool-renderers/chip.ts @@ -93,10 +93,26 @@ const grepChip: ChipProvider = (_toolCall, result) => { return pluralize(matches, 'match', 'matches'); }; +const GLOB_PAGE_HEADER = + /^Showing matches (\d+)\u2013(\d+) of (\d+)( collected matches \(partial result set\))?\.$/; +const GLOB_NOTICE = + /^(?:Continue with the same search arguments and offset=\d+\.|To remove the match-count limit, omit offset and use head_limit=0\.|Character limit reached; only complete paths are returned\.|No more matches at offset=\d+ in the (?:current|collected partial) result set \(\d+ matches\)\.|No matches collected; search incomplete\.)$/; + const globChip: ChipProvider = (_toolCall, result) => { - const files = countNonEmptyLines(result.output); + let partial = false; + let files = 0; + for (const line of result.output.split('\n')) { + if (line.trim().length === 0) continue; + const page = GLOB_PAGE_HEADER.exec(line); + if (page !== null) { + partial = Number(page[2]) < Number(page[3]) || page[4] !== undefined; + continue; + } + if (GLOB_NOTICE.test(line)) continue; + files++; + } if (files === 0) return 'no files'; - return pluralize(files, 'file'); + return `${String(files)}${partial ? '+' : ''} ${files === 1 ? 'file' : 'files'}`; }; const fetchChip: ChipProvider = (_toolCall, result) => diff --git a/apps/pythinker-code/test/tui/components/messages/tool-renderers/chip.test.ts b/apps/pythinker-code/test/tui/components/messages/tool-renderers/chip.test.ts index 25ef9e492..844d084ed 100644 --- a/apps/pythinker-code/test/tui/components/messages/tool-renderers/chip.test.ts +++ b/apps/pythinker-code/test/tui/components/messages/tool-renderers/chip.test.ts @@ -65,6 +65,41 @@ describe('chip registry', () => { expect(chipFor('Glob', { pattern: '**/*.ts' }, result('a.ts\nb.ts'))).toBe('2 files'); }); + it('counts only paths on a Glob page with a continuation notice', () => { + const output = [ + 'Showing matches 1\u2013100 of 347.', + 'Continue with the same search arguments and offset=100.', + 'To remove the match-count limit, omit offset and use head_limit=0.', + ...Array.from({ length: 100 }, (_, i) => `file-${String(i)}.ts`), + ].join('\n'); + expect(chipFor('Glob', {}, result(output))).toBe('100+ files'); + }); + + it.each([ + 'No more matches at offset=347 in the current result set (347 matches).', + 'No matches collected; search incomplete.', + ])('does not count an empty Glob page as a file: %s', (output) => { + expect(chipFor('Glob', {}, result(output))).toBe('no files'); + }); + + it('distinguishes the last Glob page from a partial result set', () => { + expect(chipFor('Glob', {}, result('Showing matches 3\u20134 of 4.\nc.ts\nd.ts'))).toBe('2 files'); + expect( + chipFor( + 'Glob', + {}, + result('Showing matches 3\u20134 of 4 collected matches (partial result set).\nc.ts\nd.ts'), + ), + ).toBe('2+ files'); + }); + + it('keeps notice-like file names and leaves Grep interpretation unchanged', () => { + expect( + chipFor('Glob', {}, result('Showing matches.ts\nContinue with.txt\nNo more matches.ts')), + ).toBe('3 files'); + expect(chipFor('Grep', {}, result('Showing matches 1\u20132 of 3.'))).toBe('1 match'); + }); + it('FetchURL chip shows size and is non-empty', () => { const out = chipFor('FetchURL', { url: 'https://example.com' }, result('hello world')); expect(out).toMatch(/\d+\s*B/); diff --git a/docs/reference/tools.md b/docs/reference/tools.md index eb1f1fbc7..cb967b612 100644 --- a/docs/reference/tools.md +++ b/docs/reference/tools.md @@ -25,7 +25,9 @@ File tools handle reading, writing, and searching the local filesystem — the f **`Grep`** invokes ripgrep to search file contents, supporting regular expressions (`pattern`), a search path (`path`), file type filtering (`type`, e.g., `ts`, `py`), glob filtering (`glob`), and output mode (`output_mode`: `files_with_matches` / `content` / `count_matches`; defaults to `files_with_matches`). `content` mode supports context lines (`-A`, `-B`, `-C`), case-insensitive matching (`-i`), line numbers (`-n`, default true), and multiline matching (`multiline`). All modes support `offset` + `head_limit` pagination; `head_limit` defaults to 250 and `0` means unlimited. Sensitive files such as `.env` files and private keys are automatically filtered out; set `include_ignored=true` to search files ignored by `.gitignore`, though sensitive files remain filtered. -**`Glob`** matches files in a specified directory (`path`; defaults to the working directory) by glob pattern (`pattern`). Results are sorted by modification time in descending order, with a maximum of 100 entries. It respects `.gitignore`, `.ignore`, and `.rgignore` by default; set `include_ignored=true` to include ignored files such as build outputs, while sensitive files remain filtered. Brace patterns such as `*.{ts,tsx}` are supported, and broad wildcard patterns are allowed but usually truncate at the match cap. +**`Glob`** matches files in a specified directory (`path`; defaults to the working directory) by glob pattern (`pattern`). Results are sorted by modification time in descending order, returning 100 entries by default. It respects `.gitignore`, `.ignore`, and `.rgignore` by default; set `include_ignored=true` to include ignored files such as build outputs, while sensitive files remain filtered. Brace patterns such as `*.{ts,tsx}` are supported, and broad wildcard patterns are allowed. + +Use `offset` (default 0) and `head_limit` (default 100) to page through matching paths; the result provides the next offset when more matches are available. Set `head_limit: 0` to remove the match-count limit. The character limit still applies: pages end at a complete path and provide the next offset when necessary. Large pages are saved to a file that the agent can read with `Read`. Each call searches the current filesystem again, so file changes can shift results between pages. Timeouts, unreadable directories, or the output capture limit can still leave the search incomplete; the result warns about these cases, and increasing the offset cannot recover uncollected paths. **`ReadMediaFile`** sends an image or video to the model as multimodal content. It accepts `path`, plus optional image-detail controls such as `region` and `full_resolution`; the file size limit is 100 MB. Default image reads are compressed to the configured model limits. If automatic compression cannot meet those limits safely, the tool returns an error without sending the original image and directs the model to create and read a smaller copy. Availability depends on the current model's vision capabilities (`image_in` / `video_in`). diff --git a/packages/agent-core-v2/src/agent/tools/os/glob/glob.md b/packages/agent-core-v2/src/agent/tools/os/glob/glob.md index ad299e29a..6e364bd91 100644 --- a/packages/agent-core-v2/src/agent/tools/os/glob/glob.md +++ b/packages/agent-core-v2/src/agent/tools/os/glob/glob.md @@ -10,7 +10,9 @@ Good patterns: - `*.{ts,tsx}` — brace expansion is supported - `{src,test}/**/*.ts` — cartesian brace expansion is supported too -Results are capped at the first 100 matching paths. If a search would return more, a truncation marker is appended. Refine the pattern (extension, subdirectory) when 100 is not enough, or call again with a narrower anchor. +Results default to 100 matching paths. Use `offset` (default 0) and `head_limit` (default 100) to page through results. When more matches are available, the result gives the next offset; keep the other search arguments unchanged. Set `head_limit=0` to remove the match-count limit. Pages still stay within the character retention limit, including notices: when it is reached, only complete paths are returned, with the next offset for continuation. Large pages are saved to a file with a path for Read. + +Each call searches the current filesystem again; pagination is not a snapshot, and file changes can shift results between pages. To collect a large list, use `head_limit=0`, read any saved output, and follow continuation offsets if the character limit is reached. Search timeouts, traversal errors, and output capture limits can still produce partial results; the result reports these limits, and pagination cannot recover paths that were never collected. Narrow the search and retry when it is incomplete. Large-directory caveat — avoid recursing into dependency / build output even with an anchor, especially when `include_ignored` is set: -- `node_modules/**/*.js`, `.venv/**/*.py`, `__pycache__/**`, `target/**` can produce thousands of results that truncate at the match cap and waste context. Prefer specific subpaths like `node_modules/react/src/**/*.js`. +- `node_modules/**/*.js`, `.venv/**/*.py`, `__pycache__/**`, `target/**` can produce thousands of results and waste search time and context. Prefer specific subpaths like `node_modules/react/src/**/*.js` unless you need a complete listing. diff --git a/packages/agent-core-v2/src/agent/tools/os/glob/glob.ts b/packages/agent-core-v2/src/agent/tools/os/glob/glob.ts index 5043a2e68..a26fc4320 100644 --- a/packages/agent-core-v2/src/agent/tools/os/glob/glob.ts +++ b/packages/agent-core-v2/src/agent/tools/os/glob/glob.ts @@ -5,6 +5,22 @@ import { type AgentTool } from '#/tool/toolContract'; export const GlobInputSchema = z.object({ pattern: z.string().describe('Glob pattern to match files.'), + head_limit: z + .number() + .int() + .nonnegative() + .optional() + .describe( + 'Maximum number of matching paths to return after offset. Defaults to 100. Pass 0 to remove the match-count limit. The character limit still applies: large pages are saved for Read, and a continuation offset is provided when more paths remain. Search time and output capture limits still apply.', + ), + offset: z + .number() + .int() + .nonnegative() + .optional() + .describe( + 'Number of matching paths to skip. Defaults to 0. Each call searches the current filesystem again; changes can shift results between pages.', + ), path: z .string() .optional() @@ -27,7 +43,7 @@ export const GlobInputSchema = z.object({ export type GlobInput = z.infer; -export const MAX_MATCHES = 100; +export const DEFAULT_HEAD_LIMIT = 100; export const WINDOWS_PATH_HINT = '\n\nWindows note: the `path` argument accepts both Windows paths ' + diff --git a/packages/agent-core-v2/src/agent/tools/os/glob/globTool.ts b/packages/agent-core-v2/src/agent/tools/os/glob/globTool.ts index 75110f802..7be6dda07 100644 --- a/packages/agent-core-v2/src/agent/tools/os/glob/globTool.ts +++ b/packages/agent-core-v2/src/agent/tools/os/glob/globTool.ts @@ -17,6 +17,7 @@ import { ISessionSkillCatalog } from '#/features/skill/session/skillCatalog'; import { ISessionWorkspaceContext } from '#/session/workspaceContext/workspaceContext'; import { ITelemetryService } from '#/app/telemetry/telemetry'; import { + DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS, ToolAccesses, type ExecutableToolResult, type ToolExecution, @@ -37,7 +38,7 @@ import { type GlobInput, GlobInputSchema, IGlobTool, - MAX_MATCHES, + DEFAULT_HEAD_LIMIT, WINDOWS_PATH_HINT, } from './glob'; @@ -238,50 +239,90 @@ export class GlobTool implements IGlobTool { } } - const truncated = kept.length > MAX_MATCHES; - const limited = truncated ? kept.slice(0, MAX_MATCHES) : kept; - - if (limited.length === 0 && !timedOut) { - if (filteredSensitive > 0) { - return { - output: `No non-sensitive matches found (${String(filteredSensitive)} sensitive file(s) filtered).`, - }; - } - return { output: 'No matches found' }; - } + const offset = args.offset ?? 0; + const headLimit = args.head_limit ?? DEFAULT_HEAD_LIMIT; + const limited = headLimit === 0 ? kept.slice(offset) : kept.slice(offset, offset + headLimit); + const partial = bufferTruncated || timedOut || traversalWarning !== undefined; const pathClass = env.pathClass; const shouldRelativize = isWithinDirectory(searchRoot, workspace.workspaceDir, pathClass); - const displayLines = limited.map((p) => + const candidates = limited.map((p) => shouldRelativize ? relativizeIfUnder(p, searchRoot, pathClass) : p, ); - const lines: string[] = []; + const warnings: string[] = []; if (timedOut) { - lines.push( + warnings.push( `Glob timed out after ${String(DEFAULT_TIMEOUT_MS / 1000)}s; partial results returned.`, ); } if (bufferTruncated) { - lines.push( + warnings.push( `[stdout truncated at ${String(MAX_OUTPUT_BYTES)} bytes; results may be incomplete — use a more specific pattern]`, ); } if (traversalWarning !== undefined) { - lines.push(traversalWarning); + warnings.push(traversalWarning); } - if (truncated) { - lines.push(`[Truncated at ${String(MAX_MATCHES)} matches — use a more specific pattern]`); - lines.push(`Only the first ${String(MAX_MATCHES)} matches are returned.`); - } - lines.push(...displayLines); - if (filteredSensitive > 0) { - lines.push(`Filtered ${String(filteredSensitive)} sensitive file(s).`); + const pageNotices = (count: number, characterLimited: boolean) => { + const lines = [...warnings]; + const footer: string[] = []; + const truncated = characterLimited || offset + count < kept.length; + if (count === 0) { + if (kept.length > 0) { + const resultSet = partial ? 'collected partial result set' : 'current result set'; + lines.push( + `No more matches at offset=${String(offset)} in the ${resultSet} (${String(kept.length)} matches).`, + ); + } else if (partial) { + lines.push('No matches collected; search incomplete.'); + } else if (filteredSensitive > 0) { + lines.push( + `No non-sensitive matches found (${String(filteredSensitive)} sensitive file(s) filtered).`, + ); + } else { + lines.push('No matches found'); + } + } else if (truncated || offset > 0 || partial) { + const total = partial + ? `${String(kept.length)} collected matches (partial result set)` + : String(kept.length); + lines.push(`Showing matches ${String(offset + 1)}–${String(offset + count)} of ${total}.`); + } + if (characterLimited) lines.push('Character limit reached; only complete paths are returned.'); + if (truncated) { + lines.push( + `Continue with the same search arguments and offset=${String(offset + count)}.`, + ); + if (!characterLimited) lines.push('To remove the match-count limit, omit offset and use head_limit=0.'); + } + if (filteredSensitive > 0 && (kept.length > 0 || partial)) { + footer.push(`Filtered ${String(filteredSensitive)} sensitive file(s).`); + } + if (!truncated && !partial && offset === 0 && headLimit > 0 && count === headLimit) { + footer.push(`Found ${String(count)} matches`); + } + return { lines, footer }; + }; + const noticeChars = Math.max(...[false, true].map((characterLimited) => { + const { lines, footer } = pageNotices(candidates.length, characterLimited); + return [...lines, ...footer].join('\n').length + 2; + })); + let remaining = DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS - noticeChars; + const displayLines: string[] = []; + for (const path of candidates) { + if (path.length + 1 > remaining) break; + displayLines.push(path); + remaining -= path.length + 1; } - if (!truncated && limited.length === MAX_MATCHES) { - lines.push(`Found ${String(limited.length)} matches`); + if (candidates.length > 0 && displayLines.length === 0) { + return { + isError: true, + output: 'Glob cannot fit a complete path and its diagnostics within the output limit. Narrow the search path or pattern.', + }; } - return { output: lines.join('\n') }; + const notices = pageNotices(displayLines.length, displayLines.length < candidates.length); + return { output: [...notices.lines, ...displayLines, ...notices.footer].join('\n') }; } } diff --git a/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts b/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts index 9c3b55096..14a672ae5 100644 --- a/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts +++ b/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts @@ -658,7 +658,7 @@ describe('FullCompaction', () => { event: 'compaction_finished', properties: expect.objectContaining({ source: 'manual', - tokens_before: 18_193, + tokens_before: expect.any(Number), retry_count: 1, trace_id: 'trace-compact-1', }), @@ -1125,7 +1125,7 @@ describe('FullCompaction', () => { properties: expect.objectContaining({ agent_id: 'main', source: 'manual', - tokens_before: 18_193, + tokens_before: expect.any(Number), duration_ms: expect.any(Number), round: 1, retry_count: 0, @@ -1350,7 +1350,7 @@ describe('FullCompaction', () => { event: 'compaction_failed', properties: expect.objectContaining({ source: 'manual', - tokens_before: 18_193, + tokens_before: expect.any(Number), duration_ms: expect.any(Number), retry_count: 4, error_type: 'APIConnectionError', @@ -1551,6 +1551,7 @@ describe('FullCompaction', () => { const ctx = testAgent(); ctx.configure({ provider: CATALOGUED_PROVIDER, + tools: SNAPSHOT_VISIBLE_TOOLS, modelCapabilities: { ...CATALOGUED_MODEL_CAPABILITIES, max_context_tokens: maxContextTokens, diff --git a/packages/agent-core-v2/test/agent/loop/loop.test.ts b/packages/agent-core-v2/test/agent/loop/loop.test.ts index 2660f6a44..531cb4212 100644 --- a/packages/agent-core-v2/test/agent/loop/loop.test.ts +++ b/packages/agent-core-v2/test/agent/loop/loop.test.ts @@ -161,8 +161,8 @@ describe('Agent loop', () => { [emit] turn.step.started { "time": "