From ec0f6612394b0e3dc50e7f0ebeedeae95bb980cf Mon Sep 17 00:00:00 2001 From: bo Date: Mon, 10 Aug 2026 16:30:05 +0800 Subject: [PATCH 1/2] fix(agent-core): make delegation schemas provider-safe --- AGENTS.md | 6 +- ...afe-delegation-skill-contract-plan-goal.md | 184 ++++++++++++++++++ ...safe-delegation-skill-contract-progress.md | 77 ++++++++ .../src/agents/configured-agent.test.ts | 29 ++- .../agent-core/src/agents/configured-agent.ts | 16 +- packages/agent-core/src/agents/constants.ts | 1 - packages/agent-core/src/agents/errors.ts | 10 - .../agent-core/src/agents/factory-types.ts | 13 ++ .../agent-core/src/agents/factory.test.ts | 155 ++++++++++++--- packages/agent-core/src/agents/factory.ts | 81 ++++++-- packages/agent-core/src/agents/index.ts | 3 +- .../src/agents/model-tool-projection.test.ts | 124 ++++++++++++ .../src/agents/model-tool-projection.ts | 105 ++++++++++ packages/agent-core/src/agents/query/loop.ts | 3 + packages/agent-core/src/agents/query/types.ts | 1 + .../src/agents/session-agent-manager.test.ts | 2 +- .../src/delegation/contract.test.ts | 21 +- packages/agent-core/src/delegation/schema.ts | 17 +- .../session-execution-manager.test.ts | 177 ++++++++++++++++- .../execution/session-execution-manager.ts | 60 ++---- .../session-tool-batch-scheduler.test.ts | 119 +++++++++++ .../src/tools/builtins/delegate.test.ts | 48 ++++- .../agent-core/src/tools/builtins/delegate.ts | 37 +++- .../builtins/model-visible-contract.test.ts | 38 ++-- .../src/tools/builtins/skill-list.test.ts | 114 ++++++++++- .../src/tools/builtins/skill-list.ts | 26 ++- .../src/tools/builtins/skill-read.test.ts | 15 ++ .../src/tools/builtins/skill-read.ts | 19 ++ packages/agent-core/src/tools/types.ts | 11 +- 29 files changed, 1338 insertions(+), 174 deletions(-) create mode 100644 docs/goals/provider-safe-delegation-skill-contract-plan-goal.md create mode 100644 docs/goals/provider-safe-delegation-skill-contract-progress.md create mode 100644 packages/agent-core/src/agents/model-tool-projection.test.ts create mode 100644 packages/agent-core/src/agents/model-tool-projection.ts diff --git a/AGENTS.md b/AGENTS.md index f0b9efba..f54ec4d1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -390,7 +390,7 @@ All six implement `Agent`: `store: StoreApi`, `run(options) - `lead` uses `childPolicy.maxDepth = 3`; `discussion`, `analyst`, and `build` use `maxDepth = 2`. Discussion may delegate Explore/Librarian. - Lead targets Analyst/Build/Explore/Librarian; Analyst targets Explore/Librarian; Build targets Explore. - `explore` and `librarian` have no `delegateTargets`; they are terminal read-only support agents. -- `agents/factory.ts` resolves definition-based allowed tools and removes delegation capabilities at the runtime depth boundary; SessionExecutionManager enforces each role's child policy before child creation. +- `agents/factory.ts` owns one immutable current-Agent/depth delegation capability snapshot and removes delegation tools at each definition's `childPolicy.maxDepth` or when no direct target exists. Prompt/Tool projection and SessionExecutionManager admission consume that same target/Profile/builtin-Skill authority; Provider-facing Tool schemas remain portable presentation contracts while strict internal schemas still validate execution input. - `delegate` persists Agent, Profile, Skills, title, objective, and background choice. `resume_session` preserves that identity. Multiple Builds share general Session concurrency; there is no owned-scope or Build lease subsystem. **Workflow Skills:** @@ -399,8 +399,8 @@ All six implement `Agent`: `store: StoreApi`, `run(options) - `review-work` guides Lead review orchestration. Analyst analysis/review Skills include `analyze-work`, `review-change`, and the reserved `goal-review` final gate. - A Skill is one package: required `SKILL.md`; optional `scripts/`, `references/`, `assets/`, and other contained resources. Its strict YAML frontmatter accepts `name`, `description`, optional `license`, `compatibility`, and `metadata`; `description` states both method and activation timing. - Skill precedence is whole-package and strict: project `.archcode/skills//` > project `.agents/skills//` > user `~/.archcode/skills//` > user `~/.agents/skills//` > embedded builtin. Bodies and resources never merge or fall through. Reserved lifecycle builtins remain unshadowable and Agent-gated. -- Discovery (`skill_list` and available Prompt metadata) returns exactly name, description, and source. Prompt projection is bounded and reports omitted entries; `skill_list` returns digest-bound metadata pages with cursors for continuation. Entry activation (`skill_read({ name })`) returns the entry plus sorted resource descriptors; `skill_read({ name, resource })` reads exactly one listed text resource on demand. Binary assets are valid package resources but are not returned by the text-only tool. -- Invalid package candidates are surfaced as `SKILL_INVALID_PACKAGE` diagnostics. A winning invalid package fails closed; resolution never falls through to a lower-precedence package. The same winning package is claimed once for one `/skill use` logical Execution; `skill_read` uses that Execution snapshot and resume revalidates its digest. +- Discovery (`skill_list` and available Prompt metadata) returns exactly name, description, and source. Prompt projection is bounded and reports omitted entries; `skill_list` returns digest-bound metadata pages with cursors for continuation. `skill_list({ agent_type })` may inspect one currently allowed direct child's catalog for exact `delegate.skills` names, but that target page grants no parent `skill_read` authority. Entry activation (`skill_read({ name })`) returns one current-Agent entry plus sorted resource descriptors; `skill_read({ name, resource })` reads exactly one listed UTF-8 text resource on demand. Binary assets are valid package resources but are not returned by the text-only tool. +- Invalid package candidates are surfaced as `SKILL_INVALID_PACKAGE` diagnostics. A winning invalid package fails closed; resolution never falls through to a lower-precedence package. The same winning package is claimed once for one explicit `/skill use` logical Execution; an in-process resume reuses that snapshot, while process-restart recovery revalidates its persisted source/digest and fails closed on change. - Skills remain guidance only: their package metadata and resources cannot grant tools or permissions, execute scripts automatically, change Agent/Profile/MCP/workspace scope/delegation, or grant completion authority. Scripts use only existing Bash permissions. **MCP visibility**: User MCP servers are process-global and visible to all six diff --git a/docs/goals/provider-safe-delegation-skill-contract-plan-goal.md b/docs/goals/provider-safe-delegation-skill-contract-plan-goal.md new file mode 100644 index 00000000..25a6a645 --- /dev/null +++ b/docs/goals/provider-safe-delegation-skill-contract-plan-goal.md @@ -0,0 +1,184 @@ +# Provider-safe contextual delegation and Skill contract + +Status: proposed +Date: 2026-08-10 + +## Goal + +Make every model-visible `delegate`, `skill_list`, and `skill_read` contract reflect the current Agent's real delegation and Skill capabilities without exposing provider-incompatible validation rules, unbounded Skill catalogs, cross-project state, or a second Skill lifecycle. + +The work is complete only when the configured default model can execute the real Discussion, Work, direct Session, and Automation entry paths without Tool Schema rejection, while ArchCode still rejects nonexistent or unauthorized Skills before a child Session is created. + +## Current failure and evidence + +- `DelegationRequestSchema` reuses `SKILL_NAME_REGEX` in `skills.items`. Zod therefore emits `pattern: "^(?!.*--)[a-z0-9]+(?:-[a-z0-9]+)*$"` into the Provider-facing JSON Schema. +- The configured default Provider rejects the negative lookaround before the first model token with `Invalid JSON schema: regex lookaround is not supported` at `$.properties.skills.items.pattern`. +- The same internal-name rule is exposed by `skill_read.name`. +- `delegate.agent_type`, `profile`, and `skills` are static model contracts, while actual targets, Profiles, and Skill availability depend on the current Agent, delegation depth, execution cwd, target Agent definition, and live Skill sources. +- Runtime admission already validates target Agent, Profile, Skill existence, and reserved builtin policy, but that authoritative state is not projected accurately to the model. +- The current Skill control plane intentionally bounds Prompt projection and continues discovery through digest-bound `skill_list` pages. Copying the complete catalog into a Tool enum would bypass that bound. + +## Locked decisions + +1. Use bounded discovery plus authoritative local validation. Do not place the complete Skill catalog in a Provider Tool enum. +2. Provider-facing schemas use only portable structural JSON Schema. Skill name format, existence, source precedence, and Agent authorization remain internal runtime validation. +3. Extend `skill_list` with optional `agent_type`. Omitted means the current Agent; provided means one currently allowed direct child target. +4. Project and user non-reserved Skills retain their current visibility policy. Reserved and embedded builtin Skills remain Agent-gated and unshadowable. +5. `skills: []` remains a valid delegation request. +6. Preserve the existing lifecycle split: delegated and lifecycle Skills resolve live on each Execution; only explicit `/skill use` is snapshot-bound within one logical Execution. +7. One factory capability resolver owns current role/depth target and target Profile authorization. Internal Zod schemas own structure and static value domains only; they do not duplicate the authorization matrix. +8. Do not add persisted catalog ids, new Session/Execution fields, a Skill registry/cache, a second activation state machine, or new Skill snapshot semantics. +9. Replace the old model-visible schema path outright. Do not retain fallback schemas, legacy branches, compatibility aliases, or tombstone tests. + +## Architecture and ownership + +### SkillService + +`SkillService` remains the sole owner of Skill source precedence, package validation, catalog construction, diagnostics, digest-bound pagination, and package reads. It receives an Agent definition's allowed builtin names but does not learn delegation policy or import Agent modules. + +### AgentFactory and ConfiguredAgent + +`AgentFactory` remains the owner of Agent definitions and provides the single narrow capability resolver for current-depth delegate targets, target Profiles, and the target definition used to request a Skill page. Both model projection and `SessionExecutionManager` admission consume this resolver; neither re-encodes the role/depth matrix. + +`ConfiguredAgent` builds a fresh model-call-local projection at the existing model boundary. It may replace the model presentation of `delegate` and `skill_list` for that call, but it must not mutate globally registered Tool descriptors. A projected clone preserves the original `execute` implementation and changes presentation fields only. + +### Tool descriptors and Registry + +Internal `inputSchema` remains the strict ArchCode execution schema. Provider-facing `aiInputSchema` is a distinct presentation contract: + +- `delegate.skills` is an array of strings with no name regex or full catalog enum. +- `skill_read.name` is a string with no name regex or full catalog enum. +- `delegate.agent_type` contains only current-depth targets. +- `delegate.profile` exposes only Profiles reachable through those targets; the description gives the exact target-to-Profile mapping. +- `skill_list.agent_type`, when model-visible, contains only current-depth targets and is optional. + +`ToolRegistry` stays domain-neutral. It converts run-local descriptors to AI tools and does not depend on SkillService, AgentFactory, or delegation policy. Target-aware `skill_list` execution obtains a narrow capability resolver from reconstructible execution context, not from a temporary descriptor closure; normal execution and interrupted Tool Batch recovery therefore use the same authorization semantics even when the scheduler resolves the global descriptor. + +### SessionExecutionManager + +`SessionExecutionManager` remains the only child-execution admission owner. It reconstructs the capability resolver from persisted Session Agent/depth/cwd for normal execution and recovery, then immediately before child creation revalidates the target, Profile, and every requested Skill. `DelegationRequestSchema` validates only strict object structure, static value domains, Skill name format, and unknown fields. A rejected request creates no child Session, child link, or active execution. + +### Required flow + +```text +model-call boundary + -> AgentFactory resolves current-depth targets and target Profiles + -> ConfiguredAgent creates run-local delegate/skill_list presentation + -> model optionally calls skill_list({ agent_type, cursor }) + -> SkillService returns one digest-bound target catalog page + -> model copies exact returned names into delegate.skills + -> SessionExecutionManager revalidates target/Profile/Skills + -> child Session is created only after admission succeeds +``` + +## Implementation plan + +1. **Hard-cut the Provider schema boundary** + - Stop using the strict delegation and Skill-read Zod schemas directly as their model-visible schemas. + - Add explicit portable Provider-facing schemas for `delegate` and `skill_read`; retain strict internal parsing and typed runtime errors. + - Test the actual `ResolvedToolSet.toAITools()` output for `delegate`, `skill_read`, and `skill_list` across every Agent and delegation-depth boundary, proving no Skill name `pattern`, lookaround, transform, or internal refinement leaks to the Provider. + +2. **Project current delegation capabilities per model call** + - Add a narrow factory-owned resolver for current-depth direct targets and each target's permitted Profiles. + - At `ConfiguredAgent.resolveModelTools`, clone only the affected run-local descriptors and attach contextual descriptions/schemas. Never mutate Registry descriptors or cache a project-specific projection globally. + - Remove `delegate` at the existing depth boundary as today; when visible, its target enum must equal the runtime target set exactly. + - Remove the target/Profile relationship refinement from `DelegationRequestSchema`; model projection and runtime admission must obtain that authorization from the factory resolver only. + +3. **Add target-aware bounded Skill discovery** + - Extend `skill_list` input with optional `agent_type` and keep `cursor` pagination. + - Omitted target uses the current Agent definition. A provided target must be an allowed direct child at the current depth; otherwise return a stable typed error without reading an unauthorized catalog. + - Agents with no current-depth targets receive a model-facing `skill_list` schema with no `agent_type` property; do not emit an empty enum. The strict internal schema still rejects unknown fields and invalid static target values. + - Resolve target pages through the existing SkillService catalog and target definition builtin allow-list. Preserve five-tier precedence, invalid-winner isolation, reserved builtin policy, page limits, and stale-digest cursor behavior. + - Keep target authorization in a narrow execution-context resolver that can be reconstructed from Session Agent/depth/cwd. Descriptor clones change presentation only and must not capture target policy in `execute` closures. + - Update Tool descriptions to distinguish the two flows: names from `skill_list({})` may be opened by the current Agent with `skill_read`; names from `skill_list({ agent_type })` are for exact copying into `delegate.skills` and do not grant the parent Agent read access. Remove any `skill_read` wording that implies arbitrary target-page names are readable, and state that invented or stale names are rejected during delegation admission. + +4. **Keep admission authoritative and side-effect free on rejection** + - Reuse one target/Skill resolution path for delegation admission; do not introduce a second validator that can drift from Skill discovery. + - Preserve deduplication order and empty arrays. Reject invalid name syntax, missing winners, invalid winning packages, disallowed reserved builtins, invalid target Profiles, and targets outside the current role/depth matrix. + - Hard-cut known failures to this code matrix: malformed/unknown input `TOOL_SCHEMA_INVALID_INPUT`; forbidden `skill_list` target `TOOL_SKILL_TARGET_NOT_ALLOWED`; stale cursor `TOOL_SKILL_CATALOG_CHANGED`; forbidden delegate target `TOOL_DELEGATE_TARGET_NOT_ALLOWED`; target/Profile mismatch `TOOL_DELEGATE_PROFILE_NOT_ALLOWED`; missing Skill `TOOL_DELEGATE_SKILL_NOT_FOUND`; invalid winning package `TOOL_DELEGATE_SKILL_INVALID`; disallowed reserved Skill `TOOL_DELEGATE_SKILL_NOT_ALLOWED`. `TOOL_DELEGATE_FAILED` may represent only an unclassified internal execution failure, never a known admission rejection. + - Prove all rejection paths occur before child Session creation and persistence. + +5. **Update active contracts and tests** + - Update model-visible contract tests, Agent/factory tests, Skill tool tests, SessionExecutionManager tests, and active architecture documentation. Historical `docs/**` records remain unchanged except this Goal and any explicitly active architecture document. + - Add an interrupted Tool Batch recovery test proving `skill_list({ agent_type })` reconstructs the same current-role/depth authorization and never falls back to an unscoped catalog. + - Add cross-project concurrency coverage proving two simultaneous projects expose isolated Prompt catalogs and `skill_list` results while the same Agent/depth has identical Skill-name-free Tool presentation and unchanged global descriptors. + - Do not add compatibility code or tests whose only purpose is asserting that an old schema is dead. + +6. **Run automated and real-product acceptance** + - Run targeted tests while iterating, then repository `typecheck`, full `test`, `build`, and `git diff --check` in prescribed order. + - Rebuild and launch the isolated QA Worktree, reuse an isolated project fixture, and execute the configured default model through all real browser lanes defined below. + +## Acceptance criteria + +### AC-01: Provider Tool schemas are valid and portable + +- Actual `ResolvedToolSet.toAITools()` output for `delegate`, `skill_read`, and `skill_list`, across every Agent and delegation-depth boundary, contains no Skill name `pattern` and no regex lookaround. +- Their internal schemas still reject malformed Skill names and unknown fields. +- Agents with no current-depth targets still receive `skill_list`, but its model-facing schema omits `agent_type` rather than emitting an empty enum. The internal schema rejects unknown fields and values outside the static Agent domain; the contextual resolver rejects a statically valid but currently unauthorized target with `TOOL_SKILL_TARGET_NOT_ALLOWED`. +- A real configured-default-model call reaches model execution without `Invalid JSON schema`, `lookaround is not supported`, or any Tool Schema finalization error. + +### AC-02: the model sees the exact current delegation matrix + +- Lead sees Analyst, Build, Explore, and Librarian; Discussion and Analyst see Explore and Librarian; Build sees Explore; Explore and Librarian do not receive `delegate`. +- Depth exhaustion removes delegation rather than advertising unusable targets. +- Model-visible Profile guidance exactly matches the selected target rules, and runtime rejects a mismatched Profile before child creation. + +### AC-03: Skill discovery is target-aware, bounded, and current + +- `skill_list({})` returns the current Agent's digest-bound first page. +- `skill_list({ agent_type })` returns the selected allowed target's catalog using that target's builtin allow-list and the existing project/user visibility rules. +- Names returned by `skill_list({})` remain readable by the current Agent through `skill_read`. A target page is delegation discovery only: its names may be copied exactly into `delegate.skills`, but it neither grants nor implies parent-Agent `skill_read` access; model-visible descriptions state this distinction. A target-only reserved-builtin test proves the parent read is rejected while admissible delegation succeeds. +- Pagination respects existing item/byte limits; a changed catalog invalidates an old cursor with `TOOL_SKILL_CATALOG_CHANGED`. +- Neither Prompt projection nor any Provider Tool schema contains an unbounded full Skill catalog. Existing 7,999/8,000/8,001-byte Prompt boundary tests remain green. + +### AC-04: nonexistent or unauthorized Skills cannot create a child + +- A valid discovered Skill creates a child with the requested deduplicated Skill names in stable order. +- Every known rejection returns the exact code locked in the implementation plan; `delegate` does not collapse those cases into `TOOL_DELEGATE_FAILED`. +- For every rejection, no child Session file, child link, child slot, active execution, or durable delegation identity is created. +- `skills: []` still creates a valid child when the remaining request is admissible. + +### AC-05: model projections are isolated and non-persistent + +- Two concurrent projects with different custom Skills receive the correct isolated Prompt catalogs and `skill_list` pages without cross-project leakage. +- For the same Agent/depth, their model-visible Tool presentation is byte-identical and contains no project or user Skill names; target/Profile presentation may differ only when Agent/depth capabilities differ. +- Global registered descriptors are unchanged before and after model projection. +- No new Skill catalog, descriptor copy, target matrix, or Provider schema is persisted in Session or Execution records. + +### AC-06: existing Skill and delegation lifecycle semantics are preserved + +- A delegated child persists Skill names only. Each later Execution or resume resolves the current winning package; deletion or an invalid new winner fails closed without lower-precedence fallback. +- Lifecycle Skills are derived from current authoritative Session/Todo/Goal state on every Execution. +- Explicit `/skill use` alone remains snapshot-bound within one logical Execution: an in-process resume reuses the captured snapshot, while process-restart recovery reconstructs it from the persisted source/digest and fails closed if revalidation detects a change. +- Five-tier precedence, invalid/shadowed diagnostics, reserved builtin behavior, progressive resource reads, and durable delegated Agent/Profile/Skill-name/title/objective/background identity retain their existing tests and behavior. No new snapshot, fallback, or migration path exists. + +### AC-07: automated and real browser gates pass + +- `bun run typecheck`, `bun run test`, `bun run build`, and `git diff --check` all pass with zero failures. +- Using the configured default model in the isolated QA project: +- For each Discussion, Todo Work, direct Session, and Automation-created Session lane, the latest Execution status is exactly `completed`, the final assistant output is nonempty, and no execution error is recorded; `failed`, `cancelled`, `aborted`, `timed_out`, `max_steps`, or merely “terminal” do not pass. +- The direct Session completes one read-only file Tool task and displays both its finalized Tool result and final response. +- The manual Automation invocation status is exactly `dispatched`; its linked generated Session meets the same `completed`/nonempty/no-error conditions, and the invocation-to-Session link remains visible after refresh. +- Browser console errors are zero for these lanes, and Session/Automation terminal state survives a full service restart. +- If the configured default Provider is unavailable, acceptance remains `NOT_DONE`; another Provider cannot substitute for the default-model gate. + +## Non-goals + +- No Agent-level configuration UI or new permission system for custom Skills. +- No full Skill catalog enum, conditional `oneOf`/`if`/`then` schema, opaque Skill handle protocol, or extra discovery state persisted in an Execution. +- No change to Skill package format, source precedence, installation, enable/disable behavior, resource disclosure, or automatic script execution. +- No change to child concurrency, depth, cancellation, resume, Tool finalization, MCP projection, Goal, Todo, or Automation lifecycle ownership. +- No unrelated UI redesign or prototype work. + +## Risks and controls + +- **Provider dialect drift:** keep Provider schemas to portable structural fields and require a real default-model gate, not only Zod snapshot tests. +- **Model invents a name despite instructions:** runtime admission remains authoritative and side-effect free; the Tool result tells the model to refresh target discovery. +- **Catalog changes between listing and delegation:** admission resolves the current winner; stale or removed names fail explicitly. No old package fallback or silent rebinding contract is added. +- **Cross-project leakage from contextual schemas:** use model-call-local descriptor clones only and add concurrent isolation tests. +- **Coupling Tool Registry to Agent/Skill policy:** pass a narrow contextual projection into `ConfiguredAgent`; keep Registry and SkillService unaware of each other's domain policy. +- **Schema or catalog context growth:** preserve Prompt and page byte limits and never enumerate the full catalog in model-visible schemas. + +## Definition of done + +This Goal is complete only when AC-01 through AC-07 all have code, automated-test evidence, and the required real-browser evidence; the default-model blocker is no longer reproducible; no acceptance item is waived; and an independent final review reports no blocking or material architecture finding. diff --git a/docs/goals/provider-safe-delegation-skill-contract-progress.md b/docs/goals/provider-safe-delegation-skill-contract-progress.md new file mode 100644 index 00000000..1049d233 --- /dev/null +++ b/docs/goals/provider-safe-delegation-skill-contract-progress.md @@ -0,0 +1,77 @@ +# Provider-safe contextual delegation and Skill contract progress + +Status: complete +Started: 2026-08-10 +Completed: 2026-08-10 +Plan: `docs/goals/provider-safe-delegation-skill-contract-plan-goal.md` + +## Execution log + +### 2026-08-10 — baseline and decomposition + +- Confirmed execution is isolated in Worktree `/Users/bo/.codex/worktrees/system-qa/archcode` on `codex/system-qa` at `4b2be9b8`. +- Preserved the accepted Plan Goal as the normative scope; progress and evidence are recorded only in this file. +- Confirmed the only pre-existing Worktree change is the untracked Plan Goal document. +- Reconfirmed the live blocker: internal Skill-name regexes are projected into Provider JSON Schema, while delegation capability and target Skill discovery are not projected from one contextual authority. +- Locked implementation boundaries: no full Skill enum, no persisted projection/catalog state, no fallback schema, no legacy compatibility, no tombstone tests, and no new Skill lifecycle. +- Started three independent codebase investigations: Agent capability/model projection, target-aware `skill_list` recovery context, and stable delegation error mapping/test seams. +- Reproduced the Provider boundary from the current source: both `delegate.skills.items` and `skill_read.name` serialize the negative-lookaround Skill regex. + +## Decisions and corrections + +- First-principles correction: `factoryResolveAllowedTools` currently hides delegation at the global depth `3`, while Discussion, Analyst, and Build declare `childPolicy.maxDepth = 2`. The model projection and runtime admission can therefore disagree at depth 2. The accepted exact-capability criterion requires replacing the global filter with each Agent definition's own child policy; the old global behavior will not be retained. +- Recovery-safe target discovery will execute through the global `skill_list` descriptor and a capability resolver reconstructed in `ToolExecutionContext`; model-call descriptor clones will contain presentation only. +- Known child-admission exceptions will be translated narrowly at the `delegate` Tool boundary. Generic `TOOL_DELEGATE_FAILED` remains only for genuinely unclassified failures. + +## Verification evidence + +- Internal/Provider boundary reproduction before the fix: + - `delegate.skills.items.pattern = ^(?!.*--)[a-z0-9]+(?:-[a-z0-9]+)*$` + - `skill_read.name.pattern = ^(?!.*--)[a-z0-9]+(?:-[a-z0-9]+)*$` +- Parallel implementation completed: + - Factory capability authority and model-call-local `delegate`/`skill_list` presentation. + - Recovery-safe target Skill resolver in every reconstructed `ToolExecutionContext`. + - Strict internal schemas plus regex-free Provider schemas. + - Stable delegate admission error mapping. + - Fresh and resumed child admission migrated to the same capability snapshot. +- `packages/agent-core` typecheck passed after integration. +- Focused Session Agent/Execution tests passed: 119 tests, zero failures. +- Skill, scheduler, Tool contract, Factory, ConfiguredAgent, and delegation focused tests passed in their scoped runs. A combined first run exposed only sandbox-denied fixture writes plus two stale depth-error expectations; the fixture tests passed when rerun with Worktree write access, and the expectations were updated to the new capability-level rejection. +- Repository `bun run typecheck` passed: 5/5 packages, zero failures. +- Repository `bun run test` passed with exit code 0: 8/8 Turbo tasks, including Agent Core unit, integration, and architecture lanes. +- Production `bun run build` passed with exit code 0 after its required typecheck, Vite build, generated embedded-asset entrypoint, and binary compilation pipeline. +- `git diff --check` passed, and source search confirms the removed `MAX_SUB_AGENT_DEPTH` and `getDelegateTargetsFor` contracts have no remaining references. + +### 2026-08-10 — real default-model browser acceptance + +- Built and launched the isolated QA Worktree production binary on `127.0.0.1:4196` with the configured default `principal` model, `local:gpt-5.6-luna`; no alternate Provider or model was substituted. +- Temporarily isolated the global project index because unrelated registered projects contain pre-hard-cut Session records that the current strict schema rejects. The original index was restored byte-for-file after QA; the QA-only index was retained as `/Users/bo/.archcode/projects/index.provider-safe-qa-20260810.json`. The old QA fixture runtime was moved, not deleted, to `.archcode/runtime.pre-provider-safe-qa-20260810`. +- Used the real in-app browser against the rebuilt product and completed all required lanes: + - Discussion / Generate Plan: Session `cba19064-00c1-4c7b-9540-344e5a9ee73d` reached `completed`, wrote the ordinary Plan, and displayed a nonempty final response. + - Todo Ready / Start Work: Session `3dd6ec36-2158-40a7-a522-f523b2e2f99d` reached `completed`, executed `bun test` through the Agent, displayed `1 pass, 0 fail`, and returned a nonempty final response. + - Direct Session: Session `16c16944-f185-4222-81ce-591c132a7ddb` reached `completed`, displayed finalized `file_read / README.md / Completed`, and returned the exact heading plus a nonempty summary. + - Manual Automation: invocation `cad41c98-907b-4e69-82bb-517c7432d0cc` is exactly `dispatched`, links to Session `48ee77b2-71f8-4d9b-992f-b4b4f2694c13`, and that Session reached `completed` with finalized `file_read` work and a nonempty final response. +- Persisted records for all four latest Executions are exactly `completed` with `error = null`; final-answer text lengths are 567, 790, 153, and 57 characters respectively. +- Browser console errors were zero before restart and zero after restart. +- Performed a full graceful service shutdown and production-binary restart. After reload, all four Sessions remained `Completed`; the Automation remained paused with its latest invocation `Dispatched`, and the invocation-to-Session link remained visible and usable. +- The configured Provider accepted every real Tool schema; no `Invalid JSON schema`, lookaround rejection, or Tool Schema finalization error occurred. + +### 2026-08-10 — independent review and fix-review loop + +- An independent `gpt-5.6-sol` reviewer at maximum reasoning reviewed the complete Plan Goal, progress evidence, active architecture contract, production diff, tests, and browser evidence. +- First review found and fixed: + - Existing child/resume Profile admission still read `AgentDefinition.profiles`; that second authority was deleted, and recovery now validates target, Profile, and Skills from the same parent/depth capability snapshot before activation side effects. + - The global Registry contract test still treated the unprojected static `delegate` schema as contextual truth; contextual target/Profile assertions now live only in Factory/ConfiguredAgent/model-projection tests. + - The now-unreachable `DelegationToolNotAllowedError` mapping and test were hard-deleted rather than retained as a legacy branch or tombstone. +- Second review found and fixed: + - AC-05 now uses one shared `SkillService` and one concurrent `Promise.all` to prove two projects isolate both Prompt catalogs and `skill_list` pages. + - Internal/global delegation descriptions no longer copy the target-to-Profile authorization matrix; exact mapping exists only in the contextual capability projection. +- Third review found only one stale `aiInputSchema` comment; it now documents the hard-cut portable model-schema versus strict internal-schema boundary for both builtin and MCP tools. +- Final independent verdict: `PASS`, with no blocking or material finding. The reviewer independently passed 240 focused tests, the full 8/8 repository test graph, production build, and `git diff --check` on the final diff. +- Primary-agent post-review gates also passed: 168 combined delegation/Skill/recovery tests, cold-cache repository typecheck 5/5, repository test 8/8 with exit code 0, production build with exit code 0, and `git diff --check`. + +## Residual risks + +- Recovery, target-only Skill read isolation, and zero-child-artifact rejection are closed by automated tests and the independent review. +- Out-of-scope baseline finding: one registered project containing an incompatible pre-hard-cut Session currently aborts startup for the entire multi-project Runtime. This Goal does not add a migration, fallback, or compatibility branch; a separate product decision is needed if per-project quarantine is preferred over the current fail-fast behavior. +- The real browser gate exercised default-model Tool schema acceptance through all four required product entry paths, while the exact custom-Skill `skill_list → delegate` chain remains covered by automated capability, recovery, and admission tests rather than a dedicated browser run. diff --git a/packages/agent-core/src/agents/configured-agent.test.ts b/packages/agent-core/src/agents/configured-agent.test.ts index eed283c4..cf5ed280 100644 --- a/packages/agent-core/src/agents/configured-agent.test.ts +++ b/packages/agent-core/src/agents/configured-agent.test.ts @@ -13,13 +13,13 @@ import type { AnyToolDescriptor } from "../tools/types"; import { createTextToolResult } from "../tools/results"; import { createTestToolRegistryFixture, type TestToolRegistryFixture } from "../tools/test-registry"; import { worktreeEnterTool, worktreeExitTool } from "../tools/builtins/worktree"; -import { DELEGATION_CORE_TOOLS, MAX_SUB_AGENT_DEPTH } from "./constants"; +import { DELEGATION_CORE_TOOLS } from "./constants"; import { ConfiguredAgent, IneligibleSessionWorktreeToolError, UnknownExtraToolError, } from "./configured-agent"; -import { discussionAgentDefinition, exploreAgentDefinition, leadAgentDefinition } from "./definitions"; +import { defaultAgentDefinitions, discussionAgentDefinition, exploreAgentDefinition, leadAgentDefinition } from "./definitions"; import { isRootAgentName } from "./root-session-identity"; import type { AgentDefinition } from "./factory-types"; import type { VersionControl } from "../version-control/detector"; @@ -270,6 +270,20 @@ function createAgent(options: { }, }); } + const depth = options.depth ?? 0; + const canDelegate = options.definition.childPolicy !== undefined + && depth < options.definition.childPolicy.maxDepth; + const delegationTargets = canDelegate + ? (options.definition.tools.delegateTargets ?? []).map((agentName) => { + const target = defaultAgentDefinitions.find((candidate) => candidate.name === agentName); + if (target === undefined) throw new Error(`Missing test Agent definition: ${agentName}`); + return Object.freeze({ + agentName: target.name, + profiles: Object.freeze([...target.profiles]), + builtinSkillNames: Object.freeze([...target.skills]), + }); + }) + : []; return new ConfiguredAgent({ definition: options.definition, toolRegistry, @@ -287,10 +301,19 @@ function createAgent(options: { attachmentProjector: EMPTY_ATTACHMENT_MODEL_PROJECTOR, resolveAttachmentReadPaths: resolveEmptyAttachmentReadPaths, logger: options.logger ?? silentLogger, + delegationCapabilities: Object.freeze({ + parentAgentName: options.definition.name, + depth, + targets: Object.freeze(delegationTargets), + }), resolveAllowedTools: (definition, depth) => { const requested = [...definition.tools.tools, ...definition.roleContract.requiredCapabilities]; const resolved = toolRegistry.resolveForAgent(requested).descriptors.map((tool) => tool.name); - if (depth >= MAX_SUB_AGENT_DEPTH) { + if ( + definition.childPolicy === undefined + || (definition.tools.delegateTargets?.length ?? 0) === 0 + || depth >= definition.childPolicy.maxDepth + ) { return resolved.filter((name) => !(DELEGATION_CORE_TOOLS as readonly string[]).includes(name)); } return resolved; diff --git a/packages/agent-core/src/agents/configured-agent.ts b/packages/agent-core/src/agents/configured-agent.ts index 49fb229f..ca58ced5 100644 --- a/packages/agent-core/src/agents/configured-agent.ts +++ b/packages/agent-core/src/agents/configured-agent.ts @@ -31,7 +31,8 @@ import { ProjectTodoNotFoundError } from "../todos/errors"; import { TOOL_WORKTREE_ENTER, TOOL_WORKTREE_EXIT } from "../tools/names"; import type { ChildExecutionHandle, ChildExecutionRequest, ResumeChildRequest } from "../delegation/types"; import type { VersionControl, VersionControlDetector } from "../version-control/detector"; -import type { AgentDefinition, AgentMcpToolSnapshot } from "./factory-types"; +import type { AgentDefinition, AgentMcpToolSnapshot, DelegationCapabilitySnapshot } from "./factory-types"; +import { projectModelToolDescriptors } from "./model-tool-projection"; import { createAutoInjectReminderHook, createHybridCompressionHook, @@ -81,6 +82,7 @@ export interface ConfiguredAgentOptions { readonly sessionGoalService?: SessionGoalService; readonly resolveVersionControl: VersionControlDetector; readonly resolveAllowedTools: (definition: AgentDefinition, depth: number) => readonly string[]; + readonly delegationCapabilities: DelegationCapabilitySnapshot; readonly startChildExecution?: (request: ChildExecutionRequest) => Promise; readonly cancelChildSession?: (workspaceRoot: string, parentSessionId: string, childSessionId: string) => boolean; readonly resumeChildSession?: (workspaceRoot: string, request: ResumeChildRequest) => Promise; @@ -174,6 +176,7 @@ export class ConfiguredAgent implements Agent { private readonly backgroundTaskManager: BackgroundTaskManager; private readonly ownsBackgroundTaskManager: boolean; private readonly resolveAllowedTools: (definition: AgentDefinition, depth: number) => readonly string[]; + private readonly delegationCapabilities: DelegationCapabilitySnapshot; private readonly startChildExecution: ((request: ChildExecutionRequest) => Promise) | undefined; private readonly cancelChildSession: ((workspaceRoot: string, parentSessionId: string, childSessionId: string) => boolean) | undefined; private readonly resumeChildSession: ((workspaceRoot: string, request: ResumeChildRequest) => Promise) | undefined; @@ -212,6 +215,7 @@ export class ConfiguredAgent implements Agent { }); this.ownsBackgroundTaskManager = options.backgroundTaskManager === undefined; this.resolveAllowedTools = options.resolveAllowedTools; + this.delegationCapabilities = options.delegationCapabilities; this.startChildExecution = options.startChildExecution; this.cancelChildSession = options.cancelChildSession; this.resumeChildSession = options.resumeChildSession; @@ -437,6 +441,9 @@ export class ConfiguredAgent implements Agent { acquireSessionCwdTransition: this.acquireSessionCwdTransition, agentName: this.definition.name, currentDepth: this.depth, + resolveSkillListTargetSkills: (agentType: string) => this.delegationCapabilities.targets + .find((target) => target.agentName === agentType) + ?.builtinSkillNames, hooks, maxSteps: totalMaxSteps, }, @@ -537,7 +544,7 @@ export class ConfiguredAgent implements Agent { throw new Error(`Parent Session "${state.parentSessionId}" identity is unavailable while compiling the Prompt contract`); } const allowedDelegateTargets = input.allowedTools.includes("delegate") - ? [...(this.definition.tools.delegateTargets ?? [])] + ? this.delegationCapabilities.targets.map((target) => target.agentName) : []; const effectiveMaxDepth = this.definition.childPolicy?.maxDepth ?? this.depth; const runtime: RuntimePromptEnvelope = { @@ -650,7 +657,10 @@ export class ConfiguredAgent implements Agent { readonly tools: ResolvedToolSet; readonly mcpStatuses: ReadonlyMap; } { - const base = this.toolRegistry.resolveForAgent(baseAllowedTools).descriptors; + const base = projectModelToolDescriptors( + this.toolRegistry.resolveForAgent(baseAllowedTools).descriptors, + this.delegationCapabilities, + ); const mcp = this.resolveMcpToolSnapshot?.(this.definition.builtinMcpServers); const descriptors = [...base]; const names = new Set(base.map((descriptor) => descriptor.name)); diff --git a/packages/agent-core/src/agents/constants.ts b/packages/agent-core/src/agents/constants.ts index 26427dcc..5eb09cdc 100644 --- a/packages/agent-core/src/agents/constants.ts +++ b/packages/agent-core/src/agents/constants.ts @@ -15,7 +15,6 @@ export const SKILL_ACCESS_TOOLS = [TOOL_SKILL_LIST, TOOL_SKILL_READ] as const; export const DELEGATION_CORE_TOOLS = [TOOL_DELEGATE, TOOL_RESUME_SESSION, TOOL_BACKGROUND_OUTPUT, TOOL_WAIT_FOR_REMINDER] as const; export const DEFAULT_SUB_AGENT_TIMEOUT_MS = 20 * 60 * 1000; -export const MAX_SUB_AGENT_DEPTH = 3; export const MAX_CONCURRENT_SUB_AGENTS = 10; export type AgentType = AgentName; diff --git a/packages/agent-core/src/agents/errors.ts b/packages/agent-core/src/agents/errors.ts index 6ced6131..549e4c03 100644 --- a/packages/agent-core/src/agents/errors.ts +++ b/packages/agent-core/src/agents/errors.ts @@ -33,16 +33,6 @@ export class DepthLimitError extends Error { } } -export class DelegationToolNotAllowedError extends SubAgentError { - constructor( - public readonly parentAgentName: string, - public readonly currentDepth: number, - ) { - super(`Agent "${parentAgentName}" is not allowed to delegate at depth ${currentDepth}: delegate tool is unavailable`); - this.name = "DelegationToolNotAllowedError"; - } -} - export class DelegateTargetNotAllowedError extends SubAgentError { constructor( public readonly parentAgentName: string, diff --git a/packages/agent-core/src/agents/factory-types.ts b/packages/agent-core/src/agents/factory-types.ts index 53a176ac..34bfb4e7 100644 --- a/packages/agent-core/src/agents/factory-types.ts +++ b/packages/agent-core/src/agents/factory-types.ts @@ -44,3 +44,16 @@ export interface AgentChildPolicy { readonly abortCascade: boolean; readonly terminalReminders: boolean; } + +export interface DelegationTargetCapability { + readonly agentName: AgentName; + readonly profiles: readonly ProfileName[]; + readonly builtinSkillNames: readonly string[]; +} + +/** Immutable current-role/depth delegation authority derived from registered Agent definitions. */ +export interface DelegationCapabilitySnapshot { + readonly parentAgentName: AgentName; + readonly depth: number; + readonly targets: readonly DelegationTargetCapability[]; +} diff --git a/packages/agent-core/src/agents/factory.test.ts b/packages/agent-core/src/agents/factory.test.ts index f53149ef..b30449cb 100644 --- a/packages/agent-core/src/agents/factory.test.ts +++ b/packages/agent-core/src/agents/factory.test.ts @@ -5,6 +5,10 @@ import { storeManager } from "../store/store"; import type { ToolRegistry } from "../tools/registry"; import type { AnyToolDescriptor } from "../tools/types"; import { createTextToolResult } from "../tools/results"; +import { delegateTool } from "../tools/builtins/delegate"; +import { skillListTool } from "../tools/builtins/skill-list"; +import { skillReadTool } from "../tools/builtins/skill-read"; +import { ResolvedToolSet } from "../tools/registry"; import { createTestToolRegistryFixture, type TestToolRegistryFixture } from "../tools/test-registry"; import { DELEGATION_CORE_TOOLS } from "./constants"; import { SkillNotAllowedError } from "./errors"; @@ -17,7 +21,8 @@ import { import { ConfiguredAgent } from "./configured-agent"; import type { AgentDefinition, AgentName } from "./factory-types"; import { leadRoleContract } from "./definitions/role-contracts"; -import { discussionAgentDefinition } from "./definitions"; +import { defaultAgentDefinitions, discussionAgentDefinition } from "./definitions"; +import { projectModelToolDescriptors } from "./model-tool-projection"; import { silentLogger } from "../logger"; import { createTestProjectContextResolver } from "./test-project-context-resolver"; import { createTestTempRoot } from "../testing/test-temp-root"; @@ -83,6 +88,13 @@ function definition(overrides: Partial = {}): AgentDefinition { titleGeneration: "enabled", }, includeMemoryInPrompt: true, + childPolicy: { + maxDepth: 3, + maxConcurrent: 10, + timeoutMs: 1_000, + abortCascade: true, + terminalReminders: true, + }, skills: [], ...overrides, }; } @@ -90,7 +102,16 @@ function makeFactory( definitions: readonly AgentDefinition[] = [definition()], options: { skillService?: SkillService } = {}, ) { - return createAgentFactory({ definitions, + const completeDefinitions = [...definitions]; + for (const agentDefinition of definitions) { + for (const targetName of agentDefinition.tools.delegateTargets ?? []) { + if (completeDefinitions.some((candidate) => candidate.name === targetName)) continue; + const target = defaultAgentDefinitions.find((candidate) => candidate.name === targetName); + if (target === undefined) throw new Error(`Missing test target definition: ${targetName}`); + completeDefinitions.push(target); + } + } + return createAgentFactory({ definitions: completeDefinitions, toolRegistry: createTestRegistry([ makeTool("unknown_tool"), ...READ_ONLY_FIXTURE_TOOLS.map(makeTool), @@ -197,7 +218,7 @@ describe("createAgentFactory", () => { test("keeps active Skill identity on the supplied Session store", () => { const skillService = createTestSkillService(); - const factory = createAgentFactory({ definitions: [definition()], + const factory = createAgentFactory({ definitions: [definition({ childPolicy: undefined })], toolRegistry: createTestRegistry([ makeTool("unknown_tool"), ...READ_ONLY_FIXTURE_TOOLS.map(makeTool), @@ -261,12 +282,21 @@ describe("createAgentFactory", () => { expect(child.store.getState().parentSessionId).toBe(parentSessionId); }); - test("resolves explicit tool lists and strips delegation tools at depth three", () => { + test("resolves explicit tool lists and strips delegation at the definition boundary", () => { const factory = makeFactory(); - const customDefinition = definition({ tools: { tools: ["grep", "missing", "delegate"] } }); + const customDefinition = definition({ + tools: { tools: ["grep", "missing", "delegate"], delegateTargets: ["explore"] }, + childPolicy: { + maxDepth: 2, + maxConcurrent: 10, + timeoutMs: 1_000, + abortCascade: true, + terminalReminders: true, + }, + }); const delegatingDefinition = definition({ name: "lead", - tools: { tools: ["unknown_tool", ...explorerTools] }, + tools: { tools: ["unknown_tool", ...explorerTools], delegateTargets: ["explore"] }, }); expect(factory.resolveAllowedTools(definition(), 0)).toEqual([ @@ -275,10 +305,8 @@ describe("createAgentFactory", () => { ...DELEGATION_CORE_TOOLS, ]); expect(factory.resolveAllowedTools(customDefinition, 0)).toEqual(["grep", "delegate"]); - // depth < MAX_SUB_AGENT_DEPTH (3): delegation tools still present - expect(factory.resolveAllowedTools(customDefinition, 2)).toEqual(["grep", "delegate"]); - // depth >= MAX_SUB_AGENT_DEPTH (3): delegation tools stripped - expect(factory.resolveAllowedTools(customDefinition, 3)).toEqual(["grep"]); + expect(factory.resolveAllowedTools(customDefinition, 1)).toEqual(["grep", "delegate"]); + expect(factory.resolveAllowedTools(customDefinition, 2)).toEqual(["grep"]); expect(factory.resolveAllowedTools(delegatingDefinition, 1)).toEqual([ "unknown_tool", ...READ_ONLY_FIXTURE_TOOLS, @@ -292,36 +320,81 @@ describe("createAgentFactory", () => { ]); // depth 3 (>= 3): delegation tools stripped expect(factory.resolveAllowedTools(delegatingDefinition, 3)).toEqual(["unknown_tool", ...READ_ONLY_FIXTURE_TOOLS]); + expect(factory.resolveAllowedTools(definition({ childPolicy: undefined }), 0)).toEqual([ + "unknown_tool", + ...READ_ONLY_FIXTURE_TOOLS, + ]); + expect(factory.resolveAllowedTools(definition({ + tools: { tools: ["grep", "delegate"] }, + }), 0)).toEqual(["grep"]); }); - test("resolves delegate targets only when depth allows delegation", () => { - const factory = makeFactory(); - const depthFilteredDefinition = definition({ - name: "explore", - tools: { tools: explorerTools, delegateTargets: ["explore", "analyst"] }, - }); - const explicitWithoutDelegate = definition({ - name: "analyst", - tools: { tools: ["grep"], delegateTargets: ["explore"] }, - }); - - expect(factory.getDelegateTargetsFor(depthFilteredDefinition, 1)).toEqual(["explore", "analyst"]); - // depth 2 (< MAX_SUB_AGENT_DEPTH=3): delegation still allowed, targets returned - expect(factory.getDelegateTargetsFor(depthFilteredDefinition, 2)).toEqual(["explore", "analyst"]); - // depth 3 (>= MAX_SUB_AGENT_DEPTH): delegation stripped, targets empty - expect(factory.getDelegateTargetsFor(depthFilteredDefinition, 3)).toEqual([]); - expect(factory.getDelegateTargetsFor(explicitWithoutDelegate, 0)).toEqual([]); + test("resolves one immutable capability matrix from canonical registered definitions", () => { + const factory = makeFactory(defaultAgentDefinitions); + const cases = [ + ["lead", 0, ["analyst", "build", "explore", "librarian"]], + ["lead", 2, ["analyst", "build", "explore", "librarian"]], + ["lead", 3, []], + ["discussion", 0, ["explore", "librarian"]], + ["discussion", 2, []], + ["analyst", 0, ["explore", "librarian"]], + ["analyst", 2, []], + ["build", 0, ["explore"]], + ["build", 2, []], + ["explore", 0, []], + ["librarian", 0, []], + ] as const; + + for (const [parent, depth, expectedTargets] of cases) { + const snapshot = factory.resolveDelegationCapabilities(parent, depth); + expect(snapshot.targets.map((target) => target.agentName)).toEqual([...expectedTargets]); + expect(Object.isFrozen(snapshot)).toBe(true); + expect(Object.isFrozen(snapshot.targets)).toBe(true); + for (const target of snapshot.targets) { + const canonical = factory.getDefinition(target.agentName); + expect(target.profiles).toEqual(canonical.profiles); + expect(target.builtinSkillNames).toEqual(canonical.skills); + expect(Object.isFrozen(target)).toBe(true); + expect(Object.isFrozen(target.profiles)).toBe(true); + expect(Object.isFrozen(target.builtinSkillNames)).toBe(true); + } + + const descriptors = expectedTargets.length === 0 + ? [skillListTool, skillReadTool] + : [delegateTool, skillListTool, skillReadTool]; + const aiTools = new ResolvedToolSet( + projectModelToolDescriptors(descriptors, snapshot), + ).toAITools(); + const listSchema = z.toJSONSchema(aiTools.skill_list!.inputSchema as z.ZodType) as { + readonly properties: Record; + }; + if (expectedTargets.length === 0) { + expect(listSchema.properties.agent_type).toBeUndefined(); + expect(aiTools.delegate).toBeUndefined(); + } else { + expect(listSchema.properties.agent_type?.enum).toEqual([...expectedTargets]); + const delegateSchema = z.toJSONSchema(aiTools.delegate!.inputSchema as z.ZodType) as { + readonly properties: Record; + }; + expect(delegateSchema.properties.agent_type?.enum).toEqual([...expectedTargets]); + expect(delegateSchema.properties.profile?.enum).toEqual([ + ...new Set(snapshot.targets.flatMap((target) => target.profiles)), + ]); + } + } }); test("validates and deduplicates delegated Skill names before persistence", async () => { - const target = definition({ name: "explore", tools: { tools: nonDelegatingExplorerTools }, skills: ["codemap", "git-master"] }); - const factory = makeFactory([definition(), target], { skillService: createSkillServiceWithBuiltins() }); + const target = definition({ name: "explore", tools: { tools: nonDelegatingExplorerTools }, skills: ["codemap", "git-master"], childPolicy: undefined }); + const parent = definition({ tools: { tools: explorerTools, delegateTargets: ["explore"] } }); + const factory = makeFactory([parent, target], { skillService: createSkillServiceWithBuiltins() }); + const targetCapability = factory.resolveDelegationCapabilities("lead", 0).targets[0]!; - const skillNames = await factory.resolveDelegatedSkillNames(target, ["codemap", "git-master", "codemap"], import.meta.dir); + const skillNames = await factory.resolveDelegatedSkillNames(targetCapability, ["codemap", "git-master", "codemap"], import.meta.dir); expect(skillNames).toEqual(["codemap", "git-master"]); try { - await factory.resolveDelegatedSkillNames(target, ["run-goal"], import.meta.dir); + await factory.resolveDelegatedSkillNames(targetCapability, ["run-goal"], import.meta.dir); throw new Error("Expected delegated Skill validation to fail"); } catch (error) { expect(error).toBeInstanceOf(SkillNotAllowedError); @@ -332,6 +405,26 @@ describe("createAgentFactory", () => { }); } }); + + test("admits a target-only reserved builtin through the target capability", async () => { + const factory = makeFactory(defaultAgentDefinitions, { + skillService: new SkillService({ + userSkillsRoot: `${TEST_WORKSPACE_ROOT}/missing-user-skills`, + userAgentsSkillsRoot: `${TEST_WORKSPACE_ROOT}/missing-user-agent-skills`, + }), + }); + const lead = factory.getDefinition("lead"); + const analyst = factory.resolveDelegationCapabilities("lead", 0).targets + .find((target) => target.agentName === "analyst"); + + expect(lead.skills).not.toContain("goal-review"); + expect(analyst?.builtinSkillNames).toContain("goal-review"); + await expect(factory.resolveDelegatedSkillNames( + analyst!, + ["goal-review"], + TEST_WORKSPACE_ROOT, + )).resolves.toEqual(["goal-review"]); + }); }); describe("factoryResolveAllowedTools static base-tool projection", () => { diff --git a/packages/agent-core/src/agents/factory.ts b/packages/agent-core/src/agents/factory.ts index e5329a43..38313d60 100644 --- a/packages/agent-core/src/agents/factory.ts +++ b/packages/agent-core/src/agents/factory.ts @@ -12,8 +12,14 @@ import { ConfiguredAgent } from "./configured-agent"; import { SkillNotAllowedError } from "./errors"; import type { StoreApi } from "zustand"; import type { ChildExecutionHandle, ChildExecutionRequest, ResumeChildRequest } from "../delegation/types"; -import type { AgentDefinition, AgentMcpToolSnapshot, AgentName } from "./factory-types"; -import { DELEGATION_CORE_TOOLS, MAX_SUB_AGENT_DEPTH } from "./constants"; +import type { + AgentDefinition, + AgentMcpToolSnapshot, + AgentName, + DelegationCapabilitySnapshot, + DelegationTargetCapability, +} from "./factory-types"; +import { DELEGATION_CORE_TOOLS } from "./constants"; import type { Agent } from "./types"; import { detectVersionControl, type VersionControlDetector } from "../version-control/detector"; import type { ToolOutputAccessService } from "../tool-output/access-service"; @@ -59,8 +65,8 @@ export interface AgentFactory { getDefinition(name: string): AgentDefinition; listAgentNames(): string[]; resolveAllowedTools(definition: AgentDefinition, depth: number): string[]; - getDelegateTargetsFor(definition: AgentDefinition, depth: number): string[]; - resolveDelegatedSkillNames(targetDefinition: AgentDefinition, requestedSkills: readonly string[], cwd: string): Promise; + resolveDelegationCapabilities(parentAgentName: AgentName, depth: number): DelegationCapabilitySnapshot; + resolveDelegatedSkillNames(target: DelegationTargetCapability, requestedSkills: readonly string[], cwd: string): Promise; } export class DuplicateAgentDefinitionError extends Error { @@ -107,11 +113,21 @@ export function createAgentFactory(config: AgentFactoryConfig): AgentFactory { agentName: definition.name, source: { kind: "direct" }, }); - return createConfiguredAgent(rootConfig, definition, { ...options, store }); + return createConfiguredAgent( + rootConfig, + definition, + { ...options, store }, + factory.resolveDelegationCapabilities, + ); }, createAgent(name, options = {}) { - return createConfiguredAgent(agentConfig, factory.getDefinition(name), options); + return createConfiguredAgent( + agentConfig, + factory.getDefinition(name), + options, + factory.resolveDelegationCapabilities, + ); }, getDefinition(name) { @@ -130,17 +146,26 @@ export function createAgentFactory(config: AgentFactoryConfig): AgentFactory { return factoryResolveAllowedTools(config, definition, depth); }, - getDelegateTargetsFor(definition, depth) { + resolveDelegationCapabilities(parentAgentName, depth) { + const definition = factory.getDefinition(parentAgentName); const allowedTools = factory.resolveAllowedTools(definition, depth); if (!allowedTools.includes("delegate")) { - return []; + return freezeDelegationCapabilities(parentAgentName, depth, []); } - return [...(definition.tools.delegateTargets ?? [])]; + const targets = (definition.tools.delegateTargets ?? []).map((agentName) => { + const target = factory.getDefinition(agentName); + return { + agentName: target.name, + profiles: target.profiles, + builtinSkillNames: target.skills, + }; + }); + return freezeDelegationCapabilities(parentAgentName, depth, targets); }, - resolveDelegatedSkillNames(targetDefinition, requestedSkills, cwd) { - return resolveDelegatedSkillNames(agentConfig.skillService, cwd, targetDefinition, requestedSkills); + resolveDelegatedSkillNames(target, requestedSkills, cwd) { + return resolveDelegatedSkillNames(agentConfig.skillService, cwd, target, requestedSkills); }, }; @@ -150,7 +175,7 @@ export function createAgentFactory(config: AgentFactoryConfig): AgentFactory { async function resolveDelegatedSkillNames( skillService: SkillService, workspaceRoot: string, - targetDefinition: AgentDefinition, + target: DelegationTargetCapability, requestedSkills: readonly string[], ): Promise { const dedupedNames: string[] = []; @@ -158,8 +183,8 @@ async function resolveDelegatedSkillNames( for (const skillName of requestedSkills) { assertSkillName(skillName); - if (RESERVED_BUILTIN_SKILL_NAMES.has(skillName) && !targetDefinition.skills.includes(skillName)) { - throw new SkillNotAllowedError(targetDefinition.name, skillName, targetDefinition.skills); + if (RESERVED_BUILTIN_SKILL_NAMES.has(skillName) && !target.builtinSkillNames.includes(skillName)) { + throw new SkillNotAllowedError(target.agentName, skillName, target.builtinSkillNames); } if (seen.has(skillName)) continue; seen.add(skillName); @@ -167,7 +192,7 @@ async function resolveDelegatedSkillNames( } for (const skillName of dedupedNames) { - const skill = await skillService.discoverForAgent(workspaceRoot, skillName, targetDefinition.skills); + const skill = await skillService.discoverForAgent(workspaceRoot, skillName, target.builtinSkillNames); if (skill === null) { throw new SkillNotFoundError(skillName); } @@ -180,8 +205,10 @@ function createConfiguredAgent( config: AgentFactoryConfig, definition: AgentDefinition, options: CreateAgentOptions, + resolveDelegationCapabilities: AgentFactory["resolveDelegationCapabilities"], ): Agent { const store = prepareStore(config, definition, options); + const delegationCapabilities = resolveDelegationCapabilities(definition.name, options.depth ?? 0); return new ConfiguredAgent({ definition, @@ -203,6 +230,7 @@ function createConfiguredAgent( ...(config.sessionGoalService === undefined ? {} : { sessionGoalService: config.sessionGoalService }), resolveVersionControl: config.versionControlDetector ?? detectVersionControl, logger: config.logger, + delegationCapabilities, resolveAllowedTools: (agentDefinition, depth) => factoryResolveAllowedTools(config, agentDefinition, depth), startChildExecution: config.startChildExecution, cancelChildSession: config.cancelChildSession, @@ -219,13 +247,34 @@ function factoryResolveAllowedTools( ): string[] { const all = config.toolRegistry.resolveForAgent(definition.tools.tools).descriptors.map((tool) => tool.name); - if (depth >= MAX_SUB_AGENT_DEPTH) { + if ( + definition.childPolicy === undefined + || (definition.tools.delegateTargets?.length ?? 0) === 0 + || depth >= definition.childPolicy.maxDepth + ) { return all.filter((name) => !(DELEGATION_CORE_TOOLS as readonly string[]).includes(name)); } return all; } +function freezeDelegationCapabilities( + parentAgentName: AgentName, + depth: number, + targets: readonly DelegationTargetCapability[], +): DelegationCapabilitySnapshot { + const frozenTargets = targets.map((target) => Object.freeze({ + agentName: target.agentName, + profiles: Object.freeze([...target.profiles]), + builtinSkillNames: Object.freeze([...target.builtinSkillNames]), + })); + return Object.freeze({ + parentAgentName, + depth, + targets: Object.freeze(frozenTargets), + }); +} + function prepareStore(config: AgentFactoryConfig, definition: AgentDefinition, options: CreateAgentOptions): StoreApi { const store = options.store ?? config.storeManager.create(crypto.randomUUID(), config.workspaceRoot, { agentName: definition.name, diff --git a/packages/agent-core/src/agents/index.ts b/packages/agent-core/src/agents/index.ts index 6355f490..16b3ef01 100644 --- a/packages/agent-core/src/agents/index.ts +++ b/packages/agent-core/src/agents/index.ts @@ -3,7 +3,6 @@ export { DEFAULT_SUB_AGENT_TIMEOUT_MS, DELEGATION_CORE_TOOLS, MAX_CONCURRENT_SUB_AGENTS, - MAX_SUB_AGENT_DEPTH, SKILL_ACCESS_TOOLS, } from "./constants"; export type { AgentType } from "./constants"; @@ -21,6 +20,8 @@ export type { AgentHookPolicy, AgentName, AgentToolPolicy, + DelegationCapabilitySnapshot, + DelegationTargetCapability, } from "./factory-types"; export { AGENT_NAMES } from "./names"; export { diff --git a/packages/agent-core/src/agents/model-tool-projection.test.ts b/packages/agent-core/src/agents/model-tool-projection.test.ts new file mode 100644 index 00000000..34294d5b --- /dev/null +++ b/packages/agent-core/src/agents/model-tool-projection.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, test } from "bun:test"; +import { z } from "zod/v4"; +import { delegateTool } from "../tools/builtins/delegate"; +import { skillListTool } from "../tools/builtins/skill-list"; +import { skillReadTool } from "../tools/builtins/skill-read"; +import { ResolvedToolSet } from "../tools/registry"; +import type { DelegationCapabilitySnapshot } from "./factory-types"; +import { projectModelToolDescriptors } from "./model-tool-projection"; + +function capabilities( + targets: DelegationCapabilitySnapshot["targets"], +): DelegationCapabilitySnapshot { + return Object.freeze({ + parentAgentName: "lead", + depth: 0, + targets: Object.freeze(targets), + }); +} + +function jsonSchema(value: unknown): Record { + if (typeof value === "object" && value !== null && "jsonSchema" in value) { + return (value as { readonly jsonSchema: Record }).jsonSchema; + } + return z.toJSONSchema(value as z.ZodType) as Record; +} + +describe("projectModelToolDescriptors", () => { + test("projects portable schemas through the actual ResolvedToolSet boundary", () => { + const snapshot = capabilities([ + Object.freeze({ + agentName: "analyst", + profiles: Object.freeze(["deep"] as const), + builtinSkillNames: Object.freeze(["analyze-work"]), + }), + Object.freeze({ + agentName: "build", + profiles: Object.freeze(["deep", "fast"] as const), + builtinSkillNames: Object.freeze(["safe-refactor"]), + }), + Object.freeze({ + agentName: "explore", + profiles: Object.freeze(["fast"] as const), + builtinSkillNames: Object.freeze(["codemap"]), + }), + ]); + const originals = [delegateTool, skillListTool, skillReadTool] as const; + const originalState = originals.map((descriptor) => ({ + inputSchema: descriptor.inputSchema, + aiInputSchema: descriptor.aiInputSchema, + execute: descriptor.execute, + description: descriptor.description, + })); + + const projected = projectModelToolDescriptors(originals, snapshot); + const aiTools = new ResolvedToolSet(projected).toAITools(); + const schemas = Object.fromEntries(Object.entries(aiTools).map(([name, tool]) => [ + name, + jsonSchema(tool.inputSchema), + ])); + const serialized = JSON.stringify(schemas); + + expect(serialized).not.toContain("pattern"); + expect(serialized).not.toContain("lookahead"); + expect(serialized).not.toContain("lookaround"); + expect(schemas.delegate).toMatchObject({ + properties: { + agent_type: { enum: ["analyst", "build", "explore"] }, + profile: { enum: ["deep", "fast"] }, + skills: { type: "array", items: { type: "string" } }, + }, + additionalProperties: false, + }); + expect(schemas.skill_list).toMatchObject({ + properties: { + agent_type: { enum: ["analyst", "build", "explore"] }, + }, + additionalProperties: false, + }); + expect((aiTools.delegate?.description ?? "")).toContain( + "analyst=deep, build=deep|fast, explore=fast", + ); + const repeatedAiTools = new ResolvedToolSet( + projectModelToolDescriptors(originals, snapshot), + ).toAITools(); + for (const name of Object.keys(aiTools)) { + expect(JSON.stringify(jsonSchema(repeatedAiTools[name]!.inputSchema))).toBe( + JSON.stringify(jsonSchema(aiTools[name]!.inputSchema)), + ); + expect(repeatedAiTools[name]!.description).toBe(aiTools[name]!.description); + } + expect(JSON.stringify(aiTools)).not.toContain("analyze-work"); + expect(JSON.stringify(aiTools)).not.toContain("safe-refactor"); + expect(JSON.stringify(aiTools)).not.toContain("codemap"); + + projected.forEach((descriptor, index) => { + expect(descriptor.inputSchema).toBe(originalState[index]!.inputSchema); + expect(descriptor.execute).toBe(originalState[index]!.execute); + }); + originals.forEach((descriptor, index) => { + expect(descriptor.aiInputSchema).toBe(originalState[index]!.aiInputSchema); + expect(descriptor.description).toBe(originalState[index]!.description); + }); + }); + + test("omits skill_list.agent_type when the current depth has no targets", () => { + const projected = projectModelToolDescriptors( + [skillListTool, skillReadTool], + capabilities([]), + ); + const aiTools = new ResolvedToolSet(projected).toAITools(); + const listSchema = jsonSchema(aiTools.skill_list!.inputSchema); + const properties = listSchema.properties as Record; + + expect(properties.agent_type).toBeUndefined(); + expect(properties.cursor).toBeDefined(); + expect(JSON.stringify(jsonSchema(aiTools.skill_read!.inputSchema))).not.toContain("pattern"); + }); + + test("fails closed if delegate is visible without an allowed target", () => { + expect(() => projectModelToolDescriptors([delegateTool], capabilities([]))).toThrow( + "delegate is model-visible", + ); + }); +}); diff --git a/packages/agent-core/src/agents/model-tool-projection.ts b/packages/agent-core/src/agents/model-tool-projection.ts new file mode 100644 index 00000000..665ab716 --- /dev/null +++ b/packages/agent-core/src/agents/model-tool-projection.ts @@ -0,0 +1,105 @@ +import { z } from "zod/v4"; +import { TOOL_DELEGATE, TOOL_SKILL_LIST } from "../tools/names"; +import type { AnyToolDescriptor } from "../tools/types"; +import type { DelegationCapabilitySnapshot } from "./factory-types"; + +export function projectModelToolDescriptors( + descriptors: readonly AnyToolDescriptor[], + capabilities: DelegationCapabilitySnapshot, +): readonly AnyToolDescriptor[] { + return descriptors.map((descriptor) => { + if (descriptor.name === TOOL_DELEGATE) { + return projectDelegateDescriptor(descriptor, capabilities); + } + if (descriptor.name === TOOL_SKILL_LIST) { + return projectSkillListDescriptor(descriptor, capabilities); + } + return descriptor; + }); +} + +function projectDelegateDescriptor( + descriptor: AnyToolDescriptor, + capabilities: DelegationCapabilitySnapshot, +): AnyToolDescriptor { + if (capabilities.targets.length === 0) { + throw new Error( + `delegate is model-visible for ${capabilities.parentAgentName} at depth ${capabilities.depth} without an allowed target`, + ); + } + const targetNames = capabilities.targets.map((target) => target.agentName); + const profiles = unique(capabilities.targets.flatMap((target) => target.profiles)); + const profileMapping = capabilities.targets + .map((target) => `${target.agentName}=${target.profiles.join("|")}`) + .join(", "); + + return { + ...descriptor, + description: [ + "Create one direct child Session using the current role/depth delegation authority.", + `Allowed target-to-Profile mapping: ${profileMapping}.`, + "Discover target Skills with skill_list({ agent_type }), then copy only exact returned names into skills. An empty skills array is valid; invented, stale, missing, invalid, or unauthorized names are rejected before child creation.", + "The objective must be a self-contained handoff because the child does not inherit the parent conversation.", + ].join("\n"), + aiInputSchema: z.strictObject({ + agent_type: z.enum(asNonEmptyEnum(targetNames)).describe( + "Allowed direct child Agent identity at the current delegation depth.", + ), + profile: z.enum(asNonEmptyEnum(profiles)).describe( + `Allowed Profile union for visible targets. Exact target mapping: ${profileMapping}.`, + ), + title: z.string().min(1).describe("Short user-facing title for the child Session."), + objective: z.string().min(1).describe("Self-contained task-specific handoff for the fresh child."), + skills: z.array(z.string()).describe( + "Exact target Skill names copied from skill_list({ agent_type }); use [] when no workflow Skill is needed.", + ), + background: z.boolean().describe( + "False waits for the child final output; true returns its Session ID for later retrieval.", + ), + }), + }; +} + +function projectSkillListDescriptor( + descriptor: AnyToolDescriptor, + capabilities: DelegationCapabilitySnapshot, +): AnyToolDescriptor { + const shared = { + cursor: z.string().min(1).optional().describe( + "Opaque cursor from a previous page; omit for the first current catalog page.", + ), + }; + const aiInputSchema = capabilities.targets.length === 0 + ? z.strictObject(shared) + : z.strictObject({ + ...shared, + agent_type: z.enum(asNonEmptyEnum(capabilities.targets.map((target) => target.agentName))) + .optional() + .describe( + "Optional allowed direct child target. Omit for the current Agent catalog; provide a target only to discover exact names for delegate.skills.", + ), + }); + + return { + ...descriptor, + description: [ + "List one bounded, digest-bound page of current Skill metadata.", + "Call skill_list({}) for the current Agent; those exact names may be opened with skill_read.", + ...(capabilities.targets.length === 0 + ? [] + : ["Call skill_list({ agent_type }) for an allowed direct child only when selecting names for delegate.skills. A target page grants no parent read access."]), + ].join("\n"), + aiInputSchema, + }; +} + +function unique(values: readonly T[]): T[] { + return [...new Set(values)]; +} + +function asNonEmptyEnum( + values: readonly T[], +): [T, ...T[]] { + if (values.length === 0) throw new Error("Model Tool enum must not be empty"); + return [...values] as [T, ...T[]]; +} diff --git a/packages/agent-core/src/agents/query/loop.ts b/packages/agent-core/src/agents/query/loop.ts index 0e8f4226..6028666f 100644 --- a/packages/agent-core/src/agents/query/loop.ts +++ b/packages/agent-core/src/agents/query/loop.ts @@ -384,6 +384,9 @@ export async function runQueryLoop( attachmentReadPaths, agentSkills: options.agentSkills.filter((skill) => persistedSkills.has(skill)), skillService: options.skillService, + ...(options.resolveSkillListTargetSkills === undefined + ? {} + : { resolveSkillListTargetSkills: options.resolveSkillListTargetSkills }), ...(options.executionSkillSnapshots === undefined ? {} : { executionSkillSnapshots: options.executionSkillSnapshots }), diff --git a/packages/agent-core/src/agents/query/types.ts b/packages/agent-core/src/agents/query/types.ts index 7c40816f..3f294e79 100644 --- a/packages/agent-core/src/agents/query/types.ts +++ b/packages/agent-core/src/agents/query/types.ts @@ -28,6 +28,7 @@ export interface QueryLoopOptions { allowedTools: readonly string[]; agentSkills: readonly string[]; skillService: SkillService; + resolveSkillListTargetSkills?: (agentType: string) => readonly string[] | undefined; executionSkillSnapshots?: ReadonlyMap; storeManager: SessionStoreManager; /** Required model-boundary attachment projection; never inferred from provider identity. */ diff --git a/packages/agent-core/src/agents/session-agent-manager.test.ts b/packages/agent-core/src/agents/session-agent-manager.test.ts index f8754883..4b0a7bc9 100644 --- a/packages/agent-core/src/agents/session-agent-manager.test.ts +++ b/packages/agent-core/src/agents/session-agent-manager.test.ts @@ -403,7 +403,7 @@ describe("SessionAgentManager", () => { return { depth, allowedTools: [...context.allowedTools].sort(), - delegateTargets: factory.getDelegateTargetsFor(definition, depth), + delegateTargets: factory.resolveDelegationCapabilities(definition.name, depth).targets.map((target) => target.agentName), activeSkillNames: [...agent.store.getState().activeSkillNames], hasActiveSkillBody: prompt.includes(IDENTITY_SKILL_BODY), }; diff --git a/packages/agent-core/src/delegation/contract.test.ts b/packages/agent-core/src/delegation/contract.test.ts index 1771ae9c..4ada5260 100644 --- a/packages/agent-core/src/delegation/contract.test.ts +++ b/packages/agent-core/src/delegation/contract.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { z } from "zod/v4"; import { DelegationRequestSchema } from "./schema"; function request(overrides: Record = {}): Record { @@ -35,19 +36,33 @@ describe("DelegationRequestSchema", () => { } }); - test("enforces the target Profile matrix before child creation", () => { + test("accepts the static delegated Profile domain without owning target authorization", () => { expect(DelegationRequestSchema.parse(request({ agent_type: "analyst", profile: "deep" })).profile).toBe("deep"); - expect(() => DelegationRequestSchema.parse(request({ agent_type: "analyst", profile: "fast" }))).toThrow(); + expect(DelegationRequestSchema.parse(request({ agent_type: "analyst", profile: "fast" })).profile).toBe("fast"); expect(DelegationRequestSchema.parse(request({ agent_type: "build", profile: "fast" })).profile).toBe("fast"); expect(DelegationRequestSchema.parse(request({ agent_type: "build", profile: "deep" })).profile).toBe("deep"); for (const agent_type of ["explore", "librarian"]) { expect(DelegationRequestSchema.parse(request({ agent_type, profile: "fast" })).profile).toBe("fast"); - expect(() => DelegationRequestSchema.parse(request({ agent_type, profile: "deep" }))).toThrow(); + expect(DelegationRequestSchema.parse(request({ agent_type, profile: "deep" })).profile).toBe("deep"); } expect(() => DelegationRequestSchema.parse(request({ profile: "principal" }))).toThrow(); expect(() => DelegationRequestSchema.parse(request({ profile: "visual" }))).toThrow(); }); + test("describes static values while leaving target and Profile authorization to runtime capabilities", () => { + const schema = z.toJSONSchema(DelegationRequestSchema) as { + readonly properties: Record; + }; + const agentDescription = schema.properties.agent_type?.description ?? ""; + const profileDescription = schema.properties.profile?.description ?? ""; + + expect(agentDescription).toContain("Delegated child Agent identity value"); + expect(agentDescription).toContain("parent/depth capability admission"); + expect(profileDescription).toContain("Delegated model-resource Profile value"); + expect(profileDescription).toContain("selected Agent/Profile pair is authorized"); + expect(profileDescription).not.toMatch(/analyst|build|explore|librarian/i); + }); + test("keeps objective, title, and Skill names strict and non-empty", () => { expect(() => DelegationRequestSchema.parse(request({ title: " " }))).toThrow(); expect(() => DelegationRequestSchema.parse(request({ objective: "" }))).toThrow(); diff --git a/packages/agent-core/src/delegation/schema.ts b/packages/agent-core/src/delegation/schema.ts index af71bb3e..447b0021 100644 --- a/packages/agent-core/src/delegation/schema.ts +++ b/packages/agent-core/src/delegation/schema.ts @@ -3,10 +3,10 @@ import { SKILL_NAME_REGEX } from "../skills/schema"; const NON_EMPTY = z.string().trim().min(1); const DELEGATION_AGENT_TYPE = z.enum(["analyst", "build", "explore", "librarian"]).describe( - "Allowed child Agent identity to assign this task to.", + "Delegated child Agent identity value. Current parent/depth capability admission determines whether the selected target is authorized.", ); const DELEGATION_PROFILE = z.enum(["deep", "fast"]).describe( - "Model-resource Profile for the child: analyst requires deep; explore and librarian require fast; build allows deep or fast.", + "Delegated model-resource Profile value. Current parent/depth capability admission determines whether the selected Agent/Profile pair is authorized.", ); const DELEGATION_TITLE = NON_EMPTY.describe( "Short user-facing title for the child Session.", @@ -31,19 +31,6 @@ export const DelegationRequestSchema = z.strictObject({ objective: DELEGATION_OBJECTIVE, skills: DELEGATION_SKILLS, background: DELEGATION_BACKGROUND, -}).superRefine((request, ctx) => { - const requiredProfile = request.agent_type === "analyst" - ? "deep" - : request.agent_type === "explore" || request.agent_type === "librarian" - ? "fast" - : undefined; - if (requiredProfile !== undefined && request.profile !== requiredProfile) { - ctx.addIssue({ - code: "custom", - path: ["profile"], - message: `${request.agent_type} delegation requires the ${requiredProfile} Profile`, - }); - } }); export type DelegationRequestInput = z.input; diff --git a/packages/agent-core/src/execution/session-execution-manager.test.ts b/packages/agent-core/src/execution/session-execution-manager.test.ts index 0d4bd5d0..3eace0ea 100644 --- a/packages/agent-core/src/execution/session-execution-manager.test.ts +++ b/packages/agent-core/src/execution/session-execution-manager.test.ts @@ -22,13 +22,13 @@ import { import { buildAgentDefinition, leadAgentDefinition, exploreAgentDefinition } from "../agents/definitions"; import { ProviderRegistry } from "../provider"; import { ModelInfo } from "../provider/model"; -import { SkillService } from "../skills"; +import { SkillNotFoundError, SkillService, SkillValidationError } from "../skills"; import { createTestProjectContextResolver } from "../agents/test-project-context-resolver"; import { createTestToolRegistryFixture } from "../tools/test-registry"; import { testExecutionEnd, testExecutionRecord, testExecutionStart, testExecutionSuspended } from "../testing/test-execution-fixtures"; import { applySessionToolBatchChildOutcome } from "./session-tool-batch-scheduler"; import { setLlmAdapterForTest } from "../llm/adapter"; -import { AgentRunningError, ConcurrentLimitError, DelegateTargetNotAllowedError, DepthLimitError, ChildSessionNotFoundError, ChildSessionParentMismatchError, ChildSessionNotDescendantError, ChildSessionCwdMismatchError, SessionCwdTransitionConflictError, SessionCwdTransitionInProgressError, SessionToolBatchActiveError } from "../agents/errors"; +import { AgentRunningError, ConcurrentLimitError, DelegateTargetNotAllowedError, ChildSessionNotFoundError, ChildSessionParentMismatchError, ChildSessionNotDescendantError, ChildSessionCwdMismatchError, SessionCwdTransitionConflictError, SessionCwdTransitionInProgressError, SessionToolBatchActiveError, SkillNotAllowedError } from "../agents/errors"; import type { SessionAgentManager } from "../agents/session-agent-manager"; import { NotRootSessionError, SessionDeleteConflictError, SessionFileNotFoundError } from "../store/errors"; import { SessionDeleteInProgressError } from "./session-deletion"; @@ -497,7 +497,7 @@ function makeFactory(overrides: Partial = {}): AgentFactory { tools: { tools: [] }, childPolicy: undefined, }; - return { + const factory = { createRootAgent: mock(() => { throw new Error("unused"); }), createAgent: mock(() => { throw new Error("unused"); }), getDefinition: mock((name: string) => { @@ -507,10 +507,30 @@ function makeFactory(overrides: Partial = {}): AgentFactory { }), listAgentNames: mock(() => ["lead", "explore"]), resolveAllowedTools: mock((definition: AgentDefinition) => definition.tools.tools), - getDelegateTargetsFor: mock((definition: AgentDefinition) => definition.tools.delegateTargets ?? []), + resolveDelegationCapabilities: mock((parentAgentName: AgentName, depth: number) => { + const definition = factory.getDefinition(parentAgentName); + const targetNames = definition.childPolicy !== undefined + && depth < definition.childPolicy.maxDepth + && definition.tools.tools.includes("delegate") + ? definition.tools.delegateTargets ?? [] + : []; + return { + parentAgentName, + depth, + targets: targetNames.map((agentName) => { + const target = factory.getDefinition(agentName); + return { + agentName: target.name, + profiles: [...target.profiles], + builtinSkillNames: [...target.skills], + }; + }), + }; + }), resolveDelegatedSkillNames: mock(async () => []), ...overrides, } as AgentFactory; + return factory; } function makeFactoryWithChildPolicy( @@ -2425,6 +2445,11 @@ describe("SessionExecutionManager", () => { projectContextResolver: createTestProjectContextResolver(storeManager), resolveVersionControl: async () => "git", resolveAllowedTools: (agentDefinition) => agentDefinition.tools.tools, + delegationCapabilities: { + parentAgentName: "lead", + depth: 0, + targets: [], + }, logger: silentLogger, }); const { manager } = createManager({ [sessionId]: configuredAgent as unknown as MockAgent }); @@ -3980,7 +4005,7 @@ describe("SessionExecutionManager", () => { expect(sessionAgentManager.get(workspaceRoot, failedChildId)).toBeUndefined(); }); - test("depth limit is checked before child session creation", async () => { + test("exhausted depth removes target capability before child session creation", async () => { const rootId = crypto.randomUUID(); const middleId = crypto.randomUUID(); const parentId = crypto.randomUUID(); @@ -4011,12 +4036,79 @@ describe("SessionExecutionManager", () => { toolName: "delegate", request: delegationRequest({ agent_type: "explore", title: "Delegated child", objective: "inspect", skills: [], background: false }), parentAbort: undefined, - })).rejects.toThrow(DepthLimitError); + })).rejects.toThrow(DelegateTargetNotAllowedError); expect(sessionAgentManager.createChildAgent).not.toHaveBeenCalled(); expect(parentStore.getState().childSessionLinks).toEqual([]); }); + test("all known delegation admission failures leave no child artifact", async () => { + const cases: readonly { + label: string; + request: DelegationRequest; + factory: AgentFactory; + expected: Error | { readonly code: string }; + }[] = [ + { + label: "target", + request: delegationRequest({ agent_type: "build", profile: "deep" }), + factory: makeFactory(), + expected: new DelegateTargetNotAllowedError("lead", "build", 0), + }, + { + label: "profile", + request: delegationRequest({ agent_type: "explore", profile: "deep" }), + factory: makeFactory(), + expected: { code: "DELEGATION_PROFILE_NOT_ALLOWED" }, + }, + ...[ + new SkillNotFoundError("missing-skill"), + new SkillValidationError("invalid-skill", "project-archcode", "missing SKILL.md"), + new SkillNotAllowedError("explore", "run-goal", ["codemap"]), + ].map((error) => ({ + label: error.name, + request: delegationRequest({ skills: ["candidate-skill"] }), + factory: makeFactory({ + resolveDelegatedSkillNames: mock(async () => { throw error; }), + }), + expected: error, + })), + ]; + + for (const testCase of cases) { + const parentId = crypto.randomUUID(); + const childSessionId = crypto.randomUUID(); + const parentStore = storeManager.create(parentId, workspaceRoot, { + source: { kind: "direct" }, + agentName: "lead", + }); + const { manager, sessionAgentManager } = createManager({}, { factory: testCase.factory }); + + const start = manager.startChildExecution(workspaceRoot, { + parentStore, + parentSessionId: parentId, + parentToolCallId: `rejected-${testCase.label}`, + childSessionId, + toolName: "delegate", + request: testCase.request, + parentAbort: undefined, + }); + if (testCase.expected instanceof Error) { + await expect(start, testCase.label).rejects.toMatchObject({ + name: testCase.expected.name, + message: testCase.expected.message, + }); + } else { + await expect(start, testCase.label).rejects.toMatchObject(testCase.expected); + } + + expect(sessionAgentManager.createChildAgent, testCase.label).not.toHaveBeenCalled(); + expect(parentStore.getState().childSessionLinks, testCase.label).toEqual([]); + expect(storeManager.get(childSessionId, workspaceRoot), testCase.label).toBeUndefined(); + expect(await Bun.file(getSessionPath(workspaceRoot, childSessionId)).exists(), testCase.label).toBe(false); + } + }); + test("startChildExecution appends link and canonical prompt before model execution", async () => { const parentId = crypto.randomUUID(); const parentStore = storeManager.create(parentId, workspaceRoot, { source: { kind: "direct" }, agentName: "lead" }); @@ -5181,6 +5273,75 @@ describe("SessionExecutionManager", () => { }); }); + test("resumeChildExecution rejects a durable Profile outside the parent capability snapshot before activation", async () => { + const parentId = crypto.randomUUID(); + const childId = crypto.randomUUID(); + const parentStore = storeManager.create(parentId, workspaceRoot, { + source: { kind: "direct" }, + agentName: "lead", + }); + const childStore = storeManager.create(childId, workspaceRoot, { + rootSessionId: parentId, + parentSessionId: parentId, + agentName: "explore", + title: "Profile mismatch child", + activeSkillNames: [], + delegationRequest: delegationRequest({ + agent_type: "explore", + profile: "deep", + title: "Profile mismatch child", + skills: [], + }), + }); + const baseFactory = makeFactory(); + const resolveDelegationCapabilities = mock((parentAgentName: AgentName, depth: number) => ({ + parentAgentName, + depth, + targets: [{ + agentName: "explore" as const, + profiles: ["fast" as const], + builtinSkillNames: [], + }], + })); + const resolveDelegatedSkillNames = mock(async () => [] as readonly string[]); + const factory = makeFactory({ + getDefinition: mock((name: string) => { + const definition = baseFactory.getDefinition(name); + return name === "explore" + ? { ...definition, profiles: ["deep"] as const } + : definition; + }), + resolveDelegationCapabilities, + resolveDelegatedSkillNames, + }); + const executionScopeValidator = { validate: mock(async () => undefined) }; + const { manager, sessionAgentManager } = createManager({}, { + factory, + executionScopeValidator, + }); + + await expect(manager.resumeChildExecution(workspaceRoot, { + parentStore, + parentSessionId: parentId, + parentToolCallId: "profile-mismatch-resume", + toolName: "resume_session", + sessionId: childId, + instruction: "resume", + background: false, + })).rejects.toMatchObject({ + name: "DelegationExecutionAdmissionError", + code: "DELEGATION_PROFILE_NOT_ALLOWED", + }); + + expect(resolveDelegationCapabilities).toHaveBeenCalledWith("lead", 0); + expect(resolveDelegatedSkillNames).not.toHaveBeenCalled(); + expect(executionScopeValidator.validate).not.toHaveBeenCalled(); + expect(sessionAgentManager.createChildAgent).not.toHaveBeenCalled(); + expect(childStore.getState().executions).toEqual([]); + expect(childStore.getState().messages).toEqual([]); + expect(parentStore.getState().childSessionLinks).toEqual([]); + }); + test("blocks cwd transitions for active descendants and never resumes an old child across checkouts", async () => { const parentId = crypto.randomUUID(); const parentStore = storeManager.create(parentId, workspaceRoot, { source: { kind: "direct" }, agentName: "lead" }); @@ -5998,7 +6159,7 @@ describe("SessionExecutionManager", () => { expect(parentStore.getState().childSessionLinks).toEqual([]); }); - test("resumeChildExecution re-enforces canonical maxDepth", async () => { + test("resumeChildExecution re-enforces the canonical depth capability", async () => { const rootId = crypto.randomUUID(); const middleId = crypto.randomUUID(); const parentId = crypto.randomUUID(); @@ -6038,7 +6199,7 @@ describe("SessionExecutionManager", () => { sessionId: childId, instruction: "resume", background: false, - })).rejects.toThrow(DepthLimitError); + })).rejects.toThrow(DelegateTargetNotAllowedError); }); test("resumeChildExecution re-enforces maxConcurrent", async () => { diff --git a/packages/agent-core/src/execution/session-execution-manager.ts b/packages/agent-core/src/execution/session-execution-manager.ts index a5958021..c352364c 100644 --- a/packages/agent-core/src/execution/session-execution-manager.ts +++ b/packages/agent-core/src/execution/session-execution-manager.ts @@ -38,7 +38,6 @@ import { ChildSessionParentMismatchError, ConcurrentLimitError, DelegateTargetNotAllowedError, - DelegationToolNotAllowedError, DepthLimitError, SessionCwdTransitionConflictError, SessionCwdTransitionInProgressError, @@ -1927,14 +1926,9 @@ export class SessionExecutionManager { const parentState = request.parentStore.getState(); const targetAgentName = request.request.agent_type as AgentName; const parentDefinition = factory.getDefinition(parentAgentName); - const allowedTools = factory.resolveAllowedTools(parentDefinition, currentDepth); - - if (!allowedTools.includes("delegate")) { - throw new DelegationToolNotAllowedError(parentAgentName, currentDepth); - } - - const delegateTargets = factory.getDelegateTargetsFor(parentDefinition, currentDepth); - if (!delegateTargets.includes(targetAgentName)) { + const delegationCapabilities = factory.resolveDelegationCapabilities(parentAgentName, currentDepth); + const targetCapability = delegationCapabilities.targets.find((target) => target.agentName === targetAgentName); + if (targetCapability === undefined) { throw new DelegateTargetNotAllowedError(parentAgentName, targetAgentName, currentDepth); } @@ -1945,12 +1939,8 @@ export class SessionExecutionManager { } const childPolicy = configuredChildPolicy; - if (currentDepth >= childPolicy.maxDepth) { - throw new DepthLimitError(currentDepth); - } - const validatedRequest = request.request; - if (!targetDefinition.profiles.includes(validatedRequest.profile)) { + if (!targetCapability.profiles.includes(validatedRequest.profile)) { throw new DelegationExecutionAdmissionError( "DELEGATION_PROFILE_NOT_ALLOWED", `${targetDefinition.displayName} does not allow Profile "${validatedRequest.profile}"`, @@ -1966,7 +1956,7 @@ export class SessionExecutionManager { let activeSkillNames: readonly string[]; try { activeSkillNames = await factory.resolveDelegatedSkillNames( - targetDefinition, + targetCapability, validatedRequest.skills, parentState.cwd, ); @@ -3903,7 +3893,6 @@ export class SessionExecutionManager { ): Promise { const claimedChild = childActivationIdentitySnapshot(childStore.getState()); this.#assertDurableChildDelegationIdentity(childStore.getState()); - await this.#validatePersistedDelegationRequest(workspaceRoot, childStore.getState()); const parentSessionId = childStore.getState().parentSessionId; if (parentSessionId === undefined) { throw new DelegationExecutionAdmissionError( @@ -3963,23 +3952,26 @@ export class SessionExecutionManager { const parentDefinition = factory.getDefinition(parentState.agentName); const parentDepth = await this.#config.resolveSessionDepth(workspaceRoot, parentState.sessionId); const childDepth = await this.#config.resolveSessionDepth(workspaceRoot, childState.sessionId); + const delegationCapabilities = factory.resolveDelegationCapabilities(parentState.agentName, parentDepth); + const targetCapability = delegationCapabilities.targets.find((target) => target.agentName === childState.agentName); + if (targetCapability === undefined) { + throw new DelegateTargetNotAllowedError(parentState.agentName, childState.agentName, parentDepth); + } + const durableRequest = childState.delegationRequest!; + if (!targetCapability.profiles.includes(durableRequest.profile)) { + throw new DelegationExecutionAdmissionError( + "DELEGATION_PROFILE_NOT_ALLOWED", + `Child Agent "${targetCapability.agentName}" does not allow durable Profile "${durableRequest.profile}" at depth ${parentDepth}`, + ); + } const configuredChildPolicy = parentDefinition.childPolicy; if (configuredChildPolicy === undefined) throw new AgentChildPolicyMissingError(parentState.agentName); const childPolicy = configuredChildPolicy; - if (parentDepth >= childPolicy.maxDepth || childDepth !== parentDepth + 1 || childDepth > childPolicy.maxDepth) { + if (childDepth !== parentDepth + 1 || childDepth > childPolicy.maxDepth) { throw new DepthLimitError(parentDepth); } - const allowedTools = factory.resolveAllowedTools(parentDefinition, parentDepth); - if (!allowedTools.includes("delegate")) { - throw new DelegationToolNotAllowedError(parentState.agentName, parentDepth); - } - const delegateTargets = factory.getDelegateTargetsFor(parentDefinition, parentDepth); - if (!delegateTargets.includes(childState.agentName)) { - throw new DelegateTargetNotAllowedError(parentState.agentName, childState.agentName, parentDepth); - } - const targetDefinition = factory.getDefinition(childState.agentName); const activeSkillNames = await factory.resolveDelegatedSkillNames( - targetDefinition, + targetCapability, childState.activeSkillNames, childState.cwd, ); @@ -4067,20 +4059,6 @@ export class SessionExecutionManager { } } - async #validatePersistedDelegationRequest( - workspaceRoot: string, - state: SessionStoreState, - ): Promise { - if (state.parentSessionId === undefined) return; - const request = state.delegationRequest!; - const definition = this.#config.sessionAgentManager.getFactory(workspaceRoot).getDefinition(state.agentName); - if (definition.profiles.includes(request.profile)) return; - throw new DelegationExecutionAdmissionError( - "DELEGATION_PROFILE_NOT_ALLOWED", - `${definition.displayName} does not allow durable Profile "${request.profile}"`, - ); - } - async #validateProspectiveChildExecutionScope( workspaceRoot: string, parentState: SessionStoreState, diff --git a/packages/agent-core/src/execution/session-tool-batch-scheduler.test.ts b/packages/agent-core/src/execution/session-tool-batch-scheduler.test.ts index f1bf5f03..74847e91 100644 --- a/packages/agent-core/src/execution/session-tool-batch-scheduler.test.ts +++ b/packages/agent-core/src/execution/session-tool-batch-scheduler.test.ts @@ -11,11 +11,13 @@ import type { SessionToolBatchCall } from "../store/types"; import { ToolOutputArtifactStore } from "../tool-output/artifact-store"; import { ToolOutputFinalizer } from "../tool-output/finalizer"; import { askUserTool } from "../tools/builtins/ask-user"; +import { skillListTool } from "../tools/builtins/skill-list"; import { defineTool } from "../tools/define-tool"; import { createToolErrorResult } from "../tools/errors"; import { ToolRegistry } from "../tools/registry"; import { createTextToolResult } from "../tools/results"; import { SecretRedactionPolicy } from "../security"; +import { SkillService } from "../skills"; import { createTestProjectContext } from "../tools/test-project-context"; import type { AnyToolDescriptor, RawToolResult, ToolCallLike, ToolExecutionContext } from "../tools/types"; import { testExecutionStart } from "../testing/test-execution-fixtures"; @@ -1174,6 +1176,123 @@ describe("SessionToolBatchScheduler output ownership", () => { expect(harness.scheduler.activeBatch()!.calls[0]).toMatchObject({ state: "completed", attempt: 2 }); }); + test("rebuilds target Skill authorization when retrying skill_list after restart", async () => { + const harness = await createHarness(); + harness.registry.register(skillListTool); + const skillService = new SkillService({ + userSkillsRoot: join(TMP_DIR, "user-skills"), + userAgentsSkillsRoot: join(TMP_DIR, "user-agent-skills"), + }); + const resolveSkillListTargetSkills = mock((agentType: string) => ( + agentType === "explore" ? ["codemap", "research-docs"] : undefined + )); + const createRestartableContext = async (call: ToolCallLike, step: number): Promise => ({ + ...await harness.createContext(call, step), + agentSkills: [], + skillService, + resolveSkillListTargetSkills, + }); + const schedulerOptions = { + executionId: "test-execution", + runOrdinal: 0, + store: harness.store, + storeManager: harness.storeManager, + workspaceRoot: TMP_DIR, + registry: harness.registry, + hitlQueue: harness.hitlQueue, + agentName: "lead", + allowedTools: ["skill_list"], + agentSkills: [], + logger: harness.logger, + createContext: createRestartableContext, + } as const; + const initialScheduler = new SessionToolBatchScheduler(schedulerOptions); + const batch = await initialScheduler.createBatch([{ + toolCallId: "skill-list-target", + toolName: "skill_list", + input: { agent_type: "explore" }, + }], "step-0", 0); + await markRunning(harness, batch.calls[0]!, 1); + + const restartedScheduler = new SessionToolBatchScheduler(schedulerOptions); + expect(await restartedScheduler.recoverInterruptedBatch()).toMatchObject({ + status: "ready_for_continuation", + }); + expect(resolveSkillListTargetSkills).toHaveBeenCalledWith("explore"); + expect(restartedScheduler.activeBatch()!.calls[0]).toMatchObject({ + state: "completed", + attempt: 2, + result: { isError: false }, + }); + expect(restartedScheduler.activeBatch()!.calls[0]!.result?.output.preview).toContain('"name":"codemap"'); + }); + + test("fails closed when target Skill authorization is revoked before restart recovery", async () => { + const harness = await createHarness(); + harness.registry.register(skillListTool); + const skillService = new SkillService({ + userSkillsRoot: join(TMP_DIR, "user-skills"), + userAgentsSkillsRoot: join(TMP_DIR, "user-agent-skills"), + }); + const listPageForAgent = mock(async () => ({ + items: [{ name: "parent-only", description: "Must not leak", source: "project-archcode" as const }], + })); + Object.defineProperty(skillService, "listPageForAgent", { value: listPageForAgent }); + const createContextWith = ( + resolveSkillListTargetSkills: ToolExecutionContext["resolveSkillListTargetSkills"], + ) => async (call: ToolCallLike, step: number): Promise => ({ + ...await harness.createContext(call, step), + agentSkills: ["parent-only"], + skillService, + resolveSkillListTargetSkills, + }); + const schedulerOptions = { + executionId: "test-execution", + runOrdinal: 0, + store: harness.store, + storeManager: harness.storeManager, + workspaceRoot: TMP_DIR, + registry: harness.registry, + hitlQueue: harness.hitlQueue, + agentName: "lead", + allowedTools: ["skill_list"], + agentSkills: ["parent-only"], + logger: harness.logger, + } as const; + const initialScheduler = new SessionToolBatchScheduler({ + ...schedulerOptions, + createContext: createContextWith((agentType) => ( + agentType === "explore" ? ["codemap", "research-docs"] : undefined + )), + }); + const batch = await initialScheduler.createBatch([{ + toolCallId: "skill-list-revoked-target", + toolName: "skill_list", + input: { agent_type: "explore" }, + }], "step-0", 0); + await markRunning(harness, batch.calls[0]!, 1); + + const restartedScheduler = new SessionToolBatchScheduler({ + ...schedulerOptions, + createContext: createContextWith(() => undefined), + }); + expect(await restartedScheduler.recoverInterruptedBatch()).toMatchObject({ + status: "ready_for_continuation", + }); + const recoveredCall = restartedScheduler.activeBatch()!.calls[0]!; + expect(recoveredCall).toMatchObject({ + state: "failed", + attempt: 2, + result: { + isError: true, + details: { error: { code: "TOOL_SKILL_TARGET_NOT_ALLOWED" } }, + }, + }); + expect(listPageForAgent).not.toHaveBeenCalled(); + expect(recoveredCall.result?.output.preview).not.toContain("parent-only"); + expect(recoveredCall.result?.output.preview).not.toContain("codemap"); + }); + test("finalizes an exhausted read-only recovery through the Registry system lane", async () => { const harness = await createHarness(); const batch = await harness.scheduler.createBatch([{ toolCallId: "read-1", toolName: "read_tool", input: {} }], "step-0", 0); diff --git a/packages/agent-core/src/tools/builtins/delegate.test.ts b/packages/agent-core/src/tools/builtins/delegate.test.ts index e3d91397..425ce8a4 100644 --- a/packages/agent-core/src/tools/builtins/delegate.test.ts +++ b/packages/agent-core/src/tools/builtins/delegate.test.ts @@ -1,7 +1,13 @@ import { describe, expect, it, mock } from "bun:test"; import type { DelegationRequest } from "@archcode/protocol"; import type { ChildExecutionHandle, ChildExecutionRequest } from "../../delegation/types"; -import { SkillNotAllowedError } from "../../agents/errors"; +import { + AgentChildPolicyMissingError, + DelegateTargetNotAllowedError, + DepthLimitError, + SkillNotAllowedError, +} from "../../agents/errors"; +import { SkillNotFoundError, SkillValidationError } from "../../skills"; import { storeManager } from "../../store/store"; import { expectTextDraft } from "../test-results"; import type { ToolExecutionContext } from "../types"; @@ -134,15 +140,47 @@ describe("delegate request", () => { }); }); - it("returns target Skill recovery details", async () => { - const result = await executeDelegate(request({ skills: ["research-docs"] }), makeContext({ + it("maps known child-start admission failures to stable tool error codes", async () => { + const profileError = Object.assign(new Error('Explore does not allow Profile "deep"'), { + code: "DELEGATION_PROFILE_NOT_ALLOWED", + name: "DelegationExecutionAdmissionError", + }); + const cases: readonly [string, Error, string][] = [ + ["delegate target", new DelegateTargetNotAllowedError("lead", "build", 3), "TOOL_DELEGATE_TARGET_NOT_ALLOWED"], + ["missing child policy", new AgentChildPolicyMissingError("lead"), "TOOL_DELEGATE_TARGET_NOT_ALLOWED"], + ["depth limit", new DepthLimitError(3), "TOOL_DELEGATE_TARGET_NOT_ALLOWED"], + ["profile", profileError, "TOOL_DELEGATE_PROFILE_NOT_ALLOWED"], + ["missing Skill", new SkillNotFoundError("research-docs"), "TOOL_DELEGATE_SKILL_NOT_FOUND"], + ["invalid Skill", new SkillValidationError("research-docs", "project-archcode", "missing SKILL.md"), "TOOL_DELEGATE_SKILL_INVALID"], + ["disallowed Skill", new SkillNotAllowedError("explore", "research-docs", ["codemap"]), "TOOL_DELEGATE_SKILL_NOT_ALLOWED"], + ]; + + for (const [label, error, expectedCode] of cases) { + const result = await executeDelegate(request({ skills: ["research-docs"] }), makeContext({ + startChildExecution: async () => { + throw error; + }, + })); + expect(JSON.parse(textResult(result)), label).toMatchObject({ + code: expectedCode, + name: error.name, + message: error.message, + }); + } + }); + + it("uses the generic code only for an unknown child-start failure", async () => { + const error = new Error("unexpected launch failure"); + error.name = "UnexpectedLaunchError"; + const result = await executeDelegate(request(), makeContext({ startChildExecution: async () => { - throw new SkillNotAllowedError("explore", "research-docs", ["codemap"]); + throw error; }, })); expect(JSON.parse(textResult(result))).toMatchObject({ code: "TOOL_DELEGATE_FAILED", - name: "SkillNotAllowedError", + name: error.name, + message: error.message, }); }); diff --git a/packages/agent-core/src/tools/builtins/delegate.ts b/packages/agent-core/src/tools/builtins/delegate.ts index 7ecdf7ca..866b1fb2 100644 --- a/packages/agent-core/src/tools/builtins/delegate.ts +++ b/packages/agent-core/src/tools/builtins/delegate.ts @@ -3,8 +3,15 @@ import type { SessionExecutionRecord, SessionExecutionTerminalStatus, } from "@archcode/protocol"; +import { + AgentChildPolicyMissingError, + DelegateTargetNotAllowedError, + DepthLimitError, + SkillNotAllowedError, +} from "../../agents/errors"; import { DelegationRequestSchema } from "../../delegation/schema"; import type { ChildExecutionHandle, ChildExecutionOutcome } from "../../delegation/types"; +import { SkillNotFoundError, SkillValidationError } from "../../skills"; import { defineTool } from "../define-tool"; import { createToolErrorResult } from "../errors"; import { createTextToolResult } from "../results"; @@ -60,7 +67,7 @@ export async function executeDelegate( const safeError = error instanceof Error ? error : new Error(String(error)); return createToolErrorResult({ kind: "execution", - code: "TOOL_DELEGATE_FAILED", + code: delegateStartErrorCode(error), message: safeError.message, name: safeError.name, error: safeError, @@ -79,6 +86,32 @@ export async function executeDelegate( }, outcome)); } +function delegateStartErrorCode(error: unknown): string { + if ( + error instanceof DelegateTargetNotAllowedError + || error instanceof AgentChildPolicyMissingError + || error instanceof DepthLimitError + ) { + return "TOOL_DELEGATE_TARGET_NOT_ALLOWED"; + } + + if (hasErrorCode(error, "DELEGATION_PROFILE_NOT_ALLOWED")) { + return "TOOL_DELEGATE_PROFILE_NOT_ALLOWED"; + } + + if (error instanceof SkillNotFoundError) return "TOOL_DELEGATE_SKILL_NOT_FOUND"; + if (error instanceof SkillValidationError) return "TOOL_DELEGATE_SKILL_INVALID"; + if (error instanceof SkillNotAllowedError) return "TOOL_DELEGATE_SKILL_NOT_ALLOWED"; + return "TOOL_DELEGATE_FAILED"; +} + +function hasErrorCode(error: unknown, code: string): boolean { + return typeof error === "object" + && error !== null + && "code" in error + && (error as { readonly code?: unknown }).code === code; +} + export function formatAsyncChildOutput(handle: ChildExecutionHandle): string { return JSON.stringify({ session_id: handle.sessionId, @@ -154,7 +187,7 @@ export const delegateTool = defineTool({ name: "delegate", description: [ "Create one direct child Session from a strict DelegationRequest.", - "Select the allowed child Agent and its permitted Profile; use deep or fast for Build according to task intensity, and list only the workflow Skills it needs.", + "Provide the delegated Agent, Profile, and only the workflow Skills it needs. Current parent/depth capability admission authorizes the selected Agent/Profile pair before child creation.", "A fresh child receives its own runtime-provided system context and visible tools, but does not inherit the parent conversation or prior tool results.", "Write objective as the minimum self-contained task-specific handoff: state the requested outcome or question, material parent-local facts or decisions, scope or non-goals, expected final output or evidence, and verification when applicable.", "Do not rely on the child to reconstruct parent-local understanding, and do not repeat generic context already supplied by the runtime.", diff --git a/packages/agent-core/src/tools/builtins/model-visible-contract.test.ts b/packages/agent-core/src/tools/builtins/model-visible-contract.test.ts index c708a654..589ba131 100644 --- a/packages/agent-core/src/tools/builtins/model-visible-contract.test.ts +++ b/packages/agent-core/src/tools/builtins/model-visible-contract.test.ts @@ -165,13 +165,13 @@ const CONTRACTS: readonly ModelVisibleContract[] = [ }, { tool: "delegate", + // This fixture owns only the global descriptor contract. Current-depth target/Profile + // presentation is projected later and is covered by model-tool-projection/factory tests. competitorEvidenceIds: ["CC-160-A:Agent", "OC:task", "OMO-D", "CX-MA:spawn_agent"], runtimeSourceIds: ["tools/builtins/delegate.ts", "delegation/schema.ts", "execution/session-execution-manager.ts"], descriptionPatterns: [ /direct child Session/i, - /strict DelegationRequest/, - /Agent and its permitted Profile/i, - /deep or fast for Build according to task intensity/i, + /current parent\/depth capability admission authorizes the selected Agent\/Profile pair/i, /fresh child receives its own runtime-provided system context and visible tools/i, /does not inherit the parent conversation or prior tool results/i, /minimum self-contained task-specific handoff/i, @@ -189,24 +189,15 @@ const CONTRACTS: readonly ModelVisibleContract[] = [ /terminal reminder/, /background_output/, ], + descriptionExcludes: [/Build.*deep or fast/i, /analyst requires deep/i], schema: [ { path: ["properties", "agent_type"], - expectedEnum: ["analyst", "build", "explore", "librarian"], - descriptionPatterns: [ - /allowed child Agent identity/i, - /assign this task/i, - ], + descriptionPatterns: [/Delegated child Agent identity value/i, /parent\/depth capability admission/i, /selected target is authorized/i], }, { path: ["properties", "profile"], - expectedEnum: ["deep", "fast"], - descriptionPatterns: [ - /model-resource Profile for the child/i, - /analyst requires deep/i, - /explore and librarian require fast/i, - /build allows deep or fast/i, - ], + descriptionPatterns: [/Delegated model-resource Profile value/i, /parent\/depth capability admission/i, /selected Agent\/Profile pair is authorized/i], }, { path: ["properties", "title"], @@ -326,7 +317,7 @@ const CONTRACTS: readonly ModelVisibleContract[] = [ tool: "skill_list", competitorEvidenceIds: ["CC-160-A:Skill", "OC:skill"], runtimeSourceIds: ["tools/builtins/skill-list.ts:6-35"], - descriptionPatterns: [/currently allowed for this Agent/i, /System Prompt normally already lists the same allowed metadata/i, /fresh machine-readable copy/i, /call skill_read directly/i, /skill_list\(\{\}\)/, /exact returned name/i, /Never guess or invent/i, /exactly name, description, and source/i, /resource contents are omitted/i], + descriptionPatterns: [/current Agent or.*allowed direct delegation target/i, /System Prompt normally already lists current-Agent metadata/i, /fresh machine-readable copy/i, /call skill_read directly/i, /skill_list\(\{\}\)/, /same target's delegate\.skills/i, /do not grant.*parent Agent permission/i, /Never guess or invent/i, /exactly name, description, and source/i, /resource contents are omitted/i], }, { tool: "skill_read", @@ -334,7 +325,7 @@ const CONTRACTS: readonly ModelVisibleContract[] = [ runtimeSourceIds: ["tools/builtins/skill-read.ts:10-14,83-105"], descriptionPatterns: [/allowed.*Agent/i, /available names are already listed in the System Prompt/i, /skill_read\(/, /metadata, filesystem root when available, sorted resource descriptors, and entry body/i, /exactly one listed UTF-8 text resource/i, /unsupported-binary error/i, /Read the Skill before the work it governs/i, /supporting resources only when needed/i, /Do not load unrelated Skills/i, /cannot expand/i, /permissions/, /workspace/], schema: [ - { path: ["properties", "name"], descriptionPatterns: [/System Prompt's available-skill list or skill_list/i, /exact/i] }, + { path: ["properties", "name"], descriptionPatterns: [/current-Agent Skill name/i, /System Prompt or skill_list/i, /target-scoped delegation result/i, /exact/i] }, { path: ["properties", "resource"], descriptionPatterns: [/Skill-root-relative/i, /Resources list/i, /cannot.*arbitrary filesystem path/i] }, ], }, @@ -421,6 +412,17 @@ function expectPatterns(value: string, patterns: readonly RegExp[]): void { } } +function toModelJsonSchema(inputSchema: unknown): JsonObject { + if ( + typeof inputSchema === "object" + && inputSchema !== null + && "jsonSchema" in inputSchema + ) { + return (inputSchema as { readonly jsonSchema: JsonObject }).jsonSchema; + } + return z.toJSONSchema(inputSchema as z.ZodType) as JsonObject; +} + const registryFixture = createTestToolRegistryFixture(); const registry = registryFixture.registry; registerBuiltinTools(registry, silentLogger, { github: { enabled: false } }); @@ -448,7 +450,7 @@ describe("Lead model-visible Tool Contract", () => { expect(tool.description).not.toMatch(excluded); } - const schema = z.toJSONSchema(tool.inputSchema as z.ZodType) as JsonObject; + const schema = toModelJsonSchema(tool.inputSchema); for (const field of contract.schema ?? []) { const node = getSchemaNode(schema, field.path); if (field.descriptionPatterns !== undefined) { diff --git a/packages/agent-core/src/tools/builtins/skill-list.test.ts b/packages/agent-core/src/tools/builtins/skill-list.test.ts index 005c5b02..f1afe8ec 100644 --- a/packages/agent-core/src/tools/builtins/skill-list.test.ts +++ b/packages/agent-core/src/tools/builtins/skill-list.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeEach, describe, expect, test } from "bun:test"; +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; import { mkdir, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -10,6 +10,7 @@ import { expectTextDraft } from "../test-results"; import { createToolExecutionContext, type ToolExecutionContext } from "../types"; import { createBuiltinToolDescriptors } from "./index"; import { SkillListInputSchema, skillListTool } from "./skill-list"; +import { skillReadTool } from "./skill-read"; const tmpRoot = join(tmpdir(), "archcode-skill-list-tool", crypto.randomUUID()); const projectRoot = join(tmpRoot, "project"); @@ -25,7 +26,12 @@ type SkillListPage = { readonly nextCursor?: string; }; -function makeContext(agentSkills: readonly string[], cwd = projectRoot): ToolExecutionContext { +function makeContext( + agentSkills: readonly string[], + cwd = projectRoot, + resolveSkillListTargetSkills?: ToolExecutionContext["resolveSkillListTargetSkills"], + skillService = new SkillService({ userSkillsRoot, userAgentsSkillsRoot }), +): ToolExecutionContext { return createToolExecutionContext({ store: createMockStore(), storeManager, toolName: "skill_list", toolCallId: "skill-list-call", input: {}, @@ -37,7 +43,8 @@ function makeContext(agentSkills: readonly string[], cwd = projectRoot): ToolExe startedAt: 0, allowedTools: new Set(["skill_list"]), agentSkills, - skillService: new SkillService({ userSkillsRoot, userAgentsSkillsRoot }), + skillService, + ...(resolveSkillListTargetSkills === undefined ? {} : { resolveSkillListTargetSkills }), projectContext: createTestProjectContext(projectRoot), cwd, }); } @@ -88,6 +95,58 @@ describe("skill_list tool", () => { expect(JSON.parse(expectTextDraft(result))).toEqual({ items: [] }); }); + test("allowed target uses its resolved builtin allow-list", async () => { + const resolveTarget = mock((agentType: string) => agentType === "explore" ? exploreSkills : undefined); + + const result = await skillListTool.execute( + { agent_type: "explore" }, + makeContext(leadSkills, projectRoot, resolveTarget), + ); + const page = JSON.parse(expectTextDraft(result)) as SkillListPage; + + expect(resolveTarget).toHaveBeenCalledWith("explore"); + expect(page.items.map((entry) => entry.name)).toEqual(["codemap", "research-docs"]); + }); + + test("target-only reserved Skill discovery does not grant the parent read access", async () => { + const ctx = makeContext( + leadSkills, + projectRoot, + (agentType) => agentType === "analyst" ? ["goal-review"] : undefined, + ); + const targetPage = JSON.parse(expectTextDraft(await skillListTool.execute( + { agent_type: "analyst" }, + ctx, + ))) as SkillListPage; + const parentRead = await skillReadTool.execute( + { name: "goal-review" }, + { ...ctx, toolName: "skill_read", allowedTools: new Set(["skill_read"]) }, + ); + + expect(targetPage.items.map((entry) => entry.name)).toContain("goal-review"); + expect(parentRead.isError).toBe(true); + expect(parentRead.details?.error?.code).toBe("TOOL_SKILL_NOT_FOUND"); + }); + + test("target discovery fails closed when delegation context is missing", async () => { + const result = await skillListTool.execute({ agent_type: "explore" }, makeContext(leadSkills)); + + expect(result.isError).toBe(true); + expect(result.details?.error?.code).toBe("TOOL_SKILL_CONTEXT_MISSING"); + }); + + test("disallowed target returns a typed error without querying SkillService", async () => { + const ctx = makeContext(leadSkills, projectRoot, () => undefined); + const listPageForAgent = mock(async () => ({ items: [] })); + Object.defineProperty(ctx.skillService!, "listPageForAgent", { value: listPageForAgent }); + + const result = await skillListTool.execute({ agent_type: "build" }, ctx); + + expect(result.isError).toBe(true); + expect(result.details?.error?.code).toBe("TOOL_SKILL_TARGET_NOT_ALLOWED"); + expect(listPageForAgent).not.toHaveBeenCalled(); + }); + test("resolves same-name project Skills from the Session cwd", async () => { const name = "worktree-catalog-skill"; for (const [root, description] of [ @@ -120,8 +179,55 @@ describe("skill_list tool", () => { })); }); - test("input schema rejects unknown keys including agentName", () => { + test("isolates concurrent Prompt catalogs and pages on one shared SkillService", async () => { + const secondProjectRoot = join(tmpRoot, "second-project"); + const fixtures = [ + [projectRoot, "first-project-skill", "First project only."], + [secondProjectRoot, "second-project-skill", "Second project only."], + ] as const; + for (const [root, name, description] of fixtures) { + const skillRoot = join(root, ".archcode", "skills", name); + await mkdir(skillRoot, { recursive: true }); + await Bun.write(join(skillRoot, "SKILL.md"), [ + "---", + `name: ${name}`, + `description: ${description}`, + "---", + "", + description, + ].join("\n")); + } + + const sharedSkillService = new SkillService({ userSkillsRoot, userAgentsSkillsRoot }); + const [firstPrompt, secondPrompt, firstResult, secondResult] = await Promise.all([ + sharedSkillService.projectPromptCatalog(projectRoot, []), + sharedSkillService.projectPromptCatalog(secondProjectRoot, []), + skillListTool.execute({}, makeContext([], projectRoot, undefined, sharedSkillService)), + skillListTool.execute({}, makeContext([], secondProjectRoot, undefined, sharedSkillService)), + ]); + const firstPromptNames = firstPrompt.includedEntries.map((entry) => entry.name); + const secondPromptNames = secondPrompt.includedEntries.map((entry) => entry.name); + const firstNames = (JSON.parse(expectTextDraft(firstResult)) as SkillListPage).items.map((entry) => entry.name); + const secondNames = (JSON.parse(expectTextDraft(secondResult)) as SkillListPage).items.map((entry) => entry.name); + + expect(firstPromptNames).toEqual(["first-project-skill"]); + expect(firstPrompt.renderedText).toContain("first-project-skill"); + expect(firstPrompt.renderedText).not.toContain("second-project-skill"); + expect(secondPromptNames).toEqual(["second-project-skill"]); + expect(secondPrompt.renderedText).toContain("second-project-skill"); + expect(secondPrompt.renderedText).not.toContain("first-project-skill"); + expect(firstNames).toEqual(["first-project-skill"]); + expect(firstNames).not.toContain("second-project-skill"); + expect(secondNames).toEqual(["second-project-skill"]); + expect(secondNames).not.toContain("first-project-skill"); + }); + + test("input schema accepts delegated targets and rejects unknown keys or roles", () => { expect(SkillListInputSchema.safeParse({}).success).toBe(true); + expect(SkillListInputSchema.safeParse({ agent_type: "analyst" }).success).toBe(true); + expect(SkillListInputSchema.safeParse({ agent_type: "build", cursor: "next" }).success).toBe(true); + expect(SkillListInputSchema.safeParse({ agent_type: "lead" }).success).toBe(false); + expect(SkillListInputSchema.safeParse({ agent_type: "discussion" }).success).toBe(false); expect(SkillListInputSchema.safeParse({ agentName: "lead" }).success).toBe(false); expect(SkillListInputSchema.safeParse({ source: "builtin" }).success).toBe(false); }); diff --git a/packages/agent-core/src/tools/builtins/skill-list.ts b/packages/agent-core/src/tools/builtins/skill-list.ts index a89b135b..a4ece16d 100644 --- a/packages/agent-core/src/tools/builtins/skill-list.ts +++ b/packages/agent-core/src/tools/builtins/skill-list.ts @@ -7,6 +7,7 @@ import { DigestBoundCursorError } from "../../skills"; export const SkillListInputSchema = z.object({ cursor: z.string().min(1).optional(), + agent_type: z.enum(["analyst", "build", "explore", "librarian"]).optional(), }).strict(); type SkillListInput = z.infer; @@ -15,9 +16,9 @@ export function createSkillListTool() { return defineTool({ name: "skill_list", description: [ - "Discover the Skills currently allowed for this Agent. The System Prompt normally already lists the same allowed metadata; call skill_list only when you need a fresh machine-readable copy, and call skill_read directly when an exact matching Skill is already visible.", + "Discover Skills for the current Agent or for an allowed direct delegation target. The System Prompt normally already lists current-Agent metadata; call skill_list only when you need a fresh machine-readable copy, and call skill_read directly when an exact current-Agent Skill is already visible.", "", - "Call `skill_list({})`, inspect each returned name, description, and source, choose only an exact returned name, then call `skill_read({\"name\":\"\"})` before doing the governed work. Never guess or invent a Skill name. The result is metadata-only JSON containing exactly name, description, and source; Skill bodies and resource contents are omitted. An empty list means no Skill is available to this Agent.", + "Call `skill_list({})` for current-Agent Skills. Those exact returned names may be passed to current-Agent skill_read. Call `skill_list({\"agent_type\":\"\"})` only to choose exact names for that same target's delegate.skills; target results do not grant the parent Agent permission to read or activate them. Never guess or invent a Skill name. The result is metadata-only JSON containing exactly name, description, and source; Skill bodies and resource contents are omitted. An empty list means no Skill is available in the requested scope.", ].join("\n"), inputSchema: SkillListInputSchema, traits: { readOnly: true, destructive: false, concurrencySafe: true }, @@ -33,10 +34,29 @@ export function createSkillListTool() { message: "Skill tools require an explicit SkillService and agent Skill allow-list", }); } + let agentSkills = ctx.agentSkills; + if (input.agent_type !== undefined) { + if (ctx.resolveSkillListTargetSkills === undefined) { + return createToolErrorResult({ + kind: "execution", + code: "TOOL_SKILL_CONTEXT_MISSING", + message: "Target Skill discovery requires an explicit delegation capability resolver", + }); + } + const targetSkills = ctx.resolveSkillListTargetSkills(input.agent_type); + if (targetSkills === undefined) { + return createToolErrorResult({ + kind: "not-allowed", + code: "TOOL_SKILL_TARGET_NOT_ALLOWED", + message: `Agent target "${input.agent_type}" is not allowed at the current delegation depth`, + }); + } + agentSkills = targetSkills; + } try { const page = await ctx.skillService.listPageForAgent( ctx.cwd, - ctx.agentSkills, + agentSkills, input.cursor, ); return createTextToolResult(JSON.stringify(page)); diff --git a/packages/agent-core/src/tools/builtins/skill-read.test.ts b/packages/agent-core/src/tools/builtins/skill-read.test.ts index 513dae66..edb17ba3 100644 --- a/packages/agent-core/src/tools/builtins/skill-read.test.ts +++ b/packages/agent-core/src/tools/builtins/skill-read.test.ts @@ -356,6 +356,21 @@ Broken body. } }); + test("model-facing schema is portable and omits the internal Skill-name regex", () => { + const schema = (skillReadTool.aiInputSchema as { readonly jsonSchema: Record }).jsonSchema; + const serialized = JSON.stringify(schema); + + expect(schema).toMatchObject({ + type: "object", + additionalProperties: false, + required: ["name"], + }); + expect(serialized).not.toContain("pattern"); + expect(serialized).not.toContain("?!"); + expect(serialized).toContain("current-Agent Skill name"); + expect(serialized).toContain("target-scoped delegation result"); + }); + test("has correct read-only concurrency-safe traits and is registered", () => { expect(skillReadTool.traits).toEqual({ readOnly: true, diff --git a/packages/agent-core/src/tools/builtins/skill-read.ts b/packages/agent-core/src/tools/builtins/skill-read.ts index cbfb9e54..40f25156 100644 --- a/packages/agent-core/src/tools/builtins/skill-read.ts +++ b/packages/agent-core/src/tools/builtins/skill-read.ts @@ -1,3 +1,4 @@ +import { jsonSchema } from "ai"; import { z } from "zod"; import { defineTool } from "../define-tool"; import { createToolErrorResult } from "../errors"; @@ -18,6 +19,23 @@ import { SKILL_NAME_REGEX } from "../../skills/schema"; const SKILL_NAME_PATTERN = "^(?!.*--)[a-z0-9]+(?:-[a-z0-9]+)*$"; const SKILL_NAME_MESSAGE = `Skill name must match pattern ${SKILL_NAME_PATTERN} (no consecutive hyphens)`; +const SkillReadAiInputSchema = jsonSchema({ + type: "object", + additionalProperties: false, + required: ["name"], + properties: { + name: { + type: "string", + description: "Exact current-Agent Skill name copied from the System Prompt or skill_list({}). Do not guess or use a target-scoped delegation result.", + }, + resource: { + type: "string", + minLength: 1, + description: "Optional Skill-root-relative resource path copied exactly from the entry's Resources list. It cannot read an arbitrary filesystem path.", + }, + }, +}); + export const SkillReadInputSchema = z .object({ name: z.string().regex(SKILL_NAME_REGEX, SKILL_NAME_MESSAGE).describe(`Exact allowed Skill name matching ${SKILL_NAME_PATTERN}, with no consecutive hyphens; copy it from the System Prompt's available-skill list or skill_list instead of guessing.`), @@ -163,6 +181,7 @@ export function createSkillReadTool() { "Read the Skill before the work it governs, then load supporting resources only when needed. Copy resource paths from the entry's Resources list; they are Skill-root-relative and cannot read arbitrary filesystem paths. Do not load unrelated Skills for ceremony. This tool accepts no agent, role, source, or filesystem-root override. Skill instructions guide existing capabilities but cannot expand the Agent's tools, permissions, delegation targets, or workspace scope.", ].join("\n"), inputSchema: SkillReadInputSchema, + aiInputSchema: SkillReadAiInputSchema, traits: { readOnly: true, destructive: false, concurrencySafe: true }, outputPolicy: { kind: "artifact", previewDirection: "head-tail" }, execute: async ( diff --git a/packages/agent-core/src/tools/types.ts b/packages/agent-core/src/tools/types.ts index b9f87bf4..93cf84e3 100644 --- a/packages/agent-core/src/tools/types.ts +++ b/packages/agent-core/src/tools/types.ts @@ -76,6 +76,8 @@ export interface ToolExecutionContext { agentSkills?: readonly string[]; /** Shared Skill service for agent-scoped skill lookup. */ skillService?: SkillService; + /** Resolves the builtin Skill allow-list for an allowed direct delegation target. */ + resolveSkillListTargetSkills?: (agentType: string) => readonly string[] | undefined; /** Execution-owned immutable package snapshots for explicit one-shot Skills. */ executionSkillSnapshots?: ReadonlyMap; projectContext: ProjectContext; @@ -188,12 +190,9 @@ export interface ToolDescriptor< /** * Optional schema for the LLM. When set, this is used instead of * `inputSchema` when presenting the tool definition to the AI model via - * `toAITools()`. This allows MCP tools to expose their real JSON Schema - * parameter definitions so the model knows what arguments to pass, while - * keeping the loose Zod `inputSchema` for ArchCode's execution pipeline. - * - * Builtin tools leave this undefined — their Zod `inputSchema` serves both - * roles. + * `toAITools()`. Builtin and MCP tools may use this boundary to expose a + * portable model-facing contract while retaining `inputSchema` as the + * authoritative internal execution schema. */ aiInputSchema?: AiToolInputSchema; traits: ToolTraits; From fc63eb2deb53891ec48b01cd0e0e2e3bc79802be Mon Sep 17 00:00:00 2001 From: bo Date: Mon, 10 Aug 2026 16:50:21 +0800 Subject: [PATCH 2/2] test(agent-core): harden delegation contract coverage --- .../src/agents/configured-agent.test.ts | 28 +++++++++---------- .../src/runtime-skill-command.test.ts | 17 ++++++++++- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/packages/agent-core/src/agents/configured-agent.test.ts b/packages/agent-core/src/agents/configured-agent.test.ts index cf5ed280..abc772fb 100644 --- a/packages/agent-core/src/agents/configured-agent.test.ts +++ b/packages/agent-core/src/agents/configured-agent.test.ts @@ -271,8 +271,19 @@ function createAgent(options: { }); } const depth = options.depth ?? 0; - const canDelegate = options.definition.childPolicy !== undefined - && depth < options.definition.childPolicy.maxDepth; + const resolveAllowedTools = (definition: AgentDefinition, agentDepth: number) => { + const requested = [...definition.tools.tools, ...definition.roleContract.requiredCapabilities]; + const resolved = toolRegistry.resolveForAgent(requested).descriptors.map((tool) => tool.name); + if ( + definition.childPolicy === undefined + || (definition.tools.delegateTargets?.length ?? 0) === 0 + || agentDepth >= definition.childPolicy.maxDepth + ) { + return resolved.filter((name) => !(DELEGATION_CORE_TOOLS as readonly string[]).includes(name)); + } + return resolved; + }; + const canDelegate = resolveAllowedTools(options.definition, depth).includes("delegate"); const delegationTargets = canDelegate ? (options.definition.tools.delegateTargets ?? []).map((agentName) => { const target = defaultAgentDefinitions.find((candidate) => candidate.name === agentName); @@ -306,18 +317,7 @@ function createAgent(options: { depth, targets: Object.freeze(delegationTargets), }), - resolveAllowedTools: (definition, depth) => { - const requested = [...definition.tools.tools, ...definition.roleContract.requiredCapabilities]; - const resolved = toolRegistry.resolveForAgent(requested).descriptors.map((tool) => tool.name); - if ( - definition.childPolicy === undefined - || (definition.tools.delegateTargets?.length ?? 0) === 0 - || depth >= definition.childPolicy.maxDepth - ) { - return resolved.filter((name) => !(DELEGATION_CORE_TOOLS as readonly string[]).includes(name)); - } - return resolved; - }, + resolveAllowedTools, }); } diff --git a/packages/agent-core/src/runtime-skill-command.test.ts b/packages/agent-core/src/runtime-skill-command.test.ts index de858ddb..fd564e09 100644 --- a/packages/agent-core/src/runtime-skill-command.test.ts +++ b/packages/agent-core/src/runtime-skill-command.test.ts @@ -8,7 +8,7 @@ import { ServerConfigService, resolveServerConfigPath } from "./config"; import { silentLogger } from "./logger"; import { setLlmAdapterForTest } from "./llm"; import { ProjectRegistry } from "./projects/registry"; -import { createRuntime as createProductionRuntime } from "./runtime"; +import { type AgentRuntime, createRuntime as createProductionRuntime } from "./runtime"; import { sessionFileInternals } from "./store/helpers"; import { createTestMcpRuntime } from "./testing/test-mcp-runtime"; @@ -82,6 +82,19 @@ async function createRuntime() { }); } +function waitForFamilyIdle(runtime: AgentRuntime, projectSlug: string, rootSessionId: string): Promise { + return new Promise((resolve) => { + let unsubscribe = () => {}; + unsubscribe = runtime.subscribeSessionRuntimeChanges((event) => { + if (event.projectSlug !== projectSlug + || event.rootSessionId !== rootSessionId + || event.activity !== "idle") return; + unsubscribe(); + resolve(); + }); + }); +} + describe("runtime Skill command admission", () => { test("admits a custom Skill outside the Agent builtin allow-list and replays its normalized activation", async () => { const workspaceRoot = await makeTempRoot(); @@ -126,6 +139,7 @@ describe("runtime Skill command admission", () => { source: "user" as const, requestedModelSelection, }; + const familyIdle = waitForFamilyIdle(runtime, project.slug, session.sessionId); const accepted = await runtime.acceptSessionMessage({ ...base, text: `/skill use ${skillName} inspect changes`, @@ -140,6 +154,7 @@ describe("runtime Skill command admission", () => { ...base, text: "/skill use codemap inspect", })).rejects.toMatchObject({ reason: "idempotency" }); + await familyIdle; const file = await runtime.getSessionFile(workspaceRoot, session.sessionId); expect(file.inputRequestReceipts).toEqual([