diff --git a/docs/prompt-history.md b/docs/prompt-history.md index 4a4262c40..ee37b4b06 100644 --- a/docs/prompt-history.md +++ b/docs/prompt-history.md @@ -1,13 +1,13 @@ # Prompt history -Prompt history stores captured prompts per pi instance and can import older -history and project session transcripts. Deletion and compaction arrive in -later slices of the chain. +Prompt history stores captured prompts per pi instance, can import older +history and project session transcripts, and lets you delete prompts from the +history selector. Compaction arrives in a later slice 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: +Recording is **off by default**. Delivered prompts can contain secrets, so +nothing is stored unless you explicitly opt in: ```bash GENTLE_PI_HISTORY_CAPTURE=1 pi @@ -18,13 +18,15 @@ GENTLE_PI_HISTORY_CAPTURE=1 pi - 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. + prompts are never written. The history selector only warns; it reads, + imports, and deletes nothing. ## Legacy migration and seeding are opt-in -Importing past prompts is part of capture: the first delivered prompt in an -opted-in session attempts legacy migration and one-time bootstrap from project -session transcripts. The selector reads the store but does not initiate import. +Importing past prompts is part of capture: an opted-in session attempts legacy +migration and one-time bootstrap from project session transcripts shortly +after the extension loads, or at its first delivered prompt if that comes +first. The selector reads the store but does not initiate import. With capture off, both capture and the selector leave the store untouched. Failed migration reads can be retried on a later session; untrusted deletion records defer transcript bootstrap until they can be read safely. @@ -44,6 +46,7 @@ Everything sits under `~/.pi/agent/history/`: process. - `projects//seed.jsonl` — one-time transcript import for this project. - `history-global.jsonl` — imported legacy editor-history prompts. +- `hidden.json` — deletion records (tombstones); see "Delete" below. `` 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: @@ -67,11 +70,80 @@ 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): +already written — and the registry entry — stay on disk until you remove them. +Individual prompts can be deleted from the history selector while capture is +on (see "Delete" below); the store directory itself is removed by hand: ```bash rm -rf ~/.pi/agent/history # whole store rm -rf ~/.pi/agent/history/projects/ # one project (see registry.json) ``` + +## Delete + +The selector's delete key (`ctrl+shift+backspace`) is a two-step y/n +confirmation: + +1. The first press **arms** the delete for the selected row: the footer + shows "Delete this prompt from history (y/n)? Prompt stays in session + log" and the row highlights in red. +2. While armed, the next key decides: `y` executes the delete, `n` or + `Esc` cancels, and any other key is ignored — nothing is typed into the + search box and the overlay stays open. + +A delete removes the prompt by its identity: whitespace runs collapsed, +leading and trailing whitespace trimmed, letter case ignored. Only that exact +prompt is affected — prompts that merely share a beginning stay. + +1. **Store copies are removed.** In the project scope, every copy in the + current project's files (`.jsonl` and `seed.jsonl`) is removed; + in the global scope, every copy in every project's files and in + `history-global.jsonl`. Each affected file is rewritten atomically (temp + file + rename). Files are never removed, even when they end up empty. + Lines that another pi instance appends while a file is being rewritten + are carried over into the new file. +2. **A tombstone is written** to `hidden.json`, so the prompt stays hidden + everywhere the selector reads, and a later transcript bootstrap does not + import it again. The session transcripts themselves are never modified. + +Deletes only run from the selector, so they need capture enabled. + +### What `hidden.json` contains + +`hidden.json` is a JSON array of strings, oldest first. Each deletion adds +`sha256:` followed by the SHA-256 hex digest of the normalized prompt, so the +file does not hold the text of deleted prompts. Plain-text entries written by +earlier builds (a prompt's first 120 normalized characters) are still read +and keep their original meaning: an entry shorter than 120 characters hides +that exact prompt, and a 120-character entry hides every prompt that begins +with it. They are kept as they are, and new deletions never add them. + +A tombstone hides every copy of its prompt, including one you type again +later: that prompt is captured, but it stays hidden while the tombstone +exists. + +The file is a bounded cache, not a retention guarantee: it holds at most +**1000 entries** in recency order, and deleting the same prompt again moves +its entry to the end. Past the cap, the oldest entry is dropped. Its prompt +can reappear if a copy is still on disk (for example, in a file that could +not be rewritten), and can be deleted again. + +### Failures + +Failures surface an error notification and never report a clean delete: + +- If the store delete fails before touching any file, nothing is removed and + no tombstone is written ("Store delete failed; nothing was removed."). +- If some files cannot be read or rewritten, the others are still cleaned, + temp files are removed, and the tombstone is still written ("Some history + files could not be rewritten; the prompt is hidden, but copies may remain + on disk."). +- If the tombstone write fails after store copies were removed, the prompt + may reappear from session transcripts ("Deleted from the store, but + hiding failed — the prompt may reappear from session transcripts."). + +`hidden.json` fails closed: if it exists but cannot be trusted (unreadable, +corrupt, or not an array), history is blocked with a recovery warning +instead of resurfacing hidden prompts, transcript bootstrap waits, and +deletes refuse to rewrite it. Recovery is explicit — restore the file or +delete it yourself (hidden prompts may then reappear). diff --git a/extensions/history/hide-prompts.ts b/extensions/history/hide-prompts.ts index 6d91a57d6..e948e6d22 100644 --- a/extensions/history/hide-prompts.ts +++ b/extensions/history/hide-prompts.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history // SPDX-License-Identifier: MIT +import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import { writeJsonAtomic } from "./atomic-write.ts"; @@ -9,6 +10,50 @@ import { promptDedupKey } from "./selector-helpers.ts"; /** Name of the tombstone file inside the injected state dir (spec C4). */ const HIDE_FILE_NAME = "hidden.json"; +/** Marks tombstone entries written in the exact (hashed) key format. */ +const TOMBSTONE_KEY_PREFIX = "sha256:"; + +/** + * UI-level prompt identity: whitespace-collapsed, trimmed, case-insensitive, + * never truncated. The store's scope deletes sweep by this same identity, + * so a tombstone hides exactly the copies a delete removes. + */ +export function promptIdentity(text: string): string { + return text.replace(/\s+/g, " ").trim().toLowerCase(); +} + +/** + * Tombstone key for `text`: a SHA-256 of its full prompt identity. Exact — + * two prompts that merely share a prefix get different keys — and hashed, + * so hidden.json never holds the text of a deleted prompt. + */ +export function tombstoneKey(text: string): string { + return ( + TOMBSTONE_KEY_PREFIX + + createHash("sha256").update(promptIdentity(text)).digest("hex") + ); +} + +/** + * Whether `text` is hidden by the tombstone set `keys`. Hashed keys match + * the exact prompt identity. Plaintext entries from the earlier prefix + * format (`promptDedupKey`: the first 120 normalized characters) stay + * honored as written, so upgrading never resurfaces a hidden prompt; only + * new deletions use the exact format. + */ +export function isPromptHidden(keys: ReadonlySet, text: string): boolean { + if (keys.size === 0) return false; + return keys.has(tombstoneKey(text)) || keys.has(promptDedupKey(text)); +} + +/** + * Retention cap for hidden.json (slice-05 D5): the tombstone file is a + * rebuildable derived cache, not a retention guarantee, so it holds at + * most this many keys in recency order; hiding past the cap drops the + * OLDEST keys from the front. + */ +export const HIDE_FILE_MAX_ENTRIES = 1000; + /** * Shared recovery warning for a file that exists but cannot be trusted * (spec C4, fail-closed READ half): toast-suitable, names hidden.json, and @@ -48,8 +93,11 @@ export type HiddenRead = * 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. + * trusted; junk items inside it are ignored, never trusted. Keys are the + * `tombstoneKey` hashes written by `hidePrompt`, or plaintext entries from + * the earlier prefix format (see `isPromptHidden`); the call never throws. + * A valid array's stored order is preserved (the recency order — oldest + * first — that `hidePrompt` maintains and caps). */ export function readHiddenPrompts(stateDir: string): HiddenRead { let raw: string; @@ -90,14 +138,21 @@ export function readHiddenPrompts(stateDir: string): HiddenRead { /** * 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. 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. + * `tombstoneKey` (byte-match normative with the drain and seed filters via + * `isPromptHidden` — never a re-implementation). The file array is RECENCY-ordered — oldest key + * first, newest key appended last — and re-hiding an existing key + * refreshes it to the end (delete + add, since Set.add on a present + * member keeps its old position). The file is capped at + * `HIDE_FILE_MAX_ENTRIES` (1000): after the append, keys drop from the + * FRONT until the file fits, so hidden.json stays a bounded cache — a + * dropped (oldest) prompt may reappear in the list and can be deleted + * again. Keys persist in that insertion order — NO sort — 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 read = readHiddenPrompts(stateDir); @@ -105,10 +160,19 @@ export function hidePrompt(stateDir: string, text: string): HideResult { // Refuse without writing: never reset the untrusted state silently. return { status: "error", message: read.message }; } - read.keys.add(promptDedupKey(text)); + // Recency order (slice-05 D5): the set iterates in stored file order + // (oldest first); delete+add refreshes a re-hidden key to the END. + const key = tombstoneKey(text); + read.keys.delete(key); + read.keys.add(key); + // Cap: drop the OLDEST keys from the front once over the limit. + const ordered = [...read.keys]; + if (ordered.length > HIDE_FILE_MAX_ENTRIES) { + ordered.splice(0, ordered.length - HIDE_FILE_MAX_ENTRIES); + } const written = writeJsonAtomic( path.join(stateDir, HIDE_FILE_NAME), - [...read.keys].sort(), + ordered, ); return written ? { status: "written" } diff --git a/extensions/history/index.ts b/extensions/history/index.ts index f26971e4b..21d41e75e 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -1,20 +1,20 @@ // SPDX-FileCopyrightText: 2026 ExoPro. Inspired by @jasonish/pi-prompt-history // SPDX-License-Identifier: MIT -// Prompt-history extension entry (slice 3, stage 3): the selector open flow -// over the slice-1 writer and slice-2 drains, with the search input -// (filterPrompts + forwardToSearch fallthrough), the lazy loaded window -// (initial batch, prefetch growth, PgUp/PgDn, Home/End), the header loaded -// segment, the project<->global scope toggle under the expanded-globals -// contract, and the preview panel + wheel handling over the fixed 30-row -// overlay geometry. Deletion (slice 5) and GC (slice 6) arrive later. +// Prompt-history extension entry: the selector open flow over the slice-1 +// writer and slice-2 drains, with the search input (filterPrompts + +// forwardToSearch fallthrough), the lazy loaded window, the project<->global +// scope toggle, the preview panel + wheel handling, the slice-4 import +// (legacy migration + seed bootstrap inside getWriter), and the slice-5 +// modal delete (store sweep + exact tombstone). GC/compaction arrives in a +// later slice. // -// Capture is OPT-IN while the deletion/privacy behavior is unshipped: -// nothing is recorded unless GENTLE_PI_HISTORY_CAPTURE=1|true|on. The -// selector honors the same gate: with the switch off, opening the selector -// is a no-op — no registry entry, no writer init, no store reads. -// Unsetting the switch only stops NEW captures; files already written stay -// on disk (docs/prompt-history.md). +// Capture is OPT-IN: nothing is recorded unless +// GENTLE_PI_HISTORY_CAPTURE=1|true|on. The selector honors the same gate: +// with the switch off, opening the selector is a no-op — no registry entry, +// no writer init, no store reads, no deletes. 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"; @@ -36,9 +36,12 @@ import { type TuiMouseEvent, truncateToWidth, } from "@earendil-works/pi-tui"; +import { hidePrompt } from "./hide-prompts.ts"; import { appendSessionCapture, bootstrapProjectSeed, + deleteFromGlobal, + deleteFromProject, type DrainResult, drainGlobal, drainProject, @@ -46,21 +49,29 @@ import { migrateLegacyStores, openSessionWriter, type SessionWriterState, + type SweepResult, } from "./store.ts"; import { buildPromptRecords, clampPreviewOffset, clampSelectedIndex, dedupePromptEntries, + deleteConfirmFooterText, + deleteConfirmStep, + deletionActionsFor, + EDITOR_HIDE_FAILED_TEXT, filterPrompts, getVisiblePromptRecords, initialLoadedCount, + loadedCountAfterDelete, loadedCountForQuery, loadedCountForTarget, moveSelectedIndex, nextLoadedCount, pageSelectedIndex, shouldGrowWindow, + STORE_DELETE_FAILED_TEXT, + storeDeleteFollowUp, withExpandedHistoryGlobals, type PiHistoryGlobals, type PromptEntry, @@ -85,13 +96,24 @@ const LIST_WHEEL_Y_FIRST = 5; const LIST_WHEEL_Y_LAST = 14; const PREVIEW_WHEEL_Y_FIRST = 17; const PREVIEW_WHEEL_Y_LAST = 26; + +// Default selector footer line (PR #1393): shown whenever a delete is not +// armed; the armed state swaps it for the confirmation copy. +const SELECTOR_FOOTER_HELP = + "↑↓ move • PgUp/PgDn page • tab scope • enter select and quit • ctrl+shift+↑/↓ preview • ctrl+shift+backspace delete • esc cancel"; + /** Width of the "→ " / " " prefix on each entry line. */ const ENTRY_PREFIX_WIDTH = 2; -// v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). +// Legacy agent dir: pre-v1 editor-history files live directly here and are +// migrated into the store root by migrateLegacyStores(). const AGENT_DIR = join(homedir(), ".pi", "agent"); +// v2 multi-concurrency store root (design: tmp/multi-concurrency-design.md). +// It is also the tombstone state dir: /hidden.json. const PI_HISTORY_ROOT = join(AGENT_DIR, "history"); -const SESSIONS_ROOT = join(homedir(), ".pi", "agent", "sessions"); +// Sessions root for the one-level transcript scan (spec C1, design §D5). +// Read-only by invariant — transcripts are never written by this extension. +const SESSIONS_ROOT = join(AGENT_DIR, "sessions"); export interface HistoryDeps { env?: NodeJS.ProcessEnv; @@ -107,7 +129,7 @@ export interface HistoryDeps { * 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. + * are left untouched (deletes run from the selector while capture is on). */ export function captureEnabled(env: NodeJS.ProcessEnv = process.env): boolean { const value = env.GENTLE_PI_HISTORY_CAPTURE?.trim().toLowerCase(); @@ -115,7 +137,7 @@ export function captureEnabled(env: NodeJS.ProcessEnv = process.env): boolean { } // --------------------------------------------------------------------------- -// Sanitization (a22588fc) +// Sanitization // --------------------------------------------------------------------------- /** @@ -147,7 +169,7 @@ function sanitizeForDisplay(text: string): string { } // --------------------------------------------------------------------------- -// TUI Selector (stage 3: search + lazy list + scope toggle + preview + wheel) +// Types // --------------------------------------------------------------------------- /** Keybinding lookup returned by getKeybindings(). */ @@ -164,10 +186,19 @@ interface DispatchEntry { } /** Notification sink for selector feedback; an absent callback drops notifications. */ -type SelectorNotify = ( - message: string, - level: "error" | "warning" | "info", -) => void; +type SelectorNotify = (message: string, level: "error" | "warning" | "info") => void; + +/** + * Store access the open flow hands the selector (per-load deps root/cwd): + * the selector never resolves store paths itself, so a bare constructor + * (tests, tooling) cannot touch any store. `root` is also the tombstone + * state dir (hidden.json); `drain` is the fail-closed scope drain. + */ +interface SelectorStore { + root: string; + cwd: string; + drain: (scope: HistoryScope) => DrainResult; +} /** Single rendered row; always occupies exactly one terminal row. */ class FixedRowText { @@ -205,8 +236,8 @@ 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. - // Measure the VISIBLE width: SGR escape sequences (colored rows) - // occupy no terminal cells. + // 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))]; } @@ -241,12 +272,17 @@ function wordWrapText(text: string, maxWidth: number): string[] { 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 readonly footerRow: FixedRowText; private records: PromptRecord[]; private readonly theme: Theme; private readonly tui: TUI; @@ -254,12 +290,8 @@ class PromptHistorySelector extends Container implements Focusable { private readonly onCancel: () => void; /** Notification sink for selector feedback (wired by the factory). */ private readonly onNotify?: SelectorNotify; - /** - * Scope drain injectable (slice-02 DrainResult contract): the open flow - * hands the selector its drainForScope so tab can re-drain the other - * scope without the selector touching store paths itself. - */ - private readonly drainScope: (scope: HistoryScope) => DrainResult; + /** Injected store access; null disables scope drains and deletes. */ + private readonly store: SelectorStore | null; private filteredRecords: PromptRecord[] = []; private selectedIndex = 0; /** Number of records loaded (newest-first) from the top of `records`. */ @@ -272,9 +304,15 @@ class PromptHistorySelector extends Container implements Focusable { private wrappedPreviewLines: string[] = []; /** Scroll offset into wrappedPreviewLines for the preview viewport. */ private previewScrollOffset = 0; + /** + * Modal delete confirmation (PR #1393 follow-up): armed by the first + * ctrl+shift+backspace press; while armed, y executes, n/Esc cancels, + * and every other key is swallowed. Nothing is deleted on the arming + * press. + */ + private confirmArmed = false; - /** Dispatch table: first match wins, fallthrough last. The - * ctrl+shift+backspace delete entry joins with deletion (slice 5). */ + /** Dispatch table: first match wins, fallthrough last. */ private readonly dispatch: readonly DispatchEntry[] = [ { match: (_d, kb) => kb.matches(_d, "tui.select.up"), @@ -309,6 +347,10 @@ class PromptHistorySelector extends Container implements Focusable { match: (d, _kb) => matchesKey(d, "end"), handler: () => this.jumpToLast(), }, + { + match: (d, _kb) => matchesKey(d, "ctrl+shift+backspace"), + handler: () => this.deleteCurrent(), + }, { match: (d, _kb) => matchesKey(d, "ctrl+shift+up"), handler: () => this.previewPageUp(), @@ -335,7 +377,7 @@ class PromptHistorySelector extends Container implements Focusable { onSelect: (record: PromptRecord) => void, onCancel: () => void, onNotify?: SelectorNotify, - drainScope?: (scope: HistoryScope) => DrainResult, + store?: SelectorStore, ) { super(); this.tui = tui; @@ -345,10 +387,7 @@ class PromptHistorySelector extends Container implements Focusable { this.onSelect = onSelect; this.onCancel = onCancel; this.onNotify = onNotify; - // Default injectable: an empty drain so a bare constructor (tests, - // tooling) never touches the store; the open flow always passes the - // real fail-closed drainForScope. - this.drainScope = drainScope ?? (() => ({ status: "ok", prompts: [] })); + this.store = store ?? null; // ── Search panel (top) ── this.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); @@ -358,10 +397,7 @@ class PromptHistorySelector extends Container implements Focusable { this.addChild(this.headerRow); this.addChild( new Text( - theme.fg( - "dim", - "Type to filter (multi-word AND substring, case-insensitive)", - ), + theme.fg("dim", "Type to filter (multi-word AND substring, case-insensitive)"), 0, 0, ), @@ -385,15 +421,11 @@ class PromptHistorySelector extends Container implements Focusable { 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 • esc cancel", - ), - true /* centered */, - ), + this.footerRow = new FixedRowText( + theme.fg("dim", SELECTOR_FOOTER_HELP), + true /* centered */, ); + this.addChild(this.footerRow); this.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); this.applyFilter(""); @@ -477,7 +509,13 @@ class PromptHistorySelector extends Container implements Focusable { for (const { record, isSelected } of visible) { const prefix = isSelected ? "→ " : " "; - const color = isSelected ? "accent" : "text"; + // Armed delete (PR #1393): the armed row repaints in the error color + // while the confirmation is pending, then reverts on disarm. + const color = isSelected + ? this.confirmArmed + ? "error" + : "accent" + : "text"; const compacted = sanitizeForDisplay(record.text) .replace(/\s+/g, " ") .trim(); @@ -569,7 +607,10 @@ class PromptHistorySelector extends Container implements Focusable { private toggleScope(): void { const previous = this.scope; this.scope = this.scope === "project" ? "global" : "project"; - const drained = this.drainScope(this.scope); + const drained: DrainResult = this.store?.drain(this.scope) ?? { + status: "ok", + prompts: [], + }; if (drained.status === "blocked") { this.scope = previous; this.onNotify?.(drained.message, "error"); @@ -580,6 +621,121 @@ class PromptHistorySelector extends Container implements Focusable { this.applyFilter(this.searchInput.getValue()); } + /** + * Delete-combo entry (slice-05 D3): the FIRST press arms the modal + * confirm for the selected row; while armed, the modal router in + * handleInput calls executeDelete() on `y`. Session-derived rows are + * read-only (slice-05 D1): a delete press on one is a silent no-op. + */ + private deleteCurrent(): void { + const selected = this.filteredRecords[this.selectedIndex]; + if (!selected) return; + + // Session rows are read-only: session transcripts are immutable and + // owned by Pi core — the extension never deletes from or writes to + // them. Silent no-op: no arm, no footer change, no tombstone. + if ((selected.source ?? "editor") === "session") return; + + if (!this.confirmArmed) { + this.armDelete(); + return; + } + this.executeDelete(); + } + + /** Arm the confirm: footer copy + error-colored row, nothing executes. */ + private armDelete(): void { + this.confirmArmed = true; + this.refreshDeleteFooter(); + this.rebuildList(); // repaint the armed-row highlight + } + + /** The executing half of the delete: leave the armed state, then mutate. */ + private executeDelete(): void { + // Leave the armed state first: help footer back, highlight dropped. + this.confirmArmed = false; + this.refreshDeleteFooter(); + this.rebuildList(); // drop the highlight before the flow mutates rows + + const selected = this.filteredRecords[this.selectedIndex]; + if (!selected || !this.store) return; + const { root, cwd } = this.store; + + // C4 delete flows (design §F): the record's provenance decides the + // actions via the pure planner over the injected store root/cwd. + const actions = deletionActionsFor(selected.source ?? "editor"); + + if (actions.deleteFromEditorStore) { + // Store path: physically remove EVERY copy from the JSONL store, one + // atomic rewrite per file. A thrown store failure is contained here + // (PR #1393): toast + abort — nothing was removed and no tombstone is + // written, so the delete never lies about state. + let sweep: SweepResult; + try { + sweep = + this.scope === "global" + ? deleteFromGlobal(root, selected.text) + : deleteFromProject(root, cwd, selected.text); + } catch { + this.onNotify?.(STORE_DELETE_FAILED_TEXT, "error"); + return; + } + // Files that could not be read or rewritten may still hold a copy: + // say so, and still write the tombstone that hides them. + const followUp = storeDeleteFollowUp(sweep); + if (followUp.notice) this.onNotify?.(followUp.notice, "error"); + if (!followUp.proceed) return; + } + + // Tombstone ALWAYS: the session transcripts are immutable and would + // re-supply the deleted prompt on the next merge (hide-file suppresses + // the twin). Only the session path aborts on a hide error — the store + // row is already gone on the editor path, so the splice proceeds; its + // toast says exactly that (PR #1393). + const hide = hidePrompt(root, selected.text); + if (hide.status === "error") { + if (!actions.deleteFromEditorStore) { + this.onNotify?.(hide.message, "error"); + return; + } + this.onNotify?.(EDITOR_HIDE_FAILED_TEXT, "error"); + } + // Remove from the master records array so a subsequent filter doesn't + // bring it back. + const idx = this.records.indexOf(selected); + if (idx !== -1) { + this.records.splice(idx, 1); + // C4 delete backfill (design §B3): shrink the window with the splice, + // then pull the next unloaded row while any remain — genuine shrink + // only at exhaustion. + this.loadedCount = loadedCountAfterDelete( + this.loadedCount, + this.records.length, + ); + } + + // Re-apply current filter (rebuilds filteredRecords, list, preview). + this.applyFilter(this.searchInput.getValue()); + } + + /** Footer line: confirm copy while armed, help otherwise. */ + private refreshDeleteFooter(): void { + if (!this.confirmArmed) { + this.footerRow.setText(this.theme.fg("dim", SELECTOR_FOOTER_HELP)); + return; + } + this.footerRow.setText( + this.theme.fg("warning", deleteConfirmFooterText()), + ); + } + + /** Leave the armed state: restore the help footer and the plain row. */ + private disarmDeleteConfirm(): void { + this.confirmArmed = false; + this.refreshDeleteFooter(); + this.rebuildList(); + } + // -- Navigation --------------------------------------------------------- private moveUp(): void { @@ -722,6 +878,23 @@ class PromptHistorySelector extends Container implements Focusable { } handleInput(data: string): void { + // Modal armed confirm (slice-05 D3): while a delete is armed the pure + // router consumes EVERY key — y executes, n/Esc cancels (esc must NOT + // close the overlay here), anything else stays armed and is swallowed + // — so no key reaches the dispatch table or the search input. When not + // armed, behavior is unchanged. + if (this.confirmArmed) { + const step = deleteConfirmStep( + this.confirmArmed, + matchesKey(data, "ctrl+shift+backspace"), + matchesKey(data, "escape"), + data, + ); + if (step.execute) this.executeDelete(); + else if (step.cancel) this.disarmDeleteConfirm(); + this.tui.requestRender(); + return; + } const kb = getKeybindings(); let handled = false; for (const { match, handler } of this.dispatch) { @@ -748,6 +921,9 @@ class PromptHistorySelector extends Container implements Focusable { event: TuiMouseEvent, ): ReturnType { if (event.type !== "wheel") return undefined; + // A wheel scroll can move the selection off the armed row — disarm so + // the next delete press re-arms for the NEW row first (PR #1393). + if (this.confirmArmed) this.disarmDeleteConfirm(); 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); @@ -836,7 +1012,7 @@ let activeOverlayClose: (() => void) | null = null; function createPromptHistorySelectorFactory( records: PromptRecord[], onNotify?: SelectorNotify, - drainScope?: (scope: HistoryScope) => DrainResult, + store?: SelectorStore, ): SelectorFactory { return (tui, theme, _keybindings, done) => { selectorTui = tui as { requestRender(): void }; @@ -854,7 +1030,7 @@ function createPromptHistorySelectorFactory( (record) => finish(record), () => finish(null), onNotify, - drainScope, + store, ); }; } @@ -862,7 +1038,7 @@ function createPromptHistorySelectorFactory( async function runPromptHistorySelection( ctx: Pick, records: PromptRecord[], - drainScope?: (scope: HistoryScope) => DrainResult, + store?: SelectorStore, ): Promise { const historyGlobals: PiHistoryGlobals = globalThis as Record< string, @@ -873,7 +1049,7 @@ async function runPromptHistorySelection( createPromptHistorySelectorFactory( records, (message, level) => ctx.ui.notify(message, level), - drainScope, + store, ), { overlay: true, @@ -898,10 +1074,10 @@ type HistoryScope = "project" | "global"; /** * Per-load open flow over the slice-1 deps (env/root/cwd): the selector is - * a pure store reader, so it never initializes the capture writer — the - * capture gate in openHistorySelector runs before any store access and a - * capture-off session performs no registry/writer side effects on the - * open path. + * a pure store reader plus the explicit delete action, so it never + * initializes the capture writer — the capture gate in openHistorySelector + * runs before any store access and a capture-off session performs no + * registry/writer/delete side effects on the open path. */ function createOpenFlow(env: NodeJS.ProcessEnv, root: string, cwd: string) { /** @@ -918,6 +1094,8 @@ function createOpenFlow(env: NodeJS.ProcessEnv, root: string, cwd: string) { : drainGlobal(root, 1000, root); } + const store: SelectorStore = { root, cwd, drain: drainForScope }; + async function openHistorySelector( ctx: Pick, ): Promise { @@ -948,11 +1126,7 @@ function createOpenFlow(env: NodeJS.ProcessEnv, root: string, cwd: string) { } const records = recordsFromEntries(entries); - const selected = await runPromptHistorySelection( - ctx, - records, - drainForScope, - ); + const selected = await runPromptHistorySelection(ctx, records, store); if (selected) { // pasteToEditor routes through the editor's input pipeline (bracketed // paste), so the text renders immediately (a22588fc). @@ -1011,6 +1185,18 @@ export default function promptHistoryExtension( return writerState; }; + // Warm migrate/registry/seed OFF the first-prompt path, but only for + // opted-in sessions: with capture disabled nothing may be written — + // no registry entry, no seed files, no store (docs/prompt-history.md). + setImmediate(() => { + if (!captureEnabled(env)) return; + try { + getWriter(); + } catch { + // init is best-effort; the lazy path retries on the next prompt + } + }); + // 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. diff --git a/extensions/history/selector-helpers.ts b/extensions/history/selector-helpers.ts index a50796cf2..7cb29ce78 100644 --- a/extensions/history/selector-helpers.ts +++ b/extensions/history/selector-helpers.ts @@ -25,13 +25,11 @@ export interface PromptEntry { ts?: number; } -/** Half-open window of list rows currently rendered (spec: centered cursor). */ export interface VisibleRange { start: number; end: number; } -/** One rendered list row: the record, its master index, and cursor state. */ export interface VisiblePromptRecord { index: number; record: PromptRecord; @@ -79,6 +77,41 @@ export function clampPreviewOffset( 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 @@ -120,73 +153,6 @@ export function dedupePromptEntries( return deduped; } -// --------------------------------------------------------------------------- -// Selector navigation & windowing (open-flow surface; search/paging helpers -// join in later stages) -// --------------------------------------------------------------------------- - -/** Wrapped cursor move: (+/-delta) with modulo wrap over the 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); -} - -/** - * Centered visible window (a22588fc shape): keep the cursor near the middle - * once the list outgrows maxVisible; small lists render in full. - */ -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), - }; -} - -/** The rows to render for the current cursor: sliced, indexed, cursor-flagged. */ -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, - })); -} - -// --------------------------------------------------------------------------- -// Lazy windowing (spec C1/C2, design §D3/§D4) and search visibility -// --------------------------------------------------------------------------- - /** * First-paint window size (spec C1, AC-L1-1): min(initialBatch, total), * floored at 0 — small stores open fully loaded (exhausted at open), @@ -249,6 +215,173 @@ export function loadedCountForTarget( return next; } +/** + * Delete backfill (spec C4's two steps verbatim, AC-L4-1..3): decrement the + * window against the splice-shrunk snapshot; while unloaded rows remain, + * backfill one row (clamped) so the next unloaded record slides into the + * deleted slot and the visible list length stays stable; at exhaustion the + * decrement is the genuine shrink. Written stepwise — NOT the algebraic + * min(L, T') shortcut — so the unit tests pin the contract, not an + * equivalence. Callers guarantee the deleted row sits inside the loaded + * prefix (idx < loadedCount by construction). + */ +export function loadedCountAfterDelete( + loadedCount: number, + totalCountAfterSplice: number, +): number { + const decrement = loadedCount - 1; + if (decrement < totalCountAfterSplice) { + return Math.min(decrement + 1, totalCountAfterSplice); + } + return decrement; +} + +/** + * Pure delete-flow planner (spec C4, design §F): maps a record's provenance + * to the two delete actions. "editor" deletes from the editor store on disk + * AND writes the tombstone (twin suppression — the session copy of the same + * text would otherwise resurface next open); "session" plans NOTHING — + * session-derived rows are read-only (slice-05 D1): transcripts are + * immutable and owned by Pi core, so the extension never deletes from or + * writes to them, and deleteCurrent guards the source before the flow. + * Takes source as a plain parameter (no member reads — the T23 provenance + * pin keeps overlay consumers source-agnostic outside deleteCurrent); the + * only consumer is the delete flow in history/index.ts. + */ +export function deletionActionsFor( + source: PromptSource, +): { deleteFromEditorStore: boolean; writeTombstone: boolean } { + if (source === "editor") { + return { deleteFromEditorStore: true, writeTombstone: true }; + } + return { deleteFromEditorStore: false, writeTombstone: false }; +} + +/** + * One transition of the modal delete confirmation (PR #1393 follow-up, + * slice-05 D3). Disarmed, only the delete combo matters: it ARMS the + * confirm and executes nothing. While armed the confirm is MODAL: `y`/`Y` + * executes, `n`/`N`/Esc cancels, and every other key — including a second + * press of the combo — is swallowed with the confirm still armed (nothing + * reaches the dispatch table or the search input). The TUI keybinding + * matches (ctrl+shift+backspace, escape) are computed by the caller via + * matchesKey and passed as plain booleans so this router stays pure and + * testable without the TUI; the y/n semantics read the raw data here. + * ONE definition: the selector's handleInput routes every armed-state key + * through this function. + */ +export interface DeleteConfirmStep { + /** The armed state AFTER this transition. */ + armed: boolean; + /** True only when `y`/`Y` confirms the armed delete — run the flow. */ + execute: boolean; + /** True when `n`/`N`/Esc cancels — disarm and resume normal input. */ + cancel: boolean; +} + +export function deleteConfirmStep( + armed: boolean, + isDeleteKey: boolean, + isEscapeKey: boolean, + data: string, +): DeleteConfirmStep { + if (!armed) { + return isDeleteKey + ? { armed: true, execute: false, cancel: false } + : { armed: false, execute: false, cancel: false }; + } + if (data === "y" || data === "Y") { + return { armed: false, execute: true, cancel: false }; + } + if (data === "n" || data === "N" || isEscapeKey) { + return { armed: false, execute: false, cancel: true }; + } + return { armed: true, execute: false, cancel: false }; +} + +/** + * Confirmation copy shown in the footer while a delete is armed (PR + * #1393): one line, one variant — a y/n question carrying the standing + * guarantee that the session log keeps the original either way. + */ +export function deleteConfirmFooterText(): string { + return "Delete this prompt from history (y/n)? Prompt stays in session log"; +} + +/** + * Toast copy when the store delete THROWS (PR #1393): the flow aborts + * before any tombstone write, so nothing was removed — the store keeps the + * prompt and no tombstone is written. + */ +export const STORE_DELETE_FAILED_TEXT = + "Store delete failed; nothing was removed."; + +/** + * Toast copy when the tombstone write fails on the EDITOR path (PR + * #1393): the store row was already removed, so only the hide failed — + * the prompt may reappear from session transcripts. + */ +export const EDITOR_HIDE_FAILED_TEXT = + "Deleted from the store, but hiding failed — the prompt may reappear from session transcripts."; + +/** + * Toast copy when some store files could not be read or rewritten: copies + * may remain on disk, and only the tombstone keeps them out of the list. + */ +export const STORE_DELETE_PARTIAL_TEXT = + "Some history files could not be rewritten; the prompt is hidden, but copies may remain on disk."; + +/** The counts a scope delete reports (structural twin of store's SweepResult). */ +export interface StoreSweepCounts { + filesAffected: number; + removed: number; + failed: number; +} + +/** + * What the delete flow does after the store sweep: proceed to the + * tombstone when anything was removed OR any file failed (a failed file + * may still hold a copy the tombstone must hide), surfacing an error + * notice for failures; stop quietly when there was nothing to delete. + */ +export function storeDeleteFollowUp( + counts: StoreSweepCounts, +): { proceed: boolean; notice?: string } { + if (counts.failed > 0) { + return { proceed: true, notice: STORE_DELETE_PARTIAL_TEXT }; + } + return { proceed: counts.removed > 0 }; +} + +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 @@ -280,15 +413,3 @@ export function filterPrompts( return filtered.slice(0, MAX_RESULTS); } - -export async function withExpandedHistoryGlobals( - globals: PiHistoryGlobals, - run: () => Promise, -): Promise { - globals.__piHistoryExpand?.(); - try { - return await run(); - } finally { - globals.__piHistoryTrim?.(); - } -} diff --git a/extensions/history/store.ts b/extensions/history/store.ts index fc608af58..0dd5b6331 100644 --- a/extensions/history/store.ts +++ b/extensions/history/store.ts @@ -5,14 +5,18 @@ // and identity, the advisory registry, entry primitives, the per-instance // session writer, the scope drain/reader/query section (ordering, dedup, // tombstone filter, project/global drains), legacy migration, and the -// project seed bootstrap. Scope deletes and GC/compaction arrive in later -// slices. Formerly store-paths.ts + registry.ts + multi-store.ts (+ v1 +// project seed bootstrap, and scope deletes (slice 5). GC/compaction +// arrives in a later slice. 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 { readHiddenPrompts } from "./hide-prompts.ts"; +import { + isPromptHidden, + promptIdentity, + readHiddenPrompts, +} from "./hide-prompts.ts"; import { extractPromptsFromFile, listSessionFiles, @@ -192,8 +196,8 @@ export function parseStoreLine(raw: string): StoreEntry | null { } // =========================================================================== -// Instance writer (formerly multi-store.ts; scope deletes and GC arrive -// in later slices) +// Instance writer (formerly multi-store.ts; GC/compaction arrives in a +// later slice) // =========================================================================== /** Mutable state of ONE pi instance's exclusive capture file. */ @@ -258,9 +262,7 @@ export function appendSessionCapture( // --------------------------------------------------------------------------- /** UI-level prompt identity: whitespace-collapsed, case-insensitive. */ -function promptKey(text: string): string { - return text.replace(/\s+/g, " ").trim().toLowerCase(); -} +const promptKey = promptIdentity; function fileMtimeMs(file: string): number { try { @@ -312,11 +314,6 @@ function fileSortKey(file: string, entries: StoreEntry[]): number { 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`. @@ -333,9 +330,7 @@ function drainFiles( 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; - } + if (isPromptHidden(hidden, entries[i].text)) continue; seen.add(key); out.push(entries[i].text); if (out.length >= limit) return out; @@ -441,6 +436,149 @@ export function drainGlobal( return drainWithHidden(sorted, limit, stateDir); } +// --------------------------------------------------------------------------- +// Scope delete (design v2) +// --------------------------------------------------------------------------- + +export interface SweepResult { + /** Files rewritten without the prompt. */ + filesAffected: number; + /** Lines removed across those files. */ + removed: number; + /** + * Files that could not be read or rewritten. They may still hold a copy, + * so callers must never report such a delete as clean. + */ + failed: number; +} + +const NEWLINE = 0x0a; + +/** Read every byte of `fd` from `position` to its current end. */ +function readFrom(fd: number, position: number): Buffer { + const chunks: Buffer[] = []; + const chunk = Buffer.alloc(64 * 1024); + for (;;) { + const read = fs.readSync(fd, chunk, 0, chunk.length, position); + if (read === 0) break; + chunks.push(Buffer.from(chunk.subarray(0, read))); + position += read; + } + return Buffer.concat(chunks); +} + +/** + * Rewrite ONE file without the lines whose prompt identity is `key`, via + * tmp + rename; returns the number of removed lines (0 leaves the file + * untouched). Other pi instances append to their own files by path at any + * moment, so only COMPLETE lines (up to the last newline) are filtered, and + * the descriptor kept open on the replaced inode supplies everything + * appended after the read — including a torn last line — which is carried + * over verbatim into the new file after the rename. Kept lines are copied + * byte-for-byte. On failure the tmp file is removed, the original stays in + * place unless the rename already happened, and the error is rethrown. + */ +function sweepFile(file: string, key: string): number { + const fd = fs.openSync(file, "r"); + let tmp: string | null = null; + try { + const snapshot = readFrom(fd, 0); + const complete = snapshot.lastIndexOf(NEWLINE) + 1; + const kept: string[] = []; + let removed = 0; + const lines = snapshot.subarray(0, complete).toString("utf8").split("\n"); + for (const lineText of lines) { + if (lineText.length === 0) continue; + const parsed = parseStoreLine(lineText); + if (parsed && promptKey(parsed.text) === key) { + removed += 1; + } else { + kept.push(lineText); + } + } + if (removed === 0) return 0; + tmp = `${file}.tmp-${process.pid}-${Date.now()}`; + fs.writeFileSync(tmp, kept.length > 0 ? kept.join("\n") + "\n" : "", "utf8"); + fs.renameSync(tmp, file); + tmp = null; + // Lines another instance appended to the replaced inode since the read. + const carried = readFrom(fd, complete); + if (carried.length > 0) fs.appendFileSync(file, carried); + return removed; + } catch (error) { + if (tmp !== null) { + try { + fs.unlinkSync(tmp); + } catch { + // best effort: the tmp name never matches a *.jsonl store file + } + } + throw error; + } finally { + fs.closeSync(fd); + } +} + +/** + * Remove every line whose prompt identity matches `text` from each file in + * `files`, one atomic rewrite per affected file (see sweepFile). Files whose + * every line matched are kept as empty files (never removed — the instance + * owning a session file may still append to it). A file that cannot be + * read or rewritten is counted in `failed` and the sweep moves on; a file + * that vanished before it could be opened holds nothing to delete. + */ +function sweepFiles(files: string[], text: string): SweepResult { + const key = promptKey(text); + const result: SweepResult = { filesAffected: 0, removed: 0, failed: 0 }; + for (const file of files) { + try { + const removed = sweepFile(file, key); + if (removed > 0) { + result.filesAffected += 1; + result.removed += removed; + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") continue; + result.failed += 1; + } + } + return result; +} + +/** Delete every copy of a prompt from the CURRENT project's scope. */ +export function deleteFromProject( + root: string, + cwd: string, + text: string, +): SweepResult { + return sweepFiles( + listProjectFiles(path.join(root, "projects", projectHash(cwd))), + text, + ); +} + +/** Delete every copy of a prompt from the GLOBAL scope (all projects + seed). */ +export function deleteFromGlobal(root: string, text: string): SweepResult { + const files: string[] = []; + const globalSeed = globalSeedPath(root); + if (fs.existsSync(globalSeed)) files.push(globalSeed); + 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)), + ); + } + return sweepFiles(files, text); +} + // --------------------------------------------------------------------------- // Legacy migration (design v2: one-time, gated) // --------------------------------------------------------------------------- @@ -637,7 +775,7 @@ export function bootstrapProjectSeed( for (let i = prompts.length - 1; i >= 0; i--) { const text = prompts[i].text; if (/^\/[A-Za-z]/.test(text.trim())) continue; - if (hidden.size > 0 && hidden.has(promptDedupKeyOf(text))) continue; + if (isPromptHidden(hidden, text)) continue; const key = promptKey(text); if (existingKeys.has(key)) continue; existingKeys.add(key); diff --git a/odd/tasks/history-contributor-chain.md b/odd/tasks/history-contributor-chain.md index 4718a6d37..361fb4458 100644 --- a/odd/tasks/history-contributor-chain.md +++ b/odd/tasks/history-contributor-chain.md @@ -11,7 +11,7 @@ Delegated direct writer for multi-file implementation and preparatory reading. O ## Tasks - [ ] H1: Reconcile #1391 against current main, fix migration/seed privacy and retry issues with deterministic tests; verify and merge only when eligible. Route: delegated, multiple nontrivial source/test files. Commit and merge identities pending. -- [ ] H2: Reconcile #1393 against post-H1 main, preserve `GENTLE_PI_HISTORY_CAPTURE`, ensure exact tombstones, race-safe deletion, and failure reporting; verify and merge only when eligible. Route: delegated, multiple nontrivial source/test files. Commit and merge identities pending. +- [ ] H2: Reconcile #1393 against post-H1 main, preserve `GENTLE_PI_HISTORY_CAPTURE`, ensure exact tombstones, race-safe deletion, and failure reporting; verify and merge only when eligible. Route: delegated, multiple nontrivial source/test files. Commit and merge identities pending. Merge resolution + fixes staged in `fix/history-pr1393-adaptation` (see Progress); commit pending. - [ ] H3: Reconcile #1394 against post-H2 main, preserve capture compatibility and seed gate, protect active writers during GC, and update truthful docs; verify and merge only when eligible. Route: delegated, multiple nontrivial source/test files. Commit and merge identities pending. ## Acceptance and checks @@ -20,5 +20,9 @@ Run focused `tests/history-*.test.ts`, project verification/typecheck, `git diff ## Progress 2026-09-26: Remote heads inspected: #1391 `649711c`, #1393 `e302337`, #1394 `5981153`; origin/main `06c9915` at the H1 snapshot. #1391 cumulative diff since #1455 is 1796 additions/23 deletions across 13 files; later cumulative PRs are larger. Separate #1391 worktree created at `fix/history-pr1391-adaptation`. H1 writer observed RED 2 migration failures, GREEN 14/14 focused tests. With node_modules linked from the main checkout, independent verification passed 168 history tests, typecheck had 188 baseline diagnostics and no regression, and `git diff --check` passed. Integration with main, commit, PR checks and merge remain pending. Engram mirror `odd/history-contributor-chain/tasks` pending: this session is bound to the separate gentle-ai project and Engram rejects a gentle-pi write. +2026-09-26 (H2, worktree `fix/history-pr1393-adaptation`, contributor head `e30233751` merging origin/main `1d128c981`): route delegated (writer; 2+ non-trivial source/test files). Conflicts in `docs/prompt-history.md` and `extensions/history/index.ts` resolved on main's #1391 structure: per-load `createOpenFlow(env, root, cwd)` (the selector never initializes the writer), injected `agentDir`/`sessionsRoot`, store root as tombstone state dir, `GENTLE_PI_HISTORY_CAPTURE` gate; the contributor's module-level `getWriter`/`drainForScope` (selector-triggered migration/seed writes, hard-wired real root, `GENTLE_PI_HISTORY_ENABLE`) was not kept. Deletion is wired through an injected `SelectorStore`; the gated `setImmediate` warm-up from #1393 stays. Fixes with strict TDD: (a) unbound/duplicated module constants resolved (single `AGENT_DIR`/`PI_HISTORY_ROOT`/`SESSIONS_ROOT`; no `CURRENT_CWD`/`INSTANCE_ID`/`PI_HISTORY_NAV_STATE_DIR`); (b) `sweepFile` filters only complete lines and carries over bytes appended to the replaced inode (via the still-open fd) after the rename; (c) per-file read/rewrite failures are counted in `SweepResult.failed`, tmp files are unlinked, and `storeDeleteFollowUp` surfaces `STORE_DELETE_PARTIAL_TEXT` while still writing the tombstone; (d) tombstones are `sha256:` hashes of the full normalized prompt (`tombstoneKey`/`isPromptHidden`), legacy plaintext 120-char entries stay honored. RED observed: sweep tests 11 fail (torn line lost, rename failure thrown, shape), prefix sibling hidden + plaintext in hidden.json (scratch script), index-importing tests failed on conflict markers/missing exports. GREEN: `node --experimental-strip-types --test tests/history-*.test.ts` 231/231 pass; `node scripts/check-types.mjs` 188 baseline, no regressions; `git diff --check` clean. Merge resolved and staged; no commit, push, or merge. + ## Next step +H2: parent reviews the staged merge resolution, commits it, and runs review/CI per policy. H1 note below is historical. + H1 is blocked: native review lineage `review-88445da92b015b5c` escalated with `targeted_validator_rejected` for R3-001/R3-002 after the single bounded correction. Do not publish or merge this candidate as approved. Diagnose the native refusal through supported maintainer inspection or make a separately authorized fresh candidate; keep H2/H3 pending. Last integrated commit `15249c57b`, correction commit `cc6ecca68`; independent recheck: 172 focused history tests pass, typecheck retains 188 baseline diagnostics without regressions, diff check passed. `pnpm test` aborted before tests because pnpm attempted a noninteractive `node_modules` purge; the equivalent direct unit stage ran 3,754 tests (3,707 pass, 4 fail, 43 skip), while direct provider-contract and runtime-harness stages passed. Three failures are presence-poll timeouts in `agents-view-thread-identity.test.ts`; one `gentle-shell.test.ts` border mismatch includes unexpected `INSERT`. Baseline attribution unverified. Native review is still escalated. No PR push or merge occurred. diff --git a/tests/history-command-registration.test.ts b/tests/history-command-registration.test.ts index 70a84bc25..c78f457b4 100644 --- a/tests/history-command-registration.test.ts +++ b/tests/history-command-registration.test.ts @@ -121,3 +121,35 @@ test("a blocked drain stops the open flow with an error and no records", () => { "the blocked recovery message surfaces as an error notification", ); }); + +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)", + ); +}); + +test("writer init is scheduled off the first-prompt path via setImmediate", () => { + const entry = source.indexOf("export default function promptHistoryExtension"); + assert.notStrictEqual(entry, -1, "extension entry point should exist"); + + const body = source.slice(entry); + assert.ok( + body.includes("setImmediate(() => {"), + "init must be scheduled with setImmediate so bootstrap never runs on\nthe first-prompt path", + ); + assert.ok( + /setImmediate\(\(\) => \{[\s\S]*?getWriter\(\);/.test(body), + "the scheduled callback should warm getWriter()", + ); + // The synchronous fallback stays: a prompt arriving before the + // scheduled call still initializes lazily inside the capture handler. + assert.ok( + /before_agent_start[\s\S]*?appendSessionCapture\(getWriter\(\)/.test(body), + "capture handler keeps the synchronous getWriter() fallback", + ); +}); diff --git a/tests/history-delete-backfill.test.ts b/tests/history-delete-backfill.test.ts new file mode 100644 index 000000000..db276bc28 --- /dev/null +++ b/tests/history-delete-backfill.test.ts @@ -0,0 +1,190 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { + deletionActionsFor, + loadedCountAfterDelete, +} from "../extensions/history/selector-helpers.ts"; + +// Unit 3 — L4 delete backfill (spec C4, design §B3). +// +// C4's contract as a verbatim two-step: a successful delete splices the +// master snapshot AND shrinks the loaded window together; while unloaded +// rows remain, the window backfills one row (clamped) so the next unloaded +// record slides into the deleted slot and the visible list length stays +// stable; at exhaustion (loadedCount == records.length after the decrement) +// there is NO backfill — the visible set genuinely shrinks by one row, by +// design. +// +// The deleted record always comes from filteredRecords ⊆ the loaded prefix, +// so idx < loadedCount by construction (§B3). Change 1's delete contract +// (delete-prompt.ts, error-only notify) is UNTOUCHED — AC-L4-4's regression +// pin is test/history/delete-prompt.test.ts itself, green and unmodified. + +// T11 — AC-L4-1 + AC-L4-2: mid-window delete with unloaded rows remaining — +// the count is preserved by pulling the next record: (30, 99) decrements to +// 29, 29 < 99, so backfill min(29 + 1, 99) = 30 (stable window). + +test("loadedCountAfterDelete backfills while unloaded rows remain — stable window (AC-L4-1, AC-L4-2)", () => { + assert.equal(loadedCountAfterDelete(30, 99), 30); +}); + +// T11 — AC-L4-3: exhaustion shrink — the window was fully loaded (100 of 100, +// 99 after the splice), so the decrement is the genuine shrink, no backfill: +// 99 < 99 is false → 99. + +test("loadedCountAfterDelete shrinks genuinely at exhaustion (AC-L4-3)", () => { + assert.equal(loadedCountAfterDelete(100, 99), 99); +}); + +// T11 — AC-L4-3 terminal case: deleting the last loaded row on an exhausted +// window bottoms out at 0: (1, 0) decrements to 0, 0 < 0 is false → 0. + +test("loadedCountAfterDelete bottoms out at 0 on the terminal delete (AC-L4-3)", () => { + assert.equal(loadedCountAfterDelete(1, 0), 0); +}); + +// T11 — defensive degenerate row: an empty window stays 0 even when counts +// disagree: (0, 5) decrements to −1, −1 < 5, so min(−1 + 1, 5) = 0. +// Unreachable via executeDelete (a delete implies a selected row inside the +// loaded prefix) — pinned as C4's defensive bound. + +test("loadedCountAfterDelete is defensive for an empty window (AC-L4-1)", () => { + assert.equal(loadedCountAfterDelete(0, 5), 0); +}); + +// T11 — AC-L4-1 + AC-L4-4 (source-parse): ordering shape inside executeDelete +// — the bookkeeping call sits strictly between the existing splice and the +// trailing applyFilter, INSIDE the existing `if (idx !== -1)` guarded block, +// and the non-`deleted` early return still precedes every mutation +// (Change 1 C1 interplay unchanged). + +const selectorSource = fs.readFileSync( + path.join(process.cwd(), "extensions", "history", "index.ts"), + "utf8", +); + +test("executeDelete splices, backfills, then re-filters — inside the guarded block (AC-L4-1, AC-L4-4)", () => { + const decl = selectorSource.indexOf("private executeDelete(): void {"); + assert.ok(decl >= 0, "executeDelete should exist"); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, "executeDelete's body should close"); + const body = selectorSource.slice(decl, end); + + // The early return now follows the sweep follow-up: nothing removed and + // nothing failed stops before any mutation (a failed file still hides). + const earlyReturnAt = body.indexOf("if (!followUp.proceed) return;"); + const spliceAt = body.indexOf("this.records.splice("); + const backfillAt = body.indexOf("loadedCountAfterDelete("); + const refilterAt = body.lastIndexOf("this.applyFilter("); + assert.ok(earlyReturnAt >= 0, "the Change 1 early return must stay"); + assert.ok(spliceAt >= 0, "the existing splice must stay"); + assert.ok( + backfillAt >= 0, + "the loadedCountAfterDelete bookkeeping call must exist", + ); + assert.ok(refilterAt >= 0, "the trailing applyFilter must stay"); + assert.ok( + earlyReturnAt < spliceAt && + spliceAt < backfillAt && + backfillAt < refilterAt, + "ordering must be: early return → splice → backfill → re-filter", + ); + + // Inside the guarded block: no 4-space block closer may appear between the + // `if (idx !== -1)` guard and the bookkeeping call (the block's own close + // sits only AFTER the call). + const guardAt = body.indexOf("if (idx !== -1)"); + assert.ok(guardAt >= 0, "the `if (idx !== -1)` guard must stay"); + const guardToCall = body.slice(guardAt, backfillAt); + assert.ok( + !guardToCall.includes("\n }"), + "the bookkeeping must sit inside the `if (idx !== -1)` block", + ); + + // The call assigns this.loadedCount from the unfiltered counts only. + assert.ok( + body.includes("this.loadedCount = loadedCountAfterDelete("), + "the call must assign this.loadedCount", + ); + const callRegion = body.slice(backfillAt, refilterAt); + assert.ok( + callRegion.includes("this.loadedCount") && + callRegion.includes("this.records.length"), + "the bookkeeping must read the unfiltered window and the shrunk snapshot", + ); +}); + +// Slice 5 scenario pins (porting contract): the editor-path tombstone rule +// and the partial-failure toast path. The dev suite pins the planner + +// these delete-flow branch shapes in delete-confirm.test.ts; this file +// carries the delete-flow source-parse half so the slice-5 branch stays +// pinned inside the delete slice's own tests. The mutation flow lives in +// executeDelete() (slice-05 D3 split), so the parse targets that method. + +test("deletionActionsFor plans a store delete + tombstone for editor rows and NOTHING for session rows", () => { + // Session/seed-born records are READ-ONLY (slice-05 D1): no store delete + // and no tombstone — deleteCurrent guards the source before the flow, so + // a transcript-born prompt is never written or deleted by this + // extension. + assert.deepEqual(deletionActionsFor("session"), { + deleteFromEditorStore: false, + writeTombstone: false, + }); + // Editor records: disk delete AND tombstone (twin suppression). + assert.deepEqual(deletionActionsFor("editor"), { + deleteFromEditorStore: true, + writeTombstone: true, + }); + + const decl = selectorSource.indexOf("private executeDelete(): void {"); + assert.ok(decl >= 0, "executeDelete should exist"); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, "executeDelete's body should close"); + const body = selectorSource.slice(decl, end); + + // Branch shape: the tombstone write follows (never sits inside) the + // editor-store guard — the executing path is editor-only, and its hide + // suppresses the session twin that would re-supply the prompt. + const editorGuardAt = body.indexOf("if (actions.deleteFromEditorStore)"); + assert.ok(editorGuardAt >= 0, "the editor-store guard must exist"); + const guardCloseAt = body.indexOf("\n }", editorGuardAt); + assert.ok(guardCloseAt > editorGuardAt, "the editor-store guard must close"); + const hideAt = body.indexOf("hidePrompt("); + assert.ok(hideAt >= 0, "the tombstone write must exist"); + assert.ok( + hideAt > guardCloseAt, + "the tombstone must follow (not sit inside) the editor-store guard", + ); +}); + +test("a failed hide toasts and only the session path aborts — the editor path still splices", () => { + const decl = selectorSource.indexOf("private executeDelete(): void {"); + assert.ok(decl >= 0, "executeDelete should exist"); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, "executeDelete's body should close"); + const body = selectorSource.slice(decl, end); + + const gateAt = body.indexOf('if (hide.status === "error")'); + assert.ok(gateAt >= 0, "hide errors must be gated"); + const spliceAt = body.indexOf("this.records.splice("); + assert.ok( + gateAt < spliceAt, + "the hide-error gate must precede the splice", + ); + const gate = body.slice(gateAt, spliceAt); + assert.ok( + gate.includes('this.onNotify?.(hide.message, "error")'), + "a hide error must toast", + ); + const abortGuardAt = gate.indexOf("if (!actions.deleteFromEditorStore)"); + assert.ok( + abortGuardAt >= 0, + "the early return must be exclusive to the session path", + ); + assert.ok( + !gate.slice(0, abortGuardAt).includes("return;"), + "no unconditional abort before the editor/session split — the editor path splices", + ); +}); diff --git a/tests/history-delete-confirm.test.ts b/tests/history-delete-confirm.test.ts new file mode 100644 index 000000000..115b1ef30 --- /dev/null +++ b/tests/history-delete-confirm.test.ts @@ -0,0 +1,386 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; +import { + deleteConfirmFooterText, + deleteConfirmStep, + deletionActionsFor, + EDITOR_HIDE_FAILED_TEXT, + STORE_DELETE_FAILED_TEXT, + STORE_DELETE_PARTIAL_TEXT, + storeDeleteFollowUp, +} from "../extensions/history/selector-helpers.ts"; + +// Slice-05 delete-confirm tests (PR #1393 follow-up): the delete +// confirmation is a MODAL y/n step. The first ctrl+shift+backspace press +// ARMS the delete for the selected row (confirmation footer + highlighted +// record) and executes NOTHING; while armed, y executes, n/Esc cancels, +// and every other key is swallowed with the confirm still armed — nothing +// reaches the dispatch table or the search input. Session-derived rows +// are read-only: a delete press on one is a silent no-op. +// +// PromptHistorySelector is private to extensions/history/index.ts and +// needs the pi-tui runtime graph (openflow-integration.test.ts +// discipline), so the confirm DECISION is factored into the pure +// deleteConfirmStep router tested here directly, and the wiring semantics are pinned by source-parse on +// deleteCurrent/armDelete/executeDelete/handleInput (delete-backfill +// discipline). No test in this file touches the user's real store. + +// --------------------------------------------------------------------------- +// Pure modal router: arm → y executes / n·Esc cancels / rest swallowed. +// --------------------------------------------------------------------------- + +const ARM = { armed: true, execute: false, cancel: false }; +const IDLE = { armed: false, execute: false, cancel: false }; +const EXECUTE = { armed: false, execute: true, cancel: false }; +const CANCEL = { armed: false, execute: false, cancel: true }; + +test("the first delete press arms only — nothing executes, nothing cancels", () => { + assert.deepEqual(deleteConfirmStep(false, true, false, ""), ARM); +}); + +test("an unarmed non-delete key is a no-op — the confirm stays out of the way", () => { + assert.deepEqual(deleteConfirmStep(false, false, false, "x"), IDLE); +}); + +test("while armed, y (and Y) executes the delete", () => { + assert.deepEqual(deleteConfirmStep(true, false, false, "y"), EXECUTE); + assert.deepEqual(deleteConfirmStep(true, false, false, "Y"), EXECUTE); +}); + +test("while armed, n / N / Esc cancel — the confirm disarms without executing", () => { + assert.deepEqual(deleteConfirmStep(true, false, false, "n"), CANCEL); + assert.deepEqual(deleteConfirmStep(true, false, false, "N"), CANCEL); + assert.deepEqual(deleteConfirmStep(true, false, true, "\x1b"), CANCEL); +}); + +test("while armed, any other key is swallowed and the confirm STAYS armed", () => { + // Plain typing, digits, empty data, arrow-key bytes, and a SECOND + // delete-combo press: none of them execute or cancel. + assert.deepEqual(deleteConfirmStep(true, false, false, "x"), ARM); + assert.deepEqual(deleteConfirmStep(true, false, false, "1"), ARM); + assert.deepEqual(deleteConfirmStep(true, false, false, ""), ARM); + assert.deepEqual(deleteConfirmStep(true, false, false, "\x1b[A"), ARM); + assert.deepEqual(deleteConfirmStep(true, true, false, "\x1b[27;6~"), ARM); +}); + +test("full machine: arm → y executes; a fresh arm is needed per delete", () => { + const armed = deleteConfirmStep(false, true, false, ""); + assert.equal(armed.armed, true); + assert.equal(armed.execute, false); + const done = deleteConfirmStep(armed.armed, false, false, "y"); + assert.equal(done.execute, true); + assert.equal(done.armed, false, "executing leaves the confirm disarmed"); + // After execution the confirm is idle: typing resumes as usual. + assert.deepEqual(deleteConfirmStep(done.armed, false, false, "x"), IDLE); +}); + +test("full machine: arm → n cancels → disarmed without executing", () => { + const armed = deleteConfirmStep(false, true, false, ""); + const cancelled = deleteConfirmStep(armed.armed, false, true, "\x1b"); + assert.equal(cancelled.cancel, true); + assert.equal(cancelled.armed, false); + assert.equal(cancelled.execute, false); +}); + +// The executing press composes with the pure planner: an editor-source +// record deletes from the store AND tombstones; a session-source record is +// read-only — the planner plans NOTHING for it (slice-05 D1). + +test("y on an editor row runs the store-delete + tombstone plan", () => { + const armed = deleteConfirmStep(false, true, false, ""); + const step = deleteConfirmStep(armed.armed, false, false, "y"); + assert.equal(step.execute, true); + assert.deepEqual(deletionActionsFor("editor"), { + deleteFromEditorStore: true, + writeTombstone: true, + }); +}); + +test("session rows are read-only: the planner plans nothing for them", () => { + assert.deepEqual(deletionActionsFor("session"), { + deleteFromEditorStore: false, + writeTombstone: false, + }); +}); + +// --------------------------------------------------------------------------- +// Copy: one confirmation line for every row; the failure toasts state +// exactly what state remains. +// --------------------------------------------------------------------------- + +test("the confirmation footer is the single y/n line (PR #1393)", () => { + const text = deleteConfirmFooterText(); + assert.ok(!text.includes("\n"), "the confirmation stays on one line"); + assert.ok(text.includes("Delete this prompt from history (y/n)?")); + assert.ok(text.includes("Prompt stays in session log")); +}); + +test("failure toasts state the remaining state exactly (PR #1393)", () => { + // A thrown store delete aborts before any tombstone: nothing removed. + assert.equal( + STORE_DELETE_FAILED_TEXT, + "Store delete failed; nothing was removed.", + ); + // Editor-path hide failure: the store row is gone, the prompt may + // reappear from transcripts. + assert.equal( + EDITOR_HIDE_FAILED_TEXT, + "Deleted from the store, but hiding failed — the prompt may reappear from session transcripts.", + ); +}); + +// --------------------------------------------------------------------------- +// Source-parse: the wiring inside the selector (the class itself is not +// instantiable under node:test — see the header note). +// --------------------------------------------------------------------------- + +const selectorSource = fs.readFileSync( + fileURLToPath(new URL("../extensions/history/index.ts", import.meta.url)), + "utf8", +); + +/** Slice out a 2-space-indented method body by its exact signature. */ +function methodBodyOf(signature: string): string { + const decl = selectorSource.indexOf(signature); + assert.ok(decl >= 0, `${signature} should exist`); + const end = selectorSource.indexOf("\n }", decl); + assert.ok(end > decl, `${signature}'s body should close`); + return selectorSource.slice(decl, end); +} + +function deleteCurrentBody(): string { + return methodBodyOf("private deleteCurrent(): void {"); +} + +function executeDeleteBody(): string { + return methodBodyOf("private executeDelete(): void {"); +} + +function handleInputBody(): string { + return methodBodyOf("handleInput(data: string): void {"); +} + +test("deleteCurrent: session rows no-op FIRST — before any arm or mutation", () => { + const body = deleteCurrentBody(); + const guardAt = body.indexOf('(selected.source ?? "editor") === "session"'); + assert.ok(guardAt >= 0, "the session read-only guard must exist"); + const guardReturnAt = body.indexOf("return;", guardAt); + assert.ok(guardReturnAt > guardAt, "the session guard must return"); + // The guard precedes the arm/execute split and every mutation helper. + const armAt = body.indexOf("this.armDelete()"); + const executeAt = body.indexOf("this.executeDelete()"); + assert.ok(armAt > guardAt, "the session guard must precede arming"); + assert.ok(executeAt > guardAt, "the session guard must precede executing"); + // The combo entry stays two-step: unarmed arms, armed executes. + assert.ok( + body.includes("if (!this.confirmArmed)"), + "the unarmed press must arm", + ); + assert.ok( + armAt < executeAt, + "armDelete is the unarmed branch, executeDelete the armed one", + ); +}); + +test("armDelete only paints: armed + footer + rebuild — never mutates", () => { + const body = methodBodyOf("private armDelete(): void {"); + assert.ok(body.includes("this.confirmArmed = true;")); + assert.ok(body.includes("this.refreshDeleteFooter()")); + assert.ok(body.includes("this.rebuildList()")); + assert.ok(!body.includes("hidePrompt("), "arming never writes a tombstone"); + assert.ok( + !body.includes("this.records.splice("), + "arming never mutates rows", + ); +}); + +test("executeDelete leaves the armed state before any mutation", () => { + const body = executeDeleteBody(); + const disarmAt = body.indexOf("this.confirmArmed = false;"); + assert.ok(disarmAt >= 0, "executing must leave the armed state"); + assert.ok(body.includes("this.refreshDeleteFooter()")); + const actionsAt = body.indexOf("deletionActionsFor("); + const hideAt = body.indexOf("hidePrompt("); + const spliceAt = body.indexOf("this.records.splice("); + assert.ok( + disarmAt < actionsAt && actionsAt < hideAt && hideAt < spliceAt, + "disarm → plan → tombstone → splice ordering", + ); +}); + +test("while armed, handleInput is modal: the router runs FIRST and returns", () => { + const body = handleInputBody(); + const modalAt = body.indexOf("if (this.confirmArmed) {"); + assert.ok(modalAt >= 0, "the modal branch must exist"); + assert.ok( + body.includes("deleteConfirmStep("), + "the armed branch routes through the pure router", + ); + assert.ok( + body.includes('matchesKey(data, "ctrl+shift+backspace")') && + body.includes('matchesKey(data, "escape")'), + "the combo and escape matches come from the TUI keymap", + ); + const executeAt = body.indexOf("this.executeDelete()"); + const cancelAt = body.indexOf("this.disarmDeleteConfirm()"); + assert.ok(executeAt > modalAt, "y must execute inside the modal branch"); + assert.ok(cancelAt > modalAt, "n/Esc must disarm inside the modal branch"); + // Full swallow: the modal branch RETURNS before the dispatch loop and + // the search fallthrough can see the key — esc cannot close the overlay. + const modalReturnAt = body.indexOf("return;", modalAt); + assert.ok(modalReturnAt > modalAt, "the modal branch must return"); + const loopAt = body.indexOf( + "for (const { match, handler } of this.dispatch) {", + ); + const fallthroughAt = body.indexOf( + "if (!handled) this.forwardToSearch(data);", + ); + assert.ok(loopAt > modalReturnAt, "armed keys never reach dispatch"); + assert.ok(fallthroughAt > modalReturnAt, "armed keys never reach search"); +}); + +test("the old disarm pre-pass is superseded — disarm only on the modal cancel path", () => { + const body = handleInputBody(); + assert.ok( + !body.includes('!matchesKey(data, "ctrl+shift+backspace")'), + "the unconditional disarm pre-pass must be gone", + ); + const modalAt = body.indexOf("if (this.confirmArmed) {"); + const disarmAt = body.indexOf("this.disarmDeleteConfirm()"); + assert.ok(disarmAt > modalAt, "disarm must sit inside the modal branch"); +}); + +test("Esc while DISARMED still cancels the overlay via the dispatch entry", () => { + const table = selectorSource.slice( + selectorSource.indexOf("private readonly dispatch"), + selectorSource.indexOf("\n ];"), + ); + const cancelAt = table.indexOf('kb.matches(_d, "tui.select.cancel")'); + assert.ok(cancelAt >= 0, "the cancel dispatch entry must stay"); + const entry = table.slice(cancelAt, table.indexOf("},", cancelAt)); + assert.ok( + entry.includes("this.onCancel()"), + "disarmed esc must still close the overlay", + ); +}); + +test("a wheel scroll disarms (and never executes) the armed delete", () => { + const handleMouseAt = selectorSource.indexOf("override handleMouse("); + assert.ok(handleMouseAt >= 0, "handleMouse should exist"); + const mouseEnd = selectorSource.indexOf("\n }", handleMouseAt); + const mouseBody = selectorSource.slice(handleMouseAt, mouseEnd); + assert.ok( + mouseBody.indexOf("this.disarmDeleteConfirm()") >= 0, + "a wheel scroll can move the selection off the armed row — it must disarm", + ); + assert.ok( + !mouseBody.includes("this.executeDelete()"), + "a wheel scroll must never execute the delete", + ); +}); + +test("the armed state drives the footer copy and the error-colored highlight", () => { + const footerBody = methodBodyOf("private refreshDeleteFooter(): void {"); + assert.ok( + footerBody.includes("deleteConfirmFooterText()"), + "the armed footer uses the single pure copy (no source argument)", + ); + assert.ok( + !footerBody.includes("deleteConfirmFooterText" + "(source)"), + "the footer must not route through a source variant", + ); + assert.ok( + footerBody.includes("SELECTOR_FOOTER_HELP"), + "disarming restores the help line", + ); + + const rebuildBody = methodBodyOf( + "private rebuildListWithWidth(width: number): void {", + ); + assert.ok( + rebuildBody.includes("this.confirmArmed"), + "the armed state repaints the selected row", + ); +}); + +// --------------------------------------------------------------------------- +// Capture gate: the opt-in switch stays GENTLE_PI_HISTORY_CAPTURE (#1390). +// --------------------------------------------------------------------------- + +// The contributor branch briefly renamed the switch; the rename must not +// ship. Assemble the rejected literal from parts so this file stays +// grep-clean for it. +const renamedSwitch = `GENTLE_PI_HISTORY_${"ENABLE"}`; + +test("captureEnabled reads GENTLE_PI_HISTORY_CAPTURE (strict 1/true/on unchanged)", () => { + const decl = selectorSource.indexOf("export function captureEnabled("); + assert.ok(decl >= 0, "captureEnabled should exist"); + const end = selectorSource.indexOf("\n}", decl); + assert.ok(end > decl, "captureEnabled's body should close"); + const body = selectorSource.slice(decl, end); + assert.ok( + body.includes("env.GENTLE_PI_HISTORY_CAPTURE"), + "the shipped switch must be read", + ); + assert.ok(!body.includes(renamedSwitch), "the rename must not ship"); + assert.ok( + body.includes('?.trim().toLowerCase()'), + "whitespace + case normalization unchanged", + ); + assert.ok( + body.includes('value === "1" || value === "true" || value === "on"'), + "strict 1/true/on opt-in unchanged", + ); +}); + +test("the history extension never mentions the renamed switch", () => { + assert.equal(selectorSource.includes(renamedSwitch), false); +}); + +// --------------------------------------------------------------------------- +// Partial sweep failures (PR #1393 adaptation): a store file that could not +// be read or rewritten may still hold a copy, so the delete must say so — +// and still write the tombstone that hides the remaining copies. +// --------------------------------------------------------------------------- + +test("storeDeleteFollowUp: a clean sweep proceeds without a notice", () => { + assert.deepEqual( + storeDeleteFollowUp({ filesAffected: 1, removed: 2, failed: 0 }), + { proceed: true }, + ); +}); + +test("storeDeleteFollowUp: nothing removed and nothing failed stops quietly", () => { + assert.deepEqual( + storeDeleteFollowUp({ filesAffected: 0, removed: 0, failed: 0 }), + { proceed: false }, + ); +}); + +test("storeDeleteFollowUp: any failed file proceeds to hide AND surfaces an error", () => { + for (const removed of [0, 3]) { + assert.deepEqual( + storeDeleteFollowUp({ filesAffected: removed > 0 ? 1 : 0, removed, failed: 1 }), + { proceed: true, notice: STORE_DELETE_PARTIAL_TEXT }, + ); + } + assert.ok(STORE_DELETE_PARTIAL_TEXT.includes("could not")); + assert.ok(STORE_DELETE_PARTIAL_TEXT.includes("hidden")); +}); + +test("executeDelete routes the sweep result through storeDeleteFollowUp before hiding", () => { + const body = executeDeleteBody(); + const followAt = body.indexOf("storeDeleteFollowUp("); + const noticeAt = body.indexOf('this.onNotify?.(followUp.notice, "error")'); + const hideAt = body.indexOf("hidePrompt("); + assert.ok(followAt >= 0, "the sweep result must be interpreted"); + assert.ok(noticeAt > followAt, "a partial failure must surface as an error"); + assert.ok(hideAt > noticeAt, "the tombstone still follows a partial failure"); + assert.equal( + body.includes("if (removed === 0) return;"), + false, + "a zero-removal partial failure must not skip the tombstone", + ); +}); diff --git a/tests/history-dispatch.test.ts b/tests/history-dispatch.test.ts index 575a4d5a1..cdda32505 100644 --- a/tests/history-dispatch.test.ts +++ b/tests/history-dispatch.test.ts @@ -10,8 +10,7 @@ import { fileURLToPath } from "node:url"; * 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 + * exactly 12 explicit entries in a fixed order, then the implicit * forwardToSearch fallthrough inside handleInput. */ @@ -34,6 +33,7 @@ const EXPECTED_MATCHERS = [ 'kb.matches(_d, "tui.select.cancel")', 'matchesKey(d, "home")', 'matchesKey(d, "end")', + 'matchesKey(d, "ctrl+shift+backspace")', 'matchesKey(d, "ctrl+shift+up")', 'matchesKey(d, "ctrl+shift+down")', ]; @@ -49,6 +49,7 @@ const EXPECTED_HANDLERS = [ "this.onCancel()", "this.jumpToFirst()", "this.jumpToLast()", + "this.deleteCurrent()", "this.previewPageUp()", "this.previewPageDown()", ]; @@ -84,13 +85,13 @@ function methodBody(name: string): string { } describe("dispatch table (source-parsed, §B2)", () => { - it("has exactly 11 explicit match: entries (AC-P2-4.1)", () => { + it("has exactly 12 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}`, + 12, + `expected 12 explicit entries, found ${matchCount}`, ); }); diff --git a/tests/history-hide-prompts.test.ts b/tests/history-hide-prompts.test.ts index ccd1f8ea8..4a5a03f55 100644 --- a/tests/history-hide-prompts.test.ts +++ b/tests/history-hide-prompts.test.ts @@ -6,17 +6,19 @@ import path from "node:path"; import { hidePrompt, readHiddenPrompts, + tombstoneKey, } 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 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. +// Unit WU4 — tombstone write half + read half (spec C4, design §D6; slice-05 +// D5 recency cap). fs-only 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 +// WRITE half persists keys in RECENCY order (oldest first, newest last, +// never sorted) capped at HIDE_FILE_MAX_ENTRIES (1000, oldest dropped). +// 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}-`)); @@ -52,10 +54,10 @@ function trustedKeys(stateDir: string): Set { } // 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", () => { +// shared tombstoneKey helper for the same text — never a re-implementation: +// the stored file content is compared against tombstoneKey's own output +// with strict equality. +test("T24 (AC-S4-1): hide keys byte-match tombstoneKey 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. @@ -69,8 +71,9 @@ test("T24 (AC-S4-1): hide keys byte-match promptDedupKey across whitespace, case } 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()); + // Byte-match: the file holds EXACTLY the shared helper's output, in + // insertion (recency) order — the write half never sorts. + assert.deepEqual(stored, texts.map((text) => tombstoneKey(text))); // The read half agrees. const keys = trustedKeys(stateDir); assert.equal(keys.size, stored.length); @@ -91,10 +94,74 @@ test("T25 (AC-S4-2): duplicate hides compact to one key; a missing file reads tr 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")]); + assert.deepEqual(stored, [tombstoneKey("same text")]); const keys = trustedKeys(stateDir); assert.equal(keys.size, 1); - assert.ok(keys.has(promptDedupKey("same text"))); + assert.ok(keys.has(tombstoneKey("same text"))); +}); + +// D5 — recency order: re-hiding an existing key REFRESHES it to the end +// (newest); the file array is oldest-first, newest-appended-last — the +// write half never sorts. +test("re-hiding an existing key refreshes it to the end (recency order, no sort)", () => { + const stateDir = makeStateDir("recency"); + const keysOf = (texts: string[]) => texts.map((text) => tombstoneKey(text)); + for (const text of ["alpha prompt", "beta prompt", "gamma prompt"]) { + assert.deepEqual(hidePrompt(stateDir, text), { status: "written" }); + } + assert.deepEqual(readHideFile(stateDir), keysOf([ + "alpha prompt", + "beta prompt", + "gamma prompt", + ])); + // Re-hide the oldest key: it moves to the END; the others keep order. + assert.deepEqual(hidePrompt(stateDir, "alpha prompt"), { + status: "written", + }); + assert.deepEqual(readHideFile(stateDir), keysOf([ + "beta prompt", + "gamma prompt", + "alpha prompt", + ])); + assert.equal(trustedKeys(stateDir).size, 3); +}); + +// D5 — cap: hidden.json keeps at most 1000 keys in recency order; the +// 1001st distinct prompt drops the OLDEST key from the front. The file is +// a rebuildable cache, not a retention guarantee — a dropped prompt may +// reappear and be deleted again. +test("the 1001st distinct prompt drops the oldest key — the file keeps exactly 1000", () => { + const stateDir = makeStateDir("cap"); + const oldest = "oldest prompt"; + assert.deepEqual(hidePrompt(stateDir, oldest), { status: "written" }); + for (let i = 1; i <= 999; i++) { + assert.deepEqual(hidePrompt(stateDir, `prompt number ${i}`), { + status: "written", + }); + } + // Exactly at the cap: 1000 keys, oldest first, newest last. + const atCap = readHideFile(stateDir); + assert.equal(atCap.length, 1000); + assert.equal(atCap[0], tombstoneKey(oldest)); + assert.equal(atCap[atCap.length - 1], tombstoneKey("prompt number 999")); + // The 1001st distinct prompt: the front (oldest) drops, the newest lands. + assert.deepEqual(hidePrompt(stateDir, "prompt number 1000"), { + status: "written", + }); + const after = readHideFile(stateDir); + assert.equal(after.length, 1000, "the cap holds the file at exactly 1000"); + assert.equal( + after.includes(tombstoneKey(oldest)), + false, + "the oldest key must be dropped from the front", + ); + assert.equal( + after[after.length - 1], + tombstoneKey("prompt number 1000"), + "the newest key must sit at the end", + ); + // The read half agrees with the capped file. + assert.equal(trustedKeys(stateDir).size, 1000); }); // T26 — AC-S4-5: corrupt hidden.json FAILS CLOSED for history reads. The @@ -123,7 +190,7 @@ test("T26 (AC-S4-5): corrupt hidden.json reads untrusted; hide refuses without r assert.deepEqual(hidePrompt(stateDir, "beta prompt"), { status: "written" }); const keys = trustedKeys(stateDir); assert.equal(keys.size, 1); - assert.ok(keys.has(promptDedupKey("beta prompt"))); + assert.ok(keys.has(tombstoneKey("beta prompt"))); }); // Wrong-shaped file: valid JSON that is not an array fails closed too (both diff --git a/tests/history-off-path.test.ts b/tests/history-off-path.test.ts index ef0eb0701..8503dd973 100644 --- a/tests/history-off-path.test.ts +++ b/tests/history-off-path.test.ts @@ -42,6 +42,9 @@ function loadWithCommand(env: NodeJS.ProcessEnv, root: string): Harness { cwd: CWD, instanceId: "inst-off", now: () => 1700000000000, + // Keep any opted-in warm-up away from the real ~/.pi/agent. + agentDir: path.join(root, "agent"), + sessionsRoot: path.join(root, "sessions"), }); const command = commands.find(([name]) => name === "history"); assert.ok(command, "the history command must be registered"); @@ -61,12 +64,8 @@ function fakeCtx(notifyCalls: Array<[string, string]>) { }; } -// NOTE: there is deliberately no enabled-path smoke test here. The selector -// drain is a module-level path hard-wired to PI_HISTORY_ROOT -// (~/.pi/agent/history) with no injection point, and bun's os.homedir() -// ignores runtime HOME overrides — invoking the command with capture on -// would migrate/seed/write the real user store. The enabled direction stays -// covered by the deps-injected tests in history-session-writer.test.ts. +// The open flow reads the injected deps (env/root/cwd), never the module +// defaults, so the enabled direction is testable against a temp root too. test("with capture disabled, extension load writes nothing", async () => { const root = makeRoot(); @@ -92,3 +91,14 @@ test("with capture disabled, the history command imports nothing and warns", asy // The gate must fire before the drain: no migration, no seed, no store. assert.deepEqual(fs.readdirSync(root), []); }); + +test("with capture enabled, opening the selector reads without initializing the store", async () => { + const root = makeRoot(); + const { commandHandler } = loadWithCommand({ GENTLE_PI_HISTORY_CAPTURE: "1" }, root); + // Open before the opted-in warm-up tick: the open flow alone must not + // migrate, register, seed, or create a capture file. + const notifyCalls: Array<[string, string]> = []; + await commandHandler([], fakeCtx(notifyCalls)); + assert.deepEqual(notifyCalls, [["No prompt history available.", "warning"]]); + assert.deepEqual(fs.readdirSync(root), []); +}); diff --git a/tests/history-openflow-integration.test.ts b/tests/history-openflow-integration.test.ts index 287f35e0e..7d88296a2 100644 --- a/tests/history-openflow-integration.test.ts +++ b/tests/history-openflow-integration.test.ts @@ -115,3 +115,59 @@ test("the selection result pastes into the editor via pasteToEditor", () => { "the selected prompt enters the editor through the paste pipeline (a22588fc)", ); }); + +/** 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); +} + +// --------------------------------------------------------------------------- +// 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-scope-delete.test.ts b/tests/history-scope-delete.test.ts new file mode 100644 index 000000000..a2e7dd7df --- /dev/null +++ b/tests/history-scope-delete.test.ts @@ -0,0 +1,247 @@ +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, + deleteFromGlobal, + deleteFromProject, + globalSeedPath, + openSessionWriter, + projectHash, +} from "../extensions/history/store.ts"; + +// Scope delete (design v2): sweepFiles' atomic per-file rewrite semantics +// plus the project/global delete entry points. Synthetic project cwds — +// never real directories on any machine (identity only feeds projectHash; +// the fixtures live in tmpdirs and never touch the user's real ~/.pi). +const PROJECT_A = "/fixtures/pi-history/project-a"; +const PROJECT_B = "/fixtures/pi-history/project-b"; + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-del-")); +} + +function writeLines(file: string, texts: string[]): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + `${texts.map((t) => JSON.stringify({ v: 1, text: t })).join("\n")}\n`, + "utf8", + ); +} + +function fileTexts(file: string): string[] { + return fs + .readFileSync(file, "utf8") + .trim() + .split("\n") + .filter((l) => l.length > 0) + .map((l) => (JSON.parse(l) as { text: string }).text); +} + +test("project delete removes every copy across the project's files", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + writeLines(path.join(dir, "s1.jsonl"), ["keep", "victim"]); + writeLines(path.join(dir, "s2.jsonl"), ["VICTIM ", "also-keep"]); + const result = deleteFromProject(root, PROJECT_A, "victim"); + assert.deepEqual(result, { filesAffected: 2, removed: 2, failed: 0 }); + assert.deepEqual(fileTexts(path.join(dir, "s1.jsonl")), ["keep"]); + assert.deepEqual(fileTexts(path.join(dir, "s2.jsonl")), ["also-keep"]); +}); + +test("project delete leaves other projects untouched", () => { + const root = makeRoot(); + const dirA = path.join(root, "projects", projectHash(PROJECT_A)); + const dirB = path.join(root, "projects", projectHash(PROJECT_B)); + writeLines(path.join(dirA, "s.jsonl"), ["victim"]); + writeLines(path.join(dirB, "s.jsonl"), ["victim", "b-keep"]); + deleteFromProject(root, PROJECT_A, "victim"); + assert.deepEqual(fileTexts(path.join(dirB, "s.jsonl")), ["victim", "b-keep"]); +}); + +test("project delete of unknown prompt is a no-op", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + writeLines(path.join(dir, "s.jsonl"), ["a"]); + const result = deleteFromProject(root, PROJECT_A, "missing"); + assert.deepEqual(result, { filesAffected: 0, removed: 0, failed: 0 }); + assert.deepEqual(fileTexts(path.join(dir, "s.jsonl")), ["a"]); +}); + +test("project delete on a missing dir is a no-op", () => { + const root = makeRoot(); + const result = deleteFromProject(root, PROJECT_A, "x"); + assert.deepEqual(result, { filesAffected: 0, removed: 0, failed: 0 }); +}); + +test("global delete on a root without a projects dir is a zero-delete no-op", () => { + const root = makeRoot(); + const result = deleteFromGlobal(root, "x"); + assert.deepEqual(result, { filesAffected: 0, removed: 0, failed: 0 }); +}); + +test("global delete sweeps every project dir plus the legacy seed", () => { + const root = makeRoot(); + const dirA = path.join(root, "projects", projectHash(PROJECT_A)); + const dirB = path.join(root, "projects", projectHash(PROJECT_B)); + writeLines(path.join(dirA, "s.jsonl"), ["victim", "a-keep"]); + writeLines(path.join(dirB, "s.jsonl"), ["victim"]); + writeLines(globalSeedPath(root), ["victim", "legacy-keep"]); + const result = deleteFromGlobal(root, "victim"); + assert.deepEqual(result, { filesAffected: 3, removed: 3, failed: 0 }); + assert.deepEqual(fileTexts(path.join(dirA, "s.jsonl")), ["a-keep"]); + assert.deepEqual(fileTexts(path.join(dirB, "s.jsonl")), []); + assert.deepEqual(fileTexts(globalSeedPath(root)), ["legacy-keep"]); +}); + +test("delete leaves no tmp files behind", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + writeLines(path.join(dir, "s.jsonl"), ["victim"]); + deleteFromProject(root, PROJECT_A, "victim"); + const leftovers = fs.readdirSync(dir).filter((f) => f.includes(".tmp-")); + assert.deepEqual(leftovers, []); +}); + +// node:test has no test.skipIf (Bun-ism): root skips via the options object. +test( + "an unreadable store file (chmod 000) is counted as failed; readable copies still swept", + { skip: process.getuid?.() === 0 ? "requires non-root" : false }, + () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + const readable = path.join(dir, "readable.jsonl"); + const sealed = path.join(dir, "sealed.jsonl"); + writeLines(readable, ["victim", "keep"]); + writeLines(sealed, ["victim"]); + fs.chmodSync(sealed, 0o000); + try { + const result = deleteFromProject(root, PROJECT_A, "victim"); + // The unreadable file may still hold a copy: it is counted as a + // failure (never reported as a clean delete); the readable copy is + // removed and the sweep is never fatal. + assert.deepEqual(result, { filesAffected: 1, removed: 1, failed: 1 }); + assert.deepEqual(fileTexts(readable), ["keep"]); + assert.equal(fs.existsSync(sealed), true); + } finally { + fs.chmodSync(sealed, 0o644); // restore before cleanup + } + }, +); + +test("a file whose every line is deleted becomes empty (kept, not removed)", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + const file = path.join(dir, "s.jsonl"); + writeLines(file, ["only-victim"]); + deleteFromProject(root, PROJECT_A, "only-victim"); + assert.equal(fs.existsSync(file), true); + assert.equal(fs.readFileSync(file, "utf8"), ""); +}); + +// Active-writer safety (design v2): the sweep rewrites the writer's own +// file IN PLACE (tmp + rename, never a removal — emptied files are kept), +// so a concurrently live writer keeps working by path: its next capture +// appends into the swept file, and the surviving + new lines parse fine. +test("a sweep with a concurrent live writer keeps the writer's file functional", () => { + const root = makeRoot(); + const state = openSessionWriter(root, PROJECT_A, "instance-1"); + appendSessionCapture(state, "victim"); + appendSessionCapture(state, "keeper"); + + const result = deleteFromProject(root, PROJECT_A, "victim"); + assert.deepEqual(result, { filesAffected: 1, removed: 1, failed: 0 }); + + // The same writer state keeps appending after the sweep — the file was + // rewritten under the writer's feet, not removed. + appendSessionCapture(state, "after-delete"); + assert.equal(state.lineCount, 3); + assert.equal(fs.existsSync(state.filePath), true); + assert.deepEqual(fileTexts(state.filePath), ["keeper", "after-delete"]); +}); + +// Concurrent appends (PR #1393 adaptation): another pi instance appends to +// its own file by path at any time. A line that lands between the sweep's +// read and its rename must survive the rewrite instead of vanishing with +// the replaced inode. The hook simulates that instance: it appends right +// before the sweep's rename of the swept file. +function withAppendBeforeRename( + target: string, + appended: string, + run: () => void, +): void { + const realRename = fs.renameSync; + let fired = false; + fs.renameSync = ((from: fs.PathLike, to: fs.PathLike) => { + if (!fired && String(to) === target) { + fired = true; + fs.appendFileSync(target, appended, "utf8"); + } + return realRename(from, to); + }) as typeof fs.renameSync; + try { + run(); + } finally { + fs.renameSync = realRename; + } + assert.ok(fired, "the simulated concurrent append must fire"); +} + +test("a line appended between the sweep's read and rename survives", () => { + const root = makeRoot(); + const state = openSessionWriter(root, PROJECT_A, "other-instance"); + appendSessionCapture(state, "victim"); + appendSessionCapture(state, "keeper"); + withAppendBeforeRename( + state.filePath, + `${JSON.stringify({ v: 1, text: "raced in" })}\n`, + () => { + const result = deleteFromProject(root, PROJECT_A, "victim"); + assert.deepEqual(result, { filesAffected: 1, removed: 1, failed: 0 }); + }, + ); + assert.deepEqual(fileTexts(state.filePath), ["keeper", "raced in"]); +}); + +test("a torn last line completed during the sweep is preserved whole", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + const file = path.join(dir, "other.jsonl"); + const torn = JSON.stringify({ v: 1, text: "in flight" }); + writeLines(file, ["victim", "keeper"]); + fs.appendFileSync(file, torn.slice(0, 10), "utf8"); + withAppendBeforeRename(file, `${torn.slice(10)}\n`, () => { + deleteFromProject(root, PROJECT_A, "victim"); + }); + assert.deepEqual(fileTexts(file), ["keeper", "in flight"]); +}); + +test("a failed rewrite is counted, keeps the original, and leaves no tmp file", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(PROJECT_A)); + const broken = path.join(dir, "broken.jsonl"); + const healthy = path.join(dir, "healthy.jsonl"); + writeLines(broken, ["victim", "broken-keep"]); + writeLines(healthy, ["victim", "healthy-keep"]); + const realRename = fs.renameSync; + fs.renameSync = ((from: fs.PathLike, to: fs.PathLike) => { + if (String(to) === broken) { + throw Object.assign(new Error("simulated EIO"), { code: "EIO" }); + } + return realRename(from, to); + }) as typeof fs.renameSync; + let result; + try { + result = deleteFromProject(root, PROJECT_A, "victim"); + } finally { + fs.renameSync = realRename; + } + assert.deepEqual(result, { filesAffected: 1, removed: 1, failed: 1 }); + assert.deepEqual(fileTexts(broken), ["victim", "broken-keep"]); + assert.deepEqual(fileTexts(healthy), ["healthy-keep"]); + const leftovers = fs.readdirSync(dir).filter((f) => f.includes(".tmp-")); + assert.deepEqual(leftovers, []); +}); diff --git a/tests/history-session-writer.test.ts b/tests/history-session-writer.test.ts index a249e8b18..3dd494ac6 100644 --- a/tests/history-session-writer.test.ts +++ b/tests/history-session-writer.test.ts @@ -157,6 +157,8 @@ test("captureEnabled is a strict opt-in", () => { 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); + // The unshipped rename from the contributor branch is not a switch. + assert.equal(captureEnabled({ [`GENTLE_PI_HISTORY_${"ENABLE"}`]: "1" }), false); }); test("the capture handler is a no-op unless the user opts in", () => { diff --git a/tests/history-tombstone-exact.test.ts b/tests/history-tombstone-exact.test.ts new file mode 100644 index 000000000..cdaf9ca1d --- /dev/null +++ b/tests/history-tombstone-exact.test.ts @@ -0,0 +1,139 @@ +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, + readHiddenPrompts, + tombstoneKey, +} from "../extensions/history/hide-prompts.ts"; +import { + bootstrapProjectSeed, + drainGlobal, + drainProject, + projectHash, + seedFilePath, +} from "../extensions/history/store.ts"; + +// Exact tombstones (PR #1393 adaptation): a deletion hides ONLY the exact +// normalized prompt (whitespace-collapsed, trimmed, case-insensitive) — +// never every prompt that shares a 120-character prefix. hidden.json keeps +// its JSON-array-of-strings shape; new entries are hashes, and plaintext +// entries written by the earlier prefix format stay readable and honored. + +const CWD = "/pi-history-test/tombstone-exact"; + +function makeRoot(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-tomb-")); +} + +function writeLines(file: string, texts: string[]): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + `${texts.map((t) => JSON.stringify({ v: 1, text: t })).join("\n")}\n`, + "utf8", + ); +} + +function drainedProject(root: string): string[] { + const drained = drainProject(root, CWD, 1000, root); + assert.equal(drained.status, "ok"); + if (drained.status !== "ok") throw new Error("unreachable"); + return drained.prompts; +} + +const PREFIX = `${"shared context ".repeat(10)}`; // 150 chars, > 120 +const LONG_A = `${PREFIX}then deploy staging`; +const LONG_B = `${PREFIX}then deploy production`; + +test("tombstoneKey normalizes whitespace and case but never truncates", () => { + assert.equal(tombstoneKey("Fix\t the BUILD "), tombstoneKey("fix the build")); + assert.notEqual(tombstoneKey(LONG_A), tombstoneKey(LONG_B)); + assert.match(tombstoneKey("fix the build"), /^sha256:[0-9a-f]{64}$/); +}); + +test("hidden.json stores a hash, never the deleted prompt's text", () => { + const root = makeRoot(); + assert.deepEqual(hidePrompt(root, "my secret token abc123"), { + status: "written", + }); + const raw = fs.readFileSync(path.join(root, "hidden.json"), "utf8"); + assert.equal(raw.includes("secret"), false); + assert.deepEqual(JSON.parse(raw), [tombstoneKey("my secret token abc123")]); +}); + +test("hiding one long prompt keeps a sibling that shares its 120-char prefix", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(CWD)); + writeLines(path.join(dir, "s.jsonl"), [LONG_A, LONG_B, "short keep"]); + assert.deepEqual(hidePrompt(root, LONG_A), { status: "written" }); + + const prompts = drainedProject(root); + assert.equal(prompts.includes(LONG_A), false, "the deleted prompt stays hidden"); + assert.ok(prompts.includes(LONG_B), "the prefix sibling must stay visible"); + assert.ok(prompts.includes("short keep")); + + const global = drainGlobal(root, 1000, root); + assert.equal(global.status, "ok"); + if (global.status !== "ok") throw new Error("unreachable"); + assert.equal(global.prompts.includes(LONG_A), false); + assert.ok(global.prompts.includes(LONG_B)); +}); + +test("a tombstone hides whitespace and case variants of the exact prompt", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(CWD)); + writeLines(path.join(dir, "s.jsonl"), ["Deploy THE api", "deploy the api now"]); + hidePrompt(root, "deploy the api"); + assert.deepEqual(drainedProject(root), ["deploy the api now"]); +}); + +test("plaintext entries from the earlier prefix format stay readable and honored", () => { + const root = makeRoot(); + const dir = path.join(root, "projects", projectHash(CWD)); + writeLines(path.join(dir, "s.jsonl"), ["Legacy Hidden", "visible prompt"]); + // The earlier writer stored promptDedupKey output: the whitespace- + // collapsed, 120-char, lowercased text. + fs.writeFileSync( + path.join(root, "hidden.json"), + JSON.stringify(["legacy hidden"]), + "utf8", + ); + assert.equal(readHiddenPrompts(root).status, "trusted"); + assert.deepEqual(drainedProject(root), ["visible prompt"]); + + // A new hide appends a hash entry and preserves the legacy entry. + hidePrompt(root, "visible prompt"); + const stored = JSON.parse( + fs.readFileSync(path.join(root, "hidden.json"), "utf8"), + ); + assert.deepEqual(stored, ["legacy hidden", tombstoneKey("visible prompt")]); + assert.deepEqual(drainedProject(root), []); +}); + +test("seed bootstrap skips an exact tombstone but seeds its prefix sibling", () => { + const root = makeRoot(); + const sessionsRoot = path.join(root, "sessions"); + const sessions = path.join(sessionsRoot, "--pi-history-test-tombstone-exact--"); + fs.mkdirSync(sessions, { recursive: true }); + fs.writeFileSync( + path.join(sessions, "s.jsonl"), + [ + JSON.stringify({ type: "session", version: 3 }), + JSON.stringify({ type: "message", message: { role: "user", content: LONG_A } }), + JSON.stringify({ type: "message", message: { role: "user", content: LONG_B } }), + ].join("\n") + "\n", + ); + hidePrompt(root, LONG_A); + + const result = bootstrapProjectSeed(root, CWD, sessionsRoot, 500, root); + assert.deepEqual(result, { seeded: 1, ran: true }); + const seeded = fs + .readFileSync(seedFilePath(root, CWD), "utf8") + .trim() + .split("\n") + .map((line) => (JSON.parse(line) as { text: string }).text); + assert.deepEqual(seeded, [LONG_B]); +}); diff --git a/tests/history-wheel-mouse.test.ts b/tests/history-wheel-mouse.test.ts index 37ddcd6b6..fbb4a33b7 100644 --- a/tests/history-wheel-mouse.test.ts +++ b/tests/history-wheel-mouse.test.ts @@ -34,7 +34,7 @@ const selectorSource = fs.readFileSync( // 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)", () => { +test("handleMouse override is wheel-only and the dispatch table keeps 12 entries (AC-L6-1)", () => { const decl = selectorSource.indexOf("override handleMouse("); assert.ok(decl >= 0, "PromptHistorySelector should override handleMouse"); const end = selectorSource.indexOf("\n }", decl); @@ -60,8 +60,8 @@ test("handleMouse override is wheel-only and the dispatch table keeps 11 entries const entries = table.split("match:").length - 1; assert.equal( entries, - 11, - "wheel is not a keybinding: exactly the 11 §B2 dispatch entries, no extra", + 12, + "wheel is not a keybinding: exactly 12 dispatch entries, no 13th", ); });