Add the M1 implementation review report
This commit is contained in:
@@ -0,0 +1,995 @@
|
||||
# M1 — Implementation Review Report
|
||||
|
||||
**Date:** 2026-09-02
|
||||
**Milestone:** M1, *Establish Production Fork and Offline Baseline*
|
||||
**Audience:** the architecture/design reviewer deciding whether to prepare M2
|
||||
**Companion:** `planning/reports/M1-BASELINE-REPORT.md` holds the raw run logs
|
||||
and packet-capture output this report summarises. Where the two differ in
|
||||
detail, that one is the primary record.
|
||||
|
||||
Hostnames and LAN addresses are **placeholders** (`inference.lan`,
|
||||
`192.168.0.0/24`). The real ones are in the workspace's untracked notes, never
|
||||
in this repository. Packet counts, digests, timings and command output are
|
||||
verbatim.
|
||||
|
||||
---
|
||||
|
||||
# A. Executive Result
|
||||
|
||||
**Overall M1 result: PASS.**
|
||||
|
||||
- **Is the application a playable production baseline?** Yes. A campaign can be
|
||||
created, played, persisted, restarted and resumed from the inherited browser
|
||||
UI, with outbound Internet blocked. This was exercised end to end, not
|
||||
inferred: three separate runs, plus a by-hand browser check in which the
|
||||
campaign list rendered and a campaign was opened and read.
|
||||
- **Were both Ollama paths demonstrated?** Yes, both at runtime. Same-host
|
||||
loopback in an air-gapped container; trusted-LAN against Ollama 0.33.0 on a
|
||||
**second physical machine**, over TLS, with the storyteller's default route
|
||||
deleted so the LAN was reachable and the Internet was not.
|
||||
- **Proceed to M2?** Yes.
|
||||
- **Blockers before M2?** **None.** Section M lists debt and inherited
|
||||
behaviour, none of which blocks M2 and most of which M2 removes by design.
|
||||
|
||||
One qualification worth the reviewer's attention: M1 required a code change
|
||||
that was **not in its planned scope** — outbound TLS verification (§G, §J).
|
||||
Without it the trusted-LAN path did not work at all against a realistic host,
|
||||
so it was in M1's critical path even though the milestone text never mentions
|
||||
it.
|
||||
|
||||
---
|
||||
|
||||
# B. Repository / Provenance
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| Branch | `m1-production-baseline` |
|
||||
| HEAD | `c1a73b3d77e48491196e8887ee5abc2f818af05e` |
|
||||
| HEAD signature | good (`%G? = G`) |
|
||||
| Working tree | clean apart from one documentation correction, below |
|
||||
|
||||
### M1 commits
|
||||
|
||||
| Commit | Sig | Parents | Subject |
|
||||
| --- | --- | --- | --- |
|
||||
| `c1a73b3` | G | `7f182a8` | M1: make the first story turn work with no Internet |
|
||||
| `7f182a8` | G | `717670a`, `d72f7c1` | Fork AI-DnD at d72f7c1 as the production base |
|
||||
|
||||
`7f182a8` is the fork import: a merge with **two parents** — the planning
|
||||
package's own history (`717670a`) and upstream AI-DnD (`d72f7c1`). `c1a73b3`
|
||||
is all of the M1 work.
|
||||
|
||||
### Upstream
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| Project | AI-DnD, <https://github.com/parththakkar106/AI-DnD> |
|
||||
| Pinned commit | `d72f7c1bda0f34fccd84afb7a25c34eb01c901de` |
|
||||
| Subject | "Stop paying twice for a block a retry can still throw away" |
|
||||
| Position | tip of `upstream/main` when the fork was taken, 1 Sep 2026 |
|
||||
| Substituted? | **No.** The pinned commit was fetched and verified before use. |
|
||||
|
||||
### How provenance is preserved
|
||||
|
||||
Upstream history is *in* this repository rather than copied out of it, so the
|
||||
claim is checkable rather than asserted:
|
||||
|
||||
```console
|
||||
$ git cat-file -t d72f7c1bda0f34fccd84afb7a25c34eb01c901de
|
||||
commit
|
||||
$ git merge-base --is-ancestor d72f7c1bda0f34fccd84afb7a25c34eb01c901de HEAD && echo yes
|
||||
yes
|
||||
$ git rev-list --count d72f7c1bda0f34fccd84afb7a25c34eb01c901de
|
||||
172
|
||||
```
|
||||
|
||||
All 172 upstream commits are reachable, not a squashed snapshot. Upstream paths
|
||||
are unchanged (`backend/`, `frontend/`, `docs/`, …), so a later upstream commit
|
||||
can still be fetched and cherry-picked against matching files.
|
||||
|
||||
### Licence and provenance files
|
||||
|
||||
| File | State |
|
||||
| --- | --- |
|
||||
| `LICENSE` | **unmodified.** `git diff d72f7c1 HEAD -- LICENSE` is empty. MIT, © 2026 Parth Thakkar. |
|
||||
| `PROVENANCE.md` | **new.** Upstream commit, licence terms, re-verification commands, both vendored assets with sources and digests, and the full list of what M1 changed. |
|
||||
| `frontend/public/fonts/OFL-*.txt` | **new.** SIL OFL 1.1 text for each vendored family, shipped beside the fonts as the licence requires. |
|
||||
|
||||
### Final `git status`
|
||||
|
||||
Clean except for one file, which is a **documentation correction, not an
|
||||
implementation change**:
|
||||
|
||||
```text
|
||||
M planning/reports/M1-BASELINE-REPORT.md
|
||||
```
|
||||
|
||||
Signing the fork-import commit re-hashed it from `46d34dc` to `7f182a8`, and
|
||||
the baseline report's change table still cited the pre-signing hash. That one
|
||||
line now cites `7f182a8`. It is uncommitted and needs a signed commit; §N has
|
||||
the command. No other file differs from `c1a73b3`.
|
||||
|
||||
---
|
||||
|
||||
# C. What Changed
|
||||
|
||||
`c1a73b3` — **35 files changed, 102 102 insertions, 23 deletions.** The
|
||||
insertion count is dominated by two vendored assets: the tokenizer table
|
||||
(100 256 lines) and three OFL licence texts (279 lines). Excluding vendored
|
||||
data and the baseline report, M1 is **1 101 inserted lines against 23 deleted**
|
||||
— code, tests, generated CSS and documentation. Of those, roughly 390 are new
|
||||
application code and tests, 352 are documentation, and the rest is the
|
||||
generated font CSS, the lockfile and the vendoring script.
|
||||
|
||||
```text
|
||||
.gitattributes | 4 +
|
||||
.gitignore | 4 +
|
||||
DEVELOPMENT.md | 246 +
|
||||
Dockerfile | 5 +
|
||||
PROVENANCE.md | 106 +
|
||||
backend/app/context/builder.py | 9 +-
|
||||
backend/app/context/encoding.py | 90 +
|
||||
backend/app/context/vendor/cl100k_base.tiktoken | 100256 +++++++++++++++
|
||||
backend/app/main.py | 29 +-
|
||||
backend/app/providers/openai_compatible.py | 14 +-
|
||||
backend/app/routers/settings.py | 4 +-
|
||||
backend/app/tlstrust.py | 47 +
|
||||
backend/requirements.lock | 57 +
|
||||
backend/requirements.txt | 4 +
|
||||
backend/tests/test_offline_assets.py | 160 +
|
||||
backend/tests/test_tls_trust.py | 93 +
|
||||
docker-compose.yml | 7 +-
|
||||
frontend/index.html | 10 +-
|
||||
frontend/public/fonts/OFL-*.txt | 279 +
|
||||
frontend/public/fonts/*.woff2 | Bin 0 -> 351196 bytes
|
||||
frontend/src/index.css | 1 +
|
||||
frontend/src/styles/fonts.css | 90 +
|
||||
frontend/tools/vendor_fonts.py | 137 +
|
||||
planning/reports/M1-BASELINE-REPORT.md | 466 +
|
||||
start.ps1 | 2 +-
|
||||
start.sh | 5 +-
|
||||
```
|
||||
|
||||
**Nothing inherited was deleted.** The 23 deletions are lines replaced in
|
||||
place, not features removed. Removal is M2's job.
|
||||
|
||||
### Tokenizer
|
||||
|
||||
| File | Origin | What changed | Why M1 needed it |
|
||||
| --- | --- | --- | --- |
|
||||
| `backend/app/context/vendor/cl100k_base.tiktoken` | **new** (vendored data) | The `cl100k_base` BPE table, 1.7 MB, SHA-256 `223921b7…65b2a7`. | The download this replaces is what killed the first story turn offline. |
|
||||
| `backend/app/context/encoding.py` | **new** | Builds a `tiktoken.Encoding` from the vendored table, verifying its SHA-256 against the digest `tiktoken` itself pins for the source URL. | Removes the network from the code path entirely, rather than relying on a warm cache or an env var. |
|
||||
| `backend/app/context/builder.py` | **inherited, modified** | `_encoding()` delegates to the new module; its `functools.lru_cache` moves there (one cache instead of two). | Single call site; every turn goes through it. |
|
||||
|
||||
### Fonts and browser assets
|
||||
|
||||
| File | Origin | What changed | Why M1 needed it |
|
||||
| --- | --- | --- | --- |
|
||||
| `frontend/public/fonts/*.woff2` | **new** (vendored data) | Cinzel, Crimson Pro, Inter as variable fonts, Latin + Latin-Ext, 343 KiB total. | The SPA fetched these from Google on every page load. |
|
||||
| `frontend/public/fonts/OFL-*.txt` | **new** | SIL OFL 1.1 licence text per family. | Required by the OFL for redistribution. |
|
||||
| `frontend/src/styles/fonts.css` | **new, generated** | `@font-face` declarations pointing at `/fonts/…`. | Replaces the Google stylesheet. |
|
||||
| `frontend/tools/vendor_fonts.py` | **new** | Regenerates both of the above from the Google Fonts API. | Keeps the vendored bytes reproducible instead of opaque. |
|
||||
| `frontend/index.html` | **inherited, modified** | Two `preconnect` hints and the Google stylesheet `<link>` removed, replaced by a comment pointing at the script. | The actual remote-asset request. |
|
||||
| `frontend/src/index.css` | **inherited, modified** | One `@import` for `fonts.css`, first in a load-bearing cascade order. | Faces must be declared before `tokens.css` names the families. |
|
||||
| `.gitattributes` | **inherited, modified** | `*.woff2`/`*.woff` marked binary. | Prevents line-ending normalisation corrupting a font. |
|
||||
|
||||
### Security headers and listener
|
||||
|
||||
| File | Origin | What changed | Why M1 needed it |
|
||||
| --- | --- | --- | --- |
|
||||
| `backend/app/main.py` | **inherited, modified** | CSP: `fonts.googleapis.com` and `fonts.gstatic.com` dropped, `font-src 'self'` added, plus `object-src 'none'`, `base-uri 'none'`, `form-action 'self'`. Separately, `mimetypes.add_type("font/woff2", …)` so the fonts are served with their real type instead of `application/octet-stream`. | M1's "tighten the CSP so runtime assets are local". The policy now names no remote origin at all. |
|
||||
| `start.sh`, `start.ps1` | **inherited, modified** | `--host 127.0.0.1` stated explicitly rather than inherited from uvicorn's default. | A02 is a requirement, not a default worth inheriting silently. |
|
||||
| `docker-compose.yml` | **inherited, modified** | Publishes `127.0.0.1:8000:8000` instead of `8000:8000`. | `8000:8000` publishes on every host interface — the storyteller on the LAN, unauthenticated. |
|
||||
| `Dockerfile` | **inherited, modified** | Comment only, explaining why the in-container listener is `0.0.0.0` and that the port must be published to loopback. | The 0.0.0.0 bind reads like a contradiction of A02 without it. |
|
||||
|
||||
### Outbound TLS *(unplanned; see §G and §J)*
|
||||
|
||||
| File | Origin | What changed | Why M1 needed it |
|
||||
| --- | --- | --- | --- |
|
||||
| `backend/app/tlstrust.py` | **new** | One cached `SSLContext` unioning the platform CA store with certifi's bundle. | Without it the trusted-LAN path fails against any host with a locally-issued certificate. |
|
||||
| `backend/app/providers/openai_compatible.py` | **inherited, modified** | Three `httpx.AsyncClient(…)` calls take `verify=tlstrust.ssl_context()`. | Turns, completions and embeddings. |
|
||||
| `backend/app/routers/settings.py` | **inherited, modified** | The connection-test client takes the same context. | Otherwise **Test connection** disagrees with what a turn would do. |
|
||||
| `backend/requirements.txt` | **inherited, modified** | `certifi` declared. | It is now imported by name rather than arriving via httpx. |
|
||||
|
||||
### Environment, tests, documentation
|
||||
|
||||
| File | Origin | What changed | Why M1 needed it |
|
||||
| --- | --- | --- | --- |
|
||||
| `backend/requirements.lock` | **new** | The exact tested closure, 40 pins including transitive and dev dependencies. | M1's "reproducible dev/test environment". `requirements.txt` keeps the ranges. |
|
||||
| `backend/tests/test_offline_assets.py` | **new** | 10 tests: vendored-table integrity, tokenizer opens no socket, golden token counts, CSP names no remote origin, no remote URL in markup/CSS, every declared font file exists, built SPA clean. | Both fixed bugs were invisible on a machine that had been online once. |
|
||||
| `backend/tests/test_tls_trust.py` | **new** | 6 tests: verification not weakened, context cached, certifi roots survive the union, and an AST walk asserting every `httpx.AsyncClient` passes `verify=` — including that no third module starts making requests. | Guards the §G change in both directions. |
|
||||
| `DEVELOPMENT.md` | **new** | Setup, run modes, same-host and trusted-LAN Ollama (including the private-CA case), test commands, the offline re-verification procedure, and what M1 deliberately left alone. | M1's environment/configuration documentation. |
|
||||
| `.gitignore` | **inherited, modified** | `/phase0b/` ignored. | Phase 0B research scratch — virtualenvs, databases, downloaded models. |
|
||||
|
||||
---
|
||||
|
||||
# D. User-Visible M1 Capability
|
||||
|
||||
Everything below was done through the running application. The browser step was
|
||||
confirmed by hand by the maintainer: the campaign list rendered at
|
||||
`http://127.0.0.1:8000`, a campaign was clicked, and the adventure and its
|
||||
transcript displayed.
|
||||
|
||||
### What a user can do now
|
||||
|
||||
1. **Start the application locally** — `./start.sh` for development, or a built
|
||||
SPA served by FastAPI on `127.0.0.1:8000` for production. Both bind loopback.
|
||||
2. **Configure a model** — Settings → endpoint URL, model, API mode, output and
|
||||
context budgets. **Test connection** lists the endpoint's models.
|
||||
3. **Create a campaign** and give it a persona.
|
||||
4. **Generate narration** — Do / Say / Story / Continue, streamed over SSE.
|
||||
5. **Persist and resume** — campaigns survive a clean restart with transcript
|
||||
and head position intact; the adventures list reopens them.
|
||||
6. **Use same-host Ollama** — `http://127.0.0.1:11434/v1`, the default endpoint,
|
||||
with no Internet at any point.
|
||||
7. **Use trusted-LAN Ollama** — an explicitly configured endpoint on another
|
||||
machine, `http://…:11434/v1` or `https://…/v1` with a private CA, while the
|
||||
storyteller UI/API stays on loopback.
|
||||
8. **Recover from a failed model call** — a clear error, accepted history
|
||||
untouched, and play continues.
|
||||
|
||||
The inherited surface also still works and is reachable from the nav: Home,
|
||||
Adventures, Scenarios (with the stat-schema and NPC editors), Scripts (the
|
||||
CodeMirror JavaScript editor), Settings, plus AI Chat and the Visitors
|
||||
dashboard, which local installs always see.
|
||||
|
||||
### First-run step worth knowing
|
||||
|
||||
`Settings.model` defaults to `""`, so **a user must pick a model before the
|
||||
first turn**; the endpoint already defaults to `http://localhost:11434/v1`.
|
||||
Nothing tells them this on the way in. Cosmetic in M1, a real onboarding
|
||||
question for M8.
|
||||
|
||||
### Inherited limitations still visible, deferred by design
|
||||
|
||||
| What the user sees | Milestone that addresses it |
|
||||
| --- | --- |
|
||||
| **Undo deletes turns and there is no Redo.** Verified in code, not assumed: `POST /adventures/{id}/undo` deletes the trailing AI action and its player action and prunes covering memories; no redo endpoint or control exists anywhere in the backend or SPA. | M3 |
|
||||
| Account, hosted and cloud-provider surfaces exist in the tree; the endpoint field accepts any URL. | M2 |
|
||||
| RPG world-state machinery — stats, flags, milestones, cast — with relative-delta proposals. | M5 |
|
||||
| JavaScript campaign scripting, sandboxed in QuickJS. | M2 removes it |
|
||||
| The Visitors analytics dashboard (local counters, two SQLite tables, no outbound request). | M2 |
|
||||
| Memory bank and auto-summarisation are **off per adventure by default**, even when an embedding model is configured globally. | M6 |
|
||||
| No named Save Points; no imported knowledge; UI is still AI-DnD's. | M4, M7, M8 |
|
||||
|
||||
---
|
||||
|
||||
# E. Acceptance-Test Results
|
||||
|
||||
Every row is runtime behaviour observed in a running system. Where a check was
|
||||
source-level, the row says so and does not claim PASS on that basis.
|
||||
|
||||
| ID | Result | Procedure | Evidence | Caveat |
|
||||
| --- | --- | --- | --- | --- |
|
||||
| **A01** Start application offline | **PASS** | Run 1 (§F). `--internal` Docker network; app started; campaign created; six turns. | Isolation proven first — `1.1.1.1:443` → `Network is unreachable`, every name → `gaierror`. Six turns generated and persisted. Whole browser asset graph fetched over loopback, all 200. | The UI render was confirmed in a browser on the *native* run, which had Internet (§F). |
|
||||
| **A02** Storyteller loopback default | **PASS** | Listener enumerated from `/proc/net/tcp` in each container; `ss -ltnp` on the native run; a TCP connect to this host's LAN address. | Runs 1, 2, 3 all show `LISTEN 127.0.0.1:8000` and nothing else. Native run: `connect 192.168.0.10:8000 → Connection refused`. `docker-compose.yml` publishes `127.0.0.1:8000:8000`. | In Docker the *in-container* listener is `0.0.0.0`; loopback-only exposure comes from the published port. A hand-run `docker run -p 8000:8000` would defeat it — §M. |
|
||||
| **A03** No cloud API key | **PASS** | `api_key: ""` in every run; connection test, turns, summaries and embeddings all exercised. | `"api_key_set": false` in each run's settings dump; all operations succeeded. | — |
|
||||
| **A04** Campaign survives restart | **PASS** | Run 1: 6 turns → `docker restart` → re-read. Run 2: same, plus a further turn. | Run 1: 12 actions, head 27, identical transcript before and after. Run 2: digest `f24caf86a744ab36` identical across restart; endpoint, embedding model and memories preserved; next turn produced in 6.4 s. | — |
|
||||
| **A05** Failed model call does not corrupt story | **PASS** | Two induced failures (nonexistent model on a live endpoint; dead endpoint port) plus one natural timeout, then recovery. | Accepted-prefix digest `2ca6ab528178e44e` unchanged throughout; AI-action count stayed at 6 across both failures; recovery via `continue` produced turn 7 with the prefix still unchanged. | The player's own typed action **is** committed before the model call, so the row count grows. §H. |
|
||||
| **A06** Trusted-LAN Ollama inference | **PASS** | Run 2 (§F). Ollama 0.33.0 on a second physical machine over HTTPS; storyteller's default route deleted, LAN-only route added, resolver pointed at nothing, host supplied as a static hosts entry. | Model discovery returned both models; nine turns (4–14 s); memories embedded on the remote host; restart and resume; capture shows 893 packets to the approved host, 730 loopback, **0** elsewhere, **0** DNS queries. | Required the §G TLS fix first — the first attempt failed with `CERTIFICATE_VERIFY_FAILED`. |
|
||||
| **H01** No unexpected outbound connections | **PASS for the application** | `tcpdump -i any` inside each run's network namespace, for the whole run. | Run 1: 6 074 packets, 6 062 loopback, **0 non-loopback unicast**. Run 2: 1 633 packets, 730 loopback, 893 to the approved host, **0** other unicast, **0** DNS. | Ollama itself queried `ollama.com` in Run 1 — not the storyteller, and it failed. §K. |
|
||||
| **H02** No telemetry | **PASS** | The same captures, plus reading `analytics.py`. | No outbound destination in either capture. `analytics.py` writes two local SQLite tables, opens no socket, records no IP or user agent, and HMACs the user id. | The dashboard still exists in the UI. M2 removes it. |
|
||||
| **H03** No cloud provider required | **PASS as written** | Runs 1 and 2, with nothing cloud reachable. | Every operation succeeded with no cloud endpoint and no key. | The test's *preferred* final state — "controls are absent, not merely unused" — is **not** met. That is M2's scope by design, not an M1 gap. |
|
||||
| **H11** No first-use runtime asset download | **PASS** | Run 1 on a **fresh database and fresh container** with no route out: first turn generated, then the full asset graph fetched. | First turn succeeded where upstream raised `ConnectionError`. `index.html` references only same-origin URLs; all 8 woff2 files served locally as `font/woff2`; CSP names no remote origin. Independently: with sockets blocked, upstream's code path raises `AssertionError: socket opened` and the vendored path returns a token count. | Browser devtools were not inspected; the claim rests on the server-side asset graph and the CSP. |
|
||||
|
||||
**Not tested, and not claimed:** any acceptance test outside M1's scope (B, C,
|
||||
D, E, F, G, I, J, K series). No result above is inferred from source
|
||||
inspection alone.
|
||||
|
||||
---
|
||||
|
||||
# F. Offline and Network Evidence
|
||||
|
||||
### Run 1 — same-host Ollama, air-gapped
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| Topology | Ollama in one container; the production image in a second container **sharing Ollama's network namespace**, so Ollama is genuinely on the storyteller's loopback. Network is Docker `--internal`: no NAT, no external DNS. |
|
||||
| Storyteller listener | `127.0.0.1:8000` |
|
||||
| Ollama endpoint | `http://127.0.0.1:11434/v1` — same-host loopback |
|
||||
| Outbound Internet actually blocked? | **Yes**, proven before testing: `1.1.1.1:443` → `OSError: Network is unreachable`; `openaipublic.blob.core.windows.net`, `fonts.googleapis.com`, `fonts.gstatic.com`, `openrouter.ai`, `github.com` → `gaierror` |
|
||||
| Expected destinations observed | `127.0.0.1:8000` (API), `127.0.0.1:11434` (Ollama), `127.0.0.1:11499` (the deliberately dead port in A05), two ephemeral loopback ports (Ollama's model runner) |
|
||||
| Unexpected attempts | **None from the application.** 6 062 of 6 074 packets loopback; **0 non-loopback unicast**; the remaining 12 are received mDNS/ICMPv6 multicast from the bridge. |
|
||||
| DNS | Four queries, all `ollama.com`, all `ServFail` — issued by **the Ollama server**, which shares the namespace. Not the storyteller. §K. |
|
||||
| First-turn asset download | **None.** Fresh database, fresh container; the first turn narrated instead of raising `ConnectionError`. No font, tokenizer or script request appears in the capture. |
|
||||
|
||||
### Run 2 — trusted-LAN Ollama, Internet blocked
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| Topology | Ollama 0.33.0 on `inference.lan` (`192.168.0.50`), a **separate physical machine** on the trusted LAN, HTTPS, certificate from a local StartOS CA. Storyteller in a container on this host with `NET_ADMIN`: default route **deleted**, replaced by a route to `192.168.0.0/24` only; resolver pointed at nothing; the host supplied as a static hosts entry. |
|
||||
| Storyteller listener | `127.0.0.1:8000` — nothing on the container's own LAN-facing address |
|
||||
| Ollama endpoint | `https://inference.lan:8443/v1` — explicitly configured, non-loopback, TLS |
|
||||
| Outbound Internet actually blocked? | **Yes.** By IP: `1.1.1.1`, `140.82.121.4`, `104.16.0.1` → `Network is unreachable`. By name: `github.com`, `openrouter.ai`, `fonts.gstatic.com`, `openaipublic.blob.core.windows.net` → `gaierror`. The LAN host resolved and TLS-verified: `peer CN = inference.lan`. |
|
||||
| Expected destinations observed | `127.0.0.1:8000` (17 connections), `192.168.0.50:8443` (19 connections) |
|
||||
| Unexpected attempts | **None.** 1 633 packets: 730 loopback, 893 to the approved host, **0 other unicast**. |
|
||||
| DNS | **Zero queries of any kind** — the endpoint was configured, not resolved. |
|
||||
| First-turn asset download | **None.** No name resolves at all, and the turn succeeded. |
|
||||
|
||||
Confirmed by the application's own request log: every model request went to
|
||||
`…/v1/chat/completions` and `…/v1/embeddings` on the configured host, and no
|
||||
other URL — narrator, summariser and embedder alike.
|
||||
|
||||
### Run 3 — native, non-Docker
|
||||
|
||||
Production build from the venv, SPA served by FastAPI, Ollama on host loopback.
|
||||
`LISTEN 127.0.0.1:8000` and `127.0.0.1:11434`; the LAN address refuses
|
||||
connections. A campaign was created and played, and this is the instance the
|
||||
browser check used. **This run had Internet available** — its purpose was the
|
||||
native listener check and a real out-of-Docker run, not the offline proof.
|
||||
|
||||
---
|
||||
|
||||
# G. tiktoken, Fonts, CSP, and Runtime Asset Fixes
|
||||
|
||||
### The tokenizer download
|
||||
|
||||
**Cause.** `backend/app/context/builder.py` called
|
||||
`tiktoken.get_encoding("cl100k_base")`. That fetches the BPE table from
|
||||
`openaipublic.blob.core.windows.net` on first use and caches it under the
|
||||
system temp directory. `count_tokens` runs on **every** turn, for context
|
||||
budgeting. On a developer machine that had been online once the cache was warm
|
||||
and the download invisible; on an air-gapped install the first turn died with
|
||||
`ConnectionError` instead of narrating.
|
||||
|
||||
**Fix.** The table is vendored at
|
||||
`backend/app/context/vendor/cl100k_base.tiktoken`, and
|
||||
`backend/app/context/encoding.py` constructs the `Encoding` directly from it —
|
||||
the same merge table, pattern string and special tokens `tiktoken` uses.
|
||||
Nothing in the tokenizer path can reach the network: not a cache that happens
|
||||
to be warm, not an environment variable a deployment could forget.
|
||||
|
||||
Three things make this trustworthy rather than merely working:
|
||||
|
||||
- The file's SHA-256 is `223921b76ee99bde995b7ff738513eef100fb51d18c93597a113bcffe865b2a7`,
|
||||
**identical to the digest `tiktoken_ext/openai_public.py` pins for that URL**,
|
||||
and it is re-checked every time the encoding is built. A truncated checkout
|
||||
or a substituted table fails loudly instead of silently changing every token
|
||||
count the context budget derives from.
|
||||
- A test asserts that digest still appears in `tiktoken`'s own source, so a
|
||||
future upgrade pointing `cl100k_base` at a different table is caught.
|
||||
- The encoding was compared token-for-token against `tiktoken.get_encoding`
|
||||
across ASCII, accented text, CJK, emoji, CRLF and special-token literals.
|
||||
|
||||
**Proof the guard is real**, with `TIKTOKEN_CACHE_DIR` pointed at an empty
|
||||
directory and Python's socket functions replaced:
|
||||
|
||||
```text
|
||||
UPSTREAM PATH raises: AssertionError socket opened
|
||||
VENDORED PATH: 2 tokens, no socket opened
|
||||
```
|
||||
|
||||
### Remote fonts
|
||||
|
||||
**Behaviour.** `frontend/index.html` carried two `preconnect` hints and a
|
||||
stylesheet `<link>` to `fonts.googleapis.com` for Cinzel, Crimson Pro and
|
||||
Inter. Every page load fetched that stylesheet and then font files from
|
||||
`fonts.gstatic.com` — an Internet dependency at runtime, and a third party
|
||||
learning when the story is being read.
|
||||
|
||||
**Fix.** Self-hosted. `frontend/tools/vendor_fonts.py` downloads the same faces
|
||||
once at development time into `frontend/public/fonts/` and generates
|
||||
`frontend/src/styles/fonts.css`. Variable fonts and the Latin + Latin-Ext
|
||||
subsets: 8 files, 343 KiB, covering every weight the design uses. OFL text
|
||||
ships beside them. Greek, Cyrillic and Vietnamese subsets are deliberately not
|
||||
vendored; text in them falls back to the system stack.
|
||||
|
||||
### CSP
|
||||
|
||||
```diff
|
||||
- style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;
|
||||
- font-src https://fonts.gstatic.com;
|
||||
+ style-src 'self' 'unsafe-inline';
|
||||
+ font-src 'self';
|
||||
+ object-src 'none'; base-uri 'none'; form-action 'self';
|
||||
```
|
||||
|
||||
Final policy, as served:
|
||||
|
||||
```text
|
||||
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'
|
||||
```
|
||||
|
||||
No remote origin remains. `'unsafe-inline'` stays on `style-src` because React
|
||||
writes inline `style` attributes; it is deliberately absent from `script-src`.
|
||||
|
||||
Separately, `woff2` was being served as `application/octet-stream` because
|
||||
Python's mimetypes table has no entry for it on a slim Debian image. Browsers
|
||||
accept it anyway — a `@font-face src` carries its own `format()` hint — but
|
||||
`main.py` now registers the correct type.
|
||||
|
||||
### Outbound TLS *(unplanned — see §J)*
|
||||
|
||||
**Cause.** `httpx` verifies against the `certifi` bundle, which carries the
|
||||
public web's CAs and nothing else. A trusted-LAN Ollama frequently has no
|
||||
public certificate. Against a real StartOS-hosted Ollama the connection test
|
||||
returned:
|
||||
|
||||
```text
|
||||
{"ok": false, "detail": "Connection failed: [SSL: CERTIFICATE_VERIFY_FAILED]
|
||||
certificate verify failed: self-signed certificate in certificate chain"}
|
||||
```
|
||||
|
||||
while `curl` and the browser on the same machine accepted the identical
|
||||
endpoint, because the CA was installed in the **system** store. Measured:
|
||||
|
||||
```text
|
||||
certifi bundle (httpx default) FAIL SSLCertVerificationError
|
||||
system trust store OK peer CN=inference.lan
|
||||
```
|
||||
|
||||
**Fix.** `backend/app/tlstrust.py` builds one cached context that **unions**
|
||||
the platform CA store with certifi's bundle; all four outbound clients use it.
|
||||
A union rather than a swap on purpose: the platform store alone would be a
|
||||
behaviour *change*, and an image with an empty or stale system store would
|
||||
start failing on endpoints that previously worked. A union can only add trust
|
||||
the user already granted at the OS level.
|
||||
|
||||
Verification is untouched — `verify_mode=CERT_REQUIRED`, `check_hostname=True`,
|
||||
and **no "insecure" escape hatch was added**. After the change:
|
||||
|
||||
```text
|
||||
OK inference.lan (local CA) CN=inference.lan
|
||||
OK github.com (public CA) CN=github.com
|
||||
OK pypi.org (public CA) CN=pypi.org
|
||||
certifi-only context still rejects inference.lan (so the union is what changed)
|
||||
```
|
||||
|
||||
### Fresh-data offline first-turn test
|
||||
|
||||
**Passed.** Run 1 used a fresh Docker volume and a fresh container on a network
|
||||
with no route out and no external DNS. The first turn of a newly created
|
||||
campaign generated and persisted. No tokenizer, font or other runtime asset
|
||||
request appears in the 6 074-packet capture.
|
||||
|
||||
---
|
||||
|
||||
# H. Persistence and Failure Recovery
|
||||
|
||||
### Clean restart
|
||||
|
||||
| Run | Before | After |
|
||||
| --- | --- | --- |
|
||||
| 1 (same-host) | 12 actions, head 27 | 12 actions, head 27, transcript identical line for line |
|
||||
| 2 (trusted-LAN) | 18 actions, head 18, digest `f24caf86a744ab36` | 18 actions, head 18, digest `f24caf86a744ab36` |
|
||||
|
||||
Run 2 additionally confirmed the endpoint (`https://inference.lan:8443/v1`),
|
||||
the embedding model and all memories survived, and produced a further turn in
|
||||
6.4 s afterwards.
|
||||
|
||||
### Failed model request
|
||||
|
||||
Baseline: 12 actions, 6 of them AI, accepted digest `2ca6ab528178e44e`.
|
||||
|
||||
```text
|
||||
invalid local model -> events ['player','error']
|
||||
"Endpoint or model not found (HTTP 404) … model
|
||||
'no-such-model-v9' not found"
|
||||
13 actions, still 6 AI actions
|
||||
endpoint down (:11499) -> events ['player','error']
|
||||
"Could not connect to http://127.0.0.1:11499/v1 —
|
||||
is the AI server running?"
|
||||
14 actions, still 6 AI actions
|
||||
accepted prefix through the pre-failure head:
|
||||
12 actions, digest 2ca6ab528178e44e — unchanged
|
||||
rows added by the failures:
|
||||
[28] story 'I strike a match.'
|
||||
[29] story 'I strike a match again.'
|
||||
recovery (continue) -> 40 SSE events, done; 15 actions, 7 AI actions
|
||||
prefix digest still 2ca6ab528178e44e
|
||||
```
|
||||
|
||||
**No partially accepted turn, in either failure.** The AI-action count never
|
||||
moved; the accepted prefix is bit-identical before, during and after.
|
||||
|
||||
**One behaviour the reviewer should know.** The player's own typed action is
|
||||
committed *before* the model is called (`run_player_turn`), so a failed turn
|
||||
leaves the player's text at the head with no reply, and the row count grows.
|
||||
That satisfies A05 as written — prior story intact, failed turn not committed
|
||||
as accepted, retry available — and it is deliberate: a model failure never eats
|
||||
what the player typed. It is stated because "the story is unchanged" is not
|
||||
literally true; "the accepted story is unchanged" is. Whether a dangling player
|
||||
action is the right *user-facing* recovery state is an M3 question.
|
||||
|
||||
---
|
||||
|
||||
# I. Test Suite / Regression Baseline
|
||||
|
||||
| Check | Command | Result |
|
||||
| --- | --- | --- |
|
||||
| Backend, before M1 | `python -m pytest tests/ -q` on the pinned upstream commit | **632 passed, 0 failed**, 184.7 s |
|
||||
| Backend, after M1 | `python -m pytest tests/ -q` | **648 passed, 0 failed, 0 skipped**, 212.1 s |
|
||||
| Backend, **with no Internet** | same suite in a container on an `--internal` network | **648 passed, 0 failed**, 253.6 s |
|
||||
| Frontend lint | `npm run lint` (oxlint) | **exit 0**, 6 warnings, 0 errors |
|
||||
| Frontend build | `npm run build` (vite 8.1.3) | **succeeds**, 1.13 s |
|
||||
| Image | `docker build .` | **succeeds** |
|
||||
| Typecheck | — | none exists; the SPA is plain JSX with no TypeScript config. |
|
||||
|
||||
The 632-test baseline was captured on the pinned upstream commit *before* any
|
||||
change and matches the Phase 0B figure independently.
|
||||
|
||||
### New M1 tests — 16
|
||||
|
||||
**`backend/tests/test_offline_assets.py` (10)** — vendored table present and
|
||||
intact; the pinned digest still matches `tiktoken`'s own; token counting opens
|
||||
no socket (sockets monkeypatched to raise); golden token counts; round-trip
|
||||
over awkward characters; CSP names no remote origin; `index.html` fetches
|
||||
nothing remote; stylesheets fetch nothing remote; every declared font file
|
||||
exists; the built SPA is clean.
|
||||
|
||||
**`backend/tests/test_tls_trust.py` (6)** — verification not weakened; context
|
||||
cached; every certifi root survives the union; and an **AST walk** over the two
|
||||
modules that make outbound requests asserting each `httpx.AsyncClient` passes
|
||||
`verify=`, plus that no third module has started making requests. That last one
|
||||
catches the failure no runtime test would: a *new* client added later, which
|
||||
would work perfectly until someone pointed it at a LAN endpoint.
|
||||
|
||||
### Remaining failures
|
||||
|
||||
**None.** No failing test, no skipped test, no documented exception, and **no
|
||||
M1 regression**. Nothing was inherited red — the 632-test baseline was green on
|
||||
the first attempt on the pinned commit.
|
||||
|
||||
Two `dist`-reading tests in `test_offline_assets.py` skip when
|
||||
`frontend/dist/` has not been built. In the runs above it had been, so they
|
||||
executed and are counted in the 648.
|
||||
|
||||
### Internet requirement
|
||||
|
||||
**No test requires Internet access** — confirmed by running the entire suite in
|
||||
a container with no route out and no DNS. All 648 passed. Two pre-existing
|
||||
warnings persist: a Starlette `httpx` deprecation notice, and a `SyntaxWarning`
|
||||
for an invalid escape sequence in `tools/rewrite_memories.py`, both inherited
|
||||
and both untouched by M1.
|
||||
|
||||
---
|
||||
|
||||
# J. Deviations From the M1 Plan
|
||||
|
||||
### Added scope
|
||||
|
||||
1. **Outbound TLS trust (`backend/app/tlstrust.py`) — the significant one.**
|
||||
Not in M1's scope list. `BUILD-MILESTONES.md` places "explicit Ollama
|
||||
endpoint policy" in M2. But M1's own scope requires *verifying* trusted-LAN
|
||||
generation, and its Definition of Done requires turns through such a host.
|
||||
Against a realistic LAN Ollama — TLS with a locally-issued certificate — that
|
||||
was impossible without this change. The choice was to change it or to report
|
||||
A06 as blocked. It is small (one module, four call sites), does not weaken
|
||||
verification, and adds no configuration surface.
|
||||
|
||||
2. **`woff2` media type** (one line in `main.py`). Found while verifying the
|
||||
self-hosted fonts. Cosmetic, but wrong is wrong.
|
||||
|
||||
3. **`backend/requirements.lock`.** M1 says "reproducible dev/test
|
||||
environment"; the inherited `requirements.txt` uses `>=` throughout, so a
|
||||
fresh checkout resolved to whatever was newest that day. The lock is the
|
||||
concrete deliverable for that line.
|
||||
|
||||
4. **`base-uri`, `object-src`, `form-action` in the CSP.** M1 says "tighten the
|
||||
CSP as needed". Removing the two Google hosts was the requirement; these
|
||||
three cost nothing and were added while the policy was open.
|
||||
|
||||
5. **`/phase0b/` in `.gitignore`** — housekeeping, so `git status` is readable.
|
||||
|
||||
### Omitted scope
|
||||
|
||||
**None.** Every item in M1's scope list was implemented and demonstrated.
|
||||
|
||||
### Changed assumptions
|
||||
|
||||
1. **"Trusted-LAN" was assumed to mean plain HTTP.** Every planning reference
|
||||
writes `http://…:11434/v1`, and Ollama's own default is cleartext. The real
|
||||
LAN host serves **HTTPS only** (plain HTTP 307-redirects), because it is a
|
||||
StartOS server. This is what surfaced the TLS defect. §L.
|
||||
|
||||
2. **A06 was initially reproduced without physical separation.** Before a
|
||||
second machine was available, Run 2 used a separate container, namespace and
|
||||
IP, and was going to be reported as PASS-with-qualification. A real second
|
||||
machine then became available and the run was redone properly — which is
|
||||
what exposed the TLS defect the container stand-in had hidden, since a
|
||||
container endpoint was plain HTTP.
|
||||
|
||||
### Workarounds
|
||||
|
||||
1. **Blocking Internet without root.** No passwordless sudo here, so host
|
||||
firewall rules were unavailable. Run 1 used a Docker `--internal` network.
|
||||
Run 2 used `NET_ADMIN` inside the container to delete its default route and
|
||||
add a LAN-only route — stronger than a firewall rule, since there is no
|
||||
route to drop packets on.
|
||||
|
||||
2. **Packet capture without root.** `tcpdump` in a container joined to the
|
||||
target namespace with `NET_RAW`.
|
||||
|
||||
3. **Browser verification by hand.** No browser automation in the session; the
|
||||
maintainer confirmed the render directly. Devtools were not inspected. §K.
|
||||
|
||||
### Unexpected inherited behaviour
|
||||
|
||||
Four, all in §K: the model timeout, Ollama's own DNS lookup, the per-adventure
|
||||
memory-bank defaults, and the pre-model commit of the player's action.
|
||||
|
||||
---
|
||||
|
||||
# K. Technical Findings / Surprises
|
||||
|
||||
### Affecting local-only security
|
||||
|
||||
1. **Ollama phones home.** In Run 1 the capture shows four DNS queries for
|
||||
`ollama.com`, issued by the **Ollama server**, not the storyteller. They
|
||||
failed and nothing depended on them. But it sits inside the user's trust
|
||||
boundary and outside this application's code, and a user reading a packet
|
||||
capture will see it. The local-only claim covers what *this* application
|
||||
sends; that distinction should be stated in the release material rather than
|
||||
discovered. Suppressing it is an Ollama configuration question, deliberately
|
||||
not investigated here.
|
||||
|
||||
2. **Loopback in Docker depends on the publish flag, not the bind.** The
|
||||
in-container listener is `0.0.0.0`, which is the only address a published
|
||||
port can reach. `docker-compose.yml` publishes `127.0.0.1:8000:8000`, so the
|
||||
shipped path is safe — but a hand-run `docker run -p 8000:8000` puts an
|
||||
unauthenticated storyteller on the LAN. Comments now say so in both files.
|
||||
Whether the app should *refuse* to serve on a non-loopback bind without an
|
||||
explicit opt-in is an M2 policy question.
|
||||
|
||||
### Affecting trusted-LAN inference
|
||||
|
||||
3. **The TLS defect (§G) is the most important finding in M1.** Beyond the fix,
|
||||
the lesson is methodological: it was undetectable by static review and
|
||||
undetectable by every local run that used loopback HTTP, because
|
||||
certificate verification never happens there. It surfaced within minutes of
|
||||
pointing the application at a real host. **M2's endpoint-policy work should
|
||||
be validated against a real LAN host, not a container stand-in.**
|
||||
|
||||
4. **A CA in a container is not the CA on the host.** Running against a
|
||||
private-CA endpoint from Docker requires the CA inside the image or
|
||||
bind-mounted. Documented in `DEVELOPMENT.md`; it will matter for any future
|
||||
packaged distribution.
|
||||
|
||||
5. **The model timeout is 120 s and hardcoded** (`httpx.Timeout(120,
|
||||
connect=10)`). On this GPU-less four-core host a *cold* model load, or two
|
||||
models contending after the memory bank pulls in the embedder, exceeded it
|
||||
three times. Once warm, a full turn took 6–9 s. Presented as a tuning
|
||||
finding rather than a defect — but a first-run user on modest hardware will
|
||||
likely meet it, and a configurable timeout is a small change.
|
||||
|
||||
### Affecting dependency packaging
|
||||
|
||||
6. **The vendored tokenizer table is 100 256 lines / 1.7 MB.** It dominates the
|
||||
M1 diff. The alternative — `TIKTOKEN_CACHE_DIR` — was rejected as a
|
||||
deployment-time promise that a packaging step could forget. The digest check
|
||||
makes the vendored copy auditable.
|
||||
|
||||
7. **`certifi` is now a direct dependency**, declared in `requirements.txt`
|
||||
rather than arriving via httpx.
|
||||
|
||||
8. **Node version.** The Dockerfile builds on Node 24 with a comment saying
|
||||
npm 10 refuses the lockfile; `npm ci` in fact succeeded on Node 22.23.1 /
|
||||
npm 10.9.8. No `.nvmrc` was added, because asserting a constraint that was
|
||||
not tested would be worse than documenting what was.
|
||||
|
||||
### Affecting browser/runtime assets
|
||||
|
||||
9. **`docs/*.html` still links Google Fonts.** Upstream's GitHub Pages project
|
||||
site — not served by the application, not part of any build, not covered by
|
||||
the runtime rule. Deliberately untouched; the regression tests scope
|
||||
themselves to `frontend/` so they do not give a false signal about it.
|
||||
|
||||
10. **Only devtools were left unchecked.** The page's whole asset graph was
|
||||
fetched and verified same-origin with no route out, the source and built
|
||||
bundle contain no remote reference, and the CSP would block one. A devtools
|
||||
capture during an offline session is the one more direct form of this
|
||||
evidence and costs about a minute.
|
||||
|
||||
### Affecting model-provider configuration
|
||||
|
||||
11. **`Settings.model` defaults to `""`.** A new install cannot generate a turn
|
||||
until a model is chosen, and nothing prompts for it (§D).
|
||||
|
||||
12. **`netguard` is inert locally by design.** It rejects non-public addresses
|
||||
only when `AIDND_MULTI_USER=1`, which local installs never set — which is
|
||||
why a LAN endpoint is accepted at all. M2's endpoint policy replaces this,
|
||||
and should keep that asymmetry deliberate rather than inherit it silently.
|
||||
|
||||
### Affecting repository structure and testability
|
||||
|
||||
13. **The fork merge preserves all 172 upstream commits**, so upstream fixes
|
||||
can still be cherry-picked. Worth protecting: a future squash or filter
|
||||
would throw it away.
|
||||
|
||||
14. **The AST-based test in `test_tls_trust.py`** guards a class of regression
|
||||
no runtime test can reach — a new HTTP client added later. The same shape
|
||||
may be worth reusing in M2 when the provider surface is cut down.
|
||||
|
||||
15. **The suite is fully offline-capable** (§I), so CI needs no network beyond
|
||||
dependency install.
|
||||
|
||||
### Affecting future removal of hosted/cloud/scripting code
|
||||
|
||||
16. **M1 removed nothing**, so M2's removal surface is exactly what Phase 0B
|
||||
described. Two M1 changes touch files M2 will edit heavily —
|
||||
`providers/openai_compatible.py` and `routers/settings.py` — but both are
|
||||
one-line `verify=` additions, so the TLS work should survive that
|
||||
refactoring as long as the shared context follows any new client.
|
||||
|
||||
---
|
||||
|
||||
# L. Planning Documents That May Need Revision
|
||||
|
||||
Reported, not edited. No planning architecture document was changed by M1.
|
||||
|
||||
### 1. `planning/DECISIONS/002-ollama-only-v1.md` — Consequences
|
||||
|
||||
**Discrepancy.** The ADR treats a trusted-LAN Ollama as an addressing question
|
||||
only. It does not anticipate that such a host may be reachable **only over
|
||||
TLS**, with a certificate from a private CA. The real test host is exactly
|
||||
that, and the application could not talk to it until M1 changed how outbound
|
||||
TLS is verified.
|
||||
|
||||
**Recommended correction.** Add a consequence: the storyteller must verify
|
||||
against the operating system's CA store as well as the bundled one, so a user
|
||||
who has installed their own CA is honoured; and no option to skip verification
|
||||
should be offered.
|
||||
|
||||
### 2. `planning/DECISIONS/004-local-only-production.md` — Consequences
|
||||
|
||||
**Discrepancy.** "Production packaging must contain all runtime assets required
|
||||
for ordinary story use" is right but understates it. Both M1 findings were
|
||||
*first-use* downloads invisible on any machine that had been online once, and
|
||||
neither was findable by static analysis.
|
||||
|
||||
**Recommended correction.** Add that offline claims must be validated on a
|
||||
network with no route out and a fresh cache, and that vendored runtime assets
|
||||
should carry a verifiable digest.
|
||||
|
||||
### 3. `planning/V1-ACCEPTANCE-TESTS.md` — A06
|
||||
|
||||
**Discrepancy.** A06's preconditions do not say whether the LAN endpoint is
|
||||
HTTP or HTTPS, and every example elsewhere shows `http://`. The HTTPS
|
||||
private-CA case is the one that broke, and A06 as written could be passed
|
||||
against a plain-HTTP host without ever exercising it.
|
||||
|
||||
**Recommended correction.** Add a step covering an HTTPS endpoint with a
|
||||
locally-issued certificate, and a pass condition that verification is performed
|
||||
rather than bypassed.
|
||||
|
||||
### 4. `planning/V1-ACCEPTANCE-TESTS.md` — A05
|
||||
|
||||
**Discrepancy.** "Failed turn is not partially committed as accepted" is
|
||||
satisfied, but the observable state is a dangling player action at the head
|
||||
with no reply (§H). A reader could reasonably expect the row count not to move.
|
||||
|
||||
**Recommended correction.** State that the player's own input is retained by
|
||||
design, and that the invariant is about *accepted* history.
|
||||
|
||||
### 5. `planning/BUILD-MILESTONES.md` — M1 Scope
|
||||
|
||||
**Discrepancy.** M1's scope lists four hardening items but not outbound TLS
|
||||
trust, which turned out to be on its critical path.
|
||||
|
||||
**Recommended correction.** Note retrospectively that M1 also covered it, so
|
||||
M2's "explicit Ollama endpoint policy" is not planned as if it were still open.
|
||||
|
||||
### 6. `planning/V1-ACCEPTANCE-TESTS.md` §3 — Standard Test Environment
|
||||
|
||||
**Discrepancy.** The environment list assumes one local Ollama and does not
|
||||
mention hardware class. Model cold-load time on a GPU-less host exceeded the
|
||||
application's fixed 120 s timeout three times during M1.
|
||||
|
||||
**Recommended correction.** Record hardware class alongside model name, and
|
||||
note that a cold load may exceed the client timeout on modest hardware.
|
||||
|
||||
---
|
||||
|
||||
# M. Risks / Technical Debt Carried Into M2
|
||||
|
||||
### Blockers before M2
|
||||
|
||||
**None.**
|
||||
|
||||
### Acceptable technical debt
|
||||
|
||||
| Item | Risk | Suggested milestone |
|
||||
| --- | --- | --- |
|
||||
| **120 s hardcoded model timeout.** Cold loads on modest hardware exceed it; the user sees "The AI endpoint timed out." | Medium — a first-run user on a slow box may conclude the app is broken. | M2, with the endpoint policy |
|
||||
| **Ollama's own `ollama.com` lookup.** Outside this codebase, inside the user's trust boundary. | Low technically, medium for the local-only claim. | Decide before release; document either way |
|
||||
| **Docker loopback depends on the publish flag.** A hand-run `-p 8000:8000` exposes an unauthenticated API to the LAN. | Medium. | M2 |
|
||||
| **`Settings.model` defaults to `""`.** No first turn until a model is chosen; nothing prompts. | Low. | M8 |
|
||||
| **Memory bank off per adventure by default** even with an embedding model configured. Cost me a wasted verification cycle. | Low, but it makes M6's features look absent. | M6 |
|
||||
| **Vendored tokenizer table, 1.7 MB.** Needs re-vendoring if the encoding ever changes. | Low — digest-checked, and a test catches a `tiktoken` change. | — |
|
||||
| **No frontend typecheck.** Plain JSX, lint only. | Low. | M8 |
|
||||
| **`docs/*.html` links Google Fonts.** Not served by the app. | Very low. | M2 or M8 |
|
||||
| **The `dist` tests skip when the SPA is unbuilt.** A green suite alone is not full evidence. | Low — documented in `DEVELOPMENT.md`. | — |
|
||||
|
||||
### Inherited AI-DnD behaviour intentionally left in place
|
||||
|
||||
All of this was outside M1's scope and is **explicitly non-scope** in the M1
|
||||
brief. It is listed so nobody reports it as an M1 gap:
|
||||
|
||||
- multi-user/account/guest/auth flows, demo-key behaviour, hosted rate limits;
|
||||
- the self-hosted analytics tables and Visitors dashboard (local-only, no
|
||||
outbound request);
|
||||
- Render and Neon deployment paths; Postgres/`psycopg` support;
|
||||
- the OpenRouter default-endpoint constant and attribution-host check;
|
||||
- arbitrary remote model-provider configuration in the Settings UI;
|
||||
- QuickJS campaign scripting and the AI-Dungeon-compatible script surface;
|
||||
- **destructive Undo with no Redo** — verified in code, and the single largest
|
||||
inherited correctness gap. M3;
|
||||
- RPG world-state machinery and relative-delta proposals. M5;
|
||||
- Story Cards as the only imported-knowledge mechanism. M7;
|
||||
- AI-DnD's UI organisation and vocabulary. M8.
|
||||
|
||||
---
|
||||
|
||||
# N. Reproduction / Verification Commands
|
||||
|
||||
### Environment
|
||||
|
||||
```bash
|
||||
git clone <this repo> && cd interactive-story
|
||||
git checkout m1-production-baseline
|
||||
python3 -m venv backend/.venv
|
||||
backend/.venv/bin/pip install -r backend/requirements.lock
|
||||
(cd frontend && npm ci && npm run build)
|
||||
```
|
||||
|
||||
### Provenance
|
||||
|
||||
```bash
|
||||
git remote add upstream https://github.com/parththakkar106/AI-DnD.git
|
||||
git fetch --no-tags upstream
|
||||
git cat-file -t d72f7c1bda0f34fccd84afb7a25c34eb01c901de # -> commit
|
||||
git merge-base --is-ancestor d72f7c1bda0f34fccd84afb7a25c34eb01c901de HEAD
|
||||
git diff --stat d72f7c1bda0f34fccd84afb7a25c34eb01c901de HEAD -- LICENSE # -> empty
|
||||
```
|
||||
|
||||
### Tests
|
||||
|
||||
```bash
|
||||
(cd backend && .venv/bin/python -m pytest tests/ -q) # 648 passed
|
||||
(cd frontend && npm run lint && npm run build)
|
||||
docker build -t storyteller .
|
||||
```
|
||||
|
||||
Prove the suite needs no network:
|
||||
|
||||
```bash
|
||||
docker network create --internal offline
|
||||
docker build -t storyteller . && \
|
||||
printf 'FROM storyteller\nRUN pip install --no-cache-dir pytest\n' | docker build -q -t storyteller-test -
|
||||
docker run --rm --network offline -v "$PWD":/src:ro -w /src/backend \
|
||||
-e AIDND_DB_PATH=/tmp/test.db storyteller-test python -m pytest tests/ -q
|
||||
```
|
||||
|
||||
### Run 1 — offline, same-host Ollama
|
||||
|
||||
```bash
|
||||
docker network create --internal offline
|
||||
docker run -d --name ollama --network offline -v ollama-models:/root/.ollama ollama/ollama
|
||||
docker exec ollama ollama pull qwen2.5:3b-instruct
|
||||
|
||||
# The app shares Ollama's network namespace, so Ollama is on its loopback and
|
||||
# neither has a route out.
|
||||
docker run -d --name app --network container:ollama -v story-data:/data \
|
||||
storyteller uvicorn app.main:app --host 127.0.0.1 --port 8000
|
||||
|
||||
# Isolation, before testing anything:
|
||||
docker exec app python -c "import socket; socket.create_connection(('1.1.1.1',443),timeout=4)"
|
||||
# -> OSError: Network is unreachable
|
||||
|
||||
# Listener:
|
||||
docker exec app sh -c "grep -c . /proc/net/tcp" # or the /proc parser in the baseline report
|
||||
```
|
||||
|
||||
Configure and play:
|
||||
|
||||
```bash
|
||||
docker exec app python - <<'PY'
|
||||
import json, urllib.request
|
||||
B = "http://127.0.0.1:8000/api"
|
||||
def call(m, p, b=None):
|
||||
d = json.dumps(b).encode() if b is not None else None
|
||||
r = urllib.request.Request(B+p, data=d, method=m, headers={"Content-Type":"application/json"})
|
||||
with urllib.request.urlopen(r, timeout=900) as f: t = f.read().decode()
|
||||
return json.loads(t) if t.strip() else None
|
||||
call("PUT", "/settings", {"endpoint_url":"http://127.0.0.1:11434/v1",
|
||||
"model":"qwen2.5:3b-instruct","api_mode":"chat","api_key":"",
|
||||
"max_output_tokens":120,"context_token_budget":2048})
|
||||
print(call("POST", "/settings/test"))
|
||||
adv = call("POST", "/adventures", {"title":"Continuity Test","persona_name":"Vale"})["id"]
|
||||
d = json.dumps({"type":"story","text":"I am Vale, a lighthouse keeper."}).encode()
|
||||
r = urllib.request.Request(f"{B}/adventures/{adv}/actions", data=d, method="POST",
|
||||
headers={"Content-Type":"application/json","Accept":"text/event-stream"})
|
||||
with urllib.request.urlopen(r, timeout=900) as f:
|
||||
for line in f:
|
||||
line = line.decode().strip()
|
||||
if line.startswith("data: ") and '"done"' in line:
|
||||
print(json.loads(line[6:])["action"]["text"][:120])
|
||||
PY
|
||||
```
|
||||
|
||||
Restart and resume: `docker restart app`, then re-read
|
||||
`GET /api/adventures/{id}/actions?limit=500` and compare.
|
||||
|
||||
### Run 2 — trusted-LAN Ollama, Internet blocked
|
||||
|
||||
On the **inference machine**, if it serves plain HTTP:
|
||||
|
||||
```bash
|
||||
OLLAMA_HOST=0.0.0.0:11434 ollama serve
|
||||
ollama pull qwen2.5:3b-instruct && ollama pull nomic-embed-text
|
||||
```
|
||||
|
||||
If it serves HTTPS with a private CA, install that CA where the storyteller
|
||||
runs — inside the image for a container:
|
||||
|
||||
```dockerfile
|
||||
FROM storyteller
|
||||
COPY local-ca.crt /usr/local/share/ca-certificates/
|
||||
RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates iproute2 \
|
||||
&& update-ca-certificates && rm -rf /var/lib/apt/lists/*
|
||||
```
|
||||
|
||||
On the **storyteller machine** — LAN reachable, Internet not:
|
||||
|
||||
```bash
|
||||
docker run -d --name app-lan --cap-add=NET_ADMIN \
|
||||
--add-host inference.lan:192.168.0.50 --dns 127.0.0.1 \
|
||||
-v story-data-lan:/data storyteller-lan \
|
||||
uvicorn app.main:app --host 127.0.0.1 --port 8000
|
||||
|
||||
docker exec app-lan ip route del default
|
||||
docker exec app-lan ip route add 192.168.0.0/24 via 172.17.0.1 dev eth0
|
||||
```
|
||||
|
||||
Then set `endpoint_url` to `https://inference.lan:8443/v1` (or
|
||||
`http://192.168.0.50:11434/v1`) and play as above. `POST /api/settings/test`
|
||||
should list the remote host's models.
|
||||
|
||||
### Packet capture (no host root needed)
|
||||
|
||||
```bash
|
||||
printf 'FROM alpine\nRUN apk add --no-cache tcpdump\n' | docker build -q -t tcpdump-img -
|
||||
docker run -d --name cap --network container:app --cap-add=NET_RAW \
|
||||
-v "$PWD/cap":/cap tcpdump-img tcpdump -i any -n -w /cap/run.pcap
|
||||
# … run the campaign …
|
||||
docker stop cap
|
||||
docker run --rm -v "$PWD/cap":/cap tcpdump-img sh -c '
|
||||
tcpdump -r /cap/run.pcap -nn "not net 127.0.0.0/8 and not host 192.168.0.50 and not multicast" | wc -l
|
||||
tcpdump -r /cap/run.pcap -nn -vv "udp port 53" | grep -oE "q: [A-Z]+\? [^ ]+" | sort | uniq -c'
|
||||
```
|
||||
|
||||
### The one uncommitted file
|
||||
|
||||
```bash
|
||||
git add planning/reports/M1-BASELINE-REPORT.md planning/reports/M1-IMPLEMENTATION-REPORT.md
|
||||
git commit -S -m "Add the M1 implementation review report"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# O. Recommendation
|
||||
|
||||
## READY FOR M2 WITH NOTED NON-BLOCKING ISSUES
|
||||
|
||||
M1 delivered its whole scope and its Definition of Done is met on runtime
|
||||
evidence rather than inspection: the application starts and plays with no route
|
||||
to the Internet, through same-host Ollama and through Ollama on a second
|
||||
physical machine; campaigns persist across restart; a failed model call leaves
|
||||
accepted history bit-identical; no runtime asset is fetched; the listener is
|
||||
loopback in every run; and the suite is green at 648 tests, including with no
|
||||
network at all. Nothing was removed from the inherited codebase, so M2 begins
|
||||
against exactly the surface Phase 0B described.
|
||||
|
||||
The issues in §M are non-blocking and mostly land naturally inside milestones
|
||||
that already exist. Two deserve a decision rather than a queue entry: the
|
||||
hardcoded 120 s model timeout, which a first-run user on modest hardware will
|
||||
probably meet before anything else, and Ollama's own `ollama.com` lookup, which
|
||||
is outside this codebase but inside the claim the product makes about itself.
|
||||
|
||||
The finding worth carrying into M2 planning is not a defect but a method. The
|
||||
TLS gap was invisible to static review and to every local run that used
|
||||
loopback HTTP, and it appeared within minutes of pointing the application at a
|
||||
real LAN host. M2 owns the endpoint policy; it should be validated the same
|
||||
way, against a real second machine rather than a container stand-in.
|
||||
Reference in New Issue
Block a user