Skip to content

Match a risk topic only on the line that registers the tool - #684

Merged
Birfy merged 1 commit into
mainfrom
fix/679-risk-topic-registration
Oct 22, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/679-risk-topic-registration

Conversation

@Birfy

@Birfy Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes #679

Tip SHA: 75ebb1a1f621ec977ea0848916c57dd00ab92e33

Reproduce

.github/agent/risk-rules.json lists "topics": ["create_child_bot"] for T3 and .github/scripts/risk-gate.mjs applied it as added.includes(w) over every added line of every changed code file. Any diff that merely named the tool was therefore T3: #636 adds one prompt sentence for the model that names create_child_bot (a tool that already exists) and its paths (plugins/**) are T2, yet it reported T3 by that rule alone. The new test #679: a topic counts on the registration line… reproduces it: with the old line, that shape computes T3 with prompt.js: topic create_child_bot in the basis.

What changed

  • .github/scripts/risk-gate.mjs: a topic is compiled once per rule into /name\s*:\s*['"]<tool>['"]/ (topicName) and matched against the added lines instead of a substring test. The registration shape is the one plugins/tree/src/tools.js uses, so registering the tool still counts; naming it in prose, a comment or a prompt no longer does.
  • .github/scripts/risk-gate.test.mjs: the existing content-hit case now uses the registration line, plus one new test with both acceptance cases — tree: add move_bot for reorganising an existing tree #636's shape (plugins/tree/src/prompt.js naming the tool + a plugins/tree/src/tree.js change) is T2 and its basis has no topic line, and a + name: 'create_child_bot', line in plugins/tree/src/tools.js is T3 with plugins/tree/src/tools.js: topic create_child_bot.
  • No change to .github/agent/risk-rules.json: the rule list stays ["create_child_bot"]; only the shape it is matched against is narrower. No other rule, workflow or label behaviour is touched, and merge-gate.mjs gets the same judge() by import, so the queue's tier matches the report by construction.
  • No subsystem doc owns .github/scripts, so no doc change is needed (arch-docs reports OK).

What I ran

At 75ebb1a1 (repo root, TZ=UTC):

  • node --test .github/scripts/*.test.mjs → 154 tests, 150 pass, 4 fail. The 4 failures are present on unmodified main in this sandbox and are the ds-bot-patches.mjs tests that shell out to git, which this box does not have (Cannot read properties of undefined (reading 'split')); on a clean extraction of 8e80237b the same suite is 153 tests, 149 pass, 4 fail — so this change adds one passing test and breaks nothing.
  • node --check on both changed files.

Red without the source change

Put the loop back to if (added.includes(w)) (tests kept): node --test --test-name-pattern="a topic counts" → 1 fail, expected T2, actual T3 — the #636 shape is raised by the prose sentence again. The registration case passes in both versions on purpose: it is the positive control that shows the rule was narrowed, not switched off.

Not run

  • npm run verify and the CI package job: this box has no core/node_modules and no git, so the core/plugin suites and the pack check cannot run here; CI covers them. This change only touches .github/scripts, so tests-ok (core, plugins, package) runs against unchanged code.
  • risk-gate.mjs --pr <n> against tree: add move_bot for reorganising an existing tree #636 live: it needs GH_TOKEN and network to the API, which the sandbox has, but it would only re-derive what the test replays from tree: add move_bot for reorganising an existing tree #636's actual diff; the test asserts that shape directly.

Risk

  • T3 by .github/agent/risk-rules.json (.github/** is on the T3 path list), so this needs a PASS at the tip plus the maintainer's approve. risk-tier.yml's summary line is authoritative for the report.
  • Behaviour change is one-way and narrow: tiers can only go down for diffs that name a tool without registering it, and never up. A diff that registers create_child_bot (or, later, another listed topic) still reports T3.
  • The regex is anchored on name: with an optional quote and whitespace; a registration written some other way (for example a positional array) would no longer match. No such form exists in the repo today (plugins/tree/src/tools.js is the only registration), and the positive-control test fails if that changes.

The T3 topic rule was a plain substring match, so any diff that merely named
create_child_bot in prose became T3; #636 hit it with one prompt sentence while
its paths are T2 (#679). A topic now matches the registration shape
`name: 'create_child_bot'`, the form plugins/tree/src/tools.js uses. The rule
list in risk-rules.json is unchanged; only the shape it is matched against is.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026
@Birfy Birfy self-assigned this Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 75ebb1a
Written-by: Reviewer

Ran:

Not run:

  • a risk-tier run on the workflow itself — no workflow file is touched

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean, 0 commits behind main (6b9820bb). .github/scripts is 154 tests, 150 pass, 4 fail — the same four patch cases that need a .git, and they fail on main here too.

What I checked

  • A topic is compiled to /name\s*:\s*['"]<name>['"]/ (.github/scripts/risk-gate.mjs:46-50) and applied to added lines like the other content rules (:85), so name: 'create_child_bot' in plugins/tree/src/tools.js:100 is the positive control and a prompt sentence naming the tool no longer counts. The name is escaped before it reaches the regex, so a topic with regex metacharacters cannot break the gate.
  • risk-rules.json is untouched, and merge-gate.mjs imports the same judge(), so the tier the report prints and the tier the queue uses stay one definition.
  • Red without the source change: reverting risk-gate.mjs to 6b9820bb makes exactly the new case red (18/19), and the old substring behaviour is what it goes red on.

Non-blocking

  • The trigger now needs a literal registration line. A child-bot tool registered through a constant (name: CHILD_TOOL) or under a different key would stay T2 — the test's positive control uses the same shape as the one file the rule was derived from, so nothing covers that shape. A second pattern for ['"]<topic>['"] on a line containing name would close it without re-widening to prose.
  • .github/agent/risk-rules.json never said what a topics entry means; it is the file a future editor will read when adding one. One line there saying "a registration line, not a mention" would keep the narrowing from being undone by accident.

Risk: T3 (.github/scripts/** is a T3 path). Added awaiting-approval; needs a maintainer APPROVED review at this tip next to this PASS. Not enqueued.

@Birfy Birfy added the awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip label Oct 22, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 0583c10 Oct 22, 2026
9 checks passed
@Birfy
Birfy deleted the fix/679-risk-topic-registration branch October 22, 2026 06:22
@Birfy Birfy removed awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip agent-pr Pull request opened by an agent labels Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

risk-gate: the create_child_bot topic rule fires on a prompt sentence, not only on a tool registration

1 participant