Skip to content

Own dashboard execution with durable queue leases - #441

Merged
trask merged 7 commits into
mainfrom
trask-dashboard-execution-ownership
Oct 8, 2026
Merged

trask merged 7 commits into
mainfrom
trask-dashboard-execution-ownership

Conversation

@trask

@trask trask commented Oct 2, 2026

Copy link
Copy Markdown
Member

Move dashboard execution under the existing queue leases so a run waiting for a runner cannot block its replacement. Stable and canary work acknowledge success only after calculation, persistence, delivery, publication, and receipt updates succeed.

Execution stays opt-in. After the pause-and-retire cutover, set the GitHub repository variable and Netlify environment to:

PR_DASHBOARD_EXECUTION_MODE=owned

legacy remains the default. paused accepts work without starting new execution.

  • PR/head refreshes, draft opens, bounded resumable backfills, and reminder write sweeps share the processor. Reminder dry runs stay read-only.
  • Stable code and dependencies load from promoted immutable pins; canaries use the triggering main commit. Repository policy stays current in both channels.
  • Enqueue summaries report acceptance, not completion, with an item key, generation, and request ID for progress queries.

Lease loss stops subprocess trees. Failures retry combined work with existing accepted state and delivery receipts. Delivery is not exactly once: an external call can succeed before its receipt persists.

Deployment and rollback instructions cover retiring legacy writers, draining control items before rollback, and preserving queued refreshes. No release promotion or production-mode change is included. This change is independent of #440.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 2, 2026 19:03
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-10-08 15:58 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

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.

Copilot review overview

🟡 Changes recommended

Promotion tests will fail after the next pin update, and repository configuration changes do not trigger the required Netlify redeployment.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Moves dashboard processing behind durable queue leases while preserving opt-in legacy, paused, and owned execution modes.

Changes:

  • Adds owned queue execution, lease fencing, retries, and immutable code loading.
  • Queues backfills and reminder sweeps and exposes request progress.
  • Documents migration, rollback, and delivery guarantees.
File Description
pull-request-dashboard/​README.md Documents owned execution.
.github/​workflows/​pull-request-dashboard.yml Adds enqueue and status paths.
.github/​workflows/​pull-request-dashboard-repo.yml Rejects unowned writers.
.github/​workflows/​pull-request-dashboard-refresh-author-nudges.yml Queues write sweeps.
.github/​workflows/​pull-request-dashboard-drain.yml Adds owned drain job.
.github/​workflows/​pull-request-dashboard-deploy-webhook.yml Deploys execution mode.
.github/​scripts/​pull-request-dashboard/​WEBHOOK_SETUP.md Documents deployment and rollback.
.github/​scripts/​pull-request-dashboard/​test_rollout.py Updates rollout assertions.
.github/​scripts/​pull-request-dashboard/​test_process_queue_batch.py Tests inactive delivery retries.
.github/​scripts/​pull-request-dashboard/​test_owned_execution.py Tests owned execution behavior.
.github/​scripts/​pull-request-dashboard/​test_github_webhook.mjs Tests owned and paused webhooks.
.github/​scripts/​pull-request-dashboard/​test_github_oidc.mjs Tests enqueue identities.
.github/​scripts/​pull-request-dashboard/​test_drain_queue.py Tightens lease-monitor mocking.
.github/​scripts/​pull-request-dashboard/​test_dashboard_queue.mjs Tests queue leases and progress.
.github/​scripts/​pull-request-dashboard/​test_dashboard_queue_worker.mjs Tests worker execution modes.
.github/​scripts/​pull-request-dashboard/​process_queue_batch.py Combines queue processing stages.
.github/​scripts/​pull-request-dashboard/​netlify/​lib/​github-oidc.mjs Authorizes enqueue workflows.
.github/​scripts/​pull-request-dashboard/​netlify/​lib/​execution-mode.mjs Validates execution mode.
.github/​scripts/​pull-request-dashboard/​netlify/​lib/​dashboard-queue.mjs Extends queue ownership and status.
.github/​scripts/​pull-request-dashboard/​netlify/​functions/​github-webhook.mjs Routes webhooks by execution mode.
.github/​scripts/​pull-request-dashboard/​netlify/​functions/​dashboard-workflow-watchdog.mjs Narrows owned-mode monitoring.
.github/​scripts/​pull-request-dashboard/​netlify/​functions/​dashboard-queue-worker.mjs Adds enqueue and status APIs.
.github/​scripts/​pull-request-dashboard/​netlify/​functions/​dashboard-queue-recover.mjs Recovers and redispatches leases.
.github/​scripts/​pull-request-dashboard/​execution_process.py Monitors subprocess trees.
.github/​scripts/​pull-request-dashboard/​execution_code.py Loads immutable execution bundles.
.github/​scripts/​pull-request-dashboard/​enqueue_dashboard.py Enqueues and reports progress.
.github/​scripts/​pull-request-dashboard/​drain_queue.py Implements owned draining.
.github/​scripts/​pull-request-dashboard/​dashboard.py Updates execution documentation.
.github/​scripts/​pull-request-dashboard/​CONTEXT.md Describes ownership architecture.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 3, 2026 02:38

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.

Copilot review overview

🟡 Changes recommended

Dependency installation exposes production credentials, and the reminder workflow unnecessarily broadens token permissions.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid broadening pull request permissions to write

.github/​workflows/​pull-request-dashboard-refresh-author-nudges.yml:112

The non-dry sweep only reads pull requests and mutates issue comments (refresh_author_nudges.py uses the issues comment API plus comment minimization), and this same path previously ran with pull-requests: read. Granting pull-requests: write unnecessarily broadens the installation token; keep this permission read-only while toggling only issues.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 3, 2026 17:12

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.

Copilot review overview

🔵 Needs a closer look

Delayed retries can remain undispatched until hourly recovery, and one validation message omits the new outcome.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Backoff retries can remain idle until hourly recovery

.github/​scripts/​pull-request-dashboard/​process_queue_batch.py:689

Adding a backoff here makes the item non-runnable until notBefore, but the drain excludes retry items and finishDispatcher() only dispatches a successor when an item is runnable at that moment. With recovery configured hourly, a 60-second retry can therefore remain idle for nearly an hour unless unrelated work arrives. Add a wake-up mechanism for the earliest notBefore (or keep retries immediately runnable until one exists).

Low severity Validation error omits newly supported continue outcome

.github/​scripts/​pull-request-dashboard/​netlify/​functions/​dashboard-queue-worker.mjs:8

continue is now accepted, but acknowledgmentOutcome() still tells clients that only success, retry, and dead are valid. Include continue in that validation error so malformed requests receive accurate guidance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 3, 2026 17:57

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.

Copilot review overview

🔵 Needs a closer look

The stateful lease fencing, immutable-code loading, and production cutover span multiple failure domains requiring final human validation.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

@trask
trask requested a balanced review from Copilot October 4, 2026 01:45

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.

Copilot review overview

🟡 Changes recommended

Failed code preparation can poison the same immutable ref for all later waves in a drain.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

…e ref load

Copilot comment:

A failed first preparation leaves this directory behind, but the ref is not cached until all installs finish. If later waves in the same drain need the same ref (the configured repository set exceeds the 16-item wave limit), `load()` reaches this line again and deterministically raises `FileExistsError`, so one transient fetch/install failure poisons that code ref for the rest of the drain. Allow retrying the existing immutable-ref staging directory (or clean it before retrying).

Analysis: Remove any leftover staging directory before preparing a ref, so retries after a failed load succeed.

Upsides: One transient failure no longer poisons the ref for later waves.

Downsides: Discards partial staging contents on retry.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 4, 2026 02:15

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.

Copilot review overview

🔵 Needs a closer look

The production queue, lease, delivery, and migration changes require final human validation despite comprehensive test coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@trask
trask requested a balanced review from Copilot October 6, 2026 17:39

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.

Copilot review overview

🔵 Needs a closer look

The production execution migration is broad and the processor summary currently misreports continuation outcomes as successes.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

…in process_claims summary

Copilot comment:

Adding `continue` as a nonterminal outcome leaves `process_claims()`'s summary inconsistent: its `successes` calculation subtracts only retries and dead letters, so every bounded backfill slice acknowledged as `continue` is reported as a success. Track/subtract continued outcomes (and ideally expose a `continues` count) so processor summaries do not imply completion before full publication.

Copilot comment:

<!-- ccr-overview-v2 -->

## Copilot review overview

### 🔵 Needs a closer look

The production execution migration is broad and the processor summary currently misreports continuation outcomes as successes.

**Review effort:** Balanced  
**Findings:** 1 <picture><source media="(prefers-color-scheme: dark)" srcset="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-light.svg"><img src="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-light.png" alt="Medium severity" width="62" height="18" align="texttop"></picture>

<details open>
<summary><strong>Open (1)</strong></summary>

- <picture><source media="(prefers-color-scheme: dark)" srcset="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-light.svg"><img src="https://github.githubassets.com/static/images/icons/copilot-code-review/medium-v2-light.png" alt="Medium severity" width="62" height="18" align="texttop"></picture> [Count continued outcomes separately in process_claims summaries](#discussion_r4198617138) · New
</details>

Analysis: process_claims subtracted only retries and dead letters, so acknowledged continue outcomes were counted as successes. Subtract them and expose a continues count.

Upsides: Summary no longer implies completion for bounded slices awaiting further publication.

Downsides: Adds a new summary key.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@trask
trask requested a balanced review from Copilot October 6, 2026 18:13

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.

Copilot review overview

🔵 Needs a closer look

The distributed lease, retry, deployment, and rollback changes affect production execution ownership across GitHub Actions and Netlify.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@trask
trask marked this pull request as ready for review October 7, 2026 21:32
@trask
trask requested a review from a team as a code owner October 7, 2026 21:32
@trask
trask requested a review from mx-psi October 7, 2026 21:32
@trask

trask commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

…tion-ownership

# Conflicts:
#	.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md

Co-authored-by: trask <218610+trask@users.noreply.github.com>

Copilot AI commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in a58c01b by merging the latest main, preserving both owned-mode guidance and legacy publisher-recovery documentation. Validation passed: 920 Python tests (one skipped), 163 JavaScript tests, and CodeQL with no alerts.

@trask
trask enabled auto-merge (squash) October 8, 2026 15:56
@trask
trask merged commit c717adb into main Oct 8, 2026
6 checks passed
@trask
trask deleted the trask-dashboard-execution-ownership branch October 8, 2026 15:57
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.

4 participants