Skip to content

Implement task execution with Claude Code#23

Open
etsraphael wants to merge 3 commits intomainfrom
feature/claude-code-execution
Open

Implement task execution with Claude Code#23
etsraphael wants to merge 3 commits intomainfrom
feature/claude-code-execution

Conversation

@etsraphael
Copy link
Copy Markdown
Owner

Introduce functionality for executing tasks using Claude Code, including tracking execution status and handling output. This update adds new routes for task execution, cancellation, and retrieval of execution history. Additionally, it establishes a database schema for task executions and integrates the Claude Code CLI for processing tasks.

Copilot AI review requested due to automatic review settings March 19, 2026 07:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds “Claude Code task execution” to Forge so a task’s description can be run via the Claude Code CLI, streamed back to the UI, and recorded in SQLite for later review.

Changes:

  • Add server-side execution service that spawns the claude CLI and streams stream-json output.
  • Add task execution API endpoints for start (SSE), cancel, and history retrieval.
  • Add task_executions table + frontend UI panel to start/cancel execution and display output.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
server/services/claude-code.js Spawns Claude Code CLI, tracks active executions for cancellation, persists execution metadata.
server/routes/execute.js Adds endpoints to start/cancel executions and list recent execution history (with SSE streaming).
server/index.js Mounts the new task execution routes under /api/tasks.
server/db.js Adds task_executions schema + index.
package.json Adds @anthropic-ai/claude-code dependency.
package-lock.json Locks the new dependency and its transitive optional deps.
forge-fe/src/types/index.ts Adds execution status + TaskExecution types for FE consumption.
forge-fe/src/components/board/task-detail-modal.tsx Adds “Execute with Claude Code” UI with streaming output + cancel button.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/db.js
Comment on lines +63 to +72
CREATE TABLE IF NOT EXISTS task_executions (
id TEXT PRIMARY KEY,
task_id TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'running' CHECK (status IN ('running', 'completed', 'failed', 'cancelled')),
prompt TEXT NOT NULL,
output TEXT NOT NULL DEFAULT '',
error TEXT,
started_at TEXT DEFAULT (datetime('now')),
completed_at TEXT
);
const [isDeleting, setIsDeleting] = useState(false)
const [executionStatus, setExecutionStatus] = useState<ExecutionStatus | 'idle'>('idle')
const [executionOutput, setExecutionOutput] = useState<Array<{ type: string; content: string }>>([])
const [executionId, setExecutionId] = useState<string | null>(null)
Comment on lines +186 to +193
} else if (event.type === 'done') {
setExecutionStatus('completed')
} else if (event.type === 'error') {
setExecutionOutput((prev) => [
...prev,
{ type: 'error', content: event.error || 'Unknown error' },
])
setExecutionStatus('failed')
Comment on lines +82 to +88
const child = spawn(
'claude',
[
'-p', prompt,
'--output-format', 'stream-json',
'--dangerously-skip-permissions',
],
Comment on lines +82 to +94
const child = spawn(
'claude',
[
'-p', prompt,
'--output-format', 'stream-json',
'--dangerously-skip-permissions',
],
{
cwd,
stdio: ['ignore', 'pipe', 'pipe'],
signal,
},
)
Comment on lines +150 to +165
child.on('close', (code) => {
// flush remaining buffer
if (buffer.trim()) {
try {
const msg = JSON.parse(buffer.trim())
enqueue({ type: 'message', data: msg })
} catch {
// ignore
}
}
if (code !== 0 && code !== null) {
exitError = new Error(`claude exited with code ${code}`)
}
done = true
enqueue({ type: 'end' })
})
Comment thread server/routes/execute.js
Comment on lines +33 to +40
const send = (obj) => res.write(`data: ${JSON.stringify(obj)}\n\n`)

send({ type: 'execution_start', execution_id: executionId, task_id: taskId })

// Cancel on client disconnect
req.on('close', () => {
cancelExecution(executionId)
})
Comment thread server/routes/execute.js
})
res.json({ ok: true, execution_id: execution.id })
} else {
res.status(404).json({ error: 'Execution not found or already finished' })
@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

Backend Security Review

Critical findings — fix before merging

1. Reflected XSS — server/routes/githubOAuth.js:160
The OAuth callback passes req.query.error directly into an HTML string without escaping: 'Authorization failed: ' + error. An attacker crafts a callback URL with error=<img src=x onerror=alert(1)> to trigger XSS in the popup window. Fix: HTML-escape the error value before embedding it in the HTML response.

2. SSRF — server/routes/connectors.js:129-130
User-supplied config.baseUrl is fetched without URL validation: fetch(baseUrl + '/api/tags'). A connector owner can point this at AWS instance metadata (169.254.169.254), Redis, or internal services. Fix: validate the scheme and block RFC-1918 / link-local ranges before fetching.

3. Prompt injection + --dangerously-skip-permissions — server/services/claude-code.js:39,87
User-controlled task.title and task.description are interpolated into the Claude CLI prompt, which is spawned with --dangerously-skip-permissions (bypasses all permission guards). A malicious task description can instruct Claude to delete files, exfiltrate secrets, or run arbitrary shell commands with zero prompts. Evaluate whether this flag is strictly necessary; if so, restrict via --allowedTools and audit task content before execution.


High severity

4. Secrets returned in plaintext — server/routes/connectors.js:16-26
GET /api/connectors returns full config including GitHub PATs verbatim. Mask sensitive fields (token: '***' or hasToken: true) in responses.

5. No authentication on any endpoint
All routes are unauthenticated. Any process reaching the server can read tokens, trigger Claude executions, or modify the board. Known trade-off for a local tool, but should be documented. CORS correctly disabled in production — good.


Medium severity

6. Unbounded numeric query params — routes/local-git.js:74, routes/github.js:51-52
count and per_page accepted without capping. Passing count=1000000 causes excessive load or rate-limit burn. Cap values, e.g. Math.min(Number(count) || 20, 100).

7. column_id not validated in PUT /api/tasks/reorder — routes/tasks.js:118-119
SQL is parameterized (no injection risk) but column_id can be any string, letting tasks land in invalid columns. Validate against VALID_COLUMNS from action-executor.js.

8. Missing global error handler — server/index.js
No app.use((err, req, res, next) => ...) middleware. Uncaught synchronous errors produce empty responses or crash the process. Add a catch-all at the end of index.js.


Low / Informational

  1. sort query param in githubOAuth.js:118 passed to Octokit without validating against allowed values (created, updated, pushed, full_name).
  2. console.error in context-builder.js:314,394 is acceptable but a structured logger is better for production.

What looks good

  • All SQL uses parameterized statements throughout — no SQL injection found
  • execFileSync with array args prevents shell injection in local-git.js
  • safePath() correctly blocks path traversal and .git directory access
  • OAuth CSRF protection with crypto.randomUUID() and pendingStates cleanup is correct
  • postMessage uses window.location.origin (not '*') — correct
  • Field allowlists in action-executor.js and routes/tasks.js are enforced before dynamic SQL column name construction, making those UPDATE statements safe
  • AbortSignal.timeout(3000) prevents indefinite upstream hangs
  • No hardcoded secrets — all from env vars or DB

@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

Frontend Code Review - forge-fe/ Overall this is a well-structured React 19 + TypeScript app. See full review details in the PR thread.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

test body no hash lines here just plain text to see if this works without heading lines

@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

Frontend Code Review — forge-fe/

Overall this is a well-structured React 19 + TypeScript app with clean component organization, good use of Tailwind, and solid TypeScript typing throughout. Below are the issues found, grouped by severity.

Security

1. XSS via javascript: protocol links in Markdown (markdown-content.tsx)

react-markdown without rehype-raw is safe from raw HTML injection, but markdown links using the javascript: protocol still render as clickable anchor tags. An AI response containing click me would execute in the user browser.

Add a custom link renderer that allowlists safe protocols:

The renderer should check href?.startsWith for http://, https://, or hash prefix and render a span for unsafe URLs.

This applies anywhere MarkdownContent is used: chat messages, task descriptions (task-detail-modal.tsx), and AI command output.


Accessibility

2. span used as interactive delete button (Chat.tsx line 142)

The session delete button is a span — it is not reachable by keyboard and does not announce its role to screen readers. Replace with a button element with type=button.

3. Missing aria-label on icon-only interactive elements

  • Chat.tsx line 618 — Send button has no aria-label
  • app-shell.tsx line 83 — Settings icon NavLink reads nothing to screen readers
  • approval-detail-modal.tsx lines 96-111 — Approve and Reject icon-only buttons

4. label elements not associated with inputs (Connectors.tsx)

Labels in the add-connector forms lack htmlFor attributes, and inputs lack id attributes. Screen readers cannot associate them.

5. Missing aria-label on chat textarea (Chat.tsx line 597)

Placeholders are not reliable labels for assistive technology. Add aria-label=Message input.


Missing Fetch Abort on Unmount

6. Streaming fetches have no AbortController (Chat.tsx line 380, task-detail-modal.tsx line 132)

If the user navigates away mid-stream, the ReadableStream reader continues consuming network and calling state setters on an unmounted component. Use an AbortController and tie its abort() to a useEffect cleanup.


Console Statements Left In

File Lines Issue
task-detail-modal.tsx 262, 278 console.error(err) in save/delete — user sees nothing on failure
kanban-board.tsx 53, 163 console.error only
Connectors.tsx 1070, 1073, 1107, 1113 console.error with no fallback UI

For handleSave and handleDelete in the task modal especially, the user should see an inline error message if the operation fails.


React and TypeScript

7. Reused key in INITIAL_MESSAGES after chat reset (Chat.tsx lines 38, 357)

INITIAL_MESSAGES is a module-level constant with a fixed ID generated once at module load. startNewChat() resets to this same object, so every new chat session reuses the same React key. Generate a fresh ID each time instead of reusing the constant.

8. Weak typing via as casts in kanban-board.tsx lines 36-40

RawTask types type, column_id, and priority as plain string, then casts with as. If the API returns an unexpected value the cast silently lies to TypeScript. Consider a type guard or runtime validation at the API boundary.

9. activeSessionId stale closure in streaming handler (Chat.tsx line 411)

The streaming loop captures activeSessionId from the outer render closure. This works correctly today but is fragile — worth a comment explaining the intentional snapshot.


Error Handling and UX

10. No error boundary

There is no ErrorBoundary wrapping any route or subtree. A rendering crash (e.g., malformed task data from API) will white-screen the whole app with no user feedback.

11. Connector fetch failures are silent (Connectors.tsx)

handleAdd, handleToggle, and handleDelete all .catch(console.error) without surfacing errors to the user. If the server is down the UI appears to succeed but silently fails.


Positives

  • react-markdown used without rehype-raw — raw HTML injection from markdown is blocked.
  • OAuth postMessage handler correctly validates event.origin (Connectors.tsx line 196).
  • MarkdownContent is memoized with React.memo.
  • API endpoints use relative /api/... paths — no hardcoded base URLs.
  • No API keys or secrets in frontend code.
  • Consistent, well-typed props throughout — almost no untyped any.
  • ModelSelector click-outside handler properly cleans up its event listener.
  • dnd-kit includes KeyboardSensor for accessible drag-and-drop.

Priority fixes: (1) javascript: link XSS in markdown renderer, (2) replace the span delete button with a proper button element, (3) aria-label on icon-only interactive elements, (4) streaming fetch AbortController cleanup.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

Frontend Code Review — forge-fe/

Overall the code is well-structured, readable, and TypeScript-typed. Below are the issues found, ordered by severity.


🔴 Bugs

1. Server-sent error events are silently swallowed — Chat.tsx:405-463

The throw new Error(event.error) on line 458 is inside the inner try/catch block that is meant to skip malformed JSON. It gets caught immediately by catch { // skip malformed line } and never propagates to the outer catch that shows an error message to the user.

try {
  const event = JSON.parse(line.slice(6))
  // ...
  } else if (event.type === 'error') {
    throw new Error(event.error)   // ← caught by ↓ not the outer catch
  }
} catch {
  // skip malformed line            // ← swallows the thrown error
}

Fix: handle the error type before the try/catch, or set a local flag and break out of the loop.


2. persistReorder uses stale closure state — kanban-board.tsx:126

When a card is dropped onto a column (not onto another card), handleDragEnd calls persistReorder(tasks) where tasks is the state value from the current render. But handleDragOver has already called setTasks(...) optimistically — that update is batched and tasks still reflects the pre-drag column. The server receives the wrong column assignment.

if (isOverColumn) {
  persistReorder(tasks)   // ← stale; should use a ref or a functional read
  return
}

Fix: store latest tasks in a useRef that tracks state, or read from a ref that is kept in sync.


🟠 Accessibility

3. Session delete button is a <span>, not a <button>Chat.tsx:142-151

<span
  onClick={(e) => { e.stopPropagation(); onDelete(s.id) }}
  title="Delete"
  className="..."
>
  <Trash2 className="size-3" />
</span>

<span> elements are not keyboard-focusable and have no implicit ARIA role. Keyboard-only users cannot reach or activate this control. Replace with <button type="button">.


4. Settings NavLink has no accessible label — app-shell.tsx:83-95

The link renders only an icon (<Settings className="size-4" />). Screen readers will announce it as an unlabelled link. Add aria-label="Settings".


5. <nav> has no label — app-shell.tsx:100

When a page has multiple <nav> elements (or even just one), it should carry an aria-label so screen-reader users can distinguish it. Add aria-label="Main navigation".


🟡 Error Handling / UX

6. No AbortController for SSE streams — Chat.tsx:379-489, task-detail-modal.tsx:125-209

If the user navigates away while a message is generating (or a task is executing), the ReadableStream reader keeps the request alive indefinitely. Use AbortController and cancel on component unmount / session change.

const controller = new AbortController()
const res = await fetch('/api/chat/completions', { signal: controller.signal, ... })
// in cleanup: controller.abort()

7. Silent error swallowing for session load/delete — Chat.tsx:344, Chat.tsx:353

Both loadSession and deleteSession end with .catch(() => {}). Failed loads leave the UI in an inconsistent state (spinner gone, old messages shown); failed deletes leave the session in the sidebar. Show at minimum a toast or inline error.


8. Destructive delete has no confirmation — task-detail-modal.tsx:268-279

Clicking the Delete button immediately fires a DELETE /api/tasks/:id with no confirmation prompt. This is a common usability pitfall for irreversible actions. Add a confirmation step (window.confirm, an AlertDialog, or a two-step "click once to arm, click again to confirm" pattern).


🟡 Code Quality

9. console.error left in production code (9 occurrences)

Found in:

  • kanban-board.tsx:53, 163
  • task-detail-modal.tsx:262, 275
  • connectors.tsx:1069, 1072, 1076, 1106, 1112

These should be replaced with proper error state / user-visible feedback. In task-detail-modal.tsx, the save/delete errors are already caught but never surfaced to the user — only logged.


10. Array index used as key for streaming output — task-detail-modal.tsx:487

{executionOutput.map((entry, i) => (
  <div key={i} ...>

Since entries are only appended (never reordered or removed mid-stream) this is low-risk here, but it is still an anti-pattern. A stable key (e.g. index baked in at push time, or a counter) is cleaner.


11. Hardcoded pendingCount={3} in RootLayout.tsx

export default function RootLayout() {
  return <AppShell pendingCount={3} />
}

This is placeholder data. The badge on the Overview nav tab always shows "3" regardless of actual pending approvals. The count should be fetched or derived from real state.


🟢 Security (Low Risk / Informational)

12. rehype-sanitize is absent — markdown-content.tsx

react-markdown v10 does not render raw HTML by default (requires rehype-raw), so the current setup is not directly exploitable. However, adding rehype-sanitize as a defence-in-depth measure is recommended, especially since AI-generated content is being rendered and rehype-raw could be added later. This is informational, not an active vulnerability.


Summary

Severity Count
🔴 Bugs 2
🟠 Accessibility 3
🟡 Error handling / UX 4
🟡 Code quality 3
🟢 Security (informational) 1

The two bugs (#1 and #2) are the most important to fix before merging. The accessibility issues (#3#5) are straightforward to address. Everything else is good engineering hygiene.

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown

claude Bot commented Mar 19, 2026

Frontend Code Review — forge-fe/

Reviewed the following changed files: app-shell.tsx, kanban-board.tsx, task-detail-modal.tsx, Chat.tsx, types/index.ts


✅ Good Changes

  • app-shell.tsx: Adding aria-label=\"Settings\" to the NavLink is a good accessibility fix.
  • Chat.tsx: Replacing <span onClick> with <button type=\"button\"> for the delete action is the correct semantic change — spans are not keyboard-accessible or announced to screen readers.
  • Chat.tsx: Refactoring the SSE parser to declare event outside the try/catch block is cleaner — it prevents a malformed line from accidentally swallowing a real error event.
  • types/index.ts: ExecutionStatus discriminated union and TaskExecution interface are well-typed.

🔴 Issues — Must Fix

1. task-detail-modal.tsx — No cleanup when modal closes during execution

handleExecute starts a while(true) reader loop but the component has no useEffect cleanup to abort it when the modal closes (or the component unmounts). If the user closes the modal mid-execution, the reader loop continues in the background, holding the connection open and updating unmounted component state (React will warn about this).

Fix: Use an AbortController tied to the fetch and clean it up on unmount or modal close:

const abortRef = useRef<AbortController | null>(null)

// In handleExecute:
const controller = new AbortController()
abortRef.current = controller
const response = await fetch(`/api/tasks/${task.id}/execute`, {
  method: 'POST',
  signal: controller.signal,
})

// Add cleanup effect:
useEffect(() => {
  return () => { abortRef.current?.abort() }
}, [])

2. task-detail-modal.tsxhandleCancelExecution doesn't abort the in-flight reader

handleCancelExecution sends a POST to the cancel endpoint but does not abort the active fetch reader started in handleExecute. The while(true) loop continues streaming until the server closes the connection. In practice there could be a gap or the server response may not immediately propagate.

Fix: Share the AbortController from fix #1 — call abortRef.current?.abort() inside handleCancelExecution in addition to (or instead of) the cancel POST.


3. Chat.tsxevent.error as string unsafe cast

// line ~406
throw new Error(event.error as string)

If event.error is undefined or a non-string, this throws Error: undefined. The as string cast bypasses TypeScript's safety.

Fix:

throw new Error(String(event.error ?? 'Unknown error'))

🟡 Issues — Should Fix

4. task-detail-modal.tsxsetExecutionId written but never read

const [, setExecutionId] = useState<string | null>(null)

The execution ID is written on execution_start but never read back. It appears intended for use in handleCancelExecution (which currently identifies the execution by task ID via the server). Either:

  • Use the stored ID in the cancel request for precision (/api/tasks/${task.id}/execute/cancel works, but passing the execution ID would be more robust), or
  • Remove the state entirely if it's not needed.

5. task-detail-modal.tsx — Non-null assertions on stream parsing

const reader = response.body!.getReader()   // response.body could be null
buffer = lines.pop()!                        // pop() returns undefined when empty (though split always gives ≥1 element)

Fix:

if (!response.body) {
  setExecutionStatus('failed')
  return
}
const reader = response.body.getReader()
buffer = lines.pop() ?? ''

6. kanban-board.tsxuseRef sync pattern is an anti-pattern

const tasksRef = useRef(tasks)
useEffect(() => { tasksRef.current = tasks }, [tasks])
// used in handleDragEnd: persistReorder(tasksRef.current)

This pattern (syncing state to a ref to escape stale closures) is generally discouraged per React docs because it makes data flow harder to reason about and can lead to subtle bugs if the ref isn't updated synchronously before the event fires.

Preferred alternative: Pass tasks as a dependency to the useCallback for handleDragEnd, or extract the relevant derived value (e.g. reorder snapshot) at event-start inside the handler itself. The ref approach works but will be harder to maintain.


7. Chat.tsx — Multiple as string casts instead of type narrowing

event.session_id as string
event.token as string
event.error as string

event is typed as Record<string, unknown>, so these casts skip actual runtime type checking. A runtime check would catch malformed server payloads early:

if (event.type === 'session' && typeof event.session_id === 'string') { ... }

🟢 Minor Notes

  • task-detail-modal.tsx — execution output area: The output is rendered as plain text (not via react-markdown), so no XSS risk here. Good.
  • Execute/Cancel icon buttons: Consider adding aria-label to the Terminal and Square icon buttons in the execution panel (same pattern as the aria-label=\"Settings\" fix in this PR).
  • handleCancelExecution empty catch: The catch {} silently ignores cancel errors. At minimum, setting executionStatus to 'failed' on catch would give the user feedback.

Summary

Severity Count
🔴 Must Fix 3
🟡 Should Fix 4
🟢 Minor 3

The most important fix is #1 (missing abort on unmount) — it will cause a React warning in development and a real memory/connection leak in production. Issues #2 and #3 are also important for correctness.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants