You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/16 07:12:58 UTC
(magpie) branch main updated: fix(vetted-ops): find and write the apache-magpie status-rollup marker (#1665)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git
The following commit(s) were added to refs/heads/main by this push:
new ccead706 fix(vetted-ops): find and write the apache-magpie status-rollup marker (#1665)
ccead706 is described below
commit ccead7069b3b7348f108773adcae245e5350b13a
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Fri Oct 16 08:12:53 2026 +0100
fix(vetted-ops): find and write the apache-magpie status-rollup marker (#1665)
rollup-append matched only the legacy `<!-- <tracker> status rollup v1 -->`
marker, so on trackers whose rollup carries `<!-- apache-magpie: status-rollup v1 -->`
it missed the rollup and created a second, legacy-marker comment. Both vetted-ops
and github-rollup now prefer the canonical marker, still recognise the legacy one,
and always write the canonical marker for a new rollup; the spec says the same.
Generated-by: Claude Opus 5
---
.../skills/issue-invalidate/SKILL.md | 9 +-
.../magpie-vetted-ops/tools/vetted-ops/README.md | 6 +-
.../tools/vetted-ops/src/vetted_ops/procedures.py | 10 +-
.../vetted-ops/src/vetted_ops/rollup_format.py | 65 ++++++++++---
tools/github-rollup/README.md | 7 +-
tools/github-rollup/src/github_rollup/__init__.py | 10 ++
tools/github-rollup/src/github_rollup/cli.py | 14 ++-
tools/github-rollup/src/github_rollup/rollup.py | 65 ++++++++++---
tools/github-rollup/tests/test_cli.py | 39 +++++++-
tools/github-rollup/tests/test_rollup.py | 44 ++++++++-
tools/github/status-rollup.md | 21 ++--
tools/vetted-ops/README.md | 6 +-
tools/vetted-ops/src/vetted_ops/procedures.py | 10 +-
tools/vetted-ops/src/vetted_ops/rollup_format.py | 65 ++++++++++---
tools/vetted-ops/tests/test_procedures.py | 106 +++++++++++++++++++--
15 files changed, 391 insertions(+), 86 deletions(-)
diff --git a/plugins/magpie-security/skills/issue-invalidate/SKILL.md b/plugins/magpie-security/skills/issue-invalidate/SKILL.md
index 7f35713b..0d6b8e2f 100644
--- a/plugins/magpie-security/skills/issue-invalidate/SKILL.md
+++ b/plugins/magpie-security/skills/issue-invalidate/SKILL.md
@@ -20,7 +20,7 @@ argument-hint: "[issue-number]"
capability: capability:resolve
surface_hash: sha256:43a163bcbdb06f11
license: Apache-2.0
-measured_tokens: 7563
+measured_tokens: 7582
---
<!-- Placeholder convention (see AGENTS.md#placeholder-convention-used-in-skill-files):
@@ -296,9 +296,10 @@ the close. Read the *Security mailing list thread* body field:
For `security@`-imported trackers, locate the Gmail `threadId`:
1. Read the rollup comment on the tracker (the first
- `<details>` block with the `<tracker> status rollup v1`
- marker). Look for `threadId` references in the *Provenance:*
- line of the import entry.
+ `<details>` block with the `apache-magpie: status-rollup v1`
+ marker, or the legacy `<tracker> status rollup v1` one). Look
+ for `threadId` references in the *Provenance:* line of the import
+ entry.
2. If the rollup is missing or thin, fall back to a Gmail subject
search: `mcp__claude_ai_Gmail__search_threads` with the
tracker title (or a distinctive phrase from the body). One
diff --git a/plugins/magpie-vetted-ops/tools/vetted-ops/README.md b/plugins/magpie-vetted-ops/tools/vetted-ops/README.md
index 0a65303a..bf66aea8 100644
--- a/plugins/magpie-vetted-ops/tools/vetted-ops/README.md
+++ b/plugins/magpie-vetted-ops/tools/vetted-ops/README.md
@@ -474,8 +474,10 @@ prompts. `vetted-op` can run the same operations, but only inside the sandbox.
`<`, `>` or `·` (the summary-line separator); `field` is a heading name of
1-80 characters with no leading `-` and no `#`.
- `rollup-append` attributes the entry to the authenticated `gh` login and today's
- UTC date, and creates the rollup (marker line for the tracker's name) when the
- issue has none. It finds a rollup written under any tracker name.
+ UTC date, and creates the rollup (marker line `<!-- apache-magpie: status-rollup v1 -->`)
+ when the issue has none. It finds the rollup by that marker first, then by the
+ legacy `<!-- <name> status rollup v<N>` marker, so a tracker carrying both keeps
+ growing the canonical one.
- `rollup-amend-latest` refuses unless the latest entry's action equals `action`,
so a concurrent entry by someone else is never overwritten.
- `rollup-fold` keeps the legacy comment's date and author, left-trims its lines,
diff --git a/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/procedures.py b/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/procedures.py
index 0069434b..8c8987b4 100644
--- a/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/procedures.py
+++ b/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/procedures.py
@@ -59,6 +59,7 @@ from .body_field_format import FieldNotFoundError, extract_field, replace_field
from .rollup_format import (
build_entry,
build_new_rollup_body,
+ find_rollup_index,
is_rollup_body,
iter_entries,
left_trim_lines,
@@ -279,10 +280,9 @@ def _list_comments(r: Runner, number: str) -> list[dict[str, object]]:
def _find_rollup(comments: list[dict[str, object]]) -> dict[str, object] | None:
- for c in comments:
- if is_rollup_body(str(c.get("body") or "")):
- return c
- return None
+ """The rollup comment: the canonical marker first, else a legacy one."""
+ idx = find_rollup_index([str(c.get("body") or "") for c in comments])
+ return None if idx is None else comments[idx]
_NODE_ID = re.compile(r"^[A-Za-z0-9_-]{1,120}$")
@@ -328,7 +328,7 @@ def _write_entry(r: Runner, number: str, entry: str) -> tuple[str, str]:
if rollup is None:
out = r.write(
["gh", "issue", "comment", number, "--repo", r.tracker, "--body-file", "-"],
- stdin=build_new_rollup_body(entry, r.tracker.split("/")[-1]),
+ stdin=build_new_rollup_body(entry),
)
# `gh issue comment` prints the new comment's URL.
return f"created rollup on {r.tracker}#{number}", out.strip()
diff --git a/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/rollup_format.py b/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/rollup_format.py
index 9ff220b0..14c0d177 100644
--- a/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/rollup_format.py
+++ b/plugins/magpie-vetted-ops/tools/vetted-ops/src/vetted_ops/rollup_format.py
@@ -27,28 +27,67 @@ backticks, trailing whitespace) without mocking subprocess.
from __future__ import annotations
import re
+from collections.abc import Sequence
from dataclasses import dataclass
-# First line of any rollup comment: `<!-- <name> status rollup v<N> ...`.
-# `<name>` is the tracker repository's name (for example `airflow-s` for
-# `airflow-s/airflow-s`). Detection accepts any name so a rollup is found
-# whatever repository wrote it, and any version so a future `v2` bump
-# stays findable.
-ROLLUP_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+# First line the tools write when they create a rollup: the
+# framework-wide `<!-- apache-magpie: <kind> v<N> -->` skill-comment
+# marker, with `status-rollup` as the kind.
+ROLLUP_MARKER = "<!-- apache-magpie: status-rollup v1 -->"
+
+# The canonical marker, any version so a future `v2` bump stays findable.
+CANONICAL_MARKER_RE = re.compile(r"^<!-- apache-magpie: status-rollup v\d+\b")
+
+# The legacy marker earlier versions wrote,
+# `<!-- <name> status rollup v<N> — … -->`, where `<name>` is the tracker
+# repository's name (for example `airflow-s`). Still recognised so a
+# tracker whose rollup predates the canonical marker keeps working.
+LEGACY_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+
+# Either marker — what "is this comment a rollup" detection matches.
+ROLLUP_MARKER_RE = re.compile(r"^<!-- (?:apache-magpie: status-rollup|\S+ status rollup) v\d+")
# Kept for callers that imported it; detection no longer depends on it.
ROLLUP_MARKER_PREFIX = "<!-- airflow-s status rollup v"
def is_rollup_body(body: str) -> bool:
- """True when ``body`` starts with a status-rollup marker line."""
+ """True when ``body`` starts with a status-rollup marker line,
+ canonical or legacy."""
return bool(ROLLUP_MARKER_RE.match(body))
-def marker_line(tracker_name: str) -> str:
- """The first-line marker the tool writes when creating a rollup for
- the tracker repository named ``tracker_name`` (the part after `/`)."""
- return f"<!-- {tracker_name} status rollup v1 — all bot-authored status updates fold into this single comment. -->"
+def is_canonical_rollup_body(body: str) -> bool:
+ """True when ``body`` starts with the canonical
+ `<!-- apache-magpie: status-rollup v<N> -->` marker."""
+ return bool(CANONICAL_MARKER_RE.match(body))
+
+
+def find_rollup_index(bodies: Sequence[str]) -> int | None:
+ """Index of the rollup among the comment ``bodies``, or ``None``.
+
+ The first comment with the canonical marker wins; failing that, the
+ first with a legacy marker. Preferring the canonical one means a
+ tracker that carries both (a legacy-marker comment written next to
+ the skills' rollup) keeps growing the rollup the skills read.
+ """
+ legacy: int | None = None
+ for i, body in enumerate(bodies):
+ if is_canonical_rollup_body(body):
+ return i
+ if legacy is None and is_rollup_body(body):
+ legacy = i
+ return legacy
+
+
+def marker_line(tracker_name: str = "") -> str:
+ """The first-line marker the tool writes when creating a rollup.
+
+ ``tracker_name`` is accepted so existing callers keep working; the
+ canonical marker no longer names the tracker, so it is ignored.
+ """
+ del tracker_name
+ return ROLLUP_MARKER
# Between consecutive `<details>` entries we always write exactly one
@@ -161,9 +200,11 @@ def build_entry(*, date: str, user: str, action: str, body: str) -> str:
return f"<details><summary>{summary}</summary>\n\n{body_stripped}\n\n{_CLOSE_TAG}"
-def build_new_rollup_body(entry: str, tracker_name: str) -> str:
+def build_new_rollup_body(entry: str, tracker_name: str = "") -> str:
"""Compose a brand-new rollup comment body wrapping the first
entry. Use when no rollup comment exists yet on the tracker.
+ The body always starts with the canonical marker; ``tracker_name``
+ is accepted for existing callers and ignored.
"""
return f"{marker_line(tracker_name)}\n{entry}"
diff --git a/tools/github-rollup/README.md b/tools/github-rollup/README.md
index 4fe8dbbc..28bdf96c 100644
--- a/tools/github-rollup/README.md
+++ b/tools/github-rollup/README.md
@@ -96,9 +96,10 @@ Optional flags:
- `--dry-run` — print the decision (create vs append) without
writing.
-A new rollup's marker line names the tracker repository
-(`<!-- <repo-name> status rollup v1 — … -->`); an existing rollup is
-found by any `<!-- <name> status rollup v<N>` marker.
+A new rollup's marker line is `<!-- apache-magpie: status-rollup v1 -->`.
+An existing rollup is found by that marker first, then by the legacy
+`<!-- <name> status rollup v<N>` marker, so a tracker with both keeps
+growing the canonical one.
### `amend-latest <issue> --action "<label>" ...`
diff --git a/tools/github-rollup/src/github_rollup/__init__.py b/tools/github-rollup/src/github_rollup/__init__.py
index c331d80d..3bd67a58 100644
--- a/tools/github-rollup/src/github_rollup/__init__.py
+++ b/tools/github-rollup/src/github_rollup/__init__.py
@@ -16,11 +16,16 @@
# under the License.
from github_rollup.cli import main
from github_rollup.rollup import (
+ CANONICAL_MARKER_RE,
+ LEGACY_MARKER_RE,
+ ROLLUP_MARKER,
ROLLUP_MARKER_PREFIX,
ROLLUP_MARKER_RE,
RollupEntry,
build_entry,
build_new_rollup_body,
+ find_rollup_index,
+ is_canonical_rollup_body,
is_rollup_body,
iter_entries,
left_trim_lines,
@@ -31,11 +36,16 @@ from github_rollup.rollup import (
)
__all__ = [
+ "CANONICAL_MARKER_RE",
+ "LEGACY_MARKER_RE",
+ "ROLLUP_MARKER",
"ROLLUP_MARKER_PREFIX",
"ROLLUP_MARKER_RE",
"RollupEntry",
"build_entry",
"build_new_rollup_body",
+ "find_rollup_index",
+ "is_canonical_rollup_body",
"is_rollup_body",
"iter_entries",
"left_trim_lines",
diff --git a/tools/github-rollup/src/github_rollup/cli.py b/tools/github-rollup/src/github_rollup/cli.py
index c26b3950..3cb28a66 100644
--- a/tools/github-rollup/src/github_rollup/cli.py
+++ b/tools/github-rollup/src/github_rollup/cli.py
@@ -37,6 +37,7 @@ from pathlib import Path
from github_rollup.rollup import (
build_entry,
build_new_rollup_body,
+ find_rollup_index,
is_rollup_body,
iter_entries,
left_trim_lines,
@@ -150,13 +151,10 @@ def _print_url(url: str) -> None:
def _find_rollup(comments: list[dict]) -> dict | None:
- """Return the first comment whose body begins with the rollup
- marker, or None if no rollup exists on the issue."""
- for c in comments:
- body = c.get("body") or ""
- if is_rollup_body(body):
- return c
- return None
+ """Return the rollup comment — the first with the canonical marker,
+ else the first with a legacy one — or None if the issue has none."""
+ idx = find_rollup_index([c.get("body") or "" for c in comments])
+ return None if idx is None else comments[idx]
def _read_entry_body(args: argparse.Namespace) -> str:
@@ -206,7 +204,7 @@ def _write_entry(issue: str, repo: str, entry: str, action: str, date: str, dry_
sys.stderr.write(f"dry-run: would APPEND to existing rollup comment {rollup.get('url')}\n")
return True
if rollup is None:
- url = _gh_post_comment(issue, repo, build_new_rollup_body(entry, repo.split("/")[-1]))
+ url = _gh_post_comment(issue, repo, build_new_rollup_body(entry))
sys.stderr.write(f"created rollup on {repo}#{issue} ({action!r}, date={date})\n")
_print_url(url)
return True
diff --git a/tools/github-rollup/src/github_rollup/rollup.py b/tools/github-rollup/src/github_rollup/rollup.py
index 9ff220b0..14c0d177 100644
--- a/tools/github-rollup/src/github_rollup/rollup.py
+++ b/tools/github-rollup/src/github_rollup/rollup.py
@@ -27,28 +27,67 @@ backticks, trailing whitespace) without mocking subprocess.
from __future__ import annotations
import re
+from collections.abc import Sequence
from dataclasses import dataclass
-# First line of any rollup comment: `<!-- <name> status rollup v<N> ...`.
-# `<name>` is the tracker repository's name (for example `airflow-s` for
-# `airflow-s/airflow-s`). Detection accepts any name so a rollup is found
-# whatever repository wrote it, and any version so a future `v2` bump
-# stays findable.
-ROLLUP_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+# First line the tools write when they create a rollup: the
+# framework-wide `<!-- apache-magpie: <kind> v<N> -->` skill-comment
+# marker, with `status-rollup` as the kind.
+ROLLUP_MARKER = "<!-- apache-magpie: status-rollup v1 -->"
+
+# The canonical marker, any version so a future `v2` bump stays findable.
+CANONICAL_MARKER_RE = re.compile(r"^<!-- apache-magpie: status-rollup v\d+\b")
+
+# The legacy marker earlier versions wrote,
+# `<!-- <name> status rollup v<N> — … -->`, where `<name>` is the tracker
+# repository's name (for example `airflow-s`). Still recognised so a
+# tracker whose rollup predates the canonical marker keeps working.
+LEGACY_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+
+# Either marker — what "is this comment a rollup" detection matches.
+ROLLUP_MARKER_RE = re.compile(r"^<!-- (?:apache-magpie: status-rollup|\S+ status rollup) v\d+")
# Kept for callers that imported it; detection no longer depends on it.
ROLLUP_MARKER_PREFIX = "<!-- airflow-s status rollup v"
def is_rollup_body(body: str) -> bool:
- """True when ``body`` starts with a status-rollup marker line."""
+ """True when ``body`` starts with a status-rollup marker line,
+ canonical or legacy."""
return bool(ROLLUP_MARKER_RE.match(body))
-def marker_line(tracker_name: str) -> str:
- """The first-line marker the tool writes when creating a rollup for
- the tracker repository named ``tracker_name`` (the part after `/`)."""
- return f"<!-- {tracker_name} status rollup v1 — all bot-authored status updates fold into this single comment. -->"
+def is_canonical_rollup_body(body: str) -> bool:
+ """True when ``body`` starts with the canonical
+ `<!-- apache-magpie: status-rollup v<N> -->` marker."""
+ return bool(CANONICAL_MARKER_RE.match(body))
+
+
+def find_rollup_index(bodies: Sequence[str]) -> int | None:
+ """Index of the rollup among the comment ``bodies``, or ``None``.
+
+ The first comment with the canonical marker wins; failing that, the
+ first with a legacy marker. Preferring the canonical one means a
+ tracker that carries both (a legacy-marker comment written next to
+ the skills' rollup) keeps growing the rollup the skills read.
+ """
+ legacy: int | None = None
+ for i, body in enumerate(bodies):
+ if is_canonical_rollup_body(body):
+ return i
+ if legacy is None and is_rollup_body(body):
+ legacy = i
+ return legacy
+
+
+def marker_line(tracker_name: str = "") -> str:
+ """The first-line marker the tool writes when creating a rollup.
+
+ ``tracker_name`` is accepted so existing callers keep working; the
+ canonical marker no longer names the tracker, so it is ignored.
+ """
+ del tracker_name
+ return ROLLUP_MARKER
# Between consecutive `<details>` entries we always write exactly one
@@ -161,9 +200,11 @@ def build_entry(*, date: str, user: str, action: str, body: str) -> str:
return f"<details><summary>{summary}</summary>\n\n{body_stripped}\n\n{_CLOSE_TAG}"
-def build_new_rollup_body(entry: str, tracker_name: str) -> str:
+def build_new_rollup_body(entry: str, tracker_name: str = "") -> str:
"""Compose a brand-new rollup comment body wrapping the first
entry. Use when no rollup comment exists yet on the tracker.
+ The body always starts with the canonical marker; ``tracker_name``
+ is accepted for existing callers and ignored.
"""
return f"{marker_line(tracker_name)}\n{entry}"
diff --git a/tools/github-rollup/tests/test_cli.py b/tools/github-rollup/tests/test_cli.py
index b58ddbb8..7adf1e7f 100644
--- a/tools/github-rollup/tests/test_cli.py
+++ b/tools/github-rollup/tests/test_cli.py
@@ -50,8 +50,14 @@ class FakeGh:
"""Records every `gh` call and returns canned output for each
pattern. Tests assert on the recorded calls."""
- def __init__(self, has_rollup: bool = True, rollup_body: str = ROLLUP_BODY) -> None:
+ def __init__(
+ self,
+ has_rollup: bool = True,
+ rollup_body: str = ROLLUP_BODY,
+ earlier_comments: list[dict] | None = None,
+ ) -> None:
self.calls: list[tuple[list[str], str | None]] = []
+ self.earlier_comments = earlier_comments or []
self.has_rollup = has_rollup
self.rollup_body = rollup_body
self.deleted: list[str] = []
@@ -74,7 +80,7 @@ class FakeGh:
joined = " ".join(cmd)
if "issue view" in joined and "--json comments" in joined:
- comments = []
+ comments = list(self.earlier_comments)
if self.has_rollup:
comments.append(
{
@@ -265,7 +271,7 @@ def test_append_creates_new_rollup_when_none_exists(
assert rc == 0
body = fake_gh_no_rollup.posted_body
assert body is not None
- assert body.startswith("<!-- r status rollup v1 — all bot-authored status updates fold")
+ assert body.startswith("<!-- apache-magpie: status-rollup v1 -->\n<details><summary>")
assert "· @potiuk · Import" in body
assert "Imported from security@" in body
err = capsys.readouterr().err
@@ -373,11 +379,34 @@ def test_latest_exits_3_when_no_rollup(fake_gh_no_rollup: FakeGh, capsys: pytest
# ---------------------------------------------------------------------------
-def test_new_rollup_marker_names_the_tracker_repository(fake_gh_no_rollup: FakeGh) -> None:
+def test_new_rollup_uses_the_canonical_marker_whatever_the_tracker(fake_gh_no_rollup: FakeGh) -> None:
rc = cli.main(["--repo", "acme/acme-security", "append", "7", "--action", "Import", "--entry-body", "x"])
assert rc == 0
assert fake_gh_no_rollup.posted_body is not None
- assert fake_gh_no_rollup.posted_body.startswith("<!-- acme-security status rollup v1 ")
+ assert fake_gh_no_rollup.posted_body.startswith("<!-- apache-magpie: status-rollup v1 -->\n")
+
+
+def test_a_canonical_marker_rollup_is_found(monkeypatch: pytest.MonkeyPatch) -> None:
+ canonical = ROLLUP_BODY.replace(MARKER_LINE, "<!-- apache-magpie: status-rollup v1 -->")
+ f = FakeGh(rollup_body=canonical)
+ monkeypatch.setattr(subprocess, "run", f)
+ rc = cli.main(["--repo", "o/r", "append", "1", "--action", "Sync", "--entry-body", "x"])
+ assert rc == 0
+ assert f.posted_body is None
+ assert f.patched_body is not None and f.patched_body.startswith(canonical)
+
+
+def test_the_canonical_rollup_wins_over_an_earlier_legacy_one(monkeypatch: pytest.MonkeyPatch) -> None:
+ canonical = ROLLUP_BODY.replace(MARKER_LINE, "<!-- apache-magpie: status-rollup v1 -->")
+ legacy = {"id": "IC_LEGACY", "body": ROLLUP_BODY, "url": "https://github.com/o/r/issues/1#issuecomment-9"}
+ f = FakeGh(rollup_body=canonical, earlier_comments=[legacy])
+ monkeypatch.setattr(subprocess, "run", f)
+ rc = cli.main(["--repo", "o/r", "append", "1", "--action", "Sync", "--entry-body", "x"])
+ assert rc == 0
+ joined = [" ".join(c) for c, _ in f.calls]
+ assert any("PATCH repos/o/r/issues/comments/1" in j for j in joined)
+ assert not any("issues/comments/9" in j for j in joined)
+ assert f.patched_body is not None and f.patched_body.startswith(canonical)
def test_a_rollup_written_for_another_tracker_is_still_found(monkeypatch: pytest.MonkeyPatch) -> None:
diff --git a/tools/github-rollup/tests/test_rollup.py b/tools/github-rollup/tests/test_rollup.py
index 307b5f2f..225ca98c 100644
--- a/tools/github-rollup/tests/test_rollup.py
+++ b/tools/github-rollup/tests/test_rollup.py
@@ -19,9 +19,12 @@ from __future__ import annotations
import pytest
from github_rollup.rollup import (
- ROLLUP_MARKER_PREFIX,
+ ROLLUP_MARKER,
build_entry,
build_new_rollup_body,
+ find_rollup_index,
+ is_canonical_rollup_body,
+ is_rollup_body,
iter_entries,
parse_summary_line,
rebuild_with_appended_entry,
@@ -195,7 +198,8 @@ def test_build_entry_strips_body_whitespace():
def test_build_new_rollup_body_includes_marker_first():
entry = build_entry(date="2026-05-30", user="@a", action="b", body="c")
body = build_new_rollup_body(entry, "airflow-s")
- assert body.startswith(ROLLUP_MARKER_PREFIX)
+ assert body.startswith(f"{ROLLUP_MARKER}\n")
+ assert ROLLUP_MARKER == "<!-- apache-magpie: status-rollup v1 -->"
assert "<details>" in body
# Marker line + immediately the entry — no blank line gap.
first_nl = body.find("\n")
@@ -240,3 +244,39 @@ def test_round_trip_three_appends_keeps_count():
build_entry(date=f"2026-05-0{i}", user="@a", action=action, body=f"b{i}"),
)
assert [e.action for e in iter_entries(existing)] == ["x1", "x2", "x3"]
+
+
+# ---------------------------------------------------------------------------
+# marker detection: canonical first, legacy still recognised
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+ ("body", "canonical", "rollup"),
+ [
+ ("<!-- apache-magpie: status-rollup v1 -->\n<details>", True, True),
+ ("<!-- apache-magpie: status-rollup v2 -->", True, True),
+ (f"{MARKER_LINE}\n<details>", False, True),
+ ("<!-- acme-security status rollup v1 — x -->", False, True),
+ ("<!-- apache-magpie: release-manager-handoff v1 -->", False, False),
+ ("<!-- apache-magpie: status-rollup-x v1 -->", False, False),
+ ("quoted <!-- apache-magpie: status-rollup v1 -->", False, False),
+ ("", False, False),
+ ],
+)
+def test_marker_detection(body, canonical, rollup):
+ assert is_canonical_rollup_body(body) is canonical
+ assert is_rollup_body(body) is rollup
+
+
+def test_find_rollup_index_prefers_canonical_then_first_legacy():
+ canonical = f"{ROLLUP_MARKER}\n<details><summary>2026-05-28 · @a · Import</summary>\n\nA.\n\n</details>"
+ legacy = f"{MARKER_LINE}\n<details><summary>2026-05-28 · @a · Import</summary>\n\nA.\n\n</details>"
+ assert find_rollup_index(["hi", legacy, canonical]) == 2
+ assert find_rollup_index(["hi", legacy, legacy]) == 1
+ assert find_rollup_index(["hi"]) is None
+
+
+def test_iter_entries_skips_the_canonical_marker_line():
+ body = f"{ROLLUP_MARKER}\n<details><summary>2026-05-28 · @a · Import</summary>\n\nA.\n\n</details>"
+ assert [(e.action, e.body) for e in iter_entries(body)] == [("Import", "A.")]
diff --git a/tools/github/status-rollup.md b/tools/github/status-rollup.md
index dcb32f13..aac981e7 100644
--- a/tools/github/status-rollup.md
+++ b/tools/github/status-rollup.md
@@ -46,7 +46,7 @@ One comment per tracker, identified by an opening HTML marker on the
first line:
```markdown
-<!-- <tracker-name> status rollup v1 — all bot-authored status updates fold into this single comment. -->
+<!-- apache-magpie: status-rollup v1 -->
<details><summary>YYYY-MM-DD · @user · <Action></summary>
<entry body>
@@ -65,11 +65,15 @@ first line:
Rules (all load-bearing — breaking any of them breaks GitHub's
Markdown rendering):
-- **First line is the marker.** `<!-- <tracker-name> status rollup v1 — … -->`
- identifies the comment as the rollup, where `<tracker-name>` is the
- tracker repository's name (the part of `<tracker>` after the `/`).
- Detection matches `<!-- <any-name> status rollup v<N>` so a rollup
- is found whatever wrote it, and future `v2` bumps remain findable.
+- **First line is the marker.** `<!-- apache-magpie: status-rollup v1 -->`
+ identifies the comment as the rollup — the framework-wide
+ `<!-- apache-magpie: <kind> v<N> -->` skill-comment marker with
+ `status-rollup` as the kind. Detection matches any `v<N>` so future
+ `v2` bumps remain findable. The **legacy** marker earlier tool
+ versions wrote, `<!-- <tracker-name> status rollup v1 — … -->`, is
+ still recognised so older trackers keep working; when a tracker
+ carries both, the canonical one is the rollup. New rollups always
+ get the canonical marker.
- **Every entry is its own `<details>` block.** Including the very
first one (the import receipt). There is no always-visible
preamble above the first `<details>`. The reason: every entry —
@@ -218,7 +222,8 @@ original.
A comment is a candidate for folding when **all** of the following hold:
1. **Not already a rollup.** Its body does not start with a rollup
- marker (`<!-- <tracker-name> status rollup v`).
+ marker (`<!-- apache-magpie: status-rollup v`, or the legacy
+ `<!-- <tracker-name> status rollup v`).
2. **Author is on the security-team roster.** Cross-check
`.comments[].author.login` against the collaborator list (see
[`operations.md`](operations.md#collaborator-lookup-security-team-roster))
@@ -327,7 +332,7 @@ audit the change.
subsequent line of the entry. Compose entries with all lines
flush-left.
- **Never create two rollups on the same tracker.** If Step 1 of the
- upsert finds more than one `status rollup v` marker,
+ upsert finds more than one rollup marker (canonical or legacy),
stop and ask the user which to keep — the cheapest recovery is a
manual merge, not a silent overwrite.
- **Never name or describe other ASF projects' vulnerabilities** in a
diff --git a/tools/vetted-ops/README.md b/tools/vetted-ops/README.md
index d546f19c..da2ee39f 100644
--- a/tools/vetted-ops/README.md
+++ b/tools/vetted-ops/README.md
@@ -474,8 +474,10 @@ prompts. `vetted-op` can run the same operations, but only inside the sandbox.
`<`, `>` or `·` (the summary-line separator); `field` is a heading name of
1-80 characters with no leading `-` and no `#`.
- `rollup-append` attributes the entry to the authenticated `gh` login and today's
- UTC date, and creates the rollup (marker line for the tracker's name) when the
- issue has none. It finds a rollup written under any tracker name.
+ UTC date, and creates the rollup (marker line `<!-- apache-magpie: status-rollup v1 -->`)
+ when the issue has none. It finds the rollup by that marker first, then by the
+ legacy `<!-- <name> status rollup v<N>` marker, so a tracker carrying both keeps
+ growing the canonical one.
- `rollup-amend-latest` refuses unless the latest entry's action equals `action`,
so a concurrent entry by someone else is never overwritten.
- `rollup-fold` keeps the legacy comment's date and author, left-trims its lines,
diff --git a/tools/vetted-ops/src/vetted_ops/procedures.py b/tools/vetted-ops/src/vetted_ops/procedures.py
index 0069434b..8c8987b4 100644
--- a/tools/vetted-ops/src/vetted_ops/procedures.py
+++ b/tools/vetted-ops/src/vetted_ops/procedures.py
@@ -59,6 +59,7 @@ from .body_field_format import FieldNotFoundError, extract_field, replace_field
from .rollup_format import (
build_entry,
build_new_rollup_body,
+ find_rollup_index,
is_rollup_body,
iter_entries,
left_trim_lines,
@@ -279,10 +280,9 @@ def _list_comments(r: Runner, number: str) -> list[dict[str, object]]:
def _find_rollup(comments: list[dict[str, object]]) -> dict[str, object] | None:
- for c in comments:
- if is_rollup_body(str(c.get("body") or "")):
- return c
- return None
+ """The rollup comment: the canonical marker first, else a legacy one."""
+ idx = find_rollup_index([str(c.get("body") or "") for c in comments])
+ return None if idx is None else comments[idx]
_NODE_ID = re.compile(r"^[A-Za-z0-9_-]{1,120}$")
@@ -328,7 +328,7 @@ def _write_entry(r: Runner, number: str, entry: str) -> tuple[str, str]:
if rollup is None:
out = r.write(
["gh", "issue", "comment", number, "--repo", r.tracker, "--body-file", "-"],
- stdin=build_new_rollup_body(entry, r.tracker.split("/")[-1]),
+ stdin=build_new_rollup_body(entry),
)
# `gh issue comment` prints the new comment's URL.
return f"created rollup on {r.tracker}#{number}", out.strip()
diff --git a/tools/vetted-ops/src/vetted_ops/rollup_format.py b/tools/vetted-ops/src/vetted_ops/rollup_format.py
index 9ff220b0..14c0d177 100644
--- a/tools/vetted-ops/src/vetted_ops/rollup_format.py
+++ b/tools/vetted-ops/src/vetted_ops/rollup_format.py
@@ -27,28 +27,67 @@ backticks, trailing whitespace) without mocking subprocess.
from __future__ import annotations
import re
+from collections.abc import Sequence
from dataclasses import dataclass
-# First line of any rollup comment: `<!-- <name> status rollup v<N> ...`.
-# `<name>` is the tracker repository's name (for example `airflow-s` for
-# `airflow-s/airflow-s`). Detection accepts any name so a rollup is found
-# whatever repository wrote it, and any version so a future `v2` bump
-# stays findable.
-ROLLUP_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+# First line the tools write when they create a rollup: the
+# framework-wide `<!-- apache-magpie: <kind> v<N> -->` skill-comment
+# marker, with `status-rollup` as the kind.
+ROLLUP_MARKER = "<!-- apache-magpie: status-rollup v1 -->"
+
+# The canonical marker, any version so a future `v2` bump stays findable.
+CANONICAL_MARKER_RE = re.compile(r"^<!-- apache-magpie: status-rollup v\d+\b")
+
+# The legacy marker earlier versions wrote,
+# `<!-- <name> status rollup v<N> — … -->`, where `<name>` is the tracker
+# repository's name (for example `airflow-s`). Still recognised so a
+# tracker whose rollup predates the canonical marker keeps working.
+LEGACY_MARKER_RE = re.compile(r"^<!-- \S+ status rollup v\d+")
+
+# Either marker — what "is this comment a rollup" detection matches.
+ROLLUP_MARKER_RE = re.compile(r"^<!-- (?:apache-magpie: status-rollup|\S+ status rollup) v\d+")
# Kept for callers that imported it; detection no longer depends on it.
ROLLUP_MARKER_PREFIX = "<!-- airflow-s status rollup v"
def is_rollup_body(body: str) -> bool:
- """True when ``body`` starts with a status-rollup marker line."""
+ """True when ``body`` starts with a status-rollup marker line,
+ canonical or legacy."""
return bool(ROLLUP_MARKER_RE.match(body))
-def marker_line(tracker_name: str) -> str:
- """The first-line marker the tool writes when creating a rollup for
- the tracker repository named ``tracker_name`` (the part after `/`)."""
- return f"<!-- {tracker_name} status rollup v1 — all bot-authored status updates fold into this single comment. -->"
+def is_canonical_rollup_body(body: str) -> bool:
+ """True when ``body`` starts with the canonical
+ `<!-- apache-magpie: status-rollup v<N> -->` marker."""
+ return bool(CANONICAL_MARKER_RE.match(body))
+
+
+def find_rollup_index(bodies: Sequence[str]) -> int | None:
+ """Index of the rollup among the comment ``bodies``, or ``None``.
+
+ The first comment with the canonical marker wins; failing that, the
+ first with a legacy marker. Preferring the canonical one means a
+ tracker that carries both (a legacy-marker comment written next to
+ the skills' rollup) keeps growing the rollup the skills read.
+ """
+ legacy: int | None = None
+ for i, body in enumerate(bodies):
+ if is_canonical_rollup_body(body):
+ return i
+ if legacy is None and is_rollup_body(body):
+ legacy = i
+ return legacy
+
+
+def marker_line(tracker_name: str = "") -> str:
+ """The first-line marker the tool writes when creating a rollup.
+
+ ``tracker_name`` is accepted so existing callers keep working; the
+ canonical marker no longer names the tracker, so it is ignored.
+ """
+ del tracker_name
+ return ROLLUP_MARKER
# Between consecutive `<details>` entries we always write exactly one
@@ -161,9 +200,11 @@ def build_entry(*, date: str, user: str, action: str, body: str) -> str:
return f"<details><summary>{summary}</summary>\n\n{body_stripped}\n\n{_CLOSE_TAG}"
-def build_new_rollup_body(entry: str, tracker_name: str) -> str:
+def build_new_rollup_body(entry: str, tracker_name: str = "") -> str:
"""Compose a brand-new rollup comment body wrapping the first
entry. Use when no rollup comment exists yet on the tracker.
+ The body always starts with the canonical marker; ``tracker_name``
+ is accepted for existing callers and ignored.
"""
return f"{marker_line(tracker_name)}\n{entry}"
diff --git a/tools/vetted-ops/tests/test_procedures.py b/tools/vetted-ops/tests/test_procedures.py
index cc00f89d..19d488e7 100644
--- a/tools/vetted-ops/tests/test_procedures.py
+++ b/tools/vetted-ops/tests/test_procedures.py
@@ -27,7 +27,10 @@ import pytest
from vetted_ops import body_field_format, cli, config, ops, procedures, rollup_format
TRACKER = "acme/tracker"
-MARKER = rollup_format.marker_line("tracker")
+MARKER = rollup_format.ROLLUP_MARKER
+LEGACY_MARKER = (
+ "<!-- airflow-s status rollup v1 — all bot-authored status updates fold into this single comment. -->"
+)
CONFIG_TOML = """
workspace = '{workspace}'
@@ -112,12 +115,12 @@ def rollup_comment(body: str, rest_id: str = "555") -> dict[str, object]:
}
-def existing_rollup(*actions: str) -> str:
+def existing_rollup(*actions: str, marker: str = MARKER) -> str:
entries = [
rollup_format.build_entry(date="2026-01-0" + str(i + 1), user="bob", action=a, body=f"entry {i}")
for i, a in enumerate(actions)
]
- body = f"{MARKER}\n{entries[0]}"
+ body = f"{marker}\n{entries[0]}"
for e in entries[1:]:
body = rollup_format.rebuild_with_appended_entry(body, e)
return body
@@ -197,7 +200,7 @@ def test_append_creates_the_rollup_when_absent() -> None:
assert argv == ["gh", "issue", "comment", "7", "--repo", TRACKER, "--body-file", "-"]
assert stdin is not None
assert stdin.decode() == (
- "<!-- tracker status rollup v1 — all bot-authored status updates fold into this single comment. -->\n"
+ "<!-- apache-magpie: status-rollup v1 -->\n"
"<details><summary>2026-09-28 · @alice · Sync</summary>\n\nhello\n\n</details>"
)
@@ -233,14 +236,98 @@ def test_amend_prints_the_rollup_url(capsys: pytest.CaptureFixture[str]) -> None
assert capsys.readouterr().out == "https://github.com/acme/tracker/issues/7#issuecomment-555\n"
-def test_append_finds_a_rollup_written_for_another_tracker_name() -> None:
- old = existing_rollup("Import").replace("tracker status rollup", "airflow-s status rollup")
+def test_append_finds_a_legacy_marker_rollup() -> None:
+ old = existing_rollup("Import", marker=LEGACY_MARKER)
+ fake = FakeGh(comments=[rollup_comment(old)])
+ procedures.rollup_append(runner(fake), number="7", action="Sync", text="new", date="2026-09-28")
+ [(argv, stdin)] = fake.writes
+ assert argv == ["gh", "api", f"repos/{TRACKER}/issues/comments/555", "-X", "PATCH", "-F", "body=@-"]
+ assert stdin is not None
+ # The legacy rollup keeps its own marker line; only a new rollup gets the canonical one.
+ assert stdin.decode().startswith(f"{LEGACY_MARKER}\n")
+
+
+def test_append_finds_a_legacy_rollup_written_for_another_tracker_name() -> None:
+ old = existing_rollup("Import", marker=LEGACY_MARKER.replace("airflow-s", "acme-security"))
fake = FakeGh(comments=[rollup_comment(old)])
procedures.rollup_append(runner(fake), number="7", action="Sync", text="new", date="2026-09-28")
[(argv, _)] = fake.writes
assert "-X" in argv and argv[argv.index("-X") + 1] == "PATCH"
+def test_append_prefers_the_canonical_rollup_over_an_earlier_legacy_one() -> None:
+ legacy = existing_rollup("Import", marker=LEGACY_MARKER)
+ canonical = existing_rollup("Import", "Sync")
+ fake = FakeGh(comments=[rollup_comment(legacy, rest_id="111"), rollup_comment(canonical, rest_id="222")])
+ procedures.rollup_append(runner(fake), number="7", action="Sync", text="new", date="2026-09-28")
+ [(argv, stdin)] = fake.writes
+ assert argv[2] == f"repos/{TRACKER}/issues/comments/222"
+ assert stdin is not None
+ assert stdin.decode().startswith(f"{MARKER}\n")
+ assert [e.action for e in rollup_format.iter_entries(stdin.decode())] == ["Import", "Sync", "Sync"]
+
+
+def test_append_never_creates_a_second_rollup_beside_a_canonical_one() -> None:
+ fake = FakeGh(comments=[{"body": "hi", "url": "x"}, rollup_comment(existing_rollup("Import"))])
+ procedures.rollup_append(runner(fake), number="7", action="Sync", text="new", date="2026-09-28")
+ [(argv, _)] = fake.writes
+ assert argv[:3] == ["gh", "api", f"repos/{TRACKER}/issues/comments/555"]
+
+
+def test_amend_prefers_the_canonical_rollup_over_an_earlier_legacy_one() -> None:
+ legacy = existing_rollup("Sync", marker=LEGACY_MARKER)
+ canonical = existing_rollup("Import", "Sync")
+ fake = FakeGh(comments=[rollup_comment(legacy, rest_id="111"), rollup_comment(canonical, rest_id="222")])
+ procedures.rollup_amend_latest(runner(fake), number="7", action="Sync", text="rewritten")
+ [(argv, stdin)] = fake.writes
+ assert argv[2] == f"repos/{TRACKER}/issues/comments/222"
+ assert stdin is not None
+ assert [e.body for e in rollup_format.iter_entries(stdin.decode())] == ["entry 0", "rewritten"]
+
+
+# --- rollup_format: markers ---------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+ ("body", "canonical", "rollup"),
+ [
+ ("<!-- apache-magpie: status-rollup v1 -->\n<details>", True, True),
+ ("<!-- apache-magpie: status-rollup v2 -->", True, True),
+ (f"{LEGACY_MARKER}\n<details>", False, True),
+ ("<!-- acme-security status rollup v1 — x -->", False, True),
+ ("<!-- apache-magpie: release-manager-handoff v1 -->", False, False),
+ ("<!-- apache-magpie: status-rollup-x v1 -->", False, False),
+ ("a reply quoting <!-- apache-magpie: status-rollup v1 -->", False, False),
+ ("", False, False),
+ ],
+)
+def test_marker_detection(body: str, canonical: bool, rollup: bool) -> None:
+ assert rollup_format.is_canonical_rollup_body(body) is canonical
+ assert rollup_format.is_rollup_body(body) is rollup
+
+
+def test_find_rollup_index_prefers_canonical_then_first_legacy() -> None:
+ canonical = existing_rollup("Import")
+ legacy = existing_rollup("Import", marker=LEGACY_MARKER)
+ assert rollup_format.find_rollup_index(["hi", legacy, canonical]) == 2
+ assert rollup_format.find_rollup_index(["hi", legacy, legacy]) == 1
+ assert rollup_format.find_rollup_index(["hi", "there"]) is None
+ assert rollup_format.find_rollup_index([]) is None
+
+
+def test_new_rollup_body_starts_with_the_canonical_marker_whatever_the_tracker() -> None:
+ entry = rollup_format.build_entry(date="2026-09-28", user="alice", action="Sync", body="x")
+ for name in ("", "airflow-s", "acme-security"):
+ body = rollup_format.build_new_rollup_body(entry, name)
+ assert body == f"<!-- apache-magpie: status-rollup v1 -->\n{entry}"
+ assert rollup_format.is_canonical_rollup_body(body)
+
+
+def test_iter_entries_skips_the_canonical_marker_line() -> None:
+ entries = rollup_format.iter_entries(existing_rollup("Import", "Sync"))
+ assert [(e.action, e.body) for e in entries] == [("Import", "entry 0"), ("Sync", "entry 1")]
+
+
def test_dry_run_makes_no_writes_and_never_echoes_the_body(capsys: pytest.CaptureFixture[str]) -> None:
old = existing_rollup("Import")
fake = FakeGh(comments=[rollup_comment(old)])
@@ -327,6 +414,13 @@ def test_fold_refuses_the_rollup_itself() -> None:
assert fake.writes == []
+def test_fold_refuses_a_legacy_marker_rollup() -> None:
+ fake = FakeGh(legacy=legacy_comment(body=existing_rollup("Import", marker=LEGACY_MARKER)))
+ with pytest.raises(procedures.ProcedureRefused, match="is the rollup itself"):
+ procedures.rollup_fold(runner(fake), number="7", comment_id="555", action="x")
+ assert fake.writes == []
+
+
def test_fold_refuses_a_comment_from_another_issue() -> None:
fake = FakeGh(legacy=legacy_comment(issue_url="https://api.github.com/repos/acme/tracker/issues/70"))
with pytest.raises(procedures.ProcedureRefused, match="does not belong"):