From 9c45a703b7eee5104c9e1e820376fa3f95e657df Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 16:41:30 +0300 Subject: [PATCH 1/7] =?UTF-8?q?feat(client):=20card=20external=20links=20?= =?UTF-8?q?=E2=80=94=20list,=20add,=20update,=20remove=20(#21)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ExternalLink { id, uid, url, description, created, updated }` and the `external_links()` facade over `/cards/{id}/external-links`. Shapes taken from the live API: updates are PATCH with only the given fields (PUT answers 404), `description` is optional and stored as null when absent. The API validates neither the url nor duplicates; the client passes requests through as they are. --- .../kaiten-client/src/api/external_links.rs | 86 ++++++++++++ crates/kaiten-client/src/api/mod.rs | 1 + crates/kaiten-client/src/client.rs | 5 + crates/kaiten-client/src/models.rs | 17 +++ .../tests/external_links_test.rs | 125 ++++++++++++++++++ .../tests/fixtures/external_link_add.json | 9 ++ .../tests/fixtures/external_links_list.json | 24 ++++ 7 files changed, 267 insertions(+) create mode 100644 crates/kaiten-client/src/api/external_links.rs create mode 100644 crates/kaiten-client/tests/external_links_test.rs create mode 100644 crates/kaiten-client/tests/fixtures/external_link_add.json create mode 100644 crates/kaiten-client/tests/fixtures/external_links_list.json diff --git a/crates/kaiten-client/src/api/external_links.rs b/crates/kaiten-client/src/api/external_links.rs new file mode 100644 index 0000000..644785c --- /dev/null +++ b/crates/kaiten-client/src/api/external_links.rs @@ -0,0 +1,86 @@ +use crate::client::KaitenClient; +use crate::error::Result; +use crate::models::ExternalLink; + +/// Card external links facade (`Links (common links)` in Kaiten). Construct +/// via [`KaitenClient::external_links`]. +/// +/// The API itself neither validates `url` nor rejects duplicates — this +/// client does not second-guess it. +pub struct ExternalLinks<'a> { + pub(crate) client: &'a KaitenClient, +} + +impl ExternalLinks<'_> { + /// GET /cards/{card_id}/external-links + pub async fn list(&self, card_id: u64) -> Result> { + self.client + .request( + reqwest::Method::GET, + &format!("/cards/{card_id}/external-links"), + None, + None, + ) + .await + } + + /// POST /cards/{card_id}/external-links — `description` is sent only when given. + pub async fn add( + &self, + card_id: u64, + url: &str, + description: Option<&str>, + ) -> Result { + let mut body = serde_json::json!({ "url": url }); + if let Some(description) = description { + body["description"] = serde_json::Value::String(description.to_owned()); + } + self.client + .request( + reqwest::Method::POST, + &format!("/cards/{card_id}/external-links"), + None, + Some(body), + ) + .await + } + + /// PATCH /cards/{card_id}/external-links/{link_id} — only the given fields + /// are sent (the API answers 404 to PUT). + pub async fn update( + &self, + card_id: u64, + link_id: u64, + url: Option<&str>, + description: Option<&str>, + ) -> Result { + let mut body = serde_json::Map::new(); + if let Some(url) = url { + body.insert("url".into(), serde_json::Value::String(url.to_owned())); + } + if let Some(description) = description { + body.insert( + "description".into(), + serde_json::Value::String(description.to_owned()), + ); + } + self.client + .request( + reqwest::Method::PATCH, + &format!("/cards/{card_id}/external-links/{link_id}"), + None, + Some(serde_json::Value::Object(body)), + ) + .await + } + + /// DELETE /cards/{card_id}/external-links/{link_id} + pub async fn remove(&self, card_id: u64, link_id: u64) -> Result<()> { + self.client + .request_empty( + reqwest::Method::DELETE, + &format!("/cards/{card_id}/external-links/{link_id}"), + ) + .await + } +} diff --git a/crates/kaiten-client/src/api/mod.rs b/crates/kaiten-client/src/api/mod.rs index 802e32a..7c3f37e 100644 --- a/crates/kaiten-client/src/api/mod.rs +++ b/crates/kaiten-client/src/api/mod.rs @@ -2,6 +2,7 @@ pub mod boards; pub mod cards; pub mod checklists; pub mod comments; +pub mod external_links; pub mod files; pub mod links; pub mod members; diff --git a/crates/kaiten-client/src/client.rs b/crates/kaiten-client/src/client.rs index ed525c5..25c6e0a 100644 --- a/crates/kaiten-client/src/client.rs +++ b/crates/kaiten-client/src/client.rs @@ -102,6 +102,11 @@ impl KaitenClient { crate::api::checklists::Checklists { client: self } } + /// Card external links facade. + pub fn external_links(&self) -> crate::api::external_links::ExternalLinks<'_> { + crate::api::external_links::ExternalLinks { client: self } + } + /// Card file attachments facade. pub fn files(&self) -> crate::api::files::Files<'_> { crate::api::files::Files { client: self } diff --git a/crates/kaiten-client/src/models.rs b/crates/kaiten-client/src/models.rs index e236807..bc3296c 100644 --- a/crates/kaiten-client/src/models.rs +++ b/crates/kaiten-client/src/models.rs @@ -517,6 +517,23 @@ pub struct SelectValue { pub sort_order: Option, } +/// An external link of a card (`Links (common links)` in Kaiten): a URL +/// with an optional description. `GET /cards/{id}` embeds these too, under +/// `external_links`. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ExternalLink { + pub id: u64, + #[serde(default)] + pub uid: Option, + pub url: String, + #[serde(default)] + pub description: Option, + #[serde(default)] + pub created: Option, + #[serde(default)] + pub updated: Option, +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/kaiten-client/tests/external_links_test.rs b/crates/kaiten-client/tests/external_links_test.rs new file mode 100644 index 0000000..2b98adc --- /dev/null +++ b/crates/kaiten-client/tests/external_links_test.rs @@ -0,0 +1,125 @@ +//! Card external links (`Links (common links)` in Kaiten), issue #21. Shapes +//! captured live from the API on 2026-08-27. + +use kaiten_client::KaitenClient; +use wiremock::matchers::{body_json, header, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const LIST: &str = include_str!("fixtures/external_links_list.json"); +const ADDED: &str = include_str!("fixtures/external_link_add.json"); + +#[tokio::test] +async fn list_parses_links_with_optional_description() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_raw(LIST, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + let links = client.external_links().list(67_089_469).await.unwrap(); + assert_eq!(links.len(), 2); + assert_eq!(links[0].id, 21_177_131); + assert_eq!(links[0].url, "https://example.com/spike"); + assert_eq!(links[0].description.as_deref(), Some("Source")); + assert_eq!( + links[0].uid.as_deref(), + Some("b4efebe0-0000-0000-0000-000000000000") + ); + assert_eq!( + links[0].created.as_deref(), + Some("2026-08-27T13:24:19.088Z") + ); + assert_eq!(links[1].description, None); +} + +#[tokio::test] +async fn add_posts_url_and_description_and_parses_the_link() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .and(body_json(serde_json::json!({ + "url": "https://example.com/spike", + "description": "Source" + }))) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + let link = client + .external_links() + .add(67_089_469, "https://example.com/spike", Some("Source")) + .await + .unwrap(); + assert_eq!(link.id, 21_177_131); + assert_eq!(link.url, "https://example.com/spike"); +} + +/// Without a description only the url is sent — the API stores `null`. +#[tokio::test] +async fn add_without_description_sends_only_the_url() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/cards/67089469/external-links")) + .and(body_json( + serde_json::json!({ "url": "https://example.net/nodesc" }), + )) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + client + .external_links() + .add(67_089_469, "https://example.net/nodesc", None) + .await + .unwrap(); +} + +/// Updates are PATCH with only the changed fields (PUT answers 404). +#[tokio::test] +async fn update_patches_only_the_given_fields() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(header("Authorization", "Bearer test-token")) + .and(body_json(serde_json::json!({ "description": "patched" }))) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + let link = client + .external_links() + .update(67_089_469, 21_177_131, None, Some("patched")) + .await + .unwrap(); + assert_eq!(link.id, 21_177_131); +} + +#[tokio::test] +async fn remove_deletes_the_card_scoped_link() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/cards/67089469/external-links/21177131")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_string(r#"{"id": 21177131}"#)) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + client + .external_links() + .remove(67_089_469, 21_177_131) + .await + .unwrap(); +} diff --git a/crates/kaiten-client/tests/fixtures/external_link_add.json b/crates/kaiten-client/tests/fixtures/external_link_add.json new file mode 100644 index 0000000..3f2dc01 --- /dev/null +++ b/crates/kaiten-client/tests/fixtures/external_link_add.json @@ -0,0 +1,9 @@ +{ + "created": "2026-08-27T13:24:19.088Z", + "description": "Source", + "external_link_uid": null, + "id": 21177131, + "uid": "b4efebe0-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:24:19.088Z", + "url": "https://example.com/spike" +} diff --git a/crates/kaiten-client/tests/fixtures/external_links_list.json b/crates/kaiten-client/tests/fixtures/external_links_list.json new file mode 100644 index 0000000..c86caf5 --- /dev/null +++ b/crates/kaiten-client/tests/fixtures/external_links_list.json @@ -0,0 +1,24 @@ +[ + { + "card_id": 67089469, + "created": "2026-08-27T13:24:19.088Z", + "description": "Source", + "external_link_id": 21177131, + "external_link_uid": null, + "id": 21177131, + "uid": "b4efebe0-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:25:12.448Z", + "url": "https://example.com/spike" + }, + { + "card_id": 67089469, + "created": "2026-08-27T13:25:20.273Z", + "description": null, + "external_link_id": 21177153, + "external_link_uid": null, + "id": 21177153, + "uid": "6edb3acc-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:25:20.273Z", + "url": "https://example.net/nodesc" + } +] From 7f6c297ce374f6b342add50819d0168681ed6365 Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 16:48:09 +0300 Subject: [PATCH 2/7] feat(cli): card external-link list|add|edit|rm and card view --include (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `card external-link add --url … [--description …]`, `edit --url/--description` (PATCH with only the given fields), `list` (table or --json), `rm`. `--url` must be an absolute http(s) URL without credentials — Kaiten stores anything, the CLI refuses obvious garbage before sending; the value is never echoed in the error. `card view --include external_links,comments` (repeatable or comma-separated) fetches extra sections; the value names match the MCP `get_card` `include` values. `--comments` keeps working but is marked deprecated in --help and prints one warning line on stderr. Plain `card view` output and the `--comments --json` shape are unchanged. --- crates/kaiten/src/cli.rs | 47 +++- crates/kaiten/src/commands/card.rs | 172 +++++++++++-- crates/kaiten/src/main.rs | 1 + crates/kaiten/src/urls.rs | 65 +++++ .../kaiten/tests/card_external_link_test.rs | 234 ++++++++++++++++++ crates/kaiten/tests/card_view_test.rs | 125 ++++++++++ .../tests/fixtures/external_link_add.json | 9 + .../tests/fixtures/external_links_list.json | 24 ++ 8 files changed, 652 insertions(+), 25 deletions(-) create mode 100644 crates/kaiten/src/urls.rs create mode 100644 crates/kaiten/tests/card_external_link_test.rs create mode 100644 crates/kaiten/tests/fixtures/external_link_add.json create mode 100644 crates/kaiten/tests/fixtures/external_links_list.json diff --git a/crates/kaiten/src/cli.rs b/crates/kaiten/src/cli.rs index 10cae54..fb4ba6e 100644 --- a/crates/kaiten/src/cli.rs +++ b/crates/kaiten/src/cli.rs @@ -182,9 +182,13 @@ pub enum CardCmd { /// Show one card (accepts id or browser URL) View { card: String, - /// Also fetch and print comments + /// Deprecated: use `--include comments` #[arg(long)] comments: bool, + /// Extra sections to fetch with the card: external_links, comments + /// (comma-separated or repeated) + #[arg(long, value_delimiter = ',', value_enum)] + include: Vec, }, /// Create a card Create { @@ -287,6 +291,9 @@ pub enum CardCmd { /// Card comments #[command(subcommand)] Comment(CardCommentCmd), + /// Card external links (`Links (common links)` in Kaiten) + #[command(subcommand)] + ExternalLink(CardExternalLinkCmd), /// Card checklists #[command(subcommand)] Checklist(CardChecklistCmd), @@ -383,6 +390,44 @@ pub enum CardCommentCmd { Rm { card: String, comment_id: u64 }, } +#[derive(Subcommand)] +pub enum CardExternalLinkCmd { + /// List external links + List { card: String }, + /// Add an external link + Add { + card: String, + /// Absolute http(s) URL + #[arg(long)] + url: String, + #[arg(long)] + description: Option, + }, + /// Edit an external link (at least one of --url / --description) + Edit { + card: String, + link_id: u64, + /// New absolute http(s) URL + #[arg(long)] + url: Option, + #[arg(long)] + description: Option, + }, + /// Remove an external link + Rm { card: String, link_id: u64 }, +} + +/// Sections `card view --include` fetches in addition to the card. The +/// names match the MCP `get_card` `include` values. +#[derive(Clone, Copy, Debug, PartialEq, Eq, ValueEnum)] +pub enum ViewSection { + /// External links (`Links (common links)`) + #[value(name = "external_links")] + ExternalLinks, + /// Comments + Comments, +} + #[derive(Subcommand)] pub enum CardChecklistCmd { /// List checklists with items diff --git a/crates/kaiten/src/commands/card.rs b/crates/kaiten/src/commands/card.rs index 8ea173c..403967e 100644 --- a/crates/kaiten/src/commands/card.rs +++ b/crates/kaiten/src/commands/card.rs @@ -3,14 +3,15 @@ use std::path::Path; use kaiten_client::{CardFilter, CreateCard, FileRef, KaitenClient, UpdateCard}; use crate::cli::{ - CardChecklistCmd, CardChecklistItemCmd, CardCmd, CardCommentCmd, CardFileCmd, CardMemberCmd, - CardTagCmd, CardTimeCmd, + CardChecklistCmd, CardChecklistItemCmd, CardCmd, CardCommentCmd, CardExternalLinkCmd, + CardFileCmd, CardMemberCmd, CardTagCmd, CardTimeCmd, ViewSection, }; use crate::config::Defaults; use crate::download; use crate::error::CliError; use crate::output; use crate::properties; +use crate::urls; /// Accepts a numeric card id or a browser URL containing `card/`. pub fn parse_card_ref(s: &str) -> Result { @@ -214,7 +215,11 @@ pub async fn run( ) .await } - CardCmd::View { card, comments } => run_view(client, json, &card, comments).await, + CardCmd::View { + card, + comments, + include, + } => run_view(client, json, &card, comments, &include).await, CardCmd::Create { title, board, @@ -300,6 +305,7 @@ pub async fn run( CardCmd::Time(cmd) => run_time(client, json, cmd).await, CardCmd::Member(cmd) => run_member(client, json, cmd).await, CardCmd::Comment(cmd) => run_comment(client, json, cmd).await, + CardCmd::ExternalLink(cmd) => run_external_link(client, json, cmd).await, CardCmd::Checklist(cmd) => run_checklist(client, json, cmd).await, CardCmd::Tag(cmd) => run_tag(client, json, cmd).await, CardCmd::File(cmd) => run_file(client, json, cmd).await, @@ -405,38 +411,75 @@ async fn run_view( client: &KaitenClient, json: bool, card: &str, - comments: bool, + comments_flag: bool, + include: &[ViewSection], ) -> Result<(), CliError> { + if comments_flag { + eprintln!("warning: --comments is deprecated, use --include comments"); + } + let with_links = include.contains(&ViewSection::ExternalLinks); + let with_comments = comments_flag || include.contains(&ViewSection::Comments); let card_id = parse_card_ref(card)?; let card = client.cards().get(card_id).await?; + let links = if with_links { + Some(client.external_links().list(card_id).await?) + } else { + None + }; + let comments = if with_comments { + Some(client.comments().list(card_id).await?) + } else { + None + }; if json { - if comments { - let list = client.comments().list(card_id).await?; - return output::print_json(&serde_json::json!({ - "card": card, - "comments": list, - })); + if links.is_none() && comments.is_none() { + return output::print_json(&card); } - return output::print_json(&card); + let mut doc = serde_json::Map::new(); + doc.insert("card".into(), serde_json::json!(card)); + if let Some(links) = &links { + doc.insert("external_links".into(), serde_json::json!(links)); + } + if let Some(comments) = &comments { + doc.insert("comments".into(), serde_json::json!(comments)); + } + return output::print_json(&doc); } print_card_details(&card); - if comments { - let list = client.comments().list(card_id).await?; - println!(); - println!("Comments:"); - for comment in &list { - let author = comment - .author - .as_ref() - .map_or_else(|| "-".into(), output::user_label); - let date = date_cell(comment.created.as_deref()); - println!("{date} {author}:"); - println!("{}", comment.text); - } + if let Some(links) = &links { + print_links_section(links); + } + if let Some(comments) = &comments { + print_comments_section(comments); } Ok(()) } +fn print_links_section(links: &[kaiten_client::ExternalLink]) { + println!(); + println!("Links:"); + for link in links { + match link.description.as_deref().filter(|d| !d.is_empty()) { + Some(description) => println!(" {} {} - {description}", link.id, link.url), + None => println!(" {} {}", link.id, link.url), + } + } +} + +fn print_comments_section(comments: &[kaiten_client::Comment]) { + println!(); + println!("Comments:"); + for comment in comments { + let author = comment + .author + .as_ref() + .map_or_else(|| "-".into(), output::user_label); + let date = date_cell(comment.created.as_deref()); + println!("{date} {author}:"); + println!("{}", comment.text); + } +} + struct CardCreateArgs { title: String, board: Option, @@ -673,6 +716,87 @@ async fn run_comment( } } +async fn run_external_link( + client: &KaitenClient, + json: bool, + cmd: CardExternalLinkCmd, +) -> Result<(), CliError> { + match cmd { + CardExternalLinkCmd::Add { + card, + url, + description, + } => { + let card_id = parse_card_ref(&card)?; + let url = link_url(&url)?; + let link = client + .external_links() + .add(card_id, &url, description.as_deref()) + .await?; + if json { + return output::print_json(&link); + } + println!("{}", link.id); + Ok(()) + } + CardExternalLinkCmd::List { card } => { + let card_id = parse_card_ref(&card)?; + let links = client.external_links().list(card_id).await?; + if json { + return output::print_json(&links); + } + let mut table = output::table(&["ID", "URL", "DESCRIPTION", "CREATED"]); + for link in &links { + table.add_row(vec![ + link.id.to_string(), + truncate_text(&link.url, 70), + truncate_text(link.description.as_deref().unwrap_or("-"), 40), + date_cell(link.created.as_deref()), + ]); + } + println!("{table}"); + Ok(()) + } + CardExternalLinkCmd::Edit { + card, + link_id, + url, + description, + } => { + if url.is_none() && description.is_none() { + return Err(CliError::InvalidArg( + "nothing to change: pass --url and/or --description".into(), + )); + } + let card_id = parse_card_ref(&card)?; + let url = url.as_deref().map(link_url).transpose()?; + let link = client + .external_links() + .update(card_id, link_id, url.as_deref(), description.as_deref()) + .await?; + if json { + return output::print_json(&link); + } + println!("updated external link {} on card {card_id}", link.id); + Ok(()) + } + CardExternalLinkCmd::Rm { card, link_id } => { + let card_id = parse_card_ref(&card)?; + client.external_links().remove(card_id, link_id).await?; + if json { + return output::print_json(&serde_json::json!({ "removed": true })); + } + println!("removed external link {link_id} from card {card_id}"); + Ok(()) + } + } +} + +/// `--url` must be an absolute http(s) URL; the value is never echoed. +fn link_url(raw: &str) -> Result { + urls::absolute_http_url(raw).map_err(|e| CliError::InvalidArg(format!("--url {e}"))) +} + async fn run_checklist( client: &KaitenClient, json: bool, diff --git a/crates/kaiten/src/main.rs b/crates/kaiten/src/main.rs index 118daba..3849932 100644 --- a/crates/kaiten/src/main.rs +++ b/crates/kaiten/src/main.rs @@ -6,6 +6,7 @@ mod error; mod mcp; mod output; mod properties; +mod urls; use std::process::ExitCode; diff --git a/crates/kaiten/src/urls.rs b/crates/kaiten/src/urls.rs new file mode 100644 index 0000000..ea85fee --- /dev/null +++ b/crates/kaiten/src/urls.rs @@ -0,0 +1,65 @@ +//! Syntax check for user-supplied links. Kaiten stores any string as an +//! external link url, so the CLI and the MCP server refuse obvious garbage +//! before sending it. + +/// Trims `raw` and checks that it is an absolute `http(s)` URL without +/// embedded credentials. Returns the trimmed text as typed (no +/// normalization) so the API stores what the user wrote. The error never +/// echoes the value — it may contain a secret. +pub(crate) fn absolute_http_url(raw: &str) -> Result { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err("is empty".into()); + } + let parsed = + url::Url::parse(trimmed).map_err(|_| String::from("is not an absolute http(s) URL"))?; + if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + return Err("is not an absolute http(s) URL".into()); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err("must not contain credentials".into()); + } + Ok(trimmed.to_owned()) +} + +#[cfg(test)] +mod tests { + use super::absolute_http_url; + + #[test] + fn accepts_http_and_https_and_trims() { + assert_eq!( + absolute_http_url(" https://example.com/a?b=1#c ").unwrap(), + "https://example.com/a?b=1#c" + ); + assert_eq!( + absolute_http_url("http://host:8080/x").unwrap(), + "http://host:8080/x" + ); + } + + /// `http:///x` is deliberately absent: the WHATWG parser reads it as + /// `http://x/`, which is a valid link. + #[test] + fn rejects_garbage_without_echoing_it() { + assert_eq!(absolute_http_url("").unwrap_err(), "is empty"); + assert_eq!(absolute_http_url(" ").unwrap_err(), "is empty"); + for bad in [ + "notaurl", + "example.com/x", + "ftp://host/x", + "mailto:a@b", + "http:", + ] { + let err = absolute_http_url(bad).unwrap_err(); + assert_eq!(err, "is not an absolute http(s) URL", "{bad:?}"); + } + } + + #[test] + fn rejects_credentials_without_echoing_them() { + let err = absolute_http_url("https://user:s3cret@host/x").unwrap_err(); + assert_eq!(err, "must not contain credentials"); + assert!(!err.contains("s3cret")); + } +} diff --git a/crates/kaiten/tests/card_external_link_test.rs b/crates/kaiten/tests/card_external_link_test.rs new file mode 100644 index 0000000..c6fb360 --- /dev/null +++ b/crates/kaiten/tests/card_external_link_test.rs @@ -0,0 +1,234 @@ +//! `kaiten card external-link list|add|edit|rm` (issue #21). + +use assert_cmd::Command; +use predicates::prelude::*; +use wiremock::matchers::{body_json, header, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const LIST: &str = include_str!("fixtures/external_links_list.json"); +const ADDED: &str = include_str!("fixtures/external_link_add.json"); + +fn kaiten(base_url: &str, config_dir: &std::path::Path) -> Command { + let mut cmd = Command::cargo_bin("kaiten").unwrap(); + cmd.env_remove("KAITEN_DOMAIN") + .env("KAITEN_BASE_URL", base_url) + .env("KAITEN_TOKEN", "test-token") + .env("KAITEN_CONFIG_DIR", config_dir) + .env("NO_COLOR", "1"); + cmd +} + +#[tokio::test(flavor = "multi_thread")] +async fn list_renders_table() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_raw(LIST, "application/json")) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args(["card", "external-link", "list", "67089469"]) + .assert() + .success() + .stdout(predicate::str::contains("21177131")) + .stdout(predicate::str::contains("https://example.com/spike")) + .stdout(predicate::str::contains("Source")) + .stdout(predicate::str::contains("https://example.net/nodesc")) + .stdout(predicate::str::contains("2026-08-27")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn list_json_prints_models() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .respond_with(ResponseTemplate::new(200).set_body_raw(LIST, "application/json")) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + let out = kaiten(&server.uri(), tmp.path()) + .args(["--json", "card", "external-link", "list", "67089469"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let value: serde_json::Value = serde_json::from_slice(&out).unwrap(); + assert_eq!(value.as_array().unwrap().len(), 2); + assert_eq!(value[0]["url"], "https://example.com/spike"); + assert!(value[1]["description"].is_null()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn add_posts_url_and_description_and_prints_id() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .and(body_json(serde_json::json!({ + "url": "https://example.com/spike", + "description": "Source" + }))) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args([ + "card", + "external-link", + "add", + "67089469", + "--url", + "https://example.com/spike", + "--description", + "Source", + ]) + .assert() + .success() + .stdout(predicate::str::contains("21177131")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn add_json_prints_model() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/cards/67089469/external-links")) + .and(body_json( + serde_json::json!({ "url": "https://example.com/spike" }), + )) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + let out = kaiten(&server.uri(), tmp.path()) + .args([ + "--json", + "card", + "external-link", + "add", + "67089469", + "--url", + "https://example.com/spike", + ]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let value: serde_json::Value = serde_json::from_slice(&out).unwrap(); + assert_eq!(value["id"], 21_177_131); +} + +/// The API stores anything as a url; the CLI refuses garbage before sending. +#[tokio::test(flavor = "multi_thread")] +async fn add_rejects_a_non_http_url_without_any_request() { + let server = MockServer::start().await; // no mocks + let tmp = tempfile::tempdir().unwrap(); + + for bad in ["notaurl", "ftp://host/x", "https://user:secret@host/x", ""] { + kaiten(&server.uri(), tmp.path()) + .args(["card", "external-link", "add", "67089469", "--url", bad]) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("--url")) + .stderr(predicate::str::contains("secret").not()); + } + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn edit_patches_only_the_given_fields() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(body_json( + serde_json::json!({ "description": "Updated source" }), + )) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args([ + "card", + "external-link", + "edit", + "67089469", + "21177131", + "--description", + "Updated source", + ]) + .assert() + .success() + .stdout(predicate::str::contains( + "updated external link 21177131 on card 67089469", + )); +} + +#[tokio::test(flavor = "multi_thread")] +async fn edit_without_changes_is_an_error_without_any_request() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args(["card", "external-link", "edit", "67089469", "21177131"]) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("--url")) + .stderr(predicate::str::contains("--description")); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn rm_deletes_and_reports() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/cards/67089469/external-links/21177131")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_string(r#"{"id": 21177131}"#)) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args(["card", "external-link", "rm", "67089469", "21177131"]) + .assert() + .success() + .stdout(predicate::str::contains( + "removed external link 21177131 from card 67089469", + )); +} + +/// Kaiten answers 403 for a link id that is not on the card. +#[tokio::test(flavor = "multi_thread")] +async fn rm_unknown_link_surfaces_the_api_error() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/cards/67089469/external-links/1")) + .respond_with(ResponseTemplate::new(403).set_body_string("Forbidden")) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args(["card", "external-link", "rm", "67089469", "1"]) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("API error 403")); +} diff --git a/crates/kaiten/tests/card_view_test.rs b/crates/kaiten/tests/card_view_test.rs index bbe4776..fcefcea 100644 --- a/crates/kaiten/tests/card_view_test.rs +++ b/crates/kaiten/tests/card_view_test.rs @@ -139,3 +139,128 @@ async fn card_view_garbage_ref_fails() { .code(1) .stderr(predicate::str::contains("invalid card reference")); } + +// --- issue #21: `--include` sections and the deprecated `--comments` --- + +const EXTERNAL_LINKS: &str = include_str!("fixtures/external_links_list.json"); + +async fn mock_external_links(server: &MockServer, expect: u64) { + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_raw(EXTERNAL_LINKS, "application/json")) + .expect(expect) + .mount(server) + .await; +} + +async fn mock_comments(server: &MockServer, expect: u64) { + Mock::given(method("GET")) + .and(path("/cards/67089469/comments")) + .respond_with(ResponseTemplate::new(200).set_body_raw(COMMENTS, "application/json")) + .expect(expect) + .mount(server) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn card_view_include_external_links_makes_second_request_and_prints_section() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_external_links(&server, 1).await; + mock_comments(&server, 0).await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args(["card", "view", "67089469", "--include", "external_links"]) + .assert() + .success() + .stdout(predicate::str::contains("Links:")) + .stdout(predicate::str::contains("21177131")) + .stdout(predicate::str::contains("https://example.com/spike")) + .stdout(predicate::str::contains("Source")) + .stdout(predicate::str::contains("Comments:").not()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn card_view_include_both_sections_comma_separated() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_external_links(&server, 1).await; + mock_comments(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args([ + "card", + "view", + "67089469", + "--include", + "external_links,comments", + ]) + .assert() + .success() + .stdout(predicate::str::contains("Links:")) + .stdout(predicate::str::contains("Comments:")) + .stderr(predicate::str::contains("deprecated").not()); +} + +/// `--comments` keeps working but says what to use instead — on stderr, so +/// stdout consumers are unaffected. +#[tokio::test(flavor = "multi_thread")] +async fn card_view_comments_flag_is_deprecated_but_still_works() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_comments(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args(["card", "view", "67089469", "--comments"]) + .assert() + .success() + .stdout(predicate::str::contains("Comments:")) + .stderr(predicate::str::contains("--comments is deprecated")) + .stderr(predicate::str::contains("--include comments")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn card_view_json_include_external_links_adds_the_key() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_external_links(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + let out = kaiten(tmp.path(), &server.uri()) + .args([ + "--json", + "card", + "view", + "67089469", + "--include", + "external_links", + ]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let value: serde_json::Value = serde_json::from_slice(&out).unwrap(); + assert_eq!(value["card"]["id"], 67_089_469, "{value}"); + assert_eq!(value["external_links"][0]["id"], 21_177_131, "{value}"); + assert!(value.get("comments").is_none(), "{value}"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn card_view_include_rejects_an_unknown_section() { + let server = MockServer::start().await; // nothing may be requested + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args(["card", "view", "67089469", "--include", "attachments"]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("external_links")) + .stderr(predicate::str::contains("comments")); + assert!(server.received_requests().await.unwrap().is_empty()); +} diff --git a/crates/kaiten/tests/fixtures/external_link_add.json b/crates/kaiten/tests/fixtures/external_link_add.json new file mode 100644 index 0000000..3f2dc01 --- /dev/null +++ b/crates/kaiten/tests/fixtures/external_link_add.json @@ -0,0 +1,9 @@ +{ + "created": "2026-08-27T13:24:19.088Z", + "description": "Source", + "external_link_uid": null, + "id": 21177131, + "uid": "b4efebe0-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:24:19.088Z", + "url": "https://example.com/spike" +} diff --git a/crates/kaiten/tests/fixtures/external_links_list.json b/crates/kaiten/tests/fixtures/external_links_list.json new file mode 100644 index 0000000..c86caf5 --- /dev/null +++ b/crates/kaiten/tests/fixtures/external_links_list.json @@ -0,0 +1,24 @@ +[ + { + "card_id": 67089469, + "created": "2026-08-27T13:24:19.088Z", + "description": "Source", + "external_link_id": 21177131, + "external_link_uid": null, + "id": 21177131, + "uid": "b4efebe0-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:25:12.448Z", + "url": "https://example.com/spike" + }, + { + "card_id": 67089469, + "created": "2026-08-27T13:25:20.273Z", + "description": null, + "external_link_id": 21177153, + "external_link_uid": null, + "id": 21177153, + "uid": "6edb3acc-0000-0000-0000-000000000000", + "updated": "2026-08-27T13:25:20.273Z", + "url": "https://example.net/nodesc" + } +] From bc57d2e802861cb517ab667367cc7099c29b3fa2 Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 16:54:05 +0300 Subject: [PATCH 3/7] feat(mcp): external link tools and get_card include (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tools — list_card_external_links, add_card_external_link, update_card_external_link, remove_card_external_link — over the card external links resource; `url` is checked to be an absolute http(s) URL before any request (Kaiten stores anything) and never echoed in the error. `get_card` gains an optional `include: ["external_links", "comments"]` (names shared with the CLI `card view --include`): each section is one extra request and one extra key in the result; without `include` the tool stays a single request with the same shape as before. 36 → 40 tools; the stdio contract test covers `include` and rejects an unknown section before any request. --- crates/kaiten/src/mcp/mod.rs | 435 +++++++++++++++++++++++++- crates/kaiten/src/mcp/projections.rs | 37 ++- crates/kaiten/tests/mcp_stdio_test.rs | 61 +++- 3 files changed, 524 insertions(+), 9 deletions(-) diff --git a/crates/kaiten/src/mcp/mod.rs b/crates/kaiten/src/mcp/mod.rs index 2124e39..a77f286 100644 --- a/crates/kaiten/src/mcp/mod.rs +++ b/crates/kaiten/src/mcp/mod.rs @@ -12,9 +12,10 @@ use rmcp::{ErrorData as McpError, ServerHandler, tool, tool_handler, tool_router use crate::download; use crate::error::CliError; use crate::properties; +use crate::urls; use projections::{ CardDetail, CardSummary, ChecklistItemView, ChecklistView, CommentResult, CommentView, - MemberView, MutationResult, TimeLogView, UserView, + ExternalLinkView, MemberView, MutationResult, TimeLogView, UserView, }; #[derive(Clone)] @@ -60,6 +61,12 @@ fn coerce_properties( /// `try_api!` for this crate's own argument validation: the `Err` is already /// a user-facing message, returned as a tool-level error before any API call. +/// `url` of an external link must be an absolute http(s) URL; the value is +/// never echoed (it may hold a secret). +fn link_url(raw: &str) -> Result { + urls::absolute_http_url(raw).map_err(|e| format!("url {e}")) +} + macro_rules! try_args { ($e:expr) => { match $e { @@ -168,6 +175,20 @@ pub struct ListCardsParams { pub struct GetCardParams { /// Card id pub card_id: u64, + /// Extra sections to fetch with the card, one more request each: + /// "external_links" (Links (common links)), "comments". Omit for the + /// plain card. + #[serde(default)] + pub include: Option>, +} + +/// Sections `get_card` can fetch in addition to the card. The names match +/// the CLI `card view --include` values. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Deserialize, schemars::JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum IncludeSection { + ExternalLinks, + Comments, } #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] @@ -423,6 +444,46 @@ pub struct RemoveCommentParams { pub comment_id: u64, } +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct ListCardExternalLinksParams { + /// Card id + pub card_id: u64, +} + +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct AddCardExternalLinkParams { + /// Card id + pub card_id: u64, + /// Absolute http(s) URL + pub url: String, + /// Optional description shown next to the link + pub description: Option, +} + +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct UpdateCardExternalLinkParams { + /// Card id + pub card_id: u64, + /// External link id (see list_card_external_links) + pub link_id: u64, + /// New absolute http(s) URL + pub url: Option, + /// New description + pub description: Option, +} + +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct RemoveCardExternalLinkParams { + /// Card id + pub card_id: u64, + /// External link id (see list_card_external_links) + pub link_id: u64, +} + #[derive(Debug, serde::Deserialize, schemars::JsonSchema)] #[serde(deny_unknown_fields)] pub struct SetCardResponsibleParams { @@ -616,14 +677,24 @@ impl KaitenMcp { } #[tool( - description = "Get a full card by id: description, members, tags, checklists, custom properties, linked cards (children/parents), blockers and attached files. For the raw API JSON use the CLI (kaiten card view --json)." + description = "Get a full card by id: description, members, tags, checklists, custom properties, linked cards (children/parents), blockers and attached files. For the raw API JSON use the CLI (kaiten card view --json). Pass include: [\"external_links\", \"comments\"] to fetch those sections too (one extra request each)." )] async fn get_card( &self, Parameters(p): Parameters, ) -> Result { let card = try_api!(self.client.cards().get(p.card_id).await); - json_result(&CardDetail::from(&card)) + let mut detail = CardDetail::from(&card); + let include = p.include.unwrap_or_default(); + if include.contains(&IncludeSection::ExternalLinks) { + let links = try_api!(self.client.external_links().list(p.card_id).await); + detail.external_links = Some(links.iter().map(ExternalLinkView::from).collect()); + } + if include.contains(&IncludeSection::Comments) { + let comments = try_api!(self.client.comments().list(p.card_id).await); + detail.comments = Some(comments.iter().map(CommentView::from).collect()); + } + json_result(&detail) } #[tool(description = "List all comments of a card.")] @@ -1072,6 +1143,77 @@ impl KaitenMcp { json_result(&serde_json::json!({ "removed": true, "comment_id": p.comment_id })) } + #[tool(description = "List the external links (Links (common links)) of a card.")] + async fn list_card_external_links( + &self, + Parameters(p): Parameters, + ) -> Result { + let links = try_api!(self.client.external_links().list(p.card_id).await); + let views: Vec = links.iter().map(ExternalLinkView::from).collect(); + json_result(&views) + } + + #[tool( + description = "Add an external link to a card. `url` must be an absolute http(s) URL. Kaiten does not reject duplicates — check list_card_external_links first when that matters." + )] + async fn add_card_external_link( + &self, + Parameters(p): Parameters, + ) -> Result { + let url = try_args!(link_url(&p.url)); + let link = try_api!( + self.client + .external_links() + .add(p.card_id, &url, p.description.as_deref()) + .await + ); + json_result(&ExternalLinkView::from(&link)) + } + + #[tool( + description = "Change the url and/or description of an external link on a card (at least one of them)." + )] + async fn update_card_external_link( + &self, + Parameters(p): Parameters, + ) -> Result { + if p.url.is_none() && p.description.is_none() { + return Ok(CallToolResult::error(vec![ContentBlock::text( + "nothing to change: pass url and/or description", + )])); + } + let url = match p.url.as_deref() { + Some(raw) => Some(try_args!(link_url(raw))), + None => None, + }; + let link = try_api!( + self.client + .external_links() + .update( + p.card_id, + p.link_id, + url.as_deref(), + p.description.as_deref() + ) + .await + ); + json_result(&ExternalLinkView::from(&link)) + } + + #[tool(description = "Remove an external link from a card.")] + async fn remove_card_external_link( + &self, + Parameters(p): Parameters, + ) -> Result { + try_api!( + self.client + .external_links() + .remove(p.card_id, p.link_id) + .await + ); + json_result(&serde_json::json!({ "removed": true, "link_id": p.link_id })) + } + #[tool( description = "Make a card member the responsible person (or demote with responsible=false). The user must already be a member." )] @@ -1291,7 +1433,11 @@ mod tests { use wiremock::matchers::{body_json, header, method, path, query_param}; use wiremock::{Mock, MockServer, ResponseTemplate}; - use super::{CreateCardParams, GetCardParams, KaitenMcp, ListCardsParams}; + use super::{ + AddCardExternalLinkParams, CreateCardParams, GetCardParams, IncludeSection, KaitenMcp, + ListCardExternalLinksParams, ListCardsParams, RemoveCardExternalLinkParams, + UpdateCardExternalLinkParams, + }; const SPACES_FIXTURE: &str = include_str!("../../tests/fixtures/mcp_spaces.json"); const CARD_CREATE_FIXTURE: &str = include_str!("../../tests/fixtures/mcp_card_create.json"); @@ -1342,7 +1488,10 @@ mod tests { let mcp = mcp_for(&server); let result = mcp - .get_card(Parameters(GetCardParams { card_id: 999 })) + .get_card(Parameters(GetCardParams { + card_id: 999, + include: None, + })) .await .unwrap(); assert_eq!(result.is_error, Some(true)); @@ -1367,7 +1516,10 @@ mod tests { let mcp = mcp_for(&server); let result = mcp - .get_card(Parameters(GetCardParams { card_id: 999 })) + .get_card(Parameters(GetCardParams { + card_id: 999, + include: None, + })) .await .unwrap(); assert_eq!(result.is_error, Some(true)); @@ -1511,6 +1663,7 @@ mod tests { let result = mcp .get_card(Parameters(GetCardParams { card_id: 67_089_469, + include: None, })) .await .unwrap(); @@ -1998,7 +2151,7 @@ mod tests { } #[test] - fn registers_exactly_36_tools_with_spec_names() { + fn registers_exactly_40_tools_with_spec_names() { let tools = KaitenMcp::tool_router().list_all(); let mut names: Vec = tools.iter().map(|t| t.name.to_string()).collect(); names.sort(); @@ -2039,6 +2192,10 @@ mod tests { "add_time_log", "list_time_logs", "download_file", + "list_card_external_links", + "add_card_external_link", + "update_card_external_link", + "remove_card_external_link", ]; expected.sort_unstable(); assert_eq!(names, expected); @@ -2418,4 +2575,268 @@ mod tests { ); } } + + // --- issue #21: external links --------------------------------------- + + const EXTERNAL_LINKS_FIXTURE: &str = + include_str!("../../tests/fixtures/external_links_list.json"); + const EXTERNAL_LINK_ADD_FIXTURE: &str = + include_str!("../../tests/fixtures/external_link_add.json"); + + async fn mount_external_links(server: &MockServer, expect: u64) { + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .respond_with( + ResponseTemplate::new(200).set_body_raw(EXTERNAL_LINKS_FIXTURE, "application/json"), + ) + .expect(expect) + .mount(server) + .await; + } + + #[tokio::test] + async fn list_card_external_links_returns_compact_views() { + let server = MockServer::start().await; + mount_external_links(&server, 1).await; + let mcp = mcp_for(&server); + + let result = mcp + .list_card_external_links(Parameters(ListCardExternalLinksParams { + card_id: 67_089_469, + })) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true)); + let links: serde_json::Value = serde_json::from_str(&tool_text(&result)).unwrap(); + assert_eq!(links.as_array().unwrap().len(), 2); + assert_eq!(links[0]["id"], 21_177_131); + assert_eq!(links[0]["url"], "https://example.com/spike"); + assert_eq!(links[0]["description"], "Source"); + assert_eq!(links[0]["created"], "2026-08-27T13:24:19.088Z"); + assert!(links[1].get("description").is_none(), "{links}"); + for noisy in ["uid", "card_id", "external_link_id", "external_link_uid"] { + assert!(links[0].get(noisy).is_none(), "{noisy} leaked: {links}"); + } + } + + #[tokio::test] + async fn add_card_external_link_posts_and_returns_the_view() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .and(wiremock::matchers::body_json(serde_json::json!({ + "url": "https://example.com/spike", + "description": "Source" + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_raw(EXTERNAL_LINK_ADD_FIXTURE, "application/json"), + ) + .expect(1) + .mount(&server) + .await; + let mcp = mcp_for(&server); + + let result = mcp + .add_card_external_link(Parameters(AddCardExternalLinkParams { + card_id: 67_089_469, + url: "https://example.com/spike".into(), + description: Some("Source".into()), + })) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true), "{}", tool_text(&result)); + let link: serde_json::Value = serde_json::from_str(&tool_text(&result)).unwrap(); + assert_eq!(link["id"], 21_177_131); + assert_eq!(link["url"], "https://example.com/spike"); + } + + /// Kaiten stores any string as a url; the tool refuses garbage before + /// any request and never echoes the value (it may hold a secret). + #[tokio::test] + async fn add_card_external_link_rejects_a_bad_url_before_any_request() { + let server = MockServer::start().await; + let mcp = mcp_for(&server); + + for bad in ["notaurl", "ftp://host/x", "https://user:s3cret@host/x", " "] { + let result = mcp + .add_card_external_link(Parameters(AddCardExternalLinkParams { + card_id: 67_089_469, + url: bad.into(), + description: None, + })) + .await + .unwrap(); + assert_eq!(result.is_error, Some(true), "{bad:?}"); + let text = tool_text(&result); + assert!(text.starts_with("url "), "{text}"); + assert!(!text.contains("s3cret"), "{text}"); + } + assert!(server.received_requests().await.unwrap().is_empty()); + } + + #[tokio::test] + async fn update_card_external_link_patches_only_the_given_fields() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(wiremock::matchers::body_json(serde_json::json!({ + "description": "patched" + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_raw(EXTERNAL_LINK_ADD_FIXTURE, "application/json"), + ) + .expect(1) + .mount(&server) + .await; + let mcp = mcp_for(&server); + + let result = mcp + .update_card_external_link(Parameters(UpdateCardExternalLinkParams { + card_id: 67_089_469, + link_id: 21_177_131, + url: None, + description: Some("patched".into()), + })) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true), "{}", tool_text(&result)); + let link: serde_json::Value = serde_json::from_str(&tool_text(&result)).unwrap(); + assert_eq!(link["id"], 21_177_131); + } + + #[tokio::test] + async fn update_card_external_link_without_changes_is_a_tool_error() { + let server = MockServer::start().await; + let mcp = mcp_for(&server); + + let result = mcp + .update_card_external_link(Parameters(UpdateCardExternalLinkParams { + card_id: 67_089_469, + link_id: 21_177_131, + url: None, + description: None, + })) + .await + .unwrap(); + assert_eq!(result.is_error, Some(true)); + let text = tool_text(&result); + assert!( + text.contains("url") && text.contains("description"), + "{text}" + ); + assert!(server.received_requests().await.unwrap().is_empty()); + } + + #[tokio::test] + async fn remove_card_external_link_reports_the_link_id() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/cards/67089469/external-links/21177131")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_string(r#"{"id": 21177131}"#)) + .expect(1) + .mount(&server) + .await; + let mcp = mcp_for(&server); + + let result = mcp + .remove_card_external_link(Parameters(RemoveCardExternalLinkParams { + card_id: 67_089_469, + link_id: 21_177_131, + })) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true), "{}", tool_text(&result)); + let value: serde_json::Value = serde_json::from_str(&tool_text(&result)).unwrap(); + assert_eq!( + value, + serde_json::json!({ "removed": true, "link_id": 21_177_131 }) + ); + } + + /// Without `include` get_card stays a single request and its shape is + /// untouched; `include` opts into extra requests and extra keys. + #[tokio::test] + async fn get_card_include_external_links_and_comments_makes_extra_requests() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/cards/67089469")) + .respond_with( + ResponseTemplate::new(200).set_body_raw(CARD_FULL_FIXTURE, "application/json"), + ) + .expect(3) + .mount(&server) + .await; + mount_external_links(&server, 2).await; + Mock::given(method("GET")) + .and(path("/cards/67089469/comments")) + .respond_with(ResponseTemplate::new(200).set_body_raw( + r#"[{"id": 5, "text": "hi", "created": "2026-08-27T10:00:00.000Z"}]"#, + "application/json", + )) + .expect(1) + .mount(&server) + .await; + let mcp = mcp_for(&server); + + let plain = mcp + .get_card(Parameters(GetCardParams { + card_id: 67_089_469, + include: None, + })) + .await + .unwrap(); + let plain: serde_json::Value = serde_json::from_str(&tool_text(&plain)).unwrap(); + assert!(plain.get("external_links").is_none(), "{plain}"); + assert!(plain.get("comments").is_none(), "{plain}"); + + let links_only = mcp + .get_card(Parameters(GetCardParams { + card_id: 67_089_469, + include: Some(vec![IncludeSection::ExternalLinks]), + })) + .await + .unwrap(); + let links_only: serde_json::Value = serde_json::from_str(&tool_text(&links_only)).unwrap(); + assert_eq!( + links_only["external_links"][0]["url"], + "https://example.com/spike" + ); + assert!(links_only.get("comments").is_none(), "{links_only}"); + + let both = mcp + .get_card(Parameters(GetCardParams { + card_id: 67_089_469, + include: Some(vec![ + IncludeSection::ExternalLinks, + IncludeSection::Comments, + ]), + })) + .await + .unwrap(); + let both: serde_json::Value = serde_json::from_str(&tool_text(&both)).unwrap(); + assert_eq!(both["external_links"].as_array().unwrap().len(), 2); + assert_eq!(both["comments"][0]["text"], "hi"); + assert_eq!(both["id"], 67_089_469, "{both}"); + } + + #[test] + fn get_card_include_schema_lists_the_section_names() { + let tools = KaitenMcp::tool_router().list_all(); + let get_card = tools.iter().find(|t| t.name == "get_card").unwrap(); + let schema = serde_json::to_value(&get_card.input_schema).unwrap(); + let text = schema.to_string(); + assert!(text.contains("external_links"), "{schema}"); + assert!(text.contains("\"comments\""), "{schema}"); + assert!( + schema["required"] + .as_array() + .is_none_or(|r| !r.iter().any(|v| v == "include")), + "include must be optional: {schema}" + ); + } } diff --git a/crates/kaiten/src/mcp/projections.rs b/crates/kaiten/src/mcp/projections.rs index cc6aad1..f45f943 100644 --- a/crates/kaiten/src/mcp/projections.rs +++ b/crates/kaiten/src/mcp/projections.rs @@ -6,7 +6,8 @@ //! the CLI (`kaiten card view --json`, `kaiten api`). use kaiten_client::{ - Blocker, Card, CardFile, CardMember, Checklist, ChecklistItem, Comment, TimeLog, User, + Blocker, Card, CardFile, CardMember, Checklist, ChecklistItem, Comment, ExternalLink, TimeLog, + User, }; #[allow(clippy::trivially_copy_pass_by_ref)] // signature dictated by serde @@ -276,6 +277,12 @@ pub struct CardDetail { pub blockers: Vec, #[serde(skip_serializing_if = "Vec::is_empty")] pub files: Vec, + /// Only with `get_card` `include: ["external_links"]` (a second request). + #[serde(skip_serializing_if = "Option::is_none")] + pub external_links: Option>, + /// Only with `get_card` `include: ["comments"]` (a second request). + #[serde(skip_serializing_if = "Option::is_none")] + pub comments: Option>, } impl From<&Card> for CardDetail { @@ -297,6 +304,8 @@ impl From<&Card> for CardDetail { parents: card.parents.iter().map(LinkedCardView::from).collect(), blockers: card.blockers.iter().map(BlockerView::from).collect(), files: card.files.iter().map(FileView::from).collect(), + external_links: None, + comments: None, } } } @@ -381,6 +390,32 @@ impl From<&Comment> for CommentView { } } +/// An external link of a card, without the ids Kaiten repeats +/// (`uid`, `card_id`, `external_link_id`, `external_link_uid`). +#[derive(Debug, serde::Serialize)] +pub struct ExternalLinkView { + pub id: u64, + pub url: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub description: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub created: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub updated: Option, +} + +impl From<&ExternalLink> for ExternalLinkView { + fn from(link: &ExternalLink) -> Self { + Self { + id: link.id, + url: link.url.clone(), + description: link.description.clone(), + created: link.created.clone(), + updated: link.updated.clone(), + } + } +} + #[derive(Debug, serde::Serialize)] pub struct TimeLogView { pub id: u64, diff --git a/crates/kaiten/tests/mcp_stdio_test.rs b/crates/kaiten/tests/mcp_stdio_test.rs index cdc6948..1904167 100644 --- a/crates/kaiten/tests/mcp_stdio_test.rs +++ b/crates/kaiten/tests/mcp_stdio_test.rs @@ -17,7 +17,7 @@ const USER_CURRENT: &str = include_str!("fixtures/mcp_user_current.json"); const READ_TIMEOUT: Duration = Duration::from_secs(20); -const EXPECTED_TOOLS: [&str; 36] = [ +const EXPECTED_TOOLS: [&str; 40] = [ "current_user", "list_spaces", "list_boards", @@ -54,6 +54,10 @@ const EXPECTED_TOOLS: [&str; 36] = [ "add_time_log", "list_time_logs", "download_file", + "list_card_external_links", + "add_card_external_link", + "update_card_external_link", + "remove_card_external_link", ]; struct McpProc { @@ -534,3 +538,58 @@ async fn mcp_stdio_download_file_saves_locally() { assert_eq!(std::fs::read_to_string(path).unwrap(), "attachment body"); assert_eq!(descriptor["size"], 15); } + +/// `get_card` with `include` keeps the legacy wire shape and adds the +/// requested section; an unknown section is a parameter error before any +/// request, listing the accepted names. +#[tokio::test(flavor = "multi_thread")] +async fn mcp_stdio_get_card_include_external_links_keeps_legacy_shape() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/cards/67089469")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_raw( + include_str!("fixtures/mcp_card_full.json"), + "application/json", + )) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/cards/67089469/external-links")) + .and(header("Authorization", "Bearer test-token")) + .respond_with(ResponseTemplate::new(200).set_body_raw( + include_str!("fixtures/external_links_list.json"), + "application/json", + )) + .expect(1) + .mount(&server) + .await; + + let mut mcp = McpProc::spawn_with_base_url(&server.uri()); + mcp.initialize("2025-03-26"); + let card = mcp.call_tool( + 3, + "get_card", + &serde_json::json!({ "card_id": 67_089_469, "include": ["external_links"] }), + ); + assert!(card.get("error").is_none(), "{card}"); + assert_ne!(card["result"]["isError"], serde_json::json!(true), "{card}"); + assert_legacy_shape(&card["result"], "tools/call (get_card include)"); + let detail: serde_json::Value = serde_json::from_str(tool_text(&card)).unwrap(); + assert_eq!(detail["id"], 67_089_469, "{detail}"); + assert_eq!(detail["external_links"][0]["id"], 21_177_131, "{detail}"); + + let bad = mcp.call_tool( + 4, + "get_card", + &serde_json::json!({ "card_id": 67_089_469, "include": ["attachments"] }), + ); + assert!(bad.get("error").is_none(), "{bad}"); + assert_eq!(bad["result"]["isError"], serde_json::json!(true), "{bad}"); + let text = tool_text(&bad); + assert!( + text.contains("external_links") && text.contains("comments"), + "{text}" + ); +} From 2f59229581f5d22a3f4de22a18aa01c3886b6bd1 Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 16:54:06 +0300 Subject: [PATCH 4/7] docs(readme): external links, card view --include, 40 MCP tools; --comments deprecated --- README.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e53d581..69ee3ef 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Command-line client and MCP server for the [Kaiten](https://kaiten.ru) tracker, in the spirit of `gh` / `glab`. - Browse spaces, boards and cards from the terminal -- Create, edit, move and archive cards; manage members, tags, comments and checklists +- Create, edit, move and archive cards; manage members, tags, comments, checklists and external links - `--json` output on every command for scripting - Built-in MCP server (`kaiten mcp serve`) so coding agents can work with the tracker - Raw API escape hatch: `kaiten api GET /users/current` @@ -88,7 +88,7 @@ kaiten board view 456 # columns and lanes (ids for `card move kaiten card list --mine kaiten card list --board 456 --query "deploy" --limit 20 -kaiten card view 67089469 --comments # a full card URL works too +kaiten card view 67089469 --include external_links,comments # a full card URL works too kaiten card create --board 456 --title "Fix the flaky test" --description "..." kaiten card edit 67089469 --title "New title" --asap true kaiten card move 67089469 --column 6308511 @@ -97,6 +97,8 @@ kaiten card archive 67089469 kaiten card member add 67089469 user@example.com # user id or email kaiten card member responsible 67089469 user@example.com kaiten card comment add 67089469 --body "Done, please review" +kaiten card external-link add 67089469 --url https://example.com/spec --description "Spec" +kaiten card external-link list 67089469 # also edit / rm kaiten card checklist add 67089469 --name "Release steps" kaiten card checklist item add 67089469 91011 --text "Bump version" kaiten card checklist item check 67089469 91011 121314 @@ -118,6 +120,10 @@ kaiten api POST /cards --data '{"board_id":456,"title":"Raw"}' Add `--json` to any command to print the raw JSON of the API response. +`card view --include external_links,comments` fetches extra sections with the card +(one request each; the names match the MCP `get_card` `include` values). +`card view --comments` still works but is deprecated — use `--include comments`. + ## Shell completion ```sh @@ -133,9 +139,11 @@ kaiten completion fish > ~/.config/fish/completions/kaiten.fish ## MCP server -The same binary is an MCP server (stdio transport, 36 tools mirroring the CLI, +The same binary is an MCP server (stdio transport, 40 tools mirroring the CLI, including compact card projections and a cursor-based `poll_updates` for -event-like agent workflows). +event-like agent workflows). `get_card` takes an optional +`include: ["external_links", "comments"]` to return those sections in the same +call (one extra request each). Claude Code: @@ -183,7 +191,7 @@ by area (✅ covered, ◐ partial, — not covered): | Users list (id lookup) | ✅ | ✅ | | Card links: children / parents / blockers | ✅ `card link/unlink/unblock` | ✅ `link_cards` etc. | | Files: attach / detach / list / download | ✅ (uploads get a PUBLIC url!) | ✅ (`download_file` saves locally) | -| External links | — | — | +| External links (Links (common links)): list / add / edit / remove | ✅ `card external-link`, `card view --include external_links` | ✅ four tools + `get_card` `include` | | Custom properties: reference + set values | ✅ `property list/values`, `--properties-json` | ✅ two tools + `properties` (a JSON object — a wrong shape is rejected before the API call; mutations echo the resulting `properties`) | | Time logs | ✅ `card time add/list` | ✅ | | Events: polling for changes | — | ✅ `poll_updates` (cursor-based) | From b18cc5ef5a72fb2c2cfe27d340cfc554d93fb98d Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 17:12:55 +0300 Subject: [PATCH 5/7] fix(cli,mcp): validate the link url text, not only its parsed form; review follow-ups (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WHATWG parser strips control characters and repairs `http:x`, `http:/x` and `http:///x` into `http://x/`, while Kaiten stores the raw text — so the url check now requires an `http(s)://` prefix followed by a host on the trimmed text and no control characters, before the parsed host/credentials checks. Also from review: `card view` prints the section as `External links:`; `--description` gets help text; `try_args!` keeps its doc comment (the url helper moved below the macro); the server instructions mention external links; `Card` doc says why `include` costs a request; tests pin `edit --url` at every layer, the `{card, comments}` JSON shape of the deprecated `--comments`, single fetch for `--comments --include comments`, the repeated `--include` form and the exact `IncludeSection` schema. --- .../kaiten-client/src/api/external_links.rs | 3 +- crates/kaiten-client/src/models.rs | 6 +- .../tests/external_links_test.rs | 53 ++++++++++++ crates/kaiten/src/cli.rs | 2 + crates/kaiten/src/commands/card.rs | 10 ++- crates/kaiten/src/mcp/mod.rs | 82 ++++++++++++++++--- crates/kaiten/src/urls.rs | 41 ++++++++-- .../kaiten/tests/card_external_link_test.rs | 56 +++++++++++++ crates/kaiten/tests/card_view_test.rs | 81 +++++++++++++++++- 9 files changed, 309 insertions(+), 25 deletions(-) diff --git a/crates/kaiten-client/src/api/external_links.rs b/crates/kaiten-client/src/api/external_links.rs index 644785c..65b137b 100644 --- a/crates/kaiten-client/src/api/external_links.rs +++ b/crates/kaiten-client/src/api/external_links.rs @@ -46,7 +46,8 @@ impl ExternalLinks<'_> { } /// PATCH /cards/{card_id}/external-links/{link_id} — only the given fields - /// are sent (the API answers 404 to PUT). + /// are sent (the API answers 404 to PUT). Pass at least one of them: with + /// both `None` the request body is `{}`. pub async fn update( &self, card_id: u64, diff --git a/crates/kaiten-client/src/models.rs b/crates/kaiten-client/src/models.rs index bc3296c..bb2cdc4 100644 --- a/crates/kaiten-client/src/models.rs +++ b/crates/kaiten-client/src/models.rs @@ -518,8 +518,10 @@ pub struct SelectValue { } /// An external link of a card (`Links (common links)` in Kaiten): a URL -/// with an optional description. `GET /cards/{id}` embeds these too, under -/// `external_links`. +/// with an optional description. `GET /cards/{id}` embeds them under +/// `external_links` as well, but [`Card`] does not model that field yet +/// (adding one would be a breaking change), so they are read through +/// [`crate::api::external_links::ExternalLinks::list`]. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ExternalLink { pub id: u64, diff --git a/crates/kaiten-client/tests/external_links_test.rs b/crates/kaiten-client/tests/external_links_test.rs index 2b98adc..e5223b3 100644 --- a/crates/kaiten-client/tests/external_links_test.rs +++ b/crates/kaiten-client/tests/external_links_test.rs @@ -123,3 +123,56 @@ async fn remove_deletes_the_card_scoped_link() { .await .unwrap(); } + +#[tokio::test] +async fn update_sends_url_and_description_when_both_are_given() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(body_json(serde_json::json!({ + "url": "https://example.org/moved", + "description": "moved" + }))) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + client + .external_links() + .update( + 67_089_469, + 21_177_131, + Some("https://example.org/moved"), + Some("moved"), + ) + .await + .unwrap(); +} + +#[tokio::test] +async fn update_with_url_only_sends_only_the_url() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(body_json( + serde_json::json!({ "url": "https://example.org/moved" }), + )) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + + let client = KaitenClient::new(&server.uri(), "test-token").unwrap(); + client + .external_links() + .update( + 67_089_469, + 21_177_131, + Some("https://example.org/moved"), + None, + ) + .await + .unwrap(); +} diff --git a/crates/kaiten/src/cli.rs b/crates/kaiten/src/cli.rs index fb4ba6e..1dc0750 100644 --- a/crates/kaiten/src/cli.rs +++ b/crates/kaiten/src/cli.rs @@ -400,6 +400,7 @@ pub enum CardExternalLinkCmd { /// Absolute http(s) URL #[arg(long)] url: String, + /// Optional description shown next to the link #[arg(long)] description: Option, }, @@ -410,6 +411,7 @@ pub enum CardExternalLinkCmd { /// New absolute http(s) URL #[arg(long)] url: Option, + /// New description #[arg(long)] description: Option, }, diff --git a/crates/kaiten/src/commands/card.rs b/crates/kaiten/src/commands/card.rs index 403967e..9d33071 100644 --- a/crates/kaiten/src/commands/card.rs +++ b/crates/kaiten/src/commands/card.rs @@ -457,7 +457,7 @@ async fn run_view( fn print_links_section(links: &[kaiten_client::ExternalLink]) { println!(); - println!("Links:"); + println!("External links:"); for link in links { match link.description.as_deref().filter(|d| !d.is_empty()) { Some(description) => println!(" {} {} - {description}", link.id, link.url), @@ -750,7 +750,13 @@ async fn run_external_link( table.add_row(vec![ link.id.to_string(), truncate_text(&link.url, 70), - truncate_text(link.description.as_deref().unwrap_or("-"), 40), + truncate_text( + link.description + .as_deref() + .filter(|d| !d.is_empty()) + .unwrap_or("-"), + 40, + ), date_cell(link.created.as_deref()), ]); } diff --git a/crates/kaiten/src/mcp/mod.rs b/crates/kaiten/src/mcp/mod.rs index a77f286..75b2459 100644 --- a/crates/kaiten/src/mcp/mod.rs +++ b/crates/kaiten/src/mcp/mod.rs @@ -61,12 +61,6 @@ fn coerce_properties( /// `try_api!` for this crate's own argument validation: the `Err` is already /// a user-facing message, returned as a tool-level error before any API call. -/// `url` of an external link must be an absolute http(s) URL; the value is -/// never echoed (it may hold a secret). -fn link_url(raw: &str) -> Result { - urls::absolute_http_url(raw).map_err(|e| format!("url {e}")) -} - macro_rules! try_args { ($e:expr) => { match $e { @@ -76,6 +70,12 @@ macro_rules! try_args { }; } +/// `url` of an external link must be an absolute http(s) URL; the value is +/// never echoed (it may hold a secret). +fn link_url(raw: &str) -> Result { + urls::absolute_http_url(raw).map_err(|e| format!("url {e}")) +} + /// Unwrap a `Result` produced by a client call inside a tool /// method, returning early with `Ok(error_result(e))` on failure (see /// `error_result` for why this is `Ok`, not `Err`, at the tool boundary). @@ -178,7 +178,6 @@ pub struct GetCardParams { /// Extra sections to fetch with the card, one more request each: /// "external_links" (Links (common links)), "comments". Omit for the /// plain card. - #[serde(default)] pub include: Option>, } @@ -1398,7 +1397,7 @@ impl ServerHandler for KaitenMcp { let mut info = ServerInfo::default(); info.instructions = Some( "Kaiten tracker tools: browse spaces, boards and cards, create and edit \ - cards, manage members, comments and checklists. Start with list_spaces \ + cards, manage members, comments, checklists and external links. Start with list_spaces \ to discover structure, or list_cards with mine=true to see the current \ user's cards." .into(), @@ -2626,7 +2625,7 @@ mod tests { Mock::given(method("POST")) .and(path("/cards/67089469/external-links")) .and(header("Authorization", "Bearer test-token")) - .and(wiremock::matchers::body_json(serde_json::json!({ + .and(body_json(serde_json::json!({ "url": "https://example.com/spike", "description": "Source" }))) @@ -2682,7 +2681,7 @@ mod tests { let server = MockServer::start().await; Mock::given(method("PATCH")) .and(path("/cards/67089469/external-links/21177131")) - .and(wiremock::matchers::body_json(serde_json::json!({ + .and(body_json(serde_json::json!({ "description": "patched" }))) .respond_with( @@ -2708,6 +2707,57 @@ mod tests { assert_eq!(link["id"], 21_177_131); } + #[tokio::test] + async fn update_card_external_link_sends_both_fields_when_given() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(body_json(serde_json::json!({ + "url": "https://example.org/moved", + "description": "moved" + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_raw(EXTERNAL_LINK_ADD_FIXTURE, "application/json"), + ) + .expect(1) + .mount(&server) + .await; + let mcp = mcp_for(&server); + + let result = mcp + .update_card_external_link(Parameters(UpdateCardExternalLinkParams { + card_id: 67_089_469, + link_id: 21_177_131, + url: Some("https://example.org/moved".into()), + description: Some("moved".into()), + })) + .await + .unwrap(); + assert_ne!(result.is_error, Some(true), "{}", tool_text(&result)); + } + + #[tokio::test] + async fn update_card_external_link_rejects_a_bad_url_before_any_request() { + let server = MockServer::start().await; + let mcp = mcp_for(&server); + + let result = mcp + .update_card_external_link(Parameters(UpdateCardExternalLinkParams { + card_id: 67_089_469, + link_id: 21_177_131, + url: Some("https://user:s3cret@host/x".into()), + description: Some("not sent".into()), + })) + .await + .unwrap(); + assert_eq!(result.is_error, Some(true)); + let text = tool_text(&result); + assert!(text.starts_with("url "), "{text}"); + assert!(!text.contains("s3cret"), "{text}"); + assert!(server.received_requests().await.unwrap().is_empty()); + } + #[tokio::test] async fn update_card_external_link_without_changes_is_a_tool_error() { let server = MockServer::start().await; @@ -2829,9 +2879,15 @@ mod tests { let tools = KaitenMcp::tool_router().list_all(); let get_card = tools.iter().find(|t| t.name == "get_card").unwrap(); let schema = serde_json::to_value(&get_card.input_schema).unwrap(); - let text = schema.to_string(); - assert!(text.contains("external_links"), "{schema}"); - assert!(text.contains("\"comments\""), "{schema}"); + assert_eq!( + schema["properties"]["include"]["items"]["$ref"], "#/$defs/IncludeSection", + "{schema}" + ); + assert_eq!( + schema["$defs"]["IncludeSection"]["enum"], + serde_json::json!(["external_links", "comments"]), + "{schema}" + ); assert!( schema["required"] .as_array() diff --git a/crates/kaiten/src/urls.rs b/crates/kaiten/src/urls.rs index ea85fee..c94ec62 100644 --- a/crates/kaiten/src/urls.rs +++ b/crates/kaiten/src/urls.rs @@ -11,9 +11,25 @@ pub(crate) fn absolute_http_url(raw: &str) -> Result { if trimmed.is_empty() { return Err("is empty".into()); } + // The WHATWG parser strips control characters and repairs `http:x`, + // `http:/x` and `http:///x` into `http://x/`, but Kaiten stores the raw + // text — so the text itself is checked, not only its parsed form: an + // `http(s)://` prefix, a host right after it, no control characters. + let after_scheme = ["http://", "https://"].iter().find_map(|prefix| { + trimmed + .get(..prefix.len()) + .filter(|head| head.eq_ignore_ascii_case(prefix)) + .map(|_| &trimmed[prefix.len()..]) + }); + let Some(authority) = after_scheme else { + return Err("is not an absolute http(s) URL".into()); + }; + if authority.starts_with(['/', '\\']) || trimmed.chars().any(char::is_control) { + return Err("is not an absolute http(s) URL".into()); + } let parsed = url::Url::parse(trimmed).map_err(|_| String::from("is not an absolute http(s) URL"))?; - if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + if parsed.host_str().is_none() { return Err("is not an absolute http(s) URL".into()); } if !parsed.username().is_empty() || parsed.password().is_some() { @@ -26,6 +42,8 @@ pub(crate) fn absolute_http_url(raw: &str) -> Result { mod tests { use super::absolute_http_url; + /// Surrounding whitespace (including a trailing newline from a shell) + /// is trimmed; only interior control characters are refused. #[test] fn accepts_http_and_https_and_trims() { assert_eq!( @@ -36,10 +54,12 @@ mod tests { absolute_http_url("http://host:8080/x").unwrap(), "http://host:8080/x" ); + assert_eq!( + absolute_http_url("HTTPS://Example.com/x\r\n").unwrap(), + "HTTPS://Example.com/x" + ); } - /// `http:///x` is deliberately absent: the WHATWG parser reads it as - /// `http://x/`, which is a valid link. #[test] fn rejects_garbage_without_echoing_it() { assert_eq!(absolute_http_url("").unwrap_err(), "is empty"); @@ -50,9 +70,20 @@ mod tests { "ftp://host/x", "mailto:a@b", "http:", + // the WHATWG parser would "repair" these, but Kaiten stores the raw text + "http:x", + "http:/x", + "http:///x", + "http://\\host/x", + "HTTP://ho\rst/x", + "https://exam\nple.com/x", + "https://exa\tmple.com/x", + "https://host/x\0", ] { - let err = absolute_http_url(bad).unwrap_err(); - assert_eq!(err, "is not an absolute http(s) URL", "{bad:?}"); + match absolute_http_url(bad) { + Ok(accepted) => panic!("{bad:?} was accepted as {accepted:?}"), + Err(err) => assert_eq!(err, "is not an absolute http(s) URL", "{bad:?}"), + } } } diff --git a/crates/kaiten/tests/card_external_link_test.rs b/crates/kaiten/tests/card_external_link_test.rs index c6fb360..7237819 100644 --- a/crates/kaiten/tests/card_external_link_test.rs +++ b/crates/kaiten/tests/card_external_link_test.rs @@ -47,6 +47,7 @@ async fn list_json_prints_models() { Mock::given(method("GET")) .and(path("/cards/67089469/external-links")) .respond_with(ResponseTemplate::new(200).set_body_raw(LIST, "application/json")) + .expect(1) .mount(&server) .await; let tmp = tempfile::tempdir().unwrap(); @@ -232,3 +233,58 @@ async fn rm_unknown_link_surfaces_the_api_error() { .code(1) .stderr(predicate::str::contains("API error 403")); } + +#[tokio::test(flavor = "multi_thread")] +async fn edit_url_patches_the_url() { + let server = MockServer::start().await; + Mock::given(method("PATCH")) + .and(path("/cards/67089469/external-links/21177131")) + .and(body_json( + serde_json::json!({ "url": "https://example.org/moved" }), + )) + .respond_with(ResponseTemplate::new(200).set_body_raw(ADDED, "application/json")) + .expect(1) + .mount(&server) + .await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args([ + "card", + "external-link", + "edit", + "67089469", + "21177131", + "--url", + "https://example.org/moved", + ]) + .assert() + .success() + .stdout(predicate::str::contains("updated external link 21177131")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn edit_rejects_a_bad_url_without_any_request() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(&server.uri(), tmp.path()) + .args([ + "card", + "external-link", + "edit", + "67089469", + "21177131", + "--url", + "notaurl", + "--description", + "still not sent", + ]) + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains( + "--url is not an absolute http(s) URL", + )); + assert!(server.received_requests().await.unwrap().is_empty()); +} diff --git a/crates/kaiten/tests/card_view_test.rs b/crates/kaiten/tests/card_view_test.rs index fcefcea..a96e48e 100644 --- a/crates/kaiten/tests/card_view_test.rs +++ b/crates/kaiten/tests/card_view_test.rs @@ -175,7 +175,7 @@ async fn card_view_include_external_links_makes_second_request_and_prints_sectio .args(["card", "view", "67089469", "--include", "external_links"]) .assert() .success() - .stdout(predicate::str::contains("Links:")) + .stdout(predicate::str::contains("External links:")) .stdout(predicate::str::contains("21177131")) .stdout(predicate::str::contains("https://example.com/spike")) .stdout(predicate::str::contains("Source")) @@ -200,7 +200,7 @@ async fn card_view_include_both_sections_comma_separated() { ]) .assert() .success() - .stdout(predicate::str::contains("Links:")) + .stdout(predicate::str::contains("External links:")) .stdout(predicate::str::contains("Comments:")) .stderr(predicate::str::contains("deprecated").not()); } @@ -219,10 +219,87 @@ async fn card_view_comments_flag_is_deprecated_but_still_works() { .assert() .success() .stdout(predicate::str::contains("Comments:")) + .stdout(predicate::str::contains("deprecated").not()) .stderr(predicate::str::contains("--comments is deprecated")) .stderr(predicate::str::contains("--include comments")); } +/// The pre-`--include` JSON contract: `--comments --json` prints exactly +/// `{card, comments}` and nothing else on stdout. +#[tokio::test(flavor = "multi_thread")] +async fn card_view_comments_json_shape_is_unchanged() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_comments(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + let out = kaiten(tmp.path(), &server.uri()) + .args(["--json", "card", "view", "67089469", "--comments"]) + .assert() + .success() + .stderr(predicate::str::contains("--comments is deprecated")) + .get_output() + .stdout + .clone(); + let value: serde_json::Value = serde_json::from_slice(&out).unwrap(); + let mut keys: Vec<&str> = value + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + keys.sort_unstable(); + assert_eq!(keys, ["card", "comments"], "{value}"); + assert_eq!(value["card"]["id"], 67_089_469); + assert!(value["comments"].is_array()); +} + +/// `--comments` together with `--include comments` fetches comments once. +#[tokio::test(flavor = "multi_thread")] +async fn card_view_comments_flag_and_include_comments_fetch_once() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_comments(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args([ + "card", + "view", + "67089469", + "--comments", + "--include", + "comments", + ]) + .assert() + .success() + .stdout(predicate::str::contains("Comments:")); +} + +#[tokio::test(flavor = "multi_thread")] +async fn card_view_include_can_be_repeated() { + let server = MockServer::start().await; + mock_card(&server).await; + mock_external_links(&server, 1).await; + mock_comments(&server, 1).await; + let tmp = tempfile::tempdir().unwrap(); + + kaiten(tmp.path(), &server.uri()) + .args([ + "card", + "view", + "67089469", + "--include", + "external_links", + "--include", + "comments", + ]) + .assert() + .success() + .stdout(predicate::str::contains("External links:")) + .stdout(predicate::str::contains("Comments:")); +} + #[tokio::test(flavor = "multi_thread")] async fn card_view_json_include_external_links_adds_the_key() { let server = MockServer::start().await; From 6dc81eeaa0bc7a03c29072251be152cb9a0afc0d Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 17:12:55 +0300 Subject: [PATCH 6/7] ci: line coverage with cargo-llvm-cov, Codecov badge in the README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `coverage` job on ubuntu runs the workspace tests instrumented (cargo-llvm-cov also counts the `kaiten` binary the integration tests spawn — 84% lines on master today) and uploads lcov to Codecov. The upload token lives in the CODECOV_TOKEN Actions and Dependabot secrets; PRs from forks have neither, so an upload failure fails the job only for pushes and same-repo PRs. --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++++ README.md | 1 + 2 files changed, 25 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d12f0d..31d8525 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,30 @@ jobs: - uses: Swatinem/rust-cache@v2 - run: cargo test --workspace + # Line coverage of the whole workspace test suite, including the `kaiten` + # binary the integration tests spawn (cargo-llvm-cov instruments child + # processes too). Uploaded to Codecov for the README badge and the PR + # coverage comment. CODECOV_TOKEN is an Actions secret and a Dependabot + # secret; PRs from forks have neither, so an upload failure only fails the + # job for pushes and same-repo PRs. + coverage: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + with: + components: llvm-tools-preview + - uses: taiki-e/install-action@cargo-llvm-cov + - uses: Swatinem/rust-cache@v2 + - run: cargo llvm-cov --workspace --lcov --output-path lcov.info + - uses: codecov/codecov-action@v5 + with: + files: lcov.info + token: ${{ secrets.CODECOV_TOKEN }} + fail_ci_if_error: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }} + # Tests use unix-only permission APIs; on Windows we verify the build only. build-windows: runs-on: windows-latest diff --git a/README.md b/README.md index 69ee3ef..960a23d 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ [![CI](https://github.com/dsociative/kaiten-cli/actions/workflows/ci.yml/badge.svg)](https://github.com/dsociative/kaiten-cli/actions/workflows/ci.yml) [![Security](https://github.com/dsociative/kaiten-cli/actions/workflows/security.yml/badge.svg)](https://github.com/dsociative/kaiten-cli/actions/workflows/security.yml) [![CodeQL](https://github.com/dsociative/kaiten-cli/actions/workflows/codeql.yml/badge.svg)](https://github.com/dsociative/kaiten-cli/actions/workflows/codeql.yml) +[![Coverage](https://codecov.io/gh/dsociative/kaiten-cli/graph/badge.svg)](https://codecov.io/gh/dsociative/kaiten-cli) [![Release](https://img.shields.io/github/v/release/dsociative/kaiten-cli)](https://github.com/dsociative/kaiten-cli/releases) [![Crates.io](https://img.shields.io/crates/v/kaiten-cli.svg)](https://crates.io/crates/kaiten-cli) [![Downloads](https://img.shields.io/crates/d/kaiten-cli.svg)](https://crates.io/crates/kaiten-cli) From 6372a8bdd85508e70b38e9aa927abf772178c51f Mon Sep 17 00:00:00 2001 From: dsociative Date: Thu, 27 Aug 2026 17:33:10 +0300 Subject: [PATCH 7/7] =?UTF-8?q?ci(codeql):=20exclude=20rust/cleartext-logg?= =?UTF-8?q?ing=20=E2=80=94=20the=20CLI=20prints=20user=20names=20on=20stdo?= =?UTF-8?q?ut=20by=20design?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The query models `println!` as a log file and any identifier matching user(name|id)/uid/account as sensitive, so every `card view`, `card member` and `auth status` line naming a user is an alert (8 open, all false positives). The only real log sinks are the `tracing` calls, which print host, path and status only. The rest of the default suite stays on. --- .github/codeql/codeql-config.yml | 11 +++++++++++ .github/workflows/codeql.yml | 1 + 2 files changed, 12 insertions(+) create mode 100644 .github/codeql/codeql-config.yml diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 0000000..1c9018a --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,11 @@ +name: kaiten-cli CodeQL config + +# `rust/cleartext-logging` treats `println!` as a log file and any identifier +# matching user(name|id) / uid / account as sensitive, so every `card view`, +# `card member` and `auth status` line that names a user is an alert — printing +# them on stdout is what a CLI does. The only real log sinks here are the +# `tracing` calls, which print host, path and status only. The query is +# therefore excluded; everything else in the default suite stays on. +query-filters: + - exclude: + id: rust/cleartext-logging diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 3aa2dea..d17caa8 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -18,6 +18,7 @@ jobs: - uses: actions/checkout@v7 - uses: github/codeql-action/init@v4 with: + config-file: ./.github/codeql/codeql-config.yml languages: rust build-mode: none - uses: github/codeql-action/analyze@v4