crueber/walhub

Watch

Fork 0

Import: SPA canonical-URL hint diverges from server normalization on .git/trailing-slash/uppercase non-GitHub URLs #401

New issue

Closed

opened 2026-09-12 17:52:02 +00:00 by crueber · 3 comments

crueber commented

2026-09-12 17:52:02 +00:00

Owner

Import: SPA canonical-URL hint diverges from server normalization on `.git`/trailing-slash/uppercase non-GitHub URLs

Refiles #398 (closed, re-filed) — the earlier ticket was filed inline in violation of the Issue Writer contract and closed in favor of this one. Investigation for this re-file found that the server-side bug #398 described no longer reproduces; what remains is a client-side hint inconsistency. Scope of this ticket is therefore the SPA, not internal/repoimport.

What's requested

The import form's "canonical:" hint (and the prefill of owner/name for generic forges) must match the server's normalization contract so the user sees the same canonical source the server will clone. Today it doesn't for non-GitHub URLs.

Evidence

Server side — `.git` handling is correct and pinned (the #398 report does not reproduce)

internal/repoimport/url.go:

Header contract (url.go:4-13): shorthand and full GitHub URLs normalize to https://github.com/<owner>/<repo>.git; everything else is rebuilt as scheme://host/path with host lowercased, default ports stripped, trailing slashes trimmed, and one trailing .git removed — so :443/uppercase/trailing-slash/.git/ variants of one source collapse to a single canonical string. "The gated string IS the cloned string" — CloneMirror receives Normalized.URL (task.go:77 passes params.Source.URL).
canonicalGenericURL (url.go:196-208) implements exactly that: strings.TrimRight(cu.Path, "/") then strings.TrimSuffix(p, ".git") (url.go:204-205).

Tests prove the behavior end to end (all pass, go test ./internal/repoimport/ -count=1 → ok):

issue237_test.go:27-35 — the variant table includes https://git.packden.us/crueber/dotfiles.git, …/dotfiles/, …/dotfiles.git/, and uppercase host; all must collapse to the canonical https://git.packden.us/crueber/dotfiles (TestNormalizeCanonical237, issue237_test.go:41).
TestCloneReceivesCanonicalURL237 (issue237_test.go:355) proves through the real Begin → drive → CloneMirror path that the clone argv consumes the gated canonical URL — the exact "normalization survives to the clone args" check #398 asked for.
TestVariantGateParityHTTP237 proves one gate decision per logical source.

A https://git.example.com/owner/repo.git-shaped input lands on the same code path (NormalizeSource → canonicalGenericURL); there is no branch that carries a non-GitHub URL verbatim into the clone.

Client side — the hint does not follow the contract (this ticket)

web/sdk/src/import.js normalizeSource (import.js:20-41):

GitHub shorthand and GitHub full URLs are rewritten to canonical .git form (import.js:23-35).
Everything else is returned verbatim (return { url: s, kind: … }, import.js:40): no .git strip, no trailing-slash trim, no case folding, no default-port strip.
The import form shows this raw string as "canonical:" under the source field (web/src/pages/Import.jsx:73, 235-237, suggestion().url) and prefills owner/name from it (Import.jsx:227).

So a user pasting https://git.example.com/owner/repo.git sees a "canonical" hint with the suffix (and slash/port/case variants produce five different hints), while the server normalizes all of them to one form. The label "canonical" is a lie on the generic path. For mirrored/POST /api/v1/repos/mirrors usage the same SDK helper is the client's only normalization.

Architecture notes

normalizeSource is documented as client-side prefill only ("the server re-validates everything") — the fix is to make the client mirror the server's canonicalGenericURL rules for the hint (strip one trailing .git after trimming trailing slashes, lowercase host, strip default ports — or simply label the hint as a preview and defer canonicalization to the server response), not to move trust client-side. The server gate and clone URL are already correct and must not change.
The mirrored contract to implement against is the header comment in internal/repoimport/url.go:4-13 and the pinned variants in issue237_test.go:27-35.
Prefer extracting the shared rules so the SPA hint and the server can't drift again; if duplication is kept (JS/Go split), pin it with a comment on both sides naming the other.

Acceptance criteria

normalizeSource in web/sdk/src/import.js returns the same canonical URL the server's NormalizeSource produces for every variant row in issue237_test.go:27-35 (plus the plain .git form), for non-GitHub https hosts.
The import form's "canonical:" hint (Import.jsx:235-237) and the owner/name prefill reflect the stripped/lowercased form for https://git.example.com/owner/repo.git-shaped input.
GitHub-path behavior of normalizeSource is unchanged (still suggests https://github.com/<owner>/<repo>.git).
No change to internal/repoimport server behavior; existing repoimport tests stay green.
A comment in import.js names the server contract it mirrors (url.go canonical rules) so the two sides are cross-referenced.

# Import: SPA canonical-URL hint diverges from server normalization on `.git`/trailing-slash/uppercase non-GitHub URLs Refiles **#398 (closed, re-filed)** — the earlier ticket was filed inline in violation of the Issue Writer contract and closed in favor of this one. Investigation for this re-file found that the server-side bug #398 described **no longer reproduces**; what remains is a client-side hint inconsistency. Scope of this ticket is therefore the SPA, not `internal/repoimport`. ## What's requested The import form's "canonical:" hint (and the prefill of owner/name for generic forges) must match the server's normalization contract so the user sees the same canonical source the server will clone. Today it doesn't for non-GitHub URLs. ## Evidence ### Server side — `.git` handling is correct and pinned (the #398 report does not reproduce) `internal/repoimport/url.go`: - Header contract (url.go:4-13): shorthand and full GitHub URLs normalize to `https://github.com/<owner>/<repo>.git`; everything else is rebuilt as `scheme://host/path` with host lowercased, default ports stripped, trailing slashes trimmed, and **one trailing `.git` removed** — so `:443`/uppercase/trailing-slash/`.git/` variants of one source collapse to a single canonical string. "The gated string IS the cloned string" — `CloneMirror` receives `Normalized.URL` (task.go:77 passes `params.Source.URL`). - `canonicalGenericURL` (url.go:196-208) implements exactly that: `strings.TrimRight(cu.Path, "/")` then `strings.TrimSuffix(p, ".git")` (url.go:204-205). Tests prove the behavior end to end (all pass, `go test ./internal/repoimport/ -count=1` → ok): - `issue237_test.go:27-35` — the variant table includes `https://git.packden.us/crueber/dotfiles.git`, `…/dotfiles/`, `…/dotfiles.git/`, and uppercase host; all must collapse to the canonical `https://git.packden.us/crueber/dotfiles` (`TestNormalizeCanonical237`, issue237_test.go:41). - `TestCloneReceivesCanonicalURL237` (issue237_test.go:355) proves through the real Begin → drive → CloneMirror path that the clone argv consumes the gated canonical URL — the exact "normalization survives to the clone args" check #398 asked for. - `TestVariantGateParityHTTP237` proves one gate decision per logical source. A `https://git.example.com/owner/repo.git`-shaped input lands on the same code path (`NormalizeSource` → `canonicalGenericURL`); there is no branch that carries a non-GitHub URL verbatim into the clone. ### Client side — the hint does not follow the contract (this ticket) `web/sdk/src/import.js` `normalizeSource` (import.js:20-41): - GitHub shorthand and GitHub full URLs are rewritten to canonical `.git` form (import.js:23-35). - **Everything else is returned verbatim** (`return { url: s, kind: … }`, import.js:40): no `.git` strip, no trailing-slash trim, no case folding, no default-port strip. - The import form shows this raw string as "canonical:" under the source field (`web/src/pages/Import.jsx:73, 235-237`, `suggestion().url`) and prefills owner/name from it (Import.jsx:227). So a user pasting `https://git.example.com/owner/repo.git` sees a "canonical" hint with the suffix (and slash/port/case variants produce five different hints), while the server normalizes all of them to one form. The label "canonical" is a lie on the generic path. For mirrored/POST `/api/v1/repos/mirrors` usage the same SDK helper is the client's only normalization. ## Architecture notes - `normalizeSource` is documented as client-side prefill only ("the server re-validates everything") — the fix is to make the client mirror the server's `canonicalGenericURL` rules for the hint (strip one trailing `.git` after trimming trailing slashes, lowercase host, strip default ports — or simply label the hint as a preview and defer canonicalization to the server response), not to move trust client-side. The server gate and clone URL are already correct and must not change. - The mirrored contract to implement against is the header comment in `internal/repoimport/url.go:4-13` and the pinned variants in `issue237_test.go:27-35`. - Prefer extracting the shared rules so the SPA hint and the server can't drift again; if duplication is kept (JS/Go split), pin it with a comment on both sides naming the other. ## Acceptance criteria - [ ] `normalizeSource` in `web/sdk/src/import.js` returns the same canonical URL the server's `NormalizeSource` produces for every variant row in `issue237_test.go:27-35` (plus the plain `.git` form), for non-GitHub https hosts. - [ ] The import form's "canonical:" hint (Import.jsx:235-237) and the owner/name prefill reflect the stripped/lowercased form for `https://git.example.com/owner/repo.git`-shaped input. - [ ] GitHub-path behavior of `normalizeSource` is unchanged (still suggests `https://github.com/<owner>/<repo>.git`). - [ ] No change to `internal/repoimport` server behavior; existing repoimport tests stay green. - [ ] A comment in `import.js` names the server contract it mirrors (url.go canonical rules) so the two sides are cross-referenced.

crueber added the

git storage

label

2026-09-12 17:52:07 +00:00

crueber added this to the v1 milestone

2026-09-12 17:52:07 +00:00

crueber referenced this issue from a commit

2026-09-12 19:04:00 +00:00

Fix #401: SPA import hint mirrors server canonicalGenericURL

crueber referenced this issue from a pull request that will close it,

2026-09-12 19:04:02 +00:00

Fix #401: import hint parity #406

crueber commented

2026-09-12 19:04:18 +00:00

Author

Owner

Fixed by #406 (#406): SPA-only change — normalizeSource mirrors canonicalGenericURL for non-GitHub URLs (hint + prefill now show the server canonical form). Server untouched; repoimport tests green.

Fixed by #406 (https://git.packden.us/crueber/walhub/pulls/406): SPA-only change — normalizeSource mirrors canonicalGenericURL for non-GitHub URLs (hint + prefill now show the server canonical form). Server untouched; repoimport tests green.

crueber commented

2026-09-12 19:10:11 +00:00

Author

Owner

Review: PR #406 (fix/issue-401) — verified in scratch worktrees, main untouched

MERGE RECOMMENDATION: ready to merge. No fixes pushed (nothing needed fixing); no structural issues. All verification done in scratch worktrees (origin/fix/issue-401 vs pristine origin/main), both removed afterward. Main worktree left clean. No browser used — not needed (change is pure JS normalization logic + headless tests; verified with node tests + reasoning, as instructed). No docker, no live instances touched.

1. Mirror fidelity — canonicalGenericHint matches canonicalGenericURL rule-for-rule

web/sdk/src/import.js vs internal/repoimport/url.go:196-208: host lowercase ✓, default-port set {443/80/22/9418} identical to Go isDefaultPort ✓, trailing-slash trim (/\ youthful+$/ ≡ TrimRight(p,"/")) ✓, exactly ONE .git strip (case-sensitive on both sides) ✓, query/fragment preserved verbatim ✓, path case preserved ✓, IPv6 bracket wrap ✓, userinfo / non-default port / unsupported scheme / unparseable → verbatim for the server to 400 ✓. Edge probes (node, against Go semantics): r.git.git→r.git, r.GIT kept, ?x=1#frag preserved, o//r// interior doubles kept, https://…:443/…/ folds, HTTPS://HOST/… lowercases host only — all identical to the server. Two degenerate-input observations, both non-blocking: (a) https:///o/r.git — WHATWG URL parses host o where Go sees empty host → JS canonicalizes something the server 400s (unfixable parser difference, garbage-in); (b) ssh:///git:// URLs are canonicalized in the hint though ValidateTransport later 400s them — this still matches the mirrored NormalizeSource contract, so it is correct per the issue's stated mirror target.

2. Test oracle — exact rows pinned

sdk-import.test.js pins all 5 issue237_test.go:27-35 variants + the bare canonical form (6 rows → one canonical URL, kind/owner/name asserted), the http twin + :80 fold, GitHub .git-form unchanged, and the two verbatim-refusal rows (non-default :8443, embedded user:token) — the refusal set matches the server 400s (url.go:93-94, 108-110). sdk-import.test.js: 8/8 pass.

3. Owner/name prefill — Import.jsx untouched, claim holds

Diff touches exactly 3 files (10_git_import.md, sdk/import.js, sdk-import.test.js). Import.jsx:227-229 (prefill) and :235-237 (hint) both consume normalizeSource(), so the SDK fix corrects both with zero UI changes.

4-5. GitHub path byte-identical; server untouched

GitHub shorthand/regex branches are untouched lines; old assertions pass unchanged. Zero .go files in the diff; go test ./internal/repoimport/ -count=1 → ok (26s).

6. Full-suite failures — byte-identical on pristine main, environmental

PR branch: node 797 tests / 795 pass / 2 fail; pristine main: 796 / 794 / same 2 fail — delta is exactly the +1 new passing test. Both failures are smoke.test.js (needs a live dev server on :8080; this env answers 401 from a foreign instance — environmental, untouched by this change, nowhere near import/markdown paths). vite build + esbuild green on both. Go fast tier (go test -short -count=1 ./...) fully green on the PR branch. One count discrepancy to note: the PR description says "13 pre-existing failures" but I reproduce only these 2 (node) + 0 (Go) in my scratch — the failures I do see are all environmental and identical on main, so this changes nothing about the verdict, but the description number looks stale.

7. Contract comment, docs, deps

import.js header + canonicalGenericHint docstring both name internal/repoimport/url.go:196-208 and header contract url.go:4-13 ✓ (one-sided cross-ref is correct — server intentionally untouched per acceptance criterion 4). Law-12 decision appended under docs/features/10_git_import.md ## Decisions, accurate (verbatim-refusal set, GitHub unchanged, #398 non-repro stated) ✓. No package.json change, no new imports (pure JS + new URL) — Law 1 holds ✓.

## Review: PR #406 (fix/issue-401) — verified in scratch worktrees, main untouched **MERGE RECOMMENDATION: ready to merge.** No fixes pushed (nothing needed fixing); no structural issues. All verification done in scratch worktrees (`origin/fix/issue-401` vs pristine `origin/main`), both removed afterward. Main worktree left clean. No browser used — not needed (change is pure JS normalization logic + headless tests; verified with node tests + reasoning, as instructed). No docker, no live instances touched. ### 1. Mirror fidelity — canonicalGenericHint matches canonicalGenericURL rule-for-rule `web/sdk/src/import.js` vs `internal/repoimport/url.go:196-208`: host lowercase ✓, default-port set `{443/80/22/9418}` identical to Go `isDefaultPort` ✓, trailing-slash trim (`/\ youthful+$/` ≡ `TrimRight(p,"/")`) ✓, exactly ONE `.git` strip (case-sensitive on both sides) ✓, query/fragment preserved verbatim ✓, path case preserved ✓, IPv6 bracket wrap ✓, userinfo / non-default port / unsupported scheme / unparseable → verbatim for the server to 400 ✓. Edge probes (node, against Go semantics): `r.git.git`→`r.git`, `r.GIT` kept, `?x=1#frag` preserved, `o//r//` interior doubles kept, `https://…:443/…/` folds, `HTTPS://HOST/…` lowercases host only — all identical to the server. Two degenerate-input observations, both non-blocking: (a) `https:///o/r.git` — WHATWG URL parses host `o` where Go sees empty host → JS canonicalizes something the server 400s (unfixable parser difference, garbage-in); (b) `ssh://`/`git://` URLs are canonicalized in the hint though `ValidateTransport` later 400s them — this still matches the mirrored `NormalizeSource` contract, so it is correct per the issue's stated mirror target. ### 2. Test oracle — exact rows pinned `sdk-import.test.js` pins all 5 `issue237_test.go:27-35` variants + the bare canonical form (6 rows → one canonical URL, kind/owner/name asserted), the http twin + `:80` fold, GitHub `.git`-form unchanged, and the two verbatim-refusal rows (non-default `:8443`, embedded `user:token`) — the refusal set matches the server 400s (url.go:93-94, 108-110). `sdk-import.test.js`: 8/8 pass. ### 3. Owner/name prefill — Import.jsx untouched, claim holds Diff touches exactly 3 files (`10_git_import.md`, `sdk/import.js`, `sdk-import.test.js`). `Import.jsx:227-229` (prefill) and `:235-237` (hint) both consume `normalizeSource()`, so the SDK fix corrects both with zero UI changes. ### 4-5. GitHub path byte-identical; server untouched GitHub shorthand/regex branches are untouched lines; old assertions pass unchanged. Zero `.go` files in the diff; `go test ./internal/repoimport/ -count=1` → ok (26s). ### 6. Full-suite failures — byte-identical on pristine main, environmental PR branch: node 797 tests / 795 pass / 2 fail; pristine main: 796 / 794 / same 2 fail — delta is exactly the +1 new passing test. Both failures are `smoke.test.js` (needs a live dev server on :8080; this env answers 401 from a foreign instance — environmental, untouched by this change, nowhere near import/markdown paths). `vite build` + `esbuild` green on both. Go fast tier (`go test -short -count=1 ./...`) fully green on the PR branch. One count discrepancy to note: the PR description says "13 pre-existing failures" but I reproduce only these 2 (node) + 0 (Go) in my scratch — the failures I do see are all environmental and identical on main, so this changes nothing about the verdict, but the description number looks stale. ### 7. Contract comment, docs, deps `import.js` header + `canonicalGenericHint` docstring both name `internal/repoimport/url.go:196-208` and header contract `url.go:4-13` ✓ (one-sided cross-ref is correct — server intentionally untouched per acceptance criterion 4). Law-12 decision appended under `docs/features/10_git_import.md ## Decisions`, accurate (verbatim-refusal set, GitHub unchanged, #398 non-repro stated) ✓. No `package.json` change, no new imports (pure JS + `new URL`) — Law 1 holds ✓.

crueber closed this issue

2026-09-12 19:10:31 +00:00

crueber commented

2026-09-12 19:10:31 +00:00

Author

Owner

Fixed by PR #406 (review clean — rule-for-rule mirror fidelity incl. edge probes, oracle pinned, server untouched), merged. Closing.

crueber referenced this issue from a commit

2026-09-12 19:10:32 +00:00

Merge pull request 'Fix #401: import hint parity' (#406) from fix/issue-401 into main