[comp] Production Deploy#3205
Open
github-actions[bot] wants to merge 23 commits into
Open
Conversation
…-profiles feat(browserbase): add per-site auth profiles
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![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
…-profiles fix(browserbase): request identity encoded api responses
…down ## Problem Users can only filter by SOC2 and ISO27001 in the Overview > Findings view, even when their org has other frameworks enabled like ISO42001 or HIPAA. This blocks them from seeing findings specific to those frameworks. ## Root cause FindingsTab.tsx hardcodes the framework filter options to [all, soc2, iso27001] instead of deriving them from the org's actual enabled frameworks. The CreateFindingSheet on the same page does this correctly via the /v1/frameworks endpoint and extractOrgFrameworkTypes(), but the filter dropdown never got that fix. ## Fix Apply the same pattern from CreateFindingSheet to FindingsTab: fetch enabled frameworks at component load and use those to populate the filter dropdown. The client-side filter logic and FindingType enum already support all 7 framework types, so this is just surfacing what's already wired. ## Explicitly NOT touched Finding creation flow (already works correctly). The filter behavior when no frameworks are enabled (edge case, won't happen in practice). ## Verification ✅ org_69d943ca3fbbf2c473e97b0a now shows ISO42001 in the framework filter after enabling it ✅ findings correctly filter when selecting non-SOC2/ISO27001 frameworks ✅ hardcoded filter list replaced with dynamic org config ✅ no regression on existing SOC2/ISO27001 filtering
…-auth-flow fix(browserbase): make task auth flow primary
fix(findings): include all enabled frameworks in overview filter dropdown
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
1 participant
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
Ship per-site browser auth profiles and a sturdier browser automation stack. This adds saved login sessions per hostname, stable run outcomes, a task-first auth flow with a live view, and fixes the overview findings filter to include all enabled frameworks.
New Features
@trycompai/design-system.Migration
BrowserAuthProfile, failure enums, run fields, andblockedstatus).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 f28bcef. Summary will update on new commits.