Separate PR fetch source from review engine

[Iron-Ham]

Summary

• Add explicit --fetch local|github PR content source selection and share GitHub PR analysis between pr and local review.
• Add explicit --engine local|hosted review execution selection so hosted review is no longer implied by --remote.
• Make local inspect pr <n> use PR commit OIDs and a force-safe pull/<n>/head fetch fallback.
• Update README and docs command references.

Closes #16

Test plan

• rustfmt --edition 2021 --check crates/inspect-cli/src/commands/pr.rs crates/inspect-cli/src/commands/pr_source.rs crates/inspect-cli/src/commands/review.rs
• cargo test --workspace
• cargo build -p inspect-cli
• target/debug/inspect pr --help
• target/debug/inspect review --help
• target/debug/inspect pr 18 --fetch github --remote Ataraxy-Labs/inspect --format json
• target/debug/inspect pr 18 --format json
• target/debug/inspect review 18 --fetch github --remote Ataraxy-Labs/inspect --min-risk low --max-entities 0 --format json
• target/debug/inspect review 18 --remote Ataraxy-Labs/inspect (expected error)
• target/debug/inspect review 18 --engine hosted --fetch github --remote Ataraxy-Labs/inspect (expected error)

[vercel]

@Iron-Ham is attempting to deploy a commit to the rs545837's projects Team on Vercel.

A member of the Team first needs to authorize it.

[inspect-review]

inspect review

Triage: 42 entities analyzed | 0 critical, 0 high, 22 medium, 20 low
Verdict: standard_review

Findings (5)

1. [low] In analyze_local_pr, the function uses base_ref_oid as fallback to base_ref_name, but then passes these values directly to git commands. If base_ref_oid is a commit SHA, it should be preferred over base_ref_name (branch name) for accuracy, but the fallback order is reversed - it tries OID first, then falls back to name, then to 'main'. This means if both exist, it uses the OID, but the comment in analyze_github_pr says 'Use commit SHAs instead of branch names' for reliability. The logic should prefer OIDs consistently.
2. [low] In run function in pr.rs, when args.base.is_some() || args.head.is_some() is true but only one of them is provided, the code calls analyze_explicit_range which will return an error. However, the error is wrapped in and_then(|result| result.ok_or_else(...)) which expects analyze_explicit_range to return Ok(None) when base/head are missing. But analyze_explicit_range returns Err in those cases, not Ok(None). This means the error message 'missing --base/--head' will never be shown - instead the actual error from analyze_explicit_range will be shown.
3. [low] In validate_review_mode, when engine == ReviewEngine::Hosted and fetch != FetchSource::Local, it returns an error saying to 'omit --fetch github'. However, the default value for fetch is FetchSource::Local (from PrArgs and ReviewArgs). This validation will only trigger if the user explicitly passes --fetch github with --engine hosted, which is correct. But the error message is confusing because it says 'hosted reviews fetch PR content through the inspect API' which implies fetch is ignored, yet it's checking and rejecting non-Local values. This could be a logic error if the intent was to ignore the fetch parameter rather than reject it.
4. [low] In ensure_local_pr_commits, when constructing the error message, if pr.base_ref_name is None but the base commit doesn't exist, the commands vector will be empty for the base ref, but the error message will still claim 'PR commits are not available locally' without providing a way to fetch the base. This creates an unrecoverable error state.
5. [low] In the pr.rs run function, the logic checks if args.base.is_some() || args.head.is_some() and then calls analyze_explicit_range which can return Ok(None) when base is None. However, the subsequent .and_then(|result| result.ok_or_else(|| "missing --base/--head".to_string())) will convert this to an error. But the outer condition already checked that at least one of base/head is Some, so if only head is provided, it will enter this branch, call analyze_explicit_range(repo, None, Some(head)), which returns Err("--head requires --base"), not Ok(None). The error message "missing --base/--head" is unreachable.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 38 entities analyzed | 0 critical, 0 high, 19 medium, 19 low
Verdict: standard_review

Findings (6)

1. [low] In analyze_local_pr, the function falls back to branch names when commit OIDs are missing, but then passes these potentially non-existent branch names to ensure_local_pr_commits which checks if commits exist. If the PR is from a fork and only branch names are available (no OIDs), the branch names won't exist locally, causing the commit existence check to fail even though the function should attempt to fetch them first.
2. [low] In ensure_local_pr_commits, when a commit doesn't exist and fetch fails, the function still checks commit_exists again at the end. However, if the fetch failed, the commits still won't exist, so the function will always return an error even though it already knows the fetch failed. The logic should short-circuit after attempting fetches rather than re-checking.
3. [low] In run function in pr.rs, when args.base.is_some() || args.head.is_some() is true but only one of them is provided, analyze_explicit_range will return an error. However, the error is then passed through .and_then(|result| result.ok_or_else(|| "missing --base/--head".to_string())) which would never trigger because analyze_explicit_range already returns Err for this case, not Ok(None). The error message "missing --base/--head" is unreachable.
4. [low] In ensure_local_pr_commits, when base_ref_name is used to fetch but the commit still doesn't exist, the function tries to generate a fetch command using base_ref which may be a commit OID (from the earlier fallback logic), not a branch name. Running git fetch origin <commit-oid> will fail.
5. [low] In validate_review_mode, when engine == ReviewEngine::Hosted and fetch != FetchSource::Local, the error message says to 'omit --fetch github', but the actual check is fetch != FetchSource::Local which would also trigger for any future fetch sources, making the error message potentially incorrect.
6. [low] In analyze_explicit_range, when only head is provided without base, the function returns Err("--head requires --base"). But when only base is provided without head, it returns Err("--base requires --head"). However, the outer logic in pr.rs checks args.base.is_some() || args.head.is_some(), so if only base is provided, it enters this branch but then gets an error. The Ok(None) return path is only hit when both are None, but that case is already filtered out by the outer condition.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes

[inspect-review]

inspect review

Triage: 37 entities analyzed | 0 critical, 0 high, 18 medium, 19 low
Verdict: standard_review

Findings (5)

1. [low] In analyze_local_pr, the function falls back to branch names when commit OIDs are missing, but then passes these branch names to ensure_local_pr_commits which expects commit SHAs. If the branch names don't exist as commits locally, commit_exists will fail even though the commits might exist under different refs.
2. [low] In ensure_local_pr_commits, when commit_exists(repo, base) returns false and base_ref_name is Some, the function attempts to fetch but doesn't verify the fetch actually made the base commit available. The function only checks commit_exists again after all fetches, so if the base_ref_name doesn't contain the base_ref_oid, the error message will be misleading.
3. [low] In run function in pr.rs, when args.base.is_some() || args.head.is_some() is true but only one of them is provided, analyze_explicit_range will return an error. However, the code then calls .and_then(|result| result.ok_or_else(|| "missing --base/--head".to_string())) which would never trigger because analyze_explicit_range already returns Err for this case, not Ok(None). This creates unreachable error handling.
4. [low] In analyze_explicit_range, when base is None and head is Some, the function returns an error. But when base is Some and head is None, it also returns an error. However, the function signature returns Result<Option<ReviewResult>, String> and returns Ok(None) when both are None. This asymmetry means the caller's .ok_or_else() in pr.rs will never execute because all error cases already return Err, not Ok(None).
5. [low] In ensure_local_pr_commits, when fetching the PR head ref fails, the function still checks commit_exists(repo, head) where head is the original ref (potentially a branch name or OID). However, the fetch creates a new ref at refs/remotes/inspect/pr-{number}, so the commit check should verify that the fetched ref exists, not the original head value.

---

Reviewed by inspect | Entity-level triage found 0 high-risk changes