Skip to content
Open
12 changes: 9 additions & 3 deletions apps/web/src/components/DesignSystemFlow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ import { DesignSystemPicker } from './DesignSystemPicker';
import { LibraryPicker } from './LibraryPicker';
import { notifyConnectorsChanged } from './connectors-events';
import { connectorAuthSnapshotChanged } from './connectors-state';
import { FileWorkspace, type FileRefreshResult } from './FileWorkspace';
import {
FileWorkspace,
type FileRefreshResult,
type WorkspaceOpenRequest,
} from './FileWorkspace';
import { Icon, type IconName } from './Icon';
import { Spinner } from './Loading';
import { Toast } from './Toast';
Expand Down Expand Up @@ -1706,7 +1710,7 @@ export function DesignSystemDetailView({
tabs: [],
active: null,
});
const [workspaceOpenRequest, setWorkspaceOpenRequest] = useState<{ name: string; nonce: number } | null>(null);
const [workspaceOpenRequest, setWorkspaceOpenRequest] = useState<WorkspaceOpenRequest | null>(null);
const chatAbortRef = useRef<AbortController | null>(null);
const chatCancelRef = useRef<AbortController | null>(null);
const pendingWorkspaceFileWritesRef = useRef<Map<string, string>>(new Map());
Expand Down Expand Up @@ -2327,7 +2331,9 @@ export function DesignSystemDetailView({

const requestWorkspaceFileOpen = useCallback((name: string) => {
if (!name) return;
setWorkspaceOpenRequest({ name, nonce: Date.now() });
// This flow has no post-turn auto-open watch; every open here is a click in
// the design-system workspace, so it is reported as the user's.
setWorkspaceOpenRequest({ name, nonce: Date.now(), source: 'user' });
}, []);

// Known-file set for the design-system chat's file-link routing — same
Expand Down
146 changes: 130 additions & 16 deletions apps/web/src/components/FileWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,28 @@ export async function settleManualEditFiles(
return results.every(Boolean);
}

// Who asked for an open. `openRequest` started life as the chat file-chip
// click (see ProjectView's declaration) and later picked up the agent-driven
// auto-opens, so the request alone cannot say whether the user chose this file
// or a run did. Required rather than optional: a caller that forgets it would
// silently reclassify a user's click, which is the failure this exists to stop.
export type WorkspaceOpenRequestSource = 'user' | 'internal';

export interface WorkspaceOpenRequest {
name: string;
nonce: number;
source: WorkspaceOpenRequestSource;
// Re-asked at the moment the activation actually runs, not when the request
// is made. An activation can be parked behind an unsettled manual edit for an
// unbounded time (see `afterActiveManualEditSettles`), and the requester's own
// checks have long since passed by then — so whatever made this request the
// right one to honour has to stay askable. Only the requester knows what that
// is, which is why this is a predicate rather than data: 'internal' requests
// carry their run's ownership, and a user's click carries nothing because a
// user's choice cannot go stale.
isStillOwned?: () => boolean;
}

interface Props {
projectId: string;
projectKind: TrackingProjectKind;
Expand All @@ -225,7 +247,7 @@ interface Props {
streaming?: boolean;
commentQueueOnSend?: boolean;
commentSendDisabled?: boolean;
openRequest?: { name: string; nonce: number } | null;
openRequest?: WorkspaceOpenRequest | null;
browserOpenRequest?: BrowserOpenRequest | null;
// Browser tab whose <webview> must stay mounted even while another workspace
// tab is active. Set for programmatic brand extraction: the chat "Continue
Expand All @@ -249,6 +271,13 @@ interface Props {
// daemon's SQLite store can hold the source of truth and survive reloads.
tabsState: OpenTabsState;
onTabsStateChange: (next: OpenTabsState) => void;
// Fired once for every activation the USER makes in here that the parent did
// not ask for. `onTabsStateChange` cannot serve as that signal: transient tabs
// (an unsaved sketch) are activated locally and never round-trip through the
// persisted state, and a persisted change carries no clue about who caused it.
// ProjectView's post-turn auto-open watch needs both — see
// `AutoOpenSettleRequest.userActivationsAtTurnEnd`.
onUserActivateTab?: () => void;
previewComments?: PreviewComment[];
onSavePreviewComment?: (target: PreviewCommentTarget, note: string, attachAfterSave: boolean, images?: File[], commentId?: string) => Promise<PreviewComment | null>;
onRemovePreviewComment?: (commentId: string) => Promise<boolean>;
Expand Down Expand Up @@ -1295,6 +1324,7 @@ export function FileWorkspace({
designSystemActivityEvents = [],
tabsState,
onTabsStateChange,
onUserActivateTab,
previewComments = NO_PREVIEW_COMMENTS,
onSavePreviewComment,
onRemovePreviewComment,
Expand Down Expand Up @@ -1572,11 +1602,35 @@ export function FileWorkspace({
};
}, [protectedHtmlViewerFileNames.size, settleProtectedManualEdits]);

function afterActiveManualEditSettles(action: () => void) {
// Every activation in here funnels through this gate, and when a manual edit
// is open the action it wraps does not run until that edit has flushed — which
// can be after ProjectView's settle watcher has already made its decision. So
// the user's intent is reported HERE, at the gesture, rather than waiting for
// the `activeTab` effect below to observe an activation that may land too late
// (or, if the edit fails to settle, never). `origin` defaults to 'user' so a
// new activation site added later is counted rather than silently dropped;
// only the parent-driven entry points opt out, since counting one of those
// would retire the very watch it belongs to.
function afterActiveManualEditSettles(
action: () => void,
origin: WorkspaceOpenRequestSource = 'user',
isStillOwned?: () => boolean,
) {
// Over-reporting is the safe direction and is deliberate here: this can
// double-count with the `activeTab` effect when the activation lands
// promptly. The count is only ever compared for equality, so an extra
// increment can retire a watch early — never move focus off the user's tab.
if (origin === 'user') onUserActivateTab?.();
// The owner is re-checked HERE rather than at the callsite, because this is
// the only place that knows whether the action ran immediately or waited.
const run = () => {
if (isStillOwned && !isStillOwned()) return;
action();
};
const sourceTab = activeTabRef.current;
const exit = manualEditExitHandlersRef.current.get(sourceTab);
if (!exit) {
action();
run();
return;
}
const sequence = ++requestedActivationSequenceRef.current;
Expand All @@ -1586,7 +1640,7 @@ export function FileWorkspace({
ok
&& sequence === requestedActivationSequenceRef.current
&& activeTabRef.current === sourceTab
) action();
) run();
});
}

Expand Down Expand Up @@ -1725,13 +1779,46 @@ export function FileWorkspace({
&& liveArtifactEntries.length === 0
&& projectFolders.length === 0;

// The activation the parent asked for and we have not reported yet. Set by the
// two places an activation originates OUTSIDE this component — the persisted
// `tabsState.active` hydration below and the `openRequest` effect — so the
// reporter can tell those apart from a gesture made in here. Anything not
// marked counts as the user's, which is the safe default: over-reporting only
// retires an auto-open watch early, while under-reporting would let it move
// focus away from a tab the user picked.
//
// Not the whole story on its own: `openRequest` carries the user's chat
// file-link clicks as well as the parent's auto-opens, and this ref cannot
// tell those apart. That distinction is the request's `source`, and it is
// applied at the settle gate, which is also early enough that an open manual
// edit cannot delay the report past the watcher's deadline.
const parentRequestedActivationRef = useRef<string | null>(null);
const reportedActiveTabRef = useRef(activeTab);

// Report user activations upward. One effect on `activeTab` rather than a call
// at each activation site: there are twenty-odd of those, and a new one added
// later would silently go unreported.
useEffect(() => {
if (reportedActiveTabRef.current === activeTab) return;
reportedActiveTabRef.current = activeTab;
const parentRequested = parentRequestedActivationRef.current;
// Cleared either way: a request that never changed the active tab (openFile
// short-circuits when it is already active) must not be left behind to
// swallow a later activation of the same name.
parentRequestedActivationRef.current = null;
if (parentRequested === activeTab) return;
onUserActivateTab?.();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeTab]);

// Pull the persisted active tab in when the parent's hydration completes
// (or on project switch). Fall back to the Design Files browser so a
// fresh project lands in a useful place.
useEffect(() => {
const nextActive = tabsState.active ?? defaultRootTab;
if (nextActive === activeTabRef.current) return;
afterActiveManualEditSettles(() => setActiveTab(nextActive));
parentRequestedActivationRef.current = nextActive;
afterActiveManualEditSettles(() => setActiveTab(nextActive), 'internal');
// afterActiveManualEditSettles reads post-commit refs and intentionally
// remains stable across this externally-driven hydration transition.
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -1798,17 +1885,21 @@ export function FileWorkspace({
onTabsStateChange(next);
}

function setPersistedActive(name: string | null) {
function setPersistedActive(
name: string | null,
origin: WorkspaceOpenRequestSource = 'user',
isStillOwned?: () => boolean,
) {
const nextActive = name ?? defaultRootTab;
if (nextActive === activeTab) return;
afterActiveManualEditSettles(() => {
setActiveTab(nextActive);
commitTabsState(workspaceTabsState(persistedTabs, name));
});
}, origin, isStillOwned);
}

function openRequestedBrowserTab(request: BrowserOpenRequest) {
afterActiveManualEditSettles(() => commitRequestedBrowserTab(request));
afterActiveManualEditSettles(() => commitRequestedBrowserTab(request), 'internal');
}

function commitRequestedBrowserTab(request: BrowserOpenRequest) {
Expand Down Expand Up @@ -1988,15 +2079,15 @@ export function FileWorkspace({
}
if (sketches[activeTab] && !sketches[activeTab]!.persisted) return;
if (!latestPersistedTabs.includes(activeTab)) {
setPersistedActive(latestPersistedTabs[latestPersistedTabs.length - 1] ?? null);
setPersistedActive(latestPersistedTabs[latestPersistedTabs.length - 1] ?? null, 'internal');
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [persistedTabs, activeTab]);

useEffect(() => {
if (!designSystemEditRequest) return;
setUploadError(null);
setPersistedActive(designSystemProject ? DESIGN_SYSTEM_TAB : DESIGN_FILES_TAB);
setPersistedActive(designSystemProject ? DESIGN_SYSTEM_TAB : DESIGN_FILES_TAB, 'internal');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [designSystemEditRequest?.nonce]);

Expand All @@ -2007,19 +2098,35 @@ export function FileWorkspace({
if (!openRequest) return;
const name = openRequest.name;
if (!name) return;
// This prop carries both kinds of open. A chat file-link or produced-file
// chip click arrives here exactly like a run's auto-open, so only the
// request's own `source` can tell them apart — marking on arrival would
// classify the user's click as the parent's and leave the settle watcher
// free to open a higher-ranked artifact over it.
const origin = openRequest.source;
const isStillOwned = openRequest.isStillOwned;
// Still marked for BOTH sources, so the reporter above never fires off the
// landed activation for a request that came through this prop: a run's own
// auto-open must not retire the watch that issued it, and a user-sourced one
// is reported at the gate instead (see `afterActiveManualEditSettles`) so an
// open manual edit cannot hold the signal past the watcher's deadline. The
// gate is where `origin` does its work.
if (name === DESIGN_FILES_TAB || name === DESIGN_SYSTEM_TAB) {
const nextActive =
name === DESIGN_SYSTEM_TAB && !designSystemProject
? DESIGN_FILES_TAB
: name;
setPersistedActive(nextActive);
parentRequestedActivationRef.current = nextActive;
setPersistedActive(nextActive, origin, isStillOwned);
return;
}
if (isBrowserTabId(name) && browserTabs.some((tab) => tab.id === name)) {
setPersistedActive(name);
parentRequestedActivationRef.current = name;
setPersistedActive(name, origin, isStillOwned);
return;
}
openFile(name, { forcePersist: true });
parentRequestedActivationRef.current = name;

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.

Blocking: this marker treats every openRequest as a parent-owned activation, but requestOpenFile is also used for user-originated ChatPane file-link and chip clicks, and the only user signal is emitted later from the activeTab effect. That creates concrete under-reporting paths: a chat click is marked here and never increments the count, while a pending-sketch activation can be held in afterActiveManualEditSettles until after the settle watcher evaluates. In either case ProjectView still sees the baseline persisted focus (or a run-owned name) and can open the later higher-ranked artifact over the user's choice. Carry explicit request-source metadata and mark only run-owned requests, or report user intent before the asynchronous activation gate; add regressions for a ChatPane open while the watcher is pending and for a delayed pending-sketch activation.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

openFile(name, { forcePersist: true, origin, isStillOwned });
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [openRequest]);

Expand Down Expand Up @@ -2078,11 +2185,18 @@ export function FileWorkspace({
afterActiveManualEditSettles(() => {
setSlideNavDeliverableNonce(slideNavRequest!.nonce);
setActiveTab(slideNavRequest!.name);
});
}, 'internal');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [slideNavRequest]);

function openFile(name: string, options?: { forcePersist?: boolean }) {
function openFile(
name: string,
options?: {
forcePersist?: boolean;
origin?: WorkspaceOpenRequestSource;
isStillOwned?: () => boolean;
},
) {
if (name === activeTab) return;
afterActiveManualEditSettles(() => {
setUploadError(null);
Expand All @@ -2101,7 +2215,7 @@ export function FileWorkspace({
if (nextBrowserTabs !== browserTabs) setBrowserTabs(nextBrowserTabs);
commitTabsState(workspaceTabsState(nextTabs, name, nextBrowserTabs));
setActiveTab(name);
});
}, options?.origin ?? 'user', options?.isStillOwned);
}
openFileRef.current = openFile;

Expand Down
Loading
Loading