Skip to content

Merge pull request #739 from alirezarezvani/claude/andreessen-productivity-plugin-PlOzn#746

Merged
alirezarezvani merged 2 commits into
mainfrom
dev
May 26, 2026
Merged

Merge pull request #739 from alirezarezvani/claude/andreessen-productivity-plugin-PlOzn#746
alirezarezvani merged 2 commits into
mainfrom
dev

Conversation

@alirezarezvani

Copy link
Copy Markdown
Owner

Summary

Checklist

  • Target branch is dev (not main — PRs to main will be auto-closed)
  • Skill has SKILL.md with valid YAML frontmatter (name, description, license)
  • Scripts (if any) run with --help without errors
  • No hardcoded API keys, tokens, or secrets
  • No vendor-locked dependencies without open-source fallback
  • Follows existing directory structure (domain/skill-name/SKILL.md)

Type of Change

  • New skill
  • Improvement to existing skill
  • Bug fix
  • Documentation
  • Infrastructure / CI

Testing

claude and others added 2 commits May 25, 2026 05:07
…in run)

The previous vibe sync was run with --domain productivity, which rewrote
.vibe/skills/claude-skills/skills-index.json to contain only the 6 productivity
skills, dropping the other ~317. The symlink tree was unaffected (all 14 domain
dirs intact) — only the index JSON was clobbered. Re-running the full sync
restores total_skills 6 -> 323 across all 14 domains. andreessen present.

https://claude.ai/code/session_01SF6MzfjHurZMt5JUFET9h3
@alirezarezvani alirezarezvani merged commit 10c1ceb into main May 26, 2026
4 checks passed
@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown

Code Review — PR #746

Summary: This is a merged dev → main PR that: (1) restores the .vibe/skills/claude-skills/skills-index.json after it was truncated to 6 entries by a previous --domain productivity sync, and (2) lands the Andreessen productivity plugin from PR #739.


What's good

  • Clear root-cause commit message — the fix commit explains exactly what broke (--domain productivity clobbered the full index) and why the fix (full re-sync) works. Future maintainers will understand this immediately.
  • Minimal blast radius — only the index JSON is touched. Symlink tree and all skill files are unaffected, which is the right scope.
  • Fast recovery — the fix was identified and landed quickly.

Issues Found

1. total_skills: 323 is inconsistent with file content (data integrity)

The fix re-ran the sync without the --domain filter, adding 323 entries across 13 domains. However, the 6 productivity entries from the old index are preserved in the file as unchanged lines (visible in the diff as context, e.g. "path": "productivity/reflect"). The total_skills counter reflects only the new additions, not the combined total — so the field likely undercounts by the number of productivity skills in the old index.

The counter should match jq '[.domains[]] | add | length' skills-index.json.

2. Productivity domain skipped by the fix sync (14 → 13 domains)

The commit message claims "14 domains" but the added entries only cover 13 top-level domain prefixes:

business-growth, business-operations, c-level-advisor, commercial, engineering, engineering-team, finance, marketing-skill, marketing, product-team, project-management, ra-qm-team, research

productivity/ is absent from the new additions. This means the productivity domain's index entries are still from the old --domain productivity run — different provenance than the rest. The new andreessen skill from PR #739 was the whole point of that PR, but it only lands in the index if it was already in those 6 old entries.

Worth verifying: does andreessen actually appear in the current state of the file on disk?

3. Five skills have "description": ">" (sync script bug)

Five entries were indexed with a literal > as their description:

  • engineering/command-guide
  • engineering/ship-gate
  • engineering/skill-security-auditor
  • engineering/caveman
  • engineering-team/epic-design

This looks like the sync script is picking up a markdown blockquote marker from a SKILL.md file instead of the actual description text. These should surface as failures in scripts/audit_skills.py. Worth investigating whether the description: YAML front-matter in those SKILL.md files has a malformed value.


Root Cause Remains Open

The underlying footgun is that sync-vibe-skills.py --domain X silently overwrites the full skills-index.json with only domain X, dropping all other domains. This will happen again. Consider either:

  • Option A: Make --domain filter append/update rather than replace the full index.
  • Option B: Add a guard that warns if total_skills drops by more than 10% vs the previous file.

Process

  • PR description body is empty — future PRs should at minimum note what's being merged and how it was tested.
  • PR checklist is entirely unchecked. For fix PRs, checking the applicable boxes (e.g. "no hardcoded secrets") signals the author reviewed the change, even if it's minor.

Overall: The fix is correct and necessary, and the commit message is exemplary. The issues above are follow-up items rather than blockers, but the total_skills count and missing productivity domain regeneration are worth a quick follow-up sync to make the index self-consistent.

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