Skip to content

fix(secrets): inactive web refs no longer block agent turns#108485

Merged
steipete merged 3 commits into
openclaw:mainfrom
VACInc:fix/partial-web-secret-fallback
Jul 16, 2026
Merged

fix(secrets): inactive web refs no longer block agent turns#108485
steipete merged 3 commits into
openclaw:mainfrom
VACInc:fix/partial-web-secret-fallback

Conversation

@VACInc

@VACInc VACInc commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related: #105536

Summary

Accept locally classified inactive SecretRefs as terminal when a successful Gateway secrets.resolve response contains only a partial command snapshot.

What Problem This Solves

Fixes an issue where ordinary agent turns could fail before model execution when a disabled or non-selected web provider retained a SecretRef and the active Gateway returned a valid but incomplete secrets snapshot.

Root Cause

PR #105160 changed the Gateway command resolver to preserve authoritative assignments from partial runtime snapshots. That made the caller's targeted local-fallback merge reachable for paths omitted by a successful Gateway response.

Before this change, that merge accepted only resolved_local. The existing local resolver correctly classified omitted disabled-provider paths as inactive_surface, but the merge ignored that terminal state and relabeled the path unresolved. In strict agent preparation, it then threw is unresolved in the active runtime snapshot before the agent run began.

The related #105536 addresses how new runtime snapshots collect inactive web-provider credentials. This change is complementary: it repairs the established partial-result fallback contract even when a Gateway snapshot omits an inactive path.

Why This Change Was Made

The partial-result merge now handles both locally materialized values and locally proven inactive paths. Genuine unresolved paths keep the existing strict failure behavior, and the local-resolution summary still counts only paths that materialized a value.

User Impact

Agent turns no longer fail merely because an inactive web provider credential is absent from an otherwise valid partial Gateway snapshot. Missing credentials on active surfaces still fail as before.

Evidence

On current main, the added regression failed with 1 failed / 35 passed and agent: plugins.entries.google.config.webSearch.apiKey is unresolved in the active runtime snapshot. With this patch, the focused CLI and runtime-secret command passes both Vitest shards with 40 tests passed; strict preparation records the omitted path as inactive_surface with no unresolved targets.

Real behavior proof

  • Before behavior: the added focused regression failed on current main with 1 failed / 35 passed and agent: plugins.entries.google.config.webSearch.apiKey is unresolved in the active runtime snapshot.
  • Exact after-patch command: node scripts/run-vitest.mjs src/cli/command-secret-gateway.test.ts src/secrets/runtime-command-secrets.test.ts
  • Copied after result: 2 Vitest shards passed; 40 tests passed.
  • Observed after behavior: strict agent preparation returns hadUnresolvedTargets: false, preserves the configured SecretRef, records inactive_surface, and retains the existing inactive-surface diagnostic.

Verification

  • node scripts/run-vitest.mjs src/cli/command-secret-gateway.test.ts src/secrets/runtime-command-secrets.test.ts — 40 tests passed.
  • ./node_modules/.bin/oxlint src/cli/command-secret-gateway.ts src/cli/command-secret-gateway.test.ts — passed.
  • ./node_modules/.bin/oxfmt --check src/cli/command-secret-gateway.ts src/cli/command-secret-gateway.test.ts — passed.
  • git diff --check — passed.
  • .agents/skills/autoreview/scripts/autoreview --mode uncommitted --stream-engine-output — clean, no findings; patch correct at 0.94 confidence.

What was not tested

A live Telegram turn against a restarted patched Gateway was not run because that would mutate the operator's live service. The full remote changed gate was attempted but did not execute: Blacksmith Testbox authentication and the owned AWS Crabbox broker login are unavailable on this host. Exact-head non-draft PR CI is the authoritative broad gate.

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 15, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 15, 2026, 7:37 PM ET / 23:37 UTC.

Summary
The PR updates partial Gateway SecretRef fallback reconciliation to accept locally classified inactive paths as terminal and adds a disabled-web-provider regression test.

PR surface: Source +5, Tests +50. Total +55 across 2 files.

Reproducibility: yes. at source and regression-test level: a successful empty secrets.resolve response followed by local inactive_surface classification reaches the current resolved_local-only merge and remains unresolved. A real current-main Gateway run demonstrating that exact path was not supplied.

Review metrics: 1 noteworthy metric.

  • Partial-fallback terminal states: 1 added, 2 handled total. The merge now recognizes both resolved_local and inactive_surface as handled while retaining unresolved as fail-closed.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal or live-output proof from an actual patched CLI and Gateway showing an inactive ref permits the agent turn and an active selected-provider ref still blocks before model execution.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The supplied proof is focused Vitest/mock output rather than an after-fix agent turn against an actual Gateway; add redacted terminal or live-output evidence for inactive success and selected-provider failure, then update the PR body to trigger a fresh ClawSweeper review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR has no after-fix proof from an actual CLI agent turn using a real Gateway partial snapshot, so the key process-boundary behavior remains demonstrated only through mocked Gateway responses.
  • [P1] This changes credential/provider terminal-state handling: an incorrect local inactive classification could allow a credential required by the active provider to proceed, so proof must also show that a selected provider with an unresolved SecretRef still fails before model execution.
  • [P1] The branch is mergeable but behind current main; the focused review and relevant checks should be refreshed on the eventual merge head if the surrounding resolver changes during rebase.

Maintainer options:

  1. Prove both inactive and active paths (recommended)
    Before merge, run an actual patched CLI and Gateway agent turn showing the disabled or non-selected ref does not block execution while the selected provider's missing ref still fails before the model request.
  2. Pause for secret-resolution owner review
    If real partial-snapshot proof cannot be produced, keep the PR open until a secret-resolution owner confirms that local inactive classification is authoritative at this Gateway/client boundary.

Next step before merge

  • [P1] Wait for contributor-supplied real Gateway agent-turn proof, then route the exact merge head through secret-resolution and security-sensitive owner review; no mechanical code repair is currently identified.

Security
Cleared: The diff adds no dependency, workflow, permission, persistence, logging, or secret-value exposure surface, and active unresolved paths remain fail-closed.

Review details

Best possible solution:

Keep one reconciliation rule that preserves authoritative Gateway assignments, accepts the existing local resolver's terminal inactive_surface state, and continues failing every genuinely unresolved active path, while coordinating the upstream collection improvement in #105536.

Do we have a high-confidence way to reproduce the issue?

Yes at source and regression-test level: a successful empty secrets.resolve response followed by local inactive_surface classification reaches the current resolved_local-only merge and remains unresolved. A real current-main Gateway run demonstrating that exact path was not supplied.

Is this the best way to solve the issue?

Yes, this is the narrowest maintainable repair: it preserves the established targeted fallback owner boundary and recognizes the local resolver's existing terminal state without weakening failure behavior for active unresolved refs.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8e6f966482b0.

Label changes

Label justifications:

  • P2: This is a focused fix for an edge-case agent-turn blocker with limited configuration and version-skew conditions rather than a broad runtime outage.
  • merge-risk: 🚨 auth-provider: The patch changes whether a provider credential state is terminal or blocking, so an incorrect inactive classification could bypass required active-provider credential resolution.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The supplied proof is focused Vitest/mock output rather than an after-fix agent turn against an actual Gateway; add redacted terminal or live-output evidence for inactive success and selected-provider failure, then update the PR body to trigger a fresh ClawSweeper review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +5, Tests +50. Total +55 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 19 14 +5
Tests 1 50 0 +50
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 69 14 +55

What I checked:

Likely related people:

  • steipete: Merged the partial-Gateway-assignment implementation in fix(secrets): preserve partial gateway assignments #105160, including the targeted fallback branch this PR adjusts. (role: introduced behavior; confidence: high; commits: 00cab8d42b8b; files: src/cli/command-secret-gateway.ts, src/cli/command-secret-gateway.test.ts)
  • joshavant: Earlier scoped command-time SecretRef resolution and hardened the same CLI resolver/test surface, making them a strong routing candidate for the underlying contract. (github.com) (role: feature owner; confidence: medium; commits: 1501008; files: src/cli/command-secret-gateway.ts, src/cli/command-secret-gateway.test.ts, src/secrets/command-config.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-15T22:55:58.910Z sha 631222d :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. label Jul 16, 2026
@steipete steipete self-assigned this Jul 16, 2026
@steipete
steipete force-pushed the fix/partial-web-secret-fallback branch from 631222d to 4d80ae3 Compare July 16, 2026 09:52
@steipete
steipete requested a review from a team as a code owner July 16, 2026 09:52
@steipete
steipete force-pushed the fix/partial-web-secret-fallback branch 2 times, most recently from 4a28f2b to 4c58e2c Compare July 16, 2026 09:55
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M and removed size: S labels Jul 16, 2026
@steipete
steipete force-pushed the fix/partial-web-secret-fallback branch 2 times, most recently from 861a518 to cd9e1ee Compare July 16, 2026 10:52
@steipete
steipete force-pushed the fix/partial-web-secret-fallback branch from cd9e1ee to b81cdc7 Compare July 16, 2026 11:00
@steipete

steipete commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Maintainer proof complete on exact head b81cdc77f7b69ccd95e70e62ec3c57c221086ef7.

Best-fix judgment: ready. This keeps authoritative Gateway assignments, accepts the existing terminal inactive state, and preserves strict failure for active or unproven ownership without adding a fallback stack.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@steipete
steipete merged commit 92075dd into openclaw:main Jul 16, 2026
116 checks passed
@steipete

steipete commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

This was referenced Jul 16, 2026
Closed
Merged
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 17, 2026
@VACInc @steipete
fix(secrets): inactive web refs no longer block agent turns (openclaw…
82d52b1 
…#108485)

* fix(secrets): accept inactive refs from partial fallback

* fix(secrets): fail closed on unknown web provider owners

* test(gateway): isolate web secret startup fixtures

---------

Co-authored-by: VACInc <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

cli CLI command changes gateway Gateway runtime merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VACInc @steipete