Fix direct QA reporting + configurable retention + orphan recording tracking#6951
Conversation
P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if (msg.recording && !(meta?.reportToSessionId)) { | ||
| try { | ||
| createFileBackedAttachment({ | ||
| filename: `qa-recording-${msg.sessionId}.mp4`, | ||
| mimeType: msg.recording.mimeType || 'video/mp4', | ||
| sizeBytes: msg.recording.sizeBytes, | ||
| filePath: msg.recording.localPath, | ||
| sha256: undefined, | ||
| expiresAt: msg.recording.expiresAt, | ||
| }); | ||
| log.info({ sessionId: msg.sessionId }, 'Created orphan file-backed attachment for cleanup tracking (no reportToSessionId)'); | ||
| } catch (err) { | ||
| log.error({ err, sessionId: msg.sessionId }, 'Failed to create file-backed attachment for orphan recording'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Recording file untracked when reportToSessionId exists but conversation was deleted
When meta?.reportToSessionId is set but conversationStore.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 is false when a reportToSessionId exists. But the normal attachment-creation path at line 256-287 only runs inside if (conversation) (line 243). When the conversation is missing, neither path creates an attachment:
- Line 243:
if (conversation)is false → attachment creation at line 258 is skipped - Line 327:
elsebranch just logs a warning - Line 337:
!(meta?.reportToSessionId)is false → orphan tracking is skipped
Result: 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.
| if (msg.recording && !(meta?.reportToSessionId)) { | |
| try { | |
| createFileBackedAttachment({ | |
| filename: `qa-recording-${msg.sessionId}.mp4`, | |
| mimeType: msg.recording.mimeType || 'video/mp4', | |
| sizeBytes: msg.recording.sizeBytes, | |
| filePath: msg.recording.localPath, | |
| sha256: undefined, | |
| expiresAt: msg.recording.expiresAt, | |
| }); | |
| log.info({ sessionId: msg.sessionId }, 'Created orphan file-backed attachment for cleanup tracking (no reportToSessionId)'); | |
| } catch (err) { | |
| log.error({ err, sessionId: msg.sessionId }, 'Failed to create file-backed attachment for orphan recording'); | |
| } | |
| } | |
| // Create a file-backed attachment for recordings without a reporting session | |
| // (or when the reporting conversation was deleted) so cleanup can track orphan files. | |
| if (msg.recording && !(meta?.reportToSessionId && conversationStore.getConversation(meta.reportToSessionId))) { | |
| try { | |
| createFileBackedAttachment({ | |
| filename: `qa-recording-${msg.sessionId}.mp4`, | |
| mimeType: msg.recording.mimeType || 'video/mp4', | |
| sizeBytes: msg.recording.sizeBytes, | |
| filePath: msg.recording.localPath, | |
| sha256: undefined, | |
| expiresAt: msg.recording.expiresAt, | |
| }); | |
| log.info({ sessionId: msg.sessionId }, 'Created orphan file-backed attachment for cleanup tracking (no reportToSessionId)'); | |
| } catch (err) { | |
| log.error({ err, sessionId: msg.sessionId }, 'Failed to create file-backed attachment for orphan recording'); | |
| } | |
| } | |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 416899300f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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.
Safely parse routed retentionDays before Int conversion
routed.retentionDays.flatMap { Int($0) } performs a trapping conversion, so a very large (or non-finite) retentionDays value from the daemon will crash the macOS client when a QA session is created; this is reachable because daemon config currently only requires qaRecording.defaultRetentionDays to be a positive integer, not bounded to Int range. Please use a non-trapping conversion with bounds checks/fallback (and apply the same guard to the identical conversion in the other session-construction path).
Useful? React with 👍 / 👎.
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ays (#6951) P1 #3: Add conversationId to TaskSubmit so direct CU QA sessions can set reportToSessionId. Client passes active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize. P1 #4: Add retentionDays to TaskRouted from daemon config. Client reads it instead of hardcoding 7 days. Also: create file-backed attachment for recordings without reportToSessionId so cleanup can track orphan files. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
conversationIdtoTaskSubmitso direct CU QA sessions can setreportToSessionId. Client passes the active thread ID. Daemon routes it to CU session metadata, enabling attachment creation on finalize.retentionDaystoTaskRoutedfrom daemon config (qaRecording.defaultRetentionDays). Client reads it instead of hardcoding7 * 24 * 3600.reportToSessionIdso cleanup can track orphan files.Test plan
conversationIdis sent intask_submittask_submitget attached to the originating chat threadretentionDaysflows from daemon config throughtask_routedto the client'sfinalizeQARecordingqaRecording.defaultRetentionDaysin config and verify the client uses the updated valuereportToSessionId) still get tracked as file-backed attachments for cleanupreportToSessionIdset tosourceSessionId🤖 Generated with Claude Code