feat(connections): show real provider logos across the connect flow - #3375
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
benjaminshafii
marked this pull request as ready for review
July 31, 2026 09:59
Member
Author
VerificationMounted the real Provider list
API-key step
Also run: typecheck, One note: the shared |
src-opn
approved these changes
Jul 31, 2026
Providers were identifiable only by a two-letter monogram unless they were one of the three brands with a bundled inline SVG, which made the connect dialog read as a wall of grey initials. Adds a logo resolution ladder (inline SVG, Simple Icons, favicon, monogram last) so a provider only falls back to initials once every source is exhausted. Long-tail catalog ids are slugified domains, so unslugging "302ai" to "302.ai" recovers a real mark instead of "30". Builds on that to group the dialog into Connected and All providers, lead the API-key step with the provider's own logo, and make the model picker's API-keys row reflect whether any keys exist yet. Co-authored-by: Cursor <cursoragent@cursor.com>
benjaminshafii
force-pushed
the
feat/connect-providers-logos
branch
from
July 31, 2026 10:36
f6c43b2 to
c196208
Compare
benjaminshafii
enabled auto-merge (squash)
July 31, 2026 10:36
Member
Author
|
@OmarMcAdam @src-opn re-requesting a look, please. After your approval the single commit on this branch was rewritten to be signed — the default-branch ruleset requires verified signatures and the original commit was unsigned. The new commit was created through GitHub's API so it carries a verified signature, and the PR then auto-merged. The rewrite was content-preserving: Flagging it explicitly so the re-review is a conscious sign-off rather than an inherited one. |
benjaminshafii
added a commit
that referenced
this pull request
Jul 31, 2026
The org connections surface renders empty for a signed-in org member on current dev — correct route, empty body text. org-connection-lifecycle (untouched by this branch) independently regressed the same way today after the sandbox pulled merged dev, which includes #3375 across the connect flow. Everything before that surface is verified. Opt-in via OPENWORK_EVAL_CONNECTOR_SPEC=1 so a product defect cannot masquerade as a nightly failure; remove the gate when the surface renders.
benjaminshafii
added a commit
that referenced
this pull request
Aug 1, 2026
…box-aware ports, profile pruning (#3388) * fix(evals): ask the host for its workspace root instead of assuming the driver's A spec passing process.cwd() as a workspace path is only correct when the driver and the app share a filesystem. Driving a sandbox from a laptop, the app was asked to open a directory that exists only on the laptop — observed as onboarding hanging 120s on 'Power your first task' with no error, which reads like a broken app rather than a bad path. Host.workspaceRoot states it (repoRoot locally, /workspace on Daytona) and DesktopHandle re-exposes it, so app-smoke asks the app's own host. * fix(evals): allocate Daytona surface ports that are free INSIDE the sandbox The local host finds a free port by binding one; the Daytona host cannot, because the port must be free on another machine. It allocated from a local counter instead, so the OpenCode sidecar — which picks its own port at boot — was observed holding 9825, the CDP primary. Electron's debugger never bound and the preview URL timed out after 180s with nothing naming the cause. Ask the sandbox which ports are listening and skip those. * fix(evals): app-smoke opens a fresh small folder, not the repo root Routing app-smoke through Host.workspaceRoot made it open the whole monorepo (/workspace, node_modules included). The engine scan then blocked the renderer past 240s and the spec failed reading location.hash — a regression versus the old process.cwd() (which happened to be the smaller /workspace/evals). workspaceRoot stays the right primitive for specs that genuinely need the repo (skills-local reads .opencode/skills); a workspace path just needs to be small and valid on the app's host. * fix(evals): prune stale surface profiles, not just their processes Disposal removes a profile on the happy path, but every killed or failed run leaks one at ~50MB. Twenty-seven filled a 10GB sandbox to 99%, and the symptom was not 'disk full' — it was the renderer failing to answer Runtime.evaluate for 240s, indistinguishable from a broken app. Cost four debug cycles to find. clearStaleSurfaces already pkills by rootDir and so already assumes exclusive ownership; deleting the directories it just orphaned is the matching half. * test(evals): two members each connect their own account to one org connector and call its tools Two SEPARATE desktops, own profiles, signed in as different members. A single desktop clearing localStorage only simulates per-member isolation; two make it real — the load-bearing assertion is that member B still needs their own sign-in after member A connected. The connector is the authority on use: mock MCP now records tool ARGUMENTS and a sha256 fingerprint of the caller's bearer token, so the spec asserts on calls actually served and on DISTINCT credentials rather than the app's own 'Connected' text. Placement: both desktops use the ambient host; moving either to its own sandbox is desktop({ host: daytonaSandbox(id) }) once mode B is green. * fix(evals): email verification is best effort; the sign-in that follows is the test ensureMemberSession threw when mark-verified failed, even though the member had just been created successfully and could sign in. On the eval sandbox the helper uses -uroot with no password while den's MariaDB root has one, so bootstrapping a second member aborted the whole spec at 2s. Warn and continue; if the subsequent sign-in fails, report both causes. * test(evals): wait for the connections surface to settle, and refresh the way the proven spec does * test(evals): name what was on screen when a connection card never renders * test(evals): steer back to settings when the app opens a new session mid-poll The card poll was watching the session surface: the app opens a freshly created session on its own and navigates away from settings. Re-navigate each iteration instead of assuming the route stays put. * test(evals): short, failure-tolerant probes while two desktops contend Two desktops in one sandbox make the renderer freeze in bursts; a bare 20s Runtime.evaluate turns one freeze into a failed spec. Probe briefly and retry, which is what the polling loop is for. * test(evals): gate the connector spec on the blank-connections defect The org connections surface renders empty for a signed-in org member on current dev — correct route, empty body text. org-connection-lifecycle (untouched by this branch) independently regressed the same way today after the sandbox pulled merged dev, which includes #3375 across the connect flow. Everything before that surface is verified. Opt-in via OPENWORK_EVAL_CONNECTOR_SPEC=1 so a product defect cannot masquerade as a nightly failure; remove the gate when the surface renders. * test(evals): match the canonical extensions route the app rewrites to The app canonicalises /workspace/<id>/settings/extensions/connections to /workspace/<id>/extensions/connections. Checking for the pre-rewrite form made the steer-back re-navigate every iteration, fighting the rewrite so the surface never settled — which presented as a blank page and which I wrongly attributed to a product regression. A probe found no render error and 31 buttons on the canonical route. * fix(evals): name the expression when a CDP evaluation times out The rejection fires from a timer, so the caller's stack is already gone: 'CDP call Runtime.evaluate timed out after 20000ms' cannot say which of a spec's dozens of evaluations blocked. Include a truncated expression. * fix(evals): retry navigation through renderer freeze bursts go() set the hash with a bare 20s evaluate. Under contention (two desktops on one host) a freeze burst killed the whole journey at its last phase. Setting a hash is idempotent, so retry it. * test(evals): frame the detail panel once it has painted, not once its text is in the DOM waitForText proves the DOM; a frame proves pixels. The detail panel paints slightly after its text lands, so the vision check intermittently caught a blank panel — it passed one run and failed the next. Wait for a laid-out node, scroll it into view, allow a paint. * test(evals): match rendered text when revealing, not raw textContent waitForText reads innerText, which is render-aware: a badge styled with CSS text-transform reads NEEDS YOUR SIGN-IN on screen while textContent holds 'Needs your sign-in'. The reveal could never agree with the wait. * fix(evals): give navigation a budget that survives a starved renderer * test(evals): settle canonical connection lifecycle frames * test(evals): avoid duplicate lifecycle proof frame * test(evals): prove two explicit Daytona desktop placements * test(evals): deduplicate disconnected lifecycle frame * fix(evals): make an externally-managed mock connector usable across runs publicUrl was a dead seam: waitForHealth read child?.exitCode on the null child and declared the (never-spawned) server exited, and the request log of a long-lived mock satisfied atLeast with a PREVIOUS run's tool calls, returning before this run's calls arrived. Guard the health check to only fail on a spawned child that exited, carry the mock's timestamp on each tool call, and let callers scope toolCalls with sinceIso. * test(evals): place each connector member's desktop on its own Daytona sandbox Two desktops plus two engines starve renderers on one 9GB sandbox — that ceiling, not any product defect, is what kept the tool-call phase red, so the header's retracted defect story goes too. OPENWORK_EVAL_DAYTONA_SANDBOX_A/_B now place each member via desktop({ host: daytonaSandbox(id) }), the mock publishes through OPENWORK_EVAL_CONNECTOR_MOCK_PUBLIC_URL so Den, both desktop browsers, and the driver can all reach it, and this run's tool calls are scoped with sinceIso. Green twice end to end (133.0s, 135.2s): both members OAuth-connect their own account, call mock_echo from the composer, and the connector witnesses two distinct bearer fingerprints. * fix(evals): parse the mock's bearer header in linear time CodeQL: js/polynomial-redos — ^Bearer\s+(.+)$ backtracks polynomially on 'Bearer' followed by header spam. One shared linear parse for the auth check and the fingerprint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Providers were identifiable only by a two-letter monogram unless they were one of the three brands with a bundled inline SVG, so the connect dialog read as a wall of grey initials.
provider-logo-src.ts): inline SVG → Simple Icons → favicon → monogram last.ProviderIconwalks the candidates and advances on eachimgerror, so a dead source degrades silently per row rather than breaking the list. Long-tail catalog ids are slugified domains, so302aiis unslugged to302.aito recover a real mark instead of "30".Test plan
Ran and passing:
npx tsc --noEmit— cleanbun test src/react-app/design-system/provider-icon.test.tsx— 9 passpnpm build— cleanI verified the logo sources live rather than trusting the spec: Simple Icons returns 200 for
anthropic,googlegemini,mistralai,deepseek,ollama,openrouterbut 404s foropenaiandabacus; the favicon step returns real images for302.ai,groq.com,abacus.ai. Those misses are encoded so they skip straight to the favicon step.Not yet verified visually. I have not driven this dialog in a running app — it needs a backend to populate the provider list, so I have not seen it render a real grid of logos end to end. Reviewer repro: open Settings → Connect, confirm providers show brand marks rather than initials, and that a provider with no resolvable logo still falls back cleanly.
Made with Cursor