fix(apply): restore bare-bundle wire compat on the plan endpoints
§7 M3 wrapped the three plan request bodies in a `{bundle, project}` struct,
silently breaking version-skew compatibility: a pre-§7 `pic plan` POSTs a BARE
bundle at the JSON top level and now fails deserialization (422 "missing field
bundle"), even though the sibling apply request deliberately kept its added
fields additive (`#[serde(default)]`) so an older CLI still works.
`#[serde(flatten)]` the bundle in `PlanRequest`/`TreePlanRequest` so the bundle
fields sit at the top level again: a bare bundle deserializes (`project`
defaults to `None`) and a newer client sends the same fields plus a top-level
`project` key. The CLI now builds that shape via a `plan_body` helper (bundle
object + optional `project`), which an older server also accepts (Bundle has no
`deny_unknown_fields`, so the extra key is ignored) — compatible in both skew
directions. Apply is left unwrapped (it was already `{bundle, ...}` pre-§7).
Pinned by `plan_request_accepts_a_bare_bundle` / `_a_flattened_project` /
`tree_plan_request_accepts_a_bare_bundle` unit tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -183,6 +183,12 @@ async fn group_extension_points_handler(
|
|||||||
/// `[project]`, so the plan can preview the ownership outcome + attach ceiling.
|
/// `[project]`, so the plan can preview the ownership outcome + attach ceiling.
|
||||||
#[derive(Deserialize)]
|
#[derive(Deserialize)]
|
||||||
pub struct PlanRequest {
|
pub struct PlanRequest {
|
||||||
|
// Flattened so the bundle fields sit at the JSON top level: an older `pic`
|
||||||
|
// (pre-§7) that POSTs a bare bundle still deserializes (`project` defaults
|
||||||
|
// to `None`), and a newer client sends the same bundle fields plus a
|
||||||
|
// top-level `project` key. Keeps the plan wire compatible in both skew
|
||||||
|
// directions, matching the additive compatibility of the apply request.
|
||||||
|
#[serde(flatten)]
|
||||||
pub bundle: Bundle,
|
pub bundle: Bundle,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub project: Option<ProjectDecl>,
|
pub project: Option<ProjectDecl>,
|
||||||
@@ -327,6 +333,8 @@ struct TreeApplyRequest {
|
|||||||
|
|
||||||
#[derive(Deserialize)]
|
#[derive(Deserialize)]
|
||||||
struct TreePlanRequest {
|
struct TreePlanRequest {
|
||||||
|
// Flattened for the same bare-bundle wire compatibility as `PlanRequest`.
|
||||||
|
#[serde(flatten)]
|
||||||
bundle: TreeBundle,
|
bundle: TreeBundle,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
project: Option<ProjectDecl>,
|
project: Option<ProjectDecl>,
|
||||||
@@ -577,3 +585,41 @@ impl IntoResponse for ApplyError {
|
|||||||
(status, Json(body)).into_response()
|
(status, Json(body)).into_response()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
// §7 wire compat: the plan endpoints `#[serde(flatten)]` the bundle, so a
|
||||||
|
// pre-§7 client that POSTs a BARE bundle (no `{bundle: ...}` wrapper, no
|
||||||
|
// `project`) must still deserialize — `project` defaults to `None`.
|
||||||
|
#[test]
|
||||||
|
fn plan_request_accepts_a_bare_bundle() {
|
||||||
|
let bare = serde_json::json!({
|
||||||
|
"scripts": [],
|
||||||
|
"secrets": ["TOKEN"],
|
||||||
|
});
|
||||||
|
let req: PlanRequest = serde_json::from_value(bare).expect("bare bundle deserializes");
|
||||||
|
assert!(req.project.is_none());
|
||||||
|
assert_eq!(req.bundle.secrets, vec!["TOKEN".to_string()]);
|
||||||
|
}
|
||||||
|
|
||||||
|
// The newer wire shape — the same bundle fields at the top level plus a
|
||||||
|
// `project` key (what the current CLI sends) — also deserializes.
|
||||||
|
#[test]
|
||||||
|
fn plan_request_accepts_a_flattened_project() {
|
||||||
|
let with_proj = serde_json::json!({
|
||||||
|
"scripts": [],
|
||||||
|
"project": { "slug": "acme" },
|
||||||
|
});
|
||||||
|
let req: PlanRequest = serde_json::from_value(with_proj).expect("flattened project");
|
||||||
|
assert_eq!(req.project.expect("project").slug, "acme");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn tree_plan_request_accepts_a_bare_bundle() {
|
||||||
|
let bare = serde_json::json!({ "nodes": [] });
|
||||||
|
let req: TreePlanRequest = serde_json::from_value(bare).expect("bare tree bundle");
|
||||||
|
assert!(req.project.is_none());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -39,6 +39,25 @@ fn seg(s: &str) -> String {
|
|||||||
utf8_percent_encode(s, PATH_SEGMENT).to_string()
|
utf8_percent_encode(s, PATH_SEGMENT).to_string()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build a `plan` request body: the bundle fields at the JSON top level plus an
|
||||||
|
/// optional `project` key (§7). The plan endpoints `#[serde(flatten)]` the
|
||||||
|
/// bundle, so a bare bundle (no `[project]`) is the pre-§7 wire shape — this
|
||||||
|
/// keeps `pic plan` compatible across a CLI/server version skew in either
|
||||||
|
/// direction (an old server ignores the extra `project`; a new server accepts
|
||||||
|
/// the bare bundle).
|
||||||
|
fn plan_body(
|
||||||
|
bundle: &serde_json::Value,
|
||||||
|
project: Option<&crate::manifest::ManifestProject>,
|
||||||
|
) -> Result<serde_json::Value> {
|
||||||
|
let mut body = bundle.clone();
|
||||||
|
if let Some(p) = project {
|
||||||
|
if let Some(obj) = body.as_object_mut() {
|
||||||
|
obj.insert("project".to_string(), serde_json::to_value(p)?);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(body)
|
||||||
|
}
|
||||||
|
|
||||||
pub struct Client {
|
pub struct Client {
|
||||||
http: reqwest::Client,
|
http: reqwest::Client,
|
||||||
url: String,
|
url: String,
|
||||||
@@ -1233,7 +1252,7 @@ impl Client {
|
|||||||
project: Option<&crate::manifest::ManifestProject>,
|
project: Option<&crate::manifest::ManifestProject>,
|
||||||
) -> Result<PlanDto> {
|
) -> Result<PlanDto> {
|
||||||
let slug = seg(slug);
|
let slug = seg(slug);
|
||||||
let body = serde_json::json!({ "bundle": bundle, "project": project });
|
let body = plan_body(bundle, project)?;
|
||||||
let resp = self
|
let resp = self
|
||||||
.request(
|
.request(
|
||||||
Method::POST,
|
Method::POST,
|
||||||
@@ -1291,7 +1310,7 @@ impl Client {
|
|||||||
bundle: &serde_json::Value,
|
bundle: &serde_json::Value,
|
||||||
project: Option<&crate::manifest::ManifestProject>,
|
project: Option<&crate::manifest::ManifestProject>,
|
||||||
) -> Result<TreePlanDto> {
|
) -> Result<TreePlanDto> {
|
||||||
let body = serde_json::json!({ "bundle": bundle, "project": project });
|
let body = plan_body(bundle, project)?;
|
||||||
let resp = self
|
let resp = self
|
||||||
.request(Method::POST, "/api/v1/admin/tree/plan")
|
.request(Method::POST, "/api/v1/admin/tree/plan")
|
||||||
.json(&body)
|
.json(&body)
|
||||||
|
|||||||
Reference in New Issue
Block a user