From d5b4e5c50e9d9c7914b8cf4b2b7cb9ac5db4de84 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 15:56:51 -0400 Subject: [PATCH 1/7] feat(agent): extend RTK rewrite hook to chains, guarded find, and rg Rewrite eligible segments inside top-level &&/; chains, guard rtk find behind an allowlist of supported primaries with a pipefail pipe fallback for pure file-list shapes, and route rg through native rtk rg when it is a real PATH executable (pipe fallback when it is a shell function). Adds scripts/rtk-bench.mjs, a deterministic benchmark applying the real hook policy. Generated-By: PostHog Code Task-Id: 5330cf5e-2ca2-4f33-82e9-173dd4a01af6 --- .../src/adapters/claude/session/rtk.test.ts | 138 ++++++++- .../agent/src/adapters/claude/session/rtk.ts | 281 +++++++++++++++++- scripts/rtk-bench.mjs | 154 ++++++++++ 3 files changed, 552 insertions(+), 21 deletions(-) create mode 100644 scripts/rtk-bench.mjs diff --git a/packages/agent/src/adapters/claude/session/rtk.test.ts b/packages/agent/src/adapters/claude/session/rtk.test.ts index 8c7f89ba46..a5c73a74da 100644 --- a/packages/agent/src/adapters/claude/session/rtk.test.ts +++ b/packages/agent/src/adapters/claude/session/rtk.test.ts @@ -9,6 +9,7 @@ import { detectRtkBinary, resolveRtkPrefix, rewriteBashForRtk, + rgOnPath, } from "./rtk"; describe("rewriteBashForRtk", () => { @@ -47,12 +48,30 @@ describe("rewriteBashForRtk", () => { ["npm test"], ["cat file.ts"], ["echo hello"], - // Shell operators mean more than one invocation — never rewrite. + // Pipes, redirection, and substitution disqualify the segment they're in. ["git status | grep foo"], - ["git status && ls"], ["grep foo src > out.txt"], - ["ls; pwd"], ["echo $(git status)"], + ["git status || echo failed"], + ["ls & wait"], + // Chains whose segments are all ineligible stay untouched. + ["pnpm install && pnpm test"], + ["cd /tmp && cat file.ts"], + // Unbalanced quotes — the splitter can't model the line, fail safe. + ["ls 'unclosed && git status"], + // find actions that execute, delete, or reformat output — never touched: + // the pipe fallback's find filter would mis-summarize non-list output. + ["find . -name '*.tmp' -exec rm {} \\;"], + ["find . -name '*.log' -delete"], + ["find . -type f -print0"], + ["find . -maxdepth 1 -ls"], + ["find . -printf '%p %s\\n'"], + // rg output shapes the grep pipe filter would corrupt stay raw. + ["rg --json foo src"], + ["rg -l foo src"], + ["rg --files src"], + ["rg -c foo src"], + ["rg --stats foo src"], // A leading env assignment or explicit path is not a bare allowlisted head. ["FOO=bar git status"], ["/usr/bin/git status"], @@ -63,8 +82,96 @@ describe("rewriteBashForRtk", () => { expect(rewriteBashForRtk(input, "rtk")).toBeNull(); }); - test("is idempotent — does not double-wrap", () => { - expect(rewriteBashForRtk("rtk git status", "rtk")).toBeNull(); + test.each([ + // Every eligible segment in a chain gets the prefix. + ["git status && ls", "rtk git status && rtk ls"], + ["ls; pwd", "rtk ls; pwd"], + [ + "grep -rn foo src && git diff --stat", + "rtk grep -rn foo src && rtk git diff --stat", + ], + // Ineligible segments run untouched alongside rewritten ones. + ["cd /tmp && ls -la", "cd /tmp && rtk ls -la"], + ["pnpm build && git status", "pnpm build && rtk git status"], + ["git add -A && git status", "git add -A && rtk git status"], + // A piped segment stays raw while its siblings are rewritten. + [ + "git log --oneline | head -5; git status", + "git log --oneline | head -5; rtk git status", + ], + // Operators inside quotes don't split or disqualify. + [ + `grep -rn "a && b" src && git status`, + `rtk grep -rn "a && b" src && rtk git status`, + ], + ["grep 'x;y' src", "rtk grep 'x;y' src"], + // rtk-supported find primaries are still rewritten… + [ + "find src -type f -name '*.ts' -maxdepth 2", + "rtk find src -type f -name '*.ts' -maxdepth 2", + ], + // find predicates rtk's parser rejects but that still emit a plain file + // list fall back to compacting stdout through rtk's pipe filter, inside a + // pipefail subshell so the command's own exit code survives the pipe. + [ + "find . -not -path '*/node_modules/*'", + "( set -o pipefail; find . -not -path '*/node_modules/*' | rtk pipe -f find )", + ], + [ + "find . -mtime -1", + "( set -o pipefail; find . -mtime -1 | rtk pipe -f find )", + ], + [ + "find . ! -name '*.md'", + "( set -o pipefail; find . ! -name '*.md' | rtk pipe -f find )", + ], + // …and each find shape resolves independently inside a chain. + [ + "find src -name '*.ts' && find . -not -path '*/dist/*'", + "rtk find src -name '*.ts' && ( set -o pipefail; find . -not -path '*/dist/*' | rtk pipe -f find )", + ], + // rg runs natively (it is often a shell function rtk cannot exec) with + // stdout compacted through the grep pipe filter. + [ + "rg -n foo packages/agent/src", + "( set -o pipefail; rg -n foo packages/agent/src | rtk pipe -f grep )", + ], + [ + "cd /tmp && rg -i pattern .", + "cd /tmp && ( set -o pipefail; rg -i pattern . | rtk pipe -f grep )", + ], + ])("rewrites chained %j", (input, expected) => { + expect(rewriteBashForRtk(input, "rtk")).toBe(expected); + }); + + test.each([ + // Native `rtk rg` compresses far harder than the pipe filter, but only + // works when rg is a real executable rtk can exec. + ["rg -n foo packages/agent/src", "rtk rg -n foo packages/agent/src"], + ["cd /tmp && rg -i pattern .", "cd /tmp && rtk rg -i pattern ."], + ])("routes rg natively when rg is on PATH: %j", (input, expected) => { + expect(rewriteBashForRtk(input, "rtk", { rgOnPath: true })).toBe(expected); + }); + + test("unsafe rg output flags stay raw even with rg on PATH", () => { + expect( + rewriteBashForRtk("rg --json foo src", "rtk", { rgOnPath: true }), + ).toBeNull(); + }); + + test("chained rewrite preserves untouched-segment whitespace and `;;`", () => { + expect(rewriteBashForRtk("case x in x) echo hi;; esac; ls", "rtk")).toBe( + "case x in x) echo hi;; esac; rtk ls", + ); + }); + + test.each([ + ["rtk git status", "rtk"], + ["rtk git status && rtk ls", "rtk"], + ["( set -o pipefail; find . -mtime -1 | rtk pipe -f find )", "rtk"], + ["( set -o pipefail; rg -n foo src | rtk pipe -f grep )", "rtk"], + ])("is idempotent — does not double-wrap %j", (input, prefix) => { + expect(rewriteBashForRtk(input, prefix)).toBeNull(); }); test("shell-quotes a binary path containing spaces", () => { @@ -178,6 +285,27 @@ describe("detectRtkBinary", () => { }); }); +describe("rgOnPath", () => { + let dir: string; + + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "rg-test-")); + fs.writeFileSync(path.join(dir, "rg"), "#!/bin/sh\n"); + }); + + afterAll(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("detects a real rg executable on PATH", () => { + expect(rgOnPath({ PATH: dir })).toBe(true); + }); + + test("reports false when rg is absent (e.g. only a shell function)", () => { + expect(rgOnPath({ PATH: "/nonexistent" })).toBe(false); + }); +}); + describe("createRtkRewriteHook", () => { const logger = { info() {}, diff --git a/packages/agent/src/adapters/claude/session/rtk.ts b/packages/agent/src/adapters/claude/session/rtk.ts index 0b7d256826..c4c80dd5bb 100644 --- a/packages/agent/src/adapters/claude/session/rtk.ts +++ b/packages/agent/src/adapters/claude/session/rtk.ts @@ -42,9 +42,32 @@ export const GIT_COMPRESSIBLE_SUBCOMMANDS = new Set([ "grep", ]); -// Any shell control operator means the line is more than one simple invocation; -// wrapping only its head would change the meaning of the rest. -const SHELL_OPERATORS = /[|&;<>`\n]|\$\(/; +// Operators that make a segment more than one simple invocation. `&&` and `;` +// are handled by splitting into segments first; anything left over (pipes, +// redirection, background `&`, `||`, substitution) disqualifies its segment. +// Only top-level occurrences count — quoted arguments may contain anything. +function hasTopLevelOperator(segment: string): boolean { + let quote: "'" | '"' | null = null; + for (let i = 0; i < segment.length; i++) { + const ch = segment[i]; + if (quote) { + if (ch === "\\" && quote === '"') i++; + else if (ch === quote) quote = null; + continue; + } + if (ch === "\\") { + i++; + continue; + } + if (ch === "'" || ch === '"') { + quote = ch; + continue; + } + if ("|&<>`\n".includes(ch)) return true; + if (ch === "$" && segment[i + 1] === "(") return true; + } + return false; +} // Exported so the instruction-level Codex guidance quotes the prefix the same way. export function shQuote(value: string): string { @@ -52,19 +75,192 @@ export function shQuote(value: string): string { return `'${value.replace(/'/g, `'\\''`)}'`; } +interface CommandSegment { + text: string; + separator: string; +} + /** - * Returns `command` rewritten to run through the RTK binary at `rtkPrefix`, or - * null when it isn't safe or worthwhile to rewrite. Pure and side-effect free. + * Splits a command line into top-level segments joined by `&&` or `;`, + * tracking single/double quotes and backslash escapes so operators inside + * quoted arguments don't split. Returns null when the line uses syntax the + * splitter doesn't model (unbalanced quotes), so callers fail safe. */ -export function rewriteBashForRtk( +export function splitTopLevelSegments( command: string, - rtkPrefix: string, +): CommandSegment[] | null { + const segments: CommandSegment[] = []; + let current = ""; + let quote: "'" | '"' | null = null; + for (let i = 0; i < command.length; i++) { + const ch = command[i]; + if (quote) { + current += ch; + if (ch === "\\" && quote === '"' && i + 1 < command.length) { + current += command[++i]; + } else if (ch === quote) { + quote = null; + } + continue; + } + if (ch === "\\" && i + 1 < command.length) { + current += ch + command[++i]; + continue; + } + if (ch === "'" || ch === '"') { + quote = ch; + current += ch; + continue; + } + if (ch === "&" && command[i + 1] === "&") { + segments.push({ text: current, separator: "&&" }); + current = ""; + i++; + continue; + } + if (ch === ";") { + segments.push({ text: current, separator: ";" }); + current = ""; + continue; + } + current += ch; + } + if (quote) return null; + segments.push({ text: current, separator: "" }); + return segments; +} + +// find primaries rtk's own parser accepts (verified against rtk; it hard-fails +// with exit 1 on anything else — `-not`, `!`, `-and`, `-newer`, `-mtime`, +// `-exec`, `-delete`, …). Rewriting an unsupported invocation would replace the +// file list with an error, costing a raw retry, so only invocations whose +// every `-flag` token is in this allowlist are rewritten. +const RTK_FIND_SUPPORTED_FLAGS = new Set([ + "-name", + "-iname", + "-path", + "-ipath", + "-type", + "-maxdepth", + "-mindepth", + "-print", +]); + +// rg flags that change the output away from the plain file:line:content +// matches the grep pipe filter models — compacting them would corrupt counts, +// file lists, or JSON that a consumer (or the model) reads structurally. +// rg is piped (`rg … | rtk pipe -f grep`) rather than prefix-routed because +// rg is commonly a shell function (Claude Code's embedded ripgrep), which +// `rtk rg` cannot exec. +const RG_UNSAFE_FLAGS = new Set([ + "--json", + "--files", + "-l", + "--files-with-matches", + "--files-without-match", + "-c", + "--count", + "--count-matches", + "--stats", + "-p", + "--pretty", + "-q", + "--quiet", + "-r", + "--replace", +]); + +// find actions that execute commands, write output elsewhere, or change the +// stdout format away from a plain file list. Their invocations are never +// touched: the pipe fallback's find filter would mis-summarize non-list output. +const FIND_ACTION_FLAGS = new Set([ + "-exec", + "-execdir", + "-ok", + "-okdir", + "-delete", + "-ls", + "-fls", + "-print0", + "-printf", + "-fprint", + "-fprint0", + "-fprintf", +]); + +function findFlagTokens(trimmed: string): string[] { + return trimmed + .split(/\s+/) + .slice(1) + .map((token) => + (token.startsWith("'") && token.endsWith("'")) || + (token.startsWith('"') && token.endsWith('"')) + ? token.slice(1, -1) + : token, + ); +} + +function rtkSupportsFindInvocation(trimmed: string): boolean { + for (const bare of findFlagTokens(trimmed)) { + if (bare === "!" || bare === "(" || bare === ")") return false; + if (bare.startsWith("-") && !RTK_FIND_SUPPORTED_FLAGS.has(bare)) { + return false; + } + } + return true; +} + +function findEmitsPlainFileList(trimmed: string): boolean { + return !findFlagTokens(trimmed).some((bare) => FIND_ACTION_FLAGS.has(bare)); +} + +/** + * Compacts a command's stdout through rtk's pipe filter without rtk running + * the command itself — for commands rtk can't exec (rg as a shell function) + * or can't parse (find predicates outside its allowlist). The subshell scopes + * `pipefail` so the segment's exit code is still the command's own (a bare + * pipe would report rtk's exit and flip rg's no-match 1 to 0, breaking `&&` + * short-circuits), and on re-entry its `|` disqualifies the segment, keeping + * the rewrite idempotent. + */ +function pipeFallback( + trimmed: string, + quotedPrefix: string, + filter: string, +): string { + return `( set -o pipefail; ${trimmed} | ${quotedPrefix} pipe -f ${filter} )`; +} + +export interface RtkRewriteOptions { + /** + * Whether `rg` is a real executable on PATH. Native `rtk rg` compresses far + * harder than the pipe filter (it applies the proxy's result caps and match + * summary) but execs rg itself, which fails when rg is only a shell + * function (Claude Code's embedded ripgrep). Detected once per session via + * `rgOnPath(env)`. + */ + rgOnPath?: boolean; +} + +export function rgOnPath(env: NodeJS.ProcessEnv): boolean { + return findOnPath("rg", env) !== undefined; +} + +/** + * Returns the rewritten form of one chain segment, or null to leave it + * untouched. Two shapes: ` ` when rtk can run the command itself, + * and the pipeFallback subshell when only the command's stdout can be + * compacted. + */ +function rewriteSegmentInvocation( + segment: string, + quotedPrefix: string, + options: RtkRewriteOptions, ): string | null { - const trimmed = command.trim(); - if (!trimmed || SHELL_OPERATORS.test(trimmed)) return null; + const trimmed = segment.trim(); + if (!trimmed || hasTopLevelOperator(trimmed)) return null; // Already routed through rtk — keep the rewrite idempotent. - const quotedPrefix = shQuote(rtkPrefix); if ( trimmed === quotedPrefix || trimmed.startsWith(`${quotedPrefix} `) || @@ -77,13 +273,61 @@ export function rewriteBashForRtk( if (head === "git") { const sub = gitSubcommand(trimmed); if (!sub || !GIT_COMPRESSIBLE_SUBCOMMANDS.has(sub)) return null; - } else if (!RTK_PLAIN_COMMANDS.has(head)) { + return `${quotedPrefix} ${trimmed}`; + } + if (head === "find") { + if (rtkSupportsFindInvocation(trimmed)) return `${quotedPrefix} ${trimmed}`; + if (findEmitsPlainFileList(trimmed)) { + return pipeFallback(trimmed, quotedPrefix, "find"); + } return null; } - + if (head === "rg") { + const tokens = trimmed.split(/\s+/).slice(1); + if (tokens.some((t) => RG_UNSAFE_FLAGS.has(t))) return null; + if (options.rgOnPath) return `${quotedPrefix} ${trimmed}`; + return pipeFallback(trimmed, quotedPrefix, "grep"); + } + if (!RTK_PLAIN_COMMANDS.has(head)) return null; return `${quotedPrefix} ${trimmed}`; } +/** + * Returns `command` rewritten to run through the RTK binary at `rtkPrefix`, or + * null when it isn't safe or worthwhile to rewrite. Pure and side-effect free. + * + * Lines chained with top-level `&&` or `;` are rewritten segment by segment: + * each eligible segment gets the prefix, everything else runs untouched. The + * chain operators themselves are semantics-preserving to keep — rtk proxies + * the command unchanged and forwards its exit code, so `a && b` short-circuits + * identically. Segments with pipes, redirection, `||`, or substitution are + * never rewritten. + */ +export function rewriteBashForRtk( + command: string, + rtkPrefix: string, + options: RtkRewriteOptions = {}, +): string | null { + const segments = splitTopLevelSegments(command); + if (!segments) return null; + + const quotedPrefix = shQuote(rtkPrefix); + let rewroteAny = false; + const out = segments.map(({ text, separator }) => { + const rewritten = rewriteSegmentInvocation(text, quotedPrefix, options); + if (rewritten === null) return text + separator; + rewroteAny = true; + // Splice the rewritten invocation over the trimmed span, keeping the + // segment's original surrounding whitespace so untouched syntax + // (`;;`, `do`, …) round-trips byte-identically. + const start = text.length - text.trimStart().length; + const end = start + text.trim().length; + return text.slice(0, start) + rewritten + text.slice(end) + separator; + }); + + return rewroteAny ? out.join("") : null; +} + function findOnPath(bin: string, env: NodeJS.ProcessEnv): string | undefined { const pathVar = env.PATH ?? env.Path ?? ""; const exts = @@ -155,9 +399,13 @@ export function detectRtkBinary(env: NodeJS.ProcessEnv): string | undefined { return findOnPath("rtk", env); } -export const createRtkRewriteHook = - (rtkPrefix: string, logger: Logger): HookCallback => - async (input: HookInput, _toolUseID: string | undefined) => { +export const createRtkRewriteHook = ( + rtkPrefix: string, + logger: Logger, + env: NodeJS.ProcessEnv = process.env, +): HookCallback => { + const options: RtkRewriteOptions = { rgOnPath: rgOnPath(env) }; + return async (input: HookInput, _toolUseID: string | undefined) => { if (input.hook_event_name !== "PreToolUse") return { continue: true }; if (input.tool_name !== "Bash") return { continue: true }; @@ -165,7 +413,7 @@ export const createRtkRewriteHook = const command = toolInput?.command; if (typeof command !== "string") return { continue: true }; - const rewritten = rewriteBashForRtk(command, rtkPrefix); + const rewritten = rewriteBashForRtk(command, rtkPrefix, options); if (!rewritten) return { continue: true }; logger.info(`[RtkRewriteHook] Rewriting: ${command} → ${rewritten}`); @@ -177,3 +425,4 @@ export const createRtkRewriteHook = }, }; }; +}; diff --git a/scripts/rtk-bench.mjs b/scripts/rtk-bench.mjs new file mode 100644 index 0000000000..0bdf16a359 --- /dev/null +++ b/scripts/rtk-bench.mjs @@ -0,0 +1,154 @@ +#!/usr/bin/env node +// Deterministic token benchmark of the RTK rewrite policy. Runs a fixed +// command corpus against this repo's own files, once raw and once after +// applying the REAL `rewriteBashForRtk` hook policy (imported from +// packages/agent via tsx, not reimplemented), and reports the weighted-average +// % token reduction. Token estimate is bytes/4 (rtk's own heuristic). +// +// The corpus mixes single bare commands with `&&`-chained lines, mirroring the +// harness guidance that instructs the agent to batch commands with `&&`. +// +// Usage: node scripts/rtk-bench.mjs [--json] + +import { execFileSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", +); + +const CORPUS = [ + // Single bare commands (the pre-existing rewrite surface). + { label: "grep import", cmd: `grep -rn "^import" packages/agent/src` }, + { label: "grep export", cmd: `grep -rn "^export" packages/core/src` }, + { label: "find ts", cmd: `find packages/agent/src -name "*.ts"` }, + { + label: "find test", + cmd: `find packages -name "*.test.ts" -not -path "*/node_modules/*"`, + }, + { label: "ls -la agent/src", cmd: "ls -la packages/agent/src" }, + { label: "ls -la core/src", cmd: "ls -la packages/core/src" }, + { label: "git status", cmd: "git status" }, + { label: "git branch -a", cmd: "git branch -a" }, + // &&-chained lines, as the harness's batching guidance produces. + { + label: "chain: status+diff", + cmd: "git status && git diff --stat", + }, + { + label: "chain: ls+find", + cmd: `ls -la packages/agent/src/adapters && find packages/agent/src/adapters -name "*.test.ts"`, + }, + { + label: "chain: grep+grep", + cmd: `grep -rn "describe(" packages/agent/src/adapters/claude/session && grep -rn "test.each" packages/agent/src/adapters/claude/session`, + }, + { + label: "chain: mixed heads", + cmd: `echo checking && ls packages/core/src && git branch -a`, + }, + // rg: often a shell function (Claude Code's embedded ripgrep) rather than a + // PATH binary, hence the shim in runShell. + { label: "rg import", cmd: `rg -n "^import" packages/core/src` }, + { + label: "chain: rg+status", + cmd: `rg -n "Symbol.for" packages/core/src && git status`, + }, +]; + +// Non-interactive bash lacks the profile function that exposes Claude Code's +// embedded ripgrep. Expose it as a real executable on PATH instead, matching +// a dev machine with ripgrep installed — which also lets `rtk rg` exec it. +function ensureRgOnPath() { + try { + execFileSync("bash", ["-c", "command -v rg"], { encoding: "utf8" }); + return process.env.PATH; + } catch { + const dir = path.join(os.tmpdir(), "rtk-bench-rg"); + fs.mkdirSync(dir, { recursive: true }); + const wrapper = path.join(dir, "rg"); + fs.writeFileSync( + wrapper, + `#!/bin/bash\nexec -a rg "\${CLAUDE_CODE_EXECPATH:-$HOME/.local/bin/claude}" "$@"\n`, + { mode: 0o755 }, + ); + return `${process.env.PATH}:${dir}`; + } +} + +const BENCH_PATH = ensureRgOnPath(); + +function tokensOf(text) { + return Math.ceil(Buffer.byteLength(text, "utf8") / 4); +} + +function runShell(cmd) { + try { + return execFileSync("bash", ["-c", cmd], { + cwd: repoRoot, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + env: { ...process.env, PATH: BENCH_PATH }, + }); + } catch (err) { + return err.stdout ?? ""; + } +} + +// Apply the real hook policy from packages/agent in one tsx subprocess. +function applyRewritePolicy(commands) { + const script = ` + import { rewriteBashForRtk, rgOnPath } from "./src/adapters/claude/session/rtk"; + const cmds = JSON.parse(process.env.RTK_BENCH_CMDS); + const options = { rgOnPath: rgOnPath(process.env) }; + console.log(JSON.stringify(cmds.map((c) => rewriteBashForRtk(c, "rtk", options)))); + `; + const stdout = execFileSync("pnpm", ["exec", "tsx", "-e", script], { + cwd: path.join(repoRoot, "packages/agent"), + encoding: "utf8", + env: { + ...process.env, + PATH: BENCH_PATH, + RTK_BENCH_CMDS: JSON.stringify(commands), + }, + }); + const lines = stdout.trim().split("\n"); + return JSON.parse(lines[lines.length - 1]); +} + +const rewritten = applyRewritePolicy(CORPUS.map((c) => c.cmd)); + +const rows = CORPUS.map(({ label, cmd }, i) => { + const raw = runShell(cmd); + const optimized = rewritten[i] ? runShell(rewritten[i]) : raw; + const rawTokens = tokensOf(raw); + const optTokens = tokensOf(optimized); + const pct = rawTokens > 0 ? (1 - optTokens / rawTokens) * 100 : 0; + return { label, rewritten: rewritten[i] ?? null, rawTokens, optTokens, pct }; +}); + +const totalRaw = rows.reduce((s, r) => s + r.rawTokens, 0); +const totalOpt = rows.reduce((s, r) => s + r.optTokens, 0); +const weightedPct = totalRaw > 0 ? (1 - totalOpt / totalRaw) * 100 : 0; + +if (process.argv.includes("--json")) { + console.log( + JSON.stringify({ rows, totalRaw, totalOpt, weightedPct }, null, 2), + ); +} else { + console.log("RTK Bench -- raw vs hook-policy token estimate (bytes/4)"); + console.log("=".repeat(64)); + for (const r of rows) { + console.log( + `${r.label.padEnd(22)} raw=${String(r.rawTokens).padStart(7)} opt=${String(r.optTokens).padStart(6)} ${r.pct.toFixed(1)}%${r.rewritten ? "" : " (not rewritten)"}`, + ); + } + console.log("-".repeat(64)); + console.log( + `TOTAL raw=${String(totalRaw).padStart(7)} opt=${String(totalOpt).padStart(6)} ${weightedPct.toFixed(1)}%`, + ); +} From 6b8fa900fbc5b9726357fd803bda9c823a56b281 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 15:59:20 -0400 Subject: [PATCH 2/7] feat(agent): mirror hook policy in Codex rtk guidance The Codex adapter has no command-rewrite channel, so its instruction-level guidance must track the hook policy: advertise the find predicate allowlist (rtk hard-fails on -not/-exec/! shapes), allow prefixing eligible segments of && chains, and advertise rg only when it is a real PATH executable, since rtk rg execs rg and fails where rg is only a shell function. Generated-By: PostHog Code Task-Id: 5330cf5e-2ca2-4f33-82e9-173dd4a01af6 --- .../agent/src/adapters/claude/session/rtk.ts | 5 ++- .../agent/src/adapters/rtk-guidance.test.ts | 26 ++++++++++++ packages/agent/src/adapters/rtk-guidance.ts | 42 +++++++++++++++---- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/packages/agent/src/adapters/claude/session/rtk.ts b/packages/agent/src/adapters/claude/session/rtk.ts index c4c80dd5bb..aa2802fc19 100644 --- a/packages/agent/src/adapters/claude/session/rtk.ts +++ b/packages/agent/src/adapters/claude/session/rtk.ts @@ -134,8 +134,9 @@ export function splitTopLevelSegments( // with exit 1 on anything else — `-not`, `!`, `-and`, `-newer`, `-mtime`, // `-exec`, `-delete`, …). Rewriting an unsupported invocation would replace the // file list with an error, costing a raw retry, so only invocations whose -// every `-flag` token is in this allowlist are rewritten. -const RTK_FIND_SUPPORTED_FLAGS = new Set([ +// every `-flag` token is in this allowlist are rewritten. Exported so the +// instruction-level Codex guidance advertises the same constraint. +export const RTK_FIND_SUPPORTED_FLAGS = new Set([ "-name", "-iname", "-path", diff --git a/packages/agent/src/adapters/rtk-guidance.test.ts b/packages/agent/src/adapters/rtk-guidance.test.ts index 334f36053f..cd98b5249e 100644 --- a/packages/agent/src/adapters/rtk-guidance.test.ts +++ b/packages/agent/src/adapters/rtk-guidance.test.ts @@ -54,6 +54,32 @@ describe("rtk guidance for codex", () => { const guidance = buildRtkGuidance("rtk"); expect(guidance).toContain("Never prefix `git commit`, `git push`"); }); + + // Parity with the Claude hook's find allowlist: rtk hard-fails on + // unsupported predicates, so guided models must not prefix those shapes. + test("advertises the find predicate allowlist", () => { + const guidance = buildRtkGuidance("rtk"); + expect(guidance).toContain("`-name`"); + expect(guidance).toContain("`-maxdepth`"); + expect(guidance).toContain("-not"); + expect(guidance).toContain("unprefixed"); + }); + + // rtk rg execs a real rg binary; advertising it where rg is only a shell + // function would make every guided rg command fail. + test("advertises rg only when it is a real PATH executable", () => { + expect(buildRtkGuidance("rtk", { rgOnPath: true })).toContain("`rg`"); + expect(buildRtkGuidance("rtk", { rgOnPath: false })).not.toContain( + "`rg`", + ); + expect(buildRtkGuidance("rtk")).not.toContain("`rg`"); + }); + + // Parity with the Claude hook's chain-segment rewriting. + test("allows prefixing eligible segments of && chains", () => { + const guidance = buildRtkGuidance("rtk"); + expect(guidance).toContain("cd pkg && rtk git status"); + }); }); describe("appendRtkGuidanceForCodex", () => { diff --git a/packages/agent/src/adapters/rtk-guidance.ts b/packages/agent/src/adapters/rtk-guidance.ts index 962913a32c..ead029a98d 100644 --- a/packages/agent/src/adapters/rtk-guidance.ts +++ b/packages/agent/src/adapters/rtk-guidance.ts @@ -1,7 +1,9 @@ import { GIT_COMPRESSIBLE_SUBCOMMANDS, + RTK_FIND_SUPPORTED_FLAGS, RTK_PLAIN_COMMANDS, resolveRtkPrefix, + rgOnPath, shQuote, } from "./claude/session/rtk"; @@ -15,20 +17,37 @@ import { * only integration point is the developer instructions: tell the model to * prefix eligible commands itself. * - * The advertised command set and rules mirror the Claude hook exactly - * (RTK_PLAIN_COMMANDS + GIT_COMPRESSIBLE_SUBCOMMANDS, bare invocations only, - * never commit/push), so token-usage cohorts stay comparable across adapters. + * The advertised command set and rules mirror the Claude hook (chain-segment + * prefixing, the find predicate allowlist, rg only when it is a real PATH + * executable, never commit/push), so token-usage cohorts stay comparable + * across adapters. */ -export function buildRtkGuidance(rtkPrefix: string): string { +export interface RtkGuidanceOptions { + /** + * Whether `rg` is a real executable on PATH. `rtk rg` execs rg itself, so + * advertising it where rg is only a shell function (Claude Code's embedded + * ripgrep) would make every guided rg command fail. + */ + rgOnPath?: boolean; +} + +export function buildRtkGuidance( + rtkPrefix: string, + options: RtkGuidanceOptions = {}, +): string { // Same quoting as the Claude rewrite hook: a resolved path containing // spaces must stay one shell token in the commands the model copies. const prefix = shQuote(rtkPrefix); - const plainCommands = [...RTK_PLAIN_COMMANDS].join("`, `"); + const plainCommands = [ + ...RTK_PLAIN_COMMANDS, + ...(options.rgOnPath ? ["rg"] : []), + ].join("`, `"); const gitSubcommands = [...GIT_COMPRESSIBLE_SUBCOMMANDS].join(", "); + const findFlags = [...RTK_FIND_SUPPORTED_FLAGS].join("`, `"); return `## rtk command-output compression -\`${prefix}\` is installed. It runs a command unchanged and compresses its output before you read it, so prefixed commands cost far less context. When you execute one of these as a single, bare command, prefix it with \`${prefix}\`: +\`${prefix}\` is installed. It runs a command unchanged and compresses its output before you read it, so prefixed commands cost far less context. When you execute one of these as a bare command, prefix it with \`${prefix}\`: - \`${plainCommands}\` - these git subcommands: ${gitSubcommands} @@ -36,7 +55,8 @@ export function buildRtkGuidance(rtkPrefix: string): string { Examples: \`${prefix} git status\`, \`${prefix} grep -rn "foo" src\`, \`${prefix} ls -la\`. Rules: -- Only prefix a single bare invocation. Never use it when the command is part of a pipe, uses \`&&\`, \`;\`, or redirection, or when another program parses the output — compression would corrupt what the consumer reads. +- Only prefix a bare invocation. In a command chained with \`&&\` or \`;\` you may prefix each eligible sub-command individually (e.g. \`cd pkg && ${prefix} git status\`). Never prefix a command that is part of a pipe or redirection, or whose output another program parses — compression would corrupt what the consumer reads. +- Only prefix \`find\` when it uses these predicates alone: \`${findFlags}\`. Run any other find (\`-not\`, \`-exec\`, \`!\`, \`-mtime\`, …) unprefixed — rtk rejects them with an error instead of output. - Never prefix \`git commit\`, \`git push\`, or any other command not listed above. - Skip the prefix when you need the exact, complete output (for example, copying a diff verbatim).`; } @@ -45,7 +65,8 @@ Rules: * Appends the RTK guidance to Codex developer instructions when an RTK binary * is usable. Gated on `resolveRtkPrefix` — not `detectRtkBinary` — so the * per-run `POSTHOG_RTK=0` opt-out (the cloud kill-switch flag) disables the - * guidance along with everything else. + * guidance along with everything else. rg is advertised only when it is a + * real executable on the host PATH, matching the Claude hook's routing check. */ export function appendRtkGuidanceForCodex( instructions: string, @@ -53,7 +74,10 @@ export function appendRtkGuidanceForCodex( ): string { const rtkPrefix = resolveRtkPrefix(env); if (!rtkPrefix) return instructions; - return [instructions, buildRtkGuidance(rtkPrefix)] + return [ + instructions, + buildRtkGuidance(rtkPrefix, { rgOnPath: rgOnPath(env) }), + ] .filter(Boolean) .join("\n\n"); } From e1b998458a6654d98acea17975767b8a3172745d Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 16:10:14 -0400 Subject: [PATCH 3/7] test(agent): add live RTK context-fidelity e2e suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proves RTK compression preserves the information the agent needs, end to end: three sequential turns in one session against a seeded ~130-file repo must recover an exact file+line from compressed grep output, an exact file count through the find -not pipe fallback, and per-file git state from a compressed && chain — asserted as JSON side effects against seeded ground truth, never prose. RTK_DB_PATH isolates rtk telemetry so rtk gain must report tracked commands with positive savings, making a silently disabled hook fail instead of passing vacuously. Generated-By: PostHog Code Task-Id: 5330cf5e-2ca2-4f33-82e9-173dd4a01af6 --- packages/agent/e2e/README.md | 12 + .../e2e/rtk-context-fidelity.e2e.test.ts | 244 ++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 packages/agent/e2e/rtk-context-fidelity.e2e.test.ts diff --git a/packages/agent/e2e/README.md b/packages/agent/e2e/README.md index 1cac7a1503..f4581fdbc7 100644 --- a/packages/agent/e2e/README.md +++ b/packages/agent/e2e/README.md @@ -46,6 +46,18 @@ the constrained decode. big input blob trips auto-compaction, and the adapter must surface `_posthog/compact_boundary`. +`rtk-context-fidelity.e2e.test.ts` — claude only (RTK is a deterministic +PreToolUse rewrite hook there; codex gets instruction-level guidance with +nothing assertable). Proves RTK compression preserves the information the +agent needs: over three sequential turns in one session against a seeded +~130-file repo, the agent must recover an exact file+line from compressed +grep output, an exact file count through the `find -not` pipe fallback, and +per-file git state from a compressed `&&` chain — asserted as JSON side +effects against seeded ground truth, never prose. Anti-skip-to-green: +`RTK_DB_PATH` isolates rtk telemetry to the run and `rtk gain` must report +tracked commands with positive savings, so a disabled hook fails instead of +passing vacuously. Self-skips (visibly) when rtk is not on PATH. + `guard.e2e.test.ts` — always runs: fails loudly when the token is missing (every arm would self-skip) or the codex binary is absent despite a token, so the suite can never skip itself green. diff --git a/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts new file mode 100644 index 0000000000..721dac0ef8 --- /dev/null +++ b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts @@ -0,0 +1,244 @@ +import { execFileSync } from "node:child_process"; +import { + appendFileSync, + existsSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { detectRtkBinary } from "../src/adapters/claude/session/rtk"; +import { E2E } from "./config"; +import { type Capture, cleanupRepo, openSession, setupRepo } from "./driver"; + +/** + * Live RTK context-fidelity e2e (claude arm only — the RTK integration is a + * deterministic PreToolUse rewrite hook there; codex gets instruction-level + * guidance the model may or may not follow, so nothing is assertable). + * + * The claim under test: routing Bash output through rtk compresses it WITHOUT + * deleting the information the agent needs. Each turn forces the agent to + * recover a specific ground-truth fact (file path, line number, file count, + * git state) from compressed output at a scale where rtk's filters genuinely + * engage (hundreds of files), across sequential turns in ONE session so + * compressed context compounds the way long real sessions do. Assertions are + * deterministic JSON side effects against seeded ground truth — never prose. + * + * Anti-skip-to-green: RTK_DB_PATH points rtk's telemetry at a throwaway DB; + * after the turns, `rtk gain` against that DB must show commands were tracked + * with positive savings. A run where the hook never engaged (so every fact + * came from raw output) fails here instead of passing vacuously. + */ + +const NEEDLE = "NEEDLE_e2e_7f3a9c"; +const NEEDLE_FILE = "src/mod7/service.ts"; +const NEEDLE_LINE = 47; +const FIXTURE_COUNT = 9; // *.fixture.md outside vendor/ +const MODIFIED_FILES = [ + "src/mod1/file3.ts", + "src/mod4/file0.ts", + "src/mod9/file7.ts", +]; +const UNTRACKED_FILE = "src/mod2/new-module.ts"; + +/** + * Seeds a repo big enough that rtk's compaction engages for real: 120 noise + * files across 12 dirs (~40 lines each), a needle file, fixture files inside + * and outside an excluded vendor/ tree. + */ +function seedRepo(repo: string): void { + for (let m = 0; m < 12; m++) { + const dir = join(repo, "src", `mod${m}`); + mkdirSync(dir, { recursive: true }); + for (let f = 0; f < 10; f++) { + const lines = Array.from( + { length: 40 }, + (_, l) => `export const mod${m}_file${f}_line${l} = ${l};`, + ); + writeFileSync(join(dir, `file${f}.ts`), `${lines.join("\n")}\n`); + } + } + + const serviceLines = Array.from( + { length: 80 }, + (_, l) => `export const service_line${l + 1} = ${l + 1};`, + ); + serviceLines[NEEDLE_LINE - 1] = `export const ${NEEDLE} = "recover-me";`; + writeFileSync(join(repo, NEEDLE_FILE), `${serviceLines.join("\n")}\n`); + + for (let i = 0; i < FIXTURE_COUNT; i++) { + writeFileSync( + join(repo, "src", `mod${i}`, `case${i}.fixture.md`), + `# fixture ${i}\n`, + ); + } + const vendor = join(repo, "vendor", "dep"); + mkdirSync(vendor, { recursive: true }); + for (let i = 0; i < 5; i++) { + writeFileSync(join(vendor, `vendored${i}.fixture.md`), `# vendored ${i}\n`); + writeFileSync(join(vendor, `dep${i}.ts`), `export const dep${i} = ${i};\n`); + } + + execFileSync("git", ["add", "-A"], { cwd: repo }); + execFileSync( + "git", + [ + "-c", + "commit.gpgsign=false", + "-c", + "user.email=e2e@posthog.dev", + "-c", + "user.name=e2e", + "commit", + "-qm", + "seed", + ], + { cwd: repo }, + ); +} + +function readAnswer(repo: string, name: string): Record { + const path = join(repo, name); + expect(existsSync(path), `agent did not write ${name}`).toBe(true); + return JSON.parse(readFileSync(path, "utf8")); +} + +/** The agent may answer "./src/x" or "src/x"; both name the same file. */ +function normalizePath(p: unknown): string { + return String(p).replace(/^\.\//, ""); +} + +function rtkGainSummary(dbPath: string): Record { + const stdout = execFileSync( + detectRtkBinary(process.env) as string, + ["gain", "--format", "json"], + { encoding: "utf8", env: { ...process.env, RTK_DB_PATH: dbPath } }, + ); + return (JSON.parse(stdout) as { summary: Record }).summary; +} + +const rtkBinary = detectRtkBinary(process.env); +const skip = E2E.skipReason("claude") ?? (rtkBinary ? null : "rtk not on PATH"); +const title = `rtk context fidelity (claude)${skip ? ` — SKIPPED (${skip})` : ""}`; + +describe.skipIf(!!skip)(title, () => { + let repo: string; + let rtkDbPath: string; + let savedRtkDbPath: string | undefined; + const turns: Array<{ stopReason?: string; capture: Capture }> = []; + let turnError: unknown; + + beforeAll(async () => { + E2E.configureEnv("claude"); + repo = setupRepo(); + seedRepo(repo); + // Isolate rtk telemetry to this run so the engagement assertion cannot be + // satisfied by a developer's ambient rtk history. Sibling of the repo, not + // inside it — the DB must not appear in turn 3's untracked ground truth. + rtkDbPath = `${repo}-rtk-gain.db`; + savedRtkDbPath = process.env.RTK_DB_PATH; + process.env.RTK_DB_PATH = rtkDbPath; + + const s = await openSession({ + adapter: "claude", + cwd: repo, + meta: { + systemPrompt: + "You are a coding assistant in a test repo. Follow instructions exactly.", + model: E2E.model("claude"), + permissionMode: "bypassPermissions", + taskRunId: "e2e-rtk-fidelity", + }, + }); + + const prompts = [ + // Turn 1 — grep recovery: file + line number must survive compression. + `Step 1: run exactly this single Bash command: grep -rn "${NEEDLE}" src\n` + + `Step 2: from its output, write a file named answer-grep.json containing ` + + `exactly {"file": "", "line": }. ` + + `Do nothing else, then stop.`, + // Turn 2 — find with -not (rtk-unsupported predicate → pipe fallback): + // the full file list must survive compression to count it. + `Step 1: run exactly this single Bash command: find . -type f -name "*.fixture.md" -not -path "./vendor/*"\n` + + `Step 2: count the files in its output and write a file named answer-find.json containing ` + + `exactly {"count": }. Do nothing else, then stop.`, + // Turn 3 — chained git segments: per-file status must survive compression. + `Step 1: run exactly this single Bash command: git status && git diff --stat\n` + + `Step 2: from its output, write a file named answer-git.json containing exactly ` + + `{"modified": [], ` + + `"untracked": []}. ` + + `Exclude answer-grep.json and answer-find.json from both lists. Do nothing else, then stop.`, + ]; + + try { + for (const [i, text] of prompts.entries()) { + // Mutate the worktree between turn 2 and 3 so turn 3 has real git state. + if (i === 2) { + for (const file of MODIFIED_FILES) { + appendFileSync(join(repo, file), "export const changed = true;\n"); + } + writeFileSync( + join(repo, UNTRACKED_FILE), + "export const fresh = true;\n", + ); + } + const res = await s.conn.prompt({ + sessionId: s.sessionId, + prompt: [{ type: "text", text }], + }); + turns.push({ stopReason: res.stopReason, capture: s.capture }); + } + } catch (err) { + turnError = err; + } finally { + await s.cleanup(); + } + }, 360_000); + + afterAll(() => { + if (savedRtkDbPath === undefined) delete process.env.RTK_DB_PATH; + else process.env.RTK_DB_PATH = savedRtkDbPath; + cleanupRepo(repo); + rmSync(rtkDbPath, { force: true }); + }); + + it("completes all three turns", () => { + if (turnError) throw turnError; + expect(turns).toHaveLength(3); + for (const t of turns) expect(t.stopReason).toBe("end_turn"); + }); + + it("recovers an exact file + line number from compressed grep output", () => { + if (turnError) throw turnError; + const answer = readAnswer(repo, "answer-grep.json"); + expect(normalizePath(answer.file)).toBe(NEEDLE_FILE); + expect(answer.line).toBe(NEEDLE_LINE); + }); + + it("recovers an exact file count from the find pipe fallback", () => { + if (turnError) throw turnError; + const answer = readAnswer(repo, "answer-find.json"); + expect(answer.count).toBe(FIXTURE_COUNT); + }); + + it("recovers per-file git state from a compressed && chain", () => { + if (turnError) throw turnError; + const answer = readAnswer(repo, "answer-git.json"); + const modified = (answer.modified as unknown[]).map(normalizePath).sort(); + const untracked = (answer.untracked as unknown[]).map(normalizePath); + expect(modified).toEqual([...MODIFIED_FILES].sort()); + expect(untracked).toContain(UNTRACKED_FILE); + }); + + // The teeth: rtk's own telemetry must show the hook engaged with real + // savings. Without this, a disabled hook would pass every fact check above + // against raw output and the suite would prove nothing about compression. + it("routed the session's commands through rtk with positive savings", () => { + if (turnError) throw turnError; + const summary = rtkGainSummary(rtkDbPath); + expect(summary.total_commands).toBeGreaterThanOrEqual(3); + expect(summary.total_saved).toBeGreaterThan(0); + }); +}); From b4c202d1fd9a78d2ea7868fcef5d173b69bad1d3 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 16:27:20 -0400 Subject: [PATCH 4/7] =?UTF-8?q?fix(agent):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20drop=20lossy=20pipe=20fallbacks,=20baseline-controlled=20ben?= =?UTF-8?q?ch,=20executable=20PATH=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the pipe fallbacks silently truncate large result sets (10 files per dir with +N markers), the bench had no main-policy control and could count erroring rewrites as savings, PATH detection accepted non-executable files, and the e2e sailed under rtk's truncation cap. Drops both pipe fallbacks: rtk-unsupported finds now run raw (they are used when the complete matched set matters) and rg routes only through native rtk rg when a real executable is on PATH, which matches the long-shipped grep proxy's caps and reports uncapped totals. findOnPath requires the execute bit. The bench now runs main's policy as a control arm via git show, captures exit status and stderr, and fidelity-gates every row (exit parity, output presence, per-row facts) — candidate-only failures fail the run, baseline-only failures are flagged as fixed. The e2e gains a fourth turn recovering an exact match total past rtk's 200-result cap, where only the uncapped summary header carries the truth. Generated-By: PostHog Code Task-Id: 5330cf5e-2ca2-4f33-82e9-173dd4a01af6 --- packages/agent/e2e/README.md | 16 +- .../e2e/rtk-context-fidelity.e2e.test.ts | 50 +++- .../src/adapters/claude/session/rtk.test.ts | 87 +++--- .../agent/src/adapters/claude/session/rtk.ts | 96 +++---- .../agent/src/adapters/rtk-guidance.test.ts | 2 +- scripts/rtk-bench.mjs | 272 +++++++++++++----- 6 files changed, 325 insertions(+), 198 deletions(-) diff --git a/packages/agent/e2e/README.md b/packages/agent/e2e/README.md index f4581fdbc7..0d32a4b43c 100644 --- a/packages/agent/e2e/README.md +++ b/packages/agent/e2e/README.md @@ -49,14 +49,16 @@ big input blob trips auto-compaction, and the adapter must surface `rtk-context-fidelity.e2e.test.ts` — claude only (RTK is a deterministic PreToolUse rewrite hook there; codex gets instruction-level guidance with nothing assertable). Proves RTK compression preserves the information the -agent needs: over three sequential turns in one session against a seeded +agent needs: over four sequential turns in one session against a seeded ~130-file repo, the agent must recover an exact file+line from compressed -grep output, an exact file count through the `find -not` pipe fallback, and -per-file git state from a compressed `&&` chain — asserted as JSON side -effects against seeded ground truth, never prose. Anti-skip-to-green: -`RTK_DB_PATH` isolates rtk telemetry to the run and `rtk gain` must report -tracked commands with positive savings, so a disabled hook fails instead of -passing vacuously. Self-skips (visibly) when rtk is not on PATH. +grep output, an exact file count from a guarded (raw) `find -not`, an exact +match total past rtk's 200-result truncation cap (only the uncapped summary +header carries it), and per-file git state from a compressed `&&` chain — +asserted as JSON side effects against seeded ground truth, never prose. +Anti-skip-to-green: `RTK_DB_PATH` isolates rtk telemetry to the run and +`rtk gain` must report tracked commands with positive savings, so a disabled +hook fails instead of passing vacuously. Self-skips (visibly) when rtk is +not on PATH. `guard.e2e.test.ts` — always runs: fails loudly when the token is missing (every arm would self-skip) or the codex binary is absent despite a token, so the suite diff --git a/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts index 721dac0ef8..bb1ec1b03c 100644 --- a/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts +++ b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts @@ -19,12 +19,13 @@ import { type Capture, cleanupRepo, openSession, setupRepo } from "./driver"; * guidance the model may or may not follow, so nothing is assertable). * * The claim under test: routing Bash output through rtk compresses it WITHOUT - * deleting the information the agent needs. Each turn forces the agent to - * recover a specific ground-truth fact (file path, line number, file count, - * git state) from compressed output at a scale where rtk's filters genuinely - * engage (hundreds of files), across sequential turns in ONE session so - * compressed context compounds the way long real sessions do. Assertions are - * deterministic JSON side effects against seeded ground truth — never prose. + * deleting the information the agent needs — including past rtk's truncation + * caps, where only the uncapped summary header carries the truth. Each turn + * forces the agent to recover a specific ground-truth fact (file path, line + * number, file count, an above-cap match total, git state) from compressed + * output, across sequential turns in ONE session so compressed context + * compounds the way long real sessions do. Assertions are deterministic JSON + * side effects against seeded ground truth — never prose. * * Anti-skip-to-green: RTK_DB_PATH points rtk's telemetry at a throwaway DB; * after the turns, `rtk gain` against that DB must show commands were tracked @@ -36,6 +37,10 @@ const NEEDLE = "NEEDLE_e2e_7f3a9c"; const NEEDLE_FILE = "src/mod7/service.ts"; const NEEDLE_LINE = 47; const FIXTURE_COUNT = 9; // *.fixture.md outside vendor/ +// 12 dirs × 10 files × 40 lines, every line matching "export const mod" — far +// past rtk grep's 200-shown-results cap, so the only place the true total +// survives compression is the "N matches in M files" header. +const NOISE_MATCH_TOTAL = 12 * 10 * 40; const MODIFIED_FILES = [ "src/mod1/file3.ts", "src/mod4/file0.ts", @@ -159,23 +164,29 @@ describe.skipIf(!!skip)(title, () => { `Step 2: from its output, write a file named answer-grep.json containing ` + `exactly {"file": "", "line": }. ` + `Do nothing else, then stop.`, - // Turn 2 — find with -not (rtk-unsupported predicate → pipe fallback): - // the full file list must survive compression to count it. + // Turn 2 — find with -not (rtk-unsupported predicate): the guard must + // leave it raw, so the complete list arrives and the count is exact. `Step 1: run exactly this single Bash command: find . -type f -name "*.fixture.md" -not -path "./vendor/*"\n` + `Step 2: count the files in its output and write a file named answer-find.json containing ` + `exactly {"count": }. Do nothing else, then stop.`, - // Turn 3 — chained git segments: per-file status must survive compression. + // Turn 3 — the lossy threshold: thousands of matches, far past rtk's + // 200-shown cap. The exact total only survives in the summary header. + `Step 1: run exactly this single Bash command: grep -rn "export const mod" src\n` + + `Step 2: from its output, determine the TOTAL number of matches (the output may show ` + + `a summary with the total alongside a subset of matches) and write a file named ` + + `answer-count.json containing exactly {"matches": }. Do nothing else, then stop.`, + // Turn 4 — chained git segments: per-file status must survive compression. `Step 1: run exactly this single Bash command: git status && git diff --stat\n` + `Step 2: from its output, write a file named answer-git.json containing exactly ` + `{"modified": [], ` + `"untracked": []}. ` + - `Exclude answer-grep.json and answer-find.json from both lists. Do nothing else, then stop.`, + `Exclude answer-grep.json, answer-find.json and answer-count.json from both lists. Do nothing else, then stop.`, ]; try { for (const [i, text] of prompts.entries()) { - // Mutate the worktree between turn 2 and 3 so turn 3 has real git state. - if (i === 2) { + // Mutate the worktree before the git turn so it has real state. + if (i === 3) { for (const file of MODIFIED_FILES) { appendFileSync(join(repo, file), "export const changed = true;\n"); } @@ -204,9 +215,9 @@ describe.skipIf(!!skip)(title, () => { rmSync(rtkDbPath, { force: true }); }); - it("completes all three turns", () => { + it("completes all four turns", () => { if (turnError) throw turnError; - expect(turns).toHaveLength(3); + expect(turns).toHaveLength(4); for (const t of turns) expect(t.stopReason).toBe("end_turn"); }); @@ -217,12 +228,21 @@ describe.skipIf(!!skip)(title, () => { expect(answer.line).toBe(NEEDLE_LINE); }); - it("recovers an exact file count from the find pipe fallback", () => { + it("recovers an exact file count from a guarded (raw) find -not", () => { if (turnError) throw turnError; const answer = readAnswer(repo, "answer-find.json"); expect(answer.count).toBe(FIXTURE_COUNT); }); + // The lossy threshold the small turns sail under: rtk grep shows at most + // 200 matches, so the agent can only get the exact total from the uncapped + // summary header. This is the discoverability contract at real scale. + it("recovers the exact match total past rtk's truncation cap", () => { + if (turnError) throw turnError; + const answer = readAnswer(repo, "answer-count.json"); + expect(answer.matches).toBe(NOISE_MATCH_TOTAL); + }); + it("recovers per-file git state from a compressed && chain", () => { if (turnError) throw turnError; const answer = readAnswer(repo, "answer-git.json"); diff --git a/packages/agent/src/adapters/claude/session/rtk.test.ts b/packages/agent/src/adapters/claude/session/rtk.test.ts index a5c73a74da..c077f4315c 100644 --- a/packages/agent/src/adapters/claude/session/rtk.test.ts +++ b/packages/agent/src/adapters/claude/session/rtk.test.ts @@ -59,19 +59,19 @@ describe("rewriteBashForRtk", () => { ["cd /tmp && cat file.ts"], // Unbalanced quotes — the splitter can't model the line, fail safe. ["ls 'unclosed && git status"], - // find actions that execute, delete, or reformat output — never touched: - // the pipe fallback's find filter would mis-summarize non-list output. + // find predicates rtk's parser rejects run raw: rewriting would swap the + // file list for an error, and these shapes are typically used when the + // complete matched set matters (rtk's filters truncate large lists). + ["find . -not -path '*/node_modules/*'"], ["find . -name '*.tmp' -exec rm {} \\;"], ["find . -name '*.log' -delete"], + ["find . -newer package.json"], + ["find . ! -name '*.md'"], + ["find . -mtime -1"], ["find . -type f -print0"], - ["find . -maxdepth 1 -ls"], - ["find . -printf '%p %s\\n'"], - // rg output shapes the grep pipe filter would corrupt stay raw. - ["rg --json foo src"], - ["rg -l foo src"], - ["rg --files src"], - ["rg -c foo src"], - ["rg --stats foo src"], + // rg stays raw unless rgOnPath is passed (rtk rg execs a real rg binary). + ["rg -n foo src"], + ["rg -i pattern ."], // A leading env assignment or explicit path is not a bare allowlisted head. ["FOO=bar git status"], ["/usr/bin/git status"], @@ -110,35 +110,11 @@ describe("rewriteBashForRtk", () => { "find src -type f -name '*.ts' -maxdepth 2", "rtk find src -type f -name '*.ts' -maxdepth 2", ], - // find predicates rtk's parser rejects but that still emit a plain file - // list fall back to compacting stdout through rtk's pipe filter, inside a - // pipefail subshell so the command's own exit code survives the pipe. - [ - "find . -not -path '*/node_modules/*'", - "( set -o pipefail; find . -not -path '*/node_modules/*' | rtk pipe -f find )", - ], - [ - "find . -mtime -1", - "( set -o pipefail; find . -mtime -1 | rtk pipe -f find )", - ], - [ - "find . ! -name '*.md'", - "( set -o pipefail; find . ! -name '*.md' | rtk pipe -f find )", - ], - // …and each find shape resolves independently inside a chain. + // …and each find shape resolves independently inside a chain: an + // rtk-unsupported find runs raw while its sibling still compresses. [ "find src -name '*.ts' && find . -not -path '*/dist/*'", - "rtk find src -name '*.ts' && ( set -o pipefail; find . -not -path '*/dist/*' | rtk pipe -f find )", - ], - // rg runs natively (it is often a shell function rtk cannot exec) with - // stdout compacted through the grep pipe filter. - [ - "rg -n foo packages/agent/src", - "( set -o pipefail; rg -n foo packages/agent/src | rtk pipe -f grep )", - ], - [ - "cd /tmp && rg -i pattern .", - "cd /tmp && ( set -o pipefail; rg -i pattern . | rtk pipe -f grep )", + "rtk find src -name '*.ts' && find . -not -path '*/dist/*'", ], ])("rewrites chained %j", (input, expected) => { expect(rewriteBashForRtk(input, "rtk")).toBe(expected); @@ -153,10 +129,14 @@ describe("rewriteBashForRtk", () => { expect(rewriteBashForRtk(input, "rtk", { rgOnPath: true })).toBe(expected); }); - test("unsafe rg output flags stay raw even with rg on PATH", () => { - expect( - rewriteBashForRtk("rg --json foo src", "rtk", { rgOnPath: true }), - ).toBeNull(); + test.each([ + ["rg --json foo src"], + ["rg -l foo src"], + ["rg --files src"], + ["rg -c foo src"], + ["rg --stats foo src"], + ])("unsafe rg output flags stay raw even with rg on PATH: %j", (input) => { + expect(rewriteBashForRtk(input, "rtk", { rgOnPath: true })).toBeNull(); }); test("chained rewrite preserves untouched-segment whitespace and `;;`", () => { @@ -168,8 +148,6 @@ describe("rewriteBashForRtk", () => { test.each([ ["rtk git status", "rtk"], ["rtk git status && rtk ls", "rtk"], - ["( set -o pipefail; find . -mtime -1 | rtk pipe -f find )", "rtk"], - ["( set -o pipefail; rg -n foo src | rtk pipe -f grep )", "rtk"], ])("is idempotent — does not double-wrap %j", (input, prefix) => { expect(rewriteBashForRtk(input, prefix)).toBeNull(); }); @@ -197,7 +175,7 @@ describe("resolveRtkPrefix", () => { beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-test-")); binary = path.join(dir, "rtk"); - fs.writeFileSync(binary, "#!/bin/sh\n"); + fs.writeFileSync(binary, "#!/bin/sh\n", { mode: 0o755 }); }); afterAll(() => { @@ -248,7 +226,7 @@ describe("detectRtkBinary", () => { beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-detect-")); binary = path.join(dir, "rtk"); - fs.writeFileSync(binary, "#!/bin/sh\n"); + fs.writeFileSync(binary, "#!/bin/sh\n", { mode: 0o755 }); }); afterAll(() => { @@ -290,7 +268,7 @@ describe("rgOnPath", () => { beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "rg-test-")); - fs.writeFileSync(path.join(dir, "rg"), "#!/bin/sh\n"); + fs.writeFileSync(path.join(dir, "rg"), "#!/bin/sh\n", { mode: 0o755 }); }); afterAll(() => { @@ -304,6 +282,23 @@ describe("rgOnPath", () => { test("reports false when rg is absent (e.g. only a shell function)", () => { expect(rgOnPath({ PATH: "/nonexistent" })).toBe(false); }); + + // The shell would skip a non-executable file; counting it as available + // would rewrite rg into `rtk rg`, which then dies with permission denied. + test.skipIf(process.platform === "win32")( + "reports false for a non-executable rg file on PATH", + () => { + const noExecDir = fs.mkdtempSync(path.join(os.tmpdir(), "rg-noexec-")); + fs.writeFileSync(path.join(noExecDir, "rg"), "#!/bin/sh\n", { + mode: 0o644, + }); + try { + expect(rgOnPath({ PATH: noExecDir })).toBe(false); + } finally { + fs.rmSync(noExecDir, { recursive: true, force: true }); + } + }, + ); }); describe("createRtkRewriteHook", () => { diff --git a/packages/agent/src/adapters/claude/session/rtk.ts b/packages/agent/src/adapters/claude/session/rtk.ts index aa2802fc19..ccab8fa30d 100644 --- a/packages/agent/src/adapters/claude/session/rtk.ts +++ b/packages/agent/src/adapters/claude/session/rtk.ts @@ -148,11 +148,8 @@ export const RTK_FIND_SUPPORTED_FLAGS = new Set([ ]); // rg flags that change the output away from the plain file:line:content -// matches the grep pipe filter models — compacting them would corrupt counts, -// file lists, or JSON that a consumer (or the model) reads structurally. -// rg is piped (`rg … | rtk pipe -f grep`) rather than prefix-routed because -// rg is commonly a shell function (Claude Code's embedded ripgrep), which -// `rtk rg` cannot exec. +// matches rtk's rg filter models — compacting them would corrupt counts, file +// lists, or JSON that a consumer (or the model) reads structurally. const RG_UNSAFE_FLAGS = new Set([ "--json", "--files", @@ -171,24 +168,6 @@ const RG_UNSAFE_FLAGS = new Set([ "--replace", ]); -// find actions that execute commands, write output elsewhere, or change the -// stdout format away from a plain file list. Their invocations are never -// touched: the pipe fallback's find filter would mis-summarize non-list output. -const FIND_ACTION_FLAGS = new Set([ - "-exec", - "-execdir", - "-ok", - "-okdir", - "-delete", - "-ls", - "-fls", - "-print0", - "-printf", - "-fprint", - "-fprint0", - "-fprintf", -]); - function findFlagTokens(trimmed: string): string[] { return trimmed .split(/\s+/) @@ -211,33 +190,11 @@ function rtkSupportsFindInvocation(trimmed: string): boolean { return true; } -function findEmitsPlainFileList(trimmed: string): boolean { - return !findFlagTokens(trimmed).some((bare) => FIND_ACTION_FLAGS.has(bare)); -} - -/** - * Compacts a command's stdout through rtk's pipe filter without rtk running - * the command itself — for commands rtk can't exec (rg as a shell function) - * or can't parse (find predicates outside its allowlist). The subshell scopes - * `pipefail` so the segment's exit code is still the command's own (a bare - * pipe would report rtk's exit and flip rg's no-match 1 to 0, breaking `&&` - * short-circuits), and on re-entry its `|` disqualifies the segment, keeping - * the rewrite idempotent. - */ -function pipeFallback( - trimmed: string, - quotedPrefix: string, - filter: string, -): string { - return `( set -o pipefail; ${trimmed} | ${quotedPrefix} pipe -f ${filter} )`; -} - export interface RtkRewriteOptions { /** - * Whether `rg` is a real executable on PATH. Native `rtk rg` compresses far - * harder than the pipe filter (it applies the proxy's result caps and match - * summary) but execs rg itself, which fails when rg is only a shell - * function (Claude Code's embedded ripgrep). Detected once per session via + * Whether `rg` is a real executable on PATH. `rtk rg` execs rg itself, so + * it fails where rg is only a shell function (Claude Code's embedded + * ripgrep) — those environments leave rg raw. Detected once per session via * `rgOnPath(env)`. */ rgOnPath?: boolean; @@ -248,10 +205,12 @@ export function rgOnPath(env: NodeJS.ProcessEnv): boolean { } /** - * Returns the rewritten form of one chain segment, or null to leave it - * untouched. Two shapes: ` ` when rtk can run the command itself, - * and the pipeFallback subshell when only the command's stdout can be - * compacted. + * Returns the ` ` rewrite of one chain segment, or null to leave + * it untouched. rtk's filters are explicitly lossy at scale (result caps with + * `+N` truncation markers), so only commands rtk proxies natively — where the + * truncation is discoverable from an uncapped total header — are rewritten; + * a command piped anywhere (e.g. `rg … | cat`) always bypasses the rewrite, + * which is the exact-output escape hatch. */ function rewriteSegmentInvocation( segment: string, @@ -277,17 +236,23 @@ function rewriteSegmentInvocation( return `${quotedPrefix} ${trimmed}`; } if (head === "find") { - if (rtkSupportsFindInvocation(trimmed)) return `${quotedPrefix} ${trimmed}`; - if (findEmitsPlainFileList(trimmed)) { - return pipeFallback(trimmed, quotedPrefix, "find"); - } - return null; + // Predicates outside rtk's parser (-not, -exec, !, …) run raw. They tend + // to be used when the agent needs the complete matched set, and rtk's + // pipe filters truncate large lists (10 per dir, `+N` markers) — + // compressing there deletes exactly what the invocation exists to produce. + if (!rtkSupportsFindInvocation(trimmed)) return null; + return `${quotedPrefix} ${trimmed}`; } if (head === "rg") { + // Only when rg is a real executable — `rtk rg` execs rg itself, which + // fails where rg is a shell function (Claude Code's embedded ripgrep). + // The rg proxy applies the same result caps as the long-shipped grep + // proxy and reports the uncapped total ("N matches in M files"), so + // truncation stays discoverable. + if (!options.rgOnPath) return null; const tokens = trimmed.split(/\s+/).slice(1); if (tokens.some((t) => RG_UNSAFE_FLAGS.has(t))) return null; - if (options.rgOnPath) return `${quotedPrefix} ${trimmed}`; - return pipeFallback(trimmed, quotedPrefix, "grep"); + return `${quotedPrefix} ${trimmed}`; } if (!RTK_PLAIN_COMMANDS.has(head)) return null; return `${quotedPrefix} ${trimmed}`; @@ -331,16 +296,21 @@ export function rewriteBashForRtk( function findOnPath(bin: string, env: NodeJS.ProcessEnv): string | undefined { const pathVar = env.PATH ?? env.Path ?? ""; - const exts = - process.platform === "win32" ? [".exe", ".cmd", ".bat", ""] : [""]; + const isWindows = process.platform === "win32"; + const exts = isWindows ? [".exe", ".cmd", ".bat", ""] : [""]; for (const dir of pathVar.split(path.delimiter)) { if (!dir) continue; for (const ext of exts) { const full = path.join(dir, bin + ext); try { - if (fs.statSync(full).isFile()) return full; + if (!fs.statSync(full).isFile()) continue; + // The shell would skip a non-executable file and resolve the next PATH + // entry (or a shell function); treating it as available would produce + // rewrites that die with EACCES. Windows has no execute bit. + if (!isWindows) fs.accessSync(full, fs.constants.X_OK); + return full; } catch { - // Not in this dir; keep looking. + // Not here or not executable; keep looking. } } } diff --git a/packages/agent/src/adapters/rtk-guidance.test.ts b/packages/agent/src/adapters/rtk-guidance.test.ts index cd98b5249e..a695fdbb38 100644 --- a/packages/agent/src/adapters/rtk-guidance.test.ts +++ b/packages/agent/src/adapters/rtk-guidance.test.ts @@ -15,7 +15,7 @@ describe("rtk guidance for codex", () => { beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-guidance-test-")); binary = path.join(dir, "rtk"); - fs.writeFileSync(binary, "#!/bin/sh\n"); + fs.writeFileSync(binary, "#!/bin/sh\n", { mode: 0o755 }); }); afterAll(() => { diff --git a/scripts/rtk-bench.mjs b/scripts/rtk-bench.mjs index 0bdf16a359..c85654329e 100644 --- a/scripts/rtk-bench.mjs +++ b/scripts/rtk-bench.mjs @@ -1,16 +1,25 @@ #!/usr/bin/env node -// Deterministic token benchmark of the RTK rewrite policy. Runs a fixed -// command corpus against this repo's own files, once raw and once after -// applying the REAL `rewriteBashForRtk` hook policy (imported from -// packages/agent via tsx, not reimplemented), and reports the weighted-average -// % token reduction. Token estimate is bytes/4 (rtk's own heuristic). +// Benchmark of the RTK rewrite policy: candidate (working tree) vs baseline +// (main's policy, materialized via `git show`) vs raw, on a fixed command +// corpus against this repo's own files. Both arms apply the REAL +// `rewriteBashForRtk` (imported via tsx), so policy drift between bench and +// production is impossible, and the reported delta is candidate-vs-baseline — +// the net policy gain — not candidate-vs-raw. // -// The corpus mixes single bare commands with `&&`-chained lines, mirroring the -// harness guidance that instructs the agent to batch commands with `&&`. +// Every row is fidelity-gated: exit-status parity with raw, output-presence +// parity, and per-row fact checks (ls: every filename survives; grep/rg/find: +// the exact uncapped totals survive in the summary header, since rtk +// truncates shown results by design). A candidate-only failure fails the run — +// a rewrite that errors into empty output registers as a REGRESSION, never as +// savings; a baseline-only failure is a main bug the candidate fixed. +// +// Token figures are bytes/4, a size heuristic comparable to `rtk gain` — NOT +// provider tokenizer counts, and this measures command output only (no prompt +// guidance, retries, or session-level effects). // // Usage: node scripts/rtk-bench.mjs [--json] -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -20,49 +29,83 @@ const repoRoot = path.resolve( path.dirname(fileURLToPath(import.meta.url)), "..", ); +const RTK_MODULE_DIR = "packages/agent/src/adapters/claude/session"; + +const lines = (s) => s.split("\n").filter(Boolean); +// rtk's search/list filters are lossy by design past their result caps but +// must report accurate uncapped totals ("924 matches in 209 files", "229F +// 23D") so truncation stays discoverable. The facts below encode exactly that +// contract; completeness is asserted only where rtk is complete (ls). +const MATCH_HEADER = /\d+ matches in \d+ files/; +const findHeader = (raw) => + `${lines(raw).filter((l) => l.includes("/")).length}F`; +const lsNames = (raw) => + lines(raw) + .filter((l) => !/^total \d+/.test(l)) + .map((l) => path.basename(l.trim().split(/\s+/).pop() ?? "")) + .filter((n) => n && n !== "." && n !== ".."); +// facts(rawStdout) → strings/regexes the rewritten arm's output must contain. const CORPUS = [ - // Single bare commands (the pre-existing rewrite surface). - { label: "grep import", cmd: `grep -rn "^import" packages/agent/src` }, - { label: "grep export", cmd: `grep -rn "^export" packages/core/src` }, - { label: "find ts", cmd: `find packages/agent/src -name "*.ts"` }, { - label: "find test", - cmd: `find packages -name "*.test.ts" -not -path "*/node_modules/*"`, + label: "grep import", + cmd: `grep -rn "^import" packages/agent/src`, + facts: () => [MATCH_HEADER], + }, + { + label: "grep export", + cmd: `grep -rn "^export" packages/core/src`, + facts: () => [MATCH_HEADER], + }, + { + label: "grep no-match", + cmd: `grep -rn "zz-no-match-zz" scripts`, + facts: () => [], + }, + { + label: "find ts", + cmd: `find packages/agent/src -name "*.ts"`, + facts: (raw) => [findHeader(raw)], + }, + { + label: "find -not (raw)", + cmd: `find packages/agent -name "*.test.ts" -not -path "*/node_modules/*"`, + facts: (raw) => [findHeader(raw)], }, - { label: "ls -la agent/src", cmd: "ls -la packages/agent/src" }, - { label: "ls -la core/src", cmd: "ls -la packages/core/src" }, - { label: "git status", cmd: "git status" }, - { label: "git branch -a", cmd: "git branch -a" }, - // &&-chained lines, as the harness's batching guidance produces. + { + label: "ls -la agent/src", + cmd: "ls -la packages/agent/src", + facts: lsNames, + }, + { label: "git status", cmd: "git status", facts: () => [] }, + { label: "git branch -a", cmd: "git branch -a", facts: () => [] }, { label: "chain: status+diff", cmd: "git status && git diff --stat", + facts: () => [], }, { label: "chain: ls+find", - cmd: `ls -la packages/agent/src/adapters && find packages/agent/src/adapters -name "*.test.ts"`, + cmd: `ls packages/agent/src/adapters && find packages/agent/src/adapters -name "*.test.ts"`, + facts: (raw) => [findHeader(raw)], }, { - label: "chain: grep+grep", - cmd: `grep -rn "describe(" packages/agent/src/adapters/claude/session && grep -rn "test.each" packages/agent/src/adapters/claude/session`, + label: "rg import", + cmd: `rg -n "^import" packages/core/src`, + facts: () => [MATCH_HEADER], }, - { - label: "chain: mixed heads", - cmd: `echo checking && ls packages/core/src && git branch -a`, - }, - // rg: often a shell function (Claude Code's embedded ripgrep) rather than a - // PATH binary, hence the shim in runShell. - { label: "rg import", cmd: `rg -n "^import" packages/core/src` }, + // Under rtk's 200-result cap rg passes through verbatim (no header), so the + // fact is presence of a known raw line, not the header. { label: "chain: rg+status", cmd: `rg -n "Symbol.for" packages/core/src && git status`, + facts: (raw) => lines(raw).slice(0, 1), }, ]; -// Non-interactive bash lacks the profile function that exposes Claude Code's -// embedded ripgrep. Expose it as a real executable on PATH instead, matching -// a dev machine with ripgrep installed — which also lets `rtk rg` exec it. +// rg in agent sessions is often Claude Code's embedded ripgrep behind a shell +// function; expose it as a real executable so the bench mirrors a dev machine +// and `rtk rg` can exec it. function ensureRgOnPath() { try { execFileSync("bash", ["-c", "command -v rg"], { encoding: "utf8" }); @@ -70,9 +113,8 @@ function ensureRgOnPath() { } catch { const dir = path.join(os.tmpdir(), "rtk-bench-rg"); fs.mkdirSync(dir, { recursive: true }); - const wrapper = path.join(dir, "rg"); fs.writeFileSync( - wrapper, + path.join(dir, "rg"), `#!/bin/bash\nexec -a rg "\${CLAUDE_CODE_EXECPATH:-$HOME/.local/bin/claude}" "$@"\n`, { mode: 0o755 }, ); @@ -81,74 +123,172 @@ function ensureRgOnPath() { } const BENCH_PATH = ensureRgOnPath(); +// Keep bench traffic out of the user's real rtk gain telemetry. +const BENCH_ENV = { + ...process.env, + PATH: BENCH_PATH, + RTK_DB_PATH: path.join(os.tmpdir(), "rtk-bench-gain.db"), +}; function tokensOf(text) { return Math.ceil(Buffer.byteLength(text, "utf8") / 4); } function runShell(cmd) { - try { - return execFileSync("bash", ["-c", cmd], { + const started = Date.now(); + const res = spawnSync("bash", ["-c", cmd], { + cwd: repoRoot, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + env: BENCH_ENV, + }); + return { + stdout: res.stdout ?? "", + stderr: res.stderr ?? "", + code: res.status ?? -1, + ms: Date.now() - started, + }; +} + +// Materialize main's policy module so the baseline arm runs the code actually +// shipped, not a reimplementation of it. +function materializeBaselinePolicy() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-bench-baseline-")); + fs.mkdirSync(path.join(dir, "session"), { recursive: true }); + for (const [gitPath, outPath] of [ + [`${RTK_MODULE_DIR}/rtk.ts`, "session/rtk.ts"], + ["packages/agent/src/adapters/claude/git-command.ts", "git-command.ts"], + ]) { + const content = execFileSync("git", ["show", `main:${gitPath}`], { cwd: repoRoot, encoding: "utf8", - maxBuffer: 64 * 1024 * 1024, - env: { ...process.env, PATH: BENCH_PATH }, + maxBuffer: 16 * 1024 * 1024, }); - } catch (err) { - return err.stdout ?? ""; + fs.writeFileSync(path.join(dir, outPath), content); } + return path.join(dir, "session", "rtk.ts"); } -// Apply the real hook policy from packages/agent in one tsx subprocess. -function applyRewritePolicy(commands) { +// One tsx eval applies both real policies to the whole corpus. +function applyPolicies(commands) { + const baselineModule = materializeBaselinePolicy(); const script = ` - import { rewriteBashForRtk, rgOnPath } from "./src/adapters/claude/session/rtk"; + import * as cand from ${JSON.stringify(path.join(repoRoot, RTK_MODULE_DIR, "rtk.ts"))}; + import * as base from ${JSON.stringify(baselineModule)}; const cmds = JSON.parse(process.env.RTK_BENCH_CMDS); - const options = { rgOnPath: rgOnPath(process.env) }; - console.log(JSON.stringify(cmds.map((c) => rewriteBashForRtk(c, "rtk", options)))); + const options = { rgOnPath: "rgOnPath" in cand ? cand.rgOnPath(process.env) : false }; + console.log(JSON.stringify(cmds.map((c) => ({ + base: base.rewriteBashForRtk(c, "rtk"), + cand: cand.rewriteBashForRtk(c, "rtk", options), + })))); `; const stdout = execFileSync("pnpm", ["exec", "tsx", "-e", script], { cwd: path.join(repoRoot, "packages/agent"), encoding: "utf8", - env: { - ...process.env, - PATH: BENCH_PATH, - RTK_BENCH_CMDS: JSON.stringify(commands), - }, + env: { ...BENCH_ENV, RTK_BENCH_CMDS: JSON.stringify(commands) }, }); const lines = stdout.trim().split("\n"); return JSON.parse(lines[lines.length - 1]); } -const rewritten = applyRewritePolicy(CORPUS.map((c) => c.cmd)); +function fidelityIssues(facts, raw, arm) { + const issues = []; + if (arm.code !== raw.code) { + issues.push(`exit ${arm.code} != raw ${raw.code}`); + } + if (raw.stdout.trim() && !arm.stdout.trim()) { + issues.push("raw produced output, arm produced none"); + } + if (arm.stdout === raw.stdout) return issues; // passthrough + + for (const fact of facts(raw.stdout)) { + const present = + fact instanceof RegExp + ? fact.test(arm.stdout) + : arm.stdout.includes(fact); + if (!present) { + issues.push(`missing fact ${fact}`); + break; + } + } + return issues; +} + +const rewritten = applyPolicies(CORPUS.map((c) => c.cmd)); -const rows = CORPUS.map(({ label, cmd }, i) => { +const rows = CORPUS.map(({ label, cmd, facts }, i) => { const raw = runShell(cmd); - const optimized = rewritten[i] ? runShell(rewritten[i]) : raw; - const rawTokens = tokensOf(raw); - const optTokens = tokensOf(optimized); - const pct = rawTokens > 0 ? (1 - optTokens / rawTokens) * 100 : 0; - return { label, rewritten: rewritten[i] ?? null, rawTokens, optTokens, pct }; + const arms = {}; + for (const arm of ["base", "cand"]) { + const cmdForArm = rewritten[i][arm]; + const run = cmdForArm ? runShell(cmdForArm) : raw; + arms[arm] = { + rewritten: cmdForArm, + tokens: tokensOf(run.stdout), + ms: run.ms, + issues: cmdForArm ? fidelityIssues(facts, raw, run) : [], + }; + } + return { label, rawTokens: tokensOf(raw.stdout), rawMs: raw.ms, ...arms }; }); -const totalRaw = rows.reduce((s, r) => s + r.rawTokens, 0); -const totalOpt = rows.reduce((s, r) => s + r.optTokens, 0); -const weightedPct = totalRaw > 0 ? (1 - totalOpt / totalRaw) * 100 : 0; +const total = (pick) => rows.reduce((s, r) => s + pick(r), 0); +const totalRaw = total((r) => r.rawTokens); +const totalBase = total((r) => r.base.tokens); +const totalCand = total((r) => r.cand.tokens); +// A candidate-only failure is a regression this PR would ship — hard fail. +// A baseline failure the candidate fixed is evidence, not an error. +const regressions = rows.filter((r) => r.cand.issues.length); +const preexisting = rows.filter( + (r) => r.base.issues.length && !r.cand.issues.length, +); if (process.argv.includes("--json")) { console.log( - JSON.stringify({ rows, totalRaw, totalOpt, weightedPct }, null, 2), + JSON.stringify({ rows, totalRaw, totalBase, totalCand }, null, 2), ); } else { - console.log("RTK Bench -- raw vs hook-policy token estimate (bytes/4)"); - console.log("=".repeat(64)); + console.log( + "RTK Bench -- raw vs baseline(main) vs candidate; tokens = bytes/4 heuristic", + ); + console.log("=".repeat(78)); for (const r of rows) { + const fid = r.cand.issues.length + ? "REGRESSION" + : r.base.issues.length + ? "fixed-vs-main" + : "ok"; console.log( - `${r.label.padEnd(22)} raw=${String(r.rawTokens).padStart(7)} opt=${String(r.optTokens).padStart(6)} ${r.pct.toFixed(1)}%${r.rewritten ? "" : " (not rewritten)"}`, + `${r.label.padEnd(20)} raw=${String(r.rawTokens).padStart(7)} base=${String(r.base.tokens).padStart(7)} cand=${String(r.cand.tokens).padStart(7)} fid=${fid}`, ); + for (const [arm, a] of [ + ["base", r.base], + ["cand", r.cand], + ]) { + for (const issue of a.issues) console.log(` !! ${arm}: ${issue}`); + } } - console.log("-".repeat(64)); + console.log("-".repeat(78)); + const pct = (n, d) => (d > 0 ? `${((1 - n / d) * 100).toFixed(1)}%` : "n/a"); console.log( - `TOTAL raw=${String(totalRaw).padStart(7)} opt=${String(totalOpt).padStart(6)} ${weightedPct.toFixed(1)}%`, + `TOTAL raw=${String(totalRaw).padStart(7)} base=${String(totalBase).padStart(7)} cand=${String(totalCand).padStart(7)}`, ); + console.log( + `reduction vs raw: base=${pct(totalBase, totalRaw)} cand=${pct(totalCand, totalRaw)}`, + ); + console.log( + `net policy gain (candidate vs baseline): ${pct(totalCand, totalBase)} of baseline output`, + ); + if (preexisting.length) { + console.log( + `\n${preexisting.length} row(s) broken on main's policy are fixed by the candidate.`, + ); + } + if (regressions.length) { + console.error( + `\nCANDIDATE FIDELITY REGRESSIONS in ${regressions.length} row(s) — savings figures are not trustworthy.`, + ); + } } + +process.exit(regressions.length ? 1 : 0); From 24b8fc53964837cd763f9ce04fb1835fb5375bf8 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 16:44:16 -0400 Subject: [PATCH 5/7] fix(agent): harden rtk splitter against heredocs/substitutions, exact bench facts, rg flag normalization Review found the segment splitter treats ; and && inside heredoc bodies and $(...) substitutions as separators, rewriting text that is document content or programmatically consumed output; the bench fidelity facts accepted any numeric summary; and rtk intercepts rg -h/--help/-V itself, changing command meaning. The splitter now refuses multiline input entirely and tracks paren depth, failing safe on unbalanced parens (case arms included). The rg guard normalizes clustered short flags and =-attached long forms and blocks help/version, mirrored into the Codex guidance. Bench facts are now exact values derived from raw output (match totals for anchored patterns, per-file git status facts, full passthrough line presence for under-cap rg), stderr counts toward token cost, temp paths are per-run private dirs with an upfront shim probe, and the baseline ref falls back to origin/main. Also strips shell escapes in find token guards and guards e2e cleanup against partial setup. Generated-By: PostHog Code Task-Id: 5330cf5e-2ca2-4f33-82e9-173dd4a01af6 --- .../e2e/rtk-context-fidelity.e2e.test.ts | 10 +- .../src/adapters/claude/session/rtk.test.ts | 35 ++++- .../agent/src/adapters/claude/session/rtk.ts | 84 +++++++++--- .../agent/src/adapters/rtk-guidance.test.ts | 10 ++ packages/agent/src/adapters/rtk-guidance.ts | 5 +- scripts/rtk-bench.mjs | 128 ++++++++++++++---- 6 files changed, 218 insertions(+), 54 deletions(-) diff --git a/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts index bb1ec1b03c..b4082471c5 100644 --- a/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts +++ b/packages/agent/e2e/rtk-context-fidelity.e2e.test.ts @@ -209,10 +209,16 @@ describe.skipIf(!!skip)(title, () => { }, 360_000); afterAll(() => { + // Guarded: a beforeAll that throws before assignment must not have its + // real failure masked by cleanup throwing on undefined paths. if (savedRtkDbPath === undefined) delete process.env.RTK_DB_PATH; else process.env.RTK_DB_PATH = savedRtkDbPath; - cleanupRepo(repo); - rmSync(rtkDbPath, { force: true }); + if (repo) cleanupRepo(repo); + if (rtkDbPath) { + for (const suffix of ["", "-wal", "-shm"]) { + rmSync(`${rtkDbPath}${suffix}`, { force: true }); + } + } }); it("completes all four turns", () => { diff --git a/packages/agent/src/adapters/claude/session/rtk.test.ts b/packages/agent/src/adapters/claude/session/rtk.test.ts index c077f4315c..da8efc3a4f 100644 --- a/packages/agent/src/adapters/claude/session/rtk.test.ts +++ b/packages/agent/src/adapters/claude/session/rtk.test.ts @@ -59,6 +59,22 @@ describe("rewriteBashForRtk", () => { ["cd /tmp && cat file.ts"], // Unbalanced quotes — the splitter can't model the line, fail safe. ["ls 'unclosed && git status"], + // Multiline input (heredocs, scripts): a `;` inside a heredoc body is + // document text — splitting there would rewrite the *contents* of a + // generated file. The splitter refuses all multiline input. + ["cat > out.sh < { "find src -name '*.ts' && find . -not -path '*/dist/*'", "rtk find src -name '*.ts' && find . -not -path '*/dist/*'", ], + // A balanced $(...) stays intact within its own segment; only the + // substitution-free sibling is rewritten. + ["echo $(git status) && ls", "echo $(git status) && rtk ls"], ])("rewrites chained %j", (input, expected) => { expect(rewriteBashForRtk(input, "rtk")).toBe(expected); }); @@ -135,16 +154,20 @@ describe("rewriteBashForRtk", () => { ["rg --files src"], ["rg -c foo src"], ["rg --stats foo src"], + // Clustered short flags and =-attached long forms are the same modes. + ["rg -nl foo src"], + ["rg -cn foo src"], + ["rg --replace=x foo src"], + // rtk intercepts help/version itself — `rtk rg -h` prints the wrapper's + // help, not ripgrep's, so these change meaning entirely. + ["rg -h"], + ["rg --help"], + ["rg -V"], + ["rg --version"], ])("unsafe rg output flags stay raw even with rg on PATH: %j", (input) => { expect(rewriteBashForRtk(input, "rtk", { rgOnPath: true })).toBeNull(); }); - test("chained rewrite preserves untouched-segment whitespace and `;;`", () => { - expect(rewriteBashForRtk("case x in x) echo hi;; esac; ls", "rtk")).toBe( - "case x in x) echo hi;; esac; rtk ls", - ); - }); - test.each([ ["rtk git status", "rtk"], ["rtk git status && rtk ls", "rtk"], diff --git a/packages/agent/src/adapters/claude/session/rtk.ts b/packages/agent/src/adapters/claude/session/rtk.ts index ccab8fa30d..43992f9c09 100644 --- a/packages/agent/src/adapters/claude/session/rtk.ts +++ b/packages/agent/src/adapters/claude/session/rtk.ts @@ -82,16 +82,24 @@ interface CommandSegment { /** * Splits a command line into top-level segments joined by `&&` or `;`, - * tracking single/double quotes and backslash escapes so operators inside - * quoted arguments don't split. Returns null when the line uses syntax the - * splitter doesn't model (unbalanced quotes), so callers fail safe. + * tracking single/double quotes, backslash escapes, and parenthesis depth so + * operators inside quoted arguments, `$(...)`/`<(...)` substitutions, and + * subshells don't split — a split there would put the substitution's tail in + * its own segment, hiding the `$(` from the per-segment operator guard and + * letting compressed output leak into text another program consumes. Returns + * null when the line uses syntax the splitter doesn't model (unbalanced + * quotes or parens, multiline input such as heredocs — a `;` inside a heredoc + * body is document text, not a separator), so callers fail safe. */ export function splitTopLevelSegments( command: string, ): CommandSegment[] | null { + if (command.includes("\n")) return null; + const segments: CommandSegment[] = []; let current = ""; let quote: "'" | '"' | null = null; + let parenDepth = 0; for (let i = 0; i < command.length; i++) { const ch = command[i]; if (quote) { @@ -112,20 +120,33 @@ export function splitTopLevelSegments( current += ch; continue; } - if (ch === "&" && command[i + 1] === "&") { + if (ch === "(") { + parenDepth++; + current += ch; + continue; + } + if (ch === ")") { + // A closer with no opener (`case x in x) …`) is syntax the splitter + // doesn't model — fail safe rather than guess. + if (parenDepth === 0) return null; + parenDepth--; + current += ch; + continue; + } + if (parenDepth === 0 && ch === "&" && command[i + 1] === "&") { segments.push({ text: current, separator: "&&" }); current = ""; i++; continue; } - if (ch === ";") { + if (parenDepth === 0 && ch === ";") { segments.push({ text: current, separator: ";" }); current = ""; continue; } current += ch; } - if (quote) return null; + if (quote || parenDepth !== 0) return null; segments.push({ text: current, separator: "" }); return segments; } @@ -149,35 +170,57 @@ export const RTK_FIND_SUPPORTED_FLAGS = new Set([ // rg flags that change the output away from the plain file:line:content // matches rtk's rg filter models — compacting them would corrupt counts, file -// lists, or JSON that a consumer (or the model) reads structurally. -const RG_UNSAFE_FLAGS = new Set([ +// lists, or JSON that a consumer (or the model) reads structurally. `-h`/`-V` +// and their long forms are intercepted by rtk itself (`rtk rg -h` prints the +// wrapper's help, not ripgrep's), so they change meaning, not just shape. +const RG_UNSAFE_LONG_FLAGS = new Set([ "--json", "--files", - "-l", "--files-with-matches", "--files-without-match", - "-c", "--count", "--count-matches", "--stats", - "-p", "--pretty", - "-q", "--quiet", - "-r", "--replace", + "--vimgrep", + "--help", + "--version", ]); +const RG_UNSAFE_SHORT_LETTERS = new Set(["l", "c", "p", "q", "r", "h", "V"]); + +// rg accepts clustered short options (`-nl` contains `-l`) and `=`-attached +// long values (`--replace=x`), so the guard must normalize before matching — +// exact-token comparison is trivially bypassed by common spellings. +function hasUnsafeRgFlag(tokens: string[]): boolean { + for (const token of tokens) { + if (token.startsWith("--")) { + const bare = token.split("=", 1)[0]; + if (RG_UNSAFE_LONG_FLAGS.has(bare)) return true; + } else if (/^-[a-zA-Z]+$/.test(token)) { + for (const letter of token.slice(1)) { + if (RG_UNSAFE_SHORT_LETTERS.has(letter)) return true; + } + } + } + return false; +} function findFlagTokens(trimmed: string): string[] { return trimmed .split(/\s+/) .slice(1) - .map((token) => - (token.startsWith("'") && token.endsWith("'")) || - (token.startsWith('"') && token.endsWith('"')) - ? token.slice(1, -1) - : token, - ); + .map((token) => { + const unquoted = + (token.startsWith("'") && token.endsWith("'")) || + (token.startsWith('"') && token.endsWith('"')) + ? token.slice(1, -1) + : token; + // `\(`/`\)`/`\!` reach find as bare grouping tokens — strip the shell + // escape so the guard sees what find sees. + return unquoted.startsWith("\\") ? unquoted.slice(1) : unquoted; + }); } function rtkSupportsFindInvocation(trimmed: string): boolean { @@ -250,8 +293,7 @@ function rewriteSegmentInvocation( // proxy and reports the uncapped total ("N matches in M files"), so // truncation stays discoverable. if (!options.rgOnPath) return null; - const tokens = trimmed.split(/\s+/).slice(1); - if (tokens.some((t) => RG_UNSAFE_FLAGS.has(t))) return null; + if (hasUnsafeRgFlag(trimmed.split(/\s+/).slice(1))) return null; return `${quotedPrefix} ${trimmed}`; } if (!RTK_PLAIN_COMMANDS.has(head)) return null; diff --git a/packages/agent/src/adapters/rtk-guidance.test.ts b/packages/agent/src/adapters/rtk-guidance.test.ts index a695fdbb38..caa828f9c7 100644 --- a/packages/agent/src/adapters/rtk-guidance.test.ts +++ b/packages/agent/src/adapters/rtk-guidance.test.ts @@ -75,6 +75,16 @@ describe("rtk guidance for codex", () => { expect(buildRtkGuidance("rtk")).not.toContain("`rg`"); }); + // Parity with the Claude hook's RG unsafe-flag guard: guided models must + // not prefix output-mode or help/version rg invocations. + test("advertises the rg unsafe-flag rule alongside rg", () => { + const withRg = buildRtkGuidance("rtk", { rgOnPath: true }); + expect(withRg).toContain("--json"); + expect(withRg).toContain("-nl"); + expect(withRg).toContain("--help"); + expect(buildRtkGuidance("rtk", { rgOnPath: false })).not.toContain("-nl"); + }); + // Parity with the Claude hook's chain-segment rewriting. test("allows prefixing eligible segments of && chains", () => { const guidance = buildRtkGuidance("rtk"); diff --git a/packages/agent/src/adapters/rtk-guidance.ts b/packages/agent/src/adapters/rtk-guidance.ts index ead029a98d..50f319ef78 100644 --- a/packages/agent/src/adapters/rtk-guidance.ts +++ b/packages/agent/src/adapters/rtk-guidance.ts @@ -44,6 +44,9 @@ export function buildRtkGuidance( ].join("`, `"); const gitSubcommands = [...GIT_COMPRESSIBLE_SUBCOMMANDS].join(", "); const findFlags = [...RTK_FIND_SUPPORTED_FLAGS].join("`, `"); + const rgRule = options.rgOnPath + ? `\n- Never prefix \`rg\` when it uses output-mode or meta flags (\`--json\`, \`-l\`/\`--files*\`, \`-c\`/\`--count*\`, \`--stats\`, \`-r\`/\`--replace\`, \`--vimgrep\`, \`-h\`/\`--help\`, \`-V\`/\`--version\`, including clustered forms like \`-nl\`) — compression corrupts that output, and rtk intercepts help/version itself.` + : ""; return `## rtk command-output compression @@ -56,7 +59,7 @@ Examples: \`${prefix} git status\`, \`${prefix} grep -rn "foo" src\`, \`${prefix Rules: - Only prefix a bare invocation. In a command chained with \`&&\` or \`;\` you may prefix each eligible sub-command individually (e.g. \`cd pkg && ${prefix} git status\`). Never prefix a command that is part of a pipe or redirection, or whose output another program parses — compression would corrupt what the consumer reads. -- Only prefix \`find\` when it uses these predicates alone: \`${findFlags}\`. Run any other find (\`-not\`, \`-exec\`, \`!\`, \`-mtime\`, …) unprefixed — rtk rejects them with an error instead of output. +- Only prefix \`find\` when it uses these predicates alone: \`${findFlags}\`. Run any other find (\`-not\`, \`-exec\`, \`!\`, \`-mtime\`, …) unprefixed — rtk rejects them with an error instead of output.${rgRule} - Never prefix \`git commit\`, \`git push\`, or any other command not listed above. - Skip the prefix when you need the exact, complete output (for example, copying a diff verbatim).`; } diff --git a/scripts/rtk-bench.mjs b/scripts/rtk-bench.mjs index c85654329e..23f69105e4 100644 --- a/scripts/rtk-bench.mjs +++ b/scripts/rtk-bench.mjs @@ -13,9 +13,14 @@ // a rewrite that errors into empty output registers as a REGRESSION, never as // savings; a baseline-only failure is a main bug the candidate fixed. // -// Token figures are bytes/4, a size heuristic comparable to `rtk gain` — NOT -// provider tokenizer counts, and this measures command output only (no prompt -// guidance, retries, or session-level effects). +// SCOPE — what this bench does and does not prove. Token figures are +// bytes/4 over stdout+stderr, a size heuristic comparable to `rtk gain`, NOT +// provider tokenizer counts. The corpus is fixed and NOT usage-weighted, and +// the bench measures command output only: no prompt-guidance overhead, no +// model retries, no session outcomes. It can prove output reduction and +// fidelity for these command shapes; a net harness gain claim additionally +// requires a session-level A/B (total input/output tokens, retries, task +// success) — see the rtk-context-fidelity e2e for the fidelity half. // // Usage: node scripts/rtk-bench.mjs [--json] @@ -35,8 +40,16 @@ const lines = (s) => s.split("\n").filter(Boolean); // rtk's search/list filters are lossy by design past their result caps but // must report accurate uncapped totals ("924 matches in 209 files", "229F // 23D") so truncation stays discoverable. The facts below encode exactly that -// contract; completeness is asserted only where rtk is complete (ls). -const MATCH_HEADER = /\d+ matches in \d+ files/; +// contract with EXACT expected values derived from raw output — a generic +// "any number" pattern would let materially wrong totals pass. Completeness +// is asserted only where rtk is complete (ls, under-cap rg passthrough). +// +// The exact-count fact is only valid for anchored patterns (one match per +// line); rtk counts match occurrences, raw output counts matching lines. +const matchHeader = (raw) => { + const files = new Set(lines(raw).map((l) => l.split(":", 1)[0])); + return `${lines(raw).length} matches in ${files.size} files`; +}; const findHeader = (raw) => `${lines(raw).filter((l) => l.includes("/")).length}F`; const lsNames = (raw) => @@ -44,18 +57,34 @@ const lsNames = (raw) => .filter((l) => !/^total \d+/.test(l)) .map((l) => path.basename(l.trim().split(/\s+/).pop() ?? "")) .filter((n) => n && n !== "." && n !== ".."); +// Per-file facts from `git status` long format: every modified/new/deleted +// path and the current branch must survive compression. +const gitStatusFacts = (raw) => { + const facts = []; + const branch = raw.match(/^On branch (\S+)/m); + if (branch) facts.push(branch[1]); + for (const m of raw.matchAll( + /^\s+(?:modified|new file|deleted):\s+(\S+)/gm, + )) { + facts.push(m[1]); + } + return facts; +}; +// Under rtk's 200-result cap rg passes through verbatim, so every raw +// file:line match must be present, not just a summary. +const rgLines = (raw) => lines(raw).filter((l) => /^\S+:\d+:/.test(l)); // facts(rawStdout) → strings/regexes the rewritten arm's output must contain. const CORPUS = [ { label: "grep import", cmd: `grep -rn "^import" packages/agent/src`, - facts: () => [MATCH_HEADER], + facts: (raw) => [matchHeader(raw)], }, { label: "grep export", cmd: `grep -rn "^export" packages/core/src`, - facts: () => [MATCH_HEADER], + facts: (raw) => [matchHeader(raw)], }, { label: "grep no-match", @@ -77,12 +106,19 @@ const CORPUS = [ cmd: "ls -la packages/agent/src", facts: lsNames, }, - { label: "git status", cmd: "git status", facts: () => [] }, - { label: "git branch -a", cmd: "git branch -a", facts: () => [] }, + { label: "git status", cmd: "git status", facts: gitStatusFacts }, + { + label: "git branch -a", + cmd: "git branch -a", + facts: (raw) => { + const current = raw.match(/^\* (\S+)/m); + return current ? [current[1]] : []; + }, + }, { label: "chain: status+diff", cmd: "git status && git diff --stat", - facts: () => [], + facts: gitStatusFacts, }, { label: "chain: ls+find", @@ -92,32 +128,50 @@ const CORPUS = [ { label: "rg import", cmd: `rg -n "^import" packages/core/src`, - facts: () => [MATCH_HEADER], + facts: (raw) => [matchHeader(raw)], }, - // Under rtk's 200-result cap rg passes through verbatim (no header), so the - // fact is presence of a known raw line, not the header. { label: "chain: rg+status", cmd: `rg -n "Symbol.for" packages/core/src && git status`, - facts: (raw) => lines(raw).slice(0, 1), + facts: (raw) => [...rgLines(raw), ...gitStatusFacts(raw)], }, ]; // rg in agent sessions is often Claude Code's embedded ripgrep behind a shell // function; expose it as a real executable so the bench mirrors a dev machine // and `rtk rg` can exec it. +// Per-run private temp dir: fixed shared-tmp paths are plantable (an attacker +// pre-owning the dir can swap the shim executable) and collide across +// concurrent runs. +const RUN_TMP = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-bench-")); +process.on("exit", () => { + fs.rmSync(RUN_TMP, { recursive: true, force: true }); +}); + function ensureRgOnPath() { try { execFileSync("bash", ["-c", "command -v rg"], { encoding: "utf8" }); return process.env.PATH; } catch { - const dir = path.join(os.tmpdir(), "rtk-bench-rg"); - fs.mkdirSync(dir, { recursive: true }); + const dir = path.join(RUN_TMP, "rg-shim"); + fs.mkdirSync(dir); + const wrapper = path.join(dir, "rg"); fs.writeFileSync( - path.join(dir, "rg"), + wrapper, `#!/bin/bash\nexec -a rg "\${CLAUDE_CODE_EXECPATH:-$HOME/.local/bin/claude}" "$@"\n`, { mode: 0o755 }, ); + // The shim assumes the Claude Code binary multiplexes into ripgrep on + // argv[0]; verify once so a wrong assumption fails here with a clear + // message instead of surfacing as a fake fidelity regression. + try { + execFileSync(wrapper, ["--version"], { encoding: "utf8" }); + } catch { + console.error( + "rg is not on PATH and the Claude Code ripgrep shim does not work here; rg rows would misreport. Install ripgrep and re-run.", + ); + process.exit(2); + } return `${process.env.PATH}:${dir}`; } } @@ -127,11 +181,18 @@ const BENCH_PATH = ensureRgOnPath(); const BENCH_ENV = { ...process.env, PATH: BENCH_PATH, - RTK_DB_PATH: path.join(os.tmpdir(), "rtk-bench-gain.db"), + RTK_DB_PATH: path.join(RUN_TMP, "gain.db"), }; -function tokensOf(text) { - return Math.ceil(Buffer.byteLength(text, "utf8") / 4); +// stderr counts too: in an agent session the model reads both streams, and +// an arm that moves output from stdout to an error on stderr must not score +// as savings. +function tokensOf(run) { + return Math.ceil( + (Buffer.byteLength(run.stdout, "utf8") + + Buffer.byteLength(run.stderr, "utf8")) / + 4, + ); } function runShell(cmd) { @@ -152,14 +213,33 @@ function runShell(cmd) { // Materialize main's policy module so the baseline arm runs the code actually // shipped, not a reimplementation of it. +function resolveBaselineRef() { + for (const ref of ["main", "origin/main"]) { + try { + execFileSync("git", ["rev-parse", "--verify", `${ref}^{commit}`], { + cwd: repoRoot, + stdio: "pipe", + }); + return ref; + } catch { + // Try the next ref. + } + } + console.error( + "Neither `main` nor `origin/main` resolves here (shallow or detached checkout?) — cannot materialize the baseline policy arm.", + ); + process.exit(2); +} + function materializeBaselinePolicy() { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rtk-bench-baseline-")); + const ref = resolveBaselineRef(); + const dir = path.join(RUN_TMP, "baseline"); fs.mkdirSync(path.join(dir, "session"), { recursive: true }); for (const [gitPath, outPath] of [ [`${RTK_MODULE_DIR}/rtk.ts`, "session/rtk.ts"], ["packages/agent/src/adapters/claude/git-command.ts", "git-command.ts"], ]) { - const content = execFileSync("git", ["show", `main:${gitPath}`], { + const content = execFileSync("git", ["show", `${ref}:${gitPath}`], { cwd: repoRoot, encoding: "utf8", maxBuffer: 16 * 1024 * 1024, @@ -224,12 +304,12 @@ const rows = CORPUS.map(({ label, cmd, facts }, i) => { const run = cmdForArm ? runShell(cmdForArm) : raw; arms[arm] = { rewritten: cmdForArm, - tokens: tokensOf(run.stdout), + tokens: tokensOf(run), ms: run.ms, issues: cmdForArm ? fidelityIssues(facts, raw, run) : [], }; } - return { label, rawTokens: tokensOf(raw.stdout), rawMs: raw.ms, ...arms }; + return { label, rawTokens: tokensOf(raw), rawMs: raw.ms, ...arms }; }); const total = (pick) => rows.reduce((s, r) => s + pick(r), 0); From bfafcd1c3935a72d88336018174bba70269450b1 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 17:57:40 -0400 Subject: [PATCH 6/7] chore(agent): remove redundant benchmark comments Generated-By: PostHog Code Task-Id: b24a0c15-8e64-4fd4-a9eb-b8528f3bb9f4 --- scripts/rtk-bench.mjs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/scripts/rtk-bench.mjs b/scripts/rtk-bench.mjs index 23f69105e4..fb5169c9f1 100644 --- a/scripts/rtk-bench.mjs +++ b/scripts/rtk-bench.mjs @@ -221,9 +221,7 @@ function resolveBaselineRef() { stdio: "pipe", }); return ref; - } catch { - // Try the next ref. - } + } catch {} } console.error( "Neither `main` nor `origin/main` resolves here (shallow or detached checkout?) — cannot materialize the baseline policy arm.", @@ -249,7 +247,6 @@ function materializeBaselinePolicy() { return path.join(dir, "session", "rtk.ts"); } -// One tsx eval applies both real policies to the whole corpus. function applyPolicies(commands) { const baselineModule = materializeBaselinePolicy(); const script = ` @@ -316,8 +313,6 @@ const total = (pick) => rows.reduce((s, r) => s + pick(r), 0); const totalRaw = total((r) => r.rawTokens); const totalBase = total((r) => r.base.tokens); const totalCand = total((r) => r.cand.tokens); -// A candidate-only failure is a regression this PR would ship — hard fail. -// A baseline failure the candidate fixed is evidence, not an error. const regressions = rows.filter((r) => r.cand.issues.length); const preexisting = rows.filter( (r) => r.base.issues.length && !r.cand.issues.length, From 19b645d9f75543218afa20c8b1a9d99a722bb416 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Thu, 23 Jul 2026 18:07:46 -0400 Subject: [PATCH 7/7] chore(agent): clarify RTK benchmark scope Generated-By: PostHog Code Task-Id: b24a0c15-8e64-4fd4-a9eb-b8528f3bb9f4 --- scripts/rtk-bench.mjs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/rtk-bench.mjs b/scripts/rtk-bench.mjs index fb5169c9f1..938704c06f 100644 --- a/scripts/rtk-bench.mjs +++ b/scripts/rtk-bench.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node -// Benchmark of the RTK rewrite policy: candidate (working tree) vs baseline -// (main's policy, materialized via `git show`) vs raw, on a fixed command -// corpus against this repo's own files. Both arms apply the REAL +// Stress benchmark of the RTK rewrite policy: candidate (working tree) vs +// baseline (main's policy, materialized via `git show`) vs raw, on a fixed +// command corpus against this repo's own files. Both arms apply the REAL // `rewriteBashForRtk` (imported via tsx), so policy drift between bench and // production is impossible, and the reported delta is candidate-vs-baseline — // the net policy gain — not candidate-vs-raw. @@ -13,7 +13,9 @@ // a rewrite that errors into empty output registers as a REGRESSION, never as // savings; a baseline-only failure is a main bug the candidate fixed. // -// SCOPE — what this bench does and does not prove. Token figures are +// SCOPE — what this bench does and does not prove. The corpus intentionally +// overrepresents large compressible commands and is not a production-traffic +// estimate. Token figures are // bytes/4 over stdout+stderr, a size heuristic comparable to `rtk gain`, NOT // provider tokenizer counts. The corpus is fixed and NOT usage-weighted, and // the bench measures command output only: no prompt-guidance overhead, no @@ -324,7 +326,7 @@ if (process.argv.includes("--json")) { ); } else { console.log( - "RTK Bench -- raw vs baseline(main) vs candidate; tokens = bytes/4 heuristic", + "RTK stress bench -- raw vs baseline(main) vs candidate; tokens = bytes/4 heuristic", ); console.log("=".repeat(78)); for (const r of rows) {