From 18b7fb387fa62d4f2beeeb05fff02e68674d1015 Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Mon, 10 Aug 2026 13:23:46 -0700 Subject: [PATCH 1/6] feat(ci): hourly personal-staging refresh with extras manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nightly ring rebuilds `staging` once a day, so upstream main and open PR pushes sit unstaged for up to 24h; and PRs closed without merge silently fall out of the composition even when the fork still needs them. Add an hourly workflow that reuses the composer in a new `--staging-only` mode: same composition, but no nightly pin branch, no nightly/dev tags — only `refs/heads/staging` is pushed, under `--force-with-lease`. Since the composition is byte-reproducible, an unchanged hour skips the push and reports "unchanged"; when it does push, the summary names which of upstream HEAD, the open PR set, or the extras moved. `extras.txt` pins PR numbers that must stay baked into staging after their PR closes (GitHub keeps `refs/pull/N/head` fetchable). Extras join the same merge stream as open PRs — union deduped by number with the open entry winning, ascending — so both the hourly and the nightly pick them up with no workflow change. An unfetchable extra is a loud skip carrying its own reason, distinct from a conflict skip, and never fails the run. The hourly `sync-main` soft-fails on conflict (abort, summary, `::warning::`, exit 0): compose stacks from upstream HEAD and never reads fork main, so hourly noise for a conflict the nightly already hard-fails on helps nobody. It also runs in its own concurrency group so a coalescing hourly run can never cancel a running nightly. Also fix the README's stale "05:37 UTC" nightly cron claim (actual: `0 10 * * *`). Signed-off-by: Bryan Li Co-authored-by: omnigent --- .github/scripts/personal-staging/README.md | 50 +++- .github/scripts/personal-staging/extras.txt | 9 + .github/scripts/personal-staging/stage.py | 221 ++++++++++++++++-- .../scripts/personal-staging/test_stage.py | 167 ++++++++++++- .github/workflows/personal-staging-hourly.yml | 131 +++++++++++ 5 files changed, 548 insertions(+), 30 deletions(-) create mode 100644 .github/scripts/personal-staging/extras.txt create mode 100644 .github/workflows/personal-staging-hourly.yml diff --git a/.github/scripts/personal-staging/README.md b/.github/scripts/personal-staging/README.md index 2830981cb9..e48531b516 100644 --- a/.github/scripts/personal-staging/README.md +++ b/.github/scripts/personal-staging/README.md @@ -1,11 +1,13 @@ # Personal staging nightly (fork-only) -`.github/workflows/personal-staging.yml` runs nightly (05:37 UTC, plus -`workflow_dispatch`) on the `btli/omnigent` fork only. It: +`.github/workflows/personal-staging.yml` runs nightly (10:00 UTC — cron +`0 10 * * *` — plus `workflow_dispatch`) on the `btli/omnigent` fork only. +It: 1. Composes fork branch `staging` = upstream `omnigent-ai/omnigent` main + - every open btli PR, merged sequentially ascending by PR number - (`stage.py`). A conflicting PR is skipped — its conflict paths land in + every open btli PR plus the [`extras.txt`](#extras-manifest-extrastxt) + pins, merged sequentially ascending by PR number (`stage.py`). A + conflicting PR is skipped — its conflict paths land in `merge-report.json` — and the run continues. All PRs conflicting is a reported outcome, not a failure. 2. Pins an immutable `nightly-YYYYMMDD` branch + tag at the staging commit @@ -39,6 +41,46 @@ homelab deploy branch driven by `just test-branch`. Nothing here flips any existing behavior. +## Hourly staging refresh (`personal-staging-hourly.yml`) + +`Personal Staging Hourly` (cron `0 * * * *`, plus `workflow_dispatch`) +keeps branch `staging` fresh between nightlies. It runs the same composer +with `--staging-only`: compose upstream main + open btli PRs + extras +exactly as the nightly does, then push ONLY `refs/heads/staging` +(`--force-with-lease`). It mints no `nightly-*` pins and no dev tags, and +builds no APK/releases/images — those stay nightly-only. + +Composition is byte-reproducible, so when the composed commit equals the +current remote `staging` sha the run is a no-op: the push is skipped and +the summary reports "unchanged". When it does push, the summary's one-line +result names which of upstream HEAD, the open PR set, or the extras +changed. + +Its `sync-main` job soft-fails on a merge conflict (abort + `::warning::` + +summary, exit 0) — unlike the nightly's, which hard-fails — because the +compose job stacks from upstream HEAD and never reads fork main. The +workflow uses its own concurrency group (`personal-staging-hourly`, +`cancel-in-progress: true`) so stale hourly runs coalesce and can never +cancel a running nightly; if an hourly push loses a `--force-with-lease` +race with the nightly, the next hour retries. + +## Extras manifest (`extras.txt`) + +`stage.py` always reads `.github/scripts/personal-staging/extras.txt` +(missing file == no extras), so extras land in BOTH the hourly and the +nightly composition. Format: one PR number per line; blank lines and `#` +comments allowed; anything else fails the run loudly. + +Extras are PR numbers that must stay baked into `staging` even though +they are no longer open (typically closed-without-merge) — GitHub keeps +`refs/pull/N/head` fetchable after close. The merge stream is the union +of open PRs and extras, deduped by PR number (the open entry wins), +sorted ascending — the same ordering rule as always. **Remove an entry +once the change lands upstream.** An extra whose ref can no longer be +fetched is skipped loudly, with reason `extra unfetchable (likely +deleted; remove from extras.txt)` in the report and step summary — +distinct from a conflict skip — but does not fail the run. + ## Stable download URL The floating prerelease keeps asset names fixed, so the newest nightly APK is diff --git a/.github/scripts/personal-staging/extras.txt b/.github/scripts/personal-staging/extras.txt new file mode 100644 index 0000000000..a045878d2e --- /dev/null +++ b/.github/scripts/personal-staging/extras.txt @@ -0,0 +1,9 @@ +# Personal-staging extras manifest: one PR number per line. +# +# PRs listed here are merged into `staging` even when no longer open +# (GitHub keeps refs/pull/N/head fetchable after close). Blank lines and +# `#` comments are ignored. Remove an entry once the change lands +# upstream; a deleted/unfetchable ref is skipped loudly in the run report. +# +# Example: +# 1234 # keep the composer hotfix until upstream merges it diff --git a/.github/scripts/personal-staging/stage.py b/.github/scripts/personal-staging/stage.py index 248eefc4d7..d561bcfc82 100644 --- a/.github/scripts/personal-staging/stage.py +++ b/.github/scripts/personal-staging/stage.py @@ -1,11 +1,14 @@ #!/usr/bin/env python3 -"""Compose the nightly personal staging ring on the fork. +"""Compose the personal staging ring on the fork. -Builds branch ``staging`` = upstream main + every open btli PR merged -sequentially (ascending PR number; conflicting PRs are skipped and reported), -then pins an immutable ``nightly-YYYYMMDD`` branch + tag and a PEP 440 -``vX.Y.Z.devYYYYMMDD`` tag at the same commit. The only refs ever pushed are -``staging``, the ``nightly-*`` pin, and the dev tag. +Builds branch ``staging`` = upstream main + every open btli PR plus the +``extras.txt`` pins, merged sequentially (ascending PR number; conflicting +PRs are skipped and reported). The nightly mode then pins an immutable +``nightly-YYYYMMDD`` branch + tag and a PEP 440 ``vX.Y.Z.devYYYYMMDD`` tag +at the same commit; ``--staging-only`` (the hourly mode) pushes only +``staging`` and skips the push entirely when the composition is unchanged. +The only refs ever pushed are ``staging``, the ``nightly-*`` pin, and the +dev tag. Stdlib + git subprocess only; the gh CLI is used solely to list PRs and can be bypassed with --prs-json (how the tests stay offline). @@ -34,6 +37,11 @@ UPSTREAM_REPO = "omnigent-ai/omnigent" PR_AUTHOR = "btli" PR_LIST_LIMIT = 100 +EXTRAS_FILE = Path(__file__).resolve().parent / "extras.txt" +EXTRA_UNFETCHABLE = "extra unfetchable (likely deleted; remove from extras.txt)" +# Subject line minted below for every staging merge commit; also decoded to +# diff two compositions (what changed between the old and new staging). +STAGING_MERGE_RE = re.compile(r"staging: merge PR #(\d+) \(.+ @ ([0-9a-f]{12})\)") # Fixed identity, no gpg signature: the merge commit must be byte-reproducible # so an unchanged same-day rerun lands on the identical sha (no-op detection). COMMIT_IDENT = [ @@ -113,6 +121,34 @@ def check_not_truncated(prs: list[dict]) -> list[dict]: return prs +def parse_extras(path: Path) -> list[int]: + """PR numbers from the extras manifest: one per line, blank lines and + ``#``-to-end-of-line comments allowed; a missing file means no extras. + A non-numeric entry is a config error — fail loud, don't drop a pin.""" + try: + text = path.read_text() + except FileNotFoundError: + return [] + numbers: list[int] = [] + for lineno, raw in enumerate(text.splitlines(), 1): + entry = raw.split("#", 1)[0].strip() + if not entry: + continue + if not entry.isdigit(): + raise StageError(f"{path}:{lineno}: invalid PR number {entry!r}") + numbers.append(int(entry)) + return numbers + + +def merge_stream(open_prs: list[dict], extras: list[int]) -> list[dict]: + """Union of open PRs and extras, deduped by PR number (the open entry + wins), ascending — the same ordering rule the composition always used.""" + open_nums = {p["number"] for p in open_prs} + stream = [{**p, "source": "open"} for p in open_prs] + stream += [{"number": n, "source": "extra"} for n in sorted(set(extras) - open_nums)] + return sorted(stream, key=lambda p: p["number"]) + + def remote_ref(cwd: str | Path, remote: str, ref: str) -> str: out = git(cwd, "ls-remote", remote, ref).stdout.strip() return out.split("\t")[0] if out else "" @@ -143,19 +179,40 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic applied: list[dict] = [] skipped: list[dict] = [] for pr in sorted(prs, key=lambda p: p["number"]): - num, branch, oid = pr["number"], pr["headRefName"], pr["headRefOid"] - git(cwd, "fetch", upstream, f"refs/pull/{num}/head") - # The listed head may have been force-pushed away between list and fetch. - if git(cwd, "cat-file", "-e", f"{oid}^{{commit}}", check=False).returncode != 0: - skipped.append( - { - "pr": num, - "branch": branch, - "conflict_paths": [], - "reason": "pinned head unreachable", - } - ) - continue + num, source = pr["number"], pr.get("source", "open") + if source == "extra": + # No pinned head for an extra: its refs/pull/N/head is frozen by + # GitHub after close. A deleted/garbage ref is a loud skip. + branch = f"pull/{num}/head" + fetch = git(cwd, "fetch", upstream, f"refs/pull/{num}/head", check=False) + head = git(cwd, "rev-parse", "-q", "--verify", "FETCH_HEAD^{commit}", check=False) + oid = head.stdout.strip() if fetch.returncode == 0 and head.returncode == 0 else "" + if not oid: + skipped.append( + { + "pr": num, + "branch": branch, + "conflict_paths": [], + "reason": EXTRA_UNFETCHABLE, + "source": source, + } + ) + continue + else: + branch, oid = pr["headRefName"], pr["headRefOid"] + git(cwd, "fetch", upstream, f"refs/pull/{num}/head") + # The listed head may have been force-pushed away between list and fetch. + if git(cwd, "cat-file", "-e", f"{oid}^{{commit}}", check=False).returncode != 0: + skipped.append( + { + "pr": num, + "branch": branch, + "conflict_paths": [], + "reason": "pinned head unreachable", + "source": source, + } + ) + continue merge = git(cwd, "merge", "--no-ff", "--no-commit", oid, check=False) if merge.returncode != 0: # Only a genuine content conflict (unmerged index entries) is @@ -166,7 +223,9 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic ) paths = conflict_paths(cwd) git(cwd, "merge", "--abort") - skipped.append({"pr": num, "branch": branch, "conflict_paths": paths}) + skipped.append( + {"pr": num, "branch": branch, "conflict_paths": paths, "source": source} + ) continue # --no-commit leaves MERGE_HEAD behind on a real merge; an # already-merged PR returns 0 with nothing to commit. @@ -184,10 +243,52 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic f"staging: merge PR #{num} ({branch} @ {oid[:12]})", env={"GIT_AUTHOR_DATE": when, "GIT_COMMITTER_DATE": when}, ) - applied.append({"pr": num, "branch": branch, "oid": oid}) + applied.append({"pr": num, "branch": branch, "oid": oid, "source": source}) return applied, skipped +def composition_of(cwd: str | Path, sha: str) -> tuple[str, dict[int, str]] | None: + """Decode a staging commit into (upstream base sha, {pr: head12}) from + its first-parent merge subjects; None when the sha isn't readable or the + base isn't found within a sane window.""" + if not sha or git(cwd, "cat-file", "-e", f"{sha}^{{commit}}", check=False).returncode != 0: + return None + window = str(PR_LIST_LIMIT * 2) + out = git(cwd, "log", "--first-parent", "-n", window, "--format=%H%x09%s", sha).stdout + merges: dict[int, str] = {} + for line in out.splitlines(): + commit, _, subject = line.partition("\t") + m = STAGING_MERGE_RE.fullmatch(subject) + if not m: + return commit, merges + merges[int(m.group(1))] = m.group(2) + return None + + +def push_causes( + old: tuple[str, dict[int, str]] | None, + upstream_sha: str, + new_merges: dict[int, str], + extra_nums: set[int], +) -> list[str]: + """Best-effort one-liner inputs for why staging moved: which of upstream + HEAD, the open PR set, or the extras changed since the old composition.""" + if old is None: + return ["no previous composition to compare"] + old_base, old_merges = old + causes: list[str] = [] + if old_base != upstream_sha: + causes.append("upstream HEAD") + changed = { + n for n in old_merges.keys() | new_merges.keys() if old_merges.get(n) != new_merges.get(n) + } + if changed - extra_nums: + causes.append("open PR set") + if changed & extra_nums: + causes.append("extras") + return causes or ["composition changed (cause unknown)"] + + def pin_name(cwd: str | Path, fork: str, datestamp: str, sha: str) -> tuple[str, bool]: """Immutable name for tonight's pin: nightly-YYYYMMDD, -rerunN if the day already has a pin at a different commit, no-op if it already points here.""" @@ -208,6 +309,7 @@ def stage( date: dt.date, upstream: str = "upstream", fork: str = "origin", + staging_only: bool = False, ) -> dict: datestamp = date.strftime("%Y%m%d") @@ -217,6 +319,41 @@ def stage( applied, skipped = merge_prs(cwd, prs, upstream) staging_sha = git(cwd, "rev-parse", "HEAD").stdout.strip() + + if staging_only: + # Hourly mode: only refs/heads/staging moves — no pins, no tags. + # Composition is byte-reproducible, so an identical remote sha means + # nothing changed and the push is skipped outright. + expected_staging = remote_ref(cwd, fork, "refs/heads/staging") + report = { + "date": datestamp, + "upstream_sha": upstream_sha, + "staging_sha": staging_sha, + "staging_only": True, + "pushed": expected_staging != staging_sha, + "causes": [], + "applied": applied, + "skipped": skipped, + } + if not report["pushed"]: + return report + git(cwd, "fetch", fork, "refs/heads/staging", check=False) + new_comp = composition_of(cwd, staging_sha) + report["causes"] = push_causes( + composition_of(cwd, expected_staging), + upstream_sha, + new_comp[1] if new_comp else {}, + {p["number"] for p in prs if p.get("source") == "extra"}, + ) + git( + cwd, + "push", + f"--force-with-lease=refs/heads/staging:{expected_staging}", + fork, + f"{staging_sha}:refs/heads/staging", + ) + return report + dev_tag = dev_version(cwd, upstream_sha, datestamp) name, created = pin_name(cwd, fork, datestamp, staging_sha) @@ -301,6 +438,25 @@ def notes(report: dict, signed: bool) -> str: def summarize(report: dict) -> str: + if report.get("staging_only"): + result = ( + f"pushed (changed: {', '.join(report['causes'])})" + if report["pushed"] + else "unchanged — push skipped" + ) + rows = [ + "## Personal staging hourly", + "", + "| | |", + "| --- | --- |", + f"| Upstream main | `{report['upstream_sha']}` |", + f"| Staging | `{report['staging_sha']}` |", + f"| Result | {result} |", + f"| Applied / skipped | {len(report['applied'])} / {len(report['skipped'])} |", + ] + rows += [f"| Skipped #{p['pr']} | {_skip_reason(p)} |" for p in report["skipped"]] + return "\n".join(rows) + "\n" + pin_note = "" if report["pin_created"] else " (already pinned — rerun no-op)" rows = [ "## Personal staging nightly", @@ -328,6 +484,16 @@ def main(argv: list[str] | None = None) -> int: p_stage.add_argument("--date", help="UTC datestamp YYYYMMDD (default: today)") p_stage.add_argument("--prs-json", help="read PRs from this JSON file instead of gh") p_stage.add_argument("--report", default="merge-report.json") + p_stage.add_argument( + "--staging-only", + action="store_true", + help="push only refs/heads/staging; mint no nightly pins or dev tags", + ) + p_stage.add_argument( + "--extras", + default=str(EXTRAS_FILE), + help="extras manifest path (missing file == no extras)", + ) p_notes = sub.add_parser("notes", help="render release notes from a merge report") p_notes.add_argument("--report", default="merge-report.json") @@ -344,6 +510,7 @@ def main(argv: list[str] | None = None) -> int: date = dt.date(int(args.date[:4]), int(args.date[4:6]), int(args.date[6:8])) else: date = dt.datetime.now(dt.timezone.utc).date() + title = "Personal staging hourly" if args.staging_only else "Personal staging nightly" try: # Fail the stage path before any fetch/merge if the parser is absent. if tomllib is None: @@ -353,12 +520,20 @@ def main(argv: list[str] | None = None) -> int: if args.prs_json else list_prs() ) + # Extras are read here, before any merge touches the worktree, so the + # manifest always comes from the trusted checkout. + prs = merge_stream(prs, parse_extras(Path(args.extras))) report = stage( - args.workdir, prs, date, upstream=args.upstream_remote, fork=args.fork_remote + args.workdir, + prs, + date, + upstream=args.upstream_remote, + fork=args.fork_remote, + staging_only=args.staging_only, ) except Exception as e: # The step summary is the failure surface — never exit without one. - append_summary(f"## Personal staging nightly\n\n**FAILED:** {e}\n") + append_summary(f"## {title}\n\n**FAILED:** {e}\n") raise Path(args.report).write_text(json.dumps(report, indent=2) + "\n") append_summary(summarize(report)) diff --git a/.github/scripts/personal-staging/test_stage.py b/.github/scripts/personal-staging/test_stage.py index c7fbd8bd31..545bbac186 100644 --- a/.github/scripts/personal-staging/test_stage.py +++ b/.github/scripts/personal-staging/test_stage.py @@ -83,8 +83,8 @@ def fork_ref(self, ref: str) -> str: def fork_log(self, ref: str) -> str: return git(self.fork, "log", "--format=%s", ref).stdout - def run(self, prs, date=DATE) -> dict: - return stage_mod.stage(self.work, prs, date) + def run(self, prs, date=DATE, **kwargs) -> dict: + return stage_mod.stage(self.work, prs, date, **kwargs) @pytest.fixture @@ -112,7 +112,9 @@ def test_conflicting_pr_skipped_with_paths(env): report = env.run([good, bad]) assert [p["pr"] for p in report["applied"]] == [2] - assert report["skipped"] == [{"pr": 3, "branch": "pr-3", "conflict_paths": ["a.txt"]}] + assert report["skipped"] == [ + {"pr": 3, "branch": "pr-3", "conflict_paths": ["a.txt"], "source": "open"} + ] # the good PR still landed on staging assert "merge PR #2" in env.fork_log("staging") @@ -398,3 +400,162 @@ def test_cli_writes_report_and_notes(env, tmp_path, capsys): assert "#11" in out assert "runner-ephemeral keystore" in out assert f"v1.2.3.dev{STAMP}" in out + + +def test_parse_extras_comments_blanks_and_missing(tmp_path): + manifest = tmp_path / "extras.txt" + manifest.write_text("# pinned fixes\n\n12 # trailing comment\n7\n12\n") + assert stage_mod.parse_extras(manifest) == [12, 7, 12] + assert stage_mod.parse_extras(tmp_path / "missing.txt") == [] + + +def test_parse_extras_garbage_line_fails_loud(tmp_path): + manifest = tmp_path / "extras.txt" + manifest.write_text("12\nnot-a-number\n") + with pytest.raises(stage_mod.StageError, match="invalid PR number"): + stage_mod.parse_extras(manifest) + + +def test_merge_stream_union_dedupe_open_wins(): + open_prs = [{"number": 9, "headRefName": "pr-9", "headRefOid": "x" * 40}] + stream = stage_mod.merge_stream(open_prs, [9, 4, 4, 20]) + assert [(p["number"], p["source"]) for p in stream] == [ + (4, "extra"), + (9, "open"), + (20, "extra"), + ] + # the open entry wins the dedupe: its pinned head survives + assert stream[1]["headRefOid"] == "x" * 40 + + +def test_extra_pr_merges_from_pull_ref_with_source(env): + open_pr = env.add_pr(3, "o.txt", "o\n") + env.add_pr(8, "x.txt", "x\n") # stands in for a closed PR: ref exists, not listed open + report = env.run(stage_mod.merge_stream([open_pr], [8])) + assert [(p["pr"], p["source"]) for p in report["applied"]] == [(3, "open"), (8, "extra")] + assert "merge PR #8 (pull/8/head" in env.fork_log("staging") + + +def test_unfetchable_extra_is_loud_skip(env): + ok = env.add_pr(2, "k.txt", "k\n") + report = env.run(stage_mod.merge_stream([ok], [999])) + assert [p["pr"] for p in report["applied"]] == [2] + assert report["skipped"] == [ + { + "pr": 999, + "branch": "pull/999/head", + "conflict_paths": [], + "reason": "extra unfetchable (likely deleted; remove from extras.txt)", + "source": "extra", + } + ] + # distinct from conflict skips, and loud in the human surfaces + assert "remove from extras.txt" in stage_mod.notes(report, signed=True) + report["pin_created"] = True + assert "remove from extras.txt" in stage_mod.summarize(report) + + +def test_staging_only_pushes_only_staging_with_lease(env, monkeypatch): + pr = env.add_pr(5, "s.txt", "s\n") + pushes = [] + real_git = stage_mod.git + + def spy(cwd, *args, **kwargs): + if args and args[0] == "push": + pushes.append(args) + return real_git(cwd, *args, **kwargs) + + monkeypatch.setattr(stage_mod, "git", spy) + report = env.run([pr], staging_only=True) + + assert report["pushed"] is True + assert env.fork_ref("refs/heads/staging") == report["staging_sha"] + # exactly one push, leased, carrying exactly one refspec + assert pushes == [ + ( + "push", + "--force-with-lease=refs/heads/staging:", + "origin", + f"{report['staging_sha']}:refs/heads/staging", + ) + ] + refs = [ + line.split("\t")[1] + for line in git(env.work, "ls-remote", str(env.fork)).stdout.strip().splitlines() + ] + assert refs == ["refs/heads/staging"] + for key in ("branch", "tag", "dev_tag", "pin_created"): + assert key not in report + + +def test_staging_only_noop_fast_path_skips_push(env, monkeypatch): + pr = env.add_pr(6, "n.txt", "n\n") + first = env.run([pr], staging_only=True) + assert first["pushed"] is True + + pushes = [] + real_git = stage_mod.git + + def spy(cwd, *args, **kwargs): + if args and args[0] == "push": + pushes.append(args) + return real_git(cwd, *args, **kwargs) + + monkeypatch.setattr(stage_mod, "git", spy) + second = env.run([pr], staging_only=True) + assert second["pushed"] is False + assert second["staging_sha"] == first["staging_sha"] + assert pushes == [] + assert "unchanged — push skipped" in stage_mod.summarize(second) + + +def test_staging_only_reports_change_causes(env): + pr = env.add_pr(7, "c7.txt", "7\n") + env.run([pr], staging_only=True) + + env.advance_main("c8.txt", "8\n") + second = env.run([pr], staging_only=True) + assert second["pushed"] is True + assert second["causes"] == ["upstream HEAD"] + + env.add_pr(9, "c9.txt", "9\n") + third = env.run(stage_mod.merge_stream([pr], [9]), staging_only=True) + assert third["causes"] == ["extras"] + + +def test_cli_staging_only_with_extras_manifest(env, tmp_path, monkeypatch, capsys): + open_pr = env.add_pr(11, "i.txt", "i\n") + env.add_pr(12, "j.txt", "j\n") # reachable only through the manifest + prs_json = tmp_path / "prs.json" + prs_json.write_text(json.dumps([open_pr])) + extras = tmp_path / "extras.txt" + extras.write_text("# pinned\n12\n999\n") + summary = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + report_path = tmp_path / "r.json" + + rc = stage_mod.main( + [ + "stage", + "--workdir", + str(env.work), + "--date", + STAMP, + "--prs-json", + str(prs_json), + "--extras", + str(extras), + "--staging-only", + "--report", + str(report_path), + ] + ) + assert rc == 0 + report = json.loads(report_path.read_text()) + assert [(p["pr"], p["source"]) for p in report["applied"]] == [(11, "open"), (12, "extra")] + assert [p["pr"] for p in report["skipped"]] == [999] + assert report["skipped"][0]["reason"].startswith("extra unfetchable") + text = summary.read_text() + assert "## Personal staging hourly" in text + assert "extra unfetchable" in text + capsys.readouterr() diff --git a/.github/workflows/personal-staging-hourly.yml b/.github/workflows/personal-staging-hourly.yml new file mode 100644 index 0000000000..500ccb8cc2 --- /dev/null +++ b/.github/workflows/personal-staging-hourly.yml @@ -0,0 +1,131 @@ +# Hourly personal staging refresh for the btli fork. +# +# Keeps fork branch `staging` = upstream main + open btli PRs + extras.txt +# pins between nightlies, via `stage.py stage --staging-only`: no nightly +# pins, no dev tags, no APK/releases/images — those stay on the nightly +# (personal-staging.yml). Composition is byte-reproducible, so an unchanged +# hour is a skipped push ("unchanged" in the summary). +name: Personal Staging Hourly + +on: + schedule: + - cron: "0 * * * *" + workflow_dispatch: + +permissions: + contents: read + +# Deliberately NOT the nightly's `personal-staging` group: hourly runs +# coalesce via cancel-in-progress, and sharing the group would let an hourly +# run cancel a running nightly. A rare push race with the nightly resolves +# loudly via --force-with-lease; a lost lease here is acceptable — the next +# hourly run retries. +concurrency: + group: personal-staging-hourly + cancel-in-progress: true + +jobs: + # Soft-fail twin of the nightly's sync-main: keep fork main tracking + # upstream, but a merge conflict only warns (abort + summary + exit 0) — + # the compose job stacks from upstream HEAD and never reads fork main, + # and hourly noise for a conflict the nightly already reports helps nobody. + sync-main: + # Fork-only, mirroring upstream's `if: github.repository == 'omnigent-ai/omnigent'` guards. + if: github.repository == 'btli/omnigent' + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write + steps: + - name: Checkout fork main with full history + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: main + fetch-depth: 0 + + - name: Merge upstream main and push (soft-fail on conflict) + run: | + set -euo pipefail + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git remote add upstream https://github.com/omnigent-ai/omnigent.git + git fetch upstream main + if ! git merge --no-edit FETCH_HEAD; then + { + echo "## sync-main: merge conflict with upstream main" + echo "" + git diff --name-only --diff-filter=U | while read -r f; do printf -- '- %s\n' "\`$f\`"; done + echo "" + echo "Resolve manually; this hour's staging compose still runs from upstream HEAD." + } >> "$GITHUB_STEP_SUMMARY" + git merge --abort + echo "::warning::sync-main: merge conflict with upstream main — resolve manually; staging still composes from upstream HEAD" + exit 0 + fi + git push origin main + + # Secretless test gate: `uv run` installs third-party dev deps, so this job + # never holds a write token (the repo's default pytest testpaths never + # collect .github/scripts/**, hence the explicit run here). + test-composer: + if: github.repository == 'btli/omnigent' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version-file: ".python-version" + + - name: Install uv + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + enable-cache: false + + - name: Test the staging composer + run: uv run --frozen --extra dev python -m pytest .github/scripts/personal-staging/ + + # Privileged (pushes refs/heads/staging) but runs ONLY the stdlib composer. + # Independent of sync-main by design: staging stacks from upstream HEAD, + # never fork main. + compose: + needs: test-composer + # Accident prevention for non-main dispatches (schedule always runs main); + # a dispatched ref executes its own workflow copy, so this is not a + # security boundary — see the nightly workflow's integrate job. + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: write + steps: + - name: Checkout fork with full history + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - name: Compose staging from upstream main + open btli PRs + extras + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + # git merge --no-ff --no-commit checks identity before the composer reaches + # its commit phase (run 31338098075 failed here). This identity is preflight-only; + # stage.py's COMMIT_IDENT -c overrides set reproducible staging commit identities. + # Do not use GIT_AUTHOR_* / GIT_COMMITTER_* env instead: env beats -c, + # changing those identities and breaking same-day SHA reproducibility. + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git remote add upstream https://github.com/omnigent-ai/omnigent.git + python3 .github/scripts/personal-staging/stage.py stage --staging-only \ + --upstream-remote upstream --fork-remote origin \ + --report merge-report.json + + - name: Report failure + if: failure() + run: echo "## Personal staging hourly — compose job FAILED (see logs)" >> "$GITHUB_STEP_SUMMARY" From 2b18c509f38169e40dfaaee6a65b102215e5f2ce Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Mon, 10 Aug 2026 16:17:32 -0700 Subject: [PATCH 2/6] fix(ci): tell an unreachable upstream apart from a deleted extra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the hourly staging ring found the same failure-mode conflation in two new places: a transient error was being reported as a permanent one. The extras path classified ANY fetch failure as "likely deleted; remove from extras.txt". A five-second network blip therefore dropped a required pin from staging and from the nightly's published artifacts, while advising the operator to delete a pin that was never gone — following that advice loses the PR from the build permanently. Retry the fetch, then ask ls-remote whether the ref actually exists: only a definite negative keeps the "remove it" wording. When upstream simply can't be reached, the hourly skips the push entirely so staging keeps its previous content instead of silently regressing, and the nightly fails before publishing releases from a composition missing a required pin. The hourly sync-main soft-passed EVERY nonzero `git merge` as "merge conflict, resolve manually", hiding repository corruption and bad objects behind an hourly warning. Check for unmerged index entries first — the rule the composer already applies — and only soft-fail a genuine content conflict. Also from review: - sync-main gets the same main-only ref guard as compose; it holds a write token and had none. - Move the schedule to :17. At :00 the hourly collided with the nightly's 10:00 UTC slot, where both push fork main and one loses non-fast-forward; the hourly now re-fetches and retries once, then warns and exits 0 rather than failing or ever force-pushing main. - Rebuild the push-cause summary from the in-scope applied list instead of re-parsing subjects this run just wrote, walk the previous composition to its real upstream base rather than a fixed window that unbounded extras could exceed, and report a PR that left the composition as dropped instead of mislabelling an unpinned extra as an open-PR change. - Document that staging is ephemeral and force-pushed up to 24x a day, so consumers pin a nightly-* pin or the dev tag rather than tracking the tip. Tests cover the three extras outcomes separately (confirmed deleted, transport failure, success after retry), the lease carrying a real prior sha, and each cause label including removal and an extra-to-open transition. Signed-off-by: Bryan Li Co-authored-by: omnigent --- .github/scripts/personal-staging/README.md | 61 +++++-- .github/scripts/personal-staging/stage.py | 127 +++++++++---- .../scripts/personal-staging/test_stage.py | 167 +++++++++++++++--- .github/workflows/personal-staging-hourly.yml | 57 ++++-- 4 files changed, 329 insertions(+), 83 deletions(-) diff --git a/.github/scripts/personal-staging/README.md b/.github/scripts/personal-staging/README.md index e48531b516..6671d3f6f2 100644 --- a/.github/scripts/personal-staging/README.md +++ b/.github/scripts/personal-staging/README.md @@ -43,8 +43,11 @@ existing behavior. ## Hourly staging refresh (`personal-staging-hourly.yml`) -`Personal Staging Hourly` (cron `0 * * * *`, plus `workflow_dispatch`) -keeps branch `staging` fresh between nightlies. It runs the same composer +`Personal Staging Hourly` (cron `17 * * * *`, plus `workflow_dispatch`) +keeps branch `staging` fresh between nightlies. The odd minute is +deliberate: `:00` would collide with the nightly's 10:00 UTC slot (both +push fork main) and GitHub delays or drops runs scheduled on that +congested minute. It runs the same composer with `--staging-only`: compose upstream main + open btli PRs + extras exactly as the nightly does, then push ONLY `refs/heads/staging` (`--force-with-lease`). It mints no `nightly-*` pins and no dev tags, and @@ -56,14 +59,39 @@ the summary reports "unchanged". When it does push, the summary's one-line result names which of upstream HEAD, the open PR set, or the extras changed. -Its `sync-main` job soft-fails on a merge conflict (abort + `::warning::` + -summary, exit 0) — unlike the nightly's, which hard-fails — because the -compose job stacks from upstream HEAD and never reads fork main. The -workflow uses its own concurrency group (`personal-staging-hourly`, +Its `sync-main` job soft-fails on a merge *conflict* (abort + `::warning::` ++ summary, exit 0) — unlike the nightly's, which hard-fails — because the +compose job stacks from upstream HEAD and never reads fork main. Only a +genuine content conflict is soft-failed: a merge that fails with no +unmerged paths (bad object, corrupt repo) still fails the job loudly. If +its `git push origin main` loses a race with the nightly or a human push, +it re-fetches and retries once, then warns and exits 0 — `main` is never +force-pushed. + +The workflow uses its own concurrency group (`personal-staging-hourly`, `cancel-in-progress: true`) so stale hourly runs coalesce and can never cancel a running nightly; if an hourly push loses a `--force-with-lease` race with the nightly, the next hour retries. +## `staging` is ephemeral — do not track it + +`staging` is **rebuilt from scratch and force-pushed, now up to 24× a +day**. Its history is rewritten every time upstream or a PR moves: commit +shas are not stable, and a commit that was on the branch an hour ago may +be gone. Nothing should track the branch tip. + +- **Pin instead:** for anything reproducible — homelab deploys, container + builds, bisecting — use a `nightly-YYYYMMDD` pin (branch + tag, both + immutable) or the `vX.Y.Z.devYYYYMMDD` tag from the nightly. +- **Existing clone:** `git pull` on `staging` will refuse or conflict + after a rewrite. Recover with: + + ```sh + git fetch origin && git reset --hard origin/staging + ``` + + (discards local work on the branch — keep none there). + ## Extras manifest (`extras.txt`) `stage.py` always reads `.github/scripts/personal-staging/extras.txt` @@ -76,10 +104,23 @@ they are no longer open (typically closed-without-merge) — GitHub keeps `refs/pull/N/head` fetchable after close. The merge stream is the union of open PRs and extras, deduped by PR number (the open entry wins), sorted ascending — the same ordering rule as always. **Remove an entry -once the change lands upstream.** An extra whose ref can no longer be -fetched is skipped loudly, with reason `extra unfetchable (likely -deleted; remove from extras.txt)` in the report and step summary — -distinct from a conflict skip — but does not fail the run. +once the change lands upstream.** + +An extra that can't be resolved gets one of two distinct outcomes, because +a deleted ref and an unreachable server are different problems: + +- **Confirmed gone** (`ls-remote` says the ref no longer exists): skipped + loudly with reason `extra unfetchable (likely deleted; remove from + extras.txt)` — distinct from a conflict skip — and the run continues. + This is the only reason that invites editing the manifest. +- **Could not reach upstream** (the fetch keeps failing after retries, and + the existence probe itself errors): reason `extra fetch failed (cannot + reach upstream; pin kept, staging not advanced)`. The hourly run does + **not** push — `staging` keeps its previous content rather than silently + losing a required pin — and emits a `::warning::`. The nightly fails the + job instead, before any ref moves, since it publishes releases from that + composition. **Do not delete the pin on this reason**; it means the + fetch failed, not that the PR is gone. ## Stable download URL diff --git a/.github/scripts/personal-staging/stage.py b/.github/scripts/personal-staging/stage.py index d561bcfc82..82f8421c49 100644 --- a/.github/scripts/personal-staging/stage.py +++ b/.github/scripts/personal-staging/stage.py @@ -23,6 +23,7 @@ import re import subprocess import sys +import time from pathlib import Path # Resolved and cached at module load, while sys.path[0] (this script's dir) @@ -38,10 +39,16 @@ PR_AUTHOR = "btli" PR_LIST_LIMIT = 100 EXTRAS_FILE = Path(__file__).resolve().parent / "extras.txt" -EXTRA_UNFETCHABLE = "extra unfetchable (likely deleted; remove from extras.txt)" +# Two different answers about a pinned extra: only a ref confirmed absent +# invites editing the manifest, and only a failure to reach the remote blocks +# the push (dropping a required pin would silently regress staging). +EXTRA_MISSING = "extra unfetchable (likely deleted; remove from extras.txt)" +EXTRA_FETCH_FAILED = "extra fetch failed (cannot reach upstream; pin kept, staging not advanced)" +EXTRA_FETCH_ATTEMPTS = 3 +EXTRA_FETCH_BACKOFF_S = 2 # Subject line minted below for every staging merge commit; also decoded to # diff two compositions (what changed between the old and new staging). -STAGING_MERGE_RE = re.compile(r"staging: merge PR #(\d+) \(.+ @ ([0-9a-f]{12})\)") +STAGING_MERGE_RE = re.compile(r"staging: merge PR #(\d+) \((.+) @ ([0-9a-f]{12})\)") # Fixed identity, no gpg signature: the merge commit must be byte-reproducible # so an unchanged same-day rerun lands on the identical sha (no-op detection). COMMIT_IDENT = [ @@ -140,6 +147,26 @@ def parse_extras(path: Path) -> list[int]: return numbers +def fetch_extra(cwd: str | Path, upstream: str, num: int) -> tuple[str, str]: + """Resolve a pinned extra's frozen ``refs/pull/N/head`` to a commit oid, + returning ``(oid, "")`` on success and ``("", reason)`` otherwise. A + deleted ref and an unreachable remote are different answers: only + ls-remote reporting the ref absent proves deletion, so only that outcome + invites editing the manifest.""" + ref = f"refs/pull/{num}/head" + for attempt in range(EXTRA_FETCH_ATTEMPTS): + if git(cwd, "fetch", upstream, ref, check=False).returncode == 0: + head = git(cwd, "rev-parse", "-q", "--verify", "FETCH_HEAD^{commit}", check=False) + if head.returncode == 0: + return head.stdout.strip(), "" + if attempt + 1 < EXTRA_FETCH_ATTEMPTS: + time.sleep(EXTRA_FETCH_BACKOFF_S) + probe = git(cwd, "ls-remote", upstream, ref, check=False) + if probe.returncode == 0 and not probe.stdout.strip(): + return "", EXTRA_MISSING + return "", EXTRA_FETCH_FAILED + + def merge_stream(open_prs: list[dict], extras: list[int]) -> list[dict]: """Union of open PRs and extras, deduped by PR number (the open entry wins), ascending — the same ordering rule the composition always used.""" @@ -182,18 +209,16 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic num, source = pr["number"], pr.get("source", "open") if source == "extra": # No pinned head for an extra: its refs/pull/N/head is frozen by - # GitHub after close. A deleted/garbage ref is a loud skip. + # GitHub after close, so the ref itself is the only pin. branch = f"pull/{num}/head" - fetch = git(cwd, "fetch", upstream, f"refs/pull/{num}/head", check=False) - head = git(cwd, "rev-parse", "-q", "--verify", "FETCH_HEAD^{commit}", check=False) - oid = head.stdout.strip() if fetch.returncode == 0 and head.returncode == 0 else "" + oid, reason = fetch_extra(cwd, upstream, num) if not oid: skipped.append( { "pr": num, "branch": branch, "conflict_paths": [], - "reason": EXTRA_UNFETCHABLE, + "reason": reason, "source": source, } ) @@ -247,45 +272,49 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic return applied, skipped -def composition_of(cwd: str | Path, sha: str) -> tuple[str, dict[int, str]] | None: - """Decode a staging commit into (upstream base sha, {pr: head12}) from - its first-parent merge subjects; None when the sha isn't readable or the - base isn't found within a sane window.""" +def composition_of(cwd: str | Path, sha: str) -> tuple[str, dict[int, tuple[str, str]]] | None: + """Decode a pushed staging commit into (upstream base sha, {pr: (branch, + head12)}) from its first-parent merge subjects, walking to the real base — + the merge count is unbounded. None when the sha isn't readable.""" if not sha or git(cwd, "cat-file", "-e", f"{sha}^{{commit}}", check=False).returncode != 0: return None - window = str(PR_LIST_LIMIT * 2) - out = git(cwd, "log", "--first-parent", "-n", window, "--format=%H%x09%s", sha).stdout - merges: dict[int, str] = {} + merges: dict[int, tuple[str, str]] = {} + out = git(cwd, "log", "--first-parent", "--format=%H%x09%s", sha).stdout for line in out.splitlines(): commit, _, subject = line.partition("\t") m = STAGING_MERGE_RE.fullmatch(subject) if not m: return commit, merges - merges[int(m.group(1))] = m.group(2) + merges[int(m.group(1))] = (m.group(2), m.group(3)) return None def push_causes( - old: tuple[str, dict[int, str]] | None, + old: tuple[str, dict[int, tuple[str, str]]] | None, upstream_sha: str, - new_merges: dict[int, str], - extra_nums: set[int], + applied: list[dict], ) -> list[str]: - """Best-effort one-liner inputs for why staging moved: which of upstream - HEAD, the open PR set, or the extras changed since the old composition.""" + """One-line answer to "why did staging move": which composition inputs + differ from the one the remote branch already carried.""" if old is None: return ["no previous composition to compare"] old_base, old_merges = old + new = {p["pr"]: (p["branch"], str(p["oid"])[:12], p["source"]) for p in applied} + label = {"open": "open PR set", "extra": "extras"} causes: list[str] = [] if old_base != upstream_sha: causes.append("upstream HEAD") - changed = { - n for n in old_merges.keys() | new_merges.keys() if old_merges.get(n) != new_merges.get(n) - } - if changed - extra_nums: - causes.append("open PR set") - if changed & extra_nums: - causes.append("extras") + dropped: list[str] = [] + for num in sorted(old_merges.keys() | new.keys()): + entry = new.get(num) + if entry is None: + # No longer composed at all (unpinned extra, closed PR, skip) — + # say so rather than guessing which input it used to come from. + dropped.append(f"#{num}") + elif old_merges.get(num) != entry[:2] and label[entry[2]] not in causes: + causes.append(label[entry[2]]) + if dropped: + causes.append("dropped PR " + ", ".join(dropped)) return causes or ["composition changed (cause unknown)"] @@ -320,6 +349,10 @@ def stage( applied, skipped = merge_prs(cwd, prs, upstream) staging_sha = git(cwd, "rev-parse", "HEAD").stdout.strip() + # An extra we could not even reach is an infrastructure failure, not a + # composition: publishing without it would silently regress staging. + blocked = [p["pr"] for p in skipped if p.get("reason") == EXTRA_FETCH_FAILED] + if staging_only: # Hourly mode: only refs/heads/staging moves — no pins, no tags. # Composition is byte-reproducible, so an identical remote sha means @@ -330,7 +363,8 @@ def stage( "upstream_sha": upstream_sha, "staging_sha": staging_sha, "staging_only": True, - "pushed": expected_staging != staging_sha, + "blocked": blocked, + "pushed": not blocked and expected_staging != staging_sha, "causes": [], "applied": applied, "skipped": skipped, @@ -338,12 +372,8 @@ def stage( if not report["pushed"]: return report git(cwd, "fetch", fork, "refs/heads/staging", check=False) - new_comp = composition_of(cwd, staging_sha) report["causes"] = push_causes( - composition_of(cwd, expected_staging), - upstream_sha, - new_comp[1] if new_comp else {}, - {p["number"] for p in prs if p.get("source") == "extra"}, + composition_of(cwd, expected_staging), upstream_sha, applied ) git( cwd, @@ -354,6 +384,16 @@ def stage( ) return report + # The nightly publishes releases from this composition, and its downstream + # jobs consume the report's sha/tag — so refuse loudly before any ref moves + # rather than shipping artifacts that quietly omit a required pin. + if blocked: + raise StageError( + "extras unreachable due to infrastructure failure (not deleted): " + + ", ".join(f"#{n}" for n in blocked) + + "; refusing to publish a composition without them" + ) + dev_tag = dev_version(cwd, upstream_sha, datestamp) name, created = pin_name(cwd, fork, datestamp, staging_sha) @@ -439,11 +479,16 @@ def notes(report: dict, signed: bool) -> str: def summarize(report: dict) -> str: if report.get("staging_only"): - result = ( - f"pushed (changed: {', '.join(report['causes'])})" - if report["pushed"] - else "unchanged — push skipped" - ) + if report["blocked"]: + result = ( + "**NOT pushed** — extras unreachable: " + + ", ".join(f"#{n}" for n in report["blocked"]) + + " (staging left at its previous commit)" + ) + elif report["pushed"]: + result = f"pushed (changed: {', '.join(report['causes'])})" + else: + result = "unchanged — push skipped" rows = [ "## Personal staging hourly", "", @@ -537,6 +582,12 @@ def main(argv: list[str] | None = None) -> int: raise Path(args.report).write_text(json.dumps(report, indent=2) + "\n") append_summary(summarize(report)) + if report.get("blocked"): + print( + "::warning::could not reach upstream for extras " + + ", ".join(f"#{n}" for n in report["blocked"]) + + " — staging left unchanged; this is an infrastructure failure, not a deleted ref" + ) print(json.dumps(report, indent=2)) return 0 diff --git a/.github/scripts/personal-staging/test_stage.py b/.github/scripts/personal-staging/test_stage.py index 545bbac186..c5dfc88fa3 100644 --- a/.github/scripts/personal-staging/test_stage.py +++ b/.github/scripts/personal-staging/test_stage.py @@ -70,6 +70,14 @@ def add_pr(self, number: int, filename: str, content: str) -> dict: git(self.seed, "push", str(self.upstream), f"{branch}:refs/pull/{number}/head") return {"number": number, "headRefName": branch, "headRefOid": oid} + def advance_pr(self, number: int, filename: str, content: str) -> dict: + """New head on an existing PR branch, force-pushed to its pull ref.""" + branch = f"pr-{number}" + git(self.seed, "checkout", "-q", branch) + oid = commit_file(self.seed, filename, content, f"pr {number} update") + git(self.seed, "push", "-f", str(self.upstream), f"{branch}:refs/pull/{number}/head") + return {"number": number, "headRefName": branch, "headRefOid": oid} + def advance_main(self, filename: str, content: str) -> str: git(self.seed, "checkout", "-q", "main") sha = commit_file(self.seed, filename, content, f"main: {filename}") @@ -92,6 +100,28 @@ def env(tmp_path): return Env(tmp_path) +@pytest.fixture +def pushes(monkeypatch): + """Record the argv of every `git push` the composer issues (still running + them), so tests can assert exactly which refs and leases were used.""" + recorded: list[tuple] = [] + real_git = stage_mod.git + + def spy(cwd, *args, **kwargs): + if args and args[0] == "push": + recorded.append(args) + return real_git(cwd, *args, **kwargs) + + monkeypatch.setattr(stage_mod, "git", spy) + return recorded + + +@pytest.fixture(autouse=True) +def no_backoff(monkeypatch): + """The extras fetch sleeps between retries in production; no test waits.""" + monkeypatch.setattr(stage_mod, "EXTRA_FETCH_BACKOFF_S", 0) + + def test_merges_prs_ascending_and_pushes_staging(env): pr9 = env.add_pr(9, "nine.txt", "9\n") pr4 = env.add_pr(4, "four.txt", "4\n") @@ -405,7 +435,9 @@ def test_cli_writes_report_and_notes(env, tmp_path, capsys): def test_parse_extras_comments_blanks_and_missing(tmp_path): manifest = tmp_path / "extras.txt" manifest.write_text("# pinned fixes\n\n12 # trailing comment\n7\n12\n") - assert stage_mod.parse_extras(manifest) == [12, 7, 12] + # the contract is WHICH numbers are pinned — comments and blank lines are + # ignored; ordering and duplicates are normalized later by merge_stream + assert sorted(set(stage_mod.parse_extras(manifest))) == [7, 12] assert stage_mod.parse_extras(tmp_path / "missing.txt") == [] @@ -436,7 +468,9 @@ def test_extra_pr_merges_from_pull_ref_with_source(env): assert "merge PR #8 (pull/8/head" in env.fork_log("staging") -def test_unfetchable_extra_is_loud_skip(env): +def test_extra_confirmed_deleted_is_loud_advisory_skip(env): + """ls-remote proves the ref is gone: skip it, keep composing, and tell the + operator to edit the manifest.""" ok = env.add_pr(2, "k.txt", "k\n") report = env.run(stage_mod.merge_stream([ok], [999])) assert [p["pr"] for p in report["applied"]] == [2] @@ -445,7 +479,7 @@ def test_unfetchable_extra_is_loud_skip(env): "pr": 999, "branch": "pull/999/head", "conflict_paths": [], - "reason": "extra unfetchable (likely deleted; remove from extras.txt)", + "reason": stage_mod.EXTRA_MISSING, "source": "extra", } ] @@ -455,17 +489,63 @@ def test_unfetchable_extra_is_loud_skip(env): assert "remove from extras.txt" in stage_mod.summarize(report) -def test_staging_only_pushes_only_staging_with_lease(env, monkeypatch): - pr = env.add_pr(5, "s.txt", "s\n") - pushes = [] +def _break_ref(monkeypatch, ref_fragment: str, *, commands: tuple[str, ...], fail_times: int): + """Make the named git commands fail for one ref, like a transport blip.""" real_git = stage_mod.git + seen: list[tuple] = [] - def spy(cwd, *args, **kwargs): - if args and args[0] == "push": - pushes.append(args) + def flaky(cwd, *args, **kwargs): + if args[:1] in {(c,) for c in commands} and any(ref_fragment in a for a in args): + seen.append(args) + if len(seen) <= fail_times: + return subprocess.CompletedProcess(args, 128, "", "fatal: unable to access") return real_git(cwd, *args, **kwargs) - monkeypatch.setattr(stage_mod, "git", spy) + monkeypatch.setattr(stage_mod, "git", flaky) + return seen + + +def test_extra_transport_failure_blocks_push_and_spares_the_manifest(env, monkeypatch): + """An unreachable upstream must not masquerade as a deleted ref: staging + keeps its previous content and nobody is told to delete a live pin.""" + ok = env.add_pr(3, "t.txt", "t\n") + env.add_pr(4, "u.txt", "u\n") + stream = stage_mod.merge_stream([ok], [4]) + first = env.run(stream, staging_only=True) + assert [p["pr"] for p in first["applied"]] == [3, 4] + + _break_ref(monkeypatch, "pull/4/head", commands=("fetch", "ls-remote"), fail_times=99) + report = env.run(stream, staging_only=True) + + skip = report["skipped"][0] + assert skip["reason"] == stage_mod.EXTRA_FETCH_FAILED + assert "extras.txt" not in skip["reason"] + assert report["blocked"] == [4] + assert report["pushed"] is False + # previous staging content preserved — no silent regression + assert env.fork_ref("refs/heads/staging") == first["staging_sha"] + assert "NOT pushed" in stage_mod.summarize(report) + + +def test_extra_transport_failure_fails_the_nightly_before_publishing(env, monkeypatch): + env.add_pr(4, "u.txt", "u\n") + _break_ref(monkeypatch, "pull/4/head", commands=("fetch", "ls-remote"), fail_times=99) + with pytest.raises(stage_mod.StageError, match="infrastructure failure"): + env.run(stage_mod.merge_stream([], [4])) + assert env.fork_ref("refs/heads/staging") == "" + + +def test_extra_fetch_succeeds_after_retry(env, monkeypatch): + env.add_pr(5, "r.txt", "r\n") + seen = _break_ref(monkeypatch, "pull/5/head", commands=("fetch",), fail_times=1) + report = env.run(stage_mod.merge_stream([], [5]), staging_only=True) + assert len(seen) == 2 # one blip, then the retry lands + assert [(p["pr"], p["source"]) for p in report["applied"]] == [(5, "extra")] + assert report["skipped"] == [] and report["blocked"] == [] + + +def test_staging_only_pushes_only_staging_with_lease(env, pushes): + pr = env.add_pr(5, "s.txt", "s\n") report = env.run([pr], staging_only=True) assert report["pushed"] is True @@ -488,20 +568,31 @@ def spy(cwd, *args, **kwargs): assert key not in report -def test_staging_only_noop_fast_path_skips_push(env, monkeypatch): - pr = env.add_pr(6, "n.txt", "n\n") +def test_staging_only_lease_pins_the_previous_remote_sha(env, pushes): + """The repeat-run case: the lease must carry the sha staging actually had, + not the cold-start empty value.""" + pr = env.add_pr(30, "l.txt", "l\n") first = env.run([pr], staging_only=True) - assert first["pushed"] is True + pushes.clear() - pushes = [] - real_git = stage_mod.git + env.advance_main("l2.txt", "l2\n") + second = env.run([pr], staging_only=True) + assert pushes == [ + ( + "push", + f"--force-with-lease=refs/heads/staging:{first['staging_sha']}", + "origin", + f"{second['staging_sha']}:refs/heads/staging", + ) + ] - def spy(cwd, *args, **kwargs): - if args and args[0] == "push": - pushes.append(args) - return real_git(cwd, *args, **kwargs) - monkeypatch.setattr(stage_mod, "git", spy) +def test_staging_only_noop_fast_path_skips_push(env, pushes): + pr = env.add_pr(6, "n.txt", "n\n") + first = env.run([pr], staging_only=True) + assert first["pushed"] is True + pushes.clear() + second = env.run([pr], staging_only=True) assert second["pushed"] is False assert second["staging_sha"] == first["staging_sha"] @@ -523,6 +614,42 @@ def test_staging_only_reports_change_causes(env): assert third["causes"] == ["extras"] +def test_staging_only_cause_open_pr_head_moved(env): + pr = env.add_pr(31, "o.txt", "o\n") + env.run([pr], staging_only=True) + moved = env.advance_pr(31, "o.txt", "o2\n") + second = env.run([moved], staging_only=True) + assert second["causes"] == ["open PR set"] + + +def test_staging_only_cause_extra_removed_then_promoted(env): + keep = env.add_pr(32, "k.txt", "k\n") + extra = env.add_pr(33, "x.txt", "x\n") + env.run(stage_mod.merge_stream([keep], [33]), staging_only=True) + + # unpinned from extras.txt: reported as dropped, never as an open-PR change + dropped = env.run(stage_mod.merge_stream([keep], []), staging_only=True) + assert dropped["causes"] == ["dropped PR #33"] + + # the same number returns as an open PR — a source transition, not a drop + promoted = env.run(stage_mod.merge_stream([keep, extra], []), staging_only=True) + assert promoted["causes"] == ["open PR set"] + + +def test_old_composition_decoded_past_any_merge_count(env, monkeypatch): + """The previous composition is decoded by walking to the real upstream + base, so a manifest longer than any PR-list bound still yields real + causes instead of degrading to 'no previous composition'.""" + extras = [env.add_pr(40 + i, f"w{i}.txt", f"{i}\n") for i in range(3)] + stream = stage_mod.merge_stream([], [p["number"] for p in extras]) + env.run(stream, staging_only=True) + + monkeypatch.setattr(stage_mod, "PR_LIST_LIMIT", 1) + env.advance_main("wmain.txt", "m\n") + second = env.run(stream, staging_only=True) + assert second["causes"] == ["upstream HEAD"] + + def test_cli_staging_only_with_extras_manifest(env, tmp_path, monkeypatch, capsys): open_pr = env.add_pr(11, "i.txt", "i\n") env.add_pr(12, "j.txt", "j\n") # reachable only through the manifest diff --git a/.github/workflows/personal-staging-hourly.yml b/.github/workflows/personal-staging-hourly.yml index 500ccb8cc2..89e5290ac2 100644 --- a/.github/workflows/personal-staging-hourly.yml +++ b/.github/workflows/personal-staging-hourly.yml @@ -9,7 +9,10 @@ name: Personal Staging Hourly on: schedule: - - cron: "0 * * * *" + # Off the top of the hour on purpose: :00 would collide with the nightly's + # 10:00 UTC slot (both push fork main), and GitHub delays or drops runs + # scheduled on the congested :00 minute. + - cron: "17 * * * *" workflow_dispatch: permissions: @@ -26,12 +29,17 @@ concurrency: jobs: # Soft-fail twin of the nightly's sync-main: keep fork main tracking - # upstream, but a merge conflict only warns (abort + summary + exit 0) — + # upstream, but a content conflict only warns (abort + summary + exit 0) — # the compose job stacks from upstream HEAD and never reads fork main, # and hourly noise for a conflict the nightly already reports helps nobody. + # Only conflicts are soft: a merge or push failing for any other reason + # still fails the job. sync-main: - # Fork-only, mirroring upstream's `if: github.repository == 'omnigent-ai/omnigent'` guards. - if: github.repository == 'btli/omnigent' + # Fork-only, and main-only for the same accident-prevention reason as the + # compose job below: a dispatched ref runs its own workflow copy, so this + # guard is not a server-enforced boundary — it just keeps a stray dispatch + # from pushing fork main with this job's write token. + if: github.repository == 'btli/omnigent' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -50,19 +58,38 @@ jobs: git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git remote add upstream https://github.com/omnigent-ai/omnigent.git git fetch upstream main - if ! git merge --no-edit FETCH_HEAD; then - { - echo "## sync-main: merge conflict with upstream main" - echo "" - git diff --name-only --diff-filter=U | while read -r f; do printf -- '- %s\n' "\`$f\`"; done - echo "" - echo "Resolve manually; this hour's staging compose still runs from upstream HEAD." - } >> "$GITHUB_STEP_SUMMARY" - git merge --abort - echo "::warning::sync-main: merge conflict with upstream main — resolve manually; staging still composes from upstream HEAD" + + soft_fail() { + { echo "## sync-main: $1"; echo ""; printf '%s\n' "$2"; } >> "$GITHUB_STEP_SUMMARY" + echo "::warning::sync-main: $1 (see step summary) — this hour's staging compose still runs from upstream HEAD" exit 0 + } + + # Same rule the composer applies: only genuine content conflicts + # (unmerged index entries) are skippable. A bad object or a transport + # error must stay loud instead of hiding behind an hourly warning. + merge_or_soft_fail() { + if git merge --no-edit "$1"; then + return 0 + fi + if [ -z "$(git ls-files -u)" ]; then + echo "::error::sync-main: git merge failed with no conflicting paths — not a content conflict" + exit 1 + fi + paths="$(git diff --name-only --diff-filter=U | while read -r f; do printf -- '- %s\n' "\`$f\`"; done)" + git merge --abort + soft_fail "merge conflict with upstream main — resolve manually" "$paths" + } + + merge_or_soft_fail FETCH_HEAD + if ! git push origin main; then + # Lost a race (the nightly, or a human push). Integrate and retry + # once; never force-push main, and never fail the hourly for it. + git fetch origin main + merge_or_soft_fail FETCH_HEAD + git push origin main \ + || soft_fail "could not push fork main" "Lost the push race twice; the next hourly run retries." fi - git push origin main # Secretless test gate: `uv run` installs third-party dev deps, so this job # never holds a write token (the repo's default pytest testpaths never From 1a0cb1b798661e34117bcfd594b7157118bf7dc8 Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Mon, 10 Aug 2026 16:39:59 -0700 Subject: [PATCH 3/6] fix(ci): stop blaming an already-merged PR for a staging move The hourly cause line builds the new composition from `applied` but decodes the old one from merge-commit subjects, and an open PR whose head upstream already contains mints no merge commit. It showed up on the new side only, so an upstream move read as "upstream HEAD, open PR set". Compare only entries that actually minted a merge commit. Co-authored-by: omnigent Signed-off-by: Bryan Li --- .github/scripts/personal-staging/stage.py | 14 ++++++++++++-- .github/scripts/personal-staging/test_stage.py | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.github/scripts/personal-staging/stage.py b/.github/scripts/personal-staging/stage.py index 82f8421c49..f229967471 100644 --- a/.github/scripts/personal-staging/stage.py +++ b/.github/scripts/personal-staging/stage.py @@ -254,6 +254,7 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic continue # --no-commit leaves MERGE_HEAD behind on a real merge; an # already-merged PR returns 0 with nothing to commit. + minted = False if git(cwd, "rev-parse", "-q", "--verify", "MERGE_HEAD", check=False).returncode == 0: # Stamp the merge with the PR head's committer date: identical # inputs reproduce the exact staging sha, so a same-day rerun with @@ -268,7 +269,10 @@ def merge_prs(cwd: str | Path, prs: list[dict], upstream: str) -> tuple[list[dic f"staging: merge PR #{num} ({branch} @ {oid[:12]})", env={"GIT_AUTHOR_DATE": when, "GIT_COMMITTER_DATE": when}, ) - applied.append({"pr": num, "branch": branch, "oid": oid, "source": source}) + minted = True + applied.append( + {"pr": num, "branch": branch, "oid": oid, "source": source, "minted": minted} + ) return applied, skipped @@ -299,7 +303,13 @@ def push_causes( if old is None: return ["no previous composition to compare"] old_base, old_merges = old - new = {p["pr"]: (p["branch"], str(p["oid"])[:12], p["source"]) for p in applied} + # Only minted merges are comparable: the decoded old composition knows + # merge subjects, and an already-merged PR mints none. + new = { + p["pr"]: (p["branch"], str(p["oid"])[:12], p["source"]) + for p in applied + if p.get("minted", True) + } label = {"open": "open PR set", "extra": "extras"} causes: list[str] = [] if old_base != upstream_sha: diff --git a/.github/scripts/personal-staging/test_stage.py b/.github/scripts/personal-staging/test_stage.py index c5dfc88fa3..0d3de24c72 100644 --- a/.github/scripts/personal-staging/test_stage.py +++ b/.github/scripts/personal-staging/test_stage.py @@ -636,6 +636,21 @@ def test_staging_only_cause_extra_removed_then_promoted(env): assert promoted["causes"] == ["open PR set"] +def test_staging_only_cause_already_merged_pr_is_not_a_change(env): + """An open PR already merged upstream mints no merge commit, so the old + composition can't mention it either: an upstream move must not be + misreported as an open-PR change.""" + git(env.seed, "checkout", "-q", "main") + head = git(env.seed, "rev-parse", "HEAD").stdout.strip() + git(env.seed, "push", str(env.upstream), "main:refs/pull/50/head") + pr = {"number": 50, "headRefName": "pr-50", "headRefOid": head} + env.run([pr], staging_only=True) + + env.advance_main("am.txt", "am\n") + second = env.run([pr], staging_only=True) + assert second["causes"] == ["upstream HEAD"] + + def test_old_composition_decoded_past_any_merge_count(env, monkeypatch): """The previous composition is decoded by walking to the real upstream base, so a manifest longer than any PR-list bound still yields real From 45a223b74ee31397d46a47b2209736bec9ed3f18 Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Mon, 10 Aug 2026 21:18:29 -0700 Subject: [PATCH 4/6] fix(ci): classify hourly push soft-fails and quiet no-op summaries Soft-fail fork/main pushes only on confirmed stale-ref rejection so auth and transport failures stay red; collapse the hourly skip table on no-op runs; label unpublished candidates distinctly; correct extras post-F1 docs; drop a dead PR_LIST_LIMIT monkeypatch. Signed-off-by: Bryan Li --- .github/scripts/personal-staging/README.md | 8 +- .github/scripts/personal-staging/extras.txt | 5 +- .github/scripts/personal-staging/stage.py | 27 ++++- .../scripts/personal-staging/test_stage.py | 100 ++++++++++++++++-- .github/workflows/personal-staging-hourly.yml | 34 ++++-- 5 files changed, 155 insertions(+), 19 deletions(-) diff --git a/.github/scripts/personal-staging/README.md b/.github/scripts/personal-staging/README.md index 6671d3f6f2..caf552ab8a 100644 --- a/.github/scripts/personal-staging/README.md +++ b/.github/scripts/personal-staging/README.md @@ -64,9 +64,11 @@ Its `sync-main` job soft-fails on a merge *conflict* (abort + `::warning::` compose job stacks from upstream HEAD and never reads fork main. Only a genuine content conflict is soft-failed: a merge that fails with no unmerged paths (bad object, corrupt repo) still fails the job loudly. If -its `git push origin main` loses a race with the nightly or a human push, -it re-fetches and retries once, then warns and exits 0 — `main` is never -force-pushed. +its `git push origin main` is rejected as a confirmed stale ref +(`! [rejected]` with `non-fast-forward` / `fetch first` / `stale info` — +typically a race with the nightly or a human push), it re-fetches and +retries once, then warns and exits 0. Auth, permissions, or transport +failures still fail the job. `main` is never force-pushed. The workflow uses its own concurrency group (`personal-staging-hourly`, `cancel-in-progress: true`) so stale hourly runs coalesce and can never diff --git a/.github/scripts/personal-staging/extras.txt b/.github/scripts/personal-staging/extras.txt index a045878d2e..405f08e2e1 100644 --- a/.github/scripts/personal-staging/extras.txt +++ b/.github/scripts/personal-staging/extras.txt @@ -3,7 +3,10 @@ # PRs listed here are merged into `staging` even when no longer open # (GitHub keeps refs/pull/N/head fetchable after close). Blank lines and # `#` comments are ignored. Remove an entry once the change lands -# upstream; a deleted/unfetchable ref is skipped loudly in the run report. +# upstream. A ref confirmed deleted (`ls-remote` reports it absent) is +# skipped loudly in the run report; a transport failure reaching upstream +# blocks the push and fails the run instead — that is not a cue to edit +# the manifest. # # Example: # 1234 # keep the composer hotfix until upstream merges it diff --git a/.github/scripts/personal-staging/stage.py b/.github/scripts/personal-staging/stage.py index f229967471..90c62c0087 100644 --- a/.github/scripts/personal-staging/stage.py +++ b/.github/scripts/personal-staging/stage.py @@ -372,6 +372,10 @@ def stage( "date": datestamp, "upstream_sha": upstream_sha, "staging_sha": staging_sha, + # Pre-push remote tip: distinct from the local candidate when the + # push is blocked or a no-op, so the summary never labels an + # unpublished composition as "Staging". + "remote_staging_sha": expected_staging, "staging_only": True, "blocked": blocked, "pushed": not blocked and expected_staging != staging_sha, @@ -489,6 +493,10 @@ def notes(report: dict, signed: bool) -> str: def summarize(report: dict) -> str: if report.get("staging_only"): + # A no-op hour produces the same ~19-row skip table every time; collapse + # it to a one-line count so the few lines that matter stay visible. + # Runs that push (or that block on unreachable extras) keep full detail. + noop = not report["pushed"] and not report["blocked"] if report["blocked"]: result = ( "**NOT pushed** — extras unreachable: " @@ -498,15 +506,30 @@ def summarize(report: dict) -> str: elif report["pushed"]: result = f"pushed (changed: {', '.join(report['causes'])})" else: - result = "unchanged — push skipped" + result = ( + f"no-op: staging unchanged; {len(report['applied'])} PRs applied, " + f"{len(report['skipped'])} skipped" + ) + if report["pushed"]: + sha_rows = [f"| Staging | `{report['staging_sha']}` |"] + else: + remote = report.get("remote_staging_sha") or "(none)" + sha_rows = [ + f"| candidate (not pushed) | `{report['staging_sha']}` |", + f"| Remote staging | `{remote}` |", + ] rows = [ "## Personal staging hourly", "", "| | |", "| --- | --- |", f"| Upstream main | `{report['upstream_sha']}` |", - f"| Staging | `{report['staging_sha']}` |", + *sha_rows, f"| Result | {result} |", + ] + if noop: + return "\n".join(rows) + "\n" + rows += [ f"| Applied / skipped | {len(report['applied'])} / {len(report['skipped'])} |", ] rows += [f"| Skipped #{p['pr']} | {_skip_reason(p)} |" for p in report["skipped"]] diff --git a/.github/scripts/personal-staging/test_stage.py b/.github/scripts/personal-staging/test_stage.py index 0d3de24c72..5e1f383139 100644 --- a/.github/scripts/personal-staging/test_stage.py +++ b/.github/scripts/personal-staging/test_stage.py @@ -524,7 +524,11 @@ def test_extra_transport_failure_blocks_push_and_spares_the_manifest(env, monkey assert report["pushed"] is False # previous staging content preserved — no silent regression assert env.fork_ref("refs/heads/staging") == first["staging_sha"] - assert "NOT pushed" in stage_mod.summarize(report) + text = stage_mod.summarize(report) + assert "NOT pushed" in text + assert "candidate (not pushed)" in text + assert f"| Remote staging | `{first['staging_sha']}` |" in text + assert "| Skipped #4 |" in text def test_extra_transport_failure_fails_the_nightly_before_publishing(env, monkeypatch): @@ -597,7 +601,90 @@ def test_staging_only_noop_fast_path_skips_push(env, pushes): assert second["pushed"] is False assert second["staging_sha"] == first["staging_sha"] assert pushes == [] - assert "unchanged — push skipped" in stage_mod.summarize(second) + text = stage_mod.summarize(second) + assert "no-op: staging unchanged; 1 PRs applied, 0 skipped" in text + assert "| Skipped #" not in text + assert "candidate (not pushed)" in text + assert f"| Remote staging | `{first['staging_sha']}` |" in text + + +def test_summarize_hourly_noop_collapses_skip_detail(): + """No-op hours must not dump the full skip table — one count line only.""" + skipped = [ + {"pr": 90 + i, "branch": f"b{i}", "conflict_paths": [f"p{i}.txt"], "source": "open"} + for i in range(5) + ] + report = { + "staging_only": True, + "upstream_sha": "u" * 40, + "staging_sha": "c" * 40, + "remote_staging_sha": "c" * 40, + "pushed": False, + "blocked": [], + "causes": [], + "applied": [{"pr": i} for i in range(3)], + "skipped": skipped, + } + text = stage_mod.summarize(report) + assert "no-op: staging unchanged; 3 PRs applied, 5 skipped" in text + assert "| Skipped #" not in text + assert "candidate (not pushed)" in text + assert f"| Remote staging | `{'c' * 40}` |" in text + + +def test_summarize_hourly_push_keeps_full_skip_table(): + """A real push still renders every skip row — the detail that matters.""" + report = { + "staging_only": True, + "upstream_sha": "u" * 40, + "staging_sha": "s" * 40, + "remote_staging_sha": "r" * 40, + "pushed": True, + "blocked": [], + "causes": ["upstream HEAD"], + "applied": [{"pr": 1}], + "skipped": [ + { + "pr": 99, + "branch": "pull/99/head", + "conflict_paths": [], + "reason": stage_mod.EXTRA_MISSING, + "source": "extra", + } + ], + } + text = stage_mod.summarize(report) + assert "pushed (changed: upstream HEAD)" in text + assert "| Staging |" in text and "candidate (not pushed)" not in text + assert "| Skipped #99 |" in text and "remove from extras.txt" in text + assert "| Applied / skipped | 1 / 1 |" in text + + +def test_summarize_hourly_blocked_labels_candidate_not_staging(): + report = { + "staging_only": True, + "upstream_sha": "u" * 40, + "staging_sha": "c" * 40, + "remote_staging_sha": "r" * 40, + "pushed": False, + "blocked": [4], + "causes": [], + "applied": [], + "skipped": [ + { + "pr": 4, + "branch": "pull/4/head", + "conflict_paths": [], + "reason": stage_mod.EXTRA_FETCH_FAILED, + "source": "extra", + } + ], + } + text = stage_mod.summarize(report) + assert "NOT pushed" in text + assert "candidate (not pushed)" in text and f"| Remote staging | `{'r' * 40}` |" in text + assert "| Skipped #4 |" in text + assert "| Staging |" not in text.replace("Remote staging", "") def test_staging_only_reports_change_causes(env): @@ -651,15 +738,14 @@ def test_staging_only_cause_already_merged_pr_is_not_a_change(env): assert second["causes"] == ["upstream HEAD"] -def test_old_composition_decoded_past_any_merge_count(env, monkeypatch): - """The previous composition is decoded by walking to the real upstream - base, so a manifest longer than any PR-list bound still yields real - causes instead of degrading to 'no previous composition'.""" +def test_old_composition_decoded_past_any_merge_count(env): + """composition_of walks first-parent to the real upstream base, so a + multi-merge previous composition still yields real causes instead of + degrading to 'no previous composition'.""" extras = [env.add_pr(40 + i, f"w{i}.txt", f"{i}\n") for i in range(3)] stream = stage_mod.merge_stream([], [p["number"] for p in extras]) env.run(stream, staging_only=True) - monkeypatch.setattr(stage_mod, "PR_LIST_LIMIT", 1) env.advance_main("wmain.txt", "m\n") second = env.run(stream, staging_only=True) assert second["causes"] == ["upstream HEAD"] diff --git a/.github/workflows/personal-staging-hourly.yml b/.github/workflows/personal-staging-hourly.yml index 89e5290ac2..b74044ec60 100644 --- a/.github/workflows/personal-staging-hourly.yml +++ b/.github/workflows/personal-staging-hourly.yml @@ -32,8 +32,9 @@ jobs: # upstream, but a content conflict only warns (abort + summary + exit 0) — # the compose job stacks from upstream HEAD and never reads fork main, # and hourly noise for a conflict the nightly already reports helps nobody. - # Only conflicts are soft: a merge or push failing for any other reason - # still fails the job. + # Soft-fail is classified, not blanket: only a genuine content conflict + # (unmerged paths) or a confirmed stale-ref push rejection soft-fails; + # auth, permissions, transport, or any other merge/push failure stays loud. sync-main: # Fork-only, and main-only for the same accident-prevention reason as the # compose job below: a dispatched ref runs its own workflow copy, so this @@ -81,14 +82,35 @@ jobs: soft_fail "merge conflict with upstream main — resolve manually" "$paths" } + # Soft-fail ONLY a confirmed non-fast-forward / stale-ref rejection + # (! [rejected] + non-fast-forward|fetch first|stale info). Auth, + # permissions, or transport failures must fail the step — same + # classification discipline as merge_or_soft_fail above. + is_stale_ref_rejection() { + printf '%s\n' "$1" | grep -q '! \[rejected\]' \ + && printf '%s\n' "$1" | grep -qiE 'non-fast-forward|fetch first|stale info' + } + merge_or_soft_fail FETCH_HEAD - if ! git push origin main; then + if ! push_err=$(git push origin main 2>&1); then + printf '%s\n' "$push_err" >&2 + if ! is_stale_ref_rejection "$push_err"; then + echo "::error::sync-main: git push failed (not a stale-ref race)" + exit 1 + fi # Lost a race (the nightly, or a human push). Integrate and retry - # once; never force-push main, and never fail the hourly for it. + # once; never force-push main. git fetch origin main merge_or_soft_fail FETCH_HEAD - git push origin main \ - || soft_fail "could not push fork main" "Lost the push race twice; the next hourly run retries." + if ! push_err=$(git push origin main 2>&1); then + printf '%s\n' "$push_err" >&2 + if is_stale_ref_rejection "$push_err"; then + soft_fail "could not push fork main" \ + "Lost the push race twice; the next hourly run retries." + fi + echo "::error::sync-main: git push failed after retry (not a stale-ref race)" + exit 1 + fi fi # Secretless test gate: `uv run` installs third-party dev deps, so this job From 74968417d2ed1fbc3b2fc9bbdd40fccfb95d9ef0 Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Mon, 10 Aug 2026 21:40:08 -0700 Subject: [PATCH 5/6] fix(ci): classify hourly main push via porcelain, not prose Parse git push --porcelain status lines in stage.py so only a confirmed non-fast-forward/stale race on refs/heads/main soft-fails; [remote rejected], auth, and transport stay red. Align extras.txt transport wording with the README's hourly-vs-nightly split. Signed-off-by: Bryan Li --- .github/scripts/personal-staging/extras.txt | 4 +- .github/scripts/personal-staging/stage.py | 49 ++++++++ .../personal-staging/test_push_classify.py | 117 ++++++++++++++++++ .github/workflows/personal-staging-hourly.yml | 26 ++-- 4 files changed, 183 insertions(+), 13 deletions(-) create mode 100644 .github/scripts/personal-staging/test_push_classify.py diff --git a/.github/scripts/personal-staging/extras.txt b/.github/scripts/personal-staging/extras.txt index 405f08e2e1..2df098407b 100644 --- a/.github/scripts/personal-staging/extras.txt +++ b/.github/scripts/personal-staging/extras.txt @@ -5,8 +5,8 @@ # `#` comments are ignored. Remove an entry once the change lands # upstream. A ref confirmed deleted (`ls-remote` reports it absent) is # skipped loudly in the run report; a transport failure reaching upstream -# blocks the push and fails the run instead — that is not a cue to edit -# the manifest. +# blocks the hourly push (run stays green with a warning); fails the +# nightly run — that is not a cue to edit the manifest. # # Example: # 1234 # keep the composer hotfix until upstream merges it diff --git a/.github/scripts/personal-staging/stage.py b/.github/scripts/personal-staging/stage.py index 90c62c0087..d0f631f035 100644 --- a/.github/scripts/personal-staging/stage.py +++ b/.github/scripts/personal-staging/stage.py @@ -65,6 +65,42 @@ class StageError(RuntimeError): pass +# Soft-failable ``git push --porcelain`` rejection reasons for refs/heads/main. +# Distinct from ``[remote rejected]`` (hooks/auth policy), which must stay loud. +_STALE_REF_REASONS = frozenset({"non-fast-forward", "fetch first", "stale info"}) + + +def is_stale_ref_push_rejection(porcelain: str, *, ref: str = "refs/heads/main") -> bool: + """True iff ``git push --porcelain`` shows a confirmed stale-ref race on ``ref``. + + Classifies from porcelain status lines only — never from free-form stderr + prose — so a pre-receive hook that prints ``fetch first`` while emitting + ``[remote rejected]`` cannot soft-fail. Soft-fail only when the target + ref's line is flag ``!`` with summary ``[rejected]`` (not + ``[remote rejected]``) and a non-fast-forward / fetch-first / stale-info + reason. No matching status line (auth, transport, empty output) is a + hard failure. + """ + for line in porcelain.splitlines(): + if "\t" not in line: + continue + # Status lines are ``\\t:\\t``; ignore To/Done + # and any hook noise that lacks this shape. + flag, *fields = line.split("\t") + if len(flag) != 1 or len(fields) < 2 or ":" not in fields[0]: + continue + dst = fields[0].rsplit(":", 1)[-1] + if dst != ref: + continue + summary = fields[1] + # Soft-fail ONLY canonical local rejection — never [remote rejected]. + if flag != "!" or not summary.startswith("[rejected]"): + return False + m = re.fullmatch(r"\[rejected\] \((.+)\)", summary) + return bool(m and m.group(1) in _STALE_REF_REASONS) + return False + + def git( cwd: str | Path, *args: str, check: bool = True, env: dict[str, str] | None = None ) -> subprocess.CompletedProcess: @@ -577,8 +613,21 @@ def main(argv: list[str] | None = None) -> int: p_notes.add_argument("--report", default="merge-report.json") p_notes.add_argument("--signed", choices=["true", "false"], required=True) + p_stale = sub.add_parser( + "is-stale-ref-rejection", + help="exit 0 iff stdin is git-push --porcelain for a main stale-ref race", + ) + p_stale.add_argument( + "--ref", + default="refs/heads/main", + help="destination ref to classify (default: refs/heads/main)", + ) + args = parser.parse_args(argv) + if args.cmd == "is-stale-ref-rejection": + return 0 if is_stale_ref_push_rejection(sys.stdin.read(), ref=args.ref) else 1 + if args.cmd == "notes": report = json.loads(Path(args.report).read_text()) sys.stdout.write(notes(report, signed=args.signed == "true")) diff --git a/.github/scripts/personal-staging/test_push_classify.py b/.github/scripts/personal-staging/test_push_classify.py new file mode 100644 index 0000000000..624d7f4920 --- /dev/null +++ b/.github/scripts/personal-staging/test_push_classify.py @@ -0,0 +1,117 @@ +"""Unit tests for hourly sync-main push soft-fail classification. + +Covers ``is_stale_ref_push_rejection``: the safety-critical path that decides +whether a failed ``git push --porcelain origin main`` is a benign stale-ref +race (soft-fail) or everything else (hard-fail the job). +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import pytest +import stage as stage_mod + +DIR = Path(__file__).resolve().parent + + +def _porcelain(*status_lines: str, url: str = "/tmp/fork.git") -> str: + """Assemble a realistic porcelain transcript (To/status/Done).""" + body = "\n".join(status_lines) + return f"To {url}\n{body}\nDone\n" + + +def test_canonical_non_fast_forward_is_stale_ref(): + text = _porcelain("!\trefs/heads/main:refs/heads/main\t[rejected] (non-fast-forward)") + assert stage_mod.is_stale_ref_push_rejection(text) is True + + +def test_fetch_first_and_stale_info_are_stale_ref(): + for reason in ("fetch first", "stale info"): + text = _porcelain(f"!\tHEAD:refs/heads/main\t[rejected] ({reason})") + assert stage_mod.is_stale_ref_push_rejection(text) is True, reason + + +def test_remote_rejected_by_pre_receive_hook_is_hard_fail(): + text = _porcelain( + "!\trefs/heads/main:refs/heads/main\t[remote rejected] (pre-receive hook declined)" + ) + assert stage_mod.is_stale_ref_push_rejection(text) is False + + +def test_hook_prose_fetch_first_with_remote_rejected_is_hard_fail(): + # Exact false-positive from the tribunal: hook reason text says + # "fetch first" but the structured summary is [remote rejected]. + text = _porcelain("!\trefs/heads/main:refs/heads/main\t[remote rejected] (fetch first)") + assert stage_mod.is_stale_ref_push_rejection(text) is False + + +def test_auth_permission_failure_with_no_status_line_is_hard_fail(): + # Transport/auth often leaves porcelain stdout empty (prose on stderr only). + assert stage_mod.is_stale_ref_push_rejection("") is False + assert ( + stage_mod.is_stale_ref_push_rejection( + "remote: Permission to btli/omnigent.git denied to github-actions[bot].\n" + "fatal: unable to access 'https://github.com/btli/omnigent.git/': " + "The requested URL returned error: 403\n" + ) + is False + ) + + +def test_transport_failure_is_hard_fail(): + assert ( + stage_mod.is_stale_ref_push_rejection( + "fatal: unable to access 'https://github.com/btli/omnigent.git/': " + "Could not resolve host: github.com\n" + ) + is False + ) + + +def test_unrelated_rejected_markers_in_prose_do_not_soft_fail(): + # Independent greps over a combined transcript would false-positive here. + prose = ( + "remote: tip: if rejected, fetch first before retrying\n" + "!\trefs/heads/main:refs/heads/main\t[remote rejected] (hook declined)\n" + "error: failed to push some refs\n" + ) + assert stage_mod.is_stale_ref_push_rejection(prose) is False + + +def test_cli_is_stale_ref_rejection_exit_codes(): + script = str(DIR / "stage.py") + good = _porcelain("!\trefs/heads/main:refs/heads/main\t[rejected] (non-fast-forward)") + bad = _porcelain( + "!\trefs/heads/main:refs/heads/main\t[remote rejected] (pre-receive hook declined)" + ) + ok = subprocess.run( + [sys.executable, script, "is-stale-ref-rejection"], + input=good, + text=True, + capture_output=True, + check=False, + ) + assert ok.returncode == 0, ok.stderr + hard = subprocess.run( + [sys.executable, script, "is-stale-ref-rejection"], + input=bad, + text=True, + capture_output=True, + check=False, + ) + assert hard.returncode == 1, hard.stderr + + +@pytest.mark.parametrize( + "line", + [ + "!\trefs/heads/other:refs/heads/other\t[rejected] (non-fast-forward)", + "!\trefs/heads/main:refs/heads/main\t[rejected] (already exists)", + "=\trefs/heads/main:refs/heads/main\t[up to date]", + ], +) +def test_non_main_or_other_reasons_are_hard_fail(line: str): + assert stage_mod.is_stale_ref_push_rejection(_porcelain(line)) is False diff --git a/.github/workflows/personal-staging-hourly.yml b/.github/workflows/personal-staging-hourly.yml index b74044ec60..be9da235a2 100644 --- a/.github/workflows/personal-staging-hourly.yml +++ b/.github/workflows/personal-staging-hourly.yml @@ -83,18 +83,21 @@ jobs: } # Soft-fail ONLY a confirmed non-fast-forward / stale-ref rejection - # (! [rejected] + non-fast-forward|fetch first|stale info). Auth, - # permissions, or transport failures must fail the step — same - # classification discipline as merge_or_soft_fail above. + # on refs/heads/main, classified from `git push --porcelain` status + # lines (stage.py is-stale-ref-rejection) — never from free-form + # stderr. Auth, permissions, transport, or [remote rejected] hooks + # must fail the step — same discipline as merge_or_soft_fail above. is_stale_ref_rejection() { - printf '%s\n' "$1" | grep -q '! \[rejected\]' \ - && printf '%s\n' "$1" | grep -qiE 'non-fast-forward|fetch first|stale info' + printf '%s\n' "$1" | python3 .github/scripts/personal-staging/stage.py \ + is-stale-ref-rejection } merge_or_soft_fail FETCH_HEAD - if ! push_err=$(git push origin main 2>&1); then - printf '%s\n' "$push_err" >&2 - if ! is_stale_ref_rejection "$push_err"; then + # Porcelain status → stdout; prose errors → stderr. Classify stdout only. + if ! push_out=$(git push --porcelain origin main 2>push_err.txt); then + cat push_err.txt >&2 + printf '%s\n' "$push_out" >&2 + if ! is_stale_ref_rejection "$push_out"; then echo "::error::sync-main: git push failed (not a stale-ref race)" exit 1 fi @@ -102,9 +105,10 @@ jobs: # once; never force-push main. git fetch origin main merge_or_soft_fail FETCH_HEAD - if ! push_err=$(git push origin main 2>&1); then - printf '%s\n' "$push_err" >&2 - if is_stale_ref_rejection "$push_err"; then + if ! push_out=$(git push --porcelain origin main 2>push_err.txt); then + cat push_err.txt >&2 + printf '%s\n' "$push_out" >&2 + if is_stale_ref_rejection "$push_out"; then soft_fail "could not push fork main" \ "Lost the push race twice; the next hourly run retries." fi From 75fce0b33b06cd03aaf5b0407a9cdf04e2b3e010 Mon Sep 17 00:00:00 2001 From: Bryan Li Date: Tue, 11 Aug 2026 06:35:45 -0700 Subject: [PATCH 6/6] test(ci): pin is_stale_ref_push_rejection flag != "!" guard A [rejected] (non-fast-forward) porcelain line with a non-'!' flag must hard-fail; existing cases all used '!' so the flag guard was unpinned. Signed-off-by: Bryan Li --- .github/scripts/personal-staging/test_push_classify.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/scripts/personal-staging/test_push_classify.py b/.github/scripts/personal-staging/test_push_classify.py index 624d7f4920..09079d9fce 100644 --- a/.github/scripts/personal-staging/test_push_classify.py +++ b/.github/scripts/personal-staging/test_push_classify.py @@ -111,6 +111,8 @@ def test_cli_is_stale_ref_rejection_exit_codes(): "!\trefs/heads/other:refs/heads/other\t[rejected] (non-fast-forward)", "!\trefs/heads/main:refs/heads/main\t[rejected] (already exists)", "=\trefs/heads/main:refs/heads/main\t[up to date]", + # Non-'!' flag with a [rejected] summary must hard-fail (not soft-fail). + " \trefs/heads/main:refs/heads/main\t[rejected] (non-fast-forward)", ], ) def test_non_main_or_other_reasons_are_hard_fail(line: str):