You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-ups from the review of #902 (non-blocking), tracked here as one issue:
Secrets loader (docker/tps-office-supervisor.sh)
The loader exports every file in /run/secrets as a variable. test(supervisor): the seating-loop control fails on exit forms it cannot verify #902 validates the control values after the load and makes the supervisor's own constants readonly, but a file named PATH, HOME, IFS, LD_*, BASH_ENV (carried into the agent's bash -c by su -m), SECONDS (a huge value overflows the grace deadline and goes straight to KILL) or TPS_SUPERVISOR_TEST_SIGNAL_AFTER_LAUNCH (a valid TERM shuts the office down after the first launch) still replaces that value. The fix accepts only the names the office CLI writes (an allowlist), so every other name is refused.
Shutdown timing
2. tps office stop runs docker stop with Docker's default 10 s timeout. With the default 10 s grace the supervisor now needs 10-11 s, so Docker's KILL can land before the supervisor's own KILL and PID-file removal (kill_stale_pids clears the stale file on the next start). The fix passes a docker stop -t above the supervisor's bound. The office tests' fake docker only knows docker sandbox subcommands and needs updating to cover it.
3. The extra second added in round 10 is pinned only probabilistically (a lower-bound check that catches its removal when the signal lands late in a wall-clock second). Part A has about 0.3-0.5 s of margin under its 3000 ms bound and Part B 3c about 0.5 s under 12 s; a loaded runner could flake.
4. SECONDS follows the wall clock, so an NTP step lengthens or shortens the grace.
Test and comment accuracy
5. The C.5 comment says "as C.1 checks, so it is never nono's own supervisor_pid", but is_descendant_of also accepts equality; the "never equal" fact rests on the real-nono CI result.
6. The PIDS_FILE secret-file fixture could also assert the "readonly variable" message.
Carried from earlier rounds
7. Quote-joined dynamic trap overrides are outside the seating-loop control.
8. The PID-reuse interval between a stop and its confirmation.
One more for this list, from Kern's review of #902 (pre-existing at the base, not a #902 regression):
kill_stale_pids signals every PID number recorded in pids.json without confirming it is still the process the supervisor recorded. The fix records an identity with each PID (its /proc/<pid>/stat start time, or a command-line match) and signals only a PID whose identity still matches, or only supervisor descendants. Pin it with a fixture that plants a stale pids.json whose number belongs to an unrelated live process and asserts that process is not signalled. Land it with item 2 (docker stop -t).
Activity
tps-flint commentedon Nov 10, 2026
One more for this list, from Kern's review of #902 (pre-existing at the base, not a #902 regression):
kill_stale_pidssignals every PID number recorded inpids.jsonwithout confirming it is still the process the supervisor recorded. The fix records an identity with each PID (its/proc/<pid>/statstart time, or a command-line match) and signals only a PID whose identity still matches, or only supervisor descendants. Pin it with a fixture that plants a stalepids.jsonwhose number belongs to an unrelated live process and asserts that process is not signalled. Land it with item 2 (docker stop -t).