feat(cli): add cloud redis show command#108
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019b0df3-dd26-71fe-b18f-67c4c269267b Co-authored-by: Amp <amp@ampcode.com>
WalkthroughAdds a new cloud Redis CLI command group with a show/info subcommand to retrieve a Redis connection URL, registers the command in the cloud entrypoint, exposes a plain-text output helper, and extends the server Region resources response schema with an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
robindiddams
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/cli/src/cmd/cloud/redis/index.ts (2)
2-2: Naming inconsistency between file and export.The file is named
get.ts(and PR title mentions "get command"), but it exportsshowSubcommandwith the command name "show". This creates confusion between the filename, PR objectives, and the actual implementation.Consider either:
- Renaming the subcommand to "get" with "show" as an alias:
name: 'get', aliases: ['show', 'info']- Or renaming the file to
show.tsto match the subcommand name</comment_end>
8-8: Tag inconsistency between parent and child commands.The parent
rediscommand is tagged as'slow', but theshowSubcommand(the only subcommand) is tagged as'fast'. These contradictory tags may confuse users about the expected performance characteristics of the command.Consider aligning the tags - either both should be 'fast' (if the API call is typically quick) or both should be 'slow' (if network latency is expected).
</comment_end>
packages/cli/src/cmd/cloud/redis/get.ts (1)
47-53: Consider adding error handling for the API call.The
listResourcescall can throw aRegionResponseError(per the implementation inpackages/server/src/api/region/resources.ts), but there's no try-catch block here. While the error may be handled by the command framework, wrapping this in explicit error handling could provide a more user-friendly error message.Consider adding error handling:
- const resources = await tui.spinner({ - message: `Fetching Redis for ${orgId} in ${region}`, - clearOnSuccess: true, - callback: async () => { - return listResources(catalystClient, orgId, region); - }, - }); + let resources; + try { + resources = await tui.spinner({ + message: `Fetching Redis for ${orgId} in ${region}`, + clearOnSuccess: true, + callback: async () => { + return listResources(catalystClient, orgId, region); + }, + }); + } catch (error) { + logger.fatal('Failed to fetch Redis resources'); + throw error; + }</comment_end>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cli/src/cmd/cloud/redis/get.ts(1 hunks)packages/cli/src/cmd/cloud/redis/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier using tabs (width 3), single quotes, semicolons, and 100 character line width
Files:
packages/cli/src/cmd/cloud/redis/index.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/*.{ts,tsx}: Always useInferInputandInferOutputfrom@agentuity/corefor schema type inference, notStandardSchemaV1.InferOutput
StructuredError properties are directly on the error instance, not under a.dataproperty
Files:
packages/cli/src/cmd/cloud/redis/index.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use the
StructuredErrorfrom@agentuity/coreto create Error classes with structured data properties
Files:
packages/cli/src/cmd/cloud/redis/index.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/**/*.ts: All code must be TypeScript with strict mode enabled
Usetui.*helpers for formatted output without log prefixes (success, error, warning, info, bold, muted, link)
Usectx.loggerfor standard logging with prefixes (fatal, error, info, debug) instead of console methods
Uselogger.fatal(message)for error logging with exit code 1 instead oflogger.error()+process.exit(1)
Use Bun-provided APIs (e.g.,Bun.file(f).exists()) instead of legacy Node.js APIs (e.g.,existsSync(f))
All I/O operations must use async/await pattern
Files:
packages/cli/src/cmd/cloud/redis/index.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Use commander.js for command parsing and routing following commander.js best practices
Always define interfaces for command options (never useany)
Subcommands must be defined in separate files and exported asSubcommandDefinition
Commands must validate their own config requirements
Files:
packages/cli/src/cmd/cloud/redis/index.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/cli/src/cmd/**/index.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Each command must be a directory in
src/cmd/with index.ts file exporting CommandDefinition
Files:
packages/cli/src/cmd/cloud/redis/index.ts
🧬 Code graph analysis (2)
packages/cli/src/cmd/cloud/redis/index.ts (1)
packages/cli/src/cmd/cloud/redis/get.ts (1)
showSubcommand(12-72)
packages/cli/src/cmd/cloud/redis/get.ts (2)
packages/cli/src/config.ts (1)
getCatalystAPIClient(628-632)packages/server/src/api/region/resources.ts (1)
listResources(43-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Cloud Deployment Tests
🔇 Additional comments (1)
packages/cli/src/cmd/cloud/redis/get.ts (1)
1-10: LGTM!The imports and schema definition look correct. The Zod 4 usage with
z.string().optional()is appropriate for the optional URL field.</comment_end>
| export const showSubcommand = createSubcommand({ | ||
| name: 'show', | ||
| aliases: ['info'], |
There was a problem hiding this comment.
Command name inconsistency with PR objectives and filename.
The PR is titled "feat(cli): add cloud redis get command" and this file is named get.ts, but the subcommand is implemented as name: 'show' with aliases ['info'] — "get" appears nowhere in the command definition.
This creates confusion between:
- PR title/objectives: "cloud redis get"
- Filename:
get.ts - Actual command: "cloud redis show"
Apply this diff to align with the PR title and filename:
export const showSubcommand = createSubcommand({
- name: 'show',
- aliases: ['info'],
+ name: 'get',
+ aliases: ['show', 'info'],
description: 'Show Redis connection URL',Then rename the export and file references accordingly, or clarify the intended command name in the PR description.
</comment_end>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const showSubcommand = createSubcommand({ | |
| name: 'show', | |
| aliases: ['info'], | |
| export const showSubcommand = createSubcommand({ | |
| name: 'get', | |
| aliases: ['show', 'info'], |
🤖 Prompt for AI Agents
In packages/cli/src/cmd/cloud/redis/get.ts around lines 12 to 14, the subcommand
is declared as name: 'show' with aliases ['info'] which conflicts with the PR
title and filename that indicate this should be a "get" command; change the
subcommand name to 'get' (and adjust or remove aliases as desired), rename the
exported constant showSubcommand to getSubcommand, and update any imports/usage
sites or the filename references so the symbol, file name, and PR intent are
consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli/src/cmd/cloud/redis/get.ts (1)
12-14: Command name inconsistency remains unresolved.The mismatch between the PR title ("add cloud redis get command"), filename (
get.ts), and the actual command name (showwith aliasinfo) creates confusion. This issue was flagged in previous reviews.The past review suggestion to rename to
name: 'get'withaliases: ['show', 'info']would resolve this inconsistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cli/src/cmd/cloud/redis/get.ts(1 hunks)packages/cli/src/tui.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier using tabs (width 3), single quotes, semicolons, and 100 character line width
Files:
packages/cli/src/tui.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/*.{ts,tsx}: Always useInferInputandInferOutputfrom@agentuity/corefor schema type inference, notStandardSchemaV1.InferOutput
StructuredError properties are directly on the error instance, not under a.dataproperty
Files:
packages/cli/src/tui.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use the
StructuredErrorfrom@agentuity/coreto create Error classes with structured data properties
Files:
packages/cli/src/tui.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/**/*.ts: All code must be TypeScript with strict mode enabled
Usetui.*helpers for formatted output without log prefixes (success, error, warning, info, bold, muted, link)
Usectx.loggerfor standard logging with prefixes (fatal, error, info, debug) instead of console methods
Uselogger.fatal(message)for error logging with exit code 1 instead oflogger.error()+process.exit(1)
Use Bun-provided APIs (e.g.,Bun.file(f).exists()) instead of legacy Node.js APIs (e.g.,existsSync(f))
All I/O operations must use async/await pattern
Files:
packages/cli/src/tui.tspackages/cli/src/cmd/cloud/redis/get.ts
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Use commander.js for command parsing and routing following commander.js best practices
Always define interfaces for command options (never useany)
Subcommands must be defined in separate files and exported asSubcommandDefinition
Commands must validate their own config requirements
Files:
packages/cli/src/cmd/cloud/redis/get.ts
🧬 Code graph analysis (1)
packages/cli/src/cmd/cloud/redis/get.ts (2)
packages/cli/src/config.ts (1)
getCatalystAPIClient(628-632)packages/server/src/api/region/resources.ts (1)
listResources(43-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build
- GitHub Check: Cloud Deployment Tests
- GitHub Check: SDK Integration Test Suite
🔇 Additional comments (5)
packages/cli/src/tui.ts (1)
349-355: LGTM! Provides the correct path for plain command output.The new
output()helper fills an important gap by providing a clean interface for primary command output without prefixes or formatting. This addresses the need identified in past reviews where commands needed plain output (e.g., the Redis get command at line 65 inget.ts).packages/cli/src/cmd/cloud/redis/get.ts (4)
8-10: Well-defined response schema.The schema correctly uses Zod 4 syntax with an optional URL field and descriptive documentation. This aligns with the command's behavior of returning
undefinedwhen Redis is not provisioned.
55-58: Good handling of the no-Redis case.The check correctly uses
tui.info()per coding guidelines, provides a clear user message, and returns a schema-compliant response withurl: undefined.
63-66: Output formatting correctly uses TUI helpers.The code now uses
tui.output()(introduced inpackages/cli/src/tui.ts) instead of directconsole.log(), which resolves the concern raised in previous reviews. The credential masking logic is sound: credentials are masked in terminal output by default but shown in JSON mode or when--show-credentialsis specified.Note: This addresses the previous review comment about avoiding
console.log()for command output.
68-70: Return value correctly provides unmasked URL.The handler appropriately returns the raw Redis URL (output masking is handled separately for display), which matches the response schema and ensures JSON output contains the full URL.
| const resources = await tui.spinner({ | ||
| message: `Fetching Redis for ${orgId} in ${region}`, | ||
| clearOnSuccess: true, | ||
| callback: async () => { | ||
| return listResources(catalystClient, orgId, region); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add error handling for the resource fetch.
The listResources call can throw RegionResponseError (per packages/server/src/api/region/resources.ts), but there's no try-catch block to handle failures gracefully. Users will see a raw error instead of a friendly message.
Consider wrapping the spinner callback in a try-catch:
const resources = await tui.spinner({
message: `Fetching Redis for ${orgId} in ${region}`,
clearOnSuccess: true,
callback: async () => {
- return listResources(catalystClient, orgId, region);
+ try {
+ return listResources(catalystClient, orgId, region);
+ } catch (err) {
+ if (err instanceof Error) {
+ logger.fatal(`Failed to fetch resources: ${err.message}`);
+ }
+ throw err;
+ }
},
});🤖 Prompt for AI Agents
In packages/cli/src/cmd/cloud/redis/get.ts around lines 47–53, the spinner
callback calls listResources which can throw RegionResponseError and currently
has no error handling; wrap the callback body in a try-catch, catch the
RegionResponseError (import the error type from its module where it's exported),
call tui.error with a friendly message including orgId and region, and return an
empty array or null so the spinner resolves gracefully; rethrow or propagate
non-RegionResponseError exceptions after logging so unexpected errors still
surface.
1 participant
Summary
Add new
cloud redis getcommand to fetch the Redis connection URL for an organization.New Commands
Example Output
Changes
redisfield to resource list response schemaRelated
Requires catalyst changes to expose Redis in the
/resource/2025-03-17/:orgId/:regionendpoint response.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.