feat(slurm): deadline timer for confined jobs too, plus WIP snapshotter

[ligon]

Step (1) of docs/local-disk-tiering.org's implementation order.

Bug fixed. Confined (sbatch) jobs never had a deadline watchdog: _start_slurm_timer is only reached via _ensure_slurm_node on the salloc path, and the batch body starts tmux and nothing else. Verified on n0036.savio4 (job 38661192): no slurm-timer.sh process, despite prompts/carleton-htc.org promising 30/15/5-minute warnings.

Change. The script moves out of cli.py's inline f-string into sucoder/slurm_timer.py (a @TOKEN@ template shared by both launch paths). The confined launch stages slurm-timer-<mirror>.sh next to the batch script and the batch body nohups it after the new-session rc check, inside the job cgroup. Job id from $SLURM_JOB_ID at run time; every tmux call carries the -L socket. State files are per mirror (several confined mirrors share $HOME); the legacy slurm-deadline.warn is still written.

The same script snapshots the dirty tree to refs/sucoder/wip/<mirror> on the tree's origin at each warning and every slurm.wip_snapshot_minutes (default 10, 0 off). No origin means no snapshot, so it is a no-op on today's shared mirror and becomes live with step (2).

Tests. New tests/test_slurm_timer_script.py (render both modes, bash -n, socket threading, quoting, snapshot_wip driven under bash against a real temporary origin). The scancel guard in test_cli.py now inspects the rendered script (the refactor would have made the source-inspection version vacuous). Batch-script, confined-launch, and config tests added. Full suite CI-style: 680 passed. mypy unchanged.

Impact analysis by hand (gitnexus cannot load here, libssl.so.3): _start_slurm_timer <- _ensure_slurm_node only; _build_batch_script <- _launch_confined only; _SLURM_TIME_LEFT_TO_MINS_SH kept as an alias for tests/test_slurm_timer.py.

🤖 Generated with Claude Code

https://claude.ai/code/session_01StcQgXQDE4F6eJer1sEXRb

[ligon]

Review: needs work

The confined-watchdog fix is real and the snapshotter is genuinely non-destructive — I confirmed that independently. But a documented state file regressed, and the snapshotter can stall the very watchdog it lives inside.

Review note: git worktree add fails on this repo (git-crypt smudge filter on .mcp.json), so this used git archive exports of gh/main and gh/pr9. Nothing was pushed.

Findings

1. The legacy slurm-deadline.warn regressed — sucoder/slurm_timer.py:110,114

CONFIRMED. warn() writes $STATE_DIR/slurm-deadline.warn unconditionally, but the startup rm -f at line 114 clears only $WARN_FILE, the new per-mirror path. On main those were the same file, so it was cleared.

Reproduced: mirror B's timer writes SLURM: ~5 min left (job 999). Commit and save NOW. → job 999 ends → mirror A launches → the legacy file still holds job 999's message, and an agent polling the documented legacy path believes it has five minutes. Two concurrent confined mirrors in one $HOME also interleave on this file.

Inconsistent as well: the "no longer queued" message goes through warn() and so reaches the legacy file, but "tmux session is gone" and "timed out" (lines 118, 148) write only $WARN_FILE — legacy readers never see the terminal states.

This contradicts the description's "State files are per mirror …; the legacy slurm-deadline.warn is still written." It is still written, but it is not per-mirror and it is no longer cleared.

2. snapshot_wip runs synchronously in the deadline loop with no timeout — sucoder/slurm_timer.py:183

CONFIRMED (measured: a pre-receive hook sleeping 6s stalled snapshot_wip for exactly 6s).

git add -A (NFS) and git push (network) block the loop. No timeout, no GIT_TERMINAL_PROMPT=0, no ssh BatchMode. With origin behind a TCP blackhole — VPN drop, firewall — the push hangs for the kernel connect timeout or indefinitely: no squeue poll, no 15- or 5-minute warning, no .warn update, for exactly the window the watchdog exists to cover.

It also means the "every N minutes" cadence is really "every N iterations of (60s + snapshot time)".

3. Mirror names containing . produce invalid git refs, then silently never snapshot — sucoder/slurm_timer.py:85

CONFIRMED. mirror._sanitize_session_token maps [^A-Za-z0-9._-] → _, which is tmux/shell-safe but not ref-safe. Tokens proj.lock, repo., .hidden, my..repo all give fatal: refusing to update ref with bad name 'refs/sucoder/wip/…'. || exit 0 swallows it and stderr goes to /dev/null via the nohup. A mirror named analysis. gets zero snapshots forever with no signal anywhere.

4. The snapshotter has no success or failure channel at all — sucoder/slurm_timer.py:64-89

Every step is || exit 0, and the process is nohup … > /dev/null 2>&1.

Concrete scenario: mirror.py:2380 deliberately wraps the agent in bash -lc because "sbatch runs the batch body with a minimal, non-login env". The timer gets no such wrap — so if git is a module on the cluster, git remote get-url is command not found and the snapshotter is a silent permanent no-op. Same silence for auth failure, a rejected push, or a full $TMPDIR. Nothing is written to $WARN_FILE, the job log, or anywhere else. A one-line status file next to the warn files would be enough.

5. No snapshot at teardown, no SIGTERM trap — sucoder/slurm_timer.py:153-187

Slurm SIGTERMs the step at the wall clock and the timer simply dies. Best case the last snapshot is the 5-minute warning, so roughly five minutes of work is guaranteed unsnapshotted at the deadline; edits landing after that warning are lost. Same on clean agent exit — keeper loop ends, job ends, timer killed mid-cycle. For a feature whose stated purpose is not losing work, trap snapshot_wip TERM INT is the obvious gap.

6. The salloc path did not get the per-mirror treatment — sucoder/cli.py:1207,1218

CONFIRMED. It still writes and executes a single shared $HOME/.cache/sucoder/slurm-timer.sh. _ensure_slurm_node ends by calling _start_slurm_timer unconditionally (cli.py:1132), and _build_executor runs it for every non-confined SLURM command — five call sites — so connect, attach and sync each spawn another timer for the same job, with no lock or pidfile. Each new timer's line-114 rm -f deletes the shared WARN5/15/30 markers, so the older timers re-fire every warning.

Worse: rewriting the script truncates it while previous timers are executing it. Verified that bash re-reads a running script from its saved offset and dies with unexpected EOF (rc=2) when the file is rewritten, even with the loop as the last statement.

The confined path got slurm-timer-<mirror>.sh; the salloc path did not, which is what makes the description's "per mirror" framing only half true.

7. read-tree HEAD gives a stat-less index, so every cycle re-hashes the whole tree — sucoder/slurm_timer.py:74-77

CONFIRMED (measured). The unchanged-tree short-circuit at line 77 fires after add -A / write-tree. On a 4000-file local tree: 0.50s per no-op snapshot vs 0.041s for git status against the real index, about 12×. On the NFS mirror the module's own docstring flags as "not free", repeated every 10 minutes for the job's life, inside the blocking loop of finding 2. Seeding with cp "$(git rev-parse --git-path index)" "$GIT_INDEX_FILE" — falling back to read-tree — keeps the stat cache.

8. Smaller items

• sucoder/config.py:63, README.org:750-762, config.example.yaml — wip_snapshot_minutes is dead config on non-confined targets: cli._start_slurm_timer never passes snapshot_dir or snapshot_minutes, so a salloc target with wip_snapshot_minutes: 10 gets nothing. The README says "Every SLURM-backed session (salloc or confined)".
• sucoder/slurm_timer.py:168-180 — the elif chain de-escalates when the timer starts under 30 minutes; observed ~4 min left … Commit and save NOW. followed by ~3 min left … Start wrapping up. Pre-existing structure, but the PR now hangs a snapshot_wip off each branch, so the mis-ordered branches trigger extra full-tree scans.
• sucoder/slurm_timer.py:74 — read-tree HEAD fails before the first commit, so a freshly-initialised mirror is silently unprotected during exactly the session where losing work hurts most.
• sucoder/slurm_timer.py:75 (PLAUSIBLE) — untracked-but-not-ignored files are swept into every snapshot and force-pushed; a mirror without node_modules/, target/ or *.nc in .gitignore force-pushes a multi-GB tree every 10 minutes.
• sucoder/cli.py:1154 — stale docstring, still says the timer "Writes warnings to $HOME/.cache/sucoder/slurm-deadline.warn".

Tests

The snapshot tests are the strong part: real bash against real repos, asserting non-mutation. I independently confirmed non-mutation under detached HEAD, a conflicted mid-merge (MERGE_HEAD survives, conflict markers snapshotted, worktree untouched), a stale .git/index.lock, and four concurrent snapshots. Nothing touches stash, reflog, branches, or HEAD. Also confirmed separately that snapshot_minutes=0 correctly disables the periodic branch via && short-circuit, and that 10 snapshots immediately at elapsed=0.

The gap: no test executes the timer's main loop, so findings 1, 5, the elif ordering, and the config gap are all invisible to the suite. test_bash_syntax_both_modes covers snapshot_minutes=0 with bash -n only.

test_state_files_are_per_mirror_and_legacy_warn_kept passes precisely because of finding 1 — it asserts the legacy line exists, never that it is cleared or unique.

test_snapshot_unchanged_tree_skips_via_marker overstates its name: only the commit and push are skipped, the full add -A still runs (finding 7).

The rewritten test_slurm_timer_script_omits_scancel is a fair change — on main the script was inline in _start_slurm_timer so inspect.getsource covered it, and after the move it genuinely would have been vacuous.

Claims

Verified true: confined jobs had no watchdog before (no timer/nohup anywhere in main's mirror.py; _start_slurm_timer reachable only from _ensure_slurm_node); the @TOKEN@ template is shared by both paths; staging order is batch script → timer → sbatch, with nohup after the new-session rc check and before the keeper loop; confined mode renders JOB="${SLURM_JOB_ID:-}" and routes every tmux call through TMUX_BIN=(tmux -L <socket>); no origin ⇒ no snapshot; only refs/sucoder/wip/<mirror> is written and no work can be lost; 680 tests pass; mypy unchanged (identical error profile on both refs, none in slurm_timer.py).

Verified false or partial: the per-mirror state-file claim (finding 1) and the shared salloc timer script (finding 6).

Could not check: anything against live Slurm (squeue -o %L on the real cluster, cgroup teardown ordering, SLURM_JOB_ID propagation through nohup); whether git, tmux and squeue are on the batch job's non-login PATH on Savio; real NFS timings; whether the production mirror has an origin; the "verified by hand on n0036.savio4" note at slurm_timer.py:62.

---

🤖 Review generated with Claude Code