fix(video): guard portal sessions during negotiation

[shermerL]

Problem

After CreateSession succeeds, cancelling while waiting for Start or failing OpenPipeWireRemote can leave the portal session open. SessionGuard is currently created only after negotiation completes.

Approach

Create the existing session guard immediately after CreateSession succeeds and retain it throughout negotiation. On success, transfer it to the capture loop, preserving the existing thread-join-before-session-close order.

Impact

• No public API changes.
• No wire-format changes.
• Limited to Linux portal screen capture.

Alternatives

Explicit close calls on error returns would not cover a dropped negotiation future. Reusing the existing guard covers both errors and cancellation after session creation.

Follow-ups

• Cancellation while CreateSession is still pending remains out of scope: the guard is installed only after a session is returned. If the caller cancels before then, a session created by the portal may be left without an owner to close it. Handling this requires separate request-cancellation and late-session cleanup.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8c5085ca-4b77-4281-9d5e-8728cd3abca0

📥 Commits

Reviewing files that changed from the base of the PR and between 9d2a4f6 and 93b8311.

📒 Files selected for processing (1)

• rs/moq-video/src/capture/pipewire.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

portal_negotiate now wraps the portal session in Arc and creates a SessionGuard from a clone. It returns that guard with the PipeWire node and file descriptor. SessionGuard::new now accepts the shared session handle.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 93b83

Negotiation failures clean up the portal session, and successful capture retains it until the worker has stopped. No actionable merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 93b83

The change improves cleanup when screen-capture negotiation fails or is cancelled after a session is created. It does not appear to expand capture permissions or expose a new entrypoint. Cancellation while session creation is still pending remains outside the fix.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected authority is the existing Linux portal screen-capture session and its PipeWire remote; no additional caller or authority-bearing session is evident in the traced handoff.

Trust Boundaries and Controls

• observed — After CreateSession returns, dropping negotiation on an error or cancellation drops its guard sender, allowing the spawned task to close that session. No guard exists while CreateSession itself is pending; that interval predates this revision.

Resilience and Maintainability Implications

• inferred — The earlier guard placement left post-creation negotiation exits without this explicit close path. The new placement improves containment for those exits, while successful capture retains the prior join-before-close ordering.

Hardening Proposals

• proposed — Separately establish the portal request-cancellation and late-result semantics for CreateSession, then provide cleanup if cancellation can leave a newly created session without an owner.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the fix: guarding portal sessions during video negotiation. It is concise and matches the main change.
Description check	✅ Passed	The description explains the session-leak problem, the guard-based approach, impact, alternatives, and out-of-scope cancellation case. It directly relates to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

MERGE

Positive improvement? Yes. After CreateSession succeeds, cancel or fail during Start / OpenPipeWireRemote previously left the portal session open because SessionGuard was only built at the end of negotiation. Installing the guard immediately after session creation closes that leak for both error returns and a dropped future.

Worth the complexity? Yes. +4/−2 in one Linux portal path, reusing the existing RAII guard. Wrapping in Arc so the close task and the live &session API calls can share ownership is the minimal change that preserves the join-before-close order once the guard moves into LoopGuard.

Different approach? Explicit close on each error path would miss cancellation of the negotiation future. The PR correctly prefers the guard. The noted follow-up (cancel while CreateSession is still pending) is a separate problem and rightfully out of scope.

CI is green. Ship it.

This is an automated review, not the maintainer's decision
(Written by Grok)