From 52c1312b227d5dd4ab47c9cee36abeae5e15ef51 Mon Sep 17 00:00:00 2001 From: Anthony Giacalone Date: Sun, 28 Jun 2026 16:33:04 -0700 Subject: [PATCH] fix(feedback_deliver): skip un-clonable repo instead of aborting the run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If `gh repo clone` failed (a non-submission whose repo was never created), `dest` didn't exist and the next `open(dest/FEEDBACK.md, "w")` raised FileNotFoundError, aborting the ENTIRE cohort delivery mid-run — every repo after the missing one was silently left undelivered. Guard on the clone + checkout-feedback exit codes: record the repo as `main_state="no-repo"` and continue. One missing repo no longer halts the cohort. Regression test: test_execute_skips_unclonable_repo_and_continues. Full suite green (399 passed). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_015cY2osrcunEXb5a8xtTy2V --- lectern/feedback_deliver.py | 16 ++++++++++++++-- tests/test_feedback_deliver.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lectern/feedback_deliver.py b/lectern/feedback_deliver.py index c36d5a3..81abbac 100644 --- a/lectern/feedback_deliver.py +++ b/lectern/feedback_deliver.py @@ -132,8 +132,20 @@ def deliver(cohort, manifest, workdir, *, execute=False, close=True, merge_main= pr_state, main_state = "-", "-" if execute: dest = os.path.join(workdir, gid) - gh("repo", "clone", repo, dest) # full clone: both branches present - git("-C", dest, "checkout", manifest.feedback_branch) + cl = gh("repo", "clone", repo, dest) # full clone: both branches present + co = git("-C", dest, "checkout", manifest.feedback_branch) + if getattr(cl, "returncode", 0) != 0 or getattr(co, "returncode", 0) != 0: + # repo absent / empty / no feedback branch — typically a + # non-submission that never accepted the assignment. Record and + # skip; never abort the whole cohort run on one missing repo. + entries.append({"github_id": gid, "student": r["student"], + "auto": r.get("points"), "writeup": r.get("writeup_score"), + "total": total, "grand": r.get("grand"), + "components": r.get("components"), + "student_comment": r.get("student_comment") or r.get("comment", ""), + "posted": False, "signed": False, + "pr_state": "-", "main_state": "no-repo"}) + continue fb = os.path.join(dest, "FEEDBACK.md") existing = open(fb).read() if os.path.exists(fb) else None if existing == md: diff --git a/tests/test_feedback_deliver.py b/tests/test_feedback_deliver.py index 7b64f3e..01c4beb 100644 --- a/tests/test_feedback_deliver.py +++ b/tests/test_feedback_deliver.py @@ -204,3 +204,22 @@ def test_default_gh_git_callbacks_prepend_binary(monkeypatch): sig = inspect.signature(fd.deliver) assert sig.parameters["gh"].default is fd._gh, "deliver default gh must wrap the gh binary" assert sig.parameters["git"].default is fd._git, "deliver default git must wrap the git binary" + + +def test_execute_skips_unclonable_repo_and_continues(tmp_path): + # A non-submission repo that doesn't exist: gh clone returns non-zero (and no + # dest dir). deliver must record it as no-repo and CONTINUE to the next repo, + # not abort the whole cohort run (the crash hit live on the Lab 2 delivery). + import os as _os + _os.makedirs(tmp_path / "skyle") + def gh(*a, **k): + if a[:2] == ("repo", "clone") and str(a[2]).endswith("-bwayne"): + return _R("Could not resolve to a Repository", 1) + return _R("", 0) + gcalls = [] + git = make_git(gcalls) + entries = deliver([row(), row(github_id="skyle", student="Selina Kyle")], + M, str(tmp_path), execute=True, gh=gh, git=git) + by = {e["github_id"]: e for e in entries} + assert by["bwayne"]["main_state"] == "no-repo" and by["bwayne"]["posted"] is False + assert by["skyle"]["posted"] is True # run continued; next repo delivered