diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 62e5eae6..c4a4f416 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -156,19 +156,25 @@ jobs: while IFS= read -r line; do sha256=$(echo "$line" | awk '{print $1}') filename=$(echo "$line" | awk '{print $2}') - # Strip -{version}-wasm32-wasip2.tar.gz to get the extension name. - # Use '.*' (greedy) so pre-release suffixes like -alpha.1 are consumed too. - name=$(echo "$filename" | sed 's/-[0-9].*-wasm32-wasip2\.tar\.gz$//') + # Skip non-WASM entries (e.g. binary tarballs from cargo-dist) + case "$filename" in *-wasm32-wasip2.tar.gz) ;; *) continue ;; esac + # Parse kind-prefixed filename: "tool-slack-0.2.1-wasm32-wasip2.tar.gz" + # → kind=tool, name=slack + kind=$(echo "$filename" | cut -d'-' -f1) + if [ "$kind" != "tool" ] && [ "$kind" != "channel" ]; then + echo "::warning::Skipping '$filename': unrecognized kind prefix '$kind'" + continue + fi + name=$(echo "$filename" | sed "s/^${kind}-//" | sed 's/-[0-9].*-wasm32-wasip2\.tar\.gz$//') url="https://github.com/nearai/ironclaw/releases/download/${RELEASE_TAG}/${filename}" - for manifest in registry/tools/${name}.json registry/channels/${name}.json; do - if [ -f "$manifest" ]; then - jq --arg sha "$sha256" --arg url "$url" \ - '.artifacts["wasm32-wasip2"].sha256 = $sha | .artifacts["wasm32-wasip2"].url = $url' \ - "$manifest" > "${manifest}.tmp" && mv "${manifest}.tmp" "$manifest" - echo "Patched $manifest with sha256=$sha256 url=$url" - fi - done + manifest="registry/${kind}s/${name}.json" + if [ -f "$manifest" ]; then + jq --arg sha "$sha256" --arg url "$url" \ + '.artifacts["wasm32-wasip2"].sha256 = $sha | .artifacts["wasm32-wasip2"].url = $url' \ + "$manifest" > "${manifest}.tmp" && mv "${manifest}.tmp" "$manifest" + echo "Patched $manifest with sha256=$sha256 url=$url" + fi done < "$CHECKSUMS" - name: Install dependencies run: | @@ -276,9 +282,14 @@ jobs: [ -f "$manifest" ] || continue # file_stem: JSON filename without extension (e.g. "slack" for slack.json). - # Used for the bundle filename and CI manifest lookup, so patching always - # finds the right file regardless of whether manifest.name matches the filename. file_stem=$(basename "$manifest" .json) + # kind: "tool" or "channel" — used as bundle filename prefix to avoid + # collisions when a tool and channel share the same file_stem (e.g. slack). + kind=$(jq -r '.kind' "$manifest") + if [ "$kind" != "tool" ] && [ "$kind" != "channel" ]; then + echo "::error::Manifest '$manifest' has invalid or missing .kind ('$kind'); expected 'tool' or 'channel'" + exit 1 + fi # ext_name: the manifest's .name field (e.g. "slack-tool"). # Used for file names *inside* the archive — the installer extracts by manifest.name. ext_name=$(jq -r '.name' "$manifest") @@ -340,18 +351,19 @@ jobs: echo "::warning::No capabilities file at '$caps_path' for '$file_stem'" fi - # Bundle filename uses file_stem so CI patching can find the manifest by - # filename (e.g. slack-0.1.0-wasm32-wasip2.tar.gz → registry/tools/slack.json). - bundle="target/wasm-bundles/${file_stem}-${ext_version}-wasm32-wasip2.tar.gz" + # Bundle filename uses kind+file_stem to avoid collisions when a tool + # and channel share the same name (e.g. tool-slack vs channel-slack). + bundle_name="${kind}-${file_stem}-${ext_version}-wasm32-wasip2.tar.gz" + bundle="target/wasm-bundles/${bundle_name}" (cd target/wasm-bundles && if [ -f "${ext_name}.capabilities.json" ]; then - tar czf "${file_stem}-${ext_version}-wasm32-wasip2.tar.gz" "${ext_name}.wasm" "${ext_name}.capabilities.json" + tar czf "${bundle_name}" "${ext_name}.wasm" "${ext_name}.capabilities.json" else - tar czf "${file_stem}-${ext_version}-wasm32-wasip2.tar.gz" "${ext_name}.wasm" + tar czf "${bundle_name}" "${ext_name}.wasm" fi) # Compute SHA256 sha256=$(sha256sum "$bundle" | cut -d' ' -f1) - echo "$sha256 ${file_stem}-${ext_version}-wasm32-wasip2.tar.gz" >> target/wasm-bundles/checksums.txt + echo "$sha256 ${bundle_name}" >> target/wasm-bundles/checksums.txt # Clean up intermediate files rm -f "target/wasm-bundles/${ext_name}.wasm" "target/wasm-bundles/${ext_name}.capabilities.json" @@ -474,19 +486,25 @@ jobs: while IFS= read -r line; do sha256=$(echo "$line" | awk '{print $1}') filename=$(echo "$line" | awk '{print $2}') - # Strip -{version}-wasm32-wasip2.tar.gz to get the extension name. - # Use '.*' (greedy) so pre-release suffixes like -alpha.1 are consumed too. - name=$(echo "$filename" | sed 's/-[0-9].*-wasm32-wasip2\.tar\.gz$//') + # Skip non-WASM entries (defensive — this checksums.txt should only have WASM) + case "$filename" in *-wasm32-wasip2.tar.gz) ;; *) continue ;; esac + # Parse kind-prefixed filename: "tool-slack-0.2.1-wasm32-wasip2.tar.gz" + # → kind=tool, name=slack + kind=$(echo "$filename" | cut -d'-' -f1) + if [ "$kind" != "tool" ] && [ "$kind" != "channel" ]; then + echo "::warning::Skipping '$filename': unrecognized kind prefix '$kind'" + continue + fi + name=$(echo "$filename" | sed "s/^${kind}-//" | sed 's/-[0-9].*-wasm32-wasip2\.tar\.gz$//') url="https://github.com/nearai/ironclaw/releases/download/${RELEASE_TAG}/${filename}" - for manifest in registry/tools/${name}.json registry/channels/${name}.json; do - if [ -f "$manifest" ]; then - jq --arg sha "$sha256" --arg url "$url" \ - '.artifacts["wasm32-wasip2"].sha256 = $sha | .artifacts["wasm32-wasip2"].url = $url' \ - "$manifest" > "${manifest}.tmp" && mv "${manifest}.tmp" "$manifest" - echo "Patched $manifest with sha256=$sha256 url=$url" - fi - done + manifest="registry/${kind}s/${name}.json" + if [ -f "$manifest" ]; then + jq --arg sha "$sha256" --arg url "$url" \ + '.artifacts["wasm32-wasip2"].sha256 = $sha | .artifacts["wasm32-wasip2"].url = $url' \ + "$manifest" > "${manifest}.tmp" && mv "${manifest}.tmp" "$manifest" + echo "Patched $manifest with sha256=$sha256 url=$url" + fi done < "$CHECKSUMS" - name: Create PR with updated manifests run: | diff --git a/scripts/test-ci-artifact-naming.sh b/scripts/test-ci-artifact-naming.sh new file mode 100755 index 00000000..290f3f21 --- /dev/null +++ b/scripts/test-ci-artifact-naming.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Test that kind-prefixed artifact filenames are parsed correctly into +# manifest paths. Mirrors the parsing logic in release.yml. +set -euo pipefail + +cd "$(dirname "$0")/.." + +PASS=0 +FAIL=0 + +assert_parse() { + local filename="$1" expected_kind="$2" expected_name="$3" + local kind name manifest + + kind=$(echo "$filename" | cut -d'-' -f1) + name=$(echo "$filename" | sed "s/^${kind}-//" | sed 's/-[0-9].*-wasm32-wasip2\.tar\.gz$//') + manifest="registry/${kind}s/${name}.json" + + if [[ "$kind" != "$expected_kind" ]]; then + echo "FAIL: $filename → kind=$kind, expected $expected_kind" + FAIL=$((FAIL + 1)) + return + fi + if [[ "$name" != "$expected_name" ]]; then + echo "FAIL: $filename → name=$name, expected $expected_name" + FAIL=$((FAIL + 1)) + return + fi + echo "OK: $filename → $manifest" + PASS=$((PASS + 1)) +} + +# Tool and channel with same name must produce different manifest paths +assert_parse "tool-slack-0.2.1-wasm32-wasip2.tar.gz" "tool" "slack" +assert_parse "channel-slack-0.2.1-wasm32-wasip2.tar.gz" "channel" "slack" + +# Same collision case for telegram +assert_parse "tool-telegram-0.2.2-wasm32-wasip2.tar.gz" "tool" "telegram" +assert_parse "channel-telegram-0.2.2-wasm32-wasip2.tar.gz" "channel" "telegram" + +# Hyphenated extension names +assert_parse "tool-web-search-0.2.0-wasm32-wasip2.tar.gz" "tool" "web-search" +assert_parse "tool-google-calendar-0.1.0-wasm32-wasip2.tar.gz" "tool" "google-calendar" +assert_parse "tool-google-docs-0.1.0-wasm32-wasip2.tar.gz" "tool" "google-docs" +assert_parse "tool-google-drive-0.1.0-wasm32-wasip2.tar.gz" "tool" "google-drive" +assert_parse "tool-google-sheets-0.1.0-wasm32-wasip2.tar.gz" "tool" "google-sheets" +assert_parse "tool-google-slides-0.1.0-wasm32-wasip2.tar.gz" "tool" "google-slides" + +# Simple names +assert_parse "channel-discord-0.2.0-wasm32-wasip2.tar.gz" "channel" "discord" +assert_parse "channel-whatsapp-0.1.0-wasm32-wasip2.tar.gz" "channel" "whatsapp" +assert_parse "tool-github-0.2.0-wasm32-wasip2.tar.gz" "tool" "github" +assert_parse "tool-gmail-0.1.0-wasm32-wasip2.tar.gz" "tool" "gmail" + +# Pre-release versions +assert_parse "tool-slack-0.2.1-alpha.1-wasm32-wasip2.tar.gz" "tool" "slack" + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ $FAIL -eq 0 ]] || exit 1 diff --git a/src/registry/installer.rs b/src/registry/installer.rs index 342af326..91f536f4 100644 --- a/src/registry/installer.rs +++ b/src/registry/installer.rs @@ -629,6 +629,7 @@ fn is_gzip(bytes: &[u8]) -> bool { } /// Result of extracting a tar.gz bundle. +#[derive(Debug)] struct ExtractResult { has_capabilities: bool, } @@ -1112,4 +1113,169 @@ mod tests { "ChecksumMismatch on version-pinned URL must remain a hard block" ); } + + // Regression tests for tool/channel artifact name collision (PR #964). + // When a tool and channel share the same registry filename (e.g. slack.json), + // CI produces kind-prefixed bundles (tool-slack-*.tar.gz vs channel-slack-*.tar.gz). + // The files *inside* each archive use manifest.name (slack-tool.wasm vs slack.wasm). + // These tests verify the installer extracts by manifest.name correctly. + + fn build_test_tar_gz(wasm_name: &str, caps_name: Option<&str>) -> Vec { + use flate2::Compression; + use flate2::write::GzEncoder; + use tar::Builder; + + let mut encoder = GzEncoder::new(Vec::new(), Compression::default()); + { + let mut builder = Builder::new(&mut encoder); + + let wasm_data = b"\0asm\x01\x00\x00\x00"; + let mut header = tar::Header::new_gnu(); + header.set_size(wasm_data.len() as u64); + header.set_cksum(); + builder + .append_data(&mut header, wasm_name, &wasm_data[..]) + .unwrap(); + + if let Some(caps) = caps_name { + let caps_data = br#"{"auth":null}"#; + let mut header = tar::Header::new_gnu(); + header.set_size(caps_data.len() as u64); + header.set_cksum(); + builder + .append_data(&mut header, caps, &caps_data[..]) + .unwrap(); + } + + builder.finish().unwrap(); + } + encoder.finish().unwrap() + } + + #[test] + fn test_extract_rejects_archive_with_wrong_wasm_name() { + // Simulates the collision bug: archive contains channel's slack.wasm, + // but installer tries to extract tool's slack-tool.wasm. + let gz_bytes = build_test_tar_gz("slack.wasm", Some("slack.capabilities.json")); + + let tmp = tempfile::tempdir().unwrap(); + let result = extract_tar_gz( + &gz_bytes, + "slack-tool", + &tmp.path().join("slack-tool.wasm"), + &tmp.path().join("slack-tool.capabilities.json"), + "test://url", + ); + + let err = result.expect_err("should fail when archive has wrong wasm name"); + match err { + RegistryError::DownloadFailed { reason, .. } => { + assert!( + reason.contains("slack-tool.wasm"), + "error should mention expected filename: {}", + reason + ); + } + other => panic!("expected DownloadFailed, got: {:?}", other), + } + } + + #[test] + fn test_extract_correct_wasm_from_tool_bundle() { + // Tool bundle contains slack-tool.wasm — extraction by name="slack-tool" succeeds. + let gz_bytes = build_test_tar_gz("slack-tool.wasm", Some("slack-tool.capabilities.json")); + + let tmp = tempfile::tempdir().unwrap(); + let wasm_path = tmp.path().join("slack-tool.wasm"); + let caps_path = tmp.path().join("slack-tool.capabilities.json"); + + let result = extract_tar_gz( + &gz_bytes, + "slack-tool", + &wasm_path, + &caps_path, + "test://url", + ) + .unwrap(); + + assert!(wasm_path.exists()); + assert!(caps_path.exists()); + assert!(result.has_capabilities); + } + + #[test] + fn test_extract_correct_wasm_from_channel_bundle() { + // Channel bundle contains slack.wasm — extraction by name="slack" succeeds. + let gz_bytes = build_test_tar_gz("slack.wasm", Some("slack.capabilities.json")); + + let tmp = tempfile::tempdir().unwrap(); + let wasm_path = tmp.path().join("slack.wasm"); + let caps_path = tmp.path().join("slack.capabilities.json"); + + let result = + extract_tar_gz(&gz_bytes, "slack", &wasm_path, &caps_path, "test://url").unwrap(); + + assert!(wasm_path.exists()); + assert!(caps_path.exists()); + assert!(result.has_capabilities); + } + + #[tokio::test] + async fn test_tool_and_channel_install_to_separate_directories() { + // Tool and channel manifests with the same file_stem ("slack") install + // to different directories without collision. + let temp = tempfile::tempdir().expect("tempdir"); + let installer = RegistryInstaller::new( + temp.path().to_path_buf(), + temp.path().join("tools"), + temp.path().join("channels"), + ); + + let tool_manifest = test_manifest_with_kind( + "slack-tool", + "tools-src/slack", + None, + None, + ManifestKind::Tool, + ); + let channel_manifest = test_manifest_with_kind( + "slack", + "channels-src/slack", + None, + None, + ManifestKind::Channel, + ); + + // Both fail because source dirs don't exist, but the error path reveals + // the target directory — tool goes to tools/, channel goes to channels/. + let tool_err = installer + .install_from_source(&tool_manifest, false) + .await + .expect_err("no source dir"); + let channel_err = installer + .install_from_source(&channel_manifest, false) + .await + .expect_err("no source dir"); + + match tool_err { + RegistryError::ManifestRead { path, .. } => { + assert!( + path.ends_with("tools-src/slack"), + "tool should resolve to tools-src/slack, got: {}", + path.display() + ); + } + other => panic!("expected ManifestRead for tool, got: {:?}", other), + } + match channel_err { + RegistryError::ManifestRead { path, .. } => { + assert!( + path.ends_with("channels-src/slack"), + "channel should resolve to channels-src/slack, got: {}", + path.display() + ); + } + other => panic!("expected ManifestRead for channel, got: {:?}", other), + } + } }