fix(update): resolve dot-dir subdirs in batch update before stale classification - #262
Merged
Merged
Conversation
Batch update looks up each skill's subdir in the discovery map, but discovery deliberately skips target dot-dirs (.claude, .codex, ...) to avoid counting target-synced copies as source skills. A skill installed from such a subdir therefore never appears in the map and is falsely reported as stale (deleted upstream), steering users toward --prune on a healthy skill. Fall back to a direct SKILL.md stat in the cloned repo -- mirroring the single-skill update path -- before declaring the path missing. Fixes runkids#261
runkids
self-requested a review
August 6, 2026 02:55
runkids
marked this pull request as ready for review
August 6, 2026 02:56
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
runkids
approved these changes
Aug 6, 2026
Owner
|
Thanks! |
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
2 participants
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.
Type
proposals/only — see CONTRIBUTING.md)Linked Issue
Closes #261
Checklist
make check) — for code changesSummary
update --allfalsely reports a skill asstale (deleted upstream)when the upstream repo ships the skill only inside a target dot-dir (.claude/,.codex/,.github/, ...). Batch update looks upmeta.Subdirin the discovery map, but discovery deliberately skipsTargetDotDirs, so the lookup can never succeed for such skills — and--prunewould then trash a healthy install. Full analysis in #261.Fix
On a discovery-map miss,
UpdateSkillsFromReponow falls back tolookupSkillSubdir, a directSKILL.mdstat of the subdir in the cloned repo (mirroring theresolveSubdirfast path that makes single-skillupdate <name>succeed for the same skill). Only a genuinely absent path is still reported asskill path %q not found in repositoryand classified stale. The fallback rejects../absolute subdirs and requiresSKILL.mdto exist, and fillsSkillInfolicense/description from frontmatter like regular discovery does.Tests
TestLookupSkillSubdir— resolves a skill under.claude/skills/, rejects missing dirs, dirs withoutSKILL.md, and traversal/absolute inputs.TestUpdateSkillsFromRepo_SkillOnlyInTargetDotDirNotStale— end-to-end batch update from afile://repo whose only skill lives in.claude/skills/; asserts no error and the skill installs. Fails onmainwith the stale misclassification, passes with the fix.Note: the existing dot-dir e2e coverage (
ai_docs/tests/discovery_skip_target_dotdirs_runbook.md) only exercises repos that also carry skills outside the dot-dirs, so this case was previously untested.