Skip to content

feat(skills): add Go, Python, Java, Rust skills + restructure language skills into optional plugins#76

Merged
dean0x merged 2 commits intomainfrom
feat/polyglot-skills
Mar 5, 2026
Merged

feat(skills): add Go, Python, Java, Rust skills + restructure language skills into optional plugins#76
dean0x merged 2 commits intomainfrom
feat/polyglot-skills

Conversation

@dean0x
Copy link
Owner

@dean0x dean0x commented Mar 4, 2026

Summary

  • 4 new language skills (Go, Python, Java, Rust) with SKILL.md + references/ each — following existing TypeScript skill conventions (Iron Law, categories, checklist, detection patterns)
  • 8 optional plugins — all language/ecosystem skills (typescript, react, accessibility, frontend-design, go, python, java, rust) restructured into standalone optional plugins users select during devflow init
  • Dynamic skill detection in code-review commands — orchestrator checks if language skill is installed before spawning a Reviewer, so only installed language plugins are used
  • Agent updates — Reviewer and Coder agents aware of all 8 language skills; graceful degradation when a skill isn't installed

Before → After

Metric Before After
Skills 26 30
Plugins 9 17 (9 core + 8 optional)
Optional plugins 1 (audit-claude) 9 (+8 language)
Language support TypeScript, React only + Go, Python, Java, Rust

Key Design Decisions

  • All language skills are optional — users install only what they need via devflow init multiselect or --plugin=go,python
  • Skill availability check — code-review command verifies ~/.claude/skills/{focus}/SKILL.md exists before spawning a conditional language reviewer; skips if not installed
  • No init code changes — existing optional: true handling in init.ts already supports this; optional plugins appear unchecked by default

Test plan

  • npm run build passes — 30 skills, 17 plugins distributed correctly
  • npm test passes — 174/174 tests (updated accessibility mapping assertion)
  • Verified skill count: 30, plugin count: 17, optional count: 9
  • Verified all 8 language skills distributed to plugin directories
  • Verified core-skills no longer contains language skills
  • Manual: devflow init shows 8 language plugins as unchecked optional choices
  • Manual: devflow init --plugin=go installs only go skill + core-skills
  • Manual: /code-review on a Go project with devflow-go installed spawns Go reviewer

…age skills into optional plugins

- Add 4 new language skills (go, python, java, rust) in shared/skills/ with
  SKILL.md + references/ (violations, patterns, detection, language deep-dive)
- Create 8 optional plugin directories (devflow-typescript, devflow-react,
  devflow-accessibility, devflow-frontend-design, devflow-go, devflow-python,
  devflow-java, devflow-rust) — each with plugin.json referencing its skill
- Move typescript, react, accessibility, frontend-design out of core-skills,
  implement, and code-review plugins into their own optional plugins
- Register all 8 optional plugins in plugins.ts with optional: true
- Update reviewer agent: add go/java/python/rust to Focus Areas table and
  conditional activation entries
- Update coder agent: add 4 new languages to frontmatter skills and DOMAIN hints
- Update code-review commands (both variants): add go/python/java/rust to
  conditional activation table, add skill availability check instruction
  so orchestrator verifies skill exists before spawning language reviewers
- Update ambient-router SKILL.md and skill-catalog.md with new languages
- Update marketplace.json with 8 new plugin entries
- Update skills-architecture.md with Tier 3 entries and glob patterns
- Update tests: fix accessibility mapping assertion (now in devflow-accessibility)
- Update CLAUDE.md (30 skills, 17 plugins) and README.md (language plugins section)

Totals: 30 skills (was 26), 17 plugins (was 9), 9 optional (was 1)
All 174 tests pass, build verified.
@@ -2,7 +2,7 @@
name: Coder
description: Autonomous task implementation on feature branch. Implements, tests, and commits.
model: inherit
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: Coder agent frontmatter lists 8 optional skills as hard dependencies

The skills: frontmatter now declares go, python, java, rust (plus 4 pre-existing: typescript, react, accessibility, frontend-design) as required. These are now in optional plugins that may not be installed. While the agent body has conditional loading logic, the frontmatter contradicts this by unconditionally listing all 14 skills.

Impact: When users install devflow-implement without the language plugins, the agent will reference non-existent skill files.

Fix: Remove optional language skills from frontmatter. Let the body's domain-loading logic handle dynamic skill loading:

- skills: core-patterns, git-safety, implementation-patterns, git-workflow, typescript, react, test-patterns, input-validation, accessibility, frontend-design, go, python, java, rust
+ skills: core-patterns, git-safety, implementation-patterns, git-workflow, test-patterns, input-validation

The existing body text at line 41 ('For non-TypeScript backends: load the corresponding language skill') already handles this correctly.


func (c *Client) connection() (*grpc.ClientConn, error) {
c.once.Do(func() {
c.conn, c.err = grpc.Dial("localhost:50051", grpc.WithInsecure())
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: Example uses deprecated grpc.WithInsecure() pattern

This example teaches an insecure pattern using deprecated APIs (deprecated since gRPC-Go v1.53+). Using grpc.WithInsecure() normalizes unencrypted transport in reference code that developers copy.

Fix: Use modern gRPC API with explicit comment about TLS:

func (c *Client) connection() (*grpc.ClientConn, error) {
    c.once.Do(func() {
        // For production: use credentials.NewTLS(&tls.Config{})
        c.conn, c.err = grpc.NewClient("localhost:50051",
            grpc.WithTransportCredentials(insecure.NewCredentials()), // local dev only
        )
    })
    return c.conn, c.err
}

```python
from typing import AsyncGenerator

async def stream_results(query: str) -> AsyncGenerator[Record, None]:
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: Example shows SQL injection pattern (raw string queries)

The stream_results function accepts raw SQL strings and passes them directly to conn.execute(query). While the usage example is hardcoded, the function signature invites SQL interpolation. The example should demonstrate parameterized queries.

Fix: Show parameterized query pattern:

async def stream_results(
    query: str, params: tuple[Any, ...] = ()
) -> AsyncGenerator[Record, None]:
    async with get_connection() as conn:
        cursor = await conn.execute(query, params)
        async for row in cursor:
            yield Record.from_row(row)

# Usage
async for record in stream_results(
    "SELECT * FROM events WHERE status = ?", ("active",)
):
    await process(record)

@dean0x
Copy link
Owner Author

dean0x commented Mar 4, 2026

PR #76 Review Summary

Blocking Issues (3 inline comments + 1 summary)

  1. Coder agent frontmatter - Lists 8 optional skills as hard dependencies. Must remove from frontmatter and rely on body's dynamic loading.

    • Reviewers: Architecture, Regression, Dependencies, TypeScript
  2. Security: Deprecated gRPC API - Example uses deprecated grpc.WithInsecure(). Must use modern gRPC API with explicit TLS comment.

    • Reviewer: Security
  3. Security: SQL injection pattern - Example accepts raw SQL strings without parameterization. Must show parameterized query pattern.

    • Reviewer: Security

Additional Blocking Issues (Not Inline Commentable)

  1. CHANGELOG.md missing - No entry documenting polyglot skills addition and skill restructuring (typescript, react, accessibility, frontend-design moved to optional). This is a breaking change requiring migration path documentation.

    • Reviewer: Documentation (CRITICAL)
    • Required: Add CHANGELOG entry with migration instructions
  2. Test coverage for optional field - 8 new language plugins with optional: true have zero test coverage. A regression removing optional would go undetected.

    • Reviewer: Tests (HIGH)
    • Required: Add test validating optional flag and that new skills (go, python, java, rust) appear in registry
  3. Missing upgrade migration path - 4 skills moved from bundled core to optional plugins without upgrade detection. Existing users will lose these skills on upgrade unless they explicitly install the optional plugins.

    • Reviewer: Regression (HIGH)
    • Required: Implement upgrade detection or auto-include previously-installed optional plugins

High-Priority Should-Fix Issues

  1. Language skill ordering inconsistency - Languages listed in different orders across files (go,python,java,rust vs go,java,python,rust). Should use consistent alphabetical ordering.

    • File: code-review.md, code-review-teams.md, reviewer.md, ambient-router/SKILL.md
    • Reviewer: Consistency (HIGH)
  2. Repetitive plugin definitions - 8 identical plugin shapes in src/cli/plugins.ts. Should use factory function to reduce boilerplate.

    • File: src/cli/plugins.ts:86-152
    • Reviewer: Complexity (HIGH), TypeScript (MEDIUM)
  3. Performance: Sequential skill availability checks - Checks for 8 optional skills run sequentially, adding latency. Should batch into single Glob call.

    • File: code-review.md:53, code-review-teams.md:53
    • Reviewer: Performance (MEDIUM)
  4. Go test file exclusion inconsistency - Go skill excludes test files (**/*_test.go) but Python, Java, Rust don't. Should be consistent.

    • File: shared/skills/go/SKILL.md:11
    • Reviewer: Architecture (MEDIUM), Regression (MEDIUM), Documentation (HIGH)
  5. Implement command missing skill availability check - code-review has the check, but implement.md doesn't. Should add parity.

    • File: plugins/devflow-implement/commands/implement.md (not modified, but should be)
    • Reviewer: Regression (HIGH)
  6. Ambient router missing skill availability guards - BUILD intent references optional skills without checking they exist.

    • File: shared/skills/ambient-router/SKILL.md:54
    • Reviewer: Architecture (MEDIUM), Regression (MEDIUM)

Review Summary Statistics

  • Total Reviewers: 10 (Security, Architecture, Performance, Complexity, Consistency, Regression, Tests, TypeScript, Documentation, Dependencies)
  • Inline Comments Created: 3
  • Blocking Issues: 6 (3 inline + 3 not commentable)
  • Should-Fix Issues: 6
  • Deduplicated Issues: 12 (consolidated from ~80 individual findings)
  • Pre-existing Issues: Not commented (per protocol)

Claude Code Review Attribution: Generated by multi-perspective code review system.

- Remove Go test file exclusion from skill activation
- Fix deprecated grpc.WithInsecure() → WithTransportCredentials
- Fix deprecated datetime.utcnow → datetime.now(timezone.utc)
- Fix SQL injection in Python async streaming example
- Fix deprecated React Context.Provider → Context (React 19)
- Fix deprecated useRef<T>() → useRef<T | undefined>(undefined)
- Replace non-portable NodeJS.Timeout with ReturnType<typeof setTimeout>
- Replace unsafe Function type with typed function signature
- Coder agent: remove optional skills from frontmatter, add dynamic loading
- Alphabetize language ordering (go, java, python, rust) across all files
- Fix code-review count text to be dynamic
- Add tests for optional: true flag on language plugins
- Add CHANGELOG [Unreleased] section for polyglot-skills feature
@dean0x dean0x merged commit d74a4de into main Mar 5, 2026
4 checks passed
@dean0x dean0x deleted the feat/polyglot-skills branch March 5, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant