Commit graph

6 commits

Author SHA1 Message Date
Claude
0e41cb2357
feat(code-reviewer): C-specific smell detector + fixtures
Phase 2 / Tier 1 of the post-#769 audit. Until now, the deterministic
analyzer (scripts/code_quality_checker.py) had language-specific smell
detectors only for C# and Java; C / C++ / Rust / Ruby / PHP / Dart all
fell through to generic checks. This PR brings C onto the same footing
as C# and Java -- the security delta is largest for memory-unsafe
languages, so C goes first per the audit ranking.

What's detected (CERT C + CWE catalogue patterns)

  - Banned functions: gets, strcpy, strcat, sprintf, vsprintf
    (CWE-242 / CWE-120 family -- no bounds check on any of them)
  - Format-string vulnerability: printf(var) / syslog(var) where the
    first arg is a bare identifier instead of a literal (CWE-134).
    Suppressed when the first arg is a literal string.
  - Unbounded scanf: %s without a width specifier (CWE-120).
    Suppressed when a width is present (e.g. %31s).
  - malloc/calloc/realloc result not NULL-checked within 5 lines
    (CWE-690). Recognises if (p == NULL), if (NULL == p), if (!p),
    if (p != NULL).
  - free(p) without setting p = NULL on the next real line
    (CWE-416 use-after-free guardrail). Low severity since some
    style guides skip the zeroing convention.
  - system() with a non-literal argument (CWE-78 command injection).
    Suppressed when the argument is a string literal or NULL.

Implementation

  - New function check_c_specific_smells() in code_quality_checker.py,
    placed after check_java_specific_smells(). Reuses the existing
    _strip_csharp_comments helper -- C, C#, and Java share // and /* */
    comment syntax.
  - Wired into analyze_file() via the existing dispatcher pattern:
    `if language == "c": smells.extend(check_c_specific_smells(content))`.

Fixtures (regression-detection harness)

  - assets/sample_c_smells.c -- 67 lines, every detector pattern
    labelled inline with its CWE. Smells fixture produces 10 C-specific
    detector hits (strcpy fires twice intentionally, once in each
    function). Score: 4/100 (F).
  - assets/sample_c_clean.c -- same surface area refactored per
    rules/universal.md + languages/c.md. Zero C-specific hits.
    Score: 100/100 (A).
  - expected_outputs/sample_c_smells_quality.json and
    expected_outputs/sample_c_clean_quality.json -- committed JSON
    output mirrors the existing C# / Java regression-guard pattern.

Documentation

  - engineering-team/skills/code-reviewer/README.md
    - "Language-specific smell packs" line extended to enumerate the
      6 C-pack patterns alongside the existing C# and Java packs.
    - Bundled-fixtures table adds the 2 new C fixture rows.
  - engineering-team/skills/code-reviewer/SKILL.md
    - "Adding a New Language" step 5 reference: C# and Java -> C#,
      Java, and C.
    - "Regression Fixtures" paragraph reference: C# and Java -> C#,
      Java, and C.
  - docs/skills/engineering-team/code-reviewer.md mirrors the same
    SKILL.md updates.
  - CHANGELOG.md gets a new [Unreleased] section above the existing
    code-reviewer entry, documenting the detector + fixtures.

Regression

  - All 6 fixtures (C# / Java / C x smells / clean) pass byte-for-byte
    against expected_outputs/*.json. No drift introduced in C# or Java
    behaviour.

Not in this PR (Phase 2 audit, subsequent PRs)

  - check_<name>_specific_smells for C++, Rust, Python, Kotlin, PHP,
    Ruby, Dart, Go, Swift, TypeScript, JavaScript. C++ and Rust are
    the next-highest-leverage targets (smart-pointer ownership,
    unsafe block discipline). Same fixture + expected_outputs pattern
    will apply to each.

https://claude.ai/code/session_01SnXMhpyuAwrws26Wy4fizz
2026-05-28 14:27:26 +00:00
Claude
6e7130ccc0
fix(code-reviewer): wire 6 new languages into deterministic analyzer
PR #769 added language-rule files for C, C++, Rust, Ruby, PHP, and
Dart/Flutter, and updated SKILL.md's dispatch table + the --language
valid-values comment. But scripts/code_quality_checker.py was left
unchanged, so the deterministic analyzer silently skipped any file in
those 6 languages — the dispatch table promised coverage the script
didn't deliver.

Closes that gap by extending three structures in code_quality_checker.py:

  - LANGUAGE_EXTENSIONS: 6 new entries matching SKILL.md exactly
    (c declared before cpp so `.h` resolves to C per the dispatch table)
  - find_functions patterns: language-aware function regexes
  - find_classes patterns + nested method_patterns: type-defs and
    method counting for class-like constructs (struct/enum/trait/union
    for Rust; class/module for Ruby; class/interface/trait/enum for PHP;
    class/mixin/enum/extension with Dart 3 modifier prefixes; struct for
    C; class/struct for C++)

The function regexes for C and C++ require a trailing `{` so prototypes
and call sites aren't misclassified as definitions, and exclude
control-flow keywords (if/while/for/switch/return/sizeof) by negative
lookahead. This matches the discipline already used for the Java and C#
patterns (require modifier keywords).

Verification

  - python3 scripts/code_quality_checker.py --help → all 14 languages
    now appear as --language choices.
  - Smoke-tested with a minimal sample per new language; each correctly
    detects language, counts functions and classes, and produces a
    quality score (no "Unsupported file type" errors).
  - Regression: all 4 bundled fixtures (csharp/java × smells/clean)
    still match their committed expected_outputs/*.json byte-for-byte.

Not in this PR (Phase 2 of the audit, separate PRs):
  - Language-specific check_<name>_specific_smells detectors. Only C#
    and Java have these today; the audit flagged C/C++/Rust as the
    next-highest-leverage additions (memory safety, unsafe block
    discipline, smart pointer ownership).
  - Asset fixtures + expected_outputs JSON for the 6 new languages as
    regression guards.
  - Modernity / version-anchor sweep on the language markdown files.

https://claude.ai/code/session_01SnXMhpyuAwrws26Wy4fizz
2026-05-28 06:44:31 +00:00
Claude
5ff4375603
feat(code-reviewer): wire Java into analyzer + complete the refactor
Builds on @mitnick2012's universal+per-language restructure (PR #742).

- Add Java as a first-class deterministic language in code_quality_checker.py
  (LANGUAGE_EXTENSIONS + function/class/method patterns + check_java_specific_smells),
  so the documented `--language java` command works instead of erroring on an
  invalid choice. Add Java debug + @SuppressWarnings signals to pr_analyzer.py.
- Add Java regression fixtures (sample_java_smells/clean.java) with committed
  expected_outputs JSON, mirroring the existing C# fixtures.
- Delete references/{code_review_checklist,coding_standards,common_antipatterns}.md,
  now duplicated by rules/universal.md + languages/*.md; repoint README and the
  C# clean fixture header at the new structure.
- Document the optional analyzer-wiring + fixture steps in the "Adding a New
  Language" guide and restore a Regression Fixtures section in SKILL.md.

https://claude.ai/code/session_01DjuELpoFdFbFscr3kAatni
2026-05-26 14:02:19 +00:00
Claude
7dc7fae1fe
feat(code-reviewer): raise quality score 54.8/D → 72.5/B- with assets, expected outputs, README
Addresses the Phase 3 quality_scorer roadmap items from the plugin audit:
adds the bundled fixtures, sample outputs, and quick-reference README that
the scorer expects, without diverging from the project's minimal-frontmatter
SKILL.md convention.

assets/:
- sample_csharp_smells.cs: a C# fixture with every pattern the skill
  detects (async void, blocking on Task, swallowed Exception, undisposed
  IDisposable, new HttpClient(), missing await, null-forgiving, hardcoded
  connection string, unsafe, dynamic, #pragma warning disable,
  [SuppressMessage], SQL concatenation), each smell labelled inline
- sample_csharp_clean.cs: the same code refactored per the standards in
  references/coding_standards.md — verifies the analyzer produces 0 HIGH
  smells on idiomatic code

expected_outputs/:
- sample_csharp_smells_quality.json: committed analyzer output for the
  smells fixture (F/45, 3 HIGH smells)
- sample_csharp_clean_quality.json: committed analyzer output for the
  clean fixture (A/98, 0 HIGH smells)

These act as a regression harness: diff the live output against the
committed JSON to detect any behaviour change in the analyzer.

scripts/code_quality_checker.py:
- Add _strip_csharp_comments() that removes // line and /* */ block
  comments before running C#-specific regex detectors. Fixes false
  positives where comment prose ("// FIX: await instead of .Result")
  matched a detection pattern.

SKILL.md:
- New ## Examples section pointing at the fixtures + showing how to
  reproduce the expected output with diff
- TOC updated to list "C# / .NET Review Notes" and "Examples"

README.md (new):
- Quick-reference card with how-to, 3 worked examples (one per script),
  pointer to fixtures, pointer to references

Phase re-scores after this change:
- Structure: 86.4/GOOD → 91.3/EXCELLENT (+4.9)
- Quality:   54.8/D    → 72.5/B-       (+17.7)
- Scripts:   3/3 PASS (unchanged)
- Security:  0/0       (unchanged)
2026-05-23 02:24:39 +00:00
Claude
f5f6ee46bf
feat(code-reviewer): deliver C# / .NET implementation behind PR #723 docs
PR #723 added C# / .NET claims to SKILL.md but did not update the scripts
or references. This wires up the actual implementation so the documented
behavior matches reality.

scripts/code_quality_checker.py:
- add "csharp": [".cs", ".csx", ".razor", ".cshtml"] to LANGUAGE_EXTENSIONS
  (fixes argparse error on --language csharp)
- add C# function, class, and method regex patterns
- add check_csharp_specific_smells(): async void, blocking on Task
  (.Result/.Wait/.GetAwaiter().GetResult()), swallowed Exception,
  undisposed IDisposable, new HttpClient() in method body, missing
  await on *Async() calls, unused using directives

scripts/pr_analyzer.py:
- extend hardcoded_secrets to catch connection strings
- extend console_log to catch Debug.WriteLine
- extend disable_eslint -> analyzer_disable to catch #pragma warning
  disable and [SuppressMessage]
- extend any_type -> loose_type to catch C# dynamic
- extend sql_concatenation to catch FromSql/ExecuteSql interpolation
- add csharp_unsafe_block (both block and modifier forms)
- add csharp_null_forgiving (!. operator overuse)
- add csharp_async_void
- add csharp_blocking_async

references/coding_standards.md:
- add C# / .NET Standards section: nullable reference types,
  async/await, exception handling, IDisposable, LINQ, dependency
  injection, records + pattern matching, ASP.NET Core security

references/common_antipatterns.md:
- add C# / .NET Antipatterns section: async void, blocking on async,
  swallowing Exception, undisposed IDisposable, new HttpClient() in
  method, missing ConfigureAwait in libraries, mutable public setters,
  dynamic overuse, unjustified analyzer suppression

SKILL.md: add trailing newline (fixes "No newline at end of file" warning).

Verified: code_quality_checker fires all 8 documented C# smell categories
on a fixture; pr_analyzer fires all 8 risk patterns on the same diff;
Python and TypeScript regression checks pass.
2026-05-23 01:35:06 +00:00
Reza Rezvani
1851c8fb09 fix(plugins): restructure 9 multi-skill domain plugins into ./skills/ layout
Same root cause as #587/#591 — Claude Code's runtime loader rejects
array-form skills paths like ["./content-production", "./ai-seo", ...]
even when each entry is a valid subdirectory containing SKILL.md.
`claude plugin validate` accepts them but the loader does not.

The proven canonical layout (used by self-improving-agent in #536):

  <plugin>/
  ├── .claude-plugin/plugin.json    skills: "./skills"
  └── skills/
      ├── <skill-1>/SKILL.md
      ├── <skill-2>/SKILL.md
      └── ...

Restructured 9 multi-skill domain plugins:
- business-growth (4 skills moved)
- c-level-advisor (28)
- engineering (36)
- engineering-team (32)
- finance (2)
- marketing-skill (43)
- product-team (12)
- project-management (8)
- ra-qm-team (13)

Also fixed standalone plugins that had root SKILL.md + ./skills/ subdir
(agenthub, autoresearch-agent, executive-mentor, playwright-pro). The
loader rejected them despite skills="./skills" because of the conflicting
root SKILL.md (compare self-improving-agent which works because PR #536
moved its root SKILL.md). Moved each root SKILL.md into ./skills/<name>/.

Restored standalone plugin folders to their original paths after the
multi-skill restructure swept them into parent skills/ directories
(marketplace.json source paths require original locations).

Removed 7 orphaned marketplace entries that pointed to skill folders
without their own plugin.json (content-creator, demand-gen,
fullstack-engineer, aws-architect, product-manager, scrum-master,
skill-security-auditor) — these were already non-functional.

Bumped patch versions on every changed plugin and synced
marketplace.json. Marketplace now lists 29 working plugins (down
from 36).

After merge: users run `/plugin marketplace update claude-code-skills`
followed by `/plugin update --all` to pick up the working layout.
2026-05-02 22:51:20 +02:00