Apply logs timeout and count globally across concurrent targets by Copilot · Pull Request #60058 · github/gh-aw

Copilot · 2026-09-10T21:41:28Z

Multi-target logs downloads applied --timeout and --count independently per target. These limits should bound the combined concurrent operation.

Timeout
- Use one deadline for the full multi-target download.
- Prevent queued targets from receiving a fresh timeout.
- Keep final API operations within the same deadline.
Count
- Share one atomic run budget across targets.
- Stop collection when the global count is reached.
- Globally cap and sort merged results.
Partial results
- Preserve runs collected before a target fails or reaches a limit.
- Emit resumable continuations for queued date-range targets that make no progress.
Documentation
- Clarify global multi-target semantics in CLI help and reference documentation.

gh aw logs workflow-a workflow-b --count 100 --timeout 10

This returns up to 100 runs combined and limits the overall download to 10 minutes.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-09-10T21:44:48Z

✅ Ponytail Reviewer completed successfully!

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #60058

github-actions · 2026-09-10T21:44:50Z

⚠️ PR Code Quality Reviewer failed during code quality review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-09-10T21:44:54Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-09-10T21:44:54Z

⚠️ Security scanning failed for Design Decision Gate 🏗️. Review the logs for details.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-09-10T21:45:01Z

🧠 Matt Pocock Skills Reviewer failed 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 happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

pelikhan · 2026-09-10T21:47:45Z

@copilot make the sure cancelation context is propagated correctly through all the layers and workers

github-actions

Ponytail review: a few opportunities to trim wrapper and duplicate-limit plumbing without changing behavior.

net: -12 lines possible.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by ✂️ Ponytail Reviewer for #60058 · codex · gpt53codex · 3.13 AIC · ⌖ 4.12 AIC · ⊞ 12.8K
_{Comment /ponytail to run again}

github-actions · 2026-09-10T21:49:25Z

 	return shouldStopAfterWorkflowRunBatch(batch, opts.Verbose), nil
 }

+func markSharedLogsCountReached(state *logsCollectionState, fetchAllInRange bool, limit *logsCountLimit) bool {


L340: shrink: markSharedLogsCountReached only wraps limit.isReached() and assignment. Inline the two lines at call sites; delete helper.

Keeping markSharedLogsCountReached as a helper: it's called from two sites (collectProcessedWorkflowRuns and fetchAndProcessLogsBatch/finishLogsBatch), so inlining would duplicate the isReached()/assignment logic rather than simplify it.

github-actions · 2026-09-10T21:49:25Z

 		if len(processedRuns) >= opts.count {
 			continue
 		}
+		if !opts.countLimit.tryAdd() {


L582: shrink: double-guard (len(processedRuns) < opts.count and countLimit.tryAdd) duplicates budget checks. Keep shared countLimit.tryAdd gate only; remove local length check.

Not removing the local length check: opts.countLimit is nil in single-target mode (only set via newLogsCountLimit in the multi-target path), and countLimit.tryAdd() returns true unconditionally for a nil receiver. The len(processedRuns) < opts.count guard is the only enforcement of --count for single-target downloads, so dropping it would break that case.

github-actions · 2026-09-10T21:49:25Z

 	rateLimitFirstRequest  bool
 	maxConcurrentDownloads int
 	storageLimit           *logsStorageLimit
+	countLimit             *logsCountLimit


L64: yagni: carrying both Count and countLimit in options creates parallel limit mechanisms. Keep one source of truth (countLimit) and derive Count at boundary only.

Keeping both fields: Count is the public per-command option set for every download (including single-target, where countLimit stays nil), while countLimit is an internal cross-goroutine atomic budget only allocated for multi-target runs. They aren't parallel copies of the same limit — countLimit doesn't exist unless multiple targets share one budget.

github-actions · 2026-09-10T21:49:40Z

🏗️ ADR required — draft added for PR #60058

I enforced the design-decision gate for this PR because it adds more than 100 lines in business-logic directories (pkg/).

Evidence used

Prefetch summary: default_business_additions = 192, has_implementation_label = false, requires_adr_by_default_volume = true
PR title: Apply logs timeout and count globally across concurrent targets
Diff evidence:
- adds a shared wall-clock timeout context for multi-target logs downloads
- adds a shared atomic count limiter across concurrent targets
- preserves partial results and resumable continuations when shared limits or failures stop progress
- updates CLI/docs text so --count and --timeout are global across all targets

Outcome

I added a draft ADR at:

docs/adr/60058-apply-global-limits-to-multi-target-logs.md

Next action

Please review and refine the draft ADR so the decision rationale and trade-offs match maintainer intent before merge.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · gpt54 · 14.6 AIC · ⊞ 10.1K · ◷
_{Comment /review to run again}

Copilot

🟡 Changes recommended

Global selection and continuation handling have correctness gaps under concurrent limits.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Applies count and timeout limits globally across concurrent multi-target log downloads.

Changes:

Adds shared deadline and atomic run-count budgeting.
Preserves partial results and adds continuation handling.
Updates tests, CLI help, and documentation.

File summaries

File	Description
`pkg/cli/logs_orchestrator.go`	Preserves partial runs and expands continuation behavior.
`pkg/cli/logs_orchestrator_unit_test.go`	Tests zero-progress count continuations.
`pkg/cli/logs_orchestrator_types.go`	Adds shared count-limit state.
`pkg/cli/logs_orchestrator_download.go`	Enforces the shared count budget during collection.
`pkg/cli/logs_multi.go`	Coordinates global timeout, count, sorting, and partial results.
`pkg/cli/logs_multi_test.go`	Tests global limits and partial-result merging.
`pkg/cli/logs_command.go`	Clarifies global flag semantics.
`docs/src/content/docs/setup/cli.md`	Documents multi-target limits.

Review details

Files reviewed: 9/9 changed files
Comments generated: 3
Review effort level: Balanced

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

Copilot · 2026-09-10T22:20:36Z

+		if !opts.countLimit.tryAdd() {
+			continue


This is a real trade-off, not a bug we're silently accepting: enforcing strict CreatedAt ordering across concurrent targets would require buffering/merging candidate metadata before any artifact download starts, which conflicts with the concurrent early-stop design this PR relies on to bound total work under the shared timeout. That trade-off (global admission-order fairness vs. concurrent early-stopping) is captured in docs/adr/60058-apply-global-limits-to-multi-target-logs.md for maintainer review. Not changing the admission strategy in this pass; happy to revisit if the ADR discussion lands on stricter ordering.

Copilot · 2026-09-10T22:20:28Z

+	allAPIRateLimits := startGitHubAPIRateLimitReports(activeCtx, logsTargetRateLimitHosts(targets))
+	results := collectLogsTargets(activeCtx, opts, targets)


Fixed in b56d1da: a target still waiting on the semaphore when the shared context is canceled no longer returns a bare error. queuedLogsTargetResult builds a resumable continuation from the target's own options (preserving its cursor and timeout flag) instead. Covered by TestCollectLogsTargetsEmitsContinuationForQueuedTarget.

Copilot · 2026-09-10T22:20:23Z

+	if markSharedLogsCountReached(state, runtime.fetchAllInRange, opts.countLimit) {
+		return true, nil


Fixed in e572154: the allRunsConsumed cursor advance now runs before the shared count-limit check in fetchAndProcessLogsBatch (extracted into finishLogsBatch), so a continuation built after the shared budget is exhausted mid-batch resumes from the correct pagination boundary instead of re-scanning the batch. Added TestFetchAndProcessLogsBatchAdvancesCursorWhenSharedCountLimitReached to cover it.

pelikhan · 2026-09-10T21:55:10Z

@copilot run pr-finisher skill

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-09-10T22:06:45Z

@copilot make the sure cancelation context is propagated correctly through all the layers and workers

Fixed in 30ddcb5 (or the latest push): a target that is still waiting on the worker semaphore when the shared deadline/cancellation fires no longer returns a bare ctx.Err(). It now builds a resumable continuation from its own options (preserving its BeforeRunID cursor) so it isn't silently dropped from the report. Also relaxed buildContinuationIfNeeded so a target that made zero progress can still emit that continuation. Verified downstream API calls (gh CLI invocations, rate-limit checks, artifact download pool) already thread the same context/deadline through; added TestCollectLogsTargetsEmitsContinuationForQueuedTarget to cover the queued-target path.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-09-11T01:38:53Z

🎉 This pull request is included in a new release.

Release: v0.89.3

Copilot AI and others added 2 commits September 10, 2026 21:31

Enforce global logs timeout and count

757951b

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Preserve partial logs at shared limits

f1b84f4

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI assigned Copilot and pelikhan Sep 10, 2026

Copilot AI changed the title ~~Apply logs timeout and count globally across targets~~ Apply logs timeout and count globally across concurrent targets Sep 10, 2026

Copilot finished work on behalf of pelikhan September 10, 2026 21:42

Copilot created this pull request from a session on behalf of pelikhan September 10, 2026 21:42 View session

Copilot AI requested a review from pelikhan September 10, 2026 21:42

pelikhan marked this pull request as ready for review September 10, 2026 21:42

Copilot AI balanced review requested due to automatic review settings September 10, 2026 21:42

Copilot started reviewing on behalf of pelikhan September 10, 2026 21:43 View session

github-actions Bot added the needs-review label Sep 10, 2026

github-actions Bot reviewed Sep 10, 2026

View reviewed changes

Add draft ADR for multi-target logs limits

2280177

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Copilot started work on behalf of pelikhan September 10, 2026 21:50 View session

github-actions Bot mentioned this pull request Sep 10, 2026

[pr-sous-chef] PR Sous Chef run report #59940

Closed

Propagate cancellation context through queued multi-target workers

b56d1da

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan September 10, 2026 22:07

Copilot started work on behalf of pelikhan September 10, 2026 22:07 View session

Merge branch 'main' into copilot/review-logs-command-timeout

bfa2a2e

Advance beforeDate cursor before shared count-limit stop

e572154

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan September 10, 2026 22:28

pelikhan merged commit 6f90863 into main Sep 10, 2026
34 checks passed

pelikhan deleted the copilot/review-logs-command-timeout branch September 10, 2026 23:07

		allAPIRateLimits := startGitHubAPIRateLimitReports(activeCtx, logsTargetRateLimitHosts(targets))
		results := collectLogsTargets(activeCtx, opts, targets)

		if markSharedLogsCountReached(state, runtime.fetchAllInRange, opts.countLimit) {
		return true, nil

Conversation

Copilot AI commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pelikhan commented Sep 10, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 10, 2026

Evidence used

Outcome

Next action

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Sep 10, 2026

Uh oh!

Copilot AI commented Sep 10, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading