mosaicstack/stack
14
0
Fork 1
Code Issues 315 Pull Requests 11 Actions Packages Projects Releases 15 Wiki Activity

D29: revoke-lease.py denies every lifecycle transition for a session that never held a lease #1338

New Issue
Closed
opened 2026-08-20 22:38:14 +00:00 by fred · 0 comments
fred commented 2026-08-20 22:38:14 +00:00
Collaborator
Copy Link
Copy Source

The defect (D29)

packages/mosaic/framework/tools/lease-broker/revoke-lease.py read MOSAIC_LEASE_BROKER_SOCKET and MOSAIC_LEASE_SESSION_ID with direct dict access inside the try block:

socket_path = Path(source_environment["MOSAIC_LEASE_BROKER_SOCKET"])
session_id = source_environment["MOSAIC_LEASE_SESSION_ID"]

KeyError is in that block's except tuple. So 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. Measured on a live seat: pane pid 8596 has zero MOSAIC_LEASE_* variables in /proc/8596/environ, and could not compact.

The fix

A no-op guard before the try block. Absence must be total to qualify: 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.

Evidence

Check Result
python3 src/lease-broker/revoke_noop_unittest.py 5 tests, OK, rc=0
Red control (guard reverted, same tests) FAILED (failures=2), rc=1
check-test-enumeration.sh rc=0, population 62, 47 enumerated, 16 signed exclusions, 67 surface paths all present
prettier --check packages/mosaic/package.json clean

The red control is the part that matters: without the guard, test_no_lease_variables_is_a_noop_success and test_empty_string_counts_as_absent both fail with AssertionError: 2 != 0. The three fail-closed tests pass in both directions, which is what proves the guard did not widen.

What I added on top of the handoff

The test was not enumerated anywhere, so CI would never have run it. Every other src/lease-broker/*_unittest.py is enumerated in test:framework-shell, and the #1017 membership guard cannot catch this: its population is *test*.sh under framework/tools/, so it does not see python suites at all. Added to test:framework-shell, which pnpm test chains.

Provenance and review

The guard and its five tests were written by jarvis-vscode (Jason's VS Code session, not a fleet seat) and handed over uncommitted. fred verified the defect at the source, ran the green and red controls, found and closed the enumeration gap, and committed.

Reviewer must be neither jarvis-vscode nor fred (Gate 16).

Two things this does NOT fix

  1. Why seat panes carry no MOSAIC_LEASE_* at all. That is plausibly a second defect. This PR makes the symptom non-fatal; it does not explain the absence.
  2. packages/mosaic/src/lease-broker/state_store_unittest.py is enumerated nowhere and is therefore also never run in CI. Pre-existing, found while closing the gap above, deliberately not fixed here. Worth its own issue.
## The defect (D29) `packages/mosaic/framework/tools/lease-broker/revoke-lease.py` read `MOSAIC_LEASE_BROKER_SOCKET` and `MOSAIC_LEASE_SESSION_ID` with direct dict access **inside the try block**: ```python socket_path = Path(source_environment["MOSAIC_LEASE_BROKER_SOCKET"]) session_id = source_environment["MOSAIC_LEASE_SESSION_ID"] ``` `KeyError` is in that block's `except` tuple. So 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. Measured on a live seat: pane pid 8596 has zero `MOSAIC_LEASE_*` variables in `/proc/8596/environ`, and could not compact. ## The fix A no-op guard before the try block. Absence must be **total** to qualify: 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. ## Evidence | Check | Result | |---|---| | `python3 src/lease-broker/revoke_noop_unittest.py` | 5 tests, OK, rc=0 | | Red control (guard reverted, same tests) | `FAILED (failures=2)`, rc=1 | | `check-test-enumeration.sh` | rc=0, population 62, 47 enumerated, 16 signed exclusions, 67 surface paths all present | | `prettier --check packages/mosaic/package.json` | clean | The red control is the part that matters: without the guard, `test_no_lease_variables_is_a_noop_success` and `test_empty_string_counts_as_absent` both fail with `AssertionError: 2 != 0`. The three fail-closed tests pass in both directions, which is what proves the guard did not widen. ## What I added on top of the handoff The test was not enumerated anywhere, so **CI would never have run it**. Every other `src/lease-broker/*_unittest.py` is enumerated in `test:framework-shell`, and the #1017 membership guard cannot catch this: its population is `*test*.sh` under `framework/tools/`, so it does not see python suites at all. Added to `test:framework-shell`, which `pnpm test` chains. ## Provenance and review The guard and its five tests were written by jarvis-vscode (Jason's VS Code session, not a fleet seat) and handed over uncommitted. fred verified the defect at the source, ran the green and red controls, found and closed the enumeration gap, and committed. **Reviewer must be neither jarvis-vscode nor fred** (Gate 16). ## Two things this does NOT fix 1. **Why seat panes carry no `MOSAIC_LEASE_*` at all.** That is plausibly a second defect. This PR makes the symptom non-fatal; it does not explain the absence. 2. **`packages/mosaic/src/lease-broker/state_store_unittest.py` is enumerated nowhere** and is therefore also never run in CI. Pre-existing, found while closing the gap above, deliberately not fixed here. Worth its own issue.
fred closed this issue 2026-08-20 23:40:11 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
code-be-01 (Mosaic fleet seat code-be-01) code-infra-01 (Mosaic fleet seat code-infra-01) fargo fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) pepper rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) sanity scooby (Scooby) scrappy shaggy tiny velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1338