Repository navigation
Codex/local capability store links - #1322
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR extends global capability synchronization from Skills to include Plugins and MCPs, gates standalone workspace creation on WeWork + project_id=0, refactors executor capability storage with managed runtime symlinks for Claude/Codex, normalizes MCP configs for SDK compatibility, consolidates frontend chat component contracts around project selection, and aligns all tests and documentation. The changes span backend indexing/payload resolution, executor global capability management, frontend navigation/routing, and chat UI simplification. ChangesBackend: Plugin payload naming and standalone workspace gating
Executor: Standalone workspace gating, capability store, and MCP handling
Frontend: Routing, navigation, and component contracts
Documentation and metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
README.md (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd required Markdown frontmatter (
sidebar_position) at the top.This
.mdfile is missing the required frontmatter block.Suggested patch
+--- +sidebar_position: 1 +--- + # WegentAs per coding guidelines, "
**/*.md: All documentation files MUST include frontmatter withsidebar_positionfor ordering".🤖 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 `@README.md` at line 1, The README.md is missing required YAML frontmatter; add a frontmatter block at the very top that includes a sidebar_position key (e.g., a numeric ordering value) so the file complies with the "*.md" guideline; ensure the block is valid YAML delimited by `---` lines and contains at minimum `sidebar_position: <number>` and optional descriptive keys like `title` to preserve rendering order.Source: Coding guidelines
README_zh.md (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win补充必需的 Markdown frontmatter（
sidebar_position）。该
.md文件顶部缺少必需的 frontmatter。Suggested patch
+--- +sidebar_position: 1 +--- + # WegentAs per coding guidelines, "
**/*.md: All documentation files MUST include frontmatter withsidebar_positionfor ordering".🤖 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 `@README_zh.md` at line 1, This README_zh.md is missing required Markdown frontmatter; add a YAML frontmatter block at the top of the file that includes at minimum the sidebar_position key (e.g., sidebar_position: <number>) and any other required frontmatter fields (title, description, etc.) so the document complies with the `**/*.md` guideline; update the top of README_zh.md (the section around the existing "# Wegent" header) to prepend the new frontmatter.Source: Coding guidelines
wework/src/features/workbench/WorkbenchProvider.test.tsx (1)
1-1992: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftThis test suite is too large and should be partitioned.
The file is 1992 lines, far above the project’s size cap. Please split into targeted suites (bootstrap/device selection, task/history behavior, guidance queueing, send payloads, archive flows) to improve readability and long-term upkeep.
As per coding guidelines: “File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules”.
🤖 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/features/workbench/WorkbenchProvider.test.tsx` around lines 1 - 1992, The test file is too large; split the single 1992-line WorkbenchProvider.test.tsx into multiple focused test files (e.g. bootstrap.test.tsx, devices.test.tsx, project_and_task_flow.test.tsx, guidance_queue.test.tsx, send_payloads_and_archives.test.tsx). Move related probe components and their tests together—keep WorkbenchProvider and useWorkbench imports in each new file and relocate Probe/DeviceListProbe/ProjectSelectionProbe/TaskSelectionProbe/GuidanceRaceProbe/ProjectChatProbe/ArchiveProbe/HistoryProbe/ToolBlockHistoryProbe/LockedProbe/AttachmentOnlyProbe/StandaloneDeviceProbe/FollowUpProbe into the files that test the behaviors they support, and update describe blocks accordingly; ensure shared test utilities (mock services setup) are extracted to a helper module if duplicated. Run tests to verify imports and mocks are updated and that each file stays under the 1000-line limit.Source: Coding guidelines
wework/src/components/chat/ChatInput.test.tsx (1)
1-1741: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this oversized test module into smaller suites.
This file is currently 1741 lines, which exceeds the repository limit and makes ownership/debugging harder. Please split it into focused sub-modules (for example: composer behavior, model menu, attachments, project-work interactions).
As per coding guidelines: “File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules”.
🤖 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/ChatInput.test.tsx` around lines 1 - 1741, This test file is too large—split the monolithic ChatInput.test.tsx into smaller focused test modules (e.g., ChatInput.composer.test.tsx, ChatInput.modelMenu.test.tsx, ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx). Extract and export shared helpers ControlledChatInput, projectChatControls, and projectWorkControls into a small test-utils module and import them from each new test file, then move the related test blocks (tests that reference chat-message-input, voice-input-button, expand-input-button into the composer file; tests referencing model-selector-button, model-option-*, model-control-* into the modelMenu file; attachment-related tests and fetch/createObjectURL stubs into attachments; project-work-button and device/project selection tests into projectWork), update imports in each new file and ensure afterEach cleanup (vi.restoreAllMocks, localStorage.clear, URL.createObjectURL restore) is preserved in each suite.Source: Coding guidelines
wework/src/components/layout/DesktopWorkbenchLayout.test.tsx (1)
1-1355: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftBreak this test file into smaller modules to meet repo limits.
At 1355 lines, this exceeds the configured max file size. Please split by domain (e.g., sidebar/settings, project creation, workspace panels, environment info) to keep tests maintainable and faster to navigate.
As per coding guidelines: “File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules”.
🤖 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/layout/DesktopWorkbenchLayout.test.tsx` around lines 1 - 1355, This test file exceeds the repo size limit; split it into smaller spec modules by domain—create separate test files that each import DesktopWorkbenchLayout and reuse shared test utilities: extract createRect, vi.mock setup, createDeviceApiMock/createQuotaApiMock, fetchQuotaMock, and baseProps into a shared test helper (e.g., a setup/testUtils module) and import it from each new spec; then move related tests into focused files (e.g., sidebar_and_settings tests containing tests referencing data-testid like 'projects-section-toggle', 'settings-button', 'settings-menu'; project_creation tests with 'projects-create-button', 'project-create-dialog', onCreateProject mocks; workspace_panels tests with 'toggle-right-workspace-panel-button', 'bottom-workspace-resize-handle'; environment_info tests with 'environment-info-button', onLoadEnvironmentInfo, onCommitEnvironmentChanges) and update imports to use the shared helpers and DesktopWorkbenchLayout so each file stays under the line limit and avoids duplicating mocks.Source: Coding guidelines
wework/src/components/chat/composer/ComposerTextarea.tsx (1)
289-305:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBug: Mention validation compares against wrong field, causing mentions to be lost.
The filter at line 303 compares
value.slice(mention.start, mention.end)againstmention.label(the display name like "My Skill"), but the actual text in the value ismention.reference(the markdown like[$my-skill](skill://...)). This comparison will never match, causing all skill mentions to be incorrectly filtered out after any text edit.Proposed fix
.filter((mention): mention is SkillMention => - Boolean(mention && nextValue.slice(mention.start, mention.end) === mention.label), + Boolean(mention && nextValue.slice(mention.start, mention.end) === mention.reference), ),🤖 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/composer/ComposerTextarea.tsx` around lines 289 - 305, The filter is validating mentions against the wrong field so mentions get dropped: in the setSelectedSkillMentions update (inside the map/filter), change the validation to compare nextValue.slice(mention.start, mention.end) === mention.reference (not mention.label) so the stored markdown reference matches the text buffer; keep the existing adjustments that shift start/end by delta and the null return for overlapping mentions, but use mention.reference in the Boolean check to preserve mentions after edits.
🧹 Nitpick comments (7)
wework/src/components/common/ActionMenu.tsx (1)
128-129: 💤 Low valueHardcoded dark colors break theme consistency.
The menu now uses hardcoded dark colors (
bg-[#2b2b2b],text-white,text-[#ff7b7b]) instead of theme tokens. This means the menu will always render in dark mode regardless of the app's current theme setting, which could cause visual inconsistency in light mode contexts.If this is intentional (e.g., the menu should always be dark), this is fine. Otherwise, consider keeping the theme tokens for consistency.
Also applies to: 145-145, 165-169
🤖 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/common/ActionMenu.tsx` around lines 128 - 129, The ActionMenu component contains hardcoded color classes ('bg-[`#2b2b2b`]', 'text-white', 'text-[`#ff7b7b`]') inside the menu button/menu item class strings which forces a dark appearance; update those class strings in ActionMenu to use the app's theme tokens or conditional theme-aware classes instead (e.g., replace the hex classes with the established token class names or add light/dark variants) so the menu respects the current theme; specifically locate and change the occurrences of 'bg-[`#2b2b2b`]', 'text-white', and 'text-[`#ff7b7b`]' in the ActionMenu component and use the project's theme token names or a theme prop/dark: variants to produce the correct colors in both light and dark modes.executor/modes/local/capabilities.py (6)
1046-1059: ⚡ Quick winConsider documenting backward compatibility behavior.
Lines 1046-1059 handle migration from pre-store installs by recording the runtime_link as the store_path. While functional, this creates records where
store_pathequalsruntime_link, which might confuse future maintainers. A comment explaining the migration scenario would improve clarity.🤖 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/modes/local/capabilities.py` around lines 1046 - 1059, Add an inline comment in the branch that handles legacy pre-store installs (the block guarded by checks on managed_record, _runtime_skill_package_exists(runtime_link), and not store_path.exists()) explaining that we intentionally set record["store_path"] = runtime_link to preserve backward compatibility for skills installed before the store existed; reference the symbols _runtime_skill_package_exists, _codex_skill_link, store._skill_record, and results so maintainers understand this migration behavior and that these records intentionally have identical store_path and runtime_link values to avoid future confusion.
1191-1314: ⚖️ Poor tradeoffFunction significantly exceeds length guideline.
The
_sync_pluginsmethod is ~124 lines, exceeding the 50-line guideline. Consider extracting helpers like_validate_plugin_entry,_download_plugin_if_needed, and_update_plugin_recordsto improve readability and testability.🤖 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/modes/local/capabilities.py` around lines 1191 - 1314, The _sync_plugins method is too large—extract logical sections into small helper methods to improve readability and testability: create a _validate_plugin_entry(item) to handle the dict/type checks and early-result entries, a _download_plugin_if_needed(client, item, store_path, expected_checksum, installed_map, key) to encapsulate download logic and return (store_path, result_failure_dict|None), and an _update_plugin_records(item, key, marketplace, store_path, runtime_link, codex_link, installed_map) to handle creating/updating installed_map entries, records via _plugin_record and appending to results; call existing helpers (_plugin_marketplace, _plugin_key, _plugin_runtime_path, _plugin_store_path, _persisted_plugin_checksum, _ensure_runtime_symlink, _ensure_codex_plugin_link) from these new functions and keep _sync_plugins as the orchestrator that iterates plugins, invokes the helpers, collects results/records, and calls self.store._save_installed_plugins when records exist.Source: Coding guidelines
354-448: ⚖️ Poor tradeoffConsider refactoring to reduce complexity.
The
reconcile_managed_pluginsmethod is ~95 lines, nearly double the 50-line guideline. While it's cohesive, consider extracting helpers like_restore_plugin_symlinksand_update_plugin_bookkeepingto improve readability.🤖 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/modes/local/capabilities.py` around lines 354 - 448, reconcile_managed_plugins is too long; extract the symlink restoration and bookkeeping into small helpers to reduce complexity: create a helper like _restore_plugin_symlinks(self, key, record, runtime, store_path) that performs the claude/codex Path resolution and calls _ensure_runtime_symlink (returns True if any symlink was restored), and a helper like _update_plugin_bookkeeping(self, key, record, claude_link, installed_map, settings) that computes entry via _plugin_install_entry, merges with existing entries, updates installed_map and enabled_plugins, and returns (installed_changed_delta, settings_changed_delta, restored_added_flag). Replace the inline symlink and entry/settings mutation blocks in reconcile_managed_plugins with calls to these two helpers, ensure you preserve semantics for restored list, installed_changed and settings_changed flags, and keep calls to _save_installed_plugins, _save_claude_settings and _sync_local_plugin_marketplace unchanged.Source: Coding guidelines
71-78: ⚡ Quick winConsider adding a docstring for clarity.
The
_peer_codex_capability_dirfunction derives Codex directories from Claude directories. The logic handles both~/.claude/skillsand other cases, but a docstring explaining the intended behavior would improve maintainability.📝 Suggested docstring
def _peer_codex_capability_dir(source_dir: Path, directory_name: str) -> Path: + """Derive Codex directory from Claude directory (e.g., ~/.claude/skills → ~/.codex/skills).""" root = (🤖 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/modes/local/capabilities.py` around lines 71 - 78, Add a clear docstring to the _peer_codex_capability_dir function that explains its purpose (derive the corresponding .codex capability directory for a given source_dir), describes the two cases it handles (when source_dir is inside a ".claude" tree it climbs two levels, otherwise one level), states the meaning of the directory_name parameter, and documents the return value (Path to root/.codex/directory_name). Reference the function name _peer_codex_capability_dir and the conditional check source_dir.parent.name == ".claude" in the docstring so future readers understand the branching logic.
983-1125: ⚖️ Poor tradeoffFunction significantly exceeds length guideline.
The
_sync_skillsmethod is ~143 lines, nearly 3x the 50-line guideline. Consider extracting helpers like_validate_skill_entry,_handle_existing_skill_package, and_download_and_link_skillto improve maintainability.🤖 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/modes/local/capabilities.py` around lines 983 - 1125, _sync_skills is too long; break it into smaller helpers: extract input validation into _validate_skill_entry(skill) which returns (name, skill_id, namespace) or an error dict used to append a failed result; extract the block that handles already-present/managed packages (the checks using manifest, _is_runtime_path_occupied_by_local_user, _runtime_skill_package_exists, store._skill_record and the early skips) into _handle_existing_skill_package(skill, runtime_link, store_path) that returns either (record, result_status) or None; and extract the download-and-link flow (calling _download_skill_to_store, _ensure_runtime_symlink, _ensure_codex_skill_link, computing before/after via _skill_digest_at, building the record with store._skill_record and appending results) into _download_and_link_skill(skill, name, skill_id, namespace, runtime_link, store_path). Keep _sync_skills orchestration logic: iterate skills, call _validate_skill_entry, check auth and dirs, call _handle_existing_skill_package, call _download_and_link_skill, and append results/records using the same keys and behaviors already in _sync_skills.Source: Coding guidelines
1-1633: ⚖️ Poor tradeoffFile exceeds the 1000-line guideline.
This file is 1633 lines, exceeding the guideline limit of 1000 lines. As per coding guidelines, consider splitting it into multiple sub-modules (e.g.,
store.py,reporter.py,sync_handler.py).🤖 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/modes/local/capabilities.py` around lines 1 - 1633, This file is too large; split it into focused submodules: move ManagedCapabilityManifest and GlobalCapabilityStore (and related helpers like _atomic_write_json, _file_lock, _set_owner_only, default_* and _safe_path_segment/_is_child/_is_runtime_child/_remove_runtime_entry) into store.py; move GlobalCapabilityReporter and its helpers (_scan_skills, _scan_plugins, _scan_mcps, _read_skill_metadata, _scan_plugin_skills, _managed_plugin_scan_path, _normalized_plugin_root, _is_macos_zip_metadata) into reporter.py; move CapabilitySyncHandler and its sync helpers (_sync_skills, _sync_plugins, _sync_mcps and download/extract helpers, _plugin_marketplace/_plugin_key/_plugin_runtime_path/_plugin_store_path/_persisted_plugin_checksum/_plugin_record/_server_summary/_is_runtime_path_occupied_by_local_user/_ensure_runtime_symlink/_ensure_optional_runtime_symlink/_skill_store_path/_skill_digest/_directory_digest/_skill_digest_at and skill download helpers) into sync_handler.py; keep top-level small shared utilities (utc_now_iso, MANIFEST_VERSION, DEFAULT_FULL_REPORT_INTERVAL_SECONDS) in a new utils.py or __init__.py and adjust imports accordingly; add an __init__.py that re-exports the public classes (ManagedCapabilityManifest, GlobalCapabilityStore, GlobalCapabilityReporter, CapabilitySyncHandler) so external imports remain stable and update internal imports to the new module paths.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 `@backend/tests/models/test_task_model_indexes.py`:
- Line 8: The test function test_task_resource_has_project_task_listing_index is
missing a return type hint; update its signature to add "-> None" (i.e., change
the def for test_task_resource_has_project_task_listing_index to include the
return type hint) so it conforms to the repository typing rule for all Python
code.
In `@executor/agents/claude_code/local_mode_strategy.py`:
- Around line 123-130: The current assignment to settings_config["hooks"]
replaces any user hooks; instead retrieve the existing hooks map
(settings_config.get("hooks", {})) and merge into it: get or create the
"PostToolUse" list, find an entry with matcher "Write|Edit" and append the new
{"type":"command","command": hook_command} into its "hooks" list (or if no such
matcher exists, append a new entry with matcher "Write|Edit" and the hooks
list). Update settings_config["hooks"] with the merged structure so other keys
(e.g., "PreToolUse" or other PostToolUse matchers) are preserved.
In `@executor/tests/agents/test_claude_code_config_security.py`:
- Around line 102-104: The tests (e.g., test_local_mode_writes_claude_json) lack
type hints on parameters and return types; update the function signatures to
annotate fixture parameters (e.g., task_data: dict,
agent_config_with_sensitive_data: dict, temp_workspace: Path or str,
monkeypatch: pytest.MonkeyPatch) and add -> None to the test functions, and
apply the same pattern to the other modified tests referenced (the tests near
the other modified blocks) so all test functions have typed parameters and a
return annotation of -> None.
In `@executor/tests/agents/test_codex_config_builder.py`:
- Around line 88-89: Add Python type hints to the new test functions: annotate
parameters (e.g., tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch or
pytest.MonkeyPatch) and add explicit return type -> None for each test function
such as test_build_codex_config_injects_global_mcp_overrides and the other tests
referenced around lines 142-144; update function signatures to include these
parameter types and a trailing "-> None" return annotation so all new tests
conform to the project's "Type hints are required for all Python code"
guideline.
In `@executor/tests/agents/test_mode_strategy.py`:
- Around line 109-111: Annotate the updated test function signatures (e.g.,
test_get_config_directory_uses_global_for_project_tasks) so fixture params are
typed and the function return is -> None; a minimal safe change is to import Any
from typing and change signatures like (self, strategy, temp_workspace,
tmp_path, monkeypatch) to (self, strategy: Any, temp_workspace: Any, tmp_path:
Any, monkeypatch: Any) -> None; apply the same pattern to the other updated test
functions called out in the review so all altered tests include fixture param
type hints and -> None.
In `@executor/tests/agents/test_skill_mcp_extraction.py`:
- Around line 168-170: The test function
test_global_streamable_http_mcp_is_normalized_for_claude_sdk should include
Python type hints: annotate the tmp_path parameter as pathlib.Path (or
pytest.TempPathFactory/Path depending on fixture type) and monkeypatch as
pytest.MonkeyPatch (or simply builtins.MonkeyPatch), and add a return type of ->
None; update the function signature to include these type annotations so it
complies with the project's "Type hints are required" guideline.
- Around line 172-174: The test relies on HOME-based defaults but can be
affected by an externally set WEGENT_EXECUTOR_HOME; update the test (e.g., in
test_skill_mcp_extraction) to explicitly isolate executor home by removing or
clearing WEGENT_EXECUTOR_HOME from the environment before setting HOME (use
monkeypatch.delenv("WEGENT_EXECUTOR_HOME", raising=False) or equivalent), then
proceed to create the temporary manifest_path and parent directories so manifest
resolution is deterministic.
In `@executor/tests/modes/local/test_global_capabilities.py`:
- Around line 229-231: The test function
test_reporter_scans_managed_plugin_store_when_claude_cache_path_is_missing needs
parameter and return type hints: add types for tmp_path and monkeypatch and
annotate return as -> None. Change the signature to accept tmp_path: Path (from
pathlib) and monkeypatch: pytest.MonkeyPatch (or MonkeyPatch from pytest) and
add/import those symbols if missing so the function reads like "def
test_reporter_scans_managed_plugin_store_when_claude_cache_path_is_missing(tmp_path:
Path, monkeypatch: pytest.MonkeyPatch) -> None:".
In `@executor/tests/test_local_capability_sync_handler.py`:
- Around line 176-178: The test signature for
test_apply_sync_redownloads_managed_skill_when_runtime_link_is_broken lacks type
hints; change its signature to include fixture types (e.g., tmp_path:
pathlib.Path, monkeypatch: pytest.MonkeyPatch) and an explicit return type ->
None. Update the function definition for
test_apply_sync_redownloads_managed_skill_when_runtime_link_is_broken and any
other newly introduced tests in this file to use explicit fixture type
annotations and -> None return (use pathlib.Path and pytest.MonkeyPatch where
appropriate).
In `@wework/src/components/chat/composer/CompactChatComposer.tsx`:
- Around line 154-163: The voice input button in CompactChatComposer renders
without interaction; update the button with a proper affordance: either add a
no-op onClick handler that opens the voice flow later (e.g.,
onClick={handleVoiceInputClick}) or mark it explicitly disabled and style it
accordingly so users know it's a placeholder (add disabled attribute and adjust
classNames), and reference the button by data-testid="voice-input-button" and
the hasText condition to control visibility; if you add an onClick, implement a
stub function handleVoiceInputClick inside CompactChatComposer to
dispatch/trigger the real voice input when implemented.
In `@wework/src/components/chat/composer/ComposerToolbar.tsx`:
- Around line 55-62: The voice input button in ComposerToolbar (data-testid
"voice-input-button", element rendering the Mic icon) lacks an onClick handler
or disabled state like in CompactChatComposer; update ComposerToolbar to either
add a proper onClick that triggers the voice input flow or set the button to
disabled/hidden until the feature is implemented (e.g., add disabled prop or
conditional render) and ensure aria-label and data-testid remain consistent for
tests; reference the button element and the Mic component when making the
change.
In `@wework/src/components/chat/MessageList.tsx`:
- Around line 354-359: The UI shows duplicate "正在思考" because MessageList.tsx
renders the same span for both isThinking and (isStreaming && hasContent);
update the rendering logic to avoid showing the thinking indicator after content
starts: either remove the second branch that checks (isStreaming && hasContent)
or replace it with a distinct streaming indicator (e.g., change the
text/className or render a different element) so only isThinking renders "正在思考"
and isStreaming renders a separate streaming state; adjust the conditional
around isStreaming/hasContent where the span is returned to reflect this change.
---
Outside diff comments:
In `@README_zh.md`:
- Line 1: This README_zh.md is missing required Markdown frontmatter; add a YAML
frontmatter block at the top of the file that includes at minimum the
sidebar_position key (e.g., sidebar_position: <number>) and any other required
frontmatter fields (title, description, etc.) so the document complies with the
`**/*.md` guideline; update the top of README_zh.md (the section around the
existing "# Wegent" header) to prepend the new frontmatter.
In `@README.md`:
- Line 1: The README.md is missing required YAML frontmatter; add a frontmatter
block at the very top that includes a sidebar_position key (e.g., a numeric
ordering value) so the file complies with the "*.md" guideline; ensure the block
is valid YAML delimited by `---` lines and contains at minimum
`sidebar_position: <number>` and optional descriptive keys like `title` to
preserve rendering order.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1-1741: This test file is too large—split the monolithic
ChatInput.test.tsx into smaller focused test modules (e.g.,
ChatInput.composer.test.tsx, ChatInput.modelMenu.test.tsx,
ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx). Extract and
export shared helpers ControlledChatInput, projectChatControls, and
projectWorkControls into a small test-utils module and import them from each new
test file, then move the related test blocks (tests that reference
chat-message-input, voice-input-button, expand-input-button into the composer
file; tests referencing model-selector-button, model-option-*, model-control-*
into the modelMenu file; attachment-related tests and fetch/createObjectURL
stubs into attachments; project-work-button and device/project selection tests
into projectWork), update imports in each new file and ensure afterEach cleanup
(vi.restoreAllMocks, localStorage.clear, URL.createObjectURL restore) is
preserved in each suite.
In `@wework/src/components/chat/composer/ComposerTextarea.tsx`:
- Around line 289-305: The filter is validating mentions against the wrong field
so mentions get dropped: in the setSelectedSkillMentions update (inside the
map/filter), change the validation to compare nextValue.slice(mention.start,
mention.end) === mention.reference (not mention.label) so the stored markdown
reference matches the text buffer; keep the existing adjustments that shift
start/end by delta and the null return for overlapping mentions, but use
mention.reference in the Boolean check to preserve mentions after edits.
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 1-1355: This test file exceeds the repo size limit; split it into
smaller spec modules by domain—create separate test files that each import
DesktopWorkbenchLayout and reuse shared test utilities: extract createRect,
vi.mock setup, createDeviceApiMock/createQuotaApiMock, fetchQuotaMock, and
baseProps into a shared test helper (e.g., a setup/testUtils module) and import
it from each new spec; then move related tests into focused files (e.g.,
sidebar_and_settings tests containing tests referencing data-testid like
'projects-section-toggle', 'settings-button', 'settings-menu'; project_creation
tests with 'projects-create-button', 'project-create-dialog', onCreateProject
mocks; workspace_panels tests with 'toggle-right-workspace-panel-button',
'bottom-workspace-resize-handle'; environment_info tests with
'environment-info-button', onLoadEnvironmentInfo, onCommitEnvironmentChanges)
and update imports to use the shared helpers and DesktopWorkbenchLayout so each
file stays under the line limit and avoids duplicating mocks.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 1-1992: The test file is too large; split the single 1992-line
WorkbenchProvider.test.tsx into multiple focused test files (e.g.
bootstrap.test.tsx, devices.test.tsx, project_and_task_flow.test.tsx,
guidance_queue.test.tsx, send_payloads_and_archives.test.tsx). Move related
probe components and their tests together—keep WorkbenchProvider and
useWorkbench imports in each new file and relocate
Probe/DeviceListProbe/ProjectSelectionProbe/TaskSelectionProbe/GuidanceRaceProbe/ProjectChatProbe/ArchiveProbe/HistoryProbe/ToolBlockHistoryProbe/LockedProbe/AttachmentOnlyProbe/StandaloneDeviceProbe/FollowUpProbe
into the files that test the behaviors they support, and update describe blocks
accordingly; ensure shared test utilities (mock services setup) are extracted to
a helper module if duplicated. Run tests to verify imports and mocks are updated
and that each file stays under the 1000-line limit.
---
Nitpick comments:
In `@executor/modes/local/capabilities.py`:
- Around line 1046-1059: Add an inline comment in the branch that handles legacy
pre-store installs (the block guarded by checks on managed_record,
_runtime_skill_package_exists(runtime_link), and not store_path.exists())
explaining that we intentionally set record["store_path"] = runtime_link to
preserve backward compatibility for skills installed before the store existed;
reference the symbols _runtime_skill_package_exists, _codex_skill_link,
store._skill_record, and results so maintainers understand this migration
behavior and that these records intentionally have identical store_path and
runtime_link values to avoid future confusion.
- Around line 1191-1314: The _sync_plugins method is too large—extract logical
sections into small helper methods to improve readability and testability:
create a _validate_plugin_entry(item) to handle the dict/type checks and
early-result entries, a _download_plugin_if_needed(client, item, store_path,
expected_checksum, installed_map, key) to encapsulate download logic and return
(store_path, result_failure_dict|None), and an _update_plugin_records(item, key,
marketplace, store_path, runtime_link, codex_link, installed_map) to handle
creating/updating installed_map entries, records via _plugin_record and
appending to results; call existing helpers (_plugin_marketplace, _plugin_key,
_plugin_runtime_path, _plugin_store_path, _persisted_plugin_checksum,
_ensure_runtime_symlink, _ensure_codex_plugin_link) from these new functions and
keep _sync_plugins as the orchestrator that iterates plugins, invokes the
helpers, collects results/records, and calls self.store._save_installed_plugins
when records exist.
- Around line 354-448: reconcile_managed_plugins is too long; extract the
symlink restoration and bookkeeping into small helpers to reduce complexity:
create a helper like _restore_plugin_symlinks(self, key, record, runtime,
store_path) that performs the claude/codex Path resolution and calls
_ensure_runtime_symlink (returns True if any symlink was restored), and a helper
like _update_plugin_bookkeeping(self, key, record, claude_link, installed_map,
settings) that computes entry via _plugin_install_entry, merges with existing
entries, updates installed_map and enabled_plugins, and returns
(installed_changed_delta, settings_changed_delta, restored_added_flag). Replace
the inline symlink and entry/settings mutation blocks in
reconcile_managed_plugins with calls to these two helpers, ensure you preserve
semantics for restored list, installed_changed and settings_changed flags, and
keep calls to _save_installed_plugins, _save_claude_settings and
_sync_local_plugin_marketplace unchanged.
- Around line 71-78: Add a clear docstring to the _peer_codex_capability_dir
function that explains its purpose (derive the corresponding .codex capability
directory for a given source_dir), describes the two cases it handles (when
source_dir is inside a ".claude" tree it climbs two levels, otherwise one
level), states the meaning of the directory_name parameter, and documents the
return value (Path to root/.codex/directory_name). Reference the function name
_peer_codex_capability_dir and the conditional check source_dir.parent.name ==
".claude" in the docstring so future readers understand the branching logic.
- Around line 983-1125: _sync_skills is too long; break it into smaller helpers:
extract input validation into _validate_skill_entry(skill) which returns (name,
skill_id, namespace) or an error dict used to append a failed result; extract
the block that handles already-present/managed packages (the checks using
manifest, _is_runtime_path_occupied_by_local_user,
_runtime_skill_package_exists, store._skill_record and the early skips) into
_handle_existing_skill_package(skill, runtime_link, store_path) that returns
either (record, result_status) or None; and extract the download-and-link flow
(calling _download_skill_to_store, _ensure_runtime_symlink,
_ensure_codex_skill_link, computing before/after via _skill_digest_at, building
the record with store._skill_record and appending results) into
_download_and_link_skill(skill, name, skill_id, namespace, runtime_link,
store_path). Keep _sync_skills orchestration logic: iterate skills, call
_validate_skill_entry, check auth and dirs, call _handle_existing_skill_package,
call _download_and_link_skill, and append results/records using the same keys
and behaviors already in _sync_skills.
- Around line 1-1633: This file is too large; split it into focused submodules:
move ManagedCapabilityManifest and GlobalCapabilityStore (and related helpers
like _atomic_write_json, _file_lock, _set_owner_only, default_* and
_safe_path_segment/_is_child/_is_runtime_child/_remove_runtime_entry) into
store.py; move GlobalCapabilityReporter and its helpers (_scan_skills,
_scan_plugins, _scan_mcps, _read_skill_metadata, _scan_plugin_skills,
_managed_plugin_scan_path, _normalized_plugin_root, _is_macos_zip_metadata) into
reporter.py; move CapabilitySyncHandler and its sync helpers (_sync_skills,
_sync_plugins, _sync_mcps and download/extract helpers,
_plugin_marketplace/_plugin_key/_plugin_runtime_path/_plugin_store_path/_persisted_plugin_checksum/_plugin_record/_server_summary/_is_runtime_path_occupied_by_local_user/_ensure_runtime_symlink/_ensure_optional_runtime_symlink/_skill_store_path/_skill_digest/_directory_digest/_skill_digest_at
and skill download helpers) into sync_handler.py; keep top-level small shared
utilities (utc_now_iso, MANIFEST_VERSION, DEFAULT_FULL_REPORT_INTERVAL_SECONDS)
in a new utils.py or __init__.py and adjust imports accordingly; add an
__init__.py that re-exports the public classes (ManagedCapabilityManifest,
GlobalCapabilityStore, GlobalCapabilityReporter, CapabilitySyncHandler) so
external imports remain stable and update internal imports to the new module
paths.
In `@wework/src/components/common/ActionMenu.tsx`:
- Around line 128-129: The ActionMenu component contains hardcoded color classes
('bg-[`#2b2b2b`]', 'text-white', 'text-[`#ff7b7b`]') inside the menu button/menu
item class strings which forces a dark appearance; update those class strings in
ActionMenu to use the app's theme tokens or conditional theme-aware classes
instead (e.g., replace the hex classes with the established token class names or
add light/dark variants) so the menu respects the current theme; specifically
locate and change the occurrences of 'bg-[`#2b2b2b`]', 'text-white', and
'text-[`#ff7b7b`]' in the ActionMenu component and use the project's theme token
names or a theme prop/dark: variants to produce the correct colors in both light
and dark modes.
🪄 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: 941c16e8-28e2-47c7-a48f-da2d7bf356cf
📒 Files selected for processing (36)
README.mdREADME_zh.mdbackend/alembic/versions/20260609_d5e6f7a8b9c0_add_project_task_listing_index.pybackend/app/models/task.pybackend/app/services/device/capability_sync_service.pybackend/tests/models/test_task_model_indexes.pybackend/tests/services/device/test_capability_sync_service.pydocs/en/developer-guide/global-skill-sync.mddocs/zh/developer-guide/global-skill-sync.mdexecutor/agents/claude_code/config_manager.pyexecutor/agents/claude_code/local_mode_strategy.pyexecutor/agents/codex/config_builder.pyexecutor/modes/local/capabilities.pyexecutor/tests/agents/test_claude_code_config_security.pyexecutor/tests/agents/test_codex_config_builder.pyexecutor/tests/agents/test_mode_strategy.pyexecutor/tests/agents/test_skill_mcp_extraction.pyexecutor/tests/modes/local/test_global_capabilities.pyexecutor/tests/test_local_capability_sync_handler.pywework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/composer/AddContextMenu.tsxwework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerTextarea.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/common/ActionMenu.tsxwework/src/components/common/TextInputDialog.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.tswework/src/types/workbench.ts
💤 Files with no reviewable changes (2)
- wework/src/components/chat/MessageList.test.tsx
- wework/src/components/common/TextInputDialog.test.tsx
| from app.models.task import TaskResource | ||
|
|
||
|
|
||
| def test_task_resource_has_project_task_listing_index(): |
Contributor
There was a problem hiding this comment.
Add a return type hint to the test function.
Line 8 should include -> None to satisfy the repository-wide Python typing rule.
Suggested patch
-def test_task_resource_has_project_task_listing_index():
+def test_task_resource_has_project_task_listing_index() -> None:As per coding guidelines, "Type hints are required for all Python code."
📝 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
| def test_task_resource_has_project_task_listing_index(): | |
| def test_task_resource_has_project_task_listing_index() -> None: |
🤖 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 `@backend/tests/models/test_task_model_indexes.py` at line 8, The test function
test_task_resource_has_project_task_listing_index is missing a return type hint;
update its signature to add "-> None" (i.e., change the def for
test_task_resource_has_project_task_listing_index to include the return type
hint) so it conforms to the repository typing rule for all Python code.
Source: Coding guidelines
Comment on lines
+123
to
+130
| settings_config["hooks"] = { | ||
| "PostToolUse": [ | ||
| { | ||
| "matcher": "Write|Edit", | ||
| "hooks": [{"type": "command", "command": hook_command}], | ||
| } | ||
| ] | ||
| } |
Contributor
There was a problem hiding this comment.
Existing user hooks will be overwritten.
The current implementation replaces the entire "hooks" key, which will discard any user-configured hooks (e.g., PreToolUse, other PostToolUse matchers). Consider merging the new hook into the existing hooks structure instead.
🛠️ Suggested fix: merge into existing hooks
- settings_config["hooks"] = {
- "PostToolUse": [
- {
- "matcher": "Write|Edit",
- "hooks": [{"type": "command", "command": hook_command}],
- }
- ]
- }
+ existing_hooks = settings_config.get("hooks", {})
+ post_tool_use = existing_hooks.get("PostToolUse", [])
+ post_tool_use.append({
+ "matcher": "Write|Edit",
+ "hooks": [{"type": "command", "command": hook_command}],
+ })
+ existing_hooks["PostToolUse"] = post_tool_use
+ settings_config["hooks"] = existing_hooks🤖 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/agents/claude_code/local_mode_strategy.py` around lines 123 - 130,
The current assignment to settings_config["hooks"] replaces any user hooks;
instead retrieve the existing hooks map (settings_config.get("hooks", {})) and
merge into it: get or create the "PostToolUse" list, find an entry with matcher
"Write|Edit" and append the new {"type":"command","command": hook_command} into
its "hooks" list (or if no such matcher exists, append a new entry with matcher
"Write|Edit" and the hooks list). Update settings_config["hooks"] with the
merged structure so other keys (e.g., "PreToolUse" or other PostToolUse
matchers) are preserved.
Comment on lines
102
to
104
| def test_local_mode_writes_claude_json( | ||
| self, task_data, agent_config_with_sensitive_data, temp_workspace | ||
| self, task_data, agent_config_with_sensitive_data, temp_workspace, monkeypatch | ||
| ): |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add type hints to the updated test signatures.
These modified tests still use untyped parameters/returns. Please annotate fixture params and add -> None on test functions.
As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 167-169, 194-196
🤖 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/tests/agents/test_claude_code_config_security.py` around lines 102 -
104, The tests (e.g., test_local_mode_writes_claude_json) lack type hints on
parameters and return types; update the function signatures to annotate fixture
parameters (e.g., task_data: dict, agent_config_with_sensitive_data: dict,
temp_workspace: Path or str, monkeypatch: pytest.MonkeyPatch) and add -> None to
the test functions, and apply the same pattern to the other modified tests
referenced (the tests near the other modified blocks) so all test functions have
typed parameters and a return annotation of -> None.
Source: Coding guidelines
Comment on lines
+88
to
+89
| def test_build_codex_config_injects_global_mcp_overrides(tmp_path, monkeypatch): | ||
| monkeypatch.setenv("WEGENT_EXECUTOR_HOME", str(tmp_path / ".wegent-executor")) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Type-annotate the newly added tests.
Please add explicit parameter types and -> None return annotations for these new test functions.
As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 142-144
🤖 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/tests/agents/test_codex_config_builder.py` around lines 88 - 89, Add
Python type hints to the new test functions: annotate parameters (e.g.,
tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch or pytest.MonkeyPatch)
and add explicit return type -> None for each test function such as
test_build_codex_config_injects_global_mcp_overrides and the other tests
referenced around lines 142-144; update function signatures to include these
parameter types and a trailing "-> None" return annotation so all new tests
conform to the project's "Type hints are required for all Python code"
guideline.
Source: Coding guidelines
Comment on lines
+109
to
+111
| def test_get_config_directory_uses_global_for_project_tasks( | ||
| self, strategy, temp_workspace, tmp_path, monkeypatch | ||
| ): |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Apply type hints on the updated test methods.
These changed signatures should be annotated (fixture param types + -> None) to keep the test module compliant.
As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 121-129, 149-156, 202-210, 280-287, 357-357, 389-391, 402-404, 462-464, 471-473
🤖 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/tests/agents/test_mode_strategy.py` around lines 109 - 111, Annotate
the updated test function signatures (e.g.,
test_get_config_directory_uses_global_for_project_tasks) so fixture params are
typed and the function return is -> None; a minimal safe change is to import Any
from typing and change signatures like (self, strategy, temp_workspace,
tmp_path, monkeypatch) to (self, strategy: Any, temp_workspace: Any, tmp_path:
Any, monkeypatch: Any) -> None; apply the same pattern to the other updated test
functions called out in the review so all altered tests include fixture param
type hints and -> None.
Source: Coding guidelines
Comment on lines
+229
to
+231
| def test_reporter_scans_managed_plugin_store_when_claude_cache_path_is_missing( | ||
| tmp_path, monkeypatch | ||
| ): |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Type-annotate the newly added test function.
Please add fixture parameter annotations and -> None for this test.
As per coding guidelines, "Type hints are required for all Python code".
🤖 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/tests/modes/local/test_global_capabilities.py` around lines 229 -
231, The test function
test_reporter_scans_managed_plugin_store_when_claude_cache_path_is_missing needs
parameter and return type hints: add types for tmp_path and monkeypatch and
annotate return as -> None. Change the signature to accept tmp_path: Path (from
pathlib) and monkeypatch: pytest.MonkeyPatch (or MonkeyPatch from pytest) and
add/import those symbols if missing so the function reads like "def
test_reporter_scans_managed_plugin_store_when_claude_cache_path_is_missing(tmp_path:
Path, monkeypatch: pytest.MonkeyPatch) -> None:".
Source: Coding guidelines
Comment on lines
+176
to
+178
| def test_apply_sync_redownloads_managed_skill_when_runtime_link_is_broken( | ||
| tmp_path, monkeypatch | ||
| ): |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add type hints to the newly introduced test definitions.
These added test signatures should include explicit fixture types and -> None returns to satisfy repository Python standards.
As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 320-320, 520-520, 649-649, 797-797
🤖 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/tests/test_local_capability_sync_handler.py` around lines 176 - 178,
The test signature for
test_apply_sync_redownloads_managed_skill_when_runtime_link_is_broken lacks type
hints; change its signature to include fixture types (e.g., tmp_path:
pathlib.Path, monkeypatch: pytest.MonkeyPatch) and an explicit return type ->
None. Update the function definition for
test_apply_sync_redownloads_managed_skill_when_runtime_link_is_broken and any
other newly introduced tests in this file to use explicit fixture type
annotations and -> None return (use pathlib.Path and pytest.MonkeyPatch where
appropriate).
Source: Coding guidelines
Comment on lines
+154
to
+163
| {!hasText && ( | ||
| <button | ||
| type="button" | ||
| data-testid="voice-input-button" | ||
| className="absolute bottom-1.5 right-12 flex h-10 w-10 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | ||
| aria-label={t('workbench.voice_input', '语音输入')} | ||
| > | ||
| <Mic className="h-5 w-5" /> | ||
| </button> | ||
| )} |
Contributor
There was a problem hiding this comment.
Voice input button is non-functional.
The button renders but has no onClick handler, so it won't respond to user interaction. If this is intentional as a placeholder for future implementation, consider either:
- Adding a
disabledattribute and appropriate styling to indicate it's not yet functional - Hiding the button until the feature is implemented
Suggested fix to disable the placeholder button
<button
type="button"
data-testid="voice-input-button"
+ disabled
className="absolute bottom-1.5 right-12 flex h-10 w-10 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted"
aria-label={t('workbench.voice_input', '语音输入')}
>
<Mic className="h-5 w-5" />
</button>📝 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
| {!hasText && ( | |
| <button | |
| type="button" | |
| data-testid="voice-input-button" | |
| className="absolute bottom-1.5 right-12 flex h-10 w-10 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | |
| aria-label={t('workbench.voice_input', '语音输入')} | |
| > | |
| <Mic className="h-5 w-5" /> | |
| </button> | |
| )} | |
| {!hasText && ( | |
| <button | |
| type="button" | |
| data-testid="voice-input-button" | |
| disabled | |
| className="absolute bottom-1.5 right-12 flex h-10 w-10 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | |
| aria-label={t('workbench.voice_input', '语音输入')} | |
| > | |
| <Mic className="h-5 w-5" /> | |
| </button> | |
| )} |
🤖 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/composer/CompactChatComposer.tsx` around lines 154
- 163, The voice input button in CompactChatComposer renders without
interaction; update the button with a proper affordance: either add a no-op
onClick handler that opens the voice flow later (e.g.,
onClick={handleVoiceInputClick}) or mark it explicitly disabled and style it
accordingly so users know it's a placeholder (add disabled attribute and adjust
classNames), and reference the button by data-testid="voice-input-button" and
the hasText condition to control visibility; if you add an onClick, implement a
stub function handleVoiceInputClick inside CompactChatComposer to
dispatch/trigger the real voice input when implemented.
Comment on lines
+55
to
+62
| <button | ||
| type="button" | ||
| data-testid="voice-input-button" | ||
| className="flex h-8 w-8 shrink-0 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | ||
| aria-label={t('workbench.voice_input', '语音输入')} | ||
| > | ||
| <Mic className="h-4 w-4" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Voice input button is non-functional.
Same as in CompactChatComposer, this button has no onClick handler. Consider adding disabled attribute or hiding until the feature is ready.
Suggested fix
<button
type="button"
data-testid="voice-input-button"
+ disabled
className="flex h-8 w-8 shrink-0 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted"
aria-label={t('workbench.voice_input', '语音输入')}
>
<Mic className="h-4 w-4" />
</button>📝 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
| <button | |
| type="button" | |
| data-testid="voice-input-button" | |
| className="flex h-8 w-8 shrink-0 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | |
| aria-label={t('workbench.voice_input', '语音输入')} | |
| > | |
| <Mic className="h-4 w-4" /> | |
| </button> | |
| <button | |
| type="button" | |
| data-testid="voice-input-button" | |
| disabled | |
| className="flex h-8 w-8 shrink-0 items-center justify-center rounded-full p-0 text-text-secondary hover:bg-muted" | |
| aria-label={t('workbench.voice_input', '语音输入')} | |
| > | |
| <Mic className="h-4 w-4" /> | |
| </button> |
🤖 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/composer/ComposerToolbar.tsx` around lines 55 -
62, The voice input button in ComposerToolbar (data-testid "voice-input-button",
element rendering the Mic icon) lacks an onClick handler or disabled state like
in CompactChatComposer; update ComposerToolbar to either add a proper onClick
that triggers the voice input flow or set the button to disabled/hidden until
the feature is implemented (e.g., add disabled prop or conditional render) and
ensure aria-label and data-testid remain consistent for tests; reference the
button element and the Mic component when making the change.
Comment on lines
+354
to
+359
| {isThinking && ( | ||
| <span className="text-text-muted">正在思考</span> | ||
| )} | ||
| {isStreaming && hasContent && ( | ||
| <span className="text-text-muted">正在思考</span> | ||
| )} |
Contributor
There was a problem hiding this comment.
Duplicate "正在思考" indicator creates confusing UX.
The second condition at lines 357-359 shows "正在思考" when isStreaming && hasContent, which displays the thinking indicator even after the assistant has started producing content. This contradicts the purpose of a thinking indicator (shown before content appears).
Consider removing the second branch or replacing it with a different indicator (e.g., a subtle streaming indicator) if you want to show ongoing activity:
🔧 Suggested fix
{isThinking && (
<span className="text-text-muted">正在思考</span>
)}
- {isStreaming && hasContent && (
- <span className="text-text-muted">正在思考</span>
- )}📝 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
| {isThinking && ( | |
| <span className="text-text-muted">正在思考</span> | |
| )} | |
| {isStreaming && hasContent && ( | |
| <span className="text-text-muted">正在思考</span> | |
| )} | |
| {isThinking && ( | |
| <span className="text-text-muted">正在思考</span> | |
| )} |
🤖 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 354 - 359, The UI
shows duplicate "正在思考" because MessageList.tsx renders the same span for both
isThinking and (isStreaming && hasContent); update the rendering logic to avoid
showing the thinking indicator after content starts: either remove the second
branch that checks (isStreaming && hasContent) or replace it with a distinct
streaming indicator (e.g., change the text/className or render a different
element) so only isThinking renders "正在思考" and isStreaming renders a separate
streaming state; adjust the conditional around isStreaming/hasContent where the
span is returned to reflect this change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
executor/tests/modes/local/test_global_capabilities.py (3)
109-109: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd type hints to the test function.
As per coding guidelines, "Type hints are required for all Python code". Please add parameter type annotation for
tmp_pathand-> Nonereturn type.📝 Suggested change
+from pathlib import Path + -def test_manifest_store_records_skills_and_preserves_mcp_section(tmp_path): +def test_manifest_store_records_skills_and_preserves_mcp_section(tmp_path: Path) -> None:🤖 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/tests/modes/local/test_global_capabilities.py` at line 109, Add Python type hints to the test function signature: change def test_manifest_store_records_skills_and_preserves_mcp_section(tmp_path) to annotate the tmp_path parameter as pathlib.Path (tmp_path: pathlib.Path) and add a return type of -> None; if pathlib.Path is not imported, add the appropriate import (from pathlib import Path or import pathlib) and use that symbol in the annotation so the test function is fully type-annotated.Source: Coding guidelines
100-101: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd type hints to the test function.
As per coding guidelines, "Type hints are required for all Python code". Please add parameter type annotations for
tmp_pathandmonkeypatch, and-> Nonereturn type.📝 Suggested change
+from pathlib import Path +import pytest + -def test_default_manifest_path_lives_with_device_config(tmp_path, monkeypatch): +def test_default_manifest_path_lives_with_device_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None:🤖 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/tests/modes/local/test_global_capabilities.py` around lines 100 - 101, Add explicit type hints to the test function test_default_manifest_path_lives_with_device_config: annotate tmp_path as pathlib.Path and monkeypatch as pytest.MonkeyPatch (or MonkeyPatch if already imported), and add the return annotation -> None; update the function signature to def test_default_manifest_path_lives_with_device_config(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> None (or use the local MonkeyPatch import) so it conforms to the project typing guideline.Source: Coding guidelines
36-37: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd type hints to the test function.
As per coding guidelines, "Type hints are required for all Python code". Please add parameter type annotations for
tmp_pathandmonkeypatch, and-> Nonereturn type.📝 Suggested change
+from pathlib import Path +import pytest + def test_project_runtime_uses_global_claude_capability_dirs_without_merging_global_mcp( - tmp_path, monkeypatch -): + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None:🤖 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/tests/modes/local/test_global_capabilities.py` around lines 36 - 37, The test function test_project_runtime_uses_global_claude_capability_dirs_without_merging_global_mcp should include type hints: annotate tmp_path with pathlib.Path (or pytest's tmp_path type) and monkeypatch with pytest.MonkeyPatch (or typing.Any if pytests types unavailable), and add -> None as the return type; update the function signature in the test file accordingly to satisfy the project typing requirement.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 `@executor/tests/modes/local/test_global_capabilities.py`:
- Around line 17-24: Add a return type annotation to the test function
definition: change the signature of
test_wework_standalone_chat_uses_global_capabilities to include "-> None".
Locate the function by its name
test_wework_standalone_chat_uses_global_capabilities in the file (the block that
constructs ExecutionRequest with task_id=1901, project_id=0,
standalone_chat_workspace=True and asserts is_project_task(request) is True) and
annotate it as returning None per the project's type-hint guideline.
- Around line 27-33: Add a return type annotation to the test function
declaration for test_frontend_project_zero_chat_keeps_task_local_capabilities so
it complies with the project typing policy; update the function signature to
include "-> None" (while leaving body unchanged) — locate the test function name
test_frontend_project_zero_chat_keeps_task_local_capabilities and add the type
hint to its def line; no other logic changes required (references:
ExecutionRequest, is_project_task remain the same).
---
Outside diff comments:
In `@executor/tests/modes/local/test_global_capabilities.py`:
- Line 109: Add Python type hints to the test function signature: change def
test_manifest_store_records_skills_and_preserves_mcp_section(tmp_path) to
annotate the tmp_path parameter as pathlib.Path (tmp_path: pathlib.Path) and add
a return type of -> None; if pathlib.Path is not imported, add the appropriate
import (from pathlib import Path or import pathlib) and use that symbol in the
annotation so the test function is fully type-annotated.
- Around line 100-101: Add explicit type hints to the test function
test_default_manifest_path_lives_with_device_config: annotate tmp_path as
pathlib.Path and monkeypatch as pytest.MonkeyPatch (or MonkeyPatch if already
imported), and add the return annotation -> None; update the function signature
to def test_default_manifest_path_lives_with_device_config(tmp_path:
pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> None (or use the local
MonkeyPatch import) so it conforms to the project typing guideline.
- Around line 36-37: The test function
test_project_runtime_uses_global_claude_capability_dirs_without_merging_global_mcp
should include type hints: annotate tmp_path with pathlib.Path (or pytest's
tmp_path type) and monkeypatch with pytest.MonkeyPatch (or typing.Any if pytests
types unavailable), and add -> None as the return type; update the function
signature in the test file accordingly to satisfy the project typing
requirement.
🪄 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: a849a873-5090-4ff0-a3b1-dcc64fd38ccd
📒 Files selected for processing (2)
executor/modes/local/capabilities.pyexecutor/tests/modes/local/test_global_capabilities.py
🚧 Files skipped from review as they are similar to previous changes (1)
- executor/modes/local/capabilities.py
Comment on lines
+17
to
+24
| def test_wework_standalone_chat_uses_global_capabilities(): | ||
| request = ExecutionRequest( | ||
| task_id=1901, | ||
| project_id=0, | ||
| standalone_chat_workspace=True, | ||
| ) | ||
|
|
||
| assert is_project_task(request) is True |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add type hints to the test function.
As per coding guidelines, "Type hints are required for all Python code". Please add -> None return type annotation.
📝 Suggested change
-def test_wework_standalone_chat_uses_global_capabilities():
+def test_wework_standalone_chat_uses_global_capabilities() -> None:📝 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
| def test_wework_standalone_chat_uses_global_capabilities(): | |
| request = ExecutionRequest( | |
| task_id=1901, | |
| project_id=0, | |
| standalone_chat_workspace=True, | |
| ) | |
| assert is_project_task(request) is True | |
| def test_wework_standalone_chat_uses_global_capabilities() -> None: | |
| request = ExecutionRequest( | |
| task_id=1901, | |
| project_id=0, | |
| standalone_chat_workspace=True, | |
| ) | |
| assert is_project_task(request) is True |
🤖 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/tests/modes/local/test_global_capabilities.py` around lines 17 - 24,
Add a return type annotation to the test function definition: change the
signature of test_wework_standalone_chat_uses_global_capabilities to include "->
None". Locate the function by its name
test_wework_standalone_chat_uses_global_capabilities in the file (the block that
constructs ExecutionRequest with task_id=1901, project_id=0,
standalone_chat_workspace=True and asserts is_project_task(request) is True) and
annotate it as returning None per the project's type-hint guideline.
Source: Coding guidelines
Comment on lines
+27
to
+33
| def test_frontend_project_zero_chat_keeps_task_local_capabilities(): | ||
| request = ExecutionRequest( | ||
| task_id=1902, | ||
| project_id=0, | ||
| ) | ||
|
|
||
| assert is_project_task(request) is False |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add type hints to the test function.
As per coding guidelines, "Type hints are required for all Python code". Please add -> None return type annotation.
📝 Suggested change
-def test_frontend_project_zero_chat_keeps_task_local_capabilities():
+def test_frontend_project_zero_chat_keeps_task_local_capabilities() -> None:🤖 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/tests/modes/local/test_global_capabilities.py` around lines 27 - 33,
Add a return type annotation to the test function declaration for
test_frontend_project_zero_chat_keeps_task_local_capabilities so it complies
with the project typing policy; update the function signature to include "->
None" (while leaving body unchanged) — locate the test function name
test_frontend_project_zero_chat_keeps_task_local_capabilities and add the type
hint to its def line; no other logic changes required (references:
ExecutionRequest, is_project_task remain the same).
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wework/src/features/workbench/WorkbenchProvider.test.tsx (2)
1102-1114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWait for the post-ACK navigation before asserting the URL.
sendCurrentInput()is kicked off withvoid, and thiswaitForonly provessendMessagewas called. The route update happens later in the async ACK path, so the final location check can race and turn flaky.Suggested fix
await waitFor(() => expect(sendMessage).toHaveBeenCalledWith( expect.objectContaining({ team_id: 2, project_id: 0, client_origin: 'wework', device_id: 'cloud-online', task_type: 'code', message: 'run pwd', }) ) ) - expect(window.location.pathname + window.location.search).toBe('/tasks/100?projectId=0') + await waitFor(() => + expect(window.location.pathname + window.location.search).toBe( + '/tasks/100?projectId=0' + ) + )🤖 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/features/workbench/WorkbenchProvider.test.tsx` around lines 1102 - 1114, The test currently only waits for sendMessage to be called (via expect(sendMessage) in waitFor) but then immediately asserts the URL, which can race because sendCurrentInput() continues async ACK handling that triggers navigation; update the test to wait for the post-ACK navigation before asserting the location by awaiting the ACK-driven path change instead of immediately checking pathname—e.g., after confirming sendMessage was called, add a waitFor that expects window.location.pathname + window.location.search toBe('/tasks/100?projectId=0') or otherwise await the ACK handler (the async work started by sendCurrentInput) so the final URL assertion runs only after the ACK completion. Ensure references to sendCurrentInput(), sendMessage, and the post-ACK navigation behavior are used to locate the logic to await.
405-485:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThis no longer verifies
startNewChat()from the remembered-project state.After the
standalone chatclick on Line 478,currentProjectis already cleared. Thenew chatassertions on Lines 482-484 only exercise a second clear from standalone state, so a regression instartNewChat()during initial bootstrap would still pass here. Split these into separate tests or re-render before the second action.🤖 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/features/workbench/WorkbenchProvider.test.tsx` around lines 405 - 485, The test currently clicks "standalone chat" then immediately clicks "new chat", so the second assertion never verifies startNewChat() from the remembered-project bootstrap state; update the test named "starts new chat as standalone project zero even with a remembered project" to either split into two tests or re-render/reset state between actions: ensure initial state is the remembered project by setting localStorage ('wework.lastProjectId.1') and rendering WorkbenchProvider/ProjectSelectionProbe, assert current project is "Wegent", then call the action under test startNewChat() (simulate clicking "new chat") in a fresh render (or a separate test) and assert currentProject becomes "no-project" and location is "/?projectId=0" so you actually exercise startNewChat() from the bootstrapped remembered-project state.
🤖 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/lib/navigation.ts`:
- Around line 31-45: getNumericSearchParam currently uses Number(value) which
accepts "1e2" and "0x10"; restrict accepted values to plain decimal digits only
by first validating value against a decimal-only pattern (e.g. /^\d+$/) before
converting to a number. Update getNumericSearchParam to iterate names, skip
null/empty, test value matches the decimal-only pattern, then parse (Number or
parseInt) and return the non-negative integer; otherwise continue to next name
and return undefined if none match.
---
Outside diff comments:
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 1102-1114: The test currently only waits for sendMessage to be
called (via expect(sendMessage) in waitFor) but then immediately asserts the
URL, which can race because sendCurrentInput() continues async ACK handling that
triggers navigation; update the test to wait for the post-ACK navigation before
asserting the location by awaiting the ACK-driven path change instead of
immediately checking pathname—e.g., after confirming sendMessage was called, add
a waitFor that expects window.location.pathname + window.location.search
toBe('/tasks/100?projectId=0') or otherwise await the ACK handler (the async
work started by sendCurrentInput) so the final URL assertion runs only after the
ACK completion. Ensure references to sendCurrentInput(), sendMessage, and the
post-ACK navigation behavior are used to locate the logic to await.
- Around line 405-485: The test currently clicks "standalone chat" then
immediately clicks "new chat", so the second assertion never verifies
startNewChat() from the remembered-project bootstrap state; update the test
named "starts new chat as standalone project zero even with a remembered
project" to either split into two tests or re-render/reset state between
actions: ensure initial state is the remembered project by setting localStorage
('wework.lastProjectId.1') and rendering
WorkbenchProvider/ProjectSelectionProbe, assert current project is "Wegent",
then call the action under test startNewChat() (simulate clicking "new chat") in
a fresh render (or a separate test) and assert currentProject becomes
"no-project" and location is "/?projectId=0" so you actually exercise
startNewChat() from the bootstrapped remembered-project state.
🪄 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: 6f1ad42a-fa83-4fe5-a3a7-8e258cf8dcac
📒 Files selected for processing (4)
wework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/lib/navigation.test.tswework/src/lib/navigation.ts
✅ Files skipped from review due to trivial changes (1)
- wework/src/lib/navigation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/features/workbench/WorkbenchProvider.tsx
Comment on lines
+31
to
+45
| function getNumericSearchParam( | ||
| searchParams: URLSearchParams, | ||
| ...names: string[] | ||
| ): number | undefined { | ||
| for (const name of names) { | ||
| const value = searchParams.get(name) | ||
| if (value === null || value.trim() === '') continue | ||
|
|
||
| const numberValue = Number(value) | ||
| if (Number.isInteger(numberValue) && numberValue >= 0) { | ||
| return numberValue | ||
| } | ||
| } | ||
|
|
||
| return undefined |
Contributor
There was a problem hiding this comment.
Reject non-decimal query IDs here.
Number(value) treats inputs like 1e2 and 0x10 as valid integers, so malformed URLs can resolve to the wrong task or project. Route IDs should only accept plain decimal digits.
Suggested fix
for (const name of names) {
const value = searchParams.get(name)
if (value === null || value.trim() === '') continue
- const numberValue = Number(value)
- if (Number.isInteger(numberValue) && numberValue >= 0) {
- return numberValue
+ if (/^\d+$/.test(value)) {
+ return Number(value)
}
}🤖 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/lib/navigation.ts` around lines 31 - 45, getNumericSearchParam
currently uses Number(value) which accepts "1e2" and "0x10"; restrict accepted
values to plain decimal digits only by first validating value against a
decimal-only pattern (e.g. /^\d+$/) before converting to a number. Update
getNumericSearchParam to iterate names, skip null/empty, test value matches the
decimal-only pattern, then parse (Number or parseInt) and return the
non-negative integer; otherwise continue to next name and return undefined if
none match.
FicoHu
force-pushed
the
codex/local-capability-store-links
branch
2 times, most recently
from
June 9, 2026 10:48
5c12384 to
0c9ce86
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/zh/developer-guide/global-skill-sync.md (1)
167-173:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
skill_idexample consistent with payload semantics.Line 169 and Line 172 use
1336(installed ID) asskill_id/store key. In the same example payload,skill_idis101, so the manifest snippet should reflect that to avoid ID confusion.Suggested doc correction
- "skill_id": 1336, + "skill_id": 101, ... - "store_path": "~/.wegent-executor/capabilities/store/skills/1336-default-browser", + "store_path": "~/.wegent-executor/capabilities/store/skills/101-default-browser",🤖 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 `@docs/zh/developer-guide/global-skill-sync.md` around lines 167 - 173, The manifest example under "skills" uses skill_id and store_path keyed with 1336 while the example payload earlier uses skill_id 101; update the "skill_id" value and any store_path/keys that reference 1336 (e.g., the "skills" -> "browser" block and its "store_path") to use 101 so the manifest matches the payload semantics and avoids ID confusion.wework/src/components/chat/ChatInput.test.tsx (1)
1-1751: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this test file into smaller modules to comply with repository limits.
This file exceeds the configured maximum size and should be split into focused submodules (e.g., composer layout, model menu, attachments, project-work selection).
As per coding guidelines
**/*.{ts,tsx,js,jsx}:File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.🤖 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/ChatInput.test.tsx` around lines 1 - 1751, The test file is too large — split ChatInput.test.tsx into focused submodules (e.g., ChatInput.composer.test.tsx, ChatInput.model.test.tsx, ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx) by grouping related tests (composer/layout, model menu, local-skill/autocomplete, attachments/image previews, project-work selection) into separate files; extract shared helpers ControlledChatInput, projectChatControls, projectWorkControls and common stubs (originalCreateObjectUrl, afterEach cleanup logic) into a new test utility module (e.g., ChatInput.test.utils.tsx) and import them in each new test file; ensure each new file imports ChatInput and necessary types, preserves the original describe/test names, and keeps per-test mocks (vi.stubGlobal, URL.createObjectURL) local to the tests; run the test suite and adjust any test IDs/import paths if splitting exposes relative import issues.Source: Coding guidelines
wework/src/components/layout/DesktopWorkbenchLayout.test.tsx (1)
1-2597: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftBreak this test suite into multiple files to satisfy the size limit and keep scope focused.
At 2k+ lines, this suite is beyond the configured threshold and should be partitioned (e.g., sidebar/search, settings, workspace panels, device gating, environment popover).
As per coding guidelines
**/*.{ts,tsx,js,jsx}:File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.🤖 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/layout/DesktopWorkbenchLayout.test.tsx` around lines 1 - 2597, The test file DesktopWorkbenchLayout.test.tsx is too large and must be split into focused suites; extract groups like sidebar/search tests, settings/quota/environment popover tests, workspace panel tests, and device gating tests into separate files. Move shared fixtures/helpers (baseProps, createCloudWorkspacePanelState, renderWorkspacePanelLayout, createRect, and top-level vi.mock calls) into a new test-utils/shared setup module and import them in each new test file, keeping mocks (createDeviceApi, createQuotaApi, navigator.clipboard stub) initialized once (or via a beforeEach in the shared module). In each new test file, import DesktopWorkbenchLayout and only the relevant helpers and then copy the related test cases (e.g., the "opens and filters the desktop search dialog..." and "searches inside conversation messages..." tests into sidebar-search.test.tsx; the environment/branch/commit tests into environment-popover.test.tsx; workspace open/resize/close tests into workspace-panels.test.tsx; device gating and project create tests into devices-and-projects.test.tsx). Ensure test names, data-testid selectors, and any referenced handlers (onOpenTask, onLoadEnvironmentInfo, onCreateProject, onUpgradeDevice, etc.) are preserved and wired via the shared baseProps where needed.Source: Coding guidelines
♻️ Duplicate comments (3)
executor/tests/agents/test_codex_config_builder.py (1)
88-89: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winType-annotate the newly added test signatures.
Please annotate fixture params (e.g.,
tmp_path: Path,monkeypatch: pytest.MonkeyPatch) and add-> Nonefor these new tests.As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 142-144
🤖 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/tests/agents/test_codex_config_builder.py` around lines 88 - 89, The test function signatures (e.g., test_build_codex_config_injects_global_mcp_overrides) must include type annotations for fixture params and an explicit return type; update the function defs to annotate tmp_path as Path and monkeypatch as pytest.MonkeyPatch (and any other newly added tests referenced around the same change) and add -> None to the signature; ensure you import Path and pytest if not already imported so the annotations are valid for the functions like test_build_codex_config_injects_global_mcp_overrides and the other new tests mentioned.Source: Coding guidelines
executor/agents/claude_code/local_mode_strategy.py (1)
123-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve existing hook definitions when adding
PostToolUse.This assignment replaces the entire
hooksobject, so existing user hooks are lost on save. Please merge into existing hooks instead of overwriting.Suggested fix
- settings_config["hooks"] = { - "PostToolUse": [ - { - "matcher": "Write|Edit", - "hooks": [{"type": "command", "command": hook_command}], - } - ] - } + hooks = settings_config.get("hooks", {}) + post_tool_use = hooks.get("PostToolUse", []) + target = next( + ( + item + for item in post_tool_use + if isinstance(item, dict) and item.get("matcher") == "Write|Edit" + ), + None, + ) + new_hook = {"type": "command", "command": hook_command} + if target is None: + post_tool_use.append({"matcher": "Write|Edit", "hooks": [new_hook]}) + else: + target_hooks = target.get("hooks") + if not isinstance(target_hooks, list): + target_hooks = [] + target_hooks.append(new_hook) + target["hooks"] = target_hooks + hooks["PostToolUse"] = post_tool_use + settings_config["hooks"] = hooks🤖 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/agents/claude_code/local_mode_strategy.py` around lines 123 - 130, When adding the PostToolUse hook, don't overwrite settings_config["hooks"]; instead merge into any existing hooks object: read settings_config.get("hooks", {}) then ensure PostToolUse is a list (create if missing), append or extend with the new dict containing matcher "Write|Edit" and hooks [{"type":"command","command": hook_command}] (avoid replacing other hook types or entries). Update the logic around settings_config["hooks"] so it preserves existing entries and only inserts/merges the new PostToolUse entry.executor/tests/test_local_capability_sync_handler.py (1)
33-37: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd type annotations to newly introduced helper/tests.
These new function signatures still miss typed fixture params and explicit return types (
-> Nonefor tests).Example pattern to apply
-def test_apply_sync_downloads_uploaded_plugin_into_store(tmp_path, monkeypatch): +def test_apply_sync_downloads_uploaded_plugin_into_store( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None:As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 176-178, 320-320, 520-520, 649-649, 797-797
🤖 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/tests/test_local_capability_sync_handler.py` around lines 33 - 37, Add missing type hints to the new helper and test functions: ensure all test functions have explicit return types of -> None and all pytest fixtures and helper parameters are fully annotated (e.g., the helper create_nested_plugin_zip parameters are typed and its return -> bytes is present; apply the same pattern to other new helpers/fixtures referenced in this file). Update any untyped fixture parameters to explicit types (e.g., fixture: pytest.Fixture or concrete types like str, dict[str, str], bytes as appropriate) and add -> None to test functions so every function in the file has complete type annotations per the coding guidelines.Source: Coding guidelines
🧹 Nitpick comments (1)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
1-2034: 🏗️ Heavy liftSplit
WorkbenchProvider.tsxinto smaller modules/hooks to meet repository limits.This file and the main provider function are now far beyond the repository’s size/complexity limits, which will keep making feature work riskier.
As per coding guidelines,
File size limit: If a file exceeds 1000 lines, split it into multiple sub-modulesandFunction length: Max 50 lines per function (preferred).🤖 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/features/workbench/WorkbenchProvider.tsx` around lines 1 - 2034, The WorkbenchProvider function and file are too large—split responsibilities by extracting related helper functions, reducer logic, and effectful behavior into smaller modules: move createDefaultServices, getCurrentAppPath, getTaskRouteKey and other pure helpers (isRecord, normalizeStoredModelOptions, getSubtaskResult, normalizeProcessingBlock, normalizeChatBlock, normalizeAttachmentStatus, getSubtaskAttachments, subtaskToMessage, sortSubtasksForDisplay, read/writeTaskId, read/writeLastProjectId, getTaskModelSelection, getNewChatModelSelection, isRunningTaskStatus, normalizeGuidanceError, resolveOpenedTaskProjectId, getRememberedStandaloneDeviceId, getUpgradeStatusMessage, isTerminalDeviceUpgradeStatus) into a wework/utils or wework/helpers module; extract device upgrade and timer logic (setDeviceUpgradeState, schedule/clearUpgradeStateTimer, upgradeDevice, related useEffect subscriptions) into a useDeviceUpgrade hook; move message handling and streaming effects and messageReducer usage into a useWorkbenchMessages hook (wrap dispatchMessages, sendPreparedMessage, pauseCurrentResponse, queued message flows, sendQueuedAsGuidance); move model/skill/attachment selection wiring into useWorkbenchModels/useWorkbenchSkills/useWorkbenchAttachments (or re-use existing hooks) and the createDefaultServices into a services factory module; ensure workbenchReducer and initialWorkbenchState live in their own reducers file (if not already), then refactor WorkbenchProvider to just compose these hooks, wire the WorkbenchContext value, and keep the main provider function under ~50 lines, importing the named symbols (WorkbenchProvider, workbenchReducer, messageReducer, useWorkbenchModels, useWorkbenchSkills, useWorkbenchAttachments, createDefaultServices, setDeviceUpgradeState, sendPreparedMessage, listLocalSkills) so callers and tests keep the same API.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 `@executor/hooks/rthook_version.py`:
- Around line 29-30: The current broad except Exception: that sets version =
None silently swallows unexpected errors; narrow the handler (or remove it) so
only expected failures are caught — for example, catch FileNotFoundError,
OSError, subprocess.CalledProcessError, ValueError (as appropriate for how
version is resolved) around the code that assigns version, or remove the
try/except and let the caller handle exceptions; ensure the variable version is
still assigned a sensible default (None) only for the specific, anticipated
failure modes.
In `@executor/tests/test_version_cli.py`:
- Line 95: Add a type hint to the test function parameter by changing the
signature of test_get_version_uses_env_override to accept monkeypatch:
pytest.MonkeyPatch; ensure pytest is imported in the test file (import pytest)
so the annotation resolves. This updates the function definition
(test_get_version_uses_env_override) to comply with the required type-hinting
rule.
In `@README_zh.md`:
- Line 17: The TOC anchor(s) in README_zh.md don't match the generated heading
IDs (MD051); update the fragment(s) such as "`#-快速开始`" to the exact slug used by
your Markdown renderer (e.g., remove the unexpected leading hyphen or use the
renderer's slug format for "快速开始" and other headings like "核心场景" and "工作方式"),
ensuring each [label](`#fragment`) matches the actual heading IDs so in-page links
resolve correctly.
- Around line 1-3: The README_zh.md is missing the required Markdown frontmatter
block with sidebar_position; add a YAML frontmatter block at the top of the file
(above the existing "# Wegent" heading) that includes at minimum a
sidebar_position key with an appropriate integer value (e.g., sidebar_position:
1) so the doc conforms to the “**/*.md” guideline requiring frontmatter.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 70-79: The returned object in projectWorkControls (in
ChatInput.test.tsx) contains a duplicated key devices; remove the redundant
devices entry so the object has only one devices property (ensure you keep the
intended devices value and preserve the overrides spread order), leaving
projects, devices, currentProjectId, currentStandaloneDeviceId, onSelectProject,
onSelectStandaloneDevice and ...overrides exactly once.
---
Outside diff comments:
In `@docs/zh/developer-guide/global-skill-sync.md`:
- Around line 167-173: The manifest example under "skills" uses skill_id and
store_path keyed with 1336 while the example payload earlier uses skill_id 101;
update the "skill_id" value and any store_path/keys that reference 1336 (e.g.,
the "skills" -> "browser" block and its "store_path") to use 101 so the manifest
matches the payload semantics and avoids ID confusion.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1-1751: The test file is too large — split ChatInput.test.tsx into
focused submodules (e.g., ChatInput.composer.test.tsx, ChatInput.model.test.tsx,
ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx) by grouping
related tests (composer/layout, model menu, local-skill/autocomplete,
attachments/image previews, project-work selection) into separate files; extract
shared helpers ControlledChatInput, projectChatControls, projectWorkControls and
common stubs (originalCreateObjectUrl, afterEach cleanup logic) into a new test
utility module (e.g., ChatInput.test.utils.tsx) and import them in each new test
file; ensure each new file imports ChatInput and necessary types, preserves the
original describe/test names, and keeps per-test mocks (vi.stubGlobal,
URL.createObjectURL) local to the tests; run the test suite and adjust any test
IDs/import paths if splitting exposes relative import issues.
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 1-2597: The test file DesktopWorkbenchLayout.test.tsx is too large
and must be split into focused suites; extract groups like sidebar/search tests,
settings/quota/environment popover tests, workspace panel tests, and device
gating tests into separate files. Move shared fixtures/helpers (baseProps,
createCloudWorkspacePanelState, renderWorkspacePanelLayout, createRect, and
top-level vi.mock calls) into a new test-utils/shared setup module and import
them in each new test file, keeping mocks (createDeviceApi, createQuotaApi,
navigator.clipboard stub) initialized once (or via a beforeEach in the shared
module). In each new test file, import DesktopWorkbenchLayout and only the
relevant helpers and then copy the related test cases (e.g., the "opens and
filters the desktop search dialog..." and "searches inside conversation
messages..." tests into sidebar-search.test.tsx; the environment/branch/commit
tests into environment-popover.test.tsx; workspace open/resize/close tests into
workspace-panels.test.tsx; device gating and project create tests into
devices-and-projects.test.tsx). Ensure test names, data-testid selectors, and
any referenced handlers (onOpenTask, onLoadEnvironmentInfo, onCreateProject,
onUpgradeDevice, etc.) are preserved and wired via the shared baseProps where
needed.
---
Duplicate comments:
In `@executor/agents/claude_code/local_mode_strategy.py`:
- Around line 123-130: When adding the PostToolUse hook, don't overwrite
settings_config["hooks"]; instead merge into any existing hooks object: read
settings_config.get("hooks", {}) then ensure PostToolUse is a list (create if
missing), append or extend with the new dict containing matcher "Write|Edit" and
hooks [{"type":"command","command": hook_command}] (avoid replacing other hook
types or entries). Update the logic around settings_config["hooks"] so it
preserves existing entries and only inserts/merges the new PostToolUse entry.
In `@executor/tests/agents/test_codex_config_builder.py`:
- Around line 88-89: The test function signatures (e.g.,
test_build_codex_config_injects_global_mcp_overrides) must include type
annotations for fixture params and an explicit return type; update the function
defs to annotate tmp_path as Path and monkeypatch as pytest.MonkeyPatch (and any
other newly added tests referenced around the same change) and add -> None to
the signature; ensure you import Path and pytest if not already imported so the
annotations are valid for the functions like
test_build_codex_config_injects_global_mcp_overrides and the other new tests
mentioned.
In `@executor/tests/test_local_capability_sync_handler.py`:
- Around line 33-37: Add missing type hints to the new helper and test
functions: ensure all test functions have explicit return types of -> None and
all pytest fixtures and helper parameters are fully annotated (e.g., the helper
create_nested_plugin_zip parameters are typed and its return -> bytes is
present; apply the same pattern to other new helpers/fixtures referenced in this
file). Update any untyped fixture parameters to explicit types (e.g., fixture:
pytest.Fixture or concrete types like str, dict[str, str], bytes as appropriate)
and add -> None to test functions so every function in the file has complete
type annotations per the coding guidelines.
---
Nitpick comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 1-2034: The WorkbenchProvider function and file are too
large—split responsibilities by extracting related helper functions, reducer
logic, and effectful behavior into smaller modules: move createDefaultServices,
getCurrentAppPath, getTaskRouteKey and other pure helpers (isRecord,
normalizeStoredModelOptions, getSubtaskResult, normalizeProcessingBlock,
normalizeChatBlock, normalizeAttachmentStatus, getSubtaskAttachments,
subtaskToMessage, sortSubtasksForDisplay, read/writeTaskId,
read/writeLastProjectId, getTaskModelSelection, getNewChatModelSelection,
isRunningTaskStatus, normalizeGuidanceError, resolveOpenedTaskProjectId,
getRememberedStandaloneDeviceId, getUpgradeStatusMessage,
isTerminalDeviceUpgradeStatus) into a wework/utils or wework/helpers module;
extract device upgrade and timer logic (setDeviceUpgradeState,
schedule/clearUpgradeStateTimer, upgradeDevice, related useEffect subscriptions)
into a useDeviceUpgrade hook; move message handling and streaming effects and
messageReducer usage into a useWorkbenchMessages hook (wrap dispatchMessages,
sendPreparedMessage, pauseCurrentResponse, queued message flows,
sendQueuedAsGuidance); move model/skill/attachment selection wiring into
useWorkbenchModels/useWorkbenchSkills/useWorkbenchAttachments (or re-use
existing hooks) and the createDefaultServices into a services factory module;
ensure workbenchReducer and initialWorkbenchState live in their own reducers
file (if not already), then refactor WorkbenchProvider to just compose these
hooks, wire the WorkbenchContext value, and keep the main provider function
under ~50 lines, importing the named symbols (WorkbenchProvider,
workbenchReducer, messageReducer, useWorkbenchModels, useWorkbenchSkills,
useWorkbenchAttachments, createDefaultServices, setDeviceUpgradeState,
sendPreparedMessage, listLocalSkills) so callers and tests keep the same API.
🪄 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: 55b34c1f-67c0-43c0-b135-8e92454d8faf
📒 Files selected for processing (43)
README.mdREADME_zh.mdbackend/app/services/device/capability_sync_service.pybackend/app/services/execution/request_builder.pybackend/tests/services/device/test_capability_sync_service.pybackend/tests/services/execution/test_request_builder_standalone_workspace.pydocs/en/developer-guide/global-skill-sync.mddocs/en/user-guide/ai-devices/local-device-support.mddocs/zh/developer-guide/global-skill-sync.mddocs/zh/user-guide/ai-devices/local-device-support.mdexecutor/agents/claude_code/config_manager.pyexecutor/agents/claude_code/local_mode_strategy.pyexecutor/agents/claude_code/standalone_chat_workspace.pyexecutor/agents/codex/config_builder.pyexecutor/hooks/rthook_version.pyexecutor/modes/local/capabilities.pyexecutor/tests/agents/test_claude_code_config_security.pyexecutor/tests/agents/test_codex_config_builder.pyexecutor/tests/agents/test_mode_strategy.pyexecutor/tests/agents/test_project_workspace_paths.pyexecutor/tests/agents/test_skill_mcp_extraction.pyexecutor/tests/agents/test_standalone_chat_workspace.pyexecutor/tests/modes/local/test_global_capabilities.pyexecutor/tests/test_local_capability_sync_handler.pyexecutor/tests/test_version_cli.pyexecutor/version.pyshared/models/execution.pywework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/composer/AddContextMenu.tsxwework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerTextarea.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/common/ActionMenu.tsxwework/src/components/common/TextInputDialog.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.test.tswework/src/lib/navigation.test.tswework/src/lib/navigation.ts
💤 Files with no reviewable changes (2)
- wework/src/components/common/TextInputDialog.test.tsx
- wework/src/components/chat/MessageList.test.tsx
✅ Files skipped from review due to trivial changes (6)
- wework/src/lib/navigation.test.ts
- docs/en/user-guide/ai-devices/local-device-support.md
- wework/src/components/common/ActionMenu.tsx
- docs/zh/user-guide/ai-devices/local-device-support.md
- README.md
- docs/en/developer-guide/global-skill-sync.md
🚧 Files skipped from review as they are similar to previous changes (17)
- shared/models/execution.py
- backend/tests/services/execution/test_request_builder_standalone_workspace.py
- backend/app/services/device/capability_sync_service.py
- backend/tests/services/device/test_capability_sync_service.py
- wework/src/components/chat/composer/ComposerToolbar.tsx
- executor/tests/agents/test_claude_code_config_security.py
- executor/agents/claude_code/config_manager.py
- executor/agents/claude_code/standalone_chat_workspace.py
- executor/tests/modes/local/test_global_capabilities.py
- executor/tests/agents/test_skill_mcp_extraction.py
- wework/src/components/chat/ChatInput.tsx
- wework/src/components/chat/composer/CompactChatComposer.tsx
- wework/src/components/chat/MessageList.tsx
- executor/tests/agents/test_standalone_chat_workspace.py
- wework/src/components/chat/composer/ComposerTextarea.tsx
- executor/tests/agents/test_mode_strategy.py
- executor/modes/local/capabilities.py
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/zh/developer-guide/global-skill-sync.md (1)
167-173:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
skill_idexample consistent with payload semantics.Line 169 and Line 172 use
1336(installed ID) asskill_id/store key. In the same example payload,skill_idis101, so the manifest snippet should reflect that to avoid ID confusion.Suggested doc correction
- "skill_id": 1336, + "skill_id": 101, ... - "store_path": "~/.wegent-executor/capabilities/store/skills/1336-default-browser", + "store_path": "~/.wegent-executor/capabilities/store/skills/101-default-browser",🤖 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 `@docs/zh/developer-guide/global-skill-sync.md` around lines 167 - 173, The manifest example under "skills" uses skill_id and store_path keyed with 1336 while the example payload earlier uses skill_id 101; update the "skill_id" value and any store_path/keys that reference 1336 (e.g., the "skills" -> "browser" block and its "store_path") to use 101 so the manifest matches the payload semantics and avoids ID confusion.wework/src/components/chat/ChatInput.test.tsx (1)
1-1751: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this test file into smaller modules to comply with repository limits.
This file exceeds the configured maximum size and should be split into focused submodules (e.g., composer layout, model menu, attachments, project-work selection).
As per coding guidelines
**/*.{ts,tsx,js,jsx}:File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.🤖 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/ChatInput.test.tsx` around lines 1 - 1751, The test file is too large — split ChatInput.test.tsx into focused submodules (e.g., ChatInput.composer.test.tsx, ChatInput.model.test.tsx, ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx) by grouping related tests (composer/layout, model menu, local-skill/autocomplete, attachments/image previews, project-work selection) into separate files; extract shared helpers ControlledChatInput, projectChatControls, projectWorkControls and common stubs (originalCreateObjectUrl, afterEach cleanup logic) into a new test utility module (e.g., ChatInput.test.utils.tsx) and import them in each new test file; ensure each new file imports ChatInput and necessary types, preserves the original describe/test names, and keeps per-test mocks (vi.stubGlobal, URL.createObjectURL) local to the tests; run the test suite and adjust any test IDs/import paths if splitting exposes relative import issues.Source: Coding guidelines
wework/src/components/layout/DesktopWorkbenchLayout.test.tsx (1)
1-2597: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftBreak this test suite into multiple files to satisfy the size limit and keep scope focused.
At 2k+ lines, this suite is beyond the configured threshold and should be partitioned (e.g., sidebar/search, settings, workspace panels, device gating, environment popover).
As per coding guidelines
**/*.{ts,tsx,js,jsx}:File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules.🤖 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/layout/DesktopWorkbenchLayout.test.tsx` around lines 1 - 2597, The test file DesktopWorkbenchLayout.test.tsx is too large and must be split into focused suites; extract groups like sidebar/search tests, settings/quota/environment popover tests, workspace panel tests, and device gating tests into separate files. Move shared fixtures/helpers (baseProps, createCloudWorkspacePanelState, renderWorkspacePanelLayout, createRect, and top-level vi.mock calls) into a new test-utils/shared setup module and import them in each new test file, keeping mocks (createDeviceApi, createQuotaApi, navigator.clipboard stub) initialized once (or via a beforeEach in the shared module). In each new test file, import DesktopWorkbenchLayout and only the relevant helpers and then copy the related test cases (e.g., the "opens and filters the desktop search dialog..." and "searches inside conversation messages..." tests into sidebar-search.test.tsx; the environment/branch/commit tests into environment-popover.test.tsx; workspace open/resize/close tests into workspace-panels.test.tsx; device gating and project create tests into devices-and-projects.test.tsx). Ensure test names, data-testid selectors, and any referenced handlers (onOpenTask, onLoadEnvironmentInfo, onCreateProject, onUpgradeDevice, etc.) are preserved and wired via the shared baseProps where needed.Source: Coding guidelines
♻️ Duplicate comments (3)
executor/tests/agents/test_codex_config_builder.py (1)
88-89: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winType-annotate the newly added test signatures.
Please annotate fixture params (e.g.,
tmp_path: Path,monkeypatch: pytest.MonkeyPatch) and add-> Nonefor these new tests.As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 142-144
🤖 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/tests/agents/test_codex_config_builder.py` around lines 88 - 89, The test function signatures (e.g., test_build_codex_config_injects_global_mcp_overrides) must include type annotations for fixture params and an explicit return type; update the function defs to annotate tmp_path as Path and monkeypatch as pytest.MonkeyPatch (and any other newly added tests referenced around the same change) and add -> None to the signature; ensure you import Path and pytest if not already imported so the annotations are valid for the functions like test_build_codex_config_injects_global_mcp_overrides and the other new tests mentioned.Source: Coding guidelines
executor/agents/claude_code/local_mode_strategy.py (1)
123-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve existing hook definitions when adding
PostToolUse.This assignment replaces the entire
hooksobject, so existing user hooks are lost on save. Please merge into existing hooks instead of overwriting.Suggested fix
- settings_config["hooks"] = { - "PostToolUse": [ - { - "matcher": "Write|Edit", - "hooks": [{"type": "command", "command": hook_command}], - } - ] - } + hooks = settings_config.get("hooks", {}) + post_tool_use = hooks.get("PostToolUse", []) + target = next( + ( + item + for item in post_tool_use + if isinstance(item, dict) and item.get("matcher") == "Write|Edit" + ), + None, + ) + new_hook = {"type": "command", "command": hook_command} + if target is None: + post_tool_use.append({"matcher": "Write|Edit", "hooks": [new_hook]}) + else: + target_hooks = target.get("hooks") + if not isinstance(target_hooks, list): + target_hooks = [] + target_hooks.append(new_hook) + target["hooks"] = target_hooks + hooks["PostToolUse"] = post_tool_use + settings_config["hooks"] = hooks🤖 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/agents/claude_code/local_mode_strategy.py` around lines 123 - 130, When adding the PostToolUse hook, don't overwrite settings_config["hooks"]; instead merge into any existing hooks object: read settings_config.get("hooks", {}) then ensure PostToolUse is a list (create if missing), append or extend with the new dict containing matcher "Write|Edit" and hooks [{"type":"command","command": hook_command}] (avoid replacing other hook types or entries). Update the logic around settings_config["hooks"] so it preserves existing entries and only inserts/merges the new PostToolUse entry.executor/tests/test_local_capability_sync_handler.py (1)
33-37: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd type annotations to newly introduced helper/tests.
These new function signatures still miss typed fixture params and explicit return types (
-> Nonefor tests).Example pattern to apply
-def test_apply_sync_downloads_uploaded_plugin_into_store(tmp_path, monkeypatch): +def test_apply_sync_downloads_uploaded_plugin_into_store( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None:As per coding guidelines, "Type hints are required for all Python code".
Also applies to: 176-178, 320-320, 520-520, 649-649, 797-797
🤖 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/tests/test_local_capability_sync_handler.py` around lines 33 - 37, Add missing type hints to the new helper and test functions: ensure all test functions have explicit return types of -> None and all pytest fixtures and helper parameters are fully annotated (e.g., the helper create_nested_plugin_zip parameters are typed and its return -> bytes is present; apply the same pattern to other new helpers/fixtures referenced in this file). Update any untyped fixture parameters to explicit types (e.g., fixture: pytest.Fixture or concrete types like str, dict[str, str], bytes as appropriate) and add -> None to test functions so every function in the file has complete type annotations per the coding guidelines.Source: Coding guidelines
🧹 Nitpick comments (1)
wework/src/features/workbench/WorkbenchProvider.tsx (1)
1-2034: 🏗️ Heavy liftSplit
WorkbenchProvider.tsxinto smaller modules/hooks to meet repository limits.This file and the main provider function are now far beyond the repository’s size/complexity limits, which will keep making feature work riskier.
As per coding guidelines,
File size limit: If a file exceeds 1000 lines, split it into multiple sub-modulesandFunction length: Max 50 lines per function (preferred).🤖 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/features/workbench/WorkbenchProvider.tsx` around lines 1 - 2034, The WorkbenchProvider function and file are too large—split responsibilities by extracting related helper functions, reducer logic, and effectful behavior into smaller modules: move createDefaultServices, getCurrentAppPath, getTaskRouteKey and other pure helpers (isRecord, normalizeStoredModelOptions, getSubtaskResult, normalizeProcessingBlock, normalizeChatBlock, normalizeAttachmentStatus, getSubtaskAttachments, subtaskToMessage, sortSubtasksForDisplay, read/writeTaskId, read/writeLastProjectId, getTaskModelSelection, getNewChatModelSelection, isRunningTaskStatus, normalizeGuidanceError, resolveOpenedTaskProjectId, getRememberedStandaloneDeviceId, getUpgradeStatusMessage, isTerminalDeviceUpgradeStatus) into a wework/utils or wework/helpers module; extract device upgrade and timer logic (setDeviceUpgradeState, schedule/clearUpgradeStateTimer, upgradeDevice, related useEffect subscriptions) into a useDeviceUpgrade hook; move message handling and streaming effects and messageReducer usage into a useWorkbenchMessages hook (wrap dispatchMessages, sendPreparedMessage, pauseCurrentResponse, queued message flows, sendQueuedAsGuidance); move model/skill/attachment selection wiring into useWorkbenchModels/useWorkbenchSkills/useWorkbenchAttachments (or re-use existing hooks) and the createDefaultServices into a services factory module; ensure workbenchReducer and initialWorkbenchState live in their own reducers file (if not already), then refactor WorkbenchProvider to just compose these hooks, wire the WorkbenchContext value, and keep the main provider function under ~50 lines, importing the named symbols (WorkbenchProvider, workbenchReducer, messageReducer, useWorkbenchModels, useWorkbenchSkills, useWorkbenchAttachments, createDefaultServices, setDeviceUpgradeState, sendPreparedMessage, listLocalSkills) so callers and tests keep the same API.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 `@executor/hooks/rthook_version.py`:
- Around line 29-30: The current broad except Exception: that sets version =
None silently swallows unexpected errors; narrow the handler (or remove it) so
only expected failures are caught — for example, catch FileNotFoundError,
OSError, subprocess.CalledProcessError, ValueError (as appropriate for how
version is resolved) around the code that assigns version, or remove the
try/except and let the caller handle exceptions; ensure the variable version is
still assigned a sensible default (None) only for the specific, anticipated
failure modes.
In `@executor/tests/test_version_cli.py`:
- Line 95: Add a type hint to the test function parameter by changing the
signature of test_get_version_uses_env_override to accept monkeypatch:
pytest.MonkeyPatch; ensure pytest is imported in the test file (import pytest)
so the annotation resolves. This updates the function definition
(test_get_version_uses_env_override) to comply with the required type-hinting
rule.
In `@README_zh.md`:
- Line 17: The TOC anchor(s) in README_zh.md don't match the generated heading
IDs (MD051); update the fragment(s) such as "`#-快速开始`" to the exact slug used by
your Markdown renderer (e.g., remove the unexpected leading hyphen or use the
renderer's slug format for "快速开始" and other headings like "核心场景" and "工作方式"),
ensuring each [label](`#fragment`) matches the actual heading IDs so in-page links
resolve correctly.
- Around line 1-3: The README_zh.md is missing the required Markdown frontmatter
block with sidebar_position; add a YAML frontmatter block at the top of the file
(above the existing "# Wegent" heading) that includes at minimum a
sidebar_position key with an appropriate integer value (e.g., sidebar_position:
1) so the doc conforms to the “**/*.md” guideline requiring frontmatter.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 70-79: The returned object in projectWorkControls (in
ChatInput.test.tsx) contains a duplicated key devices; remove the redundant
devices entry so the object has only one devices property (ensure you keep the
intended devices value and preserve the overrides spread order), leaving
projects, devices, currentProjectId, currentStandaloneDeviceId, onSelectProject,
onSelectStandaloneDevice and ...overrides exactly once.
---
Outside diff comments:
In `@docs/zh/developer-guide/global-skill-sync.md`:
- Around line 167-173: The manifest example under "skills" uses skill_id and
store_path keyed with 1336 while the example payload earlier uses skill_id 101;
update the "skill_id" value and any store_path/keys that reference 1336 (e.g.,
the "skills" -> "browser" block and its "store_path") to use 101 so the manifest
matches the payload semantics and avoids ID confusion.
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1-1751: The test file is too large — split ChatInput.test.tsx into
focused submodules (e.g., ChatInput.composer.test.tsx, ChatInput.model.test.tsx,
ChatInput.attachments.test.tsx, ChatInput.projectWork.test.tsx) by grouping
related tests (composer/layout, model menu, local-skill/autocomplete,
attachments/image previews, project-work selection) into separate files; extract
shared helpers ControlledChatInput, projectChatControls, projectWorkControls and
common stubs (originalCreateObjectUrl, afterEach cleanup logic) into a new test
utility module (e.g., ChatInput.test.utils.tsx) and import them in each new test
file; ensure each new file imports ChatInput and necessary types, preserves the
original describe/test names, and keeps per-test mocks (vi.stubGlobal,
URL.createObjectURL) local to the tests; run the test suite and adjust any test
IDs/import paths if splitting exposes relative import issues.
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 1-2597: The test file DesktopWorkbenchLayout.test.tsx is too large
and must be split into focused suites; extract groups like sidebar/search tests,
settings/quota/environment popover tests, workspace panel tests, and device
gating tests into separate files. Move shared fixtures/helpers (baseProps,
createCloudWorkspacePanelState, renderWorkspacePanelLayout, createRect, and
top-level vi.mock calls) into a new test-utils/shared setup module and import
them in each new test file, keeping mocks (createDeviceApi, createQuotaApi,
navigator.clipboard stub) initialized once (or via a beforeEach in the shared
module). In each new test file, import DesktopWorkbenchLayout and only the
relevant helpers and then copy the related test cases (e.g., the "opens and
filters the desktop search dialog..." and "searches inside conversation
messages..." tests into sidebar-search.test.tsx; the environment/branch/commit
tests into environment-popover.test.tsx; workspace open/resize/close tests into
workspace-panels.test.tsx; device gating and project create tests into
devices-and-projects.test.tsx). Ensure test names, data-testid selectors, and
any referenced handlers (onOpenTask, onLoadEnvironmentInfo, onCreateProject,
onUpgradeDevice, etc.) are preserved and wired via the shared baseProps where
needed.
---
Duplicate comments:
In `@executor/agents/claude_code/local_mode_strategy.py`:
- Around line 123-130: When adding the PostToolUse hook, don't overwrite
settings_config["hooks"]; instead merge into any existing hooks object: read
settings_config.get("hooks", {}) then ensure PostToolUse is a list (create if
missing), append or extend with the new dict containing matcher "Write|Edit" and
hooks [{"type":"command","command": hook_command}] (avoid replacing other hook
types or entries). Update the logic around settings_config["hooks"] so it
preserves existing entries and only inserts/merges the new PostToolUse entry.
In `@executor/tests/agents/test_codex_config_builder.py`:
- Around line 88-89: The test function signatures (e.g.,
test_build_codex_config_injects_global_mcp_overrides) must include type
annotations for fixture params and an explicit return type; update the function
defs to annotate tmp_path as Path and monkeypatch as pytest.MonkeyPatch (and any
other newly added tests referenced around the same change) and add -> None to
the signature; ensure you import Path and pytest if not already imported so the
annotations are valid for the functions like
test_build_codex_config_injects_global_mcp_overrides and the other new tests
mentioned.
In `@executor/tests/test_local_capability_sync_handler.py`:
- Around line 33-37: Add missing type hints to the new helper and test
functions: ensure all test functions have explicit return types of -> None and
all pytest fixtures and helper parameters are fully annotated (e.g., the helper
create_nested_plugin_zip parameters are typed and its return -> bytes is
present; apply the same pattern to other new helpers/fixtures referenced in this
file). Update any untyped fixture parameters to explicit types (e.g., fixture:
pytest.Fixture or concrete types like str, dict[str, str], bytes as appropriate)
and add -> None to test functions so every function in the file has complete
type annotations per the coding guidelines.
---
Nitpick comments:
In `@wework/src/features/workbench/WorkbenchProvider.tsx`:
- Around line 1-2034: The WorkbenchProvider function and file are too
large—split responsibilities by extracting related helper functions, reducer
logic, and effectful behavior into smaller modules: move createDefaultServices,
getCurrentAppPath, getTaskRouteKey and other pure helpers (isRecord,
normalizeStoredModelOptions, getSubtaskResult, normalizeProcessingBlock,
normalizeChatBlock, normalizeAttachmentStatus, getSubtaskAttachments,
subtaskToMessage, sortSubtasksForDisplay, read/writeTaskId,
read/writeLastProjectId, getTaskModelSelection, getNewChatModelSelection,
isRunningTaskStatus, normalizeGuidanceError, resolveOpenedTaskProjectId,
getRememberedStandaloneDeviceId, getUpgradeStatusMessage,
isTerminalDeviceUpgradeStatus) into a wework/utils or wework/helpers module;
extract device upgrade and timer logic (setDeviceUpgradeState,
schedule/clearUpgradeStateTimer, upgradeDevice, related useEffect subscriptions)
into a useDeviceUpgrade hook; move message handling and streaming effects and
messageReducer usage into a useWorkbenchMessages hook (wrap dispatchMessages,
sendPreparedMessage, pauseCurrentResponse, queued message flows,
sendQueuedAsGuidance); move model/skill/attachment selection wiring into
useWorkbenchModels/useWorkbenchSkills/useWorkbenchAttachments (or re-use
existing hooks) and the createDefaultServices into a services factory module;
ensure workbenchReducer and initialWorkbenchState live in their own reducers
file (if not already), then refactor WorkbenchProvider to just compose these
hooks, wire the WorkbenchContext value, and keep the main provider function
under ~50 lines, importing the named symbols (WorkbenchProvider,
workbenchReducer, messageReducer, useWorkbenchModels, useWorkbenchSkills,
useWorkbenchAttachments, createDefaultServices, setDeviceUpgradeState,
sendPreparedMessage, listLocalSkills) so callers and tests keep the same API.
🪄 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: 55b34c1f-67c0-43c0-b135-8e92454d8faf
📒 Files selected for processing (43)
README.mdREADME_zh.mdbackend/app/services/device/capability_sync_service.pybackend/app/services/execution/request_builder.pybackend/tests/services/device/test_capability_sync_service.pybackend/tests/services/execution/test_request_builder_standalone_workspace.pydocs/en/developer-guide/global-skill-sync.mddocs/en/user-guide/ai-devices/local-device-support.mddocs/zh/developer-guide/global-skill-sync.mddocs/zh/user-guide/ai-devices/local-device-support.mdexecutor/agents/claude_code/config_manager.pyexecutor/agents/claude_code/local_mode_strategy.pyexecutor/agents/claude_code/standalone_chat_workspace.pyexecutor/agents/codex/config_builder.pyexecutor/hooks/rthook_version.pyexecutor/modes/local/capabilities.pyexecutor/tests/agents/test_claude_code_config_security.pyexecutor/tests/agents/test_codex_config_builder.pyexecutor/tests/agents/test_mode_strategy.pyexecutor/tests/agents/test_project_workspace_paths.pyexecutor/tests/agents/test_skill_mcp_extraction.pyexecutor/tests/agents/test_standalone_chat_workspace.pyexecutor/tests/modes/local/test_global_capabilities.pyexecutor/tests/test_local_capability_sync_handler.pyexecutor/tests/test_version_cli.pyexecutor/version.pyshared/models/execution.pywework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.test.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/composer/AddContextMenu.tsxwework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerTextarea.tsxwework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/common/ActionMenu.tsxwework/src/components/common/TextInputDialog.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/messageReducer.test.tswework/src/lib/navigation.test.tswework/src/lib/navigation.ts
💤 Files with no reviewable changes (2)
- wework/src/components/common/TextInputDialog.test.tsx
- wework/src/components/chat/MessageList.test.tsx
✅ Files skipped from review due to trivial changes (6)
- wework/src/lib/navigation.test.ts
- docs/en/user-guide/ai-devices/local-device-support.md
- wework/src/components/common/ActionMenu.tsx
- docs/zh/user-guide/ai-devices/local-device-support.md
- README.md
- docs/en/developer-guide/global-skill-sync.md
🚧 Files skipped from review as they are similar to previous changes (17)
- shared/models/execution.py
- backend/tests/services/execution/test_request_builder_standalone_workspace.py
- backend/app/services/device/capability_sync_service.py
- backend/tests/services/device/test_capability_sync_service.py
- wework/src/components/chat/composer/ComposerToolbar.tsx
- executor/tests/agents/test_claude_code_config_security.py
- executor/agents/claude_code/config_manager.py
- executor/agents/claude_code/standalone_chat_workspace.py
- executor/tests/modes/local/test_global_capabilities.py
- executor/tests/agents/test_skill_mcp_extraction.py
- wework/src/components/chat/ChatInput.tsx
- wework/src/components/chat/composer/CompactChatComposer.tsx
- wework/src/components/chat/MessageList.tsx
- executor/tests/agents/test_standalone_chat_workspace.py
- wework/src/components/chat/composer/ComposerTextarea.tsx
- executor/tests/agents/test_mode_strategy.py
- executor/modes/local/capabilities.py
🛑 Comments failed to post (5)
executor/hooks/rthook_version.py (1)
29-30:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid blind exception handling in version resolution.
Catching
Exceptionhere hides unrelated failures during the runtime hook path. Narrow the exception (or remove thetry/except) so unexpected errors are not silently swallowed.Suggested change
- except Exception: + except OSError: version = None📝 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.except OSError: version = None🧰 Tools
🪛 Ruff (0.15.15)
[warning] 29-29: Do not catch blind exception:
Exception(BLE001)
🤖 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/hooks/rthook_version.py` around lines 29 - 30, The current broad except Exception: that sets version = None silently swallows unexpected errors; narrow the handler (or remove it) so only expected failures are caught — for example, catch FileNotFoundError, OSError, subprocess.CalledProcessError, ValueError (as appropriate for how version is resolved) around the code that assigns version, or remove the try/except and let the caller handle exceptions; ensure the variable version is still assigned a sensible default (None) only for the specific, anticipated failure modes.Source: Linters/SAST tools
executor/tests/test_version_cli.py (1)
95-95:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a type hint for the
monkeypatchparameter.This test method introduces an untyped parameter in Python code.
Suggested change
+from typing import Any ... - def test_get_version_uses_env_override(self, monkeypatch) -> None: + def test_get_version_uses_env_override(self, monkeypatch: Any) -> None:As per coding guidelines, "
Type hints are required for all Python code".📝 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.def test_get_version_uses_env_override(self, monkeypatch: Any) -> None:🤖 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/tests/test_version_cli.py` at line 95, Add a type hint to the test function parameter by changing the signature of test_get_version_uses_env_override to accept monkeypatch: pytest.MonkeyPatch; ensure pytest is imported in the test file (import pytest) so the annotation resolves. This updates the function definition (test_get_version_uses_env_override) to comply with the required type-hinting rule.Source: Coding guidelines
README_zh.md (2)
1-3:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing required Markdown frontmatter block.
This file does not include the required frontmatter with
sidebar_position.Suggested fix
+--- +sidebar_position: 1 +--- + # WegentAs per coding guidelines, “
**/*.md: All documentation files MUST include frontmatter withsidebar_position...”.📝 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.--- sidebar_position: 1 --- # Wegent > 可自部署的 AI 工作台：把对话、代码、知识库、自动化和本地执行放到一个入口里。🤖 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 `@README_zh.md` around lines 1 - 3, The README_zh.md is missing the required Markdown frontmatter block with sidebar_position; add a YAML frontmatter block at the top of the file (above the existing "# Wegent" heading) that includes at minimum a sidebar_position key with an appropriate integer value (e.g., sidebar_position: 1) so the doc conforms to the “**/*.md” guideline requiring frontmatter.Source: Coding guidelines
17-17:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTOC anchor appears invalid for at least one section link.
Line 17 is flagged by markdownlint (MD051). Please align fragment(s) with the exact generated heading IDs to avoid broken in-page navigation.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 17-17: Link fragments should be valid
(MD051, link-fragments)
🤖 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 `@README_zh.md` at line 17, The TOC anchor(s) in README_zh.md don't match the generated heading IDs (MD051); update the fragment(s) such as "`#-快速开始`" to the exact slug used by your Markdown renderer (e.g., remove the unexpected leading hyphen or use the renderer's slug format for "快速开始" and other headings like "核心场景" and "工作方式"), ensuring each [label](`#fragment`) matches the actual heading IDs so in-page links resolve correctly.Source: Linters/SAST tools
wework/src/components/chat/ChatInput.test.tsx (1)
70-79:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the duplicate
deviceskey inprojectWorkControlsreturn object.
devicesis declared twice in the same object literal, which triggerslint/suspicious/noDuplicateObjectKeysand can fail CI.Suggested fix
return { projects: [], - devices, currentProjectId: undefined, currentStandaloneDeviceId: null, onSelectProject: vi.fn(), onSelectStandaloneDevice: vi.fn(), ...overrides, devices, }📝 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.return { projects: [], currentProjectId: undefined, currentStandaloneDeviceId: null, onSelectProject: vi.fn(), onSelectStandaloneDevice: vi.fn(), ...overrides, devices, }🧰 Tools
🪛 Biome (2.4.16)
[error] 72-72: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
🤖 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/ChatInput.test.tsx` around lines 70 - 79, The returned object in projectWorkControls (in ChatInput.test.tsx) contains a duplicated key devices; remove the redundant devices entry so the object has only one devices property (ensure you keep the intended devices value and preserve the overrides spread order), leaving projects, devices, currentProjectId, currentStandaloneDeviceId, onSelectProject, onSelectStandaloneDevice and ...overrides exactly once.Source: Linters/SAST tools
FicoHu
force-pushed
the
codex/local-capability-store-links
branch
from
June 11, 2026 08:53
a609232 to
b6d8dbd
Compare
This was referenced Jun 15, 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
2 participants
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 by CodeRabbit
New Features
Bug Fixes
Documentation