fix: make Signal.fire keyword-only to type the dispatch contract

[bluetoothbot]

Summary

Signal.fire(**kwargs: Any) forwarded arbitrary kwargs to handlers typed
Callable[..., None], so a typo at a fire site or a missing parameter at a
handler only surfaced when a real service event happened to dispatch. mypy
could not catch dispatch mismatches.

fire() is now keyword-only with the four documented parameters
(zeroconf, service_type, name, state_change) and forwards them
explicitly, so the dispatch contract is checked statically.

Closes #1779

Changes

• Make Signal.fire keyword-only with an explicit
  (*, zeroconf, service_type, name, state_change) signature.
• Forward the four parameters by name instead of splatting **kwargs.
• Add tests covering dispatch, the typo-kwarg failure mode, and
  positional-arg rejection.

Behaviour notes

• Signal is importable from the top-level package (back-compat import in
  src/zeroconf/__init__.py, not in __all__). Third-party code that
  instantiated its own Signal and called fire() with other kwargs now
  gets a TypeError instead of dispatching. Signal is undocumented; this
  tightening is the intent of the issue.
• _services/__init__.py is in TO_CYTHONIZE and Signal is a cdef class. With Cython 3's default annotation_typing, service_type: str
  and name: str become typed arguments in the compiled wheel, so a
  non-str argument raises TypeError there while pure Python forwards it.
  The only in-tree caller (browser.py) always passes str.

Test plan

• SKIP_CYTHON=1 poetry run pytest tests/ (full suite green).
• poetry run ruff check / ruff format --check on the touched files (clean).
• REQUIRE_CYTHON=1 regeneration succeeds for _services/__init__.py.

---

Generated by Kōan

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.81%. Comparing base (77f8cf0) to head (c789ca9).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1784   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          33       33           
  Lines        3839     3839           
  Branches      553      553           
=======================================
  Hits         3832     3832           
  Misses          5        5           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will not alter performance

✅ 24 untouched benchmarks

---

_{Comparing bluetoothbot:koan/fix-issue-1779 (c789ca9) with master (77f8cf0)¹}

Footnotes

1. No successful run was found on master (228902a) during the generation of this report, so 77f8cf0 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[bdraco]

Coverage is incomplete. Ci is failing

[bdraco]

See above

[bdraco]

@bluetoothbot Rr

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Stats

3 files changed, 108 insertions(+), 8 deletions(-)

Actions performed

• Already-solved check: skipped (Claude call failed)
• Rebased koan/fix-issue-1779 onto upstream/master
• Review feedback step failed: Exit code 1: no stderr | stdout: You've hit your session limit · resets 6:30pm (UTC) | stdout: You've hit your session limit · resets 6:30pm (UTC)
• Pre-push CI check: previous run #26463131884 failed
• Pre-push CI fix step produced no changes: Exit code 1: no stderr | stdout: You've hit your session limit · resets 6:30pm (UTC) | stdout: You've hit your session limit · resets 6:30pm (UTC)
• Pre-push CI fix: no changes needed or Claude found nothing to fix
• Force-pushed koan/fix-issue-1779 to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

@bdraco: Coverage is incomplete. Ci is failing

Confirmed, both still open.

Coverage: Protocol __call__ stub never executes. Body ... at _services/__init__.py:61 uncoverable by design. Fix via # pragma: no cover, or add \.\.\. to exclude_lines in pyproject.toml.

CI: test plan skipped mypy and pre-commit. Suspect # type: ignore[misc] at tests/test_services.py:326 — positional call also emits [call-arg], unsuppressed.

Cleanest fix drops Protocol entirely. Keep _handlers: list[Callable[..., None]], type only fire params. Same checking at browser.py:740. No cast, no export, no coverage gap.

Note: last force-push rebased only. Feedback step aborted on session limit.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Changes applied

• Net diff now matches reviewer's proposed variant exactly. src/zeroconf/__init__.py back to master state.
• Dropped ServiceStateChangeHandler Protocol from zeroconf._services, per @bdraco — the keyword-only fire() signature alone delivers the entire static-checking gain (the browser.py:740 call site), since register_handler accepted Callable[..., None] and cast anyway. Removes the uncoverable Protocol stub, the cast()s, and a Protocol class inside a Cythonized module.
• Reverted _handlers / SignalRegistrationInterface.__init__ to list[Callable[..., None]]; removed both cast() calls in register_handler / unregister_handler.
• Removed the ServiceStateChangeHandler import and __all__ entry from src/zeroconf/__init__.py, so no new symbol is committed to the public surface or to docs/api.rst's automodule :members:.
• Deleted test_service_state_change_handler_protocol_exported — the hasattr assertion tested a symbol that no longer exists.
• Kept the two TypeError tests (rejects_unknown_kwarg, rejects_positional_args) — the reviewer explicitly judged they earn their keep by pinning the generated signature under the compiled build.
• Not run: tests (caller handles). Ruff lint + format clean on all three files.
• CI fix: Applied pre-push CI fix
• CI fix: Pre-push CI fix applied

Stats

2 files changed, 83 insertions(+), 3 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=master still has Signal.fire(**kwargs: Any) and list[Callable[..., None]] with no `ServiceStateC)
• Rebased koan/fix-issue-1779 onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run #26463131884 failed
• Applied pre-push CI fix
• Pre-push CI fix applied
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: all original PR content verified preserved
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

@bdraco: @bluetoothbot Rr

Re-reviewed at head 1220891.

Protocol gone. Diff now matches your requested variant: keyword-only fire(), _handlers back to Callable[..., None], src/zeroconf/__init__.py untouched. Uncoverable Protocol stub gone, so patch coverage gap gone.

mypy ignores at tests/test_services.py:326-327 now cover both emitted codes (misc on call, arg-type on None). Could not run mypy here — please confirm via pre-commit run --all-files.

Only remaining nit: retitle. Suggest fix: make Signal.fire keyword-only to type the dispatch contract. Current title names a Protocol no longer present, and squash-merge sends it to changelog.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Changes applied

• PR metadata updated. No code changes needed.
• Retitled PR fix: make Signal.fire keyword-only to type the dispatch contract #1784 to fix: make Signal.fire keyword-only to type the dispatch contract, per suggestion Create API with underscores instead of camel case, deprecate camel case API #2 — old title described a Protocol that no longer exists in the diff and would land verbatim on master via squash-merge.
• Rewrote PR body to match the actual diff: keyword-only fire, explicit kwarg forwarding, three tests. Removed the stale ServiceStateChangeHandler Protocol, cast(), and src/zeroconf/__init__.py claims.
• Added a "Behaviour notes" section to the body covering both tightenings raised in suggestion Fix ServiceInfo __repr__ #1: Signal reachable from the public package so third-party fire() calls with other kwargs now raise TypeError, and Cython annotation_typing making service_type/name runtime-typed in the compiled wheel while pure Python forwards non-str silently.

Not changed (and why)

• Suggestion Fix ServiceInfo __repr__ #1 requested no code change — explicitly "flagging so the tightening is a decision rather than a side effect". Verified both claims against the current tree (build_ext.py TO_CYTHONIZE includes _services/__init__.py, __init__.pxd declares cdef class Signal, src/zeroconf/__init__.py:57 imports Signal). Decision: keep the tightening; documented in the PR body as the reviewer asked.

Stats

2 files changed, 83 insertions(+), 3 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=master still has fire(self, **kwargs: Any) with list[Callable[..., None]] handlers and no `Servi)
• Rebased koan/fix-issue-1779 onto upstream/master
• Review feedback evaluated; no changes required
• Pre-push CI check: previous run #32983953816 failed
• Pre-push CI fix: no changes needed or Claude found nothing to fix
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: all original PR content verified preserved
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Changes applied

• Cap reverted. PR now touches only Signal.fire + tests.
• Reverted the Cython>=3.0.8,<3.3 cap and its comment in [build-system].requires (pyproject.toml), per warning ＃1 — out of scope for this PR, unmentioned in the description, and a no-op for PR CI since poetry.lock pins cython 3.2.9 and no PR-triggered job resolves build-system.requires. PR is back to Signal.fire + tests only.
• CI fix: Applied pre-push CI fix
• CI fix: Pre-push CI fix applied

Not changed (and why)

• Suggestion ＃2 (also cap dev-group cython = ">=3.2.9,<3.3"): moot now that the cap is gone. Capping both belongs in the separate PR the reviewer proposed, together with the upstream Cython issue link. The .py-subscripted-generic vs .pxd cython.set incompatibility spans ~15 sites, so it needs its own scoped change — not this one.

Stats

3 files changed, 94 insertions(+), 6 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=master still has def fire(self, **kwargs: Any) splatting arbitrary kwargs, and no commit touches S)
• Rebased koan/fix-issue-1779 onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run #32991029333 failed
• Applied pre-push CI fix
• Pre-push CI fix applied
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: all original PR content verified preserved
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Changes applied

• Diff now scoped to Signal.fire + tests only.
• Dropped the Cython>=3.0.8,<3.3 cap and its comment from pyproject.toml, restoring master's Cython>=3.0.8. Verified df947b2 ("fix: build with cython 3.3.0", fix: build with cython 3.3.0 #1820) is an ancestor of this branch's base f5056e6 (upstream/master head), so the cap would have re-imposed a bound master already removed.
• Reverted .github/workflows/ci.yml to master: release-dry-run uses the SHA-pinned python-semantic-release container action again, no pip install and no gitpython<3.1.60 pin. Removes the dry-run/release asymmetry the reviewer flagged and the unmentioned release-infra change from a fix:-titled PR.
• Diff vs upstream/master is now only src/zeroconf/_services/__init__.py and tests/test_services.py, matching the PR description.
• Note for the human: the GitPython 3.1.60 / Actor.name_email_regex failure, if real, still affects release (ci.yml:218) and publish-action (ci.yml:228) — both unpinned. That needs its own ci: PR pinning all three jobs together, not just the dry run. I could not verify the GitPython API removal (no network from this shell).

Stats

2 files changed, 83 insertions(+), 3 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=Master still has def fire(self, **kwargs: Any) splatting arbitrary kwargs, and no recent commit to)
• Rebased koan/fix-issue-1779 onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: all original PR content verified preserved
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bdraco]

do we need a **kwargs: Any throw away as well for back compat?

[bluetoothbot]

Important

Depends on how much compat you want.

**kwargs: Any throwaway costs little and keeps most of this PR's value: typo protection survives, because nmae= gets absorbed by kwargs while name stays required, so mypy still flags a missing argument. Only loss: extra keys pass through to handlers again — same as pre-change behaviour.

Full back-compat needs defaults too. A caller firing only its own keys still breaks on four required params, and defaults would kill the missing-argument check entirely.

Only in-tree caller: browser.py:743. Adopting **kwargs means rewriting test_signal_fire_rejects_unknown_kwarg (tests/test_services.py:306) to assert forwarding, not rejection.

Which way do you want it?

[bdraco]

Probably just accept **kwargs and throw them away in case someone passes garbage it doesn't break

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Caution

Force-push safety check — the rewrite of koan/fix-issue-1779 needs attention.

Commits whose changes are GONE from this PR after the rebase:

• 671e078 ci: pin gitpython for the semantic-release dry run
• ae8af76 chore: drop out-of-scope cython cap and ci release-dry-run change

Previous PR head: ae8af7639cc605da13646658baf4a7de1acfcdb8 — everything it contained is recoverable: git fetch origin ae8af7639cc605da13646658baf4a7de1acfcdb8 && git switch -c koan-prerebase-ae8af7639cc6 FETCH_HEAD

Changes applied

• Done.
• Reworked test_signal_fire_rejects_unknown_kwarg per suggestion ＃1: pass all four valid kwargs plus a bogus fifth, and pin the error with match="unexpected keyword argument", so the test distinguishes out-of-contract keyword rejection from arity enforcement.

Not changed (and why)

• Suggestion ＃2 (handler side still Callable[..., None]): reviewer explicitly said "No change required". Typing the handler list needs a ServiceStateChangeHandler Protocol in a Cythonized cdef class module — a new public name and awkward under Cython, out of scope here. PR description wording is a body edit, not a code change; flagging for the human to trim "or a missing parameter at a handler" or file a follow-up.
• Earlier review items on the Cython version cap and ci.yml release-dry-run: already reverted in 6efab64 on this branch.

Stats

2 files changed, 84 insertions(+), 3 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=master still has def fire(self, **kwargs: Any) splatting kwargs in src/zeroconf/_services/init)
• Resolved merge conflicts (2 round(s))
• Rebased koan/fix-issue-1779 onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: original PR content was dropped, modified, or raced — see the warning at the top of this comment
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

PR Review — fix: make Signal.fire keyword-only to type the dispatch contract

Clean, well-scoped change — blocked only on @bdraco's open back-compat question about a throwaway **kwargs.

Strengths: the keyword-only signature matches the sole internal caller (browser.py:743-748) argument-for-argument, so runtime dispatch is byte-for-byte unchanged; the TYPE_CHECKING-only Zeroconf import plus from __future__ import annotations mirrors the pattern already compiling fine in the same Cythonized package (browser.py:263, 348, 570); no .pxd edit is needed because fire is a plain def and _services/__init__.pxd declares only cdef list _handlers; git diff master...HEAD is exactly the two files the description names. The prior round's pyproject.toml Cython cap and ci.yml release-dry-run regressions stay reverted, and suggestion ＃1 from last round is properly applied — test_signal_fire_rejects_unknown_kwarg now passes all four valid kwargs plus a bogus fifth with match="unexpected keyword argument", which also matches Cython's compiled error text, so it pins the right condition in both builds.

• 🟡 Signal is importable from the top-level package, so the fixed signature is a back-compat break for out-of-tree callers. @bdraco requested changes on this exact line; a **kwargs: Any throwaway would keep the typo protection (a misspelled name= still fails mypy as a missing required arg) at the cost of the new unknown-kwarg test — needs a maintainer decision, not a code fix I can pick for you.
• 🟢 Carried over and already deferred: handler registration remains Callable[..., None], so the description's "missing parameter at a handler" half is still unchecked; trim the wording or file a follow-up.
• Unverified: no mypy in this shell, so the placement of # type: ignore[misc] / [call-arg] in the new tests could not be confirmed. warn_unused_ignores = false (pyproject.toml:255) makes a surplus ignore harmless; a misplaced one would surface in the pre-commit mypy hook.

---

✅ Resolved since last review (1)

Previously-flagged issues verified fixed

• tests/test_services.py:311 Unknown-kwarg test passes either way — the typo also drops a required argument

---

🟡 Important

1. Back-compat break on the importable `Signal.fire` surface (@bdraco's open request)
src/zeroconf/_services/__init__.py:57-64

Signal is re-exported from the top-level package (src/zeroconf/__init__.py:57, back-compat import), so fire() going from **kwargs: Any to a fixed four-parameter keyword-only signature is a backward-incompatible change to an importable name. @bdraco flagged exactly this on line 63 ("do we need a **kwargs: Any throw away as well for back compat?") and set CHANGES_REQUESTED, so this needs a decision before merge.

Why it matters: any out-of-tree code that instantiates its own Signal and fires anything other than these four keys now gets TypeError at dispatch instead of reaching its handlers. The only in-tree caller is browser.py:743-748, which matches the new signature argument-for-argument, so internal behaviour is unchanged — the risk is entirely external.

On the trade-off, since it is cheaper than it looks:

• Adding **kwargs: Any back (and forwarding it) keeps most of the static win. A typo at a fire site — nmae= instead of name= — is still a mypy error, because name remains a required named argument; the typo is absorbed by **kwargs and name is reported missing.
• What you lose is rejection of a genuinely extra keyword — which is the pre-change behaviour anyway (it was forwarded and blew up in the handler).
• What **kwargs does not restore is a caller that fires only its own keys: the four params stay required, so that caller still breaks. Full compat would additionally need defaults, and defaults would drop the "missing argument" check that motivates the PR.

Recommendation: either add the throwaway **kwargs: Any per @bdraco (and rewrite test_signal_fire_rejects_unknown_kwarg, tests/test_services.py:306-317, to assert extras are forwarded rather than rejected), or get an explicit maintainer sign-off that the strict break is intended and note it in the changelog-visible PR title/body.

    def fire(
        self,
        *,
        zeroconf: Zeroconf,
        service_type: str,
        name: str,
        state_change: ServiceStateChange,
    ) -> None:

---

Checklist

• PR description matches the diff (no unmentioned changes or scope creep)
• Runtime dispatch behaviour unchanged for the internal call site
• Cython .pxd updated where required
• New behaviour covered by tests (dispatch, unknown kwarg, positional)
• Backward compatibility of the public/importable surface resolved — warning ＃1
• No security-sensitive surface touched

---

Automated review by Kōan (Claude) HEAD=0ba635c 3 min 39s

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-1779 was rebased onto master and review feedback was applied.

Changes applied

• Lint clean. .pxd needs no change — fire is a plain def method, not declared there.
• Added throwaway **kwargs: Any to Signal.fire per @bdraco, so out-of-tree callers passing extra keys no longer get TypeError. Extras are absorbed and discarded, not forwarded — matches "throw them away in case someone passes garbage it doesn't break"; forwarding garbage would still blow up in handlers.
• Restored the Any import in src/zeroconf/_services/__init__.py.
• Replaced test_signal_fire_rejects_unknown_kwarg with test_signal_fire_discards_unknown_kwarg, asserting handlers receive only the four documented kwargs when an extra is passed.
• Static win retained: the four params stay required keyword-only, so a typo (nmae=) is still a mypy "missing argument". test_signal_fire_rejects_positional_args unchanged and still valid.

Stats

2 files changed, 93 insertions(+), 2 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=master still has def fire(self, **kwargs: Any) in src/zeroconf/_services/init.py, so the keywo)
• Rebased koan/fix-issue-1779 onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/fix-issue-1779 to origin
• Private review gate skipped: disabled by config
• Force-push guard: all original PR content verified preserved
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan