fix(lease-broker): no lease held is a no-op success, not a denied transition #1339

Merged

fred merged 2 commits from fix/d29-lease-revoke-noop into next 2026-08-20 23:40:10 +00:00

Conversation 8 Commits 2 Files Changed 4

+141 -4

2 Commits

Author	SHA1	Message	Date
fred	dc6b593c1e	test(#1338): amend runtime_tools pins that encoded the D29 defect semantics ... Two assertions in packages/mosaic/src/mutator-gate/runtime_tools_unittest.py pinned the behaviour D29 identifies as the defect: an envless revoke-lease.py returning rc=2. They are changed here under the test-is-the-defect exemption, stated on the record rather than made quietly. One new assertion is added to cover what the relaxation must NOT reach. Why the reclassification is justified. Not because the old pin was careless: it was deliberate entrypoint doctrine, born in e4d7d45 (WI-3, #842) alongside the tool itself, and its twin pins the identical property for mutator-gate. Two measurements beat the doctrine. 1. Unsatisfiable precondition. The doctrine's only reachable firing point cannot be reached by the live fleet. Seats launch with zero MOSAIC_LEASE_* (measured, pane pid 8596; 19 of 19 live panes, see #1340), the framework ships no provisioning path for fleet launches, and the deployed extension does not enforce the total gate, since live seats use tools daily. 2. Gate-enforced redundancy. For the one case the doctrine protects, a leased session whose identity env is stripped: the strip must occur in the extension host's own env to reach the observer, at which point the gate also loses identity and denies GATE_UNAVAILABLE rc=2 on every tool call (measured). The stripped session may compact but cannot mutate. Pre-fix it merely died at compaction instead. No mutation path opens either way. The no-ungated-mutation property lives in the gate, not the revoker. That is why the revoker moves and the gate does not. Changes: 1. test_revoker_entrypoint_denies_when_identity_environment_is_absent becomes test_revoker_entrypoint_noops_when_identity_environment_is_absent and asserts rc=0. The stem is kept so history greps still find it. 2. NEW: test_revoker_entrypoint_denies_when_identity_environment_is_half_provisioned asserts rc=2 for socket-only and session-only environments. The no-op is reachable only on TOTAL absence; half-provisioned is a machinery-present failure and must still fail closed. main() already pinned this, but the entrypoint did not, and the entrypoint is what the runtime extension spawns. 3. In test_revoker_fails_closed_on_identity_reply_and_transport_errors, ONLY the ({}, ...) element leaves the cases list. The five machinery-present cases (malformed session id, ok:false, state:VERIFIED, OSError, JSONDecodeError) stay fail-closed and are untouched. 4. test_gate_entrypoint_denies_when_identity_environment_is_absent is left UNCHANGED, with a comment recording why. Relaxing it the same way would be a real downgrade. The revoker/gate asymmetry is intentional and must not be "fixed" later. Controls run, all three directions, none assumed: - Fix plus amendments: 26/26 OK, rc=0. - revoke-lease.py alone reverted to origin/next, amendments kept: FAILED (failures=1), rc=1. The amended entrypoint assertion still bites. - Guard weakened to `len(present) < 2` so half-provisioned falls through to the no-op: FAILED (failures=1), rc=1. The new assertion bites. Required by two independent re-verdicts, both REQUEST_CHANGES on head 9fed3838, both of which superseded their own earlier APPROVE after CI surfaced the pinned contract: rev-security-01 review id 242, rev-security-02 review id 243. Each was asked to refute the reclassification before accepting it and each attempted it in writing. Item 2 above is rev-security-02's requirement and was not in the first re-verdict. Both re-affirm the guard's own semantics as correct; what was missing was the second half of a deliberate contract change. Refs #1338, #1340	2026-08-20 18:11:51 -05:00
fred	9fed383884	fix(lease-broker): no lease held is a no-op success, not a denied transition ... revoke-lease.py read MOSAIC_LEASE_BROKER_SOCKET and MOSAIC_LEASE_SESSION_ID with direct dict access inside the try block. A session that never held a lease raised KeyError, fell into the fail-closed handler, and returned rc=2 on every lifecycle transition -- including compaction. Denying compaction to a session that holds no lease protects nothing; it converts a recoverable context limit into a lost session. That is D29. Absence must be TOTAL to qualify for the no-op. If exactly one variable is present the session is half-provisioned, which is real misconfiguration, and it still takes the fail-closed path. An exported-but-empty variable counts as absent. Five unit tests cover the contract: no-op success, vacuous no-op (the broker must not be contacted), both half-provisioned mirrors still rc=2, and empty string as absent. Red control: 2 of the 5 fail without the guard. The test is enumerated in test:framework-shell. Every other src/lease-broker/*_unittest.py is enumerated there, and the #1017 membership guard's population is *test*.sh under framework/tools/, so it does not see python suites -- an unenumerated python test is silently never run. Still open, not fixed here: why seat panes carry no MOSAIC_LEASE_* variables at all.	2026-08-20 17:37:39 -05:00