ship it

The change correctly limits advertised --json support on leaf commands, but it introduces a CLI compatibility regression for valid commands when users place the former persistent flag at the group level.
Review comment:
- [P2] Preserve group-level --json placement — /Users/gtrrz-victor/wks/cli/cli/cmd/entire/cli/corecmd.go:42-47
With
--jsonregistered only as a local leaf flag, Cobra recognizes it only after the final command name. Existing invocations that used the previously persistent group flag, such asentire repo --json listorentire repo mirror --json list, now fail withunknown flag: --jsoneven though those target commands still honor JSON output. If the intent is only to reject no-op flags on side-effect verbs, this needs a compatibility path for honoring subcommands rather than moving the flag exclusively to local flags.
Let's tal about P2, that expected bahaviour is something other cli written on cobra expect to work in that way ? Specify the flag at root command even thought root command does not understand the flag?
I’ll keep this at the Cobra semantics level first, because the review point is really about flag ownership and placement rather than JSON itself.
Yes, in Cobra that is expected only for persistent flags, not arbitrary leaf flags.
The subtle point: entire repo --json list is not really “root command has a flag it does not understand.” It means the repo group used to understand --json as a persistent flag inherited by its children.
Cobra roughly behaves like this:
works if --json is registered on repo.PersistentFlags().
works if --json is registered on list.Flags() or inherited from a parent.
But if --json exists only on the list leaf command, then this fails:
because Cobra is still parsing flags for repo when it sees --json, and repo no longer has that flag.
So the P2 is about compatibility with the previous Cobra contract. If users had a persistent group flag before, they could reasonably place it at the group level. Moving it exclusively to leaf-local flags removes that valid placement.
The compatibility-preserving shape is usually:
- keep a hidden/deprecated persistent
--jsonon the group for parsing compatibility - only honor it for descendants that actually support JSON
- reject or ignore it on side-effect commands according to the intended behavior
I would not make entire --json repo list work unless --json was previously a root persistent flag. That would be a broader global flag contract.
without thinking on compatiblity, what behaviour your think is more straighforward from a brand new user ?
For a brand-new user, the most straightforward behavior is:
The flag should live on the command whose output it changes. That maps cleanly to “run repo list, but format that result as JSON.”
I would not expect a new user to naturally write:
That is a Cobra-supported pattern for inherited persistent flags, but it is less obvious because the flag appears before the action it affects. It also gets confusing when some subcommands under repo support JSON and others do not.
So ignoring compatibility, I’d prefer:
entire repo list --json: supportedentire repo mirror list --json: supportedentire repo --json list: probably not supportedentire --json repo list: not supported unless JSON is truly global for every command
The clean mental model is: flags belong as close as possible to the verb/output they modify.
thanks! Check if any of the commands that now specify json flag it is really implemented
I’ll inspect where --json is registered now and trace each registered command to confirm it actually switches output format, not just advertises the flag.
The broad search is too noisy because this repo has many JSON structs. I’m narrowing to Cobra flag registrations and the helper paths those flags feed.
The new control-plane registration points are easy to isolate: addJSONFlag(cmd) appears only on org/project/repo/grant/mirror commands. I’m checking each path now against the run helper it calls.
So far every addJSONFlag I’ve traced is either using runCoreList, runCoreObject, or runCoreMutation, each of which branches on jsonRequested(cmd). I’m also checking the structural test and full command tree for accidental leaf flags outside that helper.
There are existing focused tests for the structural map plus list/mutation JSON behavior. I’m running those now so the answer is tied to both code inspection and test coverage.
I checked the new control-plane addJSONFlag(cmd) registrations. I did not find any command that advertises --json without implementing it.
All of these route through helpers that branch on jsonRequested(cmd):
runCoreList/renderCoreList: emits JSON arraysrunCoreObject/renderCoreObject: emits JSON objectrunCoreMutation: emits the wire object instead of the human success line
Covered commands:
org create/list/get, project create/list/get, repo create/list/get, repo mirror list/get, repo mirror collaborators list, repo visibility get/set, grant org add/list, grant project add/list, grant repo add/list.
The intentionally non-JSON commands still do not register it: delete, repo clone, repo mirror create/remove, and grant * remove.
I also ran the focused tests:
Passed.
Small cleanup note: a couple of test comments still say --json is persistent on the group root, but the assertions use the current leaf placement.
I’ll update the stale test comments only; the behavior and assertions are already correct.
I found two stale comments, both in focused tests. I’m changing them to describe why the test drives the full group command without claiming the flag is persistent.
The stale wording is fixed. I’ll run the narrow tests that touch those files to catch any accidental edit issue.
Fixed the stale comments in corecmd_list_test.go and corecmd_mutation_test.go. They now describe the leaf-local --json flag correctly.
Verification passed:
I’m using the ship-it skill because this is a commit/push/PR request. I’ll read its local instructions, then verify the current tree and ship only the relevant changes.
I’ll first confirm the branch and exact diff so the commit only contains the comment cleanup we just made, plus any existing user changes if they are already part of this branch’s work.
The tree only has the two comment edits, both in tests. I’ve already run the narrow test after the edit; before pushing I still need to run the repo-required lint check.
mise run lint is failing before it checks code: the local golangci-lint binary is v1, while this repo’s config is v2. I’m checking the repo’s mise config to use the intended toolchain instead of pushing past a tooling failure.
The correct golangci-lint 2.11.3 is installed through mise, but PATH is picking an older Go-bin copy first. I’m checking the lint task’s invocation so I can run it with the intended mise tool path.
The task script calls golangci-lint by name. Under mise run, that resolves to the stale Go-bin binary; under mise exec, it resolves to 2.11.3. I’ll run the lint task through mise exec with the same repo task so the checks execute against the configured version.
The wrapper behavior is odd enough that I’m going to bypass the stale PATH entry directly for the Go lint portion and still run the same underlying checks. Then I’ll confirm the tree stayed clean before committing.
The installed binary is nested under mise’s extracted package directory. I’ll use that absolute binary for the Go lint checks and run the other lint subtasks normally where they already passed.
The non-Go lint subtasks completed during the failed mise run lint; only the Go linter stopped because it resolved v1. I’m now running golangci-lint 2.11.3 directly with the repo’s config.
The configured Go linter passed with zero issues. I’m checking the tree again, then I’ll stage and commit the two comment edits.
The linter did not change anything. I’m staging just those two test files and committing them with a small, direct subject.
Before pushing, I’m rerunning the full mise run lint with the configured golangci-lint 2.11.3 path placed before the stale Go-bin copy, so the required command itself can complete cleanly.
mise run still prepends Go’s bin directory ahead of the dedicated golangci install, so the forced PATH did not take. I’m checking whether there’s a mise invocation mode that orders the dedicated tool correctly before I decide whether to treat this as a local tooling blocker.
mise x golangci-lint@2.11.3 resolves correctly. I’m trying the equivalent mise run tool override next; if that still inherits the stale Go-bin binary, I’ll keep the direct v2 lint result and report the local mise run quirk in the PR verification.
mise run -t golangci-lint@2.11.3 lint now picks the correct linter but fails when config verification tries to fetch golangci’s JSON schema, which is blocked by the restricted network. Per the sandbox rules, I’m requesting one escalated rerun for that lint command.
The required lint command passed with the correct linter after network approval. I’m checking whether lint changed the tree, then I’ll push the committed branch.
The tree is clean and the branch already tracks origin/move-json-flag-to-specific-commands. I’m pushing the new commit now.
The branch pushed cleanly. I’m checking the current PR and will update its body if the new commit makes it stale or incomplete.
Shipped.
Pushed commit 2c1573ece to origin/move-json-flag-to-specific-commands on existing PR #1680:
https://github.com/entireio/cli/pull/1680
Verification run:
Note: plain mise run lint initially picked up a stale Go-bin golangci-lint v1.64.8. The tool override used the repo-configured v2.11.3 and passed cleanly.