Apps fixes

fileglancer/alembic/versions/b8e4f1a92c37_canonicalize_github_urls.py

@@ -0,0 +1,94 @@
+"""canonicalize stored GitHub URLs
+
+Normalizes app/listing/job GitHub URLs to a single canonical form (no ".git"
+suffix, no trailing slash, no redundant "/tree/main"; SSH folded to https), so
+that an app's URL matches consistently across the catalog, a user's library and
+the launch page. Mirrors fileglancer.giturls.canonical_github_url, inlined here
+so the migration stays self-contained.
+
+Revision ID: b8e4f1a92c37
+Revises: e1a7c93d04f5
+Create Date: 2026-06-26 00:00:00.000000
+
+"""
+import re
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'b8e4f1a92c37'
+down_revision = 'e1a7c93d04f5'
+branch_labels = None
+depends_on = None
+
+
+_HTTPS_RE = re.compile(r"https?://github\.com/([^/]+)/([^/]+?)(?:\.git)?(?:/tree/(.+?))?/?$")
+_SSH_SCP_RE = re.compile(r"git@github\.com:([^/]+)/([^/]+?)(?:\.git)?/?$")
+_SSH_PROTO_RE = re.compile(r"ssh://git@github\.com/([^/]+)/([^/]+?)(?:\.git)?/?$")
+
+
+def _canonical(url):
+    if not url:
+        return url
+    m = _HTTPS_RE.match(url)
+    if m:
+        owner, repo, branch = m.group(1), m.group(2), m.group(3)
+    else:
+        m = _SSH_SCP_RE.match(url) or _SSH_PROTO_RE.match(url)
+        if not m:
+            return url
+        owner, repo, branch = m.group(1), m.group(2), None
+    if branch and branch != "main":
+        return f"https://github.com/{owner}/{repo}/tree/{branch}"
+    return f"https://github.com/{owner}/{repo}"
+
+
+def _canonicalize_unique_table(conn, table, owner_col):
+    """Canonicalize ``url`` for a table with a UNIQUE(owner, url, manifest_path)
+    constraint. Rows that would collide with an existing canonical row are
+    dropped (the canonical row wins) rather than triggering a constraint error."""
+    rows = conn.execute(sa.text(
+        f"SELECT id, {owner_col} AS owner, url, manifest_path FROM {table}"
+    )).fetchall()
+
+    # Pass 1: register rows already in canonical form so non-canonical rows that
+    # map onto them are treated as duplicates.
+    seen = {}
+    for r in rows:
+        if _canonical(r.url) == r.url:
+            seen[(r.owner, r.url, r.manifest_path)] = r.id
+
+    # Pass 2: rewrite the rest, deleting any that collide with a canonical row.
+    for r in rows:
+        canonical = _canonical(r.url)
+        if canonical == r.url:
+            continue
+        key = (r.owner, canonical, r.manifest_path)
+        if key in seen:
+            conn.execute(sa.text(f"DELETE FROM {table} WHERE id = :id"),
+                         {"id": r.id})
+        else:
+            conn.execute(sa.text(f"UPDATE {table} SET url = :url WHERE id = :id"),
+                         {"url": canonical, "id": r.id})
+            seen[key] = r.id
+
+
+def upgrade() -> None:
+    conn = op.get_bind()
+    _canonicalize_unique_table(conn, "user_apps", "username")
+    _canonicalize_unique_table(conn, "app_listings", "owner_username")
+
+    # jobs has no uniqueness constraint on the URL; canonicalize in place.
+    for r in conn.execute(sa.text("SELECT id, app_url FROM jobs")).fetchall():
+        canonical = _canonical(r.app_url)
+        if canonical != r.app_url:
+            conn.execute(sa.text("UPDATE jobs SET app_url = :url WHERE id = :id"),
+                         {"url": canonical, "id": r.id})
+
+
+def downgrade() -> None:
+    # Canonicalization is lossy (the original ".git"/trailing-slash/"/tree/main"
+    # form can't be recovered), so there is nothing to undo.
+    pass

fileglancer/alembic/versions/c1f9a4e7b2d8_bake_revision_into_app_urls.py

@@ -0,0 +1,140 @@
+"""bake the cloned revision into app URLs
+
+Rewrites stored app/listing URLs so the canonical URL always carries the
+revision actually cloned (e.g. ".../tree/master" for a repo whose default is
+"master"; "main" still folds to the bare URL). The ``branch`` column flips to
+mean the *requested* revision — "" when the app was added from a bare URL. The
+revision is fixed at migration/add time; a bare stored URL means the fixed
+"main" revision, not "whatever the default branch is now".
+
+Before this migration ``branch`` held the resolved revision and the bare/master
+ambiguity meant a bare URL and an explicit "/tree/master" were stored as two
+different rows for the same app. Baking the resolved revision into the URL
+closes that gap, so colliding rows are de-duplicated here (the canonical row
+wins), mirroring b8e4f1a92c37.
+
+The requested revision is recovered from the URL's shape: a stored "/tree/<x>"
+was an explicit pin (requested = x), while a bare URL was unpinned
+(requested = "").
+
+Rows whose ``branch`` is NULL are legacy entries migrated from
+user_preferences whose resolved default was never recorded. We can't resolve it
+here (no network), and assuming "main" would break a repo defaulting to e.g.
+"master", so those rows are left untouched and keep tracking the default branch
+until they are re-added. jobs carry no branch column and are historical, so
+they are left untouched too.
+
+Revision ID: c1f9a4e7b2d8
+Revises: b8e4f1a92c37
+Create Date: 2026-06-29 00:00:00.000000
+
+"""
+import re
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'c1f9a4e7b2d8'
+down_revision = 'b8e4f1a92c37'
+branch_labels = None
+depends_on = None
+
+
+_HTTPS_RE = re.compile(r"https?://github\.com/([^/]+)/([^/]+?)(?:\.git)?(?:/tree/(.+?))?/?$")
+_SSH_SCP_RE = re.compile(r"git@github\.com:([^/]+)/([^/]+?)(?:\.git)?/?$")
+_SSH_PROTO_RE = re.compile(r"ssh://git@github\.com/([^/]+)/([^/]+?)(?:\.git)?/?$")
+
+
+def _parse(url):
+    """Return (owner, repo, branch) or None if not a parseable GitHub URL."""
+    if not url:
+        return None
+    m = _HTTPS_RE.match(url)
+    if m:
+        return m.group(1), m.group(2), m.group(3)
+    m = _SSH_SCP_RE.match(url) or _SSH_PROTO_RE.match(url)
+    if m:
+        return m.group(1), m.group(2), None
+    return None
+
+
+def _at_branch(owner, repo, branch):
+    """Canonical URL for owner/repo at branch ("main" folds to the bare URL)."""
+    if branch and branch != "main":
+        return f"https://github.com/{owner}/{repo}/tree/{branch}"
+    return f"https://github.com/{owner}/{repo}"
+
+
+def _target(row):
+    """Return (new_url, requested_branch) for a row, or None to leave it alone.
+
+    A NULL branch is a legacy row (migrated from user_preferences) whose resolved
+    default was never recorded. We can't resolve it here without network, and
+    assuming "main" would break a repo defaulting to e.g. "master". Leave such
+    rows untouched (branch stays NULL) so they keep tracking the default until
+    re-added; only rows with a known resolved revision are rewritten.
+    """
+    if row.branch is None:
+        return None
+    parsed = _parse(row.url)
+    if parsed is None:
+        return None
+    owner, repo, url_branch = parsed
+    requested = url_branch or ""
+    resolved = row.branch or url_branch or "main"
+    return _at_branch(owner, repo, resolved), requested
+
+
+def _migrate_unique_table(conn, table, owner_col):
+    """Bake the resolved revision into ``url`` and set ``branch`` to the
+    requested revision, for a table with UNIQUE(owner, url, manifest_path).
+    Rows whose new URL collides with another row are dropped (canonical wins)."""
+    rows = conn.execute(sa.text(
+        f"SELECT id, {owner_col} AS owner, url, branch, manifest_path FROM {table}"
+    )).fetchall()
+
+    targets = {r.id: _target(r) for r in rows}
+
+    # Pass 1: rows that will remain at their current URL are the winners on
+    # collision. This includes NULL-branch legacy rows (target is None): they are
+    # deliberately left untouched, so another row that bakes to their URL must be
+    # dropped instead of violating the table's UNIQUE(owner, url, manifest_path)
+    # constraint.
+    seen = {}
+    for r in rows:
+        t = targets[r.id]
+        if t is None:
+            seen[(r.owner, r.url, r.manifest_path)] = r.id
+        elif t[0] == r.url:
+            seen[(r.owner, t[0], r.manifest_path)] = r.id
+
+    # Pass 2: rewrite the rest, dropping any that collide with a claimed URL.
+    for r in rows:
+        t = targets[r.id]
+        if t is None:
+            continue
+        new_url, requested = t
+        key = (r.owner, new_url, r.manifest_path)
+        if new_url != r.url and key in seen:
+            conn.execute(sa.text(f"DELETE FROM {table} WHERE id = :id"),
+                         {"id": r.id})
+            continue
+        conn.execute(
+            sa.text(f"UPDATE {table} SET url = :url, branch = :branch WHERE id = :id"),
+            {"url": new_url, "branch": requested, "id": r.id},
+        )
+        seen[key] = r.id
+
+
+def upgrade() -> None:
+    conn = op.get_bind()
+    _migrate_unique_table(conn, "user_apps", "username")
+    _migrate_unique_table(conn, "app_listings", "owner_username")
+
+
+def downgrade() -> None:
+    # Recovering the pre-migration url/branch split would require re-resolving
+    # each repo's default branch over the network, so there is nothing to undo.
+    pass

fileglancer/apps/__init__.py

@@ -3,9 +3,11 @@
 from fileglancer.apps.manifest import (  # noqa: F401
     MANIFEST_FILENAME,
     _ensure_repo_cache,
+    clone_url_for_stored_app,
     discover_app_manifests,
     fetch_app_manifest,
     get_app_branch,
+    canonical_app_url,
     get_or_load_manifest,
     refresh_cached_manifest,
     set_worker_exec,

fileglancer/apps/jobs.py

@@ -19,6 +19,7 @@
 
 from fileglancer import database as db
 from fileglancer.apps.manifest import (
+    clone_url_for_stored_app,
     _dispatch,
     _ensure_repo_cache,
     get_or_load_manifest,
@@ -34,6 +35,7 @@
     _URI_PREFIXES,
 )
 from fileglancer.apps.jobfiles import _build_work_dir
+from fileglancer.giturls import canonical_github_url
 from fileglancer.model import AppEntryPoint
 from fileglancer.settings import get_settings
 
@@ -538,16 +540,30 @@ async def submit_job(
             if v is not None
         }
 
+    stored_app_url = app_url
+    # Not in the user's library: clone the URL as given (a bare URL resolves the
+    # current default). Overridden below with the pinned URL when installed.
+    app_clone_url = app_url
+
     with db.get_db_session(settings.db_url) as session:
         # Read user's container cache dir preference
         cache_dir_pref = db.get_user_preference(session, username, "apptainerCacheDir")
         container_cache_dir = cache_dir_pref.get("value") if cache_dir_pref else None
 
+        # Prefer the name the user saved for this app (which may be a custom name
+        # chosen when adding it from the catalog) over the raw manifest name, so
+        # jobs are labeled consistently with the user's library.
+        user_app = db.get_user_app(session, username, app_url, manifest_path)
+        app_name = user_app.name if user_app is not None else manifest.name
+        if user_app is not None:
+            stored_app_url = user_app.url
+            app_clone_url = clone_url_for_stored_app(stored_app_url, user_app.branch)
+
         db_job = db.create_job(
             session=session,
             username=username,
             app_url=app_url,
-            app_name=manifest.name,
+            app_name=app_name,
             entry_point_id=entry_point.id,
             entry_point_name=entry_point.name,
             entry_point_type=entry_point.type,
@@ -576,15 +592,15 @@ async def submit_job(
     # Ensure the repo is cached in the user's cache (~username/.fileglancer/apps).
     # Pulling is never done here; updates are an explicit user action via the
     # "Update" app endpoint. The manifest read above already reflects the cache.
-    if manifest.repo_url and manifest.repo_url != app_url:
+    if manifest.repo_url and canonical_github_url(manifest.repo_url) != stored_app_url:
         # Manifest and tool code live in separate repos: cache the code repo
         # and run from its root.
         cached_repo_dir = await _ensure_repo_cache(manifest.repo_url, username=username)
         cd_suffix = "repo"
     else:
         # Manifest and tool code share one repo: cache it and run from the
         # subdirectory that contains the manifest.
-        cached_repo_dir = await _ensure_repo_cache(app_url, username=username)
+        cached_repo_dir = await _ensure_repo_cache(app_clone_url, username=username)
         cd_suffix = f"repo/{manifest_path}" if manifest_path else "repo"
 
     # Build environment variable export lines