Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
name: Release

on:
push:
tags: ["v*"]

permissions:
contents: read

concurrency:
group: release
cancel-in-progress: true

jobs:
build-and-push:
runs-on: ubuntu-latest
permissions:
contents: read
packages: write
outputs:
digest: ${{ steps.build.outputs.digest }}
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false

- uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3

- uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5
id: meta
with:
images: ghcr.io/${{ github.repository }}
tags: |
type=semver,pattern={{version}}
type=raw,value=latest

- uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6
id: build
with:
context: .
push: true
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}

release:
needs: build-and-push
runs-on: ubuntu-latest
permissions:
contents: write
steps:
- uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2
id: app-token
with:
app-id: ${{ secrets.RELEASE_APP_ID }}
private-key: ${{ secrets.RELEASE_PRIVATE_KEY }}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
aliasunder marked this conversation as resolved.
permission-contents: write

- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
token: ${{ steps.app-token.outputs.token }}
ref: ${{ github.ref }}
Comment thread
aliasunder marked this conversation as resolved.
persist-credentials: true

- name: Flip action.yml image to digest-pinned GHCR reference
env:
DIGEST: ${{ needs.build-and-push.outputs.digest }}
IMAGE: ghcr.io/${{ github.repository }}
run: |
sed -i "s|image: Dockerfile|image: docker://${IMAGE}@${DIGEST}|" action.yml
Comment thread
aliasunder marked this conversation as resolved.
Comment thread
aliasunder marked this conversation as resolved.
Comment thread
aliasunder marked this conversation as resolved.
git config user.name "umm-actually[bot]"
git config user.email "umm-actually[bot]@users.noreply.github.com"
git add action.yml
git commit -m "chore(release): pin image to ${DIGEST}"
Comment on lines +70 to +79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[high/correctness] Release workflow fails on second release due to empty commit (confidence: high)

The step "Flip action.yml image to digest-pinned GHCR reference" uses sed to replace "image: Dockerfile" with a pinned image reference. After the first release, action.yml no longer contains that string, so sed makes no change, resulting in no diff. The subsequent git commit command fails with "nothing to commit", breaking the job. Subsequent releases will fail.

Failure scenario: After the first release publishes and pins the image, any further tag push triggers the release workflow, causing the flip step to fail because of the empty commit, preventing the release from completing.

Use sed -i "s|^image: .*|image: docker://${IMAGE}@${DIGEST}|" action.yml to always replace the image line, or check for changes before committing (e.g., git diff --quiet && exit 0).


- name: Force-move semver tag to the digest-flipped commit
env:
TAG: ${{ github.ref_name }}
run: |
git tag -f "$TAG"
git push origin "$TAG" --force

- name: Force-move floating major tag
env:
TAG: ${{ github.ref_name }}
run: |
MAJOR="${TAG%%.*}"
git tag -f "$MAJOR"
git push origin "$MAJOR" --force
Comment thread
aliasunder marked this conversation as resolved.
Comment thread
aliasunder marked this conversation as resolved.

- name: Create GitHub Release
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
TAG: ${{ github.ref_name }}
run: gh release create "$TAG" --generate-notes
54 changes: 54 additions & 0 deletions .github/workflows/self_review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: Self Review

on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
issue_comment:
types: [created]

permissions:
contents: read

concurrency:
group: self-review-${{ github.event.pull_request.number || github.event.issue.number }}
cancel-in-progress: true

jobs:
review:
runs-on: ubuntu-latest
if: >-
github.event_name == 'pull_request' ||
(
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
contains(github.event.comment.body, '@umm review')
)
Comment on lines +19 to +25

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[medium/correctness] Self-review workflow may loop if the bot's own review comment contains '@umm review' (confidence: medium)

The workflow triggers on issue_comment events where the comment contains @umm review. Since the bot itself posts a review comment that could inadvertently include that string (either from the LLM output or from the bot's own markdown), the workflow would re-trigger, potentially causing an infinite loop and wasteful API calls.

Failure scenario: The LLM generates a finding that includes the string "@umm review" in a code suggestion or description; the bot posts the review comment, which triggers the workflow again, leading to repeated runs until the action is cancelled or GitHub rate limits are hit.

Add a condition like github.actor != 'umm-actually[bot]' to prevent the bot from triggering itself.

permissions:
contents: read
Comment thread
aliasunder marked this conversation as resolved.
Comment thread
aliasunder marked this conversation as resolved.
pull-requests: write
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false

- uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2
id: app-token
with:
app-id: ${{ secrets.UMM_APP_ID }}
private-key: ${{ secrets.UMM_PRIVATE_KEY }}
Comment thread
aliasunder marked this conversation as resolved.
permission-contents: read
permission-pull-requests: write

- name: Request review from umm-actually bot
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
run: |
gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/requested_reviewers" \
--method POST -f 'reviewers[]=umm-actually[bot]' || true

- uses: ./
with:
github_token: ${{ steps.app-token.outputs.token }}
openrouter_api_key: ${{ secrets.OPENROUTER_KEY }}
model: ${{ vars.OPENROUTER_MODEL || 'anthropic/claude-sonnet-4-6' }}
Comment thread
aliasunder marked this conversation as resolved.
10 changes: 7 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ action.yml # action metadata — inputs/outputs, runs.using: doc
Dockerfile # multi-stage: build (tsc) → slim runtime
fixtures/ # test fixtures (event payloads, sample diff, LLM responses)
src/
main.ts # entrypoint — collects/validates inputs; pipeline + outputs land next PR
main.ts # entrypoint — collects/validates inputs, wires clients into orchestrate, sets outputs
config.ts # action inputs → validated ActionConfig
logger.ts # structured JSON logger — levels, child contexts, lazy props
github/ # GitHub I/O: event payload → PrContext, octokit wrappers (diff fetch, review posting)
openrouter/ # OpenRouter I/O: @openrouter/sdk wrapper, structured-output retry ladder, cost summary
diff/ # pure transforms over parse-diff output
context/ # workspace I/O: conventions file, changed files, related-files reverse-import scan
review/ # pure review logic: finding schema, phases, prompt, selection, comment mapping
orchestrate.ts # (planned — next PR) the pipeline — fully testable with stub clients
orchestrate.ts # pipeline + createPromptedGenerateFindings — fully testable with stub clients
```

## Module layering
Expand Down Expand Up @@ -90,7 +90,11 @@ files. Prefer SDK-provided types over redefining shapes.
- `const` per test via factory helpers; `beforeEach` only when per-test
creation is genuinely impractical.
- Exact assertions over loose matchers; assert whole values over substrings
when output is deterministic.
when output is deterministic. When fixtures and stubs produce deterministic
results, assert the entire return value or call params — not just individual
fields. Asserting fragments is the cheap option; asserting the whole value
catches drift in formatting, structure, and attribution that field-level
checks miss.
- Two-bar rule: a test must (1) fail when the behavior breaks and (2) pass
only because the intended behavior occurred. Guard against silent no-op,
wrong-error, and early-return passes.
Expand Down
123 changes: 121 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,134 @@

LLM-powered pull request review as a GitHub Action. One consolidated review per PR with inline findings, powered by any model on [OpenRouter](https://openrouter.ai).

> Full documentation, inputs reference, and setup guide land with the first release. This repo is under active initial development.

## What it does

- Reviews the PR diff **and traces changed code into its callers** — regressions and pre-existing bugs in affected code are findings, not noise
- Reads your repo's conventions file (`AGENTS.md` by default) and reviews against it
- Posts exactly **one** PR review with inline comments anchored to diff lines — no duplicate comments, no unrequested-reviewer badges
- Structured output end to end: every finding carries a category, severity, confidence, and a concrete failure scenario
- Model-agnostic via OpenRouter — pick your model, see your per-call costs
- Findings that can't be anchored to the diff (e.g. callers outside the changed files) render in the review body under "Findings beyond the diff"
- PRs with oversized diffs are skipped gracefully with a body-only review stating the reason

## Setup

umm-actually runs as a Docker-based action. It needs a GitHub token (for fetching the diff and posting the review) and an OpenRouter API key.

For the best experience, use a [GitHub App](https://docs.github.com/en/apps/creating-github-apps) installation token so reviews are attributed to a bot identity rather than a personal account.

## Usage

```yaml
name: Review

on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
issue_comment:
types: [created]

permissions:
contents: read

concurrency:
group: review-${{ github.event.pull_request.number || github.event.issue.number }}
cancel-in-progress: true

jobs:
review:
runs-on: ubuntu-latest
if: >-
github.event_name == 'pull_request' ||
(
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
contains(github.event.comment.body, '@umm review')
)
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v7
with:
persist-credentials: false
Comment thread
aliasunder marked this conversation as resolved.

- uses: actions/create-github-app-token@v2
id: app-token
with:
app-id: ${{ secrets.UMM_APP_ID }}
private-key: ${{ secrets.UMM_PRIVATE_KEY }}

- uses: aliasunder/umm-actually@v0
with:
github_token: ${{ steps.app-token.outputs.token }}
openrouter_api_key: ${{ secrets.OPENROUTER_KEY }}
```

The `@umm review` comment trigger lets you re-request a review on any PR by commenting. The `issue_comment` event fires for PR comments — the `if` condition filters to PRs only.

## Inputs

| Input | Default | Description |
| ----------------------- | ----------------------------- | ----------------------------------------------------------------------------------------------------------- |
| `github_token` | _(required)_ | Token for fetching the diff and posting the review. A GitHub App installation token keeps the bot identity. |
| `openrouter_api_key` | _(required)_ | OpenRouter API key |
| `model` | `anthropic/claude-sonnet-4-6` | OpenRouter model slug exactly as listed on openrouter.ai/models |
| `fallback_model` | `""` | Model to retry with if the primary model fails the structured-output ladder |
| `max_findings` | `""` _(uncapped)_ | Cap on posted findings, highest severity first. Empty = all validated findings post. |
| `severity_threshold` | `low` | Minimum severity to post: `low` \| `medium` \| `high` \| `critical` |
| `conventions_file` | `AGENTS.md` | Repo-relative path to the conventions file included in the prompt |
| `phases` | `combined` | Review phases to run. V1 supports: `combined` |
| `context_budget_tokens` | `80000` | Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) |
| `trace_related_files` | `true` | Include files that reference changed files in the prompt so the model can trace regressions into callers |
| `cost_summary` | `true` | Write a per-run cost report (model, prompt/completion tokens, USD) to the workflow step summary |
| `pr_number` | `""` | PR number override — required only when the triggering event does not identify a PR directly |

## Outputs

| Output | Description |
| ---------------- | ------------------------------------------------------------ |
| `findings_count` | Number of findings posted (after threshold and cap) |
| `review_url` | URL of the submitted review; empty when no review was posted |
| `model_used` | Model that produced the accepted response |
| `skipped_reason` | Non-empty when the review was skipped (e.g. diff too large) |

## How it works

1. Resolves the PR from the triggering event (supports `pull_request`, `pull_request_target`, and `issue_comment` events)
2. Fetches the unified diff via the GitHub API — PRs that exceed the API's diff size limit are skipped
3. Reads the conventions file and changed source files (token-budgeted), then traces imports to find related files that reference the changes
4. Builds a structured prompt with randomized delimiter nonces (prompt injection defense) and sends it to OpenRouter
5. Validates the response against a strict Zod schema, retrying with a fallback model if the primary fails
6. Filters findings by severity threshold, deduplicates overlapping findings, and caps if configured
Comment thread
aliasunder marked this conversation as resolved.
7. Maps findings to inline PR review comments anchored to diff lines, with a snap-to-nearest-hunk fallback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[medium/correctness] README claims deduplication of overlapping findings, but no deduplication is performed (confidence: high)

The "How it works" section states that findings are deduplicated after filtering, but the code only uses selectFindings which filters by severity threshold and caps count; no deduplication logic exists. This misleads users about the review quality.

Failure scenario: A user expects the bot to avoid posting duplicate findings for the same issue; they may receive redundant findings, degrading the review's usefulness and violating the documentation promise.

Remove the deduplication claim or implement deduplication logic.

8. Posts one consolidated review — findings that can't be inlined render in the review body

## Status

umm-actually is in early development — the core review pipeline works but there's more to build. Here's what's shipped and what's in progress:

**Shipped (V1)**

- Single-pass review with inline findings anchored to diff lines
- Structured output with retry ladder and fallback model
- Import-tracing: changed code is traced into callers via reverse-import scan
- Token-budgeted context (changed files + related files + conventions)
- Prompt injection defense (randomized delimiter nonces)
- Skip-path handling with posted reasons (oversized diff, empty diff, API limits)
- Cost transparency (per-run model/token/USD report in workflow summary)
- `@umm review` comment trigger for on-demand re-reviews

**In progress**

- **Review dedup on re-runs** — currently each push posts a new review; working on deduplicating findings across runs and updating a single summary comment instead of creating new ones
- **Doc-staleness detection** — extending the workspace scan to doc files (`.md`, `.json`) so unchanged docs that describe changed code reach the prompt and staleness becomes a finding
- **Branded check run** — using the Checks API so the CI check shows the umm-actually avatar instead of the generic GitHub Actions logo

**Planned**

- V1.5: `read_file` verification tool — the model can read additional files before finalizing findings
- V2: bounded agentic exploration — multi-step investigation with tool use behind a `generateFindings` seam

## License

Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ inputs:
required: false
default: combined
context_budget_tokens:
description: Approximate token budget for prompt context (conventions + file contents + diff)
description: Approximate token budget for prompt context (file contents + diff — conventions have a separate cap)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[medium/correctness] action.yml claims conventions have a separate token cap, but no such cap exists (confidence: high)

The input description for context_budget_tokens says conventions have a separate cap, implying they are not counted in this budget. However, the implementation reads the conventions file without any token limit and does not apply a separate cap, potentially allowing an arbitrarily large conventions file to blow up the prompt context beyond model limits.

Failure scenario: A repository has a very large conventions file; the action includes the entire file in the prompt, possibly exceeding the model's context window and causing the LLM call to fail or produce truncated analysis, with no budget safeguard.

Update the description to clarify that conventions are included without a strict cap, or implement a token budget cap for conventions.

required: false
default: "80000"
trace_related_files:
Expand Down
Loading
Loading