fix(apply): resolve inherited-bound triggers in diff, state-token, and prune

Adversarial review of Phase 4-lite found two real gaps, both rooted in the
apply engine resolving script `id → name` from `current.scripts` (app-own)
only — so a route/trigger bound to an inherited GROUP script (whose id is not
app-own) couldn't be named:

  * **Prune gap (reachable via pure declarative apply):** a trigger bound to an
    inherited group script, once dropped from the manifest, never diffed to a
    Delete (the diff couldn't resolve its identity once the bundle stopped
    referencing the name), so `apply --prune` silently orphaned it.
  * **Bound-plan false-negative:** `state_token` excluded such a trigger from
    its fingerprint, so the StateMoved check missed a concurrent edit to that
    binding class. (Routes were already safe — they hash the raw `script_id`.)

Fix: `resolve_current_target_names` resolves the names of group scripts the
app's CURRENT routes/triggers are bound to (by id, independent of the bundle),
and that map now feeds `compute_diff_with_names`, `state_token_with_names`, and
the prune trigger-identity map. App-owned bindings are unaffected (their names
are already in `current.scripts`). Also fixes a stale "in this app" conflict
message for group-owned scripts.

New journey `inherited_bound_trigger_is_pruned_when_dropped` proves prune now
removes an inherited-bound cron trigger. Full journey suite 105/105.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

crates/manager-core/src/apply_service.rs

@@ -423,12 +423,13 @@ impl ApplyService {
         self.validate_bundle(bundle, &keys_set(&inherited))?;
         self.validate_route_hosts(app_id, bundle).await?;
         let current = self.load_current(app_id).await?;
+        let current_names = self.resolve_current_target_names(&current).await?;
         validate_email_secrets_present(bundle, &current.secret_names)?;
         Ok(PlanResult {
-            plan: compute_diff_with_inherited(&current, bundle, &inherited),
+            plan: compute_diff_with_names(&current, bundle, &current_names),
             // Fingerprint of the live state this plan was computed against, so
             // `apply` can refuse if the app changed underneath it (§4.2).
-            state_token: state_token(&current),
+            state_token: state_token_with_names(&current, &current_names),
         })
     }
 
@@ -471,6 +472,9 @@ impl ApplyService {
             .map_err(|e| ApplyError::Backend(e.to_string()))?;
 
         let current = self.load_current(app_id).await?;
+        // Phase 4: names of group scripts the current routes/triggers bind to,
+        // so the token / diff / prune resolve inherited bindings (see method).
+        let current_names = self.resolve_current_target_names(&current).await?;
         // Bound-plan check (§4.2): if the caller passed the token from a prior
         // `pic plan`, refuse when the live state changed since. Computed from
         // the same `load_current` snapshot the diff uses, before any mutation
@@ -480,14 +484,14 @@ impl ApplyService {
         // apply-vs-interactive-write window — it is not a substitute for the
         // tx-scoped read the follow-up will add.
         if let Some(expected) = expected_token {
-            if state_token(&current) != expected {
+            if state_token_with_names(&current, &current_names) != expected {
                 return Err(ApplyError::StateMoved);
             }
         }
         // Surface a missing email-secret reference here so `plan` and `apply`
         // agree, rather than only failing deep in `resolve_and_seal` below.
         validate_email_secrets_present(bundle, &current.secret_names)?;
-        let plan = compute_diff_with_inherited(&current, bundle, &inherited);
+        let plan = compute_diff_with_names(&current, bundle, &current_names);
         let mut report = ApplyReport::default();
         // §4.7 warning: an enabled binding pointing at a disabled script is
         // deployed-but-unreachable. Not an error (it's valid desired state),
@@ -700,10 +704,14 @@ impl ApplyService {
         // Secret pruning is deliberately deferred (destructive + irreversible):
         // stale secrets are surfaced in the plan but never deleted here.
         if prune {
+            // Include inherited group-script bindings (Phase 4) so a trigger
+            // bound to a group script resolves its identity and is pruned when
+            // dropped from the manifest — not silently orphaned.
             let id_to_name: HashMap<ScriptId, String> = current
                 .scripts
                 .iter()
                 .map(|s| (s.id, s.name.clone()))
+                .chain(current_names.iter().map(|(id, n)| (*id, n.clone())))
                 .collect();
             let trigger_id_by_identity: HashMap<String, TriggerId> = current
                 .triggers
@@ -981,6 +989,38 @@ impl ApplyService {
         Ok(out)
     }
 
+    /// Phase 4: resolve the names of group-owned scripts the app's CURRENT
+    /// routes/triggers are bound to, `script_id → name`. These ids aren't in
+    /// `current.scripts` (app-own only), so this is what lets the diff /
+    /// state-token / prune resolve an existing inherited binding back to its
+    /// name — independent of whether the bundle still references it (so a
+    /// dropped inherited binding still diffs to a Delete, and an inherited-bound
+    /// trigger still moves the state token).
+    async fn resolve_current_target_names(
+        &self,
+        current: &CurrentState,
+    ) -> Result<HashMap<ScriptId, String>, ApplyError> {
+        let own: HashSet<ScriptId> = current.scripts.iter().map(|s| s.id).collect();
+        let mut ids: HashSet<ScriptId> = HashSet::new();
+        for r in &current.routes {
+            if !own.contains(&r.script_id) {
+                ids.insert(r.script_id);
+            }
+        }
+        for t in &current.triggers {
+            if !own.contains(&t.script_id) {
+                ids.insert(t.script_id);
+            }
+        }
+        let mut out = HashMap::new();
+        for id in ids {
+            if let Some(s) = self.scripts.get(id).await.map_err(map_repo)? {
+                out.insert(id, s.name);
+            }
+        }
+        Ok(out)
+    }
+
     /// `inherited_endpoints` (lowercased names) are group-owned endpoint
     /// scripts reachable from the app's chain that a route/trigger may bind to
     /// even though the manifest doesn't declare them (Phase 4). They count as
@@ -1175,26 +1215,28 @@ impl ApplyService {
 /// Diff desired `bundle` against `current`. Pure — unit-tested directly.
 #[must_use]
 pub fn compute_diff(current: &CurrentState, bundle: &Bundle) -> Plan {
-    compute_diff_with_inherited(current, bundle, &HashMap::new())
+    compute_diff_with_names(current, bundle, &HashMap::new())
 }
 
-/// `compute_diff` with the set of inherited group-endpoint targets (Phase 4),
-/// `name(lowercased) → script_id`. Those ids are added to the script-name map
-/// so a route/trigger bound to an inherited group script resolves to its name
-/// and re-apply is idempotent (otherwise the unknown id reads as a rebind
-/// every plan). The public `compute_diff` passes an empty map (no inheritance).
-fn compute_diff_with_inherited(
+/// `compute_diff` with `current_names` — the names of group-owned (inherited)
+/// scripts the app's CURRENT routes/triggers are bound to, `script_id → name`
+/// (Phase 4). Those ids aren't in `current.scripts` (app-own only), so without
+/// them a current binding to a group script resolves to no name: it reads as a
+/// perpetual rebind, and a binding dropped from the manifest never diffs to a
+/// Delete (so `--prune` can't remove it). The public `compute_diff` passes an
+/// empty map (no inheritance).
+fn compute_diff_with_names(
     current: &CurrentState,
     bundle: &Bundle,
-    inherited: &HashMap<String, ScriptId>,
+    current_names: &HashMap<ScriptId, String>,
 ) -> Plan {
     let script_name_by_id: HashMap<ScriptId, String> = current
         .scripts
         .iter()
         .map(|s| (s.id, s.name.clone()))
-        // Inherited group targets, inverted to id → name. Disjoint from
-        // app-own ids by construction, so order doesn't matter.
-        .chain(inherited.iter().map(|(name, id)| (*id, name.clone())))
+        // Inherited group bindings' ids → names. Disjoint from app-own ids by
+        // construction, so order doesn't matter.
+        .chain(current_names.iter().map(|(id, n)| (*id, n.clone())))
         .collect();
 
     Plan {
@@ -1991,6 +2033,17 @@ fn map_trig(e: TriggerRepoError) -> ApplyError {
 /// is the same risk class as not checking at all — never a false refusal.
 #[must_use]
 pub fn state_token(current: &CurrentState) -> String {
+    state_token_with_names(current, &HashMap::new())
+}
+
+/// `state_token` with `current_names` (group-bound binding ids → names, Phase
+/// 4) so a trigger bound to an inherited group script contributes to the
+/// fingerprint. Without it such a trigger is silently excluded and the
+/// bound-plan (StateMoved) check has a false-negative for that binding class.
+fn state_token_with_names(
+    current: &CurrentState,
+    current_names: &HashMap<ScriptId, String>,
+) -> String {
     let mut parts: Vec<String> = Vec::with_capacity(
         current.scripts.len()
             + current.routes.len()
@@ -2031,6 +2084,7 @@ pub fn state_token(current: &CurrentState) -> String {
         .scripts
         .iter()
         .map(|s| (s.id, s.name.clone()))
+        .chain(current_names.iter().map(|(id, n)| (*id, n.clone())))
         .collect();
     for t in &current.triggers {
         if let Some(id) = current_trigger_identity(t, &script_name_by_id) {

crates/manager-core/src/repo.rs

@@ -591,7 +591,7 @@ pub(crate) async fn update_script_tx(
         Ok(None) => return Err(ScriptRepositoryError::NotFound(id)),
         Err(sqlx::Error::Database(e)) if e.is_unique_violation() => {
             return Err(ScriptRepositoryError::Conflict(
-                "a script with that name already exists in this app".into(),
+                "a script with that name already exists in this owner".into(),
             ));
         }
         Err(e) => return Err(e.into()),

crates/picloud-cli/tests/group_scripts.rs

@@ -196,6 +196,90 @@ fn manifest_binds_route_to_inherited_group_script_idempotently() {
     );
 }
 
+#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
+#[test]
+fn inherited_bound_trigger_is_pruned_when_dropped() {
+    // Regression: a trigger bound to an inherited group script must still
+    // resolve its identity in the diff/prune path even after the manifest
+    // stops referencing it — otherwise `--prune` silently orphans it.
+    let Some(fx) = common::fixture_or_skip() else {
+        return;
+    };
+    let env = common::admin_env(fx);
+    let group = common::unique_slug("gst-grp");
+    let child = common::unique_slug("gst-child");
+
+    let _g = GroupGuard::new(&env.url, &env.token, &group);
+    common::pic_as(&env)
+        .args(["groups", "create", &group])
+        .assert()
+        .success();
+
+    let dir = manifest_dir();
+    fs::write(dir.path().join("scripts/work.rhai"), "\"ok\"").unwrap();
+    common::pic_as(&env)
+        .args(["scripts", "deploy"])
+        .arg(dir.path().join("scripts/work.rhai"))
+        .args(["--group", &group, "--name", "work"])
+        .assert()
+        .success();
+    let gscript_id = group_script_id(&env, &group);
+    let _gs = ScriptGuard::new(&env.url, &env.token, &gscript_id);
+
+    let _child = AppGuard::new(&env.url, &env.token, &child);
+    common::pic_as(&env)
+        .args(["apps", "create", &child, "--group", &group])
+        .assert()
+        .success();
+
+    let manifest_path = dir.path().join("picloud.toml");
+    // 1. Manifest binds a cron trigger to the INHERITED `work`.
+    let with_trigger = format!(
+        "[app]\nslug = \"{child}\"\nname = \"GST\"\n\n\
+         [[triggers.cron]]\nscript = \"work\"\nschedule = \"0 0 * * * *\"\ntimezone = \"UTC\"\n"
+    );
+    fs::write(&manifest_path, &with_trigger).unwrap();
+    common::pic_as(&env)
+        .args(["apply", "--file"])
+        .arg(&manifest_path)
+        .assert()
+        .success();
+    let listed = String::from_utf8(
+        common::pic_as(&env)
+            .args(["triggers", "ls", "--app", &child])
+            .output()
+            .unwrap()
+            .stdout,
+    )
+    .unwrap();
+    assert!(
+        listed.contains("cron"),
+        "cron trigger bound to inherited `work` should exist:\n{listed}"
+    );
+
+    // 2. Drop the trigger + `--prune` → it must be removed (not orphaned).
+    let empty = format!("[app]\nslug = \"{child}\"\nname = \"GST\"\n");
+    fs::write(&manifest_path, &empty).unwrap();
+    common::pic_as(&env)
+        .args(["apply", "--file"])
+        .arg(&manifest_path)
+        .args(["--prune", "--yes"])
+        .assert()
+        .success();
+    let after = String::from_utf8(
+        common::pic_as(&env)
+            .args(["triggers", "ls", "--app", &child])
+            .output()
+            .unwrap()
+            .stdout,
+    )
+    .unwrap();
+    assert!(
+        !after.contains("cron"),
+        "prune must remove the inherited-bound cron trigger:\n{after}"
+    );
+}
+
 /// First script id from `pic scripts ls --group <g>`.
 fn group_script_id(env: &common::TestEnv, group: &str) -> String {
     let ls = common::pic_as(env)