From 4d03783236ef189dc35ae51f25bd2cdd8655482d Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 15:22:22 -0500 Subject: [PATCH 01/23] feat(coach): sign in with ChatGPT to use your ChatGPT plan instead of an API key Adds OpenAI's official "Sign in with ChatGPT" plan usage for open-source apps as a coach provider. Dynamic client registration (no secret), PKCE + state + nonce, loopback callback on 127.0.0.1 in an in-app browser, tokens in the keychain with first-unlock access, serialized rotating refresh, revoke on sign-out. Requests go through the one provider POST to the public Responses API with stream:true/store:false; system input becomes developer. Billing errors (usage limit, not eligible) stop the request and never fall back to an API key. --- PRIVACY.md | 4 +- android/app/src/main/AndroidManifest.xml | 6 + guides/AI_COACH.md | 24 +- lib/coach/coach_chatgpt.dart | 554 +++++++++++++++++++++++ lib/coach/coach_config.dart | 36 +- lib/coach/coach_engine.dart | 30 +- lib/state/app_state.dart | 3 +- lib/ui2/screens/coach.dart | 501 ++++++++++++++------ test/coach_chatgpt_test.dart | 353 +++++++++++++++ test/russian_review_findings_test.dart | 2 + 10 files changed, 1362 insertions(+), 151 deletions(-) create mode 100644 lib/coach/coach_chatgpt.dart create mode 100644 test/coach_chatgpt_test.dart diff --git a/PRIVACY.md b/PRIVACY.md index 8e71cbdf5..7f98edbf9 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -161,7 +161,9 @@ configure: - **AI Coach** — if you enable this feature and supply your own API key, summaries of your data are sent to the AI provider you configure (by default, OpenAI) to generate coaching responses. Off by default and - requires your own API key. + requires your own API key, or signing in with ChatGPT, in which case the + same data goes to OpenAI under your ChatGPT account. The sign-in tokens are + kept in the device keychain/keystore and sent only to OpenAI. - **Health app integration** — if you enable it, the App can write derived daily metrics to Apple Health or Google Health Connect, which are controlled by your device's own OS-level health app, not by us. The App can also *read* diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index dac711e32..c9b27cee1 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -344,5 +344,11 @@ + + + + diff --git a/guides/AI_COACH.md b/guides/AI_COACH.md index 0cb5f1765..b6216f1a5 100644 --- a/guides/AI_COACH.md +++ b/guides/AI_COACH.md @@ -14,7 +14,29 @@ the provider you configure. Before enabling the feature, check that provider's data retention and training policies — or point the app at a model you host yourself. -## Setup +## Use your ChatGPT plan + +If you pay for ChatGPT (Plus, Pro and similar plans), you can skip the API +key: open **AI Coach → settings**, tap **ChatGPT**, then **Continue with +ChatGPT** and sign in. Eligible coach, briefing and journal requests then use +the usage included in your ChatGPT plan instead of API billing. Tap **List +models** and pick one, then **Save**. + +- This uses OpenAI's official + [Sign in with ChatGPT](https://developers.openai.com/siwc/token-sharing-open-source) + for open-source apps. OpenStrap never sees your password, and never charges + for it. +- Set a limit, or disconnect OpenStrap, under + [ChatGPT settings → Usage](https://chatgpt.com/settings/usage). The + **Manage usage** button in AI settings goes there. +- When the limit is reached the coach says so and stops; it never quietly + switches to an API key. +- Your questions and the rows the coach reads go to OpenAI under your ChatGPT + account, exactly as with an OpenAI API key. +- **Sign out of ChatGPT** revokes the sign-in. The coach then stays off until + you sign in again or pick another provider. + +## Setup with an API key diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart new file mode 100644 index 000000000..64ad5c12a --- /dev/null +++ b/lib/coach/coach_chatgpt.dart @@ -0,0 +1,554 @@ +// "Sign in with ChatGPT" plan usage for the coach: the user's own ChatGPT plan +// pays for eligible requests instead of an API key. Open-source apps register +// themselves dynamically — no client secret, nothing of ours in the loop. The +// tokens live in the platform keychain beside the BYOK key and are only ever +// sent to auth.openai.com and api.openai.com. +// https://developers.openai.com/siwc/token-sharing-open-source + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'dart:math'; + +import 'package:clock/clock.dart'; +import 'package:crypto/crypto.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; +import 'package:http/http.dart' as http; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:uuid/uuid.dart'; + +const kChatGptIssuer = 'https://auth.openai.com'; + +/// Both the OAuth `resource` and the Responses API base requests go to. +const kChatGptApiBase = 'https://api.openai.com/v1'; +const kChatGptUsageUrl = 'https://chatgpt.com/settings/usage'; +const kChatGptPlanScope = 'chatgpt.tokens.use.direct'; + +// ponytail: endpoints copied from $kChatGptIssuer/.well-known/openid-configuration +// rather than discovered per sign-in; switch to discovery if they ever move. +const _authorizeUrl = '$kChatGptIssuer/api/accounts/authorize'; +const _tokenUrl = '$kChatGptIssuer/api/accounts/oauth/token'; +const _revokeUrl = '$kChatGptIssuer/api/accounts/oauth/revoke'; +const _scopes = + 'openid profile email offline_access resource.invoke $kChatGptPlanScope'; +const _agentName = 'OpenStrap Edge'; + +/// A sign-in or token failure with a message fit to show the user. +class ChatGptException implements Exception { + final String message; + ChatGptException(this.message); + @override + String toString() => message; +} + +/// The authorize URL. A null [clientId] registers this app on the account +/// (`dynamic_agent_client`); a saved one signs back in to that registration. +Uri chatGptAuthorizeUri({ + required String? clientId, + required String hostId, + required String redirectUri, + required String state, + required String nonce, + required String codeVerifier, + String? idTokenHint, +}) => + Uri.parse(_authorizeUrl).replace(queryParameters: { + 'client_id': clientId ?? 'dynamic_agent_client', + if (clientId == null) 'agent_name_hint': _agentName, + 'ext_agent_host_id': hostId, + 'id_token_hint': ?idTokenHint, + 'response_type': 'code', + 'redirect_uri': redirectUri, + 'scope': _scopes, + 'resource': kChatGptApiBase, + 'state': state, + 'nonce': nonce, + 'code_challenge_method': 'S256', + 'code_challenge': chatGptCodeChallenge(codeVerifier), + }); + +String chatGptCodeChallenge(String verifier) => + base64Url.encode(sha256.convert(ascii.encode(verifier)).bytes).replaceAll('=', ''); + +/// The ID token's `sub` and `email` once its claims check out. +/// +/// The signature is not verified: the token comes straight from the token +/// endpoint over TLS in a PKCE code exchange, where OIDC Core §3.1.3.7(6) +/// allows TLS server validation in its place. iss/aud/exp/nonce still are. +({String sub, String email}) chatGptIdentity( + String idToken, { + required String clientId, + required String nonce, + DateTime? now, +}) { + final Map claims; + try { + final parts = idToken.split('.'); + claims = jsonDecode( + utf8.decode(base64Url.decode(base64Url.normalize(parts[1]))), + ) as Map; + } catch (_) { + throw ChatGptException('ChatGPT returned an unreadable sign-in.'); + } + final aud = claims['aud']; + final exp = claims['exp']; + final ok = claims['iss'] == kChatGptIssuer && + (aud == clientId || (aud is List && aud.contains(clientId))) && + claims['nonce'] == nonce && + exp is num && + exp * 1000 > (now ?? clock.now()).millisecondsSinceEpoch && + claims['sub'] is String; + if (!ok) throw ChatGptException('ChatGPT returned a sign-in that did not check out.'); + return (sub: claims['sub'] as String, email: (claims['email'] ?? '').toString()); +} + +/// The final `response` of a streamed Responses call. This route only +/// streams; the whole body is buffered and the completed response — which +/// carries the full output — is handed to the same parser as non-streamed +/// replies. Anything short of `response.completed` is a failure. +Map chatGptCompletedResponse(String sse) { + for (final line in const LineSplitter().convert(sse)) { + if (!line.startsWith('data:')) continue; + final Object? event; + try { + event = jsonDecode(line.substring(5).trim()); + } catch (_) { + continue; // `[DONE]` and keep-alives + } + if (event is! Map) continue; + switch (event['type']) { + case 'response.completed': + final r = event['response']; + if (r is Map) return r.cast(); + case 'response.failed' || 'error': + final err = (event['response'] is Map ? event['response']['error'] : null) ?? + event['error'] ?? + event; + throw ChatGptException(chatGptErrorMessage( + null, + err is Map ? err['code']?.toString() : null, + err is Map ? err['message']?.toString() : null, + )); + case 'response.incomplete': + throw ChatGptException('ChatGPT stopped before finishing the reply.'); + } + } + throw ChatGptException('The ChatGPT reply ended early. Try again.'); +} + +/// A failed plan-usage request, worded for the user. Billing errors stop the +/// request — they never fall back to another way of paying. +String chatGptErrorMessage(int? status, String? code, String? detail) { + switch (code) { + case 'subscription_sharing_usage_limit_exceeded': + return 'Usage limit reached. Review your plan or this app’s limit in ' + 'ChatGPT settings: $kChatGptUsageUrl'; + case 'subscription_sharing_user_not_eligible': + return 'This ChatGPT account can’t use its plan in other apps.'; + case 'subscription_sharing_usage_unavailable' || + 'subscription_sharing_user_unavailable': + return 'ChatGPT plan usage is unavailable right now. Try again later.'; + case 'subscription_sharing_unsupported_capability': + return 'ChatGPT plan usage doesn’t support part of this request' + '${detail == null ? '' : ': $detail'}'; + case 'subscription_sharing_invalid_user': + return 'ChatGPT didn’t accept this sign-in. If you disconnected ' + 'OpenStrap in ChatGPT settings, sign in again.'; + } + if (status == 401) { + return 'ChatGPT didn’t accept this sign-in. Sign in again from AI settings.'; + } + if (status == 503) { + return 'ChatGPT plan usage is unavailable right now. Try again later.'; + } + final what = detail ?? code ?? 'unknown error'; + return status == null ? 'ChatGPT error: $what' : 'ChatGPT error ($status): $what'; +} + +/// [chatGptErrorMessage] for a non-200 body, which is either a Responses +/// `{"error": {...}}` or an admission `{"detail": "..."}`. +String chatGptHttpError(int status, String body) { + try { + final j = jsonDecode(body); + final e = j is Map ? j['error'] : null; + if (e is Map) { + return chatGptErrorMessage( + status, e['code']?.toString(), (e['param'] ?? e['message'])?.toString()); + } + if (j is Map && j['detail'] != null) { + return chatGptErrorMessage(status, null, j['detail'].toString()); + } + } catch (_) {} + return chatGptErrorMessage( + status, null, body.length > 200 ? body.substring(0, 200) : body); +} + +/// The Responses body this route accepts, from the coach's normal one: it +/// must stream, and rejects `system` input items (developer is the same thing +/// here). Sampling params are already never sent on Responses. +Map chatGptRequest(Map body) => { + ...body, + 'stream': true, + 'store': false, + 'input': [ + for (final item in body['input'] as List) + if (item is Map && item['role'] == 'system') + {...item, 'role': 'developer'} + else + item, + ], + }; + +class ChatGptSession { + final String sub, email, idToken, accessToken, refreshToken, scope; + final DateTime expiresAt; + + const ChatGptSession({ + required this.sub, + required this.email, + required this.idToken, + required this.accessToken, + required this.refreshToken, + required this.scope, + required this.expiresAt, + }); + + /// Signed in is not enough: the user can sign in and decline plan usage. + bool get sharing => scope.split(' ').contains(kChatGptPlanScope); + + Map toJson() => { + 'sub': sub, + 'email': email, + 'id_token': idToken, + 'access_token': accessToken, + 'refresh_token': refreshToken, + 'scope': scope, + 'expires_at': expiresAt.millisecondsSinceEpoch, + }; + + static ChatGptSession fromJson(Map j) => ChatGptSession( + sub: j['sub'] as String, + email: j['email'] as String, + idToken: j['id_token'] as String, + accessToken: j['access_token'] as String, + refreshToken: j['refresh_token'] as String, + scope: j['scope'] as String, + expiresAt: DateTime.fromMillisecondsSinceEpoch(j['expires_at'] as int), + ); +} + +/// One ChatGPT account per install. The issued client id and host id are kept +/// across sign-outs — OpenAI wants a returning sign-in to reuse both. +class ChatGptAuth extends ChangeNotifier { + ChatGptAuth({http.Client? client}) : _client = client; + + static const _kClientId = 'coach_chatgpt_client_id'; + static const _kHostId = 'coach_chatgpt_host_id'; + static const _kPresent = 'coach_chatgpt_present'; + static const _kSession = 'coach_chatgpt_session'; // secure storage + + // Same first-unlock reasoning as CoachConfig's key: background relaunches + // happen while the phone is locked. + static const _apple = IOSOptions(accessibility: KeychainAccessibility.first_unlock); + static const _macos = MacOsOptions(accessibility: KeychainAccessibility.first_unlock); + final FlutterSecureStorage _secure = const FlutterSecureStorage(); + final http.Client? _client; + + ChatGptSession? _session; + bool _unreadable = false; + HttpServer? _pending; + Future? _refreshing; + + ChatGptSession? get session => _session; + bool get signedIn => _session != null; + bool get sharing => _session?.sharing ?? false; + + /// Signed in, but the keychain could not be read (locked phone). Fixes + /// itself on the next foreground [load]. + bool get unreadable => _unreadable; + + Future _http(Future Function(http.Client) f) async { + final c = _client ?? http.Client(); + try { + return await f(c); + } finally { + if (_client == null) c.close(); + } + } + + Future load({bool trusted = false}) async { + final prefs = await SharedPreferences.getInstance(); + final present = prefs.getBool(_kPresent) ?? false; + try { + final raw = await _secure.read(key: _kSession, iOptions: _apple, mOptions: _macos); + if (raw != null && raw.isNotEmpty) { + _session = ChatGptSession.fromJson(jsonDecode(raw) as Map); + _unreadable = false; + } else if (trusted || !present) { + _session = null; + _unreadable = false; + if (present) await prefs.setBool(_kPresent, false); + } else { + _unreadable = true; + } + } catch (_) { + _unreadable = present && _session == null; + } + notifyListeners(); + } + + Future _store(ChatGptSession? s) async { + final prefs = await SharedPreferences.getInstance(); + if (s == null) { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } else { + final raw = jsonEncode(s.toJson()); + try { + await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); + } catch (_) { + // A leftover item from a previous install (see CoachConfig.save). + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); + } + } + await prefs.setBool(_kPresent, s != null); + _session = s; + _unreadable = false; + notifyListeners(); + } + + static String _random() { + final r = Random.secure(); + return base64Url.encode(List.generate(32, (_) => r.nextInt(256))).replaceAll('=', ''); + } + + /// Runs the browser sign-in. [open] shows the authorize page and must keep + /// this app alive while it is up (an in-app browser), because the redirect + /// lands on a listener on this device's loopback. Returns true for a first + /// registration, which is when the welcome note is due. + Future signIn(Future Function(Uri) open) async { + final prefs = await SharedPreferences.getInstance(); + final hostId = prefs.getString(_kHostId) ?? 'urn:uuid:${const Uuid().v4()}'; + await prefs.setString(_kHostId, hostId); + final savedClientId = prefs.getString(_kClientId); + final server = _pending = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + try { + final redirectUri = 'http://127.0.0.1:${server.port}/auth/callback'; + final state = _random(), nonce = _random(), verifier = _random(); + final uri = chatGptAuthorizeUri( + clientId: savedClientId, + hostId: hostId, + redirectUri: redirectUri, + state: state, + nonce: nonce, + codeVerifier: verifier, + idTokenHint: _session?.idToken, + ); + // Listening BEFORE the browser opens: the redirect can beat [open]'s + // own return. ignore() only keeps a failed open from leaving this + // future's error unhandled; it is still awaited below. + final callback = _callback(server, state)..ignore(); + if (!await open(uri)) throw ChatGptException('Could not open the browser.'); + final q = await callback.timeout( + const Duration(minutes: 10), + onTimeout: () => throw ChatGptException('Sign-in timed out. Try again.'), + ); + if (q['error'] != null) { + throw ChatGptException(q['error'] == 'access_denied' + ? 'Sign-in was cancelled.' + : 'ChatGPT sign-in failed: ${q['error_description'] ?? q['error']}'); + } + final issued = q['client_id']; + if (savedClientId != null && issued != null && issued != savedClientId) { + throw ChatGptException('ChatGPT returned a different app registration.'); + } + final clientId = savedClientId ?? issued; + if (clientId == null || q['code'] == null) { + throw ChatGptException('ChatGPT sign-in returned no code.'); + } + // Saved BEFORE the exchange, so a retry after a failed one reuses it. + await prefs.setString(_kClientId, clientId); + final Map t; + try { + t = await _token({ + 'grant_type': 'authorization_code', + 'client_id': clientId, + 'code': q['code']!, + 'code_verifier': verifier, + 'redirect_uri': redirectUri, + }); + } on _TokenError catch (e) { + throw ChatGptException( + 'ChatGPT sign-in failed (${e.code ?? e.status}). Try again.'); + } + final idToken = t['id_token'] as String?; + if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); + final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); + await _store(_sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken)); + return savedClientId == null; + } finally { + if (identical(_pending, server)) _pending = null; + await server.close(force: true); + } + } + + /// Ends a [signIn] still waiting on the browser. Nothing tells the app when + /// the user simply closes the in-app browser, so this is the way out. + void cancelSignIn() => _pending?.close(force: true); + + /// The first callback carrying our [state]. Anything else that reaches the + /// loopback port is turned away without ending the sign-in. + static Future> _callback(HttpServer server, String state) async { + await for (final req in server) { + final q = req.uri.queryParameters; + final ours = req.uri.path == '/auth/callback' && q['state'] == state; + req.response + ..statusCode = ours ? 200 : 400 + ..headers.contentType = ContentType.html + ..write(ours + ? '' + '

Done. You can close ' + 'this page and go back to OpenStrap.

' + : 'Bad request'); + await req.response.close(); + if (ours) return q; + } + throw ChatGptException('Sign-in was cancelled.'); + } + + ChatGptSession _sessionFrom(Map t, + {required String sub, required String email, required String idToken}) => + ChatGptSession( + sub: sub, + email: email, + idToken: idToken, + accessToken: t['access_token'] as String, + refreshToken: t['refresh_token'] as String, + scope: (t['scope'] ?? '').toString(), + expiresAt: clock.now().add(Duration(seconds: (t['expires_in'] as num?)?.toInt() ?? 3600)), + ); + + Future> _token(Map form) async { + final resp = await _http((c) => c + .post(Uri.parse(_tokenUrl), body: {...form, 'resource': kChatGptApiBase}) + .timeout(const Duration(seconds: 30))); + Object? j; + try { + j = jsonDecode(resp.body); + } catch (_) {} + if (resp.statusCode == 200 && j is Map && j['access_token'] is String) { + return j.cast(); + } + final err = j is Map ? j['error']?.toString() : null; + throw _TokenError(resp.statusCode, err); + } + + /// A current access token, refreshed when it is within five minutes of + /// expiry. Concurrent callers share one refresh — the refresh token rotates, + /// and a second refresh with the old one would be `refresh_token_reused`. + Future accessToken() { + final s = _session; + if (s == null) { + return Future.error(ChatGptException(_unreadable + ? 'Your ChatGPT sign-in could not be read while the phone was locked. Open the app and try again.' + : 'Sign in with ChatGPT first.')); + } + if (s.expiresAt.isAfter(clock.now().add(const Duration(minutes: 5)))) { + return Future.value(s.accessToken); + } + return _refreshing ??= _refresh(s).whenComplete(() => _refreshing = null); + } + + Future _refresh(ChatGptSession s) async { + final prefs = await SharedPreferences.getInstance(); + final clientId = prefs.getString(_kClientId); + if (clientId == null) throw ChatGptException('Sign in with ChatGPT again.'); + final Map t; + try { + t = await _token({ + 'grant_type': 'refresh_token', + 'client_id': clientId, + 'refresh_token': s.refreshToken, + }); + } on _TokenError catch (e) { + if (!e.terminal) { + throw ChatGptException('Could not renew the ChatGPT sign-in (${e.status}). Try again.'); + } + await _store(null); + throw ChatGptException('Your ChatGPT sign-in has ended. Sign in again from AI settings.'); + } catch (e) { + // A network hiccup is not a revoked session — the tokens stay. + throw ChatGptException('Could not reach ChatGPT to renew the sign-in: $e'); + } + final next = _sessionFrom( + {'refresh_token': s.refreshToken, 'scope': s.scope, ...t}, + sub: s.sub, + email: s.email, + idToken: (t['id_token'] as String?) ?? s.idToken, + ); + await _store(next); + return next.accessToken; + } + + /// Revokes the refresh token and forgets the session. Returns false when + /// revocation could not be confirmed; the tokens are cleared regardless, and + /// the user can still disconnect the app in ChatGPT settings. + Future signOut() async { + final s = _session; + final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); + var revoked = s == null; + if (s != null && clientId != null) { + try { + final r = await _http((c) => c.post(Uri.parse(_revokeUrl), body: { + 'token': s.refreshToken, + 'token_type_hint': 'refresh_token', + 'client_id': clientId, + }).timeout(const Duration(seconds: 15))); + revoked = r.statusCode == 200; + } catch (_) {} + } + await _store(null); + return revoked; + } + + /// Model slugs this account may use, in the server's order. + Future> listModels() async { + final token = await accessToken(); + final resp = await _http((c) => c.get( + Uri.parse('$kChatGptApiBase/models'), + headers: {'Authorization': 'Bearer $token'}, + ).timeout(const Duration(seconds: 20))); + if (resp.statusCode != 200) { + throw ChatGptException(chatGptHttpError(resp.statusCode, resp.body)); + } + final j = jsonDecode(resp.body); + final models = j is Map ? j['models'] : null; + return [ + if (models is List) + for (final m in models) + if (m is Map && m['visibility'] == 'list' && m['slug'] is String) + m['slug'] as String, + ]; + } + + @visibleForTesting + Future debugSetSession(ChatGptSession? s) => _store(s); +} + +class _TokenError implements Exception { + final int status; + final String? code; + _TokenError(this.status, this.code); + + /// The refresh token itself is dead, as opposed to a transient failure. + bool get terminal => + const { + 'invalid_grant', + 'invalid_refresh_token', + 'token_expired', + 'refresh_token_expired', + 'refresh_token_invalidated', + 'refresh_token_reused', + }.contains(code); +} diff --git a/lib/coach/coach_config.dart b/lib/coach/coach_config.dart index df5986dd1..0ead1d9db 100644 --- a/lib/coach/coach_config.dart +++ b/lib/coach/coach_config.dart @@ -8,6 +8,8 @@ import 'package:flutter/services.dart' show PlatformException; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:shared_preferences/shared_preferences.dart'; +import 'coach_chatgpt.dart'; + /// The origin (`scheme://host[:port]`) [url] resolves to, or the trimmed /// string itself when it doesn't parse as a URI with a host. /// @@ -139,6 +141,7 @@ class CoachConfig extends ChangeNotifier { static const _kApi = 'coach_api'; static const _kKey = 'coach_api_key'; // secure storage static const _kTimeoutSeconds = 'coach_timeout_seconds'; + static const _kUseChatGpt = 'coach_use_chatgpt'; /// A local model's first response includes Ollama/LM Studio loading the /// whole model into memory, which alone can pass two minutes on modest @@ -189,6 +192,17 @@ class CoachConfig extends ChangeNotifier { final FlutterSecureStorage _secure = const FlutterSecureStorage(); + CoachConfig({ChatGptAuth? chatgpt}) : chatgpt = chatgpt ?? ChatGptAuth() { + this.chatgpt.addListener(notifyListeners); + } + + /// "Sign in with ChatGPT". When [usesChatGpt], requests go to OpenAI's + /// Responses API on the user's ChatGPT plan instead of the BYOK endpoint; + /// the BYOK settings are kept untouched for switching back. + final ChatGptAuth chatgpt; + bool _useChatGpt = false; + bool get usesChatGpt => _useChatGpt; + String _baseUrl = defaultBaseUrl; CoachApi _api = CoachApi.defaultFor(defaultBaseUrl); String _model = ''; @@ -200,7 +214,7 @@ class CoachConfig extends ChangeNotifier { /// user can fix by pasting one — this one fixes itself on the next unlocked /// read, and telling them to set a key up again would be wrong. bool _keyUnreadable = false; - bool get keyUnreadable => _keyUnreadable; + bool get keyUnreadable => _useChatGpt ? chatgpt.unreadable : _keyUnreadable; /// True while it is still unknown whether a key is stored — an install that /// predates the marker, read while the keychain was unavailable. Not shown to @@ -255,7 +269,7 @@ class CoachConfig extends ChangeNotifier { } String get baseUrl => _baseUrl; - CoachApi get api => _api; + CoachApi get api => _useChatGpt ? CoachApi.responses : _api; String get model => _model; /// The saved, user-configurable timeout — meaningful only for a local /// endpoint (see [requestTimeout]), but kept intact and readable regardless @@ -284,11 +298,13 @@ class CoachConfig extends ChangeNotifier { /// Narrower than [isLocalEndpoint]; see [isPrivateCoachHost]. bool get isPrivateEndpoint => isPrivateCoachHost(apiBase); - bool get configured => - (hasKey || isLocalEndpoint) && _baseUrl.isNotEmpty && _model.isNotEmpty; + bool get configured => _useChatGpt + ? chatgpt.sharing && _model.isNotEmpty + : (hasKey || isLocalEndpoint) && _baseUrl.isNotEmpty && _model.isNotEmpty; /// Normalised base, no trailing slash. String get apiBase { + if (_useChatGpt) return kChatGptApiBase; var b = _baseUrl.trim(); while (b.endsWith('/')) { b = b.substring(0, b.length - 1); @@ -300,7 +316,11 @@ class CoachConfig extends ChangeNotifier { /// device unlocked — the only condition under which an EMPTY read is real /// evidence about what is stored. Future load({bool trusted = false}) async { + // Its own keychain item, read alongside the key rather than ahead of it so + // one hung read cannot hold up the other. + final chatgptLoad = chatgpt.load(trusted: trusted).catchError((_) {}); final prefs = await SharedPreferences.getInstance(); + _useChatGpt = prefs.getBool(_kUseChatGpt) ?? false; _baseUrl = prefs.getString(_kBaseUrl) ?? defaultBaseUrl; _api = switch (prefs.getString(_kApi)) { 'responses' => CoachApi.responses, @@ -388,6 +408,7 @@ class CoachConfig extends ChangeNotifier { _keyUnreadable = marker == true; _keyUndetermined = marker == null; } + await chatgptLoad; notifyListeners(); } @@ -396,7 +417,7 @@ class CoachConfig extends ChangeNotifier { /// otherwise, so it is safe on every resume, which is exactly when a phone /// that was locked during a background relaunch becomes readable again. Future refreshKeyOnResume() async { - if (!_keyUnreadable && !_keyUndetermined) return; + if (!_keyUnreadable && !_keyUndetermined && !chatgpt.unreadable) return; await load(trusted: true); } @@ -409,8 +430,13 @@ class CoachConfig extends ChangeNotifier { String? model, String? apiKey, int? timeoutSeconds, + bool? useChatGpt, }) async { final prefs = await SharedPreferences.getInstance(); + if (useChatGpt != null) { + _useChatGpt = useChatGpt; + await prefs.setBool(_kUseChatGpt, useChatGpt); + } if (baseUrl != null) { final previousOrigin = coachEndpointOrigin(_baseUrl); _baseUrl = baseUrl.trim().isEmpty ? defaultBaseUrl : baseUrl.trim(); diff --git a/lib/coach/coach_engine.dart b/lib/coach/coach_engine.dart index fed12ddbd..6dbb1d1ce 100644 --- a/lib/coach/coach_engine.dart +++ b/lib/coach/coach_engine.dart @@ -24,6 +24,7 @@ import '../data/local_repository.dart'; import '../models/payloads.dart' show todayHeadlineOf; import 'coach_actions.dart'; import 'coach_chat_completions.dart'; +import 'coach_chatgpt.dart'; import 'coach_config.dart'; import 'coach_db.dart'; import 'coach_prompt.dart'; @@ -686,11 +687,16 @@ class CoachEngine { final model = body['model'] as String? ?? ''; final api = config.api; final apiBase = config.apiBase; + final chatgpt = config.usesChatGpt; + // Replayed reasoning is tied to who paid for it, so a ChatGPT-plan turn + // never replays an API-key turn's output on the same base, or vice versa. + final replayBase = chatgpt ? '$apiBase#chatgpt' : apiBase; try { body = switch (api) { - CoachApi.responses => CoachResponses.request(body, apiBase: apiBase), + CoachApi.responses => CoachResponses.request(body, apiBase: replayBase), CoachApi.chatCompletions => CoachChatCompletions.request(body), }; + if (chatgpt) body = chatGptRequest(body); // Recent Claude models reject sampling params with a 400, on Anthropic's // own endpoint and through any pass-through provider alike. Strip them for // exactly those model versions; older Claude models and every other @@ -714,24 +720,32 @@ class CoachEngine { 'time range, or start a new chat.', ); } + final bearer = chatgpt + ? await config.chatgpt.accessToken() + : (config.hasKey ? config.apiKey : null); final resp = await c .post( Uri.parse('$apiBase/${api.endpoint}'), headers: { - if (config.hasKey) 'Authorization': 'Bearer ${config.apiKey}', + if (bearer != null) 'Authorization': 'Bearer $bearer', 'content-type': 'application/json', }, body: payload, ) .timeout(config.requestTimeout); if (resp.statusCode != 200) { - throw CoachException( - 'Provider error (${resp.statusCode}): ${_briefErr(resp.body)}', - ); + throw CoachException(chatgpt + ? chatGptHttpError(resp.statusCode, utf8.decode(resp.bodyBytes)) + : 'Provider error (${resp.statusCode}): ${_briefErr(resp.body)}'); } final Object? j; try { - j = jsonDecode(utf8.decode(resp.bodyBytes)); + // The plan route only streams; the buffered body is its SSE events. + j = chatgpt + ? chatGptCompletedResponse(utf8.decode(resp.bodyBytes)) + : jsonDecode(utf8.decode(resp.bodyBytes)); + } on ChatGptException { + rethrow; } catch (_) { throw CoachException( 'Provider returned a non-JSON response. Check the API base URL — ' @@ -741,12 +755,14 @@ class CoachEngine { if (j is! Map) throw CoachException('Unexpected response from provider.'); return switch (api) { CoachApi.responses => CoachResponses.reply( - j.cast(), model, apiBase: apiBase), + j.cast(), model, apiBase: replayBase), CoachApi.chatCompletions => CoachChatCompletions.reply( j.cast()), }; } on FormatException catch (e) { throw CoachException(e.message); + } on ChatGptException catch (e) { + throw CoachException(e.message); } finally { if (client == null) c.close(); } diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 2dbac2d31..f42c7632f 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -2884,7 +2884,8 @@ class AppState extends ChangeNotifier { await NotificationCenter.instance.scheduleAiReminders( prefs, ai, - aiConfigured: coachConfig?.hasKey ?? false, + aiConfigured: (coachConfig?.hasKey ?? false) || + (coachConfig?.usesChatGpt ?? false), bedtimeMinOfDay: cd.bedtimeMin, journalDoneToday: BriefingStore.journalDoneToday(), sweepHeadline: await _sweepHeadlineNow(), diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index 18aa06773..e65b32d7e 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -28,6 +28,8 @@ import 'package:provider/provider.dart'; import '../../l10n/app_localizations.dart'; import '../../ai/briefing.dart' show currentBriefingPeriod; +import 'package:url_launcher/url_launcher.dart'; +import '../../coach/coach_chatgpt.dart'; import '../../coach/coach_config.dart'; import '../../coach/coach_engine.dart'; import '../../state/app_state.dart'; @@ -431,7 +433,11 @@ class _CoachScreenState extends State { padding: const EdgeInsets.symmetric(horizontal: S.x4), child: NavBar( l?.coachNavTitle ?? 'Coach', - sub: cfg.configured ? cfg.model : (l?.coachNotSetUp ?? 'Not set up'), + sub: cfg.configured + ? (cfg.usesChatGpt + ? '${cfg.model} · ${uiText(c, 'Using ChatGPT plan')}' + : cfg.model) + : (l?.coachNotSetUp ?? 'Not set up'), trailing: Pressable( semanticLabel: l?.coachMenuSemantic ?? 'Chats and AI settings', onTap: _menu, @@ -835,6 +841,12 @@ class _CoachSetupState extends State { late final TextEditingController _search; late final TextEditingController _timeout; late CoachApi _api; + + /// "ChatGPT" is chosen instead of a base URL + key. Saved by [_save] only; + /// signing in alone does not switch how the coach is paid for. + late bool _chatgpt; + late final ChatGptAuth _chatgptAuth; // for dispose, which must not read context + bool _signingIn = false; String _model = ''; List _models = const []; bool _loading = false; @@ -866,6 +878,8 @@ class _CoachSetupState extends State { _search = TextEditingController(); _timeout = TextEditingController(text: cfg.timeoutSeconds.toString()); _api = cfg.api; + _chatgpt = cfg.usesChatGpt; + _chatgptAuth = cfg.chatgpt; _model = cfg.model; _keyOrigin = coachEndpointOrigin(cfg.baseUrl); // The base URL decides which preset is lit and whether a key is needed, and @@ -905,6 +919,7 @@ class _CoachSetupState extends State { @override void dispose() { + if (_signingIn) _chatgptAuth.cancelSignIn(); _base.dispose(); _key.dispose(); _search.dispose(); @@ -912,15 +927,18 @@ class _CoachSetupState extends State { super.dispose(); } - bool get _isLocal => isLocalCoachHost(_base.text.trim()); + bool get _isLocal => !_chatgpt && isLocalCoachHost(_base.text.trim()); Future _fetch() async { + final chatgpt = _chatgptAuth; setState(() { _loading = true; _msg = null; }); try { - final ids = await CoachEngine.fetchModels(_base.text, _key.text); + final ids = _chatgpt + ? await chatgpt.listModels() + : await CoachEngine.fetchModels(_base.text, _key.text); if (!mounted) return; final l = AppLocalizations.of(context); setState(() { @@ -934,8 +952,8 @@ class _CoachSetupState extends State { if (!mounted) return; final l = AppLocalizations.of(context); setState( - () => _msg = e is CoachException - ? e.message + () => _msg = e is CoachException || e is ChatGptException + ? '$e' : (l?.coachEndpointUnreachable('$e') ?? 'Could not reach that endpoint: $e'), ); } finally { @@ -958,8 +976,15 @@ class _CoachSetupState extends State { // the existing timeout untouched (CoachConfig also rejects <=0) rather // than blocking the rest of the save over one bad field. final timeoutSeconds = _isLocal ? int.tryParse(_timeout.text.trim()) : null; + if (_chatgpt) { + // The BYOK fields are left exactly as they were, for switching back. + await cfg.save(useChatGpt: true, model: chosen); + if (mounted && nav.canPop()) nav.pop(); + return; + } try { await cfg.save( + useChatGpt: false, baseUrl: _base.text, apiKey: coachApiKeyToSave( keyText: _key.text, @@ -981,6 +1006,252 @@ class _CoachSetupState extends State { if (mounted && nav.canPop()) nav.pop(); } + Future _closeBrowser() async { + try { + if (await supportsCloseForLaunchMode(LaunchMode.inAppBrowserView)) { + await closeInAppWebView(); + } + } catch (_) {} + } + + /// The redirect lands on a loopback listener inside this app, so the page + /// opens in an in-app browser (SFSafariViewController / Custom Tab): with + /// the system browser in front, iOS suspends the app and the listener dies. + Future _signInChatGpt() async { + final chatgpt = _chatgptAuth; + setState(() { + _signingIn = true; + _msg = null; + }); + try { + final first = await chatgpt.signIn( + (uri) => launchUrl(uri, mode: LaunchMode.inAppBrowserView), + ); + await _closeBrowser(); + if (!mounted) return; + setState(() { + _chatgpt = true; + _models = const []; + _model = ''; + if (!chatgpt.sharing) { + _msg = uiText(context, + 'Signed in, but ChatGPT plan usage was not allowed. Continue with ChatGPT again and allow it to use your plan here.'); + } + }); + if (!chatgpt.sharing) return; + if (first) await _welcome(); + if (mounted) await _fetch(); + } catch (e) { + await _closeBrowser(); + if (mounted) { + setState(() => _msg = e is ChatGptException ? e.message : 'ChatGPT sign-in failed: $e'); + } + } finally { + if (mounted) setState(() => _signingIn = false); + } + } + + /// Leaves the coach set to ChatGPT, now not configured, rather than + /// quietly falling back to an API key the user may be billed for. + Future _signOutChatGpt() async { + final revoked = await _chatgptAuth.signOut(); + if (!mounted) return; + setState(() { + _models = const []; + _msg = revoked + ? uiText(context, 'Signed out of ChatGPT.') + : uiText(context, + 'Signed out here, but ChatGPT did not confirm it. You can also disconnect OpenStrap in ChatGPT settings.'); + }); + } + + Future _welcome() => showModalBottomSheet( + context: context, + sheetAnimationStyle: sheetMotion(context), + backgroundColor: P.of(context).card, + shape: const RoundedRectangleBorder( + borderRadius: BorderRadius.vertical(top: Radius.circular(R.xxl)), + ), + builder: (s) => SafeArea( + child: Padding( + padding: const EdgeInsets.all(S.x5), + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + Text(uiText(s, 'You’re using your ChatGPT plan'), + style: F.head.copyWith(color: P.of(s).ink)), + const SizedBox(height: S.x2), + Text( + uiText(s, + 'Eligible usage in this app uses your ChatGPT plan. Manage usage in your ChatGPT settings.'), + style: F.cap.copyWith(color: P.of(s).ink2, height: 1.5), + ), + const SizedBox(height: S.x5), + BigButton( + uiText(s, 'Got it'), + color: kCoachAccent, + onTap: () => Navigator.of(s).pop(), + ), + ], + ), + ), + ), + ); + + Widget _presetTile(BuildContext c, P p, _Preset preset) => Padding( + padding: const EdgeInsets.only(bottom: S.x2), + child: Surface( + elevation: 0, + color: !_chatgpt && + _base.text.trim() == preset.baseUrl + ? p.wash( + preset.local ? C.green : kCoachAccent, + ) + : p.card2, + onTap: () => setState(() { + // BEFORE assigning _base.text: that assignment + // fires _onBaseChanged synchronously, which + // may set _msg to explain a cleared key — + // clearing _msg after it runs would silently + // discard that explanation. + _msg = null; + _chatgpt = false; + _base.text = preset.baseUrl; + _api = CoachApi.defaultFor(preset.baseUrl); + _models = const []; + _model = ''; + }), + child: Row( + children: [ + Icon( + preset.local + ? LucideIcons.house + : LucideIcons.cloud, + size: 17, + color: p.on( + preset.local ? C.green : kCoachAccent, + ), + ), + const SizedBox(width: S.x3), + Expanded( + child: Column( + crossAxisAlignment: + CrossAxisAlignment.start, + children: [ + Text( + preset.label, + style: F.body.copyWith( + color: p.ink, + fontWeight: FontWeight.w600, + ), + ), + Text( + preset.sub, + style: F.cap.copyWith(color: p.ink3), + ), + ], + ), + ), + ], + ), + ), + ); + + Widget _chatGptCard(BuildContext c, P p) { + final chatgpt = c.watch().chatgpt; + final session = chatgpt.session; + final sub = session == null + ? uiText(c, 'Use your ChatGPT plan') + : chatgpt.sharing + ? '${uiText(c, 'Using your ChatGPT plan')} · ${session.email}' + : '${uiText(c, 'Plan usage not allowed')} · ${session.email}'; + return Padding( + padding: const EdgeInsets.only(bottom: S.x2), + child: Surface( + elevation: 0, + color: _chatgpt ? p.wash(kCoachAccent) : p.card2, + onTap: _signingIn + ? null + : () { + if (chatgpt.sharing) { + setState(() { + _msg = null; + _chatgpt = true; + _models = const []; + _model = ''; + }); + } else { + _signInChatGpt(); + } + }, + child: Row( + children: [ + Icon(LucideIcons.sparkles, size: 17, color: p.on(kCoachAccent)), + const SizedBox(width: S.x3), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text('ChatGPT', + style: F.body.copyWith(color: p.ink, fontWeight: FontWeight.w600)), + Text(sub, style: F.cap.copyWith(color: p.ink3)), + ], + ), + ), + ], + ), + ), + ); + } + + List _chatGptFields(BuildContext c, P p) { + final chatgpt = c.watch().chatgpt; + return [ + const SizedBox(height: S.x2), + Text( + uiText(c, + 'Complete eligible AI requests in this app with usage included in your ChatGPT plan or credits balance. OpenStrap never charges for this.'), + style: F.cap.copyWith(color: p.ink3, height: 1.5), + ), + const SizedBox(height: S.x4), + if (!chatgpt.sharing) + BigButton( + _signingIn + ? uiText(c, 'Cancel sign-in') + : uiText(c, 'Continue with ChatGPT'), + icon: _signingIn ? LucideIcons.x : LucideIcons.logIn, + color: kCoachAccent, + soft: _signingIn, + onTap: _signingIn ? _chatgptAuth.cancelSignIn : _signInChatGpt, + ), + if (chatgpt.signedIn) ...[ + if (!chatgpt.sharing) const SizedBox(height: S.x3), + BigButton( + uiText(c, 'Manage usage'), + icon: LucideIcons.externalLink, + color: kCoachAccent, + soft: true, + onTap: () => launchUrl(Uri.parse(kChatGptUsageUrl), + mode: LaunchMode.externalApplication), + ), + const SizedBox(height: S.x3), + BigButton( + uiText(c, 'Sign out of ChatGPT'), + icon: LucideIcons.logOut, + soft: true, + onTap: _signOutChatGpt, + ), + ], + const SizedBox(height: S.x3), + Text( + uiText(c, + 'Your questions and the rows the coach reads are sent to OpenAI under your ChatGPT account. See exactly what that is on "What was sent".'), + style: F.cap.copyWith(color: p.ink3, height: 1.5), + ), + ]; + } + @override Widget build(BuildContext c) { final p = P.of(c); @@ -1010,151 +1281,109 @@ class _CoachSetupState extends State { l?.coachWhereModelRuns ?? 'Where the model runs', Column( children: [ - for (final preset in _presets(c)) - Padding( - padding: const EdgeInsets.only(bottom: S.x2), - child: Surface( - elevation: 0, - color: _base.text.trim() == preset.baseUrl - ? p.wash( - preset.local ? C.green : kCoachAccent, - ) - : p.card2, - onTap: () => setState(() { - // BEFORE assigning _base.text: that assignment - // fires _onBaseChanged synchronously, which - // may set _msg to explain a cleared key — - // clearing _msg after it runs would silently - // discard that explanation. - _msg = null; - _base.text = preset.baseUrl; - _api = CoachApi.defaultFor(preset.baseUrl); - _models = const []; - _model = ''; - }), - child: Row( - children: [ - Icon( - preset.local - ? LucideIcons.house - : LucideIcons.cloud, - size: 17, - color: p.on( - preset.local ? C.green : kCoachAccent, - ), + // After the local presets: they are the only option + // where the data never leaves the user's control. + for (final preset in _presets(c).where((x) => x.local)) + _presetTile(c, p, preset), + _chatGptCard(c, p), + for (final preset in _presets(c).where((x) => !x.local)) + _presetTile(c, p, preset), + ], + ), + ), + if (_chatgpt) + ..._chatGptFields(c, p) + else ...[ + const SizedBox(height: S.x2), + OsTextField( + controller: _base, + label: l?.coachBaseUrlLabel ?? 'Base URL', + hint: 'http://localhost:11434/v1', + ), + const SizedBox(height: S.x4), + Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text(l?.coachApiFormatLabel ?? 'API', + style: F.over.copyWith(color: p.ink3)), + const SizedBox(height: S.x2), + Container( + padding: const EdgeInsets.symmetric(horizontal: S.x3), + decoration: BoxDecoration( + color: p.card, + borderRadius: R.rMd, + border: Border.all(color: p.line), + ), + child: Semantics( + label: l?.coachApiFormatLabel ?? 'API', + child: DropdownButtonHideUnderline( + child: DropdownButton( + value: _api, + isExpanded: true, + dropdownColor: p.card, + style: F.body.copyWith(color: p.ink), + items: const [ + DropdownMenuItem( + value: CoachApi.responses, + child: Text('Responses'), ), - const SizedBox(width: S.x3), - Expanded( - child: Column( - crossAxisAlignment: - CrossAxisAlignment.start, - children: [ - Text( - preset.label, - style: F.body.copyWith( - color: p.ink, - fontWeight: FontWeight.w600, - ), - ), - Text( - preset.sub, - style: F.cap.copyWith(color: p.ink3), - ), - ], - ), + DropdownMenuItem( + value: CoachApi.chatCompletions, + child: Text('Chat Completions'), ), ], + onChanged: (api) { + if (api != null) setState(() => _api = api); + }, ), ), ), - ], - ), - ), - const SizedBox(height: S.x2), - OsTextField( - controller: _base, - label: l?.coachBaseUrlLabel ?? 'Base URL', - hint: 'http://localhost:11434/v1', - ), - const SizedBox(height: S.x4), - Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text(l?.coachApiFormatLabel ?? 'API', - style: F.over.copyWith(color: p.ink3)), - const SizedBox(height: S.x2), - Container( - padding: const EdgeInsets.symmetric(horizontal: S.x3), - decoration: BoxDecoration( - color: p.card, - borderRadius: R.rMd, - border: Border.all(color: p.line), ), - child: Semantics( - label: l?.coachApiFormatLabel ?? 'API', - child: DropdownButtonHideUnderline( - child: DropdownButton( - value: _api, - isExpanded: true, - dropdownColor: p.card, - style: F.body.copyWith(color: p.ink), - items: const [ - DropdownMenuItem( - value: CoachApi.responses, - child: Text('Responses'), - ), - DropdownMenuItem( - value: CoachApi.chatCompletions, - child: Text('Chat Completions'), - ), - ], - onChanged: (api) { - if (api != null) setState(() => _api = api); - }, - ), - ), + const SizedBox(height: S.x2), + Text( + l?.coachApiFormatHelp ?? + 'Choose the API your endpoint supports. Responses ' + 'supports OpenAI reasoning with tools.', + style: F.cap.copyWith(color: p.ink3, height: 1.5), ), - ), - const SizedBox(height: S.x2), - Text( - l?.coachApiFormatHelp ?? - 'Choose the API your endpoint supports. Responses ' - 'supports OpenAI reasoning with tools.', - style: F.cap.copyWith(color: p.ink3, height: 1.5), - ), - ], - ), - const SizedBox(height: S.x4), - OsTextField( - controller: _key, - label: _isLocal - ? (l?.coachApiKeyLocalLabel ?? 'API key (not needed locally)') - : (l?.coachApiKeyLabel ?? 'API key'), - hint: 'sk-…', - ), - const SizedBox(height: S.x3), - // The one sentence that decides whether a cloud key is a - // reasonable choice. It is here, next to the field, and not - // in a settings page nobody opens. Private hosts only: a - // 100.64.0.0/10 address needs no key but may not be yours. - Text( - isPrivateCoachHost(_base.text.trim()) - ? (l?.coachLocalDataNote ?? - 'Your questions and the rows the coach reads stay on ' - 'your own machine.') - : (l?.coachCloudDataNote ?? - 'Your questions and the rows the coach reads are sent ' - 'to this endpoint. See exactly what that is on ' - '"What was sent".'), - style: F.cap.copyWith(color: p.ink3, height: 1.5), - ), + ], + ), + const SizedBox(height: S.x4), + OsTextField( + controller: _key, + label: _isLocal + ? (l?.coachApiKeyLocalLabel ?? 'API key (not needed locally)') + : (l?.coachApiKeyLabel ?? 'API key'), + hint: 'sk-…', + ), + const SizedBox(height: S.x3), + // The one sentence that decides whether a cloud key is a + // reasonable choice. It is here, next to the field, and not + // in a settings page nobody opens. Private hosts only: a + // 100.64.0.0/10 address needs no key but may not be yours. + Text( + isPrivateCoachHost(_base.text.trim()) + ? (l?.coachLocalDataNote ?? + 'Your questions and the rows the coach reads stay on ' + 'your own machine.') + : (l?.coachCloudDataNote ?? + 'Your questions and the rows the coach reads are sent ' + 'to this endpoint. See exactly what that is on ' + '"What was sent".'), + style: F.cap.copyWith(color: p.ink3, height: 1.5), + ), + ], const SizedBox(height: S.x4), BigButton( _loading ? (l?.coachAsking ?? 'Asking…') : (l?.coachListModels ?? 'List models'), icon: LucideIcons.refreshCw, color: kCoachAccent, soft: true, - onTap: _loading ? null : _fetch, + onTap: _loading || + (_chatgpt && + !context.watch().chatgpt.sharing) + ? null + : _fetch, ), if (_msg != null) ...[ const SizedBox(height: S.x3), diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart new file mode 100644 index 000000000..3d1a897db --- /dev/null +++ b/test/coach_chatgpt_test.dart @@ -0,0 +1,353 @@ +// "Sign in with ChatGPT" plan usage: the OAuth round trip through a real +// loopback listener, token refresh, and the coach's one provider POST routed +// to OpenAI's streaming-only Responses route with the plan's access token. +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/io_client.dart'; +import 'package:http/testing.dart'; +import 'package:openstrap_edge/coach/coach_chatgpt.dart'; +import 'package:openstrap_edge/coach/coach_config.dart'; +import 'package:openstrap_edge/coach/coach_engine.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +// Flutter's test binding fakes HttpClient; this one really reaches loopback. +class _RealHttpOverrides extends HttpOverrides {} + +String _jwt(Map claims) { + String b64(Object o) => base64Url.encode(utf8.encode(jsonEncode(o))).replaceAll('=', ''); + return '${b64({'alg': 'RS256'})}.${b64(claims)}.sig'; +} + +String _idToken(String clientId, String nonce, {String iss = kChatGptIssuer, int? exp}) => + _jwt({ + 'iss': iss, + 'aud': clientId, + 'sub': 'user-1', + 'email': 'me@example.com', + 'nonce': nonce, + 'exp': exp ?? DateTime.now().add(const Duration(hours: 1)).millisecondsSinceEpoch ~/ 1000, + }); + +const _fullScope = 'chatgpt.tokens.use.direct email offline_access openid profile resource.invoke'; + +ChatGptSession _session({DateTime? expiresAt, String scope = _fullScope}) => ChatGptSession( + sub: 'user-1', + email: 'me@example.com', + idToken: 'old-id-token', + accessToken: 'at-1', + refreshToken: 'rt-1', + scope: scope, + expiresAt: expiresAt ?? DateTime.now().add(const Duration(hours: 1)), + ); + +String _sse(List> events) => + events.map((e) => 'event: ${e['type']}\ndata: ${jsonEncode(e)}\n\n').join(); + +Map _completed(String text) => { + 'type': 'response.completed', + 'response': { + 'status': 'completed', + 'output': [ + { + 'type': 'message', + 'status': 'completed', + 'role': 'assistant', + 'content': [ + {'type': 'output_text', 'text': text, 'annotations': []}, + ], + }, + ], + }, + }; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('plugins.it_nomads.com/flutter_secure_storage'); + late Map keychain; + + setUp(() { + keychain = {}; + SharedPreferences.setMockInitialValues({}); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + final args = (call.arguments as Map?) ?? const {}; + switch (call.method) { + case 'read': + return keychain[args['key']]; + case 'write': + keychain[args['key'] as String] = args['value'] as String; + case 'delete': + keychain.remove(args['key']); + case 'containsKey': + return keychain.containsKey(args['key']); + } + return null; + }); + }); + + tearDown(() => TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, null)); + + test('PKCE challenge is the RFC 7636 S256 transform', () { + expect(chatGptCodeChallenge('dBjftJeZ4CVP-mJ92K9qn6MWHFJM1yN_GgcsRmJN0_w'), + 'BCCYQ8sMKFQG5yknl8BJ9-8mva0FhJQJZSboXfUVfsc'); + }); + + group('sign-in', () { + late List> tokenForms; + late ChatGptAuth auth; + late http.Client loopback; + + setUp(() { + tokenForms = []; + loopback = IOClient(_RealHttpOverrides().createHttpClient(null)); + addTearDown(loopback.close); + }); + + /// Plays the browser: first a stray hit with the wrong state, which must + /// not end the sign-in, then the real redirect. + Future browser(Uri authorize, {String? clientId, String? error}) async { + final q = authorize.queryParameters; + final redirect = Uri.parse(q['redirect_uri']!); + expect(redirect.host, '127.0.0.1'); + expect(redirect.path, '/auth/callback'); + final stray = await loopback.get(redirect.replace(queryParameters: {'state': 'nope', 'code': 'x'})); + expect(stray.statusCode, 400); + unawaited(loopback.get(redirect.replace(queryParameters: { + 'state': q['state']!, + if (error != null) 'error': error else 'code': 'code-1', + 'client_id': ?clientId, + }))); + return true; + } + + ChatGptAuth makeAuth(String Function() nonce) => ChatGptAuth( + client: MockClient((req) async { + expect(req.url.toString(), '$kChatGptIssuer/api/accounts/oauth/token'); + tokenForms.add(req.bodyFields); + return http.Response( + jsonEncode({ + 'access_token': 'at-1', + 'refresh_token': 'rt-1', + 'id_token': _idToken(req.bodyFields['client_id']!, nonce()), + 'scope': _fullScope, + 'expires_in': 3600, + }), + 200, + ); + }), + ); + + test('registers, exchanges the code, and signs back in to the same registration', () async { + late Uri first, second; + auth = makeAuth(() => (second.hasQuery ? second : first).queryParameters['nonce']!); + second = Uri(); + expect( + await auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); + }), + isTrue, + ); + final q = first.queryParameters; + expect(q['client_id'], 'dynamic_agent_client'); + expect(q['agent_name_hint'], 'OpenStrap Edge'); + expect(q['ext_agent_host_id'], startsWith('urn:uuid:')); + expect(q['scope']!.split(' '), containsAll(['offline_access', 'resource.invoke', kChatGptPlanScope])); + expect(q['resource'], kChatGptApiBase); + expect(q['code_challenge_method'], 'S256'); + expect(tokenForms.single, containsPair('client_id', 'oaiapp_1')); + expect(tokenForms.single, containsPair('grant_type', 'authorization_code')); + expect(tokenForms.single, containsPair('redirect_uri', q['redirect_uri'])); + expect(chatGptCodeChallenge(tokenForms.single['code_verifier']!), q['code_challenge']); + expect(auth.sharing, isTrue); + expect(auth.session!.email, 'me@example.com'); + + // Returning sign-in: the saved client id and host id, no name hint. + expect( + await auth.signIn((u) { + second = u; + return browser(u); + }), + isFalse, + ); + expect(second.queryParameters['client_id'], 'oaiapp_1'); + expect(second.queryParameters['ext_agent_host_id'], q['ext_agent_host_id']); + expect(second.queryParameters.containsKey('agent_name_hint'), isFalse); + expect(second.queryParameters['id_token_hint'], isNotNull); + }); + + test('a declined consent ends sign-in without a token exchange', () async { + auth = makeAuth(() => ''); + await expectLater( + auth.signIn((u) => browser(u, error: 'access_denied')), + throwsA(isA().having((e) => e.message, 'message', contains('cancelled'))), + ); + expect(tokenForms, isEmpty); + expect(auth.signedIn, isFalse); + }); + }); + + test('closing the browser can be cancelled instead of waiting it out', () async { + final auth = ChatGptAuth(client: MockClient((_) async => fail('no token exchange'))); + await expectLater( + auth.signIn((_) async { + auth.cancelSignIn(); // the user gave up while the page was up + return true; + }), + throwsA(isA().having((e) => e.message, 'message', contains('cancelled'))), + ); + }); + + test('ID token claims are checked', () { + final good = _idToken('oaiapp_1', 'n1'); + expect(chatGptIdentity(good, clientId: 'oaiapp_1', nonce: 'n1').sub, 'user-1'); + for (final (token, client, nonce) in [ + (good, 'oaiapp_1', 'other-nonce'), + (good, 'oaiapp_2', 'n1'), + (_idToken('oaiapp_1', 'n1', iss: 'https://evil.example'), 'oaiapp_1', 'n1'), + (_idToken('oaiapp_1', 'n1', exp: 1), 'oaiapp_1', 'n1'), + ('garbage', 'oaiapp_1', 'n1'), + ]) { + expect(() => chatGptIdentity(token, clientId: client, nonce: nonce), + throwsA(isA())); + } + }); + + group('refresh', () { + Future expired(http.Client c) async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final auth = ChatGptAuth(client: c); + await auth.debugSetSession(_session(expiresAt: DateTime.now())); + return auth; + } + + test('concurrent callers share one rotation', () async { + final forms = >[]; + final auth = await expired(MockClient((req) async { + forms.add(req.bodyFields); + return http.Response( + jsonEncode({'access_token': 'at-2', 'refresh_token': 'rt-2', 'expires_in': 3600}), 200); + })); + final tokens = await Future.wait([auth.accessToken(), auth.accessToken()]); + expect(tokens, ['at-2', 'at-2']); + expect(forms.single, { + 'grant_type': 'refresh_token', + 'client_id': 'oaiapp_1', + 'refresh_token': 'rt-1', + 'resource': kChatGptApiBase, + }); + expect(auth.session!.refreshToken, 'rt-2'); + expect(auth.sharing, isTrue, reason: 'scope carried over when not re-sent'); + }); + + test('a dead refresh token signs out; a server hiccup does not', () async { + final hiccup = await expired(MockClient((_) async => http.Response('oops', 502))); + await expectLater(hiccup.accessToken(), throwsA(isA())); + expect(hiccup.signedIn, isTrue); + + final dead = await expired( + MockClient((_) async => http.Response(jsonEncode({'error': 'invalid_grant'}), 400))); + await expectLater(dead.accessToken(), throwsA(isA())); + expect(dead.signedIn, isFalse); + }); + }); + + group('coach requests on the ChatGPT plan', () { + late CoachConfig cfg; + setUp(() async { + cfg = CoachConfig(); + await cfg.chatgpt.debugSetSession(_session()); + await cfg.save(useChatGpt: true, model: 'gpt-6.1-sol'); + }); + + Future> post(http.Client c) => CoachEngine.postChat(cfg, { + 'model': 'gpt-6.1-sol', + 'messages': [ + {'role': 'system', 'content': 'be brief'}, + {'role': 'user', 'content': 'hi'}, + ], + 'temperature': 0.3, + }, client: c); + + test('streams to the public Responses API with the plan token', () async { + expect(cfg.configured, isTrue); + expect(cfg.isPrivateEndpoint, isFalse, reason: 'health data leaving the phone still asks first'); + late http.Request sent; + final reply = await post(MockClient((req) async { + sent = req; + return http.Response( + _sse([ + {'type': 'response.output_text.delta', 'delta': 'hel'}, + _completed('hello'), + ]), + 200, + ); + })); + expect(sent.url.toString(), '$kChatGptApiBase/responses'); + expect(sent.headers['Authorization'], 'Bearer at-1'); + final body = jsonDecode(sent.body) as Map; + expect(body['stream'], isTrue); + expect(body['store'], isFalse); + expect(body.containsKey('temperature'), isFalse); + expect([for (final i in body['input'] as List) i['role']], ['developer', 'user']); + expect(reply['content'], 'hello'); + }); + + test('usage limit, mid-stream failure and a cut-off stream are errors, never a fallback', () async { + for (final (status, body, expected) in [ + ( + 429, + jsonEncode({'error': {'code': 'subscription_sharing_usage_limit_exceeded'}}), + 'Usage limit reached' + ), + ( + 200, + _sse([ + { + 'type': 'response.failed', + 'response': {'error': {'code': 'subscription_sharing_usage_limit_exceeded'}}, + }, + ]), + 'Usage limit reached' + ), + (200, _sse([{'type': 'response.output_text.delta', 'delta': 'he'}]), 'ended early'), + ]) { + await expectLater( + post(MockClient((_) async => http.Response(body, status))), + throwsA(isA().having((e) => e.message, 'message', contains(expected))), + ); + } + }); + + test('signed in without plan usage is not configured', () async { + await cfg.chatgpt.debugSetSession(_session(scope: 'openid email profile offline_access')); + expect(cfg.configured, isFalse); + }); + }); + + test('model list keeps listed slugs in server order', () async { + final auth = ChatGptAuth( + client: MockClient((req) async { + expect(req.headers['Authorization'], 'Bearer at-1'); + return http.Response( + jsonEncode({ + 'models': [ + {'slug': 'gpt-6.1-sol', 'visibility': 'list'}, + {'slug': 'hidden', 'visibility': 'hide'}, + {'slug': 'gpt-5.6-terra', 'visibility': 'list'}, + ], + }), + 200, + ); + }), + ); + await auth.debugSetSession(_session()); + expect(await auth.listModels(), ['gpt-6.1-sol', 'gpt-5.6-terra']); + }); +} diff --git a/test/russian_review_findings_test.dart b/test/russian_review_findings_test.dart index 15702d8a0..59ac92395 100644 --- a/test/russian_review_findings_test.dart +++ b/test/russian_review_findings_test.dart @@ -410,6 +410,8 @@ void main() { 400, scrollable: find.byType(Scrollable).first, ); + await t.ensureVisible(find.text(en.coachListModels)); + await t.pumpAndSettle(); await t.tap(find.text(en.coachListModels)); await t.pumpAndSettle(); const raw = From ec6035b45a98a2d42995d4c81d7973f4e0c08594 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 16:14:43 -0500 Subject: [PATCH 02/23] fix(coach): ChatGPT stream output, session races, and review findings - Take output items from response.output_item.done: on the plan route the completed event's output can be empty, which surfaced on a real device as "Empty response from provider". - Session generation so a refresh or keychain load that overlaps sign-in or sign-out cannot restore or clear the wrong session; sign-out waits for an in-flight refresh and revokes the latest rotated token. - Reset all data also forgets the ChatGPT session and registration. - Only the keychain duplicate-item error deletes and retries a write. - Sign-out completes even if the keychain delete fails. - AI reminders require a configured ChatGPT session, not just the mode. - Welcome notice tracked separately from registration. - Reasoning replay namespaced per ChatGPT account. - A hung ChatGPT keychain read no longer holds back BYOK settings. - Guide: sign-out asks OpenAI to revoke; disconnect in settings if unconfirmed. - Tests: exact code exchange, no API-key fallback request, sign-out during refresh, write failure, reset, welcome-once, empty completed output. --- guides/AI_COACH.md | 6 +- lib/coach/coach_chatgpt.dart | 134 ++++++++++++++++++++++++++++++----- lib/coach/coach_config.dart | 26 ++----- lib/coach/coach_engine.dart | 7 +- lib/state/app_state.dart | 8 ++- lib/ui2/screens/coach.dart | 4 +- test/coach_chatgpt_test.dart | 124 ++++++++++++++++++++++++++++---- 7 files changed, 249 insertions(+), 60 deletions(-) diff --git a/guides/AI_COACH.md b/guides/AI_COACH.md index b6216f1a5..c6238c5fd 100644 --- a/guides/AI_COACH.md +++ b/guides/AI_COACH.md @@ -33,8 +33,10 @@ models** and pick one, then **Save**. switches to an API key. - Your questions and the rows the coach reads go to OpenAI under your ChatGPT account, exactly as with an OpenAI API key. -- **Sign out of ChatGPT** revokes the sign-in. The coach then stays off until - you sign in again or pick another provider. +- **Sign out of ChatGPT** asks OpenAI to revoke the sign-in and clears it from + the phone. If the app says revocation wasn't confirmed, also disconnect + OpenStrap in ChatGPT settings. The coach then stays off until you sign in + again or pick another provider. ## Setup with an API key diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 64ad5c12a..d741ce190 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -13,6 +13,7 @@ import 'dart:math'; import 'package:clock/clock.dart'; import 'package:crypto/crypto.dart'; import 'package:flutter/foundation.dart'; +import 'package:flutter/services.dart' show PlatformException; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:http/http.dart' as http; import 'package:shared_preferences/shared_preferences.dart'; @@ -34,6 +35,20 @@ const _scopes = 'openid profile email offline_access resource.invoke $kChatGptPlanScope'; const _agentName = 'OpenStrap Edge'; +/// True for the iOS/macOS errSecDuplicateItem shape the plugin surfaces when +/// its own check-then-act (containsKey -> update, else add) still lands on +/// top of an item it did not find — a leftover from a prior install is the +/// reported real-world trigger, since Keychain items routinely outlive an app +/// uninstall while everything else this app stores does not. +bool isKeychainDuplicateItemError(Object e) { + if (e is! PlatformException) return false; + final details = e.details; + if (details is int && details == -25299) return true; + final text = '${e.message ?? ''} ${e.details ?? ''}'; + return text.contains('-25299') || + text.contains('already exists in the keychain'); +} + /// A sign-in or token failure with a message fit to show the user. class ChatGptException implements Exception { final String message; @@ -104,10 +119,13 @@ String chatGptCodeChallenge(String verifier) => } /// The final `response` of a streamed Responses call. This route only -/// streams; the whole body is buffered and the completed response — which -/// carries the full output — is handed to the same parser as non-streamed -/// replies. Anything short of `response.completed` is a failure. +/// streams; the whole body is buffered and the completed response is handed +/// to the same parser as non-streamed replies. `response.completed` is the +/// success signal, but on this route its `output` can be empty — the items +/// arrive earlier as `response.output_item.done`, so they are taken from +/// there. Anything short of `response.completed` is a failure. Map chatGptCompletedResponse(String sse) { + final items = <(int, Map)>[]; for (final line in const LineSplitter().convert(sse)) { if (!line.startsWith('data:')) continue; final Object? event; @@ -118,9 +136,19 @@ Map chatGptCompletedResponse(String sse) { } if (event is! Map) continue; switch (event['type']) { + case 'response.output_item.done': + final item = event['item']; + if (item is Map) { + final i = event['output_index']; + items.add((i is int ? i : items.length, item.cast())); + } case 'response.completed': final r = event['response']; - if (r is Map) return r.cast(); + if (r is! Map) break; + final out = r['output']; + if (out is List && out.isNotEmpty) return r.cast(); + items.sort((a, b) => a.$1.compareTo(b.$1)); + return {...r.cast(), 'output': [for (final e in items) e.$2]}; case 'response.failed' || 'error': final err = (event['response'] is Map ? event['response']['error'] : null) ?? event['error'] ?? @@ -246,6 +274,7 @@ class ChatGptAuth extends ChangeNotifier { static const _kClientId = 'coach_chatgpt_client_id'; static const _kHostId = 'coach_chatgpt_host_id'; static const _kPresent = 'coach_chatgpt_present'; + static const _kWelcomed = 'coach_chatgpt_welcomed'; static const _kSession = 'coach_chatgpt_session'; // secure storage // Same first-unlock reasoning as CoachConfig's key: background relaunches @@ -260,6 +289,17 @@ class ChatGptAuth extends ChangeNotifier { HttpServer? _pending; Future? _refreshing; + /// Bumped whenever a sign-in commits a session, and at the very start of + /// [signOut]/[forgetAll]. A [load] or [_refresh] that started under an older + /// value must not apply its result — a slow keychain read or token call + /// would otherwise bring back a session the user just ended (same idea as + /// CoachConfig._generation). + int _generation = 0; + + /// The refresh token a refresh rotated to after [signOut] began. It is not + /// stored, but it is the live one, so it is the one to revoke. + String? _rotatedAfterSignOut; + ChatGptSession? get session => _session; bool get signedIn => _session != null; bool get sharing => _session?.sharing ?? false; @@ -278,10 +318,12 @@ class ChatGptAuth extends ChangeNotifier { } Future load({bool trusted = false}) async { + final generation = _generation; final prefs = await SharedPreferences.getInstance(); final present = prefs.getBool(_kPresent) ?? false; try { final raw = await _secure.read(key: _kSession, iOptions: _apple, mOptions: _macos); + if (generation != _generation) return; if (raw != null && raw.isNotEmpty) { _session = ChatGptSession.fromJson(jsonDecode(raw) as Map); _unreadable = false; @@ -293,26 +335,37 @@ class ChatGptAuth extends ChangeNotifier { _unreadable = true; } } catch (_) { + if (generation != _generation) return; _unreadable = present && _session == null; } notifyListeners(); } - Future _store(ChatGptSession? s) async { + /// A refresh passes the [generation] it started under: if the session was + /// signed out or replaced since, its result is dropped before anything is + /// written. [signOut] waits for an in-flight refresh, so none can land after + /// it clears. + Future _store(ChatGptSession? s, {int? generation}) async { final prefs = await SharedPreferences.getInstance(); + if (generation != null && generation != _generation) { + throw ChatGptException('Signed out.'); + } if (s == null) { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); } else { final raw = jsonEncode(s.toJson()); try { await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); - } catch (_) { - // A leftover item from a previous install (see CoachConfig.save). + } on Object catch (e) { + // Only a leftover item from a previous install (see CoachConfig.save) + // is worth deleting; any other failure leaves the stored session alone. + if (!isKeychainDuplicateItemError(e)) rethrow; await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); } } await prefs.setBool(_kPresent, s != null); + if (generation == null) _generation++; _session = s; _unreadable = false; notifyListeners(); @@ -325,9 +378,8 @@ class ChatGptAuth extends ChangeNotifier { /// Runs the browser sign-in. [open] shows the authorize page and must keep /// this app alive while it is up (an in-app browser), because the redirect - /// lands on a listener on this device's loopback. Returns true for a first - /// registration, which is when the welcome note is due. - Future signIn(Future Function(Uri) open) async { + /// lands on a listener on this device's loopback. + Future signIn(Future Function(Uri) open) async { final prefs = await SharedPreferences.getInstance(); final hostId = prefs.getString(_kHostId) ?? 'urn:uuid:${const Uuid().v4()}'; await prefs.setString(_kHostId, hostId); @@ -386,7 +438,6 @@ class ChatGptAuth extends ChangeNotifier { if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); await _store(_sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken)); - return savedClientId == null; } finally { if (identical(_pending, server)) _pending = null; await server.close(force: true); @@ -461,6 +512,7 @@ class ChatGptAuth extends ChangeNotifier { } Future _refresh(ChatGptSession s) async { + final generation = _generation; final prefs = await SharedPreferences.getInstance(); final clientId = prefs.getString(_kClientId); if (clientId == null) throw ChatGptException('Sign in with ChatGPT again.'); @@ -475,7 +527,7 @@ class ChatGptAuth extends ChangeNotifier { if (!e.terminal) { throw ChatGptException('Could not renew the ChatGPT sign-in (${e.status}). Try again.'); } - await _store(null); + await _store(null, generation: generation); throw ChatGptException('Your ChatGPT sign-in has ended. Sign in again from AI settings.'); } catch (e) { // A network hiccup is not a revoked session — the tokens stay. @@ -487,7 +539,13 @@ class ChatGptAuth extends ChangeNotifier { email: s.email, idToken: (t['id_token'] as String?) ?? s.idToken, ); - await _store(next); + if (generation != _generation) { + // Signed out while the call was out: do not bring the session back, but + // the rotated token is the live one and [signOut] has to revoke it. + _rotatedAfterSignOut = next.refreshToken; + throw ChatGptException('Signed out.'); + } + await _store(next, generation: generation); return next.accessToken; } @@ -495,23 +553,63 @@ class ChatGptAuth extends ChangeNotifier { /// revocation could not be confirmed; the tokens are cleared regardless, and /// the user can still disconnect the app in ChatGPT settings. Future signOut() async { + _generation++; // before any await: see [_generation] + // An in-flight refresh may have rotated the token; revoke the latest one. + try { + await _refreshing; + } catch (_) {} final s = _session; - final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); - var revoked = s == null; - if (s != null && clientId != null) { + final token = _rotatedAfterSignOut ?? s?.refreshToken; + _rotatedAfterSignOut = null; + // Memory first, so nothing keeps using the session while revocation is out. + _session = null; + _unreadable = false; + notifyListeners(); + final prefs = await SharedPreferences.getInstance(); + final clientId = prefs.getString(_kClientId); + var revoked = token == null; + if (token != null && clientId != null) { try { final r = await _http((c) => c.post(Uri.parse(_revokeUrl), body: { - 'token': s.refreshToken, + 'token': token, 'token_type_hint': 'refresh_token', 'client_id': clientId, }).timeout(const Duration(seconds: 15))); revoked = r.statusCode == 200; } catch (_) {} } - await _store(null); + // Signed out either way. If the delete fails the item may linger, but its + // token was just revoked, so a later load just ends it at the next refresh. + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} + await prefs.setBool(_kPresent, false); return revoked; } + /// "Reset all data": the session plus the registration and welcome flag. + Future forgetAll() async { + _generation++; + _session = null; + _unreadable = false; + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} + final prefs = await SharedPreferences.getInstance(); + for (final k in [_kClientId, _kHostId, _kPresent, _kWelcomed]) { + await prefs.remove(k); + } + notifyListeners(); + } + + /// True exactly once, on the first sign-in that has plan usage. + Future takeWelcome() async { + final prefs = await SharedPreferences.getInstance(); + if (prefs.getBool(_kWelcomed) ?? false) return false; + await prefs.setBool(_kWelcomed, true); + return true; + } + /// Model slugs this account may use, in the server's order. Future> listModels() async { final token = await accessToken(); diff --git a/lib/coach/coach_config.dart b/lib/coach/coach_config.dart index 0ead1d9db..48ad2c8b6 100644 --- a/lib/coach/coach_config.dart +++ b/lib/coach/coach_config.dart @@ -3,8 +3,9 @@ // SharedPreferences. NOTHING here ever touches our backend — the key stays on the // device and the app calls the OpenAI-compatible provider directly. +import 'dart:async'; + import 'package:flutter/foundation.dart'; -import 'package:flutter/services.dart' show PlatformException; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -121,20 +122,6 @@ bool isPrivateCoachHost(String url) { } } -/// True for the iOS/macOS errSecDuplicateItem shape the plugin surfaces when -/// its own check-then-act (containsKey -> update, else add) still lands on -/// top of an item it did not find — a leftover from a prior install is the -/// reported real-world trigger, since Keychain items routinely outlive an app -/// uninstall while everything else this app stores does not. -bool _isDuplicateItemError(Object e) { - if (e is! PlatformException) return false; - final details = e.details; - if (details is int && details == -25299) return true; - final text = '${e.message ?? ''} ${e.details ?? ''}'; - return text.contains('-25299') || - text.contains('already exists in the keychain'); -} - class CoachConfig extends ChangeNotifier { static const _kBaseUrl = 'coach_base_url'; static const _kModel = 'coach_model'; @@ -316,9 +303,9 @@ class CoachConfig extends ChangeNotifier { /// device unlocked — the only condition under which an EMPTY read is real /// evidence about what is stored. Future load({bool trusted = false}) async { - // Its own keychain item, read alongside the key rather than ahead of it so - // one hung read cannot hold up the other. - final chatgptLoad = chatgpt.load(trusted: trusted).catchError((_) {}); + // Its own keychain item, and not awaited: a hung read of it must not hold + // back the BYOK settings. It notifies its own listeners, which are forwarded. + unawaited(chatgpt.load(trusted: trusted).catchError((_) {})); final prefs = await SharedPreferences.getInstance(); _useChatGpt = prefs.getBool(_kUseChatGpt) ?? false; _baseUrl = prefs.getString(_kBaseUrl) ?? defaultBaseUrl; @@ -408,7 +395,6 @@ class CoachConfig extends ChangeNotifier { _keyUnreadable = marker == true; _keyUndetermined = marker == null; } - await chatgptLoad; notifyListeners(); } @@ -488,7 +474,7 @@ class CoachConfig extends ChangeNotifier { // sitting there — delete it and retry once. A second failure is // rethrown as-is: this recovers the one known stale-item case, it // does not mask a keychain that is genuinely stuck. - if (!_isDuplicateItemError(e)) rethrow; + if (!isKeychainDuplicateItemError(e)) rethrow; await _secure.delete(key: _kKey, iOptions: _apple, mOptions: _macos); await _secure.write( key: _kKey, diff --git a/lib/coach/coach_engine.dart b/lib/coach/coach_engine.dart index 6dbb1d1ce..1c3a3001a 100644 --- a/lib/coach/coach_engine.dart +++ b/lib/coach/coach_engine.dart @@ -689,8 +689,11 @@ class CoachEngine { final apiBase = config.apiBase; final chatgpt = config.usesChatGpt; // Replayed reasoning is tied to who paid for it, so a ChatGPT-plan turn - // never replays an API-key turn's output on the same base, or vice versa. - final replayBase = chatgpt ? '$apiBase#chatgpt' : apiBase; + // never replays an API-key turn's output on the same base, or another + // account's, or vice versa. + final replayBase = chatgpt + ? '$apiBase#chatgpt:${config.chatgpt.session?.sub ?? ''}' + : apiBase; try { body = switch (api) { CoachApi.responses => CoachResponses.request(body, apiBase: replayBase), diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index f42c7632f..e699cadc0 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -1304,6 +1304,11 @@ class AppState extends ChangeNotifier { } catch (e) { _log('[reset] keychain clear failed: $e'); } + try { + await coachConfig?.chatgpt.forgetAll(); + } catch (e) { + _log('[reset] ChatGPT sign-in clear failed: $e'); + } // 3 · the whole preference namespace, not a remembered subset — same // reason as wipeAll. A fresh install is the state being restored, and a @@ -2885,7 +2890,8 @@ class AppState extends ChangeNotifier { prefs, ai, aiConfigured: (coachConfig?.hasKey ?? false) || - (coachConfig?.usesChatGpt ?? false), + ((coachConfig?.usesChatGpt ?? false) && + (coachConfig?.configured ?? false)), bedtimeMinOfDay: cd.bedtimeMin, journalDoneToday: BriefingStore.journalDoneToday(), sweepHeadline: await _sweepHeadlineNow(), diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index e65b32d7e..d04fec347 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -1024,7 +1024,7 @@ class _CoachSetupState extends State { _msg = null; }); try { - final first = await chatgpt.signIn( + await chatgpt.signIn( (uri) => launchUrl(uri, mode: LaunchMode.inAppBrowserView), ); await _closeBrowser(); @@ -1039,7 +1039,7 @@ class _CoachSetupState extends State { } }); if (!chatgpt.sharing) return; - if (first) await _welcome(); + if (await chatgpt.takeWelcome() && mounted) await _welcome(); if (mounted) await _fetch(); } catch (e) { await _closeBrowser(); diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 3d1a897db..0e0a02fa0 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -130,6 +130,9 @@ void main() { client: MockClient((req) async { expect(req.url.toString(), '$kChatGptIssuer/api/accounts/oauth/token'); tokenForms.add(req.bodyFields); + if (req.bodyFields['grant_type'] == 'authorization_code') { + expect(req.bodyFields['code'], 'code-1'); + } return http.Response( jsonEncode({ 'access_token': 'at-1', @@ -147,13 +150,10 @@ void main() { late Uri first, second; auth = makeAuth(() => (second.hasQuery ? second : first).queryParameters['nonce']!); second = Uri(); - expect( - await auth.signIn((u) { - first = u; - return browser(u, clientId: 'oaiapp_1'); - }), - isTrue, - ); + await auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); + }); final q = first.queryParameters; expect(q['client_id'], 'dynamic_agent_client'); expect(q['agent_name_hint'], 'OpenStrap Edge'); @@ -169,13 +169,10 @@ void main() { expect(auth.session!.email, 'me@example.com'); // Returning sign-in: the saved client id and host id, no name hint. - expect( - await auth.signIn((u) { - second = u; - return browser(u); - }), - isFalse, - ); + await auth.signIn((u) { + second = u; + return browser(u); + }); expect(second.queryParameters['client_id'], 'oaiapp_1'); expect(second.queryParameters['ext_agent_host_id'], q['ext_agent_host_id']); expect(second.queryParameters.containsKey('agent_name_hint'), isFalse); @@ -299,7 +296,30 @@ void main() { expect(reply['content'], 'hello'); }); + test('output comes from output_item.done when completed.output is empty', () async { + Map done(int i, Map item) => + {'type': 'response.output_item.done', 'output_index': i, 'item': item}; + Map empty() => { + 'type': 'response.completed', + 'response': {'status': 'completed', 'output': []}, + }; + final message = (_completed('hello')['response'] as Map)['output'][0] as Map; + var reply = await post(MockClient( + (_) async => http.Response(_sse([done(0, message), empty()]), 200))); + expect(reply['content'], 'hello'); + reply = await post(MockClient((_) async => http.Response( + _sse([ + done(1, {'type': 'function_call', 'call_id': 'c1', 'name': 'f', 'arguments': '{}', 'status': 'completed'}), + done(0, message), + empty(), + ]), + 200))); + expect(reply['content'], 'hello'); + expect((reply['tool_calls'] as List).single['id'], 'c1'); + }); + test('usage limit, mid-stream failure and a cut-off stream are errors, never a fallback', () async { + await cfg.save(apiKey: 'sk-test'); for (final (status, body, expected) in [ ( 429, @@ -318,10 +338,16 @@ void main() { ), (200, _sse([{'type': 'response.output_text.delta', 'delta': 'he'}]), 'ended early'), ]) { + final sent = []; await expectLater( - post(MockClient((_) async => http.Response(body, status))), + post(MockClient((req) async { + sent.add(req); + return http.Response(body, status); + })), throwsA(isA().having((e) => e.message, 'message', contains(expected))), ); + expect(sent, hasLength(1), reason: 'no fallback request'); + expect(sent.single.headers['Authorization'], 'Bearer at-1'); } }); @@ -331,6 +357,74 @@ void main() { }); }); + group('session lifecycle', () { + test('sign-out during an in-flight refresh does not resurrect the session', () async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final gate = Completer(); + final revoked = []; + final auth = ChatGptAuth(client: MockClient((req) async { + if (req.url.path.endsWith('/revoke')) { + revoked.add(req.bodyFields['token']!); + return http.Response('', 200); + } + await gate.future; + return http.Response( + jsonEncode({'access_token': 'at-2', 'refresh_token': 'rt-2', 'expires_in': 3600}), 200); + })); + await auth.debugSetSession(_session(expiresAt: DateTime.now())); + final refresh = auth.accessToken(); + final refreshed = expectLater(refresh, throwsA(isA())); + await pumpEventQueue(); + final out = auth.signOut(); + await pumpEventQueue(); + gate.complete(); + expect(await out, isTrue); + await refreshed; + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + expect(revoked, ['rt-2'], reason: 'the rotated token is the live one'); + }); + + test('a non-duplicate keychain write failure leaves the stored session alone', () async { + final auth = ChatGptAuth(); + await auth.debugSetSession(_session()); + final stored = keychain.values.single; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + if (call.method == 'write') throw PlatformException(code: 'boom', message: 'disk full'); + fail('${call.method} must not run'); + }); + await expectLater( + auth.debugSetSession(ChatGptSession( + sub: 'user-1', + email: 'me@example.com', + idToken: 'i', + accessToken: 'at-9', + refreshToken: 'rt-9', + scope: _fullScope, + expiresAt: DateTime.now().add(const Duration(hours: 1)))), throwsA(isA())); + expect(auth.session!.accessToken, 'at-1'); + expect(keychain.values.single, stored); + }); + + test('forgetAll clears the session, registration and welcome flag; welcome shows once', () async { + SharedPreferences.setMockInitialValues({ + 'coach_chatgpt_client_id': 'oaiapp_1', + 'coach_chatgpt_host_id': 'urn:uuid:h', + }); + final auth = ChatGptAuth(); + await auth.debugSetSession(_session()); + expect(await auth.takeWelcome(), isTrue); + expect(await auth.takeWelcome(), isFalse); + await auth.forgetAll(); + final prefs = await SharedPreferences.getInstance(); + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + expect(prefs.getKeys().where((k) => k.startsWith('coach_chatgpt_')), isEmpty); + expect(await auth.takeWelcome(), isTrue); + }); + }); + test('model list keeps listed slugs in server order', () async { final auth = ChatGptAuth( client: MockClient((req) async { From d482905f90929549ab0e25d364414d1281ae0967 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 16:24:18 -0500 Subject: [PATCH 03/23] fix(coach): serialize ChatGPT session commits with sign-out; fail closed on load - signIn captures the session generation and commits through the same serialized store as refresh; the generation is checked inside the lock before and after the write, so a sign-out during a pending sign-in or a mid-write store cannot leave a session behind. - signOut/forgetAll cancel a pending browser sign-in and clear the keychain inside the same queue. - load restores a keychain record only when the present marker vouches for it; an orphan (failed sign-out delete, reinstall leftover) is deleted. --- lib/coach/coach_chatgpt.dart | 152 +++++++++++++++++++++++------------ test/coach_chatgpt_test.dart | 58 +++++++++++++ 2 files changed, 157 insertions(+), 53 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index d741ce190..6d926cdb2 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -290,12 +290,25 @@ class ChatGptAuth extends ChangeNotifier { Future? _refreshing; /// Bumped whenever a sign-in commits a session, and at the very start of - /// [signOut]/[forgetAll]. A [load] or [_refresh] that started under an older - /// value must not apply its result — a slow keychain read or token call + /// [signOut]/[forgetAll]. A [load], [_refresh] or [signIn] that started under + /// an older value must not apply its result — a slow keychain read or token call /// would otherwise bring back a session the user just ended (same idea as /// CoachConfig._generation). int _generation = 0; + /// Keychain writes, deletes and the marker run one at a time, like + /// CoachConfig._serialized: a sign-out can then never interleave with a + /// store's awaits — it runs after one already writing, and ahead of any + /// still queued (which see the new generation and are rejected). + Future _lock = Future.value(); + + Future _serialized(Future Function() op) { + final done = _lock.then((_) => op()); + // A failed operation must not wedge the queue. + _lock = done.catchError((_) {}); + return done; + } + /// The refresh token a refresh rotated to after [signOut] began. It is not /// stored, but it is the live one, so it is the one to revoke. String? _rotatedAfterSignOut; @@ -324,7 +337,19 @@ class ChatGptAuth extends ChangeNotifier { try { final raw = await _secure.read(key: _kSession, iOptions: _apple, mOptions: _macos); if (generation != _generation) return; - if (raw != null && raw.isNotEmpty) { + if (raw != null && raw.isNotEmpty && !present) { + // Fails closed: every store writes the marker, so a record without it + // is one a sign-out could not delete (or a reinstall's leftover). + await _serialized(() async { + if (generation != _generation) return; // a sign-in just committed + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} + }); + if (generation != _generation) return; + _session = null; + _unreadable = false; + } else if (raw != null && raw.isNotEmpty) { _session = ChatGptSession.fromJson(jsonDecode(raw) as Map); _unreadable = false; } else if (trusted || !present) { @@ -341,35 +366,36 @@ class ChatGptAuth extends ChangeNotifier { notifyListeners(); } - /// A refresh passes the [generation] it started under: if the session was - /// signed out or replaced since, its result is dropped before anything is - /// written. [signOut] waits for an in-flight refresh, so none can land after - /// it clears. - Future _store(ChatGptSession? s, {int? generation}) async { - final prefs = await SharedPreferences.getInstance(); - if (generation != null && generation != _generation) { - throw ChatGptException('Signed out.'); - } - if (s == null) { - await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); - } else { - final raw = jsonEncode(s.toJson()); - try { - await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); - } on Object catch (e) { - // Only a leftover item from a previous install (see CoachConfig.save) - // is worth deleting; any other failure leaves the stored session alone. - if (!isKeychainDuplicateItemError(e)) rethrow; - await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); - await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); - } - } - await prefs.setBool(_kPresent, s != null); - if (generation == null) _generation++; - _session = s; - _unreadable = false; - notifyListeners(); - } + /// Every caller passes the [generation] it started under, checked INSIDE the + /// lock before any write: if the session was signed out or replaced since, + /// the result is dropped. A sign-in or [debugSetSession] passes `commit` to + /// start a new generation once its write lands. + Future _store(ChatGptSession? s, {required int generation, bool commit = false}) => + _serialized(() async { + if (generation != _generation) throw ChatGptException('Signed out.'); + final prefs = await SharedPreferences.getInstance(); + if (s == null) { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } else { + final raw = jsonEncode(s.toJson()); + try { + await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); + } on Object catch (e) { + // Only a leftover item from a previous install (see CoachConfig.save) + // is worth deleting; any other failure leaves the stored session alone. + if (!isKeychainDuplicateItemError(e)) rethrow; + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + await _secure.write(key: _kSession, value: raw, iOptions: _apple, mOptions: _macos); + } + } + await prefs.setBool(_kPresent, s != null); + // A sign-out that began mid-write clears the keychain right after us. + if (generation != _generation) throw ChatGptException('Signed out.'); + if (commit) _generation++; + _session = s; + _unreadable = false; + notifyListeners(); + }); static String _random() { final r = Random.secure(); @@ -380,6 +406,7 @@ class ChatGptAuth extends ChangeNotifier { /// this app alive while it is up (an in-app browser), because the redirect /// lands on a listener on this device's loopback. Future signIn(Future Function(Uri) open) async { + final generation = _generation; final prefs = await SharedPreferences.getInstance(); final hostId = prefs.getString(_kHostId) ?? 'urn:uuid:${const Uuid().v4()}'; await prefs.setString(_kHostId, hostId); @@ -437,7 +464,11 @@ class ChatGptAuth extends ChangeNotifier { final idToken = t['id_token'] as String?; if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); - await _store(_sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken)); + await _store( + _sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken), + generation: generation, + commit: true, + ); } finally { if (identical(_pending, server)) _pending = null; await server.close(force: true); @@ -545,7 +576,13 @@ class ChatGptAuth extends ChangeNotifier { _rotatedAfterSignOut = next.refreshToken; throw ChatGptException('Signed out.'); } - await _store(next, generation: generation); + try { + await _store(next, generation: generation); + } catch (_) { + // Signed out while it was writing: the rotated token is the live one. + if (generation != _generation) _rotatedAfterSignOut = next.refreshToken; + rethrow; + } return next.accessToken; } @@ -554,17 +591,18 @@ class ChatGptAuth extends ChangeNotifier { /// the user can still disconnect the app in ChatGPT settings. Future signOut() async { _generation++; // before any await: see [_generation] + cancelSignIn(); + final s = _session; + // Memory first, so nothing keeps using the session while revocation is out. + _session = null; + _unreadable = false; + notifyListeners(); // An in-flight refresh may have rotated the token; revoke the latest one. try { await _refreshing; } catch (_) {} - final s = _session; final token = _rotatedAfterSignOut ?? s?.refreshToken; _rotatedAfterSignOut = null; - // Memory first, so nothing keeps using the session while revocation is out. - _session = null; - _unreadable = false; - notifyListeners(); final prefs = await SharedPreferences.getInstance(); final clientId = prefs.getString(_kClientId); var revoked = token == null; @@ -578,27 +616,34 @@ class ChatGptAuth extends ChangeNotifier { revoked = r.statusCode == 200; } catch (_) {} } - // Signed out either way. If the delete fails the item may linger, but its - // token was just revoked, so a later load just ends it at the next refresh. - try { - await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); - } catch (_) {} - await prefs.setBool(_kPresent, false); + // Signed out either way. If the delete fails the item may linger, but the + // marker is cleared and [load] fails closed on a record without it, so it + // is never restored (it is deleted then); the credential is not guaranteed + // gone until that happens. + await _serialized(() async { + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} + await prefs.setBool(_kPresent, false); + }); return revoked; } /// "Reset all data": the session plus the registration and welcome flag. Future forgetAll() async { _generation++; + cancelSignIn(); _session = null; _unreadable = false; - try { - await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); - } catch (_) {} - final prefs = await SharedPreferences.getInstance(); - for (final k in [_kClientId, _kHostId, _kPresent, _kWelcomed]) { - await prefs.remove(k); - } + await _serialized(() async { + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} + final prefs = await SharedPreferences.getInstance(); + for (final k in [_kClientId, _kHostId, _kPresent, _kWelcomed]) { + await prefs.remove(k); + } + }); notifyListeners(); } @@ -631,7 +676,8 @@ class ChatGptAuth extends ChangeNotifier { } @visibleForTesting - Future debugSetSession(ChatGptSession? s) => _store(s); + Future debugSetSession(ChatGptSession? s) => + _store(s, generation: _generation, commit: true); } class _TokenError implements Exception { diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 0e0a02fa0..3e3a53f70 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -69,9 +69,11 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); const channel = MethodChannel('plugins.it_nomads.com/flutter_secure_storage'); late Map keychain; + Completer? writeGate; // hangs keychain writes until completed setUp(() { keychain = {}; + writeGate = null; SharedPreferences.setMockInitialValues({}); TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(channel, (call) async { @@ -80,6 +82,7 @@ void main() { case 'read': return keychain[args['key']]; case 'write': + await writeGate?.future; keychain[args['key'] as String] = args['value'] as String; case 'delete': keychain.remove(args['key']); @@ -179,6 +182,50 @@ void main() { expect(second.queryParameters['id_token_hint'], isNotNull); }); + test('sign-out while the code exchange is pending leaves nothing stored', () async { + final gate = Completer(), arrived = Completer(); + late Uri first; + auth = ChatGptAuth(client: MockClient((req) async { + arrived.complete(); + await gate.future; + return http.Response( + jsonEncode({ + 'access_token': 'at-1', + 'refresh_token': 'rt-1', + 'id_token': _idToken(req.bodyFields['client_id']!, first.queryParameters['nonce']!), + 'scope': _fullScope, + }), + 200); + })); + final signIn = expectLater( + auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); + }), + throwsA(isA())); + await arrived.future; + await auth.signOut(); + gate.complete(); + await signIn; + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + }); + + test('a sign-in already writing when sign-out starts is cleared afterwards', () async { + writeGate = Completer(); + final auth = ChatGptAuth(); + final store = expectLater(auth.debugSetSession(_session()), throwsA(isA())); + await pumpEventQueue(); + final out = auth.signOut(); + await pumpEventQueue(); + writeGate!.complete(); + await out; + await store; + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + expect((await SharedPreferences.getInstance()).getBool('coach_chatgpt_present'), isFalse); + }); + test('a declined consent ends sign-in without a token exchange', () async { auth = makeAuth(() => ''); await expectLater( @@ -407,6 +454,17 @@ void main() { expect(keychain.values.single, stored); }); + for (final prefs in [{'coach_chatgpt_present': false}, {}]) { + test('load drops a keychain record the present marker does not vouch for ($prefs)', () async { + SharedPreferences.setMockInitialValues(prefs); + keychain['coach_chatgpt_session'] = jsonEncode(_session().toJson()); + final auth = ChatGptAuth(); + await auth.load(trusted: true); + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + }); + } + test('forgetAll clears the session, registration and welcome flag; welcome shows once', () async { SharedPreferences.setMockInitialValues({ 'coach_chatgpt_client_id': 'oaiapp_1', From 1dc294ed40e3f21da198e9478348c298bee7a6e2 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 16:34:05 -0500 Subject: [PATCH 04/23] fix(coach): forget a parked rotated token on reset and on a new sign-in forgetAll waits for an in-flight refresh and drops the rotated token it parks for sign-out; a committed sign-in clears it too, so a later sign-out revokes the current session's refresh token rather than a stale one. --- lib/coach/coach_chatgpt.dart | 13 ++++++++++++- test/coach_chatgpt_test.dart | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 6d926cdb2..473a74895 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -391,7 +391,11 @@ class ChatGptAuth extends ChangeNotifier { await prefs.setBool(_kPresent, s != null); // A sign-out that began mid-write clears the keychain right after us. if (generation != _generation) throw ChatGptException('Signed out.'); - if (commit) _generation++; + if (commit) { + _generation++; + // A new session: an earlier one's rotated token is not this one's. + _rotatedAfterSignOut = null; + } _session = s; _unreadable = false; notifyListeners(); @@ -635,6 +639,13 @@ class ChatGptAuth extends ChangeNotifier { cancelSignIn(); _session = null; _unreadable = false; + // An in-flight refresh parks its rotated token for a sign-out to revoke; + // with the registration forgotten there is nothing to revoke it with, and + // left behind it would be revoked in place of a later session's token. + try { + await _refreshing; + } catch (_) {} + _rotatedAfterSignOut = null; await _serialized(() async { try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 3e3a53f70..b1c914b9b 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -432,6 +432,40 @@ void main() { expect(revoked, ['rt-2'], reason: 'the rotated token is the live one'); }); + test('a reset during a refresh does not make a later sign-out revoke the stale token', () async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final gate = Completer(); + final revoked = []; + final auth = ChatGptAuth(client: MockClient((req) async { + if (req.url.path.endsWith('/revoke')) { + revoked.add(req.bodyFields['token']!); + return http.Response('', 200); + } + await gate.future; + return http.Response( + jsonEncode({'access_token': 'at-2', 'refresh_token': 'rt-2', 'expires_in': 3600}), 200); + })); + await auth.debugSetSession(_session(expiresAt: DateTime.now())); + final refreshed = expectLater(auth.accessToken(), throwsA(isA())); + await pumpEventQueue(); + final reset = auth.forgetAll(); + gate.complete(); + await reset; + await refreshed; + // Signed in again, then out: the new session's token is the one revoked. + (await SharedPreferences.getInstance()).setString('coach_chatgpt_client_id', 'oaiapp_2'); + await auth.debugSetSession(ChatGptSession( + sub: 'user-1', + email: 'me@example.com', + idToken: 'i', + accessToken: 'at-new', + refreshToken: 'rt-new', + scope: _fullScope, + expiresAt: DateTime.now().add(const Duration(hours: 1)))); + await auth.signOut(); + expect(revoked, ['rt-new']); + }); + test('a non-duplicate keychain write failure leaves the stored session alone', () async { final auth = ChatGptAuth(); await auth.debugSetSession(_session()); From 2ad8eb85f2212625a49826e72c45dbef7b641de1 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 16:46:02 -0500 Subject: [PATCH 05/23] fix(coach): a slow sign-out no longer deletes a newer sign-in signOut remembers its generation and skips its serialized keychain delete when a sign-in committed (or another sign-out began) while revocation was out, so signing back in during that window keeps the new session. --- lib/coach/coach_chatgpt.dart | 5 ++++- test/coach_chatgpt_test.dart | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 473a74895..2a619d21b 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -594,7 +594,7 @@ class ChatGptAuth extends ChangeNotifier { /// revocation could not be confirmed; the tokens are cleared regardless, and /// the user can still disconnect the app in ChatGPT settings. Future signOut() async { - _generation++; // before any await: see [_generation] + final generation = ++_generation; // before any await: see [_generation] cancelSignIn(); final s = _session; // Memory first, so nothing keeps using the session while revocation is out. @@ -625,6 +625,9 @@ class ChatGptAuth extends ChangeNotifier { // is never restored (it is deleted then); the credential is not guaranteed // gone until that happens. await _serialized(() async { + // The button to sign in again is back while revocation is out: a sign-in + // that committed since (or a later sign-out) owns the keychain now. + if (generation != _generation) return; try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); } catch (_) {} diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index b1c914b9b..15440b006 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -466,6 +466,24 @@ void main() { expect(revoked, ['rt-new']); }); + test('a sign-in during a slow sign-out survives it', () async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final gate = Completer(); + final auth = ChatGptAuth(client: MockClient((req) async { + await gate.future; // revocation still out + return http.Response('', 200); + })); + await auth.debugSetSession(_session()); + final out = auth.signOut(); + await pumpEventQueue(); + await auth.debugSetSession(_session(scope: '$_fullScope x')); // signed back in + gate.complete(); + await out; + expect(auth.signedIn, isTrue); + expect(keychain, isNotEmpty); + expect((await SharedPreferences.getInstance()).getBool('coach_chatgpt_present'), isTrue); + }); + test('a non-duplicate keychain write failure leaves the stored session alone', () async { final auth = ChatGptAuth(); await auth.debugSetSession(_session()); From f71cef9261acf81a64280267f96a16d1c60ce182 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 22 Oct 2026 12:32:57 -0500 Subject: [PATCH 06/23] fix(coach): revoke on reset, keep a rotated token when the keychain write fails - forgetAll revokes the live refresh token (best effort) before forgetting the registration it needs to do so - _refresh keeps the rotated session in memory when persisting it fails, so the spent refresh token is never reused - AGENTS.md lists the ChatGPT sign-in's network calls --- AGENTS.md | 3 +- lib/coach/coach_chatgpt.dart | 49 +++++++++++++++++---------- test/coach_chatgpt_test.dart | 65 +++++++++++++++++++++++++++--------- 3 files changed, 84 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 19a8af877..4e2962023 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,8 @@ symbol names don't; verify against the source, not this doc. A Flutter health app for wearables (WHOOP 4.0/5.0/MG supported today). **Fully on-device, local-first**: BLE offload → SQLite → on-device analytics → UI. No backend owns user data. Network use is limited to OTA update pointers, opt-in -telemetry/Crashlytics, and BYOK LLM calls. +telemetry/Crashlytics, and BYOK LLM calls (or the user's ChatGPT sign-in, +whose OAuth calls go to auth.openai.com). Three sibling repos, strict separation — push work to the right one: - `OpenStrap/protocol` — bytes: GATT, framing, CRC, opcodes, record decode. diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 2a619d21b..149184cfa 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -583,9 +583,14 @@ class ChatGptAuth extends ChangeNotifier { try { await _store(next, generation: generation); } catch (_) { - // Signed out while it was writing: the rotated token is the live one. - if (generation != _generation) _rotatedAfterSignOut = next.refreshToken; - rethrow; + if (generation != _generation) { + // Signed out while it was writing: the rotated token is the live one. + _rotatedAfterSignOut = next.refreshToken; + rethrow; + } + // The keychain write failed, but the old refresh token is already spent: + // keep the rotated session in memory so the next refresh does not reuse it. + _session = next; } return next.accessToken; } @@ -609,17 +614,7 @@ class ChatGptAuth extends ChangeNotifier { _rotatedAfterSignOut = null; final prefs = await SharedPreferences.getInstance(); final clientId = prefs.getString(_kClientId); - var revoked = token == null; - if (token != null && clientId != null) { - try { - final r = await _http((c) => c.post(Uri.parse(_revokeUrl), body: { - 'token': token, - 'token_type_hint': 'refresh_token', - 'client_id': clientId, - }).timeout(const Duration(seconds: 15))); - revoked = r.statusCode == 200; - } catch (_) {} - } + final revoked = token == null || (clientId != null && await _revoke(token, clientId)); // Signed out either way. If the delete fails the item may linger, but the // marker is cleared and [load] fails closed on a record without it, so it // is never restored (it is deleted then); the credential is not guaranteed @@ -636,19 +631,39 @@ class ChatGptAuth extends ChangeNotifier { return revoked; } + /// True when OpenAI confirmed the refresh token is revoked. + Future _revoke(String token, String clientId) async { + try { + final r = await _http((c) => c.post(Uri.parse(_revokeUrl), body: { + 'token': token, + 'token_type_hint': 'refresh_token', + 'client_id': clientId, + }).timeout(const Duration(seconds: 15))); + return r.statusCode == 200; + } catch (_) { + return false; + } + } + /// "Reset all data": the session plus the registration and welcome flag. + /// The session is revoked first, best effort, while the registration to + /// revoke it with is still here. Future forgetAll() async { _generation++; cancelSignIn(); + final s = _session; _session = null; _unreadable = false; - // An in-flight refresh parks its rotated token for a sign-out to revoke; - // with the registration forgotten there is nothing to revoke it with, and - // left behind it would be revoked in place of a later session's token. + // An in-flight refresh may have rotated the token; revoke the latest one. + // It is cleared here too: left behind, it would be revoked in place of a + // later session's token. try { await _refreshing; } catch (_) {} + final token = _rotatedAfterSignOut ?? s?.refreshToken; _rotatedAfterSignOut = null; + final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); + if (token != null && clientId != null) await _revoke(token, clientId); await _serialized(() async { try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 15440b006..ec57036e2 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -463,7 +463,8 @@ void main() { scope: _fullScope, expiresAt: DateTime.now().add(const Duration(hours: 1)))); await auth.signOut(); - expect(revoked, ['rt-new']); + // The reset revoked the rotated token; the sign-out revoked only its own. + expect(revoked, ['rt-2', 'rt-new']); }); test('a sign-in during a slow sign-out survives it', () async { @@ -517,21 +518,55 @@ void main() { }); } - test('forgetAll clears the session, registration and welcome flag; welcome shows once', () async { - SharedPreferences.setMockInitialValues({ - 'coach_chatgpt_client_id': 'oaiapp_1', - 'coach_chatgpt_host_id': 'urn:uuid:h', + for (final status in [200, 500]) { + test('forgetAll revokes, then clears the session, registration and welcome flag; ' + 'welcome shows once (revoke $status)', () async { + SharedPreferences.setMockInitialValues({ + 'coach_chatgpt_client_id': 'oaiapp_1', + 'coach_chatgpt_host_id': 'urn:uuid:h', + }); + final revoked = []; + final auth = ChatGptAuth(client: MockClient((req) async { + expect(req.url.path, endsWith('/revoke')); + expect(req.bodyFields['client_id'], 'oaiapp_1'); + revoked.add(req.bodyFields['token']!); + return http.Response('', status); + })); + await auth.debugSetSession(_session()); + expect(await auth.takeWelcome(), isTrue); + expect(await auth.takeWelcome(), isFalse); + await auth.forgetAll(); + final prefs = await SharedPreferences.getInstance(); + expect(revoked, ['rt-1']); + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + expect(prefs.getKeys().where((k) => k.startsWith('coach_chatgpt_')), isEmpty); + expect(await auth.takeWelcome(), isTrue); }); - final auth = ChatGptAuth(); - await auth.debugSetSession(_session()); - expect(await auth.takeWelcome(), isTrue); - expect(await auth.takeWelcome(), isFalse); - await auth.forgetAll(); - final prefs = await SharedPreferences.getInstance(); - expect(auth.signedIn, isFalse); - expect(keychain, isEmpty); - expect(prefs.getKeys().where((k) => k.startsWith('coach_chatgpt_')), isEmpty); - expect(await auth.takeWelcome(), isTrue); + } + + test('a refresh whose keychain write fails keeps the rotated token in memory', () async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final sent = []; + var n = 0; + final auth = ChatGptAuth(client: MockClient((req) async { + sent.add(req.bodyFields['refresh_token']!); + n++; + return http.Response( + jsonEncode({'access_token': 'at-${n + 1}', 'refresh_token': 'rt-${n + 1}', 'expires_in': 1}), + 200); + })); + await auth.debugSetSession(_session(expiresAt: DateTime.now())); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + if (call.method == 'write') throw PlatformException(code: 'boom', message: 'disk full'); + fail('${call.method} must not run'); + }); + expect(await auth.accessToken(), 'at-2'); + expect(auth.session!.refreshToken, 'rt-2'); + // Still expiring (expires_in: 1), so the next call refreshes again. + expect(await auth.accessToken(), 'at-3'); + expect(sent, ['rt-1', 'rt-2'], reason: 'the spent token is never sent again'); }); }); From bac96d6a2bdd1c73a4c9420d806ab76a341b384f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:36:09 +0530 Subject: [PATCH 07/23] coach: verify the chatgpt id token signature against openai's jwks the sign-in trusted the id token's claims without checking its signature. it is now verified as rs256 against the keys at the discovery document's jwks_uri before sub/email are used. the keys are cached, an unknown kid refetches once, and anything that can't be verified fails the sign-in. --- lib/coach/coach_chatgpt.dart | 22 +++- lib/coach/coach_chatgpt_jwks.dart | 178 ++++++++++++++++++++++++++++++ test/coach_chatgpt_test.dart | 154 +++++++++++++++++++++++++- 3 files changed, 346 insertions(+), 8 deletions(-) create mode 100644 lib/coach/coach_chatgpt_jwks.dart diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 149184cfa..c68b034e4 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -19,6 +19,8 @@ import 'package:http/http.dart' as http; import 'package:shared_preferences/shared_preferences.dart'; import 'package:uuid/uuid.dart'; +import 'coach_chatgpt_jwks.dart'; + const kChatGptIssuer = 'https://auth.openai.com'; /// Both the OAuth `resource` and the Responses API base requests go to. @@ -86,11 +88,9 @@ Uri chatGptAuthorizeUri({ String chatGptCodeChallenge(String verifier) => base64Url.encode(sha256.convert(ascii.encode(verifier)).bytes).replaceAll('=', ''); -/// The ID token's `sub` and `email` once its claims check out. -/// -/// The signature is not verified: the token comes straight from the token -/// endpoint over TLS in a PKCE code exchange, where OIDC Core §3.1.3.7(6) -/// allows TLS server validation in its place. iss/aud/exp/nonce still are. +/// The ID token's `sub` and `email` once its claims check out. Only the +/// claims are checked here (iss/aud/exp/nonce); the caller verifies the +/// signature first with [ChatGptJwks.verify]. ({String sub, String email}) chatGptIdentity( String idToken, { required String clientId, @@ -284,6 +284,12 @@ class ChatGptAuth extends ChangeNotifier { final FlutterSecureStorage _secure = const FlutterSecureStorage(); final http.Client? _client; + /// OpenAI's signing keys, cached for the life of this object. + late final ChatGptJwks _jwks = ChatGptJwks( + issuer: kChatGptIssuer, + get: (u) => _http((c) => c.get(u).timeout(const Duration(seconds: 20))), + ); + ChatGptSession? _session; bool _unreadable = false; HttpServer? _pending; @@ -467,6 +473,12 @@ class ChatGptAuth extends ChangeNotifier { } final idToken = t['id_token'] as String?; if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); + try { + await _jwks.verify(idToken); + } on ChatGptSignatureException catch (e) { + debugPrint('[chatgpt] $e'); + throw ChatGptException('ChatGPT returned a sign-in that did not check out.'); + } final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); await _store( _sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken), diff --git a/lib/coach/coach_chatgpt_jwks.dart b/lib/coach/coach_chatgpt_jwks.dart new file mode 100644 index 000000000..af18a94c8 --- /dev/null +++ b/lib/coach/coach_chatgpt_jwks.dart @@ -0,0 +1,178 @@ +// Signature check for the ChatGPT ID token: RS256 against the keys OpenAI +// publishes at the discovery document's `jwks_uri`. Everything here fails +// closed — a token is trusted only when its header, key and signature all +// check out, and any fetch or parse problem is a failed sign-in, never a skip. + +import 'dart:convert'; +import 'dart:typed_data'; + +import 'package:clock/clock.dart'; +import 'package:http/http.dart' as http; +import 'package:pointycastle/export.dart'; + +/// Raised for any ID token whose signature could not be verified. +class ChatGptSignatureException implements Exception { + final String reason; + ChatGptSignatureException(this.reason); + @override + String toString() => 'ID token signature not verified: $reason'; +} + +/// One RSA signing key from a JWKS. +class ChatGptJwk { + final String kid; + final RSAPublicKey key; + const ChatGptJwk(this.kid, this.key); +} + +BigInt _uint(String b64) { + final bytes = base64Url.decode(base64Url.normalize(b64)); + var v = BigInt.zero; + for (final b in bytes) { + v = (v << 8) | BigInt.from(b); + } + return v; +} + +/// The usable RS256 signing keys in a JWKS document. Keys for another +/// algorithm, for encryption, or shorter than 2048 bits are left out. +Map parseChatGptJwks(String body) { + final Object? j; + try { + j = jsonDecode(body); + } catch (_) { + throw ChatGptSignatureException('unreadable key set'); + } + final keys = j is Map ? j['keys'] : null; + if (keys is! List) throw ChatGptSignatureException('unreadable key set'); + final out = {}; + for (final k in keys) { + if (k is! Map) continue; + final kid = k['kid'], n = k['n'], e = k['e']; + if (k['kty'] != 'RSA' || kid is! String || n is! String || e is! String) continue; + if (k['alg'] != null && k['alg'] != 'RS256') continue; + if (k['use'] != null && k['use'] != 'sig') continue; + try { + final modulus = _uint(n); + if (modulus.bitLength < 2048) continue; + out[kid] = ChatGptJwk(kid, RSAPublicKey(modulus, _uint(e))); + } catch (_) { + continue; + } + } + return out; +} + +/// The `kid` of an RS256 JWT, after checking the header asks for exactly +/// that algorithm (no `none`, no HMAC, no key embedded in the token). +String chatGptIdTokenKid(String jwt) { + final parts = jwt.split('.'); + if (parts.length != 3) throw ChatGptSignatureException('not a signed JWT'); + final Object? header; + try { + header = jsonDecode(utf8.decode(base64Url.decode(base64Url.normalize(parts[0])))); + } catch (_) { + throw ChatGptSignatureException('unreadable header'); + } + if (header is! Map) throw ChatGptSignatureException('unreadable header'); + if (header['alg'] != 'RS256') { + throw ChatGptSignatureException('unexpected alg ${header['alg']}'); + } + if (header.containsKey('jwk') || header.containsKey('jku') || header.containsKey('x5u')) { + throw ChatGptSignatureException('token names its own key'); + } + if (header['crit'] != null) throw ChatGptSignatureException('unsupported crit header'); + final kid = header['kid']; + if (kid is! String || kid.isEmpty) throw ChatGptSignatureException('no kid'); + return kid; +} + +/// True when [jwt]'s RS256 signature verifies under [key]. +bool chatGptVerifyRs256(String jwt, RSAPublicKey key) { + final parts = jwt.split('.'); + if (parts.length != 3) return false; + final Uint8List sig; + try { + sig = Uint8List.fromList(base64Url.decode(base64Url.normalize(parts[2]))); + } catch (_) { + return false; + } + final signer = RSASigner(SHA256Digest(), '0609608648016503040201') + ..init(false, PublicKeyParameter(key)); + try { + return signer.verifySignature( + Uint8List.fromList(ascii.encode('${parts[0]}.${parts[1]}')), + RSASignature(sig), + ); + } catch (_) { + return false; + } +} + +/// OpenAI's published signing keys, fetched from the discovery document's +/// `jwks_uri` and cached for the life of the object. A token signed with a +/// `kid` not in the cache triggers one refetch (key rotation); a `kid` still +/// missing after that is rejected. +class ChatGptJwks { + ChatGptJwks({required this.issuer, required this.get}); + + final String issuer; + + /// Runs one request with the caller's client. + final Future Function(Uri) get; + + Map? _keys; + DateTime? _fetchedAt; + + /// Refetching on every unknown kid would let a stream of forged tokens turn + /// into a stream of requests; one sign-in only ever needs one refetch. + static const _minRefetch = Duration(seconds: 30); + + Future> _fetch() async { + final discovery = await get(Uri.parse('$issuer/.well-known/openid-configuration')); + if (discovery.statusCode != 200) { + throw ChatGptSignatureException('discovery returned ${discovery.statusCode}'); + } + final Object? d; + try { + d = jsonDecode(discovery.body); + } catch (_) { + throw ChatGptSignatureException('unreadable discovery document'); + } + if (d is! Map || d['issuer'] != issuer || d['jwks_uri'] is! String) { + throw ChatGptSignatureException('unexpected discovery document'); + } + final jwksUri = Uri.tryParse(d['jwks_uri'] as String); + if (jwksUri == null || jwksUri.scheme != 'https' || jwksUri.host.isEmpty) { + throw ChatGptSignatureException('jwks_uri is not https'); + } + final r = await get(jwksUri); + if (r.statusCode != 200) throw ChatGptSignatureException('jwks returned ${r.statusCode}'); + final keys = parseChatGptJwks(r.body); + _keys = keys; + _fetchedAt = clock.now(); + return keys; + } + + /// Throws [ChatGptSignatureException] unless [jwt] is RS256-signed by one of + /// the issuer's current keys. + Future verify(String jwt) async { + final kid = chatGptIdTokenKid(jwt); + var keys = _keys; + if (keys == null || !keys.containsKey(kid)) { + final last = _fetchedAt; + if (keys == null || last == null || clock.now().difference(last) >= _minRefetch) { + try { + keys = await _fetch(); + } on ChatGptSignatureException { + rethrow; + } catch (e) { + throw ChatGptSignatureException('could not fetch keys: $e'); + } + } + } + final jwk = keys[kid]; + if (jwk == null) throw ChatGptSignatureException('unknown kid'); + if (!chatGptVerifyRs256(jwt, jwk.key)) throw ChatGptSignatureException('bad signature'); + } +} diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index ec57036e2..c08c36c66 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -5,22 +5,82 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:clock/clock.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/io_client.dart'; import 'package:http/testing.dart'; import 'package:openstrap_edge/coach/coach_chatgpt.dart'; +import 'package:openstrap_edge/coach/coach_chatgpt_jwks.dart'; import 'package:openstrap_edge/coach/coach_config.dart'; import 'package:openstrap_edge/coach/coach_engine.dart'; +import 'package:pointycastle/export.dart'; import 'package:shared_preferences/shared_preferences.dart'; // Flutter's test binding fakes HttpClient; this one really reaches loopback. class _RealHttpOverrides extends HttpOverrides {} -String _jwt(Map claims) { - String b64(Object o) => base64Url.encode(utf8.encode(jsonEncode(o))).replaceAll('=', ''); - return '${b64({'alg': 'RS256'})}.${b64(claims)}.sig'; +// A real RS256 key pair, so the ID token goes through the same signature +// check as OpenAI's. Generated once: 2048-bit generation is the slow part. +final AsymmetricKeyPair _rsa = () { + final rnd = FortunaRandom() + ..seed(KeyParameter(Uint8List.fromList(List.generate(32, (i) => i * 7 + 1)))); + final gen = RSAKeyGenerator() + ..init(ParametersWithRandom(RSAKeyGeneratorParameters(BigInt.from(65537), 2048, 64), rnd)); + final pair = gen.generateKeyPair(); + return AsymmetricKeyPair(pair.publicKey as RSAPublicKey, pair.privateKey as RSAPrivateKey); +}(); + +const _kid = 'test-kid'; + +String _b64(Object o) => base64Url.encode(utf8.encode(jsonEncode(o))).replaceAll('=', ''); + +String _b64Int(BigInt v) { + final bytes = []; + while (v > BigInt.zero) { + bytes.insert(0, (v & BigInt.from(0xff)).toInt()); + v = v >> 8; + } + return base64Url.encode(bytes).replaceAll('=', ''); +} + +String _sign(String header, String claims, [RSAPrivateKey? key]) { + final signer = RSASigner(SHA256Digest(), '0609608648016503040201') + ..init(true, PrivateKeyParameter(key ?? _rsa.privateKey)); + final sig = signer.generateSignature(Uint8List.fromList(ascii.encode('$header.$claims'))); + return base64Url.encode(sig.bytes).replaceAll('=', ''); +} + +String _jwt(Map claims, {Map? header}) { + final h = _b64(header ?? {'alg': 'RS256', 'kid': _kid, 'typ': 'JWT'}); + final c = _b64(claims); + return '$h.$c.${_sign(h, c)}'; +} + +String _jwksBody([String kid = _kid]) => jsonEncode({ + 'keys': [ + { + 'kty': 'RSA', + 'kid': kid, + 'alg': 'RS256', + 'use': 'sig', + 'n': _b64Int(_rsa.publicKey.modulus!), + 'e': _b64Int(_rsa.publicKey.exponent!), + }, + ], + }); + +/// Serves OpenAI's discovery document and key set; null for anything else. +http.Response? _oidc(http.BaseRequest req, {String kid = _kid}) { + final u = req.url.toString(); + if (u == '$kChatGptIssuer/.well-known/openid-configuration') { + return http.Response( + jsonEncode({'issuer': kChatGptIssuer, 'jwks_uri': '$kChatGptIssuer/.well-known/jwks.json'}), + 200); + } + if (u == '$kChatGptIssuer/.well-known/jwks.json') return http.Response(_jwksBody(kid), 200); + return null; } String _idToken(String clientId, String nonce, {String iss = kChatGptIssuer, int? exp}) => @@ -131,6 +191,8 @@ void main() { ChatGptAuth makeAuth(String Function() nonce) => ChatGptAuth( client: MockClient((req) async { + final oidc = _oidc(req); + if (oidc != null) return oidc; expect(req.url.toString(), '$kChatGptIssuer/api/accounts/oauth/token'); tokenForms.add(req.bodyFields); if (req.bodyFields['grant_type'] == 'authorization_code') { @@ -263,6 +325,92 @@ void main() { } }); + group('ID token signature', () { + late int fetches; + late String servedKid; + late bool down; + late ChatGptJwks jwks; + setUp(() { + fetches = 0; + servedKid = _kid; + down = false; + jwks = ChatGptJwks( + issuer: kChatGptIssuer, + get: (u) async { + if (down) throw const SocketException('offline'); + if (u.path.endsWith('jwks.json')) fetches++; + return _oidc(http.Request('GET', u), kid: servedKid)!; + }, + ); + }); + + String claims() => _b64({'iss': kChatGptIssuer, 'sub': 'user-1'}); + + test('a token signed by a published key verifies, and the keys are cached', () async { + await jwks.verify(_idToken('oaiapp_1', 'n1')); + await jwks.verify(_idToken('oaiapp_1', 'n2')); + expect(fetches, 1); + }); + + test('a forged, re-signed or unsigned token is rejected', () async { + final good = _idToken('oaiapp_1', 'n1').split('.'); + final h = _b64({'alg': 'RS256', 'kid': _kid}); + for (final token in [ + '${good[0]}.${claims()}.${good[2]}', // claims swapped under the old signature + '${good[0]}.${good[1]}.${good[2].substring(0, good[2].length - 4)}AAAA', + '${_b64({'alg': 'none', 'kid': _kid})}.${good[1]}.', + '${_b64({'alg': 'HS256', 'kid': _kid})}.${good[1]}.${good[2]}', + '${_b64({'alg': 'RS256'})}.${good[1]}.${good[2]}', // no kid + '${_b64({'alg': 'RS256', 'kid': _kid, 'jwk': {'kty': 'RSA'}})}.${good[1]}.${good[2]}', + '$h.${good[1]}', + 'garbage', + ]) { + await expectLater(jwks.verify(token), throwsA(isA()), + reason: token); + } + }); + + test('an unknown kid refetches once, then is rejected', () async { + var now = DateTime(2026, 10, 1); + await withClock(Clock(() => now), () async { + await jwks.verify(_idToken('oaiapp_1', 'n1')); + expect(fetches, 1); + // A kid that is nowhere: rejected, and not a refetch per token. + for (var i = 0; i < 3; i++) { + await expectLater( + jwks.verify(_jwt({'iss': kChatGptIssuer}, header: {'alg': 'RS256', 'kid': 'nope'})), + throwsA(isA())); + } + expect(fetches, 1); + // Rotated: the token's kid is not cached yet, the new key set has it. + now = now.add(const Duration(minutes: 1)); + servedKid = 'rotated'; + await jwks.verify(_jwt({'iss': kChatGptIssuer}, header: {'alg': 'RS256', 'kid': 'rotated'})); + expect(fetches, 2); + }); + }); + + test('keys that cannot be fetched fail closed', () async { + down = true; + await expectLater( + jwks.verify(_idToken('oaiapp_1', 'n1')), throwsA(isA())); + }); + + test('short, non-RSA and encryption keys are not usable', () { + final n = _b64Int(_rsa.publicKey.modulus!), e = _b64Int(_rsa.publicKey.exponent!); + final keys = parseChatGptJwks(jsonEncode({ + 'keys': [ + {'kty': 'RSA', 'kid': 'ok', 'n': n, 'e': e}, + {'kty': 'RSA', 'kid': 'enc', 'use': 'enc', 'n': n, 'e': e}, + {'kty': 'RSA', 'kid': 'ps', 'alg': 'PS256', 'n': n, 'e': e}, + {'kty': 'RSA', 'kid': 'short', 'n': _b64Int(BigInt.two.pow(1023) + BigInt.one), 'e': e}, + {'kty': 'EC', 'kid': 'ec', 'x': 'AA', 'y': 'AA'}, + ], + })); + expect(keys.keys, ['ok']); + }); + }); + group('refresh', () { Future expired(http.Client c) async { SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); From c9fb2c42b693266536b806f6c45ce49d4fcc78b2 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:36:43 +0530 Subject: [PATCH 08/23] coach: a signed-out chatgpt coach no longer schedules briefings reminders counted the coach as set up when either a byok key was stored or chatgpt was configured, so switching to chatgpt and signing out left a leftover api key keeping morning and evening briefings scheduled. on the chatgpt route only the chatgpt sign-in counts now. --- lib/coach/coach_config.dart | 5 +++++ lib/state/app_state.dart | 4 +--- test/coach_chatgpt_test.dart | 10 ++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/coach/coach_config.dart b/lib/coach/coach_config.dart index 48ad2c8b6..a96da09db 100644 --- a/lib/coach/coach_config.dart +++ b/lib/coach/coach_config.dart @@ -289,6 +289,11 @@ class CoachConfig extends ChangeNotifier { ? chatgpt.sharing && _model.isNotEmpty : (hasKey || isLocalEndpoint) && _baseUrl.isNotEmpty && _model.isNotEmpty; + /// Whether AI reminders (briefings) may be scheduled. On the ChatGPT route + /// only the ChatGPT sign-in counts: a BYOK key left over from before the + /// switch is not used there, so a signed-out ChatGPT coach is not set up. + bool get remindersConfigured => _useChatGpt ? configured : hasKey; + /// Normalised base, no trailing slash. String get apiBase { if (_useChatGpt) return kChatGptApiBase; diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index e699cadc0..1716554db 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -2889,9 +2889,7 @@ class AppState extends ChangeNotifier { await NotificationCenter.instance.scheduleAiReminders( prefs, ai, - aiConfigured: (coachConfig?.hasKey ?? false) || - ((coachConfig?.usesChatGpt ?? false) && - (coachConfig?.configured ?? false)), + aiConfigured: coachConfig?.remindersConfigured ?? false, bedtimeMinOfDay: cd.bedtimeMin, journalDoneToday: BriefingStore.journalDoneToday(), sweepHeadline: await _sweepHeadlineNow(), diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index c08c36c66..c5397ad9a 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -550,6 +550,16 @@ void main() { await cfg.chatgpt.debugSetSession(_session(scope: 'openid email profile offline_access')); expect(cfg.configured, isFalse); }); + + test('a leftover API key does not schedule reminders for a signed-out ChatGPT coach', () async { + await cfg.save(apiKey: 'sk-left-over'); + expect(cfg.remindersConfigured, isTrue); + await cfg.chatgpt.signOut(); + expect(cfg.hasKey, isTrue); + expect(cfg.remindersConfigured, isFalse); + await cfg.save(useChatGpt: false); + expect(cfg.remindersConfigured, isTrue, reason: 'the key route still counts its key'); + }); }); group('session lifecycle', () { From d588aefcb12d9b79a4561b63406ed16ec097263e Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:38:03 +0530 Subject: [PATCH 09/23] coach: revoke chatgpt tokens from a sign-in that ends without a session a code exchange that came back after sign-out, a reset or a cancelled sign-in threw its tokens away, which left a live grant on the account with nothing on the phone to revoke it. the same happened when the id token did not check out. those tokens are revoked now, refresh and access both, using the client id the exchange was made with since a reset deletes the saved one. cancelling also stops an exchange that is already out from being stored. --- lib/coach/coach_chatgpt.dart | 88 ++++++++++++++++++++++++++++-------- test/coach_chatgpt_test.dart | 86 +++++++++++++++++++++++++++-------- 2 files changed, 135 insertions(+), 39 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index c68b034e4..33e9426f5 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -295,6 +295,11 @@ class ChatGptAuth extends ChangeNotifier { HttpServer? _pending; Future? _refreshing; + /// Bumped by every [signIn] and every [cancelSignIn]. A code exchange that + /// finishes under an older value was cancelled (or replaced) while it was + /// out, so its tokens are revoked rather than stored. + int _attempt = 0; + /// Bumped whenever a sign-in commits a session, and at the very start of /// [signOut]/[forgetAll]. A [load], [_refresh] or [signIn] that started under /// an older value must not apply its result — a slow keychain read or token call @@ -376,9 +381,17 @@ class ChatGptAuth extends ChangeNotifier { /// lock before any write: if the session was signed out or replaced since, /// the result is dropped. A sign-in or [debugSetSession] passes `commit` to /// start a new generation once its write lands. - Future _store(ChatGptSession? s, {required int generation, bool commit = false}) => + Future _store(ChatGptSession? s, + {required int generation, bool commit = false, int? attempt}) => _serialized(() async { - if (generation != _generation) throw ChatGptException('Signed out.'); + void current() { + if (generation != _generation) throw ChatGptException('Signed out.'); + if (attempt != null && attempt != _attempt) { + throw ChatGptException('Sign-in was cancelled.'); + } + } + + current(); final prefs = await SharedPreferences.getInstance(); if (s == null) { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); @@ -396,7 +409,7 @@ class ChatGptAuth extends ChangeNotifier { } await prefs.setBool(_kPresent, s != null); // A sign-out that began mid-write clears the keychain right after us. - if (generation != _generation) throw ChatGptException('Signed out.'); + current(); if (commit) { _generation++; // A new session: an earlier one's rotated token is not this one's. @@ -417,6 +430,7 @@ class ChatGptAuth extends ChangeNotifier { /// lands on a listener on this device's loopback. Future signIn(Future Function(Uri) open) async { final generation = _generation; + final attempt = ++_attempt; final prefs = await SharedPreferences.getInstance(); final hostId = prefs.getString(_kHostId) ?? 'urn:uuid:${const Uuid().v4()}'; await prefs.setString(_kHostId, hostId); @@ -471,20 +485,37 @@ class ChatGptAuth extends ChangeNotifier { throw ChatGptException( 'ChatGPT sign-in failed (${e.code ?? e.status}). Try again.'); } - final idToken = t['id_token'] as String?; - if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); + // From here on tokens exist on OpenAI's side. Any way this attempt ends + // other than a stored session — a sign-out, reset or cancel that landed + // while the exchange was out, or a token that does not check out — + // revokes them, with the client id captured above (a reset deletes the + // saved one). try { - await _jwks.verify(idToken); - } on ChatGptSignatureException catch (e) { - debugPrint('[chatgpt] $e'); - throw ChatGptException('ChatGPT returned a sign-in that did not check out.'); + if (generation != _generation) throw ChatGptException('Signed out.'); + if (attempt != _attempt) throw ChatGptException('Sign-in was cancelled.'); + final idToken = t['id_token'] as String?; + if (idToken == null) throw ChatGptException('ChatGPT returned no identity.'); + try { + await _jwks.verify(idToken); + } on ChatGptSignatureException catch (e) { + debugPrint('[chatgpt] $e'); + throw ChatGptException('ChatGPT returned a sign-in that did not check out.'); + } + final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); + await _store( + _sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken), + generation: generation, + commit: true, + attempt: attempt, + ); + } catch (_) { + await _revokeIssued( + refreshToken: t['refresh_token'] is String ? t['refresh_token'] as String : null, + accessToken: t['access_token'] as String, // [_token] checked it + clientId: clientId, + ); + rethrow; } - final who = chatGptIdentity(idToken, clientId: clientId, nonce: nonce); - await _store( - _sessionFrom(t, sub: who.sub, email: who.email, idToken: idToken), - generation: generation, - commit: true, - ); } finally { if (identical(_pending, server)) _pending = null; await server.close(force: true); @@ -493,7 +524,10 @@ class ChatGptAuth extends ChangeNotifier { /// Ends a [signIn] still waiting on the browser. Nothing tells the app when /// the user simply closes the in-app browser, so this is the way out. - void cancelSignIn() => _pending?.close(force: true); + void cancelSignIn() { + _attempt++; + _pending?.close(force: true); + } /// The first callback carrying our [state]. Anything else that reaches the /// loopback port is turned away without ending the sign-in. @@ -643,12 +677,13 @@ class ChatGptAuth extends ChangeNotifier { return revoked; } - /// True when OpenAI confirmed the refresh token is revoked. - Future _revoke(String token, String clientId) async { + /// True when OpenAI confirmed [token] is revoked. + Future _revoke(String token, String clientId, + {String hint = 'refresh_token'}) async { try { final r = await _http((c) => c.post(Uri.parse(_revokeUrl), body: { 'token': token, - 'token_type_hint': 'refresh_token', + 'token_type_hint': hint, 'client_id': clientId, }).timeout(const Duration(seconds: 15))); return r.statusCode == 200; @@ -657,6 +692,21 @@ class ChatGptAuth extends ChangeNotifier { } } + /// Revokes tokens this app holds but will not keep: the refresh token, which + /// is what keeps the grant alive, then the access token so it does not stay + /// usable for the rest of its hour. True only when both were confirmed. + Future _revokeIssued( + {required String? refreshToken, + required String? accessToken, + required String clientId}) async { + var ok = true; + if (refreshToken != null) ok = await _revoke(refreshToken, clientId) && ok; + if (accessToken != null) { + ok = await _revoke(accessToken, clientId, hint: 'access_token') && ok; + } + return ok; + } + /// "Reset all data": the session plus the registration and welcome flag. /// The session is revoked first, best effort, while the registration to /// revoke it with is still here. diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index c5397ad9a..375e236fa 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -244,33 +244,79 @@ void main() { expect(second.queryParameters['id_token_hint'], isNotNull); }); - test('sign-out while the code exchange is pending leaves nothing stored', () async { - final gate = Completer(), arrived = Completer(); - late Uri first; - auth = ChatGptAuth(client: MockClient((req) async { - arrived.complete(); - await gate.future; - return http.Response( - jsonEncode({ - 'access_token': 'at-1', - 'refresh_token': 'rt-1', - 'id_token': _idToken(req.bodyFields['client_id']!, first.queryParameters['nonce']!), - 'scope': _fullScope, + /// A token endpoint that holds the code exchange until [gate] opens, and + /// records every revocation. + ChatGptAuth pendingExchange(Completer gate, Completer arrived, + List<(String, String, String)> revoked, Uri Function() authorize, + {bool signed = true}) => + ChatGptAuth(client: MockClient((req) async { + final oidc = _oidc(req); + if (oidc != null) return oidc; + if (req.url.path.endsWith('/revoke')) { + revoked.add((req.bodyFields['token']!, req.bodyFields['token_type_hint']!, + req.bodyFields['client_id']!)); + return http.Response('', 200); + } + arrived.complete(); + await gate.future; + var id = _idToken(req.bodyFields['client_id']!, authorize().queryParameters['nonce']!); + if (!signed) id = '${id.substring(0, id.lastIndexOf('.'))}.AAAA'; + return http.Response( + jsonEncode({ + 'access_token': 'at-1', + 'refresh_token': 'rt-1', + 'id_token': id, + 'scope': _fullScope, + }), + 200); + })); + + const issued = [ + ('rt-1', 'refresh_token', 'oaiapp_1'), + ('at-1', 'access_token', 'oaiapp_1'), + ]; + + for (final (what, end) in <(String, Future Function(ChatGptAuth))>[ + ('sign-out', (a) => a.signOut()), + ('cancel', (a) async => a.cancelSignIn()), + ('reset', (a) => a.forgetAll()), + ]) { + test('a code exchange that lands after a $what revokes what it was issued', () async { + final gate = Completer(), arrived = Completer(); + final revoked = <(String, String, String)>[]; + late Uri first; + auth = pendingExchange(gate, arrived, revoked, () => first); + final signIn = expectLater( + auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); }), - 200); - })); - final signIn = expectLater( + throwsA(isA())); + await arrived.future; + await end(auth); + gate.complete(); + await signIn; + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + // The reset deleted the saved client id; the revocation still has it. + expect(revoked, issued); + }); + } + + test('an ID token that fails its signature check is revoked, not stored', () async { + final gate = Completer()..complete(), arrived = Completer(); + final revoked = <(String, String, String)>[]; + late Uri first; + auth = pendingExchange(gate, arrived, revoked, () => first, signed: false); + await expectLater( auth.signIn((u) { first = u; return browser(u, clientId: 'oaiapp_1'); }), - throwsA(isA())); - await arrived.future; - await auth.signOut(); - gate.complete(); - await signIn; + throwsA(isA().having((e) => e.message, 'message', contains('check out')))); expect(auth.signedIn, isFalse); expect(keychain, isEmpty); + expect(revoked, issued); }); test('a sign-in already writing when sign-out starts is cleared afterwards', () async { From b1dd5e9d6a4d4b957ee24fd672dd3c501d62e06c Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:39:47 +0530 Subject: [PATCH 10/23] coach: revoke a chatgpt refresh that lands after its session ended a refresh still out when the user signed out parked its rotated token for the next sign-out to revoke. if they signed back in before it landed, that token was parked for the new session, so its sign-out revoked the old session's token and left the new one live. the stale refresh now revokes its rotated tokens itself as soon as it lands, and a new session no longer shares a refresh that an older one started. --- lib/coach/coach_chatgpt.dart | 87 ++++++++++++++++++++++-------------- test/coach_chatgpt_test.dart | 55 +++++++++++++++++++++-- 2 files changed, 106 insertions(+), 36 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 33e9426f5..ba25ecd59 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -295,6 +295,11 @@ class ChatGptAuth extends ChangeNotifier { HttpServer? _pending; Future? _refreshing; + /// The [_generation] [_refreshing] was started under. A session that came + /// after it never shares it: that refresh is spending another session's + /// token. + int _refreshingGeneration = -1; + /// Bumped by every [signIn] and every [cancelSignIn]. A code exchange that /// finishes under an older value was cancelled (or replaced) while it was /// out, so its tokens are revoked rather than stored. @@ -320,10 +325,6 @@ class ChatGptAuth extends ChangeNotifier { return done; } - /// The refresh token a refresh rotated to after [signOut] began. It is not - /// stored, but it is the live one, so it is the one to revoke. - String? _rotatedAfterSignOut; - ChatGptSession? get session => _session; bool get signedIn => _session != null; bool get sharing => _session?.sharing ?? false; @@ -410,11 +411,7 @@ class ChatGptAuth extends ChangeNotifier { await prefs.setBool(_kPresent, s != null); // A sign-out that began mid-write clears the keychain right after us. current(); - if (commit) { - _generation++; - // A new session: an earlier one's rotated token is not this one's. - _rotatedAfterSignOut = null; - } + if (commit) _generation++; _session = s; _unreadable = false; notifyListeners(); @@ -589,7 +586,14 @@ class ChatGptAuth extends ChangeNotifier { if (s.expiresAt.isAfter(clock.now().add(const Duration(minutes: 5)))) { return Future.value(s.accessToken); } - return _refreshing ??= _refresh(s).whenComplete(() => _refreshing = null); + final pending = _refreshing; + if (pending != null && _refreshingGeneration == _generation) return pending; + _refreshingGeneration = _generation; + late final Future f; + f = _refresh(s).whenComplete(() { + if (identical(_refreshing, f)) _refreshing = null; + }); + return _refreshing = f; } Future _refresh(ChatGptSession s) async { @@ -621,18 +625,19 @@ class ChatGptAuth extends ChangeNotifier { idToken: (t['id_token'] as String?) ?? s.idToken, ); if (generation != _generation) { - // Signed out while the call was out: do not bring the session back, but - // the rotated token is the live one and [signOut] has to revoke it. - _rotatedAfterSignOut = next.refreshToken; - throw ChatGptException('Signed out.'); + // Signed out (or replaced) while the call was out: the rotated tokens + // belong to a session that has ended, so they are revoked here rather + // than handed on — a later sign-out is about a later session. + throw _StaleRefresh(await _revokeIssued( + refreshToken: next.refreshToken, accessToken: next.accessToken, clientId: clientId)); } try { await _store(next, generation: generation); } catch (_) { if (generation != _generation) { - // Signed out while it was writing: the rotated token is the live one. - _rotatedAfterSignOut = next.refreshToken; - rethrow; + // Signed out while it was writing: same as above. + throw _StaleRefresh(await _revokeIssued( + refreshToken: next.refreshToken, accessToken: next.accessToken, clientId: clientId)); } // The keychain write failed, but the old refresh token is already spent: // keep the rotated session in memory so the next refresh does not reuse it. @@ -648,19 +653,19 @@ class ChatGptAuth extends ChangeNotifier { final generation = ++_generation; // before any await: see [_generation] cancelSignIn(); final s = _session; + final refreshing = _refreshing; // Memory first, so nothing keeps using the session while revocation is out. _session = null; _unreadable = false; notifyListeners(); - // An in-flight refresh may have rotated the token; revoke the latest one. - try { - await _refreshing; - } catch (_) {} - final token = _rotatedAfterSignOut ?? s?.refreshToken; - _rotatedAfterSignOut = null; + // An in-flight refresh revokes what it rotated to itself; the token it + // spent is then dead and there is nothing left to revoke here. + final rotated = await _rotationOf(refreshing); final prefs = await SharedPreferences.getInstance(); final clientId = prefs.getString(_kClientId); - final revoked = token == null || (clientId != null && await _revoke(token, clientId)); + final token = s?.refreshToken; + final revoked = rotated ?? + (token == null || (clientId != null && await _revoke(token, clientId))); // Signed out either way. If the delete fails the item may linger, but the // marker is cleared and [load] fails closed on a record without it, so it // is never restored (it is deleted then); the credential is not guaranteed @@ -677,6 +682,19 @@ class ChatGptAuth extends ChangeNotifier { return revoked; } + /// Waits out a refresh that was in flight when the session ended. Non-null + /// when it had rotated the tokens (and so revoked them itself): whether that + /// revocation was confirmed. + static Future _rotationOf(Future? refreshing) async { + if (refreshing == null) return null; + try { + await refreshing; + } on _StaleRefresh catch (e) { + return e.revoked; + } catch (_) {} + return null; + } + /// True when OpenAI confirmed [token] is revoked. Future _revoke(String token, String clientId, {String hint = 'refresh_token'}) async { @@ -714,18 +732,14 @@ class ChatGptAuth extends ChangeNotifier { _generation++; cancelSignIn(); final s = _session; + final refreshing = _refreshing; _session = null; _unreadable = false; - // An in-flight refresh may have rotated the token; revoke the latest one. - // It is cleared here too: left behind, it would be revoked in place of a - // later session's token. - try { - await _refreshing; - } catch (_) {} - final token = _rotatedAfterSignOut ?? s?.refreshToken; - _rotatedAfterSignOut = null; + // As in [signOut]: an in-flight refresh revokes its own rotated tokens. + final rotated = await _rotationOf(refreshing); + final token = s?.refreshToken; final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); - if (token != null && clientId != null) await _revoke(token, clientId); + if (rotated == null && token != null && clientId != null) await _revoke(token, clientId); await _serialized(() async { try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); @@ -771,6 +785,13 @@ class ChatGptAuth extends ChangeNotifier { _store(s, generation: _generation, commit: true); } +/// A refresh that came back after its session ended. Its rotated tokens were +/// revoked on the spot; [revoked] says whether OpenAI confirmed it. +class _StaleRefresh extends ChatGptException { + final bool revoked; + _StaleRefresh(this.revoked) : super('Signed out.'); +} + class _TokenError implements Exception { final int status; final String? code; diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 375e236fa..a70b8531d 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -633,7 +633,7 @@ void main() { await refreshed; expect(auth.signedIn, isFalse); expect(keychain, isEmpty); - expect(revoked, ['rt-2'], reason: 'the rotated token is the live one'); + expect(revoked, ['rt-2', 'at-2'], reason: 'the rotated tokens are the live ones'); }); test('a reset during a refresh does not make a later sign-out revoke the stale token', () async { @@ -667,8 +667,57 @@ void main() { scope: _fullScope, expiresAt: DateTime.now().add(const Duration(hours: 1)))); await auth.signOut(); - // The reset revoked the rotated token; the sign-out revoked only its own. - expect(revoked, ['rt-2', 'rt-new']); + // The reset revoked the rotated tokens; the sign-out revoked only its own. + expect(revoked, ['rt-2', 'at-2', 'rt-new']); + }); + + test('a refresh that lands after a new sign-in is revoked, not left for its sign-out', () async { + SharedPreferences.setMockInitialValues({'coach_chatgpt_client_id': 'oaiapp_1'}); + final oldGate = Completer(); + final revoked = []; + final refreshed = []; + final auth = ChatGptAuth(client: MockClient((req) async { + if (req.url.path.endsWith('/revoke')) { + revoked.add(req.bodyFields['token']!); + return http.Response('', 200); + } + final rt = req.bodyFields['refresh_token']!; + refreshed.add(rt); + if (rt == 'rt-1') await oldGate.future; // the old session's refresh hangs + return http.Response( + jsonEncode({ + 'access_token': rt == 'rt-1' ? 'at-2' : 'at-new-2', + 'refresh_token': rt == 'rt-1' ? 'rt-2' : 'rt-new-2', + 'expires_in': 3600, + }), + 200); + })); + await auth.debugSetSession(_session(expiresAt: DateTime.now())); + final stale = expectLater(auth.accessToken(), throwsA(isA())); + await pumpEventQueue(); + // Signed out with the refresh still out (revocation unconfirmed until it + // lands), then signed straight back in. + final out = auth.signOut(); + await pumpEventQueue(); + await auth.debugSetSession(ChatGptSession( + sub: 'user-1', + email: 'me@example.com', + idToken: 'i', + accessToken: 'at-new', + refreshToken: 'rt-new', + scope: _fullScope, + expiresAt: DateTime.now())); + // The new session refreshes on its own token, not through the old call. + expect(await auth.accessToken(), 'at-new-2'); + oldGate.complete(); + await stale; + expect(await out, isTrue); + expect(revoked, ['rt-2', 'at-2'], reason: 'the old rotation is revoked as it lands'); + expect(auth.session!.refreshToken, 'rt-new-2'); + revoked.clear(); + await auth.signOut(); + expect(revoked, ['rt-new-2'], reason: "the sign-out revokes the current session's token"); + expect(refreshed, ['rt-1', 'rt-new']); }); test('a sign-in during a slow sign-out survives it', () async { From 401aa886fbda4083806c608d603ee34f8d5a4606 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:40:09 +0530 Subject: [PATCH 11/23] coach: say chatgpt data controls may apply, and name plus and pro only the guide said plan usage sends data to openai exactly as an api key does, which isn't something i can promise: these requests run under the user's chatgpt account, so its data controls and training settings may apply. the guide, the privacy policy and the in-app note now say that and point at chatgpt settings, and the eligible plans are plus and pro. --- PRIVACY.md | 8 +++++--- guides/AI_COACH.md | 10 ++++++---- lib/ui2/screens/coach.dart | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/PRIVACY.md b/PRIVACY.md index 7f98edbf9..10c5a7294 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -161,9 +161,11 @@ configure: - **AI Coach** — if you enable this feature and supply your own API key, summaries of your data are sent to the AI provider you configure (by default, OpenAI) to generate coaching responses. Off by default and - requires your own API key, or signing in with ChatGPT, in which case the - same data goes to OpenAI under your ChatGPT account. The sign-in tokens are - kept in the device keychain/keystore and sent only to OpenAI. + requires your own API key, or signing in with ChatGPT (Plus or Pro), in + which case the same data goes to OpenAI under your ChatGPT account and your + ChatGPT data controls, including model-training settings, may apply to it. + Check them in ChatGPT settings. The sign-in tokens are kept in the device + keychain/keystore and sent only to OpenAI. - **Health app integration** — if you enable it, the App can write derived daily metrics to Apple Health or Google Health Connect, which are controlled by your device's own OS-level health app, not by us. The App can also *read* diff --git a/guides/AI_COACH.md b/guides/AI_COACH.md index c6238c5fd..21bf945ce 100644 --- a/guides/AI_COACH.md +++ b/guides/AI_COACH.md @@ -16,9 +16,9 @@ model you host yourself. ## Use your ChatGPT plan -If you pay for ChatGPT (Plus, Pro and similar plans), you can skip the API -key: open **AI Coach → settings**, tap **ChatGPT**, then **Continue with -ChatGPT** and sign in. Eligible coach, briefing and journal requests then use +If you pay for ChatGPT Plus or Pro, you can skip the API key: open +**AI Coach → settings**, tap **ChatGPT**, then **Continue with ChatGPT** and +sign in. Eligible coach, briefing and journal requests then use the usage included in your ChatGPT plan instead of API billing. Tap **List models** and pick one, then **Save**. @@ -32,7 +32,9 @@ models** and pick one, then **Save**. - When the limit is reached the coach says so and stops; it never quietly switches to an API key. - Your questions and the rows the coach reads go to OpenAI under your ChatGPT - account, exactly as with an OpenAI API key. + account. Your ChatGPT data controls, including whether your content may be + used to train models, may apply to these requests, and they are not the same + terms as an OpenAI API key. Check them in ChatGPT settings before using this. - **Sign out of ChatGPT** asks OpenAI to revoke the sign-in and clears it from the phone. If the app says revocation wasn't confirmed, also disconnect OpenStrap in ChatGPT settings. The coach then stays off until you sign in diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index d04fec347..5ce693150 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -1246,7 +1246,7 @@ class _CoachSetupState extends State { const SizedBox(height: S.x3), Text( uiText(c, - 'Your questions and the rows the coach reads are sent to OpenAI under your ChatGPT account. See exactly what that is on "What was sent".'), + 'Your questions and the rows the coach reads are sent to OpenAI under your ChatGPT account, where your ChatGPT data controls, including model training, may apply. Check them in ChatGPT settings. See exactly what is sent on "What was sent".'), style: F.cap.copyWith(color: p.ink3, height: 1.5), ), ]; From 81f0aec89030300de95b2279ab72fbc6cdaa5dec Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 28 Oct 2026 21:40:53 +0530 Subject: [PATCH 12/23] coach: send function tools in a namespace on the chatgpt plan route openai documents the plan route as accepting function tools only when they're grouped in a namespace or supplied through additional_tools, and the coach sent them as bare top-level tools. on that route they now go under one namespace, and function calls replayed from history carry it. the api-key route is unchanged. --- lib/coach/coach_chatgpt.dart | 64 ++++++++++++++++++++++++----- test/coach_chatgpt_test.dart | 80 ++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 11 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index ba25ecd59..cb7b3d077 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -212,21 +212,63 @@ String chatGptHttpError(int status, String body) { status, null, body.length > 200 ? body.substring(0, 200) : body); } +/// The namespace the coach's function tools are grouped under on the +/// ChatGPT-plan route. +const kChatGptToolNamespace = 'openstrap'; + /// The Responses body this route accepts, from the coach's normal one: it /// must stream, and rejects `system` input items (developer is the same thing /// here). Sampling params are already never sent on Responses. -Map chatGptRequest(Map body) => { - ...body, - 'stream': true, - 'store': false, - 'input': [ - for (final item in body['input'] as List) - if (item is Map && item['role'] == 'system') - {...item, 'role': 'developer'} - else - item, +/// +/// Function tools are only accepted here grouped in a namespace (or through +/// `additional_tools` input items), not as bare top-level tools, so they go +/// under [kChatGptToolNamespace]. The model's calls then carry that +/// namespace beside the bare function name, and a `function_call` replayed +/// from history without one gets it too, so it matches a declared tool. +/// https://developers.openai.com/siwc/token-sharing-open-source/preview-limitations +Map chatGptRequest(Map body) { + final tools = body['tools']; + final functions = >[]; + final others = []; + if (tools is List) { + for (final t in tools) { + if (t is Map && t['type'] == 'function') { + functions.add(t.cast()); + } else { + others.add(t); + } + } + } + final names = {for (final f in functions) f['name']}; + return { + ...body, + 'stream': true, + 'store': false, + 'input': [ + for (final item in body['input'] as List) + if (item is Map && item['role'] == 'system') + {...item, 'role': 'developer'} + else if (item is Map && + item['type'] == 'function_call' && + item['namespace'] == null && + names.contains(item['name'])) + {...item, 'namespace': kChatGptToolNamespace} + else + item, + ], + if (tools is List) + 'tools': [ + if (functions.isNotEmpty) + { + 'type': 'namespace', + 'name': kChatGptToolNamespace, + 'description': 'Read and present this person\'s OpenStrap health data.', + 'tools': functions, + }, + ...others, ], - }; + }; +} class ChatGptSession { final String sub, email, idToken, accessToken, refreshToken, scope; diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index a70b8531d..2d9a62ca9 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -592,6 +592,86 @@ void main() { } }); + test('function tools go in a namespace on this route, and history calls carry it', () async { + final tool = { + 'type': 'function', + 'function': { + 'name': 'run_sql', + 'description': 'query', + 'parameters': {'type': 'object', 'properties': {}}, + }, + }; + late Map sent; + final reply = await CoachEngine.postChat(cfg, { + 'model': 'gpt-6.1-sol', + 'messages': [ + {'role': 'user', 'content': 'hi'}, + { + 'role': 'assistant', + 'content': '', + 'tool_calls': [ + { + 'id': 'c0', + 'type': 'function', + 'function': {'name': 'run_sql', 'arguments': '{}'}, + }, + ], + }, + {'role': 'tool', 'tool_call_id': 'c0', 'content': '[]'}, + ], + 'tools': [tool], + 'tool_choice': 'auto', + }, client: MockClient((req) async { + sent = jsonDecode(req.body) as Map; + return http.Response( + _sse([ + { + 'type': 'response.completed', + 'response': { + 'status': 'completed', + 'output': [ + { + 'type': 'function_call', + 'name': 'run_sql', + 'namespace': kChatGptToolNamespace, + 'call_id': 'c1', + 'arguments': '{"sql":"SELECT 1"}', + 'status': 'completed', + }, + ], + }, + }, + ]), + 200); + })); + final tools = sent['tools'] as List; + expect(tools, hasLength(1)); + expect(tools.single['type'], 'namespace'); + expect(tools.single['name'], kChatGptToolNamespace); + expect((tools.single['tools'] as List).single, + allOf(containsPair('type', 'function'), containsPair('name', 'run_sql'))); + expect(sent['tool_choice'], 'auto'); + final call = (sent['input'] as List).firstWhere((i) => i['type'] == 'function_call'); + expect(call['namespace'], kChatGptToolNamespace); + expect(call['name'], 'run_sql'); + final out = (reply['tool_calls'] as List).single; + expect(out['function'], {'name': 'run_sql', 'arguments': '{"sql":"SELECT 1"}'}); + + // The API-key route still sends bare top-level function tools. + await cfg.save(useChatGpt: false, apiKey: 'sk-test'); + await CoachEngine.postChat(cfg, { + 'model': 'gpt-6.1-sol', + 'messages': [ + {'role': 'user', 'content': 'hi'}, + ], + 'tools': [tool], + }, client: MockClient((req) async { + sent = jsonDecode(req.body) as Map; + return http.Response(jsonEncode(_completed('ok')['response']), 200); + })); + expect((sent['tools'] as List).single['type'], 'function'); + }); + test('signed in without plan usage is not configured', () async { await cfg.chatgpt.debugSetSession(_session(scope: 'openid email profile offline_access')); expect(cfg.configured, isFalse); From 3d17f0026eb4e0fa804c18ccec68659578dc3a63 Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Wed, 28 Oct 2026 18:56:43 -0500 Subject: [PATCH 13/23] coach: the chatgpt card shows cancel while its first sign-in is out tapping the card before chatgpt was selected started the sign-in with the card's fields still hidden, so the cancel button wasn't there and closing the browser left the card disabled until the sign-in timed out. the card selects chatgpt first now, and the guide drops the separate continue step. --- guides/AI_COACH.md | 3 +-- lib/ui2/screens/coach.dart | 19 +++++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/guides/AI_COACH.md b/guides/AI_COACH.md index 21bf945ce..a0e0e71f3 100644 --- a/guides/AI_COACH.md +++ b/guides/AI_COACH.md @@ -17,8 +17,7 @@ model you host yourself. ## Use your ChatGPT plan If you pay for ChatGPT Plus or Pro, you can skip the API key: open -**AI Coach → settings**, tap **ChatGPT**, then **Continue with ChatGPT** and -sign in. Eligible coach, briefing and journal requests then use +**AI Coach → settings**, tap **ChatGPT** and sign in. Eligible coach, briefing and journal requests then use the usage included in your ChatGPT plan instead of API billing. Tap **List models** and pick one, then **Save**. diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index 5ce693150..82de9c084 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -1174,16 +1174,15 @@ class _CoachSetupState extends State { onTap: _signingIn ? null : () { - if (chatgpt.sharing) { - setState(() { - _msg = null; - _chatgpt = true; - _models = const []; - _model = ''; - }); - } else { - _signInChatGpt(); - } + // Selected before a sign-in starts too, so its fields (and the + // "Cancel sign-in" button) show while the browser is up. + setState(() { + _msg = null; + _chatgpt = true; + _models = const []; + _model = ''; + }); + if (!chatgpt.sharing) _signInChatGpt(); }, child: Row( children: [ From 6a663d4f997cf9dd70dade15a8e1fe85bf72017b Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 29 Oct 2026 19:43:11 -0500 Subject: [PATCH 14/23] coach: a chatgpt sign-in can't land during a reset; terminal cleanup is best effort a reset doesn't block the ui and clears every preference after forgetAll, so a sign-in started or finished while it ran could leave a session in the keychain and in memory with nothing vouching for it. sign-in now refuses to start while ResetGate is up, and a session that would commit during a reset is revoked instead of stored. a dead refresh token that hit a failing keychain delete kept the spent session in memory; the delete is best effort now, as in signOut. --- lib/coach/coach_chatgpt.dart | 14 +++++++++++- test/coach_chatgpt_test.dart | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index cb7b3d077..09c812906 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -19,6 +19,7 @@ import 'package:http/http.dart' as http; import 'package:shared_preferences/shared_preferences.dart'; import 'package:uuid/uuid.dart'; +import '../sync/reset_gate.dart'; import 'coach_chatgpt_jwks.dart'; const kChatGptIssuer = 'https://auth.openai.com'; @@ -432,12 +433,18 @@ class ChatGptAuth extends ChangeNotifier { if (attempt != null && attempt != _attempt) { throw ChatGptException('Sign-in was cancelled.'); } + // A sign-in that lands during a reset is revoked, not stored (see [signIn]). + if (commit && ResetGate.active) throw ChatGptException('Signed out.'); } current(); final prefs = await SharedPreferences.getInstance(); if (s == null) { - await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + // Best effort: the marker below is what [load] trusts, and the spent + // session must leave memory even if the keychain item lingers. + try { + await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); + } catch (_) {} } else { final raw = jsonEncode(s.toJson()); try { @@ -468,6 +475,11 @@ class ChatGptAuth extends ChangeNotifier { /// this app alive while it is up (an in-app browser), because the redirect /// lands on a listener on this device's loopback. Future signIn(Future Function(Uri) open) async { + // A reset does not block the UI, and it clears every preference after + // [forgetAll]: a sign-in it overlapped would be left half-stored. + if (ResetGate.active) { + throw ChatGptException('Wait for the reset to finish, then sign in again.'); + } final generation = _generation; final attempt = ++_attempt; final prefs = await SharedPreferences.getInstance(); diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 2d9a62ca9..908f7534c 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -15,6 +15,7 @@ import 'package:openstrap_edge/coach/coach_chatgpt.dart'; import 'package:openstrap_edge/coach/coach_chatgpt_jwks.dart'; import 'package:openstrap_edge/coach/coach_config.dart'; import 'package:openstrap_edge/coach/coach_engine.dart'; +import 'package:openstrap_edge/sync/reset_gate.dart'; import 'package:pointycastle/export.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -303,6 +304,36 @@ void main() { }); } + test('a sign-in does not start while a reset is running', () async { + ResetGate.enter(); + addTearDown(ResetGate.resetForTest); + auth = ChatGptAuth(client: MockClient((_) async => fail('no network'))); + await expectLater( + auth.signIn((_) async => fail('no browser')), + throwsA(isA().having((e) => e.message, 'message', contains('reset')))); + }); + + test('a code exchange that lands while a reset is running is revoked, not stored', () async { + addTearDown(ResetGate.resetForTest); + final gate = Completer(), arrived = Completer(); + final revoked = <(String, String, String)>[]; + late Uri first; + auth = pendingExchange(gate, arrived, revoked, () => first); + final signIn = expectLater( + auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); + }), + throwsA(isA())); + await arrived.future; + ResetGate.enter(); // the reset has started but not reached forgetAll yet + gate.complete(); + await signIn; + expect(auth.signedIn, isFalse); + expect(keychain, isEmpty); + expect(revoked, issued); + }); + test('an ID token that fails its signature check is revoked, not stored', () async { final gate = Completer()..complete(), arrived = Completer(); final revoked = <(String, String, String)>[]; @@ -494,6 +525,19 @@ void main() { await expectLater(dead.accessToken(), throwsA(isA())); expect(dead.signedIn, isFalse); }); + + test('a dead refresh token signs out even if the keychain delete fails', () async { + final dead = await expired( + MockClient((_) async => http.Response(jsonEncode({'error': 'invalid_grant'}), 400))); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + if (call.method == 'delete') throw PlatformException(code: 'boom', message: 'locked'); + fail('${call.method} must not run'); + }); + await expectLater(dead.accessToken(), throwsA(isA())); + expect(dead.signedIn, isFalse); + expect((await SharedPreferences.getInstance()).getBool('coach_chatgpt_present'), isFalse); + }); }); group('coach requests on the ChatGPT plan', () { From 2d2bddd4688def18494909824ff8cffa81ad27f5 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 31 Oct 2026 21:45:05 +0530 Subject: [PATCH 15/23] coach: drop a model list from the other provider, cancel sign-in on a preset a model list asked for under chatgpt could fill the list after the user picked a preset with the same base url, and the reverse. picking a preset while a chatgpt sign-in was out left the sign-in running, and when it landed it switched the coach back to chatgpt. --- lib/ui2/screens/coach.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index 936a48807..dd0840853 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -1052,11 +1052,14 @@ class _CoachSetupState extends State { // A reply for an endpoint the user has since edited away from would list // models the new one may not serve. final asked = _base.text; + // Switching between ChatGPT and a preset need not change the base URL. + final askedChatgpt = _chatgpt; + bool stale() => _base.text != asked || _chatgpt != askedChatgpt; try { - final ids = _chatgpt + final ids = askedChatgpt ? await chatgpt.listModels() : await CoachEngine.fetchModels(asked, _key.text); - if (!mounted || _base.text != asked) return; + if (!mounted || stale()) return; final l = AppLocalizations.of(context); setState(() { _models = ids; @@ -1066,7 +1069,7 @@ class _CoachSetupState extends State { : (l?.coachModelsFound(ids.length) ?? '${ids.length} models. Tap one.'); }); } catch (e) { - if (!mounted || _base.text != asked) return; + if (!mounted || stale()) return; final l = AppLocalizations.of(context); setState( () => _msg = e is CoachException || e is ChatGptException @@ -1233,6 +1236,9 @@ class _CoachSetupState extends State { // clearing _msg after it runs would silently // discard that explanation. _msg = null; + // A sign-in still out would otherwise land later and switch the + // coach back to ChatGPT over this choice. + if (_signingIn) _chatgptAuth.cancelSignIn(); _chatgpt = false; _base.text = preset.baseUrl; _api = CoachApi.defaultFor(preset.baseUrl); From 668eb69b8ae035d1a193a5fb77848439d6cb35e3 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 31 Oct 2026 21:45:05 +0530 Subject: [PATCH 16/23] coach: keep the sign-in-ended message when its cleanup fails --- lib/coach/coach_chatgpt.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 09c812906..ec601305a 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -666,7 +666,11 @@ class ChatGptAuth extends ChangeNotifier { if (!e.terminal) { throw ChatGptException('Could not renew the ChatGPT sign-in (${e.status}). Try again.'); } - await _store(null, generation: generation); + // Best effort: a failed clear (or a sign-out that got there first) must + // not replace the message saying why the coach stopped. + try { + await _store(null, generation: generation); + } catch (_) {} throw ChatGptException('Your ChatGPT sign-in has ended. Sign in again from AI settings.'); } catch (e) { // A network hiccup is not a revoked session — the tokens stay. From bd3f0305cba90aac45a366cc8f43fecf66a783cb Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 11:54:30 +0530 Subject: [PATCH 17/23] coach: point at openai's terms and usage policies before chatgpt sign-in signing in puts coach requests under the user's own openai account, so the sign-in now opens a sheet linking both documents and only starts after an explicit continue. cancel or dismiss does nothing. --- lib/coach/coach_chatgpt.dart | 2 + lib/ui2/screens/coach.dart | 67 +++++++++++++++++++++++++++++ test/chatgpt_terms_notice_test.dart | 57 ++++++++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 test/chatgpt_terms_notice_test.dart diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index ec601305a..99351df52 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -27,6 +27,8 @@ const kChatGptIssuer = 'https://auth.openai.com'; /// Both the OAuth `resource` and the Responses API base requests go to. const kChatGptApiBase = 'https://api.openai.com/v1'; const kChatGptUsageUrl = 'https://chatgpt.com/settings/usage'; +const kOpenAiTermsUrl = 'https://openai.com/policies/terms-of-use/'; +const kOpenAiUsagePoliciesUrl = 'https://openai.com/policies/usage-policies/'; const kChatGptPlanScope = 'chatgpt.tokens.use.direct'; // ponytail: endpoints copied from $kChatGptIssuer/.well-known/openid-configuration diff --git a/lib/ui2/screens/coach.dart b/lib/ui2/screens/coach.dart index dd0840853..faefdb557 100644 --- a/lib/ui2/screens/coach.dart +++ b/lib/ui2/screens/coach.dart @@ -1139,6 +1139,7 @@ class _CoachSetupState extends State { /// the system browser in front, iOS suspends the app and the listener dies. Future _signInChatGpt() async { final chatgpt = _chatgptAuth; + if (!await confirmChatGptTerms(context) || !mounted) return; setState(() { _signingIn = true; _msg = null; @@ -1598,3 +1599,69 @@ class _CoachSetupState extends State { ); } } + +/// Shown before every ChatGPT sign-in. Signing in puts the coach's requests +/// under the user's own OpenAI account, so OpenAI's terms and usage policies +/// apply to them; the sign-in only starts after an explicit Continue. +/// Returns false when the sheet is cancelled or dismissed. +Future confirmChatGptTerms(BuildContext context) async { + final ok = await showModalBottomSheet( + context: context, + isScrollControlled: true, + sheetAnimationStyle: sheetMotion(context), + backgroundColor: P.of(context).card, + shape: const RoundedRectangleBorder( + borderRadius: BorderRadius.vertical(top: Radius.circular(R.xxl)), + ), + builder: (s) => SafeArea( + child: SingleChildScrollView( + padding: const EdgeInsets.all(S.x5), + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + Text(uiText(s, 'Before you sign in'), + style: F.head.copyWith(color: P.of(s).ink)), + const SizedBox(height: S.x2), + Text( + uiText(s, + 'Signing in uses your own ChatGPT account and plan. Your use of it here is covered by OpenAI’s Terms of Use and Usage Policies, so please read them before you continue.'), + style: F.cap.copyWith(color: P.of(s).ink2, height: 1.5), + ), + const SizedBox(height: S.x4), + BigButton( + uiText(s, 'OpenAI Terms of Use'), + icon: LucideIcons.externalLink, + color: kCoachAccent, + soft: true, + onTap: () => launchUrl(Uri.parse(kOpenAiTermsUrl), + mode: LaunchMode.externalApplication), + ), + const SizedBox(height: S.x3), + BigButton( + uiText(s, 'OpenAI Usage Policies'), + icon: LucideIcons.externalLink, + color: kCoachAccent, + soft: true, + onTap: () => launchUrl(Uri.parse(kOpenAiUsagePoliciesUrl), + mode: LaunchMode.externalApplication), + ), + const SizedBox(height: S.x5), + BigButton( + uiText(s, 'I’ve read them, continue'), + color: kCoachAccent, + onTap: () => Navigator.of(s).pop(true), + ), + const SizedBox(height: S.x3), + BigButton( + uiText(s, 'Cancel'), + soft: true, + onTap: () => Navigator.of(s).pop(false), + ), + ], + ), + ), + ), + ); + return ok ?? false; +} diff --git a/test/chatgpt_terms_notice_test.dart b/test/chatgpt_terms_notice_test.dart new file mode 100644 index 000000000..623bb4e65 --- /dev/null +++ b/test/chatgpt_terms_notice_test.dart @@ -0,0 +1,57 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/ui2/screens/coach.dart' show confirmChatGptTerms; + +// ChatGPT sign-in only starts after the user has been pointed at OpenAI's +// terms and usage policies and chosen to continue. +void main() { + Future> open(WidgetTester tester) async { + tester.view.physicalSize = const Size(390 * 3, 900 * 3); + tester.view.devicePixelRatio = 3; + addTearDown(tester.view.reset); + final results = []; + await tester.pumpWidget(MaterialApp( + home: Scaffold( + body: Builder( + builder: (c) => TextButton( + onPressed: () async => results.add(await confirmChatGptTerms(c)), + child: const Text('open'), + ), + ), + ), + )); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + return results; + } + + testWidgets('the notice names both OpenAI documents before sign-in', + (tester) async { + await open(tester); + expect(find.text('Before you sign in'), findsOneWidget); + expect(find.textContaining('your own ChatGPT account'), findsOneWidget); + expect(find.text('OpenAI Terms of Use'), findsOneWidget); + expect(find.text('OpenAI Usage Policies'), findsOneWidget); + }); + + testWidgets('cancel does not start sign-in', (tester) async { + final results = await open(tester); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + expect(results, [false]); + }); + + testWidgets('dismissing the sheet does not start sign-in', (tester) async { + final results = await open(tester); + await tester.tapAt(const Offset(10, 10)); + await tester.pumpAndSettle(); + expect(results, [false]); + }); + + testWidgets('continue starts sign-in', (tester) async { + final results = await open(tester); + await tester.tap(find.text('I’ve read them, continue')); + await tester.pumpAndSettle(); + expect(results, [true]); + }); +} From 885f527c228939340553cada2edaccc8f8b3dad1 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 12:06:08 +0530 Subject: [PATCH 18/23] coach: revoke the access token too on chatgpt sign-out and reset revoking only the refresh token left the issued access token usable for the rest of its hour. --- lib/coach/coach_chatgpt.dart | 11 ++++++++--- test/coach_chatgpt_test.dart | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 99351df52..aed9db4c4 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -706,7 +706,7 @@ class ChatGptAuth extends ChangeNotifier { return next.accessToken; } - /// Revokes the refresh token and forgets the session. Returns false when + /// Revokes the refresh and access tokens and forgets the session. Returns false when /// revocation could not be confirmed; the tokens are cleared regardless, and /// the user can still disconnect the app in ChatGPT settings. Future signOut() async { @@ -725,7 +725,10 @@ class ChatGptAuth extends ChangeNotifier { final clientId = prefs.getString(_kClientId); final token = s?.refreshToken; final revoked = rotated ?? - (token == null || (clientId != null && await _revoke(token, clientId))); + (s == null || + (clientId != null && + await _revokeIssued( + refreshToken: token, accessToken: s.accessToken, clientId: clientId))); // Signed out either way. If the delete fails the item may linger, but the // marker is cleared and [load] fails closed on a record without it, so it // is never restored (it is deleted then); the credential is not guaranteed @@ -799,7 +802,9 @@ class ChatGptAuth extends ChangeNotifier { final rotated = await _rotationOf(refreshing); final token = s?.refreshToken; final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); - if (rotated == null && token != null && clientId != null) await _revoke(token, clientId); + if (rotated == null && s != null && clientId != null) { + await _revokeIssued(refreshToken: token, accessToken: s.accessToken, clientId: clientId); + } await _serialized(() async { try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 908f7534c..e080362cb 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -792,7 +792,7 @@ void main() { expiresAt: DateTime.now().add(const Duration(hours: 1)))); await auth.signOut(); // The reset revoked the rotated tokens; the sign-out revoked only its own. - expect(revoked, ['rt-2', 'at-2', 'rt-new']); + expect(revoked, ['rt-2', 'at-2', 'rt-new', 'at-new']); }); test('a refresh that lands after a new sign-in is revoked, not left for its sign-out', () async { @@ -840,7 +840,7 @@ void main() { expect(auth.session!.refreshToken, 'rt-new-2'); revoked.clear(); await auth.signOut(); - expect(revoked, ['rt-new-2'], reason: "the sign-out revokes the current session's token"); + expect(revoked, ['rt-new-2', 'at-new-2'], reason: "the sign-out revokes the current session's tokens"); expect(refreshed, ['rt-1', 'rt-new']); }); @@ -914,7 +914,7 @@ void main() { expect(await auth.takeWelcome(), isFalse); await auth.forgetAll(); final prefs = await SharedPreferences.getInstance(); - expect(revoked, ['rt-1']); + expect(revoked, ['rt-1', 'at-1']); expect(auth.signedIn, isFalse); expect(keychain, isEmpty); expect(prefs.getKeys().where((k) => k.startsWith('coach_chatgpt_')), isEmpty); From 753f7df0b2b9445b97f457708962f57505a6c99e Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 12:30:10 +0530 Subject: [PATCH 19/23] coach: revoke the refresh and access tokens in parallel with both requests timing out, sign-out and reset waited 30 s before clearing anything; now it is one timeout. --- lib/coach/coach_chatgpt.dart | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index aed9db4c4..e95a0092d 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -774,18 +774,19 @@ class ChatGptAuth extends ChangeNotifier { } /// Revokes tokens this app holds but will not keep: the refresh token, which - /// is what keeps the grant alive, then the access token so it does not stay + /// is what keeps the grant alive, and the access token so it does not stay /// usable for the rest of its hour. True only when both were confirmed. Future _revokeIssued( {required String? refreshToken, required String? accessToken, required String clientId}) async { - var ok = true; - if (refreshToken != null) ok = await _revoke(refreshToken, clientId) && ok; - if (accessToken != null) { - ok = await _revoke(accessToken, clientId, hint: 'access_token') && ok; - } - return ok; + // Both at once, so a dead network costs one request timeout, not two: + // sign-out and "Reset all data" wait on this before clearing local state. + final results = await Future.wait([ + if (refreshToken != null) _revoke(refreshToken, clientId), + if (accessToken != null) _revoke(accessToken, clientId, hint: 'access_token'), + ]); + return results.every((ok) => ok); } /// "Reset all data": the session plus the registration and welcome flag. From e27509f1213a5784d38d821ff62800bd85032739 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 12:52:07 +0530 Subject: [PATCH 20/23] coach: reset clears chatgpt mode and model; guide names both setup paths reset deleted the api key and the chatgpt session but left the coach in chatgpt mode with the old model, which came back on the next sign-in. the guide's opening only described the api-key route. --- guides/AI_COACH.md | 13 ++++++++++--- lib/state/app_state.dart | 6 +++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/guides/AI_COACH.md b/guides/AI_COACH.md index a0e0e71f3..a88d59b9c 100644 --- a/guides/AI_COACH.md +++ b/guides/AI_COACH.md @@ -1,9 +1,11 @@ -# AI Coach — bring your own key +# AI Coach — your own model, or your ChatGPT plan The AI features in OpenStrap Edge (the Coach chat, the morning and evening briefings, and the pre-sleep journal) are all powered by a model **you** -provide. There's no OpenStrap AI service: you paste in an API key for any -OpenAI-compatible provider and the app talks to that provider directly. +provide. There's no OpenStrap AI service. There are two ways to set it up: +paste in an API key for any OpenAI-compatible provider, or +[sign in with your ChatGPT plan](#use-your-chatgpt-plan). Either way the app +talks to that provider directly. Your key is stored in the device keychain/keystore and is only ever sent to the base URL you configure — never to OpenStrap. @@ -21,6 +23,11 @@ If you pay for ChatGPT Plus or Pro, you can skip the API key: open the usage included in your ChatGPT plan instead of API billing. Tap **List models** and pick one, then **Save**. +Before the sign-in starts, the app links OpenAI's +[Terms of Use](https://openai.com/policies/terms-of-use/) and +[Usage Policies](https://openai.com/policies/usage-policies/). They apply to +your use of ChatGPT here, so read them before you continue. + - This uses OpenAI's official [Sign in with ChatGPT](https://developers.openai.com/siwc/token-sharing-open-source) for open-source apps. OpenStrap never sees your password, and never charges diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index b4ce0b6de..8bd3f9eaf 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -1454,7 +1454,11 @@ class AppState extends ChangeNotifier { _log('[reset] widget clear failed: $e'); } try { - await coachConfig?.save(apiKey: ''); // deletes the keychain entry + await coachConfig?.save( + apiKey: '', // deletes the keychain entry + useChatGpt: false, + model: '', + ); } catch (e) { _log('[reset] keychain clear failed: $e'); } From 0fb01be56035a9ef95bd459ba93dcfe37e78f1c2 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 13:03:39 +0530 Subject: [PATCH 21/23] reset: say so when the chatgpt sign-in may have outlived it forgetAll swallowed an unconfirmed revocation and a failed keychain delete, so delete everything finished as if the sign-in was gone. it now reports both, and the reset ends with the same disconnect-in-chatgpt hint sign-out already shows. --- lib/coach/coach_chatgpt.dart | 18 +++++++++++++----- lib/state/app_state.dart | 7 ++++++- lib/ui2/profile/settings.dart | 8 ++++++++ test/coach_chatgpt_test.dart | 3 ++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index e95a0092d..59d8243a6 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -792,7 +792,9 @@ class ChatGptAuth extends ChangeNotifier { /// "Reset all data": the session plus the registration and welcome flag. /// The session is revoked first, best effort, while the registration to /// revoke it with is still here. - Future forgetAll() async { + /// Returns false when a session may outlive the reset: revocation was not + /// confirmed, or the keychain item could not be deleted. + Future forgetAll() async { _generation++; cancelSignIn(); final s = _session; @@ -803,19 +805,25 @@ class ChatGptAuth extends ChangeNotifier { final rotated = await _rotationOf(refreshing); final token = s?.refreshToken; final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); - if (rotated == null && s != null && clientId != null) { - await _revokeIssued(refreshToken: token, accessToken: s.accessToken, clientId: clientId); - } + final revoked = rotated ?? + (s == null || + (clientId != null && + await _revokeIssued( + refreshToken: token, accessToken: s.accessToken, clientId: clientId))); + var deleted = true; await _serialized(() async { try { await _secure.delete(key: _kSession, iOptions: _apple, mOptions: _macos); - } catch (_) {} + } catch (_) { + deleted = false; + } final prefs = await SharedPreferences.getInstance(); for (final k in [_kClientId, _kHostId, _kPresent, _kWelcomed]) { await prefs.remove(k); } }); notifyListeners(); + return revoked && deleted; } /// True exactly once, on the first sign-in that has plan usage. diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index 8bd3f9eaf..b581d5318 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -1345,6 +1345,10 @@ class AppState extends ChangeNotifier { /// confirmation would otherwise read as a clean wipe. bool resetLeftPairingKeys = false; + /// Set by [resetAllData] when the ChatGPT sign-in may outlive it: OpenAI did + /// not confirm the revocation, or the keychain item could not be deleted. + bool resetLeftChatGpt = false; + /// "Delete everything", and mean it. /// /// The old reset walked `availableDays()` — the non-skipped DERIVED days — @@ -1463,8 +1467,9 @@ class AppState extends ChangeNotifier { _log('[reset] keychain clear failed: $e'); } try { - await coachConfig?.chatgpt.forgetAll(); + resetLeftChatGpt = !(await coachConfig?.chatgpt.forgetAll() ?? true); } catch (e) { + resetLeftChatGpt = true; _log('[reset] ChatGPT sign-in clear failed: $e'); } // Coach chats are files, not rows, so wipeAll does not reach them. diff --git a/lib/ui2/profile/settings.dart b/lib/ui2/profile/settings.dart index 3e92ba294..df607a15e 100644 --- a/lib/ui2/profile/settings.dart +++ b/lib/ui2/profile/settings.dart @@ -527,6 +527,9 @@ Future _confirmReset(BuildContext c, AppState app) async { ); if (ok != true || !c.mounted) return; final messenger = ScaffoldMessenger.of(c); + // Read while [c] is still mounted: the reset pops this screen. + final chatGptLeft = uiText(c, + 'App data deleted, but ChatGPT did not confirm the sign-in was revoked. You can also disconnect OpenStrap in ChatGPT settings.'); String? backupCleanupError; try { backupCleanupError = await app.resetAllData(); @@ -549,6 +552,11 @@ Future _confirmReset(BuildContext c, AppState app) async { 'Some files or folder access may remain.'), )); } + if (app.resetLeftChatGpt && messenger.mounted) { + messenger.showSnackBar(SnackBar( + content: Text(chatGptLeft), + )); + } if (app.resetLeftPairingKeys && messenger.mounted) { messenger.showSnackBar(SnackBar( content: Text(l?.settingsResetPairingKeyWarning ?? diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index e080362cb..4bdaa9d03 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -912,7 +912,8 @@ void main() { await auth.debugSetSession(_session()); expect(await auth.takeWelcome(), isTrue); expect(await auth.takeWelcome(), isFalse); - await auth.forgetAll(); + expect(await auth.forgetAll(), status == 200, + reason: 'reset reports a revocation it could not confirm'); final prefs = await SharedPreferences.getInstance(); expect(revoked, ['rt-1', 'at-1']); expect(auth.signedIn, isFalse); From ef5ce9346687c53a4f1e91dd0e699f9405ce0521 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 13:17:30 +0530 Subject: [PATCH 22/23] reset: cover a session not loaded yet and an exchange still out load runs in the background, so a reset right after launch saw no session, deleted the keychain item and never revoked it; it now reads the stored session first and does not call it revoked when it cannot. a reset during a code exchange now waits for that exchange to revoke its tokens and reports the result. the reset warning is translated and no longer claims revocation failed when only the local delete did. --- lib/coach/coach_chatgpt.dart | 51 ++++++++++++++++++++++++------ lib/l10n/app_de.arb | 1 + lib/l10n/app_en.arb | 4 +++ lib/l10n/app_es.arb | 1 + lib/l10n/app_fr.arb | 1 + lib/l10n/app_hi.arb | 1 + lib/l10n/app_ru.arb | 1 + lib/l10n/app_zh.arb | 1 + lib/ui2/profile/settings.dart | 7 ++--- test/coach_chatgpt_test.dart | 58 +++++++++++++++++++++++++++++++++-- 10 files changed, 109 insertions(+), 17 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 59d8243a6..1c01f88ff 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -338,6 +338,12 @@ class ChatGptAuth extends ChangeNotifier { ChatGptSession? _session; bool _unreadable = false; HttpServer? _pending; + + /// A code exchange that is out: completes with whether every token it was + /// issued is either stored or confirmed revoked. [forgetAll] waits on it so + /// a reset cannot report a clean wipe while a late exchange still holds + /// tokens it failed to revoke. + Future? _exchanging; Future? _refreshing; /// The [_generation] [_refreshing] was started under. A session that came @@ -489,6 +495,7 @@ class ChatGptAuth extends ChangeNotifier { await prefs.setString(_kHostId, hostId); final savedClientId = prefs.getString(_kClientId); final server = _pending = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + Completer? exchanged; try { final redirectUri = 'http://127.0.0.1:${server.port}/auth/callback'; final state = _random(), nonce = _random(), verifier = _random(); @@ -525,6 +532,8 @@ class ChatGptAuth extends ChangeNotifier { } // Saved BEFORE the exchange, so a retry after a failed one reuses it. await prefs.setString(_kClientId, clientId); + exchanged = Completer(); + _exchanging = exchanged.future; final Map t; try { t = await _token({ @@ -562,14 +571,17 @@ class ChatGptAuth extends ChangeNotifier { attempt: attempt, ); } catch (_) { - await _revokeIssued( + exchanged.complete(await _revokeIssued( refreshToken: t['refresh_token'] is String ? t['refresh_token'] as String : null, accessToken: t['access_token'] as String, // [_token] checked it clientId: clientId, - ); + )); rethrow; } } finally { + // Stored, or no tokens were issued: nothing is left behind. + if (exchanged != null && !exchanged.isCompleted) exchanged.complete(true); + if (exchanged != null && identical(_exchanging, exchanged.future)) _exchanging = null; if (identical(_pending, server)) _pending = null; await server.close(force: true); } @@ -797,19 +809,38 @@ class ChatGptAuth extends ChangeNotifier { Future forgetAll() async { _generation++; cancelSignIn(); - final s = _session; + final exchanging = _exchanging; + var s = _session; final refreshing = _refreshing; _session = null; _unreadable = false; // As in [signOut]: an in-flight refresh revokes its own rotated tokens. final rotated = await _rotationOf(refreshing); + final prefs = await SharedPreferences.getInstance(); + final clientId = prefs.getString(_kClientId); + // [load] runs in the background at startup, so nothing in memory need not + // mean nothing stored: read the keychain before deleting it, and do not + // call a session revoked when it could not be read. + var unreadable = false; + if (s == null && rotated == null) { + try { + final raw = await _secure.read(key: _kSession, iOptions: _apple, mOptions: _macos); + if (raw != null && raw.isNotEmpty) { + s = ChatGptSession.fromJson(jsonDecode(raw) as Map); + } + } catch (_) { + unreadable = prefs.getBool(_kPresent) ?? false; + } + } final token = s?.refreshToken; - final clientId = (await SharedPreferences.getInstance()).getString(_kClientId); - final revoked = rotated ?? - (s == null || - (clientId != null && - await _revokeIssued( - refreshToken: token, accessToken: s.accessToken, clientId: clientId))); + final revoked = !unreadable && + (rotated ?? + (s == null || + (clientId != null && + await _revokeIssued( + refreshToken: token, accessToken: s.accessToken, clientId: clientId)))); + // A sign-in whose code exchange was out revokes what it was issued itself. + final exchangeClean = exchanging == null || await exchanging; var deleted = true; await _serialized(() async { try { @@ -823,7 +854,7 @@ class ChatGptAuth extends ChangeNotifier { } }); notifyListeners(); - return revoked && deleted; + return revoked && exchangeClean && deleted; } /// True exactly once, on the first sign-in that has plan usage. diff --git a/lib/l10n/app_de.arb b/lib/l10n/app_de.arb index 8043da17e..506424778 100644 --- a/lib/l10n/app_de.arb +++ b/lib/l10n/app_de.arb @@ -2247,6 +2247,7 @@ "settingsResetTitle": "Alles löschen?", "settingsResetBackupWarning": "App-Daten gelöscht. Die Backup-Bereinigung war unvollständig. Einige Dateien oder Ordnerzugriffe können erhalten bleiben.", "settingsResetPairingKeyWarning": "App-Daten gelöscht, aber ein Kopplungsschlüssel eines Sensors konnte nicht aus dem sicheren Speicher entfernt werden. Lösche alles erneut, um es noch einmal zu versuchen.", + "settingsResetChatGptWarning": "App-Daten gelöscht, aber die ChatGPT-Anmeldung wurde möglicherweise nicht vollständig entfernt. Lösche alles erneut, um es noch einmal zu versuchen, oder trenne OpenStrap in den ChatGPT-Einstellungen.", "settingsResetBody": "Diese App-Daten werden auf diesem Gerät dauerhaft gelöscht:\n\n· alle gemessenen Tage, Schlafdaten, Trainings und Routen\n· alle Laborwerte, Mahlzeiten, Medikamenteneinnahmen, Gewohnheiten, Atemübungen und eingetragenen Sätze\n· dein Journal, dein Zykluskalender und deine Baselines\n· dein Profil, alle Einstellungen und ein gespeicherter KI-Schlüssel\n· das Widget und alle geplanten Erinnerungen\n\nExportierte Kopien und Backups in früheren oder nicht verfügbaren Ordnern können erhalten bleiben. Lösche sie separat.\n\nDas Band wird entkoppelt und kann bereits übertragene Daten nicht erneut senden. Wenn du eine Kopie möchtest, exportiere vorher unter „Deine Daten“.", "settingsResetKeepData": "Meine Daten behalten", "settingsResetDeleteEverything": "Alles löschen", diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 724f9d181..8208da4f5 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -11373,6 +11373,10 @@ "@settingsResetPairingKeyWarning": { "description": "Warning after reset when a sensor pairing key could not be removed from secure storage" }, + "settingsResetChatGptWarning": "App data deleted, but the ChatGPT sign-in may not be fully removed. Delete everything again to retry, or disconnect OpenStrap in ChatGPT settings.", + "@settingsResetChatGptWarning": { + "description": "Warning after reset when the ChatGPT sign-in may not have been revoked or removed from secure storage" + }, "settingsResetBody": "This permanently deletes this app's data on this device:\n\n· every measured day, sleep, workout and route\n· every lab result, meal, medication dose, habit, breathing session and logged set\n· your journal, cycle log and what the app has learned as your usual\n· your profile, every preference and any stored AI key\n· the home-screen widget and every scheduled reminder\n\nExported copies and backups in previous or unavailable folders may remain. Delete those separately.\n\nThe band is unpaired, and it cannot re-send history it has already handed over. Export from Your data first if you want a copy.", "@settingsResetBody": { "description": "Confirmation dialog body listing exactly what a full reset deletes" diff --git a/lib/l10n/app_es.arb b/lib/l10n/app_es.arb index e99aba10e..2ace5f13f 100644 --- a/lib/l10n/app_es.arb +++ b/lib/l10n/app_es.arb @@ -2196,6 +2196,7 @@ "settingsResetTitle": "¿Eliminar todo?", "settingsResetBackupWarning": "Datos de la app eliminados. La limpieza de las copias de seguridad no se completó. Pueden quedar archivos o permisos de acceso a carpetas.", "settingsResetPairingKeyWarning": "Datos de la app eliminados, pero no se pudo quitar una clave de vinculación de un sensor del almacenamiento seguro. Vuelve a eliminar todo para reintentarlo.", + "settingsResetChatGptWarning": "Datos de la app eliminados, pero puede que el inicio de sesión de ChatGPT no se haya quitado del todo. Vuelve a eliminar todo para reintentarlo o desconecta OpenStrap en los ajustes de ChatGPT.", "settingsResetBody": "Esto elimina permanentemente los datos de esta app en este dispositivo:\n\n· cada día medido, sueño, entrenamiento y ruta\n· cada resultado de laboratorio, comida, dosis de medicación, hábito, sesión de respiración y serie registrada\n· tu diario, registro de ciclo y líneas base continuas\n· tu perfil, cada preferencia y cualquier clave de IA guardada\n· el widget de pantalla de inicio y cada recordatorio programado\n\nLas copias exportadas y los backups en carpetas anteriores o no disponibles pueden permanecer. Elimínalos por separado.\n\nEl band queda desvinculado, y no puede reenviar el historial que ya entregó. Exporta desde Tus datos primero si quieres una copia.", "settingsResetKeepData": "Conservar mis datos", "settingsResetDeleteEverything": "Eliminar todo", diff --git a/lib/l10n/app_fr.arb b/lib/l10n/app_fr.arb index 58840a3d3..517be1538 100644 --- a/lib/l10n/app_fr.arb +++ b/lib/l10n/app_fr.arb @@ -2184,6 +2184,7 @@ "settingsResetTitle": "Tout supprimer ?", "settingsResetBackupWarning": "Données de l’application supprimées. Le nettoyage des sauvegardes est incomplet. Certains fichiers ou droits d’accès aux dossiers peuvent rester.", "settingsResetPairingKeyWarning": "Données de l’application supprimées, mais une clé d’appairage d’un capteur n’a pas pu être retirée du stockage sécurisé. Supprimez tout à nouveau pour réessayer.", + "settingsResetChatGptWarning": "Données de l’application supprimées, mais la connexion ChatGPT n’a peut-être pas été entièrement retirée. Supprimez tout à nouveau pour réessayer, ou déconnectez OpenStrap dans les paramètres de ChatGPT.", "settingsResetBody": "Ceci supprime définitivement les données de cette application sur cet appareil :\n\n· chaque jour mesuré, sommeil, séance et itinéraire\n· chaque résultat de labo, repas, dose de médicament, habitude, séance de respiration et série enregistrée\n· votre journal, suivi de cycle et lignes de base glissantes\n· votre profil, chaque préférence et toute clé IA stockée\n· le widget d’écran d’accueil et chaque rappel programmé\n\nLes copies exportées et les sauvegardes dans des dossiers précédents ou indisponibles peuvent rester. Supprimez-les séparément.\n\nLe bracelet est dissocié et ne peut pas renvoyer l’historique déjà transmis. Exportez depuis Vos données d’abord si vous voulez une copie.", "settingsResetKeepData": "Conserver mes données", "settingsResetDeleteEverything": "Tout supprimer", diff --git a/lib/l10n/app_hi.arb b/lib/l10n/app_hi.arb index 027cccad3..fa1356412 100644 --- a/lib/l10n/app_hi.arb +++ b/lib/l10n/app_hi.arb @@ -2184,6 +2184,7 @@ "settingsResetTitle": "सब कुछ मिटा दें?", "settingsResetBackupWarning": "ऐप का डेटा मिटा दिया गया। बैकअप की सफ़ाई पूरी नहीं हुई। कुछ फ़ाइलें या फ़ोल्डर तक पहुंच की अनुमति रह सकती है।", "settingsResetPairingKeyWarning": "ऐप का डेटा मिटा दिया गया, लेकिन किसी सेंसर की पेयरिंग कुंजी सुरक्षित स्टोरेज से नहीं हटाई जा सकी। फिर से कोशिश करने के लिए सब कुछ दोबारा मिटाएँ।", + "settingsResetChatGptWarning": "ऐप का डेटा मिटा दिया गया, लेकिन ChatGPT साइन-इन शायद पूरी तरह नहीं हटा। फिर से कोशिश करने के लिए सब कुछ दोबारा मिटाएँ, या ChatGPT सेटिंग्स में OpenStrap को डिस्कनेक्ट करें।", "settingsResetBody": "यह इस डिवाइस पर इस ऐप का डेटा स्थायी रूप से मिटा देता है:\n\n· हर मापा गया दिन, नींद, वर्कआउट और रूट\n· हर लैब परिणाम, भोजन, दवा की खुराक, आदत, श्वास सत्र और लॉग किया गया सेट\n· आपकी डायरी, चक्र लॉग और चलती बेसलाइन\n· आपकी प्रोफ़ाइल, हर प्राथमिकता और कोई भी सहेजी गई AI कुंजी\n· होम-स्क्रीन विजेट और हर शेड्यूल किया गया रिमाइंडर\n\nएक्सपोर्ट की गई प्रतियां और पुराने या उपलब्ध न होने वाले फ़ोल्डरों में बैकअप रह सकते हैं। उन्हें अलग से मिटाएं।\n\nबैंड की जोड़ी हटा दी जाती है, और वह पहले से भेजा गया इतिहास दोबारा नहीं भेज सकता। यदि आपको एक प्रति चाहिए तो पहले 'आपका डेटा' से एक्सपोर्ट करें।", "settingsResetKeepData": "मेरा डेटा रखें", "settingsResetDeleteEverything": "सब कुछ मिटाएँ", diff --git a/lib/l10n/app_ru.arb b/lib/l10n/app_ru.arb index c1d4b9fe6..746712303 100644 --- a/lib/l10n/app_ru.arb +++ b/lib/l10n/app_ru.arb @@ -6774,6 +6774,7 @@ "coachApiFormatLabel": "API", "settingsResetBackupWarning": "Данные приложения удалены. Очистка резервных копий завершилась не полностью: некоторые файлы или доступ к папке могут остаться.", "settingsResetPairingKeyWarning": "Данные приложения удалены, но ключ сопряжения датчика не удалось удалить из защищённого хранилища. Удалите всё ещё раз, чтобы повторить попытку.", + "settingsResetChatGptWarning": "Данные приложения удалены, но вход в ChatGPT, возможно, удалён не полностью. Удалите всё ещё раз, чтобы повторить попытку, или отключите OpenStrap в настройках ChatGPT.", "dataBackupFolder": "Папка резервных копий", "dataBackupFolderDefault": "Папка приложения (по умолчанию)", "dataBackupFolderSub": "Выберите, куда сохранять новые резервные копии. Существующие копии останутся на месте", diff --git a/lib/l10n/app_zh.arb b/lib/l10n/app_zh.arb index 786598d17..8d2a2b5de 100644 --- a/lib/l10n/app_zh.arb +++ b/lib/l10n/app_zh.arb @@ -2184,6 +2184,7 @@ "settingsResetTitle": "删除全部数据？", "settingsResetBackupWarning": "应用数据已删除。备份清理未完成，部分文件或文件夹访问权限可能仍然保留。", "settingsResetPairingKeyWarning": "应用数据已删除，但某个传感器的配对密钥无法从安全存储中移除。请再次删除全部以重试。", + "settingsResetChatGptWarning": "应用数据已删除，但 ChatGPT 登录可能未完全移除。请再次删除全部以重试，或在 ChatGPT 设置中断开 OpenStrap。", "settingsResetBody": "此操作将永久删除此设备上本应用的数据：\n\n· 每一天的测量数据、睡眠、锻炼和路线\n· 每一项化验结果、饮食、用药剂量、习惯、呼吸训练和记录的组数\n· 你的日记、周期记录和滚动基线\n· 你的个人资料、所有偏好设置以及保存的任何 AI 密钥\n· 主屏幕小组件和所有已安排的提醒\n\n导出的副本以及旧文件夹或无法访问的文件夹中的备份可能仍然保留。请单独删除这些文件。\n\n手环会解除配对，并且无法重新发送已经交出的历史数据。如果需要保留副本，请先在“你的数据”中导出。", "settingsResetKeepData": "保留我的数据", "settingsResetDeleteEverything": "删除全部", diff --git a/lib/ui2/profile/settings.dart b/lib/ui2/profile/settings.dart index df607a15e..ca3316e5a 100644 --- a/lib/ui2/profile/settings.dart +++ b/lib/ui2/profile/settings.dart @@ -527,9 +527,6 @@ Future _confirmReset(BuildContext c, AppState app) async { ); if (ok != true || !c.mounted) return; final messenger = ScaffoldMessenger.of(c); - // Read while [c] is still mounted: the reset pops this screen. - final chatGptLeft = uiText(c, - 'App data deleted, but ChatGPT did not confirm the sign-in was revoked. You can also disconnect OpenStrap in ChatGPT settings.'); String? backupCleanupError; try { backupCleanupError = await app.resetAllData(); @@ -554,7 +551,9 @@ Future _confirmReset(BuildContext c, AppState app) async { } if (app.resetLeftChatGpt && messenger.mounted) { messenger.showSnackBar(SnackBar( - content: Text(chatGptLeft), + content: Text(l?.settingsResetChatGptWarning ?? + 'App data deleted, but the ChatGPT sign-in may not be fully removed. ' + 'Delete everything again to retry, or disconnect OpenStrap in ChatGPT settings.'), )); } if (app.resetLeftPairingKeys && messenger.mounted) { diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index 4bdaa9d03..a88295be0 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -249,14 +249,14 @@ void main() { /// records every revocation. ChatGptAuth pendingExchange(Completer gate, Completer arrived, List<(String, String, String)> revoked, Uri Function() authorize, - {bool signed = true}) => + {bool signed = true, int revokeStatus = 200}) => ChatGptAuth(client: MockClient((req) async { final oidc = _oidc(req); if (oidc != null) return oidc; if (req.url.path.endsWith('/revoke')) { revoked.add((req.bodyFields['token']!, req.bodyFields['token_type_hint']!, req.bodyFields['client_id']!)); - return http.Response('', 200); + return http.Response('', revokeStatus); } arrived.complete(); await gate.future; @@ -294,8 +294,11 @@ void main() { }), throwsA(isA())); await arrived.future; - await end(auth); + // A reset waits for the exchange it interrupted, so it is not awaited + // before the exchange is let through. + final ended = end(auth); gate.complete(); + await ended; await signIn; expect(auth.signedIn, isFalse); expect(keychain, isEmpty); @@ -304,6 +307,55 @@ void main() { }); } + for (final status in [200, 500]) { + test('a reset reports whether an interrupted code exchange revoked its tokens ' + '(revoke $status)', () async { + final gate = Completer(), arrived = Completer(); + final revoked = <(String, String, String)>[]; + late Uri first; + auth = pendingExchange(gate, arrived, revoked, () => first, revokeStatus: status); + final signIn = expectLater( + auth.signIn((u) { + first = u; + return browser(u, clientId: 'oaiapp_1'); + }), + throwsA(isA())); + await arrived.future; + final reset = auth.forgetAll(); + gate.complete(); + expect(await reset, status == 200); + await signIn; + expect(revoked, issued); + }); + } + + test('a reset before the stored session has loaded still revokes it', () async { + SharedPreferences.setMockInitialValues( + {'coach_chatgpt_client_id': 'oaiapp_1', 'coach_chatgpt_present': true}); + keychain['coach_chatgpt_session'] = jsonEncode(_session().toJson()); + final revoked = []; + final auth = ChatGptAuth(client: MockClient((req) async { + revoked.add(req.bodyFields['token']!); + return http.Response('', 200); + })); + expect(auth.signedIn, isFalse, reason: 'load has not run'); + expect(await auth.forgetAll(), isTrue); + expect(revoked, ['rt-1', 'at-1']); + expect(keychain, isEmpty); + }); + + test('a reset that cannot read the stored session does not call it revoked', () async { + SharedPreferences.setMockInitialValues( + {'coach_chatgpt_client_id': 'oaiapp_1', 'coach_chatgpt_present': true}); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + if (call.method == 'read') throw PlatformException(code: 'locked'); + return null; + }); + final auth = ChatGptAuth(client: MockClient((_) async => fail('nothing to revoke with'))); + expect(await auth.forgetAll(), isFalse); + }); + test('a sign-in does not start while a reset is running', () async { ResetGate.enter(); addTearDown(ResetGate.resetForTest); From 3ae4eca4b707021831f47107577e6777fc73181a Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 1 Nov 2026 13:46:11 +0530 Subject: [PATCH 23/23] coach: close two sign-in windows a cancel or reset could slip through a cancel while the loopback listener was still binding had no server to close, so the attempt opened the browser anyway. a reset that started while the client id was being written did not see the exchange yet and could report a clean wipe before it. the reset warning now says that only disconnecting in chatgpt settings revokes access. --- lib/coach/coach_chatgpt.dart | 12 ++++++++++-- lib/l10n/app_de.arb | 2 +- lib/l10n/app_en.arb | 2 +- lib/l10n/app_es.arb | 2 +- lib/l10n/app_fr.arb | 2 +- lib/l10n/app_hi.arb | 2 +- lib/l10n/app_ru.arb | 2 +- lib/l10n/app_zh.arb | 2 +- lib/ui2/profile/settings.dart | 3 ++- test/coach_chatgpt_test.dart | 7 +++++++ 10 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/coach/coach_chatgpt.dart b/lib/coach/coach_chatgpt.dart index 1c01f88ff..2111c04e7 100644 --- a/lib/coach/coach_chatgpt.dart +++ b/lib/coach/coach_chatgpt.dart @@ -497,6 +497,9 @@ class ChatGptAuth extends ChangeNotifier { final server = _pending = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); Completer? exchanged; try { + // Cancelled while the listener was binding: [cancelSignIn] had no + // server to close yet, so this attempt would open the browser anyway. + if (attempt != _attempt) throw ChatGptException('Sign-in was cancelled.'); final redirectUri = 'http://127.0.0.1:${server.port}/auth/callback'; final state = _random(), nonce = _random(), verifier = _random(); final uri = chatGptAuthorizeUri( @@ -530,10 +533,15 @@ class ChatGptAuth extends ChangeNotifier { if (clientId == null || q['code'] == null) { throw ChatGptException('ChatGPT sign-in returned no code.'); } - // Saved BEFORE the exchange, so a retry after a failed one reuses it. - await prefs.setString(_kClientId, clientId); + // Registered before the first await from here on, so a reset that + // starts while the client id is being written still waits for this + // exchange (and deletes the client id only after it). exchanged = Completer(); _exchanging = exchanged.future; + // Saved BEFORE the exchange, so a retry after a failed one reuses it. + await prefs.setString(_kClientId, clientId); + if (generation != _generation) throw ChatGptException('Signed out.'); + if (attempt != _attempt) throw ChatGptException('Sign-in was cancelled.'); final Map t; try { t = await _token({ diff --git a/lib/l10n/app_de.arb b/lib/l10n/app_de.arb index 506424778..3f7c9e49f 100644 --- a/lib/l10n/app_de.arb +++ b/lib/l10n/app_de.arb @@ -2247,7 +2247,7 @@ "settingsResetTitle": "Alles löschen?", "settingsResetBackupWarning": "App-Daten gelöscht. Die Backup-Bereinigung war unvollständig. Einige Dateien oder Ordnerzugriffe können erhalten bleiben.", "settingsResetPairingKeyWarning": "App-Daten gelöscht, aber ein Kopplungsschlüssel eines Sensors konnte nicht aus dem sicheren Speicher entfernt werden. Lösche alles erneut, um es noch einmal zu versuchen.", - "settingsResetChatGptWarning": "App-Daten gelöscht, aber die ChatGPT-Anmeldung wurde möglicherweise nicht vollständig entfernt. Lösche alles erneut, um es noch einmal zu versuchen, oder trenne OpenStrap in den ChatGPT-Einstellungen.", + "settingsResetChatGptWarning": "App-Daten gelöscht, aber die ChatGPT-Anmeldung wurde möglicherweise nicht vollständig entfernt. Trenne OpenStrap in den ChatGPT-Einstellungen, damit der Zugriff sicher widerrufen ist. Erneutes Löschen entfernt nur noch, was auf diesem Gerät übrig ist.", "settingsResetBody": "Diese App-Daten werden auf diesem Gerät dauerhaft gelöscht:\n\n· alle gemessenen Tage, Schlafdaten, Trainings und Routen\n· alle Laborwerte, Mahlzeiten, Medikamenteneinnahmen, Gewohnheiten, Atemübungen und eingetragenen Sätze\n· dein Journal, dein Zykluskalender und deine Baselines\n· dein Profil, alle Einstellungen und ein gespeicherter KI-Schlüssel\n· das Widget und alle geplanten Erinnerungen\n\nExportierte Kopien und Backups in früheren oder nicht verfügbaren Ordnern können erhalten bleiben. Lösche sie separat.\n\nDas Band wird entkoppelt und kann bereits übertragene Daten nicht erneut senden. Wenn du eine Kopie möchtest, exportiere vorher unter „Deine Daten“.", "settingsResetKeepData": "Meine Daten behalten", "settingsResetDeleteEverything": "Alles löschen", diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 8208da4f5..5686693e1 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -11373,7 +11373,7 @@ "@settingsResetPairingKeyWarning": { "description": "Warning after reset when a sensor pairing key could not be removed from secure storage" }, - "settingsResetChatGptWarning": "App data deleted, but the ChatGPT sign-in may not be fully removed. Delete everything again to retry, or disconnect OpenStrap in ChatGPT settings.", + "settingsResetChatGptWarning": "App data deleted, but the ChatGPT sign-in may not be fully removed. Disconnect OpenStrap in ChatGPT settings to make sure its access is revoked. Deleting everything again only retries removing what is left on this device.", "@settingsResetChatGptWarning": { "description": "Warning after reset when the ChatGPT sign-in may not have been revoked or removed from secure storage" }, diff --git a/lib/l10n/app_es.arb b/lib/l10n/app_es.arb index 2ace5f13f..44c78ec7c 100644 --- a/lib/l10n/app_es.arb +++ b/lib/l10n/app_es.arb @@ -2196,7 +2196,7 @@ "settingsResetTitle": "¿Eliminar todo?", "settingsResetBackupWarning": "Datos de la app eliminados. La limpieza de las copias de seguridad no se completó. Pueden quedar archivos o permisos de acceso a carpetas.", "settingsResetPairingKeyWarning": "Datos de la app eliminados, pero no se pudo quitar una clave de vinculación de un sensor del almacenamiento seguro. Vuelve a eliminar todo para reintentarlo.", - "settingsResetChatGptWarning": "Datos de la app eliminados, pero puede que el inicio de sesión de ChatGPT no se haya quitado del todo. Vuelve a eliminar todo para reintentarlo o desconecta OpenStrap en los ajustes de ChatGPT.", + "settingsResetChatGptWarning": "Datos de la app eliminados, pero puede que el inicio de sesión de ChatGPT no se haya quitado del todo. Desconecta OpenStrap en los ajustes de ChatGPT para asegurarte de que su acceso queda revocado. Volver a eliminar todo solo reintenta quitar lo que quede en este dispositivo.", "settingsResetBody": "Esto elimina permanentemente los datos de esta app en este dispositivo:\n\n· cada día medido, sueño, entrenamiento y ruta\n· cada resultado de laboratorio, comida, dosis de medicación, hábito, sesión de respiración y serie registrada\n· tu diario, registro de ciclo y líneas base continuas\n· tu perfil, cada preferencia y cualquier clave de IA guardada\n· el widget de pantalla de inicio y cada recordatorio programado\n\nLas copias exportadas y los backups en carpetas anteriores o no disponibles pueden permanecer. Elimínalos por separado.\n\nEl band queda desvinculado, y no puede reenviar el historial que ya entregó. Exporta desde Tus datos primero si quieres una copia.", "settingsResetKeepData": "Conservar mis datos", "settingsResetDeleteEverything": "Eliminar todo", diff --git a/lib/l10n/app_fr.arb b/lib/l10n/app_fr.arb index 517be1538..08423710e 100644 --- a/lib/l10n/app_fr.arb +++ b/lib/l10n/app_fr.arb @@ -2184,7 +2184,7 @@ "settingsResetTitle": "Tout supprimer ?", "settingsResetBackupWarning": "Données de l’application supprimées. Le nettoyage des sauvegardes est incomplet. Certains fichiers ou droits d’accès aux dossiers peuvent rester.", "settingsResetPairingKeyWarning": "Données de l’application supprimées, mais une clé d’appairage d’un capteur n’a pas pu être retirée du stockage sécurisé. Supprimez tout à nouveau pour réessayer.", - "settingsResetChatGptWarning": "Données de l’application supprimées, mais la connexion ChatGPT n’a peut-être pas été entièrement retirée. Supprimez tout à nouveau pour réessayer, ou déconnectez OpenStrap dans les paramètres de ChatGPT.", + "settingsResetChatGptWarning": "Données de l’application supprimées, mais la connexion ChatGPT n’a peut-être pas été entièrement retirée. Déconnectez OpenStrap dans les paramètres de ChatGPT pour vous assurer que son accès est révoqué. Tout supprimer à nouveau ne fait que réessayer d’effacer ce qui reste sur cet appareil.", "settingsResetBody": "Ceci supprime définitivement les données de cette application sur cet appareil :\n\n· chaque jour mesuré, sommeil, séance et itinéraire\n· chaque résultat de labo, repas, dose de médicament, habitude, séance de respiration et série enregistrée\n· votre journal, suivi de cycle et lignes de base glissantes\n· votre profil, chaque préférence et toute clé IA stockée\n· le widget d’écran d’accueil et chaque rappel programmé\n\nLes copies exportées et les sauvegardes dans des dossiers précédents ou indisponibles peuvent rester. Supprimez-les séparément.\n\nLe bracelet est dissocié et ne peut pas renvoyer l’historique déjà transmis. Exportez depuis Vos données d’abord si vous voulez une copie.", "settingsResetKeepData": "Conserver mes données", "settingsResetDeleteEverything": "Tout supprimer", diff --git a/lib/l10n/app_hi.arb b/lib/l10n/app_hi.arb index fa1356412..39a97f40a 100644 --- a/lib/l10n/app_hi.arb +++ b/lib/l10n/app_hi.arb @@ -2184,7 +2184,7 @@ "settingsResetTitle": "सब कुछ मिटा दें?", "settingsResetBackupWarning": "ऐप का डेटा मिटा दिया गया। बैकअप की सफ़ाई पूरी नहीं हुई। कुछ फ़ाइलें या फ़ोल्डर तक पहुंच की अनुमति रह सकती है।", "settingsResetPairingKeyWarning": "ऐप का डेटा मिटा दिया गया, लेकिन किसी सेंसर की पेयरिंग कुंजी सुरक्षित स्टोरेज से नहीं हटाई जा सकी। फिर से कोशिश करने के लिए सब कुछ दोबारा मिटाएँ।", - "settingsResetChatGptWarning": "ऐप का डेटा मिटा दिया गया, लेकिन ChatGPT साइन-इन शायद पूरी तरह नहीं हटा। फिर से कोशिश करने के लिए सब कुछ दोबारा मिटाएँ, या ChatGPT सेटिंग्स में OpenStrap को डिस्कनेक्ट करें।", + "settingsResetChatGptWarning": "ऐप का डेटा मिटा दिया गया, लेकिन ChatGPT साइन-इन शायद पूरी तरह नहीं हटा। पहुँच पक्का रद्द करने के लिए ChatGPT सेटिंग्स में OpenStrap को डिस्कनेक्ट करें। सब कुछ दोबारा मिटाने से सिर्फ़ इस डिवाइस पर बचा हिस्सा हटाने की फिर कोशिश होती है।", "settingsResetBody": "यह इस डिवाइस पर इस ऐप का डेटा स्थायी रूप से मिटा देता है:\n\n· हर मापा गया दिन, नींद, वर्कआउट और रूट\n· हर लैब परिणाम, भोजन, दवा की खुराक, आदत, श्वास सत्र और लॉग किया गया सेट\n· आपकी डायरी, चक्र लॉग और चलती बेसलाइन\n· आपकी प्रोफ़ाइल, हर प्राथमिकता और कोई भी सहेजी गई AI कुंजी\n· होम-स्क्रीन विजेट और हर शेड्यूल किया गया रिमाइंडर\n\nएक्सपोर्ट की गई प्रतियां और पुराने या उपलब्ध न होने वाले फ़ोल्डरों में बैकअप रह सकते हैं। उन्हें अलग से मिटाएं।\n\nबैंड की जोड़ी हटा दी जाती है, और वह पहले से भेजा गया इतिहास दोबारा नहीं भेज सकता। यदि आपको एक प्रति चाहिए तो पहले 'आपका डेटा' से एक्सपोर्ट करें।", "settingsResetKeepData": "मेरा डेटा रखें", "settingsResetDeleteEverything": "सब कुछ मिटाएँ", diff --git a/lib/l10n/app_ru.arb b/lib/l10n/app_ru.arb index 746712303..b735545ba 100644 --- a/lib/l10n/app_ru.arb +++ b/lib/l10n/app_ru.arb @@ -6774,7 +6774,7 @@ "coachApiFormatLabel": "API", "settingsResetBackupWarning": "Данные приложения удалены. Очистка резервных копий завершилась не полностью: некоторые файлы или доступ к папке могут остаться.", "settingsResetPairingKeyWarning": "Данные приложения удалены, но ключ сопряжения датчика не удалось удалить из защищённого хранилища. Удалите всё ещё раз, чтобы повторить попытку.", - "settingsResetChatGptWarning": "Данные приложения удалены, но вход в ChatGPT, возможно, удалён не полностью. Удалите всё ещё раз, чтобы повторить попытку, или отключите OpenStrap в настройках ChatGPT.", + "settingsResetChatGptWarning": "Данные приложения удалены, но вход в ChatGPT, возможно, удалён не полностью. Отключите OpenStrap в настройках ChatGPT, чтобы его доступ точно был отозван. Повторное удаление всего лишь ещё раз попробует убрать то, что осталось на этом устройстве.", "dataBackupFolder": "Папка резервных копий", "dataBackupFolderDefault": "Папка приложения (по умолчанию)", "dataBackupFolderSub": "Выберите, куда сохранять новые резервные копии. Существующие копии останутся на месте", diff --git a/lib/l10n/app_zh.arb b/lib/l10n/app_zh.arb index 8d2a2b5de..d9969ca7f 100644 --- a/lib/l10n/app_zh.arb +++ b/lib/l10n/app_zh.arb @@ -2184,7 +2184,7 @@ "settingsResetTitle": "删除全部数据？", "settingsResetBackupWarning": "应用数据已删除。备份清理未完成，部分文件或文件夹访问权限可能仍然保留。", "settingsResetPairingKeyWarning": "应用数据已删除，但某个传感器的配对密钥无法从安全存储中移除。请再次删除全部以重试。", - "settingsResetChatGptWarning": "应用数据已删除，但 ChatGPT 登录可能未完全移除。请再次删除全部以重试，或在 ChatGPT 设置中断开 OpenStrap。", + "settingsResetChatGptWarning": "应用数据已删除，但 ChatGPT 登录可能未完全移除。请在 ChatGPT 设置中断开 OpenStrap，以确保其访问权限已撤销。再次删除全部只会重试清除此设备上剩余的内容。", "settingsResetBody": "此操作将永久删除此设备上本应用的数据：\n\n· 每一天的测量数据、睡眠、锻炼和路线\n· 每一项化验结果、饮食、用药剂量、习惯、呼吸训练和记录的组数\n· 你的日记、周期记录和滚动基线\n· 你的个人资料、所有偏好设置以及保存的任何 AI 密钥\n· 主屏幕小组件和所有已安排的提醒\n\n导出的副本以及旧文件夹或无法访问的文件夹中的备份可能仍然保留。请单独删除这些文件。\n\n手环会解除配对，并且无法重新发送已经交出的历史数据。如果需要保留副本，请先在“你的数据”中导出。", "settingsResetKeepData": "保留我的数据", "settingsResetDeleteEverything": "删除全部", diff --git a/lib/ui2/profile/settings.dart b/lib/ui2/profile/settings.dart index ca3316e5a..81f088ecf 100644 --- a/lib/ui2/profile/settings.dart +++ b/lib/ui2/profile/settings.dart @@ -553,7 +553,8 @@ Future _confirmReset(BuildContext c, AppState app) async { messenger.showSnackBar(SnackBar( content: Text(l?.settingsResetChatGptWarning ?? 'App data deleted, but the ChatGPT sign-in may not be fully removed. ' - 'Delete everything again to retry, or disconnect OpenStrap in ChatGPT settings.'), + 'Disconnect OpenStrap in ChatGPT settings to make sure its access is revoked. ' + 'Deleting everything again only retries removing what is left on this device.'), )); } if (app.resetLeftPairingKeys && messenger.mounted) { diff --git a/test/coach_chatgpt_test.dart b/test/coach_chatgpt_test.dart index a88295be0..4e4c1de02 100644 --- a/test/coach_chatgpt_test.dart +++ b/test/coach_chatgpt_test.dart @@ -329,6 +329,13 @@ void main() { }); } + test('a sign-in cancelled before its listener is up never opens the browser', () async { + auth = ChatGptAuth(client: MockClient((_) async => fail('no network'))); + final signIn = auth.signIn((_) async => fail('the browser must not open')); + auth.cancelSignIn(); + await expectLater(signIn, throwsA(isA())); + }); + test('a reset before the stored session has loaded still revokes it', () async { SharedPreferences.setMockInitialValues( {'coach_chatgpt_client_id': 'oaiapp_1', 'coach_chatgpt_present': true});