Skip to content

Commit 308b8d5

Browse files
fix(promotion): commit the files of transitions that applied when a later one fails
When a promotion failed partway, the workflow's commit step staged only the UUID state, but the failing transition had already rewritten tracked files in its target org, so `git pull --rebase` refused and nothing was pushed — not the state, and not the files of transitions that had already reached the platform. - promote-cmd truncates tmp/promotion-applied.txt at the start of each --apply run and, after each successful apply.ts, appends the paths git reports changed under resources/<target>/ (from git, not the plan: apply's own pull and push can rewrite other files). - On a non-success outcome, the commit step adds the state files plus exactly those paths, commits, then resets and cleans resources/ so the failed transition's rewrites can't block the rebase. - promotionCommandRun(args, deps) takes an injectable child runner, used by the new tests/promote-cmd.test.ts. Refs TEST-141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 69c7e83 commit 308b8d5

4 files changed

Lines changed: 324 additions & 3 deletions

File tree

‎.github/workflows/promotion.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,13 @@ jobs:
9999
paths=(':(glob).vapi-state.*.json')
100100
if [[ "$PROMOTION_OUTCOME" == "success" ]]; then
101101
paths+=(resources)
102+
elif [[ -s tmp/promotion-applied.txt ]]; then
103+
# A later transition failed: keep the files of the transitions
104+
# that finished applying, so git matches what reached the
105+
# platform. The failed transition's rewrites are discarded below.
106+
while IFS= read -r applied; do
107+
[[ -n "$applied" ]] && paths+=(":(literal)$applied")
108+
done < tmp/promotion-applied.txt
102109
fi
103110
104111
if [[ -z "$(git status --porcelain -- "${paths[@]}")" ]]; then
@@ -110,6 +117,10 @@ jobs:
110117
git config user.email "vapi-gitops[bot]@users.noreply.github.com"
111118
git add -A -- "${paths[@]}"
112119
git commit -m "chore: record promoted Vapi state [skip promotion]"
120+
# Unstaged rewrites from a failed transition would make the rebase
121+
# refuse to run, and then nothing would be pushed, state included.
122+
git reset --hard HEAD
123+
git clean -fd -- resources
113124
for attempt in 1 2 3; do
114125
git pull --rebase origin main
115126
if git push; then

‎improvements.md‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ you which stack PR closes the row.**
8484
| 30 | Tool-linking pass could PATCH a raw assistant slug | Mid-push 400 naming the wrong resource | None | RESOLVED 2026-08-03 (#51) |
8585
| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open |
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) |
87+
| 35 | A failed promotion pushed nothing, not even state | git lost track of resources already on the platform | None | RESOLVED 2026-10-01 |
8788

8889
**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.
8990

@@ -1711,6 +1712,61 @@ errors today; widening `include` is a separate change.
17111712

17121713
---
17131714

1715+
## 35. A failed promotion pushed nothing, not even state
1716+
1717+
**[RESOLVED 2026-10-01]**
1718+
1719+
**Discovered:** 2026-10-01, while planning the promotion check gate (TEST-141).
1720+
1721+
### Problem
1722+
1723+
When a multi-transition promotion failed partway, the workflow meant to
1724+
commit the UUID state and leave resource files alone. But the failing
1725+
transition had already rewritten tracked files in its target org, so the
1726+
commit step's `git pull --rebase` refused ("You have unstaged changes") and
1727+
the job pushed nothing: not the state, and not the files of the transitions
1728+
that had already reached the platform.
1729+
1730+
### Current behavior (Verified, before the fix)
1731+
1732+
- `.github/workflows/promotion.yml` "Commit reconciled files and UUID state"
1733+
staged only `.vapi-state.*.json` on a non-success outcome, then ran
1734+
`git pull --rebase origin main` with the failed transition's rewrites
1735+
still in the working tree.
1736+
- `promotionPlanApply` writes target files before `apply.ts` runs, so any
1737+
update to an existing target file left a tracked modification behind.
1738+
- Reproduced in a scratch repo: transitions a→b (applies) then b→c (fails
1739+
with an existing file in c) → `error: cannot pull with rebase`, exit 128,
1740+
origin unchanged.
1741+
1742+
### Risk
1743+
1744+
Git and the platform disagree after any partial failure: b's resources are
1745+
live but not in git, and the next promotion plans from stale files.
1746+
1747+
### Current mitigation
1748+
1749+
None needed once the fix below lands.
1750+
1751+
### Possible fix (landed)
1752+
1753+
- `src/promote-cmd.ts` truncates `tmp/promotion-applied.txt` at the start of
1754+
each `--apply` run and, after each successful `apply.ts`, appends the paths
1755+
`git status` reports under `resources/<target>/` — read from git rather
1756+
than the plan, because apply's own pull and push can rewrite other files.
1757+
- On a non-success outcome the commit step adds the state files plus exactly
1758+
those paths, commits, then `git reset --hard HEAD && git clean -fd --
1759+
resources` before rebasing, so the failed transition's rewrites can't block
1760+
the push.
1761+
- `promotionCommandRun(args, deps)` takes an injectable child runner for
1762+
tests (`tests/promote-cmd.test.ts`).
1763+
1764+
### Status
1765+
1766+
**RESOLVED 2026-10-01.**
1767+
1768+
---
1769+
17141770
## Out of scope (intentionally not improvements)
17151771

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

‎src/promote-cmd.ts‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { resolveApiKey } from "./api-key.ts";
2-
import { spawnSync } from "node:child_process";
3-
import { existsSync, readFileSync } from "node:fs";
4-
import { resolve } from "node:path";
2+
import { execFileSync, spawnSync } from "node:child_process";
3+
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
4+
import { dirname, resolve } from "node:path";
55
import { fileURLToPath } from "node:url";
66
import type { PromotionConfig, PromotionPipeline } from "./promotion.ts";
77
import {
@@ -36,6 +36,16 @@ const ROOT_DIR = resolve(
3636
process.env.VAPI_GITOPS_ROOT ?? fileURLToPath(new URL("..", import.meta.url)),
3737
);
3838

39+
// Files that transitions which finished applying left changed, one path per
40+
// line. When a later transition fails, the promotion workflow commits exactly
41+
// these (plus state) and discards the failed transition's rewrites, so what
42+
// reached the platform is still recorded in git.
43+
export const APPLIED_PATHS_FILE = "tmp/promotion-applied.txt";
44+
45+
export interface PromotionDeps {
46+
childRun: typeof childRun;
47+
}
48+
3949
function argumentsParse(args: string[]): PromotionArguments {
4050
const parsed: PromotionArguments = { all: false, apply: false };
4151
for (let index = 0; index < args.length; index++) {
@@ -189,6 +199,51 @@ function childRun(
189199
if (result.status !== 0) throw new Error(`${script} failed for ${org}`);
190200
}
191201

202+
// The paths git reports changed under resources/<org>/, including new and
203+
// deleted files. Read from git rather than the plan because apply's pull and
204+
// push can rewrite files the plan didn't name.
205+
function changedPathsRead(org: string): string[] {
206+
const output = execFileSync(
207+
"git",
208+
[
209+
"status",
210+
"--porcelain",
211+
"-z",
212+
"--untracked-files=all",
213+
"--",
214+
`resources/${org}`,
215+
],
216+
{ cwd: ROOT_DIR, encoding: "utf8" },
217+
);
218+
const entries = output.split("\0").filter(Boolean);
219+
const paths: string[] = [];
220+
for (let index = 0; index < entries.length; index++) {
221+
const entry = entries[index]!;
222+
paths.push(entry.slice(3));
223+
// A rename or copy is followed by its original path.
224+
if (entry[0] === "R" || entry[0] === "C") index++;
225+
}
226+
return paths;
227+
}
228+
229+
function appliedPathsRecord(org: string): void {
230+
const file = resolve(ROOT_DIR, APPLIED_PATHS_FILE);
231+
const recorded = existsSync(file)
232+
? readFileSync(file, "utf8").split("\n").filter(Boolean)
233+
: [];
234+
let changed: string[];
235+
try {
236+
changed = changedPathsRead(org);
237+
} catch (error) {
238+
console.warn(
239+
`⚠️ Could not record applied paths for ${org}: ${error instanceof Error ? error.message : String(error)}`,
240+
);
241+
return;
242+
}
243+
const paths = [...new Set([...recorded, ...changed])];
244+
writeFileSync(file, paths.length > 0 ? `${paths.join("\n")}\n` : "");
245+
}
246+
192247
function stateLoad(org: string) {
193248
const path = resolve(ROOT_DIR, `.vapi-state.${org}.json`);
194249
if (!existsSync(path))
@@ -204,7 +259,9 @@ async function transitionRun(
204259
apply: boolean,
205260
tokens: Map<string, string>,
206261
allowEmptySourceDeletion: boolean,
262+
deps: PromotionDeps,
207263
): Promise<boolean> {
264+
const { childRun } = deps;
208265
if (apply) {
209266
childRun(
210267
"src/pull.ts",
@@ -246,11 +303,13 @@ async function transitionRun(
246303
connectionLoad(config, transition.target, tokens),
247304
["--force", "--allow-new-files", "--resolve=ours", ...changedPaths],
248305
);
306+
appliedPathsRecord(transition.target);
249307
return plan.changes.some((change) => change.kind === "delete");
250308
}
251309

252310
export async function promotionCommandRun(
253311
args = process.argv.slice(2),
312+
deps: PromotionDeps = { childRun },
254313
): Promise<void> {
255314
const parsed = argumentsParse(args);
256315
const configPath = resolve(ROOT_DIR, "promotion.yml");
@@ -259,6 +318,11 @@ export async function promotionCommandRun(
259318
const config = promotionConfigParse(readFileSync(configPath, "utf8"));
260319
const tokens = parsed.apply ? tokensParse() : new Map<string, string>();
261320
delete process.env.VAPI_PROMOTION_TOKENS;
321+
if (parsed.apply) {
322+
const file = resolve(ROOT_DIR, APPLIED_PATHS_FILE);
323+
mkdirSync(dirname(file), { recursive: true });
324+
writeFileSync(file, "");
325+
}
262326
// Applying a deletion removes the intermediate org's state entry. Carry the
263327
// reviewed authorization forward so the same deletion can reach later orgs.
264328
const deletionAuthorizedSources = new Set<string>();
@@ -270,6 +334,7 @@ export async function promotionCommandRun(
270334
parsed.apply,
271335
tokens,
272336
deletionAuthorizedSources.has(sourceKey),
337+
deps,
273338
);
274339
if (deleted)
275340
deletionAuthorizedSources.add(

0 commit comments

Comments
 (0)