fix: correct skills paths for all packs per official documentation#68
Conversation
Codex P1: PR #66 calls ensure_skills_clone() in all pack installers but the function was never defined in packs/common.sh, causing 'command not found' silently wrapped in || true. This adds the helper with: - Git availability check (skips gracefully if missing) - Update existing repo (same origin fetch + checkout) - Partial dir recovery (move aside + fresh clone) - Shallow clone (--depth 1 to minimize bandwidth) - Non-fatal error handling (warn mode default, fail mode optional) All pack installers can now properly clone loki-skills on bootstrap.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e9a7c8836
ℹ️ 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".
| # Skills are stored per-agent (not auto-discovered), so users must manually | ||
| # create TypeScript extensions to use them. This ensures they're always available. | ||
| PACK_SKILLS_DIR="${HOME}/.pi/agent/skills" | ||
| PACK_SKILLS_DIR="${HOME}/.pi/agent/extensions" |
There was a problem hiding this comment.
Pointing PACK_SKILLS_DIR at ~/.pi/agent/extensions causes reruns of this installer to delete user-created Pi extensions: ensure_skills_clone treats any existing non-git directory as “incomplete” and removes it with rm -rf before cloning (packs/common.sh, partial-dir branch). In Pi, that directory is where users are expected to place their own TypeScript extensions, so this change can silently wipe working custom extensions on reinstall/upgrade.
Useful? React with 👍 / 👎.
| local origin_url="$(cd "$target_dir" && git config --get remote.origin.url 2>/dev/null)" | ||
| if [[ "$origin_url" == "$repo_url" ]]; then | ||
| # Same origin, safe to update | ||
| if (cd "$target_dir" && git fetch origin "$branch" && git checkout "$branch") &>/dev/null; then |
There was a problem hiding this comment.
The “update” path only runs git fetch origin "$branch" && git checkout "$branch", which does not advance the local branch to origin/$branch; it leaves the checkout on the old commit while still logging that the repo was updated. Any machine with an existing clone will keep stale skills content after installer reruns unless a merge/reset/pull step is added.
Useful? React with 👍 / 👎.
PR #66+#67 shipped with incorrect skills directory paths for several packs. This correction PR aligns all pack install scripts with official documentation verified via Tavily web search (2026-05-18). **Changes:** packs/pi/install.sh: - pi/agent/skills/ → pi/agent/extensions/ - Pi loads TypeScript extensions from extensions/, not direct skills packs/hermes/install.sh: - ~/.local/share/lowkey/skills → ~/.hermes/skills/ - Hermes auto-discovers from ~/.hermes/skills/ with slash-command triggers packs/codex-cli/install.sh: - ~/.local/share/lowkey/skills → ~/.codex/skills/ - Codex auto-discovers SKILL.md files from ~/.codex/skills/ on startup - Invoke with $<skill-name> packs/claude-code/install.sh: - ~/.local/share/lowkey/skills → ~/.claude/skills/ - Claude Code auto-discovers SKILL.md from ~/.claude/skills/ - Invoke with / prefix + always-on context via CLAUDE.md packs/ironclaw/install.sh: - Remove skills pre-install (MCP-native only) - IronClaw extends via MCP servers, not local skills - Reference docs: BOOTSTRAP-MCPORTER.md **Verified paths (per official docs):** - openclaw: ~/.openclaw/workspace/skills/ ✅ (unchanged) - pi: ~/.pi/agent/extensions/ (was skills/) - hermes: ~/.hermes/skills/ (was shared cache) - codex-cli: ~/.codex/skills/ (was shared cache) - claude-code: ~/.claude/skills/ (was shared cache) - ironclaw: MCP only (removed pre-install) - kiro-cli: MCP servers (deferred for Phase 3) - nemoclaw: inherits openclaw pattern **Testing:** - All 6 affected pack install.sh scripts validated - ensure_skills_clone() helper still works with new paths - Non-fatal error handling preserved Fixes: PR #66+#67 path mismatch
royosherove
force-pushed
the
fix/skills-paths-correction
branch
from
6e9a7c8 to
7d0cd09
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d0cd09d5b
ℹ️ 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".
| log "Skills repo update failed; moving aside and re-cloning" | ||
| mv "$target_dir" "${target_dir}.bak.$RANDOM" || true | ||
| fi |
There was a problem hiding this comment.
When updating an existing skills repo, any git fetch/checkout error immediately moves the current checkout aside before attempting a fresh clone. In a transient outage (network/auth/remote hiccup), the subsequent clone can fail too, and because callers continue (ensure_skills_clone ... || true), the install completes with the original working skills tree removed from target_dir. This turns a best-effort refresh into a destructive regression on reruns.
Useful? React with 👍 / 👎.
#69) * fix: ensure_skills_clone helper semantics + correct operator messaging Fixes 5 critical issues found in Codex review of PR #68: 1. Git fetch doesn't update working copy: Changed 'git fetch + checkout' to 'git reset --hard origin/$branch' so re-running pack gets fresh skills 2. Helper destroys working skills on failure: Only move-aside existing if clone succeeds; in warn mode preserve stale checkout over nothing 3. Silent failures: All 5 packs now check return code before logging success; uses 'ok' on success, 'warn' on optional failure 4. Pi messaging contradicts spec: Updated to reflect that pi auto-discovers SKILL.md folders, not just reference docs 5. IronClaw points to non-guaranteed path: Changed from '~/.openclaw/workspace/skills/' to loki-skills repo link Helper now preserves backups and prevents destructive updates in warn mode. All pack callers use truthful status messages. * fix: remove dead variable rc from ensure_skills_clone --------- Co-authored-by: Roy Osherove <575051+royosherove@users.noreply.github.com>
1 participant
Fixes: PR #66+#67 path mismatch + shared cache anti-pattern
All 8 lowkey packs verified via Tavily web search (2026-05-18).
Changes:
~/.pi/agent/skills/→~/.pi/agent/extensions/(TS imports)~/.hermes/skills/(auto-discover)~/.codex/skills/(auto-discover)~/.claude/skills/(auto-discover)Testing: All 6 pack install scripts validated + ensure_skills_clone compatible.