refactor: dedup installer helpers and simplify marked-block splicing - #254
Merged
Conversation
|
ScriptedAlchemy
force-pushed
the
simplify/dedup-installers
branch
from
July 3, 2026 21:40
93ca93b to
f764f8c
Compare
ScriptedAlchemy
added a commit
that referenced
this pull request
Jul 3, 2026
…255) `broker_cancels_partial_refresh_without_poisoning_warm_client` asserts that aborting a partial refresh does not poison the broker: a subsequent refresh must spin up a clean client. On a loaded macOS CI runner the recovery client's `python3` cold-start intermittently exceeds the 3s initialize floor, surfacing a transient "initialize timed out" — a slow start, not a poisoned broker — which failed the test on two consecutive unrelated PRs (#253, #254). Retry the recovery refresh a bounded number of times (5, 50ms apart), tolerating only the transient initialize-timeout error and still failing fast on any other error. The Ready/total_errors assertions are unchanged. Co-authored-by: Claude <noreply@anthropic.com>
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>
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>
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>
- 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>
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>
ScriptedAlchemy
force-pushed
the
simplify/dedup-installers
branch
from
July 3, 2026 22:04
d00e2e0 to
60bfa38
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal: finish the Claude-deferred
/simplifyfollow-ups from the plugin-bundle/installer cleanup without duplicating merged/open work, while preserving installer safety boundaries.Detailed execution plan:
masterand verify the branch containsorigin/master.simplify/dedup-installersinstead of creating a duplicate PR.job_webhooktransport replacementgit.rsPATH resolver/broker reusesafe_relative_pathguard patternremove_rangevsprompt_rules::splice_outApplied:
perf(mcp): hook route cache now clones once under the lock viarefresh_into, removing the priorsnapshot()plusclone_fromdouble-clone.agents:skill_contents_have_tracedecay_markerand recursivecollect_regular_fileslive inagents::modinstead of cursor/codex copies.agents: Codex retired-skill sweep now derives from the live bundle keep-set, matching Cursor's safer pattern and removing the stale 19-entry list.agents: manifest version stamping and MCP command rewriting moved toplugin_bundle::{stamp_manifest_version,set_mcp_command}.skills:replace_or_append_marked_blockresolves the target range once viasplice_range;remove_all_marked_blocksderives target files viawrites_prompt_index()and reusesprompt_rules::SKILL_INDEX_START.agents: Kiro file URI handling reusesfile_uri_from_path_text, gaining the same Windows-drive and UNC behavior as LSP diagnostics.tests: the canceled-refresh recovery test now uses the existing recovery timeout on the recovery leg, matching nearby crash-recovery tests and avoiding macOS full-suite spawn/scheduling flakes.Deliberately deferred, after re-check:
job_webhooktoureq: not safe. The hand-rolled transport connects to a pre-validatedSocketAddrto preserve SSRF/DNS-rebind pinning.git.rsPATH resolver to broker: not applicable. Broker answersbool; the resolver needs the actual path.safe_relative_path: left alone as the existing codebase-wide guard pattern.remove_rangetoprompt_rules::splice_out: left alone.remove_rangepreserves trailing-newline semantics for managed skill files;splice_outtrims final whitespace.Validation:
scripts/check-conventional-commits.sh origin/master..HEAD: passcargo fmt --all --check: passcargo clippy --all-targets -- -D warnings: passcargo build: passcargo nextest run --lib: 795 passedcargo nextest run --test agent_suite: 427 passedcargo nextest run --test core_cli_suite: 189 passedcargo nextest run --test hooks_lsp_suite broker_cancels_partial_refresh_without_poisoning_warm_client: passcargo nextest run --test hooks_lsp_suite: 116 passedProduction code tests were not modified; the only test change widens an already-designated recovery path timeout.