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

feat(hierarchies): template {var:NAME} resolves in-transaction (M6)

Browse Source 
Route/trigger template `{var:NAME}` placeholders now resolve against vars
applied in the SAME `pic apply` transaction, not committed-only state — so a
var and a template referencing it converge in one apply instead of two.

- config_resolver: add tx-scoped `fetch_var_candidates_tx`, sharing the SQL
  tail (`VAR_CANDIDATES_TAIL`) with the pool variant so they can't drift.
- apply_service: `expand_route_templates_tx`/`expand_trigger_templates_tx`
  read vars via the tx (vars are reconciled in Phase A / the app's own
  reconcile, both before Phase B expansion).
- templates journey: `route_template_var_resolves_in_same_apply` — would fail
  against the old committed-only read.
- doc §4.5: strike the `{var:NAME}` committed-only limitation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-28 18:11:44 +02:00
parent 4d2eed4e81
commit 7c17d6e363
4 changed files with 116 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -2184,10 +2184,11 @@ impl ApplyService {
}
}
// Effective vars for `{var:NAME}` — committed state (a var set in this
// same apply is visible to templates on the NEXT apply, not this one).
// Effective vars for `{var:NAME}` — read in-transaction so a var set in
// THIS same apply (reconciled earlier in the tx) is visible to the
// template now, not one apply late.
let vars = {
let cands = crate::config_resolver::fetch_var_candidates(&self.pool, app_id)
let cands = crate::config_resolver::fetch_var_candidates_tx(tx, app_id)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
crate::config_resolver::resolve(cands).0
@@ -2428,9 +2429,10 @@ impl ApplyService {
}
}
// Effective vars for `{var:NAME}` (committed state — see route expansion).
// Effective vars for `{var:NAME}` — read in-transaction (see route
// expansion): a var set in this same apply is visible now.
let vars = {
let cands = crate::config_resolver::fetch_var_candidates(&self.pool, app_id)
let cands = crate::config_resolver::fetch_var_candidates_tx(tx, app_id)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
crate::config_resolver::resolve(cands).0

45
crates/manager-core/src/config_resolver.rs
View File

@@ -226,17 +226,7 @@ pub async fn fetch_var_candidates(
pool: &PgPool,
app_id: AppId,
) -> Result<Vec<Candidate>, sqlx::Error> {
let sql = format!(
"{CHAIN_LEVELS_CTE} \
SELECT c.depth, \
CASE WHEN v.app_id IS NOT NULL THEN 'app' ELSE 'group' END AS owner_kind, \
COALESCE(v.app_id, v.group_id) AS owner_id, \
v.environment_scope, v.key, v.value, v.is_tombstone \
FROM chain c \
JOIN vars v ON (v.app_id = c.app_owner OR v.group_id = c.group_owner) \
WHERE v.environment_scope = '*' OR v.environment_scope = c.app_env \
ORDER BY c.depth ASC"
);
let sql = format!("{CHAIN_LEVELS_CTE} {VAR_CANDIDATES_TAIL}");
let rows = sqlx::query_as::<_, VarCandidateRow>(&sql)
.bind(app_id.into_inner())
.fetch_all(pool)
@@ -244,6 +234,39 @@ pub async fn fetch_var_candidates(
Ok(rows.into_iter().map(Into::into).collect())
}
/// Transaction-scoped twin of [`fetch_var_candidates`]: runs the identical
/// `CHAIN_LEVELS_CTE` query against an open transaction, so it sees vars
/// **written earlier in the same transaction** (not just committed state).
/// Used by template expansion so a `{var:NAME}` placeholder resolves against
/// a var set in the *same* `pic apply` rather than landing one apply late.
///
/// # Errors
/// Propagates sqlx errors.
pub async fn fetch_var_candidates_tx(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
app_id: AppId,
) -> Result<Vec<Candidate>, sqlx::Error> {
let sql = format!("{CHAIN_LEVELS_CTE} {VAR_CANDIDATES_TAIL}");
let rows = sqlx::query_as::<_, VarCandidateRow>(&sql)
.bind(app_id.into_inner())
.fetch_all(&mut **tx)
.await?;
Ok(rows.into_iter().map(Into::into).collect())
}
/// The `SELECT … FROM chain …` tail appended after [`CHAIN_LEVELS_CTE`] to
/// pull env-eligible `vars` candidates, nearest-first. Shared by the pool and
/// transaction variants so the two can't drift.
const VAR_CANDIDATES_TAIL: &str = "\
SELECT c.depth, \
CASE WHEN v.app_id IS NOT NULL THEN 'app' ELSE 'group' END AS owner_kind, \
COALESCE(v.app_id, v.group_id) AS owner_id, \
v.environment_scope, v.key, v.value, v.is_tombstone \
FROM chain c \
JOIN vars v ON (v.app_id = c.app_owner OR v.group_id = c.group_owner) \
WHERE v.environment_scope = '*' OR v.environment_scope = c.app_env \
ORDER BY c.depth ASC";
/// The masked, resolved view of one inherited secret for `config/effective`:
/// which owner/level/scope supplies it — **never** the value.
#[derive(Debug, Clone)]

72
crates/picloud-cli/tests/templates.rs
View File

@@ -159,6 +159,78 @@ fn route_template_fans_out_per_app_idempotently_then_prunes() {
);
}
/// M6 (§4.5): a `{var:NAME}` placeholder resolves against a var set in the SAME
/// apply (in-transaction), so a var and a template referencing it converge in
/// one `pic apply`. Before M6 the var (written earlier in the tx) was invisible
/// to expansion — the route would only pick it up on a *second* apply.
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
#[test]
fn route_template_var_resolves_in_same_apply() {
let Some(fx) = common::fixture_or_skip() else {
return;
};
let env = common::admin_env(fx);
let group = common::unique_slug("tplv-g");
let a = common::unique_slug("tplv-a");
let _g = GroupGuard::new(&env.url, &env.token, &group);
common::pic_as(&env)
.args(["groups", "create", &group])
.assert()
.success();
let _aa = AppGuard::new(&env.url, &env.token, &a);
common::pic_as(&env)
.args(["apps", "create", &a, "--group", &group])
.assert()
.success();
// The group sets `region` AND declares a template referencing it — one
// manifest, one apply.
let dir = TempDir::new().expect("tempdir");
fs::create_dir_all(dir.path().join("scripts")).unwrap();
fs::write(dir.path().join("scripts/shared.rhai"), r#""hi""#).unwrap();
fs::write(
dir.path().join("picloud.toml"),
format!(
"[group]\nslug = \"{group}\"\nname = \"G\"\n\n\
[vars]\nregion = \"eu\"\n\n\
[[scripts]]\nname = \"shared\"\nfile = \"scripts/shared.rhai\"\n\n\
[[route_templates]]\nname = \"geo\"\nscript = \"shared\"\n\
host_kind = \"any\"\npath_kind = \"exact\"\npath = \"/tv/{{app_slug}}/{{var:region}}\"\n"
),
)
.unwrap();
fs::create_dir_all(dir.path().join(&a)).unwrap();
fs::write(
dir.path().join(&a).join("picloud.toml"),
format!("[app]\nslug = \"{a}\"\nname = \"App\"\n"),
)
.unwrap();
common::pic_as(&env)
.args(["apply", "--dir"])
.arg(dir.path())
.assert()
.success();
let _gs = ScriptGuard::new(&env.url, &env.token, &group_script_id(&env, &group));
// The expanded route resolved `{var:region}` → `eu` in THIS apply.
let url = std::env::var("DATABASE_URL").expect("DATABASE_URL");
let mut pg = PgClient::connect(&url, NoTls).expect("pg connect");
let want = format!("/tv/{a}/eu");
let rows = pg
.query(
"SELECT path FROM routes WHERE from_template IS NOT NULL AND path = $1",
&[&want],
)
.expect("query");
assert_eq!(
rows.len(),
1,
"`{{var:region}}` must resolve to `eu` in the same apply (expected path {want})"
);
}
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
#[test]
fn expansion_colliding_with_hand_declared_route_is_rejected() {

5
docs/design/groups-and-project-tool.md
View File

@@ -404,8 +404,9 @@ Two distinct constraints:
> check (cron schedule, queue timeout, …) — so a `{var:…}`-injected reserved path, unclaimed strict
> host, or malformed schedule/timeout can't slip in. **Scope:** expansion targets app NODES present in the tree apply
> (the common `pic apply --dir` case), not yet every descendant in another repo — deleting a template
> leaves expansions in out-of-apply descendants until they're re-applied (the apply warns);
> `{var:NAME}` resolves against *committed* vars (a var set in the same apply lands next apply). A
> leaves expansions in out-of-apply descendants until they're re-applied (the apply warns).
> `{var:NAME}` resolves **in-transaction** (M6, 2026-06-28) — a var set in the *same* apply is visible to
> the template immediately, so var + template converge in one apply. A
> trigger template whose only change is a non-identity field (e.g. dispatch_mode) is a no-op on
> re-apply, mirroring the existing trigger-identity limitation (recreate to change those).