🤖 fix: include universal skill roots in project-runtime discovery (#3170)

## Summary

Broadens `project-runtime` skill discovery so `agent_skill_list`
discovers skills from all four standard roots — `.mux/skills`,
`.agents/skills`, `~/.mux/skills`, and `~/.agents/skills` — instead of
only the two `.mux/skills` roots. Hidden (`advertise: false`) skills
from the newly-included legacy roots follow the same filter: omitted by
default, included with `includeUnadvertised: true`.

## Background

The `project-runtime` branch in `agent_skill_list.ts` constructed a
custom root object containing only `.mux/skills` and `~/.mux/skills`,
deliberately excluding the legacy/universal `.agents/skills` and
`~/.agents/skills` roots. This meant skills placed under those legacy
roots were invisible to `agent_skill_list` in runtime mode, even with
`includeUnadvertised: true`. The shared root model
(`getDefaultAgentSkillsRoots()`) already included all four roots, so the
runtime branch was an undocumented narrower fork.

## Implementation

**Production code** (`agent_skill_list.ts`): Replaced the hand-built
runtime root object with `getDefaultAgentSkillsRoots(skillCtx.runtime,
skillCtx.workspacePath)` (~net -6 LoC). This aligns runtime discovery
with the shared root contract while keeping `dedupeByName: false` and
containment unchanged.

**Tests** (`agent_skill_list.test.ts`):
- Flipped the two existing exclusion tests (lines ~664-717 and ~779-815)
to inclusion tests, asserting that `.agents/skills` and
`~/.agents/skills` skills are now present with correct scope tags.
- Added 4 new test cases for hidden-skill coverage in both legacy roots
(absent by default, present with `includeUnadvertised: true`).

**Tool description** (`toolDefinitions.ts`): Updated to document all
four discovery roots.

## Validation

- All 22 `agent_skill_list` tests pass (82 expect() calls)
- `make typecheck` ✅
- `make lint` ✅
- `make test` — 87 pre-existing failures in unrelated files
(HeartbeatSection, GeneralSection, RetryBarrier, BrowserToolbar, etc.),
none in changed files
- Integration dogfood script exercising actual production discovery with
real filesystem operations confirmed correct behavior

## Risks

Low risk. The change unifies an already-divergent code path with the
shared root model. All existing containment, deduplication, and
scope-tagging behavior is preserved. The only behavioral change is that
skills under `.agents/skills` and `~/.agents/skills` are now visible in
runtime mode.


---

<details>
<summary>📋 Implementation Plan</summary>

# Plan: Make `agent_skill_list({ includeUnadvertised: true })` discover
legacy project and global universal skills

## Objective

Broaden `project-runtime` discovery so hidden legacy skills from both:

- project universal root `.agents/skills`
- global universal root `~/.agents/skills`

are discovered and then filtered consistently alongside `.mux/skills`
and `~/.mux/skills`, while keeping the tool description and tests
aligned with that broader contract.

## Verified current state

- `src/node/services/tools/agent_skill_list.ts:166-183` has a special
`project-runtime` branch that constructs a custom root set containing
only:
  - `projectRoot = .mux/skills`
  - `globalRoot = ~/.mux/skills`
- That branch **does not include** `projectUniversalRoot =
.agents/skills` or `universalRoot = ~/.agents/skills`, even though
`src/node/services/agentSkills/agentSkillsService.ts:39-52` defines both
as standard discovery roots.
- `src/node/services/tools/agent_skill_list.test.ts:664-717` currently
codifies exclusion of `~/.agents/skills` in `project-runtime` by
expecting the legacy tilde global skill to be missing.
- `src/node/services/tools/agent_skill_list.test.ts:779-815` currently
codifies exclusion of `.agents/skills` in `project-runtime` by expecting
the legacy project-universal skill to be missing.
- The advertise filter itself is already correct for discovered skills:
  - `src/node/services/tools/agent_skill_list.ts:182`
- it only decides whether discovered `advertise: false` skills are
filtered out.
- The repo already contains an unadvertised project skill at
`.mux/skills/deep-review/SKILL.md`, and local invocation returns it when
`includeUnadvertised: true`, which confirms the issue is about
**discovery scope**, not the `advertise` filter.

<details>
<summary>Why the current behavior is surprising</summary>

The tool description in
`src/common/utils/tools/toolDefinitions.ts:1189-1198` says:

- project workspaces list project skills from `.mux/skills/` and global
skills from `~/.mux/skills/`
- `includeUnadvertised: true` includes skills with `advertise: false`

A caller can reasonably infer that hidden project and global legacy
skills should appear whenever they are part of the runtime-visible skill
surface. In practice, `project-runtime` has an additional, undocumented
root restriction, so `.agents/skills` and `~/.agents/skills` entries
never reach the filter stage.

</details>

## Recommended approach

### Approach B — Broader compatibility fix (**recommended per requested
scope**)
**Net product-code estimate:** **~-2 to +4 LoC**  
**Expected test-only delta:** ~35-65 LoC

Replace the hand-built runtime root object with
`getDefaultAgentSkillsRoots(skillCtx.runtime, skillCtx.workspacePath)`
so `project-runtime` discovers the full standard set of roots:

- `.mux/skills`
- `.agents/skills`
- `~/.mux/skills`
- `~/.agents/skills`

#### Why this is now the right tradeoff
- Matches the requested scope expansion instead of stopping at
project-only legacy skills.
- Reuses the existing shared source of truth for skill roots rather than
maintaining a partial runtime-only fork.
- Makes `includeUnadvertised` behavior easier to explain: it affects all
discovered roots, including both universal roots.

#### Concrete code change
In `src/node/services/tools/agent_skill_list.ts:169-177`, remove the
custom runtime-only root object and instead call:

```ts
const roots = getDefaultAgentSkillsRoots(skillCtx.runtime, skillCtx.workspacePath);
```

Then pass `roots` into `discoverAgentSkills(...)`.

#### Required comment update
Update the nearby comment so it no longer claims runtime mode excludes
`.agents/skills` / `~/.agents/skills`. The comment should say runtime
mode now aligns with the shared discovery root model while paired
mutation tools may still target a narrower subset of those roots.

## Alternative approaches

### Approach A — Smaller project-only fix
**Net product-code estimate:** **~4-8 LoC**  
**Expected test-only delta:** ~25-45 LoC

Keep the runtime-specific root object, but add only
`projectUniversalRoot = .agents/skills` and continue excluding
`~/.agents/skills`.

#### Pros
- Smaller behavior change.
- Lower risk of surfacing unexpected legacy global skills in
remote/runtime sessions.

#### Cons
- No longer matches the requested broader scope.
- Leaves runtime discovery split across two subtly different root
definitions.

### Approach C — Doc-only clarification
**Net product-code estimate:** **~1-4 LoC**  
**Expected test-only delta:** 0-10 LoC

Leave behavior unchanged and only update
`src/common/utils/tools/toolDefinitions.ts` to clarify that:
- `includeUnadvertised` affects only **discovered** skills, and
- `project-runtime` may exclude legacy read-only roots.

#### Pros
- Lowest implementation risk.

#### Cons
- Does not solve the reported issue.
- Leaves an existing behavior mismatch in place for anyone expecting
hidden legacy skills to appear.

## Implementation plan

### Phase 1 — Unify `project-runtime` with the shared root model
**Files:**
- `src/node/services/tools/agent_skill_list.ts`

**Steps:**
1. Replace the hand-built runtime root object with
`getDefaultAgentSkillsRoots(skillCtx.runtime, skillCtx.workspacePath)`.
2. Keep `dedupeByName: false` unchanged so same-name skills from
different scopes/roots continue surfacing as separate descriptors.
3. Keep the existing runtime-aware path normalization and global-runtime
handling intact by relying on `discoverAgentSkills(...)` plus the shared
root helper.
4. Update the inline comment to describe the new contract precisely:
runtime listing now includes both universal roots even if mutation tools
remain narrower.

**Phase gate:**
- Run the targeted unit file immediately after this edit and confirm the
old exclusion tests fail before rewriting expectations.

### Phase 2 — Recast tests around the broader runtime contract
**Files:**
- `src/node/services/tools/agent_skill_list.test.ts`

**Steps:**
1. Update the test at `:664-717` so `~/.agents/skills` is intentionally
included in `project-runtime` rather than excluded.
2. Update the test at `:779-815` so `.agents/skills` is intentionally
included in `project-runtime` rather than excluded.
3. Add a `project-runtime` hidden-skill case for `.agents/skills`:
   - create `.agents/skills/<name>/SKILL.md` with `advertise: false`
   - call `agent_skill_list({})` and assert the skill is **absent**
- call `agent_skill_list({ includeUnadvertised: true })` and assert the
skill is **present** with `advertise: false`
4. Add a parallel `project-runtime` hidden-skill case for
`~/.agents/skills` with the same default-hidden / include-when-requested
assertions.
5. Preserve or extend existing coverage for duplicate names, host-global
skills, and containment behavior so the broader root set does not
regress those paths.

**Phase gate:**
- Re-run the targeted test file until the new runtime contract is fully
green.

### Phase 3 — Update the tool description to match the broader discovery
surface
**Files:**
- `src/common/utils/tools/toolDefinitions.ts`

**Steps:**
1. Update the `agent_skill_list` description so project workspaces
mention both modern and legacy roots:
   - project: `.mux/skills/`, `.agents/skills/`
   - global: `~/.mux/skills/`, `~/.agents/skills/`
2. Keep the `includeUnadvertised` wording explicit that it applies to
discovered skills with `advertise: false`.
3. Avoid duplicating too much implementation detail; a brief legacy-root
note is enough.

**Phase gate:**
- Re-run lint/typecheck/static checks after the description update.

## Acceptance criteria

### For the recommended Approach B
- In `project-runtime`, a visible skill located at
`.agents/skills/<name>/SKILL.md` is listed with `scope: "project"`.
- In `project-runtime`, a visible skill located at
`~/.agents/skills/<name>/SKILL.md` is listed with `scope: "global"`.
- Hidden skills under both `.agents/skills` and `~/.agents/skills` are
omitted by default.
- The same hidden skills under both universal roots appear when
`includeUnadvertised: true` is passed.
- Existing `.mux/skills` and `~/.mux/skills` behavior remains unchanged.
- Duplicate-name project/global behavior remains unchanged.
- Existing containment protections continue blocking escaped/symlinked
project roots.

### If Approach A is chosen instead
- Same as above, except only `.agents/skills` is newly included and
`~/.agents/skills` remains intentionally excluded.

### If Approach C is chosen instead
- The tool description makes the discovery-vs-filter distinction
explicit and no longer implies that `includeUnadvertised` broadens which
roots are scanned.

## Validation plan

### Fast feedback loop
- Run the focused unit file first:
  - `bun test src/node/services/tools/agent_skill_list.test.ts`

### Required local validation before calling the work complete
- `make static-check`
- `make test`

### If runtime-root behavior or shared discovery code is refactored more
broadly than planned
- Re-run:
  - `make typecheck`
  - `make lint`
- If `toolDefinitions.ts` changes, keep `make static-check` in the
required set.

## Dogfooding and reviewer-verifiable evidence

### Setup
1. Launch an isolated desktop environment using the existing sandbox
workflow (prefer the `dev-desktop-sandbox` skill or an equivalent
isolated `make dev` setup).
2. Prepare a project workspace whose storage authority resolves to
`project-runtime`.
3. Seed the workspace with:
   - one visible project skill under `.mux/skills`
- one hidden project skill under `.agents/skills` (`advertise: false`)
- one hidden global legacy skill under `~/.agents/skills` (`advertise:
false`)
- optionally one visible global legacy skill under `~/.agents/skills` to
prove the broader root is actually scanned

### Manual dogfood flow
1. Invoke `agent_skill_list({})` and confirm both hidden legacy skills
are absent.
2. Invoke `agent_skill_list({ includeUnadvertised: true })` and confirm
the hidden `.agents/skills` project skill is present.
3. In the same invocation, confirm the hidden `~/.agents/skills` global
skill is also present.
4. Confirm same-name project/global skills still appear as separate
entries if that scenario is seeded.

### Evidence to capture
- **Screenshot 1:** tool result without `includeUnadvertised`, showing
both hidden universal-root skills absent.
- **Screenshot 2:** tool result with `includeUnadvertised: true`,
showing the hidden `.agents/skills` project skill present.
- **Screenshot 3:** tool result with `includeUnadvertised: true`,
highlighting the hidden `~/.agents/skills` global skill present.
- **Video recording:** short end-to-end capture of the desktop/app
interaction from workspace setup to the two tool invocations and result
inspection.
- Attach the screenshots (and video, if supported) in the final handoff
so a reviewer can verify the behavioral contract without rerunning the
environment.

## Risks and watchpoints

- The current code comments and two existing tests encode the old
exclusion behavior; update them together or the repo will contain
contradictory intent.
- Broadening runtime discovery to `~/.agents/skills` is intentional in
this plan; verify that remote/runtime sessions still present a sensible
skill list and do not regress existing host-global behavior.
- Preserve runtime path normalization for both universal roots; using
host-local path joins in the runtime branch would break SSH/remote
behavior.
- Preserve containment coverage so project-controlled paths cannot
escape the workspace boundary.

## Recommended execution order

1. Implement Approach B in `agent_skill_list.ts` by switching the
runtime branch to `getDefaultAgentSkillsRoots(...)`.
2. Update the runtime-specific exclusion tests and add hidden-skill
coverage for both universal roots.
3. Run the targeted unit file.
4. Update `toolDefinitions.ts` so the public description matches the
broader behavior.
5. Run `make static-check` and `make test`.
6. Dogfood in an isolated desktop sandbox and capture screenshots +
video.
7. Hand off with the captured evidence and note that `project-runtime`
now treats both `.agents/skills` and `~/.agents/skills` as part of its
discovery surface.

</details>

---

_Generated with `mux` • Model: `anthropic:claude-opus-4-6` • Thinking:
`xhigh` • Cost: `$6.20`_

<!-- mux-attribution: model=anthropic:claude-opus-4-6 thinking=xhigh
costs=6.20 -->

Author: ThomasK33

src/common/utils/tools/toolDefinitions.ts

@@ -1192,7 +1192,7 @@ export const TOOL_DEFINITIONS = {
   },
   agent_skill_list: {
     description:
-      "List available skills. In a project workspace, lists both project skills (.mux/skills/) and global skills (~/.mux/skills/), each tagged with its scope. In the system workspace, lists global skills only.",
+      "List available skills. In a project workspace, lists project skills from .mux/skills/ and legacy/universal .agents/skills/, plus global skills from ~/.mux/skills/ and legacy/universal ~/.agents/skills/, each tagged with its scope. In the system workspace, lists global skills only.",
     schema: z
       .object({
         includeUnadvertised: z