Conversation
- Lang union ('ja' | 'en'), Config interface, DEFAULT_CONFIG export
- readConfig / writeConfig: ~/Library/Application Support/dennoh/config.json,
recursive mkdir on write, schema validation on read (object shape,
vaultPath: string, lang in {ja,en})
- resolveLang: DENNOH_LANG env > config file lang > DEFAULT_CONFIG.lang
- tests/config.test.ts: 10 cases covering round-trip, dir creation,
missing-file/invalid-lang/invalid-vaultPath/non-object errors, env
precedence, invalid env fallback, missing config fallback, full
priority chain
- tests/smoke.test.ts: include @/config alongside other subpackages
Tests use spyOn(os, "homedir") to redirect to a temp dir, so real
user config is never touched during tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CLI infrastructure
- src/cli/types.ts: CliIO interface (stdout/stderr writers)
- src/cli/main.ts: argv routing for --help / --version / init /
config / serve, async, gated by import.meta.main so tests can
import without exiting
config commands (T1.3)
- configGet: reads vaultPath / lang from disk; lang respects
DENNOH_LANG env via resolveLang
- configSet: validates lang in {ja,en} and rejects empty vaultPath
before writing; requires existing config (caller must run init)
- configList: emits key=value lines; annotates lang with
"(from DENNOH_LANG)" when the env override is active
- exit 1 + stderr message for unknown key / invalid value /
missing config / usage errors
init command (T1.4)
- Composable steps: expandTilde, resolveVaultPath, ensureVaultDirs,
initGitRepo, updateGitignore, detectCloudSync, formatCloudWarning
- Prompt fn is injected (defaultPromptVaultPath uses node:readline/
promises) so tests can drive it deterministically
- mkdir -p vault + vault/.dennoh; skip git init when .git/ exists;
append .dennoh/ to .gitignore without duplicating .dennoh /
.dennoh/ / .dennoh/*; write { vaultPath, lang: "ja" } config
- Cloud sync detection for iCloud Drive, Dropbox, ~/OneDrive, and
~/Library/CloudStorage/OneDrive*; service-specific advisory text
goes to stderr without blocking init
Tests (37 new cases)
- tests/cli/config.test.ts: set->get round-trip, list formatting,
env annotation, every error path
- tests/cli/init.test.ts: full happy path, ~ expansion, whitespace
trim, pre-existing .git preservation, .gitignore append /
dedupe / bare-form recognition, four cloud paths + non-cloud
baseline, validation failures
.tmp/tasks.md: mark T1.1-T1.5 as done.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughこのPRはCLIにvault初期化(init)と設定管理(config)機能、設定の永続化と言語解決、CLIエントリポイント、およびそれらを検証するテスト群を追加します。 ChangesCLI と設定管理の統合実装
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/cli/commands/config.ts`:
- Around line 61-63: writeConfig(next) can throw and currently escapes the CLI's
exit-code contract; wrap the call to writeConfig(next) inside a try/catch (for
both places where next.lang = value; writeConfig(next); return 0; appears) and
on error write a clear message to stderr (e.g., console.error or
process.stderr.write) and return 1 so the CLI exits with a consistent non-zero
code instead of bubbling an uncaught exception.
- Around line 15-23: When handling the "lang" branch, don't force readConfig()
before honoring the DENNOH_LANG environment variable; instead, first check
whether process.env.DENNOH_LANG is present and valid (or call a helper that
validates it) and if so skip readConfig(), write resolveLang() to stdout and
return 0; only call readConfig() (and handle its error with io.stderr + return
1) when the env var is absent or invalid. Update the key === "lang" block to
prefer the env var, then fall back to readConfig() and finally
io.stdout(`${resolveLang()}\n`) with the same exit codes.
In `@src/cli/commands/init.ts`:
- Around line 50-53: The .git existence check uses fs.existsSync(gitDir) which
returns true for files as well, causing initialization to be skipped
incorrectly; change the check to verify that gitDir is a directory (use
fs.lstatSync or fs.statSync and call isDirectory()) before treating it as an
existing repo so the early return that logs "Using existing git repository at
${vaultPath}." only runs when gitDir is actually a directory.
- Around line 59-62: The error construction using implicit fallback must be
replaced with an explicit branch: in the git init failure handling (the block
that checks result.exitCode in src/cli/commands/init.ts), call new
TextDecoder().decode(result.stderr), assign to a variable (e.g., stderr),
compute trimmed = stderr.trim(), then if trimmed.length > 0 use that in the
thrown Error message and otherwise use `exit code ${result.exitCode}` (or throw
a distinct error when both are absent); do this inside the same failure branch
that currently references result to keep behavior unchanged.
In `@tests/cli/init.test.ts`:
- Around line 154-184: The cloud-warning tests only assert stderr text but not
the process exit status; update each test that calls initCommand({ io, ... })
(e.g., the tests using makeIO, stderr(), and initCommand) to capture the
returned result from initCommand and assert the exit code is 0 (e.g., const
result = await initCommand({...}); expect(result.code).toBe(0)); keep the
existing stderr() assertions as-is while adding the code === 0 assertion in each
cloud-related test.
In `@tests/config.test.ts`:
- Around line 25-29: Extract the environment restore logic into a shared helper
(e.g., restoreDennohLang) and call that from both tests instead of leaving an if
in each test; the helper should accept the captured originalLang and perform the
proper action on process.env.DENNOH_LANG (delete when originalLang is undefined,
otherwise restore the value), replacing the inline if block that references
originalLang and process.env.DENNOH_LANG in the tests.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec32a991-4f0d-422c-803d-a3daebca89f3
📒 Files selected for processing (11)
.tmp/tasks.mdsrc/cli/commands/config.tssrc/cli/commands/init.tssrc/cli/index.tssrc/cli/main.tssrc/cli/types.tssrc/config/index.tstests/cli/config.test.tstests/cli/init.test.tstests/config.test.tstests/smoke.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 6 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 4 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/commands/config.ts (1)
15-29: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
lang取得ロジックが冗長で、設定を二重に読み込む可能性があります。現在のコード:
- 16-20行目:
envLangが有効な場合、resolveLang()を呼び出していますが、resolveLang()内でenvLangを再度検証するため冗長です。- 21-28行目:
envLangが無効な場合、22行目でreadConfig()を呼び出した後、27行目でresolveLang()を呼び出すと、resolveLang()内で再度readConfig()が実行されます。より明確で効率的な実装:
♻️ 推奨リファクタリング
if (key === "lang") { const envLang = process.env.DENNOH_LANG; if (envLang === "ja" || envLang === "en") { - io.stdout(`${resolveLang()}\n`); + io.stdout(`${envLang}\n`); return 0; } try { - readConfig(); + const cfg = readConfig(); + io.stdout(`${cfg.lang}\n`); + return 0; } catch (e) { io.stderr(`${readError(e)}\n`); return 1; } - io.stdout(`${resolveLang()}\n`); - return 0; }🤖 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/cli/commands/config.ts` around lines 15 - 29, When handling the "lang" key, avoid calling resolveLang() twice and prevent double-reading config: if process.env.DENNOH_LANG is "ja" or "en", write that value directly to io.stdout and return; otherwise call readConfig() once inside the try/catch and pass the loaded config into resolveLang (or add an overload/variant of resolveLang that accepts the config) so resolveLang does not call readConfig() again; ensure you still catch and log readConfig() errors via readError(e) and return the proper exit code.
🤖 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.
Outside diff comments:
In `@src/cli/commands/config.ts`:
- Around line 15-29: When handling the "lang" key, avoid calling resolveLang()
twice and prevent double-reading config: if process.env.DENNOH_LANG is "ja" or
"en", write that value directly to io.stdout and return; otherwise call
readConfig() once inside the try/catch and pass the loaded config into
resolveLang (or add an overload/variant of resolveLang that accepts the config)
so resolveLang does not call readConfig() again; ensure you still catch and log
readConfig() errors via readError(e) and return the proper exit code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 04a33e85-2caa-4f1c-a365-c4b18b33c44d
📒 Files selected for processing (4)
src/cli/commands/config.tssrc/cli/commands/init.tstests/cli/init.test.tstests/config.test.ts
When DENNOH_LANG is invalid the old path called readConfig() to verify existence, then resolveLang() — which calls readConfig() again. Read once inside the try/catch and use the loaded config directly. Drop the now-unused resolveLang import. When DENNOH_LANG is valid, output it directly instead of going through resolveLang (which only re-evaluates the same env var). Behavior is identical; existing tests cover both branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements T1 (Configuration & Initialization) from
.tmp/tasks.md:~/Library/Application Support/dennoh/config.jsondennoh initinteractive setup (vault +.dennoh/+git init+.gitignore+ config)dennoh config get / set / listwithDENNOH_LANGenv overrideAll five subtasks (T1.1〜T1.5) are checked off in
.tmp/tasks.md.Architecture
Config module (
src/config/index.ts)Lang = 'ja' | 'en',Config = { vaultPath, lang },DEFAULT_CONFIGreadConfigvalidates JSON shape and rejects unknown lang or non-string vaultPathwriteConfigcreates the parent directory recursivelyresolveLangpriority:DENNOH_LANGenv > config file > default'ja'CLI shell (
src/cli/)types.ts:CliIOinterface so handlers don't touchprocess.stdout/process.exitdirectlymain.ts: argv routing, gated byimport.meta.mainso tests can importmainsafelycommands/config.ts:configGet/configSet/configList/configCommandcommands/init.ts: composable steps (expandTilde,resolveVaultPath,ensureVaultDirs,initGitRepo,updateGitignore,detectCloudSync,formatCloudWarning) +initCommandthat sequences them with an injectable promptWhy dependency injection
configCommand(args, io)returns an exit code instead of callingprocess.exitso tests can capture stdout/stderr without subprocess overheadinitCommand({ io, promptVaultPath })lets tests skip interactive input entirely; production wires updefaultPromptVaultPath(node:readline/promises)Cloud sync detection
detectCloudSyncrecognizes:~/Library/Mobile Documents/*(iCloud Drive)~/Dropbox/*~/OneDrive/*~/Library/CloudStorage/OneDrive*/*For each,
formatCloudWarningwrites a service-specific exclusion tip (.nosync/.dropboxignore/ OneDrive client) to stderr — informational, never blocking.Test coverage
55 → 92 tests total (+37 new):
tests/config.test.ts(10): round-trip, recursive mkdir, every read-error path, env precedencetests/cli/config.test.ts(18): set→get round-trip, list formatting + env annotation, every error path, routingtests/cli/init.test.ts(17): full happy path,~expansion, whitespace trim, existing.gitpreservation,.gitignoreappend / dedupe / bare-form, four cloud paths + non-cloud baseline, validation failuresAll tests redirect
os.homedir()to a temp directory viaspyOn, so the developer's real~/Library/Application Support/dennoh/config.jsonis never touched.Test plan
bun installbun run lint— cleanbun run typecheck— cleanbun run test— 92 passbun run build— producesdist/cli.jsbun dist/cli.js --version→0.0.0echo /tmp/dennoh-vault | bun dist/cli.js initcreates vault +.dennoh/+.git/+.gitignorebun dist/cli.js config listshows the new configDENNOH_LANG=en bun dist/cli.js config listannotateslang=en (from DENNOH_LANG)rm -rf /tmp/dennoh-vault ~/Library/Application\ Support/dennoh/config.json🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Config
Tests
Chores