From 28b230a6e8ee7ba8f27303ca0bcae7b6b502a7d0 Mon Sep 17 00:00:00 2001 From: "MagicMock/mock.effective_git_name/132483215555760" Date: Tue, 26 May 2026 09:01:06 +0000 Subject: [PATCH 1/6] feat(responses): add WrapUpResponse model --- src/clayde/responses.py | 6 ++++++ tests/test_responses.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/clayde/responses.py b/src/clayde/responses.py index 79e20bf..ef1099d 100644 --- a/src/clayde/responses.py +++ b/src/clayde/responses.py @@ -10,6 +10,12 @@ class WorkResponse(BaseModel): summary: str +class WrapUpResponse(BaseModel): + title: str + body: str + success: bool + + def _extract_json(text: str) -> str: """Extract a JSON object from LLM output that may contain surrounding text. diff --git a/tests/test_responses.py b/tests/test_responses.py index fc94c17..c3dac20 100644 --- a/tests/test_responses.py +++ b/tests/test_responses.py @@ -4,6 +4,7 @@ from clayde.responses import ( WorkResponse, + WrapUpResponse, _extract_json, parse_response, ) @@ -88,3 +89,22 @@ def test_multiline_summary_preserved(self): text = json.dumps({"summary": content}) result = parse_response(text, WorkResponse) assert result.summary == content + + +class TestWrapUpResponse: + def test_parses_valid_payload(self): + raw = '{"title": "Wrap-up done", "body": "Fixed auth bug", "success": true}' + result = parse_response(raw, WrapUpResponse) + assert result.title == "Wrap-up done" + assert result.body == "Fixed auth bug" + assert result.success is True + + def test_parses_failure_payload(self): + raw = '{"title": "Wrap-up error", "body": "timeout", "success": false}' + result = parse_response(raw, WrapUpResponse) + assert result.success is False + + def test_parses_from_fenced_block(self): + raw = 'Some narrative\n```json\n{"title": "t", "body": "b", "success": true}\n```' + result = parse_response(raw, WrapUpResponse) + assert result.title == "t" From 90d5409de294bd1a88bd1a13f8620617e4623ff9 Mon Sep 17 00:00:00 2001 From: "MagicMock/mock.effective_git_name/132483215555760" Date: Tue, 26 May 2026 09:01:44 +0000 Subject: [PATCH 2/6] feat(github): add get_pull() helper --- src/clayde/github.py | 5 +++++ tests/test_github.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/clayde/github.py b/src/clayde/github.py index 8860951..82bdafa 100644 --- a/src/clayde/github.py +++ b/src/clayde/github.py @@ -178,3 +178,8 @@ def get_issue_author(g: Github, owner: str, repo: str, number: int) -> str: def get_pr_title(g: Github, owner: str, repo: str, pr_number: int) -> str: """Return the title of a pull request.""" return _get_repo(g, owner, repo).get_pull(pr_number).title + + +def get_pull(g: Github, owner: str, repo: str, pr_number: int): + """Return the PullRequest object for the given PR number.""" + return _get_repo(g, owner, repo).get_pull(pr_number) diff --git a/tests/test_github.py b/tests/test_github.py index 4267bdf..2a07485 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -22,6 +22,7 @@ is_blocked, parse_issue_url, parse_pr_url, + get_pull, post_comment, ) @@ -256,3 +257,18 @@ def test_returns_author_login(self): g = MagicMock() g.get_repo.return_value.get_issue.return_value.user.login = "alice" assert get_issue_author(g, "o", "r", 1) == "alice" + + +class TestGetPull: + def test_returns_pull_request_object(self): + mock_pr = MagicMock() + mock_repo = MagicMock() + mock_repo.get_pull.return_value = mock_pr + g = MagicMock() + g.get_repo.return_value = mock_repo + + result = get_pull(g, "owner", "repo", 42) + + g.get_repo.assert_called_once_with("owner/repo") + mock_repo.get_pull.assert_called_once_with(42) + assert result is mock_pr From 36e5c12222f1b642c7275e0b07df7780899013f3 Mon Sep 17 00:00:00 2001 From: "MagicMock/mock.effective_git_name/132483215555760" Date: Tue, 26 May 2026 09:02:14 +0000 Subject: [PATCH 3/6] feat(prompts): add wrap_up.j2 template --- src/clayde/prompts/wrap_up.j2 | 49 +++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 src/clayde/prompts/wrap_up.j2 diff --git a/src/clayde/prompts/wrap_up.j2 b/src/clayde/prompts/wrap_up.j2 new file mode 100644 index 0000000..6d47990 --- /dev/null +++ b/src/clayde/prompts/wrap_up.j2 @@ -0,0 +1,49 @@ +You are Clayde, an autonomous AI agent. A pull request you opened has just been merged on GitHub. + +## Context + +- Repository: {{ owner }}/{{ repo }} +- Issue: #{{ number }} — {{ title }} +- PR URL: {{ pr_url }} +- Branch: {{ branch_name }} +- Issue URL: {{ issue_url }} + +## Task + +Run the wrap-up skill. The skill file is at `skills/wrap-up/SKILL.md` relative to your current +working directory (the knowledge base root at `{{ kb_path }}`). + +Read the file first, then follow its complete workflow. The wrap-up skill invokes the reflect +skill — read `skills/reflect/SKILL.md` and follow that too. Both skills may reference the +capture skill — follow that chain as needed. + +## Non-Interactive Constraints + +You are running autonomously inside a container. Override any interactive step in the skills: + +1. **Do NOT edit** `CLAUDE.md`, `AGENTS.md`, `settings.json`, or any system config file. + Instead, write any proposed edit as a draft inbox note (path: `inbox/{{ today }}-clayde-proposals.md`). +2. **Do NOT ask the user for confirmation** — decide autonomously at every step. +3. **Capture everything** — for any "should I capture?" decision, yes. +4. **CLAUDE.md / AGENTS.md proposals** — write proposed wording as a section in the inbox draft, + not directly into the file. Prefix the section with `## Proposed CLAUDE.md edit:` or similar. +5. **Skill stub candidates** — write stub to `inbox/{{ today }}-clayde-proposals.md` as a section, + not to `skills/`. +6. **Do NOT invoke** `fewer-permission-prompts` or any skill that edits settings files. +7. **Worklog entry** — write it directly to the daily note (`daily/{{ today }}.md`). This is a + definite action, not a proposal. Use the issue title and PR URL as the session reference. + PR link format: `[{{ owner }}/{{ repo }}#{{ number }}]({{ pr_url }})`. +8. **Freeshard detection** — check if this work touched the freeshard repo + (`{{ owner }}/{{ repo }}` or concept keywords in the issue title). If yes, run `freeshard-share-scan` + with the same non-interactive constraints: write any share candidate to `inbox/` as a section. + +## Output + +When all wrap-up steps are complete, your LAST output MUST be a single fenced JSON block: + +```json +{"title": "", "body": "", "success": true} +``` + +Set `success` to `false` only if you could not complete the wrap-up. The `body` should match +the worklog entry you wrote to the daily note — this text will be sent as an ntfy notification. From 94a1b66e1ee6375e6c685890d615824ceb73b91e Mon Sep 17 00:00:00 2001 From: "MagicMock/mock.effective_git_name/132483215555760" Date: Tue, 26 May 2026 09:03:17 +0000 Subject: [PATCH 4/6] feat(tasks): add post-merge wrap_up task --- src/clayde/tasks/wrap_up.py | 102 +++++++++++++++++++++++ tests/test_tasks_wrap_up.py | 162 ++++++++++++++++++++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 src/clayde/tasks/wrap_up.py create mode 100644 tests/test_tasks_wrap_up.py diff --git a/src/clayde/tasks/wrap_up.py b/src/clayde/tasks/wrap_up.py new file mode 100644 index 0000000..c5205bc --- /dev/null +++ b/src/clayde/tasks/wrap_up.py @@ -0,0 +1,102 @@ +"""Post-merge wrap-up task — runs wrap-up skill in KB context, notifies via ntfy.""" + +from __future__ import annotations + +import logging +from datetime import date + +import requests + +from clayde.claude import InvocationTimeoutError, UsageLimitError, invoke_claude +from clayde.config import get_settings +from clayde.github import parse_issue_url +from clayde.prompts import render_template +from clayde.responses import WrapUpResponse, parse_response +from clayde.state import get_issue_state +from clayde.telemetry import get_tracer + +log = logging.getLogger("clayde.tasks.wrap_up") + + +def run(issue_url: str) -> None: + """Run post-merge wrap-up: invoke wrap-up skill in KB context, notify.""" + tracer = get_tracer() + with tracer.start_as_current_span("clayde.task.wrap_up") as span: + settings = get_settings() + owner, repo, number = parse_issue_url(issue_url) + issue_state = get_issue_state(issue_url) + + pr_url = issue_state.get("pr_url", "") + title = ( + issue_state.get("pr_title") + or issue_state.get("issue_title") + or "(unknown)" + ) + branch_name = issue_state.get("branch_name", f"clayde/issue-{number}") + + span.set_attribute("issue.number", number) + span.set_attribute("issue.owner", owner) + span.set_attribute("issue.repo", repo) + + log.info("[%s/%s#%d] Running post-merge wrap-up", owner, repo, number) + + prompt = render_template( + "wrap_up.j2", + owner=owner, + repo=repo, + number=number, + title=title, + pr_url=pr_url, + branch_name=branch_name, + issue_url=issue_url, + kb_path=settings.kb_path, + today=date.today().isoformat(), + ) + + ntfy_title = f"Wrapped up: {owner}/{repo}#{number}" + ntfy_body = title + success = False + + try: + result = invoke_claude(prompt, settings.kb_path) + try: + parsed = parse_response(result.output, WrapUpResponse) + ntfy_title = parsed.title + ntfy_body = parsed.body + success = parsed.success + except ValueError as e: + log.warning( + "[%s/%s#%d] Could not parse wrap-up JSON: %s", owner, repo, number, e + ) + ntfy_body = f"Wrap-up complete for {owner}/{repo}#{number} (no summary)" + success = True + span.set_attribute("wrap_up.success", success) + except (UsageLimitError, InvocationTimeoutError) as e: + log.warning("[%s/%s#%d] Wrap-up invoke failed: %s", owner, repo, number, e) + ntfy_title = f"Wrap-up failed: {owner}/{repo}#{number}" + ntfy_body = str(e)[:300] + except Exception as e: + log.error("[%s/%s#%d] Wrap-up unexpected error: %s", owner, repo, number, e) + ntfy_title = f"Wrap-up error: {owner}/{repo}#{number}" + ntfy_body = str(e)[:300] + + if settings.ntfy_topic: + _notify(title=ntfy_title, body=ntfy_body, success=success, settings=settings) + + +def _notify(*, title: str, body: str, success: bool, settings) -> None: + """Best-effort synchronous ntfy POST. Errors are logged, never raised.""" + url = f"{settings.ntfy_base_url.rstrip('/')}/{settings.ntfy_topic}" + try: + requests.post( + url, + data=body[:300].encode(), + headers={ + "Title": title[:40], + "Priority": "3" if success else "5", + "Tags": "white_check_mark" if success else "rotating_light", + }, + timeout=settings.ntfy_timeout_s, + ) + except Exception as exc: + log.warning("ntfy POST failed: %s", exc) diff --git a/tests/test_tasks_wrap_up.py b/tests/test_tasks_wrap_up.py new file mode 100644 index 0000000..132258d --- /dev/null +++ b/tests/test_tasks_wrap_up.py @@ -0,0 +1,162 @@ +"""Tests for clayde.tasks.wrap_up.""" + +from unittest.mock import MagicMock, patch + +from clayde.claude import InvocationResult, InvocationTimeoutError, UsageLimitError + + +def _make_result(output: str, cost_eur: float = 0.10) -> InvocationResult: + return InvocationResult(output=output, cost_eur=cost_eur, input_tokens=50, output_tokens=25) + + +def _mock_settings(): + s = MagicMock() + s.kb_path = "/fake/kb" + s.ntfy_topic = "testtopic" + s.ntfy_base_url = "https://ntfy.sh" + s.ntfy_timeout_s = 5 + return s + + +def _mock_state(): + return { + "owner": "o", + "repo": "r", + "number": 7, + "pr_url": "https://github.com/o/r/pull/7", + "branch_name": "clayde/issue-7", + "pr_title": "Fix login bug", + } + + +class TestRun: + def test_invokes_claude_with_kb_path(self): + output = ( + "wrap-up done\n" + "```json\n" + '{"title": "Wrapped up o/r#7", "body": "Fixed login bug", "success": true}\n' + "```" + ) + with patch("clayde.tasks.wrap_up.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)) as mock_claude, \ + patch("clayde.tasks.wrap_up._notify"): + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + mock_claude.assert_called_once() + # Second positional arg is repo_path (kb_path) + assert mock_claude.call_args[0][1] == "/fake/kb" + + def test_sends_ntfy_on_success(self): + output = ( + "```json\n" + '{"title": "Wrap done", "body": "Fixed it", "success": true}\n' + "```" + ) + with patch("clayde.tasks.wrap_up.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)), \ + patch("clayde.tasks.wrap_up._notify") as mock_notify: + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + mock_notify.assert_called_once_with( + title="Wrap done", + body="Fixed it", + success=True, + settings=mock_notify.call_args[1]["settings"], + ) + call_kw = mock_notify.call_args[1] + assert call_kw["title"] == "Wrap done" + assert call_kw["body"] == "Fixed it" + assert call_kw["success"] is True + + def test_sends_ntfy_on_usage_limit(self): + with patch("clayde.tasks.wrap_up.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", + side_effect=UsageLimitError("limit hit", cost_eur=0.5)), \ + patch("clayde.tasks.wrap_up._notify") as mock_notify: + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + mock_notify.assert_called_once() + assert mock_notify.call_args[1]["success"] is False + + def test_sends_ntfy_on_timeout(self): + with patch("clayde.tasks.wrap_up.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", + side_effect=InvocationTimeoutError("timed out", cost_eur=0.0)), \ + patch("clayde.tasks.wrap_up._notify") as mock_notify: + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + assert mock_notify.call_args[1]["success"] is False + + def test_fallback_on_bad_json(self): + """Wrap-up still notifies even when Claude outputs no valid JSON.""" + with patch("clayde.tasks.wrap_up.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", + return_value=_make_result("done, no json here")), \ + patch("clayde.tasks.wrap_up._notify") as mock_notify: + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + mock_notify.assert_called_once() + # Fallback assumes success + assert mock_notify.call_args[1]["success"] is True + + def test_notify_skipped_when_no_topic(self): + settings = _mock_settings() + settings.ntfy_topic = "" + output = '```json\n{"title":"t","body":"b","success":true}\n```' + with patch("clayde.tasks.wrap_up.get_settings", return_value=settings), \ + patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ + patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ + patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)), \ + patch("clayde.tasks.wrap_up._notify") as mock_notify: + from clayde.tasks.wrap_up import run + run("https://github.com/o/r/issues/7") + + mock_notify.assert_not_called() + + +class TestNotify: + def test_posts_to_ntfy(self): + settings = _mock_settings() + with patch("clayde.tasks.wrap_up.requests") as mock_req: + from clayde.tasks.wrap_up import _notify + _notify(title="hi", body="body text", success=True, settings=settings) + + mock_req.post.assert_called_once() + call_args = mock_req.post.call_args + assert "ntfy.sh/testtopic" in call_args[0][0] + headers = call_args[1]["headers"] + assert headers["Title"] == "hi" + assert headers["Priority"] == "3" + assert headers["Tags"] == "white_check_mark" + + def test_uses_failure_priority(self): + settings = _mock_settings() + with patch("clayde.tasks.wrap_up.requests") as mock_req: + from clayde.tasks.wrap_up import _notify + _notify(title="err", body="fail", success=False, settings=settings) + + headers = mock_req.post.call_args[1]["headers"] + assert headers["Priority"] == "5" + assert headers["Tags"] == "rotating_light" + + def test_swallows_errors(self): + settings = _mock_settings() + with patch("clayde.tasks.wrap_up.requests") as mock_req: + mock_req.post.side_effect = Exception("network down") + from clayde.tasks.wrap_up import _notify + _notify(title="x", body="y", success=True, settings=settings) # must not raise From 5a39121c143913e2ff11f6d90d735ac1c8162029 Mon Sep 17 00:00:00 2001 From: "MagicMock/mock.effective_git_name/130407769905136" Date: Tue, 26 May 2026 09:15:07 +0000 Subject: [PATCH 5/6] feat(orchestrator): detect PR merge; port wrap-up to event-driven loop review.py was removed in the event-driven refactor (PR #76). Port merge detection and wrap-up invocation to _handle_issue() in orchestrator.py. On each cycle, if the issue has a pr_url, check pr.merged before processing reviews. On first merge detection, set merged=True in state and run wrap_up.run() (errors caught, DONE not affected). Subsequent cycles skip wrap-up and return immediately. Unmerged PRs fall through to normal work invocation as before. --- src/clayde/orchestrator.py | 22 ++++++- tests/test_orchestrator.py | 125 +++++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/src/clayde/orchestrator.py b/src/clayde/orchestrator.py index 209d060..74006e2 100644 --- a/src/clayde/orchestrator.py +++ b/src/clayde/orchestrator.py @@ -36,6 +36,7 @@ get_assigned_issues, get_pr_review_comments, get_pr_reviews, + get_pull, is_blocked, issue_ref, parse_issue_url, @@ -43,7 +44,7 @@ ) from clayde.safety import get_new_visible_comments, has_visible_content from clayde.state import get_issue_state, load_state, save_state, update_issue_state -from clayde.tasks import work +from clayde.tasks import work, wrap_up from clayde.telemetry import get_tracer, init_tracer log = logging.getLogger("clayde.orchestrator") @@ -109,9 +110,26 @@ def _handle_issue(g: Github, issue: Issue, url: str) -> None: # Check for new visible comments since last cycle new_comments = get_new_visible_comments(comments, last_seen_at) + # Check for merged PR — run wrap-up once, then stop processing this issue + pr_url = issue_state.get("pr_url") + if pr_url: + try: + _, _, pr_number = parse_pr_url(pr_url) + pr = get_pull(g, owner, repo, pr_number) + if pr.merged: + if not issue_state.get("merged"): + log.info("[%s] PR #%d merged — running wrap-up", label, pr_number) + update_issue_state(url, {"merged": True}) + try: + wrap_up.run(url) + except Exception as e: + log.error("[%s] Wrap-up failed: %s", label, e) + return + except Exception as e: + log.warning("[%s] Failed to check PR merge status: %s", label, e) + # Check for new PR review activity has_new_review_activity = False - pr_url = issue_state.get("pr_url") if pr_url and last_seen_at is not None: try: _, _, pr_number = parse_pr_url(pr_url) diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index 86f6732..a1f8590 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -394,3 +394,128 @@ class _S: monkeypatch.setattr(orchestrator, "get_settings", lambda: _S()) orchestrator.run_loop() assert invoked.get("called") is True + + +class TestMergeDetection: + """Tests for PR merge detection and wrap-up invocation.""" + + def _base_patches(self, pr_url="https://github.com/o/r/pull/5", merged=True, already_wrapped=False): + """Return a list of common patch targets for merge-detection tests.""" + return [ + patch("clayde.orchestrator.is_blocked", return_value=False), + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), + patch("clayde.orchestrator.has_visible_content", return_value=True), + patch("clayde.orchestrator.get_issue_state", return_value={ + "pr_url": pr_url, + "last_seen_at": "2024-01-01T12:00:00+00:00", + "merged": already_wrapped, + }), + patch("clayde.orchestrator.parse_pr_url", return_value=("o", "r", 5)), + patch("clayde.orchestrator.get_pull", return_value=MagicMock(merged=merged)), + patch("clayde.orchestrator.update_issue_state"), + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), + ] + + def test_merged_pr_runs_wrap_up_once(self): + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + patches = self._base_patches(merged=True, already_wrapped=False) + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + patches[5], patches[6], patches[7], patches[8], \ + patch("clayde.orchestrator.wrap_up") as mock_wrap_up, \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + + mock_wrap_up.run.assert_called_once_with(issue.html_url) + mock_work.run.assert_not_called() + + def test_merged_pr_sets_merged_flag(self): + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + patches = self._base_patches(merged=True, already_wrapped=False) + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + patches[5], patches[6], patches[7] as mock_update, patches[8], \ + patch("clayde.orchestrator.wrap_up"), \ + patch("clayde.orchestrator.work"): + _handle_issue(g, issue, issue.html_url) + + assert any(c[0][1].get("merged") is True for c in mock_update.call_args_list) + + def test_already_wrapped_up_skips_wrap_up(self): + """Second cycle after merge: merged flag set → skip wrap-up, just return.""" + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + patches = self._base_patches(merged=True, already_wrapped=True) + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + patches[5], patches[6], patches[7], patches[8], \ + patch("clayde.orchestrator.wrap_up") as mock_wrap_up, \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + + mock_wrap_up.run.assert_not_called() + mock_work.run.assert_not_called() + + def test_wrap_up_failure_does_not_propagate(self): + """Wrap-up errors are caught — _handle_issue must not raise.""" + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + patches = self._base_patches(merged=True, already_wrapped=False) + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + patches[5], patches[6], patches[7], patches[8], \ + patch("clayde.orchestrator.wrap_up") as mock_wrap_up, \ + patch("clayde.orchestrator.work"): + mock_wrap_up.run.side_effect = Exception("wrap-up exploded") + _handle_issue(g, issue, issue.html_url) # must not raise + + def test_unmerged_pr_falls_through_to_work(self): + """PR exists but not merged → normal work invocation.""" + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + patches = self._base_patches(merged=False, already_wrapped=False) + with patches[0], patches[1], patches[2], patches[3], \ + patch("clayde.orchestrator.get_issue_state", return_value={ + "pr_url": "https://github.com/o/r/pull/5", + }), \ + patches[5], patches[6], patches[7], \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[MagicMock()]), \ + patch("clayde.orchestrator.wrap_up") as mock_wrap_up, \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + + mock_wrap_up.run.assert_not_called() + mock_work.run.assert_called_once_with(issue.html_url) + + def test_get_pull_failure_falls_through(self): + """get_pull error → logged and falls through to normal work.""" + g = MagicMock() + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Fix bug" + with patch("clayde.orchestrator.is_blocked", return_value=False), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", return_value={ + "pr_url": "https://github.com/o/r/pull/5", + }), \ + patch("clayde.orchestrator.parse_pr_url", return_value=("o", "r", 5)), \ + patch("clayde.orchestrator.get_pull", side_effect=Exception("API error")), \ + patch("clayde.orchestrator.update_issue_state"), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[MagicMock()]), \ + patch("clayde.orchestrator.wrap_up") as mock_wrap_up, \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + + mock_wrap_up.run.assert_not_called() + mock_work.run.assert_called_once_with(issue.html_url) From 7a63a8c01b75dd95428aa36f3ce905ea4c4c7c83 Mon Sep 17 00:00:00 2001 From: Clayde Date: Tue, 26 May 2026 13:20:55 +0000 Subject: [PATCH 6/6] refactor(wrap_up): address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Inbox filename now includes topic slug ({{ today }}-wrap-up-{{ topic }}.md) so multiple invocations on the same day don't clobber each other. - Replace inline _notify/_requests.post with send_ntfy_sync from webhook.notify — the shared helper that already existed for Pebble. - Update tests to mock send_ntfy_sync; drop TestNotify class that tested the now-removed _notify helper. Co-Authored-By: Claude Sonnet 4.6 --- src/clayde/prompts/wrap_up.j2 | 4 +-- src/clayde/tasks/wrap_up.py | 35 ++++++++++------------- src/clayde/webhook/notify.py | 34 +++++++++++++++++++++++ tests/test_tasks_wrap_up.py | 52 +++++------------------------------ 4 files changed, 57 insertions(+), 68 deletions(-) diff --git a/src/clayde/prompts/wrap_up.j2 b/src/clayde/prompts/wrap_up.j2 index 6d47990..cb2859c 100644 --- a/src/clayde/prompts/wrap_up.j2 +++ b/src/clayde/prompts/wrap_up.j2 @@ -22,12 +22,12 @@ capture skill — follow that chain as needed. You are running autonomously inside a container. Override any interactive step in the skills: 1. **Do NOT edit** `CLAUDE.md`, `AGENTS.md`, `settings.json`, or any system config file. - Instead, write any proposed edit as a draft inbox note (path: `inbox/{{ today }}-clayde-proposals.md`). + Instead, write any proposed edit as a draft inbox note (path: `inbox/{{ today }}-wrap-up-{{ topic }}.md`). 2. **Do NOT ask the user for confirmation** — decide autonomously at every step. 3. **Capture everything** — for any "should I capture?" decision, yes. 4. **CLAUDE.md / AGENTS.md proposals** — write proposed wording as a section in the inbox draft, not directly into the file. Prefix the section with `## Proposed CLAUDE.md edit:` or similar. -5. **Skill stub candidates** — write stub to `inbox/{{ today }}-clayde-proposals.md` as a section, +5. **Skill stub candidates** — write stub to `inbox/{{ today }}-wrap-up-{{ topic }}.md` as a section, not to `skills/`. 6. **Do NOT invoke** `fewer-permission-prompts` or any skill that edits settings files. 7. **Worklog entry** — write it directly to the daily note (`daily/{{ today }}.md`). This is a diff --git a/src/clayde/tasks/wrap_up.py b/src/clayde/tasks/wrap_up.py index c5205bc..b5f6ff6 100644 --- a/src/clayde/tasks/wrap_up.py +++ b/src/clayde/tasks/wrap_up.py @@ -3,10 +3,9 @@ from __future__ import annotations import logging +import re from datetime import date -import requests - from clayde.claude import InvocationTimeoutError, UsageLimitError, invoke_claude from clayde.config import get_settings from clayde.github import parse_issue_url @@ -14,6 +13,7 @@ from clayde.responses import WrapUpResponse, parse_response from clayde.state import get_issue_state from clayde.telemetry import get_tracer +from clayde.webhook.notify import send_ntfy_sync log = logging.getLogger("clayde.tasks.wrap_up") @@ -34,6 +34,9 @@ def run(issue_url: str) -> None: ) branch_name = issue_state.get("branch_name", f"clayde/issue-{number}") + words = re.sub(r"[^a-z0-9\s]", "", title.lower()).split()[:3] + title_slug = "-".join(words) if words else "issue" + span.set_attribute("issue.number", number) span.set_attribute("issue.owner", owner) span.set_attribute("issue.repo", repo) @@ -51,6 +54,7 @@ def run(issue_url: str) -> None: issue_url=issue_url, kb_path=settings.kb_path, today=date.today().isoformat(), + topic=title_slug, ) ntfy_title = f"Wrapped up: {owner}/{repo}#{number}" @@ -81,22 +85,11 @@ def run(issue_url: str) -> None: ntfy_body = str(e)[:300] if settings.ntfy_topic: - _notify(title=ntfy_title, body=ntfy_body, success=success, settings=settings) - - -def _notify(*, title: str, body: str, success: bool, settings) -> None: - """Best-effort synchronous ntfy POST. Errors are logged, never raised.""" - url = f"{settings.ntfy_base_url.rstrip('/')}/{settings.ntfy_topic}" - try: - requests.post( - url, - data=body[:300].encode(), - headers={ - "Title": title[:40], - "Priority": "3" if success else "5", - "Tags": "white_check_mark" if success else "rotating_light", - }, - timeout=settings.ntfy_timeout_s, - ) - except Exception as exc: - log.warning("ntfy POST failed: %s", exc) + send_ntfy_sync( + title=ntfy_title, + body=ntfy_body, + success=success, + base_url=settings.ntfy_base_url, + topic=settings.ntfy_topic, + timeout_s=settings.ntfy_timeout_s, + ) diff --git a/src/clayde/webhook/notify.py b/src/clayde/webhook/notify.py index c0aa5a3..2549bf5 100644 --- a/src/clayde/webhook/notify.py +++ b/src/clayde/webhook/notify.py @@ -58,6 +58,40 @@ def _clamp_body(cls, v): return v[:300] if isinstance(v, str) else v +def send_ntfy_sync( + *, + title: str, + body: str, + success: bool, + base_url: str, + topic: str, + timeout_s: int, +) -> None: + """Synchronous POST to ntfy.sh. Best-effort: errors are logged, never raised.""" + url = f"{base_url.rstrip('/')}/{topic}" + headers = { + "Title": _encode_header_value(title[:40]), + "Priority": "3" if success else "5", + "Tags": "white_check_mark" if success else "rotating_light", + } + tracer = get_tracer() + with tracer.start_as_current_span("clayde.pebble.notify") as span: + span.set_attribute("pebble.notify_topic", topic) + span.set_attribute("pebble.notify_title", title) + span.set_attribute("pebble.outcome_success", success) + try: + with httpx.Client(timeout=timeout_s) as client: + resp = client.post(url, content=body[:300], headers=headers) + span.set_attribute("pebble.notify_http_status", resp.status_code) + span.set_attribute("pebble.notify_ok", 200 <= resp.status_code < 300) + if resp.status_code >= 400: + log.warning("ntfy returned %d: %s", resp.status_code, resp.text[:200]) + except Exception as exc: + span.set_attribute("pebble.notify_ok", False) + span.set_attribute("pebble.notify_error", type(exc).__name__) + log.warning("ntfy POST failed: %s", exc) + + async def send_ntfy( *, title: str, diff --git a/tests/test_tasks_wrap_up.py b/tests/test_tasks_wrap_up.py index 132258d..452c049 100644 --- a/tests/test_tasks_wrap_up.py +++ b/tests/test_tasks_wrap_up.py @@ -41,7 +41,7 @@ def test_invokes_claude_with_kb_path(self): patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)) as mock_claude, \ - patch("clayde.tasks.wrap_up._notify"): + patch("clayde.tasks.wrap_up.send_ntfy_sync"): from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") @@ -59,16 +59,11 @@ def test_sends_ntfy_on_success(self): patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)), \ - patch("clayde.tasks.wrap_up._notify") as mock_notify: + patch("clayde.tasks.wrap_up.send_ntfy_sync") as mock_notify: from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") - mock_notify.assert_called_once_with( - title="Wrap done", - body="Fixed it", - success=True, - settings=mock_notify.call_args[1]["settings"], - ) + mock_notify.assert_called_once() call_kw = mock_notify.call_args[1] assert call_kw["title"] == "Wrap done" assert call_kw["body"] == "Fixed it" @@ -80,7 +75,7 @@ def test_sends_ntfy_on_usage_limit(self): patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", side_effect=UsageLimitError("limit hit", cost_eur=0.5)), \ - patch("clayde.tasks.wrap_up._notify") as mock_notify: + patch("clayde.tasks.wrap_up.send_ntfy_sync") as mock_notify: from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") @@ -93,7 +88,7 @@ def test_sends_ntfy_on_timeout(self): patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", side_effect=InvocationTimeoutError("timed out", cost_eur=0.0)), \ - patch("clayde.tasks.wrap_up._notify") as mock_notify: + patch("clayde.tasks.wrap_up.send_ntfy_sync") as mock_notify: from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") @@ -106,7 +101,7 @@ def test_fallback_on_bad_json(self): patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result("done, no json here")), \ - patch("clayde.tasks.wrap_up._notify") as mock_notify: + patch("clayde.tasks.wrap_up.send_ntfy_sync") as mock_notify: from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") @@ -122,41 +117,8 @@ def test_notify_skipped_when_no_topic(self): patch("clayde.tasks.wrap_up.get_issue_state", return_value=_mock_state()), \ patch("clayde.tasks.wrap_up.parse_issue_url", return_value=("o", "r", 7)), \ patch("clayde.tasks.wrap_up.invoke_claude", return_value=_make_result(output)), \ - patch("clayde.tasks.wrap_up._notify") as mock_notify: + patch("clayde.tasks.wrap_up.send_ntfy_sync") as mock_notify: from clayde.tasks.wrap_up import run run("https://github.com/o/r/issues/7") mock_notify.assert_not_called() - - -class TestNotify: - def test_posts_to_ntfy(self): - settings = _mock_settings() - with patch("clayde.tasks.wrap_up.requests") as mock_req: - from clayde.tasks.wrap_up import _notify - _notify(title="hi", body="body text", success=True, settings=settings) - - mock_req.post.assert_called_once() - call_args = mock_req.post.call_args - assert "ntfy.sh/testtopic" in call_args[0][0] - headers = call_args[1]["headers"] - assert headers["Title"] == "hi" - assert headers["Priority"] == "3" - assert headers["Tags"] == "white_check_mark" - - def test_uses_failure_priority(self): - settings = _mock_settings() - with patch("clayde.tasks.wrap_up.requests") as mock_req: - from clayde.tasks.wrap_up import _notify - _notify(title="err", body="fail", success=False, settings=settings) - - headers = mock_req.post.call_args[1]["headers"] - assert headers["Priority"] == "5" - assert headers["Tags"] == "rotating_light" - - def test_swallows_errors(self): - settings = _mock_settings() - with patch("clayde.tasks.wrap_up.requests") as mock_req: - mock_req.post.side_effect = Exception("network down") - from clayde.tasks.wrap_up import _notify - _notify(title="x", body="y", success=True, settings=settings) # must not raise