Skip to content

perf(uploader): reuse edit-mode credentials and start minting on menu open - #636

Closed
alukach wants to merge 2 commits into
mainfrom
perf/edit-mode-credentials
Closed

alukach wants to merge 2 commits into
mainfrom
perf/edit-mode-credentials

Conversation

@alukach

@alukach alukach commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Why

Turning on Edit Mode is slow. Every enable ran getTemporaryCredentials, which on a cookie miss drives a server-side Ory Hydra authorization-code flow (4–6 sequential HTTP calls through the admin API) and then a data-proxy /.sts AssumeRoleWithWebIdentity. Four things made it worse than it needs to be. The client waited a deliberate 100ms before calling the action. Switching to "Read Only" threw away credentials that were still valid for up to an hour, so the next enable minted again. The upload SDK's credential provider called the action directly, so the first upload after entering edit mode minted once more. And the action resolved the Ory session twice: once in getPageSession, again inside readProxyCredentials.

What changes

  • CredentialsProvider keeps minted credentials in a ref-held cache per scope and reuses them while they have at least 15 minutes left. "Read Only" still takes the scope out of edit mode but keeps the credentials, so re-enabling within the hour is instant. In-flight mints are shared, so concurrent callers mint once. clearAllCredentials empties the cache too. The 100ms sleep is gone.
  • Every reader goes through that cache. The new loadCredentials backs the upload SDK's credential provider in UploadProvider, and its 15-minute floor sits above the SDK's own 5-minute refresh window, so long uploads still rotate before expiry. "View Credentials" also goes through it, so it re-mints rather than showing keys about to expire.
  • prefetchCredentials warms the cache without turning edit mode on. FetchCredentialsButton calls it from the menu's onOpenChange, which covers pointer and keyboard opens and nothing else, so the mint runs while the user reads the menu. A failed prefetch is silent: a click that joins the failing mint reports the failure, and the next click mints again.
  • getTemporaryCredentials hands out the sc_proxy_creds cookie's credentials only if they have at least 15 minutes left, matching the browser's floor, so a re-mint never returns the same short-lived keys.
  • readProxyCredentials(sessionIdentityId?) takes the identity of an already-verified session and skips its own whoami. getTemporaryCredentials and deleteProduct pass session.identity_id; render-path callers are unchanged.

Known tradeoffs

  • The permission check is skipped on a cache hit. Re-enabling edit mode from the cache doesn't run getTemporaryCredentials' WriteRepositoryData check. If write access is revoked mid-session, the UI can show edit mode until the credentials near expiry. The data proxy authorizes every request itself (ADR-005), so the writes are refused all the same.
  • The cache is per product, not per user. The credentials are per user, so a second product still mints. The per-product key keeps the server's write check in front of each product's first use.

Considered and rejected

A review across performance, security and OAuth practice ruled out:

  • Minting an ID token at login. A cached ID token can be exchanged at /.sts repeatedly, making it a credential that mints credentials, and it would usually expire before the user opened Edit Mode anyway.
  • Writing the sc_proxy_creds cookie on mint in getTemporaryCredentials. In Next 15.5, cookies().set in an action marks the path revalidated, so every mint, including the upload SDK's hourly refresh, would re-render the product page server-side.
  • Pre-warming on mount. FetchCredentialsButton sits in the [[...path]] layout and remounts on every folder navigation. That would cost a whoami plus DB reads per click, and a full mint for writers who never use Edit Mode.
  • A longer STS DurationSeconds. Issued credentials can't be revoked short of rotating the proxy's signing key, and logout only clears the cookie.

The larger win, replacing the Hydra admin-API flow with an app-signed short-lived JWT the proxy trusts, moves an ADR-004 decision and is proposed separately in data.source.coop#249 (ADR-015).

Docs and ADRs

Checked the data-proxy ADRs: 004 (STS) and 005 (authorization) still describe the model. Credentials are still minted the same way and still cover the user, and this PR only changes how long the browser keeps them. Checked docs.source.coop docs/using-source/; the upload flow a user walks through is unchanged.

Storybook

FetchCredentialsButton has no story and renders in none, and nothing changes visually. The only UI difference is when the mint starts.

Testing

As of 8427eb0:

  • npm run type-check passes.
  • npm run lint shows no new warnings in the touched files.
  • jest --forceExit on src/lib/actions/credentials.test.ts, src/lib/services/proxy-credentials-read.test.ts, src/lib/actions/products.test.ts, src/lib/services/upload-queue-manager.test.ts and src/components/features/uploader: 51 tests pass. New cases cover:
    • one mint shared by a prefetch, the click and a later re-enable
    • a re-mint near expiry
    • a retry after a failed prefetch
    • the upload SDK path served from the cache, and clearAllCredentials forgetting it
    • the server re-minting rather than handing out a cookie with under 15 minutes left
  • Not exercised against a running app; the real Hydra/STS latency saved is unmeasured.

🤖 Generated with Claude Code

… open

Entering edit mode ran the full Ory Hydra + data-proxy STS mint on every enable, after a 100ms artificial delay, and switching to "Read Only" discarded credentials that were still valid for up to an hour. The provider now keeps minted credentials per scope until they have under 15 minutes left, shares an in-flight mint between callers, and pressing the edit-mode menu trigger starts the mint so it is usually finished by the time "Edit Mode" is chosen. readProxyCredentials accepts the identity of an already-resolved session, which spares getTemporaryCredentials and deleteProduct a second Ory whoami.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
source-coop-ui Ready Ready Preview Oct 5, 2026 1:45am UTC
source-cooperative Ready Ready Preview Oct 5, 2026 1:45am UTC

Request Review

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 20s —— View job


✅ No blocking issues — safe to merge.

The cache logic holds up. Concurrent mints are shared through inflightRef, a failed mint is cleared in .finally so the next click retries, and readProxyCredentials(session.identity_id) takes the identity only from a verified session. The 15-minute floor is applied the same way in the client cache and in the server cookie reuse, so a re-mint won't hand back the same short-lived credentials. I didn't run the tests or the app.

Non-blocking notes

  • CredentialsProvider.tsx:99: clearAllCredentials now empties the cache, but nothing calls it (grep finds only the definition and the context value). The cache is keyed by accountId:productId and not by user. If the provider stays mounted across a logout and a different user then logs in through client-side navigation, that user would get the previous user's credentials from the cache for up to an hour. A full reload on logout would avoid this. If logout is a client-side navigation, call clearAllCredentials() there, or key the cache by identity.
  • CredentialsProvider.tsx:136: an in-flight mint that resolves after clearAllCredentials() writes its result back into the cache. This is the same logout race and is narrow. If you fix the first point, it is worth guarding.
  • CredentialsProvider.tsx:104-110: the comment says a revoked permission keeps edit mode on until the credentials near expiry, and that the proxy refuses the writes anyway. That is an accepted trade-off, and the comment states it clearly.

Simplify (ponytail)

  • CredentialsProvider.tsx:15 and credentials.ts:11: the same 15-minute constant is defined twice, in a client component and in a server action. Two copies are fine given the comments that tie them together. Export one from a shared module only if they start to drift.
  • CredentialsProvider.tsx:61: prefetchCredentials and loadCredentials are both exposed on the context. prefetchCredentials is a one-line wrapper that FetchCredentialsButton could replace with loadCredentials(scope).catch(() => {}). This is minor.

Docs

The description names ADR 004 and ADR 005 and docs/using-source/, and says why each still holds. The Docs requirement is met.


💰 Estimated review cost: $0.15 · 0m19s · 5 turns

The upload SDK's credential provider called getTemporaryCredentials directly, so the first upload after entering edit mode minted again; it now uses the provider's cache, which re-mints below 15 minutes left. View Credentials goes through the cache too, so it never shows keys from a cache hit that have since expired. getTemporaryCredentials hands out the cookie's credentials only with at least 15 minutes left, matching the browser's floor, so a re-mint can't return the same short-lived keys. The prefetch moves from onPointerDown to the menu's onOpenChange, which covers keyboard opens and ignores pointer-downs that open nothing, and clearAllCredentials now empties the cache as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alukach

alukach commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Not sufficiently performant

@alukach alukach closed this Oct 9, 2026
@alukach
alukach deleted the perf/edit-mode-credentials branch October 9, 2026 05:50

This branch was successfully deployed

2 active deployments
Preview – source-cooperative — 8427eb06 Deployed Oct 5, 2026 by vercel[bot]
Preview – source-coop-ui — 8427eb06 Deployed Oct 5, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant