-
Notifications
You must be signed in to change notification settings - Fork 7
Add powershell (windows) support #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| ## Stage 1: Inspect Current CLI Launch Flow | ||
| **Goal**: Confirm how PromptKit detects and spawns supported CLIs on Windows. | ||
| **Success Criteria**: Relevant launcher code and tests identified. | ||
| **Tests**: Read existing `cli/lib/launch.js`, `cli/bin/cli.js`, and `cli/tests/launch.test.js`. | ||
| **Status**: Complete | ||
|
|
||
| ## Stage 2: Add Codex Windows-Compatible Launch Support | ||
| **Goal**: Support `--cli codex` and ensure Windows launches the npm `.cmd` shim instead of relying on shell-specific resolution. | ||
| **Success Criteria**: `codex` is accepted, launched correctly on Windows, and existing CLIs retain behavior. | ||
| **Tests**: Update launch unit tests for `codex` command resolution and dry-run output. | ||
| **Status**: Complete | ||
|
|
||
| ## Stage 3: Verify and Finalize | ||
| **Goal**: Run the CLI test suite and confirm the change is isolated. | ||
| **Success Criteria**: Relevant verification completed and docs/help text reflect Codex support. | ||
| **Tests**: `node .\\bin\\cli.js interactive --cli codex --dry-run`; attempted `node --test --test-concurrency=1 tests\\launch.test.js` | ||
| **Status**: Complete |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ | |
| "llm", | ||
| "ai", | ||
| "copilot", | ||
| "codex", | ||
| "prompt-templates", | ||
| "agentic-ai", | ||
| "developer-tools" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,6 +140,14 @@ describe("Launch Module", () => { | |
| assert.strictEqual(runDetectCli(), "claude"); | ||
| }); | ||
|
|
||
| it("TC-CLI-072A: detectCli finds codex after claude", () => { | ||
| removeMockCmd("copilot"); | ||
| removeMockCmd("gh"); | ||
| removeMockCmd("claude"); | ||
| createMockCmd("codex"); | ||
| assert.strictEqual(runDetectCli(), "codex"); | ||
| }); | ||
|
|
||
| it("TC-CLI-074: gh without copilot extension is not detected as gh-copilot", () => { | ||
| removeMockCmd("copilot"); | ||
| removeMockCmd("claude"); | ||
|
|
@@ -322,7 +330,7 @@ describe("Launch Module", () => { | |
| return JSON.parse(fs.readFileSync(captureFile, "utf8")); | ||
| } | ||
|
|
||
| for (const cliName of ["claude", "copilot", "gh-copilot"]) { | ||
| for (const cliName of ["claude", "copilot", "gh-copilot", "codex"]) { | ||
| // TC-CLI-082 and TC-CLI-083 combined — run once per CLI | ||
| it(`TC-CLI-082/083: ${cliName} spawned with originalCwd and --add-dir for staging dir`, () => { | ||
| const mockBinDir = path.join(cwdTestTmpDir, `mock-bin-${cliName}`); | ||
|
|
@@ -371,7 +379,7 @@ describe("Launch Module", () => { | |
| }); | ||
|
|
||
| describe("--dry-run flag", () => { | ||
| for (const cliName of ["copilot", "gh-copilot", "claude"]) { | ||
| for (const cliName of ["copilot", "gh-copilot", "claude", "codex"]) { | ||
| it(`TC-CLI-085: --dry-run prints spawn command for ${cliName} without launching`, () => { | ||
| // --dry-run must print the command and args then exit 0 without | ||
| // spawning the real LLM CLI. We run with an empty PATH so that | ||
|
|
@@ -406,10 +414,25 @@ describe("Launch Module", () => { | |
|
|
||
| // Parse the args line as JSON so we verify structure, not wording. | ||
| const lines = stdout.split("\n"); | ||
| const cmdLine = lines.find((l) => l.trim().startsWith("cmd:")); | ||
| const argsLine = lines.find((l) => l.trim().startsWith("args:")); | ||
| assert.ok(cmdLine, `--dry-run output should include a 'cmd:' line for ${cliName}`); | ||
| assert.ok(argsLine, `--dry-run output should include an 'args:' line for ${cliName}`); | ||
| const parsedCmd = cmdLine.trim().slice("cmd:".length).trim(); | ||
| const parsedArgs = JSON.parse(argsLine.trim().slice("args:".length).trim()); | ||
|
|
||
| if (cliName === "gh-copilot") { | ||
| assert.strictEqual(parsedCmd, "gh", "gh-copilot should spawn gh"); | ||
| } else if (process.platform === "win32") { | ||
| assert.strictEqual( | ||
| parsedCmd, | ||
| `${cliName}.cmd`, | ||
| `${cliName} should spawn the Windows .cmd shim` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion will fail on Windows — which is ironic given the PR's intent! 😄 The dry-run test uses an empty PATH directory ( To fix, create a // Create a .cmd stub so resolveSpawnCommand finds it
if (process.platform === "win32" && cliName !== "gh-copilot") {
fs.writeFileSync(path.join(emptyBinDir, `${cliName}.cmd`), "");
}Alternatively, accept the bare command name when no |
||
| ); | ||
| } else { | ||
| assert.strictEqual(parsedCmd, cliName, `${cliName} should spawn its bare command`); | ||
| } | ||
|
|
||
| // The bootstrap prompt must appear as exactly one element containing bootstrap.md, | ||
| // not split across multiple elements (the shell: true regression). | ||
| const bootstrapArgs = parsedArgs.filter((a) => a.includes("bootstrap.md")); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing nit (optional fix): This line says detection uses
execFileSyncwithwhere/which, but the implementation was changed to use direct PATH scanning in a previous PR. Since you're editing this section anyway, it might be worth correcting. Totally optional — not something you introduced.