From e4336c8a296f764a18603db9679b8e17f5fb00b3 Mon Sep 17 00:00:00 2001 From: jaeyunha Date: Sun, 24 May 2026 03:47:20 -0700 Subject: [PATCH] fix: verify search file finder path list --- build-progress.txt | 6 ++ crates/api/src/domain/repositories.rs | 2 +- crates/api/src/routes/repositories.rs | 15 ++- .../api/tests/repository_tree_navigation.rs | 51 ++++++++++ prd.json | 3 +- qa-hints.json | 11 ++- qa-report-summary.json | 25 ++++- qa-report.json | 44 +++++++++ .../app/[owner]/[repo]/find/[ref]/page.tsx | 3 +- web/src/lib/api.ts | 4 +- web/src/lib/server-session.ts | 2 +- web/tests/e2e/repository-file-finder.spec.ts | 93 +++++++++++++++++++ 12 files changed, 242 insertions(+), 17 deletions(-) create mode 100644 web/tests/e2e/repository-file-finder.spec.ts diff --git a/build-progress.txt b/build-progress.txt index 538f30f6..8153c6d4 100644 --- a/build-progress.txt +++ b/build-progress.txt @@ -10786,3 +10786,9 @@ Blockers: full make test Cargo passed but broad web Vitest still has unrelated u Status: QA PASS. Fresh controller recovery accepted for Network/Forks and ready for main merge. Evidence: make doctor healthy; make check passed; DB-backed package_detail_contract 4/4, projects_list_contract 1/1, and repository_network_contract 1/1 passed against localhost:55433; focused Network/Forks/API docs Vitest passed 9/9; focused system-Chrome repository-network Playwright passed setup + flow 2/2 in 21.0s. Fixes accepted: package settings camelCase serde IDs, project copy FOR UPDATE OF projects + workflow_key clone + org membership/base-role guard, corrected projects default-open expectation, repository_network_contract rate-limit identity isolation, and repository-network E2E now uses shared auth plus container-backed psql fallback instead of host psql. + +2026-05-24 - search-007 QA final lane +- Fixed /api/repos/:owner/:repo/find to honor the PRD path-list contract: q is ignored for /find, default pageSize is 10000, and repository_ref_files is refreshed while legacy /file-finder filtering remains intact. +- Updated the dedicated ///find/ page fetch to use the /find path-list contract for client-side fuzzy scoring. +- Added focused DB-backed API/security coverage and focused Playwright UI/a11y/keyboard coverage; make check, focused Rust contract, and focused Playwright with /snap/bin/chromium passed. +- QA remains blocked (qa_pass=false): full make test fails in unrelated web unit-test timeouts, and default make test-e2e cannot provision bundled Chromium on ubuntu26.04-x64 without executable override. diff --git a/crates/api/src/domain/repositories.rs b/crates/api/src/domain/repositories.rs index fbd71ab0..10eb972b 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, if query.query.is_none() { 10_000 } else { 100 }); 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..46548685 100644 --- a/crates/api/src/routes/repositories.rs +++ b/crates/api/src/routes/repositories.rs @@ -1,7 +1,7 @@ use axum::{ body::Bytes, - extract::{Path, Query, State}, - http::{header, HeaderMap, HeaderValue, StatusCode}, + extract::{MatchedPath, Path, Query, State}, + http::{header, HeaderMap, HeaderValue, StatusCode, Uri}, response::{IntoResponse, Response}, routing::{delete, get, patch, post, put}, Json, Router, @@ -4435,9 +4435,12 @@ async fn file_finder( headers: HeaderMap, Path((owner, repo)): Path<(String, String)>, Query(query): Query, + uri: Uri, + matched_path: MatchedPath, ) -> 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_list_contract = uri.path().ends_with("/find") || matched_path.as_str().contains("/:owner/:repo/find"); let envelope = repository_file_finder_for_actor_by_owner_name( pool, actor.0.id, @@ -4445,9 +4448,13 @@ async fn file_finder( &repo, RepositoryFileFinderQuery { ref_name: query.ref_name.as_deref(), - query: query.q.as_deref(), + // The /find contract returns the cached full path list; filtering is intentionally client-side. + query: if is_path_list_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_list_contract { 10_000 } else { 20 }) + .clamp(1, if is_path_list_contract { 10_000 } else { 100 }), }, ) .await diff --git a/crates/api/tests/repository_tree_navigation.rs b/crates/api/tests/repository_tree_navigation.rs index e4cc02c0..6ed2d09a 100644 --- a/crates/api/tests/repository_tree_navigation.rs +++ b/crates/api/tests/repository_tree_navigation.rs @@ -336,6 +336,57 @@ 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 (find_status, find_body) = send_json( + app.clone(), + &format!("{base}/find?ref={encoded_feature}&q=guide"), + Some(&owner_cookie), + ) + .await; + assert_eq!(find_status, StatusCode::OK); + assert_eq!(find_body["resolvedRef"]["shortName"], "feature/tree-nav"); + assert_eq!(find_body["page"], 1); + assert_eq!(find_body["pageSize"], 10000); + assert_eq!(find_body["total"], 107); + assert!(find_body["items"] + .as_array() + .expect("find items should be an array") + .iter() + .any(|item| item["path"] == "docs/guide.md")); + assert_eq!( + find_body["items"] + .as_array() + .expect("find items should be an array") + .len(), + 107, + "/find should return the full cached path list for client-side fuzzy scoring" + ); + + let cached_paths: serde_json::Value = sqlx::query_scalar( + "SELECT paths FROM repository_ref_files WHERE repository_id = $1 AND ref = $2", + ) + .bind(repository.id) + .bind("feature/tree-nav") + .fetch_one(&pool) + .await + .expect("finder should refresh repository_ref_files"); + assert!(cached_paths + .as_array() + .expect("cached paths should be an array") + .iter() + .any(|path| path == "docs/guide.md")); + + let (find_unauth_status, find_unauth_body) = send_json( + app.clone(), + &format!("{base}/find?ref={encoded_feature}"), + None, + ) + .await; + assert_eq!(find_unauth_status, StatusCode::UNAUTHORIZED); + assert_eq!(find_unauth_body["error"]["code"], "not_authenticated"); + assert!(!find_unauth_body.to_string().contains("docs/guide.md")); + assert!(!find_unauth_body.to_string().to_lowercase().contains("stack")); + 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..d2bc5edb 100644 --- a/prd.json +++ b/prd.json @@ -2171,7 +2171,8 @@ "repocode-002", "search-005" ], - "build_pass": true + "build_pass": true, + "qa_pass": false }, { "id": "security-002", diff --git a/qa-hints.json b/qa-hints.json index 91907b17..f6d26ae2 100644 --- a/qa-hints.json +++ b/qa-hints.json @@ -7892,13 +7892,14 @@ "Extended /docs/api with the file finder API contract and cache semantics.", "Added focused Vitest coverage for fuzzy filtering, highlighted/concrete result links, keyboard open, empty state, and Escape clearing.", "Browser smoke passed on /mona/octo-app/find/main with a local API-compatible stub: filtered to src/app/page.tsx, Enter navigated to the blob route, verified empty state, checked zero href=\"#\", checked no horizontal overflow, and saved ralph/screenshots/build/search-007-file-finder.jpg.", - "Verification passed: cargo check -p opengithub-api --tests, focused Vitest, web TypeScript, focused Biome, full make check, full make test with Cargo tests plus 631 web tests, and mandatory Editorial banned-value scan." + "Verification passed: cargo check -p opengithub-api --tests, focused Vitest, web TypeScript, focused Biome, full make check, full make test with Cargo tests plus 631 web tests, and mandatory Editorial banned-value scan.", + "QA final lane added DB-backed /find contract assertions for full path-list/no server q filtering, repository_ref_files cache refresh, unauthenticated 401 shape, and no path/stack leak.", + "QA final lane added Playwright repository-file-finder.spec.ts covering t shortcut, labeled focused combobox, empty cached list, local fuzzy filtering, keyboard navigation, Enter open, no-match state, and dead-link check." ], "needs_deeper_qa": [ - "Run full signed-session Playwright after the local TEST_DATABASE_URL path is healthy; bounded timeout 120 make test-e2e terminated with no Playwright detail.", - "Run DB-backed API assertions after migrations against a credentialed Postgres to verify repository_ref_files rows are created/updated for branch and tag refs.", - "Probe very large repositories beyond the current 100-item page fetch; the UI scores the fetched cached list locally, so a later backend/page-size contract may be needed for huge path lists.", - "Verify keyboard-only behavior on mobile/desktop with long paths, duplicate filenames in different folders, refs containing slashes, binary files, and private repository permission boundaries." + "Unblock full make test web unit-suite timeouts in unrelated repository-code-overview and repository-dependency-graph-page tests before setting qa_pass true.", + "Unblock default make test-e2e browser provisioning on ubuntu26.04-x64 or configure the committed test runner to use /snap/bin/chromium; focused E2E passed with PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH.", + "After full gates are green, rerun full make test-e2e in addition to the focused repository-file-finder spec." ] }, { diff --git a/qa-report-summary.json b/qa-report-summary.json index 38a50f82..28dc1b1d 100644 --- a/qa-report-summary.json +++ b/qa-report-summary.json @@ -3407,9 +3407,30 @@ "description": "File finder widget \u2014 keyboard-driven `t` shortcut on a repo to fuzzy-find a file", "category": "feature", "qa_pass": false, - "attempts": 0, + "attempts": 1, "exhausted": false, - "sub_phases": {} + "sub_phases": { + "functional": { + "status": "pass", + "notes": "Focused Playwright passed via system Chromium: repository-file-finder.spec.ts covered t shortcut, /find/main page, focused labeled combobox, empty cached list display, local fuzzy filtering, ArrowUp/ArrowDown, Enter-to-open README.md, no-match empty state, and no dead href controls." + }, + "api_contract": { + "status": "pass", + "notes": "DB-backed Rust contract test repository_tree_navigation passed for GET /api/repos/:owner/:repo/find?ref=... returning the full 107-path list despite q=guide, pageSize 10000, repository_ref_files refresh, legacy /file-finder server filtering preserved, and unauthenticated 401 JSON shape." + }, + "security": { + "status": "pass", + "notes": "Focused API test verified private path-list endpoint requires auth and unauthenticated response contains no repository path data or stack trace." + }, + "accessibility": { + "status": "pass", + "notes": "Focused browser test verified keyboard-only t shortcut, focus lands on named combobox, listbox/option semantics are usable, Arrow navigation and Enter activation work." + }, + "regression": { + "status": "blocked", + "notes": "Required full make test is red in unrelated pre-existing web unit tests: repository-code-overview.test.tsx large-directory paging and repository-dependency-graph-page.test.tsx dependency page timed out at 5000ms. make test-e2e without executable override is blocked because Playwright does not support installing Chromium on ubuntu26.04-x64; focused E2E passed with PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/snap/bin/chromium." + } + } }, { "feature_id": "security-002", diff --git a/qa-report.json b/qa-report.json index 3430d8e2..496224b8 100644 --- a/qa-report.json +++ b/qa-report.json @@ -6898,5 +6898,49 @@ "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": "blocked", + "sub_phases": { + "functional": { + "status": "pass", + "notes": "Focused Playwright passed via system Chromium: repository-file-finder.spec.ts covered t shortcut, /find/main page, focused labeled combobox, empty cached list display, local fuzzy filtering, ArrowUp/ArrowDown, Enter-to-open README.md, no-match empty state, and no dead href controls." + }, + "api_contract": { + "status": "pass", + "notes": "DB-backed Rust contract test repository_tree_navigation passed for GET /api/repos/:owner/:repo/find?ref=... returning the full 107-path list despite q=guide, pageSize 10000, repository_ref_files refresh, legacy /file-finder server filtering preserved, and unauthenticated 401 JSON shape." + }, + "security": { + "status": "pass", + "notes": "Focused API test verified private path-list endpoint requires auth and unauthenticated response contains no repository path data or stack trace." + }, + "accessibility": { + "status": "pass", + "notes": "Focused browser test verified keyboard-only t shortcut, focus lands on named combobox, listbox/option semantics are usable, Arrow navigation and Enter activation work." + }, + "regression": { + "status": "blocked", + "notes": "Required full make test is red in unrelated pre-existing web unit tests: repository-code-overview.test.tsx large-directory paging and repository-dependency-graph-page.test.tsx dependency page timed out at 5000ms. make test-e2e without executable override is blocked because Playwright does not support installing Chromium on ubuntu26.04-x64; focused E2E passed with PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/snap/bin/chromium." + } + }, + "tested_steps": [ + "make doctor: local verification stack healthy.", + "Inspected existing /find page, /file-finder route, Rust /find alias, migration, and prior artifacts.", + "Fixed Rust /api/repos/:owner/:repo/find contract to ignore q and default/allow pageSize 10000 for full path-list delivery while preserving legacy /file-finder filtering.", + "Fixed Next ///find/ loader to call the /find path-list contract with pageSize 10000 for client-side fuzzy scoring.", + "Added DB-backed contract assertions to repository_tree_navigation.rs for full path list, cache refresh, auth 401 shape, and no path/stack leak.", + "Added focused Playwright coverage in repository-file-finder.spec.ts for t shortcut, modal page, empty list, fuzzy filtering, keyboard nav, Enter open, no-match state, a11y basics, and dead-link check.", + "Passed: make check.", + "Passed: TEST_DATABASE_URL from .env.test ./hack/cargo_locked.sh test -p opengithub-api --test repository_tree_navigation -- --nocapture.", + "Passed: PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/snap/bin/chromium npx playwright test tests/e2e/repository-file-finder.spec.ts --project=chromium.", + "Blocked: make test full suite red in two unrelated web unit-test timeouts; make test-e2e default red before tests because Playwright browser binary missing and npx playwright install chromium reports unsupported ubuntu26.04-x64." + ], + "bugs_found": [ + "/find alias was still effectively constrained to 100 items by domain clamp, so the dedicated page could not honestly claim full path-list client-side fuzzy scoring for repositories over 100 files." + ], + "fix_description": "Route /find now uses path-list semantics (no q filtering, default pageSize 10000), domain allows 10000 when query is None, and the dedicated page fetches /find with pathList=true. Legacy /file-finder filtering remains for toolbar clients.", + "blocker": "Required full regression gates are not green: make test fails in unrelated web unit-test timeouts; default make test-e2e is blocked by missing/unsupported Playwright bundled Chromium on ubuntu26.04-x64. Focused API and focused E2E evidence for search-007 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..aba2393c 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: 10000, + pathList: true, }), ]) : [null, null]; diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 1c8138e2..aeb14727 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -19059,7 +19059,7 @@ export async function getRepositoryFileFinderFromCookie( repo: string, refName: string, query: string, - options: { page?: number; pageSize?: number } = {}, + options: { page?: number; pageSize?: number; pathList?: boolean } = {}, ): Promise { const params = new URLSearchParams({ ref: refName }); if (query.trim()) { @@ -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.pathList ? "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..2b7d9a11 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; pathList?: 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..7c19fa21 --- /dev/null +++ b/web/tests/e2e/repository-file-finder.spec.ts @@ -0,0 +1,93 @@ +import { execFileSync } from "node:child_process"; +import { expect, type Page, test } from "@playwright/test"; + +const databaseUrl = process.env.TEST_DATABASE_URL ?? process.env.DATABASE_URL; + +type SeededSession = { + cookieName: string; + cookieValue: string; + firstRepositoryHref: string; +}; + +function seedSession(): SeededSession { + if (!databaseUrl) + throw new Error("TEST_DATABASE_URL or DATABASE_URL is required"); + return JSON.parse( + execFileSync( + "cargo", + [ + "run", + "--quiet", + "-p", + "opengithub-api", + "--example", + "dashboard_e2e_seed", + ], + { + cwd: "..", + env: { + ...process.env, + SESSION_COOKIE_NAME: "og_session", + }, + }, + ).toString(), + ) as SeededSession; +} + +async function signIn(page: Page, seeded: SeededSession) { + await page.context().addCookies([ + { + name: seeded.cookieName, + value: seeded.cookieValue, + domain: "localhost", + path: "/", + httpOnly: true, + sameSite: "Lax", + secure: false, + }, + ]); +} + +test.skip( + !databaseUrl, + "repository file finder E2E needs TEST_DATABASE_URL or DATABASE_URL", +); + +test("repo t shortcut opens file finder with local fuzzy filtering and keyboard open", async ({ + page, +}) => { + const seeded = seedSession(); + await signIn(page, seeded); + const repositoryHref = seeded.firstRepositoryHref; + const [, owner, repositoryName] = repositoryHref.split("/"); + + await page.goto(repositoryHref); + await page.keyboard.press("t"); + await expect(page).toHaveURL( + new RegExp(`/${owner}/${repositoryName}/find/main$`), + ); + + const input = page.getByRole("combobox", { name: "Fuzzy-find a file path" }); + await expect(input).toBeFocused(); + await expect(page.getByRole("listbox")).toBeVisible(); + await expect(page.getByText(/cached paths/)).toBeVisible(); + await expect(page.getByRole("option", { name: /README\.md/ })).toBeVisible(); + + await input.fill("read"); + await expect(page.getByRole("option", { name: /README\.md/ })).toBeVisible(); + await expect(page.getByText(/matching paths/)).toBeVisible(); + + await page.keyboard.press("ArrowDown"); + await page.keyboard.press("ArrowUp"); + await page.keyboard.press("Enter"); + await expect(page).toHaveURL( + new RegExp(`/${owner}/${repositoryName}/blob/main/README.md$`), + ); + + await page.goto(`/${owner}/${repositoryName}/find/main`); + await page + .getByRole("combobox", { name: "Fuzzy-find a file path" }) + .fill("zzzz-no-file"); + await expect(page.getByRole("status")).toContainText("No matching files"); + await expect(page.locator('a[href="#"], a:not([href])')).toHaveCount(0); +});