fix(coding-agent): suppressed artifact truncation notice when nothing was elided
Codex review on PR #2083 caught a corruption case in `OutputSink`: when a stream exceeds `artifactHeadBytes` but still fits below `artifactMaxBytes`, post-head bytes flow into the tail ring without any eviction (`droppedBytes === 0`) — yet `#flushArtifactTailIfCapped` unconditionally injected `[ARTIFACT TRUNCATED: kept first … + last … of …; 0 B elided from the middle]` between head and tail. The resulting artifact-on-disk is then no longer verbatim and falsely advertises truncation for outputs that actually fit. With the default 4 MiB / 3 MiB split, every ~3–4 MiB bash capture in this band tripped the bug. The notice is now gated on `droppedBytes > 0`. The tail ring is still flushed unconditionally so head + tail still equal the verbatim stream in this band. Regression pinned by a new test in `test/streaming-output.test.ts` that pushes 24 bytes into a 16-head / 16-tail cap and asserts the file equals the payload byte-for-byte with no `[ARTIFACT TRUNCATED:` marker. Refs #2081
This commit is contained in:
@@ -1082,10 +1082,18 @@ export class OutputSink {
|
||||
}
|
||||
|
||||
/**
|
||||
* Replay the rolling tail ring back into the artifact sink and inject a
|
||||
* single notice line so a reader of `artifact://<id>` sees
|
||||
* `<head>` + `[ARTIFACT TRUNCATED: …]` + `<tail>`. No-op when the cap was
|
||||
* never hit (head budget never exhausted, tail ring empty).
|
||||
* Replay the rolling tail ring back into the artifact sink. When bytes
|
||||
* were actually dropped from the middle (the head budget was exhausted
|
||||
* *and* the tail ring evicted), a single `[ARTIFACT TRUNCATED: …]`
|
||||
* notice is injected between head and tail so a reader of
|
||||
* `artifact://<id>` understands the gap. When the total stream simply
|
||||
* spilled past the head budget but still fits below `artifactMaxBytes`,
|
||||
* `droppedBytes` is zero — head + tail together are the verbatim stream
|
||||
* and the notice is suppressed so we don't corrupt the artifact with a
|
||||
* misleading "0 B elided" marker (PR #2083 review by codex).
|
||||
*
|
||||
* No-op when the cap was never hit at all (head budget never exhausted,
|
||||
* tail ring empty).
|
||||
*/
|
||||
#flushArtifactTailIfCapped(): void {
|
||||
if (!this.#file) return;
|
||||
@@ -1094,14 +1102,16 @@ export class OutputSink {
|
||||
const droppedBytes = Math.max(0, this.#artifactTailIncomingBytes - tailBytes);
|
||||
if (tailBytes === 0 && droppedBytes === 0) return;
|
||||
|
||||
const headWritten = this.#artifactHeadBytesWritten;
|
||||
const totalCapped = headWritten + this.#artifactTailIncomingBytes;
|
||||
const headSep = headWritten > 0 ? "\n" : "";
|
||||
const tailSep = tailBytes > 0 && !this.#artifactTailRing.startsWith("\n") ? "\n" : "";
|
||||
const notice =
|
||||
`${headSep}[ARTIFACT TRUNCATED: kept first ${formatBytes(headWritten)} + last ${formatBytes(tailBytes)} ` +
|
||||
`of ${formatBytes(totalCapped)}; ${formatBytes(droppedBytes)} elided from the middle]${tailSep}`;
|
||||
this.#file.sink.write(notice);
|
||||
if (droppedBytes > 0) {
|
||||
const headWritten = this.#artifactHeadBytesWritten;
|
||||
const totalCapped = headWritten + this.#artifactTailIncomingBytes;
|
||||
const headSep = headWritten > 0 ? "\n" : "";
|
||||
const tailSep = tailBytes > 0 && !this.#artifactTailRing.startsWith("\n") ? "\n" : "";
|
||||
const notice =
|
||||
`${headSep}[ARTIFACT TRUNCATED: kept first ${formatBytes(headWritten)} + last ${formatBytes(tailBytes)} ` +
|
||||
`of ${formatBytes(totalCapped)}; ${formatBytes(droppedBytes)} elided from the middle]${tailSep}`;
|
||||
this.#file.sink.write(notice);
|
||||
}
|
||||
if (tailBytes > 0) {
|
||||
this.#file.sink.write(this.#artifactTailRing);
|
||||
}
|
||||
|
||||
@@ -366,6 +366,31 @@ describe("OutputSink", () => {
|
||||
expect(artifactText).not.toContain("[ARTIFACT TRUNCATED:");
|
||||
});
|
||||
|
||||
test("artifact stays verbatim when spillover exceeds head budget but still fits inside the cap", async () => {
|
||||
// Regression for the PR #2083 review: when the head budget is filled
|
||||
// but the rest still fits in the tail ring, droppedBytes is zero —
|
||||
// the file MUST be the verbatim stream with no `[ARTIFACT TRUNCATED: …]`
|
||||
// marker spliced into the middle.
|
||||
const dir = await createTempDir();
|
||||
const artifactPath = path.join(dir, "spilled.log");
|
||||
const sink = new OutputSink({
|
||||
artifactPath,
|
||||
artifactId: "art-spilled",
|
||||
spillThreshold: 8,
|
||||
artifactMaxBytes: 32,
|
||||
artifactHeadBytes: 16,
|
||||
});
|
||||
|
||||
// 24 bytes total: head takes 16, tail ring receives 8 (fits, no eviction).
|
||||
const payload = "0123456789ABCDEFghijklmn";
|
||||
await sink.push(payload);
|
||||
await sink.dump();
|
||||
const artifactText = await Bun.file(artifactPath).text();
|
||||
|
||||
expect(artifactText).toBe(payload);
|
||||
expect(artifactText).not.toContain("[ARTIFACT TRUNCATED:");
|
||||
});
|
||||
|
||||
test("artifact cap stays bounded across many small streaming chunks", async () => {
|
||||
const dir = await createTempDir();
|
||||
const artifactPath = path.join(dir, "stream.log");
|
||||
|
||||
Reference in New Issue
Block a user