Skip to content

Send doghouse the reason a scan is not incremental - #185

Open
Ibrahimrahhal wants to merge 3 commits into
mainfrom
cursor/incremental-skip-reason-3620
Open

Ibrahimrahhal wants to merge 3 commits into
mainfrom
cursor/incremental-skip-reason-3620

Conversation

@Ibrahimrahhal

@Ibrahimrahhal Ibrahimrahhal commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Why

When corgea scan blast analyzes every file, it prints why ("Scanning every file: ..."), but the server never hears about it. So a full scan that someone expected to be incremental can't be explained once the terminal output is gone.

Change

  • Every way an upload ends up without a diff now produces a FullScanReason: a stable cause code plus the sentence the run already prints. That includes --disable-incremental and --target/--only-uncommitted, which never reach the baseline lookup and are still silent in the terminal.
  • upload_zip sends these as incremental_skipped_reason and incremental_skipped_detail whenever it sends no diff fields. Doghouse stores them in the scan's internal meta_info and treats disabled_by_flag as an opt-out of its own SCM-integration diff (Corgea/doghouse#2311).
  • Codes: disabled_by_flag, targeted_upload, no_baseline_scan, baseline_lookup_failed, baseline_checksums_unreadable, exclude_needs_checksums, dirty_worktree, no_git_commit, baseline_has_no_commit, baseline_commit_not_in_clone, git_diff_failed, submodule_moved, too_many_changed_files. They are a wire contract: new ones can be added, but shipped ones must not be renamed. A unit test pins them.
  • When the checksum diff and the git diff both refuse, the detail keeps both clauses, the same as the printed line. For the code:
    • If the baseline stored no checksums (the normal case for older scans), the git diff's refusal is the code.
    • If the checksums exist but could not be downloaded or decoded, or are an unknown version, the code is baseline_checksums_unreadable. That fault is why a diff that should have worked did not.
  • The internal Err(String) refusals in incremental.rs became FullScanReason, and UploadOptions.incremental is now Result<IncrementalPlan, FullScanReason>, so no full-scan path can leave out its reason. The "Scanning every file" line is now printed in one place in blast.rs. Terminal output is unchanged.

Known limitation

On a dirty or --exclude run, the baseline lookup skips baselines that have a commit but no checksums. If those were the only candidates, the run reports no_baseline_scan. That message logic predates this change and is left alone here.

Tests

  • ./harness check: clippy (strict), fmt, and 954 tests passed.
  • The e2e cases in tests/cloud_commands_e2e/scan_incremental.rs now check the reason fields for these cases: no baseline, a failed lookup, --disable-incremental, --target, --exclude without checksums, a dirty tree without checksums, and a dirty tree with unreadable checksums. They also check that incremental uploads (checksum diff and git diff) send neither field.
  • A unit test checks that each git-diff refusal reports its own cause.

Deploy notes

None. Older servers ignore the extra fields. No version bump is included.

Open in Web Open in Cursor 

cursoragent and others added 3 commits October 5, 2026 16:40
Every way a blast upload ends up analyzing every file now carries a stable
cause code and the sentence the run printed, as incremental_skipped_reason
and incremental_skipped_detail. That includes --disable-incremental and
--target/--only-uncommitted, which never reach the baseline lookup.

Co-authored-by: ibrahim <ibrahim@corgea.com>
Only the planner's refusals are printed; the two flags were the user's own
choice. Doc comments now describe what the code does.

Co-authored-by: ibrahim <ibrahim@corgea.com>
A baseline whose stored checksums cannot be downloaded, decoded, or are an
unknown version is why a diff that should have worked did not, so it outranks
git's refusal as the cause sent to the server. Pins every cause code the
server stores.

Co-authored-by: ibrahim <ibrahim@corgea.com>
@Ibrahimrahhal
Ibrahimrahhal marked this pull request as ready for review October 7, 2026 06:41

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated review risk: 2/5.

No actionable critical, high, or nitpick findings are supported by the supplied diff. The new full-scan reason propagation is internally consistent and covered by focused unit and end-to-end tests.

No critical or high-priority changes were found.

@corgea-security corgea-security added the dennis-reviewed Dennis completed an automated review label Oct 7, 2026

@corgea-security corgea-security left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved by Dennis: high policy risk and automated risk 2/5.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No merge blockers on bd3a7bc.

The upload stays reason XOR diff. upload_zip sends incremental_skipped_reason and incremental_skipped_detail only on Err, and the base/changed-file fields only on Ok, on every chunk including the final one. --disable-incremental on a clean tree still sends dirty=false with disabled_by_flag. --target, --only-uncommitted, and --exclude still force dirty=true.

A missing manifest leaves git's cause (exclude_needs_checksums, dirty_worktree). Unreadable checksums become baseline_checksums_unreadable only when git also refuses; a git fallback that succeeds still uploads a diff and neither skip field. Flag refusals stay silent. Every other refusal is still Scanning every file: {detail}., and the clauses match the previous strings. All 13 codes match [a-z0-9_]{1,64}. No earlier review threads on this PR.

Open in Web View Automation 

Sent by Cursor Automation: pr-flow

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dennis-reviewed Dennis completed an automated review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants