Skip to content

fix: stop parsing email domains as maintainer handles - #533

Merged
mlieberman85 merged 1 commit into
darnitdevorg:mainfrom
akshatshahh:fix/issue-464-maintainers-email-domain
Oct 6, 2026
Merged

mlieberman85 merged 1 commit into
darnitdevorg:mainfrom
akshatshahh:fix/issue-464-maintainers-email-domain

Conversation

@akshatshahh

@akshatshahh akshatshahh commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes #464.

What: ContextSieve._parse_maintainers_file() ran its handle regex over the whole file, so the domain of every email address became a handle: - Alice Example <alice@realcorp.io> @alice produced ['@realcorp', '@alice', 'Alice Example'], scored 0.95 (EXPLICIT_FILE), auto-accepted, and the phantom handle was later rendered into CODEOWNERS.

Fix: the parser now strips email addresses before matching handles and requires the @ not to be glued to a word character (covers address-like leftovers such as user@localhost). The issue's example now yields ['@alice']. Display names are no longer emitted as maintainers either: a bare name is not a GitHub handle and GitHub rejects it as a code owner.

Tests: 6 new tests in tests/darnit/context/test_context_sieve.py — the issue's exact example, name+email with no handle, a bare email address, plain @mentions (behavior preserved), a handle glued directly after an email, and an end-to-end detect('maintainers') check. 5 of the 6 fail on the pre-fix code. Full context + remediation suites pass (346 passed), ruff check clean.

AI assistance disclosure (per AI_POLICY.md): this PR was prepared with Muse (muse.ai), an AI coding assistant, which drafted the parser fix in packages/darnit/src/darnit/context/sieve.py and the 6 new tests in tests/darnit/context/test_context_sieve.py. I reviewed every line of the change, ran the full test suite, and stand behind it.

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the parsing fix looks right. I checked the issue's example plus user@localhost @erin, [@bob](url) and (alice@corp.io), and all parse correctly. Three things before we can merge:

  1. The DCO check is failing. Please sign off your commits (git rebase --signoff main, then force-push).
  2. Please say in the AI-assistance section which tool you used and for which parts, and add an Assisted-by: trailer (see AI_POLICY.md).
  3. Rebase onto current main.

Optional, or as a follow-up: detect('maintainers') still reports confidence 1.0 for this source (context/sieve.py), which #464 asks to lower.

Drafted with Claude Code; reviewed and posted by me.

ContextSieve._parse_maintainers_file() matched @([a-zA-Z0-9]...) against
the whole file, so the domain of every email address became a handle:
'- Alice Example <alice@realcorp.io> @alice' produced ['@RealCorp',
'@alice', 'Alice Example'], scored 0.95 and auto-accepted, and the
phantom handle was later rendered into CODEOWNERS.

The parser now strips email addresses before matching handles and
requires the @ not to be glued to a word character, so the issue's
example yields ['@alice']. Display names are no longer emitted as
maintainers: a bare name is not a GitHub handle and GitHub rejects it
as a code owner.

Signed-off-by: Akshat Shah <akshatdi@usc.edu>
Assisted-by: Muse (muse.ai)
@akshatshahh
akshatshahh force-pushed the fix/issue-464-maintainers-email-domain branch from 508a4b6 to c5c14cc Compare October 6, 2026 17:11
@akshatshahh

Copy link
Copy Markdown
Contributor Author

All three addressed:

  1. DCO: rebased onto current main with --signoff and force-pushed; the commit now carries Signed-off-by.
  2. AI disclosure: the PR body now names the tool (Muse) and which parts it drafted (the parser fix and the 6 new tests), and the commit has an Assisted-by: trailer per AI_POLICY.md.
  3. Rebased onto current main (a5ec003); the 25 sieve tests pass on the rebased branch.

Leaving the confidence-1.0 follow-up for a separate PR as suggested.

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three points addressed. The parser fix and tests look good; merging.

@mlieberman85
mlieberman85 merged commit 14fe951 into darnitdevorg:main Oct 6, 2026
8 checks passed
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.

MAINTAINERS.md parser turns email domains into GitHub handles (alice@realcorp.io yields @realcorp)

2 participants