feat(session): spec mode — the renderer stops resolving policy and stops writing settings
C2 of design/client-architecture-split.md, and the last of the session's overreach.
`--resolved-spec <path>` hands the session everything it needs already resolved —
effective settings, the host's clipboard decision, the profile's name — and in that mode it
performs ZERO store reads. It had been re-deriving all three, which meant policy was being
evaluated inside the thing that draws pixels, and that the spawner and the child could
disagree about a file either of them might have written in between. First-party spawns
(the shells, the CLI) always pass one now.
The compat path stays for hand-run `punktfunk-session --connect` and old Decky scripts —
but it calls the SAME helper, so the two modes cannot drift; it is the identical function
invoked in-process instead of by the parent. A spec that is named but unreadable fails
loudly rather than quietly falling back: a spawner that asked for exact settings must not
get store-derived ones instead.
The match-window write-back is gone too. The callback used to load-modify-save the shared
settings file from inside the renderer — one of that file's five concurrent writers, for a
value only the parent needs. It now reports `{"window":{w,h}}` on stdout and the spawner
persists it, on a real change only. A hand-run session still persists its own window,
because nobody is listening to its stdout there and the event alone would drop the value.
Verified on .21: a spec naming a profile that doesn't exist in the catalog is honoured
(proving no lookup happened), a missing spec errors instead of falling back, and the CLI's
spec file is written and cleaned up per launch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -571,6 +571,8 @@ punktfunk:// link takes. Exit codes: 0 ok, 2 connect, 3 trust, 4 renderer, 5 not
|
||||
trust_rejected,
|
||||
} => failure = Some((msg, trust_rejected)),
|
||||
SessionEvent::Ended(reason) => eprintln!("{reason}"),
|
||||
// Persisted by the brain on the way past; nothing to report here.
|
||||
SessionEvent::Window { .. } => {}
|
||||
SessionEvent::Exited(code) => {
|
||||
return match failure {
|
||||
Some((msg, true)) => {
|
||||
|
||||
@@ -78,6 +78,12 @@ pub fn spawn_session(
|
||||
wake: false,
|
||||
connect_timeout_secs: opts.connect_timeout_secs,
|
||||
tofu,
|
||||
// The per-host clipboard decision, resolved here so the child doesn't look it up
|
||||
// again — matched by address, the way every other per-host lookup matches.
|
||||
clipboard: pf_client_core::trust::KnownHosts::load()
|
||||
.hosts
|
||||
.iter()
|
||||
.any(|h| h.addr == req.addr && h.port == req.port && h.clipboard_sync),
|
||||
};
|
||||
|
||||
let persist_paired = opts.persist_paired;
|
||||
@@ -96,6 +102,8 @@ pub fn spawn_session(
|
||||
trust_rejected,
|
||||
} => error = Some((msg, trust_rejected)),
|
||||
SessionEvent::Ended(msg) => ended = Some(msg),
|
||||
// The brain persists the window size; the shell has nothing to do with it.
|
||||
SessionEvent::Window { .. } => {}
|
||||
SessionEvent::Exited(code) => {
|
||||
let _ = sender.send(AppMsg::SessionExited {
|
||||
req: req.clone(),
|
||||
|
||||
@@ -183,7 +183,9 @@ pub fn run(target: Option<&str>) -> u8 {
|
||||
window_size: crate::session_main::window_size(&settings_at_start),
|
||||
// Latched at console start (like the stats tier above): toggling Match window in
|
||||
// the console's settings screen applies from the next console launch.
|
||||
match_window: crate::session_main::match_window(&settings_at_start),
|
||||
// The console owns its own window across every launch, and no parent is listening to
|
||||
// its stdout, so it keeps persisting the size itself.
|
||||
match_window: crate::session_main::match_window(&settings_at_start, true),
|
||||
render_scale: settings_at_start.render_scale,
|
||||
render_scale_max_dim: punktfunk_core::render_scale::max_dimension(&settings_at_start.codec),
|
||||
};
|
||||
@@ -218,6 +220,8 @@ pub fn run(target: Option<&str>) -> u8 {
|
||||
let mut params = session_params(
|
||||
&settings,
|
||||
profile.map(|p| p.name),
|
||||
// In-process launch: no spawner resolved a clipboard decision for us.
|
||||
None,
|
||||
addr.clone(),
|
||||
port,
|
||||
pin,
|
||||
|
||||
+56
-22
@@ -159,6 +159,7 @@ mod session_main {
|
||||
pub(crate) fn session_params(
|
||||
settings: &trust::Settings,
|
||||
profile: Option<String>,
|
||||
clipboard_override: Option<bool>,
|
||||
addr: String,
|
||||
port: u16,
|
||||
pin: [u8; 32],
|
||||
@@ -169,14 +170,16 @@ mod session_main {
|
||||
force_software: Arc<AtomicBool>,
|
||||
vulkan: Option<pf_client_core::video::VulkanDecodeDevice>,
|
||||
) -> SessionParams {
|
||||
// Per-host clipboard opt-in (design/clipboard-and-file-transfer.md §5.3), resolved
|
||||
// here rather than passed in so every caller — a direct connect and the console's
|
||||
// own launches — honors the same stored decision. `addr` is moved into the struct
|
||||
// Per-host clipboard opt-in (design/clipboard-and-file-transfer.md §5.3). In spec
|
||||
// mode the spawner already resolved it; otherwise this looks it up itself, which is
|
||||
// the last store read the compat path still owes. `addr` is moved into the struct
|
||||
// below, so read it first.
|
||||
let clipboard = trust::KnownHosts::load()
|
||||
.hosts
|
||||
.iter()
|
||||
.any(|h| h.addr == addr && h.port == port && h.clipboard_sync);
|
||||
let clipboard = clipboard_override.unwrap_or_else(|| {
|
||||
trust::KnownHosts::load()
|
||||
.hosts
|
||||
.iter()
|
||||
.any(|h| h.addr == addr && h.port == port && h.clipboard_sync)
|
||||
});
|
||||
// Re-apply the shell-persisted forwarded-controller pin (stable `vid:pid:name`
|
||||
// key) to OUR gamepad service — the shells' in-process services can't reach this
|
||||
// process. Applied per params-build (idempotent; browse re-launches included) so
|
||||
@@ -280,14 +283,24 @@ mod session_main {
|
||||
/// debounced resize→`Reconfigure` machinery on; the callback stores each resize-end's
|
||||
/// logical window size (load-modify-save, like the console settings screen) so the
|
||||
/// next launch opens at it.
|
||||
pub(crate) fn match_window(settings: &trust::Settings) -> Option<Box<dyn FnMut(u32, u32)>> {
|
||||
/// The Match-window policy hook (design/midstream-resolution-resize.md D1/D2). The
|
||||
/// callback used to load-modify-save the shared settings file from inside the renderer —
|
||||
/// one of that file's five concurrent writers, for a value only the parent needs. It now
|
||||
/// REPORTS the size on stdout and the spawner persists it
|
||||
/// (design/client-architecture-split.md §5).
|
||||
///
|
||||
/// `persist_locally` keeps a hand-run session remembering its own window: nobody is
|
||||
/// listening to stdout there, so the event alone would drop the value. A spawned session
|
||||
/// leaves the write to its parent, which is the whole point.
|
||||
pub(crate) fn match_window(
|
||||
settings: &trust::Settings,
|
||||
persist_locally: bool,
|
||||
) -> Option<Box<dyn FnMut(u32, u32)>> {
|
||||
settings.match_window.then(|| {
|
||||
Box::new(|w: u32, h: u32| {
|
||||
let mut s = trust::Settings::load();
|
||||
if (s.last_window_w, s.last_window_h) != (w, h) {
|
||||
s.last_window_w = w;
|
||||
s.last_window_h = h;
|
||||
s.save();
|
||||
Box::new(move |w: u32, h: u32| {
|
||||
println!("{{\"window\":{{\"w\":{w},\"h\":{h}}}}}");
|
||||
if persist_locally {
|
||||
pf_client_core::orchestrate::persist_window_size(w, h);
|
||||
}
|
||||
}) as Box<dyn FnMut(u32, u32)>
|
||||
})
|
||||
@@ -473,12 +486,31 @@ mod session_main {
|
||||
return EXIT_CONNECT_FAILED;
|
||||
}
|
||||
};
|
||||
// Global defaults with this host's settings profile overlaid — the binding on the
|
||||
// host record, or `--profile <id|name>` for a one-off (`--profile ""` forces the
|
||||
// defaults). Resolved through the shared helper, exactly like the console's launches.
|
||||
let (settings, profile) = trust::effective_settings(&addr, port, profile_arg().as_deref());
|
||||
if let Some(p) = &profile {
|
||||
tracing::info!(profile = %p.name, id = %p.id, "streaming with a settings profile");
|
||||
// `--resolved-spec <path>`: the spawner already did the resolving, so this process
|
||||
// performs ZERO store reads (design/client-architecture-split.md §5) — no Settings
|
||||
// load, no known-hosts lookup, no profile resolution. Without it (a hand-run
|
||||
// `--connect`, an old Decky script) the session resolves for itself through the SAME
|
||||
// helper, so the two modes cannot drift.
|
||||
let spec = arg_value("--resolved-spec").map(std::path::PathBuf::from);
|
||||
let (settings, profile_name, clipboard_override) = match &spec {
|
||||
Some(path) => match pf_client_core::orchestrate::ResolvedSpec::read(path) {
|
||||
Ok(s) => {
|
||||
tracing::info!(path = %path.display(), "running from a resolved spec");
|
||||
(s.settings, s.profile, Some(s.clipboard))
|
||||
}
|
||||
Err(e) => {
|
||||
json_line("error", &format!("resolved spec: {e}"), None);
|
||||
return EXIT_CONNECT_FAILED;
|
||||
}
|
||||
},
|
||||
None => {
|
||||
let (settings, profile) =
|
||||
trust::effective_settings(&addr, port, profile_arg().as_deref());
|
||||
(settings, profile.map(|p| p.name), None)
|
||||
}
|
||||
};
|
||||
if let Some(name) = &profile_name {
|
||||
tracing::info!(profile = %name, "streaming with a settings profile");
|
||||
}
|
||||
|
||||
// Trust follows the GTK client's `--connect` rules: a stored (or `--fp`) pin
|
||||
@@ -540,7 +572,8 @@ mod session_main {
|
||||
#[cfg(not(feature = "ui"))]
|
||||
overlay: None,
|
||||
window_size: window_size(&settings),
|
||||
match_window: match_window(&settings),
|
||||
// A spawned session (spec mode) reports its window; a hand-run one persists it.
|
||||
match_window: match_window(&settings, spec.is_none()),
|
||||
render_scale: settings.render_scale,
|
||||
render_scale_max_dim: punktfunk_core::render_scale::max_dimension(&settings.codec),
|
||||
};
|
||||
@@ -549,7 +582,8 @@ mod session_main {
|
||||
pf_presenter::run_session(opts, move |gamepad, native, force_software, vulkan| {
|
||||
session_params(
|
||||
&settings,
|
||||
profile.map(|p| p.name),
|
||||
profile_name,
|
||||
clipboard_override,
|
||||
addr,
|
||||
port,
|
||||
pin,
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
use crate::deeplink::{DeepLink, HostResolution, Route};
|
||||
use crate::profiles::{ProfilesFile, Resolution, StreamProfile};
|
||||
use crate::trust::{effective_settings, KnownHost, KnownHosts, Settings};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::process::{Child, Command, Stdio};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
@@ -79,6 +80,10 @@ pub struct ConnectPlan {
|
||||
/// The pin came from an advert rather than the store: persist it once the session reports
|
||||
/// ready (ready proves the host really holds that identity).
|
||||
pub tofu: bool,
|
||||
/// Share this machine's clipboard with this host — a trust decision about the HOST, so it
|
||||
/// lives on the record rather than in a profile, and the spawner resolves it once here
|
||||
/// instead of the renderer looking it up again.
|
||||
pub clipboard: bool,
|
||||
}
|
||||
|
||||
impl ConnectPlan {
|
||||
@@ -101,6 +106,7 @@ impl ConnectPlan {
|
||||
settings,
|
||||
connect_timeout_secs: None,
|
||||
tofu: false,
|
||||
clipboard: host.clipboard_sync,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -140,6 +146,17 @@ impl ConnectPlan {
|
||||
settings,
|
||||
connect_timeout_secs: None,
|
||||
tofu: false,
|
||||
clipboard: host.clipboard_sync,
|
||||
}
|
||||
}
|
||||
|
||||
/// This plan as a [`ResolvedSpec`] — what a first-party spawner hands the session so it
|
||||
/// performs no store reads of its own.
|
||||
pub fn spec(&self, clipboard: bool) -> ResolvedSpec {
|
||||
ResolvedSpec {
|
||||
settings: self.settings.clone(),
|
||||
clipboard,
|
||||
profile: self.profile.as_ref().map(|p| p.name.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -448,6 +465,49 @@ pub enum ConnectOutcome {
|
||||
// Session spawn + the stdout contract.
|
||||
// ---------------------------------------------------------------------------------------
|
||||
|
||||
/// Everything a session needs, resolved by the caller — the spec `--resolved-spec` carries
|
||||
/// (design/client-architecture-split.md §5).
|
||||
///
|
||||
/// The session binary is a renderer: given this, it performs ZERO store reads. Its old habit of
|
||||
/// re-deriving state (loading `Settings`, looking up the host's `clipboard_sync`, resolving the
|
||||
/// profile) meant policy was being evaluated inside the thing that draws pixels, and that the
|
||||
/// spawner and the child could disagree about a file either of them might have written since.
|
||||
///
|
||||
/// The compat path — a hand-run `punktfunk-session --connect` with no spec — still resolves for
|
||||
/// itself, but through the *same* helper (`effective_settings`), so the two modes cannot drift.
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
pub struct ResolvedSpec {
|
||||
/// Effective settings: the global defaults with the chosen profile already applied.
|
||||
pub settings: Settings,
|
||||
/// Whether this host may share the clipboard — a per-host trust decision, resolved by the
|
||||
/// spawner rather than re-looked-up here.
|
||||
pub clipboard: bool,
|
||||
/// The profile's name, for the stats overlay. `None` = the global defaults.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub profile: Option<String>,
|
||||
}
|
||||
|
||||
impl ResolvedSpec {
|
||||
/// Write the spec somewhere the child can read it, returning the path. Temp files rather
|
||||
/// than a pipe because the session already takes a file path elsewhere and a crashed
|
||||
/// spawner leaves something inspectable; the name carries the pid so concurrent launches
|
||||
/// (a shell and a CLI, or two Decky invocations) never overwrite each other's spec.
|
||||
pub fn write_temp(&self) -> std::io::Result<std::path::PathBuf> {
|
||||
let path = std::env::temp_dir().join(format!("punktfunk-spec-{}.json", std::process::id()));
|
||||
let json = serde_json::to_vec_pretty(self)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
std::fs::write(&path, json)?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// Read a spec written by the spawner.
|
||||
pub fn read(path: &std::path::Path) -> std::io::Result<ResolvedSpec> {
|
||||
let text = std::fs::read_to_string(path)?;
|
||||
serde_json::from_str(&text)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
|
||||
}
|
||||
}
|
||||
|
||||
/// One event from the session child's stdout contract (`{"ready":true}`, `{"error":…}`,
|
||||
/// `{"ended":…}`, then EOF and an exit code). Parsed in one place so a shell, the console and
|
||||
/// the CLI cannot disagree about what "ready" or "trust rejected" means.
|
||||
@@ -460,6 +520,13 @@ pub enum SessionEvent {
|
||||
trust_rejected: bool,
|
||||
},
|
||||
Ended(String),
|
||||
/// The session window's logical size settled at this, under the match-window policy. The
|
||||
/// SPAWNER persists it (design §5): a renderer that load-modify-saves the shared settings
|
||||
/// file was one of its five concurrent writers, for a value only the parent needs.
|
||||
Window {
|
||||
w: u32,
|
||||
h: u32,
|
||||
},
|
||||
/// EOF: the child is gone. `-1` = killed by a signal.
|
||||
Exited(i32),
|
||||
}
|
||||
@@ -480,9 +547,26 @@ pub fn parse_session_line(line: &str) -> Option<SessionEvent> {
|
||||
if let Some(msg) = v.get("ended").and_then(|m| m.as_str()) {
|
||||
return Some(SessionEvent::Ended(msg.to_string()));
|
||||
}
|
||||
if let Some(win) = v.get("window") {
|
||||
let dim = |k: &str| win.get(k).and_then(|n| n.as_u64()).map(|n| n as u32);
|
||||
if let (Some(w), Some(h)) = (dim("w"), dim("h")) {
|
||||
return Some(SessionEvent::Window { w, h });
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Persist a window size the session reported. The spawner's job now, not the renderer's — and
|
||||
/// it writes only on a real change, so a session that never resizes never touches the file.
|
||||
pub fn persist_window_size(w: u32, h: u32) {
|
||||
let mut s = Settings::load();
|
||||
if (s.last_window_w, s.last_window_h) != (w, h) {
|
||||
s.last_window_w = w;
|
||||
s.last_window_h = h;
|
||||
s.save();
|
||||
}
|
||||
}
|
||||
|
||||
/// The session binary: installed next to this executable, else `$PATH` (a dev run out of
|
||||
/// `target/…` lands on the sibling).
|
||||
pub fn session_binary() -> std::path::PathBuf {
|
||||
@@ -525,7 +609,23 @@ pub fn spawn_session(
|
||||
on_event: impl FnMut(SessionEvent) + Send + 'static,
|
||||
) -> Result<CancelHandle, String> {
|
||||
let mut cmd = Command::new(session_binary());
|
||||
cmd.args(plan.session_args())
|
||||
let mut args = plan.session_args();
|
||||
// Spec mode: hand the child the settings we already resolved, so it reads no stores and
|
||||
// cannot disagree with us about a file either of us might write (design §5). A spec we
|
||||
// fail to write is not fatal — the child's compat path resolves the same values through
|
||||
// the same helper, which is exactly why that path was kept.
|
||||
let spec_path = match plan.spec(plan.clipboard).write_temp() {
|
||||
Ok(path) => {
|
||||
args.push("--resolved-spec".into());
|
||||
args.push(path.to_string_lossy().into_owned());
|
||||
Some(path)
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(error = %e, "couldn't write the resolved spec; the session will resolve for itself");
|
||||
None
|
||||
}
|
||||
};
|
||||
cmd.args(args)
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::inherit()); // the session's logs interleave with the front-end's
|
||||
@@ -550,9 +650,19 @@ pub fn spawn_session(
|
||||
for line in std::io::BufReader::new(stdout).lines() {
|
||||
let Ok(line) = line else { break };
|
||||
if let Some(ev) = parse_session_line(&line) {
|
||||
// The window size is the spawner's to persist — the renderer only reports
|
||||
// it. Front-ends still see the event; they just don't have to act on it.
|
||||
if let SessionEvent::Window { w, h } = ev {
|
||||
persist_window_size(w, h);
|
||||
}
|
||||
on_event(ev);
|
||||
}
|
||||
}
|
||||
// The spec has done its job the moment the child has read it; a leftover temp file
|
||||
// in %TEMP% is litter, and one per launch adds up.
|
||||
if let Some(path) = &spec_path {
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
// EOF — reap (a cancel-killed child lands here too; -1 = died on a signal).
|
||||
let code = reader_slot
|
||||
.0
|
||||
@@ -675,6 +785,7 @@ mod tests {
|
||||
wake: true,
|
||||
connect_timeout_secs: None,
|
||||
tofu: false,
|
||||
clipboard: false,
|
||||
};
|
||||
assert_eq!(
|
||||
plan.session_args(),
|
||||
@@ -807,6 +918,65 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
/// The spec is the whole of what a session needs, and it round-trips — a field lost here
|
||||
/// is a setting the stream silently doesn't get.
|
||||
#[test]
|
||||
fn resolved_spec_round_trips() {
|
||||
let spec = ResolvedSpec {
|
||||
settings: Settings {
|
||||
width: 2560,
|
||||
height: 1440,
|
||||
bitrate_kbps: 55000,
|
||||
codec: "av1".into(),
|
||||
..Default::default()
|
||||
},
|
||||
clipboard: true,
|
||||
profile: Some("Work".into()),
|
||||
};
|
||||
let json = serde_json::to_string(&spec).unwrap();
|
||||
assert_eq!(serde_json::from_str::<ResolvedSpec>(&json).unwrap(), spec);
|
||||
|
||||
// A spec without a profile is the defaults, and the key is simply absent.
|
||||
let plain = ResolvedSpec {
|
||||
profile: None,
|
||||
..spec.clone()
|
||||
};
|
||||
let json = serde_json::to_string(&plain).unwrap();
|
||||
assert!(!json.contains("profile"));
|
||||
assert_eq!(serde_json::from_str::<ResolvedSpec>(&json).unwrap(), plain);
|
||||
}
|
||||
|
||||
/// A plan's spec carries the settings the plan resolved — including the profile's name for
|
||||
/// the overlay, and the host's clipboard decision the renderer no longer looks up.
|
||||
#[test]
|
||||
fn plan_spec_carries_what_the_session_may_not_re_derive() {
|
||||
let h = KnownHost {
|
||||
name: "Desk".into(),
|
||||
addr: "192.168.1.50".into(),
|
||||
fp_hex: "a".repeat(64),
|
||||
clipboard_sync: true,
|
||||
profile_id: Some("aaaaaaaaaaaa".into()),
|
||||
..Default::default()
|
||||
};
|
||||
let catalog = ProfilesFile {
|
||||
version: 1,
|
||||
profiles: vec![crate::profiles::StreamProfile {
|
||||
id: "aaaaaaaaaaaa".into(),
|
||||
name: "Game".into(),
|
||||
overrides: crate::profiles::SettingsOverlay {
|
||||
bitrate_kbps: Some(80000),
|
||||
..Default::default()
|
||||
},
|
||||
..crate::profiles::StreamProfile::new("")
|
||||
}],
|
||||
};
|
||||
let plan = ConnectPlan::resolve(&h, None, None, &catalog, &Settings::default());
|
||||
let spec = plan.spec(plan.clipboard);
|
||||
assert_eq!(spec.settings.bitrate_kbps, 80000, "the overlay is baked in");
|
||||
assert_eq!(spec.profile.as_deref(), Some("Game"));
|
||||
assert!(spec.clipboard, "the host's decision, resolved once");
|
||||
}
|
||||
|
||||
/// The stdout contract, parsed once for every front-end.
|
||||
#[test]
|
||||
fn session_contract_lines() {
|
||||
@@ -832,6 +1002,14 @@ mod tests {
|
||||
parse_session_line(r#"{"ended":"Host ended the session"}"#),
|
||||
Some(SessionEvent::Ended("Host ended the session".into()))
|
||||
);
|
||||
// The window report the spawner persists on the session's behalf.
|
||||
assert_eq!(
|
||||
parse_session_line(r#"{"window":{"w":1600,"h":900}}"#),
|
||||
Some(SessionEvent::Window { w: 1600, h: 900 })
|
||||
);
|
||||
// A half-formed window line is not an event — persisting half a size would be worse
|
||||
// than ignoring it.
|
||||
assert_eq!(parse_session_line(r#"{"window":{"w":1600}}"#), None);
|
||||
// Stats lines and stray output are never events.
|
||||
assert_eq!(parse_session_line("stats: 1280×800@60 · 60 fps"), None);
|
||||
assert_eq!(parse_session_line(""), None);
|
||||
|
||||
Reference in New Issue
Block a user