fix(ci): scope release runs to per-sha concurrency group
The workflow-wide concurrency group was `${{ github.workflow }}-${{ github.ref }}`
with `cancel-in-progress: true`, so the release-script's atomic
`refs/heads/main + v* tag` push shared the `CI-refs/heads/main` group with every
later main push. The newer run cancelled the older release run before
`release_binary` / `release_github` / `release_npm` could execute, and no
future run carried the tag at HEAD, so the tag stayed published-as-a-ref but
unreleased on GitHub and npm (v15.12.6 in the wild).
Release runs are now routed to a per-sha group with `cancel-in-progress: false`
when either:
* the push subject starts with `chore: bump version to ` (the release-script
commit convention from scripts/release.ts), or
* `github.ref` is a `v*` tag (workflow_dispatch recovery from a tag ref).
Other events keep the cheap branch-wide cancel-in-progress for PR/main churn.
release.ts's retry hint now uses the same release commit subject so manual
retries also land in the per-sha group.
Added scripts/ci-concurrency.test.ts: a regression test with a minimal GHA
expression evaluator that asserts the resolved group / cancel-in-progress for
auto-release pushes, retry pushes, tag-ref dispatches, plain main pushes, PRs,
distinct release shas, and a benign `revert: chore: bump version to ...`
follow-up.
Fixes #2564
This commit is contained in:
@@ -12,9 +12,17 @@ on:
|
||||
type: boolean
|
||||
default: false
|
||||
|
||||
# Release runs publish a `v*` tag pushed atomically with main HEAD; sharing
|
||||
# the cheap branch-wide `CI-refs/heads/main` group meant a later main push
|
||||
# silently cancelled the in-flight release and left the tag without a GitHub
|
||||
# Release or npm publish (#2564). Detect release runs at workflow-scheduling
|
||||
# time via the release-script commit subject (`chore: bump version to vX.Y.Z`,
|
||||
# see scripts/release.ts) and via `v*` tag-ref dispatches, then scope them to
|
||||
# a per-sha group with no cancellation. Every other event keeps the
|
||||
# branch-wide cancel-in-progress for PR/main churn.
|
||||
concurrency:
|
||||
group: ${{ github.workflow }}-${{ github.ref }}
|
||||
cancel-in-progress: true
|
||||
group: "${{ github.workflow }}-${{ (startsWith(github.event.head_commit.message, 'chore: bump version to ') || startsWith(github.ref, 'refs/tags/v')) && format('release-{0}', github.sha) || github.ref }}"
|
||||
cancel-in-progress: "${{ !(startsWith(github.event.head_commit.message, 'chore: bump version to ') || startsWith(github.ref, 'refs/tags/v')) }}"
|
||||
|
||||
env:
|
||||
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed release runs being silently cancelled by a later `main` push, which left tagged versions (`v15.12.6` in the wild) without a GitHub Release or npm publish. The CI workflow's `concurrency` group was `${{ github.workflow }}-${{ github.ref }}`, and since the release-script commit + `v*` tag are pushed atomically to `refs/heads/main`, the release run shared the `CI-refs/heads/main` group with every subsequent push; `cancel-in-progress: true` then killed it before `release_binary` / `release_github` / `release_npm` could run, and no future run carried the release tag at HEAD. The group now resolves to a per-sha `release-<sha>` slot with `cancel-in-progress: false` whenever the push subject matches `chore: bump version to ` (the release-script convention) or `github.ref` is a `v*` tag (`workflow_dispatch` recovery), so release runs are isolated from PR/main churn ([#2564](https://github.com/can1357/oh-my-pi/issues/2564)).
|
||||
- Fixed the tool-result renderer re-shaping on every `invalidate()` (spinner tick, stream chunk, resize, keystroke), which made large grep/find/read results block the main thread for seconds and made typing sluggish. `ToolExecutionComponent.#updateDisplay()` now memoizes on a dirty key (result version, expand state, partial flag, spinner frame, image visibility, theme epoch, background-task freeze state, the resolved terminal image protocol, and a display-input version that covers streamed call args, the async edit-diff preview, and Kitty image conversions) and a `#displayBuilt` guard that also fast-paths the `#contentText` fallback, so the O(result-size) shaping runs once per change instead of every frame without freezing streamed args, previews, converted images, a backgrounded task settling to its static form, or images that arrive before the async image-protocol probe resolves. Image-bearing results also re-shape on terminal resize (keyed on the resolved image dimensions only when images are present) so inline images rescale, while image-free results never re-shape on resize ([#2484](https://github.com/can1357/oh-my-pi/issues/2484))
|
||||
- Fixed `setTheme()` not bumping the theme epoch on its invalid-theme fallback path: a failed theme load swaps the active theme to the dark fallback, so memoized renderers (the tool-result renderer above) must re-shape — previously they kept the failed theme's stale colors until some other state changed ([#2484](https://github.com/can1357/oh-my-pi/issues/2484))
|
||||
- Fixed tool-call spinners animating out of phase across parallel tool calls — each live tool block advanced its glyph from its own per-instance start time, so concurrent spinners showed different frames. Glyphs now derive from a single shared monotonic clock (`sharedSpinnerFrame`), keeping every live block in lockstep.
|
||||
|
||||
@@ -0,0 +1,301 @@
|
||||
// Regression test for #2564: the CI workflow's `concurrency` block must route
|
||||
// release runs to a per-sha group with no cancellation, so a later main push
|
||||
// can't kill the in-flight release and leave the tag unpublished. The block is
|
||||
// evaluated by GitHub at workflow-scheduling time (before any job can produce
|
||||
// the signal), so this test re-implements the small subset of GitHub
|
||||
// expression semantics the block uses and asserts the resolved group / cancel
|
||||
// flag for every event shape we care about.
|
||||
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
|
||||
const WORKFLOW_PATH = path.resolve(import.meta.dir, "..", ".github", "workflows", "ci.yml");
|
||||
|
||||
type Value = string | boolean | null;
|
||||
|
||||
// `github` context fed into the evaluator. Nested objects are walked the same
|
||||
// way as in real GHA expressions; missing keys resolve to `null`.
|
||||
interface GhaCtx {
|
||||
workflow: string;
|
||||
ref: string;
|
||||
sha: string;
|
||||
event_name: string;
|
||||
event: {
|
||||
head_commit?: { message?: string };
|
||||
};
|
||||
}
|
||||
|
||||
// Single-purpose, hand-rolled evaluator for the operators / functions the
|
||||
// workflow's `concurrency` block uses: `startsWith`, `format`, `!`, `&&`, `||`,
|
||||
// parens, single-quoted strings, dotted property access. Matches GitHub's
|
||||
// short-circuit semantics: `&&`/`||` return the underlying value (not a coerced
|
||||
// bool), missing identifiers resolve to `null`, and `startsWith(null, …)` is
|
||||
// false because the searchString coerces to `""`.
|
||||
class GhaEval {
|
||||
#pos = 0;
|
||||
|
||||
private constructor(
|
||||
private readonly src: string,
|
||||
private readonly ctx: { github: GhaCtx },
|
||||
) {}
|
||||
|
||||
static run(expr: string, ctx: { github: GhaCtx }): Value {
|
||||
const ev = new GhaEval(expr.trim(), ctx);
|
||||
const value = ev.#or();
|
||||
ev.#skipWs();
|
||||
if (ev.#pos !== ev.src.length) {
|
||||
throw new Error(`trailing input at offset ${ev.#pos}: ${ev.src.slice(ev.#pos)}`);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
// Substitute every `${{ … }}` placeholder in a workflow template string.
|
||||
static template(template: string, ctx: { github: GhaCtx }): string {
|
||||
let out = "";
|
||||
let i = 0;
|
||||
while (i < template.length) {
|
||||
const start = template.indexOf("${{", i);
|
||||
if (start === -1) {
|
||||
out += template.slice(i);
|
||||
break;
|
||||
}
|
||||
out += template.slice(i, start);
|
||||
const end = template.indexOf("}}", start);
|
||||
if (end === -1) throw new Error("unterminated ${{ expression");
|
||||
const v = GhaEval.run(template.slice(start + 3, end), ctx);
|
||||
out += v === null ? "" : String(v);
|
||||
i = end + 2;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
#or(): Value {
|
||||
let left = this.#and();
|
||||
while (this.#consume("||")) {
|
||||
const right = this.#and();
|
||||
// Truthy left wins; only null/false/""/0 fall through.
|
||||
if (left !== null && left !== false && left !== "" && left !== 0) continue;
|
||||
left = right;
|
||||
}
|
||||
return left;
|
||||
}
|
||||
|
||||
#and(): Value {
|
||||
let left = this.#unary();
|
||||
while (this.#consume("&&")) {
|
||||
const right = this.#unary();
|
||||
// Falsy left short-circuits and is returned verbatim.
|
||||
if (left === null || left === false || left === "" || left === 0) continue;
|
||||
left = right;
|
||||
}
|
||||
return left;
|
||||
}
|
||||
|
||||
#unary(): Value {
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] === "!") {
|
||||
this.#pos++;
|
||||
const v = this.#unary();
|
||||
return v === null || v === false || v === "" || v === 0;
|
||||
}
|
||||
return this.#primary();
|
||||
}
|
||||
|
||||
#primary(): Value {
|
||||
this.#skipWs();
|
||||
const ch = this.src[this.#pos];
|
||||
if (ch === "(") {
|
||||
this.#pos++;
|
||||
const v = this.#or();
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] !== ")") throw new Error("expected `)`");
|
||||
this.#pos++;
|
||||
return v;
|
||||
}
|
||||
if (ch === "'") return this.#string();
|
||||
// Identifier or function call.
|
||||
const ident = this.#identifier();
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] === "(") return this.#call(ident);
|
||||
return this.#readPath(ident);
|
||||
}
|
||||
|
||||
#string(): string {
|
||||
// GHA single-quoted: `''` is an escaped quote.
|
||||
this.#pos++; // opening quote
|
||||
let out = "";
|
||||
while (this.#pos < this.src.length) {
|
||||
const c = this.src[this.#pos];
|
||||
if (c === "'") {
|
||||
if (this.src[this.#pos + 1] === "'") {
|
||||
out += "'";
|
||||
this.#pos += 2;
|
||||
continue;
|
||||
}
|
||||
this.#pos++;
|
||||
return out;
|
||||
}
|
||||
out += c;
|
||||
this.#pos++;
|
||||
}
|
||||
throw new Error("unterminated string literal");
|
||||
}
|
||||
|
||||
#identifier(): string {
|
||||
const start = this.#pos;
|
||||
while (this.#pos < this.src.length && /[A-Za-z0-9_.]/.test(this.src[this.#pos]!)) {
|
||||
this.#pos++;
|
||||
}
|
||||
if (start === this.#pos) throw new Error(`expected identifier at ${this.#pos}`);
|
||||
return this.src.slice(start, this.#pos);
|
||||
}
|
||||
|
||||
#call(name: string): Value {
|
||||
this.#pos++; // opening paren
|
||||
const args: Value[] = [];
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] !== ")") {
|
||||
for (;;) {
|
||||
args.push(this.#or());
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] === ",") {
|
||||
this.#pos++;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
this.#skipWs();
|
||||
if (this.src[this.#pos] !== ")") throw new Error("expected `)` closing call");
|
||||
this.#pos++;
|
||||
switch (name) {
|
||||
case "startsWith": {
|
||||
const hay = args[0] === null || args[0] === false ? "" : String(args[0]);
|
||||
const needle = args[1] === null || args[1] === false ? "" : String(args[1]);
|
||||
return hay.startsWith(needle);
|
||||
}
|
||||
case "format": {
|
||||
const tmpl = args[0] === null ? "" : String(args[0]);
|
||||
return tmpl.replace(/\{(\d+)\}/g, (_, idx) => {
|
||||
const v = args[Number(idx) + 1];
|
||||
return v === null || v === false ? "" : String(v);
|
||||
});
|
||||
}
|
||||
default:
|
||||
throw new Error(`unsupported function: ${name}`);
|
||||
}
|
||||
}
|
||||
|
||||
#readPath(dotted: string): Value {
|
||||
let cur: unknown = this.ctx;
|
||||
for (const seg of dotted.split(".")) {
|
||||
if (cur == null || typeof cur !== "object") return null;
|
||||
cur = (cur as Record<string, unknown>)[seg];
|
||||
}
|
||||
if (cur === undefined || cur === null) return null;
|
||||
if (typeof cur === "object") return null;
|
||||
return cur as Value;
|
||||
}
|
||||
|
||||
#consume(op: string): boolean {
|
||||
this.#skipWs();
|
||||
if (this.src.startsWith(op, this.#pos)) {
|
||||
this.#pos += op.length;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
#skipWs(): void {
|
||||
while (this.#pos < this.src.length && /\s/.test(this.src[this.#pos]!)) this.#pos++;
|
||||
}
|
||||
}
|
||||
|
||||
const workflowYaml = await Bun.file(WORKFLOW_PATH).text();
|
||||
// The block sits at indent 0 immediately under the top-level `concurrency:`
|
||||
// key and uses single-line values, so a flat-line extract is unambiguous.
|
||||
// Values are double-quoted in YAML (the GitHub expression contains `: ` from
|
||||
// the `'chore: bump version to '` literal which would otherwise trip plain
|
||||
// scalar parsing), so we unwrap the wrapping `"…"` here.
|
||||
const concurrencySection = workflowYaml.slice(workflowYaml.indexOf("\nconcurrency:") + 1);
|
||||
const groupRaw = /^\s*group:\s*(\S.*?)\s*$/m.exec(concurrencySection)?.[1];
|
||||
const cancelRaw = /^\s*cancel-in-progress:\s*(\S.*?)\s*$/m.exec(concurrencySection)?.[1];
|
||||
const groupTemplate =
|
||||
groupRaw && groupRaw.startsWith('"') && groupRaw.endsWith('"') ? groupRaw.slice(1, -1) : groupRaw;
|
||||
const cancelTemplate =
|
||||
cancelRaw && cancelRaw.startsWith('"') && cancelRaw.endsWith('"')
|
||||
? cancelRaw.slice(1, -1)
|
||||
: cancelRaw;
|
||||
if (!groupTemplate || !cancelTemplate) {
|
||||
throw new Error("could not locate concurrency.group / cancel-in-progress in ci.yml");
|
||||
}
|
||||
|
||||
const RELEASE_SUBJECT = "chore: bump version to 15.12.6";
|
||||
|
||||
const baseCtx = (overrides: Partial<GhaCtx> = {}): { github: GhaCtx } => ({
|
||||
github: {
|
||||
workflow: "CI",
|
||||
ref: "refs/heads/main",
|
||||
sha: "deadbeefcafebabe",
|
||||
event_name: "push",
|
||||
event: {},
|
||||
...overrides,
|
||||
},
|
||||
});
|
||||
|
||||
describe("ci.yml concurrency", () => {
|
||||
it("auto release push: per-sha group, no cancellation (#2564 root cause)", () => {
|
||||
const ctx = baseCtx({ event: { head_commit: { message: `${RELEASE_SUBJECT}\n\nbody` } } });
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-release-deadbeefcafebabe");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("false");
|
||||
});
|
||||
|
||||
it("retry release push (release subject preserved): same per-sha behavior", () => {
|
||||
const ctx = baseCtx({
|
||||
sha: "feedfacedeadbeef",
|
||||
event: { head_commit: { message: `${RELEASE_SUBJECT}\n\nretry: fix sccache 100 exit` } },
|
||||
});
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-release-feedfacedeadbeef");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("false");
|
||||
});
|
||||
|
||||
it("workflow_dispatch from a `v*` tag ref: per-sha group, no cancellation", () => {
|
||||
const ctx = baseCtx({
|
||||
ref: "refs/tags/v15.12.6",
|
||||
event_name: "workflow_dispatch",
|
||||
sha: "abc123",
|
||||
event: {},
|
||||
});
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-release-abc123");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("false");
|
||||
});
|
||||
|
||||
it("regular main push: branch-wide group, cancel-in-progress enabled", () => {
|
||||
const ctx = baseCtx({ event: { head_commit: { message: "fix(ux): theme tweak" } } });
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-refs/heads/main");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("true");
|
||||
});
|
||||
|
||||
it("pull_request (no head_commit): branch-wide group, cancel enabled", () => {
|
||||
const ctx = baseCtx({ ref: "refs/pull/42/merge", event_name: "pull_request", event: {} });
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-refs/pull/42/merge");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("true");
|
||||
});
|
||||
|
||||
it("two release commits with distinct shas land in disjoint groups", () => {
|
||||
const a = baseCtx({ sha: "aaaa1111", event: { head_commit: { message: RELEASE_SUBJECT } } });
|
||||
const b = baseCtx({ sha: "bbbb2222", event: { head_commit: { message: RELEASE_SUBJECT } } });
|
||||
expect(GhaEval.template(groupTemplate, a)).not.toBe(GhaEval.template(groupTemplate, b));
|
||||
});
|
||||
|
||||
it("benign commit subject that merely contains the release prefix is not a release", () => {
|
||||
// startsWith is anchored, so `revert: chore: bump version to 15.12.6` (a
|
||||
// follow-up commit) keeps the cancel-on-newer-push behavior — it has no
|
||||
// tag to publish.
|
||||
const ctx = baseCtx({
|
||||
event: { head_commit: { message: `revert: ${RELEASE_SUBJECT}` } },
|
||||
});
|
||||
expect(GhaEval.template(groupTemplate, ctx)).toBe("CI-refs/heads/main");
|
||||
expect(GhaEval.template(cancelTemplate, ctx)).toBe("true");
|
||||
});
|
||||
});
|
||||
+5
-1
@@ -368,8 +368,12 @@ async function cmdRelease(version: string): Promise<void> {
|
||||
if (success) {
|
||||
console.log(`=== Released v${version} ===`);
|
||||
} else {
|
||||
// CI's `concurrency` block (.github/workflows/ci.yml) recognizes a
|
||||
// release run by its `chore: bump version to vX.Y.Z` subject (#2564),
|
||||
// so retries that keep that subject also get the per-sha, never-cancel
|
||||
// group. Reword the body, not the subject.
|
||||
console.log("\nTo retry after fixing (repeat until CI passes):");
|
||||
console.log(" git commit -m \"fix: <brief description>\"");
|
||||
console.log(` git commit -m "chore: bump version to ${version}" -m "<what was fixed>"`);
|
||||
console.log(` git tag -f v${version}`);
|
||||
console.log(
|
||||
` git push --atomic origin refs/heads/main:refs/heads/main "+$(git rev-parse HEAD):refs/tags/v${version}"`,
|
||||
|
||||
Reference in New Issue
Block a user