🤖 fix: include universal skill roots in project-runtime discovery

[ThomasK33]

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.

---

📋 Implementation Plan

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.

Why the current behavior is surprising

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.

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:

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.

---

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

[ThomasK33]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7acfa8a72

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ThomasK33]

@codex review

Addressed the feedback: expanded the project-local branch to also discover from .agents/skills and ~/.agents/skills, matching the runtime branch and the tool description. Added 2 new local-path tests (24 total, all green).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2403977775

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ThomasK33]

@codex review

Fixed: now using os.homedir() instead of path.dirname(muxScope.muxHome) for the universal root derivation. Updated tests to explicitly control os.homedir() via spy, and added a regression test with custom MUX_ROOT. All 24 tests green.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".