Compare commits

..

1 Commits

Author SHA1 Message Date
enricobuehler 11cca33300 fix(host/hooks): honor hand-edits to hooks.json without a restart
android / android (push) Successful in 19m27s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Successful in 13m51s
windows-host / package (push) Successful in 18m48s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Successful in 20m58s
ci / rust (push) Failing after 14m6s
ci / web (push) Successful in 51s
ci / docs-site (push) Successful in 58s
apple / swift (push) Successful in 1m21s
decky / build-publish (push) Successful in 20s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 10s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 12s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 11s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 9s
ci / bench (push) Successful in 5m56s
apple / screenshots (push) Successful in 6m15s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 8m35s
docker / deploy-docs (push) Successful in 27s
deb / build-publish (push) Successful in 12m33s
arch / build-publish (push) Successful in 18m31s
docs/automation.md promises 'changes apply immediately, no restart' for BOTH
config paths, but the store loaded hooks.json once (OnceLock) — only PUT
/api/v1/hooks applied live, and a hand-edited file silently did nothing until
the next host start. Since the file is the accessible path most users will
take, make it real: get() re-stats the file per event (mtime + length) and
re-reads it on change, with the same lenient contract as startup (missing =
no hooks, invalid = disabled loudly). set() records the identity it wrote so
the API path doesn't trigger a spurious reload.

Validated live on the 0.13.0 host (.21): full connect/disconnect lifecycle
fires exec hooks (client.*/session.*/stream.*) — the reload gap was found
setting that test up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 23:53:25 +02:00
+88 -9
View File
@@ -176,16 +176,48 @@ impl HooksConfig {
/// The persisted hooks store — the [`crate::vdisplay::policy::DisplayPolicyStore`] recipe:
/// private dir, temp-write + atomic rename, in-memory value changes only if the write succeeds.
///
/// A hand-edited `hooks.json` is honored WITHOUT a restart (the documented contract): [`get`]
/// re-stats the file and reloads when its identity (mtime + length) moved. The stat rides the
/// per-event dispatch, so the check costs one `metadata()` call per event, and a full re-read
/// happens only when the file actually changed.
///
/// [`get`]: HooksStore::get
pub struct HooksStore {
path: PathBuf,
cur: Mutex<Option<HooksConfig>>,
cur: Mutex<StoreState>,
}
struct StoreState {
cfg: Option<HooksConfig>,
/// Identity of the file revision `cfg` was parsed from (mtime + length); `None` = the file
/// did not exist. `get` compares against a fresh stat to detect hand edits.
file_id: Option<(std::time::SystemTime, u64)>,
}
impl HooksStore {
/// Load from `path`. Missing file ⇒ no hooks; corrupt file ⇒ no hooks with a warning
/// (never fail host startup over a settings file).
pub fn load_from(path: PathBuf) -> Self {
let cur = match std::fs::read(&path) {
let (cfg, file_id) = Self::read_disk(&path);
HooksStore {
path,
cur: Mutex::new(StoreState { cfg, file_id }),
}
}
/// The file's on-disk identity, `None` when it does not exist (or cannot be stat'd —
/// indistinguishable on purpose: both mean "no usable hooks file").
fn file_identity(path: &PathBuf) -> Option<(std::time::SystemTime, u64)> {
let meta = std::fs::metadata(path).ok()?;
Some((meta.modified().ok()?, meta.len()))
}
/// Read + validate the file. Same lenient contract as startup: missing ⇒ no hooks;
/// invalid/unreadable ⇒ no hooks with a warning naming the problem.
fn read_disk(path: &PathBuf) -> (Option<HooksConfig>, Option<(std::time::SystemTime, u64)>) {
let file_id = Self::file_identity(path);
let cfg = match std::fs::read(path) {
Ok(bytes) => match serde_json::from_slice::<HooksConfig>(&bytes) {
Ok(c) => {
if let Err(e) = c.validate() {
@@ -204,15 +236,23 @@ impl HooksStore {
},
Err(_) => None,
};
HooksStore {
path,
cur: Mutex::new(cur),
}
(cfg, file_id)
}
/// The stored configuration (empty when unconfigured) — the mgmt GET and the dispatcher.
/// Re-reads `hooks.json` first if it changed on disk since last load, so hand edits apply
/// on the next event, no restart ("changes apply immediately" — docs/automation.md).
pub fn get(&self) -> HooksConfig {
self.cur.lock().unwrap().clone().unwrap_or_default()
let mut st = self.cur.lock().unwrap();
let now_id = Self::file_identity(&self.path);
if now_id != st.file_id {
let (cfg, file_id) = Self::read_disk(&self.path);
tracing::info!(path = %self.path.display(), hooks = cfg.as_ref().map_or(0, |c| c.hooks.len()),
"hooks.json changed on disk — reloaded");
st.cfg = cfg;
st.file_id = file_id;
}
st.cfg.clone().unwrap_or_default()
}
/// Persist + adopt a new configuration (caller validates first). The in-memory value
@@ -224,12 +264,15 @@ impl HooksStore {
let tmp = self.path.with_extension("json.tmp");
pf_paths::write_secret_file(&tmp, &serde_json::to_vec_pretty(&cfg)?)?;
std::fs::rename(&tmp, &self.path)?;
*self.cur.lock().unwrap() = Some(cfg);
let mut st = self.cur.lock().unwrap();
st.file_id = Self::file_identity(&self.path);
st.cfg = Some(cfg);
Ok(())
}
}
/// The process-wide hooks store (`<config_dir>/hooks.json`), loaded once on first access.
/// The process-wide hooks store (`<config_dir>/hooks.json`), loaded on first access and
/// re-loaded whenever the file changes on disk (see [`HooksStore::get`]).
pub fn store() -> &'static HooksStore {
static STORE: OnceLock<HooksStore> = OnceLock::new();
STORE.get_or_init(|| HooksStore::load_from(pf_paths::config_dir().join("hooks.json")))
@@ -860,6 +903,42 @@ mod tests {
let _ = std::fs::remove_file(&path);
}
#[test]
fn hand_edited_file_reloads_without_restart() {
let path = std::env::temp_dir().join(format!(
"pf-hooks-reload-test-{}-{:p}.json",
std::process::id(),
&0u8 as *const u8
));
let _ = std::fs::remove_file(&path);
let store = HooksStore::load_from(path.clone());
assert!(store.get().hooks.is_empty());
// The documented flow: the operator writes hooks.json by hand and the SAME running
// store honors it on the next event — no restart, no PUT.
std::fs::write(
&path,
br#"{"hooks":[{"on":"stream.started","run":"true"}]}"#,
)
.unwrap();
assert_eq!(store.get().hooks.len(), 1, "hand edit applies on next read");
assert_eq!(store.get().hooks[0].on, "stream.started");
// A second edit applies too (length differs, so same-second mtime granularity can't
// mask it).
std::fs::write(
&path,
br#"{"hooks":[{"on":"stream.started","run":"true"},{"on":"client.*","run":"true"}]}"#,
)
.unwrap();
assert_eq!(store.get().hooks.len(), 2, "second hand edit applies too");
// Deleting the file removes the hooks.
std::fs::remove_file(&path).unwrap();
assert!(store.get().hooks.is_empty(), "deleted file = no hooks");
}
#[test]
fn filters_constrain_and_missing_fields_never_match() {
let ev = sample_event();