refactor(tools): isolate invocation scope from pipeline state#1641
Conversation
a823849 to
6b6d8de
Compare
c9352e4 to
22ca0cf
Compare
|
This is largely a cleanup / refactoring PR - we had something like 24 parameters on the |
There was a problem hiding this comment.
Pull request overview
Refactors Netclaw’s internal tool-execution plumbing to enforce a required, immutable ToolRunScope and a sealed tool-facing ToolInvocationContext, isolating per-invocation outputs/approval attempt state while keeping tool schemas, persistence contracts, and runtime authorization behavior unchanged.
Changes:
- Removes context-free / nullable internal tool execution APIs and migrates call sites to required
ToolInvocationContext. - Separates tool-visible invocation context from pipeline-owned mutable approval/output state via composition.
- Updates production + test pipelines (daemon, actors, subagents, reminders, MCP) to construct validated scopes and per-call output sinks.
Reviewed changes
Copilot reviewed 131 out of 131 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Netclaw.Tools.Abstractions/NetclawTool.cs | Requires invocation context in base tool API |
| src/Netclaw.Tools.Abstractions/Netclaw.Tools.Abstractions.csproj | Exposes internals to test assemblies |
| src/Netclaw.Tools.Abstractions/IShellTrustZonePolicy.cs | Switches policy to invocation context |
| src/Netclaw.Tools.Abstractions/INetclawTool.cs | Removes context-free execution overload |
| src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs | Builds explicit sessionless run scope for REST reminders |
| src/Netclaw.Daemon/Mcp/McpClientManager.cs | Makes MCP invocation context required |
| src/Netclaw.Daemon/Configuration/ChannelSendTools.cs | Migrates channel send tool to invocation context |
| src/Netclaw.Daemon/Configuration/ChannelLookupTools.cs | Adds required invocation context parameter |
| src/Netclaw.Daemon.Tests/Tools/ToolExecutionTestExtensions.cs | Test-only adapter for old test call sites |
| src/Netclaw.Daemon.Tests/Tools/TestToolExecutionContext.cs | Shared test context builder for daemon tests |
| src/Netclaw.Daemon.Tests/Reminder/ReminderTargetResolutionPathTests.cs | Updates reminder tests to new context creation |
| src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerHttpHeaderTests.cs | Updates MCP smoke tests for required context |
| src/Netclaw.Daemon.Tests/Mcp/SmokeMcpServerArgumentCoercionTests.cs | Updates MCP coercion tests to pass context |
| src/Netclaw.Daemon.Tests/Mcp/McpProcessBoundStdioTests.cs | Updates MCP stdio tests to pass invocation context |
| src/Netclaw.Daemon.Tests/GlobalUsings.cs | Adds global using for tools test helpers |
| src/Netclaw.Daemon.Tests/Configuration/ChannelSendToolTests.cs | Migrates channel send tests to new context |
| src/Netclaw.Daemon.Tests/Configuration/ChannelLookupToolTests.cs | Migrates channel lookup tests to new context |
| src/Netclaw.Channels.Slack/Tools/LookupSlackUserTool.cs | Updates Slack tool ExecuteAsync signature |
| src/Netclaw.Channels.Mattermost/Tools/LookupMattermostUserTool.cs | Updates Mattermost tool signature + alias cleanup |
| src/Netclaw.Actors/Tools/WebSearchTool.cs | Updates tool signature to require invocation context |
| src/Netclaw.Actors/Tools/WebFetchTool.cs | Removes context-free fallback path |
| src/Netclaw.Actors/Tools/ToolRegistry.cs | Removes synthetic context creation for indexing |
| src/Netclaw.Actors/Tools/ToolOutputSpill.cs | Requires invocation context for spill decisions |
| src/Netclaw.Actors/Tools/ToolAudienceProfileResolver.cs | Moves audience decisions to required context/audience |
| src/Netclaw.Actors/Tools/ToolAccessPolicy.cs | Reworks authorization/approval flow around composed state |
| src/Netclaw.Actors/Tools/SkillReadResourceTool.cs | Requires invocation context; preserves audience guard |
| src/Netclaw.Actors/Tools/SkillManageTool.cs | Updates signature to require invocation context |
| src/Netclaw.Actors/Tools/SkillLoadTool.cs | Requires invocation context; preserves audience guard |
| src/Netclaw.Actors/Tools/ShellTrustZonePolicy.cs | Accepts invocation context for trust zone checks |
| src/Netclaw.Actors/Tools/ShellTool.cs | Uses semantic timeout from invocation context |
| src/Netclaw.Actors/Tools/SetWorkingDirectoryTool.cs | Removes context-free overload; uses invocation context |
| src/Netclaw.Actors/Tools/SetWebhookTool.cs | Removes context-free fallback; clarifies provenance |
| src/Netclaw.Actors/Tools/SearchToolsTool.cs | Uses invocation context for visibility filtering |
| src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs | Converts safe-verb policy to invocation context |
| src/Netclaw.Actors/Tools/ScopedFileAccessPolicy.cs | Converts file access policy to invocation context |
| src/Netclaw.Actors/Tools/McpToolAdapter.cs | Requires invocation context for execution |
| src/Netclaw.Actors/Tools/LoadToolTool.cs | Removes context-free overload; requires context |
| src/Netclaw.Actors/Tools/ListWebhooksTool.cs | Requires invocation context for execution |
| src/Netclaw.Actors/Tools/IToolExecutor.cs | Makes execution/authorization require context |
| src/Netclaw.Actors/Tools/IMcpToolInvoker.cs | Requires invocation context for MCP tool calls |
| src/Netclaw.Actors/Tools/FileWriteTool.cs | Removes context-free fallback; requires context |
| src/Netclaw.Actors/Tools/FileReadTool.cs | Writes model-input files via per-call output sink |
| src/Netclaw.Actors/Tools/FileListTool.cs | Removes context-free fallback; requires context |
| src/Netclaw.Actors/Tools/FileEditTool.cs | Routes writes through invocation context + access policy |
| src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs | Always dispatches with required context + output spill |
| src/Netclaw.Actors/Tools/DeleteWebhookTool.cs | Requires invocation context for execution |
| src/Netclaw.Actors/Tools/AttachFileTool.cs | Uses per-call output sink for attachments |
| src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs | Subagent activity emitted via per-call outputs sink |
| src/Netclaw.Actors/SubAgents/SubAgentSpawnBreadcrumbs.cs | Breadcrumbs now take invocation context |
| src/Netclaw.Actors/SubAgents/SubAgentActor.cs | Constructs run scope + per-call contexts for tool calls |
| src/Netclaw.Actors/SubAgents/SpawnAgentTool.cs | Requires invocation context for execution/streaming |
| src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs | Builds composed run scope + outputs, isolates approval state |
| src/Netclaw.Actors/Sessions/LlmSessionActor.cs | Builds scoped context for routed skill/subagent path |
| src/Netclaw.Actors/Reminders/SetReminderTool.cs | Requires invocation context for reminder tool |
| src/Netclaw.Actors/Reminders/ListRemindersTool.cs | Requires invocation context for reminder tool |
| src/Netclaw.Actors/Reminders/GetReminderHistoryTool.cs | Requires invocation context for reminder tool |
| src/Netclaw.Actors/Reminders/CancelReminderTool.cs | Requires invocation context for reminder tool |
| src/Netclaw.Actors/Memory/SqliteUpdateMemoryTool.cs | Requires invocation context for memory tool |
| src/Netclaw.Actors/Memory/SqliteStoreMemoryTool.cs | Requires invocation context for memory tool |
| src/Netclaw.Actors/Memory/SqliteGetMemoriesTool.cs | Requires invocation context for memory tool |
| src/Netclaw.Actors/Memory/SqliteFindMemoriesTool.cs | Requires invocation context for memory tool |
| src/Netclaw.Actors/Jobs/CheckBackgroundJobTool.cs | Requires invocation context for jobs tool |
| src/Netclaw.Actors.Tests/Tools/WebSearchToolTests.cs | Updates tests to pass required context |
| src/Netclaw.Actors.Tests/Tools/WebFetchToolTests.cs | Updates tests to pass required context |
| src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs | Updates spill tests to use invocation context |
| src/Netclaw.Actors.Tests/Tools/ToolLivenessValidatorTests.cs | Updates stub tools to new ExecuteAsync signature |
| src/Netclaw.Actors.Tests/Tools/ToolExecutionValueObjectTests.cs | Adds tests for new value objects + isolation |
| src/Netclaw.Actors.Tests/Tools/ToolExecutionTestExtensions.cs | Test-only adapters for ToolExecutionContext usage |
| src/Netclaw.Actors.Tests/Tools/ToolArgumentValidatorTests.cs | Updates meta/tool validation tests for new context |
| src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs | Updates approval gate tests for invocation context |
| src/Netclaw.Actors.Tests/Tools/TestToolExecutionContext.cs | Shared test context builder for actors tests |
| src/Netclaw.Actors.Tests/Tools/SkillToolTests.cs | Updates skill tool tests to required context |
| src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs | Updates shell tool tests to required context/timeout |
| src/Netclaw.Actors.Tests/Tools/ShellToolStreamingTests.cs | Updates streaming tests to required context |
| src/Netclaw.Actors.Tests/Tools/SetWorkingDirectoryAudienceTests.cs | Uses audience-only exposure checks |
| src/Netclaw.Actors.Tests/Tools/SetWebhookToolProvenanceTests.cs | Updates webhook tests to required context |
| src/Netclaw.Actors.Tests/Tools/SearchToolsToolTests.cs | Updates search tool tests to required context |
| src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs | Updates safe-verb tests to invocation context |
| src/Netclaw.Actors.Tests/Tools/ScopedFileAccessPolicyHomeExpansionTests.cs | Updates file policy tests to invocation context |
| src/Netclaw.Actors.Tests/Tools/SchedulingToolAudienceTests.cs | Uses audience-only exposure checks |
| src/Netclaw.Actors.Tests/Tools/PublicAudienceFileAccessPolicyTests.cs | Updates tests to invocation context |
| src/Netclaw.Actors.Tests/Tools/MetaFieldResolutionTests.cs | Updates stub tools to new ExecuteAsync signature |
| src/Netclaw.Actors.Tests/Tools/MessyCommandOneTimeApprovalTests.cs | Updates one-time approval tests to new context |
| src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs | Updates grants tests to use audience-only exposure |
| src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs | Updates MCP adapter tests to required context |
| src/Netclaw.Actors.Tests/Tools/ListWebhooksToolTests.cs | Updates list webhooks tests to required context |
| src/Netclaw.Actors.Tests/Tools/GeneratedToolStrictBindingTests.cs | Updates strict binding tests to pass context |
| src/Netclaw.Actors.Tests/Tools/FileWriteToolTests.cs | Updates file write tests to required context |
| src/Netclaw.Actors.Tests/Tools/FileReadToolTests.cs | Updates file read tests for output sink behavior |
| src/Netclaw.Actors.Tests/Tools/FileListToolTests.cs | Updates file list tests to required context |
| src/Netclaw.Actors.Tests/Tools/FileEditToolTests.cs | Updates file edit tests to required context |
| src/Netclaw.Actors.Tests/Tools/AutonomousZoneClampTests.cs | Updates autonomous clamp tests to invocation context |
| src/Netclaw.Actors.Tests/Tools/AttachFileToolTests.cs | Updates attach tool tests to required context |
| src/Netclaw.Actors.Tests/SubAgents/ToolExecutionContextResolveShellCwdTests | Updates CWD resolution tests to new context builder |
| src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnObservabilityTests.cs | Updates subagent observability tests to invocation context |
| src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs | Updates spawner tests to use invocation context + outputs sink |
| src/Netclaw.Actors.Tests/SubAgents/SubAgentActorTests.cs | Updates MCP invoker test double signature |
| src/Netclaw.Actors.Tests/SubAgents/SpawnAgentToolTests.cs | Updates spawn agent tests to required context |
| src/Netclaw.Actors.Tests/SubAgents/SpawnAgentStreamingTests.cs | Updates streaming spawn tests to new context builder |
| src/Netclaw.Actors.Tests/Sessions/ToolExecutionIntegrationTests.cs | Updates fake executor to required context + spill |
| src/Netclaw.Actors.Tests/Sessions/SubAgentSpawnIntegrationTests.cs | Asserts semantic timeout propagation into child tool calls |
| src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs | Updates pipeline tests for output sink model inputs |
| src/Netclaw.Actors.Tests/Sessions/Pipelines/StreamingToolCallTests.cs | Updates pipeline streaming test to required context |
| src/Netclaw.Actors.Tests/Sessions/Pipelines/MetaValidationAndNoticeTests.cs | Updates timeout-hint assertions to semantic timeout |
| src/Netclaw.Actors.Tests/Reminders/ReminderToolConfigGateTests.cs | Updates reminder gate tests to required context |
| src/Netclaw.Actors.Tests/Reminders/GetReminderHistoryToolTests.cs | Updates reminder history tests to required context |
| src/Netclaw.Actors.Tests/Memory/SqliteMemoryToolsTests.cs | Updates memory tool tests to required context |
| src/Netclaw.Actors.Tests/Memory/SqliteMemoryPolicyScopeTests.cs | Updates policy scope tests to required context |
| src/Netclaw.Actors.Tests/Memory/MemoryRedesignedEvalSuiteTests.cs | Updates eval suite harness tests to required context |
| src/Netclaw.Actors.Tests/Memory/FakeNetclawTool.cs | Updates fake tool to receive invocation context |
| src/Netclaw.Actors.Tests/Jobs/CheckBackgroundJobToolTests.cs | Updates job tool tests to new context builder |
| src/Netclaw.Actors.Tests/Jobs/BackgroundJobIntegrationTests.cs | Updates integration tests to required context |
| src/Netclaw.Actors.Tests/GlobalUsings.cs | Adds global using for tools test helpers |
| src/Netclaw.Actors.Tests/Channels/SlackProactiveThreadTests.cs | Updates Slack tool invocation in tests |
| openspec/changes/simplify-tool-execution-context/tasks.md | Tracks staged OpenSpec work items |
| openspec/changes/simplify-tool-execution-context/specs/tool-approval-gates/spec.md | Defines approval-gate invariants for refactor |
| openspec/changes/simplify-tool-execution-context/specs/session-cwd/spec.md | Updates working-context/Git requirements (future stages) |
| openspec/changes/simplify-tool-execution-context/specs/netclaw-tools/spec.md | Defines required invocation context requirements |
| openspec/changes/simplify-tool-execution-context/specs/netclaw-testing/spec.md | Defines required deterministic test coverage |
| openspec/changes/simplify-tool-execution-context/specs/netclaw-subagents/spec.md | Updates subagent working-context requirements |
| openspec/changes/simplify-tool-execution-context/specs/audience-context-filtering/spec.md | Specifies Public-audience suppression rules |
| openspec/changes/simplify-tool-execution-context/specs/actor-message-protocol/spec.md | Reaffirms external contract preservation |
| openspec/changes/simplify-tool-execution-context/proposal.md | Captures motivation and staged plan |
| openspec/changes/simplify-tool-execution-context/design.md | Documents architecture decisions and tradeoffs |
| openspec/changes/simplify-tool-execution-context/.openspec.yaml | Initializes OpenSpec change metadata |
| IMPLEMENTATION_PLAN.md | Adds “NOW” plan entry for staged execution-scope work |
| docs/spec/SPEC-011-daemon-architecture.md | Documents new tool execution scope model |
| AGENTS.md | Adds guideline forbidding global:: qualifier |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Stage 1 of the tool execution/context cleanup:
ToolRunScopeToolInvocationContextfrom pipeline-owned mutable approval state via compositionToolSessionScope.Sessionlesswithout changing its audience, boundary, approval, or delivery behaviorBehavior
This is an internal refactor. Tool authorization, MCP schemas, persistence, background routing, reminder behavior, output spilling, and model-visible skill guidance are intentionally unchanged.
An independent adversarial review initially found that inheritance allowed tools to downcast the invocation context and mutate pipeline approval state. The implementation now uses composition;
ToolInvocationContextis sealed and the dispatcher passes onlycontext.Invocationto tools.Validation
dotnet build Netclaw.slnx --no-restore --nologodotnet test Netclaw.slnx --no-build --nologodotnet slopwatch analyze— 0 issuespwsh ./scripts/Add-FileHeaders.ps1 -Verifyopenspec validate simplify-tool-execution-context --strictgit diff --checkThe behavioral eval harness was inspected but cannot run in this environment because the required eval provider type, endpoint, and model ID are not configured. No model-visible tool definition or skill behavior changed in this stage.
OpenSpec
Change:
simplify-tool-execution-contextStage 1 tasks 1.1–1.6 are complete. Task 1.7 remains open until CI/review, merge, and post-merge verification finish.