Drive ReviewMonitor reviews through CodexKit#78
Conversation
liveStoreCompletesBrowserLoginFromAccountNotifications synchronized only on the account update, so on slow CI runners the trailing account/rateLimits/read request could be missing from the asserted sequence. Wait for the full request count first, matching the sibling add-account test that already synchronizes through the refresh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
![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: df5b4d2d45
ℹ️ 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".
The first workspace-group drag operated on an empty preferred-order array, so the reorder could not place the dragged group relative to the target; carry the currently displayed root order through the resolved drop and merge it before reordering, mirroring the chat reorder path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bounded startReview overload lacked the cancellation handler the unbounded path has, so MCP clients disconnecting mid-wait orphaned the active worker. Timeouts still return with the worker running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pendingReviewCancellation advertised an enabled capability with no action, leaving a clickable no-op Cancel menu item; the pending state is now visible but disabled, and the context-menu test pins the disabled state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodexAppServerKit's loginChatGPT cannot yet forward the native web-authentication callback scheme to account/login/start, so the challenge must not be marked native; record why until the CodexKit API exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9be9a6539d
ℹ️ 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".
The CodexReview scheme still referenced CodexReviewTests (renamed to CodexReviewKitTests) and the deleted ArchitectureFenceTests, so Xcode and xcodebuild -scheme CodexReview test hit unresolved testables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Listed chats frequently omit status; the latest-finished scan skipped them, hiding completed chats unless the backend supplied an explicit idle status. Classify finished as not-actively-running, matching the rest of the sidebar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
orderedEntryIds/activeEntryIds still serialized every projected entry, so detailed review_read responses kept scaling with the full transcript despite the 100-item tail page; filter both arrays to the returned window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The runtime-home fallback ignored CODEX_HOME, starting the app-server against HOME/.codex_review for users and CI that isolate homes with the environment variable; honor it first, matching the pre-migration AppServerCodexHome.url order. Account registries written before the kind field also failed to decode wholesale, dropping persisted accounts; restore the decodeIfPresent + legacyDefault behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
structuredContent still built finalReview/hasFinalReview/reviewResult from core.finalReview alone, so clients consuming structured results saw a terminal review with finalReview: null whenever only the CodexChat log projection held the final review; pass the resolved value from the read path, matching the text content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd05d11c93
ℹ️ 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".
Switching chats intentionally keeps the previous transcript on screen until the next baseline renders (anti-flash, pinned by test), but when the new observation throws no baseline ever arrives and another chat's log stayed visible indefinitely; clear the pane in the failure path while the failed chat is still bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cee5f307e
ℹ️ 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".
Dragging a group while a chat filter was active passed only the visible roots as the current order, so hidden groups were dropped from the remembered order and moved when the filter cleared. Use the unfiltered source-section group order and map end-of-visible-list drops to the next unfiltered group, mirroring the chat reorder path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b652a921b0
ℹ️ 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".
The final MCP result took the last message item regardless of role, so a trailing user prompt could replace the review findings; only agent messages qualify now, with the existing core.finalReview fallback covering transcripts without one. Item revisions also digested only the text length, hiding same-length edits from clients that compare log.revision; digest the content instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51b27e1065
ℹ️ 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".
| if let session = unregisterReviewEventSession(for: run) { | ||
| await session.abandon() | ||
| let metrics = await session.metricsSnapshot() | ||
| for threadID in await session.cleanupThreadIDs() { | ||
| for threadID in localCleanupThreadIDs(for: run, additional: await session.cleanupThreadIDs()) { |
There was a problem hiding this comment.
When a network recovery restart succeeds but the restarted review gets a different reviewThreadID than the interrupted attempt, this branch unregisters and abandons the old session without carrying its cleanupThreadIDs into any deferred cleanup. The normal terminal cleanupReview later runs only for the recovered attempt, so the interrupted review thread is never passed to cleanupAppServerReview/thread/delete and can remain as a stale chat until the app-server exits. Please store these cleanup IDs with the restart token or merge them into the recovered attempt’s cleanup path.
Useful? React with 👍 / 👎.
| return ReviewMonitorWindowController( | ||
| appStore: dependencies.store, | ||
| codexModelSource: codexModelSource, | ||
| showSettings: showSettings |
There was a problem hiding this comment.
In the non-preview launch path this calls ReviewMonitorWindowController with an appStore: label, but the class only defines overloads taking store: (including store:codexModelSource:showSettings:). As written, the ReviewMonitor app target cannot compile once this branch is selected; switch this call to the existing store: label.
Useful? React with 👍 / 👎.
| auth: auth, | ||
| activation: activation | ||
| ) | ||
| await closeIsolatedLoginRuntime(client: loginClient, codexHomeURL: loginCodexHomeURL) | ||
| await closeIsolatedLoginRuntime(appServer: loginAppServer, codexHomeURL: loginCodexHomeURL) | ||
| return |
There was a problem hiding this comment.
When a user has an existing account and chooses Add Account while the primary review runtime is stopped or failed, loginRuntime(for: .preserveActiveAccount) has already created a temporary app-server backend, but this guard still rejects the flow because self.appServerBackend is nil and reports “Review runtime is not running.” The account sidebar and Add Account control remain available in unavailable runtime states, so this blocks the isolated login path exactly when it should let users add or refresh accounts without starting the main runtime.
Useful? React with 👍 / 👎.
1 participant
Purpose
Rebuild ReviewMonitor's review data flow on CodexKit. The legacy in-repo app-server wire/domain stack (
CodexReviewDomain,CodexReviewApplication,CodexReviewAppServerWire,CodexReviewMCPAdapter) is replaced byCodexAppServerKit/CodexDataKitfrom lynnswap/CodexKit#16, with the UI driven by observable DataKit models.Changes
CodexReviewAppServerbackend drives reviews throughCodexAppServerKit(detached review sessions, thread event generations, model-actor-owned DataKit context). Runtime teardown now always releases the UI model source through one boundary (clearAppServerModelContainer).CodexDataKitfetched results and chat observations (ReviewMonitorCodexModelSource,ReviewMonitorCodexChatLogTarget). Chat log re-observation is serialized across selection changes and observation errors are surfaced instead of swallowed.ReviewChatLogUIrenderer/projection pipeline (revision-gated incremental updates, per-chat scroll restoration, command output panels with live duration).review_readrequiresrunIdin its schema to match the handler, and detailed reads return a bounded 100-item tail page with honest paging metadata.CodexKitis pinned to the reviewed main revision6de9e01(no tagged releases yet), replacing the temporary local path reference.ReviewUITests/CodexReviewMCPServerTestsadditions; command-output-panel block id helpers now include the item kind segment (<turnID>:<kind>:<itemID>), fixing five stale click-by-block-id expectations.Validation
swift test --build-system swiftbuild --no-parallel— 378 tests, all passingxcodebuild test -project Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj -scheme CodexReviewMonitor -destination 'platform=macOS,arch=arm64'— 15 tests, all passingmain(3156c4c): three rounds; final round reports no findings (rounds 1–2 findings fixed ind61a345and8d47d91)🤖 Generated with Claude Code