Skip to content

Conversation

@zinzied
Copy link

@zinzied zinzied commented May 23, 2025

Summary

This PR fixes two critical issues preventing the application from running properly:

  1. Frontend hydration error: Resolves nested button elements causing React hydration failures
  2. Backend permission error: Handles PermissionError when attempting to remove log files

Changes Made

Frontend Fix (frontend/components/question-input.tsx)

  • Added asChild prop to TooltipTrigger component to prevent nested button DOM structure
  • Fixes React hydration error: "In HTML, cannot be a descendant of "
  • Ensures proper Radix UI component composition

Before:

<TooltipTrigger>
  <Button>...</Button>
</TooltipTrigger>
**After:**
<TooltipTrigger asChild>
  <Button>...</Button>
</TooltipTrigger>
### Backend Fix ( cli.py)
- Added try-catch block to handle PermissionError when removing existing log files
- Prevents application crash when log files are in use by other processes
- Allows graceful continuation when log file cleanup fails
**Before:**
if os.path.exists(args.logs_path):
    os.remove(args.logs_path)
**After:**
if os.path.exists(args.logs_path):
    try:
        os.remove(args.logs_path)
    except PermissionError:
        # File is in use, just continue
        pass

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zinzied Could you remove changes of yarn.lock file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sure

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've already added feature cancel so this code is duplicate. Please revert the changes.

return " ".join(docker_parts)


class BashTool(LLMTool):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR! For running commands on Windows (cmd), I think it would be cleaner to separate the logic into a dedicated tool (e.g., CmdTool) rather than mixing it into the existing implementation.

In the future, when we add sandbox support, we'll only need to support Linux, so keeping the platforms decoupled now will make things easier to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants