diff --git a/docs/superpowers/specs/2026-07-18-apps-improvements-design.md b/docs/superpowers/specs/2026-07-18-apps-improvements-design.md new file mode 100644 index 00000000..78a0b5ee --- /dev/null +++ b/docs/superpowers/specs/2026-07-18-apps-improvements-design.md @@ -0,0 +1,47 @@ +# Apps feature — minor improvements design (2026-07-18) + +Four small, independent improvements to the Apps feature. Each ships as its own commit. + +## 1. Paste a GitHub URL with an embedded branch/tag → move it to the Revision box + +When adding an app, a user often pastes a URL copied from GitHub's branch/tag view, e.g. `https://github.com/org/repo/tree/my-branch`. Today that whole string goes into the URL field and the Revision field stays empty; the ref survives only because `buildAppUrl` re-parses it later. We make the split explicit and visible. + +`parseGithubUrl` (`frontend/src/utils/appUrls.ts`) already extracts `{owner, repo, branch}` from a `/tree/` URL. Add a one-line pure helper `splitGithubRef(url)` that returns `{ repoUrl, ref }` — the bare `https://github.com/owner/repo` plus the embedded ref (empty string when the URL carried no ref or the ref is `main`). + +In `AddAppDialog.tsx`, the repo-URL field's `onChange` calls `splitGithubRef` on the new value; when it yields a non-empty ref, set the URL field to the bare repo URL and the Revision field to the ref. Only rewrites when the URL actually carried a `/tree/` ref, so typing a bare URL is undisturbed. A pasted URL's ref is authoritative, so it overwrites whatever is in the Revision box. + +Test: unit test for `splitGithubRef` covering a bare URL, a `/tree/`, a `/tree/main`, a `.git` suffix, and a non-GitHub string. + +## 2. Breadcrumbs on the app launch page + +Replace the back-arrow + title portion of `AppPageHeader` on the launch page (`AppLaunch.tsx`) with a breadcrumb trail styled like the file-browser `Crumbs`: a leading apps-home icon, then `›` (HiChevronRight) delimited segments. + +Trail: `[⊞ apps home → top page] › App Name → app-level page › [entry-point icon] Entry Point Name`. Before an entry point is selected it is just `[⊞] › App Name`. The description, GitHub URL line, and the launch-button actions slot are preserved below the breadcrumb. + +Origin is inferred from install status (no URL/router plumbing): + +- Installed: home → `/apps` (My Apps); App Name → the app detail page (`buildAppDetailPath`). +- Not installed: home → `/apps/catalog` (App Catalog); App Name → the matching catalog listing (`/apps/catalog/:id`), resolved from the already-cached `useCatalogQuery` by canonical url + manifest path. If no match is found, the App Name segment renders as plain text. + +New component `frontend/src/components/ui/AppsPage/AppBreadcrumbs.tsx`. It is a thin flex row of `FgLink`/text + `FgIcon` separators — not a reuse of `BreadcrumbSegment`, whose separator is `/`, because the file-browser trail uses `›` between segments here. + +## 3. Install count in the catalog + +An "install" is a row in `user_apps`; there is no counter column and no association table. Count current installs live rather than denormalizing, so there is no migration and no counter to keep in sync. Semantics: how many users currently have the app installed. + +Backend: add `install_count: int` to the `AppListing` Pydantic model (`fileglancer/model.py`). In `list_catalog` (`fileglancer/server.py`), run one grouped query over `user_apps` — `COUNT` grouped by `(url, manifest_path)` — and join it in memory to the listings, matching by canonical GitHub URL + manifest path (the same identity used elsewhere). A DB helper `count_installs_by_app(session)` returns the grouped counts. This is an O(listings) in-memory join, fine at catalog scale. + +Frontend: add a sortable "Installs" column to `createCatalogColumns` (table view) and show the count on the catalog listing detail page via `ListingInfoTable`. `AppListing` in `shared.types.ts` gains `install_count`. + +Interpretation note: "app detail page" is taken to mean the catalog listing detail page (`ListingDetail`), where the `AppListing` — and therefore the count — is available. The installed-app detail page (`AppDetail`) renders a `UserApp` with no count, so it is out of scope unless requested. + +Test: backend pytest — seed a listing plus N `user_apps` rows for its url/manifest path across distinct users and assert `list_catalog` reports `install_count == N`. + +## 4. Short commit display + rename to "Commit" + +The Job page (`JobDetail.tsx`) shows a "Version" row: `commit_sha.slice(0, 7)` in `text-xs font-mono`, linked to the GitHub commit. The app page (`AppInfoTable.tsx`) shows the full untruncated SHA under an "App commit" label. + +- App page: change `CommitValue` to render `sha.slice(0, 7)` in `text-xs font-mono` (matching the Job page) and rename the "App commit" label to "Commit". The "Code commit" row keeps its label but also renders short via the shared `CommitValue`. +- Job page: rename the "Version" label to "Commit". + +No shared helper is extracted — `.slice(0, 7)` appears in two files and inlining is smaller than a util. diff --git a/fileglancer/apps/jobs.py b/fileglancer/apps/jobs.py index cc115933..6f7e7721 100644 --- a/fileglancer/apps/jobs.py +++ b/fileglancer/apps/jobs.py @@ -37,7 +37,7 @@ _WINDOWS_DRIVE_PATTERN, ) from fileglancer.apps.jobfiles import _build_work_dir -from fileglancer.giturls import canonical_github_url +from fileglancer.giturls import canonical_github_url, same_github_repo from fileglancer.model import AppEntryPoint from fileglancer.settings import get_settings @@ -591,11 +591,12 @@ def _container_bind_paths(entry_point, parameters: dict, dangling. Cloud-storage URIs and non-absolute values are skipped — they are not bind-mountable and would otherwise produce garbage binds. 2. The runnable's explicit `bind_paths`. - 3. The cached repo clone, but only when the command runs from `repo`. The - `repo` symlink lives inside the (already-bound) work dir yet points at - the clone outside it, so without this bind the symlink dangles in the - container and the `cd` into it fails. Container runnables default to - `work`, so this is only added when the author opts into `repo`. + 3. The cached repo clone, but only when the command runs from the clone + (`manifest` or `repo`). The `repo` symlink lives inside the + (already-bound) work dir yet points at the clone outside it, so without + this bind the symlink dangles in the container and the `cd` into it + fails. Container runnables default to `work`, so this is only added when + the author opts into `manifest` or `repo`. The work dir itself is always bound by `_build_container_script`, so it is not included here. @@ -619,7 +620,7 @@ def _container_bind_paths(entry_point, parameters: dict, bind_paths.append(str(PurePosixPath(expanded).parent)) if entry_point.bind_paths: bind_paths.extend(entry_point.bind_paths) - if entry_point.effective_working_dir == "repo": + if entry_point.effective_working_dir != "work": bind_paths.append(str(cached_repo_dir)) return bind_paths @@ -796,12 +797,11 @@ async def submit_job( # so it never drifts again. Pulling is never done here; updates are an # explicit user action via the "Update" app endpoint. executed_repo_url = None - if manifest.repo_url and canonical_github_url(manifest.repo_url) != stored_app_url: - # Manifest and tool code live in separate repos: the job runs from the - # code repo's snapshot root. + if manifest.repo_url and not same_github_repo(manifest.repo_url, stored_app_url): + # Manifest and tool code live in separate repos: the code repo is what + # gets cloned as `repo`. cached_repo_dir, executed_sha = await ensure_repo_snapshot( manifest.repo_url, sha=pinned_code_sha, username=username) - cd_suffix = "repo" executed_repo_url = canonical_github_url(manifest.repo_url) if app_installed and (pinned_sha is None or pinned_code_sha is None): app_sha = pinned_sha @@ -817,16 +817,19 @@ async def submit_job( commit_sha=app_sha, code_commit_sha=executed_sha) else: - # Manifest and tool code share one repo: run from the subdirectory - # that contains the manifest. + # Manifest and tool code share one repo: it gets cloned as `repo`. cached_repo_dir, executed_sha = await ensure_repo_snapshot( app_clone_url, sha=pinned_sha, username=username) - cd_suffix = f"repo/{manifest_path}" if manifest_path else "repo" if app_installed and pinned_sha is None: with db.get_db_session(settings.db_url) as session: db.set_user_app_pins(session, username, stored_app_url, manifest_path, commit_sha=executed_sha) + # 'manifest' working_dir runs from the manifest's subdirectory within the + # clone; 'repo' runs from the clone root. The subdir is the same relative + # path regardless of whether the manifest and code share a repo. + manifest_suffix = f"repo/{manifest_path}" if manifest_path else "repo" + with db.get_db_session(settings.db_url) as session: db_job = db.create_job( session=session, @@ -878,11 +881,11 @@ async def submit_job( # Set up the script preamble: # - FG_WORK_DIR: the job's working directory (used by subsequent variables) - # - FG_MANIFEST_DIR: the directory the job should treat as the app's project root - # (repo[/manifest_path] when the manifest lives with the code; otherwise - # the code repo root), so commands can reference it explicitly instead - # of relying on the cwd. Always points at the repo-side directory, even - # when effective_working_dir is "work" (the repo stays reachable there) + # - FG_MANIFEST_DIR: the manifest's directory inside the clone + # (repo[/manifest_path]), so commands can reference it explicitly + # instead of relying on the cwd. Always points at the repo-side + # directory, even when effective_working_dir is "work" (the repo stays + # reachable there) # - SERVICE_URL_PATH: for service-type jobs, where to write the service URL # - cd into the working directory chosen by effective_working_dir (see below); # this sets where the task's command runs, not how tools locate the manifest @@ -900,7 +903,7 @@ async def submit_job( ] preamble_lines += [ f"export FG_WORK_DIR={shlex.quote(str(work_dir))}", - f'export FG_MANIFEST_DIR="$FG_WORK_DIR"/{shlex.quote(cd_suffix)}', + f'export FG_MANIFEST_DIR="$FG_WORK_DIR"/{shlex.quote(manifest_suffix)}', # Where the script reports its startup phase (e.g. pulling a container # image). The UI reads this to explain a wait before a service is ready. 'export FG_PHASE_PATH="$FG_WORK_DIR/phase"', @@ -928,13 +931,17 @@ async def submit_job( ) # Choose the working directory. 'work' runs from the job's work dir (the # repo is still reachable via the `repo` symlink); 'repo' runs from the - # cloned project (optionally the manifest's subdirectory). cd_suffix may - # include a Git-derived directory name, so shell-escape it — FG_WORK_DIR - # stays in its own double-quoted segment so it still expands. + # cloned project's root; 'manifest' runs from the manifest's directory + # inside the clone (the repo root plus the manifest's subdirectory). + # manifest_suffix may include a Git-derived directory name, so + # shell-escape it — FG_WORK_DIR stays in its own double-quoted segment so + # it still expands. if entry_point.effective_working_dir == "work": preamble_lines.append('cd "$FG_WORK_DIR"') + elif entry_point.effective_working_dir == "repo": + preamble_lines.append('cd "$FG_WORK_DIR"/repo') else: - preamble_lines.append(f'cd "$FG_WORK_DIR"/{shlex.quote(cd_suffix)}') + preamble_lines.append(f'cd "$FG_WORK_DIR"/{shlex.quote(manifest_suffix)}') script_parts = ["\n".join(preamble_lines)] # Conda environment activation diff --git a/fileglancer/database.py b/fileglancer/database.py index fd7d6253..8acd6e59 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -4,11 +4,11 @@ import os from functools import lru_cache -from sqlalchemy import create_engine, Boolean, Column, String, Integer, DateTime, JSON, UniqueConstraint +from sqlalchemy import create_engine, Boolean, Column, String, Integer, DateTime, JSON, UniqueConstraint, func from sqlalchemy.orm import sessionmaker, declarative_base, Session from sqlalchemy.engine.url import make_url from sqlalchemy.pool import StaticPool -from typing import Optional, Dict, List +from typing import Optional, Dict, List, Tuple from loguru import logger from cachetools import LRUCache @@ -1269,6 +1269,25 @@ def get_app_listing(session: Session, listing_id: int) -> Optional[AppListingDB] return session.query(AppListingDB).filter_by(id=listing_id).first() +def count_installs_by_app(session: Session) -> Dict[Tuple[str, str], int]: + """Number of users who currently have each app installed, keyed by + (canonical url, manifest_path). The user_apps unique constraint on + (username, url, manifest_path) means one row per user, so a plain COUNT per + (url, manifest_path) is the distinct-user install count. Both user_apps and + app_listings store canonicalized URLs, so listings can look up their count + by exact (url, manifest_path).""" + rows = ( + session.query( + UserAppDB.url, + UserAppDB.manifest_path, + func.count().label("n"), + ) + .group_by(UserAppDB.url, UserAppDB.manifest_path) + .all() + ) + return {(url, manifest_path): n for url, manifest_path, n in rows} + + def get_app_listings_by_owner(session: Session, owner_username: str) -> List[AppListingDB]: """Get all app listings owned by a user.""" return session.query(AppListingDB).filter_by(owner_username=owner_username).all() diff --git a/fileglancer/giturls.py b/fileglancer/giturls.py index 47d8ff65..b4a9e6c9 100644 --- a/fileglancer/giturls.py +++ b/fileglancer/giturls.py @@ -101,3 +101,19 @@ def github_url_with_branch(owner: str, repo: str, branch: str) -> str: would mean "current default branch" and therefore could move over time. """ return f"https://github.com/{owner}/{repo}/tree/{branch}" + + +def same_github_repo(a: str, b: str) -> bool: + """True if two GitHub URLs point at the same owner/repo, ignoring branch. + + Used to tell "manifest and code live in different repositories" apart from + "same repo, different branch". Comparing canonical URLs conflates the two, + because a bare repo_url (no /tree/branch) never equals a stored app URL that + carries its branch. Returns False if either side isn't a parseable GitHub URL. + """ + try: + oa, ra, _ = _parse_github_url(a) + ob, rb, _ = _parse_github_url(b) + except ValueError: + return False + return (oa, ra) == (ob, rb) diff --git a/fileglancer/model.py b/fileglancer/model.py index 0bf2d2dd..e4472153 100644 --- a/fileglancer/model.py +++ b/fileglancer/model.py @@ -489,12 +489,12 @@ class AppEntryPoint(BaseModel): description="Default extra arguments for container exec (e.g. '--nv')", default=None, ) - working_dir: Optional[Literal["work", "repo"]] = Field( + working_dir: Optional[Literal["work", "manifest", "repo"]] = Field( description=( - "Directory the command runs from: 'repo' (the cloned project, " - "optionally the manifest's subdirectory) or 'work' (the job's work " - "directory). Defaults to 'work' for container entry points and " - "'repo' otherwise." + "Directory the command runs from: 'manifest' (the manifest's " + "directory inside the cloned project), 'repo' (the cloned project's " + "root), or 'work' (the job's work directory). Defaults to 'work' for " + "container entry points and 'manifest' otherwise." ), default=None, ) @@ -595,15 +595,15 @@ def validate_container_args(cls, v): @property def effective_working_dir(self) -> str: - """Resolve where the command runs: 'work' or 'repo'. + """Resolve where the command runs: 'work', 'manifest', or 'repo'. An explicit working_dir wins; otherwise container entry points default to 'work' (the repo clone isn't bind-mounted into the container, but the - work dir always is) and everything else defaults to 'repo'. + work dir always is) and everything else defaults to 'manifest'. """ if self.working_dir: return self.working_dir - return "work" if self.container else "repo" + return "work" if self.container else "manifest" def flat_parameters(self) -> List[AppParameter]: """Return a flat list of all parameters, traversing sections.""" @@ -780,6 +780,7 @@ class AppListing(BaseModel): description: Optional[str] = Field(description="Description for the catalog", default=None) published_at: datetime = Field(description="When this listing was published") updated_at: Optional[datetime] = Field(description="When this listing was last edited", default=None) + install_count: int = Field(description="Number of users who currently have this app installed", default=0) def validate_catalog_listing_name(name: Optional[str]) -> Optional[str]: diff --git a/fileglancer/server.py b/fileglancer/server.py index 351a0c58..662c20fb 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -2099,7 +2099,7 @@ async def validate_paths(body: PathValidationRequest, # --- Catalog (shared apps) API --- - def _listing_to_model(row) -> AppListing: + def _listing_to_model(row, install_count: int = 0) -> AppListing: return AppListing( id=row.id, owner_username=row.owner_username, @@ -2110,6 +2110,7 @@ def _listing_to_model(row) -> AppListing: description=row.description, published_at=row.published_at, updated_at=row.updated_at, + install_count=install_count, ) @app.get("/api/catalog", response_model=list[AppListing], @@ -2117,7 +2118,11 @@ def _listing_to_model(row) -> AppListing: async def list_catalog(username: str = Depends(get_current_user)): with db.get_db_session(settings.db_url) as session: rows = db.list_app_listings(session) - return [_listing_to_model(r) for r in rows] + counts = db.count_installs_by_app(session) + return [ + _listing_to_model(r, counts.get((r.url, r.manifest_path), 0)) + for r in rows + ] @app.post("/api/catalog", response_model=AppListing, description="Share one of the user's apps to the catalog") diff --git a/frontend/src/__tests__/componentTests/AppBreadcrumbs.test.tsx b/frontend/src/__tests__/componentTests/AppBreadcrumbs.test.tsx new file mode 100644 index 00000000..a3e6059a --- /dev/null +++ b/frontend/src/__tests__/componentTests/AppBreadcrumbs.test.tsx @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router'; +import { IoTerminal } from 'react-icons/io5'; + +import AppBreadcrumbs from '@/components/ui/AppsPage/AppBreadcrumbs'; + +function renderCrumbs(props: React.ComponentProps) { + return render( + + + + ); +} + +describe('AppBreadcrumbs', () => { + it('links home and app-name, and renders the entry point as a non-link leaf', () => { + renderCrumbs({ + homeTo: '/apps', + homeLabel: 'My Apps', + appName: 'Demo App', + appTo: '/apps/detail/org/repo', + entryPointName: 'Run', + entryPointIcon: IoTerminal + }); + + // Leading icon links back to the top apps page (named by its sr-only label). + expect(screen.getByRole('link', { name: 'My Apps' })).toHaveAttribute( + 'href', + '/apps' + ); + // App name links to the detail page. + expect(screen.getByRole('link', { name: 'Demo App' })).toHaveAttribute( + 'href', + '/apps/detail/org/repo' + ); + // Entry point is the current, non-clickable leaf. + expect(screen.getByText('Run')).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Run' })).toBeNull(); + }); + + it('renders the app name as plain text when no detail link is available', () => { + renderCrumbs({ + homeTo: '/apps/catalog', + homeLabel: 'App Catalog', + appName: 'Uninstalled App' + }); + + expect(screen.getByRole('link', { name: 'App Catalog' })).toHaveAttribute( + 'href', + '/apps/catalog' + ); + expect(screen.getByText('Uninstalled App')).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Uninstalled App' })).toBeNull(); + }); +}); diff --git a/frontend/src/__tests__/unitTests/appUrls.test.ts b/frontend/src/__tests__/unitTests/appUrls.test.ts index 416f848d..b2c2291a 100644 --- a/frontend/src/__tests__/unitTests/appUrls.test.ts +++ b/frontend/src/__tests__/unitTests/appUrls.test.ts @@ -9,7 +9,8 @@ import { canonicalGithubUrl, isGithubRepoUrl, manifestPathInfo, - parseGithubUrl + parseGithubUrl, + splitGithubRef } from '@/utils/appUrls'; describe('app URL helpers', () => { @@ -82,6 +83,43 @@ describe('app URL helpers', () => { ); }); + test('splitGithubRef separates an embedded ref from the bare repo URL', () => { + // A pasted /tree/ URL yields the bare URL plus the ref. + expect(splitGithubRef('https://github.com/org/tool/tree/dev')).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: 'dev' + }); + // Branch names with slashes are preserved. + expect( + splitGithubRef('https://github.com/org/tool/tree/feature/my-tool') + ).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: 'feature/my-tool' + }); + // A .git suffix is dropped from the bare URL. + expect(splitGithubRef('https://github.com/org/tool.git/tree/v1')).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: 'v1' + }); + // No embedded ref (bare URL, /tree/main, SSH, or garbage) yields ref=''. + expect(splitGithubRef('https://github.com/org/tool')).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: '' + }); + expect(splitGithubRef('https://github.com/org/tool/tree/main')).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: '' + }); + expect(splitGithubRef('git@github.com:org/tool.git')).toEqual({ + repoUrl: 'https://github.com/org/tool', + ref: '' + }); + expect(splitGithubRef('not a url')).toEqual({ + repoUrl: 'not a url', + ref: '' + }); + }); + test('builds launch paths with slash branches in the query string', () => { expect( buildLaunchPath('org', 'tool', 'feature/my-tool', 'run', 'apps/demo') diff --git a/frontend/src/components/AppDetail.tsx b/frontend/src/components/AppDetail.tsx index 8ec6c76e..59843d71 100644 --- a/frontend/src/components/AppDetail.tsx +++ b/frontend/src/components/AppDetail.tsx @@ -5,8 +5,8 @@ import { HiOutlineEllipsisVertical } from 'react-icons/hi2'; import { FaUsers, FaUsersSlash } from 'react-icons/fa6'; import AppActionDialogs from '@/components/ui/AppsPage/AppActionDialogs'; +import AppBreadcrumbs from '@/components/ui/AppsPage/AppBreadcrumbs'; import AppInfoTable from '@/components/ui/AppsPage/AppInfoTable'; -import AppPageHeader from '@/components/ui/AppsPage/AppPageHeader'; import EntryPointsList from '@/components/ui/AppsPage/EntryPointsList'; import CardActionsMenu from '@/components/ui/Menus/CardActionsMenu'; import type { MenuItem } from '@/components/ui/Menus/FgMenuItems'; @@ -18,12 +18,7 @@ import UpdateAvailableBadge from '@/components/ui/AppsPage/UpdateAvailableBadge' import { useAppsQuery, useAppUpdateAvailable } from '@/queries/appsQueries'; import { useJobsQuery } from '@/queries/jobsQueries'; import { useAppActions } from '@/hooks/useAppActions'; -import { - buildGithubUrl, - canonicalGithubUrl, - formatDateString, - getAppIconType -} from '@/utils'; +import { buildGithubUrl, canonicalGithubUrl, formatDateString } from '@/utils'; import type { UserApp } from '@/shared.types'; const RECENT_JOBS_LIMIT = 5; @@ -68,7 +63,11 @@ export default function AppDetail() { if (!app) { return (
- + This app is not in your apps. It may have been removed. @@ -101,7 +100,7 @@ export default function AppDetail() { return (
- {isShared ? ( @@ -159,8 +158,9 @@ export default function AppDetail() { /> } - icon={getAppIconType()} - title={app.name} + appName={app.name} + homeLabel="My Apps" + homeTo="/apps" > {isShared ? ( @@ -168,7 +168,7 @@ export default function AppDetail() { ) : null} {updateAvailable ? : null} - +
diff --git a/frontend/src/components/AppLaunch.tsx b/frontend/src/components/AppLaunch.tsx index f8f2c066..5f047a13 100644 --- a/frontend/src/components/AppLaunch.tsx +++ b/frontend/src/components/AppLaunch.tsx @@ -10,24 +10,28 @@ import { Card, Typography } from '@material-tailwind/react'; import { HiOutlineDownload, HiOutlinePlay } from 'react-icons/hi'; import toast from 'react-hot-toast'; +import AppBreadcrumbs from '@/components/ui/AppsPage/AppBreadcrumbs'; import AppLaunchForm from '@/components/ui/AppsPage/AppLaunchForm'; -import AppPageHeader from '@/components/ui/AppsPage/AppPageHeader'; import SharedBadge from '@/components/ui/AppsPage/SharedBadge'; import { buildAppDetailPath, buildGithubUrl, canonicalGithubUrl, - getEntryPointTypeIconType, - getAppIconType + getEntryPointTypeIconType } from '@/utils'; import { showErrorToast } from '@/utils/errorToast'; import { useAppsQuery, useAddAppMutation, + useCatalogQuery, useManifestPreviewMutation } from '@/queries/appsQueries'; import { useSubmitJobMutation } from '@/queries/jobsQueries'; -import type { AppEntryPoint, AppResourceDefaults } from '@/shared.types'; +import type { + AppEntryPoint, + AppResourceDefaults, + LaunchOrigin +} from '@/shared.types'; import FgButton from './designSystem/atoms/FgButton'; export default function AppLaunch() { @@ -48,6 +52,7 @@ export default function AppLaunch() { const manifestMutation = useManifestPreviewMutation(); const submitJobMutation = useSubmitJobMutation(); const appsQuery = useAppsQuery(); + const catalogQuery = useCatalogQuery(); const addAppMutation = useAddAppMutation(); const [selectedEntryPoint, setSelectedEntryPoint] = useState(null); @@ -99,6 +104,31 @@ export default function AppLaunch() { ); const isInstalled = installedApp !== undefined; + // Breadcrumb origin: prefer the origin recorded in navigation state (set when + // launching from My Apps vs the App Catalog) so the breadcrumbs link back to + // where the user actually came from. On reload or direct navigation state is + // lost, so fall back to inferring the origin from install status: an installed + // app is treated as reached from My Apps and links to its detail page; an + // uninstalled app is treated as browsed from the catalog and links to its + // listing (matched in the already-cached catalog by canonical URL + path). + const originState = (location.state as { from?: LaunchOrigin } | null)?.from; + const catalogListing = catalogQuery.data?.find( + l => + canonicalGithubUrl(l.url) === appUrl && l.manifest_path === manifestPath + ); + const fromCatalog = !isInstalled && catalogListing !== undefined; + const homeTo = + originState?.homeTo ?? (fromCatalog ? '/apps/catalog' : '/apps'); + const homeLabel = + originState?.homeLabel ?? (fromCatalog ? 'App Catalog' : 'My Apps'); + const appDetailTo = + originState?.appTo ?? + (installedApp + ? buildAppDetailPath(installedApp.url, installedApp.manifest_path) + : catalogListing + ? `/apps/catalog/${catalogListing.id}` + : undefined); + useEffect(() => { if (appUrl) { // The manifest identity is (url, manifest_path); reset the selection so @@ -115,10 +145,6 @@ export default function AppLaunch() { // Prefer the user's saved app name (which may be a custom name chosen when // adding the app from the catalog) over the raw manifest name. const displayName = installedApp?.name ?? manifest?.name; - const headerTitle = - displayName && selectedEntryPoint - ? `${displayName} - ${selectedEntryPoint.name}` - : displayName; const isShared = installedApp?.listing_id !== undefined && installedApp.listing_id !== null; @@ -198,29 +224,27 @@ export default function AppLaunch() { return (
- ) : null } - backLabel={installedApp ? 'Back to app details' : 'Back to My Apps'} - backTo={ - installedApp - ? buildAppDetailPath(installedApp.url, installedApp.manifest_path) - : '/apps' - } + appName={displayName} + appTo={appDetailTo} description={selectedEntryPoint?.description ?? null} - githubUrl={selectedEntryPoint ? appUrl : null} - icon={ + entryPointIcon={ selectedEntryPoint ? getEntryPointTypeIconType(selectedEntryPoint.type) - : getAppIconType() + : undefined } - title={headerTitle} + entryPointName={selectedEntryPoint?.name} + githubUrl={selectedEntryPoint ? appUrl : null} + homeLabel={homeLabel} + homeTo={homeTo} > {isShared ? : null} - + {/* Not-installed banner — only once the apps list has actually loaded, so a failed /api/apps query doesn't wrongly flag every app (including diff --git a/frontend/src/components/Catalog.tsx b/frontend/src/components/Catalog.tsx index 37c4214e..9ed07bef 100644 --- a/frontend/src/components/Catalog.tsx +++ b/frontend/src/components/Catalog.tsx @@ -129,7 +129,7 @@ export default function Catalog() { data={filteredListings} dataType="shared apps" errorState={catalogQuery.error} - gridColsClass="grid-cols-[2fr_2fr_3fr_1fr_1fr_1fr_1fr]" + gridColsClass="grid-cols-[minmax(8rem,2fr)_minmax(10rem,5fr)_7rem_7rem_5rem_6.5rem_4.5rem]" initialPageSize={50} loadingState={catalogQuery.isPending} /> diff --git a/frontend/src/components/JobDetail.tsx b/frontend/src/components/JobDetail.tsx index d226904d..145292be 100644 --- a/frontend/src/components/JobDetail.tsx +++ b/frontend/src/components/JobDetail.tsx @@ -368,11 +368,9 @@ function JobOverview({ label="App" value={ detailPath ? ( - - {`${job.app_name} — ${job.entry_point_name}`} - + {`${job.app_name}`} ) : ( - `${job.app_name} — ${job.entry_point_name}` + `${job.app_name}` ) } /> @@ -387,7 +385,7 @@ function JobOverview({ } /> (); @@ -81,7 +75,11 @@ export default function ListingDetail() { if (!listing) { return (
- + This catalog listing does not exist. It may have been unshared. @@ -118,8 +116,7 @@ export default function ListingDetail() { return (
- {!alreadyAdded ? ( @@ -170,15 +167,16 @@ export default function ListingDetail() { ) : null} } - icon={getAppIconType()} - title={listing.name} + appName={listing.name} + homeLabel="App Catalog" + homeTo="/apps/catalog" > {alreadyAdded ? ( In your apps ) : null} - +
) : ( + onLaunch={ep => { + // Record the App Catalog origin so the launch breadcrumbs link + // back to this listing, even when the app is also installed. + const from: LaunchOrigin = { + homeTo: '/apps/catalog', + homeLabel: 'App Catalog', + appTo: buildListingDetailPath(listing.id) + }; navigate( buildLaunchPathFromApp( listing.url, listing.manifest_path, ep.id - ) - ) - } + ), + { state: { from } } + ); + }} runnables={manifest?.runnables ?? []} /> )} diff --git a/frontend/src/components/ui/AppsPage/AddAppDialog.tsx b/frontend/src/components/ui/AppsPage/AddAppDialog.tsx index d7b7c2ad..60fb0ed2 100644 --- a/frontend/src/components/ui/AppsPage/AddAppDialog.tsx +++ b/frontend/src/components/ui/AppsPage/AddAppDialog.tsx @@ -7,7 +7,7 @@ import FgFormField from '@/components/designSystem/molecules/FgFormField'; import FgInput from '@/components/designSystem/atoms/formElements/FgInput'; import FgCheckbox from '@/components/designSystem/atoms/formElements/FgCheckbox'; import AppTrustNotice from '@/components/ui/AppsPage/AppTrustNotice'; -import { buildAppUrl, isGithubRepoUrl } from '@/utils'; +import { buildAppUrl, isGithubRepoUrl, splitGithubRef } from '@/utils'; import type { DiscoveredApp } from '@/shared.types'; interface AddAppDialogProps { @@ -153,9 +153,19 @@ export default function AddAppDialog({ { - const value = e.target.value; - setRepoUrl(value); - validateUrl(value); + // If the pasted URL embeds a branch/tag (…/tree/), move + // the ref into the Revision field and keep the URL field bare. + const { repoUrl: bareUrl, ref } = splitGithubRef( + e.target.value + ); + if (ref) { + setRepoUrl(bareUrl); + setBranch(ref); + validateUrl(bareUrl); + } else { + setRepoUrl(e.target.value); + validateUrl(e.target.value); + } }} onKeyDown={e => { if ( diff --git a/frontend/src/components/ui/AppsPage/AppBreadcrumbs.tsx b/frontend/src/components/ui/AppsPage/AppBreadcrumbs.tsx new file mode 100644 index 00000000..3bc420c7 --- /dev/null +++ b/frontend/src/components/ui/AppsPage/AppBreadcrumbs.tsx @@ -0,0 +1,141 @@ +import type { ReactNode } from 'react'; +import { Typography } from '@material-tailwind/react'; +import { HiChevronRight } from 'react-icons/hi'; +import { HiOutlineSquares2X2 } from 'react-icons/hi2'; +import { TbBrandGithub } from 'react-icons/tb'; +import type { IconType } from 'react-icons'; + +import FgExternalLink from '@/components/designSystem/atoms/FgExternalLink'; +import FgIcon from '@/components/designSystem/atoms/FgIcon'; +import FgLink from '@/components/designSystem/atoms/FgLink'; +import FgTooltip from '@/components/ui/widgets/FgTooltip'; + +interface AppBreadcrumbsProps { + /** Link target for the leading apps-home icon (My Apps or the Catalog). */ + readonly homeTo: string; + /** Accessible/tooltip label for the home icon. */ + readonly homeLabel: string; + /** App name segment; omitted while the manifest is still loading. */ + readonly appName?: string; + /** When set, the app name links to the app/listing detail page. */ + readonly appTo?: string; + /** Entry-point segment (the current, non-clickable leaf). */ + readonly entryPointName?: string; + readonly entryPointIcon?: IconType; + /** Right-aligned actions (e.g. the launch button). */ + readonly actions?: ReactNode; + readonly description?: string | null; + readonly githubUrl?: string | null; + /** Trailing content next to the trail, e.g. a "Shared" badge. */ + readonly children?: ReactNode; +} + +function Separator() { + return ( + + ); +} + +/** + * Breadcrumb header for the app launch page, styled like the file-browser + * breadcrumbs: a leading apps-home icon that returns to the top apps page, + * followed by `›`-delimited segments for the app and the selected entry point. + * Description and GitHub URL render below, mirroring AppPageHeader. + */ +export default function AppBreadcrumbs({ + homeTo, + homeLabel, + appName, + appTo, + entryPointName, + entryPointIcon, + actions, + description, + githubUrl, + children +}: AppBreadcrumbsProps) { + return ( +
+
+ + {actions ? ( +
{actions}
+ ) : null} +
+ {description ? ( + + {description} + + ) : null} + {githubUrl ? ( +
+ + + {githubUrl} + +
+ ) : null} +
+ ); +} diff --git a/frontend/src/components/ui/AppsPage/AppInfoTable.tsx b/frontend/src/components/ui/AppsPage/AppInfoTable.tsx index 0f4c32fe..e49dfbe7 100644 --- a/frontend/src/components/ui/AppsPage/AppInfoTable.tsx +++ b/frontend/src/components/ui/AppsPage/AppInfoTable.tsx @@ -40,15 +40,17 @@ function CommitValue({ readonly sha: string; readonly href: string | null; }) { + // Short SHA in monospace, formatted identically to the Job page "Commit" row. + const short = sha.slice(0, 7); if (href) { return ( - - {sha} + + {short} ); } - return {sha}; + return {short}; } function GithubUrlValue({ url }: { readonly url: string }) { @@ -116,7 +118,7 @@ export default function AppInfoTable({ app }: { readonly app: UserApp }) { )} {app.commit_sha ? ( - + ) : null} diff --git a/frontend/src/components/ui/AppsPage/ListingInfoTable.tsx b/frontend/src/components/ui/AppsPage/ListingInfoTable.tsx index 5ee1b8e0..cf806af9 100644 --- a/frontend/src/components/ui/AppsPage/ListingInfoTable.tsx +++ b/frontend/src/components/ui/AppsPage/ListingInfoTable.tsx @@ -90,6 +90,7 @@ export default function ListingInfoTable({ {listing.owner_username} {publishedAt} {editedAt} + {listing.install_count} {installedApp ? ( [] { return [ - { - id: 'repository', - accessorFn: row => repoLabel(row.url), - header: 'Repository', - cell: ({ getValue, table }) => { - const value = getValue() as string; - const onContextMenu = table.options.meta?.onCellContextMenu; - return ( -
{ - e.preventDefault(); - onContextMenu?.(e, { value }); - }} - > - - {value} - -
- ); - }, - enableSorting: true - }, { accessorKey: 'name', header: 'App Name', @@ -133,6 +110,26 @@ export function createCatalogColumns( }, enableSorting: true }, + { + accessorKey: 'install_count', + header: () => Installs, + cell: ({ getValue, table }) => { + const value = getValue() as number; + const onContextMenu = table.options.meta?.onCellContextMenu; + return ( +
{ + e.preventDefault(); + onContextMenu?.(e, { value: String(value) }); + }} + > + {value} +
+ ); + }, + enableSorting: true + }, { id: 'status', accessorFn: row => (getInstalledApp(row) ? 'In your apps' : ''), diff --git a/frontend/src/hooks/useAppActions.ts b/frontend/src/hooks/useAppActions.ts index 724197f3..8df7bc81 100644 --- a/frontend/src/hooks/useAppActions.ts +++ b/frontend/src/hooks/useAppActions.ts @@ -10,7 +10,7 @@ import { } from '@/queries/appsQueries'; import { buildAppDetailPath, buildLaunchPathFromApp } from '@/utils'; import { showErrorToast } from '@/utils/errorToast'; -import type { UserApp } from '@/shared.types'; +import type { LaunchOrigin, UserApp } from '@/shared.types'; export interface AppActions { launch: (app: UserApp, entryPointId?: string) => void; @@ -58,7 +58,16 @@ export function useAppActions(opts?: { onRemoved?: () => void }): AppActions { const unshareListingMutation = useUnshareListingMutation(); const launch = (app: UserApp, entryPointId?: string) => { - navigate(buildLaunchPathFromApp(app.url, app.manifest_path, entryPointId)); + // Record the My Apps origin so the launch breadcrumbs link back here rather + // than inferring the origin from install status. + const from: LaunchOrigin = { + homeTo: '/apps', + homeLabel: 'My Apps', + appTo: buildAppDetailPath(app.url, app.manifest_path) + }; + navigate(buildLaunchPathFromApp(app.url, app.manifest_path, entryPointId), { + state: { from } + }); }; const view = (app: UserApp) => { diff --git a/frontend/src/layouts/AppsLayout.tsx b/frontend/src/layouts/AppsLayout.tsx index a602cc6e..79ea62bc 100644 --- a/frontend/src/layouts/AppsLayout.tsx +++ b/frontend/src/layouts/AppsLayout.tsx @@ -12,6 +12,7 @@ import { import FgBadge from '@/components/designSystem/atoms/FgBadge'; import FgIcon from '@/components/designSystem/atoms/FgIcon'; import { useActiveJobCount } from '@/hooks/useActiveJobCount'; +import type { LaunchOrigin } from '@/shared.types'; interface TabItem { to: string; @@ -46,15 +47,32 @@ function TabAccent({ active }: { readonly active: boolean }) { export default function AppsLayout() { const activeJobCount = useActiveJobCount(); - const { pathname } = useLocation(); + const location = useLocation(); + const { pathname } = location; - // App detail and launch pages are drill-downs from My Apps, so keep that tab - // highlighted there (NavLink's own matching would mark it inactive). + // The launch/relaunch pages are drill-downs with no tab of their own, so keep + // the originating tab selected. The origin is recorded in navigation state + // (My Apps vs App Catalog); without it (reload/direct nav) default to My Apps. + const launchOrigin = (location.state as { from?: LaunchOrigin } | null)?.from; + const onLaunchPage = + pathname.startsWith('/apps/launch/') || + pathname.startsWith('/apps/relaunch/'); + const catalogLaunch = + onLaunchPage && launchOrigin?.homeTo === '/apps/catalog'; + + // App detail and launches originating from My Apps keep My Apps highlighted + // (NavLink's own matching would mark it inactive on these drill-downs). const myAppsActive = pathname === '/apps' || pathname.startsWith('/apps/detail/') || - pathname.startsWith('/apps/launch/') || - pathname.startsWith('/apps/relaunch/'); + (onLaunchPage && !catalogLaunch); + + // App Catalog stays highlighted on its own pages and on a catalog-originated + // launch (where the pathname alone wouldn't match). + const catalogActive = + pathname === '/apps/catalog' || + pathname.startsWith('/apps/catalog/') || + catalogLaunch; const tabs: TabItem[] = [ { @@ -64,7 +82,12 @@ export default function AppsLayout() { end: true, isActive: myAppsActive }, - { to: '/apps/catalog', label: 'App Catalog', icon: HiOutlineSquares2X2 }, + { + to: '/apps/catalog', + label: 'App Catalog', + icon: HiOutlineSquares2X2, + isActive: catalogActive + }, { to: '/apps/jobs', label: 'Jobs', diff --git a/frontend/src/shared.types.ts b/frontend/src/shared.types.ts index 7f9112fe..fd632d86 100644 --- a/frontend/src/shared.types.ts +++ b/frontend/src/shared.types.ts @@ -130,7 +130,7 @@ type AppEntryPoint = { container?: string; bind_paths?: string[]; container_args?: string; - working_dir?: 'work' | 'repo'; + working_dir?: 'work' | 'manifest' | 'repo'; auto_url?: boolean; service_url_suffix?: string; requirements?: string[]; @@ -184,6 +184,7 @@ type AppListing = { description?: string; published_at: string; updated_at?: string; + install_count: number; }; type JobFileInfo = { @@ -299,6 +300,21 @@ function parseAppLaunchParamsFile(text: string): AppLaunchParamsFile { return parsed as AppLaunchParamsFile; } +/** + * Breadcrumb origin for the app launch page, passed via React Router location + * state so the launch breadcrumbs point back to the page the user actually came + * from (My Apps vs the App Catalog) rather than inferring it from install + * status. Absent on reload or direct navigation, where the launch page falls + * back to inferring the origin. + */ +type LaunchOrigin = { + /** Top-level page the leading breadcrumb icon returns to. */ + homeTo: string; + homeLabel: string; + /** App-level page the app-name segment links to (detail or listing page). */ + appTo?: string; +}; + export type { AppEntryPoint, AppLaunchParamsFile, @@ -318,6 +334,7 @@ export type { JobFileInfo, JobStatus, JobSubmitRequest, + LaunchOrigin, Profile, Result, Success, diff --git a/frontend/src/utils/appUrls.ts b/frontend/src/utils/appUrls.ts index 969d14e5..8cd90208 100644 --- a/frontend/src/utils/appUrls.ts +++ b/frontend/src/utils/appUrls.ts @@ -92,6 +92,29 @@ export function buildGithubUrl( return `https://github.com/${owner}/${repo}/tree/${branch}`; } +/** + * Split a pasted GitHub URL that embeds a branch/tag (e.g. + * `.../repo/tree/my-branch`) into the bare repo URL and the ref, so the ref can + * be surfaced in a separate revision field. `ref` is empty when the URL carries + * no ref (or the default `main`), or when the string isn't a parseable GitHub + * URL — callers should only rewrite the input when `ref` is non-empty, leaving + * bare and SSH URLs untouched. + */ +export function splitGithubRef(url: string): { + repoUrl: string; + ref: string; +} { + try { + const { owner, repo, branch } = parseGithubUrl(url); + return { + repoUrl: buildGithubUrl(owner, repo, 'main'), + ref: branch === 'main' ? '' : branch + }; + } catch { + return { repoUrl: url, ref: '' }; + } +} + /** * The revision actually cloned, parsed out of the canonical app URL (which * always carries it). Falls back to the requested branch, then null. diff --git a/frontend/src/utils/index.ts b/frontend/src/utils/index.ts index 9ee0f82d..d1087dfc 100644 --- a/frontend/src/utils/index.ts +++ b/frontend/src/utils/index.ts @@ -420,7 +420,8 @@ export { buildLaunchPathFromApp, buildAppDetailPath, buildRelaunchPath, - manifestPathInfo + manifestPathInfo, + splitGithubRef } from './appUrls'; // Re-export app icon utilities diff --git a/tests/test_apps.py b/tests/test_apps.py index 68ef7fc0..1f4b271a 100644 --- a/tests/test_apps.py +++ b/tests/test_apps.py @@ -870,15 +870,17 @@ def test_explicit_bind_paths_included(self): binds = _container_bind_paths(ep, {}, None, None, "/cache/repo") assert "/shared/ref" in binds and "/scratch" in binds - def test_repo_bound_only_when_working_dir_repo(self): + def test_repo_bound_only_when_running_from_clone(self): # Container default is working_dir=work → repo NOT bound. work_ep = self._ep() assert "work" == work_ep.effective_working_dir assert "/cache/repo" not in _container_bind_paths(work_ep, {}, None, None, "/cache/repo") - # Opt into repo → the cached clone is bound so the repo symlink resolves. - repo_ep = self._ep(working_dir="repo") - assert "/cache/repo" in _container_bind_paths(repo_ep, {}, None, None, "/cache/repo") + # Opt into manifest or repo → the cached clone is bound so the repo + # symlink resolves. + for wd in ("manifest", "repo"): + ep = self._ep(working_dir=wd) + assert "/cache/repo" in _container_bind_paths(ep, {}, None, None, "/cache/repo") def test_file_directory_default_is_bound(self): # A file/directory param the user did not override still contributes its @@ -2496,11 +2498,11 @@ def test_slashed_branch_naming(self, tmp_path): class TestEffectiveWorkingDir: """working_dir resolution: explicit wins; containers default to 'work', - everything else to 'repo'.""" + everything else to 'manifest'.""" - def test_default_is_repo(self): + def test_default_is_manifest(self): ep = AppEntryPoint(id="r", name="r", command="python x.py") - assert ep.effective_working_dir == "repo" + assert ep.effective_working_dir == "manifest" def test_container_defaults_to_work(self): ep = AppEntryPoint(id="r", name="r", command="cowsay hi", @@ -2879,6 +2881,27 @@ def test_script_layout_and_parameter_quoting(self, tmp_path, monkeypatch): # The local executor records the exit code for PID polling. assert 'trap \'echo $? > "$FG_WORK_DIR/exit_code"\' EXIT' in script + def test_working_dir_repo_vs_manifest_with_subdir(self, tmp_path, monkeypatch): + # With a manifest in a subdirectory: 'manifest' cd's into the subdir, + # 'repo' always cd's into the clone root. + _, calls = self._submit( + tmp_path, monkeypatch, + entry_point={"working_dir": "manifest"}, + manifest_path="tools/rnaseq", + ) + assert 'cd "$FG_WORK_DIR"/repo/tools/rnaseq' in self._submitted(calls)["command"] + + _, calls = self._submit( + tmp_path, monkeypatch, + entry_point={"working_dir": "repo"}, + manifest_path="tools/rnaseq", + ) + script = self._submitted(calls)["command"] + assert 'cd "$FG_WORK_DIR"/repo\n' in script or script.endswith('cd "$FG_WORK_DIR"/repo') + # The cd goes to the clone root, not the manifest subdir (FG_MANIFEST_DIR + # still points at the subdir, so check the cd line specifically). + assert 'cd "$FG_WORK_DIR"/repo/tools/rnaseq' not in script + def test_non_local_executor_omits_exit_code_trap(self, tmp_path, monkeypatch): _, calls = self._submit(tmp_path, monkeypatch, cluster={"executor": "lsf"}) assert "exit_code" not in self._submitted(calls)["command"] diff --git a/tests/test_catalog_endpoints.py b/tests/test_catalog_endpoints.py index ca00d85a..148d48e7 100644 --- a/tests/test_catalog_endpoints.py +++ b/tests/test_catalog_endpoints.py @@ -554,6 +554,26 @@ def test_list_catalog_visible_to_other_users(client_factory, db_session): assert body[0]["name"] == "Shared by Alice" +def test_list_catalog_reports_install_count(client_factory, db_session): + """install_count is the number of distinct users who currently have the app + installed, matched to each listing by (url, manifest_path).""" + _seed_listing(db_session, owner_username=OWNER) + for user in (OWNER, ADOPTER, "carol"): + _seed_user_app(db_session, username=user) + + # A second listing that nobody has installed must report 0. + _seed_listing( + db_session, owner_username=OWNER, + url="https://github.com/owner/other", name="Uninstalled") + + client = client_factory(ADOPTER) + response = client.get("/api/catalog") + assert response.status_code == 200 + by_url = {listing["url"]: listing for listing in response.json()} + assert by_url["https://github.com/owner/repo"]["install_count"] == 3 + assert by_url["https://github.com/owner/other"]["install_count"] == 0 + + def test_listings_owner_helper(db_session): """Sanity check: get_app_listings_by_owner returns only owner's listings.""" _seed_listing(db_session, owner_username=OWNER,