-
Notifications
You must be signed in to change notification settings - Fork 33
feat: Added a shared task model and updated user relationships and co… #302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a Task Sharing feature: DB migration and model for shared tasks, a SharedTaskService with AES tokenization and copy/join flows, API endpoints, frontend pages/components and i18n for public/read-only viewing and joining, Redis history reinitialization during task copy, docs, and a dependency bump. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant FE as Frontend
participant BE as Backend API
participant DB as Database
participant Redis as Redis Cache
User->>FE: Click "Share" on task
FE->>BE: POST /tasks/{id}/share
BE->>DB: Read task & subtasks
BE->>BE: AES encrypt (user_id,task_id) -> token
BE->>FE: Return {share_url, share_token}
FE->>User: Show share modal with link
User->>FE: Open public link (/shared/task?token=...)
FE->>BE: GET /api/tasks/share/public?token=...
BE->>BE: Decrypt token -> (user_id, task_id)
BE->>DB: Fetch original task & subtasks
BE->>FE: Return PublicSharedTaskResponse
FE->>User: Render read-only conversation
User->>FE: Click "Copy/Join" (auth)
FE->>BE: POST /tasks/share/join with token
BE->>BE: Decode token, validate user/team
BE->>DB: Copy task, subtasks, attachments -> new records
BE->>DB: Insert SharedTask record
BE->>Redis: Check chat history for new task
alt Redis empty
BE->>DB: Fetch completed subtasks ordered
BE->>BE: Rebuild chat history messages
BE->>Redis: Save rebuilt history
end
BE->>FE: Return new task_id
FE->>User: Navigate to /chat?taskId=new
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (21)
backend/app/services/adapters/task_kinds.py (1)
99-114: Confirm non-codetask expiration behavior and consider small hardeningThe new logic now enforces expiration only when
taskType == "code", and becausetask_typedefaults to"chat"when the label is missing, any task without ataskTypelabel (or with a non-"code"value) will never expire when appending. If the product requirement is strictly “only code tasks expire, everything else is unlimited,” this is fine, but if you ever introduce new task types or have mis-labeled code tasks, they will bypass expiration.You might want to:
- Explicitly handle unknown
taskTypevalues (e.g., log and/or treat as expiring), and/or- Add a defensive check in case
existing_task.updated_atis unexpectedlyNonebefore doing the subtraction.These would make the behavior more robust without changing the current intended semantics for
codevschat.backend/app/api/endpoints/adapter/chat.py (2)
340-380: Consider adding explicit error handling for Redis operations.While the session_manager methods handle errors internally, adding explicit try-except around the Redis initialization block would improve resilience and provide better context-specific error handling. If Redis initialization fails, the function can still proceed with task creation.
Apply this pattern:
# Initialize Redis chat history from existing subtasks if needed # This is crucial for shared tasks that were copied with historical messages if existing_subtasks: from app.services.chat.session_manager import session_manager + + try: + # Check if history exists in Redis + redis_history = await session_manager.get_chat_history(task_id) - # Check if history exists in Redis - redis_history = await session_manager.get_chat_history(task_id) - - # If Redis history is empty but we have subtasks, rebuild history from DB - if not redis_history: - logger.info(f"Initializing chat history from DB for task {task_id} with {len(existing_subtasks)} existing subtasks") - history_messages = [] - - # Sort subtasks by message_id to ensure correct order - sorted_subtasks = sorted(existing_subtasks, key=lambda s: s.message_id) - - for subtask in sorted_subtasks: - # Only include completed subtasks with results - if subtask.status == SubtaskStatus.COMPLETED: - if subtask.role == SubtaskRole.USER: - # User message - use prompt field - if subtask.prompt: - history_messages.append({ - "role": "user", - "content": subtask.prompt - }) - elif subtask.role == SubtaskRole.ASSISTANT: - # Assistant message - use result.value field - if subtask.result and isinstance(subtask.result, dict): - content = subtask.result.get("value", "") - if content: - history_messages.append({ - "role": "assistant", - "content": content - }) - - # Save to Redis if we found any history - if history_messages: - await session_manager.save_chat_history(task_id, history_messages) - logger.info(f"Initialized {len(history_messages)} messages in Redis for task {task_id}") + # If Redis history is empty but we have subtasks, rebuild history from DB + if not redis_history: + logger.info(f"Initializing chat history from DB for task {task_id} with {len(existing_subtasks)} existing subtasks") + history_messages = [] + + # Sort subtasks by message_id to ensure correct order + sorted_subtasks = sorted(existing_subtasks, key=lambda s: s.message_id) + + for subtask in sorted_subtasks: + # Only include completed subtasks with results + if subtask.status == SubtaskStatus.COMPLETED: + if subtask.role == SubtaskRole.USER: + # User message - use prompt field + if subtask.prompt: + history_messages.append({ + "role": "user", + "content": subtask.prompt + }) + elif subtask.role == SubtaskRole.ASSISTANT: + # Assistant message - use result.value field + if subtask.result and isinstance(subtask.result, dict): + content = subtask.result.get("value", "") + if content: + history_messages.append({ + "role": "assistant", + "content": content + }) + + # Save to Redis if we found any history + if history_messages: + await session_manager.save_chat_history(task_id, history_messages) + logger.info(f"Initialized {len(history_messages)} messages in Redis for task {task_id}") + except Exception as e: + logger.error(f"Failed to initialize Redis history for task {task_id}: {e}", exc_info=True) + # Continue with task creation even if Redis initialization fails
133-381: Consider refactoring to reduce function length.The function now exceeds 248 lines, well beyond the 50-line guideline. Consider extracting helper functions for better maintainability:
_create_workspacefor lines 203-229_create_task_recordfor lines 231-276_create_subtask_recordsfor lines 278-338_initialize_redis_historyfor lines 340-380This would improve readability, testability, and adherence to the single responsibility principle.
As per coding guidelines, Python functions should not exceed 50 lines (preferred maximum).
frontend/src/features/common/AuthGuard.tsx (1)
25-35: Public shared-task route is correctly exempted from auth; consider future-proofing path matchAdding
/shared/tasktoallowedPathscleanly makes the shared-task page public while keeping other routes guarded. If you later introduce subpaths (e.g.,/shared/task/something) or variant paths, consider switching to a startsWith-style check or centralizing this path in yourpathsconfig to avoid scattered literals.backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py (1)
21-48: Migration is functionally sound but consider FK constraints and index cleanupThe DDL looks consistent with the SharedTask model, but there are a few things worth tightening up:
KEY idx_shared_tasks_id (id)is redundant withPRIMARY KEY (id)and can be dropped to avoid an unnecessary secondary index.user_id,original_user_id,original_task_id, andcopied_task_idare all NOT NULL withDEFAULT 0. If any insert path forgets to set them, you’ll silently get0instead of a hard failure; removing theDEFAULT 0is usually safer for referential columns.- There are no explicit foreign key constraints on
user_id/original_user_id/original_task_id/copied_task_id. If you rely on DB-level integrity (rather than only app logic), adding FKs to the correspondingusersandtaskstables will prevent orphan rows.If you keep using raw SQL in migrations, it may also be worth standardizing a helper or commenting the expected FK targets for future maintainers.
Based on learnings, Alembic migrations should be carefully reviewed before apply.
docs/TASK_SHARING_FEATURE.md (1)
39-61: Add language specifier to fenced code blocks.Per static analysis, fenced code blocks should have a language specified for proper syntax highlighting. This applies to the flow diagram and other code blocks throughout the document.
-``` +```text 用户 A 分享任务 ↓ ...frontend/src/features/tasks/components/PublicTaskSidebar.tsx (2)
96-107: Inconsistent button text based on login state.The top button (line 59) changes text based on
isLoggedIn, but this bottom CTA always showscontinue_chat. Consider making them consistent:<Button variant="outline" size="sm" onClick={onLoginClick} className="w-full text-text-primary hover:text-text-primary hover:bg-hover" > <LogIn className="h-3.5 w-3.5 mr-2" /> - {t('shared_task.continue_chat')} + {isLoggedIn ? t('shared_task.continue_chat') : t('shared_task.login_to_continue')} </Button>
86-86: Consider using CSS variables for colors.Per coding guidelines, frontend components should use provided CSS variables for the color system. The hardcoded
blue-50 dark:bg-blue-900/20 border-blue-200 dark:border-blue-800could be replaced with semantic color variables if available.frontend/src/app/(tasks)/chat/page.tsx (1)
46-54: Consider URL encoding the token parameter.When redirecting with the token from localStorage, ensure it's properly URL-encoded to handle any special characters:
useEffect(() => { const pendingToken = localStorage.getItem('pendingTaskShare'); if (pendingToken) { // Clear the pending token localStorage.removeItem('pendingTaskShare'); // Redirect to chat page with taskShare parameter to trigger the copy modal - router.push(`/chat?taskShare=${pendingToken}`); + router.push(`/chat?taskShare=${encodeURIComponent(pendingToken)}`); } }, [router]);If the token is already URL-encoded when stored,
encodeURIComponentis still safe as it won't double-encode valid characters.frontend/src/features/tasks/components/TaskShareModal.tsx (3)
66-70: Consider using CSS variables for colors.The hardcoded
text-blue-600color should use CSS variables as per the frontend styling guidelines (e.g.,--color-primaryor similar). This ensures consistency with the design system.- <span className="text-lg font-semibold text-blue-600"> {taskTitle} </span> + <span className="text-lg font-semibold text-primary"> {taskTitle} </span>
78-81: Remove redundant empty div.The empty
<div className="mt-2"></div>is unnecessary since the followingAlertalready hasmt-2class applied. This creates duplicate spacing.<Alert variant="default" className="text-sm"> <AlertDescription>{t('shared_task.share_link_info')}</AlertDescription> </Alert> - <div className="mt-2"></div> <Alert variant="default" className="text-sm mt-2">
39-52: Fallback copy method lacks error handling.The fallback method using
document.execCommand('copy')can fail silently. The function returns a boolean indicating success, but it's not checked. Users may see a success toast even when copy fails.} catch { // Fallback to traditional method if clipboard API is not available - const textArea = document.createElement('textarea'); - textArea.value = shareUrl; - document.body.appendChild(textArea); - textArea.select(); - document.execCommand('copy'); - document.body.removeChild(textArea); - toast({ - title: t('shared_task.link_copied'), - description: t('shared_task.link_copied_desc'), - }); - onClose(); + try { + const textArea = document.createElement('textarea'); + textArea.value = shareUrl; + textArea.style.cssText = 'position:fixed;opacity:0'; + document.body.appendChild(textArea); + textArea.select(); + const success = document.execCommand('copy'); + document.body.removeChild(textArea); + if (success) { + toast({ + title: t('shared_task.link_copied'), + description: t('shared_task.link_copied_desc'), + }); + onClose(); + } else { + toast({ + variant: 'destructive', + title: t('shared_task.copy_failed'), + }); + } + } catch { + toast({ + variant: 'destructive', + title: t('shared_task.copy_failed'), + }); + } }frontend/src/features/tasks/components/MessagesArea.tsx (1)
701-708: Use translation for fallback task title.The hardcoded fallback string
'Untitled Task'should use a translation key for consistency with i18n support throughout the component.<TaskShareModal visible={showShareModal} onClose={() => setShowShareModal(false)} - taskTitle={selectedTaskDetail?.title || 'Untitled Task'} + taskTitle={selectedTaskDetail?.title || t('shared_task.untitled_task')} shareUrl={shareUrl} />backend/app/models/shared_task.py (1)
1-1: Remove unused import.
Stringis imported from SQLAlchemy but not used in this model.-from sqlalchemy import Column, Integer, String, Boolean, DateTime, ForeignKey, UniqueConstraint +from sqlalchemy import Column, Integer, Boolean, DateTime, ForeignKey, UniqueConstraintfrontend/src/features/tasks/components/TaskShareHandler.tsx (3)
52-52: Remove unused state variable.
_isLoadingis set but never used (the underscore prefix indicates it's intentionally unused). Consider removing it if not needed, or use it to show a loading indicator.- const [_isLoading, setIsLoading] = useState(false);Then remove
setIsLoading(true)on line 94 andsetIsLoading(false)on line 120, or alternatively, use_isLoadingto conditionally render a loading state in the UI.
231-248: Use CSS variables for colors.Multiple instances of hardcoded
text-blue-600should use CSS variable-based classes liketext-primaryfor consistency with the design system.- <span className="text-lg font-semibold text-blue-600"> {shareInfo.task_title} </span> + <span className="text-lg font-semibold text-primary"> {shareInfo.task_title} </span>Apply the same change to lines 241 and 243.
154-158: Consider storing share token in state.The share token is re-fetched from
searchParamshere, but since the component already fetchedshareInfousing the token, consider storing the token in state during initial load. This avoids potential edge cases where URL might be modified between modal open and copy action.This is a minor defensive coding suggestion. The current implementation works correctly in normal use cases.
backend/app/services/shared_task.py (3)
513-539: Consider exception chaining for better debugging context.The static analyzer flagged missing exception chaining. While the current implementation works, using
raise ... from Noneexplicitly indicates the exception is intentionally not chained.try: user_id = int(user_id_str) task_id = int(task_id_str) except ValueError: - raise HTTPException( + raise HTTPException( status_code=400, detail="Invalid share link format" - ) + ) from None except HTTPException: raise except Exception: - raise HTTPException( + raise HTTPException( status_code=400, detail="Invalid share link format" - ) + ) from None
243-367: Consider extracting helper methods from this large function.This method is ~125 lines, which exceeds the 50-line guideline. While the logic is cohesive (it's a single transaction), you could improve readability by extracting helpers:
_copy_subtask(...)- copy a single subtask_copy_attachments(...)- copy attachments for a subtaskThis is optional since the current structure maintains transaction integrity and the logic flows clearly.
131-131: SQLAlchemy filter pattern is correct, but consideris_(True)for linter compatibility.The
== Truecomparison is correct for SQLAlchemy filters (it generates proper SQL). The linter incorrectly flags this becauseKind.is_activeis a Column object, not a Python boolean. If you want to silence the linter, you can use:Kind.is_active.is_(True)This applies to all similar occurrences (lines 141, 181, 198, 231, 257, 393, 422, 476, 491, 548, 562).
frontend/src/app/shared/task/page.tsx (1)
53-70: Error message matching is fragile.The error handling relies on string matching against backend error messages. If backend error messages change, the UI will fall back to generic errors. Consider using error codes instead.
Backend could return structured errors with codes:
{ "detail": { "code": "INVALID_LINK", "message": "..." } }Then frontend can match on codes:
if (json?.detail?.code === 'INVALID_LINK') { setError(t('shared_task.error_invalid_link')); }This makes the contract explicit and less prone to breakage from message changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor_manager/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py(1 hunks)backend/app/api/endpoints/adapter/chat.py(3 hunks)backend/app/api/endpoints/adapter/tasks.py(10 hunks)backend/app/core/config.py(1 hunks)backend/app/models/shared_task.py(1 hunks)backend/app/models/user.py(1 hunks)backend/app/schemas/shared_task.py(1 hunks)backend/app/services/adapters/task_kinds.py(1 hunks)backend/app/services/shared_task.py(1 hunks)docs/TASK_SHARING_FEATURE.md(1 hunks)executor_manager/pyproject.toml(1 hunks)frontend/src/apis/tasks.ts(2 hunks)frontend/src/app/(tasks)/chat/page.tsx(7 hunks)frontend/src/app/(tasks)/code/page.tsx(3 hunks)frontend/src/app/shared/task/page.tsx(1 hunks)frontend/src/features/common/AuthGuard.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(6 hunks)frontend/src/features/tasks/components/PublicTaskSidebar.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareHandler.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareModal.tsx(1 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/common/AuthGuard.tsxbackend/app/models/user.pybackend/app/core/config.pyfrontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxbackend/app/schemas/shared_task.pyfrontend/src/app/shared/task/page.tsxbackend/app/api/endpoints/adapter/chat.pybackend/app/services/adapters/task_kinds.pyfrontend/src/apis/tasks.tsbackend/app/models/shared_task.pyfrontend/src/app/(tasks)/chat/page.tsxbackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/services/shared_task.pybackend/app/api/endpoints/adapter/tasks.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/common/AuthGuard.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/apis/tasks.tsfrontend/src/app/(tasks)/chat/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/common/AuthGuard.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/apis/tasks.tsfrontend/src/app/(tasks)/chat/page.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/common/AuthGuard.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/common/AuthGuard.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/models/user.pybackend/app/core/config.pybackend/app/schemas/shared_task.pybackend/app/api/endpoints/adapter/chat.pybackend/app/services/adapters/task_kinds.pybackend/app/models/shared_task.pybackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/services/shared_task.pybackend/app/api/endpoints/adapter/tasks.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/user.pybackend/app/models/shared_task.py
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsx
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/shared_task.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/task_kinds.pybackend/app/services/shared_task.py
**/backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
🧠 Learnings (1)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
🧬 Code graph analysis (6)
frontend/src/features/tasks/components/PublicTaskSidebar.tsx (2)
frontend/src/components/ui/button.tsx (1)
Button(55-55)frontend/e2e/utils/auth.ts (1)
isLoggedIn(94-110)
frontend/src/features/tasks/components/TaskShareHandler.tsx (2)
frontend/src/apis/tasks.ts (1)
TaskShareInfo(91-96)frontend/src/features/tasks/components/ModelSelector.tsx (3)
allBotsHavePredefinedModel(83-100)DEFAULT_MODEL_NAME(40-40)ModelSelector(102-606)
frontend/src/features/tasks/components/MessagesArea.tsx (2)
frontend/src/apis/tasks.ts (1)
taskApis(130-261)frontend/src/features/tasks/components/TaskShareModal.tsx (1)
TaskShareModal(22-98)
backend/app/schemas/shared_task.py (1)
frontend/src/apis/tasks.ts (6)
TaskShareInfo(91-96)TaskShareResponse(86-89)JoinSharedTaskRequest(98-103)JoinSharedTaskResponse(105-108)PublicSubtaskData(110-118)PublicSharedTaskResponse(120-126)
backend/app/api/endpoints/adapter/chat.py (3)
backend/app/services/chat/session_manager.py (2)
get_chat_history(56-82)save_chat_history(84-115)backend/app/models/subtask.py (2)
SubtaskStatus(17-23)SubtaskRole(26-28)backend/app/schemas/subtask.py (2)
SubtaskStatus(25-31)SubtaskRole(34-36)
backend/app/api/endpoints/adapter/tasks.py (4)
backend/app/schemas/shared_task.py (5)
JoinSharedTaskRequest(27-33)JoinSharedTaskResponse(36-40)PublicSharedTaskResponse(80-87)TaskShareInfo(11-17)TaskShareResponse(20-24)backend/app/services/shared_task.py (4)
share_task(188-215)get_share_info(217-241)get_public_shared_task(508-604)join_shared_task(369-468)backend/app/models/user.py (1)
User(14-38)backend/app/models/kind.py (1)
Kind(25-42)
🪛 markdownlint-cli2 (0.18.1)
docs/TASK_SHARING_FEATURE.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
259-259: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
318-318: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
459-459: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
backend/app/services/shared_task.py
90-90: Do not catch blind exception: Exception
(BLE001)
131-131: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
141-141: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
164-164: Do not catch blind exception: Exception
(BLE001)
181-181: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
198-198: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
231-231: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
257-257: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
393-393: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
422-422: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
476-476: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
491-491: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
518-521: Abstract raise to an inner function
(TRY301)
529-532: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Do not catch blind exception: Exception
(BLE001)
536-539: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
548-548: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
562-562: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
backend/app/api/endpoints/adapter/tasks.py
330-330: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
331-331: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
353-353: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
365-365: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
378-378: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
379-379: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
395-395: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
412-412: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (28)
executor_manager/pyproject.toml (1)
16-16: httpx 0.27.0 is available and stable; loose version constraint is acceptable.httpx 0.27.0 is published on PyPI (released February 2024) with no known security vulnerabilities (CVE-2021-41945 only affected versions <0.23.0). The minimum version constraint
>=0.27.0differs from other pinned dependencies in the file, but this approach is intentional for allowing compatible minor/patch updates. No action required.backend/app/api/endpoints/adapter/chat.py (2)
133-140: LGTM: Async conversion is correct.The function signature change to
async defis appropriate given the Redis operations added in the history initialization block.
473-475: All call sites of_create_task_and_subtaskscorrectly use await.The codebase contains only one call site at line 473, which correctly awaits the async function. No further action needed.
frontend/src/i18n/locales/zh-CN/chat.json (1)
41-48: Shared-task chat translations look consistentThe new
shared_taskblock mirrors the English keys and the JSON structure is valid. No issues from a code perspective.backend/app/models/user.py (1)
28-30: Newshared_tasksrelationship wiring looks correct; ensure counterpart mapping matchesDefining
shared_tasks = relationship("SharedTask", foreign_keys="[SharedTask.user_id]", back_populates="user")is the right approach whenSharedTaskhas multiple FKs tousers. Just double-check that:
SharedTaskdefines auser = relationship("User", foreign_keys="[SharedTask.user_id]", back_populates="shared_tasks")(or equivalent), and- The
user_idcolumn onSharedTaskis actually declared as a foreign key tousers.id.That will keep the ORM join path unambiguous and avoid mapper-configuration warnings.
backend/app/core/config.py (1)
73-89: Team share base URL now targets/chat; verify this matches frontend routingPointing
TEAM_SHARE_BASE_URLtohttp://localhost:3000/chatmakes sense if shared-team links are meant to land directly on the chat page. Please confirm that:
- The frontend expects team-share links at
/chatwith query paramteamShare, and- Existing environments that override this via
.envare updated as needed so old links (if any) still behave as expected.This avoids subtle mismatches between generated URLs and actual entry points.
frontend/src/i18n/locales/en/chat.json (1)
41-48: English shared-task chat strings are well-formedThe new
shared_taskblock is consistent with the feature’s intent and mirrors the zh-CN structure. No issues from a code/i18n-structure standpoint.frontend/src/app/(tasks)/code/page.tsx (1)
57-63: Share button lift-up via callback is clean and idiomaticCapturing the share button via
onShareButtonRenderand rendering{shareButton}insideTopNavigationis a straightforward way to keep layout concerns in the page while lettingChatAreaown the button implementation. TheReact.ReactNodestate typing and callback signature are appropriate, and the new prop usage onChatAreais clear.As long as
onShareButtonRenderis optional (or provided in all call sites), this looks good.Also applies to: 176-185, 203-209
frontend/src/i18n/locales/en/common.json (1)
604-654: Well-structured localization namespace for the sharing feature.The
shared_tasknamespace provides comprehensive coverage for the sharing workflow including read-only views, login prompts, copy actions, and error states. Key naming is consistent with the existing patterns in this file.docs/TASK_SHARING_FEATURE.md (1)
379-395: Good documentation of security considerations and future improvements.The documentation clearly identifies current limitations (no link expiration, no access logs, no rate limiting, no revocation) and provides actionable improvement suggestions. This transparency helps future development planning.
frontend/src/features/tasks/components/ChatArea.tsx (1)
45-45: Clean prop-forwarding implementation for share button rendering.The optional
onShareButtonRendercallback enables parent components to control where the share button is displayed without coupling the rendering logic. This follows a good composition pattern.Also applies to: 54-55, 814-814
frontend/src/app/(tasks)/chat/page.tsx (1)
114-116: Good integration of TaskShareHandler with proper Suspense boundary.The TaskShareHandler is correctly wrapped in Suspense and connected to
refreshTasksto update the task list after copying. The share button rendering callback cleanly lifts the button to TopNavigation.Also applies to: 155-155
frontend/src/features/tasks/components/MessagesArea.tsx (2)
216-241: LGTM!The
handleShareTaskfunction is properly memoized withuseCallback, has appropriate error handling with user-friendly toast notifications, and correctly manages loading state withisSharing.
540-565: LGTM!The share button is properly memoized to prevent infinite re-renders, and the
useEffectcorrectly delegates rendering to the parent component. The conditional rendering based on task existence and message count is appropriate.backend/app/api/endpoints/adapter/tasks.py (2)
388-421: LGTM - Static analysis false positive.The
Kind.is_active == Truepattern is correct for SQLAlchemy boolean column filtering. Using justKind.is_activewould not work as expected in SQLAlchemy filter clauses. The Ruff E712 hints are false positives in this context.The team validation logic correctly ensures the user owns the specified team or falls back to their first active team.
327-347: LGTM!The
share_taskendpoint correctly validates task ownership before delegating to the service layer. The separation of concerns between validation (endpoint) and business logic (service) is well-maintained.backend/app/models/shared_task.py (1)
7-49: LGTM!The
SharedTaskmodel is well-designed with:
- Appropriate foreign keys with CASCADE delete behavior
- Indexes on frequently queried columns
- A unique constraint preventing duplicate task copies
- Clear docstring explaining the model's purpose
- Proper relationships defined for ORM navigation
frontend/src/features/tasks/components/TaskShareHandler.tsx (2)
86-125: LGTM!The effect correctly:
- Guards against missing token
- Fetches share info and teams in parallel for performance
- Handles errors with user-friendly toast notifications
- Cleans up URL parameters on error
- Auto-selects the first team for better UX
381-400: LGTM!The modal action buttons properly handle:
- Disabling during copy operation
- Preventing self-share copies
- Enforcing model selection when required
- Showing appropriate loading text
backend/app/schemas/shared_task.py (1)
1-88: LGTM! Well-structured Pydantic schemas for the task-sharing feature.The schemas are well-defined with:
- Proper type hints on all fields
- English docstrings as per coding guidelines
- Correct use of
ConfigDict(from_attributes=True)for ORM mapping inSharedTaskInDB- Alignment with frontend TypeScript interfaces
frontend/src/apis/tasks.ts (3)
85-126: LGTM! Well-defined TypeScript interfaces for task sharing.The interfaces properly align with the backend Pydantic schemas and follow TypeScript conventions with proper optional field markers.
206-214: Verify the intentional swap of branch parameters.The source and target branch values are swapped when constructing the query parameters. This appears intentional but creates a confusing mismatch between the
BranchDiffRequestinterface field names and what's actually sent to the API.Consider either:
- Renaming the interface fields to reflect the actual API semantics
- Adding a comment explaining why the swap is necessary
233-260: LGTM! Correct use of native fetch for unauthenticated public endpoint.Using native
fetchinstead ofapiClientis the right approach to bypass the authentication interceptor for public viewing. The error handling properly attempts to parse JSON error details with a safe fallback.backend/app/services/shared_task.py (2)
103-165: LGTM! Token decoding with graceful error handling.The method properly handles malformed tokens by returning
Noneand includes database validation when a session is available. The placeholder names for thedb=Nonecase are acceptable since the token itself is encrypted.
369-468: LGTM! Robust join logic with duplicate prevention.The method properly:
- Prevents users from copying their own tasks
- Handles both active and inactive existing share records
- Reuses records to avoid unique constraint violations
- Validates the original task still exists before copying
frontend/src/app/shared/task/page.tsx (3)
37-38: LGTM! Synchronous token check is appropriate here.The
getToken()call is synchronous (likely reading from cookies/localStorage), so computingisLoggedIninline is fine.
262-317: LGTM! Well-structured message rendering.The subtask rendering properly:
- Uses unique
keyprop (subtask.id)- Applies secure link attributes (
target="_blank"withnoopener noreferrer)- Integrates theme context with the Markdown editor
- Differentiates user vs AI message styling
341-358: LGTM! Correct use of Suspense for Next.js App Router.Wrapping
SharedTaskContentinSuspenseis required since it usesuseSearchParams(). The fallback provides appropriate loading feedback.
| @router.get("/share/info", response_model=TaskShareInfo) | ||
| def get_task_share_info( | ||
| share_token: str = Query(..., description="Share token from URL"), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Get task share information from share token. | ||
| This endpoint doesn't require authentication, so anyone with the link can view. | ||
| """ | ||
| return shared_task_service.get_share_info(db=db, share_token=share_token) | ||
|
|
||
|
|
||
| @router.get("/share/public", response_model=PublicSharedTaskResponse) | ||
| def get_public_shared_task( | ||
| token: str = Query(..., description="Share token from URL"), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Get public shared task data for read-only viewing. | ||
| This endpoint doesn't require authentication - anyone with the link can view. | ||
| Only returns public data (no sensitive information like team config, bot details, etc.) | ||
| """ | ||
| return shared_task_service.get_public_shared_task(db=db, share_token=token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "router\.(get|post)" backend/app/api/endpoints/adapter/tasks.py | head -40Repository: wecode-ai/Wegent
Length of output: 828
🌐 Web query:
FastAPI route matching order path parameter vs literal routes
💡 Result:
Short answer: FastAPI (via Starlette) matches routes in the order they are added. More specific literal routes (e.g. /users/me) must be declared before general path-parameter routes (e.g. /users/{user_id}), otherwise the parameter route will capture the literal. The same applies to catch‑all patterns — place them last. [1][2]
Example: declare @app.get("/users/me") before @app.get("/users/{user_id}"). [1]
Sources: FastAPI Path Parameters docs and Starlette routing notes. [1][2]
🏁 Script executed:
# Verify the exact line numbers and structure to understand the full context
sed -n '150,160p' backend/app/api/endpoints/adapter/tasks.pyRepository: wecode-ai/Wegent
Length of output: 371
Move /share/* routes before /{task_id} route.
The routes /share/info (line 350), /share/public (line 362), and /share/join (line 375) must be defined before the /{task_id} route (line 155). FastAPI matches routes in definition order, and the parameterized /{task_id} route will capture "share" as a task_id before the literal /share/* routes are evaluated, preventing those endpoints from being reached.
🧰 Tools
🪛 Ruff (0.14.7)
353-353: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
365-365: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/tasks.py around lines 350 to 372, the
literal routes /share/info and /share/public (and /share/join at ~line 375) are
defined after the parameterized /{task_id} route (around line 155), so FastAPI
will match /{task_id} first and capture "share" as a task_id; move the /share/*
route definitions to appear before the /{task_id} route definition so the
literal /share paths are matched first; update file order by cutting the three
/share route blocks and pasting them above the /{task_id} route, keeping imports
and dependencies intact.
| created_at = Column(DateTime, default=datetime.utcnow, nullable=False) | ||
| updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
datetime.utcnow deprecated Python 3.12
💡 Result:
Yes — in Python 3.12 datetime.datetime.utcnow() (and utcfromtimestamp()) is deprecated. Use timezone-aware UTC datetimes instead, e.g.:
from datetime import datetime, timezone
now = datetime.now(timezone.utc)
The deprecation is documented in Python 3.12 “What’s New” / deprecations notes. [1][2]
Sources:
[1] What’s New In Python 3.12 (datetime deprecation).
[2] Deprecations — Python 3.12 (utcnow / utcfromtimestamp).
🏁 Script executed:
# First, let's examine the shared_task.py file to understand the current state
find . -path "*/backend/app/models/shared_task.py" -type fRepository: wecode-ai/Wegent
Length of output: 95
🏁 Script executed:
# Check if the file exists and view its content
cat -n backend/app/models/shared_task.py 2>/dev/null || echo "File not found at expected path, searching..."Repository: wecode-ai/Wegent
Length of output: 2529
🏁 Script executed:
# Search for shared_task.py file
fd -t f "shared_task.py" backend/Repository: wecode-ai/Wegent
Length of output: 164
🏁 Script executed:
# Search for datetime.utcnow usage in backend
rg "utcnow" backend/ --type py -B 2 -A 2Repository: wecode-ai/Wegent
Length of output: 4168
Replace deprecated datetime.utcnow() with timezone-aware alternative.
datetime.utcnow() is deprecated as of Python 3.12. Use datetime.now(timezone.utc) wrapped in a lambda to ensure timezone-aware UTC datetimes. Add timezone to the import statement.
-from datetime import datetime
+from datetime import datetime, timezone
from sqlalchemy import Column, Integer, String, Boolean, DateTime, ForeignKey, UniqueConstraint
from sqlalchemy.orm import relationship
from app.db.base import Base
class SharedTask(Base):
# Timestamps
- created_at = Column(DateTime, default=datetime.utcnow, nullable=False)
- updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False)
+ created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), nullable=False)
+ updated_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc), nullable=False)📝 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.
| created_at = Column(DateTime, default=datetime.utcnow, nullable=False) | |
| updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False) | |
| from datetime import datetime, timezone | |
| from sqlalchemy import Column, Integer, String, Boolean, DateTime, ForeignKey, UniqueConstraint | |
| from sqlalchemy.orm import relationship | |
| from app.db.base import Base | |
| class SharedTask(Base): | |
| # Timestamps | |
| created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), nullable=False) | |
| updated_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc), nullable=False) |
🤖 Prompt for AI Agents
In backend/app/models/shared_task.py around lines 34 to 35, the model uses the
deprecated naive datetime.utcnow(); replace it with timezone-aware calls by
importing timezone from datetime and using lambdas: set default=lambda:
datetime.now(timezone.utc) and onupdate=lambda: datetime.now(timezone.utc) so
SQLAlchemy stores UTC-aware datetimes; update the import line to include
timezone.
| def __init__(self): | ||
| # Initialize AES key and IV from settings (reuse team share settings) | ||
| self.aes_key = settings.SHARE_TOKEN_AES_KEY.encode("utf-8") | ||
| self.aes_iv = settings.SHARE_TOKEN_AES_IV.encode("utf-8") | ||
|
|
||
| def _aes_encrypt(self, data: str) -> str: | ||
| """Encrypt data using AES-256-CBC""" | ||
| cipher = Cipher( | ||
| algorithms.AES(self.aes_key), | ||
| modes.CBC(self.aes_iv), | ||
| backend=default_backend(), | ||
| ) | ||
| encryptor = cipher.encryptor() | ||
|
|
||
| # Pad the data to 16-byte boundary (AES block size) | ||
| padder = padding.PKCS7(128).padder() | ||
| padded_data = padder.update(data.encode("utf-8")) + padder.finalize() | ||
|
|
||
| # Encrypt the data | ||
| encrypted_bytes = encryptor.update(padded_data) + encryptor.finalize() | ||
|
|
||
| # Return base64 encoded encrypted data | ||
| return base64.b64encode(encrypted_bytes).decode("utf-8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Static IV weakens AES-CBC encryption.
Using the same IV for every encryption operation means identical plaintexts will produce identical ciphertexts. An attacker observing multiple share tokens could identify when the same user shares the same task repeatedly.
Consider one of these approaches:
- Generate a random IV per encryption and prepend it to the ciphertext (common pattern)
- Use AES-GCM which handles nonces more safely and provides authentication
- If share tokens don't require secrecy (only integrity), consider HMAC-based tokens instead
def _aes_encrypt(self, data: str) -> str:
"""Encrypt data using AES-256-CBC"""
+ # Generate a random IV for each encryption
+ import os
+ iv = os.urandom(16)
cipher = Cipher(
algorithms.AES(self.aes_key),
- modes.CBC(self.aes_iv),
+ modes.CBC(iv),
backend=default_backend(),
)
encryptor = cipher.encryptor()
# Pad the data to 16-byte boundary (AES block size)
padder = padding.PKCS7(128).padder()
padded_data = padder.update(data.encode("utf-8")) + padder.finalize()
# Encrypt the data
encrypted_bytes = encryptor.update(padded_data) + encryptor.finalize()
- # Return base64 encoded encrypted data
- return base64.b64encode(encrypted_bytes).decode("utf-8")
+ # Prepend IV to ciphertext and return base64 encoded
+ return base64.b64encode(iv + encrypted_bytes).decode("utf-8")Note: You'll also need to update _aes_decrypt to extract the IV from the first 16 bytes.
Committable suggestion skipped: line range outside the PR's diff.
| if (subtask.status === 'COMPLETED') { | ||
| return 'Task completed'; | ||
| } else if (subtask.status === 'FAILED') { | ||
| return 'Task failed'; | ||
| } | ||
|
|
||
| return 'Processing...'; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded English fallback strings should use i18n.
These status messages are displayed to users but aren't internationalized.
if (subtask.status === 'COMPLETED') {
- return 'Task completed';
+ return t('shared_task.status_completed');
} else if (subtask.status === 'FAILED') {
- return 'Task failed';
+ return t('shared_task.status_failed');
}
- return 'Processing...';
+ return t('shared_task.status_processing');
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/app/shared/task/page.tsx around lines 117 to 124 the status
messages are hardcoded in English; replace them with translation keys by using
the app's i18n mechanism (e.g., call the translation function from
useTranslation or the project's i18n util) instead of literal strings, e.g.
t('task.status.completed'), t('task.status.failed'),
t('task.status.processing'), and add those keys and translations to the locale
JSONs for each supported language.
| if (isLoading) { | ||
| return ( | ||
| <div className="flex smart-h-screen bg-base text-text-primary box-border"> | ||
| {/* Sidebar placeholder during loading */} | ||
| <ResizableSidebar> | ||
| <div className="flex items-center justify-center h-full"> | ||
| <div className="text-xs text-text-muted">Loading...</div> | ||
| </div> | ||
| </ResizableSidebar> | ||
|
|
||
| <div className="flex-1 flex flex-col min-w-0"> | ||
| <TopNavigation activePage="chat" variant="standalone" showLogo> | ||
| <GithubStarButton /> | ||
| </TopNavigation> | ||
| <div className="flex-1 flex items-center justify-center"> | ||
| <div className="text-center"> | ||
| <div className="animate-spin rounded-full h-12 w-12 border-b-2 border-primary mx-auto mb-4"></div> | ||
| <p className="text-text-muted">Loading shared conversation...</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loading text should also use i18n.
The "Loading..." and "Loading shared conversation..." strings at lines 132, 143, and 349 are hardcoded. Consider using translation keys for consistency.
🤖 Prompt for AI Agents
In frontend/src/app/shared/task/page.tsx around lines 126 to 149 (and also
update the occurrence at line 349), the hardcoded strings "Loading..." and
"Loading shared conversation..." should be replaced with i18n translation keys;
import and use the app's translation hook (e.g. useTranslation or t from
next-intl/react) at the top of the component, replace the literal strings with
t('loading') and t('loading.sharedConversation') (or your project's existing
keys), and ensure corresponding keys are added to the locale files; keep markup
and classes unchanged while using the translated strings.
| alt="Weibo Logo" | ||
| width={20} | ||
| height={20} | ||
| className="object-container" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible typo in Tailwind class name.
object-container is not a valid Tailwind CSS class. Did you mean object-contain?
- className="object-container"
+ className="object-contain"📝 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.
| className="object-container" | |
| className="object-contain" |
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/PublicTaskSidebar.tsx at line 43, the
className "object-container" is a typo (not a valid Tailwind class); replace it
with the correct Tailwind utility "object-contain" (or another intended object-*
utility such as object-cover) so the CSS behavior works as expected, updating
the className value accordingly.
| "error_load_failed_desc": "Unable to load the shared task. Please try again later.", | ||
| "go_home": "Go to Chat", | ||
| "share_success_title": "Share Task Successfully", | ||
| "share_success_message": "Your task {taskTitle} has been shared successfully!", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent interpolation syntax.
This line uses {taskTitle} but other translations in this file use {{variable}} syntax (e.g., line 177: "Shared by {{author}}"). Verify this matches your i18n library's expected format.
- "share_success_message": "Your task {taskTitle} has been shared successfully!",
+ "share_success_message": "Your task {{taskTitle}} has been shared successfully!",📝 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.
| "share_success_message": "Your task {taskTitle} has been shared successfully!", | |
| "share_success_message": "Your task {{taskTitle}} has been shared successfully!", |
🤖 Prompt for AI Agents
In frontend/src/i18n/locales/en/common.json around line 623, the interpolation
uses "{taskTitle}" which is inconsistent with the project's "{{variable}}"
syntax used elsewhere; replace "{taskTitle}" with "{{taskTitle}}" to match the
i18n library's expected format (and scan other entries for similar single-brace
occurrences), then run the app/tests to confirm translation interpolation still
works.
| "shared_task": { | ||
| "continue_chat": "继续聊天", | ||
| "login_to_continue": "登录以继续", | ||
| "read_only_view": "只读视图", | ||
| "login_prompt": "登录以将此任务复制到您的工作区并继续对话。", | ||
| "continue_prompt": "点击上方按钮将此任务复制到您的工作区并继续对话。", | ||
| "want_to_continue": "想要继续这个对话吗?", | ||
| "copy_and_chat": "将此任务复制到您的工作区并继续聊天", | ||
| "shared_task": "分享的任务", | ||
| "shared_by": "分享者", | ||
| "read_only_notice": "这是一个只读的分享对话。要将其复制到您的任务列表并继续对话,请先登录", | ||
| "error_invalid_link": "无效的分享链接", | ||
| "error_invalid_link_desc": "分享链接格式无效,请检查链接后重试。", | ||
| "error_task_deleted": "任务已不可用", | ||
| "error_task_deleted_desc": "此分享任务已被所有者删除,无法访问。", | ||
| "error_load_failed": "加载失败", | ||
| "error_load_failed_desc": "无法加载分享任务,请稍后重试。", | ||
| "go_home": "返回聊天页", | ||
| "share_success_title": "任务分享成功", | ||
| "share_success_message": "您的任务 {taskTitle} 已成功分享!", | ||
| "share_success_message_prefix": "您的任务", | ||
| "share_success_message_suffix": "已成功分享!", | ||
| "share_link_info": "任何拥有此链接的人都可以查看此任务的对话历史。", | ||
| "share_continue_info": "其他人打开链接后,可以将此任务复制到他们的任务列表并继续对话。", | ||
| "copy_link": "复制链接", | ||
| "link_copied": "分享链接已复制!", | ||
| "link_copied_desc": "您现在可以将此链接分享给他人", | ||
| "share_task": "分享任务", | ||
| "sharing": "分享中...", | ||
| "no_task_selected": "未选择任务", | ||
| "no_task_selected_desc": "请选择一个任务进行分享", | ||
| "share_failed": "分享任务失败", | ||
| "share_failed_desc": "请重试", | ||
| "handler_load_failed": "获取分享任务信息失败", | ||
| "handler_select_team": "请选择一个团队", | ||
| "handler_copy_success": "任务复制成功!", | ||
| "handler_copy_success_desc": "已添加到您的任务列表", | ||
| "handler_self_task_title": "这是您自己的任务", | ||
| "handler_self_task_desc": "您不能复制自己分享的任务", | ||
| "handler_modal_title": "分享的任务", | ||
| "handler_is_your_own_task": "是您自己的任务,无法复制。", | ||
| "handler_shared_by": "分享了任务", | ||
| "handler_with_you": "给您", | ||
| "handler_copy_description": "复制此任务将把", | ||
| "handler_copy_description_suffix": "及其所有对话历史添加到您的任务列表。您可以继续对话。", | ||
| "handler_select_team_label": "选择团队", | ||
| "handler_no_teams": "没有可用的团队", | ||
| "handler_create_team_hint": "您需要至少创建一个团队才能复制此任务。", | ||
| "handler_copying": "复制中...", | ||
| "handler_copy_to_tasks": "复制到我的任务" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix placeholder syntax for shared-task success message
Most placeholders in this file use the {{...}} syntax (e.g., {{teamName}}), but share_success_message uses {taskTitle}:
"share_success_message": "您的任务 {taskTitle} 已成功分享!",This will likely render the literal {taskTitle} instead of the task title. Recommend aligning with the existing pattern:
- "share_success_message": "您的任务 {taskTitle} 已成功分享!",
+ "share_success_message": "您的任务 {{taskTitle}} 已成功分享!",Other keys and structure in the shared_task block look good.
🤖 Prompt for AI Agents
In frontend/src/i18n/locales/zh-CN/common.json around lines 605 to 655, the
share_success_message uses single-brace placeholder "{taskTitle}" which will
render literally; change it to use the project's double-brace syntax
"{{taskTitle}}" so the task title is interpolated (i.e., update "您的任务
{taskTitle} 已成功分享!" → use "{{taskTitle}}" in the same sentence).
5ba8b47 to
c245627
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/apis/tasks.ts (1)
205-214: Remove the parameter swap ingetBranchDiff—it inverts branch semantics and causes incorrect diffs.The function currently swaps
source_branchandtarget_branchbefore sending to the backend:query.append('source_branch', params.target_branch); query.append('target_branch', params.source_branch);However, the caller in
Workbench.tsxpasses parameters with natural semantics (source branch = feature branch, target branch = base branch), and the backend API expects these semantics consistently. This swap inverts the diff direction, flipping ahead/behind counts and changed files logic.Fix by removing the swap:
query.append('source_branch', params.source_branch); query.append('target_branch', params.target_branch);
♻️ Duplicate comments (6)
backend/app/models/shared_task.py (1)
34-35: Replace deprecateddatetime.utcnow()with timezone-aware alternative.This issue was previously flagged.
datetime.utcnow()is deprecated as of Python 3.12. Usedatetime.now(timezone.utc)instead.-from datetime import datetime +from datetime import datetime, timezone- created_at = Column(DateTime, default=datetime.utcnow, nullable=False) - updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False) + created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), nullable=False) + updated_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc), nullable=False)frontend/src/app/shared/task/page.tsx (4)
117-124: Hardcoded English status strings should use i18n.This was previously flagged. The status messages 'Task completed', 'Task failed', and 'Processing...' should use translation keys.
if (subtask.status === 'COMPLETED') { - return 'Task completed'; + return t('shared_task.status_completed'); } else if (subtask.status === 'FAILED') { - return 'Task failed'; + return t('shared_task.status_failed'); } - return 'Processing...'; + return t('shared_task.status_processing'); };
126-149: Loading text should use i18n.This was previously flagged. The "Loading..." and "Loading shared conversation..." strings should use translation keys for consistency.
232-235: Redundant duplicate spans with identical content.This was previously flagged. Both spans display the same text. Simplify to a single element.
<LogIn className="w-4 h-4" /> - <span className="hidden sm:inline">{t('shared_task.continue_chat')}</span> - <span className="sm:hidden">{t('shared_task.continue_chat')}</span> + {t('shared_task.continue_chat')} </Button>
345-350: Suspense fallback also has hardcoded "Loading..." text.The Suspense fallback at line 349 should also use i18n for consistency.
- <p className="text-text-muted">Loading...</p> + <p className="text-text-muted">{/* Consider using a static loader or pre-rendered text */}</p>Note: Since
useTranslationrequires client-side context and is unavailable in Suspense fallback, consider using a simple spinner without text, or a pre-rendered loading indicator.backend/app/services/shared_task.py (1)
39-61: Security concern: Static IV weakens AES-CBC encryption.This was previously flagged. Using the same IV for every encryption means identical plaintexts produce identical ciphertexts, allowing an attacker to identify patterns. Generate a random IV per encryption and prepend it to the ciphertext.
🧹 Nitpick comments (10)
backend/app/services/adapters/task_kinds.py (1)
98-120: Consider extracting expiration logic to a helper method.The expiration checking logic (lines 98-120) involves multiple conditions and responsibilities, making it harder to understand the flow of
create_task_or_append. Consider extracting this logic to a dedicated helper method for improved readability and testability.Example helper method structure:
def _should_check_task_expiration( self, task_crd: Task, existing_task: Kind ) -> Tuple[bool, int]: """ Determine if task expiration should be checked and return expire_hours. Returns: Tuple of (should_check, expire_hours) """ task_type = ( (task_crd.metadata.labels and task_crd.metadata.labels.get("taskType")) or "chat" ) expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS if task_type == "code": expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS task_shell_source = ( (task_crd.chat_shell and task_crd.chat_shell.labels and task_crd.chat_shell.labels.get("source")) or None ) # Skip expiration check for chat_shell source should_check = task_shell_source != CHAT_SHELL_SOURCE return should_check, expire_hoursThen use it in the main method:
should_check, expire_hours = self._should_check_task_expiration( task_crd, existing_task ) if should_check: if (datetime.now() - existing_task.updated_at).total_seconds() > expire_hours * 3600: task_type = ( (task_crd.metadata.labels and task_crd.metadata.labels.get("taskType")) or "chat" ) raise HTTPException( status_code=400, detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", )backend/app/api/endpoints/adapter/chat.py (1)
340-379: Redis history reconstruction is correct but could be more tolerant of legacy result shapes.The history bootstrap from
existing_subtaskslooks sound and safely guarded, butASSISTANTmessages are only included whensubtask.resultis adictwith a non-empty"value"field. If you have older subtasks whereresultis a plain string or a differently shaped object, those replies will be silently dropped from Redis history.Consider extending the extraction logic to handle:
resultas a plainstrresultdicts where the actual content might live under a different key (or nested) but is still what the chat UI displaysThat would make history reconstruction more robust across schema evolutions while keeping the current behavior for the new format.
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py (1)
21-43: Schema is workable; consider adding foreign keys and time-based indexes.The
shared_tasksDDL covers the needed fields and uniqueness, but:
- There are no foreign key constraints tying
user_id,original_user_id,original_task_id, andcopied_task_idback to their parent tables, so referential integrity is enforced only in application code.- If you’ll query by recency (e.g., “recent shares”), adding indexes on
created_at/updated_atcould help.If operationally acceptable, consider amending the migration to add FKs and time-based indexes for better data integrity and query performance. Based on learnings, Alembic migrations are a good place to enforce these guarantees once.
docs/TASK_SHARING_FEATURE.md (1)
39-75: Tighten Markdown formatting (code fence languages and emphasis-as-heading).markdownlint is flagging a few style issues here:
- Fenced code blocks without a language (e.g., at the flow diagrams and sample requests/responses). Consider annotating them with an appropriate language (
sql,bash,ts,text, etc.) so syntax highlighters and linters work better.- Several sections use bold/emphasis instead of actual headings (MD036). Converting those to proper
###/####headings will improve structure and navigation.These are non-functional, but cleaning them up will keep docs tooling (preview, lint, search) happier.
Also applies to: 137-160, 197-209, 247-260, 318-337, 408-475
frontend/src/features/tasks/components/MessagesArea.tsx (1)
170-211: Messages sharing flow and parent share-button wiring look solid.The new sharing integration is well structured:
handleShareTaskcorrectly guards against missingselectedTaskDetail.id, togglesisSharing, callstaskApis.shareTask, and surfaces both success (viashareUrl+TaskShareModal) and failures (viatoast).- The share button is only rendered when there’s a selected task and at least one displayed message, which matches the UX description.
- Memoizing
shareButtonand then pushing it to the parent withonShareButtonRenderviauseEffectis a clean way to letTopNavigationown the actual placement while MessagesArea controls behavior.TaskShareModalreceives the right props (visibility, title, URL) and is decoupled from the button itself.You could optionally wrap the parent’s
handleShareButtonRenderinuseCallbackto reduce redundantonShareButtonRenderidentity changes, but functionally this is already sound.Also applies to: 215-241, 540-566, 702-708
backend/app/models/shared_task.py (1)
1-1: Remove unusedStringimport.The
Stringtype is imported but never used in this model.-from sqlalchemy import Column, Integer, String, Boolean, DateTime, ForeignKey, UniqueConstraint +from sqlalchemy import Column, Integer, Boolean, DateTime, ForeignKey, UniqueConstraintfrontend/src/features/tasks/components/TaskShareHandler.tsx (1)
386-397: Consider using Tailwind class instead of inline style for flex.The
style={{ flex: 1 }}can be replaced with the Tailwind classflex-1for consistency with the rest of the codebase.<Button onClick={handleCloseModal} variant="outline" size="sm" - style={{ flex: 1 }} + className="flex-1" disabled={isCopying} > {t('common.cancel')} </Button> <Button onClick={handleConfirmCopy} variant="default" size="sm" disabled={!!isSelfShare || isCopying || teams.length === 0 || isModelSelectionRequired} - style={{ flex: 1 }} + className="flex-1" >backend/app/services/shared_task.py (3)
528-539: Add exception chaining for proper error context.When re-raising exceptions, use
raise ... from errorraise ... from Noneto preserve the exception chain and improve debugging.try: user_id = int(user_id_str) task_id = int(task_id_str) except ValueError: - raise HTTPException( + raise HTTPException( status_code=400, detail="Invalid share link format" - ) + ) from None except HTTPException: raise - except Exception: + except Exception as e: raise HTTPException( status_code=400, detail="Invalid share link format" - ) + ) from e
88-91: Consider logging the exception before returning None.The blind
except Exceptionsuppresses all errors silently. While returningNonefor invalid tokens is appropriate, logging the exception at debug level would aid troubleshooting.except Exception: + logger.debug("Failed to decrypt share token", exc_info=True) return None
243-367: Consider extracting subtask copying to a separate method.The
_copy_task_with_subtasksmethod is quite long (~125 lines) and handles multiple concerns: team validation, task creation, subtask copying, and attachment copying. Per coding guidelines, functions should not exceed 50 lines. Consider extracting subtask/attachment copying to helper methods for improved readability.This could be split into:
_create_copied_task()- handles task creation with team reference updates_copy_subtasks()- handles subtask and attachment copying
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor_manager/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py(1 hunks)backend/app/api/endpoints/adapter/chat.py(3 hunks)backend/app/api/endpoints/adapter/tasks.py(10 hunks)backend/app/core/config.py(1 hunks)backend/app/models/shared_task.py(1 hunks)backend/app/models/user.py(1 hunks)backend/app/schemas/shared_task.py(1 hunks)backend/app/services/adapters/task_kinds.py(1 hunks)backend/app/services/shared_task.py(1 hunks)docs/TASK_SHARING_FEATURE.md(1 hunks)executor_manager/pyproject.toml(1 hunks)frontend/src/apis/tasks.ts(2 hunks)frontend/src/app/(tasks)/chat/page.tsx(7 hunks)frontend/src/app/(tasks)/code/page.tsx(3 hunks)frontend/src/app/shared/task/page.tsx(1 hunks)frontend/src/features/common/AuthGuard.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(6 hunks)frontend/src/features/tasks/components/PublicTaskSidebar.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareHandler.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareModal.tsx(1 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/src/features/common/AuthGuard.tsx
- frontend/src/features/tasks/components/ChatArea.tsx
- frontend/src/i18n/locales/zh-CN/common.json
- backend/app/core/config.py
- frontend/src/i18n/locales/en/chat.json
- frontend/src/features/tasks/components/PublicTaskSidebar.tsx
- frontend/src/i18n/locales/zh-CN/chat.json
- backend/app/models/user.py
- frontend/src/features/tasks/components/TaskShareModal.tsx
- frontend/src/i18n/locales/en/common.json
- executor_manager/pyproject.toml
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/apis/tasks.tsbackend/app/models/shared_task.pyfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/app/shared/task/page.tsxbackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/shared_task.pyfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/(tasks)/chat/page.tsxbackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/schemas/shared_task.pyfrontend/src/app/(tasks)/code/page.tsxbackend/app/services/adapters/task_kinds.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/apis/tasks.tsfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/app/(tasks)/code/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/apis/tasks.tsfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/app/(tasks)/code/page.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/models/shared_task.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/shared_task.pybackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/schemas/shared_task.pybackend/app/services/adapters/task_kinds.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/shared_task.py
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/MessagesArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/app/(tasks)/code/page.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/app/(tasks)/code/page.tsx
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/shared_task.pybackend/app/services/adapters/task_kinds.py
**/backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/shared_task.py
🧠 Learnings (2)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.{py,ts,tsx,js,jsx} : All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Applied to files:
frontend/src/app/shared/task/page.tsx
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
🧬 Code graph analysis (3)
frontend/src/app/shared/task/page.tsx (9)
frontend/src/features/theme/ThemeProvider.tsx (1)
useTheme(106-112)backend/app/schemas/shared_task.py (1)
PublicSharedTaskResponse(80-87)frontend/src/apis/tasks.ts (1)
PublicSharedTaskResponse(120-126)frontend/e2e/utils/auth.ts (1)
isLoggedIn(94-110)frontend/src/apis/user.ts (1)
getToken(54-59)frontend/src/features/tasks/components/ResizableSidebar.tsx (1)
ResizableSidebar(19-157)frontend/src/features/layout/TopNavigation.tsx (1)
TopNavigation(23-86)frontend/src/features/layout/GithubStarButton.tsx (1)
GithubStarButton(19-137)frontend/src/features/tasks/components/PublicTaskSidebar.tsx (1)
PublicTaskSidebar(24-110)
frontend/src/features/tasks/components/MessagesArea.tsx (6)
frontend/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)frontend/src/features/tasks/contexts/taskContext.tsx (1)
useTaskContext(284-290)frontend/src/features/theme/ThemeProvider.tsx (1)
useTheme(106-112)frontend/src/hooks/useTypewriter.ts (1)
useTypewriter(17-67)frontend/src/apis/tasks.ts (1)
taskApis(130-261)frontend/src/features/tasks/components/TaskShareModal.tsx (1)
TaskShareModal(22-98)
frontend/src/app/(tasks)/chat/page.tsx (2)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
useTaskContext(284-290)frontend/src/features/tasks/components/TaskShareHandler.tsx (1)
TaskShareHandler(43-403)
🪛 markdownlint-cli2 (0.18.1)
docs/TASK_SHARING_FEATURE.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
259-259: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
318-318: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
459-459: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
backend/app/api/endpoints/adapter/tasks.py
330-330: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
331-331: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
353-353: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
365-365: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
378-378: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
379-379: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
395-395: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
412-412: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/services/shared_task.py
90-90: Do not catch blind exception: Exception
(BLE001)
131-131: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
141-141: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
164-164: Do not catch blind exception: Exception
(BLE001)
181-181: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
198-198: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
231-231: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
257-257: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
393-393: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
422-422: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
476-476: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
491-491: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
518-521: Abstract raise to an inner function
(TRY301)
529-532: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
535-535: Do not catch blind exception: Exception
(BLE001)
536-539: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
548-548: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
562-562: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
backend/app/services/adapters/task_kinds.py
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (10)
backend/app/api/endpoints/adapter/tasks.py (1)
17-23: Shared-task endpoints are well wired to the service and validate ownership/team correctly.The new sharing endpoints look consistent and safe:
share_taskfirst checks that the task belongs tocurrent_userbefore delegating toshared_task_service.share_task.get_task_share_infoandget_public_shared_taskare explicitly unauthenticated, which matches the documented public-read / login-to-copy behavior.join_shared_taskvalidates thatteam_id(when provided) belongs to the current user, or falls back to the user’s first active team, and errors cleanly if none exist.Assuming
shared_task_serviceenforces token integrity, duplicate-copy prevention, and cross-user checks as documented, this API surface is solid.Also applies to: 33-34, 327-431
frontend/src/apis/tasks.ts (1)
85-127: Sharing API types and client methods align well with the backend design.The new task sharing types and client calls look consistent:
TaskShareResponse(share_url,share_token) matches the documented backend response.TaskShareInfo,JoinSharedTaskRequest/Response, andPublicSharedTaskResponseline up with the endpoints inbackend/app/api/endpoints/adapter/tasks.py.shareTask,getTaskShareInfo, andjoinSharedTaskcall the expected/tasks/{task_id}/share,/tasks/share/info, and/tasks/share/joinroutes.getPublicSharedTaskcorrectly uses a rawfetchagainst/api/tasks/share/public?token=...to bypass the authenticatedapiClientinterceptor and has reasonable error handling (trying to surfacedetailwhen available).This should give the front-end all the pieces it needs for the share/generate/copy flows.
Also applies to: 216-260
backend/app/models/shared_task.py (1)
7-49: LGTM on model structure.The model correctly defines foreign key relationships with cascade deletes, proper indexing on FK columns, and a unique constraint to prevent duplicate shares. The
__repr__method aids debugging.frontend/src/features/tasks/components/TaskShareHandler.tsx (2)
86-125: LGTM on data fetching and initialization logic.The parallel fetch of share info and teams using
Promise.allis efficient. Auto-selecting the first team provides a good UX. Error handling with toast notifications and URL cleanup is well implemented.
127-198: LGTM on copy confirmation handler.The handler properly validates team and model selection, handles the self-share case, and provides appropriate feedback via toasts. The navigation to the copied task after success is a good UX pattern.
frontend/src/app/shared/task/page.tsx (1)
79-96: LGTM on login and redirect handling.The
handleLoginAndCopyfunction correctly handles both authenticated and unauthenticated users, using proper URL encoding for the share token and redirect target.backend/app/schemas/shared_task.py (1)
1-88: LGTM on Pydantic schema definitions.The schemas are well-structured with appropriate type hints, docstrings, and sensible defaults. The use of
ConfigDict(from_attributes=True)for ORM mapping inSharedTaskInDBfollows Pydantic v2 best practices.backend/app/services/shared_task.py (3)
131-131: Note:== Trueis intentional for SQLAlchemy boolean filters.The static analysis E712 warnings about
== Truecomparisons are false positives in SQLAlchemy context. SQLAlchemy requires explicit== Truefor boolean column filters to generate correct SQL. No changes needed.
369-468: LGTM on join_shared_task logic.The method correctly validates the share token, prevents self-copying, checks for existing shares, handles re-copying after deletion, and properly manages the SharedTask record lifecycle. The unique constraint violation is avoided by reusing existing records.
508-604: LGTM on get_public_shared_task implementation.The method properly validates tokens, checks task existence, and returns only public-safe data. Sensitive fields are excluded from the response.
| task_type = ( | ||
| task_crd.metadata.labels | ||
| and task_crd.metadata.labels.get("taskType") | ||
| or "chat" | ||
| ) | ||
| # Only check expiration for code tasks, chat tasks have no expiration | ||
| if task_type == "code": | ||
| expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading comment and improve expression clarity.
The comment on line 105 states "chat tasks have no expiration", but this is misleading because:
- Chat tasks DO have expiration (set to
APPEND_CHAT_TASK_EXPIRE_HOURSon line 98) - Only chat tasks from
chat_shellsource skip expiration checks (lines 113-120)
Additionally, the chained and/or expression on lines 101-103 should be parenthesized for clarity.
Apply this diff to clarify the comment and improve readability:
- expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS
- # Check if task is expired
task_type = (
- task_crd.metadata.labels
- and task_crd.metadata.labels.get("taskType")
+ (task_crd.metadata.labels
+ and task_crd.metadata.labels.get("taskType"))
or "chat"
)
- # Only check expiration for code tasks, chat tasks have no expiration
+
+ # Set expiration hours based on task type
+ expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS
if task_type == "code":
expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURSCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
🤖 Prompt for AI Agents
In backend/app/services/adapters/task_kinds.py around lines 100 to 107, the
inline comment is misleading and the chained and/or expression is unclear;
change the comment to state that chat tasks do have an expiration but tasks
originating from the "chat_shell" source skip expiration checks, and rewrite the
task_type assignment to a clearer expression (e.g., parenthesize the current
expression or replace with an explicit conditional) so it's unambiguous which
value is selected; ensure subsequent logic uses APPEND_CHAT_TASK_EXPIRE_HOURS
for chat tasks and only bypasses expiration checks for chat tasks with source ==
"chat_shell".
| task_shell_source = ( | ||
| task_crd.chat_shell.labels | ||
| and task_crd.chat_shell.labels.get("source") | ||
| or None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential AttributeError when accessing chat_shell.
Line 110-111 accesses task_crd.chat_shell.labels without checking if chat_shell exists or is not None. This could raise an AttributeError if chat_shell is None or missing from the task object.
Additionally, the chained and/or expression should be parenthesized for clarity (as flagged by static analysis).
Apply this diff to add defensive checks and improve clarity:
task_shell_source = (
- task_crd.chat_shell.labels
- and task_crd.chat_shell.labels.get("source")
+ (task_crd.chat_shell
+ and task_crd.chat_shell.labels
+ and task_crd.chat_shell.labels.get("source"))
or None)🧰 Tools
🪛 Ruff (0.14.7)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
🤖 Prompt for AI Agents
In backend/app/services/adapters/task_kinds.py around lines 109 to 112, the code
accesses task_crd.chat_shell.labels without ensuring chat_shell exists and uses
an unclear chained and/or expression; update it to first check that
task_crd.chat_shell is not None and that task_crd.chat_shell.labels is truthy
(and a mapping) before trying to read .get("source"), and replace the chained
and/or with a clear conditional expression (e.g., parentheses around the checks
or an explicit inline if-else) that returns the label value or None.
| if task_shell_source != "chat_shell": | ||
| if ( | ||
| datetime.now() - existing_task.updated_at | ||
| ).total_seconds() > expire_hours * 3600: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract magic string to a named constant.
The string "chat_shell" on line 113 is a magic value that should be extracted to a named constant.
As per coding guidelines, Python code must extract magic numbers and strings to named constants for better maintainability.
Add a constant at the module level:
# After the imports, around line 28
CHAT_SHELL_SOURCE = "chat_shell"Then update line 113:
- if task_shell_source != "chat_shell":
+ if task_shell_source != CHAT_SHELL_SOURCE:🤖 Prompt for AI Agents
In backend/app/services/adapters/task_kinds.py around lines 113 to 120, the
literal string "chat_shell" is a magic value; add a module-level constant named
CHAT_SHELL_SOURCE = "chat_shell" after the imports (around line 28) and replace
the inline literal at line 113 with CHAT_SHELL_SOURCE so the comparison uses the
named constant instead of a hard-coded string.
| timestamp = datetime.utcnow().strftime("%Y%m%d%H%M%S%f") | ||
| unique_task_name = f"Copy of {original_task.name}-{timestamp}" | ||
|
|
||
| # Create new task with updated team reference | ||
| new_task = Kind( | ||
| kind="Task", | ||
| name=unique_task_name, | ||
| user_id=new_user_id, | ||
| namespace=original_task.namespace, | ||
| json=task_crd.model_dump(mode="json", exclude_none=True), # Use updated JSON | ||
| is_active=True, | ||
| created_at=datetime.utcnow(), | ||
| updated_at=datetime.utcnow(), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace deprecated datetime.utcnow() with timezone-aware alternative.
Multiple usages of datetime.utcnow() exist in this method. As of Python 3.12, this is deprecated. Use datetime.now(timezone.utc) instead.
+from datetime import datetime, timezone
+
# In _copy_task_with_subtasks method:
- timestamp = datetime.utcnow().strftime("%Y%m%d%H%M%S%f")
+ timestamp = datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S%f")
# ...
- created_at=datetime.utcnow(),
- updated_at=datetime.utcnow(),
+ created_at=datetime.now(timezone.utc),
+ updated_at=datetime.now(timezone.utc),Apply the same fix to all other occurrences at lines 332, 333, 360, 449, and 502.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/app/services/shared_task.py around lines 283 to 296 (and also at
lines 332, 333, 360, 449, 502), replace deprecated uses of datetime.utcnow()
with timezone-aware calls: use datetime.now(timezone.utc) and ensure you import
timezone from datetime (e.g., from datetime import datetime, timezone) or
reference datetime.timezone; update each occurrence to create timezone-aware
timestamps (created_at, updated_at, timestamp generation) consistently across
the method so all datetimes are timezone-aware.
| import { Suspense, useState, useEffect } from 'react'; | ||
| import { useSearchParams } from 'next/navigation'; | ||
| import { useSearchParams, useRouter } from 'next/navigation'; | ||
| import { teamService } from '@/features/tasks/service/teamService'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/app/\(tasks\)/chat/page.tsx | head -160Repository: wecode-ai/Wegent
Length of output: 6646
Import ReactNode type to fix TypeScript strict mode error.
shareButton is typed as React.ReactNode (lines 66, 68) without importing the React namespace, causing a type-checking failure in strict mode. Import ReactNode directly from react:
-import { Suspense, useState, useEffect } from 'react';
+import { Suspense, useState, useEffect, type ReactNode } from 'react';Then update the type annotations:
- const [shareButton, setShareButton] = useState<React.ReactNode>(null);
+ const [shareButton, setShareButton] = useState<ReactNode | null>(null);
- const handleShareButtonRender = (button: React.ReactNode) => {
+ const handleShareButtonRender = (button: ReactNode | null) => {The pendingTaskShare → /chat?taskShare=... redirect and TaskShareHandler integration are correctly implemented.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/app/(tasks)/chat/page.tsx around lines 7 to 9 and referencing
usages at ~66 and ~68, the file uses the ReactNode type as React.ReactNode
without importing React; add a direct import for ReactNode from 'react' (e.g.,
import { Suspense, useState, useEffect, ReactNode } from 'react') and update the
shareButton type annotations to use ReactNode instead of React.ReactNode so
TypeScript strict mode resolves the type correctly.
| // Share button state | ||
| const [shareButton, setShareButton] = useState<React.ReactNode>(null); | ||
|
|
||
| const handleShareButtonRender = (button: React.ReactNode) => { | ||
| setShareButton(button); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/app/\(tasks\)/code/page.tsx | head -30Repository: wecode-ai/Wegent
Length of output: 1842
🏁 Script executed:
rg "import.*React|import.*ReactNode" frontend/src/app/\(tasks\)/code/page.tsxRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
sed -n '57,62p; 176,185p; 203,209p' frontend/src/app/\(tasks\)/code/page.tsxRepository: wecode-ai/Wegent
Length of output: 879
Import ReactNode instead of using React.ReactNode without a React import.
shareButton is typed as React.ReactNode, but this file doesn't import the React namespace, so TypeScript will fail type-checking with "Cannot find name 'React'".
Fix by importing the type and using it directly:
-import { Suspense, useState, useEffect, useMemo } from 'react';
+import { Suspense, useState, useEffect, useMemo, type ReactNode } from 'react';
- const [shareButton, setShareButton] = useState<React.ReactNode>(null);
+ const [shareButton, setShareButton] = useState<ReactNode | null>(null);
- const handleShareButtonRender = (button: React.ReactNode) => {
+ const handleShareButtonRender = (button: ReactNode | null) => {
setShareButton(button);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/app/(tasks)/code/page.tsx around lines 57 to 62, the code uses
the React namespace (React.ReactNode) but the file does not import React,
causing TypeScript to error; import the ReactNode type from 'react' (e.g. import
type { ReactNode } from 'react') and update the state and handler signatures to
use ReactNode (useState<ReactNode>(null) and (button: ReactNode) => void) so the
types resolve correctly.
| {!isUser && ( | ||
| <div className="flex items-center gap-2 mb-2 text-xs opacity-80"> | ||
| <Bot className="w-4 h-4" /> | ||
| <span className="font-semibold">AI Assistant</span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded "AI Assistant" label should use i18n.
For consistency with the internationalization approach used elsewhere in this component, this label should be translated.
{!isUser && (
<div className="flex items-center gap-2 mb-2 text-xs opacity-80">
<Bot className="w-4 h-4" />
- <span className="font-semibold">AI Assistant</span>
+ <span className="font-semibold">{t('shared_task.ai_assistant')}</span>
</div>
)}📝 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.
| {!isUser && ( | |
| <div className="flex items-center gap-2 mb-2 text-xs opacity-80"> | |
| <Bot className="w-4 h-4" /> | |
| <span className="font-semibold">AI Assistant</span> | |
| </div> | |
| )} | |
| {!isUser && ( | |
| <div className="flex items-center gap-2 mb-2 text-xs opacity-80"> | |
| <Bot className="w-4 h-4" /> | |
| <span className="font-semibold">{t('shared_task.ai_assistant')}</span> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In frontend/src/app/shared/task/page.tsx around lines 283 to 288, the hardcoded
"AI Assistant" label is not internationalized; replace the literal string with a
translation lookup using the component's i18n setup (e.g., call the existing
translation function or use the appropriate useTranslation hook/key used in this
file) and update the JSX to render t('aiAssistant') or the matching locale key,
ensuring you add the new key to the locale resource files if it doesn't exist.
c245627 to
f5881d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/apis/tasks.ts (1)
218-226: Fix parameter swap ingetBranchDiffquery builder.The function receives
source_branchandtarget_branchparameters with correct values (as seen inWorkbench.tsx:220-221), but swaps them when building the URL query string. This causes the API to compare branches in the wrong direction. Lines 221-222 should be:query.append('source_branch', params.source_branch); query.append('target_branch', params.target_branch);
♻️ Duplicate comments (9)
backend/app/services/adapters/task_kinds.py (2)
99-107: Clarify expiration comment and simplifytask_typeextraction.
- Line 105’s comment (“Only check expiration for code tasks, chat tasks have no expiration”) no longer matches the logic:
expire_hoursis initialized to the chat value and later overridden only for code tasks, and the expiration check below now runs for both chat and code tasks when the source is notchat_shell. This is misleading for future readers. Consider something like:# Set expiration hours based on task type (chat/code); actual expiration # check is applied below for non-chat_shell sources.
- Lines 101‑103 still use a chained
and/orexpression that Ruff flags (RUF021) and is harder to read. A clearer (and equivalent) version avoids precedence ambiguities:- task_type = ( - task_crd.metadata.labels - and task_crd.metadata.labels.get("taskType") - or "chat" - ) + labels = task_crd.metadata.labels or {} + task_type = labels.get("taskType") or "chat"This keeps the same defaulting behavior while improving readability and satisfying the linter.
109-120: Guardchat_shellaccess, extract constants, and avoid chainedand/or.
Lines 109‑112 can raise
AttributeErroriftask_crd.chat_shellisNoneor missing (e.g., legacy tasks or non-chat-shell tasks), since you unconditionally access.labels. That would break appends for such tasks. The source check should treat “no chat_shell info” as “not from chat_shell” without crashing.The expression also uses the same chained
and/orpattern Ruff flags (RUF021), and"chat_shell"plus3600are magic values that should be named constants (per coding guidelines and retrieved learnings).A safer and clearer version might look like:
# Near the top of the module, after logger definition CHAT_SHELL_SOURCE = "chat_shell" SECONDS_PER_HOUR = 3600Then in this block:
- task_shell_source = ( - task_crd.chat_shell.labels - and task_crd.chat_shell.labels.get("source") - or None) - if task_shell_source != "chat_shell": - if ( - datetime.now() - existing_task.updated_at - ).total_seconds() > expire_hours * 3600: + chat_shell = getattr(task_crd, "chat_shell", None) + task_shell_source = ( + chat_shell.labels.get("source") + if chat_shell and chat_shell.labels + else None + ) + + if task_shell_source != CHAT_SHELL_SOURCE: + if ( + datetime.now() - existing_task.updated_at + ).total_seconds() > expire_hours * SECONDS_PER_HOUR: raise HTTPException( status_code=400, detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", )This defends against missing
chat_shell, removes the ambiguousand/orchain, and aligns with the “no magic constants” rule. Based on learnings, extracting these literals to constants improves maintainability.frontend/src/app/(tasks)/code/page.tsx (1)
57-62: Import and useReactNodeinstead ofReact.ReactNodeto satisfy TS.
shareButtonandhandleShareButtonRenderuseReact.ReactNode, but this module doesn’t importReact, so TypeScript will complain (Cannot find name 'React') in strict setups. Import the type and use it directly, also allowingnullexplicitly:-import { Suspense, useState, useEffect, useMemo } from 'react'; +import { Suspense, useState, useEffect, useMemo, type ReactNode } from 'react'; @@ - // Share button state - const [shareButton, setShareButton] = useState<React.ReactNode>(null); - - const handleShareButtonRender = (button: React.ReactNode) => { + // Share button state + const [shareButton, setShareButton] = useState<ReactNode | null>(null); + + const handleShareButtonRender = (button: ReactNode | null) => { setShareButton(button); };This keeps the new sharing hook while ensuring the file type-checks cleanly.
Also applies to: 181-209
frontend/src/i18n/locales/en/common.json (1)
604-654: Fix interpolation to use double braces fortaskTitle.Within the new
shared_taskblock,share_success_messageuses{taskTitle}, but the rest of the file (e.g., team sharing strings) follows the{{variable}}pattern. This is likely inconsistent with your i18n library and will render the braces literally.- "share_success_message": "Your task {taskTitle} has been shared successfully!", + "share_success_message": "Your task {{taskTitle}} has been shared successfully!",You may also want to quickly scan for other single-brace placeholders in this file.
frontend/src/app/(tasks)/chat/page.tsx (1)
65-70: ImportReactNodetype to fix TypeScript strict mode compliance.The
shareButtonstate andhandleShareButtonRendercallback useReact.ReactNodewithout importing theReactnamespace orReactNodetype directly. This may cause TypeScript strict mode errors.Apply this diff:
-import { Suspense, useState, useEffect } from 'react'; +import { Suspense, useState, useEffect, type ReactNode } from 'react';Then update the type annotations:
- const [shareButton, setShareButton] = useState<React.ReactNode>(null); + const [shareButton, setShareButton] = useState<ReactNode | null>(null); - const handleShareButtonRender = (button: React.ReactNode) => { + const handleShareButtonRender = (button: ReactNode | null) => {frontend/src/app/shared/task/page.tsx (3)
142-148: Hardcoded English status strings should use i18n.These status messages are displayed to users but aren't internationalized.
165-188: Loading text should use i18n.The "Loading..." and "Loading shared conversation..." strings are hardcoded.
271-274: Redundant duplicate spans with identical content.Both spans display the same text with different responsive visibility classes, but since they show identical content, one can be removed.
backend/app/api/endpoints/adapter/tasks.py (1)
350-372: Critical: Move/share/*routes before/{task_id}route.The routes
/share/info,/share/public, and/share/joinare defined after the/{task_id}route (line 96). FastAPI matches routes in definition order, so/{task_id}will capture "share" as a task_id before these literal routes are evaluated.
🧹 Nitpick comments (8)
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py (1)
21-43: Schema looks correct; consider minor index/constraint refinements.The DDL for
shared_tasksmatches the intended model and should work as-is. Two minor, non-blocking improvements you might consider:
KEY idx_shared_tasks_id (id)is redundant withPRIMARY KEY (id)and can be dropped to avoid an extra index that will never be used.- Depending on how strictly you enforce referential integrity elsewhere, you may want
FOREIGN KEYconstraints onuser_id,original_user_id, andoriginal_task_idto keepshared_tasksrows aligned withusers/tasks(or explicitly document why they’re omitted).docs/TASK_SHARING_FEATURE.md (1)
39-75: Address markdownlint warnings: add fence languages and avoid emphasis-as-heading.The content is solid, but markdownlint is flagging a few style issues:
- Multiple fenced code blocks (e.g., the flow diagram, sample URLs, API examples, ASCII UI blocks) use bare
```fences. Add an appropriate language ortext(e.g.,text,sql,ts,json) so MD040 passes.- Some FAQ entries and section labels are written as bolded lines (e.g.,
**Q: 分享链接会过期吗?**) instead of proper headings, which triggers MD036. Consider converting them to real headings (### Q: ...) or keeping them plain text.These are cosmetic but will make CI markdownlint happier.
Also applies to: 137-160, 197-210, 247-260, 318-325, 408-475
backend/app/schemas/shared_task.py (1)
27-41: Minor Pydantic typing/default tweaks for clarity and safety.Schemas look good and match the frontend types; two small cleanups would make them more idiomatic:
force_override_bot_modelis typedOptional[bool] = False. Since you default toFalseandNoneis never semantically used, simplify to a plainbool:- force_override_bot_model: Optional[bool] = False + force_override_bot_model: bool = False
- Use
Field(default_factory=list)for list defaults to make the mutable default pattern explicit and idiomatic in Pydantic v2:-from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, Field- attachments: List[PublicAttachmentData] = [] + attachments: List[PublicAttachmentData] = Field(default_factory=list)These don't change behavior but align with Pydantic v2 best practices.
Also applies to: 81-92
frontend/src/apis/tasks.ts (1)
247-272: Consider adding a timeout to the public fetch call.The
getPublicSharedTaskfunction uses nativefetchwithout a timeout. For consistency withapiClientbehavior and to prevent indefinitely hanging requests, consider adding anAbortControllertimeout.getPublicSharedTask: async (token: string): Promise<PublicSharedTaskResponse> => { const query = new URLSearchParams(); query.append('token', token); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + - const response = await fetch(`/api/tasks/share/public?${query}`, { + const response = await fetch(`/api/tasks/share/public?${query}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, + signal: controller.signal, }); + clearTimeout(timeoutId);frontend/src/app/shared/task/page.tsx (1)
150-152: Magic string separator should be documented or extracted to a constant.The separator
'${$$}$'is a special format used to trigger markdown rendering in MessageBubble. Consider extracting this to a shared constant for maintainability.// At module level or in a shared constants file const MESSAGE_CONTENT_SEPARATOR = '${$$}$'; // Then use it: const content = MESSAGE_CONTENT_SEPARATOR + resultContent;backend/app/services/shared_task.py (3)
243-366: Consider extracting helper methods to reduce function length.This function is 124 lines, significantly exceeding the 50-line guideline. Breaking it into focused helpers (e.g.,
_validate_and_get_team,_create_copied_task,_copy_subtasks_and_attachments) would improve readability and testability.As per coding guidelines, Python functions should not exceed 50 lines.
368-467: Consider extracting helper methods to reduce function length.This function is 100 lines, doubling the 50-line guideline. Extracting helpers (e.g.,
_validate_share_token_for_join,_check_duplicate_share,_create_or_update_share_record) would improve maintainability and align with coding standards.As per coding guidelines, Python functions should not exceed 50 lines.
507-627: Consider extracting helper methods to reduce function length.This function is 121 lines, significantly exceeding the 50-line guideline. Breaking it into focused helpers (e.g.,
_parse_and_validate_token,_fetch_task_and_user,_build_public_subtasks_data) would improve maintainability and testability.As per coding guidelines, Python functions should not exceed 50 lines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor_manager/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py(1 hunks)backend/app/api/endpoints/adapter/chat.py(3 hunks)backend/app/api/endpoints/adapter/tasks.py(10 hunks)backend/app/core/config.py(1 hunks)backend/app/models/shared_task.py(1 hunks)backend/app/models/user.py(1 hunks)backend/app/schemas/shared_task.py(1 hunks)backend/app/services/adapters/task_kinds.py(1 hunks)backend/app/services/shared_task.py(1 hunks)docs/TASK_SHARING_FEATURE.md(1 hunks)executor_manager/pyproject.toml(1 hunks)frontend/src/apis/tasks.ts(2 hunks)frontend/src/app/(tasks)/chat/page.tsx(7 hunks)frontend/src/app/(tasks)/code/page.tsx(3 hunks)frontend/src/app/shared/task/page.tsx(1 hunks)frontend/src/features/common/AuthGuard.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(6 hunks)frontend/src/features/tasks/components/PublicTaskSidebar.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareHandler.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareModal.tsx(1 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/src/features/common/AuthGuard.tsx
- frontend/src/features/tasks/components/TaskShareHandler.tsx
- frontend/src/features/tasks/components/ChatArea.tsx
- backend/app/models/user.py
- backend/app/core/config.py
- executor_manager/pyproject.toml
- frontend/src/features/tasks/components/TaskShareModal.tsx
- frontend/src/i18n/locales/zh-CN/common.json
- backend/app/models/shared_task.py
- frontend/src/features/tasks/components/PublicTaskSidebar.tsx
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/app/(tasks)/code/page.tsxbackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/services/adapters/task_kinds.pyfrontend/src/apis/tasks.tsbackend/app/schemas/shared_task.pyfrontend/src/features/tasks/components/MessagesArea.tsxbackend/app/services/shared_task.pyfrontend/src/app/shared/task/page.tsxbackend/app/api/endpoints/adapter/chat.pyfrontend/src/app/(tasks)/chat/page.tsxbackend/app/api/endpoints/adapter/tasks.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/app/(tasks)/code/page.tsxfrontend/src/apis/tasks.tsfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/app/(tasks)/code/page.tsxfrontend/src/apis/tasks.tsfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/app/(tasks)/code/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/chat/page.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/services/adapters/task_kinds.pybackend/app/schemas/shared_task.pybackend/app/services/shared_task.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
**/backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/task_kinds.pybackend/app/services/shared_task.py
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/shared_task.py
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
🧠 Learnings (6)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : React components MUST use functional components with hooks instead of class-based components
Applied to files:
frontend/src/app/(tasks)/code/page.tsx
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.py : Python code MUST extract magic numbers to named constants
Applied to files:
backend/app/services/adapters/task_kinds.py
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/schemas/**/*.py : Backend Pydantic schemas MUST be created in `app/schemas/` directory
Applied to files:
backend/app/schemas/shared_task.py
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.{py,ts,tsx,js,jsx} : All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Applied to files:
frontend/src/app/shared/task/page.tsx
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.{ts,tsx} : TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
Applied to files:
frontend/src/app/(tasks)/chat/page.tsx
🧬 Code graph analysis (5)
backend/app/schemas/shared_task.py (1)
frontend/src/apis/tasks.ts (7)
TaskShareInfo(91-96)TaskShareResponse(86-89)JoinSharedTaskRequest(98-103)JoinSharedTaskResponse(105-108)PublicAttachmentData(110-119)PublicSubtaskData(121-130)PublicSharedTaskResponse(132-138)
frontend/src/features/tasks/components/MessagesArea.tsx (3)
frontend/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)frontend/src/apis/tasks.ts (1)
taskApis(142-273)frontend/src/features/tasks/components/TaskShareModal.tsx (1)
TaskShareModal(22-98)
backend/app/api/endpoints/adapter/chat.py (3)
backend/app/services/chat/session_manager.py (2)
get_chat_history(56-82)save_chat_history(84-115)backend/app/models/subtask.py (2)
SubtaskStatus(17-23)SubtaskRole(26-28)backend/app/schemas/subtask.py (2)
SubtaskStatus(25-31)SubtaskRole(34-36)
frontend/src/app/(tasks)/chat/page.tsx (3)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
useTaskContext(284-290)frontend/src/features/common/UserContext.tsx (1)
useUser(148-148)frontend/src/features/tasks/components/TaskShareHandler.tsx (1)
TaskShareHandler(43-403)
backend/app/api/endpoints/adapter/tasks.py (2)
backend/app/schemas/shared_task.py (5)
JoinSharedTaskRequest(27-33)JoinSharedTaskResponse(36-40)PublicSharedTaskResponse(94-101)TaskShareInfo(11-17)TaskShareResponse(20-24)backend/app/services/shared_task.py (4)
share_task(188-215)get_share_info(217-241)get_public_shared_task(507-627)join_shared_task(368-467)
🪛 markdownlint-cli2 (0.18.1)
docs/TASK_SHARING_FEATURE.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
259-259: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
318-318: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
459-459: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
backend/app/services/adapters/task_kinds.py
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
backend/app/services/shared_task.py
90-90: Do not catch blind exception: Exception
(BLE001)
131-131: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
141-141: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
164-164: Do not catch blind exception: Exception
(BLE001)
181-181: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
198-198: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
231-231: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
257-257: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
392-392: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
421-421: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
475-475: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
490-490: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
517-520: Abstract raise to an inner function
(TRY301)
528-531: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
534-534: Do not catch blind exception: Exception
(BLE001)
535-538: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
547-547: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
561-561: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
backend/app/api/endpoints/adapter/tasks.py
330-330: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
331-331: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
353-353: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
365-365: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
378-378: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
379-379: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
395-395: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
412-412: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (16)
frontend/src/i18n/locales/zh-CN/chat.json (1)
41-48: Shared-task translations are consistent and well-scoped.The new
shared_taskblock keys and structure match the English locale and fit the existing chat namespace cleanly. No issues from an i18n or structure perspective.frontend/src/i18n/locales/en/chat.json (1)
41-48: Shared-task copy is clear and aligned with zh-CN keys.The
shared_taskmessages are well-phrased, map one-to-one with the Chinese locale, and stay within the chat namespace without collisions.backend/app/api/endpoints/adapter/chat.py (1)
133-141: Redis history bootstrap from existing subtasks looks correct and well-scoped.The refactor of
_create_task_and_subtasksto async and the added Redis initialization logic are coherent:
existing_subtasksis captured before inserting the new user/assistant subtasks, so the history rebuilt contains only prior conversation turns, letting the new message be appended by the streaming path.- History is rebuilt only when
existing_subtasksis non-empty and Redis has no history, which is exactly what shared/copied tasks need and avoids clobbering any existing cache.- Filtering to
COMPLETEDsubtasks and mapping USER →prompt, ASSISTANT →result["value"]matches the documented result shape and avoids half-finished messages.- All call sites use
awaitcorrectly; no sync call sites remain.Overall, this should make shared/copied tasks work seamlessly with direct-chat history without introducing regressions.
frontend/src/app/(tasks)/chat/page.tsx (2)
46-54: Pending task share redirect logic looks correct.The flow properly reads from localStorage, clears the pending token immediately to prevent re-processing, and redirects with the token as a query parameter. This handles the case where a user lands on the public share page, logs in, and needs to be redirected back to complete the copy flow.
114-116: TaskShareHandler integration is properly wired.The component is wrapped in Suspense (required since it uses
useSearchParams) and receivesrefreshTasksto update the task list after a successful copy operation.frontend/src/apis/tasks.ts (1)
85-138: Task sharing types are well-defined.The new type definitions cover all aspects of the sharing feature: share responses, share info, join requests/responses, and public task data structures. The optional fields are properly typed.
frontend/src/features/tasks/components/MessagesArea.tsx (4)
180-181: New share button render prop is correctly implemented.The
onShareButtonRendercallback pattern allows the parent component (ChatPage → TopNavigation) to control where the share button is rendered, maintaining proper component hierarchy.Also applies to: 200-200
216-241: Share task handler has proper error handling and loading state.The
handleShareTaskfunction correctly:
- Validates task selection before proceeding
- Sets loading state during the API call
- Shows success modal with share URL on success
- Displays error toast on failure with proper message extraction
540-565: Share button memoization prevents infinite re-render loop.Good use of
useMemofor the share button and proper dependency tracking. TheuseEffectthat callsonShareButtonRenderhas correct dependencies to avoid stale closures while preventing infinite loops.
701-708: TaskShareModal integration is clean.The modal receives all required props and properly handles close state. The fallback title "Untitled Task" is a reasonable default.
frontend/src/app/shared/task/page.tsx (2)
38-38: Consider potential race condition with synchronous token check.The
isLoggedIncheck usinggetToken()is evaluated once during render. If the user logs in via a different tab, this won't update until a page refresh. This is acceptable for a public page, but worth noting.
79-96: Login and copy redirect flow is correctly implemented.The
handleLoginAndCopyfunction properly:
- Uses
encodeURIComponentfor URL safety- Checks auth state before redirecting
- Chains redirects correctly (login → chat with taskShare param)
backend/app/api/endpoints/adapter/tasks.py (4)
327-347: Share task endpoint correctly validates ownership before generating link.The endpoint properly validates that the task belongs to the current user before generating the share token. Good authorization pattern.
387-421: Team validation logic for join_shared_task is correct.The endpoint properly:
- Validates provided team_id belongs to current user
- Falls back to user's first active team if not specified
- Returns clear error if user has no teams
395-395: SQLAlchemy filter requires== Truecomparison.The Ruff E712 warnings about
== Truecomparisons are false positives in this context. SQLAlchemy filter expressions require explicit boolean comparisons; usingKind.is_activealone would not generate the correct SQL WHERE clause.Also applies to: 412-412
330-331: Ruff B008 warnings are false positives for FastAPI.The
Depends()calls in function argument defaults are the standard FastAPI dependency injection pattern and should not be refactored.Also applies to: 353-353, 365-365, 378-379
| def _aes_decrypt(self, encrypted_data: str) -> Optional[str]: | ||
| """Decrypt data using AES-256-CBC""" | ||
| try: | ||
| # Decode base64 encrypted data | ||
| encrypted_bytes = base64.b64decode(encrypted_data.encode("utf-8")) | ||
|
|
||
| # Create cipher object | ||
| cipher = Cipher( | ||
| algorithms.AES(self.aes_key), | ||
| modes.CBC(self.aes_iv), | ||
| backend=default_backend(), | ||
| ) | ||
| decryptor = cipher.decryptor() | ||
|
|
||
| # Decrypt the data | ||
| decrypted_padded_bytes = ( | ||
| decryptor.update(encrypted_bytes) + decryptor.finalize() | ||
| ) | ||
|
|
||
| # Unpad the data | ||
| unpadder = padding.PKCS7(128).unpadder() | ||
| decrypted_bytes = ( | ||
| unpadder.update(decrypted_padded_bytes) + unpadder.finalize() | ||
| ) | ||
|
|
||
| # Return decrypted string | ||
| return decrypted_bytes.decode("utf-8") | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Log exception details before returning None.
The bare except Exception at line 90 silently suppresses all decryption failures without recording any diagnostic information. This makes debugging invalid tokens or encryption issues extremely difficult in production.
Apply this diff to add logging:
except Exception:
+ logger.warning("Failed to decrypt share token", exc_info=True)
return NoneCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
90-90: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In backend/app/services/shared_task.py around lines 63 to 91, the except block
currently swallows all decryption errors; change it to catch the exception as a
variable and log the exception details before returning None. Specifically,
replace the bare except with "except Exception as e:" and call the module logger
(or create one via logging.getLogger(__name__) if none exists) using
logger.exception or logger.error(..., exc_info=True) with a short message like
"AES decryption failed" to record the stack trace and error, then return None.
| def decode_share_token( | ||
| self, share_token: str, db: Optional[Session] = None | ||
| ) -> Optional[TaskShareInfo]: | ||
| """Decode share token to get task information using AES decryption""" | ||
| try: | ||
| # First URL decode the token, then use AES decryption | ||
| decoded_token = urllib.parse.unquote(share_token) | ||
| share_data_str = self._aes_decrypt(decoded_token) | ||
| if not share_data_str: | ||
| logger.info("Invalid share token format: %s", share_token) | ||
| return None | ||
|
|
||
| # Parse the "user_id#task_id" format | ||
| if "#" not in share_data_str: | ||
| return None | ||
|
|
||
| user_id_str, task_id_str = share_data_str.split("#", 1) | ||
| try: | ||
| user_id = int(user_id_str) | ||
| task_id = int(task_id_str) | ||
| except ValueError: | ||
| return None | ||
|
|
||
| # If database session is provided, query user_name and task_title from database | ||
| if db is not None: | ||
| # Query user name | ||
| user = ( | ||
| db.query(User) | ||
| .filter(User.id == user_id, User.is_active == True) | ||
| .first() | ||
| ) | ||
|
|
||
| # Query task | ||
| task = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.id == task_id, | ||
| Kind.kind == "Task", | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
|
|
||
| if not user or not task: | ||
| logger.info("User or task not found in the database.") | ||
| return None | ||
|
|
||
| return TaskShareInfo( | ||
| user_id=user_id, | ||
| user_name=user.user_name, | ||
| task_id=task_id, | ||
| task_title=task.name or "Untitled Task", | ||
| ) | ||
| else: | ||
| # Without database session, return basic info with placeholder names | ||
| return TaskShareInfo( | ||
| user_id=user_id, | ||
| user_name=f"User_{user_id}", | ||
| task_id=task_id, | ||
| task_title=f"Task_{task_id}", | ||
| ) | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Log exception details in token decoding.
The bare except Exception at line 164 silently suppresses all token decoding failures. Add logging to capture the exception details for troubleshooting.
Apply this diff:
except Exception:
+ logger.warning("Failed to decode share token: %s", share_token, exc_info=True)
return None🧰 Tools
🪛 Ruff (0.14.7)
131-131: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
141-141: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
164-164: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In backend/app/services/shared_task.py around lines 103 to 165, the bare "except
Exception:" swallows errors when decoding share tokens; update the except block
to log the exception details before returning None (e.g., use logger.exception
or logger.error with exc_info=True and include the share_token and/or
decoded_token in the message for context), then return None as before.
| raise HTTPException( | ||
| status_code=400, | ||
| detail="Invalid share link format" | ||
| ) | ||
| except HTTPException: | ||
| raise | ||
| except Exception: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="Invalid share link format" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Use exception chaining for better error context.
When re-raising exceptions within an except clause, use raise ... from err to preserve the original exception context, or raise ... from None to explicitly suppress it. This helps with debugging by maintaining the full error chain.
Apply this diff:
except ValueError:
raise HTTPException(
status_code=400,
detail="Invalid share link format"
- )
+ ) from None
except HTTPException:
raise
except Exception:
raise HTTPException(
status_code=400,
detail="Invalid share link format"
- )
+ ) from NoneCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
528-531: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
534-534: Do not catch blind exception: Exception
(BLE001)
535-538: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In backend/app/services/shared_task.py around lines 528 to 538, the broad except
block swallows the original exception; change it to capture the exception (e.g.,
except Exception as err:) and re-raise the HTTPException using exception
chaining (raise HTTPException(status_code=400, detail="Invalid share link
format") from err) so the original traceback is preserved for debugging; leave
the existing bare except HTTPException re-raise as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/app/shared/task/page.tsx (2)
144-177: Status fallbacks and bot label should use i18n instead of hardcoded English.The fallback texts
'Task completed','Task failed','Processing...'and thebotName: 'AI Assistant'are user-visible and bypass the translation system. Similar feedback was raised previously for this file.You already have
tin scope, so consider:- } else if (subtask.status === 'COMPLETED') { - resultContent = 'Task completed'; - } else if (subtask.status === 'FAILED') { - resultContent = 'Task failed'; - } else { - resultContent = 'Processing...'; - } + } else if (subtask.status === 'COMPLETED') { + resultContent = t('shared_task.status_completed'); + } else if (subtask.status === 'FAILED') { + resultContent = t('shared_task.status_failed'); + } else { + resultContent = t('shared_task.status_processing'); + } @@ - type: 'ai', - content, - timestamp: new Date(subtask.created_at).getTime(), - botName: 'AI Assistant', + type: 'ai', + content, + timestamp: new Date(subtask.created_at).getTime(), + botName: t('shared_task.ai_assistant'),
180-194: Loading texts should be localized through i18n.Both
"Loading shared conversation..."and"Loading..."are hardcoded English strings. Prior review already called out these specific texts.In the in-component loading state you can reuse
t:- <p className="text-text-muted">Loading shared conversation...</p> + <p className="text-text-muted"> + {t('shared_task.loading_shared_conversation')} + </p>For the Suspense fallback, consider extracting a small component that uses
useTranslation:+function SharedTaskLoadingFallback() { + const { t } = useTranslation('common'); + return ( + <div className="flex smart-h-screen bg-base text-text-primary box-border"> + <div className="flex-1 flex items-center justify-center"> + <div className="text-center"> + <div className="animate-spin rounded-full h-12 w-12 border-b-2 border-primary mx-auto mb-4" /> + <p className="text-text-muted">{t('shared_task.loading')}</p> + </div> + </div> + </div> + ); +} @@ export default function SharedTaskPage() { return ( <Suspense - fallback={ - <div className="flex smart-h-screen bg-base text-text-primary box-border"> - <div className="flex-1 flex items-center justify-center"> - <div className="text-center"> - <div className="animate-spin rounded-full h-12 w-12 border-b-2 border-primary mx-auto mb-4"></div> - <p className="text-text-muted">Loading...</p> - </div> - </div> - </div> - } + fallback={<SharedTaskLoadingFallback />} >Also applies to: 341-357
🧹 Nitpick comments (2)
frontend/src/app/shared/task/page.tsx (2)
72-85: String-matching on error messages is brittle; prefer structured error codes.Mapping backend errors to i18n keys via
errorMessage.includes(...)will break if message text changes or gets localized server-side. If possible, havetaskApis.getPublicSharedTaskexpose structured information (HTTP status, error code enum, or typed error) and branch on that instead of string includes. This keeps UI behavior stable across backend copy changes.
289-295: Align Tailwind colors with design system CSS variables.This Alert uses raw Tailwind blue tokens (
bg-blue-50,border-blue-200, etc.), which conflicts with the guideline to use the shared color system (CSS variables) insrccomponents. Consider switching to your standardized semantic classes (e.g., variants based onbg-surface,border-border, or utility classes wired to--color-*variables) so the component respects global theming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/shared/task/page.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/app/shared/task/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/app/shared/task/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/app/shared/task/page.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/app/shared/task/page.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/app/shared/task/page.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.{py,ts,tsx,js,jsx} : All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Applied to files:
frontend/src/app/shared/task/page.tsx
🔇 Additional comments (2)
frontend/src/app/shared/task/page.tsx (2)
253-338: Overall layout, auth flow, and MessageBubble integration look solid.The shared-task page mirrors the chat layout cleanly, handles logged-in vs guest flows via
handleLoginAndCopy, and reusesMessageBubblewith a well-shaped adapter (convertSubtaskToMessage), which keeps the UI consistent with the main chat experience.
341-357: Suspense wrapper correctly isolates URL search param usage.Wrapping
SharedTaskContent(which usesuseSearchParams) in a Suspense boundary with a lightweight client fallback is aligned with Next 15’s app-router patterns for async navigation-aware hooks.
| useEffect(() => { | ||
| const token = searchParams.get('token'); | ||
|
|
||
| if (!token) { | ||
| setError(t('shared_task.error_invalid_link')); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| const fetchSharedTask = async () => { | ||
| try { | ||
| const data = await taskApis.getPublicSharedTask(token); | ||
| setTaskData(data); | ||
| } catch (err) { | ||
| console.error('Failed to load shared task:', err); | ||
| const errorMessage = (err as Error)?.message || ''; | ||
|
|
||
| // Map error messages to i18n keys | ||
| if ( | ||
| errorMessage.includes('Invalid share link format') || | ||
| errorMessage.includes('Invalid share token') | ||
| ) { | ||
| setError(t('shared_task.error_invalid_link')); | ||
| } else if ( | ||
| errorMessage.includes('no longer available') || | ||
| errorMessage.includes('deleted') | ||
| ) { | ||
| setError(t('shared_task.error_task_deleted')); | ||
| } else { | ||
| setError(t('shared_task.error_load_failed')); | ||
| } | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchSharedTask(); | ||
| }, [searchParams, t]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Error handling should clear stale error state on successful refetch.
If fetchSharedTask fails once and later succeeds (e.g., search params or language change), error is never reset to null, so the if (error || !taskData) branch will continue to render the error view even with valid taskData. Consider explicitly clearing error (and optionally resetting loading) when starting a new fetch:
useEffect(() => {
const token = searchParams.get('token');
if (!token) {
setError(t('shared_task.error_invalid_link'));
setIsLoading(false);
return;
}
+ setError(null);
+ setIsLoading(true);
+
const fetchSharedTask = async () => {
try {
const data = await taskApis.getPublicSharedTask(token);
setTaskData(data);🤖 Prompt for AI Agents
In frontend/src/app/shared/task/page.tsx around lines 55 to 92, the
fetchSharedTask flow never clears a previous error so a past failure can keep
the error view even after a successful refetch; fix this by resetting error
state when starting a new fetch and on success: call setIsLoading(true) and
setError(null) before invoking the API, and after a successful response ensure
setTaskData(data) is followed by setError(null) (and setIsLoading(false) in
finally) so stale errors don't persist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
backend/app/api/endpoints/adapter/chat.py (1)
1280-1280: Remove or replace the non-standardContent-Encoding: noneheader.The value "none" is not defined in RFC 7231 as a valid Content-Encoding. Standard values are limited to: compress, deflate, and gzip. If no encoding is applied, the header should be omitted entirely. This pattern appears in two locations (backend/app/services/chat/chat_service.py:253 and backend/app/api/endpoints/adapter/chat.py:1280) with a comment referencing "no compression buffering." Investigate the original intent and use a compliant approach instead—either remove the header or use a standard encoding value if compression control is needed.
♻️ Duplicate comments (2)
backend/app/services/adapters/task_kinds.py (2)
99-107: Clarify expiration comments and avoidand/orchaining fortask_type.The comment and expression around
task_typeare misleading and hard to read:
- Chat tasks do expire (using
APPEND_CHAT_TASK_EXPIRE_HOURS); only tasks withsource == "chat_shell"skip the expiration check.- The
metadata.labels and ... or "chat"pattern is exactly what Ruff flags and is non‑obvious.You can clarify both intent and readability like this:
- # Check if task is expired - task_type = ( - task_crd.metadata.labels - and task_crd.metadata.labels.get("taskType") - or "chat" - ) - # Only check expiration for code tasks, chat tasks have no expiration - if task_type == "code": - expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS + # Determine task type for expiration; default to "chat" when label is missing + labels = task_crd.metadata.labels or {} + task_type = labels.get("taskType", "chat") + + # Use a longer expiration window for code tasks + if task_type == "code": + expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURSYou may also want to move the "Check if task is expired" comment down to the actual time comparison block for accuracy.
109-120: Guardchat_shellaccess and extract magic values to constants.Current code risks an
AttributeErrorand still embeds magic values:
task_crd.chat_shell.labelsis accessed without confirmingtask_crd.chat_shellexists.- The
and/orchain is again non‑obvious and flagged by Ruff."chat_shell"and3600are magic values that should be named constants per your guidelines.Consider:
- Add module‑level constants (near the top of the file, after
logger):CHAT_SHELL_SOURCE = "chat_shell" SECONDS_PER_HOUR = 3600
- Rewrite the block defensively and more readably:
- task_shell_source = ( - task_crd.chat_shell.labels - and task_crd.chat_shell.labels.get("source") - or None) - if task_shell_source != "chat_shell": - if ( - datetime.now() - existing_task.updated_at - ).total_seconds() > expire_hours * 3600: - raise HTTPException( - status_code=400, - detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", - ) + chat_shell = getattr(task_crd, "chat_shell", None) + task_shell_source = None + if chat_shell and getattr(chat_shell, "labels", None): + task_shell_source = chat_shell.labels.get("source") + + # Only skip expiration for tasks originating from the chat shell + if task_shell_source != CHAT_SHELL_SOURCE: + if ( + datetime.now() - existing_task.updated_at + ).total_seconds() > expire_hours * SECONDS_PER_HOUR: + raise HTTPException( + status_code=400, + detail=( + f"{task_type} task has expired. You can only append tasks " + f"within {expire_hours} hours after last update." + ), + )This keeps the intended behavior (only
CHAT_SHELL_SOURCEtasks bypass expiration), avoids runtime errors, and aligns with the “no magic values” rule.
🧹 Nitpick comments (7)
frontend/e2e/tests/settings-github.spec.ts (2)
23-46: Defensive checks for tokens vs empty state are good; consider trimming fixed waitsThe
hasTokens || hasEmptyState || hasLoadingOrContentpattern with.catch(() => false)is a nice way to keep the test resilient to different valid UI states. The only minor nit is the explicitawait page.waitForTimeout(1000);after anothernetworkidlewait, which adds time and can still be flaky on slow environments.If you see flakiness or want to speed this up later, consider replacing the fixed timeout with a more specific wait (e.g.,
expecton a loading skeleton orgit-tokenscontainer) rather than sleeping.If you tweak this later, verify with a focused run like
npx playwright test frontend/e2e/tests/settings-github.spec.ts -g "token list or empty state"to catch any new flakiness.
48-70: Add-token dialog flow is well covered; static delay could be replaced by a conditionThe test correctly:
- Waits for the Integrations header,
- Locates the “New Token” button (including the
"新建"label, which is nice for i18n),- Clicks it and asserts that dialog content inside
[role="dialog"]becomes visible.This should be robust across the common headlessui Dialog render paths. As a small follow-up improvement, you could likely drop the
waitForTimeout(500)and instead wait directly on the dialog locator (the subsequenttoBeVisible({ timeout: 10000 })already does most of that work).After any future change to the dialog implementation (e.g., different headlessui version), re-run just this test block to ensure the role and selectors still match.
backend/app/api/endpoints/adapter/chat.py (2)
344-383: Consider checking the return value ofsave_chat_history.The code calls
session_manager.save_chat_historybut doesn't verify the boolean return value. According to the relevant snippets, this method returnsFalseon error. A silent failure could lead to the history not being initialized, potentially causing issues for shared tasks.Apply this diff to log failures:
# Save to Redis if we found any history if history_messages: - await session_manager.save_chat_history(task_id, history_messages) - logger.info(f"Initialized {len(history_messages)} messages in Redis for task {task_id}") + success = await session_manager.save_chat_history(task_id, history_messages) + if success: + logger.info(f"Initialized {len(history_messages)} messages in Redis for task {task_id}") + else: + logger.warning(f"Failed to initialize chat history in Redis for task {task_id}")
133-383: Consider extracting the Redis initialization logic to a helper function.The
_create_task_and_subtasksfunction now exceeds 50 lines, which violates the coding guideline that Python functions SHOULD NOT exceed 50 lines. Extracting the Redis history initialization logic (lines 344-383) into a separate async helper function would improve readability and maintainability.As per coding guidelines, Python functions SHOULD NOT exceed 50 lines.
Example refactor:
async def _initialize_redis_history_from_subtasks( task_id: int, existing_subtasks: list[Subtask] ) -> None: """Initialize Redis chat history from existing DB subtasks.""" from app.services.chat.session_manager import session_manager redis_history = await session_manager.get_chat_history(task_id) if not redis_history: logger.info(f"Initializing chat history from DB for task {task_id} with {len(existing_subtasks)} existing subtasks") history_messages = [] sorted_subtasks = sorted(existing_subtasks, key=lambda s: s.message_id) for subtask in sorted_subtasks: if subtask.status == SubtaskStatus.COMPLETED: if subtask.role == SubtaskRole.USER: if subtask.prompt: history_messages.append({ "role": "user", "content": subtask.prompt }) elif subtask.role == SubtaskRole.ASSISTANT: if subtask.result and isinstance(subtask.result, dict): content = subtask.result.get("value", "") if content: history_messages.append({ "role": "assistant", "content": content }) if history_messages: await session_manager.save_chat_history(task_id, history_messages) logger.info(f"Initialized {len(history_messages)} messages in Redis for task {task_id}")Then call it in
_create_task_and_subtasks:if existing_subtasks: await _initialize_redis_history_from_subtasks(task_id, existing_subtasks)backend/app/api/endpoints/adapter/tasks.py (3)
290-293: Preserve traceback by usinglogger.exceptionin theexceptblockCatching
Exceptionbut logging onlystr(e)loses traceback context and is what Ruff’s TRY400 is flagging. Switching tologger.exceptionhere provides full stack traces for debugging.- except Exception as e: - logger.error( - f"Failed to update Chat Shell task {task_id} status: {str(e)}" - ) + except Exception: + logger.exception( + "Failed to update Chat Shell task %s status", task_id + )
395-429: Avoid== Truein ORM filters forKind.is_activeBoth branches in
join_shared_taskfilter onKind.is_active == True. Ruff flags this (E712), and using the column expression directly is clearer and more idiomatic SQLAlchemy.- user_team = ( - db.query(Kind) - .filter( - Kind.user_id == current_user.id, - Kind.kind == "Team", - Kind.id == request.team_id, - Kind.is_active == True, - ) - .first() - ) + user_team = ( + db.query(Kind) + .filter( + Kind.user_id == current_user.id, + Kind.kind == "Team", + Kind.id == request.team_id, + Kind.is_active, + ) + .first() + ) @@ - user_team = ( - db.query(Kind) - .filter( - Kind.user_id == current_user.id, - Kind.kind == "Team", - Kind.is_active == True, - ) - .first() - ) + user_team = ( + db.query(Kind) + .filter( + Kind.user_id == current_user.id, + Kind.kind == "Team", + Kind.is_active, + ) + .first() + )
370-380: Renamingtokentoshare_tokenrequires coordinated changes across frontend and documentation
get_public_shared_taskusestokenas the query parameter, whileget_task_share_infoandJoinSharedTaskRequestuseshare_token. While unifying the naming would improve consistency, this change impacts the public API contract:
- Frontend code (
frontend/src/apis/tasks.ts) explicitly usestokenparameter- Documentation (
docs/TASK_SHARING_FEATURE.md) documents the endpoint as?token=<token>Renaming requires updating both the frontend and API documentation. If proceeding with this refactoring, ensure all three locations are updated together in the same PR to maintain consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/app/api/endpoints/adapter/chat.py(3 hunks)backend/app/api/endpoints/adapter/tasks.py(4 hunks)backend/app/core/config.py(1 hunks)backend/app/services/adapters/task_kinds.py(1 hunks)frontend/e2e/tests/settings-github.spec.ts(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(6 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/i18n/locales/zh-CN/chat.json
- frontend/src/i18n/locales/en/chat.json
- frontend/src/features/tasks/components/ChatArea.tsx
- backend/app/core/config.py
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/e2e/tests/settings-github.spec.tsbackend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.pyfrontend/src/features/tasks/components/MessagesArea.tsxbackend/app/services/adapters/task_kinds.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/e2e/tests/settings-github.spec.tsfrontend/src/features/tasks/components/MessagesArea.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/e2e/tests/settings-github.spec.tsfrontend/src/features/tasks/components/MessagesArea.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.pybackend/app/services/adapters/task_kinds.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.py
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/task_kinds.py
🧠 Learnings (1)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/*.py : Python code MUST extract magic numbers to named constants
Applied to files:
backend/app/services/adapters/task_kinds.py
🧬 Code graph analysis (3)
backend/app/api/endpoints/adapter/tasks.py (4)
backend/app/schemas/shared_task.py (5)
JoinSharedTaskRequest(27-33)JoinSharedTaskResponse(36-40)PublicSharedTaskResponse(94-101)TaskShareInfo(11-17)TaskShareResponse(20-24)backend/app/services/shared_task.py (2)
share_task(188-215)join_shared_task(368-467)backend/app/core/security.py (1)
get_current_user(28-50)backend/app/models/kind.py (1)
Kind(25-42)
backend/app/api/endpoints/adapter/chat.py (3)
backend/app/services/chat/session_manager.py (2)
get_chat_history(56-84)save_chat_history(86-116)backend/app/models/subtask.py (2)
SubtaskStatus(17-23)SubtaskRole(26-28)backend/app/schemas/subtask.py (2)
SubtaskStatus(25-31)SubtaskRole(34-36)
frontend/src/features/tasks/components/MessagesArea.tsx (3)
frontend/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)frontend/src/components/ui/button.tsx (1)
Button(55-55)frontend/src/features/tasks/components/TaskShareModal.tsx (1)
TaskShareModal(22-98)
🪛 Ruff (0.14.7)
backend/app/api/endpoints/adapter/tasks.py
291-293: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
338-338: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
339-339: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
361-361: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
373-373: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
386-386: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
387-387: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
403-403: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
420-420: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/services/adapters/task_kinds.py
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (11)
frontend/e2e/tests/settings-github.spec.ts (2)
1-8: Shared setup for integrations tab looks solidUsing a single
beforeEachto navigate to/settings?tab=integrationsand waiting fornetworkidleshould keep the suite stable and avoids duplication across tests. No issues here.Please re-run this suite in your CI environment to confirm the navigation still works with your latest routing/layout changes.
10-21: Basic page and section visibility assertions are appropriateThe checks for the
/settingsURL and the"Integrations"header are straightforward and give good early failure signals if the tab structure changes. This looks good as a smoke check for the Git integrations area.Ensure these tests pass against both light/dark themes or any A/B variants of the settings page if applicable.
backend/app/api/endpoints/adapter/chat.py (2)
133-140: LGTM: Async conversion enables Redis operations.The function signature change to
async defis appropriate for the new Redis-backed history initialization logic. The enriched summary confirms all call sites have been updated to useawait.
477-479: LGTM: Correct await usage.The call site correctly uses
awaitfor the async_create_task_and_subtasksfunction.backend/app/api/endpoints/adapter/tasks.py (1)
17-23: Shared-task imports and service wiring look correctThe new schema and service imports align with the usage in the new endpoints and keep responsibilities well factored into shared_task_service. No issues here.
Also applies to: 33-33
frontend/src/features/tasks/components/MessagesArea.tsx (6)
7-7: LGTM!The new imports are appropriate for the task sharing functionality:
useCallbackanduseMemofor optimization,Share2icon for the UI,useToastfor notifications,TaskShareModalfor the modal component, andtaskApisfor the API call.Also applies to: 17-17, 20-20, 27-28
180-181: LGTM!The render prop pattern for
onShareButtonRenderis a clean approach to allow parent components to control the placement of the share button in the UI.Also applies to: 200-200
207-210: LGTM!The state variables are appropriately named and typed for managing the task sharing flow.
215-241: LGTM!The
handleShareTaskcallback is well-implemented with proper error handling, loading states, and user feedback via toasts. The use ofuseCallbackwith correct dependencies prevents unnecessary re-renders.
540-558: LGTM!The share button is properly memoized with correct dependencies. The condition
displayMessages.length === 0prevents sharing empty tasks, which is good UX. Note that the button may be briefly hidden during initial load if messages haven't loaded yet, but this is acceptable behavior.
708-714: LGTM!The
TaskShareModalis rendered with correct props, and the fallback'Untitled Task'provides a sensible default when the task title is unavailable.
| @router.post("/{task_id}/share", response_model=TaskShareResponse) | ||
| def share_task( | ||
| task_id: int, | ||
| current_user: User = Depends(security.get_current_user), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Generate a share link for a task. | ||
| The share link allows others to view the task history and copy it to their task list. | ||
| """ | ||
| # Validate that the task belongs to the current user | ||
| if not shared_task_service.validate_task_exists( | ||
| db=db, task_id=task_id, user_id=current_user.id | ||
| ): | ||
| raise HTTPException( | ||
| status_code=404, detail="Task not found or you don't have permission" | ||
| ) | ||
|
|
||
| return shared_task_service.share_task( | ||
| db=db, task_id=task_id, user_id=current_user.id | ||
| ) | ||
|
|
||
|
|
||
| @router.get("/share/info", response_model=TaskShareInfo) | ||
| def get_task_share_info( | ||
| share_token: str = Query(..., description="Share token from URL"), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Get task share information from share token. | ||
| This endpoint doesn't require authentication, so anyone with the link can view. | ||
| """ | ||
| return shared_task_service.get_share_info(db=db, share_token=share_token) | ||
|
|
||
|
|
||
| @router.get("/share/public", response_model=PublicSharedTaskResponse) | ||
| def get_public_shared_task( | ||
| token: str = Query(..., description="Share token from URL"), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Get public shared task data for read-only viewing. | ||
| This endpoint doesn't require authentication - anyone with the link can view. | ||
| Only returns public data (no sensitive information like team config, bot details, etc.) | ||
| """ | ||
| return shared_task_service.get_public_shared_task(db=db, share_token=token) | ||
|
|
||
|
|
||
| @router.post("/share/join", response_model=JoinSharedTaskResponse) | ||
| def join_shared_task( | ||
| request: JoinSharedTaskRequest, | ||
| current_user: User = Depends(security.get_current_user), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Copy a shared task to the current user's task list. | ||
| This creates a new task with all the subtasks (messages) from the shared task. | ||
| """ | ||
| from app.models.kind import Kind | ||
|
|
||
| # If team_id is provided, validate it belongs to the user | ||
| if request.team_id: | ||
| user_team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == current_user.id, | ||
| Kind.kind == "Team", | ||
| Kind.id == request.team_id, | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
|
|
||
| if not user_team: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="Invalid team_id or team does not belong to you", | ||
| ) | ||
| else: | ||
| # Get user's first active team if not specified | ||
| user_team = ( | ||
| db.query(Kind) | ||
| .filter( | ||
| Kind.user_id == current_user.id, | ||
| Kind.kind == "Team", | ||
| Kind.is_active == True, | ||
| ) | ||
| .first() | ||
| ) | ||
|
|
||
| if not user_team: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="You need to have at least one team to copy a shared task", | ||
| ) | ||
|
|
||
| return shared_task_service.join_shared_task( | ||
| db=db, | ||
| share_token=request.share_token, | ||
| user_id=current_user.id, | ||
| team_id=user_team.id, | ||
| model_id=request.model_id, | ||
| force_override_bot_model=request.force_override_bot_model or False, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add return type hints and avoid magic HTTP status numbers on new endpoints
The new sharing endpoints look logically correct and delegate cleanly to shared_task_service, but they currently lack explicit return type hints and use bare 400/404 literals in HTTPException, even though status is already imported.
To align with the Python coding guidelines (type hints + avoiding magic numbers), consider updating all four endpoints like this (pattern shown; apply similarly to the others):
-@router.post("/{task_id}/share", response_model=TaskShareResponse)
-def share_task(
+@router.post("/{task_id}/share", response_model=TaskShareResponse)
+def share_task(
task_id: int,
current_user: User = Depends(security.get_current_user),
db: Session = Depends(get_db),
-):
+ ) -> TaskShareResponse:
@@
- if not shared_task_service.validate_task_exists(
- db=db, task_id=task_id, user_id=current_user.id
- ):
- raise HTTPException(
- status_code=404, detail="Task not found or you don't have permission"
- )
+ if not shared_task_service.validate_task_exists(
+ db=db, task_id=task_id, user_id=current_user.id
+ ):
+ raise HTTPException(
+ status_code=status.HTTP_404_NOT_FOUND,
+ detail="Task not found or you don't have permission",
+ )
@@
-@router.get("/share/info", response_model=TaskShareInfo)
-def get_task_share_info(
+@router.get("/share/info", response_model=TaskShareInfo)
+def get_task_share_info(
share_token: str = Query(..., description="Share token from URL"),
db: Session = Depends(get_db),
-):
+ ) -> TaskShareInfo:
@@
-@router.get("/share/public", response_model=PublicSharedTaskResponse)
-def get_public_shared_task(
- token: str = Query(..., description="Share token from URL"),
- db: Session = Depends(get_db),
-):
+@router.get("/share/public", response_model=PublicSharedTaskResponse)
+def get_public_shared_task(
+ token: str = Query(..., description="Share token from URL"),
+ db: Session = Depends(get_db),
+) -> PublicSharedTaskResponse:
@@
-@router.post("/share/join", response_model=JoinSharedTaskResponse)
-def join_shared_task(
+@router.post("/share/join", response_model=JoinSharedTaskResponse)
+def join_shared_task(
request: JoinSharedTaskRequest,
current_user: User = Depends(security.get_current_user),
db: Session = Depends(get_db),
-):
+ ) -> JoinSharedTaskResponse:
@@
- raise HTTPException(
- status_code=400,
- detail="Invalid team_id or team does not belong to you",
- )
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="Invalid team_id or team does not belong to you",
+ )
@@
- raise HTTPException(
- status_code=400,
- detail="You need to have at least one team to copy a shared task",
- )
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="You need to have at least one team to copy a shared task",
+ )As per coding guidelines, this keeps the API signatures explicit and removes magic status-code literals.
🧰 Tools
🪛 Ruff (0.14.7)
338-338: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
339-339: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
361-361: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
373-373: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
386-386: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
387-387: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
403-403: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
420-420: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/tasks.py around lines 335 to 438, the four
share endpoints should have explicit function return type hints and avoid magic
HTTP status numbers; add return type annotations matching each response_model
(e.g., -> TaskShareResponse, -> TaskShareInfo, -> PublicSharedTaskResponse, ->
JoinSharedTaskResponse) to the respective function signatures, and replace
literal status_code=400/404 in HTTPException calls with the status constants
(status.HTTP_400_BAD_REQUEST, status.HTTP_404_NOT_FOUND) already available from
fastapi; keep behavior the same otherwise.
| // Pass share button to parent for rendering in TopNavigation | ||
| useEffect(() => { | ||
| if (onShareButtonRender) { | ||
| onShareButtonRender(shareButton); | ||
| } | ||
| }, [onShareButtonRender, shareButton]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find MessagesArea component and its prop interface
rg -n "MessagesArea" --type=tsx --type=ts frontend/src | head -20Repository: wecode-ai/Wegent
Length of output: 87
🏁 Script executed:
# Search for onShareButtonRender usage and definitions
rg -n "onShareButtonRender" --type=tsx --type=ts -B 3 -A 3Repository: wecode-ai/Wegent
Length of output: 87
🏁 Script executed:
# Check the MessagesArea component definition to understand its props
rg -n "interface.*Props\|type.*Props" frontend/src/features/tasks/components/MessagesArea.tsx -A 10Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Find MessagesArea component and its usage
rg "MessagesArea" frontend/src -n | head -30Repository: wecode-ai/Wegent
Length of output: 657
🏁 Script executed:
# Search for onShareButtonRender usage
rg "onShareButtonRender" frontend/src -n -B 2 -A 2Repository: wecode-ai/Wegent
Length of output: 3834
🏁 Script executed:
# Check the MessagesArea component file directly
head -100 frontend/src/features/tasks/components/MessagesArea.tsxRepository: wecode-ai/Wegent
Length of output: 3072
🏁 Script executed:
# Check chat/page.tsx for handleShareButtonRender definition
rg -n "handleShareButtonRender" frontend/src/app/\(tasks\)/chat/page.tsx -B 5 -A 10Repository: wecode-ai/Wegent
Length of output: 1042
🏁 Script executed:
# Check code/page.tsx for handleShareButtonRender definition
rg -n "handleShareButtonRender" frontend/src/app/\(tasks\)/code/page.tsx -B 5 -A 10Repository: wecode-ai/Wegent
Length of output: 1346
🏁 Script executed:
# Find shareButton creation in MessagesArea
rg -n "shareButton" frontend/src/features/tasks/components/MessagesArea.tsx | head -20Repository: wecode-ai/Wegent
Length of output: 191
🏁 Script executed:
# Check ChatArea to see if it also doesn't memoize onShareButtonRender
rg -n "onShareButtonRender" frontend/src/features/tasks/components/ChatArea.tsx -B 10 -A 2 | head -40Repository: wecode-ai/Wegent
Length of output: 1332
Wrap handleShareButtonRender in useCallback in parent components to prevent unnecessary effect triggers.
The useEffect at lines 560-565 depends on onShareButtonRender, which is passed from parent pages (chat/page.tsx and code/page.tsx). Currently, these parent components define handleShareButtonRender as a regular arrow function without useCallback, causing a new function to be created on every render. This makes the dependency change on each parent re-render, triggering the effect unnecessarily even though shareButton is memoized.
In frontend/src/app/(tasks)/chat/page.tsx (lines 68-70) and frontend/src/app/(tasks)/code/page.tsx (lines 60-62), wrap handleShareButtonRender with useCallback:
const handleShareButtonRender = useCallback((button: React.ReactNode) => {
setShareButton(button);
}, []);🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/MessagesArea.tsx around lines 560 to
565, the effect depends on onShareButtonRender which is recreated on every
parent render; update the parent handlers in
frontend/src/app/(tasks)/chat/page.tsx (lines ~68-70) and
frontend/src/app/(tasks)/code/page.tsx (lines ~60-62) to wrap
handleShareButtonRender in useCallback with an empty dependency array so the
function identity is stable (e.g., useCallback((button) =>
setShareButton(button), [])), then re-run tests to ensure the MessagesArea
effect only triggers when shareButton actually changes.
#302) * feat: Added a shared task model and updated user relationships and configurations. * feat: 分享页面移除侧边栏
* feat(uv): improve startup and fix chat flash * style: improve UI (#305) * Fix unread status (#306) * fix: fix unread status * refactor: backend isort&black * style: update user message style (#307) * feat: Added a shared task model and updated user relationships and co… (#302) * feat: Added a shared task model and updated user relationships and configurations. * feat: 分享页面移除侧边栏 * feat(uv): improve startup and fix chat flash * style(backend): fix black and isort formatting * fix(frontend): prevent chat flash and improve message UI consistency - Fix flash of empty state during task refresh by keeping messages visible when subtasks exist - Move streaming message rendering to use MessageBubble component for consistency - Show copy button for user messages at all times (not just on hover) - Enable copy/download buttons during streaming instead of hiding them - Remove unnecessary group hover effects that caused UI inconsistency - Keep export PDF button visible but disabled during streaming - Simplify message bubble spacing and remove redundant wrapper divs * feat(merge): merge main * feat(merge): 回滚修改 * fix(docker): fix: 复制 postinstall 脚本以支持构建 * feat(merge): fix: 修正后端服务器启动命令 --------- Co-authored-by: hongyu9 <hongyu9@staff.weibo.com> Co-authored-by: axb <qindi@staff.weibo.com> Co-authored-by: feifei <yansheng3@staff.weibo.com>
…nfigurations.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.