Skip to content

fix(scripts): use direct-run helper for Windows guards#110877

Open
qingminglong wants to merge 1 commit into
openclaw:mainfrom
qingminglong:codex/script-direct-run-guard
Open

fix(scripts): use direct-run helper for Windows guards#110877
qingminglong wants to merge 1 commit into
openclaw:mainfrom
qingminglong:codex/script-direct-run-guard

Conversation

@qingminglong

@qingminglong qingminglong commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

AI-assisted.

What Problem This Solves

Fixes an issue where several Windows-invoked OpenClaw scripts could silently skip their CLI entrypoint because file://${process.argv[1]} does not resolve reliably there.

Why This Change Was Made

The affected scripts now use the shared isDirectRunUrl helper, so direct-run detection behaves consistently on Windows without changing the actual script behavior.

User Impact

--help and direct execution now reach the intended script code on Windows instead of exiting with no output.

Evidence

Current head: ade35fa9e8a8fae0453f842cefae95e795fc60bb.

Windows PowerShell / Node v24.18.0 direct-run transcript at this exact head:

  • node --import tsx scripts/android-pin-version.ts --help exited 0 and printed Usage: node --import tsx scripts/android-pin-version.ts (--from-gateway | --version <YYYY.M.PATCH>) ....
  • node --import tsx scripts/android-app-i18n.ts check exited 0 and completed the entrypoint check, reporting sourceKeys=1525 and keys=1525 for all 21 locales.
  • node scripts/run-additional-boundary-checks.mjs --help exited 0 and printed Usage: node scripts/run-additional-boundary-checks.mjs [--shard <N/TOTAL>[,<N/TOTAL>]].
  • node scripts/verify-docker-attestations.mjs --help exited 0 and printed Usage: node scripts/verify-docker-attestations.mjs --platform linux/amd64 --platform linux/arm64 IMAGE....
  • node scripts/e2e/lib/package-compat.mjs 2026.4.25 exited 0 and printed 1.
  • node scripts/plan-release-workflow-matrix.mjs exited 0 and printed docker_e2e_count=0 plus live_models_count=0.
  • With GH_REPO=openclaw/openclaw, node scripts/ci-run-timings.mjs 29655772776 --limit 1 exited 0 and reported run 29655772776 as completed/success, including its slowest-job and longest-start-delay summaries.

Supporting checks at the same head:

  • node scripts/run-vitest.mjs test/scripts/android-direct-run.test.ts passed: 1 file, 2 tests.
  • git diff --check upstream/main...HEAD exited 0 with no whitespace errors.

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts docker Docker and sandbox tooling size: S labels Jul 18, 2026
@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. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Summary
The branch replaces eight script-local ESM direct-run guards with isDirectRunUrl and adds direct-entrypoint coverage for the two Android TypeScript scripts.

PR surface: Tests +30, Other +9. Total +39 across 8 files.

Reproducibility: no. independent current-main Windows reproduction was run in this read-only review, but the affected guard pattern and the exact-head Windows transcript make the path source-reproducible with medium confidence.

Review metrics: none identified.

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] Diagnose and clear checks-node-compact-small-1 and the aggregate CI gate on the current head before merge.

Next step before merge

  • No discrete mechanical repair is established; normal maintainer review should resolve the failing CI gate and verify the shared helper contract before merge.

Security
Cleared: The patch only adopts an existing local helper and adds focused subprocess tests; it introduces no dependency, permission, secret, workflow, or artifact-execution change.

Review details

Best possible solution:

Land the bounded helper adoption once the failing required CI lane is diagnosed and the merged result retains the demonstrated Windows direct-run behavior.

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

No independent current-main Windows reproduction was run in this read-only review, but the affected guard pattern and the exact-head Windows transcript make the path source-reproducible with medium confidence.

Is this the best way to solve the issue?

Likely yes: consolidating the affected guards on the existing shared helper is narrower and less drift-prone than maintaining Windows-specific comparisons in each script; the helper implementation itself still needs normal maintainer source review.

AGENTS.md: unclear because the file could not be read completely.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies an exact-head Windows PowerShell transcript covering the changed direct-run paths with successful observed output; redact any private machine details if fuller terminal artifacts are later attached.
  • 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 supplies an exact-head Windows PowerShell transcript covering the changed direct-run paths with successful observed output; redact any private machine details if fuller terminal artifacts are later attached.
  • remove rating: 🦐 gold shrimp: 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: This is a bounded Windows script-entrypoint regression fix with limited blast radius and no demonstrated core-runtime outage.
  • 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 supplies an exact-head Windows PowerShell transcript covering the changed direct-run paths with successful observed output; redact any private machine details if fuller terminal artifacts are later attached.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies an exact-head Windows PowerShell transcript covering the changed direct-run paths with successful observed output; redact any private machine details if fuller terminal artifacts are later attached.
Evidence reviewed

PR surface:

Tests +30, Other +9. Total +39 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 30 0 +30
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 7 16 7 +9
Total 8 46 7 +39

What I checked:

  • Changed entrypoint guards: The PR consistently replaces direct import.meta.url === file://… comparisons with the shared helper in the Android, CI, Docker, release-planning, boundary-check, and E2E package-compat scripts. (scripts/android-pin-version.ts:195, ade35fa9e8a8)
  • Focused regression coverage: The added test invokes both Android TypeScript scripts through Node with --import tsx and asserts that their direct CLI entrypoints produce usage output. (test/scripts/android-direct-run.test.ts:13, ade35fa9e8a8)
  • After-fix Windows proof: The PR body reports seven Windows PowerShell direct-run commands at the exact head SHA, including help and normal execution paths, with successful output; this addresses the prior stale-head proof concern. (ade35fa9e8a8)
  • Current check state: The supplied PR state shows the direct-behavior proof check and most script-related checks succeeding, but checks-node-compact-small-1 and the aggregate CI gate are failing and need ordinary maintainer/CI follow-up before landing. (ade35fa9e8a8)

Likely related people:

  • unresolved: The relevant owner should be identified from history of scripts/lib/direct-run.mjs; read-only shell inspection was unavailable, so no evidence-backed individual can be named conservatively. (role: shared script-helper history owner; confidence: low; files: scripts/lib/direct-run.mjs, scripts/android-pin-version.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 (6 earlier review cycles)
  • reviewed 2026-07-18T18:18:55.665Z sha 2991792 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-18T18:55:39.028Z sha 6fc8e5c :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-18T19:06:17.075Z sha 6fc8e5c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-18T19:48:24.531Z sha 6fc8e5c :: needs maintainer review before merge. :: none
  • reviewed 2026-07-20T09:03:37.291Z sha dbf9d9f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-20T11:42:44.396Z sha ade35fa :: needs real behavior proof before merge. :: none

@qingminglong
qingminglong force-pushed the codex/script-direct-run-guard branch from 2991792 to 6fc8e5c Compare July 18, 2026 18:52
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 18, 2026
@qingminglong

qingminglong commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 18, 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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jul 18, 2026
@BradGroux BradGroux mentioned this pull request Jul 19, 2026
Draft
@qingminglong
qingminglong force-pushed the codex/script-direct-run-guard branch from 6fc8e5c to dbf9d9f Compare July 20, 2026 08:59
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 20, 2026
@qingminglong
qingminglong force-pushed the codex/script-direct-run-guard branch from dbf9d9f to ade35fa Compare July 20, 2026 11:38
@qingminglong

qingminglong commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 20, 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 proof: sufficient ClawSweeper judged the real behavior proof convincing. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 20, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 20, 2026
@qingminglong qingminglong reopened this Jul 24, 2026
@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(scripts): use direct-run helper for Windows guards This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

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

docker Docker and sandbox tooling 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: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@qingminglong @Patrick-Erichsen