From 9b56e0b3c575d1c8a766a152c0f2fcf4d0b12132 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:24:40 -0300 Subject: [PATCH 01/14] feat(history): per-instance JSONL store, project identity, storage tests Slice 1/6 of the PR #819 split (maintainer-requested review slices). - atomic-write: same-dir tmp+rename JSON writer, concurrent-instance-safe staging names, never throws - store: project identity (realpath+sha256[:16], raw-path fallback, 24-char collision re-key), project/seed/global/registry path derivations, advisory registry with fail-open reads and atomic writes, tolerant JSONL line parser, lazy per-instance session writer with command filtering - extension entry: identity constants and capture-only wiring (before_agent_start -> appendSessionCapture); migration, seeding, selector, deletion, and GC join in later slices - tests: 32 node:test cases covering storage concurrency and recovery (parallel writers, interleaved captures, burst order integrity, torn-line matrix + crash-tail recovery window, rapid same-target atomic writes with zero staging residue, two-instance registry interleaving, collision re-key, corrupt/wrong-shape fail-open) - test vectors are machine-independent: literal cwds exercise the documented raw-string fallback identically on every platform Gates: scoped history tests 32/32 green. verify-package-files and package-manifest failures are pre-existing environmental (gitignored contracts/.DS_Store; missing node_modules) and reproduce on vanilla origin/main. --- extensions/history/atomic-write.ts | 38 +++++ extensions/history/index.ts | 60 +++++++ extensions/history/store.ts | 240 +++++++++++++++++++++++++++ tests/history-atomic-write.test.ts | 57 +++++++ tests/history-multi-reader.test.ts | 203 ++++++++++++++++++++++ tests/history-registry.test.ts | 109 ++++++++++++ tests/history-session-writer.test.ts | 109 ++++++++++++ tests/history-store-paths.test.ts | 79 +++++++++ 8 files changed, 895 insertions(+) create mode 100644 extensions/history/atomic-write.ts create mode 100644 extensions/history/index.ts create mode 100644 extensions/history/store.ts create mode 100644 tests/history-atomic-write.test.ts create mode 100644 tests/history-multi-reader.test.ts create mode 100644 tests/history-registry.test.ts create mode 100644 tests/history-session-writer.test.ts create mode 100644 tests/history-store-paths.test.ts diff --git a/extensions/history/atomic-write.ts b/extensions/history/atomic-write.ts new file mode 100644 index 000000000..cc6ae8147 --- /dev/null +++ b/extensions/history/atomic-write.ts @@ -0,0 +1,38 @@ +import path from "node:path"; +// SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history +// SPDX-License-Identifier: MIT + +import fs from "node:fs"; + +/** + * Shared atomic JSON writer (design §D3): serialize to a `.tmp` file in the + * SAME directory as the target, then renameSync it into place — a same-dir + * rename is atomic on POSIX/APFS, so readers see the old or the new file, + * never a partial write. Any error returns false and never throws. + * + * No fsync: both consumers treat lost writes as derived cache (a lost index + * rebuilds on the next open; a lost tombstone resurfaces a prompt the user + * can re-delete), so the per-write fsync cost is not justified — the crash + * window is documented, not fixed. The staging name is unique per write + * (`.tmp--`, the same convention as the store.ts writers): the + * state dir is shared across concurrent pi instances, so a fixed + * `${filePath}.tmp` would let two writers clobber the same staging file + * (torn target JSON, spurious rename failures). A failed write unlinks its + * staging file, so orphaned `.tmp` files do not accumulate. + */ +export function writeJsonAtomic(filePath: string, value: unknown): boolean { + const tmpPath = `${filePath}.tmp-${process.pid}-${Date.now()}`; + try { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(tmpPath, JSON.stringify(value), "utf8"); + fs.renameSync(tmpPath, filePath); + return true; + } catch { + try { + fs.unlinkSync(tmpPath); + } catch { + // staging file never created or already renamed + } + return false; + } +} diff --git a/extensions/history/index.ts b/extensions/history/index.ts new file mode 100644 index 000000000..26616733f --- /dev/null +++ b/extensions/history/index.ts @@ -0,0 +1,60 @@ +// SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history +// SPDX-License-Identifier: MIT + +// Prompt-history extension entry (slice 1): identity constants, the +// per-instance writer lifecycle, and the before_agent_start capture +// handler. Selector UI, shortcut/command, scope drains, legacy migration +// and seed bootstrap, and GC arrive in later slices. + +import { randomUUID } from "node:crypto"; +import { homedir } from "node:os"; +import { join } from "node:path"; +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import { + appendSessionCapture, + ensureRegistryEntry, + openSessionWriter, + type SessionWriterState, +} from "./store.ts"; + +// v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). +const PI_HISTORY_ROOT = join(homedir(), ".pi", "agent", "history"); +const AGENT_DIR = join(homedir(), ".pi", "agent"); +const CURRENT_CWD = process.cwd(); +// Instance identity: one exclusive capture file per pi process. +const INSTANCE_ID = randomUUID(); + +let writerState: SessionWriterState | null = null; + +/** + * One-time init per extension load: register the project in the advisory + * registry, then open this instance's exclusive capture file. Legacy + * migration and seed bootstrap join this init order in a later slice. + */ +function getWriter(): SessionWriterState { + if (!writerState) { + try { + ensureRegistryEntry(PI_HISTORY_ROOT, CURRENT_CWD); + } catch { + // registry is advisory + } + writerState = openSessionWriter(PI_HISTORY_ROOT, CURRENT_CWD, INSTANCE_ID); + } + return writerState; +} + +export default function promptHistoryExtension(pi: ExtensionAPI) { + // One writer per extension load; see getWriter() for the init order. + + // Persist every delivered user prompt (write-through, append-only JSONL). + // The local ExtensionAPI stub types handler args as unknown; narrow here. + pi.on("before_agent_start", (...args: unknown[]) => { + try { + const event = args[0] as { prompt?: string } | undefined; + appendSessionCapture(getWriter(), event?.prompt ?? "", Date.now()); + } catch { + // A capture failure must never break the agent loop or unregister + // the handler - swallow and keep the next prompt capturable. + } + }); +} diff --git a/extensions/history/store.ts b/extensions/history/store.ts new file mode 100644 index 000000000..8631707b9 --- /dev/null +++ b/extensions/history/store.ts @@ -0,0 +1,240 @@ +// SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history +// SPDX-License-Identifier: MIT + +// Consolidated multi-concurrency store (v2), slice 1: project paths and +// identity, the advisory registry, entry primitives, and the per-instance +// session writer. Scope drains/deletes, legacy migration and seed +// bootstrap, and GC/compaction arrive in later slices. +// Formerly store-paths.ts + registry.ts + multi-store.ts (+ v1 primitives). + +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; + +// =========================================================================== +// Paths (formerly store-paths.ts) +// =========================================================================== + +/** + * Project identity for the multi-concurrency store (design v2). + * + * The cwd is canonicalized through realpath — the same resolution pi's + * session-manager applies — so symlinked or differently-spelled paths to one + * project merge into a single identity. A failed resolution (deleted cwd) + * falls back to hashing the raw string: identity degrades, never throws. + */ +export function projectHash(cwd: string): string { + let canonical = cwd; + try { + canonical = fs.realpathSync(cwd); + } catch { + // fall back to the raw path + } + return createHash("sha256").update(canonical).digest("hex").slice(0, 16); +} + +/** The project's directory under the store root. */ +export function projectDir(root: string, cwd: string): string { + return path.join(root, "projects", projectHash(cwd)); +} + +/** The capture file owned by one pi instance (per session/process). */ +export function sessionFilePath( + root: string, + cwd: string, + instanceId: string, +): string { + return path.join(projectDir(root, cwd), `${instanceId}.jsonl`); +} + +/** The rebuildable bootstrap output for a project. */ +export function seedFilePath(root: string, cwd: string): string { + return path.join(projectDir(root, cwd), "seed.jsonl"); +} + +/** The one-time legacy/global seed (never GC'd). */ +export function globalSeedPath(root: string): string { + return path.join(root, "history-global.jsonl"); +} + +/** Advisory hash → cwd map for display labels. */ +export function registryPath(root: string): string { + return path.join(root, "registry.json"); +} + +// =========================================================================== +// Registry (formerly registry.ts) +// =========================================================================== + +export interface RegistryEntryResult { + hash: string; + created: boolean; +} + +type RegistryData = Record; + +function readRegistry(root: string): RegistryData { + try { + const raw = fs.readFileSync(registryPath(root), "utf8"); + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return {}; + } + const out: RegistryData = {}; + for (const [key, value] of Object.entries( + parsed as Record, + )) { + if (typeof value === "string") out[key] = value; + } + return out; + } catch { + return {}; + } +} + +function writeRegistryAtomic(root: string, data: RegistryData): void { + const target = registryPath(root); + const tmp = `${target}.tmp-${process.pid}-${Date.now()}`; + fs.mkdirSync(root, { recursive: true }); + fs.writeFileSync(tmp, JSON.stringify(data, null, 2) + "\n", "utf8"); + fs.renameSync(tmp, target); +} + +/** + * Ensure the advisory registry maps this project's hash to its cwd. + * Idempotent: an existing identical entry writes nothing. A hash mapped to a + * DIFFERENT cwd is a (practically unreachable) collision — the entry is + * re-keyed at 24 hash chars so both identities coexist. + */ +export function ensureRegistryEntry( + root: string, + cwd: string, +): RegistryEntryResult { + const hash = projectHash(cwd); + const data = readRegistry(root); + if (data[hash] === cwd) return { hash, created: false }; + if (data[hash] !== undefined) { + // Collision: re-key the EXISTING occupant at 24 hash chars so both + // identities coexist; the incoming cwd keeps the short hash — the + // key shape projectDir/sessionFilePath/drains derive. + const existing = data[hash]; + data[projectHashLong(existing)] = existing; + data[hash] = cwd; + writeRegistryAtomic(root, data); + return { hash, created: true }; + } + data[hash] = cwd; + writeRegistryAtomic(root, data); + return { hash, created: true }; +} + +function projectHashLong(cwd: string): string { + // Reuse the same canonicalization as projectHash but keep 24 chars. + let canonical = cwd; + try { + canonical = fs.realpathSync(cwd); + } catch { + // fall back to the raw path + } + return createHash("sha256").update(canonical).digest("hex").slice(0, 24); +} + +// =========================================================================== +// Entry primitives (from v1 history-store.ts) +// =========================================================================== + +/** One line of `editor-history.jsonl`. */ +export interface StoreEntry { + /** Schema version; 1 when absent in the source line. */ + v: number; + text: string; + /** Capture epoch-ms; optional, line order is authoritative for recency. */ + ts?: number; +} + +/** + * Parse one JSONL line. Returns null for malformed lines (bad JSON, + * non-string or whitespace-only text) so callers can skip them; a torn + * last line from a crash is handled the same way. + */ +export function parseStoreLine(raw: string): StoreEntry | null { + if (raw.length === 0) return null; + try { + const value: unknown = JSON.parse(raw); + if (!value || typeof value !== "object") return null; + const record = value as { v?: unknown; text?: unknown; ts?: unknown }; + if (typeof record.text !== "string") return null; + if (record.text.trim().length === 0) return null; + const entry: StoreEntry = { v: 1, text: record.text }; + if (typeof record.v === "number" && Number.isFinite(record.v)) { + entry.v = record.v; + } + if (typeof record.ts === "number" && Number.isFinite(record.ts)) { + entry.ts = record.ts; + } + return entry; + } catch { + return null; + } +} + +// =========================================================================== +// Instance writer (formerly multi-store.ts; scope drains/deletes and GC +// arrive in later slices) +// =========================================================================== + +/** Mutable state of ONE pi instance's exclusive capture file. */ +export interface SessionWriterState { + filePath: string; + /** Logical line count of this instance's file. */ + lineCount: number; +} + +/** Command-like prompts (`/name ...`) are UI commands, not prompts. */ +function isLikelyCommand(text: string): boolean { + return /^\/[A-Za-z]/.test(text.trim()); +} + +function serializeEntry(entry: StoreEntry): string { + const out: { v: number; text: string; ts?: number } = { + v: entry.v, + text: entry.text, + }; + if (entry.ts !== undefined) out.ts = entry.ts; + return JSON.stringify(out); +} + +/** + * Open the writer for this pi instance. The file is created LAZILY by the + * first capture — starting pi must not litter empty files. Only this + * instance ever appends here (design v2: zero shared writes). + */ +export function openSessionWriter( + root: string, + cwd: string, + instanceId: string, +): SessionWriterState { + return { + filePath: sessionFilePath(root, cwd, instanceId), + lineCount: 0, + }; +} + +/** + * Append one prompt line to the instance's own file (write-through). + * Skips empty/whitespace-only and command-like prompts. + */ +export function appendSessionCapture( + state: SessionWriterState, + text: string, + ts?: number, +): void { + if (typeof text !== "string" || text.trim().length === 0) return; + if (isLikelyCommand(text)) return; + + const entry: StoreEntry = { v: 1, text }; + if (ts !== undefined) entry.ts = ts; + fs.mkdirSync(path.dirname(state.filePath), { recursive: true }); + fs.appendFileSync(state.filePath, serializeEntry(entry) + "\n", "utf8"); + state.lineCount += 1; +} diff --git a/tests/history-atomic-write.test.ts b/tests/history-atomic-write.test.ts new file mode 100644 index 000000000..5448398e1 --- /dev/null +++ b/tests/history-atomic-write.test.ts @@ -0,0 +1,57 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { writeJsonAtomic } from "../extensions/history/atomic-write.ts"; + +// Shared atomic writer (design §D3): tmp+rename in the TARGET's directory. +// Fixtures live under the OS temp dir — never the workspace tmp. Failure +// paths exercise the catch branch: false return, no throw, and the staging +// file unlinked so `.tmp-*` files never accumulate beside the target. + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "atomic-write-")); +} + +function tmpLeftovers(dir: string): string[] { + return fs.readdirSync(dir).filter((f) => f.includes(".tmp-")); +} + +test("success: missing parent dirs are created recursively and the JSON parses back", () => { + const root = makeRoot(); + const target = path.join(root, "deep", "nested", "state.json"); + const value = { key: "value", nested: { n: 1 } }; + assert.equal(writeJsonAtomic(target, value), true); + assert.deepEqual(JSON.parse(fs.readFileSync(target, "utf8")), value); + assert.deepEqual(tmpLeftovers(path.dirname(target)), []); +}); + +test("a parent chain holding a regular file (ENOTDIR) returns false without throwing", () => { + const root = makeRoot(); + const blocker = path.join(root, "blocker"); + fs.writeFileSync(blocker, "regular file", "utf8"); + const target = path.join(blocker, "child", "state.json"); + assert.equal(writeJsonAtomic(target, { a: 1 }), false); + assert.equal(fs.existsSync(target), false); +}); + +test("an existing directory as the target fails the rename: false and no leftover staging file", () => { + const root = makeRoot(); + const target = path.join(root, "state.json"); + fs.mkdirSync(target, { recursive: true }); + assert.equal(writeJsonAtomic(target, { a: 1 }), false); + // The catch branch unlinked its staging file — no `.tmp-*` accumulation. + assert.deepEqual(tmpLeftovers(root), []); + // The directory itself is untouched. + assert.equal(fs.statSync(target).isDirectory(), true); +}); + +test("overwrite of an existing target replaces the content", () => { + const root = makeRoot(); + const target = path.join(root, "state.json"); + assert.equal(writeJsonAtomic(target, { v: 1 }), true); + assert.equal(writeJsonAtomic(target, { v: 2 }), true); + assert.deepEqual(JSON.parse(fs.readFileSync(target, "utf8")), { v: 2 }); + assert.deepEqual(tmpLeftovers(root), []); +}); diff --git a/tests/history-multi-reader.test.ts b/tests/history-multi-reader.test.ts new file mode 100644 index 000000000..c82ebcbc7 --- /dev/null +++ b/tests/history-multi-reader.test.ts @@ -0,0 +1,203 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { writeJsonAtomic } from "../extensions/history/atomic-write.ts"; +import { + appendSessionCapture, + ensureRegistryEntry, + openSessionWriter, + parseStoreLine, + projectDir, + projectHash, + registryPath, + sessionFilePath, +} from "../extensions/history/store.ts"; + +// Slice-1 concurrency/recovery coverage. The dev-suite multi-reader drain +// scenarios are re-expressed against the slice-1 surface (per-instance +// writers, parseStoreLine, atomic writes): parallel writers on one project +// dir, torn-line tolerance, and same-target atomic-write collisions. The +// drain/read ordering scenarios themselves arrive with the slice-2 reader. + +const PROJECT_A = "/pi-history-test/project-a"; +const PROJECT_B = "/pi-history-test/project-b"; + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-multi-reader-")); +} + +function storedTexts(file: string): string[] { + return fs + .readFileSync(file, "utf8") + .split("\n") + .filter((l) => l.trim().length > 0) + .map((l) => (JSON.parse(l) as { text: string }).text); +} + +// --- parallel writers --- + +test("growth from a concurrent instance is visible on the next read", () => { + const root = makeRoot(); + const a = openSessionWriter(root, PROJECT_A, "inst-a"); + appendSessionCapture(a, "first"); + const dirA = projectDir(root, PROJECT_A); + assert.deepEqual(fs.readdirSync(dirA), ["inst-a.jsonl"]); + + // A second pi instance grows the SAME project dir through its OWN file; + // neither writer reads or rewrites the other's bytes. + const b = openSessionWriter(root, PROJECT_A, "inst-b"); + appendSessionCapture(b, "from-other-instance"); + + assert.deepEqual(fs.readdirSync(dirA).sort(), [ + "inst-a.jsonl", + "inst-b.jsonl", + ]); + assert.deepEqual(storedTexts(sessionFilePath(root, PROJECT_A, "inst-a")), [ + "first", + ]); + assert.deepEqual(storedTexts(sessionFilePath(root, PROJECT_A, "inst-b")), [ + "from-other-instance", + ]); + assert.equal(a.lineCount, 1); + assert.equal(b.lineCount, 1); +}); + +test("interleaved captures from multiple writers never clobber each other", () => { + const root = makeRoot(); + const writers = [ + openSessionWriter(root, PROJECT_A, "w0"), + openSessionWriter(root, PROJECT_A, "w1"), + openSessionWriter(root, PROJECT_A, "w2"), + ]; + for (let i = 0; i < 10; i++) { + for (let w = 0; w < writers.length; w++) { + appendSessionCapture(writers[w], `w${w}-line-${i}`); + } + } + const dir = projectDir(root, PROJECT_A); + assert.deepEqual(fs.readdirSync(dir).sort(), [ + "w0.jsonl", + "w1.jsonl", + "w2.jsonl", + ]); + for (let w = 0; w < writers.length; w++) { + assert.equal(writers[w].lineCount, 10); + assert.deepEqual( + storedTexts(writers[w].filePath), + Array.from({ length: 10 }, (_, i) => `w${w}-line-${i}`), + ); + } +}); + +test("a high-volume burst on one writer keeps every line, in order", () => { + const root = makeRoot(); + const writer = openSessionWriter(root, PROJECT_A, "burst"); + const expected: string[] = []; + for (let i = 0; i < 100; i++) { + const text = `burst-${i}`; + expected.push(text); + appendSessionCapture(writer, text, 1000 + i); + } + assert.equal(writer.lineCount, 100); + const lines = fs.readFileSync(writer.filePath, "utf8").trim().split("\n"); + assert.equal(lines.length, 100); + const parsed = lines.map((l) => JSON.parse(l) as { text: string; ts: number }); + assert.deepEqual( + parsed.map((e) => e.text), + expected, + ); + assert.equal(parsed[42].ts, 1042); +}); + +// --- torn-line / crash recovery --- + +test("torn and malformed lines parse to null (crash garbage never resurfaces)", () => { + // A torn final line (process died mid-write) is a truncated JSON doc. + const torn = JSON.stringify({ v: 1, text: "survivor" }).slice(0, 12); + const malformed: string[] = [ + "", + "{torn", + torn, + "not json at all", + JSON.stringify([]), + JSON.stringify("scalar"), + JSON.stringify(null), + JSON.stringify({ v: 1 }), + JSON.stringify({ text: 42 }), + JSON.stringify({ text: " " }), + ]; + for (const line of malformed) { + assert.equal(parseStoreLine(line), null, JSON.stringify(line)); + } + // Valid lines keep parsing: absent v defaults to 1, ts/v flow through. + assert.deepEqual(parseStoreLine(JSON.stringify({ v: 1, text: "survivor" })), { + v: 1, + text: "survivor", + }); + assert.deepEqual(parseStoreLine(JSON.stringify({ text: "y" })), { + v: 1, + text: "y", + }); + assert.deepEqual(parseStoreLine(JSON.stringify({ v: 2, text: "x", ts: 7 })), { + v: 2, + text: "x", + ts: 7, + }); +}); + +test("a torn final line is tolerated: skipped by readers, later appends continue", () => { + const root = makeRoot(); + const writer = openSessionWriter(root, PROJECT_A, "torn"); + appendSessionCapture(writer, "before-crash"); + // Crash mid-write: a partial line lands WITHOUT its trailing newline. + fs.appendFileSync(writer.filePath, `{"v":1,"text":"tor`, "utf8"); + // parseStoreLine skips the torn tail instead of throwing... + assert.equal(parseStoreLine('{"v":1,"text":"tor'), null); + // ...and the instance keeps capturing. The first append after a + // newline-less torn tail merges with the fragment (one accepted lost + // entry — the same crash window the design documents for lost writes); + // the next full line parses cleanly again. + appendSessionCapture(writer, "after-crash"); + appendSessionCapture(writer, "after-crash-2"); + assert.equal(writer.lineCount, 3); + const lines = fs.readFileSync(writer.filePath, "utf8").trim().split("\n"); + assert.equal(lines.length, 3); + assert.equal((JSON.parse(lines[0]) as { text: string }).text, "before-crash"); + assert.equal(parseStoreLine(lines[1]), null); // torn fragment + merged entry + assert.equal( + (JSON.parse(lines[2]) as { text: string }).text, + "after-crash-2", + ); +}); + +// --- atomic-write collisions --- + +test("rapid same-target atomic writes leave one valid document and no staging files", () => { + const root = makeRoot(); + const target = path.join(root, "shared-state.json"); + for (let i = 0; i < 25; i++) { + assert.equal(writeJsonAtomic(target, { writer: i }), true); + } + const final = JSON.parse(fs.readFileSync(target, "utf8")) as { + writer: number; + }; + assert.ok(final.writer >= 0 && final.writer <= 24); + const leftovers = fs.readdirSync(root).filter((f) => f.includes(".tmp-")); + assert.deepEqual(leftovers, []); +}); + +test("interleaved registry updates from two instances keep both entries", () => { + const root = makeRoot(); + for (let i = 0; i < 3; i++) { + ensureRegistryEntry(root, PROJECT_A); + ensureRegistryEntry(root, PROJECT_B); + } + const raw = JSON.parse( + fs.readFileSync(registryPath(root), "utf8"), + ) as Record; + assert.equal(Object.keys(raw).length, 2); + assert.equal(raw[projectHash(PROJECT_A)], PROJECT_A); + assert.equal(raw[projectHash(PROJECT_B)], PROJECT_B); +}); diff --git a/tests/history-registry.test.ts b/tests/history-registry.test.ts new file mode 100644 index 000000000..c985972a1 --- /dev/null +++ b/tests/history-registry.test.ts @@ -0,0 +1,109 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + ensureRegistryEntry, + projectHash, + registryPath, +} from "../extensions/history/store.ts"; + +// Slice-1 port note: the dev suite asserted lookups through the dead +// `lookupCwd` export, which slice 1 drops. Every lookup assertion is +// re-expressed against the persisted registry.json content. + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-registry-")); +} + +function readRegistryFile(root: string): Record { + return JSON.parse( + fs.readFileSync(registryPath(root), "utf8"), + ) as Record; +} + +test("creates the registry with the first entry (idempotent)", () => { + const root = makeRoot(); + const result = ensureRegistryEntry(root, "/pi-history-test/project-a"); + assert.deepEqual(result, { hash: "4be15ec687e9df85", created: true }); + ensureRegistryEntry(root, "/pi-history-test/project-a"); + assert.deepEqual(readRegistryFile(root), { + "4be15ec687e9df85": "/pi-history-test/project-a", + }); +}); + +test("second project appends without touching the first", () => { + const root = makeRoot(); + ensureRegistryEntry(root, "/pi-history-test/project-a"); + const b = ensureRegistryEntry(root, "/pi-history-test/project-b"); + assert.equal(b.created, true); + const raw = readRegistryFile(root); + assert.equal(Object.keys(raw).length, 2); + assert.equal(raw[b.hash], "/pi-history-test/project-b"); +}); + +test("registry.json maps known hashes and omits unknown ones", () => { + const root = makeRoot(); + const { hash } = ensureRegistryEntry(root, "/pi-history-test/project-a"); + const raw = readRegistryFile(root); + assert.equal(raw[hash], "/pi-history-test/project-a"); + assert.equal(raw["0000000000000000"], undefined); + // A fresh root has no registry file until its first entry lands. + assert.equal(fs.existsSync(registryPath(makeRoot())), false); +}); + +test("corrupt registry json is treated as empty and rebuilt on next entry", () => { + const root = makeRoot(); + fs.writeFileSync(registryPath(root), "{not-json", "utf8"); + const result = ensureRegistryEntry(root, "/pi-history-test/project-a"); + assert.equal(result.created, true); + // The corrupt content was discarded (fail-open to empty), so the rebuilt + // registry contains exactly the new entry and nothing else. + assert.deepEqual(readRegistryFile(root), { + "4be15ec687e9df85": "/pi-history-test/project-a", + }); +}); + +test("no leftover tmp files after writes", () => { + const root = makeRoot(); + ensureRegistryEntry(root, "/a"); + ensureRegistryEntry(root, "/b"); + const leftovers = fs.readdirSync(root).filter((f) => f.includes(".tmp-")); + assert.deepEqual(leftovers, []); +}); + +test("hash collision re-keys the existing occupant; the new cwd keeps the short hash", () => { + const root = makeRoot(); + const cwd = "/pi-history-test/project-a"; + const hash = projectHash(cwd); + // Simulate a collision: the short hash is pre-mapped to a different cwd. + fs.mkdirSync(root, { recursive: true }); + fs.writeFileSync( + registryPath(root), + JSON.stringify({ [hash]: "/some/other/project" }), + "utf8", + ); + const result = ensureRegistryEntry(root, cwd); + assert.deepEqual(result, { hash, created: true }); + const raw = readRegistryFile(root); + assert.equal(raw[hash], cwd); + const longKeys = Object.keys(raw).filter((k) => k.length === 24); + assert.equal(longKeys.length, 1); + assert.equal(raw[longKeys[0]], "/some/other/project"); +}); + +test("wrong-shaped registry (array / scalar / null) fails open and is rebuilt on the next entry", () => { + // Valid JSON, wrong shape: the readRegistry shape guard treats each as an + // empty registry, and the next entry rebuilds a valid object-mapped + // registry around itself. + const shapes: unknown[] = [["an", "array"], "scalar-string", null]; + const cwd = "/pi-history-test/project-a"; + for (const shape of shapes) { + const root = makeRoot(); + fs.writeFileSync(registryPath(root), JSON.stringify(shape), "utf8"); + const result = ensureRegistryEntry(root, cwd); + assert.deepEqual(result, { hash: projectHash(cwd), created: true }); + assert.deepEqual(readRegistryFile(root), { [projectHash(cwd)]: cwd }); + } +}); diff --git a/tests/history-session-writer.test.ts b/tests/history-session-writer.test.ts new file mode 100644 index 000000000..8601cea14 --- /dev/null +++ b/tests/history-session-writer.test.ts @@ -0,0 +1,109 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + appendSessionCapture, + openSessionWriter, + projectHash, + sessionFilePath, +} from "../extensions/history/store.ts"; +import promptHistoryExtension from "../extensions/history/index.ts"; + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-writer-")); +} + +const CWD = "/pi-history-test/project-a"; + +function fileTexts(file: string): string[] { + return fs + .readFileSync(file, "utf8") + .split("\n") + .filter((l) => l.trim().length > 0) + .map((l) => (JSON.parse(l) as { text: string }).text); +} + +function openWriterForTest(root: string, instanceId: string) { + return openSessionWriter(root, CWD, instanceId); +} + +test("no file is created until the first capture", () => { + const root = makeRoot(); + const state = openWriterForTest(root, "sess-1"); + const file = sessionFilePath(root, CWD, "sess-1"); + assert.equal(fs.existsSync(file), false); + assert.equal(state.lineCount, 0); +}); + +test("first capture lazily creates the file and appends one line", () => { + const root = makeRoot(); + const state = openWriterForTest(root, "sess-1"); + appendSessionCapture(state, "hello world", 1234); + const file = sessionFilePath(root, CWD, "sess-1"); + assert.equal(fs.existsSync(file), true); + const lines = fs.readFileSync(file, "utf8").trim().split("\n"); + assert.equal(lines.length, 1); + const parsed = JSON.parse(lines[0]); + assert.equal(parsed.text, "hello world"); + assert.equal(parsed.ts, 1234); + assert.equal(parsed.v, 1); + assert.equal(state.lineCount, 1); +}); + +test("captures append in order; count tracks", () => { + const root = makeRoot(); + const state = openWriterForTest(root, "sess-2"); + appendSessionCapture(state, "one"); + appendSessionCapture(state, "two"); + appendSessionCapture(state, "three"); + assert.deepEqual(fileTexts(sessionFilePath(root, CWD, "sess-2")), [ + "one", + "two", + "three", + ]); + assert.equal(state.lineCount, 3); +}); + +test("command-like and empty captures are skipped", () => { + const root = makeRoot(); + const state = openWriterForTest(root, "sess-3"); + appendSessionCapture(state, "/compact"); + appendSessionCapture(state, " "); + appendSessionCapture(state, ""); + appendSessionCapture(state, "kept"); + assert.deepEqual(fileTexts(sessionFilePath(root, CWD, "sess-3")), ["kept"]); + assert.equal(state.lineCount, 1); +}); + +test("two writers own separate files in the same project dir", () => { + const root = makeRoot(); + const a = openWriterForTest(root, "inst-a"); + const b = openWriterForTest(root, "inst-b"); + appendSessionCapture(a, "from-a"); + appendSessionCapture(b, "from-b"); + const dir = path.join(root, "projects", projectHash(CWD)); + const files = fs.readdirSync(dir).sort(); + assert.deepEqual(files, ["inst-a.jsonl", "inst-b.jsonl"]); +}); + +test("the slice-1 extension entry registers only the capture handler", () => { + // Module load must stay side-effect free (importing index.ts parses the + // whole slice-1 graph without touching the real ~/.pi store root), and + // slice 1 wires exactly one handler: before_agent_start. + const registered: Array<[string, unknown]> = []; + const pi = { + on: (event: string, handler: unknown) => { + registered.push([event, handler]); + }, + }; + promptHistoryExtension(pi as never); + assert.deepEqual( + registered.map(([event]) => event), + ["before_agent_start"], + ); + // The handler is callable but is NEVER invoked here: a real invocation + // would run getWriter() against the user's real ~/.pi/agent/history. + assert.equal(typeof registered[0][1], "function"); +}); diff --git a/tests/history-store-paths.test.ts b/tests/history-store-paths.test.ts new file mode 100644 index 000000000..a6e5be45a --- /dev/null +++ b/tests/history-store-paths.test.ts @@ -0,0 +1,79 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + globalSeedPath, + projectDir, + projectHash, + registryPath, + seedFilePath, + sessionFilePath, +} from "../extensions/history/store.ts"; + +const ROOT = path.join(os.tmpdir(), "pi-history-test-root"); + +test("projectHash returns 16 lowercase hex chars", () => { + const hash = projectHash("/pi-history-test/project-a"); + assert.match(hash, /^[0-9a-f]{16}$/); +}); + +test("known vector: stable hash for a fixed path", () => { + // The literal exists on no machine, so every platform exercises the + // documented raw-string fallback: sha256(literal), first 16 hex chars. + assert.equal( + projectHash("/pi-history-test/project-a"), + "4be15ec687e9df85", + ); +}); + +test("distinct paths produce distinct hashes", () => { + assert.notEqual( + projectHash("/pi-history-test/project-a"), + projectHash("/pi-history-test/project-b"), + ); +}); + +test("symlinked cwd resolves to the same hash as its target", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "paths-sym-")); + const target = path.join(dir, "real-project"); + fs.mkdirSync(target); + const link = path.join(dir, "link-project"); + fs.symlinkSync(target, link); + assert.equal(projectHash(link), projectHash(target)); +}); + +test("trailing slash does not change the identity", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "paths-slash-")); + assert.equal(projectHash(dir), projectHash(`${dir}/`)); +}); + +test("nonexistent path falls back to hashing the raw string (no throw)", () => { + const missing = path.join(os.tmpdir(), "paths-missing-does-not-exist"); + const hash = projectHash(missing); + assert.match(hash, /^[0-9a-f]{16}$/); +}); + +test("path derivations compose under the root", () => { + const cwd = "/pi-history-test/project-a"; + const hash = projectHash(cwd); + assert.equal(projectDir(ROOT, cwd), path.join(ROOT, "projects", hash)); + assert.equal( + sessionFilePath(ROOT, cwd, "abc-123"), + path.join(ROOT, "projects", hash, "abc-123.jsonl"), + ); + assert.equal( + seedFilePath(ROOT, cwd), + path.join(ROOT, "projects", hash, "seed.jsonl"), + ); + assert.equal(globalSeedPath(ROOT), path.join(ROOT, "history-global.jsonl")); + assert.equal(registryPath(ROOT), path.join(ROOT, "registry.json")); +}); + +test("two cwds map to sibling project dirs", () => { + const a = projectDir(ROOT, "/pi-history-test/project-a"); + const b = projectDir(ROOT, "/pi-history-test/project-b"); + assert.notEqual(a, b); + assert.equal(path.dirname(a), path.dirname(b)); +}); From 2b90751e8ff7cb97a8c0e4eba2348a95cbbaab33 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:37:15 -0300 Subject: [PATCH 02/14] fix(history): stable registry collision mappings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fix (CodeRabbit #5160388228): ensureRegistryEntry now searches the registry for an existing mapping of the incoming cwd before the collision branch, returning the existing short or long key unchanged. Previously, re-entering a cwd that an earlier collision had re-keyed to 24 chars re-triggered the collision and flipped the other occupant's key every time — collision assignments were not stable. Adds a stability test: the re-keyed cwd keeps its long key, the short-hash holder keeps its key, and the registry bytes do not change across re-entries. Note: the atomic-write staging-name race CodeRabbit reported in the original commit was already hardened on this branch (unique .tmp-- staging + unlink-on-failure); no further change. --- extensions/history/store.ts | 5 +++++ tests/history-registry.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/extensions/history/store.ts b/extensions/history/store.ts index 8631707b9..f3723f6ad 100644 --- a/extensions/history/store.ts +++ b/extensions/history/store.ts @@ -113,6 +113,11 @@ export function ensureRegistryEntry( const hash = projectHash(cwd); const data = readRegistry(root); if (data[hash] === cwd) return { hash, created: false }; + // An earlier collision may have re-keyed THIS cwd to a long key. + // Return the existing mapping unchanged so collision assignments stay + // stable across calls instead of flipping the other occupant's key. + const existingKey = Object.keys(data).find((k) => data[k] === cwd); + if (existingKey !== undefined) return { hash: existingKey, created: false }; if (data[hash] !== undefined) { // Collision: re-key the EXISTING occupant at 24 hash chars so both // identities coexist; the incoming cwd keeps the short hash — the diff --git a/tests/history-registry.test.ts b/tests/history-registry.test.ts index c985972a1..23235f8cd 100644 --- a/tests/history-registry.test.ts +++ b/tests/history-registry.test.ts @@ -1,5 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -93,6 +94,39 @@ test("hash collision re-keys the existing occupant; the new cwd keeps the short assert.equal(raw[longKeys[0]], "/some/other/project"); }); +test("a re-keyed cwd keeps its long key on later calls (stable collision mappings)", () => { + // projectHashLong is private: derive the documented 24-char key here — + // the literals never exist, so canonicalization falls back to the raw + // string on every platform. + const longKey = (cwd: string) => + createHash("sha256").update(cwd).digest("hex").slice(0, 24); + const root = makeRoot(); + const a = "/pi-history-test/registry-collide-a"; + const b = "/pi-history-test/registry-collide-b"; + // Simulate the collision: b's short hash is pre-mapped to a different + // cwd, so entering b re-keys that occupant to a 24-char key. + const shortHash = projectHash(b); + fs.mkdirSync(root, { recursive: true }); + fs.writeFileSync( + registryPath(root), + JSON.stringify({ [shortHash]: a }), + "utf8", + ); + ensureRegistryEntry(root, b); // collision: a re-keyed to 24 chars + const before = readRegistryFile(root); + // Re-entering the re-keyed cwd must return its EXISTING long key and + // leave the other occupant's short-hash mapping untouched. + const again = ensureRegistryEntry(root, a); + assert.equal(again.created, false); + assert.equal(again.hash, longKey(a)); + const after = readRegistryFile(root); + assert.deepEqual(after, before); + // And re-entering the short-hash holder keeps the short key. + const holder = ensureRegistryEntry(root, b); + assert.equal(holder.hash, projectHash(b)); + assert.deepEqual(readRegistryFile(root), before); +}); + test("wrong-shaped registry (array / scalar / null) fails open and is rebuilt on the next entry", () => { // Valid JSON, wrong shape: the readRegistry shape guard treats each as an // empty registry, and the next entry rebuilds a valid object-mapped From a5ff13d64dae7181ea7ce68ec578d322ef40a4ae Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:46:43 -0300 Subject: [PATCH 03/14] feat(history): read, ordering, deduplication, and project/global query APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 2/6 of the PR #819 split (maintainer-requested review slices). - store: reader/query section — file listing with mtime resolution, drain ordering (newest entry ts, mtime fallback; stable under atomic rewrites), dedup + tombstone filter + cap drain, project scope drain (hash dir) and global scope drain (all project dirs, legacy global seed last); dead generator fileEntriesBackward (zero callers) dropped - hide-prompts: tombstone file contract (fail-open reader, atomic sorted writer, shared dedup key) — lands here because the drain APIs filter hidden prompts via the optional stateDir parameter; slice 5 delivers deletion semantics on top - selector-helpers (new): entry/dedup-key normalization, keep-first read-time dedup, records shaping with provenance, result filter with MAX_RESULTS cap; windowing/nav helpers follow in slice 3 - tests: 21 new node:test cases (cumulative 53/53): drain ordering across mixed mtimes, hidden-prompt filtering incl. corrupt hidden.json fail-open, dedup key normalization, cap at exactly 10000, hide/write contract incl. ENOTDIR failure; portable CWD literals throughout (no machine-specific paths) Gates: cumulative scoped history tests 53/53 green (slice-1 set unchanged). Known pre-existing environmental gate failures unchanged (contracts/.DS_Store; missing node_modules for package-manifest). --- extensions/history/hide-prompts.ts | 74 +++++++++++ extensions/history/selector-helpers.ts | 105 +++++++++++++++ extensions/history/store.ts | 170 ++++++++++++++++++++++++- tests/history-dedupe-entries.test.ts | 123 ++++++++++++++++++ tests/history-drain-hidden.test.ts | 54 ++++++++ tests/history-drain-order.test.ts | 86 +++++++++++++ tests/history-hide-prompts.test.ts | 99 ++++++++++++++ tests/history-max-results-cap.test.ts | 76 +++++++++++ 8 files changed, 781 insertions(+), 6 deletions(-) create mode 100644 extensions/history/hide-prompts.ts create mode 100644 extensions/history/selector-helpers.ts create mode 100644 tests/history-dedupe-entries.test.ts create mode 100644 tests/history-drain-hidden.test.ts create mode 100644 tests/history-drain-order.test.ts create mode 100644 tests/history-hide-prompts.test.ts create mode 100644 tests/history-max-results-cap.test.ts diff --git a/extensions/history/hide-prompts.ts b/extensions/history/hide-prompts.ts new file mode 100644 index 000000000..9cffd6954 --- /dev/null +++ b/extensions/history/hide-prompts.ts @@ -0,0 +1,74 @@ +// SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history +// SPDX-License-Identifier: MIT + +import fs from "node:fs"; +import path from "node:path"; +import { writeJsonAtomic } from "./atomic-write.ts"; +import { promptDedupKey } from "./selector-helpers.ts"; + +/** Name of the tombstone file inside the injected state dir (spec C4). */ +const HIDE_FILE_NAME = "hidden.json"; + +/** + * Result of one tombstone write (spec C4): `written` on a successful atomic + * write, or an error object carrying a short, toast-suitable reason. Never + * throws. + */ +export type HideResult = + | { status: "written" } + | { status: "error"; message: string }; + +/** + * Load the tombstone key set from `stateDir/hidden.json` — the READ half of + * the hide-file contract (spec C4). Fail-open: a missing, unreadable, + * corrupt, or wrong-shaped file is an EMPTY set and the call never throws; + * a corrupt file is rewritten clean by the next hide (the WRITE half, + * `hidePrompt`, lands in WU4). Keys are `promptDedupKey` strings written by + * `hidePrompt`; foreign values are ignored, never trusted. + */ +export function loadHiddenPrompts(stateDir: string): Set { + let raw: string; + try { + raw = fs.readFileSync(path.join(stateDir, HIDE_FILE_NAME), "utf8"); + } catch { + return new Set(); // missing or unreadable → empty tombstones + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return new Set(); // corrupt bytes → fail-open empty + } + const keys = new Set(); + if (!Array.isArray(parsed)) return keys; // wrong shape → fail-open empty + for (const item of parsed) { + if (typeof item === "string" && item !== "") keys.add(item); + } + return keys; +} + +/** + * Write the tombstone key for `text` into `stateDir/hidden.json` — the + * WRITE half of the hide-file contract (spec C4). The key is the shared + * `promptDedupKey` (byte-match normative with the merge filter — never a + * re-implementation); the set compacts on write and persists as a SORTED + * array via the shared atomic tmp+rename writer. Fail-open both ways: a + * corrupt or missing file reads as empty (this clean rewrite IS the + * recovery — the corrupt contents are untrustworthy by definition) and any + * write failure returns an error object for the delete-flow toast; the + * call never throws. + */ +export function hidePrompt(stateDir: string, text: string): HideResult { + const keys = loadHiddenPrompts(stateDir); + keys.add(promptDedupKey(text)); + const written = writeJsonAtomic( + path.join(stateDir, HIDE_FILE_NAME), + [...keys].sort(), + ); + return written + ? { status: "written" } + : { + status: "error", + message: "Could not write the hide file; the prompt may reappear.", + }; +} diff --git a/extensions/history/selector-helpers.ts b/extensions/history/selector-helpers.ts new file mode 100644 index 000000000..c5257bafe --- /dev/null +++ b/extensions/history/selector-helpers.ts @@ -0,0 +1,105 @@ +export interface PromptRecord { + text: string; + searchText: string; + /** + * Provenance (spec C3): set on records built from PromptEntry inputs; + * ABSENT on records built from bare strings so the pinned deepEqual + * record shape ({text, searchText}) stays byte-compatible (design §I). + */ + source?: PromptSource; + /** Session records only: the resolved ms-epoch ordering timestamp. */ + ts?: number; +} + +/** Provenance of a prompt record or merge-loader entry (spec C3). */ +export type PromptSource = "editor" | "session"; + +/** + * Merge-loader currency (pre-dedup, spec C3): one editor-store or + * session-derived prompt. Session entries carry the resolved ms-epoch `ts`; + * editor entries do not (block ordering at the seam, proposal R8). + */ +export interface PromptEntry { + text: string; + source: PromptSource; + ts?: number; +} + +export function buildPromptRecords( + entries: ReadonlyArray, +): PromptRecord[] { + return entries.map((entry): PromptRecord => { + if (typeof entry === "string") { + // Bare string input keeps the EXACT Change 2 runtime shape — the + // pinned deepEqual records carry only {text, searchText}. + return { text: entry, searchText: entry.toLowerCase() }; + } + const record: PromptRecord = { + text: entry.text, + searchText: entry.text.toLowerCase(), + source: entry.source, + }; + if (entry.ts !== undefined) { + record.ts = entry.ts; + } + return record; + }); +} + +/** + * Normalization key for read-time dedup (spec C3): byte-matches the + * APPLIED patch key in nav/patches/editor.cjs (:480-:586) — whitespace + * runs collapse, then trim, then a 120-char prefix slice, then lowercase. + * The literal is the single-backslash applied-patch form; the raw patch + * file stores \\s+ only because its code sits inside a template literal. + * Shared by contract (spec C4): hide-prompts tombstone keys and the + * merge-history session-half tombstone filter MUST byte-match this key. + */ +export function promptDedupKey(entry: string): string { + return entry.replace(/\s+/g, " ").trim().slice(0, 120).toLowerCase(); +} + +/** + * Read-time dedup pass (spec C3): keep-first over input order (file order + * is newest-first, mirroring the patch's keep-first dedup-on-save), and + * empty-key entries (empty or whitespace-only) are skipped — excluded from + * the output and never usable as collision keys. Removes ONLY duplicate- + * normalized entries; no snapshot cap (filterPrompts still caps output). + * Generic over string | PromptEntry (WU3): over the COMBINED merged input + * the keep-first rule makes the editor copy win at the seam — objects pass + * through with provenance intact; no new dedup logic exists anywhere. + */ +export function dedupePromptEntries( + entries: readonly T[], +): T[] { + const seen = new Set(); + const deduped: T[] = []; + for (const entry of entries) { + const key = + typeof entry === "string" + ? promptDedupKey(entry) + : promptDedupKey(entry.text); + if (key !== "" && !seen.has(key)) { + seen.add(key); + deduped.push(entry); + } + } + return deduped; +} + +const MAX_RESULTS = 10000; + +export function filterPrompts( + records: PromptRecord[], + query: string, +): PromptRecord[] { + const trimmed = query.trim(); + if (!trimmed) return records.slice(0, MAX_RESULTS); + + const tokens = trimmed.toLowerCase().split(/\s+/).filter(Boolean); + const filtered = records.filter((record) => { + return tokens.every((token) => record.searchText.includes(token)); + }); + + return filtered.slice(0, MAX_RESULTS); +} diff --git a/extensions/history/store.ts b/extensions/history/store.ts index f3723f6ad..bf2a41c16 100644 --- a/extensions/history/store.ts +++ b/extensions/history/store.ts @@ -1,15 +1,17 @@ // SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history // SPDX-License-Identifier: MIT -// Consolidated multi-concurrency store (v2), slice 1: project paths and -// identity, the advisory registry, entry primitives, and the per-instance -// session writer. Scope drains/deletes, legacy migration and seed -// bootstrap, and GC/compaction arrive in later slices. +// Consolidated multi-concurrency store (v2), slices 1+2: project paths and +// identity, the advisory registry, entry primitives, the per-instance +// session writer, and the scope drain/reader/query section (ordering, +// dedup, tombstone filter, project/global drains). Legacy migration and +// seed bootstrap, scope deletes, and GC/compaction arrive in later slices. // Formerly store-paths.ts + registry.ts + multi-store.ts (+ v1 primitives). import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { loadHiddenPrompts } from "./hide-prompts.ts"; // =========================================================================== // Paths (formerly store-paths.ts) @@ -184,8 +186,8 @@ export function parseStoreLine(raw: string): StoreEntry | null { } // =========================================================================== -// Instance writer (formerly multi-store.ts; scope drains/deletes and GC -// arrive in later slices) +// Instance writer (formerly multi-store.ts; scope deletes and GC arrive +// in later slices) // =========================================================================== /** Mutable state of ONE pi instance's exclusive capture file. */ @@ -243,3 +245,159 @@ export function appendSessionCapture( fs.appendFileSync(state.filePath, serializeEntry(entry) + "\n", "utf8"); state.lineCount += 1; } + + +// --------------------------------------------------------------------------- +// Multi-file reader (design v2: k-way backward merge) +// --------------------------------------------------------------------------- + +/** UI-level prompt identity: whitespace-collapsed, case-insensitive. */ +function promptKey(text: string): string { + return text.replace(/\s+/g, " ").trim().toLowerCase(); +} + +function fileMtimeMs(file: string): number { + try { + return fs.statSync(file).mtimeMs; + } catch { + return 0; + } +} + +function listProjectFiles(dir: string): string[] { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return []; + } + return entries + .filter((e) => e.isFile() && e.name.endsWith(".jsonl")) + .map((e) => path.join(dir, e.name)) + .sort((a, b) => fileMtimeMs(b) - fileMtimeMs(a)); +} + +/** Read one file's valid entries (chronological). */ +function readFileEntries(file: string): StoreEntry[] { + let raw = ""; + try { + raw = fs.readFileSync(file, "utf8"); + } catch { + return []; + } + const entries: StoreEntry[] = []; + for (const lineText of raw.split("\n")) { + const parsed = parseStoreLine(lineText); + if (parsed) entries.push(parsed); + } + return entries; +} + +/** + * Sort key = the newest entry ts in the file (fallback: file mtime). + * ts-based keys are STABLE under atomic rewrites (deletes/compaction + * bump mtime, which used to reshuffle the drain order). + */ +function fileSortKey(file: string, entries: StoreEntry[]): number { + let maxTs = 0; + for (const entry of entries) { + if (entry.ts !== undefined && entry.ts > maxTs) maxTs = entry.ts; + } + return maxTs > 0 ? maxTs : fileMtimeMs(file); +} + +/** Tombstone key - byte-compatible with hide-prompts' promptDedupKey. */ +function promptDedupKeyOf(text: string): string { + return text.replace(/\s+/g, " ").trim().slice(0, 120).toLowerCase(); +} + +/** + * Sequential backward drain over PRE-SORTED files: each file fully, + * newest-line-first, deduped by UI-level identity, capped at `limit`. + */ +function drainFiles( + files: string[], + limit: number, + hidden: Set = new Set(), +): string[] { + const seen = new Set(); + const out: string[] = []; + for (const file of files) { + const entries = readFileEntries(file); + for (let i = entries.length - 1; i >= 0; i--) { + const key = promptKey(entries[i].text); + if (seen.has(key)) continue; + if (hidden.size > 0 && hidden.has(promptDedupKeyOf(entries[i].text))) { + continue; + } + seen.add(key); + out.push(entries[i].text); + if (out.length >= limit) return out; + } + } + return out; +} + +/** Sort files for draining: ts-keyed, newest first, empty files dropped. */ +function sortFilesForDrain(files: string[]): string[] { + return files + .map((file) => ({ file, entries: readFileEntries(file) })) + .filter((f) => f.entries.length > 0) + .sort( + (a, b) => + fileSortKey(b.file, b.entries) - fileSortKey(a.file, a.entries), + ) + .map((f) => f.file); +} + +/** + * Drain the PROJECT scope: all .jsonl files in the project dir (seed.jsonl + * included), mtime-newest-first, deduped, capped at `limit` (default 1000). + */ +export function drainProject( + root: string, + cwd: string, + limit: number = 1000, + stateDir?: string, +): string[] { + return drainFiles( + sortFilesForDrain(listProjectFiles(path.join(root, "projects", projectHash(cwd)))), + limit, + stateDir ? loadHiddenPrompts(stateDir) : new Set(), + ); +} + +/** + * Drain the GLOBAL scope: the legacy global seed (newest single source) + * plus every project dir's files, mtime-newest-first, deduped, capped. + */ +export function drainGlobal( + root: string, + limit: number = 1000, + stateDir?: string, +): string[] { + const files: string[] = []; + const globalSeed = globalSeedPath(root); + + let projectDirs: fs.Dirent[]; + try { + projectDirs = fs.readdirSync(path.join(root, "projects"), { + withFileTypes: true, + }); + } catch { + projectDirs = []; + } + for (const dirEntry of projectDirs) { + if (!dirEntry.isDirectory()) continue; + files.push( + ...listProjectFiles(path.join(root, "projects", dirEntry.name)), + ); + } + const sorted = sortFilesForDrain(files); + if (fs.existsSync(globalSeed)) sorted.push(globalSeed); // legacy last + return drainFiles( + sorted, + limit, + stateDir ? loadHiddenPrompts(stateDir) : new Set(), + ); +} diff --git a/tests/history-dedupe-entries.test.ts b/tests/history-dedupe-entries.test.ts new file mode 100644 index 000000000..f929880e9 --- /dev/null +++ b/tests/history-dedupe-entries.test.ts @@ -0,0 +1,123 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { dedupePromptEntries } from "../extensions/history/selector-helpers.ts"; + +// AC-L5-1..AC-L5-5 — read-time dedup pass (spec C3, design §D5). +// +// The normalization key MUST byte-match the APPLIED patch form from +// nav/patches/editor.cjs :480–:586: +// +// entry.replace(/\s+/g, " ").trim().slice(0, 120).toLowerCase() +// +// The raw patch file stores `\\s+` because the replacement code sits inside +// a template literal; the code that actually runs in the editor contains +// `/\s+/g`. An implementation copying the double-backslash form would build +// a regex matching a literal backslash: whitespace variants would stop +// collapsing (T1 fails) and empty-key entries would leak through (T2 fails). +// +// The dev suite's T3 source-parse pins (dedupePromptEntries wired between +// drainForScope and buildPromptRecords inside openHistorySelector) cover the +// slice-3 selector wiring in extensions/history/index.ts and port with that +// slice — index.ts stays at its slice-1 surface here. + +// T1 — AC-L5-1: keep-first over newest-first input order (file order). + +test("keep-first: [A, B, A″] where A″ normalizes equal to A yields [A, B] (AC-L5-1)", () => { + const a = "deploy the API"; + const b = "write the tests"; + const aDoublePrime = "deploy the API"; + assert.deepEqual(dedupePromptEntries([a, b, aDoublePrime]), [a, b]); +}); + +// T1 — AC-L5-2: normalization groups each collapse to their first entry. + +test("normalization group: internal whitespace runs collapse to the first entry (AC-L5-2)", () => { + const first = "run the build now"; + assert.deepEqual( + dedupePromptEntries([ + first, + "run the build now", + "run the build now ", + " run the build now", + ]), + [first], + ); +}); + +test("normalization group: tabs and newlines collapse to the first entry (AC-L5-2)", () => { + const first = "run the build now"; + assert.deepEqual( + dedupePromptEntries([ + first, + "run\tthe\tbuild\tnow", + "run\nthe\nbuild\nnow", + "run \t the \n build now", + ]), + [first], + ); +}); + +test("normalization group: letter case collapses to the first entry (AC-L5-2)", () => { + const first = "Run The Build NOW"; + assert.deepEqual( + dedupePromptEntries([first, "run the build now", "RUN THE BUILD NOW"]), + [first], + ); +}); + +// T1 — AC-L5-2: 120-char normalized prefix collisions collapse (accepted +// patch-mirror semantics, NOT the P5 dedup-key fix). + +test("entries sharing the normalized 120-char prefix but differing later collapse (AC-L5-2)", () => { + const prefix = "x".repeat(120); + const first = `${prefix} tail one`; + const second = `${prefix} tail two`; + assert.deepEqual(dedupePromptEntries([first, second]), [first]); +}); + +test("entries differing within the first 120 normalized chars stay distinct (AC-L5-2)", () => { + const a = `${"x".repeat(119)}a ${"y".repeat(10)}`; + const b = `${"x".repeat(119)}b ${"y".repeat(10)}`; + assert.deepEqual(dedupePromptEntries([a, b]), [a, b]); +}); + +// T2 — AC-L5-3: empty-key entries (empty string, whitespace-only) are +// skipped: excluded from the output, never usable as collision keys, real +// entries pass through. + +test("empty-key entries are excluded from the output while real entries pass through (AC-L5-3)", () => { + const real = "a real prompt"; + assert.deepEqual(dedupePromptEntries(["", " ", "\t\n ", real, "\t"]), [ + real, + ]); +}); + +test("whitespace-only entries never shadow real entries as collision keys (AC-L5-3)", () => { + const first = "another real prompt"; + assert.deepEqual(dedupePromptEntries(["", " ", first]), [first]); +}); + +// T2 — AC-L5-5: no truncation beyond duplicate removal (no snapshot cap). + +test("output length equals input length minus duplicate-normalized entries (AC-L5-5)", () => { + const entries = [ + "alpha", + "alpha", // duplicate of 0 + "beta", + " ALPHA ", // duplicate of 0 (case + whitespace) + "beta\t", // duplicate of 2 + "gamma", + ]; + assert.equal(dedupePromptEntries(entries).length, 3); +}); + +test("no snapshot cap: every unique entry is kept past MAX_RESULTS (AC-L5-5)", () => { + const entries: string[] = []; + for (let i = 0; i < 1200; i++) { + entries.push(`unique prompt number ${i}`); + } + const deduped = dedupePromptEntries(entries); + assert.equal(deduped.length, 1200); + assert.equal(deduped[0], entries[0]); + assert.equal(deduped[1199], entries[1199]); +}); diff --git a/tests/history-drain-hidden.test.ts b/tests/history-drain-hidden.test.ts new file mode 100644 index 000000000..90c479758 --- /dev/null +++ b/tests/history-drain-hidden.test.ts @@ -0,0 +1,54 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + drainGlobal, + drainProject, + globalSeedPath, + projectHash, +} from "../extensions/history/store.ts"; + +// Portable project identity: a never-existing literal. projectHash falls +// back to hashing the raw string when realpath fails, so the identity is +// deterministic on every machine (no machine-specific absolute paths). + +const CWD = "/pi-history-test/drain-hidden-project"; + +function write(file: string, texts: string[], ts = 100): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + `${texts.map((t) => JSON.stringify({ v: 1, text: t, ts })).join("\n")}\n`, + "utf8", + ); +} + +test("drains skip tombstoned prompts in seeds and session files", () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "hid-")); + const root = path.join(base, "h"); + const stateDir = path.join(base, "state"); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "hidden.json"), + JSON.stringify(["deleted from seed", "deleted from session"]), + "utf8", + ); + const dir = path.join(root, "projects", projectHash(CWD)); + write(path.join(dir, "seed.jsonl"), ["keep", "deleted from seed"], 100); + write(path.join(dir, "s1.jsonl"), ["also keep", "deleted from session"], 200); + write(globalSeedPath(root), ["deleted from seed", "legacy keep"], 50); + + assert.deepEqual(drainProject(root, CWD, 1000, stateDir), [ + "also keep", + "keep", + ]); + assert.deepEqual(drainGlobal(root, 1000, stateDir), [ + "also keep", + "keep", + "legacy keep", + ]); + // Without a stateDir the filter is off (raw drain semantics). + assert.equal(drainProject(root, CWD).includes("deleted from seed"), true); +}); diff --git a/tests/history-drain-order.test.ts b/tests/history-drain-order.test.ts new file mode 100644 index 000000000..86cfddca9 --- /dev/null +++ b/tests/history-drain-order.test.ts @@ -0,0 +1,86 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + drainGlobal, + drainProject, + globalSeedPath, + projectHash, +} from "../extensions/history/store.ts"; + +// Portable project identity: a never-existing literal. projectHash falls +// back to hashing the raw string when realpath fails, so the identity is +// deterministic on every machine (no machine-specific absolute paths). + +const CWD = "/pi-history-test/drain-order-project"; + +function writeTs(file: string, texts: string[], ts: number): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + `${texts.map((t) => JSON.stringify({ v: 1, text: t, ts })).join("\n")}\n`, + "utf8", + ); +} + +test("atomic rewrite (delete) does not reshuffle the drain order", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "ord-")); + const dir = path.join(root, "projects", projectHash(CWD)); + writeTs(path.join(dir, "old.jsonl"), ["a-old"], 100); + writeTs(path.join(dir, "new.jsonl"), ["z-new"], 200); + assert.deepEqual(drainProject(root, CWD), ["z-new", "a-old"]); + // Slice 5 ports deleteFromProject; its observable effect on the drain is + // simulated directly here: an atomic rewrite of the affected file that + // empties it — the mtime jumps to NOW, and the drain order must not move. + fs.writeFileSync(path.join(dir, "old.jsonl"), "", "utf8"); + fs.utimesSync(path.join(dir, "old.jsonl"), new Date(), new Date()); + assert.deepEqual(drainProject(root, CWD), ["z-new"]); + // Re-add with an OLD ts via direct write: still ordered by ts, not mtime. + writeTs(path.join(dir, "old2.jsonl"), ["b-old"], 150); + fs.utimesSync( + path.join(dir, "old2.jsonl"), + new Date(Date.now() + 99999), + new Date(Date.now() + 99999), + ); + assert.deepEqual(drainProject(root, CWD), ["z-new", "b-old"]); +}); + +test("global drain puts the legacy seed last regardless of its fresh mtime", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "seed-")); + const dir = path.join(root, "projects", projectHash(CWD)); + writeTs(path.join(dir, "s.jsonl"), ["fresh"], 200); + const seed = globalSeedPath(root); + writeTs(seed, ["legacy-1", "legacy-2"], 10); + fs.utimesSync(seed, new Date(Date.now() + 5000), new Date(Date.now() + 5000)); + assert.deepEqual(drainGlobal(root), ["fresh", "legacy-2", "legacy-1"]); +}); + +test( + "an unreadable store file is skipped; the rest drain in the expected order", + { skip: process.getuid?.() === 0 }, + () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "ord-sealed-")); + const dir = path.join(root, "projects", projectHash(CWD)); + writeTs(path.join(dir, "old.jsonl"), ["a-old"], 100); + const sealed = path.join(dir, "sealed.jsonl"); + writeTs(sealed, ["sealed-never"], 150); + writeTs(path.join(dir, "new.jsonl"), ["z-new"], 200); + // The sealed file's fresh mtime would sort it FIRST if it were readable — + // its absence from the drain is caused by the unreadable skip alone. + fs.utimesSync( + sealed, + new Date(Date.now() + 99999), + new Date(Date.now() + 99999), + ); + fs.chmodSync(sealed, 0o000); + try { + // An unreadable file reads as zero entries and drops out of the drain; + // the readable files keep their ts order. No throw. + assert.deepEqual(drainProject(root, CWD), ["z-new", "a-old"]); + } finally { + fs.chmodSync(sealed, 0o644); // restore before cleanup + } + }, +); diff --git a/tests/history-hide-prompts.test.ts b/tests/history-hide-prompts.test.ts new file mode 100644 index 000000000..03c054863 --- /dev/null +++ b/tests/history-hide-prompts.test.ts @@ -0,0 +1,99 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { hidePrompt, loadHiddenPrompts } from "../extensions/history/hide-prompts.ts"; +import { promptDedupKey } from "../extensions/history/selector-helpers.ts"; + +// Unit WU4 — tombstone write half + read half (spec C4, design §D6). fs-only +// coverage. The dev suite's deleteCurrent source-parse pins (T27/T28) and +// the deletionActionsFor planner pins cover the slice-3 selector branch and +// the slice-5 delete flow; they port with those slices. + +function makeStateDir(name: string): string { + return fs.mkdtempSync(path.join(os.tmpdir(), `hide-prompts-${name}-`)); +} + +function readHideFile(stateDir: string) { + return JSON.parse( + fs.readFileSync(path.join(stateDir, "hidden.json"), "utf8"), + ); +} + +// T24 — AC-S4-1: hide-key fidelity. Tombstone keys must byte-match the +// Change 2 dedup key for the same text — same imported helper, never a +// re-implementation: the stored file content is compared against +// promptDedupKey's own output with strict equality. +test("T24 (AC-S4-1): hide keys byte-match promptDedupKey across whitespace, case, and >120-char groups", () => { + const stateDir = makeStateDir("t24"); + // Three normalization groups: internal whitespace runs (space + tab), + // letter case, and a text longer than the 120-char key prefix. + const texts = [ + "fix\t the build", + "Deploy THE api", + `${"pad ".repeat(40)}tail beyond one hundred twenty chars`, + ]; + for (const text of texts) { + assert.deepEqual(hidePrompt(stateDir, text), { status: "written" }); + } + const stored = readHideFile(stateDir); + assert.ok(Array.isArray(stored), "hidden.json must hold a JSON array"); + // Byte-match: the file holds EXACTLY the shared helper's output, sorted. + assert.deepEqual(stored, texts.map((text) => promptDedupKey(text)).sort()); + // The loaded set agrees. + const loaded = loadHiddenPrompts(stateDir); + for (const key of stored) { + assert.ok(loaded.has(key)); + } +}); + +// T25 — AC-S4-2: hide persistence and tolerance. Two deletes of the same +// text compact to ONE key; a missing hide file reads as an empty set; reads +// never throw. +test("T25 (AC-S4-2): duplicate hides compact to one key; a missing file reads as empty; reads never throw", () => { + const stateDir = makeStateDir("t25"); + // Missing file: empty set, no throw (before any write exists). + assert.equal(loadHiddenPrompts(stateDir).size, 0); + // Two deletes of the same text — variants differing by case + whitespace + // runs normalize onto the same key. + assert.deepEqual(hidePrompt(stateDir, "Same Text"), { status: "written" }); + assert.deepEqual(hidePrompt(stateDir, "same text"), { status: "written" }); + const stored = readHideFile(stateDir); + assert.deepEqual(stored, [promptDedupKey("same text")]); + const loaded = loadHiddenPrompts(stateDir); + assert.equal(loaded.size, 1); + assert.ok(loaded.has(promptDedupKey("same text"))); +}); + +// T26 — AC-S4-5: corrupt hidden.json is fail-open (READ half) AND the next +// hide rewrites the file clean as a sorted compact array — the rewrite half +// is the recovery path. +test("T26 (AC-S4-5): corrupt hidden.json loads as empty and the next hide rewrites it clean", () => { + const stateDir = makeStateDir("t26"); + fs.writeFileSync( + path.join(stateDir, "hidden.json"), + "{corrupt bytes", + "utf8", + ); + assert.equal(loadHiddenPrompts(stateDir).size, 0); + assert.deepEqual(hidePrompt(stateDir, "beta prompt"), { status: "written" }); + // The rewrite landed: clean JSON holding exactly the new key. + assert.deepEqual(readHideFile(stateDir), [promptDedupKey("beta prompt")]); + assert.equal(loadHiddenPrompts(stateDir).size, 1); +}); + +// WU4c — write-failure path (AC-S4-2 triangulation): a state dir that cannot +// be created (its parent is a regular file) makes the atomic write return +// false, and hidePrompt maps that to the toast-suitable error object — +// never a throw. +test("hide write failure returns the exact error shape for the delete-flow toast", () => { + const base = makeStateDir("fail"); + const blocker = path.join(base, "blocker"); + fs.writeFileSync(blocker, "regular file", "utf8"); + const stateDir = path.join(blocker, "sealed"); // parent is a file → ENOTDIR + assert.deepEqual(hidePrompt(stateDir, "kept prompt"), { + status: "error", + message: "Could not write the hide file; the prompt may reappear.", + }); +}); diff --git a/tests/history-max-results-cap.test.ts b/tests/history-max-results-cap.test.ts new file mode 100644 index 000000000..fb319c8ec --- /dev/null +++ b/tests/history-max-results-cap.test.ts @@ -0,0 +1,76 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { filterPrompts } from "../extensions/history/selector-helpers.ts"; + +/** + * WU5 tests (AC-S5-1, AC-S5-2): the MAX_RESULTS raise 1000 → 10000 is an + * OUTPUT cap only — filterPrompts caps both of its slice sites; the load + * path never snapshots. Pure import (no pi-tui graph) plus a source-parse + * pin on the selector-helpers slice sites. + * + * The dev suite's openHistorySelector body pin (no slice() on the load + * path) covers the slice-3 selector wiring in extensions/history/index.ts + * and ports with that slice — index.ts stays at its slice-1 surface here. + */ + +interface CapRecord { + text: string; + searchText: string; +} + +function record(text: string): CapRecord { + return { text, searchText: text.toLowerCase() }; +} + +// T29 — AC-S5-1: the cap value. More than 10,000 records → exactly 10,000 at +// the empty-query slice AND at a filtered-query slice. NEW pin — no existing +// test pins the old 1000 literal (design §D9; zero existing-test edits +// repo-wide, so this file ADDS the pin instead of editing one). + +test("T29 (AC-S5-1): empty-query slice caps at the raised 10000", () => { + const records: CapRecord[] = []; + for (let i = 0; i < 10500; i++) { + records.push(record(`unique prompt number ${i}`)); + } + const result = filterPrompts(records, ""); + assert.equal(result.length, 10000); +}); + +test("T29 (AC-S5-1): filtered-query slice caps at the raised 10000", () => { + const records: CapRecord[] = []; + // 10,500 matches for the query token plus non-matching padding rows: the + // FILTERED set alone is above the cap, so the filtered slice site is the + // one being exercised here. + for (let i = 0; i < 10500; i++) { + records.push(record(`match me ${i}`)); + } + records.push(record("unrelated row one")); + records.push(record("unrelated row two")); + const result = filterPrompts(records, "match"); + assert.ok(result.length > 1000, "the filtered set must exceed the old cap"); + assert.equal(result.length, 10000); + assert.ok(result.every((r) => r.searchText.includes("match"))); +}); + +// T30 — AC-S5-2: output-cap-only semantics (source-parse). Selector-helpers +// reads the constant at exactly the two sanctioned filterPrompts slice +// sites — no other cap exists in the helper module. + +const helperSource = fs.readFileSync( + fileURLToPath( + new URL("../extensions/history/selector-helpers.ts", import.meta.url), + ), + "utf8", +); + +test("T30 (AC-S5-2): filterPrompts hosts exactly the two sanctioned cap slice sites", () => { + const sliceSites = helperSource.split("slice(0, MAX_RESULTS)").length - 1; + assert.equal( + sliceSites, + 2, + "filterPrompts hosts exactly the two sanctioned cap slice sites", + ); +}); From 73c55ff99193804815c80058b77b5c3e57195c55 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:38:16 -0300 Subject: [PATCH 04/14] docs(history): correct drainGlobal seed-ordering docblock Review fix (Copilot suppressed comment, store.ts): the docblock claimed the legacy global seed is the "newest single source", but the code deliberately appends it after sorting (`// legacy last`) so per-project entries win recency and keep-first dedup. Document the actual, intended behavior instead of changing it: migrated legacy history is the least specific source. --- extensions/history/store.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/history/store.ts b/extensions/history/store.ts index bf2a41c16..fb470515c 100644 --- a/extensions/history/store.ts +++ b/extensions/history/store.ts @@ -368,8 +368,10 @@ export function drainProject( } /** - * Drain the GLOBAL scope: the legacy global seed (newest single source) - * plus every project dir's files, mtime-newest-first, deduped, capped. + * Drain the GLOBAL scope: every project dir's files, mtime-newest-first, + * deduped, capped — with the legacy global seed appended LAST (deliberate: + * it is the least specific, migrated source, so per-project entries win + * recency and keep-first dedup favors them). */ export function drainGlobal( root: string, From 366b17945cc752ebb6b858fe367b74b3502a043f Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:11:48 -0300 Subject: [PATCH 05/14] feat(history): history selector TUI and command/shortcut wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 3/6 of the PR #819 split (maintainer-requested review slices). - selector-helpers: windowing/navigation subset — clamp/visible-range math, move/page selection, lazy-window growth (initial batch, grow triggers, target loading, query full-snapshot), visible-record projection, expanded-history globals hook - index.ts: PromptHistorySelector TUI (fixed-row layout, centered preview pane, search filter, Tab project/global scope toggle, grow-before-move navigation, PgDn catch-up, End full jump, wheel handling over fixed 30-row geometry, width-change pre-clamp), overlay glue (bottom-center anchored ctx.ui.custom factory), drainForScope + recordsFromEntries, wiring for ctrl+shift+r shortcut, history command, and tool_call overlay dismissal - upstream dead code dropped: notifyIndexProgress/activeIndexProgress sink pair (never fired) and unused fs import - deletion is slice 5: no deleteCurrent, no delete dispatch entry, no delete affordance in the footer hint yet - getWriter still performs no migration/seed bootstrap (slice 4); the selector drains live stores only - tests: 57 new node:test cases (cumulative 110/110): windowing math, lazy window growth contracts, preview layout, 11-entry dispatch table, wheel routing, expanded globals, shortcut/command registration surface, open-close flow with fake ctx; superseded slice-1 registration pin updated to the slice-3 wiring surface Gates: cumulative scoped history tests 110/110 green. esbuild bundle parse of the full extension graph clean. Known pre-existing environmental gate failures unchanged. --- extensions/history/index.ts | 889 ++++++++++++++++++++- extensions/history/selector-helpers.ts | 172 ++++ tests/history-command-registration.test.ts | 84 ++ tests/history-dispatch.test.ts | 179 +++++ tests/history-expanded-globals.test.ts | 62 ++ tests/history-lazy-windowing.test.ts | 508 ++++++++++++ tests/history-openflow-integration.test.ts | 143 ++++ tests/history-preview-layout.test.ts | 56 ++ tests/history-selector-windowing.test.ts | 94 +++ tests/history-session-writer.test.ts | 26 +- tests/history-wheel-mouse.test.ts | 242 ++++++ 11 files changed, 2442 insertions(+), 13 deletions(-) create mode 100644 tests/history-command-registration.test.ts create mode 100644 tests/history-dispatch.test.ts create mode 100644 tests/history-expanded-globals.test.ts create mode 100644 tests/history-lazy-windowing.test.ts create mode 100644 tests/history-openflow-integration.test.ts create mode 100644 tests/history-preview-layout.test.ts create mode 100644 tests/history-selector-windowing.test.ts create mode 100644 tests/history-wheel-mouse.test.ts diff --git a/extensions/history/index.ts b/extensions/history/index.ts index 26616733f..9fe2d7893 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -1,21 +1,77 @@ // SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history // SPDX-License-Identifier: MIT -// Prompt-history extension entry (slice 1): identity constants, the -// per-instance writer lifecycle, and the before_agent_start capture -// handler. Selector UI, shortcut/command, scope drains, legacy migration -// and seed bootstrap, and GC arrive in later slices. +// Prompt-history extension entry (slice 3): the selector TUI, overlay glue, +// and the shortcut/command wiring over the slice-1 writer and slice-2 +// drains. Legacy migration and seed bootstrap (slice 4), deletion (slice 5), +// and GC/compaction (slice 6) arrive in later slices. -import { randomUUID } from "node:crypto"; -import { homedir } from "node:os"; import { join } from "node:path"; -import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import { homedir } from "node:os"; +import { + DynamicBorder, + type ExtensionAPI, + type ShortcutContext, + type Theme, +} from "@earendil-works/pi-coding-agent"; import { appendSessionCapture, + drainGlobal, + drainProject, ensureRegistryEntry, openSessionWriter, type SessionWriterState, } from "./store.ts"; +import { randomUUID } from "node:crypto"; +import { + buildPromptRecords, + filterPrompts, + type PromptEntry, + clampPreviewOffset, + clampSelectedIndex, + dedupePromptEntries, + getVisiblePromptRecords, + initialLoadedCount, + loadedCountForQuery, + loadedCountForTarget, + moveSelectedIndex, + nextLoadedCount, + pageSelectedIndex, + shouldGrowWindow, + withExpandedHistoryGlobals, + type PiHistoryGlobals, + type PromptRecord, +} from "./selector-helpers.ts"; +import { + Container, + type Focusable, + getKeybindings, + Input, + matchesKey, + Text, + type TUI, + type TuiMouseEvent, + truncateToWidth, +} from "@earendil-works/pi-tui"; + +const SHORTCUT = "ctrl+shift+r"; +const MAX_VISIBLE = 10; +const PREVIEW_ROWS = 10; +// Lazy windowing (design §D3; user-tuned 2026-09-08). PRELOAD_BUFFER=2 +// fires growth as the cursor enters the final 2 loaded rows; BATCH_SIZE=10 +// loads exactly one viewport per growth; INITIAL_BATCH=10 paints one +// viewport at open. PRELOAD_BUFFER <= MAX_VISIBLE keeps a jump within one +// viewport covered by the catch-up loop; review all three together. +const INITIAL_BATCH = 10; +const BATCH_SIZE = 10; +const PRELOAD_BUFFER = 3; +// Wheel regions over the fixed 30-row overlay geometry (design §D6): the +// list container renders at rows 5-14 and the preview container at rows +// 17-26; every other row is a consumed no-op. +const LIST_WHEEL_Y_FIRST = 5; +const LIST_WHEEL_Y_LAST = 14; +const PREVIEW_WHEEL_Y_FIRST = 17; +const PREVIEW_WHEEL_Y_LAST = 26; // v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). const PI_HISTORY_ROOT = join(homedir(), ".pi", "agent", "history"); @@ -24,6 +80,764 @@ const CURRENT_CWD = process.cwd(); // Instance identity: one exclusive capture file per pi process. const INSTANCE_ID = randomUUID(); +// Tombstone state dir: the store root itself (user-directed FINAL): +// ~/.pi/agent/history/hidden.json — one directory for everything. +// Derived state only — deleting the directory restores cold start and +// unhides every prompt; transcripts and the editor store are never written +// here. +const PI_HISTORY_NAV_STATE_DIR = join( + homedir(), + ".pi", + "agent", + "history", +); + +/** Width of the "→ " / " " prefix on each entry line. */ +const ENTRY_PREFIX_WIDTH = 2; + +// --------------------------------------------------------------------------- +// Sanitization +// --------------------------------------------------------------------------- + +/** + * Replace control characters with visible escape notation so the terminal + * renders them as text instead of interpreting them as commands. + * Preserves \n (newlines) and \t (tabs). + */ +function sanitizeForDisplay(text: string): string { + let out = ""; + for (let i = 0; i < text.length; i++) { + const cp = text.codePointAt(i)!; + if (cp === 0x0a) { + out += "\n"; + } else if (cp === 0x09) { + out += "\t"; + } else if (cp < 0x20 || cp === 0x7f) { + out += "\\x" + cp.toString(16).padStart(2, "0"); + } else if (cp >= 0x80 && cp < 0xa0) { + out += "\\x" + cp.toString(16).padStart(2, "0"); + } else { + out += text[i]; + } + if (cp > 0xffff) i++; // skip low surrogate of astral pair + } + return out; +} + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +/** Keybinding lookup returned by getKeybindings(). */ +interface Keybindings { + matches(data: string, action: string): boolean; +} + +type InputMatcher = (data: string, kb: Keybindings) => boolean; +type InputHandler = () => void; + +interface DispatchEntry { + match: InputMatcher; + handler: InputHandler; +} + +/** Notification sink for selector feedback; an absent callback drops notifications. */ +type SelectorNotify = (message: string, level: "error" | "warning" | "info") => void; + +/** Single rendered row; always occupies exactly one terminal row. */ +class FixedRowText { + private text: string; + private readonly centered: boolean; + + constructor(text: string = "", centered = false) { + this.text = text; + this.centered = centered; + } + + /** Replace the row content in place; padding contract comes from render(). */ + setText(next: string): void { + this.text = next; + } + + invalidate(): void {} + + render(width: number): string[] { + if (width <= 0) return [" "] as string[]; + if (this.text.length === 0) { + // Use a space so the terminal always renders this as a visible row + // and differential rendering correctly detects it as a changed line. + return [" ".repeat(width)] as string[]; + } + const rendered = this.centered + ? (() => { + // Truncate first so an overlong help row can never exceed width, + // then center the truncated copy (design §C hardening). + const truncated = truncateToWidth(this.text, width, "…"); + const visible = truncated.replace(/\x1b\[[0-9;]*m/g, ""); + const pad = Math.max(0, Math.floor((width - visible.length) / 2)); + return " ".repeat(pad) + truncated; + })() + : truncateToWidth(this.text, width, "…"); + // Pad to full terminal width so the overlay fully overwrites + // whatever is beneath it and leaves no ghost characters on dismiss. + return [rendered + " ".repeat(Math.max(0, width - rendered.length))]; + } +} + +/** Word-wrap plain text so each line fits within maxWidth characters. */ +function wordWrapText(text: string, maxWidth: number): string[] { + if (maxWidth <= 0) return [text || " "]; + const paragraphs = text.split("\n"); + const result: string[] = []; + for (const para of paragraphs) { + if (para.length === 0) { + result.push(""); + continue; + } + let remaining = para; + while (remaining.length > 0) { + if (remaining.length <= maxWidth) { + result.push(remaining); + break; + } + const breakAt = remaining.lastIndexOf(" ", maxWidth); + if (breakAt <= 0) { + result.push(remaining.substring(0, maxWidth)); + remaining = remaining.substring(maxWidth); + } else { + result.push(remaining.substring(0, breakAt)); + remaining = remaining.substring(breakAt + 1); + } + } + } + return result.length > 0 ? result : [""]; +} + +// --------------------------------------------------------------------------- +// TUI Selector +// --------------------------------------------------------------------------- + +class PromptHistorySelector extends Container implements Focusable { + private readonly searchInput: Input; + private readonly previewContainer: Container; + private readonly listContainer: Container; + private readonly headerRow: FixedRowText; + private readonly previewLabelRow: FixedRowText; + private records: PromptRecord[]; + private readonly theme: Theme; + private readonly tui: TUI; + private readonly onSelect: (record: PromptRecord) => void; + private readonly onCancel: () => void; + /** Notification sink for selector feedback (wired by the factory). */ + private readonly onNotify?: SelectorNotify; + private filteredRecords: PromptRecord[] = []; + private selectedIndex = 0; + /** Number of records loaded (newest-first) from the top of `records`. */ + private loadedCount = 0; + /** Active scope (design v2): project (default) or global. */ + private scope: "project" | "global" = "project"; + /** Last render width, used for entry truncation. */ + private lastWidth = 800; + /** Word-wrapped lines of the currently selected prompt. */ + private wrappedPreviewLines: string[] = []; + /** Scroll offset into wrappedPreviewLines for the preview viewport. */ + private previewScrollOffset = 0; + + /** Dispatch table: first match wins, fallthrough last. */ + private readonly dispatch: readonly DispatchEntry[] = [ + { + match: (_d, kb) => kb.matches(_d, "tui.select.up"), + handler: () => this.moveUp(), + }, + { + match: (_d, kb) => kb.matches(_d, "tui.select.down"), + handler: () => this.moveDown(), + }, + { + match: (_d, kb) => kb.matches(_d, "tui.select.pageUp"), + handler: () => this.pageListUp(), + }, + { + match: (_d, kb) => kb.matches(_d, "tui.select.pageDown"), + handler: () => this.pageListDown(), + }, + { + match: (d, kb) => d === "\r" || kb.matches(d, "tui.select.confirm"), + handler: () => this.selectCurrent(), + }, + { match: (d, _kb) => d === "\t", handler: () => this.toggleScope() }, + { + match: (_d, kb) => kb.matches(_d, "tui.select.cancel"), + handler: () => this.onCancel(), + }, + { + match: (d, _kb) => matchesKey(d, "home"), + handler: () => this.jumpToFirst(), + }, + { + match: (d, _kb) => matchesKey(d, "end"), + handler: () => this.jumpToLast(), + }, + { + match: (d, _kb) => matchesKey(d, "ctrl+shift+up"), + handler: () => this.previewPageUp(), + }, + { + match: (d, _kb) => matchesKey(d, "ctrl+shift+down"), + handler: () => this.previewPageDown(), + }, + ]; + + private _focused = false; + get focused(): boolean { + return this._focused; + } + set focused(value: boolean) { + this._focused = value; + this.searchInput.focused = value; + } + + constructor( + tui: TUI, + theme: Theme, + records: PromptRecord[], + onSelect: (record: PromptRecord) => void, + onCancel: () => void, + onNotify?: SelectorNotify, + ) { + super(); + this.tui = tui; + this.theme = theme; + this.records = records; + this.loadedCount = initialLoadedCount(records.length, INITIAL_BATCH); + this.onSelect = onSelect; + this.onCancel = onCancel; + this.onNotify = onNotify; + + // ── Search panel (top) ── + this.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); + this.headerRow = new FixedRowText( + theme.fg("accent", theme.bold(" History Search ")), + ); + this.addChild(this.headerRow); + this.addChild( + new Text( + theme.fg("dim", "Type to filter (multi-word AND substring, case-insensitive)"), + 0, + 0, + ), + ); + this.searchInput = new Input(); + this.searchInput.onSubmit = () => this.selectCurrent(); + this.searchInput.onEscape = () => this.onCancel(); + this.addChild(this.searchInput); + this.addChild(new DynamicBorder((s: string) => theme.fg("dim", s))); + + this.listContainer = new Container(); + this.addChild(this.listContainer); + + // ── Preview panel (bottom) ── + this.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); + this.previewLabelRow = new FixedRowText( + theme.fg("accent", theme.bold(" Preview ")), + ); + this.addChild(this.previewLabelRow); + this.previewContainer = new Container(); + this.addChild(this.previewContainer); + + this.addChild(new DynamicBorder((s: string) => theme.fg("dim", s))); + this.addChild( + new FixedRowText( + theme.fg( + "dim", + "↑↓ move • PgUp/PgDn page • tab scope • enter select and quit • ctrl+shift+↑/↓ preview • esc cancel", + ), + true /* centered */, + ), + ); + this.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); + + this.applyFilter(""); + } + + // -- Filtering & list building ------------------------------------------ + + private applyFilter(query: string): void { + // AC-L2-3r (user-directed 2026-09-08): a non-empty query implies + // full-snapshot visibility — one-shot and idempotent, never a batch — + // so per-keypress incremental loads remain impossible (C2). + this.loadedCount = loadedCountForQuery( + this.loadedCount, + this.records.length, + query, + ); + this.filteredRecords = filterPrompts( + this.records.slice(0, this.loadedCount), + query, + ); + this.selectedIndex = clampSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + ); + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + private rebuildList(): void { + this.rebuildListWithWidth(this.lastWidth); + } + + /** Rebuild list rows: header counter + entries. Always MAX_VISIBLE rows. */ + private rebuildListWithWidth(width: number): void { + const count = this.filteredRecords.length; + const position = count === 0 ? 0 : this.selectedIndex + 1; + this.headerRow.setText( + this.theme.fg("accent", this.theme.bold(" History Search ")) + + this.theme.fg("dim", ` · ${position} of ${count} `) + + this.theme.fg( + "dim", + ` · loaded ${this.loadedCount} of ${this.records.length} `, + ) + + // Right-aligned scope radio: pad from plain-text lengths so the + // radio ends flush at the header's last column at any width. + (() => { + const scopeRadio = + this.scope === "project" + ? "◉ Current project | ○ All projects" + : "○ Current project | ◉ All projects"; + const leftWidth = + " History Search ".length + + ` · ${position} of ${count} `.length + + ` · loaded ${this.loadedCount} of ${this.records.length} `.length; + return ( + " ".repeat(Math.max(1, width - leftWidth - scopeRadio.length)) + + this.theme.fg("dim", scopeRadio) + ); + })(), + ); + this.listContainer.clear(); + + if (count === 0) { + this.listContainer.addChild( + new FixedRowText(this.theme.fg("warning", "No matching prompts")), + ); + for (let i = 1; i < MAX_VISIBLE; i++) { + this.listContainer.addChild(new FixedRowText()); + } + return; + } + + const entryMax = Math.floor(width * 0.95) - ENTRY_PREFIX_WIDTH; + + const visible = getVisiblePromptRecords( + this.filteredRecords, + this.selectedIndex, + MAX_VISIBLE, + ); + + for (const { record, isSelected } of visible) { + const prefix = isSelected ? "→ " : " "; + const color = isSelected ? "accent" : "text"; + const compacted = sanitizeForDisplay(record.text) + .replace(/\s+/g, " ") + .trim(); + const truncated = truncateToWidth(compacted, entryMax, "…"); + const line = prefix + this.theme.fg(color, truncated); + this.listContainer.addChild(new FixedRowText(line)); + } + + for (let i = visible.length; i < MAX_VISIBLE; i++) { + this.listContainer.addChild(new FixedRowText()); + } + } + + /** + * Rebuild preview: word-wrap the full selected prompt text and show + * a PREVIEW_ROWS-tall viewport starting at previewScrollOffset. + * Content starts immediately below the "Preview" label (no top padding). + * PgUp/PgDn scroll through the wrapped lines. + */ + private rebuildPreviewWithWidth(width: number): void { + this.previewContainer.clear(); + + const wrapWidth = Math.max(1, width - 2); + const selected = this.filteredRecords[this.selectedIndex]; + if (selected) { + const safeText = sanitizeForDisplay(selected.text); + this.wrappedPreviewLines = wordWrapText(safeText, wrapWidth); + this.previewScrollOffset = clampPreviewOffset( + this.previewScrollOffset, + this.wrappedPreviewLines.length, + PREVIEW_ROWS, + ); + } else { + this.wrappedPreviewLines = []; + this.previewScrollOffset = 0; + } + + // P1-3 indicator: fresh wrap is known here — one update site covers all + // paths; the label appends the 1-based range only when content overflows. + this.previewLabelRow.setText(this.previewLabelRowText()); + + for (let i = 0; i < PREVIEW_ROWS; i++) { + const lineIdx = this.previewScrollOffset + i; + if (lineIdx < this.wrappedPreviewLines.length) { + // Pad the plain text to wrapWidth so FixedRowText.render() + // never truncates — the visible width is always ≤ width-2. + const raw = this.wrappedPreviewLines[lineIdx]; + const padded = raw + " ".repeat(Math.max(0, wrapWidth - raw.length)); + this.previewContainer.addChild( + new FixedRowText(this.theme.fg("text", padded)), + ); + } else { + this.previewContainer.addChild(new FixedRowText()); + } + } + } + + /** " Preview " label; appends the 1-based visible range only on overflow. */ + private previewLabelRowText(): string { + const total = this.wrappedPreviewLines.length; + if (total <= PREVIEW_ROWS) { + return this.theme.fg("accent", this.theme.bold(" Preview ")); + } + const start = this.previewScrollOffset + 1; + const end = Math.min(this.previewScrollOffset + PREVIEW_ROWS, total); + return this.theme.fg( + "accent", + this.theme.bold(` Preview — ${start}–${end}/${total} `), + ); + } + + private rebuildPreview(): void { + this.rebuildPreviewWithWidth(this.lastWidth); + } + + // -- Selection actions -------------------------------------------------- + + private selectCurrent(): void { + const selected = this.filteredRecords[this.selectedIndex]; + if (selected) this.onSelect(selected); + } + + /** + * Toggle project <-> global (design v2): re-drain the other scope, + * rebuild the merged records, reset the window. Tab's only role. + */ + private toggleScope(): void { + this.scope = this.scope === "project" ? "global" : "project"; + const entries = drainForScope(this.scope); + this.records = recordsFromEntries(entries); + this.loadedCount = initialLoadedCount(this.records.length, INITIAL_BATCH); + this.applyFilter(this.searchInput.getValue()); + } + + // -- Navigation --------------------------------------------------------- + + private moveUp(): void { + this.selectedIndex = moveSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + -1, + ); + if ( + shouldGrowWindow( + this.selectedIndex, + this.loadedCount, + this.records.length, + PRELOAD_BUFFER, + ) + ) { + this.loadedCount = nextLoadedCount( + this.loadedCount, + this.records.length, + BATCH_SIZE, + ); + this.applyFilter(this.searchInput.getValue()); + } + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + private moveDown(): void { + // Grow-before-move (design §D1): the C2 trigger fires while the cursor + // sits in the final PRELOAD_BUFFER rows of the loaded window, so the + // modulo below moves into freshly loaded rows — a wrap to index 0 is + // reachable only on the exhausted set. + if ( + shouldGrowWindow( + this.selectedIndex, + this.loadedCount, + this.records.length, + PRELOAD_BUFFER, + ) + ) { + this.loadedCount = nextLoadedCount( + this.loadedCount, + this.records.length, + BATCH_SIZE, + ); + this.applyFilter(this.searchInput.getValue()); + } + this.selectedIndex = moveSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + 1, + ); + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + /** Page the LIST up by MAX_VISIBLE with clamping (no wrap). */ + private pageListUp(): void { + this.selectedIndex = pageSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + -MAX_VISIBLE, + ); + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + /** Page the LIST down by MAX_VISIBLE with clamping (no wrap). */ + private pageListDown(): void { + // PgDn catch-up (design §D7): grow in whole batches until the paged-to + // row is loaded BEFORE the selection lands on it. + const grown = loadedCountForTarget( + this.loadedCount, + this.records.length, + this.selectedIndex + MAX_VISIBLE, + BATCH_SIZE, + ); + if (grown !== this.loadedCount) { + this.loadedCount = grown; + this.applyFilter(this.searchInput.getValue()); + } + this.selectedIndex = pageSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + MAX_VISIBLE, + ); + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + private previewPageUp(): void { + this.previewScrollOffset = Math.max( + 0, + this.previewScrollOffset - PREVIEW_ROWS, + ); + this.rebuildPreview(); + } + + private previewPageDown(): void { + this.previewScrollOffset = clampPreviewOffset( + this.previewScrollOffset + PREVIEW_ROWS, + this.wrappedPreviewLines.length, + PREVIEW_ROWS, + ); + this.rebuildPreview(); + } + + private jumpToFirst(): void { + if (this.filteredRecords.length === 0) return; + this.selectedIndex = 0; + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + private jumpToLast(): void { + // End full-jump (design §D7): one-shot load of everything BEFORE the + // empty guard, so End also surfaces matches beyond the window. + if (this.loadedCount < this.records.length) { + this.loadedCount = this.records.length; + this.applyFilter(this.searchInput.getValue()); + } + if (this.filteredRecords.length === 0) return; + this.selectedIndex = this.filteredRecords.length - 1; + this.previewScrollOffset = 0; + this.rebuildList(); + this.rebuildPreview(); + } + + // -- Input handling ----------------------------------------------------- + + private forwardToSearch(data: string): void { + this.searchInput.handleInput(data); + this.selectedIndex = 0; + this.applyFilter(this.searchInput.getValue()); + } + + handleInput(data: string): void { + const kb = getKeybindings(); + let handled = false; + for (const { match, handler } of this.dispatch) { + if (match(data, kb)) { + handler(); + handled = true; + break; + } + } + if (!handled) this.forwardToSearch(data); + this.tui.requestRender(); + } + + // -- Mouse (wheel-only) ------------------------------------------------- + + /** + * Wheel-only mouse handling over the fixed 30-row geometry (design + * §D6). Non-wheel events stay host-owned (undefined = Container child + * dispatch); EVERY wheel path — including the no-op regions — reaches + * the single consumed return, closing the pre-existing SGR-fallthrough + * hazard where raw wheel bytes were typed into the search box. + */ + override handleMouse( + event: TuiMouseEvent, + ): ReturnType { + if (event.type !== "wheel") return undefined; + const delta = event.wheelDelta ?? 0; + if (event.y >= LIST_WHEEL_Y_FIRST && event.y <= LIST_WHEEL_Y_LAST) { + const steps = Math.min(Math.abs(delta), this.filteredRecords.length); + for (let i = 0; i < steps; i++) { + if (delta > 0) this.moveDown(); + else this.moveUp(); + } + } else if ( + event.y >= PREVIEW_WHEEL_Y_FIRST && + event.y <= PREVIEW_WHEEL_Y_LAST + ) { + if (delta !== 0) { + this.previewScrollOffset = clampPreviewOffset( + this.previewScrollOffset + (delta > 0 ? 1 : -1), + this.wrappedPreviewLines.length, + PREVIEW_ROWS, + ); + this.rebuildPreview(); + } + } + return { + handled: true, + target: { + component: this, + originX: event.screenX - event.x, + originY: event.screenY - event.y, + width: event.width, + height: event.height, + }, + }; + } + + // -- Render override for dynamic entry width --------------------------- + + /** Fixed overlay height so the TUI never repositions the panel. */ + private static readonly OVERLAY_LINES = 30; + + override render(width: number): string[] { + if (width !== this.lastWidth) { + // Pre-clamp against the previous wrap so a width change can never + // drive the rebuilds with a stale selection/offset (AC-P1-4.1/4.2). + this.selectedIndex = clampSelectedIndex( + this.selectedIndex, + this.filteredRecords.length, + ); + this.previewScrollOffset = clampPreviewOffset( + this.previewScrollOffset, + this.wrappedPreviewLines.length, + PREVIEW_ROWS, + ); + } + this.lastWidth = width; + this.rebuildListWithWidth(width); + this.rebuildPreviewWithWidth(width); + const raw = super.render(width); + // Pad or trim to exactly OVERLAY_LINES so the overlay never shifts. + const blank = " ".repeat(Math.max(1, width)); + while (raw.length < PromptHistorySelector.OVERLAY_LINES) raw.push(blank); + return raw.slice(0, PromptHistorySelector.OVERLAY_LINES); + } +} + +// --------------------------------------------------------------------------- +// Overlay glue +// --------------------------------------------------------------------------- + +type SelectorDone = (result: PromptRecord | null) => void; + +type SelectorFactory = ( + tui: unknown, + theme: unknown, + keybindings: unknown, + done: SelectorDone, +) => PromptHistorySelector; + +function castSelectorArgs(tui: unknown, theme: unknown): [TUI, Theme] { + return [tui as TUI, theme as Theme]; +} + +/** Stored close callback for the currently-open overlay. Null when closed. */ +let activeOverlayClose: (() => void) | null = null; + +function createPromptHistorySelectorFactory( + records: PromptRecord[], + onNotify?: SelectorNotify, +): SelectorFactory { + return (tui, theme, _keybindings, done) => { + selectorTui = tui as { requestRender(): void }; + const finish = (result: PromptRecord | null) => { + activeOverlayClose = null; + done(result); + }; + // Expose close so the tool_call handler can dismiss the overlay. + activeOverlayClose = () => finish(null); + const [typedTui, typedTheme] = castSelectorArgs(tui, theme); + const selector = new PromptHistorySelector( + typedTui, + typedTheme, + records, + (record) => finish(record), + () => finish(null), + onNotify, + ); + return selector; + }; +} + +async function runPromptHistorySelection( + ctx: ShortcutContext, + records: PromptRecord[], +): Promise { + const historyGlobals: PiHistoryGlobals = globalThis as Record< + string, + unknown + >; + return withExpandedHistoryGlobals(historyGlobals, async () => + ctx.ui.custom( + createPromptHistorySelectorFactory(records, (message, level) => + ctx.ui.notify(message, level), + ), + { + overlay: true, + overlayOptions: { anchor: "bottom-center", width: "100%", offsetY: 5 }, + }, + ), + ); +} + +// --------------------------------------------------------------------------- +// Multi-concurrency store (v2): per-session writes, scope drains +// --------------------------------------------------------------------------- + +type HistoryScope = "project" | "global"; + +/** TUI handle captured when the selector overlay mounts. */ +let selectorTui: { requestRender(): void } | null = null; + let writerState: SessionWriterState | null = null; /** @@ -43,6 +857,51 @@ function getWriter(): SessionWriterState { return writerState; } +/** + * Scope drain for the selector: project scope drains the project's store + * files; global scope is the store-only cross-project view (all project + * dirs + the legacy global seed). Both filter tombstoned prompts. + */ +function drainForScope(scope: HistoryScope): string[] { + getWriter(); // ensure init ran + return scope === "project" + ? drainProject(PI_HISTORY_ROOT, CURRENT_CWD, 1000, PI_HISTORY_NAV_STATE_DIR) + : drainGlobal(PI_HISTORY_ROOT, 1000, PI_HISTORY_NAV_STATE_DIR); +} + +async function openHistorySelector( + ctx: Pick, +): Promise { + // Store-only drain (user-directed): both scopes read the store files + // symmetrically — no live transcript merge (the one-time seed bootstrap + // covers pre-store history). + const entries = drainForScope("project"); + if (entries.length === 0) { + ctx.ui.notify("No prompt history available.", "warning"); + return; + } + + const records = recordsFromEntries(entries); + const selected = await runPromptHistorySelection(ctx, records); + if (selected) { + // pasteToEditor routes through the editor's input pipeline + // (bracketed paste), so the text renders immediately. Plain + // setText left the editor stale until the next keypress after + // overlay close. + ctx.ui.pasteToEditor(selected.text); + // The overlay teardown can race the paste render: force one more + // frame on the next tick so the editor box shows the text at once. + setTimeout(() => selectorTui?.requestRender(), 0); + } +} + +/** Build selector records from merged/drain entries (shared by both scopes). */ +function recordsFromEntries( + entries: Array, +): PromptRecord[] { + return buildPromptRecords(dedupePromptEntries(entries)); +} + export default function promptHistoryExtension(pi: ExtensionAPI) { // One writer per extension load; see getWriter() for the init order. @@ -57,4 +916,20 @@ export default function promptHistoryExtension(pi: ExtensionAPI) { // the handler - swallow and keep the next prompt capturable. } }); + + // When a tool asks for user input while the history overlay is open, + // dismiss the overlay so the tool can take over the UI. + pi.on("tool_call", () => { + activeOverlayClose?.(); + }); + + pi.registerShortcut(SHORTCUT, { + description: "Search prompt history", + handler: async (ctx) => openHistorySelector(ctx), + }); + + pi.registerCommand("history", { + description: "Search prompt history", + handler: async (_args, ctx) => openHistorySelector(ctx), + }); } diff --git a/extensions/history/selector-helpers.ts b/extensions/history/selector-helpers.ts index c5257bafe..fd02400ce 100644 --- a/extensions/history/selector-helpers.ts +++ b/extensions/history/selector-helpers.ts @@ -25,6 +25,22 @@ export interface PromptEntry { ts?: number; } +export interface VisibleRange { + start: number; + end: number; +} + +export interface VisiblePromptRecord { + index: number; + record: PromptRecord; + isSelected: boolean; +} + +export interface PiHistoryGlobals { + __piHistoryExpand?: () => void; + __piHistoryTrim?: () => void; +} + export function buildPromptRecords( entries: ReadonlyArray, ): PromptRecord[] { @@ -46,6 +62,56 @@ export function buildPromptRecords( }); } +export function clampSelectedIndex( + selectedIndex: number, + total: number, +): number { + return Math.max(0, Math.min(selectedIndex, Math.max(0, total - 1))); +} + +export function clampPreviewOffset( + offset: number, + totalLines: number, + viewportRows: number, +): number { + return Math.max(0, Math.min(offset, Math.max(0, totalLines - viewportRows))); +} + +export function computeVisibleRange( + selectedIndex: number, + total: number, + maxVisible: number, +): VisibleRange { + if (total <= 0 || maxVisible <= 0) return { start: 0, end: 0 }; + if (total <= maxVisible) return { start: 0, end: total }; + + const half = Math.floor(maxVisible / 2); + const start = Math.max(0, Math.min(selectedIndex - half, total - maxVisible)); + + return { + start, + end: Math.min(start + maxVisible, total), + }; +} + +export function moveSelectedIndex( + selectedIndex: number, + total: number, + delta: number, +): number { + if (total === 0) return 0; + return (selectedIndex + delta + total) % total; +} + +export function pageSelectedIndex( + selectedIndex: number, + total: number, + pageSize: number, +): number { + if (total === 0) return 0; + return clampSelectedIndex(selectedIndex + pageSize, total); +} + /** * Normalization key for read-time dedup (spec C3): byte-matches the * APPLIED patch key in nav/patches/editor.cjs (:480-:586) — whitespace @@ -87,6 +153,112 @@ export function dedupePromptEntries( return deduped; } +/** + * First-paint window size (spec C1, AC-L1-1): min(initialBatch, total), + * floored at 0 — small stores open fully loaded (exhausted at open), + * identical to today's behavior for R <= INITIAL_BATCH. + */ +export function initialLoadedCount( + total: number, + initialBatch: number, +): number { + return Math.max(0, Math.min(initialBatch, total)); +} + +/** + * Prefetch trigger (spec C2's normative expression, AC-L2-1): growth fires + * iff rows remain unloaded AND the 0-based cursor sits within the final + * preloadBuffer rows of the loaded window. Reads UNFILTERED counts only — + * filteredRecords.length appears in no trigger arithmetic (AC-L2-2). + */ +export function shouldGrowWindow( + selectedIndex: number, + loadedCount: number, + totalCount: number, + preloadBuffer: number, +): boolean { + return ( + loadedCount < totalCount && selectedIndex + preloadBuffer >= loadedCount + ); +} + +/** + * One growth step (spec C2, AC-L2-1): min(L + max(1, batchSize), R). The + * max(1, ·) guard also keeps loadedCountForTarget's loop terminating on a + * degenerate batch size. + */ +export function nextLoadedCount( + loadedCount: number, + totalCount: number, + batchSize: number, +): number { + const step = Math.max(1, batchSize); + return Math.min(loadedCount + step, totalCount); +} + +/** + * PgDn catch-up (spec C1, AC-L1-5): the smallest whole-batch count that + * strictly covers targetIndex (a 0-based master row), clamped at totalCount. + * No-op when the target is already covered or the window is exhausted. + * Terminates by construction: each step adds ≥ 1, bounded by totalCount. + */ +export function loadedCountForTarget( + loadedCount: number, + totalCount: number, + targetIndex: number, + batchSize: number, +): number { + let next = loadedCount; + while (next <= targetIndex && next < totalCount) { + next = nextLoadedCount(next, totalCount, batchSize); + } + return next; +} + +export function getVisiblePromptRecords( + records: PromptRecord[], + selectedIndex: number, + maxVisible: number, +): VisiblePromptRecord[] { + const { start, end } = computeVisibleRange( + selectedIndex, + records.length, + maxVisible, + ); + return records.slice(start, end).map((record, offset) => ({ + index: start + offset, + record, + isSelected: start + offset === selectedIndex, + })); +} + +export async function withExpandedHistoryGlobals( + globals: PiHistoryGlobals, + run: () => Promise, +): Promise { + globals.__piHistoryExpand?.(); + try { + return await run(); + } finally { + globals.__piHistoryTrim?.(); + } +} + +/** + * Full-snapshot visibility for non-empty queries (AC-L2-3r, user-directed + * 2026-09-08): searching must see the whole deduped snapshot, not just the + * loaded prefix. One-shot and idempotent — returns the total, never an + * incremental batch — so per-keypress growth stays impossible. Empty or + * whitespace-only queries leave the lazy window untouched. + */ +export function loadedCountForQuery( + loadedCount: number, + totalCount: number, + query: string, +): number { + return query.trim().length > 0 ? totalCount : loadedCount; +} + const MAX_RESULTS = 10000; export function filterPrompts( diff --git a/tests/history-command-registration.test.ts b/tests/history-command-registration.test.ts new file mode 100644 index 000000000..e4aa9bf6b --- /dev/null +++ b/tests/history-command-registration.test.ts @@ -0,0 +1,84 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; + +// Source-parsing tests (preview-layout.test.ts pattern): never import +// extensions/history/index.ts — it pulls the pi-tui runtime graph (§D3). + +const sourcePath = fileURLToPath( + new URL("../extensions/history/index.ts", import.meta.url), +); +const source = fs.readFileSync(sourcePath, "utf8"); + +test("openHistorySelector is extracted once and shared by both entry points", () => { + const definitions = + source.split("async function openHistorySelector(").length - 1; + assert.strictEqual( + definitions, + 1, + "openHistorySelector should be defined exactly once", + ); + + const calls = source.split("openHistorySelector(ctx)").length - 1; + assert.strictEqual( + calls, + 2, + "registerShortcut and registerCommand handlers should both call openHistorySelector(ctx)", + ); + + // PR-branch (slice 3) behavior: the store-only drain keeps the empty + // guard — no history means a warning, not an empty overlay. (The dev + // repo's later always-open selector dropped this guard; the PR branch is + // the API truth here.) + const start = source.indexOf("async function openHistorySelector("); + const end = source.indexOf("export default function", start); + assert.notStrictEqual(end, -1, "extension entry point should follow"); + const body = source.slice(start, end); + assert.ok( + body.includes("if (entries.length === 0)") && + body.includes('"No prompt history available."'), + "an empty history warns and skips the overlay (PR-branch drain guard)", + ); +}); + +test("the /history command is registered beside the shortcut", () => { + const index = source.indexOf('pi.registerCommand("history"'); + assert.ok(index >= 0, 'pi.registerCommand("history", ...) should exist'); + + const slice = source.slice(index, index + 200); + assert.ok( + slice.includes('"Search prompt history"'), + "command should carry the same description as the shortcut", + ); + assert.ok( + slice.includes("openHistorySelector(ctx)"), + "command handler should route through the shared entry point", + ); +}); + +test("the ctrl+shift+r shortcut is registered with the shared description", () => { + const index = source.indexOf("pi.registerShortcut(SHORTCUT"); + assert.ok(index >= 0, "pi.registerShortcut(SHORTCUT, ...) should exist"); + + const slice = source.slice(index, index + 200); + assert.ok( + slice.includes('"Search prompt history"'), + "shortcut should carry the shared description", + ); + assert.ok( + slice.includes("openHistorySelector(ctx)"), + "shortcut handler should route through the shared entry point", + ); +}); + +test("in-UI hint describes multi-word AND substring matching, not fuzzy", () => { + assert.ok( + !source.includes("fzf-style fuzzy match"), + "the fzf-style fuzzy match claim must be removed (AC-P1-6.1)", + ); + assert.ok( + source.includes("multi-word AND substring"), + "hint should describe multi-word AND substring filtering (AC-P1-6.1)", + ); +}); diff --git a/tests/history-dispatch.test.ts b/tests/history-dispatch.test.ts new file mode 100644 index 000000000..575a4d5a1 --- /dev/null +++ b/tests/history-dispatch.test.ts @@ -0,0 +1,179 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; + +/** + * Dispatch table structural tests — source-parsed (AC-P2-1.3, AC-P2-2.1, + * AC-P2-4.1), following the preview-layout.test.ts pattern. + * + * PromptHistorySelector is private to extensions/history/index.ts and needs + * the pi-tui runtime (Container, Input, TUI, Theme), so these tests read the + * source file and pin the normative §B2 shape instead of importing it: + * exactly 11 explicit entries in a fixed order (the ctrl+shift+backspace + * delete entry joins with deletion in slice 5), then the implicit + * forwardToSearch fallthrough inside handleInput. + */ + +const sourcePath = fileURLToPath( + new URL("../extensions/history/index.ts", import.meta.url), +); +const source = fs.readFileSync(sourcePath, "utf8"); + +const DISPATCH_DECL = "private readonly dispatch: readonly DispatchEntry[] = ["; +const TABLE_CLOSE = "\n ];"; + +/** §B2 normative matcher order — the exact literal as it appears per entry. */ +const EXPECTED_MATCHERS = [ + 'kb.matches(_d, "tui.select.up")', + 'kb.matches(_d, "tui.select.down")', + 'kb.matches(_d, "tui.select.pageUp")', + 'kb.matches(_d, "tui.select.pageDown")', + 'd === "\\r" || kb.matches(d, "tui.select.confirm")', + 'd === "\\t"', + 'kb.matches(_d, "tui.select.cancel")', + 'matchesKey(d, "home")', + 'matchesKey(d, "end")', + 'matchesKey(d, "ctrl+shift+up")', + 'matchesKey(d, "ctrl+shift+down")', +]; + +/** Handler each entry must invoke (searched within the entry's body). */ +const EXPECTED_HANDLERS = [ + "this.moveUp()", + "this.moveDown()", + "this.pageListUp()", + "this.pageListDown()", + "this.selectCurrent()", + "this.toggleScope()", + "this.onCancel()", + "this.jumpToFirst()", + "this.jumpToLast()", + "this.previewPageUp()", + "this.previewPageDown()", +]; + +function dispatchTable(): string { + const start = source.indexOf(DISPATCH_DECL); + assert.notStrictEqual( + start, + -1, + "dispatch table declaration should exist in extensions/history/index.ts", + ); + const end = source.indexOf(TABLE_CLOSE, start); + assert.notStrictEqual(end, -1, "dispatch table closing should exist"); + return source.slice(start, end); +} + +/** Entry i's body: from its matcher literal to the next matcher (or table end). */ +function entryBody(table: string, index: number): string { + const start = table.indexOf(EXPECTED_MATCHERS[index]); + const next = + index + 1 < EXPECTED_MATCHERS.length + ? table.indexOf(EXPECTED_MATCHERS[index + 1]) + : table.length; + return table.slice(start, next === -1 ? table.length : next); +} + +function methodBody(name: string): string { + const start = source.indexOf(`private ${name}(): void {`); + assert.notStrictEqual(start, -1, `private ${name}() should exist`); + const end = source.indexOf("\n }", start); + assert.notStrictEqual(end, -1, `private ${name}() body should close`); + return source.slice(start, end); +} + +describe("dispatch table (source-parsed, §B2)", () => { + it("has exactly 11 explicit match: entries (AC-P2-4.1)", () => { + const table = dispatchTable(); + const matchCount = table.split("match:").length - 1; + assert.strictEqual( + matchCount, + 11, + `expected 11 explicit entries, found ${matchCount}`, + ); + }); + + it("keeps the exact §B2 matcher order", () => { + const table = dispatchTable(); + let cursor = -1; + EXPECTED_MATCHERS.forEach((matcher, i) => { + const at = table.indexOf(matcher); + assert.notStrictEqual( + at, + -1, + `entry #${i + 1} matcher missing: ${matcher}`, + ); + assert.ok( + at > cursor, + `entry #${i + 1} matcher out of order: ${matcher}`, + ); + cursor = at; + }); + }); + + it("wires every entry handler per §B2", () => { + const table = dispatchTable(); + EXPECTED_HANDLERS.forEach((handler, i) => { + const body = entryBody(table, i); + assert.ok( + body.includes(handler), + `entry #${i + 1} should call ${handler}`, + ); + }); + }); + + it("pages the LIST via pageSelectedIndex and resets the preview offset (AC-P2-1.3)", () => { + const up = methodBody("pageListUp"); + assert.ok( + up.includes("pageSelectedIndex("), + "pageListUp must clamp via pageSelectedIndex", + ); + assert.ok( + up.includes("-MAX_VISIBLE"), + "pageListUp must page up by one page", + ); + assert.ok( + up.includes("previewScrollOffset = 0"), + "pageListUp must reset the preview offset", + ); + const down = methodBody("pageListDown"); + assert.ok( + down.includes("pageSelectedIndex("), + "pageListDown must clamp via pageSelectedIndex", + ); + assert.ok( + down.includes("MAX_VISIBLE"), + "pageListDown must page down by one page", + ); + assert.ok( + down.includes("previewScrollOffset = 0"), + "pageListDown must reset the preview offset", + ); + }); + + it("runs the ctrl+shift combos before the implicit fallthrough (AC-P2-2.1)", () => { + const table = dispatchTable(); + const lastMatch = table.lastIndexOf("match:"); + assert.ok( + table.slice(lastMatch).includes('matchesKey(d, "ctrl+shift+down")'), + "the final table entry must be the ctrl+shift+down combo", + ); + const loopAt = source.indexOf( + "for (const { match, handler } of this.dispatch) {", + ); + const fallthroughAt = source.indexOf( + "if (!handled) this.forwardToSearch(data);", + ); + assert.notStrictEqual(loopAt, -1, "dispatch loop should exist"); + assert.notStrictEqual( + fallthroughAt, + -1, + "forwardToSearch fallthrough should exist", + ); + assert.ok( + fallthroughAt > loopAt, + "fallthrough must run after the dispatch loop", + ); + }); +}); diff --git a/tests/history-expanded-globals.test.ts b/tests/history-expanded-globals.test.ts new file mode 100644 index 000000000..e66d92163 --- /dev/null +++ b/tests/history-expanded-globals.test.ts @@ -0,0 +1,62 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + type PiHistoryGlobals, + withExpandedHistoryGlobals, +} from "../extensions/history/selector-helpers.ts"; + +// withExpandedHistoryGlobals contract: optional hooks invoked via `?.` — +// expand exactly once BEFORE the run starts, trim exactly once in the +// finally (success and rejection alike), and the run's resolution passes +// through untouched. Fixtures track call order in one array so the +// before/after ordering and the call counts are pinned together. + +function trackingGlobals(): { globals: PiHistoryGlobals; events: string[] } { + const events: string[] = []; + return { + events, + globals: { + __piHistoryExpand: () => { + events.push("expand"); + }, + __piHistoryTrim: () => { + events.push("trim"); + }, + }, + }; +} + +test("expand runs once before the run; trim once after; the result passes through", async () => { + const { globals, events } = trackingGlobals(); + let ran = 0; + const value = await withExpandedHistoryGlobals(globals, async () => { + // By the time the body executes, expand already ran — exactly once. + ran += 1; + assert.deepEqual(events, ["expand"]); + return 42; + }); + assert.equal(value, 42); + assert.equal(ran, 1); + assert.deepEqual(events, ["expand", "trim"]); +}); + +test("a rejected run still trims (finally) and the rejection propagates unchanged", async () => { + const { globals, events } = trackingGlobals(); + const boom = new Error("boom"); + let caught: unknown; + try { + await withExpandedHistoryGlobals(globals, async () => { + throw boom; + }); + } catch (error) { + caught = error; + } + assert.equal(caught, boom); + assert.deepEqual(events, ["expand", "trim"]); +}); + +test("absent hooks are tolerated: the run executes with no throw", async () => { + const empty: PiHistoryGlobals = {}; + const value = await withExpandedHistoryGlobals(empty, async () => "ok"); + assert.equal(value, "ok"); +}); diff --git a/tests/history-lazy-windowing.test.ts b/tests/history-lazy-windowing.test.ts new file mode 100644 index 000000000..5ee40838a --- /dev/null +++ b/tests/history-lazy-windowing.test.ts @@ -0,0 +1,508 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; +import { + buildPromptRecords, + filterPrompts, + initialLoadedCount, + loadedCountForQuery, + loadedCountForTarget, + moveSelectedIndex, + nextLoadedCount, + shouldGrowWindow, +} from "../extensions/history/selector-helpers.ts"; + +// Unit 2a — L1+L2 windowing helpers (spec C1/C2, design §D3/§D4). +// +// The ratified constant VALUES (design R3) are pinned here as test literals +// while the named constants themselves land in extensions/history/index.ts: +// +// INITIAL_BATCH = 10 · BATCH_SIZE = 10 · PRELOAD_BUFFER = 3 (trigger at 8th; milestones 10/20/30) +// +// Every helper is a parameterized pure function over UNFILTERED counts only: +// `filteredRecords.length` appears in no trigger or growth expression (the +// §8a regression pin, AC-L2-2). All behaviors below use the helpers exactly +// as the §B2 wiring does in the selector — grow-before-move, one batch per +// threshold crossing, derived exhaustion (no stored flag). + +// T4 — AC-L1-1: initial window clamp, min(INITIAL_BATCH, records.length). + +test("initialLoadedCount clamps the first-paint window to min(INITIAL_BATCH, records.length) (AC-L1-1)", () => { + const initialBatch = 30; + assert.equal(initialLoadedCount(0, initialBatch), 0); + assert.equal(initialLoadedCount(12, initialBatch), 12); + assert.equal(initialLoadedCount(30, initialBatch), 30); + assert.equal(initialLoadedCount(200, initialBatch), 30); +}); + +// T4 — AC-L1-2: filtering windows the loaded prefix — filterPrompts over +// records.slice(0, loadedCount) derives exclusively from that prefix; a +// match beyond the loaded count stays invisible until growth. filterPrompts +// itself is untouched (imported read-only from selector-helpers.ts). + +test("filterPrompts over the loaded prefix hides matches beyond L until growth (AC-L1-2)", () => { + const records: { text: string; searchText: string }[] = []; + for (let i = 0; i < 200; i++) { + const text = + i === 40 ? "needle40 special prompt" : `plain prompt number ${i}`; + const [record] = buildPromptRecords([text]); + assert.ok(record, "buildPromptRecords yields one record per entry"); + records.push(record); + } + + const loadedPrefix = records.slice(0, initialLoadedCount(200, 30)); + assert.equal(loadedPrefix.length, 30); + assert.equal( + filterPrompts(loadedPrefix, "needle40").length, + 0, + "the match at master index 40 sits beyond the loaded prefix — invisible until growth", + ); + assert.equal( + filterPrompts(records.slice(0, 60), "needle40").length, + 1, + "after growth to cover index 40, the match surfaces", + ); + assert.equal( + filterPrompts(loadedPrefix, "").length, + 30, + "the empty query derives exclusively from the loaded prefix", + ); +}); + +// T5 — AC-L2-1: trigger truth table with the off-by-one edges. The predicate +// is exactly `selectedIndex + preloadBuffer >= loadedCount` (0-based cursor +// within the final PRELOAD_BUFFER rows of the loaded window). + +test("shouldGrowWindow fires exactly when the cursor enters the final PRELOAD_BUFFER rows (AC-L2-1)", () => { + const totalCount = 200; + const preloadBuffer = 10; + + // One row early — a naive selected+1 paraphrase would already fire here + // (spec risk: trigger-expression off-by-one drift). + assert.equal(shouldGrowWindow(19, 30, totalCount, preloadBuffer), false); + // Exact boundary: 20 + 10 >= 30. + assert.equal(shouldGrowWindow(20, 30, totalCount, preloadBuffer), true); + assert.equal(shouldGrowWindow(29, 30, totalCount, preloadBuffer), true); + + // Grown window: cursor mid-window does not fire, the next final-buffer + // band does (exact boundary 50 + 10 >= 60). + assert.equal(shouldGrowWindow(0, 60, totalCount, preloadBuffer), false); + assert.equal(shouldGrowWindow(49, 60, totalCount, preloadBuffer), false); + assert.equal(shouldGrowWindow(50, 60, totalCount, preloadBuffer), true); + assert.equal(shouldGrowWindow(59, 60, totalCount, preloadBuffer), true); +}); + +// T5 — AC-L2-4 + AC-L1-3: exhaustion is derived — the predicate is false at +// EVERY cursor position once loadedCount equals totalCount, no stored latch. + +test("shouldGrowWindow is false at every cursor position once exhausted (AC-L2-4, AC-L1-3)", () => { + const totalCount = 200; + for (const cursor of [0, 1, 100, 189, 190, 191, 199, 500]) { + assert.equal( + shouldGrowWindow(cursor, 200, totalCount, 10), + false, + `cursor ${cursor} on the exhausted window`, + ); + } +}); + +// T5 — AC-L1-3/AC-L2-4 (source-parse): exhaustion is derivation-only — the +// selector's class-fields region stores no `exhausted`/`isLoaded` boolean +// that could go stale across query changes. + +const selectorSource = fs.readFileSync( + fileURLToPath(new URL("../extensions/history/index.ts", import.meta.url)), + "utf8", +); + +test("the selector stores no exhausted/isLoaded flag — exhaustion is derivation-only (AC-L1-3, AC-L2-4)", () => { + const classStart = selectorSource.indexOf("class PromptHistorySelector"); + assert.ok(classStart >= 0, "PromptHistorySelector should exist"); + const dispatchStart = selectorSource.indexOf( + "private readonly dispatch", + classStart, + ); + assert.ok(dispatchStart > classStart, "dispatch table should follow"); + const fieldsRegion = selectorSource.slice(classStart, dispatchStart); + assert.ok( + !fieldsRegion.includes("exhausted"), + "no stored `exhausted` flag may exist in the class fields", + ); + assert.ok( + !fieldsRegion.includes("isLoaded"), + "no stored `isLoaded` flag may exist in the class fields", + ); +}); + +// T6 — AC-L2-2 (§8a regression pin, helper half): the trigger/growth +// arithmetic lives in pure helpers over UNFILTERED counts only. The helpers +// file must carry no filteredRecords reference and must contain C2's exact +// normative predicate expression. + +test("trigger arithmetic is unfiltered-only: exact C2 predicate, no filteredRecords in growth helpers (AC-L2-2)", () => { + const helpersSource = fs.readFileSync( + fileURLToPath( + new URL("../extensions/history/selector-helpers.ts", import.meta.url), + ), + "utf8", + ); + const bodyOf = (name: string): string => { + const fnStart = helpersSource.indexOf(`export function ${name}`); + assert.ok(fnStart >= 0, `${name} should exist`); + const bodyStart = helpersSource.indexOf("{", fnStart); + const bodyEnd = helpersSource.indexOf("\n}", fnStart); + assert.ok(bodyStart >= 0 && bodyEnd > bodyStart); + return helpersSource.slice(bodyStart, bodyEnd); + }; + for (const name of [ + "shouldGrowWindow", + "nextLoadedCount", + "loadedCountForTarget", + ]) { + assert.ok( + !bodyOf(name).includes("filteredRecords"), + `${name} must read unfiltered counts only`, + ); + } + assert.ok( + bodyOf("shouldGrowWindow").includes( + "loadedCount < totalCount && selectedIndex + preloadBuffer >= loadedCount", + ), + "the predicate must be C2's exact normative expression", + ); +}); + +// T6 — AC-L2-1/AC-L2-2 (§8a walk): cursor 0→29 over total=200 with the +// ratified constants produces exactly ONE grow (+30 clamped) — one batch +// per threshold crossing, never per-keypress re-triggering. + +test("§8a walk: cursor 0→29 over total=200 produces exactly one grow (AC-L2-1, AC-L2-2)", () => { + const total = 200; + const batchSize = 30; + const preloadBuffer = 10; + let loadedCount = initialLoadedCount(total, 30); + let grows = 0; + for (let cursor = 0; cursor <= 29; cursor++) { + if (shouldGrowWindow(cursor, loadedCount, total, preloadBuffer)) { + loadedCount = nextLoadedCount(loadedCount, total, batchSize); + grows++; + } + } + assert.equal(grows, 1, "exactly one batch per threshold crossing"); + assert.equal(loadedCount, 60, "one +30 batch clamped by nothing here"); +}); + +// T6 — AC-L2-2: after that crossing the predicate stays quiet for at least +// 20 more presses — PRELOAD_BUFFER leaves a full-viewport margin (§D3). + +test("§8a walk: the next threshold crossing is at least 20 presses away (AC-L2-2)", () => { + const total = 200; + const loadedCount = 60; // state right after the first crossing (cursor 20) + let pressesToNextCrossing: number | null = null; + for (let cursor = 21; cursor <= total; cursor++) { + if (shouldGrowWindow(cursor, loadedCount, total, 10)) { + pressesToNextCrossing = cursor - 21; + break; + } + } + assert.ok( + pressesToNextCrossing !== null && pressesToNextCrossing >= 20, + "the next crossing fires at cursor 50 — 29 presses after the first (a crossing must exist)", + ); +}); + +// T7 — AC-L1-7: wrap reachability invariant as a pure simulation of the §B2 +// wiring: grow-before-move through the helpers, modulo over the loaded set. +// A wrap to index 0 occurs ONLY on the exhausted set; every record index is +// reached (no unloaded row skipped); the walk terminates. + +test("wrap-invariant walk: wrap to 0 only when exhausted, every index reached, walk terminates (AC-L1-7)", () => { + const total = 75; + const batchSize = 30; + const preloadBuffer = 10; + let loadedCount = initialLoadedCount(total, 30); + let cursor = 0; + const visited = new Set(); + let wraps = 0; + + for (let step = 0; step < 500; step++) { + visited.add(cursor); + // §B2 grow-before-move: fire the C2 trigger, one batch per crossing. + if (shouldGrowWindow(cursor, loadedCount, total, preloadBuffer)) { + loadedCount = nextLoadedCount(loadedCount, total, batchSize); + } + const next = moveSelectedIndex(cursor, loadedCount, 1); + if (next === 0) { + wraps++; + assert.ok( + loadedCount >= total, + "wrap to index 0 must occur only when loadedCount >= records.length", + ); + break; + } + cursor = next; + } + + assert.equal(wraps, 1, "the walk must terminate via a single full wrap"); + assert.equal(loadedCount, total, "the window must be exhausted at wrap time"); + assert.equal( + visited.size, + total, + "every record index 0..74 must be reached — no unloaded row skipped", + ); + for (let i = 0; i < total; i++) { + assert.ok(visited.has(i), `record index ${i} must be reachable`); + } +}); + +// T8 — AC-L1-5: loadedCountForTarget table (PgDn catch-up semantics). + +test("loadedCountForTarget: covered target is a no-op (AC-L1-5)", () => { + const total = 200; + assert.equal(loadedCountForTarget(60, total, 35, 30), 60); + assert.equal(loadedCountForTarget(30, total, 29, 30), 30); +}); + +test("loadedCountForTarget: uncovered target grows in whole batches strictly covering it (AC-L1-5)", () => { + const total = 200; + // Target row 30 is NOT loaded by loadedCount=30 (rows 0..29) — one batch. + assert.equal(loadedCountForTarget(30, total, 30, 30), 60); + assert.equal(loadedCountForTarget(30, total, 35, 30), 60); + // Strictly covers: row 60 needs rows 0..60, so two batches. + assert.equal(loadedCountForTarget(30, total, 60, 30), 90); + assert.equal(loadedCountForTarget(30, total, 61, 30), 90); +}); + +test("loadedCountForTarget: target past total clamps; exhausted window unchanged (AC-L1-5)", () => { + assert.equal(loadedCountForTarget(30, 75, 500, 30), 75); + assert.equal(loadedCountForTarget(75, 75, 500, 30), 75); + assert.equal(loadedCountForTarget(200, 200, 10, 30), 200); +}); + +// T8 — AC-L2-1: the growth step is min(L + max(1, batchSize), R); the +// max(1, ·) guard is what keeps loadedCountForTarget's loop terminating on +// a degenerate (or negative) batch size. + +test("nextLoadedCount steps min(L + max(1, batchSize), R) including the degenerate-batch guard (AC-L2-1)", () => { + assert.equal(nextLoadedCount(30, 200, 30), 60); + assert.equal(nextLoadedCount(90, 200, 30), 120); + assert.equal(nextLoadedCount(180, 200, 30), 200, "clamped at total"); + assert.equal(nextLoadedCount(200, 200, 30), 200, "exhausted: no-op clamp"); + assert.equal(nextLoadedCount(30, 200, 0), 31, "degenerate batch adds 1"); + assert.equal(nextLoadedCount(30, 200, -5), 31, "negative batch adds 1"); +}); + +// --------------------------------------------------------------------------- +// Unit 2b — §B2 wiring pins (T9) + headerRow-only constraint (T10). +// +// Source-parse tests over extensions/history/index.ts. The body extractor +// mirrors dispatch.test.ts's methodBody(): slice from the method declaration +// to the first "\n }" — which is exactly why every nested if added by the +// §B2 wiring must close at 4-space indent (a 4-space closer cannot match the +// first-close slice, so the method close is still found). +// +// Slice-3 adaptation note: upstream wires the growth trigger INLINE in +// moveUp/moveDown (no shared growLoadedWindowIfNeeded helper — that shape is +// dev-repo drift). The pins below assert the same AC contracts against the +// inline form. + +function methodBodyOf(name: string): string { + const decl = selectorSource.indexOf(`private ${name}(`); + assert.ok(decl >= 0, `private ${name}() should exist in extensions/history/index.ts`); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, `private ${name}() body should close`); + return selectorSource.slice(decl, end); +} + +// T9 — AC-L1-4: batch append points — growth wiring in the three downward +// paths ONLY (moveUp carries the older-direction growth check); every other +// upward site and applyFilter stay pure. + +test("growth wiring appears in moveDown, moveUp, pageListDown, jumpToLast (AC-L1-4)", () => { + const down = methodBodyOf("moveDown"); + assert.ok( + down.includes("shouldGrowWindow("), + "moveDown must evaluate the C2 trigger", + ); + assert.ok( + down.includes("nextLoadedCount("), + "moveDown must grow via nextLoadedCount", + ); + const up = methodBodyOf("moveUp"); + assert.ok( + up.includes("shouldGrowWindow(") && up.includes("nextLoadedCount("), + "moveUp must carry the older-direction growth check", + ); + const pageDown = methodBodyOf("pageListDown"); + assert.ok( + pageDown.includes("loadedCountForTarget("), + "pageListDown must catch up via loadedCountForTarget", + ); + const jumpLast = methodBodyOf("jumpToLast"); + assert.ok( + jumpLast.includes("this.loadedCount = this.records.length;"), + "jumpToLast must one-shot the full load (End)", + ); + for (const name of ["pageListUp", "jumpToFirst", "applyFilter"]) { + const body = methodBodyOf(name); + for (const grow of [ + "shouldGrowWindow(", + "nextLoadedCount(", + "loadedCountForTarget(", + ]) { + assert.ok( + !body.includes(grow), + `${name} must never grow (found ${grow})`, + ); + } + } +}); + +// T9 — AC-L1-7 / AC-L1-5 / AC-L1-6 ordering: growth runs BEFORE the index +// computation in every downward path (grow-before-move, design §B2/§D1). + +test("growth runs BEFORE the index computation in every downward path (AC-L1-7, AC-L1-5, AC-L1-6)", () => { + const down = methodBodyOf("moveDown"); + const growAt = down.indexOf("shouldGrowWindow("); + assert.notEqual(growAt, -1, "moveDown must evaluate the C2 trigger"); + assert.ok( + growAt < down.indexOf("moveSelectedIndex("), + "moveDown must grow before the modulo — wrap-to-0 only on the exhausted set", + ); + const page = methodBodyOf("pageListDown"); + const catchUpAt = page.indexOf("loadedCountForTarget("); + assert.notEqual(catchUpAt, -1, "pageListDown must run the PgDn catch-up"); + assert.ok( + catchUpAt < page.indexOf("pageSelectedIndex("), + "pageListDown must load the paged-to row before the selection lands", + ); + const last = methodBodyOf("jumpToLast"); + const fullLoad = last.indexOf("this.loadedCount = this.records.length;"); + const guard = last.indexOf("if (this.filteredRecords.length === 0) return;"); + assert.ok( + fullLoad !== -1 && guard !== -1 && fullLoad < guard, + "jumpToLast must full-load before the empty guard so End surfaces unloaded matches", + ); +}); + +// T9 — AC-L2-2 (§8a regression pin, wiring half): the trigger/growth call +// arguments read ONLY the unfiltered counts — `filteredRecords` appears in +// no growth region of any downward body. + +test("growth arithmetic names only this.loadedCount and this.records.length (AC-L2-2)", () => { + const down = methodBodyOf("moveDown"); + const downGrow = down.slice(0, down.indexOf("moveSelectedIndex(")); + assert.ok( + downGrow.includes("shouldGrowWindow(") && + downGrow.includes("nextLoadedCount("), + "moveDown's growth region must run the trigger + one batch before the modulo", + ); + assert.ok( + !downGrow.includes("filteredRecords"), + "moveDown's pre-modulo region must read UNFILTERED counts only", + ); + const page = methodBodyOf("pageListDown"); + const pageGrow = page.slice(0, page.indexOf("pageSelectedIndex(")); + assert.ok( + pageGrow.includes("loadedCountForTarget(") && + pageGrow.includes("this.loadedCount") && + pageGrow.includes("this.records.length"), + "pageListDown's catch-up must pass the unfiltered counts", + ); + assert.ok( + !pageGrow.includes("filteredRecords"), + "pageListDown's growth arithmetic must read UNFILTERED counts only", + ); + const last = methodBodyOf("jumpToLast"); + const guardAt = last.indexOf( + "if (this.filteredRecords.length === 0) return;", + ); + const lastGrow = last.slice(0, guardAt); + assert.ok( + lastGrow.includes("this.loadedCount = this.records.length;") && + !lastGrow.includes("filteredRecords"), + "jumpToLast's full-load region must be unfiltered-only", + ); +}); + +// T9 — AC-L2-3 (typing never loads) + AC-L1-2: applyFilter derives matches +// from the loaded prefix and contains no grow call. + +test("applyFilter windows the loaded prefix and grows only via loadedCountForQuery (AC-L2-3r, AC-L1-2)", () => { + const body = methodBodyOf("applyFilter"); + assert.ok( + body.includes("filterPrompts(") && + body.includes(".slice(0, this.loadedCount)"), + "applyFilter must derive matches from records.slice(0, loadedCount)", + ); + assert.ok( + body.includes("loadedCountForQuery("), + "applyFilter must route visibility through loadedCountForQuery (AC-L2-3r)", + ); + assert.ok( + !body.includes("nextLoadedCount("), + "typing implies one-shot full visibility via loadedCountForQuery; incremental loads stay banned (C2)", + ); + assert.ok( + !body.includes("shouldGrowWindow("), + "the filter path must never trigger growth", + ); +}); + +// T10 — AC-L3-2: headerRow-only constraint — the suffix is produced inside +// rebuildListWithWidth's existing headerRow.setText argument, adds no row +// (no new addChild in the method, constructor child sequence unchanged) and +// OVERLAY_LINES = 30 stays intact. + +test("the header keeps the position segment plus the loaded suffix on the existing headerRow.setText path (AC-L3-2)", () => { + const body = methodBodyOf("rebuildListWithWidth"); + const setTextAt = body.indexOf("headerRow.setText("); + assert.ok( + setTextAt >= 0, + "the suffix must extend the existing headerRow.setText call", + ); + const setTextRegion = body.slice( + setTextAt, + body.indexOf("this.listContainer.clear()"), + ); + assert.ok( + setTextRegion.includes("loaded ") && + setTextRegion.includes("this.loadedCount") && + setTextRegion.includes("this.records.length"), + "the ` · loaded M of T ` suffix must be produced inside the setText argument", + ); + assert.ok( + !setTextRegion.includes("indexing "), + "no indexing segment — removed by user decision", + ); + const addChildCount = body.split("addChild(").length - 1; + assert.equal( + addChildCount, + 4, + "the suffix adds no addChild call — today's 4 list-row sites unchanged", + ); + assert.ok( + selectorSource.includes("private static readonly OVERLAY_LINES = 30;"), + "OVERLAY_LINES = 30 must stay intact", + ); + const classAt = selectorSource.indexOf("class PromptHistorySelector"); + const ctorAt = selectorSource.indexOf("constructor(", classAt); + const ctorEnd = selectorSource.indexOf('this.applyFilter("")', ctorAt); + const ctorAddChild = + selectorSource.slice(ctorAt, ctorEnd).split("this.addChild(").length - 1; + assert.equal(ctorAddChild, 12, "the constructor child sequence is unchanged"); +}); + +// T14 — AC-L2-3 revision (user-directed 2026-09-08): a non-empty query +// implies full-snapshot visibility, one-shot and idempotent; an empty or +// whitespace-only query leaves the window untouched. Per-keypress +// incremental growth remains banned (the helper returns total, never +BATCH). + +test("loadedCountForQuery: non-empty query returns total, empty keeps window (AC-L2-3r)", () => { + assert.equal(loadedCountForQuery(10, 512, "deploy"), 512); + assert.equal(loadedCountForQuery(10, 512, ""), 10); + assert.equal(loadedCountForQuery(10, 512, " "), 10); + assert.equal(loadedCountForQuery(512, 512, "deploy"), 512); + assert.equal(loadedCountForQuery(10, 10, "x"), 10); +}); diff --git a/tests/history-openflow-integration.test.ts b/tests/history-openflow-integration.test.ts new file mode 100644 index 000000000..1737f887a --- /dev/null +++ b/tests/history-openflow-integration.test.ts @@ -0,0 +1,143 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; + +/** + * WU5 tests (AC-S6-1..3): the open-flow wiring in extensions/history/index.ts. + * NEVER import it — it pulls the pi-tui runtime graph (design §D3). The + * wiring is pinned by source-parse (command-registration pattern); loader + * behavior uses fs-only fixtures under the OS temp dir — NEVER the user's + * real ~/.pi/agent/history. + */ + +const indexSource = fs.readFileSync( + fileURLToPath(new URL("../extensions/history/index.ts", import.meta.url)), + "utf8", +); + +function openHistorySelectorBody(): string { + const start = indexSource.indexOf("async function openHistorySelector("); + assert.ok(start >= 0, "openHistorySelector should exist"); + const end = indexSource.indexOf("export default function", start); + assert.ok(end > start, "extension entry point should follow"); + return indexSource.slice(start, end); +} + +/** Method body slice (lazy-windowing.test.ts pattern; first "\n }" close). */ +function methodBodyOf(name: string): string { + const decl = indexSource.indexOf(`private ${name}(`); + assert.ok(decl >= 0, `private ${name}() should exist in extensions/history/index.ts`); + const end = indexSource.indexOf("\n }", decl); + assert.ok(end > decl, `private ${name}() body should close`); + return indexSource.slice(decl, end); +} + +// --------------------------------------------------------------------------- +// T31 — AC-S6-1: store-only drain wiring (source-parse, §I load-bearing shape). +// --------------------------------------------------------------------------- + +test("T31 (AC-S6-1): the store drain is the entries source — no live transcript merge (§I pin 1)", () => { + const body = openHistorySelectorBody(); + const drainIdx = body.indexOf('const entries = drainForScope("project")'); + assert.ok( + drainIdx >= 0, + "the load step must drain the store directly (wiring RED seam)", + ); + assert.ok( + !body.includes("mergeHistoryEntries("), + "the live transcript merge is GONE from the open flow (user-directed store-only scopes)", + ); + assert.ok( + body.indexOf("if (entries.length === 0)") >= 0, + "the PR-branch empty guard stands: no history warns instead of opening an empty overlay", + ); +}); + +test("T31 (AC-S6-1): records are built via recordsFromEntries over the drained entries (§I pins 2+3)", () => { + const body = openHistorySelectorBody(); + const recIdx = body.indexOf("recordsFromEntries(entries)"); + assert.ok( + recIdx >= 0, + "records build through the shared recordsFromEntries helper", + ); +}); + +test("T31 (AC-S6-1): the three command-registration pins hold beside the swap", () => { + const definitions = + indexSource.split("async function openHistorySelector(").length - 1; + assert.equal(definitions, 1, "openHistorySelector defined exactly once"); + const calls = indexSource.split("openHistorySelector(ctx)").length - 1; + assert.equal( + calls, + 2, + "exactly the two entry-point call sites — the swap adds no occurrence", + ); +}); + +// --------------------------------------------------------------------------- +// T32 — AC-S6-2: cold-start wiring (no-await source-parse). +// --------------------------------------------------------------------------- + +test("T32 (AC-S6-2): NO await on any records build inside openHistorySelector (source-parse)", () => { + const body = openHistorySelectorBody(); + assert.ok( + body.includes('const entries = drainForScope("project")'), + "wiring present (RED seam before GREEN)", + ); + assert.ok( + !body.includes("startBackgroundIndexBuild"), + "the build kick lives inside the loader — never in the selector", + ); + assert.ok( + !/await\s+mergeHistoryEntries/.test(body), + "the open path never awaits the loader (sync const declaration)", + ); +}); + +// --------------------------------------------------------------------------- +// T33 — AC-S6-3: merged header totals + third transient dim indexing segment. +// --------------------------------------------------------------------------- + +test("T33 (AC-S6-3): header totals derive from filteredRecords — derivation untouched", () => { + const body = methodBodyOf("rebuildListWithWidth"); + assert.ok( + body.includes("const count = this.filteredRecords.length;"), + "N derives from filteredRecords (merged by construction)", + ); +}); + +test("T33 (AC-S6-3): loaded segment present, indexing segment removed", () => { + const body = methodBodyOf("rebuildListWithWidth"); + const setTextAt = body.indexOf("headerRow.setText("); + assert.ok(setTextAt >= 0, "the header must keep the existing setText call"); + const setTextRegion = body.slice( + setTextAt, + body.indexOf("this.listContainer.clear()"), + ); + assert.ok( + setTextRegion.includes("loaded ") && + setTextRegion.includes("this.loadedCount"), + "the loaded segment stays (user-restored)", + ); + assert.ok( + !setTextRegion.includes("indexing "), + "the indexing segment stays removed", + ); +}); + +test("T33 (AC-S6-3): Change 2 structural pins still hold beside the third segment", () => { + assert.ok( + indexSource.includes("private static readonly OVERLAY_LINES = 30;"), + "OVERLAY_LINES = 30 intact", + ); + const body = methodBodyOf("rebuildListWithWidth"); + const addChildCount = body.split("addChild(").length - 1; + assert.equal(addChildCount, 4, "no new addChild in rebuildListWithWidth"); + const classAt = indexSource.indexOf("class PromptHistorySelector"); + const ctorAt = indexSource.indexOf("constructor(", classAt); + const ctorEnd = indexSource.indexOf('this.applyFilter("")', ctorAt); + const ctorAddChild = + indexSource.slice(ctorAt, ctorEnd).split("this.addChild(").length - 1; + assert.equal(ctorAddChild, 12, "the constructor child sequence is unchanged"); +}); diff --git a/tests/history-preview-layout.test.ts b/tests/history-preview-layout.test.ts new file mode 100644 index 000000000..782da55ce --- /dev/null +++ b/tests/history-preview-layout.test.ts @@ -0,0 +1,56 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; + +const sourcePath = fileURLToPath( + new URL("../extensions/history/index.ts", import.meta.url), +); +const source = fs.readFileSync(sourcePath, "utf8"); + +test("preview rows are bottom-padded so the panel shrinks from the bottom", () => { + const rebuildStart = source.indexOf( + "private rebuildPreviewWithWidth(width: number): void {", + ); + assert.notStrictEqual( + rebuildStart, + -1, + "rebuildPreviewWithWidth() should exist", + ); + + const rebuildEnd = source.indexOf( + "\n // -- Selection actions", + rebuildStart, + ); + assert.notStrictEqual( + rebuildEnd, + -1, + "rebuildPreview section boundary should exist", + ); + + const rebuildPreviewSource = source.slice(rebuildStart, rebuildEnd); + + const rowLoopIndex = rebuildPreviewSource.indexOf( + "for (let i = 0; i < PREVIEW_ROWS; i++)", + ); + assert.ok( + rowLoopIndex >= 0, + "fixed-height PREVIEW_ROWS row loop should exist", + ); + + const emptyRowPadIndex = rebuildPreviewSource.indexOf( + "this.previewContainer.addChild(new FixedRowText());", + rowLoopIndex, + ); + assert.ok( + emptyRowPadIndex >= 0, + "rows past the wrapped content should be added as empty bottom padding", + ); + + assert.ok( + !rebuildPreviewSource.includes( + "const topPadding = PREVIEW_ROWS - visible.length;", + ), + "preview should not compute top padding", + ); +}); diff --git a/tests/history-selector-windowing.test.ts b/tests/history-selector-windowing.test.ts new file mode 100644 index 000000000..9fca7e9d7 --- /dev/null +++ b/tests/history-selector-windowing.test.ts @@ -0,0 +1,94 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + buildPromptRecords, + clampPreviewOffset, + clampSelectedIndex, + computeVisibleRange, + getVisiblePromptRecords, + moveSelectedIndex, + pageSelectedIndex, +} from "../extensions/history/selector-helpers.ts"; + +test("buildPromptRecords lowercases search text", () => { + assert.deepEqual(buildPromptRecords(["Hello"]), [ + { text: "Hello", searchText: "hello" }, + ]); +}); + +test("computeVisibleRange centers when possible", () => { + assert.deepEqual(computeVisibleRange(8, 30, 10), { start: 3, end: 13 }); +}); + +test("computeVisibleRange pins near end", () => { + assert.deepEqual(computeVisibleRange(28, 30, 10), { start: 20, end: 30 }); +}); + +test("computeVisibleRange handles small lists", () => { + assert.deepEqual(computeVisibleRange(1, 3, 10), { start: 0, end: 3 }); +}); + +test("clampSelectedIndex stays within filtered record bounds", () => { + assert.equal(clampSelectedIndex(8, 3), 2); + assert.equal(clampSelectedIndex(1, 0), 0); +}); + +test("clampPreviewOffset clamps offsets past the last page", () => { + assert.equal(clampPreviewOffset(50, 47, 10), 37); + assert.equal(clampPreviewOffset(5, 47, 10), 5); +}); + +test("clampPreviewOffset pins to zero when content fits the viewport", () => { + assert.equal(clampPreviewOffset(3, 8, 10), 0); + assert.equal(clampPreviewOffset(7, 0, 10), 0); +}); + +test("moveSelectedIndex wraps around the list", () => { + assert.equal(moveSelectedIndex(0, 3, -1), 2); + assert.equal(moveSelectedIndex(2, 3, 1), 0); + assert.equal(moveSelectedIndex(0, 0, 1), 0); +}); + +test("pageSelectedIndex clamps within the list", () => { + assert.equal(pageSelectedIndex(8, 30, -10), 0); + assert.equal(pageSelectedIndex(2, 3, 10), 2); + assert.equal(pageSelectedIndex(0, 0, 10), 0); +}); + +test("pageSelectedIndex end-clamps a downward page at the last entry (AC-P2-1.1)", () => { + assert.equal(pageSelectedIndex(28, 30, 10), 29); + assert.equal(pageSelectedIndex(25, 30, 10), 29); +}); + +test("pageSelectedIndex clamps |pageSize| greater than total in both directions (AC-P2-1.2)", () => { + assert.equal(pageSelectedIndex(0, 3, 10), 2); + assert.equal(pageSelectedIndex(2, 3, -10), 0); + assert.equal(pageSelectedIndex(0, 30, -50), 0); +}); + +test("pageSelectedIndex never wraps past the ends (AC-P2-1.2)", () => { + assert.equal(pageSelectedIndex(29, 30, 10), 29); + assert.equal(pageSelectedIndex(0, 30, -10), 0); +}); + +test("getVisiblePromptRecords returns visible records with selection state", () => { + assert.deepEqual( + getVisiblePromptRecords( + buildPromptRecords(["one", "two", "three", "four"]), + 2, + 2, + ), + [ + { + index: 1, + record: { text: "two", searchText: "two" }, + isSelected: false, + }, + { + index: 2, + record: { text: "three", searchText: "three" }, + isSelected: true, + }, + ], + ); +}); diff --git a/tests/history-session-writer.test.ts b/tests/history-session-writer.test.ts index 8601cea14..61a2689b3 100644 --- a/tests/history-session-writer.test.ts +++ b/tests/history-session-writer.test.ts @@ -88,22 +88,36 @@ test("two writers own separate files in the same project dir", () => { assert.deepEqual(files, ["inst-a.jsonl", "inst-b.jsonl"]); }); -test("the slice-1 extension entry registers only the capture handler", () => { +test("the extension entry registers exactly the slice-3 wiring surface", () => { // Module load must stay side-effect free (importing index.ts parses the - // whole slice-1 graph without touching the real ~/.pi store root), and - // slice 1 wires exactly one handler: before_agent_start. + // whole graph without touching the real ~/.pi store root). Wiring as of + // slice 3: before_agent_start capture + tool_call overlay dismiss, the + // ctrl+shift+r shortcut, and the history command. session_shutdown is + // slice 6 and must not appear yet. const registered: Array<[string, unknown]> = []; + const shortcuts: Array<[string, unknown]> = []; + const commands: Array<[string, unknown]> = []; const pi = { on: (event: string, handler: unknown) => { registered.push([event, handler]); }, + registerShortcut: (key: string, def: unknown) => { + shortcuts.push([key, def]); + }, + registerCommand: (name: string, def: unknown) => { + commands.push([name, def]); + }, }; promptHistoryExtension(pi as never); assert.deepEqual( registered.map(([event]) => event), - ["before_agent_start"], + ["before_agent_start", "tool_call"], ); - // The handler is callable but is NEVER invoked here: a real invocation + assert.deepEqual(shortcuts.map(([key]) => key), ["ctrl+shift+r"]); + assert.deepEqual(commands.map(([name]) => name), ["history"]); + // Handlers are callable but are NEVER invoked here: a real invocation // would run getWriter() against the user's real ~/.pi/agent/history. - assert.equal(typeof registered[0][1], "function"); + for (const [, handler] of registered) { + assert.equal(typeof handler, "function"); + } }); diff --git a/tests/history-wheel-mouse.test.ts b/tests/history-wheel-mouse.test.ts new file mode 100644 index 000000000..37ddcd6b6 --- /dev/null +++ b/tests/history-wheel-mouse.test.ts @@ -0,0 +1,242 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; + +// Unit 4 — L6 wheel slice (spec C5, design §D6). +// +// Source-parse structural pins on extensions/history/index.ts (no pi-tui +// runtime graph — the same discipline as the other source-parse suites). +// The overlay renders only through pi-tui, so the unit-level contract is the +// SHAPE of the handleMouse override: +// +// - wheel-only: every non-wheel event type returns undefined (press/click/ +// drag stay host-owned) and the dispatch table gains no extra entry (wheel +// is not a keybinding — dispatch.test.ts remains the authoritative +// untouched pin); +// - ONE consumed wheel return: `handled: true` plus the synthetic target +// enrichment, reached by every wheel path including the no-op regions — +// this closes the pre-existing fullscreen SGR-fallthrough hazard by +// construction; +// - fixed 30-row geometry routing: list region y 5–14, preview region y 17–26, +// all other rows consumed no-ops; +// - list wheel: sign × |wheelDelta| steps through moveDown (the arrow grow +// path applies per step) / moveUp, magnitude clamped to the filtered list, +// zero/absent delta a no-op move — the override itself never re-implements +// growth; +// - preview wheel: 1 line per notch toward the delta direction via the +// existing clampPreviewOffset semantics + rebuildPreview. + +const selectorSource = fs.readFileSync( + fileURLToPath(new URL("../extensions/history/index.ts", import.meta.url)), + "utf8", +); + +// T13 — AC-L6-1: wheel-only override + no extra dispatch entry. + +test("handleMouse override is wheel-only and the dispatch table keeps 11 entries (AC-L6-1)", () => { + const decl = selectorSource.indexOf("override handleMouse("); + assert.ok(decl >= 0, "PromptHistorySelector should override handleMouse"); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, "handleMouse's body should close"); + const body = selectorSource.slice(decl, end); + + assert.ok( + body.includes('if (event.type !== "wheel") return undefined;'), + "non-wheel event types must return undefined (press/click/drag stay host-owned)", + ); + assert.ok( + body.includes('ReturnType'), + 'the return type must name the base contract via ReturnType', + ); + + const tableAt = selectorSource.indexOf( + "private readonly dispatch: readonly DispatchEntry[] = [", + ); + assert.ok(tableAt >= 0, "the dispatch table should exist"); + const tableEnd = selectorSource.indexOf("\n ];", tableAt); + assert.ok(tableEnd > tableAt, "the dispatch table should close"); + const table = selectorSource.slice(tableAt, tableEnd); + const entries = table.split("match:").length - 1; + assert.equal( + entries, + 11, + "wheel is not a keybinding: exactly the 11 §B2 dispatch entries, no extra", + ); +}); + +// T13 — AC-L6-2: ONE consumed wheel return with the target enrichment, +// reached by every wheel path including the no-op regions. + +test("every wheel path reaches the single handled:true return with target enrichment (AC-L6-2)", () => { + const decl = selectorSource.indexOf("override handleMouse("); + assert.ok(decl >= 0, "handleMouse should exist"); + const end = selectorSource.indexOf("\n }", decl); + const body = selectorSource.slice(decl, end); + + const returns = body.split("return").length - 1; + assert.equal( + returns, + 2, + "exactly two returns: the guard's undefined and the ONE consumed wheel return", + ); + assert.equal( + body.split("handled: true").length - 1, + 1, + "exactly one handled:true — the single wheel return", + ); + assert.equal( + body.split("return {").length - 1, + 1, + "exactly one object return, so list, preview, and no-op regions all reach it", + ); + + // Synthetic target mirroring dispatchMouseEvent's enrichment math + // (pi-tui tui.js dispatchMouseEvent: originX = screenX - x, originY = + // screenY - y, bounds from the event) — the result carries `target`, so + // dispatch passes it through verbatim. + assert.ok(body.includes("component: this,"), "target.component: this"); + assert.ok( + body.includes("originX: event.screenX - event.x,"), + "target.originX mirrors the dispatch enrichment math", + ); + assert.ok( + body.includes("originY: event.screenY - event.y,"), + "target.originY mirrors the dispatch enrichment math", + ); + assert.ok(body.includes("width: event.width,"), "target bounds width"); + assert.ok(body.includes("height: event.height,"), "target bounds height"); + + // No manual render: pi-tui re-renders handled wheels by default. + assert.ok( + !body.includes("requestRender"), + "handleMouse must not call requestRender (wheel results render by default)", + ); +}); + +// T13 — AC-L6-3: region routing truth table — the fixed 30-row geometry's +// list band 5–14 and preview band 17–26 appear as the y comparisons, all +// other rows fall through to the consumed no-op return. + +test("region constants 5-14 / 17-26 route the y comparisons (AC-L6-3)", () => { + assert.ok( + selectorSource.includes("const LIST_WHEEL_Y_FIRST = 5;"), + "LIST_WHEEL_Y_FIRST = 5 (list container rows)", + ); + assert.ok( + selectorSource.includes("const LIST_WHEEL_Y_LAST = 14;"), + "LIST_WHEEL_Y_LAST = 14", + ); + assert.ok( + selectorSource.includes("const PREVIEW_WHEEL_Y_FIRST = 17;"), + "PREVIEW_WHEEL_Y_FIRST = 17 (preview container rows)", + ); + assert.ok( + selectorSource.includes("const PREVIEW_WHEEL_Y_LAST = 26;"), + "PREVIEW_WHEEL_Y_LAST = 26", + ); + + const decl = selectorSource.indexOf("override handleMouse("); + assert.ok(decl >= 0, "handleMouse should exist"); + const end = selectorSource.indexOf("\n }", decl); + const body = selectorSource.slice(decl, end); + + assert.ok( + body.includes("event.y >= LIST_WHEEL_Y_FIRST") && + body.includes("event.y <= LIST_WHEEL_Y_LAST"), + "the list branch must compare y against the list band", + ); + assert.ok( + body.includes("event.y >= PREVIEW_WHEEL_Y_FIRST") && + body.includes("event.y <= PREVIEW_WHEEL_Y_LAST"), + "the preview branch must compare y against the preview band", + ); +}); + +// T13 — AC-L6-4: list wheel semantics — sign picks the direction, magnitude +// is clamped to the filtered list, zero/absent delta is a no-op, and the +// routing goes THROUGH moveDown's own grow path (never a re-implementation). + +test("list wheel routes sign-clamped steps through moveDown/moveUp (AC-L6-4)", () => { + const decl = selectorSource.indexOf("override handleMouse("); + assert.ok(decl >= 0, "handleMouse should exist"); + const end = selectorSource.indexOf("\n }", decl); + const body = selectorSource.slice(decl, end); + + // Absent wheelDelta normalizes to 0 → zero steps → no-op move. + assert.ok( + body.includes("const delta = event.wheelDelta ?? 0;"), + "delta must default an absent wheelDelta to 0", + ); + + const listStart = body.indexOf("if (event.y >= LIST_WHEEL_Y_FIRST"); + const listEnd = body.indexOf("} else if (", listStart); + assert.ok( + listStart >= 0 && listEnd > listStart, + "the list branch should exist", + ); + const listBranch = body.slice(listStart, listEnd); + + assert.ok( + listBranch.includes( + "const steps = Math.min(Math.abs(delta), this.filteredRecords.length);", + ), + "magnitude must clamp to the filtered list length", + ); + assert.ok( + listBranch.includes("for (let i = 0; i < steps; i++) {"), + "steps must move one row at a time (0 for a zero delta — the no-op)", + ); + assert.ok( + listBranch.includes("if (delta > 0) this.moveDown();") && + listBranch.includes("else this.moveUp();"), + "sign semantics: positive delta moves down through moveDown, else up", + ); + + // Prefetch interplay intact: growth belongs to moveDown itself — the + // override must not re-implement the trigger. + assert.ok( + !body.includes("shouldGrowWindow") && !body.includes("nextLoadedCount"), + "the override must not re-implement growth (wheel-down grows via moveDown)", + ); +}); + +// T13 — AC-L6-5: preview wheel semantics — one line per notch toward the +// delta direction through the existing clamp, zero-delta no-op. + +test("preview wheel scrolls one clamped line per notch (AC-L6-5)", () => { + const decl = selectorSource.indexOf("override handleMouse("); + assert.ok(decl >= 0, "handleMouse should exist"); + const end = selectorSource.indexOf("\n }", decl); + const body = selectorSource.slice(decl, end); + + const previewStart = body.indexOf("} else if ("); + const previewEnd = body.indexOf("return {", previewStart); + assert.ok( + previewStart >= 0 && previewEnd > previewStart, + "the preview branch should exist", + ); + const previewBranch = body.slice(previewStart, previewEnd); + + assert.ok( + previewBranch.includes("if (delta !== 0) {"), + "a zero delta must be a no-op in the preview band too", + ); + assert.ok( + previewBranch.includes("this.previewScrollOffset = clampPreviewOffset("), + "the preview must scroll through the existing clamp semantics", + ); + assert.ok( + previewBranch.includes("this.previewScrollOffset + (delta > 0 ? 1 : -1)"), + "exactly one line per notch toward the delta direction", + ); + assert.ok( + previewBranch.includes("this.wrappedPreviewLines.length") && + previewBranch.includes("PREVIEW_ROWS"), + "the clamp must run against the wrapped length and the viewport rows", + ); + assert.ok( + previewBranch.includes("this.rebuildPreview()"), + "the preview must re-render after the offset change", + ); +}); From d354c8a450b0a4cab042d0f7a24de2f5421b46d6 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:41:43 -0300 Subject: [PATCH 06/14] fix(history): intact astral characters, correct row padding, honest comments Review fixes (Copilot + CodeRabbit on PR #819): - sanitizeForDisplay: astral code points (> 0xFFFF) are re-appended via String.fromCodePoint instead of only the high surrogate at text[i]; emoji and other non-BMP characters no longer lose half their code point in list rows and previews. The low-surrogate skip is retained. - FixedRowText.render: the full-width pad now measures the VISIBLE width (SGR escape sequences stripped), matching the centered branch's measurement; colored list rows previously padded short and could leave ghost characters on overlay dismiss. - Lazy-windowing comment corrected: PRELOAD_BUFFER is 3 (fired in the final 3 loaded rows), not 2 as the stale comment claimed. - Regression pins added for both behavior fixes. --- extensions/history/index.ts | 14 ++++++++--- tests/history-preview-layout.test.ts | 37 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/extensions/history/index.ts b/extensions/history/index.ts index 9fe2d7893..f9559b9d6 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -57,8 +57,8 @@ import { const SHORTCUT = "ctrl+shift+r"; const MAX_VISIBLE = 10; const PREVIEW_ROWS = 10; -// Lazy windowing (design §D3; user-tuned 2026-09-08). PRELOAD_BUFFER=2 -// fires growth as the cursor enters the final 2 loaded rows; BATCH_SIZE=10 +// Lazy windowing (design §D3; user-tuned 2026-09-08). PRELOAD_BUFFER=3 +// fires growth as the cursor enters the final 3 loaded rows; BATCH_SIZE=10 // loads exactly one viewport per growth; INITIAL_BATCH=10 paints one // viewport at open. PRELOAD_BUFFER <= MAX_VISIBLE keeps a jump within one // viewport covered by the catch-up loop; review all three together. @@ -117,7 +117,10 @@ function sanitizeForDisplay(text: string): string { } else if (cp >= 0x80 && cp < 0xa0) { out += "\\x" + cp.toString(16).padStart(2, "0"); } else { - out += text[i]; + // Astral code points (> 0xFFFF) span a surrogate pair; append the + // full code point, not just the high surrogate at text[i], so emoji + // and other non-BMP characters survive sanitization intact. + out += cp > 0xffff ? String.fromCodePoint(cp) : text[i]; } if (cp > 0xffff) i++; // skip low surrogate of astral pair } @@ -180,7 +183,10 @@ class FixedRowText { : truncateToWidth(this.text, width, "…"); // Pad to full terminal width so the overlay fully overwrites // whatever is beneath it and leaves no ghost characters on dismiss. - return [rendered + " ".repeat(Math.max(0, width - rendered.length))]; + // Measure the VISIBLE width: SGR escape sequences (colored rows from + // rebuildListWithWidth) occupy no terminal cells. + const visible = rendered.replace(/\x1b\[[0-9;]*m/g, ""); + return [rendered + " ".repeat(Math.max(0, width - visible.length))]; } } diff --git a/tests/history-preview-layout.test.ts b/tests/history-preview-layout.test.ts index 782da55ce..02509725c 100644 --- a/tests/history-preview-layout.test.ts +++ b/tests/history-preview-layout.test.ts @@ -54,3 +54,40 @@ test("preview rows are bottom-padded so the panel shrinks from the bottom", () = "preview should not compute top padding", ); }); + +test("row padding measures visible width, stripping SGR escapes", () => { + // Colored list rows carry SGR escape sequences that occupy no terminal + // cells; padding must use the VISIBLE width or the row falls short of + // the overlay width and leaves ghost characters on dismiss. + const renderStart = source.indexOf(" render(width: number): string[] {"); + assert.notStrictEqual(renderStart, -1, "FixedRowText.render should exist"); + + const renderSource = source.slice(renderStart, renderStart + 2200); + const padLine = renderSource + .split("\n") + .find((l) => l.includes('" ".repeat(Math.max(0, width -')); + assert.ok(padLine !== undefined, "final full-width pad should exist"); + assert.ok( + padLine.includes("visible"), + "pad must measure the SGR-stripped visible width, not rendered.length", + ); + assert.ok( + /visible = rendered\.replace\(/.test(renderSource), + "visible width must be derived by stripping escape sequences", + ); +}); + +test("sanitizeForDisplay appends the full astral code point, not a lone surrogate", () => { + const fnStart = source.indexOf("function sanitizeForDisplay("); + assert.notStrictEqual(fnStart, -1, "sanitizeForDisplay should exist"); + + const fnSource = source.slice(fnStart, fnStart + 1200); + assert.ok( + fnSource.includes("String.fromCodePoint(cp)"), + "astral code points must be re-appended whole (emoji survive)", + ); + assert.ok( + fnSource.includes("if (cp > 0xffff) i++"), + "the low surrogate of the pair must still be skipped", + ); +}); From a22588fcf81747c4cf63b23739a979dbf04f8e4f Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Wed, 23 Sep 2026 21:15:26 -0300 Subject: [PATCH 07/14] fix(history): satisfy upstream typecheck gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extensions/history/index.ts imported `ShortcutContext`, a type that only exists in the dev repo's @types shim — the real @earendil-works/pi-coding-agent exports `ExtensionCommandContext`, so the type gate added to main reports TS2305 on this branch's CI merge. Import `ExtensionCommandContext` and narrow both handler contexts to `Pick` (the only member they use), mirroring the fix already carried on the slice-6 branch. --- extensions/history/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/history/index.ts b/extensions/history/index.ts index f9559b9d6..d82b413d9 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -11,7 +11,7 @@ import { homedir } from "node:os"; import { DynamicBorder, type ExtensionAPI, - type ShortcutContext, + type ExtensionCommandContext, type Theme, } from "@earendil-works/pi-coding-agent"; import { @@ -815,7 +815,7 @@ function createPromptHistorySelectorFactory( } async function runPromptHistorySelection( - ctx: ShortcutContext, + ctx: Pick, records: PromptRecord[], ): Promise { const historyGlobals: PiHistoryGlobals = globalThis as Record< @@ -876,7 +876,7 @@ function drainForScope(scope: HistoryScope): string[] { } async function openHistorySelector( - ctx: Pick, + ctx: Pick, ): Promise { // Store-only drain (user-directed): both scopes read the store files // symmetrically — no live transcript merge (the one-time seed bootstrap From 84c1232361ca221e4b77b7aa8d2d0ebb6ddf3097 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:47:37 -0300 Subject: [PATCH 08/14] fix(history): make prompt capture opt-in and document the store Review follow-up on the slice-01 PR: the before_agent_start handler recorded delivered prompts by default while the deletion UI is still unshipped, so an intermediate release could accumulate sensitive prompts with no removal path. - Capture is now strictly opt-in via GENTLE_PI_HISTORY_CAPTURE=1|true|on (default off); the switch doubles as the disable path, is checked per prompt, and a disabled session writes nothing - no registry entry, no files. - promptHistoryExtension takes injectable deps (env/root/cwd/ instanceId/now) with one writer closure per extension load. - New tests: strict opt-in matrix, default-off inertness, opted-in capture, disable-leaves-existing-files. - docs/prompt-history.md documents the switch, storage locations, permissions/readers, and disable/removal semantics; the README docs table gains a pointer. --- README.md | 1 + docs/prompt-history.md | 61 ++++++++++++++++++++++ extensions/history/index.ts | 75 +++++++++++++++++++--------- tests/history-session-writer.test.ts | 61 +++++++++++++++++++++- 4 files changed, 174 insertions(+), 24 deletions(-) create mode 100644 docs/prompt-history.md diff --git a/README.md b/README.md index 9e20d72f4..0207c389c 100644 --- a/README.md +++ b/README.md @@ -880,6 +880,7 @@ To opt out: | `docs/skill-style-guide.md` | Normative style guide used by the packaged skill creation/improvement skills. | | `docs/native-authority-architecture.md` | Post-U8 ownership boundary, reproducible slimming metrics, Windows evidence, exact #191 seam, and the `review-integration/v1`→`v2` migration status, including the "compact-v2" naming disambiguation. | | `docs/review-integration.md` | Negotiated provider/consumer contract and the current Gentle Pi adoption boundary. | +| `docs/prompt-history.md` | Prompt-history slice 1: opt-in capture switch, storage layout, readers, and disable/removal semantics. | ## Development diff --git a/docs/prompt-history.md b/docs/prompt-history.md new file mode 100644 index 000000000..1fa8eba4f --- /dev/null +++ b/docs/prompt-history.md @@ -0,0 +1,61 @@ +# Prompt history + +Slice 1 of the prompt-history extension (#819 split) ships the storage layer only: +a per-instance JSONL capture store, project identity, and the read/write +primitives later slices build on. The selector UI, deletion/scope drains, and GC +arrive in later slices of the chain. + +## Capture is opt-in + +Recording is **off by default**. Delivered prompts can contain secrets, and the +deletion UI is not shipped yet, so nothing is stored unless you explicitly opt in: + +```bash +GENTLE_PI_HISTORY_CAPTURE=1 pi +``` + +- Enabled by `1`, `true`, or `on` (case-insensitive). Unset, empty, or any other + value means **off** — the same switch is the disable path. +- The check runs per prompt: unsetting the switch (or setting it to `0`) stops + new captures immediately, no pi restart needed. +- With capture off the extension is inert: no registry entry, no files, and + prompts are never written. + +## Where the files live + +Everything sits under `~/.pi/agent/history/`: + +- `registry.json` — advisory map of project hash → cwd, used for display + labels. +- `projects//.jsonl` — one append-only capture file per pi + process. + +`` is the first 16 hex chars of the SHA-256 of the canonicalized project +cwd; `` is a per-process UUID. Each line is one delivered prompt: + +```json +{"v":1,"text":"the prompt as delivered","ts":1700000000000} +``` + +UI command-like prompts (`/name ...`) and empty lines are never stored. Later +slices add the rebuildable `seed.jsonl`, scope drains/deletes, and GC. + +## Who can read them + +The store is plain JSONL on your local disk, not encrypted. Files are created by +the pi process with default umask permissions (typically `0644` files inside +`0755` directories), so any process running as your OS user can read them, and +other local accounts can too wherever they can traverse your home directory. +Treat the store as sensitive: it holds your prompts verbatim. + +## What disabling capture does + +Turning the switch off only stops **new** captures. Nothing is deleted: files +already written — and the registry entry — stay on disk until you remove them or +the deletion UI ships. To erase the store manually while capture is off (or pi +is not running): + +```bash +rm -rf ~/.pi/agent/history # whole store +rm -rf ~/.pi/agent/history/projects/ # one project (see registry.json) +``` diff --git a/extensions/history/index.ts b/extensions/history/index.ts index 26616733f..74dab5942 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -5,6 +5,12 @@ // per-instance writer lifecycle, and the before_agent_start capture // handler. Selector UI, shortcut/command, scope drains, legacy migration // and seed bootstrap, and GC arrive in later slices. +// +// Capture is OPT-IN while the deletion/privacy behavior is unshipped: +// nothing is recorded unless GENTLE_PI_HISTORY_CAPTURE=1|true|on. With the +// switch off the handler is a no-op — no registry entry, no files, and +// prompts are never written. Unsetting the switch only stops NEW captures; +// files already written stay on disk (docs/prompt-history.md). import { randomUUID } from "node:crypto"; import { homedir } from "node:os"; @@ -19,39 +25,62 @@ import { // v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). const PI_HISTORY_ROOT = join(homedir(), ".pi", "agent", "history"); -const AGENT_DIR = join(homedir(), ".pi", "agent"); -const CURRENT_CWD = process.cwd(); -// Instance identity: one exclusive capture file per pi process. -const INSTANCE_ID = randomUUID(); -let writerState: SessionWriterState | null = null; +export interface HistoryDeps { + env?: NodeJS.ProcessEnv; + root?: string; + cwd?: string; + instanceId?: string; + now?: () => number; +} /** - * One-time init per extension load: register the project in the advisory - * registry, then open this instance's exclusive capture file. Legacy - * migration and seed bootstrap join this init order in a later slice. + * Strict opt-in: capture stays off unless GENTLE_PI_HISTORY_CAPTURE is + * explicitly 1, true, or on (case-insensitive). The same switch is the + * disable path — unsetting it stops new captures; files already on disk + * are left untouched until the deletion tooling lands. */ -function getWriter(): SessionWriterState { - if (!writerState) { - try { - ensureRegistryEntry(PI_HISTORY_ROOT, CURRENT_CWD); - } catch { - // registry is advisory - } - writerState = openSessionWriter(PI_HISTORY_ROOT, CURRENT_CWD, INSTANCE_ID); - } - return writerState; +export function captureEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + const value = env.GENTLE_PI_HISTORY_CAPTURE?.trim().toLowerCase(); + return value === "1" || value === "true" || value === "on"; } -export default function promptHistoryExtension(pi: ExtensionAPI) { - // One writer per extension load; see getWriter() for the init order. +export default function promptHistoryExtension( + pi: ExtensionAPI, + deps: HistoryDeps = {}, +): void { + const env = deps.env ?? process.env; + const root = deps.root ?? PI_HISTORY_ROOT; + const cwd = deps.cwd ?? process.cwd(); + const instanceId = deps.instanceId ?? randomUUID(); + const now = deps.now ?? Date.now; + let writerState: SessionWriterState | null = null; + + /** + * One-time init per extension load: register the project in the advisory + * registry, then open this instance's exclusive capture file. Legacy + * migration and seed bootstrap join this init order in a later slice. + */ + const getWriter = (): SessionWriterState => { + if (!writerState) { + try { + ensureRegistryEntry(root, cwd); + } catch { + // registry is advisory + } + writerState = openSessionWriter(root, cwd, instanceId); + } + return writerState; + }; - // Persist every delivered user prompt (write-through, append-only JSONL). - // The local ExtensionAPI stub types handler args as unknown; narrow here. + // Persist every delivered user prompt (write-through, append-only JSONL), + // but only for opted-in sessions — see captureEnabled(). The local + // ExtensionAPI stub types handler args as unknown; narrow here. pi.on("before_agent_start", (...args: unknown[]) => { + if (!captureEnabled(env)) return; try { const event = args[0] as { prompt?: string } | undefined; - appendSessionCapture(getWriter(), event?.prompt ?? "", Date.now()); + appendSessionCapture(getWriter(), event?.prompt ?? "", now()); } catch { // A capture failure must never break the agent loop or unregister // the handler - swallow and keep the next prompt capturable. diff --git a/tests/history-session-writer.test.ts b/tests/history-session-writer.test.ts index 8601cea14..4128b7eef 100644 --- a/tests/history-session-writer.test.ts +++ b/tests/history-session-writer.test.ts @@ -9,7 +9,7 @@ import { projectHash, sessionFilePath, } from "../extensions/history/store.ts"; -import promptHistoryExtension from "../extensions/history/index.ts"; +import promptHistoryExtension, { captureEnabled } from "../extensions/history/index.ts"; function makeRoot(): string { return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-writer-")); @@ -29,6 +29,24 @@ function openWriterForTest(root: string, instanceId: string) { return openSessionWriter(root, CWD, instanceId); } +/** Load the extension against a temp root and return the capture handler. */ +function captureHandlerWith(env: NodeJS.ProcessEnv, root: string) { + const registered: Array<[string, unknown]> = []; + const pi = { + on: (event: string, handler: unknown) => { + registered.push([event, handler]); + }, + }; + promptHistoryExtension(pi as never, { + env, + root, + cwd: CWD, + instanceId: "inst-entry", + now: () => 1700000000000, + }); + return registered[0][1] as (event: unknown) => void; +} + test("no file is created until the first capture", () => { const root = makeRoot(); const state = openWriterForTest(root, "sess-1"); @@ -107,3 +125,44 @@ test("the slice-1 extension entry registers only the capture handler", () => { // would run getWriter() against the user's real ~/.pi/agent/history. assert.equal(typeof registered[0][1], "function"); }); + +test("captureEnabled is a strict opt-in", () => { + assert.equal(captureEnabled({}), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "0" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "false" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "off" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "yes" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: " 1 " }), true); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "TRUE" }), true); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "On" }), true); +}); + +test("the capture handler is a no-op unless the user opts in", () => { + const root = makeRoot(); + const handler = captureHandlerWith({}, root); + handler({ prompt: "sensitive prompt" }); + handler({ prompt: "another one" }); + // Nothing at all: no capture file, no project dir, no registry entry. + assert.deepEqual(fs.readdirSync(root), []); +}); + +test("an opted-in session captures delivered prompts", () => { + const root = makeRoot(); + const handler = captureHandlerWith({ GENTLE_PI_HISTORY_CAPTURE: "1" }, root); + handler({ prompt: "hello store" }); + assert.deepEqual(fileTexts(sessionFilePath(root, CWD, "inst-entry")), [ + "hello store", + ]); +}); + +test("disabling capture stops new lines and leaves existing files alone", () => { + const root = makeRoot(); + const env: NodeJS.ProcessEnv = { GENTLE_PI_HISTORY_CAPTURE: "true" }; + const handler = captureHandlerWith(env, root); + handler({ prompt: "kept" }); + const file = sessionFilePath(root, CWD, "inst-entry"); + assert.equal(fs.existsSync(file), true); + delete env.GENTLE_PI_HISTORY_CAPTURE; + handler({ prompt: "never written" }); + assert.deepEqual(fileTexts(file), ["kept"]); +}); From 5501d12d1ddc39e7efed2c6f5908b23364d6fcef Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 15:34:51 -0300 Subject: [PATCH 09/14] fix(history): fail closed on untrustworthy hidden.json tombstones A corrupt or unreadable hide file previously loaded as an empty hidden set (fail open), resurfacing prompts the user may have hidden because they contain secrets. The next hide also rewrote the file clean, silently clearing the incident. - readHiddenPrompts replaces loadHiddenPrompts: ENOENT stays trusted-empty (nothing ever hidden); any other read error, JSON parse failure, or non-array shape is untrusted (unreadable/corrupt/ malformed) and carries a recovery message naming hidden.json - hidePrompt refuses to write over an untrusted file: recovery is the explicit delete-or-restore of hidden.json, never a silent rewrite - drainProject/drainGlobal return DrainResult: untrusted tombstones block the drain (status "blocked", no prompts field) so the future selector UI must surface the warning; no stateDir keeps raw drain semantics Tests: rewrite T26 to pin the refusal + byte-unchanged file + manual unlink recovery; add malformed-shape, junk-item tolerance, and chmod 000 unreadable cases; drains pin the blocked shape (no prompts field) and the missing-file-stays-ok case. --- extensions/history/hide-prompts.ts | 83 ++++++++++--- extensions/history/store.ts | 53 ++++++-- tests/history-drain-hidden.test.ts | 62 +++++++++- tests/history-drain-order.test.ts | 22 +++- tests/history-hide-prompts.test.ts | 187 +++++++++++++++++++++++------ 5 files changed, 330 insertions(+), 77 deletions(-) diff --git a/extensions/history/hide-prompts.ts b/extensions/history/hide-prompts.ts index 9cffd6954..6d91a57d6 100644 --- a/extensions/history/hide-prompts.ts +++ b/extensions/history/hide-prompts.ts @@ -9,6 +9,14 @@ import { promptDedupKey } from "./selector-helpers.ts"; /** Name of the tombstone file inside the injected state dir (spec C4). */ const HIDE_FILE_NAME = "hidden.json"; +/** + * Shared recovery warning for a file that exists but cannot be trusted + * (spec C4, fail-closed READ half): toast-suitable, names hidden.json, and + * gives the user the explicit restore-or-delete choice. + */ +const RECOVERY_MESSAGE = + "The prompt-history hide list (hidden.json) is corrupt or unreadable. History is blocked until you restore the file or delete it (hidden prompts may then reappear)."; + /** * Result of one tombstone write (spec C4): `written` on a successful atomic * write, or an error object carrying a short, toast-suitable reason. Never @@ -19,32 +27,64 @@ export type HideResult = | { status: "error"; message: string }; /** - * Load the tombstone key set from `stateDir/hidden.json` — the READ half of - * the hide-file contract (spec C4). Fail-open: a missing, unreadable, - * corrupt, or wrong-shaped file is an EMPTY set and the call never throws; - * a corrupt file is rewritten clean by the next hide (the WRITE half, - * `hidePrompt`, lands in WU4). Keys are `promptDedupKey` strings written by - * `hidePrompt`; foreign values are ignored, never trusted. + * Result of one tombstone read (spec C4): `trusted` keys when the file is + * missing or holds a valid array, or `untrusted` when the file exists but + * cannot be trusted. History reads FAIL CLOSED on `untrusted`: callers must + * block the drain instead of emptying the tombstone set, because hidden + * prompts may contain secrets an empty set would resurface. */ -export function loadHiddenPrompts(stateDir: string): Set { +export type HiddenRead = + | { status: "trusted"; keys: Set } + | { + status: "untrusted"; + reason: "unreadable" | "corrupt" | "malformed"; + message: string; + }; + +/** + * Read the tombstone key set from `stateDir/hidden.json` — the READ half of + * the hide-file contract (spec C4). Fail-closed for history: a file that + * exists but is unreadable, corrupt, or wrong-shaped returns `untrusted` + * with the recovery warning so callers block the drain; it never degrades + * to an empty trusted set. A MISSING file — before any deletion — is the + * safe empty case and reads `trusted` with no keys. A valid array is + * trusted; junk items inside it are ignored, never trusted. Keys are + * `promptDedupKey` strings written by `hidePrompt`; the call never throws. + */ +export function readHiddenPrompts(stateDir: string): HiddenRead { let raw: string; try { raw = fs.readFileSync(path.join(stateDir, HIDE_FILE_NAME), "utf8"); - } catch { - return new Set(); // missing or unreadable → empty tombstones + } catch (error) { + const code = (error as { code?: unknown } | null | undefined)?.code; + if (code === "ENOENT") { + // Missing before any deletion: the safe empty tombstone set. + return { status: "trusted", keys: new Set() }; + } + return { + status: "untrusted", + reason: "unreadable", + message: RECOVERY_MESSAGE, + }; } let parsed: unknown; try { parsed = JSON.parse(raw); } catch { - return new Set(); // corrupt bytes → fail-open empty + return { status: "untrusted", reason: "corrupt", message: RECOVERY_MESSAGE }; } const keys = new Set(); - if (!Array.isArray(parsed)) return keys; // wrong shape → fail-open empty + if (!Array.isArray(parsed)) { + return { + status: "untrusted", + reason: "malformed", + message: RECOVERY_MESSAGE, + }; + } for (const item of parsed) { if (typeof item === "string" && item !== "") keys.add(item); } - return keys; + return { status: "trusted", keys }; } /** @@ -52,18 +92,23 @@ export function loadHiddenPrompts(stateDir: string): Set { * WRITE half of the hide-file contract (spec C4). The key is the shared * `promptDedupKey` (byte-match normative with the merge filter — never a * re-implementation); the set compacts on write and persists as a SORTED - * array via the shared atomic tmp+rename writer. Fail-open both ways: a - * corrupt or missing file reads as empty (this clean rewrite IS the - * recovery — the corrupt contents are untrustworthy by definition) and any - * write failure returns an error object for the delete-flow toast; the + * array via the shared atomic tmp+rename writer. An untrusted existing file + * is never silently reset (a clean rewrite would clear the blocked state + * one hide later): hidePrompt refuses with the recovery warning until the + * user restores or deletes the file. A missing file is the clean baseline; + * any write failure returns an error object for the delete-flow toast; the * call never throws. */ export function hidePrompt(stateDir: string, text: string): HideResult { - const keys = loadHiddenPrompts(stateDir); - keys.add(promptDedupKey(text)); + const read = readHiddenPrompts(stateDir); + if (read.status === "untrusted") { + // Refuse without writing: never reset the untrusted state silently. + return { status: "error", message: read.message }; + } + read.keys.add(promptDedupKey(text)); const written = writeJsonAtomic( path.join(stateDir, HIDE_FILE_NAME), - [...keys].sort(), + [...read.keys].sort(), ); return written ? { status: "written" } diff --git a/extensions/history/store.ts b/extensions/history/store.ts index fb470515c..b63b07719 100644 --- a/extensions/history/store.ts +++ b/extensions/history/store.ts @@ -11,7 +11,7 @@ import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; -import { loadHiddenPrompts } from "./hide-prompts.ts"; +import { readHiddenPrompts } from "./hide-prompts.ts"; // =========================================================================== // Paths (formerly store-paths.ts) @@ -350,20 +350,53 @@ function sortFilesForDrain(files: string[]): string[] { .map((f) => f.file); } +/** + * Result of a scope drain: `ok` with the drained prompts, or `blocked` + * when the tombstone file is untrusted (fail-closed READ half). The + * blocked shape carries NO prompts field, so a caller cannot accidentally + * render prompts that may include hidden ones. + */ +export type DrainResult = + | { status: "ok"; prompts: string[] } + | { status: "blocked"; message: string }; + +/** + * Shared drain tail: without a `stateDir` the raw drain semantics hold (no + * filter). With one, the tombstone filter applies and fails CLOSED: an + * untrusted hidden.json (unreadable, corrupt, wrong shape) blocks the + * whole drain with the recovery message instead of resurfacing hidden + * prompts; a missing file is the safe empty tombstone set and drains + * normally. + */ +function drainWithHidden( + files: string[], + limit: number, + stateDir?: string, +): DrainResult { + if (!stateDir) return { status: "ok", prompts: drainFiles(files, limit) }; + const read = readHiddenPrompts(stateDir); + if (read.status === "untrusted") { + return { status: "blocked", message: read.message }; + } + return { status: "ok", prompts: drainFiles(files, limit, read.keys) }; +} + /** * Drain the PROJECT scope: all .jsonl files in the project dir (seed.jsonl * included), mtime-newest-first, deduped, capped at `limit` (default 1000). + * With a `stateDir`, the tombstone filter applies and fails closed: an + * untrusted hidden.json blocks the drain (see DrainResult). */ export function drainProject( root: string, cwd: string, limit: number = 1000, stateDir?: string, -): string[] { - return drainFiles( +): DrainResult { + return drainWithHidden( sortFilesForDrain(listProjectFiles(path.join(root, "projects", projectHash(cwd)))), limit, - stateDir ? loadHiddenPrompts(stateDir) : new Set(), + stateDir, ); } @@ -371,13 +404,15 @@ export function drainProject( * Drain the GLOBAL scope: every project dir's files, mtime-newest-first, * deduped, capped — with the legacy global seed appended LAST (deliberate: * it is the least specific, migrated source, so per-project entries win - * recency and keep-first dedup favors them). + * recency and keep-first dedup favors them). With a `stateDir`, the + * tombstone filter applies and fails closed: an untrusted hidden.json + * blocks the drain (see DrainResult). */ export function drainGlobal( root: string, limit: number = 1000, stateDir?: string, -): string[] { +): DrainResult { const files: string[] = []; const globalSeed = globalSeedPath(root); @@ -397,9 +432,5 @@ export function drainGlobal( } const sorted = sortFilesForDrain(files); if (fs.existsSync(globalSeed)) sorted.push(globalSeed); // legacy last - return drainFiles( - sorted, - limit, - stateDir ? loadHiddenPrompts(stateDir) : new Set(), - ); + return drainWithHidden(sorted, limit, stateDir); } diff --git a/tests/history-drain-hidden.test.ts b/tests/history-drain-hidden.test.ts index 90c479758..73a686b86 100644 --- a/tests/history-drain-hidden.test.ts +++ b/tests/history-drain-hidden.test.ts @@ -8,6 +8,7 @@ import { drainProject, globalSeedPath, projectHash, + type DrainResult, } from "../extensions/history/store.ts"; // Portable project identity: a never-existing literal. projectHash falls @@ -25,6 +26,14 @@ function write(file: string, texts: string[], ts = 100): void { ); } +// Unwrap the ok shape. Drains FAIL CLOSED: the blocked variant carries no +// prompts field at all (asserted in the blocked test below). +function okPrompts(result: DrainResult): string[] { + assert.equal(result.status, "ok"); + if (result.status !== "ok") throw new Error("unreachable"); + return result.prompts; +} + test("drains skip tombstoned prompts in seeds and session files", () => { const base = fs.mkdtempSync(path.join(os.tmpdir(), "hid-")); const root = path.join(base, "h"); @@ -40,15 +49,62 @@ test("drains skip tombstoned prompts in seeds and session files", () => { write(path.join(dir, "s1.jsonl"), ["also keep", "deleted from session"], 200); write(globalSeedPath(root), ["deleted from seed", "legacy keep"], 50); - assert.deepEqual(drainProject(root, CWD, 1000, stateDir), [ + assert.deepEqual(okPrompts(drainProject(root, CWD, 1000, stateDir)), [ "also keep", "keep", ]); - assert.deepEqual(drainGlobal(root, 1000, stateDir), [ + assert.deepEqual(okPrompts(drainGlobal(root, 1000, stateDir)), [ "also keep", "keep", "legacy keep", ]); // Without a stateDir the filter is off (raw drain semantics). - assert.equal(drainProject(root, CWD).includes("deleted from seed"), true); + assert.equal( + okPrompts(drainProject(root, CWD)).includes("deleted from seed"), + true, + ); +}); + +// Fail-closed seam: an untrusted hidden.json BLOCKS both drains with the +// recovery message and no prompts field; a stateDir whose hidden.json is +// MISSING stays the safe empty-tombstones case (the full expected prompts). +test("corrupt hidden.json blocks both drains with no prompts field; a missing file drains normally", () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "hid-blocked-")); + const root = path.join(base, "h"); + const stateDir = path.join(base, "state"); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "hidden.json"), + "{corrupt bytes", + "utf8", + ); + const dir = path.join(root, "projects", projectHash(CWD)); + write(path.join(dir, "seed.jsonl"), ["secret prompt", "keeper"], 100); + + for (const result of [ + drainProject(root, CWD, 1000, stateDir), + drainGlobal(root, 1000, stateDir), + ]) { + assert.equal(result.status, "blocked"); + if (result.status !== "blocked") throw new Error("unreachable"); + assert.ok(result.message.includes("hidden.json")); + // The blocked shape carries no prompts to render. + assert.equal("prompts" in result, false); + } + + // Missing file: safe empty tombstones — the full drain comes back. + const missingBase = fs.mkdtempSync(path.join(os.tmpdir(), "hid-missing-")); + const missingRoot = path.join(missingBase, "h"); + const missingState = path.join(missingBase, "state"); + fs.mkdirSync(missingState, { recursive: true }); + const missingDir = path.join(missingRoot, "projects", projectHash(CWD)); + write(path.join(missingDir, "seed.jsonl"), ["kept", "shown"], 100); + assert.deepEqual( + okPrompts(drainProject(missingRoot, CWD, 1000, missingState)), + ["shown", "kept"], + ); + assert.deepEqual(okPrompts(drainGlobal(missingRoot, 1000, missingState)), [ + "shown", + "kept", + ]); }); diff --git a/tests/history-drain-order.test.ts b/tests/history-drain-order.test.ts index 86cfddca9..be2f8fdfc 100644 --- a/tests/history-drain-order.test.ts +++ b/tests/history-drain-order.test.ts @@ -8,6 +8,7 @@ import { drainProject, globalSeedPath, projectHash, + type DrainResult, } from "../extensions/history/store.ts"; // Portable project identity: a never-existing literal. projectHash falls @@ -25,18 +26,25 @@ function writeTs(file: string, texts: string[], ts: number): void { ); } +// Mechanical unwrap of the ok shape (drains can also return blocked). +function okPrompts(result: DrainResult): string[] { + assert.equal(result.status, "ok"); + if (result.status !== "ok") throw new Error("unreachable"); + return result.prompts; +} + test("atomic rewrite (delete) does not reshuffle the drain order", () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), "ord-")); const dir = path.join(root, "projects", projectHash(CWD)); writeTs(path.join(dir, "old.jsonl"), ["a-old"], 100); writeTs(path.join(dir, "new.jsonl"), ["z-new"], 200); - assert.deepEqual(drainProject(root, CWD), ["z-new", "a-old"]); + assert.deepEqual(okPrompts(drainProject(root, CWD)), ["z-new", "a-old"]); // Slice 5 ports deleteFromProject; its observable effect on the drain is // simulated directly here: an atomic rewrite of the affected file that // empties it — the mtime jumps to NOW, and the drain order must not move. fs.writeFileSync(path.join(dir, "old.jsonl"), "", "utf8"); fs.utimesSync(path.join(dir, "old.jsonl"), new Date(), new Date()); - assert.deepEqual(drainProject(root, CWD), ["z-new"]); + assert.deepEqual(okPrompts(drainProject(root, CWD)), ["z-new"]); // Re-add with an OLD ts via direct write: still ordered by ts, not mtime. writeTs(path.join(dir, "old2.jsonl"), ["b-old"], 150); fs.utimesSync( @@ -44,7 +52,7 @@ test("atomic rewrite (delete) does not reshuffle the drain order", () => { new Date(Date.now() + 99999), new Date(Date.now() + 99999), ); - assert.deepEqual(drainProject(root, CWD), ["z-new", "b-old"]); + assert.deepEqual(okPrompts(drainProject(root, CWD)), ["z-new", "b-old"]); }); test("global drain puts the legacy seed last regardless of its fresh mtime", () => { @@ -54,7 +62,11 @@ test("global drain puts the legacy seed last regardless of its fresh mtime", () const seed = globalSeedPath(root); writeTs(seed, ["legacy-1", "legacy-2"], 10); fs.utimesSync(seed, new Date(Date.now() + 5000), new Date(Date.now() + 5000)); - assert.deepEqual(drainGlobal(root), ["fresh", "legacy-2", "legacy-1"]); + assert.deepEqual(okPrompts(drainGlobal(root)), [ + "fresh", + "legacy-2", + "legacy-1", + ]); }); test( @@ -78,7 +90,7 @@ test( try { // An unreadable file reads as zero entries and drops out of the drain; // the readable files keep their ts order. No throw. - assert.deepEqual(drainProject(root, CWD), ["z-new", "a-old"]); + assert.deepEqual(okPrompts(drainProject(root, CWD)), ["z-new", "a-old"]); } finally { fs.chmodSync(sealed, 0o644); // restore before cleanup } diff --git a/tests/history-hide-prompts.test.ts b/tests/history-hide-prompts.test.ts index 03c054863..ccd1f8ea8 100644 --- a/tests/history-hide-prompts.test.ts +++ b/tests/history-hide-prompts.test.ts @@ -3,13 +3,20 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { hidePrompt, loadHiddenPrompts } from "../extensions/history/hide-prompts.ts"; +import { + hidePrompt, + readHiddenPrompts, +} from "../extensions/history/hide-prompts.ts"; import { promptDedupKey } from "../extensions/history/selector-helpers.ts"; // Unit WU4 — tombstone write half + read half (spec C4, design §D6). fs-only -// coverage. The dev suite's deleteCurrent source-parse pins (T27/T28) and -// the deletionActionsFor planner pins cover the slice-3 selector branch and -// the slice-5 delete flow; they port with those slices. +// coverage. The READ half FAILS CLOSED for history: a file that exists but +// cannot be trusted (unreadable, corrupt, wrong shape) reads `untrusted` +// with a recovery warning instead of an empty tombstone set, and the WRITE +// half refuses without a silent rewrite. The dev suite's deleteCurrent +// source-parse pins (T27/T28) and the deletionActionsFor planner pins cover +// the slice-3 selector branch and the slice-5 delete flow; they port with +// those slices. function makeStateDir(name: string): string { return fs.mkdtempSync(path.join(os.tmpdir(), `hide-prompts-${name}-`)); @@ -21,6 +28,29 @@ function readHideFile(stateDir: string) { ); } +/** Assert an untrusted read of the expected reason; returns its message. */ +function assertUntrusted( + stateDir: string, + reason: "unreadable" | "corrupt" | "malformed", +): string { + const read = readHiddenPrompts(stateDir); + assert.equal(read.status, "untrusted"); + if (read.status !== "untrusted") throw new Error("unreachable"); + assert.equal(read.reason, reason); + // The recovery warning names the file and offers restore-or-delete. + assert.ok(read.message.includes("hidden.json")); + assert.ok(/restore|delete/.test(read.message)); + return read.message; +} + +/** Assert a trusted read and return its key set. */ +function trustedKeys(stateDir: string): Set { + const read = readHiddenPrompts(stateDir); + assert.equal(read.status, "trusted"); + if (read.status !== "trusted") throw new Error("unreachable"); + return read.keys; +} + // T24 — AC-S4-1: hide-key fidelity. Tombstone keys must byte-match the // Change 2 dedup key for the same text — same imported helper, never a // re-implementation: the stored file content is compared against @@ -41,59 +71,138 @@ test("T24 (AC-S4-1): hide keys byte-match promptDedupKey across whitespace, case assert.ok(Array.isArray(stored), "hidden.json must hold a JSON array"); // Byte-match: the file holds EXACTLY the shared helper's output, sorted. assert.deepEqual(stored, texts.map((text) => promptDedupKey(text)).sort()); - // The loaded set agrees. - const loaded = loadHiddenPrompts(stateDir); + // The read half agrees. + const keys = trustedKeys(stateDir); + assert.equal(keys.size, stored.length); for (const key of stored) { - assert.ok(loaded.has(key)); + assert.ok(keys.has(key)); } }); // T25 — AC-S4-2: hide persistence and tolerance. Two deletes of the same -// text compact to ONE key; a missing hide file reads as an empty set; reads -// never throw. -test("T25 (AC-S4-2): duplicate hides compact to one key; a missing file reads as empty; reads never throw", () => { +// text compact to ONE key; a MISSING hide file (before any deletion) is the +// safe empty case — trusted with no keys; reads never throw. +test("T25 (AC-S4-2): duplicate hides compact to one key; a missing file reads trusted-empty; reads never throw", () => { const stateDir = makeStateDir("t25"); - // Missing file: empty set, no throw (before any write exists). - assert.equal(loadHiddenPrompts(stateDir).size, 0); + // Missing file: trusted empty tombstones (before any write exists). + assert.equal(trustedKeys(stateDir).size, 0); // Two deletes of the same text — variants differing by case + whitespace // runs normalize onto the same key. assert.deepEqual(hidePrompt(stateDir, "Same Text"), { status: "written" }); assert.deepEqual(hidePrompt(stateDir, "same text"), { status: "written" }); const stored = readHideFile(stateDir); assert.deepEqual(stored, [promptDedupKey("same text")]); - const loaded = loadHiddenPrompts(stateDir); - assert.equal(loaded.size, 1); - assert.ok(loaded.has(promptDedupKey("same text"))); + const keys = trustedKeys(stateDir); + assert.equal(keys.size, 1); + assert.ok(keys.has(promptDedupKey("same text"))); }); -// T26 — AC-S4-5: corrupt hidden.json is fail-open (READ half) AND the next -// hide rewrites the file clean as a sorted compact array — the rewrite half -// is the recovery path. -test("T26 (AC-S4-5): corrupt hidden.json loads as empty and the next hide rewrites it clean", () => { +// T26 — AC-S4-5: corrupt hidden.json FAILS CLOSED for history reads. The +// READ half reports untrusted (corrupt) so callers block history instead of +// resurfacing hidden prompts; the WRITE half refuses WITHOUT a silent clean +// rewrite (the old fail-open behavior cleared the blocked state one hide +// later). Recovery is manual — restore or delete the file; after deletion +// the next hide succeeds and reads are trusted again. +test("T26 (AC-S4-5): corrupt hidden.json reads untrusted; hide refuses without rewriting; deleting the file recovers", () => { const stateDir = makeStateDir("t26"); - fs.writeFileSync( - path.join(stateDir, "hidden.json"), - "{corrupt bytes", - "utf8", - ); - assert.equal(loadHiddenPrompts(stateDir).size, 0); + const hidePath = path.join(stateDir, "hidden.json"); + fs.writeFileSync(hidePath, "{corrupt bytes", "utf8"); + // READ half: untrusted/corrupt — never an empty trusted set. + const message = assertUntrusted(stateDir, "corrupt"); + // WRITE half: refused, and the corrupt bytes are UNCHANGED — the blocked + // state is never silently reset (no-silent-rewrite pin). + const before = fs.readFileSync(hidePath, "utf8"); + assert.deepEqual(hidePrompt(stateDir, "beta prompt"), { + status: "error", + message, + }); + assert.equal(fs.readFileSync(hidePath, "utf8"), before); + // Manual recovery: delete the file; the next hide succeeds and reads are + // trusted with exactly the new key. + fs.unlinkSync(hidePath); assert.deepEqual(hidePrompt(stateDir, "beta prompt"), { status: "written" }); - // The rewrite landed: clean JSON holding exactly the new key. - assert.deepEqual(readHideFile(stateDir), [promptDedupKey("beta prompt")]); - assert.equal(loadHiddenPrompts(stateDir).size, 1); + const keys = trustedKeys(stateDir); + assert.equal(keys.size, 1); + assert.ok(keys.has(promptDedupKey("beta prompt"))); }); -// WU4c — write-failure path (AC-S4-2 triangulation): a state dir that cannot -// be created (its parent is a regular file) makes the atomic write return -// false, and hidePrompt maps that to the toast-suitable error object — -// never a throw. -test("hide write failure returns the exact error shape for the delete-flow toast", () => { - const base = makeStateDir("fail"); - const blocker = path.join(base, "blocker"); - fs.writeFileSync(blocker, "regular file", "utf8"); - const stateDir = path.join(blocker, "sealed"); // parent is a file → ENOTDIR - assert.deepEqual(hidePrompt(stateDir, "kept prompt"), { +// Wrong-shaped file: valid JSON that is not an array fails closed too (both +// halves), while junk items inside a VALID array are ignored and the real +// keys stay trusted. +test("malformed hidden.json fails closed for reads and writes; junk items in a valid array are ignored", () => { + const malformed = makeStateDir("malformed"); + const hidePath = path.join(malformed, "hidden.json"); + for (const shape of ["{}", JSON.stringify({ keys: [] })]) { + fs.writeFileSync(hidePath, shape, "utf8"); + assertUntrusted(malformed, "malformed"); + } + // The write half refuses the malformed file as well. + const message = assertUntrusted(malformed, "malformed"); + assert.deepEqual(hidePrompt(malformed, "kept prompt"), { status: "error", - message: "Could not write the hide file; the prompt may reappear.", + message, }); + + // Junk items are ignored, never trusted; real keys survive. + const junk = makeStateDir("junk"); + fs.writeFileSync( + path.join(junk, "hidden.json"), + JSON.stringify([42, "", "real-key", null]), + "utf8", + ); + const keys = trustedKeys(junk); + assert.equal(keys.size, 1); + assert.ok(keys.has("real-key")); }); + +// Unreadable file: a hidden.json that cannot be read at all fails closed +// for reads, and the write half refuses too. Skipped as root, where chmod +// 000 does not block reads; permissions are restored in finally. +test( + "an unreadable hidden.json fails closed for reads and refuses writes", + { skip: process.getuid?.() === 0 }, + () => { + const stateDir = makeStateDir("sealed"); + const hidePath = path.join(stateDir, "hidden.json"); + fs.writeFileSync(hidePath, '["kept-key"]', "utf8"); + fs.chmodSync(hidePath, 0o000); + try { + const message = assertUntrusted(stateDir, "unreadable"); + assert.deepEqual(hidePrompt(stateDir, "beta prompt"), { + status: "error", + message, + }); + } finally { + fs.chmodSync(hidePath, 0o644); // restore before cleanup + } + }, +); + +// WU4c — write-failure path (AC-S4-2 triangulation): a trusted read whose +// atomic write fails makes hidePrompt return the toast-suitable error +// object — never a throw. The state dir is made non-writable while the +// existing hidden.json stays readable (a state dir whose PATH is blocked +// by a regular file is now the untrusted-refusal case instead — the READ +// half fails closed before any write). Skipped as root, where chmod-based +// write blocking does not apply; permissions are restored in finally. +test( + "hide write failure returns the exact error shape for the delete-flow toast", + { skip: process.getuid?.() === 0 }, + () => { + const stateDir = makeStateDir("fail"); + fs.writeFileSync( + path.join(stateDir, "hidden.json"), + '["kept-key"]', + "utf8", + ); + fs.chmodSync(stateDir, 0o555); // read+execute, no write → EACCES on tmp + try { + assert.deepEqual(hidePrompt(stateDir, "kept prompt"), { + status: "error", + message: "Could not write the hide file; the prompt may reappear.", + }); + } finally { + fs.chmodSync(stateDir, 0o700); // restore before cleanup + } + }, +); From 831bbe9689f0d2cec8b4e39ff671b28531834af5 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:47:37 -0300 Subject: [PATCH 10/14] fix(history): make prompt capture opt-in and document the store Review follow-up on the slice-01 PR: the before_agent_start handler recorded delivered prompts by default while the deletion UI is still unshipped, so an intermediate release could accumulate sensitive prompts with no removal path. - Capture is now strictly opt-in via GENTLE_PI_HISTORY_CAPTURE=1|true|on (default off); the switch doubles as the disable path, is checked per prompt, and a disabled session writes nothing - no registry entry, no files. - promptHistoryExtension takes injectable deps (env/root/cwd/ instanceId/now) with one writer closure per extension load. - New tests: strict opt-in matrix, default-off inertness, opted-in capture, disable-leaves-existing-files. - docs/prompt-history.md documents the switch, storage locations, permissions/readers, and disable/removal semantics; the README docs table gains a pointer. --- README.md | 1 + docs/prompt-history.md | 61 +++++++++++++++++++++++++ extensions/history/index.ts | 64 ++++++++++++++++++++++++--- tests/history-session-writer.test.ts | 66 +++++++++++++++++++++++++++- 4 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 docs/prompt-history.md diff --git a/README.md b/README.md index 9e20d72f4..0207c389c 100644 --- a/README.md +++ b/README.md @@ -880,6 +880,7 @@ To opt out: | `docs/skill-style-guide.md` | Normative style guide used by the packaged skill creation/improvement skills. | | `docs/native-authority-architecture.md` | Post-U8 ownership boundary, reproducible slimming metrics, Windows evidence, exact #191 seam, and the `review-integration/v1`→`v2` migration status, including the "compact-v2" naming disambiguation. | | `docs/review-integration.md` | Negotiated provider/consumer contract and the current Gentle Pi adoption boundary. | +| `docs/prompt-history.md` | Prompt-history slice 1: opt-in capture switch, storage layout, readers, and disable/removal semantics. | ## Development diff --git a/docs/prompt-history.md b/docs/prompt-history.md new file mode 100644 index 000000000..1fa8eba4f --- /dev/null +++ b/docs/prompt-history.md @@ -0,0 +1,61 @@ +# Prompt history + +Slice 1 of the prompt-history extension (#819 split) ships the storage layer only: +a per-instance JSONL capture store, project identity, and the read/write +primitives later slices build on. The selector UI, deletion/scope drains, and GC +arrive in later slices of the chain. + +## Capture is opt-in + +Recording is **off by default**. Delivered prompts can contain secrets, and the +deletion UI is not shipped yet, so nothing is stored unless you explicitly opt in: + +```bash +GENTLE_PI_HISTORY_CAPTURE=1 pi +``` + +- Enabled by `1`, `true`, or `on` (case-insensitive). Unset, empty, or any other + value means **off** — the same switch is the disable path. +- The check runs per prompt: unsetting the switch (or setting it to `0`) stops + new captures immediately, no pi restart needed. +- With capture off the extension is inert: no registry entry, no files, and + prompts are never written. + +## Where the files live + +Everything sits under `~/.pi/agent/history/`: + +- `registry.json` — advisory map of project hash → cwd, used for display + labels. +- `projects//.jsonl` — one append-only capture file per pi + process. + +`` is the first 16 hex chars of the SHA-256 of the canonicalized project +cwd; `` is a per-process UUID. Each line is one delivered prompt: + +```json +{"v":1,"text":"the prompt as delivered","ts":1700000000000} +``` + +UI command-like prompts (`/name ...`) and empty lines are never stored. Later +slices add the rebuildable `seed.jsonl`, scope drains/deletes, and GC. + +## Who can read them + +The store is plain JSONL on your local disk, not encrypted. Files are created by +the pi process with default umask permissions (typically `0644` files inside +`0755` directories), so any process running as your OS user can read them, and +other local accounts can too wherever they can traverse your home directory. +Treat the store as sensitive: it holds your prompts verbatim. + +## What disabling capture does + +Turning the switch off only stops **new** captures. Nothing is deleted: files +already written — and the registry entry — stay on disk until you remove them or +the deletion UI ships. To erase the store manually while capture is off (or pi +is not running): + +```bash +rm -rf ~/.pi/agent/history # whole store +rm -rf ~/.pi/agent/history/projects/ # one project (see registry.json) +``` diff --git a/extensions/history/index.ts b/extensions/history/index.ts index d82b413d9..31e5c5755 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -5,6 +5,12 @@ // and the shortcut/command wiring over the slice-1 writer and slice-2 // drains. Legacy migration and seed bootstrap (slice 4), deletion (slice 5), // and GC/compaction (slice 6) arrive in later slices. +// +// Capture is OPT-IN while the deletion/privacy behavior is unshipped: +// nothing is recorded unless GENTLE_PI_HISTORY_CAPTURE=1|true|on. With the +// switch off the handler is a no-op — no registry entry, no files, and +// prompts are never written. Unsetting the switch only stops NEW captures; +// files already written stay on disk (docs/prompt-history.md). import { join } from "node:path"; import { homedir } from "node:os"; @@ -75,7 +81,6 @@ const PREVIEW_WHEEL_Y_LAST = 26; // v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). const PI_HISTORY_ROOT = join(homedir(), ".pi", "agent", "history"); -const AGENT_DIR = join(homedir(), ".pi", "agent"); const CURRENT_CWD = process.cwd(); // Instance identity: one exclusive capture file per pi process. const INSTANCE_ID = randomUUID(); @@ -908,15 +913,62 @@ function recordsFromEntries( return buildPromptRecords(dedupePromptEntries(entries)); } -export default function promptHistoryExtension(pi: ExtensionAPI) { - // One writer per extension load; see getWriter() for the init order. - // Persist every delivered user prompt (write-through, append-only JSONL). - // The local ExtensionAPI stub types handler args as unknown; narrow here. +export interface HistoryDeps { + env?: NodeJS.ProcessEnv; + root?: string; + cwd?: string; + instanceId?: string; + now?: () => number; +} + +/** + * Strict opt-in: capture stays off unless GENTLE_PI_HISTORY_CAPTURE is + * explicitly 1, true, or on (case-insensitive). The same switch is the + * disable path — unsetting it stops new captures; files already on disk + * are left untouched until the deletion tooling lands. + */ +export function captureEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + const value = env.GENTLE_PI_HISTORY_CAPTURE?.trim().toLowerCase(); + return value === "1" || value === "true" || value === "on"; +} + +export default function promptHistoryExtension( + pi: ExtensionAPI, + deps: HistoryDeps = {}, +): void { + const env = deps.env ?? process.env; + const root = deps.root ?? PI_HISTORY_ROOT; + const cwd = deps.cwd ?? CURRENT_CWD; + const instanceId = deps.instanceId ?? INSTANCE_ID; + const now = deps.now ?? Date.now; + let writerState: SessionWriterState | null = null; + + /** + * One-time init per extension load: register the project in the advisory + * registry, then open this instance's exclusive capture file. Legacy + * migration and seed bootstrap join this init order in a later slice. + */ + const getWriter = (): SessionWriterState => { + if (!writerState) { + try { + ensureRegistryEntry(root, cwd); + } catch { + // registry is advisory + } + writerState = openSessionWriter(root, cwd, instanceId); + } + return writerState; + }; + + // Persist every delivered user prompt (write-through, append-only JSONL), + // but only for opted-in sessions — see captureEnabled(). The local + // ExtensionAPI stub types handler args as unknown; narrow here. pi.on("before_agent_start", (...args: unknown[]) => { + if (!captureEnabled(env)) return; try { const event = args[0] as { prompt?: string } | undefined; - appendSessionCapture(getWriter(), event?.prompt ?? "", Date.now()); + appendSessionCapture(getWriter(), event?.prompt ?? "", now()); } catch { // A capture failure must never break the agent loop or unregister // the handler - swallow and keep the next prompt capturable. diff --git a/tests/history-session-writer.test.ts b/tests/history-session-writer.test.ts index 61a2689b3..671ca6c50 100644 --- a/tests/history-session-writer.test.ts +++ b/tests/history-session-writer.test.ts @@ -9,7 +9,7 @@ import { projectHash, sessionFilePath, } from "../extensions/history/store.ts"; -import promptHistoryExtension from "../extensions/history/index.ts"; +import promptHistoryExtension, { captureEnabled } from "../extensions/history/index.ts"; function makeRoot(): string { return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-writer-")); @@ -29,6 +29,29 @@ function openWriterForTest(root: string, instanceId: string) { return openSessionWriter(root, CWD, instanceId); } +/** Load the extension against a temp root and return the capture handler. */ +function captureHandlerWith(env: NodeJS.ProcessEnv, root: string) { + const registered: Array<[string, unknown]> = []; + const pi = { + on: (event: string, handler: unknown) => { + registered.push([event, handler]); + }, + // Slice-3 wiring surface: the factory also registers the shortcut, + // command, and tool_call dismissal; the capture handler stays the + // first registration, so these no-ops only absorb the extra wiring. + registerShortcut: () => {}, + registerCommand: () => {}, + }; + promptHistoryExtension(pi as never, { + env, + root, + cwd: CWD, + instanceId: "inst-entry", + now: () => 1700000000000, + }); + return registered[0][1] as (event: unknown) => void; +} + test("no file is created until the first capture", () => { const root = makeRoot(); const state = openWriterForTest(root, "sess-1"); @@ -121,3 +144,44 @@ test("the extension entry registers exactly the slice-3 wiring surface", () => { assert.equal(typeof handler, "function"); } }); + +test("captureEnabled is a strict opt-in", () => { + assert.equal(captureEnabled({}), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "0" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "false" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "off" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "yes" }), false); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: " 1 " }), true); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "TRUE" }), true); + assert.equal(captureEnabled({ GENTLE_PI_HISTORY_CAPTURE: "On" }), true); +}); + +test("the capture handler is a no-op unless the user opts in", () => { + const root = makeRoot(); + const handler = captureHandlerWith({}, root); + handler({ prompt: "sensitive prompt" }); + handler({ prompt: "another one" }); + // Nothing at all: no capture file, no project dir, no registry entry. + assert.deepEqual(fs.readdirSync(root), []); +}); + +test("an opted-in session captures delivered prompts", () => { + const root = makeRoot(); + const handler = captureHandlerWith({ GENTLE_PI_HISTORY_CAPTURE: "1" }, root); + handler({ prompt: "hello store" }); + assert.deepEqual(fileTexts(sessionFilePath(root, CWD, "inst-entry")), [ + "hello store", + ]); +}); + +test("disabling capture stops new lines and leaves existing files alone", () => { + const root = makeRoot(); + const env: NodeJS.ProcessEnv = { GENTLE_PI_HISTORY_CAPTURE: "true" }; + const handler = captureHandlerWith(env, root); + handler({ prompt: "kept" }); + const file = sessionFilePath(root, CWD, "inst-entry"); + assert.equal(fs.existsSync(file), true); + delete env.GENTLE_PI_HISTORY_CAPTURE; + handler({ prompt: "never written" }); + assert.deepEqual(fileTexts(file), ["kept"]); +}); From e787ce464758bb9883924cf9c468ddd62dc65ff6 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:18:01 -0300 Subject: [PATCH 11/14] chore(readme): remove README delta from history slice The history slice branches must not touch README.md: the docs table lives in main and evolves independently of the extension slices. The opt-in capture documentation stays in docs/prompt-history.md; the README pointer row introduced by the capture-gate commit is dropped and README.md is restored to upstream/main verbatim. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index db19d3529..a8a5ee727 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,6 @@ Start with the product-facing destination, then move into the operational refere | [Telemetry](docs/telemetry.md) | Approved fields and source limitations. | | [Delegated verification](docs/delegated-verification.md) | Practical verification guidance. | | [Skill style guide](docs/skill-style-guide.md) | The package skill contract. | -| [Prompt history](docs/prompt-history.md) | Opt-in capture switch, storage layout, readers, and disable/removal semantics. |

Back to top ↑

From 892da55c51d7eb56d5e9186b329fe9f95de98a81 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:18:44 -0300 Subject: [PATCH 12/14] chore(readme): remove README delta from history slice The history slice branches must not touch README.md: the docs table lives in main and evolves independently of the extension slices. The opt-in capture documentation stays in docs/prompt-history.md; the README pointer row introduced by the capture-gate commit is dropped and README.md is restored to upstream/main verbatim. --- README.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index df59b272e..a8a5ee727 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,6 @@ A bare terminal answers "what is the agent doing?" only with scrollback. gentle- ### el Gentleman — Think before you build -Diagram of el Gentleman turning human intent into clarified scope, a smallest workflow choice, evidence, and a human delivery decision - Say what you need once, then keep moving. el Gentleman helps turn intent into clear scope, a sensible next step, and evidence people can review — without making every task feel like a process meeting. **[Docs →](docs/readme-reference.md#organic-driven-development)** @@ -156,9 +154,7 @@ Model, effort, and who does what should be choices, not accidents. Named profile ### Command palette — Every command, one keystroke away -Command palette with a search field and grouped entries: Configuration, Session, Diagnostics, SDD, and Skills - -Extension commands are only useful if you can find them. `alt+k` opens a curated, grouped palette — Configuration, Session, Diagnostics, SDD, and Skills — searchable by label, command name, or description, showing entries only when they are actually registered. +Extension commands are only useful if you can find them. `alt+k` opens a curated, grouped palette — Configuration, Session, Diagnostics, and Skills — searchable by label, command name, or description, showing entries only when they are actually registered. **[Docs →](docs/gentle-shell.md#command-palette)** @@ -278,13 +274,12 @@ Start with the product-facing destination, then move into the operational refere | Destination | Purpose | | --- | --- | | [gentle-shell reference](docs/gentle-shell.md) | Workspace layout, changes, usage, agents, and todo interactions. | -| [ODD workflow](docs/readme-reference.md#organic-driven-development) · [Technical reference](docs/readme-reference.md) | Everyday work and recovery, optional SDD/OpenSpec, installation, configuration, commands, and contributor detail. | +| [ODD workflow](docs/readme-reference.md#organic-driven-development) · [Technical reference](docs/readme-reference.md) | Everyday work and recovery, installation, configuration, commands, and contributor detail. | | [Review integration](docs/review-integration.md) | The provider/consumer boundary for native review. | | [Native authority architecture](docs/native-authority-architecture.md) | Ownership boundaries and review architecture. | | [Telemetry](docs/telemetry.md) | Approved fields and source limitations. | | [Delegated verification](docs/delegated-verification.md) | Practical verification guidance. | | [Skill style guide](docs/skill-style-guide.md) | The package skill contract. | -| [Prompt history](docs/prompt-history.md) | Opt-in capture switch, storage layout and readability, and disable/removal semantics. |

Back to top ↑

From aae37cb3d22e2cabe11f83719e32d8d65fb433ef Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:19:47 -0300 Subject: [PATCH 13/14] chore(readme): remove README delta from history slice The history slice branches must not touch README.md: the docs table lives in main and evolves independently of the extension slices. The opt-in capture documentation stays in docs/prompt-history.md; the README pointer row introduced by the capture-gate commit is dropped and README.md is restored to upstream/main verbatim. --- README.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index df59b272e..a8a5ee727 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,6 @@ A bare terminal answers "what is the agent doing?" only with scrollback. gentle- ### el Gentleman — Think before you build -Diagram of el Gentleman turning human intent into clarified scope, a smallest workflow choice, evidence, and a human delivery decision - Say what you need once, then keep moving. el Gentleman helps turn intent into clear scope, a sensible next step, and evidence people can review — without making every task feel like a process meeting. **[Docs →](docs/readme-reference.md#organic-driven-development)** @@ -156,9 +154,7 @@ Model, effort, and who does what should be choices, not accidents. Named profile ### Command palette — Every command, one keystroke away -Command palette with a search field and grouped entries: Configuration, Session, Diagnostics, SDD, and Skills - -Extension commands are only useful if you can find them. `alt+k` opens a curated, grouped palette — Configuration, Session, Diagnostics, SDD, and Skills — searchable by label, command name, or description, showing entries only when they are actually registered. +Extension commands are only useful if you can find them. `alt+k` opens a curated, grouped palette — Configuration, Session, Diagnostics, and Skills — searchable by label, command name, or description, showing entries only when they are actually registered. **[Docs →](docs/gentle-shell.md#command-palette)** @@ -278,13 +274,12 @@ Start with the product-facing destination, then move into the operational refere | Destination | Purpose | | --- | --- | | [gentle-shell reference](docs/gentle-shell.md) | Workspace layout, changes, usage, agents, and todo interactions. | -| [ODD workflow](docs/readme-reference.md#organic-driven-development) · [Technical reference](docs/readme-reference.md) | Everyday work and recovery, optional SDD/OpenSpec, installation, configuration, commands, and contributor detail. | +| [ODD workflow](docs/readme-reference.md#organic-driven-development) · [Technical reference](docs/readme-reference.md) | Everyday work and recovery, installation, configuration, commands, and contributor detail. | | [Review integration](docs/review-integration.md) | The provider/consumer boundary for native review. | | [Native authority architecture](docs/native-authority-architecture.md) | Ownership boundaries and review architecture. | | [Telemetry](docs/telemetry.md) | Approved fields and source limitations. | | [Delegated verification](docs/delegated-verification.md) | Practical verification guidance. | | [Skill style guide](docs/skill-style-guide.md) | The package skill contract. | -| [Prompt history](docs/prompt-history.md) | Opt-in capture switch, storage layout and readability, and disable/removal semantics. |

Back to top ↑

From 6786efdb8fd99e66dd88e3f196178f9d06ec1e73 Mon Sep 17 00:00:00 2001 From: Carolina <26188349+carolitascl@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:28:12 -0300 Subject: [PATCH 14/14] chore(ci): re-trigger checks review-repository-windows failed with CandidateViewError "candidate view owner preparation failed (ETIMEDOUT)" during worktree preparation, while test/verify/session-transport all passed. No code change; re-running the checks via an empty commit because workflow rerun requires upstream admin rights.