fix(classification): harden registration updates
Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a02b22-9638-76d2-bbff-b7ea1770b118
This commit is contained in:
parent
fb79d0d68d
commit
57c3e08103
7 changed files with 152 additions and 14 deletions
|
|
@ -11,6 +11,7 @@
|
||||||
| workplan | ADHOC-2026-06-04 | finished | — | workplans/ADHOC-2026-06-04.md |
|
| workplan | ADHOC-2026-06-04 | finished | — | workplans/ADHOC-2026-06-04.md |
|
||||||
| workplan | ADHOC-2026-07-01 | finished | — | workplans/ADHOC-2026-07-01.md |
|
| workplan | ADHOC-2026-07-01 | finished | — | workplans/ADHOC-2026-07-01.md |
|
||||||
| workplan | ADHOC-2026-08-08 | finished | — | workplans/ADHOC-2026-08-08.md |
|
| workplan | ADHOC-2026-08-08 | finished | — | workplans/ADHOC-2026-08-08.md |
|
||||||
|
| workplan | ADHOC-2026-08-23 | finished | — | workplans/ADHOC-2026-08-23.md |
|
||||||
| workplan | CUST-WP-0003 | finished | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
| workplan | CUST-WP-0003 | finished | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
||||||
| workplan | CUST-WP-0012 | finished | — | workplans/CUST-WP-0012-multi-user-onboarding.md |
|
| workplan | CUST-WP-0012 | finished | — | workplans/CUST-WP-0012-multi-user-onboarding.md |
|
||||||
| workplan | CUST-WP-0038 | backlog | — | workplans/CUST-WP-0038-state-hub-threephoenix-ha.md |
|
| workplan | CUST-WP-0038 | backlog | — | workplans/CUST-WP-0038-state-hub-threephoenix-ha.md |
|
||||||
|
|
@ -59,6 +60,7 @@
|
||||||
| task | ADHOC-2026-08-08-T02 | done | — | workplans/ADHOC-2026-08-08.md |
|
| task | ADHOC-2026-08-08-T02 | done | — | workplans/ADHOC-2026-08-08.md |
|
||||||
| task | ADHOC-2026-08-08-T03 | done | — | workplans/ADHOC-2026-08-08.md |
|
| task | ADHOC-2026-08-08-T03 | done | — | workplans/ADHOC-2026-08-08.md |
|
||||||
| task | ADHOC-2026-08-08-T04 | done | — | workplans/ADHOC-2026-08-08.md |
|
| task | ADHOC-2026-08-08-T04 | done | — | workplans/ADHOC-2026-08-08.md |
|
||||||
|
| task | ADHOC-2026-08-23-T01 | done | — | workplans/ADHOC-2026-08-23.md |
|
||||||
| task | CUST-WP-0003-T01 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
| task | CUST-WP-0003-T01 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
||||||
| task | CUST-WP-0003-T02 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
| task | CUST-WP-0003-T02 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
||||||
| task | CUST-WP-0003-T03 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
| task | CUST-WP-0003-T03 | done | — | workplans/CUST-WP-0003-whi-kpi-card.md |
|
||||||
|
|
|
||||||
|
|
@ -604,10 +604,27 @@ async def update_repo_with_classification(
|
||||||
"""Patch repo metadata including classification spine fields."""
|
"""Patch repo metadata including classification spine fields."""
|
||||||
repo = await _get_repo_by_slug(slug, session)
|
repo = await _get_repo_by_slug(slug, session)
|
||||||
payload = body.model_dump(exclude_unset=True)
|
payload = body.model_dump(exclude_unset=True)
|
||||||
domain_result = await session.execute(select(Domain).where(Domain.id == repo.domain_id))
|
requested_domain_slug = payload.pop("domain_slug", None)
|
||||||
domain_obj = domain_result.scalar_one_or_none()
|
if requested_domain_slug is not None:
|
||||||
|
domain_result = await session.execute(
|
||||||
|
select(Domain).where(Domain.slug == requested_domain_slug)
|
||||||
|
)
|
||||||
|
domain_obj = domain_result.scalar_one_or_none()
|
||||||
|
if domain_obj is None:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=404,
|
||||||
|
detail=f"Domain '{requested_domain_slug}' not found",
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
domain_result = await session.execute(
|
||||||
|
select(Domain).where(Domain.id == repo.domain_id)
|
||||||
|
)
|
||||||
|
domain_obj = domain_result.scalar_one_or_none()
|
||||||
domain_slug = domain_obj.slug if domain_obj else ""
|
domain_slug = domain_obj.slug if domain_obj else ""
|
||||||
if classification_fields_set(payload):
|
classification_requested = classification_fields_set(payload)
|
||||||
|
if classification_requested or (
|
||||||
|
requested_domain_slug is not None and repo.category is not None
|
||||||
|
):
|
||||||
merged = {
|
merged = {
|
||||||
"category": payload.get("category", repo.category),
|
"category": payload.get("category", repo.category),
|
||||||
"secondary_domains": payload.get("secondary_domains", repo.secondary_domains),
|
"secondary_domains": payload.get("secondary_domains", repo.secondary_domains),
|
||||||
|
|
@ -620,6 +637,8 @@ async def update_repo_with_classification(
|
||||||
fields=merged,
|
fields=merged,
|
||||||
require_complete=True,
|
require_complete=True,
|
||||||
)
|
)
|
||||||
|
if requested_domain_slug is not None:
|
||||||
|
repo.domain_id = domain_obj.id
|
||||||
for field, value in payload.items():
|
for field, value in payload.items():
|
||||||
setattr(repo, field, value)
|
setattr(repo, field, value)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
|
||||||
|
|
@ -80,6 +80,7 @@ class RepoCreate(CoreRepoCreate, ClassificationFields):
|
||||||
|
|
||||||
|
|
||||||
class RepoUpdate(ClassificationFields):
|
class RepoUpdate(ClassificationFields):
|
||||||
|
domain_slug: str | None = None
|
||||||
name: str | None = None
|
name: str | None = None
|
||||||
local_path: str | None = None
|
local_path: str | None = None
|
||||||
remote_url: str | None = None
|
remote_url: str | None = None
|
||||||
|
|
@ -176,4 +177,4 @@ class RepoScopeHealth(BaseModel):
|
||||||
local_path: str | None = None
|
local_path: str | None = None
|
||||||
path_available: bool
|
path_available: bool
|
||||||
scope_needs_review: bool
|
scope_needs_review: bool
|
||||||
scope_issue_details: list[ScopeIssueDetail]
|
scope_issue_details: list[ScopeIssueDetail]
|
||||||
|
|
|
||||||
|
|
@ -136,6 +136,21 @@ def _git_root(path: Path) -> Path:
|
||||||
return Path(root) if root else path.resolve()
|
return Path(root) if root else path.resolve()
|
||||||
|
|
||||||
|
|
||||||
|
def _git_fingerprint(repo_path: Path) -> str | None:
|
||||||
|
"""Return the same single-root fingerprint used by token attribution.
|
||||||
|
|
||||||
|
A repository can contain multiple unrelated root commits. ``git rev-list``
|
||||||
|
emits one SHA per line in that case, while ``managed_repos.git_fingerprint``
|
||||||
|
stores one 40-character SHA. Token attribution already uses the first root,
|
||||||
|
so registration must use the same projection rather than sending a multiline
|
||||||
|
value to the API/database.
|
||||||
|
"""
|
||||||
|
roots = _git_value(repo_path, ["rev-list", "--max-parents=0", "HEAD"])
|
||||||
|
if not roots:
|
||||||
|
return None
|
||||||
|
return roots.splitlines()[0].strip() or None
|
||||||
|
|
||||||
|
|
||||||
def _resolve_repo_path_for_host(repo: ManagedRepo) -> str | None:
|
def _resolve_repo_path_for_host(repo: ManagedRepo) -> str | None:
|
||||||
hostname = socket.gethostname()
|
hostname = socket.gethostname()
|
||||||
host_paths = repo.host_paths or {}
|
host_paths = repo.host_paths or {}
|
||||||
|
|
@ -226,7 +241,7 @@ async def _upsert_via_db(
|
||||||
) -> None:
|
) -> None:
|
||||||
git_root = _git_root(repo_path)
|
git_root = _git_root(repo_path)
|
||||||
remote_url = _git_value(git_root, ["remote", "get-url", "origin"])
|
remote_url = _git_value(git_root, ["remote", "get-url", "origin"])
|
||||||
git_fingerprint = _git_value(git_root, ["rev-list", "--max-parents=0", "HEAD"])
|
git_fingerprint = _git_fingerprint(git_root)
|
||||||
hostname = socket.gethostname()
|
hostname = socket.gethostname()
|
||||||
display_name = git_root.name.replace("-", " ").replace("_", " ").title()
|
display_name = git_root.name.replace("-", " ").replace("_", " ").title()
|
||||||
|
|
||||||
|
|
@ -345,7 +360,7 @@ async def _upsert_via_api(
|
||||||
) -> None:
|
) -> None:
|
||||||
git_root = _git_root(repo_path)
|
git_root = _git_root(repo_path)
|
||||||
remote_url = _git_value(git_root, ["remote", "get-url", "origin"])
|
remote_url = _git_value(git_root, ["remote", "get-url", "origin"])
|
||||||
git_fingerprint = _git_value(git_root, ["rev-list", "--max-parents=0", "HEAD"])
|
git_fingerprint = _git_fingerprint(git_root)
|
||||||
hostname = socket.gethostname()
|
hostname = socket.gethostname()
|
||||||
display_name = git_root.name.replace("-", " ").replace("_", " ").title()
|
display_name = git_root.name.replace("-", " ").replace("_", " ").title()
|
||||||
|
|
||||||
|
|
@ -363,7 +378,7 @@ async def _upsert_via_api(
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
patch_body = {
|
classification_body = {
|
||||||
"category": data.category,
|
"category": data.category,
|
||||||
"secondary_domains": data.secondary_domains,
|
"secondary_domains": data.secondary_domains,
|
||||||
"capability_tags": data.capability_tags,
|
"capability_tags": data.capability_tags,
|
||||||
|
|
@ -373,9 +388,6 @@ async def _upsert_via_api(
|
||||||
"classified_by": data.classified_by,
|
"classified_by": data.classified_by,
|
||||||
"standard_version": data.standard_version,
|
"standard_version": data.standard_version,
|
||||||
"domain_slug": data.domain,
|
"domain_slug": data.domain,
|
||||||
"local_path": str(git_root),
|
|
||||||
"remote_url": remote_url,
|
|
||||||
"git_fingerprint": git_fingerprint,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if existing is None:
|
if existing is None:
|
||||||
|
|
@ -404,7 +416,7 @@ async def _upsert_via_api(
|
||||||
report.add(RowResult(slug, str(git_root), "invalid", f"POST failed: {detail}"))
|
report.add(RowResult(slug, str(git_root), "invalid", f"POST failed: {detail}"))
|
||||||
return
|
return
|
||||||
code, updated = _api_request(
|
code, updated = _api_request(
|
||||||
"PATCH", f"/repos/{slug}", api_base=api_base, body=patch_body
|
"PATCH", f"/repos/{slug}", api_base=api_base, body=classification_body
|
||||||
)
|
)
|
||||||
if code != 200:
|
if code != 200:
|
||||||
detail = updated.get("detail", updated) if isinstance(updated, dict) else updated
|
detail = updated.get("detail", updated) if isinstance(updated, dict) else updated
|
||||||
|
|
@ -432,7 +444,7 @@ async def _upsert_via_api(
|
||||||
return
|
return
|
||||||
|
|
||||||
code, updated = _api_request(
|
code, updated = _api_request(
|
||||||
"PATCH", f"/repos/{slug}", api_base=api_base, body=patch_body
|
"PATCH", f"/repos/{slug}", api_base=api_base, body=classification_body
|
||||||
)
|
)
|
||||||
if code != 200:
|
if code != 200:
|
||||||
detail = updated.get("detail", updated) if isinstance(updated, dict) else updated
|
detail = updated.get("detail", updated) if isinstance(updated, dict) else updated
|
||||||
|
|
@ -632,4 +644,4 @@ def main(argv: list[str] | None = None) -> int:
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
raise SystemExit(main())
|
raise SystemExit(main())
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,8 @@ from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
from api.classification import ClassificationData
|
||||||
|
|
||||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
SCRIPT = REPO_ROOT / "scripts" / "register_from_classification.py"
|
SCRIPT = REPO_ROOT / "scripts" / "register_from_classification.py"
|
||||||
|
|
||||||
|
|
@ -68,4 +70,58 @@ def test_json_report_shape():
|
||||||
assert payload["summary"]["invalid"] == 0
|
assert payload["summary"]["invalid"] == 0
|
||||||
assert "summary" in payload
|
assert "summary" in payload
|
||||||
assert "results" in payload
|
assert "results" in payload
|
||||||
assert set(payload["summary"]) == {"registered", "updated", "skipped", "invalid"}
|
assert set(payload["summary"]) == {"registered", "updated", "skipped", "invalid"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_fingerprint_uses_one_root(monkeypatch, tmp_path):
|
||||||
|
from scripts import register_from_classification as registration
|
||||||
|
|
||||||
|
first = "a" * 40
|
||||||
|
second = "b" * 40
|
||||||
|
monkeypatch.setattr(
|
||||||
|
registration,
|
||||||
|
"_git_value",
|
||||||
|
lambda _path, _args: f"{first}\n{second}",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert registration._git_fingerprint(tmp_path) == first
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_api_update_separates_classification_from_identity(monkeypatch, tmp_path):
|
||||||
|
from scripts import register_from_classification as registration
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def fake_api_request(method, path, *, api_base, body=None):
|
||||||
|
calls.append((method, path, body))
|
||||||
|
if method == "GET":
|
||||||
|
return 200, {"slug": "example"}
|
||||||
|
return 200, {"slug": "example"}
|
||||||
|
|
||||||
|
monkeypatch.setattr(registration, "_api_request", fake_api_request)
|
||||||
|
monkeypatch.setattr(registration, "_git_root", lambda path: path)
|
||||||
|
monkeypatch.setattr(registration, "_git_value", lambda _path, _args: "origin")
|
||||||
|
monkeypatch.setattr(registration, "_git_fingerprint", lambda _path: "a" * 40)
|
||||||
|
report = registration.RegistrationReport()
|
||||||
|
data = ClassificationData(
|
||||||
|
category="infrastructure",
|
||||||
|
domain="infotech",
|
||||||
|
capability_tags=["infrastructure.state"],
|
||||||
|
)
|
||||||
|
|
||||||
|
await registration._upsert_via_api(
|
||||||
|
slug="example",
|
||||||
|
repo_path=tmp_path,
|
||||||
|
data=data,
|
||||||
|
dry_run=False,
|
||||||
|
api_base="http://state-hub.test",
|
||||||
|
report=report,
|
||||||
|
)
|
||||||
|
|
||||||
|
patch = next(call for call in calls if call[:2] == ("PATCH", "/repos/example"))
|
||||||
|
assert patch[2]["domain_slug"] == "infotech"
|
||||||
|
assert "local_path" not in patch[2]
|
||||||
|
assert "remote_url" not in patch[2]
|
||||||
|
assert "git_fingerprint" not in patch[2]
|
||||||
|
assert ("POST", "/repos/example/paths") in [call[:2] for call in calls]
|
||||||
|
|
|
||||||
|
|
@ -108,6 +108,29 @@ class TestRepos:
|
||||||
assert r.status_code == 201
|
assert r.status_code == 201
|
||||||
assert r.json()["host_paths"] == {"workstation": "/srv/hosted-repo"}
|
assert r.json()["host_paths"] == {"workstation": "/srv/hosted-repo"}
|
||||||
|
|
||||||
|
async def test_patch_reclassifies_primary_domain(self, client):
|
||||||
|
await _create_domain(client, slug="infotech", name="Infotech")
|
||||||
|
await _create_domain(client, slug="financials", name="Financials")
|
||||||
|
await _create_repo(client, domain_slug="infotech", slug="reclassified-repo")
|
||||||
|
|
||||||
|
r = await client.patch(
|
||||||
|
"/repos/reclassified-repo",
|
||||||
|
json={
|
||||||
|
"domain_slug": "financials",
|
||||||
|
"category": "product",
|
||||||
|
"secondary_domains": [],
|
||||||
|
"capability_tags": ["accounting"],
|
||||||
|
"business_stake": [],
|
||||||
|
"business_mechanics": [],
|
||||||
|
"classified_at": "2026-08-23",
|
||||||
|
"classified_by": "human",
|
||||||
|
"standard_version": "1.0",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
assert r.json()["domain_slug"] == "financials"
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Topic tests
|
# Topic tests
|
||||||
|
|
|
||||||
25
workplans/ADHOC-2026-08-23.md
Normal file
25
workplans/ADHOC-2026-08-23.md
Normal file
|
|
@ -0,0 +1,25 @@
|
||||||
|
---
|
||||||
|
id: ADHOC-2026-08-23
|
||||||
|
type: workplan
|
||||||
|
title: "Ad hoc fixes — 2026-08-23"
|
||||||
|
domain: infotech
|
||||||
|
repo: state-hub
|
||||||
|
status: finished
|
||||||
|
owner: codex
|
||||||
|
topic_slug: state-hub
|
||||||
|
created: "2026-08-23"
|
||||||
|
updated: "2026-08-23"
|
||||||
|
---
|
||||||
|
|
||||||
|
## Repair classification registration updates
|
||||||
|
|
||||||
|
```task
|
||||||
|
id: ADHOC-2026-08-23-T01
|
||||||
|
status: done
|
||||||
|
priority: high
|
||||||
|
```
|
||||||
|
|
||||||
|
Keep existing-repo classification PATCHes separate from Git identity metadata,
|
||||||
|
support primary-domain reclassification through the API, and normalize
|
||||||
|
multi-root Git histories to the same single-root fingerprint used by token
|
||||||
|
attribution. Add regression coverage for each affected contract.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue