Remove Agent-specific proxy routes - #3946
Conversation
frandiox
left a comment
frandiox
left a comment
There was a problem hiding this comment.
The pending PRs are already merged. I think we are missing a release in Storefront Components.
| it.each(["handoff", "buyer-claims"])( | ||
| "proxies agent %s through the Shopify API proxy", | ||
| async (route) => { | ||
| const result = await handleShopifyRoutes({ | ||
| request: new Request(`https://my-app.com/__shopify/agent/${route}`), | ||
| }); | ||
|
|
||
| expect(result).toBeInstanceOf(Response); | ||
| const call = mockFetch.mock.calls[0]; | ||
| assert(call, "expected fetch to be called"); | ||
| expect(call[0].href).toBe(`https://test-store.myshopify.com/agent/${route}`); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
This test can be now made generic, not specific to the agent. It should just forward anything (except /cdn/). Perhaps just revert the change to the previous version?
There was a problem hiding this comment.
Restored the generic Shopify API proxy test in b2f3148a. Agent routes use the same /__shopify contract as every other proxied path.
| it.each(["handoff", "buyer-claims"])( | ||
| "leaves direct agent %s routes to the app router", | ||
| (route) => { | ||
| const result = handleShopifyRoutes({ | ||
| request: new Request(`https://my-app.com/agent/${route}`), | ||
| }); | ||
|
|
||
| expect(result).toBeNull(); | ||
| expect(mockFetch).not.toHaveBeenCalled(); | ||
| }); | ||
| expect(result).toBeNull(); | ||
| expect(mockFetch).not.toHaveBeenCalled(); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Same with this one, it should do be reverted to "returns null synchronously for non-matching URLs" or similar.
There was a problem hiding this comment.
Restored the generic synchronous non-match test in b2f3148a. Direct Agent routes now use the shared app-router contract.
a4dd396 to
1e965e1
Compare
1e965e1 to
b2f3148
Compare
|
@frandiox yes, Storefront Components #1656 has merged but still needs a production release. I'll keep #3946 blocked until that release is live and cached direct-route bundles have expired. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
b2f3148 to
a2ac80d
Compare
3 participants
Depends on Storefront Components #1656.
Why are these changes introduced?
Hydrogen preview added a dedicated proxy for direct Storefront Agent routes before Storefront Agent could use the generic Shopify API proxy. Storefront Components now builds buyer-claims requests with
apiProxyPrefix, so keeping both paths would duplicate routing and header behavior.What approach did you take?
Use the generic
/__shopify/*proxy as the only Storefront Agent proxy on Hydrogen preview. The browser-facing proxy also removes infrastructure control headers before forwarding requests to Shopify.Decision logs
Why is removal safe now? Storefront Components #1656 is in production. The served Agent bundle constructs the buyer-claims URL from
apiProxyPrefix, and its ten-minute CDN cache window has elapsed. Stable Hydrogen releases never proxied these direct Agent paths because this work targetspreview.What happens to older Agent bundles? Unprefixed
/agent/buyer-claimsand/agent/handoffrequests fall through to app routing, which may return a 404 or catch-all HTML response. Already-running tabs can retain an older bundle until reload, so this PR remains gated on the Phase 3 bake.How is this reversed? There is no runtime flag. Reversal requires restoring the previous Hydrogen package behavior, publishing it, and merchants upgrading and redeploying. The preview-only release limits exposure but does not provide an instant rollback.
Why remove infrastructure control headers? The generic proxy accepts browser requests. It must not forward browser-supplied values that Shopify infrastructure uses for staff-only or staging behavior.
Testing steps/scenarios
apiProxyPrefix