feat: trigger-wording check — detect skills that should have loaded but didn't

[pinchy-SA]

What does this PR do?

Skills load on-demand via description match (the Use when... trigger line). If a skill description does not match the task phrasing, the skill never fires. This miss is invisible by definition — the self-repair loop only fires after a successful load, so a badly-worded trigger creates a permanent blind spot that never surfaces.

This is the core weakness of lazy-loaded skills vs always-injected rules: guaranteed recall is traded for context efficiency, but the cost is silent misses on trigger-wording bugs.

This PR adds a TRIGGER-WORDING CHECK to the background review prompts. After the existing review pass, the reviewer now also:

1. Lists all skills (skills_list)
2. Compares descriptions against the conversation
3. If a skill covers the territory but was never loaded via skill_view, patches its trigger description to match the actual task phrasing
4. Logs the miss: TRIGGER FIX: <skill-name> — missed for <task summary>

This surfaces the previously-invisible miss and fixes it at the source (the description), not the content. No new hooks, tools, env vars, or config keys — the background review fork already runs after every turn with memory + skills toolsets. Adding a new review dimension to the prompt is the smallest-footprint change that closes the gap.

Related Issue

Fixes #429

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• agent/background_review.py — +21 lines to _SKILL_REVIEW_PROMPT, +21 lines to _COMBINED_REVIEW_PROMPT (identical block added to both prompts)
• tests/run_agent/test_review_prompt_class_first.py — +45 lines: 2 new behavior tests + shared assertion helper

How to Test

1. Run the full background-review test suite:

   pytest tests/run_agent/test_review_prompt_class_first.py -v

2. Verify the 2 new tests pass:

   tests/run_agent/test_review_prompt_class_first.py::test_skill_review_prompt_has_trigger_wording_check PASSED
   tests/run_agent/test_review_prompt_class_first.py::test_combined_review_prompt_has_trigger_wording_check PASSED
   

3. Run the entire test suite to confirm no regressions:

   pytest tests/ -q

   All 43 existing background-review tests pass plus 2 new tests (45 total).

Checklist

Code

• I have read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this is not a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I have run pytest tests/ -q and all tests pass
• I have added tests for my changes (required for bug fixes, strongly encouraged for features)
• I have tested on my platform: Ubuntu 24.04 (Linux 6.8.0)

Documentation & Housekeeping

• I have updated relevant documentation (README, docs/, docstrings) — or N/A
• I have updated cli-config.yaml.example if I added/changed config keys — or N/A
• I have updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I have considered cross-platform impact (Windows, macOS) — or N/A
• I have updated tool descriptions/schemas if I changed tool behavior — or N/A

[teknium1]

Thanks for targeting an otherwise hard-to-observe skill-selection gap. The current prompt has no explicit unloaded-skill comparison pass, but this implementation needs adjustment for current review-fork safety and reporting behavior.

Problems

• The new block asks the review fork to patch after skills_list, without first requiring skill_view (agent/background_review.py:251, mirrored at :337). Current main rejects background-review patches until that exact SKILL.md was loaded: tools/skill_manager_tool.py:382-409 and :956-963; see the regression contract in tests/tools/test_skill_manager_tool.py:1289-1317.
• TRIGGER FIX is requested only in the fork's final reply (agent/background_review.py:267), but current main exposes only successful tool actions via summarize_background_review_actions() (agent/background_review.py:869-909). That reply is not surfaced to the user.

Suggested changes

• Require skill_view(name) before patching each candidate, then add coverage for a successful view → patch review-fork path.
• Either wire a structured trigger-fix event into the existing action summary or remove the promised visible log.

Automated hermes-sweeper review.

[teknium1]

A background-review patch will be rejected here unless this prompt first requires skill_view(name) for the candidate SKILL.md. Current main enforces that read-before-write invariant in tools/skill_manager_tool.py:382-409 and :956-963; skills_list returns only metadata.

[teknium1]

The review fork's final reply is not user-visible: current main only summarizes successful memory and skill_manage tool actions in agent/background_review.py:869-909. Please either emit this as structured summary data or avoid promising a visible TRIGGER FIX log.