Skip to content

Review every pull request with GLM - #238

Open
itz4blitz wants to merge 4 commits into
mainfrom
ci/glm-pr-review
Open

itz4blitz wants to merge 4 commits into
mainfrom
ci/glm-pr-review

Conversation

@itz4blitz

Copy link
Copy Markdown
Collaborator

What changed and why

GLM-5.3 now reviews every pull request, the way it does on palamhealth, but written for Tiny. It posts an inline review (findings on the exact line, one-click suggestions where the fix is one edit), a sticky status comment, and the check Tiny Agents / Code Review. It is advice: nothing requires the check. tinyserver will call the same reusable workflow in a follow-up pull request.

Type

  • feature
  • fix
  • perf
  • refactor
  • chore
  • docs

Platforms

  • windows
  • macos
  • linux
  • android
  • ios

Proof

Not a feature or fix for the app; no scenario. What was run instead:

Scenario: none (chore)

Compatibility

No app, protocol or server change. Adds workflows, a script and docs only. Needs the Actions secret AI_API_KEY (already set on tinyclient and tinyserver).

Risk and rollback

Posts comments on pull requests and a check run, so a wrong review is noise, not breakage. It never checks out or runs the branch (pull_request_target, diff read through the API), only reviews pull requests by owners, members and collaborators on its own, and defangs @ mentions in what the model says. Roll back by deleting pr-review.yml. To stop spend, delete the secret.

Release note

None (CI only).

🤖 Generated with Claude Code

itz4blitz and others added 3 commits November 11, 2026 09:37
GLM-5.3 reads each pull request and posts what it finds as an inline
review, a sticky status comment and the check "Tiny Agents / Code Review".
Advice, not a gate: a blocker turns the check red, a provider outage is
neutral, and nothing requires it.

The script is standard-library Python (.github/scripts/pr-review.py), so
the workflow installs nothing. The work is a reusable workflow,
pr-review-run.yml, which tinyserver calls too; the instructions are in
.github/review/ (base, plus one file per repository). It runs on
pull_request_target and never checks out or runs the branch.

docs/pr-review.md has triggers, setup and how to add a repository.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…utage, test the GitHub calls

Found by the first run on a real runner: post_json passed input= to gh_json,
which did not take it. The tests had no fake gh, so nothing exercised the posting
path; they do now. A provider outage warns and exits clean so advice that could not
be given leaves no red cross; the neutral check run says why.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Tiny Code Review

🟡 2 major findings — see the review · run

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny Code Review

🟡 2 findings (0 blocking).

The change is CI-only and mostly careful about the key (no checkout of PR code, anchoring, mention defanging), but pr-review.yml declares both pull_request and pull_request_target triggers, which contradicts the design's central safety property (only the default branch's copy runs with the key) and makes the workflow-safety test in this very PR fail. A second real defect routes tooling_ref through github.head_ref on pull_request_target events, so a fork branch name is checked out of tinyclient and any fork/collaborator PR reviewed via label fails.

Residual risk

  • concurrency with cancel-in-progress: true means a new push or a /review comment cancels an in-flight review of the same PR; that is a documented choice but worth watching.
  • Anyone who can dispatch workflows can run an arbitrary branch's tooling with AI_API_KEY; documented and limited to trusted actors.
  • The model's review is posted verbatim (with @ defanging) to public PR pages; prompt-injection defenses are prompt-based and unenforceable in principle.

Agent run

Comment thread .github/workflows/pr-review.yml Outdated
# review itself is tried from its branch by dispatch (`gh workflow run pr-review.yml --ref
# <branch> -f pr_number=N`), which uses the branch's copy of both files and its tooling.
on:
pull_request:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Major

The workflow fires on plain pull_request as well as pull_request_target. A pull_request event runs the branch's own copy of the workflow and its uses: ./.github/workflows/pr-review-run.yml, i.e. the branch's code, not the default branch's copy — exactly what the file's own comment and docs/pr-review.md promise never happens. For same-repo pull requests secrets are available to pull_request runs, so a collaborator's branch executes its own workflow with AI_API_KEY. It also contradicts the PR's own test test_the_trigger_only_runs_the_default_branchs_copy, which asserts "\n pull_request:" is not in the file — the test as committed fails, and every run double-fires (two events per push, only deduplicated by the concurrency group).

Why this diff causes it: The on: block lists pull_request: with types [opened, reopened, ready_for_review, synchronize] alongside pull_request_target:; test_pr_review.py's Workflows.test_the_trigger_only_runs_the_default_branchs_copy does self.assertNotIn("\n pull_request:", text), which now fails against this file.

Smallest fix: Delete the pull_request: trigger (and its types line) so only pull_request_target, issue_comment and workflow_dispatch fire, matching the comment, the docs and the test.

Suggested change
pull_request:
pull_request_target:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 494d716. This came from the temporary test trigger (a pull_request event, and github.head_ref as the tooling ref) that let the workflow review its own PR from the branch. It is reverted: the file now triggers on pull_request_target, issue_comment and workflow_dispatch only, and only a dispatch can override tooling_ref. test_the_trigger_only_runs_the_default_branchs_copy passes again.

Comment thread .github/workflows/pr-review.yml Outdated
with:
pr_number: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }}
# A dispatch tries the branch's own script and instructions; everything else trusts main.
tooling_ref: ${{ github.head_ref || (github.event_name == 'workflow_dispatch' && github.ref_name) || 'main' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Major

On any pull_request_target event (open, ready, synchronize, and the agent:review label) github.head_ref is the source branch of the pull request. The reusable workflow checks out TinyComms/tinyclient at that ref — but for a pull request from a fork (or any branch name that does not exist in tinyclient) there is no such ref in tinyclient, so the checkout fails and the review of exactly the pull requests a maintainer asked for by label (fork PRs from collaborators) always ends as 'review unavailable'. Only workflow_dispatch should override the ref; pull_request_target events must use main.

Why this diff causes it: pr-review-run.yml does repository: TinyComms/tinyclient with ref: ${{ inputs.tooling_ref }}; docs/pr-review.md says a maintainer uses the label or /review for a stranger's pull request, and those run as pull_request_target/issue_comment, where github.head_ref is the PR author's branch name.

Smallest fix: Use the branch only for dispatch: tooling_ref: ${{ (github.event_name == 'workflow_dispatch' && github.ref_name) || 'main' }}.

Suggested change
tooling_ref: ${{ github.head_ref || (github.event_name == 'workflow_dispatch' && github.ref_name) || 'main' }}
tooling_ref: ${{ (github.event_name == 'workflow_dispatch' && github.ref_name) || 'main' }}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 494d716. This came from the temporary test trigger (a pull_request event, and github.head_ref as the tooling ref) that let the workflow review its own PR from the branch. It is reverted: the file now triggers on pull_request_target, issue_comment and workflow_dispatch only, and only a dispatch can override tooling_ref. test_the_trigger_only_runs_the_default_branchs_copy passes again.

The workflow ran from this branch once, on a runner, and reviewed this pull
request. It goes back to pull_request_target, issue_comment and
workflow_dispatch only.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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