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
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>
This commit is contained in:
@@ -176,16 +176,48 @@ impl HooksConfig {
|
|||||||
|
|
||||||
/// The persisted hooks store — the [`crate::vdisplay::policy::DisplayPolicyStore`] recipe:
|
/// 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.
|
/// 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 {
|
pub struct HooksStore {
|
||||||
path: PathBuf,
|
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 {
|
impl HooksStore {
|
||||||
/// Load from `path`. Missing file ⇒ no hooks; corrupt file ⇒ no hooks with a warning
|
/// Load from `path`. Missing file ⇒ no hooks; corrupt file ⇒ no hooks with a warning
|
||||||
/// (never fail host startup over a settings file).
|
/// (never fail host startup over a settings file).
|
||||||
pub fn load_from(path: PathBuf) -> Self {
|
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(bytes) => match serde_json::from_slice::<HooksConfig>(&bytes) {
|
||||||
Ok(c) => {
|
Ok(c) => {
|
||||||
if let Err(e) = c.validate() {
|
if let Err(e) = c.validate() {
|
||||||
@@ -204,15 +236,23 @@ impl HooksStore {
|
|||||||
},
|
},
|
||||||
Err(_) => None,
|
Err(_) => None,
|
||||||
};
|
};
|
||||||
HooksStore {
|
(cfg, file_id)
|
||||||
path,
|
|
||||||
cur: Mutex::new(cur),
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The stored configuration (empty when unconfigured) — the mgmt GET and the dispatcher.
|
/// 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 {
|
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
|
/// 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");
|
let tmp = self.path.with_extension("json.tmp");
|
||||||
pf_paths::write_secret_file(&tmp, &serde_json::to_vec_pretty(&cfg)?)?;
|
pf_paths::write_secret_file(&tmp, &serde_json::to_vec_pretty(&cfg)?)?;
|
||||||
std::fs::rename(&tmp, &self.path)?;
|
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(())
|
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 {
|
pub fn store() -> &'static HooksStore {
|
||||||
static STORE: OnceLock<HooksStore> = OnceLock::new();
|
static STORE: OnceLock<HooksStore> = OnceLock::new();
|
||||||
STORE.get_or_init(|| HooksStore::load_from(pf_paths::config_dir().join("hooks.json")))
|
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);
|
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]
|
#[test]
|
||||||
fn filters_constrain_and_missing_fields_never_match() {
|
fn filters_constrain_and_missing_fields_never_match() {
|
||||||
let ev = sample_event();
|
let ev = sample_event();
|
||||||
|
|||||||
Reference in New Issue
Block a user