From b648f9ddbe305e57f59e8d09aab2f6ede855ffc6 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Fri, 2 Oct 2026 15:02:03 +0100 Subject: [PATCH] fix(runtime): isolate historical job lookup from PID reuse --- .../wave-3-checkpoint-a.md | 11 ++++++- src/bg_jobs.py | 12 +++++-- tests/test_background_resource_identity.py | 32 +++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/docs/runtime-decomposition/wave-3-checkpoint-a.md b/docs/runtime-decomposition/wave-3-checkpoint-a.md index 1a49fe6a4..43aad681e 100644 --- a/docs/runtime-decomposition/wave-3-checkpoint-a.md +++ b/docs/runtime-decomposition/wave-3-checkpoint-a.md @@ -102,7 +102,11 @@ restoration during cancellation. ## Job history and continuations `peek()` and resolution do not refresh or reap jobs. Output refresh reconciles -only the selected job, including its owned subprocess handle. Stop/output/ack +only the selected job. It polls a cached subprocess handle only while the +selected record is running and its frozen start token still verifies as owned; +historical or unverifiable identities cannot poll a replacement handle under +the same numeric PID. Global service refresh still reaps completed handles. +Stop/output/ack require the caller's exact expected resource and revalidate linkage. Results can update only an explicit result-field whitelist, never identity, owner, generation, receipt, PID, command, path or authority fields. @@ -198,6 +202,11 @@ the integrated gate spans the 145-file manifest. Validation used `/tmp/odysseus-wave3-validation/bin/python` with functional bubblewrap. Compileall, diff whitespace, conflict-marker and unmerged-index gates passed. The post-commit integrated result is recorded in the final checkpoint report. +Final adversarial review found a numeric-PID-only cached-handle lookup in that +commit. A follow-up patch adds frozen-token validation and four PID-reuse/ +unverifiable history regressions, plus a service-cleanup regression. The patched +focused gate passes 392 tests; the patched 145-file integrated gate passes 3369 +tests, with the same 3 platform skips and 2 existing xfails. Static gates pass. Platform skips remain explicit: `/tmp` is not a symlink, RLIMIT_AS can be lowered on this host, and the Windows-specific Ollama startup guard is not applicable on Linux. No missing diff --git a/src/bg_jobs.py b/src/bg_jobs.py index e33b43352..9a258af35 100644 --- a/src/bg_jobs.py +++ b/src/bg_jobs.py @@ -231,8 +231,16 @@ def refresh(job_id=None) -> Dict[str, Dict[str, Any]]: timeout). Idempotent — safe to call from a poll loop. Returns the store.""" jobs = _load() for pid, proc in list(_LIVE_PROCS.items()): - if job_id is not None and pid != jobs.get(job_id, {}).get("pid"): - continue + if job_id is not None: + selected = jobs.get(job_id, {}) + # Historical numeric PIDs can name a replacement child's cached + # handle. Targeted reads may poll only the frozen live incarnation; + # independent service maintenance may still reap completed handles. + if (selected.get("status") != "running" + or pid != selected.get("pid") + or process_ownership.verify(pid, selected.get("start_token")) + != process_ownership.OWNED): + continue if proc.poll() is not None: _LIVE_PROCS.pop(pid, None) changed = False diff --git a/tests/test_background_resource_identity.py b/tests/test_background_resource_identity.py index d72ceb2e0..bfaed06e3 100644 --- a/tests/test_background_resource_identity.py +++ b/tests/test_background_resource_identity.py @@ -215,6 +215,38 @@ def test_target_lookup_does_not_wait_on_unrelated_live_handle(store, monkeypatch bg_jobs.get("job", expected=resource) +@pytest.mark.parametrize("status", ["done", "running"]) +@pytest.mark.parametrize("verdict", [process_ownership.FOREIGN, process_ownership.UNVERIFIABLE]) +def test_historical_lookup_does_not_reap_reused_pid_handle(store, monkeypatch, status, verdict): + resource, rec = seed(store, status=status) + from pathlib import Path + Path(rec["exit_path"]).write_text("0") + Path(rec["result_path"]).write_text(json.dumps({ + "resource_identity": resource.to_dict(), + "containment": {"id": resource.containment_id}, + })) + monkeypatch.setattr(process_ownership, "verify", lambda *a: verdict) + + class ReplacementProcess: + def poll(self): + pytest.fail("Historical lookup reaped the replacement incarnation") + + replacement = ReplacementProcess() + monkeypatch.setattr(bg_jobs, "_LIVE_PROCS", {rec["pid"]: replacement}) + assert bg_jobs.get("job", expected=resource)["status"] == "done" + assert bg_jobs._LIVE_PROCS[rec["pid"]] is replacement + + +def test_service_refresh_still_reaps_finished_handles(store, monkeypatch): + class FinishedProcess: + def poll(self): + return 0 + + monkeypatch.setattr(bg_jobs, "_LIVE_PROCS", {4321: FinishedProcess()}) + bg_jobs.refresh() + assert bg_jobs._LIVE_PROCS == {} + + def test_completed_result_outlives_lifecycle_receipt_without_signalling(store, monkeypatch): resource, rec = seed(store, status="done") from pathlib import Path