Repository navigation
Configurable Workspace Paths (Cycle 0025) - #12
Conversation
Make workspace directory layout configurable via .method.json paths section. Currently all paths are hardcoded constants.
The paths section in .method.json lets projects override where METHOD
puts backlog, design, retro, tests, graveyard, and the method dir:
{ "paths": { "backlog": ".method/backlog", "design": ".method/design", ... } }
Defaults match the current layout — zero behavioral change when no
config exists.
- Added PathsSchema with defaults to config.ts
- Removed BACKLOG_DIR/DESIGN_DIR/RETRO_DIR constants from domain.ts
- Workspace resolves all paths from config in constructor
- initWorkspace accepts optional PathsConfig
- detectWorkspaceDrift accepts testsDir parameter
- CLI passes config paths to initWorkspace
- New integration test proves custom paths work end-to-end
119 tests pass (118 prior + 1 new custom paths test).
Hill met. Workspace layout is now configurable via .method.json paths section. Defaults preserve existing behavior. 119 tests pass.
Summary by CodeRabbitRelease Notes
WalkthroughThis pull request implements configurable workspace directory paths via a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ConfigLoader
participant Workspace
participant FileSystem
User->>CLI: method init <target>
CLI->>ConfigLoader: loadConfig(target)
ConfigLoader->>FileSystem: read .method.json
ConfigLoader-->>CLI: Config {paths: PathsConfig}
CLI->>Workspace: initWorkspace(target, config.paths)
Workspace->>Workspace: resolvePaths(root, paths)
Workspace-->>Workspace: ResolvedPaths {backlog, design, retro, tests, graveyard, methodDir}
Workspace->>FileSystem: create directories at resolved paths
Workspace->>FileSystem: scaffold files (.method.json, cycles, etc.)
FileSystem-->>Workspace: success
Workspace-->>CLI: {created: [paths]}
CLI-->>User: initialization complete
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~65 minutes Rationale: This diff introduces interconnected structural changes across multiple core files with new type definitions, path resolution logic, and refactored initialization/workspace operations. Critical areas demanding verification: (1) path resolution correctness and absolute path computation; (2) exhaustive replacement of hardcoded constants throughout workspace operations; (3) proper threading of Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a4b4a470
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "mcpServers": { | ||
| "method": { | ||
| "command": "node", | ||
| "args": ["dist/cli.js", "mcp"] |
There was a problem hiding this comment.
The new MCP config points to dist/cli.js, but this repository does not include a built dist/ directory by default, so MCP clients that load .mcp.json in a fresh clone fail immediately with Cannot find module '/workspace/method/dist/cli.js'. That makes the shipped MCP integration unusable unless users discover and run a separate build step first; the config should target a runnable source entrypoint or explicitly enforce/build before launch.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config.ts`:
- Around line 22-29: The outer defaults duplicate the per-field defaults inside
PathsSchema; replace the explicit object passed to PathsSchema.default(...) with
an empty object so the inner .default() values on each PathsSchema field are
used when the paths key is missing — update the paths assignment that currently
uses PathsSchema.default({backlog:..., design:..., ...}) to use
PathsSchema.default({}) (or remove the outer defaults entirely) so only
PathsSchema's internal defaults govern values.
In `@tests/cli.test.ts`:
- Around line 434-447: Capture and assert the exit codes returned by runCli for
the inbox and pull invocations and add a runCli invocation to exercise the
custom tests directory with the method drift flow: change the two awaited
runCli([...], { cwd, stdout, stderr }) calls to store their return values (e.g.,
inboxResult and pullResult) and add assertions that inboxResult.code === 0 and
pullResult.code === 0 (or equivalent property returned by runCli); then invoke
runCli for the 'method drift' or appropriate test runner command pointing at the
custom tests folder (the tests:'spec' config) and assert its exit code is 0
and/or its output contains expected results to ensure the custom tests directory
is actually exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d8fb8c0-9ce1-4b3d-a220-4d71f984b4a5
📒 Files selected for processing (10)
.mcp.jsondocs/design/0025-configurable-workspace-paths/configurable-workspace-paths.mddocs/method/retro/0025-configurable-workspace-paths/configurable-workspace-paths.mddocs/method/retro/0025-configurable-workspace-paths/witness/verification.mdsrc/cli.tssrc/config.tssrc/domain.tssrc/drift.tssrc/index.tstests/cli.test.ts
💤 Files with no reviewable changes (1)
- src/domain.ts
| paths: PathsSchema.default({ | ||
| backlog: 'docs/method/backlog', | ||
| design: 'docs/design', | ||
| retro: 'docs/method/retro', | ||
| tests: 'tests', | ||
| graveyard: 'docs/method/graveyard', | ||
| method_dir: 'docs/method', | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
DRY violation — defaults are duplicated and will drift.
You've defined the same default values twice:
- Inner defaults (lines 6–11): via
.default()on each field ofPathsSchema - Outer defaults (lines 22–29): via
PathsSchema.default({...})with explicit values
When the paths key is missing entirely, Zod uses the outer default. But that object then gets parsed through PathsSchema, where the inner defaults would apply anyway. The explicit outer defaults are redundant — pass an empty object and let the inner defaults do their job.
This duplication is a maintenance hazard. Someone will change one set and forget the other. Murphy's law.
♻️ Eliminate duplicate defaults
export const ConfigSchema = z.object({
forge: z.enum(['github']).default('github'),
github_token: z.string().optional(),
github_repo: z.string().optional(), // owner/repo
- paths: PathsSchema.default({
- backlog: 'docs/method/backlog',
- design: 'docs/design',
- retro: 'docs/method/retro',
- tests: 'tests',
- graveyard: 'docs/method/graveyard',
- method_dir: 'docs/method',
- }),
+ paths: PathsSchema.default({}),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| paths: PathsSchema.default({ | |
| backlog: 'docs/method/backlog', | |
| design: 'docs/design', | |
| retro: 'docs/method/retro', | |
| tests: 'tests', | |
| graveyard: 'docs/method/graveyard', | |
| method_dir: 'docs/method', | |
| }), | |
| paths: PathsSchema.default({}), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config.ts` around lines 22 - 29, The outer defaults duplicate the
per-field defaults inside PathsSchema; replace the explicit object passed to
PathsSchema.default(...) with an empty object so the inner .default() values on
each PathsSchema field are used when the paths key is missing — update the paths
assignment that currently uses PathsSchema.default({backlog:..., design:...,
...}) to use PathsSchema.default({}) (or remove the outer defaults entirely) so
only PathsSchema's internal defaults govern values.
| // Inbox should work against custom paths | ||
| const inboxOut = new MemoryWriter(); | ||
| await runCli(['inbox', 'custom path test', '--legend', 'PROC'], { cwd: root, stdout: inboxOut, stderr: new MemoryWriter() }); | ||
| expect(existsSync(join(root, '.method/backlog/inbox/PROC_custom-path-test.md'))).toBe(true); | ||
|
|
||
| // Pull should work against custom paths | ||
| const pullOut = new MemoryWriter(); | ||
| await runCli(['pull', 'PROC_custom-path-test'], { cwd: root, stdout: pullOut, stderr: new MemoryWriter() }); | ||
| expect(existsSync(join(root, '.method/design/0001-custom-path-test/custom-path-test.md'))).toBe(true); | ||
|
|
||
| // Status should reflect it | ||
| const statusOut = new MemoryWriter(); | ||
| await runCli(['status'], { cwd: root, stdout: statusOut, stderr: new MemoryWriter() }); | ||
| expect(statusOut.output).toContain('0001-custom-path-test'); |
There was a problem hiding this comment.
Missing exit code assertions — test could silently pass on command failures.
You're asserting file existence and status output, but you're discarding the exit codes from inbox and pull. If either command fails with exit code 1 (e.g., due to a path resolution bug), this test would still pass as long as the filesystem state happens to match. That's a latent defect waiting to humiliate you in production.
Also: you configured tests: 'spec' but never exercised method drift against it. The custom tests directory path remains untested in this integration scenario.
🔧 Proposed fix to capture and assert exit codes
// Inbox should work against custom paths
const inboxOut = new MemoryWriter();
- await runCli(['inbox', 'custom path test', '--legend', 'PROC'], { cwd: root, stdout: inboxOut, stderr: new MemoryWriter() });
+ const inboxExitCode = await runCli(['inbox', 'custom path test', '--legend', 'PROC'], { cwd: root, stdout: inboxOut, stderr: new MemoryWriter() });
+ expect(inboxExitCode).toBe(0);
expect(existsSync(join(root, '.method/backlog/inbox/PROC_custom-path-test.md'))).toBe(true);
// Pull should work against custom paths
const pullOut = new MemoryWriter();
- await runCli(['pull', 'PROC_custom-path-test'], { cwd: root, stdout: pullOut, stderr: new MemoryWriter() });
+ const pullExitCode = await runCli(['pull', 'PROC_custom-path-test'], { cwd: root, stdout: pullOut, stderr: new MemoryWriter() });
+ expect(pullExitCode).toBe(0);
expect(existsSync(join(root, '.method/design/0001-custom-path-test/custom-path-test.md'))).toBe(true);
// Status should reflect it
const statusOut = new MemoryWriter();
- await runCli(['status'], { cwd: root, stdout: statusOut, stderr: new MemoryWriter() });
+ const statusExitCode = await runCli(['status'], { cwd: root, stdout: statusOut, stderr: new MemoryWriter() });
+ expect(statusExitCode).toBe(0);
expect(statusOut.output).toContain('0001-custom-path-test');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Inbox should work against custom paths | |
| const inboxOut = new MemoryWriter(); | |
| await runCli(['inbox', 'custom path test', '--legend', 'PROC'], { cwd: root, stdout: inboxOut, stderr: new MemoryWriter() }); | |
| expect(existsSync(join(root, '.method/backlog/inbox/PROC_custom-path-test.md'))).toBe(true); | |
| // Pull should work against custom paths | |
| const pullOut = new MemoryWriter(); | |
| await runCli(['pull', 'PROC_custom-path-test'], { cwd: root, stdout: pullOut, stderr: new MemoryWriter() }); | |
| expect(existsSync(join(root, '.method/design/0001-custom-path-test/custom-path-test.md'))).toBe(true); | |
| // Status should reflect it | |
| const statusOut = new MemoryWriter(); | |
| await runCli(['status'], { cwd: root, stdout: statusOut, stderr: new MemoryWriter() }); | |
| expect(statusOut.output).toContain('0001-custom-path-test'); | |
| // Inbox should work against custom paths | |
| const inboxOut = new MemoryWriter(); | |
| const inboxExitCode = await runCli(['inbox', 'custom path test', '--legend', 'PROC'], { cwd: root, stdout: inboxOut, stderr: new MemoryWriter() }); | |
| expect(inboxExitCode).toBe(0); | |
| expect(existsSync(join(root, '.method/backlog/inbox/PROC_custom-path-test.md'))).toBe(true); | |
| // Pull should work against custom paths | |
| const pullOut = new MemoryWriter(); | |
| const pullExitCode = await runCli(['pull', 'PROC_custom-path-test'], { cwd: root, stdout: pullOut, stderr: new MemoryWriter() }); | |
| expect(pullExitCode).toBe(0); | |
| expect(existsSync(join(root, '.method/design/0001-custom-path-test/custom-path-test.md'))).toBe(true); | |
| // Status should reflect it | |
| const statusOut = new MemoryWriter(); | |
| const statusExitCode = await runCli(['status'], { cwd: root, stdout: statusOut, stderr: new MemoryWriter() }); | |
| expect(statusExitCode).toBe(0); | |
| expect(statusOut.output).toContain('0001-custom-path-test'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cli.test.ts` around lines 434 - 447, Capture and assert the exit codes
returned by runCli for the inbox and pull invocations and add a runCli
invocation to exercise the custom tests directory with the method drift flow:
change the two awaited runCli([...], { cwd, stdout, stderr }) calls to store
their return values (e.g., inboxResult and pullResult) and add assertions that
inboxResult.code === 0 and pullResult.code === 0 (or equivalent property
returned by runCli); then invoke runCli for the 'method drift' or appropriate
test runner command pointing at the custom tests folder (the tests:'spec'
config) and assert its exit code is 0 and/or its output contains expected
results to ensure the custom tests directory is actually exercised.
1 participant
Summary
Makes METHOD's workspace directory layout configurable via
.method.json:{ "paths": { "backlog": ".method/backlog", "design": ".method/design", "retro": ".method/retro", "tests": "spec", "graveyard": ".method/graveyard", "method_dir": ".method" } }Defaults match the current layout — zero behavioral change for existing workspaces.
Changes
src/config.ts—PathsSchemawith defaults,DEFAULT_PATHSexportsrc/domain.ts— removedBACKLOG_DIR/DESIGN_DIR/RETRO_DIRconstantssrc/index.ts—Workspace.pathsresolved from config,initWorkspaceaccepts optional pathssrc/drift.ts—detectWorkspaceDriftacceptstestsDirparametersrc/cli.ts— passes config paths toinitWorkspacetests/cli.test.ts— new integration test with custom.method.jsonpathsTest plan