Plan Improvements

[nsosio]

• Async submit workflow: PR creation now runs as a background job (review → commit → PR) instead of a direct API call
• Build validation phase: New validating status shows when lint/typecheck is running, with "Quality check" task in UI
• Submit status tracking: Track progress through review → commit → PR creation with real-time polling
• Improved build UI: Status pills, always-visible progress bar, chat disabled after PR creation
• Tickets path stored: Build jobs now store path to tickets.json for review context

https://www.loom.com/share/c32a7d8e51e14766bb1cdfec8adf9d1b

See also kosuke-cli #42

[nsosio]

@claude review

[claude]

Claude finished @nsosio's task in 50s —— View job

---

I am Kosuke 🤖, the Web Expert

PR Review: Plan Improvements

Status: ✅ Review Complete

---

Summary

This is a solid architectural improvement that introduces a new Submit Worker for handling the review→commit→PR workflow as a background job. The changes follow the CLAUDE.md worker pattern correctly (factory functions, no side effects), improve database schema organization, and enhance the build/preview UI. The implementation is well-structured with proper type safety and error handling.

---

✅ Strengths

1. Worker Pattern Implementation (Excellent)

• ✅ submitWorker correctly uses factory function pattern (no module-level side effects)
• ✅ Proper initialization in src/worker.ts alongside other workers
• ✅ Safe to import queue from anywhere without side effects
• ✅ Follows CLAUDE.md Background Jobs & BullMQ Worker Pattern guidelines perfectly

2. Database Schema Changes

• ✅ New submitStatusEnum with proper pgEnum definition for database-level validation
• ✅ Added submitStatus column to buildJobs table
• ✅ Added ticketsPath column for storing context path
• ✅ Migration properly defined (0009_wild_justin_hammer.sql)
• ✅ Type inference from pgEnum: SubmitStatus type properly exported

3. API Route Design

• ✅ Clear POST/GET separation in submit-build/[buildJobId]/route.ts
• ✅ Proper access control and validation
• ✅ Comprehensive error handling with ApiErrorHandler
• ✅ Validation of build status before submission
• ✅ Supports optional PR title in request body

4. Type Safety

• ✅ SubmitJobData interface properly typed
• ✅ SubmitJobResult interface defined
• ✅ Hook response types well-defined in useLatestBuild and useSubmitBuild
• ✅ No use of any type

5. Hook Implementation

• ✅ useSubmitBuild hook properly integrated with TanStack Query mutation
• ✅ Correct error handling and toast notifications
• ✅ Query invalidation on success triggers status refresh
• ✅ useLatestBuild properly polls with appropriate intervals

6. UI/UX Improvements

• ✅ PreviewPanelProps properly typed with submit status tracking
• ✅ Submit button state management (isSubmitting, hasSubmitted, canSubmit)
• ✅ Dynamic button labels based on submit status phases
• ✅ Clear feedback during review→commit→PR workflow

---

🔍 Issues & Recommendations

1. Minor: Incomplete Cleanup in useLatestBuild ⚠️

File: src/hooks/use-latest-build.ts (lines 34-48)

The hook polls with different intervals but doesn't handle the case where submitStatus === 'failed'. Users might not see immediate updates after a failed submit retry.

Recommendation: Add polling logic for failed state:

// After retry is attempted, should return to normal polling to update status
if (data?.submitStatus === 'failed') {
  return 1000 * 5; // Medium polling for retry detection
}

2. Type Coverage: BuildTask Status ⚠️

File: src/lib/types/chat.ts (line 228)

The BuildTask.status type includes 'cancelled' state, but verify all API endpoints return this state consistently from the database. The schema uses taskStatusEnum which includes 'cancelled', so this is correct, but worth double-checking in build status API.

3. SSE Error Handling in Submit Worker ⚠️

File: src/lib/queue/workers/submit.ts (lines 214-219)

The worker catches SyntaxError separately when parsing SSE events. However, if the /api/submit endpoint returns malformed SSE for an extended time, the worker will keep retrying without backoff. Consider adding:

• Maximum retry attempts for parsing errors
• Exponential backoff for repeated failures
• Logging of pattern of failures (e.g., 3+ consecutive parse errors)

Current code:

catch (error) {
  if (error instanceof SyntaxError) {
    console.warn('[SUBMIT] ⚠️  Failed to parse SSE event');
    // No retry logic or state tracking
  }
}

4. PR URL Extraction Logic ⚠️

Files: src/lib/queue/workers/submit.ts (line 230) and src/app/api/projects/[id]/chat-sessions/[sessionId]/submit-build/[buildJobId]/route.ts (line 195-196)

Extracting PR number via regex pattern /\/pull\/(\d+) works but is fragile if GitHub API format changes. More robust approach:

// Better: Parse URL as object
const url = new URL(prUrl);
const pullRequestNumber = parseInt(url.pathname.split('/').pop() || '0', 10);

However, current implementation is acceptable since GitHub URLs are standardized.

5. Missing Timeout Configuration ⚠️

File: src/lib/queue/workers/submit.ts (line 39)

The fetch call to sandbox /api/submit has no timeout. If the sandbox hangs or becomes unresponsive, the worker will block indefinitely. Add timeout:

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout

try {
  const response = await fetch(submitUrl, {
    method: 'POST',
    signal: controller.signal,
    // ... rest of config
  });
} finally {
  clearTimeout(timeoutId);
}

6. Database Constraint Check ⚠️

File: src/lib/db/schema.ts

The buildJobs table allows nullable submitStatus (no .notNull()). This is correct design (builds can complete without being submitted), but verify in tests that:

• Null submitStatus is handled correctly in queries
• Frontend properly handles submitStatus === null case

Already handled in useLatestBuild (line 45) where it checks data.submitStatus !== 'done', good defensive coding.

---

🎯 Verification Checklist

Before merging, verify:

• Database migrations run successfully: bun run db:generate then bun run db:migrate
• bun run typecheck passes (no type errors)
• bun run lint passes (no style issues)
• bun run test passes (all tests green)
• bun run knip passes (no unused exports)
• bun run build succeeds
• Worker process starts without errors: bun run workers:dev
• End-to-end: Can submit a completed build and see status updates in UI

---

📋 Architecture Notes

Positive patterns observed:

1. ✅ Worker uses BullMQ best practices (concurrency: 1 for sequential submit)
2. ✅ SSE parsing is robust with event-by-event handling
3. ✅ Status phases are granular: pending → reviewing → committing → creating_pr → done
4. ✅ Proper separation of concerns: API validates, queue enqueues, worker executes
5. ✅ Factory pattern prevents accidental worker initialization in API routes

---

Summary

This PR successfully implements the submit workflow as a background job following the established patterns in the codebase. The code is production-ready with minor recommendations for error handling resilience. All CLAUDE.md guidelines are followed correctly.

Recommendation: Approve with minor fixes for timeout handling and SSE error resilience.

---