feat(cli): note CRUD/read commands with exit codes and i18n#18
Merged
Conversation
Add the user-facing dennoh CLI commands as thin wrappers over the core: - Data ops: add, update, delete (stdin-piped content supported) - Read ops: get, search, recent, reindex (with --json output) - Shared flag/stdin helpers (src/cli/flags.ts, src/cli/stdin.ts) Conventions applied across all new commands and the main dispatcher: - Exit codes (src/cli/types.ts): EXIT_SUCCESS=0, EXIT_USER_ERROR=1 (bad args / unknown id / validation / unknown command), EXIT_INTERNAL_ERROR=2 (DB open failure / unexpected exception). - Bilingual (ja/en) messages via per-command MESSAGES + resolveLang(), following the status.ts pattern; main usage()/unknown-command localized. stdin piping gates on `process.stdin.isTTY !== true` (a real pipe leaves isTTY undefined, so `=== false` would never trigger the stdin read path). Per-command unit tests cover happy paths, --json, not-found, usage errors, the internal-error (code 2) path, and English localization via DENNOH_LANG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 Walkthroughウォークスルーフラグパーサー・終了コード定数・stdin ヘルパーを共有インフラとして追加し、 変更点CLI コマンド群の実装と統合テスト追加
シーケンス図sequenceDiagram
participant User as ユーザー
participant main as main.ts
participant cmd as コマンド関数
participant flags as フラグ解析
participant config as readConfig
participant db as DB操作
participant mem as メモ操作
User->>main: dennoh subcommand args
main->>main: resolveLang()
main->>cmd: await command(rest, io)
cmd->>flags: takeBooleanFlag/takeOption
cmd->>config: readConfig()
cmd->>db: openDatabase/runMigrations
cmd->>mem: saveMemory/updateMemory
mem-->>cmd: 結果
cmd-->>main: 終了コード
cmd->>db: closeDatabase(finally)
推定コードレビュー工数🎯 4 (Complex) | ⏱️ ~60 minutes 関連する可能性のある Issue
詩
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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: 11
🤖 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/delete.ts`:
- Around line 60-73: The code checks for note existence before deletion, but a
race condition can occur where the note is deleted between the check and the
deleteMemory call, causing deleteMemory to throw a not-found error that gets
caught as a generic internal error. Add a check in the catch block (where error
e is caught) to detect if the error from deleteMemory is a not-found error, and
if so, respond with io.stderr using an appropriate not-found message and return
EXIT_USER_ERROR instead of EXIT_INTERNAL_ERROR, similar to how the pre-check
handles missing notes at the getNoteById verification.
In `@src/cli/commands/recent.ts`:
- Around line 39-54: The `--limit` option validation does not detect when the
flag is provided without a value. Currently when a user runs `dennoh recent
--limit` without a value, the takeOption function returns undefined for
limitArg, which is then treated as if the option was not provided at all,
allowing the code to use DEFAULT_LIMIT instead of failing. You need to
distinguish between two cases: when `--limit` is not provided at all versus when
`--limit` is explicitly provided but lacks a value. Check the return value from
takeOption to determine if the option was explicitly present with no value, and
if so, call io.stderr with an appropriate error message and return
EXIT_USER_ERROR before the code reaches the DEFAULT_LIMIT assignment.
In `@src/cli/commands/search.ts`:
- Around line 37-58: The code does not validate when option tokens like --limit,
--project, or --tag are provided without values. When takeOption consumes a
token but returns undefined for the value, the code currently continues with
defaults or undefined values instead of reporting an error. Add validation after
each takeOption call for --project, --tag, and --limit to check if the option
token was consumed (args array decreased in length) but the value is undefined,
and return EXIT_USER_ERROR with an appropriate error message in those cases.
This ensures users cannot accidentally omit option values (e.g., `search foo
--limit` without a number) without being notified.
In `@src/cli/commands/update.ts`:
- Around line 79-93: There is a TOCTOU race condition between the existence
check using getNoteById and the updateMemory call: if the note is deleted
concurrently between these two operations, updateMemory will throw an exception
that currently falls through to the catch block and returns EXIT_INTERNAL_ERROR
instead of EXIT_USER_ERROR. Fix this by explicitly checking if the exception
caught in the catch block is a "note not found" type error (either by checking
for a dedicated error type thrown by updateMemory or by examining the error
message), and if so, return EXIT_USER_ERROR instead of EXIT_INTERNAL_ERROR to
match the user error contract established at line 82-84 where a missing note is
treated as a user error.
In `@src/cli/flags.ts`:
- Around line 44-50: The flag parsing logic in the block starting with `if (arg
=== name)` unconditionally treats the next token as a value if it exists, which
causes subsequent flags (starting with `-`) to be incorrectly consumed as
values. Add an additional condition to check that the next token does not start
with a hyphen before assigning it as the value. Specifically, when checking `if
(next !== undefined)`, also verify that `next` does not begin with `-` to ensure
that flags are not misinterpreted as values, allowing proper detection of
missing required values and preventing unintended flag consumption.
In `@src/cli/stdin.ts`:
- Around line 4-7: The comment in the stdin.ts file (lines 4-7) describes the
TTY check as process.stdin.isTTY === false, but the actual implementation
approach uses process.stdin.isTTY !== true to handle cases where stdin is
undefined when piped. Update the comment to correctly reference the !== true
pattern instead of === false to ensure future callers understand the correct
guard condition and avoid implementing incorrect TTY checks.
In `@tests/cli/add.test.ts`:
- Around line 1-4: The `DENNOH_TRANSLATE_DISABLE` environment variable is being
set at the module top level in both test files, causing persistent side effects
that break test isolation. In tests/cli/add.test.ts (lines 1-4): remove the
top-level assignment of `process.env.DENNOH_TRANSLATE_DISABLE = "1"` and instead
add a `beforeEach` hook to save the original value and set it to "1", then add
an `afterEach` hook to restore the original value. In tests/cli/update.test.ts
(lines 1-4): apply the same fix by removing the top-level assignment and using
`beforeEach` and `afterEach` hooks to properly scope the environment variable
changes so they do not leak between tests.
In `@tests/cli/get.test.ts`:
- Around line 119-123: The if-else statement in the finally block (lines
119-123) violates the testing guideline that prohibits if statements for control
flow branching in test files. The condition checking whether prev is undefined
to either delete or restore the DENNOH_LANG environment variable should be
refactored to eliminate the branching statement. Replace this if-else structure
with a ternary operator or logical operator approach that accomplishes the same
restoration logic without using an if statement, ensuring the code conforms to
the no-branching style requirement for test files.
In `@tests/cli/reindex.test.ts`:
- Around line 73-77: The test for the reindex command summary in the Japanese
language configuration is incomplete and does not verify the translation errors
count, which is part of the command's public output contract. Add an additional
expect statement that explicitly verifies the translation errors message (翻訳エラー
0 件) is present in the stdout output, similar to the existing checks for the
reindex message and error count. This ensures regressions in the
translationErrors output are caught by the test.
In `@tests/cli/search.test.ts`:
- Around line 1-4: The environment variable DENNOH_TRANSLATE_DISABLE is being
set at module scope, which causes it to persist across test suites and creates
test order dependencies. Move this environment variable assignment into a
beforeEach hook where you set it, and add a corresponding afterEach hook that
restores the original value (or deletes it if it didn't exist before). This
ensures proper test isolation and prevents side effects from affecting other
test suites running in the same process.
- Around line 61-71: The test "prints one line per hit in default form" has weak
assertions that only check if the ID is contained in stdout, failing to validate
the core behavior stated in the test name. Add explicit assertions to verify
that stdout contains exactly one line of output and that the line is properly
formatted with the expected content for the single hit. Use string manipulation
or regex to count and validate the output lines, ensuring the test properly
catches regressions where the formatting or line count changes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e0a81205-65b4-4e95-8b55-1a449cb86710
📒 Files selected for processing (20)
.tmp/tasks.mdsrc/cli/commands/add.tssrc/cli/commands/delete.tssrc/cli/commands/get.tssrc/cli/commands/recent.tssrc/cli/commands/reindex.tssrc/cli/commands/search.tssrc/cli/commands/update.tssrc/cli/flags.tssrc/cli/index.tssrc/cli/main.tssrc/cli/stdin.tssrc/cli/types.tstests/cli/add.test.tstests/cli/delete.test.tstests/cli/get.test.tstests/cli/recent.test.tstests/cli/reindex.test.tstests/cli/search.test.tstests/cli/update.test.ts
AI Agents prompts に基づく自動修正 - flags.ts: takeOption が `-` 始まりの次トークンを値として誤消費しないように し、`present` を返してオプション値欠落を呼び出し側が検出可能に - search/recent: オプション値欠落（例 `search foo --limit`）を EXIT_USER_ERROR で明示的に拒否 - update/delete: 事前存在チェック後の TOCTOU race による not-found を catch 側で EXIT_USER_ERROR にマップ（isNotFoundError を types.ts に追加） - stdin.ts: TTY 判定コメントを `!== true` ベースに修正 - tests: DENNOH_TRANSLATE_DISABLE をモジュール先頭代入から beforeEach/afterEach によるスコープ化＋復元へ（add/update/search）。finally 内の if を三項+Reflect へ。 reindex の翻訳エラー件数アサーション追加、search の「1ヒット=1行」検証強化、 オプション値欠落のテスト追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/cli/recent.test.ts (1)
1-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win環境変数のモジュールスコープ固定をやめ、各テストで復元してください。
Line 1-3 で
process.env.DENNOH_TRANSLATE_DISABLEをモジュールスコープで設定しており、同一プロセス内の他スイートへ副作用が漏れます。共有原因は「グローバル状態をセットしたまま復元していないこと」です。
tests/cli/recent.test.ts#L1-L3:beforeEachで元値を退避して設定し、afterEachで元値復元（未定義なら削除）してください。tests/cli/reindex.test.ts#L1-L3: 同じくbeforeEach/afterEachでスコープ化し、環境変数のリークを防いでください。As per coding guidelines,
tests/**/*.tsでは「テストの独立性: テスト間に依存がないか。命名規約が一貫しているか。」を重点確認します。🤖 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 `@tests/cli/recent.test.ts` around lines 1 - 3, The environment variable DENNOH_TRANSLATE_DISABLE is being set at module scope, causing side effects to leak to other test suites in the same process. For both tests/cli/recent.test.ts (lines 1-3, anchor) and tests/cli/reindex.test.ts (lines 1-3, sibling), move the module-level process.env.DENNOH_TRANSLATE_DISABLE assignment into a beforeEach hook where you first save the original value of the environment variable, then set it to "1". In the corresponding afterEach hook, restore the original value or delete the property if it was undefined. This ensures test isolation and prevents environment variable leaks between test suites.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.
Duplicate comments:
In `@tests/cli/recent.test.ts`:
- Around line 1-3: The environment variable DENNOH_TRANSLATE_DISABLE is being
set at module scope, causing side effects to leak to other test suites in the
same process. For both tests/cli/recent.test.ts (lines 1-3, anchor) and
tests/cli/reindex.test.ts (lines 1-3, sibling), move the module-level
process.env.DENNOH_TRANSLATE_DISABLE assignment into a beforeEach hook where you
first save the original value of the environment variable, then set it to "1".
In the corresponding afterEach hook, restore the original value or delete the
property if it was undefined. This ensures test isolation and prevents
environment variable leaks between test suites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d594c10e-7297-4a3e-9495-cd085b7e1239
📒 Files selected for processing (13)
src/cli/commands/delete.tssrc/cli/commands/recent.tssrc/cli/commands/search.tssrc/cli/commands/update.tssrc/cli/flags.tssrc/cli/stdin.tssrc/cli/types.tstests/cli/add.test.tstests/cli/get.test.tstests/cli/recent.test.tstests/cli/reindex.test.tstests/cli/search.test.tstests/cli/update.test.ts
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.
概要
ユーザー向けの
dennohCLI コマンド群を、コア関数の薄いラッパーとして実装しました（T11 系 + T11.14/T11.15、T12.2）。実装コマンド
すべて
(args: string[], io: CliIO) => Promise<number>シグネチャ。add "<text>"update <id> "<text>"delete <id>get <id> [--json]search "<q>" [--project X] [--tag Y] [--limit N] [--json]recent [--limit N] [--json]reindex共通ヘルパー: src/cli/flags.ts（
--flag/--opt value/--opt=value解析）、src/cli/stdin.ts。規約の適用
EXIT_SUCCESS=0/EXIT_USER_ERROR=1（引数不足・ID不存在・バリデーション・不明コマンド）/EXIT_INTERNAL_ERROR=2（DB接続失敗・予期しない例外）。update/deleteはgetNoteByIdで事前存在チェックし、ID不存在を確実に 1 に分類。status.tsパターンに倣い各コマンド内MESSAGES: Record<Lang, {...}>+resolveLang()で日英対応。main.tsの usage / 不明コマンドもバイリンガル化。注意点・設計判断
process.stdin.isTTY !== true。実パイプではisTTYがundefinedになるため、仕様文言の=== falseではパイプ入力を拾えない（意図に合わせて修正）。src/i18n/ja.ts/en.tsフラットキー）は未導入で、コマンド単位のMESSAGES定数で実装。history/restore/serve/configは英語のまま（未 localize）。→ T12.1/T12.3/T12.4 は別タスクとして.tmp/tasks.mdに残置。searchのデフォルト出力は1行1件。スニペットに含まれる改行を空白へ正規化して1行を維持（<mark>タグは無加工）。テスト
各コマンドのユニットテストを追加（happy path /
--json/ not-found / usage / 終了コード2 /DENNOH_LANG=enローカライズ）。bun test: 433 pass / 0 failbun run lint/bun run typecheck: クリーン🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
add / update / delete / get / search / recent / reindexを追加get / recent / searchは JSON 形式でも出力可能（プロジェクト・タグで検索/絞り込み）改善
テスト
ドキュメント