refactor(agents): require one resolved capability profile per run#99817
Conversation
applyFinalEffectiveToolPolicy and collectAttemptExplicitToolAllowlistSources took ~15 raw identity params each and silently re-resolved a second capability profile when callers forgot to pass one — with divergent session inputs (attempt allowlist sources resolved policy off the live session key while tool construction used the sandbox policy key). The profile param is now required, the fallback resolution stacks are deleted, and the attempt threads its single resolved profile into tool construction, the final bundled pass, and allowlist sources. Doctor and gateway tools.effective callers resolve their config-scope profile explicitly at the callsite. createOpenClawCodingTools keeps its optional self-resolve as public plugin-SDK convenience.
openclaw-barnacle
Bot
added
gateway
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: refactor(agents): require one resolved capability profile per run This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33ebb71c3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| modelId: params.modelId, | ||
| sandboxToolPolicy: params.sandboxToolPolicy, | ||
| }); | ||
| const capabilityProfile = params.conversationCapabilityProfile; |
There was a problem hiding this comment.
When scripts/e2e/agent-bundle-mcp-tools-docker.sh runs the in-container harness, test/e2e/qa-lab/runtime/agent-bundle-mcp-tools-docker-client.ts still calls applyFinalEffectiveToolPolicy with only sessionKey/agentId/senderIsOwner. Because this new contract leaves params.conversationCapabilityProfile undefined and the next line dereferences .policy for any non-empty MCP tool list, that packaged Docker smoke now throws before verifying bundled MCP policy instead of using the previous self-resolve fallback. Please update that dist-only caller to resolve/pass the profile, or keep a compatibility path for existing untyped internal callers.
Useful? React with 👍 / 👎.
…enclaw#99817) applyFinalEffectiveToolPolicy and collectAttemptExplicitToolAllowlistSources took ~15 raw identity params each and silently re-resolved a second capability profile when callers forgot to pass one — with divergent session inputs (attempt allowlist sources resolved policy off the live session key while tool construction used the sandbox policy key). The profile param is now required, the fallback resolution stacks are deleted, and the attempt threads its single resolved profile into tool construction, the final bundled pass, and allowlist sources. Doctor and gateway tools.effective callers resolve their config-scope profile explicitly at the callsite. createOpenClawCodingTools keeps its optional self-resolve as public plugin-SDK convenience.
1 participant
Summary
The final bundled-tool policy pass and the attempt allowlist-source readout now consume the run's single resolved conversation capability profile instead of optionally re-resolving their own.
applyFinalEffectiveToolPolicy(src/agents/embedded-agent-runner/effective-tool-policy.ts):conversationCapabilityProfileis required; the ~15 raw identity params (sessionKey,agentId, group/sender fields, model refs,sandboxToolPolicy) and the silent fallback re-resolution are deleted. The sandbox pipeline step readspolicy.sandboxPolicyfrom the profile.collectAttemptExplicitToolAllowlistSources(src/agents/embedded-agent-runner/run/attempt.ts): same contract; it now reports allowlist sources from the profile that actually constructed and filtered the run's tools. Previously the callsite omitted the profile, so the fallback resolved a second full profile per attempt with divergent inputs — policy/group trust keyed off the live session key while tool construction used the sandbox policy session key. Where those keys differ, the reported sources could disagree with enforcement; they can no longer.runtimeCapabilityProfileinto tool construction, the final bundled pass, and allowlist sources; the redundant raw-param forwarding blocks at those callsites are gone.src/flows/doctor-core-checks.runtime.ts) and gatewaytools.effective(src/gateway/server-methods/tools-effective.ts) resolve their config-scope profile explicitly at the callsite — same inputs as before, resolution now visible instead of implicit.createOpenClawCodingToolskeeps its optional profile param and self-resolve: it is public plugin-SDK surface (src/plugin-sdk/agent-harness.ts).applyFinalEffectiveToolPolicyis not SDK-exported (verified), so its signature change is internal.Net: one canonical profile resolution per embedded run, enforced by the type system rather than by convention; +95/-176.
Proof (local)
pnpm tsgoclean.pnpm vitest runoneffective-tool-policy.test.ts,agent-bundle-mcp-tools.request-boundary.test.ts, gatewaytools-effectivesuites,tool-allowlist-guard.test.ts, doctor runtime suites, attempt tool-plan suites — all green (77 + 44 + 33 across batches; suite tests updated to the resolve-then-apply composition mirroring production).oxfmt --checkclean on touched files.Non-goals
src/agents/harness/selection.ts) still resolves per entry point; threading the attempt profile there needs thebtw.tscall-chain reshaped and is left as follow-up.