Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@
## Summary
<!-- What does this change and why? -->

Closes #<!-- issue number -->
## Issue references
Refs #<!-- issue number; use non-closing references for partial work -->
<!-- Only for intentional completion: replace the reference with an automatic-closing
keyword and the issue reference after checking every acceptance condition.
Intentional completion example: Closes <issue-reference>.
Do not negate, qualify or quote a closing phrase to keep an issue open: GitHub
may still close it. Use Refs instead. The reference warning is only a heuristic;
it does not prove how GitHub will interpret a description or change issue state. -->
<!-- Manual sidebar links and closing keywords in commit messages are outside this
warning's scope. Automatic issue closure applies when merged into the default branch. -->

## Type
- [ ] feat
Expand All @@ -12,15 +21,31 @@ Closes #<!-- issue number -->
- [ ] chore / refactor / test / perf

## Checklist
- [ ] `python3 cc_peer.py list` works locally
- [ ] `python3 cc_peer.py list --host <a real host>` works over SSH
- [ ] `--dry-run` resolves a target without posting anything
- [ ] `python session_peer.py --version` / `--help` checked where relevant
- [ ] Generated standalone checked: `python tools/generate_session_peer.py --check` (or not run, with reason below)
- [ ] Commits follow Conventional Commits
- [ ] Docs updated if behavior changed (README / SKILL.md / CLAUDE.md)
- [ ] Relevant [user documentation](https://github.com/abruption/session-peer/blob/main/docs/README.md), [CLI reference](https://github.com/abruption/session-peer/blob/main/docs/cli-reference.md) and translations updated, or marked not applicable
- [ ] No secrets, tokens, or session keys in the diff

## Validation
<!-- Select checks appropriate to this change; do not imply that unrun checks pass.
Report command, result, skips and environment limits. Use "not run: reason" or
"not applicable: reason". Real-host SSH, native agent messages, model calls and
production mutations are not required for every PR and need their own scope. -->

| Area | Relevant check / evidence |
| --- | --- |
| Core | `python -m unittest discover -s tests/core -t . -v` |
| Adapters / Codex | `python -m unittest discover -s tests/adapters -t . -v`; `python -m unittest discover -s tests/codex -t . -v` |
| Distribution / docs | `python -m unittest discover -s tests/distribution -t . -v`; `python -m unittest discover -s tests/docs -t . -v` |
| Optional MCP / Relay | Install the relevant extra, then `python -m unittest discover -s tests/mcp_integration -t . -v` / `python -m unittest discover -s tests/relay -t . -v` |
| Optional Control | In `control/`: `npm ci`, `npm run typecheck`, `npm test`, `npm audit --omit=dev` |
| Control integration / built packages | Follow [test-suite prerequisites and artifact checks](https://github.com/abruption/session-peer/blob/main/tests/README.md) when relevant |

## Verified against
<!-- Claude Code version and OS you tested on, e.g. "v2.1.263, macOS 26 + Ubuntu 24.04 arm64" -->
<!-- Record exact Python and, when relevant, Node/npm and Claude Code/Codex/
Antigravity versions; OS/architecture; extras/dependency profile; fixture versus
live acceptance. Use not run/not applicable rather than inventing evidence. -->

## Notes
<!-- Tradeoffs, follow-ups, manual test steps reviewers should know about. -->
99 changes: 99 additions & 0 deletions .github/scripts/check_pr_references.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
#!/usr/bin/env python3
"""Warn about a small class of ambiguous closing phrases, without changing issues.

This is a review aid, not GitHub's closing-reference parser. It does not decide
whether work is complete. It scans text (including quoted/code text), using a
short preceding-clause heuristic; false positives and missed cases are possible.
Manual sidebar links and commit-message references are outside this check;
merging into the default branch can close issues independently of this warning.
Only fixed messages are emitted: PR body content is never evaluated or reflected
into workflow commands or Markdown output.
"""

import json
import os
from pathlib import Path
import re


MAX_EVENT_BYTES = 2 * 1024 * 1024
CLOSING = re.compile(
r"\b(?:close[sd]?|fix(?:es|ed)?|resolve[sd]?)\b(?:\s*:\s*|\s+)"
r"(?:(?:[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#[1-9][0-9]*\b|"
r"https://github\.com/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+/issues/[1-9][0-9]*\b)",
re.IGNORECASE,
)
NEGATION = re.compile(
r"\b(?:not|never|without|no|cannot)\b|\b(?:ca|do|does|did|is|are|was|were|would|should|must|could)n['’]t\b|\bwon['’]t\b",
re.IGNORECASE,
)
PARTIAL = re.compile(r"\b(?:partial(?:ly)?|incomplete)\b", re.IGNORECASE)
MESSAGES = {
"negated": "A closing reference has nearby negation. GitHub may still close the issue; use a non-closing Refs reference for unfinished work.",
"partial": "A closing reference is described as partial or incomplete. Use Refs unless completing the issue is intentional.",
"unavailable": "PR reference analysis was unavailable. Review issue references manually; this warning does not determine GitHub closing behavior.",
}


def warnings_for_body(body):
"""Return fixed warning keys; ordinary references/intentional closure pass."""
found = set()
previous_end = 0
for match in CLOSING.finditer(body):
# Reset after earlier references and sentence/paragraph boundaries.
# A single newline remains eligible for wrapped Markdown prose.
prefix = body[max(previous_end, match.start() - 160):match.start()]
prefix = re.split(r"[.!?;]|\n\s*\n", prefix)[-1]
if NEGATION.search(prefix):
found.add("negated")
if PARTIAL.search(prefix):
found.add("partial")
previous_end = match.end()
return tuple(key for key in ("negated", "partial") if key in found)


def event_warnings(path):
try:
with Path(path).open("rb") as stream:
raw = stream.read(MAX_EVENT_BYTES + 1)
if len(raw) > MAX_EVENT_BYTES:
return ("unavailable",)
event = json.loads(raw)
pull_request = event.get("pull_request") if isinstance(event, dict) else None
if not isinstance(pull_request, dict):
return ("unavailable",)
body = pull_request.get("body")
if body is None:
body = ""
if not isinstance(body, str):
return ("unavailable",)
return warnings_for_body(body)
except (OSError, ValueError, TypeError):
return ("unavailable",)


def main():
warnings = event_warnings(os.environ.get("GITHUB_EVENT_PATH", ""))
for key in warnings:
print("::warning::" + MESSAGES[key])
if not warnings:
print("No ambiguous closing phrase detected by this limited heuristic.")
summary = os.environ.get("GITHUB_STEP_SUMMARY")
if summary:
try:
with Path(summary).open("a", encoding="utf-8") as stream:
stream.write("## Issue-reference review aid\n\n")
if warnings:
for key in warnings:
stream.write("- " + MESSAGES[key] + "\n")
else:
stream.write("No ambiguous closing phrase detected. This is not proof of GitHub's complete closing behavior.\n")
stream.write("\nThis check does not edit descriptions, close issues or block intentional completion.\n")
except OSError:
print("::warning::" + MESSAGES["unavailable"])
# The analyzer only warns; contributor/reviewer intent still controls closure.
return 0


if __name__ == "__main__":
raise SystemExit(main())
24 changes: 24 additions & 0 deletions .github/workflows/pr-reference-warning.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: PR reference warning

on:
pull_request_target:
branches: [main]
types: [opened, edited, reopened, synchronize]

permissions:
contents: read

jobs:
references:
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
# Run code from the trusted default-branch workflow commit, never the PR head.
# A newly introduced checker first runs after it is merged into main.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.sha }}
persist-credentials: false
- name: Warn about ambiguous closing references
# The script reads the event JSON as data; no body text enters the shell.
run: python3 .github/scripts/check_pr_references.py
160 changes: 160 additions & 0 deletions tests/docs/test_pr_references.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
"""PR references remain review aids and never execute contributor text."""

import importlib.util
import json
import os
from pathlib import Path
import re
import subprocess
import sys
import tempfile
import unittest


ROOT = Path(__file__).resolve().parents[2]
SCRIPT = ROOT / ".github/scripts/check_pr_references.py"
WORKFLOW = ROOT / ".github/workflows/pr-reference-warning.yml"
spec = importlib.util.spec_from_file_location("check_pr_references", SCRIPT)
checker = importlib.util.module_from_spec(spec)
spec.loader.exec_module(checker)


class PrReferences(unittest.TestCase):
def check(self, body):
return checker.warnings_for_body(body)

def test_nonclosing_references_and_intentional_completion_do_not_warn(self):
for body in ("Refs #12", "Related to #12 and #34", "Closes #12",
"Fixes owner/repository#34", "Resolves #12, #34", "",
"This does not change the protocol. Closes #12"):
with self.subTest(body=body):
self.assertEqual((), self.check(body))

def test_negated_closing_phrases_warn(self):
for body in ("Does not close #12", "This never fixes #12", "This can't close #12",
"Doesn't resolve owner/repository#12", "This won’t close #12",
"This is not intended to close #12", "Does not\nclose #12",
"This does NOT CLOSE: #12", "Does not resolve https://github.com/owner/repository/issues/12"):
with self.subTest(body=body):
self.assertEqual(("negated",), self.check(body))

def test_partial_closing_phrases_warn_without_disallowing_references(self):
self.assertEqual(("partial",), self.check("Partially fixes #12"))
self.assertEqual(("partial",), self.check("Incomplete work resolves #12"))
self.assertEqual((), self.check("Partial work. Refs #12"))

def test_multiple_references_have_separate_preceding_context(self):
self.assertEqual(("negated",), self.check("Does not close #12, but closes #34"))
self.assertEqual(("negated", "partial"), self.check("Does not close #12. Partially fixes #34"))
self.assertEqual((), self.check("Refs #12; Closes #34"))

def test_trailing_qualification_is_a_manual_review_limit(self):
# The bounded preceding-clause heuristic intentionally cannot certify
# intent stated after a closing reference. Review remains necessary.
for body in ("Closes #12, but this work is incomplete", "Fixes #12 (not yet)"):
self.assertEqual((), self.check(body))

def run_event(self, root, event):
path = root / "event.json"
path.write_text(json.dumps(event), encoding="utf-8")
summary = root / "summary.md"
result = subprocess.run(
[sys.executable, str(SCRIPT)],
env=dict(os.environ, GITHUB_EVENT_PATH=str(path), GITHUB_STEP_SUMMARY=str(summary)),
cwd=root, capture_output=True, text=True, timeout=5,
)
self.assertEqual(0, result.returncode, result.stderr)
return result.stdout, summary.read_text(encoding="utf-8")

def test_actual_event_script_handles_body_edits_without_changing_event(self):
with tempfile.TemporaryDirectory(prefix="codex-pr-reference-") as folder:
root = Path(folder)
for action, body, warning in (("opened", "Refs #12", False),
("edited", "Does not close #12", True),
("edited", "Closes #12", False)):
event = {"action": action, "pull_request": {"body": body}}
output, _ = self.run_event(root, event)
self.assertEqual(warning, "::warning::" in output)
self.assertEqual(event, json.loads((root / "event.json").read_text()))

def test_body_is_data_and_never_workflow_command_or_summary_content(self):
with tempfile.TemporaryDirectory(prefix="codex-pr-reference-") as folder:
root = Path(folder)
marker = root / "executed"
body = "Does not close #12\n::error::INJECTED\n$(touch " + str(marker) + ")\n<script>INJECTED</script>"
output, summary = self.run_event(root, {"pull_request": {"body": body}})
self.assertIn("::warning::", output)
self.assertNotIn("INJECTED", output + summary)
self.assertFalse(marker.exists())

def test_missing_invalid_and_oversize_events_only_warn(self):
with tempfile.TemporaryDirectory(prefix="codex-pr-reference-") as folder:
root = Path(folder)
for event in ({}, [], {"pull_request": {"body": 123}}):
output, _ = self.run_event(root, event)
self.assertIn(checker.MESSAGES["unavailable"], output)
path = root / "oversize.json"
path.write_bytes(b" " * (checker.MAX_EVENT_BYTES + 1))
self.assertEqual(("unavailable",), checker.event_warnings(path))
self.assertEqual(("unavailable",), checker.event_warnings(root / "absent"))

def test_long_whitespace_without_a_reference_is_not_a_regex_time_trap(self):
with tempfile.TemporaryDirectory(prefix="codex-pr-reference-") as folder:
output, _ = self.run_event(Path(folder), {
"pull_request": {"body": "Does not close" + " " * 65536 + "not-a-reference"}})
self.assertNotIn("::warning::", output)

def test_workflow_uses_only_trusted_base_code_and_read_only_permissions(self):
text = WORKFLOW.read_text()
self.assertIn("pull_request_target:", text)
self.assertIn("branches: [main]", text)
self.assertIn("types: [opened, edited, reopened, synchronize]", text)
self.assertIn("ref: ${{ github.sha }}", text)
self.assertIn("persist-credentials: false", text)
self.assertEqual(["contents: read"], re.findall(r"^ ([a-z-]+: (?:read|write))$", text, re.M))
self.assertNotRegex(text, r"(?m)^\s+[a-z-]+:\s*write\b")
self.assertNotRegex(text, r"(?m)^\s*permissions:\s*write-all\b")
self.assertNotIn("secrets.", text)
self.assertNotIn("pull_request.head", text)
self.assertNotIn("pull_request.base.sha", text)
self.assertNotIn("pull_request.body", text)
self.assertNotIn("refs/pull", text)
self.assertIn("run: python3 .github/scripts/check_pr_references.py", text)
uses = re.findall(r"uses: ([^\s]+)", text)
self.assertTrue(all(re.fullmatch(r"[\w-]+/[\w-]+@[0-9a-f]{40}", ref) for ref in uses))

def test_template_defaults_to_nonclosing_references_and_current_checks(self):
text = (ROOT / ".github/pull_request_template.md").read_text()
self.assertIn("Refs #<!-- issue number", text)
self.assertNotRegex(text, r"(?im)^\s*(?:closes|fixes|resolves)\s+#")
self.assertNotIn("cc_peer.py", text)
self.assertNotIn("CLAUDE.md", text)
for item in ("python session_peer.py --version", "python tools/generate_session_peer.py --check",
"tests/core", "tests/adapters", "tests/codex", "tests/distribution", "tests/docs",
"tests/mcp_integration", "tests/relay", "npm audit --omit=dev",
"not run: reason", "not applicable: reason", "Node/npm", "Codex", "Antigravity"):
self.assertIn(item, text)
self.assertIn("Do not negate", text)
for limitation in ("sidebar", "commit messages", "default branch"):
self.assertIn(limitation, text)
self.assertIn("Closes <issue-reference>", text)
self.assertIn("not required for every PR", text)
for link in re.findall(r"\]\(https://github.com/abruption/session-peer/blob/main/([^)]*)\)", text):
self.assertTrue((ROOT / link).is_file(), link)

def test_summary_failure_still_warns_without_reflecting_data(self):
with tempfile.TemporaryDirectory(prefix="codex-pr-reference-") as folder:
root = Path(folder)
event = root / "event.json"
event.write_text(json.dumps({"pull_request": {"body": "Closes #12"}}))
result = subprocess.run(
[sys.executable, str(SCRIPT)],
env=dict(os.environ, GITHUB_EVENT_PATH=str(event), GITHUB_STEP_SUMMARY=str(root)),
capture_output=True, text=True)
self.assertEqual(0, result.returncode)
self.assertIn(checker.MESSAGES["unavailable"], result.stdout)
self.assertEqual("", result.stderr)


if __name__ == "__main__":
unittest.main()
Loading