Compare commits

...
9 Commits
Author SHA1 Message Date
red person 6efc07c5fc Skip vanished backup list entries (#2006) 2026-07-11 05:26:42 +01:00
jagadish-zentiti bc771fbc1e fix(mcp): guard DbTokenStorage against non-dict oauth_tokens JSON (#5107)
_load() returned whatever json.loads() produced without checking it was a
dict; _update() did the same before assigning data[key] = value. If the
oauth_tokens column ever held a JSON array or primitive (DB corruption,
manual edit, migration drift), _load()'s callers crashed with
AttributeError on .get(), and _update() crashed with TypeError trying to
item-assign into a list/string/int.

Validate the parsed value is a dict in both methods, falling back to {}
otherwise - same recovery behavior already used elsewhere in the codebase
for this exact JSON-blob-is-not-a-dict shape (_parse_tool_args,
_is_sensitive_path's siblings).

Adds 3 regression tests for _load, get_tokens, and _update against a
non-dict oauth_tokens value.

Fixes #5082
2026-07-11 05:26:23 +01:00
jagadish-zentiti 21c7bf802e fix(email): atomically claim scheduled emails before sending (#5110)
_scheduled_poll_once selected rows WHERE status='pending' and only wrote
status='sent'/'failed' after the SMTP send and IMAP append completed -
no atomic claim in between. Two overlapping callers (the in-process 30s
poller and an externally cron/systemd-driven 'odysseus-mail
poll-scheduled', or the CLI run manually) can both SELECT the same
pending row before either UPDATEs it, and both send it. _start_poller's
own docstring already names this exact risk ('avoid two copies of
_scheduled_poll_once racing on the same SQLite') but nothing in the code
enforced it - it was advisory only.

Add an atomic per-row claim: UPDATE ... SET status='sending' WHERE
id=? AND status='pending', proceeding only when rowcount == 1. The
loser of the race sees rowcount == 0 and skips the row instead of
sending a duplicate.

Adds a regression test that drives two real threads through the real
_scheduled_poll_once against a shared SQLite file, synchronized with a
barrier and a widened send-path window, and asserts exactly one send
fires. Reverting the fix makes the test fail reliably (5/5 runs); with
the fix it passes reliably (5/5 runs).

Fixes #5109
2026-07-11 04:23:36 +01:00
L1 e0fd68160f fix(email): never fall back to sequence-number IMAP ops for move/flag (#2732)
_store_email_flag and _move_email_message (used by the archive / delete / move /
mark-read endpoints) had an else branch that, when _uid_exists returned False,
ran conn.store(uid, ...) / conn.copy(uid, ...) followed by a folder-wide
conn.expunge(). But imaplib's plain store()/copy() take a message SEQUENCE
NUMBER, not a UID, so the op landed on whichever message occupied sequence
position == the UID value, and the expunge then permanently removed it. A stale
cached UID (or a server whose UID probe misbehaves) therefore deleted an
unrelated email instead of reporting 'not found'.

There is no valid case where treating a UID as a sequence number is correct, so
drop the fallback: when the UID isn't present, return False — callers already
surface 'Email not found'. Only the UID command path remains.

Sibling of #1874 (which fixes the auto-spam poller's _imap_move in
email_helpers.py); this covers the user-facing endpoints in email_routes.py.
Part of #2124.
2026-07-11 03:27:28 +01:00
Afonso Coutinho def3483032 fix: TTS available crashes on non-string tts_provider (#2034) 2026-07-11 03:19:51 +01:00
Afonso Coutinho 667f9e4cae fix: _matchesCombo crashes on non-string keybind from server (#2049) 2026-07-11 03:15:19 +01:00
Afonso Coutinho 7b25b6dbdc fix: odysseus-memory cmd_add crashes on non-dict existing memory row (#2091) 2026-07-11 03:05:17 +01:00
Afonso Coutinho 7a1c7395c0 fix: hwfit params_b/is_prequantized crash on non-string catalog fields (#2094) 2026-07-11 03:00:28 +01:00
Ashvin a8d215a390 fix(tasks): scope manage_tasks mutations to an exact task owner (#5264)
The edit/delete/pause/run actions of do_manage_tasks gated ownership with
`if owner and task.owner and task.owner != owner`. The middle term made the
check a no-op whenever task.owner was null/empty — the state a scheduled task
sits in when it was created in no-login mode (or via the localhost middleware
bypass) before the periodic legacy-owner sweep reassigns it to the admin user.
Any authenticated user's agent could then edit, delete, pause, or run another
tenant's owner-less task; edit+run lets an attacker rewrite the task prompt and
execute it in the scheduler's agent context.

The sibling `list` action already scopes with an exact `owner == owner` filter,
so the mutators were strictly more permissive than the reader. Drop the middle
term so the guard fails closed on owner-less rows for authenticated callers,
matching `list` and the calendar/notes/gallery/session null-owner gates. Auth
disabled (owner falsy) and same-owner access are unchanged.
2026-07-11 01:45:14 +01:00
18 changed files with 658 additions and 36 deletions
+22
View File
@@ -1068,6 +1068,28 @@ def _scheduled_poll_once() -> dict:
for r in rows:
sid = r[0]
try:
# Atomically claim this row before doing any work. Two
# pollers can race here (the in-process asyncio task and an
# externally cron-driven `odysseus-mail poll-scheduled`, or
# an admin running the CLI manually alongside the in-process
# one despite the ODYSSEUS_INPROCESS_POLLERS=0 guidance) -
# both can SELECT the same 'pending' row before either has
# updated its status. The UPDATE...WHERE status='pending' is
# the atomicity boundary: only the poller whose UPDATE
# actually changes a row (rowcount == 1) proceeds to send;
# a loser sees rowcount == 0 and skips it instead of sending
# a duplicate.
claim_conn = sqlite3.connect(SCHEDULED_DB)
claim_cur = claim_conn.execute(
"UPDATE scheduled_emails SET status='sending' WHERE id=? AND status='pending'",
(sid,),
)
claim_conn.commit()
claimed = claim_cur.rowcount == 1
claim_conn.close()
if not claimed:
continue
attachments = json.loads(r[8] or "[]")
row_account_id = r[9] if len(r) > 9 else None
odysseus_kind = r[10] if len(r) > 10 else "scheduled"
+20 -17
View File
@@ -923,29 +923,32 @@ def _resolve_send_config(account_id: str | None = None, owner: str = "") -> dict
def _store_email_flag(conn, uid: str, flag: str, add: bool = True) -> bool:
# imaplib's plain store() takes a message SEQUENCE NUMBER, not a UID, so the
# old `else` fallback flagged whichever message happened to occupy sequence
# position == the UID value. When the UID isn't present, fail safe (callers
# surface "Email not found") rather than touch an unrelated message.
if not _uid_exists(conn, uid):
return False
op = "+FLAGS" if add else "-FLAGS"
if _uid_exists(conn, uid):
status, _ = conn.uid("STORE", _uid_bytes(uid), op, flag)
else:
status, _ = conn.store(_uid_bytes(uid), op, flag)
status, _ = conn.uid("STORE", _uid_bytes(uid), op, flag)
return status == "OK"
def _move_email_message(conn, uid: str, dest: str, role: str = "") -> bool:
dest = _resolve_mail_folder(conn, dest, role or _folder_role_from_name(dest))
if _uid_exists(conn, uid):
status, _ = conn.uid("MOVE", _uid_bytes(uid), _q(dest))
if status == "OK":
return True
status, _ = conn.uid("COPY", _uid_bytes(uid), _q(dest))
if status != "OK":
return False
status, _ = conn.uid("STORE", _uid_bytes(uid), "+FLAGS", "\\Deleted")
else:
status, _ = conn.copy(_uid_bytes(uid), _q(dest))
if status != "OK":
return False
status, _ = conn.store(_uid_bytes(uid), "+FLAGS", "\\Deleted")
# copy()/store() are SEQUENCE-NUMBER commands; using them with a UID (the old
# `else` branch) copied + \Deleted-flagged the wrong message and then
# expunge() permanently removed it. There is no valid case where treating a
# UID as a sequence number is correct, so fail safe when the UID is absent.
if not _uid_exists(conn, uid):
return False
status, _ = conn.uid("MOVE", _uid_bytes(uid), _q(dest))
if status == "OK":
return True
status, _ = conn.uid("COPY", _uid_bytes(uid), _q(dest))
if status != "OK":
return False
status, _ = conn.uid("STORE", _uid_bytes(uid), "+FLAGS", "\\Deleted")
if status == "OK":
conn.expunge()
return True
+23 -9
View File
@@ -133,18 +133,32 @@ def cmd_list(args):
emit([], args)
return
entries = []
for p in sorted(_BACKUP_DIR.iterdir(), key=lambda x: x.stat().st_mtime, reverse=True):
if not p.is_file():
continue
entries.append({
"path": str(p),
"name": p.name,
"bytes": p.stat().st_size,
"modified": datetime.fromtimestamp(p.stat().st_mtime).isoformat(),
})
for p in _BACKUP_DIR.iterdir():
entry = _backup_entry(p)
if entry is not None:
entries.append(entry)
entries.sort(key=lambda entry: entry["_mtime"], reverse=True)
for entry in entries:
entry.pop("_mtime", None)
emit(entries, args)
def _backup_entry(p):
try:
if not p.is_file():
return None
st = p.stat()
except OSError:
return None
return {
"path": str(p),
"name": p.name,
"bytes": st.st_size,
"modified": datetime.fromtimestamp(st.st_mtime).isoformat(),
"_mtime": st.st_mtime,
}
def cmd_verify(args):
"""Open the tarball read-only and walk its members — confirms
integrity without extracting anything."""
+1 -1
View File
@@ -90,7 +90,7 @@ def cmd_add(args):
# add_entry doesn't save by default — the call in chat does it
# after dedup checks. Persist here so a one-shot CLI add sticks.
all_entries = _manager().load_all()
if not any(e.get("id") == entry.get("id") for e in all_entries):
if not any(isinstance(e, dict) and e.get("id") == entry.get("id") for e in all_entries):
all_entries.append(entry)
_manager().save(all_entries)
emit(entry, args)
+2 -2
View File
@@ -145,7 +145,7 @@ def is_prequantized(model):
or re.search(r"(^|[-_/])fp8($|[-_/\s])", text) is not None
or (not (model.get("is_gguf") or model.get("gguf_sources")) and re.search(r"(^|[-_/])(?:int)?8bit($|[-_/\s])", text) is not None)
or any(x in text for x in ("awq", "gptq", "mlx"))
or any(q.startswith(p) for p in PREQUANTIZED_PREFIXES)
or any(isinstance(q, str) and q.startswith(p) for p in PREQUANTIZED_PREFIXES)
)
@@ -155,7 +155,7 @@ def params_b(model):
return raw / 1_000_000_000.0
pc = model.get("parameter_count", "")
if pc:
if isinstance(pc, str) and pc:
pc = pc.strip().upper()
m = re.match(r"^([\d.]+)\s*([BKMGT]?)$", pc)
if m:
+1 -1
View File
@@ -68,7 +68,7 @@ class TTSService:
if provider == "local":
kokoro = self._get_kokoro()
return kokoro is not None and kokoro.available
if provider.startswith("endpoint:"):
if isinstance(provider, str) and provider.startswith("endpoint:"):
return True # assume reachable; errors surface at synthesis time
return False
+5 -1
View File
@@ -96,7 +96,9 @@ class DbTokenStorage:
try:
srv = db.query(McpServer).filter(McpServer.id == self.server_id).first()
if srv and srv.oauth_tokens:
return json.loads(srv.oauth_tokens)
parsed = json.loads(srv.oauth_tokens)
if isinstance(parsed, dict):
return parsed
finally:
db.close()
return {}
@@ -111,6 +113,8 @@ class DbTokenStorage:
if srv is None:
return
data = json.loads(srv.oauth_tokens) if srv.oauth_tokens else {}
if not isinstance(data, dict):
data = {}
data[key] = value
srv.oauth_tokens = json.dumps(data)
db.commit()
+8 -4
View File
@@ -356,7 +356,11 @@ async def do_manage_tasks(content: str, owner: Optional[str] = None) -> Dict:
task = db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
if not task:
return {"error": f"Task {task_id} not found", "exit_code": 1}
if owner and task.owner and task.owner != owner:
# Strict ownership: the old `task.owner and task.owner != owner`
# skipped the check on an owner-less task (created in no-login mode
# or before the legacy-owner sweep), letting any authenticated user
# reach it. `list` already scopes to an exact owner match.
if owner and task.owner != owner:
return {"error": "Access denied", "exit_code": 1}
changed = []
@@ -402,7 +406,7 @@ async def do_manage_tasks(content: str, owner: Optional[str] = None) -> Dict:
task = db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
if not task:
return {"error": f"Task {task_id} not found", "exit_code": 1}
if owner and task.owner and task.owner != owner:
if owner and task.owner != owner:
return {"error": "Access denied", "exit_code": 1}
name = task.name
db.delete(task)
@@ -416,7 +420,7 @@ async def do_manage_tasks(content: str, owner: Optional[str] = None) -> Dict:
task = db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
if not task:
return {"error": f"Task {task_id} not found", "exit_code": 1}
if owner and task.owner and task.owner != owner:
if owner and task.owner != owner:
return {"error": "Access denied", "exit_code": 1}
if action == "pause":
@@ -437,7 +441,7 @@ async def do_manage_tasks(content: str, owner: Optional[str] = None) -> Dict:
task = db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
if not task:
return {"error": f"Task {task_id} not found", "exit_code": 1}
if owner and task.owner and task.owner != owner:
if owner and task.owner != owner:
return {"error": "Access denied", "exit_code": 1}
from src.event_bus import get_task_scheduler
+1 -1
View File
@@ -16,7 +16,7 @@ const _defaultKeybinds = {
};
export function _matchesCombo(e, combo, isMac = IS_MAC) {
if (!combo) return false;
if (typeof combo !== 'string' || !combo) return false;
// Drop AltGr keystrokes so typing characters on non-US layouts can't fire a
// Ctrl+Alt shortcut — e.g. the destructive delete_session. See platform.js.
if (isAltGrEvent(e, isMac)) return false;
+40
View File
@@ -25,6 +25,46 @@ def _verify_args(path: Path):
return SimpleNamespace(path=str(path), pretty=False)
def test_backup_entry_skips_files_that_disappear():
backup = _load_backup_cli()
class Vanished:
name = "gone.tar.gz"
def is_file(self):
return True
def stat(self):
raise FileNotFoundError("gone")
def __str__(self):
return "backups/gone.tar.gz"
assert backup._backup_entry(Vanished()) is None
def test_backup_list_sorts_by_captured_mtime(monkeypatch):
backup = _load_backup_cli()
first = SimpleNamespace(name="older.tar.gz")
second = SimpleNamespace(name="newer.tar.gz")
monkeypatch.setattr(backup, "_BACKUP_DIR", SimpleNamespace(
is_dir=lambda: True,
iterdir=lambda: [first, second],
))
monkeypatch.setattr(backup, "_backup_entry", lambda p: {
"name": p.name,
"modified": "2026-10-25T01:45:00" if p is first else "2026-10-25T01:15:00",
"_mtime": 100 if p is first else 200,
})
seen = []
monkeypatch.setattr(backup, "emit", lambda payload, args: seen.append(payload))
backup.cmd_list(SimpleNamespace(pretty=False))
assert [entry["name"] for entry in seen[0]] == ["newer.tar.gz", "older.tar.gz"]
assert all("_mtime" not in entry for entry in seen[0])
def test_snapshot_rejects_output_inside_data_dir(tmp_path, monkeypatch):
backup = _load_backup_cli()
repo = tmp_path / "repo"
+88
View File
@@ -0,0 +1,88 @@
"""Email move/flag must never fall back to sequence-number IMAP ops (#1874 sibling).
`imaplib`'s plain `store()` / `copy()` operate on message SEQUENCE NUMBERS, not
UIDs. `_store_email_flag` / `_move_email_message` (used by the archive / delete /
move / mark endpoints) had an `else` fallback that, when `_uid_exists` returned
False, ran `conn.store(uid, …)` / `conn.copy(uid, …)` + `conn.expunge()` — i.e.
it flagged/copied whichever message occupied sequence position == the UID value
and then permanently expunged it. A stale cached UID (or a server whose UID
probe misbehaves) therefore deleted an unrelated email.
The fix fails safe: when the UID isn't present, return False (callers surface
"Email not found") and never touch a message by sequence number.
This is distinct from #1874, which fixes the auto-spam poller's `_imap_move` in
`routes/email_helpers.py`; this covers the user-facing endpoints in
`routes/email_routes.py`.
"""
import pytest
from routes import email_routes
from routes.email_routes import _store_email_flag, _move_email_message
class _FakeConn:
"""Records IMAP calls. `uid_present` controls the FETCH-UID probe result.
The sequence-number commands (store/copy/expunge) raise if ever called —
the whole point of the fix is that they must not be reached.
"""
def __init__(self, uid_present, uid_move_ok=True):
self.uid_present = uid_present
self.uid_move_ok = uid_move_ok
self.uid_calls = []
self.seqno_calls = []
def uid(self, command, *args):
self.uid_calls.append((command.upper(), args))
cmd = command.upper()
if cmd == "FETCH":
return ("OK", [b"1 (UID 5031)"] if self.uid_present else [])
if cmd == "MOVE":
return ("OK" if self.uid_move_ok else "NO", [b""])
if cmd in ("COPY", "STORE"):
return ("OK", [b""])
return ("OK", [b""])
# Sequence-number APIs — must never be used with a UID.
def store(self, *a):
self.seqno_calls.append(("store", a)); return ("OK", [b""])
def copy(self, *a):
self.seqno_calls.append(("copy", a)); return ("OK", [b""])
def expunge(self, *a):
self.seqno_calls.append(("expunge", a)); return ("OK", [b""])
@pytest.fixture(autouse=True)
def _no_folder_resolution(monkeypatch):
# _move_email_message resolves the destination folder via the connection;
# short-circuit it so the test focuses on the UID-vs-seqno behaviour.
monkeypatch.setattr(email_routes, "_resolve_mail_folder", lambda conn, dest, role="": dest)
def test_store_flag_missing_uid_fails_safe():
conn = _FakeConn(uid_present=False)
assert _store_email_flag(conn, "5031", "\\Deleted", add=True) is False
assert conn.seqno_calls == [] # never touched a message by sequence number
def test_move_missing_uid_fails_safe():
conn = _FakeConn(uid_present=False)
assert _move_email_message(conn, "5031", "Trash", role="trash") is False
assert conn.seqno_calls == [] # no copy/store/expunge on a phantom seqno
def test_store_flag_present_uid_uses_uid_store():
conn = _FakeConn(uid_present=True)
assert _store_email_flag(conn, "5031", "\\Seen", add=True) is True
assert any(c[0] == "STORE" for c in conn.uid_calls)
assert conn.seqno_calls == []
def test_move_present_uid_uses_uid_move():
conn = _FakeConn(uid_present=True, uid_move_ok=True)
assert _move_email_message(conn, "5031", "Archive", role="archive") is True
assert any(c[0] == "MOVE" for c in conn.uid_calls)
assert conn.seqno_calls == []
@@ -0,0 +1,31 @@
"""Harden hwfit model-catalog parsing against non-string field values.
`params_b` and `is_prequantized` read free-form fields straight off the HF
catalog JSON. `parameter_count` is normally a string like "7B" and
`quantization` a string like "FP8", but a catalog row can carry a non-string
(e.g. an integer parameter_count, or a null/number quantization). The code
called `pc.strip()` / `q.startswith(...)` directly, so one such row raised
AttributeError and aborted the whole ranking pass (params_b/is_prequantized
run for every model). Non-strings are now treated as unknown.
"""
from services.hwfit.models import params_b, is_prequantized
def test_params_b_nonstring_count_does_not_raise():
assert params_b({"parameter_count": 7}) == 0.0
assert params_b({"parameter_count": ["7B"]}) == 0.0
def test_params_b_valid_count_still_parses():
assert params_b({"parameter_count": "7B"}) == 7.0
assert params_b({"parameters_raw": 7_000_000_000}) == 7.0
def test_is_prequantized_nonstring_quantization_does_not_raise():
assert is_prequantized({"quantization": 8}) is False
assert is_prequantized({"name": "plain-model", "quantization": 123}) is False
def test_is_prequantized_still_detects_real_markers():
assert is_prequantized({"name": "some-model-awq"}) is True
assert is_prequantized({"quantization": "FP8-Mixed"}) is True
+139
View File
@@ -0,0 +1,139 @@
"""manage_tasks mutations must fail closed on owner-less / cross-owner tasks.
The edit/delete/pause/run actions of ``do_manage_tasks`` previously gated with
``if owner and task.owner and task.owner != owner``. The middle term made the
check a no-op whenever the task had no owner — the state a scheduled task is in
when it was created in no-login mode (or via the localhost middleware bypass)
before the periodic legacy-owner sweep reassigns it to the admin user. So any
authenticated user's agent could edit, delete, pause, or *run* another tenant's
owner-less task. The sibling ``list`` action already scopes with an exact
``ScheduledTask.owner == owner`` filter, so the mutators were strictly more
permissive than the reader.
"""
import json
import tempfile
import pytest
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from sqlalchemy.pool import NullPool
from tests.helpers.import_state import clear_fake_database_modules
clear_fake_database_modules()
import core.database as cdb
from core.database import ScheduledTask
from src.tools.system import do_manage_tasks
_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
_ENGINE = create_engine(
f"sqlite:///{_TMPDB.name}",
connect_args={"check_same_thread": False},
poolclass=NullPool,
)
cdb.Base.metadata.create_all(_ENGINE)
_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False)
# do_manage_tasks does `from core.database import SessionLocal` at call time,
# so patching the module attribute is enough to point it at the temp DB.
cdb.SessionLocal = _TS
def _seed(task_id, owner):
db = _TS()
try:
db.add(ScheduledTask(
id=task_id, owner=owner, name=task_id, prompt="original",
task_type="llm", trigger_type="webhook", status="active",
output_target="session",
))
db.commit()
finally:
db.close()
def _get(task_id):
db = _TS()
try:
return db.query(ScheduledTask).filter(ScheduledTask.id == task_id).first()
finally:
db.close()
@pytest.mark.asyncio
async def test_edit_denied_on_ownerless_task_for_authenticated_user():
_seed("ownerless-edit", None)
out = await do_manage_tasks(
json.dumps({"action": "edit", "task_id": "ownerless-edit", "prompt": "pwned"}),
owner="alice",
)
assert out["exit_code"] == 1 and out["error"] == "Access denied"
assert _get("ownerless-edit").prompt == "original"
@pytest.mark.asyncio
async def test_delete_denied_on_ownerless_task_for_authenticated_user():
_seed("ownerless-del", None)
out = await do_manage_tasks(
json.dumps({"action": "delete", "task_id": "ownerless-del"}),
owner="alice",
)
assert out["exit_code"] == 1 and out["error"] == "Access denied"
assert _get("ownerless-del") is not None
@pytest.mark.asyncio
async def test_pause_denied_on_ownerless_task_for_authenticated_user():
_seed("ownerless-pause", None)
out = await do_manage_tasks(
json.dumps({"action": "pause", "task_id": "ownerless-pause"}),
owner="alice",
)
assert out["exit_code"] == 1 and out["error"] == "Access denied"
assert _get("ownerless-pause").status == "active"
@pytest.mark.asyncio
async def test_run_denied_on_ownerless_task_for_authenticated_user():
_seed("ownerless-run", None)
out = await do_manage_tasks(
json.dumps({"action": "run", "task_id": "ownerless-run"}),
owner="alice",
)
assert out["exit_code"] == 1 and out["error"] == "Access denied"
@pytest.mark.asyncio
async def test_edit_denied_on_other_owners_task():
_seed("bob-task", "bob")
out = await do_manage_tasks(
json.dumps({"action": "edit", "task_id": "bob-task", "prompt": "pwned"}),
owner="alice",
)
assert out["exit_code"] == 1 and out["error"] == "Access denied"
assert _get("bob-task").prompt == "original"
@pytest.mark.asyncio
async def test_edit_allowed_for_matching_owner():
_seed("alice-task", "alice")
out = await do_manage_tasks(
json.dumps({"action": "edit", "task_id": "alice-task", "prompt": "updated"}),
owner="alice",
)
assert out["exit_code"] == 0
assert _get("alice-task").prompt == "updated"
@pytest.mark.asyncio
async def test_edit_allowed_in_no_login_mode():
# owner is None when auth is disabled — single-user mode keeps full access
# to shared (owner-less) tasks, exactly as `list` returns them unfiltered.
_seed("shared-task", None)
out = await do_manage_tasks(
json.dumps({"action": "edit", "task_id": "shared-task", "prompt": "updated"}),
owner=None,
)
assert out["exit_code"] == 0
assert _get("shared-task").prompt == "updated"
+47
View File
@@ -0,0 +1,47 @@
"""Pin _matchesCombo (static/js/keyboard-shortcuts.js) against a non-string
keybind. Driven through `node --input-type=module` (same approach as
tests/test_markdown_table_row_js.py); skips when `node` is missing.
Regression: keybinds are merged from the server response of
`/api/auth/settings` (`{ ..._defaultKeybinds, ...s.keybinds }`). A corrupt
or malformed `keybinds` value (e.g. a number instead of "ctrl+k") reached
`combo.split('+')` and threw "combo.split is not a function", breaking the
whole keydown handler. The guard treats any non-string combo as "no match".
"""
import json
import shutil
import subprocess
from pathlib import Path
import pytest
_REPO = Path(__file__).resolve().parent.parent
_MOD = _REPO / "static" / "js" / "keyboard-shortcuts.js"
_HAS_NODE = shutil.which("node") is not None
_EVENT = "{key:'k',ctrlKey:false,altKey:false,shiftKey:false,metaKey:false}"
def _match(combo_js):
js = f"""
import {{ _matchesCombo }} from '{_MOD.as_posix()}';
console.log(JSON.stringify(_matchesCombo({_EVENT}, {combo_js})));
"""
proc = subprocess.run(
["node", "--input-type=module"],
input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30,
)
assert proc.returncode == 0, proc.stderr
return json.loads(proc.stdout.strip())
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
def test_non_string_combo_is_no_match():
assert _match("123") is False
assert _match("{}") is False
assert _match("null") is False
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
def test_matching_combo_still_fires():
assert _match("'k'") is True
+51
View File
@@ -1,4 +1,5 @@
import asyncio
import json
from src import mcp_oauth
@@ -79,3 +80,53 @@ def test_db_token_storage_round_trip():
t = asyncio.run(go())
assert t.access_token == "abc"
assert srv.oauth_tokens is not None # persisted as JSON
def _fake_storage(oauth_tokens):
class FakeSrv:
pass
srv = FakeSrv()
srv.oauth_tokens = oauth_tokens
class FakeQuery:
def filter(self, *a):
return self
def first(self):
return srv
class FakeSession:
def query(self, *a):
return FakeQuery()
def commit(self):
pass
def close(self):
pass
return srv, mcp_oauth.DbTokenStorage("srv-1", session_factory=lambda: FakeSession())
def test_load_falls_back_to_empty_dict_for_non_dict_json():
# A corrupted/migrated oauth_tokens column holding a JSON array, not an
# object, must not crash _load()'s callers with AttributeError.
_srv, storage = _fake_storage('["stale", "data"]')
assert storage._load() == {}
def test_get_tokens_returns_none_for_non_dict_oauth_tokens():
_srv, storage = _fake_storage("42")
async def go():
return await storage.get_tokens()
assert asyncio.run(go()) is None
def test_update_recovers_from_non_dict_oauth_tokens():
# _update() must not raise TypeError trying to item-assign into a list.
srv, storage = _fake_storage('["stale", "data"]')
storage._update("tokens", {"access_token": "new"})
assert json.loads(srv.oauth_tokens) == {"tokens": {"access_token": "new"}}
+46
View File
@@ -0,0 +1,46 @@
"""cmd_add (scripts/odysseus-memory) must tolerate a non-dict row in the
existing store. Every other command funnels load_all() through
`_memory_entries()` (which drops non-dicts), but cmd_add iterated the raw
list in its dedup check: `any(e.get("id") == ... for e in all_entries)`
crashed with AttributeError on a corrupt/hand-edited memory.json row that
is not a dict. The isinstance check short-circuits before `.get`.
"""
import importlib.machinery
import importlib.util
import sys
import types
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock
ROOT = Path(__file__).resolve().parents[1]
def _load_cli(monkeypatch):
svc = types.ModuleType("services.memory.memory")
svc.MemoryManager = MagicMock()
monkeypatch.setitem(sys.modules, "services.memory.memory", svc)
path = ROOT / "scripts" / "odysseus-memory"
loader = importlib.machinery.SourceFileLoader("odysseus_memory_cli_add", str(path))
spec = importlib.util.spec_from_loader(loader.name, loader)
module = importlib.util.module_from_spec(spec)
loader.exec_module(module)
return module
def test_cmd_add_tolerates_non_dict_existing_row(monkeypatch):
cli = _load_cli(monkeypatch)
cli._mgr = MagicMock()
cli._mgr.add_entry.return_value = {"id": "m2", "text": "new"}
cli._mgr.load_all.return_value = [
{"id": "m1", "text": "existing"},
"corrupt-row",
None,
]
emitted = []
monkeypatch.setattr(cli, "emit", lambda value, args: emitted.append(value))
cli.cmd_add(SimpleNamespace(text="new", category="fact", owner=None))
assert emitted == [{"id": "m2", "text": "new"}]
cli._mgr.save.assert_called_once()
+116
View File
@@ -0,0 +1,116 @@
"""Regression: two concurrent callers of `_scheduled_poll_once` (the
in-process 30s poller and the `odysseus-mail poll-scheduled` CLI, which the
project's own docstrings warn can race on the same SQLite when
ODYSSEUS_INPROCESS_POLLERS is left enabled alongside an external cron/systemd
driver) must not both send the same scheduled email.
The old code selected pending rows, then only updated their status to 'sent'
*after* the SMTP send completed - two overlapping calls can both SELECT the
same 'pending' row before either UPDATEs it, so both send it. The fix adds
an atomic claim step (`UPDATE ... SET status='sending' WHERE status='pending'`)
before any work happens; only the caller whose UPDATE actually changes a row
proceeds, the other sees rowcount == 0 and skips it.
This test drives two real threads through the real `_scheduled_poll_once`
against a shared SQLite file, synchronized with a barrier so both reach the
SELECT at (as close to) the same moment as possible, and asserts the send
callback fired exactly once.
"""
import sqlite3
import threading
import time
def test_concurrent_pollers_do_not_double_send(tmp_path, monkeypatch):
import routes.email_helpers as email_helpers
import routes.email_pollers as email_pollers
db_path = tmp_path / "scheduled_emails.db"
monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path)
monkeypatch.setattr(email_pollers, "SCHEDULED_DB", db_path)
email_helpers._init_scheduled_db()
conn = sqlite3.connect(db_path)
conn.execute(
"""
INSERT INTO scheduled_emails
(id, to_addr, subject, body, attachments, send_at, created_at, status, account_id, owner)
VALUES (?, ?, ?, ?, ?, ?, ?, 'pending', ?, ?)
""",
(
"sched-race-1",
"recipient@example.com",
"Subject",
"Body",
"[]",
"2000-01-01T00:00:00",
"1999-12-31T00:00:00",
"acct-alice",
"alice",
),
)
conn.commit()
conn.close()
send_calls = []
send_lock = threading.Lock()
barrier = threading.Barrier(2)
def fake_get_email_config(account_id=None, owner=""):
return {
"from_address": "alice@example.com",
"smtp_host": "smtp.example.com",
"smtp_user": "alice@example.com",
"smtp_password": "secret",
}
def fake_send_smtp_message(*args, **kwargs):
# Widen the window between the claim and the actual send so a
# buggy (unclaimed) second poller has every opportunity to also
# get past its SELECT and attempt to send.
time.sleep(0.05)
with send_lock:
send_calls.append(threading.get_ident())
class FakeImap:
def __init__(self, account_id=None, owner=""):
pass
def __enter__(self):
return self
def __exit__(self, exc_type, exc, tb):
return False
def append(self, folder, flags, date_time, message):
pass
monkeypatch.setattr(email_pollers, "_get_email_config", fake_get_email_config)
monkeypatch.setattr(email_pollers, "_send_smtp_message", fake_send_smtp_message)
monkeypatch.setattr(email_pollers, "_imap", FakeImap)
monkeypatch.setattr(email_pollers, "_detect_sent_folder", lambda imap: "Sent")
monkeypatch.setattr(email_pollers, "_cleanup_compose_uploads", lambda attachments: None)
results = []
def _run():
barrier.wait()
results.append(email_pollers._scheduled_poll_once())
threads = [threading.Thread(target=_run) for _ in range(2)]
for t in threads:
t.start()
for t in threads:
t.join(timeout=5)
assert len(send_calls) == 1, (
f"expected exactly one send for the racing pollers, got {len(send_calls)}: "
"the second poller must lose the atomic claim and skip the row"
)
conn = sqlite3.connect(db_path)
status = conn.execute(
"SELECT status FROM scheduled_emails WHERE id=?", ("sched-race-1",)
).fetchone()[0]
conn.close()
assert status == "sent"
@@ -0,0 +1,17 @@
from services.tts.tts_service import TTSService
def test_available_tolerates_non_string_provider(tmp_path):
"""A hand-edited/corrupt data/settings.json can store a non-string
tts_provider (e.g. null or a number). available reads it and calls
provider.startswith("endpoint:"), which raised AttributeError on a
non-str. It must instead fall through and report unavailable."""
service = TTSService(cache_dir=str(tmp_path))
service._load_settings = lambda: {
"tts_enabled": True,
"tts_provider": 123,
"tts_model": "tts-1",
"tts_voice": "alloy",
"tts_speed": "1",
}
assert service.available is False