nsaspy/prolog-rlm

Fork 0

fix: preserve resolved bindings through recoverable preflight faults #371

Closed

nsaspy wants to merge 2 commits from fix/316-original-batch-effect-isolation into main

nsaspy commented

2026-09-04 22:14:03 +00:00

Owner

Follow-up to #313 / #316. Tightens original-request effect isolation for native call batches.

1. Exact bug, and why #316's tests missed it

#316 classified per-call preflight faults as fault(Call, Cause) — the successfully resolved trusted binding was discarded, and effectful_status/1 only inspected resolved statuses. An effectful call whose arguments failed recoverable validation therefore stopped counting as a requested effectful operation:

call 1: registered effectful/write tool with malformed arguments
call 2: valid read/context call

Result: the batch was no longer recognized as containing a requested effectful operation; the write handler still never executed (no mutation escape), but a valid sibling executed beside it — violating #316's documented invariant that any effectful call in a multi-call ORIGINAL request makes the whole batch fatal before any operation executes.

#316's tests missed it because every effectful-batch case used a valid effectful call (_{value:7}): the effectful call resolved AND validated, so it stayed a resolved status and the check fired. No test exercised "resolves effectfully, then fails argument validation" — the one path where resolution metadata and fault status diverge.

2. New classification/status contract

classify_calls/4 still performs one side-effect-free classification pass (nothing re-run, nothing executed). Per call it now runs two explicit steps, each exactly once:

classify_resolution/3 — resolves against the trusted catalog and remembers the binding; a faulting resolution is remembered as a fault resolution with no binding (unknown/unavailable call → binding:none, recoverable catalog/unavailable_tool_schema when policy allows).
classify_validation/4 — validates arguments for calls that resolved; a recoverable fault becomes:

preflight_fault{call:Call, binding:Binding, cause:Cause}

retaining the resolved binding; successful calls remain resolved_call{call, binding}.

validate_requested_effect_batch/2 reads effect metadata through classified_effect/2 for both status shapes, so effect identity survives recoverable faults:

unknown/unavailable call        -> no binding -> recoverable unavailable_tool_schema
known read call + malformed args    -> read binding preserved -> recoverable malformed_arguments
known effectful call + malformed args -> effectful binding preserved -> per-call fault metadata exists,
                                         BUT original multi-call request is batch-fatal (effect isolation)

Recoverable never means executable: the faulted effectful call has no execution path, and the provider-visible repair observation carries no binding, capability, authority, or handler data.

3. Recoverability policy hardened

The whitelist is now keyed by fault phase + kind (recoverable_fault/2):

schema/malformed_arguments
catalog/unavailable_tool_schema

A same-kind fault emitted by an unrelated runtime phase (e.g. normalize/malformed_arguments) escapes classification and is batch-fatal; future kinds/phases remain fatal by default. Pinned by whitebox closure and wiring tests.

4. RED-before-fix evidence

test/rlm_direct_partial_batch_test.pl grew from 29 to 36 tests. Against the pre-fix branch (commit 5ff492b, tests-only), 6 failed as intended:

malformed_effectful_call_with_valid_read_sibling_is_batch_fatal — batch proceeded, valid sibling executed
valid_read_tool_sibling_does_not_execute_for_malformed_effectful_call — read probe counter reached 1
malformed_effectful_call_with_malformed_read_sibling_is_batch_fatal — batch continued into repair loop
malformed_read_call_with_malformed_effectful_sibling_is_batch_fatal — same, opposite order
only_phase_kind_whitelisted_faults_are_recoverable — recoverable_fault/2 did not exist
same_kind_from_unrelated_phase_escapes_classification — kind-only policy had no phase keying

The singleton malformed-effectful case passed pre- and post-fix (pin: recoverable per-call semantics preserved).

5. Exact deterministic results after the fix

All commands run locally in the worktree on the exact pushed head (c4ad710), all green:

swipl -q -s test/check_runtime.pl — supported runtime
swipl -q -s test/load_all.pl — clean load
swipl -q -s test/run_tests.pl — 1128/1128 passed, 0 failed / timeout / blocked / fixme (main: 1121)
focused rlm_direct_partial_batch: 36/36 pass, no choicepoint warnings
swipl -q -s scripts/validate_research_approval.pl — PASS (19 files)
swipl -q -s scripts/design_gate.pl — all checks passed
swipl -q -s benchmark/run.pl -- deterministic — 16/16, quality 1.00
swipl -q -s benchmark/run.pl -- deep-experiment — 15/15
bin/prolog-rlm.pl -- demo --json + graph trace smoke + trace-view — pass
fresh-process graph restart and artifact restart fixtures — pass
runner main-ownership probe + runner integrity tests — pass
credential-free live test definition loads — pass
git diff --check — clean

5. Effectful handler never executes in malformed effectful multi-call cases

Confirmed: in all four malformed-effectful multi-call tests the mutation counter stays 0, tool_calls =:= 0, and there is no second provider turn (no repair path). The faulted effectful call has no execution path at all — resolved_status/1 excludes it from resolved_batch_budget and execute_calls dispatches it to the observation-only clause.

6. Valid siblings do not execute when original-request effect isolation is fatal

Pinned by valid_read_tool_sibling_does_not_execute_for_malformed_effectful_call: an externally observable read-probe execution counter is 0, plus context_calls =:= 0, tool_calls =:= 0, and no second provider request in the fatal cases. Batch-fatal classification completes before any execution.

7. Docs updated

docs/direct-runtime.md replaces the overly broad fail-closed paragraph with the actual contract: complete batch normalized → one side-effect-free classification pass → batch-fatal invariants against the original request → whitelisted per-call repair observations → valid read siblings may execute → effectful multi-call requests remain entirely batch-fatal even with malformed effectful arguments. It states explicitly: recoverable != executable, visible != authorized, malformed effectful calls never execute, and per-call recovery cannot shrink an unsafe original request into a safe executable batch.

8. Follow-up for native batch cardinality

#323 tracks an explicit max_native_calls_per_batch-style admission limit. Today there is no cardinality cap on one provider response's tool-call list (only implicit provider max_tokens / max_output_bytes / cross-turn budget bounds); faulted calls intentionally consume no max_tool_calls/max_context_ops, and this slice does not silently repurpose those counters.

Adversarial boundary review

Re-inspected normalization → classify → preserve metadata → original-request invariants → assistant validation → execute/observe → repair continuation: no duplicate execution, no preflight re-run, no double charging, no effect laundering, no internal-structure leaks in observations (pinned NO_TERM/NO_EFFECT assertions), call-ID reuse still fatal across turns (faulted IDs marked seen), cancellation still propagates as cancelled (never a repair observation), no valid-sibling execution before batch-fatal checks, classification leaves no pending choicepoints.

Non-goals

#296 model metadata, MCP imported-tool projection, #297 research tool packs, #288 plan graph execution, retrieval/embeddings, planner/scheduler redesign, effect-journal redesign, provider redesign, direct-mode skill architecture — untouched.

Follow-up to #313 / #316. Tightens original-request effect isolation for native call batches. ## 1. Exact bug, and why #316's tests missed it #316 classified per-call preflight faults as `fault(Call, Cause)` — the successfully resolved trusted binding was discarded, and `effectful_status/1` only inspected resolved statuses. An effectful call whose **arguments** failed recoverable validation therefore stopped counting as a requested effectful operation: ```text call 1: registered effectful/write tool with malformed arguments call 2: valid read/context call ``` Result: the batch was no longer recognized as containing a requested effectful operation; the write handler still never executed (no mutation escape), but a valid sibling executed beside it — violating #316's documented invariant that any effectful call in a multi-call ORIGINAL request makes the whole batch fatal before any operation executes. #316's tests missed it because every effectful-batch case used a **valid** effectful call (`_{value:7}`): the effectful call resolved AND validated, so it stayed a resolved status and the check fired. No test exercised "resolves effectfully, then fails argument validation" — the one path where resolution metadata and fault status diverge. ## 2. New classification/status contract `classify_calls/4` still performs one side-effect-free classification pass (nothing re-run, nothing executed). Per call it now runs two explicit steps, each exactly once: - `classify_resolution/3` — resolves against the trusted catalog and remembers the binding; a faulting resolution is remembered as a fault resolution with **no binding** (unknown/unavailable call → `binding:none`, recoverable `catalog/unavailable_tool_schema` when policy allows). - `classify_validation/4` — validates arguments for calls that resolved; a recoverable fault becomes: ```prolog preflight_fault{call:Call, binding:Binding, cause:Cause} ``` retaining the resolved binding; successful calls remain `resolved_call{call, binding}`. `validate_requested_effect_batch/2` reads effect metadata through `classified_effect/2` for **both** status shapes, so effect identity survives recoverable faults: ```text unknown/unavailable call -> no binding -> recoverable unavailable_tool_schema known read call + malformed args -> read binding preserved -> recoverable malformed_arguments known effectful call + malformed args -> effectful binding preserved -> per-call fault metadata exists, BUT original multi-call request is batch-fatal (effect isolation) ``` Recoverable never means executable: the faulted effectful call has no execution path, and the provider-visible repair observation carries no binding, capability, authority, or handler data. ## 3. Recoverability policy hardened The whitelist is now keyed by fault **phase + kind** (`recoverable_fault/2`): - `schema/malformed_arguments` - `catalog/unavailable_tool_schema` A same-kind fault emitted by an unrelated runtime phase (e.g. `normalize/malformed_arguments`) escapes classification and is batch-fatal; future kinds/phases remain fatal by default. Pinned by whitebox closure and wiring tests. ## 4. RED-before-fix evidence `test/rlm_direct_partial_batch_test.pl` grew from 29 to 36 tests. Against the pre-fix branch (commit `5ff492b`, tests-only), **6 failed** as intended: 1. `malformed_effectful_call_with_valid_read_sibling_is_batch_fatal` — batch proceeded, valid sibling executed 2. `valid_read_tool_sibling_does_not_execute_for_malformed_effectful_call` — read probe counter reached 1 3. `malformed_effectful_call_with_malformed_read_sibling_is_batch_fatal` — batch continued into repair loop 4. `malformed_read_call_with_malformed_effectful_sibling_is_batch_fatal` — same, opposite order 5. `only_phase_kind_whitelisted_faults_are_recoverable` — `recoverable_fault/2` did not exist 6. `same_kind_from_unrelated_phase_escapes_classification` — kind-only policy had no phase keying The singleton malformed-effectful case passed pre- and post-fix (pin: recoverable per-call semantics preserved). ## 5. Exact deterministic results after the fix All commands run locally in the worktree on the exact pushed head (`c4ad710`), all green: - `swipl -q -s test/check_runtime.pl` — supported runtime - `swipl -q -s test/load_all.pl` — clean load - `swipl -q -s test/run_tests.pl` — **1128/1128 passed, 0 failed / timeout / blocked / fixme** (main: 1121) - focused `rlm_direct_partial_batch`: **36/36 pass, no choicepoint warnings** - `swipl -q -s scripts/validate_research_approval.pl` — PASS (19 files) - `swipl -q -s scripts/design_gate.pl` — all checks passed - `swipl -q -s benchmark/run.pl -- deterministic` — 16/16, quality 1.00 - `swipl -q -s benchmark/run.pl -- deep-experiment` — 15/15 - `bin/prolog-rlm.pl -- demo --json` + graph trace smoke + trace-view — pass - fresh-process graph restart and artifact restart fixtures — pass - runner main-ownership probe + runner integrity tests — pass - credential-free live test definition loads — pass - `git diff --check` — clean ## 5. Effectful handler never executes in malformed effectful multi-call cases Confirmed: in all four malformed-effectful multi-call tests the mutation counter stays **0**, `tool_calls =:= 0`, and there is no second provider turn (no repair path). The faulted effectful call has no execution path at all — `resolved_status/1` excludes it from `resolved_batch_budget` and `execute_calls` dispatches it to the observation-only clause. ## 6. Valid siblings do not execute when original-request effect isolation is fatal Pinned by `valid_read_tool_sibling_does_not_execute_for_malformed_effectful_call`: an externally observable read-probe execution counter is **0**, plus `context_calls =:= 0`, `tool_calls =:= 0`, and no second provider request in the fatal cases. Batch-fatal classification completes before any execution. ## 7. Docs updated `docs/direct-runtime.md` replaces the overly broad fail-closed paragraph with the actual contract: complete batch normalized → one side-effect-free classification pass → batch-fatal invariants against the original request → whitelisted per-call repair observations → valid read siblings may execute → effectful multi-call requests remain entirely batch-fatal even with malformed effectful arguments. It states explicitly: recoverable != executable, visible != authorized, malformed effectful calls never execute, and per-call recovery cannot shrink an unsafe original request into a safe executable batch. ## 8. Follow-up for native batch cardinality #323 tracks an explicit `max_native_calls_per_batch`-style admission limit. Today there is no cardinality cap on one provider response's tool-call list (only implicit provider `max_tokens` / `max_output_bytes` / cross-turn budget bounds); faulted calls intentionally consume no `max_tool_calls`/`max_context_ops`, and this slice does not silently repurpose those counters. ## Adversarial boundary review Re-inspected normalization → classify → preserve metadata → original-request invariants → assistant validation → execute/observe → repair continuation: no duplicate execution, no preflight re-run, no double charging, no effect laundering, no internal-structure leaks in observations (pinned `NO_TERM`/`NO_EFFECT` assertions), call-ID reuse still fatal across turns (faulted IDs marked seen), cancellation still propagates as `cancelled` (never a repair observation), no valid-sibling execution before batch-fatal checks, classification leaves no pending choicepoints. ## Non-goals #296 model metadata, MCP imported-tool projection, #297 research tool packs, #288 plan graph execution, retrieval/embeddings, planner/scheduler redesign, effect-journal redesign, provider redesign, direct-mode skill architecture — untouched.

nsaspy added 2 commits

2026-09-04 22:14:03 +00:00

test: add RED regressions for original-request effect isolation (#316 ) 5ff492b0c7

4 of 36 tests fail against the current implementation:
- a malformed effectful call loses its resolved binding during
  classification (fault(Call, Cause) drops it), so the effect-batch
  check no longer sees the requested effectful operation and a valid
  read sibling executes beside it
- both orderings (malformed effectful + valid read, valid read +
  malformed effectful) and the both-malformed orderings evade the
  original-request effect-batch invariant
- no phase+kind recoverability policy exists (recoverable_fault/2
  missing); the kind-only whitelist would let an unrelated runtime
  phase emit the same kind and become model-repairable

Also pins: the singleton malformed effectful call stays repairable with
the handler never executing, and no binding/effect metadata leaks into
the provider-visible fault message.

fix: preserve resolved bindings through recoverable preflight faults

Tree-sitter FFI / Direct SWI-Prolog Tree-sitter FFI (pull_request) Successful in 3m12s

Details

CI / REAL OpenRouter integration (pull_request) Has been skipped

Details

Nix flake / Flake package and clean runtime load (pull_request) Failing after 7s

Details

Paid OpenRouter / Pinned paid OpenRouter integration (pull_request) Failing after 1m35s

Details

CI / Deterministic unit and load checks (pull_request) Successful in 3m12s

Details

Clean SWI pack install / Install and load copied pack (pull_request) Successful in 3m46s

Details

c4ad710294

Issue #316's classification converted a recoverable preflight fault into
fault(Call, Cause), discarding the successfully resolved trusted
binding. Effect detection only inspected resolved statuses, so an
effectful call whose arguments failed validation stopped counting as a
requested effectful operation: a valid read sibling could execute beside
it even though the ORIGINAL requested batch contained an effectful
operation, violating #316's own original-request effect-isolation
invariant. The write handler never executed (no mutation escape), but
the batch was no longer batch-fatal as documented.

Classification is now two explicit, side-effect-free steps that each run
exactly once per call:

- classify_resolution/3 resolves the call and remembers the trusted
  binding; a faulting resolution is remembered as a fault resolution
  with no binding (nothing is re-run).
- classify_validation/4 validates arguments for calls that resolved; a
  recoverable fault becomes preflight_fault{call, binding, cause},
  retaining the resolved binding, while faulted resolutions carry
  binding none.

validate_requested_effect_batch/2 now sees effect metadata through
classified_effect/2 for both resolved and faulted statuses, so a
malformed effectful call inside a multi-call request still fails the
whole batch with effectful_batch_unsupported before anything executes.
Recoverable still never means executable: the faulted effectful call has
no execution path, an unavailable call has no binding at all, and the
provider-visible repair observation carries no binding, capability,
authority, or handler data.

The recoverability whitelist is hardened from kind-only to phase+kind
(recoverable_fault/2: schema/malformed_arguments,
catalog/unavailable_tool_schema), so a same-kind fault emitted by an
unrelated runtime phase can never become model-repairable by accident,
and future kinds stay fatal by default.

classify_validation/4 is written as a committed if-then-else so the
classification path leaves no pending choicepoints.

docs/direct-runtime.md now documents the full post-#313/#316 contract:
normalization, one classification pass, original-request batch-fatal
invariants, the whitelisted repair observations, and the
recoverable!=executable / visible!=authorized boundary. A focused
follow-up (#323) tracks an explicit native-call batch cardinality
budget instead of silently repurposing max_tool_calls.

nsaspy closed this pull request

2026-09-04 22:56:16 +00:00