Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 104 Pull Requests 12 Actions Packages Projects Releases Wiki Activity

fix(guard): derive base ref from configured upstream, not the remote-HEAD cache

Browse Source 
Remediates the REQUEST_CHANGES verdict (review 658) at head 2d5d5c9d.

B1 — refs/remotes/<remote>/HEAD is a stale local cache, not authority.

The previous derivation read that symref as "git's own record of the remote
default branch". It is a cache written once at clone time; an ordinary fetch
never refreshes it, and only an explicit `git remote set-head` updates it. On
the real target this issue exists to unblock, the cache still named `main`
while the checkout tracked and sat exactly on `dev`, so the guard compared
HEAD a37ac427c18b against MDCPS/main 9a84325a1b68 and blocked a checkout that
was not behind anything.

The resolver now derives, in order:

  1. the identity remote, exact case preserved;
  2. the checkout's own configured upstream — branch.<current>.remote plus
     branch.<current>.merge — accepted only when it names that remote and its
     remote-tracking ref actually exists;
  3. otherwise exactly one present master/main/dev remote-tracking ref.

The cached symref is demoted to an observation. It is still read and reported
as cached_remote_head_branch / cached_remote_head_conflicts, and it is named in
refusal text so an operator can see the misleading signal, but it never decides
the branch and never breaks a tie between ambiguous candidates. Requiring the
tracking ref to exist also makes `proven` honest: every proven target now names
a ref that resolves.

Verified read-only against /Users/jasonwalker/Development/weekly-briefings:
MDCPS/dev, source configured_branch_upstream, cached_remote_head_conflicts
true, checkout not stale. PRGS is unchanged — prgs/master, identical SHA.

B2 — an inferred remote must not be laundered into explicit caller intent.

assess_target_repository_parity resolved the identity remote itself and passed
it back into resolve_target_base_ref, which reads a caller-supplied remote as
"the caller already disambiguated" and skips its ambiguity gate. On a target
whose remotes claim different repositories the gate refused while the report
named a different repository with stale=false and no reasons.

The parameter is renamed `explicit_remote` through the resolver and both
root_checkout_guard entry points so the two meanings cannot be confused, and
the reporting path no longer supplies one. Identity resolution for reporting
moves to the new ambiguity-aware assess_identity_remote, so an ambiguous target
now yields a null slug, no tracking ref, and the same reason_code the gate
emits. resolve_identity_remote / repository_identity_slug keep their first-wins
behaviour for the #706/#973 canonical-root validation path, which compares
against an independently trusted expected slug and needs no ambiguity verdict.

Nothing fetches, sets a remote HEAD, writes a ref, adds a remote, invents a
branch, or changes any repository's default branch. A test snapshots refs,
remotes, local config, HEAD, branch, working-tree status, and the cached symref
across every resolver entry point and asserts all are unchanged.

Tests: tests/test_issue_983_cross_repo_base_ref.py rewritten to 37 tests. The
principal MDCPS/dev fixture now reproduces the real defect — upstream dev,
cached refs/remotes/MDCPS/HEAD -> main, both refs present at different commits
— rather than manufacturing the cache state the real checkout does not have.
Added: cache-alone never proves a target, cache never breaks a tie, gate and
report agree on ambiguous remote and ambiguous branch, explicit disambiguation
stays distinct from inferred identity, no production caller passes
explicit_remote, and the read-only proof above.

Focused suite 37 passed. Nineteen affected suites 441 passed, 167 subtests
passed. Full suite 28 failed, 6204 passed, 6 skipped, 1106 subtests passed;
clean baseline at the same base commit 108cbfa 28 failed, 6166 passed, 6
skipped, 1106 subtests passed. Sorted FAILED lists are byte-identical
(sha1 092dae4bc8c4e77d14504d90690d50e0fcd2f637) — zero introduced failures.

Refs #983

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
sysadmin
2026-07-31 00:00:14 -04:00
co-authored by Claude Opus 4.8
parent 2d5d5c9d17
commit 03b434a0b6
4 changed files with 685 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files
canonical_repository_root.py
+291 -78
View File
@@ -37,18 +37,29 @@ CANONICAL_ROOT_ENV = "GITEA_CANONICAL_REPOSITORY_ROOT"
# Candidate git remote names probed when deriving repository identity.
_IDENTITY_REMOTE_CANDIDATES = ("prgs", "origin", "dadeschools", "mdcps")
# Fallback integration-branch names, probed only when the remote publishes no
# ``refs/remotes/<remote>/HEAD`` symbolic ref (#983). Mirrors the stable base
# branches recognised elsewhere in the workflow (``stacked_pr_support``). The
# fallback is deliberately *not* ordered-first-wins: when more than one of these
# refs exists and git records no default, the integration branch is genuinely
# ambiguous and resolution fails closed instead of guessing.
# Fallback integration-branch names, probed only when the checkout declares no
# configured upstream (#983). Mirrors the stable base branches recognised
# elsewhere in the workflow (``stacked_pr_support``, ``root_checkout_guard``).
# The fallback is deliberately *not* ordered-first-wins: when more than one of
# these refs exists and the checkout records no upstream, the integration branch
# is genuinely ambiguous and resolution fails closed instead of guessing.
INTEGRATION_BRANCH_CANDIDATES: tuple[str, ...] = ("master", "main", "dev")
# Sources for a *proven* base-ref derivation (#983 B1).
#
# ``refs/remotes/<remote>/HEAD`` is deliberately absent from this list. It is a
# local symbolic-ref *cache* written once at clone time and refreshed only by an
# explicit ``git remote set-head``; an ordinary fetch never updates it. When the
# upstream default branch changes afterwards the cache keeps naming the old
# branch, so trusting it derives the wrong integration branch for a checkout
# that is sitting exactly on its tip. The cache is still read, but only as a
# corroborating observation reported back to the caller — never as an authority,
# and never as a tie-breaker between otherwise ambiguous candidates.
BASE_REF_SOURCE_CONFIGURED_UPSTREAM = "configured_branch_upstream"
BASE_REF_SOURCE_UNIQUE_CANDIDATE = "unique_integration_branch_ref"
# Reason codes for base-ref derivation outcomes (#983), so callers and tests can
# assert the refusal cause instead of string-matching prose.
BASE_REF_SOURCE_REMOTE_HEAD = "remote_head_symref"
BASE_REF_SOURCE_UNIQUE_CANDIDATE = "unique_integration_branch_ref"
DENY_NO_IDENTITY_REMOTE = "no_identity_remote"
DENY_AMBIGUOUS_REMOTE = "ambiguous_identity_remote"
DENY_AMBIGUOUS_BASE_BRANCH = "ambiguous_integration_branch"
@@ -190,6 +201,153 @@ def _configured_remote_identities(path: str, remote: str | None) -> list[tuple[s
return found
def _configured_branch_upstream(path: str) -> tuple[str | None, str | None]:
"""``(remote_name, branch)`` the checked-out branch is configured to track.
Reads ``branch.<current>.remote`` and ``branch.<current>.merge`` — the
checkout's own explicitly configured integration target, equivalent to
``@{upstream}``. Unlike ``refs/remotes/<remote>/HEAD`` this is not a
clone-time cache: it is written when the branch is set up to track an
upstream and rewritten whenever that tracking changes, so it states what the
checkout actually integrates onto today (#983 B1).
Returns ``(None, None)`` on a detached HEAD or an untracked branch. Names are
returned verbatim; git remote names are case-sensitive.
"""
text = (path or "").strip()
if not text:
return None, None
branch = _git_read(text, "symbolic-ref", "--quiet", "--short", "HEAD")
if not branch:
return None, None
remote = _git_read(text, "config", "--get", f"branch.{branch}.remote")
merge = _git_read(text, "config", "--get", f"branch.{branch}.merge")
if not remote or not merge:
return None, None
prefix = "refs/heads/"
upstream_branch = merge[len(prefix):].strip() if merge.startswith(prefix) else merge.strip()
if not upstream_branch:
return None, None
return remote, upstream_branch
def _cached_remote_head_branch(path: str, remote: str) -> str | None:
"""Branch named by the *cached* ``refs/remotes/<remote>/HEAD`` symref.
Read for observability only. This value is never authoritative (see
:data:`BASE_REF_SOURCE_CONFIGURED_UPSTREAM`); it is surfaced so an operator
can see that the local cache disagrees with the configured upstream, and so
a refusal can name the misleading signal explicitly.
"""
prefix = f"refs/remotes/{remote}/"
symref = _git_read(path, "symbolic-ref", "--quiet", f"{prefix}HEAD")
if not symref or not symref.startswith(prefix):
return None
branch = symref[len(prefix):].strip()
return branch or None
def assess_identity_remote(
path: str, *, explicit_remote: str | None = None
) -> dict:
"""Resolve the identity remote, failing closed when the target is ambiguous.
This is the ambiguity-aware counterpart to :func:`resolve_identity_remote`,
and the reason gating and reporting can no longer disagree (#983 B2).
*explicit_remote* is a **caller-supplied disambiguation** and nothing else.
It must come from an operator or an explicitly sanctioned repository
context; a value this module inferred while probing must never be handed
back in through it, because doing so re-labels an internal first-wins guess
as deliberate caller intent and silently suppresses the ambiguity gate.
Decision table:
* No remote yields a parseable identity -> :data:`DENY_NO_IDENTITY_REMOTE`.
* *explicit_remote* names one of the resolving remotes -> that remote is
authoritative, ``explicit`` True.
* Otherwise, when every resolving remote claims the **same** repository the
target is unambiguous and is accepted, ``explicit`` False. The remote
named by the checkout's configured upstream is preferred among equals so
the choice is deterministic rather than probe-order dependent.
* Otherwise distinct remotes claim different repositories and there is no
sanctioned disambiguation -> :data:`DENY_AMBIGUOUS_REMOTE`.
Returns a dict with ``remote``, ``slug``, ``identities``, ``ambiguous``,
``explicit``, ``reason_code`` and ``reasons``. ``remote``/``slug`` are None
on any refusal, so a caller cannot report a repository the gate refuses.
"""
text = (path or "").strip()
result: dict = {
"remote": None,
"slug": None,
"identities": [],
"ambiguous": False,
"explicit": False,
"reason_code": None,
"reasons": [],
}
if not text:
result["reason_code"] = DENY_NO_IDENTITY_REMOTE
result["reasons"].append(
"no repository path supplied for identity-remote resolution (fail closed)"
)
return result
named = (explicit_remote or "").strip() or None
identities = _configured_remote_identities(text, named)
result["identities"] = list(identities)
if not identities:
result["reason_code"] = DENY_NO_IDENTITY_REMOTE
result["reasons"].append(
f"no git remote at '{text}' yields a parseable repository identity "
"(fail closed)"
)
return result
if named:
for name, slug in identities:
if name == named:
result["remote"] = name
result["slug"] = slug
result["explicit"] = True
return result
distinct = {slug for _, slug in identities}
if len(distinct) > 1:
listed = ", ".join(f"{name} -> {slug}" for name, slug in identities)
result["ambiguous"] = True
result["reason_code"] = DENY_AMBIGUOUS_REMOTE
detail = (
f"explicitly named remote '{named}' does not resolve a repository identity "
"there, so it cannot disambiguate; "
if named
else ""
)
result["reasons"].append(
f"ambiguous repository identity at '{text}': {detail}remotes resolve to "
f"different repositories ({listed}); no single authoritative target can "
"be established (fail closed)"
)
return result
# One repository, possibly reachable through several remote names (a mirror).
# Prefer the remote the checkout is actually configured to track so the
# choice is deterministic instead of probe-order dependent.
upstream_remote, _ = _configured_branch_upstream(text)
chosen = identities[0]
if upstream_remote:
for entry in identities:
if entry[0] == upstream_remote:
chosen = entry
break
result["remote"], result["slug"] = chosen
return result
def resolve_identity_remote(
path: str, *, remote: str | None = None
) -> tuple[str | None, str | None]:
@@ -201,6 +359,14 @@ def resolve_identity_remote(
name to build ``refs/remotes/<remote>/<branch>``, so it is now returned
rather than discarded (#983). Returns ``(None, None)`` when no remote URL is
parseable (identity cannot be proven).
First-wins by design: this is the identity lookup behind
:func:`repository_identity_slug` and the #706/#973 canonical-root
validation, which compare an observed slug against an independently trusted
expected slug and therefore do not need an ambiguity verdict. Callers that
*derive* a target rather than validate one — the mutation guard and the
parity report — must use :func:`assess_identity_remote`, which fails closed
on ambiguity (#983 B2).
"""
text = (path or "").strip()
if not text:
@@ -229,6 +395,10 @@ def _base_ref_result(
repository_slug: str | None = None,
source: str | None = None,
reason_code: str | None = None,
identity_explicit: bool = False,
configured_upstream_remote: str | None = None,
configured_upstream_branch: str | None = None,
cached_remote_head_branch: str | None = None,
) -> dict:
"""Build the base-ref derivation payload.
@@ -237,6 +407,12 @@ def _base_ref_result(
qualified ``refs/remotes/<remote>/<branch>``. It is empty whenever the
derivation is not ``proven``, so an unresolved target can never be probed
against some other repository's ref.
``cached_remote_head_branch`` reports what the local
``refs/remotes/<remote>/HEAD`` cache claims, and
``cached_remote_head_conflicts`` whether that claim disagrees with the branch
actually derived. Both are observability only: the cache never decides the
outcome (#983 B1).
"""
tracking_ref = f"refs/remotes/{remote}/{branch}" if proven and remote and branch else None
tracking_refs: tuple[str, ...] = (
@@ -252,10 +428,33 @@ def _base_ref_result(
"tracking_refs": tracking_refs,
"source": source,
"reason_code": reason_code,
"identity_explicit": identity_explicit,
"configured_upstream_remote": configured_upstream_remote,
"configured_upstream_branch": configured_upstream_branch,
"cached_remote_head_branch": cached_remote_head_branch,
"cached_remote_head_conflicts": bool(
cached_remote_head_branch and branch and cached_remote_head_branch != branch
),
"reasons": list(reasons),
}
def _git_read(path: str, *args: str) -> str | None:
"""Run a read-only git command in *path*; ``None`` on any failure."""
try:
res = subprocess.run(
["git", "-C", path, *args],
capture_output=True,
text=True,
check=False,
)
except Exception:
return None
if res.returncode != 0:
return None
return (res.stdout or "").strip() or None
def _ref_exists(path: str, ref: str) -> bool:
"""Whether *ref* resolves in the checkout at *path*."""
try:
@@ -270,30 +469,45 @@ def _ref_exists(path: str, ref: str) -> bool:
return res.returncode == 0 and bool((res.stdout or "").strip())
def resolve_target_base_ref(path: str, *, remote: str | None = None) -> dict:
def resolve_target_base_ref(path: str, *, explicit_remote: str | None = None) -> dict:
"""Derive the authoritative integration base ref for the checkout at *path*.
This is the single resolved target shared by cross-repository mutation
gating and parity reporting, so the two can never disagree about which
commit a checkout is supposed to match (#983).
*explicit_remote* is a caller-supplied disambiguation only; see
:func:`assess_identity_remote` for why an internally inferred remote must
never be passed back in here (#983 B2).
Resolution order:
1. The identity remote — the remote that already proves repository identity
via :func:`resolve_identity_remote`, with its **exact configured case**
preserved (``MDCPS`` stays ``MDCPS``).
2. The integration branch, from ``refs/remotes/<remote>/HEAD`` — git's own
record of that remote's default branch, written by ``clone`` and
``remote set-head``. This is authoritative repository state, which is why
no new configuration field is required.
3. Only if the remote publishes no such symbolic ref, exactly one of
:data:`INTEGRATION_BRANCH_CANDIDATES` present as a remote-tracking ref.
1. **Identity remote** — via :func:`assess_identity_remote`, with its exact
configured case preserved (``MDCPS`` stays ``MDCPS``). Ambiguous identity
fails closed here rather than resolving to whichever remote probed first.
2. **The checkout's configured upstream** — ``branch.<current>.remote`` plus
``branch.<current>.merge``, accepted when it names the identity remote
and its remote-tracking ref actually exists. This is the checkout's own
declaration of what it integrates onto, and unlike the remote-HEAD cache
it is rewritten whenever that tracking changes.
3. **Fallback** — only when the checkout declares no usable upstream,
exactly one of :data:`INTEGRATION_BRANCH_CANDIDATES` present as a
remote-tracking ref.
``refs/remotes/<remote>/HEAD`` is **not** a step. It is read for reporting
(``cached_remote_head_branch`` / ``cached_remote_head_conflicts``) and never
decides the branch, because it is a clone-time cache that an ordinary fetch
does not refresh: a checkout whose upstream default moved on still has the
old branch cached, and trusting it gates that checkout against a ref it does
not integrate onto (#983 B1).
Fails closed — ``proven`` False, empty ``tracking_refs``, and a
``reason_code`` — when identity is unprovable, when distinct remotes claim
different repositories, when several candidate branches exist with no
recorded default, or when no candidate exists at all. Nothing here invents a
branch, writes a ref, or falls back to another repository's base.
configured upstream, or when no candidate exists at all. Nothing here
invents a branch, writes a ref, runs a fetch, or falls back to another
repository's base. Every ``proven`` result names a tracking ref that
resolves in this checkout.
"""
text = (path or "").strip()
if not text:
@@ -303,62 +517,45 @@ def resolve_target_base_ref(path: str, *, remote: str | None = None) -> dict:
reason_code=DENY_NO_IDENTITY_REMOTE,
)
identities = _configured_remote_identities(text, remote)
if not identities:
identity = assess_identity_remote(text, explicit_remote=explicit_remote)
remote_name, slug = identity["remote"], identity["slug"]
if not remote_name:
return _base_ref_result(
proven=False,
reasons=[
f"no git remote at '{text}' yields a parseable repository identity, so "
"the integration base ref cannot be derived (fail closed)"
],
reason_code=DENY_NO_IDENTITY_REMOTE,
reasons=list(identity["reasons"]),
reason_code=identity["reason_code"],
identity_explicit=bool(identity["explicit"]),
)
# Ambiguity only matters when the caller named no remote: if distinct remotes
# describe different repositories there is no single authoritative target,
# and picking the first would silently gate against the wrong repository.
if not (remote or "").strip():
distinct = {slug for _, slug in identities}
if len(distinct) > 1:
listed = ", ".join(f"{name} -> {slug}" for name, slug in identities)
return _base_ref_result(
proven=False,
reasons=[
f"ambiguous repository identity at '{text}': remotes resolve to "
f"different repositories ({listed}); no single authoritative "
"integration base ref can be derived (fail closed)"
],
reason_code=DENY_AMBIGUOUS_REMOTE,
)
remote_name, slug = identities[0]
symref = None
try:
res = subprocess.run(
["git", "-C", text, "symbolic-ref", "--quiet", f"refs/remotes/{remote_name}/HEAD"],
capture_output=True,
text=True,
check=False,
)
if res.returncode == 0:
symref = (res.stdout or "").strip() or None
except Exception:
symref = None
prefix = f"refs/remotes/{remote_name}/"
if symref and symref.startswith(prefix):
branch = symref[len(prefix):].strip()
if branch and branch != "HEAD":
return _base_ref_result(
proven=True,
reasons=[],
remote=remote_name,
branch=branch,
repository_slug=slug,
source=BASE_REF_SOURCE_REMOTE_HEAD,
)
cached = _cached_remote_head_branch(text, remote_name)
upstream_remote, upstream_branch = _configured_branch_upstream(text)
common = {
"repository_slug": slug,
"identity_explicit": bool(identity["explicit"]),
"configured_upstream_remote": upstream_remote,
"configured_upstream_branch": upstream_branch,
"cached_remote_head_branch": cached,
}
# 2. The checkout's configured upstream, when it belongs to the identity
# remote and its tracking ref is actually present. Requiring the ref to
# exist keeps 'proven' honest: a proven target is always resolvable.
if (
upstream_remote == remote_name
and upstream_branch
and _ref_exists(text, f"{prefix}{upstream_branch}")
):
return _base_ref_result(
proven=True,
reasons=[],
remote=remote_name,
branch=upstream_branch,
source=BASE_REF_SOURCE_CONFIGURED_UPSTREAM,
**common,
)
# 3. Exactly one known integration branch present as a remote-tracking ref.
present = [
candidate
for candidate in INTEGRATION_BRANCH_CANDIDATES
@@ -370,28 +567,44 @@ def resolve_target_base_ref(path: str, *, remote: str | None = None) -> dict:
reasons=[],
remote=remote_name,
branch=present[0],
repository_slug=slug,
source=BASE_REF_SOURCE_UNIQUE_CANDIDATE,
**common,
)
# The cached remote HEAD is named in the refusal so the operator can see the
# signal that looks authoritative but is not, and is told the read-only fix.
cache_note = (
f" the cached '{prefix}HEAD' names '{cached}', but that cache is written at "
"clone time and is not refreshed by fetch, so it cannot break the tie;"
if cached
else ""
)
remedy = (
f" Configure the checkout's upstream (git branch --set-upstream-to={remote_name}/"
"<branch>) so the integration target is declared rather than guessed."
)
if len(present) > 1:
return _base_ref_result(
proven=False,
reasons=[
f"remote '{remote_name}' publishes no '{prefix}HEAD' default and "
f"several integration branches exist ({', '.join(present)}); the "
"integration base ref is ambiguous (fail closed)"
f"the checkout at '{text}' declares no upstream on remote "
f"'{remote_name}' and several integration branches exist "
f"({', '.join(present)});{cache_note} the integration base ref is "
f"ambiguous (fail closed).{remedy}"
],
reason_code=DENY_AMBIGUOUS_BASE_BRANCH,
**common,
)
return _base_ref_result(
proven=False,
reasons=[
f"remote '{remote_name}' publishes no '{prefix}HEAD' default and none of "
f"{'/'.join(INTEGRATION_BRANCH_CANDIDATES)} exists as a remote-tracking "
f"ref under '{prefix}'; the integration base ref cannot be derived "
"(fail closed; no fetch is performed here)"
f"the checkout at '{text}' declares no upstream on remote '{remote_name}' "
f"and none of {'/'.join(INTEGRATION_BRANCH_CANDIDATES)} exists as a "
f"remote-tracking ref under '{prefix}';{cache_note} the integration base "
f"ref cannot be derived (fail closed; no fetch is performed here).{remedy}"
],
reason_code=DENY_NO_BASE_BRANCH,
**common,
)