Skip to content

Fix Google Photos Picker discovery#1203

Open
zrm625 wants to merge 2 commits into
RhysSullivan:mainfrom
zrm625:codex/fix-google-photos-picker-discovery
Open

Fix Google Photos Picker discovery#1203
zrm625 wants to merge 2 commits into
RhysSullivan:mainfrom
zrm625:codex/fix-google-photos-picker-discovery

Conversation

@zrm625

@zrm625 zrm625 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix Google APIs whose live Discovery documents only work at their service-hosted URL. This covers Google Photos Picker, Forms, and Keep. Also show required OAuth scopes in the generic OAuth app registration form.

Bug

I added Google Photos Picker support and the Picker bug in the same PR, so there was never a known-good upstream Picker path.

Picker's live Discovery URL is https://photospicker.googleapis.com/$discovery/rest?version=v1; the rewritten central URL returns 404. Its live document also omits OAuth scope metadata, so Picker could be added without the required https://www.googleapis.com/auth/photospicker.mediaitems.readonly scope.

The test fixture missed this because it was more complete than Google's live document. While validating the deployed fix, the default Google bundle exposed the same service-hosted-only Discovery issue for Forms and Keep.

Introduced in: #1137
Related normalizer change: #1106

Fix

  • Keep Picker, Forms, and Keep service-hosted Discovery URLs service-hosted.
  • Keep Picker's documented OAuth scope fallback in the same override map.
  • Leave normal central Discovery behavior in place for other Google APIs.
  • Show required OAuth scopes during manual OAuth app registration.

The UI change is disclosure-only. It does not request new permissions.

Screenshot

Headless screenshot of Google Photos OAuth app scopes

Validation

  • bun run --cwd packages/plugins/google test -- src/sdk/discovery.test.ts src/sdk/plugin.test.ts
  • bun run --cwd packages/react test -- src/components/oauth-client-form.test.ts
  • Live deploy smoke on executor v1.5.23: / returns 200 and /mcp returns unauthenticated 401.
  • Live default Google bundle replay returned 200 with 264 tools.

@zrm625 zrm625 marked this pull request as ready for review June 29, 2026 02:30
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes Google Photos Picker, Forms, and Keep discovery by routing their URLs to service-hosted endpoints instead of the central www.googleapis.com/discovery/v1/apis gateway, which returns 404 for these services. It also injects a scope fallback for Picker whose live Discovery document omits auth metadata, and surfaces required OAuth scopes in the manual registration form.

  • discovery.ts: Adds a GOOGLE_DISCOVERY_SERVICE_OVERRIDES map with per-service URL routing and scope injection logic.
  • plugin.test.ts: Updates the Picker fixture to match the real live document and fixes the stub HTTP client to match full URLs.
  • oauth-client-form.tsx: Adds a read-only scopes disclosure panel that mirrors what will be sent at registration.

Confidence Score: 5/5

Safe to merge. The change is additive: a narrow override map for three known-broken services, with all other services unaffected.

All changed paths are covered by targeted unit tests. The URL normalization function is well-exercised in both directions for each overridden service. The scope injection logic is additive and only activates when a document omits metadata. The React change is display-only.

No files require special attention.

Important Files Changed

Filename Overview
packages/plugins/google/src/sdk/discovery.ts Core fix: adds per-service override map and helper functions; logic is sound, additive, and does not affect non-override services.
packages/plugins/google/src/sdk/discovery.test.ts Adds URL normalization round-trip tests for Picker/Forms/Keep and a scope injection test for when Picker's live document omits auth metadata.
packages/plugins/google/src/sdk/plugin.test.ts Updates Picker URL and fixture to match real live document; stub client upgraded to prefer full-URL match for query-string-versioned service-hosted endpoints.
packages/react/src/components/oauth-client-form.tsx Disclosure-only UI addition showing required OAuth scopes; correctly reuses registrationScopes and only renders when scopes are present.

Reviews (3): Last reviewed commit: "Handle central Google discovery override..." | Re-trigger Greptile

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e91270abe9

ℹ️ 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".

Comment on lines +309 to +311
return override?.preserveServiceHostedUrl === true
? `https://${host}/$discovery/rest?version=${version}`
: `${DISCOVERY_SERVICE_HOST}/${service}/${version}/rest`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route central overrides through service hosts

When a user or existing stored config supplies https://www.googleapis.com/discovery/v1/apis/{forms|keep|photospicker}/v1/rest, the earlier www.googleapis.com branch returns the same central URL before this override runs, so fetchGoogleDiscoveryDocument still fetches the 404 endpoint this fix is trying to avoid. Please apply the override in the central branch too, or reject those central URLs, so refresh/manual add paths using normalized central URLs do not keep failing.

Useful? React with 👍 / 👎.

@zrm625 zrm625 force-pushed the codex/fix-google-photos-picker-discovery branch from d2a0e7d to 3ff0a97 Compare June 29, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant