Skip to content

fix(scripts): encode direct entrypoint paths - #4077

Open
orangeCatDeveloper wants to merge 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/script-entrypoint-spaces
Open

fix(scripts): encode direct entrypoint paths#4077
orangeCatDeveloper wants to merge 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/script-entrypoint-spaces

Conversation

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor

Summary

Directly running the cursor-overlay builder or DeepSeek harness toolchain preparer from a workspace path containing spaces exits successfully without executing the script body.

Each entrypoint compared its encoded import.meta.url with an unencoded file://${process.argv[1]} string, so any character requiring URL encoding made the direct-execution guard false.

Build the comparison URL with pathToFileURL(), matching the repository's other direct script guards. Add a pure Node contract that demonstrates the encoded-path difference and keeps both entrypoints on the safe comparison, then run it in CI.

Evidence

Before:
file:///tmp/Maka%20Project/script.mjs
file:///tmp/Maka Project/script.mjs
false

After:
file:///tmp/Maka%20Project/script.mjs
file:///tmp/Maka%20Project/script.mjs
true

Verification

  • node --test scripts/script-entrypoints.test.mjs — 1/1 passed
  • node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs scripts/verify-windows-harness.test.mjs — 88/88 passed
  • npm run lint
  • npm run format:check
  • node scripts/asf-license-headers.mjs check
  • git diff --check

Full build and typecheck were not run for this entrypoint-only draft.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the URL-safe entrypoint guards, regression contract, CI wiring, and PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review August 28, 2026 09:44
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Aug 28, 2026

@Astro-Han Astro-Han 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.

Approved at exact head 28c509a7ff2fb4fad1546b13d951108c6de021f6. I found no P0–P2 issues.

The bug is reachable on a normal workspace path containing spaces: import.meta.url is URL-encoded while the previous hand-built file://${process.argv[1]} value was not, so the direct-entrypoint guard silently skipped the script body. Using pathToFileURL(process.argv[1]).href on both affected entrypoints is the smallest correct fix and matches the repository’s established script convention without introducing another helper or authority.

Non-blocking P3: the new test checks source text rather than executing an entrypoint from an encoded path. A future cleanup could turn it into a repository-wide contract prohibiting raw file://${process.argv[1]} guards. This does not block the two production fixes.

The exact-head hosted test check is successful.

AI-assisted review: Codex coordinated an @reviewer pass; I verified the exact-head diff, existing entrypoint convention, CI state, and the final severity judgment. Astro-Han owns this approval decision.

简体中文

精确 head 上没有 P0–P2。带空格路径的问题真实存在,两处 pathToFileURL() 修复符合现有约定且足够小。源码正则测试可在以后改进,属于不阻塞的 P3。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants