From ee2feb7a5b1e7c4a66acb0dd70dee6aad88c56e5 Mon Sep 17 00:00:00 2001 From: Parafee41 Date: Sun, 20 Sep 2026 15:28:28 +0800 Subject: [PATCH 1/5] fix(cli): announce explicit registry alias changes (#3334) * fix(cli): announce explicit registry alias changes * fix: handle async rename observer failures * Address PR review feedback (#3334) - Invoke onRename after withRegistryLock releases so observer I/O cannot stall the registry - Spy the rejecting observer and prove lock release via re-entrant registerRepo Co-authored-by: Cursor * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- gitnexus/src/core/run-analyze.ts | 6 ++++ gitnexus/src/storage/repo-manager.ts | 32 +++++++++++++++++++-- gitnexus/test/unit/repo-manager.test.ts | 37 +++++++++++++++++++++++++ gitnexus/test/unit/run-analyze.test.ts | 4 ++- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index c2d7b485d..f0c20e737 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -1597,6 +1597,8 @@ async function runFullAnalysisInner( if (options.registryName) { await registerRepo(repoPath, existingMeta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, branch: placement.branch, storagePath, @@ -2166,6 +2168,8 @@ async function runFullAnalysisInner( if (options.registryName) { await registerRepo(repoPath, existingMeta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, branch: placement.branch, storagePath, @@ -4644,6 +4648,8 @@ async function runFullAnalysisInner( // will look up (#979). const projectName = await registerRepo(repoPath, meta, { name: options.registryName, + onRename: (previousName, nextName) => + log(`Registry name changed: "${previousName}" -> "${nextName}".`), allowDuplicateName: options.allowDuplicateName, // Non-primary branch runs upsert into the entry's branches[]; the // primary/flat run (placement.branch === undefined) refreshes the diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index a4db22b46..aee05358f 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -838,6 +838,13 @@ export interface RegisterRepoOptions { * re-analyses of the same path without `--name` preserve the alias. */ name?: string; + /** + * Best-effort notification after an explicit alias change has been committed + * to the registry. Invoked after the registry lock is released. Callback + * failures are ignored: reporting must not turn a successful registry write + * into an apparent transaction failure. + */ + onRename?: (previousName: string, nextName: string) => void | Promise; /** * Allow two DIFFERENT repo paths to register under the same alias * (#829). Mapped from the `--allow-duplicate-name` CLI flag. @@ -926,6 +933,11 @@ const hasCustomAlias = (entry: RegistryEntry, inferredName: string | null): bool return true; }; +type RegisterRepoUnlockedResult = { + name: string; + rename?: { previousName: string; nextName: string }; +}; + /** * Register (add or update) a repo in the global registry. * Called after `gitnexus analyze` completes. @@ -954,7 +966,7 @@ const registerRepoUnlocked = async ( repoPath: string, meta: RepoMeta, opts?: RegisterRepoOptions, -): Promise => { +): Promise => { // Preserve the caller's chosen path form in the registry — don't // canonicalise at write time. This matters for two reasons: // 1. `list` and error messages show the path the user actually @@ -1151,14 +1163,28 @@ const registerRepoUnlocked = async ( } await writeRegistry(fresh); - return name; + const rename = + opts?.name !== undefined && freshExisting && freshExisting.name !== name + ? { previousName: freshExisting.name, nextName: name } + : undefined; + return { name, ...(rename ? { rename } : {}) }; }; export const registerRepo = async ( repoPath: string, meta: RepoMeta, opts?: RegisterRepoOptions, -): Promise => withRegistryLock(() => registerRepoUnlocked(repoPath, meta, opts)); +): Promise => { + const { name, rename } = await withRegistryLock(() => registerRepoUnlocked(repoPath, meta, opts)); + if (rename) { + try { + await opts?.onRename?.(rename.previousName, rename.nextName); + } catch { + // The rename is already durable; observer failures cannot roll it back. + } + } + return name; +}; /** * Remove a repo from the global registry. diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index cdf1d6fa2..fddafbe39 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -963,6 +963,43 @@ describe('registerRepo name override + collision guard (#829)', () => { expect(entries[0].name).toBe('new-alias'); }); + it('keeps an alias rename committed when an async observer rejects', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + + const onRename = vi.fn(async () => { + throw new Error('observer failed'); + }); + + await expect( + registerRepo(tmpRepoA.dbPath, meta, { + name: 'new-alias', + onRename, + }), + ).resolves.toBe('new-alias'); + + expect(onRename).toHaveBeenCalledTimes(1); + expect(onRename).toHaveBeenCalledWith('old-alias', 'new-alias'); + expect(await listRegisteredRepos()).toMatchObject([{ name: 'new-alias' }]); + }); + + it('releases the registry lock before invoking onRename', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + + await registerRepo(tmpRepoA.dbPath, meta, { + name: 'new-alias', + onRename: async () => { + await registerRepo(tmpRepoB.dbPath, meta, { name: 'observer-reentry' }); + }, + }); + + expect(await listRegisteredRepos()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: 'new-alias' }), + expect.objectContaining({ name: 'observer-reentry' }), + ]), + ); + }); + it('registerRepo throws RegistryNameCollisionError when another path uses the name', async () => { await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index fe1dc8660..7b6ac9552 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -188,10 +188,11 @@ describe('run-analyze module', () => { await registerRepo(tmpRepo.dbPath, meta, { name: 'old' }); const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const logs: string[] = []; const result = await runFullAnalysis( tmpRepo.dbPath, { registryName: 'new' }, - { onProgress: () => {} }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, ); expect(result.alreadyUpToDate).toBe(true); @@ -199,6 +200,7 @@ describe('run-analyze module', () => { const entries = await readRegistry(); expect(entries).toHaveLength(1); expect(entries[0].name).toBe('new'); + expect(logs).toContain('Registry name changed: "old" -> "new".'); const agents = await fs.readFile(path.join(tmpRepo.dbPath, 'AGENTS.md'), 'utf-8'); expect(agents).toContain('**new**'); } finally { From 5d32e345bbacd0a20fa60696ca8fd62eef44697f Mon Sep 17 00:00:00 2001 From: Ahmad Othman Ammar Adi <78882424+OthmanAdi@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:12:54 +0200 Subject: [PATCH 2/5] feat(web): drop a folder onto the analyzer to upload it (#3315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): drop a folder onto the analyzer to upload it The Local Folder panel of RepoAnalyzer is now a drop target. A dropped folder is walked with the File and Directory Entries API (DataTransferItem.webkitGetAsEntry), directories on the shared exclusion list are pruned before they are read, and the resulting File objects are handed to the existing filterRepoFiles -> uploadFolder -> trackJob path with webkitRelativePath set to /, so the server receives the same manifest shape the webkitdirectory picker produces. Compared with the picker, the walk never enumerates node_modules or .git, stops at the server's 20000 file cap and 64 path segments, skips unreadable entries instead of failing, and reports progress while it runs. Loose files and several folders at once are refused with a message (the server accepts one top-level folder). The walk runs under the request controller, so a mode switch or unmount aborts it; Analyze and the picker are blocked while it runs. The panel-wide target also stops the browser from navigating to a file dropped a few pixels off the button. New strings in en and zh-CN; browsers without webkitGetAsEntry keep the picker button and get an explanation. * Address PR review feedback (#3315) Clear readingCount only when this drop still owns the request controller, and skip oversized files before they count toward the 20k drop cap. Co-authored-by: Cursor * Address PR review feedback (#3315) Count oversized files the drop walk skips in the summary droppedCount, and correct the 250-file batch-read comment. Co-authored-by: Cursor * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergő Magyar Co-authored-by: Gergo Magyar Co-authored-by: Cursor Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- gitnexus-web/src/components/RepoAnalyzer.tsx | 209 +++++++++-- gitnexus-web/src/lib/folder-drop.test.ts | 283 +++++++++++++++ gitnexus-web/src/lib/folder-drop.ts | 214 +++++++++++ gitnexus-web/src/locales/en/onboarding.json | 9 +- .../src/locales/zh-CN/onboarding.json | 9 +- .../unit/repo-analyzer-folder-drop.test.tsx | 334 ++++++++++++++++++ 6 files changed, 1035 insertions(+), 23 deletions(-) create mode 100644 gitnexus-web/src/lib/folder-drop.test.ts create mode 100644 gitnexus-web/src/lib/folder-drop.ts create mode 100644 gitnexus-web/test/unit/repo-analyzer-folder-drop.test.tsx diff --git a/gitnexus-web/src/components/RepoAnalyzer.tsx b/gitnexus-web/src/components/RepoAnalyzer.tsx index 9d44cdb91..560dbcf99 100644 --- a/gitnexus-web/src/components/RepoAnalyzer.tsx +++ b/gitnexus-web/src/components/RepoAnalyzer.tsx @@ -3,10 +3,11 @@ * * Two input modes: * - "github" → GitHub URL (https://github.com/owner/repo) - * - "local" → Select a local folder via the browser's native directory picker + * - "local" → Select a local folder via the browser's native directory picker, + * or drop one onto the upload control */ -import { useState, useRef, useEffect, useId } from 'react'; +import { useState, useRef, useEffect, useId, type DragEvent as ReactDragEvent } from 'react'; import { Github, Gitlab, @@ -27,7 +28,14 @@ import { type JobProgress, } from '../services/backend-client'; import { AnalyzeProgress } from './AnalyzeProgress'; -import { filterRepoFiles } from '@/lib/upload-filter'; +import { filterRepoFiles, type FilterResult } from '@/lib/upload-filter'; +import { + collectDropEntries, + isFolderDropSupported, + readDroppedFolder, + DropRejection, + type DropRejectionReason, +} from '@/lib/folder-drop'; import { useTranslation } from 'react-i18next'; import { formatBackendError } from '../i18n/error-messages'; @@ -55,6 +63,23 @@ function isValidAzureUrl(value: string): boolean { return AZURE_RE.test(value.trim()); } +/** i18n key (under onboarding:repoAnalyzer.upload) for a refused folder drop. */ +function dropRejectionKey(reason: DropRejectionReason): string { + switch (reason) { + case 'unsupported': + return 'dropUnsupported'; + case 'tooManyFiles': + return 'dropTooManyFiles'; + case 'notSingleFolder': + return 'dropSingleFolder'; + } +} + +/** True for a drag that carries files or folders from the OS, not text or links. */ +function isFileDrag(e: ReactDragEvent): boolean { + return Array.from(e.dataTransfer.types).includes('Files'); +} + // ── Mode tabs ──────────────────────────────────────────────────────────────── function ModeTabs({ mode, onChange }: { mode: InputMode; onChange: (m: InputMode) => void }) { @@ -198,9 +223,16 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp const inputId = useId(); const [mode, setMode] = useState('github'); const [uploading, setUploading] = useState(false); - const [uploadSummary, setUploadSummary] = useState<{ count: number; dropped: number } | null>( - null, - ); + // Files found so far while walking a dropped folder; null when not reading. + const [readingCount, setReadingCount] = useState(null); + const [dragActive, setDragActive] = useState(false); + // `skippedDirs` is set only for a dropped folder: directories the walk left + // out without enumerating them (a directory count, unlike `dropped`). + const [uploadSummary, setUploadSummary] = useState<{ + count: number; + dropped: number; + skippedDirs?: number; + } | null>(null); const [githubUrl, setGithubUrl] = useState(''); const [githubToken, setGithubToken] = useState(''); const [gitlabUrl, setGitlabUrl] = useState(''); @@ -224,6 +256,9 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp const requestControllerRef = useRef(null); const completeTimerRef = useRef | null>(null); const folderInputRef = useRef(null); + // dragenter/dragleave fire for every child boundary crossed; count them so + // the highlight does not flicker while the cursor moves over the button. + const dragDepthRef = useRef(0); useEffect(() => { return () => { @@ -277,6 +312,10 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp setValidationError(null); setUploadSummary(null); setUploading(false); + // The aborted controller also stops a folder walk that is still running. + setReadingCount(null); + setDragActive(false); + dragDepthRef.current = 0; // An aborted request no longer resolves to move `phase` off 'starting'; // reset so the new mode's form is immediately usable (also clears a stale // 'error' phase). Only reachable while showInput is true. @@ -287,14 +326,17 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp // exposes an absolute path, so the old typed-path/browse approach couldn't // work — see handleFolderUpload). A typed server path is also still accepted. + // A folder walk in progress (readingCount) blocks Analyze as well: starting a + // request would abort the walk through renewRequestController. const canSubmit = - mode === 'github' + readingCount === null && + (mode === 'github' ? isValidGithubUrl(githubUrl) && (phase === 'input' || phase === 'error') : mode === 'gitlab' ? isValidGitlabUrl(gitlabUrl) && (phase === 'input' || phase === 'error') : mode === 'azure' ? isValidAzureUrl(azureUrl) && (phase === 'input' || phase === 'error') - : localPath.trim().length > 1 && (phase === 'input' || phase === 'error'); + : localPath.trim().length > 1 && (phase === 'input' || phase === 'error')); const handleAnalyze = async () => { if (mode === 'github' && !isValidGithubUrl(githubUrl)) { @@ -398,20 +440,33 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp // Upload a browser-selected folder (webkitdirectory) and start analysis. The // upload endpoint returns a jobId, which then joins the normal SSE flow. const handleFolderUpload = async (fileList: FileList) => { - if (uploading || isLoading) return; // guard against a concurrent upload - const { files, manifest, droppedCount } = filterRepoFiles(fileList); + // guard against a concurrent upload or a folder walk still running + if (uploading || isLoading || readingCount !== null) return; + await startFolderUpload(filterRepoFiles(fileList), renewRequestController()); + }; + + // Shared tail of the picker and drop paths: `filtered` is the client-side + // filter result, `controller` owns the request, `skippedDirs` is set for a + // drop and counts the directories the walk left out before reading them + // (the picker enumerates everything and lets filterRepoFiles drop the files + // instead, so its count arrives inside `filtered.droppedCount`). + const startFolderUpload = async ( + filtered: FilterResult, + controller: AbortController, + skippedDirs?: number, + ) => { + const { files, manifest, droppedCount } = filtered; if (files.length === 0) { setValidationError(t('onboarding:repoAnalyzer.upload.empty')); return; } setValidationError(null); - setUploadSummary({ count: files.length, dropped: droppedCount }); + setUploadSummary({ count: files.length, dropped: droppedCount, skippedDirs }); setUploading(true); setPhase('starting'); // The selected folder's name (manifest entries are `/`) is a // sensible fallback if the server's complete event omits repoName. const folderName = manifest[0]?.split('/')[0] ?? null; - const controller = renewRequestController(); try { const { jobId } = await uploadFolder(files, manifest, controller.signal); if (controller.signal.aborted) { @@ -436,6 +491,76 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp } }; + // Drag and drop a folder onto the upload control. The entries have to be + // taken from dataTransfer synchronously inside the drop handler (browsers + // empty `items` once the handler yields). Only a single folder is accepted, + // mirroring the server's one-top-level-directory rule, and the walk runs + // under the same request controller as the upload so a mode switch or an + // unmount aborts both. + const isDropBlocked = () => uploading || phase === 'starting' || readingCount !== null; + + const handleDragEnter = (e: ReactDragEvent) => { + if (!isFileDrag(e)) return; + e.preventDefault(); + if (isDropBlocked()) return; + dragDepthRef.current += 1; + setDragActive(true); + }; + + const handleDragOver = (e: ReactDragEvent) => { + if (!isFileDrag(e)) return; + // Without preventDefault the drop never fires and the browser navigates + // to the dropped file instead, so it is called even while blocked. + e.preventDefault(); + e.dataTransfer.dropEffect = isDropBlocked() ? 'none' : 'copy'; + }; + + const handleDragLeave = () => { + // No type check here: some engines hand dragleave an empty `types` list, + // and a stray decrement is harmless because the depth is clamped at 0. + dragDepthRef.current = Math.max(0, dragDepthRef.current - 1); + if (dragDepthRef.current === 0) setDragActive(false); + }; + + const handleDrop = async (e: ReactDragEvent) => { + if (!isFileDrag(e)) return; + e.preventDefault(); + dragDepthRef.current = 0; + setDragActive(false); + if (isDropBlocked()) return; + const controller = renewRequestController(); + setValidationError(null); + setUploadSummary(null); + setReadingCount(0); + try { + const entries = collectDropEntries(e.dataTransfer); // sync, before any await + const { files, skipped, oversized } = await readDroppedFolder(entries, { + signal: controller.signal, + onProgress: setReadingCount, + }); + // Only the walk that still owns the request controller may clear the + // reading mutex. An aborted drop that settles after a later drop started + // must not steal the live walk's lock (readingCount === null is what + // unblocks Analyze and a second drop). + if (requestControllerRef.current === controller) setReadingCount(null); + if (controller.signal.aborted) return; + const filtered = filterRepoFiles(files); + await startFolderUpload( + { ...filtered, droppedCount: filtered.droppedCount + oversized }, + controller, + skipped, + ); + } catch (err) { + if (requestControllerRef.current === controller) setReadingCount(null); + if (controller.signal.aborted) return; + setValidationError( + err instanceof DropRejection + ? t(`onboarding:repoAnalyzer.upload.${dropRejectionKey(err.reason)}`, { max: err.max }) + : formatBackendError(err, t), + ); + } + }; + const handleCancel = async () => { sseControllerRef.current?.abort(); sseControllerRef.current = null; @@ -453,6 +578,7 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp setPhase('input'); setProgress({ phase: 'queued', percent: 0, message: t('common:analyzePhases.queued') }); setUploading(false); + setReadingCount(null); setUploadSummary(null); }; @@ -658,9 +784,19 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp )} - {/* Local folder input */} + {/* Local folder input. The whole panel is the drop target for a folder: + a drop that lands on the path input or the label is caught too, and + the browser's default (navigating to the dropped file) never fires. */} {showInput && mode === 'local' && ( -
+