Skip to content

Every command failure pickles across a worker - #3206

Merged
trissim merged 2 commits into
mainfrom
fix/git-error-pickles
Oct 25, 2026
Merged

trissim merged 2 commits into
mainfrom
fix/git-error-pickles

Conversation

@trissim

@trissim trissim commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

GitCommandError (repo.py:41) is constructed from (repository, run), but BaseException pickles an exception as type(*args). So when a ForkedProcessPool parent unpickles one raised in a worker, it calls __init__("Git in ...") and dies with TypeError: missing 1 required positional argument: 'run'. The pool then reports BrokenProcessPool and the git message is lost. This turned up while working on #3200.

CoordinationError already had the right __reduce__: it rebuilds through copyreg.__newobj__ from the class, the args and the state, and keeps __cause__. It was declared only on that one branch of the family. This PR moves it to the family owner, CommandFailure, and deletes the copy, so all 28 members pickle, including GitCommandError, InstallRunning, InstallRolledBack, InterpreterWorkerDied, StoppedOwnerFailure, ObserverDied and UnprivateSidecarFile. None of them needs its own __reduce__.

Proof

  • tests/test_command_failure_pickles.py imports every module, walks CommandFailure's subclasses and round-trips each member. It checks the class, args, extra state and __cause__ (28 cases), plus a real GitCommandError with its git message and returncode. 29 passed.
  • The observation tests that rely on the old CoordinationError reduce also pass: test_unreadable_history_observation, test_deleted_thread_observation and test_sidebar_observation.

🤖 Generated with Claude Code

https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V

trissim and others added 2 commits October 24, 2026 22:51
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V
GitCommandError takes (repository, run), so a raised one unpickled in a
ForkedProcessPool parent called __init__(message), died with TypeError and
surfaced as BrokenProcessPool with the git message lost. CoordinationError
already rebuilt itself from class, args and state; that __reduce__ now lives
on the family owner, so every member pickles, and the test walks the family.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V
@trissim
trissim marked this pull request as ready for review October 25, 2026 02:52
@trissim
trissim enabled auto-merge October 25, 2026 02:52
Copilot AI balanced review requested due to automatic review settings October 25, 2026 02:52

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim merged commit ac90cef into main Oct 25, 2026
14 checks passed
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.

2 participants