From 2b9d6034c8a6877dfe30f57ffe0003c667d2821e Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Wed, 22 Jul 2026 17:28:58 +0530 Subject: [PATCH 1/2] refactor: Automatic Safe-Name Conversion at Naming Entry Points --- app/backend/api/sessions.go | 9 +- app/backend/api/sessions_test.go | 74 +++++++ app/backend/api/windows.go | 6 +- app/backend/api/windows_test.go | 39 ++++ app/backend/internal/validate/validate.go | 20 ++ .../internal/validate/validate_test.go | 56 ++++++ app/frontend/src/app.tsx | 18 +- .../src/components/create-session-dialog.tsx | 38 ++-- .../src/components/host-overview-page.tsx | 9 +- app/frontend/src/components/sidebar.test.tsx | 13 +- app/frontend/src/components/sidebar/index.tsx | 8 +- .../src/components/sidebar/session-row.tsx | 3 +- .../src/components/sidebar/window-row.tsx | 3 +- .../src/components/spawn-agent-dialog.tsx | 10 +- app/frontend/src/components/top-bar.test.tsx | 15 ++ app/frontend/src/components/top-bar.tsx | 11 +- app/frontend/src/hooks/use-dialog-state.ts | 7 +- app/frontend/src/lib/names.test.ts | 111 +++++++++++ app/frontend/src/lib/names.ts | 92 +++++++++ app/frontend/tests/e2e/sync-latency.spec.md | 10 +- app/frontend/tests/e2e/sync-latency.spec.ts | 12 +- app/frontend/tests/e2e/window-heading.spec.md | 16 ++ app/frontend/tests/e2e/window-heading.spec.ts | 31 +++ docs/memory/run-kit/architecture.md | 8 +- docs/memory/run-kit/tmux-sessions.md | 23 ++- docs/memory/run-kit/ui-patterns.md | 55 ++++- .../.history.jsonl | 12 ++ .../.status.yaml | 54 +++++ .../intake.md | 105 ++++++++++ .../plan.md | 188 ++++++++++++++++++ 30 files changed, 990 insertions(+), 66 deletions(-) create mode 100644 app/frontend/src/lib/names.test.ts create mode 100644 app/frontend/src/lib/names.ts create mode 100644 fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl create mode 100644 fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml create mode 100644 fab/changes/260722-ln4n-auto-safe-name-conversion/intake.md create mode 100644 fab/changes/260722-ln4n-auto-safe-name-conversion/plan.md diff --git a/app/backend/api/sessions.go b/app/backend/api/sessions.go index 74777c7b..6b35646f 100644 --- a/app/backend/api/sessions.go +++ b/app/backend/api/sessions.go @@ -28,7 +28,8 @@ func (s *Server) handleSessionCreate(w http.ResponseWriter, r *http.Request) { return } - if errMsg := validate.ValidateName(body.Name, "Session name"); errMsg != "" { + // Tightened new-name rule (no spaces) — this names a to-be-created session. + if errMsg := validate.ValidateNewName(body.Name, "Session name"); errMsg != "" { writeError(w, http.StatusBadRequest, errMsg) return } @@ -70,7 +71,11 @@ func (s *Server) handleSessionRename(w http.ResponseWriter, r *http.Request) { return } - if errMsg := validate.ValidateName(body.Name, "Session name"); errMsg != "" { + // Tightened new-name rule (no spaces) — this is the renamed-TO name. The + // URL-param `session` above stays on the permissive ValidateName so a + // pre-existing spacey session (created outside run-kit) can still be the + // rename SOURCE. + if errMsg := validate.ValidateNewName(body.Name, "Session name"); errMsg != "" { writeError(w, http.StatusBadRequest, errMsg) return } diff --git a/app/backend/api/sessions_test.go b/app/backend/api/sessions_test.go index b00553d9..5dab45d0 100644 --- a/app/backend/api/sessions_test.go +++ b/app/backend/api/sessions_test.go @@ -703,6 +703,80 @@ func TestSessionCreateForbiddenChars(t *testing.T) { } } +func TestSessionCreateSpaceRejected(t *testing.T) { + // The tightened new-name rule (validate.ValidateNewName): a to-be-created + // session name with a space is rejected — spaces bite tmux CLI targeting, + // session-group naming, and the /$server/$window routes. + ops := &mockTmuxOps{} + router := newTestRouter(&mockSessionFetcher{}, ops) + + body := `{"name":"My problem"}` + req := httptest.NewRequest(http.MethodPost, "/api/sessions", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } + if ops.createSessionCalled { + t.Error("CreateSession must NOT be called for a spacey name") + } + var result map[string]string + if err := json.NewDecoder(rec.Body).Decode(&result); err != nil { + t.Fatalf("decode error: %v", err) + } + if !strings.Contains(result["error"], "cannot contain spaces") { + t.Errorf("error = %q, want containing %q", result["error"], "cannot contain spaces") + } +} + +func TestSessionRenameSpaceyNewNameRejected(t *testing.T) { + // The renamed-TO name is held to the tightened rule. + ops := &mockTmuxOps{} + router := newTestRouter(&mockSessionFetcher{}, ops) + + body := `{"name":"My problem"}` + req := httptest.NewRequest(http.MethodPost, "/api/sessions/main/rename", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } + if ops.renameSessionCalled { + t.Error("RenameSession must NOT be called for a spacey new name") + } +} + +func TestSessionRenameSpaceyOldNameAccepted(t *testing.T) { + // The rename SOURCE (URL param) stays on the permissive ValidateName so a + // pre-existing spacey session (created outside run-kit via raw tmux) can + // still be renamed TO a safe name. + ops := &mockTmuxOps{} + router := newTestRouter(&mockSessionFetcher{}, ops) + + body := `{"name":"My_problem"}` + req := httptest.NewRequest(http.MethodPost, "/api/sessions/My%20problem/rename", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + if !ops.renameSessionCalled { + t.Fatal("RenameSession was not called") + } + if ops.renameSessionSession != "My problem" { + t.Errorf("old name = %q, want %q", ops.renameSessionSession, "My problem") + } + if ops.renameSessionName != "My_problem" { + t.Errorf("new name = %q, want %q", ops.renameSessionName, "My_problem") + } +} + func TestSessionCreateInvalidJSON(t *testing.T) { router := newTestRouter(&mockSessionFetcher{}, &mockTmuxOps{}) diff --git a/app/backend/api/windows.go b/app/backend/api/windows.go index 8d12906a..b91bdc17 100644 --- a/app/backend/api/windows.go +++ b/app/backend/api/windows.go @@ -37,7 +37,8 @@ func (s *Server) handleWindowCreate(w http.ResponseWriter, r *http.Request) { // in the embedded configs). Only a non-empty name is validated. The rename // path (handleWindowRename) still requires a non-empty, validated name. if body.Name != "" { - if errMsg := validate.ValidateName(body.Name, "Window name"); errMsg != "" { + // Tightened new-name rule (no spaces) — this names a to-be-created window. + if errMsg := validate.ValidateNewName(body.Name, "Window name"); errMsg != "" { writeError(w, http.StatusBadRequest, errMsg) return } @@ -172,7 +173,8 @@ func (s *Server) handleWindowRename(w http.ResponseWriter, r *http.Request) { return } - if errMsg := validate.ValidateName(body.Name, "Window name"); errMsg != "" { + // Tightened new-name rule (no spaces) — this is the renamed-TO name. + if errMsg := validate.ValidateNewName(body.Name, "Window name"); errMsg != "" { writeError(w, http.StatusBadRequest, errMsg) return } diff --git a/app/backend/api/windows_test.go b/app/backend/api/windows_test.go index ff5000fe..55302c15 100644 --- a/app/backend/api/windows_test.go +++ b/app/backend/api/windows_test.go @@ -487,6 +487,26 @@ func TestWindowCreateNonEmptyInvalidNameRejected(t *testing.T) { } } +func TestWindowCreateSpaceyNameRejected(t *testing.T) { + // The tightened new-name rule (validate.ValidateNewName) applies to a + // non-empty to-be-created window name. + ops := &mockTmuxOps{} + router := newTestRouter(&mockSessionFetcher{}, ops) + + body := `{"name":"my window"}` + req := httptest.NewRequest(http.MethodPost, "/api/sessions/run-kit/windows", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } + if ops.createWindowCalled { + t.Error("CreateWindow must NOT be called for a spacey name") + } +} + func TestWindowKill(t *testing.T) { ops := &mockTmuxOps{} router := newTestRouter(&mockSessionFetcher{}, ops) @@ -598,6 +618,25 @@ func TestWindowRenameEmptyName(t *testing.T) { } } +func TestWindowRenameSpaceyNameRejected(t *testing.T) { + // The renamed-TO window name is held to the tightened rule. + ops := &mockTmuxOps{} + router := newTestRouter(&mockSessionFetcher{}, ops) + + body := `{"name":"my window"}` + req := httptest.NewRequest(http.MethodPost, "/api/windows/@1/rename", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest) + } + if ops.renameWindowCalled { + t.Error("RenameWindow must NOT be called for a spacey name") + } +} + func TestWindowKeys(t *testing.T) { ops := &mockTmuxOps{} router := newTestRouter(&mockSessionFetcher{}, ops) diff --git a/app/backend/internal/validate/validate.go b/app/backend/internal/validate/validate.go index 873ab95b..65665477 100644 --- a/app/backend/internal/validate/validate.go +++ b/app/backend/internal/validate/validate.go @@ -33,6 +33,26 @@ func ValidateName(name, label string) string { return "" } +// ValidateNewName validates a name for a to-be-created or renamed-to session +// or window: the permissive ValidateName rule tightened with "no spaces", so +// the safe charset the frontend's live transforms steer toward is a real +// backend contract, not just UI steering. Existing-name lookups (URL params, +// rename/kill/upload targets, session-order entries) deliberately stay on +// ValidateName — sessions created outside run-kit (raw `tmux rename-session`) +// can carry spaces and must remain operable. Hyphens stay legal here: internal +// sessions (`_rk-pin-*`, `rk-test-e2e`, group names) rely on them; the session +// hyphen→underscore rule is UI-only steering. +// Returns empty string if valid, error message if invalid. +func ValidateNewName(name, label string) string { + if msg := ValidateName(name, label); msg != "" { + return msg + } + if strings.Contains(name, " ") { + return fmt.Sprintf("%s cannot contain spaces", label) + } + return "" +} + // ValidateColorValue validates a swatch color-value descriptor: a single ANSI // index ("4") or a two-hue blend of two indices joined by '+' ("1+3"). Every // index must be an integer in [0, 15]. Returns empty string if valid, an error diff --git a/app/backend/internal/validate/validate_test.go b/app/backend/internal/validate/validate_test.go index 0b9f4987..fd0ba63a 100644 --- a/app/backend/internal/validate/validate_test.go +++ b/app/backend/internal/validate/validate_test.go @@ -355,6 +355,62 @@ func TestValidateToolName(t *testing.T) { } } +func TestValidateNewName(t *testing.T) { + tests := []struct { + name string + input string + label string + wantErr bool + contains string + }{ + // The tightened rule: spaces rejected on NEW names. + {"space rejected", "My problem", "Session name", true, "cannot contain spaces"}, + {"leading space rejected", " name", "Session name", true, "cannot contain spaces"}, + {"trailing space rejected", "name ", "Session name", true, "cannot contain spaces"}, + {"label in space error", "a b", "Window name", true, "Window name"}, + // Inherited from the permissive ValidateName rule. + {"empty rejected", "", "Session name", true, "cannot be empty"}, + {"forbidden semicolon", "my;session", "Session name", true, "forbidden characters"}, + {"contains colon", "my:session", "Session name", true, "colons or periods"}, + {"contains period", "my.session", "Session name", true, "colons or periods"}, + // Hyphens stay legal on the backend (UI-only steering): internal + // sessions (_rk-pin-*, rk-test-e2e, group names) rely on them. + {"hyphens allowed", "my-session", "Session name", false, ""}, + {"underscores allowed", "My_problem", "Session name", false, ""}, + {"alphanumeric allowed", "session123", "Session name", false, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ValidateNewName(tt.input, tt.label) + if tt.wantErr && result == "" { + t.Errorf("ValidateNewName(%q) = valid, want error", tt.input) + } + if !tt.wantErr && result != "" { + t.Errorf("ValidateNewName(%q) = %q, want valid", tt.input, result) + } + if tt.contains != "" && result != "" && !contains(result, tt.contains) { + t.Errorf("ValidateNewName(%q) = %q, want error containing %q", tt.input, result, tt.contains) + } + }) + } +} + +// TestValidateNewNameMaxLength pins that the length bound is inherited from +// ValidateName (128), not redefined. +func TestValidateNewNameMaxLength(t *testing.T) { + long := make([]byte, 129) + for i := range long { + long[i] = 'a' + } + if result := ValidateNewName(string(long), "Session name"); result == "" { + t.Error("expected max-length error, got valid") + } + if result := ValidateNewName(string(long[:128]), "Session name"); result != "" { + t.Errorf("128-char name should be valid, got %q", result) + } +} + func TestValidateWorktreeName(t *testing.T) { tests := []struct { name string diff --git a/app/frontend/src/app.tsx b/app/frontend/src/app.tsx index da850ff3..4dfb730e 100644 --- a/app/frontend/src/app.tsx +++ b/app/frontend/src/app.tsx @@ -73,7 +73,13 @@ import { selectWindow, createSession, createWindow, splitWindow, closePane, move import { useBoards } from "@/hooks/use-boards"; import { useWindowPins } from "@/hooks/use-window-pins"; import { usePinActions } from "@/hooks/use-pin-actions"; -import { deriveNameFromPath } from "@/components/create-session-dialog"; +import { + deriveNameFromPath, + finalizeSafeName, + toSafeServerName, + toSafeSessionName, + toSafeWindowName, +} from "@/lib/names"; import { useSessionContext, useUpdateNotification } from "@/contexts/session-context"; import { useOptimisticContext, useMergedSessions } from "@/contexts/optimistic-context"; import { useOptimisticAction } from "@/hooks/use-optimistic-action"; @@ -1423,7 +1429,7 @@ function AppShell() { const handleCreateIframeWindow = useCallback(() => { - const name = iframeWindowName.trim(); + const name = finalizeSafeName(iframeWindowName.trim()); const url = iframeWindowUrl.trim(); if (!name || !url || !sessionName) return; createWindow(server, sessionName, name, undefined, "iframe", url) @@ -1505,7 +1511,7 @@ function AppShell() { }); const handleCreateServer = useCallback(() => { - const trimmed = createServerName.trim(); + const trimmed = finalizeSafeName(createServerName.trim()); if (!trimmed || !/^[a-zA-Z0-9_-]+$/.test(trimmed)) return; executeCreateServer(trimmed); // Mark the just-created server pending so the route guard shows the brief @@ -2754,7 +2760,7 @@ function AppShell() { autoFocus type="text" value={iframeWindowName} - onChange={(e) => setIframeWindowName(e.target.value)} + onChange={(e) => setIframeWindowName(toSafeWindowName(e.target.value))} onKeyDown={(e) => { if (e.key === "Enter") { e.preventDefault(); @@ -2793,7 +2799,7 @@ function AppShell() { autoFocus type="text" value={dialogs.renameSessionName} - onChange={(e) => dialogs.setRenameSessionName(e.target.value)} + onChange={(e) => dialogs.setRenameSessionName(toSafeSessionName(e.target.value))} onKeyDown={(e) => e.key === "Enter" && dialogs.handleRenameSession()} onFocus={(e) => e.target.select()} aria-label="Session name" @@ -2859,7 +2865,7 @@ function AppShell() { autoFocus type="text" value={createServerName} - onChange={(e) => setCreateServerName(e.target.value)} + onChange={(e) => setCreateServerName(toSafeServerName(e.target.value))} onKeyDown={(e) => e.key === "Enter" && handleCreateServer()} onFocus={(e) => e.target.select()} aria-label="Server name" diff --git a/app/frontend/src/components/create-session-dialog.tsx b/app/frontend/src/components/create-session-dialog.tsx index 20768300..c43394ff 100644 --- a/app/frontend/src/components/create-session-dialog.tsx +++ b/app/frontend/src/components/create-session-dialog.tsx @@ -2,6 +2,7 @@ import { useState, useMemo, useRef, useEffect, useCallback } from "react"; import { createSession, createWindow, getDirectories } from "@/api/client"; import { Dialog } from "@/components/dialog"; import { LogoSpinner } from "@/components/logo-spinner"; +import { deriveNameFromPath, finalizeSafeName, toSafeSessionName } from "@/lib/names"; import { useOptimisticAction } from "@/hooks/use-optimistic-action"; import { useOptimisticContext } from "@/contexts/optimistic-context"; import { useSessionContext } from "@/contexts/session-context"; @@ -18,24 +19,6 @@ type CreateSessionDialogProps = { session?: string; }; -/** Convert a directory name into a tmux-safe session name. - * Strip colons and periods (tmux forbids them), replace hyphens with - * underscores to avoid collisions with session-group naming. */ -export function toTmuxSafeName(dirName: string): string { - return dirName - .replace(/[-]/g, "_") - .replace(/[:.]/g, "_") - .replace(/_{2,}/g, "_") - .replace(/^_|_$/g, ""); -} - -export function deriveNameFromPath(p: string): string { - const trimmed = p.replace(/\/+$/, ""); - if (trimmed === "~" || trimmed === "") return ""; - const segment = trimmed.split("/").pop() ?? ""; - return toTmuxSafeName(segment); -} - export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "session", session }: CreateSessionDialogProps) { const [name, setName] = useState(""); const [path, setPath] = useState(defaultPath ?? ""); @@ -76,9 +59,14 @@ export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "se return []; }, [suggestions, path, quickPicks]); + // The name the create will actually submit: the live-transformed input with + // the trailing separator trimmed (commit shape). Collision-checked so the + // warning matches what would be created. + const finalName = useMemo(() => finalizeSafeName(name.trim()), [name]); + const nameCollision = useMemo( - () => name.trim() !== "" && existingNames.has(name.trim()), - [name, existingNames], + () => finalName !== "" && existingNames.has(finalName), + [finalName, existingNames], ); function selectPath(p: string) { @@ -214,7 +202,7 @@ export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "se return; } - let trimmedName = name.trim(); + let trimmedName = finalName; if (!trimmedName && path.trim()) { trimmedName = deriveNameFromPath(path.trim()); } @@ -231,7 +219,7 @@ export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "se const dialogTitle = mode === "window" ? "Create window at folder" : "Create session"; const isCreateDisabled = mode === "window" ? !session - : (!name.trim() && !path.trim()) || nameCollision; + : (!finalName && !path.trim()) || nameCollision; return ( @@ -307,7 +295,9 @@ export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "se type="text" value={name} onChange={(e) => { - setName(e.target.value); + // Live safe-name conversion: the user watches "My problem" + // become "My_problem" as they type (WYSIWYG). + setName(toSafeSessionName(e.target.value)); setError(""); }} onKeyDown={(e) => e.key === "Enter" && handleCreate()} @@ -320,7 +310,7 @@ export function CreateSessionDialog({ sessions, onClose, defaultPath, mode = "se /> {nameCollision && (

- Session "{name.trim()}" already exists + Session "{finalName}" already exists

)} diff --git a/app/frontend/src/components/host-overview-page.tsx b/app/frontend/src/components/host-overview-page.tsx index da7c6ba5..7c98dbd3 100644 --- a/app/frontend/src/components/host-overview-page.tsx +++ b/app/frontend/src/components/host-overview-page.tsx @@ -2,6 +2,7 @@ import { useState, useCallback, useMemo, useRef } from "react"; import { useNavigate } from "@tanstack/react-router"; import { createServer, createSession, createWindow, getSessions, isInfraServer } from "@/api/client"; import { Dialog } from "@/components/dialog"; +import { finalizeSafeName, toSafeServerName } from "@/lib/names"; import { useOptimisticAction } from "@/hooks/use-optimistic-action"; import { useToast } from "@/components/toast"; import { useHostMetrics, useHostServices, useSessionContext } from "@/contexts/session-context"; @@ -153,7 +154,10 @@ export function HostOverviewPage() { }); const handleCreate = useCallback(() => { - const trimmed = createName.trim(); + // The input applies the live server transform; commit trims the trailing + // separator. The regex stays as defense in depth (always passes post- + // transform). + const trimmed = finalizeSafeName(createName.trim()); if (!trimmed || !/^[a-zA-Z0-9_-]+$/.test(trimmed)) return; executeCreateServer(trimmed); // Mark the just-created server pending BEFORE navigating so the route guard @@ -474,7 +478,8 @@ export function HostOverviewPage() { autoFocus type="text" value={createName} - onChange={(e) => setCreateName(e.target.value)} + // Live safe-name conversion (server kind — strictest charset). + onChange={(e) => setCreateName(toSafeServerName(e.target.value))} onKeyDown={(e) => e.key === "Enter" && handleCreate()} onFocus={(e) => e.target.select()} aria-label="Server name" diff --git a/app/frontend/src/components/sidebar.test.tsx b/app/frontend/src/components/sidebar.test.tsx index c621d8e0..939cf3dd 100644 --- a/app/frontend/src/components/sidebar.test.tsx +++ b/app/frontend/src/components/sidebar.test.tsx @@ -525,13 +525,24 @@ describe("Sidebar", () => { renderSidebar(); fireEvent.doubleClick(getSessionRowNameSpan("run-kit")); const input = screen.getByLabelText("Rename session"); + // The typed hyphen live-converts to "_" (session-kind safe-name rule, + // 260722-ln4n) — the committed name is the converted one. fireEvent.change(input, { target: { value: "blur-session" } }); await act(async () => { fireEvent.blur(input); }); expect(screen.queryByLabelText("Rename session")).not.toBeInTheDocument(); - expect(renameSessionMock).toHaveBeenCalledWith("runkit", "run-kit", "blur-session"); + expect(renameSessionMock).toHaveBeenCalledWith("runkit", "run-kit", "blur_session"); + }); + + it("live-converts unsafe chars as the user types (space → underscore)", () => { + renderSidebar(); + fireEvent.doubleClick(getSessionRowNameSpan("run-kit")); + const input = screen.getByLabelText("Rename session"); + fireEvent.change(input, { target: { value: "My problem" } }); + // WYSIWYG: the input itself shows the safe form (260722-ln4n). + expect(input).toHaveValue("My_problem"); }); it("empty input cancels without API call", async () => { diff --git a/app/frontend/src/components/sidebar/index.tsx b/app/frontend/src/components/sidebar/index.tsx index fbb43a9c..11492c76 100644 --- a/app/frontend/src/components/sidebar/index.tsx +++ b/app/frontend/src/components/sidebar/index.tsx @@ -5,6 +5,7 @@ import { killSession as killSessionApi, killWindow as killWindowApi, renameSessi import { useSessionContext } from "@/contexts/session-context"; import { useFocusedPane } from "@/contexts/focused-pane-context"; import { resolveFocusedWindow, thinWindowFromFocusedPane } from "@/lib/focused-pane-window"; +import { finalizeSafeName } from "@/lib/names"; import { useOptimisticAction } from "@/hooks/use-optimistic-action"; import { useOptimisticContext } from "@/contexts/optimistic-context"; import { useToast } from "@/components/toast"; @@ -526,7 +527,9 @@ export function Sidebar({ const handleSessionRenameCommit = useCallback(() => { if (!editingSession) return; - const trimmed = editingSessionName.trim(); + // The row input applies the live session transform; commit trims the + // trailing separator the live transform keeps visible while typing. + const trimmed = finalizeSafeName(editingSessionName.trim()); const originalName = sessionOriginalNameRef.current; const { server: srv, name: sessionName } = editingSession; setEditingSession(null); @@ -567,7 +570,8 @@ export function Sidebar({ const handleRenameCommit = useCallback(() => { if (!editingWindow) return; - const trimmed = editingName.trim(); + // Same commit-time finalize as the session rename above (window kind). + const trimmed = finalizeSafeName(editingName.trim()); const originalName = originalNameRef.current; const { server: srv, session, windowId } = editingWindow; setEditingWindow(null); diff --git a/app/frontend/src/components/sidebar/session-row.tsx b/app/frontend/src/components/sidebar/session-row.tsx index 3ae65ff4..7e8450e1 100644 --- a/app/frontend/src/components/sidebar/session-row.tsx +++ b/app/frontend/src/components/sidebar/session-row.tsx @@ -5,6 +5,7 @@ import type { RowTint } from "@/themes"; import { SwatchPopover } from "@/components/swatch-popover"; import { WaitingBadge } from "@/components/waiting-badge"; import { countWaitingWindows } from "@/lib/waiting"; +import { toSafeSessionName } from "@/lib/names"; import { PaletteIcon, BotIcon } from "./icons"; type SessionRowProps = { @@ -186,7 +187,7 @@ function SessionRowInner({ ref={sessionInputRef} type="text" value={editingSessionName} - onChange={(e) => onSessionNameChange(e.target.value)} + onChange={(e) => onSessionNameChange(toSafeSessionName(e.target.value))} onKeyDown={onSessionRenameKeyDown} onBlur={onSessionRenameBlur} onClick={(e) => e.stopPropagation()} diff --git a/app/frontend/src/components/sidebar/window-row.tsx b/app/frontend/src/components/sidebar/window-row.tsx index b71cf239..b6d14ca5 100644 --- a/app/frontend/src/components/sidebar/window-row.tsx +++ b/app/frontend/src/components/sidebar/window-row.tsx @@ -9,6 +9,7 @@ import { StatusDot } from "@/components/status-dot"; import { PinPopover } from "./pin-popover"; import { PaletteIcon } from "./icons"; import { PinIcon } from "@/components/pin-icon"; +import { toSafeWindowName } from "@/lib/names"; type ProjectWindow = ProjectSession["windows"][number]; type GhostWindow = MergedSession["windows"][number]; @@ -356,7 +357,7 @@ function WindowRowInner({ ref={inputRef} type="text" value={editingName} - onChange={(e) => onWindowNameChange(e.target.value)} + onChange={(e) => onWindowNameChange(toSafeWindowName(e.target.value))} onKeyDown={onRenameKeyDown} onBlur={onRenameBlur} onClick={(e) => e.stopPropagation()} diff --git a/app/frontend/src/components/spawn-agent-dialog.tsx b/app/frontend/src/components/spawn-agent-dialog.tsx index 10daf9f5..8cd2a124 100644 --- a/app/frontend/src/components/spawn-agent-dialog.tsx +++ b/app/frontend/src/components/spawn-agent-dialog.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, useRef, useCallback } from "react"; import { spawnRiff, getRiffPresets, type RiffPreset, type RiffWhere } from "@/api/client"; import { Dialog } from "@/components/dialog"; import { LogoSpinner } from "@/components/logo-spinner"; +import { finalizeSafeName, toSafeWorktreeName } from "@/lib/names"; type SpawnAgentDialogProps = { /** The target tmux server — the server that OWNS the target session. Supplied @@ -115,8 +116,9 @@ export function SpawnAgentDialog({ server, session, onSpawned, onClose }: SpawnA preset: preset || undefined, where, // Worktree name only applies to worktree mode; the backend rejects it with - // checkout, so drop it there. - worktreeName: where === "worktree" ? worktreeName.trim() || undefined : undefined, + // checkout, so drop it there. Commit-time finalize trims the trailing + // separator the live transform keeps visible while typing. + worktreeName: where === "worktree" ? finalizeSafeName(worktreeName.trim()) || undefined : undefined, // The tier is sent ONLY when the Agent Tier field is shown (a fab project // — non-empty tiers). When the field is hidden (non-fab repo) `tier` is // omitted entirely, matching the gate: rk never sends an inert tier. @@ -231,7 +233,9 @@ export function SpawnAgentDialog({ server, session, onSpawned, onClose }: SpawnA type="text" value={worktreeName} onChange={(e) => { - setWorktreeName(e.target.value); + // Live safe-name conversion (worktree kind — hyphens kept, no + // leading hyphen, slash converts). + setWorktreeName(toSafeWorktreeName(e.target.value)); setError(""); }} onKeyDown={handleKeyDown} diff --git a/app/frontend/src/components/top-bar.test.tsx b/app/frontend/src/components/top-bar.test.tsx index 1cf76edc..a2b17240 100644 --- a/app/frontend/src/components/top-bar.test.tsx +++ b/app/frontend/src/components/top-bar.test.tsx @@ -1345,6 +1345,21 @@ describe("WindowHeading (centered, editable, terminal mode)", () => { expect(screen.queryByRole("textbox", { name: "Window name" })).not.toBeInTheDocument(); }); + it("live-converts typed unsafe chars (space → underscore, hyphen kept)", async () => { + const { renameWindow } = await import("@/api/client"); + renderTopBar(); + act(() => fireEvent.click(screen.getByRole("button", { name: "Rename window main" }))); + const input = screen.getByRole("textbox", { name: "Window name" }) as HTMLInputElement; + // WYSIWYG (260722-ln4n): the input shows the safe form as the user types — + // spaces convert to "_", hyphens are KEPT (window-kind rule). + act(() => fireEvent.change(input, { target: { value: "riff-my problem" } })); + expect(input.value).toBe("riff-my_problem"); + act(() => fireEvent.keyDown(input, { key: "Enter" })); + await waitFor(() => { + expect(renameWindow).toHaveBeenCalledWith("runkit", "@0", "riff-my_problem"); + }); + }); + it("Escape cancels with no API call and restores the original name", async () => { const { renameWindow } = await import("@/api/client"); renderTopBar(); diff --git a/app/frontend/src/components/top-bar.tsx b/app/frontend/src/components/top-bar.tsx index eb7f923e..0f3601fb 100644 --- a/app/frontend/src/components/top-bar.tsx +++ b/app/frontend/src/components/top-bar.tsx @@ -13,6 +13,7 @@ import { displayVersion } from "@/lib/palette-version"; import { updateChipToolSummary } from "@/lib/palette-update"; import { splitWindow, closePane } from "@/api/client"; import { useWindowRename } from "@/hooks/use-window-rename"; +import { finalizeSafeName, toSafeWindowName } from "@/lib/names"; import { prefersReducedMotion } from "@/lib/motion"; import { WaitingBadge } from "@/components/waiting-badge"; import { ViewSwitcher, ViewSwitcherMenuRows } from "@/components/view-switcher"; @@ -1486,9 +1487,11 @@ function WindowHeading({ }, [editing]); const commit = useCallback(() => { - const trimmed = draft.trim(); + // The edit input applies the live window transform; commit trims the + // trailing separator the live transform keeps visible while typing. + const trimmed = finalizeSafeName(draft.trim()); setEditing(false); - // Empty/whitespace-only commit = cancel (matches the dialog's trim guard). + // Empty-after-conversion commit = cancel (matches the dialog's trim guard). if (!trimmed || trimmed === name) { sweep.resolve(); setDraft(name); @@ -1513,7 +1516,9 @@ function WindowHeading({ ref={inputRef} type="text" value={draft} - onChange={(e) => setDraft(e.target.value)} + // Live safe-name conversion (window kind — hyphens kept): a typed + // space appears as "_" so the committed name is the displayed name. + onChange={(e) => setDraft(toSafeWindowName(e.target.value))} onBlur={() => { // A key (Enter/Escape) already committed/cancelled and is tearing // the input down — swallow the trailing blur so it doesn't re-commit diff --git a/app/frontend/src/hooks/use-dialog-state.ts b/app/frontend/src/hooks/use-dialog-state.ts index e12ddabe..7431e44b 100644 --- a/app/frontend/src/hooks/use-dialog-state.ts +++ b/app/frontend/src/hooks/use-dialog-state.ts @@ -1,5 +1,6 @@ import { useState, useCallback, useMemo, useRef } from "react"; import { renameSession, killSession, killWindow } from "@/api/client"; +import { finalizeSafeName } from "@/lib/names"; import { useOptimisticAction } from "@/hooks/use-optimistic-action"; import { useOptimisticContext } from "@/contexts/optimistic-context"; import { useSessionContext } from "@/contexts/session-context"; @@ -73,8 +74,10 @@ export function useDialogState({ sessionName, windowId, onKillComplete, onSessio }); const handleRenameSession = useCallback(() => { - if (!renameSessionName.trim() || !sessionName) return; - const newName = renameSessionName.trim(); + // The input applies the live session transform; commit trims the trailing + // separator the live transform deliberately keeps visible. + const newName = finalizeSafeName(renameSessionName.trim()); + if (!newName || !sessionName) return; executeRenameSession(server, sessionName, newName); onSessionRenamed?.(newName); setShowRenameSessionDialog(false); diff --git a/app/frontend/src/lib/names.test.ts b/app/frontend/src/lib/names.test.ts new file mode 100644 index 00000000..e1972741 --- /dev/null +++ b/app/frontend/src/lib/names.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect } from "vitest"; +import { + toSafeSessionName, + toSafeWindowName, + toSafeServerName, + toSafeWorktreeName, + finalizeSafeName, + deriveNameFromPath, +} from "./names"; + +describe("toSafeSessionName", () => { + const cases: Array<[raw: string, expected: string, why: string]> = [ + ["My problem", "My_problem", "space converts, case preserved"], + ["my-session", "my_session", "hyphen converts (session-specific rule)"], + ["a:b.c", "a_b_c", "colon and period convert"], + ["a;b&c|d", "a_b_c_d", "shell metacharacters convert"], + ["a`b$c(d)e", "a_b_c_d_e", "backtick/dollar/parens convert"], + ["a{b}c[d]e", "a_b_c_d_e", "braces/brackets convert"], + ["ac!d#e*f?g", "a_b_c_d_e_f_g", "remaining forbidden set converts"], + ["a\nb\rc\td", "a_b_c_d", "control chars convert"], + ["a - b", "a_b", "consecutive converted chars collapse to one _"], + [" hello", "hello", "leading unsafe chars drop"], + ["My problem ", "My_problem_", "trailing separator stays visible (live)"], + ["a/b@c%d", "a/b@c%d", "chars outside the unsafe set pass through"], + ["", "", "empty stays empty"], + [" ", "", "only unsafe chars → empty"], + ["MiXeD CaSe", "MiXeD_CaSe", "case is never folded"], + ]; + it.each(cases)("%j → %j (%s)", (raw, expected) => { + expect(toSafeSessionName(raw)).toBe(expected); + }); + + it("caps at 128 chars (backend MaxNameLength)", () => { + expect(toSafeSessionName("a".repeat(200))).toHaveLength(128); + }); +}); + +describe("toSafeWindowName", () => { + const cases: Array<[raw: string, expected: string, why: string]> = [ + ["riff-foo", "riff-foo", "hyphens KEPT (window divergence from session)"], + ["riff-foo bar", "riff-foo_bar", "space converts, hyphen kept"], + ["my problem", "my_problem", "space converts"], + ["a:b.c", "a_b_c", "colon and period convert"], + [" x", "x", "leading unsafe chars drop"], + ["x ", "x_", "trailing separator stays visible (live)"], + ]; + it.each(cases)("%j → %j (%s)", (raw, expected) => { + expect(toSafeWindowName(raw)).toBe(expected); + }); + + it("caps at 128 chars", () => { + expect(toSafeWindowName("b".repeat(200))).toHaveLength(128); + }); +}); + +describe("toSafeServerName", () => { + const cases: Array<[raw: string, expected: string, why: string]> = [ + ["my server!", "my_server_", "anything outside [a-zA-Z0-9_-] converts"], + ["dev-2", "dev-2", "hyphens legal for servers"], + ["a.b:c", "a_b_c", "punctuation converts"], + ["héllo", "h_llo", "non-ASCII converts"], + [" x", "x", "leading run drops"], + ]; + it.each(cases)("%j → %j (%s)", (raw, expected) => { + expect(toSafeServerName(raw)).toBe(expected); + }); + + it("caps at 64 chars (backend MaxServerNameLength)", () => { + expect(toSafeServerName("c".repeat(100))).toHaveLength(64); + }); +}); + +describe("toSafeWorktreeName", () => { + const cases: Array<[raw: string, expected: string, why: string]> = [ + ["swift-fox", "swift-fox", "hyphens kept (window rule base)"], + ["a/b", "a_b", "slash converts (directory basename)"], + ["-agent", "agent", "leading hyphen dropped (flag-injection defense)"], + ["- -x", "x", "mixed leading hyphen/underscore run dropped"], + ["my agent", "my_agent", "space converts"], + ]; + it.each(cases)("%j → %j (%s)", (raw, expected) => { + expect(toSafeWorktreeName(raw)).toBe(expected); + }); +}); + +describe("finalizeSafeName", () => { + const cases: Array<[raw: string, expected: string]> = [ + ["My_problem_", "My_problem"], + ["_x_", "x"], + ["plain", "plain"], + ["_", ""], + ["", ""], + ]; + it.each(cases)("%j → %j", (raw, expected) => { + expect(finalizeSafeName(raw)).toBe(expected); + }); +}); + +describe("deriveNameFromPath", () => { + const cases: Array<[path: string, expected: string]> = [ + ["~/code/my-project", "my_project"], + ["~/code/my-project/", "my_project"], + ["~", ""], + ["", ""], + ["/tmp/My Project", "My_Project"], + ["~/x/name.with.dots", "name_with_dots"], + ]; + it.each(cases)("%j → %j", (path, expected) => { + expect(deriveNameFromPath(path)).toBe(expected); + }); +}); diff --git a/app/frontend/src/lib/names.ts b/app/frontend/src/lib/names.ts new file mode 100644 index 00000000..3c2a9d62 --- /dev/null +++ b/app/frontend/src/lib/names.ts @@ -0,0 +1,92 @@ +/** + * Per-kind safe-name transforms applied LIVE in every naming input's onChange + * (260722-ln4n): the user watches "My problem" become "My_problem" as they + * type, so the optimistic-update name is identical to the committed name + * (WYSIWYG). The backend stays reject-only — these transforms steer input to + * the charset the backend's tightened new-name validation accepts. + * + * Shape shared by all transforms: unsafe chars CONVERT to "_" (never strip — + * stripping silently shortens names and loses word boundaries), consecutive + * converted chars collapse to one "_", leading separators drop as typed (a + * space pressed in an empty field produces nothing), case is preserved, and + * length caps live at the backend maxima. A TRAILING "_" deliberately stays + * visible while typing — trimming it live would delete the separator the user + * just typed ("My " + "p" would become "Myp") — and is trimmed only at commit + * via `finalizeSafeName`. + */ + +/** Mirrors backend validate.MaxNameLength (session/window names). */ +const MAX_NAME_LENGTH = 128; +/** Mirrors backend validate.MaxServerNameLength. */ +const MAX_SERVER_NAME_LENGTH = 64; + +/** + * Chars every tmux-name transform converts: the backend forbidden set + * (validate.go forbiddenChars), the ":" / "." pair ValidateName rejects + * separately, and the space the tightened new-name rule rejects. + */ +const BASE_UNSAFE = /[ ;&|`$(){}[\]<>!#*?\n\r\t:.]/g; + +/** Collapse "_" runs, drop leading "_", cap length. */ +function squash(converted: string, max: number): string { + return converted.replace(/_{2,}/g, "_").replace(/^_+/, "").slice(0, max); +} + +/** + * Session names: base rule PLUS hyphen→"_". The hyphen rule is + * session-specific — it avoids collisions with session-group naming. + */ +export function toSafeSessionName(raw: string): string { + return squash(raw.replace(BASE_UNSAFE, "_").replace(/-/g, "_"), MAX_NAME_LENGTH); +} + +/** + * Window names: base rule, hyphens KEPT — `riff-*` windows use hyphens + * legitimately and the session-group collision rationale doesn't apply. + */ +export function toSafeWindowName(raw: string): string { + return squash(raw.replace(BASE_UNSAFE, "_"), MAX_NAME_LENGTH); +} + +/** + * Server names: strictest — anything outside [a-zA-Z0-9_-] converts to "_" + * (mirrors backend ValidateServerName's ^[a-zA-Z0-9_-]+$). + */ +export function toSafeServerName(raw: string): string { + return squash(raw.replace(/[^a-zA-Z0-9_-]/g, "_"), MAX_SERVER_NAME_LENGTH); +} + +/** + * Worktree names: the window rule plus ValidateWorktreeName's extra + * constraints — "/" converts to "_" (worktree directory basename), and a + * leading hyphen run is dropped (a leading "-" would look like a flag to + * `wt create`). + */ +export function toSafeWorktreeName(raw: string): string { + return squash( + raw.replace(BASE_UNSAFE, "_").replace(/\//g, "_").replace(/^[-_]+/, ""), + MAX_NAME_LENGTH, + ); +} + +/** + * Commit-time finisher: trims the trailing "_" the live transforms keep + * visible while typing (plus a defensive leading trim). Apply at every + * commit/submit site whose input carries a live transform — the one minimal + * deviation from strict WYSIWYG. + */ +export function finalizeSafeName(name: string): string { + return name.replace(/^_+|_+$/g, ""); +} + +/** + * Derive a session-name suggestion from a filesystem path: last segment, + * session-transformed, fully trimmed (suggestions are commit-shaped — no + * trailing separator to preserve). + */ +export function deriveNameFromPath(p: string): string { + const trimmed = p.replace(/\/+$/, ""); + if (trimmed === "~" || trimmed === "") return ""; + const segment = trimmed.split("/").pop() ?? ""; + return finalizeSafeName(toSafeSessionName(segment)); +} diff --git a/app/frontend/tests/e2e/sync-latency.spec.md b/app/frontend/tests/e2e/sync-latency.spec.md index 4b359bcd..9f1ca4f6 100644 --- a/app/frontend/tests/e2e/sync-latency.spec.md +++ b/app/frontend/tests/e2e/sync-latency.spec.md @@ -51,16 +51,20 @@ name) and a ghost entry renders in the sidebar in ≤500ms. **What it proves:** Double-clicking a session name opens an inline input; pressing Enter commits an optimistic rename and the new name renders in -≤500ms. +≤500ms. The input applies the live session-kind safe-name transform +(260722-ln4n — hyphens convert to `_`), so the committed name is the +underscored form of what was filled. **Steps:** 1. `setup`. 2. Wait for the `Navigate to ${SESSION_A}` button to exist (this test runs before any rename, so SESSION_A's original name is still present). 3. Double-click the session name to enter edit mode. -4. Clear and fill the input with `${SESSION_A}-renamed`. +4. Clear and fill the input with `${SESSION_A}-renamed`; assert the input + value is its underscored form (the live transform converted the hyphens). 5. Start timer, press Enter. -6. Wait for the new name text to appear; `record`. +6. Wait for the new (underscored) name text to appear; `record`. (The + `afterAll` sweep kills the underscored name.) ### `3. Create window via sidebar + button` diff --git a/app/frontend/tests/e2e/sync-latency.spec.ts b/app/frontend/tests/e2e/sync-latency.spec.ts index df9853a6..e828aa51 100644 --- a/app/frontend/tests/e2e/sync-latency.spec.ts +++ b/app/frontend/tests/e2e/sync-latency.spec.ts @@ -80,7 +80,9 @@ test.describe("Sync Latency Audit", () => { const names = [ SESSION_A, SESSION_B, - `${SESSION_A}-renamed`, + // Test 2's UI rename commits the underscored form (the session-kind + // live transform converts hyphens — 260722-ln4n). + `${SESSION_A}-renamed`.replace(/-/g, "_"), `e2e-kill-${SESSION_A}`, "session", ]; @@ -147,8 +149,12 @@ test.describe("Sync Latency Audit", () => { const input = sidebar.locator("input[type='text']").first(); await expect(input).toBeVisible({ timeout: 2_000 }); await input.clear(); - const newName = `${SESSION_A}-renamed`; - await input.fill(newName); + // The session rename input live-converts unsafe/steered chars as they land + // (260722-ln4n): the session-kind transform converts hyphens to "_", so + // filling the hyphenated string commits its underscored form. + await input.fill(`${SESSION_A}-renamed`); + const newName = `${SESSION_A}-renamed`.replace(/-/g, "_"); + await expect(input).toHaveValue(newName); const t0 = Date.now(); await input.press("Enter"); diff --git a/app/frontend/tests/e2e/window-heading.spec.md b/app/frontend/tests/e2e/window-heading.spec.md index e98b1aef..b5229b99 100644 --- a/app/frontend/tests/e2e/window-heading.spec.md +++ b/app/frontend/tests/e2e/window-heading.spec.md @@ -132,6 +132,22 @@ the heading reflect the new name. 4. Assert the sidebar shows the new name (rename API + SSE round-trip). 5. Assert the heading button now carries the new name. +### `typing a space live-converts to underscore and commits the safe name` + +**What it proves:** The live safe-name conversion (260722-ln4n): the inline +window-name input converts unsafe characters AS THE USER TYPES — a pressed +space appears as `_` (window-kind transform: hyphens kept) — so the input is +WYSIWYG and the committed name is exactly the displayed one. + +**Steps:** +1. Create + navigate to a window; click the heading to open the inline edit. +2. Clear the input, then type `my problem` character-by-character + (`pressSequentially` — real keystrokes, so each `onChange` runs the live + transform). +3. Assert the input value is `my_problem` (the space never appears). +4. Press Enter; assert the sidebar shows `my_problem` (rename API + SSE + round-trip) and the heading button carries the converted name. + ### `Escape cancels the edit and restores the original name` **What it proves:** Escape abandons the edit — no rename call, original name diff --git a/app/frontend/tests/e2e/window-heading.spec.ts b/app/frontend/tests/e2e/window-heading.spec.ts index 7cf3fc9c..ac87a3e7 100644 --- a/app/frontend/tests/e2e/window-heading.spec.ts +++ b/app/frontend/tests/e2e/window-heading.spec.ts @@ -197,6 +197,37 @@ test.describe("Window heading (centered, editable) + hover vocabulary", () => { ).toBeVisible({ timeout: 10_000 }); }); + test("typing a space live-converts to underscore and commits the safe name", async ({ + page, + }) => { + const name = `head-safe-${Date.now()}`; + execSync(`tmux -L ${TMUX_SERVER} new-window -t ${TEST_SESSION} -n "${name}"`, { + stdio: "ignore", + }); + const id = await resolveWindow(page, name); + await gotoWindow(page, id); + + await page.getByRole("button", { name: `Rename window ${name}` }).click(); + const input = page.getByRole("textbox", { name: "Window name" }); + await expect(input).toBeVisible(); + // Type character-by-character: the live safe-name transform (260722-ln4n) + // converts the pressed space to "_" as it lands — WYSIWYG, the input never + // shows a space. + await input.fill(""); + await input.pressSequentially("my problem"); + await expect(input).toHaveValue("my_problem"); + await input.press("Enter"); + + // The committed name is exactly the displayed (converted) name. + const sidebar = page.locator("nav[aria-label='Sessions']"); + await expect(sidebar.locator("text=my_problem").first()).toBeVisible({ + timeout: 10_000, + }); + await expect( + page.getByRole("button", { name: "Rename window my_problem" }), + ).toBeVisible({ timeout: 10_000 }); + }); + test("Escape cancels the edit and restores the original name", async ({ page, }) => { diff --git a/docs/memory/run-kit/architecture.md b/docs/memory/run-kit/architecture.md index e17f885d..a87865e6 100644 --- a/docs/memory/run-kit/architecture.md +++ b/docs/memory/run-kit/architecture.md @@ -131,23 +131,25 @@ Packages in `app/backend/internal/`: All endpoints served by the single Go binary on one port. POST-only mutations with path-based intent (no multiplexed action field). **Uniform HTTP verb (constitution §IX, enforced since `260529-jad6`)**: every mutating endpoint uses `POST`; there are zero `PUT`/`PATCH`/`DELETE` route registrations and the CORS `AllowedMethods` allowlist is exactly `[GET, POST, OPTIONS]`. `260529-jad6` migrated the five remaining `.Put(` routes to `POST`: `sessions/order`, `settings/theme`, `settings/server-color` (verb-only), plus `windows/{id}/url` and `windows/{id}/type` (folded into `POST /options`, removed rather than migrated). Endpoint semantics that would conventionally map to other verbs (e.g. partial update on `/options`) are expressed via the path and a documented body contract. +**Name validation — new-name vs. existing-name split (since `260722-ln4n-auto-safe-name-conversion`)**: `internal/validate` exposes two name rules and the API layer picks by whether the value names a **to-be-created / renamed-to** entity or an **existing** one. `ValidateNewName(name, label)` (permissive `ValidateName` + a "no spaces" rejection) guards exactly the four new-name call sites — session create + session rename-to (`api/sessions.go`), window create (non-empty name only) + window rename-to (`api/windows.go`) — so the safe charset is a real backend contract, not merely UI steering. Every existing-name lookup (session URL params, `TargetSession`, upload/riff session values, session-order entries) keeps the permissive `ValidateName`, so pre-existing spacey names created outside run-kit stay renamable/killable. The backend stays reject-only (no server-side name conversion — constitution §I) and keeps hyphens legal for both name kinds; the session hyphen→`_` steering is UI-only (see [tmux-sessions](tmux-sessions.md) § Name-Validation Charset Contract for the full contract and rationale, and [ui-patterns](ui-patterns.md) § Live Safe-Name Conversion for the frontend transforms). + **Window addressing (since `260529-chgz-window-id-routing`)**: routes that act on a *specific existing window* are keyed by the stable tmux **window ID** under `/api/windows/{windowId}/...` (kill, move, move-to-session, rename, options, keys, select, split, close-pane). The `{windowId}` path param is validated via `validate.ValidateWindowID` (`^@[0-9]+$`) by the `parseWindowID(r) (string, bool)` helper in `api/windows.go`, which delegates to the shared `decodeWindowID(r) (string, bool)` helper (since `260529-jad6` — `url.PathUnescape` + `ValidateWindowID` in exactly one place). Since `260717-803u-relay-mux` retired `handleRelay`, the `/ws/terminals` mux is the second entry point: it validates its `open` op's already-JSON-decoded `windowId` through the **same** `validate.ValidateWindowID` call (no percent-decode — the id rides a JSON control frame, not a URL segment), so the two entry points share one validator and cannot drift (the drift that caused bug #205). `400` on malformed ID before any tmux call. A window ID is a server-global, self-contained tmux target, so the session name drops out of these paths. Routes that *create* a window or enumerate a session's windows stay session-scoped (`POST /api/sessions/{session}/windows`, session enumeration) — there is no window ID at create time. | Endpoint | Method | Purpose | |----------|--------|---------| | `/api/health` | GET | Returns `200 {"status":"ok","hostname":"..."}` for health checks. Hostname computed once at startup via `os.Hostname()`, stored in `Server` struct; falls back to `""` on error | | `/api/sessions` | GET | Returns `ProjectSession[]` — one per tmux session, with auto-detected fab enrichment (`fabChange`/`fabStage` on windows). Since `260713-nh86-chat-session-identity` each window and each pane also carries the chat-session-identity fields `chatProvider`/`chatSessionRef` (from the `@rk_chat` pane option — see [agent-state](agent-state.md) § Chat Session Identity): per-window is the `rollupChat` result (active pane first, else first pane carrying a chat), per-pane is the reconciled `PaneInfo.ChatProvider/ChatSessionRef`. Additive fields riding the existing `ProjectSession` marshal — no new endpoint | -| `/api/sessions` | POST | Create session — JSON body `{"name":"...","cwd":"..."}`. Returns `201 {"ok":true}` | +| `/api/sessions` | POST | Create session — JSON body `{"name":"...","cwd":"..."}`. `name` validated via `validate.ValidateNewName` (permissive `ValidateName` + no-spaces; a spacey name → `400`, nothing created — see the name-validation split above). Returns `201 {"ok":true}` | | `/api/sessions/order` | GET | Returns `{"order":[...]}` — JSON array of session names representing the user-defined sidebar render order on `?server=`. Reads tmux server-scoped user-option `@rk_session_order`. Unset/no-server cases return `200 {"order":[]}` (not 404); other tmux failures (e.g., malformed JSON in stored option) return `500` with error body | | `/api/sessions/order` | POST | Persists the sidebar session order — JSON body `{"order":[...]}`. (Migrated PUT→POST by `260529-jad6` per §IX; body unchanged.) Each name validated via `validate.ValidateName`. JSON-encoded value written to tmux user-option `@rk_session_order` via `set-option -s` (server-scoped). After a successful write, broadcasts `event: session-order` (payload `{server, order}`) synchronously to all SSE clients on that server. Stale names (not currently a live session) are accepted — frontend's `orderedSessions` sort tolerates them. Returns `400` on malformed JSON or invalid name; `200 {"ok":true}` on success | | `/api/sessions/:session/kill` | POST | Kill session — `:session` validated via `validate.ValidateName()`. Returns `200 {"ok":true}` | -| `/api/sessions/:session/windows` | POST | Create window — JSON body `{"name":"...","cwd":"..."}`. **Session-scoped** (the window does not exist yet, so there is no window ID to key on). Returns `201 {"ok":true}` | +| `/api/sessions/:session/windows` | POST | Create window — JSON body `{"name":"...","cwd":"..."}`. **Session-scoped** (the window does not exist yet, so there is no window ID to key on). A non-empty `name` is validated via `validate.ValidateNewName` (no spaces); an omitted/empty name lets tmux auto-name (see [tmux-sessions](tmux-sessions.md) § Managed Window Creation). Returns `201 {"ok":true}` | | `/api/windows/{windowId}/kill` | POST | Kill window — `{windowId}` validated via `validate.ValidateWindowID` (`^@[0-9]+$`); `400` on malformed ID, no tmux call. Returns `200 {"ok":true}` | | `/api/windows/{windowId}/move` | POST | Reorder window within its session — JSON body `{"targetIndex": N}`. Source addressed by stable `{windowId}`; destination remains positional. Backend resolves the source's current index from the ID, then bubble-swaps to the target position via `tmux swap-window` (insert-before semantics). Returns `200 {"ok":true}` | | `/api/windows/{windowId}/move-to-session` | POST | Move window to another session — JSON body `{"targetSession":"..."}`. Runs `tmux move-window -s {windowId} -t {targetSession}:` (window ID source, session destination; tmux preserves the ID and auto-assigns the index). Returns `200 {"ok":true}` | | `/api/sessions/:session/color` | POST | Set or clear session color — JSON body `{"color": "4"}` / `{"color": "1+3"}` (color-value descriptor string) to set, `{"color": null}` to clear. `handleSessionColor` validates the session name and the descriptor via the shared `validate.ValidateColorValue` (single index 0–15 OR `a+b` each 0–15; `400` on malformed) then writes/clears the tmux `@session_color` option via `tmux.SetSessionColor`/`UnsetSessionColor` — NOT `run-kit.yaml` (since `260615-6rnr`; the body was `{"color": N}` int before). Returns `200 {"ok":true}` | | `/api/windows/{windowId}/options` | POST | **Unified partial-merge window-option setter** (since `260529-jad6` — replaced the three separate `POST /color`, `PUT /url`, `PUT /type` routes). JSON body `{"options": {"@color":"5","@rk_url":"https://…","@rk_type":null,"@rk_marker":"solid"}}` decoded as `map[string]*string`: a present key with a string sets it, an explicit `null` unsets it, an absent key is left untouched (partial-merge — never full-replace, which avoided the "client clears a field it didn't echo" bug class). **Key allowlist** `{@color, @rk_url, @rk_type, @rk_marker}` (the `optKey*` constants) — any other key → `400`, never passed to tmux (§I — bounds the option-name surface). Per-key validation (`validateWindowOption`): `@color` must be a color-value descriptor — a single ANSI index `0–15` OR a two-hue blend `a+b` (each component `0–15`), validated via the shared `validate.ValidateColorValue`; `@rk_url` non-empty after trim; `@rk_type` non-empty sets verbatim (no enum validation, preserving `handleWindowTypeUpdate`), empty/`null` unsets; `@rk_marker` must be one of `""`/`dotted`/`solid`/`double` via `validate.ValidateMarkerValue` (the closed-set rule). Like `@rk_type`, an empty-string `@rk_marker` is treated as **unset** (nil op — clear the marker). **Validate-all-before-any-tmux**: if any key fails validation the endpoint returns `400` and issues zero tmux calls (no partial application). The whole merge executes as **one atomic `\;`-chained tmux invocation** via `tmux.SetWindowOptions`, then `handleWindowOptions` wakes the SSE hub (`s.sseHub.wake(server)`, § Per-server wake seam) so the mutation repaints in one poll pass. Returns `200 {"ok":true}` | -| `/api/windows/{windowId}/rename` | POST | Rename window — JSON body `{"name":"..."}`. Returns `200 {"ok":true}` | +| `/api/windows/{windowId}/rename` | POST | Rename window — JSON body `{"name":"..."}`. The `{windowId}` target is ID-addressed (an existing window), but the rename-*to* `name` is a new name → validated via `validate.ValidateNewName` (non-empty, no spaces; `400` otherwise). Returns `200 {"ok":true}` | | `/api/windows/{windowId}/keys` | POST | Send keys — JSON body `{"keys":"..."}` (non-empty after trim). Returns `200 {"ok":true}` | | `/api/windows/{windowId}/select` | POST | Select (activate) the window. **Session-scoped (since `260529-jad6`)**: `handleWindowSelect` resolves the owning session for `{windowId}` server-side via `ResolveWindowSession`, then issues `tmux select-window -t :{windowId}` via `SelectWindowInSession` — NOT a bare `select-window -t {windowId}`. The scoped target disambiguates inside tmux session groups (group members keep independent active-window state, so a bare target may activate the window on the wrong member, causing the wrong-window-highlight symptom). The session is derived server-side; the client never supplies it. A stale `{windowId}` whose session cannot be resolved surfaces a non-2xx error and issues no select. Returns `200 {"ok":true}` | | `/api/windows/{windowId}/split` | POST | Split the window's active pane — JSON body `{"horizontal": bool}`. Returns `200 {"ok":true,"paneId":"%N"}` | diff --git a/docs/memory/run-kit/tmux-sessions.md b/docs/memory/run-kit/tmux-sessions.md index 5cebe8d4..dbc5cf7d 100644 --- a/docs/memory/run-kit/tmux-sessions.md +++ b/docs/memory/run-kit/tmux-sessions.md @@ -164,6 +164,19 @@ A *specific existing window* is addressed by its stable tmux **window ID** (`@N` This identity is consistent across all layers — `@N` is the window identity everywhere in code/API: URL (`/$server/$window`, no `$session` segment; the page URL segment carries only `@N`'s numeric part, sans `@`, parse-restored to `@N` — `260703-8mpy-numeric-window-url`), HTTP API (`/api/windows/{windowId}/...`), the terminals-mux `open` op's `windowId` field (`260717-803u-relay-mux`), tmux targets, and the fab pane-map enrichment join. The window store and the boards feature (`@rk_board` stores `:...`) are windowID-keyed. Path params are validated by `validate.ValidateWindowID` (`^@[0-9]+$`) — stricter than `ValidateName`, which permits `@` but not the `@N` shape (constitution §I). Window IDs are never user-typed; they originate from tmux's `#{window_id}`. See `architecture.md` § API Layer and § Terminal Relay, and `ui-patterns.md` § URL Structure. +## Name-Validation Charset Contract (new-name tightening vs. permissive lookups) + +Session and window names carry a **two-tier validation contract** in `internal/validate` (`validate.go`), keyed on whether the value names a **to-be-created / renamed-to** entity or an **existing** one: + +- **New names** (create / rename-to targets) validate through `ValidateNewName(name, label)` — the permissive `ValidateName` rule plus a **"no spaces"** rejection. This makes the safe charset a real backend contract rather than only UI steering. Applied at exactly four handler call sites: session create and session rename-to (`api/sessions.go`), window create (non-empty name only) and window rename-to (`api/windows.go`). +- **Existing-name lookups** stay on the permissive `ValidateName` — session URL params for rename/color/kill/order (`api/sessions.go`), the `windows.go` session param and `MoveWindowToSession`'s `TargetSession`, the `upload.go` session param, and both `riff.go` session values (`handleRiffSpawn` `body.Session` and `handleRiffPresets` — riff derives the repo root from the named session's cwd, creating nothing under that name). Sessions/windows created outside run-kit (raw `tmux rename-session`) can carry spaces, so rename/kill/upload targeting a pre-existing spacey name must keep working. + +`ValidateName` itself is unchanged: non-empty, ≤ `MaxNameLength` (128), no `forbiddenChars` (`; & | ` backtick ` $ ( ) { } [ ] < > ! # * ?` plus control chars), no `:`/`.`. `ValidateNewName` layers over it with the single space check. + +**Hyphens stay legal on the backend for both tiers** — session names included. Internal sessions (`_rk-pin-*` pin-sessions, the `rk-test-*` sockets, session-group names) rely on hyphens, so rejecting them would break run-kit itself. The **session hyphen→`_` divergence is UI-only steering** (`ui-patterns.md` § Live Safe-Name Conversion): the frontend `toSafeSessionName` converts hyphens to avoid session-group naming collisions, while `toSafeWindowName` keeps them (`riff-*` windows use hyphens legitimately) — but the backend accepts hyphens for either kind. `ValidateServerName` (`^[a-zA-Z0-9_-]+$`) and `ValidateWorktreeName` (rejects spaces, leading `-`, `/`) are unchanged and already strict. + +**Compatibility posture** (user-accepted): a pre-existing spacey name remains fully operable (lookups permissive) but cannot be the *target* of a run-kit create/rename — the new-name rule rejects the space before any tmux call. `POST /api/sessions {"name":"My problem"}` returns 400 ("cannot contain spaces") and creates nothing. Table tests in `validate_test.go` cover space rejection, `ValidateName` inheritance, and boundary cases; handler tests in `api` cover create/rename space rejection plus a permissive spacey-old-name rename that succeeds. (`260722-ln4n-auto-safe-name-conversion`) + ## Exact-Match Session Targets (`=name:` / `=session:windowSpec`) **Every `internal/tmux` command that passes a session name as a `-t` target composes it through `tmux.ExactSessionTarget(session) → "=" + session + ":"`** (exported — `internal/riff` consumes it too) (`260717-hikh-exact-tmux-session-targets`). The leading `=` disables tmux's prefix/fnmatch name matching (exact match only); the trailing `:` forces the string to parse as a **session**, never a window name. Session-qualified *window* targets (`swap-window`, `select-window`) compose through the unexported `exactWindowInSession(session, windowSpec) → "=" + session + ":" + windowSpec`, where `windowSpec` is itself unambiguous within the session — a window ID (`@N`) or a numeric index. Both live in `internal/tmux/tmux.go`. @@ -401,7 +414,7 @@ A managed window (the plain sidebar/palette/board `+ New Window` paths — NOT ` ### API contract — window name optional on CREATE only -`POST /api/sessions/{session}/windows` (`handleWindowCreate`, `api/windows.go`) makes `name` optional: `validate.ValidateName(body.Name, ...)` runs **only when `body.Name != ""`**. An omitted/empty name (with no `rkType`) is a valid request meaning "let tmux auto-name" → 201, calling `CreateWindow(session, "", cwd, server)`. A **non-empty** name that fails validation returns 400. **Guard**: when `body.RkType != ""` the handler requires a non-empty name (400 on empty/omitted) — the `rkType` path runs `CreateWindowWithOptions` with `-n ` and automatic-rename disabled, so an empty name there would pin the window to an empty name; the shipped UI always supplies one, and the 400 pins that API contract. The rename handler (`handleWindowRename`) requires a non-empty rename name (empty → 400). Contract documented in `docs/specs/api.md` § window create. +`POST /api/sessions/{session}/windows` (`handleWindowCreate`, `api/windows.go`) makes `name` optional: `validate.ValidateNewName(body.Name, ...)` runs **only when `body.Name != ""`** (the tightened new-name rule — no spaces — since the name names a to-be-created window; see § Name-Validation Charset Contract). An omitted/empty name (with no `rkType`) is a valid request meaning "let tmux auto-name" → 201, calling `CreateWindow(session, "", cwd, server)`. A **non-empty** name that fails validation returns 400. **Guard**: when `body.RkType != ""` the handler requires a non-empty name (400 on empty/omitted) — the `rkType` path runs `CreateWindowWithOptions` with `-n ` and automatic-rename disabled, so an empty name there would pin the window to an empty name; the shipped UI always supplies one, and the 400 pins that API contract. The rename handler (`handleWindowRename`) requires a non-empty rename name validated via `ValidateNewName` (empty → 400). Contract documented in `docs/specs/api.md` § window create. Existing `-n zsh`-pinned windows are NOT migrated — they have `automatic-rename` off and stay pinned. Unpinning would require a per-window `set -w automatic-rename on` sweep (not trivially cheap). @@ -539,7 +552,7 @@ The `tmuxctl` supervisor is **unaffected**: it does NOT call `ListServers` — i - `app/backend/cmd/rk/reaper.go` — thin `reaperCmd` (top-level; `--prefix` default `rk-test`, `--yes`/`--force` action gate, `--dry-run` explicit-alias for the default preview); `act := (yes||force) && !dryRun`; calls `tmux.ReapTestServers(ctx, prefix, act, force)` and renders summary/dry-run (`renderReapSummary`/`renderDryRun`); `Long` help states the brute-force/no-liveness-protection/operating contract; no scan/probe/remove/kill in `cmd/rk` - `app/backend/internal/sessions/sessions.go` — `FetchSessions(server)` builds the dashboard view, `ProjectSession` has `Name` and `Windows` (no `Server` field); pane-map enrichment re-keys from `session:index` to windowID before joining; `ProjectRoot(ctx, windowID, server)` resolves by window ID - `app/backend/api/router.go` — `serverFromRequest()` helper, `TmuxOps` interface with server params, route registration -- `app/backend/api/windows.go` — window action handlers keyed by `/api/windows/{windowId}` (kill, move, move-to-session, rename, color, url/type PUT, keys, select, split, close-pane); `parseWindowID(r) (string, bool)` helper validates the path param; `handleWindowCreate` stays session-scoped and makes `name` **optional** (validated only when non-empty; empty ⇒ tmux auto-names), except the `rkType`-present branch which requires a non-empty name (400 otherwise) (`260707-j66b`). The rename handler requires a non-empty validated name +- `app/backend/api/windows.go` — window action handlers keyed by `/api/windows/{windowId}` (kill, move, move-to-session, rename, color, url/type PUT, keys, select, split, close-pane); `parseWindowID(r) (string, bool)` helper validates the path param; `handleWindowCreate` stays session-scoped and makes `name` **optional** (a non-empty name validated via `ValidateNewName`; empty ⇒ tmux auto-names), except the `rkType`-present branch which requires a non-empty name (400 otherwise) (`260707-j66b`). The rename handler validates the new name via `ValidateNewName` (non-empty, no spaces) while its `{windowId}` target is already ID-addressed — see § Name-Validation Charset Contract - `app/backend/api/servers.go` — server list/create/kill handlers - `app/backend/api/keybindings.go` — `GET /api/keybindings` handler (runs `list-keys`, filters via whitelist, returns JSON) - `app/backend/api/sse.go` — per-server SSE polling hub @@ -572,6 +585,12 @@ Reusable patterns future changes should follow: 6. **Prefer `link-window` over `move-window` when a window must appear on two surfaces.** A window that needs dual residence (here: home session in SESSIONS + pin-session on a board) is `link-window`'d into the second session rather than moved — tmux keeps it a member of both and destroys the window only when its LAST link is removed. This keeps both surfaces tmux-derived truth (Constitution II — no frontend ghost-row synthesis, no lost original index) and simplifies teardown: dropping the extra membership is a plain `kill-session` on the single-window session, and the window survives via its surviving link. *Introduced by*: `260718-co9z-link-based-board-pinning` +### Tighten new-name validation, keep existing-name lookups permissive +**Decision**: A tightened validator (`ValidateNewName` = `ValidateName` + no-spaces) guards only to-be-created / renamed-to names; every existing-name lookup keeps the permissive `ValidateName`. The backend stays reject-only (no server-side name conversion) and keeps hyphens legal for both name kinds. +**Why**: The safe charset must be a real backend contract, not merely UI steering — but tightening the lookup paths too would strand pre-existing spacey names (created outside run-kit) as un-renamable and un-killable. Splitting the rule by new-vs-existing keeps the charset honest for created entities while leaving legacy names operable. Server-side conversion is rejected because silently rewriting a name would desynchronize the client's optimistic-update view of what got created (constitution §I — the backend is the security boundary, not a name-fixer). +**Rejected**: (a) tighten `ValidateName` itself — breaks rename/kill/upload of pre-existing spacey names and internal hyphenated sessions; (b) backend-side conversion — breaks WYSIWYG/optimistic-update parity; (c) reject hyphens for sessions to mirror the UI's session hyphen→`_` steering — would reject `_rk-pin-*`, `rk-test-*`, and group names, breaking run-kit itself. +*Introduced by*: `260722-ln4n-auto-safe-name-conversion` + 7. **STAMP-BEFORE-LINK (write derivation metadata before the entity crosses the boundary).** When a mutation both creates a session and stamps the metadata that makes it derivable, stamp on the still-empty session BEFORE linking the payload window in. This removes the window-present-but-unstamped failure window entirely: a stamp failure strands nothing (the window has not moved), and a post-link failure can't exist because the metadata (`@rk_home`) is already durable — so the entity is always recoverable, with no double-fault rollback and no un-recoverable state. *Introduced by*: `260718-co9z-link-based-board-pinning` 8. **A last-link/recovery path must be collision-guarded and never strand.** Any recovery that renames a session to a reconstructed name (`recovered`, or a recreated `@rk_home`) probes with `has-session` first and, on collision, falls back (a bounded numeric-suffix probe for `recovered`; the recovered-name rename for a raced-in `@rk_home`). The contract "a window is never left unrecoverable" outranks landing at the ideal name — a stale prior-recovery session must not make the operation error and leave the window invisible in a `_rk-pin-*` name. *Introduced by*: `260718-co9z-link-based-board-pinning` diff --git a/docs/memory/run-kit/ui-patterns.md b/docs/memory/run-kit/ui-patterns.md index b64ed7c9..32f03d54 100644 --- a/docs/memory/run-kit/ui-patterns.md +++ b/docs/memory/run-kit/ui-patterns.md @@ -1490,6 +1490,43 @@ CWD display (line 1) uses `shortenPath()` to shorten the active pane's `cwd` (fa - Examples: `/home/sahil/code/org/repo/src` → `…/repo/src`; `/home/sahil/code/org` → `~/code/org`; `/var/log/nginx` → `…/log/nginx`. - The `title` attribute on the CWD element always contains the original unmodified `activePaneCwd` — hover to see the full path. +## Live Safe-Name Conversion + +Every naming entry point converts a user-typed name to its safe/canonical form **live, in the input's `onChange`** — the user watches "My problem" become "My_problem" as they type (WYSIWYG), so the optimistic-update name is identical to the committed name. The conversion is **per name kind**, sourced from the shared pure module `app/frontend/src/lib/names.ts` (the established `src/lib/*.ts` + colocated `*.test.ts` home for shared pure logic), which exports one transform per kind plus a commit finisher: + +| Export | Rule | +|--------|------| +| `toSafeSessionName(raw)` | spaces, **hyphens**, colons, periods, and the backend forbidden set (`; & | ` backtick ` $ ( ) { } [ ] < > ! # * ?` + control chars) → `_`; collapse `_` runs; strip leading `_`; **preserve case**; cap 128 (`MaxNameLength`). Hyphen→`_` is session-specific (session-group collision avoidance). | +| `toSafeWindowName(raw)` | same as session but **keeps hyphens** — `riff-*` windows use them legitimately and the group-collision rationale does not apply. | +| `toSafeServerName(raw)` | anything outside `[a-zA-Z0-9_-]` → `_`; collapse; strip leading `_`; cap 64 (`MaxServerNameLength`). Matches backend `ValidateServerName`. | +| `toSafeWorktreeName(raw)` | the window rule plus `/`→`_` and a leading `-` dropped (matching `ValidateWorktreeName`). | +| `finalizeSafeName(name)` | commit-time finisher — strips leading **and trailing** `_` runs. | +| `deriveNameFromPath(p)` | path-derived session-name suggestion — extracts the last path segment, composes the session transform + `finalizeSafeName` (both ends trimmed, matching the old `toTmuxSafeName`). | + +Case is preserved ("My problem" → "My_problem", never lowercased). Chars outside a kind's unsafe set (e.g. `@`, `%`, `+` for session/window names) pass through unchanged — this is conversion, not stripping. + +**Live-typing edges** (§ Design Decisions → Live transform + commit finalizer split): the live `onChange` transform strips **leading** unsafe chars as typed (a space pressed in an empty field produces nothing) and collapses `_` runs live, but a **trailing** `_` stays visible while typing — trimming it live would delete the separator the user just typed ("My " + "p" would become "Myp"). Commit/submit sites run `finalizeSafeName` to trim the trailing `_`. An input that is empty after conversion falls through to the existing empty-name submit guards — no new error surface. There is no caret-position management beyond React-controlled-input defaults; a mid-string length-changing edit may move the caret to the end (an accepted edge). + +**Wired entry points** (each applies the kind's transform in `onChange` and `finalizeSafeName` at commit): + +| Surface | File | Kind | +|---------|------|------| +| Create-session dialog typed-name field | `create-session-dialog.tsx` | session | +| Session rename dialog input | `app.tsx` (state in `use-dialog-state.ts`) | session | +| Sidebar inline session rename | `sidebar/session-row.tsx` | session | +| Top-bar `WindowHeading` inline rename (also the palette's "Window: Rename" CustomEvent path — § Window Heading) | `top-bar.tsx` | window | +| Sidebar inline window rename (commits via shared `use-window-rename.ts`) | `sidebar/window-row.tsx` | window | +| "New iframe window" name field | `app.tsx` (`handleCreateIframeWindow`) | window | +| Host Overview create-server dialog | `host-overview-page.tsx` | server | +| AppShell create-server dialog | `app.tsx` (`handleCreateServer`) | server | +| Spawn/riff worktree-name field | `spawn-agent-dialog.tsx` | worktree | + +The create dialog's **window mode has no name input** (windows are created unnamed and tmux auto-names them — § Unnamed `+ New Window`), so there is nothing to wire there. + +**Known gap — board-route create-server dialog is NOT wired.** The third server-name input, the board-route create-server dialog (`src/components/board/board-page.tsx`, input at `board-page.tsx:1179`), still does a raw `setCreateServerName(e.target.value)` with no live `toSafeServerName`, and its `handleCreateServerSubmit` skips `finalizeSafeName` — it only gates submit on the pre-existing regex disabled-guard (`/^[a-zA-Z0-9_-]+$/.test(trimmed)`). So on that one surface a typed space still produces the old hard-rejection UX (submit blocked) rather than live conversion. This diverges from the "every naming entry point converts" intent and is a tracked should-fix, not shipped behavior — the other two server inputs (host-overview + AppShell) do convert live. (`260722-ln4n-auto-safe-name-conversion`) + +The backend enforcement side of this contract (the tightened `ValidateNewName` for created/renamed-to names, permissive lookups for existing names) lives in [tmux-sessions](/run-kit/tmux-sessions.md) § Name-Validation Charset Contract and [architecture](/run-kit/architecture.md) § API Layer. + ## Session Creation Pattern ### Instant Creation (Primary) @@ -1497,7 +1534,7 @@ CWD display (line 1) uses `shortenPath()` to shorten the active pane's `cwd` (fa All primary session creation entry points create a session immediately without a dialog. Implemented by `executeCreateSessionInstant` in `app.tsx`. **Algorithm**: -1. Derive a name from the active window's `worktreePath` using `deriveNameFromPath(worktreePath)` (exported from `create-session-dialog.tsx`). If the result is empty (CWD is `/`, `~`, or `worktreePath` is undefined), the name is `session`. +1. Derive a name from the active window's `worktreePath` using `deriveNameFromPath(worktreePath)` (from the shared `@/lib/names` module — § Live Safe-Name Conversion). If the result is empty (CWD is `/`, `~`, or `worktreePath` is undefined), the name is `session`. 2. Deduplicate against `sessions`: if the name is taken, try `{name}-2`, `{name}-3`, … up to `{name}-10`. If all are taken, use `{name}-11` (best-effort). 3. Call `createSession(server, derivedName, worktreePath)`. If no active window exists, call `createSession(server, "session")` (no `cwd` — tmux defaults to server CWD). 4. The session appears in the sidebar via the existing optimistic/ghost mechanism. @@ -1509,7 +1546,7 @@ All primary session creation entry points create a session immediately without a - Top-bar breadcrumb session dropdown `+ New Session` item - Cmd+K "Session: Create" action -**Name derivation utilities**: `deriveNameFromPath` and `toTmuxSafeName` are exported from `app/frontend/src/components/create-session-dialog.tsx` so `app.tsx` can import them without duplicating logic. +**Name derivation utilities**: `deriveNameFromPath` and the per-kind safe-name transforms live in the shared `app/frontend/src/lib/names.ts` module (§ Live Safe-Name Conversion), imported by `app.tsx` and every naming surface. Name conversion is split per kind (`toSafeSessionName`/`toSafeWindowName`/`toSafeServerName`/`toSafeWorktreeName`) — there is no single `toTmuxSafeName`. ### Folder-Prompted Creation (Secondary, via Cmd+K) @@ -1533,7 +1570,7 @@ The `+ New Window` action does **not** pin the window name to `"zsh"` — it cal **All three unnamed-window call sites:** -1. **Sidebar `+ New Window`** (`app.tsx`, the `create-window` "Window: Create" palette action) — `createWindow(srv, session, undefined, activeWin?.worktreePath)`. Its optimistic-ghost label becomes the **raw** (unsanitized) basename of the creation cwd via the local `rawBasename(cwd)` helper (falls back to `"window"` when no cwd). This is deliberately **NOT** `deriveNameFromPath` — that runs `toTmuxSafeName` (`.`→`_` etc.), but tmux's `#{b:pane_current_path}` uses the unsanitized basename, so the ghost must match the raw form tmux will actually display. +1. **Sidebar `+ New Window`** (`app.tsx`, the `create-window` "Window: Create" palette action) — `createWindow(srv, session, undefined, activeWin?.worktreePath)`. Its optimistic-ghost label becomes the **raw** (unsanitized) basename of the creation cwd via the local `rawBasename(cwd)` helper (falls back to `"window"` when no cwd). This is deliberately **NOT** `deriveNameFromPath` — that runs the session safe-name transform (`.`→`_` etc.), but tmux's `#{b:pane_current_path}` uses the unsanitized basename, so the ghost must match the raw form tmux will actually display. 2. **Palette "Window: Create at Folder"** (`create-session-dialog.tsx`, `mode="window"`) — `createWindow(server, session, undefined, cwd)`; this flow sets **no** optimistic ghost, so no label is derived. 3. **Board-route `+ New Window`** (`board-page.tsx`) — `createWindowApi(srv, sess)` (no name, no cwd — the backend resolves a default; this action registers no ghost). Identical behavior to the same button on `/$server`. @@ -1970,9 +2007,9 @@ The "Create session" dialog (breadcrumb `+ New Session` action, sidebar empty st 2. **Path input with autocomplete** — Text input that calls `GET /api/directories?prefix=...` with ~300ms debounce. Results appear as a dropdown below the input. Selecting a result fills the path and triggers a new autocomplete for children. Hidden directories (`.`-prefixed) are excluded from results. -3. **Session name** — Auto-derived from the last segment of the selected path (e.g., `~/code/sahil87/run-kit` yields `run_kit`). Editable — auto-derivation is a convenience, not a lock. When the name field is left empty at submit time, the name is derived from the path automatically via `deriveNameFromPath()`. The Create button is enabled when either a name or a path is provided. +3. **Session name** — Auto-derived from the last segment of the selected path (e.g., `~/code/sahil87/run-kit` yields `run_kit`). Editable — auto-derivation is a convenience, not a lock. The typed name field converts **live** as the user types via `toSafeSessionName` (§ Live Safe-Name Conversion) — a typed space appears as `_`. When the name field is left empty at submit time, the name is derived from the path automatically via `deriveNameFromPath()`. The Create button is enabled when either a name or a path is provided. -On submit, the dialog calls `createSession(server, name, cwd)` which sends `POST /api/sessions?server={server}` with `{ name, cwd }`. If the name field is empty but a path is set, the name is derived from the path's last segment (sanitized for tmux/byobu: hyphens→underscores, colons/periods replaced with underscores). Collision with existing session names is checked on the derived name and shows an error. The `cwd` field is omitted when no path is selected, preserving the original name-only behavior. Accessible from breadcrumb `+ New Session` dropdown action, sidebar empty state button, and command palette. +On submit, the dialog calls `createSession(server, name, cwd)` (the typed name run through `finalizeSafeName` first — trailing `_` trimmed, § Live Safe-Name Conversion) which sends `POST /api/sessions?server={server}` with `{ name, cwd }`. If the name field is empty but a path is set, the name is derived from the path's last segment via `deriveNameFromPath` (the session transform: hyphens→underscores, spaces and colons/periods and the forbidden set → underscores). Collision with existing session names is checked on the finalized/derived name and shows an error. The `cwd` field is omitted when no path is selected, preserving the original name-only behavior. Accessible from breadcrumb `+ New Session` dropdown action, sidebar empty state button, and command palette. ## Spawn-Agent Dialog @@ -1983,7 +2020,7 @@ On submit, the dialog calls `createSession(server, name, cwd)` which sends `POST 1. **Task** — free text, optional, autofocused; empty spawns a blank agent session. 2. **Preset** — a dropdown fetched on open (see preflight below), shown **only when the repo defines presets** (`presets.length > 0`); each option renders `name — layout (N panes)`. 3. **Where** — a `role="radiogroup"` with two options: **new worktree** (default, `where="worktree"`) and **this checkout** (`where="checkout"`, roots the window in the existing checkout — no worktree created). State is the typed `RiffWhere = "worktree" | "checkout"` union (no `as` casts). -4. **Worktree** — a text input, blank by default, placeholder `auto-named (e.g. swift-fox)`; **hidden when `where === "checkout"`** (it has no meaning there). Blank = wt auto-names; a typed name rides `worktreeName` through to `wt create --worktree-name`. No pre-filled suggestion — `wt` exposes no name-suggest seam and constitution §III forbids reimplementing its generator, so the placeholder is the honest fallback. +4. **Worktree** — a text input, blank by default, placeholder `auto-named (e.g. swift-fox)`; **hidden when `where === "checkout"`** (it has no meaning there). Converts **live** via `toSafeWorktreeName` as the user types (§ Live Safe-Name Conversion) — spaces → `_`, `/` → `_`, a leading `-` dropped (matching `ValidateWorktreeName`), hyphens kept. Blank = wt auto-names; a typed name rides `worktreeName` through to `wt create --worktree-name`. No pre-filled suggestion — `wt` exposes no name-suggest seam and constitution §III forbids reimplementing its generator, so the placeholder is the honest fallback. 5. **Agent** — a tier dropdown populated from the preflight's `tiers` (built-ins ∪ the repo's `agent.tiers`, `default` first), defaulting to the `DEFAULT_TIER = "default"` constant. Displays tier **names only** (no model IDs — brittle to parse out of command strings). **FAB-GATED**: the field renders **only when `tiers.length > 0`**. The backend fab-gates the presets response (`tiers: []` for a non-fab repo — see [architecture](/run-kit/architecture.md) § API Layer `/api/riff/presets`), so an empty list HIDES the field entirely — no label, no hint text, no disabled control — because in a non-fab repo every tier resolves to the same `DefaultLauncher` (inert noise). When hidden, `tier` is omitted from the spawn body (see § Defaults). Non-fab-repo hiding does NOT enumerate system agents (claude/codex/gemini) — tiers remain fab's abstraction (constitution §III). (`gsmu`) **Enter submits from any field** (`handleKeyDown` wired on all inputs incl. the radios and both selects). **Preflight fetch** (`getRiffPresets(server, session)`) is **best-effort**, with two branches: (`gsmu`) @@ -2225,6 +2262,12 @@ The regression test in `app/frontend/src/hooks/use-dialog-state.test.tsx` flips ## Design Decisions +### Live safe-name transform + commit finalizer split +**Decision**: The live `onChange` transform strips leading separators but keeps a trailing `_` visible; a separate `finalizeSafeName` trims leading+trailing `_` at commit. Name conversion happens live in the input (not silently at submit), per name kind, in one shared `src/lib/names.ts` module. +**Why**: Converting live gives WYSIWYG — the user sees exactly the name that will exist, so the optimistic-update name equals the committed name. Keeping the trailing `_` visible while typing is required so "My " + "p" does not collapse to "Myp" (trimming the trailing separator live would delete the separator the user just typed and break mid-word entry); commit-trim is the one minimal WYSIWYG deviation. One transform per kind in `src/lib/` (the shared-pure-logic home) keeps every entry point on one canonical rule with colocated Vitest coverage. +**Rejected**: (a) backend-side conversion — breaks WYSIWYG and optimistic-update parity (the backend stays reject-only, constitution §I); (b) submit-time silent conversion — the user commits a name they never saw; (c) keeping reject-only frontend errors (the old server-name UX) — explicitly the experience this moves away from; (d) strict-WYSIWYG commit (trailing `_` included) — leaves awkward trailing separators for no benefit. +*Introduced by*: 260722-ln4n-auto-safe-name-conversion + ### Sessions-pane scope is explicit state, not the SERVER panel's expansion **Decision**: The sessions-pane scope (`all | current`) is its own persisted state (`runkit-panel-sessions-scope`), read via `useSessionsScope`, with no migration from the old `runkit-panel-server` coupling; the SERVER panel defaults open and its expansion no longer affects the session list. **Why**: "do I want to see server tiles?" and "which servers' sessions do I want listed?" are unrelated concerns that previously shared one bit — so the SERVER panel couldn't default open without permanently filtering the list, and users couldn't filter the list without paying the tile grid's vertical cost. `runkit-panel-server` encodes panel visibility, not scope intent, so carrying it over would guess wrong as often as right. diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl b/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl new file mode 100644 index 00000000..5a351f04 --- /dev/null +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl @@ -0,0 +1,12 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-07-22T11:02:37Z"} +{"args":"Automatic conversion of user-typed names to safe/canonical forms across all naming entry points (sessions, windows, servers) — live in-input conversion, shared frontend transform module, backend ValidateName tightened to reject spaces on new names","cmd":"fab-new","event":"command","ts":"2026-07-22T11:02:37Z"} +{"delta":"+3.6","event":"confidence","score":3.6,"trigger":"calc-score","ts":"2026-07-22T11:05:37Z"} +{"cmd":"fab-switch","event":"command","ts":"2026-07-22T11:06:43Z"} +{"cmd":"fab-fff","event":"command","ts":"2026-07-22T11:07:48Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-07-22T11:08:16Z"} +{"cmd":"fab-continue","event":"command","ts":"2026-07-22T11:08:56Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-07-22T11:43:14Z"} +{"cmd":"fab-continue","event":"command","ts":"2026-07-22T11:43:49Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-07-22T11:51:05Z"} +{"event":"review","result":"passed","ts":"2026-07-22T11:51:05Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-07-22T11:58:02Z"} diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml b/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml new file mode 100644 index 00000000..773ad064 --- /dev/null +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml @@ -0,0 +1,54 @@ +id: ln4n +name: 260722-ln4n-auto-safe-name-conversion +created: 2026-07-22T11:02:37Z +created_by: sahil-noon +change_type: refactor +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: active + review-pr: pending +plan: + generated: true + task_count: 12 + acceptance_count: 17 + acceptance_completed: 17 +confidence: + certain: 6 + confident: 6 + tentative: 1 + unresolved: 0 + score: 3.6 + fuzzy: true + dimensions: + signal: 68.8 + reversibility: 77.7 + competence: 81.5 + disambiguation: 78.8 +stage_metrics: + intake: {started_at: "2026-07-22T11:02:37Z", driver: fab-new, iterations: 1, completed_at: "2026-07-22T11:08:16Z"} + apply: {started_at: "2026-07-22T11:08:16Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:43:14Z"} + review: {started_at: "2026-07-22T11:43:14Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:51:05Z"} + hydrate: {started_at: "2026-07-22T11:51:05Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:58:02Z"} + ship: {started_at: "2026-07-22T11:58:02Z", driver: fab-fff, iterations: 1} +prs: [] +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + tests: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-07-22T11:58:02Z" + computed_at_stage: hydrate +summary: live per-kind safe-name conversion (src/lib/names.ts) at naming entry points + backend ValidateNewName no-spaces tightening for created/renamed-to names +# true_impact: lazily created on first stage-finish that computes it (no placeholder here). +last_updated: 2026-07-22T11:58:02Z diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/intake.md b/fab/changes/260722-ln4n-auto-safe-name-conversion/intake.md new file mode 100644 index 00000000..ab898b76 --- /dev/null +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/intake.md @@ -0,0 +1,105 @@ +# Intake: Automatic Safe-Name Conversion at Naming Entry Points + +**Change**: 260722-ln4n-auto-safe-name-conversion +**Created**: 2026-07-22 + +## Origin + +Promptless dispatch (`/fab-proceed` create-intake, `{questioning-mode} = promptless-defer`) from a synthesized user-conversation description. The user explicitly confirmed the six design decisions recorded below during that discussion; no questions were asked at intake time (deferred-Unresolved contract — none were needed, see `## Assumptions`). + +> Feature: automatic conversion of user-typed names to safe/canonical forms across all naming entry points (sessions, windows, servers). When a user types "My problem" while creating or renaming a session/window, run-kit today either passes it through (spaces are legal in the backend's `ValidateName`) or rejects it with an error (server names). The user wants automatic conversion instead: "My problem" → "My_problem", applied live at input time, per-name-kind charset, with the backend tightened so the charset is a real contract. + +## Why + +1. **The pain point**: run-kit's name validation is inconsistent and leaky. `ValidateName` (sessions + windows, `app/backend/internal/validate/validate.go`) rejects shell metacharacters, colons, and periods — but **allows spaces**. A session literally named `My problem` gets created, and then bites three downstream consumers: tmux CLI targeting (space-splitting in ad-hoc commands), session-group naming, and the `/$server/$window` URL routes. Server names take the opposite posture — strict `^[a-zA-Z0-9_-]+$` — so a typed space there produces a hard rejection error instead. Neither experience is what the user wants: typing a natural name should *just work*, converted to the safe form. + +2. **The consequence of not fixing**: users keep creating spacey sessions that misbehave in routing and tmux targeting, or keep hitting opaque validation errors on server creation. The existing `toTmuxSafeName()` conversion helps only path-derived name *suggestions* — anything the user actually types in a name field is sent as-is (rename flows do only `trim()`), so the one place conversion exists doesn't cover the primary input path. + +3. **Why this approach**: automatic, *live*, per-kind conversion at every naming entry point (frontend), plus a *tightened* reject-only backend. Conversion in the UI gives the "it just works" experience and WYSIWYG (the user watches "My problem" become "My_problem" as they type, so the optimistic-update name is identical to the committed name). The backend stays reject-only — per constitution §I it is the security boundary, and silently rewriting names server-side would desynchronize the client's view of what got created — but it tightens (`ValidateName` rejects spaces on NEW names) so the charset is the real contract, not just UI steering. Alternatives rejected in discussion: backend-side conversion (breaks WYSIWYG and optimistic naming), submit-time silent conversion (user commits a name they never saw), and keeping reject-only frontend errors (the current server-name UX, explicitly what the user wants to move away from). + +## What Changes + +### 1. Shared frontend name-transform module — `app/frontend/src/lib/names.ts` (new) + +Promote `toTmuxSafeName()` out of `app/frontend/src/components/create-session-dialog.tsx:24` into a new shared module `app/frontend/src/lib/names.ts` (the established home for shared pure logic — see `src/lib/*.ts` + colocated `*.test.ts` pattern). One canonical transform per name kind: + +- **`toSafeSessionName(raw)`** — the current `toTmuxSafeName` rule *plus spaces*: converts spaces, hyphens, colons, periods, and every char in the backend forbidden set (`; & | ` + backtick + `$ ( ) { } [ ] < > ! # * ?` + control chars `\n \r \t`) to `_`; collapses `_` runs; preserves case ("My problem" → "My_problem", not "my_problem"); caps at 128 chars (backend `MaxNameLength`). The hyphen→`_` rule is **session-specific** — it exists to avoid collisions with session-group naming. +- **`toSafeWindowName(raw)`** — same rule but **keeps hyphens** (user-confirmed divergence): window names like `riff-foo` use hyphens legitimately, and the group-collision rationale doesn't apply to windows. +- **`toSafeServerName(raw)`** — stricter: anything outside `[a-zA-Z0-9_-]` converts to `_` (matches backend `ValidateServerName`'s `^[a-zA-Z0-9_-]+$`); collapses `_` runs; caps at 64 chars (`MaxServerNameLength`). +- **`toSafeWorktreeName(raw)`** — the window rule plus `ValidateWorktreeName`'s extra constraints: `/` converts to `_`, no leading hyphen (a leading `-` is dropped/converted), spaces already convert via the base rule. + +`deriveNameFromPath()` (path-derived suggestions) keeps its current behavior, now calling the session transform from the shared module. Existing behavior of converting unsafe chars **to `_`** (not stripping them) is retained for the whole forbidden set, so consecutive converted chars collapse to one `_` — matching the existing `toTmuxSafeName` shape. + +Each transform is a pure function with Vitest coverage in `app/frontend/src/lib/names.test.ts`. + +### 2. Live in-input conversion at every naming entry point + +The transform is applied **in the input's `onChange`, as the user types** — press space, an underscore appears. Not silently at submit: the user sees exactly the name that will exist, and the optimistic-update name (the window-rename store stamps the typed name before the API responds) is identical to the committed name. Verified entry-point inventory (all currently do raw `setState` / `trim()`-only): + +| Surface | File | Transform | +|---------|------|-----------| +| Create-session dialog typed-name field (session mode) | `src/components/create-session-dialog.tsx` (`setName`) | session | +| Create-window mode of the same dialog | same file, `mode === "window"` | window | +| Session rename dialog | input at `src/app.tsx` (~line 2795), state in `src/hooks/use-dialog-state.ts` | session | +| Sidebar inline session rename | `src/components/sidebar/index.tsx` (`editingSessionName`) | session | +| Top-bar `WindowHeading` inline rename (also serves the palette's "Window: Rename" via CustomEvent — `top-bar.tsx:1471`, registered at `app.tsx:1783`) | `src/components/top-bar.tsx` (input ~line 1512) | window | +| Sidebar inline window rename (commits via shared `use-window-rename.ts`) | `src/components/sidebar/index.tsx` (`editingName`) | window | +| Server-name input | `src/components/host-overview-page.tsx` (input ~line 473 → `createServer`) | server | +| Riff/spawn dialog worktree-name field | `src/components/spawn-agent-dialog.tsx` (~line 232) | worktree | + +Live-typing edge handling: pure charset conversion is 1:1 (caret position is stable); the length-changing pieces are handled so the transform never fights the user mid-word — leading unsafe chars are dropped as typed (a space pressed in an empty field produces nothing), `_` runs collapse live, and a *trailing* `_` remains visible while typing and is trimmed at commit/submit (the one minimal deviation from strict WYSIWYG, since trimming it live would make "My " + "p" become "Myp"). + + +### 3. Backend: `ValidateName` tightens for NEW names; existing-name lookups stay permissive + +Backend remains **reject-only** (no server-side conversion — constitution §I, validation before subprocess, backend is the security boundary), but the charset tightens so it is the real contract: + +- Add a tightened new-name rule (working shape: `ValidateNewName(name, label)` layering "no spaces" over `ValidateName`; exact factoring decided at plan time) and apply it at every call site where the value names a **to-be-created or to-be-renamed-to** entity: session create (`api/sessions.go:31`), session rename new-name (`api/sessions.go:73`), window create name (`api/windows.go:40`), window rename name (`api/windows.go:175`). +- **Current/old-name lookups stay on the permissive `ValidateName`** — sessions/windows created outside run-kit (raw `tmux rename-session`) can still carry spaces, and rename/kill/upload targeting an existing spacey name must keep working: `api/sessions.go:60,88,166,188`, `api/windows.go:19`, `api/windows.go:330` (move `TargetSession` — an existing session), `api/upload.go:23`, `api/riff.go:224`. `api/riff.go:123` (`body.Session`) is classified new-vs-existing at plan time from the handler's semantics. +- `ValidateServerName` is already strict — unchanged. `ValidateWorktreeName` already rejects spaces — unchanged (its own space rule becomes redundant if rebased on the tightened rule; keep it explicit either way). +- Backend **keeps allowing hyphens in session names**: internal sessions (`_rk-pin-*`, `rk-test-e2e`, group names) rely on hyphens; the session hyphen→`_` rule is UI-only steering, deliberately NOT a backend rejection. +- Go table tests in `app/backend/internal/validate/validate_test.go` for the tightened/added validator (space rejection, boundary cases, existing-name permissiveness), plus handler-level coverage where api tests exist. + +### 4. Tests and companion docs + +- **Vitest**: `names.test.ts` table-style cases per transform ("My problem" → "My_problem", hyphen divergence session vs window, case preservation, collapse/trim, length caps, forbidden-set conversion); component-level input-behavior tests where the surfaces already have colocated tests. +- **Go**: table tests for the new/tightened validators. +- **Playwright**: existing e2e specs assert on chrome rename flows (`tests/e2e/window-heading.spec.ts` — click-to-rename heading, inline input commit; also rename touchpoints in `sidebar-keyboard-nav`, `sidebar-window-sync`, `new-window-unnamed`, `sync-latency`, `echo-latency`). Grep/verify these against the new live-conversion behavior; extend `window-heading.spec.ts` (or add a sibling spec) with a typed-space → underscore live-conversion assertion. Per constitution **Test Companion Docs**, any `.spec.ts` change ships with its sibling `.spec.md` update in the same commit. + +## Affected Memory + +- `run-kit/ui-patterns`: (modify) — naming entry points now apply live per-kind safe-name conversion (shared `src/lib/names.ts` transforms; WYSIWYG typing behavior across create/rename dialogs, inline renames, server input, worktree field) +- `run-kit/tmux-sessions`: (modify) — session/window name charset contract: tightened new-name validation (no spaces), permissive existing-name lookups, session-vs-window hyphen divergence +- `run-kit/architecture`: (modify) — validate package: new-name vs existing-name rule split in the REST API layer + +## Impact + +- **Backend**: `app/backend/internal/validate/validate.go` (+ `validate_test.go`); call-site updates in `app/backend/api/sessions.go`, `app/backend/api/windows.go`, `app/backend/api/riff.go` (classification only), `app/backend/api/upload.go` (stays permissive — verify only). +- **Frontend**: new `app/frontend/src/lib/names.ts` + `names.test.ts`; `src/components/create-session-dialog.tsx` (transform promotion + typed-field wiring); `src/app.tsx` (session-rename dialog input, palette registration untouched); `src/hooks/use-dialog-state.ts` (rename state seam if conversion lives there); `src/components/sidebar/index.tsx` (two inline renames); `src/components/top-bar.tsx` (WindowHeading input); `src/components/host-overview-page.tsx` (server input); `src/components/spawn-agent-dialog.tsx` (worktree field). `src/hooks/use-window-rename.ts` unchanged in contract (callers own name hygiene per its doc comment) but its callers now pass converted names. +- **E2E**: `app/frontend/tests/e2e/window-heading.spec.ts` (+ `.spec.md`) and any rename-adjacent spec whose assertions the live conversion touches. +- **No API surface change**: same endpoints, same request shapes; only accepted-charset semantics tighten for new names. No routes added (constitution §IV). Uniform POST unchanged (§IX). +- **Compatibility caveat (user-accepted)**: pre-existing spacey names created outside run-kit remain operable (lookups permissive) but can no longer be the *target* of a run-kit create/rename. + +## Open Questions + +- None — all high-blast-radius decisions were explicitly confirmed in the originating discussion; remaining sub-decisions are graded in `## Assumptions` (no Unresolved rows were deferred). + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | One canonical transform per name kind in one shared frontend module (`src/lib/names.ts`), promoting `toTmuxSafeName` out of `create-session-dialog.tsx` | Explicit user decision 1; `src/lib/` is the established shared-pure-logic home | S:90 R:85 A:95 D:95 | +| 2 | Certain | Windows keep hyphens; sessions convert hyphens→`_` | Explicit user decision 2; hyphen rule is session-group-specific, `riff-*` windows use hyphens legitimately | S:95 R:80 A:90 D:95 | +| 3 | Certain | Preserve case ("My problem" → "My_problem", not lowercased) | Explicit user decision 3 | S:95 R:90 A:95 D:100 | +| 4 | Certain | Convert live in the input as the user types, not silently at submit | Explicit user decision 4; keeps optimistic-update name identical to committed name | S:90 R:70 A:90 D:90 | +| 5 | Certain | Apply at every naming entry point (create dialog both modes, session rename dialog + sidebar inline, window inline renames incl. palette path, server input, worktree field) | Explicit user decision 5; inventory verified against the codebase in this intake | S:85 R:75 A:90 D:90 | +| 6 | Certain | Backend stays reject-only but tightens: NEW names reject spaces; existing-name lookups stay permissive so pre-existing spacey names remain operable | Explicit user decision 6 including the accepted caveat | S:90 R:70 A:85 D:90 | +| 7 | Confident | Unsafe/forbidden chars convert to `_` (then runs collapse) rather than being stripped | Matches existing `toTmuxSafeName` shape; stripping would silently shorten names and lose word boundaries | S:55 R:85 A:75 D:65 | +| 8 | Confident | Backend factoring: keep permissive `ValidateName` for existing-name lookups; add a tightened variant (working name `ValidateNewName`) at create/rename-target call sites | Smallest-diff shape honoring decision 6; exact function name/factoring is plan-time detail | S:60 R:75 A:80 D:70 | +| 9 | Confident | Backend keeps allowing hyphens in session names; session hyphen→`_` is UI-only steering | Internal sessions (`_rk-pin-*`, `rk-test-e2e`, group names) carry hyphens; rejecting them would break run-kit itself | S:55 R:70 A:85 D:75 | +| 10 | Confident | Transforms cap length live at backend maxima (128 session/window, 64 server) | Backend constants are the contract; live cap preserves WYSIWYG at the boundary | S:45 R:85 A:80 D:75 | +| 11 | Confident | Empty-after-conversion input (e.g. only unsafe chars typed) falls through to the existing empty-name submit guards — no new error surface | All surfaces already guard `trim() === ""` before committing | S:50 R:80 A:80 D:75 | +| 12 | Confident | `api/riff.go:123` (`body.Session`) new-vs-existing classification resolved at plan time from handler semantics; worktree-name backend rule (`ValidateWorktreeName`) unchanged | Codebase answers this deterministically; no user preference involved | S:50 R:75 A:75 D:70 | +| 13 | Tentative | Live-typing edges: leading unsafe chars dropped as typed; trailing `_` stays visible while typing and is trimmed at commit (minimal WYSIWYG deviation at the tail) | Trimming the trailing `_` live would delete the just-typed separator and break mid-word entry; commit-trim vs keep-trailing are both defensible — easily revisited | S:35 R:70 A:40 D:35 | + +13 assumptions (6 certain, 6 confident, 1 tentative, 0 unresolved). diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/plan.md b/fab/changes/260722-ln4n-auto-safe-name-conversion/plan.md new file mode 100644 index 00000000..f7baabac --- /dev/null +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/plan.md @@ -0,0 +1,188 @@ +# Plan: Automatic Safe-Name Conversion at Naming Entry Points + +**Change**: 260722-ln4n-auto-safe-name-conversion +**Intake**: `intake.md` + +## Requirements + +### Frontend: Shared Name-Transform Module + +#### R1: One canonical transform per name kind in `src/lib/names.ts` +A new shared module `app/frontend/src/lib/names.ts` SHALL export one pure transform per name kind, promoting the logic out of `create-session-dialog.tsx`'s `toTmuxSafeName` (which is removed): + +- `toSafeSessionName(raw)` — converts spaces, **hyphens**, colons, periods, and every char in the backend forbidden set (`; & | ` backtick ` $ ( ) { } [ ] < > ! # * ?` plus control chars `\n \r \t`) to `_`; collapses `_` runs; strips leading `_`; preserves case; caps at 128 chars (backend `MaxNameLength`). The hyphen→`_` rule is session-specific (session-group collision avoidance). +- `toSafeWindowName(raw)` — same rule but **keeps hyphens**. +- `toSafeServerName(raw)` — converts anything outside `[a-zA-Z0-9_-]` to `_`; collapses; strips leading `_`; caps at 64 chars (`MaxServerNameLength`). +- `toSafeWorktreeName(raw)` — the window rule plus `/`→`_` and leading hyphens stripped (no leading `-` per `ValidateWorktreeName`). +- `finalizeSafeName(name)` — commit-time finisher: strips leading/trailing `_` runs (the trailing `_` stays visible while typing and is trimmed only at commit). +- `deriveNameFromPath(p)` — moved here from `create-session-dialog.tsx`; keeps its segment-extraction behavior, now composing the session transform + finalize (so both ends are trimmed exactly as the old `toTmuxSafeName` did). + +Chars outside the listed unsafe sets (e.g. `/`, `@`, `%`, `+` for session/window names) pass through unchanged — conversion, not stripping, only for the listed set. + +- **GIVEN** the raw input `"My problem"` **WHEN** `toSafeSessionName` is applied **THEN** the result is `"My_problem"` (case preserved, space converted). +- **GIVEN** the raw input `"riff-foo bar"` **WHEN** `toSafeWindowName` is applied **THEN** the result is `"riff-foo_bar"` (hyphen kept, space converted). +- **GIVEN** the raw input `"a b!c"` **WHEN** `toSafeServerName` is applied **THEN** the result is `"a_b_c"`. +- **GIVEN** the raw input `"-agent/x y"` **WHEN** `toSafeWorktreeName` is applied **THEN** the result is `"agent_x_y"`. +- **GIVEN** the live value `"My_problem_"` **WHEN** `finalizeSafeName` runs at commit **THEN** the committed name is `"My_problem"`. + +#### R2: Live conversion at session-name entry points +Every session-name input SHALL apply `toSafeSessionName` in its `onChange` (the user watches the conversion as they type): the create-session dialog's typed-name field (`create-session-dialog.tsx`), the session rename dialog input (`app.tsx`, state in `use-dialog-state.ts`), and the sidebar inline session rename (`sidebar/session-row.tsx` input). + +- **GIVEN** the create-session dialog **WHEN** the user types `"My problem"` in the name field **THEN** the input displays `"My_problem"` and the created session is named `"My_problem"`. +- **GIVEN** a session rename input **WHEN** the user types a hyphen **THEN** an underscore appears (session-specific hyphen steering). +- **GIVEN** an empty name field **WHEN** the user presses space **THEN** the field stays empty (leading unsafe chars are dropped as typed). + +#### R3: Live conversion at window-name entry points +Every window-name input SHALL apply `toSafeWindowName` in its `onChange`: the top-bar `WindowHeading` inline rename (`top-bar.tsx`, which also serves the palette's "Window: Rename" CustomEvent path), the sidebar inline window rename (`sidebar/window-row.tsx` input), and the "New iframe window" name field (`app.tsx`). The create dialog's window mode has no name input (windows are created unnamed and auto-named by tmux) — nothing to wire there. + +- **GIVEN** the window heading inline edit **WHEN** the user types `"my problem"` **THEN** the input displays `"my_problem"` and Enter commits `"my_problem"`. +- **GIVEN** a window rename input **WHEN** the user types `"riff-foo"` **THEN** hyphens are preserved (window-kind divergence). + +#### R4: Live conversion at server-name entry points +Both server-name inputs SHALL apply `toSafeServerName` in their `onChange`: the Host Overview create-server dialog (`host-overview-page.tsx`) and the AppShell create-server dialog (`app.tsx`). The existing regex-based disabled guards remain as defense in depth (they always pass post-transform). + +- **GIVEN** a server-name input **WHEN** the user types `"my server!"` **THEN** the input displays `"my_server_"` and the created server is `"my_server"` (trailing `_` trimmed at commit). + +#### R5: Live conversion at the worktree-name entry point +The riff/spawn dialog's worktree-name field (`spawn-agent-dialog.tsx`) SHALL apply `toSafeWorktreeName` in its `onChange`. + +- **GIVEN** the worktree field **WHEN** the user types `"-my agent"` **THEN** the input displays `"my_agent"` (leading hyphen dropped, space converted). + +#### R6: Commit-time finalization and empty-name fall-through +Every commit/submit site whose input carries a live transform SHALL apply `finalizeSafeName` before use (create-session `handleCreate` incl. the collision check, `use-dialog-state.handleRenameSession`, sidebar `handleSessionRenameCommit`/`handleRenameCommit`, top-bar `commit`, `handleCreateServer` in both server dialogs, `handleCreateIframeWindow`, spawn-dialog submit). An input that is empty after conversion falls through to the existing empty-name guards — no new error surface. + +- **GIVEN** a rename input holding `"My_"` (trailing separator kept live) **WHEN** the user commits **THEN** the API receives `"My"`. +- **GIVEN** an input where only unsafe chars were typed (live value empty) **WHEN** the user commits **THEN** the existing empty-commit guard cancels; no API call is made. + +### Backend: New-Name Charset Tightening + +#### R7: `ValidateNewName` rejects spaces for to-be-created / renamed-to names +`app/backend/internal/validate/validate.go` SHALL gain `ValidateNewName(name, label string) string` layering "no spaces" over the permissive `ValidateName`, applied at exactly the four call sites where the value names a to-be-created or renamed-to entity: session create (`api/sessions.go` `handleSessionCreate` body.Name), session rename new-name (`handleSessionRename` body.Name), window create name (`api/windows.go` `handleWindowCreate` non-empty body.Name), window rename name (`handleWindowRename` body.Name). The backend stays reject-only (no server-side conversion) and keeps allowing hyphens in session names (UI-only steering). + +- **GIVEN** `POST /api/sessions` with `{"name": "My problem"}` **WHEN** the handler validates **THEN** it returns 400 with a "cannot contain spaces" error and creates nothing. +- **GIVEN** `POST /api/windows/@N/rename` with `{"name": "a b"}` **WHEN** the handler validates **THEN** it returns 400. +- **GIVEN** `POST /api/sessions` with `{"name": "my-session"}` **THEN** it succeeds (hyphens legal on the backend). + +#### R8: Existing-name lookups stay permissive +All call sites where the value names an **existing** entity SHALL keep the permissive `ValidateName`: session URL params for rename/color/kill (`sessions.go`), session-order names, `windows.go` session param and `TargetSession` (move), `upload.go` session param, and both `riff.go` session values (`handleRiffSpawn` body.Session and `handleRiffPresets` — classified as existing: riff derives the repo root from the named session's cwd; nothing is created with that name). Pre-existing spacey names created outside run-kit remain operable but cannot be the target of a run-kit create/rename. `ValidateServerName` and `ValidateWorktreeName` are unchanged. + +- **GIVEN** a session literally named `"My problem"` (created via raw tmux) **WHEN** the user renames it via `POST /api/sessions/My%20problem/rename` with `{"name": "My_problem"}` **THEN** the rename succeeds (spacey old name accepted, tightened new name passes). + +### Testing + +#### R9: Coverage across all three layers with companion docs +Vitest table tests SHALL cover each transform in `src/lib/names.test.ts` (charset conversion per kind, hyphen divergence, case preservation, collapse/leading-strip/finalize, length caps, empty results); Go table tests SHALL cover `ValidateNewName` plus handler-level space-rejection and permissive-old-name cases; Playwright SHALL gain a typed-space → underscore live-conversion assertion in `window-heading.spec.ts`, and `sync-latency.spec.ts`'s UI session-rename expectations SHALL be updated for the hyphen→`_` session conversion. Per the constitution's Test Companion Docs rule, every touched `.spec.ts` ships its sibling `.spec.md` update in the same commit. + +- **GIVEN** the e2e window rename input **WHEN** the test types `"my problem"` **THEN** the input value is asserted `"my_problem"` and the committed sidebar name is `"my_problem"`. +- **GIVEN** `sync-latency.spec.ts` test 2 fills `${SESSION_A}-renamed` into the session rename input **THEN** the asserted (and cleaned-up) name is the underscored transform of that string. + +### Non-Goals + +- No backend-side name conversion (constitution §I; WYSIWYG/optimistic-update parity). +- No change to `ValidateServerName` / `ValidateWorktreeName` backend rules. +- No caret-position management beyond React-controlled-input defaults — mid-string edits that change length may move the caret to the end (accepted intake edge). +- No migration/renaming of pre-existing spacey sessions. + +### Design Decisions + +#### Live transform + commit finalizer split +**Decision**: The live `onChange` transform strips leading separators but keeps a trailing `_` visible; a separate `finalizeSafeName` trims it at commit. +**Why**: Trimming the trailing `_` live would delete the separator the user just typed ("My " + "p" would become "Myp") and break mid-word entry; commit-trim is the one minimal WYSIWYG deviation. +**Rejected**: Strict-WYSIWYG (commit exactly what is shown, trailing `_` included) — leaves awkward trailing separators on committed names for no user benefit. +*Introduced by*: 260722-ln4n-auto-safe-name-conversion + +## Tasks + +### Phase 1: Setup + +- [x] T001 Create `app/frontend/src/lib/names.ts` (toSafeSessionName, toSafeWindowName, toSafeServerName, toSafeWorktreeName, finalizeSafeName, deriveNameFromPath) plus table-style Vitest coverage in `app/frontend/src/lib/names.test.ts` + +### Phase 2: Core Implementation + +- [x] T002 [P] Add `ValidateNewName` to `app/backend/internal/validate/validate.go` with table tests in `validate_test.go` (space rejection, ValidateName inheritance, boundary cases) +- [x] T003 Switch the four new-name call sites to `ValidateNewName` (`api/sessions.go` create + rename body.Name, `api/windows.go` create non-empty name + rename body.Name); add handler tests for space rejection and a permissive spacey-old-name rename +- [x] T004 Rewire `app/frontend/src/components/create-session-dialog.tsx`: remove `toTmuxSafeName`/`deriveNameFromPath` definitions, import from `@/lib/names`, apply `toSafeSessionName` in the name input `onChange`, finalize in `handleCreate` and the collision check +- [x] T005 Rewire `app/frontend/src/app.tsx` (deriveNameFromPath import; session-rename dialog input → session transform; iframe window-name input → window transform + finalize in `handleCreateIframeWindow`; create-server dialog input → server transform + finalize in `handleCreateServer`) and `app/frontend/src/hooks/use-dialog-state.ts` (`handleRenameSession` finalize) +- [x] T006 Rewire sidebar inline renames: `sidebar/session-row.tsx` input → session transform, `sidebar/window-row.tsx` input → window transform, `sidebar/index.tsx` commit handlers finalize; update `sidebar.test.tsx` rename expectations (hyphenated typed value commits underscored) +- [x] T007 [P] Rewire `app/frontend/src/components/top-bar.tsx` WindowHeading edit input → window transform + finalize in `commit`; add a conversion test in `top-bar.test.tsx` +- [x] T008 [P] Rewire `app/frontend/src/components/host-overview-page.tsx` server input → server transform + finalize in `handleCreate` +- [x] T009 [P] Rewire `app/frontend/src/components/spawn-agent-dialog.tsx` worktree field → worktree transform + finalize at submit + +### Phase 3: Integration & Edge Cases + +- [x] T010 Extend `app/frontend/tests/e2e/window-heading.spec.ts` with a typed-space → underscore live-conversion test; update sibling `window-heading.spec.md` +- [x] T011 Update `app/frontend/tests/e2e/sync-latency.spec.ts` test 2 (UI session rename) for the hyphen→underscore session conversion (expected name + afterAll cleanup); update sibling `sync-latency.spec.md` +- [x] T012 Run `just test-backend`, `just test-frontend`, then targeted e2e (`just pw test window-heading sync-latency sidebar-keyboard-nav`), then full `just test-e2e`; fix any fallout + +## Acceptance + +### Functional Completeness + +- [x] A-001 R1: `src/lib/names.ts` exists with the four per-kind transforms plus `finalizeSafeName`/`deriveNameFromPath`; `toTmuxSafeName` no longer exists in `create-session-dialog.tsx` +- [x] A-002 R2: All three session-name inputs convert live in `onChange` via `toSafeSessionName` +- [x] A-003 R3: All three window-name inputs (WindowHeading, sidebar row, iframe dialog) convert live via `toSafeWindowName` +- [x] A-004 R4: Both server-name inputs the plan enumerated (host-overview + AppShell/app.tsx) convert live via `toSafeServerName`. NOTE (should-fix): a THIRD server-name input exists — the board-route create-server dialog (`src/components/board/board-page.tsx:1179`) — which the plan's R4 inventory missed and which still does a raw `setCreateServerName(e.target.value)` with no live transform (its `handleCreateServerSubmit` also skips `finalizeSafeName`). Decision 5 mandates "every naming entry point". +- [x] A-005 R5: The spawn-dialog worktree field converts live via `toSafeWorktreeName` +- [x] A-006 R7: `ValidateNewName` exists and is applied at exactly the four new-name call sites (sessions.go:32,78; windows.go:41,177) + +### Behavioral Correctness + +- [x] A-007 R1: Session transform converts hyphens; window/worktree transforms keep them; case is preserved; length caps at 128/64 (verified in `names.test.ts` + regex trace) +- [x] A-008 R6: Commit sites finalize (trailing `_` trimmed); the committed name equals the API-submitted name (WYSIWYG incl. optimistic updates) +- [x] A-009 R8: Existing-name lookups (session URL params sessions.go:61,93,171,193; TargetSession windows.go:332; session param windows.go:19; upload.go:23; both riff session values riff.go:123,224) still use permissive `ValidateName` + +### Scenario Coverage + +- [x] A-010 R1: `names.test.ts` covers "My problem"→"My_problem", hyphen divergence, collapse/trim, caps, forbidden-set conversion (195 tests pass) +- [x] A-011 R7: Go tests cover space rejection on create/rename for sessions and windows, plus a spacey-old-name rename that succeeds (validate + api packages green) +- [x] A-012 R9: e2e asserts live typed-space→underscore conversion in the window heading; touched `.spec.ts` files (`window-heading`, `sync-latency`) have updated `.spec.md` siblings in the same working tree + +### Edge Cases & Error Handling + +- [x] A-013 R2: Leading unsafe chars are dropped as typed (space in empty field produces nothing — `squash` strips leading `_`); empty-after-conversion input falls through to existing empty-name guards with no new error surface +- [x] A-014 R7: Window create with an omitted/empty name still succeeds — `windows.go:39` guards `if body.Name != ""` before `ValidateNewName` (tmux auto-naming path untouched) + +### Code Quality + +- [x] A-015 Pattern consistency: transforms live in `src/lib/` with colocated `.test.ts`, matching the established shared-pure-logic pattern; Go validator mirrors existing validate.go doc-comment style +- [x] A-016 No unnecessary duplication: all wired surfaces import from `@/lib/names`; no per-component transform copies remain (`toTmuxSafeName` absent from `src/`/`tests/`) + +### Security + +- [x] A-017 R7: Backend charset only tightens (`ValidateNewName` layers over `ValidateName`, no relaxation of `forbiddenChars`); validation stays reject-only before any subprocess (constitution §I); no `exec` changes + +## Notes + +- **Apply verification results** (2026-07-22): `just test-backend` all packages + ok; `just test-frontend` 95 files / 1672 tests passed; full `just test-e2e` + 162 passed, 2 flaky-passed, 2 skipped, 2 failed — the 2 failures + (`sync-latency` "5. Kill window via Ctrl+click" and `sidebar-window-sync` + "kill-then-create at same index") were **verified pre-existing** by stashing + this change and re-running on the base tree (commit 95e4000): identical + failures, kill-flow only, no naming surface involved. `just build` fails on a + missing `VERSION` file in this worktree — also identical on the base tree + (environmental, not fallout); the frontend production build itself succeeds + and `tsc --noEmit` is clean. +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) +- If an item is not applicable, mark checked and prefix with **N/A**: `- [x] A-NNN **N/A**: {reason}` + +## Deletion Candidates + +- `app/backend/internal/validate/validate.go:220` — `ValidateWorktreeName`'s explicit `strings.Contains(name, " ")` space check is now redundant with the new-name posture (`ValidateNewName` centralizes the no-spaces rule for created entities); the intake deliberately kept it explicit, so this is a note only, not a removal recommendation. +- `app/frontend/src/components/create-session-dialog.tsx` — the removed `toTmuxSafeName`/`deriveNameFromPath` local definitions are already deleted and re-homed in `@/lib/names`; no residual dead copy remains (verified — no `toTmuxSafeName` reference anywhere in `src/`/`tests/`). +- None beyond the above — this change is additive (a new shared module + a new backend validator) and rewires existing call sites rather than making broad swaths of code redundant. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | The create dialog's window mode has no name input (windows create unnamed, tmux auto-names) — the intake's "create-window mode" inventory row needs no wiring | Verified in `create-session-dialog.tsx` (name field hidden for `mode === "window"`, `createWindow` called with `undefined` name) | S:85 R:90 A:95 D:90 | +| 2 | Confident | Two entry points beyond the intake's table also get transforms: the AppShell create-server dialog and the "New iframe window" name field (both `app.tsx`) | Intake decision 5 mandates "every naming entry point"; both are real typed-name inputs found during grounding | S:60 R:85 A:90 D:80 | +| 3 | Certain | `riff.go` `body.Session` (spawn) and the presets `session` query are EXISTING-session references → stay on permissive `ValidateName` | `deriveRepoRoot` reads the named session's cwd; nothing is created under that name — deterministic from handler semantics (intake row 12) | S:80 R:80 A:90 D:90 | +| 4 | Confident | `finalizeSafeName` strips leading+trailing `_` runs; applied at commit after `trim()`; live transforms strip leading only | Matches the old `toTmuxSafeName` end-trim shape and intake row 13's commit-trim contract | S:55 R:85 A:85 D:80 | +| 5 | Confident | `sync-latency.spec.ts` test 2 and `sidebar.test.tsx` rename expectations updated to the underscored names the session transform now produces | Tests conform to spec (constitution Test Integrity); hyphen→`_` on session renames is the specified behavior, not a regression | S:55 R:80 A:85 D:80 | +| 6 | Confident | `deriveNameFromPath` now also converts spaces in directory basenames (previously passed through) | The tightened backend rejects spacey creates; a suggestion the backend would reject is a bug, and the intake routes the helper through the session transform | S:50 R:80 A:85 D:75 | +| 7 | Tentative | No caret-position management: transforms re-run on the whole value; caret may jump to end on mid-string length-changing edits | Intake accepts this edge ("the transform never fights the user mid-word" covers the common append case); full caret preservation adds complexity for a rare interaction | S:40 R:75 A:60 D:55 | + +7 assumptions (2 certain, 4 confident, 1 tentative). From 969b2fda0784f1e8fdfa4e87d2fb6d002c5c79e4 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Wed, 22 Jul 2026 17:29:40 +0530 Subject: [PATCH 2/2] Update ship status and record PR URL --- .../.history.jsonl | 1 + .../.status.yaml | 34 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl b/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl index 5a351f04..ef0d0eb7 100644 --- a/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/.history.jsonl @@ -10,3 +10,4 @@ {"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-07-22T11:51:05Z"} {"event":"review","result":"passed","ts":"2026-07-22T11:51:05Z"} {"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-07-22T11:58:02Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-07-22T11:59:33Z"} diff --git a/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml b/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml index 773ad064..65ed4136 100644 --- a/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml +++ b/fab/changes/260722-ln4n-auto-safe-name-conversion/.status.yaml @@ -9,8 +9,8 @@ progress: apply: done review: done hydrate: done - ship: active - review-pr: pending + ship: done + review-pr: active plan: generated: true task_count: 12 @@ -33,22 +33,24 @@ stage_metrics: apply: {started_at: "2026-07-22T11:08:16Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:43:14Z"} review: {started_at: "2026-07-22T11:43:14Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:51:05Z"} hydrate: {started_at: "2026-07-22T11:51:05Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:58:02Z"} - ship: {started_at: "2026-07-22T11:58:02Z", driver: fab-fff, iterations: 1} -prs: [] + ship: {started_at: "2026-07-22T11:58:02Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-22T11:59:33Z"} + review-pr: {started_at: "2026-07-22T11:59:33Z", driver: git-pr, iterations: 1} +prs: + - https://github.com/sahil87/run-kit/pull/441 true_impact: - added: 0 - deleted: 0 - net: 0 + added: 990 + deleted: 66 + net: 924 excluding: - added: 0 - deleted: 0 - net: 0 + added: 556 + deleted: 55 + net: 501 tests: - added: 0 - deleted: 0 - net: 0 - computed_at: "2026-07-22T11:58:02Z" - computed_at_stage: hydrate + added: 320 + deleted: 3 + net: 317 + computed_at: "2026-07-22T11:59:33Z" + computed_at_stage: ship summary: live per-kind safe-name conversion (src/lib/names.ts) at naming entry points + backend ValidateNewName no-spaces tightening for created/renamed-to names # true_impact: lazily created on first stage-finish that computes it (no placeholder here). -last_updated: 2026-07-22T11:58:02Z +last_updated: 2026-07-22T11:59:33Z