Skip to content

feat(promotion): gate promotion out of an org on a passing check - #66

Merged
scott-lowe-vapi merged 1 commit into
mainfrom
feat/promotion-check-gate
Oct 7, 2026
Merged

scott-lowe-vapi merged 1 commit into
mainfrom
feat/promotion-check-gate

Conversation

@scott-lowe-vapi

@scott-lowe-vapi scott-lowe-vapi commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Value

V.A.L.U.E. tier: project — PR 10 of 10 for inline simulation PR checks (TEST-141), the "check before deploy" step in promotion.

Stacked on #65 (the promotion partial-failure fix), now that #57–#64 have merged. This PR is the single gate commit.

  • Problem: promotion copies staging's reviewed files into production, but nothing checks that staging's agents still behave before they move on. Teams promoting dev → staging → prod need a behaviour gate between orgs, without new infrastructure.
  • Who it affects: multi-org gitops users (the promotion pipeline), who get a "check before deploy" step with one line of promotion.yml. Single-org users are unaffected.
  • What changes:
    • promotion.yml accepts orgs.<slug>.check: <name> (a slug), naming a vapi-checks.yml check.
    • New src/promotion-gate.ts:
      • Validation before any transition: the check must exist, and its org and runOrg must be the gated org, otherwise the run errors. vapi-checks.yml is required once any org is gated.
      • Plan line: what the gate would run, built offline.
      • Live gate: the check runs live and reduces to the worst target result.
    • src/promote-cmd.ts: in each transition, after the plan is built:
      • no changes skips the gate;
      • plan-only prints check would run <name> in <org> (<n> simulations × <t> targets);
      • --apply runs the check (after the bindings refresh, before promotionPlanApply writes anything). Any non-pass throws Promotion out of <org> blocked: check <name> <outcome> (<run url>).
      • A pass is cached per source org and dropped once a transition applies into that org.
      • promotionCommandRun(args, overrides) now takes Partial<PromotionDeps> (childRun, checkRun).
    • .github/workflows/promotion.yml: timeout-minutes: 90 on the "Reconcile configured promotions" step, not the job, so the if: always() commit step (fixed in fix(promotion): commit the files of transitions that applied when a later one fails #65) still runs after a blocked or slow gate.
    • Docs: promotion.example.yml (a commented check:), a README "Check before promoting" section, and a pointer from "PR Checks".

Evidence of value

The real gate, run live in the owner's test org on the TEST-141 parity squad.

  • Setup: a scratch repo whose promotion.yml gates parity on check core, with pipeline parity → parity-prod.
  • The run: promote --pipeline release --from parity --to parity-prod --apply.
  • The fake: the child runner was faked, so bindings pulls were no-ops and the downstream apply.ts was recorded but not run. No second org was needed or touched.
Variant Gate run Result Downstream apply resources/parity-prod/
Degraded scheduler prompt 7ed19587: 2 of 3 failed Promotion out of parity blocked: check core failed (https://dashboard.vapi.ai/simulations/run/7ed19587-…) none empty (nothing written)
Fixture as-is 95470670: 3 of 3 passed promoted ["parity-prod"] written; 20 applied paths recorded

The test org's resource counts were identical before and after both gate runs.

Tests: npm test goes from 484 (#65) to 492 passing, and #68's golden promotion test passes unchanged.

Testing plan

  • tests/promotion-gate.test.ts (6 tests, real git fixture, injected childRun / checkRun):
    • a pass applies;
    • failed and incomplete both block with the exact message, with no apply and the target untouched;
    • plan-only prints the line and runs nothing;
    • no changes skips the gate;
    • the three config errors (no vapi-checks.yml, unknown check, check in another org) stop before anything applies;
    • the pass cache: reused for two pipelines out of one org, and re-run after a transition applies into the gated org.
  • No gate configured, no change: with no check: in promotion.yml and an invalid vapi-checks.yml present, plan and --apply both succeed, checkRun is never called, and the plan output equals a pinned string. That string is exactly what fix(promotion): commit the files of transitions that applied when a later one fails #65's code (before the gate existed) prints for the same fixture, which I confirmed by running fix(promotion): commit the files of transitions that applied when a later one fails #65's promote-cmd on it. So the gate is invisible unless someone opts in.
  • tests/promotion.test.ts: orgs.<slug>.check is parsed, and a non-slug is rejected.
  • Not tested:
    • A real two-org promotion: only one test org was available. The downstream apply was faked, so the blocked case shows nothing written, and the pass case shows the apply was called.
    • A GitHub Actions promotion run with a gate, including the step timeout firing.
  • Found while testing (pre-existing, out of scope): promotion's dependency check rejects simulations that reference a stock personality by UUID, with "Referenced managed dependency is missing from source: personalities/a0000000-…". So a gated org's tests need local personality files until that's fixed.

Stacked on #65.

Refs TEST-141

After review

The gate now refuses, when the config loads and before anything applies:

  • an unknown key under an org in promotion.yml, so a misspelled check: can't silently drop the gate;
  • toolMocks: off and stripWebhooks: false, because a gate runs in the real org, never a CI org;
  • a check baseUrl that differs from the org's baseUrl in promotion.yml, so the org's key only goes to the host promotion uses (the gate always uses that host);
  • a gate on an org that is last in every pipeline, where it would never run;
  • gated checks whose combined budget is over 300 minutes.

It also fixes:

  • Deadline: each batch of 3 targets gets a full timeoutMinutes, so a check with more than 3 targets is no longer falsely blocked as incomplete.
  • Step timeout: raised from 90 to 330 minutes, as a safety net that no longer cuts short long ungated promotions.
  • Tests: the deadline and the worst-target rule are pure helpers with their own tests.

The guide changes (blocks stop the whole run, simulation cost, the stock-personality limitation, accurate wording) are in #71. The promote User-Agent for gate runs is in #78. The block-report detail, fetch-stubbed gate test and deduplication are follow-ups.

🤖 Generated with Claude Code

scott-lowe-vapi commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Oct 3, 5:59 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 3, 6:14 AM UTC: Graphite couldn't merge this PR because it had merge conflicts.
  • Oct 7, 5:47 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 7, 5:51 PM UTC: Graphite rebased this pull request as part of a merge.
  • Oct 7, 5:51 PM UTC: @scott-lowe-vapi merged this pull request with Graphite.

@scott-lowe-vapi
scott-lowe-vapi changed the base branch from feat/vapi-checks-workflow to graphite-base/66 October 3, 2026 06:12
@scott-lowe-vapi
scott-lowe-vapi changed the base branch from graphite-base/66 to main October 3, 2026 06:13
@scott-lowe-vapi
scott-lowe-vapi changed the base branch from main to graphite-base/66 October 3, 2026 06:18
@scott-lowe-vapi
scott-lowe-vapi force-pushed the feat/promotion-check-gate branch from 849c59f to 95379be Compare October 3, 2026 06:18
@scott-lowe-vapi
scott-lowe-vapi changed the base branch from graphite-base/66 to fix/promotion-commit-applied-transitions October 3, 2026 06:18
@scott-lowe-vapi
scott-lowe-vapi force-pushed the fix/promotion-commit-applied-transitions branch from ef619e1 to c07fed1 Compare October 3, 2026 06:28
@scott-lowe-vapi
scott-lowe-vapi force-pushed the feat/promotion-check-gate branch from 95379be to 84daba8 Compare October 3, 2026 06:28

@dhruva-vapi dhruva-vapi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, it fails closed on anything short of every target passing and a pass gets dropped once something is promoted into that org

couple of non-blocking things:

  • the promotion step timeout is 90 min but a check's timeoutMinutes can go up to 120, and two gated orgs in one run add up, so a long check just gets killed by the step instead of reporting a result. can we check the gated timeouts against that budget in promotionChecksLoad
  • this runs the sims again at promotion time on top of the PR check, so every change gets simulated twice. fine as a choice but can we call out the cost in the README so customers aren't surprised

@chris-garber-vapi chris-garber-vapi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core design holds up: the gate fails closed, the pass cache is invalidated correctly (I checked that pull --bindings-only only writes state, so a cached pass can't go stale between transitions), and an ungated promotion.yml doesn't change behavior. Tests and tsc --noEmit pass on 84daba8.

Most important, highest first:

  • 🟠 A typo'd check: key turns the gate off with no error (confirmed on this branch).
  • 🟠 toolMocks: off checks run real tools in the gated org, because runOrg can't be a CI org.
  • 🟠 Checks with more than 3 targets can be falsely blocked as incomplete, because timeoutMinutes is also the whole check's budget.
  • 🟠 The 90-minute step timeout now applies to ungated promotions too.

Severity: 🔴 blocker · 🟠 should fix before merge · 🟡 should fix, can be stacked · 🟢 nit.

Comment thread src/promotion.ts
Comment thread src/promotion-gate.ts
Comment thread src/promotion-gate.ts Outdated
Comment thread .github/workflows/promotion.yml Outdated
Comment thread src/promote-cmd.ts
Comment thread src/promotion-gate.ts
Comment thread src/promotion-gate.ts
Comment thread src/promotion-gate.ts
Comment thread README.md
Comment thread README.md
@scott-lowe-vapi
scott-lowe-vapi force-pushed the fix/promotion-commit-applied-transitions branch from c07fed1 to 4e7a5e0 Compare October 6, 2026 22:50
@scott-lowe-vapi
scott-lowe-vapi force-pushed the feat/promotion-check-gate branch from 84daba8 to 4274aee Compare October 6, 2026 22:50
@scott-lowe-vapi

Copy link
Copy Markdown
Contributor Author

@dhruva-vapi both done:

  • Timeouts: gated checks' combined budget (timeoutMinutes per batch of 3 targets, across every gated org) is checked against 300 minutes when the config loads, and the step cap is now 330 minutes, as a safety net. A long check is rejected up front instead of being killed.
  • Cost: the promotion guide's "Check before promoting" section (in docs: edit the guides for new readers and fix inaccuracies #71) says each gated promotion runs its simulations again, on top of the PR check, and uses simulation minutes.

🤖 Generated with Claude Code

scott-lowe-vapi added a commit that referenced this pull request Oct 7, 2026
… the target (#68)

## Value

**V.A.L.U.E. tier:** small — a tests-only baseline under the remaining promotion changes for TEST-141 ([TEST-141](https://linear.app/vapi/issue/TEST-141/gitops-run-simulation-suites-against-pr-changes-inline-as-ci-checks)). It changes no behaviour. It's small rather than micro because it guards a blast-radius path: promotion into production.

- **Problem:** the code that moves assistants from a lower org into production is the riskiest path in gitops. The existing tests check pieces of it (canonicalization, bindings, deletions) but never the full set of files a promotion writes. So a refactor could change what lands in prod without any test failing.
- **Who it affects:** teams that promote resources between orgs (dev → staging → prod). It's a baseline for future changes to promotion's plan/apply core. The stacked PRs don't touch that core: #65's commit behaviour is covered by its own `promote-cmd` and workflow-commit tests, and #66 only adds the `check` field.
- **What changes:** new `tests/promotion-golden.test.ts`. It runs one staging → prod promotion through `promotionPlanBuild` and `promotionPlanApply`, then asserts the plan and every file left in the target org:
  - source UUIDs canonicalized to names (`toolIds`, a handoff destination);
  - referenced tools and assistants pulled in as dependencies, and an unreferenced staging-only tool left behind;
  - a credential bound by name, and a phone number bound to the target's;
  - a target file the source no longer has deleted;
  - an unrelated target file left untouched;
  - the markdown prompt body preserved;
  - the source org never written.

  A second test re-plans right after the apply and expects no changes.

## Evidence of value

**Mutation check:** each deliberate break of `src/promotion.ts` turns the test red, and it's green again once the break is reverted.

| Break | Golden test |
|---|---|
| Phone binding returns the source phone | ❌ fails |
| Credential binding returns the source UUID | ❌ fails |
| UUID → name canonicalization disabled | ❌ fails |
| None (`main`) | ✅ passes |

The expected output was read line by line against promotion's documented behaviour before it was pinned.

**Tests:** `npm test` goes from 474 to 476 passing.

## Testing plan

- `tests/promotion-golden.test.ts`: the golden promotion, and idempotence on re-plan.
- **Not tested:**
  - the child `pull`/`apply` processes (this exercises the file-writing core, not the CLI);
  - a real two-org promotion (manual QA before #65 and #66 merge).

Base of the promotion stack: #68 ← #65 ← #66.

Refs TEST-141

## After review

Chris's mutation-tested suggestions, each verified to fail the golden when the code it guards is broken:

- `.vapi-ignore`d prod files are left alone (an ignored, prod-only `assistants/hotline.yml`).
- The whole repository is snapshotted, so a stray write to `.env.*` or a state file fails it.
- Billing arrives only as a dependency of a dependency (intake → `handoff-to-billing` → billing).
- A file promotion doesn't rewrite is copied byte for byte (it keeps a comment).
- The prompt body has a heading, blank lines and a `---` separator.

Not taken: the `omit` phone case (`promotion.test.ts` already covers it), the unused state entries, and the input/output formatting pass, which is a follow-up.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@scott-lowe-vapi
scott-lowe-vapi changed the base branch from fix/promotion-commit-applied-transitions to graphite-base/66 October 7, 2026 17:48
@scott-lowe-vapi
scott-lowe-vapi changed the base branch from graphite-base/66 to main October 7, 2026 17:49
`orgs.<slug>.check: <name>` in promotion.yml names a vapi-checks.yml
check that must pass in that org before any transition promotes out of
it. The gate runs the same inline check as the PR workflow, built from
the source org's files at the promoted commit, using that org's key from
VAPI_PROMOTION_TOKENS.

- Checks are validated before any transition: the named check must exist
  and read and run in the gated org.
- Transitions with no changes skip the gate; plan-only runs print
  `check  would run <name> in <org> (<n> simulations × <t> targets)`
  and run nothing.
- On --apply the check runs after bindings refresh and before
  promotionPlanApply writes the target. Any non-pass (failed,
  incomplete, build error) throws `Promotion out of <org> blocked: check
  <name> <outcome> (<run url>)`, so the target is untouched and earlier
  transitions are still committed (previous change).
- A pass is reused for later transitions out of the same org in the same
  run, and dropped once a transition applies into that org.
- The "Reconcile configured promotions" step gets timeout-minutes: 90 on
  the step, not the job, so the always() commit step still runs.
- With no check: configured, promotion is unchanged: a test pins the
  plan output to what the pre-gate code prints, and asserts no check runs
  and vapi-checks.yml is never read.
- Docs: promotion.example.yml and README ("Check before promoting", and a
  pointer from "PR Checks").

Refs TEST-141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scott-lowe-vapi
scott-lowe-vapi force-pushed the feat/promotion-check-gate branch from 4274aee to 4eecfc7 Compare October 7, 2026 17:50
@scott-lowe-vapi
scott-lowe-vapi merged commit b8115c0 into main Oct 7, 2026
4 checks passed
scott-lowe-vapi added a commit that referenced this pull request Oct 7, 2026
… add community files (#69)

## Value

**V.A.L.U.E. tier:** small — docs, examples and tests for the public launch of this repo, plus comment and fixture-name changes in code. No behaviour change. Not micro: it spans more than 8 files.

- **Problem:** we're about to link this repo from the Vapi docs, which will bring many more readers, and it wasn't ready for them.
  - **Leaks:** tracked files named customers and their products, described a customer's production incident, and cited internal ticket IDs and staff handles.
  - **Broken examples:** the README's simulation examples used fields the API rejects, so a new user copying them would fail on their first `apply`.
  - **No community files:** there was no `CONTRIBUTING.md` or `SECURITY.md`, so issues and vulnerability reports had nowhere to go.
- **Who it affects:** everyone arriving from the docs, and the customers named in the files.
- **What changes:**
  - **Scrub:** customer, product and person names, the incident's identifying details, an internal logging tool's name, and internal ticket IDs are removed from `CLAUDE.md`, `improvements.md`, `docs/learnings/`, code comments and test fixtures. The lessons are kept in neutral words, and renamed fixtures keep the same test meaning.
  - **Fixed examples:**
    - personalities need `assistant`, and scenarios need `instructions` and `evaluations`;
    - state files store `{"uuid": …}` entries;
    - squads hand off through handoff tools.
  - **`examples/starter/`:** a complete small org that the README's File Formats section is now built from.
  - **New `tests/examples.test.ts`**, checking that:
    - every example org passes `validate`;
    - every example has the fields the API requires;
    - the starter's PR check builds cleanly;
    - every doc snippet that starts with `# examples/<path>` matches that file exactly.
  - **`CONTRIBUTING.md`** and **`SECURITY.md`**. Security reports go through GitHub private vulnerability reporting, which is already enabled on the repo.
  - **Issue forms** (bug, feature, contact links) and a current `package.json` description.

## Evidence of value

- **Scrub:** a word-boundary search of every tracked file for the customer, product, person and ticket names now finds nothing; a broader search for internal tools and monorepo paths is also clean.
- **Examples test catches drift:** changing one character in `examples/starter/.../squads/front-desk.yml` turns the snippet test red, naming the file.
- **Starter dry run:** `npm run check -- core --dry-run` on the starter builds 1 simulation (3.5 KB) with no warnings.
- **Links:** every relative link and anchor across all 37 markdown files resolves.
- **Tests:** `npm test` goes from 492 to 496 passing.

## Testing plan

- `tests/examples.test.ts` (4 tests), plus the existing suite with renamed fixtures.
- **Not tested:**
  - a live deploy of the starter (it uses `example.com` tool URLs on purpose);
  - git history still contains the removed names. Rewriting public history is a separate decision.

Stacked on #66.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
scott-lowe-vapi added a commit that referenced this pull request Oct 7, 2026
)

## Value

**V.A.L.U.E. tier:** small — a behavior change: `validate`, and so `apply` and the Validate resources check, now fail on configs they used to pass.

- **Problem:** a reference that names no file fails in one of three ways, depending on the field (`improvements.md` #31):
  - it's **silently dropped**: `model.toolIds`, `artifactPlan.structuredOutputIds`;
  - it's **sent raw and rejected partway through a push**: squad members, hook tools, `personalityId`, `scenarioId`;
  - or it's **deferred** to a linking pass.

  `validate` never checked references, so the Validate resources check from #76 couldn't catch a typo'd tool name either. Warnings were also invisible in CI: they don't fail the check, and nobody reads the job log.
- **Who it affects:** everyone who edits resource files by hand or with a coding agent, and reviewers of their PRs.
- **What changes:**
  - **New `src/validate-refs.ts`**, run by `validate` (so by `apply` and CI) and by `push`:

    | Rule | Severity | Catches |
    | --- | --- | --- |
    | `dangling-reference` | error | A name with no local file and no state entry. Uses the shared reference walk, plus scenario judges' `evaluations[].structuredOutputId`. |
    | `malformed-reference` | error | An empty or non-name entry in a reference list (it used to crash `validate`). |
    | `override-tool-by-name` | error | A tool name in `toolIds` inside `assistantOverrides`, `membersOverrides` or `targetOverrides`, where push never resolves names. The fix it gives is `tools:append`, because `model.tools` there replaces the member's tool set. |
    | `unresolved-credential` | warning | A credential name missing from the state file. The message names the org's bootstrap pull. |
    | `reference-by-uuid` | warning | A UUID for a resource this repo tracks, which breaks promotion; it names the file to use. UUIDs the repo doesn't track (dashboard-owned, stock personalities) aren't reported. |
  - **`validate` now also runs `reference-to-ignored`,** as `push` already did. It reads the committed state file and stays offline.
  - **On GitHub Actions, every finding becomes an annotation,** so it shows on the file in the PR, warnings included.
  - **`push`** reports the new rules alongside its existing validators: warnings by default, blocking under `--strict`.
  - **Docs:**
    - a rule-by-rule table in troubleshooting;
    - the `validate` row in the commands guide;
    - `AGENTS.md`: never edit the state file to make a reference resolve;
    - `improvements.md` #31 marked resolved by validation.

## Evidence of value

The starter example with two typos, `scheduler` → `schedular` in the squad and `booking-confirmed` → `booking-confirmd` in a judge:

| | Before (#76) | After |
| --- | --- | --- |
| `npm run validate` | `0 error(s)` — ✅ Validation passed | `2 error(s)`, one `dangling-reference` per typo, naming the file and the missing name |

- **New `tests/validate-refs.test.ts`** covers:
  - names that resolve to local files and to state;
  - a typo in each reference field;
  - ignored references;
  - UUIDs and stock personalities;
  - all three override keys;
  - credentials as names, as UUIDs, and known to state.
- **Annotation format:** escaping of `%`, newlines, `:` and `,` is tested in `tests/validate.test.ts`.
- **End to end:** `tests/ci-validate-workflow.test.ts` runs the CI step with `GITHUB_ACTIONS=true` on the typo'd squad. The step fails, and the `::error` points at `resources/clinic/squads/front-desk.yml`.
- **Mutation:** dropping the judge collection or the override walk fails two tests.
- **Every example org still passes.** The cross-org promotion example (which has no state files) now shows an `unresolved-credential` warning, which is accurate.

## Testing plan

- `npm test` (523 tests) and `npx tsc --noEmit` pass.
- **Behavior to expect:** `apply` validates before it pulls. So a reference to a resource created in the dashboard and never pulled now stops `apply`; the message says to pull first. Before, `apply` went on to pull and push, and the reference resolved only if the pull happened to produce that exact name.
- **Not tested:** a live `apply` or `push`. Neither code path changed except for the added findings.
- **Not in this PR:** a check for secrets committed in resource files. It goes in its own PR, because a false positive there would block a customer's merge.

Refs TEST-141

## After review

- **Ignored files:** `validate` skips `.vapi-ignore`d files, as push does, so ignored files can't fail CI or `apply`.
- **One collector:** `referencesCollect` in `resolver.ts` feeds both `reference-to-ignored` and these rules, so a judge that references an ignored structured output is caught.
- **Lookups:** use `?.uuid`, so `constructor` and friends don't resolve.
- **Troubleshooting:** the table gives safe advice for ignored and override references, and AGENTS.md says never to edit the state file or `.vapi-ignore` to make a reference resolve.
- **improvements.md:** #31 is marked mitigated, with the remaining gaps listed (unchecked override paths, `toolRefs`, inline members, annotation lines). #38 records the promotion and stock-personality limitation found in #66.

Follow-ups: line numbers on annotations, the wider override walk, and deduplicating the credential walkers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants