diff --git a/README.md b/README.md index 57c0b8f..4492345 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ The host also decides several details, all handled internally: | `bgrun` | Launch a command detached in the background. Optional `name` gives the job a short human-readable label. Returns `started: ` immediately. Wakes the session automatically on completion. | | `bgstatus` | Show job status. With an id: any job's state + exit code — a `bg_N` id (the host's own background job on omp) is answered with its state, where its output went and how to cancel it, rather than "not found". Without: this session's running jobs (finished jobs hidden by default — pass `includeDone: true` or set `showCompletedJobs`), then the host's background jobs under their own heading. Other sessions' *running* jobs are listed only when `adoptForeignJobs` is enabled; finished foreign logs from the shared dir can also appear when finished jobs are included. | | `bgtail` | Read the newest lines of a job's log (default 40; it reads the log's **last 2 MB** — widen with `bytes`, max 64 MiB + 4 KiB of wrapper overhead), **condensed for context**: ANSI escapes stripped, repeated lines collapsed (a run of 3+ folds into one line carrying its `[xN]` count; a pair is kept as two lines — a fold needs its count to stay legible), long lines and total size capped. First read = full last-N tail; repeat reads return **only lines appended since your last read** (delta tailing) — polling a running job never re-pays for lines already seen. A repeat read asking for **more** lines than you have already been shown is a widening, not a poll: it returns the lines above your current coverage rather than the whole window again — coverage is a trailing run, so a line seen before an intervening gap can repeat. Pass `raw: true` for the unprocessed last-N window (still advances the bookmark). On omp, a native `bg_N` job's spilled output is read the same way, stamped as the host's file. | -| `bggrep` | Regex search over the **last 2 MB** of a job's log (`bytes` widens the window, max 64 MiB + 4 KiB of wrapper overhead; on omp, a native `bg_N` job's spilled output is searchable the same way): line-numbered matches, optional `context` lines, each line pre-truncated to 10 000 chars before matching, results capped (~50 matches, ~8KB) and condensed. Resolves the job id to the configured jobs dir itself — no log path to reconstruct. `ctx_execute_file` can read the same file (it takes an absolute path; only your Read-deny rules apply), but it needs that path. Matching runs under a wall-clock budget ([Bounded matching](#bounded-matching)). With no `pattern`, a generic failure-signature default is used (override it — convenience, not guarantee). | +| `bggrep` | Regex search over the **last 2 MB** of a job's log (`bytes` widens the window, max 64 MiB + 4 KiB of wrapper overhead; on omp, a native `bg_N` job's spilled output is searchable the same way): line-numbered matches, optional `context` lines, each line pre-truncated to 10 000 chars before matching, results capped (~50 matches, ~8KB) and condensed. JavaScript regex syntax: a **leading** `(?i)`/`(?m)`/`(?s)` group is accepted and translated to the equivalent flags (`(?i)error` searches case-insensitively); a *scoped* group (`(?i:error)`) is native anywhere; a bare flag group anywhere else is rejected with an explanation — JS `RegExp` has no unscoped inline flags. Resolves the job id to the configured jobs dir itself — no log path to reconstruct. `ctx_execute_file` can read the same file (it takes an absolute path; only your Read-deny rules apply), but it needs that path. Matching runs under a wall-clock budget ([Bounded matching](#bounded-matching)). With no `pattern`, a generic failure-signature default is used (override it — convenience, not guarantee). | | `bgclean` | Remove old job logs. **Default scope: this session's jobs only** — other sessions' logs are untouched — and it also drops stale per-project digest markers (`.bgrun-used-*`, `.digest-nudge-*`) in the session's jobs dir (markers are not session data). Pass `all: true` to sweep every shared jobs dir — under the project-local default that is the project's dir plus the machine-global one, while an explicit absolute `jobsDir` is swept alone — and do the same marker sweep across them. Retention: `cleanupDays` config (7 days); `days` must be a positive number (`days: 0` is rejected rather than purging everything). Never removes a running job's log. | | `bgkill` | Stop a running job. Optional `force: true` sends `SIGKILL` instead of `SIGTERM`. Signals the job's **whole process group** (the child is spawned detached, so the command and anything it started go together; where a platform cannot signal groups, it falls back to the single pid and says so). Refuses an id that already finished, one that is not a bgrun job (`bg_N` belongs to the host — `hub cancel`), another session's job unless `includeForeign: true`, and a pid that cannot be the job's — gone, unusable, or recycled onto an unrelated process. It reports the signal it sent, not a guess at the outcome: the authoritative result is the wake, or the stale-check for a foreign job. | diff --git a/extension/index.test.ts b/extension/index.test.ts index db862c8..ffd0d47 100644 --- a/extension/index.test.ts +++ b/extension/index.test.ts @@ -3261,6 +3261,118 @@ test("bggrep: invalid pattern errors clearly", async () => { }); }); +test("bggrep: a leading (?i) group searches case-insensitively", async () => { + await withJobsDir(async (_dir, h) => { + const { wakes, tools, ctx } = h; + const bgrun = tools.get("bgrun")!; + const bggrep = tools.get("bggrep")!; + const res = await bgrun.execute( + "c1", + { command: "printf 'DIAGNOSTIC_MARKER_UPSTREAM\\nplain line\\n'" }, + undefined, + undefined, + ctx, + ); + const id = ((res.content[0].text as string).match(/^started: ([^\n]+)/) || + [])[1]; + assert.ok(id, "got a job id"); + await waitForWakes(wakes, 1); + + // Lowercase pattern against an uppercase line: only the flag can match it. + const flagged = await bggrep.execute( + "c2", + { id, pattern: "(?i)diagnostic_marker" }, + undefined, + undefined, + ctx, + ); + assert.equal(flagged.details.matches, 1); + assert.match( + flagged.content[0].text as string, + /DIAGNOSTIC_MARKER_UPSTREAM/, + ); + // The header AND details quote the pattern as the caller wrote it, flags + // included — the caller must be able to see what was searched for. + assert.match( + flagged.content[0].text as string, + /1 match for \/\(\?i\)diagnostic_marker\//, + ); + assert.equal(flagged.details.pattern, "(?i)diagnostic_marker"); + + // Without the flag the same pattern matches nothing — so it is the + // translation doing the work, not something else. + const plain = await bggrep.execute( + "c3", + { id, pattern: "diagnostic_marker" }, + undefined, + undefined, + ctx, + ); + assert.equal(plain.details.matches, 0); + }); +}); + +test("bggrep: an inline flag group that is not leading is explained, not swallowed", async () => { + await withJobsDir(async (_dir, h) => { + const { wakes, tools, ctx } = h; + const bgrun = tools.get("bgrun")!; + const bggrep = tools.get("bggrep")!; + const res = await bgrun.execute( + "c1", + { command: "echo hit" }, + undefined, + undefined, + ctx, + ); + const id = ((res.content[0].text as string).match(/^started: ([^\n]+)/) || + [])[1]; + await waitForWakes(wakes, 1); + // PCRE scopes this to the rest of the pattern, so stripping it would silently + // widen the match — it must fail, and say why. + await assert.rejects( + bggrep.execute( + "c2", + { id, pattern: "hit(?i)more" }, + undefined, + undefined, + ctx, + ), + /unscoped inline flags/, + ); + }); +}); + +test("splitInlineFlags: only a leading group translates; the hint covers the rest", async () => { + // loadModule() is typed by the helper; the exports under test are declared in + // the extension module, so no cast is needed here. + const mod = await loadModule(); + assert.deepEqual(mod.splitInlineFlags("(?i)error"), { + source: "error", + flags: "i", + }); + assert.deepEqual(mod.splitInlineFlags("(?mi)x"), { source: "x", flags: "im" }); + assert.deepEqual(mod.splitInlineFlags("(?is)a(?m)b"), { + source: "a(?m)b", + flags: "is", + }); + assert.deepEqual(mod.splitInlineFlags("plain"), { source: "plain", flags: "" }); + // Not leading → left untouched for the engine to reject, with the reason. + assert.deepEqual(mod.splitInlineFlags("a(?i)b"), { + source: "a(?i)b", + flags: "", + }); + assert.equal(mod.inlineFlagHint("plain"), ""); + assert.match(mod.inlineFlagHint("a(?i)b"), /unscoped inline flags/); + assert.match(mod.inlineFlagHint("x(?s:y)"), /unscoped inline flags/); + // A leading group we honored is not the reason for a later compile failure. + assert.equal(mod.inlineFlagHint("(?i)[unclosed"), ""); + // A SECOND group further inside IS the reason, even when the pattern starts with + // one. Keying this check on the raw pattern suppressed the hint here — which is + // the only case the hint exists for. + assert.match(mod.inlineFlagHint("(?is)a(?m)b"), /unscoped inline flags/); + assert.match(mod.inlineFlagHint("(?i)(?i)b"), /unscoped inline flags/); +}); + test("bggrep: caps at 50 matches with a not-shown note", async () => { await withJobsDir(async (_dir, h) => { const { wakes, tools, ctx } = h; @@ -7203,7 +7315,7 @@ test("bggrep: the budget terminates a worker that is stuck mid-match (abort path ["a".repeat(50)], 10, 300, - stall, + { workerSource: stall }, ); const elapsed = Date.now() - t0; assert.equal(outcome.kind, "timeout", "a stuck worker is reported as a timeout"); @@ -7243,6 +7355,30 @@ test("bggrep sync fallback: same results as the worker path, including the line (await mod.matchLinesWithBudget("(", lines, 10, 2_000)).kind, "invalid", ); + + // The flags argument must reach BOTH paths. Only the worker is reachable from the + // tool in this suite (worker_threads exists on Node and Bun), so a regression that + // dropped the argument at either fallback call site — the exact bug class this + // feature fixed — would otherwise ship silently. + const upper = ["DIAGNOSTIC_MARKER"]; + assert.deepEqual( + mod.matchLinesSyncBounded("diagnostic_marker", upper, 100, 1_000, "i"), + { kind: "ok", matchIdx: [0] }, + "the sync fallback honours flags", + ); + assert.deepEqual( + await mod.matchLinesWithBudget("diagnostic_marker", upper, 100, 2_000, { + flags: "i", + }), + { kind: "ok", matchIdx: [0] }, + "the worker honours flags", + ); + // Without the flag the same pattern misses on both paths, so the assertions above + // are testing the flag rather than a pattern that would hit anyway. + assert.deepEqual( + mod.matchLinesSyncBounded("diagnostic_marker", upper, 100, 1_000), + { kind: "ok", matchIdx: [] }, + ); }); test("bggrep sync fallback: a spent budget stops the scan before the first line", async () => { diff --git a/extension/index.ts b/extension/index.ts index bc2ca55..7b819e9 100644 --- a/extension/index.ts +++ b/extension/index.ts @@ -52,6 +52,10 @@ import { } from "node:fs"; import { dirname, isAbsolute, join, relative, sep } from "node:path"; import { homedir } from "node:os"; +// Type-only, so it is erased at compile time: the runtime import below stays +// dynamic because this fallback exists for runtimes WITHOUT worker_threads, and a +// static import would fail at load there. +import type { Worker as NodeWorker } from "node:worker_threads"; import { createHash, randomBytes } from "node:crypto"; import { DIGEST_PRESET_IDS, @@ -396,7 +400,7 @@ const BGGREP_DEFAULT_TIMEOUT_MS = 2_000; const BGGREP_WORKER_SOURCE = ` const { parentPort, workerData } = require("node:worker_threads"); try { - const re = new RegExp(workerData.source); + const re = new RegExp(workerData.source, workerData.flags || ""); const lines = workerData.lines; const cap = workerData.cap; const out = []; @@ -488,6 +492,50 @@ type GrepMatchOutcome = | { kind: "timeout" } | { kind: "invalid"; message: string }; +// Callers reach for PCRE habits, and the one that keeps costing tool calls is the +// inline case-insensitive flag: `(?i)error` is valid PCRE and a syntax error in +// JavaScript, whose RegExp takes flags as a second argument and has no inline +// form at all. Accept the start-of-pattern group a PCRE user writes and translate +// it into the equivalent JS flags, so `(?i)`, `(?im)` and `(?is)` read and behave +// as expected. +// +// Only the LEADING position is translated: PCRE scopes a mid-pattern `(?i)` to +// the REST of the pattern, so stripping one there would silently widen the match +// instead of honoring it. Those get the explanation in inlineFlagHint() instead. +const GREP_LEADING_FLAGS_RE = /^\(\?([ims]+)\)/; +// Anything shaped like an inline flag group — used only to explain a failure the +// engine already reported. +const GREP_ANY_FLAGS_RE = /\(\?[a-zA-Z-]+[):]/; + +export function splitInlineFlags(raw: string): { + source: string; + flags: string; +} { + const m = raw.match(GREP_LEADING_FLAGS_RE); + if (!m) return { source: raw, flags: "" }; + // Dedupe and order them so the flags string is deterministic. + const flags = [...m[1]] + .filter((c, i, all) => all.indexOf(c) === i) + .sort() + .join(""); + return { source: raw.slice(m[0].length), flags }; +} + +// Explain a compile failure in the caller's terms when the pattern used PCRE flag +// syntax. A leading group we already translated is not the problem — there the +// engine's own message is the honest answer — so the hint is reserved for the +// forms this tool cannot honor. +export function inlineFlagHint(raw: string): string { + // Keyed on the pattern AFTER a leading group is stripped. A leading group that + // was honoured is not the reason for the failure — but a SECOND group further + // into the pattern is exactly the case this hint exists for, and suppressing it + // there (the raw pattern does start with a group) told the caller nothing. + if (!GREP_ANY_FLAGS_RE.test(splitInlineFlags(raw).source)) return ""; + return ( + " — JavaScript regexes have no unscoped inline flags: a bare (?i)/(?m)/(?s) group is only accepted at the very start of the pattern, and this one is not first. Put the flags first, write the alternatives out (`fail|FAIL`), or scope it — `(?i:error)` is understood natively" + ); +} + // Bounded between lines only — a single pathological line can still stall. // Used solely when worker_threads is unavailable (never on Node or Bun). export function matchLinesSyncBounded( @@ -495,10 +543,11 @@ export function matchLinesSyncBounded( lines: string[], cap: number, budgetMs: number, + flags: string = "", ): GrepMatchOutcome { let re: RegExp; try { - re = new RegExp(source); + re = new RegExp(source, flags); } catch (err) { return { kind: "invalid", message: (err as Error).message }; } @@ -514,32 +563,36 @@ export function matchLinesSyncBounded( return { kind: "ok", matchIdx: out }; } -// Exported, and workerSource-injectable, so a test can prove the ABORT path on -// any engine: pass a worker body that never returns and the budget must still -// yield `{kind: "timeout"}`. Input-driven catastrophic patterns cannot test it -// — engines differ (V8 backtracks exponentially where JSC does not), so the +// Exported, and workerSource-injectable via opts, so a test can prove the ABORT +// path on any engine: pass a worker body that never returns and the budget must +// still yield `{kind: "timeout"}`. Input-driven catastrophic patterns cannot test +// it — engines differ (V8 backtracks exponentially where JSC does not), so the // only portable assertion is that termination works. export async function matchLinesWithBudget( source: string, lines: string[], cap: number, budgetMs: number, - workerSource: string = BGGREP_WORKER_SOURCE, + opts: { workerSource?: string; flags?: string } = {}, ): Promise { - let WorkerCtor: typeof import("node:worker_threads").Worker; + const workerSource = opts.workerSource ?? BGGREP_WORKER_SOURCE; + const flags = opts.flags ?? ""; + let WorkerCtor: typeof NodeWorker; try { + // Runtime-selected on purpose: absent on the platforms the fallback exists + // for, so a static import cannot work here. ({ Worker: WorkerCtor } = await import("node:worker_threads")); } catch { - return matchLinesSyncBounded(source, lines, cap, budgetMs); + return matchLinesSyncBounded(source, lines, cap, budgetMs, flags); } - let worker: import("node:worker_threads").Worker; + let worker: NodeWorker; try { worker = new WorkerCtor(workerSource, { eval: true, - workerData: { source, lines, cap }, + workerData: { source, lines, cap, flags }, }); } catch { - return matchLinesSyncBounded(source, lines, cap, budgetMs); + return matchLinesSyncBounded(source, lines, cap, budgetMs, flags); } return new Promise((resolve) => { let settled = false; @@ -3984,13 +4037,16 @@ export default function (pi: ExtensionAPI) { const context = Math.max(0, Math.floor(contextParam)); if (!id) throw new Error("bggrep: id is required"); // resolveLogForJob() below validates the id; no need to double-check. - const source = pattern ?? DEFAULT_GREP_PATTERN; + const raw = pattern ?? DEFAULT_GREP_PATTERN; + // A leading PCRE-style flag group is translated, not rejected: JS RegExp has + // no inline flag syntax, and failing on `(?i)` cost a real tool call. + const { source, flags } = splitInlineFlags(raw); try { // Validate up front so a bad pattern fails immediately, without a worker. - void new RegExp(source); + void new RegExp(source, flags); } catch (err) { throw new Error( - `bggrep: invalid pattern ${JSON.stringify(source)}: ${(err as Error).message}`, + `bggrep: invalid pattern ${JSON.stringify(raw)}: ${(err as Error).message}${inlineFlagHint(raw)}`, ); } // Record-first, same as bgtail — correct across config changes. @@ -4050,10 +4106,11 @@ export default function (pi: ExtensionAPI) { rawLines, BGGREP_LINE_CAP, budgetMs, + { flags }, ); if (outcome.kind === "invalid") { throw new Error( - `bggrep: invalid pattern ${JSON.stringify(source)}: ${outcome.message}`, + `bggrep: invalid pattern ${JSON.stringify(raw)}: ${outcome.message}${inlineFlagHint(raw)}`, ); } if (outcome.kind === "timeout") { @@ -4062,7 +4119,7 @@ export default function (pi: ExtensionAPI) { { type: "text", text: - `bggrep: /${source}/ exceeded the ${budgetMs}ms match budget across ` + + `bggrep: /${raw}/ exceeded the ${budgetMs}ms match budget across ` + `${rawLines.length} line${rawLines.length === 1 ? "" : "s"} — likely ` + `catastrophic backtracking; no results computed.`, }, @@ -4073,7 +4130,7 @@ export default function (pi: ExtensionAPI) { linesSearched: rawLines.length, logPath, notFound: false, - pattern: source, + pattern: raw, timedOut: true, windowBytes: readWindow, ...truncDetails, @@ -4083,7 +4140,7 @@ export default function (pi: ExtensionAPI) { } const matchIdx = outcome.matchIdx; const header = - `${matchIdx.length} match${matchIdx.length === 1 ? "" : "es"} for /${source}/ ` + + `${matchIdx.length} match${matchIdx.length === 1 ? "" : "es"} for /${raw}/ ` + `in ${rawLines.length} line${rawLines.length === 1 ? "" : "s"}`; if (matchIdx.length === 0) { return { @@ -4139,7 +4196,7 @@ export default function (pi: ExtensionAPI) { linesSearched: rawLines.length, logPath, notFound: false, - pattern: source, + pattern: raw, capped, windowBytes: readWindow, ...truncDetails, @@ -4158,7 +4215,7 @@ export default function (pi: ExtensionAPI) { name: "bggrep", label: "Grep Background Log", description: toolDescription( - "Search the tail of a background job's log with a regex — the last 2 MiB by default, widen with `bytes` (each line is pre-truncated to 10k chars before matching); returns only matching lines with line numbers (optional context lines), capped (~50 matches, ~8KB) and condensed. Matching runs under a wall-clock budget (default 2s, PI_BGRUN_GREP_TIMEOUT_MS), so a runaway regex fails instead of hanging. Resolves the job id to the configured jobs dir itself, so there is no log path to reconstruct; reading that path directly is the whole-log escape hatch. Pass your own pattern whenever you know the log's format; with no pattern a generic failure-signature default is used (a convenience only — not a guarantee).", + "Search the tail of a background job's log with a regex — the last 2 MiB by default, widen with `bytes` (each line is pre-truncated to 10k chars before matching); returns only matching lines with line numbers (optional context lines), capped (~50 matches, ~8KB) and condensed. JavaScript regex syntax: a LEADING `(?i)`, `(?m)` or `(?s)` group is accepted and translated to the equivalent flags — `(?i)error` searches case-insensitively. A *scoped* group (`(?i:error)`) is understood natively, anywhere. A bare flag group anywhere else is rejected with an explanation, since JS has no unscoped inline flags. Matching runs under a wall-clock budget (default 2s, PI_BGRUN_GREP_TIMEOUT_MS), so a runaway regex fails instead of hanging. Resolves the job id to the configured jobs dir itself, so there is no log path to reconstruct; reading that path directly is the whole-log escape hatch. Pass your own pattern whenever you know the log's format; with no pattern a generic failure-signature default is used (a convenience only — not a guarantee).", BGGREP_GUIDELINES, ), promptSnippet: "Search a bgrun job's log for a pattern", @@ -4170,7 +4227,7 @@ export default function (pi: ExtensionAPI) { pattern: Type.Optional( Type.String({ description: - "Regex to search for. Default: generic failure signatures — override when you know the format.", + "Regex to search for (JavaScript syntax; a LEADING (?i), (?m) or (?s) group is accepted, e.g. (?i)error for case-insensitive). Default: generic failure signatures — override when you know the format.", }), ), context: Type.Optional( diff --git a/skills/run-bg/SKILL.md b/skills/run-bg/SKILL.md index dc2bb16..6d7fb87 100644 --- a/skills/run-bg/SKILL.md +++ b/skills/run-bg/SKILL.md @@ -85,7 +85,7 @@ Which to use: | Start | `bgrun(command: "make test-short", name: "unit-tests", type: "test")` → `started: ` (name is an optional short label; use it so jobs are recognizable in `bgstatus`, the live panel, and wake messages) | | Status | `bgstatus()` for one job, or `bgstatus()` for this session's running jobs — finished jobs are hidden by default; pass `includeDone: true` to list them | | Tail | `bgtail(, 40)` — first read: last-40 tail; later reads: only lines appended since (delta tailing); a *bigger* `lines` than you have been shown is a widening and returns the lines above your coverage (a line seen before a gap can repeat) | -| Grep | `bggrep(, "pattern", context?)` — line-numbered matches, capped and condensed; default pattern = generic failure signatures (override when you know the format) | +| Grep | `bggrep(, "pattern", context?)` — line-numbered matches, capped and condensed; default pattern = generic failure signatures (override when you know the format). JavaScript regex: a leading `(?i)`/`(?m)`/`(?s)` group works (`(?i)error` = case-insensitive), a scoped `(?i:…)` group is native anywhere, and a bare flag group elsewhere is rejected | | Clean | `bgclean()` for this session's old logs; `bgclean all` to sweep every session's (default 7-day retention) | | Kill | `bgkill()` — SIGTERM to the job's process group; `force: true` for SIGKILL. Refuses finished ids, non-bgrun ids, and another session's job unless `includeForeign: true` |