mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
v0.41: subagent hardening — Bug 1 + Bug 3 + Approach C composable prompt
Three independent fixes to src/core/minions/handlers/subagent.ts. Each is covered by its own test set; bundled in one commit because they touch overlapping lines of subagent.ts (cleaner than 3 hunk-split commits). Bug 1 — rate-lease default 8 → 32 + `unlimited` sentinel src/core/minions/handlers/subagent.ts:61 Pre-v0.41 the default cap of 8 starved 10-concurrency batches on upstreams with no provider-side rate limit (Azure/Bedrock/self-hosted). New resolveLeaseCap() bumps default to 32, accepts `unlimited`/`none` as POSITIVE_INFINITY sentinel, throws on NaN/negative/zero with a paste-ready hint. Codex pass-1 #7 caught the original `=0`/`NaN`-uncapped semantics as dangerous (universal convention is "0 means disabled"). Pinned by test/rate-leases-uncapped.test.ts (15 cases). Bug 3 — strip `provider:` prefix at Anthropic SDK call site src/core/minions/handlers/subagent.ts:439, ~:895 `gbrain agent run --model anthropic:claude-sonnet-4-6` pre-fix sent the qualified string straight to client.messages.create which Anthropic rejects with "model not found." New stripProviderPrefix() applies at the one SDK call site; `model` stays qualified everywhere else (persistence, recipe lookup, capability gate). Pinned by 4 new test/subagent-handler.test.ts cases. Approach C — composable system prompt renderer w/ per-tool usage_hint src/core/minions/system-prompt.ts (NEW) src/core/minions/types.ts (ToolDef.usage_hint + SubagentHandlerData.system_no_tool_preamble) src/core/minions/tools/brain-allowlist.ts (BRAIN_TOOL_USAGE_HINTS) src/core/minions/handlers/subagent.ts (wiring) Bug 4 absorbed: pre-v0.41 DEFAULT_SYSTEM was one generic line that gave the model no guidance on WHICH tool to reach for. The field-report case was a `shell` tool sitting unused because nothing told the model to reach for it. New deterministic renderer splices a tool-usage preamble listing each tool's name + usage_hint; closing paragraph names shell/bash explicitly + tells the model brain tools write to the DB (not local files). Determinism preserved for Anthropic prompt-cache marker stability. Pinned by 13 cases in test/system-prompt.test.ts (determinism, opt-out, plugin tools, cache safety). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
12429f70e7
commit
41d6ae536f
@@ -48,6 +48,7 @@ import {
|
||||
logSubagentHeartbeat,
|
||||
} from './subagent-audit.ts';
|
||||
import { resolveModel, isAnthropicProvider, TIER_DEFAULTS } from '../../model-config.ts';
|
||||
import { buildSystemPrompt, DEFAULT_SUBAGENT_SYSTEM } from '../system-prompt.ts';
|
||||
import { toolLoop as gatewayToolLoop } from '../../ai/gateway.ts';
|
||||
import type { ChatToolDef, ChatMessage, ChatBlock, ChatResult, ToolHandler } from '../../ai/gateway.ts';
|
||||
import { classifyCapabilities } from '../../ai/capabilities.ts';
|
||||
@@ -58,9 +59,36 @@ import { randomUUIDv7 } from 'bun';
|
||||
const DEFAULT_MODEL = 'claude-sonnet-4-6';
|
||||
const DEFAULT_MAX_TURNS = 20;
|
||||
const DEFAULT_RATE_KEY = 'anthropic:messages';
|
||||
const DEFAULT_MAX_CONCURRENT = Number(process.env.GBRAIN_ANTHROPIC_MAX_INFLIGHT ?? '8');
|
||||
|
||||
/**
|
||||
* Resolve the rate-lease cap from the env var.
|
||||
*
|
||||
* undefined → 32 (default; was 8 pre-v0.41, starved 10-concurrency batches)
|
||||
* "unlimited" → POSITIVE_INFINITY (Azure / Bedrock / self-hosted with no upstream cap)
|
||||
* "none" → POSITIVE_INFINITY (alias)
|
||||
* positive number → that number
|
||||
* anything else → throws (NaN / "0" / negative / typo — fail loud, NOT silent uncap)
|
||||
*
|
||||
* Codex pass-1 #7 caught the original `=0` and `NaN` silently uncapping;
|
||||
* "0 means disabled" is the universal convention, so we use an explicit
|
||||
* `unlimited` sentinel instead. Misconfig fails at startup with a hint.
|
||||
*/
|
||||
export function resolveLeaseCap(raw: string | undefined): number {
|
||||
if (raw === undefined) return 32;
|
||||
if (raw === 'unlimited' || raw === 'none') return Number.POSITIVE_INFINITY;
|
||||
const n = Number(raw);
|
||||
if (Number.isFinite(n) && n > 0) return n;
|
||||
throw new Error(
|
||||
`GBRAIN_ANTHROPIC_MAX_INFLIGHT="${raw}" is invalid. ` +
|
||||
`Use a positive integer, "unlimited" (or "none"), or omit for default 32.`,
|
||||
);
|
||||
}
|
||||
const DEFAULT_MAX_CONCURRENT = resolveLeaseCap(process.env.GBRAIN_ANTHROPIC_MAX_INFLIGHT);
|
||||
const DEFAULT_LEASE_TTL_MS = 120_000;
|
||||
const DEFAULT_SYSTEM = 'You are a helpful assistant running as a gbrain subagent.';
|
||||
// v0.41 Approach C: DEFAULT_SUBAGENT_SYSTEM lives in ./system-prompt.ts
|
||||
// so the renderer and the handler share one source of truth. Kept as
|
||||
// a re-export alias here for back-compat with any external importer.
|
||||
const DEFAULT_SYSTEM = DEFAULT_SUBAGENT_SYSTEM;
|
||||
|
||||
// ── Injectable surfaces (for tests) ─────────────────────────
|
||||
|
||||
@@ -184,7 +212,11 @@ export function makeSubagentHandler(deps: SubagentDeps) {
|
||||
fallback: TIER_DEFAULTS.subagent,
|
||||
});
|
||||
const maxTurns = data.max_turns ?? DEFAULT_MAX_TURNS;
|
||||
const systemPrompt = data.system ?? DEFAULT_SYSTEM;
|
||||
// v0.41 Approach C: systemPrompt is now built AFTER toolDefs (a few
|
||||
// lines below) so the renderer can splice a tool-usage preamble
|
||||
// listing each available tool's usage_hint. The renderer is
|
||||
// deterministic so the Anthropic prompt-cache marker on the system
|
||||
// block stays a hit across turns.
|
||||
|
||||
// v0.38 S1.10 — feature flag for the gateway-native tool loop. When ON,
|
||||
// route ALL subagent jobs through gateway.toolLoop() (works for every
|
||||
@@ -219,6 +251,13 @@ export function makeSubagentHandler(deps: SubagentDeps) {
|
||||
? filterAllowedTools(registry, data.allowed_tools)
|
||||
: registry;
|
||||
|
||||
// v0.41 Approach C: render the final system prompt now that toolDefs
|
||||
// is known. Splices a deterministic tool-usage preamble listing each
|
||||
// tool's usage_hint. Caller can opt out via data.system_no_tool_preamble.
|
||||
const systemPrompt = buildSystemPrompt(toolDefs, data.system, {
|
||||
no_tool_preamble: data.system_no_tool_preamble,
|
||||
});
|
||||
|
||||
logSubagentSubmission({
|
||||
caller: 'worker',
|
||||
remote: true,
|
||||
@@ -437,7 +476,10 @@ export function makeSubagentHandler(deps: SubagentDeps) {
|
||||
// complexity; for v0.15 we lean on the 120s TTL + abort-on-signal.
|
||||
try {
|
||||
const params: Anthropic.MessageCreateParamsNonStreaming = {
|
||||
model,
|
||||
// v0.41 Bug 3: strip `provider:` prefix at the SDK call site only.
|
||||
// `model` stays qualified everywhere else (persistence, recipe
|
||||
// lookup at recipeIdFromModel(), capability gate).
|
||||
model: stripProviderPrefix(model),
|
||||
max_tokens: 4096,
|
||||
system: [
|
||||
{ type: 'text', text: systemPrompt, cache_control: { type: 'ephemeral' } },
|
||||
@@ -897,6 +939,28 @@ function recipeIdFromModel(modelString: string): string {
|
||||
return idx > 0 ? modelString.slice(0, idx) : 'anthropic';
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip the `provider:` prefix from a model string. Returns the bare
|
||||
* model id the Anthropic Messages API expects. Idempotent on already-bare
|
||||
* strings.
|
||||
*
|
||||
* stripProviderPrefix('anthropic:claude-sonnet-4-6') === 'claude-sonnet-4-6'
|
||||
* stripProviderPrefix('claude-sonnet-4-6') === 'claude-sonnet-4-6'
|
||||
*
|
||||
* v0.41 Bug 3 — pre-fix, `gbrain agent run --model anthropic:claude-sonnet-4-6`
|
||||
* sent the prefixed string straight into `client.messages.create()`, which
|
||||
* Anthropic rejects with "model not found." Omitting `--model` worked because
|
||||
* `resolveModel()` returns the bare id; explicit-model users hit the bug.
|
||||
*
|
||||
* Used ONLY at the SDK call site. The wider `model` variable stays
|
||||
* qualified everywhere else (persistence, recipe lookup, capability gate)
|
||||
* because those readers want the provider info.
|
||||
*/
|
||||
export function stripProviderPrefix(modelString: string): string {
|
||||
const idx = modelString.indexOf(':');
|
||||
return idx > 0 ? modelString.slice(idx + 1) : modelString;
|
||||
}
|
||||
|
||||
/**
|
||||
* D5 — adapt v1 Anthropic content blocks to v2 ChatBlock shape on read.
|
||||
* Symmetric in the other direction is handled by persisting ChatBlock[] as-is
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* v0.41 Approach C — composable subagent system prompt renderer.
|
||||
*
|
||||
* Field-report case: a `shell` tool sat in the registry and the subagent
|
||||
* never used it because the default system prompt was one generic line
|
||||
* (`'You are a helpful assistant running as a gbrain subagent.'`) and gave
|
||||
* the model no guidance on WHICH tool to reach for. This module fixes that
|
||||
* by splicing a deterministic tool-guidance preamble into the system prompt
|
||||
* based on the actual `toolDefs` array, including any plugin-registered
|
||||
* tools the core has no knowledge of.
|
||||
*
|
||||
* Three properties matter for the Anthropic prompt-cache contract:
|
||||
*
|
||||
* 1. Deterministic — same `(toolDefs, userSystem, opts)` input produces
|
||||
* byte-identical output. The Anthropic `cache_control: ephemeral`
|
||||
* marker on the system block wraps OUR rendered string; if rendering
|
||||
* drifted across runs, the cache marker would miss on every turn.
|
||||
* 2. Generative — works for plugin tools the core has never heard of.
|
||||
* A downstream plugin that registers `playwright_navigate` with a
|
||||
* `usage_hint` gets that hint surfaced automatically.
|
||||
* 3. Opinionated — the closing paragraph names `shell`/`bash` explicitly
|
||||
* AND tells the model that brain tools (`put_page`, `search`, `query`)
|
||||
* write to the brain DB, not the local filesystem. Both halves of the
|
||||
* field-report bug (subagent didn't reach for shell + subagent wrote
|
||||
* brain pages when files were wanted) collapse into one fix.
|
||||
*
|
||||
* Override paths preserved: caller's `data.system` overrides the default
|
||||
* generic line; `data.system_no_tool_preamble: true` skips the preamble
|
||||
* splice entirely so the caller's prompt stays byte-for-byte as provided.
|
||||
*/
|
||||
|
||||
import type { ToolDef } from './types.ts';
|
||||
|
||||
export const DEFAULT_SUBAGENT_SYSTEM =
|
||||
'You are a helpful assistant running as a gbrain subagent.';
|
||||
|
||||
export interface SystemPromptOpts {
|
||||
/** Skip the auto-generated tool guidance preamble. */
|
||||
no_tool_preamble?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the final system prompt sent to the Anthropic Messages API.
|
||||
*
|
||||
* Composition order:
|
||||
* - userSystem (or DEFAULT) — the caller's preamble.
|
||||
* - blank line.
|
||||
* - tool guidance preamble (when toolDefs is non-empty AND opts.no_tool_preamble !== true).
|
||||
*
|
||||
* Output is fully deterministic for a given input — required for the
|
||||
* Anthropic prompt-cache marker on the system block.
|
||||
*/
|
||||
export function buildSystemPrompt(
|
||||
toolDefs: ToolDef[],
|
||||
userSystem: string | undefined,
|
||||
opts: SystemPromptOpts = {},
|
||||
): string {
|
||||
const base = userSystem ?? DEFAULT_SUBAGENT_SYSTEM;
|
||||
if (opts.no_tool_preamble || toolDefs.length === 0) return base;
|
||||
return base + '\n\n' + renderToolPreamble(toolDefs);
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the tool-usage preamble. Pure function; no I/O.
|
||||
*
|
||||
* Preamble shape (exact bytes, do not reorder — cache-marker stability):
|
||||
*
|
||||
* You have the following tools available. Reach for them by default — do NOT
|
||||
* describe file contents, hypothetical shell output, or planned database
|
||||
* writes in prose. Call the tool.
|
||||
*
|
||||
* - `tool_name` — usage_hint (when present)
|
||||
* - `tool_name` (when no usage_hint)
|
||||
* ...
|
||||
*
|
||||
* When the task asks you to write a file, run a command, or modify the
|
||||
* filesystem, prefer a `shell` or `bash` tool if one is in your registry.
|
||||
* Brain tools (`put_page`, `search`, `query`) write to the gbrain database,
|
||||
* not to local files.
|
||||
*
|
||||
* Tools are listed in the order the caller passed them — NO sorting. The
|
||||
* caller's order matches the order in the Anthropic `tools:` array, which
|
||||
* matches the order the model sees in its tool-choice context. Sorting
|
||||
* would diverge from that and confuse cache-marker reasoning.
|
||||
*/
|
||||
export function renderToolPreamble(toolDefs: ToolDef[]): string {
|
||||
const lines: string[] = [];
|
||||
for (const t of toolDefs) {
|
||||
if (t.usage_hint && t.usage_hint.trim()) {
|
||||
// Normalize any embedded whitespace/newlines so the rendered line
|
||||
// stays single-line. Defense against a plugin author shipping a
|
||||
// multi-line hint (which would corrupt the preamble layout).
|
||||
const hint = t.usage_hint.replace(/\s+/g, ' ').trim();
|
||||
lines.push(`- \`${t.name}\` — ${hint}`);
|
||||
} else {
|
||||
lines.push(`- \`${t.name}\``);
|
||||
}
|
||||
}
|
||||
return [
|
||||
'You have the following tools available. Reach for them by default — do NOT',
|
||||
'describe file contents, hypothetical shell output, or planned database',
|
||||
'writes in prose. Call the tool.',
|
||||
'',
|
||||
...lines,
|
||||
'',
|
||||
'When the task asks you to write a file, run a command, or modify the',
|
||||
'filesystem, prefer a `shell` or `bash` tool if one is in your registry.',
|
||||
'Brain tools (`put_page`, `search`, `query`) write to the gbrain database,',
|
||||
'not to local files.',
|
||||
].join('\n');
|
||||
}
|
||||
@@ -66,6 +66,36 @@ export const BRAIN_TOOL_ALLOWLIST: ReadonlySet<string> = new Set([
|
||||
'find_anomalies',
|
||||
]);
|
||||
|
||||
/**
|
||||
* v0.41 Approach C: per-tool usage_hint surfaced verbatim in the subagent
|
||||
* system prompt's tool preamble. Each entry tells the model WHEN to reach
|
||||
* for the tool (the description tells the model HOW). One line per tool;
|
||||
* no embedded newlines.
|
||||
*
|
||||
* Field-report driver: the renderer in `src/core/minions/system-prompt.ts`
|
||||
* surfaces these so a model with `shell` + brain tools in its registry
|
||||
* knows brain tools write to the gbrain DB (NOT local files) and to reach
|
||||
* for shell when the task asks for filesystem work.
|
||||
*
|
||||
* Keyed by OP name (pre-`brain_` prefix). Optional — tools without an entry
|
||||
* just render as `- \`name\`` with no hint suffix.
|
||||
*/
|
||||
export const BRAIN_TOOL_USAGE_HINTS: Readonly<Record<string, string>> = {
|
||||
query: 'Use for natural-language semantic search across the brain (vector + keyword hybrid). Returns ranked passages with citations. First choice when the user asks a question of the brain.',
|
||||
search: 'Use for hybrid keyword + vector search returning ranked page hits. Use over `query` when you want page-level not chunk-level results (e.g. "find pages about X").',
|
||||
get_page: 'Read a brain page by its slug. Returns the full markdown body + frontmatter + linked pages.',
|
||||
list_pages: 'List pages by type or slug-prefix filter. Use when you need to enumerate (e.g. "list all `people/` pages") instead of search.',
|
||||
file_list: 'List uploaded files (attachments) by slug-prefix or content type. NOT the local filesystem — only files the brain has stored.',
|
||||
file_url: 'Get a presigned URL for a brain-stored file. Read-only; expires.',
|
||||
get_backlinks: 'List every page that links TO the given slug. Use for "what references this".',
|
||||
traverse_graph: 'Walk the typed-edge graph starting from a slug (e.g. `works_at`, `founded`, `invested_in`). Use for relationship queries.',
|
||||
resolve_slugs: 'Resolve free-form entity names to canonical slugs (e.g. "Alice" → `people/alice-example`). Use before any tool that takes a slug if the user gave a name not a slug.',
|
||||
get_ingest_log: 'Read the brain ingestion log for diagnostic / verification queries.',
|
||||
put_page: 'Write a markdown page to the gbrain DATABASE (NOT the local filesystem). Page becomes searchable + linkable. Slug must match the agent\'s allowed namespace.',
|
||||
get_recent_salience: 'Read pages ranked by emotional + activity salience over a recency window. Use for "what\'s been on my mind lately".',
|
||||
find_anomalies: 'Read cohort-level activity outliers (e.g. tag-cohort or type-cohort with unusual recent volume). Use for "what\'s unusual lately".',
|
||||
};
|
||||
|
||||
/** Matches Anthropic's tool-name constraint. No dots. */
|
||||
const ANTHROPIC_NAME_RE = /^[a-zA-Z0-9_-]{1,64}$/;
|
||||
|
||||
@@ -223,6 +253,9 @@ export function buildBrainTools(opts: BuildBrainToolsOpts): ToolDef[] {
|
||||
// v0.15 ships only idempotent brain tools (every allow-listed op is
|
||||
// deterministic over its input; put_page re-writes the same slug).
|
||||
idempotent: true,
|
||||
// v0.41 Approach C: surface usage_hint to the system-prompt renderer.
|
||||
// Keyed by the unprefixed op name. Undefined when no hint is registered.
|
||||
usage_hint: BRAIN_TOOL_USAGE_HINTS[op.name],
|
||||
async execute(input: unknown, ctx: ToolCtx): Promise<unknown> {
|
||||
const opCtx = buildOpContext({
|
||||
engine: ctx.engine,
|
||||
|
||||
@@ -449,6 +449,18 @@ export interface SubagentHandlerData {
|
||||
* and direct CLI submitters set it.
|
||||
*/
|
||||
allowed_slug_prefixes?: string[];
|
||||
/**
|
||||
* v0.41 Approach C: opt out of the auto-generated tool-usage preamble
|
||||
* that `buildSystemPrompt()` splices into `system`. Default behavior
|
||||
* (omitted or false) prepends a deterministic preamble listing each
|
||||
* tool's name + usage_hint. Set to `true` to keep `system` byte-for-byte
|
||||
* as provided.
|
||||
*
|
||||
* Use when the caller has hand-tuned a complete system prompt for a
|
||||
* specific subagent (no benefit from auto-generated guidance, prompt
|
||||
* cache hits ride entirely on the caller-supplied prefix).
|
||||
*/
|
||||
system_no_tool_preamble?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -499,6 +511,20 @@ export interface ToolDef {
|
||||
input_schema: Record<string, unknown>;
|
||||
idempotent: boolean;
|
||||
execute(input: unknown, ctx: ToolCtx): Promise<unknown>;
|
||||
/**
|
||||
* v0.41 Approach C: one-line hint surfaced verbatim in the subagent
|
||||
* system prompt's tool preamble. Tells the model WHEN to use this tool.
|
||||
* The `description` tells the model HOW; the `usage_hint` tells WHEN.
|
||||
*
|
||||
* Field-report case: a `shell` tool sat in the registry and the subagent
|
||||
* never used it because nothing told the model "to write a file, use
|
||||
* shell." Per-tool hints surface that directly. Plugin authors get this
|
||||
* affordance for free.
|
||||
*
|
||||
* Optional — omitted tools just don't get a hint line. Must be a single
|
||||
* line (no embedded newlines) so the rendered preamble stays scannable.
|
||||
*/
|
||||
usage_hint?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
/**
|
||||
* v0.41 Bug 1: `unlimited` sentinel + POSITIVE_INFINITY semantics for the
|
||||
* Anthropic rate-lease cap. Closes the field-report bug where a default cap
|
||||
* of 8 starved a 10-concurrency batch on an Azure-Sweden endpoint that had
|
||||
* no upstream rate limit.
|
||||
*
|
||||
* Pure-function tests for `resolveLeaseCap()` cover the input matrix
|
||||
* (default, "unlimited", "none", positive int, NaN, zero, negative, typo).
|
||||
*
|
||||
* Integration tests against PGLite verify `acquireLease(..., Infinity)`
|
||||
* always returns acquired=true, still inserts the lease row (so TTL
|
||||
* pruning + crash recovery still work), and parallel acquires don't
|
||||
* deadlock on the advisory lock path.
|
||||
*
|
||||
* Codex pass-1 #7 caught the original `=0` + `NaN`-as-uncapped semantics
|
||||
* as dangerous (universal convention is "0 means disabled"); these tests
|
||||
* pin the corrected fail-loud behavior.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { MinionQueue } from '../src/core/minions/queue.ts';
|
||||
import { acquireLease, renewLease } from '../src/core/minions/rate-leases.ts';
|
||||
import { resolveLeaseCap } from '../src/core/minions/handlers/subagent.ts';
|
||||
|
||||
describe('resolveLeaseCap (pure)', () => {
|
||||
test('undefined returns default 32 (was 8 pre-v0.41)', () => {
|
||||
expect(resolveLeaseCap(undefined)).toBe(32);
|
||||
});
|
||||
|
||||
test('"unlimited" returns POSITIVE_INFINITY', () => {
|
||||
expect(resolveLeaseCap('unlimited')).toBe(Number.POSITIVE_INFINITY);
|
||||
});
|
||||
|
||||
test('"none" returns POSITIVE_INFINITY (alias)', () => {
|
||||
expect(resolveLeaseCap('none')).toBe(Number.POSITIVE_INFINITY);
|
||||
});
|
||||
|
||||
test('positive integer returns that integer', () => {
|
||||
expect(resolveLeaseCap('50')).toBe(50);
|
||||
expect(resolveLeaseCap('1')).toBe(1);
|
||||
});
|
||||
|
||||
test('positive float returns that float', () => {
|
||||
// Implementation note: SQL int conversion will truncate, but the parser
|
||||
// itself stays Number-typed. The `> 0` guard accepts floats.
|
||||
expect(resolveLeaseCap('2.5')).toBe(2.5);
|
||||
});
|
||||
|
||||
test('"0" THROWS (universal "0 means disabled" convention; codex #7 fix)', () => {
|
||||
expect(() => resolveLeaseCap('0')).toThrow(/invalid/);
|
||||
expect(() => resolveLeaseCap('0')).toThrow(/unlimited/);
|
||||
});
|
||||
|
||||
test('negative number THROWS', () => {
|
||||
expect(() => resolveLeaseCap('-5')).toThrow(/invalid/);
|
||||
});
|
||||
|
||||
test('typo THROWS loudly with hint (NOT silent uncap)', () => {
|
||||
expect(() => resolveLeaseCap('trnety')).toThrow(/invalid/);
|
||||
expect(() => resolveLeaseCap('trnety')).toThrow(/unlimited/);
|
||||
});
|
||||
|
||||
test('empty string THROWS', () => {
|
||||
// Number('') is 0, which fails the > 0 guard.
|
||||
expect(() => resolveLeaseCap('')).toThrow(/invalid/);
|
||||
});
|
||||
|
||||
test('whitespace-only THROWS', () => {
|
||||
expect(() => resolveLeaseCap(' ')).toThrow(/invalid/);
|
||||
});
|
||||
|
||||
test('hint mentions the invalid input verbatim for paste-back debugging', () => {
|
||||
try {
|
||||
resolveLeaseCap('garbage');
|
||||
throw new Error('expected throw');
|
||||
} catch (e) {
|
||||
expect((e as Error).message).toContain('"garbage"');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('acquireLease with cap=POSITIVE_INFINITY (integration)', () => {
|
||||
let engine: PGLiteEngine;
|
||||
let queue: MinionQueue;
|
||||
let owner: number;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({ database_url: '' });
|
||||
await engine.initSchema();
|
||||
queue = new MinionQueue(engine);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await engine.executeRaw('DELETE FROM subagent_rate_leases');
|
||||
await engine.executeRaw('DELETE FROM minion_jobs');
|
||||
const j = await queue.add('owner', {});
|
||||
owner = j.id;
|
||||
});
|
||||
|
||||
test('always returns acquired=true regardless of activeCount', async () => {
|
||||
// Drive the lease table to 50 active leases under Infinity cap.
|
||||
for (let i = 0; i < 50; i++) {
|
||||
const j = await queue.add('owner', {});
|
||||
const r = await acquireLease(engine, 'anthropic:messages', j.id, Number.POSITIVE_INFINITY);
|
||||
expect(r.acquired).toBe(true);
|
||||
expect(r.maxConcurrent).toBe(Number.POSITIVE_INFINITY);
|
||||
}
|
||||
// 51st acquire still wins.
|
||||
const r = await acquireLease(engine, 'anthropic:messages', owner, Number.POSITIVE_INFINITY);
|
||||
expect(r.acquired).toBe(true);
|
||||
});
|
||||
|
||||
test('lease row is still inserted (so TTL pruning + crash recovery still work)', async () => {
|
||||
const r = await acquireLease(engine, 'anthropic:messages', owner, Number.POSITIVE_INFINITY, {
|
||||
ttlMs: 10_000,
|
||||
});
|
||||
expect(r.acquired).toBe(true);
|
||||
expect(r.leaseId).toBeDefined();
|
||||
const rows = await engine.executeRaw<{ count: string }>(
|
||||
'SELECT count(*)::text AS count FROM subagent_rate_leases WHERE id = $1',
|
||||
[r.leaseId!],
|
||||
);
|
||||
expect(parseInt(rows[0]!.count, 10)).toBe(1);
|
||||
});
|
||||
|
||||
test('renewLease still works under Infinity cap', async () => {
|
||||
const r = await acquireLease(engine, 'anthropic:messages', owner, Number.POSITIVE_INFINITY, {
|
||||
ttlMs: 10_000,
|
||||
});
|
||||
expect(r.acquired).toBe(true);
|
||||
const renewed = await renewLease(engine, r.leaseId!, 20_000);
|
||||
expect(renewed).toBe(true);
|
||||
});
|
||||
|
||||
test('parallel acquires under Infinity cap do not deadlock on advisory lock', async () => {
|
||||
const jobs: Array<Promise<{ id: number }>> = [];
|
||||
for (let i = 0; i < 10; i++) jobs.push(queue.add('owner', {}));
|
||||
const owners = (await Promise.all(jobs)).map(j => j.id);
|
||||
// Fire 10 parallel acquires on the same key under Infinity cap.
|
||||
const acquires = await Promise.all(
|
||||
owners.map(o => acquireLease(engine, 'anthropic:messages', o, Number.POSITIVE_INFINITY)),
|
||||
);
|
||||
// All 10 should win.
|
||||
for (const r of acquires) {
|
||||
expect(r.acquired).toBe(true);
|
||||
expect(r.leaseId).toBeDefined();
|
||||
}
|
||||
// And the table should have 10 rows.
|
||||
const rows = await engine.executeRaw<{ count: string }>(
|
||||
`SELECT count(*)::text AS count FROM subagent_rate_leases WHERE key = 'anthropic:messages'`,
|
||||
);
|
||||
expect(parseInt(rows[0]!.count, 10)).toBe(10);
|
||||
});
|
||||
});
|
||||
@@ -18,6 +18,7 @@ import { MinionQueue } from '../src/core/minions/queue.ts';
|
||||
import {
|
||||
makeSubagentHandler,
|
||||
RateLeaseUnavailableError,
|
||||
stripProviderPrefix,
|
||||
type MessagesClient,
|
||||
} from '../src/core/minions/handlers/subagent.ts';
|
||||
import type { ToolDef, MinionJobContext } from '../src/core/minions/types.ts';
|
||||
@@ -468,6 +469,48 @@ describe('subagent handler lease behavior', () => {
|
||||
expect(parseInt(rows[0]!.c, 10)).toBe(0);
|
||||
});
|
||||
|
||||
test('v0.41 Bug 3: stripProviderPrefix strips `anthropic:` qualified model', async () => {
|
||||
expect(stripProviderPrefix('anthropic:claude-sonnet-4-6')).toBe('claude-sonnet-4-6');
|
||||
});
|
||||
|
||||
test('v0.41 Bug 3: stripProviderPrefix is idempotent on bare names', async () => {
|
||||
expect(stripProviderPrefix('claude-sonnet-4-6')).toBe('claude-sonnet-4-6');
|
||||
});
|
||||
|
||||
test('v0.41 Bug 3: stripProviderPrefix handles edge inputs', async () => {
|
||||
expect(stripProviderPrefix('')).toBe('');
|
||||
// Leading colon = no valid provider name; pass through unchanged.
|
||||
// The `idx > 0` guard (not `>= 0`) makes this intentional.
|
||||
expect(stripProviderPrefix(':')).toBe(':');
|
||||
expect(stripProviderPrefix('a:b:c')).toBe('b:c'); // only strips first prefix
|
||||
});
|
||||
|
||||
test('v0.41 Bug 3: handler passes bare model id to Anthropic SDK when data.model is qualified', async () => {
|
||||
const calls: Array<Anthropic.MessageCreateParamsNonStreaming> = [];
|
||||
const client: MessagesClient = {
|
||||
async create(params) {
|
||||
calls.push(params);
|
||||
return {
|
||||
content: [{ type: 'text', text: 'ok' }],
|
||||
stop_reason: 'end_turn',
|
||||
usage: { input_tokens: 1, output_tokens: 1 },
|
||||
role: 'assistant',
|
||||
} as unknown as Anthropic.Message;
|
||||
},
|
||||
};
|
||||
const handler = makeSubagentHandler({
|
||||
engine, client, toolRegistry: [], maxConcurrent: 100, rateLeaseKey: 'k_prefix',
|
||||
});
|
||||
const ctx = await makeCtx({
|
||||
prompt: 'hello',
|
||||
model: 'anthropic:claude-sonnet-4-6', // qualified — the field-report bug case
|
||||
});
|
||||
await handler(ctx);
|
||||
expect(calls.length).toBe(1);
|
||||
// The SDK MUST receive the bare model id, not the prefixed one.
|
||||
expect(calls[0]!.model).toBe('claude-sonnet-4-6');
|
||||
});
|
||||
|
||||
test('throws RateLeaseUnavailableError when cap full', async () => {
|
||||
// Preload the cap with a stale-looking-but-live lease owned by a
|
||||
// different job.
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
/**
|
||||
* v0.41 Approach C — system-prompt renderer unit tests.
|
||||
*
|
||||
* Pins the deterministic-output contract that the Anthropic prompt-cache
|
||||
* marker on the system block depends on. Drifting the renderer's output
|
||||
* across invocations would silently miss the cache on every turn.
|
||||
*
|
||||
* Covers:
|
||||
* - DEFAULT_SUBAGENT_SYSTEM fallback when userSystem is undefined.
|
||||
* - Empty toolDefs → no preamble splice (falls through to bare system).
|
||||
* - Single tool, no usage_hint → bare list entry.
|
||||
* - Single tool, with usage_hint → list entry with " — hint" suffix.
|
||||
* - Multi-tool — order preserved from input (no sort).
|
||||
* - usage_hint with embedded newlines → normalized to single line.
|
||||
* - usage_hint whitespace-only string → treated as missing.
|
||||
* - no_tool_preamble opt-out → returns userSystem unchanged.
|
||||
* - Determinism — two calls with identical input produce byte-identical output.
|
||||
* - Plugin tool integration — registry-agnostic, works for any ToolDef.
|
||||
* - Closing paragraph naming shell/bash + brain DB distinction present.
|
||||
* - userSystem override fully preserved as the leading bytes (cache-hit safety).
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
buildSystemPrompt,
|
||||
renderToolPreamble,
|
||||
DEFAULT_SUBAGENT_SYSTEM,
|
||||
} from '../src/core/minions/system-prompt.ts';
|
||||
import type { ToolDef } from '../src/core/minions/types.ts';
|
||||
|
||||
function fakeTool(name: string, opts: { usage_hint?: string } = {}): ToolDef {
|
||||
return {
|
||||
name,
|
||||
description: `description of ${name}`,
|
||||
input_schema: { type: 'object' as const },
|
||||
idempotent: true,
|
||||
usage_hint: opts.usage_hint,
|
||||
async execute() {
|
||||
return null;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('buildSystemPrompt', () => {
|
||||
test('empty toolDefs + undefined userSystem → returns the DEFAULT bare system', () => {
|
||||
const out = buildSystemPrompt([], undefined);
|
||||
expect(out).toBe(DEFAULT_SUBAGENT_SYSTEM);
|
||||
});
|
||||
|
||||
test('empty toolDefs + custom userSystem → returns userSystem unchanged (no preamble)', () => {
|
||||
const userSystem = 'You are a curator of brand archives.';
|
||||
const out = buildSystemPrompt([], userSystem);
|
||||
expect(out).toBe(userSystem);
|
||||
});
|
||||
|
||||
test('no_tool_preamble=true keeps userSystem byte-for-byte even with tools', () => {
|
||||
const userSystem = 'Hand-tuned system prompt that should not be modified.';
|
||||
const tools = [fakeTool('search', { usage_hint: 'do searches' })];
|
||||
const out = buildSystemPrompt(tools, userSystem, { no_tool_preamble: true });
|
||||
expect(out).toBe(userSystem);
|
||||
});
|
||||
|
||||
test('tools without usage_hint render as bare list entries', () => {
|
||||
const tools = [fakeTool('search'), fakeTool('get_page')];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
expect(out).toContain('- `search`');
|
||||
expect(out).toContain('- `get_page`');
|
||||
expect(out).not.toContain('- `search` —');
|
||||
expect(out).not.toContain('- `get_page` —');
|
||||
});
|
||||
|
||||
test('tools with usage_hint render with " — hint" suffix', () => {
|
||||
const tools = [fakeTool('shell', { usage_hint: 'Run a shell command.' })];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
expect(out).toContain('- `shell` — Run a shell command.');
|
||||
});
|
||||
|
||||
test('preamble preserves caller-supplied tool order (no sort)', () => {
|
||||
const tools = [
|
||||
fakeTool('z_last', { usage_hint: 'late' }),
|
||||
fakeTool('a_first', { usage_hint: 'early' }),
|
||||
];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
const idxZ = out.indexOf('- `z_last`');
|
||||
const idxA = out.indexOf('- `a_first`');
|
||||
expect(idxZ).toBeGreaterThan(-1);
|
||||
expect(idxA).toBeGreaterThan(-1);
|
||||
expect(idxZ).toBeLessThan(idxA); // input order preserved
|
||||
});
|
||||
|
||||
test('usage_hint with embedded newlines is normalized to single line', () => {
|
||||
const tools = [
|
||||
fakeTool('shell', {
|
||||
usage_hint: 'Line one.\nLine two.\n\tIndented line three.',
|
||||
}),
|
||||
];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
// Whole hint collapses to single line; preamble layout intact.
|
||||
expect(out).toContain('- `shell` — Line one. Line two. Indented line three.');
|
||||
// No literal newlines inside the hint line.
|
||||
const hintLine = out.split('\n').find(l => l.startsWith('- `shell`'));
|
||||
expect(hintLine).toBeDefined();
|
||||
expect(hintLine!).not.toContain('\n');
|
||||
});
|
||||
|
||||
test('whitespace-only usage_hint treated as missing', () => {
|
||||
const tools = [fakeTool('foo', { usage_hint: ' \t ' })];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
expect(out).toContain('- `foo`');
|
||||
expect(out).not.toContain('- `foo` —');
|
||||
});
|
||||
|
||||
test('closing paragraph names shell/bash + brain DB distinction (field-report fix)', () => {
|
||||
const tools = [fakeTool('shell', { usage_hint: 'Run commands.' })];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
expect(out).toContain('`shell` or `bash` tool');
|
||||
expect(out).toContain('gbrain database');
|
||||
expect(out).toContain('not to local files');
|
||||
});
|
||||
|
||||
test('userSystem is the leading bytes of the output (Anthropic prompt-cache safety)', () => {
|
||||
const userSystem = 'Custom-tuned-prefix-for-cache-hit-stability.';
|
||||
const tools = [fakeTool('search', { usage_hint: 'do searches' })];
|
||||
const out = buildSystemPrompt(tools, userSystem);
|
||||
expect(out.startsWith(userSystem)).toBe(true);
|
||||
// Two newlines between userSystem and the preamble.
|
||||
expect(out.startsWith(userSystem + '\n\n')).toBe(true);
|
||||
});
|
||||
|
||||
test('determinism: identical input → byte-identical output', () => {
|
||||
const tools = [
|
||||
fakeTool('a', { usage_hint: 'do A' }),
|
||||
fakeTool('b'),
|
||||
fakeTool('c', { usage_hint: 'do C' }),
|
||||
];
|
||||
const userSystem = 'System.';
|
||||
const a = buildSystemPrompt(tools, userSystem);
|
||||
const b = buildSystemPrompt(tools, userSystem);
|
||||
expect(a).toBe(b);
|
||||
expect(a.length).toBe(b.length);
|
||||
});
|
||||
|
||||
test('plugin tool (registry-agnostic) gets its usage_hint surfaced', () => {
|
||||
// Simulate a downstream OpenClaw plugin that registers a custom tool.
|
||||
const tools = [
|
||||
fakeTool('playwright_navigate', {
|
||||
usage_hint: 'Drive a browser to a URL and wait for load.',
|
||||
}),
|
||||
];
|
||||
const out = buildSystemPrompt(tools, undefined);
|
||||
expect(out).toContain('- `playwright_navigate` — Drive a browser to a URL and wait for load.');
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderToolPreamble (pure)', () => {
|
||||
test('renders header + bullets + footer in canonical order', () => {
|
||||
const tools = [fakeTool('search', { usage_hint: 'do searches' })];
|
||||
const preamble = renderToolPreamble(tools);
|
||||
const lines = preamble.split('\n');
|
||||
// Header is first 3 lines.
|
||||
expect(lines[0]).toContain('You have the following tools available');
|
||||
expect(lines[1]).toContain('describe file contents');
|
||||
expect(lines[2]).toContain('Call the tool');
|
||||
// Then a blank line, then the tool list, then a blank, then the closer.
|
||||
expect(lines[3]).toBe('');
|
||||
expect(lines[4]).toContain('- `search`');
|
||||
expect(lines[5]).toBe('');
|
||||
expect(lines[6]).toContain('When the task asks you to write a file');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user