feat(cli): plan warnings, apply confirm + blast-radius gate, files --group, unified owner-XOR
Remediate the CLI/DX findings from the 2026-07-11 audit. B4 — `pic plan` now previews the apply-time desired-state warnings (disabled binding, unreachable endpoint, dangling `[suppress]`), so plan and apply agree for CI review. Wire fields are `#[serde(default)]` (older-server tolerant). B5 — `pic apply` no longer mutates on a first run (no recorded plan) without a preview + confirm, and a large blast radius now triggers an extra confirmation. Both gates check `is_terminal()` before any read — a non-TTY never hangs on stdin and fails closed without `--yes`; `--yes`/`--force` bypass headlessly. `--force` help now notes it also skips these prompts. C3 — `pic files ls`/`get` gain `--group`, mirroring `pic kv --group`, so the shipped group shared-files admin surface is reachable from the CLI. C4 — a shared `require_one_owner(app, group)` helper (cmds/mod.rs) gives one canonical message for the `--app`/`--group` XOR, wired into every such site (kv, files, vars, secrets, suppress, extension-points, triggers ls, scripts deploy). routes ls (positional script_id) and scripts ls (lists all) keep their own shapes deliberately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,7 @@ use std::path::Path;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
|
||||
use crate::client::{Client, NodeKind};
|
||||
use crate::client::{Client, NodeKind, NodePlanDto, PlanDto};
|
||||
use crate::cmds::plan::build_bundle;
|
||||
use crate::config;
|
||||
use crate::manifest::{Manifest, ManifestProject};
|
||||
@@ -62,6 +62,24 @@ pub async fn run(
|
||||
.map(|l| l.state_token)
|
||||
};
|
||||
|
||||
// First-run guard: with no reviewed `.picloud/` plan, `apply` would mutate
|
||||
// with zero preview. Fetch a plan, show the change + blast-radius summary,
|
||||
// and confirm (interactive) or refuse a large cross-repo blast radius in CI.
|
||||
// `--yes`/`--force` opt out.
|
||||
if expected_token.is_none() && !yes && !force {
|
||||
let plan = client
|
||||
.plan_node(kind, &slug, &bundle, governing.as_ref())
|
||||
.await?;
|
||||
let changes = plan_change_count(&plan);
|
||||
let blast = plan
|
||||
.ownership
|
||||
.as_ref()
|
||||
.map_or(0, |o| o.blast_radius.iter().map(|b| b.apps).sum());
|
||||
if changes > 0 || blast > 0 {
|
||||
confirm_first_apply(&slug, changes, blast, &plan.warnings)?;
|
||||
}
|
||||
}
|
||||
|
||||
let report = client
|
||||
.apply_node(
|
||||
kind,
|
||||
@@ -170,6 +188,25 @@ pub async fn run_tree(
|
||||
.map(|l| l.state_token)
|
||||
};
|
||||
|
||||
// First-run guard (see `run`): no reviewed plan → preview + confirm the whole
|
||||
// tree's change count and cross-repo blast radius before mutating.
|
||||
if expected_token.is_none() && !yes && !force {
|
||||
let plan = client.plan_tree(&bundle, project.as_ref()).await?;
|
||||
let changes: usize = plan.nodes.iter().map(node_change_count).sum();
|
||||
let blast: u32 = plan
|
||||
.nodes
|
||||
.iter()
|
||||
.filter_map(|n| n.ownership.as_ref())
|
||||
.flat_map(|o| o.blast_radius.iter())
|
||||
.map(|b| b.apps)
|
||||
.sum();
|
||||
let warnings: Vec<String> = plan.nodes.iter().flat_map(|n| n.warnings.clone()).collect();
|
||||
if changes > 0 || blast > 0 {
|
||||
let target = format!("{node_count} node(s) under {}", dir.display());
|
||||
confirm_first_apply(&target, changes, blast, &warnings)?;
|
||||
}
|
||||
}
|
||||
|
||||
let report = client
|
||||
.apply_tree(
|
||||
&bundle,
|
||||
@@ -284,6 +321,73 @@ fn require_env_approval(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A cross-repo blast radius at or above this many other-project apps is refused
|
||||
/// non-interactively without `--yes` (§4.2 "this changes N apps — confirm").
|
||||
const BLAST_RADIUS_CONFIRM_THRESHOLD: u32 = 10;
|
||||
|
||||
fn plan_change_count(plan: &PlanDto) -> usize {
|
||||
plan.scripts.len()
|
||||
+ plan.routes.len()
|
||||
+ plan.triggers.len()
|
||||
+ plan.secrets.len()
|
||||
+ plan.vars.len()
|
||||
+ plan.extension_points.len()
|
||||
+ plan.collections.len()
|
||||
+ plan.suppressions.len()
|
||||
}
|
||||
|
||||
fn node_change_count(n: &NodePlanDto) -> usize {
|
||||
n.scripts.len()
|
||||
+ n.routes.len()
|
||||
+ n.triggers.len()
|
||||
+ n.secrets.len()
|
||||
+ n.vars.len()
|
||||
+ n.extension_points.len()
|
||||
+ n.collections.len()
|
||||
+ n.suppressions.len()
|
||||
}
|
||||
|
||||
/// First-run guard: `pic apply` with no reviewed `.picloud/` plan mutates with
|
||||
/// zero preview. When none was recorded (and not `--yes`/`--force`), the caller
|
||||
/// fetches a plan and passes its change count + cross-repo blast radius here. On
|
||||
/// a TTY we show the summary and prompt; non-interactively we allow a small
|
||||
/// change but REFUSE a large blast radius without an explicit `--yes` (so a
|
||||
/// wide-reaching change can't slip through CI unreviewed).
|
||||
fn confirm_first_apply(
|
||||
target: &str,
|
||||
changes: usize,
|
||||
blast_apps: u32,
|
||||
warnings: &[String],
|
||||
) -> Result<()> {
|
||||
if !std::io::stdin().is_terminal() {
|
||||
if blast_apps >= BLAST_RADIUS_CONFIRM_THRESHOLD {
|
||||
anyhow::bail!(
|
||||
"refusing to apply `{target}` non-interactively: no reviewed plan and this \
|
||||
change fans out to {blast_apps} app(s) across other projects. Review with \
|
||||
`pic plan`, then re-run with `--yes`."
|
||||
);
|
||||
}
|
||||
return Ok(());
|
||||
}
|
||||
eprintln!("no reviewed plan for `{target}`; this apply will make {changes} change(s).");
|
||||
if blast_apps > 0 {
|
||||
eprintln!(" cross-repo blast radius: {blast_apps} app(s) in other project(s).");
|
||||
}
|
||||
for w in warnings {
|
||||
eprintln!(" warning: {w}");
|
||||
}
|
||||
eprint!("Continue? [y/N] ");
|
||||
std::io::stderr().flush().ok();
|
||||
let mut answer = String::new();
|
||||
std::io::stdin()
|
||||
.read_line(&mut answer)
|
||||
.context("read confirmation")?;
|
||||
if !matches!(answer.trim(), "y" | "Y" | "yes" | "Yes") {
|
||||
anyhow::bail!("aborted");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn confirm_prune(slug: &str) -> Result<()> {
|
||||
if !std::io::stdin().is_terminal() {
|
||||
anyhow::bail!(
|
||||
|
||||
Reference in New Issue
Block a user