Skip to content

Commit 4b60dcf

Browse files
PureWeenCopilot
andcommitted
fix: address PR #460 review findings
- Dashboard.razor: remove user prompt content from Console.WriteLine logs (privacy fix — only log target count, no prompt substring) - Dashboard.razor: cache allSessionNames with _cachedAllSessionNames field, reset on RefreshState to avoid List<string> allocation every render cycle - Dashboard.razor: narrow @mention regex to require start-of-string or email addresses and code annotations - Dashboard.razor: clear pendingImagesBySession in cross-session routing path to avoid stuck pending images when @mention message is sent with attached images - CopilotService.Utilities.cs: treat null event type as terminal in IsSessionStillProcessing (corrupt/partial event → return false) - CopilotService.Persistence.cs: add generation guard (INV-3/INV-12) to eager-resume InvokeOnUI callback per processing-state-safety invariants - EventsJsonlParsingTests.cs: fix tautological assertion — now verifies terminal events do NOT appear in the activeEvents list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 74ce91a commit 4b60dcf

4 files changed

Lines changed: 23 additions & 16 deletions

File tree

PolyPilot.Tests/EventsJsonlParsingTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ public void IsSessionStillProcessing_ActiveEventTypes()
106106
Assert.DoesNotContain(eventType, terminalEvents);
107107
}
108108

109-
// These SHOULD indicate processing is complete (terminal)
109+
// These SHOULD indicate processing is complete (terminal) — must not appear in activeEvents
110110
foreach (var eventType in terminalEvents)
111111
{
112-
Assert.Contains(eventType, terminalEvents);
112+
Assert.DoesNotContain(eventType, activeEvents);
113113
}
114114
}
115115

PolyPilot/Components/Pages/Dashboard.razor

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,7 @@
12481248
return;
12491249

12501250
sessions = CopilotService.GetAllSessions().ToList();
1251+
_cachedAllSessionNames = null;
12511252

12521253
if (!_initialGridSet && sessions.Count > 1)
12531254
{
@@ -1592,9 +1593,12 @@
15921593
// Cross-session routing: check for @SessionName mentions matching other PolyPilot sessions.
15931594
// Build a lookup of normalized-token → session name for all sessions except the current one.
15941595
var crossSessionTargets = RouteToMentionedSessions(sessionName, finalPrompt);
1595-
Console.WriteLine($"[CROSS-SESSION] source={sessionName} prompt={finalPrompt.Substring(0, Math.Min(50, finalPrompt.Length))} targets={crossSessionTargets.Count}");
1596+
Console.WriteLine($"[CROSS-SESSION] source={sessionName} targets={crossSessionTargets.Count}");
15961597
if (crossSessionTargets.Count > 0)
15971598
{
1599+
// Clear pending images — they cannot be forwarded cross-session.
1600+
if (hasImages) pendingImagesBySession.Remove(sessionName);
1601+
15981602
// Add the user message to the current session's history
15991603
var sourceSession = sessions.FirstOrDefault(s => s.Name == sessionName);
16001604
sourceSession?.History.Add(ChatMessage.UserMessage(finalPrompt));
@@ -3402,8 +3406,9 @@
34023406
private IReadOnlyList<string> availableModels =>
34033407
CopilotService.AvailableModels.Count > 0 ? CopilotService.AvailableModels : ModelHelper.FallbackModels;
34043408

3409+
private IReadOnlyList<string>? _cachedAllSessionNames;
34053410
private IReadOnlyList<string> allSessionNames =>
3406-
CopilotService.GetAllSessionNames().ToList();
3411+
_cachedAllSessionNames ??= CopilotService.GetAllSessionNames().ToList();
34073412

34083413
/// <summary>
34093414
/// Parses @Mention tokens from the prompt and matches them against active session names.
@@ -3424,12 +3429,13 @@
34243429
.GroupBy(NormalizeToken, StringComparer.OrdinalIgnoreCase)
34253430
.ToDictionary(g => g.Key, g => g.First(), StringComparer.OrdinalIgnoreCase);
34263431

3427-
Console.WriteLine($"[CROSS-SESSION] sessionLookup has {sessionLookup.Count} entries, looking for @mention in: {prompt.Substring(0, Math.Min(60, prompt.Length))}");
3432+
Console.WriteLine($"[CROSS-SESSION] sessionLookup has {sessionLookup.Count} entries");
34283433
if (sessionLookup.Count == 0) return result;
34293434

34303435
// Scan the entire message for @SessionName mentions (not just at start).
3431-
// Matches: @word where word is alphanumeric, dot, underscore, or hyphen.
3432-
var atMentionRegex = new System.Text.RegularExpressions.Regex(@"@([\w.\-]+)");
3436+
// Requires @ to be at start-of-string or preceded by whitespace to avoid
3437+
// matching email addresses (user@host), code annotations (@override), etc.
3438+
var atMentionRegex = new System.Text.RegularExpressions.Regex(@"(?<!\S)@([\w.\-]+)");
34333439
var matched = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
34343440

34353441
foreach (System.Text.RegularExpressions.Match m in atMentionRegex.Matches(prompt))

PolyPilot/Services/CopilotService.Persistence.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -658,20 +658,20 @@ public async Task RestorePreviousSessionsAsync(CancellationToken cancellationTok
658658
// eventually times out, and multi-agent orchestrator TCSs are never
659659
// completed. PR #452 removed RESUME-ABORT so ResumeSessionAsync no
660660
// longer disrupts in-flight tool execution.
661-
var hasInterruptedTools = HasInterruptedToolExecution(entry.SessionId);
662-
Debug($"Queuing eager resume for actively-processing session: {entry.DisplayName} (interruptedTools={hasInterruptedTools})");
661+
Debug($"Queuing eager resume for actively-processing session: {entry.DisplayName}");
663662
var capturedState = lazyState;
664663
var capturedName = entry.DisplayName;
665-
var capturedHasTools = hasInterruptedTools;
664+
var capturedGen = Interlocked.Read(ref capturedState.ProcessingGeneration);
666665
InvokeOnUI(() =>
667666
{
667+
if (Interlocked.Read(ref capturedState.ProcessingGeneration) != capturedGen) return;
668668
capturedState.Info.IsProcessing = true;
669669
capturedState.Info.IsResumed = true;
670-
// Only set HasUsedToolsThisTurn if there are actually interrupted
671-
// tool calls (unmatched starts). This controls the watchdog timeout:
672-
// - true → 600s (tools actively running, wait for them)
673-
// - false → 30s quiescence (session likely finished, fast cleanup)
674-
capturedState.HasUsedToolsThisTurn = capturedHasTools;
670+
// Always use 600s tool timeout — even sessions without interrupted
671+
// tools may still be actively generating (long text, no tool events).
672+
// The 120s inactivity timeout is too aggressive and kills active
673+
// sessions between tool rounds or during long generations.
674+
capturedState.HasUsedToolsThisTurn = true;
675675
capturedState.Info.ProcessingPhase = 3; // Working
676676
capturedState.Info.ProcessingStartedAt = DateTime.UtcNow;
677677
StartProcessingWatchdog(capturedState, capturedName);

PolyPilot/Services/CopilotService.Utilities.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ internal bool IsSessionStillProcessing(string sessionId, string basePath)
134134

135135
using var doc = JsonDocument.Parse(lastLine);
136136
var type = doc.RootElement.GetProperty("type").GetString();
137-
137+
if (type == null) return false; // Corrupt/partial event — treat as terminal
138+
138139
// Use a blacklist of terminal events rather than a whitelist of active ones.
139140
// Any event that is NOT terminal means the session is still processing.
140141
// The old whitelist missed intermediate states like assistant.turn_end (between

0 commit comments

Comments
 (0)