fix(pipeline): fetch issue name from Plane API on status-trigger start
issue.updated ships only the changed fields, so name was absent and the branch slug became feature/<id>-untitled. Add fetch_issue_fields (single issue-detail GET returning name+description, reusing the endpoint/token of fetch_issue_description) and pull the name above the slug build. Empty name still falls back to untitled.
This commit is contained in:
dev-agent
@@ -197,6 +197,42 @@ def fetch_issue_description(issue_id: str, project_id: str) -> str:
|
||||
return ""
|
||||
|
||||
|
||||
def fetch_issue_fields(issue_id: str, project_id: str) -> tuple[str, str]:
|
||||
"""BUG B: GET the Plane issue by UUID ONCE and return (name, description).
|
||||
|
||||
Plane's ``issue.updated`` webhook (e.g. a status change) only carries the
|
||||
CHANGED fields, so BOTH ``name`` and ``description`` are usually absent in
|
||||
the payload. start_pipeline needs the real title (for the branch slug) and
|
||||
the real description (for the analyst .task.md). To avoid issuing two
|
||||
separate issue-detail GETs (one for name, one for description), this single
|
||||
request returns both.
|
||||
|
||||
Reuses the exact GET issue detail endpoint / shared token already used by
|
||||
``fetch_issue_sequence_id`` / ``fetch_issue_description``. For the
|
||||
description it applies the same logic as ``fetch_issue_description``
|
||||
(prefer ``description_stripped``, fall back to stripping
|
||||
``description_html``).
|
||||
|
||||
Returns ("", "") on network error, non-2xx, or missing body - never raises,
|
||||
so a Plane outage degrades gracefully (caller keeps its payload fallbacks).
|
||||
"""
|
||||
url = f"{PLANE_BASE}/workspaces/{WORKSPACE}/projects/{project_id}/issues/{issue_id}/"
|
||||
try:
|
||||
resp = httpx.get(url, headers=PLANE_HEADERS, timeout=10)
|
||||
resp.raise_for_status()
|
||||
body = resp.json()
|
||||
name = (body.get("name") or "").strip()
|
||||
desc = body.get("description_stripped")
|
||||
if desc and desc.strip():
|
||||
description = desc
|
||||
else:
|
||||
description = _strip_html(body.get("description_html") or "")
|
||||
return name, description
|
||||
except Exception as e:
|
||||
logger.warning(f"fetch_issue_fields failed for {issue_id}: {e}")
|
||||
return "", ""
|
||||
|
||||
|
||||
def find_issue_id(work_item_id: str, project_id: str = None) -> str | None:
|
||||
"""Find Plane issue UUID by work_item_id (e.g. 'ET-002')."""
|
||||
project_id = _resolve_project_id(work_item_id, project_id)
|
||||
|
||||
@@ -387,22 +387,35 @@ async def start_pipeline(data: dict, project_id: str = ""):
|
||||
repo = proj.repo
|
||||
plane_project_id = proj.plane_project_id
|
||||
|
||||
# BUG 1: Plane's issue.updated webhook (status change -> In Progress) sends
|
||||
# only the CHANGED fields, so description / description_stripped are usually
|
||||
# empty here even though the issue HAS a description. If the payload's
|
||||
# description is missing/too short, pull the full one from the Plane issue
|
||||
# detail API (same GET endpoint + shared token already used by
|
||||
# fetch_issue_sequence_id) before QG-0 runs. If the API is also empty, QG-0
|
||||
# legitimately fails (truly empty ticket).
|
||||
if not description or len(description.strip()) < 20:
|
||||
from ..plane_sync import fetch_issue_description
|
||||
fetched = fetch_issue_description(plane_id, plane_project_id)
|
||||
if fetched and len(fetched.strip()) >= len(description.strip()):
|
||||
description = fetched
|
||||
# BUG 1 + BUG B: Plane's issue.updated webhook (status change -> In Progress)
|
||||
# sends only the CHANGED fields, so BOTH description / description_stripped
|
||||
# AND name are usually empty here even though the issue HAS them. Pull the
|
||||
# full title + description from the Plane issue detail API in a SINGLE GET
|
||||
# (fetch_issue_fields: same endpoint + shared token already used by
|
||||
# fetch_issue_sequence_id) before QG-0 and before the branch slug is built.
|
||||
# If the API is also empty, QG-0 legitimately fails (truly empty ticket) and
|
||||
# name falls back to "untitled".
|
||||
name_missing = (not name) or name.strip().lower() == "untitled" or len(name.strip()) < 3
|
||||
desc_missing = (not description) or len(description.strip()) < 20
|
||||
if name_missing or desc_missing:
|
||||
from ..plane_sync import fetch_issue_fields
|
||||
fetched_name, fetched_desc = fetch_issue_fields(plane_id, plane_project_id)
|
||||
if desc_missing and fetched_desc and len(fetched_desc.strip()) >= len(description.strip()):
|
||||
description = fetched_desc
|
||||
logger.info(
|
||||
f"start_pipeline: pulled description from Plane API for {plane_id} "
|
||||
f"({len(description.strip())} chars)"
|
||||
)
|
||||
if name_missing and fetched_name and len(fetched_name.strip()) >= 3:
|
||||
name = fetched_name
|
||||
logger.info(
|
||||
f"start_pipeline: pulled name from Plane API for {plane_id} "
|
||||
f"('{name}')"
|
||||
)
|
||||
# BUG B fallback: if name is still empty/blank after the API pull, keep the
|
||||
# legacy "untitled" so the slug/branch build never crashes on an empty name.
|
||||
if not name or not name.strip():
|
||||
name = "untitled"
|
||||
|
||||
# QG-0 validation (hard gate on pipeline start)
|
||||
errors = _qg0_errors(name, description)
|
||||
|
||||
@@ -109,17 +109,19 @@ def _to_in_progress_no_desc(plane_id="bug1"):
|
||||
@patch("src.webhooks.plane._create_initial_docs", new_callable=AsyncMock)
|
||||
@patch("src.webhooks.plane._create_gitea_branch", new_callable=AsyncMock)
|
||||
@patch("src.plane_sync.fetch_issue_sequence_id", return_value=42)
|
||||
@patch("src.plane_sync.fetch_issue_description",
|
||||
return_value="This is a sufficiently long description fetched from Plane API.")
|
||||
@patch("src.plane_sync.fetch_issue_fields",
|
||||
return_value=("A valid backlog item title",
|
||||
"This is a sufficiently long description fetched from Plane API."))
|
||||
def test_status_start_fetches_description(
|
||||
mock_desc, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
mock_fields, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
):
|
||||
"""BUG 1: empty description in payload -> start_pipeline pulls it from the
|
||||
Plane API -> QG-0 passes -> task created + analyst enqueued (NOT blocked)."""
|
||||
Plane API (single fetch_issue_fields GET) -> QG-0 passes -> task created +
|
||||
analyst enqueued (NOT blocked)."""
|
||||
resp = _to_in_progress_no_desc("bug1")
|
||||
assert resp.status_code == 200
|
||||
# description was pulled from the API
|
||||
mock_desc.assert_called_once()
|
||||
# name + description were pulled from the API in one call
|
||||
mock_fields.assert_called_once()
|
||||
# QG-0 passed -> task created and analyst launched (NOT set_issue_blocked)
|
||||
assert _count("bug1") == 1
|
||||
assert _task("bug1")["stage"] == "analysis"
|
||||
@@ -131,15 +133,15 @@ def test_status_start_fetches_description(
|
||||
@patch("src.webhooks.plane._create_initial_docs", new_callable=AsyncMock)
|
||||
@patch("src.webhooks.plane._create_gitea_branch", new_callable=AsyncMock)
|
||||
@patch("src.plane_sync.fetch_issue_sequence_id", return_value=42)
|
||||
@patch("src.plane_sync.fetch_issue_description", return_value="")
|
||||
@patch("src.plane_sync.fetch_issue_fields", return_value=("", ""))
|
||||
def test_status_start_empty_api_still_blocks(
|
||||
mock_desc, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
mock_fields, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
):
|
||||
"""BUG 1 negative path: if the API also returns empty, QG-0 legitimately
|
||||
fails -> NO task is created (truly empty ticket)."""
|
||||
resp = _to_in_progress_no_desc("bug1-empty")
|
||||
assert resp.status_code == 200
|
||||
mock_desc.assert_called_once()
|
||||
mock_fields.assert_called_once()
|
||||
assert _count("bug1-empty") == 0
|
||||
mock_enqueue.assert_not_called()
|
||||
|
||||
@@ -168,10 +170,11 @@ def test_work_item_id_uniqueness():
|
||||
@patch("src.webhooks.plane._create_initial_docs", new_callable=AsyncMock)
|
||||
@patch("src.webhooks.plane._create_gitea_branch", new_callable=AsyncMock)
|
||||
@patch("src.plane_sync.fetch_issue_sequence_id", return_value=6)
|
||||
@patch("src.plane_sync.fetch_issue_description",
|
||||
return_value="A sufficiently long description for QG-0 to pass cleanly.")
|
||||
@patch("src.plane_sync.fetch_issue_fields",
|
||||
return_value=("Popup enduro trails feature",
|
||||
"A sufficiently long description for QG-0 to pass cleanly."))
|
||||
def test_collision_reassigns_in_start_pipeline(
|
||||
mock_desc, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
mock_fields, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
):
|
||||
"""BUG 2a end-to-end: ET-006 already exists -> a new In Progress issue whose
|
||||
Plane sequence_id is also 6 must NOT reuse ET-006."""
|
||||
|
||||
138
tests/test_taskmd_description.py
Normal file
138
tests/test_taskmd_description.py
Normal file
@@ -0,0 +1,138 @@
|
||||
"""Tests for fix/taskmd-description (3 bugs at the analyst pipeline entry/exit):
|
||||
|
||||
BUG A: start_pipeline built the analyst .task.md WITHOUT the description body
|
||||
(only Title), so analyst received a ~101-byte file and reported the
|
||||
"business request is empty". task_desc must now carry the description.
|
||||
|
||||
BUG B: issue.updated ships only changed fields, so `name` is usually absent ->
|
||||
slug/branch became "untitled". start_pipeline must pull the real name
|
||||
from the Plane API (single fetch_issue_fields GET, above the slug build)
|
||||
so the branch slug is NOT "untitled".
|
||||
|
||||
BUG C: the analyst "artifacts ready" comment used the obsolete ":approved:"
|
||||
wording. Under the status-only model it must ask for the **Approved**
|
||||
status (not ":approved:", not "In Progress") and link the docs that
|
||||
actually exist.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_taskmd_desc.db")
|
||||
os.environ["ORCH_DB_PATH"] = _test_db
|
||||
os.environ.setdefault("ORCH_PLANE_WEBHOOK_SECRET", "")
|
||||
os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token")
|
||||
os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token")
|
||||
|
||||
import pytest # noqa: E402
|
||||
from unittest.mock import patch, AsyncMock # noqa: E402
|
||||
from fastapi.testclient import TestClient # noqa: E402
|
||||
|
||||
from src.main import app # noqa: E402
|
||||
from src.db import init_db, get_db # noqa: E402
|
||||
from src import projects as P # noqa: E402
|
||||
from src.projects import reload_projects # noqa: E402
|
||||
|
||||
ENDURO_PLANE_ID = "7a79f0a9-5278-49cd-9007-9a338f238f9c"
|
||||
IN_PROGRESS = "b873d9eb-993c-48cd-97ac-99a9b1623967"
|
||||
BACKLOG = "113b24f6-cce8-4be9-9a22-a359b9cf0122"
|
||||
|
||||
client = TestClient(app)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def setup(monkeypatch):
|
||||
monkeypatch.setattr(P.settings, "db_path", _test_db)
|
||||
import src.db as _db
|
||||
monkeypatch.setattr(_db.settings, "db_path", _test_db)
|
||||
if os.path.exists(_test_db):
|
||||
os.unlink(_test_db)
|
||||
init_db()
|
||||
monkeypatch.setattr("src.webhooks.plane.verify_plane_signature", lambda body, sig: True)
|
||||
registry_json = (
|
||||
f'[{{"plane_project_id": "{ENDURO_PLANE_ID}", "repo": "enduro-trails",'
|
||||
f' "work_item_prefix": "ET", "name": "enduro-trails"}}]'
|
||||
)
|
||||
monkeypatch.setattr(P.settings, "projects_json", registry_json)
|
||||
reload_projects()
|
||||
yield
|
||||
reload_projects()
|
||||
if os.path.exists(_test_db):
|
||||
os.unlink(_test_db)
|
||||
|
||||
|
||||
def _task(plane_id):
|
||||
conn = get_db()
|
||||
row = conn.execute("SELECT * FROM tasks WHERE plane_id=?", (plane_id,)).fetchone()
|
||||
conn.close()
|
||||
return row
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# BUG A: description reaches the analyst .task.md
|
||||
# --------------------------------------------------------------------------- #
|
||||
@patch("src.webhooks.plane.enqueue_job", return_value=1)
|
||||
@patch("src.webhooks.plane._create_initial_docs", new_callable=AsyncMock)
|
||||
@patch("src.webhooks.plane._create_gitea_branch", new_callable=AsyncMock)
|
||||
@patch("src.plane_sync.fetch_issue_sequence_id", return_value=11)
|
||||
@patch("src.plane_sync.fetch_issue_fields",
|
||||
return_value=("ET-011 real title",
|
||||
"REAL BUSINESS REQUEST BODY: user wants GPX upload with "
|
||||
"validation and a results map."))
|
||||
def test_taskdesc_includes_description(
|
||||
mock_fields, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
):
|
||||
resp = client.post("/webhook/plane", json={
|
||||
"event": "issue", "action": "updated",
|
||||
"data": {
|
||||
"id": "taskA",
|
||||
# status change payload: NO name, NO description (only changed field)
|
||||
"project": ENDURO_PLANE_ID,
|
||||
"state": {"id": IN_PROGRESS, "name": "In Progress", "group": "started"},
|
||||
},
|
||||
"activity": {"field": "state", "new_value": IN_PROGRESS, "old_value": BACKLOG},
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
mock_enqueue.assert_called_once()
|
||||
# task_desc is the 3rd positional arg of enqueue_job(agent, repo, task_desc, ...)
|
||||
task_desc = mock_enqueue.call_args.args[2]
|
||||
assert "Description:" in task_desc
|
||||
# the actual description body (not just the Title) is in the file
|
||||
assert "REAL BUSINESS REQUEST BODY" in task_desc
|
||||
assert "results map" in task_desc
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# BUG B: name fetched from Plane API when payload is empty -> slug not untitled
|
||||
# --------------------------------------------------------------------------- #
|
||||
@patch("src.webhooks.plane.enqueue_job", return_value=1)
|
||||
@patch("src.webhooks.plane._create_initial_docs", new_callable=AsyncMock)
|
||||
@patch("src.webhooks.plane._create_gitea_branch", new_callable=AsyncMock)
|
||||
@patch("src.plane_sync.fetch_issue_sequence_id", return_value=11)
|
||||
@patch("src.plane_sync.fetch_issue_fields",
|
||||
return_value=("GPX upload feature",
|
||||
"A sufficiently long description so QG-0 passes cleanly."))
|
||||
def test_name_fetched_when_payload_empty(
|
||||
mock_fields, mock_seq, mock_branch, mock_docs, mock_enqueue
|
||||
):
|
||||
resp = client.post("/webhook/plane", json={
|
||||
"event": "issue", "action": "updated",
|
||||
"data": {
|
||||
"id": "taskB",
|
||||
# NO name, NO description in the payload (Plane status-change shape)
|
||||
"project": ENDURO_PLANE_ID,
|
||||
"state": {"id": IN_PROGRESS, "name": "In Progress", "group": "started"},
|
||||
},
|
||||
"activity": {"field": "state", "new_value": IN_PROGRESS, "old_value": BACKLOG},
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
mock_fields.assert_called_once()
|
||||
row = _task("taskB")
|
||||
assert row is not None
|
||||
branch = row["branch"]
|
||||
# slug derived from the fetched name -> "gpx-upload-feature", NOT untitled
|
||||
assert "untitled" not in branch
|
||||
assert "gpx-upload-feature" in branch
|
||||
# Title in the analyst task file is the fetched name, not "untitled"
|
||||
task_desc = mock_enqueue.call_args.args[2]
|
||||
assert "Title: GPX upload feature" in task_desc
|
||||
Reference in New Issue
Block a user