fix: safe incident_links migration order; require PR head pin
Address remaining PR #619 blockers on head 783ea88:
- Deduplicate legacy NULL-scope incident_links before normalizing to ''
- Fail closed when duplicate rows disagree on Gitea target
- Require non-empty expected_head_sha for PR assign and mutation
This commit is contained in:
+70
-17
@@ -318,33 +318,75 @@ class ControlPlaneDB:
|
||||
conn.close()
|
||||
|
||||
def _migrate_incident_links_null_scope(self, conn: sqlite3.Connection) -> None:
|
||||
"""Coerce NULL scope columns to '' and collapse any duplicate keys (#619 RC)."""
|
||||
"""Collapse legacy NULL-scope duplicates, then normalize to '' (#619 RC2).
|
||||
|
||||
Order is mandatory: SQLite UNIQUE treats multiple NULLs as distinct, so
|
||||
normalizing NULL→'' first can hit the UNIQUE constraint and abort
|
||||
migration. Deduplicate under the *normalized* key first, fail closed if
|
||||
Gitea targets conflict within a group, then coerce NULLs to ''.
|
||||
"""
|
||||
cols = {
|
||||
row[1]
|
||||
for row in conn.execute("PRAGMA table_info(incident_links)").fetchall()
|
||||
}
|
||||
if not cols:
|
||||
return
|
||||
|
||||
rows = conn.execute(
|
||||
"""
|
||||
SELECT link_id, provider,
|
||||
IFNULL(provider_base_url, '') AS base_url,
|
||||
IFNULL(provider_org, '') AS p_org,
|
||||
IFNULL(provider_project, '') AS p_project,
|
||||
provider_issue_id,
|
||||
gitea_org, gitea_repo, gitea_issue_number
|
||||
FROM incident_links
|
||||
ORDER BY link_id ASC
|
||||
"""
|
||||
).fetchall()
|
||||
|
||||
groups: dict[tuple[str, str, str, str, str], list[sqlite3.Row]] = {}
|
||||
for row in rows:
|
||||
key = (
|
||||
str(row["provider"]),
|
||||
str(row["base_url"]),
|
||||
str(row["p_org"]),
|
||||
str(row["p_project"]),
|
||||
str(row["provider_issue_id"]),
|
||||
)
|
||||
groups.setdefault(key, []).append(row)
|
||||
|
||||
to_delete: list[int] = []
|
||||
for key, members in groups.items():
|
||||
if len(members) < 2:
|
||||
continue
|
||||
# Canonical identity for observation links: Gitea issue target.
|
||||
targets = {
|
||||
(
|
||||
str(m["gitea_org"] or ""),
|
||||
str(m["gitea_repo"] or ""),
|
||||
int(m["gitea_issue_number"]),
|
||||
)
|
||||
for m in members
|
||||
}
|
||||
if len(targets) > 1:
|
||||
raise ControlPlaneError(
|
||||
"incident_links migration fail closed: conflicting Gitea "
|
||||
f"targets for provider key {key!r}: {sorted(targets)}"
|
||||
)
|
||||
keep_id = int(members[0]["link_id"]) # lowest link_id (ORDER BY)
|
||||
for m in members[1:]:
|
||||
to_delete.append(int(m["link_id"]))
|
||||
|
||||
for link_id in to_delete:
|
||||
conn.execute("DELETE FROM incident_links WHERE link_id = ?", (link_id,))
|
||||
|
||||
# Safe only after dedupe: normalize NULL scope fields to empty string.
|
||||
for col in ("provider_base_url", "provider_org", "provider_project"):
|
||||
if col in cols:
|
||||
conn.execute(
|
||||
f"UPDATE incident_links SET {col} = '' WHERE {col} IS NULL"
|
||||
)
|
||||
# Drop duplicates keeping lowest link_id (canonical row).
|
||||
conn.execute(
|
||||
"""
|
||||
DELETE FROM incident_links
|
||||
WHERE link_id NOT IN (
|
||||
SELECT MIN(link_id)
|
||||
FROM incident_links
|
||||
GROUP BY provider,
|
||||
IFNULL(provider_base_url, ''),
|
||||
IFNULL(provider_org, ''),
|
||||
IFNULL(provider_project, ''),
|
||||
provider_issue_id
|
||||
)
|
||||
"""
|
||||
)
|
||||
|
||||
# ── sessions ──────────────────────────────────────────────────────────
|
||||
|
||||
@@ -481,6 +523,11 @@ class ControlPlaneDB:
|
||||
raise InvalidWorkKindError(
|
||||
f"cannot assign kind '{kind}'; only {sorted(WORK_KINDS)}"
|
||||
)
|
||||
head_pin = (expected_head_sha or "").strip()
|
||||
if kind_norm == "pr" and not head_pin:
|
||||
raise LeaseRequiredError(
|
||||
f"pr#{int(number)} assignment requires non-empty expected_head_sha pin"
|
||||
)
|
||||
|
||||
allowed = tuple(allowed_actions or ("implement", "comment"))
|
||||
forbidden = tuple(
|
||||
@@ -825,9 +872,15 @@ class ControlPlaneDB:
|
||||
exp = _parse_ts(asn["expires_at"])
|
||||
if exp and exp <= _utc_now():
|
||||
raise LeaseRequiredError(f"lease {asn['lease_id']} expired")
|
||||
# Stale-head: pinned expected_head_sha must still match live work item.
|
||||
# Head pin: PRs require a non-empty expected_head_sha (fail closed).
|
||||
assigned_head = (asn["expected_head_sha"] or "").strip()
|
||||
current_head = (work["current_head_sha"] or "").strip()
|
||||
if kind_norm == "pr" and not assigned_head:
|
||||
raise LeaseRequiredError(
|
||||
f"assignment {asn['assignment_id']} for pr#{number} has no "
|
||||
f"expected_head_sha pin; PR mutations require a head pin"
|
||||
)
|
||||
# Stale-head: pinned expected_head_sha must still match live work item.
|
||||
if assigned_head and assigned_head != current_head:
|
||||
raise LeaseRequiredError(
|
||||
f"assignment {asn['assignment_id']} stale head: expected "
|
||||
|
||||
Reference in New Issue
Block a user