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>
321 lines
11 KiB
Rust
321 lines
11 KiB
Rust
//! Phase 4-lite group-owned scripts, end to end via `pic`:
|
|
//! * a group owns an endpoint script; an app *under* the group inherits it,
|
|
//! resolving by name down the chain (`invoke`),
|
|
//! * an app's own script of the same name shadows the inherited one (CoW),
|
|
//! * an app NOT under the group cannot resolve it (isolation),
|
|
//! * a manifest binds a route to the inherited script declaratively, and
|
|
//! re-applying is an idempotent no-op.
|
|
|
|
use std::fs;
|
|
|
|
use tempfile::TempDir;
|
|
|
|
use crate::common;
|
|
use crate::common::cleanup::{AppGuard, GroupGuard, ScriptGuard};
|
|
|
|
fn manifest_dir() -> TempDir {
|
|
let dir = TempDir::new().expect("tempdir");
|
|
fs::create_dir_all(dir.path().join("scripts")).expect("scripts dir");
|
|
dir
|
|
}
|
|
|
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
|
#[test]
|
|
fn group_script_is_inherited_with_cow_and_isolation() {
|
|
let Some(fx) = common::fixture_or_skip() else {
|
|
return;
|
|
};
|
|
let env = common::admin_env(fx);
|
|
let group = common::unique_slug("gs-grp");
|
|
let child = common::unique_slug("gs-child");
|
|
let orphan = common::unique_slug("gs-orphan");
|
|
|
|
// Group with an endpoint script `shared` returning a marker.
|
|
// GroupGuard first so it drops LAST — after the app + group script below.
|
|
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/shared.rhai"), "\"from-group\"").unwrap();
|
|
common::pic_as(&env)
|
|
.args(["scripts", "deploy"])
|
|
.arg(dir.path().join("scripts/shared.rhai"))
|
|
.args(["--group", &group, "--name", "shared"])
|
|
.assert()
|
|
.success();
|
|
// The group script blocks group deletion (ON DELETE RESTRICT) — guard it
|
|
// so it drops before the GroupGuard.
|
|
let gscript_id = group_script_id(&env, &group);
|
|
let _gs = ScriptGuard::new(&env.url, &env.token, &gscript_id);
|
|
|
|
// An app under the group inherits `shared`. Its `caller` script invokes it.
|
|
let _child = AppGuard::new(&env.url, &env.token, &child);
|
|
common::pic_as(&env)
|
|
.args(["apps", "create", &child, "--group", &group])
|
|
.assert()
|
|
.success();
|
|
fs::write(
|
|
dir.path().join("scripts/caller.rhai"),
|
|
"invoke(\"shared\", #{})",
|
|
)
|
|
.unwrap();
|
|
common::pic_as(&env)
|
|
.args(["scripts", "deploy"])
|
|
.arg(dir.path().join("scripts/caller.rhai"))
|
|
.args(["--app", &child, "--name", "caller"])
|
|
.assert()
|
|
.success();
|
|
let caller_id = app_script_id(&env, &child, "caller");
|
|
|
|
// Inheritance: caller resolves `shared` down its chain → the group's value.
|
|
assert_eq!(
|
|
invoke_body(&env, &caller_id),
|
|
serde_json::json!("from-group"),
|
|
"inherited group script should resolve by name"
|
|
);
|
|
|
|
// CoW: the app defines its OWN `shared`; the same caller now sees it.
|
|
fs::write(dir.path().join("scripts/own.rhai"), "\"from-app\"").unwrap();
|
|
common::pic_as(&env)
|
|
.args(["scripts", "deploy"])
|
|
.arg(dir.path().join("scripts/own.rhai"))
|
|
.args(["--app", &child, "--name", "shared"])
|
|
.assert()
|
|
.success();
|
|
assert_eq!(
|
|
invoke_body(&env, &caller_id),
|
|
serde_json::json!("from-app"),
|
|
"an app's own script must shadow the inherited group one (CoW)"
|
|
);
|
|
|
|
// Isolation: an app NOT under the group cannot resolve `shared`.
|
|
let _orphan = AppGuard::new(&env.url, &env.token, &orphan);
|
|
common::pic_as(&env)
|
|
.args(["apps", "create", &orphan])
|
|
.assert()
|
|
.success();
|
|
common::pic_as(&env)
|
|
.args(["scripts", "deploy"])
|
|
.arg(dir.path().join("scripts/caller.rhai"))
|
|
.args(["--app", &orphan, "--name", "caller"])
|
|
.assert()
|
|
.success();
|
|
let orphan_caller = app_script_id(&env, &orphan, "caller");
|
|
// `shared` is not in the orphan's chain → invoke fails (the script errors).
|
|
let out = common::pic_as(&env)
|
|
.args(["scripts", "invoke", &orphan_caller])
|
|
.output()
|
|
.expect("invoke");
|
|
assert!(
|
|
!out.status.success(),
|
|
"an app outside the group must NOT reach the group's `shared`"
|
|
);
|
|
}
|
|
|
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
|
#[test]
|
|
fn manifest_binds_route_to_inherited_group_script_idempotently() {
|
|
let Some(fx) = common::fixture_or_skip() else {
|
|
return;
|
|
};
|
|
let env = common::admin_env(fx);
|
|
let group = common::unique_slug("gsr-grp");
|
|
let child = common::unique_slug("gsr-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/greet.rhai"), "\"hi\"").unwrap();
|
|
common::pic_as(&env)
|
|
.args(["scripts", "deploy"])
|
|
.arg(dir.path().join("scripts/greet.rhai"))
|
|
.args(["--group", &group, "--name", "greet"])
|
|
.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();
|
|
|
|
// Manifest declares NO `greet` script — only a route binding to it. The
|
|
// apply engine must resolve `greet` to the inherited group endpoint.
|
|
let manifest = format!(
|
|
"[app]\nslug = \"{child}\"\nname = \"GSR\"\n\n\
|
|
[[routes]]\nscript = \"greet\"\nmethod = \"GET\"\n\
|
|
host_kind = \"any\"\npath_kind = \"exact\"\npath = \"/greet\"\n"
|
|
);
|
|
let manifest_path = dir.path().join("picloud.toml");
|
|
fs::write(&manifest_path, &manifest).unwrap();
|
|
|
|
// Plan: a route create that binds to `greet`, and NO script create.
|
|
let plan = String::from_utf8(
|
|
common::pic_as(&env)
|
|
.args(["plan", "--file"])
|
|
.arg(&manifest_path)
|
|
.output()
|
|
.unwrap()
|
|
.stdout,
|
|
)
|
|
.unwrap();
|
|
assert!(
|
|
plan.contains("greet") && plan.contains("route"),
|
|
"plan should bind a route to the inherited `greet`:\n{plan}"
|
|
);
|
|
|
|
common::pic_as(&env)
|
|
.args(["apply", "--file"])
|
|
.arg(&manifest_path)
|
|
.assert()
|
|
.success();
|
|
|
|
// Re-plan is a clean no-op — the diff resolved the group-bound route's id
|
|
// back to `greet`, so it is not a perpetual rebind.
|
|
let replan = String::from_utf8(
|
|
common::pic_as(&env)
|
|
.args(["plan", "--file"])
|
|
.arg(&manifest_path)
|
|
.output()
|
|
.unwrap()
|
|
.stdout,
|
|
)
|
|
.unwrap();
|
|
assert!(
|
|
!replan.contains("create") && !replan.contains("update"),
|
|
"re-plan after binding an inherited route must be a no-op:\n{replan}"
|
|
);
|
|
}
|
|
|
|
#[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)
|
|
.args(["scripts", "ls", "--group", group])
|
|
.output()
|
|
.expect("scripts ls --group");
|
|
common::parse_first_id(std::str::from_utf8(&ls.stdout).unwrap())
|
|
.expect("group should have one script")
|
|
}
|
|
|
|
/// Id of the named script in an app (`pic scripts ls --app <a>`).
|
|
fn app_script_id(env: &common::TestEnv, app: &str, name: &str) -> String {
|
|
let ls = common::pic_as(env)
|
|
.args(["scripts", "ls", "--app", app])
|
|
.output()
|
|
.expect("scripts ls --app");
|
|
let table = String::from_utf8(ls.stdout).unwrap();
|
|
// Rows are `id\tapp_slug\tname\tversion\tupdated_at`; pick the wanted name.
|
|
table
|
|
.lines()
|
|
.map(common::cells)
|
|
.find(|c| c.get(2) == Some(&name))
|
|
.and_then(|c| c.first().map(|s| (*s).to_string()))
|
|
.unwrap_or_else(|| panic!("script `{name}` not found in app `{app}`:\n{table}"))
|
|
}
|
|
|
|
fn invoke_body(env: &common::TestEnv, id: &str) -> serde_json::Value {
|
|
let out = common::pic_as(env)
|
|
.args(["scripts", "invoke", id])
|
|
.output()
|
|
.expect("scripts invoke");
|
|
assert!(
|
|
out.status.success(),
|
|
"invoke failed: {}",
|
|
String::from_utf8_lossy(&out.stderr)
|
|
);
|
|
serde_json::from_slice(&out.stdout).expect("invoke body is JSON")
|
|
}
|