Uh oh!
There was an error while loading. Please reload this page.
feat(agent): add proactive pull request review - #19
Conversation
Inspect pull-request diffs from their merge base and validate findings against changed files. Gate automatic fixes by confidence, change risk, repository policy, verification checks, and runner isolation. Closes#9
Uh oh!
There was an error while loading. Please reload this page.
| const files = await this.git(['ls-files']); | ||
| const diff = await this.git(['diff', '--no-ext-diff', '--']); | ||
| const changedFiles = await this.reviewFiles(options); | ||
| const diff = await this.git(['diff', '--no-ext-diff', ...diffRange, '--']); |
There was a problem hiding this comment.
Local review omits pending changes
With no revision range, the local proactive-review path executes plain git diff, which excludes index-only changes and untracked files. The reproduced fixture showed A staged.txt and ?? untracked.txt in Git status while the changed-file list and diff context were both empty, so zero review --proactive cannot review those pending changes. Build range-less local context from status, staged content, working-tree content, and approved untracked files while retaining the current base/head behavior for pull-request reviews.
Artifacts
- Authored Node fixture creates an isolated repository and invokes the exact empty-range `git diff` argument forms from the runner; the takeaway is that the reproduction is focused and independently executable.
Captured fixture source output
- Captured output of `cat trex-artifacts/proactive-empty-range-fixture.mjs` includes the command, working directory, full source, and exit code; the takeaway is that the uploaded test source exactly matches the executed fixture.
Clean repository baseline output
- Executed baseline fixture reports empty porcelain status, empty review files, and empty diff with exit code 0; the takeaway is that the control case behaves as expected.
Staged and untracked fixture output
- Executed changed fixture reports `A staged.txt` and `?? untracked.txt` but empty review files and diff context with exit code 0; the takeaway is that both change types are omitted.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runner/src/boundary.ts
Line: 163
Comment:
**Local review omits pending changes**
With no revision range, the local proactive-review path executes plain `git diff`, which excludes index-only changes and untracked files. The reproduced fixture showed `A staged.txt` and `?? untracked.txt` in Git status while the changed-file list and diff context were both empty, so `zero review --proactive` cannot review those pending changes. Build range-less local context from status, staged content, working-tree content, and approved untracked files while retaining the current base/head behavior for pull-request reviews.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const scoped = scopeChanges( | ||
| decision.changes, | ||
| finding, | ||
| effectiveInput, | ||
| config.agent.maxChangedFiles, |
There was a problem hiding this comment.
Autofix scope exceeds validated finding
For proactive runs, effectiveInput.files contains every file in the review diff and is passed to scopeChanges, allowing any changed file to be written even if the accepted finding and evidence name another file. The executed flow accepted a finding limited to src/supported.ts but wrote and verified src/diff-peer.ts because it was also in the review-file list. Restrict planned writes to files supported by the finding, or require separately validated evidence before authorizing any additional file.
Context Used: CLAUDE.md (source)
Artifacts
- A fake model and runner execute the real AgentZero flow for a supported-file control and a peer-diff-file attempt; takeaway: it isolates the claimed authorization path.
- The fake proactive AgentZero run validates a finding for src/supported.ts and writes only src/supported.ts with exit code 0; takeaway: the baseline in-scope edit succeeds.
- The fake proactive AgentZero run retains a finding only for src/supported.ts but writes src/diff-peer.ts and reports a completed verified result with exit code 0; takeaway: the unsupported peer-diff edit is written.
- Vitest ran packages/agent/src/agent.test.ts with all 36 tests passing and exit code 0; takeaway: the existing suite passes without catching the reproduced bypass.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/agent/src/agent.ts
Line: 138-142
Comment:
**Autofix scope exceeds validated finding**
For proactive runs, `effectiveInput.files` contains every file in the review diff and is passed to `scopeChanges`, allowing any changed file to be written even if the accepted finding and evidence name another file. The executed flow accepted a finding limited to `src/supported.ts` but wrote and verified `src/diff-peer.ts` because it was also in the review-file list. Restrict planned writes to files supported by the finding, or require separately validated evidence before authorizing any additional file.
**Context Used:** CLAUDE.md ([source](https://github.com/wolfstar-project/agent-zero/blob/main/CLAUDE.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
--proactive.Why
Closes#9.
Proactive review belongs across the existing package boundaries: GitHub normalizes webhook events, the runner obtains repository diffs, the agent validates and authorizes findings, config owns repository policy, and the CLI/server remain presentation and transport adapters.
Verification
aube run check:repoaube run lint:ciaube run typecheckaube testaube run buildAll 192 tests pass after rebasing on the current
origin/main.Safety and compatibility
observemode as read-only, or explained the policy change above.Reviewer notes
0.2.0.Need help on this PR? Tag
@codesmithwith what you need. Autofix is enabled.Confidence Score: 3/5
Not safe to merge until local review coverage includes all pending changes and automated edits are limited to evidence-supported files.
Both reported failures were reproduced through focused execution: an isolated Git fixture demonstrated omitted local changes, and an AgentZero harness demonstrated a successful write to a peer diff file not named by the finding.
Files Needing Attention:
packages/runner/src/boundary.tsneeds range-less local context collection that includes staged and untracked content;packages/agent/src/agent.tsand its change-scoping helper need write authorization limited to validated finding files.What T-Rex did
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(agent): add proactive pull request ..." | Re-trigger Greptile
Context used: