fix(auth): reconcile observed OAuth scopes

[steipete]

Summary

• Reconcile stored OAuth scope metadata from scopes observed in successful token refresh responses.
• Add the service label only when the observed grant covers that service's canonical scope set.
• Add regression coverage for observed scope upgrades and partial-scope service over-reporting.

Fixes #649.

Testing

• go test ./internal/googleapi -run 'TestPersistingTokenSource|TestTokenSourceForAccountScopes'
• go test ./internal/googleapi ./internal/googleauth ./internal/cmd
• make test
• Live clawdbot smoke: gmail labels list returned 16 labels; post-refresh auth list reported Gmail service and 47 scopes.
• Autoreview: clean.

[clawsweeper]

Codex review: needs changes before merge. Reviewed May 30, 2026, 2:18 PM ET / 18:18 UTC.

Summary
The PR updates OAuth token refresh persistence to reconcile observed granted scopes into stored token metadata and adds regression tests for service-label reporting.

Reproducibility: yes. from source inspection. Current main does not reconcile observed refresh scopes into stored metadata, and the PR branch still has a clear split-scope service gap because Contacts requests one canonical scope at a time.

Review metrics: 2 noteworthy metrics.

• Changed files: 3 files affected. The patch is focused on OAuth token persistence, its regression tests, and one changelog entry.
• Scope tests added: 3 new reconciliation tests. The added tests cover full observed grants, partial grants, and avoiding requested-scope persistence, but not accumulated split-scope service repair.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Fix the service-label check to use accumulated merged scopes and add a regression test for split-scope accumulation.

Risk before merge

• [P1] Merging as-is can leave auth list under-reporting services whose canonical scope set is accumulated across separate successful refreshes, even though the PR fixes the reported Gmail-shaped path.
• [P1] The PR changes persisted OAuth/keyring metadata during refresh, so maintainers should verify upgrade behavior for both fresh token metadata and existing stale token entries.

Maintainer options:

1. Fix service repair to use accumulated scopes (recommended)
   Change the service-label coverage check to evaluate the merged observed scope set and add a regression test that exercises Contacts or another split-scope service across multiple refreshes.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update the OAuth scope reconciliation PR so service-label backfill checks the accumulated merged stored scopes, not only the current refresh response, and add focused regression coverage for a split-scope service observed across multiple successful token refreshes.

Next step before merge

• [P2] There is one narrow code/test repair: evaluate service-label coverage against accumulated merged observed scopes and cover the split-scope case.

Security
Cleared: The diff is auth-sensitive but only persists scopes observed from OAuth refresh responses and adds no new secret exposure, dependency source, workflow, or code execution path.

Review findings

• [P2] Use accumulated scopes for service repair — internal/googleapi/client_auth.go:95

Review details

Best possible solution:

Land the reconciliation after the service-label repair checks the accumulated merged observed scope set and includes regression coverage for a split-scope service observed across multiple successful refreshes.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection. Current main does not reconcile observed refresh scopes into stored metadata, and the PR branch still has a clear split-scope service gap because Contacts requests one canonical scope at a time.

Is this the best way to solve the issue?

No, not quite. Reconciling observed scopes is the right repair direction, but the service-label check should use the accumulated merged observed scope set so split-scope services can self-heal after multiple successful calls.

Full review comments:

• [P2] Use accumulated scopes for service repair — internal/googleapi/client_auth.go:95
  This still checks only the scopes returned by the current refresh response before backfilling the service label. Split-scope services such as Contacts request one canonical scope at a time, so after several successful calls updated.Scopes can cover the service while grantedScopes from the latest response remains partial; auth list would keep omitting that usable service. Check the merged observed scope set instead and cover that accumulated case.
  Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8a9a90840823.

Label changes

Label changes:

• add P2: This is a normal-priority auth metadata fix with a concrete remaining correctness gap and limited blast radius.
• add merge-risk: 🚨 auth-provider: The PR changes OAuth scope/service metadata persistence, so an incorrect merge can keep auth capability reporting wrong for some provider-routing cases.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes copied live smoke output showing gmail labels list succeeded and post-refresh auth list reported Gmail service and 47 scopes.
• remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
• remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

• P2: This is a normal-priority auth metadata fix with a concrete remaining correctness gap and limited blast radius.
• merge-risk: 🚨 auth-provider: The PR changes OAuth scope/service metadata persistence, so an incorrect merge can keep auth capability reporting wrong for some provider-routing cases.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes copied live smoke output showing gmail labels list succeeded and post-refresh auth list reported Gmail service and 47 scopes.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live smoke output showing gmail labels list succeeded and post-refresh auth list reported Gmail service and 47 scopes.

Evidence reviewed

Acceptance criteria:

• [P1] go test ./internal/googleapi -run 'TestPersistingTokenSource|TestTokenSourceForAccountScopes'.
• [P1] go test ./internal/googleapi ./internal/googleauth ./internal/cmd.

What I checked:

• Current main token persistence lacks scope reconciliation: On current main, the persisting token source updates refresh/access token metadata, expiry, and ID-token identity, but it does not merge scopes observed from OAuth refresh responses into the stored token metadata. (internal/googleapi/client_auth.go:52, 8a9a90840823)
• PR branch checks only the latest observed grant for service repair: The PR merges observed scopes into updated.Scopes, but the service-label backfill condition still calls scopesContainAll(grantedScopes, canonicalScopes), so it only evaluates the current refresh response. (internal/googleapi/client_auth.go:95, d923e29525df)
• Contacts is a split-scope service on current main: Current main constructs Contacts clients with one contacts service label but separate contacts, other-contacts, and directory scopes, so the stored accumulated scope set can cover the service even when an individual refresh response is partial. (internal/googleapi/people.go:15, 8a9a90840823)
• PR regression coverage is focused but misses accumulated split-scope repair: The PR adds tests for full observed grant repair, partial-grant non-overreporting, and requested-but-unobserved scopes, but not for multiple partial successful refreshes that cumulatively satisfy a service. (internal/googleapi/client_more_test.go:293, d923e29525df)
• History provenance for current auth path: The current persistingTokenSource and split-scope Contacts implementation in this shallow checkout trace to the v0.19.0 release commit by Peter Steinberger. (internal/googleapi/client_auth.go:28, b25a3c029b37)
• Reported proof in PR body: The PR body reports a live clawdbot smoke where gmail labels list returned 16 labels and post-refresh auth list reported Gmail service and 47 scopes. (d923e29525df)

Likely related people:

• steipete: Git blame and log show Peter Steinberger authored the current persistingTokenSource, Contacts client construction, and the PR's OAuth scope reconciliation commits in the relevant auth path. (role: current auth implementation author and recent area contributor; confidence: high; commits: b25a3c029b37, fe3aa39ceb25, d923e29525df; files: internal/googleapi/client_auth.go, internal/googleapi/people.go, internal/googleauth/service.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe3aa39ceb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Use accumulated scopes for service repair

When a service is exercised through narrower clients, this check never backfills the service even after all of its scopes have been successfully observed. For example, the contacts clients request one scope at a time (NewPeopleContacts, NewPeopleOtherContacts, and NewPeopleDirectory in internal/googleapi/people.go:15-24), so each refresh response's grantedScopes is only a subset of the canonical contacts set; despite line 89 merging those observed scopes into updated.Scopes, this condition still tests only the current response and auth list can continue to omit contacts for a fully usable token. Consider testing the merged observed set instead of just the latest response.

Useful? React with 👍 / 👎.