diff --git a/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs b/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs index 4d4587b19..0f91800a7 100644 --- a/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs +++ b/crates/webcodex-tool-contracts/src/tests/policy_contracts.rs @@ -1,5 +1,185 @@ use super::*; +#[test] +fn tool_definitions_are_composition_contract_ssot() { + let parallel = ToolCompositionContract { + eligibility: ToolCompositionPolicy::Allowed, + concurrency: ToolConcurrencyPolicy::Parallel, + }; + assert_eq!( + ToolCompositionContract::CONSERVATIVE, + ToolCompositionContract { + eligibility: ToolCompositionPolicy::Denied, + concurrency: ToolConcurrencyPolicy::Sequential, + } + ); + assert_eq!( + runtime_tool_composition_contract("git_review_summary"), + parallel + ); + + for definition in tool_definitions() { + let expected = if definition.name == "git_review_summary" { + parallel + } else { + ToolCompositionContract::CONSERVATIVE + }; + assert_eq!(definition.composition, expected, "{}", definition.name); + assert_eq!( + runtime_tool_composition_contract(definition.name), + definition.composition, + "{} composition lookup must use ToolDefinition", + definition.name + ); + } +} + +#[test] +fn composition_defaults_cover_unreviewed_tools_and_unknown_gateways() { + for name in [ + "read_files", + "search_project_texts", + "project_overview", + "git_diff_hunks", + "show_changes", + "write_project_file", + "apply_text_edits", + "apply_patch", + "git_commit_paths", + "start_session", + "work_on_project", + "finish_coding_task", + "close_session", + "post_session_message", + "resolve_session_message", + "run_shell", + "run_script", + "run_process", + "run_detached_process", + "session_shell_exec", + "run_job", + "observe_jobs", + "stop_job", + "cargo_check", + "cargo_test", + "cargo_fmt", + "computer_observe", + "computer_control", + "plugin_tool", + "ssh_resource", + ] { + assert!(lookup_tool_definition(name).is_some(), "{name}"); + assert_eq!( + runtime_tool_composition_contract(name), + ToolCompositionContract::CONSERVATIVE, + "{name}" + ); + } + for name in [ + "__unknown_composition_tool__", + "call_runtime_tool", + "tool_search", + ] { + assert!(lookup_tool_definition(name).is_none(), "{name}"); + assert_eq!( + runtime_tool_composition_contract(name), + ToolCompositionContract::CONSERVATIVE, + "{name}" + ); + } +} + +#[test] +fn composition_opt_in_requires_an_explicit_declaration() { + let definition = lookup_tool_definition("git_status").unwrap(); + assert_eq!(definition.metadata.effect, ToolEffect::Observe); + assert_eq!(definition.metadata.risk, ToolRisk::Read); + assert_eq!(definition.metadata.idempotency, ToolIdempotency::PureRead); + assert_eq!( + definition.context_continuity_policy(), + ToolContextContinuityPolicy::REOBSERVABLE + ); + assert_eq!( + runtime_tool_composition_contract(definition.name), + ToolCompositionContract::CONSERVATIVE + ); +} + +#[test] +fn parallel_composition_requires_allowed_eligibility() { + for definition in tool_definitions() + .filter(|definition| definition.composition.concurrency == ToolConcurrencyPolicy::Parallel) + { + assert_eq!( + definition.composition.eligibility, + ToolCompositionPolicy::Allowed, + "{}", + definition.name + ); + } +} + +#[test] +fn git_review_summary_composition_opt_in_matches_reviewed_facts() { + let definition = lookup_tool_definition("git_review_summary").unwrap(); + assert!(is_adaptive_runtime_direct_tool(definition.name)); + assert_eq!(definition.metadata.effect, ToolEffect::Observe); + assert_eq!(definition.metadata.idempotency, ToolIdempotency::PureRead); + assert_eq!( + definition.context_continuity_policy(), + ToolContextContinuityPolicy::REOBSERVABLE + ); + assert_eq!(definition.execution, None); + assert_eq!( + definition.session_evidence.lifecycle, + ToolSessionLifecycleEffect::None + ); + assert!(!definition.metadata.destructive); + assert!(!definition.metadata.shell_like); +} + +#[test] +fn composition_policy_stays_out_of_tool_specs_and_annotations() { + let spec_keys = BTreeSet::from([ + "name", + "description", + "inputSchema", + "outputSchema", + "annotations", + ]); + let annotation_keys = BTreeSet::from([ + "readOnlyHint", + "destructiveHint", + "idempotentHint", + "openWorldHint", + ]); + for spec in registered_tool_specs() { + let serialized = serde_json::to_value(&spec).unwrap(); + assert_eq!( + serialized + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(), + spec_keys, + "{} ToolSpec fields", + spec.name + ); + assert_eq!( + serialized["annotations"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(), + annotation_keys, + "{} annotation fields", + spec.name + ); + } +} + #[test] fn tool_definitions_are_context_continuity_ssot() { use crate::metadata::ToolEffect; diff --git a/crates/webcodex-tool-contracts/src/tool_definition.rs b/crates/webcodex-tool-contracts/src/tool_definition.rs index 2420055f4..9d92e7989 100644 --- a/crates/webcodex-tool-contracts/src/tool_definition.rs +++ b/crates/webcodex-tool-contracts/src/tool_definition.rs @@ -59,10 +59,10 @@ pub use super::tool_policy::{ runtime_tool_activity_interaction, runtime_tool_activity_semantics, runtime_tool_advances_context_checkpoint, runtime_tool_approval_policy, runtime_tool_captures_validation_output, runtime_tool_category, - runtime_tool_effect_annotations, runtime_tool_execution_contract, - runtime_tool_is_change_summary_like, runtime_tool_is_git_like, runtime_tool_is_read_like, - runtime_tool_is_shell_like, runtime_tool_is_write_like, runtime_tool_metadata, - runtime_tool_operator_extension_family, runtime_tool_permission_risk, + runtime_tool_composition_contract, runtime_tool_effect_annotations, + runtime_tool_execution_contract, runtime_tool_is_change_summary_like, runtime_tool_is_git_like, + runtime_tool_is_read_like, runtime_tool_is_shell_like, runtime_tool_is_write_like, + runtime_tool_metadata, runtime_tool_operator_extension_family, runtime_tool_permission_risk, runtime_tool_requires_permission, runtime_tool_runner_capability, runtime_tool_session_evidence_policy, runtime_tool_session_risk_class, }; @@ -828,6 +828,37 @@ impl ToolExecutionContract { } } +/// Explicit eligibility for future canonical tool composition. This declaration +/// does not grant authority or change direct-tool admission. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolCompositionPolicy { + Denied, + Allowed, +} + +/// Reviewed concurrency within composition, covering every supported input and +/// provider path. Read-only metadata alone does not imply parallel safety. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolConcurrencyPolicy { + Sequential, + Parallel, +} + +/// Static composition facts owned only by ToolDefinition. Phase 0 records these +/// facts without adding a runtime consumer or model-facing projection. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ToolCompositionContract { + pub eligibility: ToolCompositionPolicy, + pub concurrency: ToolConcurrencyPolicy, +} + +impl ToolCompositionContract { + pub const CONSERVATIVE: Self = Self { + eligibility: ToolCompositionPolicy::Denied, + concurrency: ToolConcurrencyPolicy::Sequential, + }; +} + /// Static Stateless Operator protocol-extension classification. This declares only /// which protocol capability family admits a hidden runtime tool; authorization, /// permission, Project authority, and Runner capability remain independent. @@ -934,6 +965,8 @@ pub struct ToolDefinition { pub operator_extension_family: Option, /// Optional canonical selection semantics for ordinary execution tools. pub execution: Option, + /// Explicit composition policy; unreviewed tools remain denied/sequential. + pub composition: ToolCompositionContract, pub visibility: ToolVisibility, pub category: &'static str, pub metadata: ToolMetadata, @@ -960,6 +993,11 @@ impl ToolDefinition { self } + pub const fn with_composition(mut self, composition: ToolCompositionContract) -> Self { + self.composition = composition; + self + } + /// Override only GPT Actions presentation text. This never changes the /// canonical ToolSpec schema, semantic contract, authority, or MCP copy. pub const fn with_gpt_action_description(mut self, description: &'static str) -> Self { @@ -1136,6 +1174,7 @@ const fn def( model_surface: ToolModelSurfaceDeclaration::DEFAULT, operator_extension_family: None, execution: None, + composition: ToolCompositionContract::CONSERVATIVE, visibility, category, metadata: make_tool_metadata( diff --git a/crates/webcodex-tool-contracts/src/tool_definition/git.rs b/crates/webcodex-tool-contracts/src/tool_definition/git.rs index b58024e4c..8a30930a6 100644 --- a/crates/webcodex-tool-contracts/src/tool_definition/git.rs +++ b/crates/webcodex-tool-contracts/src/tool_definition/git.rs @@ -54,7 +54,11 @@ pub(super) const SUMMARY_DEFINITIONS: &[ToolDefinition] = &[ git_review_summary_input_schema, )))), 120, - ), + ) + .with_composition(super::ToolCompositionContract { + eligibility: super::ToolCompositionPolicy::Allowed, + concurrency: super::ToolConcurrencyPolicy::Parallel, + }), adaptive_runtime_direct( context_reobservable(change_summary_like(git_like(model_spec( def( diff --git a/crates/webcodex-tool-contracts/src/tool_policy.rs b/crates/webcodex-tool-contracts/src/tool_policy.rs index 39b9818ba..70757d435 100644 --- a/crates/webcodex-tool-contracts/src/tool_policy.rs +++ b/crates/webcodex-tool-contracts/src/tool_policy.rs @@ -5,12 +5,13 @@ use super::metadata::{ }; use super::tool_definition::{ tool_definitions, RunnerCapabilityRequirement, ToolActivityInteraction, ToolActivityKind, - ToolActivityPresentation, ToolActivitySemantics, ToolAuditPolicy, ToolContextContinuityPolicy, - ToolDefinition, ToolDiffReviewEvidence, ToolEffectAnnotations, ToolExecutionContract, - ToolExecutionForm, ToolExplorationEvidence, ToolGptActionExposure, ToolOperatorExtensionFamily, - ToolReviewEvidence, ToolSessionEvidencePolicy, ToolValidationIdentityKind, - PERMISSION_RISK_ARTIFACT_WRITE, PERMISSION_RISK_DESTRUCTIVE, PERMISSION_RISK_PATCH, - PERMISSION_RISK_SHELL, PERMISSION_RISK_VALIDATION, PERMISSION_RISK_WRITE, TOOL_CATEGORY_JOB, + ToolActivityPresentation, ToolActivitySemantics, ToolAuditPolicy, ToolCompositionContract, + ToolContextContinuityPolicy, ToolDefinition, ToolDiffReviewEvidence, ToolEffectAnnotations, + ToolExecutionContract, ToolExecutionForm, ToolExplorationEvidence, ToolGptActionExposure, + ToolOperatorExtensionFamily, ToolReviewEvidence, ToolSessionEvidencePolicy, + ToolValidationIdentityKind, PERMISSION_RISK_ARTIFACT_WRITE, PERMISSION_RISK_DESTRUCTIVE, + PERMISSION_RISK_PATCH, PERMISSION_RISK_SHELL, PERMISSION_RISK_VALIDATION, + PERMISSION_RISK_WRITE, TOOL_CATEGORY_JOB, }; impl ToolDefinition { @@ -231,6 +232,14 @@ pub fn runtime_tool_execution_contract(name: &str) -> Option ToolCompositionContract { + lookup_tool_definition(name) + .map(|definition| definition.composition) + .unwrap_or(ToolCompositionContract::CONSERVATIVE) +} + pub fn runtime_tool_session_evidence_policy(name: &str) -> ToolSessionEvidencePolicy { lookup_tool_definition(name) .map(|definition| definition.session_evidence_policy()) diff --git a/docs/architecture/tool-composition-research.md b/docs/architecture/tool-composition-research.md index 1025e9ba3..b36419019 100644 --- a/docs/architecture/tool-composition-research.md +++ b/docs/architecture/tool-composition-research.md @@ -1,14 +1,16 @@ # Tool composition research and development plan -Status: deferred exploratory design note. This document records research findings -and a staged direction for reducing model/tool round trips. It is intentionally +Status: deferred exploratory design note. Phase 0 measurement and canonical policy +inventory now have initial implementations; composition runtime remains deferred. +This document records research findings and a staged direction for reducing +model/tool round trips. It is intentionally **downstream** of the current tool-contract friction/style work in [`../agent/tool-contract-guidelines.md`](../agent/tool-contract-guidelines.md): first make primitive tools consistent and low-friction, then evaluate surface pruning, and only then decide whether composition still removes meaningful outer -turns. It is not a current runtime contract and does not authorize implementation -shortcuts around existing tool, Session, Job, permission, audit, or Project -boundaries. +turns. The composition proposal is not a current runtime contract and does not +authorize implementation shortcuts around existing tool, Session, Job, permission, +audit, or Project boundaries. ## Motivation @@ -136,8 +138,8 @@ A WebCodex composition layer should follow these rules: `tools` object. 4. **No recursive composition.** A composition program cannot invoke the composition tool itself, directly or through adaptive discovery. -5. **Parallelism is opt-in and bounded.** A tool definition or adjacent canonical - registry owns its concurrency policy. Unknown tools are sequential or denied, +5. **Parallelism is opt-in and bounded.** `ToolDefinition.composition` owns + eligibility and concurrency policy. Unknown tools are denied and sequential, never optimistically parallel. 6. **Adaptive discovery is not bypassed.** Composition must not become a backdoor for model-hidden, gateway-only, or otherwise non-admitted tools. The first @@ -262,20 +264,44 @@ Specialized gateways should be excluded from the first version. If they are ever admitted, they must continue through their existing action-specific governance boundary rather than becoming generic nested callbacks. -## Concurrency contract +## Current composition policy contract (Phase 0) -The first useful runtime contract can remain small: +`webcodex-tool-contracts` owns a small static contract: ```text -ToolConcurrencyPolicy::Parallel -ToolConcurrencyPolicy::Sequential -CompositionPolicy::Allowed | Denied +ToolCompositionPolicy::{Denied, Allowed} +ToolConcurrencyPolicy::{Sequential, Parallel} +ToolCompositionContract { eligibility, concurrency } ``` -The default should be denied or sequential until a tool is reviewed. Read-only, -independent inspection tools are the first candidates for `Parallel`. Mutation, -Session-management, publication, release, Git-index/worktree mutation, and other -shared-state tools stay sequential initially. +`ToolDefinition.composition` is the sole declaration, set explicitly with the +const `with_composition(...)` builder. The shared definition constructor defaults +to `ToolCompositionContract::CONSERVATIVE` (`Denied + Sequential`). +`runtime_tool_composition_contract(name)` returns that field, or `CONSERVATIVE` +for an unknown name, including adapter-only gateways. It does not infer opt-in +from names, effects, idempotency, capabilities, or direct exposure. + +A declaration covers every supported input and provider path. The initial +inventory is deliberately narrow: + +| Tool | Policy | Review basis | +|---|---|---| +| `git_review_summary` | `Allowed + Parallel` | Already adaptive direct; compares exact commits with lazy fetch, optional Git locks, external diff, and textconv disabled. Each Git inspection uses a separate temporary view; synchronous Runner requests create no Job and result computation is invocation-local. | +| `read_files` | `Denied + Sequential` | Uses a shared revision registry with eviction and epoch rotation. Locking alone does not prove cross-call continuation independence. | +| `search_project_texts` | `Denied + Sequential` | May use a shared external MCP provider; parallel safety is not established for every configuration. | +| `project_overview` | `Denied + Sequential` | Native metadata inspection, but currently adaptive long tail; the initial inventory stays within existing direct tools. | +| `git_diff_hunks` | `Denied + Sequential` | Its worktree mode does not disable Git's default index stat refresh; policy must cover all input modes. See [Git's `diff.autoRefreshIndex` contract](https://git-scm.com/docs/git-config#Documentation/git-config.txt-diffautoRefreshIndex). | + +All other definitions retain `CONSERVATIVE`; no current tool declares +`Allowed + Sequential`. Inspection may use invocation-owned temporary directories +that are cleaned up when it ends. Existing audit and Workflow Session evidence +recording keep their normal paths; `git_review_summary` does not advance a context +checkpoint or mutate Session lifecycle, messages, or permissions. + +This is an internal inventory only: no serialization, ToolSpec/annotation/model +fields, second registry, execution consumer, scheduler, or composition runtime is +added. Future composition must still apply ordinary admission and authority +checks. Phase 1 remains unimplemented, and direct tools remain first-class. A later, evidence-driven extension may add a resource key such as: @@ -400,8 +426,8 @@ must remain separate because only the former are wholly service-owned. - Use Window/server-trace correlation to measure model-facing outer-call counts, tool durations, Runner timing available today, and gaps outside WebCodex. - Classify existing direct tools by composition eligibility and concurrency safety. -- Identify which current ToolDefinition fields can own the policy without a - parallel registry. +- Keep eligibility and concurrency on `ToolDefinition.composition`, without a + parallel registry (the policy foundation above is implemented). - Define parent/child diagnostic identity and bounded audit projection before changing execution behavior. - Establish representative review/implementation traces to compare outer MCP @@ -409,6 +435,10 @@ must remain separate because only the former are wholly service-owned. The concrete Direct-vs-Code-Mode capture/report protocol is documented in [`../experiments/agent-loop-baseline.md`](../experiments/agent-loop-baseline.md). +The canonical policy inventory complements the baseline profiler added in +[#489](https://github.com/yyjeqhc/webcodex/pull/489); it does not implement the +measurement work or the Phase 1 runtime. + Success means we can explain where elapsed time is spent without claiming access to model-private state. @@ -531,8 +561,8 @@ A production-ready first slice should satisfy all of the following: admitted direct surface? 3. How should nested tool schemas be made ergonomic without recreating the large composed-schema projection problem observed in WebCodex Next? -4. What is the smallest canonical concurrency metadata that supports Phase 1 - without prematurely designing resource locks for mutation? +4. Which additional inspection tools can satisfy the canonical composition + contract across all input/provider paths without shared-state hazards? 5. How should parent/child invocation identities appear in ActionAudit and the Windows view while preserving current privacy policy? 6. What final Session continuity projection is correct once one parent response