fix: keep Telegram receiver locks valid across exec - #113
Merged
Conversation
added 3 commits
August 16, 2026 21:50
bin/fm-tg-recv-arm.sh forked the receiver and fingerprinted it two lines
later with fm_pid_identity, which mixes the process start time with the
process image. At that instant the child is still walking its shebang
chain, so the recorded image is one that ceases to exist as soon as the
chain's last execve lands. The identity in the lock therefore described a
process image that no longer existed, and the comparison could never match
again for the life of that receiver.
The consequence was worse than a wrong string: every later arm reported
FAILED against a healthy receiver, and the emitted operating instructions
define FAILED as an alarm to repair before the turn ends - so the false
alarm pointed at a "repair" that kills a working receiver. One home has
already had that accident.
Of the three ways to close this, fingerprinting by something that survives
the exec is the one that removes the race rather than narrowing its window.
Waiting for a stable image needs a termination condition the wrapper cannot
derive, because it does not know the receiver's interpreter and so cannot
tell the chain's last hop from an intermediate one. Having the child report
its own settled identity needs a shim process between the wrapper and the
receiver, which buys correctness at the price of a new failure mode: kill
the shim and the recorded pid is dead while the receiver lives, so a later
arm starts a second one.
So fm-wake-lib.sh now separates the two things fm_pid_identity had merged:
fm_pid_incarnation what tells this process apart from a later one that
reuses its pid - Linux stat field 22, or ps lstart -
fixed at fork and unchanged by every execve, so it is
already correct for a child just forked.
fm_pid_identity the incarnation plus the image, unchanged in output.
It now carries a warning naming exactly this trap,
and every other caller fingerprints itself (or reads
back a settled process), which is where it is sound.
The receiver lock records pid-incarnation. fm_pid_incarnation_matches_record
also accepts a record written as a full identity, because the incarnation is
that record's leading fields - so a lock left by the older wrapper, whose
image half has since exec'd away, names a receiver this one can still
confirm rather than raising the same false alarm once on update.
Dropping the image from this one comparison gives up the tick-collision
tiebreak: a false match now needs pid reuse AND the same 10ms start tick,
on top of the lock's existing home and receiver-path bindings. That residual
is negligible next to a false FAILED that is certain, permanent, and points
at killing the receiver.
When the incarnation genuinely cannot be established the wrapper still
refuses and says so, rather than recording a lock nothing can confirm.
Tests drive the exec transition itself - a receiver behind a deliberately
slow interpreter hop, asserted to have left that image before the re-arms -
because a settled process is precisely the case that hides this defect. Both
new tests fail against the previous code with the reported symptom.
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
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.
Intent
Fix a process-identity race in bin/fm-tg-recv-arm.sh that makes a healthy direct-Telegram receiver report FAILED forever.
The wrapper forked the receiver and fingerprinted it two lines later with fm_pid_identity, which mixes the process start time with the process image (cmdline). At that instant the child is still walking its shebang chain, so the recorded image is one that ceases to exist as soon as the chain's last execve lands. The identity written into the lock therefore described a process image that no longer existed, and the comparison could never match again for the life of that receiver. The consequence is worse than a wrong string: every later arm reports FAILED against a healthy receiver, and firstmate's operating instructions define FAILED as an alarm to repair before the turn ends - so the false alarm points at a 'repair' that kills a working receiver. One home has already had that accident.
The captain left the choice of fix open but required that it close the race rather than narrow it, and explicitly ruled out a fixed sleep as a non-fix. Three options were weighed and the choice is deliberate: fingerprint by something that survives the exec. Waiting for a stable image was rejected because the wrapper cannot derive a termination condition - it does not know the receiver's interpreter, so it cannot tell the shebang chain's last hop from an intermediate one. Having the child report its own settled identity was rejected because it needs a shim process between wrapper and receiver, which buys correctness at the price of a new failure mode: kill the shim and the recorded pid is dead while the receiver lives, so a later arm starts a second receiver.
So bin/fm-wake-lib.sh now separates the two things fm_pid_identity had merged. fm_pid_incarnation returns only what tells a process apart from a later one reusing its pid - Linux /proc stat field 22, or ps lstart on the portable fallback - which is fixed at fork and unchanged by every execve, so it is already correct for a child just forked. fm_pid_identity keeps returning the incarnation plus the image, with byte-identical output (verified against the previous implementation on a live process), and now carries a warning naming exactly this trap. Every other caller of fm_pid_identity in the repo fingerprints itself ($$ or BASHPID) or reads back an already-settled process, which is where it is sound; the receiver arm was the unique site fingerprinting a just-forked child.
The receiver lock now records pid-incarnation instead of pid-identity. fm_pid_incarnation_matches_record also accepts a record written as a full identity, because the incarnation is that record's leading fields - so a lock left behind by the older wrapper, whose image half has since exec'd away, still names a receiver this wrapper can confirm rather than raising the same false alarm once on update. This backwards compatibility is deliberate and tested, because the un-compatible version would reproduce the exact accident being fixed during the update itself.
One trade-off is accepted knowingly and is stated in the commit message: dropping the image from this one comparison gives up the tick-collision tiebreak that the old comment called out. A false match now needs pid reuse AND the same 10ms start tick, on top of the lock's existing fm-home and receiver-path bindings. That residual is negligible next to a false FAILED that is certain, permanent, and points at killing the receiver. This weakening is confined to the receiver arm; the watcher, delivery listener, and away-mode daemon locks still compare full fm_pid_identity.
Failure mode required by the captain: if the incarnation genuinely cannot be established, the wrapper refuses and says so ('telegram receiver: FAILED - could not identify receiver process') and leaves no lock behind - never a silent pass, and never a FAILED against a healthy receiver. That refusal is covered by a new test.
The captain also required a test that fails against the old code and exercises the exec transition rather than asserting on an already-settled process, and warned that this home is not currently affected so 'it works here' is not evidence. The race was therefore reproduced deliberately first: a receiver reached through a slow interpreter hop showed recorded cmdline-hex 'bash ' against live 'bash ' with an identical starttime, and the re-arm printed the exact reported FAILED line. The tests use that same fixture - tests/fm-tg-recv-arm.test.sh asserts the receiver actually left its interpreter image before re-arming three times plus once against a legacy-format record, and tests/fm-watcher-lock.test.sh drives a real gated exec and asserts the incarnation is stable across it while the identity is not. Both were verified to fail against the pre-fix code with the reported symptom.
Deliberate scope decisions a reviewer would not see in the diff: no docs/ change, because no doc referenced the lock's identity file (checked) and the firstmate coding guidelines route exact mechanics to script headers, which is where the explanation went; no AGENTS.md change, for the same reason. The comment on fm_pid_identity is load-bearing - it is the durable guard against a future caller repeating this - and is not decoration.
Verification run before committing: bin/fm-lint.sh clean on all four changed files, and nine suites green (fm-tg-recv-arm, fm-watcher-lock, fm-delivery, fm-daemon, fm-guard-stale-banner, fm-turnend-guard, fm-continuity-pretool-check, fm-pr-check-security, fm-afk-launch). One pre-existing failure in tests/fm-watcher-lock.test.sh (test_guard_warnings) also fails on the base commit and is unrelated to this change.
What Changed
Risk Assessment
✅ Low: The change durably closes the exec-transition race by matching the receiver on fork-stable incarnation, preserves legacy lock compatibility, confines the accepted collision trade-off to this receiver lock, and adds behavioral regression coverage for the required failure paths.
Testing
The supplied baseline verification was reviewed, both focused automated suites passed, and a manual end-user CLI run demonstrated that a receiver survives its exec transition, re-arms attach to the same process without duplication, and cleanup removes the lock; no actionable issues were found.
Evidence: Receiver exec-transition and re-arm transcript
--- initial arm --- telegram receiver: started pid=2269914 --- re-arm after final exec --- telegram receiver: attached pid=2269914 --- persisted receiver starts --- 1 --- lock after receiver exit --- absentEvidence: Process-incarnation and watcher-lock test log
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspectedgit diff --unified=40 8725f8124d786b1f4cdbf54ab25153d40d8bb412 b801beeadcf8d833a31013aaecaee8e2ca9bc213 -- bin/fm-wake-lib.sh bin/fm-tg-recv-arm.sh tests/fm-tg-recv-arm.test.sh tests/fm-watcher-lock.test.shtests/fm-tg-recv-arm.test.sh— exercised delayed shebang exec, three healthy re-arms, legacy full-identity lock compatibility, duplicate prevention, identification failure, and lock cleanuptests/fm-watcher-lock.test.sh— exercised a gated real exec, stable incarnation versus changing identity, legacy-record matching, PID reuse, and existing lock behaviorManual delayed-interpreter receiver run captured inreceiver-exec-rearm-transcript.txt— observedstarted, thenattachedto the same PID, exactly one receiver start, and no lock after exitReviewed the author-supplied pre-commit verification context; lint was not repeated because this test phase forbids linters✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.