Repository navigation
Finalize work-queue workers inline and recover stranded runs - #69790
Conversation
Reconcile exact bound worker attempts on completion without relaunching work or changing independent delivery barriers. Preserve repository action pins for workers-only event generation and cover native identity, retries, and completed siblings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer was cancelled during the skills-based review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
❌ Test Quality Sentinel was cancelled during test quality analysis. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
❌ Ponytail Reviewer was cancelled. Please review the logs for details. Ponytail ReviewerOutcome: The agent emitted a no-op output, but successful task completion is unconfirmed. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
❌ Design Decision Gate 🏗️ was cancelled during design decision gate check. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🧪 Test Quality Sentinel Report✅ Test Quality Score: 88/100 — Excellent
📊 Metrics (14 tests)
✅ Behavioral Coverage HighlightsExcellent signals:
Minor notes:
Verdict
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 88/100. 14% implementation tests (threshold: 30%). Excellent behavioral coverage with no violations.
🧪 Test quality analysis by Test Quality Sentinel · copilot · haiku45 · 19.7 AIC · ⌖ 8.59 AIC · ⊞ 9.2K
Comment /review to run again
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (145 new lines in 📄 Draft ADR committed: Decision captured: Release work queue reservations through a generated
📋 What to do next
After the PR body links an ADR, this gate will run again and check that the implementation matches the decision. ❓ Why ADRs MatterADRs keep a permanent, searchable record of why the codebase looks the way it does. Future contributors, including your future self, will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR is complete only when it has these four sections:
ADRs are stored in
|
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
There was a problem hiding this comment.
REQUEST_CHANGES: the new terminal-cleanup path still has three correctness/reliability holes that can strand reservations.
### Blocking themes
- self-trigger avoidance is keyed to the mutable display name
Agentic Workqueue, so a real worker with that name loses cleanup entirely workflow_run.workflowsis keyed to current names only, so worker renames can skip completion cleanup for older in-flight runs- the JS reconciler drops completed runs as
unboundinstead of attempting a bounded authenticated recovery path
Static review only; I did not execute the code.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 95.8 AIC · ⌖ 6.33 AIC · ⊞ 12.7K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd to the new workflow_run terminal-reservation reconciler (reconcile_work_queue_run.cjs) and its generator (work_queue_event_workflow.go); also used /codebase-design on the generator refactor.
📋 Key Themes & Highlights
Key Themes
- The core fix (independent re-authentication of the completed native run before releasing a reservation) is sound and well covered by the 123-line test suite, including the mismatch/rejection matrix (
revision,principal,repository,correlation,attempt,publisher,unavailable). - One new branch in
reconcile_work_queue_run.cjs— thework_queue_run_binding_ambiguousguard for multiple dispatches bound to the same run — has no regression test (see inline comment). - Minor maintainability nit: the
"Agentic Workqueue"self-reference string is duplicated acrossWorkQueueEventsEnabledandGenerateWorkQueueEventWorkflow, plus implicitly tied to the YAMLname:literal (see inline comment).
Positive Highlights
- ✅ Clean separation between
writeWorkQueueIssueEventsJobandwriteWorkQueueTerminationJob, each independently gated byevent_namechecks — good defense against cross-trigger confusion. - ✅ The reconciler correctly refuses to infer terminal state from the webhook payload alone, re-verifying via the GitHub API (
does not infer terminal state from the webhook alonetest) — textbook/diagnosing-bugsroot-cause handling rather than trusting untrusted event data. - ✅ Idempotency is tested (
main(options)called twice with no extra transactions), and retry-exhaustion vs. silent-reopen is explicitly covered. - ✅ Docs updated in the same PR to describe the new automatic cleanup and fallback path.
No blocking issues found; the two inline suggestions are small, non-blocking improvements.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 106.4 AIC · ⌖ 15 AIC · ⊞ 10.1K
Comment /matt to run again
There was a problem hiding this comment.
🔵 Needs a closer look
Privileged queue-state writes need human security review, and a trigger-matching defect remains unresolved.
1 open issue
What changed in this PR
Adds automatic terminal-reservation cleanup for gh-aw queue workers while preserving completed claims and independent delivery verification.
Changes:
- Generate completion handlers even without Issue mirroring.
- Reconcile exact run bindings using existing retry rules.
- Add regression coverage and document cleanup behavior.
| File | Description |
|---|---|
pkg/workflow/work_queue_event_workflow.go |
Generate separate Issue and worker-completion handlers. |
pkg/workflow/work_queue_event_workflow_test.go |
Test worker selection and event separation. |
pkg/cli/compile_post_processing.go |
Load repository configuration for workers-only generation. |
pkg/cli/compile_post_processing_work_queue_test.go |
Cover workers-only configuration loading. |
docs/src/content/docs/reference/work-queue.md |
Document automatic cleanup and fallback reconciliation. |
actions/setup/js/reconcile_work_queue_run.test.cjs |
Cover binding checks, retries, and delivery preservation. |
actions/setup/js/reconcile_work_queue_run.cjs |
Reconcile the completed run’s stored reservation. |
.github/workflows/agentic-workqueue.yml |
Add generated completion triggers and cleanup job. |
Review effort: Max
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 48a4ff9
|
…low-health Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot merge main and recompile |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
Release original worker reservations after authenticated job quiescence in the existing conclusion job. Reconcile hard-cancelled and stranded runs before scheduled compaction without an additional per-worker workflow run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve upstream updates and cover recovery of renamed workers and missed binding writes through scheduled maintenance. Keep normal termination inside the original worker's existing conclusion job and reject ambiguous bindings without exposing private identities. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
pelikhan
changed the title
|
🎉 This pull request is included in a new release. Release: |
Summary
worker_terminationinto each queue worker's existing trustedconclusionjob. There is no additional per-worker job or workflow run, and the sharedworkflow_runcompletion observer is removed.Compatibility and capacity
The closed V3 evidence contract gains
worker_termination, backed by a verified job-inventory digest and restricted to the exact original worker's reconciler identity. It permits release and cancellation of unfinished Claims, not Result or DeliveryFailure. Upgrade closed-schema readers before publishing this evidence; historical records are unchanged.Capacity now bounds task-producing worker execution, not the number of Actions runs still finishing conclusion diagnostics. Agent permissions remain unchanged. Existing native-terminal reconciliation remains the fallback for workers whose inline finalizer cannot execute.
Updated the TypeSpec contract, generated schemas, lifecycle specification, operator documentation, and all 30 compiled queue workers. The formal-model documentation explicitly distinguishes existing native-terminal coverage from the new inline inventory check.
Earlier queue recovery
Before this redesign, the unchanged trusted operator reconciled six independently verified terminal reservations. That historical snapshot had zero retained reservations and zero claimed Work; the three failed and two pending delivery barriers were preserved. This is not a newly refreshed live-state claim. No worker was dispatched or rerun for this change.
Validation
The concurrent branch update is preserved through a non-rewriting merge. This PR is not merged, and the redesign is not yet deployed.