Skip to content

fix(core): converge model-API ownership, provider auth-literal parity, and structured failover classification#104886

Merged
obviyus merged 6 commits into
mainfrom
core/hardening-batch
Jul 12, 2026
Merged

fix(core): converge model-API ownership, provider auth-literal parity, and structured failover classification#104886
obviyus merged 6 commits into
mainfrom
core/hardening-batch

Conversation

@obviyus

@obviyus obviyus commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Row 12 of #104219: the three concrete drift/misclassification exposures that survived re-verification of the tracker's remaining seams.

  1. Duplicated built-in model-API set. CORE_BUILT_IN_MODEL_APIS was defined byte-for-byte in both src/plugins/provider-config-owner.ts and src/plugins/gateway-startup-plugin-ids.ts. Both decide "is this model API plugin-owned"; nothing enforced sync, so adding a core API to one silently diverges startup plugin-id resolution from provider-config ownership hints.
  2. No manifest↔runtime auth-literal parity check. Every bundled provider plugin declares the same auth literals twice: the manifest (providerAuthChoices/setup.providers — consumed by the wizard/CLI-flag control plane without loading plugin runtime) and the plugin entry's provider.auth (consumed at request time). If optionKey/env var/CLI flag drift, the wizard stores the user's key under one name and runtime reads another — the provider looks configured but auth silently fails. No test compared the two surfaces.
  3. Structured failover reasons ignored by terminal catch classification. In src/auto-reply/reply/agent-runner-execution.ts, the context-overflow and transient-HTTP branches classified by message text only (isLikelyContextOverflowError, isTransientHttpError — the latter requires a leading HTTP status token). A typed FailoverError with reason: "context_overflow" but sanitized message fell through to the generic failure reply; a typed transient failure without a status token skipped the single transient retry. Billing and rate-limit already classified structurally; these two branches were the drift surface.

Why This Change Was Made

  • CORE_BUILT_IN_MODEL_APIS now has one owner (provider-config-owner.ts, exported); gateway-startup-plugin-ids.ts imports it. Import direction verified cycle-free (pnpm check:import-cycles green in check:changed).
  • New src/plugins/bundled-provider-auth-literal-parity.test.ts discovers bundled plugins programmatically (listBundledPluginMetadata), loads each plugin's public entry via the existing loadBundledPluginPublicSurface helper (no extensions/*/src/** deep imports), registers it with createCapturedPluginRegistration, and table-drives every api-key-style providerAuthChoices entry: manifest method ↔ runtime auth.id, cliFlag ↔ probed flagName, optionKey proven by the option value reaching flagValue, env var membership in setup.providers[].envVars/provider.envVars. Capability-only manifests (video/image onboard flags with no text provider) and oauth-style choices are excluded by construction to avoid false positives.
  • The catch classification now prefers structured reasons with message text as fallback, matching the file's existing billing/rate-limit pattern: context_overflow reason → overflow handling; timeout/server_error reasons → the existing single transient retry. Deliberately excluded: overloaded — a shipped-behavior test ("surfaces typed overloaded failures without rate-limit cooldown copy") pins typed overloaded failures to immediate dedicated user copy, and routing them into the silent retry would change that product behavior. The first draft included overloaded; the pinned test caught it and the gate was narrowed (commit c61e59abde9).

User Impact

  • Typed context-overflow failures now always produce the targeted overflow guidance instead of the generic failure reply, even when the provider message lacks overflow phrasing.
  • Typed transient provider failures (timeout/server_error) get the same single retry that status-token-bearing messages already got, so a transient blip mid-conversation recovers instead of surfacing an error.
  • Overloaded, billing, rate-limit, and compaction-failure behavior are unchanged.
  • Items 1–2 are maintainer-facing only (drift guards); no runtime behavior change.

Evidence

Real behavior proof (ephemeral gateway, real Telegram channel path)

Built this branch, ran a real gateway (temp OPENCLAW_STATE_DIR, loopback) with mock Telegram Bot API + mock OpenAI-completions provider, and drove real reply turns:

  • Transport-exhaust recovery (A4): mock returns HTTP 500 for three consecutive requests, success after. Gateway log shows the terminal catch classify + Transient HTTP provider error before reply (HTTP 500: …). Retrying once in 2500ms., inter-request gaps [470, 859, 3789]ms (the 3.8s gap is the agent-runner retry delay), and the user-visible Telegram reply is the recovered text — not an error.
  • Context-overflow guidance (A3): mock returns HTTP 400 context_length_exceeded on every request; after auto-compaction also fails, the user-visible reply is the targeted Context overflow: prompt too large for the model… copy, not the generic failure text.
  • Overloaded pin unchanged: the shipped test "surfaces typed overloaded failures without rate-limit cooldown copy" stays green (see review-loop note below).

Honest scope of the live delta: an A/B run with only agent-runner-execution.ts rolled back to main recovers identically on these paths, because the runtime currently formats provider failures with a leading status token (HTTP 500: …), which main's message-only gate already matches; held-socket timeouts route through the embedded idle-watchdog (llm-idle-timeout → surface) and never reach this catch. The structured gate therefore changes no reachable live path today — it pins the retry/overflow decision to the typed FailoverError.reason (the canonical fact) instead of the message formatter's prefix, so a formatter change can't silently alter retry behavior. The unit-level delta (typed reason + token-less message → retry/overflow copy) is covered by the new regression tests; the live runs prove behavior parity and no regression.

Review-loop corrections (found during validation)

  • First draft routed typed overloaded failures into the transient retry; the shipped-behavior test caught it (overloaded failures must surface dedicated copy immediately). Gate narrowed to timeout/server_error in c61e59abde9.
  • The parity test initially landed in the unit-fast CI lane, where loading built plugin dists poisons the shared worker module cache and broke unrelated vi.mock-based tests (memory-host-sdk embeddings in checks-node-compact-large-2; bisected to a single surface load). Fixed in 320aab1a37b by making the loader import dynamic, which moves the file to the plugins lane where dist loading is the norm.
  • ClawSweeper finding accepted: the parity test's silent skip on missing runtime registration recreated the drift class it guards; now a manifest-declared provider that never registers fails the test unless the plugin registers zero text providers (3eed7c70cad).

Static evidence

  • Duplication site: git show of gateway-startup-plugin-ids.ts before this change vs provider-config-owner.ts:5 — identical 8-element sets; now one definition (−9 prod LOC).
  • Parity test currently passes for all bundled providers (no live drift today — this is a regression guard for the silent-auth-failure mode).
  • New regression tests in agent-runner-execution.test.ts: FailoverError server_error/timeout without a leading status token → exactly one transient retry then success; structured context_overflow with non-overflow message → overflow copy, generic reply suppressed. The pre-existing overloaded pin stays green.
  • Remote (Blacksmith Testbox, lease tbx_01kx9yh69g4043rppaj23e49tv): pnpm check:changed exit 0; focused suites green on head c61e59abde9 — bundled-provider-auth-literal-parity + bundled-plugin-metadata (36 files' worth of parity cases) and full agent-runner-execution.test.ts (253 tests).
  • git diff --numstat vs base: prod +11/−13 (net −2) across provider-config-owner.ts, gateway-startup-plugin-ids.ts, agent-runner-execution.ts; tests +266.

Part of #104219 (row 12: small hardening batch from the 2026-07-12 re-verification).

obviyus added 3 commits July 12, 2026 07:25
@obviyus
refactor(plugins): single-source CORE_BUILT_IN_MODEL_APIS and add pro…
f33eeec 
…vider auth-literal parity test

gateway-startup-plugin-ids duplicated the built-in model-API set byte-for-byte
with nothing enforcing sync; provider-config-owner now owns it. New parity
test proves bundled provider manifests and runtime provider.auth declare the
same optionKey/cliFlag/envVar/method literals, guarding against silent
wizard-vs-runtime auth drift. Part of #104219 (row 12).
@obviyus
fix(auto-reply): honor structured FailoverError reasons in terminal c…
38197fb 
…atch classification

context_overflow and transient (timeout/server_error/overloaded) FailoverError
reasons now classify structurally like billing/rate_limit already do, instead
of relying on message text that may lack overflow or HTTP-status tokens. A
typed transient failure without a leading status token now takes the single
transient retry; typed context overflow suppresses the generic failure reply.
Part of #104219 (row 12).
@obviyus
fix(auto-reply): keep typed overloaded failures on the surfaced-copy …
c61e59a 
…path

Shipped behavior pins overloaded FailoverErrors to immediate dedicated copy
(agent-runner-execution.test.ts 'surfaces typed overloaded failures'); only
timeout and server_error reasons join the transient retry gate.
@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels Jul 12, 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 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 12:08 AM ET / 04:08 UTC.

Summary
The branch single-sources the core built-in model-API set, adds bundled-provider manifest/runtime auth-literal parity coverage, and classifies context-overflow, timeout, and server-error FailoverErrors by structured reason.

PR surface: Source +2, Tests +305. Total +307 across 5 files.

Reproducibility: yes. at source level: current main relies on message-only gates, and focused tests can construct tokenless typed errors that select the wrong terminal behavior. Normal live provider paths do not currently emit the exact shape, so this is not a high-confidence live reproduction.

Review metrics: 2 noteworthy metrics.

  • Canonical ownership copies: 1 removed, 1 retained. The duplicated eight-entry core model-API fact becomes a single shared owner.
  • Structured reason coverage: 3 reasons added. Context overflow, timeout, and server error no longer depend solely on error-message formatting.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/plugins/bundled-provider-auth-literal-parity.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104219
Summary: This PR is the explicit row-12 implementation candidate for the canonical core-hardening tracker; the related session-accessor PR covers a distinct seam.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Obtain the terminal-catch owner's explicit proof override or provide redacted instrumented exact-path diagnostics.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Real gateway and Telegram-path logs demonstrate reachable behavior parity and no regression, but the exact tokenless typed-reason improvement remains test-only; an explicit maintainer proof override is required. Any additional evidence should redact keys, phone numbers, IP addresses, and non-public endpoints, then update the PR body to trigger review or request @clawsweeper re-review.

Risk before merge

  • [P1] The exact tokenless typed-reason improvement is demonstrated by focused tests rather than an observed after-fix real runtime delta, so accepting the defensive proof requires explicit owner judgment.
  • [P1] The terminal-catch change can replace an immediate failure reply with a delayed retry or targeted overflow response if a typed reason reaches this path, making its user-visible precedence merge-relevant.

Maintainer options:

  1. Accept the defensive invariant (recommended)
    Land after the terminal-catch owner confirms that typed timeout/server_error retry and context-overflow classification preserve existing billing, auth, overload, control-UI, and lifecycle precedence.
  2. Split before merge
    Remove the structured classifier hunk and retain the independent model-API ownership and auth-parity guards.

Next step before merge

  • [P1] The code appears ready; the remaining maintainer action is to approve or reject the real-behavior proof override for the structured failure-classification subchange.

Maintainer decision needed

  • Question: May the structured-reason hardening merge with focused tokenless FailoverError regression tests and a real gateway no-regression run, despite no naturally reachable live A/B delta today?
  • Rationale: Current provider failures include HTTP status text, and held-socket timeouts terminate before this catch, so exact after-fix live proof would require manufacturing an error shape not currently emitted by the product.
  • Likely owner: steipete — This person introduced the current terminal catch and its retry and user-facing failure semantics.
  • Options:
    • Approve proof override (recommended): Accept the focused regression tests and real gateway no-regression evidence because the defensive typed-reason path is source-valid and the patch preserves existing higher-priority branches.
    • Require instrumented proof: Hold the runtime classifier change until a redacted instrumented run demonstrates a tokenless typed timeout or context-overflow error reaching this catch.
    • Split the runtime change: Land the ownership and auth-parity guards while deferring structured failure classification to a separately proven change.

Security
Cleared: The patch introduces no dependency, workflow, permission, secret, package-resolution, publishing, lifecycle-hook, or downloaded-code changes.

Review details

Best possible solution:

Preserve the single canonical model-API owner and fail-closed auth parity ratchet, and land the structured-reason classification after the terminal-catch owner explicitly accepts focused tests plus real no-regression logs as sufficient proof for a path current runtime formatting does not naturally emit.

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

Yes at source level: current main relies on message-only gates, and focused tests can construct tokenless typed errors that select the wrong terminal behavior. Normal live provider paths do not currently emit the exact shape, so this is not a high-confidence live reproduction.

Is this the best way to solve the issue?

Yes, subject to proof acceptance. Single-sourcing the ownership fact and using the closed FailoverError reason contract are narrower and more maintainable than synchronized lists or formatter-dependent policy, and the contract test now fails closed for included API-key methods.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 message-delivery: The terminal catch can change whether a provider failure retries, returns targeted overflow guidance, or immediately surfaces failure copy.
  • remove merge-risk: 🚨 automation: Current PR review merge-risk labels are merge-risk: 🚨 message-delivery.

Label justifications:

  • P2: This is bounded core/provider correctness hardening with limited immediate user blast radius.
  • merge-risk: 🚨 message-delivery: The terminal catch can change whether a provider failure retries, returns targeted overflow guidance, or immediately surfaces failure copy.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Real gateway and Telegram-path logs demonstrate reachable behavior parity and no regression, but the exact tokenless typed-reason improvement remains test-only; an explicit maintainer proof override is required. Any additional evidence should redact keys, phone numbers, IP addresses, and non-public endpoints, then update the PR body to trigger review or request @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +2, Tests +305. Total +307 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 15 13 +2
Tests 2 305 0 +305
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 320 13 +307

What I checked:

  • Canonical model-API ownership: The branch exports the eight-entry core model-API set from one owner and imports it into gateway startup, removing the identical local copy. (src/plugins/provider-config-owner.ts:6, 6a78697901f7)
  • Fail-closed parity coverage: Each included API-key choice must register the declared text provider and method, reach resolveApiKey non-interactively, and match its option-key, CLI-flag, and environment literals; unprobeable methods now fail. (src/plugins/bundled-provider-auth-literal-parity.test.ts:181, 6a78697901f7)
  • Structured failure classification: The terminal catch now recognizes typed context_overflow independently of message text and retries typed timeout/server_error once, while overloaded remains on its existing immediate surfaced-copy path. (src/auto-reply/reply/agent-runner-execution.ts:3067, 6a78697901f7)
  • Focused regression coverage: The branch tests tokenless typed timeout/server_error recovery and tokenless typed context-overflow guidance, while the existing overloaded behavior remains separately pinned. (src/auto-reply/reply/agent-runner-execution.test.ts:7860, 6a78697901f7)
  • Real behavior proof limitation: The contributor's gateway and Telegram-path run demonstrates reachable behavior parity and no regression, but the documented A/B behaves identically on main because current provider errors include HTTP status text and held-socket timeouts terminate earlier. (6a78697901f7)
  • Current-main drift check: Current main has only an unrelated assertion change in the touched runner file since the PR base; the reviewed terminal catch has not been superseded or materially changed. (src/auto-reply/reply/agent-runner-execution.ts:565, ec11440176bb)

Likely related people:

  • steipete: Git blame and log attribute the current terminal catch, message-dependent classification, provider-config owner, and duplicated gateway model-API set to commit 2829fe1. (role: introduced current behavior; confidence: high; commits: 2829fe107c67; files: src/auto-reply/reply/agent-runner-execution.ts, src/plugins/provider-config-owner.ts, src/plugins/gateway-startup-plugin-ids.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 (5 earlier review cycles)
  • reviewed 2026-07-12T02:22:39.175Z sha c61e59a :: needs real behavior proof before merge. :: [P2] Make missing text-provider registrations fail parity
  • reviewed 2026-07-12T02:44:16.436Z sha 3eed7c7 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-12T03:04:52.810Z sha 320aab1 :: needs real behavior proof before merge. :: [P1] Move the parity test off the forbidden core import path
  • reviewed 2026-07-12T03:13:21.752Z sha 320aab1 :: needs real behavior proof before merge. :: [P1] Move the parity test off the forbidden core import path | [P2] Fail when an API-key choice cannot be probed
  • reviewed 2026-07-12T03:21:00.655Z sha 320aab1 :: needs real behavior proof before merge. :: [P2] Fail when an API-key auth method cannot be probed

obviyus added 2 commits July 12, 2026 08:02
@obviyus
test(plugins): fail auth-literal parity when a declared provider neve…
3eed7c7 
…r registers

Silent skip on missing runtime registration recreated the drift class the
test guards (ClawSweeper finding); capability-only plugins now must register
zero text providers to be exempt.
@obviyus
test(plugins): keep auth-literal parity test out of the unit-fast lane
320aab1 
Loading built plugin dists pulls large module graphs into the shared vitest
worker cache and broke co-resident vi.mock unit tests (memory-host-sdk
embeddings, checks-node-compact-large-2). An explicit dynamic import of the
surface loader trips the lane classifier's dynamic-import rule, moving the
file to the plugins project where dist loading is the norm.
@obviyus

obviyus commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Review findings addressed, real behavior proof added (PR body Evidence updated), and CI triaged.

Findings addressed

  • Parity silent-skip (P1): a manifest-declared provider that never registers now fails the test; only plugins registering zero text providers are exempt (3eed7c70cad).
  • Live proof (P2): see below and the PR body.
  • Additionally found and fixed during validation: the parity test originally landed in the unit-fast CI lane, where loading built plugin dists poisons the shared worker module cache and broke unrelated vi.mock tests (memory-host-sdk embeddings in checks-node-compact-large-2 — bisected to a single plugin-surface load, reproduced deterministically by co-running the two files). Fixed by moving the file to the plugins lane (320aab1a37b).

Live proof (ephemeral gateway, real Telegram path — details in PR body)

  • Transport-exhaust recovery: 3× HTTP 500 then success → terminal catch logs Transient HTTP provider error before reply (HTTP 500: …). Retrying once in 2500ms., inter-request gaps [470, 859, 3789]ms, user receives the recovered reply.
  • Context overflow: HTTP 400 context_length_exceeded on every request → user receives the targeted Context overflow: … guidance after auto-compaction also fails.
  • Honest scope: an A/B with only agent-runner-execution.ts rolled back to main behaves identically on these paths, because the runtime currently prefixes provider failures with HTTP <status>: which main's message gate already matches, and held-socket timeouts route via the idle watchdog. The structured gate's value is pinning retry/overflow classification to the typed FailoverError.reason instead of the message formatter's prefix; the unit-level delta (typed reason + token-less message) is covered by the new regression tests, and the live runs prove no regression on reachable paths.

CI triage — remaining failures are current main breakage (all reproduced on clean origin/main on a Blacksmith Testbox)

  • checks-node-compact-large-4 / checks-node-compact-small-6 — src/gateway/server-methods/agent.test.ts + src/commands/agent.session.test.ts "terminal main session … transcript is newer" tests fail identically on clean main (4 failures, heads dfa580e9/f5eafc15 era).
  • build-artifacts — broken dts phase on main (predates this PR).
  • check-docs — docs/docs_map.md is out of date. Run pnpm docs:map:gen. on main; this PR touches no docs.
  • Earlier check-additional-runtime-topology-architecture (madge cycle through the src/plugins barrel) and plugin-sdk-surface-report budget pins also reproduce on clean main dfa580e9 (documented with proof on refactor(sessions): route get-reply-run session refresh through the accessor #104880).

This PR's own surface is green: pnpm check:changed exit 0; focused suites (bundled-provider-auth-literal-parity 72 cases, bundled-plugin-metadata, full agent-runner-execution.test.ts 253 tests) pass on the current head. Per repo policy the unrelated main fixes stay separate.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@obviyus obviyus mentioned this pull request Jul 12, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 12, 2026
@obviyus
test(plugins): fail closed when an api-key auth method cannot be probed
6a78697 
An unprobeable api-key choice left its flag/env literals unchecked while CI
stayed green (ClawSweeper finding). The probe now supplies a real agent dir
and placeholder preflight opts (sentinel still bound to the declared
optionKey only, preserving the key-correctness proof), so every api-key
method must reach resolveApiKey — this immediately exercised
cloudflare-ai-gateway's account/gateway preflight path.
@obviyus

obviyus commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Fail-closed finding fixed in 6a78697901f: an api-key-style choice whose method cannot be probed now fails the test instead of silently skipping its flag/env literals. The probe supplies a real agent dir and placeholder preflight opts while keeping the sentinel bound to the declared optionKey only (so flagValue === sentinel still proves the method read the correct key) — this immediately exercised cloudflare-ai-gateway's account/gateway preflight path, which the previous probe silently skipped.

Validation on Blacksmith Testbox at 6a78697901f: pnpm check:changed exit 0; bundled-provider-auth-literal-parity 72/72; agent-runner-execution 253/253.

On the remaining proof P1 (exact tokenless typed-reason live delta): as documented in the PR body, every reachable live path currently formats provider failures with a leading HTTP <status>: token, so a live A/B delta for the structured gate is not producible without contriving an error shape the runtime doesn't emit today — the gate pins classification to the typed reason so formatter changes can't silently alter retry behavior, and the unit regression tests cover the tokenless case. Per this review's own framing, that leaves a maintainer decision between exact changed-path proof and a proof override; deferring to the area owner.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jul 12, 2026
@obviyus

obviyus commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer proof override: accepting the structured failure-classification subchange on unit-test evidence. Live A/B established behavior parity on all reachable paths (no regression); the tokenless typed-reason case is contract hardening pinned by the new regression tests. Landing.

@obviyus
obviyus merged commit 33186b0 into main Jul 12, 2026
111 of 115 checks passed
@obviyus
obviyus deleted the core/hardening-batch branch July 12, 2026 04:36
@clawsweeper clawsweeper Bot mentioned this pull request Jul 12, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

maintainer Maintainer-authored PR merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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.

1 participant

@obviyus