Compare commits

..
Author SHA1 Message Date
Paul Bakaus 8951aa5073 Fix per-app design resolution in Rust hooks
Port the approach proposed by tylerjryan in #367, preserving per-file design scope and batch notice attribution.

AI assistance: Codex, under maintainer direction.
2026-09-07 14:19:20 -07:00
6 changed files with 24 additions and 193 deletions
-10
View File
@@ -139,16 +139,6 @@ npx impeccable link --source=.impeccable --providers=claude,cursor
### Option 3: Plugin install
**GitHub Copilot in VS Code:**
Install [Impeccable from the Visual Studio Marketplace](https://marketplace.visualstudio.com/items?itemName=renaissance-geek.impeccable), or run:
```bash
code --install-extension renaissance-geek.impeccable
```
Requires VS Code 1.109.3+, Copilot Chat access, and a trusted local workspace. Open Chat in Agent mode and try `/impeccable polish`. This skill-only extension does not install automatic hooks; avoid a duplicate Impeccable skill in the same workspace/profile. See [VS Code distribution details](docs/VSCODE-EXTENSION.md).
**Claude Code:**
```bash
/plugin marketplace add pbakaus/impeccable
+11 -84
View File
@@ -30,39 +30,9 @@ static LAUNCHER_HOOK_MARKER: Lazy<Regex> = Lazy::new(|| {
Regex::new(r#"skills/impeccable/scripts/impeccable(?:\.cmd|\.exe)?["']?\s+hook(?:-before-edit|-probe|-after-edit|-stop)?(?:\s|$|["'&|;)])"#).unwrap()
});
/// User-scope Windows commands embed a JSON-quoted path whose backslashes are
/// doubled in the command string (#784). #604's single `\`→`/` replace is not
/// enough: `\\` becomes `//`, which breaks `skills/impeccable` matching.
/// A leading `//` after a quote (or at the start of the string) is a UNC
/// prefix and stays two slashes, so doctor still probes `//server/share/...`.
fn normalize_hook_separators(command: &str) -> String {
let mut out = String::with_capacity(command.len());
let mut chars = command.chars().peekable();
let mut prev: Option<char> = None;
while let Some(ch) = chars.next() {
if ch != '\\' && ch != '/' {
out.push(ch);
prev = Some(ch);
continue;
}
let mut n = 1usize;
while matches!(chars.peek(), Some('\\' | '/')) {
chars.next();
n += 1;
}
out.push('/');
if n >= 2 && matches!(prev, None | Some('"' | '\'')) {
out.push('/');
}
prev = Some('/');
}
out
}
/// True when `command` invokes an Impeccable hook in either generation's spelling.
pub fn is_impeccable_hook_command(command: &str) -> bool {
let command = normalize_hook_separators(command);
LEGACY_HOOK_SCRIPT_MARKERS.iter().any(|m| command.contains(m)) || LAUNCHER_HOOK_MARKER.is_match(&command)
LEGACY_HOOK_SCRIPT_MARKERS.iter().any(|m| command.contains(m)) || LAUNCHER_HOOK_MARKER.is_match(command)
}
/// True when `command` invokes an Impeccable hook in the launcher generation
@@ -70,7 +40,7 @@ pub fn is_impeccable_hook_command(command: &str) -> bool {
/// one: install/update use this to decide which manifests still need
/// migrating to the launcher form.
pub fn is_launcher_hook_command(command: &str) -> bool {
LAUNCHER_HOOK_MARKER.is_match(&normalize_hook_separators(command))
LAUNCHER_HOOK_MARKER.is_match(command)
}
/// The launcher-era markers `context` and `doctor` treat as the design hook
@@ -84,10 +54,9 @@ static LAUNCHER_DESIGN_HOOK: Lazy<Regex> = Lazy::new(|| {
/// `hook-before-edit`) in either spelling; the JS `context.mjs` scan and
/// `staleness-deep` HOOK_SCRIPT_MARKERS both meant exactly these two.
pub fn is_design_hook_command(command: &str) -> bool {
let command = normalize_hook_separators(command);
command.contains("skills/impeccable/scripts/hook.mjs")
|| command.contains("skills/impeccable/scripts/hook-before-edit.mjs")
|| LAUNCHER_DESIGN_HOOK.is_match(&command)
|| LAUNCHER_DESIGN_HOOK.is_match(command)
}
/// True when `command` runs the design hook (`hook` / `hook-before-edit`) in
@@ -97,7 +66,7 @@ pub fn is_design_hook_command(command: &str) -> bool {
/// so the hook is dead and `MANUAL_DETECTOR_REQUIRED` must fire until an
/// install/update repairs it.
pub fn is_launcher_design_hook_command(command: &str) -> bool {
LAUNCHER_DESIGN_HOOK.is_match(&normalize_hook_separators(command))
LAUNCHER_DESIGN_HOOK.is_match(command)
}
/// The shell token that names the hook program inside `command`, for
@@ -105,11 +74,7 @@ pub fn is_launcher_design_hook_command(command: &str) -> bool {
/// path in the binary form. `None` when the command carries no marker or
/// the token cannot be isolated (a `'\''` escape sequence, for instance).
pub fn hook_program_token(command: &str) -> Option<String> {
if command.contains("'\\''") {
return None;
}
let command = normalize_hook_separators(command);
if !is_design_hook_command(&command) {
if !is_design_hook_command(command) {
return None;
}
static QUOTED: Lazy<Regex> = Lazy::new(|| {
@@ -121,13 +86,16 @@ pub fn hook_program_token(command: &str) -> Option<String> {
static BARE: Lazy<Regex> = Lazy::new(|| {
Regex::new(r#"([^\s"'|&;()]*skills/impeccable/scripts/(?:hook(?:-before-edit)?\.mjs|impeccable(?:\.cmd|\.exe)?))"#).unwrap()
});
if let Some(m) = QUOTED.captures(&command) {
if let Some(m) = QUOTED.captures(command) {
return Some(m[1].to_string());
}
if let Some(m) = SINGLE.captures(&command) {
if command.contains("'\\''") {
return None;
}
if let Some(m) = SINGLE.captures(command) {
return Some(m[1].to_string());
}
BARE.captures(&command).map(|m| m[1].to_string())
BARE.captures(command).map(|m| m[1].to_string())
}
#[cfg(test)]
@@ -215,45 +183,4 @@ mod tests {
assert_eq!(hook_program_token("'/x/it'\\''s/.claude/skills/impeccable/scripts/impeccable' hook"), None);
assert_eq!(hook_program_token("echo hi"), None);
}
#[test]
fn recognizes_json_escaped_windows_launcher_path() {
let launcher = r"C:\Users\alice\.claude\skills\impeccable\scripts\impeccable";
let quoted = serde_json::to_string(launcher).unwrap();
let cmd = format!("[ ! -f {quoted} ] || {quoted} hook");
assert!(is_impeccable_hook_command(&cmd), "{cmd}");
assert!(is_launcher_hook_command(&cmd), "{cmd}");
assert!(is_design_hook_command(&cmd), "{cmd}");
assert_eq!(
hook_program_token(&cmd).as_deref(),
Some("C:/Users/alice/.claude/skills/impeccable/scripts/impeccable")
);
}
#[test]
fn recognizes_single_backslash_windows_path() {
let cmd = r#"[ ! -f "C:\Users\alice\.claude\skills\impeccable\scripts\impeccable" ] || "C:\Users\alice\.claude\skills\impeccable\scripts\impeccable" hook"#;
assert!(is_impeccable_hook_command(cmd), "{cmd}");
assert!(is_launcher_hook_command(cmd), "{cmd}");
assert!(is_design_hook_command(cmd), "{cmd}");
}
#[test]
fn preserves_unc_prefix_in_program_token() {
let launcher = r"\\server\share\.claude\skills\impeccable\scripts\impeccable";
let quoted = serde_json::to_string(launcher).unwrap();
let json_escaped = format!("[ ! -f {quoted} ] || {quoted} hook");
assert!(is_impeccable_hook_command(&json_escaped), "{json_escaped}");
assert_eq!(
hook_program_token(&json_escaped).as_deref(),
Some("//server/share/.claude/skills/impeccable/scripts/impeccable")
);
let single = r#"[ ! -f "\\server\share\.claude\skills\impeccable\scripts\impeccable" ] || "\\server\share\.claude\skills\impeccable\scripts\impeccable" hook"#;
assert!(is_impeccable_hook_command(single), "{single}");
assert_eq!(
hook_program_token(single).as_deref(),
Some("//server/share/.claude/skills/impeccable/scripts/impeccable")
);
}
}
+9 -3
View File
@@ -219,10 +219,13 @@ fn rewrite_value(value: &Value, provider: &str, quoted: &QuotedPath, win32: bool
}
}
/// JS: valueHasImpeccableHookMarker(value).
/// JS: valueHasImpeccableHookMarker(value). Command separators are
/// normalized to `/` first so a legacy Windows-path guard is still
/// recognized as ours and replaced instead of duplicated (upstream
/// 665c51b9, #604).
pub fn value_has_impeccable_hook_marker(value: &Value) -> bool {
match value {
Value::String(s) => is_impeccable_hook_command(s),
Value::String(s) => is_impeccable_hook_command(&s.replace('\\', "/")),
Value::Array(a) => a.iter().any(value_has_impeccable_hook_marker),
Value::Object(o) => o.values().any(value_has_impeccable_hook_marker),
_ => false,
@@ -230,9 +233,12 @@ pub fn value_has_impeccable_hook_marker(value: &Value) -> bool {
}
/// True when `value` names an Impeccable hook in the launcher generation.
/// Separators are normalized to `/` first, matching
/// `value_has_impeccable_hook_marker`, so a legacy Windows-path launcher
/// command is still recognized.
pub fn value_has_launcher_hook_marker(value: &Value) -> bool {
match value {
Value::String(s) => is_launcher_hook_command(s),
Value::String(s) => is_launcher_hook_command(&s.replace('\\', "/")),
Value::Array(a) => a.iter().any(value_has_launcher_hook_marker),
Value::Object(o) => o.values().any(value_has_launcher_hook_marker),
_ => false,
+1 -33
View File
@@ -35,10 +35,6 @@ impl Prompt {
self.stdin_tty && self.stdout_tty && cfg!(unix)
}
fn uses_tty_readline(&self, io: &Io) -> bool {
cfg!(unix) && self.stdout_tty && io.env("TERM") != Some("dumb")
}
fn ansi(&self, open: &str, close: &str, value: &str) -> String {
if self.style {
format!("{open}{value}{close}")
@@ -74,7 +70,7 @@ impl Prompt {
let next = self.piped.as_mut().and_then(|v| v.pop()).unwrap_or_default();
return Ok(next.trim().to_lowercase());
}
if self.uses_tty_readline(io) {
if self.stdout_tty && io.env("TERM") != Some("dumb") {
return self.tty_readline(io, question);
}
io.out(question);
@@ -628,9 +624,6 @@ fn terminal_rows() -> Option<u16> {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::path::PathBuf;
use super::*;
#[test]
@@ -644,29 +637,4 @@ mod tests {
assert_eq!(visible_window(15, 16, 10), (6, 16));
assert_eq!(visible_window(2, 3, 10), (0, 3));
}
fn tty_prompt() -> Prompt {
Prompt { stdin_tty: true, stdout_tty: true, style: false, piped: None }
}
#[test]
fn ask_uses_raw_readline_only_on_unix() {
let prompt = tty_prompt();
let (io, _) = Io::captured("", PathBuf::from("."), HashMap::new());
assert_eq!(prompt.uses_tty_readline(&io), cfg!(unix));
}
#[cfg(not(unix))]
#[test]
fn ask_on_windows_tty_does_not_throw_unsupported() {
// Line fallback reads process stdin. Skip on a live console so the
// test cannot hang; CI pipes EOF and gets Ok("").
if std::io::IsTerminal::is_terminal(&std::io::stdin()) {
return;
}
let mut prompt = tty_prompt();
let (mut io, _) = Io::captured("", PathBuf::from("."), HashMap::new());
let result = prompt.ask(&mut io, "Update skills in 1 provider folder(s)? (Y/n) ");
assert_eq!(result, Ok(String::new()));
}
}
@@ -357,57 +357,3 @@ fn hook_artifacts_map_providers_to_manifest_files() {
assert_eq!(c[0].dest, jsp::join(&["/p", ".codex", "hooks.json"]));
assert!(c[0].shared_dest.is_none());
}
fn windows_user_scope_hook_command() -> String {
let launcher = r"C:\Users\alice\.claude\skills\impeccable\scripts\impeccable";
let q = json_string(launcher);
format!("[ ! -f {q} ] || {q} hook")
}
#[test]
fn merge_json_escaped_windows_launcher_is_idempotent() {
let cmd = windows_user_scope_hook_command();
let hook_entry = |cmd: String| {
json!({ "matcher": "Edit", "hooks": [{ "type": "command", "command": cmd }] })
};
let stop_entry = |cmd: String| {
json!({ "hooks": [{ "type": "command", "command": cmd, "timeout": 30 }] })
};
let existing = json!({
"hooks": {
"PostToolUse": [hook_entry(cmd.clone())],
"Stop": [stop_entry(cmd.clone())]
}
});
let fresh = json!({
"description": "fresh",
"hooks": {
"PostToolUse": [hook_entry(cmd.clone())],
"Stop": [stop_entry(cmd.clone())]
}
});
let merged = merge_hook_manifests(&existing, &fresh);
assert_eq!(merged["hooks"]["PostToolUse"].as_array().unwrap().len(), 1);
assert_eq!(merged["hooks"]["Stop"].as_array().unwrap().len(), 1);
let merged2 = merge_hook_manifests(&merged, &fresh);
assert_eq!(merged2["hooks"]["PostToolUse"].as_array().unwrap().len(), 1);
assert_eq!(merged2["hooks"]["Stop"].as_array().unwrap().len(), 1);
}
#[test]
fn merge_heals_triplicated_stop_groups() {
let cmd = windows_user_scope_hook_command();
let stop_entry = json!({ "hooks": [{ "type": "command", "command": cmd.clone(), "timeout": 30 }] });
let existing = json!({
"hooks": {
"Stop": [stop_entry.clone(), stop_entry.clone(), stop_entry]
}
});
let fresh = json!({
"hooks": {
"Stop": [json!({ "hooks": [{ "type": "command", "command": cmd, "timeout": 30 }] })]
}
});
let merged = merge_hook_manifests(&existing, &fresh);
assert_eq!(merged["hooks"]["Stop"].as_array().unwrap().len(), 1);
}
+3 -9
View File
@@ -2,9 +2,7 @@
The VS Code extension is a declarative delivery channel for the GitHub Copilot skill. `bun run build` stages `dist/vscode/` from the GitHub provider output; `bun run package:vscode` builds and packages a VSIX with pinned `@vscode/vsce` tooling. Nothing is published by either command.
Install [Impeccable from the Visual Studio Marketplace](https://marketplace.visualstudio.com/items?itemName=renaissance-geek.impeccable), published by Renaissance Geek, or run `code --install-extension renaissance-geek.impeccable`. Requires VS Code 1.109.3+, Copilot Chat access, and a trusted local workspace. Use Chat in Agent mode, for example `/impeccable polish`. Avoid duplicate Impeccable skill installations in the same workspace/profile.
The package version follows `.claude-plugin/plugin.json`. Do not independently bump it for feature work. Publication and updates are separate maintainer steps; building or packaging does not publish.
The package version follows `.claude-plugin/plugin.json`. Do not independently bump it for feature work. The Marketplace identifier is `renaissance-geek.impeccable`, under the registered Renaissance Geek publisher. Initial extension publication is a separate maintainer step; publisher registration alone does not publish the extension.
## Scope
@@ -39,7 +37,7 @@ Check that `/impeccable` is discoverable, the loaded SKILL.md is inside the inst
Test the oldest supported editor as well as current stable before publication. Windows and remote workspaces need separate smoke checks; do not infer them from a macOS local run. Plain browser-only VS Code cannot run the native launcher.
Initial macOS packaging checks: the VSIX installs in VS Code 1.109.3 (which resolves Copilot Chat 0.37.9) and 1.136.1. Both editor versions also passed the read-only Copilot behavior smoke below.
Initial macOS packaging checks: the VSIX installs in VS Code 1.109.3 (which resolves Copilot Chat 0.37.9) and 1.136.1. The installed launcher loads the synthetic project's context correctly. The older editor has only been install-tested, not behavior-tested.
### Recorded Copilot smoke (September 7, 2026)
@@ -50,8 +48,4 @@ VS Code 1.136.1, Copilot Chat 0.64.1, Auto routed to GPT-5.6 Luna. A single read
- The loader resolved the synthetic project root and its PRODUCT.md and DESIGN.md. Copilot then read `reference/polish.md` from the same extension, followed by the three fixture files.
- The completed report correctly described the fixture. The project still contained only its original three files, with unchanged contents. No workspace skill copy, server, or image-generation call was created.
The final `renaissance-geek.impeccable` 4.2.2 VSIX also passed the same read-only smoke in VS Code 1.109.3 / Copilot Chat 0.37.9 (Auto selected GPT-5.3-Codex): slash discovery, the installed launcher, project context, and the polish reference all resolved correctly. One scoped command approval was granted, and all three fixture files remained unchanged.
After publication, `code --install-extension renaissance-geek.impeccable` installed 4.2.2 into fresh isolated profile/extensions directories in VS Code 1.136.1. All 55 extension payload files matched the tested VSIX (ignoring VS Code's added `package.json` installation metadata). Its installed launcher loaded the same fixture context successfully; fixture hashes and file inventory stayed unchanged. This Marketplace check verified download/install and payload identity, not an additional Copilot conversation.
These are packaging/path-resolution smokes, not activation-reliability or design-quality evaluations. They do not establish Windows or remote-host behavior.
This is one packaging/path-resolution smoke, not an activation-reliability or design-quality evaluation. It does not establish Windows, remote-host, or minimum-version behavior.