test: add a sync-local regression suite and wire up CI - #17
Merged
Conversation
Closes the two gaps the simplification sweep left open: the repo had no tests
and no CI, and the PSScriptAnalyzer dependency the sweep started relying on was
recorded nowhere.
scripts/test-sync-local.sh — 49 assertions, all passing. Every case runs BOTH
twins against the same fixture and asserts the same exit code and byte-identical
stdout, because a divergence between documented parity twins is itself a defect.
Only stdout is compared: PowerShell wraps stderr in its own exception frame, so
the twins' error text matches while their framing does not. pwsh is optional --
without it the PowerShell half reports as skipped and the bash half still runs.
Fixtures are built under $TMPDIR and HOME is redirected per case, so the suite
never touches a real ~/.cursor.
It pins the defects that were found the hard way, each of which passed a linter
before it was found:
- path traversal via a plugin `name` and via an entry `source`
- a metadata.pluginRoot that escapes the source repo
- an empty plugins/ dir and "plugins": [], which used to exit 0 printing
"Synced (0)" -- a false pass in CI
- a nameless marketplace entry, which used to dump a Python traceback
- prefix-name sort order (ai before ai-briefing)
- a symlinked plugin directory, which must install as a REAL directory
scripts/validate-manifests.mjs — validates the manifests against Cursor's
PUBLISHED schemas rather than its prose reference. The two disagree in both
directions: the prose lists plugin-entry fields the schema forbids, and marks
`owner` required where the schema does not. A prose-derived checklist would pass
files a validator rejects. Verified both ways -- it passes the current manifests
and fails a fixture carrying `category`/`tags` on a marketplace entry, which is
exactly the state this repo was in before the sweep.
.github/workflows/ci.yml — four jobs: shell (shellcheck + the regression suite),
powershell (PSScriptAnalyzer, printing -SuppressedOnly so suppressions stay
visible rather than silently disappearing), manifests (jq + the schemas, fetched
at run time so an upstream schema change surfaces here), and links (internal
markdown only).
Two deliberate exclusions in CI, both to avoid failures that say nothing about
the repository: external URLs are not checked, because GitHub and cursor.directory
answer CI runners with 403/429; and the repo-root files synced from
melodic-software/standards are not re-linted, since they are validated upstream
and must not be hand-edited here.
The link checker strips inline code spans before extracting links. Without that
it reports a false positive on the plugin-name pattern
^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$, which looks exactly like ](...) to a naive
matcher. Verified it still catches a genuinely broken link.
AGENTS.md — replaced "no test framework" with how to run the suite, and added a
table of which lint/test tools are NOT preinstalled and how to install each
(shellcheck, pwsh, PSScriptAnalyzer, node+ajv), keeping the runtime dependencies
(bash, git, python3, jq) separate from the checking dependencies. Documents the
schema-vs-prose conflict, the zero-findings PSScriptAnalyzer rule and its
justification requirement, and what CI does and does not cover. The existing
claim about pwsh being absent from the base image is untouched: it is scoped to
Cursor Cloud's image, which is not observable from a dev container.
.shellcheckrc scope widened from sync-local.sh to scripts/*.sh so the new script
is covered; nothing is disabled and no `# shellcheck disable=` directive exists
in the tree. The SC2016 that the new script would have tripped was designed out
(pwsh --version) rather than suppressed.
Verified: shellcheck 0 across scripts/*.sh; bash -n clean; PSScriptAnalyzer 0;
test suite 49/49 with both twins; manifest validator passes real manifests and
fails a bad fixture; link check 0 broken; workflow YAML parses and every step
carries a uses/run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Ejst51HQobnYvjLcW1bgJ
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The first CI run failed all four jobs at "Set up job", before any repository code executed: The action actions/checkout@v4 is not allowed in melodic-software/cursor-plugins because all actions must be pinned to a full-length commit SHA. That is an organization policy (this repo's settings are managed by melodic-software/github-iac), not a defect in the workflow logic. A version tag is mutable, so pinning to one lets a compromised or retagged release run with the workflow's permissions; the policy forecloses that. Pinned to fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09, resolved from the upstream repository rather than copied from memory: refs/tags/v5 and refs/tags/v5.1.0 both dereference to that commit, and it is 40 hex characters. The version is recorded in a comment above each use so the pin stays readable and can be bumped deliberately. Also moves from v4 to v5 while pinning, since the pin is being written fresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Ejst51HQobnYvjLcW1bgJ
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b9fd3d214
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| on: | ||
| push: | ||
| branches: [main] |
There was a problem hiding this comment.
The branches: [main] filter prevents this workflow from running when changes are pushed to any feature branch without an open PR, despite both the workflow comment and AGENTS.md specifying that these checks run on every push and pull request. Remove the branch filter (or explicitly include all intended branches) so pre-PR pushes receive the documented validation.
AGENTS.md reference: AGENTS.md:L97-L101
Useful? React with 👍 / 👎.
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
2 participants
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.
Closes the two gaps #16 left open: the repo had no tests and no CI, and the PSScriptAnalyzer dependency that PR started relying on was recorded nowhere.
scripts/test-sync-local.sh— 49 assertionsEvery case runs both twins against the same fixture and asserts the same exit code and byte-identical stdout, because a divergence between documented parity twins is itself a defect. Only stdout is compared — PowerShell wraps stderr in its own exception frame, so the twins' error text matches while their framing does not.
pwshis optional: without it the PowerShell half reports as skipped and the bash half still runs. Fixtures build under$TMPDIRandHOMEis redirected per case, so it never touches a real~/.cursor.It pins the defects found the hard way in #16 — every one of which passed a linter before it was found:
nameand entrysourcerm -rfoutside the plugins rootmetadata.pluginRootescaping the repoplugins/and"plugins": []Synced (0)— a false CI passai-briefingsorted beforeaiscripts/validate-manifests.mjsValidates against Cursor's published schemas, not its prose reference. The two disagree in both directions: the prose lists plugin-entry fields the schema forbids, and marks
ownerrequired where the schema does not — so a prose-derived checklist would pass files a validator rejects.Verified both ways: it passes the current manifests and fails a fixture carrying
category/tagson a marketplace entry, which is exactly the state this repo was in before #16..github/workflows/ci.ymlFour jobs — shell (shellcheck + the suite), powershell (PSScriptAnalyzer, printing
-SuppressedOnlyso suppressions stay visible rather than silently disappearing), manifests (jq + schemas fetched at run time, so an upstream schema change surfaces here), and links (internal markdown only).Two deliberate exclusions, both to avoid failures that say nothing about the repo:
The link checker strips inline code spans before extracting links — without that it false-positives on
^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$, which looks exactly like](...)to a naive matcher. Verified it still catches a genuinely broken link.AGENTS.mdReplaced "no test framework" with how to run the suite, and added a table of the tools that are not preinstalled and how to install each — keeping runtime deps (
bash,git,python3,jq) separate from checking deps (shellcheck,pwsh,PSScriptAnalyzer,node+ajv). That was the specific gap: #16 came to depend on PSScriptAnalyzer with nothing recording it.The existing claim that
pwshis absent from the base image is untouched — it's scoped to Cursor Cloud's image, which isn't observable from a dev container. Note GitHub's runners do preinstall pwsh, so CI exercises both twins even where Cursor Cloud may not..shellcheckrcscope widened toscripts/*.sh. Nothing is disabled and no# shellcheck disable=directive exists in the tree — the SC2016 the new script would have tripped was designed out (pwsh --version) rather than suppressed.Verification
scripts/*.shbash -nuses/runLimits
CI itself has not run yet — it can only execute once this is on GitHub. I validated the workflow by parsing the YAML and running every job's commands locally;
actionlintwas not available in this container, so the workflow file has not been linted by the tool that would normally check it. Windows behavior of the.ps1remains unverified — the suite runs both twins on Linux only.Generated by Claude Code