Skip to content

Commit 520596a

Browse files
authored
gro: remove per-command --json (BREAKING) [#144] (#147)
* feat!: remove per-command --json from resource leaves JSON is reserved for round-trip payloads and control-plane envelopes per cli-common docs/output-and-rendering.md §2. Today's surviving JSON surfaces are 'gro refresh --json' (§4.6, added in #146) and 'gro config show --json' (diagnostic). Every resource-surface leaf under mail/, calendar/, contacts/, drive/, me/ now emits text only; running 'gro mail search foo --json' returns cobra's 'unknown flag: --json'. The boundary is uniform across resource reads AND mutation-success renderers — mail archive/label/markread/star/categorize/draft, calendar color/rsvp, contacts group_manage/star, drive star all lose their --json flag along with the read commands. Architecture: inverts golden principle #4 + the structural test that enforced it. Pre-#144: every leaf had to declare --json/-j (with two binary-write exemptions). Post-#144: every resource leaf must NOT declare --json — caught by TestResourceLeavesHaveNoJSONFlag. The jsonExemptCommands allowlist is gone (empty after the inversion). Helper cleanup: bulk.Result.Print(jsonOutput bool) → Print() (text only); the four package-local printJSON helpers in mail/calendar/contacts/drive output.go are deleted; me/output.go's RenderJSON + json-shaped structs are deleted. shared internal/output helpers stay (refresh + config show consume them). Tests: per-package 'has json flag' subtests, every Test*_JSON function, and noleak's me --json variants are removed; config show --json no-leak coverage stays. BREAKING CHANGE: 'gro <resource> --json' now errors. Scripts that piped 'gro ... --json | jq' must move to text parsing or use one of the surviving JSON surfaces (gro refresh --json, gro config show --json). Closes #144 * docs(adding-a-domain): align with #144 — text-only resource leaves The pre-#144 instructions required output.go to contain a printJSON() helper and every leaf to declare --json/-j. Both are inverted by the new policy: resource-surface leaves emit text only; JSON is reserved for control-plane envelopes outside the domain resource packages (refresh, config show). Updated the section so a future domain author follows the §4 rule rather than the removed surface. * docs: drop stale printJSON() references after #144 Two doc tails referenced the removed printJSON() helper: - docs/architecture.md file-naming table - internal/output/output.go's JSON() docstring referring to 'per-domain printJSON() callers' (those are gone; the only surviving JSONStdout callers are control-plane refresh + config show). * test: end-to-end resource --json rejection + config show --json carve-out Two TDD-suggested coverage adds: - TestResourceLeaf_RejectsJSON_EndToEnd dispatches 'gro drive drives --json' through cobra and asserts the user-visible 'unknown flag' error. The static guard catches missing flag declarations; this one catches a hypothetical regression where the flag is declared but the structural test's domain set is bypassed. - TestConfigShowCommand: assert the --json control-plane flag is declared with -j shorthand. config show --json had no-leak coverage in internal/noleak but no shape coverage; this pins the carve-out surface itself. * docs+test: align stale references; consolidate no-json assertion Five daemon findings on PR #147: 1. The repo's agent-guidance markdown §Key Constraints still required '--json on all leaf commands' — directly contradicts the post-#144 inversion and would mislead future contributors. Updated to mirror the new text-only rule. 2. internal/architecture E2E test comment overstated protection (claimed to catch a domain-set bypass when it shares the same list). Reworded as a spot-check complement that explicitly notes neither test guards against domainPackages drift. 3. docs/golden-principles.md §4 grew a 'carve-out criteria' paragraph articulating what qualifies a command as a control-plane carve-out (outside domain resource packages + envelope/diagnostic). 4. Removed the two duplicate per-command 'no json flag' subtests (drives_test.go, archive_test.go) for consistency — global guard in TestResourceLeavesHaveNoJSONFlag covers all leaves uniformly.
1 parent 1103d84 commit 520596a

79 files changed

Lines changed: 186 additions & 1967 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ make install # Install to /usr/local/bin
4040
- **Non-destructive by design**: Only allowlisted scopes in `auth.AllScopes`. No destructive API methods (send, delete, trash). Non-destructive modify operations (label, archive, star) are permitted.
4141
- **Interface-at-consumer**: Each `internal/cmd/{domain}/output.go` defines its client interface.
4242
- **ClientFactory DI**: Swappable factory for test mock injection.
43-
- **--json on all leaf commands**: Every leaf subcommand supports `--json/-j`.
43+
- **Text-only resource leaves (#144)**: Resource-surface leaf commands (under `mail`, `calendar`, `contacts`, `drive`, `me`) emit text only — they must NOT declare `--json/-j`. JSON is reserved for control-plane envelopes outside the domain resource packages: `gro refresh --json` (§4.6) and `gro config show --json` (diagnostic).
4444
- **Structural enforcement**: `internal/architecture/architecture_test.go` enforces all patterns at CI time.
4545

4646
See `docs/golden-principles.md` for the full set of enforced rules.

0 commit comments

Comments
 (0)