Conversation
…-profiles feat(browserbase): add per-site auth profiles
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 53 files
Confidence score: 2/5
apps/api/src/browserbase/browserbase.service.tsinnavigateToUrlcloses Stagehand/session on successful navigation, which can terminate Live View immediately after the API call and break expected user interaction flows — keep the session open on success and only close on explicit teardown/error paths before merging.apps/app/src/app/(app)/[orgId]/settings/browser-connection/components/BrowserConnectionClient.tsxcan let a mount-time profile fetch overwrite an already-establishedprofileId, so auth verification may run against the wrong profile — preserve the session-establishedprofileIdonce set and ignore later stale overwrites.apps/api/src/browserbase/browser-automation-execution.service.tsandapps/api/src/browserbase/browser-automation-crud.service.tstogether create tenant-boundary and response-surface risk: lookups are not org-scoped at query time and run responses include joined internal objects — add organization filters in Prismawhereand sanitize output to DTO-only fields before merge.apps/api/src/browserbase/browserbase-session.service.tsandapps/api/src/browserbase/browser-run-coordinator.tsexpose stability/operability issues by returning raw internal exception messages to clients and retaininglastDomainRunAtentries without eviction, which can leak internals and grow memory over time — map errors to safe typed exceptions and add cleanup/TTL eviction for inactive domains.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/browserbase/browser-automation-execution.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-automation-execution.service.ts:254">
P2: Automation lookup is not tenant-scoped at query time. Filter by organization in Prisma `where` to enforce multi-tenant isolation in the DB layer.</violation>
</file>
<file name="apps/api/src/browserbase/browser-automation-run-result.ts">
<violation number="1" location="apps/api/src/browserbase/browser-automation-run-result.ts:11">
P2: `statusForBrowserFailureCode` duplicates the identical mapping logic already present in the service's private `blockedStatusForCode`. Two independent implementations of the same failure-code-to-status mapping will diverge over time.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/settings/browser-connection/components/BrowserConnectionClient.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/settings/browser-connection/components/BrowserConnectionClient.tsx:72">
P1: Mount-time profile fetch can overwrite the active session profile ID, causing auth verification to target the wrong profile. Keep existing `profileId` once set by session start.</violation>
</file>
<file name="apps/api/src/browserbase/browser-run-coordinator.ts">
<violation number="1" location="apps/api/src/browserbase/browser-run-coordinator.ts:89">
P2: `lastDomainRunAt` is written but never cleaned up, causing unbounded in-memory growth by hostname. Add eviction/cleanup for inactive domains.</violation>
</file>
<file name="apps/api/src/browserbase/browser-automation-crud.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-automation-crud.service.ts:114">
P2: Run lookup returns joined automation/task objects directly, leaking internal fields beyond the run DTO. Strip internal relation data before returning API responses.</violation>
</file>
<file name="apps/api/src/browserbase/browser-auth-profile-context.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-auth-profile-context.service.ts:77">
P2: Timeout handling throws a generic `Error` instead of a Nest timeout exception. Clients get inconsistent status/error semantics for a retryable timeout path.</violation>
</file>
<file name="apps/api/src/browserbase/browserbase.service.ts">
<violation number="1" location="apps/api/src/browserbase/browserbase.service.ts:87">
P1: `navigateToUrl` now closes Stagehand/session on success, ending the live browser session right after navigation. This breaks flows where users must continue interacting in Live View after the API call.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| setProfiles(res.data); | ||
| const verifiedProfile = res.data.find((profile) => profile.status === 'verified'); | ||
| const firstProfile = verifiedProfile ?? res.data[0]; | ||
| setProfileId(firstProfile?.id ?? null); |
Contributor
There was a problem hiding this comment.
P1: Mount-time profile fetch can overwrite the active session profile ID, causing auth verification to target the wrong profile. Keep existing profileId once set by session start.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/settings/browser-connection/components/BrowserConnectionClient.tsx, line 72:
<comment>Mount-time profile fetch can overwrite the active session profile ID, causing auth verification to target the wrong profile. Keep existing `profileId` once set by session start.</comment>
<file context>
@@ -35,23 +53,23 @@ export function BrowserConnectionClient({ organizationId }: BrowserConnectionCli
+ setProfiles(res.data);
+ const verifiedProfile = res.data.find((profile) => profile.status === 'verified');
+ const firstProfile = verifiedProfile ?? res.data[0];
+ setProfileId(firstProfile?.id ?? null);
}
} catch {
</file context>
Suggested change
| setProfileId(firstProfile?.id ?? null); | |
| setProfileId((current) => current ?? firstProfile?.id ?? null); |
| } | ||
| // Don't close - user needs to interact via Live View | ||
| async navigateToUrl(sessionId: string, url: string) { | ||
| return this.sessions.navigateToUrl(sessionId, url); |
Contributor
There was a problem hiding this comment.
P1: navigateToUrl now closes Stagehand/session on success, ending the live browser session right after navigation. This breaks flows where users must continue interacting in Live View after the API call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browserbase.service.ts, line 87:
<comment>`navigateToUrl` now closes Stagehand/session on success, ending the live browser session right after navigation. This breaks flows where users must continue interacting in Live View after the API call.</comment>
<file context>
@@ -1,403 +1,117 @@
- }
- // Don't close - user needs to interact via Live View
+ async navigateToUrl(sessionId: string, url: string) {
+ return this.sessions.navigateToUrl(sessionId, url);
}
</file context>
| automationId: string; | ||
| organizationId: string; | ||
| }) { | ||
| const automation = await db.browserAutomation.findUnique({ |
Contributor
There was a problem hiding this comment.
P2: Automation lookup is not tenant-scoped at query time. Filter by organization in Prisma where to enforce multi-tenant isolation in the DB layer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-automation-execution.service.ts, line 254:
<comment>Automation lookup is not tenant-scoped at query time. Filter by organization in Prisma `where` to enforce multi-tenant isolation in the DB layer.</comment>
<file context>
@@ -0,0 +1,276 @@
+ automationId: string;
+ organizationId: string;
+ }) {
+ const automation = await db.browserAutomation.findUnique({
+ where: { id: input.automationId },
+ include: {
</file context>
| await delay(waitMs); | ||
| } | ||
|
|
||
| this.lastDomainRunAt.set(hostname, Date.now()); |
Contributor
There was a problem hiding this comment.
P2: lastDomainRunAt is written but never cleaned up, causing unbounded in-memory growth by hostname. Add eviction/cleanup for inactive domains.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-run-coordinator.ts, line 89:
<comment>`lastDomainRunAt` is written but never cleaned up, causing unbounded in-memory growth by hostname. Add eviction/cleanup for inactive domains.</comment>
<file context>
@@ -0,0 +1,93 @@
+ await delay(waitMs);
+ }
+
+ this.lastDomainRunAt.set(hostname, Date.now());
+ }
+}
</file context>
| async getRunWithPresignedUrl(runId: string, organizationId?: string) { | ||
| const run = await db.browserAutomationRun.findUnique({ | ||
| where: { id: runId }, | ||
| include: { automation: { include: { task: true } } }, |
Contributor
There was a problem hiding this comment.
P2: Run lookup returns joined automation/task objects directly, leaking internal fields beyond the run DTO. Strip internal relation data before returning API responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-automation-crud.service.ts, line 114:
<comment>Run lookup returns joined automation/task objects directly, leaking internal fields beyond the run DTO. Strip internal relation data before returning API responses.</comment>
<file context>
@@ -0,0 +1,203 @@
+ async getRunWithPresignedUrl(runId: string, organizationId?: string) {
+ const run = await db.browserAutomationRun.findUnique({
+ where: { id: runId },
+ include: { automation: { include: { task: true } } },
+ });
+ if (!run) return null;
</file context>
| this.logger.warn( | ||
| `Timed out waiting for Browser auth profile context ${profileId}`, | ||
| ); | ||
| throw new Error( |
Contributor
There was a problem hiding this comment.
P2: Timeout handling throws a generic Error instead of a Nest timeout exception. Clients get inconsistent status/error semantics for a retryable timeout path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-auth-profile-context.service.ts, line 77:
<comment>Timeout handling throws a generic `Error` instead of a Nest timeout exception. Clients get inconsistent status/error semantics for a retryable timeout path.</comment>
<file context>
@@ -0,0 +1,94 @@
+ this.logger.warn(
+ `Timed out waiting for Browser auth profile context ${profileId}`,
+ );
+ throw new Error(
+ 'Browser profile initialization is taking too long. Please retry.',
+ );
</file context>
| export function statusForBrowserFailureCode( | ||
| code: BrowserAutomationFailureCode | undefined, | ||
| ): 'failed' | 'blocked' { | ||
| if (code === 'captcha_blocked' || code === 'needs_user_action') { |
Contributor
There was a problem hiding this comment.
P2: statusForBrowserFailureCode duplicates the identical mapping logic already present in the service's private blockedStatusForCode. Two independent implementations of the same failure-code-to-status mapping will diverge over time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-automation-run-result.ts, line 11:
<comment>`statusForBrowserFailureCode` duplicates the identical mapping logic already present in the service's private `blockedStatusForCode`. Two independent implementations of the same failure-code-to-status mapping will diverge over time.</comment>
<file context>
@@ -0,0 +1,39 @@
+export function statusForBrowserFailureCode(
+ code: BrowserAutomationFailureCode | undefined,
+): 'failed' | 'blocked' {
+ if (code === 'captcha_blocked' || code === 'needs_user_action') {
+ return 'blocked';
+ }
</file context>
…-profiles [dev] [tofikwest] tofik/browser-automation-auth-profiles
… (FRAME-20 review)
Addresses the remaining cubic review points:
- Move is now scoped to the section it's opened from ("frameworks in the current
family", per the ticket): each family section and the Ungrouped section get a
Move action that opens the dialog with only that group's frameworks. Removes
the global all-frameworks Move button.
- UpdateFrameworkFamilyDto now rejects null (via @ValidateIf) instead of letting
it skip validation, so a null can't reach a non-nullable column. Service keeps
the `!= null` guard as a backstop. Adds a DTO validation spec.
Left as-is: the controller keeps PlatformAdminGuard — every framework-editor
controller uses it; this is a platform-admin internal tool, not customer RBAC, so
permission-based gating would make it inconsistent with its six siblings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGwXoPb6qHxuHy8miVmyxT
…lback resource identity ## Problem Customers cannot mark GCP public bucket findings as exceptions in Comp AI. The system rejects the action with "This finding cannot be marked as an exception it lacks a stable check/resource identity". This blocks workflow for accepting reviewed and acknowledged risks on SCC findings. ## Root cause The SCC emitter in gcp-security.service.ts only anchors resourceId to the finding's resourceName field. For GCP public bucket findings from SCC, this field is often empty or missing. The code never falls back to the result.resource.name that SCC provides, leaving us with an empty resourceId string. The exception guard at exception.service.ts:223 then rejects any attempt to create an exception for findings with blank resourceId. ## Fix Updated the resourceId assignment to use a cascade fallback: prefer resourceName, then fall back to result.resource.name, then f.name as a final anchor. This ensures every finding gets a stable non-empty identity without changing the behavior of findings that already have a proper resourceName. The change is one line and completely backward compatible. ## Explicitly NOT touched - Authentication or authorization logic - Database schema or migrations - Billing or metering - Secret handling - Any other SCC emitter logic ## Verification ✅ Findings with existing resourceName continue to use it unchanged ✅ GCP public bucket findings now populate resourceId from result.resource.name ✅ Exception creation succeeds for previously-blocked findings ✅ No schema or auth layer modifications needed
…oves (FRAME-20 review)
cubic review: the empty-check and delete were separate steps, so a concurrent
move could add a framework between them. Replace with a single conditional
`deleteMany({ where: { id, frameworks: { none: {} } } })` — the DB evaluates the
"no frameworks" predicate at delete time, so a race makes it a no-op (count 0 →
friendly BadRequest) rather than deleting a non-empty family. The RESTRICT FK
remains a second backstop. Specs updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGwXoPb6qHxuHy8miVmyxT
Address review findings.
fix(gcp-scc): allow exceptions on public bucket findings by using fallback resource identity
…lies feat(framework-editor): add framework families (FRAME-20)
Connecting a Browser Automation profile failed with "Invalid response body while trying to fetch https://api.browserbase.com/v1/sessions/<id>: Premature close". The failing GET /v1/sessions/<id> runs inside stagehand.init(): when resuming a session, Stagehand calls bb.sessions.retrieve on its own internal Browserbase client, which lives outside getBrowserbase() and so is covered by neither our withBrowserbaseRetry wrapper nor the accept-encoding: identity mitigation. The error is an undici body-stream error thrown during JSON parsing after a 200 response, so the SDK's own maxRetries does not cover it either. Wrap stagehand.init() in withBrowserbaseRetry, closing any half-initialized instance between attempts (keepAlive keeps the session alive so the retry resumes the same id). This covers all three createStagehand call sites (navigate, login check, evidence runs) and converts the raw error into a clean 503 once retries are exhausted. Extract the dynamic Stagehand import into a loadStagehand() seam so the retry can be unit-tested (jest cannot intercept native dynamic import under module: nodenext). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zSwXMqVNvWLJBZEot9x12
…nit-retrya fix(browserbase): retry stagehand init to survive premature close
…ild after FRAME-20) FRAME-20 added a non-null `familyId` to FrameworkEditorFramework. The API's update-policy trigger task hand-writes a `frameworks` zod schema that is consumed as `UpdatePolicyParams.frameworks: FrameworkEditorFramework[]`, so the new required field broke `nest build` (the Docker/ECS API image), leaving main un-deployable. Add `familyId` to that schema — mirrors the same fix already made in apps/app's copy of this task during FRAME-20. Verified: `bunx nest build` no longer reports the familyId error; no `familyId`-missing errors remain anywhere in the api. (The local-only auth.server.ts better-auth dup-package error does not occur in the clean Docker build — its log got past auth.server.ts and failed only here.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGwXoPb6qHxuHy8miVmyxT
…y-id fix(api): unblock API build — add familyId to update-policy frameworks schema
Contributor
|
🎉 This PR is included in version 3.87.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Ships per-site browser auth profiles with a sturdier automation stack, better framework ordering (FRAME-18), framework families (FRAME-20), and UI polish. Also hardens integration checks by retrying transient network errors, fixes missing names by falling back to email, enables email edits and reactivation for members, and adds a stable resource ID for GCP public bucket findings so exceptions work.
New Features
@trycompai/design-system; ISMS wizard resumes on the first unsaved step so progress matches restored answers; evidence/document wizards ask for confirmation before discarding on Cancel; Trust Center access request emails now link directly to the access-requests review page; shows user email when name is empty in task lists and assignee displays; admins can update employee email; reactivating a member clears the deactivated flag.result.resource.name(falling back to the finding id) whenresourceNameis empty, so exceptions can be created.sortOrderdrives ordering across the editor, manifests, export/import, and app views (sorted by sortOrder asc, nulls last, then identifier); ensures deterministic framework selection ordering to prevent wrong default selection.familyIdto the update-policy frameworks schema to unblock API build.Migration
BrowserAuthProfile, failure enums, runblockedstatus; addsFrameworkEditorRequirement.sortOrder; addsFrameworkEditorFrameworkFamilyandFrameworkEditorFramework.familyId).BROWSERBASE_API_KEY,BROWSERBASE_PROJECT_ID,APP_AWS_ACCESS_KEY_ID,APP_AWS_SECRET_ACCESS_KEY,APP_AWS_REGION,APP_AWS_BUCKET_NAME. Optional:BROWSER_AUTOMATION_GLOBAL_CONCURRENCY,BROWSER_AUTOMATION_DOMAIN_THROTTLE_MS.Written for commit ac9a041. Summary will update on new commits.