Skip to content

Commit 66d601a

Browse files
refactor: dedup installer helpers and simplify marked-block splicing (#254)
* perf(mcp): clone the route cache once per refresh The per-request route refresh deep-cloned the shared cache twice: once in `snapshot()` and again via `clone_from` inside `refresh_from_shared`. Add `SharedHookProjectRouteCache::refresh_into`, which clones the shared cache exactly once under the lock and moves it into the target, preserving the target's local `project_path`. Drops the throwaway intermediate clone. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(agents): share marker/file-walk and manifest stamping helpers Three sets of byte-identical duplication across the host installers: - Hoist `skill_contents_have_tracedecay_marker` and the recursive `collect_regular_files` walker into `agents::mod` and route the Cursor (and, next commit, Codex) copies through them. - Add `plugin_bundle::{stamp_manifest_version, set_mcp_command}` and route Claude, Cursor, and Codex manifest/MCP-command rewrites through them. Codex layers its scope-specific args/env on top of the shared base. Behavior is byte-identical; the shared helpers reproduce each host's exact output (pretty JSON + trailing newline, same marker clauses/walk order). Co-Authored-By: Claude <noreply@anthropic.com> * refactor(agents): derive codex retired-skill sweep from the bundle Replace the hand-maintained 19-entry `RETIRED_CODEX_PLUGIN_SKILL_DIRS` list with the same bundle-derived sweep Cursor already uses: enumerate `skills/<dir>` on disk, keep whatever the live embedded bundle ships plus the agent-managed overlays, and remove any other dir whose `SKILL.md` carries a tracedecay marker. A newly retired skill is now swept automatically without editing a legacy list. Security-preserving: only non-shipped dirs that are demonstrably tracedecay-owned are removed; a same-name user-authored skill without a marker is left untouched (covered by update_plugin_test). Also routes the codex marker predicate and file walk through the shared helpers. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(skills): dedup marked-block splice, derive prompt targets - C: collapse the twin if-let/else-if-let arms in `replace_or_append_marked_block` — resolve the target range once (this target's slugged block, else the legacy unslugged one) and splice via a single `splice_range` helper. - D: derive the prompt-block target subset in `remove_all_marked_blocks` from `ALL_SKILL_INSTALL_TARGETS` via a new `SkillInstallTarget::writes_prompt_index()` predicate instead of a hardcoded 5-entry list, so the two stay in sync. The derived set equals the previous {Claude, Agents, OpenCode, Kimi, Kiro} exactly. - K: point the local `PROMPT_INDEX_START` at the identical `prompt_rules::SKILL_INDEX_START` literal. `remove_range` and `splice_out` differ in trailing-newline normalization, so they are left separate. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(agents): reuse the LSP file-URI encoder in kiro Kiro had a byte-duplicated `file_resource_uri` + `percent_encode_file_uri_path`. Widen the LSP client's `file_uri_from_path_text` to `pub(crate)` and have kiro delegate to it, deleting the private copies. POSIX paths encode identically to before; kiro now also gains the client's Windows drive-path and `//` UNC handling. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d00eb01 commit 66d601a

11 files changed

Lines changed: 180 additions & 191 deletions

File tree

src/agents/claude.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -391,17 +391,13 @@ fn substitute_command_placeholder(value: &mut serde_json::Value, tracedecay_bin:
391391

392392
/// Stamp the plugin manifest `version` with the crate version.
393393
fn stamp_plugin_version(raw: &str) -> Result<String> {
394-
let mut manifest: serde_json::Value = serde_json::from_str(raw)?;
395-
manifest["version"] = json!(env!("CARGO_PKG_VERSION"));
396-
Ok(format!("{}\n", serde_json::to_string_pretty(&manifest)?))
394+
super::plugin_bundle::stamp_manifest_version(raw)
397395
}
398396

399397
/// Set the plugin `.mcp.json` server command to the resolved absolute binary
400398
/// path, so the plugin does not rely on `tracedecay` being on PATH.
401399
fn set_mcp_command(raw: &str, tracedecay_bin: &str) -> Result<String> {
402-
let mut mcp: serde_json::Value = serde_json::from_str(raw)?;
403-
mcp["mcpServers"]["tracedecay"]["command"] = json!(tracedecay_bin);
404-
Ok(format!("{}\n", serde_json::to_string_pretty(&mcp)?))
400+
super::plugin_bundle::set_mcp_command(raw, tracedecay_bin)
405401
}
406402

407403
/// Remove the deployed bundle dir (idempotent; only touches the tracedecay

src/agents/codex.rs

Lines changed: 55 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -505,15 +505,15 @@ fn write_codex_plugin_files(
505505
}
506506

507507
fn codex_plugin_manifest(raw: &str) -> Result<String> {
508-
let mut manifest: serde_json::Value = serde_json::from_str(raw)?;
509-
manifest["version"] = json!(env!("CARGO_PKG_VERSION"));
510-
Ok(format!("{}\n", serde_json::to_string_pretty(&manifest)?))
508+
super::plugin_bundle::stamp_manifest_version(raw)
511509
}
512510

513511
fn codex_plugin_mcp(raw: &str, tracedecay_bin: &str, scope: InstallScope) -> Result<String> {
514-
let mut mcp: serde_json::Value = serde_json::from_str(raw)?;
512+
// Reuse the shared command rewrite, then layer Codex's scope-specific
513+
// args/env on top of the result.
514+
let stamped = super::plugin_bundle::set_mcp_command(raw, tracedecay_bin)?;
515+
let mut mcp: serde_json::Value = serde_json::from_str(&stamped)?;
515516
let server = &mut mcp["mcpServers"]["tracedecay"];
516-
server["command"] = json!(tracedecay_bin);
517517
match scope {
518518
InstallScope::Global => {
519519
server["args"] = json!(["serve"]);
@@ -726,37 +726,16 @@ fn remove_codex_managed_skill_overlay(install_dir: &Path) {
726726
std::fs::remove_dir_all(install_dir.join("skills/agent-managed")).ok();
727727
}
728728

729-
const RETIRED_CODEX_PLUGIN_SKILL_DIRS: &[&str] = &[
730-
"architecture-overview",
731-
"assessing-test-coverage",
732-
"atomic-code-edits",
733-
"auditing-code-safety",
734-
"cleaning-up-dead-code",
735-
"code-health-report",
736-
"cross-branch-investigation",
737-
"drafting-commit-and-pr",
738-
"exploring-types-and-traits",
739-
"finding-duplicate-logic",
740-
"finding-impacted-areas",
741-
"porting-code",
742-
"project-status",
743-
"reading-code-cheaply",
744-
"refactoring-safely",
745-
"reviewing-a-diff",
746-
"running-impacted-tests",
747-
"searching-for-code",
748-
"tracking-session-health",
749-
];
750-
751729
fn remove_codex_plugin_managed_skills(install_dir: &Path, skills_dir: &Path) -> Result<()> {
752-
remove_retired_codex_plugin_skill_dirs(skills_dir)?;
730+
sweep_retired_bundle_skill_dirs(skills_dir);
753731
let managed: HashSet<PathBuf> = codex_plugin_managed_paths(install_dir)
754732
.into_iter()
755733
.filter(|path| path.starts_with(skills_dir))
756734
.collect();
757-
let mut files = collect_regular_files(skills_dir).map_err(|e| TraceDecayError::Config {
758-
message: format!("failed to list {}: {e}", skills_dir.display()),
759-
})?;
735+
let mut files =
736+
super::collect_regular_files(skills_dir).map_err(|e| TraceDecayError::Config {
737+
message: format!("failed to list {}: {e}", skills_dir.display()),
738+
})?;
760739
files.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
761740
for file in files {
762741
if managed.contains(&file) || codex_skill_file_is_legacy_tracedecay_managed(&file) {
@@ -781,43 +760,54 @@ fn codex_skill_file_is_legacy_tracedecay_managed(path: &Path) -> bool {
781760
})
782761
}
783762

784-
fn remove_retired_codex_plugin_skill_dirs(skills_dir: &Path) -> Result<()> {
785-
for name in RETIRED_CODEX_PLUGIN_SKILL_DIRS {
786-
let skill_dir = skills_dir.join(name);
787-
if !codex_skill_dir_is_retired_managed(&skill_dir, name) {
763+
/// Remove every `skills/<dir>` under the Codex plugin dir that the current
764+
/// bundle no longer ships. The keep-set is derived from the live embedded
765+
/// bundle (plus the agent-managed overlays deployed separately), so any retired
766+
/// skill is swept on upgrade without a hand-maintained legacy list.
767+
///
768+
/// Only tracedecay-owned skill dirs are swept: a same-name user-authored skill
769+
/// whose `SKILL.md` carries no tracedecay marker is left untouched, so an
770+
/// upgrade never deletes a user's private workflow that collides with a retired
771+
/// bundle slug.
772+
fn sweep_retired_bundle_skill_dirs(skills_dir: &Path) {
773+
let Ok(entries) = std::fs::read_dir(skills_dir) else {
774+
return;
775+
};
776+
let mut shipped: std::collections::BTreeSet<String> = codex_embedded_plugin_files()
777+
.into_iter()
778+
.filter_map(|(relative, _)| {
779+
relative
780+
.strip_prefix("skills/")
781+
.and_then(|rest| rest.split('/').next())
782+
.map(str::to_string)
783+
})
784+
.collect();
785+
// The agent-managed overlays are deployed/removed separately; never treat
786+
// them as retired.
787+
shipped.insert("agent-managed".to_string());
788+
shipped.insert("agent-managed-memory".to_string());
789+
for entry in entries.flatten() {
790+
if !entry.file_type().is_ok_and(|t| t.is_dir()) {
788791
continue;
789792
}
790-
std::fs::remove_dir_all(&skill_dir).map_err(|e| TraceDecayError::Config {
791-
message: format!(
792-
"failed to remove retired Codex skill {}: {e}",
793-
skill_dir.display()
794-
),
795-
})?;
793+
let name = entry.file_name().to_string_lossy().into_owned();
794+
if shipped.contains(&name) {
795+
continue;
796+
}
797+
// Preserve user-authored skills that reuse a retired slug: only sweep a
798+
// non-shipped dir that is demonstrably tracedecay-owned.
799+
if !skill_file_has_tracedecay_marker(&entry.path().join("SKILL.md")) {
800+
continue;
801+
}
802+
std::fs::remove_dir_all(entry.path()).ok();
796803
}
797-
Ok(())
798-
}
799-
800-
fn codex_skill_dir_is_retired_managed(skill_dir: &Path, expected_name: &str) -> bool {
801-
let skill_file = skill_dir.join("SKILL.md");
802-
let expected_name_line = format!("name: {expected_name}");
803-
skill_file.is_file()
804-
&& std::fs::read_to_string(&skill_file).is_ok_and(|contents| {
805-
let expected_name_matches = contents
806-
.lines()
807-
.map(str::trim)
808-
.any(|line| line == expected_name_line);
809-
expected_name_matches && skill_contents_have_tracedecay_marker(&contents)
810-
})
811804
}
812805

813-
fn skill_contents_have_tracedecay_marker(contents: &str) -> bool {
814-
contents.lines().map(str::trim).any(|line| {
815-
line.starts_with("name: tracedecay:")
816-
|| line.starts_with("description: TraceDecay ")
817-
|| line.contains("TraceDecay MCP")
818-
|| line.contains("tracedecay_")
819-
|| line.contains("`tracedecay:")
820-
})
806+
/// True when a Codex `SKILL.md` at `skill_file` carries a tracedecay authorship
807+
/// marker, marking the skill dir as tracedecay-owned.
808+
fn skill_file_has_tracedecay_marker(skill_file: &Path) -> bool {
809+
std::fs::read_to_string(skill_file)
810+
.is_ok_and(|contents| super::skill_contents_have_tracedecay_marker(&contents))
821811
}
822812

823813
fn prune_empty_dirs(root: &Path) -> std::io::Result<()> {
@@ -881,7 +871,7 @@ fn codex_plugin_dir_is_tracedecay(install_dir: &Path) -> bool {
881871
}
882872

883873
fn codex_plugin_dir_has_only_managed_files(install_dir: &Path) -> bool {
884-
let Ok(entries) = collect_regular_files(install_dir) else {
874+
let Ok(entries) = super::collect_regular_files(install_dir) else {
885875
return false;
886876
};
887877
let managed = codex_plugin_managed_paths(install_dir);
@@ -897,25 +887,6 @@ fn codex_plugin_managed_paths(install_dir: &Path) -> Vec<PathBuf> {
897887
paths
898888
}
899889

900-
fn collect_regular_files(root: &Path) -> std::io::Result<Vec<PathBuf>> {
901-
let mut out = Vec::new();
902-
collect_regular_files_inner(root, &mut out)?;
903-
Ok(out)
904-
}
905-
906-
fn collect_regular_files_inner(root: &Path, out: &mut Vec<PathBuf>) -> std::io::Result<()> {
907-
for entry in std::fs::read_dir(root)? {
908-
let entry = entry?;
909-
let file_type = entry.file_type()?;
910-
if file_type.is_dir() {
911-
collect_regular_files_inner(&entry.path(), out)?;
912-
} else if file_type.is_file() {
913-
out.push(entry.path());
914-
}
915-
}
916-
Ok(())
917-
}
918-
919890
fn remove_codex_marketplace_entry(home: &Path) -> Result<()> {
920891
let marketplace_path = codex_personal_marketplace_path(home);
921892
remove_codex_marketplace_entry_at(&marketplace_path, "personal")
@@ -1549,7 +1520,7 @@ mod tests {
15491520
/// `codex_bundle_ships_exactly_the_model_invocable_cursor_skills` checks.
15501521
/// Every file under a skills root, relative to it, forward-slashed.
15511522
fn skill_tree_files(root: &Path) -> Vec<String> {
1552-
let mut files: Vec<String> = collect_regular_files(root)
1523+
let mut files: Vec<String> = crate::agents::collect_regular_files(root)
15531524
.expect("skills dir readable")
15541525
.into_iter()
15551526
.filter_map(|path| {

src/agents/cursor.rs

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -269,15 +269,11 @@ fn write_embedded_plugin(install_dir: &Path, tracedecay_bin: &str) -> Result<()>
269269
}
270270

271271
fn cursor_plugin_manifest(raw: &str) -> Result<String> {
272-
let mut manifest: serde_json::Value = serde_json::from_str(raw)?;
273-
manifest["version"] = json!(env!("CARGO_PKG_VERSION"));
274-
Ok(format!("{}\n", serde_json::to_string_pretty(&manifest)?))
272+
super::plugin_bundle::stamp_manifest_version(raw)
275273
}
276274

277275
fn cursor_plugin_mcp(raw: &str, tracedecay_bin: &str) -> Result<String> {
278-
let mut mcp: serde_json::Value = serde_json::from_str(raw)?;
279-
mcp["mcpServers"]["tracedecay"]["command"] = json!(tracedecay_bin);
280-
Ok(format!("{}\n", serde_json::to_string_pretty(&mcp)?))
276+
super::plugin_bundle::set_mcp_command(raw, tracedecay_bin)
281277
}
282278

283279
fn cursor_plugin_hooks(raw: &str, tracedecay_bin: &str) -> Result<String> {
@@ -397,15 +393,8 @@ fn sweep_retired_bundle_skill_dirs(install_dir: &Path) {
397393
/// True when a Cursor `SKILL.md` carries a tracedecay authorship marker, marking
398394
/// the skill dir as tracedecay-owned (and therefore safe to sweep when retired).
399395
fn skill_file_has_tracedecay_marker(skill_file: &Path) -> bool {
400-
std::fs::read_to_string(skill_file).is_ok_and(|contents| {
401-
contents.lines().map(str::trim).any(|line| {
402-
line.starts_with("name: tracedecay:")
403-
|| line.starts_with("description: TraceDecay ")
404-
|| line.contains("TraceDecay MCP")
405-
|| line.contains("tracedecay_")
406-
|| line.contains("`tracedecay:")
407-
})
408-
})
396+
std::fs::read_to_string(skill_file)
397+
.is_ok_and(|contents| super::skill_contents_have_tracedecay_marker(&contents))
409398
}
410399

411400
fn cursor_plugin_dir_is_tracedecay(install_dir: &Path) -> bool {
@@ -417,7 +406,7 @@ fn cursor_plugin_dir_is_tracedecay(install_dir: &Path) -> bool {
417406
}
418407

419408
fn cursor_plugin_dir_has_only_managed_files(install_dir: &Path) -> bool {
420-
let Ok(entries) = collect_regular_files(install_dir) else {
409+
let Ok(entries) = super::collect_regular_files(install_dir) else {
421410
return false;
422411
};
423412
let managed = cursor_plugin_managed_paths(install_dir);
@@ -433,25 +422,6 @@ fn cursor_plugin_managed_paths(install_dir: &Path) -> Vec<PathBuf> {
433422
paths
434423
}
435424

436-
fn collect_regular_files(root: &Path) -> std::io::Result<Vec<PathBuf>> {
437-
let mut out = Vec::new();
438-
collect_regular_files_inner(root, &mut out)?;
439-
Ok(out)
440-
}
441-
442-
fn collect_regular_files_inner(root: &Path, out: &mut Vec<PathBuf>) -> std::io::Result<()> {
443-
for entry in std::fs::read_dir(root)? {
444-
let entry = entry?;
445-
let file_type = entry.file_type()?;
446-
if file_type.is_dir() {
447-
collect_regular_files_inner(&entry.path(), out)?;
448-
} else if file_type.is_file() {
449-
out.push(entry.path());
450-
}
451-
}
452-
Ok(())
453-
}
454-
455425
fn legacy_mcp_has_tracedecay(mcp_path: &Path) -> bool {
456426
load_json_file(mcp_path)
457427
.get("mcpServers")
@@ -943,7 +913,7 @@ mod tests {
943913

944914
/// Every file under a single skill dir, relative to it, forward-slashed.
945915
fn skill_dir_tree_files(skill_dir: &Path) -> Vec<String> {
946-
let mut files: Vec<String> = collect_regular_files(skill_dir)
916+
let mut files: Vec<String> = crate::agents::collect_regular_files(skill_dir)
947917
.expect("skill dir readable")
948918
.into_iter()
949919
.filter_map(|path| {
@@ -1491,7 +1461,7 @@ mod tests {
14911461
std::fs::read_to_string(cursor_dir.join("rules/tracedecay.mdc")).unwrap(),
14921462
rule
14931463
);
1494-
let mut files = collect_regular_files(&cursor_dir).unwrap();
1464+
let mut files = crate::agents::collect_regular_files(&cursor_dir).unwrap();
14951465
files.sort();
14961466
assert_eq!(
14971467
files,

src/agents/kiro.rs

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -336,32 +336,11 @@ fn mcp_server_entry(tracedecay_bin: &str) -> serde_json::Value {
336336
})
337337
}
338338

339+
/// Render a path as a `file://` resource URI for Kiro's agent config. Reuses
340+
/// the LSP client's encoder, which additionally handles Windows drive paths and
341+
/// UNC (`//server/share`) prefixes; POSIX paths encode identically to before.
339342
fn file_resource_uri(path: &Path) -> String {
340-
let path = path.to_string_lossy().replace('\\', "/");
341-
let path = percent_encode_file_uri_path(&path);
342-
if path.starts_with('/') {
343-
format!("file://{path}")
344-
} else {
345-
format!("file:///{path}")
346-
}
347-
}
348-
349-
fn percent_encode_file_uri_path(path: &str) -> String {
350-
const HEX: &[u8; 16] = b"0123456789ABCDEF";
351-
let mut encoded = String::with_capacity(path.len());
352-
for byte in path.bytes() {
353-
match byte {
354-
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'/' | b':' | b'-' | b'.' | b'_' | b'~' => {
355-
encoded.push(byte as char);
356-
}
357-
_ => {
358-
encoded.push('%');
359-
encoded.push(HEX[(byte >> 4) as usize] as char);
360-
encoded.push(HEX[(byte & 0x0F) as usize] as char);
361-
}
362-
}
363-
}
364-
encoded
343+
crate::diagnostics::lsp::client::file_uri_from_path_text(&path.to_string_lossy())
365344
}
366345

367346
fn managed_agent_config(

src/agents/mod.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,40 @@ or the server is disconnected, every tool is also available as a shell command:
817817
`tracedecay tool <name> --help` shows parameters). Fall back to that CLI instead of \
818818
querying `.tracedecay` databases directly or abandoning tracedecay.";
819819

820+
/// True when a `SKILL.md`'s contents carry a tracedecay authorship marker,
821+
/// marking the skill dir as tracedecay-owned (and therefore safe to sweep when
822+
/// retired). Shared by the Cursor and Codex plugin-dir sweeps.
823+
pub(crate) fn skill_contents_have_tracedecay_marker(contents: &str) -> bool {
824+
contents.lines().map(str::trim).any(|line| {
825+
line.starts_with("name: tracedecay:")
826+
|| line.starts_with("description: TraceDecay ")
827+
|| line.contains("TraceDecay MCP")
828+
|| line.contains("tracedecay_")
829+
|| line.contains("`tracedecay:")
830+
})
831+
}
832+
833+
/// Recursively collect every regular file under `root` (following the same
834+
/// hand-rolled walk both the Cursor and Codex installers rely on).
835+
pub(crate) fn collect_regular_files(root: &Path) -> std::io::Result<Vec<PathBuf>> {
836+
let mut out = Vec::new();
837+
collect_regular_files_inner(root, &mut out)?;
838+
Ok(out)
839+
}
840+
841+
fn collect_regular_files_inner(root: &Path, out: &mut Vec<PathBuf>) -> std::io::Result<()> {
842+
for entry in std::fs::read_dir(root)? {
843+
let entry = entry?;
844+
let file_type = entry.file_type()?;
845+
if file_type.is_dir() {
846+
collect_regular_files_inner(&entry.path(), out)?;
847+
} else if file_type.is_file() {
848+
out.push(entry.path());
849+
}
850+
}
851+
Ok(())
852+
}
853+
820854
pub(crate) fn hook_command(tracedecay_bin: &str, subcommand: &str) -> String {
821855
hook_command_for_platform(tracedecay_bin, subcommand, cfg!(windows))
822856
}

0 commit comments

Comments
 (0)