mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* fix(web): replace broken Browse-for-folder with server-side directory picker The "Browse for folder" button used `<input type="file" webkitdirectory>` which only exposes relative paths via `webkitRelativePath`. The code extracted just the folder name (e.g. `myproject`), causing the server to reject it with "path must be an absolute path". No browser API can expose absolute filesystem paths, so the approach was fundamentally broken on all platforms. - Add `GET /api/fs/list` endpoint that lists subdirectories at a given absolute server-side path (rate-limited, validated) - Add `listDirectories()` client function in backend-client.ts - Add `DirectoryPicker` modal component with breadcrumb navigation - Replace broken `webkitdirectory` input in RepoAnalyzer with the new server-side directory picker - Update i18n strings (en + zh-CN) - Add unit tests for the new endpoint (9 tests) Docker users can now browse `/workspace/` and other container paths directly from the UI. Manual path entry continues to work unchanged. Closes #1518 * test(e2e): add Playwright tests for server-side directory picker 13 Playwright e2e tests covering the full DirectoryPicker flow: - Open/display: modal opens, shows root dirs, displays current path - Navigation: click into dirs, breadcrumb back-nav, home button - Selection: populates path input, returns absolute path, close without selecting - Edge cases: empty dir, API error, manual typing still works Also updates existing onboarding.spec.ts to match the renamed "Browse server directories" button, and adds data-testid attributes to DirectoryPicker and RepoAnalyzer for reliable e2e targeting. * fix(a11y): add accessibility and UX polish to DirectoryPicker - Add role="dialog", aria-modal, aria-label to the modal panel - Add aria-label to close button, home button - Add aria-hidden to decorative icons (chevrons, backdrop) - Add role="status" to loading spinner with sr-only label - Add role="alert" to error state - Add aria-current="location" to active breadcrumb segment - Wrap breadcrumb in nav landmark with aria-label - Add Escape key handler to dismiss the modal - Auto-focus the modal panel on open - Add focus-visible ring styles to all interactive elements (matches existing focus-visible:ring-2 ring-accent/40 pattern) - Increase breadcrumb button padding (px-1.5 py-1) for better touch targets - Increase directory entry padding (py-2.5) for touch comfort - Add active:bg-hover/70 pressed state on directory entries - Add active:bg-accent/80 pressed state on select button * chore(autofix): apply prettier + eslint fixes via /autofix command * fix: skip traversal guard for bare root paths in /api/fs/list (#2109) * fix(web): replace server-side directory picker with secure folder upload PR #1850 review found the new GET /api/fs/list directory-browsing endpoint enumerated any absolute server path (CodeQL js/path-injection, plus a DoS and cross-origin enumeration via the CORS/PNA allow-list). Browsers can't hand the server an absolute path, so rather than harden the endpoint, remove it and upload the folder instead — webkitdirectory exposes the file contents. - Add POST /api/analyze/upload: busboy-streamed multipart ingest into an mkdtemp sandbox under UPLOAD_ROOT with resolve-then-contain write sanitization, hard size/count/dir caps, manifest-first ordering, and guaranteed cleanup; promote (atomic same-filesystem rename, no EXDEV) and analyze via the shared job/worker machinery, never returning a server path. - Frontend: <input webkitdirectory> upload flow with client-side filtering (.git/node_modules/build), XHR progress, accessibility, en/zh-CN i18n. - Remove /api/fs/list + handleFsListRequest, DirectoryPicker, listDirectories and their tests. - Harden the adjacent /api/analyze {path} route: localhost-only CORS on write routes + realpath/exists/isDir validation replacing the inert normalize!==resolve guard. - Extend DELETE /api/repo cleanup to upload dirs (by entry.path) and add a startup sweep for orphaned staging dirs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): resolve CodeQL path-injection + CSRF introduced by the upload change The first push surfaced two new CodeQL alerts in the newly-added code (the upload sandbox itself passed — its resolve-then-contain sanitizer is recognized): - HIGH js/path-injection at the analyze route: the KTD11 in-route `fs.realpath(repoLocalPath)` / `fs.stat` was a user-controlled filesystem read with no security gain (the worker already reads the path; cross-origin reach is closed by requireLocalhostOrigin). Drop the in-route fs calls; keep only the absolute-path check + the localhost-origin guard. - MEDIUM js/client-side-request-forgery: the new raw `xhr.open` was a fresh request sink. Route the upload through the shared, origin-validated fetchWithTimeout instead (the centralized sink all other calls use). Trades the upload-progress percentage for an indeterminate "Uploading…" state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): resolve tri-review findings on the upload flow A multi-agent review of the upload implementation surfaced a P0 plus several P2/P3s; all are addressed here. - P0: the upload handler took the single analysis slot (createJob) before validating/promoting, so any failure in that window left a queued job that was never failed — wedging ALL analysis until restart (trivially triggered by a single-segment manifest). Now: validate the folder before taking the slot, release it via failJob on any pre-launch error, and reject single-segment / multi-top manifests during ingest (also fixes a silent file-drop). - CI: rate-limit.test's source-regex broke when Prettier wrapped the /api/analyze registration; made it wrapping-tolerant. - Resource: the startup sweep now also removes stale promoted upload dirs with no .gitnexus index (orphans from analyses that failed before registering). - Frontend: guard against post-unmount SSE opening, reset upload state on cancel/mode-change, guard concurrent uploads, fall back to the folder name, add aria-busy, and fix the {{count}} plural ("1 files"). - Maintainability: extract launchAnalysisWorker into analyze-launch.ts (DI + typed WorkerMessage IPC), move requireLocalhostOrigin to middleware.ts, share REPO_NAME_PATTERN, tighten UploadJobRef, name the collision-retry constant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): reset isMountedRef on mount (StrictMode double-invoke) The mount effect set isMountedRef=false on cleanup but never back to true on re-mount, so under React StrictMode's mount->unmount->mount the ref stayed false for the component's lifetime — trackJob then always early-returned and the upload never advanced past 'starting' (caught by the folder-upload e2e). Set it true at the start of the effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): de-flake upload-ingest cleanup test via injectable staging root ingestUpload gains an IngestOptions.root override (mirroring SweepOptions.root) so the test asserts cleanup against a per-test mkdtemp root instead of counting global ~/.gitnexus/uploads/.staging-* entries, which raced parallel forks. Production default stays UPLOAD_ROOT (promote rename same-filesystem invariant). * fix(web): make stale analyze/upload requests inert after mode switch, cancel, or unmount A folder upload (or URL analyze) still in flight when the user switched modes could resolve later, call trackJob(), and drive the old job's SSE stream under the new mode's form. The only guard was isMountedRef — mode change and cancel never unmount the component. - requestControllerRef: per-request AbortController doubling as the staleness token (captured per closure, checked after the await; the abort error is matched via signal.aborted, never error identity, since it surfaces both as BackendError('Request aborted') and as a raw AbortError from response.json()) - uploadFolder() now takes an optional AbortSignal; fetchWithTimeout already merges caller signals via AbortSignal.any - a stale-but-created job gets a fire-and-forget cancelAnalyze(jobId) (skipped when a live tracking session owns the id) so the single analyze slot is freed - handleModeChange early-returns on same-tab clicks and resets phase to input so an aborted request can't strand the form at 'starting' - fixed the stale breaker comment: resilientFetch records AbortError as breaker-neutral (recordNeutral), not as a retryable-network penalty * refactor(web): consolidate stale-request guard plumbing - single invalidateRequest() helper for the abort+null pattern (4 sites) - drop isMountedRef checks subsumed by the aborted-controller token (unmount aborts the controller, and unlike isMountedRef the token stays correct across a StrictMode unmount/remount) - dedup the component test's render/mock scaffolding - countStaging filters on the exported STAGING_PREFIX, not a magic string * fix(web): scope stale-job cancellation to the upload path Code review caught a regression in the first cut: URL analyzes dedup-alias by repo (createJob returns the existing active job's id), so a stale resolution's fire-and-forget cancel could kill a job another session — or the user's own fresh resubmit — is actively watching; the jobIdRef ownership guard was order-dependent and instance-local. Uploads always own a fresh, never-deduped job, so the cancel is kept (unconditionally) there and dropped on the URL path, where a same-URL resubmit re-attaches via dedup and the server's job timeout / TTL sweep bounds the slot occupancy. Also: remove the isMountedRef machinery outright (zero readers remain — the aborted-controller token subsumes it and stays correct across StrictMode remounts), make the e2e abort check ERR_ABORTED-specific, and let a broken test root fail loudly instead of passing vacuously. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sparsh <73558748+prajapatisparsh@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
112 lines
4.9 KiB
TypeScript
112 lines
4.9 KiB
TypeScript
import { test, expect } from '@playwright/test';
|
|
import fs from 'node:fs';
|
|
import os from 'node:os';
|
|
import path from 'node:path';
|
|
|
|
/**
|
|
* E2E for the browser folder-upload flow (replaces the removed server-side
|
|
* directory picker). Mocks the backend so no live gitnexus server is needed.
|
|
*/
|
|
|
|
const BACKEND_URL = 'http://localhost:4747';
|
|
|
|
let fixtureDir: string;
|
|
|
|
test.beforeAll(() => {
|
|
// A tiny "repo" folder; Playwright sets webkitRelativePath = <folder>/<file>.
|
|
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-upload-e2e-'));
|
|
fixtureDir = path.join(root, 'myrepo');
|
|
fs.mkdirSync(path.join(fixtureDir, 'src'), { recursive: true });
|
|
fs.writeFileSync(path.join(fixtureDir, 'README.md'), '# hi\n');
|
|
fs.writeFileSync(path.join(fixtureDir, 'src', 'index.ts'), 'export const x = 1;\n');
|
|
});
|
|
|
|
test.beforeEach(async ({ page }) => {
|
|
await page.route(`${BACKEND_URL}/api/repos`, (route) => route.fulfill({ json: [] }));
|
|
await page.route(`${BACKEND_URL}/api/info`, (route) =>
|
|
route.fulfill({ json: { version: '1.0.0', launchContext: 'npx', nodeVersion: 'v22.0.0' } }),
|
|
);
|
|
await page.route(`${BACKEND_URL}/api/heartbeat`, (route) =>
|
|
route.fulfill({
|
|
status: 200,
|
|
headers: { 'Content-Type': 'text/event-stream' },
|
|
body: ':ok\n\n',
|
|
}),
|
|
);
|
|
});
|
|
|
|
test('uploading a folder posts a multipart upload and starts analysis', async ({ page }) => {
|
|
let uploadContentType = '';
|
|
await page.route(`${BACKEND_URL}/api/analyze/upload`, async (route) => {
|
|
uploadContentType = route.request().headers()['content-type'] ?? '';
|
|
await route.fulfill({ json: { jobId: 'job-e2e', status: 'analyzing' } });
|
|
});
|
|
// SSE progress → immediately complete.
|
|
await page.route(`${BACKEND_URL}/api/analyze/job-e2e/progress`, (route) =>
|
|
route.fulfill({
|
|
status: 200,
|
|
headers: { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache' },
|
|
body: 'event: complete\ndata: {"repoName":"myrepo"}\n\n',
|
|
}),
|
|
);
|
|
|
|
await page.goto('/');
|
|
await expect(page.getByRole('tab', { name: 'Local Folder' })).toBeVisible({ timeout: 20_000 });
|
|
await page.getByRole('tab', { name: 'Local Folder' }).click();
|
|
|
|
await expect(page.locator('[data-testid="upload-folder"]')).toBeVisible();
|
|
|
|
// Select the fixture folder via the hidden webkitdirectory input.
|
|
await page.locator('[data-testid="folder-upload-input"]').setInputFiles(fixtureDir);
|
|
|
|
// The upload endpoint should be hit with a multipart body, and the UI should
|
|
// leave the input phase (upload button no longer shown).
|
|
await expect.poll(() => uploadContentType).toContain('multipart/form-data');
|
|
await expect(page.locator('[data-testid="upload-folder"]')).toBeHidden({ timeout: 10_000 });
|
|
});
|
|
|
|
test('switching modes mid-upload aborts it and never shows progress', async ({ page }) => {
|
|
// Hold the upload response until the test releases it, so the mode switch
|
|
// happens while the POST is in flight (the review 4470339833 repro).
|
|
let releaseUpload!: () => void;
|
|
const uploadGate = new Promise<void>((res) => (releaseUpload = res));
|
|
let uploadAborted = false;
|
|
let progressOpened = false;
|
|
|
|
// The client-side AbortController kills the POST at mode-switch time; that
|
|
// surfaces as a failed request (net::ERR_ABORTED), not as a response.
|
|
page.on('requestfailed', (req) => {
|
|
if (req.url().includes('/api/analyze/upload') && /ABORTED/.test(req.failure()?.errorText ?? ''))
|
|
uploadAborted = true;
|
|
});
|
|
await page.route(`${BACKEND_URL}/api/analyze/upload`, async (route) => {
|
|
await uploadGate;
|
|
await route.fulfill({ json: { jobId: 'job-stale', status: 'analyzing' } }).catch(() => {}); // the request may already be gone — that's the point
|
|
});
|
|
await page.route(`${BACKEND_URL}/api/analyze/job-stale/progress`, (route) => {
|
|
progressOpened = true;
|
|
return route.fulfill({
|
|
status: 200,
|
|
headers: { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache' },
|
|
body: 'event: complete\ndata: {"repoName":"myrepo"}\n\n',
|
|
});
|
|
});
|
|
|
|
await page.goto('/');
|
|
await expect(page.getByRole('tab', { name: 'Local Folder' })).toBeVisible({ timeout: 20_000 });
|
|
await page.getByRole('tab', { name: 'Local Folder' }).click();
|
|
await page.locator('[data-testid="folder-upload-input"]').setInputFiles(fixtureDir);
|
|
await expect(page.locator('[data-testid="upload-progress"]')).toBeVisible();
|
|
|
|
// Switch back to GitHub while the upload POST is still pending, then let
|
|
// the (now-stale) route handler finish.
|
|
await page.getByRole('tab', { name: 'GitHub URL' }).click();
|
|
await expect.poll(() => uploadAborted, { timeout: 10_000 }).toBe(true);
|
|
releaseUpload();
|
|
|
|
// The GitHub form stays clean (no error, immediately usable), and no SSE
|
|
// progress stream is ever opened by the stale upload.
|
|
await expect(page.getByPlaceholder('https://github.com/owner/repo')).toBeEditable();
|
|
await expect(page.locator('[data-testid="upload-progress"]')).toBeHidden();
|
|
expect(progressOpened).toBe(false);
|
|
});
|