Skip to content

feat: add mongodb-query-index-check action#3

Merged
mtrunkat merged 5 commits into
mainfrom
claude/mongodb-query-index-action-rmvAP
May 16, 2026
Merged

feat: add mongodb-query-index-check action#3
mtrunkat merged 5 commits into
mainfrom
claude/mongodb-query-index-action-rmvAP

Conversation

@mtrunkat
Copy link
Copy Markdown
Member

Summary

New reusable composite action mongodb-query-index-check that runs on every PR in repos that touch the Apify MongoDB cluster (apify-core, apify-proxy, apify-web, …) and flags MongoDB queries that don't use an appropriate index.

How it works:

  1. Pre-filter (cheap): pages through pulls.listFiles, applies configurable paths / paths-ignore globs, and greps for MongoDB collection-method calls (.find, .findOne, .aggregate, .update*, .delete*, .findOneAnd*, .countDocuments, .distinct, …) in added lines only. If nothing matches, the action sets should-run=false and exits before spending Anthropic credits.
  2. Source resolution: sparse-checkouts apify/apify-core@develop (just src/packages/mongo-indexes/src/*, depth 1) into ${RUNNER_TEMP}/apify-core, or points at a local path inside the workspace (for apify-core's own use).
  3. Claude review: invokes anthropics/claude-code-action@v1 with a tight tool allowlist (GitHub MCP for pull-request read + pending review, Read, Write, read-only Bash) and a tightly-scoped prompt (in prompts/review.md). Claude reads the diff, cross-references each MongoDB call against the canonical index definitions, applies an ESR-aware rubric (🔴 critical / 🟠 high / 🟡 medium / 🟢 low), and posts inline review comments. Stays silent if nothing to flag.
  4. Finalize: reads the single-word severity Claude wrote to ${RUNNER_TEMP}/mongo-index-result.txt. Fails the check when request-changes: true and the severity meets the threshold; otherwise succeeds.

Key design choices:

  • Strictness (severity-threshold + request-changes inputs) is configurable per consuming repo — default high + request-changes: true, so Critical/High findings block merge while Medium/Low stay advisory. Repos doing a gentler rollout can flip request-changes to false and only get comments.
  • Bot identity: default ${{ github.token }}. Comments come from github-actions[bot].
  • Index source: sparse-checkout of apify-core (default) or local path. The npm package option was skipped for v1 — @apify-packages/mongo-indexes ships compiled .js + .d.ts and loses the comments that explain each index's intent, which materially degrades the review. Can be added later.
  • False-positive guardrail: the pre-filter regex is permissive (.find( matches both Users.find and array.find), so Claude does the disambiguation by looking at the receiver. Errs on running more often, never less.

Files

mongodb-query-index-check/
├── action.yaml            # composite action
├── README.md              # usage + examples for apify-core / apify-proxy / apify-web
├── index.mts              # preCheck() + glob & diff helpers
├── index.test.mts         # 31 vitest cases covering globs, diff parsing, and preCheck
└── prompts/review.md      # Claude prompt template (envsubst-rendered)

Test plan

  • pnpm run lint — 0 errors
  • pnpm run type-check — clean
  • pnpm run test — 31/31 tests passing (globToRegex, matchesAny, extractAddedLines, hasMongoCallInDiff, preCheck)
  • python3 -c "import yaml; yaml.safe_load(...)"action.yaml valid
  • envsubst placeholder allowlist matches every $VAR in prompts/review.md
  • End-to-end smoke: requires merging + wiring a consumer workflow in apify-core (or similar) with a PR that adds an unindexed Users.findOne to confirm Claude opens the expected review. Easiest to do once this lands as @v1.1.0 (or whatever release-please cuts).

Follow-ups (not in this PR)

  • Add consumer workflows in apify-core, apify-proxy, apify-web once the action ships a release tag.
  • Optional package source (download sources from a GitHub release of @apify-packages/mongo-indexes) if some repo can't get cross-org read access to apify-core.

Generated by Claude Code

New composite action that flags MongoDB queries in PR diffs lacking a
matching index. Cheap pre-filter pages through `pulls.listFiles`, applies
include/exclude globs, and only triggers the Claude review when MongoDB
collection-method calls appear in added lines.

The Claude prompt cross-references each query against the canonical
`mongo-indexes` definitions (sparse-fetched from apify/apify-core or
loaded from a path inside the workspace), classifies findings on an
ESR-aware rubric (critical / high / medium / low), and posts inline
review comments via the GitHub MCP server. A configurable
`severity-threshold` plus `request-changes` toggle decide whether the
check fails — letting consumers opt into the strictness that suits their
rollout pace.
Copilot AI review requested due to automatic review settings May 15, 2026 21:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a reusable composite GitHub Action that pre-filters PR diffs for MongoDB query changes, fetches canonical index definitions, and invokes Claude Code to review query/index compatibility.

Changes:

  • Adds the mongodb-query-index-check composite action workflow.
  • Adds pre-check helpers and Vitest coverage for glob matching, diff parsing, and PR file filtering.
  • Adds documentation and the Claude review prompt used by the action.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
mongodb-query-index-check/action.yaml Defines inputs, pre-check, index source resolution, Claude invocation, and final severity handling.
mongodb-query-index-check/index.mts Implements glob parsing, added-line extraction, MongoDB call detection, and PR pre-check logic.
mongodb-query-index-check/index.test.mts Adds tests for helper functions and pre-check behavior.
mongodb-query-index-check/prompts/review.md Provides Claude instructions for MongoDB index review and PR commenting.
mongodb-query-index-check/README.md Documents usage, inputs/outputs, behavior, severity rubric, and limitations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mongodb-query-index-check/action.yaml Outdated
Comment thread mongodb-query-index-check/action.yaml Outdated
Comment thread mongodb-query-index-check/index.mts Outdated
Comment thread mongodb-query-index-check/prompts/review.md Outdated
@mtrunkat mtrunkat marked this pull request as draft May 15, 2026 21:15
claude added 2 commits May 15, 2026 21:22
- Replace hand-rolled sparse-checkout + manual credential header injection
  with actions/checkout@v6 (uses path inside $GITHUB_WORKSPACE, the same
  pattern git-cliff-release already uses). Fixes a related bug where the
  raw sparse pattern src/.../src/* only matched direct children.
- Merge the three source-resolution steps (apify-core branch + local
  branch + result-file init) into one Resolve step that branches on
  source. Hoist RESULT_PATH into $GITHUB_ENV so Render and Finalize
  share a single source of truth.
- Merge the two Validate steps into one with set -euo pipefail and
  env-based interpolation for shell-injection safety.
- Drop the uuidgen dependency for the heredoc delimiter.
- Strengthen the makeFakeGithub test mock to assert paginate.iterator is
  invoked with pull_number/owner/repo/per_page (previously it silently
  ignored its arguments, hiding real bugs).
- Strip WHAT-comments from globToRegex.
Four small fixes addressing inline review comments and one self-found
runtime bug:

* Seed RESULT_PATH in the Validate step (always runs) so the Finalize
  step — which is gated by `if: always()` — never bombs on `set -u`
  when an upstream step fails. Previously RESULT_PATH was only written
  inside Resolve, so a checkout failure on apify-core would mask the
  real error with "RESULT_PATH: unbound variable".
* Reject pull requests opened from forks. On `pull_request_target` the
  workflow would otherwise hand a write-capable token to Claude while
  it analyses attacker-controlled diff content (prompt-injection
  vector); on `pull_request` secrets aren't exposed to forks anyway.
* Validate `request-changes` strictly against `true|false`. Previously
  a typo or `True` would silently disable blocking findings.
* Pre-filter (`hasMongoCallInDiff`) now scans the after-state of every
  hunk that contains at least one addition, not just `+` lines. This
  catches changes that add or modify filter/sort fields inside an
  existing `.findOne(`/`.aggregate(` call whose method-call token is
  on an unchanged context line. The Claude prompt was updated to match.
* Tests cover the new behaviour: context-line method call with an
  added filter field is detected, deletion-only hunks remain ignored.
Copy link
Copy Markdown
Member Author

@mtrunkat mtrunkat left a comment

Choose a reason for hiding this comment

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

Requesting changes

Comment thread mongodb-query-index-check/prompts/review.md Outdated
Comment thread mongodb-query-index-check/action.yaml Outdated
Comment thread mongodb-query-index-check/action.yaml Outdated
Comment thread mongodb-query-index-check/action.yaml Outdated
claude and others added 2 commits May 16, 2026 13:18
Address PR review feedback: collapse the action surface down to a single
opinionated path and let the workflow defaults do the rest.

* Drop `mongo-indexes-source`, `mongo-indexes-path`, and `apify-core-ref`
  inputs. The action now auto-detects the source from `apify-core-token`:
  when the token is set it sparse-checkouts `apify/apify-core@develop`,
  when empty it expects to be running on apify-core itself and reads
  `src/packages/mongo-indexes/src` from the workspace.
* Drop `claude-model` input. The action hardcodes `--model claude-opus-4-7`
  to always run on the most capable model.
* Drop `paths-ignore` input. The default exclusion list (node_modules,
  dist, build, tests, the mongo-indexes package itself) is now baked
  into the pre-check step's env block — same behaviour, one fewer knob.
* Drop `extra-prompt` input and the corresponding `$EXTRA_PROMPT` line
  in prompts/review.md. The prompt is no longer extensible per-repo.
* Update README to reflect the trimmed input surface; usage examples
  collapse to either "no apify-core-token (= running on apify-core)"
  or "apify-core-token (= other repo)".

Inputs go from 14 → 7. No behavioural change for callers using the old
defaults; callers that set the removed inputs will need to drop them.
#4)

Address PR review feedback on action.yaml line 33: collapse the
strictness surface to a single `request-changes` toggle.

* Drop `severity-threshold` input.
* Drop the `severity_rank()` bash function and the threshold/rank
comparison in the Finalize step. Finalize now just fails when
`request-changes: true` and Claude reported any severity other than
`none`.
* Drop `$SEVERITY_THRESHOLD` from the envsubst allowlist and the prompt
template. The "Decide the review event" section collapses to a single
conditional on `$REQUEST_CHANGES_MODE`.
* Drop the now-vestigial `$RESULT_PATH` file-exists fallback — the
Validate step always seeds the file before Finalize can run.
* Update README inputs table and how-it-works prose.

Inputs go from 7 → 6. No behavioural change for callers using the old
defaults; callers that set `severity-threshold` will need to drop it.

Co-authored-by: Claude <noreply@anthropic.com>
@mtrunkat mtrunkat marked this pull request as ready for review May 16, 2026 13:34
@mtrunkat mtrunkat merged commit e288951 into main May 16, 2026
2 checks passed
@mtrunkat mtrunkat deleted the claude/mongodb-query-index-action-rmvAP branch May 16, 2026 13:37
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.

4 participants