Feat/agent ready cli - #114
Conversation
📝 WalkthroughWalkthroughThe CLI adds an ChangesCLI contracts and rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant Manifest
participant CLI
participant Output
Agent->>Manifest: Build read-only command catalog
Manifest-->>Agent: Return contract metadata
Agent->>Output: Serialize text or JSON
CLI->>Output: Render structured execution errors
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@cmd/diagnostics_test.go`:
- Around line 24-26: Restore the persistent interactive flag after each test: in
cmd/diagnostics_test.go lines 24-26, save query’s existing interactive value
before setting it and restore that value during test cleanup; apply the same
save-and-restore pattern to promqlRunCmd at lines 47-49. Use test cleanup so
restoration occurs even when assertions fail.
In `@cmd/output.go`:
- Around line 52-60: Wrap every malformed server-response error in CLIError with
Code set to ErrorInvalidResponse so errorDetails emits INVALID_RESPONSE: update
the invalid and multiple-value branches in cmd/output.go (lines 52-60), and the
invalid decoding paths in cmd/promql.go at lines 351-353, 429, 485, 549, 623,
678, 779, 834, 903, and 922; preserve each existing descriptive error message
and underlying error.
In `@cmd/pre.go`:
- Around line 42-43: Update the profile validation around conf.Profiles and
conf.DefaultProfile to verify that the configured default-profile name exists as
a key in conf.Profiles before assigning DefaultProfile. Return the existing
ErrorNotFound CLI error when the map is nil or the configured profile is absent,
rather than accepting a zero-value profile.
In `@cmd/role.go`:
- Around line 169-171: Update the missing-role error path in the command
handling logic to return newCLIError with ErrorNotFound and
missingRoleMessage(name, roles), passing nil for the underlying cause; preserve
the existing cmd.Annotations["errors"] assignment using the resulting error
message.
In `@cmd/user.go`:
- Around line 115-117: Update all three missing-resource branches in
cmd/user.go: the missing role at lines 115-117, the missing user at lines
230-232, and the missing role at lines 245-247. Replace each untyped error
return with newCLIError(ErrorNotFound, ..., nil), preserving the existing
descriptive messages and command annotations so JSON consumers receive the
NOT_FOUND contract.
- Around line 365-377: Check userRoleFetchError before constructing or writing
the JSON response in the outputJSON branch. If role enrichment fails, return
that error immediately so writeJSON is never called; only serialize
usersWithRoles after confirming all role requests succeeded.
In `@main.go`:
- Around line 55-64: Resolve rootOutputFormat before the version branch in
main.go and pass the effective format plus command output writer to
PrintVersion. Update cmd/version.go’s PrintVersion implementation to accept
these parameters and stop reading the package-level outputFormat, preserving
JSON output for --version --output=json. Affected sites: main.go lines 55-64
require reordering and argument passing; cmd/version.go lines 58-88 require the
PrintVersion signature and implementation update.
In `@pkg/config/config.go`:
- Around line 184-195: Update the config file-writing flow around file.Chmod and
file.Write to remove direct os.Stderr diagnostics and return contextual wrapped
errors instead. Preserve the underlying errors when reporting permission-setting
or data-writing failures, allowing the command-level error renderer to choose
the output stream and format.
In `@pkg/iterator/iterator.go`:
- Around line 81-84: Update QueryIterator.populateNextNonEmpty so hasData is
never invoked while iter.mu is held: snapshot the needed iterator state under
the lock, mark population in flight to prevent duplicate work, release the lock
for the callback, then reacquire it to commit results and clear the in-flight
state. Preserve synchronization for concurrent iterator methods and ensure
waiting callers are notified or can retry after population completes.
In `@README.md`:
- Around line 346-348: Update the agent catalog description in README.md to
match the fields actually emitted by agentManifest: describe command strings,
scope, profile requirements, and constraints, and remove claims about structured
arguments, flags, and required permissions unless those fields are added to
agentManifest.
🪄 Autofix
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: dabfac83-f32b-4ca5-99e5-f49a1e435a07
📒 Files selected for processing (31)
README.mdcmd/agent.gocmd/agent_test.gocmd/cloud.gocmd/dataset.gocmd/diagnostics_test.gocmd/errors.gocmd/errors_test.gocmd/exit_status_test.gocmd/generate.gocmd/output.gocmd/output_test.gocmd/pre.gocmd/profile.gocmd/promql.gocmd/promql_test.gocmd/query.gocmd/queryList.gocmd/queryList_test.gocmd/role.gocmd/status.gocmd/tail.gocmd/user.gocmd/version.gomain.gomain_test.gopkg/analytics/analytics.gopkg/config/config.gopkg/datasets/datasets.gopkg/iterator/iterator.gopkg/model/savedQueries.go
| commandErr := fmt.Errorf("%s", missingRoleMessage(name, roles)) | ||
| cmd.Annotations["errors"] = commandErr.Error() | ||
| return commandErr |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use a typed not-found error.
This path returns a generic error. It has no explicit NOT_FOUND code for JSON error output. Return newCLIError(ErrorNotFound, missingRoleMessage(name, roles), nil) instead.
🤖 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 `@cmd/role.go` around lines 169 - 171, Update the missing-role error path in
the command handling logic to return newCLIError with ErrorNotFound and
missingRoleMessage(name, roles), passing nil for the underlying cause; preserve
the existing cmd.Annotations["errors"] assignment using the resulting error
message.
Summary
Makes
pbsafer and more predictable for agents, scripts, and CI while preserving the existing human-readable CLI experience.Summary by CodeRabbit
New Features
agentcommand that provides machine-readable help, read-only command discovery, permissions, arguments, constraints, and error details.Bug Fixes