Skip to content

feat(skillify): add review-only push preview - #335

Open
LittlePeter52012 wants to merge 5 commits into
activeloopai:mainfrom
LittlePeter52012:feat/skillify-push-review
Open

feat(skillify): add review-only push preview#335
LittlePeter52012 wants to merge 5 commits into
activeloopai:mainfrom
LittlePeter52012:feat/skillify-push-review

Conversation

@LittlePeter52012

@LittlePeter52012 LittlePeter52012 commented Aug 12, 2026

Copy link
Copy Markdown

Summary

  • add hivemind skillify push <skill-name> --review as a pre-publication review step
  • reuse the append-only version lookup while forcing the existing dry-run path, so no org skill row is created
  • display the same validated source snapshot used for metadata, with terminal-spoof protections and read-only legacy scope lookup
  • keep canonical help and the Pi command mirror synchronized

This is a narrow foundation for the README roadmap item Skill versioning and review: it adds a deterministic human-review checkpoint without introducing approval state or schema changes.

Version Bump

No release is requested from this contributor PR, so package.json is unchanged. Maintainers can include the feature in their next planned release.

Test plan

  • npx vitest run tests/claude-code/skillify-cli.test.ts tests/claude-code/skillify-scope-config.test.ts tests/shared/skillify-push.test.ts tests/cli/skillify-spec-self-drift.test.ts tests/pi/skillify-spec-drift.test.ts tests/claude-code/skillify-session-start-injection.test.ts

  • npm run typecheck

  • npm run build

  • git diff origin/main...HEAD --check

  • Tests pass locally (npm test; see baseline note below)

  • Relevant new tests added

  • No release requested; package.json not bumped

Full-suite baseline

npm test reports 15 failures and 5800 passes. The same 15 failures reproduce on unmodified main, in notifications, plugin cache GC, Cowork installation, skillify project-key normalization, and graph tests; this change adds no new full-suite failures.

Summary by CodeRabbit

  • New Features
    • Added skillify push --review to preview candidate content and the proposed version without publishing.
    • Review mode detects unsafe terminal and bidirectional text controls and safely displays metadata.
  • Bug Fixes
    • Preserved the exact UTF-8 content of local skill files during review.
    • Improved read-only handling of legacy configuration without modifying existing settings.
  • Documentation
    • Updated command help and extension listings to include the review option.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 604b473b-e26a-4263-ae4e-301809354764

📥 Commits

Reviewing files that changed from the base of the PR and between d13845f and 17032e8.

📒 Files selected for processing (1)
  • src/commands/skillify.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/commands/skillify.ts

📝 Walkthrough

Walkthrough

Changes

Skillify review workflow

Layer / File(s) Summary
Review data and configuration contracts
src/skillify/push.ts, src/skillify/scope-config.ts, tests/claude-code/skillify-scope-config.test.ts
Push summaries preserve the original SKILL.md text. Scope configuration can skip legacy migration and read legacy configuration in review mode.
Review execution and output
src/commands/skillify.ts, tests/claude-code/skillify-cli.test.ts
skillify push --review validates content, escapes unsafe metadata, displays the candidate and proposed version, and skips publication.
Command exposure and usage validation
src/cli/skillify-spec.ts, harnesses/pi/extension-source/hivemind.ts, src/commands/skillify.ts, tests/claude-code/skillify-cli.test.ts
Command specifications, the Pi registry, usage text, and tests document the new option.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI as skillify push
  participant Config as loadScopeConfig
  participant Push as runPush
  CLI->>Config: load configuration with migrateLegacy=false
  CLI->>Push: run dry-run push
  Push-->>CLI: return candidate source and proposed version
  CLI-->>CLI: validate content and escape metadata
  CLI-->>CLI: display review output without publishing
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a review-only preview for skillify push.
Description check ✅ Passed The description covers the change, release intent, targeted tests, validation steps, and the documented full-suite baseline failures.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/claude-code/skillify-cli.test.ts`:
- Line 371: Update tests/claude-code/skillify-cli.test.ts lines 371-371 and
383-383: store each unsafe candidate in a variable, then assert the complete
rejection message using candidate.indexOf() to verify the exact ESC and U+202E
offsets rather than only matching generic prefixes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d519133-4959-44ea-a94f-1c37848dac02

📥 Commits

Reviewing files that changed from the base of the PR and between 7d17a41 and 5d5d895.

📒 Files selected for processing (7)
  • harnesses/pi/extension-source/hivemind.ts
  • src/cli/skillify-spec.ts
  • src/commands/skillify.ts
  • src/skillify/push.ts
  • src/skillify/scope-config.ts
  • tests/claude-code/skillify-cli.test.ts
  • tests/claude-code/skillify-scope-config.test.ts

Comment thread tests/claude-code/skillify-cli.test.ts Outdated
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