remove operator as target for scheduled prompts
This commit is contained in:
parent
f263f82047
commit
b4dc09ff93
7 changed files with 130 additions and 86 deletions
|
|
@ -216,15 +216,15 @@ target drives `push_todo`'s own upsert-by-key dedup: a re-fire of the
|
|||
*same schedule* against a target that hasn't reviewed the last one
|
||||
collapses into that one todo instead of stacking up.
|
||||
|
||||
**`operator` is the one exception** — it's a valid schedule target but
|
||||
has no in-container todo inbox, so it keeps the original broker
|
||||
`Message` path (the dashboard mirrors `to == operator` into its own
|
||||
pane, same as before).
|
||||
`operator` is not a valid schedule target — she isn't a container with
|
||||
a todo inbox to push a wake into, and this mechanism is for fanning a
|
||||
prompt out to agents, not scheduling something at her. Rejected at
|
||||
submit/edit time (`ScheduledPrompts::submit`/`update`).
|
||||
|
||||
### Missing-target failure
|
||||
|
||||
When a target name doesn't resolve to a known agent (container
|
||||
destroyed, operator typo, etc.) the worker:
|
||||
destroyed, typo, etc.) the worker:
|
||||
|
||||
1. Records `last_result = "no such agent: <name>"` on the
|
||||
per-target row.
|
||||
|
|
|
|||
|
|
@ -238,22 +238,23 @@ function intervalSecondsFromFormData(fd, namePrefix) {
|
|||
|
||||
// Targets multi-select chip box — shared between the new-schedule
|
||||
// form and the edit-schedule form. Same DOM shape, same candidate
|
||||
// list (containers + operator + root); only the chip element id
|
||||
// list (containers + root); only the chip element id
|
||||
// prefix and checkbox field name vary. Used to be inlined twice in
|
||||
// near-identical 18-line blocks; consolidated here so a future
|
||||
// change (new chip kind, candidate-list source swap, etc.) lives in
|
||||
// one place. Returns the wrapping `<label class="schedule-field">`
|
||||
// ready to append to the form.
|
||||
function buildTargetChips({ idPrefix, fieldName, checked, extraNames = [] }) {
|
||||
const candidates = ["operator"];
|
||||
const candidates = [];
|
||||
const containerNames = Array.from(containersState.values())
|
||||
.map((c) => c.name)
|
||||
.filter((n) => n !== "operator")
|
||||
.sort();
|
||||
for (const n of containerNames) candidates.push(n);
|
||||
// `extraNames` lets the edit form keep showing an already-active
|
||||
// target that's vanished from the live container list (operator's
|
||||
// typo, container destroyed mid-schedule, etc.) so it's still
|
||||
// target that's vanished from the live container list (container
|
||||
// destroyed mid-schedule, a pre-existing schedule created before
|
||||
// operator stopped being a valid target, etc.) so it's still
|
||||
// explicitly uncheckable. New-schedule callers pass `[]`.
|
||||
for (const n of extraNames) if (!candidates.includes(n)) candidates.push(n);
|
||||
|
||||
|
|
@ -606,12 +607,12 @@ async function submitNewScheduleInline(tr, submitBtn) {
|
|||
}
|
||||
});
|
||||
}
|
||||
// The set of agent columns in the schedules table: operator + root
|
||||
// (manager) first, then live containers (sorted), then any extra names
|
||||
// that appear as a schedule target but aren't in the live container list
|
||||
// (operator typo, container destroyed mid-schedule, etc.) — same
|
||||
// membership rule as `buildTargetChips` so the table and the new-/
|
||||
// edit-form chip boxes agree on what's addressable.
|
||||
// The set of agent columns in the schedules table: root (manager) first,
|
||||
// then live containers (sorted), then any extra names that appear as a
|
||||
// schedule target but aren't in the live container list (container
|
||||
// destroyed mid-schedule, etc.) — same membership rule as
|
||||
// `buildTargetChips` so the table and the new-/edit-form chip boxes
|
||||
// agree on what's addressable.
|
||||
function schedulesTableAgentSet() {
|
||||
const seen = new Set();
|
||||
const out = [];
|
||||
|
|
@ -621,7 +622,6 @@ function schedulesTableAgentSet() {
|
|||
out.push(n);
|
||||
}
|
||||
};
|
||||
push("operator");
|
||||
const containerNames = Array.from(containersState.values())
|
||||
.map((c) => c.name)
|
||||
.filter((n) => n !== "operator")
|
||||
|
|
@ -643,14 +643,14 @@ function renderSchedulesTableHead(agents) {
|
|||
el("th", {}, "owner"),
|
||||
el("th", { class: "schedules-table-body-th" }, "body"),
|
||||
);
|
||||
// 'operator' is always a valid target; every other column maps to a
|
||||
// container, so flag any column whose agent is no longer in the live
|
||||
// roster — its past schedules linger in the table but the agent is gone.
|
||||
// Every column maps to a container, so flag any column whose agent is
|
||||
// no longer in the live roster — its past schedules linger in the
|
||||
// table but the agent is gone.
|
||||
const liveNames = new Set(
|
||||
Array.from(containersState.values()).map((c) => c.name),
|
||||
);
|
||||
for (const a of agents) {
|
||||
const gone = a !== "operator" && !liveNames.has(a);
|
||||
const gone = !liveNames.has(a);
|
||||
headerRow.append(
|
||||
el(
|
||||
"th",
|
||||
|
|
|
|||
|
|
@ -196,9 +196,10 @@ pub struct UpdateMetaInputsArgs {
|
|||
#[derive(Debug, serde::Deserialize, schemars::JsonSchema)]
|
||||
pub struct RequestSchedulePromptArgs {
|
||||
/// Recipient agents — one schedule fires to many inboxes at the
|
||||
/// scheduled time. `operator` is a legitimate target (mara: "we
|
||||
/// want to get rid of the manager special case so yes manager
|
||||
/// can be recipient" — the operator slot follows the same rule).
|
||||
/// scheduled time. `operator` is not a valid target — she isn't a
|
||||
/// container with a todo inbox to push a wake into, and this is
|
||||
/// for fanning a prompt out to agents, not scheduling something
|
||||
/// at her.
|
||||
pub targets: Vec<String>,
|
||||
/// Message body delivered to each target's inbox at fire time.
|
||||
/// Same size budget as `send` bodies.
|
||||
|
|
|
|||
|
|
@ -98,6 +98,14 @@ pub(super) async fn post_schedule_new(
|
|||
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
|
||||
.with_detail("schedule must have at least one target"));
|
||||
}
|
||||
if payload
|
||||
.targets
|
||||
.iter()
|
||||
.any(|t| t == hive_sh4re::manager::OPERATOR_RECIPIENT)
|
||||
{
|
||||
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
|
||||
.with_detail("operator is not a valid schedule target"));
|
||||
}
|
||||
if payload.body.trim().is_empty() {
|
||||
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
|
||||
.with_detail("schedule body must be non-empty"));
|
||||
|
|
|
|||
|
|
@ -47,6 +47,15 @@ pub(super) fn handle_request_schedule_prompt(
|
|||
message: "schedule must have at least one target".into(),
|
||||
};
|
||||
}
|
||||
if payload
|
||||
.targets
|
||||
.iter()
|
||||
.any(|t| t == hive_sh4re::manager::OPERATOR_RECIPIENT)
|
||||
{
|
||||
return Response::Err {
|
||||
message: "operator is not a valid schedule target".into(),
|
||||
};
|
||||
}
|
||||
if payload.body.trim().is_empty() {
|
||||
return Response::Err {
|
||||
message: "schedule body must be non-empty".into(),
|
||||
|
|
@ -310,8 +319,7 @@ pub fn schedule_to_wire_public(
|
|||
/// the dashboard's schedule table doesn't render ghost columns for
|
||||
/// destroyed agents. `live` is the set of logical agent names from the
|
||||
/// last `nixos-container list` scan (stopped agents included, destroyed
|
||||
/// ones absent); the `operator` pseudo-target is always retained since
|
||||
/// it isn't a container. Applied only to the dashboard wire paths
|
||||
/// ones absent). Applied only to the dashboard wire paths
|
||||
/// (`api_schedules` + the `SchedulesChanged` SSE emit) — `list_schedules`
|
||||
/// doesn't apply this filter (it still returns every live target, ghost
|
||||
/// or not, on the schedules the requester is authorized to see) so an
|
||||
|
|
@ -323,9 +331,7 @@ pub(crate) fn filter_ghost_schedule_targets(
|
|||
live: &std::collections::HashSet<String>,
|
||||
) {
|
||||
for s in schedules.iter_mut() {
|
||||
s.targets.retain(|t| {
|
||||
t.target == hive_sh4re::manager::OPERATOR_RECIPIENT || live.contains(&t.target)
|
||||
});
|
||||
s.targets.retain(|t| live.contains(&t.target));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -417,19 +423,19 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn ghost_filter_drops_dead_agents_keeps_live_and_operator() {
|
||||
fn ghost_filter_drops_dead_agents_keeps_live() {
|
||||
let live: std::collections::HashSet<String> = ["iris".to_owned(), "damocles".to_owned()]
|
||||
.into_iter()
|
||||
.collect();
|
||||
let mut schedules = vec![schedule(&["iris", "ghost", "operator", "damocles"])];
|
||||
let mut schedules = vec![schedule(&["iris", "ghost", "damocles"])];
|
||||
filter_ghost_schedule_targets(&mut schedules, &live);
|
||||
let kept: Vec<&str> = schedules[0]
|
||||
.targets
|
||||
.iter()
|
||||
.map(|t| t.target.as_str())
|
||||
.collect();
|
||||
// `ghost` (destroyed) dropped; live agents + operator pseudo-target kept.
|
||||
assert_eq!(kept, vec!["iris", "operator", "damocles"]);
|
||||
// `ghost` (destroyed) dropped; live agents kept.
|
||||
assert_eq!(kept, vec!["iris", "damocles"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -437,8 +443,6 @@ mod tests {
|
|||
let live: std::collections::HashSet<String> = std::collections::HashSet::new();
|
||||
let mut schedules = vec![schedule(&["gone1", "gone2"])];
|
||||
filter_ghost_schedule_targets(&mut schedules, &live);
|
||||
// operator is never in the live set but is always retained; here
|
||||
// there's no operator target, so everything drops.
|
||||
assert!(schedules[0].targets.is_empty());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,11 +220,21 @@ impl ScheduledPrompts {
|
|||
|
||||
/// Insert a new schedule. Returns the new id. Empty `targets` is
|
||||
/// rejected — a schedule with no recipients would silently
|
||||
/// never fan out, masking caller bugs.
|
||||
/// never fan out, masking caller bugs. `operator` is rejected too
|
||||
/// — it isn't a container with a todo inbox to push a wake into,
|
||||
/// and scheduling a prompt at the operator herself doesn't fit the
|
||||
/// "fan a prompt out to agent inboxes" shape this exists for.
|
||||
pub fn submit(&self, new: &NewSchedule) -> Result<i64> {
|
||||
if new.targets.is_empty() {
|
||||
bail!("schedule must have at least one target");
|
||||
}
|
||||
if new
|
||||
.targets
|
||||
.iter()
|
||||
.any(|t| t == hive_sh4re::manager::OPERATOR_RECIPIENT)
|
||||
{
|
||||
bail!("operator is not a valid schedule target");
|
||||
}
|
||||
let mut conn = self.conn.lock().unwrap();
|
||||
let tx = conn.transaction()?;
|
||||
tx.execute(
|
||||
|
|
@ -435,6 +445,12 @@ impl ScheduledPrompts {
|
|||
if let Some(Some(0)) = patch.interval_seconds {
|
||||
bail!("interval_seconds must be > 0 (use None for one-shot)");
|
||||
}
|
||||
if patch.targets_add.as_deref().is_some_and(|list| {
|
||||
list.iter()
|
||||
.any(|t| t == hive_sh4re::manager::OPERATOR_RECIPIENT)
|
||||
}) {
|
||||
bail!("operator is not a valid schedule target");
|
||||
}
|
||||
// Single transaction so a partial failure can't leave a
|
||||
// half-updated row.
|
||||
let tx = conn.transaction()?;
|
||||
|
|
@ -747,6 +763,39 @@ mod tests {
|
|||
assert!(format!("{err:#}").contains("at least one target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn submit_rejects_operator_as_a_target() {
|
||||
let (_dir, db) = open();
|
||||
let err = db
|
||||
.submit(&NewSchedule {
|
||||
owner: "operator".into(),
|
||||
targets: vec!["operator".into()],
|
||||
body: "wake".into(),
|
||||
first_fire_at_unix: 100,
|
||||
interval_seconds: None,
|
||||
description: None,
|
||||
source: ScheduleSource::Operator,
|
||||
})
|
||||
.unwrap_err();
|
||||
assert!(format!("{err:#}").contains("operator is not a valid schedule target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn update_rejects_operator_in_targets_add() {
|
||||
let (_dir, db) = open();
|
||||
let id = submit_one_shot(&db, 100, &["alice"]);
|
||||
let err = db
|
||||
.update(
|
||||
id,
|
||||
UpdateSchedule {
|
||||
targets_add: Some(vec!["operator".into()]),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(format!("{err:#}").contains("operator is not a valid schedule target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn due_returns_only_past_active_rows() {
|
||||
let (_dir, db) = open();
|
||||
|
|
|
|||
|
|
@ -102,11 +102,7 @@ async fn fire_schedule(coord: &Arc<Coordinator>, schedule: &Schedule, now: i64)
|
|||
continue;
|
||||
}
|
||||
let target = &target_row.target;
|
||||
// `operator` is a valid recipient (mara c4) — the operator has
|
||||
// no in-container todo inbox, so it keeps the regular broker
|
||||
// `Message` path; the dashboard mirrors `to == operator` into
|
||||
// its own pane.
|
||||
if target != hive_sh4re::manager::OPERATOR_RECIPIENT && !known.contains(target) {
|
||||
if !known.contains(target) {
|
||||
let reason = format!("no such agent: {target}");
|
||||
if let Err(e) =
|
||||
coord
|
||||
|
|
@ -159,17 +155,16 @@ async fn fire_schedule(coord: &Arc<Coordinator>, schedule: &Schedule, now: i64)
|
|||
}
|
||||
}
|
||||
|
||||
/// Deliver `body` to a single already-known-live `target` — a broker
|
||||
/// `Message` when `target` is the operator (no in-container todo inbox
|
||||
/// to push into there), a `push_todo` otherwise, keyed on the
|
||||
/// schedule's own identity (`schedule:{schedule_id}`) so a re-fire
|
||||
/// against a target that hasn't reviewed the last one collapses via
|
||||
/// `push_todo`'s own upsert-by-key dedup — but `reopen_if_acked = true`
|
||||
/// means that collapse only holds while unreviewed; once acked, the next
|
||||
/// fire reopens regardless of whether the body changed. Shared by the periodic
|
||||
/// `fire_schedule` tick and the manual `fire_now` dashboard action —
|
||||
/// the only difference between them is what each caller does with the
|
||||
/// `Result` (log/prefix and per-target `last_result`/`FireNowReport`
|
||||
/// Deliver `body` to a single already-known-live `target` via
|
||||
/// `push_todo`, keyed on the schedule's own identity
|
||||
/// (`schedule:{schedule_id}`) so a re-fire against a target that hasn't
|
||||
/// reviewed the last one collapses via `push_todo`'s own upsert-by-key
|
||||
/// dedup — but `reopen_if_acked = true` means that collapse only holds
|
||||
/// while unreviewed; once acked, the next fire reopens regardless of
|
||||
/// whether the body changed. Shared by the periodic `fire_schedule`
|
||||
/// tick and the manual `fire_now` dashboard action — the only
|
||||
/// difference between them is what each caller does with the `Result`
|
||||
/// (log/prefix and per-target `last_result`/`FireNowReport`
|
||||
/// bookkeeping), not the delivery choice itself.
|
||||
async fn deliver_to_target(
|
||||
coord: &Arc<Coordinator>,
|
||||
|
|
@ -177,36 +172,23 @@ async fn deliver_to_target(
|
|||
target: &str,
|
||||
body: &str,
|
||||
) -> Result<(), String> {
|
||||
if target == hive_sh4re::manager::OPERATOR_RECIPIENT {
|
||||
let msg = Message {
|
||||
from: hive_sh4re::manager::trusted_sender("scheduled"),
|
||||
to: target.to_owned(),
|
||||
body: body.to_owned(),
|
||||
in_reply_to: None,
|
||||
};
|
||||
coord
|
||||
.broker
|
||||
.send(&msg)
|
||||
.map_err(|e| format!("broker send failed: {e:#}"))
|
||||
} else {
|
||||
coord
|
||||
.push_todo(
|
||||
target,
|
||||
"schedule",
|
||||
Some(format!("schedule:{schedule_id}")),
|
||||
body.to_owned(),
|
||||
Some("scheduled".to_owned()),
|
||||
// Each fire is a distinct occurrence, not a restatement of
|
||||
// a persisting condition — an agent that already acked the
|
||||
// *previous* firing hasn't acked *this* one, so an acked
|
||||
// row must reopen even when the body is byte-identical
|
||||
// (the common case: most schedules don't vary their text
|
||||
// per fire). See `Todos::upsert`'s doc comment for the
|
||||
// full reconciler-vs-event rationale.
|
||||
true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
coord
|
||||
.push_todo(
|
||||
target,
|
||||
"schedule",
|
||||
Some(format!("schedule:{schedule_id}")),
|
||||
body.to_owned(),
|
||||
Some("scheduled".to_owned()),
|
||||
// Each fire is a distinct occurrence, not a restatement of
|
||||
// a persisting condition — an agent that already acked the
|
||||
// *previous* firing hasn't acked *this* one, so an acked
|
||||
// row must reopen even when the body is byte-identical
|
||||
// (the common case: most schedules don't vary their text
|
||||
// per fire). See `Todos::upsert`'s doc comment for the
|
||||
// full reconciler-vs-event rationale.
|
||||
true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Send the operator a one-line advisory when a schedule fires
|
||||
|
|
@ -307,7 +289,7 @@ pub async fn fire_now(
|
|||
continue;
|
||||
}
|
||||
let target = &target_row.target;
|
||||
if target != hive_sh4re::manager::OPERATOR_RECIPIENT && !known.contains(target) {
|
||||
if !known.contains(target) {
|
||||
let reason = format!("manual fire: no such agent: {target}");
|
||||
if let Err(e) =
|
||||
coord
|
||||
|
|
@ -374,10 +356,10 @@ pub async fn fire_now(
|
|||
/// Always seeds the manager name (which is always reachable);
|
||||
/// adds every live nspawn container that matches the `h-` prefix.
|
||||
/// On `lifecycle::list` failure the set stays at just the manager
|
||||
/// — fail-CLOSED, meaning every non-operator/non-manager target
|
||||
/// looks missing this tick and gets the same treatment as a
|
||||
/// genuinely-destroyed agent: operator advisory + per-target
|
||||
/// `last_result` annotation + skipped delivery. Recurring
|
||||
/// — fail-CLOSED, meaning every non-manager target looks missing
|
||||
/// this tick and gets the same treatment as a genuinely-destroyed
|
||||
/// agent: operator advisory + per-target `last_result` annotation +
|
||||
/// skipped delivery. Recurring
|
||||
/// schedules recover automatically on the next tick (the lifecycle
|
||||
/// listing usually works); one-shots that land on this window
|
||||
/// lose their single delivery. Logged at `warn`, not propagated.
|
||||
|
|
|
|||
Loading…
Reference in a new issue