[codex] fix review findings and organize Codex sources

[atom2ueki]

Summary

• Organize CodingPlanCodex sources into Core/ and Clients/, with matching test folder updates and refreshed llms.txt paths.
• Address the project, Claude, and Copilot review findings in the auth and Codex packages.
• Add regression coverage for the SSE parser, callback server port 0, and auth-state refresh behavior.

Review Findings Addressed

• Sources/CodingPlanCodex/Clients/OpenAICodexClient.swift parses buffered SSE responses with a stateful line walker so adjacent event: frames without blank separators are not dropped. Covered by Tests/CodingPlanCodexTests/Clients/OpenAICodexClientTests.swift.
• Sources/CodingPlanAuth/Infrastructure/Server/LocalCallbackServer.swift resolves port: 0 through best-effort ephemeral port discovery, sets SO_REUSEADDR, and retries resolved-port bind races before surfacing startup failure. Covered by Tests/CodingPlanAuthTests/Infrastructure/LocalCallbackServerTests.swift.
• Sources/CodingPlanAuth/Presentation/AuthState.swift clears stale in-memory auth state only for definitive credential failures, while preserving the current authenticated state for transient refresh/network failures. Covered by Tests/CodingPlanAuthTests/AuthStateTests.swift.
• Sources/CodingPlanAuth/Presentation/BrowserAuthSession.swift checks early task cancellation before starting ASWebAuthenticationSession, and still cancels the active session through the shared cancellation path.

Verification

• swift build -Xswiftc -warnings-as-errors
• swift test --parallel
• xcodebuild -scheme CodingPlanAuth -destination 'generic/platform=iOS Simulator' -skipPackagePluginValidation -derivedDataPath .derivedData build
• xcodebuild -scheme CodingPlanCodex -destination 'generic/platform=iOS Simulator' -skipPackagePluginValidation -derivedDataPath .derivedData build

[atom2ueki]

Review

Overview

Targeted fixes to four prior review findings (auth state clearing, callback server port: 0 resolution, browser session cancellation, SSE parser robustness), plus a Sources/CodingPlanCodex reorganization into Core/ + Clients/ with matching test folders and llms.txt updates.

---

What works well

• SSE parser rewrite (OpenAICodexClient.swift:564) — stateful line walker correctly handles adjacent event: frames without blank separators, SSE comment lines (:-prefixed) are silently skipped, multi-line data: joins still work, and trimming pre-prefix-check fixes prior fragility around leading whitespace. Regression test in OpenAICodexClientTests.swift:78 exercises the exact failure mode.
• Port-0 resolution (LocalCallbackServer.swift:162) — clean Darwin sockets implementation with proper defer { close(...) }, error propagation through AuthError.callbackServerError, and a > 0 sanity check on the kernel-assigned port.
• AuthState.checkStatus (AuthState.swift:50) — clearing currentCredentials/isAuthenticated before setError correctly prevents the stale-authenticated UI state. Test verifies the 401-after-success path.
• Reorg — SwiftPM auto-discovers files so no Package.swift churn; llms.txt paths updated consistently across all 9 entries.
• Verification matrix — swift build -Xswiftc -warnings-as-errors, parallel tests, plus iOS-simulator builds for both schemes.

---

Concerns / suggestions

🟡 BrowserAuthSession cancellation race (BrowserAuthSession.swift:39)

Narrow but real ordering race. Both the withCheckedThrowingContinuation body and the onCancel's Task { @MainActor … } serialize on the main actor — but if the parent task is already cancelled when authenticate(...) is awaited, onCancel can land on the main actor before the continuation body runs. In that case cancelActiveSession() sees continuation == nil and returns; then the body still proceeds to call session.start(), leaving an ASWebAuthenticationSession running with no awaiter (the orphaned finish would guard let continuation else { return } and drop the result silently — browser UI stays presented).

Suggestion: check Task.isCancelled at the top of the continuation body and resume-throw .cancelled before constructing/starting the session, or guard start() behind a flag that cancelActiveSession() can flip.

🟡 AuthState.checkStatus clears auth on any error

New behavior treats every thrown error from service.credentials(for:) as ''user is signed out.'' A definitive 401 is correct (and what the test covers), but a transient network blip during refresh() would also flip isAuthenticated to false and surface as .networkError to UI, effectively silently logging the user out. If that isn't the intended UX, narrow the clear to AuthError.tokenExchangeFailed / .invalidGrant-class errors and let network errors soft-fail while keeping cached creds.

🟢 Minor — reservation socket lacks SO_REUSEADDR

resolveListenPort doesn't set SO_REUSEADDR before bind. Not strictly required since you never listen() on the reservation socket, but cheap insurance against TIME_WAIT collisions when the resolved port is immediately reused by SwiftWebServer.

🟢 Minor — TOCTOU window is implicit

Reserve-then-bind has a known race (port could be taken between close() and SwiftWebServer's listen()). Standard workaround when the server doesn't surface its bound port, and reportsBindFailureDuringStartup should catch failures cleanly. A one-line comment on resolveListenPort explaining ''best-effort reservation; SwiftWebServer doesn't expose the assigned ephemeral port directly'' would help future readers.

🟢 Nit — LocalCallbackServerTests.swift:11

parseCallbackExtractsCodeAndState's comment still references ''NWListener-based servers''; this package uses SwiftWebServer. Predates this PR.

---

Risks

Low. Behavior changes are contained; all four fixes are test-guarded except BrowserAuthSession (justified by ASWebAuthenticationSession being hard to unit-test). Renames are pure path moves; llms.txt updated in lockstep.

Recommendation

Approve once the BrowserAuthSession race and the AuthState ''clear on any error'' question are addressed (or explicitly punted). Neither blocks merge — the prior bugs were worse — but both are easy to fix now.

[Copilot]

Pull request overview

This PR reorganizes CodingPlanCodex into Core/ and Clients/, addresses prior review findings in CodingPlanAuth and Codex streaming parsing, and adds regression tests for the fixed behaviors.

Changes:

• Reorganized Codex sources into Core/ and Clients/ and updated llms.txt paths accordingly.
• Fixed SSE buffered parsing for adjacent event: frames; added coverage.
• Improved auth reliability: resolve port: 0 startup for the local callback server, clear auth state on refresh/load failure, and cancel ASWebAuthenticationSession on task cancellation (with new tests).

Reviewed changes

Copilot reviewed 8 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
llms.txt	Updates documentation links to new Core/ and Clients/ layout.
Sources/CodingPlanCodex/Clients/OpenAICodexClient.swift	Updates buffered SSE parsing to handle adjacent event: frames.
Sources/CodingPlanCodex/Clients/OpenAIBackend.swift	Adds shared base URL + originator constants for Codex clients.
Sources/CodingPlanCodex/Clients/OpenAICodexUsageClient.swift	Adds usage/rate-limit + credits nudge email client.
Sources/CodingPlanCodex/Clients/OpenAICodexTasksClient.swift	Adds plan-bound cloud tasks client (+ JSON passthrough).
Sources/CodingPlanCodex/Clients/OpenAICodexSafetyClient.swift	Adds ARC monitor safety client with dual auth modes.
Sources/CodingPlanCodex/Clients/OpenAICodexModelsClient.swift	Adds models listing client with visibility parsing + ETag capture.
Sources/CodingPlanCodex/Clients/OpenAICodexEnvironmentsClient.swift	Adds environments + managed config requirements client.
Sources/CodingPlanCodex/Core/JSONValue.swift	Introduces a Sendable JSON passthrough value type with conveniences.
Sources/CodingPlanCodex/Core/CodexTool.swift	Adds tool modeling for /codex/responses (image generation).
Sources/CodingPlanCodex/Core/CodexStreamPart.swift	Adds stream part + image lifecycle event modeling.
Sources/CodingPlanCodex/Core/CodexError.swift	Adds Codex-specific error surface.
Sources/CodingPlanAuth/Infrastructure/Server/LocalCallbackServer.swift	Adds port == 0 resolution via ephemeral port discovery before server start.
Sources/CodingPlanAuth/Presentation/AuthState.swift	Clears in-memory auth state when checkStatus() fails.
Sources/CodingPlanAuth/Presentation/BrowserAuthSession.swift	Cancels active ASWebAuthenticationSession when the awaiting task is cancelled.
Tests/CodingPlanCodexTests/TestHelpers/MockHTTPClient.swift	Adds test HTTP client that records requests and serves stubbed responses.
Tests/CodingPlanCodexTests/Clients/OpenAICodexClientTests.swift	Adds regression test for adjacent SSE events in buffered parsing.
Tests/CodingPlanCodexTests/Clients/CompactAndMemoriesTests.swift	Adds tests for compact + memories endpoints.
Tests/CodingPlanCodexTests/Clients/OpenAICodexUsageClientTests.swift	Adds tests for usage/rate limit request + mapping + account id requirement.
Tests/CodingPlanCodexTests/Clients/SendAddCreditsNudgeTests.swift	Adds tests for credits nudge request body + missing account id.
Tests/CodingPlanCodexTests/Clients/OpenAICodexTasksClientTests.swift	Adds tests for tasks list/details/sibling turns/create task + missing account id.
Tests/CodingPlanCodexTests/Clients/OpenAICodexSafetyClientTests.swift	Adds tests for ARC auth modes + missing account id for plan credentials.
Tests/CodingPlanCodexTests/Clients/OpenAICodexModelsClientTests.swift	Adds tests for models request/decoding + missing account id.
Tests/CodingPlanCodexTests/Clients/OpenAICodexEnvironmentsClientTests.swift	Adds tests for environments/config requirements + missing account id.
Tests/CodingPlanAuthTests/Infrastructure/LocalCallbackServerTests.swift	Adds regression test ensuring port: 0 starts on a resolved port.
Tests/CodingPlanAuthTests/AuthStateTests.swift	Adds regression test that refresh failures clear authenticated state.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.