Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/core/src/canvas/canvasBuildSchemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest";
import {
type CanvasBuildLifecycle,
type CanvasBuildRecord,
currentHeadBuildFailure,
hasActiveCanvasBuild,
latestFinishedCanvasBuild,
publishedCanvasBuild,
Expand Down Expand Up @@ -73,6 +74,32 @@ describe("canvas build lifecycle", () => {
expect(publishedCanvasBuild(value)).toBe(ready);
});

it("surfaces a failed build of the current head even when an older published build is also ready", () => {
// b_old (sv-old) is the pinned/published live build; a newer publish (b_new,
// sv-new = current head) failed. latestFinishedCanvasBuild returns the first
// finished build in array order, which here is the ready b_old — position,
// not version identity — so it would hide the failure. currentHeadBuildFailure
// keys off the current version's own build.
const value = lifecycle([
build("b_old", "ready"),
build("b_new", "failed"),
]);
value.builds[0].sourceVersionId = "sv-old";
value.builds[1].sourceVersionId = "sv-new";
value.publishedBuildId = "b_old";
value.currentVersionId = "sv-new";

expect(latestFinishedCanvasBuild(value)?.id).toBe("b_old");
expect(currentHeadBuildFailure(value)?.id).toBe("b_new");
});

it("returns null when the current head built fine or is still in flight", () => {
const value = lifecycle([build("b1", "building")]);
value.currentVersionId = "sv-1";

expect(currentHeadBuildFailure(value)).toBeNull();
});

it("maps the builds endpoint's snake_case body to the client shape", async () => {
const fetchMock = vi.fn(
async () =>
Expand Down
20 changes: 20 additions & 0 deletions packages/core/src/canvas/canvasBuildSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,23 @@ export function latestFinishedCanvasBuild(
) ?? null
);
}

/**
* The failed build of the canvas's CURRENT head, if it failed. This is the
* build whose outcome the author actually cares about — a failed newest
* publish must surface even when an older (e.g. pinned/published) build is the
* first finished row in the list, because `latestFinishedCanvasBuild` picks by
* array position, not version identity.
*/
export function currentHeadBuildFailure(
lifecycle: CanvasBuildLifecycle,
): CanvasBuildRecord | null {
if (!lifecycle.currentVersionId) return null;
return (
lifecycle.builds.find(
(build) =>
build.sourceVersionId === lifecycle.currentVersionId &&
build.buildStatus === "failed",
) ?? null
);
}
76 changes: 75 additions & 1 deletion packages/core/src/canvas/dashboardsService.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { transform } from "esbuild";
import { describe, expect, it, vi } from "vitest";
import { DashboardsService } from "./dashboardsService";
import type { ProjectApiClient } from "./projectApiClient";
import { type ProjectApiClient, ProjectApiError } from "./projectApiClient";

// A canvas as the PostHog canvases API returns it.
function apiCanvas(overrides: Record<string, unknown> = {}) {
Expand Down Expand Up @@ -172,6 +172,80 @@ describe("DashboardsService.ensureHomeCanvas", () => {
});
});

describe("DashboardsService.ensureHomeCanvas races", () => {
it("reuses the winner's canvas when create loses the is_home uniqueness race (409)", async () => {
let lookups = 0;
const { api } = fakeApi({
// First lookup: none. After the 409, the winner's home canvas exists.
"canvases/?channel=chan-1&is_home=true": () => {
lookups += 1;
return lookups === 1
? []
: [
apiCanvas({
id: "home-winner",
is_home: true,
current_version_id: "v1",
}),
];
},
"canvases/": () => {
throw new ProjectApiError("Failed to create canvas (409)", 409);
},
});
const service = new DashboardsService(api);

const record = await service.ensureHomeCanvas("chan-1");

expect(record.id).toBe("home-winner");
});

it("rethrows a non-409 create failure instead of masking it as a race", async () => {
const { api } = fakeApi({
"canvases/?channel=chan-1&is_home=true": [],
"canvases/": () => {
throw new ProjectApiError("Failed to create canvas (403)", 403);
},
});
const service = new DashboardsService(api);

await expect(service.ensureHomeCanvas("chan-1")).rejects.toMatchObject({
status: 403,
});
});

it("retries the seed publish once on a 409 version conflict", async () => {
const home = apiCanvas({
id: "home-1",
is_home: true,
current_version_id: null,
});
let publishCalls = 0;
const { api } = fakeApi({
"canvases/?channel=chan-1&is_home=true": [home],
"canvases/home-1/publish/": (init?: RequestInit) => {
publishCalls += 1;
if (publishCalls === 1) {
throw new ProjectApiError("Failed to seed home canvas (409)", 409);
}
const body = JSON.parse(String(init?.body));
return { current_version_id: body.expected_current_version_id ?? "v1" };
},
"canvases/home-1/": apiCanvas({
id: "home-1",
is_home: true,
current_version_id: "v-fresh",
}),
});
const service = new DashboardsService(api);

const record = await service.ensureHomeCanvas("chan-1");

expect(publishCalls).toBe(2);
expect(record.id).toBe("home-1");
});
});

describe("DashboardsService.resetHomeCanvas", () => {
it("publishes a fresh default guarded on the current head", async () => {
const home = apiCanvas({
Expand Down
51 changes: 35 additions & 16 deletions packages/core/src/canvas/dashboardsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import type {
DashboardRecord,
} from "./dashboardSchemas";
import { FREEFORM_TEMPLATE_ID } from "./freeformSchemas";
import { PROJECT_API_CLIENT, type ProjectApiClient } from "./projectApiClient";
import {
apiErrorStatus,
PROJECT_API_CLIENT,
type ProjectApiClient,
} from "./projectApiClient";

// Display name (canvas h1) of a channel's auto-created home canvas.
const HOME_CANVAS_NAME = "Home";
Expand Down Expand Up @@ -306,8 +310,11 @@ export class DashboardsService {
templateId: FREEFORM_TEMPLATE_ID,
isHome: true,
});
} catch {
// Lost the uniqueness race — another client created it; reuse theirs.
} catch (error) {
// Only the is_home uniqueness race (409) means another client created
// it; reuse theirs. Any other failure (auth, capacity, network) must
// surface, not be masked as a race.
if (apiErrorStatus(error) !== 409) throw error;
record = await this.findHomeCanvas(channelId);
if (!record) throw new Error("Failed to create home canvas");
}
Expand Down Expand Up @@ -358,19 +365,31 @@ export class DashboardsService {
network: { origins: [] },
},
};
await this.api.json<unknown>(
`canvases/${encodeURIComponent(record.id)}/publish/`,
"seed home canvas",
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
project,
prompt: "Default home board",
expected_current_version_id: record.currentVersionId ?? null,
}),
},
);
const publish = (expectedVersionId: string | null) =>
this.api.json<unknown>(
`canvases/${encodeURIComponent(record.id)}/publish/`,
"seed home canvas",
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
project,
prompt: "Default home board",
expected_current_version_id: expectedVersionId,
}),
},
);
try {
await publish(record.currentVersionId ?? null);
} catch (error) {
// A concurrent seed can win the guarded publish between our read and
// POST. On the 409 version conflict, re-read the head and retry once
// against the fresh version id rather than failing the channel open.
if (apiErrorStatus(error) !== 409) throw error;
const fresh = await this.get(record.id);
await publish(fresh?.currentVersionId ?? null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Conflict retry overwrites newer head

When resetHomeCanvas conflicts with a concurrent user publish, this retry adopts the newly published version as expected_current_version_id and publishes the default project as the next head, silently replacing the user's newer content.

File Used: AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/canvas/dashboardsService.ts
Line: 389-390

Comment:
**Conflict retry overwrites newer head**

When `resetHomeCanvas` conflicts with a concurrent user publish, this retry adopts the newly published version as `expected_current_version_id` and publishes the default project as the next head, silently replacing the user's newer content.

**File Used:** AGENTS.md ([source](AGENTS.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

return fresh ?? record;
}
const fresh = await this.get(record.id);
return fresh ?? record;
}
Expand Down
22 changes: 21 additions & 1 deletion packages/core/src/canvas/projectApiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ export const PROJECT_API_CLIENT = Symbol.for(

const MAX_PAGES = 50;

/** An API call that failed with an HTTP status (so callers can branch on it). */
export class ProjectApiError extends Error {
constructor(
message: string,
public readonly status: number,
) {
super(message);
this.name = "ProjectApiError";
}
}

/** The status code of a ProjectApiError, or null for a non-API error. */
export function apiErrorStatus(error: unknown): number | null {
return error instanceof ProjectApiError ? error.status : null;
}

/**
* Thin shared client for the current PostHog project's REST API. Resolves the
* project + auth, then forwards to authenticated fetch. Owners of typed
Expand Down Expand Up @@ -36,7 +52,11 @@ export class ProjectApiClient {
init?: RequestInit,
): Promise<T> {
const res = await this.fetch(path, init);
if (!res.ok) throw new Error(`Failed to ${errorLabel} (${res.status})`);
if (!res.ok)
throw new ProjectApiError(
`Failed to ${errorLabel} (${res.status})`,
res.status,
);
return (await res.json()) as T;
}

Expand Down
11 changes: 9 additions & 2 deletions packages/ui/src/features/canvas/freeform/CanvasBuildStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import {
WarningCircleIcon,
XIcon,
} from "@phosphor-icons/react";
import { latestFinishedCanvasBuild } from "@posthog/core/canvas/canvasBuildSchemas";
import {
currentHeadBuildFailure,
latestFinishedCanvasBuild,
} from "@posthog/core/canvas/canvasBuildSchemas";
import { useHostTRPC } from "@posthog/host-router/react";
import { Button } from "@posthog/quill";
import type { CanvasDiagnostic } from "@posthog/shared";
Expand Down Expand Up @@ -114,7 +117,11 @@ export function CanvasBuildStatus({
);
}

const latest = latestFinishedCanvasBuild(lifecycle);
// Surface a failed build of the CURRENT head even when an older (pinned /
// published) finished build appears first in the list — array position must
// not hide a failed newest publish.
const failedHead = currentHeadBuildFailure(lifecycle);
const latest = failedHead ?? latestFinishedCanvasBuild(lifecycle);
if (!latest) return null;

if (latest.buildStatus === "failed") {
Expand Down
47 changes: 35 additions & 12 deletions packages/ui/src/features/canvas/freeform/FreeformCanvasView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ export function FreeformCanvasView({
const setBrowseVersion = useFreeformChatStore((s) => s.setBrowseVersion);
const setRuntimeError = useFreeformChatStore((s) => s.setRuntimeError);

// Protect this thread from LRU eviction while the view is open — a burst of
// background patches must never drop the canvas the user is looking at.
useEffect(() => {
const store = useFreeformChatStore.getState();
store.setThreadMounted(threadId, true);
return () => {
useFreeformChatStore.getState().setThreadMounted(threadId, false);
};
}, [threadId]);

// Right-hand panel state (persisted minimize + width). `startedTaskId` is a
// local bridge so the composer floats to the side immediately on submit,
// before the canvas record's polled generationTaskId catches up.
Expand Down Expand Up @@ -237,26 +247,33 @@ export function FreeformCanvasView({
// Pin the artifact to one signed URL per build: every lifecycle refetch mints
// a fresh URL for the same artifact, and adopting each one would reload the
// iframe on every 2s poll while a build runs. Adopt only when the published
// build itself changes — or when a refresh was explicitly requested because
// the pinned URL expired. Adjusted during render (not an effect) so the swap
// build itself changes. Adjusted during render (not an effect) so the swap
// can't flash a stale frame.
const [pinnedArtifact, setPinnedArtifact] = useState<PinnedArtifact | null>(
null,
);
const wantFreshArtifactUrlRef = useRef(false);
// A nonce that, when bumped, remounts the artifact frame so it revalidates
// against the live token endpoint (ETag/304 makes this cheap) — the recovery
// path when the pinned URL expired. Remounting, not URL-string compare, is
// what guarantees a wedged iframe actually retries: the token endpoint is the
// authority, and a new URL for the same bucket would be byte-identical.
const [artifactRefreshKey, setArtifactRefreshKey] = useState(0);
// Track the refresh key the current pin was adopted under, so a remount also
// re-stamps the pin's mint time (otherwise the expiry timer would keep firing
// on a URL that's already been recovered).
const [pinnedForRefreshKey, setPinnedForRefreshKey] = useState(-1);
if (publishedBuild?.artifactUrl) {
const shouldAdopt =
const adoptFresh =
!pinnedArtifact ||
pinnedArtifact.buildId !== publishedBuild.id ||
(wantFreshArtifactUrlRef.current &&
pinnedArtifact.url !== publishedBuild.artifactUrl);
if (shouldAdopt) {
wantFreshArtifactUrlRef.current = false;
pinnedForRefreshKey !== artifactRefreshKey;
if (adoptFresh) {
setPinnedArtifact({
buildId: publishedBuild.id,
url: publishedBuild.artifactUrl,
mintedAt: buildsUpdatedAt || Date.now(),
});
setPinnedForRefreshKey(artifactRefreshKey);
}
} else if (lifecycle && pinnedArtifact) {
// The lifecycle says there's no published build anymore — drop the pin.
Expand All @@ -280,10 +297,15 @@ export function FreeformCanvasView({
if (Date.now() - renderedArtifact.mintedAt < ARTIFACT_URL_FRESH_MS) {
return;
}
wantFreshArtifactUrlRef.current = true;
void queryClient.invalidateQueries({
queryKey: trpc.dashboards.builds.queryKey({ id: dashboardId }),
});
// Refetch mints the current bucket's URL (re-checking the token server-
// side even when the browser would reframe from cache), then remount the
// frame so it revalidates against those endpoints. The remount, not a URL
// string change, is what un-wedges a frame whose module fetches hung.
void queryClient
.invalidateQueries({
queryKey: trpc.dashboards.builds.queryKey({ id: dashboardId }),
})
.then(() => setArtifactRefreshKey((k) => k + 1));
}, ARTIFACT_READY_GRACE_MS);
return () => clearTimeout(timer);
}, [renderedArtifact, dashboardId, queryClient, trpc]);
Expand Down Expand Up @@ -707,6 +729,7 @@ export function FreeformCanvasView({
) : pinnedArtifact ? (
<Box className="h-full w-full">
<BuiltCanvas
key={`${pinnedArtifact.buildId}:${artifactRefreshKey}`}
artifactUrl={pinnedArtifact.url}
onDataRequest={onDataRequest}
onError={onError}
Expand Down
Loading
Loading