Repository navigation
feat(wework): mirror codex session view - #1568
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughTranscript parsing now passes device ids through runtime handlers, normalizes file-change and assistant payloads, and feeds Codex references, citations, and context events into the chat UI. Markdown, code, tool, and file-change renderers were updated with matching tests. ChangesCodex transcript and chat surface
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)wework/src/components/chat/blocks/toolBlockActivity.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.26][ERROR]: unable to find a config; path wework/src/components/chat/codexReferences.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.24][ERROR]: unable to find a config; path wework/src/features/workbench/WorkbenchProvider.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.17][ERROR]: unable to find a config; path
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
icycrystal4
force-pushed
the
codex/wework-codex-session-ui
branch
from
June 26, 2026 11:14
4cba722 to
9bac14b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
wework/src/components/chat/FileChangesCard.tsx (1)
233-235: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
basenamehelper.This is identical to the exported
basenameincodexReferences.ts(line 105). Consider importing the shared helper to avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/chat/FileChangesCard.tsx` around lines 233 - 235, The local basename helper in FileChangesCard duplicates the shared basename implementation from codexReferences, so replace the inline helper with the exported shared one. Update FileChangesCard to import basename from codexReferences and remove the duplicate function, keeping all call sites in the component unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/src/runtime_work/handler.rs`:
- Around line 218-226: The transcript normalization path is still using only
thread.cwd for file-change workspace resolution, while the surrounding logic
already computes a workspace fallback from the request payload/local link.
Update the message-building flow around transcript_messages in handler.rs so the
computed workspace_path is passed into transcript_messages (and used for
file-change normalization) instead of relying solely on thread.cwd, ensuring
fileChanges and workspacePath stay consistent when cwd is missing.
In `@executor/src/runtime_work/transcript.rs`:
- Around line 485-489: The tool-input parsing in tool_input currently drops
object-valued functioncall arguments by falling back to {"arguments": ""} when
parse_json_object_string fails. Update tool_input (and the
parse_json_object_string path it uses) to detect when item["arguments"] is
already a JSON object and return it unchanged instead of converting it to an
empty string, preserving the original tool input for functioncall records.
- Around line 345-399: The synthesized tool blocks are generating inconsistent
IDs because command_block, tool_block, and merge_tool_output call
item_id/tool_call_id more than once, and the fallback to now_ms() can differ per
call. Compute the tool identifier once per block (or per item) and reuse that
same value for both id and tool_use_id, including the fallback path, so
downstream matching in command_block, tool_block, and merge_tool_output stays
stable.
- Around line 626-631: When merging repeated file paths in the transcript
runtime, the existing entry replacement in the file-merge logic drops earlier
additions/deletions for the same path. Update the merge behavior in the code
that handles next_path / next_file so repeated paths accumulate their stats into
the existing file entry instead of overwriting it, ensuring multi-edit turns
keep correct summary counts while still preserving the combined diff patches.
In `@wework/src/components/chat/MarkdownCodeBlock.tsx`:
- Line 106: The wrap and copy control labels in MarkdownCodeBlock are hardcoded
Chinese strings, so localize them through the existing useTranslation('chat')
flow instead of inline text. Update wrapButtonLabel and the copy button
aria-label/title to call t(...) with new chat.json keys, and add matching
entries for both en and zh-CN so the UI and accessible names follow the current
locale.
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 108-114: shouldRenderMessage in MessageList.tsx is gating on raw
array lengths for references and memoryCitations, which can still produce empty
assistant bubbles after downstream filtering. Update the condition to match the
actual renderer inputs by checking the filtered results used by
getAssistantReferences and CodexMemoryCitations, while keeping the
contextEvents/context_compaction check as-is. Use the existing
shouldRenderMessage, getAssistantReferences, and CodexMemoryCitations logic to
ensure only renderable metadata makes the bubble visible.
In `@wework/src/i18n/locales/en/chat.json`:
- Line 27: The English locale entry for the memory citations summary in the chat
translation file uses a single `summary` key, which causes incorrect grammar for
a count of 1. Update the i18n entries to use i18next pluralized keys such as
`summary_one` and `summary_other` for the chat locale, and keep the existing
Chinese locale unchanged. Locate the change in the `chat.json` translation
resource where the current `summary` string is defined.
---
Nitpick comments:
In `@wework/src/components/chat/FileChangesCard.tsx`:
- Around line 233-235: The local basename helper in FileChangesCard duplicates
the shared basename implementation from codexReferences, so replace the inline
helper with the exported shared one. Update FileChangesCard to import basename
from codexReferences and remove the duplicate function, keeping all call sites
in the component unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe9b2f96-c6ff-4e68-8790-9919dc3d9daf
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
executor/src/runtime_work/handler.rsexecutor/src/runtime_work/transcript.rsexecutor/src/runtime_work/util.rsexecutor/tests/app_runtime_work_file_changes_contract.rswework/package.jsonwework/src/api/local/localServices.tswework/src/components/chat/AssistantMarkdown.tsxwework/src/components/chat/CodexTurnArtifacts.tsxwework/src/components/chat/FileChangesCard.test.tsxwework/src/components/chat/FileChangesCard.tsxwework/src/components/chat/MarkdownCodeBlock.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/assistantMarkdownLinks.tswework/src/components/chat/blocks/ToolBlockItem.test.tsxwework/src/components/chat/blocks/ToolBlockItem.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.test.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/blocks/toolBlockActivity.test.tswework/src/components/chat/blocks/toolBlockActivity.tswework/src/components/chat/codexReferences.tswework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/types/api.tswework/src/types/workbench.ts
Comment on lines
218
to
+226
| let messages = local_link | ||
| .as_ref() | ||
| .map(|link| merge_cached_messages(transcript_messages(thread), cached_messages(link))) | ||
| .unwrap_or_else(|| transcript_messages(thread)); | ||
| .map(|link| { | ||
| merge_cached_messages( | ||
| transcript_messages(thread, &self.device_id), | ||
| cached_messages(link), | ||
| ) | ||
| }) | ||
| .unwrap_or_else(|| transcript_messages(thread, &self.device_id)); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Pass the computed workspace fallback into transcript normalization.
workspace_path falls back to the request payload here, but transcript_messages still derives file-change workspace_path only from thread.cwd. If Codex omits cwd, the response has the right workspacePath while per-turn fileChanges get an empty workspace and unrelativized paths.
Proposed fix
- transcript_messages(thread, &self.device_id),
+ transcript_messages(thread, &self.device_id, &workspace_path),
cached_messages(link),
)
})
- .unwrap_or_else(|| transcript_messages(thread, &self.device_id));
+ .unwrap_or_else(|| transcript_messages(thread, &self.device_id, &workspace_path));-pub(crate) fn transcript_messages(thread: &Value, device_id: &str) -> Vec<Value> {
+pub(crate) fn transcript_messages(
+ thread: &Value,
+ device_id: &str,
+ fallback_workspace_path: &str,
+) -> Vec<Value> {
let mut messages = Vec::new();
- let workspace_path = string_field(thread, "cwd").unwrap_or_default();
+ let workspace_path =
+ string_field(thread, "cwd").unwrap_or_else(|| fallback_workspace_path.to_owned());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let messages = local_link | |
| .as_ref() | |
| .map(|link| merge_cached_messages(transcript_messages(thread), cached_messages(link))) | |
| .unwrap_or_else(|| transcript_messages(thread)); | |
| .map(|link| { | |
| merge_cached_messages( | |
| transcript_messages(thread, &self.device_id), | |
| cached_messages(link), | |
| ) | |
| }) | |
| .unwrap_or_else(|| transcript_messages(thread, &self.device_id)); | |
| let messages = local_link | |
| .as_ref() | |
| .map(|link| { | |
| merge_cached_messages( | |
| transcript_messages(thread, &self.device_id, &workspace_path), | |
| cached_messages(link), | |
| ) | |
| }) | |
| .unwrap_or_else(|| transcript_messages(thread, &self.device_id, &workspace_path)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/handler.rs` around lines 218 - 226, The transcript
normalization path is still using only thread.cwd for file-change workspace
resolution, while the surrounding logic already computes a workspace fallback
from the request payload/local link. Update the message-building flow around
transcript_messages in handler.rs so the computed workspace_path is passed into
transcript_messages (and used for file-change normalization) instead of relying
solely on thread.cwd, ensuring fileChanges and workspacePath stay consistent
when cwd is missing.
Comment on lines
+485
to
+489
| fn tool_input(item: &Value) -> Value { | ||
| match item_type(item).as_str() { | ||
| "functioncall" => parse_json_object_string(item, "arguments").unwrap_or_else( | ||
| || json!({"arguments": raw_string_field(item, "arguments").unwrap_or_default()}), | ||
| ), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve object-valued function arguments.
If arguments is already a JSON object, this currently falls through to {"arguments": ""} and loses the tool input.
Proposed fix
"functioncall" => parse_json_object_string(item, "arguments").unwrap_or_else(
- || json!({"arguments": raw_string_field(item, "arguments").unwrap_or_default()}),
- ),
+ || {
+ item.get("arguments")
+ .filter(|value| value.is_object())
+ .cloned()
+ .unwrap_or_else(|| {
+ json!({"arguments": raw_string_field(item, "arguments").unwrap_or_default()})
+ })
+ },
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn tool_input(item: &Value) -> Value { | |
| match item_type(item).as_str() { | |
| "functioncall" => parse_json_object_string(item, "arguments").unwrap_or_else( | |
| || json!({"arguments": raw_string_field(item, "arguments").unwrap_or_default()}), | |
| ), | |
| fn tool_input(item: &Value) -> Value { | |
| match item_type(item).as_str() { | |
| "functioncall" => parse_json_object_string(item, "arguments").unwrap_or_else( | |
| || { | |
| item.get("arguments") | |
| .filter(|value| value.is_object()) | |
| .cloned() | |
| .unwrap_or_else(|| { | |
| json!({"arguments": raw_string_field(item, "arguments").unwrap_or_default()}) | |
| }) | |
| }, | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/transcript.rs` around lines 485 - 489, The
tool-input parsing in tool_input currently drops object-valued functioncall
arguments by falling back to {"arguments": ""} when parse_json_object_string
fails. Update tool_input (and the parse_json_object_string path it uses) to
detect when item["arguments"] is already a JSON object and return it unchanged
instead of converting it to an empty string, preserving the original tool input
for functioncall records.
| const effectiveWrapLines = | ||
| canToggleWrap && (wrapState.key === wrapStateKey ? wrapState.value : storedWrapLines) | ||
| const displayLanguage = formatDisplayLanguage(language) | ||
| const wrapButtonLabel = effectiveWrapLines ? '禁用自动换行' : '开启自动换行' |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the hardcoded Chinese UI strings.
wrapButtonLabel (Line 106) and the copy button's aria-label/title (Lines 164-165) are hardcoded Chinese, but the chat surface is otherwise localized via useTranslation('chat') with en/zh-CN chat.json resources. English-locale users will see Chinese labels (including the accessible name for the copy/wrap controls). Route these through t(...) with new chat.json keys.
Also applies to: 164-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/MarkdownCodeBlock.tsx` at line 106, The wrap and
copy control labels in MarkdownCodeBlock are hardcoded Chinese strings, so
localize them through the existing useTranslation('chat') flow instead of inline
text. Update wrapButtonLabel and the copy button aria-label/title to call t(...)
with new chat.json keys, and add matching entries for both en and zh-CN so the
UI and accessible names follow the current locale.
Comment on lines
+108
to
+114
| if ( | ||
| message.references?.length || | ||
| message.memoryCitations?.length || | ||
| message.contextEvents?.length | ||
| ) { | ||
| return true | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect runtime normalization of references/contextEvents/memoryCitations
rg -nP -C3 'context_compaction|memoryCitations|memory_citations|references' executor/src/runtime_work/transcript.rs | head -80Repository: wecode-ai/Wegent
Length of output: 3064
🏁 Script executed:
#!/bin/bash
# Inspect references logic in transcript.rs
rg -P -B 10 -A 10 'references.*push|push.*references' executor/src/runtime_work/transcript.rs | head -60Repository: wecode-ai/Wegent
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Inspect memory_citation function logic
rg -P -B 5 -A 15 'fn memory_citation' executor/src/runtime_work/transcript.rs | head -50Repository: wecode-ai/Wegent
Length of output: 417
🏁 Script executed:
#!/bin/bash
# Search for references field assignment in transcript.rs
rg -P -B 5 -A 5 'references' executor/src/runtime_work/transcript.rs | head -80Repository: wecode-ai/Wegent
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Check frontend reference filtering logic
rg -P -B 3 -A 8 'getAssistantReferences|isCodexDocumentReferencePath' wework/src/components/chat/MessageList.tsxRepository: wecode-ai/Wegent
Length of output: 1188
shouldRenderMessage may render empty assistant bubbles for unfiltered metadata.
The gate checks raw array lengths, but downstream renderers filter out non-renderable items:
memory_citationinexecutor/src/runtime_work/transcript.rsonly validates object existence, allowing emptyentriesor non-renderable content to pushmemoryCitations.length > 0whileCodexMemoryCitationsrenders nothing.referencesare passed raw togetAssistantReferences, which filters to document paths only. Non-document references will pass the gate but render as empty.
Refactor the condition to check the length of the filtered values used for rendering:
if (
- message.references?.length ||
- message.memoryCitations?.length ||
- message.contextEvents?.length
+ getAssistantReferences(message.references, visibleContent)?.length ||
+ message.contextEvents?.length ||
+ message.memoryCitations?.some(entry => /* check for non-empty entries */)
) {
return true
}Since context_events are consistently context_compaction, that check is safe. The memoryCitations and references checks must align with their renderer logic to avoid empty UI states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/MessageList.tsx` around lines 108 - 114,
shouldRenderMessage in MessageList.tsx is gating on raw array lengths for
references and memoryCitations, which can still produce empty assistant bubbles
after downstream filtering. Update the condition to match the actual renderer
inputs by checking the filtered results used by getAssistantReferences and
CodexMemoryCitations, while keeping the contextEvents/context_compaction check
as-is. Use the existing shouldRenderMessage, getAssistantReferences, and
CodexMemoryCitations logic to ensure only renderable metadata makes the bubble
visible.
| "kind_with_extension": "Document · {{extension}}" | ||
| }, | ||
| "memory_citations": { | ||
| "summary": "{{count}} memory citations", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
English plural not handled for memory citations summary.
With {{count}} and a single key, a count of 1 renders the grammatically incorrect "1 memory citations". i18next supports plural keys (summary_one / summary_other) for the English locale. Chinese needs no change.
✏️ Suggested plural keys
- "memory_citations": {
- "summary": "{{count}} memory citations",
+ "memory_citations": {
+ "summary_one": "{{count}} memory citation",
+ "summary_other": "{{count}} memory citations",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "summary": "{{count}} memory citations", | |
| "summary_one": "{{count}} memory citation", | |
| "summary_other": "{{count}} memory citations", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/i18n/locales/en/chat.json` at line 27, The English locale entry
for the memory citations summary in the chat translation file uses a single
`summary` key, which causes incorrect grammar for a count of 1. Update the i18n
entries to use i18next pluralized keys such as `summary_one` and `summary_other`
for the chat locale, and keep the existing Chinese locale unchanged. Locate the
change in the `chat.json` translation resource where the current `summary`
string is defined.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
wework/src/components/chat/AssistantMarkdown.tsx (2)
27-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized component functions.
AssistantMarkdownand especiallyAssistantMarkdownImageexceed the preferred 50-line function limit. Extracting the markdown component map and authenticated-image loading into smaller helpers/hooks would keep the renderers focused.As per coding guidelines, "Functions should not exceed 50 lines of code (preferred maximum)".
Also applies to: 158-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/chat/AssistantMarkdown.tsx` around lines 27 - 81, Split the oversized renderers into smaller units: AssistantMarkdown should delegate its ReactMarkdown components map to a separate helper/function, and AssistantMarkdownImage should move its authenticated image loading/state logic into a dedicated hook or helper. Keep the JSX renderers focused on composition, using the existing AssistantMarkdown, AssistantMarkdownImage, and AssistantMarkdownLink symbols to refactor without changing behavior, and ensure no single function exceeds the preferred 50-line limit.Source: Coding guidelines
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this component file to kebab-case.
AssistantMarkdown.tsxviolates the component file naming convention; preferassistant-markdown.tsxwhile keeping the exported componentAssistantMarkdown.As per coding guidelines, "Component files must use kebab-case naming, but component exports must use PascalCase".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/chat/AssistantMarkdown.tsx` at line 1, The component file naming convention is violated by AssistantMarkdown.tsx; rename the file to kebab-case (assistant-markdown.tsx) while keeping the exported component name AssistantMarkdown unchanged. Update any imports or references that point to AssistantMarkdown.tsx so they resolve to the renamed file, and preserve the PascalCase export for the component itself.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/chat/AssistantMarkdown.tsx`:
- Around line 120-125: The file-link button in AssistantMarkdown is rendered
even when onOpenFile is missing, which leaves an interactive control that does
nothing. Update the AssistantMarkdown component so the file action is only
rendered as a button when onOpenFile is provided, or otherwise render a
disabled/non-interactive file label; use the AssistantMarkdown file-link button
and onOpenFile handler to locate the conditional rendering logic.
- Line 42: The language extraction in AssistantMarkdown’s code-block rendering
is too restrictive and truncates fenced identifiers that contain non-word
characters. Update the className parsing logic in the markdown renderer so it
preserves the full language label before passing it to MarkdownCodeBlock, and
verify the handling in the code path that currently uses the language-(\w*)
match. Ensure languages like c++, objective-c, and shell-session are passed
through intact.
- Around line 173-210: Clear the stale authenticated-image failure state in
AssistantMarkdown when a retry succeeds or when a new authenticated load starts,
because hasError is derived from failedSrc === rawSrc and can keep showing the
error branch after a later success. Update the loadAuthenticatedImage flow
inside the useEffect so failedSrc is reset for the current rawSrc before/after a
successful fetch, and ensure the cleanup/state handling around
setAuthenticatedPreview and setFailedSrc keeps the error state aligned with the
latest source.
---
Nitpick comments:
In `@wework/src/components/chat/AssistantMarkdown.tsx`:
- Around line 27-81: Split the oversized renderers into smaller units:
AssistantMarkdown should delegate its ReactMarkdown components map to a separate
helper/function, and AssistantMarkdownImage should move its authenticated image
loading/state logic into a dedicated hook or helper. Keep the JSX renderers
focused on composition, using the existing AssistantMarkdown,
AssistantMarkdownImage, and AssistantMarkdownLink symbols to refactor without
changing behavior, and ensure no single function exceeds the preferred 50-line
limit.
- Line 1: The component file naming convention is violated by
AssistantMarkdown.tsx; rename the file to kebab-case (assistant-markdown.tsx)
while keeping the exported component name AssistantMarkdown unchanged. Update
any imports or references that point to AssistantMarkdown.tsx so they resolve to
the renamed file, and preserve the PascalCase export for the component itself.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58cd2999-ae41-424f-acd1-f9096067a1a6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
executor/src/runtime_work/handler.rsexecutor/src/runtime_work/transcript.rsexecutor/src/runtime_work/util.rsexecutor/tests/app_runtime_work_file_changes_contract.rswework/package.jsonwework/src/api/local/localServices.tswework/src/components/chat/AssistantMarkdown.tsxwework/src/components/chat/CodexTurnArtifacts.tsxwework/src/components/chat/FileChangesCard.test.tsxwework/src/components/chat/FileChangesCard.tsxwework/src/components/chat/MarkdownCodeBlock.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/assistantMarkdownLinks.tswework/src/components/chat/blocks/ToolBlockItem.test.tsxwework/src/components/chat/blocks/ToolBlockItem.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.test.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/blocks/toolBlockActivity.test.tswework/src/components/chat/blocks/toolBlockActivity.tswework/src/components/chat/codexReferences.tswework/src/features/workbench/WorkbenchProvider.tsxwework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/types/api.tswework/src/types/workbench.ts
✅ Files skipped from review due to trivial changes (1)
- wework/src/i18n/locales/zh-CN/chat.json
🚧 Files skipped from review as they are similar to previous changes (24)
- wework/src/components/chat/blocks/toolBlockActivity.test.ts
- wework/package.json
- executor/src/runtime_work/handler.rs
- wework/src/i18n/locales/en/chat.json
- wework/src/types/api.ts
- wework/src/components/chat/blocks/toolBlockActivity.ts
- wework/src/components/chat/blocks/ToolBlockItem.test.tsx
- wework/src/components/chat/assistantMarkdownLinks.ts
- executor/src/runtime_work/util.rs
- wework/src/components/chat/MarkdownCodeBlock.tsx
- wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- wework/src/components/chat/CodexTurnArtifacts.tsx
- wework/src/api/local/localServices.ts
- wework/src/components/chat/blocks/ToolBlockItem.tsx
- wework/src/features/workbench/WorkbenchProvider.tsx
- wework/src/components/chat/codexReferences.ts
- wework/src/types/workbench.ts
- wework/src/components/chat/MessageList.test.tsx
- wework/src/components/chat/FileChangesCard.tsx
- executor/tests/app_runtime_work_file_changes_contract.rs
- executor/src/runtime_work/transcript.rs
- wework/src/components/chat/MessageList.tsx
- wework/src/components/chat/FileChangesCard.test.tsx
| li: ({ children }) => <li className="min-w-0 break-words pl-1 leading-6">{children}</li>, | ||
| strong: ({ children }) => <strong className="font-semibold">{children}</strong>, | ||
| code: ({ className, children }) => { | ||
| const match = /language-(\w*)/.exec(className || '') |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve full fenced language identifiers.
/language-(\w*)/ truncates languages containing non-word characters, such as c++, objective-c, or shell-session, before passing them to MarkdownCodeBlock.
Proposed fix
- const match = /language-(\w*)/.exec(className || '')
+ const match = /language-([^\s]+)/.exec(className || '')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const match = /language-(\w*)/.exec(className || '') | |
| const match = /language-([^\s]+)/.exec(className || '') |
🧰 Tools
🪛 OpenGrep (1.23.0)
[ERROR] 42-42: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/AssistantMarkdown.tsx` at line 42, The language
extraction in AssistantMarkdown’s code-block rendering is too restrictive and
truncates fenced identifiers that contain non-word characters. Update the
className parsing logic in the markdown renderer so it preserves the full
language label before passing it to MarkdownCodeBlock, and verify the handling
in the code path that currently uses the language-(\w*) match. Ensure languages
like c++, objective-c, and shell-session are passed through intact.
Comment on lines
+120
to
+125
| <button | ||
| type="button" | ||
| className={`${ASSISTANT_MARKDOWN_LINK_CLASS} group/file-link relative`} | ||
| data-testid="assistant-markdown-link" | ||
| onClick={() => onOpenFile?.(filePath)} | ||
| aria-label={tooltip} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid rendering a no-op file button.
onOpenFile is optional, but the file-link button remains clickable and silently does nothing when no handler is provided. Either require the handler or render a disabled/non-interactive file label in that case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/AssistantMarkdown.tsx` around lines 120 - 125, The
file-link button in AssistantMarkdown is rendered even when onOpenFile is
missing, which leaves an interactive control that does nothing. Update the
AssistantMarkdown component so the file action is only rendered as a button when
onOpenFile is provided, or otherwise render a disabled/non-interactive file
label; use the AssistantMarkdown file-link button and onOpenFile handler to
locate the conditional rendering logic.
Comment on lines
+173
to
+210
| const hasError = failedSrc === rawSrc | ||
|
|
||
| useEffect(() => { | ||
| let objectUrl: string | null = null | ||
| let isMounted = true | ||
|
|
||
| if (!rawSrc || !isAuthenticatedSrc) { | ||
| return () => { | ||
| isMounted = false | ||
| } | ||
| } | ||
|
|
||
| async function loadAuthenticatedImage() { | ||
| try { | ||
| const token = localStorage.getItem('auth_token') | ||
| const response = await fetch(getAuthenticatedImageFetchUrl(rawSrc), { | ||
| headers: token ? { Authorization: `Bearer ${token}` } : undefined, | ||
| }) | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to load markdown image: ${response.status}`) | ||
| } | ||
|
|
||
| const blob = await response.blob() | ||
| if (!blob.type.startsWith('image/')) { | ||
| throw new Error(`Markdown image response is not an image: ${blob.type || 'unknown'}`) | ||
| } | ||
|
|
||
| objectUrl = URL.createObjectURL(blob) | ||
| if (isMounted) { | ||
| setAuthenticatedPreview({ rawSrc, url: objectUrl }) | ||
| } else { | ||
| URL.revokeObjectURL(objectUrl) | ||
| } | ||
| } catch { | ||
| if (isMounted) { | ||
| setFailedSrc(rawSrc) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear stale authenticated-image errors on retry or success.
Once failedSrc equals rawSrc, a later successful load for the same source can still render the error branch because failedSrc is never cleared.
Proposed fix
if (!rawSrc || !isAuthenticatedSrc) {
return () => {
isMounted = false
}
}
+ setFailedSrc((current) => (current === rawSrc ? null : current))
+
async function loadAuthenticatedImage() {
try {
@@
objectUrl = URL.createObjectURL(blob)
if (isMounted) {
+ setFailedSrc((current) => (current === rawSrc ? null : current))
setAuthenticatedPreview({ rawSrc, url: objectUrl })
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hasError = failedSrc === rawSrc | |
| useEffect(() => { | |
| let objectUrl: string | null = null | |
| let isMounted = true | |
| if (!rawSrc || !isAuthenticatedSrc) { | |
| return () => { | |
| isMounted = false | |
| } | |
| } | |
| async function loadAuthenticatedImage() { | |
| try { | |
| const token = localStorage.getItem('auth_token') | |
| const response = await fetch(getAuthenticatedImageFetchUrl(rawSrc), { | |
| headers: token ? { Authorization: `Bearer ${token}` } : undefined, | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Failed to load markdown image: ${response.status}`) | |
| } | |
| const blob = await response.blob() | |
| if (!blob.type.startsWith('image/')) { | |
| throw new Error(`Markdown image response is not an image: ${blob.type || 'unknown'}`) | |
| } | |
| objectUrl = URL.createObjectURL(blob) | |
| if (isMounted) { | |
| setAuthenticatedPreview({ rawSrc, url: objectUrl }) | |
| } else { | |
| URL.revokeObjectURL(objectUrl) | |
| } | |
| } catch { | |
| if (isMounted) { | |
| setFailedSrc(rawSrc) | |
| } | |
| const hasError = failedSrc === rawSrc | |
| useEffect(() => { | |
| let objectUrl: string | null = null | |
| let isMounted = true | |
| if (!rawSrc || !isAuthenticatedSrc) { | |
| return () => { | |
| isMounted = false | |
| } | |
| } | |
| setFailedSrc((current) => (current === rawSrc ? null : current)) | |
| async function loadAuthenticatedImage() { | |
| try { | |
| const token = localStorage.getItem('auth_token') | |
| const response = await fetch(getAuthenticatedImageFetchUrl(rawSrc), { | |
| headers: token ? { Authorization: `Bearer ${token}` } : undefined, | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Failed to load markdown image: ${response.status}`) | |
| } | |
| const blob = await response.blob() | |
| if (!blob.type.startsWith('image/')) { | |
| throw new Error(`Markdown image response is not an image: ${blob.type || 'unknown'}`) | |
| } | |
| objectUrl = URL.createObjectURL(blob) | |
| if (isMounted) { | |
| setFailedSrc((current) => (current === rawSrc ? null : current)) | |
| setAuthenticatedPreview({ rawSrc, url: objectUrl }) | |
| } else { | |
| URL.revokeObjectURL(objectUrl) | |
| } | |
| } catch { | |
| if (isMounted) { | |
| setFailedSrc(rawSrc) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/components/chat/AssistantMarkdown.tsx` around lines 173 - 210,
Clear the stale authenticated-image failure state in AssistantMarkdown when a
retry succeeds or when a new authenticated load starts, because hasError is
derived from failedSrc === rawSrc and can keep showing the error branch after a
later success. Update the loadAuthenticatedImage flow inside the useEffect so
failedSrc is reset for the current rawSrc before/after a successful fetch, and
ensure the cleanup/state handling around setAuthenticatedPreview and
setFailedSrc keeps the error state aligned with the latest source.
This was referenced Jun 26, 2026
Merged
This was referenced Jul 5, 2026
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.
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes