From 7ef32f951857c0c56276af12af825fe3f7fc2d55 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 19 Aug 2026 20:05:01 +0530 Subject: [PATCH 1/2] feat(hosting): add rollback, deployment history and domain status tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent could deploy a site and could not get back off a bad deploy. `hosting_launch_site` had no counterweight: nothing promoted an earlier deployment, and nothing even returned a deployment id to promote except the launch that created it. A deploy that broke a live site was a one-way door. Three tools, all of them thin over `tinyhosts` calls that already exist: - `hosting_rollback` — points production back at an earlier deployment, via `Host::promote`. There is no separate promote tool: the crate models promote and rollback as one operation deliberately, and this is named for the reason an agent reaches for it. Carries `external_effect`, so it parks for approval like every other outward effect — opt-in per call rather than a standing capability. - `hosting_list_deployments` — the history a rollback picks an id from, newest first with status and target. Read-only. - `hosting_domain_status` — whether a site's domains are verified and serving, the read half of `hosting_add_domain`. Read-only. `hosting_rollback` reads the deployment before promoting it and refuses one that did not finish building. `hosting_list_deployments` returns failed and still-building deployments too — that is the history an agent is reading — so the id it picks is not necessarily one that can serve traffic, and promoting a failed build would take the site down during an attempt to bring it back up. It deliberately does *not* check that the deployment belongs to the named site. `Host::deployment` looks a deployment up by id alone and falls back to an empty name when the provider's response omits one, so comparing that against the site argument would refuse legitimate rollbacks. The provider owns that check. Tested against a mock of the provider's REST API, through the real adapter — `connect_to` takes a base URL for exactly this. The load bearing assertion is `expect(0)` on the promote route in the refusal case: the guard has to stop the outward call, not report an error after production already moved. `hosting_deployment_logs`, the fourth gap, is not here: `Host` has no logs method at all, so it needs a trait method, a provider implementation and a mock in the tinyhosts crate before a tool can wrap it. The other three needed no crate change. Refs tinyhumansai/opencompany#913 --- src/openhuman/hosting/README.md | 22 ++- src/openhuman/hosting/mod.rs | 6 +- src/openhuman/hosting/test.rs | 239 +++++++++++++++++++++++++++- src/openhuman/hosting/tools.rs | 267 +++++++++++++++++++++++++++++++- 4 files changed, 524 insertions(+), 10 deletions(-) diff --git a/src/openhuman/hosting/README.md b/src/openhuman/hosting/README.md index b6cb16e810..27c5055195 100644 --- a/src/openhuman/hosting/README.md +++ b/src/openhuman/hosting/README.md @@ -9,7 +9,7 @@ everything about a provider, and this module owns everything about OpenHuman. - Resolve the configured hosting account (`Account::from_config`) from `[hosting]`, falling back to the provider's own environment variables. - Decide which directory an agent may deploy (`resolve_in_workspace`). -- Expose the six `hosting_*` agent tools and describe their results to a model. +- Expose the nine `hosting_*` agent tools and describe their results to a model. Everything else — Vercel's endpoints, the upload-then-build deployment protocol, how a marketplace database is provisioned and connected, the order a launch runs @@ -21,7 +21,7 @@ mock of the provider's REST API. Nothing here knows the word `readyState`. | File | Role | | --- | --- | | `mod.rs` | `Account` (credential resolution + the shared `dyn Host`) and `resolve_in_workspace`. | -| `tools.rs` | The six agent tools. | +| `tools.rs` | The nine agent tools. | | `test.rs` | Account resolution, workspace containment, and each tool's contract. | ## Agent tools @@ -30,11 +30,29 @@ mock of the provider's REST API. Nothing here knows the word `readyState`. | --- | --- | | `hosting_launch_site` | Deploys a workspace directory as a live site, optionally provisioning a database and wiring it in, setting environment variables, and attaching domains. External effect. | | `hosting_deployment_status` | Whether a build has finished. Read-only. | +| `hosting_list_deployments` | A site's recent deployments, newest first, with status and target. Read-only. | +| `hosting_rollback` | Points production back at an earlier deployment that already built. External effect. | | `hosting_list_sites` | The sites on the account. Read-only. | | `hosting_set_env` | Sets environment variables on an existing site. External effect. | | `hosting_add_domain` | Attaches a custom domain. External effect. | +| `hosting_domain_status` | Whether a site's domains are verified and serving. Read-only. | | `hosting_analytics` | Traffic over the last N days. Read-only. | +### Why there is a rollback and no separate promote + +`Host::promote` is both: a rollback *is* a promote of an older deployment, and +the crate models it once rather than twice. The tool is named for the reason an +agent reaches for it. + +`hosting_rollback` reads the deployment before promoting it and refuses one that +did not finish building. `hosting_list_deployments` returns failed and +still-building deployments too — they are the history an agent is reading — so +the id it picks is not necessarily one that can serve traffic, and promoting a +failed build would take the site down during an attempt to bring it back up. It +does **not** check that the deployment belongs to the named site: a deployment is +looked up by id alone and the provider's response may omit the project name, so +that check would refuse legitimate rollbacks. The provider owns it. + ## Gating Two gates, and both matter: diff --git a/src/openhuman/hosting/mod.rs b/src/openhuman/hosting/mod.rs index d51c8df2c6..6ca3d25803 100644 --- a/src/openhuman/hosting/mod.rs +++ b/src/openhuman/hosting/mod.rs @@ -12,9 +12,11 @@ //! //! # What the agent gets //! -//! Six tools, in [`tools`]. `hosting_launch_site` is the one that matters: it +//! Nine tools, in [`tools`]. `hosting_launch_site` is the one that matters: it //! turns a directory in the workspace into a live site with a database behind -//! it. The other five read or adjust what it produced. +//! it. `hosting_rollback` is its counterweight — it points production back at +//! an earlier deployment, so an agent that ships a broken site has a way back +//! rather than only a way forward. The rest read or adjust what they produced. //! //! # The credential //! diff --git a/src/openhuman/hosting/test.rs b/src/openhuman/hosting/test.rs index 3734c23242..37dee6a7a8 100644 --- a/src/openhuman/hosting/test.rs +++ b/src/openhuman/hosting/test.rs @@ -90,9 +90,12 @@ fn an_account_exposes_every_hosting_tool() { [ "hosting_launch_site", "hosting_deployment_status", + "hosting_list_deployments", + "hosting_rollback", "hosting_list_sites", "hosting_set_env", "hosting_add_domain", + "hosting_domain_status", "hosting_analytics", ] ); @@ -109,7 +112,7 @@ fn only_the_tools_that_change_the_world_carry_an_external_effect() { for tool in account.tools() { let expected = matches!( tool.name(), - "hosting_launch_site" | "hosting_set_env" | "hosting_add_domain" + "hosting_launch_site" | "hosting_set_env" | "hosting_add_domain" | "hosting_rollback" ); assert_eq!( tool.external_effect(), @@ -246,3 +249,237 @@ async fn a_read_tool_reports_a_missing_argument_rather_than_calling_out() { assert!(result.is_error); } + +// ── The deployment-history and rollback tools (issue opencompany#913) ──────── + +#[tokio::test] +async fn the_new_read_tools_report_a_missing_site_rather_than_calling_out() { + let workspace = tempfile::tempdir().expect("tempdir"); + let config = config_with(workspace.path(), true, "token"); + let account = Account::from_config(&config) + .expect("resolution does not fail") + .expect("an account"); + + let listed = tools::ListDeploymentsTool::new(account.host()) + .execute(json!({})) + .await + .expect("the tool reports rather than panics"); + assert!(listed.is_error); + + let domains = tools::DomainStatusTool::new(account.host()) + .execute(json!({})) + .await + .expect("the tool reports rather than panics"); + assert!(domains.is_error); +} + +#[tokio::test] +async fn a_rollback_missing_either_argument_is_refused_before_any_call() { + let workspace = tempfile::tempdir().expect("tempdir"); + let config = config_with(workspace.path(), true, "token"); + let account = Account::from_config(&config) + .expect("resolution does not fail") + .expect("an account"); + + // A rollback names two things and neither can be guessed: repointing + // production at an unnamed deployment is not a recoverable mistake. + for args in [ + json!({}), + json!({"site": "shop"}), + json!({"deployment_id": "d1"}), + ] { + let result = tools::RollbackTool::new(account.host()) + .execute(args.clone()) + .await + .expect("the tool reports rather than panics"); + assert!(result.is_error, "{args} should have been refused"); + } +} + +// ── The rollback guard, against a mock of the provider's API ───────────────── + +use std::sync::Arc as TestArc; + +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// A real provider client pointed at a local mock rather than at Vercel. +/// +/// `connect_to`'s own documentation names this case, and loopback `http://` is +/// accepted precisely so a test can carry a bearer token to a server that never +/// leaves the machine. +fn host_against(server: &MockServer) -> TestArc { + let base_url = server.uri(); + TestArc::from( + tinyhosts::connect_to( + tinyhosts::ProviderKind::Vercel, + tinyhosts::Credentials::new("token").expect("credentials"), + Some(base_url.as_str()), + ) + .expect("a client against the mock"), + ) +} + +/// **The guard that makes this tool a recovery path rather than a second way to +/// break the site.** +/// +/// `hosting_list_deployments` returns failed and still-building deployments too +/// — they are part of the history an agent reads — so the id it picks is not +/// necessarily one that can serve traffic. Promoting a failed build would take +/// the site down during an attempt to bring it back up. +/// +/// The assertion that matters is `expect(0)` on the promote route: the refusal +/// has to happen *before* the outward call, not be an error message reported +/// after production already moved. +#[tokio::test] +async fn rolling_back_to_a_deployment_that_never_built_does_not_touch_production() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v13/deployments/dpl_broken")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "dpl_broken", + "name": "shop", + "readyState": "ERROR", + "errorMessage": "build failed" + }))) + .mount(&server) + .await; + + // Vercel's promote is POST /v10/projects/{project}/promote/{deployment}, + // reached only after GET /v9/projects/{site} resolves the id. Neither may + // be called at all. + Mock::given(method("GET")) + .and(path("/v9/projects/shop")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "prj_1", + "name": "shop" + }))) + .expect(0) + .mount(&server) + .await; + + let result = tools::RollbackTool::new(host_against(&server)) + .execute(json!({"site": "shop", "deployment_id": "dpl_broken"})) + .await + .expect("the tool reports rather than panics"); + + assert!(result.is_error, "a failed deployment must not be promoted"); + // The status is named, so a model can pick a different deployment rather + // than retry the same one. + let rendered = result.text(); + assert!( + rendered.contains("Failed"), + "the refusal should name the state it refused: {rendered}" + ); +} + +/// The ordinary path: a deployment that built is promoted, and the site's URL +/// comes back so the agent can say where production now points. +#[tokio::test] +async fn rolling_back_to_a_ready_deployment_promotes_it() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v13/deployments/dpl_good")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "dpl_good", + "name": "shop", + "url": "shop-abc.vercel.app", + "readyState": "READY", + "target": "production" + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/v9/projects/shop")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": "prj_1", + "name": "shop" + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/v10/projects/prj_1/promote/dpl_good")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) + .expect(1) + .mount(&server) + .await; + + let result = tools::RollbackTool::new(host_against(&server)) + .execute(json!({"site": "shop", "deployment_id": "dpl_good"})) + .await + .expect("the tool reports rather than panics"); + + assert!(!result.is_error, "{result:?}"); + let rendered = result.text(); + assert!( + rendered.contains("dpl_good") && rendered.contains("shop-abc.vercel.app"), + "the result should say what is serving and where: {rendered}" + ); +} + +/// The history read an agent uses to *find* the id above. +#[tokio::test] +async fn listing_deployments_reports_the_history_a_rollback_picks_from() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v7/deployments")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "deployments": [ + {"id": "dpl_new", "name": "shop", "readyState": "ERROR"}, + {"id": "dpl_old", "name": "shop", "readyState": "READY"} + ] + }))) + .mount(&server) + .await; + + let result = tools::ListDeploymentsTool::new(host_against(&server)) + .execute(json!({"site": "shop"})) + .await + .expect("the tool reports rather than panics"); + + assert!(!result.is_error, "{result:?}"); + let rendered = result.text(); + // Both are reported: the failed one is why the agent is here, and the ready + // one is where it is going. + assert!( + rendered.contains("dpl_new") && rendered.contains("dpl_old"), + "{rendered}" + ); +} + +/// A domain that is attached but unverified is not serving, and the tool has to +/// say so — that difference is the entire reason to read domains. +#[tokio::test] +async fn domain_status_distinguishes_a_verified_domain_from_a_pending_one() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v9/projects/shop/domains")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "domains": [ + {"name": "shop.example.com", "verified": true}, + {"name": "www.example.com", "verified": false} + ] + }))) + .mount(&server) + .await; + + let result = tools::DomainStatusTool::new(host_against(&server)) + .execute(json!({"site": "shop"})) + .await + .expect("the tool reports rather than panics"); + + assert!(!result.is_error, "{result:?}"); + let rendered = result.text(); + assert!( + rendered.contains("shop.example.com") && rendered.contains("www.example.com"), + "{rendered}" + ); + assert!( + rendered.contains("true") && rendered.contains("false"), + "{rendered}" + ); +} diff --git a/src/openhuman/hosting/tools.rs b/src/openhuman/hosting/tools.rs index 2d60d47983..12205699d9 100644 --- a/src/openhuman/hosting/tools.rs +++ b/src/openhuman/hosting/tools.rs @@ -1,14 +1,21 @@ //! The `hosting_*` agent tools. //! -//! Six tools over one hosting account. They are thin on purpose: argument +//! Nine tools over one hosting account. They are thin on purpose: argument //! parsing, one call into [`tinyhosts`], and a result described for a model. //! Anything that looks like hosting logic belongs in the crate, where it is //! provider-independent and tested against a mock of the provider's API. //! -//! `hosting_launch_site` is the only one with `external_effect`: it uploads a -//! directory to a third party and can spend money on a database, so it routes -//! through the approval gate. The rest read, or adjust something that already -//! exists. +//! `hosting_launch_site` uploads a directory to a third party and can spend +//! money on a database, and `hosting_rollback` repoints a live site's +//! production traffic; both route through the approval gate, as do +//! `hosting_set_env` and `hosting_add_domain`. The rest read. +//! +//! # Why there is a rollback but no separate "promote" +//! +//! [`Host::promote`] is both: a rollback *is* a promote of an older deployment, +//! and the crate models it once deliberately. The tool is named for the reason +//! an agent reaches for it. Without it an agent can deploy a broken site and +//! have no way back, which is the whole argument for the tool existing. use std::sync::Arc; @@ -27,9 +34,12 @@ pub fn hosting_tools(account: &Account) -> Vec> { vec![ Box::new(LaunchSiteTool::new(account.clone())), Box::new(DeploymentStatusTool::new(account.host())), + Box::new(ListDeploymentsTool::new(account.host())), + Box::new(RollbackTool::new(account.host())), Box::new(ListSitesTool::new(account.host())), Box::new(SetEnvTool::new(account.host())), Box::new(AddDomainTool::new(account.host())), + Box::new(DomainStatusTool::new(account.host())), Box::new(AnalyticsTool::new(account.host())), ] } @@ -361,6 +371,194 @@ impl Tool for DeploymentStatusTool { } } +// ── hosting_list_deployments ──────────────────────────────────────────────── + +/// Lists a site's recent deployments, newest first. +/// +/// Mostly the other half of [`RollbackTool`]: a rollback needs a deployment id +/// to promote, and before this tool nothing returned one except the launch that +/// created it. An agent that wanted to go back to the deployment *before* the +/// bad one had no way to name it. +pub struct ListDeploymentsTool { + host: Arc, +} + +impl ListDeploymentsTool { + pub fn new(host: Arc) -> Self { + Self { host } + } +} + +#[async_trait] +impl Tool for ListDeploymentsTool { + fn name(&self) -> &str { + "hosting_list_deployments" + } + + fn description(&self) -> &str { + "List a site's recent deployments, newest first, with their status, \ + target and creation time. Use it to find the deployment id of a known \ + good version before rolling back to it, or to see the history of what \ + has been shipped. Each entry's id is what hosting_rollback and \ + hosting_deployment_status take." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["site"], + "properties": { + "site": { "type": "string", "description": "The site's name." }, + "limit": { + "type": "integer", + "description": "How many to return. Defaults to 20." + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let site = match required_str(&args, "site") { + Ok(site) => site, + Err(error) => return Ok(ToolResult::error(error.to_string())), + }; + // Clamped to the same window as `hosting_list_sites`, for the same + // reason: a model that asks for everything gets a page, not a bill. + let limit = args + .get("limit") + .and_then(Value::as_u64) + .unwrap_or(20) + .clamp(1, 100) as u32; + + match self.host.list_deployments(&site, limit).await { + Ok(deployments) => Ok(ToolResult::success(serde_json::to_string_pretty( + &deployments, + )?)), + Err(error) => Ok(ToolResult::error(error.to_string())), + } + } +} + +// ── hosting_rollback ──────────────────────────────────────────────────────── + +/// Points a site's production traffic back at an earlier deployment. +pub struct RollbackTool { + host: Arc, +} + +impl RollbackTool { + pub fn new(host: Arc) -> Self { + Self { host } + } +} + +#[async_trait] +impl Tool for RollbackTool { + fn name(&self) -> &str { + "hosting_rollback" + } + + fn description(&self) -> &str { + "Roll a site back by pointing its production traffic at an earlier \ + deployment that already built successfully. Use it when a deploy broke \ + a live site: this is the recovery path. It does not rebuild anything — \ + the deployment being promoted was built when it was first deployed, \ + which is why it is the fast way back. Get the id from \ + hosting_list_deployments; only a deployment that finished building can \ + be promoted." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["site", "deployment_id"], + "properties": { + "site": { + "type": "string", + "description": "The site whose production traffic moves." + }, + "deployment_id": { + "type": "string", + "description": "The deployment to serve, from hosting_list_deployments. \ + It must have finished building." + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + /// Changes what the public sees on a live site, so it gates. + fn external_effect(&self) -> bool { + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let site = match required_str(&args, "site") { + Ok(site) => site, + Err(error) => return Ok(ToolResult::error(error.to_string())), + }; + let deployment_id = match required_str(&args, "deployment_id") { + Ok(id) => id, + Err(error) => return Ok(ToolResult::error(error.to_string())), + }; + + // Read the deployment before promoting it. `hosting_list_deployments` + // returns failed and still-building deployments too — they are part of + // the history an agent is reading — so the id it picks is not + // necessarily one that can serve traffic. Promoting a failed build + // would take the site down in the middle of an attempt to bring it + // back up, which is the one outcome this tool exists to prevent. + // + // Only the status is checked, deliberately. The site a deployment + // belongs to is *not* reliable here: `Host::deployment` looks a + // deployment up by id alone and falls back to an empty name when the + // provider's response omits one, so comparing it against `site` would + // refuse legitimate rollbacks. The provider owns that check. + let deployment = match self.host.deployment(&deployment_id).await { + Ok(deployment) => deployment, + Err(error) => return Ok(ToolResult::error(error.to_string())), + }; + + if !deployment.status.is_ready() { + return Ok(ToolResult::error(format!( + "Deployment `{deployment_id}` is {:?}, so it cannot be promoted \ + — only a deployment that finished building can serve traffic. \ + Use hosting_list_deployments to find one that is ready.", + deployment.status, + ))); + } + + tracing::info!( + site = %site, + deployment = %deployment_id, + "[hosting] rolling back" + ); + + match self.host.promote(&site, &deployment_id).await { + Ok(()) => Ok(ToolResult::success(match &deployment.url { + Some(url) => format!( + "{site} is now serving deployment `{deployment_id}` in \ + production ({url}). The change is at the provider's edge; \ + nothing was rebuilt." + ), + None => format!( + "{site} is now serving deployment `{deployment_id}` in \ + production. The change is at the provider's edge; nothing \ + was rebuilt." + ), + })), + Err(error) => Ok(ToolResult::error(error.to_string())), + } + } +} + // ── hosting_list_sites ────────────────────────────────────────────────────── /// Lists the sites on the account. @@ -587,6 +785,65 @@ impl Tool for AddDomainTool { } } +// ── hosting_domain_status ─────────────────────────────────────────────────── + +/// Reports whether a site's domains are verified and serving. +/// +/// The read half of [`AddDomainTool`]. Attaching a domain is not the end of the +/// job: it does not serve traffic until its DNS records point at the provider, +/// which the user has to do at their registrar, and until now nothing could +/// answer "did that work?" without attaching it again. +pub struct DomainStatusTool { + host: Arc, +} + +impl DomainStatusTool { + pub fn new(host: Arc) -> Self { + Self { host } + } +} + +#[async_trait] +impl Tool for DomainStatusTool { + fn name(&self) -> &str { + "hosting_domain_status" + } + + fn description(&self) -> &str { + "List the custom domains attached to a site and whether the provider \ + has verified each one. A domain that is attached but unverified is not \ + serving traffic yet — its DNS records still have to point at the \ + provider, which the user does at their registrar. Use it to check \ + whether a domain added earlier has come up." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["site"], + "properties": { + "site": { "type": "string", "description": "The site's name." } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let site = match required_str(&args, "site") { + Ok(site) => site, + Err(error) => return Ok(ToolResult::error(error.to_string())), + }; + + match self.host.list_domains(&site).await { + Ok(domains) => Ok(ToolResult::success(serde_json::to_string_pretty(&domains)?)), + Err(error) => Ok(ToolResult::error(error.to_string())), + } + } +} + // ── hosting_analytics ─────────────────────────────────────────────────────── /// Reports the traffic a site served. From 831f878e5d8c2bc0794584efa4b2475374ea026b Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 19 Aug 2026 20:40:00 +0530 Subject: [PATCH 2/2] test(hosting): bind each asserted value to the key it belongs to Both list assertions checked for presence rather than for pairing, so each passed just as happily with the values swapped: - The domain test asserted that a `true` and a `false` both appeared somewhere in the response. Verified and pending are exactly what that tool exists to tell apart, so the one arrangement it must not report was the one the test could not see. - The deployment test asserted that both ids appeared. A list returning the right ids against the wrong statuses would point a rollback at the deployment that just broke the site. Both now parse the JSON and bind the value to its key: the domain by name, the deployment status by id. The deployment test also pins the order the crate documents, newest first. Raised by CodeRabbit on the domain assertion; the same weakness in the deployment assertion is fixed with it rather than left for a second pass. --- src/openhuman/hosting/test.rs | 50 ++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/openhuman/hosting/test.rs b/src/openhuman/hosting/test.rs index 37dee6a7a8..9416655546 100644 --- a/src/openhuman/hosting/test.rs +++ b/src/openhuman/hosting/test.rs @@ -441,13 +441,21 @@ async fn listing_deployments_reports_the_history_a_rollback_picks_from() { .expect("the tool reports rather than panics"); assert!(!result.is_error, "{result:?}"); - let rendered = result.text(); - // Both are reported: the failed one is why the agent is here, and the ready - // one is where it is going. - assert!( - rendered.contains("dpl_new") && rendered.contains("dpl_old"), - "{rendered}" - ); + + // Both are reported, and each status is bound to its id. The failed one is + // why the agent is here and the ready one is where it is going, so a list + // that returned the right ids against the wrong statuses would send the + // rollback at the deployment that just broke the site. + let deployments: serde_json::Value = + serde_json::from_str(&result.text()).expect("the tool answers with JSON"); + let rows = deployments.as_array().expect("an array of deployments"); + + assert_eq!(rows.len(), 2, "{deployments}"); + // Newest first, as the crate documents and as the provider returned them. + assert_eq!(rows[0]["id"], "dpl_new", "{deployments}"); + assert_eq!(rows[0]["status"], "failed", "{deployments}"); + assert_eq!(rows[1]["id"], "dpl_old", "{deployments}"); + assert_eq!(rows[1]["status"], "ready", "{deployments}"); } /// A domain that is attached but unverified is not serving, and the tool has to @@ -473,13 +481,31 @@ async fn domain_status_distinguishes_a_verified_domain_from_a_pending_one() { .expect("the tool reports rather than panics"); assert!(!result.is_error, "{result:?}"); - let rendered = result.text(); + + // Bound to the name rather than checked for presence: asserting only that a + // `true` and a `false` both appear somewhere passes just as happily if the + // two are swapped, which is the one thing this tool must not get wrong. + let domains: serde_json::Value = + serde_json::from_str(&result.text()).expect("the tool answers with JSON"); + let verified_for = |name: &str| -> bool { + let entry = domains + .as_array() + .expect("an array of domains") + .iter() + .find(|domain| domain["name"] == name) + .unwrap_or_else(|| panic!("{name} is missing from {domains}")); + entry["verified"] + .as_bool() + .expect("`verified` is a boolean") + }; + assert!( - rendered.contains("shop.example.com") && rendered.contains("www.example.com"), - "{rendered}" + verified_for("shop.example.com"), + "the verified domain must report verified: {domains}" ); assert!( - rendered.contains("true") && rendered.contains("false"), - "{rendered}" + !verified_for("www.example.com"), + "and the pending one must not — that difference is the entire reason to \ + read domains: {domains}" ); }