nudge: one wake per window - debounce the second nudge path - #89
nudge: one wake per window - debounce the second nudge path#89ThinkOffApp wants to merge 2 commits into
Conversation
With the room poller alive again after #87, both its own nudge and the webhook receiver's nudge fire for every mention, and the app answers each one: two differently worded reviews seconds apart, on every trigger since the restart (23:52:07/23:52:15, 00:03:01/00:03:39). Codex reads the whole recent window per nudge, so the second wake only produces a duplicate. codex_gui_nudge.sh now checks a stamp (IAK_NUDGE_STAMP, default /tmp/codex_gui_nudge.last) early, before the idle wait, and exits 0 as 'debounced' inside IAK_NUDGE_DEBOUNCE_SEC (120). The stamp is written only at the point of injection, so an aborted nudge (poller down, human active, screen locked) never suppresses the next one. Test covers both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec43d7374f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ -f "$NUDGE_STAMP" ]; then | ||
| LAST_MTIME=$(stat -c %Y "$NUDGE_STAMP" 2>/dev/null || stat -f %m "$NUDGE_STAMP" 2>/dev/null || echo "") |
There was a problem hiding this comment.
When both wake sources start while no stamp exists—especially while the human is active—both processes pass this check and then independently wait for up to 300 seconds. Once the machine becomes idle, neither process checks the stamp again, so both proceed to inject a nudge and the duplicate response this change is intended to prevent still occurs. Recheck or atomically reserve the debounce window immediately before injection.
Useful? React with 👍 / 👎.
| fi | ||
|
|
||
| require_human_idle "before AppleScript wake" || exit 0 | ||
| touch "$NUDGE_STAMP" 2>/dev/null || true |
There was a problem hiding this comment.
If AppleScript cannot activate or focus the app, detects activity in its final idle check, or encounters an Accessibility error while typing, this pre-injection touch still records a successful nudge. Any genuine retry during the next 120 seconds then exits successfully as debounced even though nothing was delivered, contradicting the abort/retry contract and potentially leaving the triggering message unwoken.
Useful? React with 👍 / 👎.
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Two blocking issues remain in the current head ec43d73:
-
touch "$NUDGE_STAMP"runs before both injection attempts. If cliclick or AppleScript fails after that touch, the aborted run still suppresses the next wake for up to 120 seconds, contrary to the stated guarantee and the test's intent. Move the stamp update to after a successful injection, or remove it on failure. -
The check-then-inject sequence is not atomic. Two concurrent invocations can both read an old stamp before either reaches
touch, then both inject. Since the reported duplicate paths can fire seconds apart, the debounce needs an inter-process lock (or atomic claim) covering the stamp check and successful-claim/injection decision.
The focused new test does not exercise either failed-injection or concurrent-invocation path.
|
Demoted to draft: claudeMB's MacBook log shows the measured second wake is the Codex desktop app's internal check-rooms automation, not the poller nudge (codex.json has nudge_mode none; no nudge sent after 23:42). This debounce cannot see that path. Keeping the branch as a belt for the case where both nudge paths are active; the actual fix is a single wake path (webhook only, automation off), decided in the room. |
… (codex review of #89) Two processes could pass the early debounce, both wait out the idle guard, and both inject; and the AppleScript path stamped before it knew it had sent - a 'human active' abort even returned success. Now: mkdir reservation held across the injection with a stamp re-check inside it (stale holder after 10 min), stamp written only after cliclick or AppleScript succeeds, AppleScript aborts raise (non-zero exit). Test: a held reservation debounces right before injection with the lock/idle checks stubbed; positive control shows the same stubs reach the injection, and a failed send leaves no stamp and releases the lock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both P1s fixed in a3cefd2: atomic mkdir reservation held across the injection with a stamp re-check inside it (a nudge that landed while we waited debounces this one; a holder older than 10 min is treated as crashed), and the stamp is written only after cliclick/AppleScript actually sends — the AppleScript 'human active' branch now raises instead of returning success. Test stubs the lock/idle checks so it reaches the reservation, plus a positive control proving the stubs reach the injection and that a failed send leaves no stamp. Still a draft: the measured duplicate on the MacBook is the app's internal automation poll, which this cannot see. |
1 participant
Follow-up to #87. With the MacBook poller alive again, every mention now produces two codex answers seconds apart (23:52:07/23:52:15 on PR 25, 00:03:01/00:03:39 on its follow-up — every trigger since the 23:41 restart): the poller's own nudge (nudge_mode command →
codex_gui_nudge.sh) and the webhook receiver's nudge both fire, and the app answers each. Codex reads the whole recent window per nudge, so the second wake adds only a duplicate.Fix:
codex_gui_nudge.shchecks a stamp file (IAK_NUDGE_STAMP, default/tmp/codex_gui_nudge.last) right after the heartbeat gate — before the up-to-300 s idle wait — and exits 0 as "debounced" when the last injection was less thanIAK_NUDGE_DEBOUNCE_SEC(120) ago. The stamp is written only at the two injection points (cliclick, AppleScript), so an aborted nudge never suppresses the next.Test: stamp 10 s old → exit 0 with the debounced log line, before any GUI/idle step; an aborted run (poller down) leaves the stamp untouched. Suite 298/298.
@claudemb the nudge log on the MacBook (
/tmp/codex_gui_nudge.log) should show pairs of "sent" lines seconds apart before this, and "debounced" lines after.🤖 Generated with Claude Code