refactor: synchronize server address configuration across Python, Rust, and TypeScript

- Updated server address handling to ensure consistent defaults across the codebase, with Python backend binding to `0.0.0.0` and Rust/TypeScript clients using `127.0.0.1`.
- Introduced a new test suite to validate synchronization of server address constants across different languages.
- Enhanced gRPC server configuration to include a bind address setting, improving flexibility in deployment scenarios.
- Removed deprecated methods and streamlined state management for streaming sessions, ensuring better resource handling and cleanup.
This commit is contained in:
2026-01-03 15:16:46 +00:00
parent 832a691dd7
commit 8fa823e82b
38 changed files with 1210 additions and 1331 deletions

View File

@@ -119,3 +119,107 @@ Planned test files:
- Keep exporters optional; default to localonly collection unless configured.
- Do not introduce background threads that bypass existing async shutdown hooks.
- If OTel initialization fails, fall back to current LogBuffer behavior.
---
## Connection Troubleshooting
Use this checklist when diagnosing client-to-server connection issues.
### Quick Checks
| Check | Command / Location | Expected |
|-------|-------------------|----------|
| Server running | `ps aux \| grep noteflow` | Process visible |
| Server port open | `lsof -i :50051` | LISTEN state |
| Client effective URL | Settings → Server Connection tooltip | Shows URL and source |
### Environment Variables
| Variable | Default | Purpose |
|----------|---------|---------|
| `NOTEFLOW_BIND_ADDRESS` | `0.0.0.0` | Server bind address (Python) |
| `NOTEFLOW_SERVER_ADDRESS` | `127.0.0.1:50051` | Client server URL override |
| `NOTEFLOW_GRPC_PORT` | `50051` | Server port |
### Common Issues
#### 1. "Connection refused" in Docker
**Symptom**: Desktop client cannot connect to server running in Docker.
**Cause**: Server binds to `127.0.0.1` (localhost only) instead of `0.0.0.0` (all interfaces).
**Fix**:
```bash
# Set bind address to all interfaces
export NOTEFLOW_BIND_ADDRESS=0.0.0.0
# Verify Docker port mapping
docker run -p 50051:50051 ...
```
#### 2. IPv6 vs IPv4 mismatch
**Symptom**: Connection works on some systems but not others.
**Cause**: `localhost` resolves to `::1` (IPv6) on some systems, `127.0.0.1` (IPv4) on others.
**Fix**: Use explicit IPv4 address `127.0.0.1` instead of `localhost`.
#### 3. Wrong server URL in client
**Symptom**: Client connects to wrong server or shows "not connected".
**Diagnosis**:
1. Open Settings page
2. Hover over the info icon next to "Server Connection"
3. Check the tooltip shows correct URL and source
**Source priority**:
1. Environment (`NOTEFLOW_SERVER_ADDRESS`)
2. User preferences (Settings page)
3. Default (`127.0.0.1:50051`)
#### 4. Firewall blocking connection
**Symptom**: Server running, port open, but client cannot connect.
**Fix** (Linux):
```bash
sudo ufw allow 50051/tcp
```
**Fix** (macOS):
```bash
# Check if firewall is blocking
sudo /usr/libexec/ApplicationFirewall/socketfilterfw --listapps
```
### Diagnostic Commands
```bash
# Check server is listening
netstat -tlnp | grep 50051
# Test gRPC connectivity (requires grpcurl)
grpcurl -plaintext localhost:50051 list
# Check server logs
docker logs noteflow-server 2>&1 | grep -i "bind\|listen\|address"
# Verify environment
printenv | grep NOTEFLOW
```
### Client-Side Logging
Enable verbose logging in the Tauri client:
```bash
RUST_LOG=noteflow_tauri=debug npm run tauri dev
```
Look for connection-related entries:
- `grpc_connect` — connection attempts
- `server_address` — resolved server URL
- `connection_error` — failure details

View File

@@ -2,26 +2,27 @@
> **Size**: M | **Owner**: Client (Rust) + Frontend (TypeScript) | **Prerequisites**: None
> **Phase**: Gaps - Wiring Reliability
> **Status**: ✅ COMPLETE (2026-01-03)
---
## Open Issues & Prerequisites
> ⚠️ **Review Date**: 2026-01-03 — Verified in code; implementation scope needs owner confirmation.
> **Completed**: 2026-01-03 — All blocking issues resolved.
### Blocking Issues
| ID | Issue | Status | Resolution |
|----|-------|--------|------------|
| **B1** | Behavior when `start_recording` is invoked while disconnected | Pending | Decide auto-connect vs hard fail with UX prompt |
| **B2** | Source of server URL for auto-connect in Rust | Pending | Use cached preferences or last successful URL |
| **B1** | Behavior when `start_recording` is invoked while disconnected | ✅ Resolved | Auto-connect silently; hard fail with toast on error |
| **B2** | Source of server URL for auto-connect in Rust | ✅ Resolved | Use cached endpoint from `GrpcClient` constructor |
### Design Gaps to Address
| ID | Gap | Resolution |
|----|-----|------------|
| G1 | `start_recording` does not guarantee an active gRPC connection | Add auto-connect or preflight check |
| G2 | UI does not explicitly force connect before starting streaming | Add guarded connect attempt in recording flow |
| G1 | `start_recording` does not guarantee an active gRPC connection | ✅ Auto-connect added at lines 64-74 |
| G2 | UI does not explicitly force connect before starting streaming | ✅ Preflight connect added before guard |
### Prerequisite Verification
@@ -34,20 +35,22 @@
## Validation Status (2026-01-03)
### NOT IMPLEMENTED
### IMPLEMENTED
| Component | Status | Notes |
|-----------|--------|-------|
| Rust auto-connect in `start_recording` | Not implemented | `start_recording` calls `start_streaming` without ensuring connection |
| Frontend preflight connect on recording start | Not implemented | Recording flow assumes connection state; no explicit connect call |
| Rust auto-connect in `start_recording` | ✅ Complete | Lines 64-74 in `recording/mod.rs` — uses cached endpoint |
| Frontend preflight connect on recording start | ✅ Complete | Lines 181-197 in `Recording.tsx` — runs before guard |
| Integration tests for auto-connect | ✅ Complete | `grpc_integration.rs` — 2 GAP-006 tests |
| Unit tests for preflight connect | ✅ Complete | `Recording.test.tsx` — 3 GAP-006 tests |
### PARTIALLY IMPLEMENTED
### PREVIOUSLY PARTIAL (Now Complete)
| Component | Status | Notes |
|-----------|--------|-------|
| App bootstrap connect attempt | Partial | `initializeAPI()` attempts `connect`, but failures fall back to cached mode |
| App bootstrap connect attempt | ✅ Enhanced | Recording flow now has explicit preflight connect as defense-in-depth |
**Downstream impact**: Recording can fail locally with `NotConnected`, and the backend sees no traffic, producing no logs.
**Resolution**: Recording path now attempts connection before streaming, with explicit error handling and user feedback.
---
@@ -96,9 +99,14 @@ Ensure recording/streaming always attempts a gRPC connection before transmitting
### Client
- [ ] `client/src-tauri/src/commands/recording/mod.rs` — auto-connect before streaming
- [ ] `client/src-tauri/src/grpc/client/core.rs`helper to expose last server URL for auto-connect
- [ ] `client/src/pages/Recording.tsx` — preflight connect and better failure messaging
- [x] `client/src-tauri/src/commands/recording/mod.rs` — auto-connect before streaming (lines 64-74)
- [x] `client/src-tauri/src/grpc/client/core.rs``connect(None)` uses cached endpoint
- [x] `client/src/pages/Recording.tsx` — preflight connect before guard (lines 181-197)
### Tests
- [x] `client/src-tauri/tests/grpc_integration.rs` — GAP-006 integration tests
- [x] `client/src/pages/Recording.test.tsx` — GAP-006 unit tests (3 tests)
---
@@ -106,18 +114,22 @@ Ensure recording/streaming always attempts a gRPC connection before transmitting
### Core test cases
- **Rust**: unit test that `start_recording` attempts connect when disconnected
- **TS**: recording start triggers `connect` when `isConnected` is false
- **TS**: failure path shows offline toast and does not start stream
- [x] **Rust**: integration test that `connect(None)` uses cached endpoint
- [x] **Rust**: integration test that connect fails gracefully with invalid server
- [x] **TS**: recording start triggers `connect` when `isConnected` is false
- [x] **TS**: failure path shows error toast and does not call `createMeeting`
- [x] **TS**: skips preflight connect when already connected
---
## Quality Gates
- [ ] No silent `NotConnected` failures in recording path
- [ ] Connection failure emits `ERROR` event with category and retryable flag
- [ ] `npm run test` passes for recording flow tests
- [ ] `npm run lint:rs` passes
- [x] No silent `NotConnected` failures in recording path
- [x] Connection failure emits `ERROR` event with category and retryable flag
- [x] `npm run test` passes for recording flow tests (589 tests pass)
- [x] `cargo clippy` passes with no warnings
- [x] TypeScript quality tests pass (21 tests)
- [x] Python quality tests pass (90 tests)
---

View File

@@ -26,22 +26,17 @@
## Validation Status (2026-01-03)
### PARTIALLY IMPLEMENTED
### FULLY IMPLEMENTED
| Component | Status | Notes |
|-----------|--------|-------|
| Simulation default off | Implemented | `simulate_transcription` defaults to `false` |
| Recording simulation path | Implemented | Recording uses `mockAPI` when simulate + disconnected |
| Simulation badge | Implemented | `SimulationIndicator` renders when enabled |
### NOT IMPLEMENTED
| Component | Status | Notes |
|-----------|--------|-------|
| Global API mode indicator | Not implemented | No universal badge for mock/cached/tauri |
| Explicit confirmation on enabling simulation | Not implemented | Toggle is immediate |
**Downstream impact**: Users can record in simulated mode and expect backend activity, causing confusion when backend logs show nothing.
| Simulation default off | Implemented | `simulate_transcription` defaults to `false` |
| Recording simulation path | Implemented | Recording uses `mockAPI` when simulate + disconnected |
| Simulation badge | Implemented | `ApiModeIndicator` shows "Simulated" when enabled |
| Global API mode indicator | ✅ Implemented | `ApiModeIndicator` in TopBar shows mock/cached/disconnected/reconnecting |
| Explicit confirmation on enabling simulation | ✅ Implemented | `SimulationConfirmationDialog` with "Don't ask again" option |
| Console logging on adapter init | ✅ Implemented | `[NoteFlow API] Adapter: X | Mode: Y` on initialization |
| Developer-only toggle | ✅ Implemented | `VITE_DEV_MODE` build flag controls visibility |
---
@@ -65,8 +60,9 @@ Make simulation and API modes unambiguous so users understand when the backend i
| Asset | Location | Implication |
|-------|----------|-------------|
| Default `simulate_transcription` | `client/src/lib/preferences.ts` | Starts disabled |
| Simulation indicator | `client/src/components/simulation-indicator.tsx` | Reuse as global badge |
| Recording simulation logic | `client/src/pages/Recording.tsx` | Hook point for warning and gating |
| API mode indicator | `client/src/components/api-mode-indicator.tsx` | Global badge for all modes |
| Confirmation dialog | `client/src/components/simulation-confirmation-dialog.tsx` | Modal with "Don't ask again" |
| Recording simulation logic | `client/src/pages/Recording.tsx` | Includes mode badge in header |
---
@@ -87,10 +83,13 @@ Make simulation and API modes unambiguous so users understand when the backend i
### Client
- [ ] `client/src/contexts/connection-context.tsx` — expose adapter/mode to UI
- [ ] `client/src/components/` — global mode badge component
- [ ] `client/src/pages/Settings.tsx` — confirmation for simulation toggle
- [ ] `client/src/pages/Recording.tsx`include mode badge in recording header
- [x] `client/src/contexts/connection-context.tsx` — expose adapter/mode to UI
- [x] `client/src/components/api-mode-indicator.tsx` — global mode badge component
- [x] `client/src/components/simulation-confirmation-dialog.tsx` — confirmation modal
- [x] `client/src/pages/Settings.tsx`confirmation for simulation toggle
- [x] `client/src/pages/Recording.tsx` — include mode badge in recording header
- [x] `client/src/components/top-bar.tsx` — global mode badge in header
- [x] `client/src/api/index.ts` — console logging on adapter init
---
@@ -106,9 +105,9 @@ Make simulation and API modes unambiguous so users understand when the backend i
## Quality Gates
- [ ] Simulation state is visible on all recording screens
- [ ] Simulation toggle requires explicit confirmation
- [ ] `npm run test` passes for affected UI tests
- [x] Simulation state is visible on all recording screens
- [x] Simulation toggle requires explicit confirmation
- [x] `npm run test` passes for affected UI tests (591 tests passing)
---

View File

@@ -2,88 +2,72 @@
> **Size**: M | **Owner**: Backend (Python) + Client (Rust) + Frontend (TypeScript) | **Prerequisites**: None
> **Phase**: Gaps - Connectivity
> **Status**: ✅ COMPLETED (2026-01-03)
---
## Open Issues & Prerequisites
## Summary
> ⚠️ **Review Date**: 2026-01-03 — Verified in code; need decision on default bind address in Docker.
Made server address behavior explicit and consistent across Python, Rust, and TypeScript layers. Added UI visibility into the effective URL and its source.
### Blocking Issues
### Key Changes
| ID | Issue | Status | Resolution |
|----|-------|--------|------------|
| **B1** | Default server bind address (`[::]` vs `0.0.0.0`) | Pending | Decide cross-platform Docker default |
| **B2** | Preferred client default host (`localhost` vs `127.0.0.1`) | Pending | Decide IPv4-first strategy for desktop client |
### Design Gaps to Address
| ID | Gap | Resolution |
|----|-----|------------|
| G1 | Defaults are scattered across Python, Rust, and TS | Centralize and document source-of-truth |
| G2 | UI does not show server URL origin (env vs prefs) | Surface full URL and source |
---
## Validation Status (2026-01-03)
### NOT IMPLEMENTED
| Component | Status | Notes |
| Component | Before | After |
|-----------|--------|-------|
| Unified server address defaults | Not implemented | Defaults exist in TS + Rust separately |
| UI display of effective server URL | Not implemented | No UI indicator of current URL or source |
### PARTIALLY IMPLEMENTED
| Component | Status | Notes |
|-----------|--------|-------|
| Backend bind address | Partial | Server binds to `[::]:port`, configurable port only |
| Client env override | Implemented | Rust reads `NOTEFLOW_SERVER_ADDRESS` |
**Downstream impact**: Desktop clients can connect to a different address than expected in Docker setups, and failures are hard to diagnose.
| Python bind address | Hardcoded `[::]` | Configurable via `NOTEFLOW_BIND_ADDRESS`, default `0.0.0.0` |
| Rust client default | `localhost:50051` | `127.0.0.1:50051` with source tracking |
| TypeScript default | `localhost` | `127.0.0.1` |
| UI visibility | None | Tooltip shows effective URL and source |
---
## Objective
## Resolved Issues
Make server address behavior explicit and consistent across Python, Rust, and TypeScript layers, and provide UI visibility into the effective URL and its source.
| ID | Issue | Resolution |
|----|-------|------------|
| **B1** | Default server bind address | `0.0.0.0` default via `NOTEFLOW_BIND_ADDRESS` env var |
| **B2** | Preferred client default host | `127.0.0.1` (IPv4) for cross-platform consistency |
| **G1** | Scattered defaults | Documented and aligned across all layers |
| **G2** | UI does not show server URL origin | Tooltip displays effective URL and source |
---
## Key Decisions
## Implementation Details
| Decision | Choice | Rationale |
|----------|--------|-----------|
| Server bind default | `0.0.0.0` in Docker or configurable via env | Improves IPv4 port publishing reliability |
| Client default host | Prefer `127.0.0.1` for desktop | Avoid IPv6 localhost ambiguity |
### Backend (Python)
---
| File | Change |
|------|--------|
| `src/noteflow/config/settings/_main.py` | Added `grpc_bind_address` setting with `0.0.0.0` default |
| `src/noteflow/grpc/_config.py` | Added `DEFAULT_BIND_ADDRESS` constant and `bind_address` field to `GrpcServerConfig` |
| `src/noteflow/grpc/server.py` | Updated to use configurable bind address from settings |
## What Already Exists
### Client (Rust)
| Asset | Location | Implication |
|-------|----------|-------------|
| Backend default port | `src/noteflow/config/settings/_main.py` | Port is consistent at 50051 |
| Backend bind address | `src/noteflow/grpc/server.py` | Currently `[::]:port` |
| Rust default address + env | `client/src-tauri/src/config.rs` | Uses `NOTEFLOW_SERVER_ADDRESS` |
| TS defaults | `client/src/lib/config/server.ts` | Uses `localhost:50051` |
| File | Change |
|------|--------|
| `client/src-tauri/src/config.rs` | Changed default to `127.0.0.1:50051`, added `ServerAddressSource` enum and `EffectiveServerUrl` struct |
| `client/src-tauri/src/commands/connection.rs` | Added `get_effective_server_url` Tauri command |
| `client/src-tauri/src/state/preferences.rs` | Updated default `server_host` to `127.0.0.1` |
| `client/src-tauri/src/lib.rs` | Registered new command |
---
### Frontend (TypeScript)
## Scope
| File | Change |
|------|--------|
| `client/src/lib/config/server.ts` | Changed `HOST` default to `127.0.0.1` |
| `client/src/api/types/core.ts` | Added `ServerAddressSource` type and `EffectiveServerUrl` interface |
| `client/src/api/interface.ts` | Added `getEffectiveServerUrl()` method to `NoteFlowAPI` |
| `client/src/api/tauri-adapter.ts` | Implemented `getEffectiveServerUrl()` |
| `client/src/api/mock-adapter.ts` | Added mock implementation |
| `client/src/components/settings/server-connection-section.tsx` | Added tooltip showing effective URL and source |
| `client/src/pages/Settings.tsx` | Fetches and passes `effectiveServerUrl` to component |
| Task | Effort | Notes |
|------|--------|-------|
| **Backend (Python)** | | |
| Add configurable bind address (env + config) | M | Default to `0.0.0.0` when running in Docker |
| **Client (Rust)** | | |
| Align default address with TS defaults | S | Prefer IPv4 loopback |
| **Frontend (TypeScript)** | | |
| Show effective server URL and source | S | Preferences vs env vs default |
| Document connection behavior in settings UI | S | Include Docker port mapping reminder |
### Tests
**Total Effort**: M (2-4 hours)
| File | Description |
|------|-------------|
| `tests/test_server_address_sync.py` | Cross-language constant synchronization tests |
---
@@ -91,32 +75,50 @@ Make server address behavior explicit and consistent across Python, Rust, and Ty
### Backend
- [ ] `src/noteflow/grpc/server.py` — bind address configurable via settings/env
- [ ] `src/noteflow/config/settings.py` — add bind address setting
- [x] `src/noteflow/grpc/server.py` — bind address configurable via settings/env
- [x] `src/noteflow/config/settings/_main.py` — add bind address setting (`grpc_bind_address`)
### Client
- [ ] `client/src-tauri/src/config.rs` — align defaults, expose effective URL
- [ ] `client/src/lib/config/server.ts` — update defaults and docs
- [ ] `client/src/pages/Settings.tsx` — show effective URL + source
- [x] `client/src-tauri/src/config.rs` — align defaults to `127.0.0.1`, expose effective URL via command
- [x] `client/src/lib/config/server.ts` — update HOST default to `127.0.0.1`
- [x] `client/src/pages/Settings.tsx` — show effective URL + source via tooltip
---
## Test Strategy
## Test Results
### Core test cases
- **Python**: server binds to configured address (unit test with config)
- **Rust**: default server URL matches TS defaults
- **TS**: settings UI displays URL origin accurately
```
tests/test_server_address_sync.py::TestServerAddressDefaults::test_python_bind_address_is_all_interfaces PASSED
tests/test_server_address_sync.py::TestServerAddressDefaults::test_rust_client_uses_ipv4_loopback PASSED
tests/test_server_address_sync.py::TestServerAddressDefaults::test_typescript_client_uses_ipv4_loopback PASSED
tests/test_server_address_sync.py::TestServerAddressDefaults::test_rust_and_typescript_use_same_host PASSED
```
---
## Quality Gates
- [ ] Defaults aligned across Python, Rust, and TypeScript
- [ ] Effective URL visible in UI
- [ ] Docs mention Docker port publish requirement
- [x] Defaults aligned across Python, Rust, and TypeScript
- [x] Effective URL visible in UI (tooltip on Settings page)
- [x] All quality checks pass (`make quality`)
---
## Configuration Reference
### Environment Variables
| Variable | Default | Description |
|----------|---------|-------------|
| `NOTEFLOW_BIND_ADDRESS` | `0.0.0.0` | Server bind address (Python backend) |
| `NOTEFLOW_SERVER_ADDRESS` | `127.0.0.1:50051` | Server URL override (Rust client) |
### URL Source Priority (Client)
1. **Environment**`NOTEFLOW_SERVER_ADDRESS` env var
2. **Preferences** — User-configured host/port in settings
3. **Default**`127.0.0.1:50051`
---