diff --git a/changelogs/v2.6.2.md b/changelogs/v2.6.2.md new file mode 100644 index 00000000..cbad763d --- /dev/null +++ b/changelogs/v2.6.2.md @@ -0,0 +1,8 @@ +## What's new + +### Fixes +- **Cairn now warns when another device is on an older sync version.** Device Sync records the sync-protocol version each of your devices is using. When one of them reports a *lower* version than this device, Settings → Sync (and the mobile Sync screen) show a "device needs updating" hint — because an older version may not honour deletions, so a note you delete can reappear from it. Devices on the same version see nothing new. +- **Callouts now render inline in the editor.** In the note editor's live preview, `> [!note]`-style callouts show as the same styled callout you see in reading mode, and click into raw Markdown to edit. This was implemented earlier but switched off because the cursor landed in the wrong place when you clicked near or below a callout — text after one was effectively unselectable. The callout now mounts with its real height before the editor measures the surrounding lines, so positions below it stay accurate and you can edit around a callout normally. +- **A note deleted on another device is no longer offered for restore twice.** When you delete a note on one device and edit that same note on another before it has seen the deletion, Device Sync keeps your edit as a conflict copy — and previously it *also* listed the note as "deleted on another device" with a Restore button. Restoring then left you with two identical notes to clean up. Cairn now recognises that your edit is already safe in the conflict copy and points you to the conflicts view to resolve it, instead of offering a redundant restore. Notes deleted with no competing edit still show Restore as before. +- **Agent tool approvals read correctly on Windows and Linux.** The confirmation shown before the agent runs a tool described where the action reaches as "this Mac" regardless of platform; it now says "this device." +- **Reasoning ("thinking") models no longer break the chat.** Cairn used to cap every reply at 4,096 output tokens. A reasoning model counts its private thinking against that budget, so on a hard question it could spend the whole cap thinking and stop before writing any answer — leaving an empty reply. Worse, sending your next message then failed outright with an "invalid request" error, because the empty turn was replayed to the provider. Cairn now lets models finish naturally by default (no artificial cap), so they think as long as they need and still answer. A new **Max output tokens** control in Settings → AI lets you set a deliberate cap if you want to bound cost per reply; leave it on **Auto** (the default) for the fix. As a safety net, if a reply ever is cut off mid-thought, Cairn shows the captured reasoning instead of a blank message, and a stalled empty turn can no longer poison your next message. diff --git a/docs/plans/sync-lifecycle-hardening.md b/docs/plans/sync-lifecycle-hardening.md index 16b0baa6..e92cbf09 100644 --- a/docs/plans/sync-lifecycle-hardening.md +++ b/docs/plans/sync-lifecycle-hardening.md @@ -230,6 +230,30 @@ log stays a desktop diagnostic — on mobile the recovery path is what carries t ## 6. Risks & notes +### Wire-protocol versioning (✅ shipped v2.6.2) + +The oplog now carries a `v` field (`SYNC_PROTOCOL_VERSION`, currently **1**), stamped at the +export boundary (`exportOplog`), validated by the shared guard, and read by `applyRemote` into a +per-peer record in `sync_state` (`peer_protocol:`, monotonic). `listPeerProtocols()` +reports any peer whose version differs; a peer that is **behind** may not honour deletes, and both +desktop (Settings → Sync) and mobile (Sync screen) surface a "device needs updating" hint. + +**Skew contract:** +- A missing `v` means a pre-2.6.2 build → treated as protocol **1**. All 2.6.1 and earlier entries + read cleanly; no rewrite. +- **Bump `SYNC_PROTOCOL_VERSION` only** for a wire change a peer must understand to stay convergent + (a new causal field, a changed reconciliation rule). Additive optional fields an older peer can + safely ignore — as `observed`/`tombstone` were — do **not** need a bump. +- **Degradation when a peer predates a version:** delete-wins is only enforced on devices at that + version, so a mixed fleet is as weak as its oldest member — a behind peer can re-broadcast a note + deleted elsewhere. The new hint makes this *visible and actionable* (update the old device) rather + than silent, which was the whole point of doing this while only two shapes existed in the wild. +- **Downgrade:** an older build ignores `v` (and `sync_row_base` / `sync_oplog.observed`), reverting + to wall-clock reconciliation. Structurally safe (mobile's migration loop no-ops at a higher + `user_version`; extra columns ignored) but delete-wins is not enforced on that device. + +--- + - ~~Changing desktop to soft-delete means **every desktop live query must exclude tombstones**~~ — **resolved in Phase 2**: the list/search reads already filtered `deleted_at IS NULL`; only the by-id reads needed the guard. No ghost rows. diff --git a/electron/ipc/chat.ts b/electron/ipc/chat.ts index 6500f6e3..16666d29 100644 --- a/electron/ipc/chat.ts +++ b/electron/ipc/chat.ts @@ -11,7 +11,7 @@ import type { BrowserWindow } from "electron"; import { registerIpcHandle, registerIpcOn, broadcastEvent } from "./registry"; import { broadcastToChat } from "../chat-popout"; import type Database from "better-sqlite3"; -import { isLocalEndpoint, normaliseBaseUrl, type OpenAIMessage, calculatePromptBreakdown, scaleBreakdown, type TokenBreakdown } from "../lib/llm"; +import { isLocalEndpoint, normaliseBaseUrl, type OpenAIMessage, calculatePromptBreakdown, scaleBreakdown, type TokenBreakdown, isSendableMessage } from "../lib/llm"; import { TOOLS, buildSystemPrompt, type ChatRequest } from "../lib/tools"; import { getExternalToolDefs } from "../lib/external-tools"; import { getCachedConfig, cacheLlmConnection } from "../lib/config-cache"; @@ -154,13 +154,19 @@ export function registerChatHandler(db: Database.Database, workspacePath: string const messages: OpenAIMessage[] = [ { role: "system", content: buildSystemPrompt(req) }, - ...(req.history ?? []).map((m) => { - const out: OpenAIMessage = { role: m.role, content: m.content }; - if (m.tool_calls) out.tool_calls = m.tool_calls; - if (m.tool_call_id) out.tool_call_id = m.tool_call_id; - if (m.name) out.name = m.name; - return out; - }), + ...(req.history ?? []) + // Drop assistant turns that carry neither content nor tool_calls — a + // thinking model that timed out or stopped mid-reasoning leaves such a + // turn behind, and replaying it makes the provider reject the whole + // request with "content or tool_calls must be set" (400). + .filter(isSendableMessage) + .map((m) => { + const out: OpenAIMessage = { role: m.role, content: m.content }; + if (m.tool_calls) out.tool_calls = m.tool_calls; + if (m.tool_call_id) out.tool_call_id = m.tool_call_id; + if (m.name) out.name = m.name; + return out; + }), userMessage, ]; diff --git a/electron/ipc/pi-agent.ts b/electron/ipc/pi-agent.ts index 7e039cae..dca6e38f 100644 --- a/electron/ipc/pi-agent.ts +++ b/electron/ipc/pi-agent.ts @@ -54,6 +54,7 @@ interface PiAgentPromptRequest { apiKey?: string; maxSteps?: number; temperature?: number; + maxTokens?: number; autoApprove?: boolean; }; } @@ -72,6 +73,7 @@ interface PiAgentApprovePlanRequest { apiKey?: string; maxSteps?: number; temperature?: number; + maxTokens?: number; autoApprove?: boolean; }; } @@ -201,6 +203,7 @@ export function registerPiAgentHandler( apiKey: req.config?.apiKey, maxSteps: req.config?.maxSteps, temperature: req.config?.temperature, + maxTokens: req.config?.maxTokens, autoApprove: req.config?.autoApprove, }); @@ -214,6 +217,7 @@ export function registerPiAgentHandler( apiKey: cached.apiKey, maxSteps: reqConfig?.maxSteps || cached.maxSteps, temperature: reqConfig?.temperature || cached.temperature, + maxTokens: reqConfig?.maxTokens ?? (cached as { maxTokens?: number }).maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : cached.autoApprove, }; } else if (cached) { @@ -229,6 +233,7 @@ export function registerPiAgentHandler( apiKey: resolveLlmApiKey(reqConfig?.apiKey), maxSteps: reqConfig?.maxSteps ?? 20, temperature: reqConfig?.temperature ?? 0.3, + maxTokens: reqConfig?.maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, }; @@ -293,6 +298,7 @@ export function registerPiAgentHandler( apiKey: req.config?.apiKey, maxSteps: req.config?.maxSteps, temperature: req.config?.temperature, + maxTokens: req.config?.maxTokens, autoApprove: req.config?.autoApprove, }); @@ -306,6 +312,7 @@ export function registerPiAgentHandler( apiKey: cached.apiKey, maxSteps: reqConfig?.maxSteps || cached.maxSteps, temperature: reqConfig?.temperature || cached.temperature, + maxTokens: reqConfig?.maxTokens ?? (cached as { maxTokens?: number }).maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : cached.autoApprove, }; } else if (cached) { @@ -321,6 +328,7 @@ export function registerPiAgentHandler( apiKey: resolveLlmApiKey(reqConfig?.apiKey), maxSteps: reqConfig?.maxSteps ?? 20, temperature: reqConfig?.temperature ?? 0.3, + maxTokens: reqConfig?.maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, }; diff --git a/electron/lib/chat-loop.ts b/electron/lib/chat-loop.ts index ed0c88fa..1c677d48 100644 --- a/electron/lib/chat-loop.ts +++ b/electron/lib/chat-loop.ts @@ -70,6 +70,12 @@ export async function runToolLoop( ): Promise { const maxSteps = req.config?.maxSteps ?? 30; const temperature = req.config?.temperature ?? 0.3; + // Max output tokens. Undefined/0 → OMIT max_tokens so the model finishes + // naturally (full reasoning + answer, bounded by the provider's own limit). + // A positive value is the user's deliberate cap. The old hardcoded 4096 was + // the bug: it guillotined "thinking" models mid-reasoning, yielding empty + // content that then tripped a 400 on the next message. + const maxTokens = req.config?.maxTokens && req.config.maxTokens > 0 ? req.config.maxTokens : undefined; // Built-in tools (or a caller-supplied override) + any external tools in scope. const baseTools = toolsOverride ?? TOOLS; const combinedTools = extraTools.length > 0 ? [...baseTools, ...extraTools] : baseTools; @@ -196,7 +202,9 @@ export async function runToolLoop( messages, tools: combinedTools, tool_choice: "auto", - max_tokens: 4096, + // Only send max_tokens when the user set an explicit cap — omitting + // it lets the model finish naturally (see maxTokens above). + ...(maxTokens ? { max_tokens: maxTokens } : {}), temperature, stream: true, stream_options: { include_usage: true }, @@ -216,6 +224,7 @@ export async function runToolLoop( if (!reader) return { exhausted: true, content: "No response stream", reasoning: "" }; let contentBuffer = ""; + let streamFinishReason: string | null = null; const toolCallBuffers: Map = new Map(); for await (const jsonStr of iterSseData(reader, signal ?? undefined)) { @@ -236,6 +245,11 @@ export async function runToolLoop( ); } const delta = chunk.choices?.[0]?.delta; + // Capture the finish reason — "length" means the model hit max_tokens. + // A reasoning model can hit it having emitted only chain-of-thought. + if (chunk.choices?.[0]?.finish_reason) { + streamFinishReason = chunk.choices[0].finish_reason; + } if (!delta) continue; if (delta.content) { @@ -302,6 +316,26 @@ export async function runToolLoop( // would violate both OpenAI and Anthropic message schemas. tool_calls: toolCalls, }; + + // Reasoning-budget exhaustion safety net (cloud/streaming). By default we + // omit max_tokens so this shouldn't happen, but a user-set cap (or a + // provider's own server-side limit) can still stop a "thinking" model with + // finish_reason:"length" having emitted only reasoning and no content. + // That empty turn is useless to show AND, if persisted, poisons the next + // request (the provider rejects an assistant message with neither content + // nor tool_calls — a 400). Surface the captured reasoning so the reply is + // never empty. + if ( + (!assistantMsg.content || !assistantMsg.content.trim()) && + !assistantMsg.tool_calls?.length && + streamFinishReason === "length" && + accumulatedReasoning.trim() + ) { + const surfaced = `*[This model hit its output-token limit before giving a final answer. Its reasoning is shown below — raise or clear "Max output tokens" in AI settings for a complete reply.]*\n\n${accumulatedReasoning.trim()}`; + assistantMsg.content = surfaced; + accumulatedContent = surfaced; + if (onToken) onToken(surfaced); + } } // No tool calls — model is ready to produce its final reply diff --git a/electron/lib/chat-subagent-loop.ts b/electron/lib/chat-subagent-loop.ts index fda0d281..69159671 100644 --- a/electron/lib/chat-subagent-loop.ts +++ b/electron/lib/chat-subagent-loop.ts @@ -20,7 +20,7 @@ import type { BrowserWindow } from "electron"; import type Database from "better-sqlite3"; -import { buildApiUrl, type OpenAIMessage } from "./llm"; +import { buildApiUrl, type OpenAIMessage, isSendableMessage } from "./llm"; import { TOOLS, type ChatRequest } from "./tools"; import { runToolLoop, type RunToolLoopResult } from "./chat-loop"; import { parseToolArgs } from "./parse-tool-args"; @@ -319,7 +319,7 @@ async function runSubagent( // explicit nudge to write the findings. This is the standard "force final // answer" pattern and rescues most empty-output cases on small models. if (metrics.toolCalls > 0 || messages.some((m) => m.role === "tool")) { - const forced = await forceFinalAnswer(cfg, messages, metrics, addCost, signal); + const forced = await forceFinalAnswer(cfg, messages, metrics, addCost, signal, req.config?.maxTokens && req.config.maxTokens > 0 ? req.config.maxTokens : undefined); if (forced.trim()) { if (childId) events?.onSubagentToken?.({ childId, delta: forced.trim() }); return forced.trim(); @@ -340,6 +340,7 @@ async function forceFinalAnswer( metrics: SubagentMetrics, addCost?: (cost: unknown) => void, signal?: AbortSignal, + maxTokens?: number, ): Promise { const nudged: OpenAIMessage[] = [ ...messages, @@ -364,7 +365,7 @@ async function forceFinalAnswer( model: cfg.model, messages: nudged, tool_choice: "none", - max_tokens: 4096, + ...(maxTokens ? { max_tokens: maxTokens } : {}), temperature: 0.2, stream: false, }), @@ -416,6 +417,8 @@ export async function runDispatchLoop( const argMutator = opts?.argMutator; const events = opts?.events; const signal = opts?.signal; + // Undefined/0 → omit max_tokens (let the model finish naturally). + const maxTokens = req.config?.maxTokens && req.config.maxTokens > 0 ? req.config.maxTokens : undefined; let subSeq = 0; const date = new Date().toLocaleDateString("en-US", { weekday: "long", year: "numeric", month: "long", day: "numeric", @@ -430,7 +433,11 @@ export async function runDispatchLoop( const messages: OpenAIMessage[] = [ { role: "system", content: DISPATCH_SYSTEM_PROMPT(date) }, - ...(req.history ?? []).map((m) => { + ...(req.history ?? []) + // Drop assistant turns with neither content nor tool_calls (a stalled + // "thinking" turn) — replaying one 400s the whole request. + .filter(isSendableMessage) + .map((m) => { const out: OpenAIMessage = { role: m.role, content: m.content }; if (m.tool_calls) out.tool_calls = m.tool_calls; if (m.tool_call_id) out.tool_call_id = m.tool_call_id; @@ -468,7 +475,7 @@ export async function runDispatchLoop( messages, tools: DISPATCH_TOOLS, tool_choice: "auto", - max_tokens: 4096, + ...(maxTokens ? { max_tokens: maxTokens } : {}), temperature, stream: false, stream_options: undefined, @@ -534,7 +541,7 @@ export async function runDispatchLoop( if (!finalContent && metrics.subagentRuns > 0) { // Dispatcher-side forced answer: its cost isn't surfaced (subagent // rings carry their own), so no cost accumulator is passed. - finalContent = (await forceFinalAnswer(cfg, messages, metrics, undefined, signal)).trim(); + finalContent = (await forceFinalAnswer(cfg, messages, metrics, undefined, signal, maxTokens)).trim(); } return { content: finalContent, reasoning: finalReasoning, metrics }; } diff --git a/electron/lib/config-cache.ts b/electron/lib/config-cache.ts index 6e0e2f30..4b82ad73 100644 --- a/electron/lib/config-cache.ts +++ b/electron/lib/config-cache.ts @@ -35,6 +35,7 @@ export interface CachedConfig { apiKey?: string; maxSteps?: number; temperature?: number; + maxTokens?: number; autoApprove?: boolean; // The coding agent's active saved-provider id. The provider LIST itself is // shared and persisted under aiConfig.savedProviders (single source of truth). @@ -133,6 +134,7 @@ export function saveCachedConfig(type: "ai" | "agent" | "embeddings" | "theme" | apiKey: resolveCachedKey(configRecord.apiKey, current.agentConfig?.apiKey), maxSteps: typeof configRecord.maxSteps === "number" ? configRecord.maxSteps : current.agentConfig?.maxSteps, temperature: typeof configRecord.temperature === "number" ? configRecord.temperature : current.agentConfig?.temperature, + maxTokens: typeof configRecord.maxTokens === "number" ? configRecord.maxTokens : current.agentConfig?.maxTokens, autoApprove: typeof configRecord.autoApprove === "boolean" ? configRecord.autoApprove : current.agentConfig?.autoApprove, activeProviderId: typeof configRecord.activeProviderId === "string" ? configRecord.activeProviderId : current.agentConfig?.activeProviderId, }; @@ -179,7 +181,7 @@ export function getEmbeddingsSettingsCached(): CachedEmbeddingsConfig { */ export function cacheLlmConnection( type: "ai" | "agent", - config: { provider?: string; baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number; autoApprove?: boolean } | undefined, + config: { provider?: string; baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number; maxTokens?: number; autoApprove?: boolean } | undefined, ): void { if (!config) return; if (config.baseUrl === undefined && config.model === undefined && config.apiKey === undefined) return; diff --git a/electron/lib/llm-sendable.test.ts b/electron/lib/llm-sendable.test.ts new file mode 100644 index 00000000..6071cb47 --- /dev/null +++ b/electron/lib/llm-sendable.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { isSendableMessage } from "./llm"; + +describe("isSendableMessage", () => { + // Regression: a "thinking" model that times out or stops streaming + // mid-reasoning persists an assistant turn whose only payload was reasoning + // (stripped before re-send). Replaying it makes OpenAI-compatible providers + // reject the whole request: "content or tool_calls must be set" (400). + it("drops an assistant turn with neither content nor tool_calls", () => { + expect(isSendableMessage({ role: "assistant", content: null })).toBe(false); + expect(isSendableMessage({ role: "assistant", content: "" })).toBe(false); + expect(isSendableMessage({ role: "assistant", content: " " })).toBe(false); + expect(isSendableMessage({ role: "assistant", content: "", tool_calls: [] })).toBe(false); + }); + + it("keeps an assistant turn that has content", () => { + expect(isSendableMessage({ role: "assistant", content: "here is the draft" })).toBe(true); + }); + + it("keeps an assistant turn that has tool_calls even with empty content", () => { + expect( + isSendableMessage({ role: "assistant", content: null, tool_calls: [{ id: "c1" }] }), + ).toBe(true); + }); + + it("never filters non-assistant roles, even when empty", () => { + // user/system/tool turns with empty content are the caller's concern; the + // provider only rejects assistant turns for the missing-payload reason. + expect(isSendableMessage({ role: "user", content: "" })).toBe(true); + expect(isSendableMessage({ role: "system", content: null })).toBe(true); + expect(isSendableMessage({ role: "tool", content: "" })).toBe(true); + }); +}); diff --git a/electron/lib/llm.ts b/electron/lib/llm.ts index 99276845..8e0331ec 100644 --- a/electron/lib/llm.ts +++ b/electron/lib/llm.ts @@ -95,6 +95,26 @@ export type OpenAIMessage = { }>; }; +/** + * An assistant turn is unusable to the provider when it carries neither + * `content` nor `tool_calls` — OpenAI-compatible APIs reject the whole request + * with `Invalid assistant message: content or tool_calls must be set` (400). + * + * This happens with "thinking"/reasoning models: if the model times out or + * stops streaming mid-reasoning, the turn's only payload was reasoning (which + * is deliberately stripped before re-send, since replaying reasoning also + * 400s). Replaying that stripped, empty turn on the next message poisons the + * request. Drop such turns before sending. + */ +export function isSendableMessage(m: { + role: string; + content: string | null; + tool_calls?: unknown[]; +}): boolean { + if (m.role !== "assistant") return true; + return Boolean(m.content?.trim()) || Boolean(m.tool_calls?.length); +} + export async function callLLM(config: LLMConfig, systemPrompt: string, userPrompt: string): Promise { if (config.provider === "localllm") { const { callLocalLLMChat } = await import("./local-llm"); diff --git a/electron/lib/pi-agent-loop.ts b/electron/lib/pi-agent-loop.ts index cfb191bf..5b1245dd 100644 --- a/electron/lib/pi-agent-loop.ts +++ b/electron/lib/pi-agent-loop.ts @@ -57,6 +57,13 @@ export interface AgentLLMConfig { contextWindow?: number; /** Whether to automatically approve tool calls or prompt the user. */ autoApprove?: boolean; + /** + * Max output tokens per turn. Undefined/0 → OMIT max_tokens so the model + * finishes naturally — the old hardcoded cap could stall a "thinking" agent + * model mid-reasoning (empty content → a 400 on the next turn). A positive + * value is the user's deliberate cap. + */ + maxTokens?: number; } // ── Message types ───────────────────────────────────────────────────────────── @@ -518,6 +525,9 @@ export async function runAgentLoop( contextWindow = 128_000, } = llmConfig; + // Undefined/0 → omit max_tokens (let the model finish naturally). + const maxTokens = llmConfig.maxTokens && llmConfig.maxTokens > 0 ? llmConfig.maxTokens : undefined; + // Plan mode always uses 0.1 for deterministic analysis regardless of user setting const temperature = mode === "plan" ? 0.1 : (configTemp ?? 0.3); if (!apiKey && !isLocalEndpoint(baseUrl)) { @@ -583,7 +593,8 @@ export async function runAgentLoop( messages: contextMessages, tools: allTools, tool_choice: "auto", - max_tokens: 8192, + // Only send max_tokens when the user set an explicit cap (see maxTokens above). + ...(maxTokens ? { max_tokens: maxTokens } : {}), temperature, stream: true, stream_options: { include_usage: true }, diff --git a/electron/lib/tools.ts b/electron/lib/tools.ts index ddefb9e2..eadc03fb 100644 --- a/electron/lib/tools.ts +++ b/electron/lib/tools.ts @@ -142,7 +142,7 @@ export interface ChatRequest { projectId?: string; workspaceId?: string; history?: ChatHistoryEntry[]; - config?: { baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number }; + config?: { baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number; maxTokens?: number }; systemPrompt?: string; /** Attachments on the current user message (base64 data URLs; kind="pdf" * becomes an Anthropic-style `document` part, images become `image_url`). */ diff --git a/electron/preload.ts b/electron/preload.ts index 36114a52..91bd1e0a 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -625,6 +625,9 @@ const api = { // Retry the .md write for a restore whose DB half already succeeded. repairNoteFile: (id: string) => invoke<{ repaired: boolean; reason?: string; fileError?: string }>("sync:repairNoteFile", { id }), + // Peer devices on a different sync protocol version (behind = too old to honour deletes). + peerProtocols: () => + invoke>("sync:peerProtocols"), }, // ── AI write lock events ────────────────────── diff --git a/electron/sync/desktop-sync.ts b/electron/sync/desktop-sync.ts index dce951ff..44fdda26 100644 --- a/electron/sync/desktop-sync.ts +++ b/electron/sync/desktop-sync.ts @@ -412,6 +412,14 @@ export function listSyncActivity(db: Database.Database, limit = 100) { return getDesktopEngine(db).listSyncActivity(limit); } +/** + * Peer devices whose sync protocol version differs from this build's — used to + * prompt the user to update a device that is too old to honour deletes. + */ +export function listPeerProtocols(db: Database.Database) { + return getDesktopEngine(db).listPeerProtocols(); +} + /** * Notes tombstoned by another device that can genuinely be brought back, newest * first, plus the true total (which may exceed the returned page). diff --git a/electron/sync/sync-handlers.ts b/electron/sync/sync-handlers.ts index 04129b0d..889017c6 100644 --- a/electron/sync/sync-handlers.ts +++ b/electron/sync/sync-handlers.ts @@ -23,6 +23,7 @@ import { listRestorableNotes, restoreDeletedNote, repairNoteFile, + listPeerProtocols, type ConflictResolveDeps, } from "./desktop-sync"; @@ -130,6 +131,9 @@ export function registerSyncHandlers( (((_e: never, args?: { limit?: number }) => wrap(() => listSyncActivity(ctx.db, args?.limit ?? 100))) as never), ); + // Peers on an older (or newer) sync protocol than this build. + register("sync:peerProtocols", (() => wrap(() => listPeerProtocols(ctx.db))) as never); + // Notes a peer deleted that can still be brought back. register( "sync:listRestorable", diff --git a/mobile/app/restore.tsx b/mobile/app/restore.tsx index e8f6247b..e3f4eb5d 100644 --- a/mobile/app/restore.tsx +++ b/mobile/app/restore.tsx @@ -27,6 +27,7 @@ const REFUSAL_TEXT: Record = { orphaned: "Its project was deleted too. Restore the project first.", "self-deleted": "This device deleted that note, so it isn't offered for recovery.", "no-delete-record": "Sync has no record of that deletion, so it can't be undone from here.", + "preserved-as-copy": "Your edit is kept as a conflict copy — resolve it from the Conflicts screen instead.", }; function relativeTime(iso: string | null): string { diff --git a/mobile/app/sync.tsx b/mobile/app/sync.tsx index b791b71d..41b9831c 100644 --- a/mobile/app/sync.tsx +++ b/mobile/app/sync.tsx @@ -1,11 +1,11 @@ import { useCallback, useMemo, useState } from "react"; import { View, Text, Pressable, StyleSheet, ActivityIndicator, ScrollView } from "react-native"; import { Stack, useFocusEffect, useRouter } from "expo-router"; -import { GitMerge, ChevronRight, Trash2 } from "lucide-react-native"; +import { GitMerge, ChevronRight, Trash2, ArrowUpCircle } from "lucide-react-native"; import { iCloudAvailable, syncFolderLabel } from "@/sync/folder"; import { requestSync } from "@/sync/controller"; import { useSyncStatus } from "@/sync/useSyncStatus"; -import { restorableCount } from "@/db/queries"; +import { restorableCount, stalePeerCount } from "@/db/queries"; import { EmbeddingsCard } from "@/components/EmbeddingsCard"; import { SectionLabel } from "@/components/SectionLabel"; import { ICON_CHECK } from "@/components/toolbar-icons"; @@ -30,6 +30,7 @@ export default function SyncScreen() { const { state, pending, conflicts, lastResult: last } = useSyncStatus(); const [available, setAvailable] = useState(null); const [restorable, setRestorable] = useState(0); + const [stalePeers, setStalePeers] = useState(0); const styles = useMemo(() => makeStyles(t), [t]); const busy = state === "syncing"; @@ -43,11 +44,22 @@ export default function SyncScreen() { console.warn("[cairn] restorableCount failed", err); setRestorable(0); } + try { + setStalePeers(stalePeerCount()); + } catch (err) { + console.warn("[cairn] stalePeerCount failed", err); + setStalePeers(0); + } }, []); useFocusEffect(useCallback(() => refresh(), [refresh])); - const onSync = () => void requestSync("manual"); + // Refresh counts after a manual sync settles (success or failure) — peer + // protocol versions and restorable deletions are only discoverable once a + // sync has applied incoming changes, so don't wait for the next focus. + const onSync = () => { + void requestSync("manual").finally(refresh); + }; const close = () => { if (router.canGoBack()) router.back(); }; @@ -134,6 +146,20 @@ export default function SyncScreen() { )} + {stalePeers > 0 && ( + + + + + {stalePeers === 1 ? "Another device needs updating" : `${stalePeers} devices need updating`} + + + Update Cairn there so deletions sync correctly — until then a deleted note can reappear. + + + + )} + Bidirectional, offline-first. Edits made on this phone and the desktop reconcile via the shared sync engine. Body conflicts are kept as a "conflicted copy" note, never lost. diff --git a/mobile/changelogs/v0.1.6.md b/mobile/changelogs/v0.1.6.md new file mode 100644 index 00000000..f33f942a --- /dev/null +++ b/mobile/changelogs/v0.1.6.md @@ -0,0 +1,5 @@ +## Fixes + +- **A "device needs updating" hint when a paired device is on an older sync version.** Device Sync records each device's sync-protocol version, so if one of your devices reports a lower version — one that may not honour deletions correctly — the Sync screen tells you. Until it's updated, a note you delete can reappear from it. Devices on the same version see nothing new. +- **A note deleted on another device is no longer offered for restore twice.** When another device deleted a note you had edited offline, the Sync screen used to show both a "deleted on another device" restore prompt and a sync conflict for the same note — and restoring left two identical copies. Your edit is already kept as a conflict copy, so Cairn now sends you to the Conflicts screen to resolve it instead of offering a redundant restore. Notes deleted with no competing edit still offer Restore as before. +- **Reasoning ("thinking") models no longer stall out.** Chat replies are no longer capped at a fixed token budget, so a reasoning model can think as long as it needs and still write its answer instead of stopping empty. And if a reply ever does get cut off, an empty assistant turn can no longer break your next message. diff --git a/mobile/src/chat/models-dev.ts b/mobile/src/chat/models-dev.ts index 4ef230f7..0bd1864a 100644 --- a/mobile/src/chat/models-dev.ts +++ b/mobile/src/chat/models-dev.ts @@ -32,9 +32,11 @@ import { import { getDb } from "../db"; const API_URL = "https://models.dev/api.json"; -const CACHE_KEY = "ai.modelInfo.cache"; // JSON { [modelId]: ModelInfo } +// v2: bumped when the parsed ModelInfo shape gains a field (maxOutput) so +// existing caches re-fetch immediately instead of waiting out the weekly TTL. +const CACHE_KEY = "ai.modelInfo.cache.v2"; // JSON { [modelId]: ModelInfo } const CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; // refresh weekly -const CACHE_AT_KEY = "ai.modelInfo.cachedAt"; +const CACHE_AT_KEY = "ai.modelInfo.cachedAt.v2"; // Canonical owner map (models.json): id → provider slug for logo resolution. // Tiny (282 entries) and keyed by canonical "/" ids, so it's // cached separately with its own freshness window. diff --git a/mobile/src/chat/providers/openai.ts b/mobile/src/chat/providers/openai.ts index 58eeb653..da538566 100644 --- a/mobile/src/chat/providers/openai.ts +++ b/mobile/src/chat/providers/openai.ts @@ -35,6 +35,7 @@ import { type TextPart, type ToolPart, type UIMessage, + assistantTurnIsSendable, } from "./types"; // ── request mapping ───────────────────────────────────────────────────────── @@ -106,6 +107,10 @@ function mapMessage(m: UIMessage): OpenAIMessage[] { } const text = textParts.map((p) => p.text).join(""); + // Skip an assistant turn with no sendable payload — a thinking model that + // stopped mid-reasoning leaves one behind, and replaying it trips the + // provider's "content or tool_calls must be set" 400 on the next message. + if (!assistantTurnIsSendable(m.role, m.parts)) return out; out.push({ role: m.role, content: text }); return out; } @@ -135,6 +140,9 @@ function makeStreamer(config: OpenAIConfig) { model: config.model, messages: messages.flatMap(mapMessage), stream: true, + // Deliberately no max_tokens: omitting it lets the model finish naturally + // (full reasoning + answer). A cap only ever truncates the tail case and + // can leave "thinking" models with empty content — see mapMessage. // Ask for a final usage chunk (prompt/completion tokens) to drive the // context ring. Standard OpenAI honours it; some OpenAI-compatible // gateways reject unknown fields with a 400, so we retry without it below. diff --git a/mobile/src/chat/providers/types.test.ts b/mobile/src/chat/providers/types.test.ts new file mode 100644 index 00000000..8e48f79a --- /dev/null +++ b/mobile/src/chat/providers/types.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { assistantTurnIsSendable } from "./types"; +import type { UIPart } from "./types"; + +const text = (t: string): UIPart => ({ type: "text", text: t }); +const tool = (): UIPart => ({ type: "tool-search", toolCallId: "c1", state: "input-available" }); + +describe("assistantTurnIsSendable", () => { + // Regression: a thinking model that stops mid-reasoning leaves an assistant + // turn with no text and no tool call (reasoning isn't a text part). Replaying + // it trips the provider's "content or tool_calls must be set" 400. + it("rejects an assistant turn with no text and no tool call", () => { + expect(assistantTurnIsSendable("assistant", [])).toBe(false); + expect(assistantTurnIsSendable("assistant", [text("")])).toBe(false); + expect(assistantTurnIsSendable("assistant", [text(" ")])).toBe(false); + }); + + it("accepts an assistant turn with text", () => { + expect(assistantTurnIsSendable("assistant", [text("the draft")])).toBe(true); + }); + + it("accepts an assistant turn with a tool call and no text", () => { + expect(assistantTurnIsSendable("assistant", [tool()])).toBe(true); + }); + + it("never rejects non-assistant roles", () => { + expect(assistantTurnIsSendable("user", [text("")])).toBe(true); + expect(assistantTurnIsSendable("system", [])).toBe(true); + }); +}); diff --git a/mobile/src/chat/providers/types.ts b/mobile/src/chat/providers/types.ts index dbc9fbb3..29e8ebc5 100644 --- a/mobile/src/chat/providers/types.ts +++ b/mobile/src/chat/providers/types.ts @@ -53,6 +53,21 @@ export interface UIMessage { parts: UIPart[]; } +/** + * True when an assistant turn has something the provider will accept — real + * text or at least one tool call. A "thinking" model that stops mid-reasoning + * leaves a turn with neither (reasoning isn't a text part); replaying it trips + * the OpenAI-compatible "content or tool_calls must be set" 400 on the next + * message. Non-assistant roles are always sendable (the provider only rejects + * assistant turns for the missing-payload reason). + */ +export function assistantTurnIsSendable(role: RorkRole, parts: UIPart[]): boolean { + if (role !== "assistant") return true; + const hasText = parts.some((p) => p.type === "text" && (p as TextPart).text.trim().length > 0); + const hasTool = parts.some((p) => p.type !== "text" && p.type !== "file"); + return hasText || hasTool; +} + /** A tool the model may call. `jsonSchema` is a JSON Schema object for the args. */ export interface AiTool { description: string; diff --git a/mobile/src/db/queries.ts b/mobile/src/db/queries.ts index 3dfd67fd..7fadd2d5 100644 --- a/mobile/src/db/queries.ts +++ b/mobile/src/db/queries.ts @@ -1306,6 +1306,11 @@ export function listSyncActivity(limit = 50): SyncActivityRow[] { return getEngine().listSyncActivity(limit); } +/** Peers on an older sync protocol than this build — too old to honour deletes. */ +export function stalePeerCount(): number { + return getEngine().listPeerProtocols().filter((p) => p.behind).length; +} + /** * Resolve a conflict by keeping the CONFLICT-COPY's body: overwrite the * original note with the copy's content, then delete the copy. If the original diff --git a/shared/models/model-catalog.test.ts b/shared/models/model-catalog.test.ts index b642b4d4..8f17d7df 100644 --- a/shared/models/model-catalog.test.ts +++ b/shared/models/model-catalog.test.ts @@ -18,6 +18,8 @@ import { providerLogoUrl, providerLogoUrlFor, supportsImageInput, + resolveMaxOutputTokens, + positiveTokenLimit, } from "./model-catalog"; describe("parseModelCatalog", () => { @@ -41,6 +43,7 @@ describe("parseModelCatalog", () => { }); expect(map["gpt-5"]).toEqual({ context: 1000000, + maxOutput: 128000, input: 1.25, output: 10, modes: ["text", "image", "pdf"], @@ -49,6 +52,7 @@ describe("parseModelCatalog", () => { }); expect(map["claude-4"]).toEqual({ context: 1000000, + maxOutput: null, input: null, output: null, modes: [], @@ -63,6 +67,7 @@ describe("parseModelCatalog", () => { }); expect(map["deepseek-v4-flash"]).toEqual({ context: null, + maxOutput: null, input: null, output: null, modes: [], @@ -75,6 +80,35 @@ describe("parseModelCatalog", () => { expect(parseModelCatalog(null)).toEqual({}); expect(parseModelCatalog("nope")).toEqual({}); }); + + it("coerces a zero/negative limit.output to null (models.dev has real output:0 entries)", () => { + const map = parseModelCatalog({ + p: { + models: { + zero: { limit: { context: 8000, output: 0 } }, + neg: { limit: { context: 8000, output: -1 } }, + ok: { limit: { context: 8000, output: 4096 } }, + }, + }, + }); + expect(map["zero"].maxOutput).toBeNull(); + expect(map["neg"].maxOutput).toBeNull(); + expect(map["ok"].maxOutput).toBe(4096); + }); +}); + +describe("positiveTokenLimit", () => { + it("accepts positive integers, floors fractions, rejects the rest", () => { + expect(positiveTokenLimit(4096)).toBe(4096); + expect(positiveTokenLimit(4096.9)).toBe(4096); + expect(positiveTokenLimit(1)).toBe(1); + expect(positiveTokenLimit(0)).toBeNull(); + expect(positiveTokenLimit(0.5)).toBeNull(); + expect(positiveTokenLimit(-10)).toBeNull(); + expect(positiveTokenLimit(Number.NaN)).toBeNull(); + expect(positiveTokenLimit("4096")).toBeNull(); + expect(positiveTokenLimit(null)).toBeNull(); + }); }); describe("supportsImageInput", () => { @@ -94,12 +128,46 @@ describe("supportsImageInput", () => { }); }); +describe("resolveMaxOutputTokens", () => { + it("omits (returns undefined) by default so the model finishes naturally", () => { + // The correct default: send no cap → provider runs to finish_reason:"stop" + // with full reasoning + answer. A cap only ever truncates the tail case. + expect(resolveMaxOutputTokens()).toBeUndefined(); + expect(resolveMaxOutputTokens(undefined)).toBeUndefined(); + expect(resolveMaxOutputTokens(null)).toBeUndefined(); + }); + + it("sends a user override >= 1 verbatim (deliberate cost/latency cap), floored", () => { + expect(resolveMaxOutputTokens(5000)).toBe(5000); + expect(resolveMaxOutputTokens(200.9)).toBe(200); // floored + expect(resolveMaxOutputTokens(1)).toBe(1); + expect(resolveMaxOutputTokens(384000)).toBe(384000); + }); + + it("ignores non-positive / non-finite overrides and omits the field", () => { + expect(resolveMaxOutputTokens(0)).toBeUndefined(); + expect(resolveMaxOutputTokens(-5)).toBeUndefined(); + expect(resolveMaxOutputTokens(Number.NaN)).toBeUndefined(); + }); + + it("omits rather than sending max_tokens:0 for a fractional override in (0,1)", () => { + // Regression: `> 0` then Math.floor turned 0.1 / 0.999 into 0 — a broken cap. + expect(resolveMaxOutputTokens(0.1)).toBeUndefined(); + expect(resolveMaxOutputTokens(0.999)).toBeUndefined(); + }); +}); + describe("normalizeModelInfo", () => { it("passes well-formed entries through unchanged", () => { - const info = { context: 1000, input: 1, output: 2, modes: ["text"], toolCall: true, provider: "x" }; + const info = { context: 1000, maxOutput: 8000, input: 1, output: 2, modes: ["text"], toolCall: true, provider: "x" }; expect(normalizeModelInfo(info as never)).toEqual(info); }); + it("defaults maxOutput to null when a legacy cache entry lacks it", () => { + const legacy = { context: 1000, input: 1, output: 2, modes: ["text"], toolCall: true, provider: "x" }; + expect(normalizeModelInfo(legacy as never)?.maxOutput).toBeNull(); + }); + it("migrates the legacy pre-`modes` image boolean", () => { const withImage = normalizeModelInfo({ context: 10, image: true } as never); expect(withImage?.modes).toEqual(["text", "image"]); diff --git a/shared/models/model-catalog.ts b/shared/models/model-catalog.ts index 26a68b09..4f148ebf 100644 --- a/shared/models/model-catalog.ts +++ b/shared/models/model-catalog.ts @@ -14,9 +14,12 @@ */ /** Per-model info extracted from the models.dev catalog. */ -export interface ModelInfo { - /** Context window in tokens (limit.context). */ +export interface ModelInfo { /** Context window in tokens (limit.context). */ context: number | null; + /** Max output/completion tokens the model can emit in one reply (limit.output). + * Shown as guidance in AI/Agent settings (the ceiling a manual "Max output + * tokens" cap can use). null when unknown. */ + maxOutput: number | null; /** USD per 1M input tokens (cost.input). */ input: number | null; /** USD per 1M output tokens (cost.output). */ @@ -29,6 +32,17 @@ export interface ModelInfo { provider: string | null; } +/** + * A token *limit* is only meaningful when it's a positive integer. models.dev + * has real entries with `limit.output: 0` (~180 at time of writing); a 0 or + * negative cap is unusable (it would drive a broken `max={0}` on the settings + * stepper), so coerce zero/negative/non-finite to null. Fractions are floored. + */ +export function positiveTokenLimit(v: unknown): number | null { + if (typeof v !== "number" || !Number.isFinite(v) || v < 1) return null; + return Math.floor(v); +} + /** Provider logo URL served by models.dev. */ export function providerLogoUrl(provider: string): string { return `https://models.dev/logos/${provider}.svg`; @@ -273,6 +287,7 @@ export function normalizeModelInfo(info: ModelInfo | null | undefined): ModelInf : []; return { context: typeof info.context === "number" ? info.context : null, + maxOutput: positiveTokenLimit(info.maxOutput), input: typeof info.input === "number" ? info.input : null, output: typeof info.output === "number" ? info.output : null, modes, @@ -293,7 +308,7 @@ export function parseModelCatalog(catalog: unknown): Record { if (!models || typeof models !== "object") continue; for (const [id, model] of Object.entries(models as Record)) { const m = model as { - limit?: { context?: unknown }; + limit?: { context?: unknown; output?: unknown }; cost?: { input?: unknown; output?: unknown }; modalities?: { input?: unknown }; tool_call?: unknown; @@ -305,6 +320,7 @@ export function parseModelCatalog(catalog: unknown): Record { : []; map[id] = { context: readNum(m.limit?.context), + maxOutput: positiveTokenLimit(m.limit?.output), input: readNum(m.cost?.input), output: readNum(m.cost?.output), modes: inModes, @@ -420,3 +436,38 @@ export function formatModelCost(input: number | null, output: number | null): st }; return `${fmt(i)}/${fmt(o)}`; } + +/** + * Default max output tokens used only as a floor when a user's manual value is + * missing/invalid but they've turned Auto off. Comfortably above a "thinking" + * model's reasoning budget. NOT used in Auto mode (Auto omits the field). + */ +export const DEFAULT_MAX_OUTPUT_TOKENS = 8192; + +/** + * Resolve the `max_tokens` to send for a chat request, or `undefined` to OMIT + * the field entirely. + * + * The correct default is to send NOTHING: given no cap, providers run the model + * to its natural `finish_reason:"stop"` — emitting full reasoning AND a complete + * answer, typically using far fewer tokens than any cap we'd pick. A fixed cap + * only ever truncates the tail case: a "thinking" model can spend the whole + * budget on reasoning and stop with empty content (`finish_reason:"length"`), + * which then trips a "content or tool_calls must be set" 400 on the next + * message. (That regression is what the old hardcoded 4096 caused.) + * + * So: + * - `userOverride` (>= 1) → send it, floored. A user's explicit cap is a + * deliberate cost/latency ceiling; honour it. + * - otherwise (Auto, or a fractional/zero/negative value) → return `undefined`: + * omit `max_tokens` and let the model finish naturally, bounded only by the + * provider's own server-side limit. + */ +export function resolveMaxOutputTokens(userOverride?: number | null): number | undefined { + // Require >= 1 before flooring: a fractional cap in (0,1) would floor to 0 and + // send `max_tokens: 0` (a broken cap), which is worse than omitting the field. + if (typeof userOverride === "number" && Number.isFinite(userOverride) && userOverride >= 1) { + return Math.floor(userOverride); + } + return undefined; +} diff --git a/shared/sync/engine.test.ts b/shared/sync/engine.test.ts index a126bf94..83b1eddc 100644 --- a/shared/sync/engine.test.ts +++ b/shared/sync/engine.test.ts @@ -20,7 +20,7 @@ import path from "path"; import { applySchema } from "../../electron/db/schema"; import { SYNCABLE_TABLES } from "./schema"; import * as q from "../../electron/db/queries"; -import { SyncEngine, SYNC_ACTIVITY_LIMIT } from "./engine"; +import { SyncEngine, SYNC_ACTIVITY_LIMIT, SYNC_PROTOCOL_VERSION } from "./engine"; import { writeOplogFile, readPeerOplogs } from "./transport"; import { compareHlc } from "./hlc"; @@ -1210,6 +1210,62 @@ describe("sync engine — trigger→drain capture of real queries.ts writes", () syncFolder(dir, A.engine, B.engine); } + it("does not offer restore for a row already preserved by a live conflict copy", () => { + // The on-device repro (v2.6.1 hardware test): delete on A, edit the same + // note on B while B hasn't observed the delete. B's delete-won path + // tombstones the row (keeping its body) AND clones it into a conflict + // copy — so the tombstone and the copy hold the SAME content. Offering + // restore then duplicates the note. Suppress it; the edit is safe in the + // copy, so point the user at conflict resolution instead. + const clkA = clockFrom(14_760_000); + const clkB = clockFrom(14_760_000); + const A = makeDevice("A", clkA.now); + const B = makeDevice("B", clkB.now); + seedNoteOnBoth(A, B); + + clkA.advance(1); + q.deleteNote(A.db, "n1"); + A.engine.drainPending(); + clkB.advance(50); + q.updateNote(B.db, "n1", { content: "phone kept editing" }); + B.engine.drainPending(); + syncFolder(dir, A.engine, B.engine); + + // B has both a tombstoned n1 and a live conflict copy of it. + const copy = B.db + .prepare("SELECT id, content FROM notes WHERE id LIKE 'n1_conflict_%' AND deleted_at IS NULL") + .get() as { id: string; content: string } | undefined; + expect(copy?.content).toBe("phone kept editing"); + const tombstoned = B.db.prepare("SELECT content FROM notes WHERE id = 'n1'").get() as { content: string }; + expect(tombstoned.content).toBe("phone kept editing"); // same body → redundant restore + + // So restore is refused on B (the losing device) and n1 is not listed. + expect(B.engine.listRestorable("notes").rows.map((r) => r.entity_id)).not.toContain("n1"); + expect(B.engine.restoreDeleted("notes", "n1")).toEqual({ + restored: false, + reason: "preserved-as-copy", + }); + }); + + it("still offers restore when the delete won cleanly with no divergent copy", () => { + // Plain peer delete, no competing local edit → no conflict copy exists, so + // restore is the only way back and must still be offered. + const clkA = clockFrom(14_780_000); + const clkB = clockFrom(14_780_000); + const A = makeDevice("A", clkA.now); + const B = makeDevice("B", clkB.now); + seedNoteOnBoth(A, B); + + clkA.advance(5); + q.deleteNote(A.db, "n1"); + A.engine.drainPending(); + syncFolder(dir, A.engine, B.engine); + + expect(B.db.prepare("SELECT COUNT(*) c FROM notes WHERE id LIKE 'n1_conflict_%'").get()).toEqual({ c: 0 }); + expect(B.engine.listRestorable("notes").rows.map((r) => r.entity_id)).toContain("n1"); + expect(B.engine.restoreDeleted("notes", "n1").restored).toBe(true); + }); + it("logs an applied outcome for a peer edit that lands cleanly", () => { const clkA = clockFrom(14_000_000); const clkB = clockFrom(14_000_000); @@ -1611,4 +1667,98 @@ describe("sync engine — trigger→drain capture of real queries.ts writes", () expect((B.db.prepare("SELECT COUNT(*) c FROM notes WHERE id='n1'").get() as { c: number }).c).toBe(1); }); }); + + describe("sync protocol versioning", () => { + function seed(engine: SyncEngine) { + q.createWorkspace(engine.db as never, { id: "ws1", name: "WS" }); + q.createProject(engine.db as never, { id: "p1", workspaceId: "ws1", name: "P" }); + q.createNote(engine.db as never, { id: "n1", projectId: "p1", workspaceId: "ws1", title: "N", content: "x" }); + engine.drainPending(); + } + + it("stamps every exported entry with the current protocol version", () => { + const A = makeDevice("A", clockFrom(15_000_000).now); + seed(A.engine); + const ops = A.engine.exportOplog(); + expect(ops.length).toBeGreaterThan(0); + expect(ops.every((o) => o.v === SYNC_PROTOCOL_VERSION)).toBe(true); + }); + + it("records a legacy (unversioned) peer's version as 1", () => { + const A = makeDevice("A", clockFrom(15_100_000).now); + const B = makeDevice("B", clockFrom(15_100_000).now); + seed(A.engine); + + // Simulate an older peer by stripping the version off its entries — this + // is exactly what a pre-versioning (v2.6.1) build's oplog looks like. + const legacyOps = A.engine.exportOplog().map(({ v, ...rest }) => { + void v; + return rest; + }); + B.engine.applyRemote(legacyOps); + + // The version IS recorded internally as 1. Whether it surfaces in + // listPeerProtocols depends on the current protocol: it only appears when + // it DIFFERS from this build. While SYNC_PROTOCOL_VERSION === 1 a v1 peer + // is in step, so the list is empty; after a future bump the same peer + // would surface as `behind`. Assert whichever the current version implies, + // so this test stays honest across a bump instead of hard-coding 1. + const peers = B.engine.listPeerProtocols(); + if (SYNC_PROTOCOL_VERSION === 1) { + expect(peers).toEqual([]); + } else { + expect(peers).toEqual([{ deviceId: "A", version: 1, behind: true }]); + } + }); + + it("flags a peer whose recorded version is below this build (simulated bump)", () => { + // Can't change SYNC_PROTOCOL_VERSION at runtime, so drive listPeerProtocols' + // classification directly: write a peer record one version BELOW current + // and confirm it is reported as behind. + const B = makeDevice("B", clockFrom(15_150_000).now); + const behind = SYNC_PROTOCOL_VERSION - 1; + if (behind < 1) { + // At v1 there is no valid "below" version, so there is nothing to flag. + expect(B.engine.listPeerProtocols()).toEqual([]); + return; + } + B.db.prepare("INSERT INTO sync_state (key, value) VALUES (?, ?)").run("peer_protocol:OLD", String(behind)); + expect(B.engine.listPeerProtocols()).toContainEqual({ deviceId: "OLD", version: behind, behind: true }); + }); + + it("does not flag a peer on the same protocol version", () => { + const A = makeDevice("A", clockFrom(15_200_000).now); + const B = makeDevice("B", clockFrom(15_200_000).now); + seed(A.engine); + B.engine.applyRemote(A.engine.exportOplog()); // current-version entries + + // Same version → nothing to warn about. + expect(B.engine.listPeerProtocols()).toEqual([]); + }); + + it("keeps the highest version seen, so a stale leftover file can't mask an upgrade", () => { + const A = makeDevice("A", clockFrom(15_300_000).now); + const B = makeDevice("B", clockFrom(15_300_000).now); + seed(A.engine); + + const current = A.engine.exportOplog(); + // Peer upgrades (writes v = current), THEN an old cached file (v absent = 1) + // is re-read. Monotonic tracking must not downgrade the record to 1. + B.engine.applyRemote(current); + B.engine.applyRemote(current.map(({ v, ...rest }) => { void v; return rest; })); + + // Never regressed below the current version → no false "behind" warning. + expect(B.engine.listPeerProtocols().some((p) => p.behind)).toBe(false); + }); + + it("does not record its own device as a peer", () => { + const A = makeDevice("A", clockFrom(15_400_000).now); + const B = makeDevice("B", clockFrom(15_400_000).now); + seed(A.engine); + // Round-trip so A re-applies its own gossiped ops. + B.engine.applyRemote(A.engine.exportOplog()); + A.engine.applyRemote(B.engine.exportOplog()); + expect(A.engine.listPeerProtocols().map((p) => p.deviceId)).not.toContain("A"); + }); + }); }); diff --git a/shared/sync/engine.ts b/shared/sync/engine.ts index ab9f4478..8bb76521 100644 --- a/shared/sync/engine.ts +++ b/shared/sync/engine.ts @@ -81,7 +81,8 @@ export type RestoreRefusal = | "conflict-copy" // resolve via the conflict UI, not restore | "orphaned" // its project/workspace is gone, so reviving would diverge | "self-deleted" // this device authored the delete - | "no-delete-record"; // tombstoned, but sync holds no delete metadata for it + | "no-delete-record" // tombstoned, but sync holds no delete metadata for it + | "preserved-as-copy"; // a live conflict copy already holds this content — resolve that instead export type RestoreResult = | { restored: true; hlc: string } @@ -107,6 +108,33 @@ const LOGGED_OUTCOMES: ReadonlySet = new Set([ /** How far back a peer delete is still offered for recovery. */ const RESTORABLE_WINDOW_MS = 30 * 24 * 60 * 60 * 1000; +/** + * Oplog wire-protocol version this build writes and understands. + * + * The format was unversioned through v2.6.1; that implicit shape is **protocol + * 1**, and a missing `v` is read as 1 so legacy entries need no rewrite. Bump + * this only for a wire change that a peer must understand to stay convergent + * (a new causal field, a changed reconciliation rule). Additive optional fields + * that older peers can safely ignore (as `observed`/`tombstone` were) do NOT + * need a bump. + * + * The point of stamping it now, while only two historical shapes exist, is that + * a peer can be *told* it is behind instead of us inferring capability from + * which optional fields happen to be present. + */ +export const SYNC_PROTOCOL_VERSION = 1; + +/** sync_state key prefix for the highest protocol version seen from a peer. */ +const PEER_PROTOCOL_PREFIX = "peer_protocol:"; + +/** A peer whose observed protocol version differs from this build's. */ +export interface PeerProtocol { + deviceId: string; + version: number; + /** True when the peer is writing an OLDER protocol than this build enforces. */ + behind: boolean; +} + export interface OplogEntry { hlc: string; origin: string; @@ -118,6 +146,11 @@ export interface OplogEntry { observed?: ObservationFrontier; /** Durable delete history carried by a compacted live put for late peers. */ tombstone?: { hlc: string; origin: string }; + /** + * Wire-protocol version (SYNC_PROTOCOL_VERSION). Optional on the type so a + * pre-versioning entry deserialises cleanly; absence is treated as 1. + */ + v?: number; } interface RowBase { @@ -742,7 +775,11 @@ export class SyncEngine { const tombstone = r.op === "put" && base?.delete_hlc && base.delete_origin ? { hlc: base.delete_hlc, origin: base.delete_origin } : undefined; - return { ...r, payload: r.payload ? (JSON.parse(r.payload) as Record) : null, observed, tombstone }; + // Stamp the wire-protocol version at the export boundary. It is not stored + // per-row (it describes the format, not the edit), so every exported entry + // carries this build's version uniformly. A peer reads it to know whether + // we are ahead of or behind them. + return { ...r, payload: r.payload ? (JSON.parse(r.payload) as Record) : null, observed, tombstone, v: SYNC_PROTOCOL_VERSION }; }); } @@ -776,6 +813,13 @@ export class SyncEngine { const run = this.db.transaction(() => { this.setSuppress(true); // reconcile writes must not be re-captured as local ops for (const entry of sorted) { + // Note the peer's wire-protocol version (absent = pre-versioning = 1) so + // the UI can flag a device that is behind this build. Kept outside the + // syncable-entity guard below: even an op for a dropped entity still + // tells us how old that peer's writer is. + if (entry.origin && entry.origin !== this.deviceId) { + this.notePeerProtocol(entry.origin, entry.v ?? 1); + } // Ignore ops for entities we no longer sync (e.g. chat_threads / // chat_messages, removed in v28). A peer still running an old build — or // an oplog file it published before upgrading — can carry these; without @@ -1119,6 +1163,44 @@ export class SyncEngine { if (!exists) this.appendOplog(entry); } + // ── protocol versioning ─────────────────────────────────────────────── + + /** + * Record the highest wire-protocol version seen from a peer. Monotonic — a + * peer that upgrades and writes a newer version is never downgraded in our + * record, so a single old file lingering in the folder can't mask that the + * device has since updated. + */ + private notePeerProtocol(deviceId: string, version: number): void { + if (!Number.isFinite(version) || version < 1) return; + const key = `${PEER_PROTOCOL_PREFIX}${deviceId}`; + const current = Number(this.getState(key) ?? 0); + if (version > current) this.setState(key, String(version)); + } + + /** + * Peers whose observed protocol version differs from this build's, so the UI + * can prompt the user to update a device. `behind: true` is the important + * case — that peer may not honour deletes, so the fleet is only as strong as + * it. A peer AHEAD of us is reported too (informational: this build is old). + */ + listPeerProtocols(): PeerProtocol[] { + const rows = this.db + .prepare(`SELECT key, value FROM sync_state WHERE key LIKE ? ESCAPE '\\'`) + .all(`${PEER_PROTOCOL_PREFIX.replace(/[\\%_]/g, "\\$&")}%`) as Array<{ key: string; value: string }>; + const out: PeerProtocol[] = []; + for (const r of rows) { + const version = Number(r.value); + if (!Number.isFinite(version) || version === SYNC_PROTOCOL_VERSION) continue; + out.push({ + deviceId: r.key.slice(PEER_PROTOCOL_PREFIX.length), + version, + behind: version < SYNC_PROTOCOL_VERSION, + }); + } + return out; + } + // ── Phase 4: visibility & recovery ──────────────────────────────────── private recordActivity(entry: OplogEntry, outcome: SyncOutcome, conflictCopyId: string | null): void { @@ -1269,6 +1351,16 @@ export class SyncEngine { // orphan (the parent may have been hard-deleted on the peer). if (!this.parentsLive(entity, row)) return { ok: false, reason: "orphaned" }; + // When this device lost a delete/edit race, the delete-won path both + // tombstoned this row (keeping its body) AND cloned that body into a live + // conflict copy. Offering restore then produces two identical notes, and + // the user has to notice and delete one. If such a copy already preserves + // this content, send them to conflict resolution instead — the edit is not + // at risk, so this is a redundant second door, not a lost note. + if (this.hasLivePreservingCopy(entity, id, row)) { + return { ok: false, reason: "preserved-as-copy" }; + } + return { ok: true, row, base }; } @@ -1295,10 +1387,34 @@ export class SyncEngine { } /** - * FK metadata for a table, cached per engine instance. `listRestorable` calls - * `parentsLive` once per candidate row, and the schema cannot change under a - * live engine, so re-reading the PRAGMA every time is pure overhead. + * True when a live conflict copy already holds this tombstoned row's content. + * + * The delete-won path clones the losing body into `_conflict__` + * and then tombstones the original with its body intact, so the two match. In + * that case the edit is safe in the copy and restore would just duplicate it — + * we suppress the restore offer and point at conflict resolution instead. + * + * Only checks entities with a body column (notes); everything else has no + * conflict-copy mechanism, so the answer is trivially false. */ + private hasLivePreservingCopy(entity: SyncableTable, id: string, row: Record): boolean { + const bodyCol = BODY_COLUMN[entity]; + if (!bodyCol) return false; + const body = row[bodyCol]; + // Copies are `_conflict__`. Escape LIKE wildcards in the id + // (note ids are nanoid, but `_` is a LIKE metachar), then confirm each hit + // really parses back to THIS original — the deviceId can itself contain the + // separator, so a prefix match alone is not proof. + const likePrefix = `${id}_conflict_`.replace(/[\\%_]/g, "\\$&"); + const candidates = this.db + .prepare(`SELECT id, "${bodyCol}" AS body FROM ${entity} WHERE deleted_at IS NULL AND id LIKE ? ESCAPE '\\'`) + .all(`${likePrefix}%`) as Array<{ id: string; body: unknown }>; + for (const c of candidates) { + if (inspectConflict(c.id).originalId !== id) continue; + if (String(c.body ?? "") === String(body ?? "")) return true; + } + return false; + } private foreignKeys(entity: SyncableTable): Array<{ table: string; from: string; to: string | null }> { const cached = this.fkCache.get(entity); if (cached) return cached; diff --git a/shared/sync/oplog-guard.ts b/shared/sync/oplog-guard.ts index 7aac494e..557cbe49 100644 --- a/shared/sync/oplog-guard.ts +++ b/shared/sync/oplog-guard.ts @@ -50,6 +50,10 @@ function isOplogEntry(v: unknown): v is OplogEntry { const payloadValid = e.op === "put" ? typeof e.payload === "object" && e.payload !== null && !Array.isArray(e.payload) : e.payload === null || (typeof e.payload === "object" && e.payload !== null && !Array.isArray(e.payload)); + // Protocol version is optional (absent = pre-versioning = 1). If present it + // must be a positive integer; a garbage value would corrupt stale-peer + // detection, so reject the whole entry rather than trust it. + const versionValid = e.v === undefined || (typeof e.v === "number" && Number.isInteger(e.v) && e.v >= 1); return ( validHlc(e.hlc) && typeof e.origin === "string" && @@ -60,7 +64,8 @@ function isOplogEntry(v: unknown): v is OplogEntry { OPS.has(e.op) && payloadValid && observedValid && - tombstoneValid + tombstoneValid && + versionValid ); } diff --git a/shared/sync/transport.test.ts b/shared/sync/transport.test.ts index 617b1f49..011cd317 100644 --- a/shared/sync/transport.test.ts +++ b/shared/sync/transport.test.ts @@ -139,6 +139,26 @@ describe("transport oplog shape validation", () => { expect(got.map((e) => e.entity_id).sort()).toEqual(["causal", "legacy", "tombstoned"]); }); + it("accepts a valid protocol version and rejects a malformed one", () => { + const dir = tmpDir(); + dirs.push(dir); + const file = path.join(dir, "oplog-dev_b.ndjson"); + fs.writeFileSync( + file, + [ + JSON.stringify({ ...entry, entity_id: "versioned", v: 1 }), + JSON.stringify({ ...entry, entity_id: "no-version" }), // absent = pre-versioning, allowed + JSON.stringify({ ...entry, entity_id: "bad-version-type", v: "1" }), + JSON.stringify({ ...entry, entity_id: "bad-version-frac", v: 1.5 }), + JSON.stringify({ ...entry, entity_id: "bad-version-zero", v: 0 }), + ].join("\n") + "\n", + "utf-8", + ); + + const got = readPeerOplogs(dir, "dev_a"); + expect(got.map((e) => e.entity_id).sort()).toEqual(["no-version", "versioned"]); + }); + it("rejects put payloads that are not row records (null or array)", () => { const dir = tmpDir(); dirs.push(dir); diff --git a/src/components/agent/AgentChatPane.tsx b/src/components/agent/AgentChatPane.tsx index 0d939278..5c538e34 100644 --- a/src/components/agent/AgentChatPane.tsx +++ b/src/components/agent/AgentChatPane.tsx @@ -17,6 +17,7 @@ import { useCairnStore } from "@/store"; import { useShallow } from "zustand/react/shallow"; import { id } from "@/lib/utils"; import { getCommandsForScope } from "@/lib/slash-commands"; +import { resolveMaxOutputTokens } from "../../../shared/models/model-catalog"; import { AgentMessageBubble } from "./AgentMessageBubble"; import { PlanApprovalCard } from "./PlanApprovalCard"; import { PlanTaskList } from "./PlanTaskList"; @@ -520,6 +521,8 @@ export function AgentChatPane({ session, isActive }: AgentChatPaneProps) { apiKey: agentConfig.apiKey || undefined, maxSteps: agentConfig.maxSteps ?? 30, temperature: agentConfig.temperature ?? 0.3, + // Auto (default) → undefined → omit max_tokens so the model finishes naturally. + maxTokens: resolveMaxOutputTokens(agentConfig.maxOutputAuto === false ? agentConfig.maxOutputTokens : undefined), autoApprove: session.autoApprove ?? agentConfig.autoApprove ?? true, }, }; @@ -590,6 +593,7 @@ export function AgentChatPane({ session, isActive }: AgentChatPaneProps) { apiKey: agentConfig.apiKey || undefined, maxSteps: agentConfig.maxSteps ?? 30, temperature: agentConfig.temperature ?? 0.3, + maxTokens: resolveMaxOutputTokens(agentConfig.maxOutputAuto === false ? agentConfig.maxOutputTokens : undefined), autoApprove, }, }); diff --git a/src/components/agent/AgentMessageBubble.tsx b/src/components/agent/AgentMessageBubble.tsx index eab2819f..7e355fee 100644 --- a/src/components/agent/AgentMessageBubble.tsx +++ b/src/components/agent/AgentMessageBubble.tsx @@ -11,7 +11,7 @@ import { useCairnStore } from "@/store"; import { ContextRing } from "./ContextRing"; import type { PiAgentMessage, PiSubagentMessage } from "@/types"; import { humanizeTool } from "@/lib/humanize-tool"; -import { approvalPreview, riskForTool } from "@/lib/tool-risk"; +import { approvalPreview, riskForTool, approvalGrantScope, approvalScopeLabel } from "@/lib/tool-risk"; import { ConnectorToolCard, type ConnectorMeta } from "@/components/shared/ConnectorToolCard"; export type AgentConnectorMeta = ConnectorMeta; @@ -64,7 +64,8 @@ function ApprovalCard({ tc, sessionId }: { tc: ToolChipProps["tc"]; sessionId: s const risk = riskForTool(tc.name); const humanized = humanizeTool(tc.name, tc.args); const preview = approvalPreview(tc.name, tc.args); - const scope = risk === "EXTERNAL" ? "leaves this Mac via a connected service" : risk === "EXEC" ? "runs on this Mac" : "stays on this Mac"; + const scope = approvalScopeLabel(tc.name); + const grant = approvalGrantScope(tc.name); return (
@@ -81,8 +82,8 @@ function ApprovalCard({ tc, sessionId }: { tc: ToolChipProps["tc"]; sessionId: s {preview &&
{preview}
}
- {risk === "EXEC" && tc.name === "bash" && } - {risk !== "READ" && risk !== "EXEC" && } + {grant === "command" && } + {grant === "session" && }
diff --git a/src/components/agent/PlanApprovalCard.component.test.tsx b/src/components/agent/PlanApprovalCard.component.test.tsx new file mode 100644 index 00000000..3ab04e1b --- /dev/null +++ b/src/components/agent/PlanApprovalCard.component.test.tsx @@ -0,0 +1,100 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { PlanApprovalCard } from "./PlanApprovalCard"; + +/** + * Behaviour suite for the agent plan-approval card (plan-autonomy, + * plan-request-changes). Named by what the user does, not by implementation. + * + * PlanApprovalCard is a pure-props component, so these are deterministic with + * no Electron/IPC dependency — the autonomy choice and feedback loop are driven + * entirely through `onApprove` / `onRequestChanges`. + */ + +type Props = React.ComponentProps; + +function renderCard(overrides: Partial = {}) { + const props: Props = { + content: "1. Read the file\n2. Summarise it", + busy: false, + onApprove: vi.fn(), + onRequestChanges: vi.fn(), + ...overrides, + }; + render(); + return props; +} + +describe("plan-autonomy", () => { + it("offers both autonomy choices with the plan visible", () => { + renderCard(); + expect(screen.getByTestId("plan-approval-card")).toBeTruthy(); + expect(screen.getByTestId("plan-approve-auto")).toBeTruthy(); + expect(screen.getByTestId("plan-approve-interactive")).toBeTruthy(); + }); + + it("approves in auto mode (autoApprove=true) via 'Approve & run'", async () => { + const user = userEvent.setup(); + const props = renderCard(); + await user.click(screen.getByTestId("plan-approve-auto")); + expect(props.onApprove).toHaveBeenCalledExactlyOnceWith(true); + }); + + it("approves in interactive mode (autoApprove=false) via 'Ask per step'", async () => { + const user = userEvent.setup(); + const props = renderCard(); + await user.click(screen.getByTestId("plan-approve-interactive")); + expect(props.onApprove).toHaveBeenCalledExactlyOnceWith(false); + }); + + it("disables every action while a decision is in flight (busy)", () => { + renderCard({ busy: true }); + expect(screen.getByTestId("plan-approve-auto").disabled).toBe(true); + expect(screen.getByTestId("plan-approve-interactive").disabled).toBe(true); + expect(screen.getByTestId("plan-request-changes").disabled).toBe(true); + }); +}); + +describe("plan-request-changes", () => { + it("reveals a feedback box and sends the typed feedback", async () => { + const user = userEvent.setup(); + const props = renderCard(); + + // No feedback box until the user asks to change the plan. + expect(screen.queryByTestId("plan-feedback")).toBeNull(); + await user.click(screen.getByTestId("plan-request-changes")); + + const box = screen.getByTestId("plan-feedback"); + await user.type(box, "Add error handling"); + await user.click(screen.getByTestId("plan-submit-feedback")); + + expect(props.onRequestChanges).toHaveBeenCalledExactlyOnceWith("Add error handling"); + // Approving is not triggered by requesting changes. + expect(props.onApprove).not.toHaveBeenCalled(); + }); + + it("will not submit empty or whitespace-only feedback", async () => { + const user = userEvent.setup(); + const props = renderCard(); + await user.click(screen.getByTestId("plan-request-changes")); + + // Submit stays disabled with no input… + expect(screen.getByTestId("plan-submit-feedback").disabled).toBe(true); + // …and after only whitespace. + await user.type(screen.getByTestId("plan-feedback"), " "); + expect(screen.getByTestId("plan-submit-feedback").disabled).toBe(true); + expect(props.onRequestChanges).not.toHaveBeenCalled(); + }); + + it("returns to the action row when the feedback is cancelled", async () => { + const user = userEvent.setup(); + renderCard(); + await user.click(screen.getByTestId("plan-request-changes")); + expect(screen.getByTestId("plan-feedback")).toBeTruthy(); + + await user.click(screen.getByText("Cancel")); + expect(screen.queryByTestId("plan-feedback")).toBeNull(); + expect(screen.getByTestId("plan-approve-auto")).toBeTruthy(); + }); +}); diff --git a/src/components/chat/chat-panel/index.tsx b/src/components/chat/chat-panel/index.tsx index 1d2bb618..866b7289 100644 --- a/src/components/chat/chat-panel/index.tsx +++ b/src/components/chat/chat-panel/index.tsx @@ -29,7 +29,7 @@ import { prewarmModelCatalog, subscribeModelCatalog, } from "@/lib/models-dev"; -import { supportsImageInput } from "../../../../shared/models/model-catalog"; +import { supportsImageInput, resolveMaxOutputTokens } from "../../../../shared/models/model-catalog"; import { supportsPdfInput } from "../../../../shared/models/pdf-attach"; import { rasterizePdfToImages } from "@/lib/pdf-rasterize"; @@ -462,9 +462,14 @@ export function ChatPanel({ prefill, onPrefillConsumed, popoutMode }: ChatPanelP }); }); } else { + // Skip assistant turns with no content and no tool calls — a + // thinking model that stopped mid-reasoning leaves an empty turn + // (its reasoning is stripped before re-send), and replaying it + // trips the provider's "content or tool_calls must be set" 400. + if (!m.content?.trim()) return; history.push({ role: "assistant", - content: m.content || null, + content: m.content, }); } } else if (m.role === "system") { @@ -489,6 +494,13 @@ export function ChatPanel({ prefill, onPrefillConsumed, popoutMode }: ChatPanelP apiKey: aiConfig.apiKey || undefined, maxSteps: aiConfig.maxSteps ?? 30, temperature: aiConfig.temperature ?? 0.3, + // Max output tokens: Auto (default) sends nothing so the model finishes + // naturally — capping only ever truncates a long reasoning+answer and + // can leave "thinking" models with empty content (a 400 next turn). A + // manual value is a deliberate user cost/latency ceiling. + maxTokens: resolveMaxOutputTokens( + aiConfig.maxOutputAuto === false ? aiConfig.maxOutputTokens : undefined, + ), }, systemPrompt, images: attachmentsToSend?.map((a) => ({ name: a.name, dataUrl: a.dataUrl, kind: a.kind })), diff --git a/src/components/settings/AISettings.tsx b/src/components/settings/AISettings.tsx index 5542918e..37960d42 100644 --- a/src/components/settings/AISettings.tsx +++ b/src/components/settings/AISettings.tsx @@ -5,7 +5,7 @@ import { useShallow } from "zustand/react/shallow"; import { useEffect, useState } from "react"; import { Cpu, Globe, Download } from "lucide-react"; import { cn } from "@/lib/utils"; -import { contextLimitForModel } from "@/lib/models-dev"; +import { contextLimitForModel, modelInfoForModel } from "@/lib/models-dev"; import { SettingsGroup, SettingsRow, Toggle, StepperSettingsRow } from "./shared"; import { MCPServerSettings } from "./MCPSettings"; import { LlamaServerConsole } from "./LlamaServerConsole"; @@ -62,6 +62,24 @@ export function AISettings() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [model, provider, contextAuto]); + // Max output tokens. Auto (default) sends NO cap so the model finishes + // naturally — capping only ever truncates a long reasoning+answer. A manual + // value is a deliberate cost/latency ceiling. We surface the model's + // advertised limit.output (when known) as guidance next to the field. + const maxOutputAuto = aiConfig.maxOutputAuto ?? true; + const [advertisedMaxOutput, setAdvertisedMaxOutput] = useState(null); + useEffect(() => { + if (provider === "localllm") return; + let cancelled = false; + const id = (model ?? "").trim(); + if (!id) { setAdvertisedMaxOutput(null); return; } // eslint-disable-line react-hooks/set-state-in-effect + modelInfoForModel(id).then((info) => { + if (cancelled) return; + setAdvertisedMaxOutput(info?.maxOutput ?? null); + }); + return () => { cancelled = true; }; + }, [model, provider]); + // Shared Max-steps control — rendered after the provider-specific block for // both providers (passed into the Llama console so its position is unchanged). const maxStepsRow = ( @@ -206,6 +224,36 @@ export function AISettings() { } /> + {/* Max output tokens — Auto (default) sends no cap so the model + finishes naturally; a manual value is a deliberate ceiling. + Capping is what broke "thinking" models (empty reply → 400). */} + updateAIConfig({ maxOutputTokens: v, maxOutputAuto: false })} + presets={[4096, 8192, 16384, 32768, 65536]} + min={256} + max={advertisedMaxOutput ?? 384000} + step={256} + inputWidth="w-28" + formatPreset={(n) => (n >= 1000 ? `${(n / 1000).toFixed(n % 1000 === 0 ? 0 : 1)}k` : String(n))} + autoState={maxOutputAuto ? "detected" : "idle"} + autoActive={maxOutputAuto} + autoSuppressesValue + suppressedPlaceholder="No limit" + onAuto={() => updateAIConfig({ maxOutputAuto: true })} + /> + {/* Subagents — dispatch → research/write architecture. Cloud only (small on-device models are unreliable with the multi-hop split), mirroring the chat toolbar's original provider !== "localllm" guard. */} diff --git a/src/components/settings/AgentSettings.tsx b/src/components/settings/AgentSettings.tsx index a7dd31eb..2a69e294 100644 --- a/src/components/settings/AgentSettings.tsx +++ b/src/components/settings/AgentSettings.tsx @@ -8,7 +8,7 @@ import { useCairnStore } from "@/store"; import { useShallow } from "zustand/react/shallow"; import { Button } from "@/components/ui/button"; import { id, cn } from "@/lib/utils"; -import { contextLimitForModel } from "@/lib/models-dev"; +import { contextLimitForModel, modelInfoForModel } from "@/lib/models-dev"; import { useCopyToClipboard } from "@/hooks/useCopyToClipboard"; import type { CodingAgent } from "@/store/slices/coding-agents"; import { SettingsGroup, SettingsRow, StepperSettingsRow } from "./shared"; @@ -446,6 +446,22 @@ export function AgentSettings() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [modelAgent, contextAutoAgent]); + // Max output tokens — Auto (default) sends no cap so the model finishes + // naturally; a manual value is a deliberate ceiling. Same rationale as the + // AI Chat setting: capping stalls "thinking" models mid-reasoning. + const maxOutputAutoAgent = agentConfig.maxOutputAuto ?? true; + const [advertisedMaxOutputAgent, setAdvertisedMaxOutputAgent] = useState(null); + useEffect(() => { + let cancelled = false; + const mid = (modelAgent ?? "").trim(); + if (!mid) { setAdvertisedMaxOutputAgent(null); return; } // eslint-disable-line react-hooks/set-state-in-effect + modelInfoForModel(mid).then((info) => { + if (cancelled) return; + setAdvertisedMaxOutputAgent(info?.maxOutput ?? null); + }); + return () => { cancelled = true; }; + }, [modelAgent]); + async function handleSaveAgent(agent: Omit) { await saveAgent(agent); setAdding(false); @@ -541,6 +557,36 @@ export function AgentSettings() { } /> + {/* Max output tokens — Auto (default) sends no cap so the model finishes + naturally; a manual value is a deliberate ceiling. Capping stalls + reasoning models mid-thought. */} + updateAgent({ maxOutputTokens: v, maxOutputAuto: false })} + presets={[4096, 8192, 16384, 32768, 65536]} + min={256} + max={advertisedMaxOutputAgent ?? 384000} + step={256} + inputWidth="w-28" + formatPreset={(n) => (n >= 1000 ? `${(n / 1000).toFixed(n % 1000 === 0 ? 0 : 1)}k` : String(n))} + autoState={maxOutputAutoAgent ? "detected" : "idle"} + autoActive={maxOutputAutoAgent} + autoSuppressesValue + suppressedPlaceholder="No limit" + onAuto={() => updateAgent({ maxOutputAuto: true })} + /> + {/* Auto-approve */} = { orphaned: "Its project was deleted too. Restore the project first.", "self-deleted": "This device deleted that note, so it isn't offered for recovery.", "no-delete-record": "Sync has no record of that deletion, so it can't be undone from here.", + "preserved-as-copy": "Your edit is kept as a conflict copy — resolve it from the conflicts view instead.", }; /** @@ -164,6 +167,7 @@ export function SyncActivityPanel() { const [restoring, setRestoring] = useState>(new Set()); const [errors, setErrors] = useState>({}); const [fileFailures, setFileFailures] = useState(() => loadFileFailures()); + const [stalePeers, setStalePeers] = useState([]); const [loaded, setLoaded] = useState(false); const alive = useRef(true); const setView = useCairnStore((s) => s.setView); @@ -176,7 +180,7 @@ export function SyncActivityPanel() { const status = useSyncStatus(); const load = useCallback( - async () => Promise.all([fetchSyncActivity(50), fetchRestorableNotes(pageSize)]), + async () => Promise.all([fetchSyncActivity(50), fetchRestorableNotes(pageSize), fetchPeerProtocols()]), [pageSize], ); @@ -208,11 +212,12 @@ export function SyncActivityPanel() { useEffect(() => { let cancelled = false; void (async () => { - const [acts, restores] = await load(); + const [acts, restores, peers] = await load(); if (cancelled) return; setActivity(acts); setRestorable(restores.rows); setRestorableTotal(restores.total); + setStalePeers(peers); setLoaded(true); })(); return () => { @@ -238,11 +243,12 @@ export function SyncActivityPanel() { // so this is the only remaining trace of the problem. recordFileFailure({ id, title, error: res.fileError }); } - const [acts, restores] = await load(); + const [acts, restores, peers] = await load(); if (!alive.current) return; setActivity(acts); setRestorable(restores.rows); setRestorableTotal(restores.total); + setStalePeers(peers); } finally { if (alive.current) { setRestoring((prev) => { @@ -278,9 +284,20 @@ export function SyncActivityPanel() { } }; + // Only peers on a LOWER protocol version drive the "needs updating" banner; + // a peer merely on a different version (e.g. ahead) is informational and must + // not, on its own, keep the empty-state from showing. + const behindPeers = stalePeers.filter((p) => p.behind); + // Nothing recorded yet (fresh install, or sync never ran) — say so rather // than rendering an empty shell. - if (loaded && activity.length === 0 && restorable.length === 0 && fileFailures.length === 0) { + if ( + loaded && + activity.length === 0 && + restorable.length === 0 && + fileFailures.length === 0 && + behindPeers.length === 0 + ) { return (

No sync activity recorded yet. Once this device exchanges changes with your phone, decisions @@ -291,6 +308,22 @@ export function SyncActivityPanel() { return (

+ {behindPeers.length > 0 && ( +
+ +
+
+ {behindPeers.length === 1 + ? "Another device is on an older version" + : `${behindPeers.length} devices are on an older version`} +
+

+ Update Cairn on your other {behindPeers.length === 1 ? "device" : "devices"} so deletions + sync correctly. Until then, a note you delete here can reappear from the older device. +

+
+
+ )} {/* Rendered independently of the restorable list: once the row is live it leaves that list, but the missing file still needs fixing. */} {fileFailures.length > 0 && ( diff --git a/src/components/settings/shared.tsx b/src/components/settings/shared.tsx index 67d1370b..e7a91c9a 100644 --- a/src/components/settings/shared.tsx +++ b/src/components/settings/shared.tsx @@ -1,7 +1,7 @@ "use client"; import React, { useState, useRef, useEffect } from "react"; -import { Footprints, Thermometer, Layers, Loader2 } from "lucide-react"; +import { Footprints, Thermometer, Layers, Gauge, Loader2 } from "lucide-react"; import { cn } from "@/lib/utils"; import { Toggle as UiToggle, type ToggleProps } from "@/components/ui/toggle"; @@ -91,12 +91,13 @@ export function Toggle({ // ── Stepper settings row ─────────────────────── -export type StepperIcon = "footprints" | "thermometer" | "layers"; +export type StepperIcon = "footprints" | "thermometer" | "layers" | "gauge"; const ICON_MAP: Record> = { footprints: Footprints, thermometer: Thermometer, layers: Layers, + gauge: Gauge, }; /** @@ -119,6 +120,8 @@ export function StepperSettingsRow({ autoState, autoActive, onAuto, + autoSuppressesValue, + suppressedPlaceholder = "Auto", }: { label: string; description?: string; @@ -139,10 +142,21 @@ export function StepperSettingsRow({ autoActive?: boolean; /** Handler for the "Auto" quick-button (e.g. detect + apply a models.dev value). Always renders the button when provided. */ onAuto?: () => void; + /** + * When true, Auto means "no explicit value" (not "auto-applied number"), so + * the input shows a placeholder and no preset is highlighted — otherwise a + * previously-typed number lingers as if still selected after tapping Auto. + */ + autoSuppressesValue?: boolean; + /** Placeholder shown in the input when the value is suppressed by Auto. */ + suppressedPlaceholder?: string; }) { const Icon = ICON_MAP[icon]; const [draft, setDraft] = useState(String(value)); const blurringRef = useRef(false); + // When Auto is active and represents "no value", the row shows nothing + // selected: empty input (placeholder) and no highlighted preset. + const suppressed = Boolean(autoSuppressesValue && autoActive); useEffect(() => { if (!blurringRef.current) setDraft(String(value)); @@ -169,7 +183,8 @@ export function StepperSettingsRow({ min={min} max={max} step={step} - value={draft} + value={suppressed ? "" : draft} + placeholder={suppressed ? suppressedPlaceholder : undefined} onChange={(e) => setDraft(e.target.value)} onBlur={() => { blurringRef.current = true; @@ -215,7 +230,7 @@ export function StepperSettingsRow({ onClick={() => onChange(n)} className={cn( "px-2 py-1 text-[0.714rem] rounded border transition-colors", - value === n + !suppressed && value === n ? "border-[var(--accent)] text-[var(--accent)] bg-[var(--accent-dim)]" : "border-[var(--border)] text-[var(--text-tertiary)] hover:border-[var(--muted)] hover:text-[var(--text-secondary)]" )} diff --git a/src/components/shared/ConnectorToolCard.component.test.tsx b/src/components/shared/ConnectorToolCard.component.test.tsx new file mode 100644 index 00000000..124b57a1 --- /dev/null +++ b/src/components/shared/ConnectorToolCard.component.test.tsx @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { ConnectorToolCard, type ConnectorMeta, type ConnectorToolCall } from "./ConnectorToolCard"; + +/** + * Behaviour suite for the connector tool card (connector-inbound, + * connector-id-reveal). Pure-props, no live OAuth / external service — the + * connector metadata and tool call are supplied directly. + */ + +const connector: ConnectorMeta = { + name: "linear-mcp", + kind: "mcp", + brandColor: "#5E6AD2", + label: "Linear", +}; + +function renderCard(toolCall: ConnectorToolCall, meta: ConnectorMeta = connector) { + render(); +} + +describe("connector-inbound", () => { + it("shows the branded connector label and transport, collapsed by default", () => { + renderCard({ tool: "search_issues", args: { query: "sync" } }); + const card = screen.getByTestId("connector-message-card"); + expect(card).toBeTruthy(); + // Branded display label, not the raw internal name. + expect(screen.getByText("Linear")).toBeTruthy(); + expect(screen.getByText("via MCP")).toBeTruthy(); + // Collapsed: details (arguments/result) are not in the DOM yet. + expect(screen.getByRole("button").getAttribute("aria-expanded")).toBe("false"); + expect(screen.queryByText("Arguments")).toBeNull(); + }); + + it("labels an HTTP service connector as such", () => { + renderCard( + { tool: "send_message" }, + { name: "slack-svc", kind: "service", label: "Slack" }, + ); + expect(screen.getByText("via HTTP service")).toBeTruthy(); + }); + + it("expands to reveal the tool details on click", async () => { + const user = userEvent.setup(); + renderCard({ tool: "search_issues", args: { query: "sync" }, output: '{"count":3}' }); + + await user.click(screen.getByRole("button")); + expect(screen.getByRole("button").getAttribute("aria-expanded")).toBe("true"); + expect(screen.getByText("Arguments")).toBeTruthy(); + expect(screen.getByText("Result")).toBeTruthy(); + }); + + it("falls back to the raw name when no branded label is given", () => { + renderCard({ tool: "search_issues" }, { name: "acme-mcp", kind: "mcp" }); + expect(screen.getByText("acme-mcp")).toBeTruthy(); + }); +}); + +describe("connector-id-reveal", () => { + it("redacts secret-like arguments when the details are revealed", async () => { + const user = userEvent.setup(); + renderCard({ + tool: "create_issue", + args: { title: "Bug", apiKey: "sk-live-SUPERSECRET123", token: "ghp_abcdef" }, + }); + + await user.click(screen.getByRole("button")); + const details = screen.getByTestId("connector-message-card").textContent ?? ""; + // The visible title survives… + expect(details).toContain("Bug"); + // …but the secret values must never be rendered. + expect(details).not.toContain("sk-live-SUPERSECRET123"); + expect(details).not.toContain("ghp_abcdef"); + }); +}); diff --git a/src/hooks/useChatStream.ts b/src/hooks/useChatStream.ts index 6ab96e97..42ae5518 100644 --- a/src/hooks/useChatStream.ts +++ b/src/hooks/useChatStream.ts @@ -55,6 +55,8 @@ export interface ChatStreamRequest { apiKey?: string; maxSteps?: number; temperature?: number; + /** Max output/completion tokens per reply (resolved from model + user setting). */ + maxTokens?: number; }; systemPrompt?: string; /** Attachments on the current user message (base64 data URLs; kind tells diff --git a/src/lib/callout-widget.tsx b/src/lib/callout-widget.tsx index 6b1661ed..2d7c930f 100644 --- a/src/lib/callout-widget.tsx +++ b/src/lib/callout-widget.tsx @@ -13,6 +13,7 @@ import { EditorView, WidgetType } from "@codemirror/view"; import { createRoot, type Root } from "react-dom/client"; +import { flushSync } from "react-dom"; import { Callout, parseCalloutDirective } from "@/components/notes/Callout"; import { NoteMarkdownPreview } from "@/components/notes/NoteMarkdownPreview"; @@ -62,28 +63,40 @@ export class CalloutWidget extends WidgetType { toDOM(view: EditorView): HTMLElement { const container = document.createElement("div"); container.className = "cm-lp-callout"; - // Mount the React callout. NoteMarkdownPreview renders the body markdown so - // nested formatting (bold, links, lists) works inside the callout. + // Mount the React callout SYNCHRONOUSLY. This is the crux of the cursor-drift + // fix: CodeMirror measures the widget's height immediately after toDOM() + // returns, and it uses that height to position every line below it. If React + // painted asynchronously (a plain createRoot().render()), the container would + // still be ~0px tall at measure time, so CM placed the cursor several lines + // off — and no amount of requestMeasure() after the fact fully recovered, + // because the first (wrong) layout had already mapped click coordinates. + // flushSync forces the render to commit before we return, so the DOM has its + // real height the first time CM measures. `estimatedHeight = -1` keeps CM + // from ever assuming the widget matches the replaced source lines. this.root = createRoot(container); - this.root.render( - - {this.data.body ? : null} - , - ); - // React renders asynchronously, so the container has ~0 height when CM first - // measures it. That desyncs the layout below the widget (the cursor lands - // several lines off). Ask CM to re-measure once React has painted. A double - // rAF ensures the commit + layout have flushed before we remeasure. - requestAnimationFrame(() => { - requestAnimationFrame(() => { - if (this.root) view.requestMeasure(); - }); + flushSync(() => { + this.root?.render( + + {this.data.body ? : null} + , + ); }); + // The collapsible header toggles height at runtime (open/closed), which + // changes the widget's size AFTER the initial synchronous measure. The + // toggle is a React onClick inside ; listen on the container (fires + // for the same click) and re-measure once React has committed the new + // height — a double rAF waits out that commit. This is only for the toggle + // transition; initial layout is already correct from the flushSync above. + if (this.data.collapsible) { + container.addEventListener("click", () => { + requestAnimationFrame(() => requestAnimationFrame(() => view.requestMeasure())); + }); + } return container; } diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 20023130..1cb1e29f 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -101,6 +101,8 @@ export const DEFAULT_AI_CONFIG: AIConfig = { temperature: 0.3, contextLimit: 128000, contextAuto: true, + maxOutputTokens: 8192, + maxOutputAuto: true, aiEnabled: true, subagentsEnabled: false, }; @@ -114,6 +116,8 @@ export const DEFAULT_AGENT_CONFIG: AgentConfig = { temperature: 0.3, contextLimit: 128000, contextAuto: true, + maxOutputTokens: 8192, + maxOutputAuto: true, autoApprove: true, }; diff --git a/src/lib/livePreview.component.test.tsx b/src/lib/livePreview.component.test.tsx index ba256641..3432d352 100644 --- a/src/lib/livePreview.component.test.tsx +++ b/src/lib/livePreview.component.test.tsx @@ -88,19 +88,42 @@ describe("livePreview decorations", () => { view.destroy(); }); - it("does not render a callout widget while callouts are disabled", () => { - // Inline callout block widgets are temporarily disabled (CALLOUTS_ENABLED - // = false) due to a block-widget layout/cursor bug. Callout blockquotes - // should fall back to ordinary blockquote rendering (Tier 1 border), not a - // widget. Re-enable + restore the widget-behaviour tests when the layout - // issue is fixed. + it("renders a callout widget when the cursor is outside the block", () => { const doc = "intro line\n\n> [!note] Heads up\n> body text here\n\nafter"; + // Cursor on line 1, well outside the callout → it should render as a widget. const view = mount(doc, 0); const widget = view.contentDOM.querySelector(".cm-lp-callout"); - expect(widget).toBeNull(); - // Callout lines are still styled as blockquotes. - const quoted = view.contentDOM.querySelectorAll(".cm-lp-blockquote"); - expect(quoted.length).toBeGreaterThan(0); + expect(widget).toBeTruthy(); + view.destroy(); + }); + + it("shows raw source (no widget) when the cursor is inside the callout", () => { + const doc = "intro line\n\n> [!note] Heads up\n> body text here\n\nafter"; + // Place the cursor inside the callout's directive line so it unfolds to the + // editable `>` source instead of the widget. + const cursor = doc.indexOf("Heads up"); + const view = mount(doc, cursor); + expect(view.contentDOM.querySelector(".cm-lp-callout")).toBeNull(); + // The raw markers are visible for editing on the active block. + expect(view.contentDOM.textContent).toContain("[!note]"); + view.destroy(); + }); + + it("keeps document positions after a callout addressable (cursor-drift guard)", () => { + // The regression this whole card exists for: a callout widget must not + // desync the lines below it, so a position AFTER the callout must still map + // back to the correct line. flushSync in toDOM gives the widget real height + // at measure time, so coordsAt/lineBlockAt for a later position stay sane. + const doc = "intro\n\n> [!note] Title\n> body\n\ntarget line after callout"; + const view = mount(doc, 0); + expect(view.contentDOM.querySelector(".cm-lp-callout")).toBeTruthy(); + + const pos = doc.indexOf("target line after callout"); + const line = view.state.doc.lineAt(pos); + // The line the widget-relative geometry resolves to for this position must + // be the same line the document model reports — i.e. no off-by-N drift. + const block = view.lineBlockAt(pos); + expect(view.state.doc.lineAt(block.from).number).toBe(line.number); view.destroy(); }); }); diff --git a/src/lib/livePreview.ts b/src/lib/livePreview.ts index eae9cdc1..73eae15b 100644 --- a/src/lib/livePreview.ts +++ b/src/lib/livePreview.ts @@ -32,7 +32,7 @@ import { makeCalloutWidget, parseCalloutSource, calloutWidgetTheme } from "./cal // height/measurement issue is solved. See the Cairn "Notes Editor" board task. // When false: callout blockquotes render as ordinary blockquotes (Tier 1 border) // and the calloutField / widget are not added to the editor. -const CALLOUTS_ENABLED = false; +const CALLOUTS_ENABLED = true; // ── Widgets ──────────────────────────────────────────────────────────────── diff --git a/src/lib/models-dev.ts b/src/lib/models-dev.ts index 67839442..6e865fdc 100644 --- a/src/lib/models-dev.ts +++ b/src/lib/models-dev.ts @@ -29,11 +29,15 @@ import { parseModelCatalog, providerLogoUrl, type ModelInfo, + resolveMaxOutputTokens, } from "../../shared/models/model-catalog"; const API_URL = "https://models.dev/api.json"; -const CACHE_KEY = "ai.modelInfo.cache"; // JSON { [modelId]: ModelInfo } -const CACHE_AT_KEY = "ai.modelInfo.cachedAt"; +// v2: bumped when the parsed ModelInfo shape gains a field (maxOutput) so +// existing caches re-fetch immediately instead of waiting out the weekly TTL +// with the old field-poor entries. +const CACHE_KEY = "ai.modelInfo.cache.v2"; // JSON { [modelId]: ModelInfo } +const CACHE_AT_KEY = "ai.modelInfo.cachedAt.v2"; const CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; // refresh weekly // Canonical owner map (models.json): id → provider slug for logo resolution. // Tiny (282 entries) and keyed by canonical "/" ids, so it's @@ -250,6 +254,18 @@ export function prewarmModelCatalog(): void { void ensureMap(); } +/** + * The `max_tokens` to send for a chat request, or `undefined` to omit the field. + * `userOverride` is the user's explicit "Max output tokens" (undefined/0 = Auto, + * which omits the cap so the model finishes naturally). Kept async for a stable + * call site even though it no longer needs the catalog. + */ +export async function maxOutputTokensForModel( + userOverride?: number | null, +): Promise { + return resolveMaxOutputTokens(userOverride); +} + /** * The provider slug whose logo identifies `modelId`. Prefers the canonical * owner from models.json, then the brand heuristic, then the flattened diff --git a/src/lib/sync-client.ts b/src/lib/sync-client.ts index 9d885eb6..166fc63f 100644 --- a/src/lib/sync-client.ts +++ b/src/lib/sync-client.ts @@ -78,7 +78,8 @@ export type RestoreRefusal = | "conflict-copy" | "orphaned" | "self-deleted" - | "no-delete-record"; + | "no-delete-record" + | "preserved-as-copy"; export interface RestoreOutcome { restored: boolean; @@ -93,6 +94,14 @@ export interface RepairOutcome { fileError?: string; } +/** A peer device on a different sync protocol version than this build. */ +export interface PeerProtocol { + deviceId: string; + version: number; + /** True when the peer is on an OLDER protocol — it may not honour deletes. */ + behind: boolean; +} + type SyncApi = { status: () => Promise; onStatus: (cb: (s: SyncStatus) => void) => () => void; @@ -107,6 +116,7 @@ type SyncApi = { listRestorable: (limit?: number) => Promise<{ rows: RestorableNote[]; total: number }>; restoreNote: (id: string) => Promise; repairNoteFile: (id: string) => Promise; + peerProtocols: () => Promise; }; export function syncApi(): SyncApi | null { @@ -250,6 +260,17 @@ export async function repairNoteFile(id: string): Promise { } } +/** Peers on an older/newer sync protocol than this build. Empty outside Electron. */ +export async function fetchPeerProtocols(): Promise { + const api = syncApi(); + if (!api?.peerProtocols) return []; + try { + return (await api.peerProtocols()) ?? []; + } catch { + return []; + } +} + /** Trigger a manual sync now (from the popover). */ export async function triggerSyncNow(): Promise { const api = syncApi(); diff --git a/src/lib/tool-risk.test.ts b/src/lib/tool-risk.test.ts index 86c1101f..1a0f6254 100644 --- a/src/lib/tool-risk.test.ts +++ b/src/lib/tool-risk.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { approvalPreview, riskForTool } from "./tool-risk"; +import { approvalPreview, riskForTool, approvalGrantScope, approvalScopeLabel } from "./tool-risk"; describe("tool approval presentation rules", () => { it("classifies local, executable, and external tools", () => { @@ -20,4 +20,40 @@ describe("tool approval presentation rules", () => { expect(approvalPreview("bash", {})).toBe(""); expect(approvalPreview("write", {})).toBe(""); }); + + // approval-grant-scope: which standing-grant affordance the approval card + // offers per tool. This is the security-relevant part — a wrong mapping would + // let a consequential action be pre-authorised too broadly. + it("scopes a bash grant to the exact command", () => { + expect(approvalGrantScope("bash")).toBe("command"); + }); + + it("scopes local-write and external grants to the session", () => { + expect(approvalGrantScope("write")).toBe("session"); + expect(approvalGrantScope("update_task")).toBe("session"); + expect(approvalGrantScope("mcp__linear__create_issue")).toBe("session"); + expect(approvalGrantScope("svc__slack__post")).toBe("session"); + }); + + it("offers no standing grant for reads or non-bash exec", () => { + expect(approvalGrantScope("read")).toBe("none"); + expect(approvalGrantScope("grep")).toBe("none"); + // spawn_subagent is EXEC but not bash → no command-scoped grant, and EXEC is + // excluded from session grants, so one-off only. + expect(approvalGrantScope("spawn_subagent")).toBe("none"); + }); + + it("states where each action reaches, distinguishing local from external", () => { + expect(approvalScopeLabel("read")).toContain("stays on this device"); + expect(approvalScopeLabel("write")).toContain("stays on this device"); + expect(approvalScopeLabel("bash")).toContain("runs on this device"); + expect(approvalScopeLabel("mcp__tavily__search")).toContain("leaves this device"); + }); + + it("uses platform-neutral wording (no hardcoded 'Mac')", () => { + // Cairn is a cross-platform desktop app — the approval copy must not assume macOS. + for (const tool of ["read", "write", "bash", "mcp__x__y"]) { + expect(approvalScopeLabel(tool)).not.toMatch(/\bMac\b/); + } + }); }); diff --git a/src/lib/tool-risk.ts b/src/lib/tool-risk.ts index 0acb23bd..8f036461 100644 --- a/src/lib/tool-risk.ts +++ b/src/lib/tool-risk.ts @@ -20,3 +20,32 @@ export function approvalPreview(name: string, args: Record = {} const clamped = lines.join("\n").slice(0, 420); return clamped + (clamped.length < text.length ? "…" : ""); } + +/** + * Which "always allow" grant the approval card offers for a tool, plus a + * plain-English statement of where the action reaches. Pure so the approval + * card's scoping can be verified without an Electron/IPC harness: + * + * - `command` — bash only: bind the standing grant to this exact command. + * - `session` — writes and external calls: allow this tool for the session. + * - `none` — reads and other exec: one-off allow/deny only, no standing grant. + * + * A READ never offers a standing grant (nothing consequential to pre-authorise), + * and an EXTERNAL action is always scoped to the session (never a blanket + * "always", since it leaves the machine). + */ +export function approvalGrantScope(name: string): "command" | "session" | "none" { + const risk = riskForTool(name); + if (risk === "EXEC" && name === "bash") return "command"; + if (risk !== "READ" && risk !== "EXEC") return "session"; + return "none"; +} + +/** Where an action reaches, for the approval card's one-line scope note. */ +export function approvalScopeLabel(name: string): string { + const risk = riskForTool(name); + // "this device" rather than "this Mac" — Cairn ships on Windows and Linux too. + if (risk === "EXTERNAL") return "leaves this device via a connected service"; + if (risk === "EXEC") return "runs on this device"; + return "stays on this device"; +} diff --git a/src/store/slices/ui.ts b/src/store/slices/ui.ts index e993440f..54a3ece4 100644 --- a/src/store/slices/ui.ts +++ b/src/store/slices/ui.ts @@ -251,6 +251,16 @@ export interface AIConfig { * value (custom input or preset). Defaults to true. */ contextAuto?: boolean; + /** + * Max output (completion) tokens to request per reply. When `maxOutputAuto` + * is true this is ignored and the value is resolved from the model's + * models.dev `limit.output` (bounded). A manual value lets power users pin a + * ceiling — important for "thinking" models, whose reasoning counts against + * this budget, so too small a value yields an empty reply. Defaults to Auto. + */ + maxOutputTokens?: number; + /** When true, max output tokens auto-resolves from the catalog. Defaults to true. */ + maxOutputAuto?: boolean; /** When false, all in-app AI features are hidden/disabled. Defaults to true. */ aiEnabled: boolean; /** @@ -290,6 +300,15 @@ export interface AgentConfig { * value (custom input or preset). Defaults to true. */ contextAuto?: boolean; + /** + * Max output tokens per turn. When `maxOutputAuto` is true (default) this is + * ignored and no cap is sent, so the model finishes naturally — capping only + * ever truncates a long reasoning+answer and can stall "thinking" models. A + * manual value is a deliberate cost/latency ceiling. + */ + maxOutputTokens?: number; + /** When true, no max-output cap is sent. Defaults to true. */ + maxOutputAuto?: boolean; /** Automatically approve tool execution without prompt. */ autoApprove: boolean; /**