Skip to content

Commit bd7a8e4

Browse files
piekstrapiekstra
authored
refactor: build gro on the shared google-cli-common module (#168)
* refactor: build gro on the shared google-cli-common module Move gro's OAuth/credential/config/cache infrastructure, the Google API clients (gmail/calendar/contacts/drive/people), rendering and bulk helpers, and the shared cobra command packages (mail/init/config/setcred/refresh + root scaffolding) into github.com/open-cli-collective/google-cli-common, and consume them here. gro is now a thin CLI: its own domain commands (calendar/contacts/ drive/me), the read-only mail composition via the shared mailcmd, its identity (internal/appidentity) + config.Register in main, and its architecture tests. Behavior is unchanged and the non-destructive guarantee is preserved: the architecture test asserts appidentity.Scopes stays on the non-destructive allowlist and gro's tree contains no destructive API calls. Internal-only refactor; not a release. * docs: refresh architecture and domain-addition guides for the google-cli-common split Address the review finding on #168: docs/architecture.md, adding-a-domain.md, golden-principles.md, and development.md still described the pre-refactor tree (internal/auth, internal/gmail, internal/config, ...) and told contributors to add scopes in the deleted internal/auth/auth.go. Update them to the new seam: scopes in internal/appidentity, API clients + shared command packages in google-cli-common, dependency direction guaranteed by the module boundary. * test: enforce the non-destructive guarantee across the module boundary Address the review finding on #168: after moving the Google API clients into google-cli-common, TestNoDestructiveAPIMethodsInProductionCode only scanned gro's own tree, so the shared clients' non-destructive contract became prose-only. Add TestSharedGoogleClientsAreNonDestructive, which resolves the pinned google-cli-common module in the local cache (go list -m) and scans its gmail/calendar/contacts/drive/people packages for the same forbidden methods (.Send/.Trash/.Untrash/.BatchDelete). A future common release that adds one now fails gro's CI when gro bumps to it. Doc #5 updated to name the check.
1 parent 651f1ac commit bd7a8e4

166 files changed

Lines changed: 527 additions & 21679 deletions

File tree

Some content is hidden

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

‎cmd/gro/main.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,18 @@ import (
1212
"os/signal"
1313
"syscall"
1414

15+
"github.com/open-cli-collective/google-cli-common/config"
16+
17+
"github.com/open-cli-collective/google-readonly/internal/appidentity"
1518
"github.com/open-cli-collective/google-readonly/internal/cmd/root"
1619
)
1720

1821
func main() {
22+
// Register this CLI's identity before any config/keychain/auth call: it
23+
// stamps the config dir, keyring service, env-var prefixes, and scope set
24+
// the shared google-cli-common library resolves against.
25+
config.Register(appidentity.Identity())
26+
1927
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
2028
defer stop()
2129
root.ExecuteContext(ctx)

‎docs/adding-a-domain.md‎

Lines changed: 60 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,42 @@
11
# Adding a New Google API Domain
22

3-
This checklist covers adding a new Google API (e.g., Google Tasks, Google Sheets) to gro. The structural tests in `internal/architecture/architecture_test.go` automatically enforce steps marked with [enforced].
3+
This checklist covers adding a new Google API (e.g., Google Tasks, Google
4+
Sheets) to gro. Because gro is now a thin CLI on
5+
[`google-cli-common`](https://github.com/open-cli-collective/google-cli-common),
6+
adding a domain spans two modules: the reusable **API client** goes in
7+
google-cli-common (so grw and any future sibling can use it too), while the
8+
**command surface**, **scope registration**, and **structural-test wiring** stay
9+
here. Structural tests in `internal/architecture/architecture_test.go`
10+
automatically enforce steps marked [enforced].
411

512
## Checklist
613

7-
### 1. Add the OAuth scope
14+
### 1. Add the OAuth scope (this repo)
15+
16+
In `internal/appidentity/appidentity.go`, add the readonly scope to `Scopes`
17+
(and a human description to `ScopeDescriptions`):
818

9-
In `internal/auth/auth.go`, add the readonly scope to `AllScopes`:
1019
```go
11-
var AllScopes = []string{
12-
gmail.GmailReadonlyScope,
13-
calendar.CalendarReadonlyScope,
14-
people.ContactsReadonlyScope,
15-
drive.DriveReadonlyScope,
20+
var Scopes = []string{
21+
gmail.GmailModifyScope,
22+
// ...
1623
tasks.TasksReadonlyScope, // new
1724
}
1825
```
1926

20-
[enforced] Only `*ReadonlyScope` constants are permitted.
27+
[enforced] `TestAllScopesAreNonDestructive` requires every scope in
28+
`appidentity.Scopes` to be on the non-destructive allowlist. gro must never
29+
request a send/delete/settings scope.
2130

22-
### 2. Create the API client package
31+
### 2. Create the API client package (google-cli-common)
2332

24-
Create `internal/{domain}/` with:
33+
In the google-cli-common repo, create `{domain}/` with:
2534
- `client.go` — `Client` struct, `NewClient(ctx context.Context) (*Client, error)`, methods
2635
- Data model files as needed
2736
- `*_test.go` — Unit tests for parsing and data models
2837

29-
The constructor must follow the established pattern:
38+
The constructor follows the established pattern (auth lives in common):
39+
3040
```go
3141
func NewClient(ctx context.Context) (*Client, error) {
3242
client, err := auth.GetHTTPClient(ctx)
@@ -41,74 +51,69 @@ func NewClient(ctx context.Context) (*Client, error) {
4151
}
4252
```
4353

44-
[enforced] This package must NOT import any `internal/cmd/` package.
54+
Release a new google-cli-common version once the client lands, and bump the
55+
`require` in gro's `go.mod` to it.
4556

46-
### 3. Create the command package
57+
### 3. Create the command package (this repo)
4758

4859
Create `internal/cmd/{domain}/` with these files:
4960

5061
**`output.go`** — [enforced] Must contain:
5162
- An exported interface ending in `Client` (e.g., `TasksClient`)
52-
- A `ClientFactory` variable
53-
- A `newXClient()` wrapper function
54-
- Domain-specific text-rendering helpers (e.g., `printTask`, `printTaskSummary`)
63+
- A `ClientFactory` variable whose default calls the google-cli-common client:
64+
```go
65+
var ClientFactory = func(ctx context.Context) (TasksClient, error) {
66+
return tasks.NewClient(ctx) // tasks = google-cli-common/tasks
67+
}
68+
```
69+
- Domain-specific text-rendering helpers
5570

56-
> Do **not** add a package-local `printJSON()` helper. Per golden principle §4 (#144), resource-surface leaves emit text only. The `internal/output` package is reserved for control-plane envelopes (`refresh --json`, `config show --json`).
71+
> Do **not** add a package-local `printJSON()`. Per golden principle §4 (#144),
72+
> resource-surface leaves emit text only.
5773
58-
**`{domain}.go`** — [enforced] Must contain:
59-
- An exported `NewCommand()` function returning `*cobra.Command`
60-
- `AddCommand()` calls for all subcommands
74+
**`{domain}.go`** — [enforced] An exported `NewCommand()` returning
75+
`*cobra.Command`, with `AddCommand()` calls for all subcommands.
6176

62-
**Each subcommand file** — [enforced] Resource-surface leaves emit text only — they must NOT declare `--json/-j`. The structural test `TestResourceLeavesHaveNoJSONFlag` catches accidental re-introduction. Each leaf file contains:
63-
- Unexported `new{Sub}Command()` factory
64-
- Text rendering via the domain's printers from `output.go`
77+
**Each subcommand file** — [enforced] Resource-surface leaves emit text only;
78+
they must NOT declare `--json/-j` (`TestResourceLeavesHaveNoJSONFlag`).
6579

66-
### 4. Create test infrastructure
80+
**`main_test.go`** — a `TestMain` that registers gro's identity so config/
81+
keychain paths resolve in tests:
6782

68-
**`mock_test.go`** — Function-field mock with compile-time interface check:
6983
```go
70-
type MockTasksClient struct {
71-
ListTasksFunc func(ctx context.Context, ...) (...)
84+
func TestMain(m *testing.M) {
85+
config.Register(appidentity.Identity())
86+
os.Exit(m.Run())
7287
}
73-
74-
var _ TasksClient = (*MockTasksClient)(nil)
7588
```
7689

77-
**`handlers_test.go`** — Test helpers using centralized utilities:
78-
```go
79-
func withMockClient(mock TasksClient, f func()) {
80-
testutil.WithFactory(&ClientFactory, func(_ context.Context) (TasksClient, error) {
81-
return mock, nil
82-
}, f)
83-
}
84-
85-
func withFailingClientFactory(f func()) {
86-
testutil.WithFactory(&ClientFactory, func(_ context.Context) (TasksClient, error) {
87-
return nil, errors.New("connection failed")
88-
}, f)
89-
}
90-
```
90+
### 4. Create test infrastructure
9191

92-
Use `testutil.CaptureStdout(t, func() { ... })` for output capture.
92+
**`mock_test.go`** — Function-field mock with a compile-time interface check.
93+
**`handlers_test.go`** — `withMockClient` / `withFailingClientFactory` using the
94+
centralized `testutil.WithFactory` (from google-cli-common). Capture output with
95+
`testutil.CaptureStdout`.
9396

94-
### 5. Add test fixtures
97+
### 5. Add test fixtures (google-cli-common)
9598

96-
In `internal/testutil/fixtures.go`, add `SampleX()` functions for the new API types.
99+
In google-cli-common's `testutil/fixtures.go`, add `SampleX()` functions for the
100+
new API types.
97101

98-
### 6. Register the domain command
102+
### 6. Register the domain command (this repo)
99103

100104
In `internal/cmd/root/root.go`, add:
105+
101106
```go
102-
cmd.AddCommand(tasks.NewCommand())
107+
rootCmd.AddCommand(tasks.NewCommand())
103108
```
104109

105-
### 7. Update structural test registration
110+
### 7. Update structural test registration (this repo)
106111

107-
In `internal/architecture/architecture_test.go`, add the new domain to:
108-
- `domainPackages` slice (e.g., `"tasks"`)
109-
- `apiClientPackages` slice (e.g., `"tasks"`)
110-
- `domainCommands()` map
112+
In `internal/architecture/architecture_test.go`, add the new domain to the
113+
`domainPackages` slice and the `domainCommands()` map.
111114

112115
### 8. Verify
113116

114-
Run `make check`. The structural tests will catch any missing patterns.
117+
Run `make check`. The structural tests catch any missing patterns. If a new
118+
google-cli-common client was added, make sure `go.mod` requires the version that
119+
contains it.

‎docs/architecture.md‎

Lines changed: 61 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,79 @@
11
# Architecture
22

3+
gro is a thin CLI built on the shared
4+
[`google-cli-common`](https://github.com/open-cli-collective/google-cli-common)
5+
module. Almost everything reusable — the Google OAuth flow, the API clients,
6+
credential/config/cache state, rendering and bulk helpers, and the shared cobra
7+
command packages — lives in google-cli-common and is consumed here as a
8+
dependency. This repo contains only what is gro-specific: its identity, its own
9+
domain commands, and its structural tests.
10+
311
## Dependency Graph
412

513
```
614
cmd/gro/main.go
7-
-> internal/cmd/root/
8-
-> internal/cmd/mail/ (MailClient interface + ClientFactory)
15+
-> config.Register(appidentity.Identity()) // stamps DirName, scopes, product name
16+
-> internal/cmd/root/ // root command + rootutil global-flag wiring
17+
# shared command packages (google-cli-common):
18+
-> mailcmd/ (mail: search/read/archive/label/move/... )
19+
-> initcmd/ (OAuth setup wizard)
20+
-> configcmd/ (config management)
21+
-> setcred/ (set-credential)
22+
-> refreshcmd/ (cache refresh)
23+
# gro's own domain command packages (this repo):
924
-> internal/cmd/calendar/ (CalendarClient interface + ClientFactory)
1025
-> internal/cmd/contacts/ (ContactsClient interface + ClientFactory)
1126
-> internal/cmd/drive/ (DriveClient interface + ClientFactory)
1227
-> internal/cmd/me/ (PeopleClient interface + ClientFactory)
13-
-> internal/cmd/initcmd/ (OAuth setup wizard)
14-
-> internal/cmd/config/ (Credential management)
15-
16-
Each cmd/ package depends on its API client:
17-
internal/cmd/mail/ -> internal/gmail/
18-
internal/cmd/calendar/ -> internal/calendar/
19-
internal/cmd/contacts/ -> internal/contacts/
20-
internal/cmd/drive/ -> internal/drive/
21-
internal/cmd/me/ -> internal/people/
22-
23-
All API clients depend on:
24-
internal/auth/ -> internal/keychain/, internal/config/
25-
26-
Shared utilities (no internal deps):
27-
internal/bulk/ Bulk operation ID resolution and result types
28-
internal/testutil/ Test fixtures and assertion helpers
29-
internal/output/ JSON output encoding
30-
internal/format/ Human-readable formatting
31-
internal/errors/ Error types
32-
internal/log/ Logging
33-
internal/cache/ Response caching
34-
internal/view/ Small Success/Error/Info/Printf/Println helper used by initcmd
35-
internal/zip/ Secure zip extraction
36-
internal/version/ Build-time version injection
28+
29+
gro's domain command packages depend on google-cli-common's API clients:
30+
internal/cmd/calendar/ -> google-cli-common/calendar
31+
internal/cmd/contacts/ -> google-cli-common/contacts
32+
internal/cmd/drive/ -> google-cli-common/drive
33+
internal/cmd/me/ -> google-cli-common/people
34+
(mailcmd, in common, uses google-cli-common/gmail)
35+
36+
Provided by google-cli-common (imported, not in this repo):
37+
config, keychain, auth Config, OS-keyring token storage, OAuth flow
38+
gmail/calendar/contacts/drive/people API clients and data models
39+
bulk, output, format, view, errors, log, cache, zip, version Helpers
40+
testutil, credtest, migrationsink Test fixtures, hermetic creds, migration sink
3741
```
3842

3943
## Data Flow
4044

4145
```
42-
User -> cobra command -> ClientFactory(ctx) -> API Client -> auth.GetHTTPClient -> Google API
43-
|
44-
internal/{gmail,calendar,contacts,drive,people}/
46+
User -> cobra command -> ClientFactory(ctx) -> google-cli-common API client
47+
-> google-cli-common/auth.GetHTTPClient (keyring token, resolved via the
48+
registered config.Identity) -> Google API
4549
```
4650

51+
## The identity seam
52+
53+
Everything CLI-specific is funneled through `config.Identity` (defined in
54+
google-cli-common), which `cmd/gro/main.go` registers exactly once at startup
55+
via `config.Register(appidentity.Identity())`, before any config/keychain/auth
56+
call. `DirName` alone drives the config/cache directory, the keyring service
57+
segment, and the derived `<SERVICE>_KEYRING_BACKEND` /
58+
`<SERVICE>_KEYRING_PASSPHRASE` / `<SERVICE>_CREDENTIAL_REF` env vars. gro's
59+
identity — its dir name, default credential ref, product name, and OAuth scope
60+
set — lives in `internal/appidentity/appidentity.go`.
61+
4762
## Package Responsibilities
4863

4964
| Package | Responsibility |
5065
|---------|---------------|
51-
| `cmd/gro/` | Entry point, calls `root.NewCommand()` |
52-
| `internal/cmd/root/` | Root cobra command, registers all domain commands |
53-
| `internal/cmd/{domain}/` | Command handlers, client interface, output formatting |
54-
| `internal/{gmail,calendar,contacts,drive,people}/` | API client, data models, response parsing |
55-
| `internal/auth/` | OAuth2 config loading, HTTP client creation |
56-
| `internal/keychain/` | Platform-specific secure token storage |
57-
| `internal/testutil/` | Test assertions, fixtures, helpers |
66+
| `cmd/gro/` | Entry point: registers `appidentity.Identity()`, then runs `root.ExecuteContext` |
67+
| `internal/appidentity/` | gro's `config.Identity`: dir name, default ref, product name, OAuth scopes (+ descriptions) |
68+
| `internal/cmd/root/` | Root cobra command; registers the shared command packages and gro's domain commands; global-flag wiring delegated to `google-cli-common/rootutil` |
69+
| `internal/cmd/{calendar,contacts,drive,me}/` | gro's own domain command handlers, client interface, and output formatting |
5870
| `internal/architecture/` | Structural tests enforcing codebase conventions |
71+
| `internal/noleak/` | End-to-end secret no-leak tests |
72+
| `google-cli-common/*` | Everything shared: OAuth/credential/config infra, API clients, rendering/bulk helpers, and the `mailcmd`/`initcmd`/`configcmd`/`setcred`/`refreshcmd`/`rootutil` command packages |
5973

6074
## File Naming Conventions
6175

62-
Each domain command package (`internal/cmd/{domain}/`) contains:
76+
Each gro-owned domain command package (`internal/cmd/{domain}/`) contains:
6377

6478
| File | Purpose |
6579
|------|---------|
@@ -68,16 +82,18 @@ Each domain command package (`internal/cmd/{domain}/`) contains:
6882
| `{subcommand}.go` | One file per subcommand with `new{Sub}Command()` factory |
6983
| `mock_test.go` | Mock client with function fields + compile-time interface check |
7084
| `handlers_test.go` | `withMockClient()`, `withFailingClientFactory()`, integration tests |
85+
| `main_test.go` | `TestMain` registering `appidentity.Identity()` so config/keychain paths resolve |
7186
| `*_test.go` | Additional unit tests |
7287

73-
Each API client package (`internal/{domain}/`) contains:
74-
75-
| File | Purpose |
76-
|------|---------|
77-
| `client.go` | `Client` struct, `NewClient(ctx)`, client methods |
78-
| Additional `.go` | Data models, parsing helpers |
79-
| `*_test.go` | Unit tests |
88+
The API client packages (`gmail`, `calendar`, `contacts`, `drive`, `people`)
89+
now live in google-cli-common; see that repo for their `client.go` /
90+
`NewClient(ctx)` conventions.
8091

8192
## Structural Enforcement
8293

83-
Architectural invariants are enforced by tests in `internal/architecture/architecture_test.go`. These run as part of `make check` and CI. See `docs/golden-principles.md` for the rules being enforced.
94+
Architectural invariants are enforced by tests in
95+
`internal/architecture/architecture_test.go`. These run as part of `make check`
96+
and CI. See `docs/golden-principles.md` for the rules being enforced. Note that
97+
the dependency-direction invariants (API clients never import cmd; auth never
98+
imports API clients) are now guaranteed by the module boundary — those packages
99+
live in google-cli-common, which cannot import this module's `internal/`.

‎docs/development.md‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,17 @@ This is the repo-local source for working on `gro`. It contains google-readonly-
66

77
Binary: `gro`
88
Module: `github.com/open-cli-collective/google-readonly`
9-
Entrypoint: `cmd/gro/main.go`
9+
Entrypoint: `cmd/gro/main.go` (registers `appidentity.Identity()` via `config.Register`, then runs the root command)
1010
Root command: `internal/cmd/root`
1111

12+
`gro` is a thin CLI on the shared
13+
[`google-cli-common`](https://github.com/open-cli-collective/google-cli-common)
14+
module, which provides the OAuth flow, the Google API clients, credential/config
15+
state, and the shared command packages (mail/init/config/setcred/refresh + root
16+
scaffolding). This repo owns gro's identity (`internal/appidentity`), its own
17+
domain commands (`internal/cmd/{calendar,contacts,drive,me}`), and its structural
18+
tests. See `docs/architecture.md`.
19+
1220
`gro` is a non-destructive command-line interface for Google services. It supports read access plus non-destructive organization operations such as labeling, archiving, starring, marking read or unread, RSVP/color operations, group membership, and drafts that are never sent automatically. No send, delete, trash, or destructive file/contact/event operations should be exposed.
1321

1422
## Repo-Local Sources
@@ -88,7 +96,7 @@ make install
8896

8997
## Core Constraints
9098

91-
- OAuth scopes live in `auth.AllScopes` and must remain on the non-destructive allowlist enforced by structural tests.
99+
- OAuth scopes live in `appidentity.Scopes` (registered via `config.Register` in `main`) and must remain on the non-destructive allowlist enforced by structural tests.
92100
- Production code must not call destructive Google API methods such as send, trash, untrash, or batch delete.
93101
- Each `internal/cmd/{domain}` package defines its own client interface in `output.go`.
94102
- Each domain command package exposes a `ClientFactory` variable for test injection.
@@ -110,16 +118,17 @@ Secret ingress belongs in setup and credential-management commands only. For the
110118

111119
## Testing Notes
112120

113-
Run repo sanity with `make check`. Structural tests live in `internal/architecture/architecture_test.go`; fixtures and assertions live in `internal/testutil`.
121+
Run repo sanity with `make check`. Structural tests live in `internal/architecture/architecture_test.go`; fixtures and assertions live in `google-cli-common/testutil` (shared with grw).
114122

115123
Mock clients use function fields plus compile-time interface checks. Test helpers such as `testutil.WithFactory`, `testutil.CaptureStdout`, `testutil.Equal`, and `testutil.NoError` are the default local patterns.
116124

117125
## Dependencies
118126

127+
- `github.com/open-cli-collective/google-cli-common` — the shared Google layer: OAuth flow, API clients (Gmail/Calendar/Contacts/Drive/People), credential/config/cache state, rendering/bulk helpers, and the shared command packages. Bump its `require` version when consuming new common features.
119128
- `github.com/spf13/cobra` for the command surface.
120-
- `golang.org/x/oauth2` for OAuth2 client behavior.
121-
- `google.golang.org/api/*` for Gmail, Calendar, People, and Drive APIs.
122-
- `github.com/open-cli-collective/cli-common` for credential storage and state directory behavior.
129+
- `golang.org/x/oauth2` for OAuth2 client behavior (via google-cli-common).
130+
- `google.golang.org/api/*` for the Gmail, Calendar, People, and Drive APIs (via google-cli-common; used directly here only for OAuth scope constants in `appidentity`).
131+
- `github.com/open-cli-collective/cli-common` for credential storage and state directory behavior (via google-cli-common).
123132
- `gopkg.in/yaml.v3` for `config.yml`.
124133

125134
## Common Issues

0 commit comments

Comments
 (0)