Add review organization

.claude/agent-memory/chief-architect.md

@@ -0,0 +1,93 @@
+# Agent Memory: Chief Architect (L5/L6) — NIGHTWATCH
+
+Distilled system model after the first full review cycle (2026-07-12, branch
+claude/install-review-org-37y4ck, 61 commits, repo age 5.5 months). Update — don't replace —
+on future passes. Deliverables written: docs/review/40-architecture.md, 50-risk-register.md.
+
+## System model (one paragraph)
+
+Voice-controlled autonomous observatory. Hub-and-spoke around `nightwatch/orchestrator.py`
+(3,446-line god-file). Three layers: `nightwatch/` core, `services/` (22 capability modules),
+`voice/` (STT/TTS/Wyoming + 87-tool LLM schema catalog). Dependency direction is clean
+(services never import nightwatch). The defining property as of 2026-07: **breadth-first
+scaffolding, near-zero integration** — components are individually well unit-tested but the
+running system does not exist. Single author (Tim Hennessey = THOClabs, 97% of commits).
+
+## Verified load-bearing facts (re-check these first on any future review)
+
+- **Entry point crashes:** `main.py:308,325` `setup_logging(level=)` vs param `log_level`.
+  One-line fix; if fixed, most "dormant" findings go live (adopt security report's Tier A/B rule).
+- **Emergency roof close broken twice:** (1) `RoofController.__init__` (roof_controller.py:484-531)
+  never sets `self._gpio` (only unreferenced `setup_rain_sensor_interrupt()` does, :1061);
+  `_run_motor:848` AttributeErrors, swallowed. (2) `monitor.py:1535` calls async `close()` without
+  await from sync `_close_enclosure_safely`. Both masked by tests that patch `_run_motor`.
+- **Unwired in production (zero construction sites in main.py/orchestrator.py, all verified):**
+  LLMClient, VoicePipeline, Wyoming servers, AIServices/services-nlp, SafetyInterlock,
+  EmergencyResponse, SafeStateHandler, EventBus, CommandQueue, ToolChain, ToolRegistry
+  (~4,100 dead lines in telescope_tools.py, repo's largest file), execute_cancellable/_active_commands.
+- **Live safety boundary is exactly:** services/safety_monitor loop → orchestrator
+  `_on_safety_change`/`_on_safety_veto` cancel of the single `_active_context` (set only by
+  tool_executor.py:351,406) + inline park/close in `_safe_shutdown`/`end_session`/
+  `emergency_shutdown` + SAFE-004 watchdog heartbeat path (safety_monitor only; other services'
+  watchdog heartbeats never called → UNKNOWN forever).
+- **Tool surface:** TOOL_PARAM_MODELS has exactly 18 keys (mount/catalog/ephemeris/weather/
+  safety/session). 87 schemas defined in voice/tools. voice_pipeline.py:2086 imports nonexistent
+  `nightwatch.telescope_tools` → tools=None → LLM never gets schemas → VOX-003 inert on real traffic.
+  LLMClient.requires_confirmation's 4 critical tool names aren't in the registry (double-dead).
+- **Protocol mismatches (M3):** LX200Client park/stop/unpark are sync; orchestrator awaits them.
+  SafetyMonitor lacks `is_safe` property; emergency_response/watchdog expect roof.get_state()
+  (real API: `state` property). MagicMock tests hide all of it.
+- **CI cannot fail:** 12× continue-on-error + `|| true`/`|| echo` on every real check in ci.yml.
+  Test suite polluted by module-level `sys.modules['numpy']=MagicMock()` in test_piper_service.py:27
+  and test_whisper_service.py:35 (order-dependent failures). Coverage 48.25% vs unenforced 60/80.
+  pytest.ini wins; pyproject.toml pytest block dead. Real bugs found by suite: power_restore 300s
+  hang; double `_save_session_log` (orchestrator.py:2059+2391).
+- **Config truths:** LLMConfig (config.py:436-479) has NO api_key/endpoint/backend fields —
+  00-inventory.md:291 was wrong. Keys read from env in llm_client.py:454,572. SafetyConfig is the
+  threshold source of truth; constants.py has drifted flat copies (unreferenced). SAFETY env
+  allowlist (config.py:90) is real, empty, well-tested — the repo's best control.
+- **Ecowitt parser fails open** (defaults 70°F/dry on garbled JSON); SAFE-002 secondary rain
+  sensor unimplemented but `require_secondary_rain_sensor=True` default with no config surface.
+- **Network defaults:** Wyoming 0.0.0.0:10300/10301, enabled=True, no auth, unbounded audio buffer;
+  PDU admin/admin + SNMP "private". docker prod exposes 10300; systemd wyoming unit ExecStart's
+  `voice.wyoming_server` module does not exist. README's `nightwatch.cli` does not exist.
+  `pyindi-client~=2.0.8` has never existed on PyPI (install fails atomically).
+- **aiohttp 3.13.5 in uv.lock: 11 CVEs, fix 3.14.1** (as of 2026-07).
+
+## Git/history facts
+
+- Two identities, one person; Claude has 2 docs commits. Main branch stale vs review branch.
+- Stale-since-2026-01-20 (verified): alpaca, enclosure, encoder, ephemeris, indi, simulators,
+  voice/stt, voice/tts, voice/wyoming, services/nlp (historian's §4.1 missed the voice/nlp set;
+  voice/ recent activity is only voice/tools/telescope_tools.py). alerts/meteor stale since 01-28.
+- No tags/releases. 1 merge commit. Commit discipline high (ARCH-/SAFE-/HWS-/VOX- refs) but specs
+  are marked "Complete" at code-exists, not wired (ToolChain Step 267; SAFE-001 claim vs C1).
+
+## Judgment calls I made (keep consistent next time)
+
+- Adopted security auditor's rule: "unwired" is NOT a mitigation for anything the deploy artifacts
+  intend to run (entry fix is one line).
+- Ranked R1 (roof) and R2 (CI/no-feedback-loop) co-equal at 20; R2 is the enabling risk.
+- Treated the review corpus itself as partial bus-factor mitigation.
+- 437 broad-except sites framed as the root-cause pattern (with power_manager.py:309 bare except
+  returning success) rather than as individual findings.
+- meteor_tracking hopi_circles/lexicon_prayers: editorial/cultural-review item only, no code defect.
+
+## Where reports were wrong (recorded in 40-architecture.md §6)
+
+- 00-inventory: LLMConfig fields (wrong); SAFE-002/004 attributed to safety_interlock.py (wrong —
+  they live in monitor.py:553 / watchdog.py:505); tests/hardware "skipped in CI" (wrong — collect
+  0 items, they're manual scripts).
+- 10-history: abandoned-zones list under-counted (missed voice/* subdirs + services/nlp).
+- No domain analyst's concrete code claim was found wrong; 31-quality confirmed all flags.
+
+## Next-review checklist
+
+1. Is main.py fixed and does `--dry-run` run? If yes, re-tier all Tier-B findings.
+2. `grep -n "_gpio" services/enclosure/roof_controller.py` — still uninitialized in __init__?
+3. Does monitor.py `_close_enclosure_safely` await? Is `_action_callback` assigned?
+4. ci.yml: count continue-on-error; is unit-tests job gated? Is mypy nightwatch/ gated?
+5. Does voice_pipeline._get_tools resolve? TOOL_PARAM_MODELS key count vs live handlers?
+6. Any wire-or-delete triage done on ToolRegistry/EventBus/CommandQueue/EmergencyResponse?
+7. Tags created? Second contributor? PRs to main?
+8. aiohttp bumped? pyindi-client pin fixed? pip-audit gating?

.claude/agent-memory/domain-analyst-voice-nlp.md

@@ -0,0 +1,104 @@
+# Agent Memory: Domain Analyst — Voice & NLP (voice/, services/nlp/)
+
+Durable notes for future reviews of NIGHTWATCH. Written 2026-07-12 after first full
+domain review. Update this file (don't replace wholesale) as new reviews confirm,
+refute, or extend these findings.
+
+## Architecture facts (high confidence, file:line verified)
+
+- **The voice/NLP subsystem is built but not wired into the running app.**
+  `nightwatch/__init__.py:50-57` has the `from services.nlp import (...)` line
+  literally commented out ("to avoid circular deps"). `nightwatch/voice_pipeline.py`
+  reimplements its own `STTInterface`/`TTSInterface` (lines ~1458-1659) instead of
+  reusing `voice/stt/whisper_service.py:WhisperSTT` or
+  `voice/tts/piper_service.py:PiperTTS`, and its `TTSInterface.synthesize()` is a
+  hardcoded mock (`_generate_mock_audio`, silent WAV) — Piper is never actually
+  called from the main pipeline. `voice/wyoming/startup.py:start_wyoming_servers()`
+  and `services/ai_services.py:AIServices` (the only code that assembles
+  `ConversationContext`/`ClarificationService`/`SuggestionService`/etc.) are each
+  only called from their own module, `examples/v05_ai_demo.py`, or `tests/` — never
+  from `nightwatch/main.py` or `nightwatch/orchestrator.py`.
+  **Before trusting any "data flow" narrative about voice control in this repo,
+  grep for actual call sites — the docstrings/comments describe an architecture
+  that isn't fully assembled yet.**
+
+- **`services/ai_services.py`** (top-level file directly under `services/`, not in
+  any subdirectory) is the de facto integration facade for `services/nlp/*` but was
+  not explicitly assigned to any domain in the 00-inventory decomposition. Whoever
+  reviews "Astronomy & Hardware Services" or does synthesis should know it exists
+  and is currently a dead end (only used by `examples/v05_ai_demo.py` + tests).
+
+- **Every file in `voice/stt`, `voice/tts`, `voice/wyoming`, and all six
+  `services/nlp/*.py` files has exactly ONE commit**, all dated 2026-01-20 between
+  05:29-06:29 UTC. Zero commits since (as of 2026-07-12, ~171 days). The L2
+  git-historian's "abandoned zones" list (`docs/review/10-history.md` §4.1) did
+  NOT include voice/ or services/nlp — but by the same "no commits in 165+ days"
+  criterion it uses for alpaca/enclosure/encoder/ephemeris/indi/simulators, this
+  entire domain qualifies too. Worth checking if future historian passes catch this.
+
+- **Wyoming protocol servers (`voice/wyoming/stt_server.py`,
+  `voice/wyoming/tts_server.py`) have zero authentication anywhere**, bind
+  `0.0.0.0` by default (`nightwatch/config.py:346-349,411-414`), and are enabled by
+  default (`wyoming_enabled: bool = True`). No TLS, no token, no allowlist in
+  `voice/wyoming/protocol.py` at all. This is consistent with the general
+  Wyoming/Home-Assistant LAN-trust ecosystem model but there's no code-level
+  mitigation or documented caveat in this repo.
+
+- **`WhisperSTT.transcribe()` hardcodes `confidence=0.9`** in all code paths
+  (`voice/stt/whisper_service.py:454,465,592`), regardless of backend. This makes
+  the Wyoming STT server's "Step 317" confidence-threshold filtering
+  (`voice/wyoming/stt_server.py`, default threshold 0.6) permanently inert. Tests
+  (`tests/unit/test_whisper_service.py:800`) assert `confidence == 0.9`, so this
+  is an accepted stub, not an oversight anyone will "just fix" without prompting.
+
+- **`services/nlp/clarification.py`'s "SAFETY_CONFIRMATION" ambiguity type is a
+  conversational nicety, not a safety interlock.** It matches literal substrings
+  ("emergency", "abort", "park", "close roof", etc.) in `DANGEROUS_ACTIONS` and
+  asks a yes/no question. The real safety enforcement lives in
+  `nightwatch/safety_interlock.py` (Core Orchestration & Safety domain). Any
+  synthesis-level report should NOT conflate these two — they are unconnected.
+
+- **Naming convention across `services/nlp/*.py`:** every submodule follows the
+  same shape — dataclasses + Enum types, a `logging.getLogger("NIGHTWATCH.<Name>")`
+  logger, a service class with public methods, and a `get_<name>()` factory backed
+  by a module-level singleton (`_default_*: Optional[...] = None`). All six
+  singletons are process-wide with no session/user key — a real risk if NIGHTWATCH
+  ever needs concurrent sessions.
+
+- **Two different code-style zones inside this domain:** `voice/stt/*.py` and
+  `voice/tts/*.py` use bare `print()` for all diagnostics (no `logging` at all).
+  `voice/wyoming/*.py` and `services/nlp/*.py` use `logging.getLogger(...)`
+  consistently. If reviewing again, check whether this has been unified.
+
+## Test coverage facts
+
+- Strong, well-mocked unit tests exist for: `test_whisper_service.py` (1162 lines),
+  `test_piper_service.py` (1017 lines), `test_wyoming_protocol.py` (867 lines —
+  protocol/dataclass serialization ONLY), and all six `services/nlp` submodules
+  (`test_clarification.py`, `test_conversation_context.py`,
+  `test_session_narrator.py`, `test_sky_describer.py`, `test_suggestions.py`,
+  `test_user_preferences.py`, ~500-700 lines each). This contradicts a naive
+  "young repo = low test coverage" assumption for this specific domain — the NLP
+  side is actually well tested for pure logic.
+- **Zero test coverage found for the actual network server classes**
+  (`WyomingSTTServer`, `WyomingTTSServer`, `WyomingManager`,
+  `start_wyoming_servers`) — confirmed via repo-wide grep, no hits under `tests/`.
+  This is exactly where the unbounded-buffer-growth and silent-exception-swallowing
+  issues live. High-value gap to flag again if it persists in future reviews.
+
+## Gotchas for future analysts of this repo
+
+- Don't assume `voice/requirements.txt` dependencies are only used inside `voice/`
+  — `webrtcvad` is declared there but actually imported by
+  `nightwatch/voice_pipeline.py` (Core Orchestration domain), not by anything
+  under `voice/`.
+- `services/ai_services.py` sits at `services/` top level, outside any
+  subdirectory-based domain in the 00-inventory decomposition — easy to miss when
+  scoping a domain review strictly by directory list.
+- When checking "is X wired up," grep for the actual constructor call
+  (`ClassName(`) or function call site across the whole repo excluding `tests/`
+  and `examples/` — docstrings and `__init__.py` comments in this repo are
+  sometimes aspirational (see `nightwatch/__init__.py:50-57`).
+- Git blame/log per-file is fast and decisive for "is this maintained" questions;
+  `git log --all -- <path> | wc -l` plus `git log -1 --format=%ai --all -- <path>`
+  gave crisp, citable evidence for the "built in one hour, day one" finding.

.claude/agents/chief-architect.md

@@ -0,0 +1,24 @@
+---
+name: chief-architect
+description: L5-L6 Synthesis. Reads every review report, spot-checks the code, and produces the architecture document and prioritized risk register. Use after all analysts and auditors complete.
+tools: Read, Glob, Grep, Bash, Write
+model: inherit
+memory: project
+color: purple
+---
+
+You are the chief architect (L5-L6) of a repository review organization. Everything below you has reported; your job is synthesis and judgment. You never modify source code. You may write exactly two report files - docs/review/40-architecture.md and docs/review/50-risk-register.md - plus files in your own agent memory directory.
+
+MANDATORY inputs: every file in docs/review/ (00, 10, all 20-domain-*, 30, 31). Spot-check the actual code wherever reports conflict or a claim carries major weight - you are the fact-checker of last resort. Where two reports disagree, resolve the disagreement in the code and record which report was wrong.
+
+docs/review/40-architecture.md:
+1. System overview: what this software is and how it is shaped, one page, no fluff
+2. Module map: domains, their boundaries, and dependency direction (ASCII or Mermaid diagram)
+3. Data flow: the 2-3 most important end-to-end paths through the system
+4. Design decisions inferred from the code, each with evidence, and whether it still serves the project
+5. Coupling and boundary violations worth naming
+
+docs/review/50-risk-register.md:
+Top 10 risks max, ranked by impact x likelihood. Each entry: risk, evidence (file paths, report references), blast radius, smallest credible mitigation, suggested owner-level (quick fix / project / strategic). Draw from ALL reports - security, quality, history (bus factor and abandonment are risks too).
+
+Check your agent memory for prior architectural understanding of this repo; update it afterward with the distilled system model so future reviews start smarter.

.claude/agents/domain-analyst.md

@@ -0,0 +1,23 @@
+---
+name: domain-analyst
+description: L3 Deep dive. Analyzes ONE assigned domain of the codebase in depth - modules, data flow, invariants, external dependencies. Spawn one instance per domain, in parallel, during a full-repo review. The task prompt must name the assigned domain and its directories.
+tools: Read, Glob, Grep, Bash, Write
+model: sonnet
+memory: project
+color: green
+---
+
+You are a senior domain analyst (L3) in a repository review organization. Each invocation assigns you exactly ONE domain (named in your task prompt, with its directories). Stay inside it; note cross-domain touchpoints without wandering into them. You never modify source code. You may write exactly one report file: docs/review/20-domain-<slug>.md (slug = your assigned domain, lowercased and hyphenated), plus files in your own agent memory directory.
+
+Read docs/review/00-inventory.md and 10-history.md first. Then produce your report covering:
+
+1. Responsibility: what this domain does, in two sentences a new engineer would understand
+2. Key modules: each important file/class/function with path and one-line role
+3. Data flow: how data enters, transforms, and leaves this domain (trace a representative request/operation end to end)
+4. External dependencies: libraries, services, other domains it calls, and the contracts assumed
+5. Invariants and conventions: implicit rules the code depends on (ordering, locking, schema shape, error contracts)
+6. MATRIX FLAGS - two mandatory subsections the cross-cutting auditors will consume:
+   - "Security observations": anything touching auth, input parsing, secrets, network, filesystem, or deserialization
+   - "Quality observations": test coverage impressions, error-handling gaps, dead code suspicions, complexity hotspots
+
+Rules: every claim cites file:line where useful. Check your agent memory for patterns seen in prior reviews of this repo, and update it afterward with durable learnings (architecture facts, gotchas, invariants). Do not report speculation as fact.

.claude/agents/executive-scribe.md

@@ -0,0 +1,22 @@
+---
+name: executive-scribe
+description: L7 Board report. Distills the entire review into an executive summary and updates CLAUDE.md so every future session inherits the understanding. Use as the final phase of a full-repo review.
+tools: Read, Glob, Grep, Write, Edit
+model: inherit
+color: orange
+---
+
+You are the executive scribe (L7) of a repository review organization - the last mile between a pile of excellent reports and durable institutional understanding. You never modify source code. You may write docs/review/60-executive-summary.md and create or edit CLAUDE.md at the repository root. Nothing else.
+
+MANDATORY inputs: every file in docs/review/. Do not introduce new findings; you distill.
+
+docs/review/60-executive-summary.md (one page, board-level):
+1. What this system is, in three sentences
+2. Overall health assessment with a one-line verdict
+3. Top 5 risks (from the risk register, in the architect's priority order)
+4. Top 5 recommendations with rough effort sizing
+5. Pointers: table of contents of docs/review/ with one line per report
+
+CLAUDE.md update - add or refresh a clearly delimited section:
+<!-- BEGIN REPO-REVIEW (generated) --> ... <!-- END REPO-REVIEW (generated) -->
+containing: the distilled system map (domains + one-liners), verified build/run/test/lint commands, conventions and invariants future agents must respect, danger zones (files where extra care is required and why), and the review date. Preserve all existing human-written CLAUDE.md content outside your markers exactly as-is. Keep your section under ~120 lines - it loads into every future session, so every line must earn its context cost.