Conversation
🦋 Changeset detectedLatest commit: 8fbb88a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
googleworkspace-bot
added
area: docs
area: http
area: core
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the execution contract for AI agents by introducing semantic exit codes, an idempotency key for safe retries, and graceful SIGTERM handling for long-running processes. The changes are well-implemented and include relevant tests. However, I've found a critical issue in the new SIGTERM handling logic where a potential panic could prevent a clean shutdown, which I've detailed in the comments.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a stable execution contract for AI agents, which is a great improvement for reliability. The changes include semantic exit codes, an --idempotency-key for safe retries, and SIGTERM handling for clean shutdowns. The implementation is solid, but I have a couple of suggestions to improve the maintainability of the new exit code contract and the robustness of the exit-codes command.
romamo
force-pushed
the
feat/execution-contract
branch
from
37ce64c to
7cc3fb5
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a stable execution contract with semantic exit codes, an idempotency key for safe retries, and SIGTERM handling for graceful shutdowns. This is a great improvement for agent reliability. My review found a critical inconsistency in the implementation of the semantic exit codes. The codes defined in the code do not match what is described in the pull request description and the updated README.md, which could lead to incorrect error handling by agents.
|
Rebased onto main after PR #428 merged and addressed all review feedback: Exit code mapping — SIGTERM panic — replaced Duplicate exit code tests — removed the tests added in this PR that The exit code constants and |
- Semantic exit codes: errors now exit with typed codes (1=validation, 2=auth, 3=api, 4=discovery, 5=internal) instead of always exiting 1 - `gws exit-codes`: new command outputs machine-readable exit code taxonomy as JSON for agent retry/abort branching - Schema enrichment: `gws schema <method>` now includes `timeout_ms` (30000) and `exit_codes` array so agents can self-document a method - SIGTERM handling: `gws gmail +watch` and `gws events +subscribe` now handle SIGTERM for clean shutdown alongside existing Ctrl+C support - `--idempotency-key`: new global flag injects an `Idempotency-Key` HTTP header on POST/PUT/PATCH requests; surfaced in --dry-run output
Add exit code taxonomy, --idempotency-key usage, and gws exit-codes examples to the "For AI agents" section so agents can discover the retry/abort contract without reading source.
Replace .expect() with .context()? so a failure to register the SIGTERM handler returns a GwsError::Other rather than panicking — consistent with the clean-shutdown goal. Addresses review comment from gemini-code-assist.
romamo
force-pushed
the
feat/execution-contract
branch
from
7cc3fb5 to
84fc563
Compare
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
…ion error - Introduce EXIT_CODE_TABLE as the canonical (code, reason, meaning) array; EXIT_CODE_DOCUMENTATION and exit_codes_json() both derive from it so the three representations can no longer drift - Replace unwrap_or_default() in the exit-codes command with proper error propagation so a serialization failure surfaces as GwsError::Other instead of silently printing empty output Addresses gemini-code-assist review comments on PR googleworkspace#538.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a more robust execution contract for AI agents by adding semantic exit codes, a command to discover them, schema enrichment with timeouts, idempotency keys for safe retries, and graceful SIGTERM handling. The implementation is solid, but there are a few critical inconsistencies in the documentation regarding the new exit codes that could confuse users and agents. I've also identified a couple of areas in the code that could be made more robust to prevent future maintenance issues.
…docs - Replace hardcoded EXIT_CODE_TABLE index literals in EXIT_CODE_DOCUMENTATION with a const macro loop so adding/reordering entries never silently breaks the documentation array - Derive test_exit_codes_json_contains_all_codes expected codes from EXIT_CODE_TABLE instead of hardcoded 0..=5 - Fix incorrect exit code mapping in README and changeset (was 1=validation, 3=api; correct is 1=api, 3=validation per EXIT_CODE_API/VALIDATION constants) Addresses gemini-code-assist review 3966742572.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable execution contract for AI agents, including semantic exit codes, an idempotency key feature, and SIGTERM handling. The implementation is solid, but I've identified two issues. One is a critical discrepancy where a timeout advertised in the schema is not enforced in the HTTP client, which could cause agents to hang. The other is a high-severity issue where providing an empty idempotency key would likely lead to API errors.
…ncy key - Extract SCHEMA_TIMEOUT_MS constant from schema.rs and apply it as the reqwest client timeout in client.rs so the contract advertised in schema output is actually enforced on every request - Filter out empty --idempotency-key values so passing "" behaves the same as omitting the flag instead of sending a blank Idempotency-Key header Addresses gemini-code-assist review 3966832626.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the execution contract for AI agents by introducing semantic exit codes, a new gws exit-codes command, schema enrichment with timeouts, an --idempotency-key for safe mutations, and graceful SIGTERM handling for long-running commands. The changes are well-structured and the new features are valuable for improving reliability.
I have one high-severity suggestion to improve the implementation: to make the new test for exit codes more robust.
… extras Addresses gemini-code-assist review 3966901258.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a more robust execution contract for AI agents, including semantic exit codes, a new gws exit-codes command, schema enrichment with timeouts, an --idempotency-key flag for safe mutations, and graceful SIGTERM handling. The implementation is solid, but I've identified a couple of areas for improvement regarding code duplication and the use of modern Rust idioms, which would enhance maintainability.
…ration - Replace macro_rules! with a const fn build_exit_code_documentation() — same compile-time derivation from EXIT_CODE_TABLE, more idiomatic Rust - Extract SIGTERM signal registration into helpers::register_sigterm() so gmail::watch and events::subscribe share one definition instead of two Addresses gemini-code-assist review 3966990380.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the execution contract for agent reliability by introducing semantic exit codes, a new gws exit-codes command, schema enrichment with timeouts, an --idempotency-key for safe mutations, and SIGTERM handling for long-running processes. The implementation is robust and well-tested.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
==========================================
+ Coverage 68.10% 68.73% +0.63%
==========================================
Files 40 41 +1
Lines 17954 18761 +807
==========================================
+ Hits 12227 12895 +668
- Misses 5727 5866 +139 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Exit codes already in use, no need for a command, pr includes too many other changes. |
3 participants
Motivation
AI agents calling
gwscurrently receive exit code1for every failure —auth errors, bad input, API blips, and internal panics all look identical.
This forces agents to parse stderr JSON to decide whether to retry, abort, or
escalate, which is fragile and couples them to error message formatting.
What This Adds
PR #428 established the exit code constants and
exit_code()method. This PRbuilds the agent-facing contract on top of that foundation:
exit_codes_json()— JSON taxonomy usingEXIT_CODE_*constantssrc/error.rsgws exit-codescommand — machine-readable taxonomy at runtimesrc/main.rstimeout_ms: 30000+exit_codesarray in everygws schemaresponsesrc/schema.rs--idempotency-keyglobal flag — injectsIdempotency-Keyon POST/PUT/PATCHsrc/commands.rs,src/executor.rs+watchand+subscribepull loopssrc/helpers/gmail/watch.rs,src/helpers/events/subscribe.rsImpact
An agent can now:
gws exit-codes | jq--idempotency-keygws schema <method>before callingExit Codes (from PR #428)
Dry Run Output:
gws exit-codes{ "exit_codes": [ { "code": 0, "meaning": "Command completed successfully", "reason": "success" }, { "code": 1, "meaning": "Remote API returned an error response", "reason": "apiError" }, { "code": 2, "meaning": "Missing or invalid credentials", "reason": "authError" }, { "code": 3, "meaning": "Bad input or wrong arguments", "reason": "validationError" }, { "code": 4, "meaning": "Could not load API Discovery document", "reason": "discoveryError" }, { "code": 5, "meaning": "Unexpected or transient failure", "reason": "internalError" } ] }gws drive files create --idempotency-key my-key-123 --dry-run{ "body": null, "dry_run": true, "idempotency_key": "my-key-123", "is_multipart_upload": false, "method": "POST", "query_params": [], "url": "https://www.googleapis.com/drive/v3/files" }gws schema drive.files.list | jq '{timeout_ms, exit_codes}'{ "timeout_ms": 30000, "exit_codes": [ { "code": 0, "reason": "success", "meaning": "Command completed successfully" }, { "code": 1, "reason": "apiError", "meaning": "Remote API returned an error response" }, { "code": 2, "reason": "authError", "meaning": "Missing or invalid credentials" }, { "code": 3, "reason": "validationError", "meaning": "Bad input or wrong arguments" }, { "code": 4, "reason": "discoveryError", "meaning": "Could not load API Discovery document" }, { "code": 5, "reason": "internalError", "meaning": "Unexpected or transient failure" } ] }Checklist: