Skip to content

chore: sync main into dev — bring ECC plugin install into dev#791

Merged
alirezarezvani merged 3 commits into
devfrom
main
Jun 2, 2026
Merged

chore: sync main into dev — bring ECC plugin install into dev#791
alirezarezvani merged 3 commits into
devfrom
main

Conversation

@alirezarezvani

Copy link
Copy Markdown
Owner

What

Back-merges main into dev so the integration branch carries the ECC operator plugin install (.claude/settings.json) that landed on main via PR #790 (feature → main).

Why

The ECC install went feature → main directly (#790), skipping dev, leaving dev behind by exactly that one file. This brings dev back in sync. The only change is the addition of .claude/settings.json:

{
    "$schema": "https://json.schemastore.org/claude-code-settings.json",
    "extraKnownMarketplaces": { "ecc": { "source": { "source": "github", "repo": "affaan-m/ECC" } } },
    "enabledPlugins": { "ecc@ecc": true }
}

No conflicts; dev's in-flight v2.10.0 markdown-html work is untouched.

https://claude.ai/code/session_01Vw7LCQLH5Wi7WHrrN47Hj6


Generated by Claude Code

alirezarezvani and others added 3 commits May 29, 2026 07:09
…eference

Register the ECC marketplace (affaan-m/ECC) and enable the ecc@ecc plugin
in committed .claude/settings.json so the 63 agents / 249 skills / 79
commands / hooks load at runtime for anyone who opens this repo (one-time
trust prompt). Uses ECC's recommended plugin path; files are not vendored,
so there is no collision with the existing skills library.

https://claude.ai/code/session_01Vw7LCQLH5Wi7WHrrN47Hj6
@alirezarezvani
alirezarezvani merged commit 646691d into dev Jun 2, 2026
6 checks passed
@claude

claude Bot commented Jun 2, 2026

Copy link
Copy Markdown

Code Review — PR #791 (chore: sync main into dev — ECC plugin install)

Status: MERGED — this is a post-merge review for process learning and tracking.


Summary

This PR back-merges the single-file .claude/settings.json change from main into dev after it landed directly via PR #790 (feature → main, skipping dev). The change itself is minimal: 14 lines registering the affaan-m/ECC external marketplace and enabling the ecc@ecc plugin.


✅ What's Done Well

  • Clear PR description — explains exactly what changed, why dev was behind, and references the originating session URL.
  • Correct recovery action — a back-merge sync is the right way to re-align dev after an out-of-order merge.
  • Valid JSON with schema reference.claude/settings.json is well-formed and uses the canonical SchemaStore URL.
  • Minimal footprint — only adds the one necessary file; no unrelated edits.

⚠️ Issues & Concerns

1. No version pinning on the external plugin (Medium risk)

\"extraKnownMarketplaces\": {
    \"ecc\": {
        \"source\": {
            \"source\": \"github\",
            \"repo\": \"affaan-m/ECC\"   // ← unpinned; resolves HEAD at runtime
        }
    }
}

Pointing at the repo root without a pinned commit SHA or release tag means every claude session fetches whatever is currently on the upstream default branch. If affaan-m/ECC is ever compromised or has a breaking change pushed, all contributors are affected silently. Consider adding a ref field (commit SHA or semver tag) once the ECC plugin spec supports it, or document the accepted risk explicitly.

2. CI/security audit explicitly skips .claude/ (Coverage gap)

skill-security-audit.yml line 60 excludes .claude/* from the scanned paths:

.github/*|.claude/*|.codex/*|.gemini/*|... continue ;;

This means any future change to .claude/settings.json that registers a malicious marketplace or enables a rogue plugin will pass CI with no automated scrutiny. The ci-quality-gate.yml also has no step that validates extraKnownMarketplaces or enabledPlugins. Consider adding a lightweight allowlist check — or at minimum remove .claude/ from the security audit skip list so future additions get reviewed.

3. Root cause: PR #790 bypassed the dev→main workflow (Process)

The PR description correctly identifies that #790 went feature → main directly, violating the documented feature → dev → main strategy in CLAUDE.md. This sync PR is the right fix, but the root cause is worth tracking. If smart-sync.yml or any other automation can enforce base-branch targeting (always dev, never main directly), that would prevent similar drift.

4. Project-level settings affect all contributors (Visibility)

.claude/settings.json in the repo root auto-enables the ECC plugin for every contributor who opens this project in Claude Code (after a one-time trust prompt, per the PR #790 description). This is intentional, but it isn't documented in CLAUDE.md or the README. A short note explaining what ECC provides and why it's enabled repo-wide would help new contributors understand the setup.


Testing Checklist

Check Result
JSON syntax valid ✅ (read directly; well-formed)
Schema reference present ✅ (json.schemastore.org/claude-code-settings.json)
Only .claude/settings.json changed ✅ (1 file, 14 lines)
CI quality gate runs on this path ⚠️ No explicit validation of enabledPlugins
Security audit covers .claude/ ❌ Explicitly excluded (skill-security-audit.yml:60)
Plugin source pinned to a ref ❌ Unpinned — resolves upstream HEAD

Recommendations

  1. Short-term (no code change required): Document the ECC plugin in CLAUDE.md under Development Environment — what it is, why it's enabled, and how to disable it (enabledPlugins: { \"ecc@ecc\": false } in local settings).
  2. Medium-term: Add a CI step that validates .claude/settings.json — at minimum, assert that any extraKnownMarketplaces entry is on an approved list.
  3. Medium-term: Pin the ECC plugin source to a specific git ref once the plugin format supports it.
  4. Process: Enforce PR base-branch targeting via enforce-pr-target.yml or branch protection rules so feature branches must target dev, not main.

Review by claude[bot] — automated review gate. All findings are informational post-merge.

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.

3 participants