Repository navigation
Prefer local Git lookups for work queue replay - #70617
Conversation
Resolve queue repositories from local Git before hosted discovery and reuse validated runtime history when the advertised queue head is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Trace contract, receipt, readback, inventory and retry-budget failures in runtime and native delivery verification. Emit an always-visible runtime exhaustion summary and cover logging privacy and settlement invariants. Co-authored-by: Copilot App <223556219+Copilot@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.
|
|
✅ 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. Commit pushed:
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (239 new lines in 📄 Draft ADR committed: The draft records these decisions:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All 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.
|
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
One correctness issue found: the documented always-visible delivery-exhaustion diagnostic is still gated by DEBUG, so normal failed deliveries can remain unexplained. See the inline comment on pkg/workqueue/delivery.go:219.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 4.56 AIC · ⌖ 0.677 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /improve-codebase-architecture and /tdd (via pr-triage). One inline suggestion posted on pkg/cli/work_command.go; everything else is solid enough to approve.
📋 Key Themes & Highlights
Key Themes
- Fragile parent-name check: work_command.go:52 identifies the work-queue root via cmd.Parent().Name() == "work-queue", a stringly-typed coupling that a future rename/alias change could silently break (see inline comment).
- Minor style nit (not flagged inline): repo.go's if ... return ... else { ... } after a returning branch could drop the else for a flatter early-return style, but this is cosmetic and not worth blocking on.
Positive Highlights
- ✅ getCurrentRepoSlugFromGit() plus the local-origin-first precedence is well isolated and correctly preserves the GH_REPO override for gh CLI compatibility.
- ✅ TestCurrentRepoSlugPrefersLocalGitBeforeGH is an excellent table-driven test (https/ssh/missing-origin/unsupported-origin/explicit-env) — exactly the kind of coverage /tdd asks for.
- ✅ The new verifiedHead caching in work_queue_git_transport.cjs is subtle (skipping fetch/fsck when the advertised head is unchanged) but is correctly ordered behind advertise() and is backed by a new test asserting exact network call counts across the git-vs-remote-changed scenarios.
- ✅ All new log.debug(...) diagnostics across work_queue_delivery.cjs, finish_work_queue_claim.cjs, and pkg/workqueue/delivery.go correctly go through the existing privacy-safe diagnostic() filter (booleans/safe ints only), and are backed by a dedicated regression test (TestDeliveryVerificationLogsAreDiagnosticAndPrivate) asserting no secret/identity leakage.
- ✅ Documentation in work-queue.md was updated to match the new lookup precedence and DEBUG namespaces.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 148 AIC · ⌖ 14.9 AIC · ⊞ 10.1K
Comment /matt to run again
Use the existing warning path for sanitized exhaustion summaries, verify no-DEBUG behavior, and finalize ADR-70617 with repository inference and history reuse tradeoffs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Regarding #70617 (comment): No code change is needed for this informational result, which contains no actionable finding. I am not treating reported_no_action as evidence of test coverage; the delivery fix has focused regression coverage and separate validation. |
|
Regarding #70617 (comment): No code change is needed because this completion notice reports no actionable finding. The substantive diagnostic and ADR feedback has been addressed separately rather than inferring approval from reported_no_action. |
|
Regarding #70617 (comment): No code change is needed for this review-progress notice; it contains no finding or requested change. This is not being treated as a completed review or approval. |
|
Regarding #70617 (comment): Accepted the related ADR follow-up and finalized the generated document with the actual decision date, rationale, alternatives, and fork-selection tradeoffs. The ADR is linked in the PR description and remains Proposed pending maintainer acceptance. |
|
Regarding #70617 (comment): Accepted the review's linked correctness finding. Native delivery exhaustion now uses the existing always-visible warning path, with no-DEBUG regression coverage; other diagnostic detail remains opt-in. |
|
Regarding #70617 (comment): Accepted: completed ADR-70617, corrected its date to 2026-10-20, and linked it in the PR description. It explicitly documents the global origin-first behavior, fork-versus-upstream selection, override precedence, authenticated history reuse, and diagnostic privacy/visibility tradeoffs without claiming maintainer approval. |
|
Regarding #70617 (comment): Accepted the recorded diagnostic issue and addressed it in the same native exhaustion fix, including an assertion with DEBUG and runner-debug variables unset. No separate change to bot-managed comment memory is needed; its saved metadata was left untouched. |
There was a problem hiding this comment.
🟡 Changes recommended
Repository-targeting regressions and credential exposure through debug logging remain unresolved.
5 open issues
What changed in this PR
Improves gh-aw work-queue replay efficiency and delivery troubleshooting.
Changes:
- Adds local-origin repository inference while preserving offline snapshot replay.
- Reuses validated Git history when authenticated remote heads remain unchanged.
- Expands delivery diagnostics, regression coverage, and documentation.
| File | Description |
|---|---|
| pkg/workqueue/delivery.go | Adds verifier budget and settlement logging. |
| pkg/workqueue/delivery_test.go | Tests diagnostic output and privacy. |
| pkg/workqueue/delivery_host.go | Logs native inventory and readback checks. |
| pkg/cli/work_command.go | Infers repositories for queue commands. |
| pkg/cli/work_command_test.go | Covers inferred and explicit repositories. |
| pkg/cli/work_command_replay_test.go | Covers checkout-free snapshot replay. |
| pkg/cli/repo.go | Prefers local origin discovery. |
| pkg/cli/repo_test.go | Tests repository lookup precedence and fallback. |
| docs/src/content/docs/reference/work-queue.md | Documents lookup behavior and diagnostic namespaces. |
| actions/setup/js/work_queue_native_creation_delivery.test.cjs | Tests mismatch diagnostics, privacy, and exhaustion reporting. |
| actions/setup/js/work_queue_native_creation_delivery.cjs | Adds native creation readback diagnostics. |
| actions/setup/js/work_queue_logging.test.cjs | Covers sanitized contract and receipt logging. |
| actions/setup/js/work_queue_git_transport.test.cjs | Tests history reuse and changed-head fetching. |
| actions/setup/js/work_queue_git_transport.cjs | Reuses validated history for unchanged heads. |
| actions/setup/js/work_queue_delivery.cjs | Adds contract, receipt, and resource diagnostics. |
| actions/setup/js/finish_work_queue_claim.cjs | Adds budget diagnostics and visible exhaustion summaries. |
Review effort: Max
Replace the parent-name coupling with the captured queue root pointer, covering renamed roots, aliases, explicit repository precedence and root help. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Regarding #70617 (comment): No further code change is needed for this informational result. As previously noted, reported_no_action is not being treated as verification. |
|
Regarding #70617 (comment): No further code change is needed; this completion notice contains no actionable finding. Validation remains separate from the reported_no_action status. |
|
Regarding #70617 (comment): Accepted the associated command-identity finding and replaced the parent-name check with actual command identity, covered by renamed-root and alias regression tests. The updated notice reports completion; it is not being treated as approval. |
|
Regarding #70617 (comment): Already addressed in 6850574: the generated ADR was completed and linked in the PR description. It remains Proposed pending maintainer acceptance; no further change is needed. |
|
Regarding #70617 (comment): Already addressed in 6850574: native exhaustion now uses the always-visible warning path with no-DEBUG regression coverage. This completion notice requires no additional code change. |
|
Regarding #70617 (comment): The native exhaustion finding was addressed in 6850574 with an always-visible warning and no-DEBUG coverage. No additional change is required by this completion notice. |
|
Regarding #70617 (comment): ADR-70617 is completed, dated 2026-10-20, and linked in the PR description. This batch revises its decision and consequences to describe queue-only local-first inference while preserving existing callers' gh-selected targets. |
|
Regarding #70617 (comment): The saved exhaustion theme is already fixed with an ungated native warning and no-DEBUG regression. Bot-managed memory remains untouched; newer diagnostic-accuracy findings are handled separately. |
|
Regarding #70617 (comment): This acknowledgment remains valid: a no-op result is not verification. No additional implementation change is requested here. |
|
Regarding #70617 (comment): This acknowledgment remains valid; the completion notice is informational rather than approval. No additional implementation change is needed. |
|
Regarding #70617 (comment): The original notice now reports review completion, and its command-identity finding was fixed in b96859b. The earlier progress-only wording is outdated, but completion is still not approval. |
|
Regarding #70617 (comment): The ADR remains completed, linked, and Proposed pending maintainer acceptance. This batch updates the repository-resolution decision to queue-only scope. |
|
Regarding #70617 (comment): The native exhaustion warning and no-DEBUG coverage remain in place. This prior response requires no additional code change. |
|
Regarding #70617 (comment): The ADR's corrected date, required sections, and link remain valid, but the global origin-first decision is superseded. Existing shared callers now retain gh selection, with local-first inference scoped to queue commands. |
|
Regarding #70617 (comment): The exhaustion regression still clears debug variables and bot-managed memory remains unchanged. No additional implementation change is needed for this acknowledgment. |
|
Regarding #70617 (comment): This repeated informational acknowledgment remains valid: reported_no_action is not verification. No additional code change is needed. |
|
Regarding #70617 (comment): This repeated acknowledgment contains no outstanding change request. Validation is still performed independently of the no-op review status. |
|
Regarding #70617 (comment): The root-pointer command-identity fix and renamed-root/alias coverage remain in place from b96859b. No additional change is needed for this prior response. |
|
Regarding #70617 (comment): The ADR remains completed, linked, and Proposed. Its resolution scope is being refined in this batch to preserve existing gh-selected targets. |
|
Regarding #70617 (comment): The always-visible native exhaustion path and no-DEBUG regression remain valid. No further change is needed for this completion acknowledgment. |
|
Regarding #70617 (comment): The ADR date, required sections, and PR link remain correct; its global origin-first tradeoff is superseded by the repository-scope fix. The updated ADR now documents queue-only local-first inference. |
|
Regarding #70617 (comment): The native exhaustion issue remains fixed and bot-managed memory remains untouched. This acknowledgment adds no outstanding implementation work. |
|
Regarding #70617 (comment): No additional change is required: the underlying no-op status is still not evidence of verification. |
|
Regarding #70617 (comment): No additional change is required; this prior informational acknowledgment does not imply approval. |
|
Regarding #70617 (comment): The reviewer completion status and command-identity fix remain accurately described. No additional change is needed for this prior response. |
|
Regarding #70617 (comment): The completed ADR remains linked and Proposed. Its repository-resolution details have been updated to match the narrower queue-only behavior. |
|
Regarding #70617 (comment): Native exhaustion remains always visible, with no-DEBUG regression coverage. No additional change is needed here. |
|
Regarding #70617 (comment): The ADR still covers overrides, reuse, privacy, and fork behavior, but its repository-resolution decision is narrowed in this batch. Unrelated shared callers preserve gh selection rather than inheriting queue-local origin-first behavior. |
|
Regarding #70617 (comment): The ungated exhaustion warning and regression clearing debug variables remain in place. No memory edit or additional implementation change is needed for this acknowledgment. |
|
@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: b96859b
|
|
🎉 This pull request is included in a new release. Release: |
Why
Work queue inspection should use available local Git information before making hosted repository lookups, and repeated replay should not refetch history that is already validated locally. Delivery verification also needs actionable, privacy-preserving diagnostics to explain why completed work has not produced a verified Result.
Changes
originbefore hosted discovery, preserving explicit--repoandGH_REPOprecedence. Existing shared callers, including workflow runs and secret uploads, retain gh's selected canonical repository.replay --filewithout checkout-based repository inference. Local queue discovery cannot establish hosted renames; explicit canonical selection remains necessary for stale renamed origins, and queue identity checks remain fail-closed.ADR: ADR-70617: Prefer Local Git for Work Queue Repository Resolution and Replay
The ADR documents queue-only inference, existing callers' preserved gh selection, fork/rename limits, and history reuse. It remains Proposed pending maintainer acceptance and supersedes the earlier global origin-first proposal.
Validation
Queue health observed
Read-only inspection found 26 available items with no active claims or native reservations, including two worker-incompatible items. All eight completed items had failed delivery verification, with none verified. This change does not modify queue state or repair those operational failures.