Skip to content

chore: sync codex + gemini indices after code-reviewer C# update#734

Merged
alirezarezvani merged 1 commit into
devfrom
claude/pr-723-review-zZdrT
May 23, 2026
Merged

chore: sync codex + gemini indices after code-reviewer C# update#734
alirezarezvani merged 1 commit into
devfrom
claude/pr-723-review-zZdrT

Conversation

@alirezarezvani

Copy link
Copy Markdown
Owner

Summary

Post-merge follow-up to #730. Phase 7 of the plugin audit on engineering-team/skills/code-reviewer found the cross-platform skill indices were stale — still showing the pre-#723 description without "C#, and .NET". This commits the auto-fix from scripts/sync-codex-skills.py + scripts/sync-gemini-skills.py.

Re-running the syncers also picked up two unrelated drifted entries (.codex/skills/{review,run,status} symlinks, .gemini/skills/claude-coach/ symlink) — included in the same commit since they're all canonical-sync output.

Audit verdict for code-reviewer (PR #730 changes)

Phase Result
1 — Discovery Single-skill, engineering-team, 3 scripts, 3 references
2 — Structure PASS 86.4/100 GOOD
3 — Quality WARN 54.8/100 D (pre-existing baseline — missing README/assets/expected_outputs; not regressions)
4 — Scripts PASS 3/3
5 — Security PASS 0 critical, 0 high (9 files scanned)
6 — Marketplace PASS (bundled under parent engineering-team v2.8.1, plugin.json valid)
7 — Ecosystem PASS WITH AUTO-FIX (this PR)
8 — Code review PASS (cs-senior-engineer criteria)

Type of Change

  • Infrastructure / CI (index sync)

Draft for review.


Generated by Claude Code

Phase 7 of the plugin audit on engineering-team/skills/code-reviewer
found stale entries in the cross-platform indices — they still showed
the pre-#723 description without C# / .NET. Re-running sync also
refreshed two unrelated drifted entries (review/run/status codex
symlinks, claude-coach gemini symlink).
@claude

claude Bot commented May 23, 2026

Copy link
Copy Markdown

Code Review — PR #734: codex + gemini index sync

Overall this is a clean, well-scoped infrastructure PR. The audit verdict table in the description is a nice touch. A few observations:


What's correct

  • Gemini code-reviewer description (C#, and .NET) — matches the codex entry and the actual skill after PR feat(code-reviewer): deliver C# / .NET implementation behind PR #723 docs #730. Verified.
  • claude-coach addition to gemini — it was already in .codex/skills-index.json and .codex/skills/; gemini was the only index missing it. Source path (../../../engineering/claude-coach/skills/claude-coach/SKILL.md) resolves to a real directory. ✅
  • Count arithmetictotal_skills 393 → 394 (+1 for claude-coach) and engineering-advanced 77 → 78 are both correct. ✅
  • Symlink targets — all three repointed codex symlinks (review, run, status) point to paths that exist on disk. ✅
  • Hermes.hermes/skills/ uses a single claude-skills bundle, not per-skill symlinks, so no update needed there. ✅

Issues worth addressing before merge

1. Non-deterministic symlink collision resolution (existing design flaw, surfaced here)

Three slug names — review, run, status — each have two source skills:

Slug Option A Option B
review engineering-team/playwright-pro/skills/review engineering-team/self-improving-agent/skills/review
run engineering/agenthub/skills/run engineering/autoresearch-agent/skills/run
status engineering/agenthub/skills/status engineering/autoresearch-agent/skills/status

sync-codex-skills.py sorts by (category, name) then creates symlinks in iteration order — the last entry processed for a given name wins. Path.iterdir() order on Linux is inode/readdir order, which is not alphabetical and can differ between machines, kernels, or after a filesystem operation. This means re-running the syncer on a clean clone could flip these symlinks again and produce another drift-fix PR.

Suggested fix in create_symlinks(): add an explicit tie-breaking rule, e.g. alphabetical by source path, so the winner is deterministic:

# When multiple skills share a name, last-write wins — sort source ascending
# so the alphabetically-later source (more specific plugin) always wins.
for skill in sorted(skills, key=lambda s: (s["category"], s["name"], s["source"])):

Or keep an explicit priority list. Either way, document the rule in the script.

2. Both colliding entries remain in the JSON index

The skills-index.json keeps both review entries (and both run, both status). This means:

  • Tooling doing a name lookup gets two hits and must pick one.
  • The index does not reflect which path the actual symlink points to.

The PR only reorders the duplicate pairs — the underlying ambiguity isn't resolved. Consider either: (a) deduplicating in the index, keeping only the "winner", or (b) adding a "collision": true marker so tooling can handle it explicitly.

3. Minor: PR description overstates what was stale in codex

still showing the pre-#723 description without "C#, and .NET"

The .codex/skills-index.json already had the correct description before this PR (verified: "description": "Code review automation for TypeScript, JavaScript, Python, Go, Swift, Kotlin, C#, and .NET…"). Only the gemini index needed updating. The codex diff in this PR is purely the symlink reordering, not a description fix. This is actually fine — it just means the PR description is slightly inaccurate about the scope.


Summary

The actual file changes are correct. The main blocker before moving out of DRAFT is the symlink non-determinism — without a stable tie-breaking rule, this exact fix could regress the next time someone runs the syncer on a fresh machine. The duplicate-index entries are a pre-existing issue; at minimum, linking it to a follow-up issue would prevent the next reviewer from spending time on it.

🤖 Reviewed with Claude Code

@alirezarezvani alirezarezvani marked this pull request as ready for review May 23, 2026 02:16
@alirezarezvani alirezarezvani merged commit 7cfa9fd into dev May 23, 2026
6 checks passed
@alirezarezvani alirezarezvani deleted the claude/pr-723-review-zZdrT branch May 23, 2026 02:16
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.

2 participants