Repository navigation
Cancel obsolete worker backlog during queue maintenance - #76126
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ 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.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: 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.
|
|
✅ 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.
|
|
❌ PR Specification Reviewer failed to deliver outputs. Please review the logs for details.
|
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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
I found one merge-blocking maintenance reliability issue: the new obsolete-work sweep runs before all existing scheduled reconciliation and lets expected race/publication failures abort the rest of maintenance. This can starve stranded reservations and pending-delivery recovery. Please isolate the sweep failure or run it after the established reconciliation.
Static review only; no tests were run. The specialist pass completed and its advisory findings were independently adjudicated.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~4 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Reviewed head 0ff49eb against the Work Queue Specification (updated to v1.0.0, 2026-11-09).
No enforceable violations found. The implementation preserves pins, Claims, Results, delivery barriers, policy, debt, and reservations, and rechecks eligibility through authenticated CAS/replay validation.
Synchronized the directly affected specification: §7.10 now defines bounded scheduled-maintenance cancellation for available, unowned, unpinned Work with incompatible active contracts; the campaign-revision section references the same safeguarded reconciler path. The specification-only commit was queued to the same-repository PR branch via safe output.
Coverage: static review of the captured diff, relevant runtime/tests, and work-queue formal contract; tests were not run as required for this review mode.
Generated by PR Specification Reviewer for #76126 · copilot · gpt50 · ~8 AIC · ⌖ ~1 AIC · ⊞ 1.7K
Comment /review-specs to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd. Requesting changes on maintenance error isolation and a test that does not exercise the production sort order.
📋 Key Themes & Highlights
Key Themes
- Failure isolation: the new obsolete-work step runs before queue reconciliation with no try/catch, so a throw there skips stranded-reservation and pending-delivery repair.
- Test fidelity: the batch uses
localeComparein production, but the test picks its expected prefix with default.sort(). The fixture does not separate the two orders.
Positive Highlights
- ✅ The obsolescence predicate is identical in JS and Go, and the Go test checks parity by replaying through the JS module.
- ✅ Unavailable deployments are correctly excluded from obsolescence, and the test covers it.
- ✅ Staged mode and non-AW authorization both skip cancellation.
- ✅ The stale-batch replay guard rejects cancellations that no longer match the current ledger.
- ✅ Idempotence is covered: a second run cancels 1, then 0.
Open Question (not verified)
executionProfiletreats a missingadmission_contractas incompatible. If a worker profile can lack alogical_contractwhile a deployment exists for it, its Work would be classified obsolete. I did not confirm this path is reachable. Worth a quick check or a test.
Validation
- I did not run the test suites. The PR body reports them passing.
- Skill selection used the fallback heuristic:
pr-triagewas unavailable, so I classified the diff directly frompr-meta.jsonandpr-diff.patch.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · routed: haiku55 xhigh · ~7 AIC · ⌖ ~2 AIC · ⊞ 6.3K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
Legacy-ledger replay incompatibility can make queues unreadable, while stale expiration handling can repeatedly reject cancellation batches and stall backlog processing.
3 open issues
What changed in this PR
Adds scheduled retirement of backlog admitted under obsolete worker contracts, using the existing queue ledger and checked cancellation flow.
Changes:
- Select deterministic cancellation batches bounded by the installed operation limit and 256 Work.
- Recheck cancellation eligibility in JavaScript and Go replay while preserving pins, ownership, and delivery barriers.
- Add recovery and replay-parity tests, operator guidance, and release notes.
| File | Description |
|---|---|
pkg/workqueue/replay.go |
Rechecks obsolete cancellation eligibility during native replay. |
pkg/workqueue/deployment.go |
Defines obsolete Work eligibility in Go. |
pkg/workqueue/deployment_test.go |
Adds cancellation safety and JavaScript/Go parity tests. |
docs/src/content/docs/reference/work-queue.md |
Documents backlog retirement and checked operator recovery. |
actions/setup/js/work_queue_replay.cjs |
Rechecks obsolete cancellations during JavaScript replay. |
actions/setup/js/work_queue_deployment.cjs |
Exports the shared obsolete Work predicate. |
actions/setup/js/work_queue_deployment_control.cjs |
Selects, publishes, and verifies bounded cancellation batches. |
actions/setup/js/work_queue_deployment_authority.test.cjs |
Tests batching, protected state, and publication conflicts. |
actions/setup/js/reconcile_work_queue_run.test.cjs |
Tests scheduled retirement without worker dispatch. |
actions/setup/js/reconcile_work_queue_run.cjs |
Integrates retirement into maintenance and reports cancellation counts. |
.github/aw/work-queue.md |
Updates operational guidance for incompatible worker contracts. |
.changeset/patch-work-queue-obsolete-contracts.md |
Adds patch release notes for backlog retirement. |
Review effort: Max (auto)
Preserve core recovery after sanitized obsolete-sweep failures and use the shared deterministic comparator for bounded retirement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Regarding your comment: No code change is needed for this completion notice: it contains no actionable finding, and reported_no_action is not being treated as validation. The separate maintenance-isolation and ordering findings are addressed with focused regression coverage. Included in b5252c5. |
|
Regarding your comment: Accepted the substantive skills-review findings: obsolete-retirement failures are isolated and safely reported, and production/tests share deterministic ordering with a discriminating regression. The missing-contract question is covered by existing invariants: admission stamps admission_contract, checkpoint validation rejects its absence for deployed Work, and contractless profiles create no deployment. Included in b5252c5. |
|
Regarding your comment: No additional code or ADR is needed for this no-action gate notice. This batch keeps the existing queue architecture and authority boundaries, adding failure isolation and deterministic-ordering regressions rather than introducing a new design decision. Included in b5252c5. |
|
Regarding your comment: No standalone change is needed for this completion notice, which reports no test-quality finding. The actionable review feedback now has regressions for real reservation/delivery recovery after retirement failures, secret redaction, and case/punctuation prefix ordering. Included in b5252c5. |
|
Regarding your comment: Accepted the associated code-quality review's maintenance reliability finding. Retirement failures no longer prevent established recovery phases from running, and regressions preserve both sanitized reporting and fatal independent delivery-verification failures. Included in b5252c5. |
|
Regarding your comment: The failed step was Process Safe Outputs: its proposed specification push was blocked because the workflow integration could not read branch-protection metadata. No specification commit landed; this batch does not change credentials, bypass that protection, or rerun the workflow. Included in b5252c5. |
|
Regarding your comment: The recorded maintenance failure-isolation theme is addressed by the same guarded reporting and recovery regressions. No direct edit to the managed memory block is needed; its workflow-owned historical metadata is preserved. Included in b5252c5. |
|
🍳 A little prep to help this PR along! Copilot assignment requested for the following outstanding work:
Keeping the changes focused; human review and CI get the final taste test. No merging or CI triggering. Structured data{
"completed_task": 0,
"head": "b5252c58b7f50efbf56ee9c36f3059983e02eeb3",
"kind": "requested",
"schema_version": "1",
"state": "ea848037c4d234bae198c1e5ef02df6df1e742c37ca6605559442118ef435cf5",
"work": [
"5cc61dcd7236884ed79abae5bc89af4373a1f6aa3b510aff353199ccee320ef6"
],
"workflow": "pr-sous-chef"
}
|
…ment-compatibility Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add bounded retirement safety, mutation controls and guarded witnesses; preserve checked and historical cancellation metadata, protected queue state and isolated maintenance recovery. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🍳 A little prep to help this PR along! Copilot assignment requested for the following outstanding work:
Keeping the changes focused; human review and CI get the final taste test. No merging or CI triggering. Structured data{
"completed_task": 0,
"head": "e5c7ad06f0f0f5b6bfc1d6743ba29c44f0c5d1e7",
"kind": "requested",
"schema_version": "1",
"state": "86487498fd873e8e84752b47898e7debff9026576be22fa9148def9f378ad9f5",
"work": [
"1027833686a7050a64d8bfe136387980a34b94be7e9b120f51fd825652824672",
"12e2d92b8c5b535f1d3147e14962e78f9a50d376e8a16f13e4326968dc5aa3f5",
"1612451f13c5d54618082aa0d5b67fe89e2c3d1f16e5cfc6506fa9a24c2e9a24"
],
"workflow": "pr-sous-chef"
}
|
…ment-compatibility Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Preserve the verified expiration, checked-cancellation and maintenance-recovery fixes while integrating main at 8d04540. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Regarding your comment: No additional source change is needed for this completion notice because it contains no actionable finding. reported_no_action is not being treated as validation; the separate maintenance finding has explicit recovery coverage. |
|
Regarding your comment: The substantive isolation and ordering findings are already addressed, and fresh focused ordering and deployment-compatibility tests pass. The missing-contract concern is excluded by valid deployed admission and checkpoint restoration, so no additional change is warranted. |
|
Regarding your comment: No additional code or ADR is warranted by this no-action completion notice. The existing queue architecture and authority boundaries remain unchanged, and reported_no_action is not being used as design validation. |
|
Regarding your comment: This completion notice contains no standalone test-quality finding. Published regressions contain concrete recovery and diagnostic-redaction assertions, and fresh focused ordering and compatibility tests pass. |
|
Regarding your comment: Accepted the associated maintenance reliability finding and verified the published fix. Focused regressions confirm recovery continues after retirement failures without suppressing independent verification failures. |
|
Regarding your comment: Confirmed the failure diagnosis: run 42559589900 failed in Process Safe Outputs because its integration could not read branch-protection metadata, so the proposed specification push was blocked. The current PR separately contains the specification updates; no credential changes, protection bypass or workflow rerun is needed. |
|
Regarding your comment: The recorded maintenance failure-isolation theme is addressed and verified by the existing guarded recovery path and regressions. No edit to the workflow-managed historical memory block is needed. |
|
Regarding your comment: This earlier disposition remains accurate: the completion notice requires no separate source change and is not validation evidence. Published recovery coverage and the independently checked ordering regressions support the substantive fixes. |
|
Regarding your comment: The earlier skills-review response remains supported by current admission and checkpoint validation, and production ordering has an independent discriminating regression. Fresh focused ordering and deployment-compatibility tests pass; no further source change is needed. |
|
Regarding your comment: The earlier no-additional-ADR disposition remains appropriate because these fixes preserve the existing queue architecture and authority boundaries. The completion notice itself supplies no new design finding. |
|
Regarding your comment: The earlier response remains supported by concrete recovery, diagnostic-redaction and case/punctuation ordering assertions in the published tests. Fresh focused ordering tests pass, with no additional test change needed for this notice. |
|
Regarding your comment: Confirmed that the previously published maintenance fix remains effective. The recovery, sanitized-reporting and independent-verification regressions pass without further source changes. |
|
Regarding your comment: Confirmed independently that the historical safe-output push was blocked when its integration could not read branch-protection metadata. The current specification documents versioned cancellation and bounded TLA+ coverage; no credential changes, protection bypass or workflow rerun is needed. |
|
Regarding your comment: Confirmed that the existing failure-isolation fix and its regression coverage remain effective. The workflow-owned historical memory metadata has been preserved without edits. |
|
Regarding your comment: Accepted the branch update and preserved both word-budget fixes while integrating origin/main at 8d04540. The original shard-4 failure is fixed: the instructions have 793 words, all six unchanged documentation tests pass, and shard 4 succeeded at the previous PR head; no workflow rerun or dispatch was requested. |
…ibility' into pelikhan/queue-deployment-compatibility Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Updated with All three failures were HTTP 504 release-download errors, not reported lint/code failures. The pinned linter installed locally; Go lint, impacted Go tests, 152 JavaScript tests, and all 365 workflow drift checks passed. The NVX musl integration and download-retry regression also passed locally, so no additional source fix was needed. Secret scanning passed. CodeQL timed out and could not be rerun. No CI reruns or workflow dispatches were requested, and the PR was not merged. |
Why
Work admitted under an older worker contract can remain indefinitely available but blocked after an approved deployment changes its contract. Retire that obsolete backlog as terminally cancelled while preserving the queue's ledger history and execution safety boundaries.
Approach
worker_contract_obsoletecancellations against the publication's current ledger, rejecting concurrent ownership or compatible-route changes.Validation
make agent-report-progresspassed, including a 365-workflow compilation/drift check with unchanged locks.Live recovery status
Both affected approved worker routes already match current default-branch contracts. Native checked deployment and cancellation calls timed out; refreshed ledger verification at
1421eac95b0c6f35dfd8f8fbbffeacda37c29ecefound neither recovery request committed. Nine obsoletedeep-reportWork and onedaily-regulatoryWork remain queued. This PR implements prospective maintenance behavior; it does not claim that live backlog recovery succeeded.No workflows were dispatched, completed effects retried, queue state reset, or approval/contract checks bypassed.