From bcc01ae783df20753fb8a8f4918e25eccfcb5f09 Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:21:27 +0000 Subject: [PATCH] fix(mcp): share CLI timeout derivation --- apps/headless/MCP/main.swift | 4 +--- apps/headless/Sources/HeadlessCLI/main.swift | 16 ++------------ .../Sources/HeadlessProtocol/CLI.swift | 12 ++++++++++ .../HeadlessProtocolTests/ProtocolTests.swift | 22 +++++++++++++++++++ docs/roadmap/improvements-backlog.md | 4 +++- 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/apps/headless/MCP/main.swift b/apps/headless/MCP/main.swift index b208d7a..53f3634 100644 --- a/apps/headless/MCP/main.swift +++ b/apps/headless/MCP/main.swift @@ -65,10 +65,8 @@ while let line = readLine() { toolResult(id: id, text: "MCP accepts browser commands only; run `headless start` on the VM first.", isError: true) continue } - let isLongScreenshot = command.command == .screenshot - && command.parameters["series"]?.stringValue != nil let response = try LocalSocketClient().send( - command, timeout: command.command == .tour || isLongScreenshot ? 125 : 30 + command, timeout: requestTimeout(for: command) ) let encoded = try ProtocolCodec.encoder.encode(response) toolResult(id: id, text: String(decoding: encoded, as: UTF8.self), isError: !response.ok) diff --git a/apps/headless/Sources/HeadlessCLI/main.swift b/apps/headless/Sources/HeadlessCLI/main.swift index 396ab12..ea1d886 100644 --- a/apps/headless/Sources/HeadlessCLI/main.swift +++ b/apps/headless/Sources/HeadlessCLI/main.swift @@ -101,18 +101,6 @@ private enum HostLaunchError: Error, CustomStringConvertible { } } -private func requestTimeout(_ request: CommandRequest) -> TimeInterval { - if let milliseconds = request.parameters["timeoutMs"]?.numberValue { - return min(125, max(10, milliseconds / 1_000 + 5)) - } - if request.command == .tour { return 125 } - if request.command == .recordStop { return 30 } - if request.command == .screenshot { - return request.parameters["series"]?.stringValue == nil ? 30 : 125 - } - return 15 -} - do { let invocation = try CLIParser().parse(Array(CommandLine.arguments.dropFirst())) if let local = invocation.local { @@ -137,10 +125,10 @@ do { let launcher = HostLauncher() let response: CommandResponse do { - response = try launcher.client.send(request, timeout: requestTimeout(request)) + response = try launcher.client.send(request, timeout: requestTimeout(for: request)) } catch LocalTransportError.connectionFailed where request.command != .ping && request.command != .shutdown { _ = try launcher.start() - response = try launcher.client.send(request, timeout: requestTimeout(request)) + response = try launcher.client.send(request, timeout: requestTimeout(for: request)) } try printResponse(response) if !response.ok { exit(1) } diff --git a/apps/headless/Sources/HeadlessProtocol/CLI.swift b/apps/headless/Sources/HeadlessProtocol/CLI.swift index f0a5cca..20fe431 100644 --- a/apps/headless/Sources/HeadlessProtocol/CLI.swift +++ b/apps/headless/Sources/HeadlessProtocol/CLI.swift @@ -19,6 +19,18 @@ public struct CLIInvocation: Equatable, Sendable { } } +public func requestTimeout(for request: CommandRequest) -> TimeInterval { + if let milliseconds = request.parameters["timeoutMs"]?.numberValue { + return min(125, max(10, milliseconds / 1_000 + 5)) + } + if request.command == .tour { return 125 } + if request.command == .recordStop { return 30 } + if request.command == .screenshot { + return request.parameters["series"]?.stringValue == nil ? 30 : 125 + } + return 15 +} + public enum CLIParseError: Error, Equatable, CustomStringConvertible { case missingCommand case unknownCommand(String) diff --git a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift index 7c4c8b4..eee157e 100644 --- a/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift +++ b/apps/headless/Tests/HeadlessProtocolTests/ProtocolTests.swift @@ -350,6 +350,27 @@ struct ProtocolTests { } } + static func clientTimeoutsMatchCommandBounds() throws { + let longWait = try CLIParser().parse(["wait", "--timeout", "90000"]) + try expect( + longWait.request.map { requestTimeout(for: $0) } == 95, + "wait timeout should include the five-second transport allowance" + ) + let maximumWait = try CLIParser().parse(["wait", "--timeout", "120000"]) + try expect(maximumWait.request.map { requestTimeout(for: $0) } == 125, "wait timeout should remain capped") + let shortWait = try CLIParser().parse(["wait", "--timeout", "100"]) + try expect(shortWait.request.map { requestTimeout(for: $0) } == 10, "wait timeout should retain the transport minimum") + + try expect(requestTimeout(for: CommandRequest(command: .tour)) == 125, "tour should use the long timeout") + try expect( + requestTimeout(for: CommandRequest(command: .screenshot, parameters: ["series": .string("viewport")])) == 125, + "screenshot series should use the long timeout" + ) + try expect(requestTimeout(for: CommandRequest(command: .screenshot)) == 30, "single screenshots should get 30 seconds") + try expect(requestTimeout(for: CommandRequest(command: .recordStop)) == 30, "record stop should get 30 seconds") + try expect(requestTimeout(for: CommandRequest(command: .ping)) == 15, "ordinary commands should use the shared default") + } + static func cliP1Artifacts() throws { let screenshot = try CLIParser().parse([ "--session", "qa", "screenshot", "--role", "button", "--name", "Continue", @@ -876,6 +897,7 @@ struct ProtocolTests { ("CLI conflicting target", cliRejectsConflictingClickTarget), ("CLI settled wait", cliWaitDefaultsToSettled), ("CLI timeout bound", cliRejectsUnboundedTimeout), + ("client timeout parity", clientTimeoutsMatchCommandBounds), ("CLI P1 artifacts", cliP1Artifacts), ("CLI P2 commands and boundaries", cliP2CommandsAndBoundaries), ("Chromium runtime selection", chromiumRuntimeSelection), diff --git a/docs/roadmap/improvements-backlog.md b/docs/roadmap/improvements-backlog.md index ce6f063..90d8622 100644 --- a/docs/roadmap/improvements-backlog.md +++ b/docs/roadmap/improvements-backlog.md @@ -209,7 +209,9 @@ architecture decision ยง7. **C1. Timeout parity.** ([#29](https://github.com/LockInTime/headless/issues/29)) MCP uses flat 30 s except tour/series (`apps/headless/MCP/main.swift:70-72`); CLI derives from `--timeout` (`HeadlessCLI/main.swift:104-114`). `wait --timeout 90000` works in CLI, dies -via MCP. Derive identically. +via MCP. ~~Derive identically.~~ **Done:** both adapters now call the same +`HeadlessProtocol.requestTimeout(for:)` helper. Coverage locks the ordinary, +wait-derived, tour, screenshot-series, screenshot, and recording-stop cases. **C2. Destructive verbs over MCP.** ([#30](https://github.com/LockInTime/headless/issues/30)) `stop` (shutdown) and `session close` are callable though the tool description says "safe"; decide policy (deny, or