Skip to content

Commit 3dd7a6e

Browse files
authored
fix: shellstate-unknown-race (issue manaflow-ai#6618) [salvaged from ultracode worktree] (#31)
1 parent f6352de commit 3dd7a6e

3 files changed

Lines changed: 64 additions & 14 deletions

File tree

Sources/TabManager.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,13 +2750,19 @@ class TabManager: ObservableObject {
27502750
)
27512751
}
27522752

2753+
/// Updates the shell-activity state for a surface.
2754+
///
2755+
/// - Returns: `true` if the update was applied (workspace and panel both exist and
2756+
/// state changed), `false` otherwise. Callers that deduplicate reports MUST only
2757+
/// record the state in their dedup dict when this returns `true`.
2758+
@discardableResult
27532759
func updateSurfaceShellActivity(
27542760
tabId: UUID,
27552761
surfaceId: UUID,
27562762
state: Workspace.PanelShellActivityState
2757-
) {
2758-
guard let tab = tabs.first(where: { $0.id == tabId }) else { return }
2759-
tab.updatePanelShellActivityState(panelId: surfaceId, state: state)
2763+
) -> Bool {
2764+
guard let tab = tabs.first(where: { $0.id == tabId }) else { return false }
2765+
return tab.updatePanelShellActivityState(panelId: surfaceId, state: state)
27602766
}
27612767

27622768
private func normalizeDirectory(_ directory: String) -> String {

Sources/TerminalController.swift

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -458,21 +458,37 @@ class TerminalController {
458458
}
459459
}
460460

461+
/// Returns `true` when the incoming state differs from the last *applied* state,
462+
/// meaning the report is worth dispatching to the main thread.
463+
///
464+
/// This method only reads the dedup dict; it does NOT record the new state.
465+
/// Call `recordShellActivity` after the update is confirmed applied on the main
466+
/// thread so that a failed apply (panel absent) never suppresses the next
467+
/// identical report.
461468
func shouldPublishShellActivity(
462469
workspaceId: UUID,
463470
panelId: UUID,
464471
state: Workspace.PanelShellActivityState
465472
) -> Bool {
466473
let key = SocketSurfaceKey(workspaceId: workspaceId, panelId: panelId)
467474
return queue.sync {
468-
if lastReportedShellStates[key] == state {
469-
return false
470-
}
471-
if lastReportedShellStates.count >= maxTrackedShellStates {
472-
lastReportedShellStates.removeAll(keepingCapacity: true)
475+
lastReportedShellStates[key] != state
476+
}
477+
}
478+
479+
/// Records that the given state was successfully applied to the given panel.
480+
/// Must be called only when `updateSurfaceShellActivity` returned `true`.
481+
func recordShellActivity(
482+
workspaceId: UUID,
483+
panelId: UUID,
484+
state: Workspace.PanelShellActivityState
485+
) {
486+
let key = SocketSurfaceKey(workspaceId: workspaceId, panelId: panelId)
487+
queue.async {
488+
if self.lastReportedShellStates.count >= self.maxTrackedShellStates {
489+
self.lastReportedShellStates.removeAll(keepingCapacity: true)
473490
}
474-
lastReportedShellStates[key] = state
475-
return true
491+
self.lastReportedShellStates[key] = state
476492
}
477493
}
478494
}
@@ -15366,16 +15382,35 @@ class TerminalController {
1536615382
}
1536715383

1536815384
if let scope = Self.explicitSocketScope(options: parsed.options) {
15385+
// Fast-path check: skip dispatch if we already know this state is current.
15386+
// Note: we only READ here, not record. Recording happens after the update is
15387+
// confirmed applied on the main thread, preventing a race where the panel
15388+
// isn't registered yet and the dedup would suppress subsequent reports.
1536915389
guard Self.socketFastPathState.shouldPublishShellActivity(
1537015390
workspaceId: scope.workspaceId,
1537115391
panelId: scope.panelId,
1537215392
state: state
1537315393
) else {
1537415394
return "OK"
1537515395
}
15396+
let fastPathState = Self.socketFastPathState
1537615397
DispatchQueue.main.async {
1537715398
guard let tabManager = AppDelegate.shared?.tabManagerFor(tabId: scope.workspaceId) else { return }
15378-
tabManager.updateSurfaceShellActivity(tabId: scope.workspaceId, surfaceId: scope.panelId, state: state)
15399+
let applied = tabManager.updateSurfaceShellActivity(
15400+
tabId: scope.workspaceId,
15401+
surfaceId: scope.panelId,
15402+
state: state
15403+
)
15404+
// Only record the state in the dedup dict when the update actually
15405+
// applied (panel was registered). If the panel was absent, we do NOT
15406+
// record, so the next identical report will not be suppressed.
15407+
if applied {
15408+
fastPathState.recordShellActivity(
15409+
workspaceId: scope.workspaceId,
15410+
panelId: scope.panelId,
15411+
state: state
15412+
)
15413+
}
1537915414
}
1538015415
return "OK"
1538115416
}

Sources/Workspace.swift

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7644,17 +7644,26 @@ final class Workspace: Identifiable, ObservableObject {
76447644
}
76457645
}
76467646

7647-
func updatePanelShellActivityState(panelId: UUID, state: PanelShellActivityState) {
7648-
guard panels[panelId] != nil else { return }
7647+
/// Updates the shell-activity state for a panel.
7648+
///
7649+
/// - Returns: `true` if the update was applied (panel exists and state changed),
7650+
/// `false` if it was a no-op (panel absent or state unchanged).
7651+
/// Callers that deduplicate reports MUST only record the state in their dedup
7652+
/// dict when this returns `true`; recording on `false` would suppress the next
7653+
/// identical report even though it was never actually applied.
7654+
@discardableResult
7655+
func updatePanelShellActivityState(panelId: UUID, state: PanelShellActivityState) -> Bool {
7656+
guard panels[panelId] != nil else { return false }
76497657
let previousState = panelShellActivityStates[panelId] ?? .unknown
7650-
guard previousState != state else { return }
7658+
guard previousState != state else { return false }
76517659
panelShellActivityStates[panelId] = state
76527660
#if DEBUG
76537661
dlog(
76547662
"surface.shellState workspace=\(id.uuidString.prefix(5)) " +
76557663
"panel=\(panelId.uuidString.prefix(5)) from=\(previousState.rawValue) to=\(state.rawValue)"
76567664
)
76577665
#endif
7666+
return true
76587667
}
76597668

76607669
func panelNeedsConfirmClose(panelId: UUID, fallbackNeedsConfirmClose: Bool) -> Bool {

0 commit comments

Comments
 (0)