feat(cli): -o json now works on mutation commands (create/update/delete/...)#37
Merged
Conversation
Mutation commands (create/update/delete/archive/activate/follow/...) used
to emit a plain `console.log(chalk.green('✓ ...'))` and ignore the global
`-o, --output` flag entirely, so `abs metric create … -o json` returned
the green message instead of JSON.
Centralize the mutation success path on a new printResult helper in
api-helper.ts. Routing by globalOptions:
- structured output (json/yaml/markdown/plain/vertical/template):
emits { success, message, id? }; with --raw it emits the full API
response instead.
- --output ids: bare id to stdout.
- stdout piped without explicit --output: bare id to stdout for
pipelining, the green confirmation to stderr so the user still sees
what happened.
- interactive default (table/rendered): chalk.green(message) on stdout
— unchanged.
~30 command files migrated to printResult. A few commands intentionally
keep their existing interactive multi-line output (api key create showing
the new key, reset-password, experiment create showing Name/Type) and
fall through to printResult only when a structured -o is requested.
Bulk/stream/progress flows (experiments bulk/export/refresh-fields) are
left for a follow-up — they need a different design (NDJSON or array).
Helper has 11 dedicated tests covering every route, plus two integration
test files updated where the assertions previously expected
printFormatted on the success path.
WalkthroughThis release standardises CLI command output across the entire codebase by introducing a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
src/lib/utils/api-helper.test.ts (1)
395-538: ⚡ Quick winConsider adding test coverage for
colorDisabledbehaviour.The test suite thoroughly verifies routing logic across formats and piping states. However, no test verifies that
globalOptions.colorDisabledsuppresses chalk colours in the output. Once the implementation respectscolorDisabled(per the comment onapi-helper.ts), adding a test case would ensure this user preference is honoured.Example test case
it('respects colorDisabled and prints uncoloured message', () => { printResult( { output: 'table', outputExplicit: false, colorDisabled: true }, { message: '✓ Metric 42 updated', id: 42 } ); expect(consoleSpy).toHaveBeenCalledWith('✓ Metric 42 updated'); // Verify no ANSI colour codes present in output });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/api-helper.test.ts` around lines 395 - 538, Add a unit test for printResult to verify globalOptions.colorDisabled suppresses chalk colours: within the existing printResult describe block, mock console.log/console.error and TTY as in beforeEach, call printResult with options including colorDisabled: true (and output: 'table' / outputExplicit as appropriate) and a message containing coloured characters, then assert console.log was called with the plain string and that the output contains no ANSI escape sequences (e.g. assert the printed string does not match /\u001b\[/). Reference the printResult function and the globalOptions.colorDisabled flag when adding this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/envs/index.ts`:
- Around line 54-57: The code currently asserts result.data as Record<string,
unknown> and directly reads id (newId) without guaranteeing it exists; update
the create command handlers (where CommandResult<unknown> is used) to ensure
type-safety by either tightening the CommandResult generic to include { id?:
unknown } or by extracting and validating the id before use: retrieve const data
= result.data as Record<string, unknown> (or a narrower type), check data?.id
(or typeof data.id !== 'undefined') and build the printResult call from that—use
a fallback message when id is missing and pass raw: result.data; apply the same
pattern in functions that call validateEntityResponse, printResult, and any
create X handlers to avoid unsafe assertions.
In `@src/lib/utils/api-helper.ts`:
- Around line 227-258: The printResult function currently calls chalk.green(...)
unconditionally; update it to respect globalOptions.colorDisabled by computing a
formatted message once (e.g., const formattedMessage =
globalOptions.colorDisabled ? args.message : chalk.green(args.message)) and use
that for both the stdout console.log and the stderr console.error paths (the
locations referencing chalk.green in printResult and in the isStdoutPiped
branch). Ensure the check uses the existing globalOptions.colorDisabled flag and
leave all other branching logic (shouldOutputIdsOnly, printFormatted,
isStdoutPiped) unchanged.
---
Nitpick comments:
In `@src/lib/utils/api-helper.test.ts`:
- Around line 395-538: Add a unit test for printResult to verify
globalOptions.colorDisabled suppresses chalk colours: within the existing
printResult describe block, mock console.log/console.error and TTY as in
beforeEach, call printResult with options including colorDisabled: true (and
output: 'table' / outputExplicit as appropriate) and a message containing
coloured characters, then assert console.log was called with the plain string
and that the output contains no ANSI escape sequences (e.g. assert the printed
string does not match /\u001b\[/). Reference the printResult function and the
globalOptions.colorDisabled flag when adding this test.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5f32358-5766-4cc5-aa1f-21ad8ab71d90
📒 Files selected for processing (59)
package.jsonsrc/commands/actiondialogfields/actiondialogfields.test.tssrc/commands/actiondialogfields/index.tssrc/commands/apikeys/index.tssrc/commands/apps/index.tssrc/commands/assetroles/index.tssrc/commands/cors/index.tssrc/commands/customfields/index.tssrc/commands/customsections/index.tssrc/commands/datasources/index.tssrc/commands/envs/index.tssrc/commands/events/index.tssrc/commands/experiments/access.tssrc/commands/experiments/activity.tssrc/commands/experiments/alerts.tssrc/commands/experiments/annotations.tssrc/commands/experiments/archive.tssrc/commands/experiments/create.tssrc/commands/experiments/custom-fields.tssrc/commands/experiments/development.tssrc/commands/experiments/follow.tssrc/commands/experiments/full-on.tssrc/commands/experiments/metrics.tssrc/commands/experiments/recommendations.tssrc/commands/experiments/request-update.tssrc/commands/experiments/restart.tssrc/commands/experiments/schedule.tssrc/commands/experiments/start.tssrc/commands/experiments/stop.tssrc/commands/experiments/update.tssrc/commands/exportconfigs/index.tssrc/commands/favorites/index.tssrc/commands/goals/access.tssrc/commands/goals/follow.tssrc/commands/goals/index.tssrc/commands/goaltags/index.tssrc/commands/metriccategories/index.tssrc/commands/metrics/access.tssrc/commands/metrics/follow.tssrc/commands/metrics/index.tssrc/commands/metrics/review.tssrc/commands/metrictags/index.tssrc/commands/notifications/index.tssrc/commands/platformconfig/index.tssrc/commands/roles/index.tssrc/commands/segments/index.tssrc/commands/storageconfigs/index.tssrc/commands/storageconfigs/storageconfigs.test.tssrc/commands/tags/index.tssrc/commands/teams/index.tssrc/commands/teams/members.tssrc/commands/units/index.tssrc/commands/updateschedules/index.tssrc/commands/users/api-keys.tssrc/commands/users/index.tssrc/commands/users/reset-password.tssrc/commands/webhooks/index.tssrc/lib/utils/api-helper.test.tssrc/lib/utils/api-helper.ts
Comment on lines
+54
to
+57
| const newId = (result.data as Record<string, unknown>).id; | ||
| printResult(globalOptions, { | ||
| message: `✓ Environment created with ID: ${newId}`, | ||
| id: newId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify unguarded id dereferences introduced in command handlers
rg -nP --type=ts -C2 '\(result\.data as Record<string, unknown>\)\.id' src/commands
# Inspect related create function declarations/typing contracts
rg -nP --type=ts -C4 'export\s+(async\s+)?(function|const)\s+create(Env|Role|Segment|Unit)\b' src/core
# Inspect call sites/contracts for create API methods to confirm whether body can be empty
rg -nP --type=ts -C4 '\b(createEnv|createRole|createSegment|createUnit)\b' srcRepository: absmartly/cli-ts
Length of output: 31382
🏁 Script executed:
# Check validateEntityResponse implementation to understand its contract
rg -nP --type=ts -A10 'validateEntityResponse\s*<' src/api-client/api-client.ts | head -50Repository: absmartly/cli-ts
Length of output: 2268
🏁 Script executed:
# Check if there are any guards or null checks around result.data usage in commands
rg -nP --type=ts -B2 -A5 'result\.data.*\.id' src/commands --max-count=3Repository: absmartly/cli-ts
Length of output: 11724
🏁 Script executed:
# Examine the return type of a create function in detail
sed -n '35,45p' src/core/envs/envs.tsRepository: absmartly/cli-ts
Length of output: 354
🏁 Script executed:
# Check what the CommandResult type looks like
rg -nP --type=ts 'interface CommandResult' srcRepository: absmartly/cli-ts
Length of output: 116
🏁 Script executed:
# Get the full validateEntityResponse implementation
sed -n '147,180p' src/api-client/api-client.tsRepository: absmartly/cli-ts
Length of output: 1323
🏁 Script executed:
# Check the CommandResult interface definition
rg -nP --type=ts -A3 'export interface CommandResult' srcRepository: absmartly/cli-ts
Length of output: 283
Improve type safety for API response entity extraction.
On line 54, the code assumes result.data has an id field but relies on a type assertion that provides no runtime protection. Although validateEntityResponse ensures the entity object exists, the CommandResult<unknown> type does not guarantee the presence of an id field. Use proper typing to make the contract explicit, or validate the presence of id before use.
Suggested approach
Strengthen the type by either:
- Explicitly type the CommandResult: Change
CommandResult<unknown>toCommandResult<{ id: unknown }>in the create functions, or - Add optional chaining: Use
data?.idto handle cases where the field may not exist, or - Validate before use: Check that
idexists before constructing the success message.
Example with optional chaining:
const data = result.data as Record<string, unknown>;
const newId = data?.id;
printResult(globalOptions, {
message: newId ? `✓ Environment created with ID: ${newId}` : `✓ Environment created`,
id: newId,
raw: result.data,
});This pattern appears in multiple command handlers (envs, roles, segments, units, webhooks, custom fields, asset roles, apps). Consider applying the fix consistently across all create commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/envs/index.ts` around lines 54 - 57, The code currently asserts
result.data as Record<string, unknown> and directly reads id (newId) without
guaranteeing it exists; update the create command handlers (where
CommandResult<unknown> is used) to ensure type-safety by either tightening the
CommandResult generic to include { id?: unknown } or by extracting and
validating the id before use: retrieve const data = result.data as
Record<string, unknown> (or a narrower type), check data?.id (or typeof data.id
!== 'undefined') and build the printResult call from that—use a fallback message
when id is missing and pass raw: result.data; apply the same pattern in
functions that call validateEntityResponse, printResult, and any create X
handlers to avoid unsafe assertions.
Comment on lines
+227
to
+258
| export function printResult( | ||
| globalOptions: GlobalOptions, | ||
| args: { message: string; id?: unknown; raw?: unknown } | ||
| ): void { | ||
| const format = (globalOptions.output ?? 'table') as OutputFormat; | ||
|
|
||
| if (globalOptions.outputExplicit && STRUCTURED_FORMATS.has(format)) { | ||
| const payload = | ||
| globalOptions.raw && args.raw !== undefined | ||
| ? args.raw | ||
| : { | ||
| success: true, | ||
| message: args.message, | ||
| ...(args.id !== undefined ? { id: args.id } : {}), | ||
| }; | ||
| printFormatted(payload, globalOptions); | ||
| return; | ||
| } | ||
|
|
||
| if (shouldOutputIdsOnly(globalOptions)) { | ||
| if (args.id !== undefined) console.log(args.id); | ||
| // Piped (no explicit format) → also surface the confirmation on | ||
| // stderr so users running `cmd | xargs ...` still see success. | ||
| // Explicit `-o ids` stays quiet to keep the stream machine-clean. | ||
| if (isStdoutPiped() && !globalOptions.outputExplicit) { | ||
| console.error(chalk.green(args.message)); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| console.log(chalk.green(args.message)); | ||
| } |
There was a problem hiding this comment.
Respect globalOptions.colorDisabled before applying chalk colours.
Lines 252 and 257 use chalk.green() unconditionally, ignoring the user's explicit --no-color preference captured in globalOptions.colorDisabled. When users disable colours for accessibility, logging, or other reasons, the output should honour that choice.
🎨 Proposed fix
if (shouldOutputIdsOnly(globalOptions)) {
if (args.id !== undefined) console.log(args.id);
// Piped (no explicit format) → also surface the confirmation on
// stderr so users running `cmd | xargs ...` still see success.
// Explicit `-o ids` stays quiet to keep the stream machine-clean.
if (isStdoutPiped() && !globalOptions.outputExplicit) {
- console.error(chalk.green(args.message));
+ const message = globalOptions.colorDisabled ? args.message : chalk.green(args.message);
+ console.error(message);
}
return;
}
- console.log(chalk.green(args.message));
+ const message = globalOptions.colorDisabled ? args.message : chalk.green(args.message);
+ console.log(message);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/utils/api-helper.ts` around lines 227 - 258, The printResult function
currently calls chalk.green(...) unconditionally; update it to respect
globalOptions.colorDisabled by computing a formatted message once (e.g., const
formattedMessage = globalOptions.colorDisabled ? args.message :
chalk.green(args.message)) and use that for both the stdout console.log and the
stderr console.error paths (the locations referencing chalk.green in printResult
and in the isStdoutPiped branch). Ensure the check uses the existing
globalOptions.colorDisabled flag and leave all other branching logic
(shouldOutputIdsOnly, printFormatted, isStdoutPiped) unchanged.
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 the gap reported in PR #36 follow-up: mutation commands (create / update / delete / archive / activate / follow / grant / dismiss / …) used to print a plain
console.log(chalk.green('✓ ...'))and ignore the global-o, --outputflag, soabs metric create … -o jsonreturned the green checkmark instead of JSON.Adds a shared
printResult(globalOptions, { message, id?, raw? })helper inapi-helper.tsthat all mutation sites now route through. Behavior by output format:table/rendered(default)chalk.green(message)on stdout — unchanged interactive UXjson/yaml/markdown/plain/vertical/template{ success, message, id? }viaprintFormatted— full API response when--rawidsidon stdout-oidon stdout for chaining + green confirmation on stderr~30 command files migrated. A few commands intentionally keep their multi-line interactive output (api key create showing the new key, reset-password, experiment create showing Name/Type, schedule create showing scheduled at) and only fall through to
printResultwhen a structured-ois requested.Out of scope (deferred to follow-up — they need a different design like NDJSON or array results):
experiments bulk(per-row updates)experiments export(download progress)experiments refresh-fields/estimate-participants(data display)experiments generate-template(file write success)auth,setup,doctorTest plan
printResulthelper has 11 dedicated unit tests covering every route (table, json, yaml, markdown, plain, vertical, template, ids, piped w/ id, piped w/o id, piped w/ explicit json, --raw with and without raw data).printFormattedon the success path (storageconfigs, actiondialogfields).npm test— 2387 passing, 4 skipped.npm run build— clean.npm run lint— clean.-o jsonreturns{"success":true,"message":"✓ …","id":N}, with--raw -o jsonreturns the full API response, default emits the green checkmark.Summary by CodeRabbit
Release Notes
New Features
Improvements