Skip to content

[DO NOT MERGE] Test harness for the AI review skip gate - #98

Closed
neil-marcellini wants to merge 6 commits into
mainfrom
neil-test-skip-gate
Closed

[DO NOT MERGE] Test harness for the AI review skip gate#98
neil-marcellini wants to merge 6 commits into
mainfrom
neil-test-skip-gate

Conversation

@neil-marcellini

Copy link
Copy Markdown

Details

(Neil's AI agent)

Throwaway PR used to exercise the two scripts added in #97 on a real pull request. It carries that PR's branch plus .github/workflows/testSkipGate.yml, which stands in for an AI reviewer: it calls shouldSkipReview.sh, echoes the gate decision, and calls recordReviewComplete.sh under the same conditions the consumer workflows use.

It uses pull_request (not pull_request_target) so the workflow definition is read from this branch's head rather than main, and it writes the status under the context ai-review-completed/test so it cannot collide with a real reviewer.

Close this without merging once the gate is verified.

Related Issues

https://github.com/Expensify/Expensify/issues/669298

Manual Tests

(Neil's AI agent)

See the run history on this PR.

@neil-marcellini
neil-marcellini marked this pull request as ready for review August 7, 2026 15:24
@neil-marcellini

Copy link
Copy Markdown
Author

(Neil's AI agent) Gate verified on all four events; results recorded in the Manual Tests section of #97. Closing without merging.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d30e9112bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


```yaml
permissions:
statuses: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Grant pull-request read permission

For a private caller repository, copying this workflow-level permission block sets every unspecified permission to none, so shouldSkipReview.sh cannot perform its GET /repos/{owner}/{repo}/pulls/{pull_number} request and the review job fails before reaching the gate. GitHub documents that this endpoint requires the fine-grained Pull requests: read permission, while unauthenticated access is limited to public resources; add pull-requests: read here and to the test workflow. (workflow permission semantics, pull-request endpoint permissions)

Useful? React with 👍 / 👎.

fi

# The combined status endpoint returns only the most recent status per context.
STATE=$(gh api "/repos/$REPO/commits/$HEAD_SHA/status" --jq ".statuses[] | select(.context == \"$CONTEXT\") | .state")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Paginate the commit-status lookup

In repositories where more than one page of commit-status contexts exists and this review context is not on the first page, the filter returns an empty STATE even though a successful marker exists, causing the expensive review and its findings to be posted again. The combined-status endpoint exposes page/per_page, and gh api only follows subsequent pages when --paginate is supplied, so this lookup needs pagination or a targeted equivalent. (combined-status endpoint, gh api --paginate)

Useful? React with 👍 / 👎.

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