Do not record a no-op consistency sync as a fix.
A binding put that only refreshes the sync timestamp, a push that reports the remote is already current, and a C-11 skip on a finished workplan are not repairs. Leave those out of FIXES APPLIED so a clean second run stays clean. Assistant: grok Assistant-Session: 01a16071-5425-78d3-80b4-20fe2ee30f0a
This commit is contained in:
parent
64d698e65d
commit
bd6fd62b0a
5 changed files with 130 additions and 10 deletions
|
|
@ -2493,6 +2493,37 @@ def check_repo(
|
||||||
return report
|
return report
|
||||||
|
|
||||||
|
|
||||||
|
def _bindings_already_synced(index: Any, bindings: list[dict[str, Any]]) -> bool:
|
||||||
|
"""True when the hub index already stores each binding's file location."""
|
||||||
|
if not isinstance(index, dict) or "_error" in index:
|
||||||
|
return False
|
||||||
|
workplans = index.get("workplans")
|
||||||
|
if not isinstance(workplans, dict):
|
||||||
|
return False
|
||||||
|
for binding in bindings:
|
||||||
|
current = workplans.get(binding["workplan_id"])
|
||||||
|
if not isinstance(current, dict):
|
||||||
|
return False
|
||||||
|
if current.get("filename") != binding["filename"]:
|
||||||
|
return False
|
||||||
|
if current.get("relative_path") != binding["relative_path"]:
|
||||||
|
return False
|
||||||
|
if bool(current.get("archived")) != bool(binding["archived"]):
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def _record_push_seal(report: ConsistencyReport, ok: bool, msg: str) -> None:
|
||||||
|
"""Record a push that moved commits. An already-current remote is not a fix."""
|
||||||
|
if not ok:
|
||||||
|
report.fixes_applied.append(f"push WARN: {msg}")
|
||||||
|
return
|
||||||
|
lowered = msg.lower()
|
||||||
|
if "up-to-date" in lowered or "up to date" in lowered:
|
||||||
|
return
|
||||||
|
report.fixes_applied.append(f"push: {msg}")
|
||||||
|
|
||||||
|
|
||||||
def _sync_workplan_bindings(
|
def _sync_workplan_bindings(
|
||||||
api_base: str,
|
api_base: str,
|
||||||
repo_slug: str,
|
repo_slug: str,
|
||||||
|
|
@ -2540,10 +2571,16 @@ def _sync_workplan_bindings(
|
||||||
bindings.append(binding)
|
bindings.append(binding)
|
||||||
if not bindings:
|
if not bindings:
|
||||||
return
|
return
|
||||||
|
# A timestamp-only binding write is not drift. Skip it when the index
|
||||||
|
# already has the same filename, path, and archive flag. A failed lookup
|
||||||
|
# still falls through to the put.
|
||||||
|
current = _api_get(api_base, "/workplans/index", {"refresh": "true"})
|
||||||
|
if _bindings_already_synced(current, bindings):
|
||||||
|
return
|
||||||
result = _api_put(api_base, "/workplans/index/bindings", {"bindings": bindings})
|
result = _api_put(api_base, "/workplans/index/bindings", {"bindings": bindings})
|
||||||
if isinstance(result, dict) and "_error" in result:
|
if isinstance(result, dict) and "_error" in result:
|
||||||
report.fixes_applied.append(f"bindings WARN: {result['_error']}")
|
report.fixes_applied.append(f"bindings WARN: {result['_error']}")
|
||||||
elif isinstance(result, dict):
|
elif isinstance(result, dict) and int(result.get("updated") or 0) > 0:
|
||||||
report.fixes_applied.append(
|
report.fixes_applied.append(
|
||||||
f"bindings: synced {result.get('updated', 0)}/{result.get('received', len(bindings))}"
|
f"bindings: synced {result.get('updated', 0)}/{result.get('received', len(bindings))}"
|
||||||
)
|
)
|
||||||
|
|
@ -4172,9 +4209,9 @@ def fix_repo(
|
||||||
# Skip creating tasks for finished workstreams — the workstream is
|
# Skip creating tasks for finished workstreams — the workstream is
|
||||||
# done/archived so unlinked tasks are stale file artefacts, not gaps.
|
# done/archived so unlinked tasks are stale file artefacts, not gaps.
|
||||||
if normalise_workstream_status(ws_status) in CLOSED_WORKSTREAM_STATUSES:
|
if normalise_workstream_status(ws_status) in CLOSED_WORKSTREAM_STATUSES:
|
||||||
report.fixes_applied.append(
|
# The warn already names the unlinked task. Leaving it
|
||||||
f"C-11 skipped: task '{t_id}' in {ws_status} workstream — not created"
|
# uncreated is not a repair, so it is not a fix.
|
||||||
)
|
pass
|
||||||
else:
|
else:
|
||||||
t_status = normalise_task_status(task.get("status", "todo"))
|
t_status = normalise_task_status(task.get("status", "todo"))
|
||||||
t_priority = str(task.get("priority", "medium")).strip()
|
t_priority = str(task.get("priority", "medium")).strip()
|
||||||
|
|
@ -4358,10 +4395,7 @@ def fix_repo(
|
||||||
# timer idempotent and preventing commit pile-up.
|
# timer idempotent and preventing commit pile-up.
|
||||||
if repo_path:
|
if repo_path:
|
||||||
push_ok, push_msg = _git_push(repo_path)
|
push_ok, push_msg = _git_push(repo_path)
|
||||||
if push_ok:
|
_record_push_seal(report, push_ok, push_msg)
|
||||||
report.fixes_applied.append(f"push: {push_msg}")
|
|
||||||
else:
|
|
||||||
report.fixes_applied.append(f"push WARN: {push_msg}")
|
|
||||||
|
|
||||||
return report
|
return report
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -74,7 +74,12 @@ def push_ff(repo_path: str) -> tuple[bool, str]:
|
||||||
capture_output=True, text=True, timeout=PUSH_TIMEOUT_SECONDS,
|
capture_output=True, text=True, timeout=PUSH_TIMEOUT_SECONDS,
|
||||||
)
|
)
|
||||||
if result.returncode == 0:
|
if result.returncode == 0:
|
||||||
return True, result.stdout.strip() or "pushed"
|
detail = "\n".join(
|
||||||
|
part.strip() for part in (result.stdout, result.stderr) if part and part.strip()
|
||||||
|
)
|
||||||
|
if "everything up-to-date" in detail.lower():
|
||||||
|
return True, "Everything up-to-date"
|
||||||
|
return True, detail or "pushed"
|
||||||
return False, result.stderr.strip() or "push failed"
|
return False, result.stderr.strip() or "push failed"
|
||||||
except subprocess.TimeoutExpired:
|
except subprocess.TimeoutExpired:
|
||||||
return False, "push timed out; remote outcome unverified; inspect the remote ref before retrying"
|
return False, "push timed out; remote outcome unverified; inspect the remote ref before retrying"
|
||||||
|
|
|
||||||
|
|
@ -37,7 +37,10 @@ from consistency_check import (
|
||||||
_check_repo_manager_conformance,
|
_check_repo_manager_conformance,
|
||||||
_detect_behind_remote,
|
_detect_behind_remote,
|
||||||
_derived_work_record_uuid,
|
_derived_work_record_uuid,
|
||||||
|
_bindings_already_synced,
|
||||||
_git_pull,
|
_git_pull,
|
||||||
|
_record_push_seal,
|
||||||
|
_sync_workplan_bindings,
|
||||||
_patch_frontmatter_field,
|
_patch_frontmatter_field,
|
||||||
_patch_task_status_in_file,
|
_patch_task_status_in_file,
|
||||||
_report_needs_action,
|
_report_needs_action,
|
||||||
|
|
@ -532,6 +535,83 @@ class TestRepoLookupFailures:
|
||||||
assert report.fixes_applied == []
|
assert report.fixes_applied == []
|
||||||
|
|
||||||
|
|
||||||
|
class TestBindingSyncRecording:
|
||||||
|
def test_matching_index_is_already_synced(self):
|
||||||
|
binding = {
|
||||||
|
"workplan_id": "wp-1",
|
||||||
|
"filename": "MASON-WP-0015-construct-grant.md",
|
||||||
|
"relative_path": "workplans/MASON-WP-0015-construct-grant.md",
|
||||||
|
"archived": False,
|
||||||
|
}
|
||||||
|
index = {"workplans": {"wp-1": {
|
||||||
|
"filename": binding["filename"],
|
||||||
|
"relative_path": binding["relative_path"],
|
||||||
|
"archived": False,
|
||||||
|
}}}
|
||||||
|
assert _bindings_already_synced(index, [binding]) is True
|
||||||
|
|
||||||
|
def test_moved_file_is_not_already_synced(self):
|
||||||
|
binding = {
|
||||||
|
"workplan_id": "wp-1",
|
||||||
|
"filename": "MASON-WP-0015-construct-grant.md",
|
||||||
|
"relative_path": "workplans/archived/MASON-WP-0015-construct-grant.md",
|
||||||
|
"archived": True,
|
||||||
|
}
|
||||||
|
index = {"workplans": {"wp-1": {
|
||||||
|
"filename": binding["filename"],
|
||||||
|
"relative_path": "workplans/MASON-WP-0015-construct-grant.md",
|
||||||
|
"archived": False,
|
||||||
|
}}}
|
||||||
|
assert _bindings_already_synced(index, [binding]) is False
|
||||||
|
|
||||||
|
def test_unchanged_bindings_do_not_put_or_count_as_a_fix(self, tmp_path, monkeypatch):
|
||||||
|
workplan = tmp_path / "workplans" / "MASON-WP-0015-construct-grant.md"
|
||||||
|
workplan.parent.mkdir()
|
||||||
|
workplan.write_text("workplan")
|
||||||
|
binding_id = "384b3efc-f9a5-5ea9-b224-0ce900e79804"
|
||||||
|
puts = []
|
||||||
|
|
||||||
|
def fake_get(_api, _path, params=None, *, return_error=False):
|
||||||
|
return {"workplans": {binding_id: {
|
||||||
|
"filename": workplan.name,
|
||||||
|
"relative_path": "workplans/MASON-WP-0015-construct-grant.md",
|
||||||
|
"archived": False,
|
||||||
|
}}}
|
||||||
|
|
||||||
|
monkeypatch.setattr("consistency_check._api_get", fake_get)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"consistency_check._api_put",
|
||||||
|
lambda *args: puts.append(args) or {"updated": 1, "received": 1},
|
||||||
|
)
|
||||||
|
report = ConsistencyReport(repo_slug="ops-mason", repo_path=str(tmp_path))
|
||||||
|
_sync_workplan_bindings(
|
||||||
|
"http://127.0.0.1:8000",
|
||||||
|
"ops-mason",
|
||||||
|
[(workplan, {"state_hub_workstream_id": binding_id, "status": "finished"}, "")],
|
||||||
|
tmp_path,
|
||||||
|
report,
|
||||||
|
)
|
||||||
|
assert puts == []
|
||||||
|
assert report.fixes_applied == []
|
||||||
|
|
||||||
|
|
||||||
|
class TestPushSealRecording:
|
||||||
|
def test_up_to_date_push_is_not_a_fix(self):
|
||||||
|
report = ConsistencyReport(repo_slug="ops-mason", repo_path="/home/worsch/ops-mason")
|
||||||
|
_record_push_seal(report, True, "Everything up-to-date")
|
||||||
|
assert report.fixes_applied == []
|
||||||
|
|
||||||
|
def test_commit_push_is_a_fix(self):
|
||||||
|
report = ConsistencyReport(repo_slug="ops-mason", repo_path="/home/worsch/ops-mason")
|
||||||
|
_record_push_seal(report, True, "abcd..efgh main -> main")
|
||||||
|
assert report.fixes_applied == ["push: abcd..efgh main -> main"]
|
||||||
|
|
||||||
|
def test_failed_push_is_a_warning_fix(self):
|
||||||
|
report = ConsistencyReport(repo_slug="ops-mason", repo_path="/home/worsch/ops-mason")
|
||||||
|
_record_push_seal(report, False, "push failed")
|
||||||
|
assert report.fixes_applied == ["push WARN: push failed"]
|
||||||
|
|
||||||
|
|
||||||
class TestRenormalizationGuide:
|
class TestRenormalizationGuide:
|
||||||
def test_registry_contains_c23_rule(self):
|
def test_registry_contains_c23_rule(self):
|
||||||
rule = next(rule for rule in RENORMALIZATION_RULES if rule.check_id == "C-23")
|
rule = next(rule for rule in RENORMALIZATION_RULES if rule.check_id == "C-23")
|
||||||
|
|
|
||||||
|
|
@ -117,7 +117,7 @@ def test_cli_exact_source_omission_or_whole_batch_refusal(tmp_path,tamper,bound_
|
||||||
if tamper=='missing_scan':infos.pop(bound_index)
|
if tamper=='missing_scan':infos.pop(bound_index)
|
||||||
else:infos[bound_index]=(path,meta,'')
|
else:infos[bound_index]=(path,meta,'')
|
||||||
calls=[];report=actual_report()
|
calls=[];report=actual_report()
|
||||||
fn=extracted('scripts/consistency_check.py','_sync_workplan_bindings',dict(FROZEN_CLUSTER_BINDINGS=h.FROZEN_CLUSTER_BINDINGS,frozen_cluster_binding=h.frozen_cluster_binding,normalise_workstream_status=lambda s:s,workplan_display_path=lambda root,p:p.relative_to(root).as_posix(),_api_put=lambda *a:calls.append(a) or dict(updated=1,received=1)))
|
fn=extracted('scripts/consistency_check.py','_sync_workplan_bindings',dict(FROZEN_CLUSTER_BINDINGS=h.FROZEN_CLUSTER_BINDINGS,frozen_cluster_binding=h.frozen_cluster_binding,normalise_workstream_status=lambda s:s,workplan_display_path=lambda root,p:p.relative_to(root).as_posix(),_api_get=lambda *a, **k: None,_api_put=lambda *a:calls.append(a) or dict(updated=1,received=1),_bindings_already_synced=lambda index, bindings: False))
|
||||||
fn('http://unused','railiance-cluster',[(ordinary,dict(state_hub_workstream_id=str(uuid.uuid4()),status='active'),''),*infos],tmp_path,report)
|
fn('http://unused','railiance-cluster',[(ordinary,dict(state_hub_workstream_id=str(uuid.uuid4()),status='active'),''),*infos],tmp_path,report)
|
||||||
if tamper=='valid':assert len(calls)==1 and len(calls[0][2]['bindings'])==1
|
if tamper=='valid':assert len(calls)==1 and len(calls[0][2]['bindings'])==1
|
||||||
else:assert calls==[] and report.failures[0].check_id=='C-44' and report.automation_errors
|
else:assert calls==[] and report.failures[0].check_id=='C-44' and report.automation_errors
|
||||||
|
|
|
||||||
|
|
@ -245,6 +245,7 @@ class TestPushFf:
|
||||||
_, clone = git_pair
|
_, clone = git_pair
|
||||||
ok, msg = push_ff(str(clone))
|
ok, msg = push_ff(str(clone))
|
||||||
assert ok is True
|
assert ok is True
|
||||||
|
assert "up-to-date" in msg.lower()
|
||||||
|
|
||||||
def test_push_seal_invariant(self, git_pair):
|
def test_push_seal_invariant(self, git_pair):
|
||||||
"""After a successful push, count_local_ahead must be 0."""
|
"""After a successful push, count_local_ahead must be 0."""
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue