Skip to content

lore-server: Return NotFound for revision info on an empty branch - #184

Open
bclarke123 wants to merge 2 commits into
EpicGames:mainfrom
bclarke123:fix-empty-branch-revision-info
Open

lore-server: Return NotFound for revision info on an empty branch#184
bclarke123 wants to merge 2 commits into
EpicGames:mainfrom
bclarke123:fix-empty-branch-revision-info

Conversation

@bclarke123

Copy link
Copy Markdown
Contributor

Summary

branch::load_latest returns the zero hash as its no-revisions-yet sentinel for an existing branch, but the RevisionInfo handler marched into state/metadata loads with it and failed as Internal ("file not found: metadata key") for every freshly created repository. Guard the resolved signature and answer NotFound, mirroring the existing zero-hash handling in revision_tree and load_optional_parent.

Possibly related to #113 - a client that hides repos whose latest-revision query errors would show the symptom this PR fixes

Checklist

  • Linked to an issue above, or explained why none applies
  • Every commit carries a Signed-off-by: line (DCO -- see CONTRIBUTING.md)
  • cargo +nightly fmt --all produces no changes
  • cargo clippy --all-targets -- -D warnings --no-deps passes
  • cargo test passes
  • uv run pytest passes (when Python tooling is touched)
  • Docs under docs/ updated where behavior changed
  • If this change is large or cross-cutting, an LEP has been filed first

  branch::load_latest returns the zero hash as its no-revisions-yet
  sentinel for an existing branch, but the RevisionInfo handler marched
  into state/metadata loads with it and failed as Internal
  ("file not found: metadata key") for every freshly created repository.
  Guard the resolved signature and answer NotFound, mirroring the
  existing zero-hash handling in revision_tree and load_optional_parent.

Signed-off-by: Ben Clarke <ben@arrayofstars.com>
@github-actions github-actions Bot added the area:server Server, provider integrations, telemetry label Sep 2, 2026
@bclarke123

Copy link
Copy Markdown
Contributor Author

Would appreciate if somebody with permission can rerun the linux smoke test here, the failure (test_service.py::test_service_resolves_relative_paths_against_caller) looks unrelated to this change. The test exercises service-daemon staging and local status on a branch that has revisions, so the new zero-hash guard can never fire on its path. It passes 8/8 locally on this change, and passed on #183's run from the same base. Happy to dig in if it fails again, but this looks like a flake actions run maybe

// zero hash. Surface that as NotFound rather than proceeding into
// state/metadata loads that fail as Internal.
if signature.is_zero() {
return Err(Status::not_found("Branch has no revisions"));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is only the default branch that can have a zero revision - and in that case I would argue that it should early respond with a successful response carrying that zero information

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!

Per review: only the default branch can exist with no revisions, and
that is not an error condition. Return an OK RevisionInfoResponse with
no revision instead of NotFound; the optional revision field already
expresses it.

Signed-off-by: Ben Clarke <ben@arrayofstars.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:server Server, provider integrations, telemetry

Development

Successfully merging this pull request may close these issues.

2 participants