fix(agents): handle blank numeric params and LSP exits by morluto · Pull Request #100450 · openclaw/openclaw

morluto · 2026-07-05T19:51:07Z

What Problem This Solves

Fixes an issue where model-generated tool calls could fail when optional numeric fields were emitted as empty-string defaults instead of being omitted.

This affected finite-number inputs such as media quality, FPS, temperature, confidence, scoring thresholds, and file-size limits. It also fixes a bundled LSP runtime path where requests could wait for the full timeout if a language server started successfully and then exited.

Why This Change Was Made

This follows up on #100399, which fixed the same blank optional parameter behavior for positive and non-negative integer readers and added buffered LSP spawn-failure handling. The remaining sibling gaps were finite-number parameters and LSP processes that start successfully but then exit.

The finite-number reader now treats blank and whitespace-only strings as unset, while still rejecting nonblank invalid values. The bundled LSP runtime now routes process exits through the same session failure path as process errors, so pending and future requests reject immediately.

User Impact

Optional numeric tool fields can be left blank by model-generated tool calls without causing avoidable validation failures.

LSP-backed tools fail faster and more clearly when their server process exits unexpectedly, instead of waiting for the per-request timeout.

Evidence

Real behavior proof with a scratch Node script exercising production createImageTool and createBundleLspToolRuntime outside Vitest. The script used a repo-relative media input plus a temporary bundle .lsp.json whose stdio LSP child exits during hover:

REAL_BEHAVIOR_PROOF pr=100450 branch=codex/blank-numeric-lsp-exits
proofRoot=<temp proof dir redacted>
image blank optional finite-number maxBytesMb: errorAfterMs=1539 message="Unsupported media type: document"
image invalid nonblank finite-number maxBytesMb: errorAfterMs=1 message="maxBytesMb must be greater than 0"
lsp sessions=1 tools=lsp_hover_proof
lsp hover after child exit: errorAfterMs=8 message="LSP server \"proof\" exited (7)"
lsp hover retry after child exit: errorAfterMs=0 message="LSP server \"proof\" exited (7)"

node scripts/run-vitest.mjs src/agents/tools/common.params.test.ts src/agents/agent-bundle-lsp-runtime.test.ts - passed, 2 files
pnpm exec oxfmt --check --threads=1 src/agents/tools/common.ts src/agents/tools/common.params.test.ts src/agents/agent-bundle-lsp-runtime.ts src/agents/agent-bundle-lsp-runtime.test.ts - passed
git diff --check - passed
Fresh autoreview - clean; no accepted/actionable findings

chatgpt-codex-connector · 2026-07-05T19:51:13Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

clawsweeper · 2026-07-05T19:58:04Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 4:18 PM ET / 20:18 UTC.

Summary
The branch updates finite-number tool parameter parsing to treat blank strings as unset, routes bundled LSP process exits through the session failure path, and adds focused regression tests.

PR surface: Source +14, Tests +40. Total +54 across 4 files.

Reproducibility: yes. from source inspection: current main rejects blank optional finite-number strings and only fails LSP sessions on child-process errors, leaving successful-then-exited children to time out pending requests. I did not run a local repro because this review is read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

none.

Next step before merge

No ClawSweeper repair job is needed because this PR already contains the focused code and test changes; it needs normal maintainer merge handling.

Security
Cleared: The diff only changes in-repo TypeScript runtime/test handling for parameter parsing and LSP process lifecycle; it adds no dependency, workflow, secret, permission, or package-resolution surface.

Review details

Best possible solution:

Land this focused reliability fix after normal maintainer merge gates, keeping the behavior aligned with the sibling integer readers and the existing LSP session failure model.

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

Yes from source inspection: current main rejects blank optional finite-number strings and only fails LSP sessions on child-process errors, leaving successful-then-exited children to time out pending requests. I did not run a local repro because this review is read-only.

Is this the best way to solve the issue?

Yes: extending the existing blank-param helper to finite numbers and sharing one idempotent LSP failure path for error and exit is the narrow maintainable fix; adding a new config or separate timeout path would be unnecessary.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes redacted terminal output from production code paths outside Vitest that directly shows the changed parser and LSP-exit behavior after the fix.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body now includes redacted terminal output from production code paths outside Vitest that directly shows the changed parser and LSP-exit behavior after the fix.
remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: The PR fixes focused agent tool parsing and bundled LSP reliability issues with limited blast radius.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body now includes redacted terminal output from production code paths outside Vitest that directly shows the changed parser and LSP-exit behavior after the fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes redacted terminal output from production code paths outside Vitest that directly shows the changed parser and LSP-exit behavior after the fix.

Evidence reviewed

PR surface:

Source +14, Tests +40. Total +54 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	21	7	+14
Tests	2	41	1	+40
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	62	8	+54

What I checked:

Current finite-number gap: On current main, readFiniteNumberParam throws whenever strict parsing returns undefined and the raw value is non-null, so blank strings still fail while sibling integer readers already skip blank values. (src/agents/tools/common.ts:303, 426010dfb9a2)
Current LSP exit gap: Current main fails pending LSP requests on error events, but has no exit handler; sendRequest otherwise waits for the 10-second timeout. (src/agents/agent-bundle-lsp-runtime.ts:113, 426010dfb9a2)
PR diff: The PR adds a shared LSP failure helper, wires process exit into it, and adds tests for blank finite-number params plus immediate rejection after child exit. (src/agents/agent-bundle-lsp-runtime.ts:113, 5dd0bd1addee)
Real behavior proof: The updated PR body includes redacted terminal output from a scratch Node script exercising production createImageTool and createBundleLspToolRuntime outside Vitest, showing blank finite-number input no longer fails validation and LSP child exits reject in 8ms/0ms. (5dd0bd1addee)
Related sibling fix provenance: Merged fix: land ten small reliability fixes #100399 introduced the blank integer-param handling and LSP spawn-error failure path that this PR extends to finite numbers and successful-then-exited LSP processes. (src/agents/tools/common.ts:253, b22c36f1125c)
Node child-process contract: Node documents error for spawn/kill/send/abort failures and exit after the child process ends, and warns that both listeners should guard against double handling; the PR's idempotent failure helper matches that contract. (nodejs.org)

Likely related people:

steipete: Authored the merged sibling reliability PR that added blank integer-param handling and LSP spawn-error failure handling in the same files. (role: recent area contributor; confidence: high; commits: b22c36f1125c; files: src/agents/tools/common.ts, src/agents/agent-bundle-lsp-runtime.ts, src/agents/tools/common.params.test.ts)
Gio Della-Libera: Git blame/log show the current shared tool helper and bundled LSP runtime files first entered main in this commit, though the commit title was broader and ownership confidence is lower. (role: introduced current files; confidence: medium; commits: b1f3986b84f7; files: src/agents/tools/common.ts, src/agents/agent-bundle-lsp-runtime.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-05T20:01:03.252Z sha 5dd0bd1 :: needs real behavior proof before merge. :: none

morluto · 2026-07-05T20:09:46Z

@clawsweeper re-review

Added redacted real-behavior proof to the PR body for blank optional finite-number parsing and bundled LSP child-exit fast failure.

clawsweeper · 2026-07-05T20:09:49Z

🦞🧹
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.

Source: #100450 Co-authored-by: morluto <[email protected]>

* fix(agents): harden LSP process failures Source: #100450 Co-authored-by: morluto <[email protected]> * fix(sandbox): report effective workspace layout Sources: #100435, #100439 Co-authored-by: Aniruddha Adak <[email protected]> Co-authored-by: ZengWen-DT <[email protected]> * fix(security): fail install checks on stream errors Source: #100413 Co-authored-by: 陈宪彪0668000387 <[email protected]> * fix(android): normalize all-day calendar events Source: #100032 Co-authored-by: NianJiuZst <[email protected]> * fix(ios): serialize push-to-talk lifecycle Source: #99942 Co-authored-by: NianJiuZst <[email protected]> * fix(talk): reject inherited provider names Source: #99849 Co-authored-by: zenglingbiao <[email protected]> * fix(android): stop voice capture in background Source: #99840 Co-authored-by: xialonglee <[email protected]> * fix(cron): preserve fallback result classification Source: #99913 Co-authored-by: jincheng-xydt <[email protected]> * fix(google): bound Vertex response decompression Source: #99812 Co-authored-by: 黄剑雄0668001315 <[email protected]> * fix(plugins): report malformed discovery JSON Source: #99892 Co-authored-by: 陈宪彪0668000387 <[email protected]> * test(sandbox): configure non-default workspace fixture * test: fix small-fix batch validation --------- Co-authored-by: morluto <[email protected]> Co-authored-by: ZengWen-DT <[email protected]> Co-authored-by: 陈宪彪0668000387 <[email protected]> Co-authored-by: NianJiuZst <[email protected]> Co-authored-by: zenglingbiao <[email protected]> Co-authored-by: xialonglee <[email protected]> Co-authored-by: jincheng-xydt <[email protected]> Co-authored-by: 黄剑雄0668001315 <[email protected]>

steipete · 2026-07-05T23:10:18Z

Superseded by #100483, landed as aaf5ab9.

The landed rewrite treats blank optional numeric inputs as absent and makes LSP requests fail deterministically across spawn errors, child exit, and stdout/stderr stream failures, with pending-request cleanup and focused lifecycle coverage. Exact-head proof passed 195 focused tests, changed-surface type/lint/guards, and hosted CI. Contributor credit from this PR is preserved in the landed commit.

Thank you @morluto for the original fixes.

steipete · 2026-07-05T23:10:24Z

Thanks @morluto — your report and patch were incorporated into the strengthened canonical fix in #100483, landed as aaf5ab9. The landed version also covers blank finite-number parameters, process/stdout/stdin failures, and the exit-vs-close ordering needed to preserve a final LSP response. Contributor credit is preserved. Closing this superseded PR.

* fix(agents): harden LSP process failures Source: openclaw#100450 Co-authored-by: morluto <[email protected]> * fix(sandbox): report effective workspace layout Sources: openclaw#100435, openclaw#100439 Co-authored-by: Aniruddha Adak <[email protected]> Co-authored-by: ZengWen-DT <[email protected]> * fix(security): fail install checks on stream errors Source: openclaw#100413 Co-authored-by: 陈宪彪0668000387 <[email protected]> * fix(android): normalize all-day calendar events Source: openclaw#100032 Co-authored-by: NianJiuZst <[email protected]> * fix(ios): serialize push-to-talk lifecycle Source: openclaw#99942 Co-authored-by: NianJiuZst <[email protected]> * fix(talk): reject inherited provider names Source: openclaw#99849 Co-authored-by: zenglingbiao <[email protected]> * fix(android): stop voice capture in background Source: openclaw#99840 Co-authored-by: xialonglee <[email protected]> * fix(cron): preserve fallback result classification Source: openclaw#99913 Co-authored-by: jincheng-xydt <[email protected]> * fix(google): bound Vertex response decompression Source: openclaw#99812 Co-authored-by: 黄剑雄0668001315 <[email protected]> * fix(plugins): report malformed discovery JSON Source: openclaw#99892 Co-authored-by: 陈宪彪0668000387 <[email protected]> * test(sandbox): configure non-default workspace fixture * test: fix small-fix batch validation --------- Co-authored-by: morluto <[email protected]> Co-authored-by: ZengWen-DT <[email protected]> Co-authored-by: 陈宪彪0668000387 <[email protected]> Co-authored-by: NianJiuZst <[email protected]> Co-authored-by: zenglingbiao <[email protected]> Co-authored-by: xialonglee <[email protected]> Co-authored-by: jincheng-xydt <[email protected]> Co-authored-by: 黄剑雄0668001315 <[email protected]>

fix agents numeric params and lsp exits

5dd0bd1

openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jul 5, 2026

steipete self-assigned this Jul 5, 2026

steipete mentioned this pull request Jul 5, 2026

fix: land ten small reliability fixes #100483

Merged

steipete added a commit that referenced this pull request Jul 5, 2026

fix(agents): harden LSP process failures

1ef5114

Source: #100450 Co-authored-by: morluto <[email protected]>

steipete closed this Jul 5, 2026

clawsweeper Bot mentioned this pull request Jul 11, 2026

fix(agents): add size limit to LSP message parser #103046

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): handle blank numeric params and LSP exits#100450

fix(agents): handle blank numeric params and LSP exits#100450
morluto wants to merge 1 commit into
openclaw:mainfrom
morluto:codex/blank-numeric-lsp-exits

morluto commented Jul 5, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

morluto commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

morluto commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

chatgpt-codex-connector Bot commented Jul 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

morluto commented Jul 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

morluto commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading