From 34b863a5860b3e489a9ab56c71f2cb80ac9f1b96 Mon Sep 17 00:00:00 2001 From: Eder Ignatowicz Date: Fri, 28 Aug 2026 10:47:27 -0400 Subject: [PATCH 1/2] Stop the fork API addressing a repo name that doesn't exist clone_target.py carried its own copy of extract_slug that stripped the .git suffix with url.rstrip(".git"). rstrip takes a character set, not a suffix, so it kept eating trailing characters from {'.','g','i','t'} once .git was gone: rh-forge-ui became rh-forge-u. Every fork API call then addressed a repository that isn't there. GitHub answers 404 rather than 403 for a repo a token cannot see, so the symptom was indistinguishable from a credentials problem and RHAI-760 was diagnosed as a bad PAT for some time before the name was suspected. Latent since the copy was made. It only bites names ending in a character from that set, and no previous target had one -- odh-dashboard, kale, mlflow and codeflare-sdk all end outside it. Delete the copy rather than patch it, and point both call sites at github_utils.extract_slug, which already used .removesuffix(".git"). Two copies of one function are precisely what let the correct version sit next to the broken one without either fixing the other. Also file the follow-on: a clone fault marks the epic terminally Failed, so this 404 needed a hand-edit of the data repo to retry, where ADR-0025 would have left it Ready. Co-Authored-By: Claude Opus 5 --- .../open/bug-clone-fault-marks-epic-failed.md | 70 ++++++++++++++ ...bug-slug-extractor-truncated-repo-names.md | 91 +++++++++++++++++++ scripts/clone_target.py | 14 +-- tests/test_clone_target.py | 36 ++++---- tests/test_github_utils.py | 10 ++ 5 files changed, 190 insertions(+), 31 deletions(-) create mode 100644 docs/bugs/open/bug-clone-fault-marks-epic-failed.md create mode 100644 docs/bugs/open/bug-slug-extractor-truncated-repo-names.md diff --git a/docs/bugs/open/bug-clone-fault-marks-epic-failed.md b/docs/bugs/open/bug-clone-fault-marks-epic-failed.md new file mode 100644 index 0000000..c677171 --- /dev/null +++ b/docs/bugs/open/bug-clone-fault-marks-epic-failed.md @@ -0,0 +1,70 @@ +--- +id: bug-clone-fault-marks-epic-failed +title: A clone or credential fault marks the epic terminally Failed +type: bug +status: open +repos: [epic-code-gen] +decisions: [ADR-0025] +--- + +# Bug: A clone or credential fault marks the epic terminally Failed + +## Summary + +`setup_target_repo()` treats any non-zero exit from `clone_target.py` as an +epic failure and writes `status: Failed`. `Failed` is in `CI_TERMINAL_STATES`, +so the epic is skipped on every subsequent run and only a hand-edit of +`run-metadata.yaml` in the data repo brings it back. + +But a clone failure is an *environment* fault, not a property of the epic: +an expired or unauthorised token, a private repo, a transient GitHub 5xx, a +network blip. Nothing about the epic changed, and the next run — after the +variable is fixed — would succeed. + +This is the same distinction [ADR-0025] draws for missing tools, where the +toolchain preflight deliberately leaves status at `Ready` so the epic retries +once the image is fixed. Clone faults were never brought into line with it. + +## Reproduction + +Point an epic at a private repo whose token is wrong or absent, then run the +pipeline twice. + +## Expected + +Run 1 flags the epic and generates nothing; status stays `Ready`. Run 2, after +the credential is corrected, picks it up and proceeds. + +## Actual + +Run 1 sets `status: Failed` with a `failure_reason`. Run 2 skips the epic +because `Failed` is terminal. The epic is stuck until someone edits the data +repo by hand. + +Observed live on RHAISTRAT-2671: `RHAI-760` went `Ready → Failed` on a 404 +caused by [[bug-slug-extractor-truncated-repo-names]], and had to be reset with +a manual commit to the data repo before the fix could even be tested. + +## Impact + +Medium. It does not corrupt anything, but it converts every transient +infrastructure fault into manual data-repo surgery, and it does so silently — +the dashboard shows a red epic that looks like a codegen failure. + +## Proposed Fix + +Classify the clone failure the way preflight already classifies a missing tool: + +- Credential / not-found / network faults → leave `status: Ready`, record the + reason, generate nothing. Retryable by construction. +- Genuinely epic-caused faults (a `target_repo` that is malformed or absent + from `config/repo_mapping.json`) → `Failed`, since a re-run cannot help. + +Worth extracting the retryable-vs-terminal judgement into one helper shared +with the preflight gate, rather than a second ad-hoc copy of the rule. + +## Related + +- [[bug-slug-extractor-truncated-repo-names]] +- [[task-toolchain-preflight]] +- [[task-per-repo-github-identity]] diff --git a/docs/bugs/open/bug-slug-extractor-truncated-repo-names.md b/docs/bugs/open/bug-slug-extractor-truncated-repo-names.md new file mode 100644 index 0000000..ed23e37 --- /dev/null +++ b/docs/bugs/open/bug-slug-extractor-truncated-repo-names.md @@ -0,0 +1,91 @@ +--- +id: bug-slug-extractor-truncated-repo-names +title: Duplicate slug extractor truncated repo names ending in . g i t +type: bug +status: open +repos: [epic-code-gen] +decisions: [ADR-0030, ADR-0035] +--- + +# Bug: Duplicate slug extractor truncated repo names ending in `.` `g` `i` `t` + +## Summary + +`clone_target.py` carried its own `_extract_slug()`, a near-copy of +`github_utils.extract_slug()`. The copy stripped the `.git` suffix with +`url.rstrip(".git")`. + +`str.rstrip` takes a **character set**, not a suffix. It removes every trailing +character that appears in `{'.', 'g', 'i', 't'}`, so it does not stop once +`.git` is gone: + +``` +https://github.com/rh-forge/rh-forge-ui.git + → strip 't' 'i' 'g' '.' → rh-forge-ui (correct so far) + → strip 'i' → rh-forge-u (wrong; 'i' is in the set) +``` + +Every fork API call then addressed a repository that does not exist. + +## Reproduction + +```bash +python3 scripts/clone_target.py rh-forge/rh-forge-ui RHAI-760 --clean \ + --dest /tmp/tr --fork-owner ederign --gh-token-var RH_FORGE_GITHUB_TOKEN +``` + +## Expected + +Clone, add a `fork` remote for `ederign/rh-forge-ui`, create `epic/RHAI-760`. + +## Actual + +``` +HTTP 404: {"message":"Not Found", ... #get-a-repository} ← GET /repos/ederign/rh-forge-u +HTTP 404: {"message":"Not Found", ... #create-a-fork} ← POST /repos/rh-forge/rh-forge-u/forks +Error: HTTP Error 404: Not Found +``` + +`git clone` itself succeeded — the failure is entirely inside +`_setup_fork_remote()` → `github_utils.ensure_fork()`, which is why nothing in +the trace mentions git. GitHub answers 404 rather than 403 for a repository a +token cannot see, so the symptom reads exactly like a permissions problem: the +first hour of diagnosis went to the PAT (classic vs fine-grained, SSO +authorisation, org opt-in) and found nothing wrong with it. + +## Impact + +High, and latent since the extractor was duplicated. It fires only for repo +names whose last character is in `{'.', 'g', 'i', 't'}`, which no previous +target had — `odh-dashboard`, `kale`, `mlflow`, `codeflare-sdk` all end outside +the set. `rh-forge-ui` is the first target to end in `i`. Any future +`…-config`, `…-training` or `…-widget` target would have hit it too. + +Compounding it: `setup_target_repo()` records a clone failure as terminal +`Failed`, so both epics of RHAISTRAT-2671 had to be reset by hand in the data +repo before they could retry — see [[bug-clone-fault-marks-epic-failed]]. + +## Fix + +Deleted `clone_target._extract_slug` outright and pointed both call sites +(`_url_matches`, `_setup_fork_remote`) at `github_utils.extract_slug`, which +already used the correct `.removesuffix(".git")`. Deleting rather than patching +is the point: two copies of one function are what allowed the fix in +`github_utils` to never reach the copy that ran. + +Regression coverage: + +- `TestExtractSlug::test_repo_name_ending_in_git_suffix_chars` in + `tests/test_github_utils.py` — the character-set case directly. +- `TestSetupForkRemote::test_repo_name_ending_in_i_is_not_truncated` in + `tests/test_clone_target.py` — asserts the exact arguments reaching + `ensure_fork` for `rh-forge-ui`, pinning the call site that actually broke. + +The duplicated suite in `tests/test_clone_target.py` was removed along with the +function it covered. + +## Related + +- [[task-per-repo-github-identity]] — the change that first pointed a target at + `rh-forge/rh-forge-ui` and exposed this. +- [[bug-clone-fault-marks-epic-failed]] diff --git a/scripts/clone_target.py b/scripts/clone_target.py index bb06c81..1fb416d 100644 --- a/scripts/clone_target.py +++ b/scripts/clone_target.py @@ -128,22 +128,12 @@ def clone(repo_url, epic_id, dest=None, fork_owner=None, clean=False, def _url_matches(url, remote_output): """Check if url matches any remote URL (handles https vs git@ variants).""" - slug = _extract_slug(url) + slug = github_utils.extract_slug(url) if slug: return slug in remote_output return False -def _extract_slug(url): - """Extract org/repo from a GitHub URL.""" - url = url.rstrip("/").rstrip(".git") - if "github.com" in url: - parts = url.split("github.com")[-1].strip("/:").split("/") - if len(parts) >= 2: - return f"{parts[0]}/{parts[1]}" - return None - - def _configure_git_identity(dest, token): """Set git user.name and user.email from the GitHub token owner.""" user = github_utils.get_authenticated_user(token) @@ -263,7 +253,7 @@ def _setup_fork_remote(dest, upstream_url, fork_owner, token=None): Returns dict with: fork_url (display URL), fork_created. """ - slug = _extract_slug(upstream_url) + slug = github_utils.extract_slug(upstream_url) if not slug: return {"fork_url": None, "fork_created": False} diff --git a/tests/test_clone_target.py b/tests/test_clone_target.py index 70fc0aa..4c4f55b 100644 --- a/tests/test_clone_target.py +++ b/tests/test_clone_target.py @@ -12,7 +12,6 @@ from clone_target import ( clone, - _extract_slug, _url_matches, _ensure_branch, _setup_fork_remote, @@ -50,24 +49,6 @@ def _init_repo(path): # ─── URL Utilities ─────────────────────────────────────────────────────────── -class TestExtractSlug: - - def test_https_url(self): - assert _extract_slug("https://github.com/org/repo") == "org/repo" - - def test_https_with_git_suffix(self): - assert _extract_slug("https://github.com/org/repo.git") == "org/repo" - - def test_ssh_url(self): - assert _extract_slug("git@github.com:org/repo.git") == "org/repo" - - def test_trailing_slash(self): - assert _extract_slug("https://github.com/org/repo/") == "org/repo" - - def test_non_github(self): - assert _extract_slug("https://gitlab.com/org/repo") is None - - class TestUrlMatches: def test_matches_https(self): @@ -291,6 +272,23 @@ def test_with_token_uses_authenticated_url(self, tmp_path): ) assert "x-access-token" in out.stdout + def test_repo_name_ending_in_i_is_not_truncated(self, tmp_path): + """Regression: rh-forge-ui.git must not become rh-forge-u. + + The old module-local slug extractor used rstrip(".git"), so every + fork API call for this repo hit a name that doesn't exist and 404'd. + """ + repo = _init_repo(tmp_path / "repo") + with patch("github_utils.ensure_fork", + return_value=("ederign/rh-forge-ui", False)) as ensure_fork: + result = _setup_fork_remote( + repo, "https://github.com/rh-forge/rh-forge-ui.git", "ederign", + token="ghp_test123") + + ensure_fork.assert_called_once_with( + "rh-forge", "rh-forge-ui", "ederign", "ghp_test123") + assert result["fork_url"] == "https://github.com/ederign/rh-forge-ui.git" + def test_with_token_creates_fork(self, tmp_path): repo = _init_repo(tmp_path / "repo") with patch("github_utils.ensure_fork", return_value=("newuser/myrepo", True)): diff --git a/tests/test_github_utils.py b/tests/test_github_utils.py index 64dd0dc..fbc5cba 100644 --- a/tests/test_github_utils.py +++ b/tests/test_github_utils.py @@ -41,6 +41,16 @@ def test_non_github(self): def test_no_repo(self): assert extract_slug("https://github.com/org") is None + def test_repo_name_ending_in_git_suffix_chars(self): + """`.git` must be stripped as a suffix, not as a character set. + + rstrip(".git") eats any trailing '.', 'g', 'i' or 't', which turned + rh-forge-ui into rh-forge-u and 404'd every fork API call. + """ + for name in ("rh-forge-ui", "some-widget", "config", "kubeflow-training"): + url = f"https://github.com/org/{name}.git" + assert extract_slug(url) == f"org/{name}" + def test_empty_string(self): assert extract_slug("") is None From 726849f13f282cb04fb4090cf8a9044ce8354a70 Mon Sep 17 00:00:00 2001 From: Eder Ignatowicz Date: Fri, 28 Aug 2026 10:47:35 -0400 Subject: [PATCH 2/2] Record the slug bug as fixed with its commit Co-Authored-By: Claude Opus 5 --- .../{open => fixed}/bug-slug-extractor-truncated-repo-names.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename docs/bugs/{open => fixed}/bug-slug-extractor-truncated-repo-names.md (99%) diff --git a/docs/bugs/open/bug-slug-extractor-truncated-repo-names.md b/docs/bugs/fixed/bug-slug-extractor-truncated-repo-names.md similarity index 99% rename from docs/bugs/open/bug-slug-extractor-truncated-repo-names.md rename to docs/bugs/fixed/bug-slug-extractor-truncated-repo-names.md index ed23e37..31c14e2 100644 --- a/docs/bugs/open/bug-slug-extractor-truncated-repo-names.md +++ b/docs/bugs/fixed/bug-slug-extractor-truncated-repo-names.md @@ -2,7 +2,8 @@ id: bug-slug-extractor-truncated-repo-names title: Duplicate slug extractor truncated repo names ending in . g i t type: bug -status: open +status: fixed +commits: ["34b863a"] repos: [epic-code-gen] decisions: [ADR-0030, ADR-0035] ---