fix(apply): bound the blast-radius up-walk; fix two stale comments
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:
@@ -2192,8 +2192,7 @@ impl ApplyService {
|
||||
/// §7 M3.2: descendant apps of `gid` grouped by their nearest-claimed-
|
||||
/// ancestor project, EXCLUDING `exclude` (the planning project) — the apps
|
||||
/// 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
|
||||
/// distinct descendant group, not per app.
|
||||
/// Resolved in one set-based recursive query (see below).
|
||||
async fn group_blast_radius(
|
||||
&self,
|
||||
gid: GroupId,
|
||||
@@ -2218,7 +2217,7 @@ impl ApplyService {
|
||||
UNION ALL
|
||||
SELECT ac.app_id, g.parent_id, ac.depth + 1
|
||||
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 (
|
||||
SELECT DISTINCT ON (ac.app_id) ac.app_id, g.owner_project
|
||||
|
||||
Reference in New Issue
Block a user