Skip to content

Fix #5 #6 #7 #8: UTF-8 corruption, flush, cmd.exe mangling, redirected-stdio - #12

Merged
soroshsabz merged 6 commits into
mainfrom
fix/issues-5-8
Nov 6, 2026
Merged

soroshsabz merged 6 commits into
mainfrom
fix/issues-5-8

Conversation

@soroshsabz

@soroshsabz soroshsabz commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

ITNOA

Fixes some bugs in first real testing

This PR

Issue #5: Cast recordings corrupt multi-byte UTF-8 at 4096-byte boundaries

Root cause: CastWriter.WriteEvent() decoded each 4096-byte chunk independently with Encoding.UTF8.GetString(), replacing truncated multi-byte sequences with U+FFFD.

Fix: Persistent Decoder instances (separate for output/input streams) that carry incomplete sequences across chunk reads. The character that spans a boundary appears at the start of the next event instead of being corrupted.

Tests:

  • System test: records 60000 copies of U+2580 (180KB, far exceeds the buffer) — asserts zero U+FFFD
  • Unit test: PowerShell converter handles chunked multi-byte data through timing-file boundaries

Issue #6: Cast file loses header/tail on abnormal termination

Root cause: StreamWriter buffers ~1KB; nothing reaches disk until Dispose() runs (which never happens on crash).

Fix: AutoFlush = true — every line (header + events) goes to disk immediately.

Issue #7: -Command values with pipes/quotes mangled by cmd.exe

Root cause: cmd.exe /d /c "{command}" strips quotes and interprets |&<>^ as shell metacharacters.

Fix: Get-BsnRecordingCommandLine now wraps -Command in pwsh -NoProfile -EncodedCommand — the exact command text is preserved bit-for-bit, no shell interpretation.

Tests: Base64 round-trip verification for pipes, embedded quotes, angle brackets, carets.

Issue #8: Empty cast but success report when stdio is redirected

Root cause: Windows routes child output around the ConPTY when the caller has pipe stdio; the engine captures nothing but returns success.

Fix: Test-BsnRecordingEnvironment now throws a terminating error when both stdin and stdout are redirected on Windows. Partial redirect (one handle) remains a warning.

Validation

  • 78 tests green on PS 7.6 (61 Bsn.Host + 4 Bsn.Common + 13 Bsn.SystemManagement)
  • 74 tests green on PS 5.1 (system tests skip with clear reasons)
  • PSScriptAnalyzer clean
  • Bump: Bsn.Host 0.1.0 -> 0.2.0

…ng, redirected-stdio

Issue #5 - Cast recordings corrupt multi-byte UTF-8 at 4096-byte boundaries:
  CastWriter now uses persistent Decoder instances (separate for output
  and input streams) that carry incomplete multi-byte sequences across
  chunk reads, producing complete characters instead of U+FFFD.
  System test records 60000 multi-byte characters (180KB burst) and
  asserts zero U+FFFD in the cast.
  Unit test verifies the PowerShell converter (ConvertTo-BsnAsciicast)
  handles chunked multi-byte data through timing-file boundaries.

Issue #6 - Cast file loses header/tail on abnormal termination:
  CastWriter now uses StreamWriter.AutoFlush = true, so every line
  (header + events) goes to disk immediately. An abnormally terminated
  recording stays a valid asciicast file ending at the last completed
  event instead of losing everything to the character buffer.

Issue #7 - -Command values with pipes/quotes mangled by cmd.exe:
  Get-BsnRecordingCommandLine now wraps -Command in pwsh
  -EncodedCommand (Base64 UTF-16LE) instead of cmd.exe /d /c.
  This preserves the exact command text bit-for-bit through the
  CreateProcess command line - no shell metacharacter interpretation.
  Unit tests verify Base64 round-trip for pipes, quotes, angle brackets,
  and carets.

Issue #8 - Empty cast but success report when stdio is redirected:
  Test-BsnRecordingEnvironment now throws a terminating error when BOTH
  stdin and stdout are redirected on Windows (the recording is provably
  empty in this shape). Partial redirect (one handle) remains a warning.
  This prevents silent useless recordings from CI/scripted hosts.

All 78 tests green on PS 7 and PS 5.1.
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ba8083d1-0f34-4445-b1f8-3eca824d0714
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@soroshsabz

Copy link
Copy Markdown
Member Author

Refactored three nearly-identical Base64 round-trip tests into a single
data-driven test using Pester -TestCases. Also bumped Bsn.Host to
0.2.0 (bug fix release for issues #5-#8).
- Renamed Issue5to8.Tests.ps1 to RecordingIntegrity.Tests.ps1
- Added comprehensive file header describing each issue with hyperlinks
- Added issue hyperlinks in every Describe block and the system test
- Added inline comment explaining the cmd.exe -> EncodedCommand change
  (issue #7 fix: cmd.exe /d /c mangled metacharacters)
Removed RecordingIntegrity.Tests.ps1 (catch-all file); the tests now
live alongside the existing tests for the same functions:

- Issue #7 (command encoding) + #8 (redirected stdio) tests moved to
  Start-InteractiveTranscript.Tests.ps1 (tests the public cmdlet's
  environment checks and command line construction)
- Issue #5 (UTF-8 chunk boundaries) + #6 (incremental flush) tests
  moved to ConvertTo-BsnAsciicast.Tests.ps1 (tests the converter's
  decoding and output behavior)

Each test block has the issue hyperlink inline.
@soroshsabz

Copy link
Copy Markdown
Member Author

Distributed all tests into the existing function-specific test files (commit 3e56792). RecordingIntegrity.Tests.ps1 is deleted. Issue #7 + #8 tests are now in Start-InteractiveTranscript.Tests.ps1; issue #5 + #6 tests are in ConvertTo-BsnAsciicast.Tests.ps1. Each has inline issue hyperlinks.

…TUI (Linux)

oh-my-posh test (Windows): renders 20 prompts via 'oh-my-posh print
primary', producing powerline/nerd-font glyphs (3-4 byte UTF-8 in the
Supplementary Private Use Area). Each prompt is 200-600 bytes; 20
prompts guarantee chunk-boundary splits mid-glyph. Asserts zero U+FFFD.

vim TUI test (Linux): opens a file with UTF-8 content, box-drawing
characters, and plain text in vim (alternate screen buffer, syntax
highlighting, status line). Asserts alt-screen sequences present,
content captured, multi-byte content survived, zero U+FFFD.

Both skip gracefully when the tool is unavailable. CI installs
oh-my-posh on Windows runners via winget.

These tests provide real-world validation for the issue #5 fix
(persistent UTF-8 Decoder in CastWriter) beyond the synthetic U+2580
blocks: actual application-generated glyphs are proven to survive
recording without corruption.
@sonarqubecloud

sonarqubecloud Bot commented Nov 6, 2026

Copy link
Copy Markdown

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