ci(openspec): enforce archive-at-merge with a gate + /archive workflow#185
Open
gohabereg wants to merge 4 commits into
Open
ci(openspec): enforce archive-at-merge with a gate + /archive workflow#185gohabereg wants to merge 4 commits into
gohabereg wants to merge 4 commits into
Conversation
Add CI that keeps openspec/specs/ in sync with merged changes, plus a round of hardening on the existing package-CI workflows found along the way. OpenSpec archiving: - openspec-archive-gate: fails a PR / merge-queue build when a change has every task checked but is still un-archived. merge_group is authoritative; the openspec:defer-archive label softens the PR check only. - openspec-archive: maintainer comments `/archive` (or `/archive <name>`); runs the pinned openspec CLI, folds delta specs into openspec/specs/, and commits to the PR branch so the spec diff is reviewable. - Pin @fission-ai/openspec as a devDependency; document the flow in CONTRIBUTING.md and the authoring rules in openspec/config.yaml. Workflow hardening (existing CI): - Fix a command-injection vector: PR comment body and changed-file names were spliced into run: scripts; route them through env instead, and verify commenter write-access before running /archive. - should-run-ci.sh: force CI when shared/root files change (lockfile, workflows, root manifests) so a package can't skip CI having tested nothing. - Add concurrency guards to npm-publish and docker-image; least-privilege permissions blocks; pin third-party actions (tj-actions/changed-files, softprops/turnstyle) to full SHAs. - Remove a dead test-skip guard, a redundant PR-number action, and a broken/unused output reference in aggregate-report. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Unit Tests
Mutation Tests
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds CI enforcement and automation around the OpenSpec “archive at merge” workflow so that completed changes can’t merge without folding their delta specs into openspec/specs/. It also hardens existing GitHub Actions workflows against injection risks and improves CI correctness/robustness (root-change detection, pinned third-party actions, explicit permissions, and concurrency controls).
Changes:
- Introduces an “OpenSpec archive gate” workflow and a Node gate script to block merges when a change is fully complete but not archived.
- Adds a maintainer
/archiveissue-comment workflow that runsopenspec archiveand commits the folded specs + archived change back onto the PR branch. - Hardens existing workflows/actions (root-change CI forcing, action pinning, quoting/env routing, concurrency & permissions) and documents the new OpenSpec flow.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Pins @fission-ai/openspec as a devDependency for deterministic archiving in CI. |
yarn.lock |
Adds the lock entries for @fission-ai/openspec@1.6.0 and its dependency tree. |
openspec/config.yaml |
Documents delta-authoring constraints required for deterministic CLI folding. |
CONTRIBUTING.md |
Updates contributor workflow docs to reflect “archive-at-merge-time” enforcement and /archive usage. |
.github/workflows/openspec-archive-gate.yml |
Adds the PR/merge-queue gate workflow that runs the archive readiness check. |
.github/scripts/openspec-archive-gate.mjs |
Implements the “completed tasks but not archived” detection logic used by the gate and /archive. |
.github/workflows/openspec-archive.yml |
Adds the /archive issue-comment workflow that runs openspec archive and pushes a commit to the PR branch. |
.github/scripts/should-run-ci.sh |
Forces CI when shared/root files change (lockfile, root manifests, workflows), preventing false “green” PRs. |
.github/workflows/npm-publish.yml |
Adds least-privilege permissions and serializes publishes to avoid version-race conditions. |
.github/workflows/build-and-push-docker-image.yml |
Adds explicit permissions and concurrency to prevent tag races on rapid merges. |
.github/actions/unit-tests/action.yml |
Removes a main-branch coverage skip so coverage runs consistently. |
.github/actions/mutation-tests-changed-files/action.yml |
Pins changed-files action and routes filenames via env to avoid shell injection. |
.github/actions/mutation-tests-all-files/action.yml |
Pins changed-files and improves shell safety via env + quoting. |
.github/actions/aggregate-report/action.yml |
Removes unused/broken output wiring and pins turnstyle; uses PR number directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+20
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
|
|
Comment on lines
+84
to
+96
| const list = violations.map((c) => ` - ${c}`).join('\n'); | ||
| const message = | ||
| `OpenSpec archive gate: the following change(s) are complete but not archived:\n${list}\n\n` + | ||
| `Run \`openspec archive -y <change>\` (or comment \`/archive\` on the PR) so the ` + | ||
| `delta specs fold into openspec/specs/ before this merges.`; | ||
|
|
||
| if (deferred) { | ||
| console.warn(`::warning::${message}\n(Deferred via openspec:defer-archive — not blocking.)`); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| console.error(`::error::${message}`); | ||
| process.exit(1); |
Comment on lines
+61
to
+62
| run: | | ||
| echo "${{ steps.changed-files.outputs.src_all_changed_files }}" | sed 's/,/","/g' | sed 's/^/["/;s/$/"]/' > changed-files.json | ||
| printf '%s' "$CHANGED_FILES" | sed 's/,/","/g' | sed 's/^/["/;s/$/"]/' > changed-files.json |
- openspec-archive.yml: split into archive-run (read-only token, runs the PR branch's code) and archive-commit (write token, runs no PR code and only applies the resulting patch), so the write token is never in scope while untrusted code executes — resolves the CodeQL untrusted-checkout TOCTOU alerts. Add issues: write for the comment-reaction endpoint. - openspec-archive-gate.mjs: percent-encode ::warning::/::error:: messages so multi-line details aren't truncated in the annotation. - mutation-tests-changed-files: build changed-files.json with jq instead of sed so filenames containing " or \ can't produce invalid JSON. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CodeQL flagged archive-commit checking out the PR head (untrusted code) while
holding the write token ("checkout of untrusted code in a privileged context").
Build the archive commit in the unprivileged archive-run job and ship it as a
thin git bundle. archive-commit now checks out only the trusted default branch;
the untrusted objects arrive via `git fetch` (never a working-tree checkout,
never executed) and are pushed straight to the PR branch. A non-fast-forward
still fails rather than clobbering a moved branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
archive-run executes the PR's own code (setup lifecycle scripts, openspec archive) with a read-only token and ships the result as a git bundle; archive-commit unbundles and pushes it with the write token, but never validated that the bundle actually contained what archive-run was supposed to produce. Verify the bundled commit's parent matches the exact PR head archive-run resolved, and that its diff only touches openspec/, before pushing. This is real defense-in-depth against a compromised archive-run (e.g. a malicious package.json script), and gives a concrete basis to review CodeQL alerts #37/#38 (untrusted-checkout) against this job — it already follows GitHub's documented mitigation of never executing PR code in the privileged job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
What & why
Adds CI to keep
openspec/specs/in sync with merged changes, so a completed OpenSpec change can't merge without its delta specs being folded in. While building it I audited the existing.github/workflows and folded in a round of hardening.OpenSpec archiving
openspec-archive-gate— fails a PR / merge-queue build when a change has every task intasks.mdchecked but is still sitting inopenspec/changes/. Changes with unchecked tasks pass, so a multi-PR change only needs archiving in the PR that completes it.merge_groupis the authoritative run; theopenspec:defer-archivelabel softens the PR check only. Runs locally:node .github/scripts/openspec-archive-gate.mjs.openspec-archive— a maintainer comments/archive(or/archive <change-name>). Runs the pinnedopenspec archiveCLI, folds delta specs intoopenspec/specs/, and commits to the PR branch so the spec diff is reviewable and lands atomically with the code.@fission-ai/openspecas a devDependency; documents the flow inCONTRIBUTING.mdand the delta-authoring rules inopenspec/config.yaml.Workflow hardening (existing CI)
run:scripts; routed throughenv:instead. Added a write-access check on the commenter before/archiveruns (a job that holdscontents: write).should-run-ci.sh— force CI when shared/root files change (lockfile, workflows, root manifests), so a package can't report "no CI needed" and merge green having tested nothing.npm-publishand the docker image build; explicit least-privilegepermissions:blocks; third-party actions (tj-actions/changed-files,softprops/turnstyle) pinned to full commit SHAs.github.ref != refs/heads/maintest-skip guard, a redundant PR-number action, and a broken/unused output reference inaggregate-report.Testing
openspec-archive-gate.mjsandshould-run-ci.shpassbash -n/ node execution;should-run-ci.shbehavior verified across root-change, unrelated-change, package-change, and dependency-change scenarios.should-run-ci.shworkspace-resolution rewrite onmain.🤖 Generated with Claude Code