PR #9 - jainal09/drill

Stacked on #8. Two separate bugs, neither of which the suite could see until it
could run on Linux at all.

1. `t` stopped whatever it found, not whatever was a timer

pgrep -f "$TAG" matches the tag anywhere in a command line — so it also
matched every process that merely mentions it: a shell started as
sh -c '... DRILL_TIMER_TAG=drill-timer ...', an editor opened on a file with
that name, a grep for it. "Stop the running timer" is the first thing t does,
so t stopped those instead.

Reproduced from a script file, so no ancestor held the tag:

=== who does pgrep -f see, from inside a shell whose argv holds the tag? ===
my pid: 2086961
pgrep -f sees:
    2086961 bash -c  . '/home/jainal09/drill/drill.sh' ... DRILL_TIMER_TAG='drill-ti

=== now the real thing: t ===
Terminated
outer saw exit=143

The shell was found and killed by its own call to t. That is also exactly
what the test harness does, which is why 30 of 36 timer cases failed here
before any of this — the suite was killing itself, on unmodified main.

The fix: ps -o comm= is the process's executable, not its arguments, so it
cannot be fooled by a command line that quotes the tag.

pid 2098008  comm=zsh          <- the shell that mentions the tag
pid 2098013  comm=python3      <- the actual timer

POSIX, and identical on macOS. The suite gets its own copy of that filter
rather than calling _drill_timer_pids, so it is not checking drill.sh against
itself.

2. The sound and the notification did nothing — and so did the bell

run() returned True when a process merely launched. Measured on Ubuntu
22.04 under WSLg, both of these are installed and both exit non-zero:

command	status	exit
`pw-play …/complete.oga`	installed	1 — `no node available`
`notify-send drill test`	installed	1 — `org.freedesktop.Notifications was not provided by any .service files`

Counted as success, they consumed the chain and left the \a fallback
unreachable: no sound, no notification, and no bell — the one outcome a timer
must never have.

So: believe the exit code, add a timeout=10 so a notify-send waiting on a
dead bus cannot hang the process for the session, and give each chain the legs it
needs past macOS.

sound chain:
   afplay       which=False run=False     <- mac leg, correctly skipped
   paplay       which=True  run=True
   pw-play      which=True  run=False     <- the case that used to count as success
   play()  -> True in 2.0s
notify chain:
   notify-send  run = False
   notify() -> True in 0.4s               <- Windows toast

The mac leg stays first and unchanged in both chains, and afplay and
osascript exit 0 on a Mac, so nothing after them is ever reached there.

One judgement call worth flagging: I first put alarm-clock-elapsed.oga at the
head of the sound list, then measured it — it is an 8-second looping alarm,
and this plays three times. complete.oga stays first, as it always was.

Suite

556/556, exit 0 — the first time the gate has passed on WSL.

	timer	total
unmodified `main`	6/36	526
after checkpoint 2	6/36	526
this branch	36/36	556

Not verified here

The toast returns exit 0, which means Windows accepted it. Whether it visibly
appears also depends on Focus Assist and notification settings — worth a glance
on your side.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WveBqAyhoM49p9gWfwBDBx

Summary by CodeRabbit

Bug Fixes
- Improved timer reliability by detecting failed commands and handling timeouts correctly.
- Enhanced sound and desktop notification support across platforms, with additional fallback options.
- Prevented timer controls from affecting unrelated processes.
- Improved timer listing behavior when process discovery is unavailable.
Tests
- Expanded timer coverage to verify process handling, restarts, notifications, sound playback, and all 36 timer scenarios.
Documentation
- Updated checkpoint documentation to reflect completed validation and reliability improvements.

Greptile Summary

The PR improves cross-platform timer process discovery and makes sound and notification fallbacks depend on command exit status.

Adds Linux and Windows sound and notification fallbacks with command timeouts.
Narrows timer discovery using a fixed marker and an end-anchored configurable tag.
Expands timer tests and updates the WSL migration checklist.

Confidence Score: 4/5

The PR is not yet safe to merge because timer discovery can still classify an unrelated process carrying both public identifiers as a drill timer and terminate it.

The reply on the earlier Python-filter thread correctly addresses Python-only false positives, but the separate public-marker thread remains unresolved: _drill_timer_pids trusts two reproducible command-line strings as identity, and _drill_timer_stop sends every resulting PID directly to kill.

Files Needing Attention: drill.sh

Important Files Changed

Filename	Overview
drill.sh	Adds marker-based timer discovery, dependency checks, exit-aware subprocess handling, and broader platform notification and audio fallbacks.
tests/suite_timer.sh	Reworks timer-process checks to avoid matching the test shell and extends coverage of timer lifecycle behavior.
TODO.md	Marks earlier WSL checkpoints complete and documents the timer checkpoint’s expanded scope.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Timer command] --> B[Find marker-matching PIDs]
  B --> C[Find PIDs ending with timer tag]
  C --> D[Intersect PID sets]
  D --> E[Stop matching timers]
  A --> F[Launch Python countdown]
  F --> G[Notify through platform chain]
  G --> H[Play sound through platform chain]
  H --> I[Terminal bell fallback]

_{Reviews (11): Last reviewed commit: "Identify the timer by its own source, no..." | Re-trigger Greptile}