fix(github-status-comments): merge entries across concurrent workflows#1108
Open
gregmagolan wants to merge 4 commits into
Open
fix(github-status-comments): merge entries across concurrent workflows#1108gregmagolan wants to merge 4 commits into
gregmagolan wants to merge 4 commits into
Conversation
✨ Aspect Workflows Tasks📅 Fri May 22 00:22:41 UTC 2026
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92b8bb7dc9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
gregmagolan
force-pushed
the
fix-ghsc-cross-workflow-merge
branch
8 times, most recently
from
6062525 to
13d3f26
Compare
gregmagolan
force-pushed
the
fix-ghsc-cross-workflow-merge
branch
from
13d3f26 to
c755f86
Compare
Add a fast-path before the comment GET in `_publish`: when the swarm-local lease holds, the combined cross-workflow lease (which is `max(state.last_upsert_epoch, swarm-local)`) also holds, so we wouldn't PATCH this cycle either way. Short-circuit before the GET and the per-sibling-workflow `list_run_artifacts` refresh. Effect: N swarm members → 1 GET per cycle (the leaseholder's) instead of N. Sole-swarm and multi-workflow PRs both benefit. Correctness: skipping cross-workflow discovery for one cycle is harmless — any sibling workflow's data we missed will land in the next cycle when our swarm-local lease relaxes and we run the full path. Terminal updates (`final=True`) always run the full path so the final verdict picks up the freshest cross-workflow state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the leaseholder GETs the comment and parses the state block, bump our own `_last_pr_comment_upsert_epoch_s` to `max(state.last_upsert_epoch_s, our prior stamp)`. The next `_publish_self` carries the higher value on our artifact, so sibling swarm members' `_collect_sibling_entries` sees it via `_max_recent_upsert_epoch`. Effect: their swarm-local fast-path can short-circuit when a sibling-workflow recently published, even before our own swarm has done its own first PATCH. Only the leaseholder per swarm ever pays the cross-workflow GET cost. Field semantics widened from "epoch of our last publish" to "high-water mark of any PR-comment upsert we're aware of (ours or observed via the state block)". The lease decision math is unchanged — both meanings contribute to "someone published at T", which is what the lease cares about. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trim three comment blocks in `_publish` (swarm lease + fast path, combined lease check, two-renders) from ~45 lines to ~20. Update the file docstring to mention cross-workflow refresh and the GET role of the `prs.write` token. Replace the stale `_merge_entries` reference in `_own_payload` with the actual field roles (`_merge_state` snapshot bucketing + display-only `workflow_name`). Reword `_collect_other_workflow_entries` docstring so the race-elimination claim sits where it's accurate (the system as a whole, not this function alone). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gregmagolan
force-pushed
the
fix-ghsc-cross-workflow-merge
branch
from
c90edd4 to
551864f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When two GitHub Actions workflows run in parallel on the same PR (e.g.
ci-github-runners.yaml+ci-workflows-runners.yaml), each workflow's status-comment artifact swarm only sees its ownGITHUB_RUN_ID's artifacts (thelist_run_artifactsendpoint is workflow-scoped). Both swarms PATCH the single PR-marker comment using only their own task entries, so each upsert clobbers the other workflow's rows.The PR comment is now the canonical cross-workflow source of truth. A hidden state block at the bottom of the comment carries:
head_shalast_upsert_epoch_sworkflow_runs[{run_id, workflow_name}, ...]— one slot per workflow active on this PR.entriesEach publish cycle does:
list_run_artifacts(own_run_id)+ download → fresh OWN entries.run_idin the state block:list_run_artifacts(other_run_id)+ download — fresh sibling-workflow data. On API failure or empty listing (e.g. artifact TTL expiry), fall back to the comment's snapshot for that workflow.Why this eliminates the race: each writer only writes its own slot (its row in
workflow_runs+ its own entries). Sibling slots are passed through. Two concurrent writers in different workflows touch disjoint slots → no cross-workflow data race. Intra-workflow writers see the samelist_run_artifacts(own)output → they all write identical data, so an intra-workflow race is benign.TTL resilience: the
entriessnapshot in the state block survives past artifact retention (default 30min), so a workflow that finished hours ago still renders correctly.Backward compat: writers on older aspect-cli versions don't emit the state block. New-version writers see
parsed == Noneand fall back to swarm-only rendering for that cycle. Once both workflows ship the new version, the merge path kicks in. The visible markdown layout is unchanged — only the hidden HTML-comment block is added.API-cost optimizations:
max(state, swarm-local)) also holds, so we'd skip the PATCH either way. Short-circuit before the GET and the per-sibling-workflowlist_run_artifactsrefresh. Only the leaseholder per swarm pays the cross-workflow GET cost per cycle.final=Truealways runs the full path so terminal verdicts pick up the freshest cross-workflow state._last_pr_comment_upsert_epoch_stomax(state.last_upsert_epoch_s, our prior stamp). The next_publish_selfcarries the higher value on our artifact, so sibling swarm members'_max_recent_upsert_epochthen sees it and their fast-path skips the GET too — propagating the cross-workflow lease across the swarm without each member needing its own GET.A new
github.issues.get_commentAPI reads a single comment by id to avoid re-listing the PR's comments every cycle.This PR also wires up a new
ci-github-runners.yamlworkflow that exercises the multi-workflow path against GitHub-hosted ephemeral runners. Those runners don't have remote-cache access, so thepre-buildself-hosted job uploads the just-builtaspect-clibinary as an artifact and a sharedinstall-prebuilt-aspect-clicomposite action (marked internal-only in its description) downloads it into$HOME/.local/binfor each ephemeral job.Changes are visible to end-users: yes
Test plan
aspect dev test-pr-comment-snapshots(20 scenarios + 17 unit checks) passes. Checks cover serialize/parse round-trip, malformed payload, missing-terminator payload, wrong-shape field normalization, non-dict row filtering, merge first-writer, two-workflows-with-fresh-refresh, snapshot-fallback when refresh missing, own-slot replacement, stale-push drop, orphan-snapshot-entry drop,workflow_runssorted,entriessorted, render-body state-block presence/absence.aspect tests axlpasses — no regression elsewhere.ci-github-runners.yamlworkflow exercises the multi-workflow path end-to-end against GitHub-hosted ephemeral runners.