fix: prevent rshell server unbounded server memory growth

[k-ziomek]

No description provided.

[Copilot]

🟢 Ready to approve

The changes are focused, are covered by updated unit tests, and align with the goal of preventing unbounded output retention.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR addresses potential unbounded memory growth in the rshell_server by ensuring stored command outputs don’t accumulate indefinitely and are removed after retrieval or after a TTL expires.

Changes:

• Added timestamp tracking for stored command outputs and a TTL-based cleanup routine to evict stale entries.
• Updated get_output() to remove outputs (and timestamps) once consumed to prevent indefinite retention.
• Extended unit tests to cover timestamp bookkeeping, cleanup behavior, and queue removal on client disconnect.

File summaries

File	Description
mfd_connect/rshell_server.py	Adds output timestamp tracking + stale-output cleanup, and removes outputs after retrieval/disconnect.
tests/unit/test_mfd_connect/test_rshell_server.py	Updates tests to validate output eviction, timestamp handling, and stale cleanup behavior.

Review details

Comments suppressed due to low confidence (1)

mfd_connect/rshell_server.py:65

• For consistency with monotonic TTL comparisons, store timestamps using time.monotonic() (not time.time()).

def _store_output(command_id: str, output: str, rc: int) -> None:
    """Persist command output together with its insertion timestamp."""
    _cleanup_stale_outputs()
    output_queue[command_id] = output_object(output=output, rc=rc)
    output_queue_timestamps[command_id] = time.time()

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[coveralls-official]

coverage: 79.981% (+0.06%) from 79.925% — k-ziomek:task_MFD_8903 into intel:main

[Copilot]

🟡 Not ready to approve

The TTL cleanup uses a monotonic clock while stored timestamps use wall-clock time, which prevents stale entries from ever expiring in production.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

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

Suppressed comments (1)

mfd_connect/rshell_server/rshell_server.py:64

• _cleanup_stale_outputs() measures current_time with time.monotonic(), but _store_output() records created_at with time.time(). Mixing monotonic time with wall-clock time means current_time - created_at will typically be a large negative number, so stale entries will never be collected in production (defeating the memory-growth fix). Use the same clock source for both (prefer time.monotonic() for TTL calculations).

def _store_output(command_id: str, output: str, rc: int) -> None:
    """Persist command output together with its insertion timestamp."""
    _cleanup_stale_outputs()
    output_queue[command_id] = output_object(output=output, rc=rc)
    output_queue_timestamps[command_id] = time.time()

[Copilot]

Pull request overview

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

[adrianlasota]

fix the commits, there is merge commit