Sprint 5R: Gmail refresh-token rotation handling

BUILD_REPORT.md

@@ -2,129 +2,64 @@
 
 ## sprint objective
 
-Implement Sprint 5Q: extend the protected Gmail credential seam so the existing single-message ingestion path can renew an expired access token from refresh-token-backed credentials, while keeping Gmail account reads secret-free and leaving broader Gmail and Calendar scope deferred.
+Implement Sprint 5R: extend the existing Gmail refresh path so single-message ingestion can persist and use a provider-rotated refresh token without exposing secrets in Gmail account reads or expanding into broader Gmail, Calendar, secret-manager, runner, compile-contract, or UI scope.
 
 ## completed work
 
-- Added refresh-token-capable Gmail connect writes with an all-or-none refresh bundle:
-  - `refresh_token`
-  - `client_id`
-  - `client_secret`
-  - `access_token_expires_at`
-- Extended the protected Gmail credential blob to support two narrow shapes:
-  - `gmail_oauth_access_token_v1`
-  - `gmail_oauth_refresh_token_v2`
-- Added one explicit Gmail token-refresh path using `https://oauth2.googleapis.com/token`.
-- Updated Gmail credential resolution so ingestion:
-  - uses the stored access token directly when the credential is still usable
-  - renews first when a refresh-capable credential is expired
-  - persists the refreshed access token and new expiry back into `gmail_account_credentials`
-- Added a narrow protected-credential update store method for deterministic renewal writes.
-- Added migration `20260316_0028_gmail_refresh_token_lifecycle.py` to:
-  - allow the new refreshable credential blob shape
-  - grant runtime `UPDATE` on `gmail_account_credentials`
-  - downgrade refreshable blobs back to the v1 access-token-only shape
-- Kept Gmail account list/detail/connect responses secret-free on reads.
-- Added unit and integration coverage for:
-  - refresh-token credential persistence
-  - successful renewal before single-message ingestion
-  - deterministic invalid refresh-credential failure
-  - secret-free account responses
-  - per-user isolation
-  - migration upgrade/downgrade compatibility
-- Added direct unit coverage for the raw `refresh_gmail_access_token()` helper, including success, `400/401` rejection mapping, and malformed or transport refresh failures.
-- Updated `ARCHITECTURE.md` so the implemented slice and Gmail connector flow reflect Sprint 5Q refresh-token lifecycle behavior.
-
-## exact Gmail refresh-token credential changes introduced
-
-- `GmailAccountConnectInput` now accepts optional `refresh_token`, `client_id`, `client_secret`, and `access_token_expires_at`.
-- `ConnectGmailAccountRequest` enforces that those four refresh fields are all present together or all absent.
-- Protected credential v1 remains:
-
-```json
-{
-  "credential_kind": "gmail_oauth_access_token_v1",
-  "access_token": "<token>"
-}
-```
-
-- New protected credential v2 is:
-
-```json
-{
-  "credential_kind": "gmail_oauth_refresh_token_v2",
-  "access_token": "<token>",
-  "refresh_token": "<refresh-token>",
-  "client_id": "<oauth-client-id>",
-  "client_secret": "<oauth-client-secret>",
-  "access_token_expires_at": "<iso8601>"
-}
-```
-
-- `gmail_account_credentials` now allows both v1 and v2 blobs and runtime `UPDATE` so renewal can rewrite the protected record in place.
-
-## token-renewal rule and renewal trigger used
-
-- Renewal trigger: if the protected credential kind is `gmail_oauth_refresh_token_v2` and `access_token_expires_at <= now(UTC)` during Gmail message ingestion.
-- Renewal path: POST once to Google’s OAuth token endpoint with `client_id`, `client_secret`, `refresh_token`, and `grant_type=refresh_token`.
-- Success behavior: store the new `access_token` plus a recalculated `access_token_expires_at`, then continue the existing single-message Gmail fetch with that refreshed token.
-- Non-renewal behavior: if the credential is v1 or the v2 token has not yet expired, use the stored access token directly.
-- Deterministic failure behavior:
-  - malformed local protected credentials -> `409`
-  - Google refresh rejection (`400`/`401`) -> `409`
-  - non-deterministic refresh transport/response failures -> `502`
-- Failure guardrail: these failures occur before artifact registration, chunk ingestion, or filesystem writes for the Gmail message.
+- Added rotation-aware Gmail refresh handling in `apps/api/src/alicebot_api/gmail.py`.
+- Changed the refresh helper to capture an optional provider-returned replacement `refresh_token` alongside the renewed access token and expiry.
+- Applied the protected-credential replacement rule:
+  - if Google returns a non-empty `refresh_token`, persist that replacement token
+  - otherwise keep the existing stored `refresh_token`
+- Kept renewal writes inside the existing `gmail_account_credentials` seam and continued using the existing single-message ingestion path after a successful protected-credential update.
+- Added a deterministic Gmail-specific persistence failure when renewed protected credentials cannot be written back, and mapped that failure to the existing `409` error envelope in `apps/api/src/alicebot_api/main.py`.
+- Preserved secret-free Gmail account reads; no secret fields were added to list/detail/connect/ingest responses.
+- Added unit coverage for:
+  - renewal without refresh-token rotation
+  - renewal with refresh-token rotation
+  - deterministic failure when protected-credential persistence fails
+  - raw refresh helper parsing of provider-returned rotated refresh tokens
+- Added integration coverage for:
+  - renewal success without refresh-token rotation
+  - renewal success with refresh-token rotation
+  - deterministic failure when rotated credentials cannot be persisted
+  - unchanged secret-free response shape during the rotation-capable path
 
 ## incomplete work
 
-- None inside Sprint 5Q scope.
+- None inside Sprint 5R scope.
 
 ## files changed
 
-- `apps/api/src/alicebot_api/contracts.py`
 - `apps/api/src/alicebot_api/gmail.py`
 - `apps/api/src/alicebot_api/main.py`
-- `apps/api/src/alicebot_api/store.py`
-- `apps/api/alembic/versions/20260316_0028_gmail_refresh_token_lifecycle.py`
-- `ARCHITECTURE.md`
 - `tests/unit/test_gmail.py`
 - `tests/unit/test_gmail_main.py`
 - `tests/unit/test_gmail_refresh.py`
-- `tests/unit/test_20260316_0028_gmail_refresh_token_lifecycle.py`
 - `tests/integration/test_gmail_accounts_api.py`
-- `tests/integration/test_migrations.py`
 - `BUILD_REPORT.md`
 
-## exact commands run
+## tests run
 
-- `./.venv/bin/python -m pytest tests/unit/test_gmail.py tests/unit/test_gmail_main.py tests/unit/test_20260316_0028_gmail_refresh_token_lifecycle.py`
-- `./.venv/bin/python -m pytest tests/integration/test_gmail_accounts_api.py tests/integration/test_migrations.py`
-- `./.venv/bin/python -m pytest tests/unit/test_gmail_refresh.py tests/unit/test_gmail.py tests/unit/test_gmail_main.py`
 - `./.venv/bin/python -m pytest tests/unit`
+  - Result: `437 passed in 0.64s`
 - `./.venv/bin/python -m pytest tests/integration`
+  - Result: `139 passed in 45.52s`
 
-## tests run
+## exact commands run
 
-- `./.venv/bin/python -m pytest tests/unit/test_gmail.py tests/unit/test_gmail_main.py tests/unit/test_20260316_0028_gmail_refresh_token_lifecycle.py`
-  - Result: `27 passed in 0.52s`
-- `./.venv/bin/python -m pytest tests/integration/test_gmail_accounts_api.py tests/integration/test_migrations.py`
-  - Result: `12 passed in 4.35s`
-- `./.venv/bin/python -m pytest tests/unit/test_gmail_refresh.py tests/unit/test_gmail.py tests/unit/test_gmail_main.py`
-  - Result: `31 passed in 0.29s`
+- `./.venv/bin/python -m pytest tests/unit/test_gmail.py tests/unit/test_gmail_refresh.py tests/unit/test_gmail_main.py`
+- `./.venv/bin/python -m pytest tests/unit/test_gmail.py tests/unit/test_gmail_refresh.py tests/unit/test_gmail_main.py tests/integration/test_gmail_accounts_api.py -k 'gmail'`
 - `./.venv/bin/python -m pytest tests/unit`
-  - Result: `434 passed in 0.67s`
 - `./.venv/bin/python -m pytest tests/integration`
-  - Result: `137 passed in 41.99s`
 
 ## unit and integration test results
 
-- Required unit suite passed.
-- Required integration suite passed.
-- Gmail-focused unit and integration coverage passed before the full packet-level runs.
-- Direct unit coverage now pins the raw Gmail token-refresh helper’s success and error mapping behavior.
-- Migration coverage now verifies both:
-  - the original Gmail credential hardening round trip
-  - the new refresh-token lifecycle migration round trip back to a downgrade-safe v1 blob
+- Gmail-focused unit tests passed after updating the remaining stale test double to the new refresh return shape.
+- One intermediate sandboxed combined Gmail run could not reach the local Postgres fixture on `localhost:5432`; this was an environment restriction, not an application test failure.
+- Required final verification passed:
+  - `tests/unit`: `437 passed in 0.64s`
+  - `tests/integration`: `139 passed in 45.52s`
 
 ## one example Gmail account response proving secret-free reads remain intact
 
@@ -134,7 +69,7 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
     "id": "<gmail-account-id>",
     "provider": "gmail",
     "auth_kind": "oauth_access_token",
-    "provider_account_id": "acct-owner-refresh-001",
+    "provider_account_id": "acct-owner-rotated-001",
     "email_address": "owner@gmail.example",
     "display_name": "Owner",
     "scope": "https://www.googleapis.com/auth/gmail.readonly",
@@ -144,15 +79,15 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
 }
 ```
 
-## one example Gmail ingestion response through the renewal-capable path
+## one example Gmail ingestion response through the rotation-capable path
 
 ```json
 {
   "account": {
     "id": "<gmail-account-id>",
     "provider": "gmail",
     "auth_kind": "oauth_access_token",
-    "provider_account_id": "acct-owner-refresh-001",
+    "provider_account_id": "acct-owner-rotated-001",
     "email_address": "owner@gmail.example",
     "display_name": "Owner",
     "scope": "https://www.googleapis.com/auth/gmail.readonly",
@@ -161,7 +96,7 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
   },
   "message": {
     "provider_message_id": "msg-001",
-    "artifact_relative_path": "gmail/acct-owner-refresh-001/msg-001.eml",
+    "artifact_relative_path": "gmail/acct-owner-rotated-001/msg-001.eml",
     "media_type": "message/rfc822"
   },
   "artifact": {
@@ -170,14 +105,14 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
     "task_workspace_id": "<task-workspace-id>",
     "status": "registered",
     "ingestion_status": "ingested",
-    "relative_path": "gmail/acct-owner-refresh-001/msg-001.eml",
+    "relative_path": "gmail/acct-owner-rotated-001/msg-001.eml",
     "media_type_hint": "message/rfc822",
     "created_at": "<created-at>",
     "updated_at": "<updated-at>"
   },
   "summary": {
-    "total_count": 1,
-    "total_characters": 18,
+    "total_count": "<chunk-count>",
+    "total_characters": "<character-count>",
     "media_type": "message/rfc822",
     "chunking_rule": "normalized_utf8_text_fixed_window_1000_chars_v1",
     "order": ["sequence_no_asc", "id_asc"]
@@ -188,7 +123,7 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
 ## blockers/issues
 
 - No remaining implementation blockers.
-- Integration verification required local Postgres access outside the default sandbox.
+- Integration verification required elevated access because the default sandbox blocked connections to the local Postgres test fixture on `localhost:5432`.
 
 ## what remains intentionally deferred to later milestones
 
@@ -202,8 +137,7 @@ Implement Sprint 5Q: extend the protected Gmail credential seam so the existing
 - compile-contract changes
 - runner-style orchestration
 - UI work
-- refresh-token rotation beyond the single explicit renewal path
 
 ## recommended next step
 
-Keep the next Gmail milestone equally narrow: either add refresh-token rotation handling or move the protected Gmail secret seam into an external secret manager, but do not combine that work with search, sync, Calendar, or UI expansion.
+Keep the next Gmail sprint narrow around one adjacent auth seam only, such as external secret-manager integration for the existing protected credential store, without combining it with search, sync, Calendar, or UI expansion.

REVIEW_REPORT.md

@@ -6,46 +6,45 @@ PASS
 
 ## criteria met
 
-- The sprint remains within the Sprint 5Q boundary. I found no Gmail search, sync, attachments, write actions, Calendar, compile-contract, runner, or UI scope in the implementation or follow-up changes.
-- The protected Gmail credential seam supports the narrow refresh-token-capable credential shape required by the packet, without reintroducing secrets to `gmail_accounts`.
-- Gmail account connect, list, and detail reads remain secret-free. Coverage asserts that `access_token`, `refresh_token`, and `client_secret` do not appear in account response payloads.
-- The existing single-message Gmail ingestion path renews an expired access token through one explicit refresh path and then continues through the existing RFC822 artifact ingestion seam.
-- Missing or invalid refresh credentials fail deterministically before fetch, artifact registration, chunk ingestion, or workspace writes.
-- Per-user isolation remains intact through the existing user-scoped store access, row-level security, and the `(gmail_account_id, user_id)` ownership binding on protected credentials.
-- Migration coverage includes the refresh-token lifecycle round trip and downgrade compatibility.
-- The prior review follow-ups are now addressed:
-  - `ARCHITECTURE.md` reflects Sprint 5Q and the renewal-before-ingestion Gmail seam.
-  - `tests/unit/test_gmail_refresh.py` adds direct coverage for the raw refresh helper success path, `400/401` rejection mapping, and malformed or transport failures.
-- `BUILD_REPORT.md` now matches the implemented Sprint 5Q behavior and includes the follow-up test coverage/doc updates.
-- Verified locally:
-  - `./.venv/bin/python -m pytest tests/unit/test_gmail_refresh.py tests/unit/test_gmail.py tests/unit/test_gmail_main.py` -> `31 passed in 0.28s`
-  - `./.venv/bin/python -m pytest tests/unit` -> `434 passed in 0.64s`
-  - `./.venv/bin/python -m pytest tests/integration` -> `137 passed in 40.83s` from the prior sprint review run, with no subsequent code-path changes beyond docs and unit-test additions
+- Sprint stayed narrow. The code changes are limited to the Gmail renewal seam, the ingest endpoint error mapping, Gmail-focused tests, and `BUILD_REPORT.md`.
+- Rotated refresh tokens are now handled in the protected credential seam. `apps/api/src/alicebot_api/gmail.py` captures an optional provider-returned `refresh_token` during renewal and persists it back through `gmail_account_credentials`.
+- The replacement rule matches the packet:
+  - if the provider returns a non-empty replacement `refresh_token`, persist it
+  - otherwise keep the existing stored `refresh_token`
+- Gmail account reads remain secret-free. No secret fields were added to list/detail/connect/ingest responses, and existing account list/detail isolation tests still pass.
+- Single-message Gmail ingestion still works for both cases:
+  - stable refresh-token renewal
+  - rotated refresh-token renewal
+- Rotated-credential persistence failures are deterministic and happen before Gmail fetch/artifact writes. The new error is mapped to the existing `409` envelope, and tests verify no artifact or credential corruption when persistence fails.
+- Required verification passed:
+  - `./.venv/bin/python -m pytest tests/unit` -> `437 passed in 0.63s`
+  - `./.venv/bin/python -m pytest tests/integration` -> `139 passed in 42.27s`
+- No out-of-scope Gmail search, sync, attachments, write actions, Calendar, external secret-manager, compile-contract, runner, or UI work entered the sprint.
 
 ## criteria missed
 
 - None.
 
 ## quality issues
 
-- No blocking implementation, test, documentation, regression, or scope issues remain for Sprint 5Q.
+- None material for Sprint 5R.
 
 ## regression risks
 
-- Residual risk is limited to intentionally deferred scope already called out by the sprint packet: Gmail search, sync, attachments, write actions, Calendar, external secret-manager integration, compile-contract changes, runner orchestration, UI work, and refresh-token rotation beyond the single explicit renewal path.
+- Low. The change is localized to the existing Gmail renewal path and is covered by both unit and Postgres-backed integration tests for stable-token renewal, rotated-token renewal, failure handling, secret-free responses, and user isolation.
 
 ## docs issues
 
-- None.
+- None. `BUILD_REPORT.md` includes the rotation change summary, replacement rule, commands run, test results, secret-free account example, rotation-capable ingestion example, and deferred scope.
 
 ## should anything be added to RULES.md?
 
-- No.
+- No. This is a narrow connector implementation detail, not a new repository-wide rule.
 
 ## should anything update ARCHITECTURE.md?
 
-- No further architecture update is required for this sprint.
+- No. The sprint does not introduce a new architectural boundary or subsystem; it hardens the existing Gmail protected-credential seam.
 
 ## recommended next action
 
-- Accept Sprint 5Q as complete and proceed with the normal merge/approval flow. Keep the next Gmail milestone equally narrow and avoid combining secret-lifecycle work with search, sync, Calendar, or UI expansion.
+- Accept Sprint 5R and move to the next narrow auth-adjacent milestone without broadening scope.