Skip to content

test(skills): add Phase 1 CLI acceptance harness - #558

Merged
yacosta738 merged 1 commit into
mainfrom
fix/phase1-acceptance-harness
Aug 15, 2026
Merged

yacosta738 merged 1 commit into
mainfrom
fix/phase1-acceptance-harness

Conversation

@yacosta738

Copy link
Copy Markdown
Contributor

Description

Adds a reproducible external-CLI acceptance target for the Phase 1 curated skill migration. The harness runs the release-like agentsync executable against isolated temporary fixtures and checks the observable install, registry, companion, override, fail-closed, and project-root behaviors.

Fixes # (none)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Test A — cargo test --all-features
  • Test B — make acceptance-phase1 AGENTSYNC_BIN=target/release/agentsync AGENTSYNC_SOURCE_REPO=/Users/acosta/Dev/dallay/worktrees/agents-skills-phase1-provenance
  • Test C — tests/acceptance/test_phase1_catalog_harness.sh
  • Test D — AGENTSYNC_SOURCE_REPO=/Users/acosta/Dev/dallay/worktrees/agents-skills-phase1-provenance tests/acceptance/test_phase1_provenance.sh

Test Configuration:

  • OS/Distribution: macOS 26.6.1
  • Rust version: rustc 1.97.1
  • Node/pnpm version: Node 24.19.0, pnpm 11.21.0
  • Test command used: cargo build --release, cargo test --all-features, and the Phase 1 acceptance commands above
  • Environment variables: AGENTSYNC_BIN, AGENTSYNC_SOURCE_REPO, AGENTSYNC_NO_UPDATE_CHECK=1 (set by the harness)
  • Reproduction steps: build the release binary, then run make acceptance-phase1 AGENTSYNC_BIN=target/release/agentsync AGENTSYNC_SOURCE_REPO=../agents-skills

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules — the harness accepts an explicit source-repository path and is independently reviewable

Chain Context

Field Value
Strategy github-stacked-prs
Chain phase1-qa-remediation
Position 1 of 3
Base main
Head fix/phase1-acceptance-harness
Depends on None
Follow-up docs/phase1-qa-evidence
Review budget 372 changed lines / 400
Issue None
Linear None
Starts at 7465d920 (merged Phase 1 implementation)
Ends with A standalone black-box acceptance target for the migrated catalog behavior

Chain Overview

main
└── [current] fix/phase1-acceptance-harness
    └── docs/phase1-qa-evidence
        └── qa/phase1-acceptance-evidence

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f7fd13d-d0c7-4abb-8dd3-317a37c20cf7

📥 Commits

Reviewing files that changed from the base of the PR and between 7465d92 and 2e793c9.

📒 Files selected for processing (4)
  • Makefile
  • tests/acceptance/phase1_catalog.sh
  • tests/acceptance/test_phase1_catalog_harness.sh
  • tests/acceptance/test_phase1_provenance.sh

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added a Phase 1 catalog migration acceptance command with optional configuration for the CLI binary and source repository.
    • Added validation for skill installation, content copying, registry entries, source overrides, missing content handling, and provider suggestions.
  • Bug Fixes

    • Added checks for missing executables and provenance validators, with clear failure diagnostics.

Walkthrough

The pull request adds a Phase 1 catalog acceptance harness with isolated fixtures and installation checks. It exposes the harness through Make and adds tests for missing binaries and source provenance validation.

Changes

Phase 1 acceptance coverage

Layer / File(s) Summary
Harness setup and execution
tests/acceptance/phase1_catalog.sh
The harness validates the CLI and source paths, creates isolated fixtures, runs controlled commands, and checks files, JSON results, and registry entries.
Catalog installation scenarios
tests/acceptance/phase1_catalog.sh
The harness tests direct installation, local source override precedence, fail-closed behavior, and suggestion-based Pydantic installation.
Acceptance entry points and validation
Makefile, tests/acceptance/test_phase1_catalog_harness.sh, tests/acceptance/test_phase1_provenance.sh
Make exposes the Phase 1 harness. Tests validate missing-binary diagnostics and source provenance validation.

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

Merge Risk: ⚪ Minimal · up to 2e793

This PR adds isolated CLI acceptance coverage for Phase 1 behavior without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Developer
  participant Makefile
  participant phase1_catalog.sh
  participant AGENTSYNC_BIN
  participant IsolatedFixtures
  Developer->>Makefile: invoke acceptance-phase1
  Makefile->>phase1_catalog.sh: pass binary and source repository
  phase1_catalog.sh->>IsolatedFixtures: create test repositories and projects
  phase1_catalog.sh->>AGENTSYNC_BIN: run installation and suggestion commands
  AGENTSYNC_BIN->>IsolatedFixtures: write skills and registry entries
  phase1_catalog.sh->>IsolatedFixtures: verify content, status, and isolation
Loading

Poem

A rabbit hops through fixtures bright,
Three skills copied just right.
Overrides guide the catalog trail,
Empty sources make fallback fail.
Pydantic proves the path is sound—
“Phase One passes!” echoes round.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly identifies the addition of a Phase 1 CLI acceptance harness, which is the main change.
Description check ✅ Passed The description directly explains the acceptance harness, tested behaviors, commands, configuration, and purpose.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/phase1-acceptance-harness

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.

@linear-code
linear-code Bot marked this pull request as ready for review August 14, 2026 14:10
@github-actions github-actions Bot added the ci label Aug 14, 2026
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yacosta738
yacosta738 force-pushed the fix/phase1-acceptance-harness branch from 2e793c9 to bbf0a3e Compare August 14, 2026 17:50
@sonarqubecloud

Copy link
Copy Markdown

@yacosta738
yacosta738 added this pull request to the merge queue Aug 15, 2026
Merged via the queue into main with commit 22e9122 Aug 15, 2026
58 of 61 checks passed
@yacosta738
yacosta738 deleted the fix/phase1-acceptance-harness branch August 15, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants