mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(ci): disambiguate WASM bundle filenames to prevent tool/channel collision (#964)
* fix(ci): disambiguate WASM bundle filenames to prevent tool/channel collision When a tool and channel share the same name (e.g. slack, telegram), the CI build produced identical bundle filenames, causing the second to overwrite the first. Both manifests then pointed to the wrong binary. Prefix bundle filenames with the extension kind (tool-slack-... vs channel-slack-...) and parse the prefix when patching manifests, so each manifest receives the correct artifact URL and SHA256. Co-Authored-By: Claude Opus 4.6 <[email protected]> * test(registry): add installer tests for tool/channel name disambiguation Regression tests for the CI artifact collision fix (PR #964). Verifies: - extract_tar_gz rejects archives with wrong wasm name (the collision bug) - Tool bundle extracts slack-tool.wasm correctly - Channel bundle extracts slack.wasm correctly - Tool and channel manifests install to separate directories Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(ci): add kind validation and filter non-WASM checksum entries - Validate .kind is "tool" or "channel" before using in build-wasm-extensions (hard error) - Filter checksums.txt to *-wasm32-wasip2.tar.gz entries before parsing, avoiding noisy warnings from binary artifact entries in build-local-artifacts - Add kind validation with warning+skip in both checksum-parsing loops Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt formatting in installer tests Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Henry Park
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
977b7fde99
commit
81f7b64994
@@ -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: |
|
||||
|
||||
Executable
+60
@@ -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
|
||||
@@ -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<u8> {
|
||||
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),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user