feat: let an agent mention configured people when a change needs a human review

README.md

@@ -534,6 +534,35 @@ paths:
 - Quote each glob. A YAML value that starts with `*` must be quoted to stay valid
   YAML for other tools.
 
+`mention` lets an agent ask specific people for a human review. It takes a list of
+GitHub logins or `org/team` slugs:
+
+```yaml
+mention:
+  - octocat
+```
+
+- The agent decides when a change needs a human review: its role prompt tells it
+  when to set `humanReview`, one sentence on why. The PR comment then shows a
+  visible `📣 @octocat: the docs reviewer asks for your review.` line with that
+  reason under the overall summary. Without a `humanReview`, the agent mentions no
+  one.
+- Only the trusted config names people. The model writes only the reason, and the
+  reporter breaks every `@` mention in model-written text: the reason, the summaries,
+  and each finding's title, rationale and suggestion.
+- The bot edits its comment in place on each re-review. GitHub also notifies a
+  person when an edit adds a mention of them, so a mention that first appears on a
+  re-review still notifies. Each person is notified once per comment: if a request
+  goes away on a later run and comes back, the line names them without a live
+  mention. A comment the bot deletes and posts again starts over.
+- A team mention may not notify anyone. GitHub notifies only people with read access
+  to the repository, and a team only when the comment's author can see it. The
+  default `GITHUB_TOKEN` posts as `github-actions[bot]`, which is not an organization
+  member. This is untested: if a team must hear about it, list its people's logins.
+- An invalid `mention` (not a list or an empty list) is ignored. An entry that is
+  not a login or team is left out, and the agent still mentions the others. Either
+  way the review names the agent file and the fix under **Review setup**.
+
 For a real-world example, see eas-cli's
 [`.expo-agents/code-review/`](https://github.com/expo/eas-cli/tree/main/.expo-code-review)
 — correctness/security/consistency agents, a stronger model for security, and

llp/0002-review-engine-pipeline.explainer.md

@@ -99,6 +99,8 @@ Each successful reviewer pass may also return a bounded conclusion-only trace: u
 
 A reviewer pass may also return a `summary`: one or two sentences on what it reviewed and concluded, for the human readers of the comment. The parser fails soft in the same way as the trace, flattens the text to one line, and cuts it at 280 characters. `runReview()` keeps up to two distinct pass summaries per agent, so a chunked agent reports each part; they are the first two in diff order, not in the order the passes finish, so the same run always keeps the same parts. The parts are keyed in a `Map`, so an agent id such as `constructor` cannot read an inherited property. `buildAgentSummaries` turns them into one row per roster agent: the agents that ran (each once, even an agent whose id collides with the cross-file pass's `cross-cutting`), in run order and with the cross-cutting pass last, then the agents that did not run, marked `paths` (the run-level `paths` filter skipped them) or `router` (the router did not select them). An agent that ran but returned no summary keeps its row without text. Over the 8 KB budget, the last rows lose their text but keep their place. `attachAgentSummaries` attaches the list to `CoordinatorOutput.agentSummaries` at the same point as the trace and for the same reason: the coordinator reads untrusted PR data, so it can never author the field. The summaries never reach coordination, verification, suppression, or the decision [observed: `schema.ts` `ReviewerOutputSchema`/`AgentSummarySchema`; `review.ts` `mergeAgentSummary`/`buildAgentSummaries`/`attachAgentSummaries`].
 
+A pass may also return `humanReview`, one sentence on why a person should review the change, parsed and cut like the summary. `runReview()` keeps the first reason per agent. `buildMentions` turns a reason into a mention only for an agent that ran and whose front matter has a `mention` list, and the people come from that list, never from the model. `attachMentions` attaches the result to `CoordinatorOutput.mentions` with the other engine-owned fields and strips any coordinator-authored value. A mention never affects the decision or the finding set [observed: `review.ts` `buildMentions`/`attachMentions`].
+
 Observability must never break a review, so both output sinks swallow their own errors: `safeLog` wraps `writeRunLog` and discards any write error [observed] (`review.ts:1269-1275`), and `appendStepSummary` is a no-op outside Actions and swallows append failures [observed] (`step-summary.ts:9-19`). A consequence worth stating: a missing or corrupt run-log entry is therefore **not** an error signal — the log's presence or absence proves nothing about whether the review succeeded.
 
 Finally, `publicFailureReason()` (`src/core/util.ts`) is the only failure text allowed into PR-facing comments [observed] (`util.ts:20-27`; used in `src/commands/ci.ts:176,365,575`). `run()` throws errors of the form `"Command failed: <cmd> <argv>\n<stderr>"`, which embed a subprocess's full command line, its stderr, and absolute runner paths — CI-internal detail of no use to a PR author and exactly what must not leak into an attacker-visible artifact. Those shapes collapse to a generic pointer at the workflow log; the tool's own argv/path-free messages pass through, since they are the actionable ones the fail-fast design means to surface. Every call site still writes the full reason to the job's stderr.

llp/0005-verification-fingerprints-rendering.explainer.md

@@ -65,6 +65,10 @@ The full review may contain an engine-owned `reviewTrace`. It is deliberately in
 
 The engine-owned `agentSummaries` live in `review` for the same reason, but they are rendered: a collapsed "🔎 Reviewer summaries" section under the overall summary (and under each scope's summary in the aggregate comment) lists one line per roster agent, with its summary, "no summary", or why it did not run. Summaries are model text that reaches the visible body, so `plainModelLine` strips formatting tags, escapes the `&` of every HTML character reference that spells `@` (GitHub decodes `&#64;` or `&commat;` before it looks for mentions; a reference after the `@` needs nothing, since that `@` is broken anyway), escapes every `<` (which also defuses a forged state marker), collapses the text to one line, and puts a zero-width space after every `@` that does not follow an ASCII letter or digit, so a summary can never notify a GitHub user or team. The rule is deliberately broader than GitHub's own (which needs a non-word character before the `@`): markdown starts a new text node at punctuation such as `_`, so `_@login_` renders as `<em>@login</em>` and notifies, and only a letter or digit before the `@` is safe to trust, which keeps an email address whole [observed: `render.ts` `agentSummariesSection`/`plainModelLine`].
 
+The engine-owned `mentions` are the one place a review notifies people on purpose (the `/dismiss` and reply credits only name the person who acted). Each renders as a visible `> 📣 @login: the \`agent\` reviewer asks for your review.` line above the summaries section, followed by the agent's reason through `plainModelLine`. In the aggregate comment the line sits inside its scope's fold, so a request opens that fold like a finding does. Every other piece of model text that reaches a comment, the summaries and each finding's title, rationale, suggestion, source titles and requalification reason, goes through `modelText`, which escapes `@` character references and breaks each `@` by the same rule as the summaries while it keeps the text's lines and formatting; so do the inline finding comments [observed: `render.ts` `modelText`/`breakMentions`]. The renderer checks every login against `MENTION_TARGET_RE` again, because a re-render reads the state back from the comment body. The reporter edits the comment in place on later runs. GitHub documents that an edit which adds a mention notifies that person, so a mention that first appears on a re-review still notifies [observed: GitHub Docs, "Basic writing and formatting syntax", mentioning people and teams; `render.ts` `mentionLines`].
+
+That same rule would notify a person again each time a request returns: `humanReview` is model output recomputed on every run, so it can drop out on one push and come back on the next. The comment state therefore carries `mentioned`: the handles the body mentions for real (`live`) and every handle the comment has ever notified (`notified`). A handle stays live while it stays in the comment, and one that left and came back renders with a zero-width space after its `@`, so the line still names the person without notifying them. `report`, `reportAggregate` and the `/dismiss` re-render all pass the previous state's `mentioned` back in. A comment the bot deletes (`clear()`) and posts again starts with no state, so it notifies again [observed: `render.ts` `nextMentionState`; `reporters/github.ts`].
+
 ## Review Result Cache
 
 The same durable comment state is the cache for automated CI reviews. `reviewInputHash` (`src/core/review-cache.ts`) hashes each scoped changed file's content from the materialized PR-head tree, a normalized patch (blob ids and hunk coordinates removed; deletion/mode information retained), and every stable input that can change the result: the engine package version, resolved prompts/config/models, PR title/body, selected agents and routing mode, and external context. It deliberately excludes the temporary trusted-config path and the base/head commit OIDs. A restack changes OIDs and may move hunk line numbers; if it leaves the actual review input byte-identical, CI reuses the complete `CoordinatorOutput` already embedded in the most recent author-verified reviewer comment instead of launching the agents again. Symlinks are hashed as link targets and never followed. A package release changes the key so upgraded review logic runs once. Legacy comments store one `inputHash`; aggregate comments store one per `ScopeReviewResult`, so an unchanged scope can hit independently while a changed sibling reruns.

llp/0006-config-schema-loading-routing.explainer.md

@@ -155,7 +155,8 @@ as a scalar, exactly as before lists existed, so `description`, `alwaysRun`, `mo
 the value. Keys whose value is a list ALSO land in `lists`: a one-line
 flow list (`paths: ["a/**", 'b/**', c/**]`, trailing comma allowed) or a block list (`paths:`
 followed by `- item` lines; blank and `#` lines inside are skipped, and the next key ends it)
-[observed: `load.ts` `parseFrontmatter`, `parseFlowList`]. Only `paths` reads `lists`.
+[observed: `load.ts` `parseFrontmatter`, `parseFlowList`]. Only `paths` and `mention` read
+`lists`.
 
 `parseAgentPaths` validates `paths` at load time, because a mistyped glob would otherwise skip
 the agent on every PR with nothing reporting it. An invalid value is a **setup warning, not a load
@@ -202,6 +203,20 @@ and its review-cache key are unchanged [observed: `load.ts` agent construction].
 still runs only in scopes with a matching file. How the engine applies the filter is in
 [LLP 0002](0002-review-engine-pipeline.explainer.md#agent-paths-filter).
 
+`mention` names the people an agent may ask for a human review: a list of GitHub logins or
+`org/team` slugs, with an optional leading `@` that the loader drops. It is the only source of
+whom a review notifies. The agent itself only writes the reason, so a PR cannot make the bot
+mention anyone: CI loads agent files from the base commit, and the model never chooses a login.
+`parseAgentMention` treats a value that is not a list, or an empty list, like an invalid `paths`:
+a setup warning that names the file and the fix, and the agent mentions no one. An entry that does
+not match `MENTION_TARGET_RE` is different: it is left out with a warning that names it, and the
+agent still mentions the valid entries, because one typo should not silence the people listed
+beside it. `MENTION_TARGET_RE` follows GitHub's rules: a login is at most 39 letters, digits and
+single inner hyphens, with an optional Enterprise Managed User `_shortcode` suffix, and a team slug
+is letters, digits and underscores joined by single hyphens. An agent without `mention` gets no
+`mention` key, so its shape and review-cache key are unchanged [observed: `load.ts`
+`parseAgentMention`; `schema.ts` `MENTION_TARGET_RE`].
+
 ## Budgets and Chunking Defaults
 
 `budget.totalPassesMinutes` defaults to 55, not the scaffolded workflow's full 90-minute timeout,

src/__tests__/config-load.test.ts

@@ -298,3 +298,60 @@ test("loadReviewConfig: `includeSuggestions` loads as a floor, with a deprecatio
     }
   }
 });
+
+test("loadReviewConfig: front-matter `mention` loads GitHub logins and teams onto the agent", async () => {
+  const root = await configDirWithAgent(
+    '---\nmention:\n  - amandeepmittal\n  - "@expo/docs"\n---\nIOS',
+  );
+  try {
+    const config = await loadReviewConfig(root);
+    const ios = config.agents.find((agent) => agent.id === "ios")!;
+    const plain = config.agents.find((agent) => agent.id === "plain")!;
+    expect(ios.mention).toEqual(["amandeepmittal", "expo/docs"]);
+    expect("mention" in plain).toBe(false);
+    expect("setupWarnings" in config).toBe(false);
+  } finally {
+    await rm(root, { recursive: true, force: true });
+  }
+});
+
+test("loadReviewConfig: an invalid `mention` is a warning, and the agent mentions no one", async () => {
+  const cases: Array<[string, string]> = [
+    ["mention: amandeepmittal", "is not a list"],
+    ["mention: []", "is empty"],
+    ['mention: ["aman deep"]', 'entry 1 ("aman deep") is not a GitHub login or team'],
+  ];
+  for (const [line, expected] of cases) {
+    const root = await configDirWithAgent(`---\n${line}\n---\nIOS`);
+    try {
+      const config = await loadReviewConfig(root);
+      const ios = config.agents.find((agent) => agent.id === "ios")!;
+      expect("mention" in ios, line).toBe(false);
+      expect(config.setupWarnings, line).toHaveLength(1);
+      const [warning] = config.setupWarnings!;
+      expect(warning).toContain(path.join(CONFIG_DIRNAME, "agents", "ios.md"));
+      expect(warning).toContain(expected);
+      expect(warning).toContain("mentions no one");
+    } finally {
+      await rm(root, { recursive: true, force: true });
+    }
+  }
+});
+
+test("loadReviewConfig: an invalid `mention` entry is left out, and the valid ones still load", async () => {
+  const root = await configDirWithAgent(
+    '---\nmention: ["amandeepmittal", "aman deep", "expo/docs", "-x"]\n---\nIOS',
+  );
+  try {
+    const config = await loadReviewConfig(root);
+    const ios = config.agents.find((agent) => agent.id === "ios")!;
+    expect(ios.mention).toEqual(["amandeepmittal", "expo/docs"]);
+    expect(config.setupWarnings).toHaveLength(1);
+    const [warning] = config.setupWarnings!;
+    expect(warning).toContain(path.join(CONFIG_DIRNAME, "agents", "ios.md"));
+    expect(warning).toContain('entries 2 ("aman deep"), 4 ("-x") are not a GitHub login or team');
+    expect(warning).toContain("leaves them out");
+  } finally {
+    await rm(root, { recursive: true, force: true });
+  }
+});

src/__tests__/render-aggregate.test.ts

@@ -146,6 +146,49 @@ test("aggregate: each scope lists its own agent summaries inside its fold", () =
   expect(body).toContain("- **security:** Checked the token handling.");
 });
 
+test("aggregate: a scope's mention renders under that scope's summary", () => {
+  const results = [
+    scope("docs", false, {
+      summary: "Docs scope summary.",
+      mentions: [{ agent: "docs", logins: ["amandeepmittal"], reason: "New guide." }],
+    }),
+  ];
+  const body = renderAggregateMarkdown(results, "tag", []);
+  const line = "> 📣 @amandeepmittal: the `docs` reviewer asks for your review. New guide.";
+  expect(body.indexOf(line)).toBeGreaterThan(body.indexOf("Docs scope summary."));
+});
+
+test("aggregate: a scope with a human-review request and no findings opens its fold", () => {
+  const results = [
+    scope("docs", false, {
+      summary: "Docs summary for @summary-user.",
+      mentions: [{ agent: "docs", logins: ["amandeepmittal"], reason: "New guide." }],
+    }),
+    scope("api", false, {}),
+  ];
+  const body = renderAggregateMarkdown(results, "tag", []);
+  expect(body).toContain("<details open>\n<summary>docs — ");
+  expect(body).toContain("<details>\n<summary>api — ");
+  expect(body).toContain("Docs summary for @\u200Bsummary-user.");
+});
+
+test("aggregate: the mention state rides the comment, so a returning request does not notify", () => {
+  const asking = [
+    scope("docs", false, {
+      mentions: [{ agent: "docs", logins: ["amandeepmittal"], reason: "New guide." }],
+    }),
+  ];
+  const first = renderAggregateMarkdown(asking, "tag", []);
+  expect(first).toContain("> 📣 @amandeepmittal:");
+  const dropped = renderAggregateMarkdown([scope("docs", false, {})], "tag", [], undefined, {
+    mentioned: parseReviewState(first, "tag")!.mentioned!,
+  });
+  const back = renderAggregateMarkdown(asking, "tag", [], undefined, {
+    mentioned: parseReviewState(dropped, "tag")!.mentioned!,
+  });
+  expect(back).toContain("> 📣 @\u200Bamandeepmittal:");
+});
+
 test("aggregate: a default-scope finding dismissed pre-routing (plain fp) lands in Dismissed (risk 9)", () => {
   const dFinding = finding({ title: "Carried", evidence: "const carriedOverDismissal = 1;" });
   const plainFp = fingerprintFinding(dFinding);