diff --git a/.gemini/style_guide.md b/.gemini/style_guide.md index cf99170b..5352149c 100644 --- a/.gemini/style_guide.md +++ b/.gemini/style_guide.md @@ -33,3 +33,7 @@ Examples of scope creep to avoid: ## Severity Calibration 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. + +## Helper Commands (`+verb`) + +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. diff --git a/AGENTS.md b/AGENTS.md index 122cdffe..593f37bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,7 +137,7 @@ When a user-supplied string is used as a GCP resource identifier (project ID, to ```rust // Validates the string does not contain path traversal segments (`..`), control characters, or URL-breaking characters like `?` and `#`. -let project = crate::helpers::validate_resource_name(&project_id)?; +let project = crate::validate::validate_resource_name(&project_id)?; let url = format!("https://pubsub.googleapis.com/v1/projects/{}/topics/my-topic", project); ``` @@ -166,6 +166,15 @@ Use these labels to categorize pull requests and issues: - `area: auth` — OAuth, credentials, multi-account, ADC - `area: skills` — AI skill generation and management +## Helper Commands (`+verb`) + +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. + +> [!IMPORTANT] +> **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. + +See [`src/helpers/README.md`](src/helpers/README.md) for full guidelines, anti-patterns, and a checklist for new helpers. + ## Environment Variables ### Authentication diff --git a/src/helpers/README.md b/src/helpers/README.md index 7d7a22d3..c08559f9 100644 --- a/src/helpers/README.md +++ b/src/helpers/README.md @@ -1,66 +1,95 @@ -# Helper Modules +# Helper Commands (`+verb`) — Guidelines -This directory contains "Helper" implementations that provide high-value, simplified commands for complex Google Workspace API operations. +## Design Principle -## Philosophy +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.** -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). +## When a Helper is Justified -**Helper commands should only be added if they offer "High Usefulness":** +A `+helper` command should exist only when it provides value that Discovery-based commands **cannot**: -* **Complex Abstraction:** Does it abstract away significant complexity (e.g., MIME encoding, complex JSON structures, multiple API calls)? -* **Format Conversion:** Does it handle data format conversions that are tedious for the user? -* **Not Just an Alias:** Avoid adding helpers that simply alias a single, straightforward API call. +| Justification | Example | Why Discovery Can't Do It | +|---|---|---| +| **Multi-step orchestration** | `+subscribe` | Creates Pub/Sub topic → subscription → Workspace Events subscription (3 APIs) | +| **Format translation** | `+write` | Transforms Markdown → Docs `batchUpdate` JSON | +| **Multi-API composition** | `+triage` | Lists messages then fetches N metadata payloads concurrently | +| **Complex body construction** | `+send`, `+reply` | Builds RFC 2822 MIME from simple flags | +| **Multipart upload** | `+upload` | Handles resumable upload protocol with progress | +| **Workflow recipes** | `+standup-report` | Chains calls across multiple services | + +**Litmus test:** Can the user achieve the same result with `gws --params '{...}'`? If yes, don't add a helper. + +## Anti-Patterns + +### ❌ Anti-pattern 1: Single API Call Wrapper + +If a helper wraps one API call that Discovery already exposes, reject it. + +**Real example:** `+revisions` (PR #563) wrapped `gws drive files-revisions list` — same single API call, zero added value. + +### ❌ Anti-pattern 2: Unbounded Flag Accumulation + +Adding flags to expose data that is already in the API response creates unbounded surface area. + +**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. + +**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. + +### ❌ Anti-pattern 3: Duplicating Discovery Parameters + +Don't re-expose Discovery-defined parameters (e.g., `pageSize`, `fields`, `orderBy`) as custom helper flags. Use `--params` passthrough instead. + +## Flag Design Rules + +Helper flags must control **orchestration logic**, not API parameters or output fields. + +### ✅ Good Flags (control orchestration) + +| Flag | Helper | Why It's Good | +|---|---|---| +| `--spreadsheet`, `--range` | `+read` | Identifies which resource to operate on | +| `--to`, `--subject`, `--body` | `+send` | Inputs to MIME construction (format translation) | +| `--dry-run` | `+subscribe` | Controls whether API calls are actually made | +| `--subscription` | `+subscribe` | Switches between "create new" vs. "use existing" orchestration path | +| `--target`, `--project` | `+subscribe` | Required for multi-service resource creation | + +### ❌ Bad Flags (expose API response data) + +| Flag | Why It's Bad | Alternative | +|---|---|---| +| `--thread-id` | Already in API response | `jq '.threadId'` | +| `--delivered-to` | Already in response headers | `jq '.payload.headers[] | ...'` | +| `--include-labels` | Output field filtering | `--format` or `jq` | + +### Decision Checklist for New Flags + +1. Does this flag control **what API call to make** or **how to orchestrate** multiple calls? → ✅ Add it +2. Does this flag control **what data appears in output**? → ❌ Use `--format`/`jq` +3. Does this flag duplicate a Discovery parameter? → ❌ Use `--params` +4. Could the user achieve this with existing flags + post-processing? → ❌ Don't add it ## Architecture Helpers are implemented using the `Helper` trait defined in `mod.rs`. -```rust -pub trait Helper: Send + Sync { - fn inject_commands( - &self, - cmd: Command, - doc: &crate::discovery::RestDescription, - ) -> Command; - - fn handle<'a>( - &'a self, - doc: &'a crate::discovery::RestDescription, - matches: &'a ArgMatches - ) -> Pin> + Send + 'a>>; -} -``` - -* **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state. -* **`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. - -### Catalogue - -| Service | Command | Usage | Description | Equivalent Raw Command (Example) | -| :--- | :--- | :--- | :--- | :--- | -| **Gmail** | `+send` | `gws gmail +send ...` | Sends an email. | `gws gmail users messages send ...` | -| **Sheets** | `+append` | `gws sheets +append ...` | Appends a row. | `gws sheets spreadsheets values append ...` | -| **Sheets** | `+read` | `gws sheets +read ...` | Reads values. | `gws sheets spreadsheets values get ...` | -| **Docs** | `+write` | `gws docs +write ...` | Appends text. | `gws docs documents batchUpdate ...` | -| **Chat** | `+send` | `gws chat +send ...` | Sends message. | `gws chat spaces messages create ...` | -| **Drive** | `+upload` | `gws drive +upload ...` | Uploads file. | `gws drive files create --upload ...` | -| **Calendar** | `+insert` | `gws calendar +insert ...` | Creates event. | `gws calendar events insert ...` | -| **Script** | `+push` | `gws script +push --script ` | Pushes files. | `gws script projects updateContent ...` | -| **Events** | `+subscribe` | `gws events +subscribe ...` | Subscribe & stream events. | Pub/Sub REST + Workspace Events API | -| **Events** | `+renew` | `gws events +renew ...` | Renew subscriptions. | `gws events subscriptions reactivate ...` | - -### Development - -To add a new helper: -1. Create `src/helpers/.rs`. -2. Implement the `Helper` trait. -3. Register it in `src/helpers/mod.rs`. -4. **Prefix** the command with `+` (e.g., `+create`). - -## Current Helpers - -* **Gmail**: Sending emails (abstracts RFC 2822 encoding). -* **Sheets**: Appending rows (abstracts `ValueRange` JSON construction). -* **Docs**: Appending text (abstracts `batchUpdate` requests). -* **Chat**: Sending messages to spaces. +- **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state. +- **`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. + +## Adding a New Helper — Checklist + +1. **Passes the litmus test** — cannot be done with a single Discovery command +2. **Flags are bounded** — only flags controlling orchestration, not API params/output +3. **Uses shared infrastructure:** + - `crate::client::build_client()` for HTTP + - `crate::validate::validate_resource_name()` for user-supplied resource IDs + - `crate::validate::encode_path_segment()` for URL path segments + - `crate::output::sanitize_for_terminal()` for error messages +4. **Has tests** — at minimum: command registration, required args, happy path +5. **Supports `--dry-run`** where the helper creates or mutates resources + +### Development Steps + +1. Create `src/helpers/.rs` +2. Implement the `Helper` trait +3. Register it in `src/helpers/mod.rs` +4. **Prefix** the command with `+` (e.g., `+create`)