fix(skills): union configured skills_dir with workspace skills instead of or_else fallback

[h3c-hexin]

Problem

The system-prompt skills block is built as:

render_available_skills_context_for_workspace(workspace)
    .or_else(|| skills_dir.and_then(render_available_skills_context))

discover_in_workspace returns Some whenever the workspace contains any skill (very common when the workspace is broad). The or_else fallback then never runs, so a skills_dir configured via EngineConfig.skills_dir is silently shadowed and embedder-provided skills never reach the model.

Fix

Add render_available_skills_context_for_workspace_and_dir, built on the existing discover_for_workspace_and_dir union, and call it when skills_dir is set so the workspace view and the configured dir merge instead of one shadowing the other.

• No behaviour change when skills_dir is None (still the workspace-only view).
• Reuses the existing union discovery; no new discovery logic.

Greptile Summary

This PR fixes a silent skill-shadowing bug: when the workspace contained any skills, the or_else chain caused an embedder-configured skills_dir to be completely ignored. The fix replaces that chain with map_or_else so the two sources are always merged when skills_dir is set.

• prompts.rs: Switches from render_available_skills_context_for_workspace(...).or_else(|| skills_dir.and_then(...)) to skills_dir.map_or_else(|| workspace-only, |dir| union), ensuring the configured dir is never shadowed.
• skills/mod.rs: Adds render_available_skills_context_for_workspace_and_dir, a thin wrapper over the already-tested discover_for_workspace_and_dir union; retires the old single-dir public entry point and introduces a private render_dir_context test helper so all existing render-level unit tests continue to exercise the same code paths.

Confidence Score: 5/5

Safe to merge — the change is a targeted, well-scoped fix with no behavior change when skills_dir is absent and correct union semantics when it is set.

The fix is structurally simple: a two-branch map_or_else dispatch whose 'no skills_dir' branch is unchanged from before, and whose union branch delegates entirely to discover_for_workspace_and_dir, which is already exercised by existing tests. The retired render_available_skills_context single-dir public function had no callers outside this crate, so removing it introduces no breakage. Discovery-layer deduplication and the is_dir() guard are both preserved. No new logic is introduced.

No files require special attention.

Important Files Changed

Filename	Overview
crates/tui/src/skills/mod.rs	Replaces the removed single-dir render_available_skills_context with the new render_available_skills_context_for_workspace_and_dir union function; updates tests to use the private render_dir_context helper so unit coverage is preserved without re-exporting the old entrypoint.
crates/tui/src/prompts.rs	Rewrites the skills-block construction from or_else fallback semantics to map_or_else union semantics: when skills_dir is set, the configured directory is always merged with workspace discovery instead of being silently shadowed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[system_prompt_for_mode_with_context_skills_session_and_approval] --> B{skills_dir set?}
    B -- No --> C[render_available_skills_context_for_workspace\nworkspace-only discovery]
    B -- Yes --> D[render_available_skills_context_for_workspace_and_dir\nunion discovery]
    D --> E[discover_for_workspace_and_dir]
    E --> F[skills_directories workspace\n.agents/skills, .claude/skills, etc.]
    E --> G{skills_dir already\nin dirs list?}
    G -- No --> H[Append skills_dir to dirs]
    G -- Yes --> I[Skip duplicate]
    H --> J[discover_from_directories\nfirst-match-wins merge]
    I --> J
    F --> J
    C --> K[discover_in_workspace]
    K --> J
    J --> L[render_skills_block]
    L --> M{registry empty?}
    M -- Yes --> N[None — no block appended]
    M -- No --> O[Some — block appended to system prompt]

Loading

Comments Outside Diff (1)

1. crates/tui/src/prompts.rs, line 1067-1076 (link)

   The sentence "The legacy single-skills_dir path is honoured as a fallback…" directly contradicts the paragraph that immediately follows it, which explains that skills_dir is no longer a fallback. The old description should be removed so only the accurate explanation remains.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

   

_{Reviews (2): Last reviewed commit: "fix(skills): remove dead render_availabl..." | Re-trigger Greptile}

[github-actions]

Thanks @h3c-hexin for taking the time to contribute.

This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in .github/APPROVED_CONTRIBUTORS will be closed automatically.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant PR access by commenting /lgtm on a pull request.

[gemini-code-assist]

Code Review

This pull request updates the system prompt generation to union the workspace skills with an explicitly configured skills_dir instead of treating the directory as a fallback, preventing the workspace view from shadowing the configured directory. To support this, a new helper function render_available_skills_context_for_workspace_and_dir was added. The feedback suggests using Option::map_or_else in prompts.rs as a more idiomatic and concise alternative to the explicit match statement.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Using Option::map_or_else is a more idiomatic and concise way to handle this fallback logic in Rust compared to an explicit match statement.

    let skills_block = skills_dir.map_or_else(
        || crate::skills::render_available_skills_context_for_workspace(workspace),
        |dir| crate::skills::render_available_skills_context_for_workspace_and_dir(workspace, dir),
    );