fix(cli): console auth for console-only services and improved output rendering#1399
Open
ChiragAgg5k wants to merge 3 commits intomasterfrom
Open
fix(cli): console auth for console-only services and improved output rendering#1399ChiragAgg5k wants to merge 3 commits intomasterfrom
ChiragAgg5k wants to merge 3 commits intomasterfrom
Conversation
…e output rendering - Use sdkForConsole for console-only services (projects, organizations, etc.) based on x-appwrite.platforms metadata, fixing 401 errors on commands like `projects create-key` - Fix init project flow continuing to ask questions after user declines override - Add BigInt.prototype.toJSON polyfill to fix JSON.stringify errors on API responses - Improve table rendering: fall back to condensed key-value output when columns exceed 6, filtering out null/empty/object fields for readability - Add --raw flag for full unfiltered JSON output; --json now outputs filtered JSON - Filter out function-typed keys (toString) from rendered output - Add hint for teams service suggesting organizations command when no project is configured
Contributor
📝 WalkthroughWalkthroughAdds a CLI "raw" JSON output mode and propagates a computed Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR addresses several real pain points in the Appwrite CLI: console-only service authentication, the Key changes:
Confidence Score: 3/5Two P1 issues on changed code paths should be resolved before merging The isConsoleOnly false-positive can silently misroute generic services to console auth (the inverse of the intended fix), and the Math.max on an empty spread is a real RangeError crash in the new condensed table rendering. Both affect primary user paths. src/SDK/SDK.php (isConsoleOnly logic) and templates/cli/lib/parser.ts (condensed table Math.max guard) Important Files Changed
Reviews (1): Last reviewed commit: "fix(cli): use correct SDK client for con..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 4
🧹 Nitpick comments (1)
templates/cli/lib/parser.ts (1)
1-2: Guard the BigInt polyfill before overriding the prototype.This runs at module load and unconditionally replaces any existing
BigInt.prototype.toJSON. A polyfill should only install itself when the method is absent.🛠️ Suggested fix
-// `@ts-expect-error` BigInt toJSON polyfill for JSON.stringify support -BigInt.prototype.toJSON = function () { return this.toString(); }; +if (typeof BigInt.prototype.toJSON !== "function") { + // `@ts-expect-error` BigInt toJSON polyfill for JSON.stringify support + BigInt.prototype.toJSON = function () { + return this.toString(); + }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/cli/lib/parser.ts` around lines 1 - 2, Guard the BigInt polyfill so it only installs when missing: check that BigInt exists and BigInt.prototype.toJSON is not already defined before assigning; e.g. wrap the assignment in a conditional like if (typeof BigInt !== "undefined" && !("toJSON" in BigInt.prototype)) { BigInt.prototype.toJSON = function () { return this.toString(); }; } so you don't unconditionally overwrite an existing BigInt.prototype.toJSON implementation.
🤖 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/SDK/SDK.php`:
- Around line 1278-1287: The isConsoleOnly method incorrectly treats missing or
empty 'platforms' as console-only; update isConsoleOnly to require an explicit
platforms array equal to ['console'] for every method—if the 'platforms' key is
not set or is empty or any method's platforms is not exactly ['console'], return
false; locate and change this logic in the protected function
isConsoleOnly(array $methods): bool to ensure only methods that explicitly
declare platforms = ['console'] contribute to a true result.
In `@templates/cli/lib/commands/services/services.ts.twig`:
- Around line 50-57: The catch around sdkForProject() unconditionally shows the
'teams' hint; change the catch in the service.name == 'teams' branch to inspect
the thrown error from sdkForProject() and only call hint(...) when the error
indicates a missing-project condition (e.g., check error.code for a
PROJECT/NO_PROJECT/NOT_FOUND value or error.message contains "project" or "no
project"); for all other errors (like authentication/session errors) rethrow
without the teams hint so logged-out users are not misdirected. Ensure this
logic is applied in the catch that currently wraps sdkForProject().
In `@templates/cli/lib/parser.ts`:
- Around line 181-208: When rendering condensed output, guard against the case
where every field is filtered out by checking if rowEntries.flat() is empty
before computing maxKeyLen/separatorLen; if empty, fall back to the existing
JSON/verbose renderer (e.g., console.log(JSON.stringify(rows, null, 2))) and
return. Implement this check in the block that builds rowEntries (referencing
rowEntries, maxKeyLen, separatorLen, rows, MAX_COL_WIDTH) so you never call
Math.max on an empty array or repeat a negative length; additionally ensure
separatorLen is bounded with Math.max(0, computedValue) if you keep the repeat
call.
- Around line 61-95: filterObject and filterData currently drop any value where
typeof value === "object", which removes BigNumber fields that parse() and
formatCellValue treat as scalars; update both functions to detect and allow
BigNumber instances instead of skipping them. Change the object-check logic in
filterObject and the else-if branch in filterData so that you only skip plain
objects (and still handle arrays), but pass through BigNumber values by checking
via BigNumber.isBigNumber(value) or a safe constructor-name/instance check
(e.g., value?.constructor?.name === "BigNumber" or an imported BigNumber type),
ensuring BigNumber fields are included in the returned result.
---
Nitpick comments:
In `@templates/cli/lib/parser.ts`:
- Around line 1-2: Guard the BigInt polyfill so it only installs when missing:
check that BigInt exists and BigInt.prototype.toJSON is not already defined
before assigning; e.g. wrap the assignment in a conditional like if (typeof
BigInt !== "undefined" && !("toJSON" in BigInt.prototype)) {
BigInt.prototype.toJSON = function () { return this.toString(); }; } so you
don't unconditionally overwrite an existing BigInt.prototype.toJSON
implementation.
🪄 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: CHILL
Plan: Pro
Run ID: 3a01e8e0-c40b-4e28-985b-84e6bea97964
📒 Files selected for processing (8)
src/SDK/SDK.phpsrc/Spec/Swagger2.phptemplates/cli/cli.ts.twigtemplates/cli/lib/commands/services/services.ts.twigtemplates/cli/lib/parser.tstemplates/cli/lib/questions.tstemplates/cli/lib/sdks.tstemplates/cli/lib/types.ts
- Require explicit ['console'] platforms in isConsoleOnly (empty/missing no longer matches) - Guard Math.max against empty entries in condensed renderer - Scope teams hint to missing-project errors only, not session errors - Preserve BigNumber values in filtered JSON output
Contributor
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 `@templates/cli/lib/parser.ts`:
- Around line 196-204: The condensed renderer loop in parser.ts is erroneously
dropping BigNumber instances because it excludes all objects (if (typeof value
=== "object") continue); update the loop that builds entries from row so
BigNumber instances are detected before the generic object check and are
preserved by converting them to string (or delegating to formatCellValue),
mirroring the behavior of filterObject and filterData; specifically adjust the
logic around row, value and entries to treat BigNumber instances as primitives
rather than skipping them.
🪄 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: CHILL
Plan: Pro
Run ID: e766b9cd-91aa-4213-9846-f7d6da9f4c1d
📒 Files selected for processing (3)
src/SDK/SDK.phptemplates/cli/lib/commands/services/services.ts.twigtemplates/cli/lib/parser.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/SDK/SDK.php
- templates/cli/lib/commands/services/services.ts.twig
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
templates/cli/lib/parser.ts (2)
1-2: Consider usingJSONBig.stringifyinstead of polyfillingBigInt.prototype.toJSON.Modifying built-in prototypes affects global state. The codebase already has
JSONBig.stringifyinlib/json.tsdesigned for BigInt serialization. Using that indrawJSONwould avoid prototype pollution and keep serialization consistent.♻️ Proposed alternative
-// `@ts-expect-error` BigInt toJSON polyfill for JSON.stringify support -BigInt.prototype.toJSON = function () { return this.toString(); }; - import chalk from "chalk"; import { InvalidArgumentError } from "commander"; import Table from "cli-table3"; import packageJson from "../package.json" with { type: "json" }; +import { JSONBig } from "./json.js";Then update
drawJSON:export const drawJSON = (data: unknown): void => { - console.log(JSON.stringify(data, null, 2)); + console.log(JSONBig.stringify(data, null, 2)); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/cli/lib/parser.ts` around lines 1 - 2, The file currently polyfills BigInt.prototype.toJSON which mutates global state; remove that polyfill and update the JSON serialization to use the existing JSONBig.stringify implementation from lib/json.ts instead. Specifically, delete the BigInt.prototype.toJSON assignment and modify the drawJSON callers (or the drawJSON implementation) to call JSONBig.stringify(...) for serializing data containing BigInt values so serialization remains consistent without prototype pollution.
61-104: Consider using theisBigNumbertype guard fromjson.tsfor robustness.The
value?.constructor?.name === "BigNumber"pattern is fragile (can break with minification or multiple library instances). The codebase already has a robustisBigNumbertype guard inlib/json.tsthat checks_isBigNumberproperty and method signatures.♻️ Example of reusing isBigNumber
Export and import the helper:
// In json.ts, export the function export function isBigNumber(value: unknown): value is BigNumberLike { ... } // In parser.ts import { isBigNumber } from "./json.js";Then replace checks like:
-if (value?.constructor?.name === "BigNumber") { +if (isBigNumber(value)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/cli/lib/parser.ts` around lines 61 - 104, Replace fragile constructor-name checks with the exported isBigNumber type guard from json.ts: import isBigNumber from "./json.js" (or named import as exported) and update all places in filterObject and filterData that check value?.constructor?.name === "BigNumber" (and the similar check inside the Array.map for items) to use isBigNumber(value) / isBigNumber(item) instead, and keep the same behavior of converting BigNumber to String; ensure imports and type-narrowing follow the existing isBigNumber signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@templates/cli/lib/parser.ts`:
- Around line 1-2: The file currently polyfills BigInt.prototype.toJSON which
mutates global state; remove that polyfill and update the JSON serialization to
use the existing JSONBig.stringify implementation from lib/json.ts instead.
Specifically, delete the BigInt.prototype.toJSON assignment and modify the
drawJSON callers (or the drawJSON implementation) to call JSONBig.stringify(...)
for serializing data containing BigInt values so serialization remains
consistent without prototype pollution.
- Around line 61-104: Replace fragile constructor-name checks with the exported
isBigNumber type guard from json.ts: import isBigNumber from "./json.js" (or
named import as exported) and update all places in filterObject and filterData
that check value?.constructor?.name === "BigNumber" (and the similar check
inside the Array.map for items) to use isBigNumber(value) / isBigNumber(item)
instead, and keep the same behavior of converting BigNumber to String; ensure
imports and type-narrowing follow the existing isBigNumber signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df85c68d-7c57-4a7b-988f-a8e19d2830a1
📒 Files selected for processing (1)
templates/cli/lib/parser.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
projects,organizations,console, etc. now usesdkForConsoleinstead ofsdkForProject, fixing 401 errors on commands likeprojects create-key. Detection is based onx-appwrite.platformsmetadata from the spec — services where all methods are console-only getsdkForConsole, mixed services keepsdkForProject.BigInt.prototype.toJSONpolyfill to fixJSON.stringify cannot serialize BigInterrors on API responses.{N keys}/[N items].--rawflag:--jsonnow outputs filtered JSON (same fields as default view),--rawoutputs the full unfiltered response.toStringfrom output: Function-typed keys on API response objects are excluded from rendering.teamscommands fail due to missing project config, a hint suggests usingorganizationsinstead.Test plan
appwrite projects create-keyno longer returns 401appwrite organizations listrenders cleanly with aligned key-value outputappwrite organizations list --jsonoutputs filtered JSONappwrite organizations list --rawoutputs full JSONappwrite projects list-keysstill renders as table (few columns)appwrite teams list(without project) shows hint about organizationstoStringline in outputSummary by CodeRabbit
New Features
--rawCLI flag for unfiltered JSON output.Improvements