Skip to content

Fix the ownerless main CI reds; Linux-only CI - #3231

Merged
trissim merged 6 commits into
mainfrom
fix/fork-reds3-main-reds
Oct 25, 2026
Merged

trissim merged 6 commits into
mainfrom
fix/fork-reds3-main-reds

Conversation

@trissim

@trissim trissim commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Post-merge ci.yml failed 25 of 25 runs on main (run 40159640144: 9 failed, 3927 passed). This PR fixes the six reds nobody owned and makes CI Linux-only. The double debt_ratchet import (#3170) and the two reds in #3229 belonged to other PRs, so this one leaves them alone.

Red Cause Fix at the owner
test_raw_child_launches_in_tools_and_tests_only_fall Three raw Popen launches were added since the guard last fell: test_carry, test_installer_route_lifetime and, on main during this PR, test_install_session_bridge. A fourth (test_store_guards) also landed during this PR, because SpawnedProcess.launching hard-coded pass_fds=(), child_process.py:1865. All of them now launch through SpawnedProcess.launch, whose .identity replaces the second ProcessIdentity.capture. launching takes pass_fds. The hand-written kill/communicate cleanup is deleted. RAW_LAUNCHES goes from 33 to 32.
test_private_failed_readback_does_not_publish 8455dc8 moved the ledger's fsync and readback outside publishing_changes. A write whose readback failed still touched goal_attempts.changed. StoreWriter.transaction takes a committed hook that runs before the publish. GoalAttemptStore._change passes its attest step there, so a raise announces nothing.
test_native_thread_caller_waits_then_publishes_once b411b9a made PublishingTool.apply refuse a held bus on purpose: a wait in the connector's worker would hold up the connector's stop. test_connector::test_publishing_tool_held_mid_call_refuses_at_once checks that refusal. Not a leak from a live bus: held_bus builds its own wire(tmp_path). The test pinned the deleted wait, so it is deleted. The CLI-child cases stay.
test_original_header_preserves_project_and_fork_lineage b702b7a typed SessionEntry.cwd as ReportedText. The path is identical. The test compares against ReportedText(raw["cwd"]).
test_runtime_producer_digests_ignore_foreign_source (gw3 crashed) It is the one installed_home test without @pytest.mark.timeout(280). On a cold worker it pays for the private runtime build inside the 60 s default, and pytest-timeout's thread method then kills the xdist worker. Locally it took 67 s. pytestmark = pytest.mark.timeout(280) is declared once for the module. The five per-test copies are deleted.
test_fork_send_receive_respond (gw1 crashed) Since c2606af the fork verb really runs, and it boots Pi twice: once for the --model catalog (GetAvailableModels, cli_commands.py:2282), then for the thread. CATALOG_TIMEOUT = 10 was set against an idle 6-7 s boot (py-spy samples), so on a loaded host it times out with NativePiUnavailable. The module's 30-65 s runtime also blew the 60 s default. CATALOG_TIMEOUT = 30. The module declares 180 s, and each CLI child gets 60 s.
macOS leg Tristan wants CI on Linux only. os: [ubuntu-latest, macos-latest] and the macOS-only test step are deleted.

Still open, owner not yet assigned: fork boots Pi twice, the first time only to list models. Raising CATALOG_TIMEOUT stops the timeouts, but the structural fix is to validate --model against the thread's own boot, or against a catalog cached at prepare.

Proof (local, -p no:xdist, PYTHONPATH=src, on the merged tree)

  • guards/test_child_launch_ownership: 1 passed
  • test_watched: 16 passed
  • test_held_sends: 3 passed
  • test_native_evidence_boundary: 8 passed
  • test_installer_route_lifetime: 11 passed
  • test_carry: 16 passed
  • test_install_session_bridge: 12 passed
  • test_store_guards: 2 passed
  • test_child_process: 30 passed
  • test_pi_rpc_nominal: 41 passed
  • test_end_to_end: 3 passed, 217 s
  • test_install_rehearsal::test_runtime_producer_digests_ignore_foreign_source: 1 passed, 57 s, under the real 60 s default before the module mark
  • test_goal_attempts: 27 passed
  • test_goal_attempt_lifecycle: 1 passed
  • test_external_transcript: 9 passed

Deleted

  • test_held_sends: the native ThreadCaller case.
  • test_installer_route_lifetime: the route_holder helper.
  • test_carry, test_store_guards and test_install_session_bridge: the raw launches and their hand-written cleanup.
  • test_install_rehearsal: the five timeout(280) copies.
  • ci.yml: the macOS matrix leg and step.

Assumed

  • The rehearsal test runs 57-67 s locally because the fixture build is counted inside it. CI's 8-worker runner is slower, so 280 s is the existing budget the sibling tests already used, not a new number.
  • 30 s for the catalog child is the same bound native_backend_fixture gives a Pi boot. It still catches a hung child.
  • The two xdist crashes were the pytest-timeout thread method firing. I reproduced the 60 s timeout and the catalog TimeoutError locally, but not the crash under xdist itself.

🤖 Generated with Claude Code

https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V

trissim and others added 6 commits October 25, 2026 00:22
- Child launches: test_carry and test_installer_route_lifetime added raw
  Popen launches since the guard last fell; both now go through
  SpawnedProcess.launch (its identity replaces the second capture), and
  RAW_LAUNCHES falls to 32.
- Failed readback published: 8455dc8 moved the ledger's fsync and
  readback out of publishing_changes, so a write whose readback failed
  still announced goal_attempts.changed. StoreWriter.transaction now takes
  a committed hook that runs before the publish; GoalAttemptStore._change
  passes its attest there, and a raise announces nothing.
- Native held send: b411b9a made PublishingTool refuse a held bus on
  purpose (test_connector pins the refusal); the native ThreadCaller case
  in test_held_sends pinned the old wait and is deleted.
- Evidence header: b702b7a typed SessionEntry.cwd as ReportedText; the
  test compares the typed value.
- Rehearsal worker crash: the one installed_home test without the 280 s
  mark paid the private runtime build inside the 60 s default and
  pytest-timeout's thread method killed the xdist worker. The module
  declares the budget once; the five per-test copies are deleted.
- End-to-end fork crash: since c2606af the fork verb really runs, and
  it boots Pi twice (the --model catalog, then the thread). The catalog
  child had 10 s against a 6-7 s idle boot, so a loaded host timed out;
  CATALOG_TIMEOUT is 30. The module declares a 180 s budget and each CLI
  child 60 s.
- ci.yml runs Linux only; the macOS matrix leg and its step are deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V
Main's test_a_bridge_that_dies_during_pause_counts_as_stopped added a raw
Popen beside its second ProcessIdentity.capture; SpawnedProcess.launch owns
the child and its identity, and the guard holds at 32.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013ejYrMLnLWah8wCDDcb57V
Platform.launch always took pass_fds, but SpawnedProcess.launching handed it
(), so main's exec-adoption test in test_store_guards fell back to a raw
Popen to pass its held claim descriptor. launching now takes pass_fds; the
test launches through it, its hand-written kill/communicate cleanup is
deleted, and the guard holds at 32.

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 05:03
Copilot AI balanced review requested due to automatic review settings October 25, 2026 05:03
@trissim
trissim enabled auto-merge October 25, 2026 05:03

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 b2e8ea4 into main Oct 25, 2026
17 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