fix(agents): tolerate missing attribution baseUrl by samrusani · Pull Request #92991 · openclaw/openclaw

samrusani · 2026-06-14T14:33:00Z

Summary

Normalize the session attribution helper's runtime baseUrl read so Bedrock-shaped models that omit baseUrl do not crash before provider routing.
Keep the existing OpenRouter and Cloudflare attribution header behavior unchanged.
Add focused session SDK coverage for missing baseUrl, OpenRouter attribution, and Cloudflare attribution.

Root cause

getAttributionHeaders() treated Model.baseUrl as always present and called .includes() on it. Runtime model config can cross this boundary without baseUrl, including Bedrock models, so install telemetry attribution could throw before the provider request path ran.

Real behavior proof

Behavior addressed: A Bedrock model object without baseUrl should not throw Cannot read properties of undefined (reading 'includes') before provider routing.

Real environment tested: Local OpenClaw source checkout on macOS at exact head 4143df76efd08c506d238ef9f9ae06eb7210375f, Node v26.3.0, using a Bedrock-shaped amazon-bedrock model object with no baseUrl; Linux Testbox for the changed gate.

Exact steps or command run after this patch: Ran a local Node source harness with node --import tsx that imported createAgentSession, created a real session with the Bedrock-shaped model, and called session.agent.streamFn(...) with an already-aborted signal to avoid any live AWS request.

Evidence after fix: Copied live terminal output from the source harness:

{"reachedProviderPath":true,"errorName":"Error","message":"No API provider registered for api: bedrock-converse-stream"}

Observed result after fix: The stream path reached provider routing and failed with the expected local missing-provider setup error. It did not throw the previous baseUrl.includes TypeError.

What was not tested: No live AWS Bedrock request was made because no valid AWS credentials were available. The regression occurs before the provider network call; the real session harness and focused tests cover that boundary.

Validation

node scripts/run-vitest.mjs src/agents/sessions/sdk.test.ts src/agents/provider-attribution.test.ts
- Passed: 1 Vitest shard, 2 files, 47 tests.
CI=true pnpm exec oxfmt --check --threads=1 src/agents/sessions/sdk.ts src/agents/sessions/sdk.test.ts
- Passed.
git diff --check
- Passed.
Testbox changed gate tbx_01kv3peewwaj45fgwye7bmbsyz
- Passed core and coreTests lanes, including production/test typechecks, lint, and import-cycle checks.
Fresh autoreview after the test-type fix
- Clean: no accepted/actionable findings.

AI-assisted disclosure

AI-assisted PR prepared with Codex. I reviewed the changed code path, kept the change scoped to #92974, and ran the validation listed above.

clawsweeper · 2026-06-14T14:35:24Z

Codex review: needs maintainer review before merge. Reviewed June 14, 2026, 2:41 PM ET / 18:41 UTC.

Summary
The PR normalizes the agent session attribution helper's baseUrl read and adds session SDK tests for missing baseUrl, OpenRouter attribution, and Cloudflare attribution behavior.

PR surface: Source +2, Tests +91. Total +93 across 2 files.

Reproducibility: yes. Current main dereferences model.baseUrl.includes(...) before streaming, while the config/runtime boundary can produce Bedrock-shaped models without baseUrl; I did not run a live Bedrock request in this read-only review.

Review metrics: 1 noteworthy metric.

Open duplicate fix branches: 4 sibling PRs open. Maintainers should pick one canonical landing path for the Bedrock baseUrl null-guard and close the other branches afterward.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/sdk.test.ts, serialized state: src/agents/sessions/sdk.ts, unknown-data-model-change: src/agents/sessions/sdk.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

[P2] Wait for the remaining exact-head CI check to complete before merge.

Risk before merge

[P1] One exact-head CI job was still in progress when queried; wait for the normal required check set before merge.

Maintainer options:

Decide the mitigation before merge
Land this narrow null-guard plus regression coverage if the remaining required checks pass, then close the linked Bedrock crash issue and the duplicate fix PRs as superseded by the landed path.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No repair lane is needed; this branch already contains the narrow code fix, tests, and proof, leaving normal maintainer review and check gating.

Security
Cleared: The diff only changes TypeScript runtime/test code and does not touch dependencies, workflows, secrets, package scripts, or downloaded code paths.

Review details

Best possible solution:

Land this narrow null-guard plus regression coverage if the remaining required checks pass, then close the linked Bedrock crash issue and the duplicate fix PRs as superseded by the landed path.

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

Yes. Current main dereferences model.baseUrl.includes(...) before streaming, while the config/runtime boundary can produce Bedrock-shaped models without baseUrl; I did not run a live Bedrock request in this read-only review.

Is this the best way to solve the issue?

Yes. Normalizing the optional runtime value inside the attribution helper is the narrow crash-site fix and avoids a broader attribution refactor while preserving existing OpenRouter and Cloudflare behavior.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies copied live harness output showing a Bedrock-shaped request reaches provider routing instead of throwing the previous missing-baseUrl TypeError.

Label justifications:

P1: This PR fixes a shipped regression that can break every Bedrock-backed agent request before provider routing.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies copied live harness output showing a Bedrock-shaped request reaches provider routing instead of throwing the previous missing-baseUrl TypeError.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies copied live harness output showing a Bedrock-shaped request reaches provider routing instead of throwing the previous missing-baseUrl TypeError.

Evidence reviewed

PR surface:

Source +2, Tests +91. Total +93 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	3	+2
Tests	1	92	1	+91
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	97	4	+93

What I checked:

Current main crash site: Current main calls model.baseUrl.includes(...) inside getAttributionHeaders() after the telemetry gate, so a runtime model object without baseUrl can throw before provider routing. (src/agents/sessions/sdk.ts:181, 07dfdd4bd038)
Caller path reaches attribution before streaming: createAgentSession() computes attribution headers at the session stream boundary before delegating to streamSimple(), matching the reported pre-provider failure boundary. (src/agents/sessions/sdk.ts:418, 07dfdd4bd038)
Runtime/config model shape can omit baseUrl: The shared Model type currently requires baseUrl, but the loaded models config schema accepts optional model/provider baseUrl, which explains why this runtime boundary can receive a Bedrock-shaped object without the field. (src/agents/sessions/model-registry.ts:158, 07dfdd4bd038)
PR patch fixes the crash site: The PR normalizes the runtime baseUrl read to an empty string before checking OpenRouter and Cloudflare endpoint substrings. (src/agents/sessions/sdk.ts:178, 4143df76efd0)
PR tests cover the regression and preserved behavior: The PR adds session SDK coverage for a Bedrock-shaped model without baseUrl and for existing OpenRouter and Cloudflare attribution headers. (src/agents/sessions/sdk.test.ts:87, 4143df76efd0)
Latest release is affected: The v2026.6.6 release tag contains the same unguarded model.baseUrl.includes(...) calls in getAttributionHeaders(). (src/agents/sessions/sdk.ts:174, 8c802aa68351)

Likely related people:

vincentkoc: Merged provider-attribution registry metadata and v2026.6.6 release-tag blame both connect this account to the attribution policy and release source containing the unguarded helper. (role: provider-attribution introducer and release-line owner; confidence: high; commits: 5572e6965a76, 8c802aa68351; files: src/agents/provider-attribution.ts, src/agents/sessions/sdk.ts)
wirjo: GitHub PR history shows multiple merged Amazon Bedrock provider/auth/discovery changes, making this account relevant for Bedrock model-shape context even though the crash is in session attribution code. (role: Bedrock provider contributor; confidence: medium; commits: 019a25e35caa, 78fe96f2d4c7, 2a15a3bb53f9; files: extensions/amazon-bedrock/register.sync.runtime.ts, extensions/amazon-bedrock/discovery.ts)
Stellar鱼: Local current-main blame attributes the current getAttributionHeaders() helper block to commit 4644e0c, though that broad commit is not attribution-specific. (role: current-main line carrier; confidence: low; commits: 4644e0c10256; files: src/agents/sessions/sdk.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.

steipete · 2026-06-14T18:43:04Z

Land-ready proof for 4143df76efd08c506d238ef9f9ae06eb7210375f:

Focused regression: node scripts/run-vitest.mjs src/agents/sessions/sdk.test.ts src/agents/provider-attribution.test.ts — 2 files, 47 tests passed.
Production session path: a Bedrock model with runtime-missing baseUrl reaches provider routing instead of throwing during attribution detection (reachedProviderPath: true; expected terminal error: no registered local Bedrock API provider in the probe).
Testbox changed gate: tbx_01kv3peewwaj45fgwye7bmbsyz — passed.
Fresh autoreview: clean, no actionable findings.
Exact-head GitHub CI, CodeQL, and OpenGrep: green; merge state clean.

Known proof gap: no live AWS Bedrock request was run because valid AWS credentials were unavailable. The reported crash occurs before network dispatch, and the real session-path regression proves that boundary directly.

Avoid assuming every runtime model exposes a string `baseUrl` before provider attribution checks. Preserve OpenRouter and Cloudflare attribution behavior while allowing Bedrock session setup to reach provider routing. Fixes openclaw#92974. Co-authored-by: Sami Rusani <sr@samirusani>

openclaw-barnacle Bot added agents Agent runtime and tooling size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 14, 2026

Sami Rusani and others added 2 commits June 14, 2026 19:26

fix(agents): tolerate missing attribution baseUrl

3b521f1

test(agents): type attribution stream mock

4143df7

steipete force-pushed the fix-bedrock-attribution-baseurl branch from 3d5ecdc to 4143df7 Compare June 14, 2026 18:34

steipete marked this pull request as ready for review June 14, 2026 18:34

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 14, 2026

steipete merged commit a189baa into openclaw:main Jun 14, 2026
205 of 247 checks passed

github-actions Bot mentioned this pull request Jun 14, 2026

📡 Upstream Digest — 2026-06-14 19:12 UTC curtismercier/openclaw-mods#1079

Open

clawsweeper Bot mentioned this pull request Jun 21, 2026

fix(agents): null-guard baseUrl in getAttributionHeaders (fixes #92974) #95584

Closed

clawsweeper Bot mentioned this pull request Jul 3, 2026

improve(agents): skip whole-history deep clone on every turn when no context hook is registered #99542

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): tolerate missing attribution baseUrl#92991

fix(agents): tolerate missing attribution baseUrl#92991
steipete merged 2 commits into
openclaw:mainfrom
samrusani:fix-bedrock-attribution-baseurl

samrusani commented Jun 14, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jun 14, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

samrusani commented Jun 14, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Real behavior proof

Validation

AI-assisted disclosure

Uh oh!

clawsweeper Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jun 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

samrusani commented Jun 14, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jun 14, 2026 •

edited

Loading