generated from coulomb/repo-seed
feat(consistency): T04 push seal — closed-loop writeback for automated commits
Root cause of the 501-commit pile-up in inter-hub: fix_repo() created
git commits (brief updates, T03 writebacks) but never pushed them, so
the 15-minute timer accumulated local commits indefinitely. Once real
development landed on remote the repos diverged with no self-healing path.
Changes
-------
repo_sync.py (new module)
Extracts all git lifecycle primitives: pull_ff, push_ff,
count_remote_ahead (C-16 input), count_local_ahead (C-17/T04 input).
Module docstring documents the push-seal invariant and stable state.
consistency_check.py
- Imports primitives from repo_sync; thin _detect_behind_remote wrapper
preserves backward compat for existing callers and tests.
- C-17 backlog guard: if local has unpushed commits from a prior failed
push, retry before making more; skip all writes if push still fails.
- T04 push seal: unconditional push_ff() at end of every fix_repo() run.
- _report_needs_action: ahead_of_remote param so repos with unpushed
backlogs are not silently skipped as "clean" by fix_all_remote().
- Domain-slug fallback: brief no longer degrades to "(unknown)" when all
workplans are completed — falls back to any workstream for domain context.
- Service switched from --all --fix to --remote --all (pulls before
fixing, skips already-clean repos).
push-seal.md (new)
Capability documentation: the problem, the invariant, all three checks
(C-16/C-17/T04), stable-state description, API reference, and test map.
test_repo_sync.py (new, 32 tests)
Full coverage of all four primitives via real git repos (tmp_path).
Includes C-17 scenario, push-seal invariant, and four end-to-end
loop-stability tests.
test_consistency_check.py
Four new _report_needs_action cases for the ahead_of_remote parameter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -21,6 +21,7 @@ Checks:
|
||||
C-14 ghost-duplicate WARN No Active topic workstream with no repo_id matches a file-backed title — probable ghost from premature create_workstream() call
|
||||
C-15 task-db-ahead WARN Yes DB task status is ahead of file — regression prevented; writeback syncs file
|
||||
C-16 repo-behind-remote WARN No Local repo is behind remote tracking branch — --fix skipped to avoid clobbering remote progress
|
||||
C-17 repo-ahead-push-failed WARN No Local repo has unpushed commits and push failed — writes skipped to prevent runaway divergence
|
||||
|
||||
Usage:
|
||||
python scripts/consistency_check.py --repo SLUG [--fix] [--no-writeback] [--json] [--api-base URL]
|
||||
@@ -826,52 +827,26 @@ def _check_ghost_duplicates(
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Git helpers (T02 pull gate, T03 writeback)
|
||||
# Git sync (T02–T04: pull gate, writeback, push seal) — see repo_sync.py
|
||||
# ---------------------------------------------------------------------------
|
||||
# repo_sync.py owns the push-seal invariant and all git lifecycle primitives.
|
||||
# The aliases below keep internal call sites and existing tests unchanged.
|
||||
|
||||
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)
|
||||
from repo_sync import ( # noqa: E402 (import after top-level imports intentional)
|
||||
count_local_ahead as _detect_ahead_of_remote,
|
||||
count_remote_ahead as _count_remote_ahead,
|
||||
pull_ff as _git_pull,
|
||||
push_ff as _git_push,
|
||||
)
|
||||
|
||||
|
||||
def _detect_behind_remote(repo_path: str) -> bool:
|
||||
"""Return True if the remote tracking branch has commits the local repo lacks.
|
||||
"""True if remote has commits the local repo lacks (C-16 predicate).
|
||||
|
||||
"Ahead" (local has unpushed commits) is NOT considered behind.
|
||||
"Diverged" is treated as behind (remote progress could be lost).
|
||||
Best-effort: returns False on any error (offline, no remote, etc.) so that
|
||||
check-only mode is never blocked by network issues.
|
||||
Delegates to repo_sync.count_remote_ahead, which fetches before counting.
|
||||
Returns False on any error so C-16 is never spuriously triggered.
|
||||
"""
|
||||
try:
|
||||
subprocess.run(
|
||||
["git", "-C", repo_path, "fetch", "--quiet", "origin"],
|
||||
capture_output=True, timeout=15,
|
||||
)
|
||||
# Count commits in remote that are not in local.
|
||||
# git rev-list HEAD..@{u} → commits remote has that local lacks.
|
||||
result = subprocess.run(
|
||||
["git", "-C", repo_path, "rev-list", "--count", "HEAD..@{u}"],
|
||||
capture_output=True, text=True, timeout=5,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return False # no upstream configured or other error
|
||||
return int(result.stdout.strip() or "0") > 0
|
||||
except Exception:
|
||||
return False
|
||||
return _count_remote_ahead(repo_path) > 0
|
||||
|
||||
|
||||
def _patch_task_status_in_file(
|
||||
@@ -970,10 +945,15 @@ def _write_custodian_brief(api_base: str, repo_slug: str, repo_path: str) -> boo
|
||||
repo_id: str = repo.get("id", "")
|
||||
domain_slug: str = ""
|
||||
|
||||
# Resolve domain slug via the topic linked to any workstream
|
||||
# Resolve domain slug: prefer active workstreams, fall back to any workstream
|
||||
# so that a fully-completed repo doesn't degrade to "(unknown)".
|
||||
workstreams = _api_get(api_base, "/workstreams", {"repo_id": repo_id, "status": "active"}) or []
|
||||
if isinstance(workstreams, list) and workstreams:
|
||||
topic = _api_get(api_base, f"/topics/{workstreams[0].get('topic_id', '')}")
|
||||
_ws_for_domain = workstreams if (isinstance(workstreams, list) and workstreams) else []
|
||||
if not _ws_for_domain:
|
||||
all_ws = _api_get(api_base, "/workstreams", {"repo_id": repo_id}) or []
|
||||
_ws_for_domain = all_ws if isinstance(all_ws, list) else []
|
||||
if _ws_for_domain:
|
||||
topic = _api_get(api_base, f"/topics/{_ws_for_domain[0].get('topic_id', '')}")
|
||||
if topic:
|
||||
domain_slug = topic.get("domain_slug", "")
|
||||
|
||||
@@ -1128,6 +1108,28 @@ def fix_repo(
|
||||
)
|
||||
return report
|
||||
|
||||
# C-17: backlog guard — if local has unpushed commits from a prior failed push,
|
||||
# try to push them before making more. Skipping writes prevents runaway divergence.
|
||||
if repo_path:
|
||||
ahead = _detect_ahead_of_remote(repo_path)
|
||||
if ahead > 0:
|
||||
push_ok, push_msg = _git_push(repo_path)
|
||||
if not push_ok:
|
||||
report.add(
|
||||
severity="WARN", check_id="C-17",
|
||||
message=(
|
||||
f"Repo '{repo_slug}' has {ahead} unpushed commit(s) and push "
|
||||
f"failed ({push_msg}) — skipping writes to prevent runaway divergence"
|
||||
),
|
||||
fixable=False,
|
||||
)
|
||||
report.fixes_applied.append(
|
||||
"C-17: all write operations skipped — unpushed commits, push failed"
|
||||
)
|
||||
return report
|
||||
# Push succeeded — local is now in sync; proceed normally
|
||||
report.fixes_applied.append(f"C-17 cleared: pushed {ahead} backlogged commit(s)")
|
||||
|
||||
fixable = [i for i in report.issues if i.fixable]
|
||||
|
||||
for issue in fixable:
|
||||
@@ -1339,6 +1341,16 @@ def fix_repo(
|
||||
if brief_written:
|
||||
report.fixes_applied.append("brief: .custodian-brief.md updated")
|
||||
|
||||
# Push all commits made this run (writebacks + brief) to close the loop.
|
||||
# This ensures the next run starts with local == remote, making the
|
||||
# timer idempotent and preventing commit pile-up.
|
||||
if repo_path:
|
||||
push_ok, push_msg = _git_push(repo_path)
|
||||
if push_ok:
|
||||
report.fixes_applied.append(f"push: {push_msg}")
|
||||
else:
|
||||
report.fixes_applied.append(f"push WARN: {push_msg}")
|
||||
|
||||
return report
|
||||
|
||||
|
||||
@@ -1348,15 +1360,18 @@ def fix_repo(
|
||||
_BACKGROUND_CHECKS: frozenset[str] = frozenset({"C-08"})
|
||||
|
||||
|
||||
def _report_needs_action(report: ConsistencyReport, behind_remote: bool) -> bool:
|
||||
def _report_needs_action(
|
||||
report: ConsistencyReport, behind_remote: bool, ahead_of_remote: int = 0
|
||||
) -> 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 unpushed local commits (from a prior failed push), AND
|
||||
- It has no FAIL issues, AND
|
||||
- Every WARN/INFO issue is in the background-noise set (C-08).
|
||||
"""
|
||||
if behind_remote:
|
||||
if behind_remote or ahead_of_remote > 0:
|
||||
return True
|
||||
if report.failures:
|
||||
return True
|
||||
@@ -1399,11 +1414,14 @@ def fix_all_remote(
|
||||
skipped_missing.append(slug)
|
||||
continue
|
||||
|
||||
# Read-only pass: detect issues and remote staleness
|
||||
# Read-only pass: detect issues and remote staleness.
|
||||
# _detect_behind_remote does a fetch, so _detect_ahead_of_remote
|
||||
# after it sees an up-to-date tracking branch.
|
||||
pre_report = check_repo(api_base, slug)
|
||||
behind = _detect_behind_remote(path)
|
||||
ahead = _detect_ahead_of_remote(path)
|
||||
|
||||
if not _report_needs_action(pre_report, behind):
|
||||
if not _report_needs_action(pre_report, behind, ahead):
|
||||
skipped_clean.append(slug)
|
||||
continue
|
||||
|
||||
|
||||
Reference in New Issue
Block a user