-
Notifications
You must be signed in to change notification settings - Fork 76
Fix direct QA reporting + configurable retention + orphan recording tracking #6951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,8 @@ extension AppDelegate { | |
| notificationService: self.services.activityNotificationService, | ||
| screenRecorder: (routed.qaMode == true) ? ScreenRecorder() : nil, | ||
| reportToSessionId: routed.reportToSessionId, | ||
| qaMode: routed.qaMode ?? false | ||
| qaMode: routed.qaMode ?? false, | ||
| retentionDays: routed.retentionDays.flatMap { Int($0) } ?? 7 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| ) | ||
| // Don't bind relatedViewModel for escalated sessions — the active view model | ||
| // may be unrelated if the user switched threads. Tool calls for escalated | ||
|
|
@@ -139,13 +140,17 @@ extension AppDelegate { | |
| extractedText: $0.extractedText | ||
| ) | ||
| } | ||
| // Pass the active thread's conversation ID so the daemon can set reportToSessionId for QA sessions | ||
| let activeConversationId = self.mainWindow?.threadManager.activeViewModel?.sessionId | ||
|
|
||
| do { | ||
| try self.daemonClient.send(TaskSubmitMessage( | ||
| task: effectiveTask, | ||
| screenWidth: Int(screenBounds.width), | ||
| screenHeight: Int(screenBounds.height), | ||
| attachments: ipcAttachments, | ||
| source: submission.source | ||
| source: submission.source, | ||
| conversationId: activeConversationId | ||
| )) | ||
| } catch { | ||
| log.error("Failed to send task submit message: \(error)") | ||
|
|
@@ -199,7 +204,8 @@ extension AppDelegate { | |
| notificationService: self.services.activityNotificationService, | ||
| screenRecorder: (routed.qaMode == true) ? ScreenRecorder() : nil, | ||
| reportToSessionId: routed.reportToSessionId, | ||
| qaMode: routed.qaMode ?? false | ||
| qaMode: routed.qaMode ?? false, | ||
| retentionDays: routed.retentionDays.flatMap { Int($0) } ?? 7 | ||
| ) | ||
| // Don't bind relatedViewModel — sessions started via startSession() don't | ||
| // originate from a chat thread, so there's no ChatViewModel to extract | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Recording file untracked when reportToSessionId exists but conversation was deleted
When
meta?.reportToSessionIdis set butconversationStore.getConversation(reportSessionId)returns null (e.g., the user cleared sessions while the CU session was running), the recording file gets no file-backed attachment created.Root Cause
The new orphan-tracking code at line 337 checks
!(meta?.reportToSessionId), which isfalsewhen areportToSessionIdexists. But the normal attachment-creation path at line 256-287 only runs insideif (conversation)(line 243). When the conversation is missing, neither path creates an attachment:if (conversation)is false → attachment creation at line 258 is skippedelsebranch just logs a warning!(meta?.reportToSessionId)is false → orphan tracking is skippedResult: the recording file on disk is never registered in the attachment store, so the cleanup system cannot track or expire it.
Impact: Recording files accumulate on disk indefinitely for this edge case, causing a slow storage leak.
Was this helpful? React with 👍 or 👎 to provide feedback.