expo / expo/code-review-cli

verify pass reads the local checkout instead of the PR head tree

Open
#54 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
1
Forks
0
Avg merge
1d 21h
Merged PRs (30d)
2

Description

Seen on 0.9.2 running `ecr review --pr --stack-aware` locally against expo/universe (stacked PRs 29434-29439).

The verify pass (and sometimes reviewers) read the local checkout instead of the materialized PR head tree. Concrete cases:

On universe#29437 the verifier dropped two findings with "No env.production.yaml exists, only env.staging.yaml" and "mise.toml has no deploy:production task, line 41 hardcodes --vpc-connector=pipeline-staging". The PR adds exactly those files and changes that line. It was describing main, not the PR head.

On universe#29434 the verifier dropped a finding saying "production main.tf lines 32-36 list only 10.8.0.0/16, 172.22.0.0/18, 172.22.64.0/20, the 10.10.0.0/28 entry exists only in staging". The diff adds 10.10.0.0/28 to the production file.

On universe#29436 (stacked on 29434) the security agent reported the secret production__cf_logs_kafka as "only declared for staging" even though the head branch includes the 29434 commit that adds it to production.

In these runs the dropped findings happened to be wrong anyway, but a verifier reading the wrong tree will eventually drop a true finding for a false reason, and it already produced one artifact warning (the 29436 secret one). No "falling back to your checked-out files" warning was printed, so the fallback was either silent or the verify pass never got the pinned tree in the first place.

Contributor guide

No contributing guide indexed for this repository

Research direction

Reproduce `ecr review --pr --stack-aware` against stacked PRs 29434-29439, then trace how the verify pass selects the checkout versus the materialized PR head tree. Done means verification and reviewers consistently inspect the pinned PR head, including inherited commits such as 29434 in 29436, without a silent fallback to local files.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
cli, devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.