Skip to content

Commit 67c8282

Browse files
committed
fix: bound report issues by bytes, not just count
CI caught the first attempt: the issues array was capped at 100 entries but not by size. Issues are derived from the same events and carry the same 4 KiB message and 8 KiB URL, so 100 of them is over a megabyte on its own and the report still failed to frame. Both arrays now share one byte-budget helper. The test asserts issue accounting as well as event accounting, which is the assertion that would have caught this the first time.
1 parent eb67274 commit 67c8282

2 files changed

Lines changed: 24 additions & 9 deletions

File tree

apps/headless/Sources/HeadlessProtocol/Diagnostics.swift

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ public final class QADiagnosticStore: @unchecked Sendable {
1111
/// INVALID_REQUEST for a request that was perfectly valid, so the arrays
1212
/// are bounded here and the response says what it dropped.
1313
private let maximumReportEventBytes = 384 * 1_024
14+
/// Issues are derived from the same events and carry the same message and
15+
/// URL, so a count cap alone is not a size cap: 100 issues built from 4 KiB
16+
/// messages and 8 KiB URLs is over a megabyte on its own.
17+
private let maximumReportIssueBytes = 192 * 1_024
1418
private let maximumReportIssues = 100
1519

1620
public init() {}
@@ -81,8 +85,10 @@ public final class QADiagnosticStore: @unchecked Sendable {
8185
let warnings = issues.count - errors
8286
// Counts come from the whole snapshot; only the arrays are bounded, so
8387
// the summary stays accurate even when the payload is trimmed.
84-
let boundedIssues = Array(issues.suffix(maximumReportIssues))
85-
let boundedEvents = eventsWithinBudget(snapshot)
88+
let boundedIssues = valuesWithinBudget(
89+
Array(issues.suffix(maximumReportIssues)), bytes: maximumReportIssueBytes
90+
)
91+
let boundedEvents = valuesWithinBudget(snapshot, bytes: maximumReportEventBytes)
8692
let omittedIssues = issues.count - boundedIssues.count
8793
let omittedEvents = snapshot.count - boundedEvents.count
8894
return .object([
@@ -106,14 +112,14 @@ public final class QADiagnosticStore: @unchecked Sendable {
106112
])
107113
}
108114

109-
/// Drops the oldest events until the array fits the response budget. Newest
110-
/// events are the ones an agent is diagnosing, so they are the ones kept.
111-
private func eventsWithinBudget(_ snapshot: [JSONValue]) -> [JSONValue] {
112-
var kept = snapshot
113-
while kept.count > 1, encodedByteCount(kept) > maximumReportEventBytes {
115+
/// Drops the oldest entries until the array fits its budget. Newest entries
116+
/// are the ones an agent is diagnosing, so they are the ones kept.
117+
private func valuesWithinBudget(_ values: [JSONValue], bytes: Int) -> [JSONValue] {
118+
var kept = values
119+
while kept.count > 1, encodedByteCount(kept) > bytes {
114120
kept.removeFirst(max(1, kept.count / 8))
115121
}
116-
if kept.count == 1, encodedByteCount(kept) > maximumReportEventBytes { return [] }
122+
if kept.count == 1, encodedByteCount(kept) > bytes { return [] }
117123
return kept
118124
}
119125

apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,8 @@ struct ProtocolTests {
674674
try expect(object["truncated"] == .bool(true), "a bounded report should report truncation")
675675
guard case .object(let summary)? = object["summary"],
676676
case .object(let omitted)? = object["omitted"],
677-
case .array(let events)? = object["events"] else {
677+
case .array(let events)? = object["events"],
678+
case .array(let issues)? = object["issues"] else {
678679
throw TestFailure(description: "report bounds")
679680
}
680681
try expect(summary["events"] == .number(500), "summary counts should describe every event")
@@ -683,6 +684,14 @@ struct ProtocolTests {
683684
events.count + Int(omitted["events"]?.numberValue ?? 0) == 500,
684685
"kept plus omitted events should account for the whole buffer"
685686
)
687+
// Issues carry the same message and URL as the events they describe, so
688+
// a count cap is not a size cap — bounding them by bytes is what keeps
689+
// the report inside the frame.
690+
try expect(
691+
issues.count + Int(omitted["issues"]?.numberValue ?? 0)
692+
== Int(summary["issues"]?.numberValue ?? 0),
693+
"kept plus omitted issues should account for every issue"
694+
)
686695
}
687696

688697
static func artifactListingStaysBounded() throws {

0 commit comments

Comments
 (0)