Skip to content

feat(dgw): keep session recording logs out of the playable file list - #2001

Closed
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 3 commits into
masterfrom
feat/jrec-concurrent-log-push
Closed

feat(dgw): keep session recording logs out of the playable file list#2001
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 3 commits into
masterfrom
feat/jrec-concurrent-log-push

Conversation

@irvingoujAtDevolution

@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) commented Sep 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Players and streamers treat every entry in recording.json files as something they can play. Released RDM even picks the log over the video when both are there. So once a recording with a video gets a .slog (live next to the video, or pushed afterwards, e.g. an AI-generated log from DVLS), older clients break.

This adds an additive logs list to the manifest:

  • A .slog pushed to a recording that has media goes to logs. files keeps only the media, so released players, /shadow and ZIP consumers behave the same.
  • A log-only recording (AD Console) keeps its .slog in files, exactly like today. logs is omitted when empty, so existing manifest shapes are byte-for-byte unchanged (pinned by manifest_shapes_round_trip_unchanged).
  • A recording can take one media push and one log push at the same time. A second push in the same category is still rejected.
  • Media drives the disconnect TTL and the required-recording kill. A late log push to a finished recording doesn't revive it or make /shadow stream the old video.
  • ZIP download includes logs.

Where to look hardest: handle_connect / handle_remove in recording.rs (who drives state, and cleanup when only the log push is left).

Producers: open the log push after the media push is up, or use a separate push token. A log push that connects before any media lands in files.

Tests: cargo test -p devolutions-gateway --lib (recording, jrec, token, streaming, session) 33/33, --test dvls_compatibility 24/24, clippy clean.

Stacked: #2002 caps the .slog push size.

🤖 Generated with Claude Code

@irvingoujAtDevolution @claude
feat(dgw): allow a session recording log push next to the media push
20b9a38 
A session can now push its event log (`.slog`) while its video or
terminal recording is being pushed. Each push writes its own file and
manifest entry. A second push of the same kind is still rejected.

The media stream keeps driving the disconnect window, the terminated
state and the recording policy, so a log push never ends a recording
or kills a session. Sessions that only push a log behave as before.

Shadow streaming uses the last media file, and the recording player
only plays WebM files.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 25, 2026

Copy link
Copy Markdown

Let maintainers know that an action is required on their side

  • Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

  • Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

  • Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

  • Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

Closed
@irvingoujAtDevolution @claude
refactor(dgw): store a sidecar session recording log in the manifest …
c5ec584 
…logs list

Released players and streamers treat every entry of `recording.json`
`files` as media. A `.slog` pushed while the recording has media now
goes to a new `logs` list instead, so `files` of a media recording only
holds media. A log-only recording keeps its `.slog` in `files`, and
`logs` is left out of the JSON when empty, so existing manifest shapes
are unchanged.

File names stay unique across both lists. The session ZIP download
also packs the files listed in `logs`.

Shadow streaming and the recording player no longer need to filter by
file type, so those changes are reverted.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@irvingoujAtDevolution @claude
fix(dgw): seed recording media state from the manifest
9abe582 
A log pushed after the media push ended (for example a `.slog`
generated after the session) creates a new in-memory recording entry.
That entry started with `has_media = false`, so the log push drove the
recording state: it was reported as connected, `/shadow` could stream
the finished media file, and the entry was never removed after the log
push disconnected.

The entry now reads `has_media` from the manifest, and starts as last
seen with no recording policy. Only a push that drives the recording
marks it connected and applies the TTL and policy.

Also covers the manifest shapes (media only, log only, media with a
sidecar log, and a manifest written by Gateway 2026.3.0) with exact
JSON round-trip tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) changed the title feat(dgw): allow a session recording log push next to the media push feat(dgw): keep session recording logs out of the playable file list Sep 25, 2026
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) marked this pull request as ready for review September 25, 2026 18:16
Copilot AI balanced review requested due to automatic review settings September 25, 2026 18:16
Copilot AI reviewed Sep 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Log disconnection can restart the cleanup TTL after the media deadline has already expired.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Separates session logs from playable media while preserving log-only manifest compatibility.

Changes:

  • Adds media/log recording categories and concurrent push handling.
  • Adds optional manifest logs entries and ZIP inclusion.
  • Adds lifecycle and compatibility tests.
File Description
devolutions-gateway/​src/​token.rs Classifies recording file types.
devolutions-gateway/​src/​session.rs Adds a test session-manager mock.
devolutions-gateway/​src/​recording.rs Manages separate media/log manifests and lifecycles.
devolutions-gateway/​src/​api/​jrec.rs Includes logs in recording ZIPs.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread devolutions-gateway/src/recording.rs
Comment on lines +831 to +834
// The log push can outlive the media stream, and its manifest entry is completed on disconnect.
if ongoing.has_connected_push() {
debug!(%id, "Media stream expired while a log push is still connected");
return;
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) deleted the feat/jrec-concurrent-log-push branch September 25, 2026 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@CBenoit Benoît Cortier (CBenoit) Awaiting requested review from CBenoit

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@irvingoujAtDevolution