diff --git a/packages/studio/src/utils/sdkCutover.ts b/packages/studio/src/utils/sdkCutover.ts index e76283aa0..b17b03fe2 100644 --- a/packages/studio/src/utils/sdkCutover.ts +++ b/packages/studio/src/utils/sdkCutover.ts @@ -189,6 +189,7 @@ export async function sdkTimingPersist( hfId, "setTiming", timingSrc ? () => timingSrc(targetPath) : undefined, + { targetPath, compositionPath: deps.compositionPath }, ); // Dark-launch gate: without this, timing cutover runs whenever an SDK session // exists (it always does, for shadow/selection) — flipping the flag OFF would @@ -231,6 +232,7 @@ export async function sdkTimingBatchPersist( change.hfId, "setTiming", timingSrc ? () => timingSrc(targetPath) : undefined, + { targetPath, compositionPath: deps.compositionPath }, ); } if (!STUDIO_SDK_CUTOVER_ENABLED) return false; @@ -283,6 +285,7 @@ export function sdkGsapTweenPersist( op.target, "addGsapTween", gsapSrc ? () => gsapSrc(targetPath) : undefined, + { targetPath, compositionPath: deps.compositionPath }, ); } else { void recordAnimationResolverParity( @@ -290,6 +293,7 @@ export function sdkGsapTweenPersist( op.animationId, op.kind === "set" ? "setGsapTween" : "removeGsapTween", gsapReadSource(deps, targetPath), + { targetPath, compositionPath: deps.compositionPath }, ); } // Leading dark-launch gate so flag-off does no SDK touch (getElement) at all — @@ -329,6 +333,7 @@ async function dispatchGsapOpAndPersist( resolverTarget.animationId, resolverTarget.opLabel, gsapReadSource(deps, targetPath), + { targetPath, compositionPath: deps.compositionPath }, ); } // Dark-launch gate (shared chokepoint for every GSAP-op cutover persist): @@ -553,8 +558,12 @@ export async function sdkDeletePersist( deps: CutoverDeps, ): Promise { // Resolver tripwire — runs BEFORE the cutover gate (decoupled). - void recordResolverParity(sdkSession, hfId, "removeElement", () => - Promise.resolve(originalContent), + void recordResolverParity( + sdkSession, + hfId, + "removeElement", + () => Promise.resolve(originalContent), + { targetPath, compositionPath: deps.compositionPath }, ); // Dark-launch gate: flag OFF → legacy server delete path. if (!STUDIO_SDK_CUTOVER_ENABLED) return false; diff --git a/packages/studio/src/utils/sdkResolverShadow.test.ts b/packages/studio/src/utils/sdkResolverShadow.test.ts index 5401afd9d..8264d3bf4 100644 --- a/packages/studio/src/utils/sdkResolverShadow.test.ts +++ b/packages/studio/src/utils/sdkResolverShadow.test.ts @@ -432,6 +432,18 @@ describe("F. recordResolverParity", () => { expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); }); + it("skips entirely (no event, no attempt) for a cross-file op", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + flushAttemptCounts(); + const session = await openComposition(BASE_HTML); + await recordResolverParity(session, "hf-cross", "setTiming", undefined, { + targetPath: "compositions/other.html", + compositionPath: "index.html", + }); + expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); + expect(flushAttemptCounts()).toBeNull(); + }); + it("emits with sourceHfIdCount=1 when the hfId IS in source but missing from the session", async () => { mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; const session = await openComposition(BASE_HTML); @@ -616,6 +628,48 @@ describe("G. recordAnimationResolverParity", () => { expect(ev?.sourceReadFailed).toBe(true); expect(ev?.diskChecked).toBeUndefined(); }); + + it("fails open with sourceReadFailed when the reader throws SYNCHRONOUSLY", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(GSAP_HTML); + await recordAnimationResolverParity(session, "no-such-anim", "removeGsapKeyframe", () => { + throw new Error("sync read failed"); + }); + const ev = lastShadow(); + expect(ev?.mismatchCount).toBe(1); + expect(ev?.sourceReadFailed).toBe(true); + }); + + it("dispatches the disk read synchronously on a miss — before the caller's write can land", async () => { + // The tripwire is fire-and-forget and the caller's cutover persist writes + // this same file moments after it returns. The read request must be + // dispatched in the same sync prologue as the miss check, so it observes + // the PRE-write file (a post-write read would see a remove op's target + // legitimately gone and misclassify it as a genuine divergence). + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + const session = await openComposition(GSAP_HTML); + const read = vi.fn(() => Promise.resolve(GSAP_DISK_MOVED_HTML)); + const pending = recordAnimationResolverParity( + session, + "no-such-anim", + "removeGsapKeyframe", + read, + ); + expect(read).toHaveBeenCalledTimes(1); // already dispatched, no await yet + await pending; + }); + + it("skips entirely (no event, no attempt) for a cross-file op", async () => { + mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true; + flushAttemptCounts(); + const session = await openComposition(GSAP_HTML); + await recordAnimationResolverParity(session, "no-such-anim", "removeGsapKeyframe", undefined, { + targetPath: "compositions/other.html", + compositionPath: "index.html", + }); + expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0); + expect(flushAttemptCounts()).toBeNull(); + }); }); // ─── G2. runResolverShadow cross-file guard ─────────────────────────────────── diff --git a/packages/studio/src/utils/sdkResolverShadow.ts b/packages/studio/src/utils/sdkResolverShadow.ts index f26db5817..6508e683f 100644 --- a/packages/studio/src/utils/sdkResolverShadow.ts +++ b/packages/studio/src/utils/sdkResolverShadow.ts @@ -300,29 +300,41 @@ function reportEmptySession(session: Composition, opLabel: string): boolean { return true; } +/** Shape shared by every resolver-shadow entry point's optional path pair. */ +export interface ResolverShadowPaths { + targetPath?: string; + compositionPath?: string | null; +} + +/** + * Cross-file edit: the session models ONLY the active composition, so a + * target living in another file is structurally unresolvable — the cutover + * gates decline it (wrongCompositionFile) and it can never cut over. Every + * tripwire entry point skips entirely (no event, no attempt): the op belongs + * in neither the divergence count nor the attempt denominator of the soak + * rate. PostHog 0.7.41: one cross-file editing session emitted 479 false + * element_not_found events before this rule existed. Callers that pass no + * paths (isolated consumers/tests) run as before. + */ +function isCrossFileEdit(paths?: ResolverShadowPaths): boolean { + return ( + paths?.targetPath !== undefined && + paths.compositionPath != null && + paths.targetPath !== paths.compositionPath + ); +} + export function runResolverShadow( session: Composition, hfId: string | null | undefined, ops: PatchOperation[], sourceContent?: string, - paths?: { targetPath?: string; compositionPath?: string | null }, + paths?: ResolverShadowPaths, ): void { if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return; if (!hfId) return; try { - // Cross-file edit: the session models ONLY the active composition, so a - // target living in another file is structurally unresolvable — the cutover - // gates decline it (wrongCompositionFile) and it can never cut over. Skip - // entirely (no event, no attempt), mirroring the empty-session rule below. - // PostHog 0.7.41: one cross-file editing session emitted 479 false - // element_not_found events through this path. - if ( - paths?.targetPath !== undefined && - paths.compositionPath != null && - paths.targetPath !== paths.compositionPath - ) { - return; - } + if (isCrossFileEdit(paths)) return; if (reportEmptySession(session, "dom-edit")) return; recordAttempt("dom-edit"); const mismatches = sdkResolverShadowCheck(session, hfId, ops, sourceContent); @@ -368,15 +380,41 @@ export function runResolverShadow( * * No-op when the shadow flag is off; never throws; never mutates the session. */ +/** + * Source-truth check for a missed hf-id: read the target file and decide + * whether the miss is a runtime-generated node (id absent from source — + * suppress, the SDK cannot model it by design) or a reportable divergence + * (id present → strict attribute count for the event; read failure → fail + * open, tagged). Mirrors checkAnimationIdOnDisk's error discipline. + */ +async function checkHfIdInSource( + hfId: string, + readSource?: () => Promise, +): Promise<{ suppress: boolean; strictCount?: number; sourceReadFailed: boolean }> { + if (!readSource) return { suppress: false, sourceReadFailed: false }; + let source: string | undefined; + try { + source = await readSource(); + } catch { + return { suppress: false, sourceReadFailed: true }; // fail-open + } + if (source === undefined) return { suppress: false, sourceReadFailed: false }; + // Loose substring match — biased toward keeping signal (see sourceLooseMatchOnly). + if (!source.includes(hfId)) return { suppress: true, sourceReadFailed: false }; + return { suppress: false, strictCount: countHfIdInSource(source, hfId), sourceReadFailed: false }; +} + export async function recordResolverParity( session: Composition | null | undefined, hfId: string | null | undefined, opLabel: string, readSource?: () => Promise, + paths?: ResolverShadowPaths, ): Promise { if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return; if (!session || !hfId) return; try { + if (isCrossFileEdit(paths)) return; if (reportEmptySession(session, opLabel)) return; recordAttempt(opLabel); if (resolveSnapshot(session, hfId)) return; // resolves — parity, nothing to record @@ -387,19 +425,10 @@ export async function recordResolverParity( // state this field exists to diagnose. const sessionElementCount = session.getElements().length; // Cheap check passed above, so the source read only runs on a real divergence. - let source: string | undefined; - let sourceReadFailed = false; - if (readSource) { - try { - source = await readSource(); - } catch { - source = undefined; // fail-open: a read error must not drop a real divergence - sourceReadFailed = true; - } - } - // Runtime-generated node the static parse can't model — suppress (mirrors the dom-edit path). - if (source !== undefined && !source.includes(hfId)) return; - const strictCount = source !== undefined ? countHfIdInSource(source, hfId) : undefined; + // Runtime-generated node the static parse can't model → suppressed inside. + const verdict = await checkHfIdInSource(hfId, readSource); + if (verdict.suppress) return; + const { strictCount, sourceReadFailed } = verdict; trackStudioEvent("sdk_resolver_shadow", { hfId, opLabel, @@ -452,18 +481,27 @@ export async function recordResolverParity( */ async function checkAnimationIdOnDisk( animationId: string, - readSource: () => Promise, + sourcePromise: Promise, ): Promise<{ staleSession: boolean; diskChecked: boolean; sourceReadFailed: boolean }> { let source: string | undefined; try { - source = await readSource(); + source = await sourcePromise; } catch { return { staleSession: false, diskChecked: false, sourceReadFailed: true }; } if (source === undefined) { return { staleSession: false, diskChecked: false, sourceReadFailed: false }; } - const disk = await openComposition(source, { history: false }); + // A parse failure (malformed script the user just introduced) means the + // source can't serve as ground truth — treat it exactly like a read failure + // (fail open, tagged) instead of letting the outer never-propagate catch + // swallow the whole divergence event. + let disk: Composition; + try { + disk = await openComposition(source, { history: false }); + } catch { + return { staleSession: false, diskChecked: false, sourceReadFailed: true }; + } try { return { staleSession: disk.getAllAnimationIds().has(animationId), @@ -480,10 +518,12 @@ export async function recordAnimationResolverParity( animationId: string, opLabel: string, readSource?: () => Promise, + paths?: ResolverShadowPaths, ): Promise { if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return; if (!session || !animationId) return; try { + if (isCrossFileEdit(paths)) return; recordAttempt(opLabel); const elements = session.getElements(); const resolves = @@ -495,7 +535,14 @@ export async function recordAnimationResolverParity( let diskChecked = false; let sourceReadFailed = false; if (readSource) { - const verdict = await checkAnimationIdOnDisk(animationId, readSource); + // Start the read SYNCHRONOUSLY, before returning control to the caller: + // the caller's cutover persist writes the new content to this same file + // moments later, and a read dispatched after that write would check the + // POST-edit disk (e.g. a remove op's target legitimately gone → false + // "genuine divergence"). Dispatching the request here, in the same sync + // prologue as the miss check, orders it ahead of the caller's write. + const sourcePromise = (async () => readSource())(); + const verdict = await checkAnimationIdOnDisk(animationId, sourcePromise); if (verdict.staleSession) return; // sync gap, not a resolver bug — suppress diskChecked = verdict.diskChecked; sourceReadFailed = verdict.sourceReadFailed;