feat(frontend): Complete account authentication loop and unified session invalidation - #218
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@fennoai /review |
Review completeSubmitted one non-blocking PR review with 3 inline findings:
|
There was a problem hiding this comment.
Review summary
Found three production integration gaps in the new authentication runtime. The isolated controller/client tests are extensive, but the app composition currently leaves protected transports and account-state cleanup disconnected from the existing application services.
Validation note: git diff --check passed. I could not execute TypeScript/Jest locally because frontend/node_modules is not installed in the mounted workspace.
There was a problem hiding this comment.
No issues found in this review. I reviewed the pinned diff through code quality, performance, security, and documentation passes. The previously reported production composition, account-state cleanup, and post-handshake WebSocket delivery gaps are addressed in 734e085. git diff --check passed; frontend lint/type/tests were not rerun because frontend/node_modules is absent.
734e085 to
fed244d
Compare
…e-alarm Keep NativeAlarmScheduler wiring while adopting the auth runtime composition from 1024XEngineer#218.
…on-foundation Keep expo-location wiring while adopting the auth runtime from 1024XEngineer#218.
) - rewrite AppRoot.tsx onto 1024XEngineer#218's AuthController/useAuth() structure; AuthenticatedScheduleRoute now renders HomeScreen (voice-enabled) instead of ScheduleCalendarScreen directly, sourcing the access token via AuthController.getAccessToken() rather than the token-free AuthViewState - drop the username-display feature (its mount points no longer exist post-1024XEngineer#218); calendar header shows accountId again - mock the assistant's native-backed modules in AppRoot's test so the suite doesn't try to load a real audio/location native module - relocate useScheduleCalendar.test.ts / ScheduleCalendarScreen.test.tsx from src/ to tests/unit/, matching 1024XEngineer#218's jest testMatch which no longer picks up colocated src/**/*.test.ts files — these were silently not running after the merge otherwise Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lient Voice used to run its own session.hello/session.ready handshake over a private WebSocketVoiceTransport, duplicating what 1024XEngineer#218's AuthenticatedWebSocketClient already does for the app's one shared connection. Collapse the two: - add onClose(listener) to AuthenticatedWebSocketClient — it previously had no way to notify callers when a ready connection dropped, which voice's "连接已断开" UI relies on; only fires for post-ready drops, not handshake failures (those already reject connect()) - thread an optional location through connect() → session.hello, since voice needs to attach coordinates to the same first frame - always include the device's IANA timezone in session.hello — it was declared in the schema and read server-side (session.py) but never actually sent by any client; the backend that consumes it (feature/client-timezone-latlong) isn't merged yet, but this unblocks it without further frontend changes once it lands - replace WebSocketVoiceTransport with AuthenticatedVoiceTransport, a thin adapter that demuxes the client's generic string|ArrayBuffer channel into the JSON-control/binary-audio split VoiceTransportPort expects; it does not close the shared connection itself — that stays owned by the auth system (AuthInvalidationCoordinator) - AssistantConversationService.connect() no longer builds/sends session.hello itself; the transport is ready by the time it resolves - drop the now-dead SessionHelloMessage/SessionHelloPayload/ SessionReadyMessage from contracts/conversation.ts — that handshake lives entirely in contracts/authWebSocket.ts now - AssistantApplicationOptions shrinks to { accountId } — access_token/ device_id/wsUrl are owned by the shared client, not voice - add test coverage for onClose, location-in-hello, and timezone-in-hello Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
) - rewrite AppRoot.tsx onto 1024XEngineer#218's AuthController/useAuth() structure; AuthenticatedScheduleRoute now renders HomeScreen (voice-enabled) instead of ScheduleCalendarScreen directly, sourcing the access token via AuthController.getAccessToken() rather than the token-free AuthViewState - drop the username-display feature (its mount points no longer exist post-1024XEngineer#218); calendar header shows accountId again - mock the assistant's native-backed modules in AppRoot's test so the suite doesn't try to load a real audio/location native module - relocate useScheduleCalendar.test.ts / ScheduleCalendarScreen.test.tsx from src/ to tests/unit/, matching 1024XEngineer#218's jest testMatch which no longer picks up colocated src/**/*.test.ts files — these were silently not running after the merge otherwise Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lient Voice used to run its own session.hello/session.ready handshake over a private WebSocketVoiceTransport, duplicating what 1024XEngineer#218's AuthenticatedWebSocketClient already does for the app's one shared connection. Collapse the two: - add onClose(listener) to AuthenticatedWebSocketClient — it previously had no way to notify callers when a ready connection dropped, which voice's "连接已断开" UI relies on; only fires for post-ready drops, not handshake failures (those already reject connect()) - thread an optional location through connect() → session.hello, since voice needs to attach coordinates to the same first frame - always include the device's IANA timezone in session.hello — it was declared in the schema and read server-side (session.py) but never actually sent by any client; the backend that consumes it (feature/client-timezone-latlong) isn't merged yet, but this unblocks it without further frontend changes once it lands - replace WebSocketVoiceTransport with AuthenticatedVoiceTransport, a thin adapter that demuxes the client's generic string|ArrayBuffer channel into the JSON-control/binary-audio split VoiceTransportPort expects; it does not close the shared connection itself — that stays owned by the auth system (AuthInvalidationCoordinator) - AssistantConversationService.connect() no longer builds/sends session.hello itself; the transport is ready by the time it resolves - drop the now-dead SessionHelloMessage/SessionHelloPayload/ SessionReadyMessage from contracts/conversation.ts — that handshake lives entirely in contracts/authWebSocket.ts now - AssistantApplicationOptions shrinks to { accountId } — access_token/ device_id/wsUrl are owned by the shared client, not voice - add test coverage for onClose, location-in-hello, and timezone-in-hello Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r screen (#229) * feat(assistant): wire voice WS transport, audio pipeline, and calendar integration - add VoiceTransportPort/WebSocketVoiceTransport for the session.hello → voice.stream.* → voice.command.result WS protocol - add AudioCapturePort/AudioPlaybackPort backed by @irvingouj/expo-audio-stream (patched via patch-package), replacing the broken @mykin-ai fork - add AssistantConversationService orchestrating one press-to-talk turn, including streaming voice.dialogue.reply text and local SQLite sync of voice.command.result via LocalScheduleWriter - add VoiceTalkButton/TempoAssistantIcon (real audio-level waveform, tap-outside-to-dismiss + stop TTS) and wire into HomeScreen alongside ScheduleCalendarScreen - add ExpoLocationProvider for session.hello lat/long Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(realtime): pass client geolocation through session handshake - thread latitude/longitude from session.hello through SessionHandshake into SessionContext (accepted but not consumed yet — no geocoding or geofencing reads them) - restore logging.basicConfig, which the root logger never had, silently swallowing existing logger.info calls app-wide - drop the temporary debug_auth stub now that #217 lands real JWT-backed auth (create_auth_router/JwtAccessTokenService), which main.py wires in directly Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(realtime): move logging.basicConfig after imports to satisfy E402 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(realtime): label session.hello geolocation as WGS84 - add coordinate_system to SessionHelloPayload (frontend + backend) and SessionContext, fixed to the literal "WGS84" - client sends the raw device coordinates as-is; no client-side conversion to GCJ-02 or any other system — whoever consumes latitude/longitude downstream decides whether/how to convert Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(assistant): adapt to the merged auth architecture (#218) - rewrite AppRoot.tsx onto #218's AuthController/useAuth() structure; AuthenticatedScheduleRoute now renders HomeScreen (voice-enabled) instead of ScheduleCalendarScreen directly, sourcing the access token via AuthController.getAccessToken() rather than the token-free AuthViewState - drop the username-display feature (its mount points no longer exist post-#218); calendar header shows accountId again - mock the assistant's native-backed modules in AppRoot's test so the suite doesn't try to load a real audio/location native module - relocate useScheduleCalendar.test.ts / ScheduleCalendarScreen.test.tsx from src/ to tests/unit/, matching #218's jest testMatch which no longer picks up colocated src/**/*.test.ts files — these were silently not running after the merge otherwise Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(assistant): converge onto the shared AuthenticatedWebSocketClient Voice used to run its own session.hello/session.ready handshake over a private WebSocketVoiceTransport, duplicating what #218's AuthenticatedWebSocketClient already does for the app's one shared connection. Collapse the two: - add onClose(listener) to AuthenticatedWebSocketClient — it previously had no way to notify callers when a ready connection dropped, which voice's "连接已断开" UI relies on; only fires for post-ready drops, not handshake failures (those already reject connect()) - thread an optional location through connect() → session.hello, since voice needs to attach coordinates to the same first frame - always include the device's IANA timezone in session.hello — it was declared in the schema and read server-side (session.py) but never actually sent by any client; the backend that consumes it (feature/client-timezone-latlong) isn't merged yet, but this unblocks it without further frontend changes once it lands - replace WebSocketVoiceTransport with AuthenticatedVoiceTransport, a thin adapter that demuxes the client's generic string|ArrayBuffer channel into the JSON-control/binary-audio split VoiceTransportPort expects; it does not close the shared connection itself — that stays owned by the auth system (AuthInvalidationCoordinator) - AssistantConversationService.connect() no longer builds/sends session.hello itself; the transport is ready by the time it resolves - drop the now-dead SessionHelloMessage/SessionHelloPayload/ SessionReadyMessage from contracts/conversation.ts — that handshake lives entirely in contracts/authWebSocket.ts now - AssistantApplicationOptions shrinks to { accountId } — access_token/ device_id/wsUrl are owned by the shared client, not voice - add test coverage for onClose, location-in-hello, and timezone-in-hello Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(assistant): request mic permission before opening the stream, make the started-wait rejectable, and dispose connection listeners Addresses fennoai's review on #229: - request permission (and check its result) before sending voice.stream.start — sending it first left an orphaned active stream server-side whenever permission was denied, since there was no stream_id yet to send voice.stream.end with; capture.start() failing after the stream is already open gets the same voice.stream.end cleanup - pair streamStartedWaiter with a rejecter so a transport error or connection close settles the voice.stream.started wait instead of leaving startTurn()/endTurn() pending forever - connect() now tracks the three connection listeners it registers (message/audio/close) and dispose() unsubscribes all of them, not just connection.close() — onClose in particular forwards straight to the shared AuthenticatedWebSocketClient and was never cleaned up - AppRoot now calls assistantApplication.dispose() when the memoized instance is replaced or the route unmounts - also clear the location race's losing setTimeout, found while adding test coverage for the above (no functional bug, just a dangling timer) - add AssistantConversationService.test.ts — this class had no test coverage before Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(assistant): gate message.ack on local write success, guard transport JSON.parse message.ack{status:"applied"} was sent unconditionally before the local SQLite write settled, contradicting AGENTS.md §6's documented ordering. Unguarded JSON.parse on server frames could also throw through the shared connection callback on a malformed frame. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
关联 Issue
Refs #194
变更概述
完成前端账户认证闭环,包括安全会话存储、认证状态恢复、登录状态路由切换、HTTP Bearer Token 注入、WebSocket 会话认证、统一失效清理、账号内存清理及相关自动化测试。
认证页面和业务展示层只消费不含 Token 的
AuthViewState;HTTP、WebSocket 和主动退出共用同一个失效协调器,避免重复实现认证清理逻辑。依赖与测试配置
frontend/package.jsonexpo-secure-store,用于 Android/iOS 安全保存认证会话。verify:ci,组合代码检查和 Android Expo export。frontend/package-lock.jsonfrontend/.env.example10.0.2.2访问宿主机服务。认证契约与领域模型
frontend/src/contracts/auth.tsaccount_id、非空 opaqueaccess_token和expires_in=3600。parseAuthErrorEnvelope()解析冻结的嵌套错误外壳。frontend/src/features/auth/domain/authSession.tsAuthSession、认证状态和无 Token 的展示状态。安全会话存储
frontend/src/features/auth/data/MemoryAuthSessionStore.tsfrontend/src/features/auth/data/SecureAuthSessionStore.tsfrontend/src/features/auth/application/AuthSessionDeletionRetrier.ts认证状态与路由闭环
frontend/src/features/auth/application/AuthController.tsfrontend/src/features/auth/presentation/AuthProvider.tsxAuthViewState和认证动作。frontend/src/app/AppRoot.tsxHTTP Token 与统一失效
frontend/src/infrastructure/network/client.tsBearer <opaque-token>。AUTH_REQUIRED/AUTH_INVALID_TOKEN触发失效。frontend/src/features/auth/application/AuthInvalidationCoordinator.tssocket → account state → session/controller清理顺序。WebSocket 会话认证
frontend/src/contracts/authWebSocket.tssession.hello、session.ready和session.error冻结契约。frontend/src/infrastructure/websocket/AuthenticatedWebSocketClient.tsdisconnected | connecting | authenticating | ready状态机。session.ready。UNAUTHENTICATED触发统一失效。connect()和close()支持幂等调用。账号状态清理与脱敏诊断
frontend/src/features/auth/application/AccountStateCleanerRegistry.tsschedule-view和reminder-runtime两个固定账号状态来源。frontend/src/features/auth/application/AuthDiagnostics.tsfrontend/src/app/authRuntime.ts固定夹具与 Fake Transport
frontend/tests/fixtures/auth/expires_in=3600。server_time=2026-08-06T03:00:00+00:00。frontend/tests/fakes/FakeAuthHttpTransport.tsfrontend/src/infrastructure/websocket/testing/FakeWebSocket.ts自动化测试
frontend/tests/contracts/authContract.test.tsfrontend/tests/integration/authClosure.integration.test.ts覆盖以下完整账户闭环:
UNAUTHENTICATED只执行一次清理。MALFORMED_MESSAGE保留会话。变更原因
Issue #194 需要建立完整、低冗余且安全的前端账户认证闭环。
本 PR 将认证契约、领域状态、持久化、HTTP、WebSocket、页面展示和失效清理拆分为明确边界,并确保:
破坏性变更
无已知破坏性变更。
验证结果
frontend/,未修改后端代码。本次不包含