allow assuming trust during upward discovery#2652
Merged
Merged
Conversation
Sebastian Thiel (Byron)
force-pushed
the
override-upward-trust
branch
from
June 18, 2026 09:16
1ef03b2 to
d431934
Compare
Sebastian Thiel (Byron)
marked this pull request as ready for review
June 18, 2026 09:16
There was a problem hiding this comment.
Pull request overview
This PR updates gix-discover’s upward repository discovery options to allow callers to assume a trust level (skipping ownership-based trust computation), enabling explicit trust overrides during discovery (e.g., for downstream integrations needing custom trust handling).
Changes:
- Replaced
upwards::Options::required_trust: gix_sec::Trustwithupwards::Options::trust: upwards::Trust(new enum). - Added
Trust::Assume(level)to bypassgix_sec::Trust::from_path_ownership()and return the provided level. - Added a regression test ensuring assumed trust is returned while discovery still finds the repository.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| gix-discover/src/upwards/types.rs | Introduces upwards::Trust enum and updates Options to use it (breaking API change). |
| gix-discover/src/upwards/mod.rs | Wires Trust::{Required,Assume} into discovery logic and adjusts returned trust handling. |
| gix-discover/tests/discover/upwards/mod.rs | Adds coverage for overriding computed trust via Trust::Assume. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
upward discovery options should allow callers to override the trust level during discovery. When a trust level is provided, discovery should not compute trust from path ownership and should instead assume the provided level. This intentionally changes the public Options::trust (formerly `required_trust`) field from gix_sec::Trust to gix_discover::upwards::Trust so the field can represent either a required computed trust threshold or an assumed trust level. Existing callers that set required_trust directly need to wrap their threshold in Trust::Required(_). The fix keeps the default as the existing reduced-trust requirement, while Trust::Assume(level) skips Trust::from_path_ownership() and returns the provided trust level. Co-authored-by: Sebastian Thiel <[email protected]>
Sebastian Thiel (Byron)
force-pushed
the
override-upward-trust
branch
from
June 18, 2026 09:31
d431934 to
f211c2d
Compare
Sebastian Thiel (Byron)
changed the title
Sebastian Thiel (Byron)
force-pushed
the
override-upward-trust
branch
from
June 18, 2026 11:42
f211c2d to
bb8045d
Compare
…ons` trust handling.
Sebastian Thiel (Byron)
force-pushed
the
override-upward-trust
branch
from
June 18, 2026 11:45
bb8045d to
7ba53e1
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
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.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Reported issue
Summary
gix_discover::upwards::Options::required_trustto the newgix_discover::upwards::Trustenum.Trust::Required(gix_sec::Trust::Reduced).Trust::Assume(level)so discovery skipsTrust::from_path_ownership()and returns the caller-provided trust level.Validation
cargo test -p gix-discover --test discover upwards::can_override_computed_trustcargo test -p gix-discovercargo check -p gix-discover -p gix --testscargo check --workspace --all-targets --no-default-features --features leanRUSTDOCFLAGS="-D warnings" cargo doc -p gix-discover --no-deps --features sha1Review
codex review --commit 93d6a0b6f5aaf68b458086b1d405b223f4621472reported that the public field type change should be marked as breaking.change!:and a commit body documenting the source break.codex review --commit 1ef03b25c6b3b99efe56a0e7648267a952a6e0c4reported no issues.