Repository navigation
feat(cli): add -C/--cwd flag to set the working directory explicitly #289
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
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,45 @@ | ||
| import { describe, expect, test, vi } from "vitest"; | ||
| import type { SettingsManager } from "../../../packages/coding-agent/src/core/settings-manager.ts"; | ||
|
|
||
| const packageManagerMocks = vi.hoisted(() => { | ||
| const instance = { resolve: vi.fn<(fn: () => Promise<string>) => Promise<{ themes: never[] }>>() }; | ||
| return { instance, ctor: vi.fn<(options: { cwd: string }) => typeof instance>() }; | ||
| }); | ||
|
|
||
| // Capture the cwd the startup theme discovery builds its package manager with. | ||
| vi.mock("@step-harness/coding-agent", async (importOriginal) => { | ||
| const actual = (await importOriginal()) as Record<string, unknown>; | ||
| return { | ||
| ...actual, | ||
| DefaultPackageManager: class { | ||
| constructor(options: { cwd: string }) { | ||
| packageManagerMocks.ctor(options); | ||
| return packageManagerMocks.instance as unknown as this; | ||
| } | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| const { loadStartupThemes } = await import("../src/ui/startup-ui.ts"); | ||
|
|
||
| function fakeSettingsManager(): SettingsManager { | ||
| return { | ||
| getGlobalSettings: () => ({}), | ||
| } as unknown as SettingsManager; | ||
| } | ||
|
|
||
| describe("loadStartupThemes discovery base", () => { | ||
| test("discovers project-level themes against the resolved cwd, not the launcher", async () => { | ||
| packageManagerMocks.instance.resolve.mockResolvedValue({ themes: [] }); | ||
|
|
||
| await loadStartupThemes(fakeSettingsManager(), { | ||
| agentDir: "/agent", | ||
| configDirName: ".stepcode", | ||
| cwd: "/target/project", | ||
| }); | ||
|
|
||
| expect(packageManagerMocks.ctor).toHaveBeenCalledWith( | ||
| expect.objectContaining({ cwd: "/target/project" }), | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,8 @@ export interface Args { | |
| sessionId?: string; | ||
| fork?: string; | ||
| sessionDir?: string; | ||
| /** Working directory override. Defaults to process.cwd() when unset. */ | ||
| cwd?: string; | ||
| models?: string[]; | ||
| tools?: string[]; | ||
| excludeTools?: string[]; | ||
|
|
@@ -391,6 +393,12 @@ export function parseArgs(args: string[]): Args { | |
| result.sessionDir = taken.value; | ||
| i = taken.nextIndex; | ||
| } | ||
| } else if (arg === "--cwd" || arg === "-C") { | ||
| const taken = takeOptionValue(args, i, "--cwd", result); | ||
| if (taken) { | ||
| result.cwd = taken.value; | ||
| i = taken.nextIndex; | ||
| } | ||
| } else if (arg === "--models") { | ||
| const taken = takeOptionValue(args, i, "--models", result); | ||
| if (taken) { | ||
|
|
@@ -673,6 +681,7 @@ ${stepPermissionOptionsText} | |
| --session-id <id> Use exact project session ID, creating it if missing | ||
| --fork <path|id> Fork specific session file or partial UUID into a new session | ||
| --session-dir <dir> Directory for session storage and lookup | ||
| --cwd, -C <dir> Working directory (defaults to the process cwd) | ||
|
Collaborator
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. Ordering nit: neighbours here and in |
||
| --no-session Don't save session (ephemeral) | ||
| --name, -n <name> Set session display name | ||
| --models <patterns> Comma-separated model patterns for Ctrl+P cycling | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -649,6 +649,8 @@ describe("parseArgs", () => { | |
| ["--session-id", "--session-id"], | ||
| ["--fork", "--fork"], | ||
| ["--session-dir", "--session-dir"], | ||
| ["--cwd", "--cwd"], | ||
| ["-C", "--cwd"], | ||
| ["--models", "--models"], | ||
| ["--tools", "--tools"], | ||
| ["-t", "--tools"], | ||
|
|
@@ -698,6 +700,12 @@ describe("parseArgs", () => { | |
| expect(result.diagnostics).toEqual([]); | ||
| }); | ||
|
|
||
| test("--cwd and -C both capture the working directory", () => { | ||
|
Collaborator
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. These cases only prove the parser stores the token; nothing here covers the actual behaviour the PR claims (settings discovery, session storage and resource loading keying off the path). A cheap end-to-end assertion is possible because const seen: string[] = [];
await prepareMain(["--cwd", dir, "--version"], {
settingsManagerFactory: (cwd) => { seen.push(cwd); return SettingsManager.inMemory(); },
});
expect(seen[0]).toBe(dir);That would also have caught the dead- |
||
| expect(parseArgs(["--cwd", "/tmp/project"]).cwd).toBe("/tmp/project"); | ||
| expect(parseArgs(["-C", "/tmp/project"]).cwd).toBe("/tmp/project"); | ||
| expect(parseArgs(["-C", "/tmp/project", "fix the bug"]).messages).toEqual(["fix the bug"]); | ||
| }); | ||
|
|
||
| test("an invalid --thinking value still warns rather than erroring", () => { | ||
| const result = parseArgs(["--thinking", "bogus"]); | ||
| expect(result.thinking).toBeUndefined(); | ||
|
|
||
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.
Two small parser questions:
--cwd=/abs/pathis not handled, so it falls through to thearg.startsWith("--")branch and is recorded asunknownFlags.set("cwd", "/abs/path")— i.e. treated as an extension flag. Either support the=form like--completion-check=does, or state explicitly that=is reserved for extension flags.--cwd "") is accepted and later becomes the process cwd throughresolvePath, which is an easy typo to miss in a launcher script. Worth rejecting alongside the existence check.