From 3e97cae0aee2ec332a44518f944ea5c0aa1fa1b4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 29 Apr 2026 20:26:13 -0400 Subject: [PATCH] refactor(api): unify secret metadata types --- Cargo.lock | 1 + lib/crates/fabro-api/build.rs | 2 + lib/crates/fabro-api/src/lib.rs | 6 +-- .../fabro-api/tests/node_state_round_trip.rs | 44 ++++++++++++++++ .../tests/secret_metadata_round_trip.rs | 50 +++++++++++++++++++ lib/crates/fabro-auth/Cargo.toml | 1 + lib/crates/fabro-auth/src/vault_ext.rs | 3 +- .../fabro-cli/src/commands/secret/list.rs | 2 +- lib/crates/fabro-types/src/lib.rs | 2 +- lib/crates/fabro-types/src/secret.rs | 12 +++++ lib/crates/fabro-vault/src/lib.rs | 31 ++++-------- 11 files changed, 128 insertions(+), 26 deletions(-) create mode 100644 lib/crates/fabro-api/tests/node_state_round_trip.rs create mode 100644 lib/crates/fabro-api/tests/secret_metadata_round_trip.rs diff --git a/Cargo.lock b/Cargo.lock index 160852fb2..e79a6e281 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1605,6 +1605,7 @@ dependencies = [ "fabro-oauth", "fabro-redact", "fabro-static", + "fabro-types", "fabro-vault", "httpmock", "serde", diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index be897f31a..005afdc40 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -314,6 +314,8 @@ fn main() { ("QuestionType", "fabro_types::QuestionType", &[]), ("NodeStatusRecord", "fabro_types::NodeStatusRecord", &[]), ("InternalStageStatus", "fabro_types::StageStatus", &[]), + ("NodeState", "fabro_types::NodeState", &[]), + ("SecretMetadata", "fabro_types::SecretMetadata", &[]), ("PullRequestRecord", "fabro_types::PullRequestRecord", &[]), ("PullRequestDetail", "fabro_types::PullRequestDetail", &[]), ("PullRequestUser", "fabro_types::PullRequestUser", &[]), diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index 7218556b3..f535ed56c 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -28,9 +28,9 @@ pub mod types { BlockedReason, FailureReason, RunControlAction, RunStatus, SuccessReason, TerminalStatus, }; pub use fabro_types::{ - ActorKind, ActorRef, DiffStats, DirtyStatus, GitContext, NodeStatusRecord, - PreRunPushOutcome, QuestionType, RepositoryReference, RunSummary, SecretType, - ServerSettings, StageStatus as InternalStageStatus, WorkflowSettings, + ActorKind, ActorRef, DiffStats, DirtyStatus, GitContext, NodeState, NodeStatusRecord, + PreRunPushOutcome, QuestionType, RepositoryReference, RunSummary, SecretMetadata, + SecretType, ServerSettings, StageStatus as InternalStageStatus, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/node_state_round_trip.rs b/lib/crates/fabro-api/tests/node_state_round_trip.rs new file mode 100644 index 000000000..03044f20a --- /dev/null +++ b/lib/crates/fabro-api/tests/node_state_round_trip.rs @@ -0,0 +1,44 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::NodeState as ApiNodeState; +use fabro_types::NodeState; +use serde_json::json; + +#[test] +fn node_state_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn node_state_round_trips_representative_json() { + let value = json!({ + "prompt": "build it", + "response": "done", + "status": { + "status": "success", + "notes": null, + "failure_reason": null, + "timestamp": "2026-04-29T12:34:56Z" + }, + "provider_used": { "provider": "openai", "model": "gpt-5.2" }, + "diff": "diff --git a/file b/file", + "script_invocation": { "command": "cargo test" }, + "script_timing": { "duration_ms": 42 }, + "parallel_results": [{ "branch": 0, "status": "success" }], + "stdout": "ok", + "stderr": "" + }); + + let state: NodeState = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(serde_json::to_value(state).unwrap(), value); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs b/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs new file mode 100644 index 000000000..d2cfe597a --- /dev/null +++ b/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs @@ -0,0 +1,50 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::SecretMetadata as ApiSecretMetadata; +use fabro_types::{SecretMetadata, SecretType}; +use serde_json::json; + +#[test] +fn secret_metadata_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn secret_metadata_round_trips_representative_json() { + let value = json!({ + "name": "ANTHROPIC_API_KEY", + "type": "environment", + "description": "Anthropic API key", + "created_at": "2026-04-29T12:34:56Z", + "updated_at": "2026-04-29T12:40:00Z" + }); + + let metadata: SecretMetadata = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(metadata.name, "ANTHROPIC_API_KEY"); + assert_eq!(metadata.secret_type, SecretType::Environment); + assert_eq!(metadata.description, Some("Anthropic API key".to_string())); + assert_eq!(serde_json::to_value(metadata).unwrap(), value); +} + +#[test] +fn secret_metadata_omits_absent_description() { + let value = json!({ + "name": "/run/secrets/key.pem", + "type": "file", + "created_at": "2026-04-29T12:34:56Z", + "updated_at": "2026-04-29T12:40:00Z" + }); + + let metadata: SecretMetadata = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(serde_json::to_value(metadata).unwrap(), value); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-auth/Cargo.toml b/lib/crates/fabro-auth/Cargo.toml index 4952bff6f..bc501eec9 100644 --- a/lib/crates/fabro-auth/Cargo.toml +++ b/lib/crates/fabro-auth/Cargo.toml @@ -19,6 +19,7 @@ fabro-model = { path = "../fabro-model" } fabro-oauth = { path = "../fabro-oauth" } fabro-redact.workspace = true fabro-static.workspace = true +fabro-types = { path = "../fabro-types" } fabro-vault = { path = "../fabro-vault" } serde.workspace = true serde_json.workspace = true diff --git a/lib/crates/fabro-auth/src/vault_ext.rs b/lib/crates/fabro-auth/src/vault_ext.rs index 99d1c5306..24949cc7d 100644 --- a/lib/crates/fabro-auth/src/vault_ext.rs +++ b/lib/crates/fabro-auth/src/vault_ext.rs @@ -1,5 +1,6 @@ use fabro_model::Provider; -use fabro_vault::{SecretMetadata, SecretType, Vault}; +use fabro_types::SecretMetadata; +use fabro_vault::{SecretType, Vault}; use crate::credential::AuthCredential; diff --git a/lib/crates/fabro-cli/src/commands/secret/list.rs b/lib/crates/fabro-cli/src/commands/secret/list.rs index 0ceb23871..752109ee9 100644 --- a/lib/crates/fabro-cli/src/commands/secret/list.rs +++ b/lib/crates/fabro-cli/src/commands/secret/list.rs @@ -48,7 +48,7 @@ pub(super) async fn list_command(_args: &SecretListArgs, ctx: &CommandContext) - .map(|secret| { vec![ secret.name.clone().cell().bold(use_color), - secret.type_.to_string().cell(), + secret.secret_type.to_string().cell(), format_age(secret.updated_at, now).cell(), ] }) diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 2010a287f..d44fac1f6 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -69,7 +69,7 @@ pub use run_id::{RunId, fixtures}; pub use run_projection::{NodeState, PendingInterviewRecord, RunProjection}; pub use run_summary::RunSummary; pub use sandbox_record::SandboxRecord; -pub use secret::SecretType; +pub use secret::{SecretMetadata, SecretType}; pub use stage_id::{ParallelBranchId, StageId}; pub use start::StartRecord; pub use status::{ diff --git a/lib/crates/fabro-types/src/secret.rs b/lib/crates/fabro-types/src/secret.rs index 87d1dc94a..b6edfc956 100644 --- a/lib/crates/fabro-types/src/secret.rs +++ b/lib/crates/fabro-types/src/secret.rs @@ -1,3 +1,4 @@ +use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use strum::Display; @@ -10,3 +11,14 @@ pub enum SecretType { File, Credential, } + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SecretMetadata { + pub name: String, + #[serde(rename = "type")] + pub secret_type: SecretType, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + pub created_at: DateTime, + pub updated_at: DateTime, +} diff --git a/lib/crates/fabro-vault/src/lib.rs b/lib/crates/fabro-vault/src/lib.rs index f79cad879..a8c8f9561 100644 --- a/lib/crates/fabro-vault/src/lib.rs +++ b/lib/crates/fabro-vault/src/lib.rs @@ -7,6 +7,8 @@ use std::collections::HashMap; use std::path::{Component, Path, PathBuf}; use std::{fmt, io}; +use chrono::{DateTime, Utc}; +use fabro_types::SecretMetadata; pub use fabro_types::SecretType; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -16,19 +18,8 @@ pub struct SecretEntry { pub secret_type: SecretType, #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, - pub created_at: String, - pub updated_at: String, -} - -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct SecretMetadata { - pub name: String, - #[serde(rename = "type")] - pub secret_type: SecretType, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub description: Option, - pub created_at: String, - pub updated_at: String, + pub created_at: DateTime, + pub updated_at: DateTime, } #[derive(Debug)] @@ -90,12 +81,12 @@ impl Vault { ) -> Result { Self::validate_name(name, secret_type)?; - let now = chrono::Utc::now().to_rfc3339(); + let now = Utc::now(); let (created_at, description) = self.entries.get(name).map_or_else( - || (now.clone(), description.map(str::to_string)), + || (now, description.map(str::to_string)), |entry| { ( - entry.created_at.clone(), + entry.created_at, description .map(str::to_string) .or_else(|| entry.description.clone()), @@ -106,8 +97,8 @@ impl Vault { value: value.to_string(), secret_type, description: description.clone(), - created_at: created_at.clone(), - updated_at: now.clone(), + created_at, + updated_at: now, }; self.entries.insert(name.to_string(), entry); self.write_atomic()?; @@ -137,8 +128,8 @@ impl Vault { name: name.clone(), secret_type: entry.secret_type, description: entry.description.clone(), - created_at: entry.created_at.clone(), - updated_at: entry.updated_at.clone(), + created_at: entry.created_at, + updated_at: entry.updated_at, }) .collect::>(); data.sort_by(|a, b| a.name.cmp(&b.name));