Skip to content

fix(cli): report partial output only when present - #359

Open
Mr-Neutr0n wants to merge 1 commit into
openai:mainfrom
Mr-Neutr0n:fix/empty-partial-output-message
Open

fix(cli): report partial output only when present#359
Mr-Neutr0n wants to merge 1 commit into
openai:mainfrom
Mr-Neutr0n:fix/empty-partial-output-message

Conversation

@Mr-Neutr0n

Copy link
Copy Markdown

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 types
  • pnpm run format
  • Full suite: 975 passed and 11 skipped; 63 failures are environment-dependent on this host because it has Python 3.9 and Node 25, outside the repository matrix.

Signed-off-by: Mr-Neutr0n <harikp2002@gmail.com>
@github-actions github-actions Bot added the bug Something isn't working label Aug 12, 2026

@ting-hong-shieh ting-hong-shieh 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.

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, and git diff --check.
  • The same missing-path fixture fails on both trees. Its structured result is exit 2 / claimed=true, SIGINT 130 / claimed=true, and SIGTERM 143 / claimed=true.
  • On the current-main merge, treating ENOENT as 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.

Comment thread sdk/typescript/src/cli.ts
} catch {
// Keep the path in the diagnostic when it disappeared or cannot be read.
// Only suppress the message when emptiness was confirmed.
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(0.1.5) scan agent still creates no draft artifacts on GitHub-hosted runners

2 participants