fix: clear stale preview URL from SQLite and prevent stale white screen on preview clear (#2358)
* fix: remove auto-switch-to-summary effect, keep browser tab on clear * fix: clear browser preview when session is terminated
This commit is contained in:
parent
fadbdd26cd
commit
7e409d438b
|
|
@ -44,6 +44,10 @@ type entryState struct {
|
||||||
path string
|
path string
|
||||||
modUnix int64
|
modUnix int64
|
||||||
size int64
|
size int64
|
||||||
|
// cleared is set when the poller itself cleared the preview URL because the
|
||||||
|
// workspace entry was missing. When the file reappears, shouldRefresh uses
|
||||||
|
// this to re-discover even though the revision was bumped by the clear.
|
||||||
|
cleared bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewPoller constructs a preview poller over the supplied session source and setter.
|
// NewPoller constructs a preview poller over the supplied session source and setter.
|
||||||
|
|
@ -112,6 +116,13 @@ func (p *Poller) Poll(ctx context.Context) error {
|
||||||
}
|
}
|
||||||
entry, ok := DiscoverEntry(sess.Metadata.WorkspacePath)
|
entry, ok := DiscoverEntry(sess.Metadata.WorkspacePath)
|
||||||
if !ok {
|
if !ok {
|
||||||
|
if isWorkspacePreviewURL(sess.Metadata.PreviewURL, sess.ID) {
|
||||||
|
if _, err := p.setter.SetPreview(ctx, sess.ID, ""); err != nil {
|
||||||
|
p.logger.Error("preview poller: failed to clear stale preview",
|
||||||
|
"session", sess.ID, "err", err)
|
||||||
|
}
|
||||||
|
p.seen[sess.ID] = entryState{cleared: true}
|
||||||
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
state := stateFor(entry)
|
state := stateFor(entry)
|
||||||
|
|
@ -140,7 +151,11 @@ func (p *Poller) Poll(ctx context.Context) error {
|
||||||
func (p *Poller) shouldRefresh(sess domain.SessionRecord, target string, seenBefore bool) bool {
|
func (p *Poller) shouldRefresh(sess domain.SessionRecord, target string, seenBefore bool) bool {
|
||||||
current := strings.TrimSpace(sess.Metadata.PreviewURL)
|
current := strings.TrimSpace(sess.Metadata.PreviewURL)
|
||||||
if current == "" {
|
if current == "" {
|
||||||
return !seenBefore && sess.Metadata.PreviewRevision == 0
|
if !seenBefore {
|
||||||
|
return sess.Metadata.PreviewRevision == 0
|
||||||
|
}
|
||||||
|
previous := p.seen[sess.ID]
|
||||||
|
return previous.cleared
|
||||||
}
|
}
|
||||||
if current == target || isWorkspacePreviewURL(current, sess.ID) {
|
if current == target || isWorkspacePreviewURL(current, sess.ID) {
|
||||||
return true
|
return true
|
||||||
|
|
|
||||||
|
|
@ -118,6 +118,47 @@ func TestPollerRefreshesOnlyWhenEntrypointChanges(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestPollerRediscoverEntryAfterDeleteAndRecreate(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
entry := filepath.Join(workspace, "index.html")
|
||||||
|
writeFile(t, entry, "<main>v1</main>")
|
||||||
|
svc := &fakePreviewSessions{sessions: []domain.SessionRecord{workerSession("ao-1", workspace, "")}}
|
||||||
|
poller := NewPoller(svc, svc, "http://127.0.0.1:3001", PollerConfig{Logger: discardLogger()})
|
||||||
|
|
||||||
|
// First poll discovers the entry and sets the preview.
|
||||||
|
if err := poller.Poll(context.Background()); err != nil {
|
||||||
|
t.Fatalf("first Poll: %v", err)
|
||||||
|
}
|
||||||
|
wantURL := "http://127.0.0.1:3001/api/v1/sessions/ao-1/preview/files/index.html"
|
||||||
|
assertSets(t, svc.sets, previewSet{id: "ao-1", url: wantURL})
|
||||||
|
|
||||||
|
// Delete the entry — poller must clear the preview and mark the session cleared.
|
||||||
|
if err := os.Remove(entry); err != nil {
|
||||||
|
t.Fatalf("remove index.html: %v", err)
|
||||||
|
}
|
||||||
|
if err := poller.Poll(context.Background()); err != nil {
|
||||||
|
t.Fatalf("second Poll (delete): %v", err)
|
||||||
|
}
|
||||||
|
if len(svc.sets) != 2 {
|
||||||
|
t.Fatalf("sets after delete = %#v, want clear + set", svc.sets)
|
||||||
|
}
|
||||||
|
if svc.sets[1].url != "" {
|
||||||
|
t.Fatalf("second set.url = %q, want empty (clear)", svc.sets[1].url)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Recreate the entry — poller must re-discover.
|
||||||
|
writeFile(t, entry, "<main>v2</main>")
|
||||||
|
if err := poller.Poll(context.Background()); err != nil {
|
||||||
|
t.Fatalf("third Poll (recreate): %v", err)
|
||||||
|
}
|
||||||
|
if len(svc.sets) != 3 {
|
||||||
|
t.Fatalf("sets after recreate = %#v, want 3 sets (discover + clear + rediscover)", svc.sets)
|
||||||
|
}
|
||||||
|
if svc.sets[2].url != wantURL {
|
||||||
|
t.Fatalf("third set.url = %q, want %q", svc.sets[2].url, wantURL)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPollerDoesNotRestoreClearedPreviewAfterRestart(t *testing.T) {
|
func TestPollerDoesNotRestoreClearedPreviewAfterRestart(t *testing.T) {
|
||||||
workspace := t.TempDir()
|
workspace := t.TempDir()
|
||||||
writeFile(t, filepath.Join(workspace, "index.html"), "<main>hello</main>")
|
writeFile(t, filepath.Join(workspace, "index.html"), "<main>hello</main>")
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,7 @@ function setupHost() {
|
||||||
const webContents = {
|
const webContents = {
|
||||||
canGoBack: () => false,
|
canGoBack: () => false,
|
||||||
canGoForward: () => false,
|
canGoForward: () => false,
|
||||||
|
clearHistory: () => undefined,
|
||||||
getTitle: () => "",
|
getTitle: () => "",
|
||||||
getURL: () => currentURL,
|
getURL: () => currentURL,
|
||||||
goBack: () => undefined,
|
goBack: () => undefined,
|
||||||
|
|
@ -120,6 +121,7 @@ describe("dispose after the window is destroyed", () => {
|
||||||
webContents: {
|
webContents: {
|
||||||
canGoBack: () => false,
|
canGoBack: () => false,
|
||||||
canGoForward: () => false,
|
canGoForward: () => false,
|
||||||
|
clearHistory: () => undefined,
|
||||||
getTitle: () => "",
|
getTitle: () => "",
|
||||||
getURL: () => "",
|
getURL: () => "",
|
||||||
goBack: () => undefined,
|
goBack: () => undefined,
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,7 @@ type BrowserWebContents = Pick<
|
||||||
WebContents,
|
WebContents,
|
||||||
| "canGoBack"
|
| "canGoBack"
|
||||||
| "canGoForward"
|
| "canGoForward"
|
||||||
|
| "clearHistory"
|
||||||
| "getTitle"
|
| "getTitle"
|
||||||
| "getURL"
|
| "getURL"
|
||||||
| "goBack"
|
| "goBack"
|
||||||
|
|
@ -190,7 +191,10 @@ export function createBrowserViewHost(options: BrowserViewHostOptions): BrowserV
|
||||||
// empty state.
|
// empty state.
|
||||||
const clear = async (viewId: string): Promise<BrowserNavState> => {
|
const clear = async (viewId: string): Promise<BrowserNavState> => {
|
||||||
const entry = ensure(viewId);
|
const entry = ensure(viewId);
|
||||||
|
entry.view.setVisible?.(false);
|
||||||
|
entry.view.setBounds(OFFSCREEN_BOUNDS);
|
||||||
await entry.view.webContents.loadURL("about:blank");
|
await entry.view.webContents.loadURL("about:blank");
|
||||||
|
entry.view.webContents.clearHistory();
|
||||||
return pushNavState(options, entry);
|
return pushNavState(options, entry);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -60,6 +60,7 @@ export function SessionView({ sessionId }: SessionViewProps) {
|
||||||
sessionId,
|
sessionId,
|
||||||
active: Boolean(session && hasInspector && (browserPoppedOut || isInspectorOpen)),
|
active: Boolean(session && hasInspector && (browserPoppedOut || isInspectorOpen)),
|
||||||
poppedOut: browserPoppedOut,
|
poppedOut: browserPoppedOut,
|
||||||
|
terminated: session?.status === "terminated",
|
||||||
previewUrl,
|
previewUrl,
|
||||||
previewRevision,
|
previewRevision,
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -75,6 +75,18 @@ describe("useBrowserView", () => {
|
||||||
const { result } = renderHook(() => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false }));
|
const { result } = renderHook(() => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false }));
|
||||||
|
|
||||||
await waitFor(() => expect(bridge.ensure).toHaveBeenCalledWith("sess-1"));
|
await waitFor(() => expect(bridge.ensure).toHaveBeenCalledWith("sess-1"));
|
||||||
|
// Simulate the real IPC flow: after ensure, a navigate call sends a nav
|
||||||
|
// state with a URL so the positioning effect considers the view visible.
|
||||||
|
act(() =>
|
||||||
|
bridge.emit({
|
||||||
|
viewId: "42:sess-1",
|
||||||
|
url: "http://localhost:3000/",
|
||||||
|
title: "",
|
||||||
|
canGoBack: false,
|
||||||
|
canGoForward: false,
|
||||||
|
isLoading: false,
|
||||||
|
}),
|
||||||
|
);
|
||||||
act(() => result.current.slotRef(slot));
|
act(() => result.current.slotRef(slot));
|
||||||
|
|
||||||
await waitFor(() =>
|
await waitFor(() =>
|
||||||
|
|
@ -111,6 +123,16 @@ describe("useBrowserView", () => {
|
||||||
|
|
||||||
const { result } = renderHook(() => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false }));
|
const { result } = renderHook(() => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false }));
|
||||||
await waitFor(() => expect(bridge.ensure).toHaveBeenCalledWith("sess-1"));
|
await waitFor(() => expect(bridge.ensure).toHaveBeenCalledWith("sess-1"));
|
||||||
|
act(() =>
|
||||||
|
bridge.emit({
|
||||||
|
viewId: "42:sess-1",
|
||||||
|
url: "http://localhost:3000/",
|
||||||
|
title: "",
|
||||||
|
canGoBack: false,
|
||||||
|
canGoForward: false,
|
||||||
|
isLoading: false,
|
||||||
|
}),
|
||||||
|
);
|
||||||
act(() => result.current.slotRef(slot));
|
act(() => result.current.slotRef(slot));
|
||||||
|
|
||||||
await waitFor(() =>
|
await waitFor(() =>
|
||||||
|
|
@ -140,6 +162,17 @@ describe("useBrowserView", () => {
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
await Promise.resolve();
|
await Promise.resolve();
|
||||||
});
|
});
|
||||||
|
// Simulate a real nav state with URL so the positioning effect shows the view.
|
||||||
|
act(() =>
|
||||||
|
bridge.emit({
|
||||||
|
viewId: "42:sess-1",
|
||||||
|
url: "http://localhost:3000/",
|
||||||
|
title: "",
|
||||||
|
canGoBack: false,
|
||||||
|
canGoForward: false,
|
||||||
|
isLoading: false,
|
||||||
|
}),
|
||||||
|
);
|
||||||
act(() => result.current.slotRef(slot));
|
act(() => result.current.slotRef(slot));
|
||||||
// Flush the mount measure (immediate frame + settle timer).
|
// Flush the mount measure (immediate frame + settle timer).
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
|
|
@ -316,4 +349,27 @@ describe("useBrowserView", () => {
|
||||||
expect(bridge.navigate).not.toHaveBeenCalled();
|
expect(bridge.navigate).not.toHaveBeenCalled();
|
||||||
expect(bridge.clear).not.toHaveBeenCalled();
|
expect(bridge.clear).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("clears the view when the session is terminated, even with an active preview URL", async () => {
|
||||||
|
const bridge = setupBridge();
|
||||||
|
const { rerender } = renderHook(
|
||||||
|
({ terminated }) =>
|
||||||
|
useBrowserView({
|
||||||
|
sessionId: "sess-1",
|
||||||
|
active: true,
|
||||||
|
poppedOut: false,
|
||||||
|
terminated,
|
||||||
|
previewUrl: "http://localhost:5173/",
|
||||||
|
previewRevision: 1,
|
||||||
|
}),
|
||||||
|
{ initialProps: { terminated: false } },
|
||||||
|
);
|
||||||
|
// The preview drives a navigate on mount.
|
||||||
|
await waitFor(() => expect(bridge.navigate).toHaveBeenCalledTimes(1));
|
||||||
|
|
||||||
|
// Terminate the session – the view must be cleared and no re-navigate.
|
||||||
|
rerender({ terminated: true });
|
||||||
|
await waitFor(() => expect(bridge.clear).toHaveBeenCalledWith("42:sess-1"));
|
||||||
|
expect(bridge.navigate).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,12 @@ type UseBrowserViewOptions = {
|
||||||
sessionId: string;
|
sessionId: string;
|
||||||
active: boolean;
|
active: boolean;
|
||||||
poppedOut: boolean;
|
poppedOut: boolean;
|
||||||
|
/**
|
||||||
|
* When true, the view is cleared and the daemon-driven preview is suppressed.
|
||||||
|
* Use when the session is terminated: the old preview content should not
|
||||||
|
* remain visible even if the DB still carries a preview_url.
|
||||||
|
*/
|
||||||
|
terminated?: boolean;
|
||||||
/**
|
/**
|
||||||
* Preview target driven by the daemon (via `ao preview`, streamed over CDC).
|
* Preview target driven by the daemon (via `ao preview`, streamed over CDC).
|
||||||
* When set, the view navigates here automatically; an empty value clears it.
|
* When set, the view navigates here automatically; an empty value clears it.
|
||||||
|
|
@ -68,6 +74,7 @@ export function useBrowserView({
|
||||||
sessionId,
|
sessionId,
|
||||||
active,
|
active,
|
||||||
poppedOut,
|
poppedOut,
|
||||||
|
terminated,
|
||||||
previewUrl,
|
previewUrl,
|
||||||
previewRevision,
|
previewRevision,
|
||||||
}: UseBrowserViewOptions): BrowserViewModel {
|
}: UseBrowserViewOptions): BrowserViewModel {
|
||||||
|
|
@ -80,11 +87,16 @@ export function useBrowserView({
|
||||||
const settleTimerRef = useRef<number | null>(null);
|
const settleTimerRef = useRef<number | null>(null);
|
||||||
const observerRef = useRef<ResizeObserver | null>(null);
|
const observerRef = useRef<ResizeObserver | null>(null);
|
||||||
const previewTriggerRef = useRef<{ revision: number | null; target: string } | null>(null);
|
const previewTriggerRef = useRef<{ revision: number | null; target: string } | null>(null);
|
||||||
|
const hasUrlRef = useRef(false);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
activeRef.current = active;
|
activeRef.current = active;
|
||||||
}, [active]);
|
}, [active]);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
hasUrlRef.current = Boolean(navState.url);
|
||||||
|
}, [navState.url]);
|
||||||
|
|
||||||
const sendHiddenBounds = useCallback((id = viewIdRef.current) => {
|
const sendHiddenBounds = useCallback((id = viewIdRef.current) => {
|
||||||
if (!id) return;
|
if (!id) return;
|
||||||
window.ao?.browser.setBounds({ viewId: id, rect: HIDDEN_RECT, visible: false });
|
window.ao?.browser.setBounds({ viewId: id, rect: HIDDEN_RECT, visible: false });
|
||||||
|
|
@ -95,7 +107,7 @@ export function useBrowserView({
|
||||||
const id = viewIdRef.current;
|
const id = viewIdRef.current;
|
||||||
const node = slotNodeRef.current;
|
const node = slotNodeRef.current;
|
||||||
if (!id) return;
|
if (!id) return;
|
||||||
if (!activeRef.current || !node || !node.isConnected) {
|
if (!activeRef.current || !node || !node.isConnected || !hasUrlRef.current) {
|
||||||
sendHiddenBounds(id);
|
sendHiddenBounds(id);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -189,12 +201,12 @@ export function useBrowserView({
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (active) {
|
if (navState.url && active) {
|
||||||
scheduleSettleMeasure();
|
scheduleSettleMeasure();
|
||||||
} else {
|
} else {
|
||||||
sendHiddenBounds();
|
sendHiddenBounds();
|
||||||
}
|
}
|
||||||
}, [active, poppedOut, scheduleSettleMeasure, sendHiddenBounds]);
|
}, [active, navState.url, poppedOut, scheduleSettleMeasure, sendHiddenBounds]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const handle = () => scheduleMeasure();
|
const handle = () => scheduleMeasure();
|
||||||
|
|
@ -223,11 +235,18 @@ export function useBrowserView({
|
||||||
|
|
||||||
const clear = useCallback(() => withView((id) => window.ao!.browser.clear(id)), [withView]);
|
const clear = useCallback(() => withView((id) => window.ao!.browser.clear(id)), [withView]);
|
||||||
|
|
||||||
|
// When the session is terminated, clear the view and stop reacting to
|
||||||
|
// daemon-driven preview changes so stale content does not remain visible.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!terminated) return;
|
||||||
|
void clear();
|
||||||
|
}, [clear, terminated]);
|
||||||
|
|
||||||
// Drive the view from the daemon-set preview target. Current daemons key
|
// Drive the view from the daemon-set preview target. Current daemons key
|
||||||
// this on previewRevision (bumped on every `ao preview` call); older daemons
|
// this on previewRevision (bumped on every `ao preview` call); older daemons
|
||||||
// did not send it, so fall back to URL changes for compatibility.
|
// did not send it, so fall back to URL changes for compatibility.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!viewId) return;
|
if (!viewId || terminated) return;
|
||||||
const target = previewUrl?.trim() ?? "";
|
const target = previewUrl?.trim() ?? "";
|
||||||
const revision = typeof previewRevision === "number" ? previewRevision : null;
|
const revision = typeof previewRevision === "number" ? previewRevision : null;
|
||||||
const previous = previewTriggerRef.current;
|
const previous = previewTriggerRef.current;
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue