fix(cli): skip gateway secrets.resolve for exec-backed command targets (#96653)#97460
fix(cli): skip gateway secrets.resolve for exec-backed command targets (#96653)#97460ly-wang19 wants to merge 1 commit into
Conversation
openclaw#96653) Command-path resolution on the gateway only reads pre-resolved values from the active snapshot; exec-backed agent-runtime/model-provider SecretRefs surface as a per-turn UNAVAILABLE failure even though local fallback succeeds. Extend the preflight classifier to surface active exec-backed target paths and route them through the no-gateway path, mirroring the existing skip for gateway-credential exec refs. - collectConfiguredTargetRefPaths also exposes refsByPath - classifyConfiguredTargetRefs returns activeExecBackedPaths - resolveCommandSecretRefsViaGateway skips the RPC when exec-backed active targets are present and resolves locally instead Updates three existing exec-ref tests now that the gateway round-trip is correctly skipped, and adds a regression test for the issue's model-provider exec-ref scenario.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close: this PR targets a real SecretRef log-noise bug, but the same remaining work is already covered by the open, mergeable, proof-positive canonical PR with stronger active-target and no-local-exec safeguards. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this PR and keep review focused on So I’m closing this here and keeping the remaining discussion on #96661. Review detailsBest possible solution: Close this PR and keep review focused on Do we have a high-confidence way to reproduce the issue? Yes at source level: queued replies call Is this the best way to solve the issue? No for this branch: it is a plausible fix, but Security review: Security review cleared: No concrete supply-chain or security defect was found; the SecretRef execution-routing concern is captured as compatibility/auth-provider merge risk. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 8d168c836af3. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
rating: 🧂 unranked krab
1 participant
What Problem This Solves
On every agent reply turn, the gateway logs a noisy failure for
secrets.resolvewhen a model-provider credential is anexecSecretRef, even though the gateway resolved that same secret fine at startup and the turn proceeds normally via local fallback (~1 line/turn; thousands over time):Closes #96653.
Why This Change Was Made
The gateway's command-path resolver only reads pre-resolved values from the active snapshot; it cannot execute exec providers itself. For exec-backed agent-runtime/model-provider SecretRefs (e.g.
models.providers.<name>.apiKey), the per-turnsecrets.resolveRPC is therefore guaranteed to fail withUNAVAILABLE, after which the client falls back to local resolution and succeeds. The round-trip is pure overhead + noise, and the misleading⚠️ UNAVAILABLEline drowns out real failures during debugging.The existing skip in
resolveCommandSecretRefsViaGatewayonly covers gateway-credential exec refs (gateway.auth.*/gateway.remote.*) because those would prevent gateway authentication. The agent-runtime/model-provider case has the same futility but was not covered.User Impact
models.providers.<name>.apiKey = { source: "exec", ... }) no longer get a misleading per-turnsecrets.resolve UNAVAILABLElog entry on reply turns.reply: skipped gateway secrets.resolve because command targets use exec SecretRefs at <paths> that the gateway cannot resolve from the active snapshot; resolving locally.line that makes the intent clear.Evidence
pnpm test src/cli/command-secret-gateway.test.ts— 35/35 pass on local run (Node 22).skips gateway resolution when agent-runtime targets use exec SecretRefs (#96653)assertscallGatewayis NOT called, the exec provider resolves locally (exec-local-key), and the new skip diagnostic is emitted.keeps local exec SecretRef fallback enabled by default,skips local exec SecretRef fallback when the caller disallows exec providers, andcan preserve unresolved SecretRefs when local exec fallback is disabledto reflect that the gateway RPC is now skipped; their core assertions (exec marker present/absent, resolved/unresolved state, local exec skip diagnostic) are preserved.git diff --numstat: 40/-4 incommand-secret-gateway.ts, 75/-9 incommand-secret-gateway.test.ts. Non-test LOC growth ~36 from a new helper return field, a new skip branch, and inline comments explaining the gateway-snapshot invariant.