feat: me policies / me api-keys oauth-clients update / XACML docs (v0.8.0)#87
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
Changes
Sequence DiagramsequenceDiagram
participant User as User (CLI)
participant CLI as geonic CLI
participant Client as HTTP Client
participant API as Geonic API (/me/*)
User->>CLI: geonic me policies create [json]
CLI->>CLI: parseJsonInput / assemble body
CLI->>Client: POST /me/policies (body)
Client->>API: POST /me/policies
API-->>Client: 201 + Policy JSON
Client-->>CLI: response
CLI->>User: outputResponse + "Policy created."
User->>CLI: geonic me api-keys update keyId --policy-id p --rate-limit 60
CLI->>CLI: validate flags / assemble body
CLI->>Client: PATCH /me/api-keys/{keyId} (body)
Client->>API: PATCH /me/api-keys/{keyId}
API-->>Client: 200 + Updated ApiKey
Client-->>CLI: response
CLI->>User: outputResponse + "API key updated."
User->>CLI: geonic me oauth-clients regenerate-secret clientId
CLI->>Client: POST /me/oauth-clients/{clientId}/regenerate-secret
Client->>API: POST /me/oauth-clients/{clientId}/regenerate-secret
API-->>Client: 200 + Secret JSON
Client-->>CLI: response
CLI->>User: printWarning + outputResponse + "OAuth client secret regenerated."
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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tests/me-api-keys.test.ts (1)
454-470:--rate-limit 0の境界値テストも追加した方が安全です。非整数は検証されていますが、
0も「正の整数」違反として落とす期待値を固定しておくと、実装との不整合を防げます。テスト追加例
+ it("rejects zero --rate-limit", async () => { + const isTTY = process.stdin.isTTY; + process.stdin.isTTY = true; + try { + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + const program = makeProgram(); + await expect( + runCommand(program, ["me", "api-keys", "update", "k1", "--rate-limit", "0"]) + ).rejects.toThrow("process.exit"); + expect(printError).toHaveBeenCalledWith("--rate-limit must be a positive integer."); + exitSpy.mockRestore(); + } finally { + process.stdin.isTTY = isTTY; + } + });🤖 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 454 - 470, Add a new test mirroring the existing "rejects non-integer --rate-limit" case that asserts "--rate-limit 0" is rejected as not a positive integer: create an it block (e.g., it("rejects zero --rate-limit", ...)) that temporarily sets process.stdin.isTTY to true, spies process.exit to throw (via vi.spyOn(process, "exit").mockImplementation(() => { throw new Error("process.exit"); })), calls runCommand(makeProgram(), ["me","api-keys","update","k1","--rate-limit","0"]) and expects it to reject with "process.exit", and checks printError was called with "--rate-limit must be a positive integer."; ensure you restore the exit spy and reset process.stdin.isTTY in a finally block similar to the existing test.src/commands/me-oauth-clients.ts (1)
139-140:--activeと--inactiveの同時指定は明示的に拒否した方がよいです。現在は両方指定すると
isActive: falseで静かに上書きされます。入力ミスを早期検知できるよう、同時指定時にエラー終了する分岐を入れることを推奨します。修正案
const opts = cmd.opts() as { name?: string; description?: string; policyId?: string; active?: boolean; inactive?: boolean; }; + if (opts.active && opts.inactive) { + printError("--active and --inactive cannot be used together."); + process.exit(1); + } + let body: unknown;Also applies to: 159-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/me-oauth-clients.ts` around lines 139 - 140, Add an explicit check that prevents simultaneous use of the --active and --inactive options: after parsing options in the command's action/handler (the code that consumes the option declarations for "--active" and "--inactive" in src/commands/me-oauth-clients.ts), detect if both opts.active and opts.inactive are true and exit with an error (throw an Error or call process.exit(1) after printing a clear message via the CLI logger). This check should be placed before you compute or set isActive so the conflicting inputs are rejected early; apply the same logic to the other command block that defines the same options.
🤖 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 10-13: The changelog entry for "## [0.8.0] - 2026-03-23" is using
a future fixed release date; either move the entire "0.8.0" section under the
"Unreleased" heading or change the date to the actual release date only when
publishing (do not hardcode 2026-03-23 now). Locate the heading "## [0.8.0] -
2026-03-23" and update it to "## [0.8.0] - Unreleased" (or remove the date
entirely) until you are ready to tag the release, or set the date to 2026-03-22
only at release time. Ensure the rest of the list (the `me policies` bullet)
stays under that adjusted heading.
In `@README.md`:
- Line 273: The README contains a contradiction between the statement that
`priority` is fixed at 100 and the `/me/policies` section that says `101+`; pick
the correct rule and make both places consistent: either change the "priority is
fixed at 100" wording to explain the user-role minimum and that `/me/policies`
requires 101+ (or vice versa), and update both occurrences (the `priority`
description and the `/me/policies` policy text) so they state the same numeric
requirement and a short clarifying sentence about precedence.
In `@src/commands/me-api-keys.ts`:
- Around line 182-187: Change the truthy check of opts.origins to an explicit
undefined check and reject empty input: replace conditions using "opts.origins"
with "opts.origins !== undefined" and when building payload.allowedOrigins guard
for empty-string input (e.g. if opts.origins === "" or the split yields an empty
array) and raise/emit an explicit error instead of falling through to
parseJsonInput(); update the same pattern around the other occurrences (the
block referenced at lines ~199-201) so both the update branch and any similar
branches treat "--origins \"\"" as invalid and do not defer to parseJsonInput().
- Around line 188-195: The rate-limit parsing allows "0" even though the message
says "positive integer"; update the block that handles opts.rateLimit so after
trimming and numeric check you parse Number(raw) into a variable (e.g., const
perMinute = Number(raw)) and reject it if perMinute <= 0 by calling
printError("--rate-limit must be a positive integer.") and process.exit(1); then
assign payload.rateLimit = { perMinute } only when perMinute > 0. Reference:
opts.rateLimit, payload.rateLimit, perMinute.
In `@src/commands/me-policies.ts`:
- Around line 56-58: The create subcommand currently tied to the CLI command
builder (policies.command("create [json]") and related handlers around the
create flow) only accepts a JSON payload and lacks flag-based inputs required by
Issue `#83`; update the command registration and handler to accept explicit flags
(e.g., --policy-id, --description, --rules, --overwrite) alongside or instead of
the single [json] positional, parse those flags in the create handler (the
function that processes the "create" command) and build the policy object from
flags when JSON is not provided, add validation and precedence rules (flags
override or are used when json is absent), and apply the same change to the
similar create/update code paths referenced around the other block (the code
near lines 109-111) so users can create/update policies via flags without
needing raw JSON.
---
Nitpick comments:
In `@src/commands/me-oauth-clients.ts`:
- Around line 139-140: Add an explicit check that prevents simultaneous use of
the --active and --inactive options: after parsing options in the command's
action/handler (the code that consumes the option declarations for "--active"
and "--inactive" in src/commands/me-oauth-clients.ts), detect if both
opts.active and opts.inactive are true and exit with an error (throw an Error or
call process.exit(1) after printing a clear message via the CLI logger). This
check should be placed before you compute or set isActive so the conflicting
inputs are rejected early; apply the same logic to the other command block that
defines the same options.
In `@tests/me-api-keys.test.ts`:
- Around line 454-470: Add a new test mirroring the existing "rejects
non-integer --rate-limit" case that asserts "--rate-limit 0" is rejected as not
a positive integer: create an it block (e.g., it("rejects zero --rate-limit",
...)) that temporarily sets process.stdin.isTTY to true, spies process.exit to
throw (via vi.spyOn(process, "exit").mockImplementation(() => { throw new
Error("process.exit"); })), calls runCommand(makeProgram(),
["me","api-keys","update","k1","--rate-limit","0"]) and expects it to reject
with "process.exit", and checks printError was called with "--rate-limit must be
a positive integer."; ensure you restore the exit spy and reset
process.stdin.isTTY in a finally block similar to the existing test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41f84fd1-0273-4f81-845b-6af414c0a42d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdsrc/commands/admin/policies.tssrc/commands/auth.tssrc/commands/me-api-keys.tssrc/commands/me-oauth-clients.tssrc/commands/me-policies.tstests/me-api-keys.test.tstests/me-oauth-clients.test.tstests/me-policies.test.ts
Comment on lines
+10
to
+13
| ## [0.8.0] - 2026-03-23 | ||
|
|
||
| ### 2026-03-23 | ||
| - **Feat**: `me policies` コマンド群を新設 — `list`, `get`, `create`, `update`, `delete` で `/me/policies` セルフサービスポリシー管理に対応 (GeonicDB #764) (#83, #86) |
There was a problem hiding this comment.
リリース日が未来日付で固定されています。
このPR時点（2026-03-22）で 0.8.0 を 2026-03-23 と確定すると、未リリース状態との整合が崩れやすいです。リリース当日確定にするか、Unreleased 側で保持する運用が安全です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 10 - 13, The changelog entry for "## [0.8.0] -
2026-03-23" is using a future fixed release date; either move the entire "0.8.0"
section under the "Unreleased" heading or change the date to the actual release
date only when publishing (do not hardcode 2026-03-23 now). Locate the heading
"## [0.8.0] - 2026-03-23" and update it to "## [0.8.0] - Unreleased" (or remove
the date entirely) until you are ready to tag the release, or set the date to
2026-03-22 only at release time. Ensure the rest of the list (the `me policies`
bullet) stays under that adjusted heading.
… flags, priority矛盾修正
miya0001
commented
Mar 22, 2026
miya0001
left a comment
miya0001
left a comment
Member
Author
There was a problem hiding this comment.
coderabbit のレビューに対応しました（d949871）。
CHANGELOG 日付について
今日 2026-03-23 にリリース予定なので据え置きます。
README priority 矛盾（100固定 vs 101+）
サーバー実装（createPolicyForUser）を確認し、/me/policies で作成されるポリシーの priority は 100 固定であることを確認。priority テーブルの 101+ 行を 100 に統合しました。
--origins "" が空チェックをすり抜けるバグ
opts.origins !== undefined で判定するよう修正し、空文字の場合は早期にエラーを出すよう対応しました。
--rate-limit 0 が通ってしまうバグ
perMinute <= 0 のチェックを追加しました。
me policies create/update のフラグ不足
me policies create に --policy-id と --description フラグを追加、me policies update に --description フラグを追加しました。rules/target はネスト構造が複雑なため JSON 入力のみとしています。
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
me policies list/get/create/update/deleteを新設 —/me/policiesセルフサービスポリシー管理 (closes feat:me policiesコマンド群の新設 — /me/policies セルフサービスポリシー管理 #83, me policies: /me/policies CRUD コマンド対応 #86)me api-keys updateを追加 —PATCH /me/api-keys/{keyId}対応 (closes me patch: PATCH /me/api-keys/{keyId} と PATCH /me/oauth-clients/{clientId} 対応 #85)me oauth-clients updateを追加 —PATCH /me/oauth-clients/{clientId}対応 (closes me patch: PATCH /me/api-keys/{keyId} と PATCH /me/oauth-clients/{clientId} 対応 #85)me oauth-clients regenerate-secretを追加 —POST /me/oauth-clients/{clientId}/regenerate-secret対応Test plan
npm test— 653 tests passednpm run lint— cleannpm run typecheck— cleanSummary by CodeRabbit
新機能
me policiesを追加：個人ポリシーの一覧/取得/作成/更新/削除をCLIで操作可能にme api-keys updateを追加：名前/ポリシー/発信元/レート制限/DPoP/有効化を更新可能にme oauth-clients updateとme oauth-clients regenerate-secretを追加：クライアント更新とシークレット再生成をCLIで実行可能にドキュメント
テスト
その他