From 4d03783236ef189dc35ae51f25bd2cdd8655482d Mon Sep 17 00:00:00 2001 From: Moury Bachina Date: Thu, 15 Oct 2026 15:22:22 -0500 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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());