diff --git a/BTW_PERSISTENCE_AND_MEMORY_PLAN.md b/BTW_PERSISTENCE_AND_MEMORY_PLAN.md deleted file mode 100644 index 1c2ee60444..0000000000 --- a/BTW_PERSISTENCE_AND_MEMORY_PLAN.md +++ /dev/null @@ -1,261 +0,0 @@ -# /btw Persistence and Memory Plan - -## 1. Background - -`/btw` currently creates a transient child conversation. The frontend marks the -session as `isTransient: true` and `sessionKind: 'btw'`; the backend creates an -`EphemeralChild` session. `EphemeralChild` is intentionally excluded from -session persistence, so closing the desktop window makes an active `/btw` -conversation unavailable. - -This is unsuitable when a user turns a `/btw` discussion into a real task. The -conversation must remain available after restart, while still being presented -as a child of the parent conversation rather than as an unrelated root session. - -At the same time, a persistent `/btw` conversation must not silently become a -source of global memories. Its inclusion in the memory-generation lifecycle is -an explicit user configuration, disabled by default. - -## 2. Goals - -1. Persist every newly created `/btw` conversation and restore it after an app - restart. -2. Preserve the initial parent-context fork, model, mode, prompt-cache, and - constraints used by the current `/btw` flow. -3. Identify persistent BTW children structurally, so the UI can restore their - nested relationship with the parent. -4. Default persistent BTW children to excluded from memory generation, even - when normal-session memory generation is enabled. -5. Provide a Memory settings switch that lets a user opt new BTW conversations - into the ordinary Phase 1 and Phase 2 memory-generation lifecycle. - -## 3. Non-goals - -1. Do not change the behavior of existing ordinary sessions. -2. Do not migrate, repair, or reconstruct parent relationships for first-generation - BTW records. -3. Do not add a remote-workspace restriction to an existing remote `/btw` - workflow. Persistence must preserve its current remote behavior. -4. Do not couple "generate a new memory from this session" with "inject - existing global memory into this session". They are separate policies. - -## 4. Target Session Model - -New `/btw` sessions will be runtime `SessionKind::Standard` sessions with a -structured relationship: - -```text -session_kind: Standard -relationship: - kind: Btw - parent_session_id: - parent_request_id: - parent_dialog_turn_id: - parent_turn_index: -tags: ["btw"] -``` - -`Standard` is required for normal persistence, message updates, cancellation, -model changes, transcript reload, and memory-mode storage. `relationship.kind = -Btw` retains the product meaning that was previously inferred from the -ephemeral session kind and/or legacy metadata. - -The relationship is the source of truth. The `btw` tag is only a convenient -index/display hint and must not be used to classify structured and legacy BTW -sessions as equivalent. - -## 5. Creation and Runtime Flow - -The `/btw` command must continue to enter through the dedicated backend flow, -not through a generic frontend `createSession` followed by a normal send. The -current backend path forks the parent context before starting the first turn; -creating an empty persistent session in the frontend would lose that snapshot. - -The new backend flow is: - -1. Resolve the parent session and capture the same context snapshot currently - used by `ensure_hidden_btw_session` / `start_hidden_btw_turn`. -2. Create a persistent `Standard` child session. -3. Write `relationship.kind = Btw` and its parent linkage before the first - user turn is started. -4. Determine and persist the session's `memory_mode` as described in section - 7. -5. Persist the inherited context snapshot, then start the first BTW turn. -6. Route subsequent turns through the normal persistent-session send, cancel, - model-update, and persistence paths. - -The frontend API may retain BTW-specific names, but it must stop creating an -`isTransient` session. Its returned session identifier is a normal durable -session identifier. - -## 6. Restore and UI Behavior - -On startup, the session loader must retain structured BTW children and the -session navigation must group them under `relationship.parent_session_id`. - -- When the parent exists, opening the child restores it in the existing BTW - auxiliary panel. -- When the parent is missing or archived, opening the child falls back to an - independent session view rather than hiding an otherwise valid task. -- Remove the legacy BTW hiding predicate. First-generation records are not - recognized as BTW children and are not migrated; the ordinary session loader - may show them as independent root sessions. They have no supported parent - placement or auxiliary-panel behavior. - -## 7. Memory Policy - -### 7.1 Configuration - -Add this field to `MemoriesConfig`: - -```text -memories.generate_for_btw_sessions: false -``` - -It appears as a switch in the Memory settings page, adjacent to the global -"Generate memories" switch. The setting is effective only when the global -`memories.generate_memories` switch is also enabled. - -The field must be aligned in all configuration surfaces: - -1. Rust `MemoriesConfig`, serde default, and config persistence/default-pruning - tests. -2. Frontend `MemoriesConfig` type and config fallback/default handling. -3. Memory settings UI, locale strings, and settings search/index metadata. - -Because the Rust config is serde-defaulted, existing config files that lack the -new field resolve safely to `false`. - -### 7.2 Source Eligibility - -`SessionMetadata.memory_mode` is the durable source-eligibility contract. Phase -1 currently only extracts sessions that are both `SessionKind::Standard` and -`SessionMemoryMode::Enabled`. - -When a persistent BTW child is created, set its mode as follows: - -| Global `generate_memories` | `generate_for_btw_sessions` | BTW `memory_mode` | -| --- | --- | --- | -| false | false or true | `Disabled` | -| true | false | `Disabled` | -| true | true | `Enabled` | - -This decision is stored with the newly created session rather than only -skipping the BTW completion hook. Memory Phase 1 scans historical sessions each -time it starts; a startup-only guard would allow a disabled BTW to be extracted -later when an unrelated normal session starts a scan. - -The setting applies to conversations created after the setting is chosen. It -does not retrospectively enable previously excluded BTW transcripts. This -preserves the user's original choice not to contribute that task to global -memory. A future product request can add an explicit per-session migration or -selection action if retroactive enrollment is wanted. - -### 7.3 Existing-memory Injection - -`memories.use_memories` controls injection of the existing consolidated memory -summary into prompts. It is currently a global prompt-building decision and is -not session-kind aware. - -Recommended initial policy: a persistent BTW child continues to read existing -memory whenever `use_memories` is enabled. This is useful for a task -continuation and does not make the BTW transcript a new memory source. - -If product policy requires a fully isolated BTW environment, add a separate -future flag such as `memories.use_for_btw_sessions`, default `false`. Do not -reuse `generate_for_btw_sessions` for this purpose, because the two data flows -have different privacy and product semantics. - -## 8. Remote Workspace Support and Audit - -`/btw` already works in remote workspaces. The BTW child inherits the parent -`SessionConfig`, and each turn continues to pass the session's -`remote_connection_id` and `remote_ssh_host` into the normal dialog-turn path. -Remote sessions also have a dedicated local mirror storage path for their -session metadata and transcripts. - -The desktop command-policy registry currently marks `btw_ask_stream` and -`btw_cancel` as `LegacyUnaudited`. That is a classification backlog, not a -runtime rejection or a claim that remote BTW is unsupported. - -The persistence change must preserve this existing capability: - -1. Create the durable BTW child from the inherited remote-aware `SessionConfig`. -2. Resolve and store it through the same remote-session mirror path as the - parent, retaining remote connection identity across restart. -3. Add focused remote regression coverage for create, restart/restore, resume, - and cancel. -4. Audit the two desktop handlers with this flow and then promote - `btw_ask_stream` and `btw_cancel` to `RemoteRouted` in the command-policy - registry. - -No explicit unsupported state or remote-only feature gate is part of this plan. - -## 9. Implementation Areas - -| Area | Main files | Change | -| --- | --- | --- | -| Frontend BTW entry | `src/web-ui/src/flow_chat/services/BtwThreadService.ts` | Stop requesting a transient BTW session and consume the durable child result. | -| Desktop API | `src/apps/desktop/src/api/btw_api.rs` | Preserve the command contract while calling the persistent BTW coordinator path. | -| Coordinator | `src/crates/assembly/core/src/agentic/coordination/coordinator.rs` | Replace ephemeral BTW child construction with durable Standard creation, context fork, relationship, and memory-mode selection. | -| Session persistence | `src/crates/assembly/core/src/agentic/session/session_manager.rs` and `src/crates/assembly/core/src/agentic/persistence/manager.rs` | Persist and reload the structured BTW child without changing ordinary-session behavior. | -| Shared session contract | `src/crates/services/services-core/src/session/types.rs` and related metadata helpers | Reuse `SessionRelationshipKind::Btw`; do not add a second BTW classification format. | -| Session navigation | `src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx` | Restore/group structured BTW children; use orphan fallback. | -| Frontend metadata parsing and loading | `src/web-ui/src/flow_chat/utils/sessionMetadata.ts` and `src/web-ui/src/flow_chat/store/FlowChatStore.ts` | Use `relationship.kind = Btw` as the only BTW classifier; remove `isLegacyPersistedBtwSession` and both metadata-load skip branches. | -| Memory configuration | `src/crates/assembly/core/src/service/config/types.rs`, config manager, frontend config types, and `MemoriesConfig.tsx` | Add the default-off BTW source-generation switch and UI/i18n plumbing. | -| Memory source selection | `src/crates/assembly/core/src/agentic/memories/service.rs` | Continue honoring durable `memory_mode`; add an explicit test proving an excluded BTW is not claimed during a later ordinary scan. | -| Remote BTW audit | `src/apps/desktop/src/api/remote_workspace_policy.rs`, BTW API, and remote-session tests | Preserve inherited remote identity, verify the persistent flow, then promote the BTW commands to `RemoteRouted`. | - -## 10. Test Plan - -### Rust - -1. Config defaults and deserialization: missing - `generate_for_btw_sessions` resolves to `false`; non-default values persist - and reload correctly. -2. BTW creation: durable child is `Standard`, carries `Btw` relationship and - parent linkage, and retains the parent-context snapshot. -3. Memory mode matrix: verify all three effective cases in section 7.2. -4. Restart/reload: child transcript and relationship survive persistence. -5. Phase 1 candidate selection: a BTW created while the new flag is off is - never claimed, including during a memory run started by another session. -6. Remote persistence: create a BTW in a remote workspace, restart and restore - it, resume/cancel it, and confirm that its remote identity and mirror - storage remain intact. Promote the two BTW commands to `RemoteRouted` after - that audit passes. - -### Frontend - -1. New BTW session is not transient and can send further messages through the - normal session path. -2. Structured BTW child restores beneath its parent and opens in the auxiliary - panel. -3. Orphaned structured BTW opens independently. -4. Memory settings switch reads, writes, and renders its default-off state. - -### Commands after implementation - -Run the narrow tests that cover the changed Rust modules and Web UI behavior, -then at minimum run: - -```text -pnpm run type-check:web -cargo check --workspace -``` - -Add the focused frontend and Rust test commands to the implementation handoff -only after their exact test locations are finalized. - -## 11. Rollout and Compatibility - -The feature is forward-only. New BTW sessions use structured relationships and -durable session storage; historical transient BTW sessions do not have data to -restore. First-generation persisted BTW-shaped records receive no compatibility -handling: the implementation removes their legacy hide/recognition branches and -does not migrate their tag/custom-metadata relationship into the new structured -format. Any resulting ordinary-session loading is incidental and has no -compatibility test, parent placement, or BTW auxiliary-panel guarantee. - -The memory setting defaults to closed for both new users and existing config -files. This permits persistent BTW task recovery without widening the set of -transcripts that can contribute to global memory. diff --git a/scripts/core-boundaries/rules/source/required-rules.mjs b/scripts/core-boundaries/rules/source/required-rules.mjs index cd3425efae..c16640f639 100644 --- a/scripts/core-boundaries/rules/source/required-rules.mjs +++ b/scripts/core-boundaries/rules/source/required-rules.mjs @@ -3961,8 +3961,9 @@ export const requiredContentRules = [ 'workspace metadata may omit git worktree enrichment when service integrations are disabled', patterns: [ { - regex: /#\[cfg\(feature = "service-integrations"\)\]\s*use crate::service::git::GitService\b/s, - message: 'GitService import must stay gated for no-default builds', + regex: + /#\[cfg\(feature = "service-integrations"\)\]\s*use super::worktree_topology::global_worktree_topology_service\b/s, + message: 'worktree topology owner import must stay gated for no-default builds', }, { regex: /#\[cfg\(not\(feature = "service-integrations"\)\)\]\s*\{\s*let _ = workspace_root;\s*return None;\s*\}/s, diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index cd9b259d26..2765725e56 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -3763,7 +3763,11 @@ export function runManifestParserSelfTest({ }, { path: 'src/crates/assembly/core/src/service/workspace/manager.rs', - contracts: ['feature = "service-integrations"', 'GitService', 'return None'], + contracts: [ + 'feature = "service-integrations"', + 'global_worktree_topology_service', + 'return None', + ], }, { path: 'src/crates/assembly/core/src/service/workspace_runtime/service.rs', diff --git a/scripts/diagnostics/probe-bitfun-process-events.ps1 b/scripts/diagnostics/probe-bitfun-process-events.ps1 new file mode 100644 index 0000000000..d438bdbcb8 --- /dev/null +++ b/scripts/diagnostics/probe-bitfun-process-events.ps1 @@ -0,0 +1,370 @@ +# Event-based BitFun process-tree probe for Windows. +# +# This probe subscribes to Win32 process start/stop events instead of polling a +# fixed list of process names. It records every process event seen while it is +# active, then attributes each process to the BitFun root using a parent table +# that survives parent-process exit. +# +# Run from a separate PowerShell window. Run as Administrator when possible so +# command lines and executable paths are available for all processes. +# +# Examples: +# .\probe-bitfun-process-events.ps1 -DurationSec 60 -OutputPath .\bitfun-process-events.log +# .\probe-bitfun-process-events.ps1 -BitFunPid 8352 -DurationSec 120 + +[CmdletBinding()] +param( + [ValidateRange(1, 3600)] + [int]$DurationSec = 60, + + [int[]]$BitFunPid = @(), + + [string]$OutputPath +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +function Get-BitFunProcessIds { + @( + Get-CimInstance Win32_Process | + Where-Object { $_.Name -ieq 'bitfun-desktop.exe' } | + ForEach-Object { [int]$_.ProcessId } + ) +} + +function New-ProcessSnapshot { + param( + [Parameter(Mandatory)] + [int]$ProcessId, + + [int]$ParentProcessId, + + [Parameter(Mandatory)] + [string]$Name, + + [string]$ExecutablePath, + + [string]$CommandLine + ) + + [PSCustomObject]@{ + ProcessId = $ProcessId + ParentProcessId = $ParentProcessId + Name = $Name + ExecutablePath = $ExecutablePath + CommandLine = $CommandLine + } +} + +function Format-LogValue { + param( + [AllowNull()] + [object]$Value + ) + + if ($null -eq $Value) { + return '-' + } + + $text = [string]$Value + $text = $text.Replace("`r", '\r').Replace("`n", '\n').Replace("`t", '\t').Replace('"', '\"') + '"' + $text + '"' +} + +$rootProcessIds = if (@($BitFunPid).Count -gt 0) { + @($BitFunPid | ForEach-Object { [int]$_ } | Select-Object -Unique) +} else { + @(Get-BitFunProcessIds) +} + +if (@($rootProcessIds).Count -eq 0) { + throw 'No bitfun-desktop.exe process was found. Pass -BitFunPid explicitly or start BitFun first.' +} + +$probeProcessId = $PID +$probeStartedAt = (Get-Date).ToString('o') +$isElevated = try { + $principal = [Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent() + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) +} catch { + $false +} +$knownProcesses = [hashtable]::Synchronized(@{}) +$records = [System.Collections.Concurrent.ConcurrentQueue[object]]::new() +$callbackErrors = [System.Collections.Concurrent.ConcurrentQueue[object]]::new() + +# Seed the parent table so processes that already exist when the probe starts +# can still participate in lineage attribution for later descendants. +Get-CimInstance Win32_Process | ForEach-Object { + $knownProcesses[[int]$_.ProcessId] = New-ProcessSnapshot ` + -ProcessId ([int]$_.ProcessId) ` + -ParentProcessId ([int]$_.ParentProcessId) ` + -Name ([string]$_.Name) ` + -ExecutablePath ([string]$_.ExecutablePath) ` + -CommandLine ([string]$_.CommandLine) +} + +$state = [hashtable]::Synchronized(@{ + RootProcessIds = $rootProcessIds + ProbeProcessId = $probeProcessId + KnownProcesses = $knownProcesses + Records = $records + CallbackErrors = $callbackErrors +}) + +$startQuery = New-Object System.Management.WqlEventQuery +$startQuery.QueryString = 'SELECT * FROM Win32_ProcessStartTrace' +$startWatcher = New-Object System.Management.ManagementEventWatcher($startQuery) +$stopWatcher = $null +$startSubscription = $null +$stopSubscription = $null + +try { + $startSubscription = Register-ObjectEvent ` + -InputObject $startWatcher ` + -EventName EventArrived ` + -MessageData $state ` + -Action { + try { + $state = $event.MessageData + $eventData = $eventArgs.NewEvent + $processId = [int]$eventData.ProcessID + if ($processId -eq [int]$state.ProbeProcessId) { + return + } + + $detail = Get-CimInstance Win32_Process -Filter "ProcessId = $processId" -ErrorAction SilentlyContinue + $snapshot = if ($detail) { + [PSCustomObject]@{ + ProcessId = [int]$detail.ProcessId + ParentProcessId = [int]$detail.ParentProcessId + Name = [string]$detail.Name + ExecutablePath = [string]$detail.ExecutablePath + CommandLine = [string]$detail.CommandLine + } + } else { + [PSCustomObject]@{ + ProcessId = $processId + ParentProcessId = [int]$eventData.ParentProcessID + Name = [string]$eventData.ProcessName + ExecutablePath = $null + CommandLine = $null + } + } + $state.KnownProcesses[$processId] = $snapshot + + $lineage = [System.Collections.Generic.List[string]]::new() + $currentPid = [int]$snapshot.ParentProcessId + $isBitFunDescendant = $false + $isComplete = $true + for ($depth = 0; $depth -lt 32 -and $currentPid -gt 0; $depth++) { + if ($state.RootProcessIds -contains $currentPid) { + $isBitFunDescendant = $true + break + } + if (-not $state.KnownProcesses.Contains($currentPid)) { + $isComplete = $false + break + } + $current = $state.KnownProcesses[$currentPid] + $lineage.Add("$($current.Name)#$($current.ProcessId)") + $currentPid = [int]$current.ParentProcessId + } + if ($currentPid -eq 0) { + $isComplete = $false + } + + $state.Records.Enqueue([PSCustomObject]@{ + EventType = 'start' + ObservedAt = (Get-Date).ToString('o') + ProcessName = $snapshot.Name + ProcessId = $snapshot.ProcessId + ParentProcessId = $snapshot.ParentProcessId + ExecutablePath = $snapshot.ExecutablePath + CommandLine = $snapshot.CommandLine + IsBitFunDescendant = $isBitFunDescendant + AttributionStatus = if ($isBitFunDescendant) { + if ($state.RootProcessIds -contains $snapshot.ParentProcessId) { 'direct' } else { 'descendant' } + } elseif ($isComplete) { 'not_bitfun' } else { 'unknown' } + LineageComplete = $isComplete + Lineage = $lineage.ToArray() + }) + } + catch { + $event.MessageData.CallbackErrors.Enqueue([PSCustomObject]@{ + EventType = 'start' + ObservedAt = (Get-Date).ToString('o') + Message = $_.Exception.Message + }) + } + } + + $stopQuery = New-Object System.Management.WqlEventQuery + $stopQuery.QueryString = 'SELECT * FROM Win32_ProcessStopTrace' + $stopWatcher = New-Object System.Management.ManagementEventWatcher($stopQuery) + $stopSubscription = Register-ObjectEvent ` + -InputObject $stopWatcher ` + -EventName EventArrived ` + -MessageData $state ` + -Action { + try { + $state = $event.MessageData + $eventData = $eventArgs.NewEvent + $processId = [int]$eventData.ProcessID + if ($processId -eq [int]$state.ProbeProcessId) { + return + } + + $known = $state.KnownProcesses[$processId] + $processName = if ($known) { $known.Name } else { [string]$eventData.ProcessName } + $parentProcessId = if ($known) { + [int]$known.ParentProcessId + } elseif ($eventData.PSObject.Properties['ParentProcessID']) { + [int]$eventData.ParentProcessID + } else { + 0 + } + + $lineage = [System.Collections.Generic.List[string]]::new() + $currentPid = $parentProcessId + $isBitFunDescendant = $false + $isComplete = $true + for ($depth = 0; $depth -lt 32 -and $currentPid -gt 0; $depth++) { + if ($state.RootProcessIds -contains $currentPid) { + $isBitFunDescendant = $true + break + } + if (-not $state.KnownProcesses.Contains($currentPid)) { + $isComplete = $false + break + } + $current = $state.KnownProcesses[$currentPid] + $lineage.Add("$($current.Name)#$($current.ProcessId)") + $currentPid = [int]$current.ParentProcessId + } + if ($currentPid -eq 0) { + $isComplete = $false + } + + $state.Records.Enqueue([PSCustomObject]@{ + EventType = 'stop' + ObservedAt = (Get-Date).ToString('o') + ProcessName = $processName + ProcessId = $processId + ParentProcessId = $parentProcessId + ExecutablePath = if ($known) { $known.ExecutablePath } else { $null } + CommandLine = if ($known) { $known.CommandLine } else { $null } + IsBitFunDescendant = $isBitFunDescendant + AttributionStatus = if ($isBitFunDescendant) { 'descendant' } elseif ($isComplete) { 'not_bitfun' } else { 'unknown' } + LineageComplete = $isComplete + Lineage = $lineage.ToArray() + }) + } + catch { + $event.MessageData.CallbackErrors.Enqueue([PSCustomObject]@{ + EventType = 'stop' + ObservedAt = (Get-Date).ToString('o') + Message = $_.Exception.Message + }) + } + } +} +catch { + $message = $_.Exception.Message + if ($null -ne $startSubscription) { + Unregister-Event -SubscriptionId $startSubscription.Id -ErrorAction SilentlyContinue + } + if ($null -ne $stopSubscription) { + Unregister-Event -SubscriptionId $stopSubscription.Id -ErrorAction SilentlyContinue + } + $startWatcher.Dispose() + if ($null -ne $stopWatcher) { + $stopWatcher.Dispose() + } + if ($message -match 'Access denied') { + throw 'Process event subscription was denied. Run this script from an elevated Administrator PowerShell window.' + } + throw +} + +try { + $startWatcher.Start() + $stopWatcher.Start() + $deadline = [DateTime]::UtcNow.AddSeconds($DurationSec) + while ([DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 100 + } +} +finally { + $startWatcher.Stop() + $stopWatcher.Stop() + Start-Sleep -Milliseconds 250 + Unregister-Event -SubscriptionId $startSubscription.Id -ErrorAction SilentlyContinue + Unregister-Event -SubscriptionId $stopSubscription.Id -ErrorAction SilentlyContinue + $startWatcher.Dispose() + $stopWatcher.Dispose() +} + +$recordsArray = @($records.ToArray() | Sort-Object ObservedAt) +$callbackErrorsArray = @($callbackErrors.ToArray() | Sort-Object ObservedAt) +$probeFinishedAt = (Get-Date).ToString('o') + +$lines = [System.Collections.Generic.List[string]]::new() +$lines.Add('BitFun process event probe') +$lines.Add("probe_started_at=$(Format-LogValue $probeStartedAt)") +$lines.Add("probe_finished_at=$(Format-LogValue $probeFinishedAt)") +$lines.Add("probe_pid=$probeProcessId") +$lines.Add("bitfun_pids=$(Format-LogValue (($rootProcessIds | ForEach-Object { [string]$_ }) -join ','))") +$lines.Add("duration_sec=$DurationSec") +$lines.Add("elevated=$isElevated") +$lines.Add("powershell_version=$(Format-LogValue $PSVersionTable.PSVersion.ToString())") +$lines.Add('event_source="Win32_ProcessStartTrace/Win32_ProcessStopTrace"') +$lines.Add("record_count=$(@($recordsArray).Count)") +$lines.Add("callback_error_count=$(@($callbackErrorsArray).Count)") +$lines.Add('') +$lines.Add('[callback_errors]') +if (@($callbackErrorsArray).Count -eq 0) { + $lines.Add('none') +} else { + foreach ($callbackError in $callbackErrorsArray) { + $lines.Add( + "observed_at=$(Format-LogValue $callbackError.ObservedAt) " + + "event=$($callbackError.EventType) " + + "message=$(Format-LogValue $callbackError.Message)" + ) + } +} +$lines.Add('') +$lines.Add('[process_events]') +if (@($recordsArray).Count -eq 0) { + $lines.Add('none') +} else { + foreach ($record in $recordsArray) { + $lineage = if (@($record.Lineage).Count -gt 0) { + @($record.Lineage) -join ' > ' + } else { + $null + } + $lines.Add( + "observed_at=$(Format-LogValue $record.ObservedAt) " + + "event=$($record.EventType) " + + "name=$(Format-LogValue $record.ProcessName) " + + "pid=$($record.ProcessId) " + + "ppid=$($record.ParentProcessId) " + + "attribution=$($record.AttributionStatus) " + + "is_bitfun=$($record.IsBitFunDescendant) " + + "lineage_complete=$($record.LineageComplete) " + + "path=$(Format-LogValue $record.ExecutablePath) " + + "command_line=$(Format-LogValue $record.CommandLine) " + + "lineage=$(Format-LogValue $lineage)" + ) + } +} + +$textOutput = $lines -join [Environment]::NewLine +if ($OutputPath) { + $textOutput | Set-Content -LiteralPath $OutputPath -Encoding utf8 +} + +$textOutput diff --git a/src/apps/desktop/src/api/git_api.rs b/src/apps/desktop/src/api/git_api.rs index 90169dacc3..d98813fda0 100644 --- a/src/apps/desktop/src/api/git_api.rs +++ b/src/apps/desktop/src/api/git_api.rs @@ -15,6 +15,7 @@ use bitfun_core::service::remote_ssh::{ build_remote_git_command as build_remote_git_command_shared, lookup_remote_connection, normalize_remote_workspace_path, }; +use bitfun_core::service::workspace::WorktreeTopologyFreshness; use log::{error, info}; use serde::{Deserialize, Serialize}; use std::time::Instant; @@ -1260,7 +1261,7 @@ pub async fn git_cherry_pick_continue( #[tauri::command] pub async fn git_list_worktrees( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: GitRepositoryRequest, ) -> Result, String> { info!("Listing worktrees for '{}'", request.repository_path); @@ -1272,7 +1273,12 @@ pub async fn git_list_worktrees( return Err("Git worktrees are not supported for remote SSH workspaces yet".to_string()); } - GitService::list_worktrees(&request.repository_path) + state + .workspace_service + .list_worktrees( + std::path::Path::new(&request.repository_path), + WorktreeTopologyFreshness::ForceRefresh, + ) .await .map_err(|e| { error!( @@ -1285,7 +1291,7 @@ pub async fn git_list_worktrees( #[tauri::command] pub async fn git_add_worktree( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: GitAddWorktreeRequest, ) -> Result { let create_branch = request.create_branch.unwrap_or(false); @@ -1301,20 +1307,26 @@ pub async fn git_add_worktree( return Err("Git worktrees are not supported for remote SSH workspaces yet".to_string()); } - GitService::add_worktree(&request.repository_path, &request.branch, create_branch) - .await - .map_err(|e| { - error!( - "Failed to add worktree: path={}, branch={}, create_branch={}, error={}", - request.repository_path, request.branch, create_branch, e - ); - format!("Failed to add worktree: {}", e) - }) + let worktree = + GitService::add_worktree(&request.repository_path, &request.branch, create_branch) + .await + .map_err(|e| { + error!( + "Failed to add worktree: path={}, branch={}, create_branch={}, error={}", + request.repository_path, request.branch, create_branch, e + ); + format!("Failed to add worktree: {}", e) + })?; + state + .workspace_service + .invalidate_worktree_topology(std::path::Path::new(&request.repository_path)) + .await; + Ok(worktree) } #[tauri::command] pub async fn git_remove_worktree( - _state: State<'_, AppState>, + state: State<'_, AppState>, request: GitRemoveWorktreeRequest, ) -> Result { let force = request.force.unwrap_or(false); @@ -1330,15 +1342,21 @@ pub async fn git_remove_worktree( return Err("Git worktrees are not supported for remote SSH workspaces yet".to_string()); } - GitService::remove_worktree(&request.repository_path, &request.worktree_path, force) - .await - .map_err(|e| { - error!( - "Failed to remove worktree: path={}, worktree_path={}, force={}, error={}", - request.repository_path, request.worktree_path, force, e - ); - format!("Failed to remove worktree: {}", e) - }) + let result = + GitService::remove_worktree(&request.repository_path, &request.worktree_path, force) + .await + .map_err(|e| { + error!( + "Failed to remove worktree: path={}, worktree_path={}, force={}, error={}", + request.repository_path, request.worktree_path, force, e + ); + format!("Failed to remove worktree: {}", e) + })?; + state + .workspace_service + .invalidate_worktree_topology(std::path::Path::new(&request.repository_path)) + .await; + Ok(result) } // MARK: Git Repo History diff --git a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs index cf4fa967c0..018e56bd4c 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs @@ -61,7 +61,7 @@ use crate::service::session::{ SessionMemoryMode, SessionRelationship, SessionRelationshipKind, SessionStatus, }; use crate::service::workspace::{ - get_global_workspace_service, WorkspaceCreateOptions, WorkspaceKind, + get_global_workspace_service, WorkspaceActivityMode, WorkspaceCreateOptions, WorkspaceKind, }; use crate::service_agent_runtime::CoreServiceAgentRuntime; use crate::util::errors::{BitFunError, BitFunResult}; @@ -917,7 +917,11 @@ impl ConversationCoordinator { .map(|workspace| workspace.id) } - async fn track_session_workspace_activity_best_effort(config: &SessionConfig, reason: &str) { + async fn track_session_workspace_activity_best_effort( + config: &SessionConfig, + mode: WorkspaceActivityMode, + reason: &str, + ) { let Some(workspace_path) = config.workspace_path.as_ref() else { return; }; @@ -939,7 +943,7 @@ impl ConversationCoordinator { } if let Err(error) = workspace_service - .track_workspace_activity(PathBuf::from(workspace_path), options) + .track_workspace_activity(PathBuf::from(workspace_path), options, mode) .await { warn!( @@ -1784,8 +1788,12 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet }; if !transient { - Self::track_session_workspace_activity_best_effort(&session.config, "session_created") - .await; + Self::track_session_workspace_activity_best_effort( + &session.config, + WorkspaceActivityMode::RefreshMetadata, + "session_created", + ) + .await; } // SessionManager::create_session_with_id_and_creator already persists the @@ -3548,7 +3556,12 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet }; let effective_agent_type = Self::normalize_agent_type(&provisional_agent_type); - Self::track_session_workspace_activity_best_effort(&session.config, "dialog_started").await; + Self::track_session_workspace_activity_best_effort( + &session.config, + WorkspaceActivityMode::TouchOnly, + "dialog_started", + ) + .await; debug!( "Resolved dialog turn agent type: session_id={}, turn_id={}, requested_agent_type={}, session_agent_type={}, effective_agent_type={}, trigger_source={:?}, queue_priority={:?}", diff --git a/src/crates/assembly/core/src/external_hooks.rs b/src/crates/assembly/core/src/external_hooks.rs index dbbde8778c..a70dd61be2 100644 --- a/src/crates/assembly/core/src/external_hooks.rs +++ b/src/crates/assembly/core/src/external_hooks.rs @@ -12,6 +12,8 @@ pub use bitfun_product_domains::external_hook_catalog::{ pub use bitfun_product_domains::external_sources::{ExecutionDomainId, ExternalSourceContext}; use crate::external_sources::{host_execution_domain_id, normalize_workspace_root}; +#[cfg(feature = "service-integrations")] +use crate::service::workspace::{global_worktree_topology_service, WorktreeTopologyFreshness}; use bitfun_claude_code_adapter::{ClaudeCodeHookProvider, ClaudeCodeHookProviderOptions}; use bitfun_codex_adapter::{CodexHookProvider, CodexHookProviderOptions}; use bitfun_external_sources::ExternalHookCatalogCoordinator; @@ -20,8 +22,6 @@ use bitfun_product_domains::external_hook_catalog::ExternalHookSourceProvider; use bitfun_product_domains::external_sources::{ ExternalSourceOperationError, ExternalSourceOperationErrorCode, ExternalSourceOperationResult, }; -#[cfg(feature = "service-integrations")] -use bitfun_services_integrations::git::GitService; use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::atomic::{AtomicU64, Ordering}; @@ -248,7 +248,10 @@ async fn hook_project_topology( workspace_root: Option<&std::path::Path>, ) -> Option { let workspace_root = workspace_root?; - let worktrees = GitService::list_worktrees(workspace_root).await.ok()?; + let worktrees = global_worktree_topology_service() + .list_worktrees(workspace_root, WorktreeTopologyFreshness::Cached) + .await + .ok()?; resolve_hook_project_topology( workspace_root, &worktrees diff --git a/src/crates/assembly/core/src/external_sources.rs b/src/crates/assembly/core/src/external_sources.rs index cd2cc98b98..4ee464e840 100644 --- a/src/crates/assembly/core/src/external_sources.rs +++ b/src/crates/assembly/core/src/external_sources.rs @@ -46,10 +46,10 @@ use crate::external_subagents::{ }; use crate::external_tools::{ begin_external_tool_workspace_recovery, external_tool_workspace_requires_recovery, - merge_tool_state, project_external_tools_read_only, reconcile_external_tools, - release_external_tool_workspace, reset_external_tool_workspace_recovery_budget, - workspace_route_key, ExternalToolDecisions, ExternalToolProductState, - TOOL_CONFLICT_RESELECTION_REQUIRED, UNRESOLVED_TOOL_CONFLICT_CHOICE, + invalidate_external_tool_runtime_availability, merge_tool_state, + project_external_tools_read_only, reconcile_external_tools, release_external_tool_workspace, + reset_external_tool_workspace_recovery_budget, workspace_route_key, ExternalToolDecisions, + ExternalToolProductState, TOOL_CONFLICT_RESELECTION_REQUIRED, UNRESOLVED_TOOL_CONFLICT_CHOICE, }; use crate::service::config::{subscribe_config_updates, ConfigUpdateEvent}; use bitfun_claude_code_adapter::{ @@ -928,6 +928,15 @@ impl WorkspaceExternalSourceService { .await } + async fn refresh_with_runtime_invalidation( + self: &Arc, + ) -> Result { + if self.profile == ExternalSourceServiceProfile::LocalExecution { + invalidate_external_tool_runtime_availability().await; + } + self.refresh().await + } + async fn refresh_preserving_worker_recovery( self: &Arc, ) -> Result { @@ -2140,7 +2149,7 @@ impl WorkspaceExternalSourceService { ExternalSourceControlActionV1::Refresh => ( "refresh", ExternalSourceOperationStage::Discover, - self.refresh().await, + self.refresh_with_runtime_invalidation().await, ), ExternalSourceControlActionV1::SetSourceEnabled { source_key, @@ -5285,7 +5294,7 @@ pub async fn external_source_snapshot( ) -> Result { let service = service_for(workspace_root).await?; if force_refresh { - service.refresh().await + service.refresh_with_runtime_invalidation().await } else { service.ensure_background_refresh(); Ok(service.snapshot()) @@ -5320,10 +5329,13 @@ pub async fn get_external_source_control_snapshot( .with_stage(ExternalSourceOperationStage::ProjectResponse) })?; if force_refresh { - service.refresh().await.map_err(|error| { - sanitize_external_source_operation_error(error) - .with_stage(ExternalSourceOperationStage::Discover) - })?; + service + .refresh_with_runtime_invalidation() + .await + .map_err(|error| { + sanitize_external_source_operation_error(error) + .with_stage(ExternalSourceOperationStage::Discover) + })?; } else { service.ensure_background_refresh(); } diff --git a/src/crates/assembly/core/src/external_tools.rs b/src/crates/assembly/core/src/external_tools.rs index 9a5b68dea2..797db4a45e 100644 --- a/src/crates/assembly/core/src/external_tools.rs +++ b/src/crates/assembly/core/src/external_tools.rs @@ -991,6 +991,10 @@ impl ExternalToolRuntimeManager { self.runtime.availability().await } + async fn invalidate_availability(&self) { + self.runtime.invalidate_availability().await; + } + async fn mark_worker_lost( &self, workspace_key: &str, @@ -1303,6 +1307,10 @@ pub(super) async fn reset_external_tool_workspace_recovery_budget(workspace_root .await; } +pub(super) async fn invalidate_external_tool_runtime_availability() { + runtime_manager().invalidate_availability().await; +} + pub(super) async fn release_external_tool_workspace(workspace_root: Option<&Path>) { let workspace_key = workspace_route_key(workspace_root); router().apply_routes(&workspace_key, BTreeMap::new()).await; diff --git a/src/crates/assembly/core/src/service/workspace/manager.rs b/src/crates/assembly/core/src/service/workspace/manager.rs index 4c9e3b3192..f806b55396 100644 --- a/src/crates/assembly/core/src/service/workspace/manager.rs +++ b/src/crates/assembly/core/src/service/workspace/manager.rs @@ -1,7 +1,8 @@ //! Workspace manager. #[cfg(feature = "service-integrations")] -use crate::service::git::GitService; +use super::worktree_topology::global_worktree_topology_service; +use super::WorktreeTopologyFreshness; use crate::service::remote_ssh::workspace_state::{ canonicalize_local_workspace_root, local_workspace_roots_equal, local_workspace_stable_storage_id, normalize_local_workspace_root_for_stable_id, @@ -304,6 +305,21 @@ impl WorkspaceInfo { /// Creates a new workspace record. pub async fn new(root_path: PathBuf, options: WorkspaceOpenOptions) -> BitFunResult { + Self::new_inner(root_path, options, true).await + } + + pub(crate) async fn new_without_worktree( + root_path: PathBuf, + options: WorkspaceOpenOptions, + ) -> BitFunResult { + Self::new_inner(root_path, options, false).await + } + + async fn new_inner( + root_path: PathBuf, + options: WorkspaceOpenOptions, + load_worktree: bool, + ) -> BitFunResult { let default_name = root_path .file_name() .and_then(|n| n.to_str()) @@ -381,7 +397,11 @@ impl WorkspaceInfo { ); workspace.detect_workspace_type().await; workspace.load_identity().await; - workspace.load_worktree().await; + if load_worktree { + workspace + .load_worktree(WorktreeTopologyFreshness::Cached) + .await; + } if options.scan_options.calculate_statistics { workspace.scan_workspace(options.scan_options).await?; @@ -422,11 +442,14 @@ impl WorkspaceInfo { self.identity = identity; } - async fn load_worktree(&mut self) { - self.worktree = Self::resolve_worktree_info(&self.root_path).await; + async fn load_worktree(&mut self, freshness: WorktreeTopologyFreshness) { + self.worktree = Self::resolve_worktree_info(&self.root_path, freshness).await; } - async fn resolve_worktree_info(workspace_root: &Path) -> Option { + pub(crate) async fn resolve_worktree_info( + workspace_root: &Path, + freshness: WorktreeTopologyFreshness, + ) -> Option { #[cfg(not(feature = "service-integrations"))] { let _ = workspace_root; @@ -436,7 +459,10 @@ impl WorkspaceInfo { #[cfg(feature = "service-integrations")] { let normalized_workspace_path = workspace_root.to_string_lossy().replace('\\', "/"); - let worktrees = match GitService::list_worktrees(workspace_root).await { + let worktrees = match global_worktree_topology_service() + .list_worktrees(workspace_root, freshness) + .await + { Ok(worktrees) => worktrees, Err(_) => return None, }; @@ -886,7 +912,19 @@ impl WorkspaceManager { path: PathBuf, options: WorkspaceOpenOptions, ) -> BitFunResult { - self.upsert_workspace_with_options(path, options, true) + let worktree = + WorkspaceInfo::resolve_worktree_info(&path, WorktreeTopologyFreshness::Cached).await; + self.open_workspace_with_resolved_worktree(path, options, worktree) + .await + } + + pub(crate) async fn open_workspace_with_resolved_worktree( + &mut self, + path: PathBuf, + options: WorkspaceOpenOptions, + worktree: Option, + ) -> BitFunResult { + self.upsert_workspace_with_options(path, options, true, Some(worktree)) .await } @@ -895,8 +933,9 @@ impl WorkspaceManager { &mut self, path: PathBuf, options: WorkspaceOpenOptions, + refresh_worktree: Option>, ) -> BitFunResult { - self.upsert_workspace_with_options(path, options, false) + self.upsert_workspace_with_options(path, options, false, refresh_worktree) .await } @@ -905,6 +944,7 @@ impl WorkspaceManager { path: PathBuf, options: WorkspaceOpenOptions, keep_opened: bool, + refresh_worktree: Option>, ) -> BitFunResult { let is_remote = options.workspace_kind == WorkspaceKind::Remote; @@ -1064,8 +1104,10 @@ impl WorkspaceManager { ); } } - workspace.load_identity().await; - workspace.load_worktree().await; + if let Some(worktree) = refresh_worktree { + workspace.load_identity().await; + workspace.worktree = worktree; + } } if keep_opened { self.ensure_workspace_open(&workspace_id); @@ -1086,7 +1128,15 @@ impl WorkspaceManager { }); } - let workspace = WorkspaceInfo::new(path, options.clone()).await?; + let workspace = match refresh_worktree { + Some(worktree) => { + let mut workspace = + WorkspaceInfo::new_without_worktree(path, options.clone()).await?; + workspace.worktree = worktree; + workspace + } + None => WorkspaceInfo::new(path, options.clone()).await?, + }; let workspace_id = workspace.id.clone(); self.workspaces diff --git a/src/crates/assembly/core/src/service/workspace/mod.rs b/src/crates/assembly/core/src/service/workspace/mod.rs index 2372774f92..1d5a63f417 100644 --- a/src/crates/assembly/core/src/service/workspace/mod.rs +++ b/src/crates/assembly/core/src/service/workspace/mod.rs @@ -7,6 +7,14 @@ pub mod identity_watch; pub mod manager; pub mod provider; pub mod service; +#[cfg(feature = "service-integrations")] +pub mod worktree_topology; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WorktreeTopologyFreshness { + Cached, + ForceRefresh, +} // Re-export main components pub use factory::WorkspaceFactory; @@ -14,12 +22,14 @@ pub use identity_watch::WorkspaceIdentityWatchService; pub use manager::{ GitInfo, RelatedPath, ScanOptions, WorkspaceIdentity, WorkspaceInfo, WorkspaceKind, WorkspaceManager, WorkspaceManagerConfig, WorkspaceManagerStatistics, WorkspaceOpenOptions, - WorkspaceStatistics, WorkspaceStatus, WorkspaceSummary, WorkspaceType, + WorkspaceStatistics, WorkspaceStatus, WorkspaceSummary, WorkspaceType, WorkspaceWorktreeInfo, }; pub use provider::{WorkspaceCleanupResult, WorkspaceProvider, WorkspaceSystemSummary}; pub use service::{ get_global_workspace_service, set_global_workspace_service, BatchImportResult, - BatchRemoveResult, WorkspaceCreateOptions, WorkspaceExport, WorkspaceHealthStatus, - WorkspaceIdentityChangedEvent, WorkspaceImportResult, WorkspaceInfoUpdates, - WorkspaceQuickSummary, WorkspaceService, + BatchRemoveResult, WorkspaceActivityMode, WorkspaceCreateOptions, WorkspaceExport, + WorkspaceHealthStatus, WorkspaceIdentityChangedEvent, WorkspaceImportResult, + WorkspaceInfoUpdates, WorkspaceQuickSummary, WorkspaceService, }; +#[cfg(feature = "service-integrations")] +pub use worktree_topology::{global_worktree_topology_service, WorktreeTopologyService}; diff --git a/src/crates/assembly/core/src/service/workspace/service.rs b/src/crates/assembly/core/src/service/workspace/service.rs index c4efdf314b..980365bf01 100644 --- a/src/crates/assembly/core/src/service/workspace/service.rs +++ b/src/crates/assembly/core/src/service/workspace/service.rs @@ -7,11 +7,14 @@ use super::manager::{ WorkspaceManagerConfig, WorkspaceManagerStatistics, WorkspaceOpenOptions, WorkspaceStatus, WorkspaceSummary, WorkspaceType, }; +use super::WorktreeTopologyFreshness; use crate::infrastructure::storage::{PersistenceService, StorageOptions}; use crate::infrastructure::{try_get_path_manager_arc, PathManager}; use crate::service::bootstrap::{ ensure_workspace_gitignore_ignores_bitfun, initialize_workspace_persona_files, }; +#[cfg(feature = "service-integrations")] +use crate::service::git::{GitError, GitWorktreeInfo}; use crate::service::remote_ssh::workspace_state::{ canonicalize_local_workspace_root, get_remote_workspace_manager, init_remote_workspace_manager, local_workspace_roots_equal, normalize_remote_workspace_path, remote_workspace_stable_id, @@ -60,6 +63,12 @@ pub struct WorkspaceCreateOptions { pub stable_workspace_id: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WorkspaceActivityMode { + TouchOnly, + RefreshMetadata, +} + impl Default for WorkspaceCreateOptions { fn default() -> Self { Self { @@ -344,10 +353,16 @@ impl WorkspaceService { options: WorkspaceCreateOptions, ) -> BitFunResult { let options = self.normalize_workspace_options_for_path(&path, options); + let worktree = + WorkspaceInfo::resolve_worktree_info(&path, WorktreeTopologyFreshness::Cached).await; let result = { let mut manager = self.manager.write().await; manager - .open_workspace_with_options(path, Self::to_manager_open_options(&options)) + .open_workspace_with_resolved_worktree( + path, + Self::to_manager_open_options(&options), + worktree, + ) .await }; @@ -532,13 +547,25 @@ impl WorkspaceService { &self, path: PathBuf, options: WorkspaceCreateOptions, + mode: WorkspaceActivityMode, ) -> BitFunResult { let mut options = self.normalize_workspace_options_for_path(&path, options); options.auto_set_current = false; + let refresh_worktree = match mode { + WorkspaceActivityMode::TouchOnly => None, + WorkspaceActivityMode::RefreshMetadata => Some( + WorkspaceInfo::resolve_worktree_info(&path, WorktreeTopologyFreshness::Cached) + .await, + ), + }; let result = { let mut manager = self.manager.write().await; manager - .track_workspace_with_options(path, Self::to_manager_open_options(&options)) + .track_workspace_with_options( + path, + Self::to_manager_open_options(&options), + refresh_worktree, + ) .await }; @@ -559,6 +586,24 @@ impl WorkspaceService { result } + #[cfg(feature = "service-integrations")] + pub async fn list_worktrees( + &self, + path: &Path, + freshness: WorktreeTopologyFreshness, + ) -> Result, GitError> { + super::worktree_topology::global_worktree_topology_service() + .list_worktrees(path, freshness) + .await + } + + #[cfg(feature = "service-integrations")] + pub async fn invalidate_worktree_topology(&self, path: &Path) { + super::worktree_topology::global_worktree_topology_service() + .invalidate(path) + .await; + } + /// Quickly opens a workspace (using default options). pub async fn quick_open(&self, path: &str) -> BitFunResult { let path_buf = PathBuf::from(path); @@ -989,7 +1034,12 @@ impl WorkspaceService { workspace_id ))); }; - let new_workspace = WorkspaceInfo::new( + let worktree = WorkspaceInfo::resolve_worktree_info( + &workspace_path, + WorktreeTopologyFreshness::ForceRefresh, + ) + .await; + let new_workspace = WorkspaceInfo::new_without_worktree( workspace_path, WorkspaceOpenOptions { scan_options: ScanOptions::default(), @@ -1012,6 +1062,7 @@ impl WorkspaceService { ) .await?; let mut new_workspace = new_workspace; + new_workspace.worktree = worktree; new_workspace.id = existing_workspace.id.clone(); new_workspace.opened_at = existing_workspace.opened_at; new_workspace.description = existing_workspace.description.clone(); @@ -2274,6 +2325,7 @@ mod tests { use crate::agentic::persistence::PersistenceManager; use crate::infrastructure::storage::{PersistenceService, StorageOptions}; use crate::service::session::SessionMetadata; + use crate::service::workspace::WorkspaceWorktreeInfo; use std::collections::HashMap; use uuid::Uuid; @@ -2487,7 +2539,11 @@ mod tests { let workspace_root = env.create_workspace_dir("tracked-workspace"); let tracked = service - .track_workspace_activity(workspace_root.clone(), WorkspaceCreateOptions::default()) + .track_workspace_activity( + workspace_root.clone(), + WorkspaceCreateOptions::default(), + WorkspaceActivityMode::RefreshMetadata, + ) .await .expect("workspace tracking should succeed"); @@ -2511,6 +2567,47 @@ mod tests { ); } + #[tokio::test] + async fn touch_only_workspace_activity_preserves_worktree_metadata() { + let env = TestEnvironment::new(); + let service = build_test_workspace_service(env.path_manager.clone()).await; + let workspace_root = env.create_workspace_dir("touch-only-workspace"); + + let tracked = service + .track_workspace_activity( + workspace_root.clone(), + WorkspaceCreateOptions::default(), + WorkspaceActivityMode::RefreshMetadata, + ) + .await + .expect("workspace tracking should succeed"); + let expected_worktree = WorkspaceWorktreeInfo { + path: workspace_root.to_string_lossy().replace('\\', "/"), + branch: Some("cached-branch".to_string()), + main_repo_path: workspace_root.to_string_lossy().replace('\\', "/"), + is_main: true, + }; + { + let mut manager = service.manager.write().await; + manager + .get_workspaces_mut() + .get_mut(&tracked.id) + .expect("tracked workspace should exist") + .worktree = Some(expected_worktree.clone()); + } + + let touched = service + .track_workspace_activity( + workspace_root, + WorkspaceCreateOptions::default(), + WorkspaceActivityMode::TouchOnly, + ) + .await + .expect("touch-only tracking should succeed"); + + assert_eq!(touched.worktree, Some(expected_worktree)); + } + #[tokio::test] async fn track_workspace_activity_assigns_stable_remote_workspace_id() { let env = TestEnvironment::new(); @@ -2526,6 +2623,7 @@ mod tests { remote_ssh_host: Some("example-host".to_string()), ..Default::default() }, + WorkspaceActivityMode::RefreshMetadata, ) .await .expect("remote workspace tracking should succeed"); @@ -2554,6 +2652,7 @@ mod tests { display_name: Some("repos".to_string()), ..Default::default() }, + WorkspaceActivityMode::RefreshMetadata, ) .await .expect("remote workspace should be remembered"); diff --git a/src/crates/assembly/core/src/service/workspace/worktree_topology.rs b/src/crates/assembly/core/src/service/workspace/worktree_topology.rs new file mode 100644 index 0000000000..e5e187dc52 --- /dev/null +++ b/src/crates/assembly/core/src/service/workspace/worktree_topology.rs @@ -0,0 +1,372 @@ +use super::WorktreeTopologyFreshness; +use crate::service::git::{GitError, GitService, GitWorktreeInfo, GitWorktreeRepositoryInfo}; +use std::collections::{hash_map::DefaultHasher, HashMap}; +use std::hash::{Hash, Hasher}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, OnceLock}; +use std::time::{Duration, Instant, SystemTime}; +use tokio::sync::Mutex; + +const WORKTREE_TOPOLOGY_TTL: Duration = Duration::from_secs(5 * 60); +const MAX_CACHED_REPOSITORIES: usize = 64; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct MetadataFingerprint(u64); + +#[derive(Clone)] +struct CachedTopology { + worktrees: Arc>, + fingerprint: MetadataFingerprint, + refreshed_at: Instant, + refresh_version: u64, + last_used: u64, +} + +#[derive(Default)] +struct CacheState { + entries: HashMap, + gates: HashMap>>, + invalidation_versions: HashMap, +} + +pub struct WorktreeTopologyService { + state: Mutex, + refresh_version: AtomicU64, + invalidation_version: AtomicU64, + access_tick: AtomicU64, + #[cfg(test)] + query_count: std::sync::atomic::AtomicUsize, +} + +impl Default for WorktreeTopologyService { + fn default() -> Self { + Self { + state: Mutex::new(CacheState::default()), + refresh_version: AtomicU64::new(1), + invalidation_version: AtomicU64::new(1), + access_tick: AtomicU64::new(1), + #[cfg(test)] + query_count: std::sync::atomic::AtomicUsize::new(0), + } + } +} + +impl WorktreeTopologyService { + pub async fn list_worktrees( + &self, + path: &Path, + freshness: WorktreeTopologyFreshness, + ) -> Result, GitError> { + let repository = GitService::resolve_worktree_repository(path).await?; + let fingerprint = metadata_fingerprint(&repository); + let access_tick = self.access_tick.fetch_add(1, Ordering::Relaxed); + + let (gate, observed_refresh_version, observed_invalidation_version) = { + let mut state = self.state.lock().await; + if freshness == WorktreeTopologyFreshness::Cached { + if let Some(cached) = state.entries.get_mut(&repository.common_git_dir) { + if cached.refreshed_at.elapsed() < WORKTREE_TOPOLOGY_TTL + && cached.fingerprint == fingerprint + { + cached.last_used = access_tick; + return Ok(cached.worktrees.as_ref().clone()); + } + } + } + + let observed_refresh_version = state + .entries + .get(&repository.common_git_dir) + .map(|cached| cached.refresh_version) + .unwrap_or_default(); + let gate = state + .gates + .entry(repository.common_git_dir.clone()) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .clone(); + let observed_invalidation_version = state + .invalidation_versions + .get(&repository.common_git_dir) + .copied() + .unwrap_or_default(); + ( + gate, + observed_refresh_version, + observed_invalidation_version, + ) + }; + + let _refresh_guard = gate.lock().await; + let fingerprint = metadata_fingerprint(&repository); + { + let mut state = self.state.lock().await; + if let Some(cached) = state.entries.get_mut(&repository.common_git_dir) { + let another_request_refreshed = cached.refresh_version > observed_refresh_version; + let cached_is_fresh = cached.refreshed_at.elapsed() < WORKTREE_TOPOLOGY_TTL + && cached.fingerprint == fingerprint; + if another_request_refreshed + || (freshness == WorktreeTopologyFreshness::Cached && cached_is_fresh) + { + cached.last_used = access_tick; + return Ok(cached.worktrees.as_ref().clone()); + } + } + } + + #[cfg(test)] + { + self.query_count.fetch_add(1, Ordering::Relaxed); + tokio::time::sleep(Duration::from_millis(40)).await; + } + let worktrees = GitService::list_worktrees(&repository.query_path).await?; + let cached = CachedTopology { + worktrees: Arc::new(worktrees.clone()), + fingerprint: metadata_fingerprint(&repository), + refreshed_at: Instant::now(), + refresh_version: self.refresh_version.fetch_add(1, Ordering::Relaxed), + last_used: access_tick, + }; + + let mut state = self.state.lock().await; + let invalidated_during_query = state + .invalidation_versions + .get(&repository.common_git_dir) + .copied() + .unwrap_or_default() + > observed_invalidation_version; + if invalidated_during_query { + return Ok(worktrees); + } + if state.entries.len() >= MAX_CACHED_REPOSITORIES + && !state.entries.contains_key(&repository.common_git_dir) + { + if let Some(oldest) = state + .entries + .iter() + .min_by_key(|(_, cached)| cached.last_used) + .map(|(path, _)| path.clone()) + { + state.entries.remove(&oldest); + if state + .gates + .get(&oldest) + .map(Arc::strong_count) + .unwrap_or_default() + == 1 + { + state.gates.remove(&oldest); + } + } + } + state.entries.insert(repository.common_git_dir, cached); + Ok(worktrees) + } + + pub async fn invalidate(&self, path: &Path) { + let Ok(repository) = GitService::resolve_worktree_repository(path).await else { + return; + }; + let mut state = self.state.lock().await; + state.entries.remove(&repository.common_git_dir); + state.invalidation_versions.insert( + repository.common_git_dir, + self.invalidation_version.fetch_add(1, Ordering::Relaxed), + ); + } + + #[cfg(test)] + fn query_count(&self) -> usize { + self.query_count.load(Ordering::Relaxed) + } +} + +pub fn global_worktree_topology_service() -> &'static WorktreeTopologyService { + static SERVICE: OnceLock = OnceLock::new(); + SERVICE.get_or_init(WorktreeTopologyService::default) +} + +fn metadata_fingerprint(repository: &GitWorktreeRepositoryInfo) -> MetadataFingerprint { + let mut hasher = DefaultHasher::new(); + hash_metadata_path(&repository.common_git_dir.join("config"), &mut hasher, 0); + hash_metadata_path(&repository.common_git_dir.join("worktrees"), &mut hasher, 2); + if repository.worktree_git_marker.is_file() { + hash_metadata_path(&repository.worktree_git_marker, &mut hasher, 0); + } + MetadataFingerprint(hasher.finish()) +} + +fn hash_metadata_path(path: &Path, hasher: &mut DefaultHasher, remaining_depth: usize) { + path.hash(hasher); + match std::fs::metadata(path) { + Ok(metadata) => { + true.hash(hasher); + metadata.len().hash(hasher); + metadata.is_dir().hash(hasher); + metadata + .modified() + .ok() + .and_then(|modified| modified.duration_since(SystemTime::UNIX_EPOCH).ok()) + .map(|duration| duration.as_nanos()) + .hash(hasher); + + if metadata.is_dir() && remaining_depth > 0 { + let mut children = std::fs::read_dir(path) + .ok() + .into_iter() + .flatten() + .filter_map(Result::ok) + .map(|entry| entry.path()) + .collect::>(); + children.sort(); + children.len().hash(hasher); + for child in children { + hash_metadata_path(&child, hasher, remaining_depth - 1); + } + } + } + Err(_) => false.hash(hasher), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::process::Command; + + fn git(root: &Path, args: &[&str]) { + let output = Command::new("git") + .current_dir(root) + .args(args) + .output() + .expect("git should be available for worktree topology tests"); + assert!( + output.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&output.stderr) + ); + } + + fn initialized_repository() -> tempfile::TempDir { + let directory = tempfile::tempdir().expect("temporary repository should be created"); + git(directory.path(), &["init"]); + git(directory.path(), &["config", "user.name", "BitFun Tests"]); + git( + directory.path(), + &["config", "user.email", "bitfun@example.com"], + ); + std::fs::write(directory.path().join("tracked.txt"), "initial\n") + .expect("fixture should be written"); + git(directory.path(), &["add", "tracked.txt"]); + git(directory.path(), &["commit", "-m", "initial"]); + directory + } + + #[tokio::test] + async fn concurrent_main_and_linked_reads_share_one_query() { + let repository = initialized_repository(); + let linked_root = repository.path().join("linked"); + git( + repository.path(), + &[ + "worktree", + "add", + "-b", + "linked-test", + linked_root.to_string_lossy().as_ref(), + ], + ); + + let service = Arc::new(WorktreeTopologyService::default()); + let barrier = Arc::new(tokio::sync::Barrier::new(8)); + let mut tasks = Vec::new(); + for index in 0..8 { + let service = Arc::clone(&service); + let barrier = Arc::clone(&barrier); + let path = if index % 2 == 0 { + repository.path().to_path_buf() + } else { + linked_root.clone() + }; + tasks.push(tokio::spawn(async move { + barrier.wait().await; + service + .list_worktrees(&path, WorktreeTopologyFreshness::Cached) + .await + .expect("topology should load") + })); + } + + for task in tasks { + assert_eq!(task.await.expect("task should join").len(), 2); + } + assert_eq!(service.query_count(), 1); + } + + #[tokio::test] + async fn metadata_changes_and_explicit_invalidation_refresh_the_cache() { + let repository = initialized_repository(); + let service = WorktreeTopologyService::default(); + + let initial = service + .list_worktrees(repository.path(), WorktreeTopologyFreshness::Cached) + .await + .expect("initial topology should load"); + assert_eq!(initial.len(), 1); + assert_eq!(service.query_count(), 1); + + let linked_root = repository.path().join("external-linked"); + git( + repository.path(), + &[ + "worktree", + "add", + "-b", + "external-linked-test", + linked_root.to_string_lossy().as_ref(), + ], + ); + + let refreshed = service + .list_worktrees(repository.path(), WorktreeTopologyFreshness::Cached) + .await + .expect("metadata change should refresh topology"); + assert_eq!(refreshed.len(), 2); + assert_eq!(service.query_count(), 2); + + service.invalidate(repository.path()).await; + let after_invalidation = service + .list_worktrees(repository.path(), WorktreeTopologyFreshness::Cached) + .await + .expect("invalidated topology should reload"); + assert_eq!(after_invalidation.len(), 2); + assert_eq!(service.query_count(), 3); + } + + #[tokio::test] + async fn invalidation_during_query_does_not_restore_stale_cache_data() { + let repository = initialized_repository(); + let service = Arc::new(WorktreeTopologyService::default()); + let query_service = Arc::clone(&service); + let repository_path = repository.path().to_path_buf(); + let query = tokio::spawn(async move { + query_service + .list_worktrees(&repository_path, WorktreeTopologyFreshness::Cached) + .await + .expect("topology query should complete") + }); + + while service.query_count() == 0 { + tokio::task::yield_now().await; + } + service.invalidate(repository.path()).await; + assert_eq!(query.await.expect("query task should join").len(), 1); + + service + .list_worktrees(repository.path(), WorktreeTopologyFreshness::Cached) + .await + .expect("post-invalidation topology should reload"); + assert_eq!(service.query_count(), 2); + } +} diff --git a/src/crates/contracts/runtime-ports/src/script_tool.rs b/src/crates/contracts/runtime-ports/src/script_tool.rs index 0d062734c0..a9b94a79f7 100644 --- a/src/crates/contracts/runtime-ports/src/script_tool.rs +++ b/src/crates/contracts/runtime-ports/src/script_tool.rs @@ -73,6 +73,10 @@ pub struct ScriptToolInvokeResponse { pub trait ScriptToolRuntime: Send + Sync { async fn availability(&self) -> ScriptToolRuntimeAvailability; + /// Clears cached executable discovery and availability probes so the next + /// availability check observes runtime installation changes. + async fn invalidate_availability(&self) {} + async fn is_loaded(&self, target_id: &str) -> bool; /// Waits until the currently loaded target process exits. Implementations diff --git a/src/crates/services/services-integrations/src/git/service.rs b/src/crates/services/services-integrations/src/git/service.rs index 9d8871c6d6..1db95ffcc3 100644 --- a/src/crates/services/services-integrations/src/git/service.rs +++ b/src/crates/services/services-integrations/src/git/service.rs @@ -2,7 +2,8 @@ use super::*; /** * Git service implementation */ -use git2::{BranchType, Commit, Repository}; +use git2::{BranchType, Commit, ErrorCode, Repository}; +use std::io::Write; use std::path::Path; use std::time::Duration; use std::time::Instant; @@ -96,6 +97,43 @@ impl GitService { .map_err(|e| GitError::CommandFailed(format!("spawn_blocking join: {e}")))? } + /// Resolves the stable repository identity shared by all worktrees without + /// spawning the Git CLI. + pub async fn resolve_worktree_repository>( + path: P, + ) -> Result { + let requested_path = path.as_ref().to_path_buf(); + task::spawn_blocking(move || { + let repository = Repository::discover(&requested_path) + .map_err(|error| GitError::RepositoryNotFound(error.to_string()))?; + let query_path = repository + .workdir() + .map(Path::to_path_buf) + .unwrap_or_else(|| requested_path.clone()); + let git_dir = repository.path().to_path_buf(); + let common_git_dir = git_dir + .parent() + .filter(|parent| { + parent + .file_name() + .and_then(|name| name.to_str()) + .map(|name| name.eq_ignore_ascii_case("worktrees")) + .unwrap_or(false) + }) + .and_then(Path::parent) + .map(Path::to_path_buf) + .unwrap_or(git_dir); + + Ok(GitWorktreeRepositoryInfo { + worktree_git_marker: query_path.join(".git"), + query_path: std::fs::canonicalize(&query_path).unwrap_or(query_path), + common_git_dir: std::fs::canonicalize(&common_git_dir).unwrap_or(common_git_dir), + }) + }) + .await + .map_err(|error| GitError::CommandFailed(format!("spawn_blocking join: {error}")))? + } + /// Resolves a revision to an immutable commit id without changing repository state. pub async fn resolve_revision>( path: P, @@ -1338,6 +1376,36 @@ impl GitService { create_branch: bool, ) -> Result { let repo_path = path.as_ref().to_string_lossy(); + let repository_path = path.as_ref().to_path_buf(); + let repository_info = Self::resolve_worktree_repository(path.as_ref()).await?; + + task::spawn_blocking(move || { + let repository = Repository::discover(&repository_path) + .map_err(|error| GitError::RepositoryNotFound(error.to_string()))?; + match repository.head() { + Ok(head) if head.target().is_some() => {} + Err(error) if error.code() == ErrorCode::UnbornBranch => { + return Err(GitError::CommandFailed( + "Cannot create a worktree before the repository has an initial commit" + .to_string(), + )); + } + Ok(_) => { + return Err(GitError::CommandFailed( + "Cannot create a worktree because the repository HEAD has no commit" + .to_string(), + )); + } + Err(error) => { + return Err(GitError::CommandFailed(format!( + "Failed to inspect repository HEAD before creating worktree: {error}" + ))); + } + } + ensure_worktree_directory_excluded(&repository_info.common_git_dir) + }) + .await + .map_err(|error| GitError::CommandFailed(format!("spawn_blocking join: {error}")))??; let worktree_dir = path.as_ref().join(".worktrees"); let worktree_path = worktree_dir.join(branch); @@ -1354,17 +1422,41 @@ impl GitService { }; execute_git_command(&repo_path, &args).await?; - - let worktrees = Self::list_worktrees(&path).await?; - let normalized_expected = worktree_path_str.replace("\\", "/"); - - worktrees - .into_iter() - .find(|wt| wt.path == normalized_expected) - .ok_or_else(|| { - GitError::CommandFailed("Failed to find newly created worktree".to_string()) + let expected_branch = branch.to_string(); + task::spawn_blocking(move || { + let repository = Repository::open(&worktree_path).map_err(|error| { + GitError::CommandFailed(format!( + "Failed to inspect newly created worktree: {error}" + )) + })?; + let (branch, head) = match repository.head() { + Ok(head) => ( + head.shorthand().ok().map(str::to_string), + head.target() + .map(|target| target.to_string()) + .unwrap_or_default(), + ), + Err(error) if error.code() == ErrorCode::UnbornBranch => { + (Some(expected_branch), "0".repeat(40)) + } + Err(error) => { + return Err(GitError::CommandFailed(format!( + "Failed to resolve newly created worktree HEAD: {error}" + ))) + } + }; + Ok(GitWorktreeInfo { + path: normalized_expected, + branch, + head, + is_main: false, + is_locked: false, + is_prunable: false, }) + }) + .await + .map_err(|error| GitError::CommandFailed(format!("spawn_blocking join: {error}")))? } /// Removes a worktree. @@ -1406,6 +1498,35 @@ impl GitService { } } +fn ensure_worktree_directory_excluded(common_git_dir: &Path) -> Result<(), GitError> { + const WORKTREE_EXCLUDE_PATTERN: &str = ".worktrees/"; + + let info_dir = common_git_dir.join("info"); + std::fs::create_dir_all(&info_dir).map_err(GitError::IoError)?; + let exclude_path = info_dir.join("exclude"); + let existing = match std::fs::read_to_string(&exclude_path) { + Ok(existing) => existing, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(error) => return Err(GitError::IoError(error)), + }; + if existing + .lines() + .any(|line| line.trim() == WORKTREE_EXCLUDE_PATTERN) + { + return Ok(()); + } + + let mut exclude = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(exclude_path) + .map_err(GitError::IoError)?; + if !existing.is_empty() && !existing.ends_with('\n') { + writeln!(exclude).map_err(GitError::IoError)?; + } + writeln!(exclude, "{WORKTREE_EXCLUDE_PATTERN}").map_err(GitError::IoError) +} + #[cfg(test)] mod review_path_tests { use super::{review_path_has_parent_traversal, GitLogParams, GitService}; @@ -1507,4 +1628,51 @@ mod review_path_tests { vec!["old commit"] ); } + + #[tokio::test] + async fn add_worktree_returns_created_checkout_without_listing_all_worktrees() { + let directory = tempfile::tempdir().expect("temporary repository should be created"); + git(directory.path(), &["init"], None); + commit_file( + directory.path(), + "initial\n", + "initial commit", + "2025-01-01T00:00:00Z", + ); + + let worktree = GitService::add_worktree(directory.path(), "feature-test", true) + .await + .expect("worktree should be created"); + + assert_eq!(worktree.branch.as_deref(), Some("feature-test")); + assert!(!worktree.head.is_empty()); + assert!(!worktree.is_main); + assert!(Path::new(&worktree.path).is_dir()); + let exclude = fs::read_to_string(directory.path().join(".git/info/exclude")) + .expect("Git exclude file should be readable"); + assert_eq!( + exclude + .lines() + .filter(|line| line.trim() == ".worktrees/") + .count(), + 1 + ); + } + + #[tokio::test] + async fn add_worktree_rejects_repository_without_commits_before_side_effects() { + let directory = tempfile::tempdir().expect("temporary repository should be created"); + git(directory.path(), &["init"], None); + + let error = GitService::add_worktree(directory.path(), "unborn-test", true) + .await + .expect_err("unborn worktree creation should be rejected"); + + assert!(error.to_string().contains("initial commit")); + assert!(!directory.path().join(".worktrees").exists()); + let worktrees = GitService::list_worktrees(directory.path()) + .await + .expect("worktree list should remain readable"); + assert_eq!(worktrees.len(), 1); + } } diff --git a/src/crates/services/services-integrations/src/git/types.rs b/src/crates/services/services-integrations/src/git/types.rs index 440d4c314a..9eb6ac326d 100644 --- a/src/crates/services/services-integrations/src/git/types.rs +++ b/src/crates/services/services-integrations/src/git/types.rs @@ -2,6 +2,7 @@ * Git-related type definitions */ use serde::{Deserialize, Serialize}; +use std::path::PathBuf; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct GitRepository { @@ -255,6 +256,17 @@ pub struct GitWorktreeInfo { pub is_prunable: bool, } +/// Repository identity used to share worktree topology across linked checkouts. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitWorktreeRepositoryInfo { + /// A working directory suitable for running `git worktree list`. + pub query_path: PathBuf, + /// The common Git directory shared by the main and linked worktrees. + pub common_git_dir: PathBuf, + /// The current worktree's `.git` marker. Linked worktrees use a file here. + pub worktree_git_marker: PathBuf, +} + /// Git graph node. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/src/crates/services/services-integrations/src/script_tool.rs b/src/crates/services/services-integrations/src/script_tool.rs index 4fa3051002..babed37c7f 100644 --- a/src/crates/services/services-integrations/src/script_tool.rs +++ b/src/crates/services/services-integrations/src/script_tool.rs @@ -760,8 +760,24 @@ fn port_error_kind(kind: Option<&str>) -> PortErrorKind { } } +#[derive(Clone)] +struct CachedNodeAvailability { + executable: PathBuf, + version: String, +} + +impl CachedNodeAvailability { + fn to_runtime_availability(&self) -> ScriptToolRuntimeAvailability { + ScriptToolRuntimeAvailability::Available { + executable: self.executable.to_string_lossy().into_owned(), + version: self.version.clone(), + } + } +} + pub struct NodeScriptToolRuntime { executable: RwLock>, + availability_cache: Mutex>, workers: RwLock>>, load_gate: Mutex<()>, } @@ -776,6 +792,7 @@ impl NodeScriptToolRuntime { pub fn discover() -> Self { Self { executable: RwLock::new(which::which("node").ok()), + availability_cache: Mutex::new(None), workers: RwLock::new(HashMap::new()), load_gate: Mutex::new(()), } @@ -817,18 +834,34 @@ impl NodeScriptToolRuntime { #[async_trait] impl ScriptToolRuntime for NodeScriptToolRuntime { async fn availability(&self) -> ScriptToolRuntimeAvailability { - match self.resolve_executable().await { + let mut cache = self.availability_cache.lock().await; + if let Some(availability) = cache.as_ref() { + return availability.to_runtime_availability(); + } + + let availability = match self.resolve_executable().await { Some(executable) => match probe_node_version(&executable).await { - Ok(version) => ScriptToolRuntimeAvailability::Available { - executable: executable.to_string_lossy().into_owned(), - version, - }, + Ok(version) => { + let cached = CachedNodeAvailability { + executable, + version, + }; + let availability = cached.to_runtime_availability(); + *cache = Some(cached); + availability + } Err(reason) => ScriptToolRuntimeAvailability::Unavailable { reason }, }, None => ScriptToolRuntimeAvailability::Unavailable { reason: "BitFun could not find Node.js for external tools".to_string(), }, - } + }; + availability + } + + async fn invalidate_availability(&self) { + *self.availability_cache.lock().await = None; + *self.executable.write().await = None; } async fn is_loaded(&self, target_id: &str) -> bool { @@ -1051,7 +1084,8 @@ async fn probe_node_version(executable: &PathBuf) -> Result { #[cfg(test)] mod tests { - use super::NodeScriptToolRuntime; + use super::{CachedNodeAvailability, NodeScriptToolRuntime}; + use bitfun_runtime_ports::{ScriptToolRuntime, ScriptToolRuntimeAvailability}; use std::path::PathBuf; use tokio::sync::{Mutex, RwLock}; @@ -1059,6 +1093,7 @@ mod tests { async fn missing_node_path_can_recover_without_replacing_the_runtime() { let runtime = NodeScriptToolRuntime { executable: RwLock::new(None), + availability_cache: Mutex::new(None), workers: RwLock::new(Default::default()), load_gate: Mutex::new(()), }; @@ -1072,4 +1107,43 @@ mod tests { ); assert_eq!(*runtime.executable.read().await, Some(discovered)); } + + #[tokio::test] + async fn cached_success_is_reused_until_runtime_availability_is_invalidated() { + let cached = CachedNodeAvailability { + executable: PathBuf::from("cached-node"), + version: "v22.12.0".to_string(), + }; + let availability = cached.to_runtime_availability(); + let runtime = NodeScriptToolRuntime { + executable: RwLock::new(Some(PathBuf::from("stale-node"))), + availability_cache: Mutex::new(Some(cached)), + workers: RwLock::new(Default::default()), + load_gate: Mutex::new(()), + }; + + assert_eq!(runtime.availability().await, availability); + + runtime.invalidate_availability().await; + assert!(runtime.availability_cache.lock().await.is_none()); + assert!(runtime.executable.read().await.is_none()); + } + + #[tokio::test] + async fn unavailable_result_is_not_cached() { + let runtime = NodeScriptToolRuntime { + executable: RwLock::new(Some(PathBuf::from( + "definitely-missing-node-for-availability-cache-test", + ))), + availability_cache: Mutex::new(None), + workers: RwLock::new(Default::default()), + load_gate: Mutex::new(()), + }; + + assert!(matches!( + runtime.availability().await, + ScriptToolRuntimeAvailability::Unavailable { .. } + )); + assert!(runtime.availability_cache.lock().await.is_none()); + } } diff --git a/src/crates/services/terminal/src/exec.rs b/src/crates/services/terminal/src/exec.rs index 162ce26dcc..9b08e5db5d 100644 --- a/src/crates/services/terminal/src/exec.rs +++ b/src/crates/services/terminal/src/exec.rs @@ -4,14 +4,14 @@ //! `exec_command` starts a fresh local process; a session id is only retained //! while that process is still running so later calls can poll or write stdin. -use crate::{TerminalError, TerminalResult}; +use crate::{shell::invalidate_cached_executable, TerminalError, TerminalResult}; use chardetng::EncodingDetector; use encoding_rs::{Encoding, IBM866, WINDOWS_1252}; use portable_pty::{native_pty_system, CommandBuilder, MasterPty, PtySize, SlavePty}; use rand::Rng; use std::collections::{HashMap, VecDeque}; use std::io::{ErrorKind, Read, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Stdio; use std::sync::{Arc, Mutex as StdMutex, OnceLock}; use std::time::Duration; @@ -1009,7 +1009,13 @@ async fn spawn_pty_process(request: &ExecCommandRequest) -> TerminalResult child, + Err(error) => { + invalidate_cached_executable(Path::new(&request.argv[0])); + return Err(error.into()); + } + }; let killer = child.clone_killer(); let output = Arc::new(OutputState::new(request.output_capture_tx.clone())); let mut reader = pair.master.try_clone_reader()?; @@ -1130,7 +1136,13 @@ async fn spawn_pipe_process(request: &ExecCommandRequest) -> TerminalResult child, + Err(error) => { + invalidate_cached_executable(Path::new(&request.argv[0])); + return Err(error.into()); + } + }; #[cfg(windows)] let pipe_job = create_windows_pipe_job(&child)?; #[cfg(windows)] diff --git a/src/crates/services/terminal/src/exec_shell.rs b/src/crates/services/terminal/src/exec_shell.rs index 1dcc0e0280..0a9b6df05f 100644 --- a/src/crates/services/terminal/src/exec_shell.rs +++ b/src/crates/services/terminal/src/exec_shell.rs @@ -34,19 +34,15 @@ pub enum ConfiguredShellPreference { pub fn resolve_local_exec_shell(configured_shell: Option<&str>) -> ResolvedLocalExecShell { let configured = configured_shell.and_then(parse_configured_shell_preference); - let detected_shells: Vec<_> = ShellDetector::detect_available_shells() - .into_iter() - .map(|shell| ResolvedLocalExecShell::new(shell.display_name, shell.path, shell.shell_type)) - .collect(); - let system_default = { - let shell = ShellDetector::get_default_shell(); - ResolvedLocalExecShell::new(shell.display_name, shell.path, shell.shell_type) - }; if cfg!(windows) { - select_windows_local_exec_shell(configured, &detected_shells, &system_default) + resolve_windows_local_exec_shell_with(configured, resolve_detected_shell) } else { - select_non_windows_local_exec_shell(configured, &detected_shells, &system_default) + resolve_non_windows_local_exec_shell_with( + configured, + resolve_detected_shell, + resolve_default_shell, + ) } } @@ -77,45 +73,57 @@ pub fn parse_configured_shell_preference(raw: &str) -> Option( configured: Option, - detected_shells: &[ResolvedLocalExecShell], - system_default: &ResolvedLocalExecShell, -) -> ResolvedLocalExecShell { - configured - .and_then(shell_type_for_supported_preference) - .and_then(|shell_type| find_detected_shell(detected_shells, shell_type)) - .unwrap_or_else(|| system_default.clone()) + mut find_shell: FindShell, + default_shell: DefaultShell, +) -> ResolvedLocalExecShell +where + FindShell: FnMut(ShellType) -> Option, + DefaultShell: FnOnce() -> ResolvedLocalExecShell, +{ + if let Some(shell_type) = configured.and_then(shell_type_for_supported_preference) { + if let Some(shell) = find_shell(shell_type) { + return shell; + } + } + default_shell() } -fn select_windows_local_exec_shell( +fn resolve_windows_local_exec_shell_with( configured: Option, - detected_shells: &[ResolvedLocalExecShell], - system_default: &ResolvedLocalExecShell, -) -> ResolvedLocalExecShell { + mut find_shell: FindShell, +) -> ResolvedLocalExecShell +where + FindShell: FnMut(ShellType) -> Option, +{ // ExecCommand deliberately narrows Windows shells to the variants whose // one-shot command behavior we explicitly support well. - let pwsh = || find_detected_shell(detected_shells, ShellType::PowerShellCore); - let powershell = || find_detected_shell(detected_shells, ShellType::PowerShell); - let bash = || find_detected_shell(detected_shells, ShellType::Bash); - - match configured { - Some(ConfiguredShellPreference::PowerShellCore) => pwsh() - .or_else(powershell) - .or_else(bash) - .unwrap_or_else(|| system_default.clone()), - Some(ConfiguredShellPreference::PowerShell) => powershell() - .or_else(pwsh) - .or_else(bash) - .unwrap_or_else(|| system_default.clone()), - Some(ConfiguredShellPreference::Bash) => bash() - .or_else(powershell) - .or_else(pwsh) - .unwrap_or_else(|| system_default.clone()), - Some(ConfiguredShellPreference::Cmd) => pwsh() - .or_else(powershell) - .or_else(bash) - .unwrap_or_else(|| system_default.clone()), + let order = match configured { + Some(ConfiguredShellPreference::PowerShellCore) => [ + ShellType::PowerShellCore, + ShellType::PowerShell, + ShellType::Bash, + ShellType::Cmd, + ], + Some(ConfiguredShellPreference::PowerShell) => [ + ShellType::PowerShell, + ShellType::PowerShellCore, + ShellType::Bash, + ShellType::Cmd, + ], + Some(ConfiguredShellPreference::Bash) => [ + ShellType::Bash, + ShellType::PowerShell, + ShellType::PowerShellCore, + ShellType::Cmd, + ], + Some(ConfiguredShellPreference::Cmd) | None => [ + ShellType::PowerShellCore, + ShellType::PowerShell, + ShellType::Bash, + ShellType::Cmd, + ], Some( ConfiguredShellPreference::Zsh | ConfiguredShellPreference::Fish @@ -123,15 +131,25 @@ fn select_windows_local_exec_shell( | ConfiguredShellPreference::Ksh | ConfiguredShellPreference::Csh | ConfiguredShellPreference::Unsupported, - ) => powershell() - .or_else(pwsh) - .or_else(bash) - .unwrap_or_else(|| system_default.clone()), - None => pwsh() - .or_else(powershell) - .or_else(bash) - .unwrap_or_else(|| system_default.clone()), + ) => [ + ShellType::PowerShell, + ShellType::PowerShellCore, + ShellType::Bash, + ShellType::Cmd, + ], + }; + + for shell_type in order { + if let Some(shell) = find_shell(shell_type) { + return shell; + } } + + ResolvedLocalExecShell::new( + "Command Prompt".to_string(), + PathBuf::from("cmd.exe"), + ShellType::Cmd, + ) } fn shell_type_for_supported_preference(preference: ConfiguredShellPreference) -> Option { @@ -149,21 +167,21 @@ fn shell_type_for_supported_preference(preference: ConfiguredShellPreference) -> }) } -fn find_detected_shell( - detected_shells: &[ResolvedLocalExecShell], - shell_type: ShellType, -) -> Option { - detected_shells - .iter() - .find(|shell| shell.shell_type == shell_type) - .cloned() +fn resolve_detected_shell(shell_type: ShellType) -> Option { + ShellDetector::find_shell(&shell_type) + .map(|shell| ResolvedLocalExecShell::new(shell.display_name, shell.path, shell.shell_type)) +} + +fn resolve_default_shell() -> ResolvedLocalExecShell { + let shell = ShellDetector::get_default_shell(); + ResolvedLocalExecShell::new(shell.display_name, shell.path, shell.shell_type) } #[cfg(test)] mod tests { use super::{ - parse_configured_shell_preference, select_non_windows_local_exec_shell, - select_windows_local_exec_shell, ConfiguredShellPreference, ResolvedLocalExecShell, + parse_configured_shell_preference, resolve_non_windows_local_exec_shell_with, + resolve_windows_local_exec_shell_with, ConfiguredShellPreference, ResolvedLocalExecShell, }; use crate::shell::ShellType; use std::path::PathBuf; @@ -176,6 +194,16 @@ mod tests { } } + fn find_in( + detected: &[ResolvedLocalExecShell], + shell_type: ShellType, + ) -> Option { + detected + .iter() + .find(|shell| shell.shell_type == shell_type) + .cloned() + } + #[test] fn parses_configured_shell_values_from_enum_names_and_paths() { assert_eq!( @@ -216,10 +244,9 @@ mod tests { ShellType::Bash, ), ]; - let resolved = select_windows_local_exec_shell( + let resolved = resolve_windows_local_exec_shell_with( Some(ConfiguredShellPreference::Cmd), - &detected, - &detected[0], + |shell_type| find_in(&detected, shell_type), ); assert_eq!(resolved.shell_type, ShellType::PowerShellCore); @@ -236,10 +263,9 @@ mod tests { "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", ShellType::PowerShell, )]; - let resolved = select_windows_local_exec_shell( + let resolved = resolve_windows_local_exec_shell_with( Some(ConfiguredShellPreference::PowerShellCore), - &detected, - &detected[0], + |shell_type| find_in(&detected, shell_type), ); assert_eq!(resolved.shell_type, ShellType::PowerShell); @@ -255,10 +281,9 @@ mod tests { ), shell("Git Bash", "D:\\Tools\\Git\\bin\\bash.exe", ShellType::Bash), ]; - let resolved = select_windows_local_exec_shell( + let resolved = resolve_windows_local_exec_shell_with( Some(ConfiguredShellPreference::Bash), - &detected, - &detected[0], + |shell_type| find_in(&detected, shell_type), ); assert_eq!(resolved.shell_type, ShellType::Bash); @@ -278,10 +303,9 @@ mod tests { ShellType::PowerShell, ), ]; - let resolved = select_windows_local_exec_shell( + let resolved = resolve_windows_local_exec_shell_with( Some(ConfiguredShellPreference::Fish), - &detected, - &detected[0], + |shell_type| find_in(&detected, shell_type), ); assert_eq!(resolved.shell_type, ShellType::PowerShell); @@ -297,7 +321,9 @@ mod tests { ShellType::PowerShell, ), ]; - let resolved = select_windows_local_exec_shell(None, &detected, &detected[0]); + let resolved = resolve_windows_local_exec_shell_with(None, |shell_type| { + find_in(&detected, shell_type) + }); assert_eq!(resolved.shell_type, ShellType::PowerShell); } @@ -308,13 +334,58 @@ mod tests { shell("Bash", "/bin/bash", ShellType::Bash), shell("Zsh", "/bin/zsh", ShellType::Zsh), ]; - let resolved = select_non_windows_local_exec_shell( + let resolved = resolve_non_windows_local_exec_shell_with( Some(ConfiguredShellPreference::Zsh), - &detected, - &detected[0], + |shell_type| find_in(&detected, shell_type), + || detected[0].clone(), ); assert_eq!(resolved.shell_type, ShellType::Zsh); assert_eq!(resolved.path, PathBuf::from("/bin/zsh")); } + + #[test] + fn windows_selected_powershell_stops_before_pwsh_and_bash_fallbacks() { + let expected = shell( + "Windows PowerShell", + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", + ShellType::PowerShell, + ); + let mut requested = Vec::new(); + + let resolved = resolve_windows_local_exec_shell_with( + Some(ConfiguredShellPreference::PowerShell), + |shell_type| { + requested.push(shell_type.clone()); + (shell_type == ShellType::PowerShell).then(|| expected.clone()) + }, + ); + + assert_eq!(resolved, expected); + assert_eq!(requested, vec![ShellType::PowerShell]); + } + + #[test] + fn windows_missing_selection_stops_at_first_available_fallback() { + let expected = shell( + "Windows PowerShell", + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", + ShellType::PowerShell, + ); + let mut requested = Vec::new(); + + let resolved = resolve_windows_local_exec_shell_with( + Some(ConfiguredShellPreference::PowerShellCore), + |shell_type| { + requested.push(shell_type.clone()); + (shell_type == ShellType::PowerShell).then(|| expected.clone()) + }, + ); + + assert_eq!(resolved, expected); + assert_eq!( + requested, + vec![ShellType::PowerShellCore, ShellType::PowerShell] + ); + } } diff --git a/src/crates/services/terminal/src/shell/detection/cache.rs b/src/crates/services/terminal/src/shell/detection/cache.rs new file mode 100644 index 0000000000..16471befaf --- /dev/null +++ b/src/crates/services/terminal/src/shell/detection/cache.rs @@ -0,0 +1,244 @@ +use std::hash::{Hash, Hasher}; +use std::path::Path; +use std::sync::{Arc, Mutex, OnceLock}; +use std::time::{Duration, Instant, UNIX_EPOCH}; + +use dashmap::DashMap; + +use super::{path, ShellType}; + +const FAILED_PROBE_CACHE_TTL: Duration = Duration::from_secs(5); + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum CandidateProbeOutcome { + Available(Option), + AvailableWithProbeFailure, + Unavailable, +} + +#[derive(Clone, Debug, Eq)] +struct CandidateCacheKey { + shell_type: ShellType, + path_identity: String, +} + +impl PartialEq for CandidateCacheKey { + fn eq(&self, other: &Self) -> bool { + self.shell_type == other.shell_type && self.path_identity == other.path_identity + } +} + +impl Hash for CandidateCacheKey { + fn hash(&self, state: &mut H) { + self.shell_type.hash(state); + self.path_identity.hash(state); + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct FileFingerprint { + len: u64, + modified: Option<(u64, u32)>, +} + +#[derive(Clone, Debug)] +struct CachedCandidateProbe { + fingerprint: Option, + outcome: CandidateProbeOutcome, + retry_after: Option, +} + +type CandidateProbeSlot = Arc>>; + +fn cache() -> &'static DashMap { + static CACHE: OnceLock> = OnceLock::new(); + CACHE.get_or_init(DashMap::new) +} + +pub(super) fn probe_candidate( + shell_type: &ShellType, + executable: &Path, + probe: impl FnOnce() -> CandidateProbeOutcome, +) -> CandidateProbeOutcome { + let key = CandidateCacheKey { + shell_type: shell_type.clone(), + path_identity: path::normalized_path_identity(executable), + }; + let slot = cache() + .entry(key) + .or_insert_with(|| Arc::new(Mutex::new(None))) + .clone(); + let mut cached = slot.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + let fingerprint = file_fingerprint(executable); + let now = Instant::now(); + + if let Some(entry) = cached.as_ref().filter(|entry| { + entry.fingerprint == fingerprint + && (matches!(&entry.outcome, CandidateProbeOutcome::Available(_)) + || entry + .retry_after + .is_some_and(|retry_after| retry_after > now)) + }) { + return entry.outcome.clone(); + } + + let outcome = if fingerprint.is_some() { + probe() + } else { + CandidateProbeOutcome::Unavailable + }; + let retry_after = matches!( + &outcome, + CandidateProbeOutcome::AvailableWithProbeFailure | CandidateProbeOutcome::Unavailable + ) + .then(|| now + FAILED_PROBE_CACHE_TTL); + *cached = Some(CachedCandidateProbe { + fingerprint, + outcome: outcome.clone(), + retry_after, + }); + outcome +} + +pub(super) fn invalidate_path(executable: &Path) { + let path_identity = path::normalized_path_identity(executable); + cache().retain(|key, _| key.path_identity != path_identity); +} + +fn file_fingerprint(path: &Path) -> Option { + let metadata = path.metadata().ok()?; + if !metadata.is_file() { + return None; + } + let modified = metadata + .modified() + .ok() + .and_then(|modified| modified.duration_since(UNIX_EPOCH).ok()) + .map(|duration| (duration.as_secs(), duration.subsec_nanos())); + Some(FileFingerprint { + len: metadata.len(), + modified, + }) +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Barrier}; + use std::thread; + use std::time::Duration; + + use tempfile::tempdir; + + use super::{invalidate_path, probe_candidate, CandidateProbeOutcome}; + use crate::shell::ShellType; + + #[test] + fn successful_probe_is_reused_until_the_file_changes() { + let directory = tempdir().expect("temporary directory"); + let executable = directory.path().join("pwsh"); + fs::write(&executable, b"first").expect("write candidate"); + let probes = AtomicUsize::new(0); + + let first = probe_candidate(&ShellType::PowerShellCore, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(Some("7.5.0".to_string())) + }); + let cached = probe_candidate(&ShellType::PowerShellCore, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(Some("unexpected".to_string())) + }); + fs::write(&executable, b"second-version").expect("replace candidate"); + let refreshed = probe_candidate(&ShellType::PowerShellCore, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(Some("7.6.0".to_string())) + }); + invalidate_path(&executable); + let invalidated = probe_candidate(&ShellType::PowerShellCore, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(Some("7.7.0".to_string())) + }); + + assert_eq!( + first, + CandidateProbeOutcome::Available(Some("7.5.0".to_string())) + ); + assert_eq!(cached, first); + assert_eq!( + refreshed, + CandidateProbeOutcome::Available(Some("7.6.0".to_string())) + ); + assert_eq!( + invalidated, + CandidateProbeOutcome::Available(Some("7.7.0".to_string())) + ); + assert_eq!(probes.load(Ordering::Relaxed), 3); + } + + #[test] + fn failed_probe_is_short_cached_but_file_changes_retry_immediately() { + let directory = tempdir().expect("temporary directory"); + let executable = directory.path().join("bash"); + fs::write(&executable, b"first").expect("write candidate"); + let probes = AtomicUsize::new(0); + + assert_eq!( + probe_candidate(&ShellType::Bash, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Unavailable + }), + CandidateProbeOutcome::Unavailable + ); + assert_eq!( + probe_candidate(&ShellType::Bash, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(None) + }), + CandidateProbeOutcome::Unavailable + ); + fs::write(&executable, b"second-version").expect("replace candidate"); + assert_eq!( + probe_candidate(&ShellType::Bash, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + CandidateProbeOutcome::Available(None) + }), + CandidateProbeOutcome::Available(None) + ); + assert_eq!(probes.load(Ordering::Relaxed), 2); + } + + #[test] + fn concurrent_requests_share_one_probe() { + let directory = tempdir().expect("temporary directory"); + let executable = directory.path().join("concurrent-pwsh"); + fs::write(&executable, b"candidate").expect("write candidate"); + let probes = Arc::new(AtomicUsize::new(0)); + let start = Arc::new(Barrier::new(3)); + + let handles = (0..2) + .map(|_| { + let executable = executable.clone(); + let probes = Arc::clone(&probes); + let start = Arc::clone(&start); + thread::spawn(move || { + start.wait(); + probe_candidate(&ShellType::PowerShellCore, &executable, || { + probes.fetch_add(1, Ordering::Relaxed); + thread::sleep(Duration::from_millis(50)); + CandidateProbeOutcome::Available(Some("7.5.0".to_string())) + }) + }) + }) + .collect::>(); + start.wait(); + + for handle in handles { + assert_eq!( + handle.join().expect("probe thread"), + CandidateProbeOutcome::Available(Some("7.5.0".to_string())) + ); + } + assert_eq!(probes.load(Ordering::Relaxed), 1); + } +} diff --git a/src/crates/services/terminal/src/shell/detection/mod.rs b/src/crates/services/terminal/src/shell/detection/mod.rs index ea7cdf6a6e..18eb2270da 100644 --- a/src/crates/services/terminal/src/shell/detection/mod.rs +++ b/src/crates/services/terminal/src/shell/detection/mod.rs @@ -1,12 +1,13 @@ //! Shell discovery facade and shared candidate metadata. use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use super::ShellType; +mod cache; mod path; mod platform; mod probe; @@ -14,6 +15,10 @@ mod selection; const VERSION_PROBE_TIMEOUT_MS: u64 = 750; +pub(crate) fn invalidate_cached_executable(path: &Path) { + cache::invalidate_path(path); +} + /// The source that produced a shell candidate. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -111,19 +116,38 @@ impl ShellDetector { .collect() } + fn validate_first_candidate(candidates: Vec) -> Option { + let mut seen = HashSet::new(); + candidates + .into_iter() + .filter(|candidate| seen.insert(path::candidate_identity(candidate))) + .find_map(Self::validate_candidate) + } + fn validate_candidate(candidate: ShellCandidate) -> Option { - if !path::is_regular_file(&candidate.path) { - return None; - } - let version = match candidate.shell_type { - ShellType::PowerShellCore => Some(Self::probe_powershell_version(&candidate.path)?), - ShellType::Bash - | ShellType::Zsh - | ShellType::Fish - | ShellType::Sh - | ShellType::Ksh - | ShellType::Csh => Self::probe_shell_version(&candidate.path), - ShellType::PowerShell | ShellType::Cmd | ShellType::Custom(_) => None, + let outcome = + cache::probe_candidate(&candidate.shell_type, &candidate.path, || match &candidate + .shell_type + { + ShellType::PowerShellCore => Self::probe_powershell_version(&candidate.path) + .map(|version| cache::CandidateProbeOutcome::Available(Some(version))) + .unwrap_or(cache::CandidateProbeOutcome::Unavailable), + ShellType::Bash + | ShellType::Zsh + | ShellType::Fish + | ShellType::Sh + | ShellType::Ksh + | ShellType::Csh => Self::probe_shell_version(&candidate.path) + .map(|version| cache::CandidateProbeOutcome::Available(Some(version))) + .unwrap_or(cache::CandidateProbeOutcome::AvailableWithProbeFailure), + ShellType::PowerShell | ShellType::Cmd | ShellType::Custom(_) => { + cache::CandidateProbeOutcome::Available(None) + } + }); + let version = match outcome { + cache::CandidateProbeOutcome::Available(version) => version, + cache::CandidateProbeOutcome::AvailableWithProbeFailure => None, + cache::CandidateProbeOutcome::Unavailable => return None, }; Some(DetectedShell::new( candidate.shell_type, diff --git a/src/crates/services/terminal/src/shell/detection/platform.rs b/src/crates/services/terminal/src/shell/detection/platform.rs index 4fce1406b5..39e937977b 100644 --- a/src/crates/services/terminal/src/shell/detection/platform.rs +++ b/src/crates/services/terminal/src/shell/detection/platform.rs @@ -105,28 +105,37 @@ pub(super) fn windows_pwsh_location_candidates( #[cfg(not(windows))] pub(super) fn posix_shell_candidates() -> Vec { - let mut candidates = Vec::new(); - for shell_type in [ + [ ShellType::Bash, ShellType::Zsh, ShellType::Fish, ShellType::Sh, - ] { - let executable = shell_type.default_executable(); - candidates.extend( - ShellDetector::find_all_in_path(executable) - .into_iter() - .map(|path| { - ShellCandidate::new(path, shell_type.clone(), ShellDiscoverySource::Path) - }), - ); - for directory in ["/usr/local/bin", "/usr/bin", "/bin"] { - candidates.push(ShellCandidate::new( - PathBuf::from(directory).join(executable), - shell_type.clone(), - ShellDiscoverySource::SystemInstall, - )); - } + ] + .into_iter() + .flat_map(|shell_type| posix_shell_candidates_for(&shell_type)) + .collect() +} + +#[cfg(not(windows))] +pub(super) fn posix_shell_candidates_for(shell_type: &ShellType) -> Vec { + if !matches!( + shell_type, + ShellType::Bash | ShellType::Zsh | ShellType::Fish | ShellType::Sh + ) { + return Vec::new(); + } + + let executable = shell_type.default_executable(); + let mut candidates = ShellDetector::find_all_in_path(executable) + .into_iter() + .map(|path| ShellCandidate::new(path, shell_type.clone(), ShellDiscoverySource::Path)) + .collect::>(); + for directory in ["/usr/local/bin", "/usr/bin", "/bin"] { + candidates.push(ShellCandidate::new( + PathBuf::from(directory).join(executable), + shell_type.clone(), + ShellDiscoverySource::SystemInstall, + )); } candidates } diff --git a/src/crates/services/terminal/src/shell/detection/selection.rs b/src/crates/services/terminal/src/shell/detection/selection.rs index d54706b352..9c818dbdea 100644 --- a/src/crates/services/terminal/src/shell/detection/selection.rs +++ b/src/crates/services/terminal/src/shell/detection/selection.rs @@ -1,16 +1,17 @@ use std::path::PathBuf; -use super::{path, DetectedShell, ShellCandidate, ShellDetector, ShellDiscoverySource, ShellType}; +use super::{ + path, platform, DetectedShell, ShellCandidate, ShellDetector, ShellDiscoverySource, ShellType, +}; impl ShellDetector { /// Return the preferred local default shell for the current platform. pub fn get_default_shell() -> DetectedShell { #[cfg(windows)] { - let shells = Self::detect_available_shells(); - return Self::find_in_detected(&shells, &ShellType::PowerShellCore) - .or_else(|| Self::find_in_detected(&shells, &ShellType::PowerShell)) - .or_else(|| Self::find_in_detected(&shells, &ShellType::Cmd)) + return Self::find_shell(&ShellType::PowerShellCore) + .or_else(|| Self::find_shell(&ShellType::PowerShell)) + .or_else(|| Self::find_shell(&ShellType::Cmd)) .unwrap_or_else(|| { DetectedShell::fallback( ShellType::Cmd, @@ -26,9 +27,8 @@ impl ShellDetector { return shell; } } - let shells = Self::detect_available_shells(); - Self::find_in_detected(&shells, &ShellType::Bash) - .or_else(|| Self::find_in_detected(&shells, &ShellType::Sh)) + Self::find_shell(&ShellType::Bash) + .or_else(|| Self::find_shell(&ShellType::Sh)) .unwrap_or_else(|| { DetectedShell::fallback(ShellType::Sh, PathBuf::from("/bin/sh"), "sh") }) @@ -36,14 +36,51 @@ impl ShellDetector { } pub fn find_shell(shell_type: &ShellType) -> Option { - Self::find_in_detected(&Self::detect_available_shells(), shell_type) + #[cfg(windows)] + { + if matches!(shell_type, ShellType::Bash) { + return platform::detect_git_bash(); + } + return Self::validate_first_candidate(Self::candidates_for_shell(shell_type)); + } + #[cfg(not(windows))] + { + Self::validate_first_candidate(Self::candidates_for_shell(shell_type)) + } } + pub fn find_shell_by_id(id: &str) -> Option { - Self::detect_available_shells() + let shell_type = id + .split_once(':') + .and_then(|(shell_type, _)| Self::shell_type_from_preference(shell_type))?; + #[cfg(windows)] + if matches!(shell_type, ShellType::Bash) { + return platform::detect_git_bash().filter(|shell| shell.id == id); + } + Self::validate_candidates(Self::candidates_for_shell(&shell_type)) .into_iter() .find(|shell| shell.id == id) } + fn candidates_for_shell(shell_type: &ShellType) -> Vec { + #[cfg(windows)] + { + match shell_type { + ShellType::PowerShellCore => platform::windows_pwsh_candidates(), + ShellType::PowerShell => platform::windows_powershell_candidates(), + ShellType::Cmd => platform::windows_command_candidates(), + _ => Vec::new(), + } + } + #[cfg(not(windows))] + { + match shell_type { + ShellType::PowerShellCore => platform::non_windows_pwsh_candidates(), + _ => platform::posix_shell_candidates_for(shell_type), + } + } + } + pub fn resolve_explicit_shell(value: &str) -> Option { let path = PathBuf::from(value.trim()); if !path::is_regular_file(&path) { @@ -88,11 +125,4 @@ impl ShellDetector { _ => None, } } - - fn find_in_detected(shells: &[DetectedShell], shell_type: &ShellType) -> Option { - shells - .iter() - .find(|shell| &shell.shell_type == shell_type) - .cloned() - } } diff --git a/src/crates/services/terminal/src/shell/detection/tests.rs b/src/crates/services/terminal/src/shell/detection/tests.rs index 1c5fd47a5b..528d614687 100644 --- a/src/crates/services/terminal/src/shell/detection/tests.rs +++ b/src/crates/services/terminal/src/shell/detection/tests.rs @@ -91,6 +91,31 @@ fn detected_shell_id_is_stable_for_the_same_path() { assert_eq!(first.id, second.id); } +#[test] +fn cached_validation_keeps_the_current_discovery_source() { + let path = std::env::current_exe().expect("current test executable"); + let shell_type = ShellType::Custom("cached-source-test".to_string()); + let from_path = ShellDetector::validate_candidate(ShellCandidate::new( + path.clone(), + shell_type.clone(), + ShellDiscoverySource::Path, + )) + .expect("candidate discovered from PATH"); + let from_system = ShellDetector::validate_candidate(ShellCandidate::new( + path, + shell_type, + ShellDiscoverySource::SystemInstall, + )) + .expect("candidate discovered from system install"); + + assert_eq!(from_path.discovery_source, ShellDiscoverySource::Path); + assert_eq!( + from_system.discovery_source, + ShellDiscoverySource::SystemInstall + ); + assert_eq!(from_path.id, from_system.id); +} + #[test] fn normalized_identity_uses_canonical_path_when_available() { assert!(!path::normalized_path_identity( diff --git a/src/crates/services/terminal/src/shell/mod.rs b/src/crates/services/terminal/src/shell/mod.rs index aecba2c8c1..f5b0481b53 100644 --- a/src/crates/services/terminal/src/shell/mod.rs +++ b/src/crates/services/terminal/src/shell/mod.rs @@ -8,6 +8,7 @@ pub mod integration; mod profiles; mod scripts_manager; +pub(crate) use detection::invalidate_cached_executable; pub use detection::{DetectedShell, ShellDetector, ShellDiscoverySource}; pub use integration::{ get_injection_command, get_integration_script_content, get_integration_script_path, diff --git a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx index 0117a9a9b8..7eb3e8794f 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx @@ -10,6 +10,7 @@ import { useWorkspaceContext } from '@/infrastructure/contexts/WorkspaceContext' import { createWorktreeWorkspace, deleteWorktreeWorkspace, + WorktreeWorkspaceCreationError, } from '@/infrastructure/services/business/worktreeWorkspaceService'; import { useNavSceneStore } from '@/app/stores/navSceneStore'; import { useApp } from '@/app/hooks/useApp'; @@ -754,7 +755,7 @@ const WorkspaceItem: React.FC = ({ } catch (error) { notificationService.error( t( - result.openAfterCreate + error instanceof WorktreeWorkspaceCreationError && error.stage === 'open' ? 'nav.workspaces.worktreeCreateOrOpenFailed' : 'nav.workspaces.worktreeCreateFailed', { diff --git a/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.test.ts b/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.test.ts new file mode 100644 index 0000000000..072888182f --- /dev/null +++ b/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const addWorktree = vi.hoisted(() => vi.fn()); + +vi.mock('@/infrastructure/api/service-api/GitAPI', () => ({ + gitAPI: { addWorktree }, +})); + +import { + createWorktreeWorkspace, + WorktreeWorkspaceCreationError, +} from './worktreeWorkspaceService'; + +describe('createWorktreeWorkspace', () => { + beforeEach(() => { + addWorktree.mockReset(); + }); + + it('reports Git creation failures as create-stage errors', async () => { + addWorktree.mockRejectedValue(new Error('initial commit required')); + const openWorkspace = vi.fn(); + + const error = await createWorktreeWorkspace({ + repositoryPath: 'C:/repo', + branch: 'dev', + isNew: true, + openAfterCreate: true, + openWorkspace, + }).catch(error => error); + + expect(error).toBeInstanceOf(WorktreeWorkspaceCreationError); + expect(error.stage).toBe('create'); + expect(error.message).toBe('initial commit required'); + expect(openWorkspace).not.toHaveBeenCalled(); + }); + + it('reports workspace opening failures only after creation succeeds', async () => { + addWorktree.mockResolvedValue({ + path: 'C:/repo/.worktrees/dev', + branch: 'dev', + head: '1'.repeat(40), + isMain: false, + isLocked: false, + isPrunable: false, + }); + const openWorkspace = vi.fn().mockRejectedValue(new Error('open failed')); + + const error = await createWorktreeWorkspace({ + repositoryPath: 'C:/repo', + branch: 'dev', + isNew: true, + openAfterCreate: true, + openWorkspace, + }).catch(error => error); + + expect(error).toBeInstanceOf(WorktreeWorkspaceCreationError); + expect(error.stage).toBe('open'); + expect(error.message).toBe('open failed'); + expect(openWorkspace).toHaveBeenCalledWith('C:/repo/.worktrees/dev'); + }); +}); diff --git a/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.ts b/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.ts index 237cff44f4..b8dd1cedef 100644 --- a/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.ts +++ b/src/web-ui/src/infrastructure/services/business/worktreeWorkspaceService.ts @@ -15,6 +15,16 @@ export interface CreateWorktreeWorkspaceResult { openedWorkspace?: WorkspaceInfo; } +export class WorktreeWorkspaceCreationError extends Error { + constructor( + public readonly stage: 'create' | 'open', + error: unknown, + ) { + super(error instanceof Error ? error.message : String(error)); + this.name = 'WorktreeWorkspaceCreationError'; + } +} + export interface DeleteWorktreeWorkspaceOptions { workspace: WorkspaceInfo; closeWorkspaceById: (workspaceId: string) => Promise; @@ -23,17 +33,27 @@ export interface DeleteWorktreeWorkspaceOptions { export async function createWorktreeWorkspace( options: CreateWorktreeWorkspaceOptions, ): Promise { - const worktree = await gitAPI.addWorktree( - options.repositoryPath, - options.branch, - options.isNew, - ); + let worktree: GitWorktreeInfo; + try { + worktree = await gitAPI.addWorktree( + options.repositoryPath, + options.branch, + options.isNew, + ); + } catch (error) { + throw new WorktreeWorkspaceCreationError('create', error); + } if (!options.openAfterCreate) { return { worktree }; } - const openedWorkspace = await options.openWorkspace(worktree.path); + let openedWorkspace: WorkspaceInfo; + try { + openedWorkspace = await options.openWorkspace(worktree.path); + } catch (error) { + throw new WorktreeWorkspaceCreationError('open', error); + } return { worktree, openedWorkspace,