feat(skills): add 14 navigation and workflow skills + static validator

[HumanBean17]

Summary

Implements PR-S-2 through PR-S-4 from propose/active/AGENT-SKILLS-AND-COMMANDS-PROPOSE.md.

• Tier 1 (10 skills): Deterministic MCP-chain navigation skills — nl, controllers, routes, clients, callers, callees, handlers, who-hits-route, implements, injects. Each has graph-accurate edge sets, resolve-first resolution, and worked examples against the bank-chat-system fixture.
• Tier 2 (4 skills): Bounded workflow skills — explain-feature, impact-of, trace-request-flow, mini-map. Each has explicit stop conditions, recursion limits, and output shapes. /mini-map composes optional MCP edge_filter with skill-layer accessor/semantic heuristics.
• skills/README.md: 3-layer architecture diagram, skill index, versioning note.
• tests/test_agent_skills_static.py (127 assertions): Validates frontmatter, MCP tool/kind/direction/edge-type refs against mcp_v2 allowlists imported from production code, Tier 2 body structure, directory integrity, and AGENT-GUIDE consistency.
• docs/AGENT-GUIDE.md: Slash-style aliases section replaced with pointer to skills/.

No ontology bump, no schema delta, no MCP surface change.

Test plan

• .venv/bin/ruff check . — clean
• .venv/bin/python -m pytest tests -v — 722 passed, 9 skipped
• Verify each skill's worked example against a fresh bank-chat-system graph build after review

🤖 Generated with Claude Code

[HumanBean17]

PR #225 Review — feat(skills): add 14 navigation and workflow skills + static validator

What's done well

• Consistent skill structure. Every SKILL.md follows the same template: frontmatter → argument contract → steps → worked example → out of scope. Easy for agents and humans to parse.
• Resolve-first pattern. Skills that take identifiers consistently use the resolve → one/many/none → fallback to search/find pattern. Good defensive design.
• Tests validate against production allowlists. EdgeType, ComposedEdgeType, and NodeKind are imported from mcp_v2 and java_ontology — not hand-maintained copies. The tests will catch drift when the ontology evolves.
• Tier 2 bounds are explicit. Stop conditions, recursion limits, and total neighbors call caps are documented per skill. This matters for preventing runaway agent chains.
• AGENT-GUIDE cleanup. Replacing the inline 11-entry slash-alias bullet list with a pointer to skills/ is the right call — single source of truth.

Issues

1. Missing /producers skill (regression risk)

The old AGENT-GUIDE.md had a /producers <ms> slash alias. The new skill set drops it — 10 Tier 1 skills vs. the old 11 aliases. If intentional, the PR body should call it out as a deliberate removal. If not, it's a gap users of /producers will notice.

2. Architecture diagram is duplicated

The three-layer ASCII diagram appears verbatim in both README.md and skills/README.md. These will diverge over time. Consider keeping one canonical copy and referencing it from the other.

3. mini-map references unvalidated MCP parameters

mini-map/SKILL.md step 3 references several edge_filter parameters (callee_declaring_role, min_confidence, exclude_callee_declaring_roles, dedup_calls, include_unresolved) that the static test does not validate. If any drift from what the MCP server actually accepts, the skill will silently break. Consider extracting edge_filter keys and checking them against mcp_v2 parameter definitions.

4. r: prefix convention is undocumented

Both handlers/SKILL.md and trace-request-flow/SKILL.md accept r: as a route id prefix shorthand, but this convention isn't documented anywhere central. Agents encountering a user-typed r: prefix need to know this is equivalent to route:.

5. Test regex for old-format detection is fragile

TestAgentGuideConsistency.test_guide_does_not_embed_full_slash_alias_bullets uses a hardcoded regex listing all skill names. If a skill is renamed or added, this test won't update unless the regex is also changed. Consider building the pattern from ALL_SKILL_NAMES instead.

Verdict

Approve with minor suggestions. The /producers gap is the most actionable item — flag it explicitly as intentional or add the skill. The diagram duplication and unvalidated edge_filter params are worth addressing before or shortly after merge.

🤖 Generated with Claude Code

[HumanBean17]

Re-review — fixes applied

Re-examined the full diff after changes. Here's the status of the 5 issues from the first review:

Resolved

#	Issue	Fix
1	Missing /producers skill	Added skills/producers/SKILL.md with correct kind="producer" filter, matching the old AGENT-GUIDE alias. Tier 1 now 11 skills (total 15).
2	Architecture diagram duplication	README.md now has a one-liner pointing to skills/README.md#three-layer-architecture. Canonical diagram lives in one place.
3	edge_filter params unvalidated	Documented as intentional known gap in the test module docstring with a pointer to AGENT-SKILLS-AND-COMMANDS-PROPOSE §11. Acceptable — the static validator doesn't parse edge_filter dicts, and manual verification on re-index is the stated workflow.
5	Fragile regex in old-format test	Pattern now built dynamically from ALL_SKILL_NAMES via `"

Still open

4. r: prefix convention undocumented — handlers/SKILL.md and trace-request-flow/SKILL.md both accept r: as a route: shorthand, but this isn't documented centrally. Minor issue — agents following individual skills will see it, but a one-line note in skills/README.md or AGENT-GUIDE.md would prevent confusion.

New observations

• PR body is stale — still says "14 navigation and workflow skills" and "127 assertions" but the count is now 15 skills and ~135 parametrized test invocations (15 skills × 8 parametrized tests + 4 × 2 tier-2 tests + 7 non-parametrized = 135). Worth updating before merge.
• Unused fixture — skill_name (line 982) is parametrized over ALL_SKILL_NAMES but never referenced by any test method. All tests use their own @pytest.mark.parametrize("name", ...). Dead code — can be removed.
• /producers skill is clean — follows the exact same structure as /clients (mirrors the old AGENT-GUIDE alias: find(kind="producer", filter={microservice: <arg>}, limit=100)). Good.

Verdict

Approve. The four substantive issues are resolved. The r: prefix documentation gap and the stale PR body count are minor. Ship-ready after the body update.

🤖 Generated with Claude Code