Skip to content

fix: harden small runtime and installer edge cases#100258

Merged
steipete merged 11 commits into
mainfrom
codex/land-small-fixes-batch-3
Jul 5, 2026
Merged

fix: harden small runtime and installer edge cases#100258
steipete merged 11 commits into
mainfrom
codex/land-small-fixes-batch-3

Conversation

@steipete

@steipete steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Related: #100181, #100116, #100007, #99974, #99860, #100192, #100187, #100102, #99887, #100098

What Problem This Solves

Fixes ten small correctness gaps where users could see corrupted transcript echoes, dropped MCP event failures, missed custom silent tokens, malformed Discord previews, unbounded diagnostic responses, incomplete session status, wrong Linux package-manager routing, an incorrect TLS deep-link port, or fatal handling of a known transient Undici failure.

Why This Change Was Made

The source PRs were each reviewed against current main, then rebuilt as focused maintainer commits because the rewritten repository history made safe fork-branch refreshes impractical. The batch keeps each fix bounded, preserves contributor co-authorship, adds or retains focused regression coverage, and includes one lint-driven simplification for Unicode-safe custom-token handling.

The first exact-head CI run also exposed an unrelated deterministic assertion omission already present on main: the new gateway MCP source-checkout consumer was routed but absent from test-projects' expected list. This branch includes the one-line expectation repair so current-main CI is green again.

Source PRs and contributor credit:

User Impact

These edge cases now fail safely or produce the expected output, without broadening compatibility surfaces or changing unrelated behavior. Linux installers no longer confuse Debian's pacman game with Arch Linux, secure gateway links work without an explicit port, and network/diagnostic paths remain bounded.

Evidence

  • 282 focused Vitest assertions passed across the 12 touched test files.
  • 19 DeepLinksSecurityTests Swift tests passed.
  • 83 src/scripts/test-projects.test.ts assertions passed after repairing the current-main routing expectation.
  • bash -n scripts/install.sh
  • git diff --check
  • pnpm exec oxfmt --check on all changed TypeScript files
  • Blacksmith Testbox check:changed: passed all selected typecheck, lint, guard, cycle, and tooling-test lanes (tbx_01kwrssmpd8f5wek4ec5ce2mxe, run 28736283867).
  • Fresh branch autoreview: clean, no accepted or actionable findings (patch is correct, confidence 0.94).

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: matrix Channel integration: matrix scripts Repository scripts extensions: clawrouter Extension: clawrouter size: M maintainer Maintainer-authored PR labels Jul 5, 2026
@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 8:07 AM ET / 12:07 UTC.

Summary
The PR batches ten focused fixes across transcript echo formatting, MCP event handling, silent-token prefix matching, Discord log previews, bounded browser/ClawRouter JSON reads, session_status time output, Linux installer package-manager detection, iOS TLS deep-link defaults, and transient Undici exception classification.

PR surface: Source +63, Tests +211, Other +9. Total +283 across 24 files.

Reproducibility: yes. from source inspection: current main still shows the replacement-string transcript paths, unbounded response.json reads, pacman-or-Arch checks, TLS 18789 default, and fatal Undici shape gap that the PR targets. I did not execute tests in this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: extensions/browser/src/browser/cdp.helpers.test.ts, serialized state: extensions/browser/src/browser/chrome.test.ts, serialized state: extensions/clawrouter/usage.test.ts, serialized state: extensions/clawrouter/usage.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #100258
Summary: This PR is the canonical maintainer landing candidate for ten adjacent source PRs; the fixes are separate edge cases rather than one root cause, but the source branches are represented in this batch.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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 current-head required CI jobs to finish.
  • [P1] Have a maintainer explicitly accept or request changes for the narrow fail-closed compatibility tradeoffs.

Risk before merge

  • [P1] The PR intentionally makes several boundaries fail closed: oversized CDP/ClawRouter JSON is rejected and pacman is used only when real Arch detection succeeds, so maintainers should accept that compatibility tradeoff before merge.
  • [P1] At review time, the latest head still had queued or in-progress exact-head CI jobs even though the Real behavior proof check and several fast checks had passed.

Maintainer options:

  1. Accept bounded hardening (recommended)
    Land after current-head required checks finish, accepting that oversized ClawRouter/CDP JSON and non-Arch pacman false positives now fail closed.
  2. Add a compatibility escape hatch
    Request a bounded opt-in or broader Arch detection only if maintainers need to preserve those unusual setups.
  3. Split the batch
    Pause and split by source PR if maintainers want narrower rollback and review ownership per edge case.

Next step before merge

  • No automated repair is identified; maintainers need to finish current-head validation and make the landing-risk call for the compatibility-sensitive hardening changes.

Maintainer decision needed

  • Question: Should this maintainer-labeled batch land as one PR once current-head required checks finish, accepting the narrow fail-closed compatibility changes for bounded HTTP reads and Arch/pacman detection?
  • Rationale: The patch appears correct, but it intentionally changes existing edge-case behavior at installer and external-response boundaries, which is a maintainer landing-risk call rather than an automation repair.
  • Likely owner: steipete — The PR branch is the current maintainer rebuild and steipete is the visible owner of the batch landing decision.
  • Options:
    • Land The Batch (recommended): Accept the bounded-read and installer-routing compatibility tradeoffs and land the combined maintainer batch after current-head required checks finish.
    • Soften Compatibility: Ask for a narrower compatibility path only if maintainers need to preserve unusually large provider responses or pacman-only Arch detection environments.
    • Split By Source Fix: Pause the batch and land smaller PRs if maintainers want per-surface rollback and ownership instead of one aggregate landing unit.

Security
Cleared: No concrete security or supply-chain regression was found; the network/body-read changes reduce resource-exhaustion exposure and the installer change does not add new execution sources.

Review details

Best possible solution:

Land the batch after maintainers accept the narrow fail-closed compatibility changes and the current-head required checks finish, keeping the focused regression tests with each hardened boundary.

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

Yes, from source inspection: current main still shows the replacement-string transcript paths, unbounded response.json reads, pacman-or-Arch checks, TLS 18789 default, and fatal Undici shape gap that the PR targets. I did not execute tests in this read-only review.

Is this the best way to solve the issue?

Yes; the fixes are applied at the owning boundaries and reuse existing SDK/time/truncation helpers where appropriate. The remaining question is merge-risk acceptance for the deliberate fail-closed behavior, not a better code location.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The diff deliberately changes fail-closed behavior for oversized provider/diagnostic JSON and pacman-based installer routing, which can affect unusual existing setups.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comment include after-fix terminal output, focused test results, Blacksmith Testbox proof, and the current Real behavior proof check is passing for this head.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body/comment include after-fix terminal output, focused test results, Blacksmith Testbox proof, and the current Real behavior proof check is passing for this head.

Label justifications:

  • P2: This is a normal-priority batch of bounded correctness and hardening fixes with limited but real user-facing runtime and installer impact.
  • merge-risk: 🚨 compatibility: The diff deliberately changes fail-closed behavior for oversized provider/diagnostic JSON and pacman-based installer routing, which can affect unusual existing setups.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body/comment include after-fix terminal output, focused test results, Blacksmith Testbox proof, and the current Real behavior proof check is passing for this head.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body/comment include after-fix terminal output, focused test results, Blacksmith Testbox proof, and the current Real behavior proof check is passing for this head.
Evidence reviewed

PR surface:

Source +63, Tests +211, Other +9. Total +283 across 24 files.

View PR surface stats
Area Files Added Removed Net
Source 10 78 15 +63
Tests 11 262 51 +211
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 3 17 8 +9
Total 24 357 74 +283

What I checked:

  • Repository policy applied: The full root AGENTS.md plus scoped extensions, scripts, test, and plugin-sdk guides were read; their plugin-boundary, compatibility-risk, and protected-label guidance affected this review. (AGENTS.md:1, 785ab7477982)
  • Current main still has the fixed behaviors: Current main still uses replacement-string transcript insertion, unbounded response.json reads at the CDP/ClawRouter boundaries, pacman-or-Arch installer routing, and the TLS deep-link 18789 default that the PR changes. (src/media-understanding/echo-transcript.ts:18, 785ab7477982)
  • Patch diff matches the stated fixes: The current PR head changes only the listed runtime, plugin, app, installer, and regression-test files, and prior review-comment files from older heads are no longer in the current diff. (aeb9bc42ac78)
  • Dependency and helper contracts checked: The shared bounded response helper cancels streams on overflow, readProviderJsonResponse wraps bounded JSON parsing, and upstream Undici source shows aborted fetch bodies can error a stream with TypeError('terminated'). (src/infra/http-body.ts:262, 785ab7477982)
  • Review continuity checked: The two earlier Codex review comments targeted src/tui/tui-plugin-approvals.ts and an Android GatewayConfigResolver file from older commits; neither file is present in the current PR file list. (aeb9bc42ac78)
  • Proof and live state checked: The PR body and maintainer comment provide focused terminal proof, Blacksmith Testbox proof, autoreview status, and the current GitHub check list shows the Real behavior proof check passing while some exact-head CI jobs were still queued or running at review time. (aeb9bc42ac78)

Likely related people:

  • Sanjays2402: Current-main blame for many central touched lines points to commit 6cc534b, which recently rewrote or carried these runtime/app/plugin surfaces. (role: recent area contributor; confidence: high; commits: 6cc534bbad7b; files: src/media-understanding/echo-transcript.ts, extensions/matrix/src/matrix/monitor/preflight-audio.ts, src/mcp/channel-bridge.ts)
  • steipete: The PR branch is authored as a maintainer rebuild with preserved coauthorship, and recent main history also shows adjacent iOS app work in commit e60c510. (role: current batch owner and adjacent contributor; confidence: high; commits: aeb9bc42ac78, e60c5105607a; files: apps/shared/OpenClawKit/Sources/OpenClawKit/DeepLinks.swift, apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeepLinksSecurityTests.swift)
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-05T09:48:00.272Z sha c8d5946 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T09:57:14.718Z sha a58328f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T10:03:40.052Z sha a58328f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T10:36:59.237Z sha e0d043e :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T11:00:13.174Z sha 4a2f9e5 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 5, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: ios App: ios app: web-ui App: web-ui gateway Gateway runtime agents Agent runtime and tooling size: XL and removed size: M labels Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready at 12b08861bf387ebdfa265e3d423b4ed8d605b110.

Work completed:

  • Rebuilt the ten source fixes as focused maintainer commits with contributor co-authorship preserved.
  • Added or retained regression coverage for every changed behavior.
  • Resolved the custom-token Unicode lint finding by removing redundant string spreading.
  • Rebased onto the current gateway-fixture and browser-CDP CI stabilizations from main.

Focused proof:

  • node scripts/run-vitest.mjs across the 12 touched test files: 282 assertions passed.
  • swift test --package-path apps/shared/OpenClawKit --filter DeepLinksSecurityTests: 19 tests passed.
  • Gateway session-utils regression shard after the current-main fixture repair: 528 assertions passed.
  • bash -n scripts/install.sh
  • git diff --check
  • oxfmt --check on all changed TypeScript files.
  • Fresh branch autoreview: no accepted/actionable findings (patch is correct, confidence 0.94).

Broad proof:

  • Blacksmith Testbox check:changed: passed on tbx_01kwrssmpd8f5wek4ec5ce2mxe (run 28736283867).
  • Exact-head hosted CI: passed all 56 jobs, including QA Smoke, macOS Swift, iOS, and both Node whole-suite shards (run 28740609768).
  • scripts/pr review-validate-artifacts 100258: READY FOR /prepare-pr, zero findings.
  • OPENCLAW_TESTBOX=1 scripts/pr prepare-run 100258: passed with gates_mode=hosted_exact_head.

Known proof gaps: none.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations app: android App: android labels Jul 5, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 5, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6afca08712

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return;
}
selectedDecision = decision;
allowDecisionArmed = decision !== "deny";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep allow approvals unarmed after navigation

When an approval prompt includes Deny plus an allow option, keyboard navigation from the default Deny row triggers this selection-change handler before Enter. Setting allowDecisionArmed here means the first Enter on Allow resolves the approval immediately, bypassing the onSelect double-confirm gate added below; accidental arrow/Enter approval is exactly the scenario the confirmation is meant to prevent. Keep the allow option unarmed until the first select action for that selected row.

Useful? React with 👍 / 👎.

@steipete
steipete force-pushed the codex/land-small-fixes-batch-3 branch from dbd1baf to 12b0886 Compare July 5, 2026 12:21
@steipete
steipete merged commit deac98e into main Jul 5, 2026
104 checks passed
@steipete
steipete deleted the codex/land-small-fixes-batch-3 branch July 5, 2026 12:33
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
* fix(media): preserve dollar sequences in transcript echoes

Co-authored-by: uditDewan <[email protected]>

* fix(mcp): catch rejected gateway event handlers

Co-authored-by: 陈宪彪0668000387 <[email protected]>

* fix(auto-reply): support punctuated silent token prefixes

Co-authored-by: simon-w <[email protected]>

* fix(discord): keep voice log previews UTF-16 safe

Co-authored-by: sunlit-deng <[email protected]>

* fix(clawrouter): bound usage response reads

Co-authored-by: 杨浩宇0668001029 <[email protected]>

* fix(browser): bound CDP JSON response reads

Co-authored-by: hailory <[email protected]>

* fix(status): include current time in session status

Co-authored-by: connermo <[email protected]>

* fix(installer): require Arch detection before pacman

Co-authored-by: Iliya Abolghasemi <[email protected]>

* fix(apps): default TLS gateway deep links to port 443

Co-authored-by: ben.li <[email protected]>

* fix(infra): keep Undici terminated exceptions nonfatal

Co-authored-by: harjoth <[email protected]>

* fix(auto-reply): avoid Unicode-unsafe token spread

---------

Co-authored-by: uditDewan <[email protected]>
Co-authored-by: 陈宪彪0668000387 <[email protected]>
Co-authored-by: simon-w <[email protected]>
Co-authored-by: sunlit-deng <[email protected]>
Co-authored-by: 杨浩宇0668001029 <[email protected]>
Co-authored-by: hailory <[email protected]>
Co-authored-by: connermo <[email protected]>
Co-authored-by: Iliya Abolghasemi <[email protected]>
Co-authored-by: ben.li <[email protected]>
Co-authored-by: harjoth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: discord Channel integration: discord channel: matrix Channel integration: matrix extensions: clawrouter Extension: clawrouter 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant