Skip to content

Fix Windows Codex plugin test compatibility#1

Open
Shemshidin wants to merge 1 commit into
r266-tech:mainfrom
Shemshidin:fix/windows-codex-plugin-tests
Open

Fix Windows Codex plugin test compatibility#1
Shemshidin wants to merge 1 commit into
r266-tech:mainfrom
Shemshidin:fix/windows-codex-plugin-tests

Conversation

@Shemshidin

Copy link
Copy Markdown

Summary

  • make npm test avoid direct .mjs execution and shell redirection
  • add a cross-platform CLI help smoke test
  • replace /dev/stdin privacy probe with a temp registry file
  • support Windows PATHEXT command lookup
  • sanitize redacted path fields through the generic string redactor

Test

  • npm test
  • npm pack --dry-run --json

@r266-tech

Copy link
Copy Markdown
Owner

Thanks for this, @Shemshidin — these are exactly the right cross-platform fixes. 🙏

A few things I really appreciate here:

  • which() PATHEXT support — resolving .EXE/.CMD/.BAT (and / vs \ separators) is the classic gap that silently breaks command lookup on Windows.
  • Dropping /dev/stdin for a temp registry file in the privacy probe, plus process.execPath instead of a bare node — both are real portability wins, since neither /dev/stdin nor a node on PATH is guaranteed on Windows.
  • The new smoke-cli-help.mjs replacing ... --help >/dev/null — pulling that POSIX-only redirection out of npm test and into a spawnSync is exactly the right call.
  • Routing path fields through redactString(redactedPath(...)) is a good defense-in-depth tightening of the redactor.

Two things before this can land:

  1. The branch currently conflicts with main (mergeable: CONFLICTING). Could you rebase fix/windows-codex-plugin-tests onto the latest main and push?
  2. This repo doesn't report CI checks on PRs, and since this is specifically a Windows-compat change, could you run npm test after the rebase — ideally on Windows — and paste the result here? That gives us a concrete signal the suite is green on the platform you're targeting.

Once it's rebased and green, I'll do a proper review and we should be able to get it merged.

Thanks again for making the CLI Windows-friendly — and sorry for the slow first response here. 🚀

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.

3 participants