fix(web): de-dup mobile terminal auto-create + add state-label tests + repo-tap toast

- use-mobile-terminals: move the auto-create-first-shell guard from
  per-instance refs to a module-level Set keyed by environmentId.
  MobileTerminalPane mounts useMobileTerminals three times for the
  same session (the pane itself, the picker pill via
  useActiveTerminalPillLabel, and the picker sheet content via
  MobileTerminalsList — Vaul keeps it mounted while closed). With per-
  instance guards, all three effects ran on first commit and each
  fired createUserShell, racing into up to three shells. The shared
  Set ensures only the first effect to observe an empty list creates,
  with retry-on-failure preserved (delete + next render re-checks).
- mobile-repos-section: tapping a repo with no sessions now toasts a
  hint to open the session picker, instead of silently doing nothing.
- state-labels: add tests covering known mappings, null/undefined
  fallbacks, and unknown-state passthrough for both formatters.
This commit is contained in:
José Almeida
2026-05-06 02:21:32 +01:00
parent b14f473855
commit 9488bb0c06
3 changed files with 81 additions and 18 deletions
@@ -3,6 +3,7 @@
import { memo, useCallback, useMemo } from "react";
import { IconCheck, IconFolder, IconGitBranch } from "@tabler/icons-react";
import { useAppStore } from "@/components/state-provider";
import { useToast } from "@/components/toast-provider";
import type { Repository, TaskSession } from "@/lib/types/http";
import type { KanbanState } from "@/lib/state/slices";
@@ -141,6 +142,7 @@ export const MobileReposSection = memo(function MobileReposSection({
activeSessionId ? (s.taskSessions.items[activeSessionId]?.repository_id ?? null) : null,
);
const rows = useTaskRepoRows(taskId, workspaceId);
const { toast } = useToast();
const handleSelect = useCallback(
(row: RepoRow) => {
@@ -148,12 +150,17 @@ export const MobileReposSection = memo(function MobileReposSection({
if (row.switchToSessionId) {
setActiveSession(taskId, row.switchToSessionId);
onClose();
return;
}
// Without an existing session on this repo, the user can switch to the
// Sessions tab and launch one — we don't auto-launch here to avoid
// surprise side effects from a tap that looked like navigation.
// We don't auto-launch from a repo tap — that would create a side effect
// for what looks like navigation. Surface a hint so the tap doesn't
// appear silently broken.
toast({
title: "No session on this repo yet",
description: "Open the session picker to launch one.",
});
},
[taskId, setActiveSession, onClose],
[taskId, setActiveSession, onClose, toast],
);
if (!taskId) {
@@ -5,6 +5,17 @@ import { useTerminals } from "./use-terminals";
import { useUserShells } from "./use-user-shells";
import { useAppStore } from "@/components/state-provider";
/**
* Module-level guard for the auto-create-first-shell effect, keyed by
* environmentId. `useMobileTerminals` is called from several places that all
* end up rendering the same terminal pane (the pane itself, the picker pill,
* the picker sheet content) — a per-instance ref guard would let each one
* fire `createUserShell` on first mount, racing into multiple shells. The
* Set is shared across instances so only the first effect to run for a given
* environment triggers creation.
*/
const autoCreatedEnvironments = new Set<string>();
/**
* Mobile wrapper around `useTerminals`. Auto-creates a first shell when the
* server-side shell list loads empty so the user always sees one terminal
@@ -15,33 +26,30 @@ export function useMobileTerminals(sessionId: string | null) {
sessionId ? (s.environmentIdBySessionId[sessionId] ?? null) : null,
);
const result = useTerminals({ sessionId, environmentId });
// Read user-shell loaded flag directly so the auto-create trigger has a
// primitive dependency (depending on the result object would re-run the
// effect every render and continuously cancel the timer).
// Read user-shell loaded flag directly so the auto-create trigger has
// primitive dependencies — depending on `result` would re-run on every
// render and would also race against React's effect ordering.
const { isLoaded: shellsLoaded, shells } = useUserShells(environmentId);
const addTerminalRef = useRef(result.addTerminal);
useEffect(() => {
addTerminalRef.current = result.addTerminal;
}, [result.addTerminal]);
const autoCreatedRef = useRef<string | null>(null);
useEffect(() => {
if (!environmentId || !shellsLoaded) return;
if (autoCreatedRef.current === environmentId) return;
if (autoCreatedEnvironments.has(environmentId)) return;
if (shells.length > 0) {
autoCreatedRef.current = environmentId;
autoCreatedEnvironments.add(environmentId);
return;
}
autoCreatedRef.current = environmentId;
autoCreatedEnvironments.add(environmentId);
// Reset the guard if creation fails so the user gets a retry on the next
// render cycle (e.g. after the WS reconnects). addTerminal returns void
// render cycle (e.g. after the WS reconnects). `addTerminal` returns void
// but its inner promise can still reject; guard defensively.
const result = addTerminalRef.current() as unknown;
if (result && typeof (result as Promise<unknown>).catch === "function") {
(result as Promise<unknown>).catch(() => {
if (autoCreatedRef.current === environmentId) {
autoCreatedRef.current = null;
}
const promise = addTerminalRef.current() as unknown;
if (promise && typeof (promise as Promise<unknown>).catch === "function") {
(promise as Promise<unknown>).catch(() => {
autoCreatedEnvironments.delete(environmentId);
});
}
}, [environmentId, shellsLoaded, shells.length]);
+48
View File
@@ -0,0 +1,48 @@
import { describe, it, expect } from "vitest";
import { formatTaskStateLabel, formatTaskSessionStateLabel } from "./state-labels";
import type { TaskState, TaskSessionState } from "@/lib/types/http";
describe("formatTaskStateLabel", () => {
it("maps known task states to human labels", () => {
expect(formatTaskStateLabel("IN_PROGRESS")).toBe("In progress");
expect(formatTaskStateLabel("WAITING_FOR_INPUT")).toBe("Waiting for input");
expect(formatTaskStateLabel("TODO")).toBe("To do");
expect(formatTaskStateLabel("COMPLETED")).toBe("Completed");
expect(formatTaskStateLabel("FAILED")).toBe("Failed");
expect(formatTaskStateLabel("CANCELLED")).toBe("Cancelled");
expect(formatTaskStateLabel("BLOCKED")).toBe("Blocked");
expect(formatTaskStateLabel("REVIEW")).toBe("Review");
expect(formatTaskStateLabel("CREATED")).toBe("Created");
expect(formatTaskStateLabel("SCHEDULING")).toBe("Scheduling");
});
it("returns 'Not started' for null/undefined", () => {
expect(formatTaskStateLabel(null)).toBe("Not started");
expect(formatTaskStateLabel(undefined)).toBe("Not started");
});
it("falls back to the raw value for unknown states", () => {
expect(formatTaskStateLabel("UNKNOWN_FUTURE" as TaskState)).toBe("UNKNOWN_FUTURE");
});
});
describe("formatTaskSessionStateLabel", () => {
it("maps known session states", () => {
expect(formatTaskSessionStateLabel("RUNNING")).toBe("Running");
expect(formatTaskSessionStateLabel("STARTING")).toBe("Starting");
expect(formatTaskSessionStateLabel("WAITING_FOR_INPUT")).toBe("Waiting for input");
expect(formatTaskSessionStateLabel("COMPLETED")).toBe("Completed");
expect(formatTaskSessionStateLabel("FAILED")).toBe("Failed");
expect(formatTaskSessionStateLabel("CANCELLED")).toBe("Cancelled");
expect(formatTaskSessionStateLabel("CREATED")).toBe("Created");
});
it("returns empty string for null/undefined", () => {
expect(formatTaskSessionStateLabel(null)).toBe("");
expect(formatTaskSessionStateLabel(undefined)).toBe("");
});
it("falls back to the raw value for unknown states", () => {
expect(formatTaskSessionStateLabel("UNKNOWN" as TaskSessionState)).toBe("UNKNOWN");
});
});