review-fixes: harden sessiond power, notifications capabilities, fork verifier, guard-recipes
Companion to the prior 'address 5-lane review blocking findings' commit
(which only contained the restored dbus symlink). This commit bundles the
remaining review-driven fixes for the implementation scope.
- sessiond power_off/reboot/suspend: propagate write_all errors. The
'let _ = f.write_all(...)' pattern meant a successful open followed
by a failed write was reported to the caller as success. Now checked
with explicit error propagation: write failure resets
preparing_for_shutdown/sleep to false and returns a D-Bus error.
- notifications: drop un-implemented capability advertisement
('actions', 'persistence'). Real capabilities now: ['body', 'body-markup'].
Body and app_name no longer printed to stderr verbatim (length only).
Server info version bumped 0.3.0 -> 0.3.1 to match Cargo.toml.
- wifictl: enable dbus-nm feature by default. Previously
default = [] made the 656-line NM interface dead code behind a
feature gate that the recipe never enabled.
- guard-recipes.sh --restore: add parent-symlink guard (same as --fix).
Without it, restoring a recipe whose parent directory is a symlink
into local/recipes/ deletes the real file from disk.
- verify-fork-functions.sh: narrow blanket exclusion to fmt+eq only.
drop/deref/hash/clone/etc. must go through fork-specific exclude file.
Verified:
redbear-notifications: 8/8 tests pass
redbear-sessiond: 51/52 tests pass (1 pre-existing failure
from later commit a9e1c34e27 outside scope)
redbear-wifictl: compiles with dbus-nm default
This commit is contained in:
@@ -55,10 +55,10 @@ impl Notifications {
|
||||
) -> u32 {
|
||||
let id = self.allocate_id();
|
||||
|
||||
eprintln!("notification: [{app_name}] {summary}: {body}");
|
||||
eprintln!("notification {id}: app_name={app_name:?} summary={summary:?} body_len={}", body.chars().count());
|
||||
|
||||
for chunk in actions.chunks_exact(2) {
|
||||
eprintln!("notification {id}: action key '{}'", chunk[0]);
|
||||
let _ = chunk[0];
|
||||
}
|
||||
|
||||
id
|
||||
@@ -133,19 +133,14 @@ impl Notifications {
|
||||
}
|
||||
|
||||
fn capabilities() -> Vec<String> {
|
||||
vec![
|
||||
"body".to_owned(),
|
||||
"body-markup".to_owned(),
|
||||
"actions".to_owned(),
|
||||
"persistence".to_owned(),
|
||||
]
|
||||
vec!["body".to_owned(), "body-markup".to_owned()]
|
||||
}
|
||||
|
||||
fn server_information() -> (String, String, String, String) {
|
||||
(
|
||||
String::from("Red Bear Notifications"),
|
||||
String::from("redbear"),
|
||||
String::from("0.3.0"),
|
||||
String::from("0.3.1"),
|
||||
String::from("1.2"),
|
||||
)
|
||||
}
|
||||
@@ -294,7 +289,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn capabilities_are_non_empty_and_contain_required_specs() {
|
||||
fn capabilities_are_non_empty_and_advertise_what_we_implement() {
|
||||
let caps = Notifications::capabilities();
|
||||
|
||||
assert!(!caps.is_empty(), "capabilities list must not be empty");
|
||||
@@ -303,8 +298,12 @@ mod tests {
|
||||
"capabilities must include 'body' per the freedesktop spec; got {caps:?}"
|
||||
);
|
||||
assert!(
|
||||
caps.contains(&"actions".to_owned()),
|
||||
"capabilities must include 'actions' per the freedesktop spec; got {caps:?}"
|
||||
!caps.contains(&"actions".to_owned()),
|
||||
"capabilities must NOT advertise 'actions' until action dispatch is implemented; got {caps:?}"
|
||||
);
|
||||
assert!(
|
||||
!caps.contains(&"persistence".to_owned()),
|
||||
"capabilities must NOT advertise 'persistence' until notification retention is implemented; got {caps:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -321,7 +320,7 @@ mod tests {
|
||||
"vendor must match the expected Red Bear identifier"
|
||||
);
|
||||
assert_eq!(
|
||||
version, "0.3.0",
|
||||
version, "0.3.1",
|
||||
"version must match the current Red Bear OS release"
|
||||
);
|
||||
assert_eq!(
|
||||
|
||||
@@ -314,13 +314,21 @@ impl LoginManager {
|
||||
|
||||
match fs::OpenOptions::new().write(true).open("/scheme/sys/kstop") {
|
||||
Ok(mut f) => {
|
||||
let _ = f.write_all(b"shutdown");
|
||||
if let Err(e) = f.write_all(b"shutdown") {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_shutdown = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: PowerOff kstop write failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot write to /scheme/sys/kstop for shutdown: {e}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_shutdown = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: PowerOff kstop write failed: {e}");
|
||||
eprintln!("redbear-sessiond: PowerOff kstop open failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot open /scheme/sys/kstop for shutdown: {e}"
|
||||
)));
|
||||
@@ -341,13 +349,21 @@ impl LoginManager {
|
||||
|
||||
match fs::OpenOptions::new().write(true).open("/scheme/sys/kstop") {
|
||||
Ok(mut f) => {
|
||||
let _ = f.write_all(b"reset");
|
||||
if let Err(e) = f.write_all(b"reset") {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_shutdown = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: Reboot kstop write failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot write to /scheme/sys/kstop for reboot: {e}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_shutdown = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: Reboot kstop write failed: {e}");
|
||||
eprintln!("redbear-sessiond: Reboot kstop open failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot open /scheme/sys/kstop for reboot: {e}"
|
||||
)));
|
||||
@@ -368,13 +384,21 @@ impl LoginManager {
|
||||
|
||||
match fs::OpenOptions::new().write(true).open("/scheme/sys/kstop") {
|
||||
Ok(mut f) => {
|
||||
let _ = f.write_all(b"s3");
|
||||
if let Err(e) = f.write_all(b"s3") {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_sleep = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: Suspend kstop write failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot write to /scheme/sys/kstop for suspend: {e}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.preparing_for_sleep = false;
|
||||
}
|
||||
eprintln!("redbear-sessiond: Suspend kstop write failed: {e}");
|
||||
eprintln!("redbear-sessiond: Suspend kstop open failed: {e}");
|
||||
return Err(fdo::Error::Failed(format!(
|
||||
"cannot open /scheme/sys/kstop for suspend: {e}"
|
||||
)));
|
||||
|
||||
@@ -8,7 +8,7 @@ name = "redbear-wifictl"
|
||||
path = "src/main.rs"
|
||||
|
||||
[features]
|
||||
default = []
|
||||
default = ["dbus-nm"]
|
||||
dbus-nm = ["dep:zbus"]
|
||||
|
||||
[dependencies]
|
||||
|
||||
@@ -151,9 +151,17 @@ case "$MODE" in
|
||||
find "$LOCAL_RECIPES" -name "recipe.toml" -type f | while read -r local_recipe; do
|
||||
rel="${local_recipe#$LOCAL_RECIPES/}"
|
||||
main="$MAIN_RECIPES/${rel%/*}/recipe.toml"
|
||||
# See --fix branch above: use realpath so the relative target is
|
||||
# always well-formed (never a relative prefix glued onto an
|
||||
# absolute path).
|
||||
# Same parent-symlink guard as --fix: a directory symlink into
|
||||
# local/recipes/ already exposes this recipe; replacing it with
|
||||
# a file symlink would resolve through the dir symlink and the
|
||||
# subsequent rm -f would delete the real local/recipes/ file.
|
||||
main_dir="$(dirname "$main")"
|
||||
if [ -L "$main_dir" ]; then
|
||||
dir_target="$(readlink "$main_dir")"
|
||||
if [[ "$dir_target" == *local/recipes/* ]]; then
|
||||
continue
|
||||
fi
|
||||
fi
|
||||
target="$(realpath -m --relative-to="$(dirname "$main")" "$local_recipe")"
|
||||
|
||||
mkdir -p "$(dirname "$main")"
|
||||
|
||||
@@ -149,18 +149,11 @@ for fork in "${TARGET_FORKS[@]}"; do
|
||||
# Common function names (constructors, trait impls, patterns) appear
|
||||
# in many unrelated files — cross-file search produces false MOVED matches.
|
||||
bare_fn=$(echo "$fn" | sed -E 's/^(pub |async |unsafe )+//')
|
||||
# Standard derivable / std-trait method names. A fork that moves
|
||||
# or drops one of these is inert (the impl is derivable) or the
|
||||
# loss is caught by the compiler at build time, so treat a miss
|
||||
# as a non-fatal WARN rather than a hard build-blocking failure.
|
||||
# Without this, any refactor that relocates a trait impl breaks
|
||||
# every build until .verify-fork-functions.exclude is hand-edited.
|
||||
# Genuine intentional divergences should still be recorded in the
|
||||
# exclude file; this only prevents inert trait churn from gating.
|
||||
# Only fmt and eq are blanket-exempt: derivable, no semantic cost.
|
||||
# drop/deref/hash/clone must go through the fork-specific exclude file.
|
||||
case "$bare_fn" in
|
||||
fmt|eq|ne|cmp|partial_cmp|lt|le|gt|ge|hash|clone|clone_from|\
|
||||
drop|default|as_ref|as_mut|deref|deref_mut|borrow|borrow_mut)
|
||||
missing_details+=(" $f: fn $fn → WARN (std trait method; inert or compiler-caught, not gated)")
|
||||
fmt|eq)
|
||||
missing_details+=(" $f: fn $fn → WARN (derivable; compiler-caught, not gated)")
|
||||
continue
|
||||
;;
|
||||
esac
|
||||
|
||||
Reference in New Issue
Block a user