Repository navigation
ci: lint workflows and test the PR check's key-sharing decision - #75
Merged
Merged
Conversation
This was referenced Oct 3, 2026
Merged
Contributor
Author
This was referenced Oct 3, 2026
scott-lowe-vapi
marked this pull request as ready for review
October 3, 2026 07:19
This was referenced Oct 3, 2026
scott-lowe-vapi
force-pushed
the
test/docs-integrity
branch
from
October 6, 2026 22:50
5301080 to
4d95467
Compare
scott-lowe-vapi
force-pushed
the
ci/workflow-hardening
branch
from
October 6, 2026 22:50
69959a3 to
1aa43fe
Compare
Contributor
Author
Merge activity
|
scott-lowe-vapi
changed the base branch from
test/docs-integrity
to
graphite-base/75
October 7, 2026 18:02
The PR check workflow decides whether a pull request gets this repository's Vapi keys. That decision was untested bash, and no workflow had ever been through actionlint. - tests/vapi-checks-workflow.test.ts runs the workflow's real steps from vapi-checks.yml with bash: the live-or-dry-run decision across own branches, manual runs, forks (including a look-alike repo name), Dependabot as actor or author, a missing head repo and other events; the run step's arguments (dry vs live, named check, merge-base selection, with and without a base ref) using a stub node; empty key variables being unset; and statically, that secrets are gated on a live run, the triggers are pull_request and workflow_dispatch only, the permissions are contents: read and statuses: write, and checkout doesn't persist credentials. Dropping the Dependabot-author guard fails the decision test. - The decision step now also requires a pull_request event before going live, so adding a trigger later can't widen who gets the keys. - ci.yml gets an actionlint job: a pinned release whose sha256 is checked before it runs, with shellcheck linting every run: block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scott-lowe-vapi
force-pushed
the
ci/workflow-hardening
branch
from
October 7, 2026 18:04
1aa43fe to
302f4be
Compare
scott-lowe-vapi
added a commit
that referenced
this pull request
Oct 7, 2026
## Value **V.A.L.U.E. tier:** small — touches `.github/workflows/` (a blast-radius path), and changes what customer forks see on their pull requests. - **Problem:** nothing ran `npm run validate` before merge. `apply`, and so promotion, refuses to deploy a config with validation errors: a name over 40 characters, or a per-provider voice schema error. (The validator's other rules, structured-output lockstep, duplicated prompts and the `maxTokens` floor, are warnings and don't fail it.) That refusal came **after** merge: `main` held a config that wouldn't deploy, and promotion stopped until someone opened a fix PR. Plain `push` only warns, then can fail partway with an API 400. Each rule in the validator comes from a real mid-push failure (`improvements.md` #8, #9, #11, #18, #19). - **Who it affects:** every repository on this template, and especially multi-org repos that promote from `main`. - **What changes:** - **A new Validate resources job in `ci.yml`** runs `validate` for every folder under `resources/` on every pull request. It reports every failing org rather than stopping at the first. - **No engine change.** `validate` makes no network call; loading the engine's config only requires a key to be set, so the step sets a placeholder key and an unroutable base URL, so nothing can be sent. The job has no secrets, so it runs the same on forks and Dependabot PRs. - **It's in `ci.yml`, not the PR check workflow,** so every fork gets it without turning on PR checks. On this template, which has no org folders, it does nothing. - **Docs:** - the README quick start; - the `AGENTS.md` change loop: if the check fails, fix the errors and don't weaken the check; - the workflows guide: make it a required check; - the PR checks and troubleshooting guides; - `improvements.md` #37. **Heads-up for forks:** plain `push` only warned about these errors, so a repo may already carry some. The first PR after this lands will show them, whatever it changes. The troubleshooting guide covers it. `apply` already refused those configs, so this moves an existing failure earlier rather than adding a new one. ## Evidence of value `tests/ci-validate-workflow.test.ts` runs the job's real step, read from `ci.yml`, against copies of the starter example: | Case | Result | | --- | --- | | No org folders | passes, "nothing to validate" | | Two valid orgs | passes, both validated | | One org with a 41+ character assistant name, one valid | fails; both validated, the error names only the bad org and the reason ("Vapi caps at 40") | | A folder that isn't a valid org name | fails, naming it | | The job's secrets | none; checkout doesn't persist credentials; the only key is the placeholder | **Mutation:** making the loop ignore `validate`'s exit code fails the two failure-case tests. ## Testing plan - `npm test` (514 tests) and `npx tsc --noEmit` pass. - actionlint, from #75, lints the new job in this PR's CI. - **Not tested:** a customer repo with real resources. The validator itself is unchanged, and `apply` already runs it on every deploy. Refs TEST-141 ## After review - **Install:** `npm ci --ignore-scripts`, which skips the native audio builds `validate` never loads. - **Reproducing locally:** the failure message, the troubleshooting guide and AGENTS.md give a placeholder-key command, so nobody fetches a production key for an offline check. - **Docs:** - `workflows.md` says that requiring the check ties every team's PRs to every org's health, and that merge queues need a `merge_group:` trigger; - CI validates `.ts` resources without `.env.<org>`; - "the same validator `apply` runs" instead of "the same checks". - **Starter example:** its one-sided structured-output link is fixed, and the test asserts it validates with no warnings. - **Tests:** the no-secrets test also checks the job's permissions and that `pull_request_target` isn't a trigger. - **improvements.md #37:** reworded (errors vs. warnings) and carries the PR number. Annotations for warnings come from #77, so I didn't add the `sed` version here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Value
V.A.L.U.E. tier: small — touches
.github/workflows/(a blast-radius path) and the logic deciding which PRs receive the repository's Vapi keys.tests/vapi-checks-workflow.test.tsruns the workflow's real steps, read fromvapi-checks.yml, with bash:pull_request_targetevent all run dry;node: dry vs live, a named check on dispatch,--changed-sinceonly when the base ref exists, and empty key variables unset;pull_requestandworkflow_dispatch, permissions arecontents: readandstatuses: write, and checkout doesn't persist credentials.pull_requestevent before going live, so adding a trigger later can't widen who gets the keys.ci.ymlgains anactionlintjob: a pinned release whose sha256 is verified before it runs. It also runs shellcheck over everyrun:block.Evidence of value
Testing plan
npm test(509 tests) andnpx tsc --noEmitpass.VAPI_CHECKS_ENABLED.Refs TEST-141
🤖 Generated with Claude Code