feat(consistency): fix-consistency-remote works without REPO for all repos
Adds --remote CLI flag and fix_all_remote() function. When run without a REPO argument, the target checks all registered repos and: - Skips repos whose local path does not exist on this machine - Skips repos that are already clean (no fixable issues, no FAILs, not behind remote, only C-08 background noise allowed) - For repos that need work: git pull --ff-only then fix_repo() Prints a summary of CLEAN (skipped) and NOT ON THIS HOST (skipped) repos before the detailed fix reports. Simplifies the Makefile target from shell-level curl+git to a single uv run call using --remote. Same flag handles both single-repo and all-repos. Also adds _git_pull() helper and 13 new tests (71 total in consistency suite). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -252,19 +252,16 @@ fix-consistency:
|
||||
$(if $(REPO_PATH),--repo-path "$(REPO_PATH)",); \
|
||||
e=$$?; [ $$e -eq 2 ] && exit 0 || exit $$e
|
||||
|
||||
## Pull repo then fix consistency (safe for multi-machine workflows): make fix-consistency-remote REPO=net-kingdom
|
||||
## Pull then fix: single repo or all repos if REPO omitted
|
||||
## make fix-consistency-remote — smart pull+fix all repos that need it
|
||||
## make fix-consistency-remote REPO=slug — pull+fix one repo
|
||||
fix-consistency-remote:
|
||||
@test -n "$(REPO)" || (echo "ERROR: REPO is required. Usage: make fix-consistency-remote REPO=<slug>"; exit 1)
|
||||
$(eval _REMOTE_REPO_PATH := $(shell \
|
||||
curl -s $${API_BASE:-http://127.0.0.1:8000}/repos/?slug=$(REPO) | \
|
||||
python3 -c "import json,sys; \
|
||||
repos=json.load(sys.stdin); \
|
||||
print(next((r.get('local_path','') for r in repos if r['slug']=='$(REPO)'), ''))" \
|
||||
))
|
||||
@test -n "$(_REMOTE_REPO_PATH)" || (echo "ERROR: repo '$(REPO)' not found or has no local_path in state-hub"; exit 1)
|
||||
git -C "$(_REMOTE_REPO_PATH)" pull --ff-only || \
|
||||
(echo "WARN: pull failed (conflicts or no remote) — running fix-consistency anyway"; true)
|
||||
$(MAKE) fix-consistency REPO=$(REPO) REPO_PATH=$(_REMOTE_REPO_PATH)
|
||||
uv run python scripts/consistency_check.py \
|
||||
$(if $(REPO),--repo "$(REPO)",--all) \
|
||||
--remote \
|
||||
$(if $(API_BASE),--api-base "$(API_BASE)",) \
|
||||
$(if $(NO_WRITEBACK),--no-writeback,); \
|
||||
e=$$?; [ $$e -eq 2 ] && exit 0 || exit $$e
|
||||
|
||||
## Check all registered repos for ADR-001 consistency
|
||||
check-consistency-all:
|
||||
|
||||
@@ -747,6 +747,25 @@ def _check_ghost_duplicates(
|
||||
# Git helpers (T02 pull gate, T03 writeback)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _git_pull(repo_path: str) -> tuple[bool, str]:
|
||||
"""Run ``git pull --ff-only`` on *repo_path*.
|
||||
|
||||
Returns ``(success, message)`` where *message* describes the outcome.
|
||||
Never raises — errors are returned as ``(False, "<reason>")``.
|
||||
"""
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["git", "-C", repo_path, "pull", "--ff-only"],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
out = result.stdout.strip()
|
||||
return True, out or "already up to date"
|
||||
return False, result.stderr.strip() or "pull failed"
|
||||
except Exception as exc:
|
||||
return False, str(exc)
|
||||
|
||||
|
||||
def _detect_behind_remote(repo_path: str) -> bool:
|
||||
"""Return True if the remote tracking branch has commits the local repo lacks.
|
||||
|
||||
@@ -1078,6 +1097,92 @@ def fix_repo(
|
||||
return report
|
||||
|
||||
|
||||
# Check IDs that are known-background noise in multi-machine setups:
|
||||
# C-08 = completed/archived DB workstream with no file (pre-ADR-001 legacy)
|
||||
# These alone do not warrant a pull+fix cycle.
|
||||
_BACKGROUND_CHECKS: frozenset[str] = frozenset({"C-08"})
|
||||
|
||||
|
||||
def _report_needs_action(report: ConsistencyReport, behind_remote: bool) -> bool:
|
||||
"""Return True if the repo warrants a pull+fix cycle.
|
||||
|
||||
A repo is considered clean (no action needed) when:
|
||||
- It is not behind its remote tracking branch, AND
|
||||
- It has no FAIL issues, AND
|
||||
- Every WARN/INFO issue is in the background-noise set (C-08).
|
||||
"""
|
||||
if behind_remote:
|
||||
return True
|
||||
if report.failures:
|
||||
return True
|
||||
actionable_warns = [
|
||||
i for i in report.warnings + report.infos
|
||||
if i.check_id not in _BACKGROUND_CHECKS
|
||||
]
|
||||
return bool(actionable_warns)
|
||||
|
||||
|
||||
def fix_all_remote(
|
||||
api_base: str,
|
||||
no_writeback: bool = False,
|
||||
) -> list[ConsistencyReport]:
|
||||
"""Pull-then-fix all registered repos that need attention.
|
||||
|
||||
For each repo with a resolvable local path on this machine:
|
||||
1. Skip if the path does not exist (repo lives on another machine).
|
||||
2. Run check_repo() and detect_behind_remote() — read-only.
|
||||
3. If the repo is clean (no actionable issues, not behind remote): skip.
|
||||
4. Otherwise: git pull --ff-only, then fix_repo().
|
||||
|
||||
Returns one ConsistencyReport per repo that was *not* skipped as clean.
|
||||
Repos skipped as clean are printed to stdout but not included in the return value.
|
||||
"""
|
||||
repos = _api_get(api_base, "/repos")
|
||||
if not isinstance(repos, list):
|
||||
print("ERROR: Could not fetch repos from state-hub API", file=sys.stderr)
|
||||
return []
|
||||
|
||||
reports: list[ConsistencyReport] = []
|
||||
skipped_clean: list[str] = []
|
||||
skipped_missing: list[str] = []
|
||||
|
||||
for repo in repos:
|
||||
slug = repo["slug"]
|
||||
# Resolve path using the same priority as check_repo
|
||||
path = resolve_repo_path(repo)
|
||||
if not path or not Path(path).is_dir():
|
||||
skipped_missing.append(slug)
|
||||
continue
|
||||
|
||||
# Read-only pass: detect issues and remote staleness
|
||||
pre_report = check_repo(api_base, slug)
|
||||
behind = _detect_behind_remote(path)
|
||||
|
||||
if not _report_needs_action(pre_report, behind):
|
||||
skipped_clean.append(slug)
|
||||
continue
|
||||
|
||||
# Pull before fixing
|
||||
pull_ok, pull_msg = _git_pull(path)
|
||||
pull_tag = f"pull: {pull_msg}"
|
||||
if not pull_ok:
|
||||
pull_tag = f"pull WARN: {pull_msg} — proceeding anyway"
|
||||
|
||||
report = fix_repo(api_base, slug, no_writeback=no_writeback)
|
||||
report.fixes_applied.insert(0, pull_tag)
|
||||
reports.append(report)
|
||||
|
||||
# Print clean/missing summary lines before the detailed reports
|
||||
if skipped_clean:
|
||||
print(f" CLEAN (skipped): {', '.join(skipped_clean)}")
|
||||
if skipped_missing:
|
||||
print(f" NOT ON THIS HOST (skipped): {', '.join(skipped_missing)}")
|
||||
if skipped_clean or skipped_missing:
|
||||
print()
|
||||
|
||||
return reports
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Output / rendering
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1172,6 +1277,10 @@ def main() -> None:
|
||||
help="Run checks against all registered repos with local_path")
|
||||
parser.add_argument("--fix", action="store_true",
|
||||
help="Apply auto-fixable issues (status drift, repo mismatch, etc.)")
|
||||
parser.add_argument("--remote", action="store_true",
|
||||
help="Pull each repo before fixing; when used with --all, skips repos "
|
||||
"that are already clean (no actionable issues, not behind remote). "
|
||||
"Implies --fix.")
|
||||
parser.add_argument("--no-writeback", action="store_true", dest="no_writeback",
|
||||
help="Disable DB→file status writeback (C-15) while keeping other fixes")
|
||||
parser.add_argument("--repo-path", metavar="PATH", default=None,
|
||||
@@ -1183,30 +1292,50 @@ def main() -> None:
|
||||
help="Output JSON instead of human-readable text")
|
||||
args = parser.parse_args()
|
||||
|
||||
# Resolve repo list
|
||||
repo_slugs: list[str] = []
|
||||
if args.all:
|
||||
repos = _api_get(args.api_base, "/repos")
|
||||
if not isinstance(repos, list):
|
||||
print("ERROR: Could not fetch repos from state-hub API", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
repo_slugs = [r["slug"] for r in repos if r.get("local_path")]
|
||||
if not repo_slugs:
|
||||
print("No repos with local_path registered.", file=sys.stderr)
|
||||
no_wb = getattr(args, "no_writeback", False)
|
||||
do_fix = args.fix or args.remote
|
||||
|
||||
# --remote --all: smart pull+fix across all repos
|
||||
if args.remote and args.all:
|
||||
reports = fix_all_remote(args.api_base, no_writeback=no_wb)
|
||||
if not reports:
|
||||
sys.exit(0)
|
||||
else:
|
||||
repo_slugs = [args.repo]
|
||||
# Resolve repo list
|
||||
repo_slugs: list[str] = []
|
||||
if args.all:
|
||||
repos = _api_get(args.api_base, "/repos")
|
||||
if not isinstance(repos, list):
|
||||
print("ERROR: Could not fetch repos from state-hub API", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
repo_slugs = [r["slug"] for r in repos if r.get("local_path")]
|
||||
if not repo_slugs:
|
||||
print("No repos with local_path registered.", file=sys.stderr)
|
||||
sys.exit(0)
|
||||
else:
|
||||
repo_slugs = [args.repo]
|
||||
|
||||
# --repo-path only applies to single-repo runs; silently ignored with --all
|
||||
path_override = args.repo_path if not args.all else None
|
||||
if args.fix:
|
||||
no_wb = getattr(args, "no_writeback", False)
|
||||
reports = [
|
||||
fix_repo(args.api_base, slug, path_override, no_writeback=no_wb)
|
||||
for slug in repo_slugs
|
||||
]
|
||||
else:
|
||||
reports = [check_repo(args.api_base, slug, path_override) for slug in repo_slugs]
|
||||
# --repo-path only applies to single-repo runs; silently ignored with --all
|
||||
path_override = args.repo_path if not args.all else None
|
||||
|
||||
if args.remote:
|
||||
# Single-repo remote: pull first, then fix
|
||||
slug = repo_slugs[0]
|
||||
repo = _api_get(args.api_base, f"/repos/{slug}")
|
||||
path = resolve_repo_path(repo or {}, path_override) if repo else (path_override or "")
|
||||
if path:
|
||||
pull_ok, pull_msg = _git_pull(path)
|
||||
prefix = "pull" if pull_ok else "pull WARN"
|
||||
print(f" {prefix}: {pull_msg}")
|
||||
report = fix_repo(args.api_base, slug, path_override, no_writeback=no_wb)
|
||||
reports = [report]
|
||||
elif do_fix:
|
||||
reports = [
|
||||
fix_repo(args.api_base, slug, path_override, no_writeback=no_wb)
|
||||
for slug in repo_slugs
|
||||
]
|
||||
else:
|
||||
reports = [check_repo(args.api_base, slug, path_override) for slug in repo_slugs]
|
||||
|
||||
if args.as_json:
|
||||
output = (
|
||||
|
||||
@@ -25,8 +25,11 @@ from consistency_check import (
|
||||
Issue,
|
||||
FILE_TO_DB_WORKSTREAM_STATUS,
|
||||
STATUS_ORDER,
|
||||
_BACKGROUND_CHECKS,
|
||||
_detect_behind_remote,
|
||||
_git_pull,
|
||||
_patch_task_status_in_file,
|
||||
_report_needs_action,
|
||||
get_tasks_from_workplan,
|
||||
normalise_workstream_status,
|
||||
parse_frontmatter,
|
||||
@@ -462,7 +465,7 @@ class TestDetectBehindRemote:
|
||||
["git", "-C", str(clone), "commit", "--allow-empty", "-m", "init"],
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD:main"], capture_output=True)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD"], capture_output=True)
|
||||
# Add a local-only commit (not pushed)
|
||||
subprocess.run(
|
||||
["git", "-C", str(clone), "commit", "--allow-empty", "-m", "local only"],
|
||||
@@ -483,7 +486,7 @@ class TestDetectBehindRemote:
|
||||
["git", "-C", str(clone), "commit", "--allow-empty", "-m", "init"],
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD:main"], capture_output=True)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD"], capture_output=True)
|
||||
|
||||
# Add a commit directly in the bare repo (simulates remote-only progress)
|
||||
work = tmp_path / "work"
|
||||
@@ -554,9 +557,121 @@ class TestPatchTaskStatusInFile:
|
||||
assert "id: T01\nstatus: todo" in patched
|
||||
assert "id: T02\nstatus: done" in patched
|
||||
|
||||
def test_does_not_touch_non_status_fields(self, tmp_path):
|
||||
content = (
|
||||
"---\nid: WP\n---\n"
|
||||
"```task\nid: T01\nstatus: todo\npriority: high\nstate_hub_task_id: \"abc\"\n```\n"
|
||||
)
|
||||
f = self._make_workplan(tmp_path, content)
|
||||
_patch_task_status_in_file(f, "T01", "done")
|
||||
patched = f.read_text()
|
||||
assert "priority: high" in patched
|
||||
assert 'state_hub_task_id: "abc"' in patched
|
||||
|
||||
def test_idempotent_when_already_correct(self, tmp_path):
|
||||
content = "---\nid: WP\n---\n```task\nid: T01\nstatus: done\n```\n"
|
||||
f = self._make_workplan(tmp_path, content)
|
||||
result = _patch_task_status_in_file(f, "T01", "done")
|
||||
# status matches, re.sub produces same text → no write → False
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _git_pull (T02 remote fix helper)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestGitPull:
|
||||
def test_returns_false_for_nonexistent_path(self):
|
||||
ok, msg = _git_pull("/nonexistent/path")
|
||||
assert ok is False
|
||||
assert msg
|
||||
|
||||
def test_returns_true_for_up_to_date_repo(self, tmp_path):
|
||||
import subprocess
|
||||
bare = tmp_path / "bare.git"
|
||||
clone = tmp_path / "clone"
|
||||
bare.mkdir()
|
||||
subprocess.run(["git", "init", "--bare", str(bare)], capture_output=True)
|
||||
subprocess.run(["git", "clone", str(bare), str(clone)], capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(clone), "commit", "--allow-empty", "-m", "init"],
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD"], capture_output=True)
|
||||
ok, msg = _git_pull(str(clone))
|
||||
assert ok is True
|
||||
|
||||
def test_pulls_new_remote_commit(self, tmp_path):
|
||||
import subprocess
|
||||
bare = tmp_path / "bare.git"
|
||||
clone = tmp_path / "clone"
|
||||
work = tmp_path / "work"
|
||||
bare.mkdir()
|
||||
subprocess.run(["git", "init", "--bare", str(bare)], capture_output=True)
|
||||
subprocess.run(["git", "clone", str(bare), str(clone)], capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(clone), "commit", "--allow-empty", "-m", "init"],
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(["git", "-C", str(clone), "push", "origin", "HEAD"], capture_output=True)
|
||||
# Push a new commit from a second clone
|
||||
subprocess.run(["git", "clone", str(bare), str(work)], capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(work), "commit", "--allow-empty", "-m", "remote work"],
|
||||
capture_output=True,
|
||||
)
|
||||
subprocess.run(["git", "-C", str(work), "push"], capture_output=True)
|
||||
# Pull should succeed and bring in the new commit
|
||||
ok, msg = _git_pull(str(clone))
|
||||
assert ok is True
|
||||
head = subprocess.run(
|
||||
["git", "-C", str(clone), "log", "--oneline", "-1"],
|
||||
capture_output=True, text=True,
|
||||
).stdout
|
||||
assert "remote work" in head
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _report_needs_action (smart skip logic)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestReportNeedsAction:
|
||||
def _make_report(self, issues: list[tuple[str, str, bool]]) -> ConsistencyReport:
|
||||
"""Build a report from (severity, check_id, fixable) tuples."""
|
||||
r = ConsistencyReport(repo_slug="test", repo_path="/tmp/test")
|
||||
for severity, check_id, fixable in issues:
|
||||
r.add(severity=severity, check_id=check_id, message="test", fixable=fixable)
|
||||
return r
|
||||
|
||||
def test_clean_report_not_behind_is_skipped(self):
|
||||
r = self._make_report([])
|
||||
assert _report_needs_action(r, behind_remote=False) is False
|
||||
|
||||
def test_behind_remote_always_needs_action(self):
|
||||
r = self._make_report([])
|
||||
assert _report_needs_action(r, behind_remote=True) is True
|
||||
|
||||
def test_fail_always_needs_action(self):
|
||||
r = self._make_report([("FAIL", "C-07", False)])
|
||||
assert _report_needs_action(r, behind_remote=False) is True
|
||||
|
||||
def test_c08_only_is_background_noise(self):
|
||||
"""C-08 (legacy archived workstream) alone should not trigger a fix cycle."""
|
||||
r = self._make_report([("INFO", "C-08", False)])
|
||||
assert _report_needs_action(r, behind_remote=False) is False
|
||||
|
||||
def test_c08_plus_actionable_warn_needs_action(self):
|
||||
r = self._make_report([("INFO", "C-08", False), ("WARN", "C-10", True)])
|
||||
assert _report_needs_action(r, behind_remote=False) is True
|
||||
|
||||
def test_fixable_warn_needs_action(self):
|
||||
r = self._make_report([("WARN", "C-15", True)])
|
||||
assert _report_needs_action(r, behind_remote=False) is True
|
||||
|
||||
def test_non_fixable_warn_needs_action(self):
|
||||
"""Non-fixable WARNs (e.g. C-07 ghost) still warrant attention."""
|
||||
r = self._make_report([("WARN", "C-14", False)])
|
||||
assert _report_needs_action(r, behind_remote=False) is True
|
||||
|
||||
def test_background_checks_constant_contains_c08(self):
|
||||
assert "C-08" in _BACKGROUND_CHECKS
|
||||
|
||||
Reference in New Issue
Block a user