ci: run public skill acceptance tests - #4
Open
bruno-archastro wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review on ArchCode
Problem and author intent
ArchDev's public skills include executable acceptance scripts, but the repository's only automated checks exercise the Unix and Windows installers. The current lifecycle is:
skill package changes → pull request runs installer smoke tests only → green checks despite the skill test never runningThat leaves
npx skillspackaging, cold bootstrap, and domain workflow regressions outside the merge gate. The intended outcome is one automatic check that discovers every public skill package and executes its matching acceptance test without maintaining a second hard-coded skill list.What changed
scripts/run-skill-tests.sh, which discoversskills/*/SKILL.mdin deterministic order and runstests/<skill>-skill.shfor each package.tests/skill-test-runner.shto prove discovery, execution through a separate shell process, paths containing spaces, fail-closed missing coverage, and the no-skills bootstrap state.Skill Acceptanceworkflow for pull requests, pushes tomain, and manual dispatches. It runs the runner proof followed by all discovered domain tests with read-only repository permissions.Scope
Backend/tooling-only. This changes public repository CI and does not affect the ArchDev runtime or frontend.
Risk assessment
Low. The new workflow is additive and has read-only permissions. The main risk is CI time as more skills are added; tests run sequentially to avoid shared
npxand home-directory contention.User impact
No direct CLI behavior changes. Contributors receive a failing pull-request check when a public skill is missing coverage or its install/bootstrap workflow regresses.
Testing
Canonical end-to-end proof
tests/skill-test-runner.shcreates a synthetic public skill repository, crosses the runner-to-acceptance-script process boundary, and asserts the script's externally visible log output. It then injects an uncovered skill and verifies that the runner exits nonzero with the expected test path before proving that an empty pre-skill repository remains bootstrap-safe.Commands run
tests/skill-test-runner.sh— passedscripts/run-skill-tests.shon this branch's no-skills state — passedscripts/run-skill-tests.shagainst the Tasks and Jobs branch — both acceptance scripts passedscripts/run-skill-tests.shagainst the pending Rooms branch — Rooms acceptance passedgit diff --check— passedAutomatic execution path
.github/workflows/skill-acceptance.ymlruns on everypull_request, every push tomain, andworkflow_dispatch. ItsPublic skill acceptancejob first runstests/skill-test-runner.sh, thenscripts/run-skill-tests.sh; there are no path, environment, or platform skip conditions. The domain scripts use Unix bootstraps and therefore run on Ubuntu. Existing installer CI continues to provide separate Windows coverage.Follow-ups and known issues
This branch intentionally contains no domain skill packages so it can merge independently. As Tasks, Jobs, or Rooms land, the same workflow discovers their committed tests without additional workflow edits.