Detect coding-agent state from live terminal screens - #8309
lawrencecchen wants to merge 16 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds agent terminal process recognition, VT screen classification, debounced revision scheduling, per-surface runtime coordination, lifecycle-state resolution, terminal output wiring, and debug payload fields. ChangesAgent terminal detection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Greptile SummaryThis PR adds a clean-room terminal-state detector that reads the live bottom 48 rows of active terminal screens to classify coding agents (idle / working / blocked / unknown) across 27 agent families. It introduces a coalesced dirty-signal pipeline — PTY callbacks advance only an atomic revision, a per-surface actor waits for a quiet window, then captures and classifies off the callback path — and wires the results into the existing
Confidence Score: 5/5The change is safe to merge; the new pipeline is well-isolated, cancellation-aware, generation-checked, and covered by unit and lifecycle tests. The dirty-signal → debounce → capture → classify → publish pipeline is correctly structured: PTY callbacks only flip an atomic counter, all screen I/O is off the callback path, the scheduler actor correctly serializes per-surface state, and the generation-checked capture prevents stale snapshot use. The authority model cleanly separates authoritative from session-only lifecycle integrations. The single gap found — argument needle uniqueness not enforced in catalog validation — is a missing guard on a currently non-overlapping set of needles, not a present defect. AgentTerminalProfileCatalog.swift: the failable initializer enforces executable basename uniqueness but not argument needle uniqueness; worth hardening before the profile list grows further. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PTY as PTY Callback
participant Signal as AgentTerminalDirtySignal
participant Scheduler as AgentTerminalStateDetectionScheduler (actor)
participant Observer as AgentTerminalStateSurfaceObserver (@MainActor)
participant Inspector as AgentTerminalProcessInspector (@concurrent)
participant Worker as AgentTerminalClassificationWorker (actor)
participant Runtime as AgentTerminalStateRuntime (@MainActor)
participant Workspace as Workspace (@MainActor)
PTY->>Signal: markDirty() → advance atomic revision
Signal->>Scheduler: yield revision to bufferingNewest stream
Scheduler->>Scheduler: debounce quiet window (90ms) / max latency (350ms)
Scheduler->>Observer: evaluate(revision)
Observer->>Inspector: "identity(pid, generation) [@concurrent]"
Inspector-->>Observer: AgentTerminalProcessIdentity
Observer->>Inspector: "snapshot(pid, generation) [@concurrent]"
Inspector-->>Observer: AgentTerminalProcessSnapshot
Observer->>Observer: capture boundedActiveScreenTailText (via teardown actor)
Observer->>Inspector: identity post-capture validation
Observer-->>Scheduler: AgentTerminalScreenSnapshot
Scheduler->>Worker: classify(surfaceID, snapshot)
Worker-->>Scheduler: AgentTerminalStateClassification
Scheduler->>Scheduler: "suppress if classification == lastPublished"
Scheduler->>Runtime: yield AgentTerminalDetectionUpdate
Runtime->>Workspace: setDetectedAgentLifecycle(statusKey, familyID, state)
Workspace->>Workspace: resolvedStates → effective state
Workspace->>Workspace: recordDetectedAgentLifecycleChange if changed
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PTY as PTY Callback
participant Signal as AgentTerminalDirtySignal
participant Scheduler as AgentTerminalStateDetectionScheduler (actor)
participant Observer as AgentTerminalStateSurfaceObserver (@MainActor)
participant Inspector as AgentTerminalProcessInspector (@concurrent)
participant Worker as AgentTerminalClassificationWorker (actor)
participant Runtime as AgentTerminalStateRuntime (@MainActor)
participant Workspace as Workspace (@MainActor)
PTY->>Signal: markDirty() → advance atomic revision
Signal->>Scheduler: yield revision to bufferingNewest stream
Scheduler->>Scheduler: debounce quiet window (90ms) / max latency (350ms)
Scheduler->>Observer: evaluate(revision)
Observer->>Inspector: "identity(pid, generation) [@concurrent]"
Inspector-->>Observer: AgentTerminalProcessIdentity
Observer->>Inspector: "snapshot(pid, generation) [@concurrent]"
Inspector-->>Observer: AgentTerminalProcessSnapshot
Observer->>Observer: capture boundedActiveScreenTailText (via teardown actor)
Observer->>Inspector: identity post-capture validation
Observer-->>Scheduler: AgentTerminalScreenSnapshot
Scheduler->>Worker: classify(surfaceID, snapshot)
Worker-->>Scheduler: AgentTerminalStateClassification
Scheduler->>Scheduler: "suppress if classification == lastPublished"
Scheduler->>Runtime: yield AgentTerminalDetectionUpdate
Runtime->>Workspace: setDetectedAgentLifecycle(statusKey, familyID, state)
Workspace->>Workspace: resolvedStates → effective state
Workspace->>Workspace: recordDetectedAgentLifecycleChange if changed
Reviews (6): Last reviewed commit: "Bound transient working evidence to term..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func resolvedAgentLifecycleStates( | ||
| _ panelStates: [String: AgentHibernationLifecycleState] | ||
| ) -> [AgentHibernationLifecycleState] { | ||
| var lifecycle = panelStates.filter { | ||
| !AgentHibernationLifecycleStatusKeys.isManualKey($0.key) && | ||
| !AgentHibernationLifecycleStatusKeys.isDetectionKey($0.key) | ||
| } | ||
| var screen: [AgentHibernationLifecycleState] = [] | ||
| for (key, state) in panelStates where AgentHibernationLifecycleStatusKeys.isDetectionKey(key) { | ||
| guard let familyID = AgentHibernationLifecycleStatusKeys.detectionFamilyID(key: key), | ||
| let profile = AgentTerminalProfileCatalog.builtIn.profile(id: familyID) else { | ||
| screen.append(state) | ||
| continue | ||
| } | ||
| if profile.lifecycleAuthoritative { | ||
| if lifecycle[profile.statusKey] == nil { screen.append(state) } | ||
| } else { | ||
| lifecycle.removeValue(forKey: profile.statusKey) | ||
| screen.append(state) | ||
| } | ||
| } | ||
| return Array(lifecycle.values) + screen | ||
| } |
There was a problem hiding this comment.
resolvedAgentLifecycleStates is a file-scope func with no private/fileprivate modifier, making it accessible and actually used from two other source files: SidebarAgentActivitySummary.swift (line 16) and Workspace+AgentLifecycle.swift. This is exactly the pattern the no-ambient-global-state rule flags: a top-level free function used as API that should be a method on the type that owns the data. The natural home is a fileprivate helper within this file combined with a dedicated method on SidebarAgentActivitySummary or Workspace that replaces the free-function call site in SidebarAgentActivitySummary.swift.
Rule Used: Flag new ambient global state in production Swift:... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| func debugState(workspaceID: UUID, surfaceID: UUID) -> (family: String?, state: String, source: String) { | ||
| guard let workspace = AppDelegate.shared?.workspaceFor(tabId: workspaceID) else { | ||
| return (nil, AgentTerminalSemanticState.unknown.rawValue, "none") | ||
| } | ||
| return workspace.debugDetectedAgentState(panelId: surfaceID) | ||
| } |
There was a problem hiding this comment.
debug... in production source
debugState (here) and debugDetectedAgentState on Workspace carry the debug... prefix that the no-test-debug-seam rule flags in production Sources/ files. Both have real production callers (the TerminalController V2 command handler), so they are not test-only seams, but the naming suggests debug-only intent. Consider renaming to something like terminalDetectionState/detectedTerminalAgentState to make clear these are production diagnostic fields.
Rule Used: Flag Swift files under a production Sources path (... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProfileCatalog.swift`:
- Around line 18-29: Update the profile validation in
AgentTerminalProfileCatalog’s catalog-building loop to reject or normalize
matcher data before insertion: trim and normalize executable basenames, reject
empty values and duplicate basenames across profiles, and reject empty contains
needles. Preserve the existing profile ID and hint validation while ensuring
each executable matcher has one unambiguous owning profile.
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift`:
- Around line 26-41: Make AgentTerminalStateClassifier fail closed: at
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift:26-41,
replace normalized path and argv substring matching with direct executable
identity or a registered launch descriptor/hint; at :58-60, remove terminal
titles as semantic evidence; at :67-74, require explicit idle evidence and
return .unknown when absent. Ensure correctness-critical identity and state use
one reliable structured source.
- Around line 62-65: Update the history-evidence branch in
AgentTerminalStateClassifier so that when profile.historyViewNeedles matches
liveEvidence but snapshot.previousReliableState is nil, it sets state to
.unknown and skips blocked/working classification. Preserve returning the
previous reliable state when available, and only evaluate the subsequent
evidence groups when history is not visible.
In `@Sources/AgentTerminalStateRuntime.swift`:
- Around line 57-67: Update startUpdateConsumerIfNeeded so the Task does not
bind self before the long-lived updates stream loop; remove the outer guard let
self and weakly capture self for each iteration, binding it only immediately
before applying that update while preserving cancellation handling.
- Around line 38-48: Update drop(surfaceID:) to cancel the removed
registrationTask immediately, then stop the scheduler before awaiting the task’s
completion. Preserve the subsequent classificationWorker.remove cleanup and
workspace lifecycle clearing, ensuring teardown cannot wait indefinitely on
scheduler.start.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: caaf9e21-dab6-471c-a05c-16b4729a1e24
📒 Files selected for processing (31)
Packages/macOS/CmuxTerminalCore/CLEAN_ROOM.mdPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalAuthorityResolver.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalDetectionClock.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalDetectionConfiguration.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalDetectionUpdate.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalDirtySignal.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalFamilyProfile.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProcessIdentity.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProcessSnapshot.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProfileCatalog.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalScreenSnapshot.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalSemanticState.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassification.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateDetectionScheduler.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateDetectionSchedulerTests.swiftSources/AgentHibernation/AgentHibernationLifecycleState.swiftSources/AgentTerminalClassificationWorker.swiftSources/AgentTerminalProcessInspector.swiftSources/AgentTerminalStateRuntime.swiftSources/AgentTerminalStateSurfaceObserver.swiftSources/GhosttyTerminalView.swiftSources/SidebarAgentActivitySummary.swiftSources/TerminalController.swiftSources/TerminalOutputTeeContext.swiftSources/TerminalSurfaceRuntimeWiring.swiftSources/Workspace+AgentLifecycle.swiftSources/Workspace+AgentTerminalStateDetection.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxproj
| func drop(surfaceID: UUID) { | ||
| let observer = observers.removeValue(forKey: surfaceID) | ||
| let registrationTask = registrationTasks.removeValue(forKey: surfaceID) | ||
| Task { | ||
| _ = await registrationTask?.value | ||
| await scheduler.stop(surfaceID: surfaceID) | ||
| await classificationWorker.remove(surfaceID: surfaceID) | ||
| } | ||
| guard let observer, let workspace = AppDelegate.shared?.workspaceFor(tabId: observer.workspaceID) else { return } | ||
| workspace.clearDetectedAgentLifecycle(panelId: surfaceID) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Fix potential deadlock and task leakage by cancelling the registration task and reordering teardown.
Awaiting registrationTask?.value before calling scheduler.stop(surfaceID:) creates a deadlock risk. If scheduler.start runs continuously until explicitly stopped or cancelled, it will never complete because stop is blocked waiting for it to finish.
Additionally, the task should be explicitly cancelled to ensure any underlying suspended operations are aborted.
🔄 Proposed fix
func drop(surfaceID: UUID) {
let observer = observers.removeValue(forKey: surfaceID)
let registrationTask = registrationTasks.removeValue(forKey: surfaceID)
+ registrationTask?.cancel()
Task {
- _ = await registrationTask?.value
await scheduler.stop(surfaceID: surfaceID)
+ _ = await registrationTask?.value
await classificationWorker.remove(surfaceID: surfaceID)
}
guard let observer, let workspace = AppDelegate.shared?.workspaceFor(tabId: observer.workspaceID) else { return }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func drop(surfaceID: UUID) { | |
| let observer = observers.removeValue(forKey: surfaceID) | |
| let registrationTask = registrationTasks.removeValue(forKey: surfaceID) | |
| Task { | |
| _ = await registrationTask?.value | |
| await scheduler.stop(surfaceID: surfaceID) | |
| await classificationWorker.remove(surfaceID: surfaceID) | |
| } | |
| guard let observer, let workspace = AppDelegate.shared?.workspaceFor(tabId: observer.workspaceID) else { return } | |
| workspace.clearDetectedAgentLifecycle(panelId: surfaceID) | |
| } | |
| func drop(surfaceID: UUID) { | |
| let observer = observers.removeValue(forKey: surfaceID) | |
| let registrationTask = registrationTasks.removeValue(forKey: surfaceID) | |
| registrationTask?.cancel() | |
| Task { | |
| await scheduler.stop(surfaceID: surfaceID) | |
| _ = await registrationTask?.value | |
| await classificationWorker.remove(surfaceID: surfaceID) | |
| } | |
| guard let observer, let workspace = AppDelegate.shared?.workspaceFor(tabId: observer.workspaceID) else { return } | |
| workspace.clearDetectedAgentLifecycle(panelId: surfaceID) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/AgentTerminalStateRuntime.swift` around lines 38 - 48, Update
drop(surfaceID:) to cancel the removed registrationTask immediately, then stop
the scheduler before awaiting the task’s completion. Preserve the subsequent
classificationWorker.remove cleanup and workspace lifecycle clearing, ensuring
teardown cannot wait indefinitely on scheduler.start.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 384-385: Remove the static singleton declaration
agentTerminalStateRuntime from GhosttyTerminalView and move its ownership to a
constructable, injectable runtime owner such as AppDelegate or AppEnvironment.
Pass that instance through the surface runtime dependencies to the consumers
that need it, preserving main-actor isolation without introducing ambient global
state.
In `@Sources/Workspace`+AgentTerminalStateDetection.swift:
- Around line 18-23: Update the lifecycleAuthoritative branch to stop appending
the screen-detected state when lifecycle[profile.statusKey] is absent. Treat the
authoritative lifecycle record as the sole source of truth and fail closed when
that record is missing; leave the non-authoritative branch unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 200c4f2d-88df-4020-a179-b9c22c2b42ab
📒 Files selected for processing (6)
Sources/AgentTerminalStateRuntime.swiftSources/GhosttyTerminalView.swiftSources/SidebarAgentActivitySummary.swiftSources/TerminalController.swiftSources/Workspace+AgentLifecycle.swiftSources/Workspace+AgentTerminalStateDetection.swift
💤 Files with no reviewable changes (1)
- Sources/AgentTerminalStateRuntime.swift
| @MainActor | ||
| private static let agentTerminalStateRuntime = AgentTerminalStateRuntime() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Avoid adding new singletons for runtime state.
Using a private static let creates ambient global state for the agent terminal runtime. As per coding guidelines, "In production Swift code, avoid ambient global state and behavior: do not add... new singletons for runtime state. Put state and behavior on a constructable, injectable owning type."
Please consider moving this instance to a proper injectable owner, such as the AppDelegate or an AppEnvironment that gets passed down to the surface runtime dependencies, instead of maintaining it globally on GhosttyApp.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/GhosttyTerminalView.swift` around lines 384 - 385, Remove the static
singleton declaration agentTerminalStateRuntime from GhosttyTerminalView and
move its ownership to a constructable, injectable runtime owner such as
AppDelegate or AppEnvironment. Pass that instance through the surface runtime
dependencies to the consumers that need it, preserving main-actor isolation
without introducing ambient global state.
Source: Coding guidelines
| if profile.lifecycleAuthoritative { | ||
| if lifecycle[profile.statusKey] == nil { screen.append(state) } | ||
| } else { | ||
| lifecycle.removeValue(forKey: profile.statusKey) | ||
| screen.append(state) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the unreliable screen fallback for authoritative profiles.
When a profile is lifecycleAuthoritative, the UI must trust the single authoritative lifecycle record. Falling back to the screen-detected state when the lifecycle signal is absent violates the strict single-source-of-truth requirement. As per path instructions, "avoid “best effort” heuristics or unreliable fallback branches... replace it with the single authoritative record... failing closed when reliable signal is missing."
🐛 Proposed fix
if profile.lifecycleAuthoritative {
- if lifecycle[profile.statusKey] == nil { screen.append(state) }
+ continue
} else {
lifecycle.removeValue(forKey: profile.statusKey)
screen.append(state)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if profile.lifecycleAuthoritative { | |
| if lifecycle[profile.statusKey] == nil { screen.append(state) } | |
| } else { | |
| lifecycle.removeValue(forKey: profile.statusKey) | |
| screen.append(state) | |
| } | |
| if profile.lifecycleAuthoritative { | |
| continue | |
| } else { | |
| lifecycle.removeValue(forKey: profile.statusKey) | |
| screen.append(state) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Workspace`+AgentTerminalStateDetection.swift around lines 18 - 23,
Update the lifecycleAuthoritative branch to stop appending the screen-detected
state when lifecycle[profile.statusKey] is absent. Treat the authoritative
lifecycle record as the sole source of truth and fail closed when that record is
missing; leave the non-authoritative branch unchanged.
Source: Path instructions
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AgentTerminalStateRuntime.swift (1)
25-30: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSerialize teardown before reinstalling the same surface.
A new
installcan start while an earlierdroptask is pending. That stale teardown subsequently callsstop(surfaceID:)andremove(surfaceID:), destroying the new registration and cache. Await the prior teardown before starting the next generation, or generation-gate these destructive operations in one per-surface owner.As per coding guidelines, Swift architecture changes must preserve clear ownership and lifecycle invariants rather than leaving races representable.
Also applies to: 45-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/AgentTerminalStateRuntime.swift` around lines 25 - 30, The install/drop lifecycle for each surface can race, allowing stale teardown to remove a newer registration and cache. Update install and the corresponding drop task around classificationWorker, registrationTasks, and stop(surfaceID:)/remove(surfaceID:) to serialize teardown before reinstalling the same surface, or generation-gate destructive operations through a single per-surface owner; ensure only the current registration can be stopped and removed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/AgentTerminalStateRuntime.swift`:
- Around line 25-30: The install/drop lifecycle for each surface can race,
allowing stale teardown to remove a newer registration and cache. Update install
and the corresponding drop task around classificationWorker, registrationTasks,
and stop(surfaceID:)/remove(surfaceID:) to serialize teardown before
reinstalling the same surface, or generation-gate destructive operations through
a single per-surface owner; ensure only the current registration can be stopped
and removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 923ea4f6-c6ea-478b-9be6-d8cd5b99553a
📒 Files selected for processing (11)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProcessInspector.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalProfileCatalog.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalScreenSnapshot.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swiftSources/AgentTerminalStateRuntime.swiftSources/AgentTerminalStateSurfaceObserver.swiftSources/GhosttyTerminalView.swiftSources/TerminalController.swiftSources/Workspace+AgentTerminalStateDetection.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (3)
- Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalScreenSnapshot.swift
- Sources/AgentTerminalStateSurfaceObserver.swift
- cmux.xcodeproj/project.pbxproj
| !isManualKey($0.key) && !isDetectionKey($0.key) | ||
| } | ||
| var screen: [AgentHibernationLifecycleState] = [] | ||
| for (key, state) in panelStates where isDetectionKey(key) { | ||
| guard let familyID = detectionFamilyID(key: key), | ||
| let profile = AgentTerminalProfileCatalog.builtIn.profile(id: familyID) else { | ||
| screen.append(state) | ||
| continue | ||
| } | ||
| if profile.lifecycleAuthoritative { | ||
| if lifecycle[profile.statusKey] == nil { screen.append(state) } | ||
| } else { | ||
| lifecycle.removeValue(forKey: profile.statusKey) | ||
| screen.append(state) | ||
| } | ||
| } | ||
| return Array(lifecycle.values) + screen |
There was a problem hiding this comment.
.running for non-authoritative agents via the .idle classifier fallback
resolvedStates unconditionally removes an existing lifecycle state for non-authoritative profiles (lifecycle.removeValue(forKey: profile.statusKey)) and substitutes the screen-detected state. The screen classifier falls through to .idle for any recognized agent whose current terminal output doesn't match a working-evidence group (e.g., the agent is processing in a background tool call and the "esc to interrupt" line has already scrolled off). For pre-existing non-authoritative status keys already in allowedStatusKeys — codex, copilot, cursor, amp, grok, etc. — that have a lifecycle integration writing .running, a single evaluate cycle where no working pattern fires would replace .running with .idle, trigger recordDetectedAgentLifecycleChange, and hand an incorrect idle signal to the hibernation controller.
A safe fix would guard the override: skip removeValue when the screen state is .idle and a lifecycle .running is already present, or treat the screen .idle default as .unknown (only store non-idle positive evidence from screen detection).
Rule Used: Flag correctness-critical detection/identity deriv... (source)
cde697f to
8146f62
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift (1)
26-42: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFail closed instead of deriving agent identity and state from unreliable heuristics.
The classifier uses best-effort guesses for correctness-critical identity and state, violating the single source of truth requirement. As per path instructions, correctness-critical values must use one reliable structured source (such as direct executable identity or a registered launch hint), and missing signals must fail closed rather than relying on fallbacks.
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift#L26-L42: replace arbitrary path and argv substring recognition with direct executable identity or a registered launch hint.Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift#L65-L70: require explicit idle evidence and return.unknowninstead of falling back to.idle.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L41-L44: update or remove this test to expect.unknownwhen explicit evidence is absent.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L53-L60: update or remove this test to avoid asserting unreliable argv substring matching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift` around lines 26 - 42, Make AgentTerminalStateClassifier fail closed: replace normalized path and argv substring matching with only direct executable identity or a registered launch hint, and return nil when neither reliable signal exists. In the classifier’s state resolution, require explicit idle evidence and return .unknown rather than defaulting to .idle. Update AgentTerminalStateClassifierTests at Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift:41-44 to expect .unknown without explicit evidence, and at :53-60 to remove or revise assertions based on unreliable argv substring matching.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift`:
- Around line 26-42: Make AgentTerminalStateClassifier fail closed: replace
normalized path and argv substring matching with only direct executable identity
or a registered launch hint, and return nil when neither reliable signal exists.
In the classifier’s state resolution, require explicit idle evidence and return
.unknown rather than defaulting to .idle. Update
AgentTerminalStateClassifierTests at
Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift:41-44
to expect .unknown without explicit evidence, and at :53-60 to remove or revise
assertions based on unreliable argv substring matching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78e0a815-635b-4273-b08c-f2788e7f9659
📒 Files selected for processing (2)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift (1)
26-31: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFail closed instead of deriving agent identity and state from unreliable heuristics.
As per path instructions, correctness-critical identity and state must come from a single reliable structured source. Do not use path or argv substring heuristics to guess identity, and do not fall back to an assumed
.idlestate when explicit evidence is missing.
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift#L26-L31: remove the path substring heuristic.Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift#L38-L42: remove the argv substring heuristic.Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift#L65-L70: require explicit idle evidence from the profile and return.unknownotherwise.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L36-L44: update the assertion to expect.unknowninstead of.idle.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L53-L60: remove or adapt the test since generic wrappers without reliable hints should not guess identity based on arguments.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L106-L114: remove or adapt the test since versioned paths without reliable hints should not guess identity based on path substrings.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L180-L190: update the assertions to expect.unknown.Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift#L192-L196: update the assertion to expect.unknown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift` around lines 26 - 31, AgentTerminalStateClassifier must fail closed: remove the normalized-path and argv substring heuristics, require explicit idle evidence from the matched profile, and return .unknown when evidence is absent. In AgentTerminalStateClassifierTests, update affected expectations from .idle to .unknown and remove or adapt generic-wrapper and versioned-path tests so they no longer infer identity from arguments or paths. Apply these changes at Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift (26-31, 38-42, 65-70) and Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift (36-44, 53-60, 106-114, 180-190, 192-196).Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift`:
- Around line 26-31: AgentTerminalStateClassifier must fail closed: remove the
normalized-path and argv substring heuristics, require explicit idle evidence
from the matched profile, and return .unknown when evidence is absent. In
AgentTerminalStateClassifierTests, update affected expectations from .idle to
.unknown and remove or adapt generic-wrapper and versioned-path tests so they no
longer infer identity from arguments or paths. Apply these changes at
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swift
(26-31, 38-42, 65-70) and
Packages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift
(36-44, 53-60, 106-114, 180-190, 192-196).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e2b12dd-feac-4f71-8096-471228441c0c
📒 Files selected for processing (2)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/AgentState/AgentTerminalStateClassifier.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/AgentTerminalStateClassifierTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Folded into #7970 so the live terminal observations and session lineage ship through one |
1 participant
Adds terminal-state detection for 21 coding-agent families plus existing cmux families.
The PTY callback only advances a coalesced dirty revision. A per-surface actor waits for a quiet window, reads plain rendered text from the final 48 active-screen rows, verifies process start time and terminal generation before and after capture, and publishes effective changes only. Blocked and history evidence use the full capture; transient working evidence uses the bottom 12 physical rows so stale spinners cannot override the current composer. Complete lifecycle integrations remain authoritative.
Per-surface setup and teardown are serialized. Reinstall waits for prior cleanup, and repeated drops are idempotent. Strict blocked rules require either multiple corroborating fragments or an exact rendered-line prompt.
Verification:
CmuxTerminalCoretests and 90CmuxTerminaltests passastatefrombb89e237cb5fd5b321d74482413e6a4464a1c70e: https://github.com/manaflow-ai/cmux/actions/runs/29561148873/tmp/cmux-debug-astate.socksamplecaptures found no detector stacks; Instruments trace finalization hung, so no Instruments pass is claimedgit diff --checkpass