Skip to content

Commit fc8a7c2

Browse files
BillLeoutsakosvl346Bill Leoutsakoscursoragent
authored
Harden Playwright foundation and persona fidelity (#5820)
* Harden Playwright lifecycle and persona fidelity Close production-state and cleanup gaps so the settings E2E platform fails closed without sacrificing useful diagnostics. * Serialize E2E signal cleanup ownership Prevent opposite signals and normal finalization from racing detached cleanup or releasing its run lock. * Preserve browser fixture failures Report network isolation violations without masking the original Playwright test or fixture error. * Make E2E signal handoff resilient Keep cleanup single-flight across repeated signals and retain lock ownership when supervisor logging or startup fails. * fix(e2e): preserve replacement org coverage Co-authored-by: Cursor <cursoragent@cursor.com> * fix(e2e): derive usage from entitled coverage Co-authored-by: Cursor <cursoragent@cursor.com> * fix(e2e): serialize cleanup lock ownership Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ci): detect safe E2E diagnostics directly Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b0b0470 commit fc8a7c2

39 files changed

Lines changed: 18322 additions & 256 deletions

.github/workflows/test-build.yml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,21 @@ jobs:
318318
E2E_PG_ADMIN_URL: 'postgresql://postgres:postgres@127.0.0.1:5432/postgres'
319319
run: bun run test:e2e
320320

321-
- name: Upload E2E diagnostics
321+
- name: Check E2E diagnostics eligibility
322+
id: e2e_diagnostics
322323
if: failure()
324+
shell: bash
325+
run: |
326+
shopt -s nullglob
327+
markers=(apps/sim/e2e/.runs/*/markers/leak-scan-complete.json)
328+
if (( ${#markers[@]} > 0 )); then
329+
echo "safe=true" >> "$GITHUB_OUTPUT"
330+
else
331+
echo "safe=false" >> "$GITHUB_OUTPUT"
332+
fi
333+
334+
- name: Upload E2E diagnostics
335+
if: failure() && steps.e2e_diagnostics.outputs.safe == 'true'
323336
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
324337
with:
325338
name: settings-e2e-${{ github.run_id }}

apps/sim/app/(landing)/layout.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { XPageViewTracker } from '@/app/(landing)/x-page-view-tracker'
1111
const HUBSPOT_SCRIPT_SRC = 'https://js-na2.hs-scripts.com/246720681.js' as const
1212

1313
const X_PIXEL_ID = 'q5xbl' as const
14+
const isMarketingAnalyticsEnabled = isHosted && !process.env.E2E_PROFILE
1415

1516
/** X (Twitter) conversion tracking base code — loads uwt.js and fires the initial PageView. */
1617
const X_PIXEL_BASE_CODE = `!function(e,t,n,s,u,a){e.twq||(s=e.twq=function(){s.exe?s.exe.apply(s,arguments):s.queue.push(arguments);
@@ -43,7 +44,7 @@ export default function LandingLayout({ children }: { children: ReactNode }) {
4344
<LandingShell>
4445
{children}
4546
{/* HubSpot + X pixel tracking — hosted only */}
46-
{isHosted && (
47+
{isMarketingAnalyticsEnabled && (
4748
<>
4849
<Script id='hs-script-loader' src={HUBSPOT_SCRIPT_SRC} strategy='afterInteractive' />
4950
<Script id='x-pixel-base' strategy='afterInteractive'>

apps/sim/app/api/users/me/settings/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest) => {
6060
.values({
6161
id: generateShortId(),
6262
userId,
63+
...defaultUserSettings,
6364
...validatedData,
6465
updatedAt: new Date(),
6566
})

apps/sim/app/layout.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export const metadata: Metadata = generateBrandedMetadata()
2828

2929
const GTM_ID = 'GTM-T7PHSRX5' as const
3030
const GA_ID = 'G-DR7YBE70VS' as const
31+
const isMarketingAnalyticsEnabled = isHosted && !process.env.E2E_PROFILE
3132

3233
export default function RootLayout({ children }: { children: React.ReactNode }) {
3334
const themeCSS = generateThemeCSS()
@@ -196,7 +197,7 @@ export default function RootLayout({ children }: { children: React.ReactNode })
196197
<meta httpEquiv='x-ua-compatible' content='ie=edge' />
197198

198199
{/* Google Tag Manager — hosted only */}
199-
{isHosted && (
200+
{isMarketingAnalyticsEnabled && (
200201
<Script
201202
id='gtm'
202203
strategy='afterInteractive'
@@ -211,7 +212,7 @@ j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
211212
)}
212213

213214
{/* Google Analytics (gtag.js) — hosted only */}
214-
{isHosted && (
215+
{isMarketingAnalyticsEnabled && (
215216
<>
216217
<Script
217218
id='gtag-src'
@@ -232,7 +233,7 @@ j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
232233
</head>
233234
<body className={`${season.variable} font-season`} suppressHydrationWarning>
234235
{/* Google Tag Manager (noscript) — hosted only */}
235-
{isHosted && (
236+
{isMarketingAnalyticsEnabled && (
236237
<noscript>
237238
<iframe
238239
src={`https://www.googletagmanager.com/ns.html?id=${GTM_ID}`}

apps/sim/e2e/README.md

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,19 @@ Server telemetry currently shares the strict loopback Stripe-fake process; the
5858
generic modular fake-service refactor remains scoped to the roadmap's
5959
`e2e/06b-enterprise-integrations` phase.
6060
An exclusive checkout-level orchestrator lock prevents concurrent runs from
61-
racing on `.next`, the shared build cache, or the fixed app/realtime ports.
61+
racing on `.next`, the shared build cache, or the fixed app/realtime ports. The
62+
lock records managed process groups, so recovery from an uncatchable
63+
orchestrator termination kills verified stale descendants before admitting a
64+
new writer.
6265

6366
On interruption, the runner launches a detached cleanup supervisor before
6467
exiting. It terminates managed process groups, force-drops the guarded database,
65-
and removes temporary auth/cloud-config directories even if another Ctrl-C
66-
terminates the foreground package runner.
68+
and removes temporary auth/cloud-config directories. Repeated or opposite
69+
signals remain on the same single-flight path until the supervisor owns the
70+
lock, after which the foreground runner exits.
6771
Cleanup failures retain the lock and require the reported resources to be
6872
inspected and cleaned before manually removing `e2e/.cache/orchestrator.lock`.
73+
Failure to start the detached cleanup supervisor also retains the lock.
6974

7075
Pass Playwright arguments after `--`:
7176

@@ -160,26 +165,31 @@ the in-memory canary scans the manifest, logs, report files, and trace archives
160165
and fails if a synthetic password, invitation token, or runtime secret escaped
161166
the excluded private directories. Cancelled CI runs do not upload unscanned
162167
diagnostics, and an unreadable canary or incomplete archive scan causes all
163-
potentially unscanned diagnostic roots to be scrubbed. Storage-state session cookies are intentionally not canaried
168+
potentially unscanned diagnostic roots to be scrubbed. CI uploads failure
169+
diagnostics only when the runner wrote its successful scan marker. The fixed
170+
foundation smoke password is public test input and is intentionally not a
171+
canary, so expected login traces remain useful. Storage-state session cookies are intentionally not canaried
164172
because authenticated Playwright traces contain them by design; they are
165173
synthetic and invalid once the run database is dropped.
166174
Fresh-session recapture is deliberately deferred. Future membership-mutation
167175
coverage must explicitly restore a private credential handoff and re-review its
168176
access boundary rather than assuming credentials persist through Playwright.
169177

170178
E2E builds verify the pinned Bun executable plus reviewed sandbox-bundle
171-
source, direct dependency, and output fingerprints and never regenerate
172-
committed `.cjs` files.
179+
source, direct dependency, and output fingerprints. They also regenerate the
180+
bundles into a temporary directory and require those fresh outputs to match the
181+
reviewed fingerprints without modifying committed `.cjs` files.
173182
`bun run build:sandbox-bundles:integrity` is the explicit maintenance command
174183
that regenerates bundles and their reviewed integrity manifest together.
175-
Unrelated monorepo lockfile changes do not invalidate the bundle fingerprint;
176-
the committed output hashes still detect any transitive change that alters a
177-
bundle.
184+
Unrelated monorepo lockfile changes do not invalidate the reviewed fingerprint;
185+
the fresh-output comparison detects transitive changes that alter a bundle.
178186

179187
Reset/reseed cleanup remains deferred with keep-stack supervision. Ordinary
180188
runs own a unique guarded database and remove it wholesale rather than carrying
181189
untested row-level deletion code.
182190

183191
Provider log scans are diagnostic tripwires, not proof of zero egress. The
184192
primary boundaries are the default-deny child environment, provider disabling,
185-
loopback-only service bindings, and guarded Stripe transport.
193+
loopback-only service bindings, guarded Stripe transport, disabled hosted
194+
marketing tags, and a browser-context allowlist that rejects every HTTP(S)
195+
origin outside the app and realtime service.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { test as base } from '@playwright/test'
2+
import { installBrowserNetworkGuard } from '../support/browser-network'
3+
4+
interface BrowserFixtures {
5+
browserNetworkGuard: undefined
6+
}
7+
8+
export const test = base.extend<BrowserFixtures>({
9+
browserNetworkGuard: [
10+
async ({ context }, use) => {
11+
const guard = await installBrowserNetworkGuard(context)
12+
const failures: unknown[] = []
13+
try {
14+
await use(undefined)
15+
} catch (error) {
16+
failures.push(error)
17+
}
18+
try {
19+
guard.assertNoUnexpectedRequests()
20+
} catch (error) {
21+
failures.push(error)
22+
}
23+
if (failures.length === 1) throw failures[0]
24+
if (failures.length > 1) {
25+
throw new AggregateError(failures, 'Browser test and network isolation both failed')
26+
}
27+
},
28+
{ auto: true },
29+
],
30+
})
31+
32+
export { expect } from '@playwright/test'

apps/sim/e2e/fixtures/factories/billing.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { db } from '@sim/db'
22
import { member, organization, subscription, user, userStats } from '@sim/db/schema'
33
import { generateId } from '@sim/utils/id'
44
import { and, eq, inArray } from 'drizzle-orm'
5+
import { hasOtherEntitledOrganizationSubscription } from '@/lib/billing/subscriptions/organization-coverage'
6+
import { detachOrganizationWorkspaces } from '@/lib/workspaces/organization-workspaces'
57

68
const CREDITS_PER_DOLLAR = 200
79
const FREE_USAGE_LIMIT_DOLLARS = '5'
@@ -66,6 +68,8 @@ export async function arrangeSubscription(input: SubscriptionArrangement): Promi
6668
metadata,
6769
})
6870

71+
if (status === 'canceled') return
72+
6973
if (input.plan.startsWith('pro_')) {
7074
await tx
7175
.update(userStats)
@@ -141,6 +145,13 @@ export async function lapseOrganizationSubscription(input: {
141145
periodEnd: now,
142146
})
143147
.where(eq(subscription.id, input.subscriptionId))
148+
})
149+
150+
if (await hasOtherEntitledOrganizationSubscription(input.organizationId, input.subscriptionId)) {
151+
return
152+
}
153+
154+
await db.transaction(async (tx) => {
144155
await tx
145156
.update(organization)
146157
.set({ orgUsageLimit: null, updatedAt: now })
@@ -157,6 +168,7 @@ export async function lapseOrganizationSubscription(input: {
157168
.where(inArray(userStats.userId, input.memberUserIds))
158169
}
159170
})
171+
await detachOrganizationWorkspaces(input.organizationId)
160172
}
161173

162174
function planCredits(plan: SubscriptionArrangement['plan']): number {

apps/sim/e2e/fixtures/persona-test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { test as base } from '@playwright/test'
2+
import { type BrowserNetworkGuard, installBrowserNetworkGuard } from '../support/browser-network'
23
import {
34
type PersonaManifestEntry,
45
readScenarioManifest,
@@ -23,7 +24,7 @@ export const test = base.extend<PersonaFixtures>({
2324
{ scope: 'test' },
2425
],
2526
contextForPersona: async ({ browser, contextOptions, personaManifest }, use) => {
26-
const contexts = new Set<import('@playwright/test').BrowserContext>()
27+
const contexts = new Map<import('@playwright/test').BrowserContext, BrowserNetworkGuard>()
2728
await use(async (personaKey) => {
2829
const persona = requirePersona(personaManifest, personaKey)
2930
const context = await browser.newContext({
@@ -34,10 +35,25 @@ export const test = base.extend<PersonaFixtures>({
3435
persona.storageStatePath
3536
),
3637
})
37-
contexts.add(context)
38+
contexts.set(context, await installBrowserNetworkGuard(context))
3839
return context
3940
})
40-
await Promise.all([...contexts].map((context) => context.close()))
41+
const failures: unknown[] = []
42+
for (const [context, guard] of contexts) {
43+
try {
44+
await context.close()
45+
} catch (error) {
46+
failures.push(error)
47+
}
48+
try {
49+
guard.assertNoUnexpectedRequests()
50+
} catch (error) {
51+
failures.push(error)
52+
}
53+
}
54+
if (failures.length > 0) {
55+
throw new AggregateError(failures, 'Persona browser cleanup or network isolation failed')
56+
}
4157
},
4258
})
4359

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import type { ResolvedScenario, ScenarioSubscription } from './scenario'
2+
3+
export function isEntitledSubscription(subscription: ScenarioSubscription): boolean {
4+
return subscription.status === 'active' || subscription.status === 'past_due'
5+
}
6+
7+
export function sameBillingReference(
8+
left: ScenarioSubscription,
9+
right: ScenarioSubscription
10+
): boolean {
11+
return billingReferenceKey(left) === billingReferenceKey(right)
12+
}
13+
14+
export function initialSubscriptionStatus(
15+
scenario: ResolvedScenario,
16+
definition: ScenarioSubscription
17+
): 'active' | 'past_due' | 'canceled' {
18+
if (definition.status !== 'lapsed') return definition.status
19+
const hasEntitledReplacement = scenario.definition.subscriptions.some(
20+
(candidate) =>
21+
candidate.key !== definition.key &&
22+
isEntitledSubscription(candidate) &&
23+
sameBillingReference(candidate, definition)
24+
)
25+
return hasEntitledReplacement ? 'canceled' : 'active'
26+
}
27+
28+
export function expectedUsageLimit(scenario: ResolvedScenario, userKey: string): string | null {
29+
const organizationKeys = new Set(
30+
scenario.definition.organizationMemberships
31+
.filter((membership) => membership.userKey === userKey)
32+
.map((membership) => membership.organizationKey)
33+
)
34+
const hasEntitledOrganizationSubscription = scenario.definition.subscriptions.some(
35+
(candidate) =>
36+
candidate.billingReference.kind === 'organization' &&
37+
organizationKeys.has(candidate.billingReference.organizationKey) &&
38+
isEntitledSubscription(candidate)
39+
)
40+
if (hasEntitledOrganizationSubscription) return null
41+
42+
const personalSubscription = scenario.definition.subscriptions.find(
43+
(candidate) =>
44+
candidate.billingReference.kind === 'user' &&
45+
candidate.billingReference.userKey === userKey &&
46+
isEntitledSubscription(candidate)
47+
)
48+
if (personalSubscription?.plan.startsWith('pro_')) {
49+
return String(Number(personalSubscription.plan.split('_')[1]) / 200)
50+
}
51+
return '5'
52+
}
53+
54+
export function billingReferenceKey(subscription: ScenarioSubscription): string {
55+
return subscription.billingReference.kind === 'user'
56+
? `user/${subscription.billingReference.userKey}`
57+
: `organization/${subscription.billingReference.organizationKey}`
58+
}

0 commit comments

Comments
 (0)