Merge pull request #83 from LockInTime/t3code/bounded-responses

fix: keep qa report and artifact listings inside the protocol frame

Author: yashranaway

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<String> =
         ScreenshotFormat.artifactExtensions
         .union(RecordingFormat.artifactExtensions)

apps/headless/Sources/HeadlessProtocol/Diagnostics.swift

@@ -5,6 +5,17 @@ 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
+    /// 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() {}
 
@@ -72,6 +83,14 @@ 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 = 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([
             "summary": .object([
                 "events": .number(Double(snapshot.count)),
@@ -83,12 +102,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 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) > bytes { 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

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",

apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift

@@ -653,6 +653,71 @@ 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"],
+              case .array(let issues)? = object["issues"] 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"
+        )
+        // 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 {
+        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 +850,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),

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

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