Skip to content

ci: finalize the review tracking comment in its own job - #7082

Merged
otavio merged 1 commit into
masterfrom
ci/pr-review-config-reach
Sep 12, 2026
Merged

ci: finalize the review tracking comment in its own job#7082
otavio merged 1 commit into
masterfrom
ci/pr-review-config-reach

Conversation

@otavio

@otavio otavio commented Sep 12, 2026

Copy link
Copy Markdown
Member

Paired with shellhub-io/claude#64, which carries the substance — including a fix for
claude-code-action wiping the synced .claude/ before the review runs, which has meant CI reviews
in this repo have not been reading the repo rules or the code-review skill at all.

What changes here

The tracking-comment finalize moves out of the review job into its own needs: review, if: always()
job. As a step it shared a job with the review it exists to clean up after, so the job-level
timeout — the only kind available inside a composite action — cancelled the finalize along with the
hang that triggered it. A separate job also survives the runner dying, which the step never did. It
checks out only the claude repo, at master, sparsely.

Also drops a three-line comment that restated the commit body, which code-style.md bans — flagged
by the automated review on shellhub-io/cloud#2549.

Testing

actionlint and yamllint clean.

As before, this PR does not exercise its own change: pull_request_target runs the workflow from
the base branch. shellhub-io/cloud#2550 verifies the new path, including whether the reviewer can
now read $GITHUB_WORKSPACE/claude/.claude/skills/code-review/SKILL.md.

Merge order: claude#64, then cloud#2550 (check its run log for that read), then this one.

@otavio
otavio requested a review from a team as a code owner September 12, 2026 14:19
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown

Claude finished @otavio's task in 2m 31s —— View job


Code Review

  • Gathered PR context
  • Reviewing (Standards, Spec, Correctness)
  • Posting feedback
· branch `ci/pr-review-config-reach`

As a step it shared a job with the review it exists to clean up after, so the job-level timeout —
the only kind available inside a composite action — cancelled the finalize along with the hang that
triggered it. A separate job also survives the runner dying, which the step never did.

The job takes the claude branch the review job resolved, falling back to master when the review was
cancelled before resolving one. Pinning master outright would mean a change to the finalize action
could never be tested from a paired branch — as this very PR showed, by failing against a master
that did not have the action yet.

Also drops a comment that restated the commit body, which code-style.md bans.
@otavio
otavio force-pushed the ci/pr-review-config-reach branch from 42f9674 to b37abb7 Compare September 12, 2026 14:26
@otavio
otavio merged commit 3865323 into master Sep 12, 2026
41 checks passed
@otavio
otavio deleted the ci/pr-review-config-reach branch September 12, 2026 14:44
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.

1 participant