denis, orbit: fix silently skipped deadline checks and allow correcting full names - #311
Merged
Merged
Conversation
charliemirabile
requested changes
Sep 11, 2026
charliemirabile
left a comment
charliemirabile
left a comment
Contributor
There was a problem hiding this comment.
Could you add Fixes: tags pointing to the relevant commits for the first two since they are bug fixes? Otherwise lgtm
run_automated_checks() fetches refs/notes/* through the grading remote,
which points at PUSH_URL. That URL is served by the CGI support in
python's http.server, which does not pass the Content-Encoding request
header through to git http-backend. Once the local clone holds enough
refs, the upload-pack negotiation request grows past the size at which
git gzips it, and http-backend parses the gzip stream as pkt-lines:
fatal: protocol error: bad line length character: ?\x8b?
The client sees "the remote end hung up unexpectedly" and denis crashes
before writing any notes.
The first deadline of an assignment is unaffected because no notes
exist yet, so there is nothing to negotiate. Every later deadline
crashes, which silently drops the automated checks and the automatic
zeros for missing submissions. That is what happened at the final
deadline of the first assignment this semester, and the peer review
deadline before it left no notes either.
Fetch the notes through origin instead. The clone already uses
PULL_URL, which is served as static files and takes no request body.
Pushing is unchanged.
Fixes: 22c9694 ("denis: add notes tied to initial and final submissions")
Signed-off-by: samuel-marquis <smarquis@protonmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
peer_review.py passes run_automated_checks() the union of the review1
and review2 username-to-submission dicts. user_to_sub() returns every
user as a key, with None for users who did not submit, so the review2
dict always wins the union. check_corrupt_or_missing() looks up each
tag's user without regard to the tag's component, so review1 tags are
judged by review2 submissions:
- a student who submitted review1 but not review2 also gets an
automatic zero on review1
- a student who submitted only review2 gets review1 credited
Run the checks once per component, each with its own dict. The second
run fetches the notes pushed by the first, so this relies on the
previous patch.
Fixes: b0537c7 ("denis: automatic zero for peer review when no submission is made")
Signed-off-by: samuel-marquis <smarquis@protonmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A user's full name can only be set when the account is created. denis
uses it to build the Signed-off-by line its DCO check expects, and
orbit uses it for the name in the mutt and git configuration it
generates for each student. A misspelled name therefore makes every
correctly signed patch from that student fail the check, with no way
to fix it short of deleting and recreating the account.
Add -e/--editfullname, which sets the full name of the supplied
username:
orbit/warpdrive.sh -u <username> -f '<full name>' -e
Configuration a student already generated keeps the old name until
they fetch it again.
Signed-off-by: samuel-marquis <smarquis@protonmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The roster is the only way to list users from the admin tool, but it leaves out full names, so there is no way to check a spelling before or after correcting it with -e. Append the full name to each line. Signed-off-by: samuel-marquis <smarquis@protonmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
samuelmarquis
force-pushed
the
denis_orbit_fixes
branch
from
September 11, 2026 17:32
d3e0694 to
21b368d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Grading the first assignment of this semester turned up two bugs in the deadline automation and a gap in the admin tooling.
denis: fetch notes from the pull URL.
run_automated_checks()fetches notes through the push URL, and the CGI support in python'shttp.serverthat serves it does not passContent-Encodingthrough togit http-backend. From a clone with enough refs, git gzips the negotiation request, and http-backend misreads it as pkt-lines:The crash comes before any notes are written, so the DCO and subject tag checks and the automatic zeros for missing submissions were skipped at the peer review and final deadlines, and nothing reported it. An assignment's first deadline is unaffected because no notes exist yet.
denis: check each peer review against its own submissions.
peer_review.pychecked review1 tags against the review2 submissions: every user is a key in both dicts, so the union keeps review2's value. A student who did only review1 would get an automatic zero on it, and one who did only review2 would get review1 credited. It hasn't bitten yet only because the crash above came first.orbit: hyperspace: correct and show full names. A full name could only be set when the account is created, and the roster didn't show it. A misspelled name makes denis flag every correctly signed patch from that student. This adds
-e/--editfullname(orbit/warpdrive.sh -u <username> -f '<full name>' -e) and appends the full name to each roster line.Testing
run_automated_checks()against a localpython -m http.server --cgireplica of the git service (same CGI script, hook, and repo config), serving a mirror of this semester's grading repo with 220 tags. Before the patch it fails with the same error production logged; after it, the notes are written and pushed and the existing notes are kept. The replica ran python 3.9 on macOS; production runs alpine's python 3.12.peer_review.pywith its I/O stubbed, one student submitting only review1 and another only review2. Before the patch the wrong two tags get automatic zeros; after it, the right two.Not addressed
Content-Encoding, andCGIHTTPRequestHandleris deprecated upstream. After this PR nothing fetches through that URL, but replacing the server would remove the trap entirely.run_automated_checks()on the existing tags. Re-runningfinal.pyorpeer_review.pywhole would release the submissions to the journal a second time.🤖 Generated with Claude Code