From 05fd9ff13be569c60ecbc419a218747497aeca9f Mon Sep 17 00:00:00 2001 From: Adrian Bonpin Date: Mon, 5 Oct 2026 09:05:15 +0000 Subject: [PATCH] feat: auto-delete old conversations, and stop a tool call that repeats (#15) --- README.md | 51 ++++- package.json | 33 ++- src/__mocks__/@vicinae/api.ts | 21 ++ src/__tests__/agent.test.ts | 188 ++++++++++++++-- src/__tests__/cleanup.test.ts | 177 +++++++++++++++ src/__tests__/markdown.test.ts | 73 ++++++ src/__tests__/preferences.test.ts | 102 ++++++++- src/__tests__/retention.test.ts | 267 ++++++++++++++++++++++ src/__tests__/sessions.test.ts | 356 +++++++++++++++++++++++++++++- src/__tests__/snip.test.ts | 98 +++++++- src/lib/agent.ts | 116 +++++++++- src/lib/cleanup.ts | 72 ++++++ src/lib/components/ChatView.tsx | 52 ++++- src/lib/constants.ts | 26 ++- src/lib/hooks/useChat.ts | 32 +++ src/lib/markdown.ts | 20 +- src/lib/preferences.ts | 93 +++++++- src/lib/retention.ts | 180 +++++++++++++++ src/lib/schemas.ts | 5 + src/lib/sessions.ts | 202 ++++++++++++++++- src/lib/snip.ts | 86 +++++++- src/lib/types.ts | 21 ++ src/sessions.tsx | 22 +- src/setup.tsx | 127 ++++++++++- vitest.config.mts | 11 + 25 files changed, 2372 insertions(+), 59 deletions(-) create mode 100644 src/__tests__/cleanup.test.ts create mode 100644 src/__tests__/retention.test.ts create mode 100644 src/lib/cleanup.ts create mode 100644 src/lib/retention.ts diff --git a/README.md b/README.md index 1b71be2..f4aa94b 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,8 @@ Hit a global shortcut, type a question, get a streamed answer without leaving wh - **Readable code.** Fenced blocks render in a dark panel with the language named in a header, syntax colouring, and a Copy button. - **Real markdown.** Tables, bulleted and numbered lists, headings, blockquotes, bold, italics and links all render natively, and the default prompt asks for the shape that fits the answer rather than flattening everything into prose. - **Tools.** Lam decides when to call them: current time, location context, web search, and page fetching. A turn that uses up its tool budget still ends in an answer, written from what it gathered, rather than a notice that it stopped. +- **Tool calls you can watch and stop.** What Lam is doing right now stays on screen, including when it asks the same question twice, the round limit is a preference, Force Stop is always one keystroke away, and a call that repeats itself with identical arguments ends the turn instead of running forever. +- **Optional auto-deletion.** Set a retention window and Lam clears out the conversations you have stopped using the next time you open a command, taking their screenshots with them and collecting any capture you never sent. Off by default, and nothing at all is read or written while it is off. The setup screen also has both tidy-ups as explicit actions, confirmed first, for when you would rather press the button yourself. - **Screenshots, two ways.** Snip a region into the conversation you are already in from the action panel, or run **Snip to Lam** to start a new chat about it. The launcher window gets out of the way first, so it never ends up in the shot, and the capture appears in the transcript as a thumbnail with Open Screenshot for the full image. Neither is offered unless the current model can actually see images. - **Chat only.** Lam answers questions. It does not touch your files or run commands. The one exception is capture, and it only ever happens because you pressed the key for it. @@ -113,7 +115,7 @@ vicinae 'vicinae://launch/lam-agent/chat?fallbackText=summarise%20this' | **Chat with Lam** | The chat surface. Bind this to your global shortcut. | | **Snip to Lam** | Capture a region and start a new chat about it. A `no-view` command, so the launcher window closes before the selector appears. Bind this to a second shortcut for one-key "snip and ask". | | **Chat Sessions** | Browse, reopen, and delete past conversations. | -| **Set Up Lam Agent** | Verify the connection, list available models, and check whether the selected model can see images. | +| **Set Up Lam Agent** | Verify the connection, list available models, check whether the selected model can see images, and clear out chats or unattached photos. | ## Configuration @@ -130,6 +132,8 @@ vicinae 'vicinae://launch/lam-agent/chat?fallbackText=summarise%20this' | **Show Tool Calls** | on | Lists the tools Lam called under each answer. | | **Show Reasoning** | on | Includes the model's thinking trace. Turn off to keep replies short. | | **Start New Chat After** | 1 hour | Idle time past which opening the command starts a fresh chat. `Never` always reopens the last one. The old chat is never deleted either way. | +| **Delete Old Chats After** | Disabled | Permanently deletes a conversation once you have not opened it for this long, along with any screenshots it captured. Nothing is ever deleted while this is off. See [Auto-deletion](#auto-deletion). | +| **Tool Call Limit** | 8 rounds | How many rounds of tool calls Lam may make before it has to answer with what it has. `Unlimited` removes the cap. A call that repeats itself with identical arguments ends the turn either way. | ## Tools @@ -160,7 +164,46 @@ This extension does not use GPS. That would mean CoreLocation on macOS, which ha Web search results and fetched pages can be large. Lam truncates them before they reach the model. -**A turn that runs out of tool calls still ends in an answer.** The loop stops after 8 rounds, and the first version of it stopped by throwing away every preamble the model had written and returning a notice about the loop: asked for the fish in every Valheim biome, it researched for five rounds and then said nothing useful at all. The budget is now spent on a *closing round* with the tools taken away and an instruction to answer from what was gathered and to name what it could not confirm. So the worst case is a partial answer that says what is missing, rather than an app-generated message about its own control flow. If the closing round produces nothing either, the last preamble is used, and only then a notice. The closing round is why the cap is 8 rather than 5: the extra round is not more searching, it is the answer. +**A turn that runs out of tool calls still ends in an answer.** The loop stops after 8 rounds by default, and the first version of it stopped by throwing away every preamble the model had written and returning a notice about the loop: asked for the fish in every Valheim biome, it researched for five rounds and then said nothing useful at all. The budget is now spent on a *closing round* with the tools taken away and an instruction to answer from what was gathered and to name what it could not confirm. So the worst case is a partial answer that says what is missing, rather than an app-generated message about its own control flow. If the closing round produces nothing either, the last preamble is used, and only then a notice. The closing round is why the default cap is 8 rather than 5: the extra round is not more searching, it is the answer. + +The cap is a preference rather than a constant, because how many rounds a question needs is a property of the question and of the model. `Unlimited` is there for research that genuinely needs a dozen pages. + +**Two identical calls run, a third refuses to.** No round cap can be a complete guard, because one of the settings is `Unlimited`. What a stuck model actually does is repeat itself: the same tool, the same arguments, the same non-answer. So `agent.ts` counts the tool name together with its sorted arguments, per turn, allows the call twice (a retry after a search that failed is reasonable), and on the third occurrence does not run it. The round finishes, the loop breaks, and the closing round is reached with an instruction that names the repeated call instead of claiming a budget ran out. The refusal is recorded on the turn with `repeated: true`, so the transcript can say what happened. + +Arguments are compared with their keys sorted, because models are not consistent about key order and a call that differs only by key order is still the same call. + +**What Lam is doing is visible while it is doing it, even with tool calls hidden.** `Show Tool Calls` governs the finished transcript, where a list of names adds little. The live activity line is separate and always shown, because during a turn the difference between working and circling is the entire question: `_Reading a page…_` for a first call, and `_Reading a page · the same call again (2)_` once it has come round. Hiding that would leave the user watching a spinner they cannot interpret, which is the situation the repeat guard exists to fix. A call that failed, or that was refused for repeating, is also listed in the transcript even with tool calls hidden: those are the two cases where hiding the line leaves an answer the user cannot account for. + +## Auto-deletion + +Off by default. When it is on, Lam deletes a conversation once you have not used it for the configured period, and deletes the screenshots that belonged to it. + +It is checked when any Lam command opens (chat, sessions, setup) and after every answer, so an expired conversation goes the next time you use Lam. The alternative is a timer, which would mean waking up to do work while nothing is open. + +**Age is counted from the last time you used a conversation, not from when you started it.** A chat started last month and continued this morning is not an old chat, and deleting it because of its birthday would be a surprise. + +**The conversation you are in is never deleted**, even if it is expired. It is about to be rendered, and being the one the command opens by default is the clearest possible sign that it is still in use. The practical consequence is that a conversation you keep returning to outlives the window indefinitely, which is the intended reading of "conversations you have not opened". Sweeping *after* the startup resolution rather than before is what makes this work: sweeping first would delete the very conversation on its way to the screen. + +**A screenshot the user never sent is collected too.** Snips are written to the support directory before the chat view opens, so a capture you abandon belongs to no conversation and no amount of session deletion would ever reach it. The orphan pass reads the snip directory directly, compares it against the paths recorded on the conversation index, and deletes anything unreferenced that is more than a day old. That wait is the safety story: a snip is on disk seconds after it is taken and stays unreferenced until you send it, so without a grace period the sweep would delete the file out from under a question you were still typing. Files belonging to a conversation that has just been deleted go immediately with it instead of waiting, because the conversation they belonged to is already gone. + +The index entry for a conversation carries its image paths (`SessionSummary.attachmentPaths`), so deciding what is still referenced does not mean reading every transcript, which the sweep would otherwise do after every single answer. An index written before that field existed is repaired from its transcripts before anything is deleted, because a missing list is not the same as an empty one: reading it as empty would mark every screenshot in your history as an orphan. + +**The whole design leans towards deleting nothing.** Retention off means nothing is read and nothing is written. An unrecognised value for the preference resolves to off. The orphan pass is skipped entirely when the referenced set cannot be established, because an incomplete set of references looks exactly like a pile of orphans and guessing wrong destroys real images. A path is checked to be inside the snip directory before anything is removed, so a hand-edited entry in storage cannot reach anything else on the disk. And the sweep never throws, so a failed tidy-up cannot break the command you opened. + +### Doing it by hand + +**Set Up Lam Agent** has a Maintenance section with the same two jobs as explicit actions: + +| Action | What it deletes | +| --- | --- | +| **Clear Out All Chats** | Every conversation, and every screenshot they captured. | +| **Clear Out Unattached Photos** | Captures that no conversation refers to any more. | + +Both ask for confirmation first, and both say what they did even when the answer is nothing, because a button that appears to do nothing reads as a bug. That is deliberately the opposite of the background sweep, which stays silent unless it deleted something. + +The photo clean-up applies exactly the same rule as the sweep, including the one-day wait, and it works whether or not **Delete Old Chats After** is switched on. So it covers the case that setting exists for: you would rather decide when your history is cleared than have Lam decide it for you. + +When a conversation cannot be read there is no way to know which captures are still in use, so that action says it could not check and deletes nothing, rather than risking a screenshot a surviving chat still shows. ## Architecture @@ -178,6 +221,8 @@ src/ attachments.ts # image files -> base64 payloads vision.ts # cached "can this model see?" answer sessions.ts # LocalStorage persistence + retention.ts # what gets deleted, and when + cleanup.ts # the sweep + its notification history.ts # turns -> API messages location.ts # time/place, derived locally thread.ts # turns -> flat message list @@ -224,6 +269,8 @@ The rendered height stays capped at 200px whatever is asked for (`Math.min(_rawH **Streamed deltas are buffered in refs and flushed to state on a timer** (`FLUSH_INTERVAL_MS`). Re-rendering the transcript per token makes the launcher visibly janky. +**Stopping a turn is an action, not a keybinding.** The stop action is labelled Force Stop and stays the primary action for the whole turn, because the failure it exists for is a model spending a long time on a question you have given up on. It aborts the request, which propagates through the tool loop and into the tool that is running, so it works mid-search rather than only between rounds. There is no separate stop command: an extension cannot reach into a view running in another command, so a stop command would have to poll a flag in `LocalStorage` and could only stop at round boundaries, which is both slower and less reliable than the thing it would replace. + **The logic layer has no Vicinae imports.** `agent.ts`, `ollama.ts`, `tools.ts` and friends are plain TypeScript, which is why they unit test without a runtime. A test enforces this. **Sessions are capped twice over**, once per turn and once per session, because `LocalStorage` has no documented ceiling. diff --git a/package.json b/package.json index d0c56ef..53b5c38 100644 --- a/package.json +++ b/package.json @@ -125,17 +125,46 @@ { "name": "resumeTimeout", "title": "Start New Chat After", - "description": "When the command opens, resume the last conversation only if it was used recently. Past this much idle time it starts a fresh chat instead. The old conversation is never deleted — it stays in the sidebar.", + "description": "When the command opens, resume the last conversation only if it was used recently. Past this much idle time it starts a fresh chat instead. The old conversation is never deleted. It stays in the sidebar.", "type": "dropdown", "required": false, "default": "1h", "data": [ - { "title": "Never — always reopen the last chat", "value": "never" }, + { "title": "Never (always reopen the last chat)", "value": "never" }, { "title": "After 15 minutes", "value": "15m" }, { "title": "After 1 hour", "value": "1h" }, { "title": "After 6 hours", "value": "6h" }, { "title": "After 1 day", "value": "1d" } ] + }, + { + "name": "autoDeleteAfter", + "title": "Delete Old Chats After", + "description": "Permanently delete a conversation once you have not opened it for this long, along with any screenshots it captured. The check runs when a Lam command opens and after each answer, so conversations go when you next use Lam. Off by default: nothing is ever deleted.", + "type": "dropdown", + "required": false, + "default": "disabled", + "data": [ + { "title": "Disabled (keep conversations forever)", "value": "disabled" }, + { "title": "After 1 day", "value": "1d" }, + { "title": "After 7 days", "value": "7d" }, + { "title": "After 30 days", "value": "30d" }, + { "title": "After 90 days", "value": "90d" } + ] + }, + { + "name": "maxToolRounds", + "title": "Tool Call Limit", + "description": "How many rounds of tool calls Lam may make before it has to answer with what it has gathered. More rounds mean slower but more thorough answers. Unlimited removes the cap; a call that repeats itself with the same arguments three times ends the turn either way, so a stuck model cannot loop forever.", + "type": "dropdown", + "required": false, + "default": "8", + "data": [ + { "title": "4 rounds", "value": "4" }, + { "title": "8 rounds (default)", "value": "8" }, + { "title": "16 rounds", "value": "16" }, + { "title": "Unlimited (no cap)", "value": "unlimited" } + ] } ], "dependencies": { diff --git a/src/__mocks__/@vicinae/api.ts b/src/__mocks__/@vicinae/api.ts index 352d001..418a6dc 100644 --- a/src/__mocks__/@vicinae/api.ts +++ b/src/__mocks__/@vicinae/api.ts @@ -47,6 +47,27 @@ export function getPreferenceValues>(): T { return preferenceOverride as T; } +/** + * Toast stubs. + * + * Recorded rather than discarded, so a test can assert that a sweep only speaks + * up when it actually deleted something. + */ +export const __toasts: { title: string; message?: string }[] = []; + +export const Toast = { + Style: { Success: "success", Failure: "failure", Animated: "animated" }, +}; + +export async function showToast(options: { title: string; message?: string }): Promise { + __toasts.push({ title: options.title, message: options.message }); +} + +/** Test-only helper: forgets the toasts shown so far. */ +export function __resetToasts(): void { + __toasts.length = 0; +} + /** * The runtime `environment`. Only `supportPath` is exercised, by the snip * directory, and a stable fake path keeps those assertions independent of where diff --git a/src/__tests__/agent.test.ts b/src/__tests__/agent.test.ts index b6fd30c..da505a7 100644 --- a/src/__tests__/agent.test.ts +++ b/src/__tests__/agent.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import path from "node:path"; import { readFileSync } from "node:fs"; import { pickToolCalls, runAgent } from "../lib/agent"; -import { MAX_TOOL_ITERATIONS } from "../lib/constants"; +import { DEFAULT_MAX_TOOL_ROUNDS } from "../lib/constants"; import { OllamaError } from "../lib/ollama"; import { toolsFor } from "../lib/tools"; import type { ApiMessage, ToolCall } from "../lib/types"; @@ -28,6 +28,19 @@ function answer(text: string): Record[] { return [{ message: { role: "assistant", content: text }, done: true, done_reason: "stop" }]; } +/** + * One tool call that is distinct every round. + * + * The arguments have to differ, or the repeat guard in `agent.ts` ends the turn + * before the round cap the tests are about ever gets there. `get_datetime` is the + * tool to use because it never touches the network, so the only `fetch` calls a + * test sees are the ones to the chat endpoint. (`web_search` with a real `query` + * fetches, which quietly ate the queued mock responses.) + */ +function distinctCall(index: number): Record { + return toolCall("get_datetime", { attempt: index }); +} + const fetchMock = vi.fn(); beforeEach(() => { @@ -137,15 +150,15 @@ describe("runAgent", () => { it("stops after the iteration cap instead of looping forever", async () => { // Every response asks for another tool call, so only the cap can end this. - for (let i = 0; i < MAX_TOOL_ITERATIONS + 2; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime")])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS + 2; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); } const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); - expect(result.toolCalls).toHaveLength(MAX_TOOL_ITERATIONS); + expect(result.toolCalls).toHaveLength(DEFAULT_MAX_TOOL_ROUNDS); // Every tool round, plus one closing round with the tools taken away. - expect(fetchMock).toHaveBeenCalledTimes(MAX_TOOL_ITERATIONS + 1); + expect(fetchMock).toHaveBeenCalledTimes(DEFAULT_MAX_TOOL_ROUNDS + 1); // The closing round produced no text either, so the notice is the last resort. expect(result.content).toContain("Ran out of tool calls"); }); @@ -153,8 +166,8 @@ describe("runAgent", () => { it("gives the model a closing round so a spent budget still ends in an answer", async () => { // The reported failure: a broad research question used up the budget and the // turn ended with a notice about the loop instead of an answer. - for (let i = 0; i < MAX_TOOL_ITERATIONS; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("web_search")])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); } fetchMock.mockResolvedValueOnce( ndjsonResponse(answer("Meadows have Perch, at 2 fish each. I could not confirm the others.")), @@ -167,15 +180,15 @@ describe("runAgent", () => { }); it("takes the tools away for the closing round, so it writes rather than searches again", async () => { - for (let i = 0; i < MAX_TOOL_ITERATIONS; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("web_search")])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); } fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("Here is what I found."))); await runAgent(baseOptions([{ role: "user", content: "hi" }])); const first = JSON.parse(fetchMock.mock.calls[0][1].body as string) as { tools?: unknown }; - const last = JSON.parse(fetchMock.mock.calls[MAX_TOOL_ITERATIONS][1].body as string) as { + const last = JSON.parse(fetchMock.mock.calls[DEFAULT_MAX_TOOL_ROUNDS][1].body as string) as { tools?: unknown; }; @@ -184,14 +197,14 @@ describe("runAgent", () => { }); it("asks the closing round to name what it could not confirm", async () => { - for (let i = 0; i < MAX_TOOL_ITERATIONS; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("web_search")])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); } fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("ok"))); await runAgent(baseOptions([{ role: "user", content: "hi" }])); - const last = JSON.parse(fetchMock.mock.calls[MAX_TOOL_ITERATIONS][1].body as string) as { + const last = JSON.parse(fetchMock.mock.calls[DEFAULT_MAX_TOOL_ROUNDS][1].body as string) as { messages: ApiMessage[]; }; const nudge = last.messages[last.messages.length - 1]; @@ -204,17 +217,17 @@ describe("runAgent", () => { it("falls back to the last preamble when the closing round comes back empty", async () => { // A preamble is not normally the answer, but a turn that gathered things // should still say more than a notice about its own loop. - const preamble = { + const preamble = (i: number) => ({ message: { role: "assistant", content: "Let me look that up.", - tool_calls: [{ function: { name: "web_search", arguments: {} } }], + tool_calls: [{ function: { name: "get_datetime", arguments: { attempt: i } } }], }, done: true, - }; + }); - for (let i = 0; i < MAX_TOOL_ITERATIONS; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([preamble])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([preamble(i)])); } fetchMock.mockResolvedValueOnce(ndjsonResponse([{ message: { role: "assistant", content: "" }, done: true }])); @@ -224,14 +237,14 @@ describe("runAgent", () => { }); it("still reports the tools it used when the budget runs out", async () => { - for (let i = 0; i < MAX_TOOL_ITERATIONS; i++) { - fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime")])); + for (let i = 0; i < DEFAULT_MAX_TOOL_ROUNDS; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); } fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("done"))); const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); - expect(result.toolCalls).toHaveLength(MAX_TOOL_ITERATIONS); + expect(result.toolCalls).toHaveLength(DEFAULT_MAX_TOOL_ROUNDS); expect(result.toolCalls.every((call) => call.name === "get_datetime")).toBe(true); }); @@ -248,6 +261,138 @@ describe("runAgent", () => { }); }); + /** Queues `count` rounds that all ask for the identical call. */ + function queueRepeats(count: number): void { + for (let i = 0; i < count; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC" })])); + } + } + + it("stops when the same call comes round a third time instead of running it again", async () => { + // The rut this guards against: the model decides the first search did not + // answer the question and asks the identical question again, and again. + // Two executions is a retry; a third is not going to learn anything new. + queueRepeats(3); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("It is 03:04 UTC."))); + + const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + // Two executions, plus the refusal that ended the turn. + expect(result.toolCalls).toHaveLength(3); + expect(result.toolCalls.filter((call) => call.repeated)).toHaveLength(1); + expect(result.content).toBe("It is 03:04 UTC."); + // Two tool rounds, the refusal round, and the closing round. + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it("does not count a different call with the same tool as a repeat", async () => { + // Searching for three different things is a model doing its job, not a loop. + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "CET" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "JST" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("Three zones."))); + + const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + expect(result.toolCalls).toHaveLength(3); + expect(result.toolCalls.every((call) => !call.repeated)).toBe(true); + }); + + it("treats reordered arguments as the same call", async () => { + // Models are not consistent about key order, and a guard that only catches + // byte-identical calls would miss the ordinary shape of a repeat. + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC", unit: "h" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { unit: "h", tz: "UTC" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC", unit: "h" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("done"))); + + const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + expect(result.toolCalls.filter((call) => call.repeated)).toHaveLength(1); + }); + + it("tells the model the call was repeating rather than that a budget ran out", async () => { + queueRepeats(3); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("ok"))); + + await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + const last = JSON.parse(fetchMock.mock.calls[3][1].body as string) as { messages: ApiMessage[] }; + const nudge = last.messages[last.messages.length - 1]; + + expect(nudge.role).toBe("user"); + expect(nudge.content).toContain("identical arguments 3 times"); + expect(nudge.content).not.toContain("reached the limit"); + }); + + it("answers from what it gathered when a repeat ends the turn", async () => { + queueRepeats(3); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("I could not get past the time check."))); + + const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + expect(result.content).toBe("I could not get past the time check."); + expect(result.content).not.toContain("kept repeating"); + }); + + it("says the turn was cut short when a repeat leaves the closing round speechless", async () => { + queueRepeats(3); + fetchMock.mockResolvedValueOnce( + ndjsonResponse([{ message: { role: "assistant", content: "" }, done: true }]), + ); + + const result = await runAgent(baseOptions([{ role: "user", content: "hi" }])); + + expect(result.content).toContain("kept repeating the same call"); + }); + + it("honours a caller's round cap in place of the default", async () => { + for (let i = 0; i < 4; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); + } + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("done"))); + + const result = await runAgent({ ...baseOptions([{ role: "user", content: "hi" }]), maxToolRounds: 4 }); + + expect(result.toolCalls).toHaveLength(4); + expect(result.content).toBe("done"); + }); + + it("runs past the default cap when the caller asks for no cap at all", async () => { + // The preference can be switched to Unlimited, and the only difference that + // should make is that the cap stops arriving. + const rounds = DEFAULT_MAX_TOOL_ROUNDS + 3; + for (let i = 0; i < rounds; i++) { + fetchMock.mockResolvedValueOnce(ndjsonResponse([distinctCall(i)])); + } + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("done"))); + + const result = await runAgent({ ...baseOptions([{ role: "user", content: "hi" }]), maxToolRounds: null }); + + expect(result.toolCalls).toHaveLength(rounds); + expect(result.content).toBe("done"); + }); + + it("reports each tool start with how many times that call has come round", async () => { + // This is what the transcript shows while a turn is in flight, so a repeat + // is visible as it happens and not only in the record afterwards. + const started: { name: string; label: string; count: number }[] = []; + + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse([toolCall("get_datetime", { tz: "UTC" })])); + fetchMock.mockResolvedValueOnce(ndjsonResponse(answer("done"))); + + await runAgent({ + ...baseOptions([{ role: "user", content: "hi" }]), + events: { onToolStart: (name, label, count) => started.push({ name, label, count }) }, + }); + + expect(started).toEqual([ + { name: "get_datetime", label: "Checking the time", count: 1 }, + { name: "get_datetime", label: "Checking the time", count: 2 }, + ]); + }); + describe("module wiring", () => { it("keeps the request, tool, and agent layers free of Vicinae imports", () => { // These modules must stay renderer-free: they are pure logic, they are what @@ -263,6 +408,7 @@ describe("module wiring", () => { "capture.ts", "attachments.ts", "startup.ts", + "retention.ts", ]) { const source = readFileSync(path.resolve(__dirname, `../lib/${file}`), "utf8"); expect(source, `${file} should not import @vicinae/api`).not.toMatch(/from\s+"@vicinae\/api"/); diff --git a/src/__tests__/cleanup.test.ts b/src/__tests__/cleanup.test.ts new file mode 100644 index 0000000..2b29055 --- /dev/null +++ b/src/__tests__/cleanup.test.ts @@ -0,0 +1,177 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { access, mkdir, rm, utimes, writeFile } from "node:fs/promises"; +import { __resetLocalStorage, __resetToasts, __toasts } from "../__mocks__/@vicinae/api"; +import { clearAllChats, clearOrphanedSnips, runAutoDelete } from "../lib/cleanup"; +import { createSession, createTurn, listSessions, saveSession } from "../lib/sessions"; + +/** + * The one thing the sweep does that the pure decision in `retention.ts` cannot: + * tell the user about it. What matters here is that it stays quiet otherwise, + * because a notification on every launch is one people stop reading. + */ + +const DAY = 24 * 60 * 60 * 1000; +const DIRECTORY = "/tmp/lam-agent-support/snips"; + +beforeEach(async () => { + __resetLocalStorage(); + __resetToasts(); + await rm(DIRECTORY, { recursive: true, force: true }); +}); + +afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); +}); + + +/** A turn carrying one attached capture. */ +function snipTurn(path: string) { + return { ...createTurn("what is this", "m", Date.now()), attachments: [{ path, mime: "image/png" }] }; +} + +/** Saves a conversation that refers to a fresh capture of the given name. */ +async function referenceSnip(name: string, ageMs: number): Promise { + await mkdir(DIRECTORY, { recursive: true }); + const path = `${DIRECTORY}/${name}`; + await writeFile(path, "png"); + const when = new Date(Date.now() - ageMs); + await utimes(path, when, when); + + await saveSession({ + ...createSession("m", Date.now()), + turns: [snipTurn(path)], + }); + + return path; +} + +describe("runAutoDelete", () => { + it("deletes nothing and says nothing when auto-deletion is off", async () => { + await saveSession({ ...createSession("m", 1), updatedAt: 1 }); + + await runAutoDelete({ maxAgeMs: undefined }); + + expect(await listSessions()).toHaveLength(1); + expect(__toasts).toEqual([]); + }); + + it("stays quiet when nothing was old enough", async () => { + await saveSession({ ...createSession("m", Date.now()) }); + + await runAutoDelete({ maxAgeMs: 7 * DAY }); + + expect(__toasts).toEqual([]); + expect(await listSessions()).toHaveLength(1); + }); + + it("reports what it deleted", async () => { + const now = Date.now(); + await saveSession({ ...createSession("m", now - 60 * DAY), updatedAt: now - 60 * DAY }); + await saveSession({ ...createSession("m", now), updatedAt: now }); + + await runAutoDelete({ maxAgeMs: 7 * DAY }); + + expect(await listSessions()).toHaveLength(1); + expect(__toasts).toHaveLength(1); + expect(__toasts[0].title).toBe("Cleared out old conversations"); + expect(__toasts[0].message).toContain("1 conversation"); + }); + + it("stays quiet when the only old conversation is the one being rendered", async () => { + const now = Date.now(); + const open = { ...createSession("m", now), updatedAt: now - 60 * DAY }; + await saveSession(open); + + await runAutoDelete({ maxAgeMs: 7 * DAY, keepId: open.id }); + + expect(__toasts).toEqual([]); + expect(await listSessions()).toHaveLength(1); + }); + + it("survives a corrupt index rather than failing the command that called it", async () => { + const { LocalStorage } = await import("../__mocks__/@vicinae/api"); + await LocalStorage.setItem("lam.sessions.index", "{not json"); + + await expect(runAutoDelete({ maxAgeMs: 7 * DAY })).resolves.toBeUndefined(); + expect(__toasts).toEqual([]); + }); +}); + +describe("the maintenance actions", () => { + const DAY = 24 * 60 * 60 * 1000; + + async function makeSnip(name: string, ageMs: number): Promise { + await mkdir(`${DIRECTORY}`, { recursive: true }); + const path = `${DIRECTORY}/${name}`; + await writeFile(path, "png"); + const when = new Date(Date.now() - ageMs); + await utimes(path, when, when); + return path; + } + + it("deletes every conversation and its screenshots, and reports both counts", async () => { + const path = await makeSnip("one.png", 0); + await saveSession({ + ...createSession("m", Date.now()), + turns: [snipTurn(path)], + }); + await saveSession({ ...createSession("m", Date.now()), turns: [createTurn("hi", "m", Date.now())] }); + + await clearAllChats(); + + expect(await listSessions()).toHaveLength(0); + await expect(access(path)).rejects.toThrow(); + expect(__toasts).toEqual([ + { + title: "Cleared out all conversations", + message: "2 conversations and 1 screenshot", + }, + ]); + }); + + it("answers an explicit clear even when there was nothing to delete", async () => { + // The user pressed a button, so silence would read as a failure. The + // background sweep stays quiet in the same situation, on purpose. + await clearAllChats(); + + expect(__toasts).toEqual([ + { title: "Cleared out all conversations", message: "there was nothing to delete" }, + ]); + }); + + it("collects unattached photos and reports the count", async () => { + await referenceSnip("kept.png", 30 * DAY); + await makeSnip("orphan.png", 30 * DAY); + + await clearOrphanedSnips(); + + expect(__toasts).toEqual([{ title: "Cleared out unattached photos", message: "1 screenshot" }]); + }); + + it("says there was nothing to delete when every photo is spoken for", async () => { + await referenceSnip("kept.png", 30 * DAY); + + await clearOrphanedSnips(); + + expect(__toasts).toEqual([ + { title: "Cleared out unattached photos", message: "there was nothing to delete" }, + ]); + }); + + it("refuses rather than guessing when a conversation cannot be read", async () => { + await makeSnip("orphan.png", 30 * DAY); + await saveSession({ ...createSession("m", Date.now()) }); + + const { LocalStorage } = await import("../__mocks__/@vicinae/api"); + const [summary] = await listSessions(); + const legacy = { ...summary }; + delete legacy.attachmentPaths; + await LocalStorage.setItem("lam.sessions.index", JSON.stringify([legacy])); + await LocalStorage.removeItem(`lam.session.${summary.id}`); + + await clearOrphanedSnips(); + + expect(__toasts).toHaveLength(1); + expect(__toasts[0].title).toContain("Could not check"); + }); +}); diff --git a/src/__tests__/markdown.test.ts b/src/__tests__/markdown.test.ts index 2f3b08c..27b7322 100644 --- a/src/__tests__/markdown.test.ts +++ b/src/__tests__/markdown.test.ts @@ -231,6 +231,79 @@ describe("buildThreadMarkdown", () => { expect(reasoningOnly).toContain("Reasoning"); }); + it("names a refused repeat, and does not call it a failure", () => { + const markdown = buildThreadMarkdown([ + message({ + role: "assistant", + text: "a", + toolCalls: [ + { name: "web_search", arguments: {}, result: "ok", isError: false }, + { + name: "web_search", + arguments: {}, + result: "Refused", + isError: true, + repeated: true, + }, + ], + }), + ]); + + expect(markdown).toContain("`web_search` (repeated, stopped)"); + expect(markdown).not.toContain("(failed)"); + }); + + it("shows a failed call even when tool calls are hidden", () => { + // Hiding the line would leave the user with an answer they cannot account + // for, which is worse than a quiet italic line they did not ask for. + const markdown = buildThreadMarkdown( + [ + message({ + role: "assistant", + text: "a", + toolCalls: [{ name: "web_fetch", arguments: {}, result: "boom", isError: true }], + }), + ], + { showToolCalls: false, showReasoning: false }, + ); + + expect(markdown).toContain("`web_fetch` (failed)"); + }); + + it("shows a refused repeat even when tool calls are hidden", () => { + // This is the one the user most needs to see: a model going in circles is + // exactly the thing a hidden tool list would conceal. + const markdown = buildThreadMarkdown( + [ + message({ + role: "assistant", + text: "a", + toolCalls: [ + { name: "web_search", arguments: {}, result: "Refused", isError: true, repeated: true }, + ], + }), + ], + { showToolCalls: false, showReasoning: false }, + ); + + expect(markdown).toContain("`web_search` (repeated, stopped)"); + }); + + it("still hides the ordinary calls when tool calls are off", () => { + const markdown = buildThreadMarkdown( + [ + message({ + role: "assistant", + text: "a", + toolCalls: [{ name: "get_datetime", arguments: {}, result: "ok", isError: false }], + }), + ], + { showToolCalls: false, showReasoning: false }, + ); + + expect(markdown).not.toContain("_Tools:"); + }); + it("only shows a reasoning section when there is reasoning", () => { expect(buildThreadMarkdown([message({ role: "assistant", text: "a" })])).not.toContain("Reasoning"); expect(buildThreadMarkdown([message({ role: "assistant", text: "a", thinking: "hmm" })])).toContain( diff --git a/src/__tests__/preferences.test.ts b/src/__tests__/preferences.test.ts index ab83d66..ba68bd8 100644 --- a/src/__tests__/preferences.test.ts +++ b/src/__tests__/preferences.test.ts @@ -1,6 +1,14 @@ import { describe, expect, it } from "vitest"; -import { DEFAULT_HOST, DEFAULT_MODEL } from "../lib/constants"; -import { describeResumeTimeout, resolvePreferences, resolveResumeTimeout } from "../lib/preferences"; +import { DEFAULT_HOST, DEFAULT_MAX_TOOL_ROUNDS, DEFAULT_MODEL } from "../lib/constants"; +import { + describeAutoDelete, + describeMaxToolRounds, + describeResumeTimeout, + resolveAutoDelete, + resolveMaxToolRounds, + resolvePreferences, + resolveResumeTimeout, +} from "../lib/preferences"; const MINUTE = 60_000; const HOUR = 60 * MINUTE; @@ -53,6 +61,10 @@ describe("resolvePreferences", () => { expect(resolved.showReasoning).toBe(true); expect(resolved.think).toBeUndefined(); expect(resolved.resumeTimeoutMs).toBe(HOUR); + // Auto-deletion is the one setting here that destroys data, so its default + // is off and every unresolvable value lands on the same answer. + expect(resolved.autoDeleteMs).toBeUndefined(); + expect(resolved.maxToolRounds).toBe(DEFAULT_MAX_TOOL_ROUNDS); // An empty string rather than undefined, so callers never branch on it. expect(resolved.place).toBe(""); }); @@ -104,4 +116,90 @@ describe("resolvePreferences", () => { expect(resolvePreferences({ resumeTimeout: "15m" }).resumeTimeoutMs).toBe(15 * MINUTE); expect(resolvePreferences({ resumeTimeout: "never" }).resumeTimeoutMs).toBeUndefined(); }); + + it("carries the auto-delete window through, including disabled", () => { + expect(resolvePreferences({ autoDeleteAfter: "30d" }).autoDeleteMs).toBe(30 * DAY); + expect(resolvePreferences({ autoDeleteAfter: "disabled" }).autoDeleteMs).toBeUndefined(); + expect(resolvePreferences({ autoDeleteAfter: "garbage" }).autoDeleteMs).toBeUndefined(); + }); + + it("carries the tool-call cap through, including unlimited", () => { + expect(resolvePreferences({ maxToolRounds: "16" }).maxToolRounds).toBe(16); + expect(resolvePreferences({ maxToolRounds: "unlimited" }).maxToolRounds).toBeUndefined(); + expect(resolvePreferences({ maxToolRounds: "garbage" }).maxToolRounds).toBe(DEFAULT_MAX_TOOL_ROUNDS); + }); +}); + +describe("resolveAutoDelete", () => { + it("maps each configured window", () => { + expect(resolveAutoDelete("1d")).toBe(DAY); + expect(resolveAutoDelete("7d")).toBe(7 * DAY); + expect(resolveAutoDelete("30d")).toBe(30 * DAY); + expect(resolveAutoDelete("90d")).toBe(90 * DAY); + }); + + it("returns undefined for 'disabled', which is the default", () => { + expect(resolveAutoDelete("disabled")).toBeUndefined(); + }); + + it("returns undefined for missing or unrecognised values rather than guessing", () => { + // Both readings of an unknown value are safe here only because the fallback + // is "delete nothing". See `resolveMaxToolRounds` for the other convention. + expect(resolveAutoDelete(undefined)).toBeUndefined(); + expect(resolveAutoDelete("")).toBeUndefined(); + expect(resolveAutoDelete("banana")).toBeUndefined(); + expect(resolveAutoDelete("1y")).toBeUndefined(); + }); + + it("tolerates surrounding whitespace and casing", () => { + expect(resolveAutoDelete(" 7D ")).toBe(7 * DAY); + }); +}); + +describe("describeAutoDelete", () => { + it("names the off case rather than showing a number", () => { + expect(describeAutoDelete(undefined)).toContain("Disabled"); + }); + + it("describes days", () => { + expect(describeAutoDelete(DAY)).toBe("After 1 day"); + expect(describeAutoDelete(30 * DAY)).toBe("After 30 days"); + }); +}); + +describe("resolveMaxToolRounds", () => { + it("maps each configured cap", () => { + expect(resolveMaxToolRounds("4")).toBe(4); + expect(resolveMaxToolRounds("8")).toBe(8); + expect(resolveMaxToolRounds("16")).toBe(16); + }); + + it("returns undefined for unlimited, which is what removes the cap", () => { + expect(resolveMaxToolRounds("unlimited")).toBeUndefined(); + }); + + it("falls back to the default cap for anything unrecognised", () => { + // Deliberately the opposite of `resolveAutoDelete`: those two settings are + // safe when they do nothing, and this one guards a loop, so the safe reading + // of a value this codebase does not understand is the cap and not its absence. + expect(resolveMaxToolRounds(undefined)).toBe(DEFAULT_MAX_TOOL_ROUNDS); + expect(resolveMaxToolRounds("")).toBe(DEFAULT_MAX_TOOL_ROUNDS); + expect(resolveMaxToolRounds("banana")).toBe(DEFAULT_MAX_TOOL_ROUNDS); + expect(resolveMaxToolRounds("0")).toBe(DEFAULT_MAX_TOOL_ROUNDS); + }); + + it("tolerates surrounding whitespace and casing", () => { + expect(resolveMaxToolRounds(" UNLIMITED ")).toBeUndefined(); + }); +}); + +describe("describeMaxToolRounds", () => { + it("names the uncapped case", () => { + expect(describeMaxToolRounds(undefined)).toContain("Unlimited"); + }); + + it("describes a cap in rounds", () => { + expect(describeMaxToolRounds(8)).toBe("8 rounds per answer"); + expect(describeMaxToolRounds(1)).toBe("1 round per answer"); + }); }); diff --git a/src/__tests__/retention.test.ts b/src/__tests__/retention.test.ts new file mode 100644 index 0000000..91b34f3 --- /dev/null +++ b/src/__tests__/retention.test.ts @@ -0,0 +1,267 @@ +import { describe, expect, it, vi } from "vitest"; +import { SNIP_ORPHAN_GRACE_MS } from "../lib/constants"; +import { + describeRemoval, + expiredSessions, + orphanedSnips, + sweepExpired, + type SnipFile, + type SweepPorts, +} from "../lib/retention"; +import type { SessionSummary } from "../lib/types"; + +/** + * The deletion rules. + * + * This is the only code in the extension that destroys anything, so these tests + * are mostly about what it refuses to delete: the off switch, the open + * conversation, a snip that is still pending, and the whole orphan pass when the + * referenced set is not trustworthy. + */ + +const DAY = 24 * 60 * 60 * 1000; +const now = 1_000 * DAY; + +function summary(id: string, updatedAt: number): SessionSummary { + return { + id, + title: id, + model: "test-model", + createdAt: updatedAt, + updatedAt, + turnCount: 1, + preview: "", + attachmentPaths: [], + }; +} + +function snip(path: string, mtimeMs: number): SnipFile { + return { path, mtimeMs }; +} + +describe("expiredSessions", () => { + it("is off when no window is configured", () => { + expect(expiredSessions([summary("a", now - 400 * DAY)], undefined, now)).toEqual([]); + }); + + it("picks the conversations that have not been used for longer than the window", () => { + const recent = summary("recent", now - DAY + 1); + const stale = summary("stale", now - 8 * DAY); + + expect(expiredSessions([recent, stale], 7 * DAY, now)).toEqual([stale]); + }); + + it("ages a conversation from when it was last used, not when it was started", () => { + // A chat started last month and used this morning is not old. + const longLived = { ...summary("long-lived", now - 1_000), createdAt: now - 60 * DAY }; + + expect(expiredSessions([longLived], 7 * DAY, now)).toEqual([]); + }); + + it("counts a conversation updated exactly at the window as expired", () => { + expect(expiredSessions([summary("edge", now - 7 * DAY)], 7 * DAY, now)).toHaveLength(1); + }); + + it("never expires the conversation that is open", () => { + const open = summary("open", now - 400 * DAY); + + expect(expiredSessions([open, summary("other", now - 400 * DAY)], 7 * DAY, now, "open")).toEqual([ + expect.objectContaining({ id: "other" }), + ]); + }); + + it("treats a clock that has run backwards as fresh rather than as ancient", () => { + expect(expiredSessions([summary("future", now + DAY)], 7 * DAY, now)).toEqual([]); + }); +}); + +describe("orphanedSnips", () => { + it("collects an unreferenced file once it has waited out the grace period", () => { + const old = snip("/snips/old.png", now - SNIP_ORPHAN_GRACE_MS - 1); + + expect(orphanedSnips([old], [], now)).toEqual([old]); + }); + + it("leaves an unreferenced file alone while it could still be a pending snip", () => { + // The file is on disk before the chat view opens and stays unreferenced + // until the user sends it, so this wait is what protects it. + const fresh = snip("/snips/fresh.png", now - 1_000); + + expect(orphanedSnips([fresh], [], now)).toEqual([]); + }); + + it("keeps a file a surviving conversation still refers to, however old it is", () => { + const referenced = snip("/snips/referenced.png", now - 400 * DAY); + + expect(orphanedSnips([referenced], ["/snips/referenced.png"], now)).toEqual([]); + }); + + it("honours a caller's own grace period", () => { + const file = snip("/snips/one.png", now - 2 * 60 * 1000); + + expect(orphanedSnips([file], [], now, 60 * 1000)).toEqual([file]); + }); +}); + +describe("sweepExpired", () => { + function ports(overrides: Partial = {}): SweepPorts { + return { + listSessions: vi.fn(async () => []), + deleteSessions: vi.fn(async () => []), + referencedAttachments: vi.fn(async () => []), + listSnipFiles: vi.fn(async () => []), + removeSnipFiles: vi.fn(async () => 0), + ...overrides, + }; + } + + it("reads and writes nothing at all when it is disabled", async () => { + const fake = ports(); + + const outcome = await sweepExpired({ maxAgeMs: undefined, now }, fake); + + expect(outcome).toEqual({ sessions: 0, files: 0 }); + expect(fake.listSessions).not.toHaveBeenCalled(); + expect(fake.listSnipFiles).not.toHaveBeenCalled(); + expect(fake.removeSnipFiles).not.toHaveBeenCalled(); + }); + + it("deletes expired conversations and the screenshots they held", async () => { + const fake = ports({ + listSessions: vi.fn(async () => [summary("stale", now - 8 * DAY), summary("fresh", now)]), + deleteSessions: vi.fn(async () => ["/snips/taken-minutes-ago.png"]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.deleteSessions).toHaveBeenCalledWith(["stale"]); + // Explicitly, not by the grace-period rule: a screenshot taken minutes before + // its conversation is deleted should go with it. + expect(fake.removeSnipFiles).toHaveBeenCalledWith(["/snips/taken-minutes-ago.png"]); + expect(outcome).toEqual({ sessions: 1, files: 1 }); + }); + + it("collects orphan files and spares the ones a conversation still refers to", async () => { + const orphan = "/snips/orphan.png"; + const referenced = "/snips/referenced.png"; + + const fake = ports({ + listSessions: vi.fn(async () => [summary("kept", now)]), + referencedAttachments: vi.fn(async () => [referenced]), + listSnipFiles: vi.fn(async () => [ + snip(orphan, now - 2 * DAY), + snip(referenced, now - 2 * DAY), + ]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).toHaveBeenCalledWith([orphan]); + expect(outcome).toEqual({ sessions: 0, files: 1 }); + }); + + it("skips the orphan pass when the referenced set cannot be established", async () => { + // An incomplete set of references is indistinguishable from a pile of + // orphans, and the wrong answer deletes real screenshots. + const fake = ports({ + referencedAttachments: vi.fn(async () => undefined), + listSnipFiles: vi.fn(async () => [snip("/snips/maybe.png", now - 400 * DAY)]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).not.toHaveBeenCalled(); + expect(outcome).toEqual({ sessions: 0, files: 0 }); + }); + + it("never deletes the conversation that is open", async () => { + const fake = ports({ + listSessions: vi.fn(async () => [summary("open", now - 400 * DAY)]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now, keepId: "open" }, fake); + + expect(fake.deleteSessions).not.toHaveBeenCalled(); + expect(outcome.sessions).toBe(0); + }); + + + it("keeps a screenshot a surviving conversation also refers to", async () => { + // Two conversations can hold the same capture, so the deleted one's files are + // not simply its own. + const shared = "/snips/shared.png"; + + const fake = ports({ + listSessions: vi.fn(async () => [summary("stale", now - 8 * DAY), summary("keep", now)]), + deleteSessions: vi.fn(async () => [shared]), + referencedAttachments: vi.fn(async () => [shared]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).not.toHaveBeenCalled(); + expect(outcome).toEqual({ sessions: 1, files: 0 }); + }); + + it("does not remove a deleted conversation's files when the references are unknown", async () => { + // Leaking a file is recoverable. Deleting one a survivor still shows is not. + const fake = ports({ + listSessions: vi.fn(async () => [summary("stale", now - 8 * DAY)]), + deleteSessions: vi.fn(async () => ["/snips/unverifiable.png"]), + referencedAttachments: vi.fn(async () => undefined), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).not.toHaveBeenCalled(); + expect(outcome).toEqual({ sessions: 1, files: 0 }); + }); + + it("does not remove the same file twice when both passes find it", async () => { + const fake = ports({ + listSessions: vi.fn(async () => [summary("stale", now - 8 * DAY)]), + deleteSessions: vi.fn(async () => ["/snips/both.png"]), + listSnipFiles: vi.fn(async () => [snip("/snips/both.png", now - 8 * DAY)]), + }); + + await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).toHaveBeenCalledWith(["/snips/both.png"]); + }); + + it("still tidies files when there is nothing to expire", async () => { + // An unsent capture belongs to no conversation, so no amount of session + // deletion would ever reach it. + const fake = ports({ + listSnipFiles: vi.fn(async () => [snip("/snips/unsent.png", now - 400 * DAY)]), + }); + + const outcome = await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(outcome).toEqual({ sessions: 0, files: 1 }); + }); + + it("leaves storage and the file system alone when nothing qualifies", async () => { + const fake = ports({ listSnipFiles: vi.fn(async () => [snip("/snips/fresh.png", now)]) }); + + await sweepExpired({ maxAgeMs: 7 * DAY, now }, fake); + + expect(fake.removeSnipFiles).not.toHaveBeenCalled(); + }); +}); + +describe("describeRemoval", () => { + it("says nothing when nothing was deleted", () => { + expect(describeRemoval({ sessions: 0, files: 0 })).toBeUndefined(); + }); + + it("counts conversations, keeping the noun singular at one", () => { + expect(describeRemoval({ sessions: 1, files: 0 })).toBe("1 conversation"); + expect(describeRemoval({ sessions: 3, files: 0 })).toBe("3 conversations"); + }); + + it("mentions screenshots when it removed any", () => { + expect(describeRemoval({ sessions: 0, files: 1 })).toBe("1 screenshot"); + expect(describeRemoval({ sessions: 2, files: 4 })).toBe("2 conversations and 4 screenshots"); + }); +}); diff --git a/src/__tests__/sessions.test.ts b/src/__tests__/sessions.test.ts index 3d363f7..b29ec67 100644 --- a/src/__tests__/sessions.test.ts +++ b/src/__tests__/sessions.test.ts @@ -1,4 +1,5 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { access, mkdir, rm, utimes, writeFile } from "node:fs/promises"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { __resetLocalStorage } from "../__mocks__/@vicinae/api"; import { MAX_STORED_CHARS_PER_TURN, MAX_TURNS_PER_SESSION } from "../lib/constants"; import { @@ -7,10 +8,14 @@ import { createSession, createTurn, clearSessions, + cullOrphanedSnips, deleteSession, + deleteSessions, listSessions, loadSession, + referencedAttachments, saveSession, + sweepExpiredSessions, withTitleFromFirstTurn, } from "../lib/sessions"; import type { ChatTurn } from "../lib/types"; @@ -160,3 +165,352 @@ describe("createTurn", () => { expect(created.toolCalls).toEqual([]); }); }); + +describe("attachment paths", () => { + const PATH = "/snips/one.png"; + + function sniffedTurn(index: number, path: string): ChatTurn { + return { ...turn(index), attachments: [{ path, mime: "image/png" }] }; + } + + it("records the paths a conversation refers to on its index entry", async () => { + // The index has to know which screenshots are spoken for without reading a + // transcript, because the orphan sweep runs after every single answer. + const session = { ...createSession("m", 1_000), turns: [sniffedTurn(1, PATH)] }; + await saveSession(session); + + const [summary] = await listSessions(); + expect(summary.attachmentPaths).toEqual([PATH]); + }); + + it("deduplicates a path used by more than one turn", async () => { + const session = { + ...createSession("m", 1_000), + turns: [sniffedTurn(1, PATH), sniffedTurn(2, PATH), sniffedTurn(3, "/snips/two.png")], + }; + await saveSession(session); + + const [summary] = await listSessions(); + expect(summary.attachmentPaths).toEqual([PATH, "/snips/two.png"]); + }); + + it("reports every path the conversations that remain refer to", async () => { + await saveSession({ ...createSession("m", 1_000), turns: [sniffedTurn(1, PATH)] }); + + expect(await referencedAttachments()).toEqual([PATH]); + }); + + it("fills in an index written before attachment paths were recorded", async () => { + const { LocalStorage } = await import("../__mocks__/@vicinae/api"); + await saveSession({ ...createSession("m", 1_000), turns: [sniffedTurn(1, PATH)] }); + + // Exactly what an upgrade looks like: the field is simply absent. + const [summary] = await listSessions(); + const legacy = { ...summary }; + delete legacy.attachmentPaths; + await LocalStorage.setItem("lam.sessions.index", JSON.stringify([legacy])); + + expect(await referencedAttachments()).toEqual([PATH]); + + // And repaired, so the next sweep does not read the transcript again. + const [repaired] = await listSessions(); + expect(repaired.attachmentPaths).toEqual([PATH]); + }); + + it("refuses to guess when a transcript will not load", async () => { + // A session whose body cannot be read would look like it referred to + // nothing, and everything it referred to would be deleted. + const { LocalStorage } = await import("../__mocks__/@vicinae/api"); + await saveSession({ ...createSession("m", 1_000), turns: [sniffedTurn(1, PATH)] }); + + const [summary] = await listSessions(); + const legacy = { ...summary }; + delete legacy.attachmentPaths; + await LocalStorage.setItem("lam.sessions.index", JSON.stringify([legacy])); + await LocalStorage.removeItem(`lam.session.${summary.id}`); + + expect(await referencedAttachments()).toBeUndefined(); + }); + + it("deletes several conversations in one pass and hands back their screenshots", async () => { + const first = { ...createSession("m", 1_000), turns: [sniffedTurn(1, PATH)] }; + const second = { ...createSession("m", 2_000), turns: [sniffedTurn(1, "/snips/two.png")] }; + const kept = { ...createSession("m", 3_000), turns: [sniffedTurn(1, "/snips/three.png")] }; + + await saveSession(first); + await saveSession(second); + await saveSession(kept); + + const removed = await deleteSessions([first.id, second.id]); + + expect(removed.sort()).toEqual([PATH, "/snips/two.png"]); + expect((await listSessions()).map((summary) => summary.id)).toEqual([kept.id]); + expect(await loadSession(first.id)).toBeUndefined(); + expect(await loadSession(second.id)).toBeUndefined(); + }); + + it("does nothing when asked to delete nothing", async () => { + await saveSession(createSession("m", 1_000)); + + expect(await deleteSessions([])).toEqual([]); + expect(await listSessions()).toHaveLength(1); + }); +}); + +describe("sweepExpiredSessions", () => { + const DIRECTORY = "/tmp/lam-agent-support/snips"; + const DAY = 24 * 60 * 60 * 1000; + + async function makeSnip(name: string, ageMs: number): Promise { + const path = `${DIRECTORY}/${name}`; + await writeFile(path, "png"); + const when = new Date(Date.now() - ageMs); + await utimes(path, when, when); + return path; + } + + beforeEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + await mkdir(DIRECTORY, { recursive: true }); + }); + + afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + }); + + it("deletes expired conversations, their screenshots, and unreferenced leftovers", async () => { + const now = Date.now(); + + const takenRecently = await makeSnip("recent-deleted.png", 60 * 1000); + const referenced = await makeSnip("still-referenced.png", 30 * DAY); + const abandoned = await makeSnip("never-sent.png", 3 * DAY); + + const stale = { + ...createSession("m", now - 60 * DAY), + updatedAt: now - 60 * DAY, + turns: [{ ...turn(1), attachments: [{ path: takenRecently, mime: "image/png" }] }], + }; + const kept = { + ...createSession("m", now), + updatedAt: now, + turns: [{ ...turn(2), attachments: [{ path: referenced, mime: "image/png" }] }], + }; + + await saveSession(stale); + await saveSession(kept); + + const outcome = await sweepExpiredSessions({ maxAgeMs: 7 * DAY, now }); + + expect(outcome).toEqual({ sessions: 1, files: 2 }); + expect((await listSessions()).map((summary) => summary.id)).toEqual([kept.id]); + + // The screenshot of the conversation that went, gone with it, even though it + // was taken a minute ago. + await expect(access(takenRecently)).rejects.toThrow(); + // A snip nothing ever referred to, collected after its wait. + await expect(access(abandoned)).rejects.toThrow(); + // Still spoken for, so untouched however old it is. + await expect(access(referenced)).resolves.toBeUndefined(); + }); + + it("leaves the open conversation and its screenshots alone", async () => { + const now = Date.now(); + const referenced = await makeSnip("open-chat.png", 30 * DAY); + const open = { + ...createSession("m", now - 60 * DAY), + updatedAt: now - 60 * DAY, + turns: [{ ...turn(1), attachments: [{ path: referenced, mime: "image/png" }] }], + }; + + await saveSession(open); + + const outcome = await sweepExpiredSessions({ maxAgeMs: 7 * DAY, now, keepId: open.id }); + + expect(outcome.sessions).toBe(0); + expect((await listSessions()).map((summary) => summary.id)).toEqual([open.id]); + await expect(access(referenced)).resolves.toBeUndefined(); + }); + + + it("keeps a screenshot an expired conversation shared with a surviving one", async () => { + const now = Date.now(); + const shared = await makeSnip("shared.png", 30 * DAY); + const attachment = [{ path: shared, mime: "image/png" }]; + + const stale = { + ...createSession("m", now - 60 * DAY), + updatedAt: now - 60 * DAY, + turns: [{ ...turn(1), attachments: attachment }], + }; + const kept = { + ...createSession("m", now), + updatedAt: now, + turns: [{ ...turn(2), attachments: attachment }], + }; + + await saveSession(stale); + await saveSession(kept); + + const outcome = await sweepExpiredSessions({ maxAgeMs: 7 * DAY, now }); + + expect(outcome).toEqual({ sessions: 1, files: 0 }); + await expect(access(shared)).resolves.toBeUndefined(); + }); + + it("touches nothing when auto-deletion is off", async () => { + const now = Date.now(); + const abandoned = await makeSnip("never-sent.png", 400 * DAY); + const stale = { ...createSession("m", now - 400 * DAY), updatedAt: now - 400 * DAY }; + await saveSession(stale); + + expect(await sweepExpiredSessions({ maxAgeMs: undefined, now })).toEqual({ sessions: 0, files: 0 }); + expect(await listSessions()).toHaveLength(1); + await expect(access(abandoned)).resolves.toBeUndefined(); + }); +}); + +describe("manual deletion", () => { + const DIRECTORY = "/tmp/lam-agent-support/snips"; + + async function makeSnip(name: string): Promise { + const path = `${DIRECTORY}/${name}`; + await writeFile(path, "png"); + return path; + } + + beforeEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + await mkdir(DIRECTORY, { recursive: true }); + }); + + afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + }); + + it("takes a deleted conversation's screenshots with it", async () => { + // Otherwise a manual delete leaks images that nothing will ever collect, + // since the orphan pass only runs while auto-deletion is switched on. + const path = await makeSnip("one.png"); + const session = { + ...createSession("m", 1_000), + turns: [{ ...turn(1), attachments: [{ path, mime: "image/png" }] }], + }; + await saveSession(session); + + await deleteSession(session.id); + + await expect(access(path)).rejects.toThrow(); + }); + + it("takes every conversation's screenshots when clearing all of them", async () => { + const first = await makeSnip("one.png"); + const second = await makeSnip("two.png"); + + await saveSession({ + ...createSession("m", 1_000), + turns: [{ ...turn(1), attachments: [{ path: first, mime: "image/png" }] }], + }); + await saveSession({ + ...createSession("m", 2_000), + turns: [{ ...turn(1), attachments: [{ path: second, mime: "image/png" }] }], + }); + + await clearSessions(); + + expect(await listSessions()).toHaveLength(0); + await expect(access(first)).rejects.toThrow(); + await expect(access(second)).rejects.toThrow(); + }); + + it("does not touch a screenshot another conversation still uses", async () => { + const shared = await makeSnip("shared.png"); + const attachment = [{ path: shared, mime: "image/png" }]; + + const doomed = { ...createSession("m", 1_000), turns: [{ ...turn(1), attachments: attachment }] }; + const kept = { ...createSession("m", 2_000), turns: [{ ...turn(2), attachments: attachment }] }; + + await saveSession(doomed); + await saveSession(kept); + await deleteSession(doomed.id); + + // Deleting the file would leave the surviving conversation showing a broken + // thumbnail, which the orphan pass would never catch because it is referenced. + await expect(access(shared)).resolves.toBeUndefined(); + }); +}); + +describe("cullOrphanedSnips", () => { + const DIRECTORY = "/tmp/lam-agent-support/snips"; + const DAY = 24 * 60 * 60 * 1000; + + async function makeSnip(name: string, ageMs: number): Promise { + const path = `${DIRECTORY}/${name}`; + await writeFile(path, "png"); + const when = new Date(Date.now() - ageMs); + await utimes(path, when, when); + return path; + } + + async function reference(path: string): Promise { + await saveSession({ + ...createSession("m", Date.now()), + turns: [{ ...turn(1), attachments: [{ path, mime: "image/png" }] }], + }); + } + + beforeEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + await mkdir(DIRECTORY, { recursive: true }); + }); + + afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + }); + + it("collects a capture that no conversation refers to", async () => { + const orphan = await makeSnip("orphan.png", 2 * DAY); + + await expect(cullOrphanedSnips()).resolves.toBe(1); + await expect(access(orphan)).rejects.toThrow(); + }); + + it("keeps a capture a conversation still shows", async () => { + const referenced = await makeSnip("referenced.png", 30 * DAY); + await reference(referenced); + + await expect(cullOrphanedSnips()).resolves.toBe(0); + await expect(access(referenced)).resolves.toBeUndefined(); + }); + + it("keeps a fresh capture, which could still be one the user has not sent", async () => { + // This is the same grace period the background sweep uses. The file is on + // disk before the chat view opens and stays unreferenced until the question + // is sent, so deleting it early would break a snip in flight. + const pending = await makeSnip("pending.png", 60 * 1000); + + await expect(cullOrphanedSnips()).resolves.toBe(0); + await expect(access(pending)).resolves.toBeUndefined(); + }); + + it("answers nothing rather than zero when a transcript will not load", async () => { + // Zero would be a claim that everything referenced is accounted for. With an + // unreadable conversation that claim cannot be made, so the caller is told + // the answer is unknown and deletes nothing. + const { LocalStorage } = await import("../__mocks__/@vicinae/api"); + const orphan = await makeSnip("orphan.png", 30 * DAY); + + await saveSession({ ...createSession("m", Date.now()), turns: [turn(1)] }); + const [summary] = await listSessions(); + const legacy = { ...summary }; + delete legacy.attachmentPaths; + await LocalStorage.setItem("lam.sessions.index", JSON.stringify([legacy])); + await LocalStorage.removeItem(`lam.session.${summary.id}`); + + await expect(cullOrphanedSnips()).resolves.toBeUndefined(); + await expect(access(orphan)).resolves.toBeUndefined(); + }); + + it("reports zero when there is nothing to collect", async () => { + await expect(cullOrphanedSnips()).resolves.toBe(0); + }); +}); diff --git a/src/__tests__/snip.test.ts b/src/__tests__/snip.test.ts index e9f34f1..1bc5365 100644 --- a/src/__tests__/snip.test.ts +++ b/src/__tests__/snip.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it } from "vitest"; -import { readSnipContext, snipDirectory } from "../lib/snip"; +import { access, mkdir, rm, utimes, writeFile } from "node:fs/promises"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { listSnipFiles, readSnipContext, removeSnipFiles, snipDirectory } from "../lib/snip"; /** * The snip handoff crosses a command boundary as a serialized launch context, so @@ -55,3 +56,96 @@ describe("snipDirectory", () => { expect(snipDirectory()).toBe("/tmp/lam-agent-support/snips"); }); }); + +describe("listSnipFiles", () => { + const DIRECTORY = "/tmp/lam-agent-support/snips"; + + beforeEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + }); + + afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + }); + + it("returns nothing before anything has been snipped", async () => { + await expect(listSnipFiles()).resolves.toEqual([]); + }); + + it("reports each capture with the time it was written", async () => { + await mkdir(DIRECTORY, { recursive: true }); + const path = `${DIRECTORY}/one.png`; + await writeFile(path, "png"); + const when = new Date(Date.now() - 60_000); + await utimes(path, when, when); + + const [file] = await listSnipFiles(); + + expect(file.path).toBe(path); + expect(Date.now() - file.mtimeMs).toBeGreaterThan(30_000); + }); + + it("ignores a subdirectory, which is not a capture", async () => { + await mkdir(`${DIRECTORY}/nested`, { recursive: true }); + + await expect(listSnipFiles()).resolves.toEqual([]); + }); +}); + +describe("removeSnipFiles", () => { + const DIRECTORY = "/tmp/lam-agent-support/snips"; + const OUTSIDE = "/tmp/lam-agent-not-a-snip.png"; + + beforeEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + await mkdir(DIRECTORY, { recursive: true }); + await writeFile(OUTSIDE, "do not delete me"); + }); + + afterEach(async () => { + await rm(DIRECTORY, { recursive: true, force: true }); + await rm(OUTSIDE, { force: true }); + }); + + it("deletes a capture inside the snip directory and counts it", async () => { + const path = `${DIRECTORY}/one.png`; + await writeFile(path, "png"); + + await expect(removeSnipFiles([path])).resolves.toBe(1); + await expect(access(path)).rejects.toThrow(); + }); + + it("counts a file that was already gone, because the question is whether it is still there", async () => { + await expect(removeSnipFiles([`${DIRECTORY}/never-existed.png`])).resolves.toBe(1); + }); + + it("refuses a path outside the snip directory and does not count it", async () => { + // These paths come out of stored JSON, which is data rather than code, so + // this must not be a general-purpose delete-any-file helper. + await expect(removeSnipFiles([OUTSIDE])).resolves.toBe(0); + + await expect(access(OUTSIDE)).resolves.toBeUndefined(); + }); + + it("refuses a path that climbs out of the directory", async () => { + await expect(removeSnipFiles([`${DIRECTORY}/../../lam-agent-not-a-snip.png`])).resolves.toBe(0); + + await expect(access(OUTSIDE)).resolves.toBeUndefined(); + }); + + it("refuses the directory itself", async () => { + await expect(removeSnipFiles([DIRECTORY])).resolves.toBe(0); + + await expect(access(DIRECTORY)).resolves.toBeUndefined(); + }); + + it("keeps going when one path in the batch is refused, and counts only the one it acted on", async () => { + const inside = `${DIRECTORY}/one.png`; + await writeFile(inside, "png"); + + await expect(removeSnipFiles([OUTSIDE, inside])).resolves.toBe(1); + + await expect(access(inside)).rejects.toThrow(); + await expect(access(OUTSIDE)).resolves.toBeUndefined(); + }); +}); diff --git a/src/lib/agent.ts b/src/lib/agent.ts index 3f3918b..b757ba5 100644 --- a/src/lib/agent.ts +++ b/src/lib/agent.ts @@ -1,4 +1,4 @@ -import { MAX_TOOL_ITERATIONS } from "./constants"; +import { DEFAULT_MAX_TOOL_ROUNDS, MAX_REPEATED_TOOL_CALLS } from "./constants"; import { streamChat } from "./ollama"; import { executeTool, toolLabel, type ToolContext } from "./tools"; import type { ApiMessage, ToolCall, ToolCallRecord, ToolDefinition } from "./types"; @@ -14,6 +14,11 @@ export interface AgentEvents { onContentDelta?: (delta: string) => void; onThinkingDelta?: (delta: string) => void; onToolCall?: (record: ToolCallRecord) => void; + /** + * A tool is about to run, with how many times this turn has already made the + * identical call, so the UI can tell progress from a rut. + */ + onToolStart?: (name: string, label: string, count: number) => void; /** A short line for the UI's activity indicator, e.g. "Searching the web". */ onStatus?: (status: string) => void; } @@ -26,6 +31,16 @@ export interface AgentOptions { think?: string; messages: ApiMessage[]; tools: ToolDefinition[]; + /** + * Rounds of tool calls this turn may spend before it has to answer. + * + * Omitted means {@link DEFAULT_MAX_TOOL_ROUNDS}, and `null` means no cap at + * all. Unlimited is spelled as an explicit `null` rather than as an omitted + * option because the opposite arrangement makes "no cap" the default for a + * function whose whole job is to stop looping, and a caller that forgets to + * pass a preference should not be the one that removes the guard. + */ + maxToolRounds?: number | null; toolContext: Omit & { now?: Date }; signal?: AbortSignal; events?: AgentEvents; @@ -59,6 +74,31 @@ function normalizeToolCall(call: ToolCall): { name: string; arguments: Record }; } +/** + * A stable key for "the same call". + * + * Keys are sorted so that a model which writes the same arguments in a different + * order still counts as repeating itself, which is the shape the rut usually + * takes. Values are compared as JSON, so two searches for the same query count as + * one call, which is the point: the second one will not return anything the first + * did not. + */ +function callSignature(name: string, args: Record): string { + const sorted = Object.keys(args) + .sort() + .map((key) => [key, args[key]] as const); + + return `${name}(${JSON.stringify(sorted)})`; +} + +/** Why the tool loop stopped short: the budget ran out, or the model got stuck. */ +type StopReason = { kind: "budget"; rounds: number } | { kind: "repeat"; name: string; count: number }; + +/** Sent to the model in place of a tool result, so the protocol stays valid and it learns why. */ +function repeatedNotice(name: string, count: number): string { + return `Refused: this turn has now asked for \`${name}\` with identical arguments ${count} times, so running it again will not return anything new. No more tool calls are available. Answer with what you have already gathered.`; +} + /** * The last word, when the tool budget runs out. * @@ -72,14 +112,36 @@ function normalizeToolCall(call: ToolCall): { name: string; arguments: Record { const conversation: ApiMessage[] = [...options.messages]; const toolCalls: ToolCallRecord[] = []; + /** `Infinity` needs no special case: the loop simply never reaches its own end. */ + const rounds = + options.maxToolRounds === null ? Infinity : (options.maxToolRounds ?? DEFAULT_MAX_TOOL_ROUNDS); + /** How many times this turn has asked for each identical call. */ + const repeats = new Map(); let thinking = ""; /** * The most recent preamble the model wrote. @@ -89,8 +151,9 @@ export async function runAgent(options: AgentOptions): Promise { * something to fall back on if the closing round produces nothing. */ let lastPreamble = ""; + let stopped: StopReason | undefined; - for (let iteration = 0; iteration < MAX_TOOL_ITERATIONS; iteration++) { + for (let iteration = 0; iteration < rounds; iteration++) { let content = ""; let streamedThinking = ""; let pending: ToolCall[] | undefined; @@ -127,6 +190,8 @@ export async function runAgent(options: AgentOptions): Promise { conversation.push({ role: "assistant", content, tool_calls: pending }); + let repeated: { name: string; count: number } | undefined; + for (const rawCall of pending) { const { name, arguments: args } = normalizeToolCall(rawCall); @@ -140,6 +205,32 @@ export async function runAgent(options: AgentOptions): Promise { continue; } + const signature = callSignature(name, args); + const count = (repeats.get(signature) ?? 0) + 1; + repeats.set(signature, count); + + // Every tool call in a round needs a result, so a refused one is answered + // with the refusal rather than skipped: dropping it would leave the next + // request carrying a tool call the protocol says must be answered. + if (count >= MAX_REPEATED_TOOL_CALLS) { + repeated = { name, count }; + + const refusal = repeatedNotice(name, count); + const record: ToolCallRecord = { + name, + arguments: args, + result: refusal, + isError: true, + repeated: true, + }; + + conversation.push({ role: "tool", content: refusal, tool_name: name }); + toolCalls.push(record); + options.events?.onToolCall?.(record); + continue; + } + + options.events?.onToolStart?.(name, toolLabel(name), count); options.events?.onStatus?.(toolLabel(name)); const result = await executeTool(name, args, { @@ -161,15 +252,26 @@ export async function runAgent(options: AgentOptions): Promise { conversation.push({ role: "tool", content: result.content, tool_name: name }); } + + // The rest of the round still ran, because every call in it needs an answer, + // but the turn is over: the model is going in circles and one more round + // would only add one more circle. + if (repeated) { + stopped = { kind: "repeat", name: repeated.name, count: repeated.count }; + break; + } } - // Budget spent, so the turn gets one closing round with the tools taken away. + // The loop is done, either because the budget is spent or because the model got + // stuck. Either way the turn gets one closing round with the tools taken away. // Ending on an answer drawn from what was gathered beats ending on a notice // about the loop itself, and the model is the only one that can say what it // did and did not manage to confirm. + const reason: StopReason = stopped ?? { kind: "budget", rounds }; + options.events?.onStatus?.("Writing the answer"); - conversation.push({ role: "user", content: CLOSING_INSTRUCTION }); + conversation.push({ role: "user", content: closingInstruction(reason) }); let closing = ""; @@ -194,7 +296,7 @@ export async function runAgent(options: AgentOptions): Promise { } return { - content: closing.trim() || lastPreamble.trim() || exhaustedNotice(MAX_TOOL_ITERATIONS), + content: closing.trim() || lastPreamble.trim() || exhaustedNotice(reason), thinking, toolCalls, }; diff --git a/src/lib/cleanup.ts b/src/lib/cleanup.ts new file mode 100644 index 0000000..b87e730 --- /dev/null +++ b/src/lib/cleanup.ts @@ -0,0 +1,72 @@ +import { Toast, showToast } from "@vicinae/api"; +import { describeRemoval } from "./retention"; +import { clearSessions, cullOrphanedSnips, sweepExpiredSessions } from "./sessions"; + +/** + * The tidy-ups, and telling the user what they did. + * + * Split out from `retention.ts` so that file can stay a pure decision with every + * side effect injected, and out of the commands so that four call sites do not + * each carry their own copy of the same toast. This is the one place that knows + * both what was deleted and how to say it. + * + * Two audiences live here, and they want opposite things. The background sweep + * stays silent unless it did something, because a notification on every launch is + * one people learn to dismiss without reading. The two actions in the setup screen + * always answer, because the user pressed a button and silence would read as a + * failure. + */ + +/** + * The retention sweep, run on the way into a command. + * + * Silent when nothing was deleted. + */ +export async function runAutoDelete(options: { maxAgeMs?: number; keepId?: string }): Promise { + const outcome = await sweepExpiredSessions(options); + const summary = describeRemoval(outcome); + if (!summary) return; + + await showToast({ + style: Toast.Style.Success, + title: "Cleared out old conversations", + message: summary, + }); +} + +/** Deletes every conversation, and every screenshot they captured. */ +export async function clearAllChats(): Promise { + const counts = await clearSessions(); + + await showToast({ + style: Toast.Style.Success, + title: "Cleared out all conversations", + message: describeRemoval(counts) ?? "there was nothing to delete", + }); +} + +/** + * Deletes the captures no conversation refers to any more. + * + * The failure case is a refusal, not a failure to delete: when a conversation + * cannot be read, there is no way to know which files are still in use, and + * guessing there destroys real images. So it says so instead. + */ +export async function clearOrphanedSnips(): Promise { + const removed = await cullOrphanedSnips(); + + if (removed === undefined) { + await showToast({ + style: Toast.Style.Failure, + title: "Could not check which photos are in use", + message: "A conversation could not be read, so nothing was deleted.", + }); + return; + } + + await showToast({ + style: Toast.Style.Success, + title: "Cleared out unattached photos", + message: describeRemoval({ sessions: 0, files: removed }) ?? "there was nothing to delete", + }); +} diff --git a/src/lib/components/ChatView.tsx b/src/lib/components/ChatView.tsx index 89eee43..52395bf 100644 --- a/src/lib/components/ChatView.tsx +++ b/src/lib/components/ChatView.tsx @@ -1,7 +1,8 @@ import { Action, ActionPanel, Icon, LaunchType, List, launchCommand } from "@vicinae/api"; import { useCallback, useEffect, useMemo, useState } from "react"; import { readAttachment } from "../attachments"; -import { useChat } from "../hooks/useChat"; +import { runAutoDelete } from "../cleanup"; +import { useChat, type ToolActivity } from "../hooks/useChat"; import { useVisionSupport } from "../hooks/useVision"; import { buildThreadMarkdown, renderAttachments, type TranscriptOptions } from "../markdown"; import { getLamPreferences } from "../preferences"; @@ -61,6 +62,18 @@ const DEFAULT_IMAGE_PROMPT = "What is in this image?"; /** The reminder shown while a snip is attached but not yet sent. */ const PENDING_NOTE = "_Ask a question about it, or press Return to send it as it is._"; +/** + * The line that shows what Lam is doing right now. + * + * A count on its own reads as progress, so the second time round says what it is: + * the same call, asked again. That is the visible half of the loop guard in + * `agent.ts`, and it is the only warning the user gets before a turn is cut short. + */ +function describeActivity(activity: ToolActivity): string { + if (activity.count <= 1) return `_${activity.label}…_`; + return `_${activity.label} · the same call again (${activity.count})_`; +} + export interface ChatViewProps { /** Opens a specific conversation instead of resuming the latest one. */ initialSession?: Session; @@ -92,6 +105,7 @@ export function ChatView({ const chat = useChat({ preferences, initialSession }); const { replaceSession } = chat; const { canSeeImages } = useVisionSupport(preferences); + const { activity } = chat; const [input, setInput] = useState(initialPrompt ?? ""); const [summaries, setSummaries] = useState(undefined); @@ -110,10 +124,30 @@ export function ChatView({ .catch(() => setSummaries([])); }, []); - // Refreshed after each turn so the sidebar picks up a newly created chat. + // Refreshed after each turn so the sidebar picks up a newly created chat, and + // swept at the same time: a turn is exactly when a conversation's age changes, + // and waiting for the next launch to collect an expired one would mean showing + // the user a conversation that has already been deleted. useEffect(() => { - reload(); - }, [reload, chat.session.updatedAt]); + // Not until startup has finished, because until then the open conversation is + // still the placeholder one and the real conversation being restored has no + // id to be exempted by. Sweeping first could delete the chat on its way to + // the screen. + if (startup !== "ready") return; + + let cancelled = false; + + void (async () => { + // The open conversation is exempt: it is about to be rendered, and being + // the one it opens by default is the clearest sign it is still in use. + await runAutoDelete({ maxAgeMs: preferences.autoDeleteMs, keepId: chat.session.id }); + if (!cancelled) reload(); + })(); + + return () => { + cancelled = true; + }; + }, [startup, chat.session.id, chat.session.updatedAt, preferences.autoDeleteMs, reload]); /** * Everything that has to happen before the first render, in one place. @@ -188,6 +222,12 @@ export function ChatView({ // The preview comes first so the captured image is visible before the user // decides what to ask about it, rather than only after sending. pending ? `${renderAttachments([pending])}\n\n${PENDING_NOTE}` : undefined, + // Deliberately not gated on the Show Tool Calls preference. That setting is + // about the finished transcript, where a list of names adds nothing. This + // is about the turn in flight, where the same tool coming round a second + // time is the difference between a long answer and a stuck one, and hiding + // it would leave the user watching a spinner they cannot interpret. + activity ? describeActivity(activity) : undefined, ].filter((note): note is string => Boolean(note)); // The placeholder is what tells a user that the search bar is the composer. @@ -198,7 +238,7 @@ export function ChatView({ thread.length === 0 ? empty : buildThreadMarkdown(thread, transcriptOptions); return [...notes, transcript].filter((part) => part.length > 0).join("\n\n"); - }, [launchNote, snipNote, pending, thread, transcriptOptions]); + }, [launchNote, snipNote, pending, activity, thread, transcriptOptions]); const rows: SessionRow[] = useMemo(() => { const base = summaries ?? []; @@ -277,7 +317,7 @@ export function ChatView({ const hasPrompt = input.trim().length > 0 || pending !== undefined; - const stopAction = ; + const stopAction = ; const sendAction = send(input)} />; const newChatAction = ( diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 521afb4..4a73cf5 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -16,8 +16,32 @@ export const STORAGE_SESSION_PREFIX = "lam.session."; * preamble the model had written was discarded, and the turn ended with a notice * about the limit rather than an answer. The closing round in `agent.ts` now * turns that into an answer, and this is the room it gets to gather first. + * + * A default rather than a ceiling: the Tool Call Limit preference overrides it, + * and "Unlimited" removes it entirely. */ -export const MAX_TOOL_ITERATIONS = 8; +export const DEFAULT_MAX_TOOL_ROUNDS = 8; + +/** + * How many times one turn may ask for the exact same call before the loop gives + * up on it. + * + * A model stuck in a rut repeats itself with identical arguments, so the repeat + * is the signal: the third time it asks the same question it is not going to + * learn anything new, and the only thing a fourth round buys is another round. + * This is the backstop the rounds cap cannot be, because the cap can be set to + * Unlimited. + */ +export const MAX_REPEATED_TOOL_CALLS = 3; + +/** + * How long a snip must sit unreferenced before the orphan sweep collects it. + * + * Snips are written to disk before the chat view opens, and a capture the user + * never sends is stored nowhere. The wait is what keeps the sweep from deleting + * the file out from under a snip the user is still deciding what to ask about. + */ +export const SNIP_ORPHAN_GRACE_MS = 24 * 60 * 60 * 1000; /** Oldest sessions past this count are evicted, along with their storage keys. */ export const MAX_SESSIONS = 100; diff --git a/src/lib/hooks/useChat.ts b/src/lib/hooks/useChat.ts index 0722d95..f2701e2 100644 --- a/src/lib/hooks/useChat.ts +++ b/src/lib/hooks/useChat.ts @@ -35,6 +35,19 @@ export interface UseChatOptions { initialSession?: Session; } +/** Which tool is running right now, and how stale it is getting. */ +export interface ToolActivity { + /** Human-readable label, e.g. "Searching the web". */ + label: string; + /** + * How many times this turn has now started this *identical* call. + * + * Counted by the agent over name and arguments together, so two different + * searches are two tools doing their job rather than a repeat. + */ + count: number; +} + export interface UseChat { session: Session; isStreaming: boolean; @@ -42,6 +55,15 @@ export interface UseChat { streamingThinking: string; /** Short activity line such as "Searching the web", shown while a turn is in flight. */ status?: string; + /** + * The tool running right now. + * + * Separate from {@link UseChat.status} because `status` goes quiet as soon as + * the answer starts arriving, and the whole point of this one is to stay on + * screen: a model that has started circling looks identical to a model that is + * working, unless the tool and its repeat count stay visible. + */ + activity?: ToolActivity; error?: string; ask: (prompt: string, attachment?: PendingAttachment) => void; cancel: () => void; @@ -58,6 +80,7 @@ export function useChat(options: UseChatOptions): UseChat { const [streamingResponse, setStreamingResponse] = useState(""); const [streamingThinking, setStreamingThinking] = useState(""); const [status, setStatus] = useState(undefined); + const [activity, setActivity] = useState(undefined); const [error, setError] = useState(undefined); const sessionRef = useRef(session); @@ -149,6 +172,7 @@ export function useChat(options: UseChatOptions): UseChat { setStreamingResponse(""); setStreamingThinking(""); setError(undefined); + setActivity(undefined); setStatusValue("Thinking"); liveRef.current = true; setIsStreaming(true); @@ -173,6 +197,10 @@ export function useChat(options: UseChatOptions): UseChat { apiKey: preferences.apiKey, model: preferences.model, think: preferences.think, + // `undefined` is "unlimited" in the preferences, and `null` is what the + // agent reads as "no cap": an omitted option there means the default + // cap, so the two conventions meet here. + maxToolRounds: preferences.maxToolRounds ?? null, messages, tools: toolsFor({ webEnabled: preferences.webEnabled }), toolContext: { @@ -195,6 +223,7 @@ export function useChat(options: UseChatOptions): UseChat { scheduleFlush(); }, onStatus: (value) => setStatusValue(value), + onToolStart: (_name, label, count) => setActivity({ label, count }), onToolCall: (record) => { toolCalls = [...toolCalls, record]; }, @@ -232,6 +261,7 @@ export function useChat(options: UseChatOptions): UseChat { commit(updated); setIsStreaming(false); setStatusValue(undefined); + setActivity(undefined); setError(failure); flushNow(); @@ -249,6 +279,7 @@ export function useChat(options: UseChatOptions): UseChat { liveRef.current = false; setIsStreaming(false); setStatusValue(undefined); + setActivity(undefined); setError(undefined); setStreamingResponse(""); setStreamingThinking(""); @@ -274,6 +305,7 @@ export function useChat(options: UseChatOptions): UseChat { streamingResponse, streamingThinking, status, + activity, error, ask, cancel, diff --git a/src/lib/markdown.ts b/src/lib/markdown.ts index 184f831..2998031 100644 --- a/src/lib/markdown.ts +++ b/src/lib/markdown.ts @@ -60,7 +60,13 @@ export function quote(text: string): string { /** One italic line, well below the weight of the answer it belongs to. */ function renderTools(message: ThreadMessage): string { const names = message.toolCalls - .map((call) => `\`${call.name}\`${call.isError ? " (failed)" : ""}`) + .map((call) => { + const name = `\`${call.name}\``; + // A refused repeat did not fail, so it is not called a failure: it is the + // loop noticing it was going in circles, which is worth saying plainly. + if (call.repeated) return `${name} (repeated, stopped)`; + return call.isError ? `${name} (failed)` : name; + }) .join(", "); return `_Tools: ${names}_`; @@ -92,7 +98,17 @@ function renderMessage(message: ThreadMessage, options: TranscriptOptions): stri // something went wrong and should not be skimmed past. if (message.error) sections.push("", `**Error:** ${message.error}`); - if (options.showToolCalls && message.toolCalls.length > 0) { + /** + * Tool calls are a display preference, with one exception: a call that failed, + * or that was refused because it kept repeating, is always listed. Those are + * the two cases where hiding the line would leave the user with an answer they + * cannot account for, and a model stuck in a loop is exactly the thing they + * need to see. + */ + const showTools = + options.showToolCalls || message.toolCalls.some((call) => call.isError || call.repeated); + + if (showTools && message.toolCalls.length > 0) { sections.push("", renderTools(message)); } diff --git a/src/lib/preferences.ts b/src/lib/preferences.ts index 2ec2c99..c2e76bb 100644 --- a/src/lib/preferences.ts +++ b/src/lib/preferences.ts @@ -1,5 +1,5 @@ import { getPreferenceValues } from "@vicinae/api"; -import { DEFAULT_HOST, DEFAULT_MODEL } from "./constants"; +import { DEFAULT_HOST, DEFAULT_MAX_TOOL_ROUNDS, DEFAULT_MODEL } from "./constants"; /** * Preferences, read and normalised. @@ -22,6 +22,8 @@ export interface LamPreferences { showToolCalls?: boolean; showReasoning?: boolean; resumeTimeout?: string; + autoDeleteAfter?: string; + maxToolRounds?: string; } /** Preferences with every optional value filled in, so callers never branch on undefined. */ @@ -49,6 +51,22 @@ export interface ResolvedPreferences { showReasoning: boolean; /** Idle time after which opening the command starts a fresh chat. Undefined means never. */ resumeTimeoutMs?: number; + /** + * Age past which a conversation is deleted the next time a command runs. + * + * Undefined means the sweep is off entirely, which is the default. Every path + * that cannot answer the question confidently resolves here, so a preference + * that is missing, empty, or unrecognised deletes nothing. + */ + autoDeleteMs?: number; + /** + * Rounds of tool calls one turn may spend. Undefined means unlimited. + * + * Note the opposite convention to {@link ResolvedPreferences.autoDeleteMs}: + * there, "unrecognised" falls back to doing nothing, and here it falls back to + * the cap. See {@link resolveMaxToolRounds}. + */ + maxToolRounds?: number; } const MINUTE = 60_000; @@ -68,18 +86,82 @@ export function resolveResumeTimeout(value: string | undefined): number | undefi return RESUME_TIMEOUTS[(value ?? "").trim()]; } +/** Keep in sync with the `autoDeleteAfter` dropdown in `package.json`. */ +const AUTO_DELETE_WINDOWS: Record = { + "1d": DAY, + "7d": 7 * DAY, + "30d": 30 * DAY, + "90d": 90 * DAY, +}; + +/** + * `undefined` for "disabled" and for anything unrecognised. + * + * Nothing here can turn an unrecognised value into a deletion: the fallback is + * the off switch, which is the only safe reading of a preference this codebase + * does not understand. + */ +export function resolveAutoDelete(value: string | undefined): number | undefined { + return AUTO_DELETE_WINDOWS[(value ?? "").trim().toLowerCase()]; +} + +/** + * Round limits, keyed by the preference values in `package.json`. + * + * `undefined` here means unlimited, so the table cannot be indexed the way the + * other two are: an absent key and an "unlimited" key would be indistinguishable. + */ +const TOOL_ROUNDS: Record = { + "4": 4, + "8": 8, + "16": 16, + unlimited: undefined, +}; + +/** + * The tool-call round cap, where `undefined` means unlimited. + * + * Falls back to the default cap rather than to unlimited, unlike + * {@link resolveResumeTimeout} and {@link resolveAutoDelete}, which both treat an + * unrecognised value as "off". Those two are safe when they do nothing; this one + * guards a loop, so the safe reading of a value this codebase does not recognise + * is the cap and not the absence of one. + */ +export function resolveMaxToolRounds(value: string | undefined): number | undefined { + const key = (value ?? "").trim().toLowerCase(); + return key in TOOL_ROUNDS ? TOOL_ROUNDS[key] : DEFAULT_MAX_TOOL_ROUNDS; +} + +/** Human-readable form of {@link ResolvedPreferences.autoDeleteMs}, for the setup screen. */ +export function describeAutoDelete(milliseconds: number | undefined): string { + if (milliseconds === undefined) return "Disabled (conversations are kept)"; + + return `After ${describeSpan(milliseconds)}`; +} + +/** Human-readable form of {@link ResolvedPreferences.maxToolRounds}, for the setup screen. */ +export function describeMaxToolRounds(rounds: number | undefined): string { + if (rounds === undefined) return "Unlimited (no cap on tool calls)"; + return `${rounds} round${rounds === 1 ? "" : "s"} per answer`; +} + /** Human-readable form of {@link ResolvedPreferences.resumeTimeoutMs}, for the setup screen. */ export function describeResumeTimeout(milliseconds: number | undefined): string { if (milliseconds === undefined) return "Never (always reopen the last chat)"; + return `After ${describeSpan(milliseconds)}`; +} + +/** "15 minutes", "6 hours", "7 days". Shared so the two windows phrase themselves the same way. */ +function describeSpan(milliseconds: number): string { const minutes = Math.round(milliseconds / MINUTE); - if (minutes < 60) return `After ${minutes} minutes`; + if (minutes < 60) return `${minutes} minutes`; const hours = Math.round(minutes / 60); - if (hours < 24) return `After ${hours} hour${hours === 1 ? "" : "s"}`; + if (hours < 24) return `${hours} hour${hours === 1 ? "" : "s"}`; const days = Math.round(hours / 24); - return `After ${days} day${days === 1 ? "" : "s"}`; + return `${days} day${days === 1 ? "" : "s"}`; } export function resolvePreferences(raw: LamPreferences): ResolvedPreferences { @@ -102,6 +184,9 @@ export function resolvePreferences(raw: LamPreferences): ResolvedPreferences { // Falls back to the manifest's declared default, so a missing preference // behaves the same whether Vicinae substitutes it or not. resumeTimeoutMs: resolveResumeTimeout(raw.resumeTimeout ?? "1h"), + // Defaults to off, the one setting here that destroys data. + autoDeleteMs: resolveAutoDelete(raw.autoDeleteAfter ?? "disabled"), + maxToolRounds: resolveMaxToolRounds(raw.maxToolRounds ?? String(DEFAULT_MAX_TOOL_ROUNDS)), }; } diff --git a/src/lib/retention.ts b/src/lib/retention.ts new file mode 100644 index 0000000..e0fa4d1 --- /dev/null +++ b/src/lib/retention.ts @@ -0,0 +1,180 @@ +import { SNIP_ORPHAN_GRACE_MS } from "./constants"; +import type { SessionSummary } from "./types"; + +/** + * What gets deleted, and when. + * + * Two separate jobs live here, because they are the same job from the user's + * point of view ("Lam keeps my history tidy") but nothing like each other in code: + * + * - Conversations unused for longer than the retention window. + * - Screenshot files nothing refers to any more. A capture is written to disk + * before the chat view opens, so a snip the user never sent belongs to no + * conversation at all and no amount of session deletion would ever collect it. + * + * Both are pure decisions here, with every side effect injected, following + * `startup.ts`: the alternative is discovering from a screenshot that a sweep + * deleted a conversation it should not have. Imports nothing from Vicinae, so it + * unit tests with no runtime. + * + * ## The bias is always towards deleting nothing + * + * This is the only code in the extension that destroys data, so every ambiguity + * resolves towards leaving it alone. An unrecognised retention value disables the + * sweep rather than guessing at a window; a snip is only collected once it is + * both unreferenced *and* older than a grace period; and when the referenced set + * cannot be established at all, the orphan pass is skipped rather than run + * against a set it knows to be incomplete. + */ + +/** One file in the snip directory, with the age the sweep judges it by. */ +export interface SnipFile { + path: string; + /** Last modification time, which for a capture is when it was taken. */ + mtimeMs: number; +} + +/** + * Conversations unused for at least `maxAgeMs`, oldest first. + * + * Age is measured from `updatedAt`, not `createdAt`, so a conversation started + * last month but used this morning is not old. The open conversation is excluded + * outright: it is about to be rendered, and deleting the thing the user is + * looking at is the one outcome that would be indistinguishable from a bug. + * + * `undefined` means the sweep is off, which is the default. Every value that + * cannot be resolved to a window arrives here as `undefined`. + */ +export function expiredSessions( + summaries: SessionSummary[], + maxAgeMs: number | undefined, + now: number, + keepId?: string, +): SessionSummary[] { + if (maxAgeMs === undefined) return []; + + return summaries.filter((summary) => summary.id !== keepId && now - summary.updatedAt >= maxAgeMs); +} + +/** + * Snip files that nothing refers to and that have waited out the grace period. + * + * The wait is the whole safety story. A snip is on disk seconds after it is + * taken and stays unreferenced until the user sends it, so without a grace + * period the sweep would delete the file out from under a question the user is + * still typing. A day is far longer than any real pending snip and far shorter + * than the point at which the file matters. + */ +export function orphanedSnips( + files: SnipFile[], + referenced: Iterable, + now: number, + graceMs: number = SNIP_ORPHAN_GRACE_MS, +): SnipFile[] { + const kept = new Set(referenced); + return files.filter((file) => !kept.has(file.path) && now - file.mtimeMs >= graceMs); +} + +export interface SweepInput { + /** Age past which an unused conversation is deleted. `undefined` disables everything. */ + maxAgeMs?: number; + now: number; + /** The conversation currently open, which is never deleted. */ + keepId?: string; + /** How long an unreferenced snip must sit before it is collected. */ + snipGraceMs?: number; +} + +/** The side effects, injected so nothing here touches storage or the file system directly. */ +export interface SweepPorts { + listSessions: () => Promise; + /** Deletes conversations by id and returns every image path they referred to. */ + deleteSessions: (ids: string[]) => Promise; + /** + * Image paths still referred to by conversations that remain, or `undefined` + * when that cannot be established. + * + * `undefined` is not the same as an empty array and must not be treated as + * one: it means the referenced set is unknown, and a set that is merely + * incomplete looks exactly like a pile of orphans. + */ + referencedAttachments: () => Promise; + listSnipFiles: () => Promise; + /** Returns how many files it acted on. The sweep ignores the count; the setup screen reports it. */ + removeSnipFiles: (paths: string[]) => Promise; +} + +export interface SweepOutcome { + /** Conversations deleted. */ + sessions: number; + /** Image files deleted, from deleted conversations and unreferenced orphans alike. */ + files: number; +} + +const NOTHING_DONE: SweepOutcome = { sessions: 0, files: 0 }; + +/** + * What to tell the user was removed, or `undefined` when nothing was. + * + * The count names the thing rather than saying "items", because "2 conversations + * and 4 screenshots" is the sentence a person wants to read before deciding + * whether the tidy-up did what they expected. The background sweep treats + * `undefined` as "stay silent"; an action the user pressed has to answer even + * when the answer is nothing, so those callers spell that case out themselves. + */ +export function describeRemoval(counts: SweepOutcome): string | undefined { + const parts: string[] = []; + if (counts.sessions > 0) { + parts.push(`${counts.sessions} conversation${counts.sessions === 1 ? "" : "s"}`); + } + if (counts.files > 0) { + parts.push(`${counts.files} screenshot${counts.files === 1 ? "" : "s"}`); + } + + return parts.length > 0 ? parts.join(" and ") : undefined; +} + +/** + * Runs both passes. + * + * Disabled means disabled. When `maxAgeMs` is undefined nothing is read and + * nothing is written, which also keeps the common case off the hot path: this + * runs on every command open and after every answer. + */ +export async function sweepExpired(input: SweepInput, ports: SweepPorts): Promise { + if (input.maxAgeMs === undefined) return NOTHING_DONE; + + const expired = expiredSessions(await ports.listSessions(), input.maxAgeMs, input.now, input.keepId); + const fromSessions = + expired.length > 0 ? await ports.deleteSessions(expired.map((summary) => summary.id)) : []; + + const referenced = await ports.referencedAttachments(); + + // An incomplete referenced set would make every screenshot it does not happen + // to mention look like an orphan, so an unknown set means neither pass runs. + const kept = referenced === undefined ? undefined : new Set(referenced); + + // A deleted conversation's files are removed explicitly rather than by the + // grace-period rule, because a screenshot taken five minutes before its + // conversation is deleted should go with it rather than sit on disk for a day. + // A shared path is the exception: two conversations can refer to the same + // capture, and the survivor still needs it. + const fromDeleted = kept === undefined ? [] : fromSessions.filter((path) => !kept.has(path)); + + const orphans = + kept === undefined + ? [] + : orphanedSnips( + await ports.listSnipFiles(), + kept, + input.now, + input.snipGraceMs ?? SNIP_ORPHAN_GRACE_MS, + ); + + // Deduplicated because a file belonging to a deleted conversation can also + // satisfy the orphan rule once that conversation has left the index. + const paths = [...new Set([...fromDeleted, ...orphans.map((file) => file.path)])]; + if (paths.length > 0) await ports.removeSnipFiles(paths); + + return { sessions: expired.length, files: paths.length }; +} diff --git a/src/lib/schemas.ts b/src/lib/schemas.ts index 56b8f7b..f552461 100644 --- a/src/lib/schemas.ts +++ b/src/lib/schemas.ts @@ -12,6 +12,7 @@ export const ToolCallRecordSchema = z.object({ arguments: z.record(z.string(), z.unknown()), result: z.string(), isError: z.boolean(), + repeated: z.boolean().optional(), }); export const AttachmentSchema = z.object({ @@ -54,6 +55,10 @@ export const SessionSummarySchema = z.object({ updatedAt: z.number(), turnCount: z.number(), preview: z.string(), + // Optional rather than defaulted, so an index written before the orphan sweep + // existed is still readable. A default of `[]` would look identical to "this + // conversation has no screenshots", and the sweep would delete the files. + attachmentPaths: z.array(z.string()).optional(), }); export const SessionIndexSchema = z.array(SessionSummarySchema); diff --git a/src/lib/sessions.ts b/src/lib/sessions.ts index 75738a1..ea6f002 100644 --- a/src/lib/sessions.ts +++ b/src/lib/sessions.ts @@ -1,6 +1,8 @@ import { LocalStorage } from "@vicinae/api"; import { MAX_SESSIONS, MAX_STORED_CHARS_PER_TURN, MAX_TURNS_PER_SESSION, STORAGE_INDEX_KEY, STORAGE_SESSION_PREFIX, TITLE_MAX_LENGTH } from "./constants"; +import { orphanedSnips, sweepExpired, type SweepOutcome } from "./retention"; import { SessionIndexSchema, SessionSchema } from "./schemas"; +import { listSnipFiles, removeSnipFiles } from "./snip"; import type { Attachment, ChatTurn, Session, SessionSummary } from "./types"; import { titleFromPrompt, truncate } from "./utils"; @@ -41,9 +43,17 @@ function summarize(session: Session): SessionSummary { updatedAt: session.updatedAt, turnCount: session.turns.length, preview: lastTurn ? truncate(lastTurn.response || lastTurn.prompt, TITLE_MAX_LENGTH * 2) : "", + attachmentPaths: attachmentPathsOf(session), }; } +/** Every image path a conversation refers to, deduplicated and in turn order. */ +export function attachmentPathsOf(session: Session): string[] { + return [ + ...new Set(session.turns.flatMap((turn) => turn.attachments.map((attachment) => attachment.path))), + ]; +} + export function createSession(model: string, now: number = Date.now()): Session { return { id: newId(), @@ -161,17 +171,179 @@ export async function saveSession(session: Session): Promise { export async function deleteSession(id: string): Promise { const summaries = await listSessions(); - await writeIndex(summaries.filter((summary) => summary.id !== id)); + const survivors = summaries.filter((summary) => summary.id !== id); + const owned = await attachmentPathsFor([id], summaries); + + await writeIndex(survivors); try { await LocalStorage.removeItem(sessionKey(id)); } catch { // Already gone. } + + // Deleting a conversation takes its screenshots with it, the same as an + // expired one does. Without this a manual delete leaks image files that + // nothing will ever collect, and the orphan pass only runs while the retention + // setting is on. + await removeSnipFiles(await pathsSafeToDelete(owned, survivors)); } -export async function clearSessions(): Promise { +/** + * The paths a deleted conversation held that nothing else still needs. + * + * Ownership alone is not enough: two conversations can refer to the same + * capture, and removing the file would leave the survivor showing a broken + * thumbnail. When the surviving references cannot be established, nothing is + * removed. + */ +async function pathsSafeToDelete(owned: string[], survivors: SessionSummary[]): Promise { + if (owned.length === 0) return []; + + const kept = await pathsKeptBy(survivors); + return kept === undefined ? [] : owned.filter((path) => !kept.has(path)); +} + +/** + * Every image path the given conversations refer to, or `undefined` when that + * cannot be established. + * + * Index-backed, for the reason set out on `SessionSummary.attachmentPaths`: + * deriving this from the transcripts would mean a hundred JSON parses on every + * sweep, and the sweep runs after every answer. + * + * Entries written before that field existed are filled in from their transcripts + * and the index is written back once, so an upgrade pays for the repair on its + * first sweep rather than on every one. A transcript that will not load is the + * one case that returns `undefined` instead of an answer: its screenshots would + * look exactly like orphans, and deleting someone's images because a key failed + * to parse once is not a trade worth making. + */ +async function pathsKeptBy(summaries: SessionSummary[]): Promise | undefined> { + if (summaries.every((summary) => summary.attachmentPaths !== undefined)) { + return new Set(summaries.flatMap((summary) => summary.attachmentPaths ?? [])); + } + + let unknown = false; + + const repaired = await Promise.all( + summaries.map(async (summary): Promise => { + if (summary.attachmentPaths !== undefined) return summary; + + const session = await loadSession(summary.id); + if (!session) { + unknown = true; + return summary; + } + + return { ...summary, attachmentPaths: attachmentPathsOf(session) }; + }), + ); + + if (unknown) return undefined; + + try { + await writeIndex(repaired); + } catch { + // A failed repair only means the next sweep reads these transcripts again. + } + + return new Set(repaired.flatMap((summary) => summary.attachmentPaths ?? [])); +} + +/** + * The image paths held by the named conversations. + * + * Bodies are read before their keys are removed, because the turns are the only + * record of which screenshots belonged to them. A body that will not load + * contributes the paths its summary already knew about, which is one of the + * things `attachmentPaths` is stored for. + */ +async function attachmentPathsFor(ids: string[], summaries: SessionSummary[]): Promise { + if (ids.length === 0) return []; + + const wanted = new Set(ids); + const doomed = summaries.filter((summary) => wanted.has(summary.id)); + + const collected = await Promise.all( + doomed.map(async (summary) => { + const session = await loadSession(summary.id); + return session ? attachmentPathsOf(session) : (summary.attachmentPaths ?? []); + }), + ); + + return [...new Set(collected.flat())]; +} + +/** + * Deletes several conversations in one pass, returning the image paths they held. + * + * Separate from {@link deleteSession} rather than a loop over it because the + * sweep can expire dozens of conversations at once, and one index rewrite per + * conversation is dozens of writes to re-save a shrinking list. + * + * Bodies are read before their keys are removed, because the turns are the only + * record of which screenshots belonged to these conversations. + */ +export async function deleteSessions(ids: string[]): Promise { + if (ids.length === 0) return []; + + const removing = new Set(ids); const summaries = await listSessions(); + const paths = await attachmentPathsFor(ids, summaries); + + await writeIndex(summaries.filter((summary) => !removing.has(summary.id))); + + for (const id of ids) { + try { + await LocalStorage.removeItem(sessionKey(id)); + } catch { + // Already gone. + } + } + + // Returned rather than removed: the sweep that calls this has already listed + // the snip directory, and removing the files here would mean two passes over + // it for one tidy-up. + return paths; +} + +/** + * Image paths still referred to by a conversation that exists, or `undefined` + * when that cannot be established. + * + * See {@link pathsKeptBy}, which is where the decision actually lives. + */ +export async function referencedAttachments(): Promise { + const kept = await pathsKeptBy(await listSessions()); + return kept === undefined ? undefined : [...kept]; +} + +/** + * Runs the retention sweep against real storage and the real file system. + * + * The one place the pure decision in `retention.ts` meets its side effects, so a + * command entrypoint has a single thing to call. Resolves to a zero outcome + * rather than throwing: it runs on the way into a command the user is waiting + * for, and a tidy-up that fails is not worth an error message. + */ +export async function sweepExpiredSessions(input: { + maxAgeMs?: number; + now?: number; + keepId?: string; +}): Promise { + return sweepExpired( + { maxAgeMs: input.maxAgeMs, now: input.now ?? Date.now(), keepId: input.keepId }, + { listSessions, deleteSessions, referencedAttachments, listSnipFiles, removeSnipFiles }, + ).catch(() => ({ sessions: 0, files: 0 })); +} + +export async function clearSessions(): Promise { + const summaries = await listSessions(); + const owned = await attachmentPathsFor( + summaries.map((summary) => summary.id), + summaries, + ); for (const summary of summaries) { try { @@ -182,6 +354,32 @@ export async function clearSessions(): Promise { } await LocalStorage.removeItem(STORAGE_INDEX_KEY); + + // Every screenshot too, for the same reason a single delete takes its own. + // Nothing survives to share them with, so they are all safe to remove. + const files = await removeSnipFiles(await pathsSafeToDelete(owned, [])); + + return { sessions: summaries.length, files }; +} + +/** + * Deletes the captures no conversation refers to any more. + * + * The same rule the background sweep uses, so the two can never disagree about + * what counts as unattached: unreferenced, and older than the grace period. That + * wait matters more here than it does in the sweep, because this runs on demand + * while a chat could still be holding a snip the user has not sent yet. + * + * Returns how many files went, or `undefined` when the references could not be + * established. `undefined` is not zero: it means the answer is unknown, and the + * caller says so rather than reporting a clean bill of health it cannot support. + */ +export async function cullOrphanedSnips(now: number = Date.now()): Promise { + const referenced = await referencedAttachments(); + if (referenced === undefined) return undefined; + + const orphans = orphanedSnips(await listSnipFiles(), new Set(referenced), now); + return removeSnipFiles(orphans.map((file) => file.path)); } /** Recomputes a session's title from its first prompt, once there is a prompt worth naming it after. */ diff --git a/src/lib/snip.ts b/src/lib/snip.ts index 0406db4..3100cde 100644 --- a/src/lib/snip.ts +++ b/src/lib/snip.ts @@ -1,10 +1,13 @@ import { environment } from "@vicinae/api"; -import { join } from "node:path"; +import { readdir, rm, stat } from "node:fs/promises"; +import { isAbsolute, join, relative, resolve, sep } from "node:path"; import { CapturePermissionError, NoCaptureToolError } from "./capture"; +import type { SnipFile } from "./retention"; import { errorMessage } from "./utils"; /** - * The seam between the snip command and the chat command. + * The seam between the snip command and the chat command, and the directory they + * both live out of. * * The capture cannot happen inside the chat view. Vicinae shows the launcher * window when it activates a command, so a region selector started from a view @@ -12,6 +15,10 @@ import { errorMessage } from "./utils"; * a `no-view` command and handing the file over is what keeps the window out of * the way, which means the two commands need a small agreed contract. * + * The directory also belongs here because it is the only place that knows where + * captures go, and the retention sweep has to list and delete them without going + * through a capture. `listSnipFiles` and `removeSnipFiles` are that half. + * * Imports Vicinae for `environment.supportPath` only, and nothing else in the * logic layer depends on it. */ @@ -36,6 +43,81 @@ export function snipDirectory(): string { return join(environment.supportPath, "snips"); } +/** + * Every file in the snip directory, with the age the orphan sweep judges it by. + * + * Read from the directory rather than from a stored list of captures, because + * the directory is the only thing that cannot drift: a file whose record was + * lost to a crash, or written before the sweep existed, is still found here. + */ +export async function listSnipFiles(): Promise { + const directory = snipDirectory(); + + let entries; + try { + entries = await readdir(directory, { withFileTypes: true }); + } catch { + // No directory yet simply means nothing has been snipped. + return []; + } + + const found = await Promise.all( + entries + .filter((entry) => entry.isFile()) + .map(async (entry): Promise => { + const path = join(directory, entry.name); + try { + const stats = await stat(path); + return { path, mtimeMs: stats.mtimeMs }; + } catch { + // Deleted between the listing and the stat. + return undefined; + } + }), + ); + + return found.filter((file): file is SnipFile => file !== undefined); +} + +/** + * Deletes snip files, refusing anything outside the snip directory. + * + * The paths arrive from stored JSON, which is data rather than code and can be + * hand-edited or corrupted, so this cannot be a general-purpose delete-any-file + * helper. Resolving both sides and checking containment is what keeps an edited + * path from reaching `rm`, and a path that is refused is simply left alone. + * + * Best effort throughout: the sweep that calls this is a background tidy-up, so + * a file that is already gone or that the system will not release must not fail + * the rest of the pass. + * + * Returns how many paths it acted on, so a caller that reports back to the user + * can count rather than guess. A file that was already missing counts as removed, + * which is the honest answer to "is it still there". + */ +export async function removeSnipFiles(paths: string[]): Promise { + const directory = resolve(snipDirectory()); + let removed = 0; + + for (const path of paths) { + const target = resolve(path); + const inside = relative(directory, target); + + const contained = + inside !== "" && inside !== ".." && !inside.startsWith(`..${sep}`) && !isAbsolute(inside); + if (!contained) continue; + + try { + await rm(target, { force: true }); + removed += 1; + } catch { + // Nothing worth reporting: the file is either gone already or stuck. + } + } + + return removed; +} + /** * Reads the handoff payload. * diff --git a/src/lib/types.ts b/src/lib/types.ts index 7ce89f3..ea56e41 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -90,6 +90,14 @@ export interface ToolCallRecord { arguments: Record; result: string; isError: boolean; + /** + * Set when the call was refused because the turn had already made it twice. + * + * Stored rather than inferred from the message text, so the transcript can say + * what happened in its own words, and so a repeat that ended a turn stays + * visible even when the user has hidden tool calls from the transcript. + */ + repeated?: boolean; } /** One question and its answer. The unit the transcript is rendered from. */ @@ -125,6 +133,19 @@ export interface SessionSummary { updatedAt: number; turnCount: number; preview: string; + /** + * Every image path any turn in this conversation refers to. + * + * Kept here rather than derived from the turns for one reason: the orphan + * sweep needs to know which snips are still spoken for, and loading every + * transcript to find out would mean a hundred JSON parses on every turn. + * + * Absent on entries written before this field existed, which is deliberately + * not the same as an empty array: an absent value means "unknown", and the + * sweep resolves it by reading that one transcript rather than by assuming the + * conversation has no screenshots and deleting them. + */ + attachmentPaths?: string[]; } /** A model as reported by `GET /api/tags`. */ diff --git a/src/sessions.tsx b/src/sessions.tsx index bea6d38..de5289c 100644 --- a/src/sessions.tsx +++ b/src/sessions.tsx @@ -1,7 +1,9 @@ import { Action, ActionPanel, Clipboard, Icon, List, useNavigation } from "@vicinae/api"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; +import { runAutoDelete } from "./lib/cleanup"; import { ChatView } from "./lib/components/ChatView"; import { sessionToPlainText } from "./lib/markdown"; +import { getLamPreferences } from "./lib/preferences"; import { clearSessions, deleteSession, listSessions, loadSession } from "./lib/sessions"; import type { SessionSummary } from "./lib/types"; import { formatRelativeTime, truncate } from "./lib/utils"; @@ -16,6 +18,7 @@ import { formatRelativeTime, truncate } from "./lib/utils"; export default function SessionsCommand() { const { push } = useNavigation(); const [sessions, setSessions] = useState(undefined); + const preferences = useMemo(() => getLamPreferences(), []); const refresh = useCallback(() => { void listSessions() @@ -23,7 +26,22 @@ export default function SessionsCommand() { .catch(() => setSessions([])); }, []); - useEffect(refresh, [refresh]); + // Opening the list is the other moment the user is asking about their history, + // so it is the other moment expired conversations should be gone. No `keepId`: + // nothing is open here to spare. The refresh afterwards is what stops the list + // rendering a row that was just deleted from underneath it. + useEffect(() => { + let cancelled = false; + + void (async () => { + await runAutoDelete({ maxAgeMs: preferences.autoDeleteMs }); + if (!cancelled) refresh(); + })(); + + return () => { + cancelled = true; + }; + }, [preferences.autoDeleteMs, refresh]); const openSession = useCallback( (id: string) => { diff --git a/src/setup.tsx b/src/setup.tsx index 5443ca2..c98a445 100644 --- a/src/setup.tsx +++ b/src/setup.tsx @@ -1,6 +1,13 @@ -import { Action, ActionPanel, Icon, List, openExtensionPreferences } from "@vicinae/api"; -import { useCallback, useEffect, useMemo, useState } from "react"; -import { describeResumeTimeout, getLamPreferences, needsSetup } from "./lib/preferences"; +import { Action, ActionPanel, Alert, Icon, List, confirmAlert, openExtensionPreferences } from "@vicinae/api"; +import { useCallback, useEffect, useMemo, useState, type ReactNode } from "react"; +import { clearAllChats, clearOrphanedSnips, runAutoDelete } from "./lib/cleanup"; +import { + describeAutoDelete, + describeMaxToolRounds, + describeResumeTimeout, + getLamPreferences, + needsSetup, +} from "./lib/preferences"; import { listModels } from "./lib/ollama"; import { forgetGeoPlace } from "./lib/geo"; import { forgetVisionSupport, visionSupport, type VisionSupport } from "./lib/vision"; @@ -81,10 +88,58 @@ export default function SetupCommand() { if (!missingKey) void testConnection(); }, [missingKey, testConnection]); + // Opening any Lam command is a moment to collect expired conversations, and + // this is one of them. Nothing is open here, so nothing is exempt. + useEffect(() => { + void runAutoDelete({ maxAgeMs: preferences.autoDeleteMs }); + }, [preferences.autoDeleteMs]); + const openPreferences = useCallback(() => { void openExtensionPreferences(); }, []); + /** + * Confirms, then runs a destructive tidy-up. + * + * Both actions on this screen delete things that cannot be recovered: there is + * no trash to undo them from, and a conversation's screenshots go with it. That + * is worth one extra keystroke, and the alert says what will go rather than + * asking "are you sure" about nothing in particular. + */ + const confirmThen = useCallback( + async (options: { title: string; message: string }, run: () => Promise) => { + const confirmed = await confirmAlert({ + title: options.title, + message: options.message, + primaryAction: { title: "Delete", style: Alert.ActionStyle.Destructive }, + }); + + if (confirmed) await run(); + }, + [], + ); + + /** + * The action panel for a maintenance row. + * + * The row's own action comes first, because it is why the user is looking at + * the row, and the two read-only actions sit in their own section below so they + * can never be the default that Return fires. + */ + const maintenanceActions = (primary: ReactNode) => ( + + {primary} + + + void testConnection()} + /> + + + ); + const settingsActions = ( @@ -169,6 +224,18 @@ export default function SetupCommand() { subtitle={describeResumeTimeout(preferences.resumeTimeoutMs)} actions={settingsActions} /> + + + {/* + * Maintenance sits below the settings it acts on. Both rows are destructive + * rather than settings, so each builds its own action panel instead of + * sharing the one above: these are the only two things on this screen that + * delete anything, and neither should be one keystroke away from a row the + * user was only reading. + */} + + + void confirmThen( + { + title: "Delete unattached photos?", + message: + "Screenshots that belong to no conversation will be deleted. Photos still attached to a chat are kept.", + }, + clearOrphanedSnips, + ) + } + />, + )} + /> + + void confirmThen( + { + title: "Delete every conversation?", + message: + "Every conversation and every screenshot it captured will be deleted. This cannot be undone.", + }, + clearAllChats, + ) + } + />, + )} + /> + + {error ? (