Skip to content

fix: support native Windows project state paths - #17

Open
Linxiushen wants to merge 1 commit into
Tencent:mainfrom
Linxiushen:fix/windows-project-state-path
Open

Linxiushen wants to merge 1 commit into
Tencent:mainfrom
Linxiushen:fix/windows-project-state-path

Conversation

@Linxiushen

Copy link
Copy Markdown

Summary

Project initialization and catalog registration fail on Windows because gitRoot() returns a native path, while projectId() splits only on /. For a repository such as C:\work\sample-skill, the resulting state path contains <home>\projects\C:\work\sample-skill-<hash>\runs, and directory creation fails with ENOENT.

What changed

Use path.basename() for the project name, preserving the existing fallback and full-root hash. Add real Git/SQLite tests for state persistence after reopening, isolation between same-named repositories, and catalog registration.

Reproduction

On Windows, with the new regression file added to the upstream base:

pnpm exec tsc -p tsconfig.json
node --disable-warning=ExperimentalWarning --test dist/test/project-paths.test.js

All three tests fail at directory creation on the baseline and pass with this change. The tests use temporary native paths, including spaces, without filesystem or database mocks.

Validation and results

  • Windows Node 24.19.0: TypeScript build passes; regression tests 0/3 → 3/3 passing.
  • Linux Node 22.23.3: pnpm test passes 34/34, including the normal build; 7/7 representative POSIX project IDs exactly match the original implementation.
  • Python static-check tests 2/2, both required Skill static checks, git diff --check, gitleaks directory/history scans, dependency audit and registry-signature checks pass.
  • CLI help, package dry run, installation from the exact patched Git commit, installed version/setup/doctor and build-dependency isolation checks pass.

Effect and limits

Windows project state can now be created and reopened. The hash still distinguishes repositories with the same directory name, and existing POSIX IDs retain their format. Linux validation used the supported Node 22 line; hosted CI uses Node 24. The complete suite's existing POSIX shell fixtures were run on Linux, while the new storage regressions were also run natively on Windows.

Review checklist

  • The reproduction is self-contained above; no existing Issue was found.
  • Focused repository tests pass, with counts reported above.
  • The diff contains only files needed for this fix.
  • I ran git diff --check.
  • I ran gitleaks or an equivalent credential scan.
  • This change contains no API keys, private prompts, private traces, or generated local state.
  • No SkillHone-generated repair was pushed or merged.
  • A user has reviewed the Issue, tests, commits, and changed files before merge.

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.

1 participant