GitNexus/gitnexus-web/e2e/folder-upload.spec.ts
Gergő Magyar 6424d8b09c
fix(web): replace broken Browse-for-folder with upload directory picker (#1850)
* 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>
2026-06-10 20:50:59 +01:00

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);
});