feat: API キー管理 UX 改善 — 強調表示・注釈・refresh コマンド#92
Merged
Merged
Conversation
📝 WalkthroughWalkthroughCLI に API キーのリフレッシュサブコマンドを追加し、作成/リフレッシュ時にキーを目立たせて表示する出力を導入。一覧・取得時はマスク済み 変更内容
シーケンス図sequenceDiagram
participant User as ユーザー
participant CLI as CLI コマンド
participant API as API サーバー
participant Output as 出力モジュール
participant Config as 設定ファイル
User->>CLI: api-keys refresh {keyId} [--save]
CLI->>API: POST /me/api-keys/{keyId}/refresh
API-->>CLI: { key: "gdb_...", keyPrefix: "...", ... }
alt --save オプション有り
CLI->>Config: saveConfig(profile, key)
Config-->>CLI: 保存完了
CLI->>Output: console.error "API key saved..."
else --save オプション無し
CLI->>Output: printApiKeyBox(key)
Output-->>CLI: 表示完了
CLI->>Output: console.error "API key refreshed."
end
見積もり コード レビュー難易度🎯 4 (Complex) | ⏱️ ~45 分 詩
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 11-13: Update the three new CHANGELOG entries so their PR-number
suffix matches this PR (change `(`#91`)` to `(`#92``) for the entries describing `me
api-keys refresh` / `admin api-keys refresh`, `me api-keys create` / `admin
api-keys create`, and `me api-keys list` / `admin api-keys list`; if you still
want to reference the issue `#91`, move that mention into the body of the entry
(not the suffix) to comply with the CHANGELOG suffix convention.
In `@src/commands/admin/api-keys.ts`:
- Around line 50-67: handleSaveKey currently assumes saveConfig succeeds and
will lose the newly returned key if saving fails; wrap the saveConfig call in a
try/catch inside handleSaveKey (and apply the same change to the identical
helper in src/commands/me-api-keys.ts), on error call printApiKeyBox(key) to
display the new key as a fallback, set process.exitCode = 1 and return false (or
propagate the existing return convention) so the command exits non‑zero, and
otherwise continue to set config.apiKey, saveConfig and print the success
message as before; reference functions/idents: handleSaveKey, saveConfig,
loadConfig, printApiKeyBox.
- Around line 69-85: The branch in showKeyResult (function showKeyResult)
currently treats a missing data.key as a warning and continues; change it so
that when save is false and data.key is undefined/null you call an error path
(use the existing error logging/error helper used elsewhere) and exit with
non-zero status (or throw) so the process is treated as a failure and callers do
not display a success banner; keep the save=true path using handleSaveKey
unchanged and when key exists use printApiKeyBox as before. Apply the same
change to the sibling showKeyResult in src/commands/me-api-keys.ts to ensure
refresh/create failures are treated as errors rather than warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95b84943-e1f7-4f28-a7c2-db8f011afa3e
📒 Files selected for processing (6)
CHANGELOG.mdsrc/commands/admin/api-keys.tssrc/commands/me-api-keys.tssrc/output.tstests/admin-api-keys.test.tstests/me-api-keys.test.ts
- me api-keys refresh / admin api-keys refresh コマンドを追加 (キーローテーション) - create/refresh 時に API キー値をボックス囲みで強調表示 - list 出力末尾に「API キー値は作成時/リフレッシュ時にのみ表示」注釈を追加 - printApiKeyBox ヘルパーを output.ts に追加 - handleSaveKey/showKeyResult を共通パターンとして抽出
- list/get レスポンスから keyPrefix フィールドを除去 - マスクされた key を表示から除去 - cleanApiKeyData ヘルパーを me-api-keys.ts / admin/api-keys.ts に追加
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/commands/admin/api-keys.ts (2)
60-77:⚠️ Potential issue | 🟠 Major
saveConfig失敗時に新しいキー値が消失します。
saveConfig()が例外をスローした場合、create/refresh がサーバー側で成功しているにもかかわらず、新しいkey値が一度も表示されないまま処理が終了します。特に refresh では旧キーが既に無効化されているため、回復不能な状態になります。
saveConfigを try/catch でラップし、失敗時はprintApiKeyBox(key)でフォールバック表示してからprocess.exitCode = 1を設定してください。src/commands/me-api-keys.tsの同名ヘルパーにも同様の修正が必要です。,
🛡️ 修正案
function handleSaveKey( data: Record<string, unknown>, cmd: Command, ): boolean { const globalOpts = resolveOptions(cmd); const key = data.key as string | undefined; if (!key) { printError("Response missing key. API key was created, but it could not be saved."); process.exitCode = 1; return false; } - const config = loadConfig(globalOpts.profile); - config.apiKey = key; - saveConfig(config, globalOpts.profile); - console.error("API key saved to config. X-Api-Key header will be sent automatically."); - return true; + try { + const config = loadConfig(globalOpts.profile); + config.apiKey = key; + saveConfig(config, globalOpts.profile); + console.error("API key saved to config. X-Api-Key header will be sent automatically."); + return true; + } catch (error) { + printError(`Failed to save API key to config: ${error instanceof Error ? error.message : String(error)}`); + printApiKeyBox(key); + process.exitCode = 1; + return false; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/admin/api-keys.ts` around lines 60 - 77, Wrap the saveConfig call inside handleSaveKey in a try/catch so that if saveConfig(config, globalOpts.profile) throws you fallback to showing the new API key via printApiKeyBox(key), set process.exitCode = 1, and still return true (so the user sees the key even when persisting fails); apply the same pattern to the equivalent helper in src/commands/me-api-keys.ts. Ensure you catch the exception from saveConfig (but do not swallow other unrelated errors), call printApiKeyBox(key) in the catch block, set process.exitCode = 1, and avoid losing the newly created key in failure scenarios.
79-95:⚠️ Potential issue | 🔴 Critical
key欠落を警告のまま成功扱いにしています。
--saveなしの分岐でdata.keyが欠けている場合、警告を出力するだけでprocess.exitCodeを設定せず、処理が成功として続行されます。create/refresh でこの値が返されない時点で実質失敗であり、特に refresh では旧キーが既に無効化されているため致命的です。
keyが存在しない場合はprintError+process.exitCode = 1を設定し、呼び出し元で成功メッセージを出さないようにしてください。,
🛡️ 修正案
function showKeyResult( data: Record<string, unknown>, save: boolean, cmd: Command, -): void { - if (save) { - handleSaveKey(data, cmd); - } else { - const key = data.key as string | undefined; - if (key) { - printApiKeyBox(key); - } else { - printWarning("Save the API key now — it will not be shown again. Use --save to store it automatically."); - } - } +): boolean { + const key = data.key as string | undefined; + if (!key) { + printError("Response missing key. The new API key value was not returned."); + process.exitCode = 1; + return false; + } + if (save) return handleSaveKey(data, cmd); + printApiKeyBox(key); + return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/admin/api-keys.ts` around lines 79 - 95, The function showKeyResult treats a missing data.key as a benign warning; instead, when data.key is undefined in the non-save branch, call printError with a clear message, set process.exitCode = 1, and halt further success handling (e.g., throw a short Error after setting the exit code) so the caller cannot continue to print success; update the else branch that currently calls printWarning to use printError + process.exitCode = 1 and stop execution (refer to showKeyResult, data.key, printWarning/printError, and process.exitCode).
🤖 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/commands/admin/api-keys.ts`:
- Around line 200-203: showKeyResult may fail and returns a falsy value, yet the
success log "API key created." is always printed; update the flow in the block
where showKeyResult(data, !!opts.save, cmd) is called to capture its return
value (e.g. const ok = showKeyResult(...)) and only call console.error("API key
created.") when that return is truthy, leaving outputResponse(response, format)
unchanged; ensure you reference showKeyResult, outputResponse, response, format
and opts.save when making this conditional change so failure cases do not emit a
misleading success message.
- Around line 227-248: The refresh command currently always prints "API key
refreshed." even if showKeyResult (which may throw or fail) did not succeed;
wrap the post-request handling so that you call showKeyResult(...) and
outputResponse(...) inside a try/catch and only print the success message when
those calls complete without error; on catch rethrow or let withErrorHandler
handle the error (do not print the success message), referencing the refresh
command handler, showKeyResult, outputResponse, and the client.rawRequest call
that obtains the response.
---
Duplicate comments:
In `@src/commands/admin/api-keys.ts`:
- Around line 60-77: Wrap the saveConfig call inside handleSaveKey in a
try/catch so that if saveConfig(config, globalOpts.profile) throws you fallback
to showing the new API key via printApiKeyBox(key), set process.exitCode = 1,
and still return true (so the user sees the key even when persisting fails);
apply the same pattern to the equivalent helper in src/commands/me-api-keys.ts.
Ensure you catch the exception from saveConfig (but do not swallow other
unrelated errors), call printApiKeyBox(key) in the catch block, set
process.exitCode = 1, and avoid losing the newly created key in failure
scenarios.
- Around line 79-95: The function showKeyResult treats a missing data.key as a
benign warning; instead, when data.key is undefined in the non-save branch, call
printError with a clear message, set process.exitCode = 1, and halt further
success handling (e.g., throw a short Error after setting the exit code) so the
caller cannot continue to print success; update the else branch that currently
calls printWarning to use printError + process.exitCode = 1 and stop execution
(refer to showKeyResult, data.key, printWarning/printError, and
process.exitCode).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11570a4c-def0-4ba4-aed3-a5881b24d0a0
📒 Files selected for processing (4)
src/commands/admin/api-keys.tssrc/commands/me-api-keys.tstests/admin-api-keys.test.tstests/me-api-keys.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/admin-api-keys.test.ts
- tests/me-api-keys.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/commands/me-api-keys.ts (1)
8-59: API キー結果処理は共通 helper に寄せたいです。
cleanApiKeyData()/handleSaveKey()/showKeyResult()がsrc/commands/admin/api-keys.tsにもほぼ同一で入っています。ここは 1 箇所に寄せておくと、文言変更や保存失敗時のフォールバック修正が片側だけずれるのを防ぎやすいです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/me-api-keys.ts` around lines 8 - 59, The three functions cleanApiKeyData, handleSaveKey, and showKeyResult are duplicated in admin/api-keys and me-api-keys; extract them into a single shared helper module (e.g., commands/apiKeyHelpers.ts), export those three functions with the same signatures, update both src/commands/me-api-keys.ts and src/commands/admin/api-keys.ts to import and use cleanApiKeyData, handleSaveKey, and showKeyResult, and ensure the helper file imports any dependencies used (resolveOptions, loadConfig, saveConfig, printError, printApiKeyBox, Command type) and preserves existing error handling and return semantics so callers behave unchanged. Ensure TypeScript types/exports compile and update any relative import paths accordingly.tests/me-api-keys.test.ts (1)
203-233:saveConfig()失敗時の退避経路も 1 本押さえておきたいです。
--saveのテストは成功系とkey欠落系はありますが、handleSaveKey()に新しく入ったsaveConfig()例外時のprintApiKeyBox(key)フォールバックはまだ固定されていません。saveConfigを throw させて、キーが表示されることとprocess.exitCode = 1を確認するケースを追加しておくと、この回帰を防ぎやすいです。Also applies to: 553-607
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/me-api-keys.test.ts` around lines 203 - 233, Add a failing-save test for the --save path: mock saveConfig to throw inside the existing test suite that covers me api-keys create (the same block that exercises handleSaveKey()), then call the command with --save and assert that printApiKeyBox(key) was called with the returned key, and that process.exitCode was set to 1; ensure client.rawRequest still returns the created key (e.g., key: "gdb_saved123") so handleSaveKey() takes the fallback branch when saveConfig throws.
🤖 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/commands/admin/api-keys.ts`:
- Around line 8-16: cleanApiKeyData currently deletes keyPrefix which breaks
structured outputs (json/geojson) because this function runs before
outputResponse; change it to only remove the real masked secret (keep the logic
that deletes obj.key when it's "******") and stop deleting obj.keyPrefix. For
table-only masking, move the keyPrefix removal into the table formatter (the
code path in outputResponse that renders tables) or add an optional parameter to
cleanApiKeyData (e.g., forTable: boolean) so callers can request table-specific
stripping; apply the same change to the duplicate helper in me-api-keys.ts.
---
Nitpick comments:
In `@src/commands/me-api-keys.ts`:
- Around line 8-59: The three functions cleanApiKeyData, handleSaveKey, and
showKeyResult are duplicated in admin/api-keys and me-api-keys; extract them
into a single shared helper module (e.g., commands/apiKeyHelpers.ts), export
those three functions with the same signatures, update both
src/commands/me-api-keys.ts and src/commands/admin/api-keys.ts to import and use
cleanApiKeyData, handleSaveKey, and showKeyResult, and ensure the helper file
imports any dependencies used (resolveOptions, loadConfig, saveConfig,
printError, printApiKeyBox, Command type) and preserves existing error handling
and return semantics so callers behave unchanged. Ensure TypeScript
types/exports compile and update any relative import paths accordingly.
In `@tests/me-api-keys.test.ts`:
- Around line 203-233: Add a failing-save test for the --save path: mock
saveConfig to throw inside the existing test suite that covers me api-keys
create (the same block that exercises handleSaveKey()), then call the command
with --save and assert that printApiKeyBox(key) was called with the returned
key, and that process.exitCode was set to 1; ensure client.rawRequest still
returns the created key (e.g., key: "gdb_saved123") so handleSaveKey() takes the
fallback branch when saveConfig throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b5c426a-71fd-4d18-800b-485ab1f790d0
📒 Files selected for processing (6)
CHANGELOG.mdsrc/commands/admin/api-keys.tssrc/commands/me-api-keys.tssrc/output.tstests/admin-api-keys.test.tstests/me-api-keys.test.ts
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/output.ts
- tests/admin-api-keys.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/commands/me-api-keys.ts (1)
17-58:showKeyResult()とhandleSaveKey()の責務をもう一段整理したいです。
showKeyResult()がkeyの存在を確定してからhandleSaveKey()を呼ぶので、後者の!key分岐は現状到達しません。handleSaveKey(key, cmd)に寄せ、できればsrc/commands/admin/api-keys.tsと共通化しておくと、create / refresh の差分漏れを減らせます。♻️ 変更イメージ
function handleSaveKey( - data: Record<string, unknown>, + key: string, cmd: Command, ): boolean { const globalOpts = resolveOptions(cmd); - const key = data.key as string | undefined; - if (!key) { - printError("Response missing key. API key was created, but it could not be saved."); - process.exitCode = 1; - return false; - } try { const config = loadConfig(globalOpts.profile); config.apiKey = key; @@ const key = data.key as string | undefined; if (!key) { printError("Response missing key. The new API key value was not returned."); process.exitCode = 1; return false; } - if (save) return handleSaveKey(data, cmd); + if (save) return handleSaveKey(key, cmd);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/me-api-keys.ts` around lines 17 - 58, Refactor showKeyResult and handleSaveKey so handleSaveKey accepts the API key as its first parameter (e.g., handleSaveKey(key: string, cmd: Command)) and remove the unreachable !key branch inside handleSaveKey; have showKeyResult validate the key and call handleSaveKey(key, cmd) when saving. Update all call sites (including any similar usage in src/commands/admin/api-keys.ts) to the new signature to enable reuse and avoid duplication between create/refresh flows, and ensure error handling/logging behavior remains unchanged when save fails.tests/admin-api-keys.test.ts (1)
548-569:--saveの「保存失敗 → キーを退避表示」経路も 1 本テストで固定したいです。新しい refresh テストは成功系と
key欠落系を押さえていますが、saveConfig()例外時にprintError()とprintApiKeyBox()へフォールバックする分岐はまだ未検証です。ここが regress すると一度しか返らない新キーを失うので、1 本追加しておくと安心です。🧪 テスト追加イメージ
+ it("falls back to printing the key when saveConfig throws", async () => { + vi.mocked(resolveOptions).mockReturnValue({ + url: "https://example.com/", + profile: "default", + }); + vi.mocked(saveConfig).mockImplementation(() => { + throw new Error("disk full"); + }); + client.rawRequest.mockResolvedValue( + mockResponse({ keyId: "k1", key: "gdb_refreshed" }), + ); + + const program = makeProgram(); + await runCommand(program, ["admin", "api-keys", "refresh", "k1", "--save"]); + + expect(printError).toHaveBeenCalledWith( + expect.stringContaining("Failed to save API key"), + ); + expect(printApiKeyBox).toHaveBeenCalledWith("gdb_refreshed"); + expect(process.exitCode).toBe(1); + });Also applies to: 583-602
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/admin-api-keys.test.ts` around lines 548 - 569, Add a test for the "admin api-keys refresh ... --save" flow that simulates saveConfig throwing so the fallback path is exercised; mock resolveOptions and client.rawRequest to return { keyId: "k1", key: "gdb_refreshed" }, make saveConfig throw an error, run the command via runCommand, and assert that printError was called with the save exception and printApiKeyBox was called to display the refreshed key (also assert console output if needed); use the same test structure as the existing "saves refreshed key to config with --save" test and reference saveConfig, client.rawRequest, runCommand, printError, and printApiKeyBox to locate the code to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/me-api-keys.ts`:
- Around line 17-58: Refactor showKeyResult and handleSaveKey so handleSaveKey
accepts the API key as its first parameter (e.g., handleSaveKey(key: string,
cmd: Command)) and remove the unreachable !key branch inside handleSaveKey; have
showKeyResult validate the key and call handleSaveKey(key, cmd) when saving.
Update all call sites (including any similar usage in
src/commands/admin/api-keys.ts) to the new signature to enable reuse and avoid
duplication between create/refresh flows, and ensure error handling/logging
behavior remains unchanged when save fails.
In `@tests/admin-api-keys.test.ts`:
- Around line 548-569: Add a test for the "admin api-keys refresh ... --save"
flow that simulates saveConfig throwing so the fallback path is exercised; mock
resolveOptions and client.rawRequest to return { keyId: "k1", key:
"gdb_refreshed" }, make saveConfig throw an error, run the command via
runCommand, and assert that printError was called with the save exception and
printApiKeyBox was called to display the refreshed key (also assert console
output if needed); use the same test structure as the existing "saves refreshed
key to config with --save" test and reference saveConfig, client.rawRequest,
runCommand, printError, and printApiKeyBox to locate the code to modify.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb393f27-6f32-4f5d-b286-cc0621d86f1b
📒 Files selected for processing (6)
CHANGELOG.mdpackage.jsonsrc/commands/admin/api-keys.tssrc/commands/me-api-keys.tstests/admin-api-keys.test.tstests/me-api-keys.test.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/me-api-keys.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.
Summary
Closes #91
API 側: geolonia/geonicdb#799
me api-keys refresh/admin api-keys refreshコマンドを新設 —POST .../api-keys/{keyId}/refreshでキーローテーション (--save対応)⚠ この API キー値を安全に保存してください。二度と表示されません。警告※ API キー値は作成時 (create) またはリフレッシュ時 (refresh) にのみ表示されます。printApiKeyBoxヘルパーをoutput.tsに追加handleSaveKey/showKeyResultを共通パターンとして抽出Test plan
npm run lint— クリアnpm run typecheck— クリアnpm test— 665 passed (refresh テスト 8 件含む)Summary by CodeRabbit
New Features
api-keys refreshコマンドを追加（API キーの再生成をサポート）--saveオプションを追加：作成／再生成時にキーを自動で設定に保存可能api-keys listに注釈を追加：キー値は作成時および再生成時のみ表示される旨を明記Documentation