feat: enhance team name validation and UI interactions

- Updated TEAM_NAME_PATTERN to enforce kebab-case naming conventions with lowercase letters and hyphens.
- Introduced a sanitizeTeamName function to ensure team names are properly formatted before submission.
- Enhanced CreateTeamDialog to provide real-time feedback on team name validity and display sanitized names.
- Modified NewProjectCard to open the teams tab upon navigating to a matching worktree.
- Updated TeamListView header to clarify the selection process for teams.
This commit is contained in:
iliya 2026-03-01 19:41:19 +02:00
parent 47d979a43d
commit a0764e39d1
10 changed files with 200 additions and 57 deletions

View file

@ -12,7 +12,7 @@ const SESSION_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const SUBAGENT_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const NOTIFICATION_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const TRIGGER_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const TEAM_NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const TEAM_NAME_PATTERN = /^[a-z0-9][a-z0-9-]{0,127}$/;
const TASK_ID_PATTERN = /^\d{1,10}$/;
const MEMBER_NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
const FROM_FIELD_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;

View file

@ -371,10 +371,11 @@ function findMatchingWorktree(
}
const NewProjectCard = (): React.JSX.Element => {
const { repositoryGroups, fetchRepositoryGroups } = useStore(
const { repositoryGroups, fetchRepositoryGroups, openTeamsTab } = useStore(
useShallow((s) => ({
repositoryGroups: s.repositoryGroups,
fetchRepositoryGroups: s.fetchRepositoryGroups,
openTeamsTab: s.openTeamsTab,
}))
);
@ -396,6 +397,7 @@ const NewProjectCard = (): React.JSX.Element => {
const match = findMatchingWorktree(repositoryGroups, selectedPath);
if (match) {
navigateToMatch(match);
openTeamsTab();
return;
}
@ -405,6 +407,7 @@ const NewProjectCard = (): React.JSX.Element => {
const matchAfterRefresh = findMatchingWorktree(refreshedGroups, selectedPath);
if (matchAfterRefresh) {
navigateToMatch(matchAfterRefresh);
openTeamsTab();
return;
}
@ -437,6 +440,7 @@ const NewProjectCard = (): React.JSX.Element => {
repositoryGroups: [syntheticGroup, ...state.repositoryGroups],
}));
navigateToMatch({ repoId: encodedId, worktreeId: encodedId });
openTeamsTab();
} catch (error) {
logger.error('Error selecting folder:', error);
}

View file

@ -541,7 +541,7 @@ export const TeamListView = (): React.JSX.Element => {
const renderHeader = (): React.JSX.Element => (
<div className="mb-4">
<div className="flex items-center justify-between">
<h2 className="text-base font-semibold text-[var(--color-text)]">Teams</h2>
<h2 className="text-base font-semibold text-[var(--color-text)]">Select Team</h2>
<div className="flex items-center gap-2">
<Button
variant="outline"

View file

@ -142,15 +142,27 @@ function buildMembers(members: MemberDraft[]): TeamCreateRequest['members'] {
.filter((member): member is NonNullable<typeof member> => member !== null);
}
// eslint-disable-next-line security/detect-unsafe-regex -- kebab-case pattern is linear, no ReDoS
const TEAM_NAME_RE = /^[a-z0-9]+(?:-[a-z0-9]+)*$/;
/** Mirrors Claude CLI's `zuA()` sanitization: non-alphanumeric → `-`, then lowercase. */
function sanitizeTeamName(name: string): string {
return name
.replace(/[^a-zA-Z0-9]/g, '-')
.replace(/-{2,}/g, '-')
.replace(/^-+/g, '')
.replace(/-+$/g, '')
.toLowerCase();
}
const MEMBER_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/;
function validateTeamNameInline(name: string): string | null {
const trimmed = name.trim();
if (!trimmed) return null;
if (!TEAM_NAME_RE.test(trimmed) || trimmed.length > 64) {
return 'Use kebab-case [a-z0-9-], max 64 chars';
const sanitized = sanitizeTeamName(trimmed);
if (!sanitized) {
return 'Name must contain at least one letter or digit';
}
if (sanitized.length > 128) {
return 'Name is too long (max 128 chars)';
}
return null;
}
@ -169,11 +181,20 @@ function validateRequest(
options?: { requireCwd?: boolean }
): ValidationResult {
const requireCwd = options?.requireCwd ?? true;
if (!TEAM_NAME_RE.test(request.teamName) || request.teamName.length > 64) {
const sanitized = sanitizeTeamName(request.teamName);
if (!sanitized) {
return {
valid: false,
errors: {
teamName: 'Use kebab-case [a-z0-9-], max 64 chars',
teamName: 'Name must contain at least one letter or digit',
},
};
}
if (sanitized.length > 128) {
return {
valid: false,
errors: {
teamName: 'Name is too long (max 128 chars)',
},
};
}
@ -519,9 +540,11 @@ export const CreateTeamDialog = ({
const effectiveModel =
selectedModel && selectedModel !== '__default__' ? selectedModel : undefined;
const sanitizedTeamName = sanitizeTeamName(teamName.trim());
const request = useMemo<TeamCreateRequest>(
() => ({
teamName: teamName.trim(),
teamName: sanitizedTeamName,
description: description.trim() || undefined,
color: teamColor || undefined,
members: buildMembers(members),
@ -529,7 +552,7 @@ export const CreateTeamDialog = ({
prompt: prompt.trim() || undefined,
model: effectiveModel,
}),
[teamName, description, teamColor, members, effectiveCwd, prompt, effectiveModel]
[sanitizedTeamName, description, teamColor, members, effectiveCwd, prompt, effectiveModel]
);
const activeError = localError ?? provisioningError;
@ -576,7 +599,7 @@ export const CreateTeamDialog = ({
};
const handleSubmit = (): void => {
if (existingTeamNames.includes(request.teamName)) {
if (existingTeamNames.includes(sanitizedTeamName)) {
setFieldErrors({ teamName: 'Team name already exists' });
setLocalError('Check form fields');
return;
@ -710,13 +733,18 @@ export const CreateTeamDialog = ({
onChange={(event) => setTeamName(event.target.value)}
placeholder="team-alpha"
/>
{existingTeamNames.includes(teamName.trim()) ? (
{existingTeamNames.includes(sanitizedTeamName) ? (
<p className="text-[11px] text-red-300">Team name already exists</p>
) : validateTeamNameInline(teamName) ? (
<p className="text-[11px] text-red-300">{validateTeamNameInline(teamName)}</p>
) : fieldErrors.teamName ? (
<p className="text-[11px] text-red-300">{fieldErrors.teamName}</p>
) : null}
{sanitizedTeamName && sanitizedTeamName !== teamName.trim() ? (
<p className="text-[11px] text-[var(--color-text-muted)]">
On disk: <span className="font-mono">{sanitizedTeamName}</span>
</p>
) : null}
</div>
<div className="space-y-1.5 md:col-span-2">

View file

@ -5,7 +5,7 @@
* Loads ALL project files via backend API on mount (not limited to expanded dirs).
*/
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { useStore } from '@renderer/store';
import { Command } from 'cmdk';

View file

@ -1,11 +1,11 @@
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { undo } from '@codemirror/commands';
import { goToNextChunk, rejectChunk } from '@codemirror/merge';
import { rejectChunk } from '@codemirror/merge';
import { isElectronMode } from '@renderer/api';
import { EditorSelectionMenu } from '@renderer/components/team/editor/EditorSelectionMenu';
import { useContinuousScrollNav } from '@renderer/hooks/useContinuousScrollNav';
import { isLastChunkInFile, useDiffNavigation } from '@renderer/hooks/useDiffNavigation';
import { useDiffNavigation } from '@renderer/hooks/useDiffNavigation';
import { useViewedFiles } from '@renderer/hooks/useViewedFiles';
import { cn } from '@renderer/lib/utils';
import { useStore } from '@renderer/store';
@ -69,6 +69,7 @@ export const ChangeReviewDialog = ({
applying,
applyError,
setHunkDecision,
clearHunkDecisionByOriginalIndex,
setCollapseUnchanged,
fetchFileContent,
acceptAllFile,
@ -108,6 +109,11 @@ export const ChangeReviewDialog = ({
const scrollContainerRef = useRef<HTMLDivElement>(null);
// Last focused CM editor — for Cmd+Z outside editor
const lastFocusedEditorRef = useRef<EditorView | null>(null);
// Timestamp of last bulk accept/reject-all operation (for Ctrl/Cmd+Z UX)
const lastBulkActionAtRef = useRef<number>(0);
// Track recent per-hunk actions so Ctrl/Cmd+Z can clear persisted decisions (reopen-safe)
const lastHunkActionAtRef = useRef<Record<string, number>>({});
const hunkDecisionUndoStackRef = useRef<Record<string, number[]>>({});
// Proxy ref for useDiffNavigation (points to active file's editor)
const activeEditorViewRef = useRef<EditorView | null>(null);
@ -171,6 +177,7 @@ export const ChangeReviewDialog = ({
const handleAcceptAll = useCallback(() => {
if (!activeChangeSet) return;
pushReviewUndoSnapshot();
lastBulkActionAtRef.current = Date.now();
for (const file of activeChangeSet.files) {
acceptAllFile(file.filePath);
}
@ -184,6 +191,7 @@ export const ChangeReviewDialog = ({
const handleRejectAll = useCallback(() => {
if (!activeChangeSet) return;
pushReviewUndoSnapshot();
lastBulkActionAtRef.current = Date.now();
for (const file of activeChangeSet.files) {
rejectAllFile(file.filePath);
}
@ -197,14 +205,18 @@ export const ChangeReviewDialog = ({
// Per-file callbacks for ContinuousScrollView
const handleHunkAccepted = useCallback(
(filePath: string, hunkIndex: number) => {
setHunkDecision(filePath, hunkIndex, 'accepted');
const originalIndex = setHunkDecision(filePath, hunkIndex, 'accepted');
lastHunkActionAtRef.current[filePath] = Date.now();
(hunkDecisionUndoStackRef.current[filePath] ??= []).push(originalIndex);
},
[setHunkDecision]
);
const handleHunkRejected = useCallback(
(filePath: string, hunkIndex: number) => {
setHunkDecision(filePath, hunkIndex, 'rejected');
const originalIndex = setHunkDecision(filePath, hunkIndex, 'rejected');
lastHunkActionAtRef.current[filePath] = Date.now();
(hunkDecisionUndoStackRef.current[filePath] ??= []).push(originalIndex);
if (REVIEW_INSTANT_APPLY) {
void applySingleFileDecision(teamName, filePath, taskId, memberName);
}
@ -422,7 +434,8 @@ export const ChangeReviewDialog = ({
});
}, [activeChangeSet, initialFilePath, scrollToFile]);
// Clear selection state on close + cleanup timer
// Clear selection state on close + cleanup timer.
// setState here is intentional: reset transient UI state when dialog closes.
useEffect(() => {
if (!open) {
setSelectionInfo(null);
@ -469,32 +482,55 @@ export const ChangeReviewDialog = ({
if (!open) return;
const handler = (e: KeyboardEvent): void => {
if ((e.metaKey || e.ctrlKey) && e.code === 'KeyZ' && !e.shiftKey) {
// Don't intercept if focus is inside a CM editor — let CM handle its own undo
if (document.activeElement?.closest('.cm-editor')) return;
// Don't intercept native undo in input/textarea
const tag = document.activeElement?.tagName;
if (tag === 'INPUT' || tag === 'TEXTAREA') return;
// Try to undo in the last focused CM editor
// Prefer bulk undo (Accept All / Reject All) shortly after bulk action,
// even if focus is inside a CM editor (focus often remains there after clicking buttons).
const now = Date.now();
const bulkRecently = now - lastBulkActionAtRef.current < 10_000;
if (bulkRecently && useStore.getState().reviewUndoStack.length > 0) {
e.preventDefault();
e.stopPropagation();
handleUndoBulk();
return;
}
// If the last action was a hunk keep/undo (accept/reject) and we're undoing immediately,
// we must also clear the persisted decision. Otherwise reopening the dialog will replay it.
if (document.activeElement?.closest('.cm-editor')) {
const lastView = lastFocusedEditorRef.current;
const fp = activeFilePathRef.current;
const stack = fp ? hunkDecisionUndoStackRef.current[fp] : undefined;
const lastAt = fp ? (lastHunkActionAtRef.current[fp] ?? 0) : 0;
const hunkRecently = fp ? now - lastAt < 5_000 : false;
if (fp && stack && stack.length > 0 && hunkRecently && lastView?.dom.isConnected) {
e.preventDefault();
e.stopPropagation();
undo(lastView);
const originalIndex = stack.pop()!;
clearHunkDecisionByOriginalIndex(fp, originalIndex);
return;
}
// Otherwise, let CM handle its own undo
return;
}
// Otherwise try to undo in the last focused CM editor
const lastView = lastFocusedEditorRef.current;
if (lastView?.dom.isConnected) {
e.preventDefault();
e.stopPropagation();
undo(lastView);
return;
}
// Fall back to bulk undo (Accept All / Reject All)
if (useStore.getState().reviewUndoStack.length > 0) {
e.preventDefault();
e.stopPropagation();
handleUndoBulk();
}
}
};
document.addEventListener('keydown', handler, true);
return () => document.removeEventListener('keydown', handler, true);
}, [open, handleUndoBulk]);
}, [open, handleUndoBulk, clearHunkDecisionByOriginalIndex]);
// Cmd+N IPC listener (forwarded from main process)
useEffect(() => {

View file

@ -281,29 +281,53 @@ const portionCollapseTheme = EditorView.theme({
// CM recognizes it's the same field → keeps accumulated state (expanded regions) → no create() call.
// Config is read from portionCollapseConfigFacet (controlled by compartment).
const portionCollapseField = StateField.define<DecorationSet>({
create(state: EditorState): DecorationSet {
interface PortionCollapseState {
decorations: DecorationSet;
/**
* Once the user expands everything (i.e. removes the last collapse widget),
* we should NOT re-initialize collapse on harmless transactions like cursor moves.
*/
userExpandedAll: boolean;
}
const portionCollapseField = StateField.define<PortionCollapseState>({
create(state: EditorState): PortionCollapseState {
const cfg = state.facet(portionCollapseConfigFacet);
return buildPortionRanges(state, cfg.margin, cfg.minSize, cfg.portionSize);
return {
decorations: buildPortionRanges(state, cfg.margin, cfg.minSize, cfg.portionSize),
userExpandedAll: false,
};
},
update(deco: DecorationSet, tr: Transaction): DecorationSet {
update(value: PortionCollapseState, tr: Transaction): PortionCollapseState {
const cfg = tr.state.facet(portionCollapseConfigFacet);
// 1. Expand effects
let result = deco;
let nextDeco = value.decorations;
let userExpandedAll = value.userExpandedAll;
let hasExpandEffect = false;
for (const effect of tr.effects) {
if (effect.is(expandPortion)) {
hasExpandEffect = true;
result = handleExpandPortion(result, effect.value, tr.state, cfg.minSize, cfg.portionSize);
nextDeco = handleExpandPortion(
nextDeco,
effect.value,
tr.state,
cfg.minSize,
cfg.portionSize
);
}
if (effect.is(expandAllAtPos)) {
hasExpandEffect = true;
result = handleExpandAll(result, effect.value);
nextDeco = handleExpandAll(nextDeco, effect.value);
}
}
if (hasExpandEffect) return result;
if (hasExpandEffect) {
if (nextDeco === Decoration.none) {
userExpandedAll = true;
}
return { decorations: nextDeco, userExpandedAll };
}
// 2. Accept chunk (updateOriginalDoc) — editor doc unchanged, keep decorations.
// Full rebuild here would destroy user's expanded state (Expand All / Expand N).
@ -311,27 +335,47 @@ const portionCollapseField = StateField.define<DecorationSet>({
// When mirrorEditsAfterResolve adds updateOriginalDoc to a docChanged transaction,
// we must NOT short-circuit — fall through to docChanged handler for proper rebuild.
if (tr.effects.some((e) => e.is(updateOriginalDoc)) && !tr.docChanged) {
return deco;
return value;
}
// 3. Document changed (reject, user edit) → full rebuild
// 3. Document changed (reject, user edit)
//
// Rebuilding from scratch here causes a bad UX: after the user expands (or when a hunk is
// applied) the editor can suddenly re-collapse unchanged regions, hiding the code the user
// was actively looking at. Instead, keep the user's current collapsed/expanded state stable
// by mapping existing decorations through the document changes.
if (tr.docChanged) {
return buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize);
const mapped = value.decorations.map(tr.changes);
// If we previously had no collapse decoration but chunks are now available, initialize once.
// BUT: if the user explicitly expanded everything, never re-collapse automatically.
if (!value.userExpandedAll && value.decorations === Decoration.none) {
const chunks = getChunks(tr.state);
if (chunks) {
return {
decorations: buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize),
userExpandedAll: false,
};
}
}
return { decorations: mapped, userExpandedAll: value.userExpandedAll };
}
// 4. Lazy init
if (deco === Decoration.none) {
if (!value.userExpandedAll && value.decorations === Decoration.none) {
const chunks = getChunks(tr.state);
if (chunks) {
return buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize);
return {
decorations: buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize),
userExpandedAll: false,
};
}
}
return deco;
return value;
},
provide(f) {
return EditorView.decorations.from(f);
return EditorView.decorations.from(f, (v) => v.decorations);
},
});

View file

@ -264,14 +264,18 @@ export function useDiffNavigation(
}
}, [selectedFilePath, currentHunkIndex, onHunkRejected]);
// Store refs for stable closure
// Store refs for stable closure (avoids re-registering keydown on every render)
const onCloseRef = useRef(onClose);
const onSaveFileRef = useRef(onSaveFile);
const onHunkAcceptedRef = useRef(onHunkAccepted);
const selectedFilePathRef = useRef(selectedFilePath);
useEffect(() => {
onCloseRef.current = onClose;
onSaveFileRef.current = onSaveFile;
}, [onClose, onSaveFile]);
onHunkAcceptedRef.current = onHunkAccepted;
selectedFilePathRef.current = selectedFilePath;
}, [onClose, onSaveFile, onHunkAccepted, selectedFilePath]);
// Keyboard handler
useEffect(() => {
@ -327,11 +331,14 @@ export function useDiffNavigation(
event.preventDefault();
const view = getActiveEditorView(editorViewRef, continuousOptionsRef.current);
if (view) {
const filePath = getActiveFilePath(selectedFilePath, continuousOptionsRef.current);
if (filePath && onHunkAccepted) {
const filePath = getActiveFilePath(
selectedFilePathRef.current,
continuousOptionsRef.current
);
if (filePath && onHunkAcceptedRef.current) {
const cursorPos = view.state.selection.main.head;
const idx = computeChunkIndexAtPos(view.state, cursorPos);
onHunkAccepted(filePath, idx);
onHunkAcceptedRef.current(filePath, idx);
}
acceptChunk(view);
requestAnimationFrame(() => goToNextHunk());

View file

@ -94,7 +94,13 @@ export interface ChangeReviewSlice {
clearDecisionsFromDisk: (teamName: string, scopeKey: string) => Promise<void>;
// Phase 2 actions
setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => void;
/**
* Set decision for a hunk at the current (visible) CM index.
* Returns the stable/original hunk index used as the decision key.
*/
setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => number;
/** Clear a persisted decision using the stable/original hunk index */
clearHunkDecisionByOriginalIndex: (filePath: string, originalIndex: number) => void;
setFileDecision: (filePath: string, decision: HunkDecision) => void;
setFileChunkCount: (filePath: string, count: number) => void;
pushReviewUndoSnapshot: () => void;
@ -429,6 +435,17 @@ export const createChangeReviewSlice: StateCreator<AppState, [], [], ChangeRevie
set((s) => ({
hunkDecisions: { ...s.hunkDecisions, [key]: decision },
}));
return originalIndex;
},
clearHunkDecisionByOriginalIndex: (filePath: string, originalIndex: number) => {
const key = `${filePath}:${originalIndex}`;
set((s) => {
if (!(key in s.hunkDecisions)) return s;
const next = { ...s.hunkDecisions };
delete next[key];
return { hunkDecisions: next };
});
},
setFileDecision: (filePath: string, decision: HunkDecision) => {
@ -727,14 +744,14 @@ export const createChangeReviewSlice: StateCreator<AppState, [], [], ChangeRevie
try {
const content = fileContents[filePath];
const baseCount = getFileHunkCount(filePath, file.snippets.length, fileChunkCounts);
const maxIdx = getMaxDecisionIndexForFile(filePath, hunkDecisions);
const innerBaseCount = getFileHunkCount(filePath, file.snippets.length, fileChunkCounts);
const innerMaxIdx = getMaxDecisionIndexForFile(filePath, hunkDecisions);
const hunkContextHashes =
maxIdx < baseCount
innerMaxIdx < innerBaseCount
? buildHunkContextHashesForFile(
content?.originalFullContent,
content?.modifiedFullContent,
baseCount
innerBaseCount
)
: undefined;
await api.review.applyDecisions({

View file

@ -13,12 +13,19 @@ export class IpcError extends Error {
}
}
/** Error messages that represent expected transient states, not real failures. */
const EXPECTED_IPC_SIGNALS = ['TEAM_PROVISIONING'];
export async function unwrapIpc<T>(operation: string, fn: () => Promise<T>): Promise<T> {
try {
return await fn();
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
logger.error(`[${operation}] ${message}`);
if (EXPECTED_IPC_SIGNALS.some((sig) => message.includes(sig))) {
logger.debug(`[${operation}] ${message}`);
} else {
logger.error(`[${operation}] ${message}`);
}
throw new IpcError(operation, message, error);
}
}