refactor(tools): close execution context cleanup#1646
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR closes out the “tool execution context cleanup” refactor by removing the unused tool audit sink, making interactive approval an explicit required capability value, hardening Git working-context inspection failure classification, and tightening subagent lifecycle signaling while updating OpenSpec + engineering documentation to match shipped behavior.
Changes:
- Replace
SupportsInteractiveApproval+ nullableApprovalBridgewith a requiredInteractiveApprovalCapabilityunion (Unavailable/Available(bridge)), closing previously representable invalid states. - Remove
IToolAuditLogger/ToolAuditEntryplumbing from the session tool pipeline and composition, relying on the canonicalToolCallOutput/ToolResultOutputtranscript path. - Introduce deterministic “git executable not found” typing in working-context inspection and balance subagent activity-stream completion across snapshot/cancellation races, with updated tests.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Netclaw.Tools.Abstractions/ToolExecutionContext.cs | Makes interactive approval a required run-scope capability and removes legacy interactive-approval flag/bridge accessors. |
| src/Netclaw.Tools.Abstractions/IParentApprovalBridge.cs | Adds InteractiveApprovalCapability union type to represent interactive approval availability with a required bridge. |
| src/Netclaw.Daemon/Reminders/ReminderEndpointRouteBuilderExtensions.cs | Updates reminder tool execution to pass InteractiveApprovalCapability.Unavailable. |
| src/Netclaw.Daemon/Program.cs | Removes registration/wiring of the now-deleted IToolAuditLogger from daemon DI composition. |
| src/Netclaw.Daemon.Tests/Tools/TestToolExecutionContext.cs | Updates daemon test helper to construct contexts with explicit interactive approval capability. |
| src/Netclaw.Actors/Tools/ToolAccessPolicy.cs | Switches non-interactive enforcement checks to the new capability union. |
| src/Netclaw.Actors/Tools/ScopedFileAccessPolicy.cs | Applies autonomous-zone clamp based on InteractiveApprovalCapability.Unavailable. |
| src/Netclaw.Actors/Tools/IToolExecutor.cs | Removes ToolAuditEntry/IToolAuditLogger/NullToolAuditLogger types. |
| src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs | Balances subagent notifications and stream completion across initial snapshot failure/cancellation and spawn/ask races; forwards interactive approval as a capability. |
| src/Netclaw.Actors/SubAgents/SubAgentActor.cs | Derives _approvalBridge from InteractiveApprovalCapability.Available instead of a nullable scope property. |
| src/Netclaw.Actors/Sessions/WorkingContextSnapshot.cs | Adds typed ExecutableNotFound result for Git inspection; implements deterministic classification in runner/inspector and rendering. |
| src/Netclaw.Actors/Sessions/SessionDependencies.cs | Removes audit logger dependency from SessionToolServices. |
| src/Netclaw.Actors/Sessions/Pipelines/SessionToolExecutionPipeline.cs | Removes audit sink usage and uses InteractiveApprovalCapability for per-call scope construction. |
| src/Netclaw.Actors/Sessions/LlmSessionActor.cs | Updates pipeline construction and internal tool contexts to match removed audit logger + new interactive approval capability. |
| src/Netclaw.Actors.Tests/Tools/ToolExecutionValueObjectTests.cs | Adds null-guard test coverage for required interactive approval capability. |
| src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs | Updates test contexts to use capability union for interactive/non-interactive cases. |
| src/Netclaw.Actors.Tests/Tools/TestToolExecutionContext.cs | Updates shared test context factory and introduces a test IParentApprovalBridge implementation for Available capability. |
| src/Netclaw.Actors.Tests/Tools/MessyCommandOneTimeApprovalTests.cs | Migrates test context setup to capability union. |
| src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs | Migrates multiple approval-gate test contexts to capability union. |
| src/Netclaw.Actors.Tests/Tools/AutonomousZoneClampTests.cs | Updates autonomous clamp behavior test to reflect explicit “unavailable” capability (no nullable legacy state). |
| src/Netclaw.Actors.Tests/SubAgents/SubAgentTestScope.cs | Updates subagent scope creation to carry InteractiveApprovalCapability instead of bridge/flag fields. |
| src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs | Adds deterministic coverage for initial snapshot failure/cancellation and stream completion; migrates approval bridging assertions to capability union. |
| src/Netclaw.Actors.Tests/Sessions/WorkingContextSnapshotTests.cs | Adds stable typed missing-git coverage and updates existing Git inspection tests for the new failure kind. |
| src/Netclaw.Actors.Tests/Sessions/ToolLoopCompactionTests.cs | Removes unused audit logger fixture wiring. |
| src/Netclaw.Actors.Tests/Sessions/ToolExecutionIntegrationTests.cs | Removes audit logger assertions and deletes the fake audit logger implementation. |
| src/Netclaw.Actors.Tests/Sessions/SessionToolExecutionPipelineTests.cs | Updates assertions to validate InteractiveApprovalCapability on derived run scope/context. |
| src/Netclaw.Actors.Tests/Sessions/Pipelines/SessionToolPipelineTestFixture.cs | Removes audit logger injection path for pipeline fixture. |
| src/Netclaw.Actors.Tests/Sessions/Pipelines/MetaValidationAndNoticeTests.cs | Updates malformed-meta test to validate denial semantics via tool result content (no audit sink). |
| src/Netclaw.Actors.Tests/Sessions/MaxToolIterationTests.cs | Removes unused audit logger fixture wiring. |
| src/Netclaw.Actors.Tests/Sessions/LlmSessionTestExtensions.cs | Removes default IToolAuditLogger DI registration in test composite setup. |
| src/Netclaw.Actors.Tests/Sessions/EmptyResponseEscalationTests.cs | Removes unused audit logger fixture wiring. |
| src/Netclaw.Actors.Tests/Sessions/ApprovalRehydrationTests.cs | Updates test recording to infer interactive approval support from capability union. |
| openspec/changes/simplify-tool-execution-context/tasks.md | Marks Stage 3/closeout tasks as completed and records validation/eval environment block evidence. |
| openspec/changes/simplify-tool-execution-context/specs/tool-call-metadata/spec.md | Documents removal of the unused audit-entry enrichment requirement. |
| openspec/changes/simplify-tool-execution-context/specs/session-cwd/spec.md | Updates spec to include executable-not-found as an explicit Git inspection outcome. |
| openspec/changes/simplify-tool-execution-context/specs/netclaw-tools/spec.md | Updates tool pipeline requirements to reflect single transcript ownership and required interactive-approval capability union. |
| openspec/changes/simplify-tool-execution-context/proposal.md | Updates proposal to reflect audit-sink removal and capability changes. |
| openspec/changes/simplify-tool-execution-context/design.md | Updates design to document interactive approval union and rationale for removing the no-op audit sink. |
| IMPLEMENTATION_PLAN.md | Marks Stage 3 and quality gates complete and records eval environment block rationale. |
| docs/spec/value-object-audit.md | Updates audit doc references after removing ToolAuditEntry. |
| docs/spec/SPEC-002-session-lifecycle-and-protocol.md | Updates session lifecycle spec to reflect removal of audit dependency and new interactive approval capability union. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Aaronontheweb
enabled auto-merge (squash)
July 15, 2026 01:11
Aaronontheweb
added
the
cleanup
This was referenced Jul 16, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behavior and compatibility
This removes an internal sink whose production implementation discarded every entry. Existing ToolCallOutput and ToolResultOutput publication, session.log behavior, tool results, approval outcomes, MCP schemas, and persisted actor contracts remain unchanged. The approval union closes a previously representable invalid state; all production composition paths already constructed a definite interactive or non-interactive state.
Validation
Closes #1633