Skip to content

fix(ci): stop the remote gate before restoring an interrupted ci-remote - #491

Merged
michelhonold merged 4 commits into
mainfrom
fix/ci-remote-cleanup
Oct 21, 2026
Merged

michelhonold merged 4 commits into
mainfrom
fix/ci-remote-cleanup

Conversation

@michelhonold

@michelhonold michelhonold commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Plan

  • Make the test ssh stand-in behave like sshd (remote command in its own session) so the orphaned run reproduces
  • Failing tests: Ctrl-C, SIGTERM and a dropped connection, during make ci and during the drawing comparison
  • Record the remote run's process group; stop exactly that group before restoring
  • Refuse a new gate while a recorded run is still alive
  • Skill and script header updated

Summary

Fixes #416.

ci-remote runs make ci on the Pi over ssh without a terminal, and sshd starts that command as a session leader in its own process group. A Ctrl-C, a SIGTERM or a dropped connection ends the local ssh, but the Pi run carries on. The restore trap then reset the Pi checkout underneath it, and the next gate could start while it was still running.

  • Each remote gate step (make ci and the drawing comparison) records its process group in .git/ci-remote.pid on the Pi. sshd makes the remote shell a session leader, so $$ is the group id.
  • restore first stops that group (TERM, then KILL, up to about 10 s each) and only resets the checkout once it has ended. If the run cannot be stopped (for example, the Pi is unreachable), the checkout is left alone and the gate fails with "could not be stopped, so … was not restored".
  • A gate refuses to start while the recorded group is still alive ("an earlier gate is still running", exit 2), also with OPENFOLLOW_CI_FORCE=1. A dead record is removed.
  • Only the recorded group is signalled, never anything matched by name, so another user's run on the Pi is never touched. A record naming group 0 or 1 is ignored, since kill reads those as every process.
  • Nothing is installed on the Pi: the record and the stop are plain POSIX shell in the remote command strings (checked with bash, dash and zsh).

If the workstation disappears completely, the Pi run finishes on its own. In the meantime it holds off every later gate, and once it has ended the dirty-checkout guard applies as before.

Test plan

  • tests/test_ci_remote_script.py: the fake ssh forks and runs the command in a new session, like sshd
  • Interrupted gate × {Ctrl-C, SIGTERM, dropped connection} × {make ci, drawing comparison}: the step is stopped while the synced tree is still in place (it records this on SIGTERM), then the checkout is restored. An unrelated process on the "Pi" in its own session survives.
  • A gate left running after its workstation vanished refuses the next gate (with FORCE); after it ends, the next gate runs and restores
  • Run records 1, 0, 01, -1, empty, abc neither block nor stop anything
  • All 7 new interrupt/hold-off tests fail on the unfixed script. Mutation checks: no kill, no guard, no record validation, stop after the reset and an added kill by name each fail a test
  • tests/test_ci_remote_script.py: 37 passed, three runs in a row
  • make ci-remote on the testing Pi
  • On a Pi 5 testing Pi:
    • with a gate in its unit tests, a second gate was refused, even with OPENFOLLOW_CI_FORCE=1 ("an earlier gate is still running … (process group …)");
    • Ctrl-C on the first gate then stopped the remote run: four pytest workers before, none after;
    • the checkout was back at its pre-run commit, clean, and the run record was gone.

🤖 Generated with Claude Code

michelhonold and others added 4 commits October 21, 2026 00:34
ci-remote runs make ci on the Pi over ssh without a terminal, and sshd
starts that command in a session of its own. A Ctrl-C, a SIGTERM or a
dropped connection ends the local ssh, but the Pi run carries on, and
the restore trap then reset the Pi checkout underneath it. The next
gate could also start while that run was still going.

Each gate step (make ci and the drawing comparison) now records its
process group on the Pi in .git/ci-remote.pid. The restore first ends
that group, TERM then KILL, and only then resets the checkout; if the
run cannot be stopped the checkout is left alone and the gate fails.
A gate refuses to start while a recorded run is still alive, also with
OPENFOLLOW_CI_FORCE. Only the recorded group is signalled, never by
name, and a record naming group 0 or 1 is ignored because kill reads
those as every process.

The test ssh stand-in now runs the remote command in its own session,
as sshd does, so the tests reproduce the orphaned run.

Fixes #416

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The cleanup killed the long step's pid even after it had exited, so a
reused pid could take an unrelated process with it. It now kills the
step only while it has not reported its stop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test polls for the file and reads it at once, so a partly written
file could be read as an empty pid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a loaded Pi the dropped-connection case could stop the remote step
before it wrote its marker, so the test read the gate as never started.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michelhonold

Copy link
Copy Markdown
Contributor Author

Review rounds

Round 1 (full git diff origin/main HEAD)

  • Medium, fixed in cb7aa00: the test cleanup sent SIGKILL to the long step's pid even after that step had exited. A reused pid could take an unrelated process with it. Cleanup now kills the step only while it has not reported its stop. The script's own process group and the bystander (a Popen that is not reaped yet, so its pid cannot be reused) are the only other targets.

Round 2

  • Checked: a remote shell that is not a session leader makes kill -0 -$$ fail. The gate then behaves as before (no stop, no hold-off) and never signals a wrong group. The second Ctrl-C during the stop wait ends the gate without a restore. That matches a second Ctrl-C during the existing restore.
  • The normal path costs one extra ssh round trip for the stop (a no-op after a finished run). Accepted.
  • No findings.

Round 3

  • Medium, fixed in 8fdea7e: the long step wrote its pid file non-atomically, while the test polls for that file and reads it at once. A partly written file could be read as an empty pid, a flake risk. It now writes to a temporary file and renames it.
  • Left as rare: if the recorded pid has been reused by another of the Pi user's process groups after an orphaned run ended, the next gate refuses with a false "still running" until that group ends. Linux pid reuse needs a full wrap of pid_max.

Results: tests/test_ci_remote_script.py 37 passed in three runs. All 7 new interrupt/hold-off tests fail on the unfixed script. Mutations (no kill, no guard, no record validation, stop after reset, an added kill by name) each fail a test.

After the review (5d7ab3cf): the gate on the loaded testing Pi failed one new interrupt case. In the dropped-connection variant, the test dropped the connection before the step had started. So the script stopped the step, correctly, before it wrote its marker, and the test read the gate as never started. The connection is now dropped only once the step runs. The case passed five runs in a row, still fails against the unfixed script, and the gate then passed.

@michelhonold
michelhonold marked this pull request as ready for review October 21, 2026 06:08
@michelhonold
michelhonold merged commit 79a36d5 into main Oct 21, 2026
4 checks passed
@michelhonold
michelhonold deleted the fix/ci-remote-cleanup branch October 21, 2026 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ci): an interrupted ci-remote may leave make ci running on the Pi

1 participant