From a8b5d6543a366995473a98c2d03137a1206f1777 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 02:14:34 +0200 Subject: [PATCH 1/7] fix(eval): point analyze-runs at evals/runs + tolerate no-emoji timing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The analyzer read run dirs from evals/ but sweep writes them to evals/runs/ (sweep.ts), so every `analyze-runs N` returned 0 runs. Its TIMING regex also still required the dropped `⏱` prefix, so even direct-path runs parsed no turn timings. Both fixed — the discovery instrument works again. --- packages/core/scripts/analyze-runs.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/core/scripts/analyze-runs.ts b/packages/core/scripts/analyze-runs.ts index 278dbce..3135a96 100644 --- a/packages/core/scripts/analyze-runs.ts +++ b/packages/core/scripts/analyze-runs.ts @@ -11,6 +11,9 @@ import { join } from "node:path"; import { isRecord } from "../src/lib/guards"; const evalsRoot = join(import.meta.dir, "..", "..", "..", "evals"); +// Sweep writes each run to evals/runs/ (see sweep.ts). Run dirs are +// resolved from there, not the evals/ root (which only holds runs/, corpus/, …). +const runsRoot = join(evalsRoot, "runs"); interface IRunMetrics { runId: string; @@ -40,7 +43,10 @@ interface IRunMetrics { quality: number | undefined; } -const TIMING = /⏱ turn (\d+) took ([\d.]+)(s|ms) \(total ([\d.]+)(s|ms)\)/; +// The turn-timing line (turn.ts reportTurnTiming). The `⏱` prefix was dropped +// from the emitter, so it's optional here to stay compatible with both formats. +const TIMING = + /(?:⏱ )?turn (\d+) took ([\d.]+)(s|ms) \(total ([\d.]+)(s|ms)\)/u; const RED = /turn \d+: red \((\d+) error/; const ASKING = /turn (\d+): asking model/; // Hand-counting = the model re-typing the file with SEQUENTIAL line numbers @@ -199,16 +205,16 @@ async function resolveDirs(): Promise { if (args.length === 2 && /^\d+$/.test(args[1] ?? "")) { const prefix = args[0] ?? ""; const count = Number(args[1]); - const all = await readdir(evalsRoot, { withFileTypes: true }); + const all = await readdir(runsRoot, { withFileTypes: true }); const dirs = all .filter((d) => d.isDirectory() && d.name.startsWith(prefix)) .map((d) => d.name) .sort(); - return dirs.slice(-count).map((name) => join(evalsRoot, name)); + return dirs.slice(-count).map((name) => join(runsRoot, name)); } - return args.map((a) => (a.startsWith("/") ? a : join(evalsRoot, a))); + return args.map((a) => (a.startsWith("/") ? a : join(runsRoot, a))); } function median(values: number[]): number { From a8bcf32eb6c47ecc0dce01b6fa9cd0d1feb72397 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 02:14:48 +0200 Subject: [PATCH 2/7] fix(script-tool): accept positional string args in script stubs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A script calling the natural JS idiom read("a.ts") / run("bun test") sent a bare string to the RPC server, which coerced any non-object arg to {} — the tool then rejected for the missing field and its rejection TEXT came back as the call's RESULT, which the script silently treats as data. Observed in a migrate run: the model called read("svcN.ts") 16x inside a script, each silently returned the rejection string, its regex found "no tier", and it spiralled to a stuck-fail never seeing the reads were rejected. Stubs for tools whose entire required-arg set is a single string param (read→file, run→command, search→pattern) now wrap a bare string into the named arg. Multi-arg tools (edit/create/…) stay object-only. Map is drift-guarded against the real tool schemas by a test. --- packages/core/src/loop/tools/script-tool.ts | 34 +++++++-- packages/core/tests/script-tool.test.ts | 76 ++++++++++++++++++++- 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/packages/core/src/loop/tools/script-tool.ts b/packages/core/src/loop/tools/script-tool.ts index 89867c3..c90d399 100644 --- a/packages/core/src/loop/tools/script-tool.ts +++ b/packages/core/src/loop/tools/script-tool.ts @@ -45,15 +45,39 @@ function timeoutMs(args: Record): number { ); } +/** Script-exposable tools whose entire required-arg set is a single string param, + * so a script may call them POSITIONALLY — `read("a.ts")`, `run("bun test")`, + * `search("TODO")` — the natural JS idiom. The stub wraps the bare string into + * the named arg. Without this the RPC server coerces the non-object arg to `{}` + * (see `handle`), the tool rejects for the missing field, and the rejection TEXT + * comes back as the call's RESULT — which a script silently treats as data. + * (Observed: a `migrate` run called `read("svcN.ts")` 16× inside a script, each + * silently returned the rejection string, the script's regex found "no tier", + * and the model spiralled to a stuck-fail never seeing the reads were rejected.) + * Multi-arg tools (edit/create/…) are intentionally absent: a positional string + * is ambiguous there. Drift-guarded by a test against the real tool schemas. */ +export const SCRIPT_POSITIONAL_ARG: Readonly> = { + [TOOL_NAME.read]: "file", + [TOOL_NAME.run]: "command", + [TOOL_NAME.search]: "pattern", +}; + /** The stub module the script imports as `./tsforge-tools`. One async function per * exposed tool, each POSTing `{tool,args}` to the loopback RPC server with the - * one-time token. Plain JS (no type annotations) so `bun run` needs no config. */ + * one-time token. Plain JS (no type annotations) so `bun run` needs no config. + * Tools in `SCRIPT_POSITIONAL_ARG` also accept a bare string, wrapped into the + * named arg so the natural positional call shape works. */ export function generateToolStubs(exposed: readonly string[]): string { const fns = exposed - .map( - (name) => - `export async function ${name}(args = {}) { return __call(${JSON.stringify(name)}, args); }` - ) + .map((name) => { + const positional = SCRIPT_POSITIONAL_ARG[name]; + const coerce = + positional === undefined + ? "args ?? {}" + : `typeof args === "string" ? { ${JSON.stringify(positional)}: args } : (args ?? {})`; + + return `export async function ${name}(args = {}) { return __call(${JSON.stringify(name)}, ${coerce}); }`; + }) .join("\n"); return `// AUTO-GENERATED by tsforge — do not edit. Tool stubs for the script sandbox. diff --git a/packages/core/tests/script-tool.test.ts b/packages/core/tests/script-tool.test.ts index c11a448..e76ea53 100644 --- a/packages/core/tests/script-tool.test.ts +++ b/packages/core/tests/script-tool.test.ts @@ -6,10 +6,17 @@ import { join } from "node:path"; import { doScript, generateToolStubs, + SCRIPT_POSITIONAL_ARG, type ExecuteFn, } from "../src/loop/tools/script-tool"; import { executeTool } from "../src/loop/tools/execute-tool"; -import { READ_ONLY_TOOL_NAMES, SCRIPT_EXPOSABLE_TOOLS } from "../src/agent"; +import { + READ_ONLY_TOOL_NAMES, + READ_TOOL, + RUN_TOOL, + SEARCH_TOOL, + SCRIPT_EXPOSABLE_TOOLS, +} from "../src/agent"; import type { IToolContext } from "../src/loop/tools/tool-context"; import type { ILoopEvent } from "../src/loop/loop.types"; @@ -89,6 +96,73 @@ test("generateToolStubs emits one async fn per tool plus the __call helper", () expect(src).not.toContain("function script("); }); +test("single-string-arg tools accept a positional string; multi-arg tools don't", () => { + const src = generateToolStubs(["read", "run", "search", "create", "edit"]); + + // read/run/search wrap a bare string into their named arg… + expect(src).toContain('typeof args === "string" ? { "file": args }'); + expect(src).toContain('typeof args === "string" ? { "command": args }'); + expect(src).toContain('typeof args === "string" ? { "pattern": args }'); + // …but create/edit (multi required arg) stay object-only — no string coercion. + expect(src).toContain("export async function create(args = {}) {"); + expect(src).not.toContain('"content": args'); + expect(src).not.toContain('"oldString": args'); +}); + +test("SCRIPT_POSITIONAL_ARG maps only sole-required-string-param tools (drift guard)", () => { + // Each mapped param must be the EXACT single required arg of the real schema, + // so the map can't silently drift from the tool definitions it shadows. + const required = { + read: READ_TOOL.function.parameters.required, + run: RUN_TOOL.function.parameters.required, + search: SEARCH_TOOL.function.parameters.required, + }; + + for (const [tool, param] of Object.entries(SCRIPT_POSITIONAL_ARG)) { + expect(required[tool as keyof typeof required]).toEqual([param]); + } + + // Every mapped tool is actually script-exposable. + for (const tool of Object.keys(SCRIPT_POSITIONAL_ARG)) { + expect(SCRIPT_EXPOSABLE_TOOLS.has(tool)).toBe(true); + } +}); + +test("a positional read('file') returns content, not a silent rejection", async () => { + // Regression: the natural `read("a.ts")` idiom used to be coerced to `{}` by + // the RPC server, the tool rejected for a missing `file`, and the rejection + // TEXT came back as the read's RESULT (a script then treats it as content). + const dir = await mkdtemp(join(tmpdir(), "tsforge-script-pos-")); + + try { + await writeFile( + join(dir, "svc1.ts"), + "// tier: gold\nexport const x = 1;\n" + ); + + const events: ILoopEvent[] = []; + const code = [ + 'import { read } from "./tsforge-tools";', + 'const content = await read("svc1.ts");', + "console.log(content);", + ].join("\n"); + + const out = await doScript( + { code }, + makeCtx({ cwd: dir, files: ["svc1.ts"] }, events), + { execute: executeTool } + ); + + expect(out).toContain("tier: gold"); // real file content came back + expect(out).not.toContain("malformed args"); // NOT the rejection string + expect(events.some((e) => e.message === "tool_input_rejected:read")).toBe( + false + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + test("a script collapses N tool calls into ONE turn and returns only stdout", async () => { const calls: string[] = []; const events: ILoopEvent[] = []; From e100d7598a729bb6da4590d6509ecf33e8e0d3dc Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 02:52:52 +0200 Subject: [PATCH 3/7] fix(script-tool): surface in-script tool-call rejections as thrown errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deeper root cause behind the positional-args fix: ANY tool call rejected inside a script returned its rejection reason as the call's RESULT string, so the script silently treated the reason as data. The read positional fix removed one trigger, but a migrate run then hit the same class on edit — 8 `tool_input_ rejected:edit` inside one script, which printed "Updated …" and exited 0 while the model never saw the edits failed (it guessed "the edit stub might not have worked" and recovered by editing directly, costing turns). The RPC handler now watches for a `tool_(input_)?rejected:` event during a call and returns it as an RPC error so the stub THROWS — the script fails loudly with the real reason on the first rejection instead of carrying on with bad data. A script that wants to tolerate failures can try/catch. Scope/policy denials use a different signal and are unaffected. --- packages/core/src/loop/tools/script-tool.ts | 28 +++++++++++++++++++-- packages/core/tests/script-tool.test.ts | 28 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/core/src/loop/tools/script-tool.ts b/packages/core/src/loop/tools/script-tool.ts index c90d399..55603d3 100644 --- a/packages/core/src/loop/tools/script-tool.ts +++ b/packages/core/src/loop/tools/script-tool.ts @@ -170,11 +170,35 @@ function startRpcServer(ctx: IToolContext, deps: IScriptDeps): IScriptServer { message: `↳ script:${tool}`, }); + // A tool call that FAILS inside a script reports a `tool_(input_)?rejected:` + // event and returns the rejection reason as its result STRING. Handing that + // back as a normal value lets the script silently treat the reason as data — + // a migrate run edited 8 files this way (each `tool_input_rejected:edit`), + // printed "Updated …", exited 0, and the model never saw the edits failed. + // Watch for the rejection event and surface it as an RPC error so the stub + // THROWS: the script fails loudly with the real reason instead of carrying on. + const rejections: string[] = []; + const watched: IToolContext = { + ...ctx, + report: (event) => { + if ( + event.kind === "tool" && + /^tool_(?:input_)?rejected:/u.test(event.message) + ) { + rejections.push(event.message); + } + + ctx.report(event); + }, + }; + const result = await serialize(() => - deps.execute({ name: tool, arguments: args }, ctx) + deps.execute({ name: tool, arguments: args }, watched) ); - return { result }; + return rejections.length > 0 + ? { error: `${tool} rejected: ${result}` } + : { result }; } const server = Bun.serve({ diff --git a/packages/core/tests/script-tool.test.ts b/packages/core/tests/script-tool.test.ts index e76ea53..21bb6bb 100644 --- a/packages/core/tests/script-tool.test.ts +++ b/packages/core/tests/script-tool.test.ts @@ -163,6 +163,34 @@ test("a positional read('file') returns content, not a silent rejection", async } }); +test("a tool call rejected inside a script THROWS with the reason, not silent data", async () => { + // Regression: a rejected in-script call used to return its rejection TEXT as a + // normal value, so the script carried on treating the reason as data (observed: + // a migrate run's 8 edits were all `tool_input_rejected:edit`, the script printed + // "Updated …" and exited 0, and the model never saw they failed). Now it throws. + const events: ILoopEvent[] = []; + const code = [ + 'import { read } from "./tsforge-tools";', + "try {", + " await read({});", // no `file` → tool_input_rejected:read + ' console.log("NO_THROW");', + "} catch (e) {", + ' console.log("THREW", e.message);', + "}", + ].join("\n"); + + const out = await doScript({ code }, makeCtx({}, events), { + execute: executeTool, + }); + + expect(out).toContain("THREW"); + expect(out).toContain("read rejected:"); // surfaced with the real reason + expect(out).not.toContain("NO_THROW"); // the call did NOT silently succeed + expect(events.some((e) => e.message === "tool_input_rejected:read")).toBe( + true + ); +}); + test("a script collapses N tool calls into ONE turn and returns only stdout", async () => { const calls: string[] = []; const events: ILoopEvent[] = []; From a2450ae5eff8b814ef3d95220b32025465668894 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 02:58:00 +0200 Subject: [PATCH 4/7] fix(eval): analyze-runs tolerates a missing evals/runs dir (review) Gemini review: readdir(runsRoot) crashes with ENOENT on a clean checkout where no sweep has run yet. Treat a missing dir as "no runs" via .catch(() => []). --- packages/core/scripts/analyze-runs.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/scripts/analyze-runs.ts b/packages/core/scripts/analyze-runs.ts index 3135a96..3c42857 100644 --- a/packages/core/scripts/analyze-runs.ts +++ b/packages/core/scripts/analyze-runs.ts @@ -205,7 +205,11 @@ async function resolveDirs(): Promise { if (args.length === 2 && /^\d+$/.test(args[1] ?? "")) { const prefix = args[0] ?? ""; const count = Number(args[1]); - const all = await readdir(runsRoot, { withFileTypes: true }); + // evals/runs/ may not exist yet on a clean checkout (no sweep has run) — + // treat that as "no runs" instead of crashing with ENOENT. + const all = await readdir(runsRoot, { withFileTypes: true }).catch( + () => [] + ); const dirs = all .filter((d) => d.isDirectory() && d.name.startsWith(prefix)) .map((d) => d.name) From fca855ae27f72306d925f903efecc8f3f23d0444 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 23:19:57 +0200 Subject: [PATCH 5/7] fix(meta-rules): count one-hop transitive coverage for test-sibling A logic file reached through a facade (test imports query.ts, which imports lexer.ts) now counts as covered, so the rule stops demanding a co-located sibling for every internal module. Bounded to a single hop. --- .../rules/testing/test-sibling-required.ts | 83 +++++++++++++++++-- packages/core/tests/meta-rules.test.ts | 55 ++++++++++++ 2 files changed, 131 insertions(+), 7 deletions(-) diff --git a/packages/core/src/meta-rules/rules/testing/test-sibling-required.ts b/packages/core/src/meta-rules/rules/testing/test-sibling-required.ts index 2e5d1cc..5c0c3e3 100644 --- a/packages/core/src/meta-rules/rules/testing/test-sibling-required.ts +++ b/packages/core/src/meta-rules/rules/testing/test-sibling-required.ts @@ -164,12 +164,71 @@ function candidateTestFiles( return [...out]; } -/** A logic file is already COVERED if some EXISTING test file imports it directly - * — the harness tests it THROUGH that test, so also demanding a co-located - * sibling makes the rule unsatisfiable for specs where ONE test file covers - * several sibling modules (observed: auth/checkout/query deadlocking the model - * to its turn cap). Direct import only — a directly-imported module is exercised - * by the test; transitive coverage is deliberately NOT counted. */ +/** Source extensions tried when resolving an extension-less local import to a + * readable module file (and its `index.*` barrel form). */ +const MODULE_EXTS = [ + ".ts", + ".tsx", + ".mts", + ".cts", + ".js", + ".jsx", + ".mjs", + ".cjs", +]; + +/** Read the source of a resolved (extension-less) local module, trying the usual + * source extensions and an `index.*` barrel. Returns its text, or null. */ +function readModuleSource( + resolved: string, + readFile: (relPath: string) => string | null +): string | null { + for (const ext of MODULE_EXTS) { + const direct = readFile(`${resolved}${ext}`); + + if (direct !== null) { + return direct; + } + } + + for (const ext of MODULE_EXTS) { + const barrel = readFile(`${resolved}/index${ext}`); + + if (barrel !== null) { + return barrel; + } + } + + return null; +} + +/** One hop: does the local module at `resolved` (extension-less) import `target` + * directly? Used to credit a facade that re-exports/uses the target. The facade's + * own path is the resolution base so its relative imports resolve correctly. */ +function facadeImportsTarget( + resolved: string, + target: string, + readFile: (relPath: string) => string | null +): boolean { + const facade = readModuleSource(resolved, readFile); + + if (facade === null) { + return false; + } + + return importSpecifiers(facade).some( + (inner) => resolveSpecifier(`${resolved}.ts`, inner) === target + ); +} + +/** A logic file is already COVERED if some EXISTING test file exercises it — + * directly (the test imports it) OR through ONE hop (the test imports a local + * facade module that imports it). Without the facade hop the rule is unsatisfiable + * for the common layered-spec shape where one integration test drives several + * internal modules via a facade — e.g. `query.test.ts` imports only `query.ts`, + * which imports `lexer/parser/executor`; demanding a co-located `lexer.test.ts` + * deadlocks the model (observed: query failing 2/3 on its turn cap). Bounded to a + * SINGLE hop (no recursion) to stay cheap and not over-credit deep chains. */ function coveredByExistingTest( root: string, file: string, @@ -186,7 +245,17 @@ function coveredByExistingTest( } for (const spec of importSpecifiers(content)) { - if (resolveSpecifier(testFile, spec) === target) { + const resolved = resolveSpecifier(testFile, spec); + + if (resolved === null) { + continue; + } + + // Direct (test imports the file) OR one hop (test imports a facade that does). + if ( + resolved === target || + facadeImportsTarget(resolved, target, readFile) + ) { return true; } } diff --git a/packages/core/tests/meta-rules.test.ts b/packages/core/tests/meta-rules.test.ts index b07e69d..b97c0c4 100644 --- a/packages/core/tests/meta-rules.test.ts +++ b/packages/core/tests/meta-rules.test.ts @@ -620,6 +620,61 @@ test("test-sibling-required: exempts logic files an EXISTING test file imports d expect(violations.length).toBe(0); }); +test("test-sibling-required: exempts a logic file a test reaches THROUGH a facade (one hop)", () => { + // The layered-spec shape that deadlocked query 2/3: query.test.ts imports only + // the facade query.ts, which imports lexer.ts. lexer is exercised through the + // facade, so no co-located lexer.test.ts should be demanded. + writeFileSync( + join(tempDir, "lexer.ts"), + "export function tokenize(s: string): string[] { return s.split(' '); }" + ); + writeFileSync( + join(tempDir, "query.ts"), + 'import { tokenize } from "./lexer";\nexport const query = (s: string): number => tokenize(s).length;' + ); + writeFileSync( + join(tempDir, "query.test.ts"), + 'import { query } from "./query";\ntest("q", () => expect(query("a b")).toBe(2));\n' + ); + + const ctx = buildMetaRuleContext(tempDir, [], ["lexer.ts"]); + const violations = runMetaRules(META_RULES, ctx).filter( + (v) => v.ruleId === "test-sibling-required" + ); + + expect(violations.length).toBe(0); +}); + +test("test-sibling-required: does NOT count coverage beyond one hop", () => { + // Bound the facade exemption: a target reachable only via TWO hops (test → top → + // mid → deep) still needs its own test, so the relaxation can't silently excuse + // arbitrarily-deep untested chains. + writeFileSync( + join(tempDir, "deep.ts"), + "export function deep(): number { return 1; }" + ); + writeFileSync( + join(tempDir, "mid.ts"), + 'import { deep } from "./deep";\nexport const mid = (): number => deep();' + ); + writeFileSync( + join(tempDir, "top.ts"), + 'import { mid } from "./mid";\nexport const top = (): number => mid();' + ); + writeFileSync( + join(tempDir, "top.test.ts"), + 'import { top } from "./top";\ntest("t", () => expect(top()).toBe(1));\n' + ); + + const ctx = buildMetaRuleContext(tempDir, [], ["deep.ts"]); + const violations = runMetaRules(META_RULES, ctx).filter( + (v) => v.ruleId === "test-sibling-required" + ); + + expect(violations.length).toBeGreaterThan(0); + expect(violations[0]?.file).toBe("deep.ts"); +}); + test("test-sibling-required: still fires for a logic file NO test imports", () => { // Guard the exemption isn't too broad: an unimported sibling still needs a test. writeFileSync( From 8f367685ddf6cd7da778f919065b7ba3a8c63788 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 23:19:57 +0200 Subject: [PATCH 6/7] fix(cli): source the @-mention picker from git (gitignore-aware, uncapped) listWorkspaceFiles now uses 'git ls-files --cached --others --exclude-standard' so the picker respects .gitignore (drops eval-run artifacts/build output) and isn't truncated by the 400-file glob bound that hid real src files. Falls back to the glob walk outside a git repo. --- packages/core/src/lib/fs/fs.ts | 34 +++++++++++++++++- .../core/tests/list-workspace-files.test.ts | 35 +++++++++++++++++-- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/packages/core/src/lib/fs/fs.ts b/packages/core/src/lib/fs/fs.ts index 9ad4c17..7ec8669 100644 --- a/packages/core/src/lib/fs/fs.ts +++ b/packages/core/src/lib/fs/fs.ts @@ -2,6 +2,7 @@ import { join } from "node:path"; import { rm, stat } from "node:fs/promises"; import { Glob } from "bun"; import type { IFileView } from "./fs.types"; +import { runArgvCommand } from "./process"; /** True when the file exists on disk (the one place this check lives). */ export function fileExists(cwd: string, path: string): Promise { @@ -257,8 +258,39 @@ export async function resolveScopeFilesForRollback( * recency order means the files you're actually working on surface at the top * instead of an alphabetical dump of the whole tree. Unstattable files sort last. */ +/** Upper bound on picker candidates so a pathological repo can't stall the + * per-file mtime stat. Far above any real source tree (this one has ~840). */ +const MAX_PICKER_FILES = 10_000; + +/** Tracked + untracked-but-not-ignored files via git — respects `.gitignore`, so + * build output, eval-run artifacts, and other generated junk never reach the + * picker. Returns null outside a git repo (or if git is unavailable), so the + * caller can fall back to a glob walk. */ +async function gitWorkspaceFiles(cwd: string): Promise { + const res = await runArgvCommand(cwd, [ + "git", + "ls-files", + "--cached", + "--others", + "--exclude-standard", + "-z", + ]); + + if (res.exitCode !== 0) { + return null; + } + + return res.stdout.split("\0").filter((f) => f.length > 0); +} + +/** Workspace files for the `@`-mention picker, most-recently-modified first. + * Sourced from git (gitignore-aware, uncapped) when in a repo, else a glob walk + * (the 400-file prompt bound). Binaries are dropped either way. */ export async function listWorkspaceFiles(cwd: string): Promise { - const files = await resolveScopeFiles(cwd, ["**/*"]); + const gitFiles = await gitWorkspaceFiles(cwd); + const files = (gitFiles ?? (await resolveScopeFiles(cwd, ["**/*"]))) + .filter((f) => !isBinaryPath(f)) + .slice(0, MAX_PICKER_FILES); const stamped = await Promise.all( files.map(async (path) => { diff --git a/packages/core/tests/list-workspace-files.test.ts b/packages/core/tests/list-workspace-files.test.ts index 60d3ee8..e769b45 100644 --- a/packages/core/tests/list-workspace-files.test.ts +++ b/packages/core/tests/list-workspace-files.test.ts @@ -1,5 +1,5 @@ import { test, expect, beforeAll, afterAll } from "bun:test"; -import { mkdtemp, rm, writeFile, utimes } from "node:fs/promises"; +import { mkdtemp, mkdir, rm, writeFile, utimes } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { listWorkspaceFiles } from "../src/lib/fs"; @@ -23,6 +23,37 @@ afterAll(async () => { await rm(dir, { recursive: true, force: true }); }); -test("listWorkspaceFiles: orders most-recently-modified first", async () => { +test("listWorkspaceFiles: orders most-recently-modified first (non-git glob fallback)", async () => { expect(await listWorkspaceFiles(dir)).toEqual(["new.ts", "mid.ts", "old.ts"]); }); + +test("listWorkspaceFiles: in a git repo, respects .gitignore (no hardcoded names)", async () => { + const repo = await mkdtemp(join(tmpdir(), "tsforge-lwf-git-")); + + const git = (args: string[]): void => { + Bun.spawnSync(["git", ...args], { cwd: repo }); + }; + + git(["init", "-q"]); + git(["config", "user.email", "t@t.t"]); + git(["config", "user.name", "t"]); + + await writeFile(join(repo, ".gitignore"), "ignored.log\nbuild/\n"); + await writeFile(join(repo, "keep.ts"), "export const x = 1;\n"); + await writeFile(join(repo, "ignored.log"), "junk\n"); + await mkdir(join(repo, "build"), { recursive: true }); + await writeFile(join(repo, "build", "out.js"), "junk\n"); + git(["add", "-A"]); + + // A brand-new, never-added file that is NOT ignored — must still show (--others). + await writeFile(join(repo, "untracked.ts"), "export const y = 2;\n"); + + const files = await listWorkspaceFiles(repo); + + expect(files).toContain("keep.ts"); // tracked source + expect(files).toContain("untracked.ts"); // untracked but not ignored + expect(files).not.toContain("ignored.log"); // gitignored → gone, no denylist needed + expect(files).not.toContain("build/out.js"); // ignored dir → gone + + await rm(repo, { recursive: true, force: true }); +}); From 2249720bab699a7f31650090a3eeb2f5a8d2a113 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Mon, 29 Jun 2026 23:20:10 +0200 Subject: [PATCH 7/7] fix(editor): correct cursor/stream rendering, native @ completion, conversation UI, esc-to-clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editor-mode (default) rendering fixes: - Cursor desync: anchor the editor block ON the input row so typed text lands where the cursor is (was rendering one row above). - Streamed-response merge: a bar/spinner repaint between tokens clobbered the terminal's single cursor-save slot (ESC7/ESC8), dropping response text onto the input row. Editor-mode bar repaints now restore via absolute CUP instead. - Raw-mode newlines: writeStream normalizes to CRLF so streamed output doesn't staircase. Features: - @ completion is now editor-native (dropdown above the block, editor owns input) instead of the readline picker that fought the editor surface. - Conversation UI: '▌ you' / '▌ ' speaker blocks with indented bodies, your turns in brand color. - Esc clears the whole input (Ctrl+Z restores); fixed decodeKeys dropping a bare ESC. All covered by a real controller+StatusBar e2e harness (VirtualScreen) that asserts where text and cursor actually land. --- packages/core/src/cli.ts | 107 +++++- packages/core/src/editor/buffer.ts | 10 + packages/core/src/editor/controller.ts | 245 +++++++++++++- packages/core/src/editor/keys.ts | 5 + packages/core/src/render/ansi.ts | 40 ++- packages/core/src/render/index.ts | 3 + packages/core/src/render/render.types.ts | 2 + packages/core/src/render/status-bar.ts | 171 ++++++++-- packages/core/tests/editor-controller.test.ts | 117 ++++++- packages/core/tests/editor-e2e.test.ts | 12 +- packages/core/tests/editor-render-e2e.test.ts | 320 ++++++++++++++++++ packages/core/tests/status-bar.test.ts | 127 +++++-- 12 files changed, 1075 insertions(+), 84 deletions(-) create mode 100644 packages/core/tests/editor-render-e2e.test.ts diff --git a/packages/core/src/cli.ts b/packages/core/src/cli.ts index f373971..1eada70 100644 --- a/packages/core/src/cli.ts +++ b/packages/core/src/cli.ts @@ -9,6 +9,7 @@ import { formatHelp, takesArg } from "./cli/commands"; import { pickCommand } from "./render/command-menu"; import { pickFileInline, + filterFiles, formatCompletionRows, shouldOpenAtPicker, type IPickerView, @@ -75,6 +76,9 @@ import { renderEvent, renderMessage, renderStatus, + speakerLabel, + indentBlock, + BLOCK_INDENT, StatusBar, MIN_ROWS, welcomeBanner, @@ -633,7 +637,9 @@ function printHeader(info: { process.stdout.write("\n── resuming conversation ──\n"); for (const message of resumed.messages) { - process.stdout.write(renderMessage(message, { color: true })); + process.stdout.write( + renderMessage(message, { color: true, speaker: model.model }) + ); } process.stdout.write("\n──────────────────────────\n"); @@ -1490,14 +1496,55 @@ async function repl(args: ICliArgs): Promise { // size instead of clipping the current line at its pre-resize dimensions. let resizeEditor: ((columns: number, rows: number) => void) | null = null; + // Each agent turn renders as a "▌ " block with its body indented under the + // label (mirrors the user block). The label is emitted once, on the turn's first + // streamed output; `agentTurnOpen` is reset at the start of every runLine. + let agentTurnOpen = false; + let agentAtLineStart = true; + + // Indent each streamed line under the agent label. Stateful so indentation is + // correct even when a line is split across chunks (tokens). ANSI codes carry no + // newlines, so they're treated as ordinary characters and never mis-indented. + const indentAgentChunk = (text: string): string => { + let out = ""; + + for (const ch of text) { + if (agentAtLineStart && ch !== "\n") { + out += BLOCK_INDENT; + agentAtLineStart = false; + } + + out += ch; + + if (ch === "\n") { + agentAtLineStart = true; + } + } + + return out; + }; + // Route streamed agent output through the bar so it scrolls above the pinned // input row; cleared on loop exit so later/headless writes go straight to stdout. if (useInputRow) { interactiveStream = (text): void => { - statusBar.writeStream(text); + if (!agentTurnOpen) { + agentTurnOpen = true; + agentAtLineStart = true; + statusBar.writeStream( + `\n${speakerLabel(statusInfo().model, false, true)}\n` + ); + } + + statusBar.writeStream(indentAgentChunk(text)); }; } + // Start a fresh agent block for each turn (the label re-emits on its first output). + const beginAgentTurn = (): void => { + agentTurnOpen = false; + }; + // Mirror readline's buffer onto the input row after each keypress. setImmediate // lets readline update rl.line/rl.cursor first (it processes the key async). const syncInput = (): void => { @@ -1626,7 +1673,10 @@ async function repl(args: ICliArgs): Promise { // never echoed to scrollback — record it ourselves so the transcript reads // naturally above the (now-cleared) input row. if (useInputRow) { - echo(`${STYLE.dim}›${RESET} ${line}\n`); + echo( + `\n${speakerLabel("you", true, true)}\n` + + `${STYLE.brand}${indentBlock(line)}${RESET}\n` + ); } if (busy) { @@ -1654,6 +1704,7 @@ async function repl(args: ICliArgs): Promise { // Handle one idle line (slash command or a message), then any queued follow-up. const runLine = async (line: string): Promise => { busy = true; + beginAgentTurn(); // the agent's response opens a fresh "▌ " block try { if (line.startsWith("/")) { @@ -1720,6 +1771,9 @@ async function repl(args: ICliArgs): Promise { // argument. Cancel ⇒ back to a clean prompt. Only meaningful on a TTY. const openPalette = async (): Promise => { paletteOpen = true; + // Suspend the editor's stdin ownership so the palette's keypress loop owns + // input (see openFilePicker). Resumed in finally. + editorHandle?.suspend(); try { const picked = await pickCommand(process.stdout.isTTY); @@ -1749,6 +1803,13 @@ async function repl(args: ICliArgs): Promise { } finally { paletteOpen = false; + // Hand stdin back to the editor and repaint its input row (the overlay + // cleared it). No-op in readline mode (editorHandle is null). + if (editorHandle !== null) { + editorHandle.resume(); + repaintEditor(editorHandle); + } + if (useInputRow) { statusBar.update(statusInfo()); @@ -1767,6 +1828,10 @@ async function repl(args: ICliArgs): Promise { // the `@`; at send time `@path` expands to the file's contents (see runSend). const openFilePicker = async (): Promise => { paletteOpen = true; + // In editor mode the editor owns stdin via a `data` listener; suspend it so + // the inline picker's own `keypress` loop isn't fighting the editor for every + // keystroke (both would otherwise consume the same input). Resumed in finally. + editorHandle?.suspend(); const base = editorHandle !== null @@ -1807,6 +1872,13 @@ async function repl(args: ICliArgs): Promise { } finally { paletteOpen = false; + // Hand stdin back to the editor and repaint its input row (the overlay + // cleared it). No-op in readline mode (editorHandle is null). + if (editorHandle !== null) { + editorHandle.resume(); + repaintEditor(editorHandle); + } + if (useInputRow) { statusBar.update(statusInfo()); @@ -1862,6 +1934,34 @@ async function repl(args: ICliArgs): Promise { // called here from readline. Crucially: the editor owns stdin exclusively in // editor mode, and readline is NOT created in that case. if (useEditor) { + // Editor-native `@`-completion: preload the workspace file list once, then + // filter it synchronously as the user types. The dropdown is painted ABOVE + // the editor block (not the readline input row), so it can't fight the editor + // for the cursor — the cause of the earlier display corruption. + let completionFiles: readonly string[] = []; + + void listWorkspaceFiles(args.dir).then((files) => { + completionFiles = files; + }); + + const editorCompletion = { + items: (query: string): readonly string[] => + filterFiles(completionFiles, query), + render: (items: readonly string[], selected: number): void => { + statusBar.setEditorOverlay( + formatCompletionRows( + items, + selected, + process.stdout.columns, + process.stdout.isTTY + ) + ); + }, + clear: (): void => { + statusBar.clearEditorOverlay(); + }, + }; + editorHandle = startEditor({ stdin: { on: (event: string, cb: (data: string) => void) => { @@ -1897,6 +1997,7 @@ async function repl(args: ICliArgs): Promise { rows: process.stdout.rows, openPalette, openFilePicker, + completion: editorCompletion, }); resizeEditor = (columns, rows): void => { diff --git a/packages/core/src/editor/buffer.ts b/packages/core/src/editor/buffer.ts index a95f188..6b67f21 100644 --- a/packages/core/src/editor/buffer.ts +++ b/packages/core/src/editor/buffer.ts @@ -423,6 +423,16 @@ export class EditorBuffer { } } + /** Wipe the whole buffer in one keystroke. Snapshots first, so Ctrl-Z restores + * what was typed — a clear is never a destructive dead-end. */ + clear(): void { + this.clearSticky(); + this.maybeSnapshot("clear"); + this.lines = [""]; + this.cursorLine = 0; + this.cursorCol = 0; + } + expand(): string { return this.getText().replace( /\[paste #(\d+) \+\d+ lines\]/g, diff --git a/packages/core/src/editor/controller.ts b/packages/core/src/editor/controller.ts index 0b75204..f2ef463 100644 --- a/packages/core/src/editor/controller.ts +++ b/packages/core/src/editor/controller.ts @@ -16,6 +16,12 @@ export interface IEditorHandle { * dimensions captured when it was created, so after a resize the current * line can be clipped off the (now-shorter) block or wrapped at a stale width. */ resize(columns: number, rows: number): void; + /** Detach from stdin so an overlay (file picker / command palette) can own + * keypress input without the editor consuming the same chunks. Pair with + * resume(). No-op if not open or already suspended. */ + suspend(): void; + /** Re-attach to stdin after an overlay closes. No-op unless suspended. */ + resume(): void; close(): void; } @@ -27,6 +33,20 @@ export interface IStdin { setEncoding?(encoding: string): void; } +/** An `@`-mention completion source, supplied by the host. The editor owns the + * query (text after the `@`) and the selected index; the source filters a file + * list, paints the dropdown ABOVE the editor block, and tears it down. Keeping + * rendering here (not a separate readline overlay) is what stops the picker from + * fighting the editor for the input row. */ +export interface IEditorCompletionSource { + /** Filtered, ranked candidate paths for the current query. */ + items(query: string): readonly string[]; + /** Paint the dropdown for `items` with `selected` highlighted. */ + render(items: readonly string[], selected: number): void; + /** Tear the dropdown down. */ + clear(): void; +} + export interface IStartEditorDeps { stdin: IStdin; out: (s: string) => void; @@ -39,6 +59,7 @@ export interface IStartEditorDeps { rows?: number; openPalette?: () => Promise; openFilePicker?: () => Promise; + completion?: IEditorCompletionSource; } type KeyAction = (buffer: EditorBuffer) => void; @@ -121,6 +142,9 @@ function buildKeyDispatchTable(): Map { table.set("ctrl+k", (buf) => { buf.deleteToLineEnd(); }); // kill to line end + table.set("escape", (buf) => { + buf.clear(); + }); // wipe the whole input (Ctrl+Z restores it) // Yank operations table.set("ctrl+y", (buf) => { @@ -148,6 +172,7 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { renderEditor: renderEditorFn, openPalette, openFilePicker, + completion: completionSource, } = deps; // Mutable so a terminal resize can update them (see the handle's `resize`); @@ -160,6 +185,9 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { const keyDispatchTable = buildKeyDispatchTable(); let isOpen = true; + // True while an overlay (file picker / command palette) owns stdin: the editor + // detaches its `data` listener so it doesn't also consume the overlay's keystrokes. + let suspended = false; const submitCallbacks: ((message: string) => void)[] = []; const changeCallbacks: (() => void)[] = []; const interruptCallbacks: (() => void)[] = []; @@ -170,6 +198,13 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { let historyIndex = -1; // -1 = not in history, >= 0 = viewing history item let draftText: string | null = null; let dataListener: ((chunk: string) => void) | null = null; + // Active `@`-completion: the cursor position right AFTER the `@` (the query + // anchor) and the highlighted row. null when the dropdown is closed. + let completion: { + anchorLine: number; + anchorCol: number; + selected: number; + } | null = null; function repaint(): void { if (!isOpen) { @@ -353,26 +388,182 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { buffer.insert(text); repaint(); notifyChange(); + + if (completion !== null) { + refreshCompletion(); + } else { + triggerPaletteOrPicker(); + } + } + + /** The query typed after the `@` (anchor → cursor on the anchor line). */ + function completionQuery(): string { + if (completion === null) { + return ""; + } + + const { line, col } = buffer.getCursor(); + + if (line !== completion.anchorLine || col < completion.anchorCol) { + return ""; + } + + const lineText = buffer.getText().split("\n")[line] ?? ""; + + return graphemes(lineText).slice(completion.anchorCol, col).join(""); + } + + function closeCompletion(): void { + if (completion === null) { + return; + } + + completion = null; + completionSource?.clear(); + } + + /** Recompute the dropdown for the current query, or close it if the cursor left + * the mention (moved before the `@`, onto another line, or typed whitespace). */ + function refreshCompletion(): void { + if (completion === null || completionSource === undefined) { + return; + } + + const { line, col } = buffer.getCursor(); + + if (line !== completion.anchorLine || col < completion.anchorCol) { + closeCompletion(); + + return; + } + + const query = completionQuery(); + + if (/\s/u.test(query)) { + closeCompletion(); // a space ends the mention (paths contain none) + + return; + } + + const items = completionSource.items(query); + + completion.selected = Math.max( + 0, + Math.min(completion.selected, items.length - 1) + ); + completionSource.render(items, completion.selected); + } + + function openCompletion(): void { + if (completionSource === undefined) { + return; + } + + const { line, col } = buffer.getCursor(); + + completion = { anchorLine: line, anchorCol: col, selected: 0 }; + refreshCompletion(); + } + + function moveCompletion(delta: number): void { + if (completion === null) { + return; + } + + completion.selected = Math.max(0, completion.selected + delta); + refreshCompletion(); + } + + /** Accept the highlighted candidate: replace the typed query with ` ` + * (the `@` stays — it's part of the at-mention syntax). */ + function acceptCompletion(): void { + if (completion === null || completionSource === undefined) { + return; + } + + const items = completionSource.items(completionQuery()); + const pick = items[completion.selected]; + + if (pick === undefined) { + closeCompletion(); + + return; + } + + const { col } = buffer.getCursor(); + const remove = Math.max(0, col - completion.anchorCol); + + for (let i = 0; i < remove; i += 1) { + buffer.deleteBackward(); + } + + buffer.insert(`${pick} `); + closeCompletion(); + repaint(); + notifyChange(); + } + + /** While the dropdown is open it owns navigation/accept/cancel keys. Returns + * true if the key was consumed (so normal editing is skipped). */ + function handleCompletionKey(name: string): boolean { + if (completion === null) { + return false; + } + + if (name === "up") { + moveCompletion(-1); + + return true; + } + + if (name === "down") { + moveCompletion(1); + + return true; + } + + if (name === "return" || name === "tab") { + acceptCompletion(); + + return true; + } + + if (name === "escape") { + closeCompletion(); + + return true; + } + + return false; } function triggerPaletteOrPicker(): void { const currentText = buffer.getText(); - if ( - currentText === "/" && - openPalette !== undefined && - typeof openPalette === "function" - ) { + // `/` as the sole character opens the command palette (a slash command). + if (currentText === "/" && typeof openPalette === "function") { openPalette().catch(() => { // ignore errors }); + + return; + } + + // `@` typed at a word boundary (start of line or after whitespace) opens the + // completion dropdown. Reading buffer state means it fires only on the `@` + // keypress itself, not when backspace/motion happens to land after an `@`. + const { line, col } = buffer.getCursor(); + const lineText = currentText.split("\n")[line] ?? ""; + const at = lineText[col - 1]; + const before = lineText[col - 2]; + + if (at !== "@" || !(before === undefined || /\s/u.test(before))) { + return; } - if ( - currentText === "@" && - openFilePicker !== undefined && - typeof openFilePicker === "function" - ) { + if (completionSource !== undefined) { + openCompletion(); + } else if (typeof openFilePicker === "function") { openFilePicker().catch(() => { // ignore errors }); @@ -388,6 +579,12 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { }): void { const { name, text, ctrl, alt, shift } = event; + // The open dropdown intercepts navigation/accept/cancel; printable chars and + // backspace fall through to normal editing, then refreshCompletion() re-queries. + if (handleCompletionKey(name)) { + return; + } + if (name === "return") { handleReturnKey(ctrl, alt, shift); @@ -458,9 +655,12 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { action(buffer); repaint(); notifyChange(); - } - triggerPaletteOrPicker(); + // Backspace/delete change the query; re-query (or close if the `@` is gone). + if (completion !== null) { + refreshCompletion(); + } + } } function onDataChunk(raw: string | Buffer): void { @@ -568,6 +768,27 @@ export function startEditor(deps: IStartEditorDeps): IEditorHandle { } }, + suspend(): void { + if (!isOpen || suspended) { + return; + } + + suspended = true; + + if (stdin.removeListener !== undefined) { + stdin.removeListener("data", dataListener); + } + }, + + resume(): void { + if (!isOpen || !suspended) { + return; + } + + suspended = false; + stdin.on("data", dataListener); + }, + close(): void { if (!isOpen) { return; diff --git a/packages/core/src/editor/keys.ts b/packages/core/src/editor/keys.ts index 6e48c01..b838cd9 100644 --- a/packages/core/src/editor/keys.ts +++ b/packages/core/src/editor/keys.ts @@ -266,6 +266,11 @@ function parseOneKey(chunk: string, idx: number): IParseResult | null { } } + // A bare ESC (recognized sequences were tried above) — the user pressed Escape. + if (ch === 0x1b) { + return { event: createKeyEvent("escape"), len: 1 }; + } + // Printable return tryParsePrintable(chunk, idx); } diff --git a/packages/core/src/render/ansi.ts b/packages/core/src/render/ansi.ts index b56dd83..20bc1b1 100644 --- a/packages/core/src/render/ansi.ts +++ b/packages/core/src/render/ansi.ts @@ -97,6 +97,31 @@ export function renderStatus( * get markdown/code highlighting, tool calls collapse to a one-line summary, and * the system prompt + raw tool output are omitted (context, not conversation). */ +/** Indent under a speaker label so each turn reads as its own block. */ +export const BLOCK_INDENT = " "; + +/** Indent every non-empty line of a block (blank lines stay blank — no trailing + * whitespace). Used to inset a message body under its `▌ speaker` label. */ +export function indentBlock(text: string): string { + return text + .split("\n") + .map((line) => (line.length > 0 ? BLOCK_INDENT + line : line)) + .join("\n"); +} + +/** A `▌ name` speaker label — brand+bold for you, dim for the agent. */ +export function speakerLabel( + name: string, + accent: boolean, + color: boolean +): string { + return paint( + `▌ ${name}`, + accent ? STYLE.brand + STYLE.bold : STYLE.dim, + color + ); +} + export function renderMessage( message: IChatMessage, opts: IRenderOptions = {} @@ -108,22 +133,29 @@ export function renderMessage( } if (message.role === "user") { - return `\n${paint("›", STYLE.brand + STYLE.bold, color)} ${message.content}\n`; + // `▌ you` label + brand-colored, indented body so YOUR turns read as a distinct + // block against the agent's default-foreground prose. + return ( + `\n${speakerLabel("you", true, color)}\n` + + `${paint(indentBlock(message.content), STYLE.brand, color)}\n` + ); } const parts: string[] = []; if (message.content.length > 0) { - parts.push(renderMarkdown(message.content, color)); + parts.push(indentBlock(renderMarkdown(message.content, color))); } if (message.toolCalls !== undefined && message.toolCalls.length > 0) { const names = message.toolCalls.map((c) => c.name).join(", "); - parts.push(paint(` · used ${names}`, STYLE.dim, color)); + parts.push(indentBlock(paint(`· used ${names}`, STYLE.dim, color))); } - return parts.length > 0 ? `\n${parts.join("\n")}\n` : ""; + return parts.length > 0 + ? `\n${speakerLabel(opts.speaker ?? "assistant", false, color)}\n${parts.join("\n")}\n` + : ""; } function highlightTs(code: string, color: boolean): string { diff --git a/packages/core/src/render/index.ts b/packages/core/src/render/index.ts index eb1a22e..1c5033f 100644 --- a/packages/core/src/render/index.ts +++ b/packages/core/src/render/index.ts @@ -4,6 +4,9 @@ export { renderMessage, renderStatus, statusSegments, + speakerLabel, + indentBlock, + BLOCK_INDENT, } from "./ansi"; export { StatusBar, diff --git a/packages/core/src/render/render.types.ts b/packages/core/src/render/render.types.ts index f060192..4d94763 100644 --- a/packages/core/src/render/render.types.ts +++ b/packages/core/src/render/render.types.ts @@ -1,6 +1,8 @@ export interface IRenderOptions { /** Emit ANSI color codes (terminal) vs plain text (log files). Default true. */ color?: boolean; + /** Speaker label for assistant turns (the model name). Default "assistant". */ + speaker?: string; } /** A compact post-turn status line — the "where am I" summary modern CLIs show. */ diff --git a/packages/core/src/render/status-bar.ts b/packages/core/src/render/status-bar.ts index 0bcec7b..bc810e7 100644 --- a/packages/core/src/render/status-bar.ts +++ b/packages/core/src/render/status-bar.ts @@ -276,8 +276,9 @@ export function buildInputFrame( } /** - * The escape sequence that paints a multi-row editor input block ABOVE the input row, - * cleared and redrawn in place each call. `lines` are the visual rows (wrapped); + * The escape sequence that paints a multi-row editor input block anchored ON the input + * row (its bottom line rests there; extra lines grow upward), cleared and redrawn in + * place each call. `lines` are the visual rows (wrapped); * `cursorRow` and `cursorCol` position the cursor within them. `clearRows` is the height * of the previous block so a shrinking block erases its old top rows. The block is pinned * absolutely, with the cursor left parked at the typing position. @@ -299,18 +300,17 @@ export function buildEditorFrame( void color; const { inputRow } = computeRegions({ rows }); - const maxSpan = Math.max(0, inputRow - 1); - - // The block is BOTTOM-anchored: content always occupies the bottom - // `lines.length` rows (resting just above the input row), and any cleared - // rows from a taller previous frame sit ABOVE it. `clearRows` is the previous - // block height, so the span [inputRow - count, inputRow - 1] fully covers - // wherever the prior frame painted — a shrinking block can't orphan its old - // top rows. (Top-anchoring left ghosts: the start moved down on shrink and - // the rows above it were never erased.) Mirrors buildOverlayFrame. + const maxSpan = inputRow; + + // The block is BOTTOM-anchored ONTO the input row: its last visual row rests on + // `inputRow` (the cursor's home), and extra rows grow UPWARD from there. Anchoring + // ON the input row rather than one row above it is what keeps the cursor and the + // typed text on the SAME row — anchoring above put the text one row above the + // cursor (the reported desync). `clearRows` (the previous block height) widens the + // span so a shrinking block erases its old top rows. Mirrors buildOverlayFrame. const count = Math.min(Math.max(lines.length, clearRows), maxSpan); const blank = Math.max(0, count - lines.length); // cleared rows above content - const spanTop = Math.max(1, inputRow - count); + const spanTop = Math.max(1, inputRow - count + 1); let out = ""; for (let i = 0; i < count; i += 1) { @@ -322,8 +322,8 @@ export function buildEditorFrame( out += `${ESC}[${row};1H${ESC}[2K${line}`; } - // Park the cursor relative to the bottom-anchored content top. - const contentTop = Math.max(1, inputRow - lines.length); + // Park the cursor relative to the content top (the bottom line is the input row). + const contentTop = Math.max(1, inputRow - lines.length + 1); const cursorAbsRow = contentTop + cursorRow; out += `${ESC}[${cursorAbsRow};${cursorCol + 1}H`; @@ -380,6 +380,22 @@ export class StatusBar { /** Height of the multi-row editor block currently painted above the input row (0 = none), * so the next paint knows how many old rows to erase when the block shrinks. */ private editorRows = 0; + /** True once the multi-row editor block is the active editing surface (editor + * mode). The cursor then lives IN the block, so bar repaints and stream writes + * must NOT re-park it onto the readline input row — doing so was the cursor/text + * desync (cursor on the `›` row, text landing in the block above). Set on the + * first setEditor; a session never reverts from editor to readline. */ + private editorActive = false; + /** Absolute (1-based) cursor position the editor block last parked at, so a bar + * repaint or stream write can restore it without the editor repainting. */ + private editorCursorAbsRow = 1; + private editorCursorAbsCol = 1; + /** The last editor frame (visual lines + in-block cursor), so a stream write can + * REPAINT the input block on top of itself — guaranteeing streamed output can + * never leave anything sitting on the input row. */ + private editorLines: readonly string[] = []; + private editorCursorRow = 0; + private editorCursorCol = 0; /** Damage buffer for the bar-owned rows (border, segments, input row). The bar * frames are flushed through it so a repaint emits only the cells that changed * — no full-row `ESC[2K` rewrites, no flicker. The editor block and `@`-picker @@ -474,9 +490,9 @@ export class StatusBar { const columns = this.out.columns ?? 80; const rows = this.out.rows ?? 0; - if (this.withInput) { - // Bar body + input row in one damage-diffed flush, parking on the input row - // (the stream-cursor slot is left untouched — it isn't part of this frame). + if (this.withInput && !this.editorActive) { + // Readline input-row mode: bar body + input row in one damage-diffed flush, + // parking on the input row (the stream-cursor slot is left untouched). this.flushPinned( buildBarBody(info, columns, rows, this.color) + buildInputFrame(this.line, this.cursorPos, columns, rows, this.color), @@ -486,6 +502,25 @@ export class StatusBar { return; } + if (this.editorActive) { + // Editor mode: repaint the bar body, then restore the cursor to the editor + // block via absolute CUP. CRITICAL: do NOT use ESC7/ESC8 here — the terminal + // has a single cursor-save slot, and writeStream uses it to remember where the + // streamed response continues. A bar repaint (spinner tick) between two + // response tokens would otherwise overwrite that save with the editor cursor, + // so the next token lands on the INPUT row instead of in the scroll region. + const barBody = buildBarBody(info, columns, rows, this.color); + const delta = + this.screen === undefined ? barBody : this.screen.flush(barBody); + + this.out.write( + `${delta}${ESC}[${this.editorCursorAbsRow};${this.editorCursorAbsCol}H` + ); + + return; + } + + // Bar-only (no input row): no stream cursor to protect, so save/restore is fine. this.flushPinned(buildBarBody(info, columns, rows, this.color), false); } @@ -509,13 +544,44 @@ export class StatusBar { return; } + // The editor put the terminal in raw mode (ONLCR off), so a bare "\n" is a + // line-feed with NO carriage return — streamed output would staircase to the + // right ("text jumps"). Normalize to CRLF so every line restarts at column 1. + const normalized = text.replace(/\r?\n/gu, "\r\n"); + this.out.write(`${ESC}8`); // restore stream cursor (into the region) - this.out.write(text); // scrolls within the region + this.out.write(normalized); // scrolls within the region this.out.write(`${ESC}7`); // save the advanced stream cursor this.out.write(RESET_SGR); // don't bleed mid-stream color into the input row + + if (this.editorActive) { + // Repaint the whole input block on top of itself: even if a streamed chunk + // ever reached the input row, the editor reclaims it immediately (and the + // cursor lands back in the block). The input can never be left overwritten. + this.repaintEditorBlock(); + + return; + } + this.paintInput(); } + /** Re-emit the last editor frame (clears + redraws its rows, parks the cursor in + * the block). Used after a stream write so the input block stays intact on top. */ + private repaintEditorBlock(): void { + this.out.write( + buildEditorFrame( + this.editorLines, + this.editorCursorRow, + this.editorCursorCol, + this.out.columns ?? 80, + this.out.rows ?? 0, + this.color, + this.editorRows + ) + ); + } + /** Paint the `@`-picker dropdown of `lines` just above the input row, then * repaint the bar + input row and re-park the cursor. Safe to call repeatedly as * the list filters — a shrinking list erases its old rows. Input-row mode only; @@ -593,19 +659,36 @@ export class StatusBar { const columns = this.out.columns ?? 80; const rows = this.out.rows ?? 0; - // Clamp the block height to the available rows above the input row + // Clamp the block height to the rows from the input row upward (it may use + // the input row itself now). const { inputRow } = computeRegions({ rows }); - const maxRows = Math.max(0, inputRow - 1); + const maxRows = inputRow; const clamped = lines.slice(0, maxRows); const newHeight = clamped.length; - // If editor height changes, update the scroll region to exclude the new height. - // This pins the editor block (and bar + input) so they never scroll. + // The block is now the editing surface, so bar repaints/stream writes must + // re-park onto the block, not the readline input row. + this.editorActive = true; + + // Remember where the block parks the cursor (mirrors buildEditorFrame — the + // bottom line rests on the input row), so a bar repaint or stream write can + // restore it without the editor repainting. + const contentTop = Math.max(1, inputRow - newHeight + 1); + + this.editorCursorAbsRow = contentTop + cursorRow; + this.editorCursorAbsCol = cursorCol + 1; + this.editorLines = clamped; + this.editorCursorRow = cursorRow; + this.editorCursorCol = cursorCol; + + // If editor height changes, resize the scroll region to exclude the rows the + // block uses ABOVE the input row (newHeight - 1; the input row is already in + // `reserved`). This pins the editor block + bar so they never scroll. if (newHeight !== this.editorRows) { const { regionEnd } = computeRegions({ rows, reserved: this.reserved, - editorRows: newHeight, + editorRows: Math.max(0, newHeight - 1), }); this.out.write(`${ESC}[1;${regionEnd}r`); @@ -629,6 +712,46 @@ export class StatusBar { this.editorRows = newHeight; } + /** Paint a completion dropdown of `lines` directly ABOVE the editor block (the + * block is anchored on the input row and grows upward; the dropdown sits above + * its top row), then re-park the cursor in the block so typing continues there. + * Pass `[]` (or call clearEditorOverlay) to erase it. Editor mode only. */ + setEditorOverlay(lines: readonly string[]): void { + if (!this.installed || !this.withInput) { + return; + } + + const rows = this.out.rows ?? 0; + const { inputRow } = computeRegions({ rows }); + // Top row of the editor block: it occupies [inputRow - editorRows + 1, inputRow]. + const blockTop = Math.max(1, inputRow - Math.max(1, this.editorRows) + 1); + const maxSpan = Math.max(0, blockTop - 1); + const count = Math.min(Math.max(lines.length, this.overlayRows), maxSpan); + const blank = Math.max(0, count - lines.length); // old rows blanked above the list + let out = ""; + + for (let i = 0; i < count; i += 1) { + const row = Math.max(1, blockTop - count + i); + const line = i - blank >= 0 ? (lines[i - blank] ?? "") : ""; + + out += `${ESC}[${row};1H${ESC}[2K${line}`; + } + + this.overlayRows = lines.length; + // Re-park in the editor block (where setEditor last placed the cursor). + out += `${ESC}[${this.editorCursorAbsRow};${this.editorCursorAbsCol}H`; + this.out.write(out); + } + + /** Erase the editor-block completion dropdown. Idempotent. */ + clearEditorOverlay(): void { + if (this.overlayRows === 0) { + return; + } + + this.setEditorOverlay([]); + } + /** Re-apply the scroll region after a terminal resize, then repaint. */ resize(info: IStatusInfo): void { if (!this.installed) { @@ -652,7 +775,7 @@ export class StatusBar { const { regionEnd } = computeRegions({ rows, reserved: this.reserved, - editorRows: this.editorRows, + editorRows: Math.max(0, this.editorRows - 1), }); this.out.write(`${ESC}[1;${regionEnd}r`); diff --git a/packages/core/tests/editor-controller.test.ts b/packages/core/tests/editor-controller.test.ts index a844148..94dd3cd 100644 --- a/packages/core/tests/editor-controller.test.ts +++ b/packages/core/tests/editor-controller.test.ts @@ -67,7 +67,10 @@ class FakeStdin { } } -function makeHarness() { +function makeHarness(deps?: { + openFilePicker?: () => Promise; + openPalette?: () => Promise; +}) { const stdin = new FakeStdin(); const outputs: string[] = []; const submits: string[] = []; @@ -79,6 +82,8 @@ function makeHarness() { }, columns: 80, rows: 10, + openFilePicker: deps?.openFilePicker, + openPalette: deps?.openPalette, }); handle.onSubmit((message: string) => { @@ -403,4 +408,114 @@ describe("EditorController", () => { stdin.feed("\r"); expect(submits).toEqual(["test\nmore"]); }); + + test("Escape clears the whole input, and undo restores it", () => { + const { stdin, handle } = makeHarness(); + + stdin.feed("a draft I want to wipe"); + expect(handle.getBuffer().getText()).toBe("a draft I want to wipe"); + + stdin.feed("\x1b"); // Escape → clear everything + expect(handle.getBuffer().getText()).toBe(""); + + handle.getBuffer().undo(); // Ctrl+Z path → restore + expect(handle.getBuffer().getText()).toBe("a draft I want to wipe"); + }); + + test("Escape across multiple lines clears them all", () => { + const { stdin, handle } = makeHarness(); + + stdin.feed("line one"); + stdin.feed("\x1b[13;2u"); // Shift+Enter → newline + stdin.feed("line two"); + expect(handle.getBuffer().getText()).toBe("line one\nline two"); + + stdin.feed("\x1b"); + expect(handle.getBuffer().getText()).toBe(""); + }); +}); + +describe("EditorController @/ overlay triggers", () => { + test("typing `@` at the start opens the file picker", () => { + let calls = 0; + const { stdin } = makeHarness({ + openFilePicker: async () => { + calls += 1; + }, + }); + + stdin.feed("@"); + expect(calls).toBe(1); + }); + + test("`@` after whitespace opens the file picker", () => { + let calls = 0; + const { stdin } = makeHarness({ + openFilePicker: async () => { + calls += 1; + }, + }); + + stdin.feed("see "); + stdin.feed("@"); + expect(calls).toBe(1); + }); + + test("`@` glued to a word does NOT open the file picker", () => { + let calls = 0; + const { stdin } = makeHarness({ + openFilePicker: async () => { + calls += 1; + }, + }); + + stdin.feed("foo"); + stdin.feed("@"); // "foo@" — not a mention boundary + expect(calls).toBe(0); + }); + + test("`/` as the sole character opens the command palette", () => { + let palette = 0; + let picker = 0; + const { stdin } = makeHarness({ + openPalette: async () => { + palette += 1; + }, + openFilePicker: async () => { + picker += 1; + }, + }); + + stdin.feed("/"); + expect(palette).toBe(1); + expect(picker).toBe(0); + }); + + test("`/` not at the start does not open the palette", () => { + let palette = 0; + const { stdin } = makeHarness({ + openPalette: async () => { + palette += 1; + }, + }); + + stdin.feed("a"); + stdin.feed("/"); + expect(palette).toBe(0); + }); + + test("suspend() detaches stdin; resume() re-attaches", () => { + const { stdin, handle } = makeHarness(); + + stdin.feed("a"); + expect(handle.getBuffer().getText()).toBe("a"); + + handle.suspend(); + stdin.feed("b"); // ignored while an overlay owns input + expect(handle.getBuffer().getText()).toBe("a"); + + handle.resume(); + stdin.feed("c"); + expect(handle.getBuffer().getText()).toBe("ac"); + }); }); diff --git a/packages/core/tests/editor-e2e.test.ts b/packages/core/tests/editor-e2e.test.ts index 1dc5d9b..20a2c18 100644 --- a/packages/core/tests/editor-e2e.test.ts +++ b/packages/core/tests/editor-e2e.test.ts @@ -397,8 +397,9 @@ describe("editor e2e — aggressive interaction probes", () => { const { row, col } = h.screen().cursorPosition(); - // inputRow = 22, single line bottom-anchored at row 21, cursor after "hi". - expect(row).toBe(21); + // Single line anchored ON the input row (22) — where the cursor's home is — + // so text and cursor share the row (no "text one line above the cursor"). + expect(row).toBe(22); expect(col).toBe(3); // 1-based: after 2 graphemes }); @@ -573,9 +574,10 @@ describe("editor e2e — wrapped-line cursor math", () => { const { row, col } = h.screen().cursorPosition(); - // Block is 2 visual rows, bottom-anchored: contentTop = 22 - 2 = 20, - // cursor on visual row 1 (the wrapped tail) → row 21, after 5 chars → col 6. - expect(row).toBe(21); + // Block is 2 visual rows, bottom-anchored ON the input row: contentTop = + // 22 - 2 + 1 = 21, cursor on visual row 1 (the wrapped tail) → row 22, after + // 5 chars → col 6. + expect(row).toBe(22); expect(col).toBe(6); }); diff --git a/packages/core/tests/editor-render-e2e.test.ts b/packages/core/tests/editor-render-e2e.test.ts new file mode 100644 index 0000000..f7ffe98 --- /dev/null +++ b/packages/core/tests/editor-render-e2e.test.ts @@ -0,0 +1,320 @@ +import { describe, expect, test } from "bun:test"; +import { startEditor } from "../src/editor/controller"; +import type { IEditorCompletionSource } from "../src/editor/controller"; +import { StatusBar } from "../src/render"; +import type { IStatusInfo } from "../src/render"; +import { filterFiles, formatCompletionRows } from "../src/render/file-menu"; +import { VirtualScreen } from "./helpers/virtual-screen"; + +/** + * TRUE end-to-end render test: wire the real editor controller to the real + * StatusBar exactly as cli.ts does (editor's renderEditor callback → setEditor, + * out → writeStream), feed real keystrokes through the stdin stub, then replay + * every emitted byte into a VirtualScreen and assert where the TEXT and the + * CURSOR actually land. This catches the "text one row above the cursor" desync + * that escape-string assertions miss — no live terminal needed. + */ + +const ROWS = 24; +const COLS = 80; + +const INFO: IStatusInfo = { + model: "model", + contextTokens: 0, + contextWindow: 1000, + turns: 0, + elapsedMs: 0, + status: "ready", + scope: "workspace", +}; + +class FakeTerm { + readonly writes: string[] = []; + readonly isTTY = true; + + constructor( + public rows = ROWS, + public columns = COLS + ) {} + + write(data: string): boolean { + this.writes.push(data); + + return true; + } + + text(): string { + return this.writes.join(""); + } +} + +class FakeStdin { + private readonly dataCbs = new Set<(d: string) => void>(); + + on(event: string, cb: (d: string) => void): void { + if (event === "data") { + this.dataCbs.add(cb); + } + } + + removeListener(event: string, cb: (d: string) => void): void { + if (event === "data") { + this.dataCbs.delete(cb); + } + } + + setRawMode(): void {} + resume(): void {} + setEncoding(): void {} + + feed(s: string): void { + this.dataCbs.forEach((cb) => { + cb(s); + }); + } +} + +const FILES = ["src/lexer.ts", "src/parser.ts", "src/query.ts"]; + +function harness(withCompletion = false) { + const term = new FakeTerm(); + const bar = new StatusBar(term, true, false, true); + + bar.install(INFO); + + // Mirrors cli.ts's editor-native completion source (filter a list, paint the + // dropdown above the block via setEditorOverlay). + const completion: IEditorCompletionSource = { + items: (query: string) => filterFiles(FILES, query), + render: (items, selected) => { + bar.setEditorOverlay(formatCompletionRows(items, selected, COLS, true)); + }, + clear: () => { + bar.clearEditorOverlay(); + }, + }; + + const stdin = new FakeStdin(); + const handle = startEditor({ + stdin, + out: (s: string) => { + bar.writeStream(s); + }, + renderEditor: (lines, cursorRow, cursorCol) => { + bar.setEditor(lines, cursorRow, cursorCol); + }, + columns: COLS, + rows: ROWS, + completion: withCompletion ? completion : undefined, + }); + + const render = (): VirtualScreen => { + const screen = new VirtualScreen(ROWS, COLS); + + screen.feed(term.text()); + + return screen; + }; + + return { term, bar, stdin, handle, render }; +} + +describe("editor render e2e (real controller + StatusBar)", () => { + test("typing does NOT move the text off the cursor's home row (the reported bug)", () => { + const { stdin, render } = harness(); + + // The cursor's home BEFORE any typing (where it blinks at the prompt). + const homeRow = render().cursorPosition().row; + + stdin.feed("dsad"); + + const screen = render(); + const cur = screen.cursorPosition(); + + // The bug was: text rendered one row ABOVE where the cursor was sitting. The + // invariant the old tests missed — text must land on the cursor's home row. + expect(cur.row).toBe(homeRow); + expect(screen.row(cur.row)).toContain("dsad"); + // Cursor rests just past the 4 typed chars. + expect(cur.col).toBe(5); + // The text appears exactly once (no ghost copy on another row). + expect(screen.rowsContaining("dsad")).toBe(1); + }); + + test("multi-line: the cursor lands on the second line's text", () => { + const { stdin, render } = harness(); + + stdin.feed("aa"); + stdin.feed("\x1b[13;2u"); // Kitty Shift+Enter → newline, no submit + stdin.feed("bb"); + + const screen = render(); + const cur = screen.cursorPosition(); + + expect(screen.row(cur.row)).toContain("bb"); + expect(screen.rowsContaining("aa")).toBe(1); + expect(screen.rowsContaining("bb")).toBe(1); + }); + + test("streamed agent output uses CRLF (no staircase) and stays above the input", () => { + const { stdin, bar, render } = harness(); + + stdin.feed("howdy"); + // The agent streams a multi-line response (interactiveStream → writeStream). + bar.writeStream("Line one.\n"); + bar.writeStream("Line two.\n"); + + const screen = render(); + const cur = screen.cursorPosition(); + + const rowOf = (needle: string): number => { + for (let r = 1; r <= ROWS; r += 1) { + if (screen.row(r).includes(needle)) { + return r; + } + } + + return -1; + }; + + // The input is intact on the cursor's row, untouched by the stream. + expect(screen.row(cur.row)).toContain("howdy"); + // Each streamed line begins at column 1 — raw-mode `\n` without CR would have + // staircased line two rightward (leading spaces). row() trims only the tail, + // so an exact match proves no leading indent. + expect(screen.row(rowOf("Line two."))).toBe("Line two."); + // The whole stream sits ABOVE the input row (separated, not merged). + expect(rowOf("Line one.")).toBeLessThan(cur.row); + expect(rowOf("Line two.")).toBeLessThan(cur.row); + }); + + test("a bar repaint (spinner tick) between tokens does not drop the response onto the input", () => { + const { stdin, bar, render } = harness(); + + stdin.feed("howdy"); + // The real run ticks the spinner (bar.update) between streamed tokens. The + // terminal's single cursor-save slot is shared by writeStream's stream cursor + // and the bar repaint; if update() clobbers it, later tokens land on the input + // row and the response is lost. Interleave them to prove it doesn't. + bar.writeStream("Hey there. "); + bar.update(INFO); + bar.writeStream("What can I "); + bar.update(INFO); + bar.writeStream("help with?"); + bar.update(INFO); + + const screen = render(); + const cur = screen.cursorPosition(); + + const rowOf = (needle: string): number => { + for (let r = 1; r <= ROWS; r += 1) { + if (screen.row(r).includes(needle)) { + return r; + } + } + + return -1; + }; + + // The WHOLE response survived, assembled on one row ABOVE the input — not + // truncated to the first token, not written onto the input row. + const responseRow = rowOf("Hey there. What can I help with?"); + + expect(responseRow).toBeGreaterThan(0); + expect(responseRow).toBeLessThan(cur.row); + // The input is untouched. + expect(screen.row(cur.row)).toContain("howdy"); + }); +}); + +describe("editor render e2e — @ completion", () => { + test("typing `@` opens the dropdown ABOVE the block; the editor text/cursor stay put", () => { + const { stdin, render } = harness(true); + + const homeRow = render().cursorPosition().row; + + stdin.feed("@"); + + const screen = render(); + const cur = screen.cursorPosition(); + + // The `@` stays on the cursor's home row — the cursor does NOT jump. + expect(cur.row).toBe(homeRow); + expect(screen.row(homeRow)).toContain("@"); + // The dropdown lists files on rows ABOVE the editor block (not on its row). + expect(screen.row(homeRow)).not.toContain("lexer"); + const aboveHasFiles = [homeRow - 1, homeRow - 2, homeRow - 3].some((r) => + screen.row(r).includes("lexer") + ); + + expect(aboveHasFiles).toBe(true); + }); + + test("pressing `@` after existing text does NOT move that text to another row (the bug)", () => { + const { stdin, render } = harness(true); + + stdin.feed("hello "); // type some text first + const homeRow = render().cursorPosition().row; + + stdin.feed("@"); // open completion at the word boundary + + const screen = render(); + + // The pre-existing text stays on its row exactly once — no shoving to a line above. + expect(screen.rowsContaining("hello")).toBe(1); + expect(screen.row(homeRow)).toContain("hello @"); + expect(screen.cursorPosition().row).toBe(homeRow); + }); + + test("typing filters, then Enter accepts the highlighted file as `@ `", () => { + const { stdin, handle } = harness(true); + + stdin.feed("@"); + stdin.feed("lex"); // filters to src/lexer.ts + stdin.feed("\r"); // accept + + // Buffer holds the at-mention: `@` + path + trailing space. + const [first] = filterFiles(FILES, "lex"); + const expected = `@${first ?? ""} `; + + expect(handle.getBuffer().getText()).toBe(expected); + }); + + test("arrow-down then Enter accepts the second candidate", () => { + const { stdin, handle } = harness(true); + + stdin.feed("@"); + stdin.feed("\x1b[B"); // down → select index 1 + stdin.feed("\r"); + + const second = filterFiles(FILES, "")[1]; + const expected = `@${second ?? ""} `; + + expect(handle.getBuffer().getText()).toBe(expected); + }); + + test("Esc closes the dropdown and keeps the typed text", () => { + const { stdin, handle, render } = harness(true); + + stdin.feed("@lex"); + stdin.feed("\x1b"); // escape + + // Text preserved; dropdown gone (no file rows left on screen). + expect(handle.getBuffer().getText()).toBe("@lex"); + + const screen = render(); + + expect(screen.rowsContaining("parser")).toBe(0); + }); + + test("backspacing past the `@` closes the dropdown", () => { + const { stdin, render } = harness(true); + + stdin.feed("@"); + stdin.feed("\x7f"); // backspace removes the `@` + + const screen = render(); + + expect(screen.rowsContaining("lexer")).toBe(0); + }); +}); diff --git a/packages/core/tests/status-bar.test.ts b/packages/core/tests/status-bar.test.ts index c44acb0..5eda1f4 100644 --- a/packages/core/tests/status-bar.test.ts +++ b/packages/core/tests/status-bar.test.ts @@ -366,11 +366,12 @@ describe("buildEditorFrame", () => { false ); - // Two editor lines: one per line, all cleared first - expect(frame).toContain("\x1b[20;1H\x1b[2Kline one"); - expect(frame).toContain("\x1b[21;1H\x1b[2Kline two"); - // Cursor parked at blockStart + cursorRow = 20 + 1 = 21, cursorCol + 1 = 3 + 1 = 4 - expect(frame.endsWith("\x1b[21;4H")).toBe(true); + // Two editor lines anchored ON the input row (22): bottom line on 22, the + // other just above on 21. All cleared first. + expect(frame).toContain("\x1b[21;1H\x1b[2Kline one"); + expect(frame).toContain("\x1b[22;1H\x1b[2Kline two"); + // Cursor parked at contentTop + cursorRow = 21 + 1 = 22, cursorCol + 1 = 3 + 1 = 4 + expect(frame.endsWith("\x1b[22;4H")).toBe(true); }); test("renders all provided lines (caller must clamp beforehand)", () => { @@ -394,18 +395,18 @@ describe("buildEditorFrame", () => { false ); - // blockStart = max(1, 22 - 3) = 19; cursor at 19 + 2 = 21, col 5 + 1 = 6 - expect(frame.endsWith("\x1b[21;6H")).toBe(true); + // blockStart = max(1, 22 - 3 + 1) = 20; cursor at 20 + 2 = 22, col 5 + 1 = 6 + expect(frame.endsWith("\x1b[22;6H")).toBe(true); }); test("clamps block start to row 1 on a small terminal", () => { - // On a 5-row terminal: inputRow = max(1, 5 - 2) = 3 - // blockStart = max(1, 3 - 1) = 2 (if 1 line); never goes to row 0 or negative + // On a 5-row terminal: inputRow = max(1, 5 - 2) = 3. A single line rests ON + // the input row (3); never goes to row 0 or negative. const frame = buildEditorFrame(["only"], 0, 0, 80, 5, false); expect(frame).not.toContain("\x1b[0;1H"); expect(frame).not.toContain("\x1b[-1;1H"); - expect(frame).toContain("\x1b[2;1H"); // blockStart clamped + expect(frame).toContain("\x1b[3;1H"); // bottom line on the input row }); }); @@ -425,8 +426,65 @@ describe("StatusBar with multi-row editor", () => { expect(out).toContain("first line"); expect(out).toContain("second line"); - // blockStart = 24 - 2 - 2 = 20; cursor at 20 + 1 = 21, col 4 + 1 = 5 - expect(out.endsWith("\x1b[21;5H")).toBe(true); + // Bottom line on the input row (22); contentTop = 22 - 2 + 1 = 21; cursor at + // 21 + 1 = 22, col 4 + 1 = 5. + expect(out.endsWith("\x1b[22;5H")).toBe(true); + }); + + test("the editor block overwrites the readline `›` prompt on the input row", () => { + const term = new FakeTerm(true, 24, 80); + const bar = withInput(term); + + bar.install(INFO); // paints the `›` prompt on the input row (22) + term.writes.length = 0; + + bar.setEditor(["x"], 0, 1); + + // The block's bottom line is the input row, so it clears + repaints row 22 — + // no dangling `›` prompt is left, and the text lives where the cursor is. + expect(term.text()).toContain("\x1b[22;1H\x1b[2K"); + + const screen = new VirtualScreen(24, 80); + + screen.feed(term.text()); + expect(screen.row(22).includes("›")).toBe(false); + expect(screen.row(22)).toContain("x"); + }); + + test("after setEditor, update restores the cursor via CUP, NOT the shared ESC7/ESC8 slot", () => { + const term = new FakeTerm(true, 24, 80); + const bar = withInput(term); + + bar.install(INFO); + bar.setEditor(["hello"], 0, 5); // block cursor at row 22, col 6 + term.writes.length = 0; + + bar.update({ ...INFO, status: "stuck" }); // a status refresh mid-stream + + const out = term.text(); + + // CRITICAL: editor-mode update must NOT touch the DECSC slot (ESC7/ESC8) — it's + // owned by writeStream's stream cursor. A spinner tick that clobbered it would + // drop the next response token onto the input row. So: no save/restore at all, + // and the cursor is restored to the editor block via absolute CUP instead. + expect(out.includes("\x1b7")).toBe(false); + expect(out.includes("\x1b8")).toBe(false); + expect(out.endsWith("\x1b[22;6H")).toBe(true); + }); + + test("after setEditor, writeStream re-parks into the editor block, not the input row", () => { + const term = new FakeTerm(true, 24, 80); + const bar = withInput(term); + + bar.install(INFO); + bar.setEditor(["hello"], 0, 5); // block cursor: input row 22, col 5 + 1 = 6 + term.writes.length = 0; + + bar.writeStream("agent output\n"); + + // Streamed output lands above the block, and the cursor returns to the block + // (row 22, col 6) — never re-parked anywhere else. + expect(term.text().endsWith("\x1b[22;6H")).toBe(true); }); test("setEditor is a no-op when not installed (non-TTY)", () => { @@ -444,7 +502,8 @@ describe("StatusBar with multi-row editor", () => { bar.install(INFO); term.writes.length = 0; - // Try to set 50 lines, but only 21 rows available (24 - 2 input/bar rows - 1 for safety) + // Try to set 50 lines, but only 22 rows available (up to and including the + // input row: inputRow = 24 - 2 = 22). const manyLines = Array(50).fill("line"); bar.setEditor(manyLines, 0, 0); @@ -452,7 +511,7 @@ describe("StatusBar with multi-row editor", () => { const out = term.text(); const lineMatches = out.match(/line/g); - expect((lineMatches?.length ?? 0) <= 21).toBe(true); + expect((lineMatches?.length ?? 0) <= 22).toBe(true); }); test("setEditor shrinking a block clears old top rows (no ghost rows)", () => { @@ -467,18 +526,18 @@ describe("StatusBar with multi-row editor", () => { bar.setEditor(fourLines, 3, 0); term.writes.length = 0; - // Second: shrink to 1 row. The block is BOTTOM-anchored (content rests just - // above the input row at row 21); the old top rows from the 4-row frame must - // be explicitly cleared. clearRows = previous height (4), so the span is - // rows 18-21: rows 18-20 are blanked, the single line lands at row 21. + // Second: shrink to 1 row. The block is BOTTOM-anchored ON the input row (22); + // the old top rows from the 4-row frame must be explicitly cleared. clearRows = + // previous height (4), so the span is rows 19-22: rows 19-21 are blanked, the + // single line lands at row 22. bar.setEditor(["only line"], 0, 0); const out = term.text(); - expect(out).toContain("\x1b[18;1H\x1b[2K"); // row 18 cleared (old top row) - expect(out).toContain("\x1b[19;1H\x1b[2K"); // row 19 cleared + expect(out).toContain("\x1b[19;1H\x1b[2K"); // row 19 cleared (old top row) expect(out).toContain("\x1b[20;1H\x1b[2K"); // row 20 cleared - expect(out).toContain("\x1b[21;1H\x1b[2Konly line"); // content bottom-anchored + expect(out).toContain("\x1b[21;1H\x1b[2K"); // row 21 cleared + expect(out).toContain("\x1b[22;1H\x1b[2Konly line"); // content on the input row // The old top rows must be blanked, not left holding stale "line N" text. expect(out).not.toContain("line 1"); expect(out).not.toContain("line 4"); @@ -504,9 +563,9 @@ describe("StatusBar with multi-row editor", () => { const out = term.text(); - // Scroll region must shrink to rows 1-19 (reserved=3 + editor=2) - // New regionEnd = 24 - 3 - 2 = 19 - expect(out).toContain("\x1b[1;19r"); + // Scroll region must shrink (reserved=3 + 1 extra editor row above the input). + // New regionEnd = 24 - 3 - (2 - 1) = 20 + expect(out).toContain("\x1b[1;20r"); term.writes.length = 0; @@ -515,9 +574,9 @@ describe("StatusBar with multi-row editor", () => { const out2 = term.text(); - // Scroll region expands back to rows 1-20 (reserved=3 + editor=1) - // New regionEnd = 24 - 3 - 1 = 20 - expect(out2).toContain("\x1b[1;20r"); + // A 1-row editor uses only the input row (0 extra rows), so the region returns + // to the install default. regionEnd = 24 - 3 - 0 = 21 + expect(out2).toContain("\x1b[1;21r"); }); test("setEditor clamps editor height so scroll region stays >= 1 row", () => { @@ -525,9 +584,8 @@ describe("StatusBar with multi-row editor", () => { const bar = withInput(term); bar.install(INFO); - // On a 5-row terminal: reserved=3, so input row is at row 3 - // maxRows for editor = max(0, 3 - 1) = 2 - // But if we requested 50, it clamps to 2 + // On a 5-row terminal: reserved=3, input row at row 3, maxRows = inputRow = 3. + // 50 lines clamp to 3; extra rows above the input = 2. const many = Array(50).fill("line"); @@ -535,7 +593,6 @@ describe("StatusBar with multi-row editor", () => { const out = term.text(); - // Editor height should be clamped to 2 rows // regionEnd = 5 - 3 - 2 = 0 → clamped to max(1, 0) = 1 // So scroll region is \x1b[1;1r (just 1 row for streaming) expect(out).toContain("\x1b[1;1r"); @@ -547,8 +604,8 @@ describe("StatusBar with multi-row editor", () => { bar.install(INFO); bar.setEditor(["line 1", "line 2", "line 3"], 0, 0); - // regionEnd = 24 - 3 - 3 = 18 - expect(term.text()).toContain("\x1b[1;18r"); + // regionEnd = 24 - 3 - (3 - 1) = 19 (2 editor rows above the input row) + expect(term.text()).toContain("\x1b[1;19r"); term.rows = 30; // resize taller term.writes.length = 0; @@ -556,8 +613,8 @@ describe("StatusBar with multi-row editor", () => { const out = term.text(); - // After resize: regionEnd = 30 - 3 - 3 = 24 - expect(out).toContain("\x1b[1;24r"); + // After resize: regionEnd = 30 - 3 - (3 - 1) = 25 + expect(out).toContain("\x1b[1;25r"); }); test("teardown resets scroll region and clears editor block height", () => {