fix: build skill allowlist from session worktree, not stale base clone#1337
Open
Junveloper wants to merge 1 commit into
Open
fix: build skill allowlist from session worktree, not stale base clone#1337Junveloper wants to merge 1 commit into
Junveloper wants to merge 1 commit into
Conversation
9983fda to
057e4b3
Compare
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
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.
Fixes #1336.
Problem
For single-repo (and GitHub-mention) agent sessions, the per-session skill allowlist is built by globbing
.claude/skills/in the base clone's working tree (repository.repositoryPath). Cyrus never advances that working tree —GitService.createSingleRepoWorktreeonly runsgit fetch originand cuts each worktree fromorigin/<baseBranch>. So the allowlist reflects a frozen snapshot of the repo from whenever it was registered, while the SDK loads the actual skill definitions from the fresh worktree (the session cwd, withsettingSources: ["user","project","local"]).Result: any repo-committed skill added to the base branch after the repo was registered is loaded and offered to the model by the SDK, but is absent from the allowlist, so invoking it fails with:
This degrades silently over time — the longer a repo has been registered, the more of its newer skills are unusable. (Observed in the wild: a base clone 584 commits behind
origin/main, withshipped/exposerejected even though both load fine in the worktree.)This is the same class of bug as the recently-fixed setup/teardown hook discovery (CYPACK-1337, #1332) — reading from the persistent base checkout instead of the issue worktree.
Fix
EdgeWorker.resolveSkillRepoPathsnow prefers the session's worktree path (workspace.path) — the same path the SDK uses as cwd — overrepository.repositoryPath:The multi-repo branch already used worktree sub-paths from
workspace.repoPaths— only the single-repo fallback was wrong. Therepository.repositoryPathfallback is retained for the case where no session workspace exists yet.Test plan (TDD)
Added
packages/edge-worker/test/EdgeWorker.skill-repo-paths.test.ts:workspace.path(the worktree), notrepository.repositoryPath(this assertion failed before the fix — confirmed RED → GREEN)workspace.repoPathssub-worktreerepository.repositoryPathVerification:
vitest runfor the fulledge-workerpackage: 724 passed (64 files).tsc --noEmit(repo-wide typecheck via pre-commitpnpm -r typecheck): clean.biome check: changed files clean (pre-existing repo warnings untouched).CHANGELOG.mdupdated under## [Unreleased] → ### Fixed.Notes / out of scope
~/.claude/skills/, even though the SDK loads those viasettingSources: "user". Skills kept only in~/.claude/skills/would still hit the allowlist error. Happy to follow up separately if desired.