fix: address COE verification findings — bell-clear regression, active_view fallback, device_id consistency
- BUG 1: Replace isinstance(active_remote_id, int) guard with _lookup_remote_by_device_id() so bell-clear fires correctly when active_remote_id is a UUID string (new format). Old code silently skipped the POST for all non-integer active_remote_id values. Regression test: test_poll_cycle_fires_federation_bell_clear_for_remote_session_with_uuid - GAP 4: Add _resolveActiveView(activeView, views) helper in app.js. Falls back to 'all' when active_view references a deleted/unknown view name. Ready for Phase 2 view-switching code to call at read time. - ISSUE 6: _createDeviceSelect now uses remotes[i].device_id || String(i) for option values instead of always String(i), so session creation routes receive correct device_id. - ISSUE 9: identity.json written with indent=2 + trailing newline, consistent with settings.json and state.json formatting conventions. - ISSUE 11: Fix state.py module docstring — 'tmux-web muxplex' → 'muxplex'. - ISSUE 12: Settings panel hidden_sessions checkboxes now use s.sessionKey || s.name as the checkbox value (and checked state) so remote sessions are stored in device_id:name format, consistent with getVisibleSessions() lookups.
This commit is contained in:
+26
-4
@@ -550,6 +550,27 @@ function getVisibleSessions(sessions) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the active view name against the known views list.
|
||||||
|
*
|
||||||
|
* If active_view is "all" or "hidden" it is always valid and returned as-is.
|
||||||
|
* If active_view matches a view name in the views list it is returned as-is.
|
||||||
|
* Otherwise (e.g. the view was deleted while this device was offline) fall back
|
||||||
|
* to "all" so the user always sees sessions rather than an empty/broken state.
|
||||||
|
*
|
||||||
|
* @param {string} activeView - The stored active_view value from state.
|
||||||
|
* @param {object[]} views - The views array from settings (each has a .name field).
|
||||||
|
* @returns {string} Resolved view name — always "all", "hidden", or a known view name.
|
||||||
|
*/
|
||||||
|
function _resolveActiveView(activeView, views) {
|
||||||
|
if (activeView === 'all' || activeView === 'hidden') return activeView;
|
||||||
|
var list = views || [];
|
||||||
|
for (var i = 0; i < list.length; i++) {
|
||||||
|
if (list[i].name === activeView) return activeView;
|
||||||
|
}
|
||||||
|
return 'all';
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Render the session sidebar list. Only renders in fullscreen view.
|
* Render the session sidebar list. Only renders in fullscreen view.
|
||||||
* Shows empty state when no sessions exist.
|
* Shows empty state when no sessions exist.
|
||||||
@@ -1686,13 +1707,14 @@ function openSettings() {
|
|||||||
const hiddenList = (ss && ss.hidden_sessions) || [];
|
const hiddenList = (ss && ss.hidden_sessions) || [];
|
||||||
(_currentSessions || []).forEach(function(s) {
|
(_currentSessions || []).forEach(function(s) {
|
||||||
const name = s.name || '';
|
const name = s.name || '';
|
||||||
|
const sessionKey = s.sessionKey || name;
|
||||||
const item = document.createElement('label');
|
const item = document.createElement('label');
|
||||||
item.className = 'settings-checkbox-item';
|
item.className = 'settings-checkbox-item';
|
||||||
const cb = document.createElement('input');
|
const cb = document.createElement('input');
|
||||||
cb.type = 'checkbox';
|
cb.type = 'checkbox';
|
||||||
cb.className = 'settings-checkbox';
|
cb.className = 'settings-checkbox';
|
||||||
cb.value = name;
|
cb.value = sessionKey;
|
||||||
cb.checked = hiddenList.includes(name);
|
cb.checked = hiddenList.includes(sessionKey) || hiddenList.includes(name);
|
||||||
item.appendChild(cb);
|
item.appendChild(cb);
|
||||||
item.appendChild(document.createTextNode(' ' + name));
|
item.appendChild(document.createTextNode(' ' + name));
|
||||||
hiddenSessionsEl.appendChild(item);
|
hiddenSessionsEl.appendChild(item);
|
||||||
@@ -1936,11 +1958,11 @@ function _createDeviceSelect() {
|
|||||||
// Remote instance options
|
// Remote instance options
|
||||||
for (var i = 0; i < remotes.length; i++) {
|
for (var i = 0; i < remotes.length; i++) {
|
||||||
var opt = document.createElement('option');
|
var opt = document.createElement('option');
|
||||||
opt.value = String(i);
|
opt.value = remotes[i].device_id || String(i);
|
||||||
opt.textContent = remotes[i].name || remotes[i].url || 'Remote ' + i;
|
opt.textContent = remotes[i].name || remotes[i].url || 'Remote ' + i;
|
||||||
if (_activeFilterDevice === remotes[i].name || _activeFilterDevice === remotes[i].url) {
|
if (_activeFilterDevice === remotes[i].name || _activeFilterDevice === remotes[i].url) {
|
||||||
opt.selected = true;
|
opt.selected = true;
|
||||||
select.value = String(i);
|
select.value = remotes[i].device_id || String(i);
|
||||||
}
|
}
|
||||||
select.appendChild(opt);
|
select.appendChild(opt);
|
||||||
}
|
}
|
||||||
|
|||||||
+1
-1
@@ -45,5 +45,5 @@ def _generate_and_save() -> str:
|
|||||||
"""Generate a new UUID v4, persist it to IDENTITY_PATH, and return it."""
|
"""Generate a new UUID v4, persist it to IDENTITY_PATH, and return it."""
|
||||||
device_id = str(uuid.uuid4())
|
device_id = str(uuid.uuid4())
|
||||||
IDENTITY_PATH.parent.mkdir(parents=True, exist_ok=True)
|
IDENTITY_PATH.parent.mkdir(parents=True, exist_ok=True)
|
||||||
IDENTITY_PATH.write_text(json.dumps({"device_id": device_id}))
|
IDENTITY_PATH.write_text(json.dumps({"device_id": device_id}, indent=2) + "\n")
|
||||||
return device_id
|
return device_id
|
||||||
|
|||||||
+2
-6
@@ -220,12 +220,8 @@ async def _run_poll_cycle() -> None:
|
|||||||
if _federation_client is not None:
|
if _federation_client is not None:
|
||||||
active_remote_id = state.get("active_remote_id")
|
active_remote_id = state.get("active_remote_id")
|
||||||
if active_remote_id is not None:
|
if active_remote_id is not None:
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(str(active_remote_id))
|
||||||
remote_instances = settings.get("remote_instances", [])
|
if remote is not None:
|
||||||
if isinstance(active_remote_id, int) and 0 <= active_remote_id < len(
|
|
||||||
remote_instances
|
|
||||||
):
|
|
||||||
remote = remote_instances[active_remote_id]
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
key = remote_key
|
key = remote_key
|
||||||
|
|||||||
+1
-1
@@ -1,5 +1,5 @@
|
|||||||
"""
|
"""
|
||||||
State schema and factory functions for the tmux-web muxplex.
|
State schema and factory functions for muxplex.
|
||||||
|
|
||||||
State schema (all values are plain JSON-serialisable dicts):
|
State schema (all values are plain JSON-serialisable dicts):
|
||||||
|
|
||||||
|
|||||||
@@ -2851,6 +2851,120 @@ async def test_poll_cycle_fires_federation_bell_clear_for_remote_session(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_poll_cycle_fires_federation_bell_clear_for_remote_session_with_uuid(
|
||||||
|
monkeypatch, tmp_path
|
||||||
|
):
|
||||||
|
"""_run_poll_cycle() fires bell/clear when active_remote_id is a UUID string.
|
||||||
|
|
||||||
|
Regression test for the bug where isinstance(active_remote_id, int) was
|
||||||
|
always False for UUID strings, silently skipping the bell-clear POST.
|
||||||
|
|
||||||
|
Sets up state with active_remote_id="dead-beef-uuid", remote instance has
|
||||||
|
device_id="dead-beef-uuid", one device viewing 'build' in fullscreen with
|
||||||
|
a recent interaction. Verifies mock_client.post is called with the correct
|
||||||
|
URL and Bearer auth header.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
import time
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import muxplex.main as main_mod
|
||||||
|
import muxplex.settings as settings_mod
|
||||||
|
from muxplex.state import save_state
|
||||||
|
|
||||||
|
remote_uuid = "dead-beef-uuid-1234-abcd"
|
||||||
|
|
||||||
|
# Set up settings with one remote instance that has a device_id
|
||||||
|
settings_path = tmp_path / "settings.json"
|
||||||
|
settings_path.write_text(
|
||||||
|
json.dumps(
|
||||||
|
{
|
||||||
|
"remote_instances": [
|
||||||
|
{
|
||||||
|
"url": "http://remote-host:8088",
|
||||||
|
"key": "uuid-key",
|
||||||
|
"name": "remote-host",
|
||||||
|
"device_id": remote_uuid,
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
)
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||||
|
|
||||||
|
# Set up state with active_remote_id as a UUID string (not an integer)
|
||||||
|
state = {
|
||||||
|
"active_session": None,
|
||||||
|
"active_remote_id": remote_uuid,
|
||||||
|
"session_order": ["build"],
|
||||||
|
"sessions": {
|
||||||
|
"build": {
|
||||||
|
"bell": {"last_fired_at": None, "seen_at": None, "unseen_count": 0}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"devices": {
|
||||||
|
"dev-1": {
|
||||||
|
"label": "My Device",
|
||||||
|
"viewing_session": "build",
|
||||||
|
"view_mode": "fullscreen",
|
||||||
|
"last_interaction_at": time.time(),
|
||||||
|
"last_heartbeat_at": time.time(),
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
save_state(state)
|
||||||
|
|
||||||
|
# Mock all poll-cycle dependencies so the cycle completes without real tmux
|
||||||
|
async def mock_enumerate():
|
||||||
|
return ["build"]
|
||||||
|
|
||||||
|
async def mock_snapshot_all(names):
|
||||||
|
return {"build": "pane text"}
|
||||||
|
|
||||||
|
async def mock_process_bell_flags(names, state):
|
||||||
|
pass
|
||||||
|
|
||||||
|
monkeypatch.setattr("muxplex.main.enumerate_sessions", mock_enumerate)
|
||||||
|
monkeypatch.setattr("muxplex.main.snapshot_all", mock_snapshot_all)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"muxplex.main.update_session_cache", lambda names, snapshots: None
|
||||||
|
)
|
||||||
|
monkeypatch.setattr("muxplex.main.apply_bell_clear_rule", lambda state: None)
|
||||||
|
monkeypatch.setattr("muxplex.main.prune_devices", lambda state: None)
|
||||||
|
monkeypatch.setattr("muxplex.main.process_bell_flags", mock_process_bell_flags)
|
||||||
|
|
||||||
|
# Capture POST calls from the mocked federation client
|
||||||
|
post_calls: list[dict] = []
|
||||||
|
|
||||||
|
async def mock_post(url, **kwargs):
|
||||||
|
post_calls.append({"url": url, "kwargs": kwargs})
|
||||||
|
resp = MagicMock()
|
||||||
|
resp.status_code = 200
|
||||||
|
return resp
|
||||||
|
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client.post = mock_post
|
||||||
|
monkeypatch.setattr(main_mod, "_federation_client", mock_client)
|
||||||
|
|
||||||
|
# Run one poll cycle
|
||||||
|
await main_mod._run_poll_cycle()
|
||||||
|
|
||||||
|
# Verify mock_client.post was called — the isinstance bug causes 0 calls
|
||||||
|
assert len(post_calls) == 1, (
|
||||||
|
f"Expected exactly 1 POST call to remote bell/clear (UUID active_remote_id), "
|
||||||
|
f"got {len(post_calls)}: {post_calls}. "
|
||||||
|
f"This indicates the isinstance(active_remote_id, int) guard is still present."
|
||||||
|
)
|
||||||
|
call = post_calls[0]
|
||||||
|
assert "/api/sessions/build/bell/clear" in call["url"], (
|
||||||
|
f"Expected URL to contain '/api/sessions/build/bell/clear', got: {call['url']}"
|
||||||
|
)
|
||||||
|
headers = call["kwargs"].get("headers", {})
|
||||||
|
assert headers.get("Authorization") == "Bearer uuid-key", (
|
||||||
|
f"Expected 'Authorization: Bearer uuid-key' header, got: {headers}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# GET /api/settings/sync (task-7)
|
# GET /api/settings/sync (task-7)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -2929,3 +2929,84 @@ def test_render_grid_no_filtered_mode_check() -> None:
|
|||||||
"renderGrid must not check _gridViewMode === 'filtered' — "
|
"renderGrid must not check _gridViewMode === 'filtered' — "
|
||||||
"the 'filtered' mode has been removed; only 'flat' and 'grouped' remain"
|
"the 'filtered' mode has been removed; only 'flat' and 'grouped' remain"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# COE verification fixes
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_active_view_function_exists() -> None:
|
||||||
|
"""_resolveActiveView must exist in app.js for active_view fallback (GAP 4).
|
||||||
|
|
||||||
|
When active_view references a view name that no longer exists in the views
|
||||||
|
list, _resolveActiveView must fall back to 'all' so the user always sees
|
||||||
|
sessions rather than an empty/broken state.
|
||||||
|
"""
|
||||||
|
assert "_resolveActiveView" in _JS, (
|
||||||
|
"_resolveActiveView function must exist in app.js for active_view fallback; "
|
||||||
|
"when a view is deleted while a device is offline the stored active_view "
|
||||||
|
"must fall back to 'all'"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_active_view_falls_back_to_all() -> None:
|
||||||
|
"""_resolveActiveView must return 'all' when active_view is not in views list."""
|
||||||
|
match = re.search(
|
||||||
|
r"function _resolveActiveView\s*\(.*?\)\s*\{(.*?)^}",
|
||||||
|
_JS,
|
||||||
|
re.DOTALL | re.MULTILINE,
|
||||||
|
)
|
||||||
|
assert match, "_resolveActiveView function not found"
|
||||||
|
body = match.group(1)
|
||||||
|
assert "return 'all'" in body, (
|
||||||
|
"_resolveActiveView must return 'all' as the fallback when active_view "
|
||||||
|
"is not found in the views list"
|
||||||
|
)
|
||||||
|
assert "'all'" in body and "'hidden'" in body, (
|
||||||
|
"_resolveActiveView must pass 'all' and 'hidden' through without lookup "
|
||||||
|
"(these are reserved view names, not user-created views)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_device_select_uses_device_id_for_option_value() -> None:
|
||||||
|
"""_createDeviceSelect must use device_id (not integer index) for option values.
|
||||||
|
|
||||||
|
Session creation routes now accept a device_id string, not an integer index.
|
||||||
|
The select option value must be remotes[i].device_id when available so the
|
||||||
|
correct device_id is passed to createNewSession().
|
||||||
|
"""
|
||||||
|
match = re.search(
|
||||||
|
r"function _createDeviceSelect\s*\(\s*\)\s*\{(.*?)^}",
|
||||||
|
_JS,
|
||||||
|
re.DOTALL | re.MULTILINE,
|
||||||
|
)
|
||||||
|
assert match, "_createDeviceSelect function not found"
|
||||||
|
body = match.group(1)
|
||||||
|
assert "device_id" in body, (
|
||||||
|
"_createDeviceSelect must use remotes[i].device_id (with String(i) fallback) "
|
||||||
|
"for option values; integer index no longer matches federation API expectations"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_settings_hidden_sessions_uses_session_key() -> None:
|
||||||
|
"""Settings hidden_sessions checkbox must use sessionKey for the checkbox value.
|
||||||
|
|
||||||
|
The hidden_sessions list now stores device_id:name keys (sessionKey format).
|
||||||
|
The settings panel checkbox builder must use s.sessionKey || s.name as the
|
||||||
|
checkbox value so toggling a remote session stores the correct key format.
|
||||||
|
"""
|
||||||
|
match = re.search(
|
||||||
|
r"const hiddenSessionsEl = \$\('setting-hidden-sessions'\);.*?hiddenSessionsEl\.innerHTML = ''",
|
||||||
|
_JS,
|
||||||
|
re.DOTALL,
|
||||||
|
)
|
||||||
|
assert match, "hidden sessions settings block not found"
|
||||||
|
# Find the wider block starting from here
|
||||||
|
start = match.start()
|
||||||
|
block = _JS[start : start + 800]
|
||||||
|
assert "sessionKey" in block, (
|
||||||
|
"Settings panel hidden_sessions checkbox builder must use s.sessionKey "
|
||||||
|
"(with s.name fallback) as the checkbox value so remote sessions are "
|
||||||
|
"stored in device_id:name format in hidden_sessions"
|
||||||
|
)
|
||||||
|
|||||||
@@ -152,3 +152,24 @@ def test_reset_creates_parent_dirs(tmp_path, monkeypatch):
|
|||||||
assert _is_valid_uuid4(new_id)
|
assert _is_valid_uuid4(new_id)
|
||||||
data = json.loads(identity_path.read_text())
|
data = json.loads(identity_path.read_text())
|
||||||
assert data["device_id"] == new_id
|
assert data["device_id"] == new_id
|
||||||
|
|
||||||
|
|
||||||
|
def test_identity_json_is_human_readable(tmp_path, monkeypatch):
|
||||||
|
"""identity.json must be written with indentation for human readability.
|
||||||
|
|
||||||
|
Consistent with settings.py and state.py which both write indented JSON.
|
||||||
|
"""
|
||||||
|
identity_path = tmp_path / "identity.json"
|
||||||
|
monkeypatch.setattr("muxplex.identity.IDENTITY_PATH", identity_path)
|
||||||
|
|
||||||
|
load_device_id()
|
||||||
|
|
||||||
|
raw = identity_path.read_text()
|
||||||
|
# Human-readable JSON has newlines; compact JSON does not
|
||||||
|
assert "\n" in raw, (
|
||||||
|
"identity.json must be written with indent=2 for human readability "
|
||||||
|
"(consistent with settings.json and state.json formatting)"
|
||||||
|
)
|
||||||
|
# Must still be valid JSON
|
||||||
|
data = json.loads(raw)
|
||||||
|
assert "device_id" in data
|
||||||
|
|||||||
Reference in New Issue
Block a user