feat(cli): add workspace initialization and safe startup - #15
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request adds ChangesInit command and safe startup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant runInit
participant FileSystem
participant MCPClient
participant MCPServer
CLI->>runInit: invoke initialization
runInit->>FileSystem: resolve workspace and configuration
runInit->>MCPClient: import registrations
runInit->>FileSystem: write configuration atomically
runInit->>MCPClient: register mottainai
runInit->>MCPServer: run doctor and MCP handshake
runInit-->>CLI: return initialization summary
CLI-->>CLI: print JSON or human-readable output
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
README.md (1)
128-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded version pin in two places.
mottainai@0.1.1is hardcoded in both theclaude mcp addandcodex mcp addexamples. Each future version bump needs a manual edit here in addition topackage.jsonandCHANGELOG.md. Consider adding a short maintenance note (e.g., in CHANGELOG.md or a release checklist) to keep these three references in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 128 - 135, Add a maintenance note in CHANGELOG.md or create a release checklist documentation to remind developers that the mottainai version must be synchronized across three locations during version bumps: the package.json version field, and both the claude mcp add and codex mcp add command examples in README.md. This prevents the hardcoded version pins from drifting out of sync.CHANGELOG.md (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
[0.1.1]link reference definition.The file defines a link reference for
[0.1.0]at the bottom (line 78 context). No matching[0.1.1]reference definition appears for the new## [0.1.1] - in developmentheading. Add a[0.1.1]: ...reference line to keep the version heading consistent with the existing convention.📝 Proposed addition near the bottom of the file
[Unreleased]: https://github.com/yohn-jp/mottainai/compare/main...HEAD +[0.1.1]: https://github.com/yohn-jp/mottainai/commits/main [0.1.0]: https://github.com/yohn-jp/mottainai/commits/main🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 16, Add a link reference definition for the new version heading at the bottom of the CHANGELOG.md file to match the existing convention. Locate the existing [0.1.0] link reference definition near the end of the file and add a corresponding [0.1.1] reference definition on a new line using the same format and URL pattern. This ensures the markdown link reference for the new [0.1.1] heading is properly defined, consistent with how [0.1.0] is already handled in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/init.ts`:
- Around line 491-493: Update the validation error thrown in the init
argument-checking branch to begin with lowercase “interactive” while preserving
the rest of the message and behavior.
- Around line 178-180: Update resolveConfiguration to use resolveConfigPath so
explicit paths and the MOTTAINAI_CONFIG environment variable are resolved
consistently, while retaining the default workspace/mottainai.config.json
fallback when neither is provided.
- Line 607: Update the next-step command in the summary generation around
summary.dry_run to preserve the selected configuration context: append --config
with summary.configuration when provided, or direct users to change into
summary.workspace before running doctor. Ensure the displayed command inspects
the configuration generated by the current run.
- Around line 304-312: Add bounded timeouts to the subprocess options in
runClientList and registerClient, and enforce a single timeout covering the
complete MCP handshake around client.connect and client.listTools. When any
operation times out, surface the failure through warnings and the corresponding
InitHandshakeResult while preserving cleanup in the finally block.
- Around line 147-149: Update the comment in the catch block of the
initialization flow to Japanese, preserving its explanation that PATH entries
are user input and may disappear during initialization; keep the catch behavior
unchanged and ensure the comment describes only why the empty catch is safe.
- Around line 140-152: Update commandPath to resolve Windows executables by
trying the command itself plus extensions from the environment’s PATHEXT when
process.platform is "win32", including .exe, .cmd, and .bat candidates. Preserve
the existing PATH traversal, file validation, and non-Windows behavior.
- Around line 228-280: Update importedRegistration and its URL validation so
remote credential references in auth and headersFromEnv are preserved only for
HTTPS URLs. Reject non-HTTPS URLs before constructing the imported registration,
while retaining the existing safeRemoteUrl checks for credentials and sensitive
query parameters.
In `@src/mcp-cli.test.ts`:
- Around line 251-253: Update the TTY startup branch in the relevant startup
logic of src/index.ts to write the missing-configuration guidance to stderr
instead of stdout, while preserving the existing message. Extend the tests in
the TTY startup path around the existing server.stdout/server.stderr assertions
to exercise TTY stdin and stdout, verify stdout remains empty, and assert both
guidance messages appear on stderr.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 16: Add a link reference definition for the new version heading at the
bottom of the CHANGELOG.md file to match the existing convention. Locate the
existing [0.1.0] link reference definition near the end of the file and add a
corresponding [0.1.1] reference definition on a new line using the same format
and URL pattern. This ensures the markdown link reference for the new [0.1.1]
heading is properly defined, consistent with how [0.1.0] is already handled in
the file.
In `@README.md`:
- Around line 128-135: Add a maintenance note in CHANGELOG.md or create a
release checklist documentation to remind developers that the mottainai version
must be synchronized across three locations during version bumps: the
package.json version field, and both the claude mcp add and codex mcp add
command examples in README.md. This prevents the hardcoded version pins from
drifting out of sync.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 821529f8-92e9-42ba-a612-0f2b89e88990
📒 Files selected for processing (8)
CHANGELOG.mdREADME.mdpackage.jsonsrc/cli.tssrc/index.tssrc/init.test.tssrc/init.tssrc/mcp-cli.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/init.ts (1)
281-300: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Moderate
Reject unsafe redirects for imported credential headers.
headersFromEnvis retained for HTTPS URLs and passed toStreamableHTTPClientTransportas request headers. Its defaultfetchhandling can forward custom secret headers across redirects, including HTTP or another authority. Use a redirect-rejecting fetch wrapper or transport policy. The OAuth profile is not forwarded as a token on this path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/init.ts` around lines 281 - 300, The HTTPS import path around headersFromEnv must prevent credential headers from being forwarded through redirects to HTTP or another authority. Update the transport setup that consumes imported.headersFromEnv, using a redirect-rejecting fetch wrapper or equivalent transport policy, while preserving the existing OAuth profile handling and header validation.
🧹 Nitpick comments (1)
src/init.test.ts (1)
74-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse complete variable names.
binandpreviousPathExtadd new abbreviations.
src/init.test.ts#L74-L93: RenamebintobinaryDirectoryandpreviousPathExttopreviousPathExtensions.src/init.test.ts#L206-L236: RenamebintobinaryDirectory.As per coding guidelines, use complete words for names and do not add abbreviations such as
cfg,impl, orres.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/init.test.ts` around lines 74 - 93, Use complete variable names in src/init.test.ts: rename bin to binaryDirectory and previousPathExt to previousPathExtensions in lines 74-93, and rename bin to binaryDirectory in lines 206-236, updating all references at each site.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/init.test.ts`:
- Around line 170-175: Update the test fixture’s httpCredentials configuration
to preserve the OAuth profile reference while still exercising sanitization at
the referenced credential field near the later assertion. Ensure the init
contract retains auth type “oauth” and profile “http” so imported servers can
authenticate, rather than removing or replacing the profile-only reference.
In `@src/init.ts`:
- Around line 323-334: Update src/init.ts lines 323-334 in runClientList to
return an explicit successful-list indicator, distinguishing a zero-result
successful listing from non-zero or otherwise failed commands. Update
src/init.ts lines 424-460 to abort registration when listing failed or timed
out, and only register when a successful list conclusively confirms that
mottainai is absent.
- Around line 143-160: Update src/init.ts:143-160, src/init.ts:323-328, and
src/init.ts:449-453 so Windows .cmd and .bat results from commandPath are
executed through one shared ComSpec launcher with fixed arguments, passing
command paths and user-controlled values only as arguments. Apply this launcher
in both runClientList and registerClient, and ensure a failed runClientList
stops registration instead of continuing.
---
Outside diff comments:
In `@src/init.ts`:
- Around line 281-300: The HTTPS import path around headersFromEnv must prevent
credential headers from being forwarded through redirects to HTTP or another
authority. Update the transport setup that consumes imported.headersFromEnv,
using a redirect-rejecting fetch wrapper or equivalent transport policy, while
preserving the existing OAuth profile handling and header validation.
---
Nitpick comments:
In `@src/init.test.ts`:
- Around line 74-93: Use complete variable names in src/init.test.ts: rename bin
to binaryDirectory and previousPathExt to previousPathExtensions in lines 74-93,
and rename bin to binaryDirectory in lines 206-236, updating all references at
each site.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f06c388-c98f-4a6b-89ed-6834a8957f04
📒 Files selected for processing (3)
src/index.tssrc/init.test.tssrc/init.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 81: Update the [0.1.1] changelog reference so it no longer points to the
generic commits/main URL; remove the link until a 0.1.1 tag exists, or replace
it with a version-specific tag or comparison URL.
In `@src/upstream.test.ts`:
- Around line 32-48: Update the test around fetchWithoutRedirects to assert that
the Authorization header is forwarded in requestInit. To verify redirect
rejection as the test name claims, make the mocked fetch return a redirect
response and assert the call rejects; otherwise rename the test to describe
request-option propagation and retain the non-redirect response.
In `@src/upstream.ts`:
- Around line 195-197: Update the transport creation flow around
StreamableHTTPClientTransport to reject credentialed upstreams when
headersFromEnv is defined unless endpoint.protocol is "https:". Keep the
existing loopback HTTP broker path separate, and ensure remote credentials are
never passed to an HTTP endpoint; preserve the current transport configuration
for allowed cases.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b68cd629-76b5-49b2-9524-4b969a66eca6
📒 Files selected for processing (5)
CHANGELOG.mdsrc/init.test.tssrc/init.tssrc/upstream.test.tssrc/upstream.ts
Summary
Implement the 0.1.1 first-run onboarding contract from Issue #14. New users can initialize a workspace without hand-writing
mottainai.config.json, while the bare executable remains an MCP stdio server entry point.Linked issue
Closes #14
Scope
Included
mottainai initwith workspace detection, personal/project scope, upstream import/discovery, client registration, non-interactive options, dry-run, JSON output, force/backup, and atomic configuration writes.initialize/tools/listhandshake after initialization.Excluded
Implementation
src/init.tsowns initialization parsing, workspace/Git detection, config generation, safe JSON client import, official Claude Code/Codex registration commands, atomic writes, backups, diagnostics, and handshake verification..git/info/excludeand never edits.gitignore.mottainai@0.1.1;--latestis explicit opt-in.Behavioral changes
npx -y mottainai initstarts workspace setup.npx -y mottainaistarts only the MCP server. Without configuration, TTY users receive an init hint and non-TTY users receive the same diagnostic on stderr with empty stdout.--yesprovides safe non-interactive defaults;--dry-runwrites no configuration or Git exclusion changes;--jsonemits one JSON document.--forceis supplied, and forced replacement creates a backup.Validation
pnpm run typecheckpnpm test(467/467), focused init/CLI tests (16/16)pnpm run buildnpm_config_cache=/tmp/mottainai-npm-cache npm pack --dry-run --json(mottainai@0.1.1)node --test scripts/governance.test.mjs(18/18)initializeandtools/listsucceeded with 19 toolsRisks
mottainairegistrations are detected and left unchanged.Breaking changes
No. Existing valid configurations remain readable. The missing-configuration path now exits with a guided diagnostic instead of exposing only a raw filesystem error.
Migration / compatibility
No migration is required. Existing configurations continue through the existing loader. New workspaces use version 2 with
gateway.workspaceRootset to a portable relative path. Runmottainai initonly for a workspace without a configuration, or use--forcewhen replacement is intentional.Security impact
Initialization does not copy literal tokens, authorization headers, cookies, passwords, or credential-bearing URLs into
mottainai.config.json. Remote authentication uses environment-variable names or OAuth profiles, and client configuration changes use official CLI commands rather than direct file edits.Review focus
init --forceand personal Git exclusions.--latestclient registration commands.