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/09/18 03:20:36 UTC

(magpie) branch main updated: feat(vetted-ops): add issue-edit-body and milestone-create (#1267)

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 096cf5d8 feat(vetted-ops): add issue-edit-body and milestone-create (#1267)
096cf5d8 is described below

commit 096cf5d8fc4e83f6dbdfa35d2eb1ea666314c89c
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Fri Sep 18 05:20:30 2026 +0200

    feat(vetted-ops): add issue-edit-body and milestone-create (#1267)
    
    The catalogue had no way to change an issue body. Every skill that
    maintains issue-template body fields — the security sync's `Affected
    versions`, `Short public summary`, the embedded CVE JSON — therefore had
    to reach for `gh` directly, which is exactly the traffic the dispatcher
    exists to carry. An operator correcting a body field was stepping
    outside the policy to do it.
    
    `milestone-create` closes the matching gap on the other side: a wave
    milestone that does not exist yet could be neither created nor assigned,
    so a tracker whose release slipped had nowhere to go.
    
    Both stay inside the existing boundaries rather than widening them:
    
    - `issue-edit-body` takes its text as a body file, so it inherits the
      read-once workspace path that comments already use — the bytes that
      were validated are the bytes that get published, and `gh` opens no
      file of ours.
    - `milestone-create` is enum-gated on the same `milestones` list that
      gates assignment. The policy file remains the authority: a new
      milestone is a reviewed edit there first, and only then can it be
      created or assigned. Without that gate the operation would let a
      caller mint arbitrary milestones and then assign them, which is the
      one thing the enum was there to prevent.
    
    Tests cover the stdin path, workspace containment for the new body
    parameter, and the enum gate; the tracker/upstream crossing test now
    names `milestone-create` so it is asserted rather than skipped.
    
    Generated-by: Claude Opus 5
---
 tools/vetted-ops/src/vetted_ops/ops.py    | 37 +++++++++++++++++++++++++
 tools/vetted-ops/tests/test_vetted_ops.py | 46 ++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/tools/vetted-ops/src/vetted_ops/ops.py b/tools/vetted-ops/src/vetted_ops/ops.py
index 8a946b97..6a3aeabd 100644
--- a/tools/vetted-ops/src/vetted_ops/ops.py
+++ b/tools/vetted-ops/src/vetted_ops/ops.py
@@ -589,6 +589,43 @@ _register(
     )
 )
 
+_register(
+    Op(
+        name="issue-edit-body",
+        params=("number", "body"),
+        writes=True,
+        summary="Replace a tracker issue's body from a body file.",
+        body_files=("body",),
+        build=lambda cfg, number, body: [
+            "gh",
+            "issue",
+            "edit",
+            number,
+            "--repo",
+            _tracker(cfg),
+            "--body-file",
+            body,
+        ],
+    )
+)
+
+_register(
+    Op(
+        name="milestone-create",
+        params=("milestone",),
+        writes=True,
+        summary="Create a configured milestone on the tracker.",
+        enums={"milestone": "milestones"},
+        build=lambda cfg, milestone: [
+            "gh",
+            "api",
+            f"repos/{_tracker(cfg)}/milestones",
+            "-f",
+            f"title={milestone}",
+        ],
+    )
+)
+
 _register(
     Op(
         name="issue-close",
diff --git a/tools/vetted-ops/tests/test_vetted_ops.py b/tools/vetted-ops/tests/test_vetted_ops.py
index 94477f57..998e324c 100644
--- a/tools/vetted-ops/tests/test_vetted_ops.py
+++ b/tools/vetted-ops/tests/test_vetted_ops.py
@@ -204,6 +204,45 @@ def test_body_file_content_may_contain_anything(policy: config.Config) -> None:
     assert sent == b"`id` $(whoami) && rm -rf / ; drop table\n"
 
 
+def test_issue_edit_body_sends_the_body_on_stdin(policy: config.Config) -> None:
+    """
+    Editing a body replaces the whole issue text, so it takes the same
+    read-once path as a comment: `gh` is handed "-" and the bytes we validated
+    are the bytes that get published.
+    """
+    body = policy.workspace / "new-body.md"
+    body.write_text("### Affected versions\n\napache-airflow `< NEXT VERSION`\n")
+    op = ops.resolve("issue-edit-body")
+    params, sent = cli._validate_params(op, ["611", str(body)], policy)
+    argv = cli.build_argv(op, params, policy)
+    assert argv[:5] == ["gh", "issue", "edit", "611", "--repo"]
+    assert argv[-2:] == ["--body-file", "-"]
+    assert sent == b"### Affected versions\n\napache-airflow `< NEXT VERSION`\n"
+
+
+def test_issue_edit_body_refuses_a_body_outside_the_workspace(policy: config.Config, tmp_path: Path) -> None:
+    stray = tmp_path / "elsewhere.md"
+    stray.write_text("not mine")
+    op = ops.resolve("issue-edit-body")
+    with pytest.raises(ops.ParamError, match="must live under the workspace"):
+        cli._validate_params(op, ["611", str(stray)], policy)
+
+
+def test_milestone_create_is_gated_on_the_configured_milestones(policy: config.Config) -> None:
+    """
+    Creating a milestone is enum-gated on the same list that gates assigning
+    one. The policy file stays the authority: a new milestone is a reviewed
+    edit there first, and only then can it be created or assigned.
+    """
+    op = ops.resolve("milestone-create")
+    with pytest.raises(ops.ParamError, match="not one of the configured values"):
+        cli._validate_params(op, ["Providers 2026-10-06"], policy)
+
+    params, _ = cli._validate_params(op, ["1.2.3"], policy)
+    argv = cli.build_argv(op, params, policy)
+    assert argv == ["gh", "api", "repos/acme/tracker/milestones", "-f", "title=1.2.3"]
+
+
 # --- per-caller scoping ------------------------------------------------------
 
 
@@ -388,7 +427,12 @@ def test_tracker_and_upstream_operations_never_cross(policy: config.Config) -> N
         argv = " ".join(op.build(policy.as_mapping(), **params))
         if name.startswith("repo-issue-") or name.startswith("pr-") or name.startswith("gql-"):
             assert tracker not in argv, f"{name} reached the tracker"
-        elif name.startswith("issue-") or name in {"label-list", "milestone-list", "collaborators"}:
+        elif name.startswith("issue-") or name in {
+            "label-list",
+            "milestone-list",
+            "milestone-create",
+            "collaborators",
+        }:
             assert upstream not in argv, f"{name} reached the upstream repo"