From cfd7e612d3f781c7c54c4f32298f7318ef833796 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Wed, 11 Nov 2026 03:20:27 -0600 Subject: [PATCH 1/2] test(cli): bound supervisor cleanup with a fixed process inventory (#1873) --- .../cli/tests/case-cleanup-time-fixture.mjs | 27 ++++++++ packages/cli/tests/case-cleanup-time.test.mjs | 63 +++++++++++++++++++ .../cli/tests/case-inner-timing-fixture.mjs | 15 +++++ 3 files changed, 105 insertions(+) create mode 100644 packages/cli/tests/case-cleanup-time-fixture.mjs create mode 100644 packages/cli/tests/case-cleanup-time.test.mjs diff --git a/packages/cli/tests/case-cleanup-time-fixture.mjs b/packages/cli/tests/case-cleanup-time-fixture.mjs new file mode 100644 index 000000000..1bffa90da --- /dev/null +++ b/packages/cli/tests/case-cleanup-time-fixture.mjs @@ -0,0 +1,27 @@ +// Invoked inside a private PID namespace. Check residual processes BEFORE its +// init exits: namespace teardown would otherwise mask a missed descendant kill. +import { readFileSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { instrumentCleanupTimer } from './case-inner-timing-fixture.mjs'; +import { blockingQuerySource, killRecordedPids, recordedPids, recordedLivePids } from './case-sudo-fixture.mjs'; + +const [script,dir]=process.argv.slice(2),clock=join(dir,"clock '$;literal"),records=join(dir,'pids'); +try { + writeFileSync(clock,'',{mode:0o600}); + const worker=join(dir,'blocked.mjs'); + writeFileSync(worker,blockingQuerySource(records,'sleep')+'\nloadSystem();'); + const source=instrumentCleanupTimer(readFileSync(script,'utf8'),clock); + const body=source.slice(source.indexOf('# row253-supervisor-begin'),source.indexOf('# row253-supervisor-end')); + const result=spawnSync('/bin/bash',['-c',body+'\nexec /bin/bash -c "$TESTER_SUPERVISOR" -- 2 /usr/bin/setsid /usr/bin/sleep "$@"','--',process.execPath,worker],{ + encoding:'utf8',timeout:12000,killSignal:'SIGKILL', + }); + // These PIDs belong to this namespace; never use them to signal host PIDs. + process.stdout.write(JSON.stringify({ + status:result.status,signal:result.signal,error:result.error?.message, + stdout:result.stdout,stderr:result.stderr,pairs:recordedPids(records), + live:recordedLivePids(records),clock:readFileSync(clock,'utf8'), + })); +} finally { + killRecordedPids(records); +} diff --git a/packages/cli/tests/case-cleanup-time.test.mjs b/packages/cli/tests/case-cleanup-time.test.mjs new file mode 100644 index 000000000..b9025f2cf --- /dev/null +++ b/packages/cli/tests/case-cleanup-time.test.mjs @@ -0,0 +1,63 @@ +// A delay in finish(), after timing2's end stamp, must fail independently of +// the inner timer. One fixed process tree runs at a time, without host setup, +// fake sudo, or a parent-side elapsed-time assertion. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { instrumentCleanupTimer, removeInnerObservers, readProbeSeconds } from './case-inner-timing-fixture.mjs'; + +const scripts=['case1-accept.sh','case2-unit.sh']; +const read=name=>readFileSync(new URL(`../../../scripts/${name}`,import.meta.url),'utf8'); +// The namespace fixes /proc inventory size; it does not replace clean_session. +// See org-cleanup-time-build.md for the standalone and loaded calibration. The +// bound must reject 1s/2s delays and leave most of the 2s production headroom. +const CLEANUP_LIMIT_SECONDS=0.5; +const probe=fileURLToPath(new URL('./case-cleanup-time-fixture.mjs',import.meta.url)); + +for(const name of scripts)test(`cleanup timing ${name}: observers preserve bytes and reject exit-anchor drift`,()=>{ + const source=read(name),clock="/unused/clock '$;literal"; + const copy=instrumentCleanupTimer(source,clock); + assert.equal(removeInnerObservers(copy,clock),source); + for(const marker of ['SO_PEERCRED','MOSAIC_TESTER_SOCKET_V1','MOSAIC_LEGACY_STATUS']) + assert.equal(copy.split(marker).length,source.split(marker).length,marker); + const exit=' exit "$rc"\n'; + assert.throws(()=>instrumentCleanupTimer(source.replace(exit,''),clock),/anchor/); + assert.throws(()=>instrumentCleanupTimer(source.replace(exit,exit+exit),clock),/anchor/); +}); + +// Both shipped copies are executed: Case1 shares this supervisor across legacy +// calls, queries and P10; Case2 uses its copy for config queries. +for(const name of scripts)test(`cleanup timing ${name}: inner timeout kills descendants within the cleanup budget`,t=>{ + const dir=mkdtempSync(join(tmpdir(),'cl-')); + t.after(()=>rmSync(dir,{recursive:true,force:true})); + // No skip/fallback: this Linux test requires unprivileged user/PID/mount + // namespaces. --kill-child also removes our namespace if the probe times out. + const outer=spawnSync('/usr/bin/unshare',[ + '--user','--map-root-user','--pid','--fork','--kill-child=KILL','--mount-proc', + process.execPath,probe,fileURLToPath(new URL(`../../../scripts/${name}`,import.meta.url)),dir, + ],{ + encoding:'utf8',timeout:15000,killSignal:'SIGKILL', + }); + assert.equal(outer.error,undefined,outer.error?.message); + assert.equal(outer.status,0,outer.stdout+outer.stderr); + assert.equal(outer.stderr,''); + const result=JSON.parse(outer.stdout); + assert.equal(result.error,undefined,result.error); + assert.equal(result.signal,null,result.stderr); + assert.equal(result.status,81,result.stderr); + assert.equal(result.stdout,''); + assert.equal(result.stderr,''); + const pairs=result.pairs; + assert.equal(pairs.length,2,'the session leader and its live descendant both started'); + assert.equal(pairs[0][0],pairs[0][1],'the command leads its kernel session'); + assert.notEqual(pairs[1][0],pairs[0][0]); + assert.equal(pairs[1][1],pairs[0][1],'the descendant belongs to the supervised session'); + assert.deepEqual(result.live,[],'cleanup must kill the whole process tree before namespace teardown'); + const seconds=readProbeSeconds(result.clock); + t.diagnostic(`${name}: cleanup ${seconds}s; limit ${CLEANUP_LIMIT_SECONDS}s`); + assert.ok(seconds Date: Wed, 11 Nov 2026 03:49:22 -0600 Subject: [PATCH 2/2] test(cli): clarify cleanup timing limits and namespace failures (#1873) --- packages/cli/tests/case-cleanup-time.test.mjs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/tests/case-cleanup-time.test.mjs b/packages/cli/tests/case-cleanup-time.test.mjs index b9025f2cf..e6fa10af5 100644 --- a/packages/cli/tests/case-cleanup-time.test.mjs +++ b/packages/cli/tests/case-cleanup-time.test.mjs @@ -15,6 +15,8 @@ const read=name=>readFileSync(new URL(`../../../scripts/${name}`,import.meta.url // The namespace fixes /proc inventory size; it does not replace clean_session. // See org-cleanup-time-build.md for the standalone and loaded calibration. The // bound must reject 1s/2s delays and leave most of the 2s production headroom. +// Constant delays below 0.5s can pass; this is not a bound on host-wide scan +// cost. In particular, per-entry work needs a shipped scan-cost follow-up. const CLEANUP_LIMIT_SECONDS=0.5; const probe=fileURLToPath(new URL('./case-cleanup-time-fixture.mjs',import.meta.url)); @@ -42,8 +44,9 @@ for(const name of scripts)test(`cleanup timing ${name}: inner timeout kills desc ],{ encoding:'utf8',timeout:15000,killSignal:'SIGKILL', }); - assert.equal(outer.error,undefined,outer.error?.message); - assert.equal(outer.status,0,outer.stdout+outer.stderr); + const prerequisite='requires /usr/bin/unshare and unprivileged user/PID/mount namespaces (check kernel/AppArmor/container seccomp)'; + assert.equal(outer.error,undefined,`${prerequisite}: ${outer.error?.message}`); + assert.equal(outer.status,0,`${prerequisite}; probe output:\n${outer.stdout}${outer.stderr}`); assert.equal(outer.stderr,''); const result=JSON.parse(outer.stdout); assert.equal(result.error,undefined,result.error); @@ -57,6 +60,9 @@ for(const name of scripts)test(`cleanup timing ${name}: inner timeout kills desc assert.notEqual(pairs[1][0],pairs[0][0]); assert.equal(pairs[1][1],pairs[0][1],'the descendant belongs to the supervised session'); assert.deepEqual(result.live,[],'cleanup must kill the whole process tree before namespace teardown'); + // A positive centisecond pair relies on clean_session's current mandatory + // 0.01s retry sleep. Revisit this measurement if shipped cleanup removes it; + // keep the shared inner-timer parser strict in the meantime. const seconds=readProbeSeconds(result.clock); t.diagnostic(`${name}: cleanup ${seconds}s; limit ${CLEANUP_LIMIT_SECONDS}s`); assert.ok(seconds