The previous commit deleted `schedule_authorized` and with it three
denial tests, leaving the permit behaviour it introduced unasserted:
nothing in the suite would notice a subtree check creeping back into
the scheduling handlers.
These four pin the inverse of the decision the deleted predicate used
to make. They sit at the handler level because that is the lowest layer
where the decision still exists to be observed — the predicate, its pure
`_in` form and the wrapper are gone, so there is no function left whose
return value *is* the decision, and the layer above (`dispatch`) reaches
these verbs through `require_group("scheduling")`, a different gate that
is still present and not the one under test.
`cancel` and `edit` assert the row afterwards rather than stopping at
`Response::Ok`: a check that silently declines to act while still
answering `Ok` is the regression a response-code assertion misses.
`list_schedules` is the structurally different case — it never rejected,
it filtered per row, so its assertion is that another owner's row is
present at all. `fire` asserts the response only; the fan-out legitimately
finds no live container under test and `fire_now` reports that inside
`Ok(report)`, so the response code is the only honest signal there.
Verified by mutation, not by inspection: reintroducing an ancestry walk
into the four handlers flips all four tests to failing (the three
mutating verbs on the refusal, `list_schedules` on an empty list) and
leaves the two pre-existing ghost-filter tests untouched.
These four cover the complete set of gate points this branch removes.
The roster listing that used to be the fifth is no longer one: `main`
removed `list_containers`, `Request::ListDescendants` and the handler
behind them outright, so there is nothing left there to assert.
Refs #4472
The topology predicate `is_descendant_of` gated the four schedule-
managing verbs: a caller could only name a schedule owned by an agent at
or below itself in `topology.json`. Those gates now permit any requester,
so the predicate, its pure `_in` form and the `schedule_authorized`
wrapper built on it are gone rather than left returning a constant. The
other two wrappers went earlier with the verbs they served —
`require_descendant` with the lifecycle MCP verbs in 87970a8c, and
`resolve_agent_state_target` with `get_loose_ends`'s agent parameter.
`require_group(agent, "scheduling", ...)` is untouched and still fires at
dispatch for every one of the five scheduling verbs, so holding the tool
group remains the gate; what goes is the ownership restriction layered on
top of it.
The three schedule-mutating verbs keep their row lookup as a plain
existence check, so a caller naming a schedule that does not exist still
gets `not found` rather than a message from deeper in the cancel path.
`list_schedules` stops filtering per row: it would only have hidden rows
the requester may act on anyway.
Error messages, tool descriptions and docs that described the subtree
relation are reworded — a refusal message naming a topology that no
longer decides anything is worse than none.
The six `is_descendant_of_in` unit tests go with the function they test;
the permit behaviour they leave unasserted is picked up by the next
commit.
Refs #4472
handle_list_schedules took no requester and returned every schedule
unfiltered, unlike cancel_schedule/edit_schedule/fire_schedule_now
which all gate on the shared ownership predicate (self, operator, or
subtree via topology::is_descendant_of). list_schedules now filters
through the same predicate, renamed cancel_authorized ->
schedule_authorized since it backs all four verbs now, not just
cancel.
Fixed five stale 'every schedule' / 'unfiltered' claims found while in
here: filter_ghost_schedule_targets's doc comment, the list_schedules
MCP tool description, docs/tools/scheduling.md's per-verb section
(already self-contradicting its own top-of-file subtree-scoping claim
before this fix), and hive-core-agent-sock's ListSchedules/Schedules
wire-type doc comments (including a stale '(privileged)' marker from
the pre-topology-subtree model).
Credit to atlas: independently found the same fix while finishing
PR #4233 (which documents this bug per mara's 'fix it, don't document
it' ruling) and caught two stale doc spots I'd missed
(hive-core-agent-sock's comments) plus proposed the
schedule_authorized rename. Compared diffs directly before either of
us pushed; he dropped his scheduling.rs changes so we didn't collide.
fixes#4237
Closes the #3110 split — lib.rs is now just the crate doc comment and
the pub mod list.
journal.rs's new doc comment fixes a pre-existing bug: the old
JournalPriority doc text in lib.rs was actually half Capability's doc
(a leftover from an earlier reorder that moved the code but not the
comment above it).
Two hardening items from argus's review of #2374:
1. PR state check at submission:
- Add `pr_is_open(repo, pr)` to forge/pr_merge.rs using
`repo_get_pull_request` + `StateType` — early error if the PR is
already closed or merged instead of queuing a card that fails later
- Call it in `submit_merge_config_pr` before fetching the head sha
2. Atomic fetched_sha INSERT:
- Add `fetched_sha: Option<&str>` to `Approvals::submit_kind` so
the sha can be included in the INSERT rather than a follow-up UPDATE
- MergeConfigPr already knows the sha before inserting the row
(pr_head_sha runs first) → pass `Some(&sha)`, drop the separate
`set_fetched_sha` call → truly atomic
- ApplyCommit still needs two writes (sha resolved by git_fetch_to_tag
after the row exists) → pass `None`, `set_fetched_sha` unchanged
- All other callers (InitConfig, Spawn, UpdateMetaInputs,
SchedulePrompt) pass `None` — no behavioural change
- Add `fetched_sha_in_insert_is_readable_via_get` test covering the
MergeConfigPr path
