Fix composite transfer validation verdict

[solarcloud7]

Summary

• Compose the live transfer verdict after both gates: strict item validation before activation, then post-activation fluid validation before the controller receives success.
• On a fluid-stage failure, discard the activated destination artifact, preserve/roll back the source, report failedStage=fluids, and make the final debug import-result match the controller verdict.
• Stop the controller from re-fetching/re-deriving validation by platform name; it now consumes the Lua composite payload. The debug validation store is keyed by transfer/job id.
• Add bounded failure_stage metrics, UI failure-stage display, docs, static teeth, and a focused live probe for post-activation fluid loss.

/di-change checklist

• Destructive path touched: yes, destination discard after post-activation fluid gate failure.
• Existing source-safety invariant preserved: source delete still depends on the composite Lua verdict; fluid failure keeps the source.
• Adversarial fixture: tests/integration/fluid-gate-detects-loss injects a measured post-activation fluid shortfall through a debug-gated, one-shot hook and asserts source preserved + destination discarded.
• Test-hook safety: test_force_fluid_loss is debug-gated, consumed once, non-destructive to the destination state itself, and listed in lint:test-hooks as a pre-gate fail-safe hook.
• Data-integrity lint exceptions: none added.
• Review/audit gate: PR opened for audit before merge.

Red-to-green tooth

• Temporarily disabled TransferValidation.validate_fluids_post_activation(result) and patch/reset the cluster.
• node tools/run-integration-tests.mjs --only 'fluid-gate-detects-loss' failed as expected: 1/6 passed; verdict stayed validation_success=True, failedStage=items, fluidCountMatch=True; source was deleted and destination remained.
• Restored the gate, patch/reset again, and the same probe passed 6/6.

Verification

• node --test test/composite-transfer-verdict.test.cjs — pass 6/6
• npm run lint:lua — pass
• npm run lint:test-hooks — pass, 19 tests / 4 hooks
• npm run lint:test-grounding — pass, 19 tests
• npm run lint:pcall-logging — pass
• npm run lint:web-cache — pass
• git diff --cached --check — pass
• node tools/run-integration-tests.mjs --only 'fluid-gate-detects-loss|gate-detects-loss|transfer-fidelity|rollback|engine-invariants|platform-roundtrip' — pass 6/6
• docker exec surface-export-host-1 sh -c 'cd /clusterio/external_plugins/surface_export && npm test' — pass 153, skipped 11, failed 0

Notes

• Final local plugin/module version after the required patch/reset evidence cycles is 0.10.75.
• Five docs appear stat-dirty locally from line-ending/index refresh noise, but they have no content diff and were not staged.

[solarcloud7]

Findings:

1. [P1] Success-only import-complete payload can still delete the source

docker/seed-data/external_plugins/surface_export/instance.ts around lines 721-737 defaults the validation object to fail-closed, but then lets data.success === true override it even when data.validation is missing. That means a malformed Lua/Clusterio payload containing only success: true would emit a successful TransferValidationEvent, and the orchestrator would proceed to delete the source. Current Lua emits success and validation together, but this is the source-delete gate boundary; it should fail closed unless a validation object is present and internally consistent. Please add a negative test for success: true with missing/invalid validation and require success=false in that case.

2. [P3] Green in-game validation message fires before the final fluid gate

docker/seed-data/external_plugins/surface_export/module/core/import-completion.lua prints [Validation] ✓ Validation passed - entities activated... immediately after fluid restore, before TransferValidation.validate_fluids_post_activation(result) runs. On the new fluid-failure path, players/admins can see a success line followed by failure/discard. Move this print after the fluid gate succeeds, or reword it as an intermediate item/activation status.

CI is green, and the composite-verdict shape/test coverage is otherwise strong, but the fail-open boundary needs to close before merge.

[solarcloud7]

Independent code review — high-effort, reconciled against HEAD bc27324

Ran the mandated /code-review (workflow-backed, high effort: 4 finder angles → independent per-location verify pass) that the close-out had skipped. The review scoped the first commit b99d8dc; I then reconciled every surviving finding against current HEAD bc27324.

Already fixed in bc27324 — credit where due:

• ✅ Source-delete fail-open — success is now hasValidationPayload ? (data.success === true && validationSaysSuccess) : false. A bare {success:true} (or one contradicting the count booleans) → false. Negative test added. This was a real fail-open at the source-delete boundary; good catch and fix.
• ✅ Premature green "Validation passed" print — moved into the post-gate else branch, with an ordering test.

Refuted (stated so we don't cry wolf): the fluid gate is not skipped for item-less transfers (totalExpectedItems is always populated on the success path); the unused recon third return is pure style.

---

🔴 BLOCKER — #1: low-temp fluid per-key gate can false-fail and DESTROY a valid arrived platform

module/validators/transfer-validation.lua:22-43 · verdict PLAUSIBLE (mechanism [hypothesis])

This PR promotes fluid reconciliation from a report to a gate that deletes the destination on failure. High-temp fluids (≥HIGH_TEMP_THRESHOLD = 10000 °C) are protected by aggregate-by-name reconciliation (temperature-merge tolerant, lines 46-66). Everything below 10000 °C — all normal fluids AND steam — is gated per exact name@%.1fC key:

elseif expected_volume > 1000 and actual_volume < 1 then   -- "fluid completely lost" → gate fails → dest discarded

If any low-temp fluid restores into a temperature bucket that differs by ≥0.1 °C from export (segment merge → weighted-average temperature; steam is the obvious candidate — 165 °C boiler + 500 °C heat-exchanger sources merging in a shared segment), its exact expected key reads expected>1000 / actual<1 → "completely lost" → a fully valid transfer is rejected and the user's arrived platform is deleted. This is exactly the plan's §5.3 red line: a false-alarming fluid gate is worse than no gate.

The gate's safety rests on an untested assumption: that no fluid below 10000 °C drifts its temperature key on transfer. Pitfall #23 only establishes drift as [empirical] for fusion plasma (>1 M°C); the 10000 threshold's own comment cites a >1,000,000 °C mechanism, so the margin between 10000 and 1 M is unjustified. CI transfer-fidelity being green only proves that fixture doesn't trip it — not that no low-temp config does.

Per the lab discipline (a lab rung is MANDATORY when a data-integrity decision rests on engine behavior not tagged [empirical, current pin]), this must be grounded before merge. Two acceptable resolutions:

• (preferred) Ground it with a fluid-lab rung (spec below). If R10 shows no low-temp key drift across all rungs → tag [empirical, 2.0.77], the gate is safe, merge proceeds. If drift IS observed → the per-key low-temp gate is unsafe and must change.
• Or make it commensurate now: extend the aggregate-by-name protection (or a per-key temperature tolerance) to low-temp fluids, so a key shift within a fluid name isn't read as total loss. (This is the safer default even if R10 comes back clean, since it removes the assumption entirely.)

Lab spec — fluid-lab R10: low-temp temperature-key stability under transfer

Continues the R0–R9 ladder (those isolated fluid loss on deactivation; R10 isolates temperature-key drift — a new variable). Same pattern: append-only NOTEBOOK, controls first, one variable per rung, tick-stamped readings, --reset + zero-leftover proof.

• Question: does a <10000 °C fluid reproduce its exact name@%.1fC key across export → import → post-activation count, or can the key shift enough that the per-key gate reads expected>1000 / actual<1?
• Instrument check (trust the meter first): on the destination, read a known fluid's {name,temperature,amount} via the same path loss-analysis uses to build actualFluidCounts, and confirm the key it produces matches make_fluid_temp_key.
• R10a — control, single-temp: water (15 °C) and crude-oil in a tank+pipe, ~2000 units, transfer, assert the expected key is reproduced exactly and the gate passes. (Establishes the per-key path is trustworthy when temperature is fixed.)
• R10b — the discriminator, mixed-temp steam: a segment fed by two steam sources at different temperatures (boiler 165 °C + heat-exchanger 500 °C) so the live segment settles at a weighted average; >1000 units; transfer; compare the expected low-temp key set to the post-activation actual key set. Does any expected key with volume >1000 map to <1 actual at that exact key (i.e. the volume reappeared under a different name@tempC)? That is the false-fail.
• R10c — boundary: a fluid just under 10000 °C, to check behavior right at the aggregate/per-key threshold.
• Decision: no key drift on any rung → assumption grounded, tag [empirical, 2.0.77] in api-notes, gate is safe. Any drift → per-key low-temp gate is unsafe → adopt aggregate-by-name (or per-key temp tolerance) for low-temp fluids. Record negative/unexplained results honestly.

---

🟠 #2 — unhandled delete_platform failure on fluid-gate reject → duplicate

module/core/import-completion.lua:517-523 · verdict PLAUSIBLE

On fluid-gate failure the destination is already activated, so it's discarded via GameUtils.delete_platform. But a false return (the surfaceless edge, Pitfall #19) is only recorded in destinationDiscarded and logged — no abort / alert / retry / re-quarantine. Source is preserved (correct), the live activated dest survives → two live copies on different instances. Low-probability (delete of a valid, just-activated platform rarely fails) but it sits directly on the duplicate-prevention invariant the whole two-phase-commit design exists to protect. Fail loud + metric (and ideally retry / re-pause) instead of silently leaving a duplicate.

🟡 #3 — post-mortem debug dump lost on throw (observability) — CONFIRMED

module/core/import-completion.lua:610. export_import_result moved to the end of run_phase2, so a throw during activation / fluid restore / loss-analysis (exactly the new failure class) skips it. Also gated on job.transfer_id, so non-transfer uploads lose it. Wrap the post-item-gate phases so the dump still lands on throw, or emit it earlier.

🟡 #4 — success and nil or "items" idiom bug (latent) — PLAUSIBLE

module/core/import-completion.lua:348. On a passing item gate, success and nil or "items" evaluates to "items", so the stored intermediate result briefly reports failedStage="items" for a transfer that passed. Masked today only because validate_fluids_post_activation re-derives it. Fix: result.failedStage = (not success) and "items" or nil.

⚪ #5 / #6 — cleanups (CONFIRMED)

• import-completion.lua:514 re-derives failedStage="fluids" that validate_fluids_post_activation already set — two copies can drift; drop the caller's copy.
• lib/lua-interface.ts:137 getValidationResultJson has no production caller after the re-fetch deletion — remove or clearly mark debug-only.

---

Verdict

Not merge-ready. bc27324 correctly closed the fail-open, but #1 is a genuine blocker: the fluid gate can false-fail and delete a valid platform, and its safety rests on an ungrounded engine assumption — ground it via R10 (or make low-temp commensurate) before this gate is trusted. #2 should land in the same pass; #3-6 are cheap and can ride along. Re-run /di-change after the fix.