diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 248a2ba97..50ce67462 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 27079972c..ec04e5468 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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-` 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. diff --git a/scripts/ci-concurrency.test.ts b/scripts/ci-concurrency.test.ts new file mode 100644 index 000000000..ebfe3226b --- /dev/null +++ b/scripts/ci-concurrency.test.ts @@ -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)[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 = {}): { 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"); + }); +}); diff --git a/scripts/release.ts b/scripts/release.ts index 12ecfff5c..273b7e914 100755 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -368,8 +368,12 @@ async function cmdRelease(version: string): Promise { 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: \""); + console.log(` git commit -m "chore: bump version to ${version}" -m ""`); 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}"`,