Skip to content

feat: promote provider tool-call stream wrapper#86409

Closed
steipete wants to merge 1 commit into
openclaw:mainfrom
steipete:codex/provider-stream-public
Closed

feat: promote provider tool-call stream wrapper#86409
steipete wants to merge 1 commit into
openclaw:mainfrom
steipete:codex/provider-stream-public

Conversation

@steipete

@steipete steipete commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • promote createPlainTextToolCallPromotionWrapper onto the public provider stream SDK surface
  • migrate bundled xAI and LM Studio providers off the private local-only SDK subpath
  • leave the old internal subpath as a deprecated re-export and document the public helper
  • add focused tests for promotion and non-tool bracketed streaming pass-through

Verification

  • node scripts/run-vitest.mjs src/plugin-sdk/provider-stream-shared.test.ts src/plugin-sdk/provider-stream.test.ts extensions/xai/stream.test.ts extensions/lmstudio/src/stream.test.ts
  • pnpm plugin-sdk:api:check
  • env -u OPENCLAW_TESTBOX pnpm check:changed
  • git diff --check
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Notes

  • Autoreview initially flagged broad buffering for bracketed non-tool text; the patch now preserves the tool-name-aware buffering predicate and covers that with a regression test.
  • Direct pushes to openclaw/openclaw failed with GitHub remote 500 / commit_refs errors, so this PR is opened from the steipete/openclaw fork.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: lmstudio extensions: xai size: L maintainer Maintainer-authored PR labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 5:27 AM ET / 09:27 UTC.

Summary
The PR promotes createPlainTextToolCallPromotionWrapper to public provider stream SDK exports, migrates xAI and LM Studio imports, leaves the old private subpath as a deprecated re-export, and updates docs and focused tests.

PR surface: Source +174, Tests +93, Docs +1. Total +268 across 9 files.

Reproducibility: not applicable. This PR promotes an SDK helper rather than reporting a reproducible bug. Source inspection shows current main keeps the helper private while the branch exposes and tests it publicly.

Review metrics: 2 noteworthy metrics.

  • SDK Export Surface: 2 public subpaths expose 1 helper; 1 private subpath retained as deprecated re-export. This is a public Plugin SDK contract change, so maintainers should notice the compatibility surface before merge.
  • Merge Conflict Surface: 1 changed-in-both file. Current main and the branch both changed the same internal stream wrapper file, so a clean rebase is needed before final review.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
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:

  • Rebase onto current main and preserve 912fdfbedd ordinary-prose streaming behavior.
  • Add redacted real behavior proof for the public import or one affected provider path.
  • Get maintainer approval that this helper should become public Plugin SDK contract.

Proof guidance:
Needs real behavior proof before merge: The PR body lists tests and checks only; it needs redacted terminal/live output, provider logs, or copied runtime output showing after-fix public import or xAI/LM Studio behavior before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • Public Plugin SDK expansion means external plugins may start depending on this helper, so maintainers need to approve the long-term contract and deprecation path before merge.
  • Current main changed src/plugin-sdk/provider-stream-runtime-internal.ts after the PR base; the conflict resolution must preserve the latest ordinary-prose streaming fix.
  • The PR body provides tests/checks only, so the external-PR real behavior proof gate remains open.

Maintainer options:

  1. Rebase And Prove The SDK Seam (recommended)
    Rebase over current main, keep the latest ordinary-prose streaming behavior, and add redacted terminal/live output showing the public import and one affected provider path after the patch.
  2. Approve The Public Contract
    Maintainers can explicitly accept the new public helper and deprecated internal re-export once API baseline, docs, and compatibility expectations are satisfied.
  3. Keep The Helper Private
    If this helper is not intended as public Plugin SDK contract yet, pause or close this PR and leave bundled providers on the private local-only subpath.

Next step before merge
Needs maintainer handling because it changes public Plugin SDK surface, has a protected maintainer label, lacks real behavior proof, and conflicts with latest main.

Security
Cleared: No concrete security or supply-chain regression was found; the diff moves existing stream-wrapper logic, docs, tests, and provider imports without new dependencies, workflows, lockfiles, scripts, or secret handling.

Review details

Best possible solution:

If maintainers want this SDK seam, rebase onto current main, preserve the latest stream-promptness fix, keep the deprecated private re-export, and add redacted real xAI, LM Studio, or public-import proof before merge.

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

Not applicable: this PR promotes an SDK helper rather than reporting a reproducible bug. Source inspection shows current main keeps the helper private while the branch exposes and tests it publicly.

Is this the best way to solve the issue?

Unclear until maintainer review: moving generic provider stream behavior into the public SDK matches the scoped boundary guidance if this is intended as contract. The branch still needs conflict resolution, real behavior proof, and explicit public API acceptance before it is the best merge path.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against a1fe86a0ff0d.

Label changes

Label justifications:

  • P2: This is a normal-priority public SDK/provider improvement with limited but real compatibility impact.
  • merge-risk: 🚨 compatibility: The PR promotes a previously private helper into public Plugin SDK surface and changes bundled provider imports, creating a new contract external plugins may rely on.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists tests and checks only; it needs redacted terminal/live output, provider logs, or copied runtime output showing after-fix public import or xAI/LM Studio behavior before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +174, Tests +93, Docs +1. Total +268 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 5 422 248 +174
Tests 2 93 0 +93
Docs 2 3 2 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 518 250 +268

What I checked:

Likely related people:

  • Peter Steinberger: Authored the current-main refactor that deliberately kept the helper private before this public-SDK promotion. (role: recent area contributor; confidence: high; commits: c3ab2def0a93; files: src/plugin-sdk/provider-stream-runtime-internal.ts, src/plugin-sdk/provider-stream-shared.ts, extensions/xai/stream.ts)
  • Jason (Json): Introduced the plain-text tool-call leak fix and original shared helper behavior now being promoted. (role: introduced behavior; confidence: high; commits: cd627803a094; files: src/plugin-sdk/provider-stream-shared.ts, extensions/xai/stream.ts, extensions/lmstudio/src/stream.ts)
  • Vincent Koc: Authored the current-main streaming promptness fix in the same internal wrapper file that now conflicts with the PR branch. (role: recent adjacent contributor; confidence: high; commits: 912fdfbedd5a; files: src/plugin-sdk/provider-stream-runtime-internal.ts, src/plugin-sdk/provider-stream-runtime-internal.test.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@steipete steipete self-assigned this May 25, 2026
@steipete steipete closed this by deleting the head repository May 25, 2026
@steipete steipete mentioned this pull request May 25, 2026
Merged
@steipete

steipete commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Landed equivalent rebased change in #86489 because this PR's recorded head is orphaned: GitHub reports head.repo: null for steipete:codex/provider-stream-public, and steipete/openclaw does not resolve via gh repo view, HTTPS, or SSH.

Proof on the landed replacement:

Merged commit: 5d01803

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

docs Improvements or additions to documentation extensions: lmstudio extensions: xai maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: L 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

@steipete