packages/coding-agent/test/skills.test.ts believes it has isolated the user skill scope, but the loader reads a different home than the one the test mocks. The suite therefore observes whatever is in the developer's real home directory, and its result is not reproducible across machines.
Evidence
The test isolates by spying on os.homedir() — skills.test.ts:229, :404, :434:
const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(tempHome);
The loader does not read that. packages/coding-agent/src/extensibility/skills.ts:79 calls getTrustedHomeDir(), which resolves through packages/utils/src/dirs.ts:236-245:
const info = os.userInfo();
...
const home = info.homedir;
os.userInfo().homedir is unaffected by an os.homedir() spy, so the mock never intercepts the path the loader takes.
Measured, on current dev (ba7096f7)
| environment |
result |
my real $HOME (has skills installed) |
31 pass / 5 fail — extra skills discovered |
empty isolated HOME=/tmp/... |
33 pass / 3 fail — expected user skills missing |
Failing in both directions, with opposite causes, is the signature of ambient dependence rather than a normal bug. Examples: discovers native project and user skills with zero configuration expects exactly ["project-skill", "user-skill"] and sees four entries against a populated home and zero against an empty one; project scope shadows user scope expects 2 name-collision warnings and sees 1.
It passes on CI only because the runner's home happens to be inert.
Why it is not a one-line fix
Setting process.env.HOME in the test would work on macOS — Bun resolves os.userInfo().homedir from $HOME there — but not on Linux, where os.userInfo() reads the passwd entry and ignores $HOME. That asymmetry is already documented in dirs.ts and was the root cause behind #4766.
So closing this needs a decision I do not want to make unilaterally:
- give
getTrustedHomeDir() an explicit test seam, the way getPluginsDir(home?) already takes an optional home that short-circuits the resolver (dirs.ts:521-535); or
- have the skills loader accept an injected home in its options, so the test supplies it directly and production keeps the trusted resolver.
Option 2 keeps the trust boundary untouched, which matters given how much recent work went into making that resolver fail closed.
Impact
Not a user-facing defect — this is test hygiene. But it means skills.test.ts currently provides no real guarantee about user-scope discovery, and any developer running the suite locally sees failures unrelated to their change. I hit exactly that while trying to reproduce a CI shard failure and had to rule it out before I could attribute anything.
Found while reviewing as @probepark; not filing a PR because the seam choice is a design decision.
packages/coding-agent/test/skills.test.tsbelieves it has isolated the user skill scope, but the loader reads a different home than the one the test mocks. The suite therefore observes whatever is in the developer's real home directory, and its result is not reproducible across machines.Evidence
The test isolates by spying on
os.homedir()—skills.test.ts:229,:404,:434:The loader does not read that.
packages/coding-agent/src/extensibility/skills.ts:79callsgetTrustedHomeDir(), which resolves throughpackages/utils/src/dirs.ts:236-245:os.userInfo().homediris unaffected by anos.homedir()spy, so the mock never intercepts the path the loader takes.Measured, on current
dev(ba7096f7)$HOME(has skills installed)HOME=/tmp/...Failing in both directions, with opposite causes, is the signature of ambient dependence rather than a normal bug. Examples:
discovers native project and user skills with zero configurationexpects exactly["project-skill", "user-skill"]and sees four entries against a populated home and zero against an empty one;project scope shadows user scopeexpects 2 name-collision warnings and sees 1.It passes on CI only because the runner's home happens to be inert.
Why it is not a one-line fix
Setting
process.env.HOMEin the test would work on macOS — Bun resolvesos.userInfo().homedirfrom$HOMEthere — but not on Linux, whereos.userInfo()reads the passwd entry and ignores$HOME. That asymmetry is already documented indirs.tsand was the root cause behind #4766.So closing this needs a decision I do not want to make unilaterally:
getTrustedHomeDir()an explicit test seam, the waygetPluginsDir(home?)already takes an optionalhomethat short-circuits the resolver (dirs.ts:521-535); orOption 2 keeps the trust boundary untouched, which matters given how much recent work went into making that resolver fail closed.
Impact
Not a user-facing defect — this is test hygiene. But it means
skills.test.tscurrently provides no real guarantee about user-scope discovery, and any developer running the suite locally sees failures unrelated to their change. I hit exactly that while trying to reproduce a CI shard failure and had to rule it out before I could attribute anything.Found while reviewing as @probepark; not filing a PR because the seam choice is a design decision.