Skip to content

fix(core): use file:// URLs for command autoload imports#180

Merged
zrosenbauer merged 1 commit into
mainfrom
fix/windows-autoload-paths
May 8, 2026
Merged

fix(core): use file:// URLs for command autoload imports#180
zrosenbauer merged 1 commit into
mainfrom
fix/windows-autoload-paths

Conversation

@zrosenbauer
Copy link
Copy Markdown
Member

Summary

Fixes #179. On Windows, kidd <subcommand> printed "Unknown arguments" for every positional because the runtime autoloader silently failed to register any commands. The cause: packages/core/src/autoload.ts passed absolute filesystem paths straight to ESM import(). Node ESM rejects backslash-only paths and treats C: as an unknown URL scheme (ERR_UNSUPPORTED_ESM_URL_SCHEME). The error was swallowed unless KIDD_DEBUG=1.

path.join was doing the right thing — building OS-native paths. The bug was the next step: those native paths aren't valid ESM specifiers. Fix: convert with pathToFileURL(...).href from node:url before import(), which produces file:///... URLs that resolve identically on macOS, Linux, and Windows.

Changes

  • packages/core/src/autoload.ts — wrap the path in pathToFileURL(filePath).href before the dynamic import; update the debug warning to log the URL form
  • packages/core/src/autoload.test.ts — mock node:url's pathToFileURL to passthrough so existing path-form vi.doMock keys still match (vitest can't intercept file:// specifiers through helper indirection); add a regression test asserting pathToFileURL is invoked for command files
  • .changeset/fix-windows-autoload-paths.md@kidd-cli/core patch changeset

Test plan

  • pnpm check (typecheck + lint + format) passes
  • pnpm test passes (1075 core tests, 17 tasks)
  • New regression test verifies pathToFileURL is called on the absolute path
  • Manual verification on Windows 11: kidd build --compile, kidd dev, kidd stories all execute their handlers instead of yargs printing "Unknown arguments"

Out of scope

There is a related concern in packages/bundler/src/autoloader/generate-autoloader.ts — it embeds native filesystem paths into generated import string literals, which would also break when building a CLI on Windows. That isn't the symptom reported in #179 (which is purely runtime command discovery in @kidd-cli/cli), so it's deliberately not included here. Happy to follow up in a separate PR if Windows testing surfaces it.

Resolves #179. On Windows, kidd's runtime autoloader silently failed to
register every subcommand because absolute filesystem paths (with
backslashes and drive letters) are not valid ESM specifiers. `import()`
threw `ERR_UNSUPPORTED_ESM_URL_SCHEME` for each command file; the error
was swallowed unless `KIDD_DEBUG=1`, leaving yargs with no subcommands
and reporting "Unknown arguments: build" for `kidd build --compile`.

Convert the absolute path to a `file://` URL via `pathToFileURL` before
passing it to `import()` so command files load on both platforms.

Co-Authored-By: Claude <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 8, 2026

🦋 Changeset detected

Latest commit: e3edcf9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@kidd-cli/core Patch
@kidd-cli/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented May 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
oss-kidd Ignored Ignored May 8, 2026 3:05am

Request Review

@zrosenbauer zrosenbauer marked this pull request as ready for review May 8, 2026 03:05
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5e73d57-7200-42f2-9484-00a0ffd59c52

📥 Commits

Reviewing files that changed from the base of the PR and between 4436c4b and e3edcf9.

⛔ Files ignored due to path filters (1)
  • .changeset/fix-windows-autoload-paths.md is excluded by !.changeset/**
📒 Files selected for processing (2)
  • packages/core/src/autoload.test.ts
  • packages/core/src/autoload.ts

📝 Walkthrough

Walkthrough

This PR fixes Windows subcommand registration by converting dynamic import paths to file:// URLs. The implementation adds pathToFileURL from node:url to autoload.ts, then updates importCommand() to compute pathToFileURL(filePath).href and pass the resulting URL specifier to import() instead of raw filesystem paths. Test coverage mocks pathToFileURL and adds a new assertion verifying that filesystem paths are normalized before dynamic import, confirming Windows path resolution compatibility.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • joggrdocs/kidd#150: Modifies import error handling in the same importCommand() function, likely affected by or dependent on this path normalization change.
  • joggrdocs/kidd#113: Alters dynamic import try/catch logic in importCommand(), may interact with the new file:// URL specifier approach.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main change: converting filesystem paths to file:// URLs for ESM import compatibility on Windows.
Description check ✅ Passed Description is comprehensive and directly related to the changeset, explaining the Windows ESM path issue, the fix, and test coverage.
Linked Issues check ✅ Passed The PR fully addresses #179 by converting filesystem paths to file:// URLs via pathToFileURL().href before dynamic import(), resolving the ERR_UNSUPPORTED_ESM_URL_SCHEME error on Windows.
Out of Scope Changes check ✅ Passed All changes are scoped to runtime command discovery in autoload.ts/test.ts and a changeset entry; deliberately excludes the bundler autoloader concern identified in the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-autoload-paths

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Merging this PR will degrade performance by 27.4%

❌ 2 regressed benchmarks
⏩ 2 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime icons status 293.4 ms 404.2 ms -27.4%
WallTime icons --help 276.2 ms 377.5 ms -26.83%

Comparing fix/windows-autoload-paths (e3edcf9) with main (5fc08d1)2

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (4436c4b) during the generation of this report, so 5fc08d1 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@zrosenbauer zrosenbauer merged commit 45338ce into main May 8, 2026
13 of 14 checks passed
@zrosenbauer zrosenbauer deleted the fix/windows-autoload-paths branch May 8, 2026 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cli): subcommands not registered on Windows — "Unknown arguments" for build/dev/stories

1 participant