Skip to content

fix(dgw): adopt recording file-type helpers - #1946

Open
Krista House (kristahouse) wants to merge 5 commits into
masterfrom
DGW-406
Open

fix(dgw): adopt recording file-type helpers#1946
Krista House (kristahouse) wants to merge 5 commits into
masterfrom
DGW-406

Conversation

@kristahouse

@kristahouse Krista House (kristahouse) commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Follow-up changes requested during review of #1857.

Centralize recording extension handling in streaming through RecordingFileType so validation, stream routing, and terminal input selection are derived from one mapping.

Define explicit MIME mappings for WebM, TRP, Asciicast, and SLOG artifacts, and declare PullRecordingFile response media types in source annotations so generated OpenAPI output remains aligned and stable.

Issue: DGW-406

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

Centralize recording extension handling in streaming through
RecordingFileType so validation, stream routing, and terminal input
selection are derived from a single mapping.

Define concrete MIME values for WebM, TRP, Asciicast, and SLOG pull
artifacts, and update PullRecordingFile OpenAPI annotations so generated
spec output remains aligned with source declarations.

Issue: DGW-406

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 24, 2026 17:43
@github-actions

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

@kristahouse

Copy link
Copy Markdown
Contributor Author

Implementation notes:

  • Refactored streaming file classification to use RecordingFileType::from_extension once, then map to streaming behavior in one place.
  • Collapsed terminal input selection into the same streaming decision path, removing duplicated file-type branching.
  • Completed recording MIME mapping for all RecordingFileType variants (webm, cast, rp, slog).
  • Updated PullRecordingFile utoipa response media types at source so generated gateway-api.yaml stays aligned after regeneration.

Testing:

  • cargo +nightly fmt --all
  • cargo clippy -p devolutions-gateway --tests -- -D warnings
  • Focused tests for streaming/type mapping and pull MIME behavior.
  • Manual smoke validation completed on debug build for .slog, .webm, and .cast playback/pull paths.

Breaking changes:

  • None.

Note

Human-tuned, LLM-assisted content.

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.

Pull request overview

Centralizes recording file-type handling and aligns download MIME types with the OpenAPI contract.

Changes:

  • Adds MIME mappings for all recording formats.
  • Routes streaming by RecordingFileType.
  • Documents recording response media types and adds tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
devolutions-gateway/src/token.rs Defines centralized recording MIME mappings.
devolutions-gateway/src/streaming.rs Derives streaming mode and terminal input type from recording type.
devolutions-gateway/src/api/jrec.rs Applies MIME mappings to downloads and OpenAPI annotations.
devolutions-gateway/openapi/gateway-api.yaml Adds generated response media types.

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

Comment thread devolutions-gateway/src/api/jrec.rs
Address PR feedback by declaring application/json in PullRecordingFile
response media types, matching routes that can serve recording.json.

Regenerate gateway-api.yaml from source annotations to keep generated
OpenAPI output synchronized with the contract definition.

Issue: DGW-406

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a filename-driven streaming regression test that validates routing
for .webm/.cast/.trp and rejection for unsupported or missing
extensions.

Expand recording content-type assertions with a table-driven check that
covers supported artifacts and confirms recording.json falls back to
ServeFile behavior.

Issue: DGW-406

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Could you also add a streaming.intent.md file along the streaming.rs module and write some human-vetted invariants about the concrete invariants and intentions you encoded when authoring this PR?

Comment thread devolutions-gateway/src/api/jrec.rs Outdated
Comment thread devolutions-gateway/src/api/jrec.rs Outdated
Comment thread devolutions-gateway/src/streaming.rs
Use application/octet-stream for TRP artifacts, make pull content type
selection explicit with fallback behavior, and document streamability
intent boundaries for /shadow streaming.

Also update PullRecordingFile media types and add module-level
streaming intent documentation requested in review.

Issue: DGW-406

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread devolutions-gateway/src/streaming.intent.md
Comment thread devolutions-gateway/src/streaming.intent.md Outdated
Remove the redundant introductory sentence from
streaming.intent.md so the document starts directly with scope,
matching the intent-file convention feedback.

Issue: DGW-406

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants