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
10 changes: 8 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ bot:
escalate_label: needs-human # must already exist in your repo's Labels
max_replies: 2 # 0..100; cap on followup replies before next user comment ESCALATEs
max_runs_per_hour: 20 # 0..100; per-(repo, item) rate limit; 0 disables
allowed_labels: # labels issue-triage may apply; defaults to bug/enhancement/question/documentation/help-wanted. Add repo-specific labels here (they must exist in your repo's Labels).
- bug
- enhancement
- question
- documentation

# Override per-stage models if you want a different cost/quality trade-off.
# Env vars BEDROCK_MODEL_ID / BEDROCK_REPORTER_MODEL_ID / BEDROCK_CRITIC_MODEL_ID /
Expand All @@ -213,7 +218,8 @@ models:
| `BOT_REPORTER_MIN_REMAINING_S` | `60` | Reporter is pre-empted if less than this remains. |
| `BOT_MAX_DIFF_FOR_REVIEW_CHARS` | `100000` | Pre-flight diff cap. Diff above this → ESCALATE before any Bedrock call. |
| `BOT_MAX_FILES_FOR_REVIEW` | `50` | Pre-flight file-count cap. Same shape as above. |
| `BOT_MAX_RUNS_PER_HOUR` | `20` | Per-(repo, item) hourly run cap. `0` disables. Capped at `100`; values above clamp with a warning. Negative values fall back to default. Hit → ESCALATE with `shadow:rate-limited` label. |
| `BOT_MAX_RUNS_PER_HOUR` | `20` | Per-(repo, item) hourly run cap. `0` disables. Capped at `100`; values above clamp with a warning. Negative values fall back to default. Hit → ESCALATE with a `<bot.name>:rate-limited` label. |
| `BOT_ALLOWED_LABELS` | `bug,enhancement,question,documentation,help-wanted` | Comma-separated allowlist of labels issue-triage may apply (also settable as a list under `.shadow.yml` `bot.allowed_labels`). Model-proposed labels outside the set are dropped. Empty/garbage falls back to the default. |
| `BOT_MAX_REPLIES` | `2` | Followup-reply cap per (issue, PR). Capped at `100`; negative falls back to default. |
| `BOT_GITHUB_ACTOR` | `github-actions[bot]` | GitHub login Shadow's comments appear under. **Set this to a unique value** if your repo has other workflows that also post as `github-actions[bot]` (e.g., PR-overlap detectors, claim-checkers). Otherwise Shadow's `already_commented` dedup matches their comments and silently SKIPs every PR. |
| `BOT_REQUIRE_GUARDRAIL` | `true` | Production runs (`DRY_RUN=false`) refuse to start when `GUARDRAIL_ID` is unset — Shadow won't run without prompt-injection defense. On the reusable workflow, drive this via the `require_guardrail` **input** (`with: require_guardrail: 'false'`) — accepts `0`/`false`/`no`/`off`. `DRY_RUN=true` bypasses the gate regardless. |
Expand Down Expand Up @@ -411,7 +417,7 @@ Every analyze run writes a `shadow_result.json` artifact, retained 7 days by Git

The per-PR levers (under [What it costs](#what-it-costs)) bound a single review. These three guards bound **fleet-wide** spend, defending against PR/issue spam and runaway traffic:

- **Per-(repo, item) hourly rate limit** (`BOT_MAX_RUNS_PER_HOUR`, default `20`). Caps how many times a single PR or issue can trigger Shadow per rolling hour. Beyond the limit, the bot ESCALATES with the `shadow:rate-limited` label instead of running the agent pipeline. Defends against an adversary closing/reopening or editing a PR title in a loop. Set to `0` to disable. **Issue/issue_comment events** require `run-name: "Shadow #${{ github.event.issue.number || ... }}"` in your caller workflow so the rate-limit gate can match prior runs (see [`examples/caller-workflow.yml`](examples/caller-workflow.yml)).
- **Per-(repo, item) hourly rate limit** (`BOT_MAX_RUNS_PER_HOUR`, default `20`). Caps how many times a single PR or issue can trigger Shadow per rolling hour. Beyond the limit, the bot ESCALATES with a `<bot.name>:rate-limited` label instead of running the agent pipeline. Defends against an adversary closing/reopening or editing a PR title in a loop. Set to `0` to disable. **Issue/issue_comment events** require `run-name: "Shadow #${{ github.event.issue.number || ... }}"` in your caller workflow so the rate-limit gate can match prior runs (see [`examples/caller-workflow.yml`](examples/caller-workflow.yml)).
- **Pre-flight diff/file caps** (`BOT_MAX_DIFF_FOR_REVIEW_CHARS`, `BOT_MAX_FILES_FOR_REVIEW`, defaults `100000` / `50`). A 50-file PR makes the Investigator read 5+ files, the Critic re-reads, the Reporter formats — costs multiply. Diff or file count above the cap → ESCALATE before any Bedrock call. Pre-flight escalation is ~$0; a runaway pipeline on a giant PR is $5+.
- **AWS Budgets opt-in via CFN** (`MonthlyBudgetLimit` parameter on `shadow-iam-stack.yaml`). Set a positive USD amount + a `BudgetEmailAddress` and the stack creates an `AWS::Budgets::Budget` filtered to Amazon Bedrock spend, with email alerts at 80% and 100%. `0` skips Budget creation (default — AWS Budgets bills $0.02/budget/day, so opt-in only). Email-only today; auto-shutdown via `SHADOW_DISABLED` is a planned upgrade.

Expand Down
36 changes: 33 additions & 3 deletions src/scripts/shadow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,12 @@ def __init__(self):
self.max_context_chars = 800000
self.max_github_search_results = 8
self.github_api_timeout = 10
self.allowed_labels = {
"bug", "enhancement", "question", "documentation", "help-wanted",
}
# Extensible so adopters can allow repo-specific labels (e.g. python, java).
self.allowed_labels = _parse_allowed_labels(
os.getenv("BOT_ALLOWED_LABELS"),
shadow_config.get(yml, "bot", "allowed_labels"),
_DEFAULT_ALLOWED_LABELS,
)

# Default ON. BOT_AGENT_PIPELINE=0 makes PR events ESCALATE with
# reason `pipeline_disabled`; issue/followup paths still run their
Expand Down Expand Up @@ -326,6 +329,33 @@ def _scrub_label(val, default):
return val[:50]


_DEFAULT_ALLOWED_LABELS = frozenset({
"bug", "enhancement", "question", "documentation", "help-wanted",
})


def _parse_allowed_labels(env_val, yaml_val, default):
"""env (comma-separated) > yaml (list) > default. Entries are scrubbed like
posted labels; an empty/all-garbage result falls back to `default` rather
than silently disabling labeling."""
items = None
if env_val is not None and env_val.strip():
items = env_val.split(",")
elif isinstance(yaml_val, (list, tuple)):
items = yaml_val
elif isinstance(yaml_val, str) and yaml_val.strip():
# tolerate a scalar yaml string ("python,java") like the env form
items = yaml_val.split(",")
if items is None:
return set(default)
_sentinel = object()
scrubbed = {
lbl for lbl in (_scrub_label(x, _sentinel) for x in items)
if lbl is not _sentinel
}
return scrubbed or set(default)


def _int_in_range_or_default(val, default, *, name):
"""Yaml int, env str, or wrong type → default. Cap range [0, 100] so a
yaml typo can't set the bound absurdly high. `0` is valid and means
Expand Down
9 changes: 8 additions & 1 deletion src/scripts/shadow/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,10 @@ def analyze():
if recent is not None and recent >= cfg.max_runs_per_hour:
logger.warning("Rate limit hit on #%s: %d runs in last hour (cap %d)",
number, recent, cfg.max_runs_per_hour)
# act() adds the branded rate-limit label (keyed on this reason).
_write_artifact({
"action": "ESCALATE",
"labels": [cfg.escalate_label, "shadow:rate-limited"],
"labels": [cfg.escalate_label],
"response": "",
"reason": "rate_limited",
"title": title, "html_url": html_url, "number": number, "is_pr": is_pr,
Expand Down Expand Up @@ -584,6 +585,12 @@ def act():
escalate_labels = list(labels)
if cfg.escalate_label and cfg.escalate_label not in escalate_labels:
escalate_labels.append(cfg.escalate_label)
# Appended after the filter (like escalate_label): engine-generated, so
# not subject to the model-proposed-label allowlist. Branded with bot_name.
if reason == "rate_limited":
rate_label = f"{cfg.bot_name}:rate-limited"
if rate_label not in escalate_labels:
escalate_labels.append(rate_label)
posted_status = {
"comment": bool(gh.post_comment(number, ack)),
"labels": bool(gh.add_labels(number, escalate_labels)),
Expand Down
179 changes: 179 additions & 0 deletions tests/unit/test_allowed_labels.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
"""cfg.allowed_labels configurability + rate-limit label branding.

The issue-triage label allowlist was hardcoded, so adopters could not apply
repo-specific labels (e.g. python-deequ's `python`, dqdl's `java`) — the
engine dropped them at main.py before the Labels API call. Make it read from
.shadow.yml bot.allowed_labels (list) or BOT_ALLOWED_LABELS env (comma-sep),
defaulting to the original set so existing deployments are unchanged.

Also verifies the rate-limit escalation label tracks bot_name rather than a
hardcoded `shadow:` literal.
"""
import importlib
import tempfile
from pathlib import Path

import pytest


REQUIRED_ENV = {
"GITHUB_TOKEN": "fake-token",
"EVENT_TYPE": "pull_request_target",
"ISSUE_NUMBER": "42",
"GITHUB_REPOSITORY": "owner/repo",
"BOT_REQUIRE_GUARDRAIL": "false",
}

_DEFAULT = {"bug", "enhancement", "question", "documentation", "help-wanted"}


def _set_required_env(monkeypatch):
for k, v in REQUIRED_ENV.items():
monkeypatch.setenv(k, v)
monkeypatch.delenv("BOT_ALLOWED_LABELS", raising=False)


@pytest.fixture(autouse=True)
def _reload_config():
"""Config reads env at import-time; force fresh load per test."""
import shadow.config # noqa: F401
importlib.reload(__import__("shadow.config", fromlist=["Config"]))
yield


def _cfg(monkeypatch, tmp, yml=None):
if yml is not None:
Path(tmp, ".shadow.yml").write_text(yml)
monkeypatch.setenv("SHADOW_REPO_ROOT", tmp)
from shadow.config import Config
return Config()


def test_default_allowed_labels(monkeypatch):
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp)
assert cfg.allowed_labels == _DEFAULT


def test_yaml_list_extends_labels(monkeypatch):
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp,
"bot:\n allowed_labels:\n - bug\n - python\n - java\n")
assert cfg.allowed_labels == {"bug", "python", "java"}


def test_env_overrides_yaml(monkeypatch):
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
Path(tmp, ".shadow.yml").write_text(
"bot:\n allowed_labels:\n - bug\n"
)
monkeypatch.setenv("SHADOW_REPO_ROOT", tmp)
monkeypatch.setenv("BOT_ALLOWED_LABELS", "enhancement,question")
from shadow.config import Config
cfg = Config()
assert cfg.allowed_labels == {"enhancement", "question"}


def test_env_comma_separated(monkeypatch):
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
monkeypatch.setenv("BOT_ALLOWED_LABELS", " bug , python ,java")
monkeypatch.setenv("SHADOW_REPO_ROOT", tmp)
from shadow.config import Config
cfg = Config()
# entries are whitespace-stripped by the label scrub
assert cfg.allowed_labels == {"bug", "python", "java"}


def test_empty_yaml_list_falls_back_to_default(monkeypatch):
"""An empty list must not disable labeling entirely."""
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp, "bot:\n allowed_labels: []\n")
assert cfg.allowed_labels == _DEFAULT


def test_all_garbage_entries_fall_back(monkeypatch):
"""A list of only non-string / blank entries scrubs to empty -> default."""
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp,
"bot:\n allowed_labels:\n - 123\n - \" \"\n - null\n")
assert cfg.allowed_labels == _DEFAULT


def test_garbage_scalar_yaml_falls_back(monkeypatch):
"""A non-list, non-string scalar (int) drops to default rather than crash."""
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp, "bot:\n allowed_labels: 5\n")
assert cfg.allowed_labels == _DEFAULT


def test_mixed_valid_and_garbage_keeps_valid(monkeypatch):
"""Valid entries survive; garbage ones drop (partial list is not all-or-nothing)."""
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp,
"bot:\n allowed_labels:\n - python\n - 123\n - \"\"\n")
assert cfg.allowed_labels == {"python"}


def test_scalar_yaml_string_is_split(monkeypatch):
"""A scalar yaml string 'a,b' is treated like the comma-sep env form."""
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
cfg = _cfg(monkeypatch, tmp, "bot:\n allowed_labels: \"python,java\"\n")
assert cfg.allowed_labels == {"python", "java"}


def test_parse_helper_directly():
"""Unit-level: the pure helper, independent of env loading."""
from shadow.config import _parse_allowed_labels, _DEFAULT_ALLOWED_LABELS
d = _DEFAULT_ALLOWED_LABELS
assert _parse_allowed_labels(None, None, d) == set(d)
assert _parse_allowed_labels("python,java", None, d) == {"python", "java"}
assert _parse_allowed_labels(None, ["bug"], d) == {"bug"}
assert _parse_allowed_labels(" ", None, d) == set(d) # blank env -> default
assert _parse_allowed_labels(None, [], d) == set(d) # empty list -> default
assert _parse_allowed_labels("python", ["bug"], d) == {"python"} # env > yaml


def test_rate_limit_label_reaches_github_branded(monkeypatch):
"""End-to-end through act(): a rate_limited ESCALATE artifact must actually
apply a <bot_name>:rate-limited label to the issue. This label is NOT in
allowed_labels, so it must be appended AFTER the allowlist filter (like
escalate_label) — an earlier version wrote it into the artifact only to have
it silently dropped by the filter, so it never reached GitHub. Branded with
bot_name (deequ-bot:rate-limited), not a hardcoded shadow: literal."""
import json
from unittest import mock
_set_required_env(monkeypatch)
with tempfile.TemporaryDirectory() as tmp:
Path(tmp, ".shadow.yml").write_text("bot:\n name: deequ-bot\n")
monkeypatch.setenv("SHADOW_REPO_ROOT", tmp)
artifact = Path(tmp, "bot_result.json")
artifact.write_text(json.dumps({
"action": "ESCALATE", "reason": "rate_limited",
"labels": ["needs-human"], "response": "",
"number": 42, "is_pr": True,
"title": "t", "html_url": "https://github.com/o/r/pull/42",
"prompt_id": "n/a", "model_id": "m",
}))
monkeypatch.setenv("ARTIFACT_PATH", str(artifact))
monkeypatch.setenv("SHADOW_VERIFY_ARTIFACT", "false") # skip HMAC in unit test

import shadow.main as m
importlib.reload(m)
captured = {}
fake_gh = mock.MagicMock()
fake_gh.add_labels.side_effect = lambda n, labels: captured.setdefault("labels", labels) or True
with mock.patch.object(m, "GitHubClient", return_value=fake_gh), \
mock.patch.object(m, "SlackClient", return_value=mock.MagicMock()):
m.act()

assert "deequ-bot:rate-limited" in captured["labels"], captured
assert "needs-human" in captured["labels"] # escalate_label still applied
Loading