Skip to content

Commit 4740e25

Browse files
authored
🤖 fix: line-buffered filtering for bash_output filter_exclude (#1101)
When using `filter_exclude` with `bash_output`, the filter was incorrectly matching line fragments instead of complete lines. This caused: 1. Partial line fragments (like `)' from '`⏳ Still waiting... (1 pending)`) to not match the filter pattern and trigger early return 2. Agents polling repeatedly because each call returned tiny fragments ## Root Cause From the `runtime-16ps` chat history analysis: - The script outputs `⏳ Still waiting... (X pending, Y failed)` every 15 seconds - Each `bash_output` call reads incrementally from where the last call stopped - Previous calls consumed most of the line, leaving only `)\n` - The filter `"Still waiting"` couldn't match the fragment `")"` - So `)\n` passed through as "meaningful output" and triggered early return ## Changes - Track `incompleteLineBuffer` per process to buffer partial lines across calls - Only apply filter to complete lines (ending with newline) - On process exit, flush any remaining buffered content - Add 'better pattern' note when `filter_exclude` is used but still polling frequently ## Tests Added 3 new tests for line-buffered filtering: - `should only filter complete lines, not fragments` - `should buffer incomplete lines across calls` - `should include incomplete line on process exit` Updated existing test to verify new polling detection for filter_exclude users. --- _Generated with `mux`_
1 parent 7c74e23 commit 4740e25

File tree

2 files changed

+141
-20
lines changed

2 files changed

+141
-20
lines changed

src/node/services/backgroundProcessManager.test.ts

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,75 @@ describe("BackgroundProcessManager", () => {
871871
});
872872
});
873873

874+
describe("line-buffered filtering", () => {
875+
it("should only filter complete lines, not fragments", async () => {
876+
// Process that outputs lines that should be filtered and one that shouldn't
877+
const result = await manager.spawn(
878+
runtime,
879+
testWorkspaceId,
880+
// Output lines: some with 'progress', one without
881+
"echo 'progress 1'; echo 'progress 2'; echo 'FINAL RESULT'",
882+
{ cwd: process.cwd(), displayName: "test" }
883+
);
884+
885+
expect(result.success).toBe(true);
886+
if (!result.success) return;
887+
888+
// Wait for process to complete
889+
await new Promise((resolve) => setTimeout(resolve, 300));
890+
891+
// Filter out lines containing 'progress', should only get 'FINAL RESULT'
892+
const output = await manager.getOutput(result.processId, "progress", true, 0.5);
893+
expect(output.success).toBe(true);
894+
if (!output.success) return;
895+
expect(output.output).toContain("FINAL RESULT");
896+
expect(output.output).not.toContain("progress");
897+
});
898+
899+
it("should buffer incomplete lines across calls", async () => {
900+
// Process that outputs progress lines
901+
const result = await manager.spawn(
902+
runtime,
903+
testWorkspaceId,
904+
"echo 'progress: 50%'; sleep 0.1; echo 'progress: 100%'; echo 'DONE'",
905+
{ cwd: process.cwd(), displayName: "test" }
906+
);
907+
908+
expect(result.success).toBe(true);
909+
if (!result.success) return;
910+
911+
// Wait for process to complete
912+
await new Promise((resolve) => setTimeout(resolve, 500));
913+
914+
// Filter out progress lines, should only get 'DONE'
915+
const output = await manager.getOutput(result.processId, "progress", true, 0.5);
916+
expect(output.success).toBe(true);
917+
if (!output.success) return;
918+
expect(output.output).toContain("DONE");
919+
expect(output.output).not.toContain("progress");
920+
});
921+
922+
it("should include incomplete line on process exit", async () => {
923+
// Process that exits without final newline
924+
const result = await manager.spawn(runtime, testWorkspaceId, "printf 'no newline at end'", {
925+
cwd: process.cwd(),
926+
displayName: "test",
927+
});
928+
929+
expect(result.success).toBe(true);
930+
if (!result.success) return;
931+
932+
// Wait for process to exit
933+
await new Promise((resolve) => setTimeout(resolve, 300));
934+
935+
const output = await manager.getOutput(result.processId, undefined, undefined, 0.5);
936+
expect(output.success).toBe(true);
937+
if (!output.success) return;
938+
expect(output.output).toContain("no newline at end");
939+
expect(output.status).not.toBe("running");
940+
});
941+
});
942+
874943
describe("polling detection", () => {
875944
it("should return note after 3+ calls without filter_exclude on running process", async () => {
876945
// Long-running process
@@ -903,7 +972,7 @@ describe("BackgroundProcessManager", () => {
903972
expect(output3.note).toContain("3+ times");
904973
});
905974

906-
it("should NOT return note when filter_exclude is already used", async () => {
975+
it("should return better pattern note when filter_exclude is used but still polling", async () => {
907976
const result = await manager.spawn(
908977
runtime,
909978
testWorkspaceId,
@@ -914,14 +983,17 @@ describe("BackgroundProcessManager", () => {
914983
expect(result.success).toBe(true);
915984
if (!result.success) return;
916985

917-
// Make 3+ calls with filter_exclude
986+
// Make 3+ calls with filter_exclude - should get "better pattern" note
987+
let lastNote: string | undefined;
918988
for (let i = 0; i < 4; i++) {
919989
const output = await manager.getOutput(result.processId, "nomatch", true, 0.1);
920990
expect(output.success).toBe(true);
921991
if (!output.success) return;
922-
// Should never get the note since we're using filter_exclude
923-
expect(output.note).toBeUndefined();
992+
lastNote = output.note;
924993
}
994+
// Should get the "better pattern" note since we're using filter_exclude but still polling
995+
expect(lastNote).toContain("filter_exclude but still polling");
996+
expect(lastNote).toContain("broader pattern");
925997
});
926998

927999
it("should NOT return note when process has exited", async () => {

src/node/services/backgroundProcessManager.ts

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export interface BackgroundProcess {
4646
outputLock: AsyncMutex;
4747
/** Tracks how many times getOutput() has been called (for polling detection) */
4848
getOutputCallCount: number;
49+
/** Buffer for incomplete lines (no trailing newline) from previous read */
50+
incompleteLineBuffer: string;
4951
}
5052

5153
/**
@@ -222,6 +224,7 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
222224
outputBytesRead: 0,
223225
outputLock: new AsyncMutex(),
224226
getOutputCallCount: 0,
227+
incompleteLineBuffer: "",
225228
};
226229

227230
// Store process in map
@@ -331,6 +334,7 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
331334
outputBytesRead: 0,
332335
outputLock: new AsyncMutex(),
333336
getOutputCallCount: 0,
337+
incompleteLineBuffer: "",
334338
};
335339

336340
// Store process in map
@@ -504,10 +508,10 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
504508
}
505509
}
506510

507-
// Apply filtering to output, returns filtered result
508-
const applyFilter = (raw: string): string => {
509-
if (!filterRegex) return raw;
510-
const lines = raw.split("\n");
511+
// Apply filtering to complete lines only
512+
// Incomplete line fragments (no trailing newline) are kept in buffer for next read
513+
const applyFilter = (lines: string[]): string => {
514+
if (!filterRegex) return lines.join("\n");
511515
const filtered = filterExclude
512516
? lines.filter((line) => !filterRegex.test(line))
513517
: lines.filter((line) => filterRegex.test(line));
@@ -521,6 +525,9 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
521525
let accumulatedRaw = "";
522526
let currentStatus = proc.status;
523527

528+
// Track the previous buffer to prepend to accumulated output
529+
const previousBuffer = proc.incompleteLineBuffer;
530+
524531
while (true) {
525532
// Read new content via the handle (works for both local and SSH runtimes)
526533
// Output is already unified in output.log (stdout + stderr via 2>&1)
@@ -534,16 +541,25 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
534541
const refreshedProc = await this.getProcess(processId);
535542
currentStatus = refreshedProc?.status ?? proc.status;
536543

544+
// Line-buffered filtering: prepend incomplete line from previous call
545+
const rawWithBuffer = previousBuffer + accumulatedRaw;
546+
const allLines = rawWithBuffer.split("\n");
547+
548+
// Last element is incomplete if content doesn't end with newline
549+
const hasTrailingNewline = rawWithBuffer.endsWith("\n");
550+
const completeLines = hasTrailingNewline ? allLines.slice(0, -1) : allLines.slice(0, -1);
551+
const incompleteLine = hasTrailingNewline ? "" : allLines[allLines.length - 1];
552+
537553
// When using filter_exclude, check if we have meaningful (non-excluded) output
538-
// If all new output matches the exclusion pattern, keep waiting
539-
const filteredOutput = applyFilter(accumulatedRaw);
554+
// Only consider complete lines for filtering - fragments can't match patterns
555+
const filteredOutput = applyFilter(completeLines);
540556
const hasMeaningfulOutput = filterExclude
541557
? filteredOutput.trim().length > 0
542-
: accumulatedRaw.length > 0;
558+
: completeLines.length > 0 || incompleteLine.length > 0;
543559

544560
// Return immediately if:
545561
// 1. We have meaningful output (after filtering if filter_exclude is set)
546-
// 2. Process is no longer running (exited/killed/failed)
562+
// 2. Process is no longer running (exited/killed/failed) - flush buffer
547563
// 3. Timeout elapsed
548564
// 4. Abort signal received (user sent a new message)
549565
if (hasMeaningfulOutput || currentStatus !== "running") {
@@ -569,14 +585,52 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
569585
await new Promise((resolve) => setTimeout(resolve, pollIntervalMs));
570586
}
571587

572-
log.debug(`BackgroundProcessManager.getOutput: read rawLen=${accumulatedRaw.length}`);
588+
// Final line processing with buffer from previous call
589+
const rawWithBuffer = previousBuffer + accumulatedRaw;
590+
const allLines = rawWithBuffer.split("\n");
591+
const hasTrailingNewline = rawWithBuffer.endsWith("\n");
592+
593+
// On process exit, include incomplete line; otherwise keep it buffered
594+
const linesToReturn =
595+
currentStatus !== "running"
596+
? allLines.filter((l) => l.length > 0) // Include all non-empty lines on exit
597+
: hasTrailingNewline
598+
? allLines.slice(0, -1)
599+
: allLines.slice(0, -1);
600+
601+
// Update buffer for next call (clear on exit, keep incomplete line otherwise)
602+
proc.incompleteLineBuffer =
603+
currentStatus === "running" && !hasTrailingNewline ? allLines[allLines.length - 1] : "";
573604

574-
const filteredOutput = applyFilter(accumulatedRaw);
605+
log.debug(
606+
`BackgroundProcessManager.getOutput: read rawLen=${accumulatedRaw.length}, completeLines=${linesToReturn.length}`
607+
);
608+
609+
const filteredOutput = applyFilter(linesToReturn);
575610

576611
// Suggest filter_exclude if polling too frequently on a running process
577612
const shouldSuggestFilterExclude =
578613
callCount >= 3 && !filterExclude && currentStatus === "running";
579614

615+
// Suggest better pattern if using filter_exclude but still polling frequently
616+
const shouldSuggestBetterPattern =
617+
callCount >= 3 && filterExclude && currentStatus === "running";
618+
619+
let note: string | undefined;
620+
if (shouldSuggestFilterExclude) {
621+
note =
622+
"STOP POLLING. You've called bash_output 3+ times on this process. " +
623+
"This wastes tokens and clutters the conversation. " +
624+
"Instead, make ONE call with: filter='⏳|progress|waiting|\\\\\\.\\\\\\.\\\\\\.', " +
625+
"filter_exclude=true, timeout_secs=120. This blocks until meaningful output arrives.";
626+
} else if (shouldSuggestBetterPattern) {
627+
note =
628+
"You're using filter_exclude but still polling frequently. " +
629+
"Your filter pattern may not be matching the actual output. " +
630+
"Try a broader pattern like: filter='\\\\.|\\\\d+%|running|progress|pending|⏳|waiting'. " +
631+
"Wait for the FULL timeout before checking again.";
632+
}
633+
580634
return {
581635
success: true,
582636
status: currentStatus,
@@ -586,12 +640,7 @@ export class BackgroundProcessManager extends EventEmitter<BackgroundProcessMana
586640
? ((await this.getProcess(processId))?.exitCode ?? undefined)
587641
: undefined,
588642
elapsed_ms: Date.now() - startTime,
589-
note: shouldSuggestFilterExclude
590-
? "STOP POLLING. You've called bash_output 3+ times on this process. " +
591-
"This wastes tokens and clutters the conversation. " +
592-
"Instead, make ONE call with: filter='⏳|progress|waiting|\\\\.\\\\.\\\\.', " +
593-
"filter_exclude=true, timeout_secs=120. This blocks until meaningful output arrives."
594-
: undefined,
643+
note,
595644
};
596645
}
597646

0 commit comments

Comments
 (0)