fix(debug-files): exit non-zero when files are dropped for size#1146
Merged
Conversation
Contributor
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 40f8047. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
409
to
+418
| filesUploaded: results.length, | ||
| }); | ||
|
|
||
| // Scan-time oversized files were dropped before the queue was built, so they | ||
| // never appear in `results`. Note them on any failure hint and treat them as | ||
| // a failure in their own right below. | ||
| const scanOversize = | ||
| params.oversizedCount > 0 | ||
| ? ` ${params.oversizedCount} file(s) were skipped for exceeding the maximum file size (${params.maxFileSize} bytes).` | ||
| : ""; |
Contributor
There was a problem hiding this comment.
Bug: The filesUploaded count in JSON output is inflated by including oversized source bundles that were filtered out and never uploaded when using --include-sources.
Severity: MEDIUM
Suggested Fix
Calculate filesUploaded by filtering the results array to only include files with a successful upload state, rather than using results.length directly. This ensures the count accurately reflects the number of files that were actually uploaded to the server.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/commands/debug-files/upload.ts#L409-L418
Potential issue: When using the `--include-sources` flag, source bundles are generated
in-memory, bypassing the initial file size scan. If a generated bundle exceeds the
server's maximum file size, it is filtered out during the upload process. However, the
`filesUploaded` count in the JSON output is calculated from `results.length`, which
includes both successfully uploaded files and error entries for these oversized,
non-uploaded bundles. This results in an inaccurate count in the machine-readable
output, even though the human-readable summary and exit code are correct.
Did we get this right? 👍 / 👎 to inform future reviews.
Member
Author
There was a problem hiding this comment.
Addressed in follow-up #1167 (this PR was already merged): filesUploaded now subtracts failed/dropped results, and the --require-all missing-id note is appended to the failure/size-drop hint so it is never dropped when those return first.
Contributor
Codecov Results 📊✅ Patch coverage is 90.91%. Project has 5142 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.52% 81.52% —%
==========================================
Files 399 399 —
Lines 27811 27818 +7
Branches 18065 18069 +4
==========================================
+ Hits 22669 22676 +7
- Misses 5142 5142 —
- Partials 1863 1863 —Generated by Codecov Action |
A partial size-drop during upload silently exited 0. The all-dropped cases already fail loudly (filterBySize throws ValidationError; the scan-time path exits via doNothingToUpload), but a partial drop did not: oversized files left no result entry, so they were never counted as failures. - uploadDebugFiles now returns oversized (filterBySize-dropped) files as `error` results — most relevant for in-memory --include-sources bundles, which bypass the scan-time gate. - doUpload now receives oversizedCount/maxFileSize and exits non-zero when scan-time oversized files were dropped, matching doNothingToUpload. Surfaces Warden finding 9LL-87A on #1140.
BYK
force-pushed
the
byk/fix/debug-files-oversize-exit-code
branch
from
d876264 to
5842fad
Compare
BYK
added a commit
that referenced
this pull request
Jul 2, 2026
…nt (#1167) Follow-up to #1146, addressing two bot findings on that PR. ## 1. `filesUploaded` over-counted (Bugbot/Seer, Low) #1146 surfaces size-dropped files as `error` results so a partial drop exits non-zero. But `doUpload` reported `filesUploaded: results.length`, which then included those `error`/`not_found` entries — over-reporting the number of files actually uploaded in the JSON output. Fixed: compute `failures` before the summary and report `results.length - failures.length`. ## 2. `--require-all` hint dropped after a failure (Bugbot, Medium) When an upload failure or a size-drop returned first, `doUpload` returned before the `--require-all` branch, so the "missing requested debug id(s)" note was omitted from the hint. The exit code was still correctly non-zero, but the actionable feedback was lost. Fixed: build a `requireAllNote` once and append it to whichever hint returns (failure / size-drop), so the missing-id feedback is never swallowed. ## Tests - `filesUploaded excludes failed/dropped results` — mixed ok/error results ⇒ `filesUploaded` counts only the ok one. - `--require-all note is preserved alongside an upload failure` — failure + missing required id ⇒ hint contains both "had failures" and the missing id. `typecheck`, `lint`, and the debug-files upload suite (36 tests) all pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1140, addressing Warden finding 9LL-87A (a comment we missed before merge).
A partial size-drop during
debug-files uploadsilently exited0and printed "Uploaded N debug file(s)". The all-dropped cases already fail loudly —filterBySizethrowsValidationError, and the scan-time all-dropped path exits viadoNothingToUpload(... oversizedCount > 0 → exit 1)— but a partial drop did not honor that same "oversized ⇒ non-zero exit" contract. Oversized files left no result entry, sodoUploadnever counted them as failures.Two disjoint drop sites both had the gap:
filterBySize) — most relevant for in-memory--include-sourcessource bundles, which bypass the scan-time size gate and can only be caught here.uploadDebugFilesnow returns these aserrorresults so the existingdoUploadfailure path sets exit 1 and lists them.prepareDifs→oversizedCount) — regular oversized files are dropped before the upload queue is built.doUploadnow receivesoversizedCount/maxFileSizeand exits non-zero on a partial drop, matchingdoNothingToUpload.Accepted (in-cap) files are still uploaded; only the exit code and hint change so a partial drop is no longer mistaken for a clean success.
Warden findings on #1140
prepareFileDifnow gates onpeeked.size(fromfd.stat()) beforereadFile, so oversized files are never buffered.Tests
test/lib/api/debug-files.test.ts: the partial-drop test now asserts the oversized file is not assembled but is returned as anerrorresult.test/commands/debug-files/upload.test.ts: new test — a partial scan-time size-drop still uploads the in-cap file but exits1.pnpm run typecheck,biome check, and both affected test files (40 tests) pass.