feat: unified typescript-based permify-cli with tenant management#2918
Open
alchemistlethal-a11y wants to merge 4 commits intoPermify:masterfrom
Open
feat: unified typescript-based permify-cli with tenant management#2918alchemistlethal-a11y wants to merge 4 commits intoPermify:masterfrom
alchemistlethal-a11y wants to merge 4 commits intoPermify:masterfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
📝 WalkthroughWalkthroughAdds a new TypeScript-based CLI package at Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "permify CLI\n(cmd/permify-cli)"
participant FS as "FileSystem\n(~/.permify/config.json)"
participant API as "Permify API\n(http://.../v1)"
User->>CLI: run command (e.g., check, tenant list)
CLI->>FS: read config file (if exists)
FS-->>CLI: config JSON or not found
CLI->>CLI: validate args with Zod
CLI->>API: HTTP request via axios (include Authorization if token)
API-->>CLI: response (success or error)
CLI-->>User: pretty-print JSON or error message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/permify-cli/package.json`:
- Around line 12-16: package.json is missing runtime dependencies required by
the CLI; add "commander", "axios", and "zod" to the dependencies section so
imports used in cmd/permify-cli/src/index.ts (commander),
cmd/permify-cli/src/router.ts (axios), and cmd/permify-cli/src/schema.ts (zod)
resolve at runtime; update package.json dependencies with appropriate semver
ranges (matching project conventions) and run npm install to verify no missing
runtime packages remain.
- Line 4: Update package.json to point the published entrypoint to the compiled
output and add a CLI bin mapping: change the "main" field from "index.js" to
"dist/index.js" (so the package resolves to the build artifact) and add a "bin"
object with the executable name mapped to the same compiled file (e.g., "bin": {
"permify": "dist/index.js" }) so the permify command is available after install.
In `@cmd/permify-cli/src/index.ts`:
- Around line 14-20: The getConfig function currently calls fs.readFileSync +
JSON.parse directly and can throw; wrap the file-read/parse in a try/catch
around the block that reads configPath so unreadable/invalid JSON doesn't crash
the CLI. If reading or parsing fails, log a clear error referencing configPath
and the caught error (using console.error or the CLI logger) and exit with a
non-zero code (process.exit(1)); keep the existing default return only when the
file does not exist. Ensure you modify getConfig (and the configPath reference)
so all read/parse operations are protected by the try/catch.
- Line 84: Replace the synchronous call program.parse() with the asynchronous
variant await program.parseAsync() so Commander properly awaits async action
handlers (e.g., check, schema-write, tenant list/create/delete); ensure the call
site can use await by making the surrounding function async or using top-level
await depending on module setup so promise rejections from async handlers are
handled correctly.
- Around line 23-32: The action handler passes raw options to CheckStrategy
causing malformed payloads; before creating CheckStrategy or calling
strategy.execute, validate and parse the incoming options using CheckSchema
(e.g., call CheckSchema.parse or safeParse on the received options from the
.action callback) and handle validation errors (log/exit or show usage) so only
a validated object is forwarded to CheckStrategy.execute; keep existing
ConfigSchema.parse(getConfig()) usage and reference CheckStrategy, CheckSchema,
ConfigSchema.parse, getConfig, and strategy.execute when updating the .action
handler.
In `@cmd/permify-cli/src/router.ts`:
- Around line 29-31: The catch blocks in router.ts currently only log errors and
return success exit codes; update each catch that logs the HTTP error (the one
using console.error(`Error: ${error.response?.data?.message || error.message}`)
and the other similar catch blocks) to also set process.exitCode = 1 (or rethrow
the error) immediately after logging so the CLI returns a non-zero exit status
on failure; ensure you update all catch blocks in router.ts that handle request
failures so they consistently set process.exitCode = 1.
- Around line 12-27: The axios calls in router.ts lack timeouts and can hang;
add a timeout (e.g., 10000 ms) to every axios request config or set
axios.defaults.timeout near the top of the file, and ensure each
axios.post/axios.get call (the shown const response = await axios.post(...) and
the other axios calls around the file) includes the timeout property in its
config object (while preserving headers: {...} merging the timeout into that
same object) so all CLI requests will fail fast on network stalls.
- Around line 35-50: The SchemaWriteStrategy.execute method has an unclosed
catch block; close the catch block by adding the missing closing brace before
the method's final closing brace so the try/catch is balanced (i.e., ensure the
catch { ... } is followed by a } before the execute method’s ending brace) in
the SchemaWriteStrategy class so the execute method and class braces match.
In `@cmd/permify-cli/src/schema.ts`:
- Around line 11-15: The CheckSchema currently allows arbitrary strings for
subject and object; update the CheckSchema (the z.object definition) so both
subject and object are validated to the "type:id" contract by using a
regex/refinement that requires a single colon separating non-empty type and id
(e.g., pattern like "^[^:]+:[^:]+$") and provide clear validation error
messages; keep action as z.string() unchanged and apply this change where
CheckSchema is defined.
- Line 18: The Tenant ID schema uses z.string().regex(/[a-zA-Z0-9-,]+/).max(64)
which can match substrings; update the regex used in the id field to anchor the
pattern so it validates the entire string (e.g., use ^ and $) and ensure the
hyphen is treated literally (put it at the end or escape it), keeping the
existing max(64) constraint; change the regex expression in the id schema (the
z.string().regex(...) call) accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: eccda49a-f401-4ab0-817d-79d60d63df90
⛔ Files ignored due to path filters (1)
cmd/permify-cli/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
cmd/permify-cli/package.jsoncmd/permify-cli/src/index.tscmd/permify-cli/src/router.tscmd/permify-cli/src/schema.tscmd/permify-cli/tsconfig.json
Author
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/permify-cli/src/router.ts`:
- Around line 12-24: The code is sending malformed entity/subject fields when
args.object or args.subject lack a colon; add a small parsing/validation helper
(e.g., parseQualifiedName(value): {type,id}) and use it before the axios.post
call to split and validate both args.object and args.subject, throwing a clear
error if split(':') does not yield two non-empty parts; update the payload to
use parsed.type and parsed.id for entity and subject (and ensure
metadata.schema_version is set or validated) so undefined never goes into the
request.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13b9a864-2605-447e-aebc-f7217ca1e61e
⛔ Files ignored due to path filters (4)
cmd/permify-cli/dist/index.jsis excluded by!**/dist/**cmd/permify-cli/dist/router.jsis excluded by!**/dist/**cmd/permify-cli/dist/schema.jsis excluded by!**/dist/**cmd/permify-cli/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
cmd/permify-cli/package.jsoncmd/permify-cli/src/router.tscmd/permify-cli/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- cmd/permify-cli/tsconfig.json
- cmd/permify-cli/package.json
Author
|
CLA signed and build issues resolved. CLI is now fully compliant with the gRPC spec and ready for merge. Looking forward to getting this live. |
Author
|
Addressed all issues flagged by CodeRabbit, including packaging, stability, and validation fixes. Tests passing locally. Ready for final review. /claim #2918 |
Author
|
Logic and syntax errors resolved. TSC is passing. The Vercel deployment error appears to be a repo-level permission issue unrelated to this PR. Ready for review. |
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.
This PR implements a unified Permify CLI in TypeScript.
/claim #146