Skip to content

Commit 477f5d9

Browse files
Justin Poehneltjpoehnelt-botgemini-code-assist[bot]
authored
docs: add helper command guidelines and anti-patterns (#598)
* docs: add helper command guidelines and anti-patterns * Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: jpoehnelt-bot <jpoehnelt-bot@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 834fa7d commit 477f5d9

3 files changed

Lines changed: 99 additions & 57 deletions

File tree

.gemini/style_guide.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,7 @@ Examples of scope creep to avoid:
3333
## Severity Calibration
3434

3535
Mark issues as **critical** only when they cause data loss, security vulnerabilities, or incorrect behavior under normal conditions. Theoretical failures in infallible system APIs (e.g., `tokio::signal::ctrl_c()` registration) are **low** severity — do not label them critical. Contradicting a prior review suggestion (e.g., suggesting `expect()` then flagging `expect()` as wrong) erodes trust; verify consistency with earlier comments before posting.
36+
37+
## Helper Commands (`+verb`)
38+
39+
Helpers are handwritten commands that provide value Discovery-based commands cannot: multi-step orchestration, format translation, or multi-API composition. **Do not accept helpers that wrap a single API call, add flags to expose data already in the API response, or re-implement Discovery parameters as custom flags.** See [`src/helpers/README.md`](../src/helpers/README.md) for full guidelines and anti-patterns.

AGENTS.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ When a user-supplied string is used as a GCP resource identifier (project ID, to
137137

138138
```rust
139139
// Validates the string does not contain path traversal segments (`..`), control characters, or URL-breaking characters like `?` and `#`.
140-
let project = crate::helpers::validate_resource_name(&project_id)?;
140+
let project = crate::validate::validate_resource_name(&project_id)?;
141141
let url = format!("https://pubsub.googleapis.com/v1/projects/{}/topics/my-topic", project);
142142
```
143143

@@ -166,6 +166,15 @@ Use these labels to categorize pull requests and issues:
166166
- `area: auth` — OAuth, credentials, multi-account, ADC
167167
- `area: skills` — AI skill generation and management
168168

169+
## Helper Commands (`+verb`)
170+
171+
Helpers are handwritten commands prefixed with `+` that provide value the schema-driven Discovery commands cannot: multi-step orchestration, format translation (e.g., Markdown → Docs JSON), or multi-API composition.
172+
173+
> [!IMPORTANT]
174+
> **Do NOT add a helper that** wraps a single API call already available via Discovery, adds flags to expose data already in the API response, or re-implements Discovery parameters as custom flags. Helper flags must control orchestration logic — use `--params` and `--format`/`jq` for API parameters and output filtering.
175+
176+
See [`src/helpers/README.md`](src/helpers/README.md) for full guidelines, anti-patterns, and a checklist for new helpers.
177+
169178
## Environment Variables
170179

171180
### Authentication

src/helpers/README.md

Lines changed: 85 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,95 @@
1-
# Helper Modules
1+
# Helper Commands (`+verb`) — Guidelines
22

3-
This directory contains "Helper" implementations that provide high-value, simplified commands for complex Google Workspace API operations.
3+
## Design Principle
44

5-
## Philosophy
5+
The core design of `gws` is **schema-driven**: commands are dynamically generated from Google Discovery Documents at runtime. This avoids maintaining a hardcoded, unbounded argument surface. **Helpers must complement this design, not duplicate it.**
66

7-
The goal of the `gws` CLI is to provide raw access to the Google Workspace APIs. However, some operations are common but complex to execute via raw API calls (e.g., sending an email, appending a row to a sheet).
7+
## When a Helper is Justified
88

9-
**Helper commands should only be added if they offer "High Usefulness":**
9+
A `+helper` command should exist only when it provides value that Discovery-based commands **cannot**:
1010

11-
* **Complex Abstraction:** Does it abstract away significant complexity (e.g., MIME encoding, complex JSON structures, multiple API calls)?
12-
* **Format Conversion:** Does it handle data format conversions that are tedious for the user?
13-
* **Not Just an Alias:** Avoid adding helpers that simply alias a single, straightforward API call.
11+
| Justification | Example | Why Discovery Can't Do It |
12+
|---|---|---|
13+
| **Multi-step orchestration** | `+subscribe` | Creates Pub/Sub topic → subscription → Workspace Events subscription (3 APIs) |
14+
| **Format translation** | `+write` | Transforms Markdown → Docs `batchUpdate` JSON |
15+
| **Multi-API composition** | `+triage` | Lists messages then fetches N metadata payloads concurrently |
16+
| **Complex body construction** | `+send`, `+reply` | Builds RFC 2822 MIME from simple flags |
17+
| **Multipart upload** | `+upload` | Handles resumable upload protocol with progress |
18+
| **Workflow recipes** | `+standup-report` | Chains calls across multiple services |
19+
20+
**Litmus test:** Can the user achieve the same result with `gws <service> <resource> <method> --params '{...}'`? If yes, don't add a helper.
21+
22+
## Anti-Patterns
23+
24+
### ❌ Anti-pattern 1: Single API Call Wrapper
25+
26+
If a helper wraps one API call that Discovery already exposes, reject it.
27+
28+
**Real example:** `+revisions` (PR #563) wrapped `gws drive files-revisions list` — same single API call, zero added value.
29+
30+
### ❌ Anti-pattern 2: Unbounded Flag Accumulation
31+
32+
Adding flags to expose data that is already in the API response creates unbounded surface area.
33+
34+
**Real example:** `--thread-id`, `--delivered-to`, `--sent-last` on `+triage` (PR #597) — all three values are already present in the Gmail API response. Agents and users should extract them with `--format` or `jq`, not new flags.
35+
36+
**Why this is harmful:** Every API response contains dozens of fields. If we add a flag for each one, helpers become unbounded maintenance burdens — the exact problem Discovery-driven design solves.
37+
38+
### ❌ Anti-pattern 3: Duplicating Discovery Parameters
39+
40+
Don't re-expose Discovery-defined parameters (e.g., `pageSize`, `fields`, `orderBy`) as custom helper flags. Use `--params` passthrough instead.
41+
42+
## Flag Design Rules
43+
44+
Helper flags must control **orchestration logic**, not API parameters or output fields.
45+
46+
### ✅ Good Flags (control orchestration)
47+
48+
| Flag | Helper | Why It's Good |
49+
|---|---|---|
50+
| `--spreadsheet`, `--range` | `+read` | Identifies which resource to operate on |
51+
| `--to`, `--subject`, `--body` | `+send` | Inputs to MIME construction (format translation) |
52+
| `--dry-run` | `+subscribe` | Controls whether API calls are actually made |
53+
| `--subscription` | `+subscribe` | Switches between "create new" vs. "use existing" orchestration path |
54+
| `--target`, `--project` | `+subscribe` | Required for multi-service resource creation |
55+
56+
### ❌ Bad Flags (expose API response data)
57+
58+
| Flag | Why It's Bad | Alternative |
59+
|---|---|---|
60+
| `--thread-id` | Already in API response | `jq '.threadId'` |
61+
| `--delivered-to` | Already in response headers | `jq '.payload.headers[] | ...'` |
62+
| `--include-labels` | Output field filtering | `--format` or `jq` |
63+
64+
### Decision Checklist for New Flags
65+
66+
1. Does this flag control **what API call to make** or **how to orchestrate** multiple calls? → ✅ Add it
67+
2. Does this flag control **what data appears in output**? → ❌ Use `--format`/`jq`
68+
3. Does this flag duplicate a Discovery parameter? → ❌ Use `--params`
69+
4. Could the user achieve this with existing flags + post-processing? → ❌ Don't add it
1470

1571
## Architecture
1672

1773
Helpers are implemented using the `Helper` trait defined in `mod.rs`.
1874

19-
```rust
20-
pub trait Helper: Send + Sync {
21-
fn inject_commands(
22-
&self,
23-
cmd: Command,
24-
doc: &crate::discovery::RestDescription,
25-
) -> Command;
26-
27-
fn handle<'a>(
28-
&'a self,
29-
doc: &'a crate::discovery::RestDescription,
30-
matches: &'a ArgMatches
31-
) -> Pin<Box<dyn Future<Output = Result<bool, GwsError>> + Send + 'a>>;
32-
}
33-
```
34-
35-
* **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state.
36-
* **`handle`**: implementation of the command logic. Returns `Ok(true)` if the command was handled, or `Ok(false)` to let the default raw resource handler attempt to handle it.
37-
38-
### Catalogue
39-
40-
| Service | Command | Usage | Description | Equivalent Raw Command (Example) |
41-
| :--- | :--- | :--- | :--- | :--- |
42-
| **Gmail** | `+send` | `gws gmail +send ...` | Sends an email. | `gws gmail users messages send ...` |
43-
| **Sheets** | `+append` | `gws sheets +append ...` | Appends a row. | `gws sheets spreadsheets values append ...` |
44-
| **Sheets** | `+read` | `gws sheets +read ...` | Reads values. | `gws sheets spreadsheets values get ...` |
45-
| **Docs** | `+write` | `gws docs +write ...` | Appends text. | `gws docs documents batchUpdate ...` |
46-
| **Chat** | `+send` | `gws chat +send ...` | Sends message. | `gws chat spaces messages create ...` |
47-
| **Drive** | `+upload` | `gws drive +upload ...` | Uploads file. | `gws drive files create --upload ...` |
48-
| **Calendar** | `+insert` | `gws calendar +insert ...` | Creates event. | `gws calendar events insert ...` |
49-
| **Script** | `+push` | `gws script +push --script <ID>` | Pushes files. | `gws script projects updateContent ...` |
50-
| **Events** | `+subscribe` | `gws events +subscribe ...` | Subscribe & stream events. | Pub/Sub REST + Workspace Events API |
51-
| **Events** | `+renew` | `gws events +renew ...` | Renew subscriptions. | `gws events subscriptions reactivate ...` |
52-
53-
### Development
54-
55-
To add a new helper:
56-
1. Create `src/helpers/<service>.rs`.
57-
2. Implement the `Helper` trait.
58-
3. Register it in `src/helpers/mod.rs`.
59-
4. **Prefix** the command with `+` (e.g., `+create`).
60-
61-
## Current Helpers
62-
63-
* **Gmail**: Sending emails (abstracts RFC 2822 encoding).
64-
* **Sheets**: Appending rows (abstracts `ValueRange` JSON construction).
65-
* **Docs**: Appending text (abstracts `batchUpdate` requests).
66-
* **Chat**: Sending messages to spaces.
75+
- **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state.
76+
- **`handle`**: Implementation of the command logic. Returns `Ok(true)` if the command was handled, or `Ok(false)` to let the default raw resource handler attempt to handle it.
77+
78+
## Adding a New Helper — Checklist
79+
80+
1. **Passes the litmus test** — cannot be done with a single Discovery command
81+
2. **Flags are bounded** — only flags controlling orchestration, not API params/output
82+
3. **Uses shared infrastructure:**
83+
- `crate::client::build_client()` for HTTP
84+
- `crate::validate::validate_resource_name()` for user-supplied resource IDs
85+
- `crate::validate::encode_path_segment()` for URL path segments
86+
- `crate::output::sanitize_for_terminal()` for error messages
87+
4. **Has tests** — at minimum: command registration, required args, happy path
88+
5. **Supports `--dry-run`** where the helper creates or mutates resources
89+
90+
### Development Steps
91+
92+
1. Create `src/helpers/<service>.rs`
93+
2. Implement the `Helper` trait
94+
3. Register it in `src/helpers/mod.rs`
95+
4. **Prefix** the command with `+` (e.g., `+create`)

0 commit comments

Comments
 (0)