From eb672748f3b682fda35cfb4a647d03ba89d00d8f Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Wed, 5 Aug 2026 19:56:59 +0000 Subject: [PATCH 1/2] fix: keep qa report and artifact listings inside the protocol frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #14. A full diagnostic report is 500 events, each holding up to a 4 KiB message and an 8 KiB URL — roughly 2 MB against a 1 MiB frame. Encoding threw inside handleClient, fell into the generic catch, and the agent received INVALID_REQUEST for a request the host had accepted and executed. artifacts list had the same unbounded exposure as the store grew. Both responses are now bounded and say so. qa report trims its issue and event arrays to a byte budget, keeping the newest, while summary counts continue to describe every collected event so the numbers stay honest when the payload is trimmed. artifacts list returns the newest 250 with total and omitted counts. Both set truncated, matching the pruning contract used everywhere else. The transport now distinguishes a response that cannot be framed from a bad request: it answers RESPONSE_TOO_LARGE with a suggestion instead of blaming the caller. That path should now be unreachable for these two commands, but it is the honest fallback for any future response that outgrows the frame. Tests build a report that would previously have exceeded the frame and a store of 260 artifacts, and assert both encode within headlessMaximumMessageBytes with correct omitted accounting. P1 documents the bounds. --- .../Sources/HeadlessProtocol/Artifacts.swift | 14 ++++- .../HeadlessProtocol/Diagnostics.swift | 39 ++++++++++++- .../Sources/HeadlessProtocol/Transport.swift | 16 ++++- .../HeadlessProtocolTests/ProtocolTests.swift | 58 +++++++++++++++++++ apps/headless/docs/P1.md | 8 +++ docs/roadmap/improvements-backlog.md | 10 +++- 6 files changed, 138 insertions(+), 7 deletions(-) diff --git a/apps/headless/Sources/HeadlessProtocol/Artifacts.swift b/apps/headless/Sources/HeadlessProtocol/Artifacts.swift index dc818e6..d97b0ca 100644 --- a/apps/headless/Sources/HeadlessProtocol/Artifacts.swift +++ b/apps/headless/Sources/HeadlessProtocol/Artifacts.swift @@ -173,9 +173,21 @@ public final class ArtifactStore: @unchecked Sendable { guard case .object(let lhs) = left, case .object(let rhs) = right else { return false } return (lhs["createdAt"]?.numberValue ?? 0) > (rhs["createdAt"]?.numberValue ?? 0) } - return .object(["directory": .string(rootURL.path), "artifacts": .array(artifacts)]) + // The store grows without bound across a long session, and the listing + // has to survive the 1 MiB protocol frame. Newest first, bounded, and + // explicit about what was left out. + let listed = Array(artifacts.prefix(Self.maximumListedArtifacts)) + return .object([ + "directory": .string(rootURL.path), + "artifacts": .array(listed), + "total": .number(Double(artifacts.count)), + "omitted": .number(Double(artifacts.count - listed.count)), + "truncated": .bool(listed.count < artifacts.count), + ]) } + private static let maximumListedArtifacts = 250 + private static let listedExtensions: Set = ScreenshotFormat.artifactExtensions .union(RecordingFormat.artifactExtensions) diff --git a/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift b/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift index b3f132c..885f1bd 100644 --- a/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift +++ b/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift @@ -5,6 +5,13 @@ public final class QADiagnosticStore: @unchecked Sendable { private var events: [JSONValue] = [] private var didTruncate = false private let maximumEvents = 500 + /// A full report of 500 events, each carrying up to a 4 KiB message and an + /// 8 KiB URL, can exceed the 1 MiB protocol frame. Encoding would then + /// throw on the way out and the agent would receive a misleading + /// INVALID_REQUEST for a request that was perfectly valid, so the arrays + /// are bounded here and the response says what it dropped. + private let maximumReportEventBytes = 384 * 1_024 + private let maximumReportIssues = 100 public init() {} @@ -72,6 +79,12 @@ public final class QADiagnosticStore: @unchecked Sendable { return object["severity"] == .string("error") }.count let warnings = issues.count - errors + // Counts come from the whole snapshot; only the arrays are bounded, so + // the summary stays accurate even when the payload is trimmed. + let boundedIssues = Array(issues.suffix(maximumReportIssues)) + let boundedEvents = eventsWithinBudget(snapshot) + let omittedIssues = issues.count - boundedIssues.count + let omittedEvents = snapshot.count - boundedEvents.count return .object([ "summary": .object([ "events": .number(Double(snapshot.count)), @@ -83,12 +96,32 @@ public final class QADiagnosticStore: @unchecked Sendable { "errors": .number(Double(errors)), "warnings": .number(Double(warnings)), ]), - "issues": .array(issues), - "events": .array(snapshot), - "truncated": .bool(wasTruncated), + "issues": .array(boundedIssues), + "events": .array(boundedEvents), + "omitted": .object([ + "issues": .number(Double(omittedIssues)), + "events": .number(Double(omittedEvents)), + ]), + "truncated": .bool(wasTruncated || omittedIssues > 0 || omittedEvents > 0), ]) } + /// Drops the oldest events until the array fits the response budget. Newest + /// events are the ones an agent is diagnosing, so they are the ones kept. + private func eventsWithinBudget(_ snapshot: [JSONValue]) -> [JSONValue] { + var kept = snapshot + while kept.count > 1, encodedByteCount(kept) > maximumReportEventBytes { + kept.removeFirst(max(1, kept.count / 8)) + } + if kept.count == 1, encodedByteCount(kept) > maximumReportEventBytes { return [] } + return kept + } + + private func encodedByteCount(_ values: [JSONValue]) -> Int { + guard let data = try? ProtocolCodec.encoder.encode(JSONValue.array(values)) else { return 0 } + return data.count + } + public func console(level: String, limit: Int) -> JSONValue { lock.lock(); let snapshot = events; lock.unlock() let items = snapshot.filter { event in diff --git a/apps/headless/Sources/HeadlessProtocol/Transport.swift b/apps/headless/Sources/HeadlessProtocol/Transport.swift index 98505d2..9948cb9 100644 --- a/apps/headless/Sources/HeadlessProtocol/Transport.swift +++ b/apps/headless/Sources/HeadlessProtocol/Transport.swift @@ -216,7 +216,21 @@ public final class LocalSocketServer: @unchecked Sendable { } else { response = requestQueue.sync { handler(request) } } - try writeAll(try ProtocolCodec.encodeLine(response), to: fd) + // A response that cannot be framed is a response problem, not a bad + // request. Encoding it inside the catch below would report + // INVALID_REQUEST for a request the host accepted and executed. + let payload: Data + do { + payload = try ProtocolCodec.encodeLine(response) + } catch let codecError as CodecError { + payload = try ProtocolCodec.encodeLine(CommandResponse.failure( + id: request.id, + code: "RESPONSE_TOO_LARGE", + message: String(describing: codecError), + suggestion: "Narrow the request with --limit, or run `headless qa clear` to drop collected diagnostics." + )) + } + try writeAll(payload, to: fd) } catch { let response = CommandResponse.failure( id: "unknown", diff --git a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift index 9fdc563..97c77a2 100644 --- a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift +++ b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift @@ -653,6 +653,62 @@ struct ProtocolTests { try expect(!serialized.contains("secret@"), "diagnostics must redact URL credentials") } + static func responsesFitTheProtocolFrame() throws { + // 500 events each carrying a 4 KiB message is roughly 2 MB — twice the + // frame. Before the response bound this encoded past the limit and the + // agent saw INVALID_REQUEST for a valid `qa report`. + let store = QADiagnosticStore() + let wide = String(repeating: "d", count: 4_096) + for _ in 0..<500 { + store.append(kind: "console", level: "error", message: wide, url: "https://example.com/\(wide)") + } + let report = store.report() + let encoded = try ProtocolCodec.encodeLine( + CommandResponse.success(id: "report", result: report) + ) + try expect( + encoded.count <= headlessMaximumMessageBytes, + "a full diagnostic report must fit the protocol frame" + ) + guard case .object(let object) = report else { throw TestFailure(description: "report shape") } + try expect(object["truncated"] == .bool(true), "a bounded report should report truncation") + guard case .object(let summary)? = object["summary"], + case .object(let omitted)? = object["omitted"], + case .array(let events)? = object["events"] else { + throw TestFailure(description: "report bounds") + } + try expect(summary["events"] == .number(500), "summary counts should describe every event") + try expect((omitted["events"]?.numberValue ?? 0) > 0, "omitted events should be counted") + try expect( + events.count + Int(omitted["events"]?.numberValue ?? 0) == 500, + "kept plus omitted events should account for the whole buffer" + ) + } + + static func artifactListingStaysBounded() throws { + let root = "/tmp/headless-artifact-bound-\(UUID().uuidString)" + defer { try? FileManager.default.removeItem(atPath: root) } + let store = try ArtifactStore(environment: ["HEADLESS_ARTIFACT_DIR": root]) + for index in 0..<260 { + _ = try store.write(Data("x".utf8), requestedName: "bound-\(index).json", extension: "json", prefix: "bound") + } + guard case .object(let listing) = try store.list(), + case .array(let artifacts)? = listing["artifacts"] else { + throw TestFailure(description: "artifact listing") + } + try expect(artifacts.count == 250, "artifact listing should stay bounded") + try expect(listing["total"] == .number(260), "artifact listing should report the true total") + try expect(listing["omitted"] == .number(10), "artifact listing should report what it left out") + try expect(listing["truncated"] == .bool(true), "a bounded artifact listing is truncated") + let encoded = try ProtocolCodec.encodeLine( + CommandResponse.success(id: "artifacts", result: try store.list()) + ) + try expect( + encoded.count <= headlessMaximumMessageBytes, + "an artifact listing must fit the protocol frame" + ) + } + static func diagnosticServices() throws { let store = QADiagnosticStore() store.append(kind: "console", level: "warn", message: "first") @@ -785,6 +841,8 @@ struct ProtocolTests { ("screenshot series helpers", screenshotSeriesHelpers), ("diagnostic summary", diagnosticSummary), ("diagnostic bounds and URL redaction", diagnosticsBoundAndRedacted), + ("responses fit the protocol frame", responsesFitTheProtocolFrame), + ("artifact listing stays bounded", artifactListingStaysBounded), ("diagnostic services", diagnosticServices), ("diagnostic CLI", diagnosticCLI), ("local socket round-trip", localSocketRoundTrip), diff --git a/apps/headless/docs/P1.md b/apps/headless/docs/P1.md index 0517143..dac6a87 100644 --- a/apps/headless/docs/P1.md +++ b/apps/headless/docs/P1.md @@ -139,6 +139,14 @@ returned as data, but must not be interpreted as agent commands. Reports keep the newest 500 bounded events. Page output is data; it cannot add commands or execute shell code. +Responses are bounded so they always fit the 1 MiB protocol frame. `qa report` +keeps the newest issues and events that fit the response budget, and +`artifacts list` returns the newest entries. Both report `truncated` and an +`omitted` count, and `qa report` keeps its `summary` counts describing every +collected event rather than only the returned ones. A response that still +cannot be framed fails with `RESPONSE_TOO_LARGE` rather than reporting an +invalid request. + ## Remote files and media `inspect` reports decoded image/video dimensions, media readiness, source, and diff --git a/docs/roadmap/improvements-backlog.md b/docs/roadmap/improvements-backlog.md index fd9f8de..4850d3f 100644 --- a/docs/roadmap/improvements-backlog.md +++ b/docs/roadmap/improvements-backlog.md @@ -53,10 +53,16 @@ requirement so the guards can never become the only thing holding the path up. **A3. Oversized responses break the 1 MiB frame.** ([#14](https://github.com/LockInTime/headless/issues/14)) `qa report` can hold 500 events × ~4 KiB ≈ 2 MB; `artifact.list` is unbounded. `encodeLine` throws inside `handleClient` and the client receives a misleading -`INVALID_REQUEST` (`HP/Transport.swift:219-227`). Fix: response-side bounding — +`INVALID_REQUEST` (`HP/Transport.swift:219-227`). ~~Fix: response-side bounding — pagination (`--limit/--cursor`) or truncation with `truncated: true` — per architecture decision §4. Test: generate >1 MiB of events, assert a bounded, -well-formed response. +well-formed response.~~ **Done** by truncation: `qa report` bounds its issue and +event arrays by byte budget while keeping `summary` counts accurate over the +whole buffer, `artifacts list` returns the newest 250 with `total`/`omitted`, +and both report `truncated`. A response that still cannot be framed now fails +`RESPONSE_TOO_LARGE` instead of `INVALID_REQUEST`. Tests assert both responses +encode within `headlessMaximumMessageBytes`. Cursor pagination remains the +richer answer and is tracked separately (§G3). **A4. Accept-loop error spin.** ([#15](https://github.com/LockInTime/headless/issues/15)) All `accept()` errors are swallowed with `continue` (`HP/Transport.swift:180-184`); persistent EMFILE becomes a hot From 67c828248502ec0b4913c9512c66eb433bc2b271 Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:00:41 +0000 Subject: [PATCH 2/2] 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. --- .../HeadlessProtocol/Diagnostics.swift | 22 ++++++++++++------- .../HeadlessProtocolTests/ProtocolTests.swift | 11 +++++++++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift b/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift index 885f1bd..462ad41 100644 --- a/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift +++ b/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift @@ -11,6 +11,10 @@ public final class QADiagnosticStore: @unchecked Sendable { /// INVALID_REQUEST for a request that was perfectly valid, so the arrays /// are bounded here and the response says what it dropped. private let maximumReportEventBytes = 384 * 1_024 + /// Issues are derived from the same events and carry the same message and + /// URL, so a count cap alone is not a size cap: 100 issues built from 4 KiB + /// messages and 8 KiB URLs is over a megabyte on its own. + private let maximumReportIssueBytes = 192 * 1_024 private let maximumReportIssues = 100 public init() {} @@ -81,8 +85,10 @@ public final class QADiagnosticStore: @unchecked Sendable { let warnings = issues.count - errors // Counts come from the whole snapshot; only the arrays are bounded, so // the summary stays accurate even when the payload is trimmed. - let boundedIssues = Array(issues.suffix(maximumReportIssues)) - let boundedEvents = eventsWithinBudget(snapshot) + let boundedIssues = valuesWithinBudget( + Array(issues.suffix(maximumReportIssues)), bytes: maximumReportIssueBytes + ) + let boundedEvents = valuesWithinBudget(snapshot, bytes: maximumReportEventBytes) let omittedIssues = issues.count - boundedIssues.count let omittedEvents = snapshot.count - boundedEvents.count return .object([ @@ -106,14 +112,14 @@ public final class QADiagnosticStore: @unchecked Sendable { ]) } - /// Drops the oldest events until the array fits the response budget. Newest - /// events are the ones an agent is diagnosing, so they are the ones kept. - private func eventsWithinBudget(_ snapshot: [JSONValue]) -> [JSONValue] { - var kept = snapshot - while kept.count > 1, encodedByteCount(kept) > maximumReportEventBytes { + /// Drops the oldest entries until the array fits its budget. Newest entries + /// are the ones an agent is diagnosing, so they are the ones kept. + private func valuesWithinBudget(_ values: [JSONValue], bytes: Int) -> [JSONValue] { + var kept = values + while kept.count > 1, encodedByteCount(kept) > bytes { kept.removeFirst(max(1, kept.count / 8)) } - if kept.count == 1, encodedByteCount(kept) > maximumReportEventBytes { return [] } + if kept.count == 1, encodedByteCount(kept) > bytes { return [] } return kept } diff --git a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift index 97c77a2..6165683 100644 --- a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift +++ b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift @@ -674,7 +674,8 @@ struct ProtocolTests { try expect(object["truncated"] == .bool(true), "a bounded report should report truncation") guard case .object(let summary)? = object["summary"], case .object(let omitted)? = object["omitted"], - case .array(let events)? = object["events"] else { + case .array(let events)? = object["events"], + case .array(let issues)? = object["issues"] else { throw TestFailure(description: "report bounds") } try expect(summary["events"] == .number(500), "summary counts should describe every event") @@ -683,6 +684,14 @@ struct ProtocolTests { events.count + Int(omitted["events"]?.numberValue ?? 0) == 500, "kept plus omitted events should account for the whole buffer" ) + // Issues carry the same message and URL as the events they describe, so + // a count cap is not a size cap — bounding them by bytes is what keeps + // the report inside the frame. + try expect( + issues.count + Int(omitted["issues"]?.numberValue ?? 0) + == Int(summary["issues"]?.numberValue ?? 0), + "kept plus omitted issues should account for every issue" + ) } static func artifactListingStaysBounded() throws {