From 4390554477a6f20db731482ca7414ab0ba26660c Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 4 Sep 2026 13:30:29 +0900 Subject: [PATCH] feat: share Skill REST DTO authority --- crates/worker/src/skill.rs | 153 +---- crates/workspace-api/Cargo.toml | 4 + .../examples/generate_skill_api_types.rs | 3 + crates/workspace-api/src/lib.rs | 601 ++++++++++++++++++ crates/workspace-server/src/skills.rs | 67 +- web/workspace/src/lib/generated/skill-api.ts | 87 +++ .../src/lib/workspace/api/http.test.ts | 17 +- web/workspace/src/lib/workspace/api/http.ts | 64 +- web/workspace/src/lib/workspace/skills/api.ts | 455 +++++++++++++ web/workspace/tests/skill-api.test.ts | 221 +++++++ 10 files changed, 1487 insertions(+), 185 deletions(-) create mode 100644 crates/workspace-api/examples/generate_skill_api_types.rs create mode 100644 web/workspace/src/lib/generated/skill-api.ts create mode 100644 web/workspace/src/lib/workspace/skills/api.ts create mode 100644 web/workspace/tests/skill-api.test.ts diff --git a/crates/worker/src/skill.rs b/crates/worker/src/skill.rs index 055cb9fe..ecb47ced 100644 --- a/crates/worker/src/skill.rs +++ b/crates/worker/src/skill.rs @@ -2,127 +2,11 @@ use serde::{Deserialize, Serialize}; use crate::worker::WorkspaceClient; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "snake_case")] -pub enum SkillDiagnosticSeverity { - Error, - Warning, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillDiagnostic { - pub severity: SkillDiagnosticSeverity, - pub code: String, - pub message: String, - /// Path-free authority/provenance label such as `builtin:foo` or `workspace:foo`. - #[serde(skip_serializing_if = "Option::is_none")] - pub source: Option, -} - -impl SkillDiagnostic { - pub fn error( - code: impl Into, - message: impl Into, - source: Option, - ) -> Self { - Self { - severity: SkillDiagnosticSeverity::Error, - code: code.into(), - message: message.into(), - source, - } - } - - pub fn warning( - code: impl Into, - message: impl Into, - source: Option, - ) -> Self { - Self { - severity: SkillDiagnosticSeverity::Warning, - code: code.into(), - message: message.into(), - source, - } - } -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "snake_case")] -pub enum SkillSourceKind { - Builtin, - Workspace, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillProvenance { - pub kind: SkillSourceKind, - /// Stable id: `builtin:` or `workspace:`. - pub id: String, - /// Virtual config/resource path. Never an absolute host filesystem path. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub virtual_path: Option, - /// Active Workspace config revision for Workspace Skills. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub revision: Option, - /// Digest of the immutable `SKILL.md` source. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub source_digest: Option, - /// Digest of the active virtual config tree snapshot. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub tree_digest: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillResourceRef { - pub kind: String, - /// Skill-relative resource name/path. Never an absolute filesystem path. - pub name: String, - pub supported: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub diagnostic: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillCatalogEntry { - pub name: String, - pub description: String, - pub provenance: SkillProvenance, - #[serde(default)] - pub overrides: Vec, - #[serde(default)] - pub diagnostics: Vec, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillCatalogResponse { - /// Authority label for diagnostics; callers must not interpret it as a path. - pub authority: String, - #[serde(default)] - pub entries: Vec, - #[serde(default)] - pub diagnostics: Vec, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SkillDetailResponse { - pub name: String, - pub description: String, - pub provenance: SkillProvenance, - #[serde(default)] - pub overrides: Vec, - #[serde(default)] - pub diagnostics: Vec, - /// Imported Markdown content with YAML frontmatter delimiters removed. - /// This is intentionally omitted from catalog responses. - pub body: String, - #[serde(default)] - pub allowed_tools: Vec, - /// Explicitly documents that allowed-tools is parsed only as an experimental hint. - pub allowed_tools_status: String, - #[serde(default)] - pub resources: Vec, -} +pub use workspace_api::{ + SkillActivationStatus, SkillCatalogEntry, SkillCatalogResponse, SkillDetailResponse, + SkillDiagnostic, SkillDiagnosticSeverity, SkillProjectionIdentity, SkillProjectionStatus, + SkillProvenance, SkillResourceRef, SkillSourceKind, +}; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct SkillActivationResponse { @@ -144,6 +28,8 @@ pub enum SkillClientError { Request(#[from] crate::worker::WorkspaceClientError), #[error("Skill API response JSON is invalid: {0}")] Json(#[from] serde_json::Error), + #[error("Skill API response violates the shared contract: {0}")] + InvalidResponse(#[from] workspace_api::SkillApiValidationError), #[error("Skill API returned HTTP {status}: {body}")] Http { status: reqwest::StatusCode, @@ -155,11 +41,15 @@ pub enum SkillClientError { impl dyn WorkspaceClient + '_ { pub fn list_skills(&self) -> Result { - self.get_skill_json("skills") + let response: SkillCatalogResponse = self.get_skill_json("skills")?; + response.validate()?; + Ok(response) } pub fn read_skill(&self, name: &str) -> Result { - self.get_skill_json(&format!("skills/{name}")) + let response: SkillDetailResponse = self.get_skill_json(&format!("skills/{name}"))?; + response.validate()?; + Ok(response) } pub fn activate_skill(&self, name: &str) -> Result { @@ -229,11 +119,24 @@ mod tests { assert_eq!(worker_header, None); assert_eq!(authorization, None); let body = serde_json::json!({ - "authority": "workspace-backend-skills-v0", + "authority": "workspace-config-skills-v1", + "projection": { + "config_revision": 7, + "tree_digest": "tree-digest" + }, "entries": [{ "name": "triage-errors", "description": "Use when triaging errors.", - "provenance": { "kind": "workspace", "id": "workspace:triage-errors" }, + "activation_status": "active", + "projection_status": "valid", + "provenance": { + "kind": "workspace", + "id": "workspace:triage-errors", + "virtual_path": "skills/triage-errors/SKILL.md", + "revision": 7, + "source_digest": "source-digest", + "tree_digest": "tree-digest" + }, "overrides": [], "diagnostics": [] }], diff --git a/crates/workspace-api/Cargo.toml b/crates/workspace-api/Cargo.toml index a4e5f31e..468c6547 100644 --- a/crates/workspace-api/Cargo.toml +++ b/crates/workspace-api/Cargo.toml @@ -38,6 +38,10 @@ required-features = ["typescript"] name = "generate_memory_api_types" required-features = ["typescript"] +[[example]] +name = "generate_skill_api_types" +required-features = ["typescript"] + [[example]] name = "generate_auth_api_types" required-features = ["typescript"] diff --git a/crates/workspace-api/examples/generate_skill_api_types.rs b/crates/workspace-api/examples/generate_skill_api_types.rs new file mode 100644 index 00000000..4fa47ded --- /dev/null +++ b/crates/workspace-api/examples/generate_skill_api_types.rs @@ -0,0 +1,3 @@ +fn main() { + print!("{}", workspace_api::skill_api_typescript()); +} diff --git a/crates/workspace-api/src/lib.rs b/crates/workspace-api/src/lib.rs index ea076429..e9b16045 100644 --- a/crates/workspace-api/src/lib.rs +++ b/crates/workspace-api/src/lib.rs @@ -1934,6 +1934,407 @@ pub struct RepositoryAccessProjection { pub bindings: Vec, } +pub const SKILL_CATALOG_AUTHORITY: &str = "workspace-config-skills-v1"; +pub const SKILL_API_MAX_SAFE_INTEGER: u64 = 9_007_199_254_740_991; +pub const SKILL_API_MAX_CATALOG_ENTRIES: usize = 500; +pub const SKILL_API_MAX_OVERRIDES: usize = 64; +pub const SKILL_API_MAX_DIAGNOSTICS: usize = 100; +pub const SKILL_API_MAX_RESOURCES: usize = 500; +pub const SKILL_API_MAX_ALLOWED_TOOLS: usize = 100; +pub const SKILL_API_MAX_NAME_BYTES: usize = 128; +pub const SKILL_API_MAX_LABEL_BYTES: usize = 4_096; +pub const SKILL_API_MAX_BODY_BYTES: usize = 1_048_576; +pub const SKILL_API_MAX_PATH_BYTES: usize = 1_024; +pub const SKILL_API_MAX_DIGEST_BYTES: usize = 128; + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(rename_all = "snake_case"))] +#[serde(rename_all = "snake_case")] +pub enum SkillDiagnosticSeverity { + Error, + Warning, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillDiagnostic { + pub severity: SkillDiagnosticSeverity, + pub code: String, + pub message: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional))] + pub source: Option, +} + +impl SkillDiagnostic { + pub fn error( + code: impl Into, + message: impl Into, + source: Option, + ) -> Self { + Self { + severity: SkillDiagnosticSeverity::Error, + code: code.into(), + message: message.into(), + source, + } + } + + pub fn warning( + code: impl Into, + message: impl Into, + source: Option, + ) -> Self { + Self { + severity: SkillDiagnosticSeverity::Warning, + code: code.into(), + message: message.into(), + source, + } + } +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(rename_all = "snake_case"))] +#[serde(rename_all = "snake_case")] +pub enum SkillSourceKind { + Builtin, + Workspace, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillProvenance { + pub kind: SkillSourceKind, + pub id: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional))] + pub virtual_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional, type = "number"))] + pub revision: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional))] + pub source_digest: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional))] + pub tree_digest: Option, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(rename_all = "snake_case"))] +#[serde(rename_all = "snake_case")] +pub enum SkillActivationStatus { + Active, + Inactive, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(rename_all = "snake_case"))] +#[serde(rename_all = "snake_case")] +pub enum SkillProjectionStatus { + Valid, + Invalid, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillProjectionIdentity { + #[cfg_attr(feature = "typescript", ts(type = "number"))] + pub config_revision: u64, + pub tree_digest: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillResourceRef { + pub kind: String, + pub name: String, + pub supported: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional))] + pub diagnostic: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillCatalogEntry { + pub name: String, + pub description: String, + pub activation_status: SkillActivationStatus, + pub projection_status: SkillProjectionStatus, + pub provenance: SkillProvenance, + pub overrides: Vec, + pub diagnostics: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillCatalogResponse { + pub authority: String, + pub projection: SkillProjectionIdentity, + pub entries: Vec, + pub diagnostics: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(deny_unknown_fields)] +pub struct SkillDetailResponse { + pub authority: String, + pub projection: SkillProjectionIdentity, + pub name: String, + pub description: String, + pub provenance: SkillProvenance, + pub overrides: Vec, + pub diagnostics: Vec, + pub activation_status: SkillActivationStatus, + pub projection_status: SkillProjectionStatus, + pub body: String, + pub allowed_tools: Vec, + pub allowed_tools_status: String, + pub resources: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillApiValidationError { + CollectionTooLarge, + StringTooLarge, + InvalidProjectionIdentity, + InvalidProvenance, + InvalidVirtualPath, + StaleProjection, +} + +impl std::fmt::Display for SkillApiValidationError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let message = match self { + Self::CollectionTooLarge => "Skill API collection exceeds its limit", + Self::StringTooLarge => "Skill API string exceeds its limit", + Self::InvalidProjectionIdentity => "Skill API projection identity is invalid", + Self::InvalidProvenance => "Skill API provenance is invalid", + Self::InvalidVirtualPath => "Skill API virtual path is invalid", + Self::StaleProjection => "Workspace Skill projection is stale", + }; + formatter.write_str(message) + } +} + +impl std::error::Error for SkillApiValidationError {} + +impl SkillProjectionIdentity { + fn validate(&self) -> Result<(), SkillApiValidationError> { + validate_safe_integer(self.config_revision)?; + validate_nonempty_string(&self.tree_digest, SKILL_API_MAX_DIGEST_BYTES) + .map_err(|_| SkillApiValidationError::InvalidProjectionIdentity) + } +} + +impl SkillProvenance { + fn validate( + &self, + projection: &SkillProjectionIdentity, + ) -> Result<(), SkillApiValidationError> { + validate_nonempty_string(&self.id, SKILL_API_MAX_LABEL_BYTES)?; + validate_optional_string(self.virtual_path.as_deref(), SKILL_API_MAX_PATH_BYTES)?; + validate_optional_string(self.source_digest.as_deref(), SKILL_API_MAX_DIGEST_BYTES)?; + validate_optional_string(self.tree_digest.as_deref(), SKILL_API_MAX_DIGEST_BYTES)?; + if let Some(revision) = self.revision { + validate_safe_integer(revision)?; + } + + let expected_prefix = match self.kind { + SkillSourceKind::Builtin => "builtin:", + SkillSourceKind::Workspace => "workspace:", + }; + if !self.id.starts_with(expected_prefix) + || self + .virtual_path + .as_deref() + .is_none_or(|path| !is_virtual_path(path)) + || self.source_digest.is_none() + { + return Err(SkillApiValidationError::InvalidProvenance); + } + + match self.kind { + SkillSourceKind::Builtin => { + if self.revision.is_some() || self.tree_digest.is_some() { + return Err(SkillApiValidationError::InvalidProvenance); + } + } + SkillSourceKind::Workspace => { + let Some(revision) = self.revision else { + return Err(SkillApiValidationError::InvalidProvenance); + }; + let Some(tree_digest) = self.tree_digest.as_deref() else { + return Err(SkillApiValidationError::InvalidProvenance); + }; + if revision != projection.config_revision || tree_digest != projection.tree_digest { + return Err(SkillApiValidationError::StaleProjection); + } + } + } + Ok(()) + } +} + +impl SkillCatalogEntry { + fn validate( + &self, + projection: &SkillProjectionIdentity, + ) -> Result<(), SkillApiValidationError> { + validate_nonempty_string(&self.name, SKILL_API_MAX_NAME_BYTES)?; + validate_string(&self.description, SKILL_API_MAX_LABEL_BYTES)?; + validate_collection(&self.overrides, SKILL_API_MAX_OVERRIDES)?; + validate_diagnostics(&self.diagnostics)?; + self.provenance.validate(projection)?; + for provenance in &self.overrides { + provenance.validate(projection)?; + } + Ok(()) + } +} + +impl SkillCatalogResponse { + pub fn validate(&self) -> Result<(), SkillApiValidationError> { + validate_nonempty_string(&self.authority, SKILL_API_MAX_LABEL_BYTES)?; + if self.authority != SKILL_CATALOG_AUTHORITY { + return Err(SkillApiValidationError::InvalidProjectionIdentity); + } + self.projection.validate()?; + validate_collection(&self.entries, SKILL_API_MAX_CATALOG_ENTRIES)?; + validate_diagnostics(&self.diagnostics)?; + for entry in &self.entries { + entry.validate(&self.projection)?; + } + Ok(()) + } +} + +impl SkillDetailResponse { + pub fn validate(&self) -> Result<(), SkillApiValidationError> { + validate_nonempty_string(&self.authority, SKILL_API_MAX_LABEL_BYTES)?; + if self.authority != SKILL_CATALOG_AUTHORITY { + return Err(SkillApiValidationError::InvalidProjectionIdentity); + } + self.projection.validate()?; + validate_nonempty_string(&self.name, SKILL_API_MAX_NAME_BYTES)?; + validate_string(&self.description, SKILL_API_MAX_LABEL_BYTES)?; + validate_string(&self.body, SKILL_API_MAX_BODY_BYTES)?; + validate_strings( + &self.allowed_tools, + SKILL_API_MAX_ALLOWED_TOOLS, + SKILL_API_MAX_LABEL_BYTES, + )?; + validate_nonempty_string(&self.allowed_tools_status, SKILL_API_MAX_LABEL_BYTES)?; + validate_resources(&self.resources)?; + validate_collection(&self.overrides, SKILL_API_MAX_OVERRIDES)?; + validate_diagnostics(&self.diagnostics)?; + self.provenance.validate(&self.projection)?; + for provenance in &self.overrides { + provenance.validate(&self.projection)?; + } + Ok(()) + } +} + +fn validate_safe_integer(value: u64) -> Result<(), SkillApiValidationError> { + if value <= SKILL_API_MAX_SAFE_INTEGER { + Ok(()) + } else { + Err(SkillApiValidationError::InvalidProjectionIdentity) + } +} + +fn validate_collection(values: &[T], limit: usize) -> Result<(), SkillApiValidationError> { + if values.len() <= limit { + Ok(()) + } else { + Err(SkillApiValidationError::CollectionTooLarge) + } +} + +fn validate_string(value: &str, limit: usize) -> Result<(), SkillApiValidationError> { + if value.len() <= limit { + Ok(()) + } else { + Err(SkillApiValidationError::StringTooLarge) + } +} + +fn validate_nonempty_string(value: &str, limit: usize) -> Result<(), SkillApiValidationError> { + validate_string(value, limit)?; + if value.is_empty() { + Err(SkillApiValidationError::StringTooLarge) + } else { + Ok(()) + } +} + +fn validate_optional_string( + value: Option<&str>, + limit: usize, +) -> Result<(), SkillApiValidationError> { + if let Some(value) = value { + validate_nonempty_string(value, limit)?; + } + Ok(()) +} + +fn validate_strings( + values: &[String], + collection_limit: usize, + string_limit: usize, +) -> Result<(), SkillApiValidationError> { + validate_collection(values, collection_limit)?; + for value in values { + validate_nonempty_string(value, string_limit)?; + } + Ok(()) +} + +fn validate_resources(resources: &[SkillResourceRef]) -> Result<(), SkillApiValidationError> { + validate_collection(resources, SKILL_API_MAX_RESOURCES)?; + for resource in resources { + validate_nonempty_string(&resource.kind, SKILL_API_MAX_LABEL_BYTES)?; + validate_nonempty_string(&resource.name, SKILL_API_MAX_PATH_BYTES)?; + if !is_virtual_path(&resource.name) { + return Err(SkillApiValidationError::InvalidVirtualPath); + } + validate_optional_string(resource.diagnostic.as_deref(), SKILL_API_MAX_LABEL_BYTES)?; + } + Ok(()) +} + +fn is_virtual_path(value: &str) -> bool { + !value.starts_with('/') + && !value.contains('\\') + && value + .split('/') + .all(|component| !component.is_empty() && component != "." && component != "..") +} + +fn validate_diagnostics(diagnostics: &[SkillDiagnostic]) -> Result<(), SkillApiValidationError> { + validate_collection(diagnostics, SKILL_API_MAX_DIAGNOSTICS)?; + for diagnostic in diagnostics { + validate_nonempty_string(&diagnostic.code, SKILL_API_MAX_LABEL_BYTES)?; + validate_nonempty_string(&diagnostic.message, SKILL_API_MAX_LABEL_BYTES)?; + validate_optional_string(diagnostic.source.as_deref(), SKILL_API_MAX_PATH_BYTES)?; + } + Ok(()) +} + #[cfg(feature = "typescript")] pub fn catalog_typescript() -> String { use ts_rs::TS; @@ -2005,6 +2406,37 @@ pub fn repository_access_api_typescript() -> String { ) } +#[cfg(feature = "typescript")] +pub fn skill_api_typescript() -> String { + use ts_rs::TS; + + let config = ts_rs::Config::default(); + let declarations = [ + SkillDiagnosticSeverity::decl(&config), + SkillDiagnostic::decl(&config), + SkillSourceKind::decl(&config), + SkillProvenance::decl(&config), + SkillActivationStatus::decl(&config), + SkillProjectionStatus::decl(&config), + SkillProjectionIdentity::decl(&config), + SkillResourceRef::decl(&config), + SkillCatalogEntry::decl(&config), + SkillCatalogResponse::decl(&config), + SkillDetailResponse::decl(&config), + ]; + let limits = format!( + "export const SKILL_API_AUTHORITY = \"{SKILL_CATALOG_AUTHORITY}\" as const;\n\nexport const SKILL_API_LIMITS = {{\n maxSafeInteger: {SKILL_API_MAX_SAFE_INTEGER},\n maxCatalogEntries: {SKILL_API_MAX_CATALOG_ENTRIES},\n maxOverrides: {SKILL_API_MAX_OVERRIDES},\n maxDiagnostics: {SKILL_API_MAX_DIAGNOSTICS},\n maxResources: {SKILL_API_MAX_RESOURCES},\n maxAllowedTools: {SKILL_API_MAX_ALLOWED_TOOLS},\n maxNameBytes: {SKILL_API_MAX_NAME_BYTES},\n maxLabelBytes: {SKILL_API_MAX_LABEL_BYTES},\n maxBodyBytes: {SKILL_API_MAX_BODY_BYTES},\n maxPathBytes: {SKILL_API_MAX_PATH_BYTES},\n maxDigestBytes: {SKILL_API_MAX_DIGEST_BYTES},\n}} as const;" + ); + format!( + "// Generated from workspace-api. Do not edit by hand.\n// Regenerate: cargo run -q -p workspace-api --features typescript --example generate_skill_api_types > web/workspace/src/lib/generated/skill-api.ts\n\n{limits}\n\n{}\n", + declarations + .into_iter() + .map(|declaration| format!("export {declaration}")) + .collect::>() + .join("\n\n") + ) +} + #[cfg(feature = "typescript")] pub fn auth_api_typescript() -> String { use ts_rs::TS; @@ -2172,6 +2604,36 @@ mod memory_typescript_tests { } } +#[cfg(all(test, feature = "typescript"))] +mod skill_typescript_tests { + #[test] + fn generated_skill_api_contract_is_current() { + let expected = super::skill_api_typescript(); + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../web/workspace/src/lib/generated/skill-api.ts"); + let actual = std::fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("failed to read {}: {error}", path.display())); + assert_eq!( + normalize(&actual), + normalize(&expected), + "regenerate Skill API TypeScript types with `cargo run -q -p workspace-api --features typescript --example generate_skill_api_types > web/workspace/src/lib/generated/skill-api.ts` and format the generated file", + ); + } + + fn normalize(value: &str) -> String { + value + .chars() + .filter_map(|character| match character { + character if character.is_whitespace() => None, + ',' => Some(';'), + character => Some(character), + }) + .collect::() + .replace("=|", "=") + .replace(";}", "}") + } +} + #[cfg(all(test, feature = "typescript"))] mod workdir_typescript_tests { #[test] @@ -2205,6 +2667,145 @@ mod workdir_typescript_tests { mod tests { use super::*; + fn skill_projection() -> SkillProjectionIdentity { + SkillProjectionIdentity { + config_revision: 42, + tree_digest: "tree-digest".to_string(), + } + } + + fn builtin_skill_provenance() -> SkillProvenance { + SkillProvenance { + kind: SkillSourceKind::Builtin, + id: "builtin:errors".to_string(), + virtual_path: Some("skills/errors/SKILL.md".to_string()), + revision: None, + source_digest: Some("builtin-source-digest".to_string()), + tree_digest: None, + } + } + + fn workspace_skill_provenance() -> SkillProvenance { + SkillProvenance { + kind: SkillSourceKind::Workspace, + id: "workspace:skills/release/SKILL.md".to_string(), + virtual_path: Some("skills/release/SKILL.md".to_string()), + revision: Some(42), + source_digest: Some("workspace-source-digest".to_string()), + tree_digest: Some("tree-digest".to_string()), + } + } + + #[test] + fn skill_catalog_round_trips_builtin_workspace_and_invalid_projection_entries() { + let response = SkillCatalogResponse { + authority: "workspace-config-skills-v1".to_string(), + projection: skill_projection(), + entries: vec![ + SkillCatalogEntry { + name: "errors".to_string(), + description: "Builtin guidance".to_string(), + activation_status: SkillActivationStatus::Active, + projection_status: SkillProjectionStatus::Valid, + provenance: builtin_skill_provenance(), + overrides: vec![], + diagnostics: vec![], + }, + SkillCatalogEntry { + name: "release".to_string(), + description: "Workspace guidance".to_string(), + activation_status: SkillActivationStatus::Inactive, + projection_status: SkillProjectionStatus::Invalid, + provenance: workspace_skill_provenance(), + overrides: vec![builtin_skill_provenance()], + diagnostics: vec![SkillDiagnostic { + severity: SkillDiagnosticSeverity::Error, + code: "invalid_projection".to_string(), + message: "invalid projected Skill".to_string(), + source: Some("skills/release/SKILL.md".to_string()), + }], + }, + ], + diagnostics: vec![], + }; + + response.validate().expect("fixture should be valid"); + let json = serde_json::to_string(&response).expect("serialize Skill catalog"); + let decoded: SkillCatalogResponse = + serde_json::from_str(&json).expect("deserialize Skill catalog"); + assert_eq!(decoded, response); + assert!(!json.contains("\"revision\":null")); + assert!(!json.contains("\"tree_digest\":null")); + } + + #[test] + fn skill_detail_round_trips_shared_response() { + let response = SkillDetailResponse { + authority: "workspace-config-skills-v1".to_string(), + projection: skill_projection(), + name: "release".to_string(), + description: "Workspace guidance".to_string(), + body: "# Release\n".to_string(), + allowed_tools: vec!["Bash".to_string()], + allowed_tools_status: "experimental_hint_only".to_string(), + resources: vec![], + activation_status: SkillActivationStatus::Active, + projection_status: SkillProjectionStatus::Valid, + provenance: workspace_skill_provenance(), + overrides: vec![], + diagnostics: vec![], + }; + + response.validate().expect("fixture should be valid"); + let decoded: SkillDetailResponse = serde_json::from_value( + serde_json::to_value(&response).expect("serialize Skill detail"), + ) + .expect("deserialize Skill detail"); + assert_eq!(decoded, response); + } + + #[test] + fn skill_projection_validation_detects_stale_workspace_revision() { + let mut provenance = workspace_skill_provenance(); + provenance.revision = Some(41); + let response = SkillCatalogResponse { + authority: "workspace-config-skills-v1".to_string(), + projection: skill_projection(), + entries: vec![SkillCatalogEntry { + name: "release".to_string(), + description: String::new(), + activation_status: SkillActivationStatus::Active, + projection_status: SkillProjectionStatus::Valid, + provenance, + overrides: vec![], + diagnostics: vec![], + }], + diagnostics: vec![], + }; + + assert_eq!( + response.validate(), + Err(SkillApiValidationError::StaleProjection) + ); + } + + #[test] + fn skill_dto_rejects_unknown_fields_and_unknown_provenance_kind() { + let unknown_field = serde_json::json!({ + "authority": "workspace-config-skills-v1", + "projection": {"config_revision": 42, "tree_digest": "tree-digest"}, + "entries": [], + "diagnostics": [], + "body": "must not be accepted" + }); + assert!(serde_json::from_value::(unknown_field).is_err()); + + let mut provenance = + serde_json::to_value(workspace_skill_provenance()).expect("serialize provenance"); + provenance["kind"] = serde_json::Value::String("newer_source_kind".to_string()); + assert!(serde_json::from_value::(provenance).is_err()); + } + #[test] fn memory_evidence_origins_round_trip_as_typed_provenance() { let kinds = [ diff --git a/crates/workspace-server/src/skills.rs b/crates/workspace-server/src/skills.rs index 03bcc793..91703108 100644 --- a/crates/workspace-server/src/skills.rs +++ b/crates/workspace-server/src/skills.rs @@ -4,9 +4,11 @@ use config_source::{ ConfigSchemaContribution, MarkdownDocumentProjection, VirtualPath, project_markdown_document, }; use serde::Deserialize; -use worker::skill::{ - SkillActivationResponse, SkillCatalogEntry, SkillCatalogResponse, SkillDetailResponse, - SkillDiagnostic, SkillDiagnosticSeverity, SkillProvenance, SkillResourceRef, SkillSourceKind, +use worker::skill::SkillActivationResponse; +use workspace_api::{ + SKILL_CATALOG_AUTHORITY, SkillActivationStatus, SkillCatalogEntry, SkillCatalogResponse, + SkillDetailResponse, SkillDiagnostic, SkillDiagnosticSeverity, SkillProjectionIdentity, + SkillProjectionStatus, SkillProvenance, SkillResourceRef, SkillSourceKind, }; use crate::config_source::{ @@ -19,7 +21,6 @@ const BUILTIN_SKILL_VIRTUAL_PATH: &str = "builtin/skills/agent-skills/SKILL.md"; const SKILL_SCHEMA_PROVIDER_ID: &str = "builtin:skills"; const SKILL_SCHEMA_NAMESPACE: &str = "skills"; const SKILL_SCHEMA_VERSION: &str = "1"; -const SKILL_CATALOG_AUTHORITY: &str = "workspace-config-skills-v1"; /// Skill documents are values imported from `SKILL.md`. Known Agent Skills /// frontmatter is typed while extension keys remain concrete values. @@ -101,32 +102,54 @@ pub fn catalog(state: &WorkspaceConfigState) -> Result Result { catalog(state) } +fn projection_identity(state: &WorkspaceConfigState) -> SkillProjectionIdentity { + SkillProjectionIdentity { + config_revision: state.snapshot.revision, + tree_digest: state.snapshot.digest.clone(), + } +} + pub fn detail(state: &WorkspaceConfigState, name: &str) -> Result { let skill = merged_skills(state)? .remove(name) .ok_or_else(|| SkillError::NotFound(name.to_string()))?; - Ok(SkillDetailResponse { + let activation_status = skill.activation_status(); + let projection_status = skill.projection_status(); + let response = SkillDetailResponse { + authority: SKILL_CATALOG_AUTHORITY.to_string(), + projection: projection_identity(state), name: skill.name, description: skill.description, provenance: skill.provenance, overrides: skill.overrides, diagnostics: skill.diagnostics, + activation_status, + projection_status, body: skill.body, allowed_tools: skill.allowed_tools, allowed_tools_status: "experimental_hint_only".to_string(), resources: skill.resources, - }) + }; + response + .validate() + .map_err(|error| SkillError::InvalidProjection(error.to_string()))?; + Ok(response) } pub fn activation( @@ -415,10 +438,28 @@ impl ParsedSkill { .any(|diagnostic| diagnostic.severity == SkillDiagnosticSeverity::Error) } + fn activation_status(&self) -> SkillActivationStatus { + if self.has_errors() { + SkillActivationStatus::Inactive + } else { + SkillActivationStatus::Active + } + } + + fn projection_status(&self) -> SkillProjectionStatus { + if self.has_errors() { + SkillProjectionStatus::Invalid + } else { + SkillProjectionStatus::Valid + } + } + fn catalog_entry(&self) -> SkillCatalogEntry { SkillCatalogEntry { name: self.name.clone(), description: self.description.clone(), + activation_status: self.activation_status(), + projection_status: self.projection_status(), provenance: self.provenance.clone(), overrides: self.overrides.clone(), diagnostics: self.diagnostics.clone(), @@ -513,12 +554,20 @@ mod tests { .unwrap(); assert_eq!(item.provenance.kind, SkillSourceKind::Workspace); assert_eq!(item.provenance.revision, Some(9)); + assert_eq!(catalog.projection.config_revision, 9); + assert_eq!(catalog.projection.tree_digest, state.snapshot.digest); + assert_eq!(item.activation_status, SkillActivationStatus::Active); + assert_eq!(item.projection_status, SkillProjectionStatus::Valid); assert!( item.diagnostics .iter() .all(|diagnostic| diagnostic.severity != SkillDiagnosticSeverity::Error) ); let detail = detail(&state, "debug-rust").unwrap(); + assert_eq!(detail.authority, SKILL_CATALOG_AUTHORITY); + assert_eq!(detail.projection.config_revision, 9); + assert_eq!(detail.activation_status, SkillActivationStatus::Active); + assert_eq!(detail.projection_status, SkillProjectionStatus::Valid); assert_eq!(detail.body, "# Debug Rust\n"); assert_eq!(detail.allowed_tools, vec!["Read", "Grep"]); assert_eq!( @@ -579,6 +628,8 @@ mod tests { .into_iter() .find(|item| item.name == "debug-rust") .unwrap(); + assert_eq!(item.activation_status, SkillActivationStatus::Inactive); + assert_eq!(item.projection_status, SkillProjectionStatus::Invalid); assert!( item.diagnostics .iter() diff --git a/web/workspace/src/lib/generated/skill-api.ts b/web/workspace/src/lib/generated/skill-api.ts new file mode 100644 index 00000000..5847d06e --- /dev/null +++ b/web/workspace/src/lib/generated/skill-api.ts @@ -0,0 +1,87 @@ +// Generated from workspace-api. Do not edit by hand. +// Regenerate: cargo run -q -p workspace-api --features typescript --example generate_skill_api_types > web/workspace/src/lib/generated/skill-api.ts + +export const SKILL_API_AUTHORITY = "workspace-config-skills-v1" as const; + +export const SKILL_API_LIMITS = { + maxSafeInteger: 9007199254740991, + maxCatalogEntries: 500, + maxOverrides: 64, + maxDiagnostics: 100, + maxResources: 500, + maxAllowedTools: 100, + maxNameBytes: 128, + maxLabelBytes: 4096, + maxBodyBytes: 1048576, + maxPathBytes: 1024, + maxDigestBytes: 128, +} as const; + +export type SkillDiagnosticSeverity = "error" | "warning"; + +export type SkillDiagnostic = { + severity: SkillDiagnosticSeverity; + code: string; + message: string; + source?: string; +}; + +export type SkillSourceKind = "builtin" | "workspace"; + +export type SkillProvenance = { + kind: SkillSourceKind; + id: string; + virtual_path?: string; + revision?: number; + source_digest?: string; + tree_digest?: string; +}; + +export type SkillActivationStatus = "active" | "inactive"; + +export type SkillProjectionStatus = "valid" | "invalid"; + +export type SkillProjectionIdentity = { + config_revision: number; + tree_digest: string; +}; + +export type SkillResourceRef = { + kind: string; + name: string; + supported: boolean; + diagnostic?: string; +}; + +export type SkillCatalogEntry = { + name: string; + description: string; + activation_status: SkillActivationStatus; + projection_status: SkillProjectionStatus; + provenance: SkillProvenance; + overrides: Array; + diagnostics: Array; +}; + +export type SkillCatalogResponse = { + authority: string; + projection: SkillProjectionIdentity; + entries: Array; + diagnostics: Array; +}; + +export type SkillDetailResponse = { + authority: string; + projection: SkillProjectionIdentity; + name: string; + description: string; + provenance: SkillProvenance; + overrides: Array; + diagnostics: Array; + activation_status: SkillActivationStatus; + projection_status: SkillProjectionStatus; + body: string; + allowed_tools: Array; + allowed_tools_status: string; + resources: Array; +}; diff --git a/web/workspace/src/lib/workspace/api/http.test.ts b/web/workspace/src/lib/workspace/api/http.test.ts index eefa309f..f7a8419f 100644 --- a/web/workspace/src/lib/workspace/api/http.test.ts +++ b/web/workspace/src/lib/workspace/api/http.test.ts @@ -89,11 +89,24 @@ Deno.test("loadWorkspaceSkillCatalog fetches lightweight catalog", async () => { return Promise.resolve( new Response( JSON.stringify({ - authority: "workspace-backend-skills-v0", + authority: "workspace-config-skills-v1", + projection: { + config_revision: 7, + tree_digest: "tree-digest", + }, entries: [{ name: "triage-errors", description: "Use when triaging errors.", - provenance: { kind: "workspace", id: "workspace:triage-errors" }, + activation_status: "active", + projection_status: "valid", + provenance: { + kind: "workspace", + id: "workspace:triage-errors", + virtual_path: "skills/triage-errors/SKILL.md", + revision: 7, + source_digest: "source-digest", + tree_digest: "tree-digest", + }, overrides: [], diagnostics: [], }], diff --git a/web/workspace/src/lib/workspace/api/http.ts b/web/workspace/src/lib/workspace/api/http.ts index bd5c51b8..c86beb48 100644 --- a/web/workspace/src/lib/workspace/api/http.ts +++ b/web/workspace/src/lib/workspace/api/http.ts @@ -1,58 +1,18 @@ +import type { + SkillCatalogResponse, + SkillDetailResponse, +} from "$lib/generated/skill-api.ts"; +import { + parseSkillCatalogResponse, + parseSkillDetailResponse, +} from "$lib/workspace/skills/api.ts"; + export type ApiResult = { data: T | null; error: string | null; }; -export type SkillDiagnosticSeverity = "error" | "warning"; - -export type SkillDiagnostic = { - severity: SkillDiagnosticSeverity; - code: string; - message: string; - source?: string; -}; - -export type SkillProvenance = { - kind: "builtin" | "workspace"; - id: string; - virtual_path?: string; - revision?: number; - source_digest?: string; - tree_digest?: string; -}; - -export type SkillCatalogEntry = { - name: string; - description: string; - provenance: SkillProvenance; - overrides: SkillProvenance[]; - diagnostics: SkillDiagnostic[]; -}; - -export type SkillCatalogResponse = { - authority: string; - entries: SkillCatalogEntry[]; - diagnostics: SkillDiagnostic[]; -}; - -export type SkillResourceRef = { - kind: string; - name: string; - supported: boolean; - diagnostic?: string; -}; - -export type SkillDetailResponse = { - name: string; - description: string; - provenance: SkillProvenance; - overrides: SkillProvenance[]; - diagnostics: SkillDiagnostic[]; - body: string; - allowed_tools: string[]; - allowed_tools_status: string; - resources: SkillResourceRef[]; -}; +export type { SkillCatalogResponse, SkillDetailResponse }; function normalizePath(path: string): string { if (!path || path === "/") return ""; @@ -95,6 +55,8 @@ export async function loadWorkspaceSkillCatalog( return loadJson( fetchFn, workspaceSkillCatalogPath(workspaceId), + undefined, + parseSkillCatalogResponse, ); } @@ -106,6 +68,8 @@ export async function loadWorkspaceSkillDetail( return loadJson( fetchFn, workspaceSkillDetailPath(workspaceId, name), + undefined, + parseSkillDetailResponse, ); } diff --git a/web/workspace/src/lib/workspace/skills/api.ts b/web/workspace/src/lib/workspace/skills/api.ts new file mode 100644 index 00000000..4f5012c3 --- /dev/null +++ b/web/workspace/src/lib/workspace/skills/api.ts @@ -0,0 +1,455 @@ +import { + SKILL_API_AUTHORITY, + SKILL_API_LIMITS, + type SkillActivationStatus, + type SkillCatalogEntry, + type SkillCatalogResponse, + type SkillDetailResponse, + type SkillDiagnostic, + type SkillDiagnosticSeverity, + type SkillProjectionIdentity, + type SkillProjectionStatus, + type SkillProvenance, + type SkillResourceRef, + type SkillSourceKind, +} from "$lib/generated/skill-api.ts"; + +export class SkillApiContractError extends Error { + constructor(message: string) { + super(message); + this.name = "SkillApiContractError"; + } +} + +export function parseSkillCatalogResponse( + value: unknown, +): SkillCatalogResponse { + const record = strictObject(value, [ + "authority", + "projection", + "entries", + "diagnostics", + ], "Skill catalog response"); + const authority = boundedString( + record.authority, + "Skill catalog authority", + SKILL_API_LIMITS.maxLabelBytes, + false, + ); + if (authority !== SKILL_API_AUTHORITY) { + throw contractError("unsupported Skill catalog authority"); + } + const projection = parseProjection(record.projection); + return { + authority, + projection, + entries: boundedArray( + record.entries, + "Skill catalog entries", + SKILL_API_LIMITS.maxCatalogEntries, + ).map((entry) => parseCatalogEntry(entry, projection)), + diagnostics: parseDiagnostics(record.diagnostics), + }; +} + +export function parseSkillDetailResponse(value: unknown): SkillDetailResponse { + const record = strictObject(value, [ + "authority", + "projection", + "name", + "description", + "provenance", + "overrides", + "diagnostics", + "activation_status", + "projection_status", + "body", + "allowed_tools", + "allowed_tools_status", + "resources", + ], "Skill detail response"); + const authority = boundedString( + record.authority, + "Skill detail authority", + SKILL_API_LIMITS.maxLabelBytes, + false, + ); + if (authority !== SKILL_API_AUTHORITY) { + throw contractError("unsupported Skill detail authority"); + } + const projection = parseProjection(record.projection); + return { + authority, + projection, + name: boundedString( + record.name, + "Skill name", + SKILL_API_LIMITS.maxNameBytes, + false, + ), + description: boundedString( + record.description, + "Skill description", + SKILL_API_LIMITS.maxLabelBytes, + true, + ), + provenance: parseProvenance(record.provenance, projection), + overrides: parseProvenances(record.overrides, projection), + diagnostics: parseDiagnostics(record.diagnostics), + activation_status: activationStatus(record.activation_status), + projection_status: projectionStatus(record.projection_status), + body: boundedString( + record.body, + "Skill body", + SKILL_API_LIMITS.maxBodyBytes, + true, + ), + allowed_tools: boundedArray( + record.allowed_tools, + "Skill allowed tools", + SKILL_API_LIMITS.maxAllowedTools, + ).map((tool) => + boundedString( + tool, + "Skill allowed tool", + SKILL_API_LIMITS.maxLabelBytes, + false, + ) + ), + allowed_tools_status: boundedString( + record.allowed_tools_status, + "Skill allowed-tools status", + SKILL_API_LIMITS.maxLabelBytes, + false, + ), + resources: boundedArray( + record.resources, + "Skill resources", + SKILL_API_LIMITS.maxResources, + ).map(parseResource), + }; +} + +function parseCatalogEntry( + value: unknown, + projection: SkillProjectionIdentity, +): SkillCatalogEntry { + const record = strictObject(value, [ + "name", + "description", + "activation_status", + "projection_status", + "provenance", + "overrides", + "diagnostics", + ], "Skill catalog entry"); + return { + name: boundedString( + record.name, + "Skill name", + SKILL_API_LIMITS.maxNameBytes, + false, + ), + description: boundedString( + record.description, + "Skill description", + SKILL_API_LIMITS.maxLabelBytes, + true, + ), + activation_status: activationStatus(record.activation_status), + projection_status: projectionStatus(record.projection_status), + provenance: parseProvenance(record.provenance, projection), + overrides: parseProvenances(record.overrides, projection), + diagnostics: parseDiagnostics(record.diagnostics), + }; +} + +function parseProjection(value: unknown): SkillProjectionIdentity { + const record = strictObject( + value, + ["config_revision", "tree_digest"], + "Skill projection identity", + ); + return { + config_revision: safeInteger( + record.config_revision, + "Skill config revision", + ), + tree_digest: boundedString( + record.tree_digest, + "Skill tree digest", + SKILL_API_LIMITS.maxDigestBytes, + false, + ), + }; +} + +function parseProvenances( + value: unknown, + projection: SkillProjectionIdentity, +): SkillProvenance[] { + return boundedArray( + value, + "Skill overrides", + SKILL_API_LIMITS.maxOverrides, + ).map((provenance) => parseProvenance(provenance, projection)); +} + +function parseProvenance( + value: unknown, + projection: SkillProjectionIdentity, +): SkillProvenance { + const record = strictObject( + value, + [ + "kind", + "id", + "virtual_path", + "revision", + "source_digest", + "tree_digest", + ], + "Skill provenance", + [ + "virtual_path", + "revision", + "source_digest", + "tree_digest", + ], + ); + const kind = sourceKind(record.kind); + const id = boundedString( + record.id, + "Skill provenance id", + SKILL_API_LIMITS.maxLabelBytes, + false, + ); + const virtualPath = optionalBoundedString( + record.virtual_path, + "Skill virtual path", + SKILL_API_LIMITS.maxPathBytes, + ); + const sourceDigest = optionalBoundedString( + record.source_digest, + "Skill source digest", + SKILL_API_LIMITS.maxDigestBytes, + ); + const treeDigest = optionalBoundedString( + record.tree_digest, + "Skill provenance tree digest", + SKILL_API_LIMITS.maxDigestBytes, + ); + const revision = record.revision === undefined + ? undefined + : safeInteger(record.revision, "Skill provenance revision"); + + if ( + !id.startsWith(`${kind}:`) || virtualPath === undefined || + sourceDigest === undefined || !isVirtualPath(virtualPath) + ) { + throw contractError("invalid Skill provenance"); + } + if (kind === "builtin") { + if (revision !== undefined || treeDigest !== undefined) { + throw contractError("invalid built-in Skill provenance"); + } + } else { + if (revision === undefined || treeDigest === undefined) { + throw contractError("incomplete Workspace Skill provenance"); + } + if ( + revision !== projection.config_revision || + treeDigest !== projection.tree_digest + ) { + throw contractError("stale Workspace Skill projection"); + } + } + + return { + kind, + id, + virtual_path: virtualPath, + revision, + source_digest: sourceDigest, + tree_digest: treeDigest, + }; +} + +function parseDiagnostics(value: unknown): SkillDiagnostic[] { + return boundedArray( + value, + "Skill diagnostics", + SKILL_API_LIMITS.maxDiagnostics, + ).map((diagnostic) => { + const record = strictObject( + diagnostic, + [ + "severity", + "code", + "message", + "source", + ], + "Skill diagnostic", + ["source"], + ); + return { + severity: diagnosticSeverity(record.severity), + code: boundedString( + record.code, + "Skill diagnostic code", + SKILL_API_LIMITS.maxLabelBytes, + false, + ), + message: boundedString( + record.message, + "Skill diagnostic message", + SKILL_API_LIMITS.maxLabelBytes, + false, + ), + source: optionalBoundedString( + record.source, + "Skill diagnostic source", + SKILL_API_LIMITS.maxPathBytes, + ), + }; + }); +} + +function parseResource(value: unknown): SkillResourceRef { + const record = strictObject( + value, + [ + "kind", + "name", + "supported", + "diagnostic", + ], + "Skill resource", + ["diagnostic"], + ); + if (typeof record.supported !== "boolean") { + throw contractError("Skill resource supported must be a boolean"); + } + const name = boundedString( + record.name, + "Skill resource name", + SKILL_API_LIMITS.maxPathBytes, + false, + ); + if (!isVirtualPath(name)) { + throw contractError("invalid Skill resource virtual path"); + } + return { + kind: boundedString( + record.kind, + "Skill resource kind", + SKILL_API_LIMITS.maxLabelBytes, + false, + ), + name, + supported: record.supported, + diagnostic: optionalBoundedString( + record.diagnostic, + "Skill resource diagnostic", + SKILL_API_LIMITS.maxLabelBytes, + ), + }; +} + +function sourceKind(value: unknown): SkillSourceKind { + if (value === "builtin" || value === "workspace") return value; + throw contractError("unsupported Skill provenance kind"); +} + +function diagnosticSeverity(value: unknown): SkillDiagnosticSeverity { + if (value === "error" || value === "warning") return value; + throw contractError("unsupported Skill diagnostic severity"); +} + +function activationStatus(value: unknown): SkillActivationStatus { + if (value === "active" || value === "inactive") return value; + throw contractError("unsupported Skill activation status"); +} + +function projectionStatus(value: unknown): SkillProjectionStatus { + if (value === "valid" || value === "invalid") return value; + throw contractError("unsupported Skill projection status"); +} + +function safeInteger(value: unknown, label: string): number { + if ( + typeof value !== "number" || !Number.isSafeInteger(value) || value < 0 || + value > SKILL_API_LIMITS.maxSafeInteger + ) { + throw contractError(`${label} must be a non-negative safe integer`); + } + return value; +} + +function boundedArray( + value: unknown, + label: string, + limit: number, +): unknown[] { + if (!Array.isArray(value) || value.length > limit) { + throw contractError(`${label} must be a bounded array`); + } + return value; +} + +function optionalBoundedString( + value: unknown, + label: string, + limit: number, +): string | undefined { + return value === undefined + ? undefined + : boundedString(value, label, limit, false); +} + +function boundedString( + value: unknown, + label: string, + limit: number, + allowEmpty: boolean, +): string { + if ( + typeof value !== "string" || (!allowEmpty && value.length === 0) || + new TextEncoder().encode(value).length > limit + ) { + throw contractError(`${label} must be a bounded string`); + } + return value; +} + +function isVirtualPath(value: string): boolean { + return !value.startsWith("/") && !value.includes("\\") && + value.split("/").every((part) => + part !== "" && part !== "." && part !== ".." + ); +} + +function strictObject( + value: unknown, + allowedKeys: readonly string[], + label: string, + optionalKeys: readonly string[] = [], +): Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw contractError(`${label} must be an object`); + } + const record = value as Record; + const allowed = new Set(allowedKeys); + if (Object.keys(record).some((key) => !allowed.has(key))) { + throw contractError(`${label} contains unknown fields`); + } + const optional = new Set(optionalKeys); + if (allowedKeys.some((key) => !optional.has(key) && !(key in record))) { + throw contractError(`${label} is missing required fields`); + } + return record; +} + +function contractError(message: string): SkillApiContractError { + return new SkillApiContractError(message.slice(0, 256)); +} diff --git a/web/workspace/tests/skill-api.test.ts b/web/workspace/tests/skill-api.test.ts new file mode 100644 index 00000000..e8e6a610 --- /dev/null +++ b/web/workspace/tests/skill-api.test.ts @@ -0,0 +1,221 @@ +import { + parseSkillCatalogResponse, + parseSkillDetailResponse, + SkillApiContractError, +} from "../src/lib/workspace/skills/api.ts"; +import { SKILL_API_LIMITS } from "../src/lib/generated/skill-api.ts"; + +declare const Deno: { + test(name: string, fn: () => Promise | void): void; +}; + +function assert(condition: unknown, message: string): asserts condition { + if (!condition) throw new Error(message); +} + +function assertEquals(actual: T, expected: T): void { + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + throw new Error( + `expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`, + ); + } +} + +function assertContractError(value: () => unknown, expected: string): void { + try { + value(); + } catch (error) { + assert( + error instanceof SkillApiContractError, + "expected SkillApiContractError", + ); + assert( + error.message.includes(expected), + `expected bounded diagnostic containing ${expected}, got ${error.message}`, + ); + assert(error.message.length <= 256, "diagnostic must remain bounded"); + return; + } + throw new Error("expected parser to reject malformed Skill response"); +} + +function builtinProvenance() { + return { + kind: "builtin", + id: "builtin:errors", + virtual_path: "skills/errors/SKILL.md", + source_digest: "builtin-source-digest", + }; +} + +function workspaceProvenance() { + return { + kind: "workspace", + id: "workspace:release", + virtual_path: "skills/release/SKILL.md", + revision: 42, + source_digest: "workspace-source-digest", + tree_digest: "tree-digest", + }; +} + +function catalogFixture(): Record { + return { + authority: "workspace-config-skills-v1", + projection: { config_revision: 42, tree_digest: "tree-digest" }, + entries: [{ + name: "errors", + description: "Builtin guidance", + activation_status: "active", + projection_status: "valid", + provenance: builtinProvenance(), + overrides: [], + diagnostics: [], + }, { + name: "release", + description: "Workspace guidance", + activation_status: "inactive", + projection_status: "invalid", + provenance: workspaceProvenance(), + overrides: [builtinProvenance()], + diagnostics: [{ + severity: "error", + code: "invalid_projection", + message: "invalid projected Skill", + source: "workspace:release", + }], + }], + diagnostics: [], + }; +} + +function detailFixture(): Record { + return { + authority: "workspace-config-skills-v1", + projection: { config_revision: 42, tree_digest: "tree-digest" }, + name: "release", + description: "Workspace guidance", + provenance: workspaceProvenance(), + overrides: [], + diagnostics: [], + activation_status: "active", + projection_status: "valid", + body: "# Release\n", + allowed_tools: ["Bash"], + allowed_tools_status: "experimental_hint_only", + resources: [{ + kind: "reference", + name: "skills/release/references/checklist.md", + supported: true, + }], + }; +} + +Deno.test("Skill catalog parser accepts generated builtin, Workspace, and invalid projection shapes", () => { + const parsed = parseSkillCatalogResponse(catalogFixture()); + assertEquals(parsed.entries.length, 2); + assertEquals(parsed.entries[0].provenance.kind, "builtin"); + assertEquals(parsed.entries[1].activation_status, "inactive"); + assertEquals(parsed.entries[1].projection_status, "invalid"); + assertEquals(parsed.projection.config_revision, 42); +}); + +Deno.test("Skill detail parser preserves shared generated DTO fields", () => { + const parsed = parseSkillDetailResponse(detailFixture()); + assertEquals(parsed.name, "release"); + assertEquals(parsed.allowed_tools, ["Bash"]); + assertEquals(parsed.resources[0].supported, true); +}); + +Deno.test("Skill parser rejects stale Workspace projection revision and digest", () => { + const staleRevision = catalogFixture(); + (staleRevision.projection as Record).config_revision = 43; + assertContractError( + () => parseSkillCatalogResponse(staleRevision), + "stale Workspace Skill projection", + ); + + const staleDigest = catalogFixture(); + (staleDigest.projection as Record).tree_digest = "new-tree"; + assertContractError( + () => parseSkillCatalogResponse(staleDigest), + "stale Workspace Skill projection", + ); +}); + +Deno.test("Skill parser fails closed on unknown fields and newer enum values", () => { + const unknownField = catalogFixture(); + unknownField.unexpected = true; + assertContractError( + () => parseSkillCatalogResponse(unknownField), + "unknown fields", + ); + + const newerProvenance = catalogFixture(); + const entries = newerProvenance.entries as Record[]; + (entries[0].provenance as Record).kind = "remote_catalog"; + assertContractError( + () => parseSkillCatalogResponse(newerProvenance), + "unsupported Skill provenance kind", + ); + + const newerStatus = catalogFixture(); + const newerEntries = newerStatus.entries as Record[]; + newerEntries[0].projection_status = "stale"; + assertContractError( + () => parseSkillCatalogResponse(newerStatus), + "unsupported Skill projection status", + ); +}); + +Deno.test("Skill parser rejects unsafe revisions and oversized collections or strings", () => { + const unsafeRevision = catalogFixture(); + (unsafeRevision.projection as Record).config_revision = + Number.MAX_SAFE_INTEGER + 1; + assertContractError( + () => parseSkillCatalogResponse(unsafeRevision), + "safe integer", + ); + + const oversizedCatalog = catalogFixture(); + const firstEntry = (oversizedCatalog.entries as unknown[])[0]; + oversizedCatalog.entries = Array.from( + { length: SKILL_API_LIMITS.maxCatalogEntries + 1 }, + () => firstEntry, + ); + assertContractError( + () => parseSkillCatalogResponse(oversizedCatalog), + "bounded array", + ); + + const oversizedDetail = detailFixture(); + oversizedDetail.body = "x".repeat(SKILL_API_LIMITS.maxBodyBytes + 1); + assertContractError( + () => parseSkillDetailResponse(oversizedDetail), + "bounded string", + ); +}); + +Deno.test("Skill parser diagnostics never include rejected Skill body content", () => { + const secret = "SENSITIVE-SKILL-BODY-CONTENT"; + const malformed = detailFixture(); + malformed.body = secret; + malformed.provenance = { + ...workspaceProvenance(), + kind: "newer_source_kind", + }; + try { + parseSkillDetailResponse(malformed); + throw new Error("expected malformed provenance to fail"); + } catch (error) { + assert( + error instanceof SkillApiContractError, + "expected SkillApiContractError", + ); + assert( + !error.message.includes(secret), + "diagnostic leaked Skill body content", + ); + assert(error.message.length <= 256, "diagnostic must remain bounded"); + } +});