Skip to content

Commit 209b6c8

Browse files
kiranmagic7Kiran Magickiranmagic7steipete
authored
Fix Kiro usage command pipe hangs (#1535)
* Fix Kiro usage command pipe hangs * Fix Kiro probe lint failures * fix: harden Kiro command cleanup * fix: terminate escaped subprocess descendants --------- Co-authored-by: Kiran Magic <kiran@Alices-Laptop.local> Co-authored-by: kiranmagic7 <209323973+kiranmagic7@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
1 parent a174d17 commit 209b6c8

7 files changed

Lines changed: 368 additions & 110 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
- Provider probes: stop waiting indefinitely for inherited output pipes after subprocesses or CLI version checks exit (fixes #1531).
2525
- Menu bar: update visible usage values in place when a manual refresh completes instead of leaving the open provider card stale until the menu is reopened (fixes #1516).
2626
- Gemini: recognize the current `gemini-api-key` CLI auth setting so API-key sessions show the supported OAuth guidance instead of a misleading not-logged-in error (fixes #1511).
27+
- Kiro: keep usage refreshes bounded when CLI helpers retain output pipes, ignore termination, or are cancelled (fixes #1533). Thanks @kiranmagic7!
2728
- Xiaomi MiMo: cancel optional token-plan requests when the required balance request fails instead of delaying the error for up to 30 seconds.
2829
- Settings: make the cost history window directly editable by keyboard while preserving the existing stepper and 1–365 day bounds (fixes #1499). Thanks @kiranmagic7!
2930

Sources/CodexBarCore/Host/PTY/TTYCommandRunner.swift

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ private enum TTYCommandRunnerActiveProcessRegistry {
105105
}
106106

107107
enum TTYProcessTreeTerminator {
108+
struct ProcessIdentity: Equatable {
109+
let pid: pid_t
110+
let startToken: UInt64
111+
}
112+
108113
static func descendantPIDs(
109114
of rootPID: pid_t,
110115
childResolver: (pid_t) -> [pid_t] = Self.currentChildPIDs(of:)) -> [pid_t]
@@ -134,10 +139,49 @@ enum TTYProcessTreeTerminator {
134139
guard childCount > 0 else { return [] }
135140
return Array(pids.prefix(min(Int(childCount), pids.count))).filter { $0 > 0 }
136141
#else
137-
return []
142+
let taskPath = "/proc/\(parentPID)/task"
143+
guard let taskIDs = try? FileManager.default.contentsOfDirectory(atPath: taskPath) else { return [] }
144+
145+
var children: Set<pid_t> = []
146+
for taskID in taskIDs {
147+
let childrenPath = "\(taskPath)/\(taskID)/children"
148+
guard let text = try? String(contentsOfFile: childrenPath, encoding: .utf8) else { continue }
149+
children.formUnion(text.split(whereSeparator: \.isWhitespace).compactMap { pid_t($0) })
150+
}
151+
return children.sorted()
152+
#endif
153+
}
154+
155+
static func processIdentity(for pid: pid_t) -> ProcessIdentity? {
156+
guard pid > 0 else { return nil }
157+
158+
#if canImport(Darwin)
159+
var info = proc_bsdinfo()
160+
let size = proc_pidinfo(
161+
pid,
162+
PROC_PIDTBSDINFO,
163+
0,
164+
&info,
165+
Int32(MemoryLayout<proc_bsdinfo>.stride))
166+
guard size == Int32(MemoryLayout<proc_bsdinfo>.stride) else { return nil }
167+
let startToken = UInt64(info.pbi_start_tvsec) * 1_000_000 + UInt64(info.pbi_start_tvusec)
168+
return ProcessIdentity(pid: pid, startToken: startToken)
169+
#else
170+
guard let stat = try? String(contentsOfFile: "/proc/\(pid)/stat", encoding: .utf8),
171+
let commandEnd = stat.lastIndex(of: ")")
172+
else {
173+
return nil
174+
}
175+
let fields = stat[stat.index(after: commandEnd)...].split(whereSeparator: \.isWhitespace)
176+
guard fields.count > 19, let startToken = UInt64(fields[19]) else { return nil }
177+
return ProcessIdentity(pid: pid, startToken: startToken)
138178
#endif
139179
}
140180

181+
static func isCurrent(_ identity: ProcessIdentity) -> Bool {
182+
self.processIdentity(for: identity.pid) == identity
183+
}
184+
141185
static func terminateProcessTree(
142186
rootPID: pid_t,
143187
processGroup: pid_t?,

Sources/CodexBarCore/Host/Process/ProcessPipeCapture.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@ import Foundation
22

33
package final class ProcessPipeCapture: @unchecked Sendable {
44
private let handle: FileHandle
5+
private let onData: (@Sendable () -> Void)?
56
private let condition = NSCondition()
67
private var data = Data()
78
private var activeCallbacks = 0
89
private var isFinished = false
910
private var isStopping = false
1011
private var continuation: CheckedContinuation<Void, Never>?
1112

12-
package init(pipe: Pipe) {
13+
package init(pipe: Pipe, onData: (@Sendable () -> Void)? = nil) {
1314
self.handle = pipe.fileHandleForReading
15+
self.onData = onData
1416
}
1517

1618
package func start() {
@@ -70,6 +72,8 @@ package final class ProcessPipeCapture: @unchecked Sendable {
7072

7173
if chunk.isEmpty {
7274
handle.readabilityHandler = nil
75+
} else {
76+
self.onData?()
7377
}
7478
continuation?.resume()
7579
}

Sources/CodexBarCore/Host/Process/SubprocessRunner.swift

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,36 @@ public enum SubprocessRunner {
110110
/// Terminates a process and its process group, escalating from SIGTERM to SIGKILL.
111111
/// Returns `true` if the process was actually killed, `false` if it had already exited.
112112
@discardableResult
113-
private static func terminateProcess(_ process: Process, processGroup: pid_t?) -> Bool {
113+
package static func terminateProcess(_ process: Process, processGroup: pid_t?) -> Bool {
114114
guard process.isRunning else { return false }
115-
process.terminate()
116-
if let pgid = processGroup {
117-
kill(-pgid, SIGTERM)
118-
}
115+
let descendants = TTYProcessTreeTerminator.descendantPIDs(of: process.processIdentifier)
116+
let descendantIdentities = descendants.compactMap(TTYProcessTreeTerminator.processIdentity(for:))
117+
TTYProcessTreeTerminator.terminateProcessTree(
118+
rootPID: process.processIdentifier,
119+
processGroup: processGroup,
120+
signal: SIGTERM,
121+
knownDescendants: descendants)
119122
let killDeadline = Date().addingTimeInterval(0.4)
120123
while process.isRunning, Date() < killDeadline {
121124
usleep(50000)
122125
}
123126
if process.isRunning {
124-
if let pgid = processGroup {
125-
kill(-pgid, SIGKILL)
127+
let currentDescendants = descendantIdentities
128+
.filter(TTYProcessTreeTerminator.isCurrent(_:))
129+
.map(\.pid)
130+
TTYProcessTreeTerminator.terminateProcessTree(
131+
rootPID: process.processIdentifier,
132+
processGroup: processGroup,
133+
signal: SIGKILL,
134+
knownDescendants: currentDescendants)
135+
let reapDeadline = Date().addingTimeInterval(0.4)
136+
while process.isRunning, Date() < reapDeadline {
137+
usleep(50000)
138+
}
139+
} else {
140+
for identity in descendantIdentities where TTYProcessTreeTerminator.isCurrent(identity) {
141+
kill(identity.pid, SIGKILL)
126142
}
127-
kill(process.processIdentifier, SIGKILL)
128143
}
129144
return true
130145
}

Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift

Lines changed: 87 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import Foundation
2+
#if canImport(Darwin)
3+
import Darwin
4+
#else
5+
import Glibc
6+
#endif
27

38
public struct KiroUsageSnapshot: Sendable {
49
public let planName: String
@@ -219,7 +224,15 @@ public enum KiroStatusProbeError: LocalizedError, Sendable {
219224
}
220225

221226
public struct KiroStatusProbe: Sendable {
222-
public init() {}
227+
private let cliBinaryResolver: @Sendable () -> String?
228+
229+
public init() {
230+
self.cliBinaryResolver = { TTYCommandRunner.which("kiro-cli") }
231+
}
232+
233+
init(cliBinaryResolver: @escaping @Sendable () -> String?) {
234+
self.cliBinaryResolver = cliBinaryResolver
235+
}
223236

224237
private static let logger = CodexBarLog.logger(LogCategories.kiro)
225238

@@ -256,7 +269,7 @@ public struct KiroStatusProbe: Sendable {
256269
contextUsage: contextUsage)
257270
}
258271

259-
private struct KiroCLIResult {
272+
struct KiroCLIResult {
260273
let stdout: String
261274
let stderr: String
262275
let terminationStatus: Int32
@@ -383,12 +396,12 @@ public struct KiroStatusProbe: Sendable {
383396
return self.parseContextUsage(output: output)
384397
}
385398

386-
private func runCommand(
399+
func runCommand(
387400
arguments: [String],
388401
timeout: TimeInterval,
389402
idleTimeout: TimeInterval = 5.0) async throws -> KiroCLIResult
390403
{
391-
guard let binary = TTYCommandRunner.which("kiro-cli") else {
404+
guard let binary = self.cliBinaryResolver() else {
392405
throw KiroStatusProbeError.cliNotFound
393406
}
394407

@@ -406,124 +419,98 @@ public struct KiroStatusProbe: Sendable {
406419
env["TERM"] = "xterm-256color"
407420
process.environment = env
408421

409-
// Thread-safe state for activity tracking
410422
final class ActivityState: @unchecked Sendable {
411423
private let lock = NSLock()
412424
private var _lastActivityAt = Date()
413425
private var _hasReceivedOutput = false
414-
private var _stdoutData = Data()
415-
private var _stderrData = Data()
416426

417427
var lastActivityAt: Date {
418-
self.lock.lock()
419-
defer { lock.unlock() }
420-
return self._lastActivityAt
428+
self.lock.withLock { self._lastActivityAt }
421429
}
422430

423431
var hasReceivedOutput: Bool {
424-
self.lock.lock()
425-
defer { lock.unlock() }
426-
return self._hasReceivedOutput
427-
}
428-
429-
func appendStdout(_ data: Data) {
430-
self.lock.lock()
431-
defer { lock.unlock() }
432-
self._stdoutData.append(data)
433-
self._lastActivityAt = Date()
434-
self._hasReceivedOutput = true
432+
self.lock.withLock { self._hasReceivedOutput }
435433
}
436434

437-
func appendStderr(_ data: Data) {
438-
self.lock.lock()
439-
defer { lock.unlock() }
440-
self._stderrData.append(data)
441-
self._lastActivityAt = Date()
442-
self._hasReceivedOutput = true
443-
}
444-
445-
func getOutput() -> (stdout: Data, stderr: Data) {
446-
self.lock.lock()
447-
defer { lock.unlock() }
448-
return (self._stdoutData, self._stderrData)
435+
func markActivity() {
436+
self.lock.withLock {
437+
self._lastActivityAt = Date()
438+
self._hasReceivedOutput = true
439+
}
449440
}
450441
}
451442

452443
let state = ActivityState()
444+
let stdoutCapture = ProcessPipeCapture(pipe: stdoutPipe, onData: { state.markActivity() })
445+
let stderrCapture = ProcessPipeCapture(pipe: stderrPipe, onData: { state.markActivity() })
453446

454-
// Set up readability handlers to track activity
455-
stdoutPipe.fileHandleForReading.readabilityHandler = { handle in
456-
let data = handle.availableData
457-
if !data.isEmpty {
458-
state.appendStdout(data)
459-
}
460-
}
461-
stderrPipe.fileHandleForReading.readabilityHandler = { handle in
462-
let data = handle.availableData
463-
if !data.isEmpty {
464-
state.appendStderr(data)
465-
}
447+
do {
448+
try process.run()
449+
} catch {
450+
stdoutCapture.stop()
451+
stderrCapture.stop()
452+
throw error
466453
}
454+
stdoutCapture.start()
455+
stderrCapture.start()
456+
let pid = process.processIdentifier
457+
let processGroup: pid_t? = setpgid(pid, pid) == 0 ? pid : nil
467458

468-
return try await withCheckedThrowingContinuation { continuation in
469-
DispatchQueue.global().async {
470-
do {
471-
try process.run()
472-
} catch {
473-
stdoutPipe.fileHandleForReading.readabilityHandler = nil
474-
stderrPipe.fileHandleForReading.readabilityHandler = nil
475-
continuation.resume(throwing: error)
476-
return
477-
}
459+
let deadline = Date().addingTimeInterval(timeout)
460+
var didHitDeadline = false
461+
var didTerminateForIdle = false
478462

479-
let deadline = Date().addingTimeInterval(timeout)
480-
var didHitDeadline = false
481-
var didTerminateForIdle = false
482-
483-
while process.isRunning {
484-
if Date() >= deadline {
485-
didHitDeadline = true
486-
break
487-
}
488-
// Idle timeout: if we got output but then it went silent
489-
if state.hasReceivedOutput,
490-
Date().timeIntervalSince(state.lastActivityAt) >= idleTimeout
491-
{
492-
// Process went idle after producing output - likely done or stuck
493-
didTerminateForIdle = true
494-
break
495-
}
496-
Thread.sleep(forTimeInterval: 0.1)
463+
do {
464+
while process.isRunning {
465+
try Task.checkCancellation()
466+
if Date() >= deadline {
467+
didHitDeadline = true
468+
break
497469
}
498-
499-
// Clean up handlers
500-
stdoutPipe.fileHandleForReading.readabilityHandler = nil
501-
stderrPipe.fileHandleForReading.readabilityHandler = nil
502-
503-
if process.isRunning {
504-
process.terminate()
505-
process.waitUntilExit()
506-
if didHitDeadline || !state.hasReceivedOutput {
507-
continuation.resume(throwing: KiroStatusProbeError.timeout)
508-
return
509-
}
470+
if state.hasReceivedOutput,
471+
Date().timeIntervalSince(state.lastActivityAt) >= idleTimeout
472+
{
473+
didTerminateForIdle = true
474+
break
510475
}
476+
try await Task.sleep(for: .milliseconds(100))
477+
}
478+
} catch {
479+
await Self.terminateProcess(process, processGroup: processGroup)
480+
stdoutCapture.stop()
481+
stderrCapture.stop()
482+
throw error
483+
}
484+
485+
if process.isRunning {
486+
await Self.terminateProcess(process, processGroup: processGroup)
487+
guard !process.isRunning else {
488+
stdoutCapture.stop()
489+
stderrCapture.stop()
490+
throw KiroStatusProbeError.timeout
491+
}
492+
if didHitDeadline || !state.hasReceivedOutput {
493+
stdoutCapture.stop()
494+
stderrCapture.stop()
495+
throw KiroStatusProbeError.timeout
496+
}
497+
}
498+
499+
async let stdoutData = stdoutCapture.finish(timeout: .seconds(1))
500+
async let stderrData = stderrCapture.finish(timeout: .seconds(1))
501+
let output = await (stdout: stdoutData, stderr: stderrData)
502+
return KiroCLIResult(
503+
stdout: String(data: output.stdout, encoding: .utf8) ?? "",
504+
stderr: String(data: output.stderr, encoding: .utf8) ?? "",
505+
terminationStatus: process.terminationStatus,
506+
terminatedForIdle: didTerminateForIdle)
507+
}
511508

512-
// Read any remaining data
513-
let remainingStdout = stdoutPipe.fileHandleForReading.readDataToEndOfFile()
514-
let remainingStderr = stderrPipe.fileHandleForReading.readDataToEndOfFile()
515-
516-
var output = state.getOutput()
517-
output.stdout.append(remainingStdout)
518-
output.stderr.append(remainingStderr)
519-
520-
let stdoutOutput = String(data: output.stdout, encoding: .utf8) ?? ""
521-
let stderrOutput = String(data: output.stderr, encoding: .utf8) ?? ""
522-
continuation.resume(returning: KiroCLIResult(
523-
stdout: stdoutOutput,
524-
stderr: stderrOutput,
525-
terminationStatus: process.terminationStatus,
526-
terminatedForIdle: didTerminateForIdle))
509+
private static func terminateProcess(_ process: Process, processGroup: pid_t?) async {
510+
await withCheckedContinuation { continuation in
511+
DispatchQueue.global(qos: .userInitiated).async {
512+
SubprocessRunner.terminateProcess(process, processGroup: processGroup)
513+
continuation.resume()
527514
}
528515
}
529516
}

0 commit comments

Comments
 (0)