fix: stop the chat view hanging on "Loading conversations…" after a snip

The hand-off launch deadlocked its own startup. `resumed` was initialised to
`Boolean(initialSession)`, which is false for a snip, and the idle-resume
effect returned early whenever there was a hand-off, so nothing ever set it.
The render gate needed both `resumed` and `prepared`, so a successful snip
left the view spinning and there was no way to reach the composer.

Two effects guarding two booleans had already produced one bug here, so this
fixes the shape rather than the value:

- One startup effect and one `startup` flag, cleared in a `finally`. Every
  branch ends in the same place, including a thrown one, so no path can leave
  the view loading.
- The decision tree moves to `lib/startup.ts` as a pure `resolveStartup`,
  which takes its storage and file reads as injected ports. It is now covered
  by 16 tests, including the case that broke: a hand-off must not fall through
  to the idle resume, and must still finish startup.
- `startup.ts` joins the guard that keeps the logic layer free of Vicinae
  imports.
- ChatView is prettier-clean; it had been carrying pre-existing drift and has
  been rewritten twice this session anyway.

Verified: 273 tests, tsc, eslint, prettier, and vici build.
This commit is contained in:
2026-10-04 15:34:09 +08:00
parent ada98968ac
commit e00b49bbe5
4 changed files with 345 additions and 73 deletions
+11 -1
View File
@@ -165,7 +165,17 @@ describe("module wiring", () => {
// These modules must stay renderer-free: they are pure logic, they are what // These modules must stay renderer-free: they are pure logic, they are what
// the unit tests cover, and nothing in them should need a Vicinae runtime. // the unit tests cover, and nothing in them should need a Vicinae runtime.
// (`sessions.ts` is deliberately excluded: it legitimately needs LocalStorage.) // (`sessions.ts` is deliberately excluded: it legitimately needs LocalStorage.)
for (const file of ["agent.ts", "ollama.ts", "tools.ts", "location.ts", "history.ts", "markdown.ts"]) { for (const file of [
"agent.ts",
"ollama.ts",
"tools.ts",
"location.ts",
"history.ts",
"markdown.ts",
"capture.ts",
"attachments.ts",
"startup.ts",
]) {
const source = readFileSync(path.resolve(__dirname, `../lib/${file}`), "utf8"); const source = readFileSync(path.resolve(__dirname, `../lib/${file}`), "utf8");
expect(source, `${file} should not import @vicinae/api`).not.toMatch(/from\s+"@vicinae\/api"/); expect(source, `${file} should not import @vicinae/api`).not.toMatch(/from\s+"@vicinae\/api"/);
} }
+203
View File
@@ -0,0 +1,203 @@
import { describe, expect, it, vi } from "vitest";
import { isHandoff, resolveStartup, type StartupPorts } from "../lib/startup";
import type { PendingAttachment, Session, SessionSummary } from "../lib/types";
/**
* The chat view's startup is the part that kept breaking: three asynchronous
* paths (a pushed session, a handed-over capture, an idle resume) that all have
* to end in a rendered chat. Two of those bugs shipped, so the decision tree is
* tested here rather than discovered from a screenshot of a spinner.
*/
const NOW = 1_000_000;
const HOUR = 3_600_000;
function session(id: string, updatedAt: number): Session {
return { id, title: id, model: "m", createdAt: updatedAt, updatedAt, turns: [] };
}
function summary(id: string, updatedAt: number): SessionSummary {
return { id, title: id, model: "m", createdAt: updatedAt, updatedAt, turnCount: 0, preview: "" };
}
function attachment(path: string): PendingAttachment {
return { path, mime: "image/png", base64: "AAAA" };
}
/** Ports that record what was asked for, so "did not resume" is checkable. */
function ports(overrides: Partial<StartupPorts> = {}) {
const loadSession = vi.fn(async (id: string) => session(id, NOW));
const listSessions = vi.fn(async () => [summary("latest", NOW)]);
const readAttachment = vi.fn(async (path: string) => attachment(path));
return {
loadSession,
listSessions,
readAttachment,
...overrides,
} satisfies StartupPorts;
}
function input(overrides: Partial<Parameters<typeof resolveStartup>[0]> = {}) {
return {
hasInitialSession: false,
now: NOW,
...overrides,
};
}
describe("isHandoff", () => {
it("is true when a file was handed over", () => {
expect(isHandoff({ initialAttachmentPath: "/tmp/snip.png" })).toBe(true);
});
it("is true when a conversation was named", () => {
expect(isHandoff({ initialSessionId: "abc" })).toBe(true);
});
it("is false on an ordinary open", () => {
expect(isHandoff({})).toBe(false);
});
});
describe("resolveStartup", () => {
describe("a snip hand-off", () => {
it("reads the attachment and restores the named conversation", async () => {
const p = ports();
const result = await resolveStartup(
input({ initialSessionId: "abc", initialAttachmentPath: "/tmp/snip.png" }),
p,
);
expect(result.session?.id).toBe("abc");
expect(result.attachment?.path).toBe("/tmp/snip.png");
});
it("does not fall through to the idle resume", async () => {
// The bug this file exists for: a hand-off has to finish startup even
// though it took the resume path off the table. Guarding the resume with
// "and there was no hand-off" while forgetting to clear the readiness flag
// left the view on its loading row forever.
const p = ports();
await resolveStartup(input({ initialAttachmentPath: "/tmp/snip.png" }), p);
expect(p.listSessions).not.toHaveBeenCalled();
});
it("does not attach the most recent conversation's session", async () => {
const p = ports();
const result = await resolveStartup(input({ initialAttachmentPath: "/tmp/snip.png" }), p);
expect(result.session).toBeUndefined();
});
it("still resolves when the named conversation has been deleted", async () => {
// A snip started from a conversation someone has since deleted should mean
// a fresh chat with the image, not a spinner or a crash.
const p = ports({ loadSession: vi.fn(async () => undefined) });
const result = await resolveStartup(
input({ initialSessionId: "gone", initialAttachmentPath: "/tmp/snip.png" }),
p,
);
expect(result.session).toBeUndefined();
expect(result.attachment?.path).toBe("/tmp/snip.png");
});
it("reports an unreadable attachment without refusing to open", async () => {
const p = ports({
readAttachment: vi.fn(async () => {
throw new Error("ENOENT: no such file");
}),
});
const result = await resolveStartup(input({ initialAttachmentPath: "/tmp/gone.png" }), p);
expect(result.attachment).toBeUndefined();
expect(result.attachmentError).toContain("ENOENT");
});
it("survives a storage failure while loading the named conversation", async () => {
const p = ports({
loadSession: vi.fn(async () => {
throw new Error("storage exploded");
}),
});
const result = await resolveStartup(input({ initialSessionId: "abc" }), p);
expect(result.session).toBeUndefined();
});
});
describe("an ordinary open", () => {
it("resumes the most recent conversation when it is fresh", async () => {
const p = ports({ listSessions: vi.fn(async () => [summary("latest", NOW - 60_000)]) });
const result = await resolveStartup(input({ resumeTimeoutMs: HOUR }), p);
expect(result.session?.id).toBe("latest");
expect(p.loadSession).toHaveBeenCalledWith("latest");
});
it("starts fresh when the newest conversation is older than the window", async () => {
const p = ports({ listSessions: vi.fn(async () => [summary("latest", NOW - 2 * HOUR)]) });
const result = await resolveStartup(input({ resumeTimeoutMs: HOUR }), p);
expect(result.session).toBeUndefined();
expect(p.loadSession).not.toHaveBeenCalled();
});
it("treats exactly the window as fresh", async () => {
const p = ports({ listSessions: vi.fn(async () => [summary("latest", NOW - HOUR)]) });
const result = await resolveStartup(input({ resumeTimeoutMs: HOUR }), p);
expect(result.session?.id).toBe("latest");
});
it("always resumes when the preference is 'never time out'", async () => {
const p = ports({ listSessions: vi.fn(async () => [summary("latest", NOW - 400 * HOUR)]) });
const result = await resolveStartup(input({ resumeTimeoutMs: undefined }), p);
expect(result.session?.id).toBe("latest");
});
it("starts fresh when there are no conversations at all", async () => {
const p = ports({ listSessions: vi.fn(async () => []) });
const result = await resolveStartup(input({ resumeTimeoutMs: HOUR }), p);
expect(result).toEqual({});
expect(p.loadSession).not.toHaveBeenCalled();
});
it("starts fresh rather than failing when the index cannot be read", async () => {
const p = ports({
listSessions: vi.fn(async () => {
throw new Error("corrupt index");
}),
});
const result = await resolveStartup(input({ resumeTimeoutMs: HOUR }), p);
expect(result).toEqual({});
});
it("does not resume when a session was pushed in by hand", async () => {
// The sessions command already knows which conversation to show.
const p = ports();
await resolveStartup(input({ hasInitialSession: true }), p);
expect(p.listSessions).not.toHaveBeenCalled();
expect(p.loadSession).not.toHaveBeenCalled();
});
});
});
+36 -73
View File
@@ -6,9 +6,10 @@ import { useVisionSupport } from "../hooks/useVision";
import { buildThreadMarkdown, type TranscriptOptions } from "../markdown"; import { buildThreadMarkdown, type TranscriptOptions } from "../markdown";
import { getLamPreferences } from "../preferences"; import { getLamPreferences } from "../preferences";
import { deleteSession, listSessions, loadSession, summarizeSession } from "../sessions"; import { deleteSession, listSessions, loadSession, summarizeSession } from "../sessions";
import { resolveStartup } from "../startup";
import { buildThread } from "../thread"; import { buildThread } from "../thread";
import type { PendingAttachment, Session, SessionSummary } from "../types"; import type { PendingAttachment, Session, SessionSummary } from "../types";
import { errorMessage, recencyGroup, truncate } from "../utils"; import { recencyGroup, truncate } from "../utils";
/** /**
* The chat surface: conversations on the left, the open conversation on the right. * The chat surface: conversations on the left, the open conversation on the right.
@@ -92,13 +93,10 @@ export function ChatView({
const { replaceSession } = chat; const { replaceSession } = chat;
const { canSeeImages } = useVisionSupport(preferences); const { canSeeImages } = useVisionSupport(preferences);
/** A launch that hands something over has to wait for it before it can render. */
const needsPreparation = Boolean(initialSessionId) || Boolean(initialAttachmentPath);
const [input, setInput] = useState(initialPrompt ?? ""); const [input, setInput] = useState(initialPrompt ?? "");
const [summaries, setSummaries] = useState<SessionSummary[] | undefined>(undefined); const [summaries, setSummaries] = useState<SessionSummary[] | undefined>(undefined);
const [prepared, setPrepared] = useState(!needsPreparation); // One flag, not two. See the startup effect below for why that matters.
const [resumed, setResumed] = useState(Boolean(initialSession)); const [startup, setStartup] = useState<"loading" | "ready">(initialSession ? "ready" : "loading");
// A captured image is held here until the user says what they want to know // A captured image is held here until the user says what they want to know
// about it. Nothing is sent to the model on capture alone. // about it. Nothing is sent to the model on capture alone.
const [pending, setPending] = useState<PendingAttachment | undefined>(undefined); const [pending, setPending] = useState<PendingAttachment | undefined>(undefined);
@@ -118,76 +116,47 @@ export function ChatView({
}, [reload, chat.session.updatedAt]); }, [reload, chat.session.updatedAt]);
/** /**
* Restores whatever the launching command handed over. * Everything that has to happen before the first render, in one place.
* *
* This is the second half of the snip handoff: the capture command closed the * This used to be two effects guarding two booleans, and a hand-off launch
* window to take the shot, so it comes back here with a file to read and, when * satisfied the guard of *both* while setting neither, so `resumed` stayed
* the snip was started from a conversation, the id of the conversation to * false and the view sat on "Loading conversations…" forever. Anything that
* return to. Reloading by id rather than trusting "most recent" matters * blocks the first render belongs in one effect with one flag, and the flag is
* because the snip itself is not a turn, so nothing was saved in between. * cleared in a `finally` so that no branch, including a thrown one, can leave
* the view spinning.
*/ */
useEffect(() => { useEffect(() => {
if (!needsPreparation) return;
let cancelled = false; let cancelled = false;
void (async () => { void (async () => {
if (initialSessionId) {
const session = await loadSession(initialSessionId);
if (!cancelled && session) replaceSession(session);
}
if (initialAttachmentPath) {
try { try {
const attachment = await readAttachment(initialAttachmentPath); const outcome = await resolveStartup(
if (!cancelled) setPending(attachment); {
} catch (thrown) { hasInitialSession: Boolean(initialSession),
if (!cancelled) setSnipNote(`**Could not open the screenshot:** ${errorMessage(thrown)}`); initialSessionId,
initialAttachmentPath,
resumeTimeoutMs: preferences.resumeTimeoutMs,
now: Date.now(),
},
{ loadSession, listSessions, readAttachment },
);
if (cancelled) return;
if (outcome.session) replaceSession(outcome.session);
if (outcome.attachment) setPending(outcome.attachment);
if (outcome.attachmentError) {
setSnipNote(`**Could not open the screenshot:** ${outcome.attachmentError}`);
} }
}
if (!cancelled) setPrepared(true);
})();
return () => {
cancelled = true;
};
}, [needsPreparation, initialSessionId, initialAttachmentPath, replaceSession]);
/**
* Reopening the command continues the most recent conversation, but only if it
* was used recently. Past the configured idle window it starts a fresh chat
* instead. The previous conversation is never removed, it just drops down the
* sidebar. Setting the preference to "Never" keeps the last chat open forever.
*
* A hand-off launch opts out: it either names the conversation to return to or
* deliberately starts a new one, and quietly continuing some other chat would
* bury the screenshot in an unrelated thread.
*/
useEffect(() => {
if (initialSession || needsPreparation) return;
let cancelled = false;
void (async () => {
try {
const [latest] = await listSessions();
if (!latest) return;
const idleMs = Date.now() - latest.updatedAt;
const { resumeTimeoutMs } = preferences;
if (resumeTimeoutMs !== undefined && idleMs > resumeTimeoutMs) return;
const session = await loadSession(latest.id);
if (!cancelled && session) replaceSession(session);
} finally { } finally {
if (!cancelled) setResumed(true); if (!cancelled) setStartup("ready");
} }
})(); })();
return () => { return () => {
cancelled = true; cancelled = true;
}; };
}, [initialSession, needsPreparation, preferences, replaceSession]); }, [initialSession, initialSessionId, initialAttachmentPath, preferences.resumeTimeoutMs, replaceSession]);
const thread = useMemo( const thread = useMemo(
() => () =>
@@ -213,8 +182,8 @@ export function ChatView({
* already looking. * already looking.
*/ */
const threadMarkdown = useMemo(() => { const threadMarkdown = useMemo(() => {
const notes = [launchNote, snipNote, pending ? PENDING_NOTE : undefined].filter( const notes = [launchNote, snipNote, pending ? PENDING_NOTE : undefined].filter((note): note is string =>
(note): note is string => Boolean(note), Boolean(note),
); );
const transcript = const transcript =
@@ -237,10 +206,7 @@ export function ChatView({
if (base.some((summary) => summary.id === activeId)) return mapped; if (base.some((summary) => summary.id === activeId)) return mapped;
const active = summarizeSession(chat.session); const active = summarizeSession(chat.session);
return [ return [{ id: active.id, title: active.title, updatedAt: active.updatedAt, isActive: true }, ...mapped];
{ id: active.id, title: active.title, updatedAt: active.updatedAt, isActive: true },
...mapped,
];
}, [summaries, activeId, chat.session]); }, [summaries, activeId, chat.session]);
/** /**
@@ -282,8 +248,7 @@ export function ChatView({
/** The snip a "show me the screenshot" action would open: the pending one, else the newest. */ /** The snip a "show me the screenshot" action would open: the pending one, else the newest. */
const latestAttachment = const latestAttachment =
pending ?? pending ?? [...chat.session.turns].reverse().find((turn) => turn.attachments.length > 0)?.attachments[0];
[...chat.session.turns].reverse().find((turn) => turn.attachments.length > 0)?.attachments[0];
const send = useCallback( const send = useCallback(
(text: string) => { (text: string) => {
@@ -382,13 +347,11 @@ export function ChatView({
); );
}; };
if (!resumed || !prepared || summaries === undefined) { if (startup === "loading" || summaries === undefined) {
return <List isLoading navigationTitle="Lam" searchBarPlaceholder="Loading conversations…" />; return <List isLoading navigationTitle="Lam" searchBarPlaceholder="Loading conversations…" />;
} }
const placeholder = chat.status const placeholder = chat.status ? `${chat.status}. Press ⏎ to stop` : "Lam is thinking. Press ⏎ to stop";
? `${chat.status}. Press ⏎ to stop`
: "Lam is thinking. Press ⏎ to stop";
const idlePlaceholder = pending ? "Ask about the screenshot…" : "Ask Lam anything…"; const idlePlaceholder = pending ? "Ask about the screenshot…" : "Ask Lam anything…";
+96
View File
@@ -0,0 +1,96 @@
import type { PendingAttachment, Session, SessionSummary } from "./types";
import { errorMessage } from "./utils";
/**
* Everything the chat view resolves before its first render.
*
* Pulled out of the component and made pure because this is the part that keeps
* going wrong. Three things can happen on open, and all of them are
* asynchronous: a launch hands over a captured file, it may also name the
* conversation to return to, and an ordinary open resumes the most recent
* conversation if it is fresh enough. Every path has to end in a rendered chat.
*
* As a plain function it is testable without mounting anything, which matters
* because the alternative is finding out from a screenshot that a view is
* spinning forever.
*
* Imports nothing from Vicinae, so it belongs to the logic layer.
*/
export interface StartupInput {
/** A conversation already in hand, which is how the sessions command pushes this view. */
hasInitialSession: boolean;
/** A conversation to reopen by id, handed over by the snip command. */
initialSessionId?: string;
/** A captured image to read in. */
initialAttachmentPath?: string;
/** Idle time past which an ordinary open starts fresh. Undefined means never time out. */
resumeTimeoutMs?: number;
/** Injected so the resume window is deterministic under test. */
now: number;
}
/** The side effects, injected so nothing here touches storage or the file system directly. */
export interface StartupPorts {
loadSession: (id: string) => Promise<Session | undefined>;
listSessions: () => Promise<SessionSummary[]>;
readAttachment: (path: string) => Promise<PendingAttachment>;
}
export interface StartupResult {
/** The conversation to show, when one was restored. */
session?: Session;
/** The image that was handed over, when one could be read. */
attachment?: PendingAttachment;
/** Why a handed-over image could not be read, so the view can say so. */
attachmentError?: string;
}
/** True when the launch handed something over, which also means "do not resume". */
export function isHandoff(input: Pick<StartupInput, "initialSessionId" | "initialAttachmentPath">): boolean {
return Boolean(input.initialSessionId || input.initialAttachmentPath);
}
export async function resolveStartup(input: StartupInput, ports: StartupPorts): Promise<StartupResult> {
const result: StartupResult = {};
if (input.initialSessionId) {
// A hand-off names the conversation to return to, so that one is restored
// rather than whichever chat happens to be most recent. By id matters
// because taking a snip is not a turn, so nothing was saved in between.
// A session that has since been deleted just means starting fresh.
result.session = await ports.loadSession(input.initialSessionId).catch(() => undefined);
}
if (input.initialAttachmentPath) {
try {
result.attachment = await ports.readAttachment(input.initialAttachmentPath);
} catch (thrown) {
// A missing file leaves an empty chat with an explanation, which beats
// refusing to open at all.
result.attachmentError = errorMessage(thrown);
}
}
// Nothing was handed over, so the idle-resume rule applies: reopening the
// command continues the most recent conversation, but only if it was used
// recently. The previous conversation is never removed either way, it just
// drops down the sidebar.
if (!input.hasInitialSession && !isHandoff(input)) {
const latest = await ports
.listSessions()
.then((sessions) => sessions[0])
.catch(() => undefined);
if (latest) {
const idleMs = input.now - latest.updatedAt;
const fresh = input.resumeTimeoutMs === undefined || idleMs <= input.resumeTimeoutMs;
if (fresh) {
result.session = await ports.loadSession(latest.id).catch(() => undefined);
}
}
}
return result;
}