diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index 69d87b4..586928b 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -142,26 +142,22 @@ _running: set[int] = set() _tasks: set[asyncio.Task] = set() -# Both factories below use the user's own key by construction. They read the -# endpoint and key from `Settings` and never from `auth.DEMO_*`, so -# summarization and embedding cannot spend the shared demo key. Their call sites -# are also skipped when `using_demo` is true. -# -# Do not change these to accept a `ProviderConfig`. `summary_model` and -# `embedding_model` are free-form user input and are not on the demo allowlist. +# Both factories read the endpoint and the model names straight off `Settings`. +# They used to also read an API key, which is gone: Ollama does not use one and +# M2 removed cloud providers. `summary_model` and `embedding_model` fall back to +# the narrator model when the user has not named a separate one. def summary_provider(settings: models.Settings) -> OpenAICompatibleProvider: return OpenAICompatibleProvider( settings.endpoint_url, - settings.api_key_plain, settings.summary_model or settings.model, settings.api_mode, - settings.reasoning_max_tokens, + settings.model_timeout_seconds, ) def embedding_provider(settings: models.Settings) -> OpenAICompatibleProvider: return OpenAICompatibleProvider( - settings.endpoint_url, settings.api_key_plain, settings.embedding_model + settings.endpoint_url, settings.embedding_model ) diff --git a/backend/app/models.py b/backend/app/models.py index fdb9e4d..fca4e65 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -520,10 +520,10 @@ class Settings(Base): # 800 leaves room for a full scene; 400 tended to truncate mid-paragraph # and left reasoning models with nothing after their thinking. max_output_tokens: Mapped[int] = mapped_column(Integer, default=800) - # Separate thinking budget for reasoning models (OpenRouter-style - # `reasoning: {max_tokens}`); 0 = param not sent, -1 = reasoning explicitly - # off (`reasoning: {effort: none}`). Added on top of - # max_output_tokens so story output keeps its full budget. + # Was an OpenRouter-style thinking budget. Ollama's OpenAI-compatible + # endpoint ignores the field, so M2 stopped sending it and removed it from + # the Settings API and UI. The column stays so existing databases open + # unchanged and is never read. reasoning_max_tokens: Mapped[int] = mapped_column(Integer, default=0) context_token_budget: Mapped[int] = mapped_column(Integer, default=16384) # How long to wait for the model, in seconds, before giving up on a turn. diff --git a/backend/app/routers/adventures/turns.py b/backend/app/routers/adventures/turns.py index cae0fb9..8bf973b 100644 --- a/backend/app/routers/adventures/turns.py +++ b/backend/app/routers/adventures/turns.py @@ -170,7 +170,8 @@ async def _generate_turn( parts = PromptParts(system=system_text, story=story_text) provider = OpenAICompatibleProvider( - settings.endpoint_url, settings.model, settings.api_mode + settings.endpoint_url, settings.model, settings.api_mode, + settings.model_timeout_seconds, ) chunks: list[str] = [] reasoning_chunks: list[str] = [] diff --git a/backend/app/routers/chat.py b/backend/app/routers/chat.py index fc9479b..6a7dc65 100644 --- a/backend/app/routers/chat.py +++ b/backend/app/routers/chat.py @@ -59,7 +59,8 @@ async def run_chat( a `done` event, so the frontend reuses the same code. """ provider = OpenAICompatibleProvider( - settings.endpoint_url, model, settings.api_mode + settings.endpoint_url, model, settings.api_mode, + settings.model_timeout_seconds, ) messages = [m.model_dump() for m in payload.messages] chunks: list[str] = [] diff --git a/backend/requirements.lock b/backend/requirements.lock index 8093dfa..ca87e0e 100644 --- a/backend/requirements.lock +++ b/backend/requirements.lock @@ -19,10 +19,8 @@ annotated-doc==0.0.5 annotated-types==0.8.0 anyio==4.14.2 certifi==2026.7.22 -cffi==2.1.1 charset-normalizer==3.5.1 click==8.5.0 -cryptography==50.0.1 fastapi==0.141.1 greenlet==3.5.5 h11==0.16.0 @@ -33,16 +31,12 @@ idna==3.19 iniconfig==2.3.0 packaging==26.3 pluggy==1.6.0 -psycopg==3.3.5 -psycopg-binary==3.3.5 -pycparser==3.0 pydantic==2.13.5 pydantic_core==2.46.5 Pygments==2.21.0 pytest==9.1.1 python-dotenv==1.2.3 PyYAML==6.0.3 -quickjs==1.19.4 regex==2026.9.3 requests==2.34.2 SQLAlchemy==2.0.52 diff --git a/backend/tests/test_local_only_surface.py b/backend/tests/test_local_only_surface.py index 30f9113..239a3ad 100644 --- a/backend/tests/test_local_only_surface.py +++ b/backend/tests/test_local_only_surface.py @@ -179,3 +179,91 @@ def test_compose_publishes_to_loopback_only(): assert published, "no published ports found — has the file moved?" for mapping in published: assert mapping.startswith("127.0.0.1:"), mapping + + +# --- every provider factory must actually build --------------------------- + +def test_every_provider_factory_builds_from_a_real_settings_row(client): + """M2 shipped with a defect this test would have caught. + + `Settings.api_key_plain` was removed with the API key, but `memorybank`'s + two provider factories still read it. Nothing failed at import, and no test + noticed, because every memory test stubs those factories out — so the break + only appeared at runtime, in a background task, as a swallowed + `AttributeError` that silently stopped summaries and embeddings. + + Constructing each factory from a real row is the cheapest thing that would + have caught it, and it catches the same shape of mistake next time a + Settings column moves. + """ + from app import memorybank + + db = SessionLocal() + try: + settings = db.query(models.Settings).first() + settings.embedding_model = "nomic-embed-text:latest" + settings.summary_model = "" + db.commit() + + summary = memorybank.summary_provider(settings) + assert summary.model == settings.model # falls back to the narrator + assert summary.base_url == settings.endpoint_url.rstrip("/") + + embed = memorybank.embedding_provider(settings) + assert embed.model == "nomic-embed-text:latest" + + turn = OpenAICompatibleProvider( + settings.endpoint_url, settings.model, settings.api_mode, + settings.model_timeout_seconds, + ) + assert turn.read_timeout == settings.model_timeout_seconds + finally: + db.close() + + +def test_the_configured_timeout_reaches_every_generating_client(client, monkeypatch): + """The second defect this review caught. The setting was stored, validated + and exposed, and then not passed to the provider — so the turn engine kept + using the module default and "configurable" was a claim rather than a fact. + + Each generating path is driven for real and the constructed provider is + recorded. Embeddings are deliberately excluded: they are short, never + cold-load a large model, and keep their own shorter constant. + """ + from app import memorybank + from app.routers import chat as chat_router + from app.routers.adventures import turns as turns_router + + seen = [] + + class Recorder: + last_usage = None + + def __init__(self, endpoint_url, model, api_mode="chat", read_timeout=None): + seen.append(read_timeout) + + async def generate(self, *a, **k): + yield ("text", "narration") + + async def chat(self, *a, **k): + yield ("text", "reply") + + monkeypatch.setattr(turns_router, "OpenAICompatibleProvider", Recorder) + monkeypatch.setattr(chat_router, "OpenAICompatibleProvider", Recorder) + monkeypatch.setattr(memorybank, "OpenAICompatibleProvider", Recorder) + + assert client.put("/api/settings", json={"model_timeout_seconds": 777}).status_code == 200 + + adv = client.post("/api/adventures", json={"title": "T"}).json()["id"] + client.post(f"/api/adventures/{adv}/actions", json={"type": "story", "text": "hello"}) + assert seen and seen[-1] == 777, f"turn engine used {seen[-1]!r}" + + client.post("/api/chat/stream", json={"messages": [{"role": "user", "content": "hi"}]}) + assert seen[-1] == 777, f"chat used {seen[-1]!r}" + + db = SessionLocal() + try: + memorybank.summary_provider(db.query(models.Settings).first()) + finally: + db.close() + assert seen[-1] == 777, f"summarizer used {seen[-1]!r}" diff --git a/planning/reports/M2-BASELINE-REPORT.md b/planning/reports/M2-BASELINE-REPORT.md new file mode 100644 index 0000000..fce4124 --- /dev/null +++ b/planning/reports/M2-BASELINE-REPORT.md @@ -0,0 +1,868 @@ +# M2 — Baseline Report (evidence record) + +**Date:** 2026-09-02 +**Milestone:** M2, *Remove Hosted, Cloud, Scripting, and Unneeded Deployment Surface* +**Companion:** `planning/reports/M2-IMPLEMENTATION-REPORT.md` interprets this file. +Where the two disagree on a runtime or test fact, **this file is the record.** + +This is measurement, not commentary. Command output is quoted verbatim. +Anything inferred rather than observed is labelled **[inferred]**. + +Hostnames and LAN addresses are **placeholders** — `inference.lan`, +`192.168.0.50`, port `8443`. The real ones are in the workspace's untracked +notes, never in this repository. Packet counts, digests, timings and status +codes are exact. + +--- + +## 0. Test environment + +| | | +| --- | --- | +| Host | Ubuntu 24.04.4 LTS, x86-64, 4 cores, 15 GB RAM, **no GPU** | +| Python | 3.12.3 | +| Node / npm | 22.23.1 / 10.9.8 | +| Docker | 29.7.2 | +| Ollama (same-host) | `ollama/ollama:latest` in a container, models from a shared volume | +| Ollama (LAN) | 0.33.0 on a **second physical machine**, HTTPS, certificate from a private CA | +| Models | `qwen2.5:0.5b`, `qwen2.5:3b-instruct`, `nomic-embed-text` | +| Image under test | `m2-final`, built from the working tree by `docker build .` | + +> **Timings in this file are not benchmarks.** This host was shared with +> unrelated work during the review; `uptime` reported a 1-minute load average +> ranging from **1.79 to 74.08** across the session on four cores. Where a +> measurement was taken under load, it says so. Latency figures are recorded +> because they mattered to a *timeout* result, not as performance data. + +--- + +## 1. Repository and provenance + +```console +$ git rev-parse --abbrev-ref HEAD +m2-local-only-surface + +$ git log --format='%H %G? %s' -3 +8c65ae99deda49b22f415d5868987e00bbcb173c G M2: cut the hosted product away from the local one +1a28a9a708985e7c98dcbcb87c189e7582ec288d G Apply post-M1 corrections to the planning package +645f07f06d226e274c33d96e71d9c374413ef771 G Add the M1 implementation review report +``` + +All three commits verify (`%G? = G`). The repository signs every commit; nothing +in this milestone is unsigned. + +### Commit chain + +| Commit | Sig | Parent | Meaning | +| --- | --- | --- | --- | +| `8c65ae9` | G | `1a28a9a` | **The M2 commit.** One commit, no merges. | +| `1a28a9a` | G | `645f07f` | Post-M1 planning corrections — the M1 baseline this milestone started from | +| `7f182a8` | G | `717670a`, `d72f7c1` | the fork import, two parents | + +### Upstream ancestry + +```console +$ git cat-file -t d72f7c1bda0f34fccd84afb7a25c34eb01c901de +commit +$ git merge-base --is-ancestor d72f7c1bda0f34fccd84afb7a25c34eb01c901de HEAD && echo yes +yes +$ git rev-list --count d72f7c1bda0f34fccd84afb7a25c34eb01c901de +172 +$ git diff --stat d72f7c1bda0f34fccd84afb7a25c34eb01c901de HEAD -- LICENSE +(no output) +``` + +All 172 upstream commits reachable; `LICENSE` byte-identical to upstream; no +re-import or upstream substitution occurred. `PROVENANCE.md` and +`DEVELOPMENT.md` are present and were updated for M2. + +### Private data + +```console +$ grep -rn "\|\|" \ + --include=*.py --include=*.md --include=*.jsx --include=*.js \ + --include=*.yml --include=*.txt backend frontend *.md *.yml Dockerfile +(no output) +``` + +No private hostname, LAN address, certificate or credential is committed. + +### Working tree + +At the M2 commit the tree was clean. **This review then modified six files** +(§9), so `git status` is *not* clean as this report is written: + +```text + M backend/app/memorybank.py + M backend/app/models.py + M backend/app/routers/adventures/turns.py + M backend/app/routers/chat.py + M backend/requirements.lock + M backend/tests/test_local_only_surface.py +``` + +Every runtime result below was produced by an image built from that modified +tree, except where explicitly marked as pre-fix. + +--- + +## 2. Change inventory + +```console +$ git diff --shortstat 1a28a9a 8c65ae9 + 94 files changed, 1395 insertions(+), 6578 deletions(-) +``` + +**Added (3):** + +```text +backend/app/endpoints.py +backend/tests/test_endpoint_policy.py +backend/tests/test_local_only_surface.py +``` + +**Deleted (24):** + +```text +backend/app/accesslog.py backend/tests/test_accesslog.py +backend/app/analytics.py backend/tests/test_analytics.py +backend/app/cleanup.py backend/tests/test_guest_cleanup.py +backend/app/netguard.py backend/tests/test_netguard.py +backend/app/security.py backend/tests/test_ratelimit_hardening.py +backend/app/routers/analytics.py backend/tests/test_reasoning_param.py +backend/app/routers/auth.py +backend/app/routers/scripts.py frontend/src/pages/Analytics.jsx +backend/app/routers/adventures/scripts.py frontend/src/pages/Scripts.jsx +backend/app/scripting/__init__.py frontend/src/pages/ScriptEditor.jsx +backend/app/scripting/engine.py frontend/src/pages/Play/panels/ScriptsPanel.jsx +backend/app/scripting/pipeline.py frontend/src/pages/Play/drawers/StatusDrawer.jsx + render.yaml +``` + +**Largest modifications:** + +```text + +93 -37 backend/app/routers/settings.py + +54 -82 backend/app/providers/openai_compatible.py + +44 -166 backend/tests/test_chat.py + +38 -81 backend/app/routers/chat.py + +37 -222 backend/app/auth.py + +32 -69 backend/app/main.py + +31 -195 backend/app/limits.py + +28 -178 backend/app/models.py + +22 -43 backend/app/database.py + +19 -58 frontend/src/pages/Settings.jsx + +18 -97 backend/app/routers/adventures/turns.py + +15 -85 frontend/src/App.jsx +``` + +--- + +## 3. Surface reduction, measured + +### API routes (from the OpenAPI schema, not by grep) + +| Prefix | M1 | M2 | +| --- | ---: | ---: | +| `/api/adventures` | 25 | 21 | +| `/api/analytics` | 3 | **0** | +| `/api/auth` | 4 | **0** | +| `/api/scripts` | 5 | **0** | +| `/api/chat` | 2 | 2 | +| `/api/scenarios` | 5 | 5 | +| `/api/settings` | 2 | 2 | +| `/api/story-cards` | 4 | 4 | +| `/api/debug`, `/api/health` | 2 | 2 | +| **Total** | **52** | **36** | + +### Dependencies + +```console +$ diff <(M1 requirements.txt) <(M2 requirements.txt) | grep '^<' +< quickjs>=1.19 +< cryptography>=42 +< psycopg[binary]>=3.2 +``` + +Installed closure, measured by building a clean venv from `requirements.txt` + +`requirements-dev.txt`: + +| | M1 | M2 | +| --- | ---: | ---: | +| Python packages installed | 40 | **34** | +| Packages gone | — | `cffi`, `cryptography`, `psycopg`, `psycopg-binary`, `pycparser`, `quickjs` | +| npm runtime dependencies | 6 | **3** | +| npm packages installed (`npm ls --all`) | 53 | **32** | + +### Code size + +| | M1 | M2 | Δ | +| --- | ---: | ---: | ---: | +| `backend/app` Python lines | 14 298 | 11 632 | −2 666 | +| `frontend/src` JS/JSX lines | 6 523 | 5 239 | −1 284 | +| `frontend/src/pages` files | 21 | 16 | −5 | + +### Environment variables actually read (`os.environ`) + +```console +$ grep -rn "os.environ" backend/app/ +backend/app/database.py:18:_env_db_path = os.environ.get("AIDND_DB_PATH") +backend/app/main.py:30: for o in os.environ.get("AIDND_CORS_ORIGINS", "").split(",") +``` + +Two, down from ten. `AIDND_MULTI_USER`, `AIDND_SECRET_KEY`, `AIDND_COOKIE_SECURE`, +`AIDND_DEMO_API_KEY`, `AIDND_DEMO_ENDPOINT_URL`, `AIDND_DEMO_MODELS`, +`AIDND_DEMO_TURNS_PER_DAY`, `AIDND_POWER_USERS`, `AIDND_ANALYTICS_EMAILS`, +`AIDND_TRUSTED_PROXY_HOPS`, `AIDND_DATABASE_URL` and `DATABASE_URL` are no +longer read. Four of those names still appear in the tree **as prose in +comments** explaining what was removed; `grep` above shows no read. + +--- + +## 4. Test suite + +```console +$ cd backend && .venv/bin/python -m pytest tests/ -q +606 passed, 1 warning in 126.29s (0:02:06) +``` + +Zero failed, zero skipped, zero xfailed. The one warning is the inherited +Starlette/`httpx` deprecation notice, present since M1. + +### Accounting for every test + +Counted by diffing collected node IDs between `1a28a9a` (M1) and the working +tree, not by reading diffs: + +| | Count | +| --- | ---: | +| Distinct test functions, M1 | 610 | +| Distinct test functions, M2 | 561 | +| Node IDs gone | 88 | +| Node IDs new | 39 | +| Collected tests (with parametrisation), M1 | 648 | +| Collected tests, M2 | **606** | + +**Gone, by file:** + +| File | Gone | Disposition | +| --- | ---: | --- | +| `test_analytics.py` | 25 | whole file — subject removed | +| `test_guest_cleanup.py` | 14 | whole file — subject removed | +| `test_accesslog.py` | 12 | whole file — subject removed | +| `test_ratelimit_hardening.py` | 8 | whole file — subject removed | +| `test_chat.py` | 8 | demo-key pinning and the power-user gate | +| `test_reasoning_param.py` | 5 | whole file — subject removed | +| `test_netguard.py` | 5 | whole file — replaced by `test_endpoint_policy.py` | +| `test_prompt_caching.py` | 4 | OpenRouter upstream routing | +| `test_memory_rewrite.py` | 4 | 2 retired (Postgres DSN masking), **2 renamed** | +| `test_delete_state.py` | 1 | **renamed** | +| `test_branch_forking.py` | 1 | **renamed** | +| `test_branch_clause.py` | 1 | scripting history API | + +**Four of the 88 are renames with equivalent coverage**, so 84 tests were +genuinely retired: + +```text +test_branch_forking: test_switching_restores_the_script_and_world_state + -> test_switching_restores_the_state_a_branch_left_behind +test_delete_state: test_deleting_the_ai_turn_rewinds_the_script_state + -> test_deleting_the_ai_turn_rewinds_the_counter +test_memory_rewrite: test_an_owner_with_no_api_key_is_skipped + -> test_an_adventure_with_no_model_configured_is_skipped +test_memory_rewrite: test_an_api_key_on_the_command_line_covers_that_owner + -> test_a_model_on_the_command_line_covers_that_adventure +``` + +**New, by file:** `test_local_only_surface.py` 18, `test_endpoint_policy.py` +14, `test_chat.py` 3, plus the 4 renames. Collected with parametrisation: +`test_endpoint_policy.py` 31, `test_local_only_surface.py` 32. + +### Instrumentation conversion, not deletion + +Eight files used a QuickJS `output` hook (`state.gold += 10`) as deterministic +instrumentation for the **state snapshot and rollback machinery**, which M2 does +not touch. The counter moved to the world-state engine — the model emits a +` ```state ` delta block, the referee applies it — and the assertions are +unchanged in substance. Affected: `test_take_state`, `test_attempt_siblings`, +`test_branch_forking`, `test_bundle_v2`, `test_delete_state`, +`test_retry_variants`, `test_story_tree_baseline`, `test_turn_flow_integration`, +plus `test_state_revert` converted from `script_state`/`state_after` to +`world_state`/`world_state_after`. + +### Frontend and image + +```console +$ npm run lint # oxlint +exit 0 # 7 warnings, all pre-existing react/only-export-components +$ npm run build +dist/assets/index-C9v6AJ3R.js 395.41 kB │ gzip: 120.01 kB ✓ built in 496ms +$ docker build . +exit 0 +``` + +There are no automated frontend tests in the repository — none existed at M1 +either. + +Bundle size: **933.69 kB → 395.41 kB** (M1 → M2), from removing CodeMirror with +the script editor. + +--- + +## 5. Offline run — same-host Ollama + +**Topology.** `--internal` Docker network (no NAT, no external DNS). Ollama in +one container; the `m2-final` image in a second container sharing Ollama's +network namespace, so Ollama sits on the storyteller's own loopback. tcpdump ran +in that namespace for the whole session. + +### 5.1 Isolation, verified before any test + +```text + blocked 1.1.1.1:443 OSError + blocked 140.82.121.4:443 OSError + no resolution fonts.googleapis.com + no resolution fonts.gstatic.com + no resolution openaipublic.blob.core.windows.net + no resolution openrouter.ai + no resolution api.openai.com + no resolution github.com +``` + +### 5.2 Listeners + +```text + LISTEN 127.0.0.1:8000 <- the storyteller + LISTEN 127.0.0.11:42229 <- Docker's embedded DNS + LISTEN 127.0.0.1:43093 <- Ollama's model runner + LISTEN 127.0.0.1:43313 <- Ollama's model runner + LISTEN [::]:11434 <- Ollama itself +``` + +Nothing the storyteller owns is bound off loopback. + +### 5.3 Ollama-only settings and diagnostics + +```json +endpoint: http://127.0.0.1:11434/v1 | model: qwen2.5:0.5b +embed: nomic-embed-text:latest | timeout: 600 +connection test: {"ok": true, "models": ["qwen2.5:7b-instruct", + "qwen2.5:3b-instruct", "nomic-embed-text:latest", "qwen2.5:0.5b"]} +``` + +### 5.4 Story generation and streaming + +```text + turn 1 ( 1.7s, 90 stream chunks): 'Oh, how unfortunate! A lighthouse keeper indeed! The light' + turn 2 ( 35.2s, 90 stream chunks): 'I climb the spiral stair to the lamp room. My hand tightly' + turn 3 ( 7.1s, 90 stream chunks): "I search the keeper's log for the last entry. The last ent" + turn 4: ERROR {'type': 'error', 'detail': 'The AI endpoint timed out.'} +``` + +Turn 4 timed out at the configured 600 s. The application logged no error. Ollama +reported the model still resident. `uptime` at the time: 1-minute load average +had risen from 1.79 to the tens on four cores from unrelated work on this host. +**[inferred]** the timeout is host contention rather than an application fault; +what is *measured* is that no application error was logged and that the same +build produced turns in 1.7–35.2 s minutes earlier. + +### 5.5 Local embeddings, end to end through the app + +```text + wrote memory id=1, embedded=False + turn to trigger the pass (101.6s) + memories: 1 embedded with the local model: 1 +``` + +Direct timing of the embedding endpoint from inside the container: + +```text + embedding round trip: 3.4s, dim=768 + second embedding (warm): 0.1s +``` + +### 5.6 Branch-scoped memory isolation — positive and negative controls + +```text +main branch = 1 + memories on main: 0 ALPHA present: False +forking at AI action 22 (an earlier turn, so this starts a new line) + fork events: ['chunk', 'chunk', 'done'] + branches now: 2, fork = 5 + +ON THE FORK (0 memories) + ALPHA visible: False <- ALPHA is anchored at main's head, which is BELOW the + fork point, so the fork correctly does not inherit it + BETA written here, visible: True <- positive control + +BACK ON MAIN (1 memories) + ALPHA visible: True <- positive control + BETA visible: False <- NEGATIVE CONTROL, must be False +``` + +The decisive result is the last line: a memory written on the fork is **not** +visible on main. + +### 5.7 A05 — a failed model call, and a hand-edited database + +```text +baseline: (8 actions, 3 accepted AI turns, digest 07615da99b014b70) + +-- invalid local model -- + events: ['player', 'error'] + error: Endpoint or model not found (HTTP 404). Check the endpoint URL and that model 'n… + after: (7, 3, 'e18de9dff6455e0d') + +-- endpoint refused by policy at request time -- + (the settings row was edited directly with sqlite3, behind the app's back, + to https://openrouter.ai/api/v1) + events: ['player', 'error'] + error: This endpoint can't be used — openrouter.ai is a cloud inference service — this build… + after: (8, 3, '157c882c60579617') +``` + +The **accepted AI-turn count stayed at 3** through both failures. Only the +player's own typed action was added each time, which is M1's documented and +deliberate behaviour. + +The second case is the strongest form of the endpoint evidence: the request-time +check refused a cloud endpoint that had been written straight into SQLite, +bypassing the API's save-time validation entirely. + +### 5.8 Restart and resume + +```text +before restart: 8 actions, digest 157c882c60579617 + branches: 2 memories: 1 +after restart: 8 actions, digest 157c882c60579617 + branches: 2 memories: 1 + settings preserved: endpoint http://127.0.0.1:11434/v1 timeout 600 + context inspection: ['narrator', 'persona', 'history', 'used_memories'] +``` + +### 5.9 Packet capture — whole offline session + +```text +all packets: 5131 +loopback (127.0.0.0/8): 5086 +non-loopback unicast: 0 +TCP connections opened outside loopback: (none) +``` + +DNS queries, attributed by timestamp rather than assumed: + +| Name | First seen | Attribution | +| --- | --- | --- | +| `fonts.googleapis.com` | 16:44:02.800449 | **the isolation probe** in §5.1 | +| `openaipublic.blob.core.windows.net` | 16:44:02.802749 | the isolation probe | +| `openrouter.ai` | 16:44:02.803260 | the isolation probe | +| `api.openai.com` | 16:44:02.803956 | the isolation probe | +| `github.com` | 16:44:02.804747 | the isolation probe | +| `ollama-container` | 18:28:55.684172 | the endpoint-policy edge-case test (§7) | +| `ollama.com` | 16:53:29 … 18:28:05 | **the Ollama server's own lookup** | + +```text +capture window: 16:44:02.800136 .. 18:51:29.080336 +first packet to the storyteller API: 16:44:47.639083 +``` + +Every cloud/font/tokenizer lookup landed within **5 milliseconds of the capture +starting and 45 seconds before the storyteller received its first request** — +they are the deliberate probe, not the application. `ollama.com` recurs +throughout and is issued by the separately installed Ollama service, the same +observation M1 recorded. + +--- + +## 6. Offline run — trusted-LAN Ollama over HTTPS + +**Topology.** Ollama 0.33.0 on a **second physical machine** on the trusted LAN, +serving HTTPS with a certificate from a private CA. The storyteller ran in a +container with `NET_ADMIN`, its **default route deleted** and replaced with a +route to the LAN subnet only, its resolver pointed at nothing, and the host name +supplied as a static hosts entry. + +### 6.1 Isolation and binding + +```text +=== isolation === + blocked 1.1.1.1:443 OSError + blocked 140.82.121.4:443 OSError + no resolution github.com + no resolution openrouter.ai + no resolution fonts.gstatic.com +=== the approved LAN host === + inference.lan -> 192.168.0.50 + TLS verified, peer CN = inference.lan +=== listeners === + LISTEN 127.0.0.1:8000 +``` + +Nothing listens on the container's own LAN-facing address. + +### 6.2 Diagnostics and generation + +```text +endpoint: https://inference.lan:8443/v1 | timeout: 600 s +diagnostics: {"ok": true, "models": ["qwen2.5:3b-instruct", "nomic-embed-text:latest"]} +turn 1 ( 10.7s, 24 chunks): 'You check the oil and wick, their greasy slicks staining your hand' +turn 2 ( 4.0s, 17 chunks): 'The air is thick with the musty scent of the old building as you a' +turn 3 ( 4.6s, 26 chunks): 'You find the last entry: "Exhausted... Oil\'s low..." The room feel' + +memories: 1 embedded via the LAN host: 1 +``` + +### 6.3 Every model request the application made + +```text + 4 https://inference.lan:8443/v1/chat/completions + 1 https://inference.lan:8443/v1/embeddings +``` + +Narration and embeddings both went to the configured host over verified TLS. No +other URL appears. + +### 6.4 Retry keeps the discarded attempt + +Measured on the shipped image, against the LAN host: + +```text +head AI action before retry: 8 take_count 1 + retry produced (13.7s): 'You jot down a hurried note: "Visited lamp—oil low—oil lamp ' +head AI action after retry: 9 take_count 2 take_index 1 +alternate takes retained: 2 + - 'Your pen touches the cold wood of the log, the friction' + - 'You jot down a hurried note: "Visited lamp—oil low—oil ' +branches: 1 +``` + +A retry at the tip files the new attempt as a sibling at the same coordinate and +keeps the one it replaced. No branch is created, which is correct for a retry at +the head. + +### 6.5 Restart and resume + +```text +before restart: 8 actions, digest 66cc82944f8b6eef +after restart: 8 actions, digest 66cc82944f8b6eef + endpoint preserved: https://inference.lan:8443/v1 timeout 600 + memories: 1 +``` + +### 6.6 Packet capture + +```text +all packets: 685 +loopback: 364 +to/from the LAN Ollama host: 308 +any other unicast: 0 +DNS queries: none +``` + +Zero DNS: the endpoint was configured, not discovered. + +--- + +## 7. Endpoint policy + +Run inside the shipped image: + +```text + ALLOW same-host loopback http://127.0.0.1:11434/v1 + ALLOW localhost name http://localhost:11434/v1 + ALLOW IPv6 loopback http://[::1]:11434/v1 + ALLOW LAN literal http://192.168.1.50:11434/v1 + ALLOW link-local http://169.254.10.5:11434/v1 + ALLOW CGNAT / mesh VPN http://100.64.3.4:11434/v1 + ALLOW IPv6 unique-local http://[fd00::5]:11434/v1 + ALLOW docker internal name http://ollama-container:11434/v1 + REJECT cloud provider https://openrouter.ai/api/v1 + openrouter.ai is a cloud inference service — this build talks to Ollama on your own mach… + REJECT cloud provider https://api.openai.com/v1 + REJECT public IPv4 http://8.8.8.8:11434/v1 + 8.8.8.8 resolves to 8.8.8.8, which is a public Internet address — … + REJECT public IPv6 http://[2001:4860:4860::8888]/v1 + REJECT unspecified address http://0.0.0.0:11434/v1 + 0.0.0.0 … is not on this machine and not on your own network … + REJECT wrong scheme ftp://127.0.0.1/v1 +``` + +Over the HTTP API, from inside the offline container: + +```text +=== cloud and public endpoints are refused === + 400 https://openrouter.ai/api/v1 That endpoint can't be used — openrouter.ai is a cloud… + 400 https://api.openai.com/v1 … + 400 https://api.groq.com/openai/v1 … + 400 http://8.8.8.8:11434/v1 …8.8.8.8 … is a public Internet address… +=== local and LAN endpoints are accepted === + 200 http://127.0.0.1:11434/v1 + 200 http://192.168.1.50:11434/v1 + 200 http://[::1]:11434/v1 +``` + +Request-time enforcement against a hand-edited database is in §5.7. + +`test_endpoint_policy.py` (31 collected) resolves hostnames through a stub, so +it exercises the policy rather than the machine's DNS. It covers the split-horizon +case: a name resolving to both `192.168.1.50` and a public address is refused. + +--- + +## 8. TLS trust + +```console +$ grep -rn "verify=False\|ssl._create_unverified\|CERT_NONE\|check_hostname = False" backend/app +(the only match is prose in tlstrust.py saying such an option deliberately does not exist) + +$ grep -rn "httpx.AsyncClient(\|httpx.Client(" backend/app +backend/app/routers/settings.py:122 +backend/app/providers/openai_compatible.py:214 +backend/app/providers/openai_compatible.py:326 +backend/app/providers/openai_compatible.py:360 +``` + +Four clients, all four passing `verify=tlstrust.ssl_context()`. Enforced by +`test_tls_trust.py`, which walks the AST of the two modules that make outbound +requests and fails if any `httpx.AsyncClient` is constructed without `verify`, +or if a third module starts making requests: + +```console +$ .venv/bin/python -m pytest tests/test_tls_trust.py -q +6 passed in 0.20s +``` + +Runtime confirmation of certificate **and** hostname verification against the +real private-CA host is §6.1 (`TLS verified, peer CN = inference.lan`), and of +all four paths — settings/test, narration, model listing, embeddings — §6.2–6.3. + +### Modules that can open an outbound connection at all + +```console +$ grep -rln "httpx\|requests\.\|urllib.request\|socket\.\|aiohttp\|websocket" backend/app +backend/app/endpoints.py # getaddrinfo only, for the policy +backend/app/providers/openai_compatible.py +backend/app/routers/settings.py +backend/app/tlstrust.py # builds the SSL context; opens nothing +``` + +Remaining absolute URLs anywhere in backend application code: + +```text + 3 http://127.0.0.1 + 2 http://localhost + 1 https://openaipublic.blob.core.windows.net <- a comment and a SOURCE_URL + constant in encoding.py; never fetched +``` + +--- + +## 9. Defects found during this review + +Three, all found by running the shipped build rather than by reading it. Each was +corrected because the M2 evidence could not otherwise be accurate; each is +reported rather than absorbed. + +### 9.1 The memory bank was broken — summaries and embeddings silently stopped + +```text +Task exception was never retrieved +future: exception=AttributeError( + "'Settings' object has no attribute 'api_key_plain'")> + File "/app/backend/app/memorybank.py", line 155, in summary_provider + settings.api_key_plain, +``` + +M2 removed `Settings.api_key_plain` with the API key, but `memorybank`'s two +provider factories still read it. It failed in a fire-and-forget background +task, so nothing surfaced to the user and no test caught it: every memory test +stubs those factories out. **All 604 tests passed with this defect present.** + +Fixed by rewriting both factories. Covered by a new test that constructs every +provider factory from a real `Settings` row. + +### 9.2 The configurable model timeout never reached the turn engine + +`settings.model_timeout_seconds` was stored, validated, exposed in the API and +rendered in the UI — and not passed to `OpenAICompatibleProvider` in +`turns.py` or `chat.py`, so generation used the module default. The M2 exit +criterion "no longer an undocumented hardcoded limitation" was therefore only +half met: the constant had moved, but the setting was inert. + +Fixed in both call sites. Covered by a new test that drives the turn endpoint, +the chat endpoint and the summariser factory and asserts the configured value +reaches each. + +### 9.3 `requirements.lock` still pinned the removed packages + +```console +$ grep -n "quickjs\|psycopg\|cryptography" backend/requirements.lock +25:cryptography==50.0.1 +36:psycopg==3.3.5 +37:psycopg-binary==3.3.5 +45:quickjs==1.19.4 +``` + +`DEVELOPMENT.md` tells a new developer to install from the lock, which would have +reinstalled all three. Regenerated: 40 pins → 34. + +### Verification after the fixes + +```console +$ .venv/bin/python -m pytest tests/ -q +606 passed, 1 warning in 126.29s +$ npm run lint # exit 0 +$ npm run build # ✓ built in 496ms +$ docker build . # exit 0 +``` + +All §5 and §6 runtime evidence above was produced by an image built **after** +these fixes. + +--- + +## 10. M1 capability regression checks + +| M1 capability | Result | Evidence | +| --- | --- | --- | +| Vendored tokenizer, no first-turn download | PASS | §10.1 | +| Self-hosted fonts | PASS | §10.2 | +| Same-origin runtime assets | PASS | §10.2 | +| Restrictive CSP | PASS | §10.2 | +| Loopback storyteller default | PASS | §5.2, §6.1, §11 | +| Same-host Ollama | PASS | §5.3–5.4 | +| Trusted-LAN Ollama | PASS | §6.2 | +| HTTPS / private-CA trusted-LAN | PASS | §6.1–6.3 | +| Certificate + hostname verification | PASS | §6.1, §8 | +| Offline story generation | PASS | §5.1, §5.4 | +| SQLite persistence | PASS | §5.8, §6.4 | +| Full regression suite green | PASS | §4 | + +### 10.1 Tokenizer, inside the shipped image, offline + +```text + vendored table sha256 matches pin: True + count_tokens with sockets blocked: 6 tokens +``` + +### 10.2 Browser asset graph, fetched over loopback with no route out + +```text +CSP: default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; + font-src 'self'; img-src 'self' data:; connect-src 'self'; object-src 'none'; + base-uri 'none'; form-action 'self'; frame-ancestors 'none' + +referenced by index.html: + /favicon.svg LOCAL + /assets/index-*.js LOCAL + /assets/index-*.css LOCAL + +font urls inside the stylesheet: + /fonts/cinzel-normal-latin-ext.woff2 200 font/woff2 14540 + /fonts/cinzel-normal-latin.woff2 200 font/woff2 25904 + /fonts/crimson-pro-italic-latin-ext.woff2 200 font/woff2 39808 + /fonts/crimson-pro-italic-latin.woff2 200 font/woff2 51432 + /fonts/crimson-pro-normal-latin-ext.woff2 200 font/woff2 37988 + /fonts/crimson-pro-normal-latin.woff2 200 font/woff2 48200 + /fonts/inter-normal-latin-ext.woff2 200 font/woff2 85068 + /fonts/inter-normal-latin.woff2 200 font/woff2 48256 +``` + +Unchanged from M1, including the `object-src`/`base-uri`/`form-action` +directives M1 added. + +--- + +## 11. Storyteller network exposure + +| Launch path | Listener / publish | Verified by | +| --- | --- | --- | +| `./start.sh` (native dev) | `uvicorn --host 127.0.0.1 --port 8000` | file assertion in `test_local_only_surface.py` | +| `start.ps1` (Windows) | `--host 127.0.0.1` | same test | +| Production native (documented in `DEVELOPMENT.md`) | `--host 127.0.0.1` | documentation | +| `docker compose up` | `ports: - "127.0.0.1:8000:8000"` | test parses the published mappings and asserts every one starts `127.0.0.1:` | +| Container process itself | `--host 0.0.0.0` | deliberate; the only address a published port can reach | + +`DEVELOPMENT.md` states that publishing the port to `0.0.0.0` is a deliberate +decision this project's threat model does not cover. The `Dockerfile` carries the +same warning next to `EXPOSE`. + +Two supporting checks: + +* `AIDND_CORS_ORIGINS="*"` makes the application **refuse to start**: + `RuntimeError: AIDND_CORS_ORIGINS must not contain '*'…` +* An unknown `/api/...` path now returns **404** instead of the SPA's HTML with + status 200 (verified for ten removed endpoints, §12). + +--- + +## 12. Removed-surface verification, at runtime + +From inside the offline container: + +```text +=== removed surfaces answer 404 === + 404 GET /auth/me 404 GET /analytics/summary + 404 GET /auth/login 404 GET /analytics/collect + 404 GET /auth/register 404 GET /analytics/access + 404 GET /auth/logout 404 GET /scripts + 404 GET /adventures/2/scripts + 404 GET /adventures/2/script-state + +=== no API key in the settings surface === + api_key present: False has_api_key present: False + after trying to set one: False +``` + +```console +$ python -c "import app.scripting" +ImportError # asserted by test_the_application_has_no_scripting_engine +$ grep -rn "import quickjs" backend/app +(no output) +``` + +--- + +## 13. Inert schema retained for compatibility + +Verified present in the database and unread by the application. + +| Object | Was | Status | +| --- | --- | --- | +| table `scripts` | script library | unmapped; no model, no query | +| table `adventure_scripts` | per-adventure script copies | unmapped | +| table `analytics_daily` | visitor counters | unmapped | +| table `analytics_visitor_days` | visitor funnel | unmapped | +| table `access_log` | sign-ins, addresses, devices | unmapped | +| column `adventures.script_state` | scripting shared state | written `{}` only | +| column `actions.state_after` | scripting state per node | written `{}` only | +| column `settings.api_key` | encrypted cloud key | never read or written | +| column `settings.reasoning_max_tokens` | OpenRouter thinking budget | never read | +| column `users.demo_turns_used` / `_date` | demo cap tally | never read | +| table `users` + `user_id` FKs | multi-user ownership | **active**, one row, internal identity only | + +No destructive migration was performed. One additive migration was added: + +```python +(77, "ALTER TABLE settings ADD COLUMN model_timeout_seconds INTEGER NOT NULL DEFAULT 300") +``` + +Existing M1 databases open unchanged: the offline run in §5 used the volume +carrying campaigns created before these fixes, and §5.8 shows the transcript +digest surviving both an image replacement and a restart. + +--- + +## 14. What was not measured + +Stated so the implementation report does not overclaim. + +1. **No browser rendered the UI.** No browser automation was available. §10.2 + fetches the complete asset graph the page references, with correct media + types and the CSP, but nobody looked at the rendered page. This is unchanged + from M1, where the maintainer confirmed the render by hand. +2. **Sustained multi-turn play under the memory bank was not completed on this + host.** Turns 1–3 succeeded; turn 4 timed out under third-party CPU load + (§5.4). The individual capabilities — narration, streaming, embeddings, + memory writes, retry, fork, restart — were each measured separately. +3. **No load, soak or long-campaign testing.** Out of scope for M2. diff --git a/planning/reports/M2-IMPLEMENTATION-REPORT.md b/planning/reports/M2-IMPLEMENTATION-REPORT.md new file mode 100644 index 0000000..a8f96e3 --- /dev/null +++ b/planning/reports/M2-IMPLEMENTATION-REPORT.md @@ -0,0 +1,758 @@ +# M2 — Implementation Review Report + +**Date:** 2026-09-02 +**Milestone:** M2, *Remove Hosted, Cloud, Scripting, and Unneeded Deployment Surface* +**Audience:** the architecture/design reviewer deciding whether to accept M2 and start M3 +**Evidence:** `planning/reports/M2-BASELINE-REPORT.md`. Section references below +(§) point into it, and where this report and that one differ on a runtime or +test fact, **the baseline report is the record.** + +Hostnames and LAN addresses are placeholders (`inference.lan`, `192.168.0.50`). + +--- + +# A. Executive result + +```text +Overall M2 result: PASS +``` + +**Recommendation: ACCEPT M2 WITH NON-BLOCKING DEBT AND PROCEED TO M3.** + +Directly, in the order asked: + +| Question | Answer | +| --- | --- | +| Is it genuinely a single-user local storyteller? | **Yes.** No login, no accounts, no sessions; all four `/api/auth/*` routes 404 (§12). | +| Is Ollama the only production inference backend? | **Yes.** No cloud provider code, no key, and public addresses are refused at the wire (§7). | +| Are hosted/cloud/account/scripting surfaces actually removed? | **Yes** — removed, not hidden. 52 API routes → 36; `/api/auth`, `/api/analytics`, `/api/scripts` gone entirely (§3, §12). | +| Does same-host Ollama still work? | **Yes** (§5.3–5.4). | +| Does trusted-LAN Ollama still work? | **Yes**, against the real second machine (§6.2). | +| Does HTTPS/private-CA still work with verification on? | **Yes** — `TLS verified, peer CN = inference.lan`, all four clients on the shared trust context, no bypass exists (§6.1, §8). | +| Does it still run with Internet blocked? | **Yes** (§5.1, §6.1). | +| Did any M1 behaviour regress? | **Yes — two, both found by this review and both fixed** (§A.1). | +| Should the project proceed to M3? | **Yes.** | +| Blockers before M3? | **None.** | + +## A.1 Two M1 regressions that M2 shipped, and this review caught + +These are the most important findings and they are not buried. + +**1. The memory bank was silently broken.** M2 removed +`Settings.api_key_plain`, but `memorybank`'s two provider factories still read +it. Summaries and embeddings raised `AttributeError` inside a fire-and-forget +background task — no user-visible error, no log a player would see, no failing +test. **All 604 tests passed with the memory bank dead** (§9.1). + +**2. The configurable model timeout never reached the turn engine.** The +setting was stored, validated, exposed in the API and rendered in the UI, and +then not passed to the provider. M2's own exit criterion — "the model timeout is +no longer an undocumented hardcoded 120-second limitation" — was half met: the +constant had moved but the setting was inert (§9.2). + +A third, smaller: `requirements.lock` still pinned `quickjs`, `psycopg` and +`cryptography`, so the documented setup path would have reinstalled all three +(§9.3). + +All three are corrected in the working tree, with tests that would have caught +the first two. **The M2 commit `8c65ae9` does not contain these fixes**; the tree +is six files ahead of it and needs a follow-up commit. Every runtime result in +the baseline report was produced by an image built from the fixed tree. + +**What this says about the milestone is more useful than the defects +themselves:** a subtractive milestone's risk is not what it deletes, it is what +still reaches for the deleted thing from a code path no test exercises. Both +defects were in *background* or *plumbing* paths. §K returns to this. + +--- + +# B. Repository and provenance + +| | | +| --- | --- | +| Branch | `m2-local-only-surface` | +| HEAD | `8c65ae99deda49b22f415d5868987e00bbcb173c` (signed, `%G? = G`) | +| M2 commit | `8c65ae9`, one commit, parent `1a28a9a` | +| M1 starting point | `1a28a9a` (post-M1 planning corrections) | +| Upstream ancestry | `d72f7c1…` is an ancestor of HEAD; 172 upstream commits reachable | +| `LICENSE` | byte-identical to upstream | +| Private data committed | none (§1) | +| Working tree | **six files modified** by this review (§A.1) | + +No unrelated upstream re-import occurred. `PROVENANCE.md` records what M2 +removed and what it retained; `DEVELOPMENT.md` documents the new surface. + +--- + +# C. Implementation inventory + +**94 files changed, +1 395 −6 578.** Three added, twenty-four deleted, sixty-seven +modified. Grouped by purpose: + +### 1. Single-user / account removal +Deleted `routers/auth.py`, `cleanup.py` (guest-retention sweeper), +`accesslog.py`, `security.py` (session signing + key encryption). `auth.py` cut +from 259 to 74 lines: it now resolves one implicit local user and nothing else. +Frontend: the `/auth/me` bootstrap, `AuthModal`, guest nudge, log-in/sign-up/ +log-out controls, and the 401-and-retry dance in `api.js`. + +### 2. Analytics removal +Deleted `analytics.py`, `routers/analytics.py`, `pages/Analytics.jsx`, the +`trackPageview` beacon, the `ApiErrorMiddleware` that fed the error tally, the +lifespan flusher, and every `record_event` call site. Three model classes +unmapped. + +### 3. Hosted database / deployment removal +`render.yaml` deleted. `database.py` reduced to SQLite only — +`AIDND_DATABASE_URL`, `DATABASE_URL`, the psycopg URL normaliser and the +serverless pre-ping are gone. `psycopg[binary]` removed. + +### 4. Cloud-provider removal +`_OPENROUTER_HOST`, `_PREFERRED_UPSTREAM`, `_apply_provider_routing`, +`_apply_reasoning_budget`, the `Authorization` header, the `api_key` field and +its Fernet encryption. `cryptography` removed. + +### 5. Ollama-only configuration +`OpenAICompatibleProvider(endpoint_url, model, api_mode, read_timeout)` — no +key, no reasoning budget. `ProviderConfig`/`resolve_provider_config` deleted +outright; callers read `Settings` directly. + +### 6. Endpoint policy — **the only addition** +`backend/app/endpoints.py` (179 lines) replaces `netguard.py`, inverting its +rule (§F). + +### 7. Trusted-LAN / TLS preservation +M1's `tlstrust.py` untouched. All four HTTP clients still pass +`verify=tlstrust.ssl_context()`; `test_tls_trust.py` enforces it by AST walk. + +### 8. QuickJS / scripting removal +`app/scripting/` (3 files), both script routers, `Script`/`AdventureScript` +models, the `scenario_scripts` table, script schemas, script export/import, +`Scripts.jsx`, `ScriptEditor.jsx`, `ScriptsPanel.jsx`, `StatusDrawer.jsx`, +`ScriptReport`. `quickjs` removed; CodeMirror removed from npm. + +### 9. Hosted-policy removal +`limits.py` cut from 345 to ~150 lines: rate limiting, `X-Forwarded-For` client +IP, login throttling and per-user quotas gone. **Kept**: request body ceiling, +per-adventure row caps, import list caps. + +### 10. Settings simplification +Removed: API key field, "Remove key" button, demo banner, reasoning budget. +Added: Ollama endpoint help text stating the policy, and a model-timeout field. + +### 11. Model timeout +`CONNECT_TIMEOUT = 10`, `DEFAULT_READ_TIMEOUT = 300`, `EMBED_READ_TIMEOUT = 60`, +plus `Settings.model_timeout_seconds` (migration 77, default 300, bounded +30–3600). + +### 12. Loopback protections +`docker-compose.yml` publishes `127.0.0.1:8000:8000`; `--proxy-headers` dropped; +`AIDND_CORS_ORIGINS="*"` now refuses to start; an unknown `/api/...` path 404s +instead of returning the SPA with status 200. + +### 13. Tests +Two new files (63 collected). Eight files converted from JS instrumentation to +the world-state engine. Six files retired with their subsystems. + +### 14. Documentation +`DEVELOPMENT.md` gained the endpoint policy and the diagnostics table; +`.env.example` cut from 110 lines to 24; `PROVENANCE.md` records M2. + +## Deliberately retained + +| Retained | Why | +| --- | --- | +| `users` table and `user_id` foreign keys | The M2 brief permits it. Removing them means a migration across most of the schema to delete a column that costs nothing. One row; nothing creates a second; no request carries an identity. | +| Five inert tables, six inert columns | A destructive migration would risk an existing campaign database for tidiness. Unmapped or written-empty; nothing reads them (§13). | +| `providers/openai_compatible.py` name | It speaks OpenAI's *protocol* to Ollama. Renaming would churn a file M3 does not touch, for no behaviour change. | +| AI Chat scratchpad | Not hosted-only. It is a local tool for checking a model or prompt, and the endpoint policy constrains where it can talk. | +| RPG world-state engine | M5's scope. M2 additionally now *depends* on it as test instrumentation (§K.2). | +| Destructive Undo, no Redo | M3's scope, explicitly out of M2. | + +--- + +# D. Single-user / hosted-account removal + +| Classification | Contents | +| --- | --- | +| **Code removed** | `routers/auth.py`, `cleanup.py`, `accesslog.py`, `security.py`, 185 of 259 lines of `auth.py`, `AuthModal`, the account nav block, the session-retry logic | +| **Code retained but inert** | none in this area | +| **Schema retained for compatibility** | `users` + `user_id` FKs (active but single-row); `users.demo_turns_used`/`_date`; `settings.api_key` | +| **Production functionality still active** | **none** | + +* **Can a local user meet a login requirement?** No. There is no login UI, no + session cookie, and `get_current_user` always succeeds. +* **Are account APIs reachable?** No — 404 on all four (§12). +* **Are hosted account modules imported at runtime?** No; the files do not exist. +* **Does the database retain user identifiers?** Yes, one row. +* **Merely internal now?** Yes, and documented as such in `auth.py`'s docstring + and `PROVENANCE.md`. +* **Did avoiding the schema rewrite reduce risk?** **Materially.** `user_id` + appears on scenarios, adventures, settings and memories, and the ownership + filters run through the story-tree and memory queries M3 will modify. A + schema rewrite would have put a migration under those queries in the same + milestone that removed accounts — two risky changes entangled. Keeping the + column made M2 a deletion rather than a redesign. + +No active hosted-account functionality remains. + +--- + +# E. Analytics / telemetry + +**Removed:** the collection module, the routes, the beacon, the Visitors page and +nav link, the error-tally middleware, the batch flusher, and every call site. +**Retained inert:** three tables, unmapped, never opened. + +No destructive migration was performed because dropping tables from a live +campaign database buys nothing and can fail. + +**Verified at runtime, not asserted:** across a full offline session of 5 131 +packets — settings, campaign creation, four turns, embeddings, a fork, two +induced failures, a restart — there were **zero non-loopback unicast packets** +and no TCP connection opened outside loopback (§5.9). The only DNS names that +appear are the ones my own isolation probe deliberately looked up, all within +5 ms of the capture starting and 45 seconds before the storyteller received its +first request, plus `ollama.com` from the Ollama service itself. + +Analytics is removed, not merely hidden: collection does not occur. + +--- + +# F. Hosted database / deployment, and the endpoint policy + +## Database + +Postgres, Neon, `psycopg`, the URL normaliser and `render.yaml` are gone; +production is SQLite at `AIDND_DB_PATH`. An existing M1 database opens +unchanged — the offline run used a volume carrying campaigns created before the +fixes, survived an image replacement and a restart with digest +`157c882c60579617` unchanged (§5.8). Story-tree persistence is unaffected; +branch count, memory count and context inspection all survive restart. + +One Postgres-shaped abstraction is retained: `migrations.py` and `tools/dbmeter.py` +carry comments and a `_for_dialect` helper for SQL that differed between +SQLite and psycopg. Removing it would churn the migration history for no gain. + +## Endpoint policy — the design centre of M2 + +`endpoints.py` **inverts** the rule it replaces. `netguard.py` refused *private* +addresses, to stop a hosted visitor making the server fetch an internal service. +The threat here is the opposite: the user is trusted, and what must not happen is +the story reaching the public Internet. So the new rule refuses *public* +addresses. + +The line is drawn by **address against an explicit allowlist of networks**, not +by hostname and not by asking `ipaddress` what it thinks is private: + +```text +127.0.0.0/8 10.0.0.0/8 172.16.0.0/12 192.168.0.0/16 +169.254.0.0/16 100.64.0.0/10 ::1/128 fc00::/7 fe80::/10 +``` + +Every address a name resolves to must be in one. Applied twice: on save, for a +good error; and before every outbound request, because DNS moves. + +Measured behaviour (§7): loopback v4 and v6, `localhost`, all three RFC1918 +ranges, link-local, CGNAT, IPv6 ULA and a Docker-internal name are **accepted**; +four cloud providers, public IPv4 and IPv6, `0.0.0.0` and a wrong scheme are +**refused**, each with a reason naming what to do instead. + +### What it guarantees, and what it does not + +**Guarantees.** No request leaves for an address outside those networks, from any +of the four clients, whatever is stored. Demonstrated against a **hand-edited +SQLite row** — the settings row was rewritten to `openrouter.ai` with `sqlite3`, +bypassing the API entirely, and the next turn was refused at the wire (§5.7). +That is the strongest available form of this evidence. + +**Does not guarantee.** The allowlist is *address* scope, not *ownership* scope: +if a hostile host sits on the user's own LAN, the policy permits it — as it must, +since that is what "trusted LAN" means. A rebinding window exists in principle +between the policy's `getaddrinfo` and httpx's own connect; closing it would mean +pinning the resolved address into the connection, which is a larger change than +M2 warranted. Neither is a v1 concern: both require an attacker already inside +the trusted network. + +**One design finding worth flagging** (§K.1): writing this rule around +`ipaddress.is_private` / `is_reserved` — the obvious approach — is wrong in a way +that is easy to ship. Python classifies IPv6 loopback `::1` as *reserved*, so +that version refused `http://[::1]:11434/v1`, an ordinary same-host endpoint. It +also calls the documentation ranges *private*, so they would have been allowed. +The explicit CIDR list exists because of that, and the reasoning is recorded in +the module docstring. + +--- + +# G. Ollama-only provider + +**Providers in production code: one.** `OpenAICompatibleProvider`, speaking +OpenAI's `/v1` protocol to Ollama. + +* Provider options visible to the user: **none**. There is no selector, because + there is nothing to select between. +* OpenAI / OpenRouter / Groq / cloud APIs: **gone** — constants, routing, + attribution and the reasoning-budget parameter. +* Cloud API-key fields: **gone** from the model API, the UI and the request + headers. The column is inert. + +**The module name is not the question.** The question the brief poses — +*can production use still send story content to a public inference service +through normal configuration?* — was tested directly rather than reasoned about: +saving a cloud URL is refused with HTTP 400 (§7), and a cloud URL written +straight into the database is refused at request time (§5.7). No. + +--- + +# H. Trusted-LAN and TLS regression + +Re-verified after M2 on the **real second physical machine** used in M1, over +HTTPS with a private CA (§6). + +| Check | Result | +| --- | --- | +| Endpoint class | `https://inference.lan:8443/v1`, non-loopback, private CA | +| Connection | succeeded; diagnostics listed both installed models | +| Certificate validation | performed and passed | +| Hostname verification | performed — `peer CN = inference.lan` | +| CA trust | the machine's own CA store, unioned with certifi | +| Narration | 3 turns, 4.0–10.7 s | +| Model listing/discovery | via the same trust path | +| Embeddings | 1 memory embedded through the LAN host | +| Settings / Test Connection | same trust path | +| Retry | new take filed, both attempts retained (§6.4) | +| Restart and resume | digest `66cc82944f8b6eef` unchanged; endpoint and timeout preserved | +| Capture | 308 packets to the approved host, 364 loopback, **0 elsewhere, 0 DNS** | + +Every model request went to the configured host: 4 `chat/completions`, 1 +`embeddings`, nothing else. + +**No `verify=False`, insecure fallback or "ignore certificate errors" path +exists.** The only textual match for such a thing in the tree is prose in +`tlstrust.py` saying it deliberately does not exist. All four clients use the +same context, enforced by an AST-walking test that also fails if a *fifth* +client appears (§8). No client uses different trust behaviour. + +--- + +# I. QuickJS and scripting removal + +Removed: the QuickJS dependency, `app/scripting/` entire, both routers, the +`onInput`/`onModelContext`/`onOutput` hooks in the turn engine, the `Script` and +`AdventureScript` models, script schemas, script export/import (bundles that +carry `scripts` now import with the story intact and the scripts ignored), the +Scripts page, the script editor, the in-play Scripts panel, the script-state +drawer and the Insights script report. + +**Imported content cannot execute JavaScript.** There is no engine to execute it +in — `import app.scripting` raises `ImportError` and no module imports `quickjs` +(§12). A scenario or bundle carrying script source is data that nothing reads. + +## Test-coverage accounting + +This is where a subtractive milestone can hide damage behind a green number, so +it is spelled out. Of 88 node IDs that disappeared, **4 are renames** and 84 were +genuinely retired (§4). + +| Category | Count | Verdict | +| --- | ---: | --- | +| Whole files whose subject was removed (`analytics`, `guest_cleanup`, `accesslog`, `ratelimit_hardening`, `reasoning_param`) | 64 | Obsolete product behaviour intentionally deleted. No coverage lost — the behaviour is gone. | +| `test_netguard.py` | 5 | **Replaced**, not lost: `test_endpoint_policy.py` (31 collected) covers the inverted rule far more thoroughly. | +| `test_chat.py` demo-key pinning and power-user gate | 8 | Subject removed. The file keeps and extends what the page still does. | +| `test_prompt_caching.py` OpenRouter routing | 4 | Subject removed. The prompt-layout and usage tests, which are the file's point, are untouched. | +| `test_memory_rewrite.py` Postgres DSN masking | 2 | Subject removed with Postgres. | +| `test_branch_clause.py::test_user_scripts_are_handed_the_path` | 1 | **Equivalent coverage retained.** It asserted the scripting history API saw the branch path; the test directly above it asserts the same invariant one layer down on `history.story_actions`/`count`/`tail`, which is where the branch clause lives. | +| Instrumentation conversion, 8 files | 0 retired | **Coverage retained in full.** A JS counter was measuring the *state snapshot and rollback machinery*, which M2 does not touch. The counter moved to the world-state engine; the assertions are unchanged in substance. | + +**No meaningful coverage was lost.** One area gained a great deal: endpoint +policy went from 5 tests of the opposite rule to 31 of the current one. + +--- + +# J. Settings surface, and the model timeout + +## Settings after M2 + +**Remains:** Ollama endpoint (with help text stating the policy), narrator model, +embedding model, temperature, max output tokens, context budget, API mode, +narrator prompt, memory-bank capacity and top-k, **model timeout**, Test +connection, and the provider debug log. + +**Removed:** the API key field and its "Remove key" button, the demo-key banner, +the reasoning budget, and the "OpenAI-compatible" framing on the endpoint field. + +**Is it correct for v1?** Yes. Every field maps to something the product does, +and nothing offers a capability the product refuses. It is not *polished* — that +is M8 — but it is not misleading, which is what M2 asked for. + +One onboarding gap survives from M1 and is not new: `Settings.model` defaults to +`""`, so a fresh install cannot generate a turn until a model is named, and +nothing prompts. M2 improves the *diagnosis* — the connection test now warns when +the endpoint is reachable but has no model by the configured name — without +fixing the onboarding, which is M8's. + +## Model timeout + +| | Value | +| --- | --- | +| Connect timeout | 10 s, constant — a wrong address should fail fast | +| Generation/read timeout | `Settings.model_timeout_seconds`, default **300 s** | +| Allowed range | 30–3600, enforced by the schema (422 outside) | +| UI | a numeric field with help text about cold loads | +| Config source | database row, not an environment variable | +| Embeddings | separate 60 s constant — short calls that never cold-load | +| Connection test | separate 15 s constant — a listing, not a generation | + +**Demonstrated no longer a fixed 120 s constant:** `test_the_timeout_is_configurable` +sets 900 through the API and reads it back; +`test_the_configured_timeout_reaches_every_generating_client` drives the turn +endpoint, the chat endpoint and the summariser factory with a recording provider +and asserts the configured value reaches each; `test_an_unusable_timeout_is_refused` +rejects 0, −1, 29 and 3601. The offline run used 600 s and the LAN run used 600 s, +both read back after restart (§5.8, §6.5). + +The retained fixed timeouts are the three above, each named and each justified by +the shape of the call. + +--- + +# K. Important surprises + +Four. None invented for completeness. + +## K.1 An address policy written the obvious way is wrong + +Writing the endpoint rule around `ipaddress.is_private` / `is_global` / +`is_reserved` — which is what the inherited `netguard.py` did and what any +reviewer would expect — produces a rule that **refuses IPv6 loopback** +(`::1` is classified reserved) and **accepts the documentation ranges** (they +are classified private). The first was caught by a test asserting +`http://[::1]:11434/v1` is allowed. This is worth knowing beyond this project: +the standard library's classifications answer a different question than "is this +on my own network". + +## K.2 Removing scripting made the product depend on the world-state engine — in the tests + +Eight test files used a QuickJS counter as instrumentation for the state +rollback machinery. The natural replacement was the world-state engine, so those +tests now assert rollback *through* the RPG delta protocol. **M5 plans to replace +that protocol** with typed narrative-state events. M5 will therefore have to move +this instrumentation a second time. It is cheap — the counter is one schema entry +and one reply template in `tests/fakes.py` — but M5's brief should say so rather +than discover it. + +## K.3 The largest surviving subsystem is the one M2 could not touch + +`migrations.py` is **1 128 lines**, the biggest file in the backend, and M2 +reduced it by one line while adding one. It carries the full history of a schema +that M2 has now partly orphaned: migrations that create `scripts`, +`analytics_daily` and `access_log`, and backfills for `state_after`. This is +correct — migration history must not be rewritten — but a reviewer expecting the +"trust and maintenance surface" to have shrunk proportionally should know that a +tenth of the backend is history that only grows. + +## K.4 Two defects hid in exactly the places a subtractive milestone cannot see + +Both regressions (§A.1) were invisible to a 604-test green suite, for the same +reason: the code that still reached for a removed thing sat in a path no test +executed with real objects. The memory-bank factories are stubbed in every +memory test; the timeout argument was never asserted. **The lesson is a testing +one, not a coding one:** after removing an attribute, the cheapest useful test is +one that constructs each consumer from a real object, and after adding a setting, +one that proves it arrives. Both now exist. + +--- + +# L. Acceptance matrix + +| Requirement | Result | Evidence / notes | +| --- | --- | --- | +| Single-user, no login | PASS | §12 — four auth routes 404 | +| Hosted account/guest/demo removed | PASS | §C.1, §D | +| Hosted rate-limit policy removed | PASS | `limits.py` 345→~150 lines; resource bounds kept | +| Analytics/Visitors removed | PASS | §E | +| No telemetry generated | PASS | §5.9 — 0 non-loopback unicast in 5 131 packets | +| Render path removed | PASS | `render.yaml` deleted | +| Neon path removed | PASS | §F | +| Postgres runtime removed | PASS | `psycopg` gone from requirements and closure | +| SQLite M1 data still works | PASS | §5.8 — digest survives image replacement and restart | +| Ollama-only production provider | PASS | §G | +| Cloud API keys removed | PASS | §12 — absent from the API, unsettable | +| Public inference endpoints blocked | PASS | §7, §5.7 — including against a hand-edited DB | +| Loopback Ollama accepted | PASS | §7 — v4, v6 and `localhost` | +| Trusted-LAN Ollama accepted | PASS | §6.2 | +| TLS/private-CA support preserved | PASS | §6.1 | +| TLS verification preserved | PASS | §8 — no bypass exists; AST-enforced | +| QuickJS removed | PASS | §12 | +| Executable campaign scripting removed | PASS | §I | +| Settings narrowed to v1 model | PASS | §J | +| Connection diagnostics appropriate | PASS | five distinguished failure kinds + a model-not-found warning | +| Hardcoded 120 s limitation resolved | **PASS after the §9.2 fix** | Was PARTIAL as committed: the setting existed but was inert | +| Supported launch paths loopback-only | PASS | §11 — all four paths, test-enforced | +| Offline operation preserved | PASS | §5.1, §5.4 | +| Story/tree/retry preserved | PASS | §6.4 — `take_count 2`, both attempts retained | +| Context inspection preserved | PASS | §5.8 | +| Memory isolation preserved | **PASS after the §9.1 fix** | §5.6 — negative control holds. Was FAIL as committed: the bank was dead | +| No M3 work started | PASS | Undo still destructive, no Redo, no head cursor | + +Two rows are "PASS after the fix". As committed at `8c65ae9` they were **FAIL** +and **PARTIAL**. Both fixes are in the working tree; neither is a blocker, +because both are corrected and tested — but the commit itself does not meet the +criteria, which is why §N recommends a follow-up commit before M3 begins. + +--- + +# M. V1 acceptance-test mapping + +| ID | Result | Notes | +| --- | --- | --- | +| **A01** offline startup | PASS | §5.1, §5.4 | +| **A02** loopback default | PASS | §5.2, §6.1, §11 | +| **A03** no cloud API key | PASS | Stronger than at M1: the field no longer exists | +| **A04** restart persistence | PASS | §5.8, §6.5 | +| **A05** failed model call integrity | PASS | §5.7 — accepted AI turns held at 3 across two induced failures | +| **A06** trusted-LAN Ollama | PASS | §6 — real second machine, HTTPS, private CA | +| **H01** no unexpected outbound | PASS | §5.9, §6.6 | +| **H02** no telemetry | PASS | §E | +| **H03** no cloud provider required | **PASS, and now in its preferred form** | H03 says the *preferred* final v1 is "cloud provider controls are absent, not merely unused". M1 met the base condition; **M2 meets the preferred one.** | +| **H04** model output cannot execute shell/tools | PASS, strengthened | The only execution surface was QuickJS, now removed. No shell, MCP or tool framework exists. | +| **H10** local API/CORS behaviour | PASS, strengthened | `AIDND_CORS_ORIGINS="*"` refuses to start; an unknown `/api` path 404s instead of returning HTML 200 | +| **H11** no first-use runtime download | PASS | §10.1, §10.2 | + +Two acceptance tests deserve planning attention (§O): A05's wording, already +corrected post-M1, and H10, which M2 has now exceeded in a way the test does not +describe. + +--- + +# N. Security review of the simplified product + +## Resulting architecture + +```text +Browser (loopback only) + │ same-origin; CSP names no remote origin + ▼ +Storyteller — FastAPI + SPA, bound 127.0.0.1:8000, no authentication + │ + ├─► SQLite on the local filesystem (the only persistence) + │ + └─► exactly one approved Ollama endpoint (endpoints.py) + same-host loopback OR a user-named address on their own network + HTTP, or HTTPS verified against the machine's CA store +``` + +## Every remaining way story text could leave the machine + +Found by static search plus runtime confirmation, not by assuming dead code is +unreachable: + +| Path | Status | +| --- | --- | +| `providers/openai_compatible.py` — 3 clients | The intended egress. Policy-checked before every request, TLS-verified. | +| `routers/settings.py` — 1 client | Model listing. Same policy, same trust context. | +| `endpoints.py` — `getaddrinfo` | Resolves names for the policy. Opens no connection. | +| Analytics / telemetry | **Gone.** | +| Remote runtime assets | **Gone** since M1; re-verified (§10.2). | +| Cloud provider constants | **Gone.** | +| Hosted auth | **Gone.** | +| Arbitrary endpoint use | **Refused** by address (§7). | +| Scripting runtimes | **Gone.** | +| Shell / MCP / tool frameworks | Never existed. | +| Remote database | **Gone.** | + +Only four modules in `backend/app` can open an outbound connection at all, and +the only absolute remote URL left in backend code is a `SOURCE_URL` constant in +`encoding.py` documenting where the vendored tokenizer table came from — never +fetched (§8). + +## Residual risk, stated plainly + +1. The API is **unauthenticated by design**. Loopback binding is the whole + control. Publishing the port defeats it; the code refuses the easy mistakes + (wildcard CORS, the default compose mapping) and the documentation warns + about the deliberate one. +2. A hostile host **on the user's own LAN** is permitted by the endpoint policy, + because that is what trusted-LAN means. +3. The **Ollama service has its own network behaviour** (`ollama.com`), outside + this codebase and inside the user's trust boundary. Unchanged from M1 and + still worth settling before release. + +--- + +# O. Planning-document recommendations + +Reported, not applied. No planning document was edited by this task. + +### 1. +```text +Document: BUILD-MILESTONES.md, M5 +Recommended change: Note that eight test files now use the world-state delta + protocol as instrumentation for state rollback, and that + replacing that protocol means moving the instrumentation + (one schema entry and one helper in tests/fakes.py). +Why M2 evidence: §K.2 — the QuickJS counter those tests used was replaced + with the RPG engine, which M5 plans to replace in turn. +Urgency: later (before M5 is briefed) +``` + +### 2. +```text +Document: SECURITY-THREAT-MODEL.md +Recommended change: Record the endpoint policy as implemented — an explicit + allowlist of networks, applied on save and before every + request — together with what it does not guarantee: a + hostile host on the trusted LAN, and the rebinding window + between the policy's resolution and the connection. +Why M2 evidence: §F. The threat model predates the policy and describes the + inherited SSRF guard's opposite rule. +Urgency: before M3 +``` + +### 3. +```text +Document: V1-ACCEPTANCE-TESTS.md, H10 +Recommended change: H10 currently reads only "privileged local APIs do not + allow arbitrary wildcard cross-origin writes". M2 goes + further: a wildcard origin makes the app refuse to start, + and an unknown /api path 404s rather than returning the SPA + with status 200. State both as pass conditions. +Why M2 evidence: §11. Both were defects found and fixed during M2; without + a test that names them they can regress unnoticed. +Urgency: later +``` + +### 4. +```text +Document: TECHNICAL-DESIGN.md §5.1 +Recommended change: Mark items 3 and 4 of the hardening list resolved. Item 3 + (hosted/auth/analytics/Postgres/cloud/QuickJS) is done in + full; item 4 (endpoint validation reflecting this product's + threat model) is done by endpoints.py. +Why M2 evidence: §C, §F. The document currently marks both open — M1 left + them so. +Urgency: before M3 +``` + +### 5. +```text +Document: DECISIONS/ — a new ADR +Recommended change: Record the endpoint policy as a decision: address-based + allowlist rather than hostname matching, deny by default, + enforced at save and at request time, never traded against + TLS verification. Include the ipaddress-classification + finding as the reason the CIDRs are spelled out. +Why M2 evidence: §F, §K.1. This is a load-bearing security decision that + currently exists only as a module docstring. +Urgency: before M3 +``` + +### 6. +```text +Document: SPECIFICATION.md +Recommended change: None. M2 changed no product requirement; it removed + capability the specification never asked for. +Urgency: — +``` + +--- + +# P. Technical debt after M2 + +| Item | Classification | Note | +| --- | --- | --- | +| **The three §9 fixes are uncommitted** | **pre-release — do first** | Six files ahead of `8c65ae9`. Until committed, the branch's HEAD contains a dead memory bank. | +| `Settings.model` defaults to `""`; nothing prompts | M8 | Inherited from M1. Diagnosis improved, onboarding not. | +| Ollama's own `ollama.com` lookup | pre-release | Outside this codebase, inside the product's claim about itself. | +| Inert tables and columns | optional cleanup | Safe legacy remnants. A cleanup migration is cheap once the schema settles — after M3/M5, not before. | +| `migrations.py` at 1 128 lines | optional cleanup | Dead/inert history, not active behaviour (§K.3). | +| 4 pre-existing unused imports | optional cleanup | Present since M1; `pyflakes` is otherwise clean. | +| Unused `request: Request` parameters in three routers | optional cleanup | Left by removing the rate limiter. Harmless. | +| No frontend tests | M8 | None existed at M1 either. Lint and build only. | +| `docs/*.html` links Google Fonts | M8 or pre-release | Upstream's project site; not served by the app. | +| Rebinding window in the endpoint policy | optional | §F. Requires an attacker already on the trusted LAN. | + +**Nothing here is a blocker for M3** except committing the fixes, which is +housekeeping rather than engineering. + +--- + +# Q. M3 readiness + +M3 is the production history work: non-destructive Undo, Redo, active-head +movement, divergence, and active-head export/import. + +**Did M2 alter the chokepoints M3 will modify?** Barely, and helpfully: + +| M3 chokepoint | M2's effect | +| --- | --- | +| `attempts.py` — snapshot/restore/rollback | `ATTEMPT_KEYS` lost `"script"`; `restore_state` and `snapshot_outcome` no longer carry script state. **One less shared state to move.** | +| `tree.py` — placement, head, lineage | **Untouched.** | +| `context/lineage.py`, `context/history.py` | **Untouched.** | +| `routers/adventures/takes.py` — retry, takes, undo | Only the removal of `ScriptPipeline` arguments and the demo cap. `undo_turn` is byte-for-byte the inherited destructive implementation. | +| `routers/adventures/turns.py` | Hook calls removed, so `generate_turn` has 4 parameters instead of 5 and no longer branches on script stop. **Simpler to reason about.** | +| `bundle.py` — export/import | `scripts`/`scriptState` no longer exported; old bundles importing them are ignored gracefully. M3 adds the active-head field to a smaller format. | + +**Is the Phase 0B undo/redo spike still applicable?** Yes. It touched three +backend files — the tree, the attempt machinery and the takes router — and M2 +changed none of their logic. If anything the spike is easier to apply now: it no +longer has to carry `script_state` alongside `world_state` through every +rollback. + +**Did the removals simplify or complicate M3?** Simplified, in three concrete +ways: one shared state instead of two through the rollback paths; no demo-cap or +rate-limit preconditions wrapped around the turn and retry endpoints; and a +provider constructed from `Settings` directly rather than through a +`ProviderConfig` indirection. + +**Tests M3 should preserve or rewrite.** Preserve as the contract: +`test_story_tree_baseline`, `test_retry_variants`, `test_take_state`, +`test_take_parentage`, `test_attempt_siblings`, `test_branch_forking`, +`test_delete_state`, `test_state_revert`, `test_bundle_v2`. **Note that these now +carry the world-state instrumentation** (§K.2) — M3 must not mistake it for RPG +coverage. `test_state_revert` and `test_delete_state` assert *destructive* undo +semantics and will need rewriting when undo stops deleting; that is expected M3 +work, and those files should be rewritten rather than deleted. + +**Anything to fix before history changes?** Only the uncommitted fixes. Nothing +M2 discovered touches history semantics. + +**Should M3 proceed as planned?** **Yes, unchanged in scope.** Two additions to +its brief: it inherits a single shared state rather than two, and it must be told +that the state-rollback tests are instrumented through the world-state engine. + +--- + +# R. Final recommendation + +```text +Recommendation: ACCEPT M2 WITH NON-BLOCKING DEBT AND PROCEED TO M3 +``` + +**Reasons.** + +1. Every M2 requirement is met, and the ones that matter most were tested at + runtime rather than read: cloud endpoints refused against a hand-edited + database, trusted-LAN HTTPS working against a real second machine with + verification on, and captures showing zero packets outside loopback and the + approved host. +2. The product is materially simpler, not just smaller: 52 API routes → 36, ten + environment variables → two, 6 Python packages and 21 npm packages gone, + −2 666 backend lines, −1 284 frontend lines, and a 933 kB bundle down to + 395 kB. The trust surface shrank in the places that carry story data. +3. H03's *preferred* v1 condition — cloud controls absent rather than unused — + is now met, which M1 could not claim. +4. No M1 capability is regressed **in the working tree**. Two were regressed in + the commit; both are fixed and both now have the tests that would have caught + them. +5. M3's chokepoints are untouched or simplified, and the Phase 0B spike still + applies. + +**The one thing to do first** is commit the six-file fix (§A.1). Until then the +branch head contains a silently broken memory bank, and anyone building from +`8c65ae9` would inherit it. + +The three planning corrections marked *before M3* in §O — the threat model, the +technical-design hardening list, and a new ADR for the endpoint policy — are +documentation of decisions already made, not new work, and can be done alongside +the M3 brief.