Skip to content

feat(settings): opt-in mention trigger and import label filter - #230

Open
471402921 wants to merge 3 commits into
alvarosanchez:mainfrom
471402921:feat/mention-trigger-and-label-filter
Open

471402921 wants to merge 3 commits into
alvarosanchez:mainfrom
471402921:feat/mention-trigger-and-label-filter

Conversation

@471402921

Copy link
Copy Markdown

Problem

Two noise sources show up as soon as a repo has more than one agent-facing workflow. Both are currently all-or-nothing.

1. Every trusted comment wakes the assignee. hasTrustedGitHubCommentRecords() treats any comment from a trusted author as a reason to roll the issue back to todo and wake the agent. On a shared repo, human-to-human discussion therefore keeps waking an agent that was never addressed. Teams that only want the agent on explicit hand-off end up either muting the plugin or building their own trigger outside it.

2. Every open issue is imported. listRepositoryIssuesForImport() takes state: "open" and only filters by author (ignoredIssueAuthorUsernames, default ["renovate"]). A repo with a real backlog mirrors all of it into Paperclip, so the agent's task board is mostly work it was never asked to do. A label is the natural opt-in marker, and it is already how many teams stage work.

Solution

Two new advanced settings, both empty by default = existing behaviour:

Setting Empty (default) Non-empty
triggerHandles every trusted comment counts only comments mentioning one of the handles count
requiredLabels every open issue is imported only issues carrying at least one of the labels are imported

triggerHandles

  • Read/write in the UI as a list (one per line / comma separated), @ prefix optional, case-insensitive.
  • Matching uses a real boundary: @soulpal001-morty does not match @soulpal001-morty2 or someone@handle.example. Handles may contain -, so \b is replaced by an explicit terminator.
  • The gate sits at the single convergence point hasTrustedGitHubCommentRecords(), so it covers issue comments, linked-PR top-level comments and PR review threads in one place. Wake behaviour needed no change: it is already driven by the same "is there a trusted new comment" result.
  • Comments that do not mention a handle are still consumed (lastSeenCommentCount advances), so they are not re-evaluated on every sync.
  • Unparseable handles are kept rather than dropped: dropping would silently disable the filter and restore "wake on everything", which fails open.

requiredLabels

  • Split on commas/semicolons/newlines only — not on spaces, because GitHub labels routinely contain spaces (good first issue).
  • Applied only to the new-import candidate set. Issues already in the import registry keep syncing (status, comments, linked PRs) even when they carry none of the labels, so adding a filter later never freezes or deletes existing bindings. A never-imported issue without the label is skipped, including manual sync of that issue — documented in README/SPEC.

Backward compatibility

No migration needed: missing keys normalise to [], and [] is a no-op in both filters. The two keys always appear in settings.registration (so callers don't need ?? []); eight existing assertions were updated accordingly.

Tests

New pure module src/sync-filters.ts plus coverage in tests/sync-filters.spec.ts (12), tests/issue-interactions-integration.spec.ts (5) and tests/plugin.spec.ts (mention gating for issue and PR comments, label-filter e2e).

pnpm typecheck   # 0
pnpm test        # 384 pass / 0 fail  (19 new)
pnpm build       # ok

Open questions for the maintainer

  1. Naming — repo style is ignoredIssueAuthorUsernames; would you prefer commentTriggerHandles / requiredIssueLabels? Renaming is a key + 3 call sites.
  2. Defaults — should mention-gating ever become the default? That is breaking, so I left it opt-in.
  3. PR review threads — currently gated by mentions like any other comment. If review comments should always count, it is a one-line exemption.
  4. Code blocks — @handle inside a fenced block or quote still counts (GitHub itself does not notify for those). Excluding them needs markdown parsing; I chose the simpler "may wake once too often" behaviour.

jet added 3 commits October 25, 2026 22:07
Two opt-in settings need decision logic that can be unit tested on its own, so
this adds a dependency-free module with the normalizers and predicates the sync
flow will consume:

- normalizeTriggerHandles / normalizeRequiredLabels normalize saved arrays and
  raw settings-form text. Handles are lowercased and stripped of a leading `@`;
  labels keep their display case (and their internal spaces, so
  `good first issue` survives) and only split on commas/semicolons/newlines.
- containsTriggerHandleMention matches `@handle` case-insensitively with a
  trailing boundary that excludes characters a GitHub login can continue with,
  so `@my-agent` does not match `@my-agent-2` or `@my-agent_bot`, and an email
  address is not a mention. With no handles configured it returns true, which
  keeps the historical "any trusted comment counts" behavior.
- matchesRequiredLabels compares label names case-insensitively and returns true
  when no labels are configured, which keeps the historical "import every open
  issue" behavior.

Unparseable trigger handles are intentionally kept instead of dropped: dropping
one would silently disable the mention filter and let every trusted comment wake
an agent again, whereas keeping it fails closed (the filter stays on and simply
never matches).
…tings

Adds `triggerHandles` and `requiredLabels` to the company-scoped advanced
settings payload, the settings UI, and the sync flow. Both default to an empty
list, so existing saved settings, the settings payload shape consumers, and
every current behavior stay unchanged without a migration.

`triggerHandles`: the "does this comment need the agent" decision stays in the
single funnel every trusted-comment path already flows through,
hasTrustedGitHubCommentRecords, which is shared by source-issue comments, linked
pull request comments, and pull request review threads. A comment therefore only
counts as an interaction when it mentions a configured handle *and* comes from a
trusted author; an unmentioned comment from a maintainer or the issue author no
longer rolls the status back or wakes the assignee. Comments that do not mention
a handle still advance the stored comment watermark, so they are consumed once
rather than re-evaluated on later syncs.

`requiredLabels`: filtering happens while building the import plan and only
gates *new* imports. Any GitHub issue already in the import registry keeps its
place in the plan, and `allIssuesById` still carries every non-ignored issue, so
enabling the filter never freezes, orphans, re-creates, or deletes work that
Paperclip already tracks. An issue that has never been imported is skipped even
when a sync targets it directly.

Tests cover the pure predicates, the shared comment funnel and its trust
interaction, the linked pull request comment path, and an end-to-end sync where
an unlabeled issue is not imported while a previously imported unlabeled issue
still moves to `done` after it is closed on GitHub.
Explains both opt-in gates in the first-time setup step, in the status-mapping
behavior list, and as MUST-level requirements in SPEC.md: what an empty list
means (the pre-existing behavior), how mention matching is bounded and
case-insensitive, that non-matching comments consume the comment watermark, and
that the label filter only gates new imports so already imported issues keep
synchronizing.
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.

1 participant