Skip to content

[patch] Drop a reused handler's leftover partial line at the start of each run - #110

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/93-reset-handler-between-runs
Oct 6, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/93-reset-handler-between-runs

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #93

Problem

LineOutputHandler keeps unterminated text in outputBuffer/errorBuffer, and only Complete() clears it. RunAsync reaches Complete() only when a run succeeds. A run that was cancelled, or whose callback threw, left its partial line in the buffer, and reusing the handler glued that text onto the next run's first line. The issue's repro gives ["partialhello"] instead of ["hello"].

Change

  • New internal virtual void Reset() on OutputHandler, a no-op by default. RunAsync calls it before it starts the process.
  • LineOutputHandler.Reset() discards both buffers, including a trailing CR still waiting for a possible LF. A cancelled run's partial line isn't a real line, so it's dropped rather than delivered. The buffers are replaced rather than cleared, so a delivery still finishing from the old run can't touch the new run's buffer. That's why outputBuffer/errorBuffer are no longer readonly.

Abandoned readers, the issue's second point: I added a regression test where an orphaned descendant keeps the cancelled run's stdout open and writes to it while the next run is going. It passes with only the Reset change, because disposing the cancelled run's AsyncProcessStreamReader already ends that read, so the late write never reaches the handler. I tried a per-reader "abandoned" flag as well. No test failed without it, so I left it out to keep the change minimal.

Tests

Three new tests in RunCommandTests. They need sh and are inconclusive on Windows, like the existing shell-dependent tests:

  • ReusedLineOutputHandlerShouldDropTheLineACancelledRunLeftUnfinished: the issue's repro. Cancel after printf partial arrives, then reuse the handler for echo hello, and expect hello.
  • ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrewLeftUnfinished: same, except the first run fails because the stderr callback throws.
  • AbandonedReaderShouldNotDeliverIntoALaterRunOnTheSameHandler: an orphan writes late into the cancelled run's pipe while the next run is going, and the next run still sees only hello.

Checked locally (Linux, net10.0):

  • With the Reset() call removed, all 3 new tests fail.
  • The full suite passes with the fix (60 passed, 2 skipped elevation tests), and stayed green across 3 repeated runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C9H64dhbitTL25J7BebFyE


Generated by Claude Code

A run that was cancelled or whose callback threw never reached Complete,
so LineOutputHandler kept its partial line and glued it onto the next
run's first line ("partial" + "hello" -> "partialhello"). RunAsync now
calls a new internal OutputHandler.Reset before starting the process, and
LineOutputHandler discards both buffers there.

Fixes #93

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01C9H64dhbitTL25J7BebFyE
Comment thread RunCommand.Test/RunCommandTests.cs
Comment thread RunCommand.Test/RunCommandTests.cs
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A reused LineOutputHandler prepends a cancelled run's partial line to the next run's first line ("partial" + "hello" → "partialhello")

1 participant