refactor(ai-providers): Cursor provider review follow-ups (#1624)#1627
Conversation
…Cursor login, testable SSE parser, curated+fetched models)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a462e9171e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return try await withTaskCancellationHandler { | ||
| try await withCheckedThrowingContinuation { continuation in | ||
| process.terminationHandler = { proc in | ||
| let data = (try? stdout.fileHandleForReading.readToEnd() ?? Data()) ?? Data() |
There was a problem hiding this comment.
When agent status or agent login writes more than the OS pipe buffer to stdout/stderr, this waits to read the merged pipe until terminationHandler runs. The child can block while writing to the full pipe and therefore never terminate, so the handler never fires and status/sign-in await forever; the previous implementation drained the pipe concurrently while the process was still running.
Useful? React with 👍 / 👎.
1 participant
Follow-up fixes for the Cursor AI provider merged in #1624, addressing review feedback. No behavior change to the happy paths; these harden the CLI sign-in lifecycle and add test coverage for the streaming logic.
Changes
agent loginpreviously ran viawaitUntilExit()with no timeout or cancel path: closing the browser mid-flow left the UI stuck on "signing in" forever and parked a Swift concurrency worker thread on a live subprocess.CursorAgentCLI.runnow drives completion fromterminationHandlerinsidewithTaskCancellationHandler, andCursorAgentServiceowns the sign-in task with a newcancelSignIn(). The settings sheet gets a Cancel button while signing in.CursorProvider.streamChatinto aStreamParserstruct and covered it with unit tests (delta ordering, theresultfallback vs. skip-after-assistant dedup, blank/event lines, empty text).CursorProvider.fetchAvailableModelsnow returns the curated list first, then any extra models from/v1/models, so the curated models always appear even if the live list is sparse.AISettingsViewobservesCursorAgentService.sharedand refreshes it on appear, so a signed-in CLI user shows the right status without opening the detail sheet first.timestamp_msguard under test. ExtractedCursorAgentProvider.incrementalTextand added tests proving the consolidated final message (no timestamp) is skipped to avoid duplication.(#1624)reference to the Cursor entry.Tests
BUILD SUCCEEDED,swiftlint --strictclean on all touched files. New/updated Cursor suites pass:CursorProviderStreamParserTests,CursorAgentProviderTests(incl. the newincrementalTextcases).