Skip to content

refactor(#45 PR 2b): boundary-sensitive migration — screen cascade, /tmp relocation, exec-env, display/mode#78

Merged
aradanmn merged 7 commits into
mainfrom
feat/issue-45-pr2b-boundaries
Jul 11, 2026
Merged

refactor(#45 PR 2b): boundary-sensitive migration — screen cascade, /tmp relocation, exec-env, display/mode#78
aradanmn merged 7 commits into
mainfrom
feat/issue-45-pr2b-boundaries

Conversation

@aradanmn

@aradanmn aradanmn commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

PR 2b of #45 — the behavior-sensitive half. Four commits, each alone in its risk class for bisect:

  1. Screen cascade (closes MCSS_ namespace hygiene: prefix should mean runtime_context-owned #53 ride-along): window_manager._get_screen_resolution DELETED — mcss_resolve_screen is the one cascade. Deltas: env-override-FIRST, kscreen eDP-filter fix, retain-last-good on refresh. Entry's 4 probe blocks + sweep pair migrated; launchNested uses --no-probe (gamescope kills throwaway Wayland clients); DISPLAY_MODE_CHANGE now re-probes via --refresh. _ns_geo deliberately kept (measures the just-created nested root). Also migrated the ORCHESTRATOR_MAX_SLOTS straggler 2a missed. window_manager's module-local MCSS_* renamed WINDOW_MANAGER_* (old names honored one release).
  2. /tmp → $MCSS_HELPER_DIR (closes N6/N7): kwin wrapper shim (PATH-injected + executed by startplasma), session-env snapshot, and kwin_positioner's generated .js all move to the 0700 runtime dir. PATH=/tmp:$PATH is gone.
  3. Exec-env rewiring (top-risk, isolated): all four re-exec boundaries emit $(mcss_exec_env_string ...) — canonical context/state/FIFO/log + caller extras (env last-wins verified). MCSS_NESTED_SESSION value space widens 1plasma|kwin; reader is now != 0. Emitter switched %q → raw word-safe (a .desktop Exec / env(1) argv never shell-unquotes; unsafe values are refused loudly). Flavor-differentiated teardown deliberately deferred to D6 — nested-session teardown / Steam Abort-Game #15/Supervised reap never ran (errexit '(( var++ ))'); orphaned run trees cross-kill newer sessions via shared state/FIFO #60 Phase 3.
  4. MCSS_DISPLAY / MCSS_MODE: dex's source-time DISPLAY capture (latent pre-nested-X bug) deleted — call-time resolution, mcss_set_display fires when the nested X socket is confirmed. PULSE_SERVER derived once (×3 id -u sites). FIFO inline path-defaults → bare reads. _set_mode = single writer of the exported mirror + state .mode.

Plan deviations

  • test_orchestrator NOT added to CI baseline: the suite is inert on main (silent exit-0 before any test when sourcing the entry script — pre-existing, verified on unmodified main). Gating it would hard-fail CI. Needs its own repair issue.

Tests

+T19 (extras last-wins), +T20 (word-unsafe refusal); runtime_context baseline 18→20. All suites at/above baseline locally (7/12, 11/11, 10/10, 7/9, 8/8, 20/20).

Deck validation plan (merge gate: full stage0b–6 incl. stage2 handheld + stage3 hot-plug)

The boundary commits need the flows that cross them:

  • prod flow (launch from library): one continuous debug log across both halves, [runtime_context] env=gamescope in the nested child, wrapper found from $MCSS_HELPER_DIR (nested session at panel size), no stranded autostart after quit
  • testPlasma flow: TEST_NUMBER/observe-delay carried through the rewired Exec line
  • docked on external display (non-1280x800 ideal) + handheld launch (1280x800 path) + one hotplug transition (screen --refresh + mode interplay)
  • stage6 teardown matrix

Part of #45.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm

Scott and others added 5 commits July 10, 2026 14:42
…y probes replaced; closes #53

DELETED window_manager._get_screen_resolution (+ WINDOW_MANAGER_DEFAULT_SCREEN_W/H):
mcss_resolve_screen is the one cascade. Behavior deltas, deliberate:
- env-override-FIRST (the old copy checked SPLITSCREEN_SCREEN_W/H after all
  four probes, so forced test dimensions lost to the live display)
- kscreen eDP filtered before head (docked Deck picked the internal panel)
- retain-last-good on failed --refresh

Entry script: the four identical RES=$(xdpyinfo…) blocks (nestedPlasma,
runStaticTest, testPlasma, launchFromPlasma) and the position-sweep pair now
call mcss_resolve_screen; launchNested uses --no-probe (gamescope kills
throwaway Wayland clients — the resolver flag exists for exactly that path).
_nestedSession's _ns_geo xdpyinfo DELIBERATELY kept: it measures the
just-created nested X root, not the host screen — different semantic.

Orchestrator: DISPLAY_MODE_CHANGE handler now runs mcss_resolve_screen
--refresh so post-transition reflows tile against the new output; also
migrated the ORCHESTRATOR_MAX_SLOTS=4 straggler (missed by 2a's sweep) to
MCSS_MAX_PLAYERS + defensive runtime_context source.

#53: window_manager's module-local MCSS_SKIP_UNCHANGED / MCSS_REASSERT /
MCSS_REASSERT_DELAY_S renamed WINDOW_MANAGER_* (MCSS_ prefix now means
runtime_context-owned); old names honored one release as fallbacks.

Part of #45 (PR 2b commit 1/4). Closes #53.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
…$MCSS_HELPER_DIR

Closes security audit items N6/N7 structurally. Everything the session
GENERATES-then-EXECUTES now lives in the 0700 per-user helper dir
($XDG_RUNTIME_DIR/mcss, resolved once by mcss_resolve_paths):

- kwin_wayland_wrapper shim ×3 (nestedPlasma/testPlasma/launchFromPlasma) →
  $MCSS_KWIN_WRAPPER_PATH; PATH injection becomes $MCSS_HELPER_DIR:$PATH
  (was PATH=/tmp:$PATH — any local user could pre-plant an executable there
  that startplasma would run)
- session-env snapshot → $MCSS_SESSION_ENV_BAK (re-sourced on restore)
- kwin_positioner's generated one-shot .js ×2 → mktemp in $MCSS_HELPER_DIR
  (KWin executes these); module defensively sources runtime_context +
  resolves paths at function top — kwin-place-test sources it standalone
- dex.sh stale '/tmp/dex_backend.py' help text corrected

Bonus over /tmp: the runtime dir is cleared at logout, so stale wrappers
can't leak across sessions.

Part of #45 (PR 2b commit 2/4).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
…env_string

The top-risk commit of PR 2b, deliberately alone. The three autostart Exec=
lines (nestedPlasma/testPlasma/launchFromPlasma) and the kwin session command
previously hand-listed their env — only 2 of ~7 vars were common to all four,
and the autostart lines carried NO state/FIFO vars at all. All four now expand
$(mcss_exec_env_string ...) at write time: origin context (MCSS_ENV_CONTEXT,
resolved in-function so it can never be silently absent), state, FIFO, debug
log, observe delay, TEST_NUMBER, plus caller extras that win by env(1)
last-wins ordering (verified: canonical =0 then extra =plasma → child sees
plasma).

MCSS_NESTED_SESSION value space widens 1 → plasma|kwin (writers tagged by
boundary flavor); the dispatch reader becomes != "0". Flavor-differentiated
TEARDOWN (plasma-service sweep vs kill-PPID) is deliberately NOT wired here —
that's #15/#60 Phase-3 scope; the existing own-tree-scoped sweep is a safe
no-op under the kwin flavor.

mcss_exec_env_string emission changed %q → raw word-safe values with loud
refusal of unsafe ones: .desktop Exec parsing and env(1) argv do NOT
shell-unquote, so %q output (e.g. '' for empty) would arrive as literal
characters. Multi-word values stay caller-quoted OUTSIDE the substitution
(_NESTED_X_BEFORE).

Autostart writes/removals use MCSS_AUTOSTART_DIR + the two _DESKTOP name
constants (a missed rm literal = an autostart that relaunches the game on
every Plasma login).

Tests: +T19 (extras last-wins), +T20 (unsafe value refused) — baseline 18→20.

Part of #45 (PR 2b commit 3/4).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
…leftovers

- dex.sh: DELETED the source-time DEX_DISPLAY capture — the latent bug where
  dex sourced before the nested Xwayland existed and froze :0/stale into every
  later call. Resolution is now at call time in _dex_run: DEX_DISPLAY override
  → MCSS_DISPLAY → ambient DISPLAY → :0. _nestedSession calls
  mcss_set_display once the nested X socket is CONFIRMED up (the xdpyinfo
  wait), making mcss_set_display the single writer.
- launchSlot: --setenv DISPLAY prefers MCSS_DISPLAY over the bare :2 guess.
- PULSE_SERVER derived once (MCSS_PULSE_SERVER from the launch-time runtime
  dir snapshot) — retires three $(id -u) re-derivations (entry launchSlot +
  both instance_lifecycle builders). Builders resolve paths on call
  (idempotent) so standalone/test invocation gets canonical values.
- FIFO: the four remaining ':-/tmp/minecraft-splitscreen.fifo' inline
  defaults become bare $SPLITSCREEN_FIFO reads (named once in
  runtime_context; mkfifo stays in orchestrator main). Modules' ':-' EMPTY
  guards (fail-soft return-if-unset) deliberately kept — different semantic.
- MCSS_MODE: _set_mode is the single writer of both the exported in-shell
  mirror and state-file .mode; its missing-state init blob now uses
  ${MCSS_MODE:-docked} (jq -n) instead of a hardcoded literal. js_node
  inference retirement stays deferred to #62 per plan.

NOT done from the plan: test_orchestrator into the CI baseline — the suite is
INERT on main (sourcing the entry script exits 0 silently before any test
runs; pre-existing, verified identical on unmodified main). Gating it would
hard-fail CI on 'no summary line'. Needs its own repair issue.

Part of #45 (PR 2b commit 4/4).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
- mcss_resolve_screen: wlr-randr REMOVED from the cascade — it is a throwaway
  Wayland client and gamescope kills the launcher when one connects/disconnects
  (GAMESCOPE-WINDOWING.md; the reason --no-probe exists). The old gamescope-side
  probe sites silently enforced X11-only by being xdpyinfo one-liners; the
  resolver enforces it explicitly for every context. Latent on stock Deck
  (wlr-randr not shipped) — which is exactly why the hardware runs passed.
- mcss_resolve_screen: SPLITSCREEN_SCREEN_W/H override values must be pure
  digits (they feed $((W/2)) under leaked set -e); malformed overrides are
  ignored loudly and the cascade proceeds — restores the old degrade-don't-die
  behavior the per-site validation provided.
- MCSS_MAX_PLAYERS clamped to the structural 1-4 range (state schema, geometry
  arms, installer instances are all 4-capped) — N_SLOTS=5 previously sized only
  the static test; unclamped it now reached _find_free_slot and would
  spawn-error-loop on a nonexistent slot 5.
- MCSS_HELPER_DIR falls back to /tmp with a loud warning when uncreatable/
  unwritable (containers, root/cron, bare CI) — restores the mktemp-cannot-fail
  invariant kwin_positioner's placement path relies on; 0700 hardening is
  traded only in already-degraded environments.
- kwin session boundary additionally passes SPLITSCREEN_STATE/FIFO/DEBUG_LOG as
  explicitly QUOTED env args after the emitter substitution (env last-wins →
  quoted copies win; identical when both emitted, and they still deliver when
  the emitter refuses a whitespace value) — restores the old kwin boundary's
  any-path support. The .desktop boundaries keep their pre-existing
  unquoted-value limitation, unchanged from main.
- T19 tightened: strict ordering assertion (=0 before =plasma) with the lax
  arm removed — the old second arm also matched extras-emitted-first, which
  would break the child while the test stayed green.

Findings 2-5 of the review belong to the CurseForge BYOK work that was
accidentally swept into this branch's first commit and has been extracted to
wip/curseforge-byok (see that branch's commit message).

Part of #45 (PR 2b, review-fix commit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
@aradanmn
aradanmn force-pushed the feat/issue-45-pr2b-boundaries branch from a3d10e6 to 15ffb61 Compare July 10, 2026 19:46
Scott and others added 2 commits July 10, 2026 20:40
…hangs in Game Mode

kscreen-doctor is not a Wayland client, but with no KWin session it does
not fail — it blocks indefinitely waiting for the org.kde.KScreen D-Bus
service to appear. This hung a Game Mode launch on the Deck (2026-07-10)
at mcss_resolve_screen's first probe; killing the probe by hand let the
cascade fall through to xrandr and the launch completed normally.

All three probes (kscreen-doctor, xrandr, xdpyinfo) now run under
timeout 3, so a hang degrades into the same fall-through as a failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
…existing

SteamOS ships Plasma, so kscreen-doctor is on PATH even in Game Mode —
where nothing answers it. The timeout guard (previous commit) stops the
hang but still costs every Game Mode launch a 3s stall at the first
probe. Consult the session instead: try kscreen-doctor only under a KDE
session (XDG_CURRENT_DESKTOP contains KDE, KDE_FULL_SESSION set, or our
own nested plasma/kwin via MCSS_NESTED_SESSION) — the only contexts
where a KWin is listening AND where its per-output fidelity beats
xrandr's merged XWayland screen. The timeout stays as backstop.

Tests: T21 (gamescope session skips kscreen-doctor even when stubbed on
PATH; xrandr answers) and T22 (KDE session probes it; external output
beats eDP). CI baseline 20 -> 22.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUfc9vZUp7Fyq4UjhJa4nm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCSS_ namespace hygiene: prefix should mean runtime_context-owned

1 participant