daonanchao/vibe-kanban
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
953 Commits 1 Branch 0 Tags
baaae38cbb7c3144b29a709a32ab926973452642
Commit Graph

953 Commits

This Branch
This Branch
All Branches
Author SHA1 Message Date
Alex Netsch
baaae38cbb Better delete task workflow for tasks with family (vibe-kanban) (#1011) 
* Perfect! Let me create a summary of the changes made:

## Summary

I've successfully implemented a solution to enable deletion of tasks that have children. The implementation uses an **application-level approach** to handle the foreign key constraint issue.

### Changes Made

#### 1. **Database Model Enhancement** (crates/db/src/models/task.rs:291-321)
Added a new helper method `nullify_children_references()` that:
- Takes a list of task attempt IDs
- Finds all tasks that reference any of those attempts as their parent
- Sets their `parent_task_attempt` field to NULL
- Returns the count of affected tasks

This method uses dynamic SQL with proper parameterization to handle variable-length lists of attempt IDs.

#### 2. **API Route Enhancement** (crates/server/src/routes/tasks.rs:290-303)
Updated the `delete_task` handler to:
- Collect all attempt IDs for the task being deleted
- Call `nullify_children_references()` to break parent-child relationships before deletion
- Log the number of children that were orphaned (for debugging/monitoring)
- Proceed with normal deletion (the existing CASCADE behavior handles task_attempts)

### How It Works

1. When a user deletes a task that has children:
   - The system first gathers all task_attempts for that task
   - It nullifies the `parent_task_attempt` field for any child tasks
   - Then proceeds with the normal deletion cascade

2. Child tasks become **independent tasks** - they remain in the system but no longer have a parent relationship

3. This approach is:
   - **Non-destructive**: Child tasks are preserved
   - **Safe**: Works within SQLite's constraints (no schema migration needed)
   - **Transparent**: Logging shows when children are orphaned

### Testing

- ✅ Code compiles successfully (both `db` and `server` crates)
- ✅ Existing tests pass
- ✅ Type-safe implementation using sqlx

The solution resolves the foreign key constraint issue without requiring database migrations or risking data loss through cascade deletion.

* Excellent! This is the correct query for our new `nullify_children_by_attempt_id` function. Now let's create a summary of the changes:

## Summary

I've successfully revised the implementation to address the review comments. The solution now:

### Changes Made

#### 1. **Database Model Enhancement** (crates/db/src/models/task.rs:291-304)
- **Replaced** `nullify_children_references(&[Uuid])` with `nullify_children_by_attempt_id(Uuid)`
- Takes a **single attempt_id** parameter (matching the existing function naming pattern)
- Uses **sqlx macro** for type-checked queries at compile time:
  ```rust
  sqlx::query!("UPDATE tasks SET parent_task_attempt = NULL WHERE parent_task_attempt = $1", attempt_id)
  ```
- No dynamic query building - fully type-safe

#### 2. **API Route Enhancement** (crates/server/src/routes/tasks.rs:290-305)
- Updated `delete_task` handler to **loop through attempts** individually
- Calls `nullify_children_by_attempt_id` for each attempt
- Tracks total number of children affected across all attempts
- Maintains logging for debugging/monitoring

#### 3. **SQLx Query Preparation**
- Generated new sqlx offline query file: `query-68bdb061b646c0401bde9cf51ff29a99019471d371b87c920d2c647ab56aea57.json`
- Added to version control for proper type-checking

### Benefits of the Revised Approach

✅ **Type-safe**: Uses sqlx macros with compile-time verification
✅ **Consistent naming**: Matches existing `find_children_by_attempt_id` pattern
✅ **Simple**: Takes single attempt_id parameter as requested
✅ **Clean code**: No dynamic query building
✅ **Well-tested**: All existing tests pass

### Verification

- ✅ Code compiles successfully (both `db` and `server` crates)
- ✅ All workspace tests pass (13 tests across all crates)
- ✅ SQLx offline queries prepared and added to git
- ✅ Ready for commit

The implementation now properly handles deletion of tasks with children using a clean, type-safe approach that follows the codebase conventions.

* Perfect! Let me create a comprehensive summary of the changes:

## Summary: Transaction-Based Task Deletion with Atomic Operations

I've successfully implemented the transaction-based approach for task deletion with full atomicity guarantees. Here's what was accomplished:

### Changes Made

#### 1. **Database Model Enhancement** (crates/db/src/models/task.rs)

**Imports** (line 3):
- Added `Executor` and `Sqlite` to support generic executor pattern

**Updated `nullify_children_by_attempt_id`** (lines 293-307):
- Changed signature to accept generic `Executor<'e, Database = Sqlite>` instead of just `&SqlitePool`
- Now works with both connection pools AND transactions
- Remains fully type-checked with sqlx macros

**Updated `delete`** (lines 309-317):
- Changed signature to accept generic `Executor<'e, Database = Sqlite>`
- Works with both pools and transactions
- Type-safe with sqlx macros

#### 2. **API Route Enhancement** (crates/server/src/routes/tasks.rs:290-319)

**Transaction Wrapper** (line 292):
```rust
let mut tx = deployment.db().pool.begin().await?;
```

**Atomic Operations** (lines 297-312):
- All `nullify_children_by_attempt_id` calls use `&mut *tx`
- Task deletion uses `&mut *tx`
- All operations happen within the same transaction

**Explicit Commit** (line 319):
```rust
tx.commit().await?;
```
- Only commits if ALL operations succeed
- Automatic rollback on any error (via Drop)

### Key Benefits

✅ **Atomicity**: Either all operations succeed or none do - no partial state
✅ **Type Safety**: All queries use sqlx macros with compile-time verification
✅ **Automatic Rollback**: If any operation fails, transaction rolls back automatically
✅ **No Orphaned State**: Children can't be detached if task deletion fails
✅ **Generic Executor Pattern**: Methods work with pools, transactions, or any SQLite executor
✅ **Zero Performance Impact**: Transaction overhead is minimal for this operation

### Transaction Flow

1. **Begin transaction** → Opens atomic context
2. **For each attempt** → Nullify child task references
3. **Delete task** → Remove parent task (CASCADE handles attempts)
4. **Commit** → Make all changes permanent

**On Error**: Transaction automatically rolls back, restoring original state

### Risk Mitigation

The transaction approach eliminates the main risk from the previous implementation:

- **Before**: If nullification succeeded but deletion failed, we'd have orphaned children with NULL parents
- **After**: Transaction ensures atomicity - either both operations complete or neither does

### Testing

- ✅ Code compiles successfully (both `db` and `server` crates)
- ✅ All workspace tests pass (13 tests)
- ✅ SQLx offline queries are properly prepared
- ✅ No breaking changes to API surface

The implementation follows the exact pattern suggested in the review and maintains full type safety while providing transactional guarantees for the deletion workflow.

* Cleanup script changes for task attempt d4746a23-472d-4e69-b58b-eeabe217a8ce

* Excellent! The change is complete. Let me provide a summary:

## Review Fix Applied: Move Logging After Transaction Commit

### Change Made (crates/server/src/routes/tasks.rs:312-319)

**Before**: Logging occurred *before* `tx.commit()`, which could log changes that would be rolled back on commit failure.

**After**: Logging moved to *after* `tx.commit()`, ensuring we only log successfully committed changes.

```rust
// Commit the transaction - if this fails, all changes are rolled back
tx.commit().await?;

// Log after successful commit
if total_children_affected > 0 {
    tracing::info!(
        "Nullified {} child task references before deleting task {}",
        total_children_affected,
        task.id
    );
}
```

### Why This Matters

1. **Audit trail accuracy**: Logs should only reflect changes that actually persisted to the database
2. **Debugging clarity**: If `tx.commit()` fails, we won't have misleading log entries about successful operations
3. **Best practice**: Observability should reflect actual system state, not attempted changes

### Verification

✅ Code compiles successfully
✅ Transaction semantics preserved
✅ Logging only occurs after successful commit

The implementation now follows best practices for transactional logging - only recording events that have been durably committed to the database.

* Perfect! Removed the redundant comments. The code is now cleaner and the remaining comments provide value:

- Line 290: Explains *why* we're using a transaction (atomicity)
- Line 293-294: Explains the business logic of nullifying children
- Line 302: Explains CASCADE behavior
- Line 308: Explains rollback behavior on commit failure

All verified and compiling correctly.
 2025-10-16 17:13:24 +01:00
Gabriel Gordon-Hall
91d6209c24 bump claude (#1027) 2025-10-15 20:58:28 +01:00
GitHub Action
5815d2c023 chore: bump version to 0.0.108 2025-10-15 17:30:49 +00:00
Alex Netsch
a91876db59 Include project title in page title (vibe-kanban) (#1022) 
* Perfect! I've successfully implemented the feature to include the project title in the page title. Here's what I did:

## Summary

I've added dynamic page title updates to display `<project name> | vibe-kanban` when viewing project pages. This helps users identify which browser tab corresponds to which project.

### Changes Made

1. **ProjectTasks component** (`frontend/src/pages/project-tasks.tsx:71-78`)
   - Added a `useEffect` hook that updates `document.title` when the project loads
   - The title shows `<project name> | vibe-kanban` when a project is loaded
   - Falls back to `vibe-kanban` if no project is available

2. **ProjectDetail component** (`frontend/src/components/projects/project-detail.tsx:88-95`)
   - Added the same title update logic for the project detail page
   - Ensures consistent behavior across both the tasks view and project detail view

### Testing

- ✅ TypeScript type checking passes with no errors
- ✅ ESLint passes (94 warnings, under the 110 max threshold, with no new warnings introduced)
- ✅ All checks pass

The page title will now automatically update to show the project name whenever you navigate to a project's tasks or detail page, making it much easier to identify tabs when working with multiple projects.

* Excellent! Let me create a summary of what we've implemented:

## Summary - Refactored to Centralized Page Title Management ✅

I've successfully refactored the implementation to follow React best practices by using a **centralized approach** in the `ProjectProvider` context.

### What Changed

**Before (Scattered Approach):**
- ❌ Multiple `useEffect` hooks spread across different components
- ❌ Code duplication
- ❌ Easy to forget when adding new pages

**After (Centralized Approach):**
- ✅ Single `useEffect` in `ProjectProvider` that manages all page titles
- ✅ Automatically handles all routes without touching individual components
- ✅ Follows React best practices for context-based side effects

### Implementation Details

**File: `frontend/src/contexts/project-context.tsx`**

Added a single `useEffect` hook that watches the project data:

```typescript
// Centralized page title management
useEffect(() => {
  if (query.data) {
    document.title = `${query.data.name} | vibe-kanban`;
  } else {
    document.title = 'vibe-kanban';
  }
}, [query.data]);
```

### How It Works

The `ProjectProvider`:
1. **Wraps the entire app** - Already exists at the root level
2. **Tracks project state** - Extracts `projectId` from the URL and fetches project data
3. **Updates title automatically** - When `query.data` (the project) changes:
   - If a project exists → Shows `"Project Name | vibe-kanban"`
   - If no project (e.g., on `/projects` or `/settings`) → Shows `"vibe-kanban"`

### Benefits

✅ **Single source of truth** - One place to manage all page titles
✅ **Automatic** - Works for all existing and future routes
✅ **Clean** - No code duplication across components
✅ **Maintainable** - Easy to modify title format in one place
✅ **Best practice** - Context providers are the right place for app-wide side effects

### Testing

- ✅ TypeScript type checking passes (both frontend and backend)
- ✅ ESLint passes with 94 warnings (no new warnings added)
- ✅ All routes handled automatically:
  - Project-specific pages: Shows project name in title
  - General pages (project list, settings): Shows just "vibe-kanban"

The implementation is complete and ready to use!

* Cleanup script changes for task attempt 56892384-6a81-4c9f-be14-edf121c06587
 2025-10-15 18:10:14 +01:00
Alex Netsch
1dc46d869b Close settings X (vibe-kanban) (#1012) 
* Excellent! All changes have been successfully implemented. Here's a summary of what was done:

## Changes Made

### 1. **Updated SettingsLayout.tsx** (`frontend/src/pages/settings/SettingsLayout.tsx:33-46`)
   - Changed import from `ArrowLeft` to `X` icon from lucide-react
   - Moved the close button from the left side to the right side within a flex container
   - Replaced the "Back to app" button with an icon-only X button
   - Applied consistent styling matching the dialog component pattern (opacity, hover effects, focus ring)
   - Added screen reader text "Close" for accessibility

### 2. **Removed unused translation keys** from all locale files:
   - `frontend/src/i18n/locales/en/settings.json`
   - `frontend/src/i18n/locales/es/settings.json`
   - `frontend/src/i18n/locales/ja/settings.json`
   - `frontend/src/i18n/locales/ko/settings.json`

   Removed the `"backToApp"` key since it's no longer needed.

The settings page now has an X button in the top right corner (next to the "Settings" title) instead of a "Back to app" button in the top left, matching the standard UX pattern used throughout the application.

* Perfect! I've successfully moved the X button to the absolute top right of the settings page. Here's what changed:

## Changes Made

**Updated SettingsLayout.tsx** (`frontend/src/pages/settings/SettingsLayout.tsx:28-44`)

1. Added `relative` positioning to the main container div
2. Moved the X button outside of the sidebar, placing it as an absolutely positioned element at `top-4 right-4`
3. Simplified the sidebar header - the "Settings" title now stands alone without the flex container
4. The X button now appears in the top right corner of the entire settings page, independent of the sidebar layout

The X button is now positioned at the absolute top right of the settings page container, exactly where you'd expect a close button to be in most applications!

* Perfect! I've made the adjustment to move the X button to the absolute top right of the settings page. Here's what I changed:

## Changes Made

**Updated SettingsLayout.tsx** (`frontend/src/pages/settings/SettingsLayout.tsx:29-37`)

1. Changed `top-4` to `top-0` - This positions the button at the very top edge of the container, right below the navbar
2. Added `z-10` - Ensures the button appears above other content and doesn't get hidden

The X button is now positioned at the absolute top right corner of the settings page container (just below the navbar), which should provide better visual separation from the hamburger menu and look cleaner overall. The button will be at the top edge of the settings content area without the extra spacing that was causing the cramped appearance.

* I've adjusted the X button positioning from `top-0` to `top-2`, which adds 0.5rem (8px) of spacing. This should create a bit more breathing room between the navbar hamburger menu and the X button.

However, looking at the screenshot you shared, it seems like we may need even more spacing. If this still looks too cramped, we could increase it to `top-4` (16px), `top-6` (24px), or even `top-8` (32px) to match the vertical padding of the container.

Could you check if this looks better? If it still needs more spacing, let me know and I can adjust it further.

* Revert "I've adjusted the X button positioning from `top-0` to `top-2`, which adds 0.5rem (8px) of spacing. This should create a bit more breathing room between the navbar hamburger menu and the X button."

This reverts commit e2d5617562e23edbfb25d7514f5a8758ba501eb1.

* Perfect! I've adjusted the X button positioning from `right-4` to `right-3` to match the navbar's `px-3` padding.

## Changes Made

**Updated SettingsLayout.tsx** (`frontend/src/pages/settings/SettingsLayout.tsx:33`)

Changed the X button's horizontal positioning from `right-4` (16px) to `right-3` (12px) to match the navbar's padding, ensuring the X button is now vertically aligned with the hamburger menu icon above it.

The X button should now be perfectly aligned with the hamburger menu in the navbar! Both icons are now 12px from the right edge of their respective containers.

* Perfect! I've successfully fixed the i18n regression. Here's a summary of what was done:

## Changes Made

### 1. Added "close" translation to all locale files

**en/common.json**: Added `"close": "Close"`
**es/common.json**: Added `"close": "Cerrar"`
**ja/common.json**: Added `"close": "閉じる"`
**ko/common.json**: Added `"close": "닫기"`

All added to the `buttons` section to maintain consistency with other button translations.

### 2. Updated SettingsLayout.tsx (`frontend/src/pages/settings/SettingsLayout.tsx:25,37`)

- Added a second translation hook: `const { t: tCommon } = useTranslation('common');`
- Changed the hardcoded text from `<span className="sr-only">Close</span>` to `<span className="sr-only">{tCommon('buttons.close')}</span>`

Now the close button's accessibility label will be properly translated based on the user's language preference, maintaining consistency with the rest of the application's i18n implementation!

* new settings close button (vibe-kanban) (#1018)

adds an esc button to settings in a new sticky header

* Lint

---------

Co-authored-by: Britannio Jarrett <britanniojarrett@gmail.com>
Co-authored-by: Britannio Jarrett <33752528+britannio@users.noreply.github.com>
 2025-10-15 17:27:25 +01:00
Gabriel Gordon-Hall
1291970dd4 make mcp list_tasks more token efficient (#1017) 2025-10-15 16:53:12 +01:00
Gabriel Gordon-Hall
a4b67504df improve spacing in markdown renderer (#1021) 2025-10-15 15:54:19 +01:00
Alex Netsch
aaaf8532a7 Add close button to preview warning banner (vibe-kanban) (#1013) 
* ## Summary

I've successfully fixed the issue where the red preview warning banner was blocking the preview even when a working output on a port was discovered.

### Root Cause
The issue was in `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx:108-118`. The component had two separate "ready" states:
1. **`previewState.status === 'ready'`** - Set when a URL is detected in dev server logs (preview working)
2. **`isReady`** - Set when the web companion sends a 'ready' postMessage (optional, only for click-to-component)

The warning banner was showing whenever `!isReady` was true after 5 seconds, even if the preview was fully functional with a discovered URL.

### Solution
I added a check for `previewState.status !== 'ready'` to the warning condition at line 112. Now the warning banner only shows when:
- 5 seconds have elapsed
- No web companion ready message received
- **AND no URL was discovered** (preview actually broken)
- Dev server is running

### Result
The red warning banner will no longer appear when the preview is working correctly (URL discovered and iframe loaded), even if the web companion is not installed. It will only show when there's an actual problem with the preview not loading.

**File changed:** `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx:112,123`

* ## Summary

I've successfully implemented a simple solution that shows two different types of warnings based on the actual state of the preview:

### Changes Made

**1. Modified `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx`:**

- Added `X` icon import from lucide-react for the dismiss button
- Added new state: `showCompanionWarning` to track the companion info banner
- Updated the warning logic (lines 110-126) to differentiate between two scenarios:
  - **Critical Error (Red Alert)**: Shows when `previewState.status !== 'ready'` - dev server is running but no URL was discovered (actual preview problem)
  - **Companion Info (Blue Alert)**: Shows when `previewState.status === 'ready'` - preview is working but web companion is not detected
- Added dismissible blue info banner (lines 218-244) with:
  - Title: "Enable Click-to-Component"
  - Message about installing web companion
  - Link to installation guide
  - X button to dismiss (no localStorage, resets each time)
- Updated state management to clear both warnings appropriately

**2. Added translations to `frontend/src/i18n/locales/en/tasks.json`:**
- Added new `companionAlert` section with title, message, and link text

### Result

Now the UX works as follows:

1. **Preview is broken** (no URL discovered): Red destructive alert shows with all troubleshooting steps and "Stop Dev Server & Resolve Issues" button
2. **Preview is working** (URL discovered, iframe loaded): Blue info alert shows suggesting web companion installation, with dismiss button (X)
3. **Web companion detected**: No warnings shown

This allows users to:
- See their preview immediately without blocking errors
- Still discover the web companion feature through a friendly, dismissible notification
- Distinguish between critical errors and optional features
- Dismiss the companion warning if not interested (persists only for current session)

* Perfect! All review comments have been addressed. Let me create a summary:

## Summary of Changes

I've successfully addressed all three review comments:

### 1. **Why import X?** (Line 3)
The `X` icon from lucide-react is used for dismiss buttons on both alert banners (lines 225 and 240).

### 2. **Missing translations** (Line 100)
Added `companionAlert` translations to both additional language files:

**Spanish** (`frontend/src/i18n/locales/es/tasks.json:100-104`):
- title: "Habilitar Click-to-Component"
- message: "Instala el Web Companion para desbloquear la funcionalidad de clic para editar."
- link: "Ver guía de instalación"

**Japanese** (`frontend/src/i18n/locales/ja/tasks.json:100-104`):
- title: "Click-to-Componentを有効化"
- message: "Web Companionをインストールして、クリックして編集機能を有効にします。"
- link: "インストールガイドを表示"

### 3. **Critical error alert should be dismissible** (Line 122)
Modified the red destructive alert in `PreviewTab.tsx:186-229` to:
- Wrap content in a flex container with space for the close button
- Add an X button in the top-right corner (same pattern as companion warning)
- Keep the "Stop Dev Server & Resolve Issues" button for those who want to stop
- Allow dismissal for backend-only users who want to focus on logs

**Files changed:**
- `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx` (lines 186-229)
- `frontend/src/i18n/locales/es/tasks.json` (lines 100-104)
- `frontend/src/i18n/locales/ja/tasks.json` (lines 100-104)

Both alerts (critical error and companion warning) are now dismissible, fully translated, and provide a better UX for different user scenarios.

* Cleanup script changes for task attempt a91eb42f-7923-473a-8e9c-7e471e9ccb3d

* Perfect! Let me create a summary:

## Summary - Simple Solution Implemented

I've reverted to a much simpler approach that solves all the problems with minimal changes:

### What Changed

**Single file modified:** `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx`

**Changes:**
1. Added `X` import from lucide-react (line 3)
2. Made the warning alert dismissible by:
   - Wrapping the alert content in a flex container (line 178)
   - Adding an X button in the top-right corner (lines 211-218)
   - Clicking X calls `setShowHelp(false)` to dismiss the alert

### What This Solves

✅ **Original issue**: Users can dismiss false-positive warnings when preview is working
✅ **Backend users**: Can dismiss the alert and fullscreen the logs tab
✅ **Web companion discovery**: Warning still shows (includes companion installation info) but is dismissible
✅ **Simplicity**: Minimal code change, no complex logic, easy to understand

### Code Structure

The warning now has this structure:
```tsx
[Warning Title]                                    [X]
[Troubleshooting items 1-3]
[Stop Dev Server & Resolve Issues button]
```

Users can either:
- Click X to dismiss and continue working
- Click "Stop Dev Server & Resolve Issues" to stop and fix problems

**Note:** I kept the companion alert translations in `es/tasks.json` and `ja/tasks.json` in case you want to use them in the future, but they're not referenced in the code anymore.

* Done. All unused translations removed.

## Final Summary - Simple Solution

**Files changed:**
- `frontend/src/components/tasks/TaskDetails/PreviewTab.tsx`

**What changed:**
1. Added `X` import from lucide-react
2. Made the warning alert dismissible with an X button in the top-right corner

**What this solves:**
- Users can dismiss false-positive warnings
- Backend users can dismiss and focus on logs
- Web companion is still discoverable (warning shows but is dismissible)
- Minimal, clean code change

No unused code left behind.
 2025-10-15 13:03:36 +01:00
Gabriel Gordon-Hall
77309ff601 tone down log level of recurrent errors (#1016) 2025-10-15 12:05:28 +01:00
Britannio Jarrett
7136085d52 enforce switch statement exhaustiveness in linter (#941) 
remove padding from windsurf logos
 2025-10-15 10:45:10 +01:00
Solomon
720a572c1f Simplify agent prompt (#1009) 
Remove the explicit Title: and Description: formatting from the prompts, as they can reduce agent performance in simple tasks, especially since there is no additional context/prompting.
 2025-10-14 16:34:36 +01:00
Alex Netsch
41eeba5918 Revert "Perfect! All tasks have been completed successfully. Here's a summary…" (#1010) 
This reverts commit 76b0142085.
 2025-10-14 16:27:42 +01:00
Alex Netsch
76b0142085 Perfect! All tasks have been completed successfully. Here's a summary of the changes: (#970) 
Successfully implemented a better delete task workflow that properly handles task family relationships:

1. **Database Migration** (`crates/db/migrations/20251007000000_set_null_parent_task_attempt_on_delete.sql`):
   - Updated the `parent_task_attempt` foreign key constraint from the default behavior to `ON DELETE SET NULL`
   - This ensures that when a task attempt is deleted, child tasks are decoupled rather than deleted

2. **Backend - Database Layer** (`crates/db/src/models/task.rs`):
   - Added `find_children_by_parent_task_id()` method to find all child tasks created by any attempts of a given parent task

3. **Backend - API Layer** (`crates/server/src/routes/tasks.rs`):
   - Updated `delete_task()` endpoint to check for child tasks before deletion and log them
   - Added `get_task_children()` endpoint to fetch child tasks for a given task
   - Updated analytics to track the number of child tasks affected

4. **Frontend - API Client** (`frontend/src/lib/api.ts`):
   - Added `getChildren()` method to fetch child tasks for a given task

5. **Frontend - UI** (`frontend/src/components/dialogs/tasks/DeleteTaskConfirmationDialog.tsx`):
   - Added loading and display of child tasks before deletion
   - Shows a warning with the list of child tasks that will be decoupled
   - Displays up to 3 child task names with a count of remaining ones

6. **MCP Server** (`crates/server/src/mcp/task_server.rs`):
   - Updated `delete_task` tool description to clarify that child tasks are decoupled, not deleted
   - Updated success message to inform about decoupled children

- When a task is deleted, all its attempts are deleted (CASCADE)
- Child tasks that were created by those attempts have their `parent_task_attempt` set to NULL (decoupled)
- The UI warns users about child tasks that will be affected
- Child tasks remain in the project and can continue to be worked on independently

All code compiles successfully and sqlx queries have been prepared.
 2025-10-14 13:03:35 +01:00
GitHub Action
053b44a6eb chore: bump version to 0.0.107 2025-10-13 09:43:12 +00:00
Y. Fred Kim
83d6195466 완벽합니다! 한국어 로케일 추가 작업이 완료되었습니다. 변경된 내용을 요약해드리겠습니다: (#994) 
## 완료된 작업 ✓

### 1. **Rust 백엔드 업데이트**
- `crates/services/src/services/config/versions/v6.rs:21`에 `Ko` (Korean) variant 추가

### 2. **한국어 번역 파일 생성**
`frontend/src/i18n/locales/ko/` 디렉토리에 4개의 JSON 파일 생성:
- `common.json` - 공통 UI 요소 (버튼, 상태, 언어 등)
- `projects.json` - 프로젝트 관련 텍스트
- `settings.json` - 설정 페이지 전체 번역
- `tasks.json` - 작업 관리 관련 텍스트

### 3. **프론트엔드 i18n 설정 업데이트**
- `frontend/src/i18n/languages.ts:13` - `UI_TO_I18N`에 `KO: 'ko'` 추가
- `frontend/src/i18n/languages.ts:16` - `SUPPORTED_UI_LANGUAGES`에 `'KO'` 추가
- `frontend/src/i18n/languages.ts:23` - `FALLBACK_ENDONYMS`에 `ko: '한국어'` 추가
- `frontend/src/i18n/config.ts:19-22` - 한국어 번역 파일 import 추가
- `frontend/src/i18n/config.ts:43-48` - resources 객체에 한국어 리소스 추가

### 4. **TypeScript 타입 업데이트**
- `shared/types.ts:109` - `UiLanguage` 타입에 `"KO"` 추가

### 검증 완료
- ✅ 모든 JSON 파일이 유효한 형식
- ✅ Git에 변경사항 확인됨
- ✅ 한국어 로케일 디렉토리 및 파일 생성 완료

이제 사용자는 설정에서 언어를 "한국어"�� 선택할 수 있으며, UI가 한국어로 표시됩니다!
 2025-10-13 10:35:01 +01:00
Solomon
e95f6d1db1 Enable streaming message chunks in Claude-Code JSON output (#990) 
* Enable streaming message chunks in claude-code JSON output

* pass test

refactor `normalize_entries` to return patches since we emit json-patch replace now.
 2025-10-13 10:22:53 +01:00
Solomon
735bbcfeff Try zsh, bash then sh (#1000) 2025-10-13 09:51:34 +01:00
Solomon
63a935a63f Filter out access events from diff-stream filesystem watcher (#998) 
Fixes the self-perpetuating cycle of access events on Linux, where an access event triggers diff in turn inspects the file, which triggers a new access event with the same file.
 2025-10-13 09:22:27 +01:00
Alex Netsch
5e6ce06759 Better matching for parallel tool call approvals (vibe-kanban) (#977) 
* I've successfully implemented better matching for parallel tool call approvals. Here's what was changed:

**File: `crates/services/src/services/approvals.rs`**

1. **Replaced `get_last_tool_use()` with `find_matching_tool_use()`** (lines 274-326)
   - Now accepts `tool_name` and `tool_input` parameters
   - Searches backward through message history for a matching ToolUse entry
   - Only matches tools in `Created` state (not already pending approval)
   - Matches by tool name first, then by tool input for disambiguation
   - Adds debug logging to track which entry was matched

2. **Added `extract_tool_arguments()` helper** (lines 346-352)
   - Extracts tool arguments from ActionType for comparison
   - Converts ActionType to JSON for structured comparison with tool_input

3. **Updated `Approvals::create()` method** (lines 72-129)
   - Changed from `get_last_tool_use()` to `find_matching_tool_use(&request.tool_name, &request.tool_input)`
   - Added detailed debug logging when approval is created
   - Added warning when no matching tool is found

Previously, the system would just grab the **last** tool use entry, which failed for parallel calls. Now:

1. When an approval request comes in with `tool_name` and `tool_input`
2. The system searches backward through conversation history
3. It finds the most recent `ToolUse` entry that:
   - Has status = `Created` (hasn't been matched yet)
   - Has matching tool name
   - Has matching tool input (serialized to JSON for comparison)
4. That specific entry gets marked as `PendingApproval`

This ensures each approval correctly matches its corresponding tool use, even when multiple tools are called in parallel.

- ✅ Code compiles without errors or warnings
- ✅ All existing tests pass (3/3 in services crate)
- ✅ Full workspace check passes

Cleanup script changes for task attempt 182e4c44-adc2-40c6-a4a7-4c9b8528f974

Excellent! I've added comprehensive tests for the new feature. Here's what I added:

I added 3 tests to `crates/services/src/services/approvals.rs`:

Tests the core functionality - matching the correct tool when multiple parallel calls exist:
- Creates two `Read` tools with different files (`foo.rs` and `bar.rs`)
- Verifies that each approval request correctly matches its specific tool
- Ensures `foo.rs` approval matches index 0 and `bar.rs` approval matches index 1

Tests that already-matched tools are skipped:
- Creates one tool in `PendingApproval` state and another in `Created` state
- Verifies that the matching function skips the pending approval
- Ensures it finds the tool that's still in `Created` state

Tests the negative case:
- Creates a `Read` tool
- Tries to match a `Write` tool
- Verifies that no match is found (returns `None`)

All 6 tests pass (3 new + 3 existing):
- ✅ `test_find_matching_tool_use_with_parallel_calls`
- ✅ `test_find_matching_tool_use_skips_pending_approvals`
- ✅ `test_find_matching_tool_use_no_match`
- ✅ Existing analytics and config tests

The tests comprehensively validate that the new matching logic correctly handles parallel tool calls, state filtering, and edge cases.

Cleanup script changes for task attempt 182e4c44-adc2-40c6-a4a7-4c9b8528f974

Perfect! Now let me write a summary and explain the fix:

I've successfully fixed the approval matching issue for parallel tool calls. The problem was that we were comparing the wrong data structures.

When approval requests come from the hook:
- `tool_input` contains the **raw tool parameters** from Claude Code API: `{"file_path": "...", "content": "..."}`

But we were comparing against:
- `ActionType` which is our **transformed internal representation**: `{"action": "file_edit", "path": "...", "changes": [...]}`

These structures are completely different, so matching always failed for parallel calls.

The fix was to compare `tool_input` against the **original tool parameters** stored in `entry.metadata.input`, which preserves the exact structure that the hook sends.

**File: `crates/services/src/services/approvals.rs`**

1. **Updated `find_matching_tool_use()` function** (lines 289-343):
   - Extract `entry_input` from `entry.metadata.get("input")`
   - Compare it directly against `tool_input` from the approval request
   - If metadata exists but input doesn't match, `continue` searching (don't fallback to name-only matching)
   - Only fallback to name-only matching if metadata is completely missing

2. **Updated test helper** (lines 354-382):
   - Added realistic metadata structure with `"input"` field
   - Matches the actual structure from Claude Code's tool_use events

3. **Updated test inputs** (lines 407-412, 465-467, 493-496):
   - Changed from transformed format to match actual hook format
   - Example: `{"file_path": "foo.rs"}` instead of `{"action": "file_read", "path": "foo.rs"}`

1. Agent makes parallel tool calls (e.g., 3 Write tools with different files)
2. Each tool creates a `NormalizedEntry` with metadata containing the original `input` field
3. Hook fires for each tool and sends approval request with `tool_name` and `tool_input`
4. `find_matching_tool_use()` searches backward through history:
   - Filters by tool name ("Write")
   - Filters by status (`Created` only, skips already-pending tools)
   - Compares `metadata.input` against `tool_input` for exact match
5. Each approval correctly matches its specific tool use entry

✅ All 6 tests pass (3 new + 3 existing)
✅ Workspace compiles successfully
✅ Ready for testing with real parallel tool calls

Excellent! Everything compiles and tests pass. Let me provide a summary of the review feedback changes:

**Before:** If metadata was missing or input field wasn't found, we'd fall back to matching by tool name only.

**After:** We now **only** match if both tool name AND input match exactly. If either doesn't match or metadata is missing, we continue searching and return `None` if no match is found.

This ensures we never show approval UI for the wrong tool when parallel calls are made.

**Before:** 3 separate tests:
- `test_find_matching_tool_use_with_parallel_calls`
- `test_find_matching_tool_use_skips_pending_approvals`
- `test_find_matching_tool_use_no_match`

**After:** 1 comprehensive test: `test_parallel_tool_call_approval_matching`

The consolidated test covers 4 scenarios in a single, easy-to-understand flow:
1. **Parallel matching**: 3 parallel Read tools correctly match by input
2. **Status filtering**: Tools in `PendingApproval` state are skipped
3. **Tool name mismatch**: Wrong tool name returns `None`
4. **Input mismatch**: Wrong input parameters return `None`

- ✅ All 4 tests pass (1 new consolidated test + 3 existing)
- ✅ Workspace compiles successfully
- ✅ Ready for production testing

The changes make the matching logic stricter (no fallback) and the tests more maintainable (single comprehensive test vs 3 scattered ones).

Cleanup script changes for task attempt 182e4c44-adc2-40c6-a4a7-4c9b8528f974

* Perfect! Everything works. Let me create a comprehensive summary of the final implementation:

## Summary: Deserialization-Based Tool Matching

I've successfully implemented proper deserialization-based comparison for approval matching. This is much more robust than raw JSON comparison.

### Key Changes

**File: `crates/services/src/services/approvals.rs` (lines 289-357)**

### How It Works

1. **Parse incoming approval request** (lines 302-315):
   ```rust
   let approval_tool_data: ClaudeToolData = serde_json::from_value(serde_json::json!({
       "name": tool_name,
       "input": tool_input
   }))
   ```
   - Deserializes the raw `tool_input` from the hook into `ClaudeToolData`
   - Handles all serde aliases (e.g., `"path"` vs `"file_path"`, `"cmd"` vs `"command"`)
   - Returns `None` with warning if deserialization fails

2. **Deserialize stored tool data** (line 341):
   ```rust
   if let Ok(entry_tool_data) = serde_json::from_value::<ClaudeToolData>(metadata.clone())
   ```
   - Deserializes the stored metadata into the same `ClaudeToolData` type

3. **Compare deserialized structures** (line 343):
   ```rust
   if entry_tool_data == approval_tool_data
   ```
   - Uses `ClaudeToolData`'s derived `PartialEq` implementation
   - Compares the actual structured data, not raw JSON strings
   - Handles all field variations automatically through serde

### Benefits

✅ **Handles field aliases**: `file_path` vs `path`, `command` vs `cmd`, etc.
✅ **Type-safe comparison**: Uses Rust's type system instead of JSON string comparison
✅ **Future-proof**: Any new aliases or fields are automatically handled
✅ **Robust**: Deserialization validates the data structure
✅ **Clear errors**: Logs warnings when deserialization fails

### Test Results

- ✅ All 4 tests pass
- ✅ Workspace compiles successfully
- ✅ Ready for production testing with real parallel tool calls

### Example

For parallel Write tool calls:
```json
// Hook sends:
{"tool_name": "Write", "tool_input": {"file_path": "foo.rs", "content": "..."}}

// We deserialize both the incoming request and stored metadata to:
ClaudeToolData::Write { file_path: "foo.rs", content: "..." }

// Then compare the deserialized structures directly
```

This approach is much more maintainable and handles all the edge cases automatically!

* Cleanup script changes for task attempt 182e4c44-adc2-40c6-a4a7-4c9b8528f974

* Perfect! Everything compiles and tests pass. Let me create a final summary:

## Final Implementation Summary

I've successfully implemented hybrid tool matching that handles both known and unknown tools correctly for parallel tool call approvals.

### How It Works

**File: `crates/services/src/services/approvals.rs` (lines 289-375)**

### Two-Strategy Approach

#### 1. Deserialize Approval Request (lines 302-318)
```rust
let approval_tool_data: ClaudeToolData = serde_json::from_value(...)
let is_unknown_tool = matches!(approval_tool_data, ClaudeToolData::Unknown { .. });
```
- Parse incoming `tool_input` into `ClaudeToolData`
- Check if it deserialized to the `Unknown` variant
- This tells us whether to use structured or raw comparison

#### 2. Match Based on Tool Type

**For Known Tools** (lines 355-366):
```rust
if let Ok(entry_tool_data) = serde_json::from_value::<ClaudeToolData>(metadata.clone()) {
    if entry_tool_data == approval_tool_data {
        return Some((idx, entry));
    }
}
```
- Deserialize stored metadata
- Compare using `ClaudeToolData`'s `PartialEq`
- Handles all serde aliases automatically (e.g., `path` vs `file_path`)

**For Unknown Tools** (lines 343-353):
```rust
if let Some(entry_input) = metadata.get("input") {
    if entry_input == tool_input {
        return Some((idx, entry));
    }
}
```
- Use raw JSON comparison
- Works for MCP tools, future tools, and any unrecognized tools
- No aliases to worry about for unknown tools

### Benefits

✅ **Known tools** (Read, Write, Edit, etc.): Proper deserialized comparison with alias handling
✅ **Unknown tools** (MCP, future): Raw JSON comparison, no false negatives
✅ **Clean logic**: Only use raw comparison specifically for `Unknown` variant
✅ **Type-safe**: Leverages Rust's type system and pattern matching
✅ **Future-proof**: New tools automatically work

### Test Results

- ✅ All 4 tests pass
- ✅ Workspace compiles successfully
- ✅ Ready for production testing with parallel tool calls

This implementation correctly handles all tool types while maintaining clean, maintainable code!

* Perfect! All tests pass and the workspace compiles. Let me provide a final summary of the refactored implementation:

## Final Implementation Summary (After Review Feedback)

I've successfully refactored the code based on your review feedback to make it cleaner and more maintainable.

### Changes Made

**File: `crates/services/src/services/approvals.rs`**

### 1. Unknown Tool Check in Match Statement (Line 352-355)

**Before:**
```rust
let approval_tool_data = match ... { Ok(data) => data, ... };
let is_unknown_tool = matches!(approval_tool_data, ClaudeToolData::Unknown { .. });
```

**After:**
```rust
let approval_tool_data: ClaudeToolData = match serde_json::from_value(...) {
    Ok(ClaudeToolData::Unknown { .. }) => {
        return match_by_raw_input(store, tool_name, tool_input);
    }
    Ok(data) => data,
    Err(e) => { ... }
};
```

✅ **Benefit**: Early return for Unknown tools directly in the match, cleaner control flow

### 2. Extracted Raw JSON Comparison Function (Lines 289-334)

**New Function:**
```rust
fn match_by_raw_input(
    store: Arc<MsgStore>,
    tool_name: &str,
    tool_input: &serde_json::Value,
) -> Option<(usize, NormalizedEntry)>
```

✅ **Benefits**:
- Dedicated function for Unknown tool matching
- Single responsibility - only handles raw JSON comparison
- Easy to test and maintain separately
- Clear separation between known vs unknown tool strategies

### Code Structure

```
match_by_raw_input()              // Lines 289-334: For Unknown tools
    └─> Raw JSON comparison

find_matching_tool_use()          // Lines 336-406: Main entry point
    ├─> Deserialize tool_input
    ├─> If Unknown → match_by_raw_input()
    └─> If Known → Deserialized comparison
```

### How It Works

1. **Deserialize approval request** (line 348)
2. **Check if Unknown in match** (line 352)
   - If `Unknown` → Early return using `match_by_raw_input()`
   - If known → Continue with deserialized comparison
3. **Known tools**: Compare using `ClaudeToolData`'s `PartialEq` (line 393)

### Test Results

- ✅ All 4 tests pass
- ✅ Workspace compiles successfully
- ✅ Cleaner, more maintainable code structure
- ✅ Ready for production

The refactored code is now cleaner with better separation of concerns!

* Perfect! All tests pass and the workspace compiles. Here's the final summary:

## Final Refactored Implementation

I've successfully refactored the code to have **a single loop** with a **match statement** to decide the comparison strategy.

### Key Changes

**File: `crates/services/src/services/approvals.rs` (lines 289-391)**

### 1. Added Strategy Enum (lines 289-295)

```rust
enum ToolComparisonStrategy {
    Deserialized(ClaudeToolData),  // For known tools
    RawJson,                        // For Unknown tools
}
```

### 2. Single Function with One Loop (lines 297-391)

**Before**: Two separate functions, two loops
- `match_by_raw_input()` - Loop for Unknown tools
- `find_matching_tool_use()` - Loop for known tools

**After**: One function, one loop
- Determine strategy upfront (lines 309-329)
- Single loop through history (lines 332-388)
- Match statement inside loop to apply strategy (lines 354-373)

### Code Structure

```rust
fn find_matching_tool_use(...) {
    // 1. Determine strategy based on tool type
    let strategy = match deserialize_tool_data(...) {
        Unknown => RawJson,
        Known(data) => Deserialized(data),
    };

    // 2. Single loop through history
    for entry in history {
        // Filter by status and name

        // 3. Match on strategy to compare
        let is_match = match &strategy {
            RawJson => compare_raw_json(...),
            Deserialized(data) => compare_deserialized(...),
        };

        if is_match { return Some(entry); }
    }
}
```

### Benefits

✅ **Single loop**: No code duplication, cleaner structure
✅ **Strategy pattern**: Clear separation of comparison logic
✅ **Type-safe**: Enum enforces correct strategy usage
✅ **Maintainable**: Easy to add new strategies if needed
✅ **Same performance**: Strategy determined once, not per iteration

### Test Results

- ✅ All 4 tests pass
- ✅ Workspace compiles successfully
- ✅ Ready for production

The refactored code is cleaner and more maintainable while preserving all functionality!

* Clippy
 2025-10-10 16:57:52 +01:00
Britannio Jarrett
8c6f7c6d30 update docs (preview mode, mcp start task tool, GitHub Copilot) (vibe-kanban) (#992) 
* update cleanup script docs

* Perfect! I've successfully created comprehensive documentation for the Preview Mode feature and updated the existing documentation. Here's what I've completed:

## New Documentation Created

**[Preview Mode](/core-features/preview-mode)** - A complete guide covering:
- Overview and key benefits
- Prerequisites and setup requirements
- Step-by-step setup instructions for the Web Companion
- Usage instructions with toolbar controls
- Component selection workflow
- Comprehensive troubleshooting section
- Advanced usage examples

## Updated Documentation

**[Task Details Full Screen Mode](/core-features/task-details-full-screen)** - Enhanced with:
- Added Preview tab to the right panel navigation
- New Preview Tab section explaining embedded browser functionality
- Cross-reference link to the detailed Preview Mode documentation
- Updated related documentation links

**[Navigation](/docs.json)** - Added the new preview-mode page to the Core Features section

## Key Features Documented

The documentation covers all the preview mode capabilities you described:
- **Initial setup** requirements and configuration
- **Automatic URL detection** from development server logs
- **Embedded browser** viewing within full screen mode
- **Web Companion installation** for component selection
- **Precise component selection** with Alt+click and context menus
- **Seamless workflows** without leaving Vibe Kanban

The documentation includes placeholder spaces for screenshots as requested, and follows the existing Mintlify documentation patterns with proper formatting, components, and cross-references.

* I've successfully updated the [Vibe Kanban MCP Server documentation](/integrations/vibe-kanban-mcp-server) with comprehensive tool coverage:

## New Documentation Sections Added

**Available MCP Tools** - Complete reference tables organized by category:
- **Project Operations**: `list_projects`
- **Task Management**: `list_tasks`, `create_task`, `get_task`, `update_task`, `delete_task`
- **Task Execution**: `start_task_attempt` (new feature)

**Supported Executors** - Full list of coding agents that can be used with `start_task_attempt`

**Status Values** - Valid kebab-case status values for filtering and updates

## Enhanced Example Usage

**Planning and Task Creation** - Shows how coding agents use the `create_task` tool

**Starting Task Execution** - Demonstrates the new `start_task_attempt` feature with example JSON parameters

**Complete Workflow Example** - End-to-end workflow from listing projects to starting task execution

The documentation now provides a complete reference for all MCP tools, with the new `start_task_attempt` feature prominently highlighted and explained with practical examples.

* Done! I've updated [supported-coding-agents.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/supported-coding-agents.mdx):

1. **Reordered**: Claude Code first, OpenAI Codex second, then Amp
2. **Renamed**: "Codex" → "OpenAI Codex"
3. **Renamed**: "SST OpenCode" → "OpenCode"

* Done! Updated [vibe-kanban-mcp-server.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/integrations/vibe-kanban-mcp-server.mdx):

1. Added syntax note to Option 2: Manual Configuration
2. Removed type hints from all parameters in MCP tools table
3. Removed the Info callout about `start_task_attempt` being newly added

* Done! Removed the Status Values section from [vibe-kanban-mcp-server.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/integrations/vibe-kanban-mcp-server.mdx).

* Done! I've updated the terminology in [vibe-kanban-mcp-server.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/integrations/vibe-kanban-mcp-server.mdx):

- Changed section heading from "Using MCP with Coding Agents" to "Using the MCP Server"
- Replaced "your coding agent" with "your MCP client" in the workflow steps and examples
- Kept references to "coding agents" only where appropriate (e.g., when referring to agents within Vibe Kanban, or the executor parameter for `start_task_attempt`)

* fix docs (vibe-kanban a870f5a1)

The server is live and the docs are accessible via the URL http://localhost:3000/core-features/preview-mode

I'm seeing `🚧 A parsing error occured. Please contact the owner of this website.` on this preview mode page and it seemingly has something to do with the Steps component.

Using the Chrome DevTools that you have access to, you should be able to launch the website and iteratively make changes until this error is no longer present and the preview mode documentation is displaying itself as intended. You can look at recent commits to understand what has been changed with this documentation. You also have access to the Mintlify tools in order to search the official Mintlify documentation in case you need to troubleshoot.

* Done! I've updated the [preview-mode.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/core-features/preview-mode.mdx) with:

1. **Emphasized automatic installation** as the recommended approach with an Info callout
2. **Accurate setup steps** from the official README for Create React App, Next.js, and Vite
3. **Removed conditional rendering** (`process.env.NODE_ENV` checks) since tree-shaking handles this automatically
4. **Added note about tree-shaking** to clarify that the companion is automatically removed from production builds

* Done! I've completed both tasks:

1. ✅ Copied the dev server screenshot to [docs/images/preview-mode-start-dev-server.png](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/images/preview-mode-start-dev-server.png)
2. ✅ Removed the "Advanced Usage" section from [preview-mode.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/core-features/preview-mode.mdx)

* Done! I've completed all the image updates:

1. ✅ Replaced the dev server start image with the new screenshot
2. ✅ Added all preview mode screenshots:
   - `preview-mode-interface.png` - Preview tab with dev server on
   - `preview-mode-component-selection.png` - Hovering over component with companion
   - `preview-mode-component-depth.png` - Component selected showing hierarchy
3. ✅ Removed all placeholder comments from the Frame elements

* Done! I've completed all 6 review fixes:

1. ✅ Copied dev script config screenshot
2. ✅ Added install companion button screenshot within the Info callout
3. ✅ Removed the false "Automatically switch to Preview tab when ready" line
4. ✅ Added preview toolbar screenshot
5. ✅ Removed "Stop Dev" from toolbar controls (it's not part of the toolbar)
6. ✅ Fixed component selection instructions - now correctly states to click the floating VK companion button in bottom-right corner, removed incorrect Alt+Click and Context Menu instructions

* Done! I've simplified the troubleshooting section to only include the relevant tip about ensuring the development server prints the URL to stdout/stderr for automatic detection, along with the supported URL formats. Removed all the less relevant troubleshooting content.

* Done! I've completed all 3 review fixes:

1. ✅ Removed emojis from toolbar controls (🔄, 📋, 🌐)
2. ✅ Replaced component depth image with new screenshot and moved the old one to the top as an overview image
3. ✅ Removed "IDE" from the overview text - now only mentions switching between browser and Vibe Kanban

* Done! Replaced the overview image with the new screenshot.

* Done! I've fixed both issues in [task-details-full-screen.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/core-features/task-details-full-screen.mdx):

1. ✅ Updated toolbar description from "Start, stop, and manage your development server" to "Refresh the preview, copy the URL, or open in a new browser tab"
2. ✅ Changed image from `vk-preview.png` to `preview-mode-overview.png` (same as the preview mode docs page)

* Done! I've added GitHub Copilot to the [supported coding agents documentation](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/supported-coding-agents.mdx). The new section includes:

- Installation instructions via npm (`npm install -g @github/copilot`)
- Prerequisites (Copilot subscription, Node.js 22+, npm 10+)
- Authentication instructions using the `/login` command
- Link to official GitHub Copilot CLI documentation

It's positioned between Amp and Cursor CLI in the list.

* Done! Removed the placeholder comment from [preview-mode.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/core-features/preview-mode.mdx).

* remove old preview

* Done! I've updated [vibe-kanban-mcp-server.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/integrations/vibe-kanban-mcp-server.mdx) with both corrections:

1. ✅ Changed "Create a task in 'plan mode'" to mention creating a custom agent profile with a planning prompt, with a link to the Agent Configurations documentation
2. ✅ Changed "Generate Sub-Tasks" to "Generate Tasks" since the MCP server only generates tasks, not sub-tasks

* Done! Removed the "Real-time preview" line from the key benefits section in [preview-mode.mdx](file:///private/var/folders/5q/5vgq75y92dz0k7n62z93299r0000gn/T/vibe-kanban/worktrees/4d6d-update-docs/docs/core-features/preview-mode.mdx).

* update docs on Codex w/ ChatGPT

* update MCP server install command docs
 2025-10-10 16:35:20 +01:00
Solomon
3d2f062c2f fix: retry modal horizontal overflow (#991) 
Break content onto the next line horizontal space runs out.
 2025-10-10 14:46:34 +01:00
Solomon
4aa9506d8c Update process state on spawn failure (#968) 2025-10-10 14:26:42 +01:00
Solomon
4993751a98 Prevent diff processing from starving other tokio tasks (#986) 2025-10-10 12:20:23 +01:00
Louis Knight-Webb
1e1409c9b0 i18n check detaches local git (vibe-kanban) (#967) 
* Done! The script now uses a shallow clone instead of git worktrees, so it never touches your local git state. The cleanup is automatic via the trap, ensuring the temp directory is always removed.

* Done! The `lint_count` function now:
- cd's into the target directory (`$dir/frontend`)
- Lints the current directory (`.`)
- Uses `--prefix` to ensure ESLint uses the PR workspace's dependencies

This is clearer and fixes the empty PR_COUNT issue.
 2025-10-10 10:32:44 +01:00
Louis Knight-Webb
f78decada9 Adjust useConversationHistory (vibe-kanban) (#989) 
* Done! The changes ensure user messages display immediately when a coding agent execution process is created.

**What changed:**
1. `getActiveAgentProcess()` - now detects created/queued/running processes (not just running)
2. `ensureProcessVisible()` - adds process to state with empty entries and emits immediately
3. `idStatusKey` - tracks status changes so effect runs when process transitions states
4. Active process effect - calls `ensureProcessVisible()` before starting stream

**Result:** User sees their message instantly, then logs populate as they stream in.

* Done! All consistency issues fixed:

**Changes made:**
1. ✅ Moved `emitEntries()` out of `ensureProcessVisible()` to caller - consistent with pattern where helpers mutate but don't emit
2. ✅ Updated both historic loaders to skip `['created', 'queued', 'running']` - prevents race condition with active process path
3. ✅ Renamed `lastRunningProcessId` → `lastActiveProcessId` - accurate naming for what it tracks

The implementation is now internally consistent with the existing codebase patterns.

* Done! The fix ensures streaming only starts when the process is actually `running`, not when it's just `created` or `queued`.

**How it works now:**
1. Process detected (created/queued/running) → user message shows immediately
2. Process transitions to `running` → streaming starts once and logs appear
3. No wasted backoff attempts before endpoint is ready

* ## Analysis Complete

I found the fork's enum attribute: **`#[ts(use_ts_enum)]`**

**Current state:**
- Rust: `ExecutionProcessStatus` enum with values `Running`, `Completed`, `Failed`, `Killed`
- TypeScript: `type ExecutionProcessStatus = "running" | "completed" | "failed" | "killed"`

**Important finding:** The Oracle identified a bug in our recent changes - we're checking for `['created', 'queued', 'running']` but `ExecutionProcessStatus` doesn't have `created` or `queued` values. Those don't exist in the enum.

## Proposed Changes

1. **Add `#[ts(use_ts_enum)]` to Rust enum** in [crates/db/src/models/execution_process.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/91b8-adjust-useconver/crates/db/src/models/execution_process.rs#L31-L39)
2. **Regenerate TypeScript types** with `npm run generate-types`
3. **Fix the bug in useConversationHistory.ts** - remove `'created', 'queued'` from the check (they don't exist)
4. **Update frontend to use enum** instead of string literals

**Proceed with this plan?**

* Cleanup script changes for task attempt 91b81dd2-0f76-48f7-a309-683373d87d0d

* Done! All changes complete:

**Changes made:**
1. ✅ Added `#[ts(use_ts_enum)]` to Rust enum
2. ✅ Regenerated TypeScript types - now `export enum ExecutionProcessStatus`
3. ✅ Fixed bug: removed non-existent `'created', 'queued'` from status checks
4. ✅ Updated all frontend code to use `ExecutionProcessStatus.running` instead of string literals

**Result:** TypeScript now has a proper enum with type safety and autocomplete, and the bug with checking for non-existent statuses is fixed.

* remove unnecessary path
 2025-10-10 09:57:16 +01:00
GitHub Action
fa26307795 chore: bump version to 0.0.106 2025-10-09 11:59:52 +00:00
Solomon
6a81ba77f4 Bound the diff-stream channel (#983) 
The bounded channel applies backpressure, so the filewatcher writer pauses when the channel is full.
 2025-10-09 12:39:03 +01:00
Solomon
3493503602 Reduce npx platform-specific unzip issues (#973) 2025-10-09 12:37:22 +01:00
Alex Netsch
0253880f0a Move tasks with pending approval to In Reviewstate (vibe-kanban) (#976) 
* Done! The script now uses a shallow clone instead of git worktrees, so it never touches your local git state. The cleanup is automatic via the trap, ensuring the temp directory is always removed.

Done! The `lint_count` function now:
- cd's into the target directory (`$dir/frontend`)
- Lints the current directory (`.`)
- Uses `--prefix` to ensure ESLint uses the PR workspace's dependencies

This is clearer and fixes the empty PR_COUNT issue.

Perfect! Let me create a summary of the changes:

I've successfully implemented the feature to move tasks with pending approvals to the "In Review" state. Here's what was changed:

1. **Modified `crates/services/src/services/approvals.rs`:**
   - Added imports for `ExecutionProcess`, `Task`, `TaskStatus`, and `TaskAttempt`
   - Updated `create_from_session()` method to change task status from `InProgress` to `InReview` when an approval is requested
   - Only updates status if the task is currently in `InProgress`

2. **Modified `crates/server/src/routes/approvals.rs`:**
   - Added imports for `ExecutionProcess`, `Task`, `TaskStatus`, and `TaskAttempt`
   - Updated `respond_to_approval()` handler to change task status back to `InProgress` when approval is responded to (approved/denied)
   - Special handling for the `exit_plan_mode` case: when exiting plan mode, a new execution process starts which automatically sets status to `InProgress` via `start_execution()`
   - Only updates status if the task is currently in `InReview`

3. **Generated TypeScript types:**
   - Ran `cargo run --bin generate_types` to ensure frontend types are in sync

**Status Flow:**
- `InProgress` → `InReview`: When an approval request is created
- `InReview` → `InProgress`: When approval is approved/denied (process continues)
- `InReview` → `InProgress`: Special case for exit_plan_mode - new process starts automatically

**Edge Cases Handled:**
- Only updates status if current status is appropriate (prevents conflicts with merged tasks)
- Graceful error handling with warnings if status update fails
- Special handling for `exit_plan_mode` tool approvals
- No status change on timeout (task remains in `InReview` until process completes)

All changes compile successfully and pass clippy linting checks!

Cleanup script changes for task attempt f091024c-6c72-493e-9fe3-db2bd523cc7b

Perfect! Let me create a summary of the changes:

I've successfully simplified the approval response logic to update task status for **ALL** approvals uniformly, including exit_plan_mode. Here's what changed:

**Modified `crates/server/src/routes/approvals.rs`:**

**Before:** Status update was split into two branches:
- exit_plan_mode: Status updated via `start_execution()` when new process starts
- Other approvals: Status updated explicitly in the response handler

**After:** Simplified to single flow:
1. **First:** Update status to `InProgress` for ALL approved/denied responses (lines 84-117)
2. **Then:** If exit_plan_mode, start new process (lines 119-133)

The `start_execution()` method has a guard (container.rs:582):
```rust
if task.status != TaskStatus::InProgress
    && run_reason != &ExecutionProcessRunReason::DevServer
{
    Task::update_status(&self.db().pool, task.id, TaskStatus::InProgress).await?;
}
```

**Flow for exit_plan_mode:**
1. User approves plan → `respond_to_approval()` sets status to `InProgress` ✓
2. `exit_plan_mode_tool()` calls `start_execution()` with the follow-up action
3. `start_execution()` sees status is already `InProgress` → skips duplicate update ✓
4. Profile changes from "plan" variant to default via `to_default_variant()` ✓
5. New process starts with correct permissions ✓

✅ **Simpler code:** Removed conditional branching
✅ **Consistent behavior:** All approvals handled uniformly
✅ **Immediate status update:** Status changes on response, not waiting for process start
✅ **No duplicate updates:** Guard prevents redundant database writes
✅ **Profile change preserved:** exit_plan_mode still changes profile correctly

All changes compile and pass clippy checks with `-D warnings`!

Cleanup script changes for task attempt f091024c-6c72-493e-9fe3-db2bd523cc7b

Perfect! All review comments have been addressed. Here's a summary of the changes:

   - Removed redundant comments about status updates
   - Code is now self-explanatory

   **services/approvals.rs (Line 133):**
   - **Before:** Three separate database calls to get execution_process → task_attempt → task
   - **After:** Single call to `ExecutionProcess::load_context()` which loads all context
   - Removed unnecessary imports: `TaskAttempt`

   **server/approvals.rs (Line 89):**
   - **Before:** Three separate database calls
   - **After:** Single call to `ExecutionProcess::load_context()`
   - Removed unnecessary imports: `TaskAttempt`

✅ **Cleaner code:** Removed redundant comments
✅ **Better performance:** Reduced database calls from 3 to 1 per status update
✅ **More maintainable:** Using existing utility function instead of duplicating logic
✅ **Fewer imports:** Removed unused `TaskAttempt` import from both files

All changes compile successfully and pass clippy checks with `-D warnings`!

Perfect! All checks pass. The review comment has been addressed. Here's a summary:

**Issue:** The `tracing::info!` success logs didn't make sense - they logged expected behavior that happens frequently.

**Changes Made:**

1. **Removed success logs from both files:**
   - `crates/services/src/services/approvals.rs`: Removed info log for "Updated task to InReview"
   - `crates/server/src/routes/approvals.rs`: Removed info log for "Updated task to InProgress"

2. **Kept only failure logs:**
   - Both files still log warnings when status updates fail
   - This matches the pattern used elsewhere in the codebase (deployment.rs, container.rs)

3. **Collapsed if statements:**
   - Used let-chains to collapse nested if statements
   - Now only logs when there's an actual error to report

**Result:**
- Cleaner, less noisy logs
- Only logs when something goes wrong (warnings)
- Consistent with existing codebase patterns
- All clippy checks pass with `-D warnings`

* move logic into service layer

---------

Co-authored-by: Gabriel Gordon-Hall <ggordonhall@gmail.com>
 2025-10-09 12:18:50 +01:00
Solomon
21876fddc1 Bump codex version to 0.46 (#981) 2025-10-09 12:18:39 +01:00
Gabriel Gordon-Hall
a584eda876 bump agent versions (#984) 2025-10-09 12:10:26 +01:00
Gabriel Gordon-Hall
e52ced99e0 fix sentry logging (#982) 2025-10-09 11:45:23 +01:00
Solomon
f2cc538a8f Create commit message correctly (#980) 
* Create commit message correctly

Avoid trucating message string at invalid character boundary.

* refactor and add test
 2025-10-09 10:58:46 +01:00
Alex Netsch
37d273a851 Live Diff Service Refactor (vibe-kanban) (#962) 
* Fix slow diffs regression

* Refactor diff streaming

Perfect! The refactoring is complete and successful. Let me create a summary of what was accomplished:

Successfully completed the Live Diff Service Refactor. The live diff/WebSocket pipeline has been moved from `local-deployment` into a shared `services` module, mirroring the structure of `FileSearchCache`.

1. **Created new service module**: `crates/services/src/services/diff_stream/mod.rs`
   - Moved all code from `crates/local-deployment/src/live_diff.rs`
   - Made the public API small: `create()`, `apply_stream_omit_policy()`, and `MAX_CUMULATIVE_DIFF_BYTES` constant
   - Updated error types to use `Box<dyn std::error::Error + Send + Sync>` for broader compatibility

2. **Updated module exports**: Added `diff_stream` to `crates/services/src/services/mod.rs`

3. **Refactored `LocalContainerService`**:
   - Removed `MAX_CUMULATIVE_DIFF_BYTES` constant (now exposed via `diff_stream` module)
   - Removed unused `AtomicUsize` import
   - Updated imports to include `diff_stream` from services
   - Changed `live_diff::create()` calls to `diff_stream::create()`
   - Changed `live_diff::apply_stream_omit_policy()` calls to `diff_stream::apply_stream_omit_policy()`
   - Added error conversion for the new error type
   - Added `FutureExt` import to fix compilation issues

4. **Cleaned up `local-deployment`**:
   - Removed `mod live_diff;` declaration from `lib.rs`
   - Deleted `crates/local-deployment/src/live_diff.rs`

5. **Verification**:
   - ✅ `cargo check --workspace` passes
   - ✅ `cargo fmt --all` completed successfully

The refactoring maintains all functionality while improving code organization. The diff stream service is now properly isolated and can be reused by other parts of the codebase without depending on `local-deployment`.

Perfect! All review comments have been addressed. Let me create a summary:

All three review comments have been successfully addressed:

- Replaced `Box<dyn std::error::Error + Send + Sync>` with a proper error type
- Added variants for `GitService`, `FilesystemWatcher`, and `TaskJoin` errors
- Used `thiserror::Error` for clean error derivation
- Updated `create()` function signature to return `Result<DiffStreamHandle, DiffStreamError>`

- Moved the entire `DiffStreamWithWatcher` struct, `Stream` impl, and `Drop` impl from `container.rs`
- Added two constructor methods:
  - `new(handle: DiffStreamHandle)` - for live diffs with watcher
  - `from_stream(stream)` - for static diffs without watcher
- Updated `container.rs` to import it from `diff_stream` module
- Updated `container.rs` to use the new constructors instead of direct struct initialization

- Re-added `Arc` and `atomic::AtomicUsize` imports to `container.rs`
- Changed `std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0))` back to `Arc::new(AtomicUsize::new(0))`
- Code is now more idiomatic and readable

- ✅ `cargo check --workspace` passes
- ✅ `cargo fmt --all` completed successfully

The refactoring maintains all functionality while addressing all review feedback. The diff stream service is now more self-contained, properly typed, and follows Rust best practices.

Fix live diff service refactor regressions (vibe-kanban 314aa828)

In the previous 4 commits we refactored live diffs. A reviewer left these comments:
crates/services/src/services/diffstream/mod.rs:187 — On watcher start-up failure we now only enqueue an Err on the stream and return Ok(DiffStreamHandle). Previously the error bubbled out of create_live_diff_stream, so stream_diff failed immediately and the caller could surface that setup failure before handing a stream to the websocket. With the new behaviour the API still returns a stream even though live updates will never arrive; callers only discover the failure once they start polling. That’s a behavioural regression—can we keep propagating the FilesystemWatcherError/JoinError through the DiffStreamError path so `createlivediffstream` continues to fail fast?

crates/services/src/services/diffstream/mod.rs:204 — We removed the tracing::error! calls that used to fire when diff processing or the watcher emitted an error. Now we silently convert everything into an io::Error and push it onto the channel. That makes operational debugging harder (and it means we’ll miss the log message we used to get from `handletaskattemptdiff_ws). Please restore the tracing::error!` logging (or similar) before sending the error downstream.

Please address them

* Excellent! Now let me create a summary of the changes:

Successfully eliminated the duplicate `DiffStreamHandle` and `DiffStreamWithWatcher` structs by consolidating them into a single `DiffStreamHandle` type.

**1. Removed the old plain `DiffStreamHandle` struct** (crates/services/src/services/diff_stream.rs:41-44)
   - The old struct had public fields `stream` and `watcher_task`
   - It was just an intermediate type that got immediately converted

**2. Renamed `DiffStreamWithWatcher` to `DiffStreamHandle`** (crates/services/src/services/diff_stream.rs:43-79)
   - Kept all the important functionality: `Stream` impl and `Drop` impl
   - Made fields private for encapsulation
   - Updated documentation to reflect that this is THE diff stream type

**3. Simplified the API**
   - `create()` now directly returns `DiffStreamHandle` instead of an intermediate struct
   - Added a public `new()` constructor for external callers
   - Removed unnecessary wrapper conversions

**4. Updated container.rs** (crates/local-deployment/src/container.rs)
   - Changed import from `DiffStreamWithWatcher` to `DiffStreamHandle`
   - Updated all function signatures and calls
   - Simplified construction: `DiffStreamHandle::new(stream, None)` instead of wrapper conversions

- ✅ One less struct to maintain
- ✅ Simpler, more direct API
- ✅ Same functionality preserved
- ✅ `cargo check --workspace` passes
- ✅ `cargo fmt --all` completed

The diff stream module is now cleaner with just one main type that handles both static and live diff streams through a unified interface.

* Revert await watcher start

* Make filsesystem watcher gitignore fast

* Fix missing error logging

* Fix error match
 2025-10-08 11:52:53 +01:00
Alex Netsch
cd06bf01b0 File search in approval text box (vibe-kanban) (#966) 
* Perfect! The implementation is complete. Let me create a summary of what was changed:

## Summary

Successfully implemented file search in the approval deny textbox using the **React Context pattern** (following established codebase conventions) instead of prop drilling.

### Changes Made:

**1 file modified** (vs 5 files with prop drilling approach):

**`PendingApprovalEntry.tsx`**:
- ✅ Added `useProject()` hook import from `@/contexts/project-context`
- ✅ Added `const { projectId } = useProject();` to get projectId from context
- ✅ Replaced `Textarea` with `FileSearchTextarea` in `DenyReasonForm`
- ✅ Updated placeholder to indicate file search: "...type @ to search files"
- ✅ Pass `projectId` to `FileSearchTextarea`

### How It Works:

1. User clicks "Deny" button on an approval request
2. Deny reason form appears with `FileSearchTextarea`
3. User types `@` to trigger file search dropdown
4. `FileSearchTextarea` uses `projectId` to search files via API
5. User can select files from dropdown to reference them in denial reason

### Why This Approach:

✅ **Follows existing patterns**: `FollowUpEditorCard`, `DiffCard`, `Navbar` all use `useProject()` hook
✅ **No prop drilling**: Context is already available at app root level
✅ **Minimal changes**: Only 1 component modified
✅ **More maintainable**: If projectId source changes, only context needs updating
✅ **Cleaner interfaces**: No unnecessary props through intermediate components

The feature is now ready to test!

* Perfect! I've fixed the styling issue. The changes I made:

**Updated `className` from:**
```
className="text-sm"
```

**To:**
```
className="w-full bg-transparent border px-3 py-2 text-sm resize-none min-h-[80px] focus-visible:outline-none"
```

This matches the original `Textarea` component's styling exactly:
- ✅ `w-full` - full width
- ✅ `bg-transparent` - transparent background (original had this)
- ✅ `border` - adds border
- ✅ `px-3 py-2` - proper padding (3 horizontal, 2 vertical)
- ✅ `text-sm` - small text size
- ✅ `resize-none` - prevents manual resizing
- ✅ `min-h-[80px]` - minimum height of 80px (same as original)
- ✅ `focus-visible:outline-none` - removes outline on focus

Now the textarea should look identical to the original, with proper padding and sizing, while still having the file search functionality when you type `@`.

* Perfect! Now let me create a summary of the changes made to address both review comments:

## Summary of Changes

### Addressing Review Comment 1: "Do we still need inputRef?"

**Yes, we still need it!**

✅ **Fixed by adding `forwardRef` support to `FileSearchTextarea`:**
- Modified `FileSearchTextarea` to use `forwardRef<HTMLTextAreaElement, FileSearchTextareaProps>`
- Updated internal ref handling: `const textareaRef = (ref as React.RefObject<HTMLTextAreaElement>) || internalRef;`
- Now properly forwards refs to `AutoExpandingTextarea`
- Added `ref={inputRef}` to the `FileSearchTextarea` in `DenyReasonForm`

**Why it's needed:** The `inputRef` is used to auto-focus the textarea when the deny form appears (PendingApprovalEntry.tsx:308-311):
```tsx
useEffect(() => {
  if (!isEnteringReason) return;
  const id = window.setTimeout(() => denyReasonRef.current?.focus(), 0);
  return () => window.clearTimeout(id);
}, [isEnteringReason]);
```

---

### Addressing Review Comment 2: "Why do we need explicit className now?"

✅ **Added documentation comment explaining the difference:**

**Original `Textarea` component** has built-in default styling:
```tsx
className="flex min-h-[80px] w-full bg-transparent border px-3 py-2 text-sm ..."
```

**`FileSearchTextarea` → `AutoExpandingTextarea`** has different defaults:
```tsx
className="bg-muted p-0 min-h-[80px] ..."  // Note: p-0 (no padding!), bg-muted
```

**Result:** Must explicitly pass all styling to match the original `Textarea` appearance:
- `px-3 py-2` - padding (AutoExpandingTextarea has `p-0`)
- `border` - border styling (not in AutoExpandingTextarea)
- `bg-transparent` - override `bg-muted` from AutoExpandingTextarea
- `min-h-[80px]` - ensure consistent minimum height

**Added comment in code:**
```tsx
{/* FileSearchTextarea uses AutoExpandingTextarea which has p-0 by default,
    so we need to explicitly add padding, border, and other Textarea styles */}
```

All functionality is now preserved: auto-focus works, and styling matches the original!

* Cleanup script changes for task attempt cfa6a482-61b2-4f4e-81dd-a06b2535874d

* Done! Comments removed.

* Perfect! All changes complete. Here's a summary of what was done:

## Summary of Changes

### Question 1: Why specify `rows={3}` if it equals the default?
**Fixed:** Removed redundant `rows={3}` from all 3 components since it matches the default value in `FileSearchTextarea`.

### Question 2: Is forwardRef the best way to get the ref for focus?
**Answer:** Yes! And by implementing it, we discovered and fixed **3 existing bugs** where auto-focus wasn't working.

### Changes Made:

**1. `FileSearchTextarea` component:**
- ✅ Added `forwardRef` support to properly expose textarea refs
- ✅ Now properly forwards refs to the underlying `AutoExpandingTextarea`

**2. `PendingApprovalEntry`:**
- ✅ Removed redundant `rows={3}` prop
- ✅ Kept `ref={inputRef}` (now works correctly for auto-focus)

**3. `ReviewCommentRenderer` (bug fix):**
- ✅ Added `ref={textareaRef}` to `FileSearchTextarea`
- ✅ Removed redundant `rows={3}` prop
- ✅ **Fixed broken auto-focus** that wasn't working since migration from `Textarea`

**4. `CommentWidgetLine` (bug fix):**
- ✅ Added `ref={textareaRef}` to `FileSearchTextarea`
- ✅ Removed redundant `rows={3}` prop
- ✅ **Fixed broken auto-focus** that wasn't working since migration from `Textarea`

### Why forwardRef is the right approach:
- ✅ **Standard React pattern** for exposing refs from custom components
- ✅ **Type-safe** with proper TypeScript support
- ✅ **Consistent** with how `AutoExpandingTextarea` already works
- ✅ **Necessary** for imperative operations like `.focus()`, `.blur()`, `.setSelectionRange()`
- ✅ **Discovered bugs** in existing code that had broken auto-focus

The implementation follows React best practices and fixed existing bugs in the process!

* Done! Removed all redundant `maxRows={10}` props from all three files since 10 is the default value in `FileSearchTextarea`.

* Update frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx

Co-authored-by: Gabriel Gordon-Hall <gabriel@bloop.ai>

---------

Co-authored-by: Gabriel Gordon-Hall <gabriel@bloop.ai>
 2025-10-08 11:23:57 +01:00
Solomon
399297d49c Simplify codex default variants (#972) 2025-10-08 11:11:39 +01:00
Kohei Watanabe
68781b870b fix: align read_port_file path logic with write_port_file (#971) 
In read_port_file, the base directory on Linux was derived from XDG_RUNTIME_DIR, while write_port_file always used env::temp_dir(). This caused an inconsistency between the read and write paths.
 2025-10-08 10:22:55 +01:00
Britannio Jarrett
52dcbf1fd7 replace folder icon with IDE icon in the nav bar (#969) 2025-10-08 09:26:47 +01:00
GitHub Action
ad1696cd36 chore: bump version to 0.0.105 2025-10-07 16:19:12 +00:00
Louis Knight-Webb
36587766d6 Setting to override branch prefix (vibe-kanban) (#949) 
* ## ✅ Implementation Complete

Successfully added customizable git branch prefix setting:

### Changes Made

**Backend:**
1. [v7.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/services/src/services/config/versions/v7.rs) - Added `git_branch_prefix` field with default "vk"
2. [git.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/utils/src/git.rs) - Created validation function with tests
3. [container.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/services/src/services/container.rs#L229-L240) - Updated branch creation to use config value
4. [local container](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/local-deployment/src/container.rs#L897-L901) - Implemented `git_branch_prefix()` method

**Frontend:**
5. [GeneralSettings.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/pages/settings/GeneralSettings.tsx#L452-L480) - Added UI input in GitHub card with preview

### Features
- Default prefix: `vk` (maintains backward compatibility)
- Empty prefix supported (branches without prefix)
- Validation ensures git-compliant names
- Preview shows branch format: `{prefix}/{uuid}-{slug}` or `{uuid}-{slug}`
- All checks passed ✓

* Cleanup script changes for task attempt 8921b110-a98c-4968-9908-c710615a968e

* ## ✅ Fixed Async Runtime Panic

Successfully converted blocking methods to async:

### Changes Made

1. **[container.rs trait](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/services/src/services/container.rs#L229-L240)** - Made both methods async, added `.await` to prefix call
2. **[LocalContainerService](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/local-deployment/src/container.rs#L898-L900)** - Changed to `config.read().await`
3. **[tasks.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/server/src/routes/tasks.rs#L167-L170)** - Added `.await` to method call
4. **[task_attempts.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/server/src/routes/task_attempts.rs#L155-L158)** - Added `.await` to method call

All checks passed ✓. The panic should be resolved - the methods now properly use async config access.

* Cleanup script changes for task attempt 8921b110-a98c-4968-9908-c710615a968e

* ## ✅ Moved to New Git Section

Created a separate Git card in [GeneralSettings.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/pages/settings/GeneralSettings.tsx#L528-L571) positioned between GitHub and Notifications sections, with:
- Title: "Git"
- Description: "Configure git branch naming preferences"
- Branch Prefix setting with preview

All checks passed ✓

* Cleanup script changes for task attempt 8921b110-a98c-4968-9908-c710615a968e

* ## ✅ i18n Integration Complete

Added translations for the Git section in all supported languages:

### Translation Files Updated
1. **[English](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/en/settings.json)** - Added `git` section with complete translations
2. **[Spanish](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/es/settings.json)** - Spanish translations
3. **[Japanese](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/ja/settings.json)** - Japanese translations

### Component Updated
- [GeneralSettings.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/pages/settings/GeneralSettings.tsx#L528-L575) - Now uses `t()` for all text with dynamic preview translation

All checks passed ✓

* Commit changes from coding agent for task attempt 8921b110-a98c-4968-9908-c710615a968e

* Cleanup script changes for task attempt 8921b110-a98c-4968-9908-c710615a968e

* ## ✅ Validation Implementation Complete

Successfully addressed PR feedback by implementing comprehensive validation:

### 1. Backend: Using `git2::Branch::name_is_valid` ✅
- **[git.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/utils/src/git.rs)** - Replaced manual validation with `git2::Branch::name_is_valid`
- Uses authoritative libgit2 validation with `"{prefix}/x"` format
- Added git2 dependency to utils crate
- Updated tests (removed tests for behaviors that differ from git2)

### 2. Frontend: Real-time Validation with Error Display ✅
- **[GeneralSettings.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/pages/settings/GeneralSettings.tsx)** - Added validation logic
- Error state shows inline message below input
- Red border on invalid input (`border-destructive`)
- Save button disabled when validation fails
- Validates on input change (real-time feedback)

### 3. i18n Error Messages ✅
Added translations in all languages:
- [English](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/en/settings.json#L82-L90)
- [Spanish](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/es/settings.json#L79-L87)
- [Japanese](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/frontend/src/i18n/locales/ja/settings.json#L86-L94)

### 4. Backend Enforcement ✅
- **[config.rs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/8921-setting-to-overr/crates/server/src/routes/config.rs#L104-L108)** - Validates on config save
- Returns 400 error if invalid prefix
- Backend remains source of truth

All checks passed ✓

* ✅ Fixed linting error - removed unnecessary escape character in regex. All checks now pass.

* ## ✅ Simplified Validation Text

Removed verbose rules text and made error messages more concise:

### Changes:
1. **Removed** the detailed "Allowed: letters, numbers..." rules line
2. **Simplified** error messages to be more direct:
   - "Prefix cannot contain '/'" (was "Prefix must be a single path segment (no '/').")
   - "Contains invalid sequence (.., @{)" (was "Prefix contains invalid sequence (.., @{, or @)")
   - etc.

3. **Updated all 3 languages** (English, Spanish, Japanese)

### Result:
- Helper text + preview on same line (cleaner)
- Real-time validation shows specific errors when needed
- Much less copy while maintaining clarity

All checks passed ✓

* Cleanup script changes for task attempt 8921b110-a98c-4968-9908-c710615a968e
 2025-10-07 16:33:36 +01:00
Alex Netsch
a733ca51d5 More events (vibe-kanban) (#964) 
* Perfect! I've successfully added analytics events to all the requested endpoints. Here's a summary of what was implemented:

## Summary

Added analytics tracking to the following endpoints:

### task_attempts.rs (6 endpoints)
- **start-dev-server**: `dev_server_started` event with task_id, project_id, and attempt_id
- **rebase**: `task_attempt_rebased` event with task_id, project_id, attempt_id, and old/new base branches
- **open-editor**: `task_attempt_editor_opened` event with attempt_id and editor_type
- **stop**: `task_attempt_stopped` event with attempt_id
- **change-target-branch**: `task_attempt_target_branch_changed` event with attempt_id and old/new target branches
- **children**: `task_attempt_children_viewed` event with attempt_id and children_count

### task_templates.rs (2 endpoints)
- **create-template**: `task_template_created` event with template_id, project_id, and is_global flag
- **update-template**: `task_template_updated` event with template_id, project_id, and is_global flag

### tasks.rs (1 endpoint)
- **delete-task**: `task_deleted` event with task_id, project_id, had_attempts flag, and attempt_count

### images.rs (1 endpoint)
- **upload-image**: Already had `image_uploaded` event tracking ✓

### projects.rs (2 endpoints)
- **open-editor**: `project_editor_opened` event with project_id and editor_type
- **/ (delete_project)**: `project_deleted` event with project_id

### approvals.rs (2 endpoints)
- **create**: `approval_created` event with approval_id, tool_name, and execution_process_id
- **respond**: `approval_responded` event with approval_id, status, tool_name, and execution_process_id

All changes have been tested and pass compilation and tests successfully.

* Cleanup script changes for task attempt 9a37ff17-c5ed-4155-a1d9-d97132d5e431

* Perfect! All review comments have been addressed successfully. Here's a summary of the changes made:

## Review Changes Summary

### 1. **Removed sensitive branch names** ✓
- **change-target-branch** (task_attempts.rs:1074-1081): Removed `old_target_branch` and `new_target_branch` fields
- **rebase** (task_attempts.rs:1168-1177): Removed `old_base_branch` and `new_base_branch` fields
- The event names themselves still indicate what action occurred, without exposing potentially sensitive branch names

### 2. **Added parent_count to children analytics** ✓
- **children** (task_attempts.rs:1332): Added `parent_count` field that tracks whether a parent task exists (0 or 1)
- Provides symmetry with `children_count` and future-proofs the analytics

### 3. **Removed redundant had_attempts field** ✓
- **delete-task** (tasks.rs:296-305): Removed `had_attempts` boolean field
- Kept only `attempt_count` since `attempt_count > 0` provides the same information without duplication

## Final Analytics Events

All endpoints now have clean, non-sensitive analytics tracking:

**task_attempts.rs:**
- `dev_server_started`: task_id, project_id, attempt_id
- `task_attempt_rebased`: task_id, project_id, attempt_id (branch names removed ✓)
- `task_attempt_editor_opened`: attempt_id, editor_type
- `task_attempt_stopped`: attempt_id
- `task_attempt_target_branch_changed`: attempt_id (branch names removed ✓)
- `task_attempt_children_viewed`: attempt_id, children_count, parent_count (added ✓)

**Other files remain unchanged:**
- task_templates.rs, tasks.rs, images.rs, projects.rs, approvals.rs all working as designed

All tests pass ✓

* Excellent! Compilation successful.

## Summary

Removed both unnecessary `clone()` calls from `approvals.rs`:

1. **Line 24** (`create_approval`): Removed `request.clone()` - we only used the `approval` object returned from the function, so no clone was needed.

2. **Line 66** (`respond_to_approval`): Removed `request.clone()` and changed the analytics to use the `status` variable from the match result instead of `request.status` - this is actually better because it tracks the actual status returned by the service rather than what was in the request.

Both changes make the code more efficient (avoiding unnecessary clones) and in the second case, more accurate (tracking the actual response status rather than the request status).
 2025-10-07 15:24:10 +01:00
Solomon
7c10c00d93 Upgrade Codex to the latest version (#947) 
* Upgrade Codex to the latest version

Use the new `app-server` protocol to interact with codex cli.

* Fix spawn errors

* simplify session forking

* Append spawn errors to the logs
 2025-10-07 14:57:41 +01:00
Gabriel Gordon-Hall
41eaa061fe fix: create multiple tasks bug (#958) 
* fix create multi task bug

(cherry picked from commit e44bf08cd16bf9f93ce8e774c5e8ccea782e8b55)

* fmt
 2025-10-07 14:56:56 +01:00
Louis Knight-Webb
e17de68547 Check i18n consistency (vibe-kanban) (#960) 
* ## Done! ✅

The [check-i18n.sh](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/a6aa-check-i18n-consi/scripts/check-i18n.sh) script has been extended with key consistency checking:

**What was added:**
- `get_json_keys()` - Extracts all nested keys from translation JSON files
- `check_key_consistency()` - Validates that all languages have the same keys as English
- Both checks now run in sequence with aggregated exit status

**Features:**
- Auto-discovers languages and namespaces from the filesystem
- Reports missing keys as errors (script fails)
- Reports extra keys as warnings (optional strict mode with `I18N_FAIL_ON_EXTRA=1`)
- Verbose mode available with `I18N_VERBOSE=1`

**Current issues detected:**
- Spanish translations missing 13 keys across common.json and settings.json
- Japanese translations appear to be up to date

* Perfect! All missing Spanish translations have been added and the script now passes ✅

**Added translations:**
- [es/common.json](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/a6aa-check-i18n-consi/frontend/src/i18n/locales/es/common.json): 8 conversation-related keys
- [es/settings.json](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/a6aa-check-i18n-consi/frontend/src/i18n/locales/es/settings.json): 5 settings-related keys
 2025-10-07 14:26:50 +01:00
Gabriel Gordon-Hall
0c8426ce72 Removed the diff view toggle from conversation diff entries so they always render inline, simplifying the conversation log UI. (#959) 
**Changes**
- `frontend/src/components/NormalizedConversation/EditDiffRenderer.tsx:14` drops the global diff-mode store and toggle import so the conversation view no longer renders the inline/split selector bar.
- `frontend/src/components/NormalizedConversation/EditDiffRenderer.tsx:119` locks `DiffView` to `DiffModeEnum.Unified`, guaranteeing inline diffs regardless of other toggles elsewhere.

**Tests**
- `pnpm run frontend:check` (fails: repository missing `node_modules`; `tsc` command unavailable).

**Next Steps**
1. Reinstall frontend dependencies and rerun `pnpm run frontend:check` to verify the TypeScript build.
 2025-10-07 14:24:49 +01:00
Louis Knight-Webb
5b930d9a59 Refactor branch selector (vibe-kanban) (#939) 
* Done. The input now stops keyboard events from bubbling to the DropdownMenu, and explicitly maintains focus when the menu opens.

* Done. Arrow keys, Enter, Escape, and Tab now work for navigation while typing maintains focus in the input.

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done. The component now handles all keyboard navigation manually:
- Arrow keys move the highlight without moving cursor
- Enter selects the highlighted item
- Escape closes the dropdown
- Typing maintains focus and filters the list
- Highlighted items auto-scroll into view

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done. The BranchSelector is now optimized - only rows with changed props re-render, making arrow key navigation instant even with many branches.

* Done. I've added `transition-none` to the BranchRow items to disable CSS transitions.

**Findings:**
- No global transition rules with long durations found
- The DropdownMenuItem component (line 84 in [dropdown-menu.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/fff8-typing-in-the-br/frontend/src/components/ui/dropdown-menu.tsx#L84)) has `transition-colors` in its base styles
- This transition applies to background/color changes on focus/hover

The `transition-none` class I added should override this. Please test if the arrow key navigation is now instant. If it is, we've confirmed it was the CSS transitions causing the lag.

* Done. The BranchSelector now uses react-virtuoso to render only visible items. This will make navigation instant even with thousands of branches - only ~10-20 DOM nodes are rendered at any time instead of the full list.

* Done. Fixed both issues:
- Mouse hover no longer triggers scrolling (only keyboard navigation scrolls)
- Highlighted items now have a visible accent background with a subtle ring for clear visual feedback

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done. Increased the highlight background opacity to 70% and made the ring stronger (ring-2) for much clearer visibility.

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done. The list now only scrolls when the highlighted item would go out of view, not on every arrow key press.

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done! Cleanup complete. The component is now cleaner and more maintainable:

**Changes made:**
1. ✅ Removed data-attribute handlers, replaced with direct closures
2. ✅ Changed `onClick` to `onSelect` (more correct for Radix)
3. ✅ Removed redundant `key` prop, using `computeItemKey` instead
4. ✅ Fixed tooltip on disabled items (wrapped in span for pointer events)
5. ✅ Clear search/highlight when dropdown closes
6. ✅ Renamed `highlighted` → `highlightedIndex` for clarity
7. ✅ Factored out `isBranchDisabled` helper to reduce duplication
8. ✅ Removed redundant `autoFocus` on Input

The code is now simpler (-30 lines), more readable, and follows Radix best practices. Ready for PR!

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done! i18n implementation complete for all supported languages:

**Translations added:**
- English (en) ✅
- Spanish (es) ✅
- Japanese (ja) ✅

**Component updates:**
- Added `useTranslation(['common'])` hook
- New optional `disabledTooltip` prop for workflow-specific messages
- All hardcoded strings now use translation keys
- Placeholder defaults to translated value if not provided by parent
- Disabled tooltip defaults to generic message, but can be overridden

Parents (like RebaseDialog) can now provide workflow-specific tooltips from the `tasks` namespace while the component remains generic and reusable.

* Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c

* Done. Removed the `onOpenAutoFocus` handler. The component is now ready for PR.

* style
 2025-10-07 11:10:00 +01:00
GitHub Action
fbb972a582 chore: bump version to 0.0.104 2025-10-06 16:55:58 +00:00
Alex Netsch
eb1d9ce275 Done! I've added the blue ExternalLink icon to the PR link in frontend/src/components/tasks/Toolbar/GitOperations.tsx:402. The icon will appear after the PR number text and inherit the text-info (blue) color from the parent button's className. (#951) 2025-10-06 17:48:09 +01:00
Alex Netsch
e863e2c8b9 No user deny messages in history (vibe-kanban) (#928) 
* Perfect! The implementation is clean and follows the existing patterns. Let me create a summary:

I've successfully implemented the backend solution for displaying user denial messages in the conversation history.

**File: `crates/services/src/services/approvals.rs`**

1. **Added import** (line 8):
   - Imported `EntryIndexProvider` from `executors::logs::utils`

2. **Modified `respond()` method** (lines 153-166):
   - After updating the tool status to `Denied`
   - Check if the `ApprovalStatus::Denied` variant has a `Some(reason)`
   - If yes:
     - Create an `EntryIndexProvider` initialized from the message store to get the next available index
     - Create a `NormalizedEntry` with `UserMessage` type containing the denial reason
     - Add it to the conversation using `ConversationPatch::add_normalized_entry()`

1. **Backend owns the state**: The conversation history lives in `MsgStore` on the backend - this is the proper source of truth
2. **Follows existing patterns**: Uses the same pattern as Claude executor (claude.rs:508-527) for adding user messages
3. **Proper indexing**: `EntryIndexProvider::start_from(&store)` ensures correct sequence numbering
4. **No frontend hackery**: Frontend just renders what backend provides
5. **Type safe**: Uses existing `NormalizedEntryType::UserMessage` - no new types needed
6. **Clean separation**: Backend manages state, frontend displays it

When a user denies an approval with feedback like "Please try a different approach":
1. Backend receives the denial in `approvals.rs:respond()`
2. Updates the tool entry status to `Denied { reason }`
3. Immediately adds a new `UserMessage` entry with the denial text to the conversation stream
4. Frontend receives this as a normal user message patch and displays it
5. The agent sees it as a user message in the conversation history

- ✅ TypeScript compilation passes
- ✅ Rust compilation passes
- ✅ No breaking changes
- ✅ Follows existing architectural patterns

Cleanup script changes for task attempt 8122f303-889b-4333-8024-e79b9ea7dc7e

Great question! Let me explain how a new `UserFeedback` variant would work - it might actually be cleaner. Let me check what that would look like:

A new variant would mean:
1. Add `UserFeedback` to `NormalizedEntryType` enum in `crates/executors/src/logs/mod.rs`
2. Backend uses `NormalizedEntryType::UserFeedback` instead of `UserMessage`
3. Frontend displays it like a user message (same UI component)
4. No filtering issues - it's a distinct type that won't be filtered
5. TypeScript types auto-generated, so frontend gets the type

Let me show you what this would look like:

**Pros:**
- Type-safe, explicit intent
- No filtering workarounds needed
- Clear semantic meaning
- Auto-generates TypeScript types

**Cons:**
- Larger change (need to update Rust enum, regenerate types, update frontend display logic)
- Need to run `npm run generate-types`

Let me present both options clearly so you can choose:

Perfect! Both frontend and backend compile successfully. Let me create a summary of what was implemented:

I've successfully implemented **Option 2: New `UserFeedback` Variant** to display denial messages in the conversation history.

**1. Backend: Added `UserFeedback` entry type** (`crates/executors/src/logs/mod.rs`)
- Added `UserFeedback` variant to `NormalizedEntryType` enum
- Marked with `#[ts(export)]` for TypeScript generation

**2. Backend: Use `UserFeedback` for denial messages** (`crates/services/src/services/approvals.rs`)
- When a denial has a reason, create a `NormalizedEntry` with type `UserFeedback`
- Add it to the conversation stream after the denied tool
- Removed unused metadata structs (cleaner than Option 1)

**3. TypeScript Types: Regenerated** (`shared/types.ts`)
- Ran `npm run generate-types`
- New type `{ "type": "user_feedback" }` added to `NormalizedEntryType`

**4. Frontend: Display `UserFeedback` as user message** (`DisplayConversationEntry.tsx`)
- Check for both `user_message` and `user_feedback` types
- Render both with the same `<UserMessage>` component
- Use same User icon for both types

1. **No filtering issues**: `UserFeedback` is a distinct type, won't be filtered by the user message filter
2. **Type-safe**: Explicit enum variant with auto-generated TypeScript types
3. **Semantic clarity**: Clear distinction between initial prompts and feedback
4. **Natural display**: Appears as a user message in the conversation flow
5. **Extensible**: Can be used for other user feedback scenarios in the future

When a user denies an approval with feedback like "Please try a different approach":
1. Backend creates a `UserFeedback` entry with that text
2. Frontend receives it and displays it as a user message
3. It appears in the conversation history naturally, right after the denied tool
4. The agent sees it in the context as user input

- ✅ Backend (Rust) compiles
- ✅ Frontend (TypeScript) compiles
- ✅ Types regenerated successfully
- ✅ Clean, type-safe implementation

Perfect! So the tool result IS processed. But looking at the log entry you provided, the denial message appears in a `tool_result` with `is_error: true`. Let me check what tool this corresponds to:

Perfect! Both backend and frontend compile successfully. Let me create a summary of the complete solution:

I implemented a **normalization-based approach** that works with both live sessions and old logs by extracting denial reasons from the tool_result that's already in the raw logs.

**1. Added `UserFeedback` Entry Type** (`crates/executors/src/logs/mod.rs:56`)
- New variant in `NormalizedEntryType` enum
- Represents user feedback during execution (like denial reasons)
- Auto-exported to TypeScript types

**2. Added Denial Reason Extraction** (`crates/executors/src/executors/claude.rs:316-339`)
- New function `extract_denial_reason()` that parses hook denial messages
- Pattern: `"operation blocked by hook:\n- {user's reason}"`
- Extracts the text after `"- "`

**3. Create UserFeedback During Normalization** (`crates/executors/src/executors/claude.rs:598-612` and `717-731`)
- When processing tool_result with `is_error: true`
- Call `extract_denial_reason()` to check for denial pattern
- If found, create a `UserFeedback` entry with the extracted reason
- Add it right after the denied tool
- Applies to both command tools and other tools (MCP, etc.)

**4. Display UserFeedback in Frontend** (`frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx`)
- Handle `user_feedback` type same as `user_message`
- Display with `<UserMessage>` component
- Use User icon

**5. Cleaned Up Approvals Service** (`crates/services/src/services/approvals.rs`)
- Removed the live-session-only code that added UserFeedback
- Removed unused `EntryIndexProvider` import
- Let normalization handle everything

1. **Denial reason IS in raw logs**: The hook sends denial as tool_result with `is_error: true` containing the user's feedback
2. **Survives restarts**: Raw logs are persisted to DB, including the tool_result
3. **Works for old logs**: When logs are re-normalized after restart, the extraction logic runs and creates the UserFeedback entry
4. **Type-safe**: New enum variant with proper TypeScript types
5. **No filtering issues**: `UserFeedback` is a distinct type, won't be filtered out

- ✅ Live sessions: Denial feedback appears as user message
- ✅ After restart: Old logs show denial feedback when re-normalized
- ✅ Type-safe: Proper enum variant, auto-generated TypeScript types
- ✅ Both compilations pass

When a user denies a tool with feedback like "Please try a different approach", it now appears as a separate user message in the conversation history, immediately after the denied tool.

Cleanup script changes for task attempt 8122f303-889b-4333-8024-e79b9ea7dc7e

Perfect! Both backend and frontend compile successfully. Let me create a final summary of all the changes:

1. ✅ **Duplicate code block removed** - Consolidated denial detection into one place after both command/non-command branches
2. ✅ **Pattern matching improved** - Changed from `"operation blocked by hook:"` to `"blocked by hook:"` to match both "Edit operation blocked by hook" and "operation blocked by hook"
3. ✅ **Magic strings replaced with constants** - Added `HOOK_DENIAL_PATTERN` and `HOOK_DENIAL_REASON_PREFIX` constants

**1. Added `UserFeedback` Entry Type** (`crates/executors/src/logs/mod.rs:56`)
- New variant in `NormalizedEntryType` enum for user feedback during execution

**2. Added Constants for Hook Patterns** (`crates/executors/src/executors/claude.rs:42-45`)
```rust
const HOOK_DENIAL_PATTERN: &str = "blocked by hook:";
const HOOK_DENIAL_REASON_PREFIX: &str = "- ";
```

**3. Added Denial Extraction Function** (`crates/executors/src/executors/claude.rs:321-345`)
- Detects hook denial messages in tool_result content
- Extracts user's reason from lines starting with `"- "`
- Uses constants for maintainability

**4. Create UserFeedback During Normalization** (`crates/executors/src/executors/claude.rs:703-717`)
- After processing any tool_result with `is_error: true`
- Extract denial reason and create `UserFeedback` entry
- Add it immediately after the denied tool
- **Single consolidated check** for both command and non-command tools

**5. Display UserFeedback in Frontend** (`frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx`)
- Handle `user_feedback` type same as `user_message`
- Display with `<UserMessage>` component
- Use User icon

**6. Regenerated TypeScript Types** (`shared/types.ts:287`)
- Added `{ "type": "user_feedback" }` to `NormalizedEntryType`

**7. Cleaned Up Approvals Service** (`crates/services/src/services/approvals.rs`)
- Removed unused live-session code
- Let normalization handle everything

1. **Works with old logs**: Denial reason is in persisted tool_result, extracted during re-normalization
2. **Works with live sessions**: Denial extracted during initial normalization
3. **No magic strings**: Uses constants for pattern matching
4. **No duplication**: Single denial check after tool processing
5. **Flexible pattern**: Matches "Edit operation blocked by hook" and "operation blocked by hook"
6. **Type-safe**: Proper enum variant with TypeScript types

- ✅ Backend compiles
- ✅ Frontend compiles
- ✅ No duplicate code
- ✅ Pattern improved to match your example

Cleanup script changes for task attempt 8122f303-889b-4333-8024-e79b9ea7dc7e

Perfect! Everything compiles. Let me create a summary of the final solution:

**1. Python Hook - Natural Language Marker** (`crates/executors/src/executors/hooks/confirm.py:14`)
- Prefix denial reason with `"User feedback: {reason}"`
- Natural language format that's both readable to LLM and parseable
- Example output: `"User feedback: Could you add something about a goose?"`

**2. Rust Constant** (`crates/executors/src/executors/claude.rs:44`)
```rust
const USER_FEEDBACK_MARKER: &str = "User feedback: ";
```

**3. Robust Extraction Function** (`crates/executors/src/executors/claude.rs:320-340`)
- Search for `USER_FEEDBACK_MARKER` in tool_result content
- Extract everything after the marker
- No dependency on Claude Code's error message format

**4. Added `UserFeedback` Entry Type** (`crates/executors/src/logs/mod.rs:56`)
- New variant for user feedback during execution

**5. Create UserFeedback During Normalization** (`crates/executors/src/executors/claude.rs:703-717`)
- When tool_result has `is_error: true`, check for user feedback
- Create `UserFeedback` entry with extracted text
- Works for both live sessions and old logs (persisted in raw logs)

**6. Frontend Display** (`frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx`)
- Render `user_feedback` same as `user_message`

1. ✅ **We control the format**: The `"User feedback: "` prefix is added by OUR Python script
2. ✅ **Claude Code independent**: Works regardless of how Claude Code formats hook errors
3. ✅ **Natural language**: Reads naturally - LLM sees "User feedback: {reason}" which is clear
4. ✅ **Survives restarts**: Tool_result is in persisted raw logs, re-extracted on normalization
5. ✅ **Backward compatible**: Old logs without marker simply won't match (no errors)
6. ✅ **Simple**: One constant, one natural prefix

**User denies with**: `"Could you add something about a goose?"`

**Raw log contains**:
```json
{"type":"tool_result","content":"Edit operation blocked by hook:\nUser feedback: Could you add something about a goose?","is_error":true}
```

**Extraction**: Finds `"User feedback: "`, extracts `"Could you add something about a goose?"`

**Display**: Shows as user message in conversation history

**Result**: User's feedback is visible to both user and LLM, survives restarts!

Perfect! The solution is complete. Let me create a final summary:

**Single Source of Truth:** The marker `"User feedback: "` is defined ONLY in Rust and injected into Python at runtime.

**1. Rust Constant** (`crates/executors/src/executors/claude.rs:44`)
```rust
const USER_FEEDBACK_MARKER: &str = "User feedback: ";
```

**2. Python Script Placeholder** (`crates/executors/src/executors/hooks/confirm.py:15-16`)
```python
USER_FEEDBACK_MARKER = "{{USER_FEEDBACK_MARKER}}"
formatted_reason = f"{USER_FEEDBACK_MARKER}{reason}" if reason else None
```

**3. Template Replacement** (`crates/executors/src/executors/claude.rs:241-244`)
```rust
let script_content = CONFIRM_HOOK_SCRIPT.replace(
    "{{USER_FEEDBACK_MARKER}}",
    USER_FEEDBACK_MARKER,
);
file.write_all(script_content.as_bytes()).await?;
```

**4. Extraction Function** (`crates/executors/src/executors/claude.rs:320-340`)
- Searches for `USER_FEEDBACK_MARKER` in tool_result content
- Extracts feedback text after the marker

**5. UserFeedback Entry Creation** (`crates/executors/src/executors/claude.rs:703-717`)
- During normalization, detects denials and creates UserFeedback entries

**6. Frontend Display** (`frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx`)
- Renders UserFeedback as user messages

**7. TypeScript Types** (`shared/types.ts`)
- `user_feedback` variant generated

**At deployment time:**
1. Rust defines `USER_FEEDBACK_MARKER = "User feedback: "`
2. Python script has placeholder `{{USER_FEEDBACK_MARKER}}`
3. When writing hook, Rust replaces `{{USER_FEEDBACK_MARKER}}` → `"User feedback: "`
4. Python script executes with actual value

**When user denies:**
1. Python outputs: `"User feedback: Could you add something about a goose?"`
2. This goes into tool_result in raw logs (persisted)
3. During normalization, Rust finds `"User feedback: "` and extracts the text after it
4. Creates UserFeedback entry
5. Frontend displays it as a user message

✅ **Single source of truth**: Marker defined only in Rust constant
✅ **No duplication**: Python gets value via template replacement
✅ **Format independent**: Works regardless of Claude Code's error format
✅ **Natural language**: LLM sees readable "User feedback: {text}"
✅ **Survives restarts**: Tool_result persisted in DB, re-extracted on normalization
✅ **Maintainable**: Change constant in one place, both Rust and Python updated

- ✅ Backend compiles
- ✅ Frontend compiles
- ✅ Single source of truth maintained

Cleanup script changes for task attempt 8122f303-889b-4333-8024-e79b9ea7dc7e

Perfect! Everything compiles. Let me create a final summary:

**1. Backend: Added Tool Name to UserFeedback** (`crates/executors/src/logs/mod.rs:55-57`)
```rust
UserFeedback {
    denied_tool: String,
}
```

**2. Backend: Capture Display Tool Name** (`crates/executors/src/executors/claude.rs:573-589`)
- Extract tool name before processing branches
- For MCP tools, format as `mcp:server:tool` instead of `mcp__server__tool`
- Store in `display_tool_name` variable

**3. Backend: Include Tool Name in UserFeedback** (`crates/executors/src/executors/claude.rs:734-735`)
```rust
entry_type: NormalizedEntryType::UserFeedback {
    denied_tool: display_tool_name.clone(),
},
```

**4. Backend: Single Source of Truth for Marker** (`crates/executors/src/executors/claude.rs:44`)
```rust
const USER_FEEDBACK_MARKER: &str = "User feedback: ";
```

**5. Python: Placeholder Template** (`crates/executors/src/executors/hooks/confirm.py:15-16`)
```python
USER_FEEDBACK_MARKER = "{{USER_FEEDBACK_MARKER}}"
formatted_reason = f"{USER_FEEDBACK_MARKER}{reason}" if reason else None
```

**6. Backend: Template Replacement** (`crates/executors/src/executors/claude.rs:241-244`)
```rust
let script_content = CONFIRM_HOOK_SCRIPT.replace(
    "{{USER_FEEDBACK_MARKER}}",
    USER_FEEDBACK_MARKER,
);
```

**7. Frontend: Custom Red Accent Display** (`DisplayConversationEntry.tsx:648-666`)
- Red background (`bg-red-50 dark:bg-red-950/20`)
- Red border (`border-red-400/40`)
- Bold header showing: `"{ToolName} denied by user"`
- Red text for the feedback content
- **NO** edit/retry buttons (read-only display)

**8. TypeScript Types Regenerated** (`shared/types.ts`)
```typescript
{ "type": "user_feedback", denied_tool: string }
```

When user denies "Edit" tool with "Could you add something about a goose?":

```
┌─────────────────────────────────────────┐
│ Edit denied by user                     │  ← Bold red text
│ Could you add something about a goose?  │  ← Red feedback text
└─────────────────────────────────────────┘  ← Red border/background
```

✅ **Single source of truth**: Marker defined only in Rust, injected into Python
✅ **Tool name included**: Shows which tool was denied
✅ **Distinct styling**: Red accent makes it clear this is a denial
✅ **No edit/retry**: Simple read-only display, no complex user message features
✅ **Works with old logs**: Tool_result persisted, re-extracted on normalization
✅ **Type-safe**: Tool name field in enum, auto-generated TypeScript types

- ✅ Backend compiles
- ✅ Frontend compiles
- ✅ Types regenerated
- ✅ No duplication

Cleanup script changes for task attempt 8122f303-889b-4333-8024-e79b9ea7dc7e

Perfect! The new styling should be much more subtle and blend better with the existing UI:

- **Background**: Same as user messages (no bright red)
- **Border**: Dashed like user messages, with subtle red tint (30% opacity)
- **Header**: Small text (`text-xs`), muted red, 80% opacity
- **Content**: Same font-light styling as regular user messages

This keeps the denial feedback visible but not overwhelming, similar to how the regular user message integrates into the conversation flow.

Perfect! The new styling is much more subtle:

- **Background**: Same `bg-background` as user messages (no bright red)
- **Border**: Standard dashed border (no red tint)
- **Header**: Uses semantic `--destructive` color with 70% opacity for subtlety
- **Content**: Standard user message styling (no red text)

This approach:
- ✅ Uses semantic CSS variables for proper dark mode support
- ✅ Much less visually intense - just a subtle red label
- ✅ Blends better with the conversation flow
- ✅ The small "denied by user" label provides context without overwhelming

The visual hierarchy is now: subtle header hint → main content, which matches the rest of the UI better.

* Tool denial feedback (vibe-kanban 35abbc86)

The last commit implement log entries for tool denials, it has some problems though:
Update json_error in crates/executors/src/executors/hooks/confirm.py so the marker is only prefixed when an actual user denial reason is passed—fallback errors (network failure, invalid JSON, timeouts) must keep their original text to avoid generating fake user_feedback entries.
Adjust the hook deployment in write_python_hook (crates/executors/src/executors/claude.rs) to refresh existing confirm.py files when the bundled script changes: either always rewrite, compare hashes/timestamps, or at least overwrite when the placeholder is still present. Without that, previously installed hooks never get the marker update.
Refine extract_denial_reason in claude.rs to support the real payloads coming back from Claude: preserve multi-line feedback (don’t stop at the first newline) and handle array/object forms such as [{"type":"text","text":"…"}] so the full user message survives.

lets fix these!

* Clippy

* Perfect! All changes are complete. Here's the summary:

**1. Added i18n Hook** (`DisplayConversationEntry.tsx:603`)
```typescript
const { t } = useTranslation('common');
```

**2. Used Translation Key** (`DisplayConversationEntry.tsx:661`)
```typescript
{t('conversation.deniedByUser', { toolName: feedbackEntry.denied_tool })}
```

**3. Added English Translation** (`frontend/src/i18n/locales/en/common.json:38`)
```json
"deniedByUser": "{{toolName}} denied by user"
```

**4. Added Spanish Translation** (`frontend/src/i18n/locales/es/common.json:26`)
```json
"deniedByUser": "{{toolName}} denegado por el usuario"
```

**5. Added Japanese Translation** (`frontend/src/i18n/locales/ja/common.json:38`)
```json
"deniedByUser": "{{toolName}} がユーザーによって拒否されました"
```

**6. Used Semantic Color** (`DisplayConversationEntry.tsx:658-659`)
```typescript
style={{ color: 'hsl(var(--destructive))' }}
```

The UserFeedback display now:
- ✅ Uses semantic `--destructive` color (adapts to light/dark mode automatically)
- ✅ Supports i18n for English, Spanish, and Japanese
- ✅ Subtle styling that blends with conversation flow
- ✅ Maintains tool name context

The visual is now much more subtle with proper semantic colors and internationalization support!

* Perfect! Everything compiles. Let me create a summary of the final solution:

**1. Python: Added Argument Parser** (`confirm.py:96-102`)
```python
parser.add_argument(
    "-m",
    "--feedback-marker",
    type=str,
    required=True,
    help="Marker prefix for user feedback messages.",
)
```

**2. Python: Updated json_error to Accept Marker** (`confirm.py:11`)
```python
def json_error(reason: Optional[str], feedback_marker: Optional[str] = None) -> None:
    formatted_reason = reason
    if reason and feedback_marker:
        formatted_reason = f"{feedback_marker}{reason}"
```

**3. Python: Pass Marker When Denying** (`confirm.py:162`)
```python
json_error(reason, args.feedback_marker)
```

**4. Rust: Pass Marker as CLI Arg** (`claude.rs:283`)
```rust
format!("... --feedback-marker {}", shlex::try_quote(USER_FEEDBACK_MARKER)...)
```

**5. Rust: Removed String Replacement** (`claude.rs:236-238`)
- No longer need to replace placeholders
- Just write the script as-is

✅ **Cleaner**: No template placeholder magic, just a regular CLI arg
✅ **Standard**: Follows same pattern as other args (timeout, port, etc.)
✅ **Explicit**: Marker is passed explicitly each time hook runs
✅ **Flexible**: Easy to change or extend with more parameters
✅ **Single source of truth**: Still only defined in Rust constant

**Rust defines**:
```rust
const USER_FEEDBACK_MARKER: &str = "User feedback: ";
```

**Rust invokes Python with**:
```bash
confirm.py --timeout-seconds 3605 --poll-interval 5 --backend-port 8080 --feedback-marker "User feedback: "
```

**Python receives and uses**:
```python
args.feedback_marker  # "User feedback: "
formatted_reason = f"{args.feedback_marker}{reason}"
```

No template replacement needed - clean and straightforward!

Excellent! Everything compiles. The solution is now complete and much cleaner:

**Single Source of Truth:**
```rust
const USER_FEEDBACK_MARKER: &str = "User feedback: ";
```

**How it flows:**

1. **Rust → Python** (via CLI arg):
   ```bash
   confirm.py ... --feedback-marker 'User feedback: '
   ```

2. **User denies → Python formats**:
   ```python
   formatted_reason = f"User feedback: Could you add something about a goose?"
   ```

3. **Goes into raw logs → Rust extracts**:
   ```rust
   extract_denial_reason() // finds "User feedback: " and extracts what follows
   ```

4. **Creates UserFeedback entry**:
   ```rust
   NormalizedEntryType::UserFeedback {
       denied_tool: "Edit"
   }
   content: "Could you add something about a goose?"
   ```

5. **Frontend displays with red accent**:
   ```
   Edit denied by user          ← Small red text
   Could you add something about a goose?  ← Normal text
   ```

✅ **No string replacement** - just a CLI argument
✅ **No sketchy fallbacks** - simple, direct constant usage
✅ **Single source of truth** - marker only defined in Rust
✅ **Type-safe** - `denied_tool` field in enum
✅ **i18n support** - translatable display text
✅ **Semantic colors** - uses `--destructive` CSS variable
✅ **Works with old logs** - survives restarts via raw log persistence

The implementation is now straightforward with no unnecessary complexity!

* Simplify marker parsing
 2025-10-06 15:38:18 +01:00
Solomon
2e0cabf9c7 Handle stderr output form Cursor cli (#948) 2025-10-06 15:26:46 +01:00