From 8f5980d1e7a5d0bc9cd6bc659fcec79e6cb10d07 Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:08:33 -0400 Subject: [PATCH 01/12] feat(api): list_remote_branches via git ls-remote (#77) --- api/services/clone.py | 48 ++++++++++++++++++++++++++++++ api/tests/test_branches_service.py | 24 +++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 api/tests/test_branches_service.py diff --git a/api/services/clone.py b/api/services/clone.py index 0f698885..023d137b 100644 --- a/api/services/clone.py +++ b/api/services/clone.py @@ -61,6 +61,7 @@ class HostUnreachableError(CloneError): "RepoNotFoundError", "clone_dir_for", "ensure_clone", + "list_remote_branches", "remove_clone", ] @@ -569,3 +570,50 @@ def remove_clone(url: str, branch: str | None) -> bool: return False shutil.rmtree(target, ignore_errors=True) return True + + +# ls-remote timeout: bounded so a black-holed remote can't wedge a request. +# 20s covers a slow-but-live remote; a real hang trips it and surfaces a clean +# HostUnreachableError to the caller. +_LS_REMOTE_TIMEOUT_S = 20 + + +def list_remote_branches(url: str) -> tuple[list[str], str | None]: + """Return ``(branch_names, default_branch)`` for a remote git URL via + ``git ls-remote --symref`` — no clone required (strictly less capability + than the clone the server already does for the same URL). + + ``default_branch`` is the name HEAD points at (None if the remote has no + symbolic HEAD, e.g. an empty repo). Raises the same clean CloneError + subclasses ``ensure_clone`` raises so routers reuse one error taxonomy.""" + try: + proc = subprocess.run( + ["git", "ls-remote", "--symref", "--", url], + capture_output=True, + text=True, + check=False, + env=_git_env(), + timeout=_LS_REMOTE_TIMEOUT_S, + ) + except FileNotFoundError as e: + raise CloneError("git executable not found on PATH") from e + except subprocess.TimeoutExpired as e: + raise HostUnreachableError("timed out reaching remote") from e + if proc.returncode != 0: + _maybe_raise_clean_clone_error(url, None, proc.stderr) + raise CloneError(f"git ls-remote failed: {proc.stderr.strip()}") + + default: str | None = None + branches: list[str] = [] + for line in proc.stdout.splitlines(): + # Symref line: "ref: refs/heads/main\tHEAD" + if line.startswith("ref: ") and line.endswith("\tHEAD"): + target = line[len("ref: ") :].split("\t", 1)[0] + if target.startswith("refs/heads/"): + default = target[len("refs/heads/") :] + continue + # Ref line: "\trefs/heads/" + parts = line.split("\t") + if len(parts) == 2 and parts[1].startswith("refs/heads/"): + branches.append(parts[1][len("refs/heads/") :]) + return branches, default diff --git a/api/tests/test_branches_service.py b/api/tests/test_branches_service.py new file mode 100644 index 00000000..ccbb93db --- /dev/null +++ b/api/tests/test_branches_service.py @@ -0,0 +1,24 @@ +"""Unit coverage for list_remote_branches (git ls-remote parsing).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from api.services.clone import RepoNotFoundError, list_remote_branches + + +def test_lists_heads_and_default(make_fake_remote, tmp_path: Path) -> None: + bare, _ = make_fake_remote(tmp_path) # bare repo, HEAD → main, has 'feature' too + branches, resolved_default = list_remote_branches(f"file://{bare}") + assert "main" in branches + assert "feature" in branches + assert resolved_default == "main" + # Only heads, no tags/HEAD pseudo-refs leak in. + assert all("/" not in b for b in branches) + + +def test_bogus_url_raises(tmp_path: Path) -> None: + with pytest.raises((RepoNotFoundError, Exception)): + list_remote_branches(f"file://{tmp_path / 'nope.git'}") From 908bacbd396b87c5828a33befec0821e692ef352 Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:18:40 -0400 Subject: [PATCH 02/12] feat(app): fetchBranches client for /api/branches (#77) --- app/src/api/branches.ts | 28 ++++++++++++++++++++++++++++ app/tests/api/branches.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 app/src/api/branches.ts create mode 100644 app/tests/api/branches.test.ts diff --git a/app/src/api/branches.ts b/app/src/api/branches.ts new file mode 100644 index 00000000..67c6b8ab --- /dev/null +++ b/app/src/api/branches.ts @@ -0,0 +1,28 @@ +// api/branches.ts — Client for GET /api/branches. Fetches the remote branch +// list for a git URL so the picker can offer a valid-for-this-repo dropdown +// instead of a free-text field. Remote URLs only (local sources have no branch). + +import { URL_PARAMS } from '@/constants/urlParams'; + +export interface BranchList { + branches: string[]; + default: string | null; +} + +export async function fetchBranches(src: string): Promise { + const url = new URL('/api/branches', window.location.origin); + url.searchParams.set(URL_PARAMS.SRC, src); + const resp = await fetch(url.toString()); + if (!resp.ok) { + let message = `branch lookup failed (${resp.status})`; + try { + const body = (await resp.json()) as { detail?: string }; + if (body?.detail) message = body.detail; + } catch (_) { + /* non-JSON error body: keep the status-based message */ + } + throw new Error(message); + } + const body = (await resp.json()) as BranchList; + return { branches: body.branches ?? [], default: body.default ?? null }; +} diff --git a/app/tests/api/branches.test.ts b/app/tests/api/branches.test.ts new file mode 100644 index 00000000..a91f8d1c --- /dev/null +++ b/app/tests/api/branches.test.ts @@ -0,0 +1,22 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { fetchBranches } from '@/api/branches'; + +afterEach(() => vi.restoreAllMocks()); + +describe('fetchBranches', () => { + it('returns branches + default on ok', async () => { + vi.stubGlobal('fetch', vi.fn(async () => + new Response(JSON.stringify({ branches: ['main', 'dev'], default: 'main' }), { status: 200 }) + )); + const r = await fetchBranches('https://github.com/o/r'); + expect(r.branches).toEqual(['main', 'dev']); + expect(r.default).toBe('main'); + }); + + it('rejects with the server error on non-2xx', async () => { + vi.stubGlobal('fetch', vi.fn(async () => + new Response(JSON.stringify({ detail: 'repository not found' }), { status: 404 }) + )); + await expect(fetchBranches('https://github.com/o/nope')).rejects.toThrow(/not found/i); + }); +}); From 66a85da2b1fcc9af46348e33c4911427d5aa9f10 Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:21:05 -0400 Subject: [PATCH 03/12] feat(api): GET /api/branches endpoint (#77) Recovered onto the branch after the original commit (f298e939) was orphaned by a HEAD reset during subagent execution. Co-Authored-By: Claude Opus 4.8 (1M context) --- api/app.py | 3 +- api/models/responses.py | 5 +++ api/routers/branches.py | 35 ++++++++++++++++++ api/tests/test_server_branches.py | 36 +++++++++++++++++++ app/src/types/manifest.generated.ts | 55 +++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 api/routers/branches.py create mode 100644 api/tests/test_server_branches.py diff --git a/api/app.py b/api/app.py index fdb81068..0f19a734 100644 --- a/api/app.py +++ b/api/app.py @@ -16,7 +16,7 @@ from api.config import GZIP_MIN_BYTES from api.models.responses import ErrorResponse -from api.routers import commit, file, manifest, meta +from api.routers import branches, commit, file, manifest, meta from api.sse_compression import SSEGZipMiddleware from api.static import make_static_router @@ -84,6 +84,7 @@ def create_app(static_dir: Path | None = None) -> FastAPI: app.include_router(meta.router) app.include_router(file.router) app.include_router(commit.router) + app.include_router(branches.router) app.include_router(manifest.router) app.include_router(make_static_router(static_dir or DEFAULT_STATIC_DIR)) return app diff --git a/api/models/responses.py b/api/models/responses.py index 6cd6c0ed..469ab2fc 100644 --- a/api/models/responses.py +++ b/api/models/responses.py @@ -41,3 +41,8 @@ class CommitDetailResponse(BaseModel): date: str # YYYY-MM-DD subject: str body: str + + +class BranchListResponse(BaseModel): + branches: list[str] + default: str | None diff --git a/api/routers/branches.py b/api/routers/branches.py new file mode 100644 index 00000000..37228bdd --- /dev/null +++ b/api/routers/branches.py @@ -0,0 +1,35 @@ +"""GET /api/branches?src= — remote branch list for the branch picker. + +Remote git URLs only: local sources scan the working tree in place and ignore +branch, so there is nothing to choose. Uses git ls-remote (no clone) and reuses +the clone error taxonomy so failures map to clean HTTP statuses.""" + +from __future__ import annotations + +from fastapi import APIRouter, HTTPException, Query + +from api.models.responses import BranchListResponse +from api.services.clone import ( + CloneError, + HostUnreachableError, + RepoNotFoundError, + list_remote_branches, +) +from api.services.source import SourceKind, classify + +router = APIRouter(prefix="/api", tags=["branches"]) + + +@router.get("/branches", response_model=BranchListResponse) +def get_branches(src: str = Query(...)) -> BranchListResponse: + if classify(src) is not SourceKind.REMOTE: + raise HTTPException(400, "branches are only available for remote git URLs") + try: + branches, default = list_remote_branches(src) + except RepoNotFoundError as e: + raise HTTPException(404, str(e)) from e + except HostUnreachableError as e: + raise HTTPException(502, str(e)) from e + except CloneError as e: + raise HTTPException(400, str(e)) from e + return BranchListResponse(branches=branches, default=default) diff --git a/api/tests/test_server_branches.py b/api/tests/test_server_branches.py new file mode 100644 index 00000000..a5d92717 --- /dev/null +++ b/api/tests/test_server_branches.py @@ -0,0 +1,36 @@ +"""TestClient coverage for GET /api/branches.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from api.app import create_app + + +@pytest.fixture() +def client(tmp_path: Path) -> TestClient: + static = tmp_path / "static" + static.mkdir() + (static / "index.html").write_text("x") + return TestClient(create_app(static_dir=static)) + + +def test_branches_happy_path(client: TestClient, make_fake_remote, tmp_path: Path) -> None: + bare, _ = make_fake_remote(tmp_path) + r = client.get("/api/branches", params={"src": f"file://{bare}"}) + assert r.status_code == 200 + body = r.json() + assert "main" in body["branches"] + assert body["default"] == "main" + + +def test_branches_rejects_local_path(client: TestClient) -> None: + r = client.get("/api/branches", params={"src": "/tmp/some/local/path"}) + assert r.status_code == 400 + + +def test_branches_missing_src(client: TestClient) -> None: + assert client.get("/api/branches").status_code == 422 # FastAPI required-query diff --git a/app/src/types/manifest.generated.ts b/app/src/types/manifest.generated.ts index f0e119b9..569a0d3b 100644 --- a/app/src/types/manifest.generated.ts +++ b/app/src/types/manifest.generated.ts @@ -100,6 +100,23 @@ export interface paths { patch?: never; trace?: never; }; + "/api/branches": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Get Branches */ + get: operations["get_branches_api_branches_get"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/manifest/signature": { parameters: { query?: never; @@ -162,6 +179,13 @@ export interface components { /** Commits */ commits: number; }; + /** BranchListResponse */ + BranchListResponse: { + /** Branches */ + branches: string[]; + /** Default */ + default: string | null; + }; /** BusynessThresholds */ BusynessThresholds: { /** Avg */ @@ -700,6 +724,37 @@ export interface operations { }; }; }; + get_branches_api_branches_get: { + parameters: { + query: { + src: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["BranchListResponse"]; + }; + }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; signature_api_manifest_signature_get: { parameters: { query: { From 50b94ccd856de7d4c86ccee7e5a4184d4ab7786b Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:27:06 -0400 Subject: [PATCH 04/12] refactor(app): SOURCE_PICKER -> PROJECTS_VIEW store (#77) --- app/src/layout/App/App.tsx | 16 +++++------ app/src/layout/AppHeader/AppHeader.tsx | 4 +-- app/src/state/stores/ui.ts | 30 ++++++++++----------- app/src/views/SourcePicker/SourcePicker.tsx | 8 +++--- app/tests/state/stores/ui.test.ts | 14 ++++++++++ 5 files changed, 43 insertions(+), 29 deletions(-) create mode 100644 app/tests/state/stores/ui.test.ts diff --git a/app/src/layout/App/App.tsx b/app/src/layout/App/App.tsx index aa8e2279..62d173a6 100644 --- a/app/src/layout/App/App.tsx +++ b/app/src/layout/App/App.tsx @@ -10,7 +10,7 @@ // — self-subscribes to SCENE_HANDLE + picker // // — reads signals directly -// — reads SOURCE_PICKER + SERVER_CONFIG directly +// — reads PROJECTS_VIEW + SERVER_CONFIG directly // — reads LOADING_OVERLAY directly // — reads SHORTCUTS_OPEN directly // — reads DEBUG_OPEN directly; scene commands passed as props @@ -38,9 +38,9 @@ import { runStemDiagnostic, } from '@/state/stores/scene'; import { - openSourcePicker, - openSourcePickerForCurrentSource, - closeSourcePicker, + openProjectsView, + openProjectsViewForCurrentSource, + closeProjectsView, } from '@/state/stores/ui'; import { SOURCE_ERROR, CURRENT_SOURCE } from '@/state/stores/source'; import { MANIFEST } from '@/state/stores/manifest'; @@ -66,14 +66,14 @@ export function App() { // App coordinates the source picker; the fetch hook only reports outcomes. const dismissPicker = () => { - closeSourcePicker(); + closeProjectsView(); SOURCE_ERROR.value = null; }; // Cold boot with no ?src → prompt for a source (non-dismissible). useEffect(() => { if (!new URLSearchParams(window.location.search).has(URL_PARAMS.SRC)) { - openSourcePicker({ dismissible: false }); + openProjectsView({ dismissible: false }); } }, []); @@ -83,7 +83,7 @@ export function App() { useSignalEffect(() => { const err = SOURCE_ERROR.value; if (!err) return; - openSourcePicker({ + openProjectsView({ dismissible: !isEmptyManifest(MANIFEST.peek()), prefill: err.prefill, error: err.error, @@ -94,7 +94,7 @@ export function App() { <> diff --git a/app/src/layout/AppHeader/AppHeader.tsx b/app/src/layout/AppHeader/AppHeader.tsx index cef64cd0..e28e5ee4 100644 --- a/app/src/layout/AppHeader/AppHeader.tsx +++ b/app/src/layout/AppHeader/AppHeader.tsx @@ -15,7 +15,7 @@ import { SCENE_HANDLE } from '@/state/stores/scene'; import { MANIFEST } from '@/state/stores/manifest'; import { ROOT_PATH } from '@/constants/manifest'; import { SOURCE_INFO } from '@/state/stores/source'; -import { openSourcePicker, openShortcuts, openDebug } from '@/state/stores/ui'; +import { openProjectsView, openShortcuts, openDebug } from '@/state/stores/ui'; import { isDebugMode } from '@/utils/debugMode'; import { NodeKind, type Manifest } from '@/types'; import { ResetViewButton } from '@/components/ResetViewButton'; @@ -86,7 +86,7 @@ export function AppHeader({ openSourcePicker({ dismissible: true }))} + onSwitchSource={onSwitchSource ?? (() => openProjectsView({ dismissible: true }))} /> diff --git a/app/src/state/stores/ui.ts b/app/src/state/stores/ui.ts index a48716c6..21fab20b 100644 --- a/app/src/state/stores/ui.ts +++ b/app/src/state/stores/ui.ts @@ -8,7 +8,7 @@ import { SourceKind } from '@/utils/sources'; import { URL_PARAMS } from '@/constants/urlParams'; // These are the UI-state CONTRACTS the views render against. They live here (in -// state) so state/ stays view-independent — the SourcePicker / LoadingOverlay +// state) so state/ stays view-independent — the ProjectsView / LoadingOverlay // components import them from here, not the other way around. /** What the source picker submits when the user opens a project. */ @@ -34,33 +34,33 @@ export interface LoadingOverlayShowOpts { branch?: string; } -// ── Source picker ──────────────────────────────────────────────────────────── +// ── Projects view ──────────────────────────────────────────────────────────── -export interface SourcePickerState { +export interface ProjectsViewState { visible: boolean; opts: OpenOpts; } -export const SOURCE_PICKER = signal({ +export const PROJECTS_VIEW = signal({ visible: false, opts: {}, }); -/** Open the source picker modal. */ -export function openSourcePicker(opts: OpenOpts = {}): void { - SOURCE_PICKER.value = { visible: true, opts }; +/** Open the projects view. */ +export function openProjectsView(opts: OpenOpts = {}): void { + PROJECTS_VIEW.value = { visible: true, opts }; } -/** Close the source picker modal. */ -export function closeSourcePicker(): void { - SOURCE_PICKER.value = { ...SOURCE_PICKER.value, visible: false }; +/** Close the projects view. */ +export function closeProjectsView(): void { + PROJECTS_VIEW.value = { ...PROJECTS_VIEW.value, visible: false }; } -/** Open the picker pre-filled with the current `?src`/`?branch` — the +/** Open the view pre-filled with the current `?src`/`?branch` — the * header's "switch source" affordance. Always dismissible. */ -export function openSourcePickerForCurrentSource(): void { +export function openProjectsViewForCurrentSource(): void { const cur = new URLSearchParams(window.location.search); - openSourcePicker({ + openProjectsView({ dismissible: true, prefill: cur.has(URL_PARAMS.SRC) ? { src: cur.get(URL_PARAMS.SRC)!, branch: cur.get(URL_PARAMS.BRANCH) ?? undefined } @@ -151,8 +151,8 @@ export function closeDebug(): void { DEBUG_OPEN.value = false; } -/** True while any modal (source picker, shortcuts, debug) is open. Scene input +/** True while any modal (projects view, shortcuts, debug) is open. Scene input * handlers read this so keyboard shortcuts don't fire underneath a modal. */ export const MODAL_OPEN = computed( - () => SOURCE_PICKER.value.visible || SHORTCUTS_OPEN.value || DEBUG_OPEN.value + () => PROJECTS_VIEW.value.visible || SHORTCUTS_OPEN.value || DEBUG_OPEN.value ); diff --git a/app/src/views/SourcePicker/SourcePicker.tsx b/app/src/views/SourcePicker/SourcePicker.tsx index 2873d9fa..d4c1ba81 100644 --- a/app/src/views/SourcePicker/SourcePicker.tsx +++ b/app/src/views/SourcePicker/SourcePicker.tsx @@ -12,7 +12,7 @@ import { clearManifestCache } from '@/api/manifest'; import { srcKind, SourceKind } from '@/utils/sources'; import { URL_PARAMS } from '@/constants/urlParams'; import { Folder, Trash2, TriangleAlert, X } from 'lucide-preact'; -import { SOURCE_PICKER, type SourcePayload } from '@/state/stores/ui'; +import { PROJECTS_VIEW, type SourcePayload } from '@/state/stores/ui'; import { SERVER_CONFIG } from '@/state/stores/serverConfig'; // ── Hosting-site SVG icons ─────────────────────────────────────────────────── @@ -317,9 +317,9 @@ export function SourcePickerModal({ state, onSubmit, onClose }: SourcePickerModa } // ── Signal-driven top-level component ────────────────────────────────────── -// Reads SOURCE_PICKER + SERVER_CONFIG directly for open-state/prefill, but takes +// Reads PROJECTS_VIEW + SERVER_CONFIG directly for open-state/prefill, but takes // onSubmit/onClose via props from App (App owns the stateful submit handler from -// useManifestSource and the pure closeSourcePicker store action). Returns null +// useManifestSource and the pure closeProjectsView store action). Returns null // when the picker is closed so SourcePickerModal fully unmounts — its // useState-backed form inputs reset on the next open. @@ -329,7 +329,7 @@ export interface SourcePickerProps { } export function SourcePicker({ onSubmit, onClose }: SourcePickerProps) { - const sp = SOURCE_PICKER.value; + const sp = PROJECTS_VIEW.value; if (!sp.visible) return null; const serverCfg = SERVER_CONFIG.value; diff --git a/app/tests/state/stores/ui.test.ts b/app/tests/state/stores/ui.test.ts new file mode 100644 index 00000000..62ae0d77 --- /dev/null +++ b/app/tests/state/stores/ui.test.ts @@ -0,0 +1,14 @@ +import { describe, it, expect } from 'vitest'; +import { PROJECTS_VIEW, openProjectsView, closeProjectsView, MODAL_OPEN } from '@/state/stores/ui'; + +describe('PROJECTS_VIEW', () => { + it('opens with opts and closes', () => { + openProjectsView({ dismissible: true }); + expect(PROJECTS_VIEW.value.visible).toBe(true); + expect(PROJECTS_VIEW.value.opts.dismissible).toBe(true); + expect(MODAL_OPEN.value).toBe(true); + closeProjectsView(); + expect(PROJECTS_VIEW.value.visible).toBe(false); + expect(MODAL_OPEN.value).toBe(false); + }); +}); From 552db162a15be82cd4dc1aabad9a6a8d555d6f6e Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:34:07 -0400 Subject: [PATCH 05/12] feat(app): abortable loads + cancelLoad (abort is not an error) (#77) streamManifest now accepts an options bag ({ signal, EventSourceImpl }) instead of a positional ctor; useManifestSource threads an AbortController through loadSource/pumpManifestStream so a new load or an explicit cancelLoad() aborts the in-flight one. A user-canceled load is treated as a clean no-op (no SOURCE_ERROR write), since that signal drives App's picker-reopen reaction. The hook now returns { submitSource, cancelLoad }. --- app/src/api/manifest.ts | 11 ++++- app/src/hooks/useManifestSource.ts | 50 ++++++++++++++++++----- app/src/layout/App/App.tsx | 2 +- app/tests/api/manifest.test.ts | 24 ++++++++--- app/tests/hooks/useManifestSource.test.ts | 46 +++++++++++++++++++++ 5 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 app/tests/hooks/useManifestSource.test.ts diff --git a/app/src/api/manifest.ts b/app/src/api/manifest.ts index bdf6fb8b..c3d0b8c6 100644 --- a/app/src/api/manifest.ts +++ b/app/src/api/manifest.ts @@ -148,8 +148,9 @@ export type ScanProgressEvent = Extract< */ export function streamManifest( url: string, - EventSourceImpl: typeof EventSource = EventSource + opts: { signal?: AbortSignal; EventSourceImpl?: typeof EventSource } = {} ): AsyncIterable { + const EventSourceImpl = opts.EventSourceImpl ?? EventSource; return { [Symbol.asyncIterator](): AsyncIterator { const es = new EventSourceImpl(url); @@ -188,6 +189,14 @@ export function streamManifest( } }; + // Abort: close the stream cleanly (done, not error) so the consumer's + // for-await exits without a manifest. loadSource treats signal.aborted as + // a user cancel, not a failure. + if (opts.signal) { + if (opts.signal.aborted) finish(); + else opts.signal.addEventListener('abort', () => finish(), { once: true }); + } + // Parse an event's JSON data, ending the stream with an error (rather // than throwing into the swallowed event listener, which would leave the // iterator hanging forever) if the payload is malformed/truncated. diff --git a/app/src/hooks/useManifestSource.ts b/app/src/hooks/useManifestSource.ts index f3d5703a..9edbe0f5 100644 --- a/app/src/hooks/useManifestSource.ts +++ b/app/src/hooks/useManifestSource.ts @@ -61,11 +61,12 @@ async function pumpManifestStream( onManifest: ( manifest: Manifest, phase: ScanPhase.PartialManifest | ScanPhase.CompleteManifest - ) => Promise | void + ) => Promise | void, + signal?: AbortSignal ): Promise { let lastManifest: Manifest | null = null; - for await (const event of streamManifest(url)) { + for await (const event of streamManifest(url, { signal })) { if (event.phase === ScanPhase.Error) throw new Error(event.error); if ('display_root' in event && event.display_root) { @@ -111,6 +112,10 @@ async function pumpManifestStream( // applyManifest already uses one layer down. let loadGeneration = 0; +// The AbortController for the current foreground load, so the UI can cancel a +// slow clone/scan. A new load or a cancel aborts the previous one. +let loadController: AbortController | null = null; + // ── Canonical source load (cold-boot + user switch) ────────────────── // The one way to load a source: claim the generation → overlay → stream // skeleton+final into MANIFEST → on success commit the source (CURRENT_SOURCE + @@ -119,8 +124,11 @@ let loadGeneration = 0; // camera, App coordinates the picker off SOURCE_ERROR. (The live-update poll // below is a SEPARATE op — a background refresh of the current source that // shares only the MANIFEST sink, and yields to a foreground load via the gen.) -async function loadSource(payload: SourcePayload): Promise { +export async function loadSource(payload: SourcePayload): Promise { const myGen = ++loadGeneration; // claim authority; supersedes any in-flight load/poll + loadController?.abort(); // supersede any in-flight load + const controller = new AbortController(); + loadController = controller; const meta = { kind: srcKind(payload.src), label: labelFromSource(payload.src) ?? payload.src, @@ -136,9 +144,14 @@ async function loadSource(payload: SourcePayload): Promise { }); // Publish the skeleton as it streams (early structure, behind the overlay); // the final is published below, AFTER committing the source. - const manifest = await pumpManifestStream(url, meta, (m, phase) => { - if (phase === ScanPhase.PartialManifest && myGen === loadGeneration) setManifest(m); - }); + const manifest = await pumpManifestStream( + url, + meta, + (m, phase) => { + if (phase === ScanPhase.PartialManifest && myGen === loadGeneration) setManifest(m); + }, + controller.signal + ); if (myGen !== loadGeneration) return; // a newer load superseded this one // Commit the source BEFORE publishing the final manifest. The render layer's // camera-reframe reaction keys off CURRENT_SOURCE captured at apply-START, so @@ -148,6 +161,7 @@ async function loadSource(payload: SourcePayload): Promise { setManifest(manifest); } catch (err) { if (myGen !== loadGeneration) return; // superseded — its error isn't current + if (controller.signal.aborted) return; // user canceled: not an error, no SOURCE_ERROR SOURCE_ERROR.value = { error: err instanceof Error ? err.message : String(err), prefill: { src: payload.src, branch: payload.branch }, @@ -158,10 +172,20 @@ async function loadSource(payload: SourcePayload): Promise { if (myGen === loadGeneration) { SCAN_PROGRESS.value = null; PENDING_SOURCE_LABEL.value = null; + if (loadController === controller) loadController = null; } } } +/** + * Abort the in-flight foreground load, if any. Exposed to the UI (a cancel + * button on the loading overlay) via the hook's return; the abort is treated + * as a clean user cancel, not a failure — see the `catch` branch above. + */ +export function cancelLoad(): void { + loadController?.abort(); +} + // ── Live-update poll loop ──────────────────────────────────────────── // Hard bounds for the user-set poll interval. 1s floor — the server does a real @@ -275,10 +299,16 @@ function setupLiveUpdates(): () => void { * Scene-free — the render layer (the City component) consumes MANIFEST and paints the * scene. UI-free too: it never opens/closes the source picker; on a load failure * (boot or submit) it WRITES the canonical SOURCE_ERROR signal and App reacts to - * coordinate the picker. RETURNS the source-picker submit handler so App can pass - * it down to as a prop (no global register/invoke channel). + * coordinate the picker. A user-initiated cancel (`cancelLoad`) aborts the + * in-flight stream but is NOT surfaced as a load failure — no SOURCE_ERROR write. + * RETURNS the source-picker submit handler and a cancel handler so App can pass + * them down to /the loading overlay as props (no global + * register/invoke channel). */ -export function useManifestSource(): (payload: SourcePayload) => void { +export function useManifestSource(): { + submitSource: (payload: SourcePayload) => void; + cancelLoad: () => void; +} { const submitSource = useCallback((payload: SourcePayload) => { void loadSource(payload); }, []); @@ -311,5 +341,5 @@ export function useManifestSource(): (payload: SourcePayload) => void { }; }, []); - return submitSource; + return { submitSource, cancelLoad }; } diff --git a/app/src/layout/App/App.tsx b/app/src/layout/App/App.tsx index 62d173a6..f3384801 100644 --- a/app/src/layout/App/App.tsx +++ b/app/src/layout/App/App.tsx @@ -52,7 +52,7 @@ import { attachLoadingReactions } from '@/state/loadingReactions'; export function App() { useDocumentTitle(); - const submitSource = useManifestSource(); + const { submitSource } = useManifestSource(); useEffect(() => attachLoadingReactions(), []); diff --git a/app/tests/api/manifest.test.ts b/app/tests/api/manifest.test.ts index 69b54854..b5db715e 100644 --- a/app/tests/api/manifest.test.ts +++ b/app/tests/api/manifest.test.ts @@ -38,7 +38,7 @@ const fakeManifest = { root: '/r', tree: { type: 'directory' } }; describe('streamManifest (EventSource)', () => { it('maps named SSE events to ScanStreamEvents in order and stops after manifest-complete', async () => { const { ctor, last } = makeES(); - const it = streamManifest('/api/manifest?src=x', ctor)[Symbol.asyncIterator](); + const it = streamManifest('/api/manifest?src=x', { EventSourceImpl: ctor })[Symbol.asyncIterator](); const es = last(); es.emit('scan-progress', JSON.stringify({ display_root: 'x', files_scanned: 3 })); es.emit('manifest-partial', JSON.stringify({ manifest: fakeManifest })); @@ -57,7 +57,7 @@ describe('streamManifest (EventSource)', () => { it('maps a clone-progress event with display_root', async () => { const { ctor, last } = makeES(); - const it = streamManifest('/api/manifest', ctor)[Symbol.asyncIterator](); + const it = streamManifest('/api/manifest', { EventSourceImpl: ctor })[Symbol.asyncIterator](); last().emit('clone-progress', JSON.stringify({ display_root: 'https://example.com/foo.git' })); const a = await it.next(); const ev = a.value as ScanStreamEvent; @@ -70,7 +70,7 @@ describe('streamManifest (EventSource)', () => { it('emits a terminal Error event for a server-sent error', async () => { const { ctor, last } = makeES(); - const it = streamManifest('/api/manifest', ctor)[Symbol.asyncIterator](); + const it = streamManifest('/api/manifest', { EventSourceImpl: ctor })[Symbol.asyncIterator](); last().emit('error', JSON.stringify({ error: 'boom' })); const a = await it.next(); expect(a.value).toEqual({ phase: ScanPhase.Error, error: 'boom' }); @@ -80,15 +80,29 @@ describe('streamManifest (EventSource)', () => { it('rejects on a transport-level error (bare error event, no data)', async () => { const { ctor, last } = makeES(); - const it = streamManifest('/api/manifest', ctor)[Symbol.asyncIterator](); + const it = streamManifest('/api/manifest', { EventSourceImpl: ctor })[Symbol.asyncIterator](); last().emit('error'); // no data → connection failure await expect(it.next()).rejects.toThrow(/connection failed/i); }); it('rejects on a malformed event payload instead of hanging forever', async () => { const { ctor, last } = makeES(); - const it = streamManifest('/api/manifest', ctor)[Symbol.asyncIterator](); + const it = streamManifest('/api/manifest', { EventSourceImpl: ctor })[Symbol.asyncIterator](); last().emit('manifest-partial', '{not valid json'); // truncated/garbage frame await expect(it.next()).rejects.toThrow(/malformed/i); }); + + it('ends iteration when the signal aborts (no more events delivered)', async () => { + const { ctor, last } = makeES(); + const ac = new AbortController(); + const it = streamManifest('/api/manifest', { + signal: ac.signal, + EventSourceImpl: ctor, + })[Symbol.asyncIterator](); + const next = it.next(); // pending — no events yet + ac.abort(); + const r = await next; + expect(r.done).toBe(true); + expect(last().closed).toBe(true); // EventSource was closed on abort + }); }); diff --git a/app/tests/hooks/useManifestSource.test.ts b/app/tests/hooks/useManifestSource.test.ts new file mode 100644 index 00000000..6ab0e4b1 --- /dev/null +++ b/app/tests/hooks/useManifestSource.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { loadSource, cancelLoad } from '@/hooks/useManifestSource'; +import { SOURCE_ERROR, CURRENT_SOURCE } from '@/state/stores/source'; + +// Minimal EventSource stub that never emits — the load stays pending until +// canceled, so this test exercises exactly the abort path (no manifest ever +// arrives) rather than racing a real network response. +class StubEventSource { + static instances: StubEventSource[] = []; + closed = false; + constructor(public url: string) { + StubEventSource.instances.push(this); + } + addEventListener(): void {} + close(): void { + this.closed = true; + } +} + +describe('useManifestSource loadSource cancellation', () => { + let originalEventSource: typeof EventSource; + + beforeEach(() => { + originalEventSource = globalThis.EventSource; + StubEventSource.instances = []; + (globalThis as unknown as { EventSource: unknown }).EventSource = StubEventSource; + SOURCE_ERROR.value = null; + }); + + afterEach(() => { + (globalThis as unknown as { EventSource: unknown }).EventSource = originalEventSource; + }); + + it('canceling a load leaves SOURCE_ERROR null and CURRENT_SOURCE unchanged', async () => { + const before = CURRENT_SOURCE.value; + + const p = loadSource({ src: 'https://github.com/o/r' }); // starts the load + cancelLoad(); // aborts via loadController before any event arrives + + await p; + + expect(SOURCE_ERROR.value).toBeNull(); + expect(CURRENT_SOURCE.value).toBe(before); + expect(StubEventSource.instances[0]?.closed).toBe(true); + }); +}); From 200ed910d5e6736e962e302cce3dbae2ec9fb62d Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:44:35 -0400 Subject: [PATCH 06/12] fix(app): don't commit a skeleton manifest on cancel-after-skeleton (#77) streamManifest ends an aborted stream as done (not a throw), so a cancel landing after the skeleton but before the final made pumpManifestStream return the partial manifest and loadSource's success path committed it as CURRENT_SOURCE. Guard the commit with the abort signal, mirroring the catch-branch guard, so a user cancel never mutates CURRENT_SOURCE. Adds a cancel-after-skeleton regression test plus an already-aborted-at-iterator test that locks in the abort-wiring placement. --- app/src/hooks/useManifestSource.ts | 6 +++- app/tests/api/manifest.test.ts | 16 ++++++++++ app/tests/hooks/useManifestSource.test.ts | 39 ++++++++++++++++++++--- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/app/src/hooks/useManifestSource.ts b/app/src/hooks/useManifestSource.ts index 9edbe0f5..018d13d1 100644 --- a/app/src/hooks/useManifestSource.ts +++ b/app/src/hooks/useManifestSource.ts @@ -152,7 +152,11 @@ export async function loadSource(payload: SourcePayload): Promise { }, controller.signal ); - if (myGen !== loadGeneration) return; // a newer load superseded this one + // A newer load superseded this one, OR the user canceled after a skeleton + // arrived: streamManifest ends an aborted stream as done (not a throw), so + // pumpManifestStream RETURNS the partial manifest here. Guard the commit so + // a canceled load never writes CURRENT_SOURCE (mirrors the catch guard). + if (myGen !== loadGeneration || controller.signal.aborted) return; // Commit the source BEFORE publishing the final manifest. The render layer's // camera-reframe reaction keys off CURRENT_SOURCE captured at apply-START, so // the new key must be live for the FINAL apply (the one to frame on) and NOT diff --git a/app/tests/api/manifest.test.ts b/app/tests/api/manifest.test.ts index b5db715e..a539b4e6 100644 --- a/app/tests/api/manifest.test.ts +++ b/app/tests/api/manifest.test.ts @@ -105,4 +105,20 @@ describe('streamManifest (EventSource)', () => { expect(r.done).toBe(true); expect(last().closed).toBe(true); // EventSource was closed on abort }); + + it('ends immediately when the signal is already aborted before iteration', async () => { + const { ctor, last } = makeES(); + const ac = new AbortController(); + ac.abort(); // aborted BEFORE the iterator is created + const it = streamManifest('/api/manifest', { + signal: ac.signal, + EventSourceImpl: ctor, + })[Symbol.asyncIterator](); + // The abort-wiring block runs after finish() is declared, so an + // already-aborted signal closes the stream at iterator-creation time + // without a TDZ ReferenceError. + expect(last().closed).toBe(true); + const r = await it.next(); + expect(r.done).toBe(true); + }); }); diff --git a/app/tests/hooks/useManifestSource.test.ts b/app/tests/hooks/useManifestSource.test.ts index 6ab0e4b1..1d874040 100644 --- a/app/tests/hooks/useManifestSource.test.ts +++ b/app/tests/hooks/useManifestSource.test.ts @@ -2,21 +2,29 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { loadSource, cancelLoad } from '@/hooks/useManifestSource'; import { SOURCE_ERROR, CURRENT_SOURCE } from '@/state/stores/source'; -// Minimal EventSource stub that never emits — the load stays pending until -// canceled, so this test exercises exactly the abort path (no manifest ever -// arrives) rather than racing a real network response. +// EventSource stub with driveable events: records listeners so a test can emit +// a named SSE event (e.g. a `manifest-partial` skeleton), and records +// instances so a test can assert the stream was closed on abort. class StubEventSource { static instances: StubEventSource[] = []; closed = false; + private listeners: Record void)[]> = {}; constructor(public url: string) { StubEventSource.instances.push(this); } - addEventListener(): void {} + addEventListener(name: string, handler: (e: unknown) => void): void { + (this.listeners[name] ??= []).push(handler); + } close(): void { this.closed = true; } + emit(name: string, data: string): void { + for (const h of this.listeners[name] ?? []) h({ data }); + } } +const flush = (): Promise => new Promise((r) => setTimeout(r, 0)); + describe('useManifestSource loadSource cancellation', () => { let originalEventSource: typeof EventSource; @@ -43,4 +51,27 @@ describe('useManifestSource loadSource cancellation', () => { expect(CURRENT_SOURCE.value).toBe(before); expect(StubEventSource.instances[0]?.closed).toBe(true); }); + + it('canceling AFTER a skeleton manifest does not commit it as CURRENT_SOURCE', async () => { + const before = CURRENT_SOURCE.value; + + const p = loadSource({ src: 'https://github.com/o/r' }); + await flush(); // let the for-await attach its event listeners + + // Skeleton arrives, then the user cancels before the final manifest. An + // aborted stream ends done (not a throw), so pumpManifestStream RETURNS the + // skeleton — the success path must still refuse to commit it. + StubEventSource.instances[0]!.emit( + 'manifest-partial', + JSON.stringify({ manifest: { root: '/r', tree: { type: 'directory' }, signature: 'sig' } }) + ); + await flush(); + cancelLoad(); + + await p; + + expect(CURRENT_SOURCE.value).toBe(before); // NOT committed + expect(SOURCE_ERROR.value).toBeNull(); // cancel is not an error + expect(StubEventSource.instances[0]?.closed).toBe(true); + }); }); From 483dbf6c9d54ff4e9ef7e1948bc2288b5395a73b Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 17:51:25 -0400 Subject: [PATCH 07/12] feat(app): ProjectsView shell + recents-forward list (#77) --- app/src/views/ProjectsView/NewProjectForm.tsx | 17 +++ app/src/views/ProjectsView/ProjectsView.css | 39 +++++ app/src/views/ProjectsView/ProjectsView.tsx | 61 ++++++++ app/src/views/ProjectsView/RecentRow.tsx | 89 +++++++++++ app/src/views/ProjectsView/RecentsList.css | 141 ++++++++++++++++++ app/src/views/ProjectsView/RecentsList.tsx | 92 ++++++++++++ .../views/ProjectsView/RecentsList.test.tsx | 101 +++++++++++++ 7 files changed, 540 insertions(+) create mode 100644 app/src/views/ProjectsView/NewProjectForm.tsx create mode 100644 app/src/views/ProjectsView/ProjectsView.css create mode 100644 app/src/views/ProjectsView/ProjectsView.tsx create mode 100644 app/src/views/ProjectsView/RecentRow.tsx create mode 100644 app/src/views/ProjectsView/RecentsList.css create mode 100644 app/src/views/ProjectsView/RecentsList.tsx create mode 100644 app/tests/views/ProjectsView/RecentsList.test.tsx diff --git a/app/src/views/ProjectsView/NewProjectForm.tsx b/app/src/views/ProjectsView/NewProjectForm.tsx new file mode 100644 index 00000000..1e89e369 --- /dev/null +++ b/app/src/views/ProjectsView/NewProjectForm.tsx @@ -0,0 +1,17 @@ +// views/ProjectsView/NewProjectForm.tsx — stub; real form (Git URL / local path +// tabs, branch select, skip-cache) arrives in Task 8. Props are typed to match +// what ProjectsView already passes so the shell compiles against the real +// contract, not a loosened placeholder. + +import type { SourcePayload } from '@/state/stores/ui'; + +export interface NewProjectFormProps { + allowLocalRepos: boolean; + prefill?: SourcePayload; + onSubmit: (payload: SourcePayload) => void; + onCancel: () => void; +} + +export function NewProjectForm(_props: NewProjectFormProps) { + return null; +} diff --git a/app/src/views/ProjectsView/ProjectsView.css b/app/src/views/ProjectsView/ProjectsView.css new file mode 100644 index 00000000..cb3cb725 --- /dev/null +++ b/app/src/views/ProjectsView/ProjectsView.css @@ -0,0 +1,39 @@ +/* ── Projects view ────────────────────────────────────────────────────────── + Full-viewport surface (replaces the old .modal-backdrop overlay): opaque, + fixed, above the app shell, flex-centered panel. Panel surface (bg + radius + + shadow + color) comes from .card-overlay; this file adds size + layout. */ + +.projects-view { + position: fixed; + inset: 0; + background: var(--cc-bg-app); + display: flex; + align-items: center; + justify-content: center; + z-index: 1000; +} + +.projects-view-panel { + width: 640px; + max-width: 92vw; + max-height: 86vh; + display: flex; + flex-direction: column; + overflow: hidden; + font-family: inherit; +} + +.projects-view-header { + display: flex; + align-items: center; + justify-content: space-between; + padding: var(--cc-space-5) var(--cc-space-6); + border-bottom: 1px solid var(--cc-border-subtle); + font-weight: var(--cc-fw-semibold); +} + +.projects-view-body { + padding: var(--cc-space-5) var(--cc-space-6); + overflow-y: auto; + flex: 1; +} diff --git a/app/src/views/ProjectsView/ProjectsView.tsx b/app/src/views/ProjectsView/ProjectsView.tsx new file mode 100644 index 00000000..b8ca2c19 --- /dev/null +++ b/app/src/views/ProjectsView/ProjectsView.tsx @@ -0,0 +1,61 @@ +// views/ProjectsView/ProjectsView.tsx — full-viewport project switcher. Reads +// PROJECTS_VIEW + SERVER_CONFIG for open-state/prefill; App owns +// onSubmit/onCancel/onClose. Renders null when closed so the form/list state +// resets on next open. Replaces the old modal SourcePicker. + +import './ProjectsView.css'; +import { useEffect } from 'preact/hooks'; +import { X } from 'lucide-preact'; +import { PROJECTS_VIEW, type SourcePayload } from '@/state/stores/ui'; +import { SERVER_CONFIG } from '@/state/stores/serverConfig'; +import { SCAN_PROGRESS } from '@/state/stores/scanProgress'; +import { NewProjectForm } from './NewProjectForm'; +import { RecentsList } from './RecentsList'; + +export interface ProjectsViewProps { + onSubmit: (payload: SourcePayload) => void; + onCancel: () => void; + onClose: () => void; +} + +export function ProjectsView({ onSubmit, onCancel, onClose }: ProjectsViewProps) { + const pv = PROJECTS_VIEW.value; + const loading = SCAN_PROGRESS.value !== null; + + // Escape closes the view when dismissible and not mid-load. + useEffect(() => { + if (!pv.visible || !pv.opts.dismissible) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !loading) onClose(); + }; + document.addEventListener('keydown', onKey); + return () => document.removeEventListener('keydown', onKey); + }, [pv.visible, pv.opts.dismissible, loading, onClose]); + + if (!pv.visible) return null; + + return ( + + ); +} diff --git a/app/src/views/ProjectsView/RecentRow.tsx b/app/src/views/ProjectsView/RecentRow.tsx new file mode 100644 index 00000000..d18cfb4e --- /dev/null +++ b/app/src/views/ProjectsView/RecentRow.tsx @@ -0,0 +1,89 @@ +// views/ProjectsView/RecentRow.tsx — one recent-source row: kind glyph, label, +// sub (src + branch pill + "(default)" tag), active badge, inline-confirm +// remove. + +import { Folder, Trash2, TriangleAlert } from 'lucide-preact'; +import { HostingIcon } from '@/components/HostingIcon'; +import { srcKind, SourceKind } from '@/utils/sources'; +import type { RecentSource } from '@/state/stores/source'; + +export interface RecentRowProps { + recent: RecentSource; + active: boolean; + disabled: boolean; // local row while local repos are off + highlighted: boolean; // keyboard cursor + confirmingRemove: boolean; + onOpen: () => void; + onAskRemove: () => void; + onConfirmRemove: () => void; + onCancelRemove: () => void; +} + +export function RecentRow(props: RecentRowProps) { + const { recent: r, active, disabled, highlighted, confirmingRemove } = props; + const isLocal = srcKind(r.src) === SourceKind.Local; + const rowClass = [ + 'row recent-row', + active && 'recent-row--active', + disabled && 'recent-row--disabled', + highlighted && 'recent-row--highlighted', + ] + .filter(Boolean) + .join(' '); + + return ( +
+ + + {confirmingRemove ? ( + + Remove? + + + + ) : ( + + )} +
+ ); +} diff --git a/app/src/views/ProjectsView/RecentsList.css b/app/src/views/ProjectsView/RecentsList.css new file mode 100644 index 00000000..aadd8087 --- /dev/null +++ b/app/src/views/ProjectsView/RecentsList.css @@ -0,0 +1,141 @@ +/* ── Recents hero ──────────────────────────────────────────────────────── + Filter box + keyboard-navigable list. Rules ported from the old + SourcePicker's .recents-list block; renamed to the new recents-forward + structure (filter input, .recent-row--highlighted keyboard cursor, + inline remove-confirm). */ + +.recents { + margin-top: var(--cc-space-7); + border-top: 1px solid var(--cc-border-subtle); + padding-top: var(--cc-space-5); +} + +.recents-filter { + width: 100%; + background-color: var(--cc-bg-chrome); + border: 1px solid var(--cc-border-input); + border-radius: var(--cc-radius-sm); + color: var(--cc-text-primary); + font: inherit; + font-size: var(--cc-font-lg); + padding: var(--cc-space-2) var(--cc-space-4); + outline: none; + margin-bottom: var(--cc-space-4); +} +.recents-filter::placeholder { + color: var(--cc-text-faint); +} +.recents-filter:focus { + border-color: var(--cc-accent-border); +} + +.recents-list { + display: flex; + flex-direction: column; + gap: var(--cc-space-1); +} + +/* Each recent project is a .recent-item row: a .recent-row click target that + opens the project, plus a sibling remove control. Kept as siblings (not + parent/child) so the row's hover highlight has symmetric padding and the + remove control has its own independent hit area. */ +.recent-item { + display: flex; + align-items: center; + gap: var(--cc-space-2); +} + +.recent-row { + flex: 1 1 auto; + min-width: 0; + appearance: none; + background: none; + border: none; + text-align: left; + color: inherit; + font: inherit; + gap: var(--cc-space-5); + padding: var(--cc-space-3) var(--cc-space-4); + border-radius: var(--cc-radius-md); +} +.recent-row:hover { + background: var(--cc-overlay-bg); +} +.recent-row-body { + flex: 1 1 auto; + min-width: 0; +} +.recent-row .recent-label { + font-weight: var(--cc-fw-medium); +} +.recent-row .recent-sub { + display: flex; + align-items: center; + gap: var(--cc-space-2); + font-size: var(--cc-font-md); + color: var(--cc-text-muted); +} +.recent-row .recent-src { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} +.recent-default-tag { + flex: 0 0 auto; + color: var(--cc-text-faint); +} + +.recent-row--active { + background: var(--cc-accent-bg-soft); + cursor: default; +} +.recent-row--active:hover { + background: var(--cc-accent-bg); +} + +.recent-row--disabled { + opacity: 0.55; + cursor: not-allowed; +} +.recent-row--disabled:hover { + background: transparent; +} + +/* Keyboard cursor — accent inset ring, distinct from hover/active fills so + arrow-key navigation stays visible even over an active/disabled row. */ +.recent-row--highlighted { + box-shadow: inset 0 0 0 1px var(--cc-accent-border); +} + +.recent-row-badge { + margin-left: auto; + font-size: var(--cc-font-sm); + font-weight: var(--cc-fw-semibold); + letter-spacing: var(--cc-track-label); + color: var(--cc-accent); + background: var(--cc-accent-bg); + border: 1px solid var(--cc-accent-border); + border-radius: var(--cc-radius-lg); + padding: var(--cc-space-1) var(--cc-space-3); + white-space: nowrap; +} + +/* Destructive trash button next to each recent. */ +.recent-item > .btn-icon[aria-label='Remove from recents'] { + padding: var(--cc-space-4) var(--cc-space-2); + color: var(--cc-error); +} +.recent-item > .btn-icon[aria-label='Remove from recents']:hover { + color: var(--cc-error-light); + background: var(--cc-error-bg); +} + +/* Inline "Remove? [Cancel] [Remove]" confirm — swaps in for the trash button + so removing is a two-step, low-risk action rather than a native confirm(). */ +.recent-remove-confirm { + display: flex; + align-items: center; + gap: var(--cc-space-2); + flex: 0 0 auto; + white-space: nowrap; +} diff --git a/app/src/views/ProjectsView/RecentsList.tsx b/app/src/views/ProjectsView/RecentsList.tsx new file mode 100644 index 00000000..31a4319b --- /dev/null +++ b/app/src/views/ProjectsView/RecentsList.tsx @@ -0,0 +1,92 @@ +// views/ProjectsView/RecentsList.tsx — the recents hero: filter box + keyboard- +// navigable list. Active state derives from CURRENT_SOURCE (single source of +// truth, not the URL). Remove is non-destructive: it forgets the entry only, +// it does not clear the scan cache (that's the skip-cache control's job). + +import './RecentsList.css'; +import { useMemo, useState } from 'preact/hooks'; +import { listRecents, removeRecent, CURRENT_SOURCE } from '@/state/stores/source'; +import { SERVER_CONFIG } from '@/state/stores/serverConfig'; +import { srcKind, SourceKind } from '@/utils/sources'; +import type { SourcePayload } from '@/state/stores/ui'; +import { RecentRow } from './RecentRow'; + +export interface RecentsListProps { + onOpen: (payload: SourcePayload) => void; +} + +export function RecentsList({ onOpen }: RecentsListProps) { + const recents = listRecents(); // reads RECENTS signal + const cur = CURRENT_SOURCE.value; + const allowLocal = SERVER_CONFIG.value.allowLocalRepos; + const [filter, setFilter] = useState(''); + const [cursor, setCursor] = useState(0); + const [confirming, setConfirming] = useState(null); // key of row + + const q = filter.trim().toLowerCase(); + const shown = useMemo( + () => + recents.filter( + (r) => !q || r.label.toLowerCase().includes(q) || r.src.toLowerCase().includes(q) + ), + [recents, q] + ); + + const keyOf = (r: { src: string; branch?: string }) => `${r.src}:${r.branch ?? ''}`; + const isActive = (r: { src: string; branch?: string }) => + !!cur && r.src === cur.src && (r.branch ?? '') === (cur.branch ?? ''); + const isDisabled = (r: { src: string }) => srcKind(r.src) === SourceKind.Local && !allowLocal; + + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'ArrowDown') { + e.preventDefault(); + setCursor((c) => Math.min(c + 1, shown.length - 1)); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + setCursor((c) => Math.max(c - 1, 0)); + } else if (e.key === 'Enter') { + const r = shown[cursor]; + if (r && !isActive(r) && !isDisabled(r)) onOpen({ src: r.src, branch: r.branch }); + } + }; + + if (recents.length === 0) return null; + + return ( +
+ { + setFilter((e.target as HTMLInputElement).value); + setCursor(0); + }} + autoComplete="off" + spellcheck={false} + /> +
+ {shown.map((r, i) => ( + { + if (!isActive(r) && !isDisabled(r)) onOpen({ src: r.src, branch: r.branch }); + }} + onAskRemove={() => setConfirming(keyOf(r))} + onCancelRemove={() => setConfirming(null)} + onConfirmRemove={() => { + removeRecent(r.src, r.branch); + setConfirming(null); + }} + /> + ))} +
+
+ ); +} diff --git a/app/tests/views/ProjectsView/RecentsList.test.tsx b/app/tests/views/ProjectsView/RecentsList.test.tsx new file mode 100644 index 00000000..8c250577 --- /dev/null +++ b/app/tests/views/ProjectsView/RecentsList.test.tsx @@ -0,0 +1,101 @@ +// Native-harness tests for RecentsList — mirrors app/tests/layout/leftSidebar.test.tsx's +// render/flush/container pattern (this repo has no @testing-library/preact dependency). + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from 'preact'; +import { RECENTS, CURRENT_SOURCE } from '@/state/stores/source'; +import { SERVER_CONFIG } from '@/state/stores/serverConfig'; +import { RecentsList } from '@/views/ProjectsView/RecentsList'; +import * as manifestApi from '@/api/manifest'; +import { flush } from '../../_helpers/preact'; + +describe('RecentsList', () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + RECENTS.value = [ + { src: 'https://github.com/o/alpha', branch: 'main', branchIsDefault: true, label: 'o/alpha', lastOpenedAt: 2 }, + { src: 'https://github.com/o/beta', branch: 'dev', label: 'o/beta', lastOpenedAt: 1 }, + ]; + CURRENT_SOURCE.value = { src: 'https://github.com/o/alpha', branch: 'main' }; + }); + + afterEach(() => { + render(null, container); + document.body.removeChild(container); + RECENTS.value = []; + CURRENT_SOURCE.value = null; + SERVER_CONFIG.value = { allowLocalRepos: false }; + }); + + it('marks the CURRENT_SOURCE row active and tags default branches', async () => { + render( {}} />, container); + await flush(); + + const activeRow = container.querySelector('.recent-row--active'); + expect(activeRow).toBeTruthy(); + expect(activeRow?.textContent).toContain('o/alpha'); + expect(container.textContent).toContain('(default)'); + }); + + it('filters by label/src as you type', async () => { + render( {}} />, container); + await flush(); + + const filterInput = container.querySelector('.recents-filter')!; + filterInput.value = 'beta'; + filterInput.dispatchEvent(new Event('input', { bubbles: true })); + await flush(); + + const labels = Array.from(container.querySelectorAll('.recent-label')).map((el) => el.textContent); + expect(labels).not.toContain('o/alpha'); + expect(labels).toContain('o/beta'); + }); + + it('remove is non-destructive: forgets the entry, does not touch the cache', async () => { + const spy = vi.spyOn(manifestApi, 'clearManifestCache'); + render( {}} />, container); + await flush(); + + const removeButtons = container.querySelectorAll( + '[aria-label="Remove from recents"]' + ); + removeButtons[0].click(); // alpha row -> ask + await flush(); + + const confirmButtons = Array.from(container.querySelectorAll('button')).filter( + (b) => b.textContent === 'Remove' + ); + expect(confirmButtons.length).toBeGreaterThan(0); + (confirmButtons[0] as HTMLButtonElement).click(); + await flush(); + + expect(RECENTS.value.find((r) => r.label === 'o/alpha')).toBeUndefined(); + expect(spy).not.toHaveBeenCalled(); + }); + + it('supports keyboard navigation: arrow keys move the highlight, Enter opens it', async () => { + const onOpen = vi.fn(); + render(, container); + await flush(); + + const list = container.querySelector('.recents')!; + // Cursor starts at 0 (alpha, which is active/disabled-for-open); move down to beta. + list.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true })); + await flush(); + + const highlighted = container.querySelector('.recent-row--highlighted'); + expect(highlighted?.textContent).toContain('o/beta'); + + list.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await flush(); + + expect(onOpen).toHaveBeenCalledWith({ src: 'https://github.com/o/beta', branch: 'dev' }); + + list.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowUp', bubbles: true })); + await flush(); + expect(container.querySelector('.recent-row--highlighted')?.textContent).toContain('o/alpha'); + }); +}); From 1c258d3b6667fc8b2e4ac53c66529059998b4d27 Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 18:07:09 -0400 Subject: [PATCH 08/12] feat(app): new-project form + repo-resolved branch dropdown (#77) --- app/src/views/ProjectsView/BranchSelect.css | 18 ++ app/src/views/ProjectsView/BranchSelect.tsx | 87 +++++++++ app/src/views/ProjectsView/NewProjectForm.css | 61 +++++++ app/src/views/ProjectsView/NewProjectForm.tsx | 165 ++++++++++++++++- .../views/ProjectsView/BranchSelect.test.tsx | 98 ++++++++++ .../ProjectsView/NewProjectForm.test.tsx | 172 ++++++++++++++++++ 6 files changed, 595 insertions(+), 6 deletions(-) create mode 100644 app/src/views/ProjectsView/BranchSelect.css create mode 100644 app/src/views/ProjectsView/BranchSelect.tsx create mode 100644 app/src/views/ProjectsView/NewProjectForm.css create mode 100644 app/tests/views/ProjectsView/BranchSelect.test.tsx create mode 100644 app/tests/views/ProjectsView/NewProjectForm.test.tsx diff --git a/app/src/views/ProjectsView/BranchSelect.css b/app/src/views/ProjectsView/BranchSelect.css new file mode 100644 index 00000000..4769012e --- /dev/null +++ b/app/src/views/ProjectsView/BranchSelect.css @@ -0,0 +1,18 @@ +/* ── Branch select ── */ + +.branch-select-status { + display: flex; + align-items: center; + gap: var(--cc-space-3); + font-size: var(--cc-font-lg); + color: var(--cc-text-muted); +} + +.branch-select-spinner { + animation: branch-select-spin 0.8s linear infinite; +} +@keyframes branch-select-spin { + to { + transform: rotate(360deg); + } +} diff --git a/app/src/views/ProjectsView/BranchSelect.tsx b/app/src/views/ProjectsView/BranchSelect.tsx new file mode 100644 index 00000000..92e1489c --- /dev/null +++ b/app/src/views/ProjectsView/BranchSelect.tsx @@ -0,0 +1,87 @@ +// views/ProjectsView/BranchSelect.tsx — repo-resolved branch dropdown. Fetches +// the branch list for `url` and preselects the repo default. Owns no +// url-change tracking of its own: the parent re-keys it per resolved url +// (key={url}), so a branch pick from a previous repo can never leak into the +// next one — that remount is the actual fix for bug #2, not anything here. + +import './BranchSelect.css'; +import { useEffect, useState } from 'preact/hooks'; +import { LoaderCircle } from 'lucide-preact'; +import { fetchBranches } from '@/api/branches'; + +export interface BranchSelectProps { + url: string; // a resolvable git URL, or '' to stay idle + value: string; // '' means "use the repo default" + onChange: (branch: string) => void; +} + +type State = + | { status: 'idle' } + | { status: 'loading' } + | { status: 'ready'; branches: string[]; def: string | null } + | { status: 'error'; message: string }; + +export function BranchSelect({ url, value, onChange }: BranchSelectProps) { + const [state, setState] = useState({ status: 'idle' }); + + useEffect(() => { + if (!url) { + setState({ status: 'idle' }); + return; + } + let live = true; + setState({ status: 'loading' }); + fetchBranches(url).then( + (r) => { + if (!live) return; + setState({ status: 'ready', branches: r.branches, def: r.default }); + if (r.default) onChange(r.default); // preselect the repo default + }, + (e: unknown) => { + if (!live) return; + setState({ status: 'error', message: e instanceof Error ? e.message : String(e) }); + } + ); + return () => { + live = false; + }; + // url is the only trigger here — the parent remounts this component per + // repo (key={url}), so there's no stale-onChange/value closure risk to + // guard against by widening this dependency list. + }, [url]); + + if (state.status === 'idle') return null; + + // .new-project-field is the shared field-wrapper layout from + // NewProjectForm.css — safe to reuse because this component only ever + // renders inside , never standalone. + return ( +
+ + {state.status === 'loading' && ( +
+ + Resolving branches… +
+ )} + {state.status === 'error' && ( +
{state.message}
+ )} + {state.status === 'ready' && ( + + )} +
+ ); +} diff --git a/app/src/views/ProjectsView/NewProjectForm.css b/app/src/views/ProjectsView/NewProjectForm.css new file mode 100644 index 00000000..591cfa02 --- /dev/null +++ b/app/src/views/ProjectsView/NewProjectForm.css @@ -0,0 +1,61 @@ +/* ── New project form ── */ + +.new-project { + display: flex; + flex-direction: column; + gap: var(--cc-space-6); +} + +.new-project--loading { + flex-direction: row; + align-items: center; + justify-content: space-between; +} + +.new-project-field { + display: flex; + flex-direction: column; + gap: var(--cc-space-3); +} +.new-project-field label { + font-size: var(--cc-font-lg); + color: var(--cc-text-muted); +} +.new-project-field .form-input { + width: 100%; +} + +/* Inline notice for the disabled-local state. Quieter than .card-error: + subtle amber tint, no border, single line — the old modal's version was a + multi-paragraph wall, this one stays scannable. */ +.new-project-note { + background: var(--cc-warning-bg); + color: var(--cc-text-secondary); + border-radius: var(--cc-radius-md); + padding: var(--cc-space-4) var(--cc-space-5); + font-size: var(--cc-font-lg); +} + +.new-project-advanced-toggle { + appearance: none; + background: none; + border: none; + align-self: flex-start; + color: var(--cc-text-faint); + font: inherit; + font-size: var(--cc-font-md); + cursor: pointer; + padding: 0; +} +.new-project-advanced-toggle:hover { + color: var(--cc-text-secondary); +} + +.new-project-skip-cache { + display: flex; + align-items: center; + gap: var(--cc-space-3); + font-size: var(--cc-font-lg); + color: var(--cc-text-secondary); + margin-top: calc(var(--cc-space-4) * -1); +} diff --git a/app/src/views/ProjectsView/NewProjectForm.tsx b/app/src/views/ProjectsView/NewProjectForm.tsx index 1e89e369..7559c439 100644 --- a/app/src/views/ProjectsView/NewProjectForm.tsx +++ b/app/src/views/ProjectsView/NewProjectForm.tsx @@ -1,9 +1,16 @@ -// views/ProjectsView/NewProjectForm.tsx — stub; real form (Git URL / local path -// tabs, branch select, skip-cache) arrives in Task 8. Props are typed to match -// what ProjectsView already passes so the shell compiles against the real -// contract, not a loosened placeholder. +// views/ProjectsView/NewProjectForm.tsx — new-source entry. A single source +// field that classifies itself as you type (srcKind) and drives a Git URL / +// Local path segment — manual override still works via the segment itself. +// Git sources get a repo-resolved branch dropdown; skip-cache is tucked +// behind an Advanced disclosure so the common path stays a one-field form. +import './NewProjectForm.css'; +import { useState } from 'preact/hooks'; +import { SegmentedSelect } from '@/components/SegmentedSelect/SegmentedSelect'; +import { srcKind, SourceKind } from '@/utils/sources'; import type { SourcePayload } from '@/state/stores/ui'; +import { SCAN_PROGRESS } from '@/state/stores/scanProgress'; +import { BranchSelect } from './BranchSelect'; export interface NewProjectFormProps { allowLocalRepos: boolean; @@ -12,6 +19,152 @@ export interface NewProjectFormProps { onCancel: () => void; } -export function NewProjectForm(_props: NewProjectFormProps) { - return null; +const KIND_OPTIONS = [ + { value: SourceKind.Remote, label: 'Git URL' }, + { value: SourceKind.Local, label: 'Local path' }, +]; + +// A source string worth resolving branches for: has a scheme (https://, +// ssh://, ...) or the scp-form host (user@host:path). Guards against firing +// /api/branches on every keystroke of a half-typed URL. +function looksResolvable(v: string): boolean { + return srcKind(v) === SourceKind.Remote && (/:\/\/.+\/.+/.test(v) || /^[^@]+@[^:]+:.+/.test(v)); +} + +export function NewProjectForm({ + allowLocalRepos, + prefill, + onSubmit, + onCancel, +}: NewProjectFormProps) { + const initialKind = prefill?.src && allowLocalRepos ? srcKind(prefill.src) : SourceKind.Remote; + const [kind, setKind] = useState(initialKind); + const [source, setSource] = useState(prefill?.src ?? ''); + const [branch, setBranch] = useState(prefill?.branch ?? ''); + const [resolvedUrl, setResolvedUrl] = useState( + initialKind === SourceKind.Remote && prefill?.src && looksResolvable(prefill.src) + ? prefill.src + : '' + ); + const [skipCache, setSkipCache] = useState(false); + const [advanced, setAdvanced] = useState(false); + + const loading = SCAN_PROGRESS.value !== null; + const localOff = kind === SourceKind.Local && !allowLocalRepos; + + // Smart kind auto-select: reclassify on every keystroke, in either + // direction (a URL pasted into what was the Local tab flips back to Git). + // A full clear keeps the current kind rather than snapping to Local on + // empty input. Clicking the segment directly (onCommit below) is a manual + // override that bypasses this — it always wins until the next keystroke. + function onSourceInput(v: string) { + setSource(v); + const k = v.trim() ? srcKind(v) : kind; + if (k !== kind) setKind(k); + if (k === SourceKind.Remote) { + // Branch reset on URL change (bug #2): a stale pick from a previous + // repo must never ride along to whatever's typed now. BranchSelect is + // also remounted below (key={resolvedUrl}), so its internal fetch + // state can't straddle two repos either. + setBranch(''); + setResolvedUrl(looksResolvable(v) ? v : ''); + } + } + + const activeSrc = source.trim(); + const canSubmit = !loading && !localOff && activeSrc.length > 0; + + function submit() { + if (!canSubmit) return; + onSubmit({ + src: activeSrc, + branch: kind === SourceKind.Remote ? branch.trim() || undefined : undefined, + skipCache: skipCache || undefined, + }); + } + + if (loading) { + // Inline scan progress lives at the ProjectsView level (Task 9); the form + // collapses to just a cancel affordance while a load is in flight. + return ( +
+ Opening project… + +
+ ); + } + + return ( +
+ setKind(v as SourceKind)} + /> + + {!localOff && ( +
+ + onSourceInput((e.target as HTMLInputElement).value)} + /> +
+ )} + + {kind === SourceKind.Remote && ( + + )} + + {localOff && ( +
+ Local repos are off.{' '} + + How to enable + +
+ )} + + + {advanced && ( + + )} + + +
+ ); } diff --git a/app/tests/views/ProjectsView/BranchSelect.test.tsx b/app/tests/views/ProjectsView/BranchSelect.test.tsx new file mode 100644 index 00000000..06da84a8 --- /dev/null +++ b/app/tests/views/ProjectsView/BranchSelect.test.tsx @@ -0,0 +1,98 @@ +// Native-harness tests for BranchSelect — mirrors app/tests/layout/leftSidebar.test.tsx's +// render/flush/container pattern (this repo has no @testing-library/preact dependency). + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from 'preact'; +import { BranchSelect } from '@/views/ProjectsView/BranchSelect'; +import * as branchesApi from '@/api/branches'; +import { flush, drainAsync } from '../../_helpers/preact'; + +describe('BranchSelect', () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + render(null, container); + document.body.removeChild(container); + vi.restoreAllMocks(); + }); + + it('renders nothing while idle (no url)', async () => { + render( {}} />, container); + await flush(); + expect(container.querySelector('.branch-select')).toBeNull(); + }); + + it('shows a resolving state, then the dropdown with the repo default preselected and marked', async () => { + // Manually-controlled promise (rather than mockResolvedValue) so the + // loading assertion below can't race past the resolution on the + // microtask queue — it only settles once resolveFetch is called. + let resolveFetch!: (r: { branches: string[]; default: string | null }) => void; + vi.spyOn(branchesApi, 'fetchBranches').mockImplementation( + () => + new Promise((resolve) => { + resolveFetch = resolve; + }) + ); + const onChange = vi.fn(); + + render(, container); + // The mocked fetch never settles until resolveFetch() runs below, so it's + // safe to drain generously here — the component can only be idle or + // loading, never past it (Preact's effect scheduling has a real-timer + // hop, so a single microtask flush() is not enough to observe this). + await drainAsync(); + expect(container.querySelector('.branch-select-status')?.textContent).toMatch(/resolving/i); + + resolveFetch({ branches: ['main', 'dev'], default: 'main' }); + await drainAsync(); + + // The repo default is preselected via onChange (the parent owns `value`). + expect(onChange).toHaveBeenCalledWith('main'); + const select = container.querySelector('select')!; + expect(select).not.toBeNull(); + const optionTexts = Array.from(select.options).map((o) => o.textContent); + expect(optionTexts).toContain('main (default)'); + expect(optionTexts).toContain('dev'); + }); + + it('shows the server error message inline on resolution failure, with no dropdown', async () => { + vi.spyOn(branchesApi, 'fetchBranches').mockRejectedValue(new Error('repository not found')); + + render( + {}} />, + container + ); + await drainAsync(); + + expect(container.textContent).toMatch(/repository not found/i); + expect(container.querySelector('select')).toBeNull(); + }); + + it('re-resolves when the url prop changes (parent typically remounts via key instead)', async () => { + const resolve = vi.spyOn(branchesApi, 'fetchBranches'); + resolve.mockResolvedValueOnce({ branches: ['main'], default: 'main' }); + const onChange = vi.fn(); + + render( + , + container + ); + await drainAsync(); + expect(resolve).toHaveBeenCalledWith('https://github.com/o/first'); + + resolve.mockResolvedValueOnce({ branches: ['trunk'], default: 'trunk' }); + render( + , + container + ); + await drainAsync(); + + expect(resolve).toHaveBeenCalledWith('https://github.com/o/second'); + expect(onChange).toHaveBeenCalledWith('trunk'); + }); +}); diff --git a/app/tests/views/ProjectsView/NewProjectForm.test.tsx b/app/tests/views/ProjectsView/NewProjectForm.test.tsx new file mode 100644 index 00000000..f0e0c29d --- /dev/null +++ b/app/tests/views/ProjectsView/NewProjectForm.test.tsx @@ -0,0 +1,172 @@ +// Native-harness tests for NewProjectForm — mirrors app/tests/layout/leftSidebar.test.tsx's +// render/flush/container pattern (this repo has no @testing-library/preact dependency). + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from 'preact'; +import { NewProjectForm } from '@/views/ProjectsView/NewProjectForm'; +import * as branchesApi from '@/api/branches'; +import { SCAN_PROGRESS } from '@/state/stores/scanProgress'; +import { flush, drainAsync } from '../../_helpers/preact'; + +function setInput(el: HTMLInputElement, value: string) { + el.value = value; + el.dispatchEvent(new Event('input', { bubbles: true })); +} + +function activeSegment(container: HTMLElement): string | undefined { + return container.querySelector('.btn-toggle.is-active')?.textContent ?? undefined; +} + +describe('NewProjectForm', () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + render(null, container); + document.body.removeChild(container); + SCAN_PROGRESS.value = null; + vi.restoreAllMocks(); + }); + + it('auto-selects Git for a URL and Local for a bare path, but a manual click still overrides it', async () => { + render( {}} onCancel={() => {}} />, container); + await flush(); + expect(activeSegment(container)).toBe('Git URL'); + + const urlInput = container.querySelector('input[aria-label="URL"]')!; + setInput(urlInput, '/Users/thalida/repo'); + await flush(); + expect(activeSegment(container)).toBe('Local path'); + + const pathInput = container.querySelector('input[aria-label="Path"]')!; + setInput(pathInput, 'https://github.com/o/r'); + await flush(); + expect(activeSegment(container)).toBe('Git URL'); + + // Manual override: force Local even though the typed text still looks remote. + const localTab = Array.from(container.querySelectorAll('button')).find( + (b) => b.textContent === 'Local path' + )!; + localTab.click(); + await flush(); + expect(activeSegment(container)).toBe('Local path'); + }); + + it('resets the branch when the URL changes to a different repo (bug #2)', async () => { + const resolve = vi.spyOn(branchesApi, 'fetchBranches'); + resolve.mockResolvedValueOnce({ branches: ['main', 'feat'], default: 'main' }); + const onSubmit = vi.fn(); + + render( {}} />, container); + await flush(); + + const urlInput = container.querySelector('input[aria-label="URL"]')!; + setInput(urlInput, 'https://github.com/o/first'); + await drainAsync(); + + // User picks the non-default branch on the first repo. + const select = container.querySelector('select')!; + select.value = 'feat'; + select.dispatchEvent(new Event('change', { bubbles: true })); + await flush(); + expect(select.value).toBe('feat'); + + // Now point the field at a different repo entirely. This repo has no + // detectable default (default: null), so nothing auto-preselects a + // branch for it — if the stale 'feat' pick isn't explicitly cleared, + // it's the only branch value left standing and rides along unnoticed. + resolve.mockResolvedValueOnce({ branches: ['main'], default: null }); + setInput(urlInput, 'https://github.com/o/second'); + await drainAsync(); + + const submitBtn = container.querySelector('[aria-label="Open project"]')!; + submitBtn.click(); + await flush(); + + const payload = onSubmit.mock.calls.at(-1)?.[0]; + expect(payload.src).toBe('https://github.com/o/second'); + // The load-bearing assertion: 'feat' (the FIRST repo's pick) must never + // ride along to the second repo's submit. + expect(payload.branch).not.toBe('feat'); + expect(payload.branch).toBeUndefined(); + }); + + it('stays submit-enabled when branch resolution fails (forgiving — server resolves default)', async () => { + vi.spyOn(branchesApi, 'fetchBranches').mockRejectedValue(new Error('repository not found')); + const onSubmit = vi.fn(); + + render( {}} />, container); + await flush(); + + const urlInput = container.querySelector('input[aria-label="URL"]')!; + setInput(urlInput, 'https://github.com/o/nope'); + await drainAsync(); + + expect(container.textContent).toMatch(/repository not found/i); + const submitBtn = container.querySelector('[aria-label="Open project"]')!; + expect(submitBtn.disabled).toBe(false); + + submitBtn.click(); + await flush(); + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ src: 'https://github.com/o/nope' }) + ); + }); + + it('demotes skip-cache to an off-by-default Advanced disclosure', async () => { + render( {}} onCancel={() => {}} />, container); + await flush(); + + expect(container.querySelector('input[type="checkbox"]')).toBeNull(); + + const toggle = container.querySelector('.new-project-advanced-toggle')!; + expect(toggle.getAttribute('aria-expanded')).toBe('false'); + toggle.click(); + await flush(); + + expect(toggle.getAttribute('aria-expanded')).toBe('true'); + const checkbox = container.querySelector('input[type="checkbox"]')!; + expect(checkbox).not.toBeNull(); + expect(checkbox.checked).toBe(false); + }); + + it('shows a concise disabled-local note and blocks submit when local repos are off', async () => { + render( + {}} onCancel={() => {}} />, + container + ); + await flush(); + + const localTab = Array.from(container.querySelectorAll('button')).find( + (b) => b.textContent === 'Local path' + )!; + localTab.click(); + await flush(); + + expect(container.textContent).toMatch(/local repos are off/i); + // Concise, not a wall: no leftover path input competing with the note. + expect(container.querySelector('input[aria-label="Path"]')).toBeNull(); + + const submitBtn = container.querySelector('[aria-label="Open project"]')!; + expect(submitBtn.disabled).toBe(true); + }); + + it('never uses an em-dash in the disabled-local copy', async () => { + render( + {}} onCancel={() => {}} />, + container + ); + await flush(); + const localTab = Array.from(container.querySelectorAll('button')).find( + (b) => b.textContent === 'Local path' + )!; + localTab.click(); + await flush(); + + expect(container.querySelector('.new-project-note')?.textContent).not.toMatch(/—/); + }); +}); From 83313b9a9892c7a0374f6ca8dfba7617790ac9c6 Mon Sep 17 00:00:00 2001 From: Thalida Noel Date: Sat, 4 Jul 2026 18:15:03 -0400 Subject: [PATCH 09/12] fix(app): pin Remote when local repos off so URL field survives typing (#77) The single-field NewProjectForm unmounted the source input on the first keystroke of a git URL when allowLocalRepos=false: srcKind classifies a half-typed URL ("h") as Local, flipping kind->Local -> localOff true -> the {!localOff && } branch dropped the field and focus. Pin Remote when local repos are disabled (Local is unreachable there). Adds a char-by-char typing regression test, plus aria-controls on the Advanced toggle. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/src/views/ProjectsView/NewProjectForm.tsx | 12 ++++++--- .../ProjectsView/NewProjectForm.test.tsx | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/src/views/ProjectsView/NewProjectForm.tsx b/app/src/views/ProjectsView/NewProjectForm.tsx index 7559c439..fb6daa7c 100644 --- a/app/src/views/ProjectsView/NewProjectForm.tsx +++ b/app/src/views/ProjectsView/NewProjectForm.tsx @@ -60,8 +60,13 @@ export function NewProjectForm({ function onSourceInput(v: string) { setSource(v); const k = v.trim() ? srcKind(v) : kind; - if (k !== kind) setKind(k); - if (k === SourceKind.Remote) { + // Never reclassify to Local when local repos are disabled: Local isn't a + // reachable destination there, and doing so mid-keystroke (srcKind treats a + // half-typed URL like "h" as Local) would flip localOff true and unmount + // the very field the user is typing into, dropping focus. Pin Remote. + const nextKind = k === SourceKind.Local && !allowLocalRepos ? SourceKind.Remote : k; + if (nextKind !== kind) setKind(nextKind); + if (nextKind === SourceKind.Remote) { // Branch reset on URL change (bug #2): a stale pick from a previous // repo must never ride along to whatever's typed now. BranchSelect is // also remounted below (key={resolvedUrl}), so its internal fetch @@ -141,12 +146,13 @@ export function NewProjectForm({ type="button" class="new-project-advanced-toggle" aria-expanded={advanced} + aria-controls="new-project-advanced" onClick={() => setAdvanced((a) => !a)} > Advanced {advanced && ( -