Commit Graph
4 Commits
Author SHA1 Message Date
sysadminandClaude Opus 4.8 03b434a0b6 fix(guard): derive base ref from configured upstream, not the remote-HEAD cache
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]>
2026-07-31 00:00:14 -04:00
sysadminandClaude Opus 4.8 2d5d5c9d17 fix(guard): derive cross-repository target base ref
Cross-repository mutation gating assumed the tracking base ref was
prgs/master, and parity reporting independently assumed origin/master.
A namespace bound to any other repository -- for example remote MDCPS on
integration branch dev -- could not prove base equivalence, so every
gated mutation failed closed with no reachable remedy. The two modules
also disagreed with each other, so at most one could be right for any
given repository.

Derive the target instead of assuming it. canonical_repository_root
already discovered the correct remote while resolving repository
identity and then discarded its name; it now returns that name with its
exact configured case preserved, and resolve_target_base_ref() builds
refs/remotes/<remote>/<branch> from it. The integration branch comes
from refs/remotes/<remote>/HEAD -- git's own record of the remote's
default branch -- so no new configuration field is required. Only when
a remote publishes no such default does it fall back to exactly one
present integration-branch candidate.

Resolution fails closed with a machine-checkable 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. It never invents a branch, writes
a ref, or falls back to another repository's base.

Both the mutation guard and the parity report now consume that one
resolved target, so they cannot disagree again. Root-checkout
contamination names the ref it actually compared rather than a literal
prgs/master the target repository may not have.

Fixes an observable defect in this repository: refs/remotes/origin/master
survives as an orphan ref from a removed remote, so parity reported the
target stale against a dead commit while reporting its identity as
underivable.

PRGS behaviour is unchanged -- prgs/master still resolves via the
recorded remote HEAD to the same SHA, and an explicit remote_refs
override keeps the historical probe path verbatim.

Tests: 24 new hermetic regression tests covering PRGS prgs/master,
MDCPS/dev, no origin remote, exact remote-name case, equal/behind/
divergent targets, missing remote or ref, ambiguous remote and branch
resolution, gate/report agreement, and every affected production
caller. Full suite 6191 passed / 28 failed, byte-identical failure set
to the baseline at 108cbfa (zero introduced failures).

Refs #983

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-30 22:31:15 -04:00
sysadminandClaude Opus 4.8 9438855fdd fix: preserve actual reconciler role for #274/#475 role exemptions during comment_issue preflight (Closes #540)
resolve_task_capability("comment_issue") stamps _preflight_resolved_role
= "author" because comment_issue.required_role_kind = author.
_effective_workspace_role() prefers that stamp, so a genuine
prgs-reconciler session was classified as an author inside the #274
branch-only mutation guard and the #475 root checkout guard, blocking
gitea_create_issue_comment from the control checkout.

Fix keys the role exemptions off the actual active profile role as well
as the effective workspace role:

- Add _actual_profile_role(): derives the workspace role from the active
  profile alone, never from _preflight_resolved_role.
- _enforce_branches_only_author_mutation: exempt when EITHER the
  effective role OR the actual profile role is a non-author role.
- _enforce_root_checkout_guard: pass actual_role; assess_root_checkout_guard
  now exempts a reconciler by resolved OR actual role.

Author blocking is preserved: an actual author profile classifies as
author in both signals, so it stays blocked on a contaminated control
checkout. Merger strictness stays keyed on the resolved task role so a
merger operating from its clean branches/ workspace under a non-merge
task is not over-tightened.

Regression tests (tests/test_issue_540_comment_role_poison.py) cover the
reconciler comment path, author-blocking path, reviewer/merger/reconciler
role classification under a poisoned author stamp, and the root guard
actual_role exemption.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-08 12:03:14 -04:00
sysadminandClaude Opus 4.8 be592d6d4d feat: add root checkout guard for contaminated control checkout (Closes #475)
Introduce root_checkout_guard helper and enforce it in verify_preflight_purity
so author, reviewer, and merger mutations fail closed when the stable control
checkout is on the wrong branch, detached, dirty, or diverged from prgs/master.
Isolated branches/... worktrees and reconciler paths remain allowed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-08 02:23:27 -04:00