Skip to content

Commit 3c1e10e

Browse files
fix(promotion): commit the files of transitions that applied when a later one fails (#65)
## Value **V.A.L.U.E. tier:** project — PR 9 of 10 for inline simulation PR checks ([TEST-141](https://linear.app/vapi/issue/TEST-141/gitops-run-simulation-suites-against-pr-changes-inline-as-ci-checks)). This is a fix to promotion, independent of the check stack (cut from `main`), and the promotion gate (PR 10) depends on it. - **Problem:** when a multi-transition promotion fails partway, the "Commit reconciled files and UUID state" step pushes **nothing**: not the UUID state, and not the files of transitions that already reached the platform. - **Why:** on failure the step stages only `.vapi-state.*.json`, but the failed transition's `promotionPlanApply` has already rewritten tracked files in its target org, so `git pull --rebase` refuses with "You have unstaged changes". - **Result:** git and the platform disagree, and the next promotion plans from stale files. - **Who it affects:** teams using `promotion.yml` with two or more transitions (dev → staging → prod), and anyone who adds the promotion gate in PR 10, where a failing gate is exactly a mid-run failure. - **What changes:** - **`src/promote-cmd.ts`:** - each `--apply` run truncates `tmp/promotion-applied.txt` (gitignored); - after each successful `apply.ts`, it records each path `git status --porcelain -z --untracked-files=all -- resources/<target>` reports (new, modified, deleted and renamed files, by current path), **with its content at that moment written to git's object store** (`git hash-object -w`; `-` for a deletion). These are read from git, not the plan, because apply's own pull and push can rewrite files the plan didn't name; - `promotionCommandRun(args, deps = { childRun })` gains a test seam. - **`.github/workflows/promotion.yml`:** on a non-success outcome, the commit step: 1. stages the state files plus exactly the recorded blobs (`git update-index --cacheinfo`; deletions with `git rm --cached`), and commits. Staging the recorded content, not the working tree, means a later failed transition that rewrote the **same** file can't replace what was applied; 2. runs `git reset --hard HEAD && git clean -fd -- resources`, so the failed transition's rewrites are discarded; 3. then rebases and pushes as before. On success it's unchanged: it commits all of `resources`. - **`improvements.md`:** #35, RESOLVED. ## Evidence of value **Red/green in a scratch repo.** - **Setup:** a bare origin plus a clone; the pipeline is `a → b → c`, and `c` starts with a tracked `assistants/intake.yml`. - **The run:** `promote --all --apply` runs this branch's `promote-cmd` with a fake child runner. Apply into `b` succeeds (writing state and one extra file, as apply's pull can). Apply into `c` writes state, then fails. - **Then:** the workflow's real commit step, extracted from each ref's `promotion.yml`, runs with `PROMOTION_OUTCOME=failure`. | | `main` (69c7e83) | This branch | |---|---|---| | Commit step | `error: cannot pull with rebase: You have unstaged changes.` **exit 128** | pushed, **exit 0** | | `origin/main` | unchanged (only `base`) | `chore: record promoted Vapi state [skip promotion]` | | `.vapi-state.b.json` / `.c.json` on origin | `{}` / `{}` (state lost) | `uuid-b` / `uuid-c` (both recorded) | | `resources/b/assistants/intake.yml`, `pulled-by-apply.yml` | missing | **committed** | | `resources/c/assistants/intake.yml` | old content | old content (the failed rewrite is discarded) | | `resources/c/assistants/pulled-by-apply.yml` | — | removed by `git clean` | `tmp/promotion-applied.txt` after the run held exactly the two `resources/b/...` paths. **Same-org case: red before the fix, green after.** Two pipelines promote into the same org in one `--all` run. The first applies `resources/c/assistants/intake.yml` with a's content, and the second rewrites that file with b's content and then fails. Before the fix, the committed file was b's (the failed rewrite); after it, the commit holds a's, the content that was actually applied. **Tests:** on top of #68, `npm test` goes from 476 to 484 passing. ## Testing plan - **`tests/promote-cmd.test.ts`** (3 tests, a real temp git repo, injected child runner): - one transition applies and the next fails: only the applied transition's files are recorded, including a file apply rewrote that the plan didn't name; - every `--apply` run starts a fresh record, and a plan-only run leaves it alone; - deleted and renamed files are recorded by their current path. - **`tests/promote-cmd.test.ts`** also checks that each applied file is recorded with its content at apply time (`git cat-file` of the recorded blob, after a later rewrite), and a deletion as `-`. - **New `tests/promotion-workflow-commit.test.ts`** (4 tests) runs the workflow's **real** commit step, read from `promotion.yml` and run with `bash -e`, against a clone of a bare origin, after a promotion driven through `promote-cmd`: - partial failure: state and the applied transition's files are pushed, and the failed rewrite is not; - the same-org case above; - success commits every change; - no changes pushes nothing. A later edit to the YAML that brings either bug back fails here. - The existing `tests/promotion.test.ts`, and #68's golden promotion test, pass unchanged. - **Not tested:** - a real GitHub Actions promotion run (the step runs locally under bash, read from the YAML) — manual QA with two non-production orgs before merge. Stacked on #68. Refs TEST-141 ## After review - **Dhruva:** if recording an applied transition's files fails, the run now stops with an error naming the org and how to reconcile it (`npm run pull -- <org>`), instead of warning and letting the commit step save that org's state without its files. Test: `promote-cmd.test.ts` breaks git after the first apply and expects the run to stop there. - **Chris (commit per transition):** replied on the PR. The blob snapshot gives the same guarantee, that what's committed is what reached the platform, without the CLI making git commits mid-run. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents 2bd9f32 + 26465d7 commit 3c1e10e

5 files changed

Lines changed: 676 additions & 9 deletions

File tree

‎.github/workflows/promotion.yml‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,20 +96,36 @@ jobs:
9696
PROMOTION_OUTCOME: ${{ steps.promotion.outcome }}
9797
run: |
9898
set -euo pipefail
99-
paths=(':(glob).vapi-state.*.json')
99+
git add -A -- ':(glob).vapi-state.*.json'
100100
if [[ "$PROMOTION_OUTCOME" == "success" ]]; then
101-
paths+=(resources)
101+
git add -A -- resources
102+
elif [[ -s tmp/promotion-applied.txt ]]; then
103+
# A later transition failed. Stage each file exactly as it stood
104+
# when its transition finished applying (promote-cmd stored that
105+
# content in git's object store), so the failed transition's
106+
# rewrites — even of the same files — are never committed.
107+
while IFS=$'\t' read -r blob path; do
108+
[[ -n "$path" ]] || continue
109+
if [[ "$blob" == "-" ]]; then
110+
git rm -q --cached --ignore-unmatch -- ":(literal)$path"
111+
else
112+
git update-index --add --cacheinfo "100644,$blob,$path"
113+
fi
114+
done < tmp/promotion-applied.txt
102115
fi
103116
104-
if [[ -z "$(git status --porcelain -- "${paths[@]}")" ]]; then
117+
if git diff --cached --quiet; then
105118
echo "Promotion produced no Git changes."
106119
exit 0
107120
fi
108121
109122
git config user.name "vapi-gitops[bot]"
110123
git config user.email "vapi-gitops[bot]@users.noreply.github.com"
111-
git add -A -- "${paths[@]}"
112124
git commit -m "chore: record promoted Vapi state [skip promotion]"
125+
# Unstaged rewrites from a failed transition would make the rebase
126+
# refuse to run, and then nothing would be pushed, state included.
127+
git reset --hard HEAD
128+
git clean -fd -- resources
113129
for attempt in 1 2 3; do
114130
git pull --rebase origin main
115131
if git push; then

‎improvements.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ you which stack PR closes the row.**
8686
| 32 | Test suite never ran in CI; 20 tests rotted after the hash store | Regression guards for #22/#23 silently stopped running | None | RESOLVED 2026-09-30 (#56) |
8787
| 33 | `npm run sim` reported every run as passed | A failing suite exited 0 — false green | None | RESOLVED 2026-10-01 |
8888
| 34 | No pre-merge simulation signal; simulations only tested what was deployed | A PR that breaks an agent merges green | #33 | RESOLVED 2026-10-01 |
89+
| 35 | A failed promotion pushed nothing, not even state | git lost track of resources already on the platform | None | RESOLVED 2026-10-01 |
8990

9091
**Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, `#31`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated.
9192

@@ -1822,6 +1823,68 @@ None needed once the fix below lands.
18221823

18231824
---
18241825

1826+
## 35. A failed promotion pushed nothing, not even state
1827+
1828+
**[RESOLVED 2026-10-01]**
1829+
1830+
**Discovered:** 2026-10-01, while planning the promotion check gate (TEST-141).
1831+
1832+
### Problem
1833+
1834+
When a multi-transition promotion failed partway, the workflow meant to
1835+
commit the UUID state and leave resource files alone. But the failing
1836+
transition had already rewritten tracked files in its target org, so the
1837+
commit step's `git pull --rebase` refused ("You have unstaged changes") and
1838+
the job pushed nothing: not the state, and not the files of the transitions
1839+
that had already reached the platform.
1840+
1841+
### Current behavior (Verified, before the fix)
1842+
1843+
- `.github/workflows/promotion.yml` "Commit reconciled files and UUID state"
1844+
staged only `.vapi-state.*.json` on a non-success outcome, then ran
1845+
`git pull --rebase origin main` with the failed transition's rewrites
1846+
still in the working tree.
1847+
- `promotionPlanApply` writes target files before `apply.ts` runs, so any
1848+
update to an existing target file left a tracked modification behind.
1849+
- Reproduced in a scratch repo: transitions a→b (applies) then b→c (fails
1850+
with an existing file in c) → `error: cannot pull with rebase`, exit 128,
1851+
origin unchanged.
1852+
1853+
### Risk
1854+
1855+
Git and the platform disagree after any partial failure: b's resources are
1856+
live but not in git, and the next promotion plans from stale files.
1857+
1858+
### Current mitigation
1859+
1860+
None needed once the fix below lands.
1861+
1862+
### Possible fix (landed)
1863+
1864+
- `src/promote-cmd.ts` truncates `tmp/promotion-applied.txt` at the start of
1865+
each `--apply` run and, after each successful `apply.ts`, records every path
1866+
`git status` reports under `resources/<target>/` — read from git rather
1867+
than the plan, because apply's own pull and push can rewrite other files —
1868+
together with its content at that moment, written to git's object store
1869+
(`git hash-object -w`; `-` for a deleted file).
1870+
- On a non-success outcome the commit step stages the state files plus
1871+
exactly those recorded blobs (`git update-index --cacheinfo`), commits, then
1872+
`git reset --hard HEAD && git clean -fd -- resources` before rebasing, so the
1873+
failed transition's rewrites can't block the push. Because it stages the
1874+
recorded content rather than the working tree, a later failed transition
1875+
that rewrote the same file (two pipelines promoting into one org in one
1876+
`--all` run) can't replace what was actually applied.
1877+
- `promotionCommandRun(args, deps)` takes an injectable child runner for
1878+
tests (`tests/promote-cmd.test.ts`). `tests/promotion-workflow-commit.test.ts`
1879+
runs the workflow's real commit step against a bare origin, including the
1880+
same-org case.
1881+
1882+
### Status
1883+
1884+
**RESOLVED 2026-10-01.**
1885+
1886+
---
1887+
18251888
## Out of scope (intentionally not improvements)
18261889

18271890
- **State file is identity-only and not git-ignored.** It's intentionally

‎src/promote-cmd.ts‎

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { existsSync, readFileSync } from "node:fs";
2-
import { resolve } from "node:path";
1+
import { execFileSync } from "node:child_process";
2+
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
3+
import { dirname, resolve } from "node:path";
34
import { fileURLToPath } from "node:url";
45
import type { OrgConnection } from "./org-connection.ts";
56
import { childRun, connectionLoad, tokensParse } from "./org-connection.ts";
@@ -31,6 +32,18 @@ const ROOT_DIR = resolve(
3132
process.env.VAPI_GITOPS_ROOT ?? fileURLToPath(new URL("..", import.meta.url)),
3233
);
3334

35+
// Files that transitions which finished applying left changed, one
36+
// `<blob>\t<path>` line each: the content as it stood when that transition
37+
// finished, stored in git's object store (`-` for a deleted file). When a
38+
// later transition fails, the promotion workflow commits exactly these blobs
39+
// (plus state) and discards everything else, so git records what reached the
40+
// platform — even if the failed transition rewrote one of the same files.
41+
export const APPLIED_PATHS_FILE = "tmp/promotion-applied.txt";
42+
43+
export interface PromotionDeps {
44+
childRun: typeof orgScriptRun;
45+
}
46+
3447
function argumentsParse(args: string[]): PromotionArguments {
3548
const parsed: PromotionArguments = { all: false, apply: false };
3649
for (let index = 0; index < args.length; index++) {
@@ -126,6 +139,67 @@ function orgScriptRun(
126139
childRun({ rootDir: ROOT_DIR, script, org, connection, args });
127140
}
128141

142+
// The paths git reports changed under resources/<org>/, including new and
143+
// deleted files. Read from git rather than the plan because apply's pull and
144+
// push can rewrite files the plan didn't name.
145+
function changedPathsRead(org: string): string[] {
146+
const output = execFileSync(
147+
"git",
148+
[
149+
"status",
150+
"--porcelain",
151+
"-z",
152+
"--untracked-files=all",
153+
"--",
154+
`resources/${org}`,
155+
],
156+
{ cwd: ROOT_DIR, encoding: "utf8" },
157+
);
158+
const entries = output.split("\0").filter(Boolean);
159+
const paths: string[] = [];
160+
for (let index = 0; index < entries.length; index++) {
161+
const entry = entries[index]!;
162+
paths.push(entry.slice(3));
163+
// A rename or copy is followed by its original path.
164+
if (entry[0] === "R" || entry[0] === "C") index++;
165+
}
166+
return paths;
167+
}
168+
169+
// The content a path has right now, written to git's object store so it
170+
// survives later rewrites; "-" when the path no longer exists.
171+
function blobWrite(path: string): string {
172+
if (!existsSync(resolve(ROOT_DIR, path))) return "-";
173+
return execFileSync("git", ["hash-object", "-w", "--", path], {
174+
cwd: ROOT_DIR,
175+
encoding: "utf8",
176+
}).trim();
177+
}
178+
179+
function appliedPathsRecord(org: string): void {
180+
const file = resolve(ROOT_DIR, APPLIED_PATHS_FILE);
181+
// path → blob; a later successful transition's snapshot replaces an
182+
// earlier one for the same path.
183+
const recorded = new Map<string, string>();
184+
if (existsSync(file))
185+
for (const line of readFileSync(file, "utf8").split("\n")) {
186+
const tab = line.indexOf("\t");
187+
if (tab > 0) recorded.set(line.slice(tab + 1), line.slice(0, tab));
188+
}
189+
try {
190+
for (const path of changedPathsRead(org))
191+
recorded.set(path, blobWrite(path));
192+
} catch (error) {
193+
// The org already has these changes, so carrying on would let the commit
194+
// step save its state without its files: the drift this record prevents.
195+
throw new Error(
196+
`Applied to ${org}, but couldn't record its changed files (${error instanceof Error ? error.message : String(error)}). Stopping: the state committed for ${org} won't have its files, so reconcile resources/${org}/ by hand with npm run pull -- ${org}.`,
197+
);
198+
}
199+
const lines = [...recorded].map(([path, blob]) => `${blob}\t${path}`);
200+
writeFileSync(file, lines.length > 0 ? `${lines.join("\n")}\n` : "");
201+
}
202+
129203
function stateLoad(org: string) {
130204
const path = resolve(ROOT_DIR, `.vapi-state.${org}.json`);
131205
if (!existsSync(path))
@@ -141,15 +215,17 @@ async function transitionRun(
141215
apply: boolean,
142216
tokens: Map<string, string>,
143217
allowEmptySourceDeletion: boolean,
218+
deps: PromotionDeps,
144219
): Promise<boolean> {
220+
const run = deps.childRun;
145221
if (apply) {
146-
orgScriptRun(
222+
run(
147223
"src/pull.ts",
148224
transition.source,
149225
orgConnection(config, transition.source, tokens),
150226
["--bootstrap", "--bindings-only"],
151227
);
152-
orgScriptRun(
228+
run(
153229
"src/pull.ts",
154230
transition.target,
155231
orgConnection(config, transition.target, tokens),
@@ -177,17 +253,19 @@ async function transitionRun(
177253
const changedPaths = plan.changes.map(
178254
(change) => `resources/${transition.target}/${change.path}`,
179255
);
180-
orgScriptRun(
256+
run(
181257
"src/apply.ts",
182258
transition.target,
183259
orgConnection(config, transition.target, tokens),
184260
["--force", "--allow-new-files", "--resolve=ours", ...changedPaths],
185261
);
262+
appliedPathsRecord(transition.target);
186263
return plan.changes.some((change) => change.kind === "delete");
187264
}
188265

189266
export async function promotionCommandRun(
190267
args = process.argv.slice(2),
268+
deps: PromotionDeps = { childRun: orgScriptRun },
191269
): Promise<void> {
192270
const parsed = argumentsParse(args);
193271
const configPath = resolve(ROOT_DIR, "promotion.yml");
@@ -198,6 +276,11 @@ export async function promotionCommandRun(
198276
? tokensParse(TOKENS_ENV)
199277
: new Map<string, string>();
200278
delete process.env[TOKENS_ENV];
279+
if (parsed.apply) {
280+
const file = resolve(ROOT_DIR, APPLIED_PATHS_FILE);
281+
mkdirSync(dirname(file), { recursive: true });
282+
writeFileSync(file, "");
283+
}
201284
// Applying a deletion removes the intermediate org's state entry. Carry the
202285
// reviewed authorization forward so the same deletion can reach later orgs.
203286
const deletionAuthorizedSources = new Set<string>();
@@ -209,6 +292,7 @@ export async function promotionCommandRun(
209292
parsed.apply,
210293
tokens,
211294
deletionAuthorizedSources.has(sourceKey),
295+
deps,
212296
);
213297
if (deleted)
214298
deletionAuthorizedSources.add(

0 commit comments

Comments
 (0)