Record the 2026-07-20 session and the 2026-07-30 senior review
PROGRESS.md gains the ecosystem-hardening session log: entity-aware fact resolution, the typed Praxis lifecycle client, the durable delivery outbox, fail-closed IPC handling, correlation IDs, and the fake-ecosystem test harness. 20-07-2026-BACKLOG.md marks item 2 (morning routine engine) as core-engine done, with the wiring and the read-only /morning page described. REVIEW-30-07-2026.md is the review the preceding commits act on. Note two of its findings were wrong on the details and are corrected in the commits rather than in the document: the covdata failure was upstream Go tool-shipping behaviour, not a truncated cache, and the resident-model contradiction ran the opposite way — deploy/mavend.json pointed at a file that existed while the docs carried the stale claim, because /mnt/hdd1/llms is bind-mounted over the repo's models/llm/. One caveat, since the review itself argues against dated markdown accumulating at the root: this file should go the same way as the SESSION-*.md logs once its findings are closed. Two remain open — the kuma key rotation and the LLM router evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5JApcrCRVGmqrxnhynSik
This commit is contained in:
+90
-7
@@ -33,7 +33,40 @@ this gives digestion one stable input instead of source-specific logic.
|
||||
|
||||
---
|
||||
|
||||
### 2. explicit morning routine engine
|
||||
### 2. explicit morning routine engine — **core engine done (2026-07-20)**
|
||||
|
||||
`internal/morning` — pure checklist engine, mirrors `internal/loop`/
|
||||
`internal/routine`'s no-I/O contract. `Evaluate(routine, facts, now)` answers
|
||||
"what's still missing" any time (order-independent — checks facts, not
|
||||
sequence); `Due(routines, facts, last, now)` fires the once-per-day nag only
|
||||
at `NudgeAt` (defaults to window end) and only when something's unevidenced,
|
||||
with a `last`-map dedupe identical in shape to `routine.Due`'s cold-start/
|
||||
last-fire tracking. Evidence is just a fact timestamped inside today's
|
||||
window — manual (voice-tapped) and inferred (another daemon writing the same
|
||||
key) are indistinguishable, satisfying the manual/inferred requirement for
|
||||
free. Weekday/weekend variants are two `Routine`s with different `Weekdays`
|
||||
sets under different names. Wired into `config.MorningRoutineConfig` +
|
||||
`cmd/mavend/tick.go`'s `fireMorningRoutines` (reads only the fact keys the
|
||||
configured items reference, dispatches through the normal severity/presence
|
||||
routing table, body is literal joined item labels — not LLM-phrased, same
|
||||
no-hallucination rationale as cron routines). 13 unit tests in
|
||||
`internal/morning/morning_test.go`.
|
||||
|
||||
Added since (2026-07-20, same day): a read-only `/morning` page in mavweb —
|
||||
`ipc.CoreAPI.MorningStatus` (new wire method, mirrors `TickTrace`'s
|
||||
daemon-cache-only shape: the store adapter errors, `daemonAPI` serves it from
|
||||
a `tickLoop.morningStatus` closure) returns each routine's active/window/
|
||||
per-item done state, server-rendered same as `/trace` (no live-update loop —
|
||||
checklist state moves on minutes, not seconds).
|
||||
|
||||
Not yet done: no config wired in `deploy/mavend.json` (no morning routines
|
||||
configured on homesrv yet — add items there when the medicine/water/pets
|
||||
fact keys the phone/desktop write are settled), no voice query path for
|
||||
"what did I miss this morning" (Evaluate supports it; nothing calls it yet),
|
||||
no way to create/edit routines from the web UI — construction still means
|
||||
hand-editing config, deliberately deferred: routines are operator-declared
|
||||
config (like cron routines), and a CRUD editor would mean moving them to a
|
||||
DB table + hot-reload, a bigger change than this pass.
|
||||
|
||||
not ordinary reminders.
|
||||
|
||||
@@ -63,6 +96,15 @@ maven should know what is still missing, not merely fire four timers.
|
||||
|
||||
### 3. cross-device presence
|
||||
|
||||
**status (2026-07-20):** the hysteresis engine and 3 of the listed signals are
|
||||
already built and wired live: `internal/store/presence.go` (noisy-OR combiner
|
||||
+ Schmitt-trigger bucket resolve), fed by `desk_active` (workstation, via
|
||||
`scripts/desk-active.sh` posting to `/api/signal`), `page_heartbeat` (mavweb
|
||||
tab, `app.js`), and `wg_handshake` (`mavpoll` polling `wg show`) — threaded
|
||||
into the tick loop via `internal/loop/gather.go`. Not done: phone-reachable,
|
||||
homesrv-available, audio-output, and active-maven-client signals from the
|
||||
list below are still missing.
|
||||
|
||||
a small presence daemon on each trusted device:
|
||||
|
||||
* workstation active/idle
|
||||
@@ -84,7 +126,19 @@ useful for:
|
||||
|
||||
---
|
||||
|
||||
### 4. interruption policy
|
||||
### 4. interruption policy — **done (2026-07-20), turned out to already be built**
|
||||
|
||||
audited the existing code before writing anything new: `internal/loop.Gate`
|
||||
already answers deliver_now vs. drop (quiet-hours/cooldown/snooze/presence/
|
||||
calendar-busy), and `cmd/mavend/tick.go`'s `digestQ` + `config.DigestConfig`
|
||||
already implement queue/digest (low-severity nudges batch into one
|
||||
notification, flushed on window elapsed or max-items reached). The four
|
||||
outcomes below were already covered by these two mechanisms; nothing new to
|
||||
build for the core policy.
|
||||
|
||||
Gap that *was* real: `deploy/mavend.json` had no `digest` block, so batching
|
||||
was disabled in prod despite being fully implemented. Fixed — see the config
|
||||
change alongside this note.
|
||||
|
||||
before delivering anything, evaluate:
|
||||
|
||||
@@ -110,7 +164,27 @@ this prevents maven from becoming annoying once praxis and other sources start p
|
||||
|
||||
---
|
||||
|
||||
### 5. entity-aware memory
|
||||
### 5. entity-aware memory — **done (2026-07-20)**
|
||||
|
||||
`03fa52d`/`9876187` (Vikunja #279): facts gain `Subject`/`EntityID`/
|
||||
`ResolutionState`; an async enrichment worker resolves free-text subjects to
|
||||
canonical Nexus entity_ids (mirrors Praxis's enrichment pattern). Ambiguous
|
||||
or unreachable Nexus never guesses — the fact stays `pending` or terminal
|
||||
`ambiguous`. Voice-tapped facts (`IntentFact`) now flow into the enrichment
|
||||
queue automatically via an optional `Subject` field on `WriteFactReq` (old
|
||||
callers unaffected).
|
||||
|
||||
Landed alongside this in the same session (not originally on this list, but
|
||||
closes the plumbing gaps the last brief flagged for Nexus/Praxis maturity):
|
||||
a typed Praxis lifecycle client (`398997f` — surface/acknowledge/resolve/
|
||||
ignore/pin; fixes the surfaced≠acknowledged gap where reading an item aloud
|
||||
left no trace), correlation-ID/version headers on the Nexus/Praxis clients
|
||||
(`b743860`), entity-scoped Praxis attention queries (`0579ef9`), a durable
|
||||
delivery outbox with begin-before-send/complete-after semantics
|
||||
(`29f23e3`+`9ff726e` — closes a duplicate-send-on-crash bug), fail-closed
|
||||
handling on ambiguous IPC mutation outcomes and Nexus/Hexis dependency
|
||||
errors (`838fde1`+`d9fa4d6`), and a reusable fake-ecosystem test harness
|
||||
with fault injection (`c932cd8`).
|
||||
|
||||
connect maven memory to nexus ids.
|
||||
|
||||
@@ -163,7 +237,11 @@ this matters a lot for a 1.7b model.
|
||||
|
||||
---
|
||||
|
||||
### 7. evaluation lab
|
||||
### 7. evaluation lab — **skipped for now (2026-07-20)**
|
||||
|
||||
runs on a different machine (GPU box), and CPT is currently in progress
|
||||
there — deprioritized until the training pipeline has a checkpoint to gate.
|
||||
Not abandoned, just off the immediate list.
|
||||
|
||||
before every new checkpoint or lora deploy:
|
||||
|
||||
@@ -298,9 +376,14 @@ keep this read-only and separate from personal fact memory.
|
||||
|
||||
## recommended order
|
||||
|
||||
1. evaluation lab
|
||||
2. entity-aware memory
|
||||
3. morning routine engine
|
||||
**status as of 2026-07-20:**
|
||||
|
||||
1. ~~evaluation lab~~ — **skipped, GPU-box work, deprioritized while CPT is in progress**
|
||||
2. ~~entity-aware memory~~ — **done** (`03fa52d`/`9876187`, plus adjacent
|
||||
Nexus/Praxis plumbing hardening — see item 5 above)
|
||||
3. ~~morning routine engine~~ — **core engine done** (`internal/morning` +
|
||||
`cmd/mavend` wiring — see item 2 above; not yet configured on homesrv,
|
||||
no voice query, no web UI)
|
||||
4. interruption/delivery policy
|
||||
5. presence agents
|
||||
6. unified event intake
|
||||
|
||||
+72
-5
@@ -1,13 +1,80 @@
|
||||
## Maven — current state (updated 2026-07-18)
|
||||
## Maven — current state (updated 2026-07-20)
|
||||
|
||||
Architecture decision: the target resident router/phraser is the locally
|
||||
trained Qwen3-1.7B model. Older LFM references below describe the currently
|
||||
deployed/historical stack, not the target checkpoint. RU CPT has a successful
|
||||
### Session 2026-07-20 — ecosystem hardening + entity-aware facts
|
||||
|
||||
Ten commits, focused on closing the Nexus/Praxis integration gaps flagged
|
||||
as "wired but immature" in the prior review, plus the entity-aware-memory
|
||||
backlog item (`20-07-2026-BACKLOG.md` item 5).
|
||||
|
||||
- **Entity-aware fact resolution (Vikunja #279)** — facts gain
|
||||
`Subject`/`EntityID`/`ResolutionState`; an async worker resolves
|
||||
free-text subjects to canonical Nexus entity_ids (mirrors Praxis's own
|
||||
enrichment pattern). Ambiguous/unreachable Nexus never guesses — stays
|
||||
`pending` or terminal `ambiguous`. Voice-tapped facts (`IntentFact`) flow
|
||||
into the queue automatically via an optional `Subject` field on
|
||||
`WriteFactReq` (old callers unaffected, no signature break).
|
||||
- **Typed Praxis lifecycle client (Vikunja #271)** — `GetItem`/`Search`/
|
||||
`Surface`/`Acknowledge`/`Resolve`/`Ignore`/`Pin`, routed through new RU/EN
|
||||
dialogue verbs. Fixes a real lifecycle-invariant bug: reading an
|
||||
attention item aloud now calls `Surface` — previously the digest path
|
||||
read items without recording that they'd been surfaced, so "Maven
|
||||
mentioned it" was indistinguishable from "never came up."
|
||||
- **Durable delivery outbox (Vikunja #270)** — `BeginDeliveryAttempt`
|
||||
before `Send`, `CompleteDeliveryAttempt` after; a stale `pending` row
|
||||
found at startup reconciles to `unknown` (never silently resent or
|
||||
dropped — same rule as Hexis's execution-timeout handling). Closes a
|
||||
crash-window duplicate-send bug. Wired into `DispatchNudge`,
|
||||
`DispatchReminder`, `RepeatUnacked`; reconciliation runs once at boot
|
||||
before the tick loop resumes.
|
||||
- **Fail-closed IPC/dependency handling (Vikunja #269, #272/#273)** —
|
||||
ambiguous mutation outcomes (frame sent, reply lost) no longer blindly
|
||||
retry; Nexus/Hexis dependency errors fail closed instead of guessing.
|
||||
- **Correlation IDs + version headers (Vikunja #273)** — the hand-rolled
|
||||
Nexus/Praxis HTTP clients now send `X-Nexus-Version`/`X-Praxis-Version`
|
||||
and thread the same correlation ID already generated in
|
||||
`executeCapability` through the whole call chain, matching the Hexis
|
||||
client's existing behavior.
|
||||
- **Entity-scoped Praxis attention queries** — callers holding a resolved
|
||||
entity_id can ask "what needs attention for this entity" directly
|
||||
instead of filtering the unscoped list client-side.
|
||||
- **Fake-ecosystem test harness with fault injection** — a reusable
|
||||
`fakeServer` (Nexus/Praxis/Hexis fixtures, runtime-toggleable
|
||||
`SetFault`, fake clock) replacing ad-hoc per-test `httptest` servers;
|
||||
covers a gap that had zero test coverage (`handlePraxisAct`) and adds a
|
||||
fault-then-recovery regression test for the fail-closed fixes above.
|
||||
- **Ops fix** — `deploy/mavend.json`'s phraser was pointed at a 4B model
|
||||
with `n_gpu_layers=99`, which OOM'd under memory pressure and left a
|
||||
zombie `llama-server` child; swapped to the 2B Qwen model matching the
|
||||
intended resident-model size.
|
||||
|
||||
Net effect: the Nexus/Praxis wiring described as "plumbing exists, thin
|
||||
compared to Maven's test depth" in the prior review is now materially
|
||||
hardened — typed clients, fail-closed error handling, durable delivery,
|
||||
and a proper fault-injection test harness are all in place. Evaluation lab
|
||||
(`20-07-2026-BACKLOG.md` item 7) is explicitly skipped for now — it runs
|
||||
on the GPU box, which is occupied by CPT. Morning routine engine (backlog
|
||||
item 3) is next up, not started.
|
||||
|
||||
---
|
||||
|
||||
> **Resolved 2026-07-30 (task #318).** The resident checkpoint is
|
||||
> **Qwen3.5-0.8B** (`Q4_K_M`), set in `deploy/mavend.json`; the **target** is
|
||||
> the locally CPT'd **Qwen3-1.7B**, still training (#122). Older model claims
|
||||
> below — the LFM references, the pipeline line, and the "swapped to the 2B
|
||||
> Qwen model" ops entry above — are historical. Read them as a log of what was
|
||||
> true at the time, not as current fact. Note also that `/mnt/hdd1/llms` is
|
||||
> bind-mounted over `models/llm/`, so the LFM2.5 gguf in the repo tree is
|
||||
> never loaded.
|
||||
|
||||
Architecture decision (as written on 2026-07-20): the target resident
|
||||
router/phraser is the locally trained Qwen3-1.7B model — still the target as
|
||||
of 2026-07-30. Older LFM references below describe the then-deployed
|
||||
historical stack, not the target checkpoint. RU CPT has a successful
|
||||
full-weight checkpoint at step 1000/8077; evaluation and Qwen3 SFT tooling are
|
||||
tracked in `docs/plans/2026-07-18-qwen3-resident-training-eval.md`.
|
||||
|
||||
Consolidated status. The reactive↔proactive core is closed and testable through
|
||||
the web PWA. The SPEC's open items 1–7 are landed (protocol doc, away-channel
|
||||
the web PWA. The former SPEC's open items 1–7 (now `DESIGN.md` § execution ledger) are landed (protocol doc, away-channel
|
||||
fallthrough, CalDAV poller, quiet-hours schedule, tools enable/disable, note RAG,
|
||||
passkey step-up); item 8 (multi-user) is deliberately deferred — see the tail.
|
||||
The two big infra gaps from the jul5 revision are closed on `overnight-jul5`:
|
||||
|
||||
@@ -0,0 +1,362 @@
|
||||
# Maven — senior review, 2026-07-30
|
||||
|
||||
Scope: code, config, docs, deploy path, test suite. Runtime claims below come from
|
||||
tests, config and static reading — the daemons were not run and routing latency was
|
||||
not measured. Working tree at review time: uncommitted `internal/morning` work plus
|
||||
`cmd/mavend/eval_scenarios_test.go` (both look correct, see "keep").
|
||||
|
||||
---
|
||||
|
||||
## verdict
|
||||
|
||||
**Healthy engineering, misaligned direction.** The code is unusually disciplined for a
|
||||
personal project — clean boundaries, pure-core/impure-driver separation, argv-only tool
|
||||
execution, 12k lines of real tests, `go vet` clean. The problem is not code quality. It is
|
||||
that **the single most important architectural decision has four contradictory sources of
|
||||
truth and the intended design is switched off in production**, plus a broken green-build
|
||||
signal and a deploy path that does not enforce the security boundary its own comments
|
||||
assume.
|
||||
|
||||
## what the project is now
|
||||
|
||||
```
|
||||
purpose: self-hosted RU/EN voice assistant; 8 Go daemons over unix sockets,
|
||||
SQLite, local models
|
||||
intended users: one person, one box (homesrv, Ryzen 5 5600U, Vega iGPU, 14GB)
|
||||
actual users: same — confirmed by hardcoded TZ, single-passkey auth,
|
||||
/home/kami replace directives in go.mod
|
||||
critical workflows: voice turn (mic→STT→route→tool/reply→TTS); proactive nudges
|
||||
(tick→dispatcher→telegram/ntfy/voice); web PWA
|
||||
(/dash /chat /tools /ecosystem)
|
||||
current state: all 34 test packages pass; vet clean; `make test` still exits 1
|
||||
(finding 2)
|
||||
known failures: weak RU query routing — REARCH.md names the cause; the named fix
|
||||
is wired `nil`
|
||||
maintenance burden: 4,518 lines of root markdown vs 33,319 lines of Go; 15 top-level
|
||||
.md files, 3 of them dated session logs; several contradict
|
||||
each other
|
||||
constraints: CPU-only deploy, ~14GB shared RAM, resident model must stay small;
|
||||
never phones home; not autonomous; feminine RU persona
|
||||
what works well: internal/tool (argv-only, store allowlist, confirm gate); pure
|
||||
engines (loop, routine, morning, pattern, dialogue — 86–100%
|
||||
coverage); delivery outbox (Begin-before-send/Complete-after);
|
||||
vendored deps (5 direct, tiny surface)
|
||||
what's obsolete: llmrouter.go (built, tested, never wired); classifier seed-phrase
|
||||
cascade (interim stopgap, now 4 weeks old);
|
||||
SESSION-05/06-07-2026.md; PLANS.md; ROADMAP.md
|
||||
```
|
||||
|
||||
**Classification: healthy + misaligned.** Not fragile, not overbuilt, not abandoned. The
|
||||
architecture in `REARCH.md` is sound and mostly *built* — it just is not *connected*.
|
||||
|
||||
## what it should become
|
||||
|
||||
The thing `REARCH.md` already describes, with the switch flipped and the drift removed:
|
||||
one resident small model doing both routing and phrasing, classifier demoted from the live
|
||||
path to the failure floor, embedder demoted to RAG hint. No new architecture is needed.
|
||||
**The gap is a config/wiring decision plus doc convergence, not a redesign.**
|
||||
|
||||
---
|
||||
|
||||
## main findings
|
||||
|
||||
### 1. Four competing sources of truth on the resident model — and the target router is dead code
|
||||
|
||||
`architecture` / `repair`
|
||||
|
||||
**problem:** The most load-bearing design decision in the project is stated four different,
|
||||
incompatible ways, and the code path `REARCH.md` calls "the linchpin" is disabled.
|
||||
|
||||
**evidence** (all confirmed):
|
||||
|
||||
- `cmd/mavend/voice.go:211` — `rtr := buildRouter(emb, matcher, threshold, nil) // LLM router disabled`,
|
||||
with comment *"the classifier handles routing reliably."*
|
||||
- `REARCH.md:11` says the same classifier is *"the structural cause of 'she messes up
|
||||
queries.'"* **The code comment and the design doc make opposite claims about the same
|
||||
component.**
|
||||
- `internal/router/llmrouter.go` (139 lines) + `llmrouter_test.go` — fully built and
|
||||
tested, zero non-test callers.
|
||||
- Model identity, four ways: docs say **Qwen3-1.7B** (`CLAUDE.md:6`, `REARCH.md:15`,
|
||||
`SPEC.md:46`, `AGENTS.md:79`, `MAVEN_ECOSYSTEM_ARCHITECTURE.md:72`);
|
||||
`deploy/mavend.json:9` says **Qwen3.5-2B-UD-Q4_K_XL**; `models/llm/` on disk holds
|
||||
**LFM2.5-1.2B-Thinking**; code comments in 5 files still say **LFM**.
|
||||
- `deploy/mavend.json:11` sets `"n_gpu_layers": 99` while `CLAUDE.md:4` states the target
|
||||
is **CPU-only**. (Interpretation: compose does pass `/dev/dri` + render gid for Vulkan,
|
||||
so 99 may be intentional — but then the CPU-only claim is stale. One of the two is
|
||||
wrong; the repo alone cannot say which.)
|
||||
|
||||
**impact:** You cannot answer "which model is deployed and does it route?" from this repo.
|
||||
Every future routing decision starts by re-deriving the ground truth — that is the actual
|
||||
maintenance cost, and it is the reason a 4-week-old "interim stopgap" is still the
|
||||
committed default. The 2,453 lines of router package carry two engines where one is wanted.
|
||||
|
||||
**recommended action:** Pick the deployed model, then in one commit: set
|
||||
`phraser.model_path` to it, wire `NewLLMRouter(llmClient)` into `buildRouter` at
|
||||
`voice.go:211` behind a config flag (`voice.llm_router: true`) so the classifier stays the
|
||||
documented failure floor, and update the model name in all 5 docs + 5 code comments to
|
||||
match. Delete nothing from `internal/router` yet — the classifier is the fallback by design.
|
||||
|
||||
**why this level of change:** The components exist and are tested. This is wiring + truth
|
||||
reconciliation, not a refactor. **Do not rewrite the router.**
|
||||
|
||||
**alternatives:** Delete `llmrouter.go` and commit to the classifier — only defensible if
|
||||
the eval harness shows the classifier is actually adequate, which contradicts `REARCH.md`.
|
||||
|
||||
**risk:** Low-moderate. LLM route failures already fall through to the classifier
|
||||
(`router.go:88-96`), so a bad model cannot break a turn. The real risk is CPU latency.
|
||||
|
||||
**verification:** `cmd/mavend/eval_scenarios_test.go` + `testdata/` (already in the working
|
||||
tree, uncommitted) is exactly the right harness — run RU query scenarios against both
|
||||
paths and compare before flipping the default.
|
||||
|
||||
---
|
||||
|
||||
### 2. `make test` always fails, so there is no green signal
|
||||
|
||||
`repair` — smallest high-value fix in the repo
|
||||
|
||||
**problem:** Every test passes; `make test` exits 1 anyway.
|
||||
|
||||
**evidence** (confirmed, root-caused): `make test` ends with
|
||||
`# github.com/kami/maven/cmd/mavenclient` / `go: no such tool "covdata"` →
|
||||
`make: *** Error 1`. Cause chain:
|
||||
|
||||
- `deps/go/go/VERSION` = **go1.23.4**; `go.mod` requires **go 1.25.5** → `GOTOOLCHAIN=auto`
|
||||
re-execs into `/home/kami/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.5.linux-amd64`.
|
||||
- That cached toolchain ships only 7 of ~17 tools: `asm cgo compile cover link preprofile
|
||||
vet`. **`covdata` is missing** (source present, binary absent), as are `pprof`, `nm`,
|
||||
`objdump`, `trace`, `test2json`.
|
||||
- `-coverprofile` invokes `covdata` for the two packages with no test files
|
||||
(`cmd/mavenclient`, `cmd/mavend/seedtest`) → failure. `GOTOOLCHAIN=local` confirms the
|
||||
vendored 1.23.4 is too old to substitute
|
||||
(`go.mod requires go >= 1.25.5 (running go 1.23.4)`).
|
||||
|
||||
**impact:** A permanently-red test target trains you to ignore the exit code — the one
|
||||
signal protecting a 12k-line test suite. `go tool pprof` is also unavailable, which matters
|
||||
on a latency-sensitive CPU-only box.
|
||||
|
||||
**recommended action:** `rm -rf '/home/kami/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.5.linux-amd64'`
|
||||
and let Go re-download it complete — the cache entry is truncated, not a Makefile bug.
|
||||
Then bump `deps/go` to a real 1.25.5 so the vendored toolchain is self-sufficient and
|
||||
`GOTOOLCHAIN=local` works, which is the whole point of vendoring it.
|
||||
|
||||
**risk:** None. Worst case the re-download fails and you are where you started.
|
||||
|
||||
**verification:** `make test` exits 0.
|
||||
|
||||
---
|
||||
|
||||
### 3. Deploy path publishes an unauthenticated RCE surface; the code's stated mitigation is not in the repo
|
||||
|
||||
`security`
|
||||
|
||||
**problem:** The tool-enable gate is deliberately fail-open, documented as safe because it
|
||||
"sits behind wg+nginx+auth" — but the repo's own deploy path publishes the port with no
|
||||
such layer and no WebAuthn.
|
||||
|
||||
**evidence** (confirmed):
|
||||
|
||||
- `cmd/mavweb/main.go:918` — `if session != nil && !session.IsStepUp()`. `stepUpSession`
|
||||
is nil unless `-webauthn-origin` **and** `-webauthn-rpid` are both set (`main.go:419`).
|
||||
Nil ⇒ **gate skipped entirely.**
|
||||
- `docker-compose.yml:82` passes neither flag, and `ports: ["9201:9201"]` publishes
|
||||
mavweb on the host.
|
||||
- `handleTools` `enable` takes `name` + `cmd` from form fields and calls `core.EnableTool`
|
||||
— so this is not "approve a proposal," it is **define arbitrary argv**. `internal/tool`
|
||||
then executes it. `destructive` is a checkbox the caller sets.
|
||||
- No auth middleware wraps the mux. `/ptt` (`main.go:1037`) proxies straight to the voice
|
||||
server, so an unauthenticated caller also gets voice turns — and can answer its own
|
||||
confirm turn.
|
||||
- Separately: `docker-compose.yml` contains a literal committed credential,
|
||||
`-kuma-key uk5_mavpoll-key`.
|
||||
|
||||
**attack path:** anyone who reaches `homesrv:9201` → `POST /tools` with
|
||||
`action=enable&name=x&cmd=<any binary + args>` → `POST /ptt` uttering `x` → arbitrary
|
||||
command execution as the mavend user. No credential needed.
|
||||
|
||||
**impact:** The trust boundary the code carefully reasons about (`internal/tool` doc
|
||||
comment: *"a compromised router can't grant itself a capability"*) is real, but the outer
|
||||
boundary it depends on is an unwritten deployment assumption. Code safe, deployment not.
|
||||
|
||||
**recommended action:** Two small changes, not a redesign.
|
||||
(a) Bind mavweb to loopback in compose — `ports: ["127.0.0.1:9201:9201"]` — so reaching it
|
||||
requires the wg tunnel by construction, not by convention.
|
||||
(b) Make the fail-open explicit and loud: if `-webauthn-origin` is unset, log a startup
|
||||
warning naming `/tools` and `/api/revert` as unguarded, and add a `-require-stepup` flag
|
||||
that fails closed for when you do want it.
|
||||
Move the kuma key into `deploy/telegram.env` (already gitignored) and rotate it.
|
||||
|
||||
**note:** The fail-open is a **documented deliberate choice** (`main.go:411-418` explains
|
||||
it, and it is correct that gating on an unassertable session would 403 permanently). The
|
||||
finding is that the compensating control is absent from the repo, not that the reasoning
|
||||
is wrong.
|
||||
|
||||
**risk:** Low. Loopback binding breaks off-box access only if something other than wg
|
||||
currently reaches 9201 directly — worth confirming on the box first.
|
||||
|
||||
---
|
||||
|
||||
### 4. `cmd/mavend/voice.go` is 1,715 lines doing six unrelated jobs
|
||||
|
||||
`refactor` — later, not now
|
||||
|
||||
**problem:** One file holds daemon wiring (`wireVoice`, 240 lines), the turn handler,
|
||||
intent dispatch (`applyAction`, 280 lines), Praxis/Hexis ecosystem acts (~250 lines), RU
|
||||
text utilities (`ruPlural`, `stripWake`, `hasDurationWords`, `formatTime`), classifier
|
||||
seeding, and weather parsing.
|
||||
|
||||
**evidence:** confirmed — 33 functions, largest file in the project by 40%; `cmd/mavend`
|
||||
coverage is 29.8%, the lowest of any substantial package, precisely because this file mixes
|
||||
wiring (hard to test) with pure logic (trivial to test).
|
||||
|
||||
**impact:** Not urgent — nothing is broken. But it is the reason the router flip in finding
|
||||
1 feels risky: the wiring you must touch sits in the same file as the dispatch logic you
|
||||
must not break.
|
||||
|
||||
**recommended action:** Split along seams that already exist: `wire_voice.go`
|
||||
(construction), `handler.go` (turn), `acts_ecosystem.go` (Praxis/Hexis — already partly in
|
||||
`ecosystem.go`), and move the RU helpers to `internal/ttsnorm` or a new `internal/rutext`.
|
||||
Pure-function extraction lifts `cmd/mavend` coverage with no behavior change. **Do this
|
||||
after finding 1, not before** — and skip it if it stays purely cosmetic.
|
||||
|
||||
---
|
||||
|
||||
### 5. Documentation outweighs its own usefulness
|
||||
|
||||
`cleanup` / `deletion`
|
||||
|
||||
**problem:** 15 root markdown files, 4,518 lines, several stale or superseded, at least
|
||||
three pairs contradicting each other.
|
||||
|
||||
**evidence:** `ROADMAP.md` (759) + `MAVEN_ECOSYSTEM_ARCHITECTURE.md` (884) +
|
||||
`PROGRESS.md` (456) + `maven.md` (413) + `20-07-2026-BACKLOG.md` (396) +
|
||||
`SESSION-05-07-2026.md` + `SESSION-06-07-2026.md` (477 combined) + `PLANS.md` (25) +
|
||||
`START.md` + `SPEC.md` + `PROTOCOL.md`. `PROGRESS.md:61` annotates its own staleness:
|
||||
*"Older LFM references below describe the currently deployed..."*. `REARCH.md` announces it
|
||||
"supersedes" a model still described as current elsewhere.
|
||||
|
||||
**impact:** The doc set is the reason finding 1 exists. When five documents describe the
|
||||
architecture, the code becomes the only trustworthy one — which defeats the purpose of
|
||||
having them.
|
||||
|
||||
**recommended action:** Keep `CLAUDE.md` (agent contract), `REARCH.md` (target
|
||||
architecture), `AGENTS.md` (recipes), `PROTOCOL.md` (wire format),
|
||||
`20-07-2026-BACKLOG.md` (live queue). Delete the two `SESSION-*.md` and `PLANS.md` — git
|
||||
history holds them. Fold `SPEC.md` + `maven.md` + `ROADMAP.md` into one `DESIGN.md` and
|
||||
mark superseded sections instead of leaving them to read as current. Target ~1,500 lines.
|
||||
|
||||
---
|
||||
|
||||
## keep
|
||||
|
||||
- `internal/tool` — argv-only executor, store allowlist, confirm gate. Well-reasoned; the
|
||||
doc comment explains *why* rather than *what*.
|
||||
- `internal/loop`, `routine`, `morning`, `pattern`, `dialogue` — pure engines, impure
|
||||
drivers in `tick.go`, 86–100% coverage. That pattern is working; keep applying it.
|
||||
- The delivery outbox (Begin-before-send / Complete-after / unknown-on-crash).
|
||||
- Dependency discipline: 5 direct deps for a voice assistant is excellent.
|
||||
- The uncommitted `internal/morning` work — follows the established pattern correctly, and
|
||||
its rationale comment ("why not four timers") is the right kind of comment.
|
||||
|
||||
## remove
|
||||
|
||||
`SESSION-05-07-2026.md`, `SESSION-06-07-2026.md`, `PLANS.md`. Stale `LFM` comments in 5
|
||||
files once the model is pinned. The committed kuma key. `n_gpu_layers: 99` **or** the
|
||||
CPU-only claim in `CLAUDE.md` — whichever is false.
|
||||
|
||||
## repair now
|
||||
|
||||
1. **Toolchain / `make test` exit code** (finding 2) — ~10 minutes, restores the safety
|
||||
net. Do this first; everything else is verified through it.
|
||||
2. **Bind mavweb to loopback + startup warning on unguarded step-up + rotate the kuma key**
|
||||
(finding 3) — ~30 minutes, closes an unauthenticated RCE path.
|
||||
3. **Pin the model and reconcile all five docs + config** (finding 1, first half) — the
|
||||
decision is yours; the edit is mechanical once made.
|
||||
4. **Flip the LLM router behind a config flag and evaluate with `eval_scenarios_test.go`**
|
||||
(finding 1, second half) — the change that addresses the actual user-facing complaint.
|
||||
|
||||
## refactor later
|
||||
|
||||
Split `voice.go` (finding 4). Consolidate docs (finding 5). Raise `internal/phraser`
|
||||
coverage from 16.5% — lowest in the project, and it owns the llama-server subprocess
|
||||
lifecycle, exactly where a leak or orphan would hide (the `Pdeathsig` handling looks right,
|
||||
but it is nearly untested).
|
||||
|
||||
## rewrite only if
|
||||
|
||||
Nothing here justifies a rewrite. `internal/router` is the only candidate and it fails the
|
||||
test: the target engine is already written and tested, the fallback is deliberate, and
|
||||
there is no compatibility burden. **Incremental correction is clearly cheaper.** Revisit
|
||||
only if the LLM router needs a fundamentally different `Decision` shape than the cascade
|
||||
produces — evaluate that with the scenario harness before touching structure.
|
||||
|
||||
```
|
||||
incremental repair cost: one wiring change + config flag + doc sweep
|
||||
rewrite cost: re-derive 7-intent contract, slot extraction, RU
|
||||
grammars, stage-0 fast path
|
||||
migration cost: n/a — single user, no external consumers
|
||||
behavior at risk: stage-0 fast path, RU slot extraction, confirm turns
|
||||
tests available: router_test, slots_ru_test, eval_scenarios_test
|
||||
hidden knowledge in code: high — RU qualifier pre-processing, seed collisions,
|
||||
wake-token stripping across scripts
|
||||
expected maintenance gain: none over the incremental path
|
||||
```
|
||||
|
||||
## verification plan
|
||||
|
||||
1. `make test` exits 0 (currently broken, so this is step zero).
|
||||
2. `make build` — all 8 binaries.
|
||||
3. `go vet ./internal/... ./cmd/...` stays clean.
|
||||
4. Run `eval_scenarios_test.go` against classifier-only and LLM-router paths; compare RU
|
||||
query accuracy.
|
||||
5. `start-maven.sh` locally; exercise a voice turn and a destructive-tool confirm turn.
|
||||
6. Confirm `curl homesrv:9201/tools` is unreachable from off-box after the bind change.
|
||||
|
||||
## uncertainties
|
||||
|
||||
- **Which model is actually running on homesrv.** Repo evidence points three ways;
|
||||
unresolvable without the box. Everything in finding 1 depends on the answer.
|
||||
- **Whether `n_gpu_layers: 99` is correct.** Compose passes `/dev/dri` and the render gid,
|
||||
suggesting Vulkan offload is intended and working — but `CLAUDE.md` says CPU-only. Likely
|
||||
the doc is stale, not the config; unverified.
|
||||
- **Whether the classifier is genuinely adequate.** `voice.go:211` asserts it is;
|
||||
`REARCH.md` asserts it is not. Both are claims, neither is measured. The uncommitted eval
|
||||
harness is the instrument to settle it — resolve before flipping the router, not after.
|
||||
- **Whether wg+nginx+auth actually fronts 9201 in production.** Not in this repo. If it
|
||||
does, finding 3 drops from "unauthenticated RCE" to "the control is not reproducible from
|
||||
the repo" — still worth fixing, much less urgent.
|
||||
- The daemons were not run and routing latency was not measured.
|
||||
|
||||
---
|
||||
|
||||
## second opinion (Claude, 2026-07-30)
|
||||
|
||||
Key claims independently re-verified against the working tree; the verdict stands.
|
||||
|
||||
**Confirmed directly:**
|
||||
|
||||
- Finding 1: `voice.go:211` passes `nil` for the LLM router; `deploy/mavend.json:9`
|
||||
points at `Qwen3.5-2B-UD-Q4_K_XL.gguf`; `models/llm/` on disk holds only
|
||||
`LFM2.5-1.2B-Thinking-Q4_K_M.gguf`; `CLAUDE.md` says Qwen3-1.7B. At least three live
|
||||
contradictions, exactly as stated.
|
||||
- Finding 2: reproduced — all packages pass, `make: *** Error 1` anyway; vendored
|
||||
toolchain is go1.23.4 vs `go 1.25.5` in `go.mod`. Correctly sequenced first.
|
||||
- Finding 3: `docker-compose.yml` publishes `9201:9201` with no webauthn flags, and the
|
||||
kuma key `uk5_mavpoll-key` is committed verbatim. Attack path is coherent.
|
||||
|
||||
**Nuances to the review's framing:**
|
||||
|
||||
- Finding 3's severity depends entirely on network topology. If 9201 is only reachable
|
||||
over WireGuard on homesrv, this is hygiene, not an emergency — but the loopback bind
|
||||
and startup warning are cheap insurance either way, so do them regardless.
|
||||
- Finding 1's "flip the router" step should be gated harder on measurement.
|
||||
`REARCH.md`'s claim that the classifier causes weak RU queries is itself unmeasured —
|
||||
the review admits this under uncertainties, but the "repair now" ordering buries it.
|
||||
Run `eval_scenarios_test.go` against both paths **before** deciding to flip, not
|
||||
after. A 2B model on CPU may add enough latency that the classifier wins in practice
|
||||
even if it is dumber.
|
||||
- Finding 5 (doc consolidation) is what prevents finding 1 from recurring. Do the doc
|
||||
sweep as part of the model-pinning commit, not "later."
|
||||
|
||||
**Agreed without reservation:** the no-rewrite conclusion, the keep list, and the
|
||||
overall classification (healthy engineering, misaligned direction).
|
||||
Reference in New Issue
Block a user