fix: enhance terminal IPC handlers with error handling

- Wrapped terminal IPC handler functions (write, resize, kill) in try/catch blocks to log errors from node-pty, improving robustness against PTY failures.
- Updated the TeamAgentToolsInstaller to increment TOOL_VERSION to 10, reflecting changes in the tool's versioning.
- Refactored task comment handling in TeamAgentToolsInstaller to include retry logic for improved reliability during task updates.
- Consolidated role constants into a new teamRoles.ts file for better organization and reuse across components.
This commit is contained in:
iliya 2026-02-26 21:41:27 +02:00
parent 69f7cd333a
commit a9e0416251
9 changed files with 163 additions and 87 deletions

View file

@ -44,11 +44,28 @@ export function registerTerminalHandlers(ipcMain: IpcMain): void {
ipcMain.handle(TERMINAL_SPAWN, handleSpawn); ipcMain.handle(TERMINAL_SPAWN, handleSpawn);
// write, resize, kill are fire-and-forget (hot path, latency-sensitive) // write, resize, kill are fire-and-forget (hot path, latency-sensitive)
ipcMain.on(TERMINAL_WRITE, (_event, ptyId: string, data: string) => service.write(ptyId, data)); // Wrapped in try/catch: node-pty can throw if the PTY dies between Map.get() and .write()
ipcMain.on(TERMINAL_RESIZE, (_event, ptyId: string, cols: number, rows: number) => ipcMain.on(TERMINAL_WRITE, (_event, ptyId: string, data: string) => {
service.resize(ptyId, cols, rows) try {
); service.write(ptyId, data);
ipcMain.on(TERMINAL_KILL, (_event, ptyId: string) => service.kill(ptyId)); } catch (err) {
logger.warn('terminal:write error:', getErrorMessage(err));
}
});
ipcMain.on(TERMINAL_RESIZE, (_event, ptyId: string, cols: number, rows: number) => {
try {
service.resize(ptyId, cols, rows);
} catch (err) {
logger.warn('terminal:resize error:', getErrorMessage(err));
}
});
ipcMain.on(TERMINAL_KILL, (_event, ptyId: string) => {
try {
service.kill(ptyId);
} catch (err) {
logger.warn('terminal:kill error:', getErrorMessage(err));
}
});
logger.info('Terminal handlers registered'); logger.info('Terminal handlers registered');
} }

View file

@ -6,7 +6,7 @@ import * as path from 'path';
import { atomicWriteAsync } from './atomicWrite'; import { atomicWriteAsync } from './atomicWrite';
const TOOL_FILE_NAME = 'teamctl.js'; const TOOL_FILE_NAME = 'teamctl.js';
const TOOL_VERSION = 9; const TOOL_VERSION = 10;
function buildTeamCtlScript(): string { function buildTeamCtlScript(): string {
const script = String.raw`#!/usr/bin/env node const script = String.raw`#!/usr/bin/env node
@ -217,29 +217,43 @@ function addTaskComment(paths, taskId, flags) {
if (!text) die('Missing --text'); if (!text) die('Missing --text');
var from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'agent'; var from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'agent';
var ref = readTask(paths, taskId); var ref;
var task = ref.task; var task;
var taskPath = ref.taskPath; var taskPath;
var commentId;
var comment;
var existing;
var lastErr;
for (var attempt = 0; attempt < 8; attempt++) {
try {
ref = readTask(paths, taskId);
task = ref.task;
taskPath = ref.taskPath;
// Auto-clear needsClarification: "lead" when someone other than the task owner comments if (task.needsClarification === 'lead' && from !== task.owner) {
if (task.needsClarification === 'lead' && from !== task.owner) { delete task.needsClarification;
delete task.needsClarification; }
existing = Array.isArray(task.comments) ? task.comments : [];
commentId = crypto.randomUUID
? crypto.randomUUID()
: String(Date.now()) + '-' + String(Math.random());
comment = {
id: commentId,
author: from,
text: text,
createdAt: nowIso(),
};
task.comments = existing.concat([comment]);
writeTask(taskPath, task);
return { commentId: commentId, taskId: String(taskId), subject: task.subject, owner: task.owner };
} catch (e) {
lastErr = e;
if (attempt === 7) throw e;
}
} }
throw lastErr;
var existing = Array.isArray(task.comments) ? task.comments : [];
var commentId = crypto.randomUUID
? crypto.randomUUID()
: String(Date.now()) + '-' + String(Math.random());
var comment = {
id: commentId,
author: from,
text: text,
createdAt: nowIso(),
};
task.comments = existing.concat([comment]);
writeTask(taskPath, task);
return { commentId: commentId, taskId: String(taskId), subject: task.subject, owner: task.owner };
} }
function setNeedsClarification(paths, taskId, value) { function setNeedsClarification(paths, taskId, value) {
@ -309,25 +323,36 @@ function createTask(paths, flags) {
: undefined; : undefined;
ensureDir(paths.tasksDir); ensureDir(paths.tasksDir);
const nextId = getNextTaskId(paths);
const taskPath = path.join(paths.tasksDir, String(nextId) + '.json');
if (fs.existsSync(taskPath)) die('Task already exists: ' + String(nextId));
const from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : undefined; const from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : undefined;
let nextId;
const task = { let task;
id: nextId, let taskPath;
subject, while (true) {
description: String(description || subject), nextId = getNextTaskId(paths);
activeForm: activeForm ? String(activeForm) : undefined, taskPath = path.join(paths.tasksDir, String(nextId) + '.json');
owner, task = {
createdBy: from, id: nextId,
status, subject,
blocks: [], description: String(description || subject),
blockedBy: [], activeForm: activeForm ? String(activeForm) : undefined,
}; owner,
createdBy: from,
writeTask(taskPath, task); status,
blocks: [],
blockedBy: [],
};
try {
const fd = fs.openSync(taskPath, 'wx');
fs.closeSync(fd);
atomicWrite(taskPath, JSON.stringify(task, null, 2));
const verify = readJson(taskPath, null);
if (!verify) die('Task write verification failed');
break;
} catch (e) {
if (e && e.code === 'EEXIST') continue;
throw e;
}
}
updateHighwatermark(paths, nextId); updateHighwatermark(paths, nextId);
return task; return task;
} }

View file

@ -212,22 +212,47 @@ export const ActivityTimeline = ({
); );
})} })}
{hiddenCount > 0 && ( {hiddenCount > 0 && (
<div className="flex items-center gap-2 rounded-md border border-[var(--color-border)] bg-[var(--color-surface)] px-3 py-1.5"> <div className="relative flex justify-center pb-3 pt-1">
<span className="text-[11px] text-[var(--color-text-muted)]">+{hiddenCount} older</span> {/* Bottom-up shadow gradient: darkest at bottom edge, fades upward */}
<button <div
onClick={handleShowMore} className="pointer-events-none absolute inset-x-0 -top-24"
className="rounded px-2 py-0.5 text-[11px] font-medium text-[var(--color-text-secondary)] transition-colors hover:bg-[var(--color-surface-raised)] hover:text-[var(--color-text)]" style={{
bottom: '-1.6rem',
background:
'linear-gradient(to top, rgba(0, 0, 0, 0.4) 0%, rgba(0, 0, 0, 0.25) 25%, rgba(0, 0, 0, 0.1) 50%, rgba(0, 0, 0, 0.03) 75%, transparent 100%)',
}}
/>
<div
className="relative z-[1] flex items-center gap-3 rounded-full px-4 py-1.5"
style={{
backgroundColor: 'var(--color-surface-raised)',
boxShadow:
'0 0 12px 4px rgba(0, 0, 0, 0.3), 0 1px 3px rgba(0, 0, 0, 0.2), inset 0 1px 0 rgba(255, 255, 255, 0.04)',
border: '1px solid var(--color-border-emphasis)',
}}
> >
Show {Math.min(MESSAGES_PAGE_SIZE, hiddenCount)} more <span className="text-[11px] tabular-nums text-[var(--color-text-muted)]">
</button> +{hiddenCount} older
{hiddenCount > MESSAGES_PAGE_SIZE && ( </span>
<span className="h-3 w-px bg-[var(--color-border-emphasis)]" />
<button <button
onClick={handleShowAll} onClick={handleShowMore}
className="rounded px-2 py-0.5 text-[11px] text-[var(--color-text-muted)] transition-colors hover:bg-[var(--color-surface-raised)] hover:text-[var(--color-text-secondary)]" className="rounded-full px-2.5 py-0.5 text-[11px] font-medium text-[var(--color-text-secondary)] transition-all hover:bg-[rgba(255,255,255,0.08)] hover:text-[var(--color-text)]"
> >
Show all Show {Math.min(MESSAGES_PAGE_SIZE, hiddenCount)} more
</button> </button>
)} {hiddenCount > MESSAGES_PAGE_SIZE && (
<>
<span className="h-3 w-px bg-[var(--color-border-emphasis)]" />
<button
onClick={handleShowAll}
className="rounded-full px-2.5 py-0.5 text-[11px] text-[var(--color-text-muted)] transition-all hover:bg-[rgba(255,255,255,0.08)] hover:text-[var(--color-text-secondary)]"
>
Show all
</button>
</>
)}
</div>
</div> </div>
)} )}
</div> </div>

View file

@ -18,12 +18,9 @@ import {
SelectTrigger, SelectTrigger,
SelectValue, SelectValue,
} from '@renderer/components/ui/select'; } from '@renderer/components/ui/select';
import { CUSTOM_ROLE, NO_ROLE, PRESET_ROLES } from '@renderer/constants/teamRoles';
import { Loader2 } from 'lucide-react'; import { Loader2 } from 'lucide-react';
const PRESET_ROLES = ['lead', 'reviewer', 'developer', 'qa', 'researcher'] as const;
const CUSTOM_ROLE = '__custom__';
const NO_ROLE = '__none__';
const NAME_REGEX = /^[a-z0-9][a-z0-9-]*$/; const NAME_REGEX = /^[a-z0-9][a-z0-9-]*$/;
interface AddMemberDialogProps { interface AddMemberDialogProps {

View file

@ -86,9 +86,7 @@ interface ValidationResult {
}; };
} }
const PRESET_ROLES = ['lead', 'reviewer', 'developer', 'qa', 'researcher'] as const; import { CUSTOM_ROLE, NO_ROLE, PRESET_ROLES } from '@renderer/constants/teamRoles';
const CUSTOM_ROLE = '__custom__';
const NO_ROLE = '__none__';
const DEV_DEFAULT_TEAM = { const DEV_DEFAULT_TEAM = {
teamName: 'team-alpha', teamName: 'team-alpha',
description: 'Dev test team for provisioning flow', description: 'Dev test team for provisioning flow',

View file

@ -9,13 +9,9 @@ import {
SelectTrigger, SelectTrigger,
SelectValue, SelectValue,
} from '@renderer/components/ui/select'; } from '@renderer/components/ui/select';
import { CUSTOM_ROLE, FORBIDDEN_ROLES, NO_ROLE, PRESET_ROLES } from '@renderer/constants/teamRoles';
import { Check, Loader2, X } from 'lucide-react'; import { Check, Loader2, X } from 'lucide-react';
const ROLE_PRESETS = ['reviewer', 'developer', 'qa', 'researcher'] as const;
const FORBIDDEN_ROLES = new Set(['lead', 'team-lead']);
const NO_ROLE = '__none__';
const CUSTOM_ROLE = '__custom__';
interface MemberRoleEditorProps { interface MemberRoleEditorProps {
currentRole: string | undefined; currentRole: string | undefined;
onSave: (role: string | undefined) => Promise<void> | void; onSave: (role: string | undefined) => Promise<void> | void;
@ -29,7 +25,7 @@ export const MemberRoleEditor = ({
onCancel, onCancel,
saving, saving,
}: MemberRoleEditorProps): React.JSX.Element => { }: MemberRoleEditorProps): React.JSX.Element => {
const isPreset = currentRole && (ROLE_PRESETS as readonly string[]).includes(currentRole); const isPreset = currentRole && (PRESET_ROLES as readonly string[]).includes(currentRole);
const [selectValue, setSelectValue] = useState<string>( const [selectValue, setSelectValue] = useState<string>(
!currentRole ? NO_ROLE : isPreset ? currentRole : CUSTOM_ROLE !currentRole ? NO_ROLE : isPreset ? currentRole : CUSTOM_ROLE
); );
@ -75,7 +71,7 @@ export const MemberRoleEditor = ({
</SelectTrigger> </SelectTrigger>
<SelectContent> <SelectContent>
<SelectItem value={NO_ROLE}>No role</SelectItem> <SelectItem value={NO_ROLE}>No role</SelectItem>
{ROLE_PRESETS.map((r) => ( {PRESET_ROLES.map((r) => (
<SelectItem key={r} value={r}> <SelectItem key={r} value={r}>
{r} {r}
</SelectItem> </SelectItem>

View file

@ -48,6 +48,8 @@ interface CodeMirrorDiffViewProps {
onFullyViewed?: () => void; onFullyViewed?: () => void;
/** Ref to expose the EditorView for external navigation */ /** Ref to expose the EditorView for external navigation */
editorViewRef?: React.RefObject<EditorView | null>; editorViewRef?: React.RefObject<EditorView | null>;
/** Called whenever the internal EditorView is created or destroyed */
onViewChange?: (view: EditorView | null) => void;
/** Called when editor content changes (debounced, only when readOnly=false) */ /** Called when editor content changes (debounced, only when readOnly=false) */
onContentChanged?: (content: string) => void; onContentChanged?: (content: string) => void;
/** Cached EditorState to restore (preserves undo history between file switches) */ /** Cached EditorState to restore (preserves undo history between file switches) */
@ -293,6 +295,7 @@ export const CodeMirrorDiffView = ({
onHunkRejected, onHunkRejected,
onFullyViewed, onFullyViewed,
editorViewRef: externalViewRef, editorViewRef: externalViewRef,
onViewChange,
onContentChanged, onContentChanged,
initialState, initialState,
usePortionCollapse = false, usePortionCollapse = false,
@ -308,13 +311,15 @@ export const CodeMirrorDiffView = ({
const onAcceptRef = useRef(onHunkAccepted); const onAcceptRef = useRef(onHunkAccepted);
const onRejectRef = useRef(onHunkRejected); const onRejectRef = useRef(onHunkRejected);
const onContentChangedRef = useRef(onContentChanged); const onContentChangedRef = useRef(onContentChanged);
const onViewChangeRef = useRef(onViewChange);
const debounceTimer = useRef<ReturnType<typeof setTimeout>>(); const debounceTimer = useRef<ReturnType<typeof setTimeout>>();
useEffect(() => { useEffect(() => {
onAcceptRef.current = onHunkAccepted; onAcceptRef.current = onHunkAccepted;
onRejectRef.current = onHunkRejected; onRejectRef.current = onHunkRejected;
onContentChangedRef.current = onContentChanged; onContentChangedRef.current = onContentChanged;
onViewChangeRef.current = onViewChange;
externalViewRefHolder.current = externalViewRef; externalViewRefHolder.current = externalViewRef;
}, [onHunkAccepted, onHunkRejected, onContentChanged, externalViewRef]); }, [onHunkAccepted, onHunkRejected, onContentChanged, onViewChange, externalViewRef]);
// Auto-scroll to next chunk after accept/reject (deferred to let CM recalculate) // Auto-scroll to next chunk after accept/reject (deferred to let CM recalculate)
const scrollToNextChunk = useCallback(() => { const scrollToNextChunk = useCallback(() => {
@ -705,6 +710,8 @@ export const CodeMirrorDiffView = ({
if (extRef) { if (extRef) {
(extRef as React.MutableRefObject<EditorView | null>).current = view; (extRef as React.MutableRefObject<EditorView | null>).current = view;
} }
// Notify parent that a new EditorView was created
onViewChangeRef.current?.(view);
return () => { return () => {
clearTimeout(debounceTimer.current); clearTimeout(debounceTimer.current);
@ -713,6 +720,8 @@ export const CodeMirrorDiffView = ({
if (extRef) { if (extRef) {
(extRef as React.MutableRefObject<EditorView | null>).current = null; (extRef as React.MutableRefObject<EditorView | null>).current = null;
} }
// Notify parent that the EditorView was destroyed
onViewChangeRef.current?.(null);
}; };
// We intentionally rebuild the entire editor when key props change // We intentionally rebuild the entire editor when key props change
}, [original, modified, buildExtensions, initialState]); }, [original, modified, buildExtensions, initialState]);
@ -747,7 +756,7 @@ export const CodeMirrorDiffView = ({
return () => { return () => {
cancelled = true; cancelled = true;
}; };
}, [fileName, buildExtensions, initialState]); }, [fileName, buildExtensions, initialState, original, modified]);
// Dynamic collapse toggle — reconfigure compartments in-place, preserving undo history // Dynamic collapse toggle — reconfigure compartments in-place, preserving undo history
useEffect(() => { useEffect(() => {

View file

@ -1,4 +1,4 @@
import React, { useEffect, useRef } from 'react'; import React, { useCallback, useEffect, useRef } from 'react';
import { CodeMirrorDiffView } from './CodeMirrorDiffView'; import { CodeMirrorDiffView } from './CodeMirrorDiffView';
import { DiffErrorBoundary } from './DiffErrorBoundary'; import { DiffErrorBoundary } from './DiffErrorBoundary';
@ -41,19 +41,16 @@ export const FileSectionDiff = ({
const localEditorViewRef = useRef<EditorView | null>(null); const localEditorViewRef = useRef<EditorView | null>(null);
const sentinelRef = useRef<HTMLDivElement>(null); const sentinelRef = useRef<HTMLDivElement>(null);
// Register/unregister EditorView with parent Map // Notify parent whenever CodeMirrorDiffView creates or destroys its EditorView.
useEffect(() => { // This fires on every editor lifecycle event: initial mount, key-change remount,
return () => { // and internal recreation (e.g. when `modified` prop changes after Save).
onEditorViewReady(file.filePath, null); const handleViewChange = useCallback(
}; (view: EditorView | null) => {
}, [file.filePath, onEditorViewReady]); localEditorViewRef.current = view;
onEditorViewReady(file.filePath, view);
// Sync EditorView ref to parent after CM creates the view },
useEffect(() => { [file.filePath, onEditorViewReady]
if (localEditorViewRef.current) { );
onEditorViewReady(file.filePath, localEditorViewRef.current);
}
}, [file.filePath, onEditorViewReady, discardCounter]);
// Auto-viewed sentinel observer // Auto-viewed sentinel observer
useEffect(() => { useEffect(() => {
@ -124,6 +121,7 @@ export const FileSectionDiff = ({
onHunkRejected={(idx) => onHunkRejected(file.filePath, idx)} onHunkRejected={(idx) => onHunkRejected(file.filePath, idx)}
onContentChanged={(content) => onContentChanged(file.filePath, content)} onContentChanged={(content) => onContentChanged(file.filePath, content)}
editorViewRef={localEditorViewRef} editorViewRef={localEditorViewRef}
onViewChange={handleViewChange}
/> />
</DiffErrorBoundary> </DiffErrorBoundary>
<div ref={sentinelRef} className="h-1 shrink-0" /> <div ref={sentinelRef} className="h-1 shrink-0" />

View file

@ -0,0 +1,11 @@
/** Preset role options shown in role selectors (Add Member, Create Team, Role Editor). */
export const PRESET_ROLES = ['reviewer', 'developer', 'qa', 'researcher'] as const;
/** Sentinel value for "custom role" in select dropdowns. */
export const CUSTOM_ROLE = '__custom__';
/** Sentinel value for "no role" in select dropdowns. */
export const NO_ROLE = '__none__';
/** Roles that cannot be assigned manually (reserved for system use). */
export const FORBIDDEN_ROLES = new Set(['lead', 'team-lead']);