ESP32_Serial_Swiss_Army_Knife/docs/agent/design-decisions.md

# Durable design constraints and decisions

Only constraints supported by implementation or current project documentation belong here. When original rationale is unknown, the entry describes the observable constraint without inventing intent.

## One broker mediates all production serial transports

**Decision:** USB CDC, WebSocket, and role-`user` SSH access UART1 through `session_broker`; transports do not independently own the serial service.

**Rationale/evidence:** The broker is initialized after the serial service and all transport implementations connect broker clients. It is the normal serial RX consumer and TX gate. Project documentation requires one writer and multiple observers.

**Consequence for future changes:** New serial transports must become broker clients. Do not bypass writer checks or consume `serial_service` RX directly. `serial_service_start()` is not idempotent, so admission code must reconcile check/start races as the existing transports do. Broker paths enter serial-service APIs while holding the broker mutex; preserve that lock order and do not call back into the broker while holding the serial state mutex. Preserve binary transparency and avoid in-band ownership control.

**Relevant files:** `src/session_broker.{h,c}`, `src/serial_service.{h,c}`, `src/usb_cdc_transport.c`, `src/web_serial_transport.c`, `src/ssh_transport.c`

## Slow clients are isolated by bounded per-client storage

**Decision:** UART RX is drained and copied into independent bounded broker output streams; a full observer loses only its own copy.

**Rationale/evidence:** `session_broker` accounts per-client dropped bytes instead of blocking fan-out. The roadmap records slow-client isolation as a project-wide constraint.

**Consequence for future changes:** Do not replace fan-out with a blocking shared queue. Any added transport must tolerate partial/no-progress reads and expose drop/backpressure counters.

**Relevant files:** `src/session_broker.c`, `src/session_broker.h`, `docs/roadmap.md`

## Physical UART ownership and logical writer ownership remain separate

**Decision:** `rs232_port_owner` controls whether diagnostics or the serial service may manipulate UART/MAX3243 hardware; the broker separately controls which connected client may write.

**Rationale/evidence:** The code has explicit `NONE`, `PHASE0`, `SERVICE`, and `FAULT` hardware states plus broker client/writer IDs.

**Consequence for future changes:** A writer lease never authorizes direct UART/GPIO access. Active hardware tests must claim `PHASE0`; the production service must claim `SERVICE`. Boot-time static-safe GPIO setup and service-owned static-mode restoration are explicit exceptions to this cooperative gate. Ambiguous cleanup must keep the transceiver safe and require reboot rather than clearing fault casually.

**Relevant files:** `src/rs232_port_owner.{h,c}`, `src/rs232_hw_test.c`, `src/serial_service.c`, `src/session_broker.c`

## Resource IDs are generation-safe

**Decision:** Broker clients, SSH/WebSocket slots, queued admin work, user principals/accounts, Wi-Fi working configuration, and HTTPS lifecycle intent carry domain-specific generations or random stable IDs to reject stale references, slot reuse, and lost updates.

**Rationale/evidence:** Broker IDs encode slot generation; transports track slot generations; admin tokens include session/slot generation; user principal currentness includes account ID and authentication generation; user/Wi-Fi web editors carry optimistic generations; HTTPS snapshots expose lifecycle generation.

**Consequence for future changes:** Preserve transport-slot, account-authentication/database, Wi-Fi working-config, and HTTPS lifecycle generations as distinct concepts. Validate tokens immediately before side effects and discard late work after disconnect/reuse/revocation. Existing-account web mutations must also compare stable user ID so deletion/recreation of the same username cannot retarget stale work.

**Relevant files:** `src/session_broker.{h,c}`, `src/ssh_transport.c`, `src/web_serial_transport.c`, `src/admin_ssh_console.c`, `src/user_database.{h,c}`, `src/user_admin_service.{h,c}`, `src/wifi_manager.{h,c}`, `src/web_server.{h,c}`

## UART0 is the physical recovery authority

**Decision:** UART0 remains independent of UART1 and networking. Initial administrator bootstrap and explicit unavailable-user-database recovery are restricted to UART0.

**Rationale/evidence:** `main.c` configures UART0 separately; command policy and user handlers deny these operations remotely. README/roadmap identify UART0 as the trusted recovery console.

**Consequence for future changes:** Network failures or credential corruption must not remove UART0 recovery. Do not expose bootstrap/recovery through web or admin SSH without an explicit security redesign.

**Relevant files:** `src/main.c`, `src/admin_ssh_console.c`, `src/user_console.c`, `docs/roadmap.md`

## Admin SSH and user SSH are different routes

**Decision:** A role-`user` SSH session becomes a broker serial client. A role-`admin` session enters the administration console and never obtains a broker client/writer lease.

**Rationale/evidence:** Role routing is explicit after SSH authentication. The administrative shell is intended for command execution, not multiplexed serial data.

**Consequence for future changes:** Do not silently give administrators both streams or infer that higher privilege means UART1 ownership. A route-switch feature would require explicit protocol, lifecycle, and authorization design.

**Relevant files:** `src/ssh_transport.c`, `src/admin_ssh_console.{h,c}`, `src/session_broker.c`

## One dispatcher executes the canonical command registry

**Decision:** UART0 and admin SSH submit complete lines to one fixed queue; one task is the sole caller of `esp_console_run()`.

**Rationale/evidence:** The implementation treats ESP-IDF console execution as non-reentrant and removes the need for separate remote command implementations.

**Consequence for future changes:** Register one canonical handler rather than creating a second SSH dispatcher. Long commands/prompts block all administration, so keep handlers bounded or explicitly asynchronous. Preserve output routing and remote principal checks.

**Relevant files:** `src/admin_ssh_console.c`, `src/main.c`, `src/console_input.c`, all `src/*_console.c`

## Selected self-affecting admin SSH actions use bounded deferred control

**Decision:** Admin SSH `exit`, remote reboot, SSH stop/disconnect, and host-key rotate/reset are deferred until command state and administration/transport application buffers appear drained, with a ten-second limit and short final delay.

**Rationale/evidence:** `admin_ssh_console` has a separate bounded control task and pending-action state. The check is a best-effort application-buffer heuristic, not peer-delivery confirmation. User account mutations and their immediate revocation calls do not use this path.

**Consequence for future changes:** Actions that would invalidate their own SSH transport should integrate with deferred control when acknowledgement preservation matters. Prevent new input while an action is pending, keep the wait bounded, and do not describe it as guaranteed delivery.

**Relevant files:** `src/admin_ssh_console.c`, `src/system_console.c`, `src/ssh_console.c`, `src/ssh_transport.c`

## Authentication uses copied principals and fail-safe currentness checks

**Decision:** Network sessions retain secret-free copied principals. Account mutations invalidate generations/IDs; after commit, the command layer requests best-effort targeted transport revocation, while ongoing currentness checks are authoritative.

**Rationale/evidence:** `user_database` issues principals without secrets; web/SSH check currentness during admission and active sessions. Mutating console paths call transport revocation hooks.

**Consequence for future changes:** Do not retain pointers to database records or treat login as permanently authoritative. New authenticated sessions/transports must revalidate at admission, before sensitive input, and periodically or on relevant events. Database mutation APIs alone do not perform transport notification, and notification failure must not roll back an already committed mutation.

**Relevant files:** `src/user_database.{h,c}`, `src/user_console.c`, `src/web_server.c`, `src/web_serial_transport.c`, `src/ssh_transport.c`

## Browser authentication uses bounded explicit sessions

**Decision:** Browser access uses a same-origin login/logout flow and a fixed RAM session table rather than HTTP Basic. Raw opaque tokens exist only in host-only secure cookies; firmware storage retains token digests, copied principals, monotonic expiry, and generation-safe slot identity. State-changing requests require a session-bound CSRF token and exact Origin validation.

**Rationale/evidence:** Explicit logout and account switching cannot reliably invalidate browser-managed HTTP Basic credentials. Exact browser-session references also allow logout of one session without revoking another session for the same account.

**Consequence for future changes:** Preserve digest-only storage, the two-per-account/eight-global capacity, exact-session WebSocket binding, and current-principal checks. Do not expose tokens, CSRF values, ticket values, or internal references in logs/snapshots. New mutation endpoints must use the body-backed in-place URL-form parser's 512-byte/10-unique-field bounds, parse closed schemas, and revalidate the exact admin session immediately before typed side effects. Browser failure/close paths must clear entered and generated secret material rather than replaying it after a reload.

**Relevant files:** `src/web_session.{h,c}`, `src/web_server.c`, `src/web_serial_transport.c`, `src/web_admin_transport.c`

## Browser admin shell is separate from browser serial

**Decision:** An administrator may keep the browser serial WebSocket alive while independently opening one admin-only WebSocket frontend for the canonical command dispatcher. Terminal switching changes only browser visibility and focus; the admin route never becomes a broker client.

**Rationale/evidence:** This preserves a browser-held writer lease while giving full canonical administrative command parity without a generic HTTP command endpoint.

**Consequence for future changes:** Do not multiplex admin command bytes into `/ws/serial`, and do not close or release the serial route as a side effect of mode switching, settings navigation, or popover display. HTTPD remains the owner of WebSocket send/close calls; the web-admin task only queues bounded work. Self-affecting HTTPS commands must use deferred drain control.

**Relevant files:** `src/web_admin_transport.{h,c}`, `src/admin_ssh_console.{h,c}`, `src/web_ui.c`, `src/web_server.c`

## Browser writer transfer is atomic and generation-safe

**Decision:** Guided writer assignment compares the expected current writer and validates the exact generation-safe target under the broker mutex before making one atomic ownership change.

**Rationale/evidence:** A browser dialog can become stale while open. Unconditional force assignment could overwrite a newer legitimate lease or release ownership when its target disconnected.

**Consequence for future changes:** Use `session_broker_compare_exchange_writer()` for stale UI/API transfers. Opening or hovering a writer control must never mutate ownership, and target/current conflicts must leave the current lease unchanged.

**Relevant files:** `src/session_broker.{h,c}`, `src/web_server.c`, `src/web_ui.c`

## User mutations have one serialized typed boundary

**Decision:** `user_admin_service` owns typed account/password/role/key mutations for console and web callers. Its recursive `admin_command_gate` critical region includes the optimistic snapshot check and database commit; a committed mutation is followed by independent best-effort web and SSH revocation attempts.

**Rationale/evidence:** Browser requests can race one another and canonical shell commands. Database generation plus stable user ID reject stale editors and username delete/recreate races, while the shared gate prevents caller-specific check-then-mutate interleaving. Revocation cannot be made atomic with the NVS commit, so principal currentness remains authoritative.

**Consequence for future changes:** Route new ordinary user mutations through this service instead of calling `user_database` directly. Do not roll back or report a committed mutation as failed solely because a transport notification failed. Preserve final-admin checks, remote self-generated-password restrictions, secret wiping, and generation/user-ID conflict reporting.

**Relevant files:** `src/user_admin_service.{h,c}`, `src/user_database.{h,c}`, `src/user_console.c`, `src/admin_command_gate.{h,c}`, `src/web_server.c`, `src/ssh_transport.c`

## Display configuration writers share the administration gate

**Decision:** Typed browser display Apply/Save/Load/Defaults/Reset operations and console display-writer commands serialize through the recursive `admin_command_gate` for the complete RAM and persistence operation.

**Rationale/evidence:** Browser handlers and canonical console frontends can mutate the same local-UI working configuration and NVS record concurrently. Serializing only individual lower-level calls could allow interleaved apply/save/load/reset sequences and inconsistent final state.

**Consequence for future changes:** Keep all new display configuration writers under the same gate, including any read-modify-write and rollback sequence. Do not hold the gate for read-only status or rendering work, and do not conflate it with the display framebuffer/I2C mutex.

**Relevant files:** `src/web_server.c`, `src/local_ui_console.c`, `src/local_ui_config.{h,c}`, `src/local_status_ui.{h,c}`, `src/admin_command_gate.{h,c}`

## Guided Wi-Fi editing uses exact-generation compare-and-swap

**Decision:** Browser Wi-Fi reads return configuration metadata plus `secret_set` flags, never PSKs. Each typed edit applies a complete validated working-config copy only if its expected nonzero generation remains current, and Save persists exactly the expected generation under the same writer serialization.

**Rationale/evidence:** Multiple browser editors, console changes, and lifecycle controls can update RAM configuration concurrently. A conventional read/modify/write or copy-then-save sequence could overwrite a newer secret or persist a generation the user never reviewed.

**Consequence for future changes:** Keep credential-bearing copies tightly scoped and wiped. Add config writers under the writer mutex and advance generation without wraparound; mismatch or exhaustion must fail closed. Stale browser forms must reload without replay and clear entered secrets.

**Relevant files:** `src/wifi_manager.{h,c}`, `src/wifi_config.{h,c}`, `src/wifi_console.c`, `src/web_server.c`, `src/web_ui.c`

## HTTPS lifecycle and post-material refresh are serialized

**Decision:** HTTPS start, stop, and TLS refresh share a lifecycle mutex and generation-tagged desired-running intent. Certificate/material replacement always proceeds to a TLS refresh. Teardown disables new admin-transport HTTPD calls, tracks calls already in progress, retains a server whose stop failed, and keeps incomplete post-stop finalization pending for retry before another start.

**Rationale/evidence:** Browser-shell commands can tear down their own transport while console commands race a restart or replace persisted TLS material. HTTPD-owned queued work must finish before its handle or the admin transport's static state can be reused.

**Consequence for future changes:** Do not start a second server around a retained handle, bypass lifecycle serialization, or make post-material refresh optional after persistence. A newer explicit lifecycle intent must win over an older refresh. Finalize timed-out detach state only after HTTPD destruction, and retry a failed finalizer before attaching a replacement server.

**Relevant files:** `src/web_server.{h,c}`, `src/web_console.c`, `src/web_admin_transport.{h,c}`, `src/admin_ssh_console.{h,c}`

## Security material and configuration use bounded, versioned NVS records

**Decision:** Application settings, users, and identities use separate fixed/versioned NVS blobs. Serial, Wi-Fi, mDNS-hostname, and local-UI working edits are RAM-only until explicitly saved. User mutations and HTTPS/SSH identity changes commit directly as part of the operation. Invalid ordinary configuration generally selects RAM defaults without erasing storage; malformed security material fails closed and needs explicit reset.

**Rationale/evidence:** Serial, Wi-Fi, local UI, web security, users, and SSH security each validate schema/size and own their namespace. User/security mutations build and validate candidate state before committing it; security modules avoid silently replacing an established identity. The live user database remains internal while its 5,360-byte candidate is a persistent PSRAM-preferred allocation with internal fallback and is wiped after every transaction.

**Consequence for future changes:** Add schema versions and transactional candidate validation. Do not overwrite unknown records automatically; provide explicit migration/reset behavior. Preserve the distinct persistence contracts: explicit save/load/default/reset for working configuration and per-blob commit-before-live-install for user and identity mutation. Keep candidate ownership mutex-local and wipe/free it on initialization or recovery failure. Recheck external-buffer staging in the flash/NVS implementation when upgrading from the pinned ESP-IDF 5.5 baseline. Pre-bootstrap legacy credential rotation spans `web_sec/material` and `user_db/database`, is not cross-namespace atomic, and relies on boot reconciliation after interruption.

**Relevant files:** `src/serial_config.c`, `src/wifi_config.c`, `src/mdns_config.c`, `src/mdns_service.c`, `src/local_ui_config.c`, `src/web_security.c`, `src/user_database.c`, `src/ssh_security.c`

## NVS is persistence, not a physical security boundary

**Decision:** The current firmware stores Wi-Fi credentials, recovery credentials, and TLS/SSH private keys in unencrypted application NVS. The reserved NVS-key partition does not enable encryption.

**Rationale/evidence:** `partitions.csv`, README security notes, and current code show no NVS-encryption setup. Original rationale for deferring encryption is outside the implementation; the observable limitation is explicit.

**Consequence for future changes:** Do not claim resistance to flash extraction. Avoid increasing stored secret exposure. Enabling encryption requires migration/recovery planning, not just changing the partition table.

**Relevant files:** `partitions.csv`, `README.md`, `src/web_security.c`, `src/ssh_security.c`, `src/wifi_config.c`

## Wi-Fi callbacks enqueue; the manager owns policy

**Decision:** ESP event callbacks copy bounded event data into the Wi-Fi manager queue. A permanent manager task performs driver operations, profile/AP policy, deadlines, reconciliation, and station mDNS announcement transitions. mDNS initializes at most once, remains allocated across transient disconnects while its component handlers withdraw/re-enable the STA interface, and treats failure as nonfatal.

**Rationale/evidence:** Callback paths avoid blocking, NVS, and policy work. Manager deadlines consult authoritative driver/netif state so dropped events are recoverable.

**Consequence for future changes:** Keep callbacks short and nonblocking. Add state transitions to the manager rather than directly invoking Wi-Fi policy from consoles, UI, or callbacks. Preserve queue-drop observability.

**Relevant files:** `src/wifi_manager.{h,c}`, `src/wifi_config.{h,c}`, `src/mdns_service.{h,c}`, `src/mdns_config.{h,c}`

## Optional local UI cannot become a core dependency

**Decision:** The OLED/display may fail without stopping serial, UART0, USB, or networking. The UI consumes copied snapshots and calls public APIs; it never parses CLI output or joins the broker.

**Rationale/evidence:** `main.c` logs display failures and continues. `local_status_ui` collects snapshots before display frames and exposes limited confirmed controls.

**Consequence for future changes:** Keep OLED/I2C work bounded and outside service locks. Do not put credentials or core ownership into UI state. A missing display must remain nonfatal.

**Relevant files:** `src/main.c`, `src/local_display.{h,c}`, `src/local_status_ui.c`, `src/local_ui_config.c`

## Hardware and library access has designated owners

**Decision:** The serial task owns UART1 while active, `local_display` owns I2C/framebuffer access, the SSH owner task owns runtime wolfSSH contexts/calls after caller-side library initialization, and the console dispatcher alone runs registered commands.

**Rationale/evidence:** These constraints are enforced by module structure, mutex/task assertions, and transport indirection. Original rationale varies; the observable effect is serialized library/hardware access.

**Consequence for future changes:** Cross-task requests should use existing queues/public APIs. Do not make post-initialization wolfSSH calls, mutate display frames, or run console handlers from arbitrary tasks.

**Relevant files:** `src/serial_service.c`, `src/local_display.c`, `src/ssh_transport.c`, `src/admin_ssh_console.c`

## Software cryptography settings are a validated concurrency workaround

**Decision:** wolfSSL ESP32 AES/SHA acceleration is disabled, and HTTPS uses software AES for PSRAM-backed TLS records. Internal task stacks are retained where cache-disable safety matters.

**Rationale/evidence:** Root `CMakeLists.txt` disables wolfSSL hardware crypto. The roadmap reports a reproduced watchdog stall involving mbedTLS external-RAM hardware-AES DMA, uncoordinated mbedTLS/wolfSSL hardware locks, and a successful software-crypto concurrency retest; no standalone execution record is checked in.

**Consequence for future changes:** Do not remove these definitions as a performance cleanup. Any re-enablement needs target-hardware concurrency testing with simultaneous USB, WebSocket, SSH, and serial traffic plus watchdog/stack telemetry.

**Relevant files:** `CMakeLists.txt`, `src/CMakeLists.txt`, `docs/roadmap.md`, relevant `sdkconfig.defaults` crypto settings

## Embedded web assets are checked-in generated artifacts

**Decision:** Vendored xterm assets are compressed and embedded ahead of the normal firmware build; `src/web_assets_data.c` is compiled directly.

**Rationale/evidence:** `src/CMakeLists.txt` lists generated data as a source, and `web_assets/SOURCES.md` documents pinned versions, hashes, and deterministic gzip inputs.

**Consequence for future changes:** Edit authored web UI separately. Changes to its inline bootstrap loader must update the hard-coded CSP hash atomically and preserve the response security policy. When dependency assets change, follow the documented provenance/generation process and review generated diffs; do not hand-edit arrays or regenerate assets during unrelated work.

**Relevant files:** `web_assets/SOURCES.md`, `web_assets/generate_embedded_assets.py`, `src/web_assets_data.{h,c}`, `src/web_ui.c`