fix(kiss): the TNC built a SharedPtt and never started its watchdog (#1299) - #1306
Merged
Conversation
…1299) `openpulse-kiss` constructs `SharedPtt::new(ptt, DEFAULT_PTT_MAX)` and takes guards from it, but called `spawn_watchdog` nowhere — so `force_release_if_expired` had no caller in the crate and the deadline was never checked. A `SharedPtt` with no watchdog thread is the bare `Box` with extra steps. The RAII guard already covers an early return and an unwind. What it cannot reach is a transmit that BLOCKS rather than returns, which is the case the watchdog exists for — on a real rig, a stuck carrier with nothing to take it back. Spawned in the constructor as ARDOP does, not in `run_with_listener`: a caller holding `bridge()` can transmit without running the listener. Found while writing #1260's Twins section, whose first draft claimed the repeater was the last hand-rolled keying path. The census that corrected it put KISS in a third state — guarded but unwatched. #1299's other half stays open: `openpulse-cli` still keys a bare `PttController` at three sites. Implements: REQ-PTT-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the KISS half of #1299. The CLI half stays open.
What was wrong
openpulse-kissconstructsSharedPtt::new(ptt, DEFAULT_PTT_MAX)(bridge.rs:105) and takes guards from it (bridge.rs:131) — but calledspawn_watchdognowhere in the crate, soforce_release_if_expiredhad no caller and the 180 s deadline was never checked. ASharedPttwith no watchdog thread is the bareBoxwith extra steps, which is this PR's own phrasing from #1260.What that did and did not leave exposed
The RAII guard already covers an early return and an unwind, so this is narrower than #1260's leak. What the guard cannot reach is a transmit that blocks rather than returns — which is precisely the case a watchdog exists for, and on a real rig it is a stuck carrier with nothing to take it back.
Where it goes
The constructor (
KissServer::with_ptt), matching ARDOP (ardop/src/lib.rs:112) — notrun_with_listener, because a caller holdingbridge()can transmit without ever running the listener. TheJoinHandleis dropped deliberately: the thread exits when the lastSharedPttclone drops.No observer, unlike ARDOP's: KISS takes its guards with
keyed(None)and has no event surface to notify.How it was found
Writing #1260's Twins section, whose first draft claimed the repeater was "the last hand-rolled keying path". It was not, and the census that corrected it put KISS in a third state nobody had named — guarded but unwatched:
SharedPttTests
the_watchdog_force_releases_a_key_that_outlives_its_deadlinedrives the real constructor — a test that built its ownSharedPttwould pass against the unfixed crate, since the defect was the wiring. It shortens the deadline and keys without holding a guard, standing in for a blocking transmit; a dropped guard would release on its own and prove nothing about the watchdog.Sabotage-verified: removing the
spawn_watchdogline fails it with "the transmitter is STILL KEYED past its deadline".GATE: PASS d2f751a7ba0602814a51107103451fa838233561 clean— 325 suites, 2497 passed, 0 failed.Still open
openpulse-clikeys a barePttControllerattransmit.rs:18,calibrate.rs:201and:328. Lower risk — foreground, attended, andtransmit.rsreleases before its?— but outside the discipline every other path is now under. #1299 stays open for it.Consumer
KissServer::with_ptt→KissBridge::keyed_transmit(bridge.rs:124), the only way this TNC transmits (#1259).Prior art
ardop/src/lib.rs:112is the same line for the same reason, added by the #972 follow-up that moved the watchdog core intoopenpulse-radio"for these front-ends and never wired it" — its own comment. KISS was the front-end that stayed unwired.Twins
The full census is the table above; it was the output of #1260's Twins check, so this PR is that follow-up.
openpulse-meshhad its keying capability removed rather than guarded.Implements: REQ-PTT-01
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6