From 69f620f460acc452b6db86faa293f96992d6440f Mon Sep 17 00:00:00 2001 From: garthdb Date: Tue, 19 May 2026 12:42:24 -0600 Subject: [PATCH 1/4] feat(sdk): implement write_token operation in sdk/core (closes #976) Adds the write_token operation deferred from Phase 8 epic #830. Prerequisite for TUI RFC #973 M4 (wizard write path). Co-Authored-By: Claude Sonnet 4.6 --- .changeset/write-token-operation.md | 12 + sdk/cli/src/main.rs | 126 +++++++ sdk/core/Cargo.toml | 2 +- sdk/core/src/lib.rs | 1 + sdk/core/src/write.rs | 528 ++++++++++++++++++++++++++++ 5 files changed, 668 insertions(+), 1 deletion(-) create mode 100644 .changeset/write-token-operation.md create mode 100644 sdk/core/src/write.rs diff --git a/.changeset/write-token-operation.md b/.changeset/write-token-operation.md new file mode 100644 index 00000000..55f76955 --- /dev/null +++ b/.changeset/write-token-operation.md @@ -0,0 +1,12 @@ +--- +"design-data-core": minor +"design-data-cli": minor +--- + +Add `write_token` operation to sdk/core and CLI (closes #976). + +- **sdk/core/src/write.rs**: new `write_token` — validates against `$schema`, + merges into legacy JSON, records rationale in `product-context.json`. +- **sdk/core/Cargo.toml**: enable `preserve_order` on `serde_json`. +- **sdk/cli**: add `write-token` subcommand; auto-discovers schemas dir. +- Prerequisite for TUI RFC #973 M4 (wizard write path). diff --git a/sdk/cli/src/main.rs b/sdk/cli/src/main.rs index caf6ad2c..3204f077 100644 --- a/sdk/cli/src/main.rs +++ b/sdk/cli/src/main.rs @@ -32,6 +32,7 @@ use design_data_core::naming::NamingExceptionsFile; use design_data_core::query; use design_data_core::schema::SchemaRegistry; use design_data_core::validate; +use design_data_core::write::{WriteTokenInput, write_token}; use miette::{IntoDiagnostic, WrapErr}; const SPEC_VERSION: &str = "1.0.0-draft"; @@ -173,6 +174,33 @@ enum Commands { #[arg(short, long, value_name = "TEXT")] rationale: Option, }, + /// Validate and write a product-layer token to a target file + WriteToken { + /// Token key (its name in the target file, e.g. "checkout-background-color") + #[arg(value_name = "KEY")] + key: String, + /// Token object as a JSON string (must include $schema and value) + #[arg(long, value_name = "JSON", group = "token_source")] + token_json: Option, + /// Path to a JSON file containing the token object + #[arg(long, value_name = "FILE", group = "token_source")] + token_file: Option, + /// Target legacy JSON file to write to (created if absent, merged if present) + #[arg(long, value_name = "FILE")] + target: PathBuf, + /// Path to product-context.json for rationale capture (created if absent) + #[arg(long, value_name = "FILE")] + product_context: Option, + /// Why this token was created or changed + #[arg(long, value_name = "TEXT")] + rationale: Option, + /// Token overrides an existing foundation/platform token (records in overrides[]) + #[arg(long)] + is_override: bool, + /// Path to schemas directory (default: packages/tokens/schemas relative to target) + #[arg(long, value_name = "DIR")] + schema_path: Option, + }, } #[derive(Subcommand)] @@ -1129,6 +1157,85 @@ fn run_write(output: &Path, rationale: Option<&str>) -> miette::Result Ok(ExitCode::SUCCESS) } +fn run_write_token( + key: &str, + token_json: Option<&str>, + token_file: Option<&Path>, + target: &Path, + product_context: Option<&Path>, + rationale: Option<&str>, + is_override: bool, + schema_path: Option<&Path>, +) -> miette::Result { + let token: serde_json::Value = match (token_json, token_file) { + (Some(raw), _) => serde_json::from_str(raw) + .into_diagnostic() + .wrap_err("failed to parse --token-json")?, + (None, Some(path)) => { + let text = std::fs::read_to_string(path) + .into_diagnostic() + .wrap_err_with(|| format!("failed to read {}", path.display()))?; + serde_json::from_str(&text) + .into_diagnostic() + .wrap_err_with(|| format!("failed to parse {}", path.display()))? + } + (None, None) => { + return Err(miette::miette!( + "one of --token-json or --token-file is required" + )) + } + }; + + // Resolve schema directory: explicit flag → sibling of target → default relative path. + let schemas_dir = schema_path + .map(PathBuf::from) + .or_else(|| { + // Try target's parent up to repo root looking for packages/tokens/schemas. + target.ancestors().find_map(|p| { + let candidate = p.join("packages/tokens/schemas"); + candidate.is_dir().then_some(candidate) + }) + }) + .ok_or_else(|| { + miette::miette!( + "cannot locate schemas directory; pass --schema-path explicitly" + ) + })?; + + let registry = SchemaRegistry::load_legacy_token_schemas(&schemas_dir) + .into_diagnostic() + .wrap_err("failed to load schema registry")?; + + let result = write_token( + WriteTokenInput { + key: key.to_string(), + token, + target: target.to_path_buf(), + product_context: product_context.map(PathBuf::from), + rationale: rationale.map(str::to_string), + created_at: Some(Utc::now().format("%Y-%m-%dT%H:%M:%SZ").to_string()), + is_override, + }, + ®istry, + ) + .into_diagnostic() + .wrap_err("write_token failed")?; + + for warn in &result.warnings { + eprintln!("warning: {warn}"); + } + println!("Wrote token '{}' to {}", key, result.written_to.display()); + if result.product_context_updated { + println!( + "Updated {}", + product_context + .map(|p| p.display().to_string()) + .unwrap_or_default() + ); + } + Ok(ExitCode::SUCCESS) +} + fn main() -> ExitCode { let cli = Cli::parse(); @@ -1239,6 +1346,25 @@ fn main() -> ExitCode { Commands::Write { output, rationale } => { run_write(&output, rationale.as_deref()) } + Commands::WriteToken { + key, + token_json, + token_file, + target, + product_context, + rationale, + is_override, + schema_path, + } => run_write_token( + &key, + token_json.as_deref(), + token_file.as_deref(), + &target, + product_context.as_deref(), + rationale.as_deref(), + is_override, + schema_path.as_deref(), + ), }; match result { diff --git a/sdk/core/Cargo.toml b/sdk/core/Cargo.toml index e8ea7921..e227ac5d 100644 --- a/sdk/core/Cargo.toml +++ b/sdk/core/Cargo.toml @@ -12,7 +12,7 @@ path = "src/lib.rs" [dependencies] jsonschema = "0.29" serde = { version = "1.0", features = ["derive"] } -serde_json = "1.0" +serde_json = { version = "1.0", features = ["preserve_order"] } tempfile = "3.14" thiserror = "2.0" uuid = { version = "1.11", features = ["v4"] } diff --git a/sdk/core/src/lib.rs b/sdk/core/src/lib.rs index 5b98bb00..f2c792f9 100644 --- a/sdk/core/src/lib.rs +++ b/sdk/core/src/lib.rs @@ -25,6 +25,7 @@ pub mod registry; pub mod report; pub mod schema; pub mod validate; +pub mod write; use std::path::PathBuf; diff --git a/sdk/core/src/write.rs b/sdk/core/src/write.rs new file mode 100644 index 00000000..3f09a982 --- /dev/null +++ b/sdk/core/src/write.rs @@ -0,0 +1,528 @@ +// Copyright 2026 Adobe. All rights reserved. +// This file is licensed to you under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may obtain a copy +// of the License at http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under +// the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +// OF ANY KIND, either express or implied. See the License for the specific language +// governing permissions and limitations under the License. + +//! Token write operation — validates a token fragment and writes it to a product-layer file. + +use std::path::{Path, PathBuf}; + +use serde_json::{Map, Value}; + +use crate::schema::SchemaRegistry; +use crate::CoreError; + +const SPEC_VERSION: &str = "1.0.0-draft"; + +/// Input for the `write_token` operation. +pub struct WriteTokenInput { + /// Key used to identify the token in the target legacy JSON file. + pub key: String, + /// Full token object — MUST include `$schema` and `value`. + pub token: Value, + /// Target file path (legacy JSON: `{ "key": { ...token... } }`). Created if absent. + pub target: PathBuf, + /// Optional `product-context.json` path for agent-capture-behavior rationale recording. + pub product_context: Option, + /// Optional rationale string. Recorded in both the token's inline field and product-context. + pub rationale: Option, + /// ISO 8601 timestamp used for `createdAt` when creating a new product-context document. + pub created_at: Option, + /// When true the token overrides an existing foundation/platform token (written to + /// `overrides[]`). When false it is a net-new product-layer token (written to + /// `extensions.tokens[]`). + pub is_override: bool, +} + +/// Result of a successful `write_token` call. +#[derive(Debug)] +pub struct WriteTokenResult { + /// Path of the token file that was written. + pub written_to: PathBuf, + /// Whether `product-context.json` was created or updated. + pub product_context_updated: bool, + /// Non-fatal validation warnings (schema errors block — warnings do not). + pub warnings: Vec, +} + +/// Validate, write, and record a product-layer token. +/// +/// # Errors +/// +/// Returns `Err` when: +/// - The token object is missing `$schema`. +/// - The `$schema` URL is not in the registry. +/// - The token fails structural JSON Schema validation (first error wins). +/// - File I/O fails. +pub fn write_token( + mut input: WriteTokenInput, + registry: &SchemaRegistry, +) -> Result { + let warnings: Vec = Vec::new(); + + // Inject rationale into the token object if supplied. + if let Some(ref r) = input.rationale { + if let Some(obj) = input.token.as_object_mut() { + obj.entry("rationale") + .or_insert_with(|| Value::String(r.clone())); + } + } + + // Structural validation against the token's $schema. + validate_token_object(&input.key, &input.token, registry)?; + + // Read existing target file (if any) and merge. + let mut file_map = read_legacy_file(&input.target)?; + file_map.insert(input.key.clone(), input.token.clone()); + + write_json_file(&input.target, &Value::Object(file_map))?; + + // Update product-context.json for rationale capture. + let product_context_updated = if let Some(ref pc_path) = input.product_context { + update_product_context( + pc_path, + &input.key, + &input.token, + input.rationale.as_deref(), + input.created_at.as_deref(), + input.is_override, + )?; + true + } else { + false + }; + + Ok(WriteTokenResult { + written_to: input.target, + product_context_updated, + warnings, + }) +} + +/// Validate a single token object against its `$schema` using the registry. +/// Returns `Err(CoreError::ParseError)` on the first validation failure. +fn validate_token_object( + key: &str, + token: &Value, + registry: &SchemaRegistry, +) -> Result<(), CoreError> { + let schema_url = token + .get("$schema") + .and_then(|v| v.as_str()) + .ok_or_else(|| CoreError::ParseError(format!("{key}: missing required \"$schema\"")))?; + + let validator = registry.validator_for_url(schema_url).ok_or_else(|| { + CoreError::ParseError(format!( + "{key}: unknown \"$schema\" URL (not in registry): {schema_url}" + )) + })?; + + let mut errors = validator.iter_errors(token); + if let Some(err) = errors.next() { + return Err(CoreError::ParseError(format!( + "{key}: schema validation error: {err}" + ))); + } + + Ok(()) +} + +/// Read a legacy token file into a `Map`, or return an empty map if the file does not exist. +fn read_legacy_file(path: &Path) -> Result, CoreError> { + if !path.exists() { + return Ok(Map::new()); + } + let text = std::fs::read_to_string(path)?; + let root: Value = serde_json::from_str(&text)?; + match root { + Value::Object(map) => Ok(map), + _ => Err(CoreError::ParseError(format!( + "{}: token file root must be a JSON object", + path.display() + ))), + } +} + +/// Serialize `value` as pretty-printed JSON with a trailing newline and write to `path`. +/// Creates parent directories if needed. +fn write_json_file(path: &Path, value: &Value) -> Result<(), CoreError> { + if let Some(parent) = path.parent() { + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent)?; + } + } + let json = serde_json::to_string_pretty(value)?; + std::fs::write(path, json + "\n")?; + Ok(()) +} + +/// Update (or create) `product-context.json` to record the written token's rationale. +/// +/// For overrides (`is_override = true`): appends/updates an entry in `overrides[]`. +/// For extensions (`is_override = false`): appends the token to `extensions.tokens[]`. +fn update_product_context( + path: &Path, + key: &str, + token: &Value, + rationale: Option<&str>, + created_at: Option<&str>, + is_override: bool, +) -> Result<(), CoreError> { + let mut doc: Map = if path.exists() { + let text = std::fs::read_to_string(path)?; + let v: Value = serde_json::from_str(&text)?; + match v { + Value::Object(m) => m, + _ => { + return Err(CoreError::ParseError(format!( + "{}: product-context.json root must be a JSON object", + path.display() + ))) + } + } + } else { + // New document — build in spec field order. + let mut m = Map::new(); + m.insert("specVersion".into(), Value::String(SPEC_VERSION.into())); + m.insert("layer".into(), Value::String("product".into())); + m.insert( + "createdBy".into(), + serde_json::json!({ "type": "agent", "tool": "design-data" }), + ); + if let Some(ts) = created_at { + m.insert("createdAt".into(), Value::String(ts.into())); + } + m + }; + + let uuid = token + .get("uuid") + .and_then(|v| v.as_str()) + .map(str::to_string); + + if is_override { + let overrides = doc + .entry("overrides") + .or_insert_with(|| Value::Array(Vec::new())); + if let Value::Array(arr) = overrides { + // Upsert: update existing entry with matching UUID, or append new. + let existing = uuid.as_deref().and_then(|u| { + arr.iter_mut() + .find(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) + }); + if let Some(entry) = existing { + if let (Some(r), Some(obj)) = (rationale, entry.as_object_mut()) { + obj.insert("rationale".into(), Value::String(r.into())); + } + } else { + let mut entry = Map::new(); + if let Some(u) = uuid { + entry.insert("uuid".into(), Value::String(u)); + } + if let Some(v) = token.get("value") { + entry.insert("value".into(), v.clone()); + } + if let Some(r) = rationale { + entry.insert("rationale".into(), Value::String(r.into())); + } + arr.push(Value::Object(entry)); + } + } + } else { + let extensions = doc + .entry("extensions") + .or_insert_with(|| serde_json::json!({ "tokens": [] })); + if let Some(tokens_arr) = extensions + .as_object_mut() + .and_then(|o| o.get_mut("tokens")) + .and_then(|v| v.as_array_mut()) + { + // Upsert by UUID if present, otherwise by key name. + let exists = if let Some(u) = uuid.as_deref() { + tokens_arr + .iter() + .any(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) + } else { + tokens_arr + .iter() + .any(|e| e.get("name").map(|v| v.to_string()) == token.get("name").map(|v| v.to_string())) + }; + + if !exists { + // Record the token key alongside the token for human-readability. + let mut entry = Map::new(); + entry.insert("key".into(), Value::String(key.into())); + if let Some(obj) = token.as_object() { + for (k, v) in obj { + entry.insert(k.clone(), v.clone()); + } + } + tokens_arr.push(Value::Object(entry)); + } + } + } + + write_json_file(path, &Value::Object(doc)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::schema::SchemaRegistry; + use serde_json::json; + use std::path::Path; + use tempfile::TempDir; + + fn test_registry() -> SchemaRegistry { + let schemas = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../packages/tokens/schemas"); + SchemaRegistry::load_legacy_token_schemas(&schemas).expect("schemas load") + } + + #[test] + fn write_token_creates_new_file() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let registry = test_registry(); + + let result = write_token( + WriteTokenInput { + key: "checkout-bg".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(248, 248, 248)", + "uuid": "aaaaaaaa-0001-4001-8001-000000000001" + }), + target: target.clone(), + product_context: None, + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ) + .expect("write_token should succeed"); + + assert!(target.exists()); + let text = std::fs::read_to_string(&target).unwrap(); + let doc: Value = serde_json::from_str(&text).unwrap(); + assert_eq!( + doc["checkout-bg"]["value"].as_str(), + Some("rgb(248, 248, 248)"), + "written value should round-trip" + ); + assert!(!result.product_context_updated); + } + + #[test] + fn write_token_merges_into_existing_file() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let registry = test_registry(); + + // Write first token. + write_token( + WriteTokenInput { + key: "first".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(17, 17, 17)", + "uuid": "aaaaaaaa-0001-4001-8001-000000000001" + }), + target: target.clone(), + product_context: None, + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ) + .unwrap(); + + // Write second token — should merge. + write_token( + WriteTokenInput { + key: "second".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(34, 34, 34)", + "uuid": "bbbbbbbb-0002-4002-8002-000000000002" + }), + target: target.clone(), + product_context: None, + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ) + .unwrap(); + + let text = std::fs::read_to_string(&target).unwrap(); + let doc: Value = serde_json::from_str(&text).unwrap(); + assert!(doc.get("first").is_some(), "first token preserved"); + assert!(doc.get("second").is_some(), "second token added"); + } + + #[test] + fn write_token_injects_rationale_into_token() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let registry = test_registry(); + + write_token( + WriteTokenInput { + key: "my-token".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(255, 0, 0)", + "uuid": "cccccccc-0003-4003-8003-000000000003" + }), + target: target.clone(), + product_context: None, + rationale: Some("Checkout redesign".into()), + created_at: None, + is_override: false, + }, + ®istry, + ) + .unwrap(); + + let text = std::fs::read_to_string(&target).unwrap(); + let doc: Value = serde_json::from_str(&text).unwrap(); + assert_eq!( + doc["my-token"]["rationale"].as_str(), + Some("Checkout redesign") + ); + } + + #[test] + fn write_token_creates_product_context_extension() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let pc_path = dir.path().join("product-context.json"); + let registry = test_registry(); + + let result = write_token( + WriteTokenInput { + key: "new-token".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(100, 200, 150)", + "uuid": "dddddddd-0004-4004-8004-000000000004" + }), + target: target.clone(), + product_context: Some(pc_path.clone()), + rationale: Some("New brand color".into()), + created_at: Some("2026-05-19T00:00:00Z".into()), + is_override: false, + }, + ®istry, + ) + .unwrap(); + + assert!(result.product_context_updated); + let pc_text = std::fs::read_to_string(&pc_path).unwrap(); + let pc: Value = serde_json::from_str(&pc_text).unwrap(); + assert_eq!(pc["layer"].as_str(), Some("product")); + assert_eq!(pc["createdAt"].as_str(), Some("2026-05-19T00:00:00Z")); + let tokens = &pc["extensions"]["tokens"]; + assert!(tokens.as_array().map(|a| !a.is_empty()).unwrap_or(false)); + assert_eq!( + tokens[0]["rationale"].as_str(), + Some("New brand color") + ); + } + + #[test] + fn write_token_records_override_in_product_context() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let pc_path = dir.path().join("product-context.json"); + let registry = test_registry(); + + write_token( + WriteTokenInput { + key: "existing-token".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(10, 20, 30)", + "uuid": "eeeeeeee-0005-4005-8005-000000000005" + }), + target: target.clone(), + product_context: Some(pc_path.clone()), + rationale: Some("Override reason".into()), + created_at: None, + is_override: true, + }, + ®istry, + ) + .unwrap(); + + let pc_text = std::fs::read_to_string(&pc_path).unwrap(); + let pc: Value = serde_json::from_str(&pc_text).unwrap(); + let overrides = pc["overrides"].as_array().expect("overrides array"); + assert!(!overrides.is_empty()); + assert_eq!(overrides[0]["rationale"].as_str(), Some("Override reason")); + assert_eq!( + overrides[0]["uuid"].as_str(), + Some("eeeeeeee-0005-4005-8005-000000000005") + ); + } + + #[test] + fn write_token_errors_on_unknown_schema() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let registry = test_registry(); + + let result = write_token( + WriteTokenInput { + key: "bad".into(), + token: json!({ + "$schema": "https://example.com/unknown-schema.json", + "value": "x" + }), + target, + product_context: None, + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ); + + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("unknown"), "error should mention unknown schema: {msg}"); + } + + #[test] + fn write_token_errors_on_missing_schema_field() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let registry = test_registry(); + + let result = write_token( + WriteTokenInput { + key: "bad".into(), + token: json!({ "value": "#fff" }), + target, + product_context: None, + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ); + + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("$schema"), "error should mention $schema: {msg}"); + } +} From d62f1374c0aa0794ab70ed7b0f43298c8051c59b Mon Sep 17 00:00:00 2001 From: garthdb Date: Tue, 19 May 2026 12:59:54 -0600 Subject: [PATCH 2/4] fix(sdk): resolve clippy lints blocking CI - cli/src/main.rs: group run_write_token args into WriteTokenOpts struct to fix too_many_arguments lint (8/7) - core/src/validate/rules/spec003.rs: rewrite loop as while-let to fix while_let_loop lint Co-Authored-By: Claude Sonnet 4.6 --- sdk/cli/src/main.rs | 31 ++++++++++++++++---------- sdk/core/src/validate/rules/spec003.rs | 5 +---- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/sdk/cli/src/main.rs b/sdk/cli/src/main.rs index 3204f077..16b7e5c3 100644 --- a/sdk/cli/src/main.rs +++ b/sdk/cli/src/main.rs @@ -1157,16 +1157,21 @@ fn run_write(output: &Path, rationale: Option<&str>) -> miette::Result Ok(ExitCode::SUCCESS) } +struct WriteTokenOpts<'a> { + token_json: Option<&'a str>, + token_file: Option<&'a Path>, + product_context: Option<&'a Path>, + rationale: Option<&'a str>, + is_override: bool, + schema_path: Option<&'a Path>, +} + fn run_write_token( key: &str, - token_json: Option<&str>, - token_file: Option<&Path>, target: &Path, - product_context: Option<&Path>, - rationale: Option<&str>, - is_override: bool, - schema_path: Option<&Path>, + opts: WriteTokenOpts<'_>, ) -> miette::Result { + let WriteTokenOpts { token_json, token_file, product_context, rationale, is_override, schema_path } = opts; let token: serde_json::Value = match (token_json, token_file) { (Some(raw), _) => serde_json::from_str(raw) .into_diagnostic() @@ -1357,13 +1362,15 @@ fn main() -> ExitCode { schema_path, } => run_write_token( &key, - token_json.as_deref(), - token_file.as_deref(), &target, - product_context.as_deref(), - rationale.as_deref(), - is_override, - schema_path.as_deref(), + WriteTokenOpts { + token_json: token_json.as_deref(), + token_file: token_file.as_deref(), + product_context: product_context.as_deref(), + rationale: rationale.as_deref(), + is_override, + schema_path: schema_path.as_deref(), + }, ), }; diff --git a/sdk/core/src/validate/rules/spec003.rs b/sdk/core/src/validate/rules/spec003.rs index 435b5290..df57f8cb 100644 --- a/sdk/core/src/validate/rules/spec003.rs +++ b/sdk/core/src/validate/rules/spec003.rs @@ -30,10 +30,7 @@ impl ValidationRule for Rule { } let mut path: Vec = vec![start.name.clone()]; let mut current = start; - loop { - let Some(next_name) = current.alias_target.as_ref() else { - break; - }; + while let Some(next_name) = current.alias_target.as_ref() { if path.iter().any(|p| p == next_name) { out.push(Diagnostic { file: start.file.clone(), From eee81f25f3253d8a6f1a54054db06c3e2383d37b Mon Sep 17 00:00:00 2001 From: garthdb Date: Tue, 19 May 2026 14:39:25 -0600 Subject: [PATCH 3/4] fix(sdk): remove dead warnings field; error on malformed extensions.tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **sdk/core/src/write.rs**: drop `WriteTokenResult::warnings` — the field was never populated and always returned empty, misleading callers. Replace the silent `if let Some(tokens_arr)` drop in the extensions upsert path with `ok_or_else` so a non-object `extensions` or non-array `extensions.tokens` returns an explicit error instead of silently swallowing the write. Add regression test covering the previously-silent bad-type case. - **sdk/cli/src/main.rs**: remove now-dead `result.warnings` loop. Co-Authored-By: Claude Sonnet 4.6 --- sdk/cli/src/main.rs | 3 -- sdk/core/src/write.rs | 109 ++++++++++++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 34 deletions(-) diff --git a/sdk/cli/src/main.rs b/sdk/cli/src/main.rs index 16b7e5c3..155afeb8 100644 --- a/sdk/cli/src/main.rs +++ b/sdk/cli/src/main.rs @@ -1226,9 +1226,6 @@ fn run_write_token( .into_diagnostic() .wrap_err("write_token failed")?; - for warn in &result.warnings { - eprintln!("warning: {warn}"); - } println!("Wrote token '{}' to {}", key, result.written_to.display()); if result.product_context_updated { println!( diff --git a/sdk/core/src/write.rs b/sdk/core/src/write.rs index 3f09a982..a56bd6ae 100644 --- a/sdk/core/src/write.rs +++ b/sdk/core/src/write.rs @@ -46,8 +46,6 @@ pub struct WriteTokenResult { pub written_to: PathBuf, /// Whether `product-context.json` was created or updated. pub product_context_updated: bool, - /// Non-fatal validation warnings (schema errors block — warnings do not). - pub warnings: Vec, } /// Validate, write, and record a product-layer token. @@ -63,8 +61,6 @@ pub fn write_token( mut input: WriteTokenInput, registry: &SchemaRegistry, ) -> Result { - let warnings: Vec = Vec::new(); - // Inject rationale into the token object if supplied. if let Some(ref r) = input.rationale { if let Some(obj) = input.token.as_object_mut() { @@ -100,7 +96,6 @@ pub fn write_token( Ok(WriteTokenResult { written_to: input.target, product_context_updated, - warnings, }) } @@ -234,36 +229,49 @@ fn update_product_context( } } } else { - let extensions = doc + let ext_obj = doc .entry("extensions") - .or_insert_with(|| serde_json::json!({ "tokens": [] })); - if let Some(tokens_arr) = extensions + .or_insert_with(|| serde_json::json!({})) .as_object_mut() - .and_then(|o| o.get_mut("tokens")) - .and_then(|v| v.as_array_mut()) - { - // Upsert by UUID if present, otherwise by key name. - let exists = if let Some(u) = uuid.as_deref() { - tokens_arr - .iter() - .any(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) - } else { - tokens_arr - .iter() - .any(|e| e.get("name").map(|v| v.to_string()) == token.get("name").map(|v| v.to_string())) - }; - - if !exists { - // Record the token key alongside the token for human-readability. - let mut entry = Map::new(); - entry.insert("key".into(), Value::String(key.into())); - if let Some(obj) = token.as_object() { - for (k, v) in obj { - entry.insert(k.clone(), v.clone()); - } + .ok_or_else(|| { + CoreError::ParseError(format!( + "{}: product-context.json `extensions` field is not an object", + path.display() + )) + })?; + + let tokens_arr = ext_obj + .entry("tokens") + .or_insert_with(|| Value::Array(Vec::new())) + .as_array_mut() + .ok_or_else(|| { + CoreError::ParseError(format!( + "{}: product-context.json `extensions.tokens` is not an array", + path.display() + )) + })?; + + // Upsert by UUID if present, otherwise by key name. + let exists = if let Some(u) = uuid.as_deref() { + tokens_arr + .iter() + .any(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) + } else { + tokens_arr + .iter() + .any(|e| e.get("name").map(|v| v.to_string()) == token.get("name").map(|v| v.to_string())) + }; + + if !exists { + // Record the token key alongside the token for human-readability. + let mut entry = Map::new(); + entry.insert("key".into(), Value::String(key.into())); + if let Some(obj) = token.as_object() { + for (k, v) in obj { + entry.insert(k.clone(), v.clone()); } - tokens_arr.push(Value::Object(entry)); } + tokens_arr.push(Value::Object(entry)); } } @@ -525,4 +533,43 @@ mod tests { let msg = result.unwrap_err().to_string(); assert!(msg.contains("$schema"), "error should mention $schema: {msg}"); } + + #[test] + fn write_token_errors_when_extensions_tokens_is_not_array() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let pc_path = dir.path().join("product-context.json"); + let registry = test_registry(); + + // Seed a product-context.json where extensions.tokens is a string, not an array. + std::fs::write( + &pc_path, + r#"{"specVersion":"1.0.0-draft","layer":"product","extensions":{"tokens":"bad"}}"#, + ) + .unwrap(); + + let result = write_token( + WriteTokenInput { + key: "t".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(1, 2, 3)", + "uuid": "ffffffff-0006-4006-8006-000000000006" + }), + target, + product_context: Some(pc_path), + rationale: None, + created_at: None, + is_override: false, + }, + ®istry, + ); + + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("extensions.tokens"), + "error should name the bad field: {msg}" + ); + } } From f4e36b4e9261169285c64c19ad8f84bd72063f4a Mon Sep 17 00:00:00 2001 From: garthdb Date: Tue, 19 May 2026 14:46:34 -0600 Subject: [PATCH 4/4] fix(sdk): address five review findings on write_token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **write.rs**: destructure `input` early so only `token` is `mut`. - **write.rs**: extensions upsert now matches overrides — finds existing entry by UUID (or name string via `.as_str()`, not `.to_string()`) and updates value + rationale in-place instead of silently skipping. Adds regression test `write_token_upserts_extension_entry_in_place`. - **Cargo.toml**: document why `preserve_order` is enabled (IndexMap swap is crate-wide; needed for stable key order on write round-trip). - **main.rs**: import `ArgGroup`; annotate `WriteToken` variant so Clap enforces exactly one of `--token-json` / `--token-file` at parse time. Co-Authored-By: Claude Sonnet 4.6 --- sdk/cli/src/main.rs | 3 +- sdk/core/Cargo.toml | 2 + sdk/core/src/write.rs | 104 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 88 insertions(+), 21 deletions(-) diff --git a/sdk/cli/src/main.rs b/sdk/cli/src/main.rs index 155afeb8..383133ac 100644 --- a/sdk/cli/src/main.rs +++ b/sdk/cli/src/main.rs @@ -17,7 +17,7 @@ mod format; use std::collections::HashSet; -use clap::{Parser, Subcommand, ValueEnum}; +use clap::{ArgGroup, Parser, Subcommand, ValueEnum}; use design_data_core::cascade::{resolve, ResolutionContext}; use design_data_core::compat::{ load_snapshot, snapshot_matches, write_snapshot, ValidationSnapshot, @@ -175,6 +175,7 @@ enum Commands { rationale: Option, }, /// Validate and write a product-layer token to a target file + #[command(group(ArgGroup::new("token_source").required(true).args(["token_json", "token_file"])))] WriteToken { /// Token key (its name in the target file, e.g. "checkout-background-color") #[arg(value_name = "KEY")] diff --git a/sdk/core/Cargo.toml b/sdk/core/Cargo.toml index e227ac5d..8733cc40 100644 --- a/sdk/core/Cargo.toml +++ b/sdk/core/Cargo.toml @@ -12,6 +12,8 @@ path = "src/lib.rs" [dependencies] jsonschema = "0.29" serde = { version = "1.0", features = ["derive"] } +# preserve_order switches serde_json's Map from BTreeMap to IndexMap (crate-wide). +# Required so write_token merges tokens without resorting keys on round-trip. serde_json = { version = "1.0", features = ["preserve_order"] } tempfile = "3.14" thiserror = "2.0" diff --git a/sdk/core/src/write.rs b/sdk/core/src/write.rs index a56bd6ae..2634c70f 100644 --- a/sdk/core/src/write.rs +++ b/sdk/core/src/write.rs @@ -58,35 +58,39 @@ pub struct WriteTokenResult { /// - The token fails structural JSON Schema validation (first error wins). /// - File I/O fails. pub fn write_token( - mut input: WriteTokenInput, + input: WriteTokenInput, registry: &SchemaRegistry, ) -> Result { + // Destructure so it's clear only `token` is mutated. + let WriteTokenInput { key, mut token, target, product_context, rationale, created_at, is_override } = + input; + // Inject rationale into the token object if supplied. - if let Some(ref r) = input.rationale { - if let Some(obj) = input.token.as_object_mut() { + if let Some(ref r) = rationale { + if let Some(obj) = token.as_object_mut() { obj.entry("rationale") .or_insert_with(|| Value::String(r.clone())); } } // Structural validation against the token's $schema. - validate_token_object(&input.key, &input.token, registry)?; + validate_token_object(&key, &token, registry)?; // Read existing target file (if any) and merge. - let mut file_map = read_legacy_file(&input.target)?; - file_map.insert(input.key.clone(), input.token.clone()); + let mut file_map = read_legacy_file(&target)?; + file_map.insert(key.clone(), token.clone()); - write_json_file(&input.target, &Value::Object(file_map))?; + write_json_file(&target, &Value::Object(file_map))?; // Update product-context.json for rationale capture. - let product_context_updated = if let Some(ref pc_path) = input.product_context { + let product_context_updated = if let Some(ref pc_path) = product_context { update_product_context( pc_path, - &input.key, - &input.token, - input.rationale.as_deref(), - input.created_at.as_deref(), - input.is_override, + &key, + &token, + rationale.as_deref(), + created_at.as_deref(), + is_override, )?; true } else { @@ -94,7 +98,7 @@ pub fn write_token( }; Ok(WriteTokenResult { - written_to: input.target, + written_to: target, product_context_updated, }) } @@ -251,19 +255,30 @@ fn update_product_context( )) })?; - // Upsert by UUID if present, otherwise by key name. - let exists = if let Some(u) = uuid.as_deref() { + // Upsert by UUID if present, otherwise by name string. + let token_name = token.get("name").and_then(|v| v.as_str()); + let existing_idx = if let Some(u) = uuid.as_deref() { tokens_arr .iter() - .any(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) + .position(|e| e.get("uuid").and_then(|v| v.as_str()) == Some(u)) } else { tokens_arr .iter() - .any(|e| e.get("name").map(|v| v.to_string()) == token.get("name").map(|v| v.to_string())) + .position(|e| e.get("name").and_then(|v| v.as_str()) == token_name) }; - if !exists { - // Record the token key alongside the token for human-readability. + if let Some(idx) = existing_idx { + // Update value and rationale in-place. + if let Some(obj) = tokens_arr[idx].as_object_mut() { + if let Some(v) = token.get("value") { + obj.insert("value".into(), v.clone()); + } + if let Some(r) = rationale { + obj.insert("rationale".into(), Value::String(r.into())); + } + } + } else { + // New entry — record the token key for human-readability. let mut entry = Map::new(); entry.insert("key".into(), Value::String(key.into())); if let Some(obj) = token.as_object() { @@ -572,4 +587,53 @@ mod tests { "error should name the bad field: {msg}" ); } + + #[test] + fn write_token_upserts_extension_entry_in_place() { + let dir = TempDir::new().unwrap(); + let target = dir.path().join("tokens.json"); + let pc_path = dir.path().join("product-context.json"); + let registry = test_registry(); + + let common = WriteTokenInput { + key: "tok".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(1, 1, 1)", + "uuid": "a7a7a7a7-0007-4007-8007-000000000007" + }), + target: target.clone(), + product_context: Some(pc_path.clone()), + rationale: Some("first write".into()), + created_at: None, + is_override: false, + }; + write_token(common, ®istry).unwrap(); + + // Second write: new value, new rationale — should update in place, not append. + write_token( + WriteTokenInput { + key: "tok".into(), + token: json!({ + "$schema": "https://opensource.adobe.com/spectrum-design-data/schemas/token-types/color.json", + "value": "rgb(2, 2, 2)", + "uuid": "a7a7a7a7-0007-4007-8007-000000000007" + }), + target, + product_context: Some(pc_path.clone()), + rationale: Some("updated rationale".into()), + created_at: None, + is_override: false, + }, + ®istry, + ) + .unwrap(); + + let pc: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&pc_path).unwrap()).unwrap(); + let tokens = pc["extensions"]["tokens"].as_array().expect("tokens array"); + assert_eq!(tokens.len(), 1, "upsert must not duplicate the entry"); + assert_eq!(tokens[0]["value"].as_str(), Some("rgb(2, 2, 2)")); + assert_eq!(tokens[0]["rationale"].as_str(), Some("updated rationale")); + } }