fix(skills): union EngineConfig.skills_dir into discovery instead of dead fallback

[h3c-hexin]

Summary

The system-prompt skills block is currently assembled as:

let skills_block = render_available_skills_context_for_workspace(workspace)
    .or_else(|| skills_dir.and_then(render_available_skills_context));

The or_else fallback never fires when for_workspace(...) returns Some — which happens whenever any home-rooted skills directory (~/.agents/skills, ~/.deepseek/skills, etc.) has content. The result: an embedder that sets EngineConfig.skills_dir to ship a bundled skill directory finds its skills silently missing from the model's catalog.

This PR adds render_available_skills_context_for_workspace_and_dir(workspace, skills_dir) that unions both and updates the skills_block assembly in prompts.rs to use it via a match when skills_dir is supplied:

let skills_block = match skills_dir {
    Some(dir) => render_available_skills_context_for_workspace_and_dir(workspace, dir),
    None => render_available_skills_context_for_workspace(workspace),
};

Behavior when skills_dir is None is identical to before (workspace-only scan).

Why

Discovered when building an embedder that ships its own skill directory via EngineConfig.skills_dir = ~/.myapp/bundle/skills. The bundle's skills never showed up in the prompt because the user's ~/.agents/skills/ already had content, satisfying Some on the workspace path. Two confusing symptoms: (1) catalog appeared "complete" so the bug was easy to miss, (2) EngineConfig.skills_dir felt like dead config.

Testing

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features (no new warnings — 2 pre-existing in runtime_log.rs + main.rs not from this PR)
• cargo test --workspace --all-features (3449 passed / 1 known-flaky env test / 4 ignored)

Adds skills_dir_unions_with_home_rooted_workspace_skills test asserting:

• home-rooted skill remains visible (no behavior change for that path)
• skills_dir-supplied skill IS unioned in (the bug this fixes)
• The public render path the prompt builder uses surfaces it too

The 1 test failure is system_prompt_skips_locale_preamble_for_english, sensitive to whether ~/.agents/skills/ has non-English skill content locally — also fails on main in the same environment, should pass in CI.

Checklist

• Updated docs or comments as needed (doc comment on the new function + explanation in the dead-fallback removal site)
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes — N/A (no UI changes)

Note on render_available_skills_context

The pre-existing pub fn render_available_skills_context(skills_dir) is no longer called by prompts.rs (which goes through the union API now). I marked it #[allow(dead_code)] and left it as public API — embedders may want single-directory rendering for ad-hoc cases. Happy to drop it if the maintainers prefer.

Greptile Summary

The core skills-union fix (prompts.rs + skills/mod.rs) is correct and well-tested: the old or_else fallback silently dropped embedder-configured skills_dir content whenever any home-rooted workspace skill was present, and the new match-based union path fixes that. However, the PR also bundles three unrelated files (config.rs, main.rs, web_search.rs) that silently flip the default SearchProvider from DuckDuckGo to Bing — a user-visible behaviour change not mentioned anywhere in the PR description.

• skills/mod.rs and prompts.rs: adds discover_for_workspace_and_dir / render_available_skills_context_for_workspace_and_dir to union workspace and embedder-supplied skill paths, and updates the prompt assembly to use a match instead of a short-circuiting or_else. A focused regression test covers both the home-rooted and bundle-supplied skill paths.
• config.rs, main.rs, web_search.rs: change SearchProvider default from DuckDuckGo to Bing, rename the canonical default-provider test to forkguard_search_provider_defaults_to_bing, and embed "pinvou3 fork (Phase 2: RLM runtime primitives (llm_query, rlm_query, js, FINAL) #42)" cross-repository issue references in production doc comments — none of which is described in the PR summary or testing sections.

Confidence Score: 3/5

Safe to merge only if the search-provider changes are intentional and understood by maintainers — they are not mentioned in the PR description and represent a silent user-visible behaviour change.

The skills-union logic is correct and the regression test is adequate. The concern is config.rs/main.rs/web_search.rs: they change the default web-search backend from DuckDuckGo to Bing without any mention in the PR description, embed cross-repository fork references in production doc comments, and rename a test with a non-standard "forkguard_" prefix. A reviewer approving the skills fix would not realise the search default also changed.

crates/tui/src/config.rs, crates/tui/src/main.rs, and crates/tui/src/tools/web_search.rs carry undisclosed changes to the search provider default and fork-specific annotations; crates/tui/src/skills/mod.rs and crates/tui/src/prompts.rs look correct.

Important Files Changed

Filename	Overview
crates/tui/src/skills/mod.rs	Adds discover_for_workspace_and_dir and render_available_skills_context_for_workspace_and_dir to union workspace + configured skills_dir; adds regression test covering the bug scenario. Logic is sound: name-dedup via first-match-wins, warnings accumulate, new test only asserts from-bundle on the public render path (correct, since from-home relies on fake_home via the test-only helper).
crates/tui/src/prompts.rs	Replaces the broken or_else fallback with a match that unions workspace and embedder-supplied skills_dir. Change is minimal, well-commented, and behaviorally identical to the old path when skills_dir is None.
crates/tui/src/config.rs	Swaps SearchProvider default from DuckDuckGo to Bing with embedded Chinese-language "pinvou3 fork (#42)" annotations and renames the canonical default test to "forkguard_search_provider_defaults_to_bing". Entirely unrelated to the skills union fix and not mentioned in the PR description.
crates/tui/src/main.rs	Updates the doctor search-provider hint and two tests to match the new Bing default from config.rs. Consistent internally but unrelated to the PR's stated skills-union purpose.
crates/tui/src/tools/web_search.rs	Updates the user-visible tool description string to swap DuckDuckGo/Bing ordering, matching the new default. Also unrelated to the PR's skills-union theme.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["system_prompt_for_mode_with_context_skills_session_and_approval(workspace, skills_dir, ...)"] --> B{skills_dir supplied?}
    B -- "Some(dir)" --> C["render_available_skills_context_for_workspace_and_dir(workspace, dir)"]
    B -- "None" --> D["render_available_skills_context_for_workspace(workspace)"]
    C --> E["discover_for_workspace_and_dir(workspace, dir)"]
    D --> F["discover_in_workspace(workspace)"]
    E --> G["skills_directories(workspace) → Vec of candidate dirs"]
    E --> H["Append skills_dir if not already present"]
    G --> I["Merge loop: SkillRegistry per dir, name-dedup first-match-wins"]
    H --> I
    F --> J["skills_directories(workspace) → Vec of candidate dirs"]
    J --> K["Merge loop: SkillRegistry per dir, name-dedup first-match-wins"]
    I --> L["render_skills_block(merged)"]
    K --> L
    L --> M{registry empty?}
    M -- "Yes" --> N["None — no skills block"]
    M -- "No" --> O["Some(String) — Skills block injected into prompt"]
    style C fill:#d4edda,stroke:#28a745
    style E fill:#d4edda,stroke:#28a745
    style H fill:#d4edda,stroke:#28a745

Loading

Comments Outside Diff (1)

1. crates/tui/src/config.rs, line 647-667 (link)

   Undisclosed default change bundled into skills PR

   The PR description covers only the skills-union fix, but these hunks silently flip SearchProvider's default from DuckDuckGo to Bing — a user-visible, breaking behaviour change for anyone relying on the documented default. The renamed test (forkguard_search_provider_defaults_to_bing) and matching changes in main.rs and web_search.rs are all downstream of this single unannounced decision. Any reviewer who reads the PR title and description would approve the skills logic without realising the default search backend also changed.

   

_{Reviews (1): Last reviewed commit: "fix(skills): union EngineConfig.skills_d..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

Fork-specific metadata embedded in production source

The doc comment references "pinvou3 fork (#42)" and "上游 PR #2132" — issue numbers that belong to a different repository's tracker. These annotations appear in the enum variant's doc comment (visible via cargo doc and IDE hovers), in the renamed test (forkguard_search_provider_defaults_to_bing), and again in main.rs. Embedding external fork issue references and a "forkguard_" guard prefix in the canonical source marks this code as intentionally non-standard in a way that will confuse future contributors and tools. If the rationale for Bing-as-default is valid for this repo, it should be expressed as a normal prose comment without cross-repository issue links.

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!

[gemini-code-assist]

Code Review

This pull request reverts the default search provider from DuckDuckGo to Bing due to connection issues in certain environments, updating configuration, doctor commands, tool descriptions, and tests accordingly. Additionally, it refactors the skill discovery logic to union the user-supplied skills_dir with workspace candidates rather than treating it as a fallback, preventing bundle-distributed skills from being hidden. Feedback suggests adding the missing metaso provider to the web search tool description and translating several newly added Chinese comments into English to maintain codebase consistency.

[gemini-code-assist]

The tool description lists the alternative search providers but misses "metaso", which is fully supported by the codebase (as seen in config.rs and main.rs). We should update the description to include "metaso" so users are aware of this option.

        "Search the web and return ranked results with URLs and snippets. Default backend is Bing with DuckDuckGo fallback; set `[search] provider = \"duckduckgo\" | \"tavily\" | \"bocha\" | \"metaso\"` in config.toml to switch backends. Use this instead of scraping search engines with `curl` in `exec_shell`. For a known canonical URL, prefer `fetch_url` directly."

[gemini-code-assist]

To maintain consistency across the codebase, it is recommended to write comments in English, as the rest of the file and repository are documented in English. This ensures better maintainability and readability for all contributors.

    /// pinvou3 fork (#42): This is the default backend. Upstream PR #2132 changed the default to DuckDuckGo, but DDG (`html.duckduckgo.com`) always returns an `anomaly-modal` for non-browser requests, which always triggers the fallback in our CN deployment. Reverting to Bing (combined with the #10 entity decoding fix) takes effect immediately.

[gemini-code-assist]

To maintain consistency across the codebase, it is recommended to write comments in English, as the rest of the file and repository are documented in English. This ensures better maintainability and readability for all contributors.

        // pinvou3 fork patch #42: Upstream PR #2132 changed the default to DuckDuckGo, but the DDG
        // HTML endpoint always returns an anomaly-modal under CN networks, which always triggers the Bing fallback
        // and results in an extra wasted request. Reverting to Bing is a distinct fork behavior to prevent silent sync loss.

[gemini-code-assist]

To maintain consistency across the codebase, it is recommended to write comments in English, as the rest of the file and repository are documented in English. This ensures better maintainability and readability for all contributors.

Suggested change

	// pinvou3 fork patch #42: 默认是 Bing scrape (上游 PR #2132 想用 DDG,
	// 但 DDG HTML 在我们的网络环境恒返 bot challenge)。Bing scrape 也会被反爬
	// 退化(中文查询易返兜底页),所以在 default 状态下提示用户可切到 API 后端。
	// pinvou3 fork patch #42: The default is Bing scrape (upstream PR #2132 intended to use DDG,
	// but the DDG HTML endpoint always returns a bot challenge in our network environment). Bing scrape can also
	// degrade due to anti-scraping (Chinese queries easily return fallback pages), so we prompt users to switch to API backends by default.

[gemini-code-assist]

To maintain consistency across the codebase, it is recommended to write comments in English, as the rest of the file and repository are documented in English. This ensures better maintainability and readability for all contributors.

Suggested change

	// pinvou3 fork patch #42: 默认是 Bing(回退上游 PR #2132),提示指向 API 后端。
	// pinvou3 fork patch #42: Default is Bing (reverting upstream PR #2132), prompting to point to the API backend.

[gemini-code-assist]

To maintain consistency across the codebase, it is recommended to write comments in English, as the rest of the file and repository are documented in English. This ensures better maintainability and readability for all contributors.

Suggested change

	// 显式选择的 provider(任意值)都不再触发 switch hint —— hint 只对
	// "default + Bing" 的兜底场景说话。pinvou3 fork patch #42 调整后,
	// 这里用 DuckDuckGo 显式配置覆盖 source=Config 路径。
	// Explicitly selected providers (any value) no longer trigger the switch hint — the hint only applies to
	// the "default + Bing" fallback scenario. After adjusting pinvou3 fork patch #42,
	// we use DuckDuckGo here as an explicit configuration to override the source=Config path.