🩹 fix(policy): cascade pin/unpin re-materialise to descendants

Author: chaizhenhua

crates/oversight-server/src/api.rs

@@ -781,8 +781,10 @@ async fn pin_task_role(
     )?;
     drop(store);
 
-    // Re-materialise so the assignment row reflects the pin.
-    materialise_task_roles(&state, &task).await;
+    // Re-materialise so the assignment row reflects the pin AND cascade
+    // to descendants — children that resolve via `parent_role` would
+    // otherwise keep a stale accountable until someone touched them.
+    materialise_task_and_descendants(&state, &task).await;
 
     let store = state.store.lock().await;
     let resolved = store
@@ -813,7 +815,9 @@ async fn unpin_task_role(
         &serde_json::json!({}),
     )?;
     drop(store);
-    materialise_task_roles(&state, &task).await;
+    // Unpin restores the rule chain for this task and (transitively) for
+    // any descendants that inherited from it via `parent_role`.
+    materialise_task_and_descendants(&state, &task).await;
     Ok(StatusCode::NO_CONTENT)
 }
 
@@ -2276,6 +2280,59 @@ async fn materialise_task_roles(state: &AppState, task: &oversight_models::Task)
     }
 }
 
+/// Re-materialise the task and every descendant. Per ADR-004
+/// §"Sub-task accountability inheritance", changes to a parent's
+/// accountable (typically via pin / unpin) must cascade to children
+/// that inherit via `parent_role`. The lookup happens once per
+/// descendant under a single store lock to keep the scan cheap.
+///
+/// The traversal is breadth-first with a hard depth ceiling to defend
+/// against pathological data (e.g. an accidentally cyclic parent_id
+/// chain — schema doesn't forbid it).
+async fn materialise_task_and_descendants(state: &AppState, root: &oversight_models::Task) {
+    materialise_task_roles(state, root).await;
+
+    const MAX_VISITED: usize = 4096;
+    let to_remateriaise: Vec<oversight_models::Task> = {
+        let store = state.store.lock().await;
+        let mut queue: std::collections::VecDeque<String> =
+            std::collections::VecDeque::from([root.id.clone()]);
+        let mut seen: std::collections::HashSet<String> =
+            std::collections::HashSet::from([root.id.clone()]);
+        let mut out: Vec<oversight_models::Task> = Vec::new();
+        while let Some(parent_id) = queue.pop_front() {
+            if seen.len() >= MAX_VISITED {
+                tracing::warn!(
+                    root = %root.id,
+                    visited = seen.len(),
+                    "cascade depth ceiling hit; truncating descendant walk"
+                );
+                break;
+            }
+            let children = match store.list_child_task_ids(&parent_id) {
+                Ok(c) => c,
+                Err(e) => {
+                    tracing::warn!(parent = %parent_id, error = ?e, "child lookup failed");
+                    continue;
+                }
+            };
+            for child_id in children {
+                if seen.insert(child_id.clone()) {
+                    queue.push_back(child_id.clone());
+                    if let Ok(t) = store.get_task(&child_id) {
+                        out.push(t);
+                    }
+                }
+            }
+        }
+        out
+    };
+
+    for child in to_remateriaise {
+        materialise_task_roles(state, &child).await;
+    }
+}
+
 #[tracing::instrument(
     level = "info",
     name = "create_task",

crates/oversight-server/tests/policy_deep.rs

@@ -185,6 +185,110 @@ async fn child_task_inherits_parent_accountable_via_resolver_rule() {
     }
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn pin_handler_cascades_to_descendants_without_manual_remateriaise() {
+    // Drives the *real* HTTP pin handler, which now runs
+    // `materialise_task_and_descendants` after writing the parent pin.
+    // No manual child re-materialise — if the cascade is wired right,
+    // the child's accountable updates automatically.
+    let state = build_sqlite_app_state().await;
+    let user = "user-deep-cascade-pin";
+    let (project, wf, parent) = seed_world(&state, user, "cascade-pin");
+
+    // Add a second user we can pin to.
+    let pin_target_id = format!("uuid-{}", uuid::Uuid::new_v4());
+    {
+        let store = state.store.try_lock().expect("store");
+        store
+            .insert_user(&make_user(&pin_target_id, "Pin Target"))
+            .unwrap();
+        store
+            .add_project_member(&project.id, "user", &pin_target_id, Some("member"))
+            .unwrap();
+    }
+
+    // Spawn a child + grandchild — both inherit accountable via parent_role.
+    let mut child = Task::new("child", "bug", &wf.id, "backlog");
+    child.id = format!("task-cas-c-{}", uuid::Uuid::new_v4());
+    child.project_id = Some(project.id.clone());
+    child.parent_id = Some(parent.id.clone());
+    child.assignees = vec![oversight_models::TaskAssignee::executor("agent:claude")];
+
+    let mut grandchild = Task::new("grandchild", "bug", &wf.id, "backlog");
+    grandchild.id = format!("task-cas-gc-{}", uuid::Uuid::new_v4());
+    grandchild.project_id = Some(project.id.clone());
+    grandchild.parent_id = Some(child.id.clone());
+    grandchild.assignees = vec![oversight_models::TaskAssignee::executor("agent:claude")];
+
+    {
+        let store = state.store.try_lock().expect("store");
+        store.insert_task(&child).unwrap();
+        materialise(&store, &child);
+        store.insert_task(&grandchild).unwrap();
+        materialise(&store, &grandchild);
+
+        // Sanity: both descendants currently inherit `user`.
+        let c_acc = store
+            .get_role_assignment(&child.id, RoleName::Accountable)
+            .unwrap()
+            .unwrap();
+        assert_eq!(c_acc.actor_id, user, "child starts inheriting from parent");
+        let gc_acc = store
+            .get_role_assignment(&grandchild.id, RoleName::Accountable)
+            .unwrap()
+            .unwrap();
+        assert_eq!(
+            gc_acc.actor_id, user,
+            "grandchild starts inheriting transitively"
+        );
+    }
+
+    // Drive the real handler.
+    grant_admin(&state, user);
+    reload_authorizer(&state);
+    let app = build_api_router(state.clone());
+    let resp = app
+        .oneshot(
+            Request::builder()
+                .method("POST")
+                .uri(format!("/api/tasks/{}/roles/accountable/pin", parent.id))
+                .header("content-type", "application/json")
+                .header("x-actor", format!("user:{user}"))
+                .body(Body::from(json!({"actor_id": pin_target_id}).to_string()))
+                .unwrap(),
+        )
+        .await
+        .expect("pin");
+    assert_eq!(resp.status(), StatusCode::OK);
+
+    // No manual materialise calls. Both descendants must already see the
+    // new pin target.
+    let store = state.store.try_lock().expect("store");
+    let parent_acc = store
+        .get_role_assignment(&parent.id, RoleName::Accountable)
+        .unwrap()
+        .unwrap();
+    assert_eq!(parent_acc.actor_id, pin_target_id);
+
+    let child_acc = store
+        .get_role_assignment(&child.id, RoleName::Accountable)
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        child_acc.actor_id, pin_target_id,
+        "cascade must update direct child without manual re-materialise"
+    );
+
+    let grand_acc = store
+        .get_role_assignment(&grandchild.id, RoleName::Accountable)
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        grand_acc.actor_id, pin_target_id,
+        "cascade must traverse the full subtree (parent → child → grandchild)"
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn parent_pin_propagates_to_child_on_remateriaise() {
     let state = build_sqlite_app_state().await;

crates/oversight-store/src/repo_task.rs

@@ -448,6 +448,18 @@ impl Store {
         Ok(count > 0)
     }
 
+    /// Direct children of `parent_id` (one level only). Used by the policy
+    /// engine to cascade re-materialisation when a parent's accountable
+    /// changes — see ADR-004 §"Sub-task accountability inheritance".
+    pub fn list_child_task_ids(&self, parent_id: &str) -> Result<Vec<String>, StoreError> {
+        let conn = self.connection()?;
+        let mut stmt = conn.prepare("SELECT id FROM tasks WHERE parent_id = ?1")?;
+        let ids = stmt
+            .query_map(params![parent_id], |row| row.get::<_, String>(0))?
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok(ids)
+    }
+
     // ── Proposals ──────────────────────────────────────
 
     /// Reject all proposed tasks in `project_id` whose `proposal_created_at` is before