Skip to content

fix(melpa): cancel GNU's minibuffer background-update timer in the ffip world - #430

Merged
eval-exec merged 1 commit into
mainfrom
fix/ffip-timer-cleanup
Sep 22, 2026
Merged

eval-exec merged 1 commit into
mainfrom
fix/ffip-timer-cleanup

Conversation

@eval-exec

Copy link
Copy Markdown
Owner

The intermittent FFIP red: real-project-completion-filters-and-visits-unicode-file expected a GNU value but got the world's teardown error refusing to cancel unowned timer: function=completions--background-update args=(t #<buffer *Minibuf-1*>).

  • GNU's completion machinery schedules an idle completions--background-update timer with (FORCE-EAGER-UPDATE BUFFER) args while the minibuffer is active (minibuffer.el completions--start-background-update).
  • Batch input can end the workflow before an idle moment consumes it, so the world's cleanup finds it pending.
  • The existing exemption only matched a one-arg (t) shape, which this GNU version never produces.
  • Fix: match the real (FORCE BUFFER) shape and reset completions--background-update-timer so the next shared-process workflow sees no stale timer variable.

Verified: find_file_in_project_package_batch passes 3/3 consecutive runs. Melpa sweep locally: 940/942 (remaining: ac_helm #406, helm_lsp).

…ip world

GNU's completion machinery schedules an idle
`completions--background-update' timer with (FORCE-EAGER-UPDATE BUFFER)
arguments while the minibuffer is active (minibuffer.el
`completions--start-background-update').  Batch input can end the
workflow before an idle moment consumes it, so the timer is still
pending when the world tears down.  The world's existing exemption only
matched a one-argument (t) shape, which this GNU version never
produces, so cleanup raised

    refusing to cancel unowned timer: function=completions--background-update
    args=(t " *Minibuf-1*")

and the GNU side of `real-project-completion-filters-and-visits-unicode-file'
failed intermittently.  Broaden the exemption to the real (FORCE BUFFER)
argument shape and reset `completions--background-update-timer' so the
next shared-process workflow sees no stale timer variable.

Verified: find_file_in_project_package_batch passes three consecutive
runs.
Copilot AI balanced review requested due to automatic review settings September 22, 2026 13:35
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 47538731-8a5d-4d3a-9149-89b84d49358c

📥 Commits

Reviewing files that changed from the base of the PR and between 125215f and 738f66b.

📒 Files selected for processing (1)
  • crates/neomacs-melpa-tests/src/parity_tests/find_file_in_project/mod.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The shared-batch cleanup now recognizes GNU completion background-update timers with supported arguments. It clears completions--background-update-timer before cancellation. The existing timer ownership and cancellation calls were reformatted without behavioral changes.

Changes

Completion timer cleanup

Layer / File(s) Summary
Background timer ownership and cancellation
crates/neomacs-melpa-tests/src/parity_tests/find_file_in_project/mod.rs
The cleanup recognizes completions--background-update timers with (t) arguments or a truthy first argument followed by a buffer. It clears the associated timer variable before cancellation. The existing bookkeeping calls were reformatted.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 738f6

The cleanup prevents stale completion timers from affecting subsequent shared workflows, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: canceling GNU Emacs's minibuffer background-update timer in the FFIP test world.
Description check ✅ Passed The description directly explains the intermittent FFIP failure, the timer argument mismatch, the cleanup fix, and verification results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The setq that clears the global completions--background-update-timer executes before the argument-shape check, so an unrecognized-shape timer gets its owning variable nulled while the still-live timer is left uncancelled.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

This PR fixes an intermittent parity-test failure in the find-file-in-project (ffip) MELPA test world. During cleanup, the shared-process test harness cancels leftover timers but errors out on any timer it doesn't recognize as "owned". GNU's minibuffer completion machinery schedules an idle completions--background-update timer with (FORCE BUFFER) arguments while the minibuffer is active; batch input can end the workflow before that timer fires, so cleanup encountered it as pending. The prior exemption only matched a one-arg (t) shape that this GNU version never produces, causing the world's teardown to raise refusing to cancel unowned timer.

Changes:

  • Broaden the completions--background-update timer exemption to also match the real (FORCE BUFFER) argument shape.
  • Reset completions--background-update-timer to nil so the next shared-process workflow sees no stale timer variable.
  • Re-indent the trailing cl-pushnew/cancel-timer forms to align as siblings of the unless.
File Description
crates/​neomacs-melpa-tests/​src/​parity_tests/​find_file_in_project/​mod.rs Extends the elisp cleanup prelude's timer exemption to the (FORCE BUFFER) shape and clears the background-update timer variable

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +723 to +729
(progn
(setq completions--background-update-timer nil)
t)
(or (equal arguments '(t))
(and (consp arguments)
(car arguments)
(bufferp (car (cdr arguments))))))
@eval-exec
eval-exec merged commit be88579 into main Sep 22, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants