merge: integrate latest upstream (fit view refactor, ttyd kill-by-port, mobile viewport fix)
This commit is contained in:
+19
-40
@@ -932,13 +932,7 @@ function renderGrid(sessions) {
|
|||||||
var currentMode = currentDs.viewMode || 'auto';
|
var currentMode = currentDs.viewMode || 'auto';
|
||||||
if (currentMode === 'fit' && grid) {
|
if (currentMode === 'fit' && grid) {
|
||||||
grid.classList.add('session-grid--fit');
|
grid.classList.add('session-grid--fit');
|
||||||
requestAnimationFrame(function() {
|
applyFitLayout(grid);
|
||||||
applyFitLayout(grid);
|
|
||||||
// Scroll each tile's pre to the bottom so content anchors at the bottom (like a real terminal)
|
|
||||||
grid.querySelectorAll('.tile-body pre').forEach(function(pre) {
|
|
||||||
pre.scrollTop = pre.scrollHeight;
|
|
||||||
});
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
@@ -1310,7 +1304,7 @@ function closeSession() {
|
|||||||
var _closGrid = document.getElementById('session-grid');
|
var _closGrid = document.getElementById('session-grid');
|
||||||
if (_closGrid) {
|
if (_closGrid) {
|
||||||
_closGrid.classList.add('session-grid--fit');
|
_closGrid.classList.add('session-grid--fit');
|
||||||
requestAnimationFrame(function() { applyFitLayout(_closGrid); });
|
applyFitLayout(_closGrid);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1503,29 +1497,24 @@ function saveDisplaySettings(settings) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Calculate and apply grid layout to fill the viewport exactly (Fit mode).
|
* Set grid template for fit mode based on tile count.
|
||||||
* Determines optimal cols × rows based on tile count and available space.
|
* Pure arithmetic — no DOM measurement, no getComputedStyle, no clientHeight.
|
||||||
|
* Safe to call at any time regardless of display state or layout phase.
|
||||||
|
*
|
||||||
|
* The grid already has a definite height from CSS (flex: 1 inside height: 100dvh).
|
||||||
|
* Setting grid-template-rows: repeat(rows, 1fr) lets the browser divide that height
|
||||||
|
* equally without JS needing to know the pixel dimensions. Tiles use height: auto
|
||||||
|
* (set in CSS) so they fill their grid cells without inline style overrides.
|
||||||
|
*
|
||||||
* @param {Element} grid - The session grid element
|
* @param {Element} grid - The session grid element
|
||||||
*/
|
*/
|
||||||
function applyFitLayout(grid) {
|
function applyFitLayout(grid) {
|
||||||
var count = grid.querySelectorAll('.session-tile').length;
|
var count = grid.querySelectorAll('.session-tile').length;
|
||||||
if (count === 0) return;
|
if (count === 0) {
|
||||||
|
grid.style.removeProperty('grid-template-columns');
|
||||||
// Available space — use grid's parent container
|
grid.style.removeProperty('grid-template-rows');
|
||||||
var parent = grid.parentElement;
|
return;
|
||||||
var availH = parent ? parent.clientHeight : window.innerHeight;
|
}
|
||||||
var availW = grid.clientWidth;
|
|
||||||
|
|
||||||
// Subtract padding and gap
|
|
||||||
var style = getComputedStyle(grid);
|
|
||||||
var padT = parseFloat(style.paddingTop) || 0;
|
|
||||||
var padB = parseFloat(style.paddingBottom) || 0;
|
|
||||||
var padL = parseFloat(style.paddingLeft) || 0;
|
|
||||||
var padR = parseFloat(style.paddingRight) || 0;
|
|
||||||
var gap = parseFloat(style.gap) || 8;
|
|
||||||
|
|
||||||
var innerW = availW - padL - padR;
|
|
||||||
var innerH = availH - padT - padB;
|
|
||||||
|
|
||||||
// Calculate optimal cols/rows — start with square root
|
// Calculate optimal cols/rows — start with square root
|
||||||
var cols = Math.ceil(Math.sqrt(count));
|
var cols = Math.ceil(Math.sqrt(count));
|
||||||
@@ -1541,16 +1530,8 @@ function applyFitLayout(grid) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Tile height from available space
|
|
||||||
var tileH = (innerH - gap * (rows - 1)) / rows;
|
|
||||||
|
|
||||||
grid.style.gridTemplateColumns = 'repeat(' + cols + ', 1fr)';
|
grid.style.gridTemplateColumns = 'repeat(' + cols + ', 1fr)';
|
||||||
grid.style.gridTemplateRows = 'repeat(' + rows + ', 1fr)';
|
grid.style.gridTemplateRows = 'repeat(' + rows + ', 1fr)';
|
||||||
|
|
||||||
// Override tile height so tiles fill the grid rows
|
|
||||||
grid.querySelectorAll('.session-tile').forEach(function(t) {
|
|
||||||
t.style.height = tileH + 'px';
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -1597,9 +1578,7 @@ function applyDisplaySettings(ds) {
|
|||||||
|
|
||||||
// Reset any inline styles from previous fit calculation
|
// Reset any inline styles from previous fit calculation
|
||||||
grid.style.removeProperty('grid-template-rows');
|
grid.style.removeProperty('grid-template-rows');
|
||||||
grid.querySelectorAll('.session-tile').forEach(function(t) {
|
grid.style.removeProperty('grid-template-columns');
|
||||||
t.style.removeProperty('height');
|
|
||||||
});
|
|
||||||
|
|
||||||
if (mode === 'auto') {
|
if (mode === 'auto') {
|
||||||
// Restore grid columns setting
|
// Restore grid columns setting
|
||||||
@@ -1611,7 +1590,7 @@ function applyDisplaySettings(ds) {
|
|||||||
|
|
||||||
} else if (mode === 'fit') {
|
} else if (mode === 'fit') {
|
||||||
grid.classList.add('session-grid--fit');
|
grid.classList.add('session-grid--fit');
|
||||||
requestAnimationFrame(function() { applyFitLayout(grid); });
|
applyFitLayout(grid);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2496,7 +2475,7 @@ window.addEventListener('resize', function() {
|
|||||||
var ds = loadDisplaySettings();
|
var ds = loadDisplaySettings();
|
||||||
if ((ds.viewMode || 'auto') === 'fit') {
|
if ((ds.viewMode || 'auto') === 'fit') {
|
||||||
var grid = document.getElementById('session-grid');
|
var grid = document.getElementById('session-grid');
|
||||||
if (grid) requestAnimationFrame(function() { applyFitLayout(grid); });
|
if (grid) applyFitLayout(grid);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+17
-11
@@ -83,7 +83,8 @@ body {
|
|||||||
============================================================ */
|
============================================================ */
|
||||||
|
|
||||||
.view {
|
.view {
|
||||||
height: 100vh;
|
height: 100vh; /* fallback for browsers without dvh support */
|
||||||
|
height: 100dvh; /* dynamic viewport height — adjusts for mobile browser chrome */
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -593,7 +594,8 @@ body {
|
|||||||
top: 0 !important;
|
top: 0 !important;
|
||||||
left: 0 !important;
|
left: 0 !important;
|
||||||
width: 100vw !important;
|
width: 100vw !important;
|
||||||
height: 100vh !important;
|
height: 100vh !important; /* fallback for browsers without dvh support */
|
||||||
|
height: 100dvh !important; /* dynamic viewport height — adjusts for mobile browser chrome */
|
||||||
border-radius: 0;
|
border-radius: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1523,19 +1525,23 @@ body {
|
|||||||
/* Fit view — tiles fill viewport, no scroll */
|
/* Fit view — tiles fill viewport, no scroll */
|
||||||
.session-grid--fit {
|
.session-grid--fit {
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
|
align-content: stretch; /* let 1fr rows fill container height */
|
||||||
}
|
}
|
||||||
|
|
||||||
/* In fit mode, stretch pre to fill the full tile body height and anchor content to bottom */
|
/* In fit mode, tiles must use height:auto so the CSS grid 1fr rows control sizing.
|
||||||
.session-grid--fit .tile-body pre {
|
The base .session-tile uses var(--tile-height) which is a fixed pixel value —
|
||||||
top: 0; /* stretch to fill full tile body height in fit mode */
|
overriding with height:auto lets the grid cell (from grid-template-rows: repeat(n,1fr))
|
||||||
overflow-y: scroll; /* enable scrollTop positioning (content anchored via JS scrollTop=scrollHeight) */
|
determine the tile height instead. JS sets only grid-template-columns/rows, not inline heights. */
|
||||||
scrollbar-width: none; /* Firefox: hide scrollbar */
|
.session-grid--fit .session-tile {
|
||||||
-ms-overflow-style: none; /* IE/Edge: hide scrollbar */
|
height: auto;
|
||||||
}
|
}
|
||||||
|
|
||||||
.session-grid--fit .tile-body pre::-webkit-scrollbar {
|
/* In fit mode, tile body and pre use the base CSS:
|
||||||
display: none; /* Chrome/Safari: hide scrollbar */
|
.tile-body { position: relative; overflow: hidden; }
|
||||||
}
|
.tile-body pre { position: absolute; bottom: 0; left: 0; right: 0; }
|
||||||
|
The pre's natural height from 80 lines of content exceeds the tile height, so it
|
||||||
|
overflows from bottom:0 upward, and tile-body clips excess at the top — showing
|
||||||
|
the bottom-most content (the prompt area) just like a real terminal. */
|
||||||
|
|
||||||
/* ============================================================
|
/* ============================================================
|
||||||
Responsive overlay sidebar at <960px
|
Responsive overlay sidebar at <960px
|
||||||
|
|||||||
@@ -3528,11 +3528,22 @@ test('bindStaticEventListeners wires view-mode-btn click to cycleViewMode', () =
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('applyFitLayout is called via requestAnimationFrame for correct timing', () => {
|
test('applyFitLayout is called directly (no requestAnimationFrame needed — pure arithmetic)', () => {
|
||||||
|
// Pure arithmetic applyFitLayout is safe to call synchronously at any time —
|
||||||
|
// it does not measure DOM dimensions so there is no layout-timing dependency.
|
||||||
|
// Call sites (renderGrid, closeSession, applyDisplaySettings, resize handler)
|
||||||
|
// should call it directly, not via requestAnimationFrame.
|
||||||
const source = fs.readFileSync(new URL('../app.js', import.meta.url), 'utf8');
|
const source = fs.readFileSync(new URL('../app.js', import.meta.url), 'utf8');
|
||||||
|
// Verify applyFitLayout exists as a direct call (not only inside rAF callbacks)
|
||||||
assert.ok(
|
assert.ok(
|
||||||
source.includes('requestAnimationFrame') && source.includes('applyFitLayout'),
|
source.includes('applyFitLayout'),
|
||||||
'applyFitLayout must be deferred via requestAnimationFrame'
|
'app.js must define and call applyFitLayout'
|
||||||
|
);
|
||||||
|
// The function body itself must not measure DOM — verified by the separate
|
||||||
|
// "does NOT measure DOM dimensions" test. Here just confirm the function exists.
|
||||||
|
assert.ok(
|
||||||
|
source.includes('function applyFitLayout('),
|
||||||
|
'applyFitLayout function must be declared'
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -3557,14 +3568,29 @@ test('buildTileHTML shows up to 80 lines in fit mode', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('CSS style.css has scrollbar-width none for fit mode pre to hide scrollbar', () => {
|
test('CSS style.css uses base position:absolute bottom:0 for fit mode content anchoring (no flex override)', () => {
|
||||||
const source = fs.readFileSync(new URL('../style.css', import.meta.url), 'utf8');
|
const source = fs.readFileSync(new URL('../style.css', import.meta.url), 'utf8');
|
||||||
|
// Reverted approach: base CSS position:absolute + bottom:0 anchors content to bottom.
|
||||||
|
// The flex + justify-content:flex-end approach failed because <pre> filled 100% of parent,
|
||||||
|
// making flex-end a no-op (content started at top, excess clipped at bottom).
|
||||||
|
// The fit-mode tile-body flex override must be REMOVED.
|
||||||
assert.ok(
|
assert.ok(
|
||||||
source.includes('scrollbar-width: none'),
|
!source.includes('.session-grid--fit .tile-body {'),
|
||||||
'style.css must have scrollbar-width: none for hidden scrollbar in fit mode pre'
|
'style.css must NOT have .session-grid--fit .tile-body flex override — base position:absolute handles anchoring'
|
||||||
|
);
|
||||||
|
// The pre static-positioning override must also be removed
|
||||||
|
assert.ok(
|
||||||
|
!source.includes('.session-grid--fit .tile-body pre {'),
|
||||||
|
'style.css must NOT have .session-grid--fit .tile-body pre override — base position:absolute + bottom:0 is correct'
|
||||||
|
);
|
||||||
|
// Base .tile-body pre must still use position:absolute + bottom:0
|
||||||
|
assert.ok(
|
||||||
|
source.includes('position: absolute') && source.includes('bottom: 0'),
|
||||||
|
'base .tile-body pre must retain position:absolute and bottom:0 for content anchoring'
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
// --- document.title quality fixes ---
|
// --- document.title quality fixes ---
|
||||||
|
|
||||||
test('device name input handler restores default title when value is cleared', () => {
|
test('device name input handler restores default title when value is cleared', () => {
|
||||||
@@ -3623,3 +3649,65 @@ test('DOMContentLoaded sets document.title from server settings at page load', (
|
|||||||
"DOMContentLoaded must set document.title = _serverSettings.device_name || 'muxplex' after loadServerSettings resolves"
|
"DOMContentLoaded must set document.title = _serverSettings.device_name || 'muxplex' after loadServerSettings resolves"
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
|
// --- Fit view layout: applyFitLayout sets grid template via pure arithmetic ---
|
||||||
|
|
||||||
|
test('applyFitLayout sets gridTemplateColumns and gridTemplateRows via pure arithmetic', () => {
|
||||||
|
const assignedProps = {};
|
||||||
|
|
||||||
|
const mockTile = { style: { removeProperty: () => {} } };
|
||||||
|
|
||||||
|
const mockGrid = {
|
||||||
|
style: new Proxy(assignedProps, {
|
||||||
|
set(target, prop, value) { target[prop] = value; return true; },
|
||||||
|
get(target, prop) { return target[prop]; },
|
||||||
|
}),
|
||||||
|
querySelectorAll: (sel) => {
|
||||||
|
if (sel === '.session-tile') return [mockTile, mockTile, mockTile, mockTile];
|
||||||
|
return [];
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
app.applyFitLayout(mockGrid);
|
||||||
|
|
||||||
|
assert.ok(
|
||||||
|
typeof assignedProps.gridTemplateColumns === 'string' && assignedProps.gridTemplateColumns.includes('1fr'),
|
||||||
|
'applyFitLayout must set gridTemplateColumns with 1fr tracks'
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
typeof assignedProps.gridTemplateRows === 'string' && assignedProps.gridTemplateRows.includes('1fr'),
|
||||||
|
'applyFitLayout must set gridTemplateRows with 1fr tracks'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- Pure CSS fit layout: applyFitLayout must not measure DOM ---
|
||||||
|
|
||||||
|
test('applyFitLayout does NOT measure DOM dimensions (pure arithmetic)', () => {
|
||||||
|
const source = fs.readFileSync(new URL('../app.js', import.meta.url), 'utf8');
|
||||||
|
const fnStart = source.indexOf('function applyFitLayout(');
|
||||||
|
assert.ok(fnStart !== -1, 'applyFitLayout function must exist');
|
||||||
|
// Find end of function: next \n} at the same nesting level
|
||||||
|
let depth = 0;
|
||||||
|
let pos = source.indexOf('{', fnStart);
|
||||||
|
const fnBodyStart = pos;
|
||||||
|
while (pos < source.length) {
|
||||||
|
if (source[pos] === '{') depth++;
|
||||||
|
else if (source[pos] === '}') {
|
||||||
|
depth--;
|
||||||
|
if (depth === 0) break;
|
||||||
|
}
|
||||||
|
pos++;
|
||||||
|
}
|
||||||
|
const fnBody = source.substring(fnBodyStart, pos + 1);
|
||||||
|
|
||||||
|
assert.ok(!fnBody.includes('clientHeight'),
|
||||||
|
'applyFitLayout must NOT read clientHeight — causes wrong values when container is display:none');
|
||||||
|
assert.ok(!fnBody.includes('clientWidth'),
|
||||||
|
'applyFitLayout must NOT read clientWidth — pure 1fr CSS handles width');
|
||||||
|
assert.ok(!fnBody.includes('getComputedStyle'),
|
||||||
|
'applyFitLayout must NOT call getComputedStyle — pure 1fr CSS handles gap/padding');
|
||||||
|
assert.ok(!fnBody.includes('.style.height'),
|
||||||
|
'applyFitLayout must NOT set inline tile heights — CSS grid 1fr rows handle sizing');
|
||||||
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1971,21 +1971,91 @@ def test_css_no_compact_tile_height() -> None:
|
|||||||
# ============================================================
|
# ============================================================
|
||||||
|
|
||||||
|
|
||||||
def test_fit_view_pre_has_top_zero() -> None:
|
# ============================================================
|
||||||
"""In fit mode, .tile-body pre must have top:0 to fill the full tile height.
|
# Mobile viewport + fit view content anchoring fixes
|
||||||
|
# ============================================================
|
||||||
|
|
||||||
Bug: .tile-body pre uses position:absolute with bottom:0 but no top:0.
|
|
||||||
In fit mode where tiles are taller than auto mode, the pre is anchored
|
def test_view_uses_dvh_fallback() -> None:
|
||||||
to the bottom but only takes natural content height, leaving a black gap above.
|
"""Bug fix: .view must use 100dvh (dynamic viewport height) for mobile.
|
||||||
Fix: .session-grid--fit .tile-body pre { top: 0 } stretches it to fill the tile.
|
|
||||||
|
On mobile browsers, 100vh includes the browser chrome (address bar + bottom nav),
|
||||||
|
causing the bottom row of tiles to be cut off under overflow:hidden.
|
||||||
|
Fix: height: 100dvh with 100vh fallback (progressive enhancement).
|
||||||
|
The 100vh MUST appear before 100dvh (browsers ignore unknown values, so
|
||||||
|
100dvh overrides 100vh for browsers that support it).
|
||||||
"""
|
"""
|
||||||
css = read_css()
|
css = read_css()
|
||||||
assert ".session-grid--fit .tile-body pre" in css, (
|
# .view block must contain 100dvh
|
||||||
"Missing .session-grid--fit .tile-body pre rule — needed to fix pre height in fit mode"
|
block = _extract_rule_block(css, ".view {")
|
||||||
|
assert "100dvh" in block, (
|
||||||
|
".view must use height: 100dvh for mobile — 100vh includes browser chrome, "
|
||||||
|
"causing the bottom row to be cut off on mobile devices"
|
||||||
)
|
)
|
||||||
block = _extract_rule_block(css, ".session-grid--fit .tile-body pre {")
|
# 100vh must still be present as fallback (appears before 100dvh in file)
|
||||||
assert "top: 0" in block or "top:0" in block, (
|
assert "100vh" in block, (
|
||||||
".session-grid--fit .tile-body pre must have top: 0 to fill full tile body height"
|
".view must keep height: 100vh as fallback for browsers without dvh support"
|
||||||
|
)
|
||||||
|
# Verify order: 100vh must come before 100dvh in the block (fallback first)
|
||||||
|
assert block.index("100vh") < block.index("100dvh"), (
|
||||||
|
"height: 100vh (fallback) must appear BEFORE height: 100dvh in .view rule — "
|
||||||
|
"browsers that don't support dvh will use the last valid value"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_fit_view_no_tile_body_flex_override() -> None:
|
||||||
|
"""Bug fix: .session-grid--fit .tile-body must NOT have a flex override.
|
||||||
|
|
||||||
|
The flex + justify-content:flex-end approach failed because the <pre> with
|
||||||
|
max-height:100% fills the parent entirely, making flex-end a no-op. Content
|
||||||
|
started at the top and excess was clipped at the bottom — the opposite of what
|
||||||
|
we want.
|
||||||
|
|
||||||
|
Fix: delete the .session-grid--fit .tile-body rule entirely. The base CSS
|
||||||
|
position:absolute + bottom:0 on the <pre> anchors content to the bottom.
|
||||||
|
"""
|
||||||
|
css = read_css()
|
||||||
|
assert ".session-grid--fit .tile-body {" not in css, (
|
||||||
|
".session-grid--fit .tile-body must be removed — flex-end approach does not work "
|
||||||
|
"when <pre> fills 100% of the parent. Use base position:absolute + bottom:0."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_fit_view_session_tile_has_height_auto() -> None:
|
||||||
|
"""Pure CSS fit layout: .session-grid--fit .session-tile must use height: auto.
|
||||||
|
|
||||||
|
JS was measuring clientHeight and setting tile.style.height = tileH + 'px'.
|
||||||
|
This failed when the grid was display:none (clientHeight = 0) and after
|
||||||
|
innerHTML rebuilds destroyed inline styles every 2s.
|
||||||
|
|
||||||
|
Pure CSS fix: the grid has a definite height (flex:1 inside 100dvh).
|
||||||
|
grid-template-rows: repeat(rows, 1fr) divides that height equally.
|
||||||
|
height: auto on tiles lets them fill their grid cells without JS measurement.
|
||||||
|
"""
|
||||||
|
css = read_css()
|
||||||
|
assert ".session-grid--fit .session-tile {" in css, (
|
||||||
|
".session-grid--fit .session-tile rule must exist for pure CSS fit layout"
|
||||||
|
)
|
||||||
|
block = _extract_rule_block(css, ".session-grid--fit .session-tile {")
|
||||||
|
assert "height: auto" in block or "height:auto" in block, (
|
||||||
|
".session-grid--fit .session-tile must use height: auto — "
|
||||||
|
"JS inline height setting was unreliable (lost on innerHTML rebuild every 2s)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_fit_view_no_pre_static_override() -> None:
|
||||||
|
"""Bug fix: .session-grid--fit .tile-body pre must NOT override position to static.
|
||||||
|
|
||||||
|
The position:static override removed the pre from absolute positioning, breaking
|
||||||
|
the bottom anchoring. Base CSS already has position:absolute + bottom:0 which
|
||||||
|
anchors content to the bottom of the tile.
|
||||||
|
|
||||||
|
Fix: delete the .session-grid--fit .tile-body pre rule entirely.
|
||||||
|
"""
|
||||||
|
css = read_css()
|
||||||
|
assert ".session-grid--fit .tile-body pre {" not in css, (
|
||||||
|
".session-grid--fit .tile-body pre override must be removed — revert to base "
|
||||||
|
"position:absolute + bottom:0 for correct bottom anchoring."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -217,6 +217,100 @@ async def test_kill_orphan_ttyd_handles_pid_file_with_dead_process():
|
|||||||
assert not pid_path.exists(), "PID file should be removed after orphan cleanup"
|
assert not pid_path.exists(), "PID file should be removed after orphan cleanup"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_kill_ttyd_kills_orphan_on_port_when_pid_file_desynced():
|
||||||
|
"""kill_ttyd() must also kill orphaned ttyd processes on TTYD_PORT.
|
||||||
|
|
||||||
|
If the PID file points to a dead process (desynced), but the REAL ttyd is
|
||||||
|
still running on TTYD_PORT (orphaned from a previous spawn), kill_ttyd()
|
||||||
|
must find and kill it via lsof -ti :<port>. This is the belt-and-suspenders
|
||||||
|
fallback that prevents the session-switching bug where a new ttyd cannot bind
|
||||||
|
the port because the old one is still running.
|
||||||
|
"""
|
||||||
|
pid_path = ttyd_mod.TTYD_PID_PATH
|
||||||
|
pid_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
# PID file with a dead process (desynced)
|
||||||
|
pid_path.write_text("99999")
|
||||||
|
|
||||||
|
killed_pids: list[tuple[int, int]] = []
|
||||||
|
|
||||||
|
def mock_os_kill(pid: int, sig: int) -> None:
|
||||||
|
if pid == 99999 and sig == 0:
|
||||||
|
raise ProcessLookupError # PID file process is already dead
|
||||||
|
killed_pids.append((pid, sig))
|
||||||
|
|
||||||
|
mock_lsof_result = MagicMock()
|
||||||
|
mock_lsof_result.returncode = 0
|
||||||
|
mock_lsof_result.stdout = "12345\n" # orphan PID occupying TTYD_PORT
|
||||||
|
|
||||||
|
def mock_subprocess_run(cmd, **kwargs): # noqa: ANN001
|
||||||
|
result = MagicMock()
|
||||||
|
if "lsof" in cmd and "-ti" in cmd:
|
||||||
|
return mock_lsof_result
|
||||||
|
result.returncode = 1
|
||||||
|
result.stdout = ""
|
||||||
|
return result
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("os.kill", side_effect=mock_os_kill),
|
||||||
|
patch("muxplex.ttyd._subprocess.run", side_effect=mock_subprocess_run),
|
||||||
|
):
|
||||||
|
result = await kill_ttyd()
|
||||||
|
|
||||||
|
assert result is True, "kill_ttyd must return True when orphan found on port"
|
||||||
|
orphan_killed = any(
|
||||||
|
pid == 12345 and sig == signal.SIGTERM for pid, sig in killed_pids
|
||||||
|
)
|
||||||
|
assert orphan_killed, (
|
||||||
|
"kill_ttyd must send SIGTERM to orphan process (12345) found via "
|
||||||
|
"lsof on TTYD_PORT when PID file is desynced"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_spawn_ttyd_force_kills_process_on_port_before_binding():
|
||||||
|
"""spawn_ttyd() must force-kill any process occupying TTYD_PORT before spawning.
|
||||||
|
|
||||||
|
If kill_ttyd() completed but the port is still occupied (race condition),
|
||||||
|
spawn_ttyd() must do a final SIGKILL cleanup so the new ttyd can bind.
|
||||||
|
"""
|
||||||
|
mock_proc = _make_mock_ttyd_process(pid=22222)
|
||||||
|
|
||||||
|
# First lsof call returns an occupant; second call (after kill) returns empty
|
||||||
|
call_count = 0
|
||||||
|
|
||||||
|
def mock_subprocess_run(cmd, **kwargs): # noqa: ANN001
|
||||||
|
nonlocal call_count
|
||||||
|
result = MagicMock()
|
||||||
|
if "lsof" in cmd and "-ti" in cmd:
|
||||||
|
call_count += 1
|
||||||
|
if call_count == 1:
|
||||||
|
result.returncode = 0
|
||||||
|
result.stdout = "54321\n" # port still occupied
|
||||||
|
return result
|
||||||
|
result.returncode = 1
|
||||||
|
result.stdout = ""
|
||||||
|
return result
|
||||||
|
|
||||||
|
killed_pids: list[tuple[int, int]] = []
|
||||||
|
|
||||||
|
def mock_os_kill(pid: int, sig: int) -> None:
|
||||||
|
killed_pids.append((pid, sig))
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("asyncio.create_subprocess_exec", new=AsyncMock(return_value=mock_proc)),
|
||||||
|
patch("muxplex.ttyd._subprocess.run", side_effect=mock_subprocess_run),
|
||||||
|
patch("os.kill", side_effect=mock_os_kill),
|
||||||
|
):
|
||||||
|
await spawn_ttyd("test-session")
|
||||||
|
|
||||||
|
force_killed = any(
|
||||||
|
pid == 54321 and sig == signal.SIGKILL for pid, sig in killed_pids
|
||||||
|
)
|
||||||
|
assert force_killed, (
|
||||||
|
"spawn_ttyd must SIGKILL any process occupying TTYD_PORT before spawning "
|
||||||
|
"to prevent 'address already in use' errors"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def test_kill_orphan_ttyd_handles_invalid_pid_file_content():
|
async def test_kill_orphan_ttyd_handles_invalid_pid_file_content():
|
||||||
"""kill_orphan_ttyd() gracefully handles a PID file with non-integer content."""
|
"""kill_orphan_ttyd() gracefully handles a PID file with non-integer content."""
|
||||||
pid_path = ttyd_mod.TTYD_PID_PATH
|
pid_path = ttyd_mod.TTYD_PID_PATH
|
||||||
|
|||||||
+107
-40
@@ -18,6 +18,7 @@ Public API:
|
|||||||
import asyncio
|
import asyncio
|
||||||
import os
|
import os
|
||||||
import signal
|
import signal
|
||||||
|
import subprocess as _subprocess
|
||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -37,6 +38,43 @@ TTYD_PORT: int = 7682
|
|||||||
|
|
||||||
_active_process: asyncio.subprocess.Process | None = None
|
_active_process: asyncio.subprocess.Process | None = None
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Internal helpers
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _kill_pids_on_port(port: int, sig: int) -> bool:
|
||||||
|
"""Find and signal all processes listening on *port* via lsof.
|
||||||
|
|
||||||
|
Returns True if at least one PID was found and signalled.
|
||||||
|
Silently ignores lsof unavailability and already-dead processes.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
result = _subprocess.run(
|
||||||
|
["lsof", "-ti", f":{port}"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=5,
|
||||||
|
)
|
||||||
|
if result.returncode != 0 or not result.stdout.strip():
|
||||||
|
return False
|
||||||
|
sent = False
|
||||||
|
for pid_str in result.stdout.strip().split("\n"):
|
||||||
|
pid_str = pid_str.strip()
|
||||||
|
if not pid_str:
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
orphan_pid = int(pid_str)
|
||||||
|
os.kill(orphan_pid, sig)
|
||||||
|
sent = True
|
||||||
|
except (ValueError, ProcessLookupError, PermissionError):
|
||||||
|
pass
|
||||||
|
return sent
|
||||||
|
except Exception: # noqa: BLE001
|
||||||
|
# lsof not available, timed out, or other unexpected failure
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Public API
|
# Public API
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -45,57 +83,77 @@ _active_process: asyncio.subprocess.Process | None = None
|
|||||||
async def kill_ttyd() -> bool:
|
async def kill_ttyd() -> bool:
|
||||||
"""Kill the running ttyd process and clean up the PID file.
|
"""Kill the running ttyd process and clean up the PID file.
|
||||||
|
|
||||||
Reads the PID from TTYD_PID_PATH. If no PID file exists, returns False.
|
Belt-and-suspenders strategy:
|
||||||
If the file content is not a valid integer, removes the file and returns False.
|
|
||||||
|
|
||||||
Checks whether the process is alive via ``os.kill(pid, 0)``. If the
|
Strategy 1 — PID file:
|
||||||
process is already gone (ProcessLookupError), cleans up and returns True.
|
Reads the PID from TTYD_PID_PATH. If no PID file exists, returns False.
|
||||||
Otherwise sends SIGTERM and polls every 0.1 s for up to 2 s waiting for
|
If the file content is not a valid integer, removes the file and returns
|
||||||
the process to exit. The PID file and ``_active_process`` are cleared in
|
False. Checks whether the process is alive via ``os.kill(pid, 0)``. If
|
||||||
all cases before returning.
|
already gone (ProcessLookupError), cleans up and proceeds. Otherwise
|
||||||
|
sends SIGTERM and polls every 0.1 s for up to 2 s.
|
||||||
|
|
||||||
Uses ``asyncio.sleep`` for polling to avoid blocking the event loop.
|
Strategy 2 — port-based fallback:
|
||||||
|
After the PID-file kill, finds and kills any process still listening on
|
||||||
|
TTYD_PORT via ``lsof -ti :<port>``. This catches orphaned ttyd processes
|
||||||
|
whose PID was never recorded in the file (e.g. after a coordinator crash).
|
||||||
|
A brief 0.3 s wait is added to let the OS release the port.
|
||||||
|
|
||||||
|
The PID file and ``_active_process`` are cleared in all cases before
|
||||||
|
returning.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
True — process was killed or was already dead.
|
True — a process was killed (or was already dead) via either strategy.
|
||||||
False — no PID file found, or PID file contained invalid content.
|
False — no PID file found and no process was listening on the port.
|
||||||
"""
|
"""
|
||||||
global _active_process
|
global _active_process
|
||||||
|
|
||||||
if not TTYD_PID_PATH.exists():
|
killed = False
|
||||||
return False
|
|
||||||
|
|
||||||
try:
|
# -------------------------------------------------------------------
|
||||||
pid = int(TTYD_PID_PATH.read_text().strip())
|
# Strategy 1: PID file
|
||||||
except ValueError:
|
# -------------------------------------------------------------------
|
||||||
TTYD_PID_PATH.unlink(missing_ok=True)
|
if TTYD_PID_PATH.exists():
|
||||||
return False
|
|
||||||
|
|
||||||
# Check whether the process is still alive.
|
|
||||||
try:
|
|
||||||
os.kill(pid, 0)
|
|
||||||
except ProcessLookupError:
|
|
||||||
# Already dead — clean up and report success.
|
|
||||||
TTYD_PID_PATH.unlink(missing_ok=True)
|
|
||||||
_active_process = None
|
|
||||||
return True
|
|
||||||
|
|
||||||
# Process is alive — ask it to terminate.
|
|
||||||
os.kill(pid, signal.SIGTERM)
|
|
||||||
|
|
||||||
# Poll up to 2 s for the process to exit, yielding to the event loop each iteration.
|
|
||||||
deadline = time.time() + 2.0
|
|
||||||
while time.time() < deadline:
|
|
||||||
try:
|
try:
|
||||||
os.kill(pid, 0)
|
pid = int(TTYD_PID_PATH.read_text().strip())
|
||||||
except (ProcessLookupError, PermissionError):
|
except ValueError:
|
||||||
break
|
TTYD_PID_PATH.unlink(missing_ok=True)
|
||||||
await asyncio.sleep(0.1)
|
pid = None
|
||||||
|
else:
|
||||||
|
# Check whether the process is still alive.
|
||||||
|
try:
|
||||||
|
os.kill(pid, 0)
|
||||||
|
except ProcessLookupError:
|
||||||
|
# Already dead — clean up and note success.
|
||||||
|
TTYD_PID_PATH.unlink(missing_ok=True)
|
||||||
|
killed = True
|
||||||
|
pid = None
|
||||||
|
else:
|
||||||
|
# Process is alive — ask it to terminate.
|
||||||
|
os.kill(pid, signal.SIGTERM)
|
||||||
|
|
||||||
|
# Poll up to 2 s for the process to exit.
|
||||||
|
deadline = time.time() + 2.0
|
||||||
|
while time.time() < deadline:
|
||||||
|
try:
|
||||||
|
os.kill(pid, 0)
|
||||||
|
except (ProcessLookupError, PermissionError):
|
||||||
|
break
|
||||||
|
await asyncio.sleep(0.1)
|
||||||
|
|
||||||
|
TTYD_PID_PATH.unlink(missing_ok=True)
|
||||||
|
killed = True
|
||||||
|
pid = None # noqa: F841 (intentional)
|
||||||
|
|
||||||
|
# -------------------------------------------------------------------
|
||||||
|
# Strategy 2: port-based fallback — catch orphans not in PID file
|
||||||
|
# -------------------------------------------------------------------
|
||||||
|
if _kill_pids_on_port(TTYD_PORT, signal.SIGTERM):
|
||||||
|
killed = True
|
||||||
|
# Brief pause so the OS can release the port before the next spawn.
|
||||||
|
await asyncio.sleep(0.3)
|
||||||
|
|
||||||
# Always clean up regardless of whether the process exited in time.
|
|
||||||
TTYD_PID_PATH.unlink(missing_ok=True)
|
|
||||||
_active_process = None
|
_active_process = None
|
||||||
return True
|
return killed
|
||||||
|
|
||||||
|
|
||||||
async def kill_orphan_ttyd() -> bool:
|
async def kill_orphan_ttyd() -> bool:
|
||||||
@@ -122,6 +180,10 @@ async def spawn_ttyd(session_name: str) -> asyncio.subprocess.Process:
|
|||||||
|
|
||||||
ttyd -W -m 3 -p 7682 tmux attach -t <session_name>
|
ttyd -W -m 3 -p 7682 tmux attach -t <session_name>
|
||||||
|
|
||||||
|
Before spawning, verifies that TTYD_PORT is free. If any process is still
|
||||||
|
listening on the port (e.g. a race between kill_ttyd() and spawn_ttyd()),
|
||||||
|
it sends SIGKILL to force-free the port immediately.
|
||||||
|
|
||||||
stdout and stderr are discarded (DEVNULL). The PID is written to
|
stdout and stderr are discarded (DEVNULL). The PID is written to
|
||||||
TTYD_PID_PATH. The process handle is stored in ``_active_process``.
|
TTYD_PID_PATH. The process handle is stored in ``_active_process``.
|
||||||
|
|
||||||
@@ -133,6 +195,11 @@ async def spawn_ttyd(session_name: str) -> asyncio.subprocess.Process:
|
|||||||
"""
|
"""
|
||||||
global _active_process
|
global _active_process
|
||||||
|
|
||||||
|
# Final port-free guard — catches races where kill_ttyd() returned but
|
||||||
|
# the old ttyd hasn't fully released the socket yet.
|
||||||
|
if _kill_pids_on_port(TTYD_PORT, signal.SIGKILL):
|
||||||
|
await asyncio.sleep(0.3)
|
||||||
|
|
||||||
proc = await asyncio.create_subprocess_exec(
|
proc = await asyncio.create_subprocess_exec(
|
||||||
"ttyd",
|
"ttyd",
|
||||||
"-W",
|
"-W",
|
||||||
|
|||||||
Reference in New Issue
Block a user