diff --git a/packages/core/src/ripgrep.ts b/packages/core/src/ripgrep.ts index 99c851ed1b..0dad82bd77 100644 --- a/packages/core/src/ripgrep.ts +++ b/packages/core/src/ripgrep.ts @@ -18,9 +18,37 @@ import { RipgrepBinary } from "./ripgrep/binary" */ const ERROR_BYTES = 8 * 1024 -const MAX_RECORD_BYTES = 64 * 1024 +// altimate_change start — upstream_fix: survive oversized ripgrep JSON records. +// A single `--json` match record carries the entire matched line, so one minified bundle, source +// map, or single-line JSON/CSV fixture anywhere in the tree produces a record far past the old +// 64 KiB ceiling. That ceiling aborted the whole stream, so every other match in the search — in +// unrelated files — was lost with it. Telemetry showed 74 machines hitting this in 7 days. +// +// The ceiling never bounded memory either: `Stream.splitLines` has already materialized the full +// line by the time `parse` sees it, so the allocation is paid before the check runs. All it can +// still bound is JSON.parse cost, which is why it survives as a much higher sanity limit — 16 MiB +// clears real-world long lines by a wide margin. Bounding memory needs byte-level framing ahead of +// `splitLines`, which this does not attempt. Records past it are dropped with a warning; the search +// continues either way. +const MAX_RECORD_BYTES = 16 * 1024 * 1024 const MAX_SUBMATCHES = 100 +// The 16 MiB ceiling bounds the cost of parsing ONE line. It does not bound what the search +// retains: `run` collects rows with `Stream.runCollect` and holds them until the stream ends, and +// each row carried the FULL `lines.text` until the mapping step trimmed it at the very end. Peak +// retained memory is therefore rows x record size — and callers pass no meaningful row cap +// (`tool/grep.ts` passes `Number.MAX_SAFE_INTEGER`), so raising the per-record ceiling raised the +// retained bound with it. Capping the line here instead keeps the parse ceiling while making the +// retained bound tighter than it was before this change. Nothing downstream ever renders more. +const LINE_TEXT_CAP = 2_000 + +/** Trim a matched line to what any consumer actually shows, preserving the elision marker. */ +const capLineText = (text: string) => (text.length > LINE_TEXT_CAP ? text.slice(0, LINE_TEXT_CAP) + "..." : text) + +/** Distinct skip reasons kept for the aggregate warning; enough to diagnose, bounded for logs. */ +const SKIP_SAMPLES = 5 +// altimate_change end + const RawMatch = Schema.Struct({ type: Schema.Literal("match"), data: Schema.Struct({ @@ -40,6 +68,140 @@ const RawMatch = Schema.Struct({ type RawMatchData = (typeof RawMatch.Type)["data"] +// altimate_change start — upstream_fix: accept ripgrep's `{bytes}` form of an arbitrary-data field. +// Every `path`/`lines`/`match` field in ripgrep's JSON is a union: `{"text": "..."}` when the value +// is valid UTF-8, `{"bytes": ""}` when it is not. `RawMatch` only models the `text` arm, so +// a single stray non-UTF-8 byte anywhere in the tree failed schema decoding and — inside +// `Stream.mapEffect` — took the whole search down with it, exactly like the oversized record did. +// Normalising to the `text` arm up front keeps the schema single-shape and keeps the match usable; +// `toString("utf8")` substitutes U+FFFD for the undecodable bytes rather than dropping the match. +/** Canonical base64, so a corrupt field is left to fail decoding rather than silently becoming "". */ +const BASE64 = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/ + +/** ripgrep's control records. Anything else with an unrecognised `type` is a protocol surprise. */ +const CONTROL_TYPES = new Set(["begin", "end", "summary"]) + +const readProp = (value: unknown, key: string): unknown => + value !== null && typeof value === "object" && key in value ? Reflect.get(value, key) : undefined + +/** + * Decode one ripgrep arbitrary-data field (`{text}` or `{bytes}`) to a string. + * + * Returns undefined when the field cannot be trusted, which leaves the original shape in place so + * the schema rejects it and the record is skipped — the point being that a corrupt field must never + * be silently converted into a valid-looking empty one. `Buffer.from` makes that easy to get wrong: + * it maps unconvertible input to an EMPTY buffer instead of throwing. Hence three guards — reject + * the empty string (a matched line is never empty, so an empty `bytes` arm is always corrupt), + * check the spelling, then require the decode to round-trip so non-canonical padding bits (`Zh==` + * and `Zg==` both decode to "f") cannot slip through. + * + * `raw` is returned alongside so submatch offsets can be rebased; see `normalizeMatch`. + */ +const decodeField = (value: unknown): { text: string; raw?: Buffer } | undefined => { + if (!value || typeof value !== "object") return undefined + const text = readProp(value, "text") + if (typeof text === "string") return { text } + const bytes = readProp(value, "bytes") + if (typeof bytes !== "string" || bytes.length === 0 || !BASE64.test(bytes)) return undefined + const raw = Buffer.from(bytes, "base64") + if (raw.toString("base64") !== bytes) return undefined + return { text: raw.toString("utf8"), raw } +} + +/** + * Rewrite the `{bytes}` arm of a raw ripgrep match record into its `{text}` equivalent. + * + * `path` is deliberately NOT rewritten. Decoding it is lossy — `toString("utf8")` maps undecodable + * bytes to U+FFFD — and a path is an identifier, not display text: the caller resolves it, stats it + * and reopens it, so a lossy path is a path to a file that does not exist, and two distinct + * filenames can collapse onto the same string. Leaving it in the `{bytes}` arm fails the schema, so + * a match in a file whose NAME is not valid UTF-8 is skipped and logged. Match content is display + * text, so lossy decoding there is the right trade: the match stays useful. + * + * Submatch `start`/`end` are BYTE offsets into the raw line. A lossy decode destroys that frame of + * reference — each undecodable sequence becomes U+FFFD, three bytes wide — so they are rebased onto + * the decoded text's own UTF-8 encoding. That preserves the established byte-offset contract rather + * than silently switching these records to a different unit: without it, a line beginning with one + * bad byte reports `needle` at [3,9) of a string where [3,9) reads "edle t". + */ +const normalizeMatch = (json: object): unknown => { + const data = readProp(json, "data") + if (!data || typeof data !== "object") return json + const lines = decodeField(readProp(data, "lines")) + if (!lines) return json + const raw = lines.raw + // Submatch `start`/`end` are BYTE offsets into the RAW line, so a lossy decode invalidates them: + // every undecodable byte widens to a 3-byte U+FFFD. They are rebased onto the decoded text's own + // UTF-8 encoding, which keeps the established byte-offset contract instead of switching these + // records to a different unit. + // + // A rebase is only sound when the split point is a character boundary of the whole-buffer decode. + // Testing that by decoded-string comparison is NOT enough: for a raw line where an invalid byte + // precedes a literal U+FFFD, an offset inside that literal character still produces a decoded + // prefix that prefixes the line, because the replacement characters alias. Byte-boundary + // validation has no such blind spot — a continuation byte (0b10xxxxxx) at the offset means the + // split lands inside a sequence. Measured over 3.4M fuzzed offsets against the definition + // (decoding both halves must reconstruct the whole) this never accepts an unsafe offset; it only + // errs conservatively, on lines that begin mid-sequence. + // + // An offset that cannot be rebased drops ITS SUBMATCH, not the record: the file, line and text + // are still correct and useful, and this whole change exists to stop losing matches. `{text}`-arm + // offsets are not rebased — there is nothing to rebase onto — but they are validated the same + // way, because returning a coordinate pair that indexes nothing, or that splits a character, is + // itself a claim. + const isContinuationByte = (byte: number | undefined) => byte !== undefined && (byte & 0xc0) === 0x80 + // Encoded once per record and shared by every submatch. This also supplies the byte length, so it + // replaces a `Buffer.byteLength` walk rather than adding one; the extra cost is the allocation. + const textBytes = raw ?? Buffer.from(lines.text, "utf8") + const rebase = (offset: unknown): number | undefined => { + // `{text}` arm: nothing is rebased, but the offset must still be addressable AND land on a + // character boundary. A byte offset can fall inside a multi-byte sequence — for `éa` the byte + // offset 1 splits `é` — and a consumer slicing the line's UTF-8 encoding there gets invalid + // bytes. ripgrep never emits such an offset for a valid-UTF-8 line, so rejecting it drops + // nothing legitimate. Same rule the `{bytes}` arm applies below. + if (!raw) + return typeof offset === "number" && + Number.isInteger(offset) && + offset >= 0 && + offset <= textBytes.length && + !(offset !== 0 && offset !== textBytes.length && isContinuationByte(textBytes[offset])) + ? offset + : undefined + if (typeof offset !== "number" || !Number.isInteger(offset) || offset < 0 || offset > raw.length) return undefined + if (offset !== 0 && offset !== raw.length && isContinuationByte(raw[offset])) return undefined + return Buffer.byteLength(raw.subarray(0, offset).toString("utf8"), "utf8") + } + const submatches = readProp(data, "submatches") + const normalized = { + ...json, + data: { + ...data, + // Capped here rather than after decoding: the full line is retained by `Stream.runCollect` + // until the search ends, and nothing downstream ever shows more than this. See LINE_TEXT_CAP. + lines: { text: capLineText(lines.text) }, + // Sliced BEFORE decoding so a pathological submatch count is not decoded only to be dropped. + submatches: Array.isArray(submatches) + ? submatches.slice(0, MAX_SUBMATCHES).flatMap((submatch) => { + if (!submatch || typeof submatch !== "object") return [submatch] + const match = decodeField(readProp(submatch, "match")) + if (!match) return [submatch] + // A broad pattern such as `x.*` makes ripgrep repeat almost the whole line here, so the + // matched text needs the same bound as the line or the retained-memory cap is defeated + // by the submatches instead. + const decoded = { ...submatch, match: { text: capLineText(match.text) } } + const start = rebase(readProp(submatch, "start")) + const end = rebase(readProp(submatch, "end")) + // Endpoints are rebased independently, so ordering is checked explicitly: an inverted + // range is not a usable coordinate pair even when both endpoints are addressable. + return start === undefined || end === undefined || start > end ? [] : [{ ...decoded, start, end }] + }) + : submatches, + }, + } + return normalized +} +// altimate_change end + export class Error extends Schema.TaggedErrorClass()("Ripgrep.Error", { message: Schema.String, cause: Schema.optional(Schema.Defect), @@ -226,71 +388,130 @@ export const layer = Layer.effect( Effect.map((result) => result.items), Effect.catchTag("Ripgrep.InvalidPatternError", (cause) => Effect.fail(failure(cause.message, cause))), ), + // altimate_change start — upstream_fix: tally skipped records for one aggregate warning. + // Upstream returns `run(...)` directly. `Effect.suspend` — rather than a plain block body — + // is what makes the tally per EXECUTION: an Effect is a value that can be run more than once + // and concurrently, so a tally captured when the Effect is built would accumulate across runs + // and over-report. + // + // The marked region covers the whole implementation rather than just these lines, because the + // wrapper re-indents every line of the body: an upstream change anywhere in here genuinely + // needs manual reconciliation, which is exactly what the marker is for. grep: (input) => - run({ - ...input, - args: [ - "--no-config", - "--json", - "--hidden", - "--no-messages", - // altimate_change start — upstream_fix: preserve all debug rg search --glob entries - ...(typeof input.include === "string" - ? [`--glob=${input.include}`] - : (input.include ?? []).map((pattern) => `--glob=${pattern}`)), - // altimate_change end - "--glob=!**/.git/**", - "--", - input.pattern, - input.file ?? ".", - ], - parse: (line) => - (Buffer.byteLength(line, "utf8") > MAX_RECORD_BYTES - ? Effect.fail(failure(`Ripgrep JSON record exceeded ${MAX_RECORD_BYTES} bytes`)) - : Effect.try({ + Effect.suspend(() => { + const skipped: { count: number; samples: string[] } = { count: 0, samples: [] } + return run({ + ...input, + args: [ + "--no-config", + "--json", + "--hidden", + "--no-messages", + // altimate_change start — upstream_fix: preserve all debug rg search --glob entries + ...(typeof input.include === "string" + ? [`--glob=${input.include}`] + : (input.include ?? []).map((pattern) => `--glob=${pattern}`)), + // altimate_change end + "--glob=!**/.git/**", + "--", + input.pattern, + input.file ?? ".", + ], + // altimate_change start — upstream_fix: a bad record skips itself, never the search. + // `parse` runs inside `Stream.mapEffect`, so ANY failure here aborts the whole stream and + // discards every match already collected from unrelated files. A record is independent of + // its neighbours, so none of the three ways one can be unusable — oversized, unparseable + // JSON, or schema-rejected — justifies destroying the rest of the search. + parse: (line) => { + const bytes = Buffer.byteLength(line, "utf8") + // Captured during the walk so the aggregate warning can name a file when one is + // recoverable. Malformed JSON has no path by definition, hence "when present". + let where: string | undefined + return Effect.gen(function* () { + // Checked before JSON.parse purely to bound parse cost; `Stream.splitLines` has + // already materialized the line, so this cannot bound memory. See MAX_RECORD_BYTES. + if (bytes > MAX_RECORD_BYTES) + return yield* Effect.fail(failure(`record exceeded ${MAX_RECORD_BYTES} bytes`)) + const json = yield* Effect.try({ try: () => JSON.parse(line) as unknown, - catch: (cause) => failure("Invalid ripgrep JSON output", cause), + catch: (cause) => failure("unparseable JSON", cause), }) - ).pipe( - Effect.flatMap((json) => { - if (!json || typeof json !== "object" || !("type" in json) || json.type !== "match") - return Effect.succeed(undefined) - return Schema.decodeUnknownEffect(RawMatch)(json).pipe( - Effect.map((match) => ({ - ...match.data, - path: { text: match.data.path.text.replace(/^\.[\\/]/, "") }, - submatches: match.data.submatches.slice(0, MAX_SUBMATCHES), - })), - Effect.mapError((cause) => failure("Invalid ripgrep match output", cause)), + if (!json || typeof json !== "object" || !("type" in json)) + return yield* Effect.fail(failure("record has no type")) + // Captured before the type check so an unrecognised record can still name its file. + const pathField = readProp(readProp(json, "data"), "path") + const pathText = readProp(pathField, "text") + if (typeof pathText === "string") where = pathText + // Control records are expected and simply carry no match. An unrecognised type is a + // protocol surprise and is counted rather than dropped on the floor, so a ripgrep + // change cannot quietly turn every match into "no matches". + if (json.type !== "match") + return typeof json.type === "string" && CONTROL_TYPES.has(json.type) + ? undefined + : yield* Effect.fail(failure(`unrecognised record type ${JSON.stringify(json.type)}`)) + const match = yield* Schema.decodeUnknownEffect(RawMatch)(normalizeMatch(json)).pipe( + Effect.mapError((cause) => failure("unexpected match shape", cause)), ) + // `normalizeMatch` already caps submatches and line text, so nothing is re-trimmed. + return { ...match.data, path: { text: match.data.path.text.replace(/^\.[\\/]/, "") } } + }).pipe( + Effect.catch((cause) => + Effect.sync(() => { + skipped.count++ + if (skipped.samples.length < SKIP_SAMPLES) + skipped.samples.push(where ? `${cause.message} (${where})` : cause.message) + return undefined + }), + ), + ) + }, + // altimate_change end + }).pipe( + // altimate_change start — upstream_fix: one aggregate warning per search, not per record. + // A systematic protocol mismatch rejects every record in the tree, and a per-record log + // would bury the machine in noise while still answering with an innocent-looking empty + // result — the very failure this change exists to prevent. + // `onExit` rather than `tap`: a search that fails or is interrupted part-way is exactly + // when the diagnostic matters, and `tap` would discard the tally in both cases. + Effect.onExit(() => + skipped.count > 0 + ? Effect.logWarning("skipped unusable ripgrep records", { + skipped: skipped.count, + reasons: skipped.samples, + }) + : Effect.void, + ), + // altimate_change end + Effect.map((result) => + result.items.map((match) => { + const relative = match.path.text + .replace(/^(?:\.[\\/])+/u, "") + .replace(/^[\\/]+/u, "") + .replaceAll("\\", "/") + const absolute = path.resolve(input.cwd, relative) + return new Match({ + entry: new Entry({ + path: RelativePath.make(relative), + type: "file", + mime: FSUtil.mimeType(absolute), + }), + line: match.line_number, + offset: match.absolute_offset, + // altimate_change start — upstream_fix: capped at parse time, see LINE_TEXT_CAP. + // Re-applied here so the cap still holds if the parser ever stops trimming. + text: capLineText(match.lines.text), + // altimate_change end + submatches: match.submatches.map((submatch) => ({ + text: submatch.match.text, + start: submatch.start, + end: submatch.end, + })), + }) }), ), - }).pipe( - Effect.map((result) => - result.items.map((match) => { - const relative = match.path.text - .replace(/^(?:\.[\\/])+/u, "") - .replace(/^[\\/]+/u, "") - .replaceAll("\\", "/") - const absolute = path.resolve(input.cwd, relative) - return new Match({ - entry: new Entry({ - path: RelativePath.make(relative), - type: "file", - mime: FSUtil.mimeType(absolute), - }), - line: match.line_number, - offset: match.absolute_offset, - text: match.lines.text.length > 2_000 ? match.lines.text.slice(0, 2_000) + "..." : match.lines.text, - submatches: match.submatches.map((submatch) => ({ - text: submatch.match.text, - start: submatch.start, - end: submatch.end, - })), - }) - }), - ), - ), + ) + }), + // altimate_change end — closes the grep block opened above for the skip tally. }) }), ) diff --git a/packages/core/test/ripgrep.test.ts b/packages/core/test/ripgrep.test.ts index da8e7519ce..02e043d0de 100644 --- a/packages/core/test/ripgrep.test.ts +++ b/packages/core/test/ripgrep.test.ts @@ -1,8 +1,10 @@ -import { describe, expect } from "bun:test" +import { beforeEach, describe, expect, test as bunTest } from "bun:test" import fs from "fs/promises" import path from "path" -import { Effect } from "effect" +import { Effect, Layer, Logger } from "effect" import { Ripgrep } from "@opencode-ai/core/ripgrep" +import { RipgrepBinary } from "@opencode-ai/core/ripgrep/binary" +import { AppProcess } from "@opencode-ai/core/process" import { RelativePath } from "@opencode-ai/core/schema" import { tmpdir } from "./fixture/tmpdir" import { testEffect } from "./lib/effect" @@ -87,5 +89,429 @@ describe("Ripgrep", () => { (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), ), ) + + // upstream_fix: a ripgrep `--json` match record embeds the whole matched line, so a minified + // bundle or single-line JSON fixture emits a record far past any per-record ceiling. That used to + // fail the stream, taking every unrelated match in the search down with it. + it.live("keeps matching unrelated files when one file has an oversized line", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => + Effect.gen(function* () { + yield* Effect.promise(() => fs.writeFile(path.join(tmp.path, "a-small.txt"), "needle here\n")) + // Well past the old 64 KiB ceiling, well under the current sanity limit. + yield* Effect.promise(() => + fs.writeFile(path.join(tmp.path, "b-minified.js"), "x".repeat(100_000) + "needle" + "y".repeat(100_000)), + ) + + const matches = yield* (yield* Ripgrep.Service).grep({ cwd: tmp.path, pattern: "needle", limit: 10 }) + + // Both the bystander and the oversized file are reported; neither is lost to a failure. + expect(matches.map((item) => item.entry.path).sort()).toEqual([ + RelativePath.make("a-small.txt"), + RelativePath.make("b-minified.js"), + ]) + }), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + + // upstream_fix: ripgrep emits `{"bytes": ""}` instead of `{"text": ...}` for a line that + // is not valid UTF-8. The schema modelled only the `text` arm, so one stray byte failed the whole + // search — the same abort-everything shape as the oversized record. This drives real ripgrep; + // the exact decoding is pinned by the stubbed case below. + it.live("returns matches from files containing non-UTF8 lines", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => + Effect.gen(function* () { + yield* Effect.promise(() => fs.writeFile(path.join(tmp.path, "a-plain.txt"), "needle here\n")) + yield* Effect.promise(() => + fs.writeFile(path.join(tmp.path, "b-binary.txt"), Buffer.from("needle \xff\xfe tail\n", "binary")), + ) + yield* Effect.promise(() => fs.writeFile(path.join(tmp.path, "c-plain.txt"), "needle here\n")) + + const matches = yield* (yield* Ripgrep.Service).grep({ cwd: tmp.path, pattern: "needle", limit: 10 }) + + // The non-UTF8 file is reported like any other rather than dropped or fatal. + expect(matches.map((item) => item.entry.path).sort()).toEqual([ + RelativePath.make("a-plain.txt"), + RelativePath.make("b-binary.txt"), + RelativePath.make("c-plain.txt"), + ]) + // Undecodable bytes become U+FFFD, so the surrounding text stays readable. + const binary = matches.find((item) => item.entry.path === RelativePath.make("b-binary.txt")) + expect(binary?.text).toContain("needle") + expect(binary?.text).toContain("tail") + }), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + + // Real ripgrep cannot be coerced into emitting a chosen bad record — `--` makes the next arg the + // pattern — so these cases drive the parser through a stub `rg` that prints exactly the NDJSON + // given. Only the executable is stubbed: the real spawn, decode, line splitting, parse, collection + // and output mapping all still run. Plain `bunTest` because these supply their own Ripgrep layer, + // which the ambient `testEffect(Ripgrep.defaultLayer)` would otherwise shadow. + const matchRecord = (file: string, overrides: Record = {}) => + JSON.stringify({ + type: "match", + data: { + path: { text: `./${file}` }, + lines: { text: "needle\n" }, + line_number: 1, + absolute_offset: 0, + submatches: [{ match: { text: "needle" }, start: 0, end: 6 }], + ...overrides, + }, + }) + + /** + * Collected so the skip *count* can be asserted — it is invisible in the returned matches. + * `Effect.logWarning(message, data)` puts both into `entry.message` as a tuple, not annotations. + */ + const skipWarnings: Array<{ skipped?: number; reasons?: string[] }> = [] + const captureWarnings = Logger.layer([ + Logger.formatStructured.pipe( + Logger.map((entry): void => { + const parts: unknown[] = Array.isArray(entry.message) ? entry.message : [entry.message] + if (parts[0] !== "skipped unusable ripgrep records") return + const data = parts[1] + if (!data || typeof data !== "object") return + const skipped = Reflect.get(data, "skipped") + const reasons = Reflect.get(data, "reasons") + skipWarnings.push({ + skipped: typeof skipped === "number" ? skipped : undefined, + reasons: Array.isArray(reasons) ? reasons.map(String) : undefined, + }) + }), + ), + ]) + + // The stub is a POSIX shell script and `chmod` is a no-op on win32, so the spawn cannot work + // there. Be clear about the cost: this leaves the record-parsing behaviour below — skipping, + // decoding, offset rebasing, the size ceiling — WITHOUT Windows coverage. script/windows- + // ripgrep-e2e.ts covers only binary resolution, extraction and one real search, not any of this. + // Closing the gap needs a `.cmd` stub on win32; the parser itself is platform-independent, so the + // risk is a Windows-only spawn/quoting regression going unnoticed rather than a parsing one. + const stubTest = bunTest.skipIf(process.platform === "win32") + + const grepWithStubbedRecords = (records: string[]) => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => + Effect.gen(function* () { + const data = path.join(tmp.path, "records.jsonl") + yield* Effect.promise(() => fs.writeFile(data, records.join("\n") + "\n")) + const stub = path.join(tmp.path, "rg") + yield* Effect.promise(() => fs.writeFile(stub, `#!/bin/sh\ncat ${JSON.stringify(data)}\n`)) + yield* Effect.promise(() => fs.chmod(stub, 0o755)) + + return yield* Effect.gen(function* () { + const rg = yield* Ripgrep.Service + return yield* rg.grep({ cwd: tmp.path, pattern: "needle", limit: 100 }) + }).pipe( + Effect.provide( + Ripgrep.layer.pipe( + Layer.provide( + Layer.succeed(RipgrepBinary.Service, RipgrepBinary.Service.of({ filepath: Effect.succeed(stub) })), + ), + Layer.provide(AppProcess.defaultLayer), + ), + ), + Effect.provide(captureWarnings), + ) + }), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ) + + beforeEach(() => { + skipWarnings.length = 0 + }) + + /** The single aggregate warning for the last search, or undefined when nothing was skipped. */ + const lastSkip = () => skipWarnings.at(-1) + + stubTest("skips an unparseable record without failing the search", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt"), + '{"type":"match","data":{"path":{"text":"./b.t', // truncated mid-JSON + matchRecord("c.txt"), + ]), + ) + + // The malformed middle record is dropped; the records on either side survive. + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt"), RelativePath.make("c.txt")]) + expect(lastSkip()).toEqual({ skipped: 1, reasons: ["unparseable JSON"] }) + }) + + // The size ceiling is asserted on a record built to exceed it, rather than inferred from a large + // file — that keeps the case independent of whether a given ripgrep build emits the match at all. + stubTest( + "skips an oversized record and keeps parsing the records after it", + async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt"), + matchRecord("b-huge.txt", { lines: { text: "needle" + "x".repeat(17 * 1024 * 1024) } }), + matchRecord("c.txt"), + ]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt"), RelativePath.make("c.txt")]) + // Explicit timeout: this materialises a >16 MiB record, so it is slow enough to trip the default + // under load even though the parser rejects the record before parsing it. + }, + 30_000, + ) + + // A path is an identifier the caller reopens, so it must never be lossily decoded. Such a record + // is skipped rather than reported under a U+FFFD-mangled path that names no real file. + stubTest("skips a match whose path is not valid UTF-8, keeping the rest", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt"), + matchRecord("ignored", { path: { bytes: Buffer.from("./b\xff.txt", "binary").toString("base64") } }), + matchRecord("c.txt"), + ]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt"), RelativePath.make("c.txt")]) + }) + + // `Buffer.from` maps unconvertible base64 to an empty buffer instead of throwing, which would turn + // a corrupt record into a schema-valid EMPTY match. It must be skipped, not silently emptied. + stubTest("skips a record whose bytes field is not valid base64", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt"), + matchRecord("b.txt", { lines: { bytes: "!!!not base64!!!" } }), + matchRecord("c.txt"), + ]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt"), RelativePath.make("c.txt")]) + }) + + // Submatch offsets are BYTE offsets into the raw line. A lossy decode widens every undecodable + // byte to a 3-byte U+FFFD, so the raw offsets no longer locate the match and must be rebased onto + // the decoded text's own UTF-8 encoding. Without this the match reads "��need". + stubTest("rebases submatch offsets onto the decoded line after a lossy decode", async () => { + const raw = Buffer.concat([Buffer.from([0xff]), Buffer.from("needle tail\n")]) + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { + lines: { bytes: raw.toString("base64") }, + // "needle" sits at raw bytes [1, 7). + submatches: [{ match: { bytes: Buffer.from("needle").toString("base64") }, start: 1, end: 7 }], + }), + ]), + ) + + expect(matches).toHaveLength(1) + const [{ text, submatches }] = matches + expect(submatches[0]).toEqual({ text: "needle", start: 3, end: 9 }) + // The contract is byte offsets into the returned text, so slice its UTF-8 encoding. + expect(Buffer.from(text, "utf8").subarray(submatches[0].start, submatches[0].end).toString("utf8")).toBe("needle") + }) + + // `Buffer.subarray` clamps an out-of-range end and truncates a fractional one instead of throwing, + // so rebasing without a range check would turn a corrupt offset into a plausible-looking one. + // The MATCH still survives — only the coordinate we cannot express is dropped. + stubTest("drops a submatch whose offset is not addressable, keeping the match", async () => { + const raw = Buffer.concat([Buffer.from([0xff]), Buffer.from("needle tail\n")]) + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("b.txt", { + lines: { bytes: raw.toString("base64") }, + // Well past the end of the raw line — nonsense that must not be silently clamped. + submatches: [{ match: { text: "needle" }, start: 1, end: 9_999 }], + }), + ]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("b.txt")]) + expect(matches[0].submatches).toEqual([]) + expect(matches[0].text).toContain("needle") + }) + + // A byte-mode pattern can match inside a valid multi-byte character, where the offset is simply not + // expressible in the decoded line. Byte-boundary validation catches it; a decoded-string + // comparison does not, because an invalid byte followed by a LITERAL U+FFFD aliases — both + // prefixes decode to the same replacement text. + stubTest("drops a submatch whose offset splits a multi-byte character", async () => { + const raw = Buffer.concat([Buffer.from("é"), Buffer.from("needle")]) + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("b.txt", { + lines: { bytes: raw.toString("base64") }, + submatches: [{ match: { text: "needle" }, start: 1, end: 3 }], + }), + ]), + ) + + expect(matches[0].submatches).toEqual([]) + }) + + // The `{text}` arm is not rebased, but an offset still has to be addressable in the line. + stubTest("drops a text-arm submatch whose offset is past the end of the line", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { + lines: { text: "needle\n" }, + submatches: [{ match: { text: "needle" }, start: 0, end: 999 }], + }), + ]), + ) + + expect(matches[0].submatches).toEqual([]) + }) + + stubTest("drops a text-arm submatch whose offset splits a multi-byte character", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { lines: { text: "éa" }, submatches: [{ match: { text: "a" }, start: 1, end: 3 }] }), + matchRecord("b.txt", { lines: { text: "éa" }, submatches: [{ match: { text: "a" }, start: 2, end: 3 }] }), + ]), + ) + + // Offset 1 splits `é`; offset 2 is the character boundary, so that submatch is kept. + expect(matches[0].submatches).toEqual([]) + expect(matches[1].submatches).toHaveLength(1) + }) + + // Endpoints are rebased independently, so an inverted range can survive both endpoint checks. + stubTest("drops a submatch whose range is inverted", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { + lines: { bytes: Buffer.from("needle tail\n").toString("base64") }, + submatches: [{ match: { text: "needle" }, start: 6, end: 2 }], + }), + ]), + ) + + expect(matches[0].submatches).toEqual([]) + }) + + stubTest("drops a submatch whose offset sits inside a literal replacement character", async () => { + // Invalid byte, then the three bytes that spell U+FFFD. Offset 2 is inside that literal + // character, but both decoded prefixes read as the same replacement text. + const raw = Buffer.from([0xff, 0xef, 0xbf, 0xbd]) + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("b.txt", { + lines: { bytes: raw.toString("base64") }, + submatches: [{ match: { text: "x" }, start: 2, end: 4 }], + }), + ]), + ) + + expect(matches[0].submatches).toEqual([]) + }) + + // A broad pattern such as `x.*` makes ripgrep repeat nearly the whole line in the submatch text, + // which would defeat the retained-memory bound that capping `lines.text` establishes. + stubTest("caps the retained submatch text, not just the line", async () => { + const huge = "n".repeat(50_000) + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { + lines: { text: huge }, + submatches: [{ match: { text: huge }, start: 0, end: huge.length }], + }), + ]), + ) + + expect(matches[0].submatches[0].text).toHaveLength(2_003) + expect(matches[0].submatches[0].text.endsWith("...")).toBe(true) + }) + + stubTest("skips a record whose bytes field is empty rather than emitting an empty match", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([matchRecord("a.txt"), matchRecord("b.txt", { lines: { bytes: "" } })]), + ) + + // "" is spelled like valid base64 but a matched line is never empty, so the record is corrupt. + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt")]) + }) + + stubTest("skips a record whose bytes field uses non-canonical padding", async () => { + // "Zh==" and "Zg==" both decode to "f"; only the canonical spelling round-trips. + const matches = await Effect.runPromise( + grepWithStubbedRecords([matchRecord("a.txt"), matchRecord("b.txt", { lines: { bytes: "Zh==" } })]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt")]) + }) + + // Control records carry no match and are ignored silently; an unrecognised type is a protocol + // surprise and must be counted, or a ripgrep change turns every match into an innocent "no match". + stubTest("ignores control records but counts records with an unknown type", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + JSON.stringify({ type: "begin", data: { path: { text: "./a.txt" } } }), + matchRecord("a.txt"), + JSON.stringify({ type: "match-v2", data: { path: { text: "./b.txt" } } }), + JSON.stringify({}), + JSON.stringify({ type: "end", data: { path: { text: "./a.txt" } } }), + matchRecord("c.txt"), + ]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt"), RelativePath.make("c.txt")]) + // begin/end are silent; the unknown type and the typeless record are counted, not dropped + // silently — the whole point being that a protocol change cannot masquerade as "no matches". + expect(lastSkip()?.skipped).toBe(2) + expect(lastSkip()?.reasons).toEqual([`unrecognised record type "match-v2" (./b.txt)`, "record has no type"]) + }) + + stubTest("returns an empty result when every record is unusable, rather than failing", async () => { + const matches = await Effect.runPromise(grepWithStubbedRecords(["{oops", "{also oops", "{still oops"])) + + expect(matches).toEqual([]) + }) + + // Pins the ceiling itself: at the limit the record is kept, one byte over it is skipped. + stubTest("accepts a record at exactly the size ceiling and skips one byte over", async () => { + const sizeOf = (file: string, padding: number) => matchRecord(file, { lines: { text: "n".repeat(padding) } }) + const overhead = Buffer.byteLength(sizeOf("a.txt", 0), "utf8") + const limit = 16 * 1024 * 1024 + + const matches = await Effect.runPromise( + grepWithStubbedRecords([sizeOf("a.txt", limit - overhead), sizeOf("b.txt", limit - overhead + 1)]), + ) + + expect(matches.map((item) => item.entry.path)).toEqual([RelativePath.make("a.txt")]) + }) + + // Pins the OUTPUT contract of the cap. Note it cannot prove the retained-memory improvement that + // motivated moving the cap into the parser: capping at parse time and capping at the end produce + // byte-identical output, and only the peak heap during collection differs. + stubTest("caps the returned line text and keeps the elision marker", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([matchRecord("a.txt", { lines: { text: "needle" + "x".repeat(50_000) } })]), + ) + + expect(matches[0].text).toHaveLength(2_003) + expect(matches[0].text.endsWith("...")).toBe(true) + }) + + stubTest("decodes a non-UTF8 match line to replacement characters", async () => { + const matches = await Effect.runPromise( + grepWithStubbedRecords([ + matchRecord("a.txt", { + lines: { bytes: Buffer.from("needle \xff\xfe tail\n", "binary").toString("base64") }, + submatches: [{ match: { bytes: Buffer.from("needle", "binary").toString("base64") }, start: 0, end: 6 }], + }), + ]), + ) + + expect(matches).toHaveLength(1) + // Content is display text, so lossy decoding keeps the match usable rather than dropping it. + expect(matches[0].text).toBe("needle �� tail\n") + expect(matches[0].submatches[0].text).toBe("needle") + }) // altimate_change end }) diff --git a/packages/opencode/src/file/ripgrep-records.ts b/packages/opencode/src/file/ripgrep-records.ts new file mode 100644 index 0000000000..5006a7648d --- /dev/null +++ b/packages/opencode/src/file/ripgrep-records.ts @@ -0,0 +1,244 @@ +// altimate_change start — upstream_fix: ripgrep NDJSON record parsing, split out of ripgrep.ts. +// +// This lives in its own module rather than inside the `Ripgrep` namespace because it is pure, +// process-free logic that deserves direct tests. Inside the namespace it was reachable only two +// ways, both bad: projected through `export * as` as an implementation detail of the public +// namespace, or driven through `search()` with a stub `rg` on PATH — and that binary lookup is +// memoised per process, so such a stub leaks into every later test file in the same `bun test` +// run and breaks unrelated suites. +// +// Module shape follows packages/opencode/AGENTS.md: flat top-level exports with a self-reexport at +// the bottom, not `export namespace`. +// +// It mirrors packages/core/src/ripgrep.ts, but the two are not identical by design. The core parser +// streams; this one buffers all of stdout before splitting, and hands its records straight to the +// `/find` response. Both cap retained text, bound submatch counts, skip records they cannot use, +// and report the skips once per search rather than once per record. +import z from "zod" +import { Log } from "@/util/log" + +const log = Log.create({ service: "ripgrep" }) + +const Stats = z.object({ + elapsed: z.object({ + secs: z.number(), + nanos: z.number(), + human: z.string(), + }), + searches: z.number(), + searches_with_match: z.number(), + bytes_searched: z.number(), + bytes_printed: z.number(), + matched_lines: z.number(), + matches: z.number(), +}) + +const Begin = z.object({ + type: z.literal("begin"), + data: z.object({ + path: z.object({ + text: z.string(), + }), + }), +}) + +export const Match = z.object({ + type: z.literal("match"), + data: z.object({ + path: z.object({ + text: z.string(), + }), + lines: z.object({ + text: z.string(), + }), + line_number: z.number(), + absolute_offset: z.number(), + submatches: z.array( + z.object({ + match: z.object({ + text: z.string(), + }), + start: z.number(), + end: z.number(), + }), + ), + }), +}) + +const End = z.object({ + type: z.literal("end"), + data: z.object({ + path: z.object({ + text: z.string(), + }), + binary_offset: z.number().nullable(), + stats: Stats, + }), +}) + +const Summary = z.object({ + type: z.literal("summary"), + data: z.object({ + elapsed_total: z.object({ + human: z.string(), + nanos: z.number(), + secs: z.number(), + }), + stats: Stats, + }), +}) + +const Result = z.union([Begin, Match, End, Summary]) + +// Tolerating ripgrep's `{bytes}` arm and malformed lines. (The whole file is covered by the +// marker at the top — this module is new, not an edit to upstream code.) +// +// This mirrors packages/core/src/ripgrep.ts, but deliberately not in every respect. That parser +// streams, so it caps the retained line text and rebases submatch offsets; this one buffers all of +// stdout up front and hands its records straight to the `/find` response, where the raw ripgrep +// shape is the published contract — so it normalises and skips, and leaves the shape alone. Both +// report skipped records once per search rather than once per record. +const BASE64 = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/ + +/** Mirrors packages/core/src/ripgrep.ts. Bounds parse cost per record on this path too. */ +const MAX_RECORD_BYTES = 16 * 1024 * 1024 + +// Also mirrors core. Each submatch costs a rebase per endpoint, and a rebase allocates a string up +// to the length of the line, so an unbounded submatch array turns one in-ceiling record into +// O(count x line) work. A protocol change is exactly the shape that would emit one. +const MAX_SUBMATCHES = 100 + +// `MAX_RECORD_BYTES` bounds ONE input record; it does not bound what the response retains. This +// path buffers all of stdout and returns every match, so without a per-field cap a tree of large +// records still retains — and serialises into the `/find` response — an unbounded amount of text. +// Same cap and elision marker as the core parser, so the two paths agree on what a match shows. +const LINE_TEXT_CAP = 2_000 +const capText = (text: string) => (text.length > LINE_TEXT_CAP ? text.slice(0, LINE_TEXT_CAP) + "..." : text) + +/** Parse one NDJSON record, rewriting `{bytes: base64}` fields into the `{text}` arm. */ +const normalizeRecord = (line: string): unknown => { + let json: unknown + try { + json = JSON.parse(line) + } catch { + return undefined + } + if (!json || typeof json !== "object") return json + const read = (value: unknown, key: string): unknown => + value !== null && typeof value === "object" && key in value ? Reflect.get(value, key) : undefined + const data = read(json, "data") + if (!data || typeof data !== "object") return json + /** Decode a `{text}`/`{bytes}` field, returning the raw buffer so offsets can be rebased. */ + const decode = (value: unknown): { text: string; raw?: Buffer } | undefined => { + if (!value || typeof value !== "object") return undefined + const text = read(value, "text") + if (typeof text === "string") return { text } + const bytes = read(value, "bytes") + // Guarded three ways because `Buffer.from` decodes unconvertible input to an EMPTY buffer + // instead of throwing, which would turn a corrupt record into a schema-valid empty match: + // reject the empty string (a matched line is never empty), check the spelling, then require + // a round-trip so non-canonical padding ("Zh==" and "Zg==" both decode to "f") is rejected. + if (typeof bytes !== "string" || bytes.length === 0 || !BASE64.test(bytes)) return undefined + const decoded = Buffer.from(bytes, "base64") + if (decoded.toString("base64") !== bytes) return undefined + return { text: decoded.toString("utf8"), raw: decoded } + } + const lines = "lines" in data ? decode(read(data, "lines")) : undefined + // Submatch offsets are BYTE offsets into the RAW line, and a lossy decode widens every + // undecodable byte to a 3-byte U+FFFD — so they must be rebased onto the decoded text's own + // UTF-8 encoding or they no longer locate the match. This response shape is published by the + // `/find` route, so unrebased offsets would be newly wrong output rather than a skipped record. + // Mirrors packages/core/src/ripgrep.ts. + const raw = lines?.raw + // Byte-boundary validation, matching packages/core/src/ripgrep.ts. A decoded-string comparison + // is not sufficient: when an invalid byte precedes a LITERAL U+FFFD, an offset inside that + // character still yields a prefix that prefixes the line, because the replacement characters + // alias. A continuation byte (0b10xxxxxx) at the offset means the split lands inside a sequence. + // An offset that cannot be rebased drops ITS SUBMATCH, not the record — the file, line and text + // stay correct, and losing a highlight range beats losing the match. + const isContinuationByte = (byte: number | undefined) => byte !== undefined && (byte & 0xc0) === 0x80 + // Encoded once per record and shared by every submatch; also supplies the byte length. + const textBytes = raw ?? (lines ? Buffer.from(lines.text, "utf8") : Buffer.alloc(0)) + const rebase = (offset: unknown): number | undefined => { + // `{text}` arm: nothing is rebased, but the offset must still be addressable AND land on a + // character boundary — a byte offset can fall inside a multi-byte sequence (`éa`, offset 1 + // splits `é`). `z.number()` rejects none of that, so a corrupt record would otherwise reach the + // `/find` response with coordinates that index nothing or half a character. + if (!raw) + return typeof offset === "number" && + Number.isInteger(offset) && + offset >= 0 && + offset <= textBytes.length && + !(offset !== 0 && offset !== textBytes.length && isContinuationByte(textBytes[offset])) + ? offset + : undefined + if (typeof offset !== "number" || !Number.isInteger(offset) || offset < 0 || offset > raw.length) return undefined + if (offset !== 0 && offset !== raw.length && isContinuationByte(raw[offset])) return undefined + return Buffer.byteLength(raw.subarray(0, offset).toString("utf8"), "utf8") + } + const submatches = read(data, "submatches") + // Only rewrite keys the record actually carries — `begin`/`end`/`summary` records reach here too + // and must keep their exact shape, or the strict union below would reject them. + // `path` is deliberately left alone: decoding it is lossy, and a path is an identifier the + // caller reopens, so a U+FFFD-mangled path names a file that does not exist. Such a record + // stays in the `{bytes}` arm and is skipped. See packages/core/src/ripgrep.ts. + const normalized = { + ...json, + data: { + ...data, + ...(lines ? { lines: { text: capText(lines.text) } } : {}), + ...(Array.isArray(submatches) + ? { + submatches: submatches.slice(0, MAX_SUBMATCHES).flatMap((submatch) => { + if (!submatch || typeof submatch !== "object") return [submatch] + const match = decode(read(submatch, "match")) + if (!match) return [submatch] + const start = rebase(read(submatch, "start")) + const end = rebase(read(submatch, "end")) + // Endpoints are rebased independently, so ordering is checked explicitly. + if (start === undefined || end === undefined || start > end) return [] + return [{ ...submatch, match: { text: capText(match.text) }, start, end }] + }), + } + : {}), + }, + } + return normalized +} + +/** + * Turn ripgrep NDJSON lines into match data, skipping records that cannot be used. + * + * `JSON.parse` + a strict `Result.parse` on every line meant one unusable record threw out of + * `search()` and discarded every match already collected from unrelated files — the same defect + * fixed in packages/core/src/ripgrep.ts. Records are independent, so a bad one is dropped and + * counted. Pure and process-free, so `test/file/ripgrep-records.test.ts` drives it directly. + */ +export function parseRecords(lines: string[]): Match["data"][] { + const matches: Match["data"][] = [] + let skipped = 0 + for (const line of lines) { + // Bounds parse cost per record. This path buffers all of stdout before splitting, so it does + // not bound total memory — that needs streaming, tracked separately. + const parsed = + Buffer.byteLength(line, "utf8") > MAX_RECORD_BYTES ? undefined : Result.safeParse(normalizeRecord(line)) + if (!parsed?.success) { + skipped++ + continue + } + if (parsed.data.type === "match") matches.push(parsed.data.data) + } + // Counted and reported once rather than per record: without this a ripgrep protocol change + // would make `/find` answer `[]`, which is indistinguishable from an honest "no matches". + if (skipped > 0) log.warn("skipped unusable ripgrep records", { skipped, total: lines.length }) + return matches +} + +export type Result = z.infer +export type Match = z.infer +export type Begin = z.infer +export type End = z.infer +export type Summary = z.infer + +export * as RipgrepRecords from "./ripgrep-records" +// altimate_change end diff --git a/packages/opencode/src/file/ripgrep.ts b/packages/opencode/src/file/ripgrep.ts index 61fd9a9b6f..353c987acc 100644 --- a/packages/opencode/src/file/ripgrep.ts +++ b/packages/opencode/src/file/ripgrep.ts @@ -20,85 +20,23 @@ import { text } from "node:stream/consumers" import { ZipReader, BlobReader, BlobWriter } from "@zip.js/zip.js" import { Log } from "@/util/log" +import { RipgrepRecords } from "./ripgrep-records" export namespace Ripgrep { const log = Log.create({ service: "ripgrep" }) - const Stats = z.object({ - elapsed: z.object({ - secs: z.number(), - nanos: z.number(), - human: z.string(), - }), - searches: z.number(), - searches_with_match: z.number(), - bytes_searched: z.number(), - bytes_printed: z.number(), - matched_lines: z.number(), - matches: z.number(), - }) - - const Begin = z.object({ - type: z.literal("begin"), - data: z.object({ - path: z.object({ - text: z.string(), - }), - }), - }) - - export const Match = z.object({ - type: z.literal("match"), - data: z.object({ - path: z.object({ - text: z.string(), - }), - lines: z.object({ - text: z.string(), - }), - line_number: z.number(), - absolute_offset: z.number(), - submatches: z.array( - z.object({ - match: z.object({ - text: z.string(), - }), - start: z.number(), - end: z.number(), - }), - ), - }), - }) - - const End = z.object({ - type: z.literal("end"), - data: z.object({ - path: z.object({ - text: z.string(), - }), - binary_offset: z.number().nullable(), - stats: Stats, - }), - }) - - const Summary = z.object({ - type: z.literal("summary"), - data: z.object({ - elapsed_total: z.object({ - human: z.string(), - nanos: z.number(), - secs: z.number(), - }), - stats: Stats, - }), - }) - - const Result = z.union([Begin, Match, End, Summary]) - - export type Result = z.infer - export type Match = z.infer - export type Begin = z.infer - export type End = z.infer - export type Summary = z.infer + // altimate_change start — upstream_fix: record parsing lives in ./ripgrep-records so it can be + // tested directly, without exporting an implementation detail through this namespace and without + // a stub binary whose per-process memoisation leaks into other test files. `Match` stays exported + // here because server/routes/file.ts builds the `/find` response schema from it. + export const Match = RipgrepRecords.Match + const parseRecords = RipgrepRecords.parseRecords + // altimate_change end + + export type Result = RipgrepRecords.Result + export type Match = RipgrepRecords.Match + export type Begin = RipgrepRecords.Begin + export type End = RipgrepRecords.End + export type Summary = RipgrepRecords.Summary const PLATFORM = { "arm64-darwin": { platform: "aarch64-apple-darwin", extension: "tar.gz" }, "arm64-linux": { @@ -374,11 +312,9 @@ export namespace Ripgrep { const lines = result.text.trim().split(/\r?\n/).filter(Boolean) // Parse JSON lines from ripgrep output - return lines - .map((line) => JSON.parse(line)) - .map((parsed) => Result.parse(parsed)) - .filter((r) => r.type === "match") - .map((r) => r.data) + // altimate_change start — upstream_fix: a bad record skips itself, not the whole search. + return parseRecords(lines) + // altimate_change end } } // altimate_change end diff --git a/packages/opencode/test/file/ripgrep-records.test.ts b/packages/opencode/test/file/ripgrep-records.test.ts new file mode 100644 index 0000000000..d2b94a187d --- /dev/null +++ b/packages/opencode/test/file/ripgrep-records.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, test } from "bun:test" +import { RipgrepRecords } from "../../src/file/ripgrep-records" + +// altimate_change start — upstream_fix: legacy `/find` parsing must survive unusable records. +// `search()` used to `JSON.parse` + strictly `Result.parse` every line, so a single unusable record +// threw out of the whole call and discarded every match already collected from unrelated files — +// the same defect fixed in packages/core/src/ripgrep.ts. This path is reachable from the mounted +// `/find` route (server/routes/file.ts). +// +// These drive the parser directly. It is pure and process-free, so there is no stub binary and no +// PATH mutation: the legacy binary lookup is memoised per process, so a stub would leak into every +// later test file in the same `bun test` run. +const record = (file: string, overrides: Record = {}) => + JSON.stringify({ + type: "match", + data: { + path: { text: `./${file}` }, + lines: { text: "needle\n" }, + line_number: 1, + absolute_offset: 0, + submatches: [{ match: { text: "needle" }, start: 0, end: 6 }], + ...overrides, + }, + }) + +const paths = (records: string[]) => RipgrepRecords.parseRecords(records).map((match) => match.path.text) + +describe("RipgrepRecords.parseRecords", () => { + test("skips an unparseable record and keeps the ones around it", () => { + expect(paths([record("a.txt"), '{"type":"match","data":{"path":{"text":"./b.t', record("c.txt")])).toEqual([ + "./a.txt", + "./c.txt", + ]) + }) + + test("skips a record past the size ceiling", () => { + const huge = record("b.txt", { lines: { text: "n".repeat(17 * 1024 * 1024) } }) + expect(paths([record("a.txt"), huge, record("c.txt")])).toEqual(["./a.txt", "./c.txt"]) + }) + + test("skips a record whose path is not valid UTF-8, rather than mangling the path", () => { + const bad = record("ignored", { path: { bytes: Buffer.from("./b\xff.txt", "binary").toString("base64") } }) + expect(paths([record("a.txt"), bad])).toEqual(["./a.txt"]) + }) + + test("skips empty and non-canonical base64 rather than emitting an empty match", () => { + expect(paths([record("a.txt"), record("b.txt", { lines: { bytes: "" } })])).toEqual(["./a.txt"]) + expect(paths([record("a.txt"), record("b.txt", { lines: { bytes: "Zh==" } })])).toEqual(["./a.txt"]) + }) + + test("decodes a non-UTF8 line and ignores control records", () => { + const parsed = RipgrepRecords.parseRecords([ + JSON.stringify({ type: "begin", data: { path: { text: "./a.txt" } } }), + record("a.txt", { lines: { bytes: Buffer.from("needle \xff tail\n", "binary").toString("base64") } }), + ]) + expect(parsed).toHaveLength(1) + expect(parsed[0].lines.text).toBe("needle � tail\n") + }) + + // Offsets are byte offsets into the RAW line; a lossy decode widens each undecodable byte to a + // 3-byte U+FFFD. This shape is published by the `/find` route, so unrebased offsets would be + // newly wrong output rather than a skipped record. Mirrors the core parser. + test("rebases submatch offsets after a lossy line decode", () => { + const raw = Buffer.concat([Buffer.from([0xff]), Buffer.from("needle tail\n")]) + const parsed = RipgrepRecords.parseRecords([ + record("a.txt", { + lines: { bytes: raw.toString("base64") }, + submatches: [{ match: { bytes: Buffer.from("needle").toString("base64") }, start: 1, end: 7 }], + }), + ]) + + expect(parsed[0].submatches[0]).toEqual({ match: { text: "needle" }, start: 3, end: 9 }) + expect(Buffer.from(parsed[0].lines.text, "utf8").subarray(3, 9).toString("utf8")).toBe("needle") + }) + + // An offset that cannot be expressed in the decoded line drops its submatch, not the match. + test("drops a submatch whose offset is unaddressable, keeping the match", () => { + const raw = Buffer.concat([Buffer.from([0xff]), Buffer.from("needle tail\n")]) + const parsed = RipgrepRecords.parseRecords([ + record("a.txt", { + lines: { bytes: raw.toString("base64") }, + submatches: [{ match: { text: "needle" }, start: 1, end: 9_999 }], + }), + ]) + + expect(parsed).toHaveLength(1) + expect(parsed[0].submatches).toEqual([]) + }) + + test("drops a submatch whose offset splits a character or inverts the range", () => { + const split = RipgrepRecords.parseRecords([ + record("a.txt", { + lines: { bytes: Buffer.concat([Buffer.from("é"), Buffer.from("needle")]).toString("base64") }, + submatches: [{ match: { text: "needle" }, start: 1, end: 3 }], + }), + ]) + expect(split[0].submatches).toEqual([]) + + // Both endpoints are addressable, but the range is inverted. + const inverted = RipgrepRecords.parseRecords([ + record("a.txt", { + lines: { bytes: Buffer.from("needle tail\n").toString("base64") }, + submatches: [{ match: { text: "needle" }, start: 6, end: 2 }], + }), + ]) + expect(inverted[0].submatches).toEqual([]) + }) + + // `MAX_RECORD_BYTES` bounds one input record, not what the `/find` response retains. + test("caps the retained line and submatch text", () => { + const huge = "n".repeat(50_000) + const parsed = RipgrepRecords.parseRecords([ + record("a.txt", { lines: { text: huge }, submatches: [{ match: { text: huge }, start: 0, end: huge.length }] }), + ]) + + expect(parsed[0].lines.text).toHaveLength(2_003) + expect(parsed[0].lines.text.endsWith("...")).toBe(true) + expect(parsed[0].submatches[0].match.text).toHaveLength(2_003) + }) + + // `{text}` arm: nothing is rebased, but `z.number()` accepts negatives, fractions and values past + // the end, so a corrupt record would otherwise reach the `/find` response indexing nothing. + test("drops a text-arm submatch whose offset is not addressable in the line", () => { + const parsed = RipgrepRecords.parseRecords([ + record("a.txt", { lines: { text: "needle\n" }, submatches: [{ match: { text: "needle" }, start: 0, end: 999 }] }), + ]) + expect(parsed[0].submatches).toEqual([]) + + const fractional = RipgrepRecords.parseRecords([ + record("a.txt", { lines: { text: "needle\n" }, submatches: [{ match: { text: "needle" }, start: 0.5, end: 6 }] }), + ]) + expect(fractional[0].submatches).toEqual([]) + }) + + // A byte offset can fall inside a multi-byte character even on a perfectly valid UTF-8 line. + test("drops a text-arm submatch whose offset splits a multi-byte character", () => { + const parsed = RipgrepRecords.parseRecords([ + record("a.txt", { lines: { text: "éa" }, submatches: [{ match: { text: "a" }, start: 1, end: 3 }] }), + ]) + expect(parsed[0].submatches).toEqual([]) + + // The same line with boundary-aligned offsets is kept. + const ok = RipgrepRecords.parseRecords([ + record("a.txt", { lines: { text: "éa" }, submatches: [{ match: { text: "a" }, start: 2, end: 3 }] }), + ]) + expect(ok[0].submatches).toHaveLength(1) + }) + + // Each submatch costs a rebase per endpoint, and a rebase allocates a string up to the line + // length, so an unbounded array turns one in-ceiling record into O(count x line) work. + test("bounds the submatch count like the core parser", () => { + const many = Array.from({ length: 5_000 }, () => ({ match: { text: "n" }, start: 0, end: 1 })) + const parsed = RipgrepRecords.parseRecords([record("a.txt", { submatches: many })]) + expect(parsed[0].submatches).toHaveLength(100) + }) + + test("returns an empty array when every record is unusable, rather than throwing", () => { + expect(RipgrepRecords.parseRecords(["{oops", "{also oops"])).toEqual([]) + }) +}) +// altimate_change end