skills/pbakaus_impeccable
1
0
Fork 0
mirror of https://github.com/pbakaus/impeccable.git synced 2026-09-12 06:06:37 +03:00
Code Issues Packages Projects Releases Wiki Activity

windows: skills tests pass on Windows

Browse Source 
The two test temp roots kept `canonicalize`'s `\\?\` verbatim prefix, and the
kernel takes a verbatim path literally, so every `/`-joined path built under
them was an invalid filename. Strip it the way Node's `realpathSync` does.
The manifest, artifact and sibling-binary expectations hard-coded POSIX
separators for paths the product joins with the host's semantics; derive them
from `jsp::join` instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
This commit is contained in:
Paul Bakaus
2026-09-03 14:17:34 -07:00
co-authored by Claude Fable 5.1
parent dde3067524
commit 556702efb8
4 changed files with 63 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/skills/src/engine_binary.rs
+10 -2
View File
@@ -174,7 +174,15 @@ mod tests {
"https://github.com/pbakaus/impeccable/releases/download/engine-v1.2.3/impeccable-darwin-arm64"
);
assert_eq!(asset_url("http://x/", "1", "windows", "x64"), "http://x/engine-v1/impeccable-windows-x64.exe");
assert_eq!(binary_path("/s", "linux", "x64"), "/s/scripts/bin/linux-x64/impeccable");
assert_eq!(binary_path("/s", "windows", "arm64"), "/s/scripts/bin/windows-arm64/impeccable.exe");
// The sibling binary path is joined with the host's path semantics
// (backslashes on Windows); only the asset name is platform-keyed.
assert_eq!(
binary_path("/s", "linux", "x64"),
jsp::join(&["/s", "scripts", "bin", "linux-x64", "impeccable"])
);
assert_eq!(
binary_path("/s", "windows", "arm64"),
jsp::join(&["/s", "scripts", "bin", "windows-arm64", "impeccable.exe"])
);
}
}
crates/skills/tests/drift_ports_tests.rs
+23 -8
View File
@@ -16,7 +16,7 @@
use std::collections::HashMap;
use std::path::PathBuf;
use impeccable_common::Io;
use impeccable_common::{jsp, Io};
use impeccable_skills::bundle::{self, download_file_with, FetchResponse};
use impeccable_skills::hook_manifest::merge_hook_manifests;
use impeccable_skills::providers::{Scope, Sys};
@@ -29,7 +29,11 @@ fn temp_root(name: &str) -> String {
.unwrap_or(0);
let dir = std::env::temp_dir().join(format!("impeccable-{name}-{}-{nanos}", std::process::id()));
std::fs::create_dir_all(&dir).expect("temp root");
dir.canonicalize().unwrap().to_string_lossy().into_owned()
// Like Node's `realpathSync`: no `\\?\` verbatim prefix on Windows, so the
// `/`-joined paths these tests build under this root still resolve (the
// kernel takes a verbatim path literally and rejects a forward slash).
let real = dir.canonicalize().unwrap().to_string_lossy().into_owned();
real.strip_prefix(r"\\?\").map(str::to_string).unwrap_or(real)
}
fn write(path: &str, content: &str) {
@@ -284,7 +288,9 @@ fn download_file_http_initial_url_throws_without_calling_fetch() {
#[test]
fn local_bundle_uses_mkdtemp_staging_under_tmpdir() {
let root = temp_root("staging");
let tmp = format!("{root}/tmp");
// The staging dir the bundle builds is joined with the host's path
// semantics, so the tmpdir it is compared against is joined the same way.
let tmp = jsp::join(&[&root, "tmp"]);
std::fs::create_dir_all(&tmp).unwrap();
let bundle_root = create_fake_universal_bundle(&root, &[".claude"]);
let env = base_env(&root, &tmp, &bundle_root);
@@ -292,7 +298,7 @@ fn local_bundle_uses_mkdtemp_staging_under_tmpdir() {
let staging = bundle::download_and_extract_bundle(&sys).unwrap();
assert!(staging.starts_with(&tmp), "{staging} not under {tmp}");
let basename = staging.rsplit('/').next().unwrap();
let basename = staging.rsplit(['/', '\\']).next().unwrap();
assert!(basename.starts_with("impeccable-local-bundle-"), "{basename}");
// Random mkdtemp suffix, not the old `-<pid>-<millis>` form.
let suffix = &basename["impeccable-local-bundle-".len()..];
@@ -453,6 +459,15 @@ fn inferred_home_rooted_updates_refresh_stale_or_missing_copilot_user_agents() {
// ─── Grok project hooks on global installs (49571365, #642) ──────────────────
/// The launcher path the installer writes into a hook manifest, in the host's
/// path form and escaped the way it lands inside the JSON file (a Windows path
/// carries backslashes, which JSON doubles).
fn manifest_skill_path(home: &str, provider: &str) -> String {
let p = jsp::join(&[home, provider, "skills", "impeccable", "scripts", "impeccable"]);
let quoted = serde_json::to_string(&p).unwrap();
quoted[1..quoted.len() - 1].to_string()
}
#[test]
fn global_install_rewrites_grok_project_hooks_to_the_global_skill_path() {
let root = temp_root("grok-global");
@@ -483,12 +498,12 @@ fn global_install_rewrites_grok_project_hooks_to_the_global_skill_path() {
}
// Launcher-era adaptation: the JS asserted the rewritten hook.mjs path;
// the engine writes the launcher form pointing at the same global root.
assert!(read(&format!("{tmp}/.claude/settings.local.json")).contains(&format!("{home}/.claude/skills/impeccable/scripts/impeccable")));
assert!(read(&format!("{tmp}/.codex/hooks.json")).contains(&format!("{home}/.agents/skills/impeccable/scripts/impeccable")));
assert!(read(&format!("{tmp}/.cursor/hooks.json")).contains(&format!("{home}/.cursor/skills/impeccable/scripts/impeccable")));
assert!(read(&format!("{tmp}/.claude/settings.local.json")).contains(&manifest_skill_path(&home, ".claude")));
assert!(read(&format!("{tmp}/.codex/hooks.json")).contains(&manifest_skill_path(&home, ".agents")));
assert!(read(&format!("{tmp}/.cursor/hooks.json")).contains(&manifest_skill_path(&home, ".cursor")));
let grok_hooks = read(&format!("{tmp}/.grok/hooks/impeccable.json"));
assert!(
grok_hooks.contains(&format!("{home}/.grok/skills/impeccable/scripts/impeccable")),
grok_hooks.contains(&manifest_skill_path(&home, ".grok")),
"grok hook not rewritten to the global skill path: {grok_hooks}"
);
assert!(
crates/skills/tests/hook_manifest_tests.rs
+25 -13
View File
@@ -2,6 +2,7 @@
//! hook command path resolution (#399)", "hook manifest merge helpers"), in
//! the launcher generation.
use impeccable_common::jsp;
use impeccable_skills::hook_manifest::*;
use impeccable_skills::providers::Sys;
use serde_json::{json, Value};
@@ -17,6 +18,12 @@ fn claude_bundle_manifest() -> Value {
})
}
/// The launcher path a hook command points at, joined with the host's path
/// semantics the way `rewrite_hook_commands_for_platform` joins it.
fn skill_launcher(root: &str, provider: &str) -> String {
jsp::join(&[root, provider, "skills", "impeccable", "scripts", "impeccable"])
}
fn commands(v: &Value) -> Vec<String> {
let mut out = Vec::new();
fn walk(v: &Value, out: &mut Vec<String>) {
@@ -63,7 +70,9 @@ fn absolute_path_is_single_quoted_and_inert_under_sh() {
let root = "/tmp/imp-hook-$(touch pwned)-x";
let out = rewrite_hook_commands_for_platform(&claude_bundle_manifest(), ".claude", root, true, false);
for c in commands(&out) {
let expected_path = format!("{root}/.claude/skills/impeccable/scripts/impeccable");
// The launcher path is joined with the host's path semantics, so the
// expectation is joined the same way (backslashes on Windows).
let expected_path = skill_launcher(root, ".claude");
assert_eq!(c, format!("command=[ ! -f '{expected_path}' ] || '{expected_path}' hook"));
assert!(!c.contains("${CLAUDE_PROJECT_DIR}"));
assert!(!c.contains(&format!("\"{root}")));
@@ -89,7 +98,7 @@ fn windows_form_keeps_double_quoted_absolute_path() {
let root = "/home/u";
let out = rewrite_hook_commands_for_platform(&claude_bundle_manifest(), ".claude", root, true, true);
for c in commands(&out) {
let p = format!("{root}/.claude/skills/impeccable/scripts/impeccable");
let p = skill_launcher(root, ".claude");
assert_eq!(c, format!("command=[ ! -f \"{p}\" ] || \"{p}\" hook"));
assert!(!c.contains(&format!("'{p}")));
}
@@ -147,9 +156,10 @@ fn github_manifests_pass_through_and_grok_is_rewritten() {
"[ ! -f \".grok/skills/impeccable/scripts/impeccable\" ] || \".grok/skills/impeccable/scripts/impeccable\" hook"
);
let abs = rewrite_hook_commands_for_platform(&grok, ".grok", "/home/u", true, false);
let p = skill_launcher("/home/u", ".grok");
assert_eq!(
abs["hooks"]["PostToolUse"][0]["hooks"][0]["command"],
"[ ! -f '/home/u/.grok/skills/impeccable/scripts/impeccable' ] || '/home/u/.grok/skills/impeccable/scripts/impeccable' hook"
serde_json::Value::String(format!("[ ! -f '{p}' ] || '{p}' hook"))
);
// Grok is not Codex: no commandWindows sibling is added.
assert!(rel["hooks"]["PostToolUse"][0]["hooks"][0].get("commandWindows").is_none());
@@ -326,19 +336,21 @@ fn repair_leaves_dirs_without_an_impeccable_marker_alone() {
#[test]
fn hook_artifacts_map_providers_to_manifest_files() {
let dests = expected_hook_dests("/p", &[".claude", ".agents", ".cursor", ".github", ".grok", ".gemini"]);
// Manifest destinations are joined with the host's path semantics, so the
// expectations are joined the same way (backslashes on Windows).
assert_eq!(dests, [
"/p/.claude/settings.local.json",
"/p/.codex/hooks.json",
"/p/.cursor/hooks.json",
"/p/.github/hooks/impeccable.json",
"/p/.grok/hooks/impeccable.json",
jsp::join(&["/p", ".claude", "settings.local.json"]),
jsp::join(&["/p", ".codex", "hooks.json"]),
jsp::join(&["/p", ".cursor", "hooks.json"]),
jsp::join(&["/p", ".github", "hooks", "impeccable.json"]),
jsp::join(&["/p", ".grok", "hooks", "impeccable.json"]),
]);
let a = hook_artifacts_for_provider("/b", "/p", ".claude");
assert_eq!(a[0].src, "/b/.claude/settings.json");
assert_eq!(a[0].dest, "/p/.claude/settings.local.json");
assert_eq!(a[0].shared_dest.as_deref(), Some("/p/.claude/settings.json"));
assert_eq!(a[0].src, jsp::join(&["/b", ".claude", "settings.json"]));
assert_eq!(a[0].dest, jsp::join(&["/p", ".claude", "settings.local.json"]));
assert_eq!(a[0].shared_dest.as_deref(), Some(jsp::join(&["/p", ".claude", "settings.json"]).as_str()));
let c = hook_artifacts_for_provider("/b", "/p", ".agents");
assert_eq!(c[0].src, "/b/.codex/hooks.json");
assert_eq!(c[0].dest, "/p/.codex/hooks.json");
assert_eq!(c[0].src, jsp::join(&["/b", ".codex", "hooks.json"]));
assert_eq!(c[0].dest, jsp::join(&["/p", ".codex", "hooks.json"]));
assert!(c[0].shared_dest.is_none());
}
crates/skills/tests/provider_commands_tests.rs
+5 -1
View File
@@ -15,7 +15,11 @@ fn temp_root(name: &str) -> String {
.unwrap_or(0);
let dir = std::env::temp_dir().join(format!("impeccable-{name}-{}-{nanos}", std::process::id()));
std::fs::create_dir_all(&dir).expect("temp root");
dir.canonicalize().unwrap().to_string_lossy().into_owned()
// Like Node's `realpathSync`: no `\\?\` verbatim prefix on Windows, so the
// `/`-joined paths these tests build under this root still resolve (the
// kernel takes a verbatim path literally and rejects a forward slash).
let real = dir.canonicalize().unwrap().to_string_lossy().into_owned();
real.strip_prefix(r"\\?\").map(str::to_string).unwrap_or(real)
}
fn write(path: &str, content: &str) {