fix(ipc): ticks carry the volatile timestamps, so current-status stays known over the socket

[ChuckBuilds]

Fixes a bug from #735 (control socket stage 3, the state stream) found on the ledpi rig.

Bug

GET /api/v3/display/current-status returned {"mode": null, "plugin_id": null, "last_updated": null, "source": "socket"} once the same mode had been on screen for more than about 2 minutes, while the display was live. This hit a live game under live priority, Vegas, and a single plugin. On ledpi it returned nulls in 1059 of 1059 samples over 90 minutes. A direct state.get at the same time showed ncaa_fb_live with last_updated 0.15 s old.

Root cause

• StateHub leaves out of its version the keys a publisher marks as volatile. The display publishes display.last_updated that way, so a change to last_updated alone never sends a state event.
• The subscriber's keepalive tick carried only loop and served_at. So StateSubscription._store never refreshed display.last_updated, and the web's copy kept the timestamp of the last real change.
• display_state.current_status() treats last_updated older than 120 s (CURRENT_STATE_MAX_AGE_SECONDS) as unknown.

Other socket-fed readers:

• The plugin runtime section had the same bug. plugins.published_at is volatile, so with no plugin changing state, /plugins/state, /plugins/state/reconcile and the runtime in /plugins/installed read stale after its stale_after (180 s).
• /display/on-demand/status never nulls out: it recomputes remaining and doesn't judge age. Its last_updated was stale in the same way, and is now current too.
• /health reads only loop, which ticks already refreshed.

Fix

The server tells the reader the current timestamps. Readers keep their freshness rules as they are.

• Server: the short changed: false answer from StateHub.snapshot() (the result a tick carries, and a state.get with since) now includes volatile: {section: {key: value}}. These are the current values of each section's volatile keys: display.last_updated, on_demand.last_updated and remaining, and plugins.published_at. They are read under the same lock as the version, so they always belong to the version the reader has.
• Client: StateSubscription._store merges them into its copy on every tick with the same epoch and version. It takes only keys the section already has, so a tick never brings back a section that was dropped from a truncated snapshot.
• Readers: no changes to display_state or plugin_runtime. The cache fallback's 120 s rule is unchanged.

Freshness still follows the writers, so #726's verdicts hold:

• a render thread that stops publishing reads stalled once its heartbeat is 60 s old, and current-status reads unknown after 120 s;
• a runtime publisher that stops goes stale;
• a subscription that hears nothing for 15 s stops vouching for its copy, and the routes fall back to the cache.

I chose this over judging freshness by served_at plus the loop heartbeat in current_status. It fixes every socket reader in one place, and each section keeps its own writer as its proof of life. A display without this change sends no volatile, and the client then behaves as before.

Protocol: still version 1. volatile is a new optional field in the short answer.

Docs: docs/IPC_CONTROL_SOCKET.md and the contract docstrings are updated.

Tests

• test/test_state_stream_readers.py::TestFreshnessOverTicks runs the real DisplayController publish point, the runtime publisher, the hub and a StateSubscription on fake clocks, with ticks every 5 s:
  • the mode stays unchanged for 15 minutes, and current-status keeps returning it, with last_updated at most 5 s old. The plugins view stays live, and all of this comes from ticks: one snapshot in total;
  • the route returns the mode with source: socket after 10 minutes;
  • the render thread stops: plugins read stalled and the loop age is at least 60 s at +65 s, while current-status still shows the mode. At +185 s current-status is unknown;
  • the runtime publisher stops: plugins read stale;
  • ticks stop: latest() returns None and the route falls back to the cache.
• test/test_ipc_state_stream.py covers:
  • the hub's short answer carries the volatile values;
  • the subscription merges them, adds no section or key, and ignores a tick for another version or from an older display;
  • over a real socket, ticks carry the new last_updated and a live StateSubscription stays current without a new snapshot.
• Mutation check: with the client.py change reverted, 5 of the new tests fail (4 of them regression tests). The ticks-stop fallback test passes either way.
• WSL Ubuntu, as a normal user and as root: all 7 test_ipc_* files, test_state_stream_readers.py and the golden traces (test_run_loop_golden.py) give 315 passed, 2 skipped. The 2 skips are "Windows only" and "root may always connect" (as root).
• Full suite on Windows against an origin/main baseline worktree: the sorted FAILED/ERROR lists are identical (62 failed, 6 errors in both; these failures were already there). On the branch, 9 more pass and 2 more skip; the new socket tests skip on Windows.
• scripts/check_types.py with mypy 1.20.2: no issues in 94 modules.

I didn't touch the Pi rigs.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • State updates now carry refreshed timestamps and remaining-time information even when the underlying state version is unchanged.
  • Subscriptions apply these updates to cached state, keeping displayed status and runtime information current without treating each update as a new snapshot.
• Documentation
  • Updated control-socket examples to show volatile values in state responses and keepalive ticks.

[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: afa8fca9-f595-4e00-89bc-96da274be659

📥 Commits

Reviewing files that changed from the base of the PR and between ef69201 and 76b5a0e.

📒 Files selected for processing (7)

• CHANGELOG.md
• docs/IPC_CONTROL_SOCKET.md
• src/ipc/client.py
• src/ipc/contract.py
• src/ipc/server.py
• test/test_ipc_state_stream.py
• test/test_state_stream_readers.py

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

---

📝 Walkthrough

Walkthrough

The control socket now includes current volatile values in unchanged-version responses and ticks. Subscriptions merge matching values into cached state without changing the version. Tests cover timestamp freshness, status verdicts, and cached-status fallback.

Changes

Volatile State Stream

Layer / File(s)	Summary
Publish volatile values src/ipc/contract.py, src/ipc/server.py, docs/IPC_CONTROL_SOCKET.md, test/test_ipc_state_stream.py, CHANGELOG.md	StateHub stores volatile-key metadata and returns current volatile values in unchanged-version snapshots. The contract, documentation, changelog, and tests describe or verify these responses.
Merge volatile values into subscriptions src/ipc/client.py, test/test_ipc_state_stream.py, test/test_state_stream_readers.py	StateSubscription merges same-version tick values into existing cached keys. Tests cover refreshed values, freshness verdicts, and cached-status fallback.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant StateHub
  participant StateSubscription
  participant WebReader
  StateHub->>StateSubscription: Send unchanged-version tick with volatile values
  StateSubscription->>StateSubscription: Merge matching values into cached state
  WebReader->>StateSubscription: Read cached state
  StateSubscription-->>WebReader: Return refreshed state

Loading

Merge Risk: ⚪ Minimal · up to 76b5a

No actionable merge-blocking issue is identified; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 76b5a

The change refreshes reported status without granting new access. Its effects appear narrowly scoped, but mixed-version operation and recovery behavior are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The established exposure is this display process’s state and its socket-fed status readers. Current volatile publishers provide values already available in full snapshots; the inspected change adds no write command or credential grant. Wider deployed exposure is not established.

Trust Boundaries and Controls

• observed — The server checks available peer credentials before processing requests when peer checking is enabled. State read commands call snapshot rather than publish. These admission and authority paths are unchanged by the inspected IPC delta.

Resilience and Maintainability Implications

• observed — Ticks relay the last publisher-supplied timestamp rather than manufacturing a new proof of life. Regression assertions cover a stopped render thread becoming stalled and then unknown even while socket ticks continue, preserving the distinction between transport activity and writer freshness.

Hardening Proposals

• proposed — Consider validating nested snapshot identity types and explicitly defining whole-tick handling for version mismatches. This would strengthen the existing producer-trust contract, not remediate a demonstrated PR-introduced vulnerability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the IPC tick change that refreshes volatile timestamps so status remains known.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 26.47% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 5 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 18 complexity

Metric	Results
Complexity	18

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}