Skip to content

fix(cli): report incomplete snapshot output - #6223

Draft
ehz0ah wants to merge 1 commit into
volcengine:mainfrom
ehz0ah:fix/cli-snapshot-output-errors-6221
Draft

ehz0ah wants to merge 1 commit into
volcengine:mainfrom
ehz0ah:fix/cli-snapshot-output-errors-6221

Conversation

@ehz0ah

@ehz0ah ehz0ah commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Description

ov snapshot diff and plain ov snapshot ignore-get discarded stdout write and flush errors. A caller-provided nonblocking pipe could therefore receive only part of the output while the command still exited with status 0.

This change makes the raw snapshot output path fallible. Successful output and JSON output stay unchanged. An early reader close (BrokenPipe) remains normal pipeline completion. WouldBlock and other output failures now use the existing CLI error path and return a nonzero status.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #6221

External signal: anthropics/claude-code#103055

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Return output errors from the raw snapshot diff and ignore-file paths.
  • Treat only BrokenPipe as normal pipeline completion.
  • Cover complete writes plus write and flush failures for BrokenPipe and WouldBlock.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Validation performed:

  • rustfmt --edition 2024 --check crates/ov_cli/src/commands/snapshot.rs
  • CARGO_TARGET_DIR=/private/tmp/openviking-pass973-cargo-target CARGO_INCREMENTAL=0 cargo test -p ov_cli commands::snapshot::tests (5 passed)
  • RUST_MIN_STACK=16777216 CARGO_TARGET_DIR=/private/tmp/openviking-pass973-cargo-target CARGO_INCREMENTAL=0 cargo test -p ov_cli (509 unit tests and 4 integration tests passed)
  • Real ov snapshot diff probe: the base command wrote 512 of 110,596 bytes to a backpressured nonblocking pipe and exited 0; this branch wrote the same partial prefix, reported the existing IO Error, and exited 1. Blocking-pipe output remained complete on both revisions.
  • git diff --check

The larger test-thread stack avoids an existing stack overflow in help_ui::tests::bare_command_group_detection_allows_global_flags_before_command. The same test overflows with the default stack on the untouched base revision.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas (no new explanatory comments were needed)
  • I have made corresponding changes to the documentation (not required; the documented successful output is unchanged)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published (no dependencies)

Screenshots (if applicable)

Not applicable.

Additional Notes

This PR changes only the two raw snapshot output paths. It does not change the shared JSON renderer or other CLI commands.

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

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

fix(cli): report incomplete snapshot output

1 participant