From 4305fd97871b8e6a3dab71ee9a2ebf795863c763 Mon Sep 17 00:00:00 2001 From: Khali Date: Thu, 19 Mar 2026 15:23:27 +1100 Subject: [PATCH 01/11] feat(docs): add +revisions helper to list document revision history Adds a new `gws docs +revisions` helper command that lists revision metadata for a Google Docs document by calling the Drive v3 revisions API directly (drive.readonly scope). Flags: --document (required) Document ID --limit (optional, default 20) Number of revisions to return Each revision entry includes: id, modifiedTime, lastModifyingUser, keepForever, and size. A clear note documents the Google API limitation: content of past revisions is not accessible for native Docs files. Also adds the gws-docs-revisions SKILL.md and updates gws-docs SKILL.md to reference the new helper. --- Cargo.lock | 2 +- skills/gws-docs-revisions/SKILL.md | 72 +++++++++++++++++ skills/gws-docs/SKILL.md | 1 + src/helpers/docs.rs | 125 ++++++++++++++++++++++++++++- 4 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 skills/gws-docs-revisions/SKILL.md diff --git a/Cargo.lock b/Cargo.lock index af98fd1c..873bd275 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 4 +version = 3 [[package]] name = "aead" diff --git a/skills/gws-docs-revisions/SKILL.md b/skills/gws-docs-revisions/SKILL.md new file mode 100644 index 00000000..92d68efe --- /dev/null +++ b/skills/gws-docs-revisions/SKILL.md @@ -0,0 +1,72 @@ +--- +name: gws-docs-revisions +version: 1.0.0 +description: "Google Docs: List revision history for a document." +metadata: + openclaw: + category: "productivity" + requires: + bins: ["gws"] + cliHelp: "gws docs +revisions --help" +--- + +# docs +revisions + +> **PREREQUISITE:** Read `../gws-shared/SKILL.md` for auth, global flags, and security rules. If missing, run `gws generate-skills` to create it. + +List revision history for a Google Docs document. + +## Usage + +```bash +gws docs +revisions --document [--limit ] +``` + +## Flags + +| Flag | Required | Default | Description | +|------|----------|---------|-------------| +| `--document` | ✓ | — | Document ID (from the URL) | +| `--limit` | — | 20 | Maximum number of revisions to return (1–1000) | + +## Examples + +```bash +# Show last 20 revisions (default) +gws docs +revisions --document 1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms + +# Show last 5 revisions +gws docs +revisions --document 1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms --limit 5 +``` + +## Output + +Each revision includes: + +| Field | Description | +|-------|-------------| +| `id` | Revision ID | +| `modifiedTime` | When this revision was created | +| `lastModifyingUser.displayName` | Who made the change | +| `keepForever` | Whether this revision is pinned (won't be auto-deleted) | +| `size` | Size of the revision in bytes | + +## Limitations + +> [!IMPORTANT] +> **Content is not available.** The Google Drive API returns revision *metadata* only for +> native Google Docs files. The actual text content of past revisions cannot be retrieved +> via API — only the current content is accessible via `gws docs documents get`. +> +> To read a specific revision's content, open the document in Google Docs and use +> **File → Version history → See version history**. + +## Scope + +This command uses the `drive.readonly` OAuth scope. No write access is required. + +## See Also + +- [gws-shared](../gws-shared/SKILL.md) — Global flags and auth +- [gws-docs](../gws-docs/SKILL.md) — All Google Docs commands +- [gws-docs-write](../gws-docs-write/SKILL.md) — Append text to a document diff --git a/skills/gws-docs/SKILL.md b/skills/gws-docs/SKILL.md index 2a910aff..4a35461f 100644 --- a/skills/gws-docs/SKILL.md +++ b/skills/gws-docs/SKILL.md @@ -23,6 +23,7 @@ gws docs [flags] | Command | Description | |---------|-------------| | [`+write`](../gws-docs-write/SKILL.md) | Append text to a document | +| [`+revisions`](../gws-docs-revisions/SKILL.md) | List revision history for a document | ## API Resources diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index d3ef7fa2..4cdfd4fe 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -17,7 +17,7 @@ use crate::auth; use crate::error::GwsError; use crate::executor; use clap::{Arg, ArgMatches, Command}; -use serde_json::json; +use serde_json::{json, Value}; use std::future::Future; use std::pin::Pin; @@ -56,6 +56,39 @@ TIPS: For rich formatting, use the raw batchUpdate API instead.", ), ); + cmd = cmd.subcommand( + Command::new("+revisions") + .about("[Helper] List revision history of a document") + .arg( + Arg::new("document") + .long("document") + .help("Document ID") + .required(true) + .value_name("ID"), + ) + .arg( + Arg::new("limit") + .long("limit") + .help("Maximum number of revisions to return (default: 20)") + .value_name("N") + .value_parser(clap::value_parser!(u32)), + ) + .after_help( + "\ +EXAMPLES: + gws docs +revisions --document DOC_ID + gws docs +revisions --document DOC_ID --limit 5 + gws docs +revisions --document DOC_ID --format table + +TIPS: + The document ID is the long string in the Google Docs URL. + Returns metadata for each revision: ID, modified time, author, and + whether the revision is kept forever. + Note: the full content of past revisions is not accessible via the + Google API for native Docs files. Use the Google Docs UI (File → + Version history) to view or restore specific versions.", + ), + ); cmd } @@ -111,11 +144,70 @@ TIPS: return Ok(true); } + + if let Some(matches) = matches.subcommand_matches("+revisions") { + handle_revisions(matches).await?; + return Ok(true); + } + Ok(false) }) } } +async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { + let document_id = matches.get_one::("document").unwrap(); + let limit = matches.get_one::("limit").copied().unwrap_or(20); + + let scope = "https://www.googleapis.com/auth/drive.readonly"; + let token = auth::get_token(&[scope]) + .await + .map_err(|e| GwsError::Auth(format!("Docs auth failed: {e}")))?; + + let client = crate::client::build_client()?; + let limit_str = limit.to_string(); + + let resp = client + .get(format!( + "https://www.googleapis.com/drive/v3/files/{}/revisions", + document_id + )) + .query(&[ + ( + "fields", + "revisions(id,modifiedTime,lastModifyingUser/displayName,keepForever,size)", + ), + ("pageSize", limit_str.as_str()), + ]) + .bearer_auth(&token) + .send() + .await + .map_err(|e| GwsError::Other(anyhow::anyhow!("HTTP request failed: {e}")))?; + + if !resp.status().is_success() { + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + return Err(GwsError::Api { + code: status.as_u16(), + message: body, + reason: "revisions_request_failed".to_string(), + enable_url: None, + }); + } + + let value: Value = resp + .json() + .await + .map_err(|e| GwsError::Other(anyhow::anyhow!("JSON parse failed: {e}")))?; + + let fmt = matches + .get_one::("format") + .map(|s| crate::formatter::OutputFormat::from_str(s)) + .unwrap_or_default(); + println!("{}", crate::formatter::format_value(&value, &fmt)); + Ok(()) +} + fn build_write_request( matches: &ArgMatches, doc: &crate::discovery::RestDescription, @@ -203,4 +295,35 @@ mod tests { assert!(body.contains("endOfSegmentLocation")); assert_eq!(scopes[0], "https://scope"); } + + #[test] + fn test_revisions_command_registered() { + let helper = DocsHelper; + let base = Command::new("docs"); + let doc = RestDescription::default(); + let cmd = helper.inject_commands(base, &doc); + let subcommands: Vec<&str> = cmd + .get_subcommands() + .map(|c| c.get_name()) + .collect(); + assert!(subcommands.contains(&"+revisions")); + assert!(subcommands.contains(&"+write")); + } + + #[test] + fn test_revisions_requires_document() { + let helper = DocsHelper; + let base = Command::new("docs"); + let doc = RestDescription::default(); + let cmd = helper.inject_commands(base, &doc); + let revisions_cmd = cmd + .get_subcommands() + .find(|c| c.get_name() == "+revisions") + .unwrap(); + let doc_arg = revisions_cmd + .get_arguments() + .find(|a| a.get_id() == "document") + .unwrap(); + assert!(doc_arg.is_required_set()); + } } From 4426d1aab1d393684bf49e57d8aab558307e1f88 Mon Sep 17 00:00:00 2001 From: Khali Date: Thu, 19 Mar 2026 15:55:13 +1100 Subject: [PATCH 02/11] chore: cargo fmt and add changeset --- .changeset/docs-revisions-helper.md | 5 +++++ src/helpers/docs.rs | 5 +---- 2 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 .changeset/docs-revisions-helper.md diff --git a/.changeset/docs-revisions-helper.md b/.changeset/docs-revisions-helper.md new file mode 100644 index 00000000..b62ae440 --- /dev/null +++ b/.changeset/docs-revisions-helper.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": minor +--- + +Add `gws docs +revisions` helper to list revision history for a Google Docs document diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index 4cdfd4fe..9f893d16 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -302,10 +302,7 @@ mod tests { let base = Command::new("docs"); let doc = RestDescription::default(); let cmd = helper.inject_commands(base, &doc); - let subcommands: Vec<&str> = cmd - .get_subcommands() - .map(|c| c.get_name()) - .collect(); + let subcommands: Vec<&str> = cmd.get_subcommands().map(|c| c.get_name()).collect(); assert!(subcommands.contains(&"+revisions")); assert!(subcommands.contains(&"+write")); } From a4a9f6373fef020fc00cff2c8e4a4a2c6742b78e Mon Sep 17 00:00:00 2001 From: Khali Date: Thu, 19 Mar 2026 16:08:50 +1100 Subject: [PATCH 03/11] fix(docs): sanitize error strings to prevent terminal escape injection --- src/helpers/docs.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index 9f893d16..e1398d73 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -106,7 +106,12 @@ TIPS: let (token, auth_method) = match auth::get_token(&scope_strs).await { Ok(t) => (Some(t), executor::AuthMethod::OAuth), Err(_) if matches.get_flag("dry-run") => (None, executor::AuthMethod::None), - Err(e) => return Err(GwsError::Auth(format!("Docs auth failed: {e}"))), + Err(e) => { + return Err(GwsError::Auth(format!( + "Docs auth failed: {}", + crate::output::sanitize_for_terminal(&e.to_string()) + ))) + } }; // Method: documents.batchUpdate @@ -160,9 +165,12 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { let limit = matches.get_one::("limit").copied().unwrap_or(20); let scope = "https://www.googleapis.com/auth/drive.readonly"; - let token = auth::get_token(&[scope]) - .await - .map_err(|e| GwsError::Auth(format!("Docs auth failed: {e}")))?; + let token = auth::get_token(&[scope]).await.map_err(|e| { + GwsError::Auth(format!( + "Docs auth failed: {}", + crate::output::sanitize_for_terminal(&e.to_string()) + )) + })?; let client = crate::client::build_client()?; let limit_str = limit.to_string(); From ea937113538c1df3adeda0141476f6c33c30ce2b Mon Sep 17 00:00:00 2001 From: Khali Date: Thu, 19 Mar 2026 16:15:48 +1100 Subject: [PATCH 04/11] ci: retrigger CLA check From 0a2f31b31d68425b283a6bf960a19e46f95ade9b Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 11:36:31 +1100 Subject: [PATCH 05/11] fix(docs): address Gemini code review feedback on +revisions - Add .range(1..=1000) to --limit clap value_parser so invalid values are rejected client-side before reaching the API - Extract REVISION_FIELDS constant to avoid hardcoded string duplication and make field list easier to maintain - Percent-encode document_id in the Drive API URL to prevent URL injection / path traversal (uses existing percent-encoding crate) - Replace unwrap_or_default() on error body with unwrap_or_else so secondary failures include a meaningful error message Co-Authored-By: Claude Sonnet 4.6 --- src/helpers/docs.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index e1398d73..b397db3d 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -71,7 +71,7 @@ TIPS: .long("limit") .help("Maximum number of revisions to return (default: 20)") .value_name("N") - .value_parser(clap::value_parser!(u32)), + .value_parser(clap::value_parser!(u32).range(1..=1000)), ) .after_help( "\ @@ -161,6 +161,9 @@ TIPS: } async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { + const REVISION_FIELDS: &str = + "revisions(id,modifiedTime,lastModifyingUser/displayName,keepForever,size)"; + let document_id = matches.get_one::("document").unwrap(); let limit = matches.get_one::("limit").copied().unwrap_or(20); @@ -174,17 +177,16 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { let client = crate::client::build_client()?; let limit_str = limit.to_string(); + let encoded_id = + percent_encoding::utf8_percent_encode(document_id, percent_encoding::NON_ALPHANUMERIC); let resp = client .get(format!( "https://www.googleapis.com/drive/v3/files/{}/revisions", - document_id + encoded_id )) .query(&[ - ( - "fields", - "revisions(id,modifiedTime,lastModifyingUser/displayName,keepForever,size)", - ), + ("fields", REVISION_FIELDS), ("pageSize", limit_str.as_str()), ]) .bearer_auth(&token) @@ -194,7 +196,10 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { if !resp.status().is_success() { let status = resp.status(); - let body = resp.text().await.unwrap_or_default(); + let body = resp + .text() + .await + .unwrap_or_else(|e| format!("Failed to read error response body: {e}")); return Err(GwsError::Api { code: status.as_u16(), message: body, From 61c53904ad7ec4f6e09af65a9b05006dbd62d477 Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 12:53:19 +1100 Subject: [PATCH 06/11] fix(docs): add --dry-run support to +revisions for consistency All other helpers respect --dry-run. Without it, +revisions makes a real API call even when the user passes --dry-run, which is inconsistent with +write and the rest of the codebase. When --dry-run is set: skip auth, print the URL + query params as JSON, return early. Token is now Option to avoid fetching it unnecessarily. Co-Authored-By: Claude Sonnet 4.6 --- src/helpers/docs.rs | 47 +++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index b397db3d..fed6eccc 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -166,30 +166,53 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { let document_id = matches.get_one::("document").unwrap(); let limit = matches.get_one::("limit").copied().unwrap_or(20); + let dry_run = matches.get_flag("dry-run"); let scope = "https://www.googleapis.com/auth/drive.readonly"; - let token = auth::get_token(&[scope]).await.map_err(|e| { - GwsError::Auth(format!( - "Docs auth failed: {}", - crate::output::sanitize_for_terminal(&e.to_string()) - )) - })?; + let token = if dry_run { + None + } else { + Some(auth::get_token(&[scope]).await.map_err(|e| { + GwsError::Auth(format!( + "Docs auth failed: {}", + crate::output::sanitize_for_terminal(&e.to_string()) + )) + })?) + }; - let client = crate::client::build_client()?; let limit_str = limit.to_string(); let encoded_id = percent_encoding::utf8_percent_encode(document_id, percent_encoding::NON_ALPHANUMERIC); + let url = format!( + "https://www.googleapis.com/drive/v3/files/{}/revisions", + encoded_id + ); + + if dry_run { + let dry_run_info = json!({ + "dry_run": true, + "url": url, + "method": "GET", + "query_params": { + "fields": REVISION_FIELDS, + "pageSize": limit_str, + }, + }); + println!( + "{}", + serde_json::to_string_pretty(&dry_run_info).unwrap_or_default() + ); + return Ok(()); + } + let client = crate::client::build_client()?; let resp = client - .get(format!( - "https://www.googleapis.com/drive/v3/files/{}/revisions", - encoded_id - )) + .get(url) .query(&[ ("fields", REVISION_FIELDS), ("pageSize", limit_str.as_str()), ]) - .bearer_auth(&token) + .bearer_auth(token.unwrap()) // safe: dry_run path already returned above .send() .await .map_err(|e| GwsError::Other(anyhow::anyhow!("HTTP request failed: {e}")))?; From 8602ebedf5e61ae4ab80c5caaa03baf7d33c50a9 Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 15:00:50 +1100 Subject: [PATCH 07/11] fix(docs): use build_api_error for consistent Google API error parsing Replace bare GwsError::Api construction with a local build_api_error helper that parses the Google JSON error format (message, reason, accessNotConfigured enable URL) - consistent with the same pattern in helpers/gmail/mod.rs and mirroring executor::handle_error_response. Co-Authored-By: Claude Sonnet 4.6 --- src/helpers/docs.rs | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index fed6eccc..713e6b3c 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -223,12 +223,7 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { .text() .await .unwrap_or_else(|e| format!("Failed to read error response body: {e}")); - return Err(GwsError::Api { - code: status.as_u16(), - message: body, - reason: "revisions_request_failed".to_string(), - enable_url: None, - }); + return Err(build_api_error(status.as_u16(), &body)); } let value: Value = resp @@ -244,6 +239,42 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { Ok(()) } +/// Build a from an HTTP error response, parsing the Google +/// JSON error format when available. Mirrors the logic in . +fn build_api_error(status: u16, body: &str) -> GwsError { + let err_json: Option = serde_json::from_str(body).ok(); + let err_obj = err_json.as_ref().and_then(|v| v.get("error")); + let message = err_obj + .and_then(|e| e.get("message")) + .and_then(|m| m.as_str()) + .unwrap_or(body) + .to_string(); + let reason = err_obj + .and_then(|e| e.get("errors")) + .and_then(|e| e.as_array()) + .and_then(|arr| arr.first()) + .and_then(|e| e.get("reason")) + .and_then(|r| r.as_str()) + .or_else(|| { + err_obj + .and_then(|e| e.get("reason")) + .and_then(|r| r.as_str()) + }) + .unwrap_or("unknown") + .to_string(); + let enable_url = if reason == "accessNotConfigured" { + crate::executor::extract_enable_url(&message) + } else { + None + }; + GwsError::Api { + code: status, + message, + reason, + enable_url, + } +} + fn build_write_request( matches: &ArgMatches, doc: &crate::discovery::RestDescription, From d8d9e3dddde7497f801042a06dfb1d032d05d987 Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 15:32:56 +1100 Subject: [PATCH 08/11] fix(docs): eliminate build_api_error duplication via GwsError::from_api_response Add GwsError::from_api_response(status, body) to error.rs as the canonical shared method for parsing Google API error responses. Remove the local build_api_error function from docs.rs that duplicated this logic. Both executor::handle_error_response and the revisions helper now use the same parsing path, eliminating the duplication flagged in code review. --- src/error.rs | 39 +++++++++++++++++++++++++++++++++++++++ src/helpers/docs.rs | 38 +------------------------------------- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/error.rs b/src/error.rs index 40b14ba6..88b5169b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -96,6 +96,45 @@ impl GwsError { } } + /// Build a [] by parsing a Google API error response body. + /// + /// Extracts , , and (for errors) the + /// GCP console URL to enable the API. Falls back to the raw body when the + /// response is not well-formed Google JSON. + pub fn from_api_response(status: u16, body: &str) -> Self { + let err_json: Option = serde_json::from_str(body).ok(); + let err_obj = err_json.as_ref().and_then(|v| v.get("error")); + let message = err_obj + .and_then(|e| e.get("message")) + .and_then(|m| m.as_str()) + .unwrap_or(body) + .to_string(); + let reason = err_obj + .and_then(|e| e.get("errors")) + .and_then(|e| e.as_array()) + .and_then(|arr| arr.first()) + .and_then(|e| e.get("reason")) + .and_then(|r| r.as_str()) + .or_else(|| { + err_obj + .and_then(|e| e.get("reason")) + .and_then(|r| r.as_str()) + }) + .unwrap_or("unknown") + .to_string(); + let enable_url = if reason == "accessNotConfigured" { + crate::executor::extract_enable_url(&message) + } else { + None + }; + GwsError::Api { + code: status, + message, + reason, + enable_url, + } + } + pub fn to_json(&self) -> serde_json::Value { match self { GwsError::Api { diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index 713e6b3c..12e4c7c1 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -223,7 +223,7 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { .text() .await .unwrap_or_else(|e| format!("Failed to read error response body: {e}")); - return Err(build_api_error(status.as_u16(), &body)); + return Err(GwsError::from_api_response(status.as_u16(), &body)); } let value: Value = resp @@ -239,42 +239,6 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { Ok(()) } -/// Build a from an HTTP error response, parsing the Google -/// JSON error format when available. Mirrors the logic in . -fn build_api_error(status: u16, body: &str) -> GwsError { - let err_json: Option = serde_json::from_str(body).ok(); - let err_obj = err_json.as_ref().and_then(|v| v.get("error")); - let message = err_obj - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .unwrap_or(body) - .to_string(); - let reason = err_obj - .and_then(|e| e.get("errors")) - .and_then(|e| e.as_array()) - .and_then(|arr| arr.first()) - .and_then(|e| e.get("reason")) - .and_then(|r| r.as_str()) - .or_else(|| { - err_obj - .and_then(|e| e.get("reason")) - .and_then(|r| r.as_str()) - }) - .unwrap_or("unknown") - .to_string(); - let enable_url = if reason == "accessNotConfigured" { - crate::executor::extract_enable_url(&message) - } else { - None - }; - GwsError::Api { - code: status, - message, - reason, - enable_url, - } -} - fn build_write_request( matches: &ArgMatches, doc: &crate::discovery::RestDescription, From 6e782cf064e22a751b958fc74220294b2bd59168 Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 15:43:03 +1100 Subject: [PATCH 09/11] fix(docs): use send_with_retry for rate-limit handling; restore Cargo.lock v4 Switch +revisions from direct .send().await to crate::client::send_with_retry so HTTP 429 rate-limit responses are retried consistently with all other helpers. Restore Cargo.lock format version from 3 to 4 to match upstream; no dependency changes, only the version header was different. --- Cargo.lock | 2 +- src/helpers/docs.rs | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 873bd275..af98fd1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "aead" diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index 12e4c7c1..61f35788 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -205,17 +205,19 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { return Ok(()); } + let token = token.unwrap(); // safe: dry_run path already returned above let client = crate::client::build_client()?; - let resp = client - .get(url) - .query(&[ - ("fields", REVISION_FIELDS), - ("pageSize", limit_str.as_str()), - ]) - .bearer_auth(token.unwrap()) // safe: dry_run path already returned above - .send() - .await - .map_err(|e| GwsError::Other(anyhow::anyhow!("HTTP request failed: {e}")))?; + let resp = crate::client::send_with_retry(|| { + client + .get(url.clone()) + .query(&[ + ("fields", REVISION_FIELDS), + ("pageSize", limit_str.as_str()), + ]) + .bearer_auth(token.clone()) + }) + .await + .map_err(|e| GwsError::Other(anyhow::anyhow!("HTTP request failed: {e}")))?; if !resp.status().is_success() { let status = resp.status(); From ac5ece246cd4ca499254958f62c8601622b29b98 Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 15:52:32 +1100 Subject: [PATCH 10/11] fix(docs): fix from_api_response doc comment and use body code field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix mangled rustdoc backtick references in from_api_response doc comment. Prefer the code field from the Google API JSON error body over the HTTP status code, falling back to status when absent — consistent with executor::handle_error_response. --- src/error.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index 88b5169b..6cf76cfa 100644 --- a/src/error.rs +++ b/src/error.rs @@ -96,14 +96,19 @@ impl GwsError { } } - /// Build a [] by parsing a Google API error response body. + /// Build a [`GwsError::Api`] by parsing a Google API error response body. /// - /// Extracts , , and (for errors) the + /// Extracts `code`, `message`, `reason`, and (for `accessNotConfigured` errors) the /// GCP console URL to enable the API. Falls back to the raw body when the /// response is not well-formed Google JSON. pub fn from_api_response(status: u16, body: &str) -> Self { let err_json: Option = serde_json::from_str(body).ok(); let err_obj = err_json.as_ref().and_then(|v| v.get("error")); + let code = err_obj + .and_then(|e| e.get("code")) + .and_then(|c| c.as_u64()) + .map(|c| c as u16) + .unwrap_or(status); let message = err_obj .and_then(|e| e.get("message")) .and_then(|m| m.as_str()) @@ -128,7 +133,7 @@ impl GwsError { None }; GwsError::Api { - code: status, + code, message, reason, enable_url, From 000b9935126a2ce1d1e6a6e95ca377cb23e7ae0e Mon Sep 17 00:00:00 2001 From: Khali Date: Fri, 20 Mar 2026 16:03:51 +1100 Subject: [PATCH 11/11] test(docs): extract build_revisions_request and add unit tests Extract URL construction logic into build_revisions_request() for testability, following the build_write_request pattern. Add four unit tests covering: - correct URL construction for a plain document ID - percent-encoding of special characters in document IDs - limit_str formatting - dry-run JSON output structure and field values --- src/helpers/docs.rs | 89 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/src/helpers/docs.rs b/src/helpers/docs.rs index 61f35788..1f82c098 100644 --- a/src/helpers/docs.rs +++ b/src/helpers/docs.rs @@ -160,10 +160,28 @@ TIPS: } } -async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { - const REVISION_FIELDS: &str = - "revisions(id,modifiedTime,lastModifyingUser/displayName,keepForever,size)"; +const REVISION_FIELDS: &str = + "revisions(id,modifiedTime,lastModifyingUser/displayName,keepForever,size)"; + +struct RevisionsRequest { + url: String, + limit_str: String, +} + +fn build_revisions_request(document_id: &str, limit: u32) -> RevisionsRequest { + let encoded_id = + percent_encoding::utf8_percent_encode(document_id, percent_encoding::NON_ALPHANUMERIC); + let url = format!( + "https://www.googleapis.com/drive/v3/files/{}/revisions", + encoded_id + ); + RevisionsRequest { + url, + limit_str: limit.to_string(), + } +} +async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { let document_id = matches.get_one::("document").unwrap(); let limit = matches.get_one::("limit").copied().unwrap_or(20); let dry_run = matches.get_flag("dry-run"); @@ -180,13 +198,9 @@ async fn handle_revisions(matches: &ArgMatches) -> Result<(), GwsError> { })?) }; - let limit_str = limit.to_string(); - let encoded_id = - percent_encoding::utf8_percent_encode(document_id, percent_encoding::NON_ALPHANUMERIC); - let url = format!( - "https://www.googleapis.com/drive/v3/files/{}/revisions", - encoded_id - ); + let req = build_revisions_request(document_id, limit); + let url = req.url; + let limit_str = req.limit_str; if dry_run { let dry_run_info = json!({ @@ -356,4 +370,59 @@ mod tests { .unwrap(); assert!(doc_arg.is_required_set()); } + + #[test] + fn test_build_revisions_request_url() { + let req = build_revisions_request("abc123", 20); + assert_eq!( + req.url, + "https://www.googleapis.com/drive/v3/files/abc123/revisions" + ); + assert_eq!(req.limit_str, "20"); + } + + #[test] + fn test_build_revisions_request_percent_encodes_document_id() { + let req = build_revisions_request("abc/def 123", 5); + // slash and space in document ID must be percent-encoded + assert!( + req.url.contains("abc%2Fdef%20123"), + "document id should be percent-encoded in URL: {}", + req.url + ); + // the raw unencoded document ID must not appear + assert!( + !req.url.contains("abc/def 123"), + "raw document id must not appear unencoded in URL: {}", + req.url + ); + } + + #[test] + fn test_build_revisions_request_limit_str() { + let req = build_revisions_request("docid", 42); + assert_eq!(req.limit_str, "42"); + } + + #[test] + fn test_revisions_dry_run_output_structure() { + let req = build_revisions_request("testdoc", 10); + let dry_run_info = serde_json::json!({ + "dry_run": true, + "url": req.url, + "method": "GET", + "query_params": { + "fields": REVISION_FIELDS, + "pageSize": req.limit_str, + }, + }); + assert_eq!(dry_run_info["dry_run"], true); + assert!(dry_run_info["url"].as_str().unwrap().contains("testdoc")); + assert_eq!(dry_run_info["method"], "GET"); + assert!(dry_run_info["query_params"]["fields"] + .as_str() + .unwrap() + .contains("revisions")); + assert_eq!(dry_run_info["query_params"]["pageSize"], "10"); + } }