feat: implement issue #954 — Token Cost Report weekly run times out at 30m and silently stops posting - #955
Conversation
…t 30m and silently stops posting
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe workflow now gives token report generation more time, posts the report only after successful completion with output present, and adds a failure/cancellation comment on the tracking issue. The collection script adds per-call timeouts, bounded parallel artifact processing, and matching test coverage. ChangesToken report timeout and alert flow
Sequence Diagram(s)sequenceDiagram
participant token_report_yml
participant token_report_sh
participant github_api
participant tracking_issue
token_report_yml->>token_report_sh: Generate org-wide token report
token_report_sh->>github_api: List repos and download token-usage-* artifacts
github_api-->>token_report_sh: Artifact listings and zip contents
token_report_yml->>github_api: Post token_report.md when success() and file exists
token_report_yml->>tracking_issue: Add failure comment when failure() or cancelled()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-27T00:44:43Z. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces parallel processing and timeouts to the artifact collection process in scripts/token_report.sh to prevent hung or slow GitHub API calls from stalling the entire job. The feedback recommends adding error handling for mktemp calls to prevent execution with empty paths, replacing shell glob expansion with find to avoid "Argument list too long" errors, and skipping tests that rely on the timeout command when it is not available on the host system.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-27T00:47:52Z. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-27T01:26:50Z. |
|
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: b5604a09be6efa1e80eaa9f8dfd94fc629cab38d
Review mode: triage-approved (single reviewer)
Summary
Fixes the weekly Token Cost Report timeout (#954). collect_org_jsonl() is reworked from serial/unbounded to two bounded-parallel passes (xargs -P COLLECT_CONCURRENCY) with a per-gh-call timeout wrapper (_gh_timeout, ARTIFACT_OP_TIMEOUT) so a single hung artifact download can no longer consume the whole job. The workflow raises timeout-minutes 30->60 as a safety margin, gates the post step on success(), and adds a 'fail loudly' step that comments on the tracking issue when generation fails or is cancelled. 82 lines of new bats coverage exercise the timeout and parallel-collection paths.
Linked issue analysis
Closes #954. All four proposed fixes are implemented: (1) per-download/per-listing timeout via _gh_timeout; (2) bounded parallelism over repos and artifacts; (3) timeout-minutes 30->60 stopgap; (4) fail-loudly alert step instead of a silent if-skip. Acceptance criteria are met: render functions stay network-free, a single hung download is bounded (covered by a new test asserting elapsed<15s), and a failed/cancelled run now surfaces a visible comment. Per-record repo tagging is preserved.
Findings
No blocking findings. Prior gemini-code-assist recommendations are all addressed in the head SHA: both mktemp calls now have error guards (skip-repo WARN / return 1); pass-2 aggregation uses find -exec cat instead of shell glob (avoids 'Argument list too long'); and the new timeout-dependent tests skip cleanly when the 'timeout' binary is absent. Security: the new github-script step interpolates no untrusted PR input (only static strings and trusted process.env), and the parallel workers pass repo/id as positional args (no shell injection). coderabbit and codex were rate-limited and did not post a review; the rate-limit window (reset 01:26Z) has since passed with no further bot input, and gemini + SonarCloud + CodeQL + gitleaks all provided clean automated coverage.
CI status
All quality gates green: CI Lint/ShellCheck/shellcheck, bats, validate-agent-profiles, gh-aw-compile, Compile agentic workflows, unit-tests, CodeQL (actions+python), SonarCloud, Secret scan (gitleaks), agent-shield, holdout-guard, test-deletion guard. The two CANCELLED checks are the Dev-Lead Agent's own dispatch/ci-relay jobs (self-cancelled via concurrency; ci-relay intentionally [skip]), not PR quality gates. SKIPPED dependency-audit sub-jobs are non-applicable ecosystems. mergeStateStatus is BLOCKED only on the required org-leads review.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #955 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
2 participants
Closes #954
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
Bug Fixes