Fix workspace routing for surface.read_text
Addresses review feedback from https://github.com/manaflow-ai/cmux/pull/219 by resolving read-screen targets against requested workspace/surface instead of the selected workspace.
This commit is contained in:
Lawrence Chen
parent
71a8644411
commit
7dbd7811df
2 changed files with 127 additions and 73 deletions
|
|
@ -2391,6 +2391,10 @@ class TerminalController {
|
|||
}
|
||||
|
||||
private func v2SurfaceReadText(params: [String: Any]) -> V2CallResult {
|
||||
guard let tabManager = v2ResolveTabManager(params: params) else {
|
||||
return .err(code: "unavailable", message: "TabManager not available", data: nil)
|
||||
}
|
||||
|
||||
var includeScrollback = v2Bool(params, "scrollback") ?? false
|
||||
let lineLimit = v2Int(params, "lines")
|
||||
if let lineLimit, lineLimit <= 0 {
|
||||
|
|
@ -2400,24 +2404,98 @@ class TerminalController {
|
|||
includeScrollback = true
|
||||
}
|
||||
|
||||
let response = readTerminalTextBase64(
|
||||
surfaceArg: v2String(params, "surface_id") ?? "",
|
||||
includeScrollback: includeScrollback,
|
||||
lineLimit: lineLimit
|
||||
)
|
||||
guard response.hasPrefix("OK ") else {
|
||||
return .err(code: "internal_error", message: response, data: nil)
|
||||
var result: V2CallResult = .err(code: "internal_error", message: "Failed to read terminal text", data: nil)
|
||||
v2MainSync {
|
||||
guard let ws = v2ResolveWorkspace(params: params, tabManager: tabManager) else {
|
||||
result = .err(code: "not_found", message: "Workspace not found", data: nil)
|
||||
return
|
||||
}
|
||||
|
||||
let surfaceId = v2UUID(params, "surface_id") ?? ws.focusedPanelId
|
||||
guard let surfaceId else {
|
||||
result = .err(code: "not_found", message: "No focused surface", data: nil)
|
||||
return
|
||||
}
|
||||
guard let terminalPanel = ws.terminalPanel(for: surfaceId) else {
|
||||
result = .err(code: "invalid_params", message: "Surface is not a terminal", data: ["surface_id": surfaceId.uuidString])
|
||||
return
|
||||
}
|
||||
|
||||
let response = readTerminalTextBase64(
|
||||
terminalPanel: terminalPanel,
|
||||
includeScrollback: includeScrollback,
|
||||
lineLimit: lineLimit
|
||||
)
|
||||
guard response.hasPrefix("OK ") else {
|
||||
result = .err(code: "internal_error", message: response, data: nil)
|
||||
return
|
||||
}
|
||||
let base64 = String(response.dropFirst(3)).trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let decoded = Data(base64Encoded: base64).flatMap { String(data: $0, encoding: .utf8) }
|
||||
guard let text = decoded ?? (base64.isEmpty ? "" : nil) else {
|
||||
result = .err(code: "internal_error", message: "Failed to decode terminal text", data: nil)
|
||||
return
|
||||
}
|
||||
|
||||
let windowId = v2ResolveWindowId(tabManager: tabManager)
|
||||
result = .ok([
|
||||
"text": text,
|
||||
"base64": base64,
|
||||
"workspace_id": ws.id.uuidString,
|
||||
"workspace_ref": v2Ref(kind: .workspace, uuid: ws.id),
|
||||
"surface_id": surfaceId.uuidString,
|
||||
"surface_ref": v2Ref(kind: .surface, uuid: surfaceId),
|
||||
"window_id": v2OrNull(windowId?.uuidString),
|
||||
"window_ref": v2Ref(kind: .window, uuid: windowId)
|
||||
])
|
||||
}
|
||||
let base64 = String(response.dropFirst(3)).trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let decoded = Data(base64Encoded: base64).flatMap { String(data: $0, encoding: .utf8) }
|
||||
guard let text = decoded ?? (base64.isEmpty ? "" : nil) else {
|
||||
return .err(code: "internal_error", message: "Failed to decode terminal text", data: nil)
|
||||
return result
|
||||
}
|
||||
|
||||
private func readTerminalTextBase64(terminalPanel: TerminalPanel, includeScrollback: Bool = false, lineLimit: Int? = nil) -> String {
|
||||
guard let surface = terminalPanel.surface.surface else { return "ERROR: Terminal surface not found" }
|
||||
|
||||
let pointTag: ghostty_point_tag_e = includeScrollback ? GHOSTTY_POINT_SCREEN : GHOSTTY_POINT_VIEWPORT
|
||||
let topLeft = ghostty_point_s(
|
||||
tag: pointTag,
|
||||
coord: GHOSTTY_POINT_COORD_TOP_LEFT,
|
||||
x: 0,
|
||||
y: 0
|
||||
)
|
||||
let bottomRight = ghostty_point_s(
|
||||
tag: pointTag,
|
||||
coord: GHOSTTY_POINT_COORD_BOTTOM_RIGHT,
|
||||
x: 0,
|
||||
y: 0
|
||||
)
|
||||
let selection = ghostty_selection_s(
|
||||
top_left: topLeft,
|
||||
bottom_right: bottomRight,
|
||||
rectangle: true
|
||||
)
|
||||
var text = ghostty_text_s()
|
||||
|
||||
guard ghostty_surface_read_text(surface, selection, &text) else {
|
||||
return "ERROR: Failed to read terminal text"
|
||||
}
|
||||
defer {
|
||||
ghostty_surface_free_text(surface, &text)
|
||||
}
|
||||
|
||||
return .ok([
|
||||
"text": text,
|
||||
"base64": base64
|
||||
])
|
||||
let rawData: Data
|
||||
if let ptr = text.text, text.text_len > 0 {
|
||||
rawData = Data(bytes: ptr, count: Int(text.text_len))
|
||||
} else {
|
||||
rawData = Data()
|
||||
}
|
||||
|
||||
var output = String(decoding: rawData, as: UTF8.self)
|
||||
if let lineLimit {
|
||||
output = tailTerminalLines(output, maxLines: lineLimit)
|
||||
}
|
||||
|
||||
let base64 = output.data(using: .utf8)?.base64EncodedString() ?? ""
|
||||
return "OK \(base64)"
|
||||
}
|
||||
|
||||
private func v2SurfaceTriggerFlash(params: [String: Any]) -> V2CallResult {
|
||||
|
|
@ -6302,54 +6380,16 @@ class TerminalController {
|
|||
}
|
||||
|
||||
guard let panelId,
|
||||
let terminalPanel = tab.terminalPanel(for: panelId),
|
||||
let surface = terminalPanel.surface.surface else {
|
||||
let terminalPanel = tab.terminalPanel(for: panelId) else {
|
||||
result = "ERROR: Terminal surface not found"
|
||||
return
|
||||
}
|
||||
|
||||
let pointTag: ghostty_point_tag_e = includeScrollback ? GHOSTTY_POINT_SCREEN : GHOSTTY_POINT_VIEWPORT
|
||||
let topLeft = ghostty_point_s(
|
||||
tag: pointTag,
|
||||
coord: GHOSTTY_POINT_COORD_TOP_LEFT,
|
||||
x: 0,
|
||||
y: 0
|
||||
result = readTerminalTextBase64(
|
||||
terminalPanel: terminalPanel,
|
||||
includeScrollback: includeScrollback,
|
||||
lineLimit: lineLimit
|
||||
)
|
||||
let bottomRight = ghostty_point_s(
|
||||
tag: pointTag,
|
||||
coord: GHOSTTY_POINT_COORD_BOTTOM_RIGHT,
|
||||
x: 0,
|
||||
y: 0
|
||||
)
|
||||
var selection = ghostty_selection_s(
|
||||
top_left: topLeft,
|
||||
bottom_right: bottomRight,
|
||||
rectangle: true
|
||||
)
|
||||
var text = ghostty_text_s()
|
||||
|
||||
guard ghostty_surface_read_text(surface, selection, &text) else {
|
||||
result = "ERROR: Failed to read terminal text"
|
||||
return
|
||||
}
|
||||
defer {
|
||||
ghostty_surface_free_text(surface, &text)
|
||||
}
|
||||
|
||||
let rawData: Data
|
||||
if let ptr = text.text, text.text_len > 0 {
|
||||
rawData = Data(bytes: ptr, count: Int(text.text_len))
|
||||
} else {
|
||||
rawData = Data()
|
||||
}
|
||||
|
||||
var output = String(decoding: rawData, as: UTF8.self)
|
||||
if let lineLimit {
|
||||
output = tailTerminalLines(output, maxLines: lineLimit)
|
||||
}
|
||||
|
||||
let base64 = output.data(using: .utf8)?.base64EncodedString() ?? ""
|
||||
result = "OK \(base64)"
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,46 +66,60 @@ def main() -> int:
|
|||
methods = set(caps.get("methods") or [])
|
||||
_must("surface.read_text" in methods, f"Missing surface.read_text in capabilities: {sorted(methods)[:20]}")
|
||||
|
||||
created = c._call("workspace.create") or {}
|
||||
ws_id = str(created.get("workspace_id") or "")
|
||||
_must(bool(ws_id), f"workspace.create returned no workspace_id: {created}")
|
||||
c._call("workspace.select", {"workspace_id": ws_id})
|
||||
created_target = c._call("workspace.create") or {}
|
||||
ws_target = str(created_target.get("workspace_id") or "")
|
||||
_must(bool(ws_target), f"workspace.create returned no workspace_id: {created_target}")
|
||||
c._call("workspace.select", {"workspace_id": ws_target})
|
||||
|
||||
surfaces_payload = c._call("surface.list", {"workspace_id": ws_id}) or {}
|
||||
surfaces_payload = c._call("surface.list", {"workspace_id": ws_target}) or {}
|
||||
surfaces = surfaces_payload.get("surfaces") or []
|
||||
_must(bool(surfaces), f"Expected at least one surface in workspace: {surfaces_payload}")
|
||||
surface_id = str(surfaces[0].get("id") or "")
|
||||
_must(bool(surface_id), f"surface.list returned surface without id: {surfaces_payload}")
|
||||
surface_target = str(surfaces[0].get("id") or "")
|
||||
_must(bool(surface_target), f"surface.list returned surface without id: {surfaces_payload}")
|
||||
|
||||
created_other = c._call("workspace.create") or {}
|
||||
ws_other = str(created_other.get("workspace_id") or "")
|
||||
_must(bool(ws_other), f"workspace.create returned no workspace_id: {created_other}")
|
||||
c._call("workspace.select", {"workspace_id": ws_other})
|
||||
|
||||
selected = c._call("workspace.current") or {}
|
||||
_must(str(selected.get("workspace_id") or "") == ws_other, f"Expected selected workspace {ws_other}, got: {selected}")
|
||||
|
||||
token = f"CMUX_READ_SCREEN_{int(time.time() * 1000)}"
|
||||
c._call("surface.send_text", {
|
||||
"workspace_id": ws_id,
|
||||
"surface_id": surface_id,
|
||||
"workspace_id": ws_target,
|
||||
"surface_id": surface_target,
|
||||
"text": f"echo {token}\n",
|
||||
})
|
||||
|
||||
def has_token() -> bool:
|
||||
payload = c._call("surface.read_text", {"workspace_id": ws_id, "surface_id": surface_id}) or {}
|
||||
payload = c._call("surface.read_text", {"workspace_id": ws_target, "surface_id": surface_target}) or {}
|
||||
return token in str(payload.get("text") or "")
|
||||
|
||||
_wait_for(has_token, timeout_s=5.0)
|
||||
|
||||
read_payload = c._call("surface.read_text", {"workspace_id": ws_id, "surface_id": surface_id}) or {}
|
||||
read_payload = c._call("surface.read_text", {"workspace_id": ws_target, "surface_id": surface_target}) or {}
|
||||
text = str(read_payload.get("text") or "")
|
||||
_must(token in text, f"surface.read_text missing token {token!r}: {read_payload}")
|
||||
|
||||
cli_text = _run_cli(cli, ["read-screen", "--workspace", ws_id, "--surface", surface_id])
|
||||
ws_only_payload = c._call("surface.read_text", {"workspace_id": ws_target}) or {}
|
||||
_must(token in str(ws_only_payload.get("text") or ""), f"surface.read_text workspace-only call missing token {token!r}: {ws_only_payload}")
|
||||
|
||||
cli_text = _run_cli(cli, ["read-screen", "--workspace", ws_target, "--surface", surface_target])
|
||||
_must(token in cli_text, f"cmux read-screen output missing token {token!r}: {cli_text!r}")
|
||||
|
||||
cli_text_scrollback = _run_cli(cli, ["read-screen", "--workspace", ws_id, "--surface", surface_id, "--scrollback", "--lines", "80"])
|
||||
cli_ws_only = _run_cli(cli, ["read-screen", "--workspace", ws_target])
|
||||
_must(token in cli_ws_only, f"cmux read-screen --workspace output missing token {token!r}: {cli_ws_only!r}")
|
||||
|
||||
cli_text_scrollback = _run_cli(cli, ["read-screen", "--workspace", ws_target, "--surface", surface_target, "--scrollback", "--lines", "80"])
|
||||
_must(token in cli_text_scrollback, f"cmux read-screen --scrollback output missing token {token!r}: {cli_text_scrollback!r}")
|
||||
|
||||
cli_json = _run_cli(cli, ["--json", "read-screen", "--workspace", ws_id, "--surface", surface_id])
|
||||
cli_json = _run_cli(cli, ["--json", "read-screen", "--workspace", ws_target, "--surface", surface_target])
|
||||
payload = json.loads(cli_json or "{}")
|
||||
_must(token in str(payload.get("text") or ""), f"cmux --json read-screen missing token {token!r}: {payload}")
|
||||
|
||||
invalid = subprocess.run(
|
||||
[cli, "--socket", SOCKET_PATH, "read-screen", "--workspace", ws_id, "--surface", surface_id, "--lines", "0"],
|
||||
[cli, "--socket", SOCKET_PATH, "read-screen", "--workspace", ws_target, "--surface", surface_target, "--lines", "0"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue