diff --git a/.agents/skills/docs-seo-audit/SKILL.md b/.agents/skills/docs-seo-audit/SKILL.md index 3e2722c87..ecfc985a7 100644 --- a/.agents/skills/docs-seo-audit/SKILL.md +++ b/.agents/skills/docs-seo-audit/SKILL.md @@ -240,8 +240,12 @@ After making fixes, review every change before presenting to the user. Run throu If instructed to send a report to Slack, post a summary after the audit completes. This works regardless of whether fixes were made. +**Post at most once per run.** This mirrors the "Never post twice for one run" rule in `.agents/references/skill-authoring-guidelines.md` and the same-run guard in `weekly-404-monitor`. Revise the summary wording as many times as you like before sending — never after. Once a post attempt succeeds (`ok: true`), treat the notification as terminal for this run: do not re-post to fix a typo, tighten wording, or add a detail you forgot. A second, "cleaned up" post is a worse outcome than an imperfect first one. + +**Check for an existing same-day post before sending.** Before posting, check whether a top-level message matching `*SEO Audit — *` already exists in the target channel — this catches both a rerun of this agent for the same day and a mid-run retry after an apparent failure that actually succeeded. Skip the post if one is found. + 1. Check if `BUZZ_SLACK_TOKEN` environment variable exists. -2. If the token exists, send a summary to the channel the user specified (or the channel configured in the agent's instructions). +2. If the token exists, compose the summary (see the categorization and format rules below) and send it with the one-shot poster script in the "Sending the notification" section, which performs the dedupe check and the post in a single invocation. **Categorizing issues in the summary:** Before composing the message, cross-reference every issue against the title exceptions list above and check whether the issue has a local source file. Classify each issue into exactly one bucket: - **Fixed** — issues you resolved in this run @@ -299,21 +303,22 @@ PR: • titles too short/long ``` -Send using: +### Sending the notification + +Use the one-shot poster script rather than a raw `curl chat.postMessage` call. `curl` has no dedupe check and no `ok` verification, which is exactly the gap that let this skill post the same run's summary twice: an agent revised the wording mid-run and re-sent instead of treating the first successful post as final. The script (following the `aeo_crosslink_audit` one-shot-poster pattern) reads the token from the environment, checks channel history for an existing same-day post, and posts only when none is found: ```bash -curl -X POST https://slack.com/api/chat.postMessage \ - -H "Authorization: Bearer $BUZZ_SLACK_TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "channel": "", - "text": "", - "unfurl_links": false, - "unfurl_media": false - }' +cat > /tmp/seo-audit-summary.txt << 'EOF' + +EOF + +python3 .agents/skills/docs-seo-audit/scripts/notify_seo_slack.py \ + --channel "" \ + --date "$(date +%Y-%m-%d)" \ + --message-file /tmp/seo-audit-summary.txt ``` -If `BUZZ_SLACK_TOKEN` is not set, skip the notification and note that the token is required. +The script exits `0` whether it skipped (dedupe hit), posted successfully, or found no token. It exits non-zero both for an actual Slack API post failure and for a same-day history check that could not be verified (a paginated `conversations.history` call failing or erroring) — an unverified history check is never treated as an empty one, since that would risk posting a duplicate on exactly the ambiguous retry path this script exists to prevent. A non-zero exit means the post did not go through, so a retry there is a fresh attempt, not a duplicate of a completed post; do not retry after a `0` exit. If `BUZZ_SLACK_TOKEN` is not set, the script skips the notification and says so on stderr — no separate check is needed. ## Dependencies diff --git a/.agents/skills/docs-seo-audit/scripts/notify_seo_slack.py b/.agents/skills/docs-seo-audit/scripts/notify_seo_slack.py new file mode 100644 index 000000000..ca766d577 --- /dev/null +++ b/.agents/skills/docs-seo-audit/scripts/notify_seo_slack.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +"""One-shot Slack poster for the SEO audit summary, with same-day dedupe. + +Ensures a single audit run never posts its Slack summary more than once, +even across a "let me clean up the wording" retry: it checks the channel +for an existing top-level SEO Audit message dated today before posting, +and skips when one is already there. There is no separate post-then-check +step to race against, so an agent that runs this twice in one turn cannot +produce two messages. + +Usage: + python3 notify_seo_slack.py --channel CHANNEL_ID --date YYYY-MM-DD \ + --message-file /path/to/summary.txt + +Exits 0 when the notification was posted, skipped due to dedupe, or +skipped because the token env var is unset. Exits non-zero when the +Slack API fails to post, or when the same-day history check itself +could not be verified — an unverified history check is treated as a +failure rather than an empty history, so a duplicate is never posted +on the ambiguous retry path. A non-zero exit is the only case that +warrants a retry. +""" +import argparse +import datetime +import json +import os +import sys +import urllib.error +import urllib.request + +SLACK_API = "https://slack.com/api" + + +def is_top_level(message: dict) -> bool: + """A message is top-level (not a reply within a thread) when it has no + ``thread_ts``, or ``thread_ts`` equals its own ``ts`` (the parent of a + thread is still that thread's original top-level post).""" + thread_ts = message.get("thread_ts") + return thread_ts is None or thread_ts == message.get("ts") + + +def find_existing_post(messages: list, date_str: str) -> bool: + """Return True if ``messages`` already contains a top-level SEO Audit + summary for ``date_str`` (e.g. ``2026-08-28``).""" + prefix = f"*SEO Audit — {date_str}*" + for message in messages: + if not is_top_level(message): + continue + if message.get("text", "").startswith(prefix): + return True + return False + + +def day_start_ts(date_str: str) -> float: + """Return the Unix timestamp for the start (UTC midnight) of ``date_str``, + used as the ``oldest`` bound so pagination can stop once history is older + than the target date.""" + day = datetime.datetime.strptime(date_str, "%Y-%m-%d").replace(tzinfo=datetime.timezone.utc) + return day.timestamp() + + +def fetch_messages_for_date(token: str, channel: str, date_str: str, page_size: int = 200) -> list: + """Fetch every message posted on or after the start of ``date_str`` by + paginating ``conversations.history`` with ``oldest`` set to that day's + start. A fixed-size single page can push an earlier same-day summary out + of a busy channel's dedupe window, so this keeps requesting the next + cursor until Slack reports no more pages.""" + oldest = day_start_ts(date_str) + messages = [] + cursor = None + while True: + url = ( + f"{SLACK_API}/conversations.history?channel={channel}" + f"&limit={page_size}&oldest={oldest}" + ) + if cursor: + url += f"&cursor={cursor}" + req = urllib.request.Request(url, headers={"Authorization": f"Bearer {token}"}) + with urllib.request.urlopen(req) as resp: + result = json.load(resp) + if not result.get("ok"): + raise RuntimeError(f"conversations.history failed: {result.get('error')}") + messages.extend(result.get("messages", [])) + cursor = result.get("response_metadata", {}).get("next_cursor") + if not cursor: + break + return messages + + +def post_message(token: str, channel: str, text: str) -> dict: + payload = json.dumps( + { + "channel": channel, + "text": text, + "unfurl_links": False, + "unfurl_media": False, + } + ).encode() + req = urllib.request.Request( + f"{SLACK_API}/chat.postMessage", + data=payload, + headers={"Authorization": f"Bearer {token}", "Content-Type": "application/json"}, + ) + with urllib.request.urlopen(req) as resp: + return json.load(resp) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--channel", required=True, help="Slack channel ID to post to") + parser.add_argument("--date", required=True, help="Today's date, e.g. 2026-08-28") + parser.add_argument("--message-file", required=True, help="Path to the message text to post") + parser.add_argument("--token-env", default="BUZZ_SLACK_TOKEN", help="Env var holding the Slack bot token") + args = parser.parse_args() + + token = os.environ.get(args.token_env, "") + if not token: + print(f"{args.token_env} not set — skipping Slack notification", file=sys.stderr) + return 0 + + with open(args.message_file, "r", encoding="utf-8") as f: + message = f.read() + + try: + recent = fetch_messages_for_date(token, args.channel, args.date) + except (urllib.error.URLError, RuntimeError) as exc: + # An unverified history check can't rule out an earlier same-day post, + # so fail closed rather than risk sending a duplicate: report the + # failure and skip the post instead of posting on an empty history. + print(f"error: could not check channel history ({exc}); skipping post to avoid a possible duplicate", file=sys.stderr) + return 1 + + if find_existing_post(recent, args.date): + print(f"Skipping post: a SEO Audit summary for {args.date} already exists in {args.channel}.") + return 0 + + try: + result = post_message(token, args.channel, message) + except urllib.error.URLError as exc: + print(f"Slack post failed: {exc}", file=sys.stderr) + return 1 + + if not result.get("ok"): + print(f"Slack error: {result.get('error')}", file=sys.stderr) + return 1 + + print("Posted SEO Audit summary to Slack.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.agents/skills/docs-seo-audit/test_notify_seo_slack.py b/.agents/skills/docs-seo-audit/test_notify_seo_slack.py new file mode 100644 index 000000000..1e68cb302 --- /dev/null +++ b/.agents/skills/docs-seo-audit/test_notify_seo_slack.py @@ -0,0 +1,218 @@ +#!/usr/bin/env python3 +"""Regression cases for the docs-seo-audit Slack same-day dedupe guard. + +`find_existing_post()` is what stops the skill from posting a second SEO +Audit summary for a run that already succeeded (JAS-3: two chat.postMessage +calls ~11s apart for the same audit run, one with rough wording and one +cleaned up). These cases pin that a same-day top-level post is detected +regardless of exact text after the date prefix, that a different day or a +thread reply is not mistaken for one, and that an empty history never +blocks the first post. + +`fetch_messages_for_date()` and `main()` are also covered here: a same-day +post that only shows up on a later history page must still be found (a +fixed single-page window can miss it in a busy channel), and a failed +history check must fail closed — skipping the post — rather than treating +an unverified history as empty and posting anyway. + +Run from the repo root: + python3 .agents/skills/docs-seo-audit/test_notify_seo_slack.py +""" +import importlib.util +import json +import pathlib +import sys +import tempfile +import urllib.error +from unittest import mock + +HERE = pathlib.Path(__file__).parent +spec = importlib.util.spec_from_file_location("notify_seo_slack", HERE / "scripts" / "notify_seo_slack.py") +notify_seo_slack = importlib.util.module_from_spec(spec) +spec.loader.exec_module(notify_seo_slack) + +DATE = "2026-08-28" + + +class _FakeResponse: + """Minimal context-manager stand-in for the object urlopen() returns.""" + + def __init__(self, payload: dict): + self._body = json.dumps(payload).encode() + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return False + + def read(self): + return self._body + +# (description, messages, expected find_existing_post result) +CASES = [ + ( + "no messages at all", + [], + False, + ), + ( + "top-level message already posted for today, exact wording", + [{"ts": "1.0", "text": "*SEO Audit — 2026-08-28*\n276 pages scanned | ✅ No issues found"}], + True, + ), + ( + "top-level message already posted for today, different wording after the prefix", + [{"ts": "1.0", "text": "*SEO Audit — 2026-08-28*\nSome other draft of the same summary"}], + True, + ), + ( + "message is for a different date", + [{"ts": "1.0", "text": "*SEO Audit — 2026-08-21*\n183 issues found"}], + False, + ), + ( + "matching text but it's a thread reply, not a top-level post", + [{"ts": "2.0", "thread_ts": "1.0", "text": "*SEO Audit — 2026-08-28*\nfollow-up in thread"}], + False, + ), + ( + "thread parent (thread_ts == ts) still counts as top-level", + [{"ts": "1.0", "thread_ts": "1.0", "text": "*SEO Audit — 2026-08-28*\nhas replies now"}], + True, + ), + ( + "unrelated top-level message present, no SEO Audit post yet", + [{"ts": "1.0", "text": "good morning docs team"}], + False, + ), +] + + +def _paged_urlopen(pages: list): + """Return a urlopen() stand-in that serves ``pages`` in order, one per + call, ignoring the request URL beyond counting calls.""" + call_count = {"n": 0} + + def _urlopen(req, *args, **kwargs): + index = min(call_count["n"], len(pages) - 1) + call_count["n"] += 1 + return _FakeResponse(pages[index]) + + return _urlopen + + +def check_pagination_finds_later_page_match() -> bool: + """A same-day post that only appears on the second history page must + still be found — the fixed 50-message single page this regresses could + miss it in a busy channel.""" + page_1 = { + "ok": True, + "messages": [{"ts": "3.0", "text": "unrelated chatter"}], + "response_metadata": {"next_cursor": "cursor-abc"}, + } + page_2 = { + "ok": True, + "messages": [{"ts": "1.0", "text": f"*SEO Audit — {DATE}*\nfound on a later page"}], + "response_metadata": {}, + } + with mock.patch.object(notify_seo_slack.urllib.request, "urlopen", _paged_urlopen([page_1, page_2])): + messages = notify_seo_slack.fetch_messages_for_date("tok", "C123", DATE, page_size=1) + return notify_seo_slack.find_existing_post(messages, DATE) is True + + +def check_fetch_stops_pagination_when_no_next_cursor() -> bool: + """A response with no ``next_cursor`` ends pagination after a single + page instead of looping forever.""" + page_1 = {"ok": True, "messages": [{"ts": "1.0", "text": "only page"}], "response_metadata": {}} + with mock.patch.object(notify_seo_slack.urllib.request, "urlopen", _paged_urlopen([page_1])): + messages = notify_seo_slack.fetch_messages_for_date("tok", "C123", DATE) + return messages == [{"ts": "1.0", "text": "only page"}] + + +def check_main_fails_closed_on_history_error() -> bool: + """When the history check itself fails, main() must skip the post + (non-zero exit, no chat.postMessage call) rather than treat the + unverified history as empty and post anyway.""" + with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as f: + f.write("*SEO Audit — 2026-08-28*\nsummary body") + message_file = f.name + + argv = [ + "notify_seo_slack.py", + "--channel", "C123", + "--date", DATE, + "--message-file", message_file, + "--token-env", "TEST_SLACK_TOKEN", + ] + with mock.patch.object(sys, "argv", argv), \ + mock.patch.dict(notify_seo_slack.os.environ, {"TEST_SLACK_TOKEN": "xoxb-test"}), \ + mock.patch.object( + notify_seo_slack, "fetch_messages_for_date", + side_effect=RuntimeError("conversations.history failed: ratelimited"), + ), \ + mock.patch.object(notify_seo_slack, "post_message") as post_mock: + exit_code = notify_seo_slack.main() + + return exit_code != 0 and not post_mock.called + + +def check_main_fails_closed_on_url_error() -> bool: + """A network-level failure (URLError) during the history check must + also fail closed, not just the RuntimeError (Slack ``ok: false``) case.""" + with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as f: + f.write("*SEO Audit — 2026-08-28*\nsummary body") + message_file = f.name + + argv = [ + "notify_seo_slack.py", + "--channel", "C123", + "--date", DATE, + "--message-file", message_file, + "--token-env", "TEST_SLACK_TOKEN", + ] + with mock.patch.object(sys, "argv", argv), \ + mock.patch.dict(notify_seo_slack.os.environ, {"TEST_SLACK_TOKEN": "xoxb-test"}), \ + mock.patch.object( + notify_seo_slack, "fetch_messages_for_date", + side_effect=urllib.error.URLError("connection refused"), + ), \ + mock.patch.object(notify_seo_slack, "post_message") as post_mock: + exit_code = notify_seo_slack.main() + + return exit_code != 0 and not post_mock.called + + +BEHAVIOR_CHECKS = [ + ("same-day post found on a later pagination page", check_pagination_finds_later_page_match), + ("pagination stops when a page has no next_cursor", check_fetch_stops_pagination_when_no_next_cursor), + ("main() fails closed (skips posting) on a Slack API history error", check_main_fails_closed_on_history_error), + ("main() fails closed (skips posting) on a network history error", check_main_fails_closed_on_url_error), +] + + +def main() -> int: + failures = 0 + + for description, messages, expected in CASES: + result = notify_seo_slack.find_existing_post(messages, DATE) + ok = result == expected + failures += 0 if ok else 1 + print(f" [{'PASS' if ok else 'FAIL'}] {description:<70} got={result}") + + for description, check in BEHAVIOR_CHECKS: + ok = check() + failures += 0 if ok else 1 + print(f" [{'PASS' if ok else 'FAIL'}] {description:<70}") + + total = len(CASES) + len(BEHAVIOR_CHECKS) + print() + if failures: + print(f"{failures} of {total} cases regressed.") + return 1 + print(f"All {total} cases behave correctly.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())