From cca85e3582672de8847d4043f2459e13603d7dc0 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:49:39 +0530 Subject: [PATCH 01/16] docs(readme): rewrite against what actually ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README pitched FreeCode as a tool that "drives AI coding assistants via browser automation" with a two-phase "ask which files, then send them" flow. That is the legacy Playwright path CLAUDE.md marks as not wired into the primary path. The product is a ~198-provider API agent loop. Rewritten against source: the real installer, all 16 tools, the command list, the four-frontend architecture, and the generic provider driver. The browser path stays as a blockquote noting it exists and is not the default. Three claims were corrected while writing: `freecode session` is list + delete only, `freecode memory` is graph stats/rebuild/UI, and `freecode update` is injected by the TUI (apps/tui/src/entry.ts:194), not registered in create-cli.ts. Carries the subscription-auth warning callout too — it sits inside the rewritten body, and the docs page it links to lands later in this branch. beforeStable.md P0 #1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- README.md | 220 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 143 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 84aba3dc..4d0bf853 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ FreeCode Logo -**Open source CLI tool that drives AI coding assistants via browser automation** +**An open-source CLI coding agent that runs on whichever model you already pay for** [![License](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) @@ -11,109 +11,175 @@ -**FreeCode** is a thin-client CLI that drives AI coding assistants (ChatGPT, Claude, Gemini) via browser automation to assist with coding tasks. The architecture uses a two-phase approach: the AI first returns which files it needs, then receives those files with the prompt and returns structured file changes. +**FreeCode** is a terminal coding agent. You give it a task in plain language; it +reads your repository, runs commands, edits files, and reports back — driving the +work itself through a single agentic tool-use loop rather than asking you which +files it needs. -## Features +It talks to roughly **198 providers** derived from [models.dev](https://models.dev) +— Anthropic, OpenAI, Gemini, DeepSeek, Groq, Mistral, xAI, MiniMax, Z.ai, and +every OpenAI-compatible endpoint behind them — through one generic driver. You +can also sign in with an **Anthropic Pro/Max subscription** instead of an API +key — opt-in, and [read the stance first](#install). -- **TUI + VS Code Extension** — Choose your interface -- **JSON-RPC over stdin/stdout** — Lightweight IPC between frontends and CLI -- **Browser-based AI providers** — Direct integration with ChatGPT, Claude, Gemini -- **Two-phase context collection** — Efficient file retrieval before prompts -- **Diff preview before apply** — Review changes before writing -- **Persistent CLI daemon** — Reuses browser connection across turns +Everything intelligent lives in one CLI backend. The TUI, the VS Code extension, +the web UI, and the desktop app are presentation layers that speak JSON-RPC to it. -## Supported Tools +## Install -| Tool | Description | Parameters | -| ---------- | --------------------------------------- | --------------------------------------------------- | -| `read` | Read file or directory contents | `filePath`, `offset?`, `limit?` | -| `write` | Create or overwrite files | `filePath`, `content` | -| `edit` | Edit files in-place with smart matching | `filePath`, `oldString`, `newString`, `replaceAll?` | -| `glob` | Find files matching glob patterns | `pattern`, `path?` | -| `grep` | Search file contents via regex | `pattern`, `path?`, `include?`, `-n?`, `-i?`, `-C?` | -| `bash` | Execute shell commands | `command`, `timeout?`, `workdir?` | -| `skill` | Load specialized skills from SKILL.md | `name` | -| `question` | Ask user clarifying questions | `questions` (JSON array) | +```bash +curl -fsSL https://freecode.website/install | bash +``` -### Tool Execution Modes +On Windows, use PowerShell: -| Mode | Tools | Behavior | -| ----------------- | ---------------------- | ----------------------- | -| **Sequential** | `edit`, `write` | One at a time, in order | -| **Parallel-safe** | `read`, `glob`, `grep` | Batch concurrently | +```powershell +irm https://freecode.website/install.ps1 | iex +``` -## Skills +Then, in any project: -Skills are specialized instruction sets loaded from `SKILL.md` files. They provide structured workflows for specific tasks. +```bash +freecode +``` -**Skill locations:** +Pick a model with `/model` before your first prompt — a fresh install has no +provider selected, and FreeCode refuses to guess one rather than sending your +prompt somewhere you never chose. Full walkthrough: +[Quickstart](https://freecode.website/getting-started/quickstart). -- `~/.claude/skills/` — Global skills -- `~/.agents/skills/` — Agent skills -- `.claude/skills/` — Project skills -- `.freecode/skills/` — Project skills +Using an Anthropic subscription instead of an API key: -**Example skill structure:** +```bash +freecode auth login anthropic +``` -```markdown -# .freecode/skills/brainstorming/SKILL.md +> **Read what that does before you run it.** Subscription inference is only +> reachable by presenting FreeCode to Anthropic **as Claude Code** — its OAuth +> client id, its headers, its identity line. Anthropic reserves that inference +> for its own surfaces and has acted against tools doing this, and the account +> at risk is yours. It is opt-in, off by default, and one `freecode auth logout` +> away. Full stance: +> [Anthropic subscription login](https://freecode.website/getting-started/anthropic-subscription). + +## What it does + +- **One agentic loop.** The model receives your prompt, project context, and a + tool set, then drives the work — no separate "which files do you need" pass. + Independent tool calls run in parallel batches. +- **Real streaming, native tool calling, extended thinking, prompt caching**, and + usage accounting across every provider, through the Vercel AI SDK. +- **Agent modes** — `plan`, `build`, `review`, `explore`, `danger` — enforced by a + permission layer with per-rule allow/ask/deny and path-scoped rules in + `.freecode/settings.json`. +- **Persistent memory across sessions**, with a derived knowledge graph (local + ONNX embeddings, clustering, cascade retrieval) and an opt-in graph explorer. +- **Durable sessions** — resume, fork, export/import, and automatic compaction + when the context window fills. +- **Extensibility** — MCP servers, skills (`SKILL.md`), lifecycle hooks, and + `CLAUDE.md` / `AGENTS.md` instruction files. +- **Observability** — every model call is recorded as an event; `freecode trace` + renders the waterfall, and runs export as OTLP spans. +- **An eval harness** — behaviour changes are verified by scored agent runs, not + by eyeballing a transcript. + +## Tools + +| Tool | Description | +| ------------ | ---------------------------------------------------- | +| `read` | Read file contents | +| `ls` | List directory contents | +| `write` | Create or overwrite files | +| `edit` | Edit files in place with smart matching | +| `glob` | Find files matching glob patterns | +| `grep` | Search file contents via regex | +| `bash` | Execute shell commands | +| `agent` | Delegate to a subagent with its own capability profile | +| `skill` | Load a specialized skill from `SKILL.md` | +| `question` | Ask the user clarifying questions | +| `todowrite` | Track a multi-step plan | +| `webfetch` | Fetch a URL | +| `websearch` | Search the web | +| `lsp` | Query a language server | +| `memory` | Save or recall persistent memory | +| `output` | Retrieve stored tool output | + +MCP tools register dynamically at runtime through the same registry. + +## Commands ---- +``` +freecode open the TUI in the current project +freecode run one headless turn, streamed to stdout +freecode serve JSON-RPC backend over stdin/stdout +freecode web local web UI +freecode auth Anthropic subscription login / status / logout +freecode session list and delete sessions +freecode memory knowledge-graph stats, rebuild, and explorer UI +freecode mcp manage MCP servers +freecode trace render a session's model-call waterfall +freecode eval run the eval suites +freecode update re-run the installer +``` -name: brainstorming -description: Explore requirements before building features +## Architecture ---- +``` + ┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ + │ TUI │ │ VS Code │ │ Web │ │ Desktop │ + │ apps/tui │ │apps/vscode│ │ apps/web │ │apps/web-app│ + └────┬─────┘ └────┬─────┘ └────┬─────┘ └────┬─────┘ + └─────────────┴── JSON-RPC ─┴─────────────┘ + │ + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ CLI Backend (apps/core) — ALL intelligence │ +│ Agent loop · Tools · Context engine · Sessions · Providers │ +│ MCP client · Hooks · Skills · Memory · Compaction · Rollout │ +└──────────────────────────────┬───────────────────────────────┘ + │ Vercel AI SDK + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ ~198 providers from models.dev — Anthropic · OpenAI · │ +│ Gemini · DeepSeek · Groq · Mistral · xAI · MiniMax · Z.ai │ +└──────────────────────────────────────────────────────────────┘ +``` -# Brainstorming Skill +**Key principle:** frontends only render and speak IPC. No provider calls, no +file reading, no tool execution outside `apps/core`. -1. Clarify the goal - what problem are we solving? -2. Identify constraints - what must/must not happen? -3. Explore alternatives - what approaches exist? -4. Define success - how do we know it's done? -``` +> A legacy browser-automation path (Playwright, driving a signed-in ChatGPT or +> Gemini session) still exists under `apps/core/src/browser/` and `gemini-web`. +> It is not the default execution path. -## Quick Start +## Running from a clone ```bash -# Install dependencies -npm install - -# Start the TUI -cd apps/tui && npm run dev +pnpm install +pnpm --filter @thisisayande/freecode-shared build +pnpm dev # or: cd apps/tui && pnpm dev ``` -## Architecture +Checks: -``` -┌─────────────────────────────────────────────────────────────┐ -│ TUI │ -│ (apps/tui) — pure UI shell │ -│ Uses pi-tui for terminal rendering │ -│ IPC client sends/receives JSON-RPC │ -└──────────────────────────┬──────────────────────────────────┘ - │ - │ JSON-RPC (stdin/stdout) - ▼ -┌─────────────────────────────────────────────────────────────┐ -│ CLI Backend │ -│ (apps/core) — ALL intelligence │ -│ Browser controller, parser, tools, context engine, │ -│ agent loop, file applier │ -└──────────────────────────┬──────────────────────────────────┘ - │ - ▼ -┌─────────────────────────────────────────────────────────────┐ -│ AI Provider (Browser) │ -│ ChatGPT / Claude / Gemini │ -└─────────────────────────────────────────────────────────────┘ +```bash +pnpm check-types +pnpm test ``` ## Documentation -- [Architecture Overview](docs/superpowers/specs/2026-05-23-architecture.md) -- [Agent Loop Design](docs/superpowers/specs/2026-05-25-agent-loop.md) -- [Implementation Plan](docs/superpowers/plans/2026-05-10-freecode-mvp.md) +Full docs: **[freecode.website](https://freecode.website)** + +- [Installation](https://freecode.website/getting-started/installation) +- [Quickstart](https://freecode.website/getting-started/quickstart) +- [Configuration](https://freecode.website/getting-started/configuration) +- [Internals](https://freecode.website/internals/agent-loop) — agent loop, + providers, memory, compaction, permissions, eval + +In-repo references: [`CLAUDE.md`](CLAUDE.md) (contributor guide), +[`EVAL.md`](EVAL.md), [`TRACE.md`](TRACE.md), and the design specs under +[`docs/superpowers/specs/`](docs/superpowers/specs/). ## License From 4f5037dc64d204e5778d064534a5394c09df62c7 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:51:30 +0530 Subject: [PATCH 02/16] feat(cli): make headless `freecode run` actually usable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ways a headless run diverged from an interactive one: - **No hooks.** HookSettingsManager and registerRtkHook were constructed only in startServer(), so `freecode run` loaded no settings.json hooks — the formatter that fires after every edit interactively silently did not fire in CI, same repo, two behaviours. Both now live in hooks/bootstrap.ts (initHooks), called by startServer() and the run handler. Only `serve` passes watch: true; a one-shot run exits before a settings edit could apply and would otherwise hold an fs watcher open past its last turn. - **`build` mode denied every write.** askPermission rejects immediately with no frontend listening and promptForPermission maps that to deny, while build's default for mutating tools is ask. So `freecode run "fix the test"` read fine and was denied every edit. `--yes` (-y) answers the ask tier with allow; repeatable `--allow ` adds in-memory session grants. Both ride in on new AgentLoopConfig.autoApproveAsks / .sessionGrants. Scoped to the ask tier deliberately: a deny rule and a read-only mode still refuse, because those are decisions someone already made, not questions waiting for an answer. - **`--agent` was an unchecked cast**, so `--agent buld` fell through modeDefault's default branch and ran with build semantics. Added yargs .choices(), as `mcp add`'s type already has. Tests: agent/headless-permission.test.ts (5 — the first pins that the unattended default is still deny, two land a real write on disk via --yes and --allow, two assert --yes beats neither a deny rule nor plan mode) and hooks/bootstrap.test.ts (2). beforeStable.md P0 #2. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- TODO.md | 31 ++- .../src/agent/headless-permission.test.ts | 181 ++++++++++++++++++ apps/core/src/agent/loop.ts | 105 ++++++---- apps/core/src/cli/commands/run.ts | 49 ++++- apps/core/src/hooks/bootstrap.test.ts | 56 ++++++ apps/core/src/hooks/bootstrap.ts | 39 ++++ apps/core/src/server.ts | 14 +- apps/docs/app/reference/cli/page.mdx | 39 +++- apps/docs/app/reference/hook-events/page.mdx | 2 - apps/docs/app/reference/settings/page.mdx | 10 +- 10 files changed, 445 insertions(+), 81 deletions(-) create mode 100644 apps/core/src/agent/headless-permission.test.ts create mode 100644 apps/core/src/hooks/bootstrap.test.ts create mode 100644 apps/core/src/hooks/bootstrap.ts diff --git a/TODO.md b/TODO.md index c09b2993..2b5665c4 100644 --- a/TODO.md +++ b/TODO.md @@ -209,22 +209,21 @@ earlier audit are not repeated here. ### Real fixes -- [ ] **`freecode run` runs without hooks.** `HookSettingsManager` is constructed - only in `startServer()` (`server.ts:1106`), so a headless run loads no - `settings.json` hooks and never calls `registerRtkHook()`. Permission *rules* - do apply (the loop builds its own `PermissionSettingsManager`, - `loop.ts:571`), so the same repo behaves differently under `serve` and under - `run` — the formatter that fires after every edit interactively silently does - not fire in CI. Move both into a shared bootstrap the `run` handler also calls. -- [ ] **Headless `build` mode denies everything and says nothing useful.** - `askPermission` rejects immediately when no frontend is listening - (`bus/index.ts:413`) and `promptForPermission` maps that to deny, while - `build`'s mode default for mutating tools is `ask`. So `freecode run "fix the - test"` reads fine and is denied every write. Needs a `--yes`/`--allow ` - flag, or a one-time explanation on the first headless denial. -- [ ] **`freecode run --agent` is an unchecked cast** (`run.ts:140`). `--agent buld` - falls through `modeDefault`'s `default` branch and runs with **build** - semantics. Add yargs `choices`, as `mcp add`'s `type` already has. +- [x] **`freecode run` runs without hooks.** *Fixed 2026-09-05.* Both entrypoints + now call `hooks/bootstrap.ts`'s `initHooks()`; only `serve` passes + `watch: true`, since a one-shot run exits before a settings edit could apply. + Pinned by `hooks/bootstrap.test.ts`. +- [x] **Headless `build` mode denies everything and says nothing useful.** + *Fixed 2026-09-05.* `freecode run` gained `--yes` (answers the **ask** tier + only) and repeatable `--allow ` (in-memory session grants). Deny rules + and read-only modes still refuse — those are answered decisions, not open + questions. `AgentLoopConfig.autoApproveAsks` / `.sessionGrants` carry it into + the loop; pinned by `agent/headless-permission.test.ts`, whose first case + still asserts the unattended default is deny. +- [x] **`freecode run --agent` is an unchecked cast.** *Fixed 2026-09-05.* yargs + `choices` now rejects an unknown mode at parse time with a usage error; the + cast at the read site is kept, matching `mcp add`'s house style, and is safe + because the runtime check exists. - [ ] **Shell hooks cannot set `modifiedOutput`.** `executeCommandHook` only ever returns `blocked` / `modifiedInput` / `additionalContext`, so the two events whose purpose is rewriting — `PostToolUse` (tool output) and diff --git a/apps/core/src/agent/headless-permission.test.ts b/apps/core/src/agent/headless-permission.test.ts new file mode 100644 index 00000000..96c685de --- /dev/null +++ b/apps/core/src/agent/headless-permission.test.ts @@ -0,0 +1,181 @@ +// ============================================================================= +// Headless permission behavior for `freecode run` (--yes / --allow). +// +// With no frontend subscribed to the bus, askPermission rejects, and an +// unanswered ask is a denial by design (permission/prompt.ts). That is correct +// interactively and made `freecode run "fix the test"` in build mode read files +// fine and be denied every write. These pin the two ways out, and pin that +// neither one weakens a deny rule. +// ============================================================================= + +import test from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { makeTestLayer } from "../effect/layers.js"; +import { makeRuntime } from "../effect/runtime.js"; +import { SessionStoreTag } from "../effect/context.js"; +import { createAgentLoopEffect } from "./loop.js"; +import { registerProvider } from "../providers/registry.js"; +import type { ProviderId } from "../providers/config.js"; +import type { AIProvider, ExecuteResult } from "../providers/types.js"; + +function info(id: string) { + return { + id, + name: id, + defaultModel: "fake-model", + supportsStreaming: false, + supportsTools: true, + maxOutputTokens: 4096, + }; +} + +// A provider that asks for one write on the first turn, then stops. +function registerWriter(provider: string, filePath: string) { + let turn = 0; + registerProvider(provider as ProviderId, { + info: info(provider), + create: (): AIProvider => ({ + info: info(provider), + execute: async (): Promise => { + turn += 1; + if (turn === 1) { + return { + content: "writing the file", + toolCalls: [ + { + name: "write", + args: { filePath, content: "written\n" }, + id: "call-write-1", + }, + ], + stopReason: "tool_use", + provider, + model: "fake-model", + }; + } + return { + content: "done", + toolCalls: [], + stopReason: "stop", + provider, + model: "fake-model", + }; + }, + }), + }); +} + +async function runOnce(opts: { + sessionId: string; + provider: string; + autoApproveAsks?: boolean; + sessionGrants?: string[]; + settings?: Record; +}) { + const projectPath = mkdtempSync(join(tmpdir(), "freecode-headless-perm-")); + const target = join(projectPath, "out.txt"); + registerWriter(opts.provider, target); + + if (opts.settings) { + mkdirSync(join(projectPath, ".freecode"), { recursive: true }); + writeFileSync( + join(projectPath, ".freecode", "settings.json"), + JSON.stringify(opts.settings), + ); + } + + const runtime = makeRuntime(makeTestLayer({})); + try { + const store = await runtime.runPromise(SessionStoreTag); + await store.createSession( + { title: "t", projectPath, provider: opts.provider }, + opts.sessionId, + ); + const loop = await runtime.runPromise( + createAgentLoopEffect(opts.sessionId, { + maxIterations: 3, + autoApproveAsks: opts.autoApproveAsks, + sessionGrants: opts.sessionGrants, + }), + ); + await loop.run({ + prompt: "write the file", + sessionId: opts.sessionId, + provider: opts.provider, + projectPath, + agentMode: "build", + }); + return { target, wrote: existsSync(target) }; + } finally { + await runtime.dispose(); + } +} + +test("headless build mode denies a write when nothing can answer the ask", async () => { + const { wrote } = await runOnce({ + sessionId: "headless-deny", + provider: "headless-perm-deny", + }); + assert.equal(wrote, false, "unattended ask must not silently allow"); +}); + +test("--yes approves the ask and the write actually lands", async () => { + const { target, wrote } = await runOnce({ + sessionId: "headless-yes", + provider: "headless-perm-yes", + autoApproveAsks: true, + }); + assert.equal(wrote, true, "--yes should answer the ask with allow"); + assert.equal(readFileSync(target, "utf-8"), "written\n"); +}); + +test("--allow grants a matching rule for the run only", async () => { + const { wrote } = await runOnce({ + sessionId: "headless-allow", + provider: "headless-perm-allow", + sessionGrants: ["Write"], + }); + assert.equal(wrote, true, "a session grant should satisfy the ask"); +}); + +test("--yes never overrides a deny rule", async () => { + const { wrote } = await runOnce({ + sessionId: "headless-yes-vs-deny", + provider: "headless-perm-yes-deny", + autoApproveAsks: true, + settings: { permissions: { deny: ["Write"] } }, + }); + assert.equal(wrote, false, "deny is absolute; --yes answers asks, not denials"); +}); + +test("--yes never overrides a read-only mode", async () => { + const projectPath = mkdtempSync(join(tmpdir(), "freecode-headless-plan-")); + const target = join(projectPath, "out.txt"); + const provider = "headless-perm-plan"; + registerWriter(provider, target); + + const runtime = makeRuntime(makeTestLayer({})); + try { + const store = await runtime.runPromise(SessionStoreTag); + await store.createSession({ title: "t", projectPath, provider }, "headless-plan"); + const loop = await runtime.runPromise( + createAgentLoopEffect("headless-plan", { + maxIterations: 3, + autoApproveAsks: true, + }), + ); + await loop.run({ + prompt: "write the file", + sessionId: "headless-plan", + provider, + projectPath, + agentMode: "plan", + }); + assert.equal(existsSync(target), false, "plan mode is read-only regardless of --yes"); + } finally { + await runtime.dispose(); + } +}); diff --git a/apps/core/src/agent/loop.ts b/apps/core/src/agent/loop.ts index 06c8ec9e..460c1c67 100644 --- a/apps/core/src/agent/loop.ts +++ b/apps/core/src/agent/loop.ts @@ -92,18 +92,12 @@ import { MemoryService } from "../compaction/index.js"; import { getMaxTurnTokens } from "../compaction/tokens.js"; import { getMemoryGraphService } from "../memory/graph/index.js"; import { renderRetrievedMemories } from "../memory/mem-prompt.js"; -import { - CitationStreamFilter, - parseCitations, -} from "../memory/citations.js"; +import { CitationStreamFilter, parseCitations } from "../memory/citations.js"; import { runConsolidationIfDue } from "../memory/consolidate-run.js"; import { getSessionManager } from "../session/manager.js"; import type { MemoryEntry } from "../memory/mem-types.js"; import { extractMemories } from "../memory/extract.js"; -import { - loadMemorySettings, - shouldExtract, -} from "../memory/extract-policy.js"; +import { loadMemorySettings, shouldExtract } from "../memory/extract-policy.js"; import { createLlmSummarizer } from "../compaction/llm-summarizer.js"; import type { CompactOptions } from "../compaction/service.js"; import { @@ -228,6 +222,21 @@ export interface AgentLoopConfig { * for interactive runs. Spec `2026-08-10-autonomous-runs-design.md` §4.3. */ budgetMaxRedirects?: number; + /** + * Answer every `ask` decision with "allow" instead of prompting. Set by + * `freecode run --yes`, where there is no frontend to prompt: `askPermission` + * rejects with no subscriber, so an unattended `build` run was denied every + * mutating tool it tried. Deliberately scoped to the ask tier only — a deny + * rule and a read-only mode still refuse, because those are decisions someone + * already made, not questions waiting for an answer. + */ + autoApproveAsks?: boolean; + /** + * In-memory allow rules applied to this run's permission settings, from + * `freecode run --allow `. Session grants, so nothing is written to a + * settings file. Never beats a deny rule (`evaluate.ts` §3). + */ + sessionGrants?: string[]; } // ============================================================================= @@ -320,6 +329,7 @@ export class AgentLoop { heuristics: LoopHeuristics; redirect: boolean; budgetMaxRedirects?: number; + autoApproveAsks: boolean; }; private memory: MemoryService; private hooks: HookRuntime; @@ -337,6 +347,7 @@ export class AgentLoop { private compiler: PromptCompiler; // Per-rule permission layer: project + user settings + session grants private permissionSettings: PermissionSettingsManager | undefined; + private sessionGrants: string[] | undefined; // Cancellation: aborted on interrupt(); threaded into provider requests and // tool contexts so in-flight work stops, not just the next loop check. private abort = new AbortController(); @@ -405,7 +416,9 @@ export class AgentLoop { heuristics: { ...DEFAULT_LOOP_HEURISTICS, ...config?.heuristics }, redirect: config?.redirect ?? true, budgetMaxRedirects: config?.budgetMaxRedirects, + autoApproveAsks: config?.autoApproveAsks ?? false, }; + this.sessionGrants = config?.sessionGrants; this.memory = config?.memory ?? new MemoryService(sessionId); this.hooks = config?.hooks ?? createHookRuntime(); this.recorder = config?.recorder ?? createRecorder(sessionId); @@ -657,6 +670,14 @@ export class AgentLoop { this.permissionSettings = new PermissionSettingsManager( input.projectPath, ); + // --allow rules from a headless run, before anything can consult them. + for (const rule of this.sessionGrants ?? []) { + if (!this.permissionSettings.addSessionGrant(rule)) { + logger.warn( + `[AgentLoop] Ignoring unparseable --allow rule: ${rule}`, + ); + } + } this.permissionSettings.watch(); } @@ -1058,7 +1079,12 @@ export class AgentLoop { }; } - return await this.complete("Loop stopped", undefined, undefined, usageSoFar()); + return await this.complete( + "Loop stopped", + undefined, + undefined, + usageSoFar(), + ); } catch (error) { const message = error instanceof Error ? error.message : String(error); return await this.fail("Loop error", message); @@ -2400,29 +2426,37 @@ export class AgentLoop { } if (decision === "ask") { - // Notification Hook — agent needs user attention for approval - await this.hooks.runNotification( - `Permission needed: ${toolCall.tool}${evaluation.matchedRule ? ` — ${evaluation.matchedRule}` : ""}`, - hookContext, - ); - const outcome = this.permissionSettings - ? await promptForPermission({ - toolName: toolCall.tool, - args, - projectRoot: this.state.projectPath, - settings: this.permissionSettings, - sessionId: this.state.sessionId, - reason: - evaluation.matchedRule ?? - `${evaluation.source} (${this.state.agentMode} mode)`, - }) - : { allowed: false, reason: "Permission system unavailable" }; - if (!outcome.allowed) { - return this.denyToolCall( - toolCall, - "user", - `Permission denied: ${outcome.reason ?? "user declined"}`, + // --yes: nobody is listening, so the ask is answered here rather than + // round-tripping to a bus that would reject it and read as a denial. + if (this.config.autoApproveAsks) { + logger.debug( + `[AgentLoop] Auto-approved (--yes): ${toolCall.tool}${evaluation.matchedRule ? ` — ${evaluation.matchedRule}` : ""}`, ); + } else { + // Notification Hook — agent needs user attention for approval + await this.hooks.runNotification( + `Permission needed: ${toolCall.tool}${evaluation.matchedRule ? ` — ${evaluation.matchedRule}` : ""}`, + hookContext, + ); + const outcome = this.permissionSettings + ? await promptForPermission({ + toolName: toolCall.tool, + args, + projectRoot: this.state.projectPath, + settings: this.permissionSettings, + sessionId: this.state.sessionId, + reason: + evaluation.matchedRule ?? + `${evaluation.source} (${this.state.agentMode} mode)`, + }) + : { allowed: false, reason: "Permission system unavailable" }; + if (!outcome.allowed) { + return this.denyToolCall( + toolCall, + "user", + `Permission denied: ${outcome.reason ?? "user declined"}`, + ); + } } } } @@ -2681,7 +2715,9 @@ export class AgentLoop { jaccardSimilarity(prev, reasoning) >= heuristics.reasoningSimilarityThreshold; - const score = similar ? this.state.loopHealth.repeatedReasoningScore + 1 : 0; + const score = similar + ? this.state.loopHealth.repeatedReasoningScore + 1 + : 0; this.state = { ...this.state, loopHealth: { ...this.state.loopHealth, repeatedReasoningScore: score }, @@ -3053,7 +3089,10 @@ export const createAgentLoop = ( // Effect context, so a test layer swaps any of them without patching globals. export const createAgentLoopEffect = ( sessionId: string, - config?: Pick, + config?: Pick< + AgentLoopConfig, + "maxIterations" | "heuristics" | "autoApproveAsks" | "sessionGrants" + >, ): Effect.Effect< AgentLoop, never, diff --git a/apps/core/src/cli/commands/run.ts b/apps/core/src/cli/commands/run.ts index 3b4c6ce8..680c8e0b 100644 --- a/apps/core/src/cli/commands/run.ts +++ b/apps/core/src/cli/commands/run.ts @@ -13,10 +13,23 @@ interface RunArgs { continue: boolean; session?: string; maxTurns?: number; + yes: boolean; + allow: string[]; } type AgentMode = "plan" | "build" | "review" | "explore" | "danger"; +// Passed to yargs `choices`, which rejects anything else at parse time. That +// runtime check is what makes the cast at the read site safe: `--agent buld` +// now exits with a usage error instead of silently running as `build`. +const AGENT_MODES: AgentMode[] = [ + "plan", + "build", + "review", + "explore", + "danger", +]; + // Read piped stdin when no message positional was given (e.g. `echo ... | freecode run`). function readStdin(): Promise { return new Promise((resolve) => { @@ -46,7 +59,8 @@ export const runCommand: CommandModule = { .option("agent", { type: "string", default: "build", - describe: "agent mode: plan | build | review | explore | danger", + choices: AGENT_MODES, + describe: "agent mode", }) .option("continue", { alias: "c", @@ -63,6 +77,20 @@ export const runCommand: CommandModule = { type: "number", describe: "cap on agent iterations; unbounded (loop-health + gates only) if omitted", + }) + .option("yes", { + alias: "y", + type: "boolean", + default: false, + describe: + "approve permission prompts automatically; deny rules and read-only modes still refuse", + }) + .option("allow", { + type: "string", + array: true, + default: [] as string[], + describe: + "grant a permission rule for this run only, e.g. --allow 'Bash(npm test:*)' (repeatable)", }), handler: async (argv) => { // Lazy imports: `run` pulls in the full backend, which no other command @@ -74,6 +102,7 @@ export const runCommand: CommandModule = { const { createAgentLoopEffect } = await import("../../agent/loop.js"); const { getSessionManager } = await import("../../session/index.js"); const { bus } = await import("../../bus/index.js"); + const { initHooks } = await import("../../hooks/bootstrap.js"); let prompt = argv.message.join(" ").trim(); if (!prompt && !process.stdin.isTTY) { @@ -86,6 +115,9 @@ export const runCommand: CommandModule = { await initProviders(); await initMcpServers(); + // Same hooks a served session gets. Not watched: this process runs one turn + // and exits, so a settings.json edit mid-run could not take effect anyway. + const hookSettings = initHooks(process.cwd(), { watch: false }); const config = readConfig(); // --model provider/model overrides the configured current model. @@ -143,15 +175,20 @@ export const runCommand: CommandModule = { } }); + // Safe: yargs `choices` rejected anything outside AGENT_MODES already. const agentMode = argv.agent as AgentMode; try { const loop = await getAppRuntime().runPromise( // Unbounded unless --max-turns is passed, matching Claude Code's // headless mode (maxTurns is opt-in there too, not a default cap). - createAgentLoopEffect( - sessionId, - argv.maxTurns ? { maxIterations: argv.maxTurns } : undefined, - ), + createAgentLoopEffect(sessionId, { + ...(argv.maxTurns ? { maxIterations: argv.maxTurns } : {}), + // Without one of these, `build` denies every mutating tool here: + // there is no frontend to answer an `ask`, and an unanswered ask is + // a denial by design (permission/prompt.ts). + autoApproveAsks: argv.yes, + sessionGrants: argv.allow, + }), ); const result = await getAppRuntime().runPromise( loop.runEffect({ @@ -165,9 +202,11 @@ export const runCommand: CommandModule = { ); process.stdout.write("\n"); unsubscribe(); + hookSettings.dispose(); process.exit(result.success ? 0 : 1); } catch (err) { unsubscribe(); + hookSettings.dispose(); console.error(`\nError: ${(err as Error).message}`); process.exit(1); } diff --git a/apps/core/src/hooks/bootstrap.test.ts b/apps/core/src/hooks/bootstrap.test.ts new file mode 100644 index 00000000..7f3d5e73 --- /dev/null +++ b/apps/core/src/hooks/bootstrap.test.ts @@ -0,0 +1,56 @@ +// ============================================================================= +// The hook bootstrap is shared by `freecode serve` and `freecode run`. +// Before it existed, HookSettingsManager + registerRtkHook were constructed +// inside startServer() only, so a headless run loaded no settings.json hooks: +// the same repo behaved differently depending on which entrypoint ran it. +// ============================================================================= + +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { initHooks } from "./bootstrap.js"; +import { getHooksForEvent, unregisterAllHooks } from "./registry.js"; + +function projectWithHook(): string { + const root = mkdtempSync(join(tmpdir(), "freecode-hook-bootstrap-")); + mkdirSync(join(root, ".freecode"), { recursive: true }); + writeFileSync( + join(root, ".freecode", "settings.json"), + JSON.stringify({ + hooks: { + PostToolUse: [{ name: "fmt", command: "true" }], + }, + }), + ); + return root; +} + +test("initHooks registers project settings.json hooks", () => { + unregisterAllHooks("settings"); + const root = projectWithHook(); + const manager = initHooks(root); + try { + const names = getHooksForEvent("PostToolUse").map((h) => h.name); + assert.ok( + names.includes("fmt"), + `expected the settings.json hook to be registered, got: ${names.join(", ")}`, + ); + } finally { + manager.dispose(); + unregisterAllHooks("settings"); + } +}); + +test("initHooks does not watch unless asked", () => { + unregisterAllHooks("settings"); + const root = projectWithHook(); + // A one-shot `freecode run` must not leave an fs watcher holding the loop + // open past its last turn; dispose is still safe to call either way. + const manager = initHooks(root, { watch: false }); + manager.dispose(); + const watched = initHooks(root, { watch: true }); + watched.dispose(); + unregisterAllHooks("settings"); +}); diff --git a/apps/core/src/hooks/bootstrap.ts b/apps/core/src/hooks/bootstrap.ts new file mode 100644 index 00000000..ee47bc99 --- /dev/null +++ b/apps/core/src/hooks/bootstrap.ts @@ -0,0 +1,39 @@ +// ============================================================================= +// Hook Bootstrap - the one place hooks are turned on for a process +// Both `freecode serve` (server.ts) and `freecode run` (cli/commands/run.ts) +// call this. They used to diverge: the manager was constructed inside +// startServer() only, so a headless run loaded no settings.json hooks and never +// registered the rtk rewrite — the same repo behaved differently under `serve` +// and under `run`, and a formatter that fired after every interactive edit +// silently did not fire in CI. +// ============================================================================= + +import { registerRtkHook } from "./builtin/rtk-rewrite.js"; +import { HookSettingsManager } from "./settings.js"; + +export interface HookBootstrapOptions { + /** + * Reload hooks when settings.json changes. True for the long-lived daemon; + * false for a one-shot run, which exits before an edit could matter and + * would otherwise hold an fs watcher open past its last turn. + */ + watch?: boolean; +} + +/** + * Register built-in hooks and load `settings.json` hooks for `projectRoot`. + * Returns the manager so the caller can dispose it on shutdown. + */ +export function initHooks( + projectRoot: string, + options: HookBootstrapOptions = {}, +): HookSettingsManager { + // Optional rtk integration: rewrites bash commands to compact `rtk` + // equivalents to save tokens. No-op unless rtk resolves; FREECODE_RTK=0 opts out. + registerRtkHook(); + + const hookSettings = new HookSettingsManager(projectRoot); + hookSettings.load(); + if (options.watch) hookSettings.watch(); + return hookSettings; +} diff --git a/apps/core/src/server.ts b/apps/core/src/server.ts index 134a8d62..7355afac 100644 --- a/apps/core/src/server.ts +++ b/apps/core/src/server.ts @@ -78,8 +78,7 @@ import { getInterruptHandler } from "./session/interrupt.js"; import { generateTitleFromPrompt } from "./agent/title-generator.js"; import { initMcpServers, listClients, getMcpTools } from "./mcp/index.js"; import { getConfigDir } from "./cli/utils/config.js"; -import { registerRtkHook } from "./hooks/builtin/rtk-rewrite.js"; -import { HookSettingsManager } from "./hooks/settings.js"; +import { initHooks } from "./hooks/bootstrap.js"; import { bus, BusEvents, @@ -1255,14 +1254,9 @@ export async function startServer() { await initProviders(); await initMcpServers(); - // Optional rtk integration: rewrites bash commands to compact `rtk` - // equivalents to save tokens. No-op unless rtk resolves; FREECODE_RTK=0 opts out. - registerRtkHook(); - - // Load hooks from settings.json (project + user scopes) - const hookSettings = new HookSettingsManager(process.cwd()); - hookSettings.load(); - hookSettings.watch(); + // Built-in hooks + settings.json hooks (project + user scopes). Shared with + // `freecode run` so headless and served runs load the same hooks. + const hookSettings = initHooks(process.cwd(), { watch: true }); // Clean up on shutdown. `exit` cannot await, so the memory flush goes on the // signal handlers, which can (spec D3/D4) — quitting is how most sessions diff --git a/apps/docs/app/reference/cli/page.mdx b/apps/docs/app/reference/cli/page.mdx index 68fccac4..54185498 100644 --- a/apps/docs/app/reference/cli/page.mdx +++ b/apps/docs/app/reference/cli/page.mdx @@ -51,6 +51,7 @@ error — is swallowed and the current version opens. ```bash freecode run [message..] [--model

] [--agent ] [--continue] [--session ] + [--max-turns ] [--yes] [--allow ] ``` | Flag | Alias | Type | Default | Meaning | @@ -60,6 +61,9 @@ freecode run [message..] [--model

] [--agent ] [--continue] [--sessio | `--agent` | — | string | `build` | `plan` \| `build` \| `review` \| `explore` \| `danger` | | `--continue` | `-c` | boolean | `false` | continue the most recent *active* session for this directory | | `--session` | `-s` | string | — | continue a specific session id | +| `--max-turns` | — | number | unbounded | cap on agent iterations; loop-health and the gates are the only limit without it | +| `--yes` | `-y` | boolean | `false` | answer permission prompts with *allow*. Deny rules and read-only modes still refuse | +| `--allow` | — | string (repeatable) | `[]` | grant one permission rule for this run only, e.g. `--allow 'Bash(pnpm test:*)'` | If no message positional is given and stdin is not a TTY, the prompt is read from stdin — `echo "explain this repo" | freecode run` works. @@ -70,17 +74,33 @@ tool activity, thinking, and errors go to **stderr**. That is what makes `0` when the turn succeeded, `1` otherwise (including "no message provided" and "no provider configured"). -The turn is **unbounded** — there is no `--max-turns`. Use -`FREECODE_MAX_TURN_TOKENS` if you need a spend ceiling in CI. +The turn is **unbounded** unless you pass `--max-turns`. That caps iterations, +not spend — use `FREECODE_MAX_TURN_TOKENS` for a token ceiling in CI. > **Read this before scripting `run`.** In headless mode nobody can answer a > permission prompt, and an unanswerable prompt resolves to **deny**, never to a > silent allow. In the default `build` mode every mutating tool (`write`, `edit`, -> `bash`) defaults to *ask*, so a bare `freecode run "fix the test"` can read your -> repository but will be denied every write. Make it work by granting the rules up -> front in `.freecode/settings.json` (`"allow": ["Edit", "Write", "Bash(pnpm test:*)"]`) -> — or, if you genuinely accept the risk in a sandbox, `--agent danger`, which -> bypasses evaluation entirely. See [known gaps](#known-gaps). +> `bash`) defaults to *ask*, so a bare `freecode run "fix the test"` reads your +> repository fine and is denied every write. + +Three ways to make a headless run able to act, narrowest first: + +```bash +# 1. Grant exactly what the task needs, for this run only. +freecode run --allow 'Edit' --allow 'Bash(pnpm test:*)' "fix the failing test" + +# 2. Answer every prompt with allow. Deny rules and read-only modes still refuse. +freecode run --yes "fix the failing test" + +# 3. Persist the grants for the repo, in .freecode/settings.json. +# { "permissions": { "allow": ["Edit", "Write", "Bash(pnpm test:*)"] } } +freecode run "fix the failing test" +``` + +`--yes` answers the *ask* tier and nothing else. A `deny` rule is still absolute, +and `--agent plan|review|explore` is still read-only — those are decisions someone +already made, not questions waiting for an answer. `--agent danger` remains the +only flag that bypasses evaluation entirely; use it only in a sandbox. ## `freecode serve` — the backend alone @@ -92,8 +112,9 @@ No flags. Starts the JSON-RPC 2.0 backend on stdin/stdout — the same process t TUI spawns internally, and what any other frontend attaches to. See [IPC methods](/reference/ipc-methods). -This is also the only entry point that loads `settings.json` **hooks** and -registers the built-in `rtk` rewrite hook. +`settings.json` **hooks** and the built-in `rtk` rewrite hook load here through +`hooks/bootstrap.ts` — the same bootstrap `freecode run` calls, so a headless run +and a served session see the same hooks. ## `freecode web` — browser frontend diff --git a/apps/docs/app/reference/hook-events/page.mdx b/apps/docs/app/reference/hook-events/page.mdx index 819b4991..ec9d2439 100644 --- a/apps/docs/app/reference/hook-events/page.mdx +++ b/apps/docs/app/reference/hook-events/page.mdx @@ -209,5 +209,3 @@ Found while writing this page; each is also tracked in `TODO.md`. the registered hook and nothing reads it. - **`hook.triggered` / `hook.blocked` are published with a cast** and are not part of the typed bus union, so no consumer gets checking on them. -- **Hooks only load under `freecode serve`.** `freecode run` never constructs - `HookSettingsManager`, so headless runs silently skip every hook in the file. diff --git a/apps/docs/app/reference/settings/page.mdx b/apps/docs/app/reference/settings/page.mdx index 25d3df8d..c6f18961 100644 --- a/apps/docs/app/reference/settings/page.mdx +++ b/apps/docs/app/reference/settings/page.mdx @@ -182,9 +182,10 @@ generation first, so reloading is idempotent. The exit-code protocol (`0` continue, `2` block, anything else block) and the JSON stdout form are documented under [hook events](/reference/hook-events#the-shell-protocol). -> Hooks are only loaded by `freecode serve` — the backend the TUI and the other -> frontends spawn. `freecode run` does not load them. See -> [known gaps](#known-gaps). +> Both `freecode serve` (the backend the TUI and the other frontends spawn) and +> `freecode run` load these, through the shared bootstrap in +> `apps/core/src/hooks/bootstrap.ts`. Only `serve` watches the file for changes — +> a one-shot run exits before an edit could apply. ## `memory` @@ -256,6 +257,3 @@ Found while writing this page; each is also tracked in `TODO.md`. - **`once` is parsed but never enforced** (`hooks/settings.ts` → `RegisteredHook`). Either implement per-session tracking or reject the field so it cannot look configured when it is not. -- **Hooks are not loaded in headless runs.** `HookSettingsManager` is constructed - only in `startServer()` (`server.ts:1106`), so `freecode run` silently skips every - hook in the file. From bc8ac45388fd184a2c2a3feb96ec8c3469234c4a Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:51:49 +0530 Subject: [PATCH 03/16] fix(server): redact secrets from `config.get` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `config.get` returned readConfig() verbatim, API keys and web-session cookies included. The full JSON-RPC surface is reachable over web-server.ts's POST /api — token-gated and loopback by default, but `host` is a parameter, so one `--host 0.0.0.0` turned a debug convenience into key exfiltration. redactConfig() builds the safe view field by field: { hasApiKey, model?, authMode? } per provider entry, { hasCredential } per web entry, with current, lastAgentMode and recovery passed through. An allowlist rather than a blocklist of known secret names — WebCredentials has grown a secret-bearing field twice, and a blocklist is wrong the day it grows a third. No consumer lost anything. The only reader of the raw shape was the TUI's getConfig() wrapper, which has no call sites; its ConfigInfo type now matches. Every "is this provider set up" question already went through providers.list's hasApiKey, which never carried the key. Tests: providers/config-redaction.test.ts (4 — no secret from any block survives serialization, the exact kept shape, an anonymous web session reads as hasCredential: false rather than a missing block, an empty config stays empty). beforeStable.md P0 #3. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- TODO.md | 9 +- .../src/providers/config-redaction.test.ts | 83 +++++++++++++++++++ apps/core/src/providers/config.ts | 58 +++++++++++++ apps/core/src/server.ts | 7 +- apps/docs/app/reference/ipc-methods/page.mdx | 18 ++-- apps/tui/src/ipc/client.ts | 3 +- 6 files changed, 167 insertions(+), 11 deletions(-) create mode 100644 apps/core/src/providers/config-redaction.test.ts diff --git a/TODO.md b/TODO.md index 2b5665c4..87bed3a1 100644 --- a/TODO.md +++ b/TODO.md @@ -265,8 +265,13 @@ earlier audit are not repeated here. the default like every other numeric variable. - [ ] **`graph.explore` breaks the memory naming convention** and hard-codes `process.cwd()` while every neighbouring `memory.*` method takes `projectPath`. -- [ ] **`config.get` returns API keys verbatim.** Fine over a local stdio pipe, - wrong the moment the backend is reachable another way; no redaction anywhere. +- [x] **`config.get` returns API keys verbatim.** ~~Fine over a local stdio pipe, + wrong the moment the backend is reachable another way; no redaction anywhere.~~ + Fixed 2026-09-05: `redactConfig()` (`providers/config.ts`) builds a safe view + field by field — `{ hasApiKey, model?, authMode? }` per provider, + `{ hasCredential }` per web session — and `config.get` returns that. An + allowlist, not a blocklist, so the next credential field is excluded by + default. Tests in `providers/config-redaction.test.ts`. - [ ] **MCP servers are user-scope only.** `getConfigDir()` is hard-wired to `~/.freecode` (`cli/utils/config.ts`), so a repository cannot ship the MCP servers its contributors need the way it can ship rules and hooks. diff --git a/apps/core/src/providers/config-redaction.test.ts b/apps/core/src/providers/config-redaction.test.ts new file mode 100644 index 00000000..953f2a0c --- /dev/null +++ b/apps/core/src/providers/config-redaction.test.ts @@ -0,0 +1,83 @@ +import { test, describe } from "node:test"; +import assert from "node:assert"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +// config.ts resolves CONFIG_FILE from os.homedir() at module load, so point HOME +// at a scratch dir before importing it. +const home = fs.mkdtempSync(path.join(os.tmpdir(), "freecode-redact-")); +process.env.HOME = home; +const configFile = path.join(home, ".freecode", "config.json"); + +const { redactConfig } = await import("./config.js"); + +function writeConfigFile(config: unknown): void { + fs.mkdirSync(path.dirname(configFile), { recursive: true }); + fs.writeFileSync(configFile, JSON.stringify(config)); +} + +const SECRETS = [ + "sk-ant-secret", + "sk-openai-secret", + "SID=cookie-secret", + "xsrf-secret", + "bearer-secret", +]; + +describe("redactConfig", () => { + test("no secret survives, whatever block it sits in", () => { + writeConfigFile({ + providers: { + anthropic: { apiKey: SECRETS[0], model: "claude-x", authMode: "oauth" }, + openai: { apiKey: SECRETS[1] }, + }, + web: { + "gemini-web": { + cookie: SECRETS[2], + xsrfToken: SECRETS[3], + apiKey: SECRETS[4], + authUser: "1", + }, + }, + current: { provider: "anthropic", model: "claude-x" }, + }); + const serialized = JSON.stringify(redactConfig()); + for (const secret of SECRETS) { + assert.ok(!serialized.includes(secret), `leaked ${secret}`); + } + }); + + test("keeps what a caller actually asked: is a key set, and which model", () => { + writeConfigFile({ + providers: { + anthropic: { apiKey: SECRETS[0], model: "claude-x", authMode: "oauth" }, + openai: { apiKey: "" }, + }, + current: { provider: "anthropic", model: "claude-x" }, + lastAgentMode: "build", + recovery: { fallbackProviders: ["openai"] }, + }); + assert.deepEqual(redactConfig(), { + providers: { + anthropic: { hasApiKey: true, model: "claude-x", authMode: "oauth" }, + openai: { hasApiKey: false }, + }, + current: { provider: "anthropic", model: "claude-x" }, + lastAgentMode: "build", + recovery: { fallbackProviders: ["openai"] }, + }); + }); + + test("an anonymous web session reads as no credential, not a missing block", () => { + writeConfigFile({ web: { "gemini-web": { authUser: "0" } } }); + assert.deepEqual(redactConfig().web, { + "gemini-web": { hasCredential: false }, + }); + }); + + test("an empty config redacts to an empty object", () => { + writeConfigFile({}); + assert.deepEqual(redactConfig(), {}); + }); +}); diff --git a/apps/core/src/providers/config.ts b/apps/core/src/providers/config.ts index e5765aa9..5af61eff 100644 --- a/apps/core/src/providers/config.ts +++ b/apps/core/src/providers/config.ts @@ -291,3 +291,61 @@ export function subscriptionAuth(providerId: string): "oauth" | undefined { ? "oauth" : undefined; } + +/** + * `Config` with every secret replaced by whether it is set. + * + * `apiKey`, and each of the four secret-bearing `WebCredentials` fields, are + * the whole reason this shape exists: nothing that authenticates is worth + * sending anywhere, and the only question a caller ever asked of them was + * "is one configured". `hasApiKey`/`hasCredential` answer that. + */ +export interface RedactedConfig { + providers?: Record< + string, + { hasApiKey: boolean; model?: string; authMode?: AnthropicAuthMode } + >; + web?: Record; + current?: Config["current"]; + lastAgentMode?: string; + recovery?: Config["recovery"]; +} + +/** + * The safe view of `config.json`, for anything that leaves the process. + * + * Built field by field rather than by deleting known secrets from a spread: a + * blocklist is wrong the day a credential field is added, and this file's + * `WebCredentials` has grown one twice. + */ +export function redactConfig(config: Config = readConfig()): RedactedConfig { + const redacted: RedactedConfig = {}; + if (config.providers) { + redacted.providers = Object.fromEntries( + Object.entries(config.providers).map(([id, entry]) => [ + id, + { + hasApiKey: Boolean(entry?.apiKey), + ...(entry?.model ? { model: entry.model } : {}), + ...(entry?.authMode ? { authMode: entry.authMode } : {}), + }, + ]), + ); + } + if (config.web) { + redacted.web = Object.fromEntries( + Object.entries(config.web).map(([id, credential]) => [ + id, + { + hasCredential: Boolean( + credential?.cookie || credential?.cookieFile || credential?.apiKey, + ), + }, + ]), + ); + } + if (config.current) redacted.current = config.current; + if (config.lastAgentMode) redacted.lastAgentMode = config.lastAgentMode; + if (config.recovery) redacted.recovery = config.recovery; + return redacted; +} diff --git a/apps/core/src/server.ts b/apps/core/src/server.ts index 7355afac..98279298 100644 --- a/apps/core/src/server.ts +++ b/apps/core/src/server.ts @@ -29,6 +29,7 @@ import { } from "./models-dev.js"; import { readConfig, + redactConfig, writeConfig, setApiKey, setCurrentModel, @@ -872,8 +873,12 @@ const methodHandlers: Record< return { prompt }; }, + // Redacted, not raw: this is reachable over `web-server.ts`'s POST /api, + // whose `host` is a parameter — one `--host 0.0.0.0` would otherwise turn a + // debug convenience into key exfiltration. No caller ever wanted the key + // itself; `hasApiKey` is the question they were all asking. "config.get": async (): Promise => { - return readConfig(); + return redactConfig(); }, "config.setApiKey": async ( diff --git a/apps/docs/app/reference/ipc-methods/page.mdx b/apps/docs/app/reference/ipc-methods/page.mdx index d9ca4e39..5deac7d9 100644 --- a/apps/docs/app/reference/ipc-methods/page.mdx +++ b/apps/docs/app/reference/ipc-methods/page.mdx @@ -110,14 +110,21 @@ itself never crosses the pipe. | ✓ | Method | Params | Result | | --- | --- | --- | --- | -| | `config.get` | — | the parsed `~/.freecode/config.json` | +| | `config.get` | — | `~/.freecode/config.json`, **redacted** | | | `config.setApiKey` | `{ provider, apiKey, model? }` | `void` | | | `config.getCurrentModel` / `config.setCurrentModel` | — / `{ provider, model }` | current pair / `void` | | | `config.getLastAgentMode` / `config.setLastAgentMode` | — / `{ mode }` | mode / `void` | -`config.get` returns the file as-is, **API keys included**. It is fine over a -stdio pipe to a local frontend; it is worth knowing about before exposing the -backend any other way. +`config.get` returns the file with every secret replaced by whether it is set: +each provider entry becomes `{ hasApiKey, model?, authMode? }` and each `web` +entry `{ hasCredential }`. `current`, `lastAgentMode` and `recovery` pass +through unchanged. The redaction is an allowlist built field by field, not a +blocklist of known secret names, so a credential field added later is excluded +by default rather than leaked until someone remembers it. + +That matters because the same method is reachable over the web transport's +`POST /api`, whose `host` is a parameter — a backend bound to `0.0.0.0` would +otherwise hand out API keys. ## Memory @@ -230,6 +237,3 @@ Also tracked in `TODO.md`; the first three are shared with - **`graph.explore` breaks the naming convention** and hard-codes `process.cwd()` while every neighbouring memory method takes `projectPath`. It should be `memory.graph.explore` with the same parameter. -- **`config.get` returns API keys.** Fine for a local stdio frontend, wrong the - moment the backend is reachable any other way; the web transport gates `/api` but - the method itself does no redaction. diff --git a/apps/tui/src/ipc/client.ts b/apps/tui/src/ipc/client.ts index 77752ad8..0b67461a 100644 --- a/apps/tui/src/ipc/client.ts +++ b/apps/tui/src/ipc/client.ts @@ -689,8 +689,9 @@ export async function graphExplore(): Promise< | { error: "not-installed" }; } +/** Redacted: `config.get` reports whether a key is set, never the key. */ export interface ConfigInfo { - providers?: Record; + providers?: Record; current?: { provider: string; model: string }; } From fc6c65fbe3842a02694690e676a147aa6a156a0e Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:52:26 +0530 Subject: [PATCH 04/16] fix(cli): `freecode uninstall` keeps user data unless --purge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It removed all of ~/.freecode on one `y` — sessions, rollout logs, memory, prompt history, usage — and the prompt did not say so. scripts/uninstall.sh already drew the right line (--purge for a full wipe, data kept otherwise); the CLI command was the copy that got it wrong, so the fix is to make them agree rather than invent a third behaviour. - Default takes the launcher and ~/.freecode/builds — the program. Everything else stays. - `--purge` takes the directory, and the line it prints names what is inside it rather than the path alone. - `--dry-run` added; `--force` gained -y/--yes so both entry points accept the same flags. No `--keep-data`: keeping data is now the default, and a flag for the default reads like the other behaviour is still lurking. Two things fell out. A non-TTY stdin resolved readline.question at once with an empty answer, which the old code read as "no" — correct by accident, and silent; it is now an error naming --force. And the isDirectory() branch before rmSync/unlinkSync is gone, since rmSync with recursive covers both. Tests: cli/commands/uninstall.test.ts (5, against a pure planUninstall() so the safety rule is asserted without deleting anything). The separate gap that uninstall ignores FREECODE_HOME / FREECODE_INSTALL_DIR and misses the Windows launcher path is untouched and still tracked; it takes the fixed item's place in the CLI page's known gaps. beforeStable.md P0 #4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- TODO.md | 9 +- apps/core/src/cli/commands/uninstall.test.ts | 59 ++++++ apps/core/src/cli/commands/uninstall.ts | 169 ++++++++++++------ .../app/getting-started/installation/page.mdx | 22 +-- apps/docs/app/reference/cli/page.mdx | 31 ++-- apps/docs/app/reference/page.mdx | 5 +- 6 files changed, 216 insertions(+), 79 deletions(-) create mode 100644 apps/core/src/cli/commands/uninstall.test.ts diff --git a/TODO.md b/TODO.md index 87bed3a1..8332ab5d 100644 --- a/TODO.md +++ b/TODO.md @@ -278,9 +278,14 @@ earlier audit are not repeated here. - [ ] **`freecode session` exposes 2 of 12 session operations.** `fork`, `switch`, `archive`, `export`, `import`, `upload`, `download` are IPC-only, so scripting session management means speaking JSON-RPC by hand. -- [ ] **`freecode uninstall` deletes user data on one `y`.** It removes all of +- [x] **`freecode uninstall` deletes user data on one `y`.** ~~It removes all of `~/.freecode` — sessions, rollout logs, memory, history, usage — and the prompt - does not say so. No `--keep-data`, no backup. + does not say so. No `--keep-data`, no backup.~~ Fixed 2026-09-05: the default + now removes the launcher and `~/.freecode/builds` only, `--purge` is what takes + the data directory, and the target list names what is inside it. Added + `--dry-run`; a non-TTY stdin errors instead of resolving the prompt as a silent + "no". Same semantics as `scripts/uninstall.sh`, which was always the correct + copy. Tests in `cli/commands/uninstall.test.ts`. - [ ] **No way to print effective configuration.** Diagnosing "why is it compacting so early" means reading source. A `freecode config env` dumping name / default / effective / source would pay for itself. diff --git a/apps/core/src/cli/commands/uninstall.test.ts b/apps/core/src/cli/commands/uninstall.test.ts new file mode 100644 index 00000000..446cada6 --- /dev/null +++ b/apps/core/src/cli/commands/uninstall.test.ts @@ -0,0 +1,59 @@ +import { test, describe } from "node:test"; +import assert from "node:assert"; +import * as path from "node:path"; +import { planUninstall } from "./uninstall.js"; + +const homeDir = "/home/tester"; +const configDir = path.join(homeDir, ".freecode"); +const launcher = path.join(homeDir, ".local/bin/freecode"); +const builds = path.join(configDir, "builds"); + +/** Everything an installed-and-used freecode has on disk. */ +const onDisk = new Set([launcher, configDir, builds]); +const exists = (p: string) => onDisk.has(p); + +function plan(purge: boolean): string[] { + return planUninstall({ homeDir, configDir, purge, exists }).map((t) => t.path); +} + +describe("planUninstall", () => { + test("the default removes the program and keeps the data directory", () => { + const targets = plan(false); + assert.deepEqual(targets, [launcher, builds]); + assert.ok(!targets.includes(configDir), "would delete user data"); + }); + + test("--purge takes the data directory, and says so", () => { + const targets = planUninstall({ homeDir, configDir, purge: true, exists }); + assert.deepEqual( + targets.map((t) => t.path), + [launcher, configDir], + ); + const dataTarget = targets.find((t) => t.path === configDir); + assert.match(dataTarget!.label, /sessions/); + assert.match(dataTarget!.label, /memory/); + }); + + test("--purge does not also list builds, which is inside the directory it takes", () => { + assert.ok(!plan(true).includes(builds)); + }); + + test("a launcher that is not on disk is not listed", () => { + const targets = planUninstall({ + homeDir, + configDir, + purge: false, + exists: (p) => p === builds, + }); + assert.deepEqual( + targets.map((t) => t.path), + [builds], + ); + }); + + test("nothing installed plans nothing, in either mode", () => { + const none = () => false; + assert.deepEqual(planUninstall({ homeDir, configDir, purge: false, exists: none }), []); + assert.deepEqual(planUninstall({ homeDir, configDir, purge: true, exists: none }), []); + }); +}); diff --git a/apps/core/src/cli/commands/uninstall.ts b/apps/core/src/cli/commands/uninstall.ts index bcca81a2..fafb6d63 100644 --- a/apps/core/src/cli/commands/uninstall.ts +++ b/apps/core/src/cli/commands/uninstall.ts @@ -2,10 +2,68 @@ import type { CommandModule } from "yargs"; import * as fs from "fs"; import * as path from "path"; import * as readline from "readline"; -import { execSync } from "child_process"; +import { CONFIG_DIR } from "../../providers/config.js"; interface UninstallArgs { force: boolean; + purge: boolean; + "dry-run": boolean; +} + +/** One thing to delete, and what a reader would call it. */ +export interface UninstallTarget { + path: string; + label: string; +} + +/** Binaries the installer may have written, in the order it prefers them. */ +function binaryPaths(homeDir: string): string[] { + return [ + "/usr/local/bin/freecode", + "/usr/bin/freecode", + path.join(homeDir, ".local/bin/freecode"), + path.join(homeDir, ".cargo/bin/freecode"), + ]; +} + +/** + * What `uninstall` would delete — pure, so the safety rule is testable without + * deleting anything. + * + * The rule: **the program goes, the data stays.** `~/.freecode` is not an + * install directory that happens to hold a binary; it is sessions, rollout + * logs, per-project memory, prompt history and usage, and only `--purge` takes + * those. This matches `scripts/uninstall.sh`, which has always drawn the line + * here — the CLI was the copy that got it wrong. + */ +export function planUninstall(opts: { + homeDir: string; + configDir: string; + purge: boolean; + exists?: (p: string) => boolean; +}): UninstallTarget[] { + const exists = opts.exists ?? ((p: string) => fs.existsSync(p)); + const targets: UninstallTarget[] = []; + + for (const binPath of binaryPaths(opts.homeDir)) { + if (exists(binPath)) targets.push({ path: binPath, label: "launcher" }); + } + + if (opts.purge) { + if (exists(opts.configDir)) { + targets.push({ + path: opts.configDir, + label: "ALL user data: sessions, rollout logs, memory, history, usage", + }); + } + return targets; + } + + const builds = path.join(opts.configDir, "builds"); + if (exists(builds)) { + targets.push({ path: builds, label: "installed binaries" }); + } + return targets; } async function askConfirmation(message: string): Promise { @@ -15,25 +73,37 @@ async function askConfirmation(message: string): Promise { }); return new Promise((resolve) => { - rl.question(`${message} (y/n) `, (answer) => { + rl.question(`${message} [y/N] `, (answer) => { rl.close(); - resolve(answer.toLowerCase() === "y"); + resolve(answer.trim().toLowerCase() === "y"); }); }); } export const uninstallCommand: CommandModule = { command: "uninstall", - describe: "uninstall freecode and remove all related files", + describe: "remove the freecode binaries (user data is kept unless --purge)", builder: (yargs) => - yargs.option("force", { - type: "boolean", - default: false, - describe: "skip confirmation prompt", - alias: "f", - }), + yargs + .option("force", { + type: "boolean", + default: false, + describe: "skip confirmation prompt", + alias: ["f", "y", "yes"], + }) + .option("purge", { + type: "boolean", + default: false, + describe: `also delete ${CONFIG_DIR} — sessions, memory, history, usage`, + }) + .option("dry-run", { + type: "boolean", + default: false, + describe: "print what would be removed, delete nothing", + }), handler: async (argv) => { - const { force } = argv; + const { force, purge } = argv; + const dryRun = argv["dry-run"]; const homeDir = process.env.HOME || process.env.USERPROFILE || ""; if (!homeDir) { @@ -41,74 +111,71 @@ export const uninstallCommand: CommandModule = { process.exit(1); } - const freecodePath = path.join(homeDir, ".freecode"); - const itemsToRemove = [freecodePath]; - - // Check for binary in common locations - const binPaths = [ - "/usr/local/bin/freecode", - "/usr/bin/freecode", - path.join(homeDir, ".local/bin/freecode"), - path.join(homeDir, ".cargo/bin/freecode"), - ]; + const targets = planUninstall({ homeDir, configDir: CONFIG_DIR, purge }); - for (const binPath of binPaths) { - try { - if (fs.existsSync(binPath)) { - itemsToRemove.push(binPath); - } - } catch { - // ignore - } + if (targets.length === 0) { + console.log("Nothing to uninstall: no freecode installation found."); + process.exit(0); } - // Show what will be removed console.log("\nThe following will be removed:"); - itemsToRemove.forEach((item) => console.log(` - ${item}`)); + targets.forEach((t) => console.log(` - ${t.path} (${t.label})`)); + if (!purge) { + console.log( + `\nUser data in ${CONFIG_DIR} is kept. Use --purge for a full wipe.`, + ); + } + + if (dryRun) { + console.log("\nDry run: nothing deleted."); + process.exit(0); + } if (!force) { - const confirmed = await askConfirmation( - "\nProceed with uninstallation?", - ); + // A piped stdin resolves the prompt instantly with an empty answer, which + // would read as "no" — but silently, and only after the list scrolled by. + // Say so instead. + if (!process.stdin.isTTY) { + console.error( + "\nError: stdin is not a terminal; re-run with --force to skip the prompt.", + ); + process.exit(1); + } + const confirmed = await askConfirmation("\nProceed?"); if (!confirmed) { console.log("Uninstallation cancelled."); process.exit(0); } } - // Remove items - let removed: string[] = []; - let errors: Array<{ item: string; error: string }> = []; + const removed: string[] = []; + const errors: Array<{ item: string; error: string }> = []; - for (const item of itemsToRemove) { + for (const { path: item } of targets) { try { if (fs.existsSync(item)) { - const stat = fs.statSync(item); - if (stat.isDirectory()) { - fs.rmSync(item, { recursive: true, force: true }); - } else { - fs.unlinkSync(item); - } + fs.rmSync(item, { recursive: true, force: true }); removed.push(item); } - } catch (error: any) { - errors.push({ item, error: error.message }); + } catch (error) { + errors.push({ item, error: (error as Error).message }); } } - // Report results console.log("\n✓ Successfully removed:"); removed.forEach((item) => console.log(` - ${item}`)); if (errors.length > 0) { console.log("\n⚠ Failed to remove:"); - errors.forEach(({ item, error }) => - console.log(` - ${item}: ${error}`), - ); + errors.forEach(({ item, error }) => console.log(` - ${item}: ${error}`)); process.exit(1); } - console.log("\n✓ FreeCode has been uninstalled."); + console.log( + purge + ? "\n✓ FreeCode has been uninstalled and all data removed." + : `\n✓ FreeCode has been uninstalled. Your data is still in ${CONFIG_DIR}.`, + ); process.exit(0); }, }; diff --git a/apps/docs/app/getting-started/installation/page.mdx b/apps/docs/app/getting-started/installation/page.mdx index 0ee82907..652ece15 100644 --- a/apps/docs/app/getting-started/installation/page.mdx +++ b/apps/docs/app/getting-started/installation/page.mdx @@ -132,19 +132,21 @@ that works in the repo root and is broken everywhere else. ## Uninstalling ```bash -freecode uninstall # asks first -freecode uninstall --force # does not +freecode uninstall # removes the binaries, keeps your data +freecode uninstall --dry-run # prints the list, deletes nothing +freecode uninstall --purge # also deletes ~/.freecode +freecode uninstall --force # skips the y/N prompt ``` -It prints what it will remove, then deletes `~/.freecode` **entirely** plus a -`freecode` binary found in `/usr/local/bin`, `/usr/bin`, `~/.local/bin`, or -`~/.cargo/bin`. +It prints what it will remove, then takes a `freecode` binary found in +`/usr/local/bin`, `/usr/bin`, `~/.local/bin`, or `~/.cargo/bin`, plus +`~/.freecode/builds`. -> **That directory is not just the binary.** Your sessions, rollout logs, -> per-project memory, prompt history, and usage data all live under -> `~/.freecode/`. There is no `--keep-data` and nothing is backed up. Copy -> `~/.freecode/projects/` and `~/.freecode/sessions/` first if you might want -> them later. +> **`--purge` is the one that costs you something.** Your sessions, rollout +> logs, per-project memory, prompt history, and usage data all live under +> `~/.freecode/`, and that flag is what deletes them. Nothing is backed up, so +> copy `~/.freecode/projects/` and `~/.freecode/sessions/` first if you might +> want them later. There is also `curl -fsSL https://freecode.website/uninstall | bash`, for when the binary itself is broken. diff --git a/apps/docs/app/reference/cli/page.mdx b/apps/docs/app/reference/cli/page.mdx index 54185498..ff71f82d 100644 --- a/apps/docs/app/reference/cli/page.mdx +++ b/apps/docs/app/reference/cli/page.mdx @@ -271,21 +271,23 @@ from source. ## `freecode uninstall` ```bash -freecode uninstall [--force] +freecode uninstall [--purge] [--dry-run] [--force] ``` | Flag | Alias | Default | Meaning | | --- | --- | --- | --- | -| `--force` | `-f` | `false` | skip the confirmation prompt | - -Removes `~/.freecode` **entirely**, plus a `freecode` binary found in -`/usr/local/bin`, `/usr/bin`, `~/.local/bin`, or `~/.cargo/bin`. It prints the -list first and asks `y/n` unless `--force` is given. Exit code is `1` if any -removal failed. - -That directory is not just the binary: it contains your sessions, rollout logs, -memory, prompt history, and usage data. There is no `--keep-data`, and nothing is -backed up. Copy `~/.freecode/projects/` and `~/.freecode/sessions/` first if you +| `--purge` | — | `false` | also delete `~/.freecode` — sessions, memory, history, usage | +| `--dry-run` | — | `false` | print what would be removed, delete nothing | +| `--force` | `-f`, `-y`, `--yes` | `false` | skip the confirmation prompt | + +**The program goes, the data stays.** By default it removes a `freecode` binary +found in `/usr/local/bin`, `/usr/bin`, `~/.local/bin`, or `~/.cargo/bin`, plus +`~/.freecode/builds` (the installed versions). Your sessions, rollout logs, +memory, prompt history, and usage stay where they are. + +`--purge` is what takes `~/.freecode` entirely, and the confirmation names what +is inside it rather than saying only "proceed". Nothing is backed up either way, +so copy `~/.freecode/projects/` and `~/.freecode/sessions/` before purging if you might want them back. ## Known gaps @@ -320,6 +322,7 @@ Found while writing this page; each is also tracked in `TODO.md`. - **MCP config is user-scope only.** `getConfigDir()` is hard-wired to `~/.freecode`, so a repository cannot ship the MCP servers its contributors need the way it can ship permission rules and hooks. -- **`uninstall` deletes user data with one `y`.** No `--keep-data`, no backup, and - the prompt does not spell out that sessions and memory are inside the directory - being removed. +- **`uninstall` ignores the variables the installer honours.** It hard-codes + `~/.freecode` and four Unix bin paths, while `install.sh` supports + `FREECODE_HOME` and `FREECODE_INSTALL_DIR`, and the Windows launcher path is + not in the list at all. diff --git a/apps/docs/app/reference/page.mdx b/apps/docs/app/reference/page.mdx index be88f1cc..3b6465a1 100644 --- a/apps/docs/app/reference/page.mdx +++ b/apps/docs/app/reference/page.mdx @@ -56,8 +56,9 @@ up. Everything FreeCode persists is under one root: | `addons/graph-ui/` | the optional memory graph explorer (`freecode memory ui-install`) | | `builds/` | installed binaries; `builds/stable/freecode` is the symlink the updater rewrites | -`freecode uninstall` deletes this entire directory. That includes your sessions -and your memory — see [CLI commands](/reference/cli#freecode-uninstall). +`freecode uninstall` keeps this directory and takes only `builds/`; the entire +directory — your sessions and your memory included — goes only with +`--purge`. See [CLI commands](/reference/cli#freecode-uninstall). ## The pages From 560ad2f8d11406eabc3f7a5149f0c8af1ecac053 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:52:42 +0530 Subject: [PATCH 05/16] docs: publish the Anthropic subscription auth stance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §0.1 of the OAuth spec was the risk section and had no resolution. A v1 that ships subscription auth as a headline feature needs an explicit, user-visible stance — not a spec paragraph. Resolved as: ship it, state the risk plainly, do not euphemize. Recorded in the spec as §0.2 and, more importantly, published: - /getting-started/anthropic-subscription — a new page whose second section is the risk, before any instructions. FreeCode presents itself to Anthropic as Claude Code; Anthropic reserves subscription inference for its own surfaces and has acted against tools doing this; the account at risk is the user's. Then the three opt-ins and the narrow fourth (no key configured plus a freecode login — an imported Claude Code login deliberately does not count), the login flow, status/logout, the org-forbidden 403 latch and API-key fallback, and why cost is stamped on the call. - /reference/cli gained a `freecode auth` section; the command was undocumented entirely. - /reference/env gained FREECODE_ANTHROPIC_AUTH, flagged as an opt-in rather than a neutral switch. The code needed nothing: auth.ts already printed the §0.1 disclosure on login and the identity block was already quarantined to the OAuth path with a test. The gap was that a user could only read any of it after deciding to run the command. Not adopted: a y/N confirmation on login. §0.1 chose "print once, no repeated nagging" deliberately, and a prompt in front of a command the user just typed by name buys nothing the paragraph above it does not already say. Three known gaps found while writing (unmapped tool names, single account, anthropic-only OAuth) are recorded in TODO.md. beforeStable.md P0 #5. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- TODO.md | 17 +++ apps/docs/app/getting-started/_meta.ts | 1 + .../anthropic-subscription/page.mdx | 129 ++++++++++++++++++ apps/docs/app/reference/cli/page.mdx | 53 +++++-- apps/docs/app/reference/env/page.mdx | 9 ++ .../2026-09-05-anthropic-oauth-provider.md | 22 +++ 6 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 apps/docs/app/getting-started/anthropic-subscription/page.mdx diff --git a/TODO.md b/TODO.md index 8332ab5d..9cf57b62 100644 --- a/TODO.md +++ b/TODO.md @@ -1114,3 +1114,20 @@ Found while writing `specs/2026-09-04-harness-cost-efficiency.md`. `metadata.outputKind`, so MCP tools (which can be just as log-noisy) always take the blind head+tail path. Extend classification once the D2 A/B proves the approach. Recorded in `/internals/cost-efficiency` Known gaps. + +## Docs findings (Anthropic subscription login — 2026-09-05) + +Found while writing `/getting-started/anthropic-subscription`, the page that +publishes the OAuth ToS stance (`beforeStable.md` P0 #5). Recorded in that +page's Known gaps. + +- [ ] **Tool names are forwarded unmapped on the OAuth path.** jcode renames + tools to the ones Claude Code ships; freecode does not. Spec §9 Q1 — a + tool-use-*quality* question, not an access or billing one, and it waits on + a real turn. +- [ ] **No multi-account support.** One Anthropic login per machine + (`auth.json` is keyed by provider, not by account). Deliberate YAGNI in the + spec's §2 non-goals; listed here so the docs claim has a home. +- [ ] **`anthropic` is the only provider with an OAuth mode.** `freecode auth + login` rejects any other provider by name. Fine today — no other catalogue + entry has a subscription surface freecode can reach. diff --git a/apps/docs/app/getting-started/_meta.ts b/apps/docs/app/getting-started/_meta.ts index c2fda227..02b6921f 100644 --- a/apps/docs/app/getting-started/_meta.ts +++ b/apps/docs/app/getting-started/_meta.ts @@ -5,5 +5,6 @@ export default { installation: "Installation", quickstart: "Quickstart", providers: "Providers & API keys", + "anthropic-subscription": "Anthropic subscription", configuration: "Configuration" } satisfies MetaRecord; diff --git a/apps/docs/app/getting-started/anthropic-subscription/page.mdx b/apps/docs/app/getting-started/anthropic-subscription/page.mdx new file mode 100644 index 00000000..b4e31e3a --- /dev/null +++ b/apps/docs/app/getting-started/anthropic-subscription/page.mdx @@ -0,0 +1,129 @@ +--- +title: "Anthropic subscription login" +description: "Use a Claude Pro/Max subscription instead of an API key — how it works, and the risk you take." +--- + +# Anthropic subscription login + +FreeCode can authenticate the `anthropic` provider with your Claude Pro/Max +subscription instead of a metered API key: + +```bash +freecode auth login anthropic +``` + +Read the next section before you run it. It is not a formality. + +## Before you turn this on + +Your Pro/Max allowance is only reachable through Anthropic's OAuth surface, and +that surface only answers requests that look like Claude Code. So to use it, +FreeCode sends **Claude Code's OAuth client id, its `User-Agent` and beta +headers, and its identity line as the first system block.** It presents itself +to Anthropic as Claude Code, because that is the only thing the endpoint +accepts. + +> **This is a spoof, and we are not going to call it anything else.** +> +> - Anthropic reserves subscription inference for its own official surfaces. +> Using it from FreeCode is against the spirit, and arguably the letter, of +> your agreement with them — however common the practice is across +> open-source agents. +> - Anthropic has acted against tools that do this. The Cloudflare challenge at +> the token endpoint, and tokens that refresh cleanly but are refused at +> inference time, are both things FreeCode handles because they happen. +> - **The account at risk is yours.** Not FreeCode's keys, not FreeCode's +> infrastructure. Yours. + +That is the whole stance. We ship the feature, we do not hide what it does, and +whether the trade is worth it is your call, on your account. If it isn't, use an +[API key](/getting-started/providers) — that path is unchanged and is still the +default. + +## What is on by default + +Nothing. API key is the default auth mode, and a machine with a key configured +never silently switches to your subscription. OAuth activates on exactly three +opt-ins: + +| Opt-in | Where | +| --- | --- | +| `freecode auth login anthropic` | pins the mode for you as part of logging in | +| `providers.anthropic.authMode: "oauth"` | `~/.freecode/config.json` | +| `FREECODE_ANTHROPIC_AUTH=oauth` | environment, wins over config | + +There is one more path, and it is narrow: if you have **no** Anthropic API key +configured but do have a FreeCode login stored, it uses the login rather than +failing. An official Claude Code login sitting in `~/.claude/.credentials.json` +does **not** count — importing someone else's session is not an opt-in you made. + +## Logging in + +```bash +freecode auth login anthropic # opens a browser +freecode auth login anthropic --no-browser # print the URL, paste the code back +``` + +It runs PKCE against a `localhost` callback, waiting up to 120 seconds. If the +callback cannot be reached — a remote box, a locked-down browser — it falls back +to printing a URL you can open anywhere and pasting the resulting code back. +Either way it is a single process: the OAuth `state` **is** the PKCE verifier, so +there is no second command to run and no code to carry between terminals. + +Tokens land in `~/.freecode/auth.json` at mode `0600` — a different file from +`config.json` on purpose, because that one gets hand-edited and sometimes ends up +in a dotfiles repo. They refresh automatically. + +## Checking and reverting + +```bash +freecode auth status +``` + +``` +anthropic auth mode: oauth + oauth token: valid until 05/09/2026, 18:41:00 + scopes: user:inference user:profile +``` + +```bash +freecode auth logout anthropic +``` + +Logout deletes the stored tokens **and** un-pins the mode, so `anthropic` goes +straight back to your API key. Nothing else about your setup is touched. + +## When Anthropic says no + +If your organization is not allowed to use OAuth, the API returns a 403 that +reads like a rejected login. FreeCode detects that specific refusal at the fetch +layer, **latches it for the rest of the process** — retrying is pointless — and +falls back to your API key if one is configured. The identity block is dropped +along with it: a request authenticated with a real API key must never carry the +Claude Code identity string, and that split is enforced in code with a test. + +A Cloudflare challenge at the token endpoint gets its own message, because the +raw response looks like bad credentials and is not. + +## Cost accounting + +A subscription call is not free, it is prepaid — but it does not price like a +metered one. FreeCode stamps the auth mode onto the recorded call +(`model.response` carries `authMode`) rather than reading live config when the +log is later folded, so a session's cost does not change because you switched +modes afterwards. + +The eval harness goes further: `baselineFor` **refuses to compare across an +auth-mode switch**, so a subscription run never becomes the bar an API-key run is +measured against. + +## Known gaps + +Also tracked in `TODO.md`. + +- **Tool names are forwarded unmapped.** Other clients rename tools to the ones + Claude Code ships. FreeCode does not, which is a tool-use-*quality* question, + not an access or billing one — the endpoint accepts the calls either way. +- **No multi-account support.** One Anthropic login per machine. +- **`anthropic` is the only provider with an OAuth mode.** `freecode auth login` + rejects any other provider by name. diff --git a/apps/docs/app/reference/cli/page.mdx b/apps/docs/app/reference/cli/page.mdx index ff71f82d..cec80422 100644 --- a/apps/docs/app/reference/cli/page.mdx +++ b/apps/docs/app/reference/cli/page.mdx @@ -262,6 +262,38 @@ Servers are written to `~/.freecode/config.json` under `mcp.servers`, with `enabled: true` and `timeout: 5000` defaults. There is no project-scoped MCP config ([known gaps](#known-gaps)). +## `freecode auth` + +```bash +freecode auth login [provider] [--no-browser] +freecode auth status +freecode auth logout [provider] +``` + +Authenticates the `anthropic` provider with a Claude Pro/Max subscription +instead of an API key. `provider` defaults to `anthropic` and any other value is +rejected by name — it is the only provider with an OAuth mode. + +| Command | Flag | Notes | +| --- | --- | --- | +| `login` | `--no-browser` | skip opening the authorize URL; print it and paste the code back | +| `status` | — | auth mode, token expiry, scopes; also reports an importable Claude Code login | +| `logout` | — | deletes the stored tokens **and** un-pins the mode, reverting to your API key | + +`login` prints a disclosure before it does anything, and it means it: reaching +subscription inference requires presenting FreeCode to Anthropic **as Claude +Code** — its OAuth client id, its headers, its identity line. Anthropic reserves +that inference for its own surfaces and has acted against tools doing this, and +the account at risk is yours. The full stance, the three opt-ins, and what +happens when Anthropic refuses are on +[Anthropic subscription login](/getting-started/anthropic-subscription). + +The flow is PKCE against a `localhost` callback with a 120-second wait, falling +back to paste. It is a single process — the OAuth `state` **is** the PKCE +verifier, so there is no second command and no `--code` flag. Tokens go to +`~/.freecode/auth.json` at mode `0600`, separate from `config.json`, and refresh +automatically. + ## `freecode update` Runs `curl -fsSL https://freecode.website/install | bash` and exits with the @@ -290,25 +322,16 @@ is inside it rather than saying only "proceed". Nothing is backed up either way, so copy `~/.freecode/projects/` and `~/.freecode/sessions/` before purging if you might want them back. +It prints the list first and asks `y/N` unless `--force` is given; a +non-terminal stdin is an error telling you to pass `--force`, not a silent "no". +Exit code is `1` if any removal failed. These are the same semantics as +`curl -fsSL https://freecode.website/uninstall | bash`, which has always drawn +the line here. + ## Known gaps Found while writing this page; each is also tracked in `TODO.md`. -- **`freecode run` runs without hooks.** `HookSettingsManager` is constructed only - in `startServer()` (`server.ts:1106`), so a headless run never loads - `settings.json` hooks and never registers the built-in `rtk` hook. Permission - *rules* do apply (the loop builds its own `PermissionSettingsManager`), so the - same command behaves differently under `serve` and under `run` — the formatter - you rely on after every edit silently does not run in CI. -- **Headless `build` mode is a trap.** Every unanswerable "ask" resolves to deny - (`permission/prompt.ts:44`), and `build` defaults mutating tools to ask, so the - common first command produces a turn full of denials with no hint that a rule - would fix it. A `--yes`/`--allow ` flag, or a one-line explanation on the - first denial, would remove the whole class of confusion. -- **`--agent` is not validated.** `argv.agent as AgentMode` is an unchecked cast - (`run.ts:140`); `--agent buld` falls through `modeDefault`'s `default` branch and - silently runs with **build** semantics. yargs `choices` would catch it at parse - time, as `mcp add`'s `type` already does. - **`session` CLI covers 2 of 12 operations.** `fork`, `switch`, `archive`, `export`, `import`, `upload`, `download` all exist over IPC and none has a CLI surface, so scripting session management means speaking JSON-RPC by hand. diff --git a/apps/docs/app/reference/env/page.mdx b/apps/docs/app/reference/env/page.mdx index 276c47d7..a585eef6 100644 --- a/apps/docs/app/reference/env/page.mdx +++ b/apps/docs/app/reference/env/page.mdx @@ -42,6 +42,15 @@ things you might genuinely need — an API key and a model — are better kept i the app, exporting a different one changes nothing. A provider suffixed with `-coding-plan` falls back to its base provider's key and variable. +| Variable | Values | Effect | +| --- | --- | --- | +| `FREECODE_ANTHROPIC_AUTH` | `oauth`, `api-key` | pins how `anthropic` authenticates, overriding `providers.anthropic.authMode` | + +`oauth` bills a Claude Pro/Max subscription instead of a key, and setting it is +one of the three ways to opt in. It is not a neutral switch — see +[Anthropic subscription login](/getting-started/anthropic-subscription) for what +the OAuth path does and the risk it carries. + ## Provider requests | Variable | Default | Read | Effect | diff --git a/docs/superpowers/specs/2026-09-05-anthropic-oauth-provider.md b/docs/superpowers/specs/2026-09-05-anthropic-oauth-provider.md index 87ba0308..36d034c0 100644 --- a/docs/superpowers/specs/2026-09-05-anthropic-oauth-provider.md +++ b/docs/superpowers/specs/2026-09-05-anthropic-oauth-provider.md @@ -68,6 +68,28 @@ Consequences for the design: what this does, that it impersonates Claude Code, that Anthropic may block or action the account. No repeated nagging afterward. +### 0.2 Resolution — the published stance (2026-09-05) + +This section was the open risk item blocking v1 (`beforeStable.md` P0 #5). It is +resolved as **ship it, state the risk plainly, do not euphemize**: + +> FreeCode ships subscription auth as a real, supported, opt-in feature. It says +> in plain words that it presents itself to Anthropic as Claude Code, that +> Anthropic reserves that inference for its own surfaces and has acted against +> tools doing this, and that the account at risk is the user's. Whether the +> trade is worth it is the user's call, on the user's account. API key stays the +> default and is one `freecode auth logout` away. + +The three constraints above are what make that stance honest rather than a +disclaimer, and they stay invariants. + +What changed is that the stance is now **published**, not just implemented: a +`/getting-started/anthropic-subscription` docs page, a `freecode auth` section in +the CLI reference, `FREECODE_ANTHROPIC_AUTH` in the env reference, and a warning +callout in `README.md` beside the command itself. Before this, the only place a +user could read any of it was the paragraph `auth login` prints — after they had +already decided to run it. + ## 1. Motivation 1. The user pays for Claude Max. Freecode dev loops (evals excepted — see §8) burn From b069a5b60489788b98288ece7fe1092dce786010 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 16:52:42 +0530 Subject: [PATCH 06/16] docs: record the v1 release-readiness audit and its outcomes beforeStable.md is the 2026-09-05 audit against main at 360d1e5 (v0.29.0): health checks, the five P0 blockers, six P1s, what is explicitly not a blocker, and the path to 1.0. Each P0 now carries what was actually done and why, so the reasoning survives the branch. All five P0 items are closed. What remains before the tag is `pnpm eval:gate` with FREECODE_JUDGE_PROVIDER set. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- beforeStable.md | 245 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 beforeStable.md diff --git a/beforeStable.md b/beforeStable.md new file mode 100644 index 00000000..7f3dbb09 --- /dev/null +++ b/beforeStable.md @@ -0,0 +1,245 @@ +# Before Stable (v1.0) + +Release-readiness audit, 2026-09-05, against `main` at `360d1e5` (version `0.29.0`). + +## Health: green + +| Check | Result | +| -------------------- | --------------------------------------------------- | +| `pnpm check-types` | clean across all workspaces | +| `pnpm test` | **1206 pass / 0 fail**, 30 suites, ~30s | +| CI (`ci.yml`) | lint + typecheck + build + test on every PR | +| Eval gate (`v0.27.1`)| all three suites opened the gate | + +The engine is stable. What's left for v1 is **the front door and the headless +path**, not correctness. + +--- + +## P0 — real v1 blockers + +> **Status:** all five are done (2026-09-05). + +### 1. ~~`README.md` describes a different product~~ — DONE 2026-09-05 + +It pitches FreeCode as a tool that "drives AI coding assistants via browser +automation" using a "two-phase approach: the AI first returns which files it +needs, then receives those files". That is the legacy Playwright path that +`CLAUDE.md` explicitly marks as **not wired into the primary path**. The actual +product is a ~198-provider API agent loop with a single agentic tool-use loop. + +Anyone landing on the repo for v1 reads a stale pitch for a legacy subsystem. + +**Fixed.** `README.md` was rewritten against what ships: the real installer, all +16 tools, the command list, the four-frontend architecture, and the ~198-provider +driver. The browser path is kept as a blockquote noting it exists and is not the +default execution path. Three claims were corrected against source while writing +it — `freecode session` is list + delete only, `freecode memory` is graph +stats/rebuild/UI, and `freecode update` is injected by the TUI +(`apps/tui/src/entry.ts:194`), not registered in `create-cli.ts`. + +### 2. ~~Headless `freecode run` is broken in three ways~~ — DONE 2026-09-05 + +`apps/core/src/cli/commands/run.ts` + +- **No hooks.** `HookSettingsManager` + `registerRtkHook()` are constructed only + in `startServer()` (`server.ts:1260-1263`), so a headless run loads no + `settings.json` hooks. The formatter that fires after every edit interactively + silently does not fire in CI — same repo, two behaviours. Fix: move both into a + shared bootstrap the `run` handler also calls. +- **`build` mode denies every write.** `askPermission` rejects immediately when + no frontend is listening (`bus/index.ts:413`) and `promptForPermission` maps + that to deny, while `build`'s default for mutating tools is `ask`. So + `freecode run "fix the test"` reads fine and is denied every edit. Needs a + `--yes` / `--allow ` flag, or at minimum a one-time explanation on the + first headless denial. +- **`--agent` is an unchecked cast** (`run.ts:146`). `--agent buld` falls through + `modeDefault`'s `default` branch and runs with **build** semantics. Add yargs + `.choices()`, as `mcp add`'s `type` already has. + +If v1 is meant to be scriptable at all, these three are the difference between +"works" and "works interactively only". + +**Fixed.** + +- Hooks moved into `apps/core/src/hooks/bootstrap.ts` (`initHooks`), called by + both `startServer()` and the `run` handler. Only `serve` passes `watch: true`; + a one-shot run exits before a settings edit could apply, and would otherwise + hold an fs watcher open past its last turn. +- `--yes` (`-y`) answers the **ask** tier with allow, and repeatable + `--allow ` adds in-memory session grants. Both ride into the loop on new + `AgentLoopConfig.autoApproveAsks` / `.sessionGrants` fields. Scoped to the ask + tier deliberately: a deny rule and a read-only mode still refuse, because those + are decisions someone already made, not questions waiting for an answer. +- `--agent` gained yargs `.choices()`, so `--agent buld` exits with a usage error + instead of running as `build`. The cast at the read site stays — it matches + `mcp add`'s house style and is now backed by a real runtime check. + +Tests: `agent/headless-permission.test.ts` (5 cases — the first asserts the +unattended default is still deny, two cover `--yes` and `--allow` landing a real +write on disk, two assert `--yes` does not beat a deny rule or plan mode) and +`hooks/bootstrap.test.ts` (2). Full suite: **1213 pass / 0 fail**. + +Docs updated: `/reference/cli` (flag table, a three-option scripting section, +and the incorrect "there is no `--max-turns`" line), `/reference/settings`, +`/reference/hook-events`, and the three matching `TODO.md` entries. + +### 3. ~~`config.get` returns API keys verbatim~~ — DONE 2026-09-05 + +`server.ts` — `"config.get"` returned `readConfig()` with no redaction. The full +JSON-RPC surface is reachable over `web-server.ts`'s `POST /api`. It is +token-gated and loopback by default, but `host` is a parameter +(`web-server.ts:108`), so one `--host 0.0.0.0` turned a debug convenience into +key exfiltration. + +**Fixed.** `redactConfig()` in `providers/config.ts` builds the safe view and +`config.get` returns that: `{ hasApiKey, model?, authMode? }` per provider entry, +`{ hasCredential }` per `web` entry, with `current`, `lastAgentMode` and +`recovery` passed through. It is an **allowlist assembled field by field**, not a +blocklist of known secret names — `WebCredentials` has grown a secret-bearing +field twice, and a blocklist is wrong the day it grows a third. + +No consumer lost anything: the only reader of the raw shape was the TUI's +unused `getConfig()` wrapper, whose `ConfigInfo` type now matches. Every other +"is a provider set up" question already went through `providers.list`'s +`hasApiKey`, which never carried the key. + +Tests: `providers/config-redaction.test.ts` (4 — one asserts no secret from any +block survives serialization, one pins the exact kept shape, one that an +anonymous web session reads as `hasCredential: false` rather than a missing +block, one that an empty config stays empty). Full suite: **1217 pass / 0 fail**. + +Docs updated: `/reference/ipc-methods` (result column, the redaction paragraph +that replaced the "API keys included" warning, and the matching Known-gaps +bullet) and the `TODO.md` entry. + +### 4. ~~`freecode uninstall` deletes user data on one `y`~~ — DONE 2026-09-05 + +Removed all of `~/.freecode` — sessions, rollout logs, memory, history, usage — +and the prompt did not say so. No `--keep-data`, no backup. + +**Fixed.** `scripts/uninstall.sh` already drew the right line (`--purge` for a +full wipe, data kept otherwise); the CLI command was the copy that got it wrong, +so the fix was to make them agree rather than invent a third behaviour. The +default now takes the launcher and `~/.freecode/builds` — the program — and +leaves everything else. `--purge` takes the directory, and the line it prints +names what is in it rather than the path alone. `--dry-run` was added, and +`--force` gained `-y`/`--yes` so the two entry points accept the same flags. + +`--keep-data` was not added: keeping data is now the default, and a flag for the +default reads like the other behaviour is still lurking. + +Two smaller things fell out. A non-TTY stdin resolved `readline.question` at once +with an empty answer, which the old code read as "no" — correct by accident, and +silent; it is now an error naming `--force`. And the handler no longer branches +on `isDirectory()` before choosing `rmSync` vs `unlinkSync`, since `rmSync` with +`recursive` handles both. + +Tests: `cli/commands/uninstall.test.ts` (5, against a pure `planUninstall()` so +the safety rule is asserted without deleting anything — the default plan excludes +the data dir, `--purge` includes it *and* labels it with "sessions"/"memory", +purge does not double-list `builds`, an absent launcher is not listed, and an +uninstalled machine plans nothing in either mode). Full suite: **1222 pass / 0 +fail**. + +Docs updated: `/reference/cli` (flag table + rewritten section), the +`/getting-started/installation` uninstall block, and the `~/.freecode` note on +`/reference`. The **separate** gap that `uninstall` ignores `FREECODE_HOME` / +`FREECODE_INSTALL_DIR` and misses the Windows launcher path is untouched and +still tracked (`TODO.md:578`); it is a completeness bug, not a data-loss one. + +### 5. ~~Anthropic OAuth ToS risk is unresolved~~ — DONE 2026-09-05 + +§0.1 of the OAuth spec was the risk section and had no resolution. A v1 that +ships subscription auth as a headline feature needs an explicit, user-visible +stance — not a spec paragraph. + +**Resolved: ship it, state the risk plainly, do not euphemize.** Written into +the spec as §0.2 and, more importantly, published: + +- **`/getting-started/anthropic-subscription`** — a new page whose second + section is the risk, before any instructions. It says FreeCode presents itself + to Anthropic *as Claude Code*, that Anthropic reserves subscription inference + for its own surfaces and has acted against tools doing this, and that the + account at risk is the user's. Then: the three opt-ins and the narrow fourth + (no key configured + a freecode login — an imported Claude Code login + deliberately does not count), the login flow, `status`/`logout`, the + org-forbidden 403 latch and API-key fallback, and why cost is stamped on the + call. +- **`/reference/cli`** gained a `freecode auth` section — the command was + undocumented entirely. +- **`/reference/env`** gained `FREECODE_ANTHROPIC_AUTH`, flagged as an opt-in + rather than a neutral switch. +- **`README.md`** — a warning callout beside `freecode auth login anthropic`, + and the pitch line up top now says opt-in and links down to it. + +The code needed nothing: `auth.ts` already printed the §0.1 disclosure on login +and the identity block was already quarantined to the OAuth path with a test. +The gap was that a user could only read any of this *after* deciding to run the +command. The three §0.1 constraints (opt-in never default, identity block +quarantined, first-run disclosure) stay invariants. + +Not adopted: a y/N confirmation on login. §0.1 chose "print once, no repeated +nagging" deliberately, and a prompt in front of a command the user just typed by +name buys nothing the paragraph above it does not already say. + +--- + +## P1 — should fix, will not sink the release + +- **SSE replay lies after a full disconnect.** `tearDownIfEmpty` + (`web/stream-subscribers.ts:265`) disposes the ring buffer when the last + subscriber leaves — the normal single-browser case. A reconnect carrying + `Last-Event-ID` then hits `if (!rec) return { gap: false, events: [] }` + (`:175`) and is told nothing was missed, losing every event produced while + away. Not even a `stream_gap` marker. Strictly worse than eviction, which at + least reports a gap. Fix: decouple record lifetime from subscriber count (TTL + after the last leaves). +- **Compaction summarizes the original task away.** `selectForCompaction` + preserves only the tail and takes `messages.slice(0, firstPreservedIndex)` for + the summary (`compaction/selector.ts:54`). No head carve-out, so the founding + instruction is compacted first and, on the next compaction, the summary of it + is re-summarized. Lossy compounding on the oldest content — the plausible + mechanism behind long-session drift off the brief. +- **Compaction summaries never see tool activity.** `MemoryService` records only + user prompts and assistant text; a tool-calling turn is stored as the stub + `[Executed N tools]` (`loop.ts:1567`). The transcript handed to the summarizer + contains none of the edits, commands, or errors that were the actual work. + Also makes two heuristic-summarizer paths dead code + (`summarizer.ts:106`, `service.ts:63`). +- **`METHODS` declares 24 of 49 implemented IPC methods**, and + `METHODS["session.send"]` declares the wrong result type. `CLAUDE.md` calls + that map the source of truth; it is not one yet. All of `memory.*`, `config.*`, + `models.*` and 8 session ops get zero compile-time checking in frontends. +- **No `-32602` invalid-params validation.** Every handler does `params as {…}` + with no runtime check, so a missing or mistyped field becomes `undefined` deep + inside and surfaces as a confusing `-32603`. +- **No JSON Schema for `settings.json`, and unknown keys are silently ignored.** + `"permission"` for `"permissions"` is indistinguishable from the outside from + the feature being broken — in a security-relevant, hand-edited file. + +--- + +## Explicitly not blockers + +These are v1.1 features, not "v1 is broken without them" — **provided the README +and docs do not promise them**: + +- `autonomous/` — Phase 0 only (types, budget, storage). Nothing executes. +- MCP server (expose). The client side is done. +- Checkpoints / rewind. +- User-defined slash commands and subagents. +- Multimodal input beyond what already landed. + +--- + +## Suggested path to 1.0 + +1. ~~Rewrite `README.md` against what actually ships.~~ +2. ~~Fix the three `run.ts` issues — shared hook bootstrap, `--yes`, `.choices()`.~~ +3. ~~Redact `config.get`~~; ~~make `uninstall` keep data by default~~. +4. ~~Write the OAuth ToS stance into user-facing docs.~~ +5. `pnpm eval:gate` (needs `FREECODE_JUDGE_PROVIDER`), then tag. + +**Steps 1–4 are done (2026-09-05).** What remains before the tag is step 5: +`pnpm eval:gate` with `FREECODE_JUDGE_PROVIDER` set, then tag. From 1f961ff161eef12bccc00ef93ac36e502c27a429 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:06:56 +0530 Subject: [PATCH 07/16] fix(web): keep the SSE replay buffer alive past the last subscriber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tearDownIfEmpty` disposed a session's ring buffer the moment its last subscriber left — which, for a single browser, is every disconnect. A reconnect carrying Last-Event-ID then found no record and was told `{ gap: false, events: [] }`: nothing was missed. Everything produced while away was lost, without even a `stream_gap` marker. Strictly worse than eviction, which at least admits the loss. Record lifetime is now decoupled from subscriber count. The last leave starts a 5-minute TTL (`emptySince`) that the existing reaper enforces; a reconnect inside the window clears it and replays for real. Events emitted while nobody is attached keep being buffered, as they already were. `replayForSubscriber` also stops lying when the record is genuinely gone: a client that claims to have seen events but finds no buffer now gets a gap, since we cannot know what it missed. Tests: 4 in `web/stream-subscribers.test.ts` — replay across a full disconnect, a gap once reaped, survival of a partial disconnect, and a reconnect clearing the TTL. `runReaperForTests(now?)` is the seam; a five-minute TTL is not assertable against a 15s unref'd interval. --- apps/core/src/web/stream-subscribers.test.ts | 74 ++++++++++++++++++++ apps/core/src/web/stream-subscribers.ts | 69 ++++++++++++++---- 2 files changed, 131 insertions(+), 12 deletions(-) diff --git a/apps/core/src/web/stream-subscribers.test.ts b/apps/core/src/web/stream-subscribers.test.ts index 4e7f794a..62ac5616 100644 --- a/apps/core/src/web/stream-subscribers.test.ts +++ b/apps/core/src/web/stream-subscribers.test.ts @@ -16,6 +16,9 @@ import { publishToAll, subscriberCount, disposeSession, + replayForSubscriber, + currentSeq, + runReaperForTests, STREAM_TIMINGS, } from "./stream-subscribers.js"; @@ -177,10 +180,81 @@ describe("web/stream-subscribers", () => { }); }); + // The P1 bug this covers: the record was disposed the moment the last + // subscriber left, which for a single browser is every disconnect. The + // reconnect then replayed "nothing was missed" and lost the whole gap. + describe("reconnect after the last subscriber leaves", () => { + it("replays events produced while nobody was attached", () => { + const first = makeFakeRes(); + addSubscriber("s-C", makeFakeReq(), first.res); + publishToSession("s-C", { type: "text", content: "before" }); + const seen = currentSeq("s-C"); + + // The only browser goes away. + first.destroy(); + first.res.emit("close"); + assert.equal(subscriberCount("s-C"), 0); + + // Work continues while disconnected. + publishToSession("s-C", { type: "text", content: "while away" }); + + const replay = replayForSubscriber("s-C", seen); + assert.equal(replay.gap, false); + assert.equal(replay.gap === false && replay.events.length, 1); + assert.match(JSON.stringify(replay), /while away/); + }); + + it("reports a gap — never 'nothing missed' — once the record is reaped", () => { + const sub = makeFakeRes(); + addSubscriber("s-D", makeFakeReq(), sub.res); + publishToSession("s-D", { type: "text", content: "before" }); + sub.destroy(); + sub.res.emit("close"); + + // Run one reaper pass at a clock past the record's TTL. + runReaperForTests(Date.now() + STREAM_TIMINGS.RECORD_TTL_MS + 1); + + const replay = replayForSubscriber("s-D", 1); + assert.equal(replay.gap, true, "a vanished buffer must report a gap"); + }); + + it("keeps the record alive while a subscriber is still attached", () => { + const a = makeFakeRes(); + const b = makeFakeRes(); + addSubscriber("s-E", makeFakeReq(), a.res); + addSubscriber("s-E", makeFakeReq(), b.res); + publishToSession("s-E", { type: "text", content: "x" }); + + a.destroy(); + a.res.emit("close"); + runReaperForTests(); + + assert.equal(subscriberCount("s-E"), 1); + assert.equal(currentSeq("s-E"), 1, "buffer survives a partial disconnect"); + }); + + it("a reconnect inside the window clears the TTL, so the record survives", () => { + const a = makeFakeRes(); + addSubscriber("s-F", makeFakeReq(), a.res); + publishToSession("s-F", { type: "text", content: "x" }); + a.destroy(); + a.res.emit("close"); + + // Reconnect, then age the clock as far as the reaper can see. + const b = makeFakeRes(); + addSubscriber("s-F", makeFakeReq(), b.res); + runReaperForTests(); + + assert.equal(subscriberCount("s-F"), 1); + assert.equal(currentSeq("s-F"), 1); + }); + }); + describe("timings", () => { it("exports heartbeat and idle constants", () => { assert.ok(STREAM_TIMINGS.HEARTBEAT_MS > 0); assert.ok(STREAM_TIMINGS.IDLE_TIMEOUT_MS > STREAM_TIMINGS.HEARTBEAT_MS); + assert.ok(STREAM_TIMINGS.RECORD_TTL_MS > STREAM_TIMINGS.IDLE_TIMEOUT_MS); }); }); }); diff --git a/apps/core/src/web/stream-subscribers.ts b/apps/core/src/web/stream-subscribers.ts index 86281ad8..fd8041cc 100644 --- a/apps/core/src/web/stream-subscribers.ts +++ b/apps/core/src/web/stream-subscribers.ts @@ -3,9 +3,13 @@ // // Owns the multi-subscriber fan-out for /events (spec §4.3) and the // resumable ring buffer (spec §4.2). Each session has a record with a -// Set and a StreamBuffer; both share lifetime and tear down -// together so a session can never retain a buffer with no subscribers or -// vice versa. +// Set and a StreamBuffer. +// +// The buffer OUTLIVES the last subscriber by RECORD_TTL_MS. Disposing on +// the last leave is tempting — it keeps the two structures' lifetimes +// identical — but it breaks the one case resumability exists for: a single +// browser disconnecting is the last subscriber leaving, so the buffer was +// always gone by the time it reconnected with Last-Event-ID. // // Liveness is established positively rather than inferred from silence: // - Periodic heartbeat (HEARTBEAT_MS) — a `: heartbeat` comment frame. @@ -38,10 +42,25 @@ interface Subscriber { interface SessionRecord { subscribers: Set; buffer: StreamBuffer; + /** + * When the last subscriber left, or undefined while at least one is + * attached. The record — and with it the replay buffer — is kept for + * RECORD_TTL_MS after this so a reconnect can actually replay the gap. + * Disposing on the last leave made the single-browser case (the normal + * one) replay as "nothing was missed". + */ + emptySince?: number; } const HEARTBEAT_MS = 15_000; const IDLE_TIMEOUT_MS = 60_000; +/** + * How long a subscriber-less record (and its buffer) survives. Long enough + * to cover a laptop lid-close or a tab suspend; short enough that an + * abandoned session does not pin its buffer forever. Events emitted during + * the window are still buffered, so the reconnect replays them. + */ +const RECORD_TTL_MS = 5 * 60_000; const sessions = new Map(); @@ -86,6 +105,7 @@ export function addSubscriber( }; rec.subscribers.add(sub); + rec.emptySince = undefined; // Bind close/error on both directions. Either side closing means the // socket is gone for our purposes. We bind BEFORE writing anything because @@ -105,7 +125,7 @@ export function removeSubscriber(sessionId: string, sub: Subscriber): void { const rec = sessions.get(sessionId); if (!rec) return; rec.subscribers.delete(sub); - tearDownIfEmpty(sessionId); + markEmpty(sessionId); } /** @@ -172,10 +192,14 @@ export function replayForSubscriber( afterSeq: number | undefined, ): ReplayResult { const rec = sessions.get(sessionId); - if (!rec) return { gap: false, from: 0, to: 0, events: [] }; if (afterSeq === undefined || afterSeq < 0) { return { gap: false, from: 0, to: 0, events: [] }; } + // No record, but the client claims to have seen events: the buffer aged + // out (or the daemon restarted). We cannot know what was missed, so the + // only honest answer is a gap. Reporting "nothing missed" here is worse + // than eviction, which at least admits the loss. + if (!rec) return { gap: true, from: afterSeq + 1, to: afterSeq + 1 }; return rec.buffer.replayFrom(afterSeq); } @@ -262,12 +286,16 @@ function getOrCreateSession(sessionId: string): SessionRecord { return rec; } -function tearDownIfEmpty(sessionId: string): void { +/** + * The last subscriber left. Start the TTL rather than disposing: the buffer + * is exactly what a reconnect needs, and the common single-browser case + * always passes through here. The reaper does the actual disposal. + */ +function markEmpty(sessionId: string): void { const rec = sessions.get(sessionId); if (!rec) return; - if (rec.subscribers.size === 0) { - rec.buffer.dispose(); - sessions.delete(sessionId); + if (rec.subscribers.size === 0 && rec.emptySince === undefined) { + rec.emptySince = Date.now(); } } @@ -281,11 +309,18 @@ function tearDownIfEmpty(sessionId: string): void { const HEARTBEAT_FRAME = ": heartbeat\n\n"; -function tick(): void { - const now = Date.now(); +function tick(now: number = Date.now()): void { for (const sessionId of [...sessions.keys()]) { const rec = sessions.get(sessionId); if (!rec) continue; + if ( + rec.subscribers.size === 0 && + rec.emptySince !== undefined && + now - rec.emptySince > RECORD_TTL_MS + ) { + disposeSession(sessionId); + continue; + } for (const sub of [...rec.subscribers]) { // Idle reaper — backstop for half-open sockets that never fire close. if (now - sub.lastWriteMs > IDLE_TIMEOUT_MS) { @@ -316,4 +351,14 @@ reaper.unref?.(); export const STREAM_TIMINGS = Object.freeze({ HEARTBEAT_MS, IDLE_TIMEOUT_MS, -}); \ No newline at end of file + RECORD_TTL_MS, +}); + +/** + * Test seam: run one reaper pass synchronously, optionally at a simulated + * clock. The interval is unref'd and fires every HEARTBEAT_MS, which is far + * too slow to assert a five-minute TTL. + */ +export function runReaperForTests(now?: number): void { + tick(now); +} \ No newline at end of file From fd963a1bb94897b775c7d06a0685ee4702e0001d Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:07:09 +0530 Subject: [PATCH 08/16] fix(compaction): preserve the founding brief; record what the tools did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two failures that compounded in the same place — the summary that replaces most of a long session. **The brief was compacted first.** `selectForCompaction` preserved only the tail and summarized `messages.slice(0, firstPreservedIndex)`, so the oldest content went first and the first user message went with it. On the next compaction the *summary of it* was summarized again: the founding instruction decayed faster than anything else in the window, which is the plausible mechanism behind long-session drift off the task. The first user message is now carved out and preserved verbatim, after tail-trimming so trimming can never evict it. A head over `maxPreserveHeadTokens` (2k) is a pasted document rather than an instruction and is summarized as before. **The summary never saw the work.** A tool-calling turn was recorded as the stub `[Executed N tools]`, so the transcript handed to the summarizer contained none of the edits, commands or errors that were the actual work — and `summarizer.ts`'s `extractToolCalls`/`extractFiles` matched nothing, two paths that could not fire. `compaction/tool-transcript.ts` renders one bounded line per call (`Tool : -> `, in the format the summarizer already greps for), and `addToolTurn` records it. The turn budget moved with it. `MemoryService.normalizeContent` is gone: it clipped the *tail* of a long message, i.e. the most recent tools. The transcript drops the oldest calls instead and says how many, which is the right end to lose. `maxToolOutputChars` is now that turn budget (4k). Tests: 6 in `tool-transcript.test.ts` (incl. one pinning the format against the summarizer's own regexes), 2 in `selector.test.ts` (the brief survives two compactions; an oversized head does not), and the two existing service/pruning tests rewritten to the new invariants. 1234 pass / 0 fail. --- apps/core/src/agent/loop.ts | 15 ++- apps/core/src/compaction/pruning.test.ts | 24 +++-- apps/core/src/compaction/selector.test.ts | 40 +++++++- apps/core/src/compaction/selector.ts | 16 ++- apps/core/src/compaction/service.test.ts | 15 ++- apps/core/src/compaction/service.ts | 30 ++++-- .../src/compaction/tool-transcript.test.ts | 64 ++++++++++++ apps/core/src/compaction/tool-transcript.ts | 99 +++++++++++++++++++ apps/core/src/compaction/types.ts | 14 ++- 9 files changed, 291 insertions(+), 26 deletions(-) create mode 100644 apps/core/src/compaction/tool-transcript.test.ts create mode 100644 apps/core/src/compaction/tool-transcript.ts diff --git a/apps/core/src/agent/loop.ts b/apps/core/src/agent/loop.ts index 460c1c67..3d561af6 100644 --- a/apps/core/src/agent/loop.ts +++ b/apps/core/src/agent/loop.ts @@ -1789,10 +1789,17 @@ export class AgentLoop { await this.appendUserMessage(caption, toolImages); } - // Add assistant response to MemoryService for token tracking - this.memory.addMessage( - "assistant", - providerResult.content || `[Executed ${toolCalls.length} tools]`, + // Record the turn for compaction. The transcript carries what the tools + // actually did — the stub this replaced ("[Executed N tools]") meant a + // summary could describe a coding session without a single edit in it. + this.memory.addToolTurn( + providerResult.content, + toolCalls.map((tc, i) => ({ + tool: tc.tool, + args: tc.args, + output: toolResults[i]?.modelOutput, + error: toolResults[i]?.error, + })), ); await this.maybeCompact(provider, model); diff --git a/apps/core/src/compaction/pruning.test.ts b/apps/core/src/compaction/pruning.test.ts index 2e2fb403..a3eccf7c 100644 --- a/apps/core/src/compaction/pruning.test.ts +++ b/apps/core/src/compaction/pruning.test.ts @@ -2,16 +2,28 @@ import test from "node:test"; import assert from "node:assert/strict"; import { MemoryService } from "./service.js"; -test("assistant tool-like output is capped before storage", () => { +// The bound moved from MemoryService.normalizeContent (which clipped the +// tail — i.e. the most recent tools) into the transcript renderer, which +// drops the oldest calls instead. What is capped is a turn, not a message. +test("a tool turn is capped before storage, keeping the newest calls", () => { // Use unique session to avoid loading stale state from previous test runs const sessionId = `session-prune-${Date.now()}`; const service = new MemoryService(sessionId, { - config: { maxToolOutputChars: 20 }, + config: { maxToolOutputChars: 300 }, }); - service.addMessage("assistant", `Tool read: ${"x".repeat(100)}`); - const context = service.getPromptContext(); + service.addToolTurn( + "", + Array.from({ length: 20 }, (_, i) => ({ + tool: "read", + args: { filePath: `src/f${i}.ts` }, + output: "x".repeat(100), + })), + ); + const content = service.getPromptContext().recentMessages[0].content; - assert.ok(context.recentMessages[0].content.length < 80); - assert.match(context.recentMessages[0].content, /truncated/); + assert.ok(content.length < 600, `bounded, got ${content.length}`); + assert.match(content, /earlier tool calls omitted/); + assert.match(content, /f19\.ts/, "newest call survives"); + assert.doesNotMatch(content, /f0\.ts/); }); diff --git a/apps/core/src/compaction/selector.test.ts b/apps/core/src/compaction/selector.test.ts index f93fdd4c..6c83225c 100644 --- a/apps/core/src/compaction/selector.test.ts +++ b/apps/core/src/compaction/selector.test.ts @@ -28,16 +28,52 @@ test("selectForCompaction preserves the recent tail and summarizes older message const result = selectForCompaction(messages, DEFAULT_COMPACTION_CONFIG); + // "1" is the founding user instruction and is carved out of the summary. assert.deepEqual( result.summarize.map((item) => item.id), - ["1", "2"], + ["2"], ); assert.deepEqual( result.preserve.map((item) => item.id), - ["3", "4", "5", "6"], + ["1", "3", "4", "5", "6"], ); }); +test("selectForCompaction never summarizes the founding instruction away", () => { + const messages = [ + msg("1", "user", "build a parser for TOML"), + ...Array.from({ length: 20 }, (_, i) => + msg(String(i + 2), i % 2 === 0 ? "assistant" : "user", `chatter ${i}`), + ), + ]; + + // Compact twice: the second pass is where the old code re-summarized the + // summary of the brief. + const first = selectForCompaction(messages, DEFAULT_COMPACTION_CONFIG); + assert.equal(first.preserve[0].id, "1"); + assert.ok(!first.summarize.some((m) => m.id === "1")); + + const second = selectForCompaction(first.preserve, DEFAULT_COMPACTION_CONFIG); + assert.equal(second.preserve[0].id, "1"); + assert.ok(!second.summarize.some((m) => m.id === "1")); +}); + +test("a head too large to be a brief is summarized like anything else", () => { + const messages = [ + msg("1", "user", "a pasted 50k spec", 50_000), + msg("2", "assistant", "ack"), + msg("3", "user", "middle"), + msg("4", "assistant", "middle answer"), + msg("5", "user", "latest"), + msg("6", "assistant", "latest answer"), + ]; + + const result = selectForCompaction(messages, DEFAULT_COMPACTION_CONFIG); + + assert.ok(result.summarize.some((m) => m.id === "1")); + assert.equal(result.preserve[0].id, "3"); +}); + test("selectForCompaction returns no summarize set when history is too short", () => { const messages = [ msg("1", "user", "latest"), diff --git a/apps/core/src/compaction/selector.ts b/apps/core/src/compaction/selector.ts index 00afd3a4..710f4522 100644 --- a/apps/core/src/compaction/selector.ts +++ b/apps/core/src/compaction/selector.ts @@ -51,7 +51,21 @@ export function selectForCompaction( const firstPreservedIndex = firstPreservedId ? messages.findIndex((message) => message.id === firstPreservedId) : messages.length; - const summarize = messages.slice(0, Math.max(0, firstPreservedIndex)); + let summarize = messages.slice(0, Math.max(0, firstPreservedIndex)); + + // Head carve-out. The founding instruction is the first thing compacted + // away otherwise, and on the *next* compaction the summary of it is + // summarized again — the brief decays faster than anything else in the + // window, which is the plausible mechanism behind long-session drift off + // the task. Keeping it verbatim costs a bounded, one-off few hundred + // tokens. It is prepended after the tail-trimming loop above deliberately: + // trimming must never be able to evict the brief. + const headIndex = summarize.findIndex((message) => message.role === "user"); + const head = headIndex === -1 ? undefined : summarize[headIndex]; + if (head && head.tokenCount <= config.maxPreserveHeadTokens) { + summarize = summarize.filter((message) => message.id !== head.id); + preserve = [head, ...preserve]; + } return { summarize, diff --git a/apps/core/src/compaction/service.test.ts b/apps/core/src/compaction/service.test.ts index 598f0460..c73c9664 100644 --- a/apps/core/src/compaction/service.test.ts +++ b/apps/core/src/compaction/service.test.ts @@ -17,6 +17,8 @@ test("MemoryService compacts old messages and exposes prompt context", async () service.addMessage("user", "old request in docs/superpowers/plans/x.md"); service.addMessage("assistant", "old answer"); + service.addMessage("user", "second request"); + service.addMessage("assistant", "second answer"); service.addMessage("user", "middle request"); service.addMessage("assistant", "middle answer"); service.addMessage("user", "latest request"); @@ -26,10 +28,19 @@ test("MemoryService compacts old messages and exposes prompt context", async () const context = service.getPromptContext(); assert.equal(result.success, true); - assert.ok(context.summary?.includes("old request")); + // The founding instruction is preserved verbatim at the head, not folded + // into the summary — otherwise the next compaction summarizes the summary + // of it, and the brief decays fastest of anything in the window. + assert.ok(context.summary?.includes("second request")); assert.deepEqual( context.recentMessages.map((message) => message.content), - ["middle request", "middle answer", "latest request", "latest answer"], + [ + "old request in docs/superpowers/plans/x.md", + "middle request", + "middle answer", + "latest request", + "latest answer", + ], ); } finally { rmSync(dir, { recursive: true, force: true }); diff --git a/apps/core/src/compaction/service.ts b/apps/core/src/compaction/service.ts index 2e481402..70683117 100644 --- a/apps/core/src/compaction/service.ts +++ b/apps/core/src/compaction/service.ts @@ -12,6 +12,7 @@ import { import { estimateTokenCount, shouldCompact } from "./tokens.js"; import { selectForCompaction } from "./selector.js"; import { makeSummary, summarizeMessages } from "./summarizer.js"; +import { renderTurnForMemory, type ToolActivity } from "./tool-transcript.js"; import type { LlmSummarize } from "./llm-summarizer.js"; import { FileMemoryStorage, type MemoryStorage } from "./storage.js"; import { logger } from "../utils/logger.js"; @@ -58,21 +59,13 @@ export class MemoryService { }; } - private normalizeContent(role: MemoryRole, content: string): string { - if (role !== "assistant") return content; - if (!content.startsWith("Tool ")) return content; - if (content.length <= this.config.maxToolOutputChars) return content; - return `${content.slice(0, this.config.maxToolOutputChars)}\n[tool output truncated for memory]`; - } - addMessage(role: MemoryRole, content: string): MemoryMessage { - const normalizedContent = this.normalizeContent(role, content); const message: MemoryMessage = { id: `msg-${Date.now()}-${Math.random().toString(36).slice(2)}`, role, - content: normalizedContent, + content, timestamp: Date.now(), - tokenCount: estimateTokenCount(normalizedContent), + tokenCount: estimateTokenCount(content), }; this.state.messages.push(message); this.state.tokenCount += message.tokenCount; @@ -80,6 +73,23 @@ export class MemoryService { return message; } + /** + * Record a turn that called tools. The transcript — one bounded line per + * call, with its arguments and outcome — replaces the "[Executed N tools]" + * stub that used to stand in for it. That stub was why a compaction summary + * contained none of the edits, commands or errors that were the actual work. + */ + addToolTurn(assistantText: string, activity: ToolActivity[]): MemoryMessage { + return this.addMessage( + "assistant", + renderTurnForMemory( + assistantText, + activity, + this.config.maxToolOutputChars, + ), + ); + } + // `contextLimit` comes from models.dev (getModelContextLimit) when available; // omit it to fall back to the local model table in tokens.ts. // diff --git a/apps/core/src/compaction/tool-transcript.test.ts b/apps/core/src/compaction/tool-transcript.test.ts new file mode 100644 index 00000000..25025c17 --- /dev/null +++ b/apps/core/src/compaction/tool-transcript.test.ts @@ -0,0 +1,64 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + renderToolActivity, + renderTurnForMemory, +} from "./tool-transcript.js"; + +test("renders one line per tool call with args and outcome", () => { + const out = renderToolActivity( + [ + { tool: "read", args: { filePath: "src/a.ts" }, output: "export const a = 1;" }, + { tool: "bash", args: { command: "pnpm test" }, error: "exit 1" }, + ], + 4_000, + ); + + assert.match(out, /Tool read: \{"filePath":"src\/a\.ts"\} -> export const a = 1;/); + assert.match(out, /Tool bash: .* -> failed: exit 1/); +}); + +// The summarizer mines these two things out of the transcript; if the format +// drifts, extractToolCalls and extractFiles quietly match nothing again. +test("stays in the format summarizer.ts greps for", () => { + const out = renderToolActivity( + [{ tool: "edit", args: { filePath: "apps/core/src/x.ts" }, output: "ok" }], + 4_000, + ); + assert.ok(/Tool (\w+):/.exec(out), "extractToolCalls must match"); + assert.ok(/(?:apps|packages|docs)\/[^\s)`'"]+/.exec(out), "extractFiles must match"); +}); + +test("drops the oldest lines, not the newest, when over budget", () => { + const activity = Array.from({ length: 40 }, (_, i) => ({ + tool: "read", + args: { filePath: `src/f${i}.ts` }, + output: "x".repeat(300), + })); + + const out = renderToolActivity(activity, 1_000); + + assert.ok(out.length <= 1_200, `budget respected, got ${out.length}`); + assert.match(out, /^\[\d+ earlier tool calls omitted\]/); + assert.match(out, /f39\.ts/, "the most recent call must survive"); + assert.doesNotMatch(out, /f0\.ts/); +}); + +test("a successful call with no output still reports that it ran", () => { + assert.match(renderToolActivity([{ tool: "write" }], 4_000), /Tool write: -> ok/); +}); + +test("renderTurnForMemory keeps the assistant text above the transcript", () => { + const out = renderTurnForMemory( + "Fixing the failing test.", + [{ tool: "edit", args: { filePath: "a.ts" }, output: "ok" }], + 4_000, + ); + assert.equal(out.split("\n")[0], "Fixing the failing test."); + assert.match(out, /Tool edit:/); +}); + +test("renderTurnForMemory with no text is the transcript alone", () => { + const out = renderTurnForMemory("", [{ tool: "ls", output: "a b" }], 4_000); + assert.equal(out, "Tool ls: -> a b"); +}); diff --git a/apps/core/src/compaction/tool-transcript.ts b/apps/core/src/compaction/tool-transcript.ts new file mode 100644 index 00000000..84b931c6 --- /dev/null +++ b/apps/core/src/compaction/tool-transcript.ts @@ -0,0 +1,99 @@ +// ============================================================================= +// Renders a turn's tool activity into the line format the compaction +// transcript and the heuristic summarizer already expect: `Tool : …`. +// +// Before this, a tool-calling turn was recorded as the stub +// "[Executed N tools]" (agent/loop.ts). Every edit, command and error — the +// actual work of a coding session — was invisible to the summarizer, so the +// summary that replaced a hundred messages described only what was said +// about the work, never the work. It also left `extractToolCalls` and +// `extractFiles` in summarizer.ts matching nothing: two code paths that +// could not fire. +// +// Output is bounded twice: per tool call, and per turn. When the turn budget +// is exceeded the OLDEST lines go, not the newest — the tools that ran last +// are the ones the next turn needs. This module is the sole owner of that +// bound, which is why MemoryService no longer carries its own tool-output +// truncation: two truncators would have fought, and the outer one clipped +// the tail (the recent tools) rather than the head. +// ============================================================================= + +/** Chars kept from each tool's result. Enough for an error or a diff stat. */ +const MAX_RESULT_CHARS = 200; +/** Chars kept from each tool's arguments — a path or a command, not a file. */ +const MAX_ARGS_CHARS = 200; + +export interface ToolActivity { + tool: string; + args?: unknown; + /** The model-facing output, already capped by the orchestrator. */ + output?: string; + /** Set when the call failed; reported in place of the output. */ + error?: string; +} + +function clip(text: string, maxChars: number): string { + const oneLine = text.replace(/\s+/g, " ").trim(); + return oneLine.length <= maxChars + ? oneLine + : `${oneLine.slice(0, maxChars)}…`; +} + +function renderArgs(args: unknown): string { + if (args === undefined || args === null) return ""; + try { + return clip(JSON.stringify(args), MAX_ARGS_CHARS); + } catch { + return ""; + } +} + +/** + * One line per tool call. Failures are marked so the summarizer's blocker + * bucket can find them — an error is the single most important thing to + * carry across a compaction boundary. + */ +export function renderToolActivity( + activity: ToolActivity[], + maxChars: number, +): string { + const lines = activity + .map((entry) => { + const args = renderArgs(entry.args); + const head = args ? `Tool ${entry.tool}: ${args}` : `Tool ${entry.tool}:`; + if (entry.error) return `${head} -> failed: ${clip(entry.error, MAX_RESULT_CHARS)}`; + if (!entry.output) return `${head} -> ok`; + return `${head} -> ${clip(entry.output, MAX_RESULT_CHARS)}`; + }); + + let total = lines.reduce((sum, line) => sum + line.length + 1, 0); + let dropped = 0; + while (total > maxChars && lines.length > 1) { + total -= lines.shift()!.length + 1; + dropped++; + } + if (dropped > 0) { + lines.unshift(`[${dropped} earlier tool calls omitted]`); + } + return lines.join("\n"); +} + +/** + * The assistant memory entry for a turn: the model's own text (if any) + * followed by what its tools actually did. Never the old stub. + */ +export function renderTurnForMemory( + assistantText: string, + activity: ToolActivity[], + maxChars: number, +): string { + const tools = renderToolActivity(activity, maxChars); + if (!assistantText) return tools; + if (!tools) return assistantText; + return `${assistantText}\n${tools}`; +} + +export const TOOL_TRANSCRIPT_LIMITS = Object.freeze({ + MAX_RESULT_CHARS, + MAX_ARGS_CHARS, +}); diff --git a/apps/core/src/compaction/types.ts b/apps/core/src/compaction/types.ts index 817db457..026f4a68 100644 --- a/apps/core/src/compaction/types.ts +++ b/apps/core/src/compaction/types.ts @@ -30,7 +30,18 @@ export interface CompactionConfig { autoCompactBufferTokens: number; preserveRecentTurns: number; maxPreserveRecentTokens: number; + /** + * Total chars of tool transcript kept for one turn (tool-transcript.ts). + * Sized for a turn, not a single output: at ~450 chars per tool line this + * carries roughly a dozen calls, and drops the oldest first when it can't. + */ maxToolOutputChars: number; + /** + * Ceiling on the first user message kept verbatim as the founding brief + * (see selectForCompaction). A head larger than this is a pasted document, + * not an instruction, and is summarized like anything else. + */ + maxPreserveHeadTokens: number; } export interface SelectionResult { @@ -61,5 +72,6 @@ export const DEFAULT_COMPACTION_CONFIG: CompactionConfig = { autoCompactBufferTokens: 13_000, preserveRecentTurns: 2, maxPreserveRecentTokens: 8_000, - maxToolOutputChars: 2_000, + maxToolOutputChars: 4_000, + maxPreserveHeadTokens: 2_000, }; From 7f0f43300a3666605d9b149b0bf7b53ed847c361 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:13:47 +0530 Subject: [PATCH 09/16] feat(ipc): declare the whole IPC surface in METHODS, and enforce it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CLAUDE.md` calls `METHODS` the source of truth for the JSON-RPC surface. It declared 25 of the 50 implemented handlers, so all of `memory.*`, `config.*`, `models.*` and eight session ops had zero compile-time checking in frontends — a wrong param shape was a runtime discovery. `METHODS["session.send"]` also declared `StreamResponse` as its result, which it has never returned: the handler resolves with the loop's result, and per-token output goes over the stream channel. Its params were missing `model`, `effort` and `agentMode` too. All 25 are now declared, `session.send` is corrected, and `ipc/methods-coverage.test.ts` asserts the two sets are equal in both directions — adding a handler without declaring it fails the suite, which is what makes the source-of-truth claim true rather than aspirational. Six result shapes needed wire types, since `packages/shared` cannot import from an app: `MemoryEntry`/`MemoryType`, `MemoryGraphStats`, `RedactedConfig`, `TurnResult`, `ExportedSession` and `ModelInfo`. They follow the existing `SessionMeta`/`SerializedMessage` mirror pattern, and `ipc/wire-shapes.test.ts` pins each one with a type-level assignability assertion so drift is a `check-types` failure rather than a field that silently stops reaching frontends. Two deliberate narrowings: `TurnResult` omits `LoopResult.finalState` (the loop's internal state machine — no frontend reads it), and there is no wire type for the raw config, only the redacted one. 1237 pass / 0 fail; check-types clean across all workspaces. --- apps/core/src/ipc/methods-coverage.test.ts | 37 +++++ apps/core/src/ipc/wire-shapes.test.ts | 62 ++++++++ apps/core/src/server.ts | 2 +- packages/shared/src/ipc/protocol.ts | 175 ++++++++++++++++++++- packages/shared/src/types.ts | 154 +++++++++++++++++- 5 files changed, 422 insertions(+), 8 deletions(-) create mode 100644 apps/core/src/ipc/methods-coverage.test.ts create mode 100644 apps/core/src/ipc/wire-shapes.test.ts diff --git a/apps/core/src/ipc/methods-coverage.test.ts b/apps/core/src/ipc/methods-coverage.test.ts new file mode 100644 index 00000000..a0bd1748 --- /dev/null +++ b/apps/core/src/ipc/methods-coverage.test.ts @@ -0,0 +1,37 @@ +// ============================================================================= +// `METHODS` is described as the source of truth for the IPC surface +// (CLAUDE.md). It was not one: it declared roughly half the implemented +// handlers, so all of memory.*, config.*, models.* and eight session ops got +// zero compile-time checking in frontends — a frontend could call +// `session.fork` with the wrong params and find out at runtime. +// +// This test is what makes the claim true. Adding a handler without declaring +// it now fails here. +// ============================================================================= + +import test from "node:test"; +import assert from "node:assert/strict"; +import { METHODS } from "@thisisayande/freecode-shared"; +import { methodHandlers } from "../server.js"; + +test("every implemented handler is declared in METHODS", () => { + const declared = new Set(Object.keys(METHODS)); + const undeclared = Object.keys(methodHandlers).filter( + (name) => !declared.has(name), + ); + assert.deepEqual( + undeclared, + [], + `handlers missing from METHODS: ${undeclared.join(", ")}`, + ); +}); + +test("every declared method is implemented", () => { + const implemented = new Set(Object.keys(methodHandlers)); + const missing = Object.keys(METHODS).filter((name) => !implemented.has(name)); + assert.deepEqual( + missing, + [], + `METHODS declares methods with no handler: ${missing.join(", ")}`, + ); +}); diff --git a/apps/core/src/ipc/wire-shapes.test.ts b/apps/core/src/ipc/wire-shapes.test.ts new file mode 100644 index 00000000..4066d215 --- /dev/null +++ b/apps/core/src/ipc/wire-shapes.test.ts @@ -0,0 +1,62 @@ +// ============================================================================= +// Drift guard for the wire mirrors in packages/shared. +// +// `packages/shared` cannot import from `apps/core`, so several IPC result +// types are mirrored there by hand (the same pattern `SessionMeta` and +// `SerializedMessage` already use). A hand mirror rots silently: a field added +// in core just stops being visible to frontends, which is exactly the gap +// `METHODS` exists to close. +// +// Every declaration below is a compile-time assertion that the core type still +// satisfies its mirror. `pnpm check-types` is the real test; the runtime case +// only keeps this a valid test module. +// ============================================================================= + +import test from "node:test"; +import assert from "node:assert/strict"; +import type { + MemoryEntry as WireMemoryEntry, + MemoryType as WireMemoryType, + MemoryGraphStats as WireMemoryGraphStats, + RedactedConfig as WireRedactedConfig, + TurnResult as WireTurnResult, + ExportedSession as WireExportedSession, + ModelInfo as WireModelInfo, +} from "@thisisayande/freecode-shared"; +import type { MemoryEntry, MemoryType } from "../memory/mem-types.js"; +import type { RedactedConfig } from "../providers/config.js"; +import type { LoopResult } from "../agent/types.js"; +import type { ExportedSession } from "../store/remote.js"; +import type { ProviderModel } from "../models-dev.js"; +import type { MemoryGraphService } from "../memory/graph/index.js"; + +/** Fails to compile unless `T` satisfies the wire mirror `U`. */ +type Mirrors = T; + +// Core → wire. A widened, renamed or dropped core field fails here. These are +// type-level only: no value is constructed, so nothing runs at import time. +type _MemoryEntry = Mirrors; +type _MemoryType = Mirrors; +type _GraphStats = Mirrors< + ReturnType, + WireMemoryGraphStats +>; +type _RedactedConfig = Mirrors; +type _TurnResult = Mirrors; +type _ExportedSession = Mirrors; +type _ModelInfo = Mirrors; + +export type { + _MemoryEntry, + _MemoryType, + _GraphStats, + _RedactedConfig, + _TurnResult, + _ExportedSession, + _ModelInfo, +}; + +test("wire mirrors stay assignable from their core types", () => { + // The real assertions are the declarations above, checked by tsc. + assert.ok(true); +}); diff --git a/apps/core/src/server.ts b/apps/core/src/server.ts index 98279298..8204bafc 100644 --- a/apps/core/src/server.ts +++ b/apps/core/src/server.ts @@ -367,7 +367,7 @@ function createError( return { jsonrpc: "2.0", id, error: { code, message, data } }; } -const methodHandlers: Record< +export const methodHandlers: Record< string, (params: Record) => Promise > = { diff --git a/packages/shared/src/ipc/protocol.ts b/packages/shared/src/ipc/protocol.ts index 46e4766b..ce2c329e 100644 --- a/packages/shared/src/ipc/protocol.ts +++ b/packages/shared/src/ipc/protocol.ts @@ -218,15 +218,20 @@ export const METHODS = { params: {} as { sessionId: string; message: string; + model?: string; + effort?: import("../types.js").EffortLevel; + agentMode?: string; images?: Array<{ data: string; mediaType: string; altText?: string }>; }, - // The LoopResult shape when the turn ran normally. When the session was - // already busy, the call parks the prompt in the follow-up queue and - // resolves immediately with { queued: true, id } — the UI uses the - // `message_queued` stream event for the same data so web/SSE subscribes - // stay in sync. + // The completed turn. This said `StreamResponse` for a long time and was + // simply wrong — the handler returns the loop's result, and the per-token + // output arrives on the stream channel, never as the RPC result. When the + // session was already busy the call parks the prompt in the follow-up + // queue and resolves immediately with { queued: true, id }; the UI uses + // the `message_queued` stream event for the same data so web/SSE + // subscribers stay in sync. result: {} as - | StreamResponse + | import("../types.js").TurnResult | { queued: true; id: string }, }, "session.dequeue": { @@ -367,6 +372,164 @@ export const METHODS = { params: undefined, result: {} as { url: string } | { error: "not-installed" }, }, + + // =========================================================================== + // Models + // =========================================================================== + "models.list": { + params: { providerId: "" }, + result: [] as import("../types.js").ModelInfo[], + }, + // Context window in tokens for one provider/model pair, or 0 when the + // catalogue has no entry — never a guessed default. + "models.contextLimit": { + params: { provider: "", model: "" }, + result: 0 as number, + }, + + // =========================================================================== + // Config + // + // `config.get` returns the REDACTED view. There is deliberately no method + // that returns the raw config: the JSON-RPC surface is reachable over the + // web server's POST /api, and `host` is a parameter. + // =========================================================================== + "config.get": { + params: undefined, + result: {} as import("../types.js").RedactedConfig, + }, + "config.setApiKey": { + params: {} as { provider: string; apiKey: string; model?: string }, + result: undefined as void, + }, + "config.setCurrentModel": { + params: { provider: "", model: "" }, + result: undefined as void, + }, + "config.getCurrentModel": { + params: undefined, + result: {} as { provider: string; model: string } | undefined, + }, + "config.getLastAgentMode": { + params: undefined, + result: undefined as string | undefined, + }, + "config.setLastAgentMode": { + params: { mode: "" }, + result: undefined as void, + }, + + // =========================================================================== + // Memory + // =========================================================================== + "memory.list": { + params: {} as { + projectPath?: string; + type?: import("../types.js").MemoryType; + }, + result: [] as import("../types.js").MemoryEntry[], + }, + "memory.get": { + params: {} as { + name: string; + type: import("../types.js").MemoryType; + projectPath?: string; + }, + result: null as import("../types.js").MemoryEntry | null, + }, + "memory.save": { + params: {} as { + entry: import("../types.js").MemoryEntry; + projectPath?: string; + }, + result: undefined as void, + }, + "memory.delete": { + params: {} as { + name: string; + type: import("../types.js").MemoryType; + projectPath?: string; + }, + result: false as boolean, + }, + "memory.query": { + params: {} as { + query: string; + projectPath?: string; + limit?: number; + types?: import("../types.js").MemoryType[]; + }, + result: [] as import("../types.js").MemoryEntry[], + }, + "memory.graph.rebuild": { + params: {} as { projectPath?: string }, + result: {} as import("../types.js").MemoryGraphStats, + }, + "memory.graph.stats": { + params: {} as { projectPath?: string }, + result: {} as import("../types.js").MemoryGraphStats, + }, + // The rendered block, for previewing what a turn would inject. + "memory.buildPrompt": { + params: {} as { + projectPath?: string; + types?: import("../types.js").MemoryType[]; + limit?: number; + all?: boolean; + }, + result: "" as string, + }, + + // =========================================================================== + // Session lifecycle + // =========================================================================== + "session.switch": { + params: { sessionId: "" }, + result: undefined as void, + }, + /** Returns the new session's id. */ + "session.fork": { + params: { sessionId: "" }, + result: "" as string, + }, + "session.archive": { + params: { sessionId: "" }, + result: undefined as void, + }, + // `purge` also removes the session's on-disk artifacts; without it the + // record is marked deleted and the files stay. + "session.delete": { + params: {} as { sessionId: string; purge?: boolean }, + result: undefined as void, + }, + // The session whose last turn was killed mid-stream, if any — what the TUI + // offers to resume at startup. + "session.getInterrupted": { + params: undefined, + result: null as { sessionId: string; messageId: string } | null, + }, + + // =========================================================================== + // Remote sync + // =========================================================================== + "session.export": { + params: { sessionId: "" }, + result: {} as import("../types.js").ExportedSession, + }, + "session.import": { + params: { url: "" }, + result: { sessionId: "" }, + }, + /** Returns the share URL the session was uploaded to. */ + "session.upload": { + params: {} as { sessionId: string; endpoint: string; apiKey?: string }, + result: "" as string, + }, + /** Returns the id of the session the download created locally. */ + "session.download": { + params: {} as { url: string; endpoint?: string; apiKey?: string }, + result: "" as string, + }, } as const; export type MethodName = keyof typeof METHODS; diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 49fb3d33..d6223b7b 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -213,7 +213,15 @@ export interface SerializedMessage { * `SessionResumeResult` below. */ export interface SessionContext extends SessionMeta { - messages: SerializedMessage[]; + messages: Array<{ + id: string; + role: "user" | "assistant"; + content: string; + timestamp: number; + }>; + memories: MemoryEntry[]; + exportedAt: number; + expiresAt?: number; } /** @@ -328,3 +336,147 @@ export interface ContextBreakdown { mcpToolCount: number; messageCount: number; } + +// ============================================================================= +// Wire shapes for the remaining IPC methods +// +// These mirror types that live in `apps/core` — shared cannot import from an +// app, and an app type carries internals (Effect handles, storage paths) that +// have no business on the wire. Same pattern as `SessionMeta` and +// `SerializedMessage` above. `ipc/wire-shapes.test.ts` in core asserts each +// core type is assignable to its mirror, so drift is a typecheck failure +// rather than a runtime surprise in a frontend. +// ============================================================================= + +/** Mirrors core's `ModelLimit` (`models-dev.ts`). */ +export interface ModelLimit { + /** Context-window size (max input tokens). */ + context: number; + /** Max output tokens per response. */ + output: number; +} + +/** Mirrors core's `ModelCost` (`models-dev.ts`) — USD per million tokens. */ +export interface ModelCost { + input: number; + output: number; + cacheRead?: number; + cacheWrite?: number; +} + +/** Mirrors core's `ProviderModel` (`models-dev.ts`) — one `models.list` row. */ +export interface ModelInfo { + id: string; + name: string; + description?: string; + /** Present when models.dev reports limits for this model. */ + limit?: ModelLimit; + /** Present when models.dev publishes a rate card for this model. */ + cost?: ModelCost; + /** Input modalities, e.g. ["text", "image", "pdf"]. Absent if unreported. */ + inputModalities?: string[]; +} + +/** Mirrors core's `MemoryType` (`memory/mem-types.ts`). */ +export type MemoryType = + | "user" + | "feedback" + | "project" + | "reference" + | "episode"; + +/** Mirrors core's `MemoryEntry` (`memory/mem-types.ts`). */ +export interface MemoryEntry { + name: string; + description: string; + type: MemoryType; + content: string; + createdAt: number; + updatedAt: number; + tags?: string[]; + supersedes?: string[]; + /** ISO date (YYYY-MM-DD) an episode describes; absent means undated. */ + happened_at?: string; +} + +/** Mirrors the return of core's `MemoryGraphService.stats()`. */ +export interface MemoryGraphStats { + vectors: number; + dims: number; + nodes: number; + edges: number; + clusters: number; + embedder: boolean; +} + +export type AnthropicAuthMode = "oauth" | "api-key"; + +/** + * Mirrors core's `RedactedConfig` (`providers/config.ts`) — the ONLY config + * shape that leaves the process. There is no wire type for the raw config + * because there must never be one: `config.get` returns this. + */ +export interface RedactedConfig { + providers?: Record< + string, + { hasApiKey: boolean; model?: string; authMode?: AnthropicAuthMode } + >; + web?: Record; + current?: { provider: string; model: string }; + lastAgentMode?: string; + recovery?: { fallbackProviders?: string[] }; +} + +/** + * Mirrors the wire-facing half of core's `LoopResult` (`agent/types.ts`). + * `finalState` is deliberately omitted: it is the loop's internal state + * machine, and no frontend reads it. + */ +export interface TurnResult { + success: boolean; + message?: string; + content?: string; + thinking?: string; + turnCount: number; + iterationCount: number; + usage?: { + /** Already includes cache writes — they are billed as input. */ + inputTokens: number; + outputTokens: number; + cacheReadInputTokens?: number; + /** The same tokens as above, broken out for the hit rate. Not an addend. */ + cacheCreationInputTokens?: number; + /** The last API call's full input — true context-window occupancy. */ + contextTokens?: number; + }; +} + +/** + * Mirrors core's `ExportedSession` (`store/remote.ts`). Its `messages` are a + * flattened transcript, NOT `SerializedMessage` — the export format drops + * parts so it stays readable and stable across store versions. + */ +export interface ExportedSession { + version: 1; + metadata: { + id: string; + title: string; + projectPath: string; + provider: string; + status: "active" | "archived" | "deleted"; + createdAt: number; + updatedAt: number; + lastTurnAt: number; + turnCount: number; + parentId?: string; + }; + messages: Array<{ + id: string; + role: "user" | "assistant"; + content: string; + timestamp: number; + }>; + memories: MemoryEntry[]; + exportedAt: number; + expiresAt?: number; +} From fecc3d2f22f42ec55f5a6a30fd160db273b1b664 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:18:11 +0530 Subject: [PATCH 10/16] fix(ipc): reject bad params with -32602 instead of a confusing -32603 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every handler reads its params through `params as { … }`, a cast that checks nothing. A missing or misspelled field arrived as `undefined`, failed somewhere inside the handler, and came back as -32603 — an internal error, which tells the caller the server is broken when the request was. `REQUIRED_PARAMS` in packages/shared declares, per method, the params a handler genuinely cannot proceed without and their JSON types. `handleRequest` checks it before dispatch and answers -32602 naming the field and both types, so the call can be fixed from the error alone. The table is typed `Record`, so a new method without an entry is a compile error — there is no way to add a method that quietly skips validation. Scope is deliberately narrow. It validates presence and type of REQUIRED params only: it is not a schema validator. Optional params stay the handler's business (they have defaults), `null` counts as missing because that is how a JSON caller usually spells "nothing" and it fails the same way, and unknown params are ignored so a newer frontend still talks to an older core. The rule for what goes in the table is what the handler actually needs, not what `METHODS` types — `commands.list` types `projectPath` as required but falls back to cwd, so validation must not reject a call the handler would have served. Tests: 9 in `ipc/validate-params.test.ts`, including the point of the exercise — `handleRequest` returning -32602 rather than -32603 — plus one that an unknown method is still -32601. 1246 pass / 0 fail. --- apps/core/src/ipc/validate-params.test.ts | 86 +++++++++++++++++++++++ apps/core/src/ipc/validate-params.ts | 64 +++++++++++++++++ apps/core/src/server.ts | 8 ++- packages/shared/src/index.ts | 13 +++- packages/shared/src/ipc/protocol.ts | 81 +++++++++++++++++++++ 5 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 apps/core/src/ipc/validate-params.test.ts create mode 100644 apps/core/src/ipc/validate-params.ts diff --git a/apps/core/src/ipc/validate-params.test.ts b/apps/core/src/ipc/validate-params.test.ts new file mode 100644 index 00000000..ab984c0f --- /dev/null +++ b/apps/core/src/ipc/validate-params.test.ts @@ -0,0 +1,86 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { METHODS, REQUIRED_PARAMS } from "@thisisayande/freecode-shared"; +import { validateParams, INVALID_PARAMS } from "./validate-params.js"; +import { handleRequest } from "../server.js"; + +test("a missing required param names the field", () => { + const reason = validateParams("session.send", { sessionId: "s1" }); + assert.match(reason ?? "", /message/); + assert.match(reason ?? "", /Missing required parameter/); +}); + +test("a mistyped required param reports both types", () => { + const reason = validateParams("session.send", { + sessionId: 42, + message: "hi", + }); + assert.equal(reason, 'Parameter "sessionId" must be string, got number'); +}); + +// null is how a JSON caller most often expresses "I had nothing for this", +// and it hits the same undefined-deep-inside failure a missing key does. +test("null is treated as missing, not as a value", () => { + assert.match( + validateParams("session.stop", { sessionId: null }) ?? "", + /Missing required parameter/, + ); +}); + +test("arrays and objects are distinguished", () => { + assert.equal(validateParams("question.answer", { + requestId: "r1", + answers: ["yes"], + }), undefined); + assert.match( + validateParams("question.answer", { requestId: "r1", answers: {} }) ?? "", + /must be array, got object/, + ); +}); + +test("valid params pass, and unknown extras are ignored", () => { + assert.equal( + validateParams("session.send", { + sessionId: "s1", + message: "hi", + somethingNewer: true, + }), + undefined, + ); +}); + +test("methods with no mandatory params accept an empty object", () => { + for (const method of ["config.get", "usage.get", "session.list"]) { + assert.equal(validateParams(method, {}), undefined, method); + } +}); + +test("every declared method has a param contract", () => { + const missing = Object.keys(METHODS).filter( + (m) => !(m in REQUIRED_PARAMS), + ); + assert.deepEqual(missing, []); +}); + +// The point of the whole exercise: a bad call must not look like a server bug. +test("handleRequest answers -32602, not -32603, for bad params", async () => { + const response = await handleRequest({ + jsonrpc: "2.0", + id: 1, + method: "session.send", + params: { sessionId: "s1" }, + }); + + assert.equal(response.error?.code, INVALID_PARAMS); + assert.match(response.error?.message ?? "", /message/); + assert.equal(response.result, undefined); +}); + +test("an unknown method is still -32601", async () => { + const response = await handleRequest({ + jsonrpc: "2.0", + id: 2, + method: "nope.nope", + }); + assert.equal(response.error?.code, -32601); +}); diff --git a/apps/core/src/ipc/validate-params.ts b/apps/core/src/ipc/validate-params.ts new file mode 100644 index 00000000..d050ac64 --- /dev/null +++ b/apps/core/src/ipc/validate-params.ts @@ -0,0 +1,64 @@ +// ============================================================================= +// JSON-RPC -32602 (invalid params) validation. +// +// Every handler reads its params through `params as { … }`. A cast checks +// nothing, so a missing or misspelled field arrived as `undefined` and blew +// up somewhere inside the handler — reported as -32603 (internal error), +// which tells the caller the server is broken when in fact the request was. +// +// The contract lives in `REQUIRED_PARAMS` next to `METHODS` in +// packages/shared, typed so a new method cannot skip it. This module is only +// the check. +// +// Deliberately narrow: presence and JSON type of the REQUIRED params, nothing +// more. It is not a schema validator. Optional params keep being the +// handler's business (they have defaults), and unknown params are ignored so +// a newer frontend talking to an older core still works. +// ============================================================================= + +import { + REQUIRED_PARAMS, + type MethodName, + type ParamType, +} from "@thisisayande/freecode-shared"; + +/** JSON-RPC reserved code for "invalid method parameters". */ +export const INVALID_PARAMS = -32602; + +function jsonTypeOf(value: unknown): ParamType | "null" | "undefined" { + if (value === null) return "null"; + if (value === undefined) return "undefined"; + if (Array.isArray(value)) return "array"; + const t = typeof value; + if (t === "string" || t === "number" || t === "boolean") return t; + if (t === "object") return "object"; + // function/symbol/bigint cannot survive JSON transport; report the raw tag. + return t as ParamType; +} + +/** + * Returns a human-readable reason the params are invalid, or undefined when + * they are acceptable. The message names the field and both types, because + * the whole point is that the caller can fix the call from the error alone. + */ +export function validateParams( + method: string, + params: Record, +): string | undefined { + const required = REQUIRED_PARAMS[method as MethodName]; + // An undeclared method is -32601's problem, not ours. Reaching here at all + // means the method exists but predates the table, and refusing the call + // would be worse than letting the handler run. + if (!required) return undefined; + + for (const [name, expected] of Object.entries(required)) { + const actual = jsonTypeOf(params[name]); + if (actual === "undefined" || actual === "null") { + return `Missing required parameter "${name}" (expected ${expected})`; + } + if (actual !== expected) { + return `Parameter "${name}" must be ${expected}, got ${actual}`; + } + } + return undefined; +} diff --git a/apps/core/src/server.ts b/apps/core/src/server.ts index 8204bafc..69a9bf76 100644 --- a/apps/core/src/server.ts +++ b/apps/core/src/server.ts @@ -44,6 +44,7 @@ import { } from "./providers/config.js"; import { logger } from "./utils/logger.js"; import { formatFatalError } from "./cli/format-fatal-error.js"; +import { validateParams, INVALID_PARAMS } from "./ipc/validate-params.js"; import type { ToolContext } from "./tools/types.js"; import type { JsonRpcRequest, @@ -1201,7 +1202,12 @@ export async function handleRequest( `Method not found: ${request.method}`, ); } - const result = await handler(request.params ?? {}); + const params = request.params ?? {}; + const invalid = validateParams(request.method, params); + if (invalid) { + return createError(request.id, INVALID_PARAMS, invalid); + } + const result = await handler(params); return createResponse(request.id, result); } catch (error) { if (error instanceof JsonRpcError) { diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 41013a1e..c67cce69 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -31,6 +31,16 @@ export type { ContextSegmentId, ContextSegmentStat, ContextBreakdown, + ModelLimit, + ModelCost, + ModelInfo, + MemoryType, + MemoryEntry, + MemoryGraphStats, + AnthropicAuthMode, + RedactedConfig, + TurnResult, + ExportedSession, } from "./types.js"; // IPC Protocol @@ -44,6 +54,7 @@ export type { MethodName, MethodParams, MethodResult, + ParamType, } from "./ipc/protocol.js"; -export { METHODS } from "./ipc/protocol.js"; +export { METHODS, REQUIRED_PARAMS } from "./ipc/protocol.js"; diff --git a/packages/shared/src/ipc/protocol.ts b/packages/shared/src/ipc/protocol.ts index ce2c329e..9a05b117 100644 --- a/packages/shared/src/ipc/protocol.ts +++ b/packages/shared/src/ipc/protocol.ts @@ -533,5 +533,86 @@ export const METHODS = { } as const; export type MethodName = keyof typeof METHODS; + +// ============================================================================= +// Runtime parameter contracts +// +// Handlers reach their params through `params as { … }` — a cast, which +// checks nothing. A missing or mistyped field therefore became `undefined` +// deep inside the handler and surfaced as an internal error (-32603), which +// says "the server broke" when the truth is "you sent the wrong params". +// +// This table is what the server validates against before dispatch, so a bad +// call gets -32602 and the field name. It is typed `Record`, +// so a new method without an entry is a compile error — there is no path to +// adding a method that silently skips validation. Methods with nothing +// mandatory declare `{}`; optional params are deliberately absent, since +// omitting them is legal and the handler already defaults them. +// ============================================================================= + +export type ParamType = "string" | "number" | "boolean" | "object" | "array"; + +export const REQUIRED_PARAMS: Record< + MethodName, + Readonly> +> = { + "tools.list": {}, + "tools.call": { name: "string", args: "object" }, + // projectPath is validated by the handler, which falls back to cwd when it + // is missing or does not exist — a fallback, not a contract violation. + "session.start": {}, + "session.send": { sessionId: "string", message: "string" }, + "session.dequeue": { sessionId: "string", id: "string" }, + "session.stop": { sessionId: "string" }, + "session.compact": { sessionId: "string" }, + "session.list": {}, + "session.resume": { sessionId: "string" }, + "session.claudeList": {}, + "session.claudeTranscript": { sessionId: "string" }, + "providers.list": {}, + "config.setWebCredential": { provider: "string", credential: "object" }, + // projectPath is optional in both handlers (they fall back to the process + // cwd), so it is not required here. The rule for this table is what the + // handler actually needs — validation must never reject a call the handler + // would have served. + "commands.list": {}, + "commands.resolve": { name: "string" }, + "question.answer": { requestId: "string", answers: "array" }, + "question.reject": { requestId: "string" }, + "permission.answer": { requestId: "string", decision: "string" }, + "permission.reject": { requestId: "string" }, + "context.stats": { sessionId: "string" }, + "usage.get": {}, + "skills.list": {}, + "mcp.status": {}, + "history.list": {}, + "history.append": { text: "string" }, + "graph.explore": {}, + "models.list": { providerId: "string" }, + "models.contextLimit": { provider: "string", model: "string" }, + "config.get": {}, + "config.setApiKey": { provider: "string", apiKey: "string" }, + "config.setCurrentModel": { provider: "string", model: "string" }, + "config.getCurrentModel": {}, + "config.getLastAgentMode": {}, + "config.setLastAgentMode": { mode: "string" }, + "memory.list": {}, + "memory.get": { name: "string", type: "string" }, + "memory.save": { entry: "object" }, + "memory.delete": { name: "string", type: "string" }, + "memory.query": { query: "string" }, + "memory.graph.rebuild": {}, + "memory.graph.stats": {}, + "memory.buildPrompt": {}, + "session.switch": { sessionId: "string" }, + "session.fork": { sessionId: "string" }, + "session.archive": { sessionId: "string" }, + "session.delete": { sessionId: "string" }, + "session.getInterrupted": {}, + "session.export": { sessionId: "string" }, + "session.import": { url: "string" }, + "session.upload": { sessionId: "string", endpoint: "string" }, + "session.download": { url: "string" }, +}; export type MethodParams = (typeof METHODS)[M]["params"]; export type MethodResult = (typeof METHODS)[M]["result"]; From 8ac41e73136b1c19efddfb93e282684a4a940932 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:22:52 +0530 Subject: [PATCH 11/16] feat(settings): publish a JSON Schema and warn on keys nothing reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `settings.json` is hand-edited, security-relevant, and had neither. Four modules read it independently — permissions, hooks, memory, redirect — and each ignores what it does not recognise. Right per reader; the sum was that `"permission"` for `"permissions"` disabled every rule and looked exactly like the permission system being broken. `settings/known-keys.ts` is the one place that knows the whole shape, so it can say "unknown key" and, for a near miss, which key was meant. `settings/validate.ts` runs it over both scopes once per process, from the shared hook bootstrap — the same bootstrap, because a second call site is how `serve` and `run` diverged last time. Deliberately a NAME check, not a schema validator. Values stay each reader's business: they already validate and default their own, and a second copy of those rules would diverge. Hook event names are left to `hooks/settings.ts`, which already reports an unknown one with the full valid list — one good message beats two in different words. And it warns rather than refuses: a file with a stray key is still a usable file. `schemas/settings.schema.json` is the editor half, wired up with `$schema` (the one key FreeCode ignores on purpose). Two descriptions of the same file drift the moment a key lands in only one, so a test asserts the schema's key set matches `KNOWN_SETTINGS` section by section. Tests: 9 in `settings/known-keys.test.ts`, including the motivating typo, a section-level typo, the two deliberate non-warnings, and the schema/runtime drift guard. 1255 pass / 0 fail. Docs: `/reference/settings` gained the `$schema` section, `redirect` in the top-level table (it was reachable but undocumented there), and the four memory keys the page never listed —`retrievalJudge`, `autoConsolidate`, `consolidateMinHours`, `consolidateMinSessions`. Two Known-gaps bullets and their `TODO.md` entries are now closed. --- TODO.md | 17 ++- apps/core/src/hooks/bootstrap.ts | 8 ++ apps/core/src/settings/known-keys.test.ts | 111 ++++++++++++++++ apps/core/src/settings/known-keys.ts | 132 +++++++++++++++++++ apps/core/src/settings/validate.ts | 55 ++++++++ apps/docs/app/reference/settings/page.mdx | 46 +++++-- schemas/settings.schema.json | 146 ++++++++++++++++++++++ 7 files changed, 497 insertions(+), 18 deletions(-) create mode 100644 apps/core/src/settings/known-keys.test.ts create mode 100644 apps/core/src/settings/known-keys.ts create mode 100644 apps/core/src/settings/validate.ts create mode 100644 schemas/settings.schema.json diff --git a/TODO.md b/TODO.md index 9cf57b62..54a60032 100644 --- a/TODO.md +++ b/TODO.md @@ -245,12 +245,17 @@ earlier audit are not repeated here. the difference and `/getting-started/configuration` claims a single "project wins" rule that only holds for hooks. One loader that parses the file once and hands each subsystem its section would make one answer true. -- [ ] **Unknown `settings.json` keys are silently ignored.** `"permission"` for - `"permissions"`, or a misspelled hook field, produces no warning — identical - from the outside to the feature being broken. Each loader already warns on - malformed input. -- [ ] **No JSON Schema for `settings.json`.** No `$schema`, no generated schema, so - editors cannot complete or validate a hand-edited, security-relevant file. +- [x] **Unknown `settings.json` keys are silently ignored.** Done 2026-09-05: + `settings/known-keys.ts` + `settings/validate.ts`, called from the shared + hook bootstrap so `serve` and `run` behave alike. Reports any key nothing + reads and suggests the near miss ("did you mean `permissions`?"). A name + check only — values stay each loader's business, and hook event names stay + with `hooks/settings.ts`, which already names the valid list. +- [x] **No JSON Schema for `settings.json`.** Done 2026-09-05: + `schemas/settings.schema.json`, referenced from `/reference/settings` and + usable via `$schema`. A test asserts the schema's key set matches + `KNOWN_SETTINGS`, so the editor contract and the runtime warning cannot + drift. - [ ] **`FREECODE_HOME` is read in exactly one place** — the updater's `builds/stable/freecode` lookup (`apps/tui/src/entry.ts:101`). Every data path (`config.json`, `sessions/`, `projects/`, `rollout/`, `history.jsonl`) builds diff --git a/apps/core/src/hooks/bootstrap.ts b/apps/core/src/hooks/bootstrap.ts index ee47bc99..5653086f 100644 --- a/apps/core/src/hooks/bootstrap.ts +++ b/apps/core/src/hooks/bootstrap.ts @@ -10,6 +10,7 @@ import { registerRtkHook } from "./builtin/rtk-rewrite.js"; import { HookSettingsManager } from "./settings.js"; +import { warnOnUnknownSettings } from "../settings/validate.js"; export interface HookBootstrapOptions { /** @@ -23,6 +24,11 @@ export interface HookBootstrapOptions { /** * Register built-in hooks and load `settings.json` hooks for `projectRoot`. * Returns the manager so the caller can dispose it on shutdown. + * + * Also the one place the settings file is checked for keys nothing reads — + * it lives here for the same reason hooks do: this is the single bootstrap + * both `serve` and `run` go through, and a second call site is how the two + * diverged last time. */ export function initHooks( projectRoot: string, @@ -32,6 +38,8 @@ export function initHooks( // equivalents to save tokens. No-op unless rtk resolves; FREECODE_RTK=0 opts out. registerRtkHook(); + warnOnUnknownSettings(projectRoot); + const hookSettings = new HookSettingsManager(projectRoot); hookSettings.load(); if (options.watch) hookSettings.watch(); diff --git a/apps/core/src/settings/known-keys.test.ts b/apps/core/src/settings/known-keys.test.ts new file mode 100644 index 00000000..ebea11c2 --- /dev/null +++ b/apps/core/src/settings/known-keys.test.ts @@ -0,0 +1,111 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; +import { fileURLToPath } from "url"; +import { findUnknownSettings, KNOWN_SETTINGS } from "./known-keys.js"; +import { warnOnUnknownSettings } from "./validate.js"; + +test("a known file produces no warnings", () => { + assert.deepEqual( + findUnknownSettings({ + $schema: "https://example.com/settings.schema.json", + permissions: { allow: ["Bash(git *)"], deny: [] }, + memory: { autoExtract: false, extractEveryNRuns: 4 }, + redirect: { enabled: true }, + hooks: { PostToolUse: [] }, + }), + [], + ); +}); + +// The motivating typo: singular "permission" silently disabled every rule. +test("a near-miss top-level key suggests the right one", () => { + const [warning, ...rest] = findUnknownSettings({ permission: { allow: [] } }); + assert.deepEqual(rest, []); + assert.equal(warning.path, "permission"); + assert.match(warning.message, /did you mean "permissions"/); +}); + +test("an unrecognizable top-level key lists what is known", () => { + const [warning] = findUnknownSettings({ telemetry: { enabled: true } }); + assert.match(warning.message, /known settings: permissions, hooks, memory, redirect/); +}); + +test("a typo inside a section is reported with its full path", () => { + const [warning] = findUnknownSettings({ memory: { autoExtractt: true } }); + assert.equal(warning.path, "memory.autoExtractt"); + assert.match(warning.message, /did you mean "memory\.autoExtract"/); +}); + +// hooks sub-keys are event names, and hooks/settings.ts already reports an +// unknown one with the full valid list. Warning here too would say the same +// thing twice, in different words. +test("hook event names are left to the hooks loader", () => { + assert.deepEqual(findUnknownSettings({ hooks: { NotAnEvent: [] } }), []); +}); + +test("a wrong-typed section is left to its own reader", () => { + assert.deepEqual(findUnknownSettings({ permissions: "yes" }), []); +}); + +test("warnOnUnknownSettings reads the project scope and names the file", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "freecode-settings-")); + try { + fs.mkdirSync(path.join(dir, ".freecode")); + fs.writeFileSync( + path.join(dir, ".freecode", "settings.json"), + JSON.stringify({ permission: { allow: [] } }), + ); + + const emitted = warnOnUnknownSettings(dir); + const projectWarnings = emitted.filter((m) => m.includes(dir)); + + assert.equal(projectWarnings.length, 1); + assert.match(projectWarnings[0], /did you mean "permissions"/); + assert.match(projectWarnings[0], /settings\.json/); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a missing settings file is silent", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "freecode-settings-")); + try { + assert.deepEqual( + warnOnUnknownSettings(dir).filter((m) => m.includes(dir)), + [], + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +// Two descriptions of the same file — one for editors, one for the runtime +// warning — drift the moment a key is added to only one of them. +test("the shipped JSON Schema and the runtime key list agree", () => { + const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../..", + ); + const schema = JSON.parse( + fs.readFileSync(path.join(repoRoot, "schemas", "settings.schema.json"), "utf-8"), + ) as { + properties: Record }>; + }; + + const schemaTopLevel = Object.keys(schema.properties).filter( + (k) => k !== "$schema", + ); + assert.deepEqual(schemaTopLevel.sort(), Object.keys(KNOWN_SETTINGS).sort()); + + for (const [section, known] of Object.entries(KNOWN_SETTINGS)) { + if (known === "any") continue; + assert.deepEqual( + Object.keys(schema.properties[section].properties ?? {}).sort(), + [...known].sort(), + `${section} keys differ between the schema and known-keys.ts`, + ); + } +}); diff --git a/apps/core/src/settings/known-keys.ts b/apps/core/src/settings/known-keys.ts new file mode 100644 index 00000000..d8c496d9 --- /dev/null +++ b/apps/core/src/settings/known-keys.ts @@ -0,0 +1,132 @@ +// ============================================================================= +// The known shape of `.freecode/settings.json`. +// +// Four modules read this file independently — permissions, hooks, memory and +// redirect — and each ignores everything it does not recognise. That is the +// right behaviour per reader, but the sum of it was that an unknown key was +// indistinguishable from a broken feature: `"permission"` for `"permissions"` +// silently disabled every rule in a security-relevant, hand-edited file, with +// nothing on stderr. +// +// This module is the one place that knows the whole shape, so it can say +// "unknown key" — and, when the key is a near miss, which key was meant. +// +// It is a NAME check, not a schema validator. Values stay each reader's +// business: they already validate and default their own, and duplicating +// those rules here would create a second, slowly diverging contract. The JSON +// Schema shipped at `schemas/settings.schema.json` covers types for editors, +// and `known-keys.test.ts` pins the two together. +// ============================================================================= + +/** Ignored everywhere, but conventional and useful — editors key off it. */ +export const SCHEMA_KEY = "$schema"; + +export const KNOWN_SETTINGS: Readonly< + Record +> = { + permissions: ["allow", "ask", "deny"], + // Sub-keys are hook EVENT names, and `hooks/settings.ts` already warns on an + // unknown one with the full valid list. Left uninspected here so a typo'd + // event gets one good message rather than two in different words. + hooks: "any", + memory: [ + "autoExtract", + "extractEveryNRuns", + "retrievalJudge", + "autoConsolidate", + "consolidateMinHours", + "consolidateMinSessions", + ], + redirect: ["enabled", "maxPerRun"], +}; + +/** + * Levenshtein distance, bounded by an early exit — we only ever care whether + * a key is within a typo's reach of a real one. + */ +function distance(a: string, b: string): number { + const rows = a.length + 1; + const cols = b.length + 1; + let prev = Array.from({ length: cols }, (_, i) => i); + for (let i = 1; i < rows; i++) { + const row = [i, ...new Array(cols - 1).fill(0)]; + for (let j = 1; j < cols; j++) { + row[j] = Math.min( + prev[j] + 1, + row[j - 1] + 1, + prev[j - 1] + (a[i - 1] === b[j - 1] ? 0 : 1), + ); + } + prev = row; + } + return prev[cols - 1]; +} + +/** The known key a typo most likely meant, or undefined if none is close. */ +function nearest(key: string, candidates: readonly string[]): string | undefined { + let best: string | undefined; + let bestDistance = Infinity; + const lower = key.toLowerCase(); + for (const candidate of candidates) { + const d = distance(lower, candidate.toLowerCase()); + if (d < bestDistance) { + bestDistance = d; + best = candidate; + } + } + // Two edits on a short key is already a stretch; beyond that a suggestion + // is noise that sends the reader after the wrong fix. + const limit = Math.min(2, Math.max(1, Math.floor(key.length / 3))); + return bestDistance <= limit ? best : undefined; +} + +export interface SettingsWarning { + /** Dotted path to the offending key, e.g. `memory.autoExtracts`. */ + path: string; + message: string; +} + +/** + * Report keys this codebase does not read. Pure — the caller decides whether + * to log, print or ignore. + */ +export function findUnknownSettings( + settings: Record, +): SettingsWarning[] { + const warnings: SettingsWarning[] = []; + const topLevel = Object.keys(KNOWN_SETTINGS); + + for (const [key, value] of Object.entries(settings)) { + if (key === SCHEMA_KEY) continue; + if (!(key in KNOWN_SETTINGS)) { + const suggestion = nearest(key, topLevel); + warnings.push({ + path: key, + message: suggestion + ? `Unknown setting "${key}" — did you mean "${suggestion}"?` + : `Unknown setting "${key}" (known settings: ${topLevel.join(", ")})`, + }); + continue; + } + + const known = KNOWN_SETTINGS[key]; + if (known === "any") continue; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + // A wrong-typed section is the reader's call to make (it fails closed + // and says so); flagging it here would double up on that message. + continue; + } + for (const sub of Object.keys(value as Record)) { + if (known.includes(sub)) continue; + const suggestion = nearest(sub, known); + warnings.push({ + path: `${key}.${sub}`, + message: suggestion + ? `Unknown setting "${key}.${sub}" — did you mean "${key}.${suggestion}"?` + : `Unknown setting "${key}.${sub}"`, + }); + } + } + + return warnings; +} diff --git a/apps/core/src/settings/validate.ts b/apps/core/src/settings/validate.ts new file mode 100644 index 00000000..4ff9ebe2 --- /dev/null +++ b/apps/core/src/settings/validate.ts @@ -0,0 +1,55 @@ +// ============================================================================= +// Read both settings scopes and report keys nothing in this codebase reads. +// +// Every reader of settings.json ignores what it does not recognise, which +// meant a typo was invisible: `"permission"` for `"permissions"` disabled +// every rule and looked exactly like the permission system being broken. +// This runs once per process, at bootstrap, and says so on stderr. +// +// Warn only. A settings file with an unknown key is still a usable settings +// file, and refusing to start over a stray key would be a far worse failure +// than the one being fixed. +// ============================================================================= + +import * as fs from "fs"; +import { settingsPath, type RuleScope } from "../permission/settings.js"; +import { findUnknownSettings } from "./known-keys.js"; +import { logger } from "../utils/logger.js"; + +/** + * Returns the warnings it emitted, so a caller (or a test) can see them + * without scraping the log. + */ +export function warnOnUnknownSettings(projectRoot: string): string[] { + const emitted: string[] = []; + + for (const scope of ["project", "user"] as const satisfies RuleScope[]) { + const filePath = settingsPath(scope, projectRoot); + let parsed: unknown; + try { + parsed = JSON.parse(fs.readFileSync(filePath, "utf-8")); + } catch (error) { + // ENOENT is the normal case. A parse failure is already reported by + // each reader as it fails closed, and saying it a fifth time here + // helps nobody. + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + logger.warn(`[Settings] Could not read ${filePath}: ${error}`); + } + continue; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + logger.warn(`[Settings] ${filePath} is not a JSON object — ignoring it.`); + continue; + } + + for (const warning of findUnknownSettings( + parsed as Record, + )) { + const message = `[Settings] ${warning.message} (${filePath})`; + logger.warn(message); + emitted.push(message); + } + } + + return emitted; +} diff --git a/apps/docs/app/reference/settings/page.mdx b/apps/docs/app/reference/settings/page.mdx index c6f18961..e0fbae11 100644 --- a/apps/docs/app/reference/settings/page.mdx +++ b/apps/docs/app/reference/settings/page.mdx @@ -14,19 +14,36 @@ agreed on travel with the repository. Credentials, the current model, and MCP servers are *not* here — they live in `~/.freecode/config.json`; see [the overview](/reference#the-three-surfaces). -**Exactly three top-level keys are read.** Anything else in the file is ignored -without comment. +**Exactly four top-level keys are read.** Anything else produces a warning on +startup naming the key — and, when it is a near miss, the key you probably +meant. | Key | Read by | Purpose | | --- | --- | --- | | [`permissions`](#permissions) | `permission/settings.ts` | which tool calls are allowed, asked about, or denied | | [`hooks`](#hooks) | `hooks/settings.ts` | shell commands to run at lifecycle events | | [`memory`](#memory) | `memory/extract-policy.ts` | automatic memory extraction | +| [`redirect`](#redirect) | `agent/redirect/settings.ts` | trajectory redirection (off by default) | + +## Editor completion + +A JSON Schema ships at [`schemas/settings.schema.json`](https://github.com/ayan-de/freecode/blob/main/schemas/settings.schema.json). +Point `$schema` at it and your editor completes and validates the file as you +type: + +```json +{ + "$schema": "https://raw.githubusercontent.com/ayan-de/freecode/main/schemas/settings.schema.json" +} +``` + +`$schema` is the one key FreeCode ignores on purpose. ## A complete file ```json { + "$schema": "https://raw.githubusercontent.com/ayan-de/freecode/main/schemas/settings.schema.json", "permissions": { "allow": ["Read", "Grep", "Bash(pnpm test:*)"], "ask": ["Bash(git push:*)"], @@ -193,6 +210,10 @@ stdout form are documented under [hook events](/reference/hook-events#the-shell- | --- | --- | --- | --- | | `autoExtract` | boolean | `true` | mine finished turns for facts worth remembering | | `extractEveryNRuns` | number | `8` | how often extraction is even considered; values `< 1` are ignored, non-integers are floored | +| `retrievalJudge` | boolean | `true` | judge retrieved memories for relevance before injecting them; fails closed | +| `autoConsolidate` | boolean | `true` | one cheap merge pass per project per day — merges only, never deletes | +| `consolidateMinHours` | number | `24` | minimum hours between consolidation runs | +| `consolidateMinSessions` | number | `5` | minimum sessions since the last run before consolidating again; values `< 1` are ignored | ```json { "memory": { "autoExtract": false } } @@ -201,8 +222,9 @@ stdout form are documented under [hook events](/reference/hook-events#the-shell- Scope merge here is **first definition wins, project → user → default**, per field. `FREECODE_DISABLE_MEMORY_EXTRACTION=1` overrides both files. Throttling is safe because each extraction rebuilds the transcript from the session's whole -history, so a skipped run is covered by the next one. Background: -[memory](/internals/memory). +history, so a skipped run is covered by the next one. +`FREECODE_DISABLE_MEMORY_JUDGE=1` and `FREECODE_DISABLE_MEMORY_CONSOLIDATION=1` +do the same for the other two. Background: [memory](/internals/memory). ## `redirect` @@ -246,14 +268,14 @@ Found while writing this page; each is also tracked in `TODO.md`. `/getting-started/configuration` currently claims a single "project wins" rule that is only true for hooks. A shared loader (parse once, hand each subsystem its section) would make one answer true. -- **Unknown keys are silently ignored.** `"permission"` instead of `"permissions"`, - or a hook field typo, produces no warning anywhere — the settings simply do not - apply, which is indistinguishable from the feature being broken. Each loader - already warns on *malformed* input; warning on unrecognised top-level keys is - the same cost. -- **No published schema.** There is no `$schema` and no generated JSON Schema, so - editors cannot complete or validate the file — for a hand-edited security-relevant - file that is the highest-value missing piece. +- ~~**Unknown keys are silently ignored.**~~ Fixed: `settings/known-keys.ts` + reports any key nothing reads, with a "did you mean" for near misses. It is a + *name* check only — values stay each loader's business, since they already + validate and default their own. Hook event names are still left to + `hooks/settings.ts`, which already names the valid list. +- ~~**No published schema.**~~ Fixed: `schemas/settings.schema.json`, referenced + via `$schema` (see above). A test asserts the schema's keys and the runtime + key list agree, so the two cannot drift. - **`once` is parsed but never enforced** (`hooks/settings.ts` → `RegisteredHook`). Either implement per-session tracking or reject the field so it cannot look configured when it is not. diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json new file mode 100644 index 00000000..9f7dd77b --- /dev/null +++ b/schemas/settings.schema.json @@ -0,0 +1,146 @@ +{ + "$schema": "https://json-schema.org/draft-07/schema#", + "$id": "https://raw.githubusercontent.com/ayan-de/freecode/main/schemas/settings.schema.json", + "title": "FreeCode settings.json", + "description": "Project (.freecode/settings.json) and user (~/.freecode/settings.json) settings. Both scopes use this shape; project wins per key.", + "type": "object", + "additionalProperties": false, + "properties": { + "$schema": { "type": "string" }, + + "permissions": { + "description": "Per-rule allow/ask/deny. deny anywhere wins; scope does not break a tie within a tier.", + "type": "object", + "additionalProperties": false, + "properties": { + "allow": { "$ref": "#/$defs/ruleList" }, + "ask": { "$ref": "#/$defs/ruleList" }, + "deny": { "$ref": "#/$defs/ruleList" } + } + }, + + "hooks": { + "description": "Shell commands run at lifecycle points, keyed by event name.", + "type": "object", + "additionalProperties": false, + "properties": { + "PreToolUse": { "$ref": "#/$defs/hookList" }, + "PostToolUse": { "$ref": "#/$defs/hookList" }, + "PostToolUseFailure": { "$ref": "#/$defs/hookList" }, + "PermissionRequest": { "$ref": "#/$defs/hookList" }, + "PreCompact": { "$ref": "#/$defs/hookList" }, + "PostCompact": { "$ref": "#/$defs/hookList" }, + "SessionStart": { "$ref": "#/$defs/hookList" }, + "UserPromptSubmit": { "$ref": "#/$defs/hookList" }, + "SubagentStart": { "$ref": "#/$defs/hookList" }, + "SubagentStop": { "$ref": "#/$defs/hookList" }, + "Stop": { "$ref": "#/$defs/hookList" }, + "TurnStart": { "$ref": "#/$defs/hookList" }, + "TurnEnd": { "$ref": "#/$defs/hookList" }, + "Notification": { "$ref": "#/$defs/hookList" } + } + }, + + "memory": { + "description": "Persistent cross-session memory: extraction, retrieval judging, consolidation.", + "type": "object", + "additionalProperties": false, + "properties": { + "autoExtract": { + "type": "boolean", + "default": true, + "description": "Mine finished turns for facts worth remembering." + }, + "extractEveryNRuns": { + "type": "integer", + "minimum": 1, + "default": 8, + "description": "How often extraction is even considered. Values < 1 are ignored." + }, + "retrievalJudge": { + "type": "boolean", + "default": true, + "description": "Judge retrieved memories for relevance before injecting them. Fails closed." + }, + "autoConsolidate": { + "type": "boolean", + "default": true, + "description": "One cheap merge pass per project per day. Merges only — never deletes." + }, + "consolidateMinHours": { + "type": "number", + "minimum": 0, + "description": "Minimum hours between consolidation runs." + }, + "consolidateMinSessions": { + "type": "integer", + "minimum": 0, + "description": "Minimum sessions since the last run before consolidating again." + } + } + }, + + "redirect": { + "description": "Trajectory redirection. Off by default; see specs/2026-08-26-trajectory-redirection.md.", + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean", + "default": false, + "description": "Allow redirection at all." + }, + "maxPerRun": { + "type": "integer", + "minimum": 0, + "default": 2, + "description": "Redirections per run. 0 disables as surely as enabled: false." + } + } + } + }, + + "$defs": { + "ruleList": { + "type": "array", + "items": { + "type": "string", + "description": "Rule, e.g. Bash(git *), Write(src/**), WebFetch(https://example.com/**)." + } + }, + "hookList": { + "type": "array", + "items": { + "type": "object", + "required": ["name", "command"], + "additionalProperties": false, + "properties": { + "name": { + "type": "string", + "description": "Unique within the event; also the merge key across scopes." + }, + "command": { "type": "string", "description": "Shell command to run." }, + "matcher": { + "type": "string", + "description": "Tool-name pattern: *, exact, regex, or write|edit. Default: all tools." + }, + "if": { + "type": "string", + "description": "Argument condition, Tool(pattern) — e.g. bash(git *), write(*.ts)." + }, + "shell": { "enum": ["bash", "powershell"], "default": "bash" }, + "timeout": { + "type": "number", + "default": 300, + "description": "SECONDS (multiplied by 1000 internally)." + }, + "once": { + "type": "boolean", + "default": false, + "description": "Parsed and stored, but NOT enforced — treat as reserved." + } + } + } + } + } +} From 086373748e14f76cb60303b3d6e1bf3fa1710543 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:23:40 +0530 Subject: [PATCH 12/16] docs: record the P1 outcomes in the release-readiness audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six P1 items are closed. Each entry says what was wrong, what was built, and — where it matters more — what was deliberately not built: no shared settings loader (the three-merge-rules gap stays open and is not what made a typo invisible), no wire type for the raw config, no validation stricter than the handler it guards. Step 5 of the path to 1.0 is now the eval gate. P1 #2 and #3 both change what the model sees after a compaction, so that run is the measurement, not a formality. --- beforeStable.md | 203 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 167 insertions(+), 36 deletions(-) diff --git a/beforeStable.md b/beforeStable.md index 7f3dbb09..e4e98dec 100644 --- a/beforeStable.md +++ b/beforeStable.md @@ -187,38 +187,160 @@ name buys nothing the paragraph above it does not already say. ## P1 — should fix, will not sink the release -- **SSE replay lies after a full disconnect.** `tearDownIfEmpty` - (`web/stream-subscribers.ts:265`) disposes the ring buffer when the last - subscriber leaves — the normal single-browser case. A reconnect carrying - `Last-Event-ID` then hits `if (!rec) return { gap: false, events: [] }` - (`:175`) and is told nothing was missed, losing every event produced while - away. Not even a `stream_gap` marker. Strictly worse than eviction, which at - least reports a gap. Fix: decouple record lifetime from subscriber count (TTL - after the last leaves). -- **Compaction summarizes the original task away.** `selectForCompaction` - preserves only the tail and takes `messages.slice(0, firstPreservedIndex)` for - the summary (`compaction/selector.ts:54`). No head carve-out, so the founding - instruction is compacted first and, on the next compaction, the summary of it - is re-summarized. Lossy compounding on the oldest content — the plausible - mechanism behind long-session drift off the brief. -- **Compaction summaries never see tool activity.** `MemoryService` records only - user prompts and assistant text; a tool-calling turn is stored as the stub - `[Executed N tools]` (`loop.ts:1567`). The transcript handed to the summarizer - contains none of the edits, commands, or errors that were the actual work. - Also makes two heuristic-summarizer paths dead code - (`summarizer.ts:106`, `service.ts:63`). -- **`METHODS` declares 24 of 49 implemented IPC methods**, and - `METHODS["session.send"]` declares the wrong result type. `CLAUDE.md` calls - that map the source of truth; it is not one yet. All of `memory.*`, `config.*`, - `models.*` and 8 session ops get zero compile-time checking in frontends. -- **No `-32602` invalid-params validation.** Every handler does `params as {…}` - with no runtime check, so a missing or mistyped field becomes `undefined` deep - inside and surfaces as a confusing `-32603`. -- **No JSON Schema for `settings.json`, and unknown keys are silently ignored.** - `"permission"` for `"permissions"` is indistinguishable from the outside from - the feature being broken — in a security-relevant, hand-edited file. - ---- +> **Status:** all six are done (2026-09-05). + +### 1. ~~SSE replay lies after a full disconnect~~ — DONE 2026-09-05 + +`tearDownIfEmpty` (`web/stream-subscribers.ts`) disposed the ring buffer when +the last subscriber left — the normal single-browser case. A reconnect carrying +`Last-Event-ID` then hit `if (!rec) return { gap: false, events: [] }` and was +told nothing was missed, losing every event produced while away. Not even a +`stream_gap` marker. Strictly worse than eviction, which at least reports a gap. + +**Fixed.** Record lifetime is decoupled from subscriber count: the last leave +starts a 5-minute TTL (`emptySince`) enforced by the reaper that already runs, +and a reconnect inside the window clears it and replays for real. Events emitted +while nobody is attached keep being buffered, as they already were. + +`replayForSubscriber` also stops lying when the record is genuinely gone: a +client that claims to have seen events but finds no buffer gets a **gap**, since +we cannot know what it missed. That was the same lie in the other branch. + +Tests: 4 in `web/stream-subscribers.test.ts` (replay across a full disconnect, a +gap once reaped, survival of a partial disconnect, a reconnect clearing the +TTL). `runReaperForTests(now?)` is the seam — a five-minute TTL is not +assertable against a 15s unref'd interval. + +### 2. ~~Compaction summarizes the original task away~~ — DONE 2026-09-05 + +`selectForCompaction` preserved only the tail and took `messages.slice(0, +firstPreservedIndex)` for the summary. No head carve-out, so the founding +instruction was compacted first and, on the next compaction, the summary of it +was re-summarized. Lossy compounding on the oldest content — the plausible +mechanism behind long-session drift off the brief. + +**Fixed.** The first user message is carved out and preserved verbatim, +prepended *after* the tail-trimming loop so trimming can never evict it. A head +over `maxPreserveHeadTokens` (2k) is a pasted document rather than an +instruction and is summarized as before — the carve-out is for a brief, not for +anything that happens to be first. + +Tests: 2 in `selector.test.ts` — the brief survives two consecutive +compactions (the second is where the old code re-summarized the summary of it), +and an oversized head is not pinned. + +### 3. ~~Compaction summaries never see tool activity~~ — DONE 2026-09-05 + +`MemoryService` recorded only user prompts and assistant text; a tool-calling +turn was stored as the stub `[Executed N tools]`. The transcript handed to the +summarizer contained none of the edits, commands or errors that were the actual +work, and it left `summarizer.ts`'s `extractToolCalls`/`extractFiles` matching +nothing — two paths that could not fire. + +**Fixed.** `compaction/tool-transcript.ts` renders one bounded line per call +(`Tool : -> `, in the format the summarizer already greps +for), and `MemoryService.addToolTurn` records it in place of the stub. + +The turn budget moved with it. `normalizeContent` is gone: it clipped the *tail* +of a long message, i.e. the most recent tools. The transcript drops the **oldest** +calls and says how many, which is the right end to lose. +`maxToolOutputChars` is now that per-turn budget (4k) rather than a per-output +one — it was sized for a single raw output and would have kept about four calls. + +Tests: 6 in `tool-transcript.test.ts`, one of which pins the line format against +the summarizer's own regexes so the two cannot silently part ways again. + +### 4. ~~`METHODS` declares 24 of 49 implemented IPC methods~~ — DONE 2026-09-05 + +25 of 50, in the end. `CLAUDE.md` calls that map the source of truth; it was +not one. All of `memory.*`, `config.*`, `models.*` and eight session ops got +zero compile-time checking in frontends, and `METHODS["session.send"]` declared +`StreamResponse` as its result — a shape it has never returned (the handler +resolves with the loop's result; per-token output goes over the stream channel). +Its params were missing `model`, `effort` and `agentMode` too. + +**Fixed.** All 25 are declared, `session.send` is corrected, and +`ipc/methods-coverage.test.ts` asserts the two sets are equal **in both +directions** — adding a handler without declaring it now fails the suite. That +assertion, not the entries, is what makes the source-of-truth claim true. + +Six result shapes needed wire types, since `packages/shared` cannot import from +an app: `MemoryEntry`/`MemoryType`, `MemoryGraphStats`, `RedactedConfig`, +`TurnResult`, `ExportedSession`, `ModelInfo`. They follow the existing +`SessionMeta`/`SerializedMessage` mirror pattern, and `ipc/wire-shapes.test.ts` +pins each with a type-level assignability assertion, so drift is a +`check-types` failure rather than a field that quietly stops reaching +frontends. + +Two deliberate narrowings: `TurnResult` omits `LoopResult.finalState` (the +loop's internal state machine — no frontend reads it), and there is no wire type +for the raw config, only the redacted one. Adding one would undo P0 #3. + +### 5. ~~No `-32602` invalid-params validation~~ — DONE 2026-09-05 + +Every handler read its params through `params as { … }`, a cast that checks +nothing, so a missing or mistyped field became `undefined` deep inside and +surfaced as `-32603` — an internal error, which tells the caller the server is +broken when in fact the request was. + +**Fixed.** `REQUIRED_PARAMS` in `packages/shared` declares, per method, the +params a handler genuinely cannot proceed without and their JSON types. +`handleRequest` checks it before dispatch and answers `-32602` naming the field +and both types, so a bad call is fixable from the error alone. The table is +typed `Record`, so a new method without an entry is a **compile** +error — there is no path to adding a method that skips validation. + +Scope is deliberately narrow: presence and type of the *required* params, not a +schema validator. Optional params stay the handler's business (they have +defaults), `null` counts as missing because that is how a JSON caller usually +spells "nothing" and it fails identically, and unknown params are ignored so a +newer frontend still talks to an older core. + +The rule for what goes in the table is **what the handler actually needs, not +what `METHODS` types**: `commands.list` types `projectPath` as required but +falls back to cwd, so requiring it would have rejected calls the handler would +have served. Validation that is stricter than the handler is a new bug, not a +fix. + +Tests: 9 in `ipc/validate-params.test.ts`, including the point of the exercise +(`handleRequest` returning `-32602` rather than `-32603`) and one asserting an +unknown method is still `-32601`. + +### 6. ~~No JSON Schema for `settings.json`, and unknown keys are silently ignored~~ — DONE 2026-09-05 + +**Fixed, in two halves.** + +`settings/known-keys.ts` is the one place that knows the whole shape, so it can +say "unknown key" and, for a near miss, which key was meant — +`"permission"` → *did you mean "permissions"?*. `settings/validate.ts` runs it +over both scopes once per process from the **shared hook bootstrap**, because a +second call site is exactly how `serve` and `run` diverged last time (P0 #2). + +Deliberately a *name* check, not a schema validator. Values stay each reader's +business — they already validate and default their own, and a second copy of +those rules diverges. Hook event names are left to `hooks/settings.ts`, which +already reports an unknown one with the full valid list: one good message beats +two in different words. And it warns rather than refuses, since a file with a +stray key is still a usable file. + +`schemas/settings.schema.json` is the editor half, wired up with `$schema` — +the one key FreeCode ignores on purpose. Two descriptions of the same file drift +the moment a key lands in only one, so a test asserts the schema's key set +matches `KNOWN_SETTINGS` section by section. + +Tests: 9 in `settings/known-keys.test.ts`, including the motivating typo, a +section-level typo, both deliberate non-warnings, and the drift guard. + +Docs: `/reference/settings` gained the `$schema` section, `redirect` in the +top-level table (reachable but undocumented there), and the four `memory` keys +the page never listed — `retrievalJudge`, `autoConsolidate`, +`consolidateMinHours`, `consolidateMinSessions`. Two Known-gaps bullets and +their `TODO.md` entries are closed. + +**Not adopted:** a shared settings loader. The three-loaders/three-merge-rules +gap (`TODO.md:242`) is real and still open, but collapsing four readers into one +is a refactor with its own risk surface, and it is not what made a typo +invisible. The name check fixes that without touching a single merge rule. ## Explicitly not blockers @@ -239,7 +361,16 @@ and docs do not promise them**: 2. ~~Fix the three `run.ts` issues — shared hook bootstrap, `--yes`, `.choices()`.~~ 3. ~~Redact `config.get`~~; ~~make `uninstall` keep data by default~~. 4. ~~Write the OAuth ToS stance into user-facing docs.~~ -5. `pnpm eval:gate` (needs `FREECODE_JUDGE_PROVIDER`), then tag. - -**Steps 1–4 are done (2026-09-05).** What remains before the tag is step 5: -`pnpm eval:gate` with `FREECODE_JUDGE_PROVIDER` set, then tag. +5. ~~Clear P1: SSE replay, compaction (brief + tool activity), `METHODS`, + `-32602`, settings schema.~~ +6. `pnpm eval:gate` (needs `FREECODE_JUDGE_PROVIDER`), then tag. + +**Steps 1–5 are done (2026-09-05).** All P0 and all P1 items are closed; the +suite is **1255 pass / 0 fail** and `check-types` is clean across all five +workspaces. What remains before the tag is step 6: `pnpm eval:gate` with +`FREECODE_JUDGE_PROVIDER` set, then tag. + +Note that P1 #2 and #3 both change what the model sees after a compaction, which +is a behaviour change in the sense `CLAUDE.md`'s eval-driven-development section +means. The gate run in step 6 is the measurement; if the judged suite moves, +those two are the first place to look. From d67e0fec0ecc1f46c723cf3907f613fc463ed872 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:27:28 +0530 Subject: [PATCH 13/16] docs: correct what the eval gate can measure about the P1 work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I wrote that a moved judged suite would point at the compaction changes. That is wrong: compaction fires at ~107k input tokens, the longest eval prompt is ~65 tokens, and every case runs a single turn — so no case has ever compacted or can. The transcript from P1 #3 reaches the model only through the summary (loop.ts:1446 reads .summary, not recentMessages), so with no compaction the prompt is byte-identical before and after. The gate is a pre-tag regression check here, not a measurement. Recorded the harness gap in TODO.md, with the cheap fix (a FREECODE_COMPACT_TARGET_TOKENS override in the runner) noted over the expensive one (multi-turn replay cases). --- TODO.md | 10 ++++++++++ beforeStable.md | 24 ++++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/TODO.md b/TODO.md index 54a60032..1981a4ee 100644 --- a/TODO.md +++ b/TODO.md @@ -239,6 +239,16 @@ earlier audit are not repeated here. - [ ] **Non-zero exit is ignored when a hook prints JSON.** `command.ts` checks exit `2`, then parses JSON, then checks exit `0` — so `{"block":false}` + `exit 1` continues. Honour the exit code or state that JSON overrides it. +- [ ] **No eval case can reach the compaction path.** Every case runs one turn + from a prompt of at most 260 chars; compaction fires at ~107k input tokens + (`min(contextLimit, 120k) - 13k`). So the head carve-out and the tool + transcript (both landed 2026-09-05) are unmeasurable by the harness, and a + green `eval:gate` is silent about them rather than evidence for them. The + same holds for anything else that only manifests over a long session — + drift off the brief being the motivating case. Needs a case shape that + replays a recorded multi-turn session, or a `FREECODE_COMPACT_TARGET_TOKENS` + override in the runner so a short case can be made to compact on purpose. + The second is much cheaper and probably the right first move. - [ ] **`settings.json` has three loaders and three different merge rules.** `permissions` concatenates both scopes, `hooks` override by `event + name`, `memory` takes the first definition (project → user → default). Nothing states diff --git a/beforeStable.md b/beforeStable.md index e4e98dec..b0272f51 100644 --- a/beforeStable.md +++ b/beforeStable.md @@ -370,7 +370,23 @@ suite is **1255 pass / 0 fail** and `check-types` is clean across all five workspaces. What remains before the tag is step 6: `pnpm eval:gate` with `FREECODE_JUDGE_PROVIDER` set, then tag. -Note that P1 #2 and #3 both change what the model sees after a compaction, which -is a behaviour change in the sense `CLAUDE.md`'s eval-driven-development section -means. The gate run in step 6 is the measurement; if the judged suite moves, -those two are the first place to look. +### What the gate will and will not measure + +P1 #2 and #3 change what the model sees **after a compaction**, which is a +behaviour change in the sense `CLAUDE.md`'s eval-driven-development section +means. The eval harness cannot see it, and it is worth being precise about why +rather than reading a green gate as evidence: + +- Compaction fires at `min(contextLimit, 120k) - 13k` ≈ **107k input tokens**. + The longest prompt in any suite is **260 chars (~65 tokens)** and every case + runs a single turn. No case has ever compacted, and none can. +- The transcript from #3 reaches the model only *through* the summary — + `loop.ts:1446` reads `getPromptContext().summary` and deliberately not + `recentMessages`. With no compaction there is no summary, so on every eval + case the prompt is byte-identical before and after this work. +- #1, #4, #5 and #6 involve no model call at all. + +So `eval:gate` in step 6 is a **regression check before tagging**, not a +measurement of these six changes. A green gate says they broke nothing; it says +nothing about whether #2 and #3 helped. Long-session drift is the thing they +target, and the harness has no way to express it — see the `TODO.md` entry. From fa653f1903a85a23990088aab9cf4c807269a202 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 17:40:34 +0530 Subject: [PATCH 14/16] docs(eval): record the cheaper route to compaction-boundary in its spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My TODO entry made this look like a new discovery. It is not: the harness already tracks it in two places — CATEGORIES_WITHOUT_CASES in dataset.test.ts and §9.1 of the case-registry spec, which states the objection precisely (compacting is an accident today, and an accident is not a p >= 0.99 case). A third copy in TODO.md would drift from both, so that entry is now a pointer. The idea itself belongs in §9.1 and is recorded there: the compaction threshold is already a knob (FREECODE_COMPACT_TARGET_TOKENS via getCompactTarget), so lowering it for one case turns compacting from an accident into the point of the case. It needs a per-case env key on EvalCase, which is harness work the spec says should be specified before it is built — including whether arbitrary env is allowed (it should not be) and how the gate compares a case that runs in a different environment. Not building it. Recording it where the decision will be made. --- TODO.md | 17 ++++++-------- .../specs/2026-08-29-eval-case-registry.md | 23 ++++++++++++++++++- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/TODO.md b/TODO.md index 1981a4ee..c24cab7e 100644 --- a/TODO.md +++ b/TODO.md @@ -239,16 +239,13 @@ earlier audit are not repeated here. - [ ] **Non-zero exit is ignored when a hook prints JSON.** `command.ts` checks exit `2`, then parses JSON, then checks exit `0` — so `{"block":false}` + `exit 1` continues. Honour the exit code or state that JSON overrides it. -- [ ] **No eval case can reach the compaction path.** Every case runs one turn - from a prompt of at most 260 chars; compaction fires at ~107k input tokens - (`min(contextLimit, 120k) - 13k`). So the head carve-out and the tool - transcript (both landed 2026-09-05) are unmeasurable by the harness, and a - green `eval:gate` is silent about them rather than evidence for them. The - same holds for anything else that only manifests over a long session — - drift off the brief being the motivating case. Needs a case shape that - replays a recorded multi-turn session, or a `FREECODE_COMPACT_TARGET_TOKENS` - override in the runner so a short case can be made to compact on purpose. - The second is much cheaper and probably the right first move. +- [ ] **No eval case can reach the compaction path** — so the head carve-out and + the tool transcript (both landed 2026-09-05) are unmeasurable end to end, + and a green `eval:gate` is silent about them rather than evidence for them. + Already tracked where it belongs: `compaction-boundary` sits in + `CATEGORIES_WITHOUT_CASES` (`eval/dataset.test.ts`) and in §9.1 of + `specs/2026-08-29-eval-case-registry.md`, which is the spec to change + first. Not repeated here — see that row for the mechanism. - [ ] **`settings.json` has three loaders and three different merge rules.** `permissions` concatenates both scopes, `hooks` override by `event + name`, `memory` takes the first definition (project → user → default). Nothing states diff --git a/docs/superpowers/specs/2026-08-29-eval-case-registry.md b/docs/superpowers/specs/2026-08-29-eval-case-registry.md index b3557a21..53cb40b0 100644 --- a/docs/superpowers/specs/2026-08-29-eval-case-registry.md +++ b/docs/superpowers/specs/2026-08-29-eval-case-registry.md @@ -520,7 +520,7 @@ most expensive kind of wrong answer this harness can give. | Category | What it needs first | | --- | --- | -| `compaction-boundary` | A turn long enough to compact. Reachable only by accident today, and an accident is not a p ≥ 0.99 case. | +| `compaction-boundary` | A turn long enough to compact. Reachable only by accident today, and an accident is not a p ≥ 0.99 case — but see the note below: it may be cheaper than "larger" suggests. | | `memory-recall` | A seeded memory dir. `files` paths are sandbox-relative and `assertSafeRelativePath` refuses to escape, which is correct — so a fixture cannot reach `~/.freecode`. | | `resume` | A prior session to resume from. One `runEffect` per trial means there is no earlier turn. | | `mcp-failure` | A fixture MCP server. `initRunner` calls `initMcpServers()` against the user's real config, so the suite is not hermetic here and could not be made to fail on purpose. | @@ -531,6 +531,27 @@ The cheapest unlock is `resume` (a second `runEffect` on the same session id); larger. None of it is Phase 4 work — it is harness work, and it should be specified before it is built. +**A cheaper route to `compaction-boundary` (noted 2026-09-05, not built.)** The +objection above is that compacting is an *accident*, and an accident cannot +carry a p ≥ 0.99 case. But the threshold is already a knob: +`getCompactTarget()` reads `FREECODE_COMPACT_TARGET_TOKENS`, and +`shouldCompact` takes `min(windowLimit, target) - buffer`. Set the target to a +few thousand for one case and compaction stops being an accident and becomes +the *point* of the case — deterministic, and reached by an ordinary short +prompt instead of a manufactured 100k one. + +What it needs: a per-case `env` key on `EvalCase`, honoured by `runner.ts` and +scoped to the trial. That is a real harness change and wants its own spec pass — +in particular, whether a case may set arbitrary env (it should not; an allowlist +of compaction knobs is the safe shape) and how the gate treats a case whose +environment differs from every other case's. + +Why it matters now: the 2026-09-05 compaction changes — the head carve-out in +`selectForCompaction` and the tool transcript from `addToolTurn` — are exactly +what this category would measure, and today nothing does. They ship on unit +tests and reasoning, which is weaker evidence than this suite exists to +provide. + **Every phase in this spec is now built.** What is left is not in this spec: the harness capability §9.1 names, and plan §3's scripted provider. Phases 1–3 improved how the suite reports and Phase 4 added the coverage that could be added without new harness capability. Phase 1 changes what the trajectory suite measures, so run it **before** any From 4eca844cea13da45cfe67aab190906add321f649 Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 18:22:50 +0530 Subject: [PATCH 15/16] chore: drop the v1 release-readiness audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All eleven items (five P0, six P1) are closed and their outcomes live in the commits that made them, so the audit file was a duplicate record with a shelf life. `pnpm eval:gate` opened all three suites against anthropic/claude-opus-5 — the last step it was tracking. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- beforeStable.md | 392 ------------------------------------------------ 1 file changed, 392 deletions(-) delete mode 100644 beforeStable.md diff --git a/beforeStable.md b/beforeStable.md deleted file mode 100644 index b0272f51..00000000 --- a/beforeStable.md +++ /dev/null @@ -1,392 +0,0 @@ -# Before Stable (v1.0) - -Release-readiness audit, 2026-09-05, against `main` at `360d1e5` (version `0.29.0`). - -## Health: green - -| Check | Result | -| -------------------- | --------------------------------------------------- | -| `pnpm check-types` | clean across all workspaces | -| `pnpm test` | **1206 pass / 0 fail**, 30 suites, ~30s | -| CI (`ci.yml`) | lint + typecheck + build + test on every PR | -| Eval gate (`v0.27.1`)| all three suites opened the gate | - -The engine is stable. What's left for v1 is **the front door and the headless -path**, not correctness. - ---- - -## P0 — real v1 blockers - -> **Status:** all five are done (2026-09-05). - -### 1. ~~`README.md` describes a different product~~ — DONE 2026-09-05 - -It pitches FreeCode as a tool that "drives AI coding assistants via browser -automation" using a "two-phase approach: the AI first returns which files it -needs, then receives those files". That is the legacy Playwright path that -`CLAUDE.md` explicitly marks as **not wired into the primary path**. The actual -product is a ~198-provider API agent loop with a single agentic tool-use loop. - -Anyone landing on the repo for v1 reads a stale pitch for a legacy subsystem. - -**Fixed.** `README.md` was rewritten against what ships: the real installer, all -16 tools, the command list, the four-frontend architecture, and the ~198-provider -driver. The browser path is kept as a blockquote noting it exists and is not the -default execution path. Three claims were corrected against source while writing -it — `freecode session` is list + delete only, `freecode memory` is graph -stats/rebuild/UI, and `freecode update` is injected by the TUI -(`apps/tui/src/entry.ts:194`), not registered in `create-cli.ts`. - -### 2. ~~Headless `freecode run` is broken in three ways~~ — DONE 2026-09-05 - -`apps/core/src/cli/commands/run.ts` - -- **No hooks.** `HookSettingsManager` + `registerRtkHook()` are constructed only - in `startServer()` (`server.ts:1260-1263`), so a headless run loads no - `settings.json` hooks. The formatter that fires after every edit interactively - silently does not fire in CI — same repo, two behaviours. Fix: move both into a - shared bootstrap the `run` handler also calls. -- **`build` mode denies every write.** `askPermission` rejects immediately when - no frontend is listening (`bus/index.ts:413`) and `promptForPermission` maps - that to deny, while `build`'s default for mutating tools is `ask`. So - `freecode run "fix the test"` reads fine and is denied every edit. Needs a - `--yes` / `--allow ` flag, or at minimum a one-time explanation on the - first headless denial. -- **`--agent` is an unchecked cast** (`run.ts:146`). `--agent buld` falls through - `modeDefault`'s `default` branch and runs with **build** semantics. Add yargs - `.choices()`, as `mcp add`'s `type` already has. - -If v1 is meant to be scriptable at all, these three are the difference between -"works" and "works interactively only". - -**Fixed.** - -- Hooks moved into `apps/core/src/hooks/bootstrap.ts` (`initHooks`), called by - both `startServer()` and the `run` handler. Only `serve` passes `watch: true`; - a one-shot run exits before a settings edit could apply, and would otherwise - hold an fs watcher open past its last turn. -- `--yes` (`-y`) answers the **ask** tier with allow, and repeatable - `--allow ` adds in-memory session grants. Both ride into the loop on new - `AgentLoopConfig.autoApproveAsks` / `.sessionGrants` fields. Scoped to the ask - tier deliberately: a deny rule and a read-only mode still refuse, because those - are decisions someone already made, not questions waiting for an answer. -- `--agent` gained yargs `.choices()`, so `--agent buld` exits with a usage error - instead of running as `build`. The cast at the read site stays — it matches - `mcp add`'s house style and is now backed by a real runtime check. - -Tests: `agent/headless-permission.test.ts` (5 cases — the first asserts the -unattended default is still deny, two cover `--yes` and `--allow` landing a real -write on disk, two assert `--yes` does not beat a deny rule or plan mode) and -`hooks/bootstrap.test.ts` (2). Full suite: **1213 pass / 0 fail**. - -Docs updated: `/reference/cli` (flag table, a three-option scripting section, -and the incorrect "there is no `--max-turns`" line), `/reference/settings`, -`/reference/hook-events`, and the three matching `TODO.md` entries. - -### 3. ~~`config.get` returns API keys verbatim~~ — DONE 2026-09-05 - -`server.ts` — `"config.get"` returned `readConfig()` with no redaction. The full -JSON-RPC surface is reachable over `web-server.ts`'s `POST /api`. It is -token-gated and loopback by default, but `host` is a parameter -(`web-server.ts:108`), so one `--host 0.0.0.0` turned a debug convenience into -key exfiltration. - -**Fixed.** `redactConfig()` in `providers/config.ts` builds the safe view and -`config.get` returns that: `{ hasApiKey, model?, authMode? }` per provider entry, -`{ hasCredential }` per `web` entry, with `current`, `lastAgentMode` and -`recovery` passed through. It is an **allowlist assembled field by field**, not a -blocklist of known secret names — `WebCredentials` has grown a secret-bearing -field twice, and a blocklist is wrong the day it grows a third. - -No consumer lost anything: the only reader of the raw shape was the TUI's -unused `getConfig()` wrapper, whose `ConfigInfo` type now matches. Every other -"is a provider set up" question already went through `providers.list`'s -`hasApiKey`, which never carried the key. - -Tests: `providers/config-redaction.test.ts` (4 — one asserts no secret from any -block survives serialization, one pins the exact kept shape, one that an -anonymous web session reads as `hasCredential: false` rather than a missing -block, one that an empty config stays empty). Full suite: **1217 pass / 0 fail**. - -Docs updated: `/reference/ipc-methods` (result column, the redaction paragraph -that replaced the "API keys included" warning, and the matching Known-gaps -bullet) and the `TODO.md` entry. - -### 4. ~~`freecode uninstall` deletes user data on one `y`~~ — DONE 2026-09-05 - -Removed all of `~/.freecode` — sessions, rollout logs, memory, history, usage — -and the prompt did not say so. No `--keep-data`, no backup. - -**Fixed.** `scripts/uninstall.sh` already drew the right line (`--purge` for a -full wipe, data kept otherwise); the CLI command was the copy that got it wrong, -so the fix was to make them agree rather than invent a third behaviour. The -default now takes the launcher and `~/.freecode/builds` — the program — and -leaves everything else. `--purge` takes the directory, and the line it prints -names what is in it rather than the path alone. `--dry-run` was added, and -`--force` gained `-y`/`--yes` so the two entry points accept the same flags. - -`--keep-data` was not added: keeping data is now the default, and a flag for the -default reads like the other behaviour is still lurking. - -Two smaller things fell out. A non-TTY stdin resolved `readline.question` at once -with an empty answer, which the old code read as "no" — correct by accident, and -silent; it is now an error naming `--force`. And the handler no longer branches -on `isDirectory()` before choosing `rmSync` vs `unlinkSync`, since `rmSync` with -`recursive` handles both. - -Tests: `cli/commands/uninstall.test.ts` (5, against a pure `planUninstall()` so -the safety rule is asserted without deleting anything — the default plan excludes -the data dir, `--purge` includes it *and* labels it with "sessions"/"memory", -purge does not double-list `builds`, an absent launcher is not listed, and an -uninstalled machine plans nothing in either mode). Full suite: **1222 pass / 0 -fail**. - -Docs updated: `/reference/cli` (flag table + rewritten section), the -`/getting-started/installation` uninstall block, and the `~/.freecode` note on -`/reference`. The **separate** gap that `uninstall` ignores `FREECODE_HOME` / -`FREECODE_INSTALL_DIR` and misses the Windows launcher path is untouched and -still tracked (`TODO.md:578`); it is a completeness bug, not a data-loss one. - -### 5. ~~Anthropic OAuth ToS risk is unresolved~~ — DONE 2026-09-05 - -§0.1 of the OAuth spec was the risk section and had no resolution. A v1 that -ships subscription auth as a headline feature needs an explicit, user-visible -stance — not a spec paragraph. - -**Resolved: ship it, state the risk plainly, do not euphemize.** Written into -the spec as §0.2 and, more importantly, published: - -- **`/getting-started/anthropic-subscription`** — a new page whose second - section is the risk, before any instructions. It says FreeCode presents itself - to Anthropic *as Claude Code*, that Anthropic reserves subscription inference - for its own surfaces and has acted against tools doing this, and that the - account at risk is the user's. Then: the three opt-ins and the narrow fourth - (no key configured + a freecode login — an imported Claude Code login - deliberately does not count), the login flow, `status`/`logout`, the - org-forbidden 403 latch and API-key fallback, and why cost is stamped on the - call. -- **`/reference/cli`** gained a `freecode auth` section — the command was - undocumented entirely. -- **`/reference/env`** gained `FREECODE_ANTHROPIC_AUTH`, flagged as an opt-in - rather than a neutral switch. -- **`README.md`** — a warning callout beside `freecode auth login anthropic`, - and the pitch line up top now says opt-in and links down to it. - -The code needed nothing: `auth.ts` already printed the §0.1 disclosure on login -and the identity block was already quarantined to the OAuth path with a test. -The gap was that a user could only read any of this *after* deciding to run the -command. The three §0.1 constraints (opt-in never default, identity block -quarantined, first-run disclosure) stay invariants. - -Not adopted: a y/N confirmation on login. §0.1 chose "print once, no repeated -nagging" deliberately, and a prompt in front of a command the user just typed by -name buys nothing the paragraph above it does not already say. - ---- - -## P1 — should fix, will not sink the release - -> **Status:** all six are done (2026-09-05). - -### 1. ~~SSE replay lies after a full disconnect~~ — DONE 2026-09-05 - -`tearDownIfEmpty` (`web/stream-subscribers.ts`) disposed the ring buffer when -the last subscriber left — the normal single-browser case. A reconnect carrying -`Last-Event-ID` then hit `if (!rec) return { gap: false, events: [] }` and was -told nothing was missed, losing every event produced while away. Not even a -`stream_gap` marker. Strictly worse than eviction, which at least reports a gap. - -**Fixed.** Record lifetime is decoupled from subscriber count: the last leave -starts a 5-minute TTL (`emptySince`) enforced by the reaper that already runs, -and a reconnect inside the window clears it and replays for real. Events emitted -while nobody is attached keep being buffered, as they already were. - -`replayForSubscriber` also stops lying when the record is genuinely gone: a -client that claims to have seen events but finds no buffer gets a **gap**, since -we cannot know what it missed. That was the same lie in the other branch. - -Tests: 4 in `web/stream-subscribers.test.ts` (replay across a full disconnect, a -gap once reaped, survival of a partial disconnect, a reconnect clearing the -TTL). `runReaperForTests(now?)` is the seam — a five-minute TTL is not -assertable against a 15s unref'd interval. - -### 2. ~~Compaction summarizes the original task away~~ — DONE 2026-09-05 - -`selectForCompaction` preserved only the tail and took `messages.slice(0, -firstPreservedIndex)` for the summary. No head carve-out, so the founding -instruction was compacted first and, on the next compaction, the summary of it -was re-summarized. Lossy compounding on the oldest content — the plausible -mechanism behind long-session drift off the brief. - -**Fixed.** The first user message is carved out and preserved verbatim, -prepended *after* the tail-trimming loop so trimming can never evict it. A head -over `maxPreserveHeadTokens` (2k) is a pasted document rather than an -instruction and is summarized as before — the carve-out is for a brief, not for -anything that happens to be first. - -Tests: 2 in `selector.test.ts` — the brief survives two consecutive -compactions (the second is where the old code re-summarized the summary of it), -and an oversized head is not pinned. - -### 3. ~~Compaction summaries never see tool activity~~ — DONE 2026-09-05 - -`MemoryService` recorded only user prompts and assistant text; a tool-calling -turn was stored as the stub `[Executed N tools]`. The transcript handed to the -summarizer contained none of the edits, commands or errors that were the actual -work, and it left `summarizer.ts`'s `extractToolCalls`/`extractFiles` matching -nothing — two paths that could not fire. - -**Fixed.** `compaction/tool-transcript.ts` renders one bounded line per call -(`Tool : -> `, in the format the summarizer already greps -for), and `MemoryService.addToolTurn` records it in place of the stub. - -The turn budget moved with it. `normalizeContent` is gone: it clipped the *tail* -of a long message, i.e. the most recent tools. The transcript drops the **oldest** -calls and says how many, which is the right end to lose. -`maxToolOutputChars` is now that per-turn budget (4k) rather than a per-output -one — it was sized for a single raw output and would have kept about four calls. - -Tests: 6 in `tool-transcript.test.ts`, one of which pins the line format against -the summarizer's own regexes so the two cannot silently part ways again. - -### 4. ~~`METHODS` declares 24 of 49 implemented IPC methods~~ — DONE 2026-09-05 - -25 of 50, in the end. `CLAUDE.md` calls that map the source of truth; it was -not one. All of `memory.*`, `config.*`, `models.*` and eight session ops got -zero compile-time checking in frontends, and `METHODS["session.send"]` declared -`StreamResponse` as its result — a shape it has never returned (the handler -resolves with the loop's result; per-token output goes over the stream channel). -Its params were missing `model`, `effort` and `agentMode` too. - -**Fixed.** All 25 are declared, `session.send` is corrected, and -`ipc/methods-coverage.test.ts` asserts the two sets are equal **in both -directions** — adding a handler without declaring it now fails the suite. That -assertion, not the entries, is what makes the source-of-truth claim true. - -Six result shapes needed wire types, since `packages/shared` cannot import from -an app: `MemoryEntry`/`MemoryType`, `MemoryGraphStats`, `RedactedConfig`, -`TurnResult`, `ExportedSession`, `ModelInfo`. They follow the existing -`SessionMeta`/`SerializedMessage` mirror pattern, and `ipc/wire-shapes.test.ts` -pins each with a type-level assignability assertion, so drift is a -`check-types` failure rather than a field that quietly stops reaching -frontends. - -Two deliberate narrowings: `TurnResult` omits `LoopResult.finalState` (the -loop's internal state machine — no frontend reads it), and there is no wire type -for the raw config, only the redacted one. Adding one would undo P0 #3. - -### 5. ~~No `-32602` invalid-params validation~~ — DONE 2026-09-05 - -Every handler read its params through `params as { … }`, a cast that checks -nothing, so a missing or mistyped field became `undefined` deep inside and -surfaced as `-32603` — an internal error, which tells the caller the server is -broken when in fact the request was. - -**Fixed.** `REQUIRED_PARAMS` in `packages/shared` declares, per method, the -params a handler genuinely cannot proceed without and their JSON types. -`handleRequest` checks it before dispatch and answers `-32602` naming the field -and both types, so a bad call is fixable from the error alone. The table is -typed `Record`, so a new method without an entry is a **compile** -error — there is no path to adding a method that skips validation. - -Scope is deliberately narrow: presence and type of the *required* params, not a -schema validator. Optional params stay the handler's business (they have -defaults), `null` counts as missing because that is how a JSON caller usually -spells "nothing" and it fails identically, and unknown params are ignored so a -newer frontend still talks to an older core. - -The rule for what goes in the table is **what the handler actually needs, not -what `METHODS` types**: `commands.list` types `projectPath` as required but -falls back to cwd, so requiring it would have rejected calls the handler would -have served. Validation that is stricter than the handler is a new bug, not a -fix. - -Tests: 9 in `ipc/validate-params.test.ts`, including the point of the exercise -(`handleRequest` returning `-32602` rather than `-32603`) and one asserting an -unknown method is still `-32601`. - -### 6. ~~No JSON Schema for `settings.json`, and unknown keys are silently ignored~~ — DONE 2026-09-05 - -**Fixed, in two halves.** - -`settings/known-keys.ts` is the one place that knows the whole shape, so it can -say "unknown key" and, for a near miss, which key was meant — -`"permission"` → *did you mean "permissions"?*. `settings/validate.ts` runs it -over both scopes once per process from the **shared hook bootstrap**, because a -second call site is exactly how `serve` and `run` diverged last time (P0 #2). - -Deliberately a *name* check, not a schema validator. Values stay each reader's -business — they already validate and default their own, and a second copy of -those rules diverges. Hook event names are left to `hooks/settings.ts`, which -already reports an unknown one with the full valid list: one good message beats -two in different words. And it warns rather than refuses, since a file with a -stray key is still a usable file. - -`schemas/settings.schema.json` is the editor half, wired up with `$schema` — -the one key FreeCode ignores on purpose. Two descriptions of the same file drift -the moment a key lands in only one, so a test asserts the schema's key set -matches `KNOWN_SETTINGS` section by section. - -Tests: 9 in `settings/known-keys.test.ts`, including the motivating typo, a -section-level typo, both deliberate non-warnings, and the drift guard. - -Docs: `/reference/settings` gained the `$schema` section, `redirect` in the -top-level table (reachable but undocumented there), and the four `memory` keys -the page never listed — `retrievalJudge`, `autoConsolidate`, -`consolidateMinHours`, `consolidateMinSessions`. Two Known-gaps bullets and -their `TODO.md` entries are closed. - -**Not adopted:** a shared settings loader. The three-loaders/three-merge-rules -gap (`TODO.md:242`) is real and still open, but collapsing four readers into one -is a refactor with its own risk surface, and it is not what made a typo -invisible. The name check fixes that without touching a single merge rule. - -## Explicitly not blockers - -These are v1.1 features, not "v1 is broken without them" — **provided the README -and docs do not promise them**: - -- `autonomous/` — Phase 0 only (types, budget, storage). Nothing executes. -- MCP server (expose). The client side is done. -- Checkpoints / rewind. -- User-defined slash commands and subagents. -- Multimodal input beyond what already landed. - ---- - -## Suggested path to 1.0 - -1. ~~Rewrite `README.md` against what actually ships.~~ -2. ~~Fix the three `run.ts` issues — shared hook bootstrap, `--yes`, `.choices()`.~~ -3. ~~Redact `config.get`~~; ~~make `uninstall` keep data by default~~. -4. ~~Write the OAuth ToS stance into user-facing docs.~~ -5. ~~Clear P1: SSE replay, compaction (brief + tool activity), `METHODS`, - `-32602`, settings schema.~~ -6. `pnpm eval:gate` (needs `FREECODE_JUDGE_PROVIDER`), then tag. - -**Steps 1–5 are done (2026-09-05).** All P0 and all P1 items are closed; the -suite is **1255 pass / 0 fail** and `check-types` is clean across all five -workspaces. What remains before the tag is step 6: `pnpm eval:gate` with -`FREECODE_JUDGE_PROVIDER` set, then tag. - -### What the gate will and will not measure - -P1 #2 and #3 change what the model sees **after a compaction**, which is a -behaviour change in the sense `CLAUDE.md`'s eval-driven-development section -means. The eval harness cannot see it, and it is worth being precise about why -rather than reading a green gate as evidence: - -- Compaction fires at `min(contextLimit, 120k) - 13k` ≈ **107k input tokens**. - The longest prompt in any suite is **260 chars (~65 tokens)** and every case - runs a single turn. No case has ever compacted, and none can. -- The transcript from #3 reaches the model only *through* the summary — - `loop.ts:1446` reads `getPromptContext().summary` and deliberately not - `recentMessages`. With no compaction there is no summary, so on every eval - case the prompt is byte-identical before and after this work. -- #1, #4, #5 and #6 involve no model call at all. - -So `eval:gate` in step 6 is a **regression check before tagging**, not a -measurement of these six changes. A green gate says they broke nothing; it says -nothing about whether #2 and #3 helped. Long-session drift is the thing they -target, and the harness has no way to express it — see the `TODO.md` entry. From c39d35578b614ccc02d61ccbc99317b836b6eece Mon Sep 17 00:00:00 2001 From: Ayan De Date: Sat, 5 Sep 2026 18:30:44 +0530 Subject: [PATCH 16/16] fix(eval): score read-before-explaining-edit on sequencing, not model temperament MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The case exists to assert that the model reads before opining — its `whyModelBacked` says so. What it actually blocked on was `expectMaxTurns: 6`, a budget calibrated against minimax/MiniMax-M3, the only model it had ever run on (18/18 passes). Its first run on claude-opus-5 came in at 9, 5 and 10 turns and failed the majority. The trace says that is thoroughness, not thrashing: 17 tool calls over 9 turns, `repeatedCalls: 0`, no oscillation, monotonically widening from the named file to its consumers to the tests and spec, ending in a real answer. The prompt ("what else would that affect") has no natural stopping point, so turn count here measures how much a model explores, which differs per model — while the gate's own regression rule is already per-model. So: assert the sequencing directly with `expectFirstToolIn: ["read"]`, which is model-independent, and raise the budget to 12 so it still catches a loop. Note `expectMaxTurns` is checked before the tool assertions (`scorers/trajectory.ts:29`), so the old failure never evaluated whether `read` fired first — the trace confirms it did. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cy5ynPzdM4gJ8o1qKpp5uX --- evals/trajectory.jsonl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evals/trajectory.jsonl b/evals/trajectory.jsonl index 86d26d27..e4d9125f 100644 --- a/evals/trajectory.jsonl +++ b/evals/trajectory.jsonl @@ -56,7 +56,7 @@ {"id": "read-respects-path-arg", "prompt": "Show me the first 30 lines of apps/core/src/eval/match.ts.", "failureCategory": "tool-routing", "whyModelBacked": "The tool a prompt should open with is chosen by the model from the prompt and the system prompt, not by deterministic code — there is no function to unit test.", "expectTool": "read", "expectFirstToolIn": ["read"], "expectInArgs": {"filePath": "eval/match.ts"}, "expectMaxTurns": 4, "forbidTools": ["write", "edit"]} {"id": "todowrite-for-multistep", "prompt": "Planning-only task: immediately create a three-step todo list for adding Groq, Mistral, and Cohere providers, documenting them, and updating tests. Do not inspect or research the repository, ask questions, or begin implementation. Stop after creating the list.", "failureCategory": "tool-routing", "whyModelBacked": "Whether a multi-step request triggers a plan before exploration is a prompt-shaped decision; `todowrite` itself is trivially unit tested and proves nothing about when it fires.", "expectTool": "todowrite", "forbidTools": ["read", "ls", "glob", "grep", "bash", "webfetch", "websearch", "question", "write", "edit"]} {"id": "grep-scoped-to-path", "prompt": "Search only inside apps/core/src/permission for the string PATH_TOOLS.", "failureCategory": "tool-routing", "whyModelBacked": "The tool a prompt should open with is chosen by the model from the prompt and the system prompt, not by deterministic code — there is no function to unit test.", "expectTool": "grep", "expectFirstToolIn": ["grep"], "expectInArgs": {"pattern": "PATH_TOOLS"}, "expectMaxTurns": 4, "forbidTools": ["write", "edit"]} -{"id": "read-before-explaining-edit", "prompt": "I want to change HANG_THRESHOLD_MS in apps/core/src/rollout/trace.ts to 400000. Read the file first and tell me what else that would affect.", "failureCategory": "tool-routing", "whyModelBacked": "Reading before opining is a habit the system prompt asks for and the model may skip; nothing deterministic sequences it.", "agentMode": "explore", "expectTool": "read", "expectMaxTurns": 6, "forbidTools": ["write", "edit"]} +{"id": "read-before-explaining-edit", "prompt": "I want to change HANG_THRESHOLD_MS in apps/core/src/rollout/trace.ts to 400000. Read the file first and tell me what else that would affect.", "failureCategory": "tool-routing", "whyModelBacked": "Reading before opining is a habit the system prompt asks for and the model may skip; nothing deterministic sequences it.", "agentMode": "explore", "expectTool": "read", "expectFirstToolIn": ["read"], "expectMaxTurns": 12, "forbidTools": ["write", "edit"]} {"id": "glob-then-stop", "prompt": "How many .ts files are directly inside apps/core/src/eval? Use glob and tell me the count.", "failureCategory": "tool-routing", "whyModelBacked": "The tool a prompt should open with is chosen by the model from the prompt and the system prompt, not by deterministic code — there is no function to unit test.", "expectTool": "glob", "expectFirstToolIn": ["glob"], "expectMaxTurns": 4, "forbidTools": ["write", "edit"]} {"id": "grep-for-a-function-name", "prompt": "Which file defines the function proposeQuarantine?", "failureCategory": "tool-routing", "whyModelBacked": "The tool a prompt should open with is chosen by the model from the prompt and the system prompt, not by deterministic code — there is no function to unit test.", "expectTool": "grep", "expectFirstToolIn": ["grep", "glob"], "expectInArgs": {"pattern": "proposeQuarantine"}, "expectMaxTurns": 4, "forbidTools": ["write", "edit"]} {"id": "read-to-summarise", "prompt": "Summarise in two sentences what apps/core/src/eval/gate.ts does.", "failureCategory": "tool-routing", "whyModelBacked": "The tool a prompt should open with is chosen by the model from the prompt and the system prompt, not by deterministic code — there is no function to unit test.", "expectTool": "read", "expectFirstToolIn": ["read"], "expectInArgs": {"filePath": "gate.ts"}, "expectMaxTurns": 4, "forbidTools": ["write", "edit"]}