fix(sop): add engineers to root-cause and no-backwards-compat on main #1423

Closed
core-be wants to merge 3 commits from fix/sop-engineers-main into main
3 changed files with 56 additions and 14 deletions
+32 -6
View File
@@ -144,6 +144,16 @@ def parse_directives(
if not parts:
continue
first = parts[0]
# Em-dash (U+2014) is a common visual separator in user-written
# notes, e.g. /sop-ack Five-Axis — five-axis-review
# If raw_slug contains an em-dash, split on the first one so
# the part before becomes the slug and the rest becomes the note.
note_from_slug = ""
slug_source = raw_slug
emdash_idx = raw_slug.find("")
if emdash_idx != -1:
slug_source = raw_slug[:emdash_idx].strip()
note_from_slug = raw_slug[emdash_idx + 1 :].strip()
# If the slug-capture greedily matched multiple words (e.g.
# "comprehensive testing"), preserve normalize behavior: join
# the WHOLE first-word-token only; trailing words get appended to
@@ -156,13 +166,14 @@ def parse_directives(
# as slug and "testing extra-note" as note. We defer the
# disambiguation to the caller via the returned canonical
# slug. For simplicity: try the WHOLE captured string first.
canonical = normalize_slug(raw_slug, numeric_aliases)
canonical = normalize_slug(slug_source, numeric_aliases)
else:
canonical = normalize_slug(first, numeric_aliases)
canonical = normalize_slug(slug_source, numeric_aliases)
note_from_group = (m.group(3) or "").strip()
# If we collapsed multi-word slug into kebab and there's a
# trailing-text group too, append it.
entry = (kind, canonical, note_from_group)
# Combine note_from_slug (em-dash split) with note_from_group
# (trailing text after the slug captured by the regex group).
combined_note = (note_from_slug + " " + note_from_group).strip()
entry = (kind, canonical, combined_note)
if kind == "sop-n/a":
na_directives.append(entry)
else:
@@ -831,7 +842,22 @@ def main(argv: list[str] | None = None) -> int:
team_member_cache: dict[tuple[str, int], bool | None] = {}
def probe(slug: str, users: list[str]) -> list[str]:
item = items_by_slug[slug]
# Slugs can be either checklist item names (from items_by_slug) or
# gate names (from na_gates). compute_na_state passes gate names
# (e.g. "qa-review", "security-review") to probe, so we must look
# them up in na_gates as a fallback.
if slug in items_by_slug:
item = items_by_slug[slug]
elif slug in na_gates:
item = na_gates[slug]
else:
# Unknown slug — fail closed.
print(
f"::warning::probe received unknown slug '{slug}'"
"returning no approved users (fail-closed)",
file=sys.stderr,
)
return []
team_names: list[str] = item["required_teams"]
# Resolve names → ids. NOTE: orgs/{org}/teams/search may not be
# available — fall back to the list endpoint.
@@ -209,6 +209,22 @@ class TestParseDirectives(unittest.TestCase):
d = self.parse_ack_revoke("/sop-ack Comprehensive_Testing")
self.assertEqual(d[0][1], "comprehensive-testing")
def test_emdash_separator_parsed_correctly(self):
# Em-dash (U+2014) between slug and note is common in practice.
# /sop-ack Five-Axis — five-axis-review
# → slug = five-axis, note = — five-axis-review
d = self.parse_ack_revoke("/sop-ack Five-Axis — five-axis-review")
self.assertEqual(len(d), 1)
self.assertEqual(d[0][1], "five-axis")
self.assertIn("five-axis-review", d[0][2])
def test_emdash_no_note(self):
# Em-dash at end of slug: only slug, no note content
d = self.parse_ack_revoke("/sop-ack Five-Axis —")
self.assertEqual(len(d), 1)
self.assertEqual(d[0][1], "five-axis")
self.assertEqual(d[0][2], "") # em-dash preserved as note
# ---------------------------------------------------------------------------
# section_marker_present
+8 -8
View File
@@ -78,11 +78,11 @@ items:
- slug: root-cause
numeric_alias: 4
pr_section_marker: "Root-cause not symptom"
required_teams: [managers, ceo]
required_teams: [managers, ceo, engineers]
description: >-
One-sentence root-cause statement. Ack from managers tier
(team-leads) or ceo. Senior judgment required to attest
root-cause-versus-symptom.
(team-leads), ceo, or any senior engineer. Senior judgment
required to attest root-cause-versus-symptom.
- slug: five-axis-review
numeric_alias: 5
@@ -95,10 +95,10 @@ items:
- slug: no-backwards-compat
numeric_alias: 6
pr_section_marker: "No backwards-compat shim / dead code added"
required_teams: [managers, ceo]
required_teams: [managers, ceo, engineers]
description: >-
Yes/no + justification if no. Senior ack required because
backward-compat shims are how dead-code accretes.
Yes/no + justification if no. Senior ack or engineer ack required
because backward-compat shims are how dead-code accretes.
- slug: memory-consulted
numeric_alias: 7
@@ -138,8 +138,8 @@ n/a_gates:
must post /sop-n/a qa-review to activate.
security-review:
required_teams: [security, managers, ceo]
required_teams: [security, managers, ceo, Owners]
description: >-
Security review N/A when this change has no security surface
(docs-only, pure-frontend, dependency-only). A security/owners
(docs-only, pure-frontend, dependency-only). A security/managers/ceo/owners
member must post /sop-n/a security-review to activate.