Skip to content

Reject stale dashboard delivery workers#141

Closed
trask wants to merge 1 commit into
open-telemetry:mainfrom
trask:fix/reject-stale-delivery-workers
Closed

Reject stale dashboard delivery workers#141
trask wants to merge 1 commit into
open-telemetry:mainfrom
trask:fix/reject-stale-delivery-workers

Conversation

@trask

@trask trask commented Jul 22, 2026

Copy link
Copy Markdown
Member

Persist the active dashboard delivery release as its activation run ID and immutable shared-workflows SHA. Workers from the active SHA remain valid, newer runs can activate a new SHA, and queued workers from older SHAs exit before delivery; the same guard suppresses the downstream dashboard-issue write.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds a “worker identity” guard to dashboard delivery so only the active release (run ID + workflow SHA) performs delivery and publishes the dashboard issue, preventing stale queued workers from writing state.

Changes:

  • Pass worker run metadata (run ID + SHA) into the delivery script and emit a steps.delivery.outputs.active output.
  • Introduce a new delivery-worker-state.json state file with claim_delivery_worker() to accept/reject workers.
  • Add unit tests covering worker claiming / stale-worker behavior and GITHUB_OUTPUT reporting.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/workflows/pull-request-dashboard-repo.yml Passes worker identity into delivery and gates “Publish dashboard issue” on an active output.
.github/scripts/pull-request-dashboard/state.py Adds delivery-worker state file + load/save helpers and claim_delivery_worker() logic.
.github/scripts/pull-request-dashboard/delivery.py Claims worker before delivery; writes active=true/false to GITHUB_OUTPUT.
.github/scripts/pull-request-dashboard/test_state.py Adds tests ensuring worker identity only advances and persists expected state.
.github/scripts/pull-request-dashboard/test_delivery.py Adds test ensuring stale workers skip delivery and emit active=false.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +219 to +220
WORKER_RUN_ID: ${{ github.run_id }}
WORKER_SHA: ${{ github.sha }}
Comment on lines +227 to +241
def claim_delivery_worker(worker_run_id: int, worker_sha: str) -> bool:
if worker_run_id <= 0:
raise ValueError("worker run ID must be positive")
if not worker_sha.strip():
raise ValueError("worker SHA must not be empty")

state = load_delivery_worker_state()
active_run_id = state["active_run_id"]
if state["active_sha"] == worker_sha:
return True
if active_run_id >= worker_run_id:
return False

save_delivery_worker_state({"active_run_id": worker_run_id, "active_sha": worker_sha})
return True
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 22, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-22 22:08:13 UTC.

  • Status: Closed.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected.

@trask

trask commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Superseded by a replacement that uses an explicit dashboard delivery revision instead of treating every shared-workflows SHA as a dashboard release.

@trask trask closed this Jul 22, 2026
@trask

trask commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Replacement: #144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants