Repository navigation
Fix missing work-queue diagnostic guidance - #71665
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ 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.
|
|
🧠 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.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
No actionable correctness or security issues found in the documentation-only changes. The three diagnostics are mapped into the existing work-queue guidance table, and the remedy text remains consistent with the playbook's supported deployment/update guidance.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~2 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs (per pr-triage recommendation for this documentation-only change) and independently re-ran the test_debug_playbook_covers_diagnostic_codes and consumption-budget tests against reconstructed pre/post versions of the file to verify the PR's claims.
📋 Verification & Key Themes
Verification performed
- Reconstructed the pre-PR table rows and confirmed
work_queue_pool_not_installed,work_queue_policy_proposal_mismatch, andwork_queue_worker_not_compiler_approvedwere genuinely unmapped before this change (each code appears 2-4 times acrossactions/setup/js/work_queue_*.cjssource files picked up by the guidance test's registry scan). - Confirmed the post-PR file maps all three codes into existing rows and the remaining 10
test_debug_playbook_covers_diagnostic_codessubtest failures (e.g.ERR_COMPILE_AGENTIC_VERSION_BLOCKED,ERR_GRADER_SESSION_INVALID,ERR_WORK_QUEUE_CLAIM_SCOPE) are present identically in both pre- and post-PR versions -- i.e. pre-existing and out of scope for this PR. - Verified the word/byte consumption budget: 2,799 words / 25,665 bytes, under the 2,800-word / 26,000-byte limits asserted by
test_debug_playbook_agent_consumption_budget. - Confirmed scope: diff touches only
.github/aw/debug-playbook.md, no runtime/workflow files.
Minor note
- Left one inline comment: the
work_queue_pool_not_installedmerge into thequeue_missing/policy_missingrow slightly blurs pool-specific vs. queue-bootstrap remedies compared to the more precise per-code explanation indocs/reference/work-queue.md. Non-blocking -- wording tightening for a strict word budget is a reasonable tradeoff here.
Positive highlights
- ✅ Correctly identifies and fixes exactly the failing assertions described in the linked issue (#71636), with no scope creep.
- ✅ Stays within the hard-coded word/byte budget enforced by tests.
- ✅ Minimal, surgical diff (3 additions / 3 deletions) with no unrelated changes.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~142 AIC · ⌖ ~15 AIC · ⊞ 10.2K
Comment /matt to run again
| | `unsupported_protocol`, `codec_invalid`, `invalid_json`, `invalid_utf8`, `invalid_unicode`, `duplicate_key`, `noncanonical_number` | Version, canonical bytes, keys, Unicode/numbers. | Align producer/reader; regenerate input, not history. | | ||
| | `ledger_invalid`, `ledger_foreign`, `ledger_nonextending` | Queue identity, DAG, ancestry. | Stop publication; recover trusted queue/tip via protocol. | | ||
| | `queue_missing`, `policy_missing` | Queue location; trusted config. | Approved AW-managed bootstrap/deployment; no manual admin seeding. | | ||
| | `queue_missing`, `policy_missing`, `work_queue_pool_not_installed` | Queue/pool; compiled/installed policy. | Approved AW-managed bootstrap/deployment; no manual admin seeding. | |
There was a problem hiding this comment.
[/grill-with-docs] Merging work_queue_pool_not_installed into the queue_missing/policy_missing row conflates two different failure scopes: the former is pool-specific (per work-queue.md line 334, "the requested pool is absent from installed Policy"), while the row remedy talks about queue bootstrap broadly. A reader chasing this specific code may not immediately see that the fix is a Policy update to add the pool, not a bootstrap/deployment action.
💡 Suggestion
Consider whether the remedy column should mention "or add the missing pool via an administrator Policy update" to match the more precise guidance already present in docs/src/content/docs/reference/work-queue.md:334. This keeps the playbook row consistent with the canonical per-code explanation used elsewhere in the repo.
@copilot please address this.
There was a problem hiding this comment.
The playbook now separates queue/policy bootstrap from the missing-pool remedy: quiesce the queue, then add the pool via an administrator-authorized Policy update. Commit d696d1482a.
There was a problem hiding this comment.
🟡 Changes recommended
Recovery guidance needs to distinguish installed Policy updates from deployment and compiler-approval fixes.
2 open issues
What changed in this PR
Updates gh-aw’s debugging playbook to cover three previously unmapped work-queue diagnostics without changing runtime code or workflows.
Changes:
- Adds the diagnostics to existing evidence/remedy rows.
- Tightens table wording to retain the 2,799-word consumption budget.
| File | Description |
|---|---|
.github/aw/debug-playbook.md |
Adds work-queue diagnostic mappings and condenses guidance. |
Review effort: Max
| | `unsupported_protocol`, `codec_invalid`, `invalid_json`, `invalid_utf8`, `invalid_unicode`, `duplicate_key`, `noncanonical_number` | Version, canonical bytes, keys, Unicode/numbers. | Align producer/reader; regenerate input, not history. | | ||
| | `ledger_invalid`, `ledger_foreign`, `ledger_nonextending` | Queue identity, DAG, ancestry. | Stop publication; recover trusted queue/tip via protocol. | | ||
| | `queue_missing`, `policy_missing` | Queue location; trusted config. | Approved AW-managed bootstrap/deployment; no manual admin seeding. | | ||
| | `queue_missing`, `policy_missing`, `work_queue_pool_not_installed` | Queue/pool; compiled/installed policy. | Approved AW-managed bootstrap/deployment; no manual admin seeding. | |
There was a problem hiding this comment.
Missing-pool recovery now explicitly requires quiescence followed by an administrator-authorized Policy update that adds the pool; bootstrap remains for missing queues or policies. Commit d696d1482a.
| | `policy_invalid`, `policy_epoch_invalid`, `policy_proposal_mismatch`, `profile_invalid`, `pool_invalid`, `dispatcher_invalid` | Schema, approved revision, epoch, pools/dispatcher. | Correct trusted config via supported deployment/update. | | ||
| | `work_queue_profile_not_approved`, `work_queue_dispatch_credential_identity_unavailable` | Worker approval; verifiable dispatch identity. | Fix approved deployment/credentials; never substitute unverified identity. | | ||
| | `policy_invalid`, `policy_epoch_invalid`, `policy_proposal_mismatch`, `work_queue_policy_proposal_mismatch`, `profile_invalid`, `pool_invalid`, `dispatcher_invalid` | Schema/revision, epoch, pools/dispatcher. | Correct trusted config via supported deployment/update. | | ||
| | `work_queue_profile_not_approved`, `work_queue_worker_not_compiler_approved`, `work_queue_dispatch_credential_identity_unavailable` | Compiler-approved worker; verifiable identity. | Fix approved deployment/credentials; never substitute unverified identity. | |
There was a problem hiding this comment.
The row names the installed-profile check and caller-compiled dispatch allowlist, and separates quiescent Policy profile updates from allowlist/credential fixes. Commit d696d1482a.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@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: 5df132d
|
…rk-queue-diagnostic-guidance Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Completed the finisher pass: merged the latest |
|
🎉 This pull request is included in a new release. Release: |
Three work-queue diagnostics lacked evidence/remedy mappings, causing four guidance assertions to fail.
work_queue_pool_not_installed,work_queue_policy_proposal_mismatch, andwork_queue_worker_not_compiler_approvedinto existing table rows..github/aw/debug-playbook.md; no runtime or workflow changes.