diff --git a/CHANGELOG.md b/CHANGELOG.md index 081470bb..1712c53f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`omni-dev drive sheets info` / `drive sheets read` — reading the *cells* of a Google Sheet** ([#1589](https://github.com/rust-works/omni-dev/issues/1589), [ADR-0073](docs/adrs/adr-0073.md)): the Drive API cannot do this at all — it treats a Sheet as an opaque native document with no notion of a range, a row or a cell, which is why `drive read --content` on a Sheet exports **the first sheet only** (Drive's export API has no multi-sheet CSV format). `drive sheets read` closes that gap: with no `--range`/`--sheet` it reads every tab (one `spreadsheets.get` for the titles, then `values.batchGet`), and narrows to a range or a named tab on request. Needs no new login flag — the Sheets API accepts the `drive.readonly` scope every account already has. `-o table` (the default) emits CSV, since a grid of cells is what a spreadsheet range *is*; multi-sheet output separates each block with a `# ` line. Two rendering rules are deliberate and documented: CSV **pads** rows to the widest row (the API truncates trailing empty cells, so raw rows are ragged and a ragged CSV is malformed) while `-o json`/`-o yaml` preserve the raggedness as the truthful shape; and cell values are emitted **verbatim** rather than stripped of control characters, because they are content — a multi-line cell survives intact as a quoted CSV field — while sheet *titles*, rendered as chrome, are sanitised. `--render formatted|unformatted|formula` selects locale-formatted strings, raw typed numbers, or formula text. Sheet titles are always quoted internally, so spaces, apostrophes and `!` need no special handling and a sheet literally titled `A1` is unambiguous; A1 *grammar* is deliberately left to the server, so unbounded forms (`A:A`, `1:2`, a bare defined name) pass through rather than being rejected by a local guess. Writing cells (`sheets write`/`append`/`clear`/`create`) is the next phase. - Under the hood this adds the integration's **first second-host Google API**. Sheets lives on `sheets.googleapis.com`, which no base-URL tweak to `DriveClient` (built around `/drive/v3/...` on `www.googleapis.com`) could reach, so the host-agnostic half of that client — OAuth session, one-shot 401 refresh-and-retry, quota backoff, request logging, error-envelope parsing — moved into a shared `GoogleApiClient` that both clients wrap. They stay **distinct types** so a Sheets-hosted client cannot be handed to `FilesApi` and silently issue `/drive/v3/files` against the wrong host, and they **share one OAuth session**, so a command touching both APIs refreshes the token once rather than twice. `DriveClient`'s public API is unchanged. - Fixes a latent bug the second host exposed: Drive v3 reports error codes at `error.errors[0].reason`, but newer Google services — Sheets v4 among them — return the `google.rpc` envelope with `error.status` and **no `errors[]` array at all**. Reading only the legacy shape yields no reason code, which silently disables both the `--write-file`/`--write-full` scope hint and the quota retry — neither fails loudly, they simply never fire. Both envelope shapes are now understood. +- **`drive_sheets_info` / `drive_sheets_read` MCP tools** ([#1614](https://github.com/rust-works/omni-dev/issues/1614)): the read-only half of the MCP surface ADR-0073 §12 deferred when `drive sheets` shipped CLI-first ([#1589](https://github.com/rust-works/omni-dev/issues/1589) above). Each is a thin wrapper over the same engine functions the CLI calls (`drive::sheets::api::SheetsApi::get_spreadsheet`, `drive::sheets::read::read`), returning YAML rather than the CLI's default CSV rendering — mirrors `drive_file_read`'s shape one-for-one, including the optional `account` parameter. Like the rest of the Drive read surface, these consult no permission gate and write no request-log record (a known, deliberate gap — ADR-0071 §11, ADR-0073 §5), not something specific to Sheets. The write verbs (`write`/`append`/`clear`) still have no MCP equivalent — deferred to a follow-up, since a model-driven caller for a data-mutating surface (particularly `sheets clear`) needs its own thought around a dry-run equivalent and how a folder-permission refusal is surfaced. See [docs/mcp.md](docs/mcp.md#drive-7-tools). - **`omni-dev worktrees ui` remembers your layout, labels sessions readably, and clears all row colours (Phase 5 — completes issue [#1585](https://github.com/rust-works/omni-dev/issues/1585))**: the pane layout now survives a restart. Quitting records the shape of the workspace — how many pane groups, their relative sizes, which worktree each tab was opened in and which was active — to `~/.omni-dev/worktrees-ui-layout.yaml` (`0600` under a `0700` directory, atomic rename, beside the existing row-colour store), and the next run reopens it. What is restored is the *shape*, never a live process: PTYs are hosted in this process ([ADR-0072](docs/adrs/adr-0072.md) §2), so each restored tab is a **new** child in the same place. Restoration is best-effort and silent by design — a worktree deleted since, a shell that no longer exists, a file from a newer version: each is dropped rather than turned into a startup error, and a tab that will not spawn costs you that tab rather than the session (the count is reported in the status bar). Quitting with no tabs removes the file, so the next run starts clean. The Move/Copy Claude Session picker now labels each session with the **first user prompt from its transcript** instead of a bare UUID, which is what the VS Code companion's `readPreview` does and for the same reason: a UUID tells you nothing about which session you are about to move. The reader is deliberately tolerant of Claude's transcript schema — it scans only the head of the file for the first user message and gives up quietly on anything unexpected, rather than parsing a structure that is not ours to depend on — and no transcript content is logged or persisted. Finally `alt-⇧c` clears every row colour at once, retiring the last `#[allow(dead_code)]` placeholder in the module (`HubCommand::ClearAllRowColors` has been wired since Phase 1 with no key to reach it). - **`omni-dev worktrees ui` gains the batch git actions (Phase 4d — completes Phase 4)** ([#1585](https://github.com/rust-works/omni-dev/issues/1585), [ADR-0072](docs/adrs/adr-0072.md) §9): **Rebase on main**, **Push (force-with-lease)** and **Add to Merge Queue** join the action menu's `4_git` group, completing the VS Code tree view's parity surface. Each drives the **daemon's** `rebase`/`push`/`merge-queue` op — the same op the companion extension calls, running the same engine — never the CLI's local-only path, and each is two-phase like `close`: the daemon plans, the plan is rendered verbatim into the confirm modal, and phase 2 re-plans from scratch so what was confirmed is advisory rather than a replayable token. The plans are specific: a rebase names each worktree, its branch and how far behind it is, and warns that a conflict leaves that worktree mid-rebase to resolve in place; a push separates plain fast-forwards from leased forces; a merge-queue check lists the eligible PRs and why each skipped one was skipped. **No force option exists anywhere in the UI** — every force the daemon issues is `--force-with-lease --force-if-includes`, a repository's default branch is never force-pushed, and a refused lease is reported as such with the fix named (fetch and rebase), since there is no harder push to reach for. A guard test greps the UI's action, client and wire modules so a `force` field cannot be added to a request without failing the build; the client also sends no `remote` or `onto` override, each of which would be a way around a guard that lives in the daemon. - **`omni-dev worktrees ui` gains the glyph table, badge columns, scrollback search and a command palette (Phase 4c)** ([#1585](https://github.com/rust-works/omni-dev/issues/1585)): every row cue now comes from one table (`src/cli/worktrees/ui/glyph.rs`) with a unicode and an ASCII form, and **every form is asserted to be exactly one cell wide in both modes** — a glyph that measures two cells misaligns every column after it on that row alone, so the East-Asian *Ambiguous* characters the issue's mockups used (`✔ ⟳ ⚠ ▌ ⇊ ↑ ↓`) are deliberately excluded in favour of confirmed-Narrow substitutes. The new `--ascii` flag (or `OMNI_DEV_UI_ASCII=1`), resolved once at startup, switches the whole table over for terminals without a unicode font. Each worktree row gains a two-cell badge column — PR-check state and Claude-session state, one cell each — which is where a TUI is simply better than the VS Code companion: the extension negotiates two characters of `FileDecoration` between its two providers ([#1406](https://github.com/rust-works/omni-dev/issues/1406)), while a TUI owns every cell and gives each dimension its own. Long branch names are elided in the middle and padded to a fixed column, so paths and fields line up down the pane. `alt-f` searches the focused tab's scrollback case-insensitively and scrolls the match into view, stepping further back on each `Enter`; `:` opens a command palette that filters the same action set the menu uses, and opens the menu at the match rather than firing it, so destructive actions keep their confirm. The tree also now reports which rows are actually on screen (`SetVisibleRows`), so the per-worktree ahead/behind fetch — the dominant cost ([#1306](https://github.com/rust-works/omni-dev/issues/1306)) — covers the visible rows instead of every row in the snapshot, and is sent only when the set changes rather than every frame. diff --git a/docs/mcp.md b/docs/mcp.md index 01fc4e2a..b940587c 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -249,14 +249,16 @@ valid names. | `gmail_label_list` | List labels with unread/total counts. Label add/remove is CLI-only in this release | | `gmail_account_list` | List configured Gmail accounts — name, cached email, scope, default. Never a secret | -### Drive (5 tools) +### Drive (7 tools) -Read-only access (search, dedupe, file metadata/content) via OAuth2, mirroring -the Gmail tool surface one-for-one. Authentication uses `DRIVE_CLIENT_ID` + -`DRIVE_CLIENT_SECRET` + a refresh token stored by `omni-dev drive auth login`. -`rename`/`move` (the CLI's write operations, gated behind the opt-in -`drive.metadata` scope) have no MCP equivalent. See [Drive Guide](drive.md) -and [ADR-0069](adrs/adr-0069.md). +Read-only access (search, dedupe, file metadata/content, Sheets info/read) +via OAuth2, mirroring the Gmail tool surface one-for-one. Authentication uses +`DRIVE_CLIENT_ID` + `DRIVE_CLIENT_SECRET` + a refresh token stored by +`omni-dev drive auth login`. `rename`/`move` (the CLI's write operations, +gated behind the opt-in `drive.metadata` scope) have no MCP equivalent, and +neither do the Sheets write verbs (`write`/`append`/`clear`) — deferred +pending their own design ([#1614](https://github.com/rust-works/omni-dev/issues/1614)). +See [Drive Guide](drive.md) and [ADR-0069](adrs/adr-0069.md). Every tool below (except `drive_account_list`) takes an optional `account` parameter selecting a named Drive account configured via `drive account` @@ -269,6 +271,8 @@ parameter selecting a named Drive account configured via `drive account` | `drive_search` | Search files (Drive query syntax); returns full metadata per hit, including checksums when present | | `drive_dedupe` | Find files sharing the same content hash within a query's results, grouped by `md5Checksum` | | `drive_file_read` | Read a file's metadata (default) or content (`format: "content"`); `output_file` writes binary content to disk; `verify: true` checks the fetched SHA-256 against Drive's reported checksum | +| `drive_sheets_info` | A spreadsheet's title and the sheets (tabs) it contains — id, title, index, hidden flag, grid dimensions | +| `drive_sheets_read` | Read cell values from one range, one sheet, or the whole workbook (capped at 200 sheets); `render` selects formatted/unformatted/formula values | | `drive_account_list` | List configured Drive accounts — name, cached email, scope, default. Never a secret | ### AI / Config (5 tools) diff --git a/src/cli/drive/sheets.rs b/src/cli/drive/sheets.rs index 0f99276f..1506d2f1 100644 --- a/src/cli/drive/sheets.rs +++ b/src/cli/drive/sheets.rs @@ -28,9 +28,11 @@ pub struct SheetsCommand { /// Sheets subcommands. #[derive(Subcommand)] pub enum SheetsSubcommands { - /// Shows a spreadsheet's title and the sheets (tabs) it contains. + /// Shows a spreadsheet's title and the sheets (tabs) it contains + /// (mirrors the `drive_sheets_info` MCP tool). Info(info::InfoCommand), - /// Reads cell values from one range, or from every sheet. + /// Reads cell values from one range, or from every sheet (mirrors the + /// `drive_sheets_read` MCP tool). Read(read::ReadCommand), /// Overwrites the cells of a range, gated by the folder /// write-permission rules (issue #1589). Requires the `drive.file` or diff --git a/src/mcp/drive_tools.rs b/src/mcp/drive_tools.rs index b5d679a8..767d8800 100644 --- a/src/mcp/drive_tools.rs +++ b/src/mcp/drive_tools.rs @@ -21,6 +21,14 @@ //! `gmail_search`: `files.list` returns full metadata per hit in one call //! (see `src/drive/files_api.rs`'s module doc), so there's no //! ids-then-hydrate split to expose. +//! +//! `drive_sheets_info`/`drive_sheets_read` mirror the CLI's `drive sheets +//! info`/`read` (#1614) — read-only, so, like the rest of this file, they +//! consult no permission gate and write no request-log record (a known, +//! deliberate gap across the whole Drive read surface, not something +//! specific to Sheets; see [ADR-0071](../../docs/adrs/adr-0071.md) §11 and +//! [ADR-0073](../../docs/adrs/adr-0073.md) §5). The write verbs (`write`/ +//! `append`/`clear`) have no MCP equivalent yet, per ADR-0073 §12. use anyhow::{Context, Result}; use rmcp::{ @@ -39,6 +47,9 @@ use crate::drive::account; use crate::drive::auth; use crate::drive::client::DriveClient; use crate::drive::files_api::{FilesApi, DEFAULT_SEARCH_LIMIT}; +use crate::drive::sheets::api::{SheetsApi, ValueRenderOption}; +use crate::drive::sheets::client::SheetsClient; +use crate::drive::sheets::read::{read as sheets_read, ReadOptions}; use crate::utils::settings::Settings; use super::error::tool_error; @@ -136,6 +147,42 @@ pub struct DriveFileReadParams { pub account: Option<String>, } +/// Parameters for the `drive_sheets_info` tool. +#[derive(Debug, Deserialize, schemars::JsonSchema)] +pub struct DriveSheetsInfoParams { + /// Spreadsheet id (the `/d/<ID>/` segment of a Sheets URL). Required. + pub spreadsheet_id: String, + #[doc = account_param_doc!()] + #[serde(default)] + pub account: Option<String>, +} + +/// Parameters for the `drive_sheets_read` tool. +#[derive(Debug, Deserialize, schemars::JsonSchema)] +pub struct DriveSheetsReadParams { + /// Spreadsheet id (the `/d/<ID>/` segment of a Sheets URL). Required. + pub spreadsheet_id: String, + /// A1 range to read, optionally carrying its own `Sheet!` prefix (e.g. + /// `A1:C10`, `'My Sheet'!A:A`). Combined with `sheet` when bare. Omit + /// both `range` and `sheet` to read every sheet in the workbook (capped + /// at 200 sheets). + #[serde(default)] + pub range: Option<String>, + /// Sheet (tab) title to read. Supplies the prefix for a bare `range`, + /// or selects the whole tab on its own. Conflicts with a `range` that + /// already names a sheet. + #[serde(default)] + pub sheet: Option<String>, + /// How cell values are rendered: `formatted` (default) matches the + /// spreadsheet as displayed; `unformatted` yields raw typed numbers + /// rather than locale-formatted strings; `formula` yields formula text. + #[serde(default)] + pub render: Option<String>, + #[doc = account_param_doc!()] + #[serde(default)] + pub account: Option<String>, +} + /// Parameters for `drive_account_list` (none, mirrors Gmail's issue #1500). #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] pub struct DriveAccountListParams {} @@ -241,6 +288,48 @@ impl OmniDevServer { } } + /// Tool: a spreadsheet's title and the sheets (tabs) it contains. + #[tool( + description = "Show a Sheets spreadsheet's title and the sheets (tabs) it contains — \ + id, title, index, hidden flag, and grid dimensions per tab. Use this to \ + discover sheet titles before calling `drive_sheets_read` with a `sheet` \ + parameter. \ + Read-only. Mirrors `omni-dev drive sheets info`. Output is YAML." + )] + pub async fn drive_sheets_info( + &self, + Parameters(params): Parameters<DriveSheetsInfoParams>, + ) -> Result<CallToolResult, McpError> { + let client = create_client_for(params.account.as_deref()).map_err(tool_error)?; + let yaml = run_sheets_info(&client, ¶ms) + .await + .map_err(tool_error)?; + Ok(build_truncated_result(yaml)) + } + + /// Tool: read cell values from a Sheets spreadsheet. + #[tool( + description = "Read cell values from a Sheets spreadsheet. Set `range` (e.g. `A1:C10`, \ + `'My Sheet'!A:A`) and/or `sheet` (a tab title) to read one range or one \ + whole tab; omit both to read every sheet in the workbook (capped at 200 \ + sheets — call `drive_sheets_info` first on a larger workbook). `render` \ + controls how values come back: `formatted` (default) matches the \ + spreadsheet as displayed, `unformatted` yields raw typed numbers, \ + `formula` yields formula text. Rows are ragged, exactly as the API \ + returns them (trailing empty cells are not padded). \ + Read-only. Mirrors `omni-dev drive sheets read`. Output is YAML." + )] + pub async fn drive_sheets_read( + &self, + Parameters(params): Parameters<DriveSheetsReadParams>, + ) -> Result<CallToolResult, McpError> { + let client = create_client_for(params.account.as_deref()).map_err(tool_error)?; + let yaml = run_sheets_read(&client, ¶ms) + .await + .map_err(tool_error)?; + Ok(build_truncated_result(yaml)) + } + /// Tool: list configured named Drive accounts. #[tool( description = "List Drive accounts configured in ~/.omni-dev/settings.json — name, \ @@ -419,6 +508,43 @@ fn parse_read_format(raw: Option<&str>) -> Result<ReadFormat> { } } +async fn run_sheets_info(client: &DriveClient, params: &DriveSheetsInfoParams) -> Result<String> { + let sheets = SheetsClient::from_drive_client(client)?; + let spreadsheet = SheetsApi::new(&sheets) + .get_spreadsheet(¶ms.spreadsheet_id) + .await?; + yaml_result(&spreadsheet) +} + +async fn run_sheets_read(client: &DriveClient, params: &DriveSheetsReadParams) -> Result<String> { + let render = parse_render_option(params.render.as_deref())?; + let sheets = SheetsClient::from_drive_client(client)?; + let opts = ReadOptions { + spreadsheet_id: params.spreadsheet_id.clone(), + range: params.range.clone(), + sheet: params.sheet.clone(), + render, + }; + let outcome = sheets_read(&SheetsApi::new(&sheets), &opts).await?; + yaml_result(&outcome) +} + +/// Parses an MCP-supplied render-option string. `None` defaults to +/// `formatted`, mirroring `RenderArg::default()` in +/// `src/cli/drive/sheets/read.rs`. +fn parse_render_option(raw: Option<&str>) -> Result<ValueRenderOption> { + match raw.map(str::to_ascii_lowercase).as_deref() { + None | Some("formatted") => Ok(ValueRenderOption::Formatted), + Some("unformatted") => Ok(ValueRenderOption::Unformatted), + Some("formula") => Ok(ValueRenderOption::Formula), + Some(other) => { + anyhow::bail!( + "unknown render {other:?} (expected 'formatted', 'unformatted', or 'formula')" + ) + } + } +} + fn yaml_result<T: Serialize>(data: &T) -> Result<String> { serde_yaml::to_string(data).context("Failed to serialize result as YAML") } @@ -430,6 +556,7 @@ mod tests { use super::*; use crate::drive::auth::{DriveCredentials, DriveGrantedScopes, SCOPE_READONLY}; + use crate::drive::sheets::client::SHEETS_API_URL; use crate::drive::test_support::EnvGuard; use crate::utils::secret::Secret; @@ -1100,6 +1227,223 @@ mod tests { assert!(err.to_string().contains("verify requires format"), "{err}"); } + // ── parse_render_option ───────────────────────────────────────── + + #[test] + fn parse_render_option_defaults_to_formatted() { + assert_eq!( + parse_render_option(None).unwrap(), + ValueRenderOption::Formatted + ); + } + + #[test] + fn parse_render_option_accepts_known_strings() { + assert_eq!( + parse_render_option(Some("formatted")).unwrap(), + ValueRenderOption::Formatted + ); + assert_eq!( + parse_render_option(Some("unformatted")).unwrap(), + ValueRenderOption::Unformatted + ); + assert_eq!( + parse_render_option(Some("formula")).unwrap(), + ValueRenderOption::Formula + ); + } + + #[test] + fn parse_render_option_is_case_insensitive() { + assert_eq!( + parse_render_option(Some("FORMULA")).unwrap(), + ValueRenderOption::Formula + ); + } + + #[test] + fn parse_render_option_rejects_unknown_value() { + let err = parse_render_option(Some("bogus")).unwrap_err(); + assert!(err.to_string().contains("render"), "{err}"); + } + + // ── run_sheets_info / run_sheets_read ─────────────────────────── + + /// Points `SHEETS_API_URL` at `server` for the guard's lifetime, so + /// `SheetsClient::from_drive_client`'s real `SystemEnv` lookup (inside + /// `run_sheets_info`/`run_sheets_read`) resolves to the mock server + /// instead of the real `sheets.googleapis.com`. Mirrors + /// `EnvGuard::redirect_api_hosts_to_a_dead_port`'s pattern of setting + /// the var directly once the guard already holds the process-wide lock. + fn point_sheets_api_at(_guard: &EnvGuard, server: &wiremock::MockServer) { + std::env::set_var(SHEETS_API_URL, server.uri()); + } + + #[tokio::test] + async fn run_sheets_info_returns_spreadsheet_as_yaml() { + let guard = EnvGuard::take(); + let server = wiremock::MockServer::start().await; + point_sheets_api_at(&guard, &server); + let client = client_with_bootstrapped_token(&server).await; + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path("/v4/spreadsheets/s1")) + .respond_with( + wiremock::ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "spreadsheetId": "s1", + "properties": {"title": "Budget"}, + "sheets": [{"properties": {"title": "Q1"}}], + })), + ) + .expect(1) + .mount(&server) + .await; + + let yaml = run_sheets_info( + &client, + &DriveSheetsInfoParams { + spreadsheet_id: "s1".to_string(), + account: None, + }, + ) + .await + .unwrap(); + assert!(yaml.contains("spreadsheetId: s1"), "{yaml}"); + assert!(yaml.contains("title: Q1"), "{yaml}"); + } + + #[tokio::test] + async fn run_sheets_info_propagates_a_not_found_error() { + let guard = EnvGuard::take(); + let server = wiremock::MockServer::start().await; + point_sheets_api_at(&guard, &server); + let client = client_with_bootstrapped_token(&server).await; + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path("/v4/spreadsheets/missing")) + .respond_with( + wiremock::ResponseTemplate::new(404).set_body_json(serde_json::json!({ + "error": {"code": 404, "message": "Requested entity was not found.", + "status": "NOT_FOUND"}, + })), + ) + .mount(&server) + .await; + + let err = run_sheets_info( + &client, + &DriveSheetsInfoParams { + spreadsheet_id: "missing".to_string(), + account: None, + }, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("not found"), "{err}"); + } + + #[tokio::test] + async fn run_sheets_read_single_range_returns_values_as_yaml() { + let guard = EnvGuard::take(); + let server = wiremock::MockServer::start().await; + point_sheets_api_at(&guard, &server); + let client = client_with_bootstrapped_token(&server).await; + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path("/v4/spreadsheets/s1/values/A1:B2")) + .respond_with( + wiremock::ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "range": "Sheet1!A1:B2", + "values": [["a", "b"], ["1", "2"]], + })), + ) + .expect(1) + .mount(&server) + .await; + + let yaml = run_sheets_read( + &client, + &DriveSheetsReadParams { + spreadsheet_id: "s1".to_string(), + range: Some("A1:B2".to_string()), + sheet: None, + render: None, + account: None, + }, + ) + .await + .unwrap(); + assert!(yaml.contains("- - a"), "{yaml}"); + assert!(yaml.contains("- '1'"), "{yaml}"); + } + + #[tokio::test] + async fn run_sheets_read_whole_workbook_fetches_every_sheet() { + let guard = EnvGuard::take(); + let server = wiremock::MockServer::start().await; + point_sheets_api_at(&guard, &server); + let client = client_with_bootstrapped_token(&server).await; + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path("/v4/spreadsheets/s1")) + .respond_with( + wiremock::ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "spreadsheetId": "s1", + "properties": {"title": "Book"}, + "sheets": [ + {"properties": {"sheetId": 0, "title": "Q1", "index": 0}}, + ], + })), + ) + .mount(&server) + .await; + wiremock::Mock::given(wiremock::matchers::method("GET")) + .and(wiremock::matchers::path( + "/v4/spreadsheets/s1/values:batchGet", + )) + .respond_with( + wiremock::ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "spreadsheetId": "s1", + "valueRanges": [ + {"range": "Q1!A1:Z1000", "values": [["x"]]}, + ], + })), + ) + .mount(&server) + .await; + + let yaml = run_sheets_read( + &client, + &DriveSheetsReadParams { + spreadsheet_id: "s1".to_string(), + range: None, + sheet: None, + render: None, + account: None, + }, + ) + .await + .unwrap(); + assert!(yaml.contains("title: Q1"), "{yaml}"); + assert!(yaml.contains("- x"), "{yaml}"); + } + + #[tokio::test] + async fn run_sheets_read_rejects_unknown_render_value() { + let server = wiremock::MockServer::start().await; + let client = client_with_bootstrapped_token(&server).await; + + let err = run_sheets_read( + &client, + &DriveSheetsReadParams { + spreadsheet_id: "s1".to_string(), + range: Some("A1".to_string()), + sheet: None, + render: Some("bogus".to_string()), + account: None, + }, + ) + .await + .unwrap_err(); + assert!(err.to_string().contains("render"), "{err}"); + } + // ── Tool handler bodies (smoke + auth-status full path) ─────────── #[tokio::test(flavor = "current_thread")] @@ -1193,6 +1537,70 @@ mod tests { assert!(err.message.contains("not configured")); } + #[tokio::test(flavor = "current_thread")] + async fn drive_sheets_info_handler_propagates_credentials_error() { + let guard = EnvGuard::take(); + let _dir = guard.clear_credentials(); + + let server = OmniDevServer::new(); + let err = server + .drive_sheets_info(Parameters(DriveSheetsInfoParams { + spreadsheet_id: "s1".to_string(), + account: None, + })) + .await + .unwrap_err(); + assert!(err.message.contains("not configured")); + } + + #[tokio::test(flavor = "current_thread")] + async fn drive_sheets_read_handler_propagates_credentials_error() { + let guard = EnvGuard::take(); + let _dir = guard.clear_credentials(); + + let server = OmniDevServer::new(); + let err = server + .drive_sheets_read(Parameters(DriveSheetsReadParams { + spreadsheet_id: "s1".to_string(), + range: None, + sheet: None, + render: None, + account: None, + })) + .await + .unwrap_err(); + assert!(err.message.contains("not configured")); + } + + #[tokio::test(flavor = "current_thread")] + async fn drive_sheets_read_handler_honors_named_account_param() { + // A named `account` reaches `create_client_for` — an unknown name + // surfaces as the account-resolution error, not the generic "not + // configured" one, proving the param actually propagates. + let guard = EnvGuard::take(); + let dir = guard.clear_credentials(); + let settings_path = dir.path().join(".omni-dev").join("settings.json"); + Settings::upsert_drive_account( + &settings_path, + "work", + &[("client_id", serde_json::Value::String("id".to_string()))], + ) + .unwrap(); + + let server = OmniDevServer::new(); + let err = server + .drive_sheets_read(Parameters(DriveSheetsReadParams { + spreadsheet_id: "s1".to_string(), + range: None, + sheet: None, + render: None, + account: Some("bogus".to_string()), + })) + .await + .unwrap_err(); + assert!(err.message.contains("unknown Drive account 'bogus'")); + } + // ── run_account_list / drive_account_list ────────────────────────── #[test] diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 1e646b1b..d715c902 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -404,6 +404,8 @@ mod tests { "drive_file_read", "drive_account_list", "drive_dedupe", + "drive_sheets_info", + "drive_sheets_read", ] { assert!(server.tool_router.has_route(name), "missing route: {name}"); } diff --git a/tests/snapshots/integration_test__help_all_output.snap b/tests/snapshots/integration_test__help_all_output.snap index ff2400e6..425dd9b7 100644 --- a/tests/snapshots/integration_test__help_all_output.snap +++ b/tests/snapshots/integration_test__help_all_output.snap @@ -4054,8 +4054,8 @@ Reads and writes the cells of a Google Sheet via the Sheets v4 API (issue #1589) Usage: sheets <COMMAND> Commands: - info Shows a spreadsheet's title and the sheets (tabs) it contains - read Reads cell values from one range, or from every sheet + info Shows a spreadsheet's title and the sheets (tabs) it contains (mirrors the `drive_sheets_info` MCP tool) + read Reads cell values from one range, or from every sheet (mirrors the `drive_sheets_read` MCP tool) write Overwrites the cells of a range, gated by the folder write-permission rules (issue #1589). Requires the `drive.file` or `drive` scope (`drive auth login --write-file`/`--write-full`) append Appends rows after the last row of a range's table, gated by the folder write-permission rules (issue #1589) clear Clears a range's values, leaving formatting intact. Gated by the folder write-permission rules (issue #1589) @@ -4128,9 +4128,9 @@ Options: ================================================================================ -omni-dev drive sheets info - Shows a spreadsheet's title and the sheets (tabs) it contains +omni-dev drive sheets info - Shows a spreadsheet's title and the sheets (tabs) it contains (mirrors the `drive_sheets_info` MCP tool) -Shows a spreadsheet's title and the sheets (tabs) it contains +Shows a spreadsheet's title and the sheets (tabs) it contains (mirrors the `drive_sheets_info` MCP tool) Usage: info [OPTIONS] <SPREADSHEET_ID> @@ -4144,9 +4144,9 @@ Options: ================================================================================ -omni-dev drive sheets read - Reads cell values from one range, or from every sheet +omni-dev drive sheets read - Reads cell values from one range, or from every sheet (mirrors the `drive_sheets_read` MCP tool) -Reads cell values from one range, or from every sheet +Reads cell values from one range, or from every sheet (mirrors the `drive_sheets_read` MCP tool) Usage: read [OPTIONS] <SPREADSHEET_ID>