Skip to content

Commit 14126c5

Browse files
committed
Bound ripgrep output, isolate perf spans, and stop resume dropping blocks
The grep byte cap never bounded anything: on a breach the handler settled with everything it had accumulated, 153 KB against a 200 byte limit in one measured run. The notice fired, so it read as enforced. Collection now owns the bytes behind three explicit settle points where exactly one wins, and truncates on a line boundary. The perf span test was not order-dependent. Every perf file cleared the store after itself and assumed it began empty, but the store is process-global and one process runs them all. Resumed sessions dropped view, plan and tasks blocks with no marker. The loose history-block shape discarded their payloads before anything could paint them. Replaying all 49 sessions on this machine recovers 6 blocks that previously vanished.
1 parent 72558da commit 14126c5

14 files changed

Lines changed: 653 additions & 109 deletions

docs/audit-fleet-prompt.md

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# Audit fleet prompt
2+
3+
Paste as the task for an orchestrating agent. It spawns a review panel over the
4+
codebase's own capabilities and reports back. Adjust the scope line before use.
5+
6+
---
7+
8+
You are orchestrating a standing audit of Corbits Code's own capabilities. The
9+
goal is not to ship a feature — it is to find out what is quietly wrong, what is
10+
about to break, and what has drifted from what we believe about it.
11+
12+
**Scope for this run:** <SCOPE — e.g. "everything under src/tui-opentui and
13+
src/permission" or "every capability the agent exposes as a tool">
14+
15+
## Ground rules for every agent you spawn
16+
17+
Put these in each agent's prompt verbatim. They are not boilerplate; each one
18+
was bought with real time.
19+
20+
**Read-only on git.** Do not run `git checkout`, `switch`, `stash`, `reset`,
21+
`clean`, `add`, `commit`, or `push`. Other agents share this working tree, and a
22+
`stash` has stranded work here before. Read history with `git log` / `git show
23+
<ref>:<path>`.
24+
25+
**Verify by running, not by reading.** A green suite is not evidence. On this
26+
codebase, every genuine defect of the last cycle was found by running the app or
27+
capturing the pty byte stream, and every false finding came from trusting a
28+
document. If a claim can be executed, execute it.
29+
30+
**Distinguish VERIFIED from SUSPECT, per finding.** A suspicion clearly labelled
31+
is useful. A suspicion stated as fact wastes a day and burns the reviewer's
32+
credibility for the findings that were real.
33+
34+
**Do not trust documents, including ours.** A readiness doc in this repo
35+
recently produced four false blocking findings and a formal do-not-ship verdict;
36+
all four had been fixed months earlier. If a doc and the code disagree, the code
37+
wins, and the doc is itself a finding.
38+
39+
**Assume tests may guard the wrong path.** We shipped a bug where the test drove
40+
`pushToolCall`/`pushToolResult` while the app ran `applyToolResult` — the test
41+
asserted correct behavior on code that never executes. When a test covers a
42+
finding you believe is real, check which code path it actually exercises before
43+
concluding the finding is wrong.
44+
45+
**Watch for silent no-match.** Bindings, dispatch tables, and event channels
46+
that fail by doing nothing are this codebase's most common defect shape. We have
47+
found: four emitted event channels with zero listeners, five help rows
48+
describing behavior that did not exist, and a keybinding that never matched.
49+
None of them errored. None were caught by tests.
50+
51+
## The panel — spawn these in parallel
52+
53+
Give each its own prompt. Do not let them duplicate scope.
54+
55+
**greybeard — architecture and the long term.** Is each abstraction sound, or
56+
does it encode today's decisions so tightly that the next change fights it?
57+
Where is ownership of a constraint split across layers, so an invariant is
58+
stated in one place and violated in another? What will hurt in six months?
59+
Explicitly ask it to separate "must fix now" from "will hurt later" — and to
60+
argue against large refactors close to a release, since a rewrite of the
61+
most-exercised file is how a good release becomes a bad week.
62+
63+
**critique — correctness and completeness.** Find defects; do not fix them. For
64+
each: file, line, what breaks, and the concrete input or sequence that triggers
65+
it. Point it at error paths, disposal, double-dispose, resize mid-overlay, a
66+
source throwing mid-render, and anything whose state is valid mid-stream but
67+
wrong on screen.
68+
69+
**neckbeard — hygiene, and refactor proposals.** This is the agent that files
70+
refactor issues. Let it be pedantic; in terminal and permission code the fiddly
71+
details *are* the product. Unicode width, escape sequences, off-by-ones, type
72+
escape hatches, boundary validation, naming and comment drift. **Explicitly
73+
authorize it to propose refactors as Linear issues** rather than only complain —
74+
one issue per proposal, with the seam it would cut along, what it buys, and what
75+
it risks. Require it to separate genuine defects from taste, and tell it not to
76+
suggest rewriting anything in Rust.
77+
78+
**gaasbot (CTO) — risk and sequencing.** Not a code review. Given what the others
79+
find, what actually blocks a release, what ships with a note, and what is filed?
80+
Ask it directly what we are most likely getting wrong that nobody raised. Tell
81+
it plainly that you would rather hear "do not ship" now than at minute fifty-five.
82+
83+
**bruckheimer — the person using it.** Not a code review either. Can a new user
84+
get through the first ninety seconds? Which affordances are discoverable and
85+
which exist only in a file nobody reads? What state is the user left in when
86+
something fails — do they know what to press? Read the copy actually shown on
87+
screen and name specific strings that should change and what they should say.
88+
89+
## What each agent must return
90+
91+
- Findings ranked: blocking, should-fix, file-for-later.
92+
- Each concrete enough for another agent to act on with no follow-up questions.
93+
- Evidence for anything claimed as verified — the command run, the bytes
94+
captured, the frame rendered.
95+
- An explicit statement of what it did **not** cover, so gaps are visible rather
96+
than assumed closed.
97+
98+
Tell them a short honest review beats a padded one, and that "this is genuinely
99+
fine" is a useful finding when it is true.
100+
101+
## Your job as orchestrator
102+
103+
1. **Do not relay findings unverified.** Check the load-bearing ones yourself
104+
before acting. A CTO-level verdict here rested on a stale doc; four of its
105+
five blocking claims fell apart under a five-minute grep.
106+
2. **Dedupe across agents** — the same defect will arrive under different names.
107+
3. **File what is not being fixed now**, with enough context that the next
108+
person does not re-derive it. An unfiled finding is a lost finding.
109+
4. **Route fixes to agents that own disjoint files**, and say who owns what. Two
110+
agents editing one file will silently overwrite each other; that has already
111+
cost a re-apply here.
112+
5. **Report honestly.** If the suite is red, say so with the number. If a fix is
113+
mitigation rather than a fix, say which. If you were wrong earlier, correct it
114+
in a sentence and move on.
115+
116+
## Known traps in this codebase
117+
118+
Include whichever apply to the scope:
119+
120+
- `bun test` runs `vendor/intx-inference`; CI runs `bun test ./src ./tests
121+
./evals`. Quote the scoped number or you will report failures that do not gate.
122+
- Roughly forty git worktrees exist; worktree-enumerating tests fail
123+
environmentally because of them.
124+
- The OpenTUI headless test renderer cannot see paint, real modifier reporting,
125+
the system clipboard, or terminal-owned selection. Whole defect classes are
126+
invisible to it by construction.
127+
- `Renderable.destroy()` frees only its own buffer and detaches children without
128+
destroying them. Anything dropping a subtree must destroy it recursively.

src/perf/assert-spans.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* - full observer pipeline → snapshot → rollup → assertions
1717
*/
1818

19-
import { afterEach, describe, expect, test } from "bun:test";
19+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
2020
import type { ReactorEmittedEvent } from "@intx/inference";
2121
import {
2222
assertLessThan,
@@ -33,6 +33,12 @@ import { ALLOWED_TAG_KEYS, clear, snapshot, type PerfSpan } from "./index.js";
3333
import { createPerfReactorObserver } from "./reactor-spans.js";
3434
import { rollupByPhase, rollupByTurn, type TurnSummary } from "./rollup.js";
3535

36+
// The span store is process-wide, so a perf test cannot assume the tests that
37+
// ran before it in this process left it empty. Reset on both edges.
38+
beforeEach(() => {
39+
clear();
40+
});
41+
3642
afterEach(() => {
3743
clear();
3844
});

src/perf/index.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, describe, expect, test } from "bun:test";
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import {
33
OPEN_SPAN_CAPACITY,
44
RING_CAPACITY,
@@ -11,6 +11,12 @@ import {
1111
type PerfSpan,
1212
} from "./index.js";
1313

14+
// The span store is process-wide, so a perf test cannot assume the tests that
15+
// ran before it in this process left it empty. Reset on both edges.
16+
beforeEach(() => {
17+
clear();
18+
});
19+
1420
afterEach(() => {
1521
clear();
1622
});

src/perf/otel-sink.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, describe, expect, test } from "bun:test";
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22

33
import type { Settings } from "../config/settings.js";
44
import { clear, end, snapshot, start, type PerfSpan } from "./index.js";
@@ -13,6 +13,12 @@ import {
1313
} from "./otel-sink.js";
1414
import type { EnabledOtelExportConfig } from "./otel-config.js";
1515

16+
// The span store is process-wide, so a perf test cannot assume the tests that
17+
// ran before it in this process left it empty. Reset on both edges.
18+
beforeEach(() => {
19+
clear();
20+
});
21+
1622
afterEach(() => {
1723
clear();
1824
});

src/perf/permission-subagent-spans.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
/**
22
* CL-5170: permission.wait and subagent spans at the ask gate and task fleet.
33
*/
4-
import { afterEach, describe, expect, test } from "bun:test";
4+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
55
import type { ReactorEmittedEvent } from "@intx/inference";
66
import { createPermissionGate } from "../permission/gate.js";
77
import { createTaskTool } from "../subagent/task-tool.js";
88
import { clear, snapshot, type PerfSpan } from "./index.js";
99
import { createPerfReactorObserver, currentTurnId } from "./reactor-spans.js";
1010

11+
// The span store is process-wide, so a perf test cannot assume the tests that
12+
// ran before it in this process left it empty. Reset on both edges.
13+
beforeEach(() => {
14+
clear();
15+
});
16+
1117
afterEach(() => {
1218
clear();
1319
});

src/perf/reactor-spans.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
import { afterEach, describe, expect, test } from "bun:test";
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import type { ReactorEmittedEvent } from "@intx/inference";
33
import { clear, snapshot, type PerfSpan } from "./index.js";
44
import { createPerfReactorObserver } from "./reactor-spans.js";
55
import { createTurnContextCollector } from "../session/hooks.js";
66

7+
// The span store is process-wide, so a perf test cannot assume the tests that
8+
// ran before it in this process left it empty. Reset on both edges.
9+
beforeEach(() => {
10+
clear();
11+
});
12+
713
afterEach(() => {
814
clear();
915
});

src/perf/rollup.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { afterEach, describe, expect, test } from "bun:test";
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import { mkdtemp, readFile, rm } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
@@ -26,6 +26,12 @@ import {
2626
spanDurationNs,
2727
} from "./rollup.js";
2828

29+
// The span store is process-wide, so a perf test cannot assume the tests that
30+
// ran before it in this process left it empty. Reset on both edges.
31+
beforeEach(() => {
32+
clear();
33+
});
34+
2935
afterEach(() => {
3036
clear();
3137
});

src/plugins/rg-output.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { test, expect } from "bun:test";
2+
3+
import { createRgCollector } from "./rg-output.js";
4+
5+
const line = "big.txt:1:match line here\n";
6+
7+
test("the cap fires on the chunk that breaches it", () => {
8+
const collector = createRgCollector(200);
9+
expect(collector.push(line.repeat(4))).toBeUndefined();
10+
expect(collector.push(line.repeat(20))).toMatchObject({
11+
kind: "partial",
12+
notice: expect.stringContaining("exceeded 200 bytes"),
13+
});
14+
});
15+
16+
test("an over-cap run reports no more than the cap, cut at a line boundary", () => {
17+
const outcome = createRgCollector(200).push(line.repeat(400));
18+
if (outcome?.kind !== "partial") throw new Error("expected partial");
19+
expect(outcome.stdout.length).toBeLessThanOrEqual(200);
20+
expect(outcome.stdout).toContain("match line here");
21+
expect(outcome.stdout.endsWith("here")).toBe(true);
22+
});
23+
24+
test("a cap breach outranks the exit code at every settle point", () => {
25+
const overCap = (): string[] => [line.repeat(20)];
26+
for (const settle of [
27+
(chunks: string[]) => {
28+
const c = createRgCollector(200);
29+
chunks.forEach((chunk) => c.push(chunk));
30+
return c.close(0, "");
31+
},
32+
(chunks: string[]) => {
33+
const c = createRgCollector(200);
34+
chunks.forEach((chunk) => c.push(chunk));
35+
return c.timeout(5);
36+
},
37+
]) {
38+
// The cap has already settled the run, so no later path can widen it.
39+
expect(settle(overCap())).toBeUndefined();
40+
}
41+
});
42+
43+
test("the timeout yields whatever was collected under the cap", () => {
44+
const collector = createRgCollector(2_000);
45+
collector.push(line);
46+
expect(collector.timeout(1)).toMatchObject({
47+
kind: "partial",
48+
stdout: line,
49+
notice: expect.stringContaining("timed out after 1ms"),
50+
});
51+
});
52+
53+
test("only the first settle wins", () => {
54+
const collector = createRgCollector(2_000);
55+
collector.push(line);
56+
expect(collector.close(0, "")).toMatchObject({ kind: "output", stdout: line });
57+
expect(collector.close(0, "")).toBeUndefined();
58+
expect(collector.timeout(1)).toBeUndefined();
59+
expect(collector.push(line.repeat(400))).toBeUndefined();
60+
});
61+
62+
test("exit codes map to no-match and error", () => {
63+
expect(createRgCollector(200).close(1, "")).toMatchObject({ kind: "no-match" });
64+
expect(createRgCollector(200).close(2, "bad pattern")).toMatchObject({
65+
kind: "error",
66+
message: "bad pattern",
67+
});
68+
expect(createRgCollector(200).close(2, "")).toMatchObject({
69+
kind: "error",
70+
message: "ripgrep exited with code 2",
71+
});
72+
});

src/plugins/rg-output.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Collects a ripgrep run's stdout and decides its outcome. `data`, `close` and
2+
// the timeout fire in a platform-dependent order, so the decision lives here
3+
// rather than in the handlers: the collector owns the accumulated bytes and
4+
// settles exactly once, whichever handler gets there first. The cap is applied
5+
// to those bytes as they arrive, so an over-cap run can never be reported as a
6+
// complete success and can never hand back more than the cap.
7+
8+
export type RgOutcome =
9+
| { kind: "output"; stdout: string }
10+
| { kind: "no-match" }
11+
| { kind: "error"; message: string }
12+
| { kind: "partial"; stdout: string; notice: string };
13+
14+
export type RgCollector = {
15+
/** Returns an outcome once the cap is breached, otherwise undefined. */
16+
push: (chunk: string) => RgOutcome | undefined;
17+
close: (code: number | null, stderr: string) => RgOutcome | undefined;
18+
timeout: (timeoutMs: number) => RgOutcome | undefined;
19+
};
20+
21+
// Cutting at the cap can land mid-line; drop the trailing fragment so callers
22+
// never see a half-formed match.
23+
function truncateToWholeLines(text: string, maxBytes: number): string {
24+
if (text.length <= maxBytes) return text;
25+
const clipped = text.slice(0, maxBytes);
26+
const lastBreak = clipped.lastIndexOf("\n");
27+
return lastBreak === -1 ? clipped : clipped.slice(0, lastBreak);
28+
}
29+
30+
export function createRgCollector(maxOutputBytes: number): RgCollector {
31+
let stdout = "";
32+
let settled = false;
33+
34+
const settle = (outcome: RgOutcome): RgOutcome | undefined => {
35+
if (settled) return undefined;
36+
settled = true;
37+
return outcome;
38+
};
39+
40+
const overCap = (): RgOutcome | undefined => {
41+
if (stdout.length <= maxOutputBytes) return undefined;
42+
return settle({
43+
kind: "partial",
44+
stdout: truncateToWholeLines(stdout, maxOutputBytes),
45+
notice: `ripgrep output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`,
46+
});
47+
};
48+
49+
return {
50+
push: (chunk) => {
51+
if (settled) return undefined;
52+
stdout += chunk;
53+
return overCap();
54+
},
55+
close: (code, stderr) => {
56+
if (settled) return undefined;
57+
if (code === 0) return settle({ kind: "output", stdout });
58+
if (code === 1) return settle({ kind: "no-match" });
59+
return settle({
60+
kind: "error",
61+
message: stderr.trim() || `ripgrep exited with code ${code}`,
62+
});
63+
},
64+
timeout: (timeoutMs) => {
65+
if (settled) return undefined;
66+
return settle({
67+
kind: "partial",
68+
stdout,
69+
notice: `ripgrep timed out after ${timeoutMs}ms — showing partial results; narrow path/glob`,
70+
});
71+
},
72+
};
73+
}

0 commit comments

Comments
 (0)