Add server-driven Connections catalog#513
Conversation
Keep unreleased connection metadata in a server-only, team-targeted LaunchDarkly payload while preserving declared entries and existing client flag defaults.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Adds a server-only Reviewed by Cursor Bugbot for commit 7adb098. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7adb098. Configure here.
| slug: teamSlug, | ||
| }, | ||
| } | ||
| featureFlagContext |
There was a problem hiding this comment.
Connections page omits team slug
Medium Severity
The Connections page builds featureFlagContext with only team.id, while the dashboard layout evaluates client flags (including connectionsEnabled for the sidebar) with team.id and team.slug. LaunchDarkly evaluations for the same flag can disagree when targeting uses the team slug attribute, so the sidebar can show Connections while the page returns notFound(), or the reverse.
Reviewed by Cursor Bugbot for commit 7adb098. Configure here.
There was a problem hiding this comment.
i don't care about the slug
There was a problem hiding this comment.
everything's just evaluated on the id
There was a problem hiding this comment.
imma go take the slug out, i guess sidebar didn't need it. unless it did i don't remember was the far off times of... yesterday
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No bugs found, but this PR introduces a new payload-flag mechanism for feature flags (used to drive a new Connections page) alongside a refactor splitting boolean vs. payload flag definitions, so I think it's worth a human look before merging.
Extended reasoning...
Overview
This PR builds out the Connections page: it adds a real UI (replacing a stub that returned null), a DECLARED_CONNECTIONS/mergeConnectionCatalog catalog in src/features/dashboard/connections/catalog.ts, and a new generic payload feature-flag kind (developmentConnections) that lets ops inject additional connection entries via LaunchDarkly. It also refactors definitions.ts by extracting all boolean flags into a new boolean-definitions.ts module and adds a getPayload method to the feature-flag service, plus filters evaluateAll to exclude server-only flags from client exposure.
Security risks
No significant issues found. The new developmentConnections payload flag is exposure: 'server' only, so it's never sent to the client bundle. Its template field is constrained by a strict regex (^[a-zA-Z0-9][a-zA-Z0-9_.:/-]{0,127}$) and the payload is validated with zod, with a safe fallback to the default (empty array) on parse failure. The template value is only used inside URLSearchParams (auto-encoded) and as a React key/text node (auto-escaped), so no injection risk there.
Level of scrutiny
This is a moderate-risk change: it's not just a config tweak — it restructures the feature-flag definitions module (a shared core dependency used across the app) and introduces a new flag kind (payload) with its own validation/fallback path. That's an architectural decision (how future payload-based flags are declared and consumed) that a human should sanity-check, even though the bug hunter found nothing and test coverage is solid (catalog merge/dedup, payload validation fallback, client-vs-server exposure filtering, and page rendering are all covered).
Other factors
The diff is well-tested and self-contained, and the terminal page's handling of the template query param is pre-existing behavior, not something this PR changes. I don't see outstanding unaddressed reviewer comments in the available PR data.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7adb09881c
ℹ️ 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".
| team: { | ||
| id: teamId.data, | ||
| }, |
There was a problem hiding this comment.
When either LaunchDarkly flag is targeted by team.slug, this context now omits that attribute, so createOpenFeatureEvaluationContext sends a team context with no slug and the matching rollout returns its default instead. This regresses the previous Connections-page behavior (and differs from the Agents/BYOC pages), causing slug-targeted connectionsEnabled rollouts to 404 or slug-targeted developmentConnections payloads to be empty; include slug: teamSlug alongside the resolved ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
pulling out the slugs in another PR
2 participants
Uh oh!
There was an error while loading. Please reload this page.