diff --git a/.env.example b/.env.example index a0fab25..6422936 100644 --- a/.env.example +++ b/.env.example @@ -9,9 +9,10 @@ OLLAMA_MODEL=qwen2.5:3b # reasoning OLLAMA_VISION_MODEL=qwen2.5vl:7b # vision (needs substantial RAM) OLLAMA_EMBED_MODEL=nomic-embed-text # embeddings OLLAMA_EMBED_DIM=768 # set if OLLAMA_EMBED_MODEL is not 768-dim +# OLLAMA_VISION_TIMEOUT_MS=300000 # abort bound for vision calls (qwen2.5vl is slow: 40-150s is normal) # --- Executor --- -RORO_WORKDIR=/abs/path/to/scratch-git-repo # explicit dev repo; packaged builds use Choose Project +RORO_WORKDIR=/abs/path/to/scratch-git-repo # dev repo when no project is saved in-app (the saved choice wins) # RORO_ALLOW_CWD=1 # optional local-dev opt-in to use the launch cwd ANTHROPIC_API_KEY= # optional, only for the Claude executor # RORO_CODEX_BIN= # optional override for the codex binary path diff --git a/HANDOFF.md b/HANDOFF.md index 43c0214..566546c 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -49,7 +49,7 @@ The W7 split (#149) put **all product logic in `src/core/`** (Electron-free) wit | `src/core/orchestrator/` | The **turnRun chokepoint**, now a typed state machine. `orchestrator.ts` = the facade (`runTurn`/`runTask`/`cancelTask`/`resolveDestructiveConfirm`); `run/` = the **Turn machine** (`turnState.ts` — UI truth) + **Pump machine** (`pump.ts` — process truth) + `runRegistry.ts` (live Turns + the single-executor **slot** + the `DispatchSection` TOCTOU-lock-as-type) + `gates.ts` (the pre-dispatch pipeline) + `decisionRouter.ts` (the bounded two-pass decide loop). Plus `siblings.ts` (typed lazy brain/memory/vision loaders), `factStore.ts` (supersede-not-overwrite), `factProposals/` (the executor-facts pilot), `confirmGate.ts`, `destructive.ts`, `workdir.ts`, `identity.ts` (owner_id), `memoryContext.ts`. | | `src/core/brain/` | Local brain. `index.ts` (decide/extractFact/embed; `BRAIN_PROVIDER`≠`'ollama'` throws), `extractFact.ts` (marker gate + parser + value guard), `clarifyGate.ts`/`locateGate.ts`, `ollama.ts`, `eval/` (the eval harness: 50 behavioral fixtures + null-discipline axis + the `eval:proposals` runner + `baseline.json`). | | `src/core/memory2/` | Memory engine. `index.ts` (`createMemoryFacade` — the memory surface — + `traceExtraction`), `memoryStore.ts` (files-as-truth + reconcile), `memIndex.ts` (in-memory index behind `IndexStore`) + `vectorCache.ts` (the zero-authority `index/vectors.jsonl` sidecar), `manifestCompact.ts` (compaction **with** the fact-tier exemption), `profileFacts.ts` (correction/source view + executor-proposal provenance pass-through), `keyManager.ts`/`safeStorageWrapper.ts`/`cipher.ts` (envelope encryption), `tracer.ts`, `memoryScore.ts` (recall blend). The old `adapter.ts` + `pgliteIndex.ts` are **deleted**; the cursor is ephemeral (every open replays the manifest from files). | -| `src/core/executor/` | Coding-agent dispatch (edits files in `RORO_WORKDIR`). Default = the `codex` + `claude` **CLI** adapters (`codex.ts`, `claude.ts`). A **dark, flag-gated** (`RORO_SDK_EXECUTOR`) **Agent-SDK** adapter (`claudeSdk.ts`) adds **pre-execution** destructive gating (`claudeSdkGate.ts`: a PreToolUse hook + `canUseTool`, memoized per `toolUseId`). | +| `src/core/executor/` | Coding-agent dispatch (edits files in the configured project — the persisted in-app choice, else `RORO_WORKDIR`). Default = the `codex` + `claude` **CLI** adapters (`codex.ts`, `claude.ts`). A **dark, flag-gated** (`RORO_SDK_EXECUTOR`) **Agent-SDK** adapter (`claudeSdk.ts`) adds **pre-execution** destructive gating (`claudeSdkGate.ts`: a PreToolUse hook + `canUseTool`, memoized per `toolUseId`). | | `src/core/vision/` | Screen capture describe/locate (sharp + vision model). | | `src/core/ambient/` | The **dormant, gated-off** ambient track (belief-latch + eye + trigger). | | `src/renderer/` | PixiJS pixel cat + UI. `character/`, `ask/`, `memory/` (the correction panel + the fact-proposal confirm), `voice/` (a thin client of `packages/voice`), `bootstrap/`, `cosmetics/`, `confirm/`, `events/bridge.ts`. | @@ -292,7 +292,7 @@ memory/embeddings). Add new expensive lessons there, not here. - **Don't commit `.env`** (gitignored). Historical predecessor/demo checkouts are archived context only and must not be edited as part of Roro work. - **Verify before claiming done** — "the types check" is not done; "I observed it working" is. - **Key commands:** `npm test`, `npm run lint`, `npx tsc --noEmit -p tsconfig.json`, `npm run release:doctor` (CI-safe release/signing doctor), `npm run package` (.app), `npm run verify:floating-geometry` (default 360x420 transparent click-through pet shell), `npm run package:release` + `npm run verify:release-channel` (release-channel deferred-flag refusal), `npm run verify:packaged-memory` (packaged write/relaunch/recall), `npm run verify:packaged-live-memory-turn` (packaged relaunch + live Ollama turn uses recalled memory), `npm run verify:packaged-natural-memory-turn` (packaged natural-language teach/relaunch/recall), `npm run verify:packaged-real-codex` (opt-in packaged first task with the user's real authenticated/configured Codex CLI), `npm run verify:memory-steered` (synthetic-marker DECIDE/args.task proof), `npm run verify:memory-panel-rendered` (opt-in local GUI smoke for rendered Memory panel keyboard/focus behavior), `npm run verify:signing-readiness` (strict Developer-ID env/cert/tool doctor), `npm run verify:signing-auth` (notarytool Apple credential auth check), `npm run make` (+ distributables + signing), `npm run verify:release-artifact:dmg` (post-make DMG verifier), `npm run verify:release-artifact:signed` (post-make signed/notarized artifact verifier), `npm start` (dev — memory works here), `OLLAMA_AVAILABLE=1 npx vitest run crosslaunch.live` (live magic-moment smoke), `npm run eval:brain` (scorecard), `EVAL_SET=behavioral npm run eval:brain`. -- **State lives in:** memory + owner.json + packaged config → `app.getPath('userData')` (override `RORO_DB_DIR`). The agent's working repo resolves from explicit `RORO_WORKDIR`, then persisted `userData/config.json`, then the explicit `RORO_ALLOW_CWD=1` dev fallback. +- **State lives in:** memory + owner.json + packaged config → `app.getPath('userData')` (override `RORO_DB_DIR`). The agent's working repo resolves from the persisted in-app choice (`userData/config.json`), then the `RORO_WORKDIR` env var, then the explicit `RORO_ALLOW_CWD=1` cwd fallback (dev). --- diff --git a/PUBLIC.md b/PUBLIC.md index 5058b7c..ff0edea 100644 --- a/PUBLIC.md +++ b/PUBLIC.md @@ -102,7 +102,8 @@ Roro is public-ready when **all** of these are observed (not code-read): no shell for the Roro setup path. Executor CLIs may still require their own local auth, separate from Roro's brain/model key promise. - [ ] The chosen working repo persists in `userData/config.json` and survives relaunch; the executor never throws - "Roro has no working repo set" for a user who completed onboarding. `RORO_WORKDIR` remains an explicit env override. + "Roro has no working repo set" for a user who completed onboarding. `RORO_WORKDIR` applies only when no project + has been chosen in-app (the persisted choice wins). - [ ] **The heart:** on the signed build, a fact stored in session 1 is recalled in session 2 after a **full quit + relaunch** — `safeStorage.isEncryptionAvailable()` is true, the AES-256-GCM envelope round-trips. - [ ] **A non-founder** observes the magic moment: types a task, Roro recalls a prior fact and **uses it to shape the @@ -159,8 +160,8 @@ sharpened magic-moment gate above) across a full quit/relaunch of a packaged bui the happy packaged first-run workdir flow. **Goal:** take a stranger from launch → a successful coding turn, no shell. *(All memory-architecture-independent.)* -- ✅ `userData/config.json` read/write for the working repo; `resolveWorkdir` honors explicit env first, then persisted - config, then the explicit `RORO_ALLOW_CWD=1` dev fallback. +- ✅ `userData/config.json` read/write for the working repo; `resolveWorkdir` honors persisted config first, then + explicit env, then the explicit `RORO_ALLOW_CWD=1` dev fallback. - ✅ First-run flow: no workdir → native folder-picker ("Which project should Roro work on?") → persist; typed and floating tasks are gated until a project exists. - ✅ Raw "Roro has no working repo set" is no longer the normal user path; the UI asks for a project before dispatch. @@ -171,7 +172,7 @@ the happy packaged first-run workdir flow. - ✅ Packaged model-setup preflight: `npm run verify:packaged-model-setup` proves reachable-Ollama/missing-model setup plus daemon-down → Recheck recovery in the packaged app, then completes the essential model pull path. - ✅ Project control in Settings: after first setup, the user can see the active repo and change the saved project - without relaunching; `RORO_WORKDIR` remains an explicit read-only override. + without relaunching; `RORO_WORKDIR` applies only when no project is saved (the saved project wins). - ✅ Packaged first-task preflight: `npm run verify:packaged-first-task` proves the persisted project, local-brain readiness, public selected-executor readiness bridge, and public `turnRun` path can produce a file change in the chosen repo without debug bridges. diff --git a/README.md b/README.md index 7ae20ec..c8bc4ce 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,8 @@ npm start In a packaged build, Roro asks you to choose the project folder it should work on, stores that choice in `userData/config.json`, and reuses it after relaunch. -In development, `RORO_WORKDIR` is still the fastest explicit override. +The working repo resolves from the persisted in-app choice first, then the +`RORO_WORKDIR` env var, then the explicit `RORO_ALLOW_CWD=1` cwd fallback (dev). If Roro says no working repo is set, choose a project in Settings or relaunch with `RORO_WORKDIR=/absolute/path/to/repo npm start`; blank or stale paths are treated as unset so the executor never runs against a guessed directory. @@ -169,7 +170,8 @@ OLLAMA_VISION_MODEL=qwen2.5vl:7b OLLAMA_EMBED_MODEL=nomic-embed-text OLLAMA_EMBED_DIM=768 # set this if OLLAMA_EMBED_MODEL is not 768-dim -# Optional dev override. Packaged first-run uses the native Choose Project flow. +# Optional dev fallback (a project saved in-app wins over this). Packaged +# first-run uses the native Choose Project flow. RORO_WORKDIR=/absolute/path/to/scratch-git-repo ANTHROPIC_API_KEY=... # optional, only for the Claude executor ``` diff --git a/RUN.md b/RUN.md index c748ceb..e0d5816 100644 --- a/RUN.md +++ b/RUN.md @@ -34,7 +34,7 @@ OLLAMA_VISION_MODEL=qwen2.5vl:7b OLLAMA_EMBED_MODEL=nomic-embed-text OLLAMA_EMBED_DIM=768 # set this if OLLAMA_EMBED_MODEL is not 768-dim -RORO_WORKDIR=/abs/path/to/scratch-git-repo # the repo the agent actually codes in +RORO_WORKDIR=/abs/path/to/scratch-git-repo # the repo the agent codes in (a project saved in-app wins over this) ANTHROPIC_API_KEY=... # only if you use the Claude executor ``` @@ -109,7 +109,7 @@ npm start ``` Summon (Cmd+Shift+Space) → type a task in the floating Ask (or the dev prompt box) → the -cat thinks (driven by the local brain's content stream) → drives Codex in `RORO_WORKDIR` +cat thinks (driven by the local brain's content stream) → drives Codex in the configured project (the in-app choice, else `RORO_WORKDIR`) (each action narrated + animated) → writes to local encrypted memory. Then ask *"what did we do?"* for the semantic-recall beat. diff --git a/scripts/smoke-floating-live-turn.mjs b/scripts/smoke-floating-live-turn.mjs index 645d6ee..20b7f98 100644 --- a/scripts/smoke-floating-live-turn.mjs +++ b/scripts/smoke-floating-live-turn.mjs @@ -454,6 +454,9 @@ await writeFakeCodexBin(fakeCodexBin, fakeCodexArgsFile, fakeCodexActiveStopMark const fakeOllama = USE_REAL_OLLAMA ? null : await startFakeOllama(); const appEnv = stripV0DeferredEnv({ ...process.env, + // Disposable HOME → fresh userData: a persisted config.json on the dev machine would win over + // RORO_WORKDIR (the deliberate precedence reversal after the first dogfooding session). + HOME: root, BRAIN_PROVIDER: 'ollama', ...(fakeOllama ? { OLLAMA_HOST: fakeOllama.host, OLLAMA_TIMEOUT_MS: '5000' } : {}), RORO_DEBUG_PORT: PORT, diff --git a/scripts/smoke-memory-panel-rendered.mjs b/scripts/smoke-memory-panel-rendered.mjs index 2951b43..ad01bb9 100644 --- a/scripts/smoke-memory-panel-rendered.mjs +++ b/scripts/smoke-memory-panel-rendered.mjs @@ -19,6 +19,9 @@ const root = await mkdtemp(join(tmpdir(), 'roro-memory-panel-rendered-')); const inheritedEnv = stripV0DeferredEnv({ ...process.env }); const appEnv = { ...inheritedEnv, + // Disposable HOME → fresh userData: a persisted config.json on the dev machine would win over + // RORO_WORKDIR (the deliberate precedence reversal after the first dogfooding session). + HOME: root, RORO_DEBUG_PORT: PORT, RORO_MEMORY_PANEL_SMOKE: '1', RORO_DISABLE_MEMORY_WARMUP: '1', diff --git a/src/core/brain/index.ts b/src/core/brain/index.ts index dfac77b..2e21052 100644 --- a/src/core/brain/index.ts +++ b/src/core/brain/index.ts @@ -234,10 +234,12 @@ export async function extractFact(input: FactExtractInput): Promise 0 ? v : 300_000; } diff --git a/src/core/brain/index.visionTimeout.test.ts b/src/core/brain/index.visionTimeout.test.ts new file mode 100644 index 0000000..8ffeda7 --- /dev/null +++ b/src/core/brain/index.visionTimeout.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describeScreen, groundTarget, visionTimeoutMs } from './index'; +import { OllamaTimeoutError } from './ollama'; + +// Dogfooding fix: a wedged/slow qwen2.5vl call must never hang a capture_screen turn — the live +// vision calls carry an ABORTING per-call timeout (OLLAMA_VISION_TIMEOUT_MS, generous 300s default: +// 40-150s per call is NORMAL for the local 7B) and surface the typed OllamaTimeoutError, which the +// decision router turns into run.failed ('vision failed: ...') — never a silent hang, never a +// fabricated success. + +// A fetch that NEVER resolves but honors AbortSignal — a daemon that connected then wedged. +const hangingFetch = (_url: string, opts?: { signal?: AbortSignal }): Promise => + new Promise((_resolve, reject) => { + const signal = opts?.signal; + signal?.addEventListener('abort', () => reject(signal.reason)); + }); + +const screen = { b64: 'aGk=', mime: 'image/jpeg', width: 1280, height: 720 }; + +describe('vision call timeout (a hung daemon must fail the turn, not hang it)', () => { + beforeEach(() => { + delete process.env.BRAIN_PROVIDER; // force the local Ollama path + }); + + afterEach(() => { + vi.unstubAllGlobals(); + delete process.env.OLLAMA_VISION_TIMEOUT_MS; + delete process.env.OLLAMA_TIMEOUT_MS; + }); + + it('defaults to a generous 300s bound (vision is SLOW; 40-150s per call is normal)', () => { + expect(visionTimeoutMs()).toBe(300_000); + }); + + it('honors the OLLAMA_VISION_TIMEOUT_MS override and ignores garbage', () => { + process.env.OLLAMA_VISION_TIMEOUT_MS = '45000'; + expect(visionTimeoutMs()).toBe(45_000); + process.env.OLLAMA_VISION_TIMEOUT_MS = 'not-a-number'; + expect(visionTimeoutMs()).toBe(300_000); + process.env.OLLAMA_VISION_TIMEOUT_MS = '-1'; + expect(visionTimeoutMs()).toBe(300_000); + }); + + it('describeScreen aborts a wedged vision call at the VISION bound and surfaces the typed error', async () => { + process.env.OLLAMA_TIMEOUT_MS = '600000'; // reason-model default must not be what fires or is reported + process.env.OLLAMA_VISION_TIMEOUT_MS = '30'; + vi.stubGlobal('fetch', hangingFetch); + + const err = await describeScreen(screen).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(30); + expect((err as Error).message).toMatch(/timed out after 30ms/i); + }); + + it('groundTarget (the locate fast path) aborts at the same vision bound, typed', async () => { + process.env.OLLAMA_VISION_TIMEOUT_MS = '30'; + vi.stubGlobal('fetch', hangingFetch); + + const err = await groundTarget(screen, 'the failing test').catch((e: unknown) => e); + + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(30); + }); +}); diff --git a/src/core/brain/ollama.test.ts b/src/core/brain/ollama.test.ts index 1d065f1..115d941 100644 --- a/src/core/brain/ollama.test.ts +++ b/src/core/brain/ollama.test.ts @@ -9,6 +9,7 @@ import { assertEmbedDimMatch, ollamaChat, ollamaTags, + OllamaTimeoutError, parsePullProgress, } from './ollama'; @@ -133,9 +134,69 @@ describe('ollama fetch timeout (a wedged daemon must fail, not hang)', () => { await expect(ollamaChat({ model: 'm', user: 'hi', stream: false })).rejects.toThrow(/timed out/i); }); + it('a wedged daemon surfaces the TYPED OllamaTimeoutError carrying the effective bound', async () => { + process.env.OLLAMA_TIMEOUT_MS = '20'; + vi.stubGlobal('fetch', hangingFetch); + const err = await ollamaChat({ model: 'm', user: 'hi', stream: false }).catch((e: unknown) => e); + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(20); + }); + + // Dogfooding fix: a per-call override (the slow vision model) must abort at ITS bound and REPORT + // that bound — the old message always claimed the 120s OLLAMA_TIMEOUT_MS default, misdiagnosing a + // 300s vision timeout and pointing at the wrong env remedy. + it('a per-call timeoutMs override aborts at its own bound and reports THAT bound, typed', async () => { + process.env.OLLAMA_TIMEOUT_MS = '600000'; // the default must be neither what fires nor what is reported + vi.stubGlobal('fetch', hangingFetch); + const err = await ollamaChat({ model: 'v', user: 'see', images: ['abc'], stream: false, timeoutMs: 25 }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(25); + expect((err as Error).message).toMatch(/timed out after 25ms/i); + }); + it('ollamaTags rejects with a TIMEOUT error when /api/tags wedges', async () => { process.env.OLLAMA_TIMEOUT_MS = '20'; vi.stubGlobal('fetch', hangingFetch); await expect(ollamaTags()).rejects.toThrow(/timed out/i); }); + + // The abort can also fire AFTER headers arrive (the daemon sends headers then wedges): the rejection + // then surfaces from res.json()/reader.read(), NOT fetch(). It must be the same TYPED error carrying + // the bound that actually fired — not a raw untyped 'TimeoutError' with no bound and no remedy. + const timeoutAbort = (): Error => + Object.assign(new Error('The operation was aborted due to timeout'), { name: 'TimeoutError' }); + + it('a post-header stall on the stream:false body (res.json) surfaces the TYPED error + bound', async () => { + vi.stubGlobal('fetch', async (): Promise => ({ + ok: true, + body: null, + json: () => Promise.reject(timeoutAbort()), + })); + const err = await ollamaChat({ model: 'v', user: 'see', images: ['abc'], stream: false, timeoutMs: 25 }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(25); + }); + + it('a post-header stall mid-stream (reader.read) surfaces the TYPED error + bound', async () => { + vi.stubGlobal('fetch', async (): Promise => ({ + ok: true, + body: { getReader: () => ({ read: () => Promise.reject(timeoutAbort()) }) }, + })); + const err = await ollamaChat({ model: 'm', user: 'hi', stream: true, timeoutMs: 30 }).catch((e: unknown) => e); + expect(err).toBeInstanceOf(OllamaTimeoutError); + expect((err as OllamaTimeoutError).timeoutMs).toBe(30); + }); + + it('a non-timeout body failure propagates untouched (never masked as a timeout)', async () => { + vi.stubGlobal('fetch', async (): Promise => ({ + ok: true, + body: null, + json: () => Promise.reject(new Error('bad json')), + })); + const err = await ollamaChat({ model: 'm', user: 'hi', stream: false }).catch((e: unknown) => e); + expect(err).not.toBeInstanceOf(OllamaTimeoutError); + expect((err as Error).message).toBe('bad json'); + }); }); diff --git a/src/core/brain/ollama.ts b/src/core/brain/ollama.ts index d82e928..3fedd17 100644 --- a/src/core/brain/ollama.ts +++ b/src/core/brain/ollama.ts @@ -21,6 +21,22 @@ function ollamaTimeoutMs(): number { return Number.isFinite(v) && v > 0 ? v : 120_000; } +/** TYPED timeout (fail-loud, never a silent hang): the HTTP call was ABORTED at `timeoutMs` — the bound + * that actually fired (a per-call override like the vision calls', or the OLLAMA_TIMEOUT_MS default), + * so a 300s vision timeout is never misreported as the 120s chat default. */ +export class OllamaTimeoutError extends Error { + readonly timeoutMs: number; + + constructor(timeoutMs: number) { + super( + `Ollama timed out after ${timeoutMs}ms at ${ollamaHost()} (the daemon may be wedged or a model is ` + + `still loading). Raise OLLAMA_TIMEOUT_MS (vision calls: OLLAMA_VISION_TIMEOUT_MS), or restart it: ollama serve`, + ); + this.name = 'OllamaTimeoutError'; + this.timeoutMs = timeoutMs; + } +} + export interface ChatMessage { role: 'system' | 'user' | 'assistant'; content: string; @@ -171,13 +187,22 @@ export async function ollamaChat(opts: OllamaChatOpts): Promise { const stream = opts.stream ?? false; const body = buildChatBody({ model: opts.model, messages, stream, json: opts.json, temperature: opts.temperature }); - const res = await fetchOllama('/api/chat', body, opts.timeoutMs); + const { res, effectiveTimeoutMs } = await fetchOllama('/api/chat', body, opts.timeoutMs); if (!res.ok) { throw new Error(`Ollama chat failed ${res.status}: ${await res.text().catch(() => '')}`); } + // The abort can also fire AFTER headers arrive (the daemon sends headers then wedges): it then + // surfaces from res.json()/reader.read(), not fetch() — translate it to the same TYPED error with + // the bound that fired. Any other body failure (bad JSON, reset) propagates untouched. if (!stream || !res.body) { - const j = (await res.json()) as { message?: { content?: unknown } }; + let j: { message?: { content?: unknown } }; + try { + j = (await res.json()) as { message?: { content?: unknown } }; + } catch (err) { + if (isTimeoutAbort(err)) throw new OllamaTimeoutError(effectiveTimeoutMs); + throw err; + } const content = j.message?.content; if (typeof content !== 'string') throw new Error('Ollama chat returned no content'); return content; @@ -188,20 +213,25 @@ export async function ollamaChat(opts: OllamaChatOpts): Promise { const decoder = new TextDecoder(); let buf = ''; let content = ''; - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - buf += decoder.decode(value, { stream: true }); - let nl: number; - while ((nl = buf.indexOf('\n')) !== -1) { - const line = buf.slice(0, nl); - buf = buf.slice(nl + 1); - const { delta } = parseChatLine(line); - if (delta) { - content += delta; - opts.onContent?.(delta); + try { + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + buf += decoder.decode(value, { stream: true }); + let nl: number; + while ((nl = buf.indexOf('\n')) !== -1) { + const line = buf.slice(0, nl); + buf = buf.slice(nl + 1); + const { delta } = parseChatLine(line); + if (delta) { + content += delta; + opts.onContent?.(delta); + } } } + } catch (err) { + if (isTimeoutAbort(err)) throw new OllamaTimeoutError(effectiveTimeoutMs); + throw err; } const { delta } = parseChatLine(buf); // trailing line without a newline if (delta) { @@ -213,7 +243,7 @@ export async function ollamaChat(opts: OllamaChatOpts): Promise { /** Call /api/embed for one or many inputs; returns one vector per input, index-aligned. */ export async function ollamaEmbed(model: string, input: string | string[]): Promise { - const res = await fetchOllama('/api/embed', buildEmbedBody(model, input)); + const { res } = await fetchOllama('/api/embed', buildEmbedBody(model, input)); if (!res.ok) { throw new Error(`Ollama embed failed ${res.status}: ${await res.text().catch(() => '')}`); } @@ -226,11 +256,12 @@ export async function ollamaEmbed(model: string, input: string | string[]): Prom /** Installed model names from /api/tags (e.g. ['qwen2.5:3b', 'nomic-embed-text:latest']). */ export async function ollamaTags(): Promise { + const timeoutMs = ollamaTimeoutMs(); let res: Response; try { - res = await fetch(`${ollamaHost()}/api/tags`, { signal: AbortSignal.timeout(ollamaTimeoutMs()) }); + res = await fetch(`${ollamaHost()}/api/tags`, { signal: AbortSignal.timeout(timeoutMs) }); } catch (err) { - throw new Error(ollamaFetchError(err)); + throw ollamaFetchError(err, timeoutMs); } if (!res.ok) throw new Error(`Ollama tags failed ${res.status}`); const j = (await res.json()) as { models?: Array<{ name?: unknown }> }; @@ -283,27 +314,44 @@ export async function pullModel( emit(buf); // trailing line without a newline } -async function fetchOllama(path: string, body: unknown, timeoutMs?: number): Promise { +/** Returns the effective timeout ALONGSIDE the Response: the same AbortSignal keeps running while the + * caller consumes the body, so a post-header abort needs this bound to report the typed error truthfully. */ +async function fetchOllama( + path: string, + body: unknown, + timeoutMs?: number, +): Promise<{ res: Response; effectiveTimeoutMs: number }> { const url = `${ollamaHost()}${path}`; + const effectiveTimeoutMs = timeoutMs ?? ollamaTimeoutMs(); try { - return await fetch(url, { + const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), // a wedged daemon must fail, not hang the turn — but vision calls need a longer bound (see callers). - signal: AbortSignal.timeout(timeoutMs ?? ollamaTimeoutMs()), + signal: AbortSignal.timeout(effectiveTimeoutMs), }); + return { res, effectiveTimeoutMs }; } catch (err) { - throw new Error(ollamaFetchError(err)); + throw ollamaFetchError(err, effectiveTimeoutMs); } } +/** True when a rejection is the AbortSignal.timeout() abort — either the direct 'TimeoutError' or an + * 'AbortError' whose cause is the timeout (how some runtimes surface a mid-body abort). */ +function isTimeoutAbort(err: unknown): boolean { + const e = err as { name?: unknown; cause?: { name?: unknown } } | null; + if (e?.name === 'TimeoutError') return true; + return e?.name === 'AbortError' && e.cause?.name === 'TimeoutError'; +} + /** Distinguish a TIMEOUT (daemon wedged / model loading) from an UNREACHABLE daemon — both are actionable, - * but the remedies differ. AbortSignal.timeout() rejects with a TimeoutError. */ -function ollamaFetchError(err: unknown): string { + * but the remedies differ. AbortSignal.timeout() rejects with a TimeoutError; it becomes the TYPED + * OllamaTimeoutError reporting the bound that actually fired. */ +function ollamaFetchError(err: unknown, timeoutMs: number): Error { if ((err as Error)?.name === 'TimeoutError') { - return `Ollama timed out after ${ollamaTimeoutMs()}ms at ${ollamaHost()} (the daemon may be wedged or a model is still loading). Raise OLLAMA_TIMEOUT_MS, or restart it: ollama serve`; + return new OllamaTimeoutError(timeoutMs); } // Connection refused / DNS / network: the daemon almost certainly isn't running. - return `Ollama daemon unreachable at ${ollamaHost()} (${(err as Error).message}). Start it with: ollama serve`; + return new Error(`Ollama daemon unreachable at ${ollamaHost()} (${(err as Error).message}). Start it with: ollama serve`); } diff --git a/src/core/orchestrator/configStore.test.ts b/src/core/orchestrator/configStore.test.ts index 8147cc1..0d66ff7 100644 --- a/src/core/orchestrator/configStore.test.ts +++ b/src/core/orchestrator/configStore.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { mkdir, mkdtemp, rm, readFile, writeFile, readdir } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -7,6 +7,7 @@ import { hydrateWorkdirConfigFromStore, loadRoroConfig, parseConfigFile, + persistWorkdirChoice, saveRoroConfig, } from './configStore'; import { setPersistedWorkdir, tryResolveWorkdir } from './workdir'; @@ -40,6 +41,7 @@ describe('configStore', () => { }); afterEach(async () => { + vi.restoreAllMocks(); setPersistedWorkdir(undefined); await rm(dir, { recursive: true, force: true }); }); @@ -80,25 +82,82 @@ describe('configStore', () => { expect(tryResolveWorkdir(env, '/cwd')).toBe(repo); }); - it('does not clobber an explicit RORO_WORKDIR env var', async () => { - await saveRoroConfig(dir, { workdir: '/persisted/repo' }); + // DELIBERATE precedence reversal (first dogfooding session): a stale shell RORO_WORKDIR silently + // retargeted the executor away from the ~/Code/workforroro the user chose in-app. The persisted + // choice now wins; the env var only applies when no project has been chosen. + it('the persisted project choice wins over an explicit RORO_WORKDIR env var', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const repo = join(dir, 'repo'); + await mkdir(repo); + await saveRoroConfig(dir, { workdir: repo }); + const env: NodeJS.ProcessEnv = { RORO_WORKDIR: '/env/repo' }; + + const result = await hydrateWorkdirConfigFromStore(dir, env); + + expect(result).toEqual({ workdir: repo, source: 'config' }); + expect(env.RORO_WORKDIR).toBe('/env/repo'); // never clobbered — just ignored + expect(tryResolveWorkdir(env, '/cwd')).toBe(repo); + // The ignored override must be VISIBLE — this log line IS the never-silently-retarget guarantee, + // so it is pinned here: deleting it would otherwise pass every test. + const wins = log.mock.calls.filter((c) => /persisted user choice wins.*ignoring RORO_WORKDIR/.test(String(c[0]))); + expect(wins).toHaveLength(1); + }); + + it('applies RORO_WORKDIR when no project has been persisted (dev/first-run flow)', async () => { const env: NodeJS.ProcessEnv = { RORO_WORKDIR: '/env/repo' }; const result = await hydrateWorkdirConfigFromStore(dir, env); expect(result).toEqual({ workdir: '/env/repo', source: 'env' }); - expect(env.RORO_WORKDIR).toBe('/env/repo'); expect(tryResolveWorkdir(env, '/cwd')).toBe('/env/repo'); }); it('honors an explicit RORO_WORKDIR even when config.json is corrupt', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + await writeFile(join(dir, 'config.json'), '{ corrupt', 'utf8'); + const env: NodeJS.ProcessEnv = { RORO_WORKDIR: '/env/repo' }; + + const result = await hydrateWorkdirConfigFromStore(dir, env); + + expect(result).toEqual({ workdir: '/env/repo', source: 'env' }); + expect(tryResolveWorkdir(env, '/cwd')).toBe('/env/repo'); + }); + + // A corrupt config.json must NOT brick the app when RORO_WORKDIR is unset (the packaged reality): + // main.ts awaits hydrateWorkdirConfig before createWindow with no catch, so the old rethrow meant a + // truncated config.json = NO window and no recovery. Corrupt = no persisted choice, unconditionally. + it('recovers from a corrupt config.json with NO env: warns, quarantines the file, resolves unset', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); await writeFile(join(dir, 'config.json'), '{ corrupt', 'utf8'); + const env: NodeJS.ProcessEnv = {}; + + const result = await hydrateWorkdirConfigFromStore(dir, env); + + expect(result).toEqual({ source: 'unset' }); // the first-run picker recovers from here + expect(tryResolveWorkdir(env, '/cwd')).toBeUndefined(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('config.json')); + // The bad file is preserved as evidence (config.json.corrupt), never silently discarded — and it + // no longer shadows the next persistWorkdirChoice save. + expect(await readdir(dir)).toEqual(['config.json.corrupt']); + expect(await readFile(join(dir, 'config.json.corrupt'), 'utf8')).toBe('{ corrupt'); + }); + + // The stale-path fallback must SAY it was a stale path: the generic "no persisted project choice" + // message would misdiagnose a persisted choice whose directory has since been deleted. + it('names the stale persisted path when falling back to RORO_WORKDIR', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const gone = join(dir, 'deleted-repo'); + await saveRoroConfig(dir, { workdir: gone }); const env: NodeJS.ProcessEnv = { RORO_WORKDIR: '/env/repo' }; const result = await hydrateWorkdirConfigFromStore(dir, env); expect(result).toEqual({ workdir: '/env/repo', source: 'env' }); expect(tryResolveWorkdir(env, '/cwd')).toBe('/env/repo'); + const stale = log.mock.calls.filter((c) => /no longer exists.*falling back to RORO_WORKDIR/.test(String(c[0]))); + expect(stale).toHaveLength(1); + expect(String(stale[0][0])).toContain(gone); // the log names WHICH choice went stale + expect(log.mock.calls.some((c) => /no persisted project choice/.test(String(c[0])))).toBe(false); }); it('treats a blank RORO_WORKDIR env var as unset so packaged config can recover', async () => { @@ -114,6 +173,17 @@ describe('configStore', () => { expect(tryResolveWorkdir(env, '/cwd')).toBe(repo); }); + // Part of the same reversal: choosing a project in-app takes effect IMMEDIATELY, even when this + // launch was started with RORO_WORKDIR (previously the env kept winning until relaunch). + it('persistWorkdirChoice makes the new choice effective even with RORO_WORKDIR set', async () => { + const env: NodeJS.ProcessEnv = { RORO_WORKDIR: '/env/repo' }; + + const result = await persistWorkdirChoice(dir, '/chosen/repo', env); + + expect(result).toEqual({ workdir: '/chosen/repo', source: 'config' }); + expect(tryResolveWorkdir(env, '/cwd')).toBe('/chosen/repo'); + }); + it('treats a stale persisted workdir as unset so onboarding can recover', async () => { await saveRoroConfig(dir, { workdir: join(dir, 'deleted-repo') }); const env: NodeJS.ProcessEnv = {}; diff --git a/src/core/orchestrator/configStore.ts b/src/core/orchestrator/configStore.ts index 1e820a0..0aa678f 100644 --- a/src/core/orchestrator/configStore.ts +++ b/src/core/orchestrator/configStore.ts @@ -1,8 +1,9 @@ -// src/main/configStore.ts — persisted first-run choices owned by Electron MAIN. +// src/core/orchestrator/configStore.ts — persisted first-run choices owned by Electron MAIN. // // Packaged apps do not read the developer's .env, so user choices that must survive relaunch -// live under app.getPath('userData')/config.json. Environment variables still win when set: -// they are the explicit dev/operator override. +// live under app.getPath('userData')/config.json. The persisted in-app choice WINS over the +// RORO_WORKDIR env var (reversed deliberately after the first dogfooding session, where a stale +// shell env silently retargeted the executor); env still applies when no choice was persisted. import { mkdir, readFile, rename, stat, writeFile } from 'node:fs/promises'; import { dirname, join } from 'node:path'; import type { WorkdirConfigMsg } from '../../shared/ipc'; @@ -67,18 +68,49 @@ export async function hydrateWorkdirConfigFromStore( env: NodeJS.ProcessEnv, ): Promise { const envWorkdir = env.RORO_WORKDIR?.trim(); - if (envWorkdir) { - setPersistedWorkdir(undefined); - return { workdir: envWorkdir, source: 'env' }; + + let config: RoroConfig; + try { + config = await loadRoroConfig(dir); + } catch (err) { + // A corrupt config.json is NOT a persisted choice — unconditionally. Rethrowing would brick a + // packaged app: main.ts awaits this before createWindow with no catch, so a truncated config.json + // meant NO window and no recovery. Fail loud in the LOG (warn + quarantine the evidence), then + // fall through to env/'unset' so the first-run picker can recover. Every other error class + // (real I/O failures) still propagates. + if (!(err instanceof ConfigCorruptError)) throw err; + console.warn(`[config] ${err.message}; treating it as no persisted choice (recover via the project picker or RORO_WORKDIR)`); + await quarantineCorruptConfig(dir); + config = {}; } - const config = await loadRoroConfig(dir); - const persisted = config.workdir && await isExistingDirectory(config.workdir) ? config.workdir : undefined; + const chosen = config.workdir; + const persisted = chosen && await isExistingDirectory(chosen) ? chosen : undefined; setPersistedWorkdir(persisted); - if (!persisted) return { source: 'unset' }; + if (persisted) { + if (envWorkdir) { + // Make the ignored override VISIBLE: a stale shell RORO_WORKDIR must never silently retarget + // the executor (the first-dogfooding incident this reversal fixes). + console.log( + `[config] workdir = ${persisted} (persisted user choice wins; ignoring RORO_WORKDIR=${envWorkdir})`, + ); + } + return { workdir: persisted, source: 'config' }; + } + + if (envWorkdir) { + // Distinguish "never chose a project" from "chose one whose directory is gone" — the stale case + // must name the path so the fallback is diagnosable, not misreported as a missing choice. + if (chosen) { + console.log(`[config] workdir = ${envWorkdir} (persisted choice ${chosen} no longer exists; falling back to RORO_WORKDIR)`); + } else { + console.log(`[config] workdir = ${envWorkdir} (RORO_WORKDIR env; no persisted project choice)`); + } + return { workdir: envWorkdir, source: 'env' }; + } - return { workdir: persisted, source: 'config' }; + return { source: 'unset' }; } export async function persistWorkdirChoice( @@ -92,7 +124,11 @@ export async function persistWorkdirChoice( setPersistedWorkdir(chosen); const envWorkdir = env.RORO_WORKDIR?.trim(); - if (envWorkdir) return { workdir: envWorkdir, source: 'env' }; + if (envWorkdir) { + console.log( + `[config] workdir = ${chosen} (persisted user choice wins; ignoring RORO_WORKDIR=${envWorkdir})`, + ); + } return { workdir: chosen, source: 'config' }; } @@ -104,6 +140,19 @@ export async function hydrateWorkdirConfig(dir: string, env: NodeJS.ProcessEnv = return result; } +/** Preserve a corrupt config.json as evidence (config.json.corrupt) instead of silently discarding it. + * Best-effort by design: a failed rename must never block the recovery path this call sits on — the + * whole point is that a bad file cannot brick startup. */ +async function quarantineCorruptConfig(dir: string): Promise { + const path = join(dir, 'config.json'); + try { + await rename(path, `${path}.corrupt`); + console.warn(`[config] preserved the unreadable file as ${path}.corrupt`); + } catch (err) { + console.warn(`[config] could not preserve the unreadable config.json: ${(err as Error).message}`); + } +} + async function atomicWriteJson(path: string, value: unknown): Promise { await mkdir(dirname(path), { recursive: true }); const tmp = `${path}.tmp`; diff --git a/src/core/orchestrator/workdir.test.ts b/src/core/orchestrator/workdir.test.ts index d5d1e1f..ddff6f9 100644 --- a/src/core/orchestrator/workdir.test.ts +++ b/src/core/orchestrator/workdir.test.ts @@ -40,9 +40,12 @@ describe('resolveWorkdir — fail-loud repo selection', () => { expect(resolveWorkdir({ RORO_WORKDIR: '/repo', RORO_ALLOW_CWD: '1' }, '/cwd')).toBe('/repo'); }); - it('a chosen RORO_WORKDIR takes precedence over persisted config', () => { + // DELIBERATE precedence reversal (first dogfooding session): a stale shell RORO_WORKDIR silently + // retargeted the executor away from the project the user chose IN-APP. The persisted user choice + // now wins; env applies only when no project has been chosen (dev/first-run flows unchanged). + it('the persisted user choice takes precedence over RORO_WORKDIR', () => { setPersistedWorkdir('/persisted/repo'); - expect(resolveWorkdir({ RORO_WORKDIR: '/env/repo' }, '/cwd')).toBe('/env/repo'); + expect(resolveWorkdir({ RORO_WORKDIR: '/env/repo' }, '/cwd')).toBe('/persisted/repo'); }); }); @@ -75,4 +78,11 @@ describe('tryResolveWorkdir — best-effort repo for memory scoping (never throw setPersistedWorkdir('/persisted/repo'); expect(tryResolveWorkdir({}, '/cwd')).toBe('/persisted/repo'); }); + + // Same deliberate reversal as resolveWorkdir (the dogfooding incident): memory scoping must follow + // the repo the executor actually edits, so the persisted choice wins here too. + it('the persisted user choice takes precedence over RORO_WORKDIR', () => { + setPersistedWorkdir('/persisted/repo'); + expect(tryResolveWorkdir({ RORO_WORKDIR: '/env/repo' }, '/cwd')).toBe('/persisted/repo'); + }); }); diff --git a/src/core/orchestrator/workdir.ts b/src/core/orchestrator/workdir.ts index 8ab52d3..b7a62fd 100644 --- a/src/core/orchestrator/workdir.ts +++ b/src/core/orchestrator/workdir.ts @@ -14,7 +14,9 @@ let persistedWorkdir: string | undefined; /** * MAIN boot hook: the packaged-app workdir chosen during onboarding. Kept behind the resolver so every - * edit path shares one precedence order: explicit env > persisted user choice > explicit dev cwd opt-in. + * edit path shares one precedence order: persisted user choice > explicit env > explicit dev cwd opt-in. + * (Reversed deliberately after the first dogfooding session: a stale shell RORO_WORKDIR silently + * retargeted the executor away from the project the user chose in-app.) */ export function setPersistedWorkdir(workdir: string | undefined): void { const chosen = workdir?.trim(); @@ -22,21 +24,22 @@ export function setPersistedWorkdir(workdir: string | undefined): void { } /** - * Best-effort repo, or undefined — NEVER throws. The chosen RORO_WORKDIR (trimmed), else the persisted - * packaged-app config, else `cwd` under the explicit RORO_ALLOW_CWD=1 opt-in, else undefined. + * Best-effort repo, or undefined — NEVER throws. The persisted packaged-app config (the in-app user + * choice), else the chosen RORO_WORKDIR (trimmed), else `cwd` under the explicit RORO_ALLOW_CWD=1 + * opt-in, else undefined. */ export function tryResolveWorkdir(env: WorkdirEnv, cwd: string): string | undefined { + if (persistedWorkdir) return persistedWorkdir; const chosen = env.RORO_WORKDIR?.trim(); if (chosen) return chosen; - if (persistedWorkdir) return persistedWorkdir; if (env.RORO_ALLOW_CWD === '1') return cwd; return undefined; } /** - * Resolve the repo Roro's coding agent EDITS. RORO_WORKDIR is the chosen project. With none set we throw - * rather than silently use `cwd`; persisted config is the packaged-app choice; RORO_ALLOW_CWD=1 is the - * explicit local-dev opt-in to use the current dir. + * Resolve the repo Roro's coding agent EDITS. The persisted config (the in-app user choice) wins; + * RORO_WORKDIR applies only when no project has been chosen; RORO_ALLOW_CWD=1 is the explicit + * local-dev opt-in to use the current dir. With none set we throw rather than silently use `cwd`. * (For MEMORY SCOPING — recall/remember context, where a missing repo is fine — use tryResolveWorkdir.) */ export function resolveWorkdir(env: WorkdirEnv, cwd: string): string { diff --git a/src/main/ipc.config.test.ts b/src/main/ipc.config.test.ts index 9d238cd..17be53c 100644 --- a/src/main/ipc.config.test.ts +++ b/src/main/ipc.config.test.ts @@ -105,7 +105,21 @@ describe('config IPC — packaged-app workdir onboarding spine', () => { expect(tryResolveWorkdir({}, '/cwd')).toBe(repo); }); - it('config:get reports explicit RORO_WORKDIR without clobbering persisted config', async () => { + // DELIBERATE precedence reversal (first dogfooding session): a stale shell RORO_WORKDIR silently + // retargeted the executor away from the in-app project choice. The persisted config now wins. + it('config:get prefers the persisted project over an explicit RORO_WORKDIR', async () => { + const repo = join(dir, 'repo'); + await mkdir(repo); + await writeFile(join(dir, 'config.json'), JSON.stringify({ workdir: repo }), 'utf8'); + process.env.RORO_WORKDIR = '/env/repo'; + + const result = await handler<() => Promise>(CH.configGet)(); + + expect(result).toEqual({ workdir: repo, source: 'config' }); + expect(tryResolveWorkdir(process.env, '/cwd')).toBe(repo); + }); + + it('config:get falls back to RORO_WORKDIR when config.json is corrupt (no persisted choice)', async () => { await writeFile(join(dir, 'config.json'), '{ corrupt', 'utf8'); process.env.RORO_WORKDIR = '/env/repo'; diff --git a/src/renderer/ask/floatingAsk.test.ts b/src/renderer/ask/floatingAsk.test.ts index f84dcad..027bd39 100644 --- a/src/renderer/ask/floatingAsk.test.ts +++ b/src/renderer/ask/floatingAsk.test.ts @@ -30,7 +30,7 @@ function setup(over: { smokeLifecycle?: boolean; } = {}) { document.body.innerHTML = '
'; - const driver = { poke: vi.fn(), setState: vi.fn() }; + const driver = { poke: vi.fn(), setState: vi.fn(), setBusy: vi.fn() }; let actionCb: ((e: ActionEvent) => void) | null = null; let runEndCb: ((p: { runId: string }) => void) | null = null; let focusAskCb: (() => void) | null = null; @@ -222,6 +222,62 @@ describe('floatingAsk shell (jsdom)', () => { expect(h.stop.classList.contains('armed')).toBe(false); }); + // Dogfooding fix: only the dev typedPrompt path drove setBusy, so in FLOATING mode nothing marked + // the cat busy — the 120s presence timer put it to SLEEP (and framePolicy throttled to 6fps) in the + // middle of a long codex run. The floating run lifecycle must drive the same busy flag that keeps + // the avatar awake (actionForTick skips 'sleeping' while busy; framePolicy runs full-rate). + it('run.started marks the cat busy so it cannot fall asleep mid-run; runEnd clears it', async () => { + h.summon(); + h.input.value = 'do it'; + submit(h.form); + await flush(); + expect(h.driver.setBusy).not.toHaveBeenCalled(); // busy tracks the RUN, not the submit + h.fireAction(started); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(true); + h.fireRunEnd(); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(false); + }); + + it('run.completed also clears busy (mirrors typedPrompt), ahead of the runEnd reset', async () => { + h.summon(); + h.input.value = 'do it'; + submit(h.form); + await flush(); + h.fireAction(started); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(true); + h.fireAction({ kind: 'run.completed', runId: 'r1', ok: true, ts: 2 }); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(false); + }); + + it('a failed run clears busy so the cat does not stay wired after run.failed', async () => { + h.summon(); + h.input.value = 'do it'; + submit(h.form); + await flush(); + h.fireAction(started); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(true); + h.fireAction({ kind: 'run.failed', runId: 'r1', ok: false, error: 'boom', ts: 2 }); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(false); + }); + + // The turnRun-REJECT exit: run.started already set busy, then the IPC promise rejects. The ask + // collapses, so eventBelongsToFloatingTurn filters every later terminal event — the centralized + // collapse exit itself must clear busy, or the cat stays wired forever. + it('a rejected turnRun after run.started still clears busy (the collapse turn-exit)', async () => { + let rejectTurn!: (err: Error) => void; + h.turnRun.mockReturnValueOnce(new Promise((_resolve, reject) => { rejectTurn = reject; })); + h.summon(); + h.input.value = 'do it'; + submit(h.form); + await flush(); + h.fireAction(started); // the run began before the bridge died + expect(h.driver.setBusy).toHaveBeenLastCalledWith(true); + rejectTurn(new Error('ipc down')); + await flush(); // flush the .catch → runEnded → collapse + expect(h.form.classList.contains('collapsed')).toBe(true); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(false); + }); + it('shows NO receipt after a successful answer turn (the cat conveys "done", not a banner)', async () => { h.summon(); h.input.value = 'what did we decide?'; diff --git a/src/renderer/ask/floatingAsk.ts b/src/renderer/ask/floatingAsk.ts index cfa7e56..5eb73be 100644 --- a/src/renderer/ask/floatingAsk.ts +++ b/src/renderer/ask/floatingAsk.ts @@ -411,6 +411,10 @@ export function mountFloatingAsk(opts: { verb.textContent = WORKING_FALLBACK; // fresh turn — clear any prior turn's verb break; case 'collapse': + // The centralized turn-exit clears busy too: on the turnRun-reject (and bridge-missing) exits + // the ask leaves 'tasked' with NO runEnd coming, so a run.started busy would otherwise outlive + // the turn forever (handleRunEnd's clear is redundant-but-harmless with this). + driver.setBusy?.(false); input.value = ''; taskedText = 'Ask Roro…'; stopAvailable = false; @@ -620,15 +624,21 @@ export function mountFloatingAsk(opts: { verb.textContent = summarizeEvent(e).label; if (e.kind === 'run.started') { acceptedRunId = e.runId; + // Busy keeps the cat AWAKE while the executor works (mirrors typedPrompt): without it the + // 120s presence timer put the floating cat to sleep mid-run and framePolicy throttled it + // to 6fps — the pet looked dead exactly while working (first dogfooding session). + driver.setBusy?.(true); clearFailure(); dispatch({ type: 'runStarted' }); } else { if (e.kind === 'run.completed') { + driver.setBusy?.(false); stopAvailable = false; stopRequested = false; acceptedRunId = null; } if (e.kind === 'run.failed') { + driver.setBusy?.(false); const stopped = stopRequested || isStoppedTerminalError(e.error); stopAvailable = false; stopRequested = false; @@ -646,6 +656,9 @@ export function mountFloatingAsk(opts: { if (receiptRunId === null) receiptRunId = runId; const receipt = receiptForTurnEnd(receiptState, receiptCancelRequested); run = INITIAL_RUN_LIFECYCLE; + // The universal turn-end also clears busy: a watchdog-ended run can reach runEnd without its + // run.completed/run.failed passing the turn filter, and busy must never outlive the turn. + driver.setBusy?.(false); stopAvailable = false; stopRequested = false; acceptedRunId = null; diff --git a/src/renderer/bootstrap/typedPrompt.test.ts b/src/renderer/bootstrap/typedPrompt.test.ts new file mode 100644 index 0000000..4c70414 --- /dev/null +++ b/src/renderer/bootstrap/typedPrompt.test.ts @@ -0,0 +1,67 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mountTypedPrompt } from './typedPrompt'; +import type { ActionEvent } from '../../shared/events'; + +function setCompanion(stub: unknown): void { + (window as unknown as { companion: unknown }).companion = stub; +} + +function setup() { + document.body.innerHTML = [ + '
', + ' ', + ' ', + ' ', + '
', + ].join(''); + const driver = { setBusy: vi.fn(), setState: vi.fn() }; + const setStatus = vi.fn(); + let actionCb: ((e: ActionEvent) => void) | null = null; + let runEndCb: ((p: { runId: string }) => void) | null = null; + const unsub = (): void => undefined; + const turnRun = vi.fn().mockResolvedValue({ runId: 'r1' }); + setCompanion({ + turnRun, + getWorkdirConfig: vi.fn(async () => ({ workdir: '/repo', source: 'config' as const })), + onActionEvent: (cb: (e: ActionEvent) => void): (() => void) => { actionCb = cb; return unsub; }, + onRunEnd: (cb: (p: { runId: string }) => void): (() => void) => { runEndCb = cb; return unsub; }, + }); + const unmount = mountTypedPrompt({ + captions: { update: vi.fn() }, + driver, + brainGate: { ensureReady: () => true }, + sessionId: 'sess', + setStatus, + }); + return { + driver, turnRun, setStatus, unmount, + form: document.getElementById('prompt-form') as HTMLFormElement, + input: document.getElementById('prompt-input') as HTMLInputElement, + fireAction: (e: ActionEvent) => actionCb?.(e), + fireRunEnd: (runId = 'r1') => runEndCb?.({ runId }), + }; +} + +const submit = (form: HTMLElement): boolean => + form.dispatchEvent(new Event('submit', { cancelable: true, bubbles: true })); +const flush = (): Promise => new Promise((r) => setTimeout(r)); +const started: ActionEvent = { kind: 'run.started', runId: 'r1', agent: 'codex', ts: 1 }; + +describe('typedPrompt (dev prompt) busy lifecycle', () => { + let h: ReturnType; + beforeEach(() => { h = setup(); }); + afterEach(() => { h.unmount(); setCompanion(undefined); }); + + // Mirrors floatingAsk's universal busy clear: a watchdog-ended run can reach runEnd without its + // run.completed/run.failed passing the turn filter, and busy must never outlive the turn. + it('runEnd clears busy even when no terminal run event passed the turn filter', async () => { + h.input.value = 'do it'; + submit(h.form); + await flush(); + h.fireAction(started); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(true); + h.fireRunEnd('r1'); + expect(h.driver.setBusy).toHaveBeenLastCalledWith(false); + }); +}); diff --git a/src/renderer/bootstrap/typedPrompt.ts b/src/renderer/bootstrap/typedPrompt.ts index 267c739..69c6604 100644 --- a/src/renderer/bootstrap/typedPrompt.ts +++ b/src/renderer/bootstrap/typedPrompt.ts @@ -70,6 +70,9 @@ export function mountTypedPrompt(deps: TypedPromptDeps): () => void { } function releaseTypedTurn(): void { + // Universal turn-exit clears busy (mirrors floatingAsk): a watchdog-ended run can reach runEnd + // without its run.completed/run.failed passing the turn filter, and busy must never outlive the turn. + driver.setBusy?.(false); activeTurnSerial = 0; acceptedRunId = null; turnInFlight = false; diff --git a/src/renderer/character/avatar.ts b/src/renderer/character/avatar.ts index 9417078..d1533a6 100644 --- a/src/renderer/character/avatar.ts +++ b/src/renderer/character/avatar.ts @@ -15,7 +15,7 @@ import { catRectInWindow, type Rect } from '../../shared/floatingLayout'; import type { ActivityCue } from './types'; import { Gaze } from './gaze'; import { Activity, type Energy } from './activity'; -import { framePolicy } from './framePolicy'; +import { framePolicy, isSleepingPose } from './framePolicy'; import { catHeadOriginForAction, type CatAction } from './catGeometry'; import { derivePetState } from './petState'; import { petExpression } from './petExpression'; @@ -189,7 +189,7 @@ function buildCat(app: PIXI.Application): Cat { text.length > 28 ? `${text.slice(0, 25)}...` : text; const actionForTick = (tick = 0): CatAction => { - if (energy === 'asleep' && !busy && !inCall) return 'sleeping'; + if (isSleepingPose(energy, busy, inCall)) return 'sleeping'; if (state === 'working') return 'walking'; if (state === 'thinking') return 'sitting'; if (state === 'idle' && isFloatingWindow()) { diff --git a/src/renderer/character/framePolicy.test.ts b/src/renderer/character/framePolicy.test.ts index 3f27edd..1fe9f29 100644 --- a/src/renderer/character/framePolicy.test.ts +++ b/src/renderer/character/framePolicy.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { framePolicy } from './framePolicy'; +import { framePolicy, isSleepingPose } from './framePolicy'; describe('framePolicy', () => { it('stops the loop entirely when occluded', () => { @@ -25,3 +25,24 @@ describe('framePolicy', () => { expect(framePolicy(false, 'awake', false, true)).toEqual({ state: 'occluded', running: false, targetFps: 0 }); }); }); + +// The avatar's actual pose gate (actionForTick's first branch) — pinned here so "busy cat never +// sleeps" holds at the layer that DRAWS the sleeping pose, not only at the callers + framePolicy. +describe('isSleepingPose (the avatar pose gate)', () => { + it('sleeps only at asleep energy with nothing keeping the cat up', () => { + expect(isSleepingPose('asleep', false, false)).toBe(true); + }); + + it('a busy cat never sleeps, even at asleep energy', () => { + expect(isSleepingPose('asleep', true, false)).toBe(false); + }); + + it('an in-call cat never sleeps, even at asleep energy', () => { + expect(isSleepingPose('asleep', false, true)).toBe(false); + }); + + it('awake/drowsy energy never yields the sleeping pose', () => { + expect(isSleepingPose('awake', false, false)).toBe(false); + expect(isSleepingPose('drowsy', false, false)).toBe(false); + }); +}); diff --git a/src/renderer/character/framePolicy.ts b/src/renderer/character/framePolicy.ts index 6266dcb..0dbf8e1 100644 --- a/src/renderer/character/framePolicy.ts +++ b/src/renderer/character/framePolicy.ts @@ -26,3 +26,12 @@ export function framePolicy(visible: boolean, energy: Energy, busy: boolean, inC if (energy === 'drowsy') return { state: 'idle', running: true, targetFps: 12 }; return { state: 'active', running: true, targetFps: 60 }; } + +/** + * PURE pose gate for the avatar's sleeping pose (actionForTick's first branch): the cat draws + * 'sleeping' only at asleep ENERGY with nothing keeping it up — a busy cat (agent run in flight) + * or an in-call cat never sleeps, the same inputs that keep framePolicy at full rate. + */ +export function isSleepingPose(energy: Energy, busy: boolean, inCall: boolean): boolean { + return energy === 'asleep' && !busy && !inCall; +} diff --git a/src/renderer/settings/projectSettings.test.ts b/src/renderer/settings/projectSettings.test.ts index 9e18519..50fe194 100644 --- a/src/renderer/settings/projectSettings.test.ts +++ b/src/renderer/settings/projectSettings.test.ts @@ -101,9 +101,104 @@ describe('mountProjectSettings - change the configured working repo', () => { expect(t.onStatus).toHaveBeenCalledWith('Project unchanged.'); }); - it('shows an env override and explains that it cannot change the active repo', async () => { - const chooseWorkdir = vi.fn(); - const t = setup({ + it('canceling the picker on an ENV-sourced launch is NOT a project change (cancel returns the effective env config)', async () => { + // config:chooseWorkdir returns the CURRENT effective config on cancel — with RORO_WORKDIR set that is + // {workdir, source:'env'}, which must not broadcast WORKDIR_CONFIGURED_EVENT or claim "Project changed". + const envConfig: WorkdirConfigMsg = { workdir: '/from/env', source: 'env' }; + const chooseWorkdir = vi.fn(async (): Promise => envConfig); + const t = setup({ getConfig: vi.fn(async () => envConfig), chooseWorkdir }); + const listener = vi.fn(); + window.addEventListener(WORKDIR_CONFIGURED_EVENT, listener); + + click(q('#project-settings-toggle')); + await flush(); + click(q('#project-settings-change')); + await flush(); + + expect(listener).not.toHaveBeenCalled(); + expect(t.onStatus).toHaveBeenCalledWith('Project unchanged.'); + }); + + it('cancel is NOT a change even when Change is clicked before the initial config resolves (race)', async () => { + // knownWorkdir is only learned from an async getConfig; if Change is clicked first, `before` must be + // established in-handler (await getConfig) — else a cancel returning the saved config broadcasts a fake change. + const saved: WorkdirConfigMsg = { workdir: '/repo/one', source: 'config' }; + let releaseConfig: (c: WorkdirConfigMsg) => void = () => {}; + const getConfig = vi.fn(() => new Promise((res) => { releaseConfig = res; })); + const chooseWorkdir = vi.fn(async (): Promise => saved); // cancel → same saved config + const t = setup({ getConfig, chooseWorkdir }); + const listener = vi.fn(); + window.addEventListener(WORKDIR_CONFIGURED_EVENT, listener); + + click(q('#project-settings-change')); // BEFORE any getConfig resolution + releaseConfig(saved); + await flush(); + await flush(); + + expect(listener).not.toHaveBeenCalled(); + expect(t.onStatus).toHaveBeenCalledWith('Project unchanged.'); + }); + + it('a stale initial getConfig resolving AFTER a successful pick does not overwrite the new project (race)', async () => { + // refresh() must be sequenced: if the slow initial getConfig resolves after the user already picked a + // new project, applying the stale config would show the wrong repo and corrupt the next cancel check. + let releaseStale: (c: WorkdirConfigMsg) => void = () => {}; + const getConfig = vi.fn(() => new Promise((res) => { releaseStale = res; })); + const chooseWorkdir = vi.fn(async (): Promise => ({ workdir: '/repo/two', source: 'config' })); + setup({ getConfig, chooseWorkdir }); + const stale = releaseStale; // capture the INITIAL refresh's resolver before any later getConfig call swaps it + + click(q('#project-settings-change')); // in-handler getConfig fires (second call) — resolve it as unset + releaseStale({ source: 'unset' }); + await flush(); + await flush(); + expect(q('#project-settings-toggle')?.textContent).toBe('Project: two'); // the pick applied + + stale({ workdir: '/repo/one', source: 'config' }); // the STALE initial refresh finally resolves + await flush(); + expect(q('#project-settings-toggle')?.textContent).toBe('Project: two'); // and must NOT overwrite + }); + + it('a stale refresh cannot repaint over a picker FAILURE (the fail-loud state is authoritative)', async () => { + let releaseStale: (c: WorkdirConfigMsg) => void = () => {}; + const getConfig = vi.fn(() => new Promise((res) => { releaseStale = res; })); + const chooseWorkdir = vi.fn(async (): Promise => { throw new Error('picker exploded'); }); + setup({ getConfig, chooseWorkdir }); + const stale = releaseStale; // the INITIAL refresh's resolver + + click(q('#project-settings-change')); // in-handler getConfig fires — resolve it as unset + releaseStale({ source: 'unset' }); + await flush(); + await flush(); + expect(q('#project-settings-source')?.textContent).toBe('Settings unavailable'); // fail-loud shown + + stale({ workdir: '/repo/one', source: 'config' }); // the stale initial refresh resolves late + await flush(); + expect(q('#project-settings-source')?.textContent).toBe('Settings unavailable'); // error NOT repainted + }); + + it('canceling the picker with an existing saved project is NOT a project change (the same workdir comes back)', async () => { + const saved: WorkdirConfigMsg = { workdir: '/repo/one', source: 'config' }; + const chooseWorkdir = vi.fn(async (): Promise => saved); // cancel → same effective config + const t = setup({ getConfig: vi.fn(async () => saved), chooseWorkdir }); + const listener = vi.fn(); + window.addEventListener(WORKDIR_CONFIGURED_EVENT, listener); + + click(q('#project-settings-toggle')); + await flush(); + click(q('#project-settings-change')); + await flush(); + + expect(listener).not.toHaveBeenCalled(); + expect(t.onStatus).toHaveBeenCalledWith('Project unchanged.'); + }); + + // DELIBERATE flip (first dogfooding session): the persisted in-app choice now wins over + // RORO_WORKDIR, so choosing a project is meaningful even on an env-sourced launch — the picker + // must open instead of telling the user to unset the env var. + it('shows an env-sourced workdir but still lets the user choose a project (which then wins)', async () => { + const chooseWorkdir = vi.fn(async (): Promise => ({ workdir: '/chosen/repo', source: 'config' })); + setup({ getConfig: vi.fn(async (): Promise => ({ workdir: '/env/repo', source: 'env' })), chooseWorkdir, }); @@ -113,10 +208,12 @@ describe('mountProjectSettings - change the configured working repo', () => { expect(q('#project-settings-source')?.textContent).toBe('RORO_WORKDIR'); expect(q('#project-settings-toggle')?.textContent).toBe('Project: repo (env)'); - expect(q('#project-settings-change')?.getAttribute('aria-disabled')).toBe('true'); + expect(q('#project-settings-change')?.getAttribute('aria-disabled')).toBe('false'); click(q('#project-settings-change')); - expect(chooseWorkdir).not.toHaveBeenCalled(); - expect(t.onStatus).toHaveBeenCalledWith('This launch uses RORO_WORKDIR. Unset it to use a saved project.'); + await flush(); + expect(chooseWorkdir).toHaveBeenCalledOnce(); + expect(q('#project-settings-source')?.textContent).toBe('Saved project'); + expect(q('#project-settings-current')?.textContent).toBe('/chosen/repo'); }); it('does not open the picker while a run is active', async () => { diff --git a/src/renderer/settings/projectSettings.ts b/src/renderer/settings/projectSettings.ts index cd03125..4c5710b 100644 --- a/src/renderer/settings/projectSettings.ts +++ b/src/renderer/settings/projectSettings.ts @@ -85,14 +85,19 @@ export function mountProjectSettings(deps: ProjectSettingsDeps): () => void { panel.append(heading, row, change); host.append(toggle, panel); - let lastConfig: WorkdirConfigMsg = { source: 'unset' }; + // The last workdir this panel knows about — lets the picker handler tell a REAL change apart from a + // cancel (config:chooseWorkdir returns the current effective config on cancel, which on an env launch + // is {workdir, source:'env'} and with a saved project is the same {workdir, source:'config'} back). + let knownWorkdir: string | undefined; function apply(config: WorkdirConfigMsg): void { - lastConfig = config; + knownWorkdir = config.workdir; current.textContent = config.workdir ?? 'No project selected'; source.textContent = sourceLabel(config.source); change.textContent = config.workdir ? 'Change Project' : 'Choose Project'; - change.setAttribute('aria-disabled', String(config.source === 'env')); + // Choosing a project is always meaningful: the persisted in-app choice wins over RORO_WORKDIR + // (reversed after the first dogfooding session), so an env-sourced launch no longer blocks it. + change.setAttribute('aria-disabled', 'false'); change.disabled = false; toggle.textContent = toggleLabel(config); toggle.setAttribute('aria-label', toggleAriaLabel(config)); @@ -107,15 +112,22 @@ export function mountProjectSettings(deps: ProjectSettingsDeps): () => void { toggle.setAttribute('aria-label', message); } + // Sequencing guard (codex S-B review): only the LATEST config source may paint the panel. A slow, stale + // refresh() resolving AFTER a successful pick would otherwise overwrite the newly chosen project (and + // knownWorkdir, corrupting the next cancel-vs-change check). Authoritative applies bump the epoch; + // in-flight refreshes check they are still current before painting. + let epoch = 0; + function refresh(): void { + const mine = ++epoch; void deps.getConfig() - .then(apply) - .catch((e) => showError(`Project settings failed: ${describeError(e)}`)); + .then((config) => { if (mine === epoch) apply(config); }) + .catch((e) => { if (mine === epoch) showError(`Project settings failed: ${describeError(e)}`); }); } const configuredListener = (event: Event): void => { const config = (event as CustomEvent).detail; - if (config) apply(config); + if (config) { epoch++; apply(config); } // authoritative: invalidate any in-flight refresh }; window.addEventListener(WORKDIR_CONFIGURED_EVENT, configuredListener); refresh(); @@ -131,16 +143,24 @@ export function mountProjectSettings(deps: ProjectSettingsDeps): () => void { deps.onStatus?.('Wait for the current run to finish before changing projects.'); return; } - if (lastConfig.source === 'env') { - deps.onStatus?.('This launch uses RORO_WORKDIR. Unset it to use a saved project.'); - return; - } change.disabled = true; current.textContent = 'Opening project picker...'; - void deps.chooseWorkdir() - .then((config) => { + // Pre-picker workdir (apply() below overwrites knownWorkdir). If Change is clicked before the initial + // async refresh ever resolved, knownWorkdir is still undefined — establish it from getConfig FIRST, or + // a cancel returning the existing saved config would read as a real change (best-effort: a getConfig + // failure falls through with undefined rather than blocking the picker). + void (knownWorkdir !== undefined + ? Promise.resolve(knownWorkdir) + : deps.getConfig().then((c) => c.workdir).catch(() => undefined) + ) + .then((before) => deps.chooseWorkdir().then((config) => ({ before, config }))) + .then(({ before, config }) => { + epoch++; // authoritative: the picker's result outranks any still-in-flight refresh apply(config); - if (!config.workdir) { + // A REAL change = a persisted user choice ('config') for a DIFFERENT workdir. Cancel returns the + // current effective config — env-sourced or the same saved project — and must not broadcast a + // fake WORKDIR_CONFIGURED_EVENT or claim "Project changed" (codex S-B review finding). + if (!config.workdir || config.source !== 'config' || config.workdir === before) { deps.onStatus?.('Project unchanged.'); return; } @@ -148,6 +168,7 @@ export function mountProjectSettings(deps: ProjectSettingsDeps): () => void { deps.onStatus?.(`Project changed to ${basenameFromPath(config.workdir)}. New tasks will use it.`); }) .catch((e) => { + epoch++; // the fail-loud error state is authoritative too — a late refresh must not repaint over it change.disabled = false; showError(`Project change failed: ${describeError(e)}`); });