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..462ad41 100644 --- a/apps/headless/Sources/HeadlessProtocol/Diagnostics.swift +++ b/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 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..6165683 100644 --- a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift +++ b/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), 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