Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .ai/memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,14 @@
- **`XCUIApplication.launch()` does not reliably foreground the app on macOS.** The runner keeps focus, and a backgrounded app's window has an **empty accessibility tree** — so every element query fails with "does not exist" and the only cure is clicking the Dock icon. `BrewApp.launch` calls `activate()` and waits for `.runningForeground`. This is load-bearing, not cosmetic.
- **Element-not-found failures carry a diagnosis.** `BrewUITestDiagnostics` reports whether the app is running, foregrounded, has a window, and which identifiers are actually in the tree. "Expected installed.screen to exist within 60s" is true and explains nothing; distinguishing "never opened a window" from "wrong identifier" is the difference between a five-minute fix and a day.
- **Assertions gated on a subprocess use the command timeout, not the render timeout.** `DoctorReport.placeholder.isHealthy` is `false`, so while `brew doctor` is in flight the Doctor screen shows the redacted *issues* skeleton and the healthy text does not exist yet. Same for Configuration, whose cards only exist once `brew config` has been parsed.

## 2026-08-27 — The console assembler keeps a window of revisable rows, not a single line

- **The bug.** On a multi-cask `brew upgrade` the console repeated the whole download block every tick instead of updating it. Homebrew's parallel download queue (`download_queue.rb`) draws N rows, deliberately leaves the last one **without** a newline, then rewinds with `Tty.move_cursor_up_beginning` → `ESC[<n>F` (CPL). `TerminalLineAssembler` handled `m`/`K`/`G`/`C`/`D` and dropped everything else, so the rewind was lost. Both reported symptoms came from that one gap: the block repeated, *and* the newline-less last row spliced into the next frame's first row.
- **A single line of cells was the wrong shape**, and the old doc comment said so. The fix keeps `windowDepth` rows of history addressable rather than only the row being written. **Depth = the pty's row count**, because that *is* the screen — a terminal cannot address above it. Brew's block is `min(concurrency, Tty.height)` and `Tty.height` reads that same pty (`PseudoTerminal.defaultRows`, 40); default concurrency is `CPU cores × 2`, so 40 always covers it.
- **The non-obvious part is `\n`.** Inside a block it must **step the cursor down** (decrement `rowCursor`, reset column), *not* open a new row — only at `rowCursor == 0` does it commit. Without that the window fills with duplicates and nothing is actually fixed.
- **`windowDepth` is injected, not read.** `TerminalLineAssembler` is in `BrewCore`; `PseudoTerminal` is in `BrewCLI`, which depends on `BrewCore` and not the reverse. `BrewCLI` passes its own value in; `defaultWindowDepth` mirrors it with a comment saying why it can't reference it.
- **Rows carry a serial** so revisions coalesce per row across a chunk even when a commit shifts every position along. Offsets count **from the end**, which is what keeps them valid after `CommandJob.maxOutputLines` trims the front.
- **Over-scroll degrades to appending rather than clamping.** A move above the window is ignored — the pre-window behaviour, which looks wrong but never writes to the *wrong* row. Deliberate escape hatch for sequences this model doesn't cover (`ESC[H`, `ESC[J`, scroll regions, alt screen — brew's download queue uses none of them).
- **Trailing-newline-ness is a property of how a row ended, not of `isComplete`.** Conflating them regressed `BrewCommandServicePseudoTerminalTests`: a child that ends with `printf 'tty'` (no newline) must not gain one in `standardOutput`. `TerminalTranscript` tracks `endsWithNewline` separately so the returned transcript holds settled rows without inventing terminators.
- **Rejected: adopting SwiftTerm.** Its `Terminal` models a fixed grid + scrollback; the console models a variable-length transcript with stable row identities for SwiftUI diffing. Using the engine headless means diffing a grid to infer which rows changed — plausibly more code than this, and it fights `CONVENTIONS.md`'s "add packages sparingly". Revisit only if the app ever needs to run arbitrary interactive commands (a shell, a pager, anything using the alt screen), where hand-rolling stops being viable.
64 changes: 56 additions & 8 deletions Sources/BrewCLI/BrewCommandService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,19 +138,17 @@ private extension BrewCommandService {
await withCheckedContinuation { (continuation: CheckedContinuation<String, Never>) in
// `read` parks on `poll`; run it on a GCD worker rather than the cooperative pool.
DispatchQueue.global(qos: .utility).async {
var transcript = ""
var rows = TerminalTranscript()
var undecoded = Data()
var assembler = TerminalLineAssembler()
var assembler = TerminalLineAssembler(windowDepth: Int(PseudoTerminal.defaultRows))
var readFailure: Int32?

loop: while true {
switch terminal.read() {
case let .data(chunk):
undecoded.append(chunk)
for event in assembler.consume(UTF8StreamDecoder.takeDecodablePrefix(&undecoded)) {
if case let .committed(line) = event {
transcript += line.text + "\n"
}
rows.apply(event)
emit(event, sink: sink)
}
case .timedOut:
Expand All @@ -167,9 +165,10 @@ private extension BrewCommandService {

// Output that ended without a trailing newline is settled by the stream ending.
if let trailing = assembler.flush() {
transcript += trailing.text
rows.settle(trailing.text)
sink?(BrewCommandOutputLine(stream: .stdout, line: trailing, isComplete: true))
}
var transcript = rows.text
// The exit status may still say success, so truncation has to say so itself.
if let readFailure {
let notice = truncationNotice(errno: readFailure)
Expand All @@ -193,8 +192,57 @@ private extension BrewCommandService {
switch event {
case let .committed(line):
sink(BrewCommandOutputLine(stream: .stdout, line: line, isComplete: true))
case let .revised(line):
sink(BrewCommandOutputLine(stream: .stdout, line: line, isComplete: false))
case let .revised(line, rowOffset):
sink(BrewCommandOutputLine(stream: .stdout, line: line, isComplete: rowOffset > 0, rowOffset: rowOffset))
}
}
}

/// Mirrors what the sink is told, so the transcript holds settled rows rather than every frame.
struct TerminalTranscript {
/// `endsWithNewline` is how the row ended, not whether it can still change: output that stopped
/// without a trailing newline must not gain one.
private struct Row {
var text: String
var isComplete: Bool
var endsWithNewline: Bool
}

private var rows: [Row] = []

var text: String {
rows.map { $0.text + ($0.endsWithNewline ? "\n" : "") }.joined()
}

mutating func apply(_ event: TerminalLineEvent) {
switch event {
case let .committed(line):
put(line.text, isComplete: true, endsWithNewline: true, rowOffset: 0)
case let .revised(line, rowOffset):
put(line.text, isComplete: rowOffset > 0, endsWithNewline: false, rowOffset: rowOffset)
}
}

/// Settles the trailing row; the stream ending is not itself a newline.
mutating func settle(_ text: String) {
put(text, isComplete: true, endsWithNewline: false, rowOffset: 0)
}

private mutating func put(_ text: String, isComplete: Bool, endsWithNewline: Bool, rowOffset: Int) {
guard rowOffset == 0 else {
// A row further back has already ended; only its content can change.
let index = rows.count - 1 - rowOffset
guard rows.indices.contains(index) else {
return
}
rows[index].text = text
return
}
let row = Row(text: text, isComplete: isComplete, endsWithNewline: endsWithNewline)
if let last = rows.last, !last.isComplete {
rows[rows.count - 1] = row
} else {
rows.append(row)
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions Sources/BrewCore/Operations/BrewCommandOutputLine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public struct BrewCommandOutputLine: Identifiable, Equatable, Sendable {
/// `false` while the line may still be revised.
public let isComplete: Bool

/// Rows back from the end of the buffer this line belongs. Always 0 on the pipe path.
public let rowOffset: Int

public enum Stream: Equatable, Sendable {
case stdout
case stderr
Expand All @@ -32,12 +35,14 @@ public struct BrewCommandOutputLine: Identifiable, Equatable, Sendable {
timestamp: Date = Date(),
id: UUID = UUID(),
isComplete: Bool = true,
rowOffset: Int = 0,
) {
self.id = id
self.stream = stream
self.text = text
self.timestamp = timestamp
self.isComplete = isComplete
self.rowOffset = rowOffset
spans = ANSIParser.parse(text)
}

Expand All @@ -48,12 +53,14 @@ public struct BrewCommandOutputLine: Identifiable, Equatable, Sendable {
isComplete: Bool,
timestamp: Date = Date(),
id: UUID = UUID(),
rowOffset: Int = 0,
) {
self.id = id
self.stream = stream
text = line.text
self.timestamp = timestamp
self.isComplete = isComplete
self.rowOffset = rowOffset
spans = line.spans
}

Expand All @@ -68,6 +75,7 @@ public struct BrewCommandOutputLine: Identifiable, Equatable, Sendable {
text = other.text
timestamp = other.timestamp
isComplete = other.isComplete
rowOffset = other.rowOffset
spans = other.spans
}
}
Loading
Loading