mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-07-10 12:17:11 +00:00
Merge remote-tracking branch 'origin/dev' into fix/fenced-tool-inline-args-email-dispatch
This commit is contained in:
botinate
@@ -0,0 +1,202 @@
|
||||
# Test Layout Inventory
|
||||
|
||||
## Purpose
|
||||
|
||||
Inventory for the first low-risk split of the flat `tests/` directory
|
||||
(issue #3712, parent #2523). This document only records *what* should move
|
||||
first and *why*; it moves nothing. The actual move is a separate, mechanical
|
||||
PR that relocates the listed files verbatim and changes no test content.
|
||||
|
||||
The target layout and category definitions come from
|
||||
[`TESTING_STANDARD.md`](./TESTING_STANDARD.md); the collection-time markers
|
||||
come from [`_taxonomy.py`](./_taxonomy.py), which classifies by **filename
|
||||
tokens only** (paths are ignored, except the `tests/helpers/` rule). A file
|
||||
keeps its `area_*`/`sub_*` markers when moved into a subdirectory, and
|
||||
`conftest.py` discovers marker names recursively (`rglob`), so a move does not
|
||||
disturb marker registration or focused selection.
|
||||
|
||||
## Current low-risk candidate groups
|
||||
|
||||
Groups whose tests need no route/app setup and no real DB/session setup:
|
||||
|
||||
1. **CLI / script tests** (`area_cli`, 28 files) - load `scripts/` entry
|
||||
points via `tests.helpers.cli_loader.load_script`; DB access is stubbed
|
||||
with `tests.helpers.db_stubs` (`SessionLocal` is a plain stub attribute).
|
||||
No `TestClient`, no FastAPI app import, no SQLite files.
|
||||
2. **Helper self-tests** (`area_helpers`) - e.g. `test_helpers_import_state.py`,
|
||||
`test_db_stubs_helper.py`. Safe but tiny (two files), and they test the
|
||||
shared helpers from the #3685 audit (merged) that the rest of the suite
|
||||
depends on; little payoff as a first slice.
|
||||
3. **Pure unit / parsing tests** (`area_unit`) - `*_nonstring.py`,
|
||||
`*_nondict.py`, parsing tests. Large and heterogeneous; some touch
|
||||
provider/session modules, so the boundary is less crisp.
|
||||
4. **Static checks** - e.g. `test_readme_ascii_fenced.py`,
|
||||
`test_docs_no_orphan_images.py`. Safe but tiny and `uncategorized` in the
|
||||
taxonomy, so a move buys little and matches no existing marker.
|
||||
|
||||
Not candidates for the first move (per #3712 guidance): security/owner-scope
|
||||
tests, route/API tests, DB/session-heavy tests, auth/session concurrency
|
||||
tests, and the taxonomy/runner infrastructure tests that changed recently
|
||||
(#3491, #3556, #3659, #3711).
|
||||
|
||||
## Recommended first move
|
||||
|
||||
**CLI / script tests → `tests/cli/`**
|
||||
|
||||
Why this group over the alternatives:
|
||||
|
||||
- Lowest coupling: every file imports only the script under test (via
|
||||
`cli_loader`) plus `tests.helpers` stubs - no app, no routes, no real DB.
|
||||
- Crisp, machine-checkable boundary: the set is exactly the files classified
|
||||
`area_cli` by `_taxonomy.py`, so before/after selection counts can be
|
||||
compared mechanically.
|
||||
- Already the planned target dir for this category in `TESTING_STANDARD.md`
|
||||
(`tests/cli/`).
|
||||
- Absolute imports (`from tests.helpers...`) and unique basenames mean no
|
||||
import-order or module-name collisions after the move.
|
||||
- Lower risk than helper self-tests (tiny group, little payoff), unit tests
|
||||
(fuzzy boundary), or anything security/route/session-shaped.
|
||||
|
||||
## Files included in the first move
|
||||
|
||||
The 28 files classified `area_cli` (verified against `_taxonomy.py`):
|
||||
|
||||
Note: this inventory was refreshed against current `dev` after `tests/test_research_cli_status.py` was added to the `area_cli` set.
|
||||
|
||||
- `tests/test_calendar_cli_name.py`
|
||||
- `tests/test_contacts_cli_rows.py`
|
||||
- `tests/test_cookbook_cli_state.py`
|
||||
- `tests/test_docs_cli_content_length.py`
|
||||
- `tests/test_gallery_cli_album_count.py`
|
||||
- `tests/test_gallery_cli_preview.py`
|
||||
- `tests/test_logs_cli_resolve_nonstring.py`
|
||||
- `tests/test_mail_cli_read_empty_fetch.py`
|
||||
- `tests/test_mail_cli_recipients.py`
|
||||
- `tests/test_mcp_cli_env_serialize.py`
|
||||
- `tests/test_mcp_cli_json.py`
|
||||
- `tests/test_memory_cli_rows.py`
|
||||
- `tests/test_notes_cli_items.py`
|
||||
- `tests/test_personal_cli_rows.py`
|
||||
- `tests/test_preset_cli_invalid_entries.py`
|
||||
- `tests/test_preset_cli_set_corrupt_entry.py`
|
||||
- `tests/test_preset_cli_store.py`
|
||||
- `tests/test_research_cli_preview.py`
|
||||
- `tests/test_research_cli_status_filter.py`
|
||||
- `tests/test_research_cli_status.py`
|
||||
- `tests/test_research_cli_store.py`
|
||||
- `tests/test_sessions_cli.py`
|
||||
- `tests/test_signature_cli_export.py`
|
||||
- `tests/test_skills_cli_preview.py`
|
||||
- `tests/test_skills_cli_rows.py`
|
||||
- `tests/test_tasks_cli_preview.py`
|
||||
- `tests/test_theme_cli_store.py`
|
||||
- `tests/test_webhook_cli_mask.py`
|
||||
|
||||
## Files intentionally excluded
|
||||
|
||||
- `tests/test_backup_cli_security.py` - classifies as `area_security`
|
||||
(security outranks cli in the taxonomy); moving it into `tests/cli/` would
|
||||
make the directory disagree with its marker. It belongs with the security
|
||||
group in a later phase.
|
||||
- `tests/test_run_focus.py`, `tests/test_taxonomy.py` - taxonomy/runner
|
||||
infrastructure tests, recently changed (#3556, #3659); they also pin
|
||||
flat-layout paths (e.g. `tests/test_auth_config_lock_concurrency.py` in
|
||||
`test_run_focus.py`), so they stay put.
|
||||
- Script-like but `uncategorized` files - `test_pr_blocker_audit.py`,
|
||||
`test_update_database_script.py`, `test_windows_update_script.py`,
|
||||
`test_setup_admin_user.py`, `test_amd_gpu_check_args.py`, `test_hwfit_*.py`.
|
||||
They exercise `scripts/` too, but moving them would make `tests/cli/`
|
||||
diverge from the `area_cli` marker set. Reclassify or move them in a later,
|
||||
separate slice.
|
||||
- Everything else (security, routes, services, unit, js, helpers) - out of
|
||||
scope for the first move by design.
|
||||
|
||||
## How this was verified
|
||||
|
||||
Read-only checks, run from the repo root on this branch. Note the real API is
|
||||
`classify_test_path` (there is no `classify_test_file`).
|
||||
|
||||
```bash
|
||||
# Compute the area_cli set and confirm test_backup_cli_security.py is
|
||||
# area_security. Expected: 28 files, then "security".
|
||||
.venv/bin/python - <<'PY'
|
||||
from pathlib import Path
|
||||
from tests._taxonomy import classify_test_path
|
||||
|
||||
cli = [p for p in sorted(Path("tests").glob("test_*.py"))
|
||||
if classify_test_path(p).area == "cli"]
|
||||
print(len(cli))
|
||||
for p in cli:
|
||||
print(p)
|
||||
print(classify_test_path("tests/test_backup_cli_security.py").area)
|
||||
PY
|
||||
|
||||
# Coupling check across the CLI files. Expected: the only hits are
|
||||
# "SessionLocal" as stub attribute names passed to tests.helpers.db_stubs;
|
||||
# no TestClient, FastAPI, create_app, sqlite, or dependency_overrides.
|
||||
rg -n "TestClient|FastAPI|create_app|SessionLocal|sqlite|dependency_overrides" \
|
||||
tests/test_*cli*.py tests/test_sessions_cli.py
|
||||
|
||||
# Hard-coded flat paths to the exact CLI files outside tests/. Expected: no matches.
|
||||
.venv/bin/python - <<'PY2' > /tmp/area_cli_paths.txt
|
||||
from pathlib import Path
|
||||
from tests._taxonomy import classify_test_path
|
||||
|
||||
for path in sorted(Path("tests").glob("test_*.py")):
|
||||
if classify_test_path(path).area == "cli":
|
||||
print(path)
|
||||
PY2
|
||||
|
||||
rg -n -F -f /tmp/area_cli_paths.txt .github scripts docs \
|
||||
tests/README.md tests/TESTING_STANDARD.md pyproject.toml 2>/dev/null || true
|
||||
```
|
||||
|
||||
Also checked by reading the code: `tests/conftest.py` registers sub-markers
|
||||
from a recursive `rglob` scan, and `tests/_taxonomy.py` classifies by filename
|
||||
tokens only (plus the `tests/helpers/` directory rule), so the markers of the
|
||||
28 files do not change when they move into `tests/cli/`.
|
||||
|
||||
## Validation for the future move PR
|
||||
|
||||
Run with the project venv (`.venv/bin/python`); system `python3` may miss
|
||||
pinned deps. Before the move, record the baseline; after, compare:
|
||||
|
||||
```bash
|
||||
# Selection must match the 28 files before and after the move.
|
||||
.venv/bin/python tests/run_focus.py --dry-run --area cli
|
||||
.venv/bin/python -m pytest -m area_cli -q
|
||||
|
||||
# Moved files pass when targeted directly.
|
||||
.venv/bin/python -m pytest tests/cli/ -q
|
||||
|
||||
# Whole-suite collection still succeeds (catches import/path breakage).
|
||||
.venv/bin/python -m pytest --collect-only -q
|
||||
|
||||
# Taxonomy/runner infrastructure is unaffected.
|
||||
.venv/bin/python -m pytest tests/test_taxonomy.py tests/test_run_focus.py -q
|
||||
|
||||
# No stale flat-path references to the moved files. Expected: no matches
|
||||
# outside tests/cli/ itself.
|
||||
.venv/bin/python - <<'PY2' > /tmp/area_cli_paths.txt
|
||||
from pathlib import Path
|
||||
from tests._taxonomy import classify_test_path
|
||||
|
||||
for path in sorted(Path("tests").glob("test_*.py")):
|
||||
if classify_test_path(path).area == "cli":
|
||||
print(path)
|
||||
PY2
|
||||
|
||||
rg -n -F -f /tmp/area_cli_paths.txt .github scripts docs \
|
||||
tests/README.md tests/TESTING_STANDARD.md pyproject.toml 2>/dev/null || true
|
||||
```
|
||||
|
||||
Pass criteria: identical test counts for `-m area_cli` before/after, zero
|
||||
collection errors, and no changes outside the moved files.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- No file moves, renames, or deletions in this PR.
|
||||
- No changes to `conftest.py`, `_taxonomy.py`, `run_focus.py`, helpers,
|
||||
markers, CI workflows, or production code.
|
||||
- No recommendation to split the whole suite at once; later groups get their
|
||||
own inventory-then-move slices.
|
||||
@@ -51,10 +51,11 @@ Every new or refactored test should be:
|
||||
|
||||
## Test taxonomy
|
||||
|
||||
Tests are classified by the categories below. Today the suite is flat under
|
||||
`tests/`; the **Target dir** column is the phased layout from #2523 that we move
|
||||
toward *after* helpers and determinism are stable. Until a category is moved,
|
||||
new tests in that category stay in flat `tests/` but should still follow this
|
||||
Tests are classified by the categories below. Today the suite is mostly flat
|
||||
under `tests/` (the current `area_cli` set has moved to `tests/cli/`); the
|
||||
**Target dir** column is the phased layout from #2523 that we move toward
|
||||
*after* helpers and determinism are stable. Until a category is moved, new
|
||||
tests in that category stay in flat `tests/` but should still follow this
|
||||
standard.
|
||||
|
||||
| Category | What it covers | Examples today | Target dir |
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
"""`odysseus-research list --status complete` must match completed runs.
|
||||
|
||||
Completed research runs are persisted with status "done" (research_handler),
|
||||
but the user-facing CLI value is the friendlier "complete". The CLI offered
|
||||
"complete" yet filtered `status != args.status`, so `--status complete` never
|
||||
matched any record. The fix keeps "complete" as the CLI value and maps it to
|
||||
the stored "done" at filter time, so the on-disk corpus stays the source of
|
||||
truth and the documented CLI surface keeps working.
|
||||
"""
|
||||
import importlib.machinery
|
||||
import importlib.util
|
||||
import json
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
|
||||
def _load_cli():
|
||||
path = ROOT / "scripts" / "odysseus-research"
|
||||
loader = importlib.machinery.SourceFileLoader("odysseus_research_cli_status", 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_complete_is_a_valid_status_choice():
|
||||
cli = _load_cli()
|
||||
parser = cli._build_parser()
|
||||
ns = parser.parse_args(["list", "--status", "complete"])
|
||||
assert ns.status == "complete"
|
||||
|
||||
|
||||
def test_filter_returns_completed_runs(tmp_path, monkeypatch):
|
||||
cli = _load_cli(); cli._DATA_DIR = tmp_path
|
||||
(tmp_path / "r1.json").write_text(json.dumps({"query": "q1", "status": "done"}))
|
||||
(tmp_path / "r2.json").write_text(json.dumps({"query": "q2", "status": "running"}))
|
||||
emitted = []
|
||||
monkeypatch.setattr(cli, "emit", lambda value, args: emitted.append(value))
|
||||
# CLI "complete" must map to the stored "done" and match r1.
|
||||
cli.cmd_list(SimpleNamespace(status="complete", limit=50))
|
||||
ids = [r["id"] for r in emitted[0]]
|
||||
assert ids == ["r1"] # only the completed run
|
||||
|
||||
|
||||
def test_verbatim_status_still_filters(tmp_path, monkeypatch):
|
||||
cli = _load_cli(); cli._DATA_DIR = tmp_path
|
||||
(tmp_path / "r1.json").write_text(json.dumps({"query": "q1", "status": "done"}))
|
||||
(tmp_path / "r2.json").write_text(json.dumps({"query": "q2", "status": "running"}))
|
||||
emitted = []
|
||||
monkeypatch.setattr(cli, "emit", lambda value, args: emitted.append(value))
|
||||
cli.cmd_list(SimpleNamespace(status="running", limit=50))
|
||||
ids = [r["id"] for r in emitted[0]]
|
||||
assert ids == ["r2"] # verbatim choices pass through unchanged
|
||||
+1
-1
@@ -21,7 +21,7 @@ import json
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
|
||||
def _load_cli():
|
||||
@@ -0,0 +1,43 @@
|
||||
"""Tool-output display truncation uses _truncate with an indicator.
|
||||
|
||||
Previously agent_loop sliced tool output to a hard character limit ([:2000]
|
||||
or [:4000]) with no signal to the UI that data was lost. Now it delegates to
|
||||
tool_utils._truncate which caps at MAX_OUTPUT_CHARS (10 000) and appends
|
||||
a ``... (truncated, N chars total)`` suffix so the frontend can show a
|
||||
truncation indicator in the tool bubble.
|
||||
"""
|
||||
from src.tool_utils import _truncate, MAX_OUTPUT_CHARS
|
||||
|
||||
|
||||
def test_short_output_unchanged():
|
||||
"""Outputs within the limit pass through verbatim."""
|
||||
text = "hello world"
|
||||
assert _truncate(text) == text
|
||||
|
||||
|
||||
def test_long_output_truncated_with_indicator():
|
||||
"""Outputs exceeding MAX_OUTPUT_CHARS are truncated with a suffix."""
|
||||
text = "x" * (MAX_OUTPUT_CHARS + 500)
|
||||
result = _truncate(text)
|
||||
assert len(result) > MAX_OUTPUT_CHARS # includes suffix
|
||||
assert result.startswith("x" * MAX_OUTPUT_CHARS)
|
||||
assert "truncated" in result
|
||||
assert str(len(text)) in result # original length reported
|
||||
|
||||
|
||||
def test_exact_limit_unchanged():
|
||||
"""An output exactly at the limit is not truncated."""
|
||||
text = "a" * MAX_OUTPUT_CHARS
|
||||
assert _truncate(text) == text
|
||||
|
||||
|
||||
def test_default_limit_matches_constant():
|
||||
"""_truncate default limit equals MAX_OUTPUT_CHARS (10 000)."""
|
||||
assert MAX_OUTPUT_CHARS == 10_000
|
||||
text = "y" * 10_001
|
||||
result = _truncate(text)
|
||||
assert "truncated" in result
|
||||
|
||||
|
||||
def test_empty_string():
|
||||
assert _truncate("") == ""
|
||||
@@ -33,3 +33,19 @@ def test_api_key_manager_load_resilience(tmp_path):
|
||||
assert loaded["good_provider"] == "good_value"
|
||||
assert "bad_provider" not in loaded
|
||||
assert "garbage_provider" not in loaded
|
||||
|
||||
|
||||
def test_load_ignores_non_string_raw_values(tmp_path):
|
||||
mgr = APIKeyManager(str(tmp_path))
|
||||
|
||||
mgr.save("openai", "sk-openai")
|
||||
with open(mgr.api_keys_file, "r", encoding="utf-8") as f:
|
||||
keys = json.load(f)
|
||||
|
||||
keys["missing_provider"] = None
|
||||
keys["numeric_provider"] = 42
|
||||
keys["object_provider"] = {"encrypted": keys["openai"]}
|
||||
with open(mgr.api_keys_file, "w", encoding="utf-8") as f:
|
||||
json.dump(keys, f)
|
||||
|
||||
assert mgr.load() == {"openai": "sk-openai"}
|
||||
|
||||
@@ -287,8 +287,9 @@ def test_delete_token_deletes_and_invalidates_cache(monkeypatch, token_routes_mo
|
||||
monkeypatch.setattr(mod, "get_current_user", lambda req: req.state.current_user)
|
||||
monkeypatch.setattr(mod, "ApiToken", MagicMock())
|
||||
|
||||
fake_token = SimpleNamespace(id="abcd1234", owner="alice", name="test")
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.delete.return_value = 1
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = fake_token
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
invalidator = MagicMock()
|
||||
@@ -297,6 +298,7 @@ def test_delete_token_deletes_and_invalidates_cache(monkeypatch, token_routes_mo
|
||||
resp = delete_token(request=req, token_id="abcd1234")
|
||||
|
||||
assert resp == {"status": "deleted"}
|
||||
fake_session.delete.assert_called_once_with(fake_token)
|
||||
invalidator.assert_called_once()
|
||||
|
||||
|
||||
@@ -312,7 +314,7 @@ def test_delete_missing_token_returns_404_without_invalidating_cache(monkeypatch
|
||||
monkeypatch.setattr(mod, "ApiToken", MagicMock())
|
||||
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.delete.return_value = 0
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = None
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
invalidator = MagicMock()
|
||||
@@ -404,3 +406,99 @@ def test_update_missing_token_returns_404(monkeypatch, token_routes_mod):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
asyncio.run(update_token(request=req, token_id="missing99"))
|
||||
assert exc.value.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 7. Owner check — update/delete reject a different admin's token with 403
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _bob_patch_request(invalidator, body):
|
||||
"""An admin request from bob whose async .json() yields `body`."""
|
||||
req = _req("bob", is_admin=True, invalidator=invalidator)
|
||||
|
||||
async def _json():
|
||||
return body
|
||||
|
||||
req.json = _json
|
||||
return req
|
||||
|
||||
|
||||
def test_update_token_rejects_non_owner(monkeypatch, token_routes_mod):
|
||||
monkeypatch.setenv("AUTH_ENABLED", "true")
|
||||
mod = token_routes_mod
|
||||
monkeypatch.setattr(mod, "get_current_user", lambda req: req.state.current_user)
|
||||
|
||||
token = SimpleNamespace(
|
||||
id="tok123", name="alice-token", owner="alice",
|
||||
token_prefix="ody_alic", scopes="chat", is_active=True,
|
||||
)
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = token
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
req = _bob_patch_request(MagicMock(), {"name": "hijacked"})
|
||||
update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
asyncio.run(update_token(request=req, token_id="tok123"))
|
||||
assert exc.value.status_code == 403
|
||||
assert token.name == "alice-token"
|
||||
|
||||
|
||||
def test_delete_token_rejects_non_owner(monkeypatch, token_routes_mod):
|
||||
monkeypatch.setenv("AUTH_ENABLED", "true")
|
||||
mod = token_routes_mod
|
||||
monkeypatch.setattr(mod, "get_current_user", lambda req: req.state.current_user)
|
||||
monkeypatch.setattr(mod, "ApiToken", MagicMock())
|
||||
|
||||
fake_token = SimpleNamespace(id="tok123", owner="alice", name="alice-token")
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = fake_token
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
invalidator = MagicMock()
|
||||
req = _req("bob", is_admin=True, invalidator=invalidator)
|
||||
delete_token = _get_handler(mod, "DELETE", "/tokens/{token_id}")
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
delete_token(request=req, token_id="tok123")
|
||||
assert exc.value.status_code == 403
|
||||
fake_session.delete.assert_not_called()
|
||||
invalidator.assert_not_called()
|
||||
|
||||
|
||||
def test_update_token_owner_check_skipped_when_auth_disabled(monkeypatch, token_routes_mod):
|
||||
monkeypatch.setenv("AUTH_ENABLED", "false")
|
||||
mod = token_routes_mod
|
||||
monkeypatch.setattr(mod, "get_current_user", lambda req: None)
|
||||
|
||||
token = SimpleNamespace(
|
||||
id="tok123", name="original", owner="alice",
|
||||
token_prefix="ody_alic", scopes="chat", is_active=True,
|
||||
)
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = token
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
req = _bob_patch_request(MagicMock(), {"name": "renamed-in-single-user"})
|
||||
update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}")
|
||||
resp = asyncio.run(update_token(request=req, token_id="tok123"))
|
||||
assert resp["name"] == "renamed-in-single-user"
|
||||
|
||||
|
||||
def test_delete_token_owner_check_skipped_when_auth_disabled(monkeypatch, token_routes_mod):
|
||||
monkeypatch.setenv("AUTH_ENABLED", "false")
|
||||
mod = token_routes_mod
|
||||
monkeypatch.setattr(mod, "get_current_user", lambda req: None)
|
||||
monkeypatch.setattr(mod, "ApiToken", MagicMock())
|
||||
|
||||
fake_token = SimpleNamespace(id="tok123", owner="alice", name="alice-token")
|
||||
fake_session = MagicMock()
|
||||
fake_session.query.return_value.filter.return_value.first.return_value = fake_token
|
||||
monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session))
|
||||
|
||||
invalidator = MagicMock()
|
||||
req = _req("", is_admin=True, invalidator=invalidator)
|
||||
delete_token = _get_handler(mod, "DELETE", "/tokens/{token_id}")
|
||||
resp = delete_token(request=req, token_id="tok123")
|
||||
assert resp == {"status": "deleted"}
|
||||
fake_session.delete.assert_called_once_with(fake_token)
|
||||
|
||||
@@ -106,6 +106,9 @@ async def test_learn_sender_signatures_resolves_llm_for_task_owner(monkeypatch):
|
||||
from src.builtin_actions import action_learn_sender_signatures
|
||||
|
||||
class FakeImap:
|
||||
def __init__(self, owner=""):
|
||||
self.owner = owner
|
||||
|
||||
def select(self, *_args, **_kwargs):
|
||||
return "OK", []
|
||||
|
||||
@@ -119,13 +122,20 @@ async def test_learn_sender_signatures_resolves_llm_for_task_owner(monkeypatch):
|
||||
return None
|
||||
|
||||
calls, _fallback_calls = _resolver_spy(monkeypatch, utility_result=("", "", {}), default_result=("", "", {}))
|
||||
monkeypatch.setattr(email_helpers, "_imap_connect", lambda _account_id=None: FakeImap())
|
||||
imap_owners = []
|
||||
|
||||
def fake_imap_connect(_account_id=None, owner=""):
|
||||
imap_owners.append(owner)
|
||||
return FakeImap(owner)
|
||||
|
||||
monkeypatch.setattr(email_helpers, "_imap_connect", fake_imap_connect)
|
||||
|
||||
message, ok = await action_learn_sender_signatures("alice")
|
||||
|
||||
assert ok is False
|
||||
assert message == "No LLM endpoint available"
|
||||
assert calls == [("utility", "alice"), ("default", "alice")]
|
||||
assert imap_owners == ["alice"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""llama.cpp slot-affinity fields must never reach cloud providers (#3793).
|
||||
|
||||
_apply_local_cache_affinity adds session_id + cache_prompt to outgoing
|
||||
payloads for KV-cache slot affinity (#2927). The old gate treated any unknown
|
||||
OpenAI-compatible host as self-hosted, so strict cloud APIs added as custom
|
||||
endpoints (Mistral at api.mistral.ai) received the extra fields and rejected
|
||||
every request with 422 extra_forbidden. Self-hosted now also requires the
|
||||
endpoint to resolve as local: loopback/private/tailscale host, or endpoint
|
||||
kind explicitly configured as "local".
|
||||
"""
|
||||
import pytest
|
||||
|
||||
import src.llm_core as llm_core
|
||||
import src.model_context as model_context
|
||||
|
||||
|
||||
def _affinity_fields(url, monkeypatch, kind=None):
|
||||
monkeypatch.setattr(model_context, "_configured_endpoint_kind", lambda _u: kind)
|
||||
payload = {}
|
||||
llm_core._apply_local_cache_affinity(payload, url, "sess-123")
|
||||
return payload
|
||||
|
||||
|
||||
def test_mistral_cloud_api_gets_no_affinity_fields(monkeypatch):
|
||||
# The #3793 repro: Mistral rejects unknown body fields with 422.
|
||||
payload = _affinity_fields("https://api.mistral.ai/v1", monkeypatch)
|
||||
assert payload == {}
|
||||
|
||||
|
||||
def test_openai_api_gets_no_affinity_fields(monkeypatch):
|
||||
payload = _affinity_fields("https://api.openai.com/v1", monkeypatch)
|
||||
assert payload == {}
|
||||
|
||||
|
||||
def test_unknown_public_host_gets_no_affinity_fields(monkeypatch):
|
||||
# Any strict cloud provider added as a custom endpoint, not just Mistral.
|
||||
payload = _affinity_fields("https://llm.example-cloud.com/v1", monkeypatch)
|
||||
assert payload == {}
|
||||
|
||||
|
||||
def test_localhost_server_gets_affinity_fields(monkeypatch):
|
||||
payload = _affinity_fields("http://localhost:8080/v1", monkeypatch)
|
||||
assert payload == {"session_id": "sess-123", "cache_prompt": True}
|
||||
|
||||
|
||||
def test_private_lan_server_gets_affinity_fields(monkeypatch):
|
||||
payload = _affinity_fields("http://192.168.1.50:8000/v1", monkeypatch)
|
||||
assert payload == {"session_id": "sess-123", "cache_prompt": True}
|
||||
|
||||
|
||||
def test_public_host_with_local_kind_override_gets_affinity_fields(monkeypatch):
|
||||
# Escape hatch: a self-hosted llama.cpp exposed via a tunnel keeps the
|
||||
# slot-affinity hint when its endpoint kind is configured as "local".
|
||||
payload = _affinity_fields("https://my-llama.example.com/v1", monkeypatch, kind="local")
|
||||
assert payload == {"session_id": "sess-123", "cache_prompt": True}
|
||||
|
||||
|
||||
def test_no_session_id_is_a_noop(monkeypatch):
|
||||
monkeypatch.setattr(model_context, "_configured_endpoint_kind", lambda _u: None)
|
||||
payload = {}
|
||||
llm_core._apply_local_cache_affinity(payload, "http://localhost:8080/v1", None)
|
||||
assert payload == {}
|
||||
|
||||
|
||||
# Cloud-host sweep absorbed from #3839 (credit: Shabablinchikow) - every cloud
|
||||
# API that falls through provider detection to the OpenAI-compatible default
|
||||
# must stay clean, not just the Mistral host from the original report.
|
||||
@pytest.mark.parametrize("url", [
|
||||
"https://api.mistral.ai/v1/chat/completions",
|
||||
"https://api.deepseek.com/v1/chat/completions",
|
||||
"https://api.x.ai/v1/chat/completions",
|
||||
"https://api.together.xyz/v1/chat/completions",
|
||||
"https://api.fireworks.ai/inference/v1/chat/completions",
|
||||
"https://generativelanguage.googleapis.com/v1beta/openai/chat/completions",
|
||||
])
|
||||
def test_cloud_openai_compatible_hosts_get_no_affinity_fields(monkeypatch, url):
|
||||
assert _affinity_fields(url, monkeypatch) == {}
|
||||
|
||||
|
||||
# Tailscale CGNAT boundaries (review finding on #3945): only 100.64.0.0/10 is
|
||||
# Tailscale; the rest of 100.0.0.0/8 contains public ranges, and a strict
|
||||
# provider addressed by one must not receive the llama.cpp extras.
|
||||
def test_host_just_below_cgnat_gets_no_affinity_fields(monkeypatch):
|
||||
assert _affinity_fields("http://100.63.255.255/v1", monkeypatch) == {}
|
||||
|
||||
|
||||
def test_host_just_above_cgnat_gets_no_affinity_fields(monkeypatch):
|
||||
assert _affinity_fields("http://100.128.0.1/v1", monkeypatch) == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("host", ["100.64.0.1", "100.100.50.2", "100.127.255.254"])
|
||||
def test_hosts_inside_cgnat_get_affinity_fields(monkeypatch, host):
|
||||
payload = _affinity_fields(f"http://{host}:8080/v1", monkeypatch)
|
||||
assert payload == {"session_id": "sess-123", "cache_prompt": True}
|
||||
@@ -11,7 +11,7 @@ import src.model_context as mc
|
||||
|
||||
def _setup(monkeypatch, windows):
|
||||
"""windows: {endpoint_url: context_length}. Force the remote path."""
|
||||
monkeypatch.setattr(mc, "_is_local_endpoint", lambda url: False)
|
||||
monkeypatch.setattr(mc, "is_local_endpoint", lambda url: False)
|
||||
monkeypatch.setattr(mc, "_configured_endpoint_kind", lambda url: "api")
|
||||
monkeypatch.setattr(mc, "_query_context_length", lambda url, model: windows[url])
|
||||
mc._context_cache.clear()
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
DIAGNOSIS_JS = ROOT / "static" / "js" / "cookbook-diagnosis.js"
|
||||
|
||||
|
||||
def test_repair_kernels_pip_spec_is_shell_quoted():
|
||||
source = DIAGNOSIS_JS.read_text(encoding="utf-8")
|
||||
|
||||
assert '"kernels<0.15"' in source
|
||||
assert " --break-system-packages kernels<0.15" not in source
|
||||
@@ -0,0 +1,56 @@
|
||||
"""Behavioral guard for the cookbook error output-tail expansion.
|
||||
|
||||
When a task reaches status "error" the status endpoint previously returned
|
||||
only the last 12 lines of the subprocess log. The "Copy last 50 lines"
|
||||
context-menu action was therefore copying the same 12 lines — useless for
|
||||
diagnosing failures that emit long stack traces or build output.
|
||||
|
||||
`error_aware_output_tail` now returns the last 50 lines on error and keeps
|
||||
the cheaper 12-line tail for running/other tasks.
|
||||
"""
|
||||
from routes.cookbook_output import error_aware_output_tail
|
||||
|
||||
|
||||
def _snapshot(n):
|
||||
return "\n".join(f"line {i}" for i in range(n))
|
||||
|
||||
|
||||
def test_error_status_returns_last_50_lines():
|
||||
snap = _snapshot(200)
|
||||
tail = error_aware_output_tail(snap, "error")
|
||||
lines = tail.splitlines()
|
||||
assert len(lines) == 50, f"error tail should be 50 lines, got {len(lines)}"
|
||||
assert lines[0] == "line 150"
|
||||
assert lines[-1] == "line 199"
|
||||
|
||||
|
||||
def test_non_error_status_returns_last_12_lines():
|
||||
snap = _snapshot(200)
|
||||
for status in ("running", "ready", "completed", "stopped", "unknown"):
|
||||
tail = error_aware_output_tail(snap, status)
|
||||
lines = tail.splitlines()
|
||||
assert len(lines) == 12, f"{status} tail should be 12 lines, got {len(lines)}"
|
||||
assert lines[-1] == "line 199"
|
||||
|
||||
|
||||
def test_short_snapshot_returns_all_lines():
|
||||
# Fewer lines than the cap — return everything, no padding.
|
||||
snap = _snapshot(5)
|
||||
assert error_aware_output_tail(snap, "error").splitlines() == [
|
||||
"line 0", "line 1", "line 2", "line 3", "line 4",
|
||||
]
|
||||
assert len(error_aware_output_tail(snap, "running").splitlines()) == 5
|
||||
|
||||
|
||||
def test_empty_snapshot_returns_empty_string():
|
||||
assert error_aware_output_tail("", "error") == ""
|
||||
assert error_aware_output_tail("", "running") == ""
|
||||
|
||||
|
||||
def test_error_tail_is_wider_than_non_error():
|
||||
snap = _snapshot(100)
|
||||
err = error_aware_output_tail(snap, "error").splitlines()
|
||||
run = error_aware_output_tail(snap, "running").splitlines()
|
||||
assert len(err) > len(run)
|
||||
# The non-error tail is a strict suffix of the error tail.
|
||||
assert err[-len(run):] == run
|
||||
@@ -0,0 +1,71 @@
|
||||
"""Regression tests for _group_uid_fetch_records (Gmail FLAGS placement).
|
||||
|
||||
imaplib hands back UID FETCH responses as an interleaved list of
|
||||
``(meta, literal)`` tuples and bare ``bytes`` elements. Dovecot sends FLAGS
|
||||
before the RFC822.HEADER literal, so they sit inside the tuple meta; Gmail
|
||||
sends FLAGS *after* the literal, as a bare ``b' FLAGS (\\Seen))'`` element.
|
||||
The old grouping loop only looked at tuples, so on Gmail every message lost
|
||||
its FLAGS and rendered as unread/unflagged in the email library.
|
||||
"""
|
||||
|
||||
import re
|
||||
|
||||
from routes.email_routes import _group_uid_fetch_records, _uid_from_fetch_meta
|
||||
|
||||
|
||||
def _flags(meta_b: bytes) -> str:
|
||||
m = re.search(rb"FLAGS \(([^)]*)\)", meta_b)
|
||||
return m.group(1).decode() if m else ""
|
||||
|
||||
|
||||
# Captured shape of a real Gmail response to
|
||||
# UID FETCH a,b (UID FLAGS RFC822.HEADER RFC822.SIZE):
|
||||
GMAIL_RESPONSE = [
|
||||
(b"10779 (UID 18723 RFC822.SIZE 54308 RFC822.HEADER {24}", b"Subject: read one\r\n\r\n"),
|
||||
rb" FLAGS (\Seen))",
|
||||
(b"10780 (UID 18724 RFC822.SIZE 124310 RFC822.HEADER {26}", b"Subject: unread one\r\n\r\n"),
|
||||
rb" FLAGS ())",
|
||||
]
|
||||
|
||||
# Dovecot puts FLAGS before the literal and terminates with a bare b')'.
|
||||
DOVECOT_RESPONSE = [
|
||||
(rb"1 (UID 5 FLAGS (\Seen) RFC822.SIZE 100 RFC822.HEADER {18}", b"Subject: hi\r\n\r\n"),
|
||||
b")",
|
||||
(b"2 (UID 6 FLAGS () RFC822.SIZE 90 RFC822.HEADER {19}", b"Subject: new\r\n\r\n"),
|
||||
b")",
|
||||
]
|
||||
|
||||
|
||||
def test_gmail_post_literal_flags_attach_to_their_own_message():
|
||||
grouped = _group_uid_fetch_records(GMAIL_RESPONSE)
|
||||
|
||||
assert len(grouped) == 2
|
||||
assert _uid_from_fetch_meta(grouped[0][0]) == "18723"
|
||||
assert _flags(grouped[0][0]) == r"\Seen"
|
||||
assert grouped[0][1] == b"Subject: read one\r\n\r\n"
|
||||
|
||||
assert _uid_from_fetch_meta(grouped[1][0]) == "18724"
|
||||
assert _flags(grouped[1][0]) == ""
|
||||
assert grouped[1][1] == b"Subject: unread one\r\n\r\n"
|
||||
|
||||
|
||||
def test_dovecot_pre_literal_flags_unchanged():
|
||||
grouped = _group_uid_fetch_records(DOVECOT_RESPONSE)
|
||||
|
||||
assert len(grouped) == 2
|
||||
assert _flags(grouped[0][0]) == r"\Seen"
|
||||
assert _flags(grouped[1][0]) == ""
|
||||
assert grouped[1][1] == b"Subject: new\r\n\r\n"
|
||||
|
||||
|
||||
def test_size_and_uid_survive_grouping():
|
||||
grouped = _group_uid_fetch_records(GMAIL_RESPONSE)
|
||||
sizes = [re.search(rb"RFC822\.SIZE (\d+)", m).group(1) for m, _ in grouped]
|
||||
assert sizes == [b"54308", b"124310"]
|
||||
|
||||
|
||||
def test_empty_and_none_inputs():
|
||||
assert _group_uid_fetch_records(None) == []
|
||||
assert _group_uid_fetch_records([]) == []
|
||||
# A stray bare element before any tuple opens no record and must not crash.
|
||||
assert _group_uid_fetch_records([rb" FLAGS (\Seen))"]) == []
|
||||
@@ -1,5 +1,7 @@
|
||||
import sqlite3
|
||||
from contextlib import contextmanager
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -117,6 +119,71 @@ def test_email_ai_cache_tables_are_owner_scoped_and_migrate_legacy_rows(tmp_path
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_sender_signature_cache_is_owner_scoped_and_migrates_legacy_rows(tmp_path, monkeypatch):
|
||||
import routes.email_helpers as email_helpers
|
||||
|
||||
db_path = tmp_path / "scheduled_emails.db"
|
||||
monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path)
|
||||
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.execute(
|
||||
"""
|
||||
CREATE TABLE sender_signatures (
|
||||
from_address TEXT PRIMARY KEY,
|
||||
signature_text TEXT,
|
||||
sample_count INTEGER,
|
||||
last_built_at TEXT NOT NULL,
|
||||
model_used TEXT,
|
||||
source TEXT
|
||||
)
|
||||
"""
|
||||
)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES ('writer@example.com', 'legacy sig', 3, '2026-01-01', 'm', 'llm')
|
||||
"""
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
email_helpers._init_scheduled_db()
|
||||
|
||||
conn = sqlite3.connect(db_path)
|
||||
try:
|
||||
info = conn.execute("PRAGMA table_info(sender_signatures)").fetchall()
|
||||
pk_cols = [r[1] for r in sorted((r for r in info if r[5]), key=lambda r: r[5])]
|
||||
assert pk_cols == ["from_address", "owner"]
|
||||
assert conn.execute(
|
||||
"SELECT owner, signature_text FROM sender_signatures WHERE from_address=?",
|
||||
("writer@example.com",),
|
||||
).fetchone() == ("", "legacy sig")
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "alice", "alice sig", 3, "2026-01-02", "m", "llm"),
|
||||
)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "bob", "bob sig", 3, "2026-01-03", "m", "llm"),
|
||||
)
|
||||
rows = conn.execute(
|
||||
"SELECT owner, signature_text FROM sender_signatures WHERE from_address=? ORDER BY owner",
|
||||
("writer@example.com",),
|
||||
).fetchall()
|
||||
assert rows == [("", "legacy sig"), ("alice", "alice sig"), ("bob", "bob sig")]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ai_reply_cache_lookup_is_owner_scoped(tmp_path, monkeypatch):
|
||||
import routes.email_helpers as email_helpers
|
||||
@@ -166,6 +233,136 @@ async def test_ai_reply_cache_lookup_is_owner_scoped(tmp_path, monkeypatch):
|
||||
assert result["model_used"] == "m-b"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sender_signature_read_lookup_is_owner_scoped(tmp_path, monkeypatch):
|
||||
import routes.email_helpers as email_helpers
|
||||
import routes.email_routes as email_routes
|
||||
|
||||
db_path = tmp_path / "scheduled_emails.db"
|
||||
monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path)
|
||||
monkeypatch.setattr(email_routes, "SCHEDULED_DB", db_path)
|
||||
email_helpers._init_scheduled_db()
|
||||
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "alice", "alice private sig", 3, "2026-01-01", "m-a", "llm"),
|
||||
)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "bob", "bob private sig", 3, "2026-01-02", "m-b", "llm"),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
raw = (
|
||||
b"From: Writer <writer@example.com>\r\n"
|
||||
b"To: Bob <bob@example.com>\r\n"
|
||||
b"Subject: Hello\r\n"
|
||||
b"Message-ID: <shared@example.com>\r\n"
|
||||
b"Date: Tue, 01 Jan 2026 12:00:00 +0000\r\n"
|
||||
b"Content-Type: text/plain; charset=utf-8\r\n"
|
||||
b"\r\n"
|
||||
b"Body"
|
||||
)
|
||||
|
||||
class FakeImap:
|
||||
def select(self, *_args, **_kwargs):
|
||||
return "OK", []
|
||||
|
||||
def uid(self, command, _uid, query):
|
||||
assert command == "FETCH"
|
||||
assert query == "(BODY.PEEK[])"
|
||||
return "OK", [(b"1 (UID 1 BODY[])", raw)]
|
||||
|
||||
@contextmanager
|
||||
def fake_imap(_account_id=None, owner=""):
|
||||
assert owner == "bob"
|
||||
yield FakeImap()
|
||||
|
||||
monkeypatch.setattr(email_routes, "_imap", fake_imap)
|
||||
router = email_routes.setup_email_routes()
|
||||
read_email = _route_endpoint(router, "/api/email/read/{uid}", "GET")
|
||||
|
||||
result = await read_email("1", folder="INBOX", account_id=None, owner="bob", mark_seen=False)
|
||||
|
||||
assert result["sender_signature"] == "bob private sig"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sender_signature_clear_cache_keeps_other_owner_rows(tmp_path, monkeypatch):
|
||||
import routes.email_helpers as email_helpers
|
||||
import routes.task_routes as task_routes
|
||||
|
||||
db_path = tmp_path / "scheduled_emails.db"
|
||||
monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path)
|
||||
email_helpers._init_scheduled_db()
|
||||
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "alice", "alice private sig", 3, "2026-01-01", "m-a", "llm"),
|
||||
)
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT INTO sender_signatures
|
||||
(from_address, owner, signature_text, sample_count, last_built_at, model_used, source)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
""",
|
||||
("writer@example.com", "bob", "bob private sig", 3, "2026-01-02", "m-b", "llm"),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
class FakeQuery:
|
||||
def filter(self, *_args):
|
||||
return self
|
||||
|
||||
def first(self):
|
||||
return SimpleNamespace(
|
||||
id="task-1",
|
||||
owner="alice",
|
||||
action="learn_sender_signatures",
|
||||
)
|
||||
|
||||
class FakeDb:
|
||||
def query(self, _model):
|
||||
return FakeQuery()
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(task_routes, "SessionLocal", lambda: FakeDb())
|
||||
monkeypatch.setattr(task_routes, "get_current_user", lambda _request: "alice")
|
||||
|
||||
router = task_routes.setup_task_routes(task_scheduler=SimpleNamespace(pop_notifications=lambda owner: []))
|
||||
clear_cache = _route_endpoint(router, "/api/tasks/{task_id}/clear-cache", "POST")
|
||||
|
||||
result = await clear_cache(SimpleNamespace(), "task-1")
|
||||
|
||||
assert result["cleared"]["sender_signatures"] == 1
|
||||
conn = sqlite3.connect(db_path)
|
||||
try:
|
||||
rows = conn.execute(
|
||||
"SELECT owner, signature_text FROM sender_signatures ORDER BY owner",
|
||||
).fetchall()
|
||||
finally:
|
||||
conn.close()
|
||||
assert rows == [("bob", "bob private sig")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scheduled_email_routes_are_owner_scoped(tmp_path, monkeypatch):
|
||||
import routes.email_helpers as email_helpers
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""Regression guard: Opus 4.7+ rejects the temperature field entirely.
|
||||
|
||||
Anthropic removed the sampling parameters (temperature, top_p, top_k) starting
|
||||
with Claude Opus 4.7 — sending `temperature` at all, even 0.0, returns HTTP 400.
|
||||
This broke every native-Anthropic call to Opus 4.7/4.8, including the research
|
||||
endpoint probe (temperature=0) and all DeepResearcher LLM calls, because
|
||||
_build_anthropic_payload sent `temperature` unconditionally.
|
||||
|
||||
Earlier Claude models (Opus 4.6 and below, every Sonnet/Haiku) still accept
|
||||
temperature in [0.0, 1.0], so the omission is version-gated — the clamp-to-[0,1]
|
||||
behavior for those models (test_llm_core_anthropic_temp_clamp.py) is unchanged.
|
||||
"""
|
||||
import os
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:")
|
||||
|
||||
import pytest
|
||||
|
||||
from src.llm_core import _anthropic_rejects_temperature, _build_anthropic_payload
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model",
|
||||
[
|
||||
"claude-opus-4-7",
|
||||
"claude-opus-4-8",
|
||||
"claude-opus-4-8-20260101", # tolerate a dated snapshot suffix
|
||||
"claude-opus-4-7-20260201", # dated 4.7 snapshot — explicit minor, still >= 4.7
|
||||
"anthropic/claude-opus-4-7", # tolerate a provider-prefixed id
|
||||
"claude-opus-4-10", # future minor still >= 4.7
|
||||
"claude-opus-5-0", # future major
|
||||
],
|
||||
)
|
||||
def test_opus_47_plus_rejects_temperature(model):
|
||||
assert _anthropic_rejects_temperature(model) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model",
|
||||
[
|
||||
"claude-opus-4-6",
|
||||
"claude-opus-4-5",
|
||||
"claude-opus-4-1",
|
||||
"claude-opus-4-0",
|
||||
"claude-opus-4", # bare major (no minor) — kept
|
||||
"claude-opus-4-20250514", # Opus 4.0 dated id — the date must NOT read as a 4.7+ minor
|
||||
"claude-opus-4-1-20250805", # Opus 4.1 dated id — explicit minor before the date
|
||||
"claude-opus-4-6-20251201", # dated 4.6 snapshot — older, still keeps temperature
|
||||
"claude-sonnet-4-6",
|
||||
"claude-3-5-sonnet",
|
||||
"claude-3-opus-20240229", # legacy Claude 3 Opus — no opus-N-M pattern, kept
|
||||
"claude-haiku-4-5",
|
||||
"claude-x",
|
||||
"octopus-4-8", # "opus" only as a substring of another word — must not match
|
||||
"myproxy/octopus-4-8", # same, behind a provider prefix
|
||||
"",
|
||||
None,
|
||||
],
|
||||
)
|
||||
def test_older_claude_models_keep_temperature(model):
|
||||
assert _anthropic_rejects_temperature(model) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("model", [123, 1.5, ["claude-opus-4-8"], {"a": 1}, object()])
|
||||
def test_non_string_model_is_handled_without_crashing(model):
|
||||
# Defensive: the gate must not raise on a non-string model (the old builder
|
||||
# never called .lower() on it). Truthy non-strings should classify as False.
|
||||
assert _anthropic_rejects_temperature(model) is False
|
||||
|
||||
|
||||
def _payload(model, temperature=0.0):
|
||||
return _build_anthropic_payload(
|
||||
model, [{"role": "user", "content": "hi"}], temperature, 100
|
||||
)
|
||||
|
||||
|
||||
def test_payload_omits_temperature_for_opus_47_plus():
|
||||
# The endpoint probe sends temperature=0; on Opus 4.7+ that field must be gone.
|
||||
payload = _payload("claude-opus-4-8", 0.0)
|
||||
assert "temperature" not in payload
|
||||
|
||||
|
||||
def test_payload_keeps_temperature_for_older_models():
|
||||
payload = _payload("claude-opus-4-6", 0.3)
|
||||
assert payload["temperature"] == 0.3
|
||||
# Older models retain the [0,1] clamp (Nietzsche preset at 1.2 -> 1.0).
|
||||
assert _payload("claude-3-5-sonnet", 1.2)["temperature"] == 1.0
|
||||
|
||||
|
||||
def test_payload_keeps_temperature_for_dated_opus_4_0():
|
||||
# Anthropic's dated id for Opus 4.0 (claude-opus-4-20250514) is in this repo's
|
||||
# ANTHROPIC_MODELS list. The date must not be misread as a >= 4.7 minor, or the
|
||||
# user's temperature would be silently dropped on a model that accepts it.
|
||||
assert _payload("claude-opus-4-20250514", 0.5)["temperature"] == 0.5
|
||||
@@ -14,6 +14,7 @@ import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
import routes.memory_routes as mr
|
||||
from src.request_models import MemoryAddRequest
|
||||
|
||||
|
||||
def _route(router, path, method):
|
||||
@@ -38,6 +39,13 @@ def _router(monkeypatch, caller):
|
||||
return mr.setup_memory_routes(mem, sm)
|
||||
|
||||
|
||||
def _request(user):
|
||||
return SimpleNamespace(
|
||||
state=SimpleNamespace(current_user=user),
|
||||
app=SimpleNamespace(state=SimpleNamespace(auth_manager=None)),
|
||||
)
|
||||
|
||||
|
||||
def test_extract_rejects_other_users_session(monkeypatch):
|
||||
router = _router(monkeypatch, caller="bob")
|
||||
extract = _route(router, "/api/memory/extract", "POST")
|
||||
@@ -59,3 +67,61 @@ def test_owner_can_access_own_session(monkeypatch):
|
||||
gbs = _route(router, "/api/memory/by-session/{session_id}", "GET")
|
||||
out = gbs(request=None, session_id="alice-sess")
|
||||
assert out["session_name"] == "Secret project"
|
||||
|
||||
|
||||
def test_add_memory_rejects_other_users_session(monkeypatch):
|
||||
memory_manager = MagicMock()
|
||||
session_manager = MagicMock()
|
||||
memory_vector = MagicMock(healthy=True)
|
||||
router = mr.setup_memory_routes(
|
||||
memory_manager=memory_manager,
|
||||
session_manager=session_manager,
|
||||
memory_vector=memory_vector,
|
||||
)
|
||||
add_memory = _route(router, "/api/memory/add", "POST")
|
||||
|
||||
memory_manager.load.return_value = []
|
||||
memory_manager.find_duplicates.return_value = False
|
||||
session_manager.get_session.return_value = SimpleNamespace(owner="bob", name="Bob session")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
asyncio.run(
|
||||
add_memory(
|
||||
request=_request("alice"),
|
||||
memory_data=MemoryAddRequest(
|
||||
text="Alice note",
|
||||
category="fact",
|
||||
source="user",
|
||||
session_id="bob-session",
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 404
|
||||
assert exc.value.detail == "Session not found"
|
||||
session_manager.get_session.assert_called_once_with("bob-session")
|
||||
memory_manager.add_entry.assert_not_called()
|
||||
memory_manager.save.assert_not_called()
|
||||
memory_vector.add.assert_not_called()
|
||||
|
||||
|
||||
def test_timeline_does_not_expose_other_users_session_name():
|
||||
memory_manager = MagicMock()
|
||||
session_manager = MagicMock()
|
||||
session_manager.sessions = {"bob-session": object()}
|
||||
session_manager.get_session.return_value = SimpleNamespace(owner="bob", name="Bob roadmap")
|
||||
memory_manager.load.return_value = [
|
||||
{
|
||||
"id": "m1",
|
||||
"text": "Alice note",
|
||||
"owner": "alice",
|
||||
"session_id": "bob-session",
|
||||
"timestamp": 1,
|
||||
}
|
||||
]
|
||||
router = mr.setup_memory_routes(memory_manager, session_manager)
|
||||
timeline = _route(router, "/api/memory/timeline", "GET")
|
||||
|
||||
out = timeline(request=_request("alice"))
|
||||
|
||||
assert out["timeline"][0]["session_name"] == "Unknown"
|
||||
|
||||
+11
-11
@@ -6,7 +6,7 @@ import types
|
||||
import pytest
|
||||
|
||||
import src.model_context as model_context
|
||||
from src.model_context import _is_local_endpoint, estimate_tokens, _lookup_known
|
||||
from src.model_context import is_local_endpoint, estimate_tokens, _lookup_known
|
||||
|
||||
|
||||
class _Column:
|
||||
@@ -56,20 +56,20 @@ def _install_endpoint_db(monkeypatch, rows):
|
||||
|
||||
class TestIsLocalEndpoint:
|
||||
def test_localhost(self):
|
||||
assert _is_local_endpoint("http://localhost:5000/v1/chat/completions") is True
|
||||
assert is_local_endpoint("http://localhost:5000/v1/chat/completions") is True
|
||||
|
||||
def test_loopback_ipv4(self):
|
||||
assert _is_local_endpoint("http://127.0.0.1:8080/v1/chat/completions") is True
|
||||
assert is_local_endpoint("http://127.0.0.1:8080/v1/chat/completions") is True
|
||||
|
||||
def test_private_192_168(self):
|
||||
assert _is_local_endpoint("http://192.168.1.1:11434/v1/chat/completions") is True
|
||||
assert is_local_endpoint("http://192.168.1.1:11434/v1/chat/completions") is True
|
||||
|
||||
def test_private_10(self):
|
||||
assert _is_local_endpoint("http://10.0.0.5:8000/v1/chat/completions") is True
|
||||
assert is_local_endpoint("http://10.0.0.5:8000/v1/chat/completions") is True
|
||||
|
||||
def test_tailscale_100(self):
|
||||
# 100.64.0.0/10 is the CGNAT range Tailscale uses.
|
||||
assert _is_local_endpoint("http://100.64.0.1:5000/v1/chat/completions") is True
|
||||
assert is_local_endpoint("http://100.64.0.1:5000/v1/chat/completions") is True
|
||||
|
||||
def test_configured_tailscale_proxy_is_remote(self, monkeypatch):
|
||||
_install_endpoint_db(monkeypatch, [
|
||||
@@ -81,19 +81,19 @@ class TestIsLocalEndpoint:
|
||||
)
|
||||
])
|
||||
|
||||
assert _is_local_endpoint("http://100.117.136.97:34521/v1/chat/completions") is False
|
||||
assert is_local_endpoint("http://100.117.136.97:34521/v1/chat/completions") is False
|
||||
|
||||
def test_openai_is_remote(self):
|
||||
assert _is_local_endpoint("https://api.openai.com/v1/chat/completions") is False
|
||||
assert is_local_endpoint("https://api.openai.com/v1/chat/completions") is False
|
||||
|
||||
def test_anthropic_is_remote(self):
|
||||
assert _is_local_endpoint("https://api.anthropic.com/v1/messages") is False
|
||||
assert is_local_endpoint("https://api.anthropic.com/v1/messages") is False
|
||||
|
||||
def test_empty_url(self):
|
||||
assert _is_local_endpoint("") is False
|
||||
assert is_local_endpoint("") is False
|
||||
|
||||
def test_malformed_url(self):
|
||||
assert _is_local_endpoint("not-a-url") is False
|
||||
assert is_local_endpoint("not-a-url") is False
|
||||
|
||||
|
||||
class TestEstimateTokens:
|
||||
|
||||
@@ -54,6 +54,7 @@ with preserve_import_state("core.database", "src.database", "core.session_manage
|
||||
_endpoint_settings_using_endpoint,
|
||||
_clear_endpoint_settings_for_endpoint,
|
||||
_clear_user_pref_endpoint_refs,
|
||||
_default_endpoint_needs_assignment,
|
||||
_PROVIDER_CURATED,
|
||||
)
|
||||
from src.llm_core import ANTHROPIC_MODELS
|
||||
@@ -154,6 +155,26 @@ def test_endpoint_cleanup_updates_scoped_and_legacy_user_prefs():
|
||||
assert legacy["default_model_fallbacks"] == []
|
||||
|
||||
|
||||
# ── _default_endpoint_needs_assignment (add-endpoint auto-default) ──
|
||||
|
||||
def test_default_assignment_when_none_configured():
|
||||
# Nothing configured yet → first added endpoint should become the default.
|
||||
assert _default_endpoint_needs_assignment("", {"a", "b"}) is True
|
||||
|
||||
|
||||
def test_default_assignment_when_current_default_disabled():
|
||||
# #3586: the configured default points at an endpoint that is no longer
|
||||
# enabled (the user disabled it). Adding a new endpoint must reassign the
|
||||
# default — otherwise Memory → Tidy keeps failing with "No default model
|
||||
# configured" even though an enabled endpoint exists.
|
||||
assert _default_endpoint_needs_assignment("disabled-ep", {"new-ep"}) is True
|
||||
|
||||
|
||||
def test_default_preserved_when_current_default_enabled():
|
||||
# Normal case: the configured default is still enabled → leave it alone.
|
||||
assert _default_endpoint_needs_assignment("live-ep", {"live-ep", "new-ep"}) is False
|
||||
|
||||
|
||||
# ── _match_provider_curated ──
|
||||
|
||||
class TestMatchProviderCurated:
|
||||
@@ -966,16 +987,21 @@ def _create_form_kwargs(**overrides):
|
||||
return kwargs
|
||||
|
||||
|
||||
def _patch_create_deps(monkeypatch, db):
|
||||
def _patch_create_deps(monkeypatch, db, settings=None):
|
||||
import src.auth_helpers as auth_helpers
|
||||
# Shared, in-memory settings so the auto-default write path stays hermetic
|
||||
# (no real settings.json). Returned so tests can assert what was persisted.
|
||||
settings = {"default_endpoint_id": "exists"} if settings is None else settings
|
||||
monkeypatch.setattr(model_routes, "SessionLocal", lambda: db)
|
||||
monkeypatch.setattr(model_routes, "require_admin", lambda request: None)
|
||||
monkeypatch.setattr(model_routes, "ModelEndpoint", _RecordingEndpoint)
|
||||
monkeypatch.setattr(model_routes, "_normalize_base", lambda b: b)
|
||||
monkeypatch.setattr(model_routes, "_rewrite_loopback_for_docker", lambda b, **k: b)
|
||||
monkeypatch.setattr(model_routes, "_load_settings", lambda: {"default_endpoint_id": "exists"})
|
||||
monkeypatch.setattr(model_routes, "_load_settings", lambda: settings)
|
||||
monkeypatch.setattr(model_routes, "_save_settings", lambda s: settings.update(s))
|
||||
monkeypatch.setattr(endpoint_resolver, "resolve_url", lambda u: u)
|
||||
monkeypatch.setattr(auth_helpers, "get_current_user", lambda req: None)
|
||||
return settings
|
||||
|
||||
|
||||
def test_list_model_endpoints_returns_key_fingerprint(monkeypatch):
|
||||
@@ -1091,6 +1117,48 @@ def test_post_same_base_url_different_api_key_creates_distinct_endpoint(monkeypa
|
||||
assert db.added[0].api_key == "key-two"
|
||||
|
||||
|
||||
def test_post_reassigns_default_when_current_default_disabled(monkeypatch):
|
||||
# #3586: the configured default points at a now-disabled endpoint. Adding a
|
||||
# new endpoint must promote it to the default, otherwise raw-setting readers
|
||||
# (Memory → Tidy) keep failing with "No default model configured".
|
||||
disabled = _make_endpoint(id="dead", base_url="http://old-host/v1", is_enabled=False)
|
||||
db = _PinnedFakeDb([disabled])
|
||||
settings = _patch_create_deps(
|
||||
monkeypatch, db, settings={"default_endpoint_id": "dead", "default_model": "stale"}
|
||||
)
|
||||
create = _get_route("/api/model-endpoints", "POST")
|
||||
|
||||
create(
|
||||
_PinnedFakeRequest(),
|
||||
base_url="http://new-host:1234/v1",
|
||||
**_create_form_kwargs(),
|
||||
)
|
||||
|
||||
new_id = db.added[0].id
|
||||
assert settings["default_endpoint_id"] == new_id
|
||||
assert settings["default_endpoint_id"] != "dead"
|
||||
|
||||
|
||||
def test_post_keeps_default_when_current_default_enabled(monkeypatch):
|
||||
# Counter-case: an enabled default must be left untouched when another
|
||||
# endpoint is added.
|
||||
live = _make_endpoint(id="live", base_url="http://live-host/v1", is_enabled=True)
|
||||
db = _PinnedFakeDb([live])
|
||||
settings = _patch_create_deps(
|
||||
monkeypatch, db, settings={"default_endpoint_id": "live", "default_model": "live-model"}
|
||||
)
|
||||
create = _get_route("/api/model-endpoints", "POST")
|
||||
|
||||
create(
|
||||
_PinnedFakeRequest(),
|
||||
base_url="http://another-host:1234/v1",
|
||||
**_create_form_kwargs(),
|
||||
)
|
||||
|
||||
assert settings["default_endpoint_id"] == "live"
|
||||
assert settings["default_model"] == "live-model"
|
||||
|
||||
|
||||
def test_post_same_base_url_same_api_key_still_dedupes(monkeypatch):
|
||||
existing = _make_endpoint(
|
||||
base_url="https://api.example.test/v1",
|
||||
|
||||
@@ -47,6 +47,20 @@ def test_find_bash_checks_local_app_data_git_install(monkeypatch):
|
||||
assert platform_compat.find_bash() == expected
|
||||
|
||||
|
||||
def test_find_bash_checks_local_app_data_programs_git_install(monkeypatch):
|
||||
_reset_bash_cache(monkeypatch)
|
||||
monkeypatch.setattr(platform_compat, "IS_WINDOWS", True)
|
||||
monkeypatch.setattr(platform_compat.shutil, "which", lambda _name: None)
|
||||
for env_name in platform_compat._WINDOWS_BASH_ROOT_ENV_VARS:
|
||||
monkeypatch.delenv(env_name, raising=False)
|
||||
monkeypatch.setenv("LocalAppData", r"C:\Users\alice\AppData\Local")
|
||||
|
||||
expected = r"C:\Users\alice\AppData\Local\Programs\Git\bin\bash.exe"
|
||||
monkeypatch.setattr(platform_compat.os.path, "exists", lambda path: path == expected)
|
||||
|
||||
assert platform_compat.find_bash() == expected
|
||||
|
||||
|
||||
def test_find_bash_skips_windows_wsl_stub(monkeypatch):
|
||||
_reset_bash_cache(monkeypatch)
|
||||
monkeypatch.setattr(platform_compat, "IS_WINDOWS", True)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Renaming a user must update all three owner caches, not just the SQL DB.
|
||||
"""Renaming a user must update non-SQL owner stores, not just the SQL DB.
|
||||
|
||||
The DB owner-rename loop in the rename_user route updates every SQL-backed
|
||||
owner column, but three file-backed / in-memory stores are left stale:
|
||||
@@ -17,6 +17,9 @@ owner column, but three file-backed / in-memory stores are left stale:
|
||||
4. data/memory.json — a flat array where every entry has an `owner` field;
|
||||
memory_manager.load(owner=user) filters on it, so all memories vanish.
|
||||
|
||||
5. data/uploads/uploads.json — each upload row carries an `owner` field and
|
||||
owner-prefixed index key; stale metadata denies renamed users their uploads.
|
||||
|
||||
Regression coverage: these bugs are invisible in unit tests that mock the DB
|
||||
loop but don't exercise the file/cache patches added to the route.
|
||||
"""
|
||||
@@ -67,11 +70,12 @@ def rename_endpoint(monkeypatch, tmp_path):
|
||||
return _route(ar.setup_auth_routes(am), "rename_user"), am, tmp_path
|
||||
|
||||
|
||||
def _request(tmp_path, session_manager=None, token="t", research_handler=None):
|
||||
def _request(tmp_path, session_manager=None, token="t", research_handler=None, upload_handler=None):
|
||||
state = SimpleNamespace(
|
||||
invalidate_token_cache=lambda: None,
|
||||
session_manager=session_manager,
|
||||
research_handler=research_handler,
|
||||
upload_handler=upload_handler,
|
||||
)
|
||||
return SimpleNamespace(
|
||||
cookies={"odysseus_session": token},
|
||||
@@ -415,7 +419,56 @@ def test_rename_no_memory_json_does_not_crash(rename_endpoint):
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Skills (SKILL.md frontmatter + _usage.json sidecar)
|
||||
# 4. uploads.json
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_rename_updates_upload_metadata_owner(rename_endpoint):
|
||||
endpoint, _am, tmp_path = rename_endpoint
|
||||
from src.upload_handler import UploadHandler
|
||||
|
||||
upload_dir = tmp_path / "uploads"
|
||||
dated = upload_dir / "2026" / "06" / "09"
|
||||
dated.mkdir(parents=True)
|
||||
upload_id = "a" * 32 + ".txt"
|
||||
upload_path = dated / upload_id
|
||||
upload_path.write_text("alice private upload", encoding="utf-8")
|
||||
handler = UploadHandler(str(tmp_path), str(upload_dir))
|
||||
handler._atomic_write_json(
|
||||
str(upload_dir / "uploads.json"),
|
||||
{
|
||||
"alice:hash-alice": {
|
||||
"id": upload_id,
|
||||
"path": str(upload_path),
|
||||
"mime": "text/plain",
|
||||
"size": upload_path.stat().st_size,
|
||||
"name": "note.txt",
|
||||
"hash": "hash-alice",
|
||||
"original_name": "note.txt",
|
||||
"uploaded_at": "2026-06-09T10:00:00",
|
||||
"last_accessed": "2026-06-09T10:00:00",
|
||||
"client_ip": "127.0.0.1",
|
||||
"owner": "alice",
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
asyncio.run(
|
||||
endpoint(
|
||||
"alice",
|
||||
SimpleNamespace(username="alice2"),
|
||||
_request(tmp_path, upload_handler=handler),
|
||||
)
|
||||
)
|
||||
|
||||
updated = json.loads((upload_dir / "uploads.json").read_text(encoding="utf-8"))
|
||||
assert "alice:hash-alice" not in updated
|
||||
assert updated["alice2:hash-alice"]["owner"] == "alice2"
|
||||
assert handler.resolve_upload(upload_id, owner="alice2")["path"] == str(upload_path)
|
||||
assert handler.resolve_upload(upload_id, owner="alice") is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 5. Skills (SKILL.md frontmatter + _usage.json sidecar)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_SKILL_MD = """\
|
||||
@@ -522,7 +575,7 @@ def test_rename_usage_keys_case_insensitive(rename_endpoint):
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 5. Rollback: auth rename must be restored if SQL owner migration fails
|
||||
# 6. Rollback: auth rename must be restored if SQL owner migration fails
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_owner_migration_failure_rolls_back_auth_rename(monkeypatch, tmp_path):
|
||||
@@ -583,7 +636,7 @@ def test_self_rename_owner_migration_failure_rolls_back_auth_session(monkeypatch
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 6. P1 regression: rejected auth rename must not mutate file-backed stores
|
||||
# 7. P1 regression: rejected auth rename must not mutate file-backed stores
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_rejected_rename_does_not_mutate_files(monkeypatch, tmp_path):
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
"""FTS session search must fetch hit rows in one query, not one per hit.
|
||||
|
||||
_search_fts looked up each FTS hit's full row with its own
|
||||
db.query(...).filter(id == message_id).first(), an N+1 query. The lookup is now
|
||||
a single batched IN(...) query via _fetch_messages_by_id.
|
||||
"""
|
||||
from src.session_search import _fetch_messages_by_id
|
||||
|
||||
|
||||
class _Msg:
|
||||
def __init__(self, mid):
|
||||
self.id = mid
|
||||
|
||||
|
||||
class _Query:
|
||||
def __init__(self, rows, calls):
|
||||
self._rows = rows
|
||||
self._calls = calls
|
||||
|
||||
def join(self, *a, **k):
|
||||
return self
|
||||
|
||||
def filter(self, *a, **k):
|
||||
return self
|
||||
|
||||
def all(self):
|
||||
self._calls["all"] += 1
|
||||
return self._rows
|
||||
|
||||
|
||||
class _DB:
|
||||
def __init__(self, rows):
|
||||
self._rows = rows
|
||||
self.calls = {"query": 0, "all": 0}
|
||||
|
||||
def query(self, *a, **k):
|
||||
self.calls["query"] += 1
|
||||
return _Query(self._rows, self.calls)
|
||||
|
||||
|
||||
def test_batches_into_single_query():
|
||||
rows = [(_Msg("m1"), "Session One"), (_Msg("m2"), "Session Two")]
|
||||
db = _DB(rows)
|
||||
out = _fetch_messages_by_id(db, ["m1", "m2"])
|
||||
# One query for all hits, not one per hit.
|
||||
assert db.calls["query"] == 1
|
||||
assert db.calls["all"] == 1
|
||||
assert out["m1"][1] == "Session One"
|
||||
assert out["m2"][0].id == "m2"
|
||||
|
||||
|
||||
def test_empty_ids_does_no_query():
|
||||
db = _DB([])
|
||||
assert _fetch_messages_by_id(db, []) == {}
|
||||
assert db.calls["query"] == 0
|
||||
@@ -40,7 +40,8 @@ def test_secret_in_list_of_dicts_blanked():
|
||||
|
||||
def test_non_secret_keys_preserved():
|
||||
s = {"keybinds": {"send": "Enter"}, "theme": "dark", "image_model": "x",
|
||||
"default_endpoint_id": "ep1", "search_result_count": 5, "tts_enabled": True}
|
||||
"default_endpoint_id": "ep1", "search_result_count": 5, "tts_enabled": True,
|
||||
"tokenId": "public-id", "keyId": "public-key-id"}
|
||||
assert scrub_settings(s) == s # untouched
|
||||
|
||||
|
||||
@@ -71,6 +72,23 @@ def test_exact_name_matches():
|
||||
assert all(v == "" for v in out.values()), out
|
||||
|
||||
|
||||
def test_camel_case_secret_keys_blanked():
|
||||
out = scrub_settings({
|
||||
"apiKey": "api-secret",
|
||||
"accessToken": "access-secret",
|
||||
"refreshToken": "refresh-secret",
|
||||
"clientSecret": "client-secret",
|
||||
"hfToken": "hf-secret",
|
||||
"nested": {"privateKey": "private-secret"},
|
||||
})
|
||||
assert out["apiKey"] == ""
|
||||
assert out["accessToken"] == ""
|
||||
assert out["refreshToken"] == ""
|
||||
assert out["clientSecret"] == ""
|
||||
assert out["hfToken"] == ""
|
||||
assert out["nested"]["privateKey"] == ""
|
||||
|
||||
|
||||
def test_non_object_settings_return_empty_mapping():
|
||||
assert scrub_settings(["not", "settings"]) == {}
|
||||
assert scrub_settings("not settings") == {}
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
from src.upload_handler import UploadHandler
|
||||
|
||||
|
||||
def _make_handler(tmp_path: Path) -> UploadHandler:
|
||||
base = tmp_path / "base"
|
||||
upload = tmp_path / "uploads"
|
||||
base.mkdir()
|
||||
upload.mkdir()
|
||||
return UploadHandler(base_dir=str(base), upload_dir=str(upload))
|
||||
|
||||
|
||||
def _db_path(handler: UploadHandler) -> str:
|
||||
return os.path.join(handler.upload_dir, "uploads.json")
|
||||
|
||||
|
||||
def _write_upload_file(handler: UploadHandler, file_id: str, content: bytes = b"content") -> str:
|
||||
upload_day = Path(handler.upload_dir) / "2026" / "06" / "09"
|
||||
upload_day.mkdir(parents=True, exist_ok=True)
|
||||
path = upload_day / file_id
|
||||
path.write_bytes(content)
|
||||
return str(path)
|
||||
|
||||
|
||||
def _entry(handler: UploadHandler, owner: str, file_hash: str, file_id: str) -> dict:
|
||||
path = _write_upload_file(handler, file_id, content=f"{owner}:{file_hash}".encode())
|
||||
return {
|
||||
"id": file_id,
|
||||
"path": path,
|
||||
"mime": "text/plain",
|
||||
"size": os.path.getsize(path),
|
||||
"name": f"{file_id}.txt",
|
||||
"hash": file_hash,
|
||||
"original_name": f"{file_id}.txt",
|
||||
"uploaded_at": "2026-06-09T10:00:00",
|
||||
"last_accessed": "2026-06-09T10:00:00",
|
||||
"client_ip": "127.0.0.1",
|
||||
"owner": owner,
|
||||
}
|
||||
|
||||
|
||||
def test_rename_owner_updates_upload_metadata_key_and_resolver(tmp_path):
|
||||
handler = _make_handler(tmp_path)
|
||||
alice_id = "a" * 32 + ".txt"
|
||||
alice_entry = _entry(handler, "Alice", "hash-alice", alice_id)
|
||||
bob_entry = _entry(handler, "bob", "hash-bob", "b" * 32 + ".txt")
|
||||
handler._atomic_write_json(
|
||||
_db_path(handler),
|
||||
{
|
||||
"Alice:hash-alice": alice_entry,
|
||||
"bob:hash-bob": bob_entry,
|
||||
},
|
||||
)
|
||||
|
||||
renamed = handler.rename_owner("alice", "alice2")
|
||||
|
||||
assert renamed == 1
|
||||
updated = json.loads(Path(_db_path(handler)).read_text(encoding="utf-8"))
|
||||
assert "Alice:hash-alice" not in updated
|
||||
assert "alice2:hash-alice" in updated
|
||||
assert updated["alice2:hash-alice"]["owner"] == "alice2"
|
||||
assert updated["alice2:hash-alice"]["path"] == alice_entry["path"]
|
||||
assert updated["alice2:hash-alice"]["hash"] == alice_entry["hash"]
|
||||
assert updated["alice2:hash-alice"]["uploaded_at"] == alice_entry["uploaded_at"]
|
||||
assert updated["alice2:hash-alice"]["last_accessed"] == alice_entry["last_accessed"]
|
||||
assert updated["bob:hash-bob"]["owner"] == "bob"
|
||||
|
||||
assert handler.resolve_upload(alice_id, owner="alice2")["id"] == alice_id
|
||||
assert handler.resolve_upload(alice_id, owner="alice") is None
|
||||
|
||||
|
||||
def test_rename_owner_preserves_rows_when_target_key_collides(tmp_path):
|
||||
handler = _make_handler(tmp_path)
|
||||
migrated_id = "c" * 32 + ".txt"
|
||||
existing_id = "d" * 32 + ".txt"
|
||||
migrated = _entry(handler, "alice", "same-hash", migrated_id)
|
||||
existing = _entry(handler, "alice2", "same-hash", existing_id)
|
||||
unrelated = _entry(handler, "carol", "other-hash", "e" * 32 + ".txt")
|
||||
handler._atomic_write_json(
|
||||
_db_path(handler),
|
||||
{
|
||||
"alice:same-hash": migrated,
|
||||
"alice2:same-hash": existing,
|
||||
"carol:other-hash": unrelated,
|
||||
},
|
||||
)
|
||||
|
||||
renamed = handler.rename_owner("alice", "alice2")
|
||||
|
||||
assert renamed == 1
|
||||
updated = json.loads(Path(_db_path(handler)).read_text(encoding="utf-8"))
|
||||
assert len(updated) == 3
|
||||
assert updated["alice2:same-hash"]["id"] == existing_id
|
||||
migrated_key = f"alice2:same-hash:{migrated_id}"
|
||||
assert updated[migrated_key]["id"] == migrated_id
|
||||
assert updated[migrated_key]["owner"] == "alice2"
|
||||
assert updated[migrated_key]["path"] == migrated["path"]
|
||||
assert updated["carol:other-hash"] == unrelated
|
||||
@@ -0,0 +1,110 @@
|
||||
"""fetch_webpage_content must return plain-text and Markdown bodies verbatim.
|
||||
|
||||
raw.githubusercontent.com serves Markdown as `text/plain`, and a lot of code
|
||||
and tool documentation lives in `.md` / `.txt`. Those have no HTML structure,
|
||||
so the HTML branch extracted nothing and web_fetch reported "no readable text
|
||||
content". The plain-text branch returns the body as-is. HTML stays on the
|
||||
parsing path.
|
||||
"""
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
from services.search import content as content_mod
|
||||
|
||||
|
||||
class _FakeResponse:
|
||||
def __init__(self, text, content_type, status_code=200):
|
||||
self.text = text
|
||||
self.content = text.encode("utf-8")
|
||||
self.headers = {"Content-Type": content_type}
|
||||
self.status_code = status_code
|
||||
|
||||
def raise_for_status(self):
|
||||
return None
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_cache(monkeypatch, tmp_path):
|
||||
# Force a cache miss and skip disk writes so the test is hermetic.
|
||||
monkeypatch.setattr(content_mod, "CONTENT_CACHE_DIR", tmp_path)
|
||||
monkeypatch.setattr(content_mod, "_cache_result", lambda *a, **k: None)
|
||||
|
||||
|
||||
def _patch_fetch(monkeypatch, text, content_type):
|
||||
monkeypatch.setattr(
|
||||
content_mod,
|
||||
"_get_public_url",
|
||||
lambda url, headers=None, timeout=5: _FakeResponse(text, content_type),
|
||||
)
|
||||
|
||||
|
||||
MARKDOWN = "# Title\n\nSome **docs** with a [link](https://example.com).\n"
|
||||
|
||||
|
||||
def test_markdown_text_plain_returns_body(monkeypatch, no_cache):
|
||||
_patch_fetch(monkeypatch, MARKDOWN, "text/plain; charset=utf-8")
|
||||
r = content_mod.fetch_webpage_content(
|
||||
"https://raw.githubusercontent.com/o/r/master/Documentation/Patterns.md"
|
||||
)
|
||||
assert r["success"] is True
|
||||
assert r["content"] == MARKDOWN.strip()
|
||||
assert r["title"] == "patterns.md"
|
||||
assert r["error"] == ""
|
||||
|
||||
|
||||
def test_text_markdown_content_type_returns_body(monkeypatch, no_cache):
|
||||
_patch_fetch(monkeypatch, MARKDOWN, "text/markdown")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/readme")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == MARKDOWN.strip()
|
||||
|
||||
|
||||
def test_octet_stream_with_txt_suffix_returns_body(monkeypatch, no_cache):
|
||||
# Some servers mislabel text files; the URL-suffix fallback still reads it.
|
||||
_patch_fetch(monkeypatch, "plain notes\nline two\n", "application/octet-stream")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/notes.txt")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == "plain notes\nline two"
|
||||
|
||||
|
||||
def test_application_json_returns_body(monkeypatch, no_cache):
|
||||
# application/json is not text/*; it must still be returned verbatim
|
||||
# instead of being fed to the HTML parser (which yields empty content).
|
||||
body = '{"name": "odysseus", "items": [1, 2, 3]}'
|
||||
_patch_fetch(monkeypatch, body, "application/json")
|
||||
r = content_mod.fetch_webpage_content("https://api.example.com/data")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == body
|
||||
|
||||
|
||||
def test_ld_json_suffix_content_type_returns_body(monkeypatch, no_cache):
|
||||
body = '{"@context": "https://schema.org"}'
|
||||
_patch_fetch(monkeypatch, body, "application/ld+json")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/meta")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == body
|
||||
|
||||
|
||||
def test_json_suffix_with_octet_stream_returns_body(monkeypatch, no_cache):
|
||||
body = '{"raw": true}'
|
||||
_patch_fetch(monkeypatch, body, "application/octet-stream")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/package.json")
|
||||
assert r["success"] is True
|
||||
assert r["content"] == body
|
||||
|
||||
|
||||
def test_empty_text_body_is_not_success(monkeypatch, no_cache):
|
||||
_patch_fetch(monkeypatch, " \n ", "text/plain")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/blank.txt")
|
||||
assert r["success"] is False
|
||||
assert r["content"] == ""
|
||||
|
||||
|
||||
def test_html_still_uses_parser(monkeypatch, no_cache):
|
||||
# An HTML body must not be short-circuited by the text branch.
|
||||
html = "<html><head><title>Hi</title></head><body><p>Hello world body text</p></body></html>"
|
||||
_patch_fetch(monkeypatch, html, "text/html; charset=utf-8")
|
||||
r = content_mod.fetch_webpage_content("https://example.com/page")
|
||||
assert r["title"] == "Hi"
|
||||
assert "Hello world body text" in r["content"]
|
||||
@@ -0,0 +1,55 @@
|
||||
"""Fire-and-forget webhook tasks must be referenced until they finish.
|
||||
|
||||
asyncio keeps only a weak reference to a bare create_task() result, so a
|
||||
delivery task could be garbage-collected before it ran and the webhook silently
|
||||
dropped. WebhookManager now holds a strong reference for the task's lifetime and
|
||||
releases it on completion.
|
||||
"""
|
||||
import asyncio
|
||||
import sys
|
||||
|
||||
# webhook_manager does `from src.database import SessionLocal, Webhook` at import
|
||||
# time. The shared test harness stubs src.database without Webhook, so ensure the
|
||||
# attribute exists before importing the manager. These tests never touch the DB
|
||||
# (the manager is built via __new__), so a placeholder class is sufficient.
|
||||
_db = sys.modules.get("src.database")
|
||||
if _db is not None and not hasattr(_db, "Webhook"):
|
||||
_db.Webhook = type("Webhook", (), {})
|
||||
|
||||
from src.webhook_manager import WebhookManager # noqa: E402
|
||||
|
||||
|
||||
def test_spawn_tracked_holds_then_releases_reference():
|
||||
async def run():
|
||||
wm = WebhookManager.__new__(WebhookManager)
|
||||
wm._bg_tasks = set()
|
||||
|
||||
gate = asyncio.Event()
|
||||
|
||||
async def work():
|
||||
await gate.wait()
|
||||
|
||||
task = wm._spawn_tracked(work())
|
||||
# Referenced while in flight (this is what stops GC from collecting it).
|
||||
assert task in wm._bg_tasks
|
||||
gate.set()
|
||||
await task
|
||||
# Reference released once done, so the set does not grow unbounded.
|
||||
assert task not in wm._bg_tasks
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
|
||||
def test_spawn_tracked_runs_the_coroutine():
|
||||
async def run():
|
||||
wm = WebhookManager.__new__(WebhookManager)
|
||||
wm._bg_tasks = set()
|
||||
ran = []
|
||||
|
||||
async def work():
|
||||
ran.append(True)
|
||||
|
||||
await wm._spawn_tracked(work())
|
||||
assert ran == [True]
|
||||
|
||||
asyncio.run(run())
|
||||
@@ -0,0 +1,328 @@
|
||||
"""Workspace confinement.
|
||||
|
||||
The agent's per-turn workspace is a single context-local binding set in
|
||||
execute_tool_block. The shared path resolvers (_resolve_tool_path /
|
||||
_resolve_search_root) and the subprocess cwd helper (agent_cwd) read it, so
|
||||
confinement is enforced in ONE place: a tool that uses the shared helpers is
|
||||
confined automatically and a new tool cannot accidentally bypass it.
|
||||
|
||||
Covers: the resolver helper, the central binding (the safety net), end-to-end
|
||||
confinement of read/write/edit/grep/ls + subprocess cwd via execute_tool_block,
|
||||
the get_workspace tool, no-leak across calls, and the admin-gated browse route.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from src.tool_execution import (
|
||||
_AGENT_WORKDIR,
|
||||
_active_workspace,
|
||||
_resolve_search_root,
|
||||
_resolve_tool_path,
|
||||
_resolve_tool_path_in_workspace,
|
||||
agent_cwd,
|
||||
execute_tool_block,
|
||||
get_active_workspace,
|
||||
)
|
||||
|
||||
|
||||
def _block(tool, content=""):
|
||||
return SimpleNamespace(tool_type=tool, content=content)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def ws():
|
||||
d = tempfile.mkdtemp()
|
||||
with open(os.path.join(d, "a.txt"), "w") as f:
|
||||
f.write("x")
|
||||
return d
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def admin(monkeypatch):
|
||||
"""Pass the public-tool gate so file tools dispatch in tests."""
|
||||
monkeypatch.setattr(
|
||||
"src.tool_execution.owner_is_admin_or_single_user", lambda owner: True
|
||||
)
|
||||
|
||||
|
||||
# ── the resolver helper ────────────────────────────────────────────────
|
||||
|
||||
def test_resolver_confines(ws):
|
||||
real = os.path.realpath(os.path.join(ws, "a.txt"))
|
||||
assert _resolve_tool_path_in_workspace(ws, "a.txt") == real # relative
|
||||
assert _resolve_tool_path_in_workspace(ws, os.path.join(ws, "a.txt")) == real # abs inside
|
||||
outside = tempfile.mkdtemp()
|
||||
with pytest.raises(ValueError): # abs outside
|
||||
_resolve_tool_path_in_workspace(ws, os.path.join(outside, "x.txt"))
|
||||
with pytest.raises(ValueError): # parent escape
|
||||
_resolve_tool_path_in_workspace(ws, os.path.join("..", "..", "escape.txt"))
|
||||
|
||||
|
||||
def test_resolver_blocks_sensitive_inside_workspace(ws):
|
||||
os.makedirs(os.path.join(ws, ".ssh"), exist_ok=True)
|
||||
with pytest.raises(ValueError):
|
||||
_resolve_tool_path_in_workspace(ws, ".ssh/authorized_keys")
|
||||
|
||||
|
||||
# ── the central binding: the safety net ─────────────────────────────────
|
||||
|
||||
def test_active_binding_confines_shared_resolvers(ws):
|
||||
"""ANY tool resolving paths through the shared helpers is confined while the
|
||||
binding is active, without doing anything workspace-specific itself. This is
|
||||
what stops a newly added tool from accidentally ignoring the workspace."""
|
||||
token = _active_workspace.set(ws)
|
||||
try:
|
||||
assert get_active_workspace() == ws
|
||||
assert agent_cwd() == ws
|
||||
assert _resolve_tool_path("a.txt") == os.path.realpath(os.path.join(ws, "a.txt"))
|
||||
with pytest.raises(ValueError): # normally-allowed root, now outside ws
|
||||
_resolve_tool_path("/tmp/whatever.txt")
|
||||
assert _resolve_search_root("") == os.path.realpath(ws)
|
||||
finally:
|
||||
_active_workspace.reset(token)
|
||||
|
||||
|
||||
def test_no_binding_uses_default_roots():
|
||||
assert get_active_workspace() is None
|
||||
assert agent_cwd() == _AGENT_WORKDIR
|
||||
with pytest.raises(ValueError):
|
||||
_resolve_tool_path("/etc/hosts")
|
||||
|
||||
|
||||
# ── end-to-end via execute_tool_block (sets + resets the binding) ───────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_write_edit_confined_e2e(ws, admin):
|
||||
_, r = await execute_tool_block(_block("write_file", "note.txt\nhello"), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and os.path.isfile(os.path.join(ws, "note.txt"))
|
||||
_, r = await execute_tool_block(_block("read_file", "note.txt"), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and r["output"] == "hello"
|
||||
|
||||
with open(os.path.join(ws, "f.txt"), "w") as f:
|
||||
f.write("foo bar")
|
||||
_, r = await execute_tool_block(
|
||||
_block("edit_file", json.dumps({"path": "f.txt", "old_string": "foo", "new_string": "baz"})),
|
||||
owner="a", workspace=ws,
|
||||
)
|
||||
assert r["exit_code"] == 0
|
||||
with open(os.path.join(ws, "f.txt")) as f:
|
||||
assert f.read() == "baz bar"
|
||||
|
||||
# outside the workspace is rejected, and nothing is created
|
||||
outside = tempfile.mkdtemp()
|
||||
of = os.path.join(outside, "secret.txt")
|
||||
with open(of, "w") as f:
|
||||
f.write("nope")
|
||||
_, r = await execute_tool_block(_block("read_file", of), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
|
||||
escape = os.path.join(outside, "_esc.txt")
|
||||
_, r = await execute_tool_block(_block("write_file", f"{escape}\nx"), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
|
||||
assert not os.path.exists(escape)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_grep_and_ls_confined_e2e(ws, admin):
|
||||
with open(os.path.join(ws, "doc.txt"), "w") as f:
|
||||
f.write("hello workspace\n")
|
||||
_, r = await execute_tool_block(_block("grep", json.dumps({"pattern": "hello"})), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and "doc.txt" in r["output"]
|
||||
outside = tempfile.mkdtemp()
|
||||
_, r = await execute_tool_block(_block("grep", json.dumps({"pattern": "x", "path": outside})), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
|
||||
_, r = await execute_tool_block(_block("ls", ""), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and "doc.txt" in r["output"]
|
||||
_, r = await execute_tool_block(_block("ls", outside), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subprocess_cwd_is_workspace_e2e(ws, admin):
|
||||
"""python tool runs with cwd = workspace (OS-agnostic probe)."""
|
||||
_, r = await execute_tool_block(_block("python", "import os; print(os.getcwd())"), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0
|
||||
assert os.path.realpath(r["output"].strip()) == os.path.realpath(ws)
|
||||
|
||||
|
||||
# ── get_workspace tool ──────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_workspace_tool(ws, admin):
|
||||
_, r = await execute_tool_block(_block("get_workspace", ""), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and r["output"].startswith(ws) and "not sandboxed" in r["output"]
|
||||
_, r = await execute_tool_block(_block("get_workspace", ""), owner="a") # none active
|
||||
assert r["exit_code"] == 0 and "No workspace" in r["output"]
|
||||
|
||||
|
||||
# ── no leak across calls ────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_binding_does_not_leak(ws, admin):
|
||||
await execute_tool_block(_block("ls", ""), owner="a", workspace=ws)
|
||||
assert get_active_workspace() is None
|
||||
|
||||
|
||||
# ── tool selection: an active workspace is the file-work signal ─────────
|
||||
# A vague ("low-signal") message like "look at the local project" matches no
|
||||
# domain keywords, so retrieval is normally skipped. When a workspace is set it
|
||||
# must still surface the file tools, otherwise the agent says it has no file
|
||||
# access (the bug this guards against).
|
||||
|
||||
def _sent_tool_names(monkeypatch, *, workspace):
|
||||
import asyncio
|
||||
import src.agent_loop as al
|
||||
|
||||
monkeypatch.setattr(al, "get_setting", lambda key, default=None: default, raising=False)
|
||||
monkeypatch.setattr(al, "get_mcp_manager", lambda: None, raising=False)
|
||||
monkeypatch.setattr(al, "estimate_tokens", lambda *a, **k: 10, raising=False)
|
||||
# Isolate the selection logic from owner gating (tested separately).
|
||||
monkeypatch.setattr(al, "blocked_tools_for_owner", lambda owner: set(), raising=False)
|
||||
|
||||
captured = []
|
||||
|
||||
async def _fake_stream(_candidates, messages, **kwargs):
|
||||
captured.append(kwargs.get("tools"))
|
||||
yield "data: " + json.dumps({"delta": "ok"}) + "\n\n"
|
||||
yield "data: [DONE]\n\n"
|
||||
|
||||
monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False)
|
||||
|
||||
async def _run():
|
||||
gen = al.stream_agent_loop(
|
||||
"https://api.openai.com/v1", "gpt-test",
|
||||
[{"role": "user", "content": "look at the local project"}],
|
||||
max_rounds=1, relevant_tools=None, owner="admin", workspace=workspace,
|
||||
)
|
||||
return [c async for c in gen]
|
||||
|
||||
asyncio.run(_run())
|
||||
schemas = captured[0] or []
|
||||
return {t["function"]["name"] for t in schemas if isinstance(t, dict) and "function" in t}
|
||||
|
||||
|
||||
def test_low_signal_with_workspace_surfaces_readonly_file_tools(monkeypatch):
|
||||
names = _sent_tool_names(monkeypatch, workspace="/tmp")
|
||||
# read-only nav tools surface so the agent can explore
|
||||
assert "read_file" in names
|
||||
assert "get_workspace" in names
|
||||
assert "grep" in names
|
||||
# write/shell tools do NOT surface on a vague message
|
||||
assert "write_file" not in names
|
||||
assert "edit_file" not in names
|
||||
assert "bash" not in names
|
||||
assert "python" not in names
|
||||
|
||||
|
||||
def test_low_signal_without_workspace_excludes_file_tools(monkeypatch):
|
||||
names = _sent_tool_names(monkeypatch, workspace=None)
|
||||
assert "read_file" not in names
|
||||
assert "get_workspace" not in names
|
||||
|
||||
|
||||
# ── browse route is admin-gated ─────────────────────────────────────────
|
||||
|
||||
def test_browse_is_admin_gated(monkeypatch):
|
||||
from fastapi import HTTPException
|
||||
import routes.workspace_routes as wr
|
||||
|
||||
router = wr.setup_workspace_routes()
|
||||
browse = next(r.endpoint for r in router.routes if r.path == "/api/workspace/browse")
|
||||
|
||||
monkeypatch.setattr(wr, "get_current_user", lambda req: "bob")
|
||||
monkeypatch.setattr(wr, "owner_is_admin_or_single_user", lambda owner: False)
|
||||
with pytest.raises(HTTPException) as ei:
|
||||
browse(request=object(), path="/")
|
||||
assert ei.value.status_code == 403
|
||||
|
||||
monkeypatch.setattr(wr, "owner_is_admin_or_single_user", lambda owner: True)
|
||||
out = browse(request=object(), path=os.path.expanduser("~"))
|
||||
assert "dirs" in out and "path" in out
|
||||
assert all("name" in d and "path" in d for d in out["dirs"])
|
||||
|
||||
|
||||
# ── bind-time vetting of the workspace root ─────────────────────────────
|
||||
|
||||
def test_vet_workspace_accepts_normal_dir(ws):
|
||||
from src.tool_execution import vet_workspace
|
||||
assert vet_workspace(ws) == os.path.realpath(ws)
|
||||
|
||||
|
||||
def test_vet_workspace_rejects_sensitive_root(tmp_path):
|
||||
# The resolver deny-lists sensitive paths inside the workspace, but the
|
||||
# empty-path search root is the workspace itself - a sensitive root must
|
||||
# be rejected before it is bound or `ls` with no path would list it.
|
||||
from src.tool_execution import vet_workspace
|
||||
ssh_dir = tmp_path / ".ssh"
|
||||
ssh_dir.mkdir()
|
||||
assert vet_workspace(str(ssh_dir)) is None
|
||||
|
||||
|
||||
def test_vet_workspace_rejects_nondir_and_empty(ws):
|
||||
from src.tool_execution import vet_workspace
|
||||
assert vet_workspace(os.path.join(ws, "a.txt")) is None # file, not dir
|
||||
assert vet_workspace("/nonexistent/path/xyz") is None
|
||||
assert vet_workspace("") is None
|
||||
assert vet_workspace(" ") is None
|
||||
|
||||
|
||||
def test_vet_workspace_rejects_filesystem_root():
|
||||
# Binding / would make every absolute path "inside" the workspace,
|
||||
# collapsing confinement into host-wide file access.
|
||||
from src.tool_execution import vet_workspace
|
||||
assert vet_workspace("/") is None
|
||||
|
||||
|
||||
def test_browse_marks_root_unselectable_and_vet_endpoint(monkeypatch):
|
||||
import routes.workspace_routes as wr
|
||||
|
||||
router = wr.setup_workspace_routes()
|
||||
browse = next(r.endpoint for r in router.routes if r.path == "/api/workspace/browse")
|
||||
vet = next(r.endpoint for r in router.routes if r.path == "/api/workspace/vet")
|
||||
|
||||
monkeypatch.setattr(wr, "get_current_user", lambda req: "admin")
|
||||
monkeypatch.setattr(wr, "owner_is_admin_or_single_user", lambda owner: True)
|
||||
|
||||
out = browse(request=object(), path="/")
|
||||
assert out["selectable"] is False
|
||||
out = browse(request=object(), path=os.path.expanduser("~"))
|
||||
assert out["selectable"] is True
|
||||
|
||||
assert vet(request=object(), path="/") == {"ok": False, "path": None}
|
||||
home = os.path.realpath(os.path.expanduser("~"))
|
||||
assert vet(request=object(), path="~") == {"ok": True, "path": home}
|
||||
|
||||
from fastapi import HTTPException
|
||||
monkeypatch.setattr(wr, "owner_is_admin_or_single_user", lambda owner: False)
|
||||
with pytest.raises(HTTPException) as ei:
|
||||
vet(request=object(), path="/tmp")
|
||||
assert ei.value.status_code == 403
|
||||
|
||||
|
||||
# ── send-time privilege gate (no path oracle for non-admins) ────────────
|
||||
|
||||
def test_request_workspace_gate(ws, monkeypatch):
|
||||
"""Non-admin chat callers must get a uniform drop with no vetting: the
|
||||
workspace_rejected signal would otherwise reveal which host paths exist."""
|
||||
import routes.chat_routes as cr
|
||||
|
||||
monkeypatch.setattr(cr, "get_current_user", lambda req: "bob")
|
||||
vet_calls = []
|
||||
import src.tool_execution as te
|
||||
real_vet = te.vet_workspace
|
||||
monkeypatch.setattr(te, "vet_workspace", lambda p: vet_calls.append(p) or real_vet(p))
|
||||
|
||||
import src.tool_security as ts
|
||||
monkeypatch.setattr(ts, "owner_is_admin_or_single_user", lambda owner: False)
|
||||
# Valid and invalid paths are indistinguishable for a non-admin: both
|
||||
# drop silently, and the path never reaches the filesystem.
|
||||
assert cr._resolve_request_workspace(object(), ws) == ("", "")
|
||||
assert cr._resolve_request_workspace(object(), "/nonexistent/xyz") == ("", "")
|
||||
assert vet_calls == []
|
||||
|
||||
monkeypatch.setattr(ts, "owner_is_admin_or_single_user", lambda owner: True)
|
||||
assert cr._resolve_request_workspace(object(), ws) == (os.path.realpath(ws), "")
|
||||
assert cr._resolve_request_workspace(object(), "/nonexistent/xyz") == ("", "/nonexistent/xyz")
|
||||
Reference in New Issue
Block a user