fix(cli): report partial output only when present - #359
Conversation
Signed-off-by: Mr-Neutr0n <harikp2002@gmail.com>
ting-hong-shieh
left a comment
There was a problem hiding this comment.
Reviewed at 79e9fe1a569696160fe5d2e46d73f600e9155bc8. The new empty/non-empty directory cases work, but the same misleading retained-output claim remains when the path no longer exists.
Validation snapshot:
- Exact head and a clean synthetic merge onto current
main(216212b70a421b0bf30319d241de47a64050ec0f) both pass the two existing focused tests, TypeScript checking, formatting, andgit diff --check. - The same missing-path fixture fails on both trees. Its structured result is
exit 2 / claimed=true,SIGINT 130 / claimed=true, andSIGTERM 143 / claimed=true. - On the current-main merge, treating
ENOENTas no retained output makes all three cases pass. The PR's empty/non-empty test, TypeScript checking, formatting, and whitespace check remain clean.
I left one blocking inline finding. No model or provider call, API request, credential, or external service was used.
| } catch { | ||
| // Keep the path in the diagnostic when it disappeared or cannot be read. | ||
| // Only suppress the message when emptiness was confirmed. | ||
| return true; |
There was a problem hiding this comment.
Handle ENOENT as no retained output
Both this helper and hasPartialOutputSync turn every readdir failure into true, including ENOENT. If the path is removed after onOutputDirReady reports it, ordinary failure, SIGINT, and SIGTERM still print Partial output was kept at <missing path>. That is the same false claim this PR is meant to suppress, and #191 asks to show it only when the directory is non-empty.
At exact head 79e9fe1a, and again after a clean merge onto main 216212b7, the same fixture produced {exit: 2, claimed: true}, {signal: SIGINT, exit: 130, claimed: true}, and {signal: SIGTERM, exit: 143, claimed: true}. Returning false for ENOENT in both helpers makes all three cases pass while preserving the PR's empty/non-empty test and the static checks. Please handle the missing-path case and add ordinary plus signal-path regression coverage.
Summary
Fixes #191 by making the CLI verify that a registered scan output directory contains entries before reporting partial output. Empty directories no longer produce a misleading path, while directories that contain entries, disappear, or cannot be inspected retain the existing diagnostic.
Verification
pnpm exec bun test --timeout 30000 ./tests-ts/cli.test.ts --test-name-pattern "does not claim partial output"pnpm run typespnpm run format