fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(apply): bound the blast-radius up-walk; fix two stale comments

Browse Source 
Second-review follow-ups on the ownership cleanups:

- The new `group_blast_radius` `app_chain` recursion walked app→root with no
  depth cap, unlike the `ancestors()` CTE it replaced (`WHERE c.depth < 64`).
  Add the same `ac.depth < 64` guard so a (should-be-impossible) cycle in
  `groups.parent_id` can't spin the recursive CTE unbounded and hang a plan.
- Drop the now-false "memoized per group" line from `group_blast_radius`'s doc
  (it is a single set-based query now).
- Correct the `PlanRequest` flatten comment: apply is deliberately NOT flattened
  (its pre-§7 wire was already `{bundle, …}`), so the two endpoints are
  asymmetric by history — the old comment wrongly claimed parity.

Verified: the blast-radius journey + ownership journeys still pass; the INNER
JOIN to `projects` is safe (owner_project FK is ON DELETE SET NULL, so a
non-NULL owner always references a live row — no dangling drop).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-07 19:21:03 +02:00
parent 86b51df50d
commit 76926de369
2 changed files with 9 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
crates/manager-core/src/apply_api.rs
View File

@@ -183,11 +183,13 @@ async fn group_extension_points_handler(
/// `[project]`, so the plan can preview the ownership outcome + attach ceiling. /// `[project]`, so the plan can preview the ownership outcome + attach ceiling.
#[derive(Deserialize)] #[derive(Deserialize)]
pub struct PlanRequest { pub struct PlanRequest {
// Flattened so the bundle fields sit at the JSON top level: an older `pic` // Flattened so the bundle fields sit at the JSON top level — restoring the
// (pre-§7) that POSTs a bare bundle still deserializes (`project` defaults // pre-§7 plan wire, which took a BARE `Json<Bundle>`. An older `pic` that
// to `None`), and a newer client sends the same bundle fields plus a // POSTs a bare bundle still deserializes (`project` defaults to `None`), and
// top-level `project` key. Keeps the plan wire compatible in both skew // a newer client sends the same bundle fields plus a top-level `project`
// directions, matching the additive compatibility of the apply request. // key. (Apply is deliberately NOT flattened — its pre-§7 wire was already
// `{bundle, prune, …}`, so flattening it would instead break old apply
// clients; the two endpoints are asymmetric because their histories are.)
#[serde(flatten)] #[serde(flatten)]
pub bundle: Bundle, pub bundle: Bundle,
#[serde(default)] #[serde(default)]

5
crates/manager-core/src/apply_service.rs
View File

@@ -2192,8 +2192,7 @@ impl ApplyService {
/// §7 M3.2: descendant apps of `gid` grouped by their nearest-claimed- /// §7 M3.2: descendant apps of `gid` grouped by their nearest-claimed-
/// ancestor project, EXCLUDING `exclude` (the planning project) — the apps /// ancestor project, EXCLUDING `exclude` (the planning project) — the apps
/// a change to `gid`'s inherited config fans out to across other repos. /// a change to `gid`'s inherited config fans out to across other repos.
/// Ancestor resolution is memoized per group, so the cost is one walk per /// Resolved in one set-based recursive query (see below).
/// distinct descendant group, not per app.
async fn group_blast_radius( async fn group_blast_radius(
&self, &self,
gid: GroupId, gid: GroupId,
@@ -2218,7 +2217,7 @@ impl ApplyService {
UNION ALL UNION ALL
SELECT ac.app_id, g.parent_id, ac.depth + 1 SELECT ac.app_id, g.parent_id, ac.depth + 1
FROM app_chain ac JOIN groups g ON g.id = ac.gid FROM app_chain ac JOIN groups g ON g.id = ac.gid
WHERE g.parent_id IS NOT NULL WHERE g.parent_id IS NOT NULL AND ac.depth < 64
), ),
nearest AS ( nearest AS (
SELECT DISTINCT ON (ac.app_id) ac.app_id, g.owner_project SELECT DISTINCT ON (ac.app_id) ac.app_id, g.owner_project