Skip to content

fix(skills-init): skip existing Git skills - #2655

Open
Tethys0 wants to merge 1 commit into
kagent-dev:mainfrom
Tethys0:fix/skills-init-retry-2646
Open

fix(skills-init): skip existing Git skills#2655
Tethys0 wants to merge 1 commit into
kagent-dev:mainfrom
Tethys0:fix/skills-init-retry-2646

Conversation

@Tethys0

@Tethys0 Tethys0 commented Sep 1, 2026

Copy link
Copy Markdown

Summary

Fixes #2646.

Make Git skill downloads reentrant across skills-init restarts: a destination that already contains a real, regular SKILL.md is treated as a completed skill and is not downloaded again. This lets a restart continue after another initialization step fails without re-cloning already materialized skills.

Existing destinations are validated before they are accepted: the destination and SKILL.md must not be symbolic links, the destination must be a directory, and SKILL.md must be a regular file. Invalid existing destinations return a clear error rather than being silently accepted.

Scope

  • Git ref fetches only.
  • OCI and S3 destinations are unchanged.
  • No new dependencies or API changes.

Testing

  • go test -race ./core/internal/skillsinit (40 passed)
  • go vet ./core/internal/skillsinit
  • gofmt and git diff --check
  • Independent final code review: passed with no blocking security or logic findings.

@Tethys0
Tethys0 requested a review from a team as a code owner September 1, 2026 16:14
@github-actions github-actions Bot added the bug Something isn't working label Sep 1, 2026

@EItanya EItanya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we just make the skills download code reentrant safe? We can just check for the presence of an existing skill, and if it exists don't download the new one

Signed-off-by: Bo Zhang <187063395+Tethys0@users.noreply.github.com>
@Tethys0
Tethys0 force-pushed the fix/skills-init-retry-2646 branch from d89039e to 1f12719 Compare September 2, 2026 23:28
@Tethys0

Tethys0 commented Sep 2, 2026

Copy link
Copy Markdown
Author

Thanks — I adopted the reentrant-download approach. The updated implementation now skips a destination only when it already contains a real SKILL.md; invalid or symlinked destinations are rejected rather than treated as successful downloads.

Added focused coverage for an existing skill, an incomplete destination, and symlinked paths. Verified with go test -race ./core/internal/skillsinit and go vet ./core/internal/skillsinit.

This force-update also replaces the earlier staging/replacement implementation.

@Tethys0 Tethys0 changed the title fix(skills-init): allow Git retries after stale destinations fix(skills-init): skip existing Git skills Sep 2, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skills-init: not idempotent across init-container restarts — leftover clone dest bricks the pod until manual deletion

2 participants