feat: address all 9 agent-readiness audit findings#10
Merged
Conversation
…, AGENTS.md) Address agent-readiness audit findings: - SA-4: Add --dry-run as global alias for --plan, covering all delete commands - TE-8: Add --brief flag for compact output (first 2 fields, ndjson format) - PV-1: Add --timeout flag to bound execution time via context deadline - SD-5: Add AGENTS.md with comprehensive agent context documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- SD-7: Add actionable error messages for timeout, cancellation, and connection errors with suggestions - SD-8: Group 39 top-level subcommands into 7 logical categories in help output (Identity, Devices, Security, Directory, Insights, AI, Config) - SA-2: Reject path traversal sequences in resource identifiers - SA-3: Reject control characters in resource identifiers - SA-5: Add --if-not-exists idempotency flag to users create and groups user create commands Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add --timeout to alias expansion's value-consuming flag list so 'jc --timeout 30s myalias' works correctly - Only fall through to creation on ResolveError in --if-not-exists; surface network errors and ambiguous matches instead of silently creating duplicates Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 33e0375. Configure here.
Handle 'foo/..', '\..', and standalone '..' in addition to '../' and '..\'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jrennichjc
approved these changes
Apr 13, 2026
jklaassenjc
pushed a commit
that referenced
this pull request
Jun 5, 2026
…tion (Bugbot KLA-439) Tenth and eleventh Bugbot findings on PR #42 (both Medium). **#10: --org flag source ignored.** root.go's PersistentPreRunE calls config.OverrideActiveProfile when --org is set, but collectProfile only knew about JC_PROFILE env / config / "default". An operator running `jc doctor --org staging` saw the right ActiveProfile but the source said "config (or default)" — the report lied about how the profile was selected. Fix: collectProfile takes a flagOrgSet bool; plumb the root command's --org .Changed bit through (same pattern as flagAPIKeySet for the api-key flag). **#11: probe-timeout vs parent-context attribution.** When the parent context's deadline fires first (global --timeout, signal cancel), my ctx.Done() branch always blamed --probe-timeout. Fix: peek at parentCtx.Err() in the timeout branch — if non-nil, the parent fired first, so the error message points at the global --timeout instead. After these two, every field doctor reports has a precedence attribution that matches what jc actually used. Same surgical class as finding #9 — bounded, no compounding cases possible because the field enumerations are exhausted. Tests: - TestCollectProfile_OrgFlagOverridesAll - TestRunAPIProbe_ParentContextDeadlineBlamedCorrectly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jklaassenjc
pushed a commit
that referenced
this pull request
Jun 5, 2026
…ot KLA-439) Twelfth Bugbot finding on PR #42 (Medium). Compound of finding #10. My fix to #10 detected --org via flag.Changed, but Changed is true even for an empty value: `--org=` (literal empty assignment), or `--org "$UNSET_VAR"` (shell expansion to empty). Root's PersistentPreRunE actually checks viper.GetString("org") != "" before calling OverrideActiveProfile, so with empty --org the profile stays unchanged but my doctor reported source as "--org flag" — a lie. Fix: at the flag-set decision point in newDoctorCmd, AND the .Changed bit with a non-empty viper value: flagOrgSet = f.Changed && strings.TrimSpace(viper.GetString("org")) != "" Same precedence mirror as root.go's PreRun. The fall-through then uses JC_PROFILE env / config / "default" as before. Test: - TestCollectProfile_EmptyOrgFlagFallsThrough — passes flagOrgSet=false (simulating the corrected caller decision), asserts source is NOT "--org flag". Per my closing commitment on this PR: if Bugbot finds a 13th, it goes to KLA-447 regardless of severity. This batch covers every field and every empty-flag-value edge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Addresses all 9 findings from the agent-readiness audit (score: B/77%):
Token Efficiency:
--briefflag for compact output (first 2 default fields, ndjson format)Self-Describing:
AGENTS.mdwith comprehensive agent context documentationSafety:
--dry-runas global alias for--plan, covering all 25 delete commands--if-not-existsidempotency flag tousers createandgroups user createPredictability:
--timeoutflag to bound execution time via context deadlineCloses #1, #2, #3, #4, #5, #6, #7, #8, #9
Test plan
make testpassesmake buildsucceeds--dry-runon a delete command shows plan output--briefon a list command shows compact 2-field ndjson--timeout 1mson a slow command exits with timeout error../../../etc/passwd) is rejectedjc --helpshows grouped command categoriesjc users create --if-not-existsreturns existing user🤖 Generated with Claude Code
Note
Medium Risk
Moderate risk: touches global CLI flag handling, output formatting, and identifier resolution, which can affect many commands’ behavior and error reporting. Changes are largely additive but may alter defaults/outputs in subtle ways (e.g.,
--briefJSON switching to NDJSON, stricter identifier validation).Overview
Improves agent/automation ergonomics across the CLI by adding global flags:
--brief(token-efficient output that trims to the first 2 default fields and emits NDJSON for JSON output),--dry-run(alias for--plan), and--timeout(applies a context deadline to commands).Adds idempotent creation via
--if-not-existsforjc users createandjc groups user create, returning the existing resource instead of failing when the name already exists.Hardens and clarifies behavior by rejecting path traversal/control characters in resource identifiers during resolution, improving error mapping for timeouts/cancellation/connectivity, and reorganizing top-level commands into Cobra help groups; also adds
AGENTS.mdas agent-oriented CLI reference documentation.Reviewed by Cursor Bugbot for commit bca2ed3. Bugbot is set up for automated code reviews on this repo. Configure here.