diff --git a/skills/pr-management-triage/SKILL.md b/skills/pr-management-triage/SKILL.md index 07b5d961..3d59e9c5 100644 --- a/skills/pr-management-triage/SKILL.md +++ b/skills/pr-management-triage/SKILL.md @@ -386,6 +386,32 @@ be one click away in whatever surface it lands on: `https://github.com//actions/runs/` when citing a failing CI run. +### Terminal PR-reference renderer + +Use the bundled [`pr_link.py`](scripts/pr_link.py) helper for every +terminal-bound PR reference instead of constructing OSC 8 sequences +inside individual output paths: + +```bash +python3 /skills/pr-management-triage/scripts/pr_link.py \ + '#NNN' + +# When the repository is obvious and only #NNN should be visible: +python3 /skills/pr-management-triage/scripts/pr_link.py \ + --repo '' '#NNN' +``` + +The helper accepts `#NNN`, the full GitHub pull-request URL, or +`#NNN` with `--repo `. It preserves the visible form and always +targets the canonical `https://github.com///pull/` URL. +When `TERM` is unset or `dumb`, or `NO_COLOR` is present, it falls back to +plain text plus the URL. + +Every terminal output path goes through this helper: fetch or apply progress +lines that name a PR, classifier proposals, group and per-PR drill-in screens, +error messages, and the Step 6 session summary. Do not build a one-off OSC 8 +wrapper in any of those paths. + - **On terminal surfaces** (the group screen, the per-PR drill-in screen, the Step 6 session summary): wrap the visible short form `#NNN` (or `#NNN`) in **OSC 8 hyperlink escape @@ -403,6 +429,8 @@ emitting any user-visible screen**: grep the body for bare `#\d+` / `#\d+` tokens that aren't already inside a markdown link or an OSC 8 wrapper, and convert any match. +### Contributor-facing notification channel + **Golden rule 11 — deliver violation feedback through the configured channel, and default to the silent one.** The deterministic quality-violation feedback for `draft`, `comment` diff --git a/skills/pr-management-triage/interaction-loop.md b/skills/pr-management-triage/interaction-loop.md index 79b633b1..9d70fb91 100644 --- a/skills/pr-management-triage/interaction-loop.md +++ b/skills/pr-management-triage/interaction-loop.md @@ -18,6 +18,13 @@ The core idea: > the group, pulls individual PRs out for closer inspection, or > skips the group. +Every `#NNN` token in the examples below represents output from the +[`pr_link.py`](scripts/pr_link.py) terminal renderer described by +[Golden rule 10](SKILL.md#terminal-pr-reference-renderer). The examples omit +control characters for readability; the live group screen, drill-in view, +action progress / error lines, and summary must never print the literal bare +token. + The underlying `breeze pr auto-triage` tool presented PRs one- at-a-time (sequential mode) or as a TUI list with per-PR keys. This skill lands between those: sequential per-group, with a diff --git a/skills/pr-management-triage/scripts/pr_link.py b/skills/pr-management-triage/scripts/pr_link.py new file mode 100644 index 00000000..f35f2095 --- /dev/null +++ b/skills/pr-management-triage/scripts/pr_link.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Render GitHub pull-request references for terminal output.""" + +from __future__ import annotations + +import argparse +import os +import re +from collections.abc import Mapping, Sequence + +_SHORT_REFERENCE = re.compile( + r"(?P[A-Za-z0-9_.-]+)/(?P[A-Za-z0-9_.-]+)#(?P[1-9][0-9]*)\Z" +) +_URL_REFERENCE = re.compile( + r"https://github\.com/(?P[A-Za-z0-9_.-]+)/" + r"(?P[A-Za-z0-9_.-]+)/pull/(?P[1-9][0-9]*)/?\Z" +) +_NUMBER_REFERENCE = re.compile(r"#(?P[1-9][0-9]*)\Z") +_REPOSITORY = re.compile(r"(?P[A-Za-z0-9_.-]+)/(?P[A-Za-z0-9_.-]+)\Z") + + +def parse_pr_reference( + reference: str, repository: str | None = None +) -> tuple[str, str]: + """Return the display text and canonical URL for a GitHub PR reference.""" + short_match = _SHORT_REFERENCE.fullmatch(reference) + if short_match is not None: + display = reference + owner = short_match.group("owner") + repo = short_match.group("repo") + number = short_match.group("number") + else: + url_match = _URL_REFERENCE.fullmatch(reference) + if url_match is not None: + display = ( + f"https://github.com/{url_match.group('owner')}/" + f"{url_match.group('repo')}/pull/{url_match.group('number')}" + ) + owner = url_match.group("owner") + repo = url_match.group("repo") + number = url_match.group("number") + else: + number_match = _NUMBER_REFERENCE.fullmatch(reference) + repository_match = ( + _REPOSITORY.fullmatch(repository) if repository is not None else None + ) + if number_match is None or repository_match is None: + raise ValueError( + "expected OWNER/REPO#NUMBER, " + "https://github.com/OWNER/REPO/pull/NUMBER, or " + "#NUMBER with --repo OWNER/REPO" + ) + display = reference + owner = repository_match.group("owner") + repo = repository_match.group("repo") + number = number_match.group("number") + + url = f"https://github.com/{owner}/{repo}/pull/{number}" + return display, url + + +def terminal_supports_hyperlinks(environ: Mapping[str, str] | None = None) -> bool: + """Return whether the environment indicates OSC 8 support.""" + env = os.environ if environ is None else environ + if "NO_COLOR" in env: + return False + term = env.get("TERM", "") + return bool(term) and term.lower() != "dumb" + + +def format_pr_reference( + reference: str, + environ: Mapping[str, str] | None = None, + *, + repository: str | None = None, +) -> str: + """Format a PR reference as an OSC 8 link or a plain-text fallback.""" + display, url = parse_pr_reference(reference, repository) + if terminal_supports_hyperlinks(environ): + return f"\033]8;;{url}\033\\{display}\033]8;;\033\\" + if display == url: + return url + return f"{display} {url}" + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Render GitHub pull-request references for terminal output." + ) + parser.add_argument( + "references", + nargs="+", + metavar="PR", + help="OWNER/REPO#NUMBER or a canonical GitHub pull-request URL", + ) + parser.add_argument( + "--repo", + metavar="OWNER/REPO", + help="repository context required for references in #NUMBER form", + ) + args = parser.parse_args(argv) + + try: + rendered = [ + format_pr_reference(reference, repository=args.repo) + for reference in args.references + ] + except ValueError as error: + parser.error(str(error)) + + print("\n".join(rendered)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/skills/pr-management-triage/tests/test_pr_link.py b/skills/pr-management-triage/tests/test_pr_link.py new file mode 100644 index 00000000..7005643e --- /dev/null +++ b/skills/pr-management-triage/tests/test_pr_link.py @@ -0,0 +1,112 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) + +import pr_link # noqa: E402 + + +class PrLinkTest(unittest.TestCase): + def test_short_reference_uses_canonical_target(self) -> None: + rendered = pr_link.format_pr_reference( + "example/widget#66444", {"TERM": "xterm-256color"} + ) + + self.assertEqual( + rendered, + "\033]8;;https://github.com/example/widget/pull/66444\033\\" + "example/widget#66444\033]8;;\033\\", + ) + + def test_url_reference_is_normalised(self) -> None: + rendered = pr_link.format_pr_reference( + "https://github.com/example/widget/pull/66444/", + {"TERM": "xterm-256color"}, + ) + + self.assertEqual( + rendered, + "\033]8;;https://github.com/example/widget/pull/66444\033\\" + "https://github.com/example/widget/pull/66444\033]8;;\033\\", + ) + + def test_number_reference_uses_repository_context(self) -> None: + rendered = pr_link.format_pr_reference( + "#66444", + {"TERM": "xterm-256color"}, + repository="example/widget", + ) + + self.assertEqual( + rendered, + "\033]8;;https://github.com/example/widget/pull/66444\033\\" + "#66444\033]8;;\033\\", + ) + + def test_no_color_uses_plain_text_fallback(self) -> None: + rendered = pr_link.format_pr_reference( + "example/widget#66444", {"TERM": "xterm-256color", "NO_COLOR": ""} + ) + + self.assertEqual( + rendered, + "example/widget#66444 https://github.com/example/widget/pull/66444", + ) + + def test_dumb_terminal_uses_plain_text_fallback(self) -> None: + rendered = pr_link.format_pr_reference("example/widget#66444", {"TERM": "dumb"}) + + self.assertEqual( + rendered, + "example/widget#66444 https://github.com/example/widget/pull/66444", + ) + + def test_missing_term_uses_plain_text_fallback(self) -> None: + rendered = pr_link.format_pr_reference("example/widget#66444", {}) + + self.assertEqual( + rendered, + "example/widget#66444 https://github.com/example/widget/pull/66444", + ) + + def test_plain_url_is_not_duplicated(self) -> None: + rendered = pr_link.format_pr_reference( + "https://github.com/example/widget/pull/66444", {"TERM": "dumb"} + ) + + self.assertEqual(rendered, "https://github.com/example/widget/pull/66444") + + def test_non_pr_reference_is_rejected(self) -> None: + with self.assertRaises(ValueError): + pr_link.format_pr_reference( + "https://github.com/example/widget/issues/66444", + {"TERM": "xterm-256color"}, + ) + + def test_number_reference_without_repository_is_rejected(self) -> None: + with self.assertRaises(ValueError): + pr_link.format_pr_reference("#66444", {"TERM": "xterm-256color"}) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/skill-evals/README.md b/tools/skill-evals/README.md index 7fd43276..d46219f4 100644 --- a/tools/skill-evals/README.md +++ b/tools/skill-evals/README.md @@ -30,7 +30,7 @@ Suites are currently implemented for: - **pr-management-code-review** — 112 cases across 24 steps (selector-resolution, step-1-selectors-match-chips, step-2.5-slop-detection, step-3-security-disclosure-scan, step-3-ai-authorship-disclosure, step-4-* (12 criteria categories), step-5-adversarial-integration, step-6-disposition, step-7b-review-body-attribution, review-risk-classify, injection-guard, review-disposition, review-handoff) - **pr-management-mentor** — 20 cases across 2 steps (tone-checks, hand-off) - **pr-management-stats** — 13 cases across 2 steps (classify, pressure-weight) -- **pr-management-triage** — 26 cases across 2 steps (pre-filter, decision-table) +- **pr-management-triage** — 33 cases across 3 steps (pre-filter, decision-table, terminal-links) - **list-skills** — 7 cases across 2 steps (step-1-command, step-2-present) - **setup-isolated-setup-verify** — 12 cases across 3 steps (runtime-routing, step-1-classify, step-2-recommend) - **setup-isolated-setup-update** — 14 cases across 4 steps (runtime-routing, step-snapshot-drift, step-tool-freshness, step-after-report) diff --git a/tools/skill-evals/evals/pr-management-triage/README.md b/tools/skill-evals/evals/pr-management-triage/README.md index 1b406848..3c84d81b 100644 --- a/tools/skill-evals/evals/pr-management-triage/README.md +++ b/tools/skill-evals/evals/pr-management-triage/README.md @@ -5,12 +5,13 @@ Behavioral evals for the `pr-management-triage` skill. -## Suites (28 cases total) +## Suites (33 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| | pre-filter | Step 2 (pre-filters) | 10 | F1 (collaborator), F2 (bot), F3 (draft recent), F4 (already ready), F5a (active maintainer comment), F5b (maintainer ping unanswered), F6 (maintainer co-drafted), row-6 (viewer is author), row-7a (fresh PR); clean contributor continues | | decision-table | Step 2 (decision table) | 18 | Rows 3/4 (already-triaged via body-fold block→skip), 7b (security signal), 9 (conflict→draft), 10 (all systemic→rerun), 11 (partial systemic→rerun), 12 (static-only→comment), 13 (flaky ≤2→rerun), 14a (author confirmed→mark-ready), 14b (pending confirmation→skip), 14c (threads addressed→request-author-confirmation), 15 (threads→ping), 16 (no CI→rebase), 18 (changes-requested+new-commits→ping), 19 (already ready→skip), 20 (passing→mark-ready), 21 (stale draft sweep→close), 22 (rollup anomaly→skip) | +| terminal-links | Golden rule 10 | 5 | Short, context-short, and full PR references use the canonical target; `NO_COLOR` and `TERM=dumb` select the plain-text fallback | The two body-fold cases (`case-17-fold-already-triaged`, `case-18-fold-stale-after-push`) are the regression guard for the denoise diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/expected.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/expected.json new file mode 100644 index 00000000..03718b7e --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/expected.json @@ -0,0 +1,5 @@ +{ + "display": "example/widget#42", + "target": "https://github.com/example/widget/pull/42", + "mode": "osc8" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/report.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/report.md new file mode 100644 index 00000000..472b73f7 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-1-short-reference/report.md @@ -0,0 +1,9 @@ + + +PR reference: `example/widget#42` + +Environment: + +- `TERM=xterm-256color` +- `NO_COLOR` is unset diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/expected.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/expected.json new file mode 100644 index 00000000..e4a7598c --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/expected.json @@ -0,0 +1,5 @@ +{ + "display": "https://github.com/example/widget/pull/42", + "target": "https://github.com/example/widget/pull/42", + "mode": "osc8" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/report.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/report.md new file mode 100644 index 00000000..ba8ee374 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-2-full-url/report.md @@ -0,0 +1,9 @@ + + +PR reference: `https://github.com/example/widget/pull/42` + +Environment: + +- `TERM=screen-256color` +- `NO_COLOR` is unset diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/expected.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/expected.json new file mode 100644 index 00000000..d490422f --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/expected.json @@ -0,0 +1,5 @@ +{ + "display": "example/widget#42", + "target": "https://github.com/example/widget/pull/42", + "mode": "plain" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/report.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/report.md new file mode 100644 index 00000000..3ae47988 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-3-no-color/report.md @@ -0,0 +1,9 @@ + + +PR reference: `example/widget#42` + +Environment: + +- `TERM=xterm-256color` +- `NO_COLOR` is present and empty diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/expected.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/expected.json new file mode 100644 index 00000000..d490422f --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/expected.json @@ -0,0 +1,5 @@ +{ + "display": "example/widget#42", + "target": "https://github.com/example/widget/pull/42", + "mode": "plain" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/report.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/report.md new file mode 100644 index 00000000..84b944dc --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-4-dumb-terminal/report.md @@ -0,0 +1,9 @@ + + +PR reference: `example/widget#42` + +Environment: + +- `TERM=dumb` +- `NO_COLOR` is unset diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/expected.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/expected.json new file mode 100644 index 00000000..3ab06111 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/expected.json @@ -0,0 +1,5 @@ +{ + "display": "#42", + "target": "https://github.com/example/widget/pull/42", + "mode": "osc8" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/report.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/report.md new file mode 100644 index 00000000..4690ead3 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/case-5-context-short-reference/report.md @@ -0,0 +1,11 @@ + + +PR reference: `#42` + +Repository context: `example/widget` + +Environment: + +- `TERM=xterm-256color` +- `NO_COLOR` is unset diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/output-spec.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/output-spec.md new file mode 100644 index 00000000..9a5f3ef1 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/output-spec.md @@ -0,0 +1,18 @@ + + +## Output format + +Return ONLY valid JSON with this structure: + +```json +{ + "display": "", + "target": "", + "mode": "osc8" | "plain" +} +``` + +Use `osc8` only when the supplied environment supports terminal hyperlinks. +Use `plain` when the environment requires the fallback. Do not include any +text outside the JSON object. diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/step-config.json b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/step-config.json new file mode 100644 index 00000000..9fbe2510 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/step-config.json @@ -0,0 +1,4 @@ +{ + "skill_md": "skills/pr-management-triage/SKILL.md", + "step_heading": "### Terminal PR-reference renderer" +} diff --git a/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/user-prompt-template.md b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/user-prompt-template.md new file mode 100644 index 00000000..79f73aff --- /dev/null +++ b/tools/skill-evals/evals/pr-management-triage/terminal-links/fixtures/user-prompt-template.md @@ -0,0 +1,8 @@ + + +## Terminal rendering request + +{report} + +Resolve the visible text, canonical target, and rendering mode. Return JSON only.