Skip to content

Make the timed-out salvage test outlast the launch it races - #2116

Merged
ppXD merged 1 commit into
mainfrom
test/make-timed-out-salvage-test-deterministic
Oct 13, 2026
Merged

ppXD merged 1 commit into
mainfrom
test/make-timed-out-salvage-test-deterministic

Conversation

@ppXD

@ppXD ppXD commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • AgentBranchPushFlowTests.P2_2_a_timed_out_run_salvages_its_real_edit_as_a_real_pushed_branch gave its run a 1s wall deadline. That deadline is armed when the launch is bound (LocalProcessRunner.NativeLaunch.cs:76), before the runner host's broker, exec bootstrap and guardian processes start and before /bin/sh runs. On a loaded host the launch alone took 1.0–2.0s, so the kill sometimes landed before the agent's write. The test then failed with an empty ChangedFiles, which looked like broken salvage even though salvage never ran.
  • The deadline is now 10s. The script runs printf ... > salvaged.txt && echo written; sleep 300, and the test reads written back through the run's recorded events (IAgentRunService.GetEventsAsync) before it checks the capture. If a launch ever overruns the deadline, the failure says so instead of blaming salvage. Everything the test proves is unchanged: the run ends TimedOut, the file is in ChangedFiles, and the branch is really pushed to the bare remote and contains the file.
  • Only the test file changes.

Test plan

  • Reproduced the CI failure with the original test under 10x CPU oversubscription (120 busy loops on 12 cores): 3 of 6 runs red with ChangedFiles ... but was actually []
  • Fixed test, idle: 20/20 green
  • Fixed test, same 10x oversubscription: 20/20 green
  • Mutation: remove TimedOut from the salvage push gate in PushProducedBranchIfEnabledAsync → red at ProducedBranch
  • Mutation: skip EnrichWithWorkspaceChangesAsync for TimedOut → red at ChangedFiles, after the written precondition passes
  • Mutation: delay the agent's write past the deadline → red at the written precondition, with a message that names the launch overrun
  • AgentBranchPushFlowTests class: 4/4 green
  • CI integration lane green

The P2.2 salvage test gave its run a 1s wall deadline and expected the
agent's write to land before the kill. The deadline is armed when the
launch is bound, before the runner host's broker, exec bootstrap and
guardian start and before /bin/sh runs. On a loaded host that launch
alone took 1.0-2.0s, so the kill sometimes landed before the write and
the test failed with an empty ChangedFiles. That read as broken salvage
when salvage was never exercised. It failed this way twice on CI.

The deadline is now 10s. The script echoes "written" once the file is
on disk and only then sleeps, and the test reads that line back through
the run's recorded events before it checks the capture. If a launch
ever overruns the deadline again, the failure names the launch instead
of blaming salvage.
@ppXD
ppXD merged commit a1b7c1e into main Oct 13, 2026
6 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.

1 participant