Skip to content

fix(git): complete protocol v2 object-info support - #48

Merged
yordis merged 2 commits into
mainfrom
yordis/fix-git-object-info
Oct 29, 2026
Merged

yordis merged 2 commits into
mainfrom
yordis/fix-git-object-info

Conversation

@yordis

@yordis yordis commented Oct 29, 2026

Copy link
Copy Markdown
Member
  • Git clients could not use object-info: the capability omitted its attribute, missing objects used an invalid sentinel, requests were unbounded, and hosts excluded from serving still started object syncs.
  • Brings in Complete protocol v2 object-info support tobi/walgit#88 by behinddwalls, cherry-picked with original authorship; the upstream test fails on our main and passes with this change.

## Summary

### Why?

The object-info implementation was not compatible with the Git client: its capability omitted the supported attribute, missing objects used an invalid numeric sentinel, requests were unbounded, and excluded hosts could start object synchronization.

### What?

Advertise `object-info=size`, return protocol-defined empty values for missing objects, cap requests at 1,024 OIDs before sync, and apply the existing placement refusal before any object work. Preserve manifest-versioned positive and negative caching across local and remote-served object access.

The healthy request path remains one conditional manifest GET. Warm cache hits and local header lookups add zero bucket reads; remote misses add only the pack range reads required by `RemotePacks::header`.

## Test Plan

✅ `cargo check -p walgit-server`

✅ `cargo test -p walgit-git read_object_info_args_reads_size_and_oids_after_delimiter`

✅ `cargo test -p walgit-server --test e2e info_refs_v2_advertises_capabilities -- --exact`

✅ `cargo test -p walgit-server --test e2e object_info_v2`

✅ `cargo test -p walgit-server --test maintain host_excluded_from_serving_a_repo_refuses_object_work_with_503 -- --exact`

✅ `cargo test -p walgit-server --test e2e fetch_from_front_that_serves_the_base_remotely -- --exact`

✅ `cargo clippy -p walgit-server --all-targets -- -D warnings`

✅ `cargo fmt --all -- --check`

✅ `git diff --check`

## Issue

Closes tobi#80

(cherry picked from commit c51b778)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches upload-pack object sync and remote header reads on a new client-facing path, but bounds work (size-only, 1024 OIDs, placement checks) and version-scoped caching limit blast radius.

Overview
Completes Git protocol v2 object-info so clients can query object sizes without a full fetch.

Capability and request parsing: v2 upload-pack now advertises object-info=size (not a bare object-info line). Post-delimiter args are read via read_object_info_args, including the size attribute and oid lines.

Handler behavior in smart.rs: Requests without size are rejected; more than 1,024 OIDs return an ERR pkt-line before object sync. Wrong-host placement uses the same not_served_here path as fetch (503 + Retry-After, no materialization). After sync_objects, sizes come from local object headers or remote pack header reads, with results cached in a new ObjectInfoCache keyed by repo, manifest version, and OID. Missing objects use an empty size field instead of -1.

Tests and docs: E2E and maintain tests cover happy path, missing OIDs, OID limits, remote-served repos, and placement refusal; ROUNDTRIPS.md documents store read behavior for object-info.

Reviewed by Cursor Bugbot for commit 5e62aba. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c31359d1-7cdf-4e66-9ee1-87964d35d9dd
Commits

Reviewing files that changed from the base of the PR and between 2668870 and 5e62aba.

Files selected for processing: 6
  • crates/walgit-git/src/pkt.rs
  • crates/walgit-server/src/cache.rs
  • crates/walgit-server/src/smart.rs
  • crates/walgit-server/tests/e2e.rs
  • crates/walgit-server/tests/maintain.rs
  • docs/ROUNDTRIPS.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e06299c. Configure here.

Comment thread crates/walgit-server/src/smart.rs Outdated
A transient odb read error was stored as a miss for the manifest version, so the object kept reporting as missing until the next push.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis closed this Oct 29, 2026
@yordis yordis reopened this Oct 29, 2026
@yordis yordis closed this Oct 29, 2026
@yordis yordis reopened this Oct 29, 2026
@yordis
yordis merged commit 7226d2a into main Oct 29, 2026
13 of 19 checks passed
@yordis
yordis deleted the yordis/fix-git-object-info branch October 29, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants