Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c96621852f
ℹ️ 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".
vercel-labs/fx#788 (merged to fx main) fixes the lossy ACP `session/load` that the acp notes tracked as finding #1 (fx#624): replay now emits a `tool_call` plus terminal `tool_call_update` per recorded call instead of folding the turn into one "Previous tool execution" assistant chunk. `scripts/probe-fx-acp.ts` drives `fx acp` through moi's real ACP client, session and adapter, runs one tool-calling turn, kills the process, and compares the cold-loaded view with the live one. Verified on 2026-09-10: a fx main build (6fdbe1f83029) replays the tool card identically with no adapter change; the 0.0.8 release (43c11dcc34a9) predates the fix and is still lossy. Both runs used fx's fake-gateway shape; the script uses the real Vercel AI Gateway when AI_GATEWAY_API_KEY is set. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNrsnBvXC2cHkFL73BSwN1
The #788 replay fix now has real Vercel AI Gateway evidence (anthropic/claude-sonnet-5) on both the 0.0.8 release and a main build, matching the fake-gateway runs. The other fx findings in the acp notes were re-checked on main over raw JSON-RPC: one active session per process, mode reset after load, generic titles, invalid model acceptance, the missing effort option, resume without replay, and replay without timestamps or usage all still hold; thought replay could not be triggered live and is ruled out by the replay planner's shape. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNrsnBvXC2cHkFL73BSwN1
Use isolated chat and discovery processes, preserve structured tool replay, validate session settings, and harden cancellation and transport ownership. Retain model-specific effort choices across cold and warm chats and hide discovery-owned empty sessions. Verified official fx dev f4ea28b23764 with real and fake gateway probes, 16 live lifecycle checks, browser workspace creation and cold replay, and the complete suite (1640 pass, 4 skip). Preserve the invoking Bun runtime so the dev workspace form uses the correct bundler.
Linux Bun 1.3.14 retains the fixture stdin pipe after closeSync(0), so the old test paused input instead of producing a broken pipe. Inject write and flush failures on a real child sink and assert both pending requests reject and the child terminates. Verified on the exact CI runtime and with the full local suite (1641 pass, 4 skip).
fx clips every ACP tool result to a 200-byte preview, live and on session/load. After a history load and after each prompt, moi now reads fx's supported `fx session --id <id> --json` history and replaces the previews with the saved results: full command output with its exit code, full tool text, and fx's line diff for committed writes and edits. Backgrounded shell commands (fx 0.0.11 yields long runs and streams late output onto the finished call) keep their successful outcome and show only real output; status lines are never concatenated with it. Provider failures, the response-restart marker and replayed interrupted outcomes become notices instead of assistant replies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx file reads now show highlighted source without fx's path/content wrapper and line gutter, writes show the written file, and edits render as a diff: fx's own committed diff when its history is available, else the replaced and new text while the run is live. Tool rows name what fx actually did (waiting on or stopping a backgrounded command, reading a skill, fetching a page, messaging a subagent) instead of fx's generic titles. The thinking probe accepts PROBE_EFFORT for every model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
ACP backends (fx, Hermes) hold a message sent mid-run until the current run ends. The chat inserted it into the transcript immediately, so the running reply landed after it and folded into the next run's work log. Such sends now wait in a dimmed "Queued" bubble after the live reply; the server's echo places the turn in order when it is dispatched. Codex, OpenClaw and Claude Code keep their current behavior. fx command rows also end with how the command finished (exit code and duration, stopped, or timed out), and tool rows expose aria-expanded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Until fx generates a title (at the end of the first run), its session list entry has none, and moi replaced the client's prompt-based title with a generic "Untitled session" for the whole first turn. A chat that is open in this server now uses its first user message instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx advertises a model's effort levels only for a session's current model, so selecting a model moi had not used hid the effort control until that model's first run. When the picker selects such a model, moi now switches a throwaway discovery session to it and caches the advertised options (`GET /agent?model=…`); the probe session is hidden like discovery's. Models known to have no effort report it, so they are never re-probed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx reports failed tools as `{"error":{…}}` JSON; rows now show the message
(with reviewer advice for a held action, or "Stopped before it finished."
for a cancelled call) and keep the envelope as raw output. A replayed
message with images no longer shows fx's `[Image #1]` marker lines. Tool
output wraps at word boundaries instead of splitting identifiers.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx streams command output as one update per line, and every broadcast carries the whole accumulated tool turn, so a 400-line `seq` sent 405 frames (about 1 MB) and longer output grew quadratically. Progress that keeps a tool in the same state now updates the view immediately but reaches clients at most every 120 ms; a tool's appearance and every state change are still sent at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The shell normalizer kept the accumulated output a second time in rawOutput, so every broadcast carried it twice more than needed. It now continues from the row's own text and recognizes its status lines by an exact pattern instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Sending scrolls to the bottom before the new message renders. The scroll event could arrive after the content had grown again, measure a large distance from the new bottom, and un-pin the chat, so the streaming reply stayed below the fold. Only a scroll toward older content un-pins now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The selected chat is shared by every open tab. A tab whose chat list did not yet include a chat just started elsewhere cleared the selection at once, and the clear reached the originating tab: its running chat vanished behind an empty "New chat". A missing selection is now cleared only when that chat is not running and stays unknown for three seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
A stopped run ended with no marker, so a red tool row was the only sign anything happened; after a reload fx's replay produced a notice instead. A cancelled prompt now adds the same "This run was stopped before it finished." notice live, so both views agree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
A cold load (after the idle release or an environment change) replays the chat with new turn ids: user turns get replay ids and assistant turns restart their counters, so broadcasting them patched an open client's transcript with duplicated and overwritten turns. Replayed turns now stay server-side, and one `session_reloaded` frame makes clients refetch the transcript; events that arrive during the refetch are applied on top. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The fx notes describe the restored tool results, backgrounded commands, operational notices, queued messages, first-message titles and the effort probe, and their remaining limits now reflect what is fixed. fx's vision tool (used for models without native vision) gets a label and its focus as the brief. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Idle chats free their fx process after ten minutes. Reopening one reloaded it from fx's replay, which has no thoughts and only clipped results, so a chat the user had just read lost them. fx chats now keep the transcript moi saw live (bounded, dropped when the chat is forgotten): it is served without starting a process, and the next load still attaches through session/load but keeps the retained transcript unless the history changed elsewhere (a different number of user turns). New runs continue after its turn ids. Replayed durations are applied before replay ends, so no replayed turn is broadcast, and clients drop stale previews when a chat reloads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
- Reuse one warm fx process for session/list instead of spawning per call. - Skip the title broadcast during a prompt; the turn end already sends one. - Drop no-op tool updates and the duplicate final-turn meta emit. - Label only the latest replayed run with the loaded model, never the model picked for the send that resumed the chat. - Derive totalTokens when fx omits it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Subagent reports and vision summaries render as markdown; fetched pages, skills, saved command output and file searches drop their model-facing wrappers. Shell status sentences and fx's diff line counts move to the row's summary line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx names each model by its raw id, so a few hundred 'vendor/model' rows sat in one section and the composer truncated them to the same prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The verdict now requires replayed tool names, inputs and message text to match the live transcript, real-gateway runs drop inherited fx gateway overrides, and the cold load waits for the killed process to exit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx saves at most 4,096 bytes of a tool result, so a long SKILL.md or page reached the chat without its closing tag and kept its envelope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
fx saves at most 4,096 bytes of a result, so a long command's saved envelope is cut mid-JSON. The post-run history read replaced the complete streamed output with that raw fragment. A cut envelope now yields its exit status and, only for a row with no output of its own, the decoded prefix. Also from review: - The enrichment hook sees the current rows and sets failure text explicitly, so a cold-loaded failed command shows its saved output. - Late output from a backgrounded command no longer splits the reply the model is writing. - A stop while a cold chat reads its history cancels the send. - A reconnect that fails or is stopped keeps the retained transcript. - A new ACP chat reports running while its agent starts, so other tabs keep a shared selection they do not list yet. - fx reads cut before their closing tag are unwrapped and marked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
From the re-verification runs: - A shell row keeps the output it streamed live; fx's saved copy (at most 4,096 bytes, stdout and stderr interleaved mid-line) fills only a row with none of its own. Restored rows get their duration, a command that printed nothing reads as no output, and a backgrounded command keeps its early output after a cold load. - Replayed notices name the turn they follow: replayed turns carry no time, so a stopped run's notice used to land at the end of the chat. - An environment change keeps idle fx chats' live transcripts. - The warm chat-list process stays for ten minutes, like an idle chat. - Vendor headings for spacexai and inclusionai; '1 match' for one result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The saved result of fx's shell interact call is the output the command printed while waited on, which the command's own row already shows live. It now fills the wait row only after a cold load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
The chat merges a run's assistant turns under the first turn's id, so a notice naming a later turn in the run found no match and fell to the end of the chat. Anchors now resolve through the grouping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Every harness hears an env change, and Hermes forgot and killed every ACP session in the workspace before fx's own hook ran. An fx chat therefore lost the transcript it was meant to keep across the restart, and its long shell output shrank to fx's saved 4,096 bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy
molefrog
force-pushed
the
claude/sweet-faraday-fy7od4
branch
from
September 25, 2026 12:26
e50d57d to
116af4c
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
moi now offers fx workspaces (fx 0.0.9 or later, verified on 0.0.11) with streaming chats, images, model selection and reasoning effort over ACP. ACP clips every fx tool result to 200 bytes, so moi restores command output, exit codes and fx's own line diffs from
fx session --id <id> --json, unwraps fx's model-facing envelopes, and turns fx's operational messages (provider failures, restarts, stops) into notices instead of replies.Review closely:
server/harness/acp/session.ts: replayed turns are no longer broadcast; a cold load sendssession_reloadedand clients refetch. fx chats released when idle or by an env change keep their live transcript and reuse it only when the replay has the same number of user turns. Hermes's env and stop hooks now act only on Hermes chats; they used to discard fx's.server/harness/fx/history.ts: fx saves at most 4,096 bytes of a result, so a long shell envelope arrives cut mid-JSON. Saved shell output only fills rows with no output of their own; a streamed row is never replaced.server/harness/acp/discovery.ts: fx's chat list uses one warm process per workspace (10-minute idle release) instead of spawning per refresh.Verification:
bun test: 1,923 pass, 0 fail; typecheck, lint (existing warnings only) and format pass.PROBE_EFFORT=high bun scripts/probe-fx-acp.ts --fake: replay matches the live transcript.🤖 Generated with Claude Code
https://claude.ai/code/session_019wu29L9PZkqMDtA224wbUy