Repository navigation
Choose skill prefixes at install without losing local data - #75
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85d53d4eb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ElliotDrel
left a comment
There was a problem hiding this comment.
Found one regression-test correctness issue in the failed-replacement safety check.
What Changed and Why
Issue: Users need a persistent choice between prefixed and short skill names. Switching names must preserve saved state, credentials, and local changes.
Why this change was necessary: The earlier implementation retired old folders before replacements succeeded and rewrote some storage identifiers. Those paths could lose edits or orphan saved state.
New status quo:
--no-prefix/--prefixpersist the choice in the shared.env; updates reconcile folders, frontmatter, references, links, and hooks. Storage names stay canonical. Replacements back up old files and preserve credentials before retiring old names. Silent updates defer switches requiring review; skipped notify switches retain a valid hook. Dry runs write nothing, conflicting flags fail early, and preflight understands short names. Title-routing work is excluded.Verification
All three review findings were fixed, replied to, and resolved. The failure injector is parsed before execution and requires the exact intended write error, then verifies original content and manifest preservation.
The clean release checkout passed both installer suites, five validation gates, both migration smoke tests, and pytest: 195 passed, 1 skipped. PR CI passed on Node 18.0.0 and 24, on Windows and Ubuntu. The v1.1.0 publish workflow passed every release gate and published with signed provenance at
265a8f8.Configuration
Prefixed remains the default. Set
ESTACK_SKILL_PREFIX=nonein~/.e-stack/.envor pass--no-prefix;--prefixrestores the default. Live environment overrides the file. Deferred silent switches require a manual installer run. No production skill install was performed.After merge
Merged and published
elliot-stack@1.1.0under Elliot's authorization. Verified the public npm registry serves version 1.1.0 and its tarball, and the latest tag is 1.1.0. PR monitoring is stopped.