Support GitHub /tree/<branch>/<subdir> URLs in project import — fixes the appstore "Remix on CloudPebble" button - #77
Conversation
…mmit/ forms The import dialog and the /ide/import/github/... deep links receive many shapes of GitHub reference: bare user/repo, http(s)/www/git@/git:// URLs (host case-insensitive, percent-encoding tolerated), .git suffixes, and web URLs carrying a branch and subdirectory (/tree/<ref>/<dir>, /blob/<ref>/<file>, /commit/<sha>, and the self-qualified refs/heads|tags/ spellings GitHub's Raw button emits) — the /tree/ shape is what the Pebble appstore's "Remix on CloudPebble" button generates, and what StackBlitz, CodeSandbox, degit and create-next-app all accept. The module is dependency-free so its tests run with plain unittest. Branch names may contain slashes, so the ref-vs-path split is left to split_ref_and_path() against a real ref list, longest prefix first, the way gitpick and gitingest resolve GitHub web URLs — git's ref directory/file-conflict rule guarantees at most one branch and one tag can match, so the split is deterministic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
find_project_root_and_manifest() picks the first valid manifest in the archive, which is wrong for repositories that contain several projects (a library whose root is itself a Pebble project, plus examples and watchfaces in subdirectories). A root_hint restricts the match to the named directory — allowing at most one wrapping folder above it, since GitHub archives prefix everything with <repo>-<ref>/ — and failure reports the path it looked at. A bare suffix match is deliberately NOT enough (a hint like 'src' must not latch onto any directory of that name at any depth); tests cover both zip shapes and the rejections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Importing github.com/<user>/<repo>/tree/<branch>/<subdir> now does what it says: the API parses the URL (previously the regex silently dropped everything after the repo and imported the wrong project from the default branch), the celery task resolves the ambiguous <ref>/<path> remainder against the repository's real branch and tag names — falling back to probing the strict codeload zip/refs/heads/... endpoint when the API is unavailable, since bare archive/<x>.zip answers 200 even for junk refs (measured: archive/main/anything.zip serves main) — and the archive importer pins the project root to the subdirectory. /blob/<ref>/<file> imports the file's directory; /commit/<sha> imports at that commit. Also fixes the empty-branch default: the client used to hardcode 'master', which broke every main-default repository; an empty branch now imports codeload's HEAD, i.e. the repository's default branch. add_remote is rejected for subdirectory imports (a linked push would target the repository root) with a clear message. Legacy /ide/import/github/<user>/<repo>/<branch> deep links keep working. This makes the Pebble appstore's "Remix on CloudPebble" button work: its links have the /ide/import/github/<user>/<repo>/tree/<branch>/<dir> shape, which previously prefilled 'tree/main/...' into the branch box. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The old server regex matched github.com[/:], so the bare-colon form github.com:user/repo imported fine — the new parser now accepts it too, and a parity test walks every form the old regex accepted. On the JS side, vitest tests pin the deep-link prefill contract: the legacy /ide/import/github/<user>/<repo>[/<branch>] links (slashed branches included) behave exactly as before, while /tree/<branch>/<subdir> links hand the whole URL to the server with the branch box left empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves CloudPebble’s GitHub importer so that GitHub web URLs (not just repo roots) can be pasted/deep-linked and correctly import from the intended ref and optional subdirectory—fixing appstore “Remix on CloudPebble” links that include /tree/<branch>/<subdir> and removing the legacy master fallback.
Changes:
- Add a dependency-free GitHub URL parser (
parse_github_source,split_ref_and_path,normalize_subpath) with unit tests. - Add
root_hintsupport to archive import/project-root discovery to pin imports to a specific subdirectory when requested. - Wire parsing + ref/path resolution through the API, Celery GitHub import task, and importer UI/deep-link handling; default empty branch to
HEAD.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cloudpebble/ide/views/run.py | Expands GitHub URL extraction so the “import” deep link can include `/tree |
| cloudpebble/ide/utils/project.py | Adds root_hint matching to force project-root selection to a specified subdirectory. |
| cloudpebble/ide/utils/github_urls.py | Introduces GitHub source parsing + ref/path splitting + subpath normalization helpers. |
| cloudpebble/ide/tests/test_github_urls.py | Adds unit tests for the new GitHub URL parsing utilities. |
| cloudpebble/ide/tests/test_find_project_root.py | Adds tests for root_hint behavior during project root discovery. |
| cloudpebble/ide/tasks/git.py | Adds ref/path resolution logic for `/tree |
| cloudpebble/ide/tasks/archive.py | Plumbs root_hint through archive import into project-root discovery. |
| cloudpebble/ide/static/ide/js/project_list.js | Updates client-side validation/comments and improves deep-link prefill for /ide/import/github/... routes. |
| cloudpebble/ide/api/project.py | Switches to URL parsing via parse_github_source and passes ref/kind through to the import task. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d9997961d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Split raw query/fragment delimiters before percent-decoding, so an encoded '#' or '?' survives inside a path segment (and drop the regex's now-redundant fragment tail) - Strip refs/heads|tags qualifiers before the codeload probe fallback and pin the probe to that namespace - Keep branch-only /tree/<branch> URLs on the full legacy flow, including "Use as Git remote" - Stop recommending linking-from-settings for subdirectory imports (a linked push would re-find a project root in the whole repository and could overwrite a different project); commit URLs get their own message - Apply the 400-file limit to the hinted project subtree instead of the whole repository, with a 50k ceiling on the pre-scan - Reject any '..' segment in subpaths outright - Route the build-status GitHub link through parse_github_source (one authority, case-insensitive host) with a plain-repository fallback, and percent-encode refs in generated URLs - Translate the hinted-import error before interpolating the hint - Suggest the repository name for blob/commit deep links (tree URLs keep the subdirectory suggestion) - Tests: resolver unit tests with a mocked ref list and probe, /ide/import/github linking flows, hinted big-repository archive imports, run-view link translation, parser delimiter cases (Python 237 -> 252, JS 34 -> 35) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All 11 review comments are addressed in f7ca2f2 — item-by-item replies are in the threads. Net: 12 files, +371/−29. Beyond the direct fixes, the review round motivated three small hardening changes: the hinted pre-scan got an absolute ceiling (so the subtree limit can't turn root-finding into an unbounded walk), generated archive/probe URLs percent-encode refs, and the build-status view now reuses Tests after this round: Python 237 → 252 (full Co-Authored-By: Claude Fable 5 noreply@anthropic.com |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7ca2f2f9c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…owing - Resolve slashed /tree/<branch> URLs against the repository's real refs before refusing to link (the refusal stays, conservatively, when the ref list is unavailable) - Pin refs/heads|tags-qualified URLs to their namespace on the API path too: get_ref_names now takes the wanted namespaces, so a cross-namespace tag can no longer longest-prefix-steal a remainder the URL claimed as a branch - Treat the empty resolved path of a refpath-carrying URL as an explicit repository-root hint (e.g. /blob/<ref>/<root file>) instead of falling back to the first-manifest heuristic - Check the root hint before reading manifest contents, so a big repository's out-of-tree manifests are never opened or parsed - Bump the project_list.js cache key so cached clients pick up the deep-link fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… hints - Extract split_ref_qualifier() as the single authority for GitHub's refs/heads|tags-qualified spelling (it was implemented twice and skipped in the linking check, which refused qualified branch URLs with a misleading message) - Make the empty resolved path kind-aware: only /blob/ URLs pin the explicit repository root; /tree/ and /commit/ URLs with no subdirectory keep the same heuristic a branch-box import gets, so nested-project repositories import identically through either door - Replace the linking ref-list lookup with a single branch probe (get_ref_names paginates every branch and tag — unbounded inside a web request — and would also have linked tags as branches) - Give blob links their own linking-refusal message - Name the per-project entry limit (PROJECT_ENTRY_LIMIT) instead of duplicating a magic 400, and let a broken-manifest diagnosis win over the generic hint error - Drop the dialog's stale placeholder="master"; decode the suggested project name in deep links - Tests for all of it, including the previously-untested do_import_github glue (HEAD default branch, root_hint per URL kind) and the JS submit handler (no master fallback returns): Python 274, JS 39 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 916d49d899
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Overlap note: #76 (opened earlier the same day) addresses the import half of the same default-branch problem by flipping the hardcoded Co-Authored-By: Claude Fable 5 noreply@anthropic.com |
- /tree/<x> with "Use as Git remote" now probes the ref first and refuses a definite non-branch (a tag would import fine but leave github_branch pointing at something get_branch() can never find); an unanswerable probe fails open, matching the branch box, which never validated either - Cap the codeload probe fallback at MAX_PROBE_REF_SEGMENTS candidate prefixes so an attacker-supplied deep URL cannot occupy a worker with hundreds of sequential HEAD requests during an API outage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efd109118c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if 'heads' in namespaces: | ||
| names += [b.name for b in repo.get_branches()] | ||
| if 'tags' in namespaces: | ||
| names += [t.name for t in repo.get_tags()] |
There was a problem hiding this comment.
For a slashed /tree/ or /blob/ import from an attacker-controlled repository with many branches or tags, this eagerly paginates every ref in both namespaces before resolving even a short remainder such as main/src. The capped codeload fallback never helps while the API is responding, so repeated imports can consume a Celery worker, substantial memory, and the user's GitHub API quota for thousands of requests; resolve only the bounded set of candidate prefixes or cap enumeration and fall back to bounded probes.
Useful? React with 👍 / 👎.
The slashed subdirectory add_remote test exercised the real branch_exists, which attempts a live GitHub API call on every run (it stayed green either way, but slow and by luck). branch_exists's docstring also claimed one request where it makes two. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Pasting (or deep-linking) any GitHub web URL into the importer now does what it says:
github.com/<user>/<repo>/tree/<branch>/<subdirectory>imports that project from that branch — including a subdirectory of a repository that contains several projects./blob/<ref>/<file>imports the file's directory;/commit/<sha>imports at that commit;refs/heads|tags/spellings,.gitsuffixes, percent-encoding, case-insensitive host, query strings/fragments all handled.HEAD) — the client used to hardcodemaster, which broke everymain-default repository.Why
The appstore's "Remix on CloudPebble" button generates links shaped
/ide/import/github/<user>/<repo>/tree/<branch>/<dir>. Today that prefillstree/main/…into the branch box, and a manually pasted tree-URL passes validation but silently imports the repository root from the default branch.Repositories holding many projects are the normal case now — most commonly a dev keeping all their faces in one repo, but also a library repo whose root is itself a Pebble project plus examples and store watchfaces in subdirectories. Without subdirectory imports, none of those projects is reachable by URL at all. Live motivating case: Sloth on the appstore →
github.com/emindeniz99/pebble-signals/tree/main/faces/slothvec(the unhinted importer picks the wrong manifest from that repo).How
ide/utils/github_urls.py— a dependency-free parser for GitHub source strings. The/tree/remainder is kept unsplit because branch names may contain slashes.root_hintforfind_project_root_and_manifest()— pins the project to the named subdirectory (allowing one wrapping folder, the<repo>-<ref>/GitHub-archive prefix). A bare suffix match is deliberately rejected so a wrong split can't luck into the right directory.<ref>/<path>against the repository's real branch+tag names, longest prefix first (the gitpick/gitingest approach; git's ref directory/file-conflict rule guarantees at most one branch and one tag can match, so the split is deterministic). When the API is unavailable it probes the strictcodeload…/zip/refs/heads/<candidate>endpoint — measured: barearchive/<x>.zipanswers 200 even for junk (archive/main/anything.zipservesmain), whilezip/refs/heads/main/facescorrectly 404s.add_remoteis rejected for subdirectory imports with a clear message (a linked push would target the repo root).Backwards compatibility
/ide/import/github/<user>/<repo>deep link/ide/import/github/<user>/<repo>/<branch>deep link (slashed branches included)github.com:user/repomaster-default repoHEAD) — andmain-default repos now work instead of failing…/repo/releases,/issues/12)tree/,blob/orcommit/(e.g. branchtree/x)Tests
test_github_urls(parser incl. encoded-delimiter and legacy-parity cases),test_find_project_root(hints incl. explicit-root and narrow-before-read),test_github_import(resolver with mocked refs/probe,/ide/import/githublinking flows, thedo_import_githubglue: HEAD default branch +root_hintper URL kind),test_import_archive(subtree limits),test_run_github_link(appstore-source → link translation).masterfallback can return).main/faces/slothvec → 404,main/faces → 404,main → 200; resolvedref=main, path=faces/slothvec; hinted root =pebble-signals-main/faces/slothvec/(displayName: Sloth, watchface: true) — while the unhinted (old) behavior picksexamples/consumer/./tree/<ref>/<dir>shape.Follow-ups deliberately out of scope
ide/static/ide/js/github.js+ its template) still hardcodes themasterfallback this PR removed from import — same bug class, separate feature; happy to fix in a follow-up PR.No new dependencies. Happy to split, rename or adjust anything — and if the appstore side would rather emit a different link shape, this PR makes CloudPebble accept the one it emits today.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com