windows runtime (#567): the job fails in teardown while every test passes — a file is still held under the temp dir

[fujibee]

The bats (windows-latest, windows runtime (#567)) job fails with both of its tests passing. What fails is the teardown:

ok 1 codex-monitor: windows-native reaches the bridged handoff (#567)
# (from function `teardown_test_env' in file tests/test_helper.bash, line 38,
#  from function `teardown' in test file tests/test_codex_monitor.bats, line 76)
#   `teardown_test_env' failed
# rm: cannot remove '/tmp/tmp.FvPvYCFV64': Directory not empty

bats attributes a teardown failure to the test that ran before it, so the not ok line names a test whose body succeeded. Reading only the test name is misleading here.

The same shape on main, with a different message and a different test

run 33342369448 (main)
  ok 1     codex-monitor: windows-native reaches the bridged handoff (#567)
  not ok 2 launcher: windows-native starts the bridge (#567)
  teardown_test_env failed
  rm: cannot remove '.../db/messages.db':     Device or resource busy
  rm: cannot remove '.../db/messages.db-shm': Device or resource busy
  rm: cannot remove '.../db/messages.db-wal': Device or resource busy

So this is not tied to one test: which of the two tests carries the failure moves, and the rm error text differs — Device or resource busy on the SQLite files in one run, a bare Directory not empty on the temp root in another. Both are the same underlying condition: something still holds a file under the test's temp directory when teardown runs.

Device or resource busy on messages.db, -shm and -wal together points at a SQLite connection that has not closed. The most likely holders are the codex bridge / launcher processes these two tests start, which on Windows keep the file locked after the test body returns. Directory not empty is the same story seen one level up, after the individual unlinks failed.

Why it matters more than a flaky count

The job's exit code is 1 while every test reported ok. A summary that reads test results — or a human reading the last few lines — sees passes. Only the exit status and the # rm: lines say otherwise. A real regression in either of these two tests would look exactly like this in any view that stops at the test list.

Not fixed here

Filed rather than fixed: teardown ordering on Windows for the processes these tests launch is its own piece of work, and it is not what the branch that surfaced this was changing. The branch in question modifies scripts/drivers/types/codex/template.md and nothing in codex-monitor.sh, the launcher, or tests/test_helper.bash.

[fujibee]

Another occurrence, same shape, recorded before rerunning.

run 33962200250   PR #1046 @ 2c223eb   bats (windows-latest, windows runtime (#567))

not ok 1 codex-monitor: windows-native reaches the bridged handoff (#567)
# (from function `teardown_test_env' in tests/test_helper.bash, line 38,
#  from function `teardown' in tests/test_codex_monitor.bats, line 76)
#   `teardown_test_env' failed
# rm: cannot remove '/tmp/tmp.WdLG92ov1h': Directory not empty
ok 2 launcher: windows-native starts the bridge (#567)

The body passed; teardown_test_env is what failed, and bats attributes that to
the preceding test. The second test in the same file passed afterwards, so the
job's single red is entirely the cleanup.

That branch changes watch.sh and inbox.sh and touches nothing under
scripts/drivers/types/codex/.

[fujibee]

Third occurrence today, same shape, on a third branch. Recorded before rerunning.

run	head	branch
33850939213 att1	d2bd809	terminal-driver-v1
33962200250	2c223eb	#1046
33965718355	7ec9f81	#1047

Always:

not ok 1 codex-monitor: windows-native reaches the bridged handoff (#567)
#   `teardown_test_env' failed
# rm: cannot remove '/tmp/tmp.XXXXXXXXXX': Directory not empty
ok 2 launcher: windows-native starts the bridge (#567)

The body passes, the next test passes, and the job's only red is the cleanup
between them. Three different branches, none of which touches
scripts/drivers/types/codex/ or tests/test_helper.bash.

This is now blocking landings rather than merely being noise: each occurrence
costs a rerun of a job that was otherwise green, and reruns are the wrong
instrument for something reproducing this reliably.

The direction worth checking first: on Windows an open handle prevents removal,
where the same code on Linux unlinks happily. So the question is not "why does
rm fail" but which process still holds a file under that directory when
teardown runs — i.e. whether the test's own child (bridge/monitor) is being
waited for before cleanup, or merely signalled.

[fujibee]

Inventory of the shape, so the next one has a denominator

Re-derived against 407025b (this PR's head); the derivation is one command, at
the bottom, so the count can be repeated rather than trusted.

The shape. A background process whose output is redirected into
$TEST_SKILL_DIR — the directory the shared teardown_test_env removes. On
POSIX an open file does not stop its directory being unlinked; on Windows it
does, which is why this is a one-platform failure.

Positive control first. The pattern is checked against the two sites already
known to be of this shape. They do not appear in a scan of the test lines,
because there the log is opened script-side —
codex-monitor.sh:113 builds SERVER_LOG="$RUN_DIR/codex-app-server.$HASH.log"
and line 188 redirects the app-server into it. That is what puts them in the
table by hand rather than by grep, and it is the reason the test-line scan alone
is not the whole answer.

Correction to the count I gave earlier

I reported 11 (10 test-side + 1 script-side). Re-derived, it is 17
(15 test-side + 2 script-side). The earlier regex only reached the
mock_remote_server.py-style launches and missed the
… > "$TEST_SKILL_DIR/se.out" 2>&1 & shapes in test_remote.bats, five of them.
The number below is the one to build on.

Test-side (15)

file:line	backgrounded	outlives the test unless stopped	stopped
test_inbox.bats:70	inbox.sh	no — barrier-gated, exits	—
test_inbox.bats:327	check-inbox.sh	no — barrier-gated, exits	—
test_perf_harness.bats:34	mock_remote_server.py	yes	kill @ 25 (teardown)
test_remote.bats:32	mock_remote_server.py	yes	kill @ 65
test_remote.bats:104	mock_remote_server.py	yes	kill @ 91
test_remote.bats:406	remote.sh set-endpoint	no — barrier-gated	—
test_remote.bats:443	remote.sh set-endpoint	no — barrier-gated	—
test_remote.bats:470	remote.sh set-endpoint	no — barrier-gated	kill @ 465
test_remote.bats:1156	remote.sh disconnect	no — barrier-gated	—
test_remote.bats:1219	remote.sh disconnect	no — barrier-gated	—
test_remote.bats:2989	mock_remote_server.py (server-b)	yes	kill @ 68
test_team_list.bats:55	mock_remote_server.py	yes	kill @ 65
test_team_list.bats:78	mock_remote_server.py	yes	kill @ 85
test_watch.bats:217	watch.sh	yes	kill @ 227
test_watch_once.bats:127	watch-once.sh --timeout 5	bounded at 5 s	—

Script-side (2), reached through codex-monitor.sh

site	backgrounded	outlives the test unless stopped	stopped
test_codex_monitor.bats:170	node app-server fake, setTimeout(…, 60000)	yes, up to 60 s	wrapper forwards kill (line 181)
test_codex_monitor.bats:365	node app-server fake, setTimeout(…, 60000)	yes, up to 60 s	wrapper forwards kill (line 375)

What the table says, and what it does not

The dangerous property is not "is backgrounded" — it is outlives the test
unless something stops it. Nine of the seventeen have it; every one of those
nine is signalled somewhere.

So this table does not explain the failure, and an earlier reading of mine
that said it did was withdrawn: I reported that the failing site (365) lacked
the kill-forwarding its sibling (170) has. Both have it, identically. What the
failing one additionally had was no wait — the signal was sent and the
teardown moved on, which is enough on POSIX and not enough on Windows. That is
what the teardown in this PR now does, and what the reporter records when the
removal still fails.

Reproducing the count

python3 - <<'PY'
import re, pathlib
RED = re.compile(r'(?:\d*>|>)\s*"?\$\{?TEST_SKILL_DIR\}?/')
n = 0
for f in sorted(pathlib.Path('tests').glob('*.bats')):
    for i, l in enumerate(f.read_text().splitlines(), 1):
        if re.search(r'(?<!&)&\s*$', l) and RED.search(l):
            print(f"{f.name}:{i}"); n += 1
print("test-side:", n, "(+2 script-side via codex-monitor.sh SERVER_LOG)")
PY

[fujibee]

The same shape on ubuntu, and this time the dump names a live process inside the tree

Recorded before any rerun.

run 33982822565  job "bats (ubuntu-latest 1/4)"  head 407025b32153 (PR #1049)

not ok 357 codex-monitor: never kills a non-codex process recorded under a reused pid
not ok 359 codex-monitor: reuses a live app-server when tasklist cannot see it (#567)

# `teardown_test_env' failed
# rm: cannot remove '/tmp/tmp.nwVc6v7jyN': Directory not empty

Until now every occurrence of this was Windows. This one is Linux, where an
open file does not hold its directory — so the mechanism has to be different,
and the dump shows what it is.

What the reporter printed

##### what is still there:
/tmp/tmp.nwVc6v7jyN:
drwxr-xr-x  2 runner runner 4096 run          <- present
/tmp/tmp.nwVc6v7jyN/run:
(empty)                                       <- and empty

##### pids a teardown CALLED kill AND wait ON (captured before the rm):
  275172 exited
##### pid FILES present before the rm — NOT known to have been signalled:
  (none)

An empty directory that rm could not remove is rmdir returning
ENOTEMPTY: an entry appeared between the unlink sweep and the rmdir, and was
gone again by the time the ls ran.

The process inventory names the creator

runner  275360  1  bash /tmp/tmp.nwVc6v7jyN/scripts/drivers/types/codex/codex-bridge-launcher.sh \
                     codex /tmp/tmp.X7BNaJ0KWw ws://…

A live codex-bridge-launcher.sh, running from inside the directory being
removed, reparented to init (PPID 1), and in neither pid column — the teardown
never signalled it, because it only signals run/codex-app-server.*.pid.

And that launcher writes into the very directory that failed to go away:

codex-bridge-launcher.sh:171   IDENTITY_CACHE_MARKER="$RUN_DIR/.identity-cache.$$"
codex-bridge-launcher.sh:192   : > "$IDENTITY_CACHE_MARKER"
codex-bridge-launcher.sh:88    rm -f "$IDENTITY_CACHE_MARKER"

A file created and deleted inside run/ while rm -rf is walking it is exactly
the ENOTEMPTY-on-an-empty-directory above. It is a dotfile, so it is invisible
to the run/*.pid scan and would not have shown in a plain ls either.

What this does and does not establish

Does: a process the teardown does not signal is alive inside the tree at
removal time, it writes transient files into run/, and that is a sufficient
cause for a Linux ENOTEMPTY. Same class as this issue — a live process inside
the tree — different platform, different process.

Does not: that PR #1049 causes it. The launcher, its marker file and the
codex-app-server-only teardown are all pre-existing. What is measured is only
that bats (ubuntu-latest 1/4) was red on both of that branch's heads
(fec21ab, 407025b) and, across the last 15 main runs, the one red in that
job was a different test (join: concurrent joins…, run 32944981464). Two
observations are not a cause, and the branch does add work before the rm,
which moves the timing. Unresolved.

bats (macos-latest 4/4) also failed on that head, on
not ok 291 watch: exits when its session dies without consuming an undelivered row (#67). That job fails on main too (runs 33939222969, 33360811033), so it
is not from this branch.

[fujibee]

Same underlying shape found while root-causing the Linux side of this (the codex-monitor Directory not empty teardown reds). The general form: a detached child that the test teardown never kills holds files under TEST_SKILL_DIR while teardown_test_env removes the tree. Named concretely, it matches this issue.

The holder: codex-monitor.sh spawns codex-bridge-launcher.sh detached (… &, comment: "the launcher is detached on purpose and outlives this script"). The launcher resolves its own SKILL_DIR from its script path (codex-bridge-launcher.sh SKILL_DIR="$SCRIPT_DIR/../../../.."), which under test is TEST_SKILL_DIR, so it writes into TEST_SKILL_DIR/run (mkdir -p "$RUN_DIR" recreates the dir, plus an atomic-rename temp, a bridge pidfile, an identity-cache marker) and then starts codex-bridge.js, which opens db/messages.db.

Why it survives teardown: the codex-monitor teardown kills only the codex-app-server.*.pid set — never this launcher or the bridge it starts. So after the test body returns, the launcher/bridge are still live: on Windows they keep run/ and the SQLite messages.db/-shm/-wal locked, so the individual unlinks fail → rm: Device or resource busy on the db files, or rm: Directory not empty one level up on the temp root. That explains both this issue's observations: the failing-test name and the error text move because the holder is a background process racing an unlink, not a specific test.

Consistent with the reported failures: both codex-monitor: windows-native reaches the bridged handoff (#567) and launcher: windows-native starts the bridge (#567) spawn exactly this launcher/bridge; the issue's own note ("the most likely holders are the codex bridge / launcher processes these two tests start") points at the same processes.

What is measured vs not: the launcher spawn and its TEST_SKILL_DIR/run writes are confirmed on Linux (it fires 3× in the codex-monitor reuse tests, each with a real ws:// url + the app-server ppid). Those Linux tests are skip_on_windows, but the windows-native handoff test reaches the same launcher. The Windows-specific locking semantics (a live process keeps the file unremovable) I did not run directly — no Windows runner here — so the Windows step rests on this issue's own Device or resource busy evidence plus the Linux mechanism, not a fresh Windows measurement.

Fix direction (not done here): the teardown must tear down the launcher and its bridge (their run/ pidfiles + the bridge's messages.db connection), not only the app-server pid. Note the Linux codex-monitor unit tests can simply stub the launcher (they don't exercise the bridge), but the windows-native test is about the bridged handoff, so it needs the real bridge and must instead kill it in teardown.