Restore deterministic PlUnit aggregate runner #152

Closed

lost-rob0t wants to merge 15 commits from fix/151-plunit-main-ownership into main

lost-rob0t commented

2026-08-21 13:20:49 +00:00

(Migrated from github.com)

Closes #151.

This is intentionally isolated from active #149 and touches only the focused async runner.

Root cause

test/run_tests.pl owns the aggregate initialization(main, main), but it consults test/run_tool_mcp_async_tests.pl, which unconditionally registered a later initialization(focused_main, main). SWI-Prolog executes the last registered main, so CI ran only the focused async cases and false-greened the advertised deterministic PlUnit gate.

Fix

Use SWI's load-context script flag so run_tool_mcp_async_tests.pl registers focused_main only when that file itself is loaded with -s. When consulted by run_tests.pl, it contributes its reusable predicates/tests but cannot replace aggregate main ownership.

This preserves the standalone command:

swipl -q -s test/run_tool_mcp_async_tests.pl

while restoring:

swipl -q -s test/run_tests.pl

to the aggregate runner.

Coordination

Do not fold unrelated #144 runtime code into this PR. #149 is actively owned by another worker and remains the place for the supervised subagent implementation/test oracle. Once this lands, #149 should rebase and its test-only state should become genuinely red as intended.

Keep draft until exact-head CI confirms the deterministic job now executes the full PlUnit corpus and the standalone focused runner remains green.

Closes #151. This is intentionally isolated from active #149 and touches only the focused async runner. ## Root cause `test/run_tests.pl` owns the aggregate `initialization(main, main)`, but it consults `test/run_tool_mcp_async_tests.pl`, which unconditionally registered a later `initialization(focused_main, main)`. SWI-Prolog executes the last registered `main`, so CI ran only the focused async cases and false-greened the advertised deterministic PlUnit gate. ## Fix Use SWI's load-context `script` flag so `run_tool_mcp_async_tests.pl` registers `focused_main` only when that file itself is loaded with `-s`. When consulted by `run_tests.pl`, it contributes its reusable predicates/tests but cannot replace aggregate main ownership. This preserves the standalone command: ```sh swipl -q -s test/run_tool_mcp_async_tests.pl ``` while restoring: ```sh swipl -q -s test/run_tests.pl ``` to the aggregate runner. ## Coordination Do not fold unrelated #144 runtime code into this PR. #149 is actively owned by another worker and remains the place for the supervised subagent implementation/test oracle. Once this lands, #149 should rebase and its test-only state should become genuinely red as intended. Keep draft until exact-head CI confirms the deterministic job now executes the full PlUnit corpus and the standalone focused runner remains green.

lost-rob0t commented

2026-08-22 06:44:01 +00:00

(Migrated from github.com)

Handoff note: Issue #151 false-green gate repair (continuing work)

Passing this to the next agent. Current state lives on local branch fix/151-plunit-gate (commit 490194b + uncommitted changes) in the main checkout at /home/unseen/Documents/Projects/prolog-rlm. This branch supersedes the earlier f0b0486 approach on this PR; do not treat fix/151-plunit-main-ownership as current.

Done (uncommitted, verified locally)

Entrypoint ownership (structural, not reorder-based)
- test/run_tool_mcp_async_tests.pl: initialization(focused_main, main) guarded by prolog_load_context(script, true); standalone swipl -q -s test/run_tool_mcp_async_tests.pl still green (26 cases).
- test/deterministic_corpus.pl (new): sole manifest inventory. term_expansion hook (scoped to the corpus-loading window) rejects any initialization(_, main) inside aggregate inputs before SWI registers it. message_hook ledger converts load-time errors (incl. initialization(..., now) exceptions) into gate failure. Candidate scan requires every test/**/*_test.pl to be include/exclude-classified (live-provider, native/FFI, restart-phase, standalone-runner policies explicit).
- test/deterministic_runner.pl (new): bounded execution — per-test timeout(20), whole-run call_with_time_limit(45), discovery/planned/completed/passed/failed/timeout/blocked/fixme accounting, machine-readable JSON report via PLUNIT_GATE_REPORT, report_is_valid/1.
- test/run_tests.pl: thin entrypoint (corpus load → validate → run). Old 211-line runner deleted; no duplicate inventories.
- test/run_runner_integrity_tests.pl + test/support/*: 19/19 fresh-process adversarial regressions green (hostile main with spoofed sentinels, unstrict-vs-strict load error, missing file, unregistered candidate, empty/subset selection, early/late/multi failures, blocked, condition-skipped, setup failure, duplicates, per-test timeout, SIGTERM, run-budget abort inside and between tests, valid-report happy path).
- .github/workflows/ci.yml: 50s hard process bound; jq cross-validates the single JSON report (semantic consistency, not three independent regexes); integrity suite wired as a CI step.
- README + docs/benchmark-conformance wording updated to state the real bound semantics.

Key diagnostic finding (pre-existing production bug, NOT a runner regression)

The famous hang rlm_authority_lifecycle:cancel_queued_approved_execution_never_mutates_after_workers_release reproduces at commit 88b64db (the commit that introduced the test), verified via a temporary git worktree (since removed). Root cause chain:

rlm_tool:tool_invoke/7 → tool_invoke_async/6 → rlm_async_submit(tool_invoke_execute/6) — the entire invocation, including the authority preflight that must decide approval_required, is queued to the shared bounded scheduler.
Test saturates all 8 workers with blocker tasks, then calls the tool expecting approval_required to be decidable without any worker (that's the documented no-starvation contract; see rlm_authority_test.pl:pending_human_approvals_use_no_shared_workers_and_do_not_starve_work, which passes only because its tool_invoke_async futures happen to get workers).
Preflight never runs → rlm_future_await(Future, infinite) in tool_invoke/7 blocks forever. Checkpoint bisect confirmed the block is inside invoke_lifecycle_pending (i.e. inside tool_invoke), before rlm_approve is even reached.

Intended fix direction (design decision left to next agent): make the authority-preflight/admission phase of tool invocation worker-independent (host-side decision), so only the post-approval effectful execution uses the shared scheduler — consistent with the "pending approvals use no shared workers" invariant and the canonical async invariants in AGENTS.md. Do NOT fix by weakening tests or bumping timeouts. Second latent hang: rlm_effect:parallel_same_effect_has_one_execution_owner (times out at 20s under the watchdog; standalone it passes — order/state dependent). A full-corpus stall was also observed around test ~569 (rlm_conformance) — needs investigation; likely another starvation instance of the same class.

Out of scope (do not fix in this slice)

73 pre-existing test failures now exposed (full list in the local session log; includes all of rlm_effect*, rlm_prompt_compiler, rlm_spec_*, rlm_mcp_declaration_security, etc.). Issue #151 scope is the gate itself; the failures are pre-existing/newly-exposed product bugs. Note many share the "Arguments are not sufficiently instantiated" signature and may have a common root.

Remaining work

Fix the production starvation bug above (TDD: the lifecycle test IS the red test; it exists and hangs).
Re-check rlm_effect:parallel_same_effect... and the conformance stall after the fix.
Re-run full matrix: check_runtime, load_all, integrity suite, timeout 5m swipl -q -s test/run_tests.pl, benchmark deterministic + deep-experiment, CLI demo, git diff --check.
Commit the uncommitted slice (it is currently uncommitted!). Watch out: untracked .env (contains OPENROUTER_API_KEY), .envrc, build.log, proxmox-jobs.sqlite3*, agentProlog/deepseek-harness/ must NOT be committed — stage explicitly, and consider .gitignore additions.
Update this PR/branch reconciliation: this branch is the continuation; the old head on this PR is superseded.

Constraints from AGENTS.md apply: no gate weakening, no test deletion/renaming-away, TDD, one coherent slice, no merge without explicit authorization.

## Handoff note: Issue #151 false-green gate repair (continuing work) Passing this to the next agent. Current state lives on local branch `fix/151-plunit-gate` (commit `490194b` + uncommitted changes) in the main checkout at `/home/unseen/Documents/Projects/prolog-rlm`. This branch supersedes the earlier `f0b0486` approach on this PR; do not treat `fix/151-plunit-main-ownership` as current. ### Done (uncommitted, verified locally) 1. **Entrypoint ownership (structural, not reorder-based)** - `test/run_tool_mcp_async_tests.pl`: `initialization(focused_main, main)` guarded by `prolog_load_context(script, true)`; standalone `swipl -q -s test/run_tool_mcp_async_tests.pl` still green (26 cases). - `test/deterministic_corpus.pl` (new): sole manifest inventory. `term_expansion` hook (scoped to the corpus-loading window) rejects any `initialization(_, main)` inside aggregate inputs before SWI registers it. `message_hook` ledger converts load-time errors (incl. `initialization(..., now)` exceptions) into gate failure. Candidate scan requires every `test/**/*_test.pl` to be include/exclude-classified (live-provider, native/FFI, restart-phase, standalone-runner policies explicit). - `test/deterministic_runner.pl` (new): bounded execution — per-test `timeout(20)`, whole-run `call_with_time_limit(45)`, discovery/planned/completed/passed/failed/timeout/blocked/fixme accounting, machine-readable JSON report via `PLUNIT_GATE_REPORT`, `report_is_valid/1`. - `test/run_tests.pl`: thin entrypoint (corpus load → validate → run). Old 211-line runner deleted; no duplicate inventories. - `test/run_runner_integrity_tests.pl` + `test/support/*`: **19/19 fresh-process adversarial regressions green** (hostile main with spoofed sentinels, unstrict-vs-strict load error, missing file, unregistered candidate, empty/subset selection, early/late/multi failures, blocked, condition-skipped, setup failure, duplicates, per-test timeout, SIGTERM, run-budget abort inside and between tests, valid-report happy path). - `.github/workflows/ci.yml`: 50s hard process bound; jq cross-validates the single JSON report (semantic consistency, not three independent regexes); integrity suite wired as a CI step. - README + docs/benchmark-conformance wording updated to state the real bound semantics. ### Key diagnostic finding (pre-existing production bug, NOT a runner regression) The famous hang `rlm_authority_lifecycle:cancel_queued_approved_execution_never_mutates_after_workers_release` **reproduces at commit `88b64db`** (the commit that introduced the test), verified via a temporary git worktree (since removed). Root cause chain: - `rlm_tool:tool_invoke/7` → `tool_invoke_async/6` → `rlm_async_submit(tool_invoke_execute/6)` — the **entire** invocation, including the authority preflight that must decide `approval_required`, is queued to the shared bounded scheduler. - Test saturates all 8 workers with blocker tasks, then calls the tool expecting `approval_required` to be decidable **without** any worker (that's the documented no-starvation contract; see `rlm_authority_test.pl:pending_human_approvals_use_no_shared_workers_and_do_not_starve_work`, which passes only because its `tool_invoke_async` futures happen to get workers). - Preflight never runs → `rlm_future_await(Future, infinite)` in `tool_invoke/7` blocks forever. Checkpoint bisect confirmed the block is inside `invoke_lifecycle_pending` (i.e. inside `tool_invoke`), before `rlm_approve` is even reached. **Intended fix direction** (design decision left to next agent): make the authority-preflight/admission phase of tool invocation worker-independent (host-side decision), so only the post-approval effectful execution uses the shared scheduler — consistent with the "pending approvals use no shared workers" invariant and the canonical async invariants in AGENTS.md. Do NOT fix by weakening tests or bumping timeouts. Second latent hang: `rlm_effect:parallel_same_effect_has_one_execution_owner` (times out at 20s under the watchdog; standalone it passes — order/state dependent). A full-corpus stall was also observed around test ~569 (`rlm_conformance`) — needs investigation; likely another starvation instance of the same class. ### Out of scope (do not fix in this slice) 73 pre-existing test failures now exposed (full list in the local session log; includes all of `rlm_effect*`, `rlm_prompt_compiler`, `rlm_spec_*`, `rlm_mcp_declaration_security`, etc.). Issue #151 scope is the gate itself; the failures are pre-existing/newly-exposed product bugs. Note many share the "Arguments are not sufficiently instantiated" signature and may have a common root. ### Remaining work 1. Fix the production starvation bug above (TDD: the lifecycle test IS the red test; it exists and hangs). 2. Re-check `rlm_effect:parallel_same_effect...` and the conformance stall after the fix. 3. Re-run full matrix: check_runtime, load_all, integrity suite, `timeout 5m swipl -q -s test/run_tests.pl`, benchmark deterministic + deep-experiment, CLI demo, `git diff --check`. 4. Commit the uncommitted slice (it is currently uncommitted!). Watch out: untracked `.env` (contains OPENROUTER_API_KEY), `.envrc`, `build.log`, `proxmox-jobs.sqlite3*`, `agentProlog/deepseek-harness/` must NOT be committed — stage explicitly, and consider .gitignore additions. 5. Update this PR/branch reconciliation: this branch is the continuation; the old head on this PR is superseded. Constraints from AGENTS.md apply: no gate weakening, no test deletion/renaming-away, TDD, one coherent slice, no merge without explicit authorization.

lost-rob0t commented

2026-08-24 08:04:19 +00:00

(Migrated from github.com)

Superseded by the stronger aggregate runner now on main via #177. Verified current main 7c3561bcc3: runner-integrity 19/19 passed; ownership probe emitted aggregate_main_owner=probe; full gate discovered and completed 74 suites / 762 tests with 762 passed and zero failed, timeout, blocked, or fixme. This stale draft would duplicate and partially weaken current coverage, so it is closed rather than merged.

Superseded by the stronger aggregate runner now on main via #177. Verified current main 7c3561bcc34de55fba875dfd18b773211a3d3420: runner-integrity 19/19 passed; ownership probe emitted aggregate_main_owner=probe; full gate discovered and completed 74 suites / 762 tests with 762 passed and zero failed, timeout, blocked, or fixme. This stale draft would duplicate and partially weaken current coverage, so it is closed rather than merged.