Skip to content
162 changes: 162 additions & 0 deletions .github/scripts/lint_pr_body_refs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
"""Issue #114 Phase 1 — PR-body refs lint.

Walks a PR body looking for `#NUMBER` tokens. For each token, classifies as:
- structured (under a `## Linked issues` section OR preceded by
a recognised keyword: Closes/Fixes/Resolves/Refs/Related/See)
- bare (warning emitted)

Always returns exit 0 — advisory check, never blocks merge. The CI
workflow surfaces warnings via stderr; reviewers can act on them
manually.

SECURITY-CRITICAL: this script is the receiving end of the CI
workflow's ``--from-env PR_BODY`` invocation. The PR body is
contributor-editable text that flows directly from
``${{ github.event.pull_request.body }}`` through an environment
variable into ``os.environ[NAME]`` — no Bash interpreter in the
path. An earlier draft of the workflow used ``echo "$PR_BODY" >
/tmp/file`` which exposed OWASP A03 command-substitution injection
(`$(cmd)` expanded inside Bash double quotes). The ``--from-env``
flag is the safe alternative; never restore the echo pattern.

Stdlib only — no external deps.
"""

from __future__ import annotations

import argparse
import dataclasses
import os
import re
import sys
from collections.abc import Iterable

_NUMBER_TOKEN_RE = re.compile(r"#(\d+)")
_KEYWORDS = (
"closes",
"closed",
"fixes",
"fixed",
"resolves",
"resolved",
"refs",
"related to",
"related",
"see",
)
_LINKED_ISSUES_HEADING_RE = re.compile(
r"^\s*#{1,6}\s+linked\s+issues?\s*$",
re.IGNORECASE,
)


@dataclasses.dataclass(frozen=True)
class Warning:
"""One bare-mention warning."""

line: int # 1-indexed
number: int # the issue number


# ── Public entry (≤ 30 LOC) ──────────────────────────────────────────


def lint_pr_body(body: str) -> list[Warning]:
"""Walk a PR body's lines, classify each ``#NUMBER`` token, return
warnings for bare mentions. Pure function — no IO."""
warnings: list[Warning] = []
in_linked_section = False
for lineno, line in enumerate(body.splitlines(), start=1):
if _LINKED_ISSUES_HEADING_RE.match(line):
in_linked_section = True
continue
if in_linked_section and _is_other_heading(line):
in_linked_section = False
if in_linked_section:
continue
for match in _NUMBER_TOKEN_RE.finditer(line):
number = int(match.group(1))
if not _has_preceding_keyword(line, match.start()):
warnings.append(Warning(line=lineno, number=number))
return warnings


# ── Helpers (each ≤ 20 LOC) ──────────────────────────────────────────


def _is_other_heading(line: str) -> bool:
"""Markdown heading that closes the linked-issues section."""
return bool(re.match(r"^\s*#{1,6}\s+", line))


def _has_preceding_keyword(line: str, token_start: int) -> bool:
"""Return True when the `#NUMBER` token at ``token_start`` is
preceded on the same line by one of the recognised
issue-link keywords (case-insensitive). Looks back up to 32
characters for a keyword match."""
prefix = line[:token_start].lower()
return any(prefix.rstrip().endswith(kw) for kw in _KEYWORDS)


def _emit_warnings(warnings: Iterable[Warning], out=None) -> None:
"""Print each warning to stderr in actionable form. ``out`` is
resolved at call-time (default ``sys.stderr``) so test harnesses
that replace ``sys.stderr`` (pytest capsys) capture the output
correctly — capturing the default at function-def time would
bind a stale reference."""
target = out if out is not None else sys.stderr
for w in warnings:
print(
f"warning: bare '#{w.number}' on line {w.line} — wrap with "
f"'Closes #{w.number}' / 'Refs #{w.number}', or move to a "
"'Linked issues' section",
file=target,
)


# ── CLI entry (≤ 25 LOC) ─────────────────────────────────────────────


def main(argv: list[str] | None = None) -> int:
"""CLI entry. Body source — exactly one of:
--body <file> — read PR body from file (local dev / tests)
--from-env <NAME> — read PR body from env var (CI; security-critical
to avoid Bash shell interpolation of contributor-
controlled text — see module docstring)

Always returns 0 (advisory check)."""
args = _parse_args(argv)
body = _read_body(args)
if body is None:
return 0
_emit_warnings(lint_pr_body(body))
return 0


def _parse_args(argv: list[str] | None) -> argparse.Namespace:
parser = argparse.ArgumentParser(prog="lint_pr_body_refs")
group = parser.add_mutually_exclusive_group(required=True)
group.add_argument("--body", help="path to PR body file")
group.add_argument(
"--from-env",
metavar="NAME",
help="environment variable to read PR body from (CI safe; no shell)",
)
return parser.parse_args(argv)


def _read_body(args: argparse.Namespace) -> str | None:
"""Read the PR body from whichever source the args specified.
Returns None when the source is missing — script exits 0 silently
rather than failing loudly (advisory)."""
if args.body:
try:
with open(args.body, encoding="utf-8") as fh:
return fh.read()
except OSError:
return None
return os.environ.get(args.from_env)


if __name__ == "__main__":
sys.exit(main(sys.argv[1:]))
13 changes: 13 additions & 0 deletions .github/workflows/lint-and-typecheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,16 @@ jobs:
run: ruff format --check .
- name: Mypy
run: mypy .
- name: Plan-grounding lint (#114 Check A)
# Only lints plan-*.md files modified in this PR — historical
# plans that referenced now-deleted files would otherwise block
# CI for unrelated work. Adding/modifying a plan still gates.
run: |
git fetch origin ${{ github.base_ref }} --depth=1
PLANS=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD" -- 'plan-*.md' 'docs/Planning/plan-*.md' 2>/dev/null || true)
if [ -n "$PLANS" ]; then
echo "Linting modified plans: $PLANS"
python scripts/lint_plan_grounding.py $PLANS
else
echo "No plan files modified in this PR — skipping"
fi
40 changes: 40 additions & 0 deletions .github/workflows/pr-body-refs-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: PR body refs lint

# Issue #114 Check B — surfaces bare `#NUMBER` mentions in PR bodies that
# aren't wrapped by Closes/Fixes/Resolves/Refs keywords or under a
# `## Linked issues` section. Advisory only — never blocks merge.
#
# SECURITY: PR_BODY is contributor-controlled text. The script reads it
# via `--from-env PR_BODY` (direct os.environ read in Python). DO NOT
# replace with `echo "$PR_BODY" > file` — that pattern lets Bash
# command-substitution `$(cmd)` expand inside double quotes, which is
# OWASP A03 arbitrary code execution in CI. See module docstring for
# the full audit context (#114 audit v1 caught this).
#
# Note: when this workflow file lands, it does not run on the PR that
# adds it — pull_request workflows execute the version on the base
# branch. First execution is on the next qualifying PR after merge.

on:
pull_request:
types: [opened, edited, reopened]

permissions:
pull-requests: read
contents: read

jobs:
lint:
name: PR body refs lint (advisory)
runs-on: ubuntu-latest
# Advisory: red here doesn't gate merge.
continue-on-error: true
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Run lint
env:
PR_BODY: ${{ github.event.pull_request.body }}
run: python .github/scripts/lint_pr_body_refs.py --from-env PR_BODY
21 changes: 19 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,25 @@ All notable changes to bicameral-mcp are tracked here. Format loosely follows

### Added

- **CI grounding lint for plan files and PR bodies (#114).** Two new
checkers ship together:
- `scripts/lint_plan_grounding.py` — walks `plan-*.md` files for
backtick-wrapped path tokens and verifies each resolves on the
working tree. Marks `**new**` / `(planned)` / `(future)` /
`(v2)` / `(nonexistent)` / `(example)` as exempt. Folded into
the existing `lint-and-typecheck.yml` workflow as a blocking
gate; only runs against plans modified in the current PR.
Closes the SG-PLAN-GROUNDING-DRIFT loop after three instances
in the v0.13/v0.16 development window.
- `.github/scripts/lint_pr_body_refs.py` + new
`.github/workflows/pr-body-refs-lint.yml` advisory workflow —
warns on bare `#NUMBER` mentions in PR bodies that aren't
wrapped by `Closes`/`Fixes`/`Resolves`/`Refs`/`Related`/`See`
keywords or under a `## Linked issues` heading. Reads PR body
via `--from-env PR_BODY` (direct `os.environ` read, no shell
interpolation; OWASP A03 mitigation caught at audit v1).
- Documentation: `DEV_CYCLE.md` §2.1 (plan-grounding lint
callout) + §4.3 (PR-body keyword discipline). Issue #114.
- `handlers/preflight.py` — `_region_anchored_preflight` now expands caller-supplied `file_paths` by 1 hop along the code-locator graph's **import edges** before the `binds_to` lookup. Lifts the strict exact-match recall ceiling so a decision bound to `app/src/lib/git/reorder.ts` surfaces when the caller passes the structurally-near `app/src/ui/multi-commit-operation/reorder.tsx`. Decisions reached only via expansion carry `confidence=0.7` (vs `0.9` for direct pins). `sources_chained` includes `"graph"` (alongside `"region"`) when expansion contributed at least one hit. Bounded per #64: ≤10 input seeds × `max_neighbors_per_result` neighbors per seed. Closes #173 (and supersedes #64).
- `adapters/code_locator.py::RealCodeLocatorAdapter.expand_file_paths_via_graph` — public method backing the expansion. Filters to ``imports`` edges only (file-level structural dependency); ``invokes`` / ``inherits`` / ``contains`` are symbol-level edges that over-broaden the file-level expansion. Returns `(expanded, added)` so callers can mark provenance.
- `skills/bicameral-preflight/SKILL.md` Step 2 — documents the imports-only expansion + caller-side `confidence` and `sources_chained` semantics.
Expand All @@ -18,8 +37,6 @@ All notable changes to bicameral-mcp are tracked here. Format loosely follows
- `skills/bicameral-preflight/SKILL.md` Step 5.6 — judgment for contradiction-capture moves from the agent to the user via `AskUserQuestion` (Step 5.6.1). The agent no longer infers whether the prompt contradicts a surfaced decision; it asks the user (`supersede` / `keep_both` / `unrelated`) and acts mechanically on the answer (Step 5.6.2 — ingest + resolve_collision). The PostToolUse hook reminder now templates the disambiguation question rather than the bare ingest+resolve_collision sequence. Closes #175.
- `tests/e2e/run_e2e_flows.py::assert_flow_2a` — pass criterion changed from "ingest+resolve_collision fired" to "`AskUserQuestion` invoked with disambiguation shape after preflight surfaced ≥1 decision." The user-side response can't be driven in headless `claude -p`, so the testable signal is the question invocation. The mechanical capture (Step 5.6.2) only fires after a human answers and is exercised in interactive Claude Code sessions, not CI.

### Changed

### Fixed

### Schema
Expand Down
24 changes: 24 additions & 0 deletions docs/DEV_CYCLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,14 @@ every box in that diagram. No back-doors to `main`.
> see §4.4. Risk is a property of the change being made, knowable only after
> design. Issues carry priority (urgency); PRs carry risk (review tier).

> **CI grounding lint (Issue #114)**: every plan committed to this repo
> runs through `scripts/lint_plan_grounding.py` in the lint workflow.
> The lint rejects plan files that reference filesystem paths not
> present on the working tree, unless the path is marked **new** or
> followed by `(planned)` / `(future)` / `(v2)` / `(nonexistent)` /
> `(example)` on its bullet. Author-time `ls -d */` is faster
> feedback; CI is the durable gate. Mitigation for SG-PLAN-GROUNDING-DRIFT.

#### 2.1.1 Priority labels (one per issue, mandatory after triage)

Exactly one priority label per triaged issue. Untriaged issues carry `triage`
Expand Down Expand Up @@ -406,6 +414,22 @@ Refs #60 (depends on continuity matcher landed there)
The Plan/Audit/Seal section is **mandatory for any PR > 100 LOC or risk:L2+**.
Smaller PRs may use `Plan: trivial; risk:L1`.

> **PR-body issue references (Issue #114)**: every PR body that
> mentions `#NUMBER` tokens should wrap them with one of:
>
> - **`Closes #N`** / `Fixes #N` / `Resolves #N` — full closure, fires
> GitHub auto-close + the `merged-to-dev` labeller workflow.
> - **`Refs #N`** / `Related to #N` / `See #N` — partial / related;
> creates the cross-link without auto-closing.
> - **Place under a `## Linked issues` heading** — section-level
> wrapping, equivalent to per-line keyword.
>
> The advisory workflow `pr-body-refs-lint.yml` warns (does not block)
> when bare `#NUMBER` mentions appear in prose without a wrapping.
> Bare mentions still create GitHub's auto-cross-reference, but skip
> the auto-close + labeller paths — surface them so authors choose
> intentionally rather than implicitly.

### 4.4 Reviewers

- Code-owner from `CODEOWNERS` is auto-requested.
Expand Down
Loading
Loading