diff --git a/crates/api/src/domain/repositories.rs b/crates/api/src/domain/repositories.rs index fbd71ab0..a08af914 100644 --- a/crates/api/src/domain/repositories.rs +++ b/crates/api/src/domain/repositories.rs @@ -3053,7 +3053,7 @@ pub async fn repository_file_finder_for_actor_by_owner_name( let resolved_ref = resolve_repository_ref(pool, &repository, query.ref_name).await?; let normalized_query = query.query.unwrap_or("").trim().to_lowercase(); let page = query.page.max(1); - let page_size = query.page_size.clamp(1, 100); + let page_size = query.page_size.clamp(1, 5_000); let files = list_repository_files_for_resolved_ref(pool, repository.id, &resolved_ref).await?; refresh_repository_ref_files_cache(pool, repository.id, &resolved_ref, &files).await?; let mut items = files diff --git a/crates/api/src/routes/repositories.rs b/crates/api/src/routes/repositories.rs index 02533f64..a1ddb6ea 100644 --- a/crates/api/src/routes/repositories.rs +++ b/crates/api/src/routes/repositories.rs @@ -1,6 +1,6 @@ use axum::{ body::Bytes, - extract::{Path, Query, State}, + extract::{OriginalUri, Path, Query, State}, http::{header, HeaderMap, HeaderValue, StatusCode}, response::{IntoResponse, Response}, routing::{delete, get, patch, post, put}, @@ -4433,11 +4433,13 @@ async fn release_reaction( async fn file_finder( State(state): State, headers: HeaderMap, + OriginalUri(original_uri): OriginalUri, Path((owner, repo)): Path<(String, String)>, Query(query): Query, ) -> Result, (StatusCode, Json)> { let actor = AuthenticatedUser::from_headers(&state, &headers).await?; let pool = state.db.as_ref().ok_or_else(database_unavailable)?; + let is_path_cache_contract = original_uri.path().ends_with("/find"); let envelope = repository_file_finder_for_actor_by_owner_name( pool, actor.0.id, @@ -4445,9 +4447,12 @@ async fn file_finder( &repo, RepositoryFileFinderQuery { ref_name: query.ref_name.as_deref(), - query: query.q.as_deref(), + query: if is_path_cache_contract { None } else { query.q.as_deref() }, page: query.page.unwrap_or(1).max(1), - page_size: query.page_size.unwrap_or(20).clamp(1, 100), + page_size: query + .page_size + .unwrap_or(if is_path_cache_contract { 5_000 } else { 20 }) + .clamp(1, 5_000), }, ) .await diff --git a/crates/api/tests/repository_tree_navigation.rs b/crates/api/tests/repository_tree_navigation.rs index e4cc02c0..98a6f8a8 100644 --- a/crates/api/tests/repository_tree_navigation.rs +++ b/crates/api/tests/repository_tree_navigation.rs @@ -336,6 +336,46 @@ async fn repository_tree_contract_resolves_branches_tags_and_recovery_links() { assert_eq!(finder_page_body["total"], 105); assert_eq!(finder_page_body["items"][0]["path"], "docs/example-040.md"); + let (path_cache_status, path_cache_body) = send_json( + app.clone(), + &format!("{base}/find?ref={encoded_feature}&q=guide"), + Some(&owner_cookie), + ) + .await; + assert_eq!(path_cache_status, StatusCode::OK); + assert_eq!(path_cache_body["resolvedRef"]["shortName"], "feature/tree-nav"); + assert_eq!( + path_cache_body["total"], 107, + "the /find path-cache contract returns the full ref path list and ignores q" + ); + assert_eq!(path_cache_body["pageSize"], 5000); + assert!(path_cache_body["items"] + .as_array() + .expect("path cache items should be an array") + .iter() + .any(|entry| entry["path"] == "README.md")); + assert!(path_cache_body["items"] + .as_array() + .expect("path cache items should be an array") + .iter() + .any(|entry| entry["path"] == "docs/example-104.md")); + let cached_paths: serde_json::Value = sqlx::query_scalar( + r#" + SELECT paths + FROM repository_ref_files + WHERE repository_id = $1 AND ref = 'feature/tree-nav' + "#, + ) + .bind(repository.id) + .fetch_one(&pool) + .await + .expect("finder request should refresh repository_ref_files"); + assert!(cached_paths + .as_array() + .expect("cached paths should be a JSON array") + .iter() + .any(|path| path == "docs/guide.md")); + let (bad_path_status, bad_path_body) = send_json( app.clone(), &format!("{base}/contents/%2E%2E/secrets?ref={encoded_feature}"), diff --git a/prd.json b/prd.json index 779e6340..d0f7f953 100644 --- a/prd.json +++ b/prd.json @@ -2171,7 +2171,8 @@ "repocode-002", "search-005" ], - "build_pass": true + "build_pass": true, + "qa_pass": true }, { "id": "security-002", diff --git a/qa-report-summary.json b/qa-report-summary.json index 38a50f82..775a5231 100644 --- a/qa-report-summary.json +++ b/qa-report-summary.json @@ -3404,12 +3404,41 @@ }, { "feature_id": "search-007", - "description": "File finder widget \u2014 keyboard-driven `t` shortcut on a repo to fuzzy-find a file", + "description": "File finder widget \u2014 keyboard-driven `t` shortcut on a repo to fuzzy-find a file by path within the current ref.", "category": "feature", - "qa_pass": false, - "attempts": 0, + "qa_pass": true, + "attempts": 1, "exhausted": false, - "sub_phases": {} + "sub_phases": { + "functional": { + "status": "pass", + "notes": "Dedicated finder page now loads the current ref path cache, focuses the combobox, shows the full cached list for empty input, performs local fuzzy scoring/highlighting, supports ArrowUp/ArrowDown and Enter open, and preserves concrete blob links." + }, + "api_contract": { + "status": "pass", + "endpoints_tested": [ + "GET /api/repos/:owner/:repo/find?ref=feature%2Ftree-nav&q=guide", + "GET /api/repos/:owner/:repo/file-finder?ref=feature%2Ftree-nav&q=guide" + ], + "notes": "DB-backed Rust contract verified /find ignores q and returns all 107 custom ref paths with pageSize 5000, while legacy /file-finder filtering/pagination remains covered; repository_ref_files JSON cache row is refreshed for the resolved ref." + }, + "security": { + "status": "pass", + "checks": [ + "authenticated repository access boundary via existing route extractor", + "no server-side filtering on /find", + "no dead links", + "no stack/secret leakage in focused gates" + ], + "notes": "The finder API continues to require AuthenticatedUser and repository permission checks through repository_file_finder_for_actor_by_owner_name; browser/API gates used real signed sessions and did not expose internal data." + }, + "accessibility": { + "status": "pass", + "violations": [], + "notes": "Combobox/listbox/option roles, aria-selected, aria-activedescendant, focused input, keyboard navigation, no dead controls, and no horizontal overflow were verified in unit and Playwright coverage." + } + }, + "overall_status": "pass" }, { "feature_id": "security-002", diff --git a/qa-report.json b/qa-report.json index 3430d8e2..fd5572d7 100644 --- a/qa-report.json +++ b/qa-report.json @@ -6898,5 +6898,99 @@ "ralph/screenshots/build/settings-005-final-secrets-mobile.jpg", "ralph/screenshots/build/settings-005-final-secrets-forbidden.jpg" ] + }, + { + "feature_id": "search-007", + "attempt": 1, + "status": "pass", + "description": "File finder widget \u2014 keyboard-driven `t` shortcut on a repo to fuzzy-find a file by path within the current ref.", + "category": "feature", + "qa_pass": true, + "attempts": 1, + "exhausted": false, + "timestamp": "2026-05-14T18:58:00Z", + "tested_steps": [ + "make doctor passed with postgres-test up, .env.test present, and CARGO_TARGET_DIR on .scratch/cargo-target.", + "DB-backed Rust/API contract repository_tree_navigation::repository_tree_contract_resolves_branches_tags_and_recovery_links passed with TEST_DATABASE_URL loaded; verified /api/repos/:owner/:repo/find ignores q, returns the full ref path list, and refreshes repository_ref_files.", + "make test passed: Cargo tests passed and Web tests passed (671 passed).", + "Focused Vitest repository-file-finder-page.test.tsx passed; covers cached path rendering, client-side fuzzy scoring, matched-character highlighting, ArrowUp/ArrowDown selection, Enter open, Escape clear, empty state, and concrete links.", + "System-Chrome Playwright repository-file-finder.spec.ts passed against real Next/Rust servers and .env.test; verified repo t shortcut, /find/ page, focused input, empty query full list, no-match state, ArrowDown navigation, Enter open, concrete blob href, no dead controls, and no horizontal overflow.", + "make check passed: Cargo check, Web typecheck, Clippy, and Biome." + ], + "bugs_found": [ + { + "severity": "major", + "description": "The /api/repos/:owner/:repo/find alias reused the legacy /file-finder server-side q filtering/default page-size behavior instead of returning the unfiltered cached ref path list required for local fuzzy scoring.", + "file": "crates/api/src/routes/repositories.rs" + }, + { + "severity": "major", + "description": "The dedicated finder page fetched the legacy file-finder endpoint with pageSize 100 instead of the /find path-cache contract, so larger refs were truncated before client-side fuzzy matching.", + "file": "web/src/app/[owner]/[repo]/find/[ref]/page.tsx" + }, + { + "severity": "minor", + "description": "The finder result list was capped to the first 80 client-side matches, violating the empty-input full-list requirement for moderately sized refs.", + "file": "web/src/components/RepositoryFileFinderPage.tsx" + }, + { + "severity": "minor", + "description": "ArrowDown on an empty result set could move the active index negative and aria-activedescendant could point at a mismatched option after bounds clamping.", + "file": "web/src/components/RepositoryFileFinderPage.tsx" + } + ], + "fix_description": "Split the /find path-cache contract from legacy /file-finder filtering, raised the path-cache page-size path through domain code, taught the Next API helper/page to call /find with pathCache, removed the client 80-result cap, hardened active-option bounds, and added DB-backed API, Vitest, and system-Chrome Playwright coverage.", + "sub_phases": { + "functional": { + "status": "pass", + "notes": "Dedicated finder page now loads the current ref path cache, focuses the combobox, shows the full cached list for empty input, performs local fuzzy scoring/highlighting, supports ArrowUp/ArrowDown and Enter open, and preserves concrete blob links." + }, + "api_contract": { + "status": "pass", + "endpoints_tested": [ + "GET /api/repos/:owner/:repo/find?ref=feature%2Ftree-nav&q=guide", + "GET /api/repos/:owner/:repo/file-finder?ref=feature%2Ftree-nav&q=guide" + ], + "notes": "DB-backed Rust contract verified /find ignores q and returns all 107 custom ref paths with pageSize 5000, while legacy /file-finder filtering/pagination remains covered; repository_ref_files JSON cache row is refreshed for the resolved ref." + }, + "security": { + "status": "pass", + "checks": [ + "authenticated repository access boundary via existing route extractor", + "no server-side filtering on /find", + "no dead links", + "no stack/secret leakage in focused gates" + ], + "notes": "The finder API continues to require AuthenticatedUser and repository permission checks through repository_file_finder_for_actor_by_owner_name; browser/API gates used real signed sessions and did not expose internal data." + }, + "accessibility": { + "status": "pass", + "violations": [], + "notes": "Combobox/listbox/option roles, aria-selected, aria-activedescendant, focused input, keyboard navigation, no dead controls, and no horizontal overflow were verified in unit and Playwright coverage." + } + }, + "verification": { + "commands": [ + "make doctor", + "TEST_DATABASE_URL=postgresql://opengithub:opengithub@localhost:55433/opengithub_test DATABASE_URL=postgresql://opengithub:opengithub@localhost:55433/opengithub_test DB_SSL=false ./hack/cargo_locked.sh test -p opengithub-api --test repository_tree_navigation repository_tree_contract_resolves_branches_tags_and_recovery_links -- --nocapture", + "cd web && npx vitest run tests/repository-file-finder-page.test.tsx", + "make test", + "cd web && TEST_DATABASE_URL=postgresql://opengithub:opengithub@localhost:55433/opengithub_test DATABASE_URL=postgresql://opengithub:opengithub@localhost:55433/opengithub_test DB_SSL=false SESSION_SECRET=playwright-session-secret-with-enough-entropy SESSION_COOKIE_NAME=og_session ./node_modules/.bin/playwright test -c ../.scratch/playwright-search-007.config.cjs repository-file-finder.spec.ts --project=system-chrome", + "make check" + ], + "artifacts": [ + "ralph/screenshots/build/search-007-file-finder-final.jpg" + ] + }, + "overall_status": "pass", + "blockers": [], + "gates": [ + "make doctor: pass", + "focused DB-backed Rust/API contract: pass 1/1, no self-skip", + "focused Vitest file finder component/unit: pass 2/2", + "make test: pass (Cargo tests; Web tests 671 passed)", + "focused env-loaded system-Chrome Playwright: pass 1/1", + "make check: pass" + ] } ] diff --git a/web/src/app/[owner]/[repo]/find/[ref]/page.tsx b/web/src/app/[owner]/[repo]/find/[ref]/page.tsx index 0ebab235..fac9e98d 100644 --- a/web/src/app/[owner]/[repo]/find/[ref]/page.tsx +++ b/web/src/app/[owner]/[repo]/find/[ref]/page.tsx @@ -32,7 +32,8 @@ export default async function RepositoryFindPage({ getRepository(ownerLogin, repositoryName), getRepositoryFileFinder(ownerLogin, repositoryName, refName, "", { page: 1, - pageSize: 100, + pageSize: 5000, + pathCache: true, }), ]) : [null, null]; diff --git a/web/src/components/RepositoryFileFinderPage.tsx b/web/src/components/RepositoryFileFinderPage.tsx index 51440424..4410e633 100644 --- a/web/src/components/RepositoryFileFinderPage.tsx +++ b/web/src/components/RepositoryFileFinderPage.tsx @@ -119,9 +119,12 @@ export function RepositoryFileFinderPage({ return scored; }, [finder.items, query]); - const visibleFiles = scoredFiles.slice(0, 80); - const activeFile = - visibleFiles[Math.min(activeIndex, visibleFiles.length - 1)]; + const visibleFiles = scoredFiles; + const activeOptionIndex = Math.min( + Math.max(activeIndex, 0), + Math.max(visibleFiles.length - 1, 0), + ); + const activeFile = visibleFiles[activeOptionIndex]; useEffect(() => { inputRef.current?.focus(); @@ -163,7 +166,7 @@ export function RepositoryFileFinderPage({ - Math.min(visibleFiles.length - 1, index + 1), + Math.min(Math.max(visibleFiles.length - 1, 0), index + 1), ); } if (event.key === "ArrowUp") { @@ -233,7 +236,7 @@ export function RepositoryFileFinderPage({ role="listbox" > {visibleFiles.map((file, index) => { - const active = index === activeIndex; + const active = index === activeOptionIndex; return ( { const params = new URLSearchParams({ ref: refName }); - if (query.trim()) { + if (!options.pathCache && query.trim()) { params.set("q", query.trim()); } if (options.page) { @@ -19074,7 +19074,7 @@ export async function getRepositoryFileFinderFromCookie( let response: Response; try { response = await fetch( - `${apiBaseUrl()}/api/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/file-finder?${params.toString()}`, + `${apiBaseUrl()}/api/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${options.pathCache ? "find" : "file-finder"}?${params.toString()}`, { headers: cookie ? { cookie } : undefined, cache: "no-store", diff --git a/web/src/lib/server-session.ts b/web/src/lib/server-session.ts index 0b135843..ddda97a1 100644 --- a/web/src/lib/server-session.ts +++ b/web/src/lib/server-session.ts @@ -1577,7 +1577,7 @@ export async function getRepositoryFileFinder( repo: string, refName: string, query = "", - options: { page?: number; pageSize?: number } = {}, + options: { page?: number; pageSize?: number; pathCache?: boolean } = {}, ) { const requestHeaders = await headers(); return getRepositoryFileFinderFromCookie( diff --git a/web/tests/e2e/repository-file-finder.spec.ts b/web/tests/e2e/repository-file-finder.spec.ts new file mode 100644 index 00000000..23a9ee35 --- /dev/null +++ b/web/tests/e2e/repository-file-finder.spec.ts @@ -0,0 +1,89 @@ +import { + expect, + expectNoDeadControls, + expectNoHorizontalOverflow, + screenshotPath, + skipWithoutTestDb, + test, +} from "./_fixtures/auth"; + +test.skip( + skipWithoutTestDb(), + "file finder E2E needs TEST_DATABASE_URL or DATABASE_URL", +); + +test.setTimeout(120_000); + +test("repository t shortcut opens full-ref file finder with local fuzzy navigation", async ({ + page, + seed, + signIn, +}, testInfo) => { + const seeded = await seed({ scenes: ["treeRefs"] }); + await signIn(page, seeded); + const repositoryHref = seeded.hrefs.treeRepository; + const repositoryName = repositoryHref.split("/").at(-1); + if (!repositoryName) { + throw new Error("tree repository href did not include a repo name"); + } + + await page.goto(`${repositoryHref}/tree/feature%2Ftree-nav`); + await expect( + page.getByLabel("Switch branches or tags. Current ref feature/tree-nav"), + ).toBeVisible(); + + await page.keyboard.press("t"); + await expect(page).toHaveURL( + new RegExp(`/${repositoryName}/find/feature%2Ftree-nav$`), + ); + + const input = page.getByRole("combobox", { + name: "Fuzzy-find a file path", + }); + await expect(input).toBeFocused(); + await expect(page.getByText("75 cached paths")).toBeVisible(); + await expect(page.getByRole("option", { name: /README\.md/ })).toBeVisible(); + await expect( + page.getByRole("option", { name: /docs\/example-071\.md/ }), + ).toBeVisible(); + + await input.fill("zzzz-no-match"); + await expect(page.getByRole("status")).toContainText("No matching files"); + await input.press("Escape"); + await expect(input).toHaveValue(""); + await expect(page.getByText("75 cached paths")).toBeVisible(); + + await input.fill("example"); + await expect(page.getByText(/72 matching paths/)).toBeVisible(); + await expect( + page.getByRole("option", { name: /docs\/example-000\.md/ }), + ).toHaveAttribute("aria-selected", "true"); + await input.press("ArrowDown"); + await expect( + page.getByRole("option", { name: /docs\/example-001\.md/ }), + ).toHaveAttribute("aria-selected", "true"); + await input.press("Enter"); + await expect(page).toHaveURL( + new RegExp( + `/${repositoryName}/blob/feature%2Ftree-nav/docs/example-001.md$`, + ), + ); + await expect( + page.getByRole("heading", { name: "docs/example-001.md" }), + ).toBeVisible(); + + await page.goto(`${repositoryHref}/find/feature%2Ftree-nav`); + await input.fill("guide"); + await expect( + page.getByRole("option", { name: /docs\/guide\.md/ }), + ).toHaveAttribute( + "href", + new RegExp(`/${repositoryName}/blob/feature%2Ftree-nav/docs/guide.md$`), + ); + await expectNoDeadControls(page); + await expectNoHorizontalOverflow(page); + await page.screenshot({ + fullPage: true, + path: screenshotPath(testInfo, "search-007-file-finder-final"), + }); +}); diff --git a/web/tests/repository-file-finder-page.test.tsx b/web/tests/repository-file-finder-page.test.tsx index 1e2f3883..ceb37723 100644 --- a/web/tests/repository-file-finder-page.test.tsx +++ b/web/tests/repository-file-finder-page.test.tsx @@ -121,6 +121,11 @@ describe("RepositoryFileFinderPage", () => { within(listbox).queryByRole("option", { name: /README.md/ }), ).not.toBeInTheDocument(); expect(screen.getByText("2 matching paths")).toBeVisible(); + expect( + within(listbox) + .getByRole("option", { name: /src\/app\/page\.tsx/ }) + .querySelectorAll('span[class="text-[var(--accent)]"]'), + ).toHaveLength(3); }); it("supports arrow navigation, enter-to-open, escape clearing, and concrete links", () => { @@ -139,9 +144,21 @@ describe("RepositoryFileFinderPage", () => { name: "Fuzzy-find a file path", }); + fireEvent.keyDown(input, { key: "ArrowDown" }); + expect(screen.getByRole("option", { name: /README\.md/ })).toHaveAttribute( + "aria-selected", + "true", + ); + fireEvent.keyDown(input, { key: "ArrowUp" }); + expect( + screen.getByRole("option", { name: /crates\/api\/src\/routes/ }), + ).toHaveAttribute("aria-selected", "true"); + fireEvent.keyDown(input, { key: "ArrowDown" }); fireEvent.keyDown(input, { key: "ArrowDown" }); fireEvent.keyDown(input, { key: "Enter" }); - expect(assign).toHaveBeenCalledWith("/mona/octo-app/blob/main/README.md"); + expect(assign).toHaveBeenCalledWith( + "/mona/octo-app/blob/main/src/app/page.tsx", + ); const links = screen.getAllByRole("option"); for (const link of links) {