feat(pm): add git protocol spec parsing (1/3) by killagu · Pull Request #2622 · utooland/utoo

killagu · 2026-02-25T02:22:06Z

Summary

Split from #2603 — Part 1 of 3 for git dependency resolution.

Add PackageSpec enum (Registry, Git, GitHub) with PackageSpec::parse() associated method
Add is_non_registry_spec() with #[deprecated] alias for is_local_spec
Add ResolveError::Git { url, message } structured error variant
Add GitCloneResult data type in traits/git.rs (data only, no implementation)
Add is_http_tarball_spec() for URL routing

Architectural decisions

PackageSpec::parse() as associated method (not free function) — Rust idiomatic
Only Registry/Git/GitHub variants — no YAGNI Http/FileDir/FileTarball
traits/git.rs contains only data types, not implementations
Structured ResolveError::Git { url, message } instead of Git(String)

Test plan

cargo check -p utoo-ruborist passes
cargo test -p utoo-ruborist — 121 tests + 8 doctests pass
Review PackageSpec::parse() edge cases

Part of git dependency resolution: PR1 → #2603-pr2 → #2603-pr3

🤖 Generated with Claude Code

gemini-code-assist · 2026-02-25T02:22:27Z

Summary of Changes

Hello @killagu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request establishes foundational components for Git dependency resolution within the ruborist crate. It introduces a structured way to parse and categorize package specifications, differentiating between registry, Git, and GitHub sources. This work enhances the system's ability to handle diverse dependency types and prepares it for more advanced Git-based resolution logic in subsequent development phases.

Highlights

Package Specification Parsing: Introduced a PackageSpec enum (Registry, Git, GitHub) with an associated parse() method to robustly interpret various raw package spec strings, including Git URLs and GitHub shorthands.
Dependency Type Identification: Added is_non_registry_spec() to identify local and remote non-registry dependencies, deprecating the narrower is_local_spec() and updating dependency collection logic to use the new function.
Error Handling for Git Resolution: Included a new ResolveError::Git variant in ResolveError to specifically handle failures during Git dependency resolution.
Git Clone Result Data Structure: Defined a GitCloneResult data type in traits/git.rs to encapsulate metadata (name, version, cache path, resolved URL) returned from Git clone operations.
HTTP Tarball Spec Routing: Implemented is_http_tarball_spec() to correctly identify HTTP tarball URLs, which helps in routing these specs away from standard registry or Git resolution paths.

Changelog

crates/ruborist/src/lib.rs
- Added public re-exports for the new spec and git modules.
crates/ruborist/src/model/mod.rs
- Declared the new spec module.
crates/ruborist/src/model/spec.rs
- Created the PackageSpec enum to represent different package specification types (Registry, Git, GitHub).
- Implemented the PackageSpec::parse() method for interpreting raw spec strings.
- Added the is_http_tarball_spec() function for identifying HTTP tarball URLs.
- Included comprehensive unit tests for the PackageSpec::parse() method and is_http_tarball_spec().
crates/ruborist/src/resolver/preload.rs
- Introduced is_non_registry_spec() to detect various non-registry dependency types (local, git, github).
- Deprecated is_local_spec() in favor of the more comprehensive is_non_registry_spec().
- Updated the collect_deps function to filter dependencies using is_non_registry_spec().
- Added new unit tests for is_non_registry_spec().
crates/ruborist/src/resolver/registry.rs
- Added a Git(String) variant to the ResolveError enum to represent Git resolution failures.
- Updated the source implementation for ResolveError to correctly handle the new Git variant.
crates/ruborist/src/traits/git.rs
- Created the GitCloneResult struct to define the metadata returned by a Git clone operation.
crates/ruborist/src/traits/mod.rs
- Declared the new git module.

Activity

This pull request is explicitly identified as 'Part 1 of 3' for a larger Git dependency resolution initiative, indicating a phased approach to development.
The author has verified that cargo check -p utoo-ruborist and cargo test -p utoo-ruborist pass, confirming successful compilation and the passing of 121 tests and 8 doctests.
A self-review item is noted to 'Review PackageSpec::parse() edge cases', suggesting ongoing attention to robustness.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request is a solid first step towards git dependency resolution by adding parsing for git and GitHub package specifications. The new PackageSpec enum and its associated parser are well-designed. However, I've identified a potential issue in how malformed github: specifiers are handled, which could lead to incorrect package resolution. Additionally, there's a small discrepancy between the PR description and the implementation of the new ResolveError::Git variant. I've provided suggestions to address these points, along with a minor suggestion for more idiomatic code. With these adjustments, the changes will be even more robust.

Copilot

Pull request overview

This PR adds foundational types and parsing logic for git dependency resolution in the ruborist crate, as part 1 of a 3-part series to internalize git dependency resolution. It introduces structured spec parsing, git-related data types, and error variants without implementing the actual git clone/resolution logic yet.

Changes:

Add PackageSpec enum with parsing logic to distinguish between Registry, Git, and GitHub shorthand specs
Add is_non_registry_spec() helper to identify specs that should skip registry preloading (local + git specs)
Add ResolveError::Git variant for structured git resolution errors
Add GitCloneResult data type to represent git clone operation results
Add is_http_tarball_spec() helper for URL routing

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
crates/ruborist/src/model/spec.rs	New file implementing `PackageSpec` enum and parsing logic for registry, git, and GitHub shorthand specs
crates/ruborist/src/traits/git.rs	New file defining `GitCloneResult` struct for git clone operation metadata
crates/ruborist/src/resolver/registry.rs	Add `ResolveError::Git` variant for git resolution errors
crates/ruborist/src/resolver/preload.rs	Add `is_non_registry_spec()` and deprecate `is_local_spec()` to include git specs
crates/ruborist/src/model/mod.rs	Export new `spec` module
crates/ruborist/src/traits/mod.rs	Export new `git` module
crates/ruborist/src/lib.rs	Add public re-exports for `spec` and `git` modules

Add typed PackageSpec enum to utoo-ruborist for parsing dependency specifications into Registry, Git, and GitHub variants. This is the foundation for git dependency resolution support. - PackageSpec::parse() as idiomatic associated method - is_http_tarball_spec() for routing HTTP tarball specs - Re-export via utoo_ruborist::spec module - 10 unit tests + 2 doc-tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

… GitCloneResult Extend the git protocol spec parsing layer: - Add is_non_registry_spec() covering git+, git://, github: prefixes alongside existing local specs (file:, link:, workspace:, portal:) - Deprecate is_local_spec() with alias pointing to new function - Add ResolveError::Git variant with Display/Error impls - Add GitCloneResult struct in traits/git.rs (data type only) - Re-export GitCloneResult via utoo_ruborist::git module Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add Local and HttpTarball variants to PackageSpec enum - Handle bare user/repo GitHub shorthand in PackageSpec::parse - Handle file:/link:/workspace:/portal: as Local in PackageSpec::parse - Handle HTTP tarball URLs as HttpTarball in PackageSpec::parse - Rewrite is_non_registry_spec to delegate to PackageSpec::parse, eliminating duplicated logic and ensuring consistency - Remove is_http_tarball_spec from public re-export (now internal) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Introduce a `Protocol` enum with `PROTOCOL_PREFIXES` const table and `parse_prefix()` method, centralizing all protocol detection that was previously done via scattered `starts_with()` string checks. Rename `LocalProtocol` to `Protocol` and expand it to cover Git, GitHub, and Http protocols. Update `PackageSpec::from_str` to use `Protocol`-based routing, and rename `is_http_tarball_spec` to `has_tarball_extension`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

xusd320 · 2026-02-25T09:29:18Z

为啥不用 thiserror ？

elrrrrrrr

LGTM

…ry_spec Add `SpecStr` trait on `str` with `parse_spec()` (infallible) and `is_registry_spec()` methods. This replaces the free function `is_non_registry_spec` with the more idiomatic `spec.is_registry_spec()` pattern, and avoids the awkward `parse().is_ok_and()` on an Infallible Result. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

killagu · 2026-02-25T12:55:13Z

Addressed @xusd320's review comments:

parse_prefix → strip_prefix：语义上和 str::strip_prefix 一致，剥离前缀并返回 (Protocol, &str) 余下部分。
移除了 impl FromStr for PackageSpec，只保留 impl From<&str>。调用方通过 SpecStr 扩展 trait 的 parse_spec() 使用。
错误信息已在上一轮改为 "Unsupported protocol prefix"，并用 thiserror derive。

Introduces a Claude Code subagent that enforces idiomatic Rust style and project conventions. The review checklist is distilled from real PR #2622 feedback (xusd320, elrrrrrrr) covering 7 dimensions and 10 anti-patterns: incomplete enums, name-behavior mismatch, match guard escape, internal deprecated, excessive traits, sloppy error messages, YAGNI abstraction, string gymnastics, overly broad re-exports, and test blind spots. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…2635) * feat: add code-guard subagent for Rust code quality review Introduces a Claude Code subagent that enforces idiomatic Rust style and project conventions. The review checklist is distilled from real PR #2622 feedback (xusd320, elrrrrrrr) covering 7 dimensions and 10 anti-patterns: incomplete enums, name-behavior mismatch, match guard escape, internal deprecated, excessive traits, sloppy error messages, YAGNI abstraction, string gymnastics, overly broad re-exports, and test blind spots. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code-guard PR review feedback - Remove zero-width U+200B chars, use 4-backtick outer fence for nested code blocks in Output Format template - Fix Display/fmt::Display redundancy: clarify they are the same trait - Fix reviewer attribution: guard escape analysis was by elrrrrrrr, not xusd320 - Align error message guidance with Rust API Guidelines (lowercase by default, note project-specific override) - Make Tool Usage section tool-agnostic (renamed to Methodology), add note that rules are reusable across editors - Add severity calibration table with concrete criteria - Expand A7-A10 anti-patterns with full explanations, code examples, and real-world context - Narrow title from "Code Quality Sentinel" to "Rust Idiom & Style Review Agent" to accurately reflect scope - Shorten AGENTS.md Purpose column for readability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add YAML frontmatter and structured workflow to code-guard agent - Add required frontmatter: name, description, tools (Read-only: Read, Grep, Glob, Bash), model (sonnet), maxTurns (30) - Replace "Trigger Scenarios" with step-by-step "When Invoked" workflow - Remove redundant "Methodology" section (tools already declared in frontmatter) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: move subagents to agents/ with .claude/agents symlink - Move code-guard.md from .claude/agents/ to agents/ (alongside existing utoopack-performance-agent.md) - Symlink .claude/agents -> ../agents for Claude Code discovery - Add YAML frontmatter to utoopack-performance-agent.md (name, description, tools, model, maxTurns) - Update AGENTS.md subagent table to list both agents with canonical paths in agents/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: set Comprehensive Rust as the style authority for code-guard Reference Google Android team's Comprehensive Rust course as the primary style standard. All review judgments must align with its idioms and conventions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

…dback Address review comments from PR #2622/#2623: Phase 1 (correctness): - Add Local/Http variants to PackageSpec, unify is_non_registry_spec via parse delegation - Convert process_dependency to exhaustive match on PackageSpec variants - Fix TOCTOU race in cache writes with atomic tmp-dir + rename - Replace serde_json::Value with typed PkgNameVersion and direct VersionManifest deserialization Phase 2 (code quality): - Add thiserror derive for ResolveError, preserve error chain in Git variant - Convert GIT_CLONE_CACHE static to CloneCache instance variable in BuildDepsConfig - Consolidate cfg(native-git) checks inside git.rs, remove stubs from builder.rs - Move std::env reads out of ruborist into PM's git_resolver.rs (auth_token parameter) - Small fixes: NonZeroU32::MIN, with_cache_dir(PathBuf), cache_path→store_path, submodule skip trace, error message quote consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 25, 2026 02:22

killagu mentioned this pull request Feb 25, 2026

feat(pm): add native-git BFS resolution (2/3) #2623

Merged

4 tasks

Copilot started reviewing on behalf of killagu February 25, 2026 02:22 View session

killagu mentioned this pull request Feb 25, 2026

feat(pm): integrate git dependency support into install flow (3/3) #2624

Merged

4 tasks

gemini-code-assist Bot reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/model/spec.rs Outdated

Comment thread crates/ruborist/src/model/spec.rs

Comment thread crates/ruborist/src/resolver/registry.rs Outdated

Copilot AI reviewed Feb 25, 2026

View reviewed changes

killagu force-pushed the pr1/git-spec-parser branch 4 times, most recently from 53c271e to 41d9077 Compare February 25, 2026 03:35

elrrrrrrr reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/resolver/preload.rs Outdated

elrrrrrrr reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/model/spec.rs Outdated

elrrrrrrr reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/lib.rs Outdated

killagu force-pushed the pr1/git-spec-parser branch 3 times, most recently from ac30c61 to 27629a5 Compare February 25, 2026 07:32

killagu and others added 4 commits February 25, 2026 16:05

killagu force-pushed the pr1/git-spec-parser branch 2 times, most recently from 5bcf09b to c632bf8 Compare February 25, 2026 08:42

xusd320 reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/model/spec.rs Outdated

elrrrrrrr reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/model/spec.rs

killagu force-pushed the pr1/git-spec-parser branch from c632bf8 to 5240767 Compare February 25, 2026 09:52

xusd320 changed the title ~~feat(ruborist): add git protocol spec parsing (1/3)~~ feat(pm): add git protocol spec parsing (1/3) Feb 25, 2026

killagu force-pushed the pr1/git-spec-parser branch from 11dbfbf to 3be048c Compare February 25, 2026 11:18

elrrrrrrr approved these changes Feb 25, 2026

View reviewed changes

xusd320 reviewed Feb 25, 2026

View reviewed changes

Comment thread crates/ruborist/src/model/spec.rs Outdated

Comment thread crates/ruborist/src/model/spec.rs Outdated

killagu force-pushed the pr1/git-spec-parser branch from 3be048c to e07462e Compare February 25, 2026 12:54

killagu mentioned this pull request Feb 25, 2026

feat(agents): add code-guard subagent for Rust code quality review #2635

Merged

3 tasks

xusd320 approved these changes Feb 25, 2026

View reviewed changes

xusd320 enabled auto-merge (squash) February 25, 2026 13:18

xusd320 disabled auto-merge February 25, 2026 13:22

xusd320 enabled auto-merge (squash) February 25, 2026 13:22

Merge branch 'next' into pr1/git-spec-parser

846595d

xusd320 merged commit 6d5a998 into next Feb 26, 2026
23 checks passed

xusd320 deleted the pr1/git-spec-parser branch February 26, 2026 03:06

Conversation

killagu commented Feb 25, 2026

Summary

Architectural decisions

Test plan

Uh oh!

gemini-code-assist Bot commented Feb 25, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xusd320 commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

elrrrrrrr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

killagu commented Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xusd320 commented Feb 25, 2026 •

edited

Loading