diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4cd56c284..e4066d316 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,6 +28,39 @@ way, even fully logged in. A shell alias masked the symptom in manual terminal testing (`which`/`command -v` resolve aliases; `child_process.spawn` never does). `resolveAugmentedPath()` now also checks `~/.local/bin`. +### Fixed — a granted `memory` tool no longer lets a sub-agent write past its parent's scope (#904, part of #860) + +2026-08-27 — A sub-agent that had been granted the native `memory` tool resolved +its handler out of the process-wide `NativeToolRegistry`. That entry belongs to +the memory *provider* plugin (`@omadia/memory`, `@omadia/memory-postgres`) and is +bound to the **undecorated** root store — the one below every scoping wrapper. A +sub-agent reaching it read and wrote outside its parent agent's +`orchestrator::*` subtree, and, with the chat-context ACL from #881 +enabled, outside its team's and channel's tiers too. Granting a sub-agent the +memory tool is ordinary operator configuration, and the per-agent boundary it +crossed predates the memory-ACL epic entirely. + +The grant is now served by a tool bound to the same turn-scoped store the +parent's own dispatch uses: `Orchestrator.dispatchToolInner` publishes that +handler for the lifetime of a domain-tool dispatch, and +`adaptNativeToolForSubAgent` takes the resolver as a **required** parameter, so a +call site that forgets to thread it fails `typecheck` instead of silently +degrading to the unscoped store — the same hardening #903 applied to +`dispatchTool` / `dispatchToolDeadlined` / `dispatchToolInner`. + +Two consequences worth knowing: + +- The grant used to be a **silent no-op** on a default install: the shipped + providers register handler-only (no wire-spec) and the adapter dropped such + entries. It is now honoured — with the parent turn's scope. +- **Fail-closed, never fallback.** With no turn-bound store — a detached + `ask__start` runner, or any call outside an orchestrator turn — the tool + refuses instead of reaching for a wider one. + +Unchanged and still true: the `claude-cli` provider never constructs the +`Orchestrator`, so `context_memory` remains inert there (#899). + + ### Added — team uninstall for provisioned agent identities (#900, part of #860) 2026-08-27 — Assigning an agent to a Team was one-way: `DELETE diff --git a/docs/teams-multi-agent-identities.md b/docs/teams-multi-agent-identities.md index 85783323d..0890b5df2 100644 --- a/docs/teams-multi-agent-identities.md +++ b/docs/teams-multi-agent-identities.md @@ -1015,9 +1015,21 @@ Tiefe Details zur Scope-Auflösung, zur Turn-Bindung und zu den Tests stehen in - **Der Schalter wirkt nur auf dem Orchestrator-Pfad.** Läuft ein Agent über den `claude-cli`-Provider, beantwortet ein `CliChatAgent` den Turn, nicht der `Orchestrator` — die Bindung wird dort nie gebildet, und der Modus bleibt folgenlos. - Ebenso greift die ACL nicht für ein **Sub-Agent**, dem das native `memory`-Tool - direkt zugeteilt wurde: dessen Handler zeigt auf den undekorierten Store. Beides ist - älter als diese Wave und in #899 dokumentiert (siehe dort den Befund im PR). + Das ist älter als diese Wave und in #899 dokumentiert (siehe dort den Befund im PR). +- **Sub-Agenten mit `memory`-Grant: seit #904 geschlossen.** Ein Sub-Agent, dem das + native `memory`-Tool zugeteilt ist, bekam seinen Handler früher aus der + prozessweiten `NativeToolRegistry` — und der gehört dem Memory-*Provider*-Plugin und + hängt am **undekorierten** Store. Damit lag der Schreibzugriff nicht nur außerhalb + der Kontext-ACL, sondern auch außerhalb der älteren Pro-Agent-Isolation + (`orchestrator::*`). Der Grant läuft jetzt über genau den turn-gebundenen, + gescopten Store, den auch der Dispatch des Elternagenten benutzt. Zwei Folgen, die + man kennen sollte: + - Der Grant war vorher faktisch **wirkungslos** (die beiden ausgelieferten + Memory-Provider registrieren handler-only, ohne Wire-Spec, und der Adapter ließ + solche Einträge fallen). Ab jetzt ist er wirksam — mit dem Scope des Elternturns. + - **Fail-closed statt Fallback:** Ist kein turn-gebundener Store da — etwa in einem + abgekoppelten `ask__start`-Runner oder außerhalb eines Orchestrator-Turns — + verweigert das Tool den Aufruf, statt auf einen weiteren Store auszuweichen. ### Was in Arbeit ist diff --git a/middleware/packages/harness-orchestrator/src/index.ts b/middleware/packages/harness-orchestrator/src/index.ts index a349be2ad..9ae475baf 100644 --- a/middleware/packages/harness-orchestrator/src/index.ts +++ b/middleware/packages/harness-orchestrator/src/index.ts @@ -259,6 +259,14 @@ export type { SubAgentGraph, SubAgentToolDeps, } from './registry/subAgentTools.js'; +// #904 — the scoped `memory` tool a granted sub-agent gets, and the tool name +// both the orchestrator's dispatch and the grant adapter key on. +export { + createScopedMemorySubAgentTool, + MEMORY_TOOL_NAME, + SUB_AGENT_MEMORY_UNBOUND_ERROR, +} from './registry/subAgentMemoryTool.js'; +export type { SubAgentMemoryResolver } from './registry/subAgentMemoryTool.js'; export { DEFAULT_ORCHESTRATOR_MODEL, resolveAgentModelRouting, @@ -525,7 +533,10 @@ export { // Teardown failures are reported, never thrown — see `runGeneratorInContext`. onTurnTeardownError, } from './turnContext.js'; -export type { TurnContextValue } from './turnContext.js'; +export type { + SubAgentMemoryHandler, + TurnContextValue, +} from './turnContext.js'; export { setMcpPrivacyBypassServers, isMcpServerPrivacyBypassed, diff --git a/middleware/packages/harness-orchestrator/src/orchestrator.ts b/middleware/packages/harness-orchestrator/src/orchestrator.ts index 059e4a745..842ae38ac 100644 --- a/middleware/packages/harness-orchestrator/src/orchestrator.ts +++ b/middleware/packages/harness-orchestrator/src/orchestrator.ts @@ -201,6 +201,7 @@ import type { } from './llmProviderSeam.js'; import { streamMessageEvents } from './streaming.js'; import { steeringBus } from './steeringBus.js'; +import { MEMORY_TOOL_NAME } from './registry/subAgentMemoryTool.js'; import { buildDateHeader, today, @@ -1129,7 +1130,10 @@ async function restoreFollowUpsForUser( return out; } -const MEMORY_TOOL_NAME = 'memory'; +// `MEMORY_TOOL_NAME` now lives in `registry/subAgentMemoryTool.ts` so the +// orchestrator's dispatch and the sub-agent grant adapter cannot drift apart +// (#904) — a sub-agent path keyed on a different literal would silently reopen +// the unscoped-store bypass. const MEMORY_TOOL_TYPE = 'memory_20250818'; const MEMORY_BETA_HEADER = 'context-management-2025-06-27'; @@ -6864,7 +6868,28 @@ export class Orchestrator { if (!this.isToolAvailable(domainTool.agentId)) { return `Error: tool \`${name}\` is unavailable — plugin \`${domainTool.agentId}\` has not completed its connection/auth setup.`; } - return domainTool.handle(input, observer); + // #904 — publish THIS turn's scoped memory handler (`memoryHandler` + // above: the turn-bound stack when one is bound, the build-time + // agent-scoped one otherwise) for the lifetime of the delegation, so a + // sub-agent granted the native `memory` tool writes through the same + // store the parent's own dispatch uses. Without it the sub-agent resolved + // `memory` from the process-wide registry, whose handler is the memory + // PROVIDER plugin's — bound to the undecorated root, i.e. outside both + // the per-agent `orchestrator::*` subtree and the chat-context ACL. + // + // Ambient here, an explicit parameter in `dispatchTool*`: `DomainTool`'s + // contract is `handle(input, observer)` and has no seam for a third + // argument. What makes that acceptable is the direction of failure — a + // lost scope makes the sub-agent's memory tool REFUSE the call + // (`SUB_AGENT_MEMORY_UNBOUND_ERROR`), it never falls back to anything + // wider. Deny on loss, never widen. + const ctx = turnContext.current(); + if (memoryHandler === undefined || ctx === undefined) { + return domainTool.handle(input, observer); + } + return turnContext.run({ ...ctx, subAgentMemoryHandler: memoryHandler }, () => + Promise.resolve(domainTool.handle(input, observer)), + ); } return `Error: unknown tool \`${name}\`.`; } diff --git a/middleware/packages/harness-orchestrator/src/registry/subAgentMemoryTool.ts b/middleware/packages/harness-orchestrator/src/registry/subAgentMemoryTool.ts new file mode 100644 index 000000000..2ae2b6c16 --- /dev/null +++ b/middleware/packages/harness-orchestrator/src/registry/subAgentMemoryTool.ts @@ -0,0 +1,108 @@ +import type { LocalSubAgentTool } from '@omadia/plugin-api'; + +import type { SubAgentMemoryHandler } from '../turnContext.js'; + +/** + * Name of the Anthropic-native memory tool. Single source of truth: the + * orchestrator's dispatch, the tool-list assembly and the sub-agent grant + * adapter all key on the same literal, and a sub-agent path that spelled it + * differently would silently reopen the hole this module closes (#904). + */ +export const MEMORY_TOOL_NAME = 'memory'; + +/** + * Resolves the memory handler bound to the turn currently delegating to this + * sub-agent, or `undefined` when there is none. + * + * `undefined` MUST mean "refuse", never "fall back to something wider" — see + * {@link createScopedMemorySubAgentTool}. + */ +export type SubAgentMemoryResolver = () => SubAgentMemoryHandler | undefined; + +/** + * Model-facing spec for a sub-agent's `memory` tool. + * + * The top-level orchestrator advertises memory as Anthropic's typed tool + * (`{type: 'memory_20250818', name: 'memory'}`), a shape `LocalSubAgentToolSpec` + * cannot express — it is a `{name, description, input_schema}` contract. So the + * six commands `MemoryToolHandler` implements are spelled out here instead. The + * HANDLER is unchanged either way: the same parser, the same store, the same + * result strings, so a sub-agent's writes are indistinguishable from the + * parent's once they reach storage. + */ +const SUB_AGENT_MEMORY_TOOL_SPEC: LocalSubAgentTool['spec'] = { + name: MEMORY_TOOL_NAME, + description: + 'Read and write the long-term memory of the agent that delegated to you. ' + + 'Paths live under /memories. Commands: view (path, optional view_range), ' + + 'create (path, file_text), str_replace (path, old_str, new_str), ' + + 'insert (path, insert_line, insert_text), delete (path), ' + + 'rename (old_path, new_path).', + input_schema: { + type: 'object', + properties: { + command: { + type: 'string', + enum: ['view', 'create', 'str_replace', 'insert', 'delete', 'rename'], + description: 'Which memory operation to perform.', + }, + path: { type: 'string', description: 'Target path under /memories.' }, + file_text: { type: 'string', description: 'File content for `create`.' }, + view_range: { + type: 'array', + items: { type: 'number' }, + description: 'Optional [start, end] line range for `view`.', + }, + old_str: { type: 'string', description: 'Text to replace, for `str_replace`.' }, + new_str: { type: 'string', description: 'Replacement text, for `str_replace`.' }, + insert_line: { type: 'number', description: 'Line to insert after, for `insert`.' }, + insert_text: { type: 'string', description: 'Text to insert, for `insert`.' }, + old_path: { type: 'string', description: 'Source path, for `rename`.' }, + new_path: { type: 'string', description: 'Destination path, for `rename`.' }, + }, + required: ['command'], + }, +}; + +/** Returned verbatim to the sub-agent's model when no turn store is bound. */ +export const SUB_AGENT_MEMORY_UNBOUND_ERROR = + 'Error: tool `memory` is unavailable — this delegation is not bound to a ' + + 'scoped memory store, and writing to the unscoped one is not permitted.'; + +/** + * The `memory` tool a sub-agent gets when an operator grants it (#904). + * + * What it deliberately does NOT do is resolve `memory` out of the process-wide + * `NativeToolRegistry`. That entry belongs to the memory PROVIDER plugin + * (`@omadia/memory`, `@omadia/memory-postgres`) and is bound to the raw root + * store — the one below every scoping wrapper. A sub-agent dispatching through + * it reads and writes outside its parent agent's `orchestrator::*` + * subtree, and, with the chat-context ACL enabled, outside its team's and + * channel's tiers as well. + * + * Instead the tool resolves the handler the PARENT's own dispatch is using for + * the turn that is delegating right now — the turn-bound stack + * `MemoryBinder.forOrigin` produced, or the build-time agent-scoped handler + * when context memory is off. Sub-agent and parent therefore share one scope by + * construction rather than by two code paths agreeing. + * + * **Fail closed.** With no bound handler the call is refused. This is the + * property that makes an ambient resolver acceptable here: `DomainTool.handle` + * takes `(input, observer)` and nothing else, so a scoped store cannot be + * threaded in as a parameter the way #903 threaded `turnMemory` through + * `dispatchTool`. A lost async context therefore denies the tool — it can never + * silently widen its reach, which is the failure mode that made this a + * vulnerability in the first place. + */ +export function createScopedMemorySubAgentTool( + resolveTurnMemory: SubAgentMemoryResolver, +): LocalSubAgentTool { + return { + spec: SUB_AGENT_MEMORY_TOOL_SPEC, + handle: async (input: unknown): Promise => { + const handler = resolveTurnMemory(); + if (handler === undefined) return SUB_AGENT_MEMORY_UNBOUND_ERROR; + return handler.handle(input); + }, + }; +} diff --git a/middleware/packages/harness-orchestrator/src/turnContext.ts b/middleware/packages/harness-orchestrator/src/turnContext.ts index 20fb78c08..d785756e3 100644 --- a/middleware/packages/harness-orchestrator/src/turnContext.ts +++ b/middleware/packages/harness-orchestrator/src/turnContext.ts @@ -270,6 +270,35 @@ export interface TurnContextValue { * so memory did reach the user either way. */ memoryFileRead?: { value: boolean }; + /** + * #904 — the memory-tool handler bound to the turn that is currently + * delegating to a sub-agent. + * + * Installed by `dispatchToolInner` in a nested scope around a SINGLE + * domain-tool dispatch, and read by the `memory` tool an operator granted to + * that sub-agent. It carries the very handler the parent's own dispatch uses + * for this turn — the turn-bound stack `MemoryBinder.forOrigin` produced, or + * the build-time agent-scoped one when context memory is off — so the + * sub-agent writes inside the same scope as its parent instead of into the + * undecorated root store the memory provider plugin registered. + * + * Undefined outside a domain-tool dispatch, and that means the sub-agent's + * memory tool REFUSES the call. Deny, never widen: this is the whole reason + * an ambient field is acceptable for a security boundary that #903 + * deliberately threaded as an explicit parameter elsewhere — losing this + * scope closes the tool, whereas losing `turnMemory` in the orchestrator + * would have silently reopened a wider store. + */ + subAgentMemoryHandler?: SubAgentMemoryHandler; +} + +/** + * Structural view of `MemoryToolHandler` (`@omadia/memory`). Declared here + * rather than imported so `turnContext` — which every layer imports — keeps its + * dependency-free shape. + */ +export interface SubAgentMemoryHandler { + handle(input: unknown): Promise; } const storage = new AsyncLocalStorage(); @@ -370,6 +399,17 @@ export const turnContext = { currentTurnDate(): string { return storage.getStore()?.turnDate ?? today(); }, + /** + * #904 — the scoped memory handler of the turn delegating to the sub-agent + * that is executing right now, or `undefined` outside a domain-tool dispatch. + * + * Callers MUST treat `undefined` as "refuse the memory call". Falling back to + * a registry-resolved handler here would restore exactly the bypass this + * accessor exists to close. + */ + currentSubAgentMemoryHandler(): SubAgentMemoryHandler | undefined { + return storage.getStore()?.subAgentMemoryHandler; + }, }; /** diff --git a/middleware/src/agents/subAgentToolHydration.ts b/middleware/src/agents/subAgentToolHydration.ts index a2ae7d4fe..cb20fa22a 100644 --- a/middleware/src/agents/subAgentToolHydration.ts +++ b/middleware/src/agents/subAgentToolHydration.ts @@ -20,10 +20,12 @@ import type { LocalSubAgentTool } from '@omadia/plugin-api'; import { buildSubAgentDomainTools, createLongRunningSubAgentTool, + createScopedMemorySubAgentTool, mcpNativeHandler, mcpToolNameFromRef, mcpToolToNativeSpec, turnContext, + MEMORY_TOOL_NAME, type DomainTool, type ResumableTaskSource, type TaskStore, @@ -35,6 +37,7 @@ import { type McpToolDescriptor, type NativeToolRegistry, type SkillRow, + type SubAgentMemoryResolver, type SkillToolBindingRow, type SubAgentRow, type ToolGrantRow, @@ -185,11 +188,41 @@ export function mcpRowToConfig(row: McpServerRow): McpServerConfig { }; } -/** Adapt a top-level native tool (handler + spec) into a sub-agent tool. */ +/** + * The production resolver: the memory handler bound to the turn that is + * delegating to this sub-agent right now, installed by + * `Orchestrator.dispatchToolInner` around the domain-tool dispatch. Outside a + * delegation it returns `undefined` and the memory tool refuses the call. + */ +export const turnScopedMemoryResolver: SubAgentMemoryResolver = () => + turnContext.currentSubAgentMemoryHandler(); + +/** + * Adapt a top-level native tool (handler + spec) into a sub-agent tool. + * + * `resolveTurnMemory` is REQUIRED, not optional, and that is the point (#904). + * The `memory` grant must never be served from `registry` — that entry is the + * memory provider plugin's handler on the UNDECORATED root store, and a + * sub-agent dispatching through it reads and writes outside its parent agent's + * `orchestrator::*` subtree and, with the chat-context ACL on, outside + * its team's and channel's tiers too. A required parameter means a new call + * site that forgets to thread the scoped store fails `typecheck` instead of + * silently degrading to the unscoped one — the same hardening #903 applied to + * `dispatchTool` / `dispatchToolDeadlined` / `dispatchToolInner`. + */ export function adaptNativeToolForSubAgent( registry: NativeToolRegistry, toolRef: string, + resolveTurnMemory: SubAgentMemoryResolver, ): LocalSubAgentTool | undefined { + // Checked BEFORE the registry lookup: `memory` must not be resolvable from + // the process-wide registry on ANY registration shape. The two shipped + // providers register handler-only (no `spec`), which the guard below happened + // to drop — an accident of spec assembly, not a boundary, and one that a + // single `register()` with a spec would have removed. + if (toolRef === MEMORY_TOOL_NAME) { + return createScopedMemorySubAgentTool(resolveTurnMemory); + } const reg = registry.get(toolRef); if (!reg?.handler || !reg.spec) return undefined; const handler = reg.handler; @@ -312,7 +345,12 @@ export function registerDbSubAgentTools( defaultMaxIterations: deps.defaultMaxIterations ?? 8, mcpManager: deps.mcpManager, mcpServersById, - nativeTool: (ref) => adaptNativeToolForSubAgent(deps.nativeToolRegistry, ref), + nativeTool: (ref) => + adaptNativeToolForSubAgent( + deps.nativeToolRegistry, + ref, + turnScopedMemoryResolver, + ), ...(deps.blockedMcpGrant ? { blockedMcpGrant: deps.blockedMcpGrant } : {}), ...(deps.hostIsCliProvider !== undefined ? { hostIsCliProvider: deps.hostIsCliProvider } diff --git a/middleware/test/orchestrator/subAgentMemoryScoping.test.ts b/middleware/test/orchestrator/subAgentMemoryScoping.test.ts new file mode 100644 index 000000000..ec1052ec5 --- /dev/null +++ b/middleware/test/orchestrator/subAgentMemoryScoping.test.ts @@ -0,0 +1,408 @@ +/** + * #904 — a sub-agent granted the native `memory` tool must write through the + * SCOPED store, never the undecorated root. + * + * The parent orchestrator's `memory` dispatch is scoped twice over: by the + * build-time `OrchestratorMemoryNamespacer` + `ScopedMemoryStore` + * (`orchestrator::*`, the per-agent boundary that predates this epic) and, + * when `context_memory` is on, by the turn-bound stack `MemoryBinder.forOrigin` + * produces. The sub-agent tool path had neither: it resolved `memory` out of the + * process-wide `NativeToolRegistry`, whose handler is the one the memory PLUGIN + * registered — bound to the raw root store. + * + * Method, taken from `contextMemoryTurnBinding.test.ts` (#903): drive a REAL + * turn — parent orchestrator delegates to a real `LocalSubAgent`, which calls + * the granted tool through its own tool loop — and assert the PHYSICAL path the + * bytes reached at the undecorated root. Every decorator sits above that + * recorder, so what arrives there is what actually hit storage; an assertion on + * a decorated store would only re-state the decorator's own arithmetic. + */ +import { describe, it } from 'node:test'; +import { strict as assert } from 'node:assert'; + +import type { + LlmProvider, + LlmResponse, + LlmStreamEvent, +} from '@omadia/llm-provider'; +import { InMemoryMemoryStore, MemoryToolHandler } from '@omadia/memory'; +import { + buildSubAgentDomainTools, + MemoryBinder, + NativeToolRegistry, + Orchestrator, + type ContextMemoryMode, +} from '@omadia/orchestrator'; + +import type { TurnOrigin } from '../../packages/harness-channel-sdk/src/turnOrigin.js'; +import type { + SkillRow, + SubAgentRow, + ToolGrantRow, +} from '../../packages/harness-orchestrator/src/registry/agentGraphStore.js'; +import { + adaptNativeToolForSubAgent, + turnScopedMemoryResolver, +} from '../../src/agents/subAgentToolHydration.js'; + +const AGENT_SLUG = 'w5-agent'; +const AGENT_ROOT = `/memories/orchestrators/${AGENT_SLUG}`; +const MEMORY_PATH = '/memories/note.md'; + +/** Same shape `omadia-channel-teams` builds beside its `sessionScope`. */ +function teamsOrigin(conversationId = 'c-1', teamId = 't-alpha'): TurnOrigin { + return { + channelType: 'teams', + scope: { kind: 'conversation', channelId: 'msteams', conversationId }, + container: { kind: 'team', id: teamId }, + }; +} + +// ── scripted provider ─────────────────────────────────────────────────────── + +const providerCapabilities = { + tools: true, + vision: false, + streaming: true, + promptCaching: false, + forcedToolChoice: false, + parallelToolCalls: false, +} as const; + +interface ScriptStep { + readonly kind: 'tool' | 'text'; + readonly name?: string; + readonly input?: unknown; + readonly text?: string; +} + +function toolStep(name: string, input: unknown): ScriptStep { + return { kind: 'tool', name, input }; +} + +function textStep(text: string): ScriptStep { + return { kind: 'text', text }; +} + +function toResponse(step: ScriptStep): LlmResponse { + const usage = { + inputTokens: 10, + outputTokens: 2, + cacheReadTokens: 0, + cacheWriteTokens: 0, + }; + if (step.kind === 'tool') { + return { + content: [ + { type: 'tool_call', id: 'tu-1', name: step.name, input: step.input }, + ], + finishReason: 'tool_calls', + providerFinishReason: 'tool_use', + model: 'test', + usage, + } as unknown as LlmResponse; + } + return { + content: [{ type: 'text', text: step.text }], + finishReason: 'stop', + providerFinishReason: 'end_turn', + model: 'test', + usage, + } as unknown as LlmResponse; +} + +/** + * Serves the same script through `complete` and `stream`, so one script drives + * both the buffered and the streaming entry point. The sub-agent runs its own + * loop against its own instance — a shared one would couple the two loops' call + * ordering to an implementation detail of the orchestrator. + */ +function scriptedProvider(steps: readonly ScriptStep[]): LlmProvider { + let idx = 0; + const take = (): ScriptStep => { + if (idx >= steps.length) { + throw new Error(`no scripted step for provider call ${String(idx + 1)}`); + } + const step = steps[idx]!; + idx += 1; + return step; + }; + return { + id: 'anthropic', + capabilities: providerCapabilities, + complete: async (): Promise => toResponse(take()), + stream: (): AsyncIterable => { + const step = take(); + return { + async *[Symbol.asyncIterator]() { + if (step.kind === 'text') { + yield { type: 'text_delta', text: step.text } as LlmStreamEvent; + } else { + yield { type: 'tool_use_start' } as LlmStreamEvent; + yield { + type: 'tool_input_delta', + text: JSON.stringify(step.input), + } as LlmStreamEvent; + } + yield { type: 'final', response: toResponse(step) } as LlmStreamEvent; + }, + }; + }, + classifyError: () => ({ retryable: false, kind: 'other' as const }), + } as unknown as LlmProvider; +} + +// ── the undecorated root ──────────────────────────────────────────────────── + +/** Records the PHYSICAL path of every write that reaches storage. */ +class RecordingMemoryStore extends InMemoryMemoryStore { + readonly writes: string[] = []; + + override async createFile(virtualPath: string, content: string): Promise { + this.writes.push(virtualPath); + await super.createFile(virtualPath, content); + } + + override async writeFile(virtualPath: string, content: string): Promise { + this.writes.push(virtualPath); + await super.writeFile(virtualPath, content); + } +} + +// ── the agent graph: one sub-agent, one native `memory` grant ─────────────── + +const SUB_AGENT_TOOL = 'ask_notetaker'; + +function subAgentRow(): SubAgentRow { + return { + id: 'sub-1', + parentAgentId: 'agent-1', + name: 'Notetaker', + skillId: null, + model: null, + maxTokens: null, + maxIterations: null, + systemPromptOverride: 'You take notes.', + status: 'enabled', + position: null, + createdAt: new Date(0), + updatedAt: new Date(0), + }; +} + +/** The supported operator action this issue is about: grant `memory`. */ +function memoryGrant(): ToolGrantRow { + return { + id: 'g-1', + agentId: null, + subAgentId: 'sub-1', + toolKind: 'native', + toolRef: 'memory', + mcpServerId: null, + config: {}, + createdAt: new Date(0), + }; +} + +const NO_SKILLS: readonly SkillRow[] = []; + +type MemoryRegistration = 'handler-only' | 'with-spec'; + +/** + * Builds the registry the way a memory provider plugin does. + * + * `handler-only` is what `@omadia/memory` / `@omadia/memory-postgres` ship + * (`ctx.tools.registerHandler('memory', …)`): the kernel emits the + * `memory_20250818` wire-spec itself, so the entry carries no `spec`. + * `with-spec` is the registry's other public registration path — same root + * handler, plus a spec. Both hand out a handler bound to the RAW root store, + * which is the whole point: whichever one the deployment uses, the sub-agent + * tool path must not route through it. + */ +function registryWithMemory( + root: RecordingMemoryStore, + shape: MemoryRegistration, +): NativeToolRegistry { + const registry = new NativeToolRegistry(); + const rootHandler = new MemoryToolHandler(root); + if (shape === 'handler-only') { + registry.registerHandler('memory', { + handler: (input: unknown) => rootHandler.handle(input), + }); + } else { + registry.register('memory', { + handler: (input: unknown) => rootHandler.handle(input), + spec: { + name: 'memory', + description: 'Read and write long-term memory.', + input_schema: { + type: 'object', + properties: { command: { type: 'string' }, path: { type: 'string' } }, + required: ['command'], + }, + }, + }); + } + return registry; +} + +interface Harness { + readonly orchestrator: Orchestrator; + /** The UNDECORATED root store — assertions read physical paths from here. */ + readonly root: RecordingMemoryStore; +} + +function harness( + mode: ContextMemoryMode, + shape: MemoryRegistration = 'handler-only', +): Harness { + const root = new RecordingMemoryStore(); + const registry = registryWithMemory(root, shape); + const subAgentProvider = scriptedProvider([ + toolStep('memory', { + command: 'create', + path: MEMORY_PATH, + file_text: 'the secret', + }), + textStep('notiert'), + ]); + const domainTools = buildSubAgentDomainTools( + { subAgents: [subAgentRow()], toolGrants: [memoryGrant()], skills: NO_SKILLS }, + { + provider: subAgentProvider, + defaultModel: 'test', + defaultMaxTokens: 1024, + defaultMaxIterations: 4, + // The production resolver, not a test double: the whole claim is that the + // grant reaches the store the PARENT's dispatch bound for this turn. + nativeTool: (ref) => + adaptNativeToolForSubAgent(registry, ref, turnScopedMemoryResolver), + }, + ); + assert.equal(domainTools.length, 1, 'expected exactly one sub-agent DomainTool'); + assert.equal(domainTools[0]!.name, SUB_AGENT_TOOL); + + const binder = new MemoryBinder({ agentSlug: AGENT_SLUG, root, mode }); + return { + root, + orchestrator: new Orchestrator({ + provider: scriptedProvider([ + toolStep(SUB_AGENT_TOOL, { question: 'merk dir das' }), + textStep('fertig'), + ]), + model: 'test', + maxTokens: 1024, + maxToolIterations: 5, + domainTools, + nativeToolRegistry: registry, + agentId: AGENT_SLUG, + // The build-time handler the parent falls back to on an unbound turn. + // Deliberately bound to the ROOT here: any path that reaches it instead + // of a scoped tier shows up in the assertions below as a raw-root write. + memoryToolHandler: new MemoryToolHandler(root), + memoryBinder: binder, + }), + }; +} + +async function drain(orchestrator: Orchestrator, origin?: TurnOrigin): Promise { + for await (const _ of orchestrator.chatStream({ + userMessage: 'los', + sessionScope: 'sess-904', + ...(origin ? { origin } : {}), + })) { + // drain + } +} + +/** The write the sub-agent performed, or a readable failure. */ +function soleWrite(root: RecordingMemoryStore): string { + assert.equal( + root.writes.length, + 1, + root.writes.length === 0 + ? 'the sub-agent produced NO write at all — the granted memory tool never reached storage' + : `expected exactly one write, got ${root.writes.join(', ')}`, + ); + return root.writes[0]!; +} + +// ── 1. per-agent isolation (predates the context ACL) ─────────────────────── + +describe('#904 sub-agent memory grant — per-agent isolation', () => { + it('MUTATION CHECK: with context memory OFF the write stays inside the agent tree', async () => { + // `off` is the shipping default, so this is the boundary the product has + // promised since long before the W5 epic: whatever an Agent notes lives + // under `orchestrator::*`, never at the raw store root where every + // other Agent's tree is reachable. + const h = harness('off'); + await drain(h.orchestrator); + const written = soleWrite(h.root); + assert.equal( + written, + `${AGENT_ROOT}/note.md`, + `sub-agent write escaped the agent tree — landed at ${written}`, + ); + }); + + it('MUTATION CHECK: a spec-carrying `memory` registration is not a way around it', async () => { + // `registerHandler` (no spec) is what the two shipped memory providers use, + // and `adaptNativeToolForSubAgent`'s spec guard happened to drop the grant + // on that shape — an accident, not a boundary. The registry's other public + // registration path carries a spec, and on that shape the sub-agent got the + // plugin's ROOT handler and wrote straight to `/memories/note.md`. + const h = harness('off', 'with-spec'); + await drain(h.orchestrator); + const written = soleWrite(h.root); + assert.notEqual( + written, + MEMORY_PATH, + 'sub-agent wrote to the UNDECORATED root — the scoping wrapper was skipped', + ); + assert.equal(written, `${AGENT_ROOT}/note.md`); + }); +}); + +// ── 2. the chat-context ACL (#881) ────────────────────────────────────────── + +describe('#904 sub-agent memory grant — chat-context ACL', () => { + it('MUTATION CHECK: under `enforce` the write lands in the turn CONTEXT tier', async () => { + const h = harness('enforce'); + await drain(h.orchestrator, teamsOrigin()); + const written = soleWrite(h.root); + assert.ok( + !written.startsWith(`${AGENT_ROOT}/`), + `sub-agent write landed in the agent-global tree (${written}) — the turn binding was lost`, + ); + assert.ok( + written.startsWith(`/memories/contexts/${AGENT_SLUG}/`), + `sub-agent write did not land in a context tier: ${written}`, + ); + }); + + it('two team contexts do not resolve to the same physical path', async () => { + const alpha = harness('enforce'); + await drain(alpha.orchestrator, teamsOrigin()); + const beta = harness('enforce'); + await drain(beta.orchestrator, teamsOrigin('c-2', 't-beta')); + assert.notEqual( + soleWrite(alpha.root), + soleWrite(beta.root), + 'two distinct team contexts resolved to the SAME physical path', + ); + }); + + it('MUTATION CHECK: buffered runTurn scopes the sub-agent write too', async () => { + const h = harness('enforce'); + await h.orchestrator.runTurn({ + userMessage: 'los', + sessionScope: 'sess-904', + origin: teamsOrigin(), + }); + const written = soleWrite(h.root); + assert.ok( + written.startsWith(`/memories/contexts/${AGENT_SLUG}/`), + `sub-agent write did not land in a context tier: ${written}`, + ); + }); +});