Skip to content

ci: keep build timing report non-blocking when PR comment fails#592

Open
ainta wants to merge 1 commit into
Verified-zkEVM:mainfrom
ainta:ci/timing-report-comment-nonblocking
Open

ci: keep build timing report non-blocking when PR comment fails#592
ainta wants to merge 1 commit into
Verified-zkEVM:mainfrom
ainta:ci/timing-report-comment-nonblocking

Conversation

@ainta

@ainta ainta commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the comment upsert in the Upsert build timing PR comment step in try/catch so a failure to post/update the PR comment no longer fails the whole Build Timing Report job.
  • On HTTP 403, emit a core.warning that points at the root cause (the workflow_run token permission cap) instead of an unhandled error; on any other error, warn with the message.
  • No behavior change when the comment posts successfully.

Context

The reporter added in #490 renders the timing report fine, then fails at the comment step with:

POST /repos/Verified-zkEVM/ArkLib/issues/<n>/comments - 403
##[error]Unhandled error: HttpError: Resource not accessible by integration

Reproduced on three recent PRs:

The workflow declares issues: write (line 15), but because this is a workflow_run job, GITHUB_TOKEN is capped at the repository's default token permissions, so that declaration has no effect while the repo default is read-only. The rendered report is already written to $GITHUB_STEP_SUMMARY in the previous step, so the comment is purely cosmetic — but the unhandled 403 surfaces as a red X on otherwise-green CI and sends failure notifications. (workflow_run runs aren't required checks, so this never blocked merges, e.g. #516 merged with the failure present.)

This PR makes the reporting job resilient. To actually restore the PR comment, a maintainer still needs to enable write access under Settings → Actions → General → Workflow permissions (or supply a token/app granting issues: write); the warning emitted on 403 documents this inline.

Verification

  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/build-timing-report.yml'))" — parses cleanly after the change.
  • Confirmed the 403 and the affected step/PRs from the failed Build Timing Report run logs above.
  • Change is scoped to the comment-upsert step; the render and artifact steps are untouched.

Posted by Claude Code (model: Opus 4.8) on behalf of the user (@ainta) with approval. Made with Claude Code.

The "Upsert build timing PR comment" step throws on any GitHub API error,
which fails the whole Build Timing Report job. In practice the comment call
returns HTTP 403 "Resource not accessible by integration": this is a
`workflow_run` job, so GITHUB_TOKEN is capped at the repository's default
token permissions and the `issues: write` declared in the workflow cannot
take effect. The rendered report is already attached to the job summary, so
the comment is cosmetic, yet the failure shows up as a red X on green CI.

Wrap the comment upsert in try/catch: on failure emit a core.warning (with a
pointer to the Actions workflow-permission setting on 403) and let the job
finish green. No behavior change when the comment posts successfully.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Summary

Infrastructure / CI

  • Wraps the PR comment logic in .github/workflows/build-timing-report.yml in a try/catch block to prevent the workflow from failing if the comment cannot be posted or updated.
  • Converts HTTP 403 errors (typically caused by workflow_run permission limitations) and other API failures into descriptive warnings instead of hard errors.
  • Ensures the CI status remains green and provides the build timing report via the GitHub Step Summary even when repository permissions prevent the cosmetic PR comment.

Statistics

Metric Count
📝 Files Changed 1
Lines Added 42
Lines Removed 21

Lean Declarations

  • No declarations were added, removed, or affected.

sorry Tracking

  • No sorrys were added, removed, or affected.

📋 **Additional Analysis**

No findings.


📄 **Per-File Summaries**
  • .github/workflows/build-timing-report.yml: This change wraps the build timing comment logic in a try-catch block to prevent workflow failures when the GITHUB_TOKEN lacks sufficient permissions to post PR comments. Instead of a hard failure, it now logs a descriptive warning to ensure the CI status remains green, as the report is still available in the job summary.

Last updated: 2026-06-26 12:51 UTC.

@quangvdao

Copy link
Copy Markdown
Collaborator

Thanks for diagnosing the noisy failure. I’d like to retain the PR comment, so please revise this PR to incorporate the permission correction while keeping the reporting step resilient:

  • change pull-requests: read to pull-requests: write;
  • remove issues: write so the workflow has only the write scope it actually uses;
  • preferably scope these permissions to the report job;
  • keep expected comment API failures non-blocking and emit the HTTP status/message;
  • do not catch every exception indiscriminately—unexpected JavaScript/programming errors should still fail so genuine regressions remain visible;
  • update the PR explanation: GitHub permits workflow-level permissions to override the repository’s restricted default, so the current “default token cap” diagnosis is not quite right.

I’ve closed #632 in favor of consolidating both fixes here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants