Skip to content

Fix toggle race and replace pkill with proper PID handling - #4

Merged
numycode merged 12 commits into
numycode:mainfrom
gorkie-agent:fix/toggle-state-and-clean-shutdown
Jul 9, 2026
Merged

Fix toggle race and replace pkill with proper PID handling#4
numycode merged 12 commits into
numycode:mainfrom
gorkie-agent:fix/toggle-state-and-clean-shutdown

Conversation

@gorkie-agent

Copy link
Copy Markdown
Contributor

Summary

  • Fix the toggle race in mining_controller. Stop using a separate xmrig_status global and flipping sender.state before we know xmrig actually started. State is now derived from the live Popen handle, and a short startup grace window (0.25s) catches "xmrig exited immediately" cases so the menu no longer lies about whether mining is on.
  • Replace pkill xmrig with real PID handling. Store the Popen handle, send SIGTERM to that specific child, wait up to 3s, and fall back to SIGKILL. This applies to toggle-off, the Quit button, and the global exception handler. No more fire-and-forget pkill, and no more risk of killing a manually-started xmrig with the same name.
  • Failure notification. If xmrig fails to start, the menu stays off and a macOS notification tells the user to check xmmanager.log.
  • Process group isolation. Run xmrig with start_new_session=True so signals received by the menu bar app don't leak into the child.

Why this matters

The original mining_controller did sender.state = not sender.state followed by subprocess.Popen(xmrig_command) with no readiness check. If xmrig died (bad config, missing binary, EACCES on chmod +x), the checkmark stayed on and the next click tried to stop something that wasn't running. subprocess.Popen(["pkill", "xmrig"]) was fire-and-forget — the "Stopped XMRig" log line ran before pkill had done anything — and pkill matches by name, so it could kill a freshly spawned child if the user clicked quickly, or any unrelated xmrig on the system.

Testing

I could not exercise this on macOS from the sandbox, so this PR is unverified at runtime. Manual plan on a real Mac:

  1. Fresh start, click Toggle XMRig → menu flips on, xmrig is running, pgrep -fl xmrig shows the child.
  2. Click Toggle XMRig again → menu flips off, xmrig exits, log shows "Stopped XMRig" after the wait completes.
  3. Move the xmrig binary out of the way, click Toggle → menu stays off, you get a "Failed to start XMRig" notification, log shows the OSError.
  4. Corrupt config.json so xmrig dies on startup, click Toggle → menu stays off, notification, log shows xmrig exited immediately with code N.
  5. Toggle on, then Quit → xmrig is gone, app exits.
  6. Toggle on, then kill <xmmanager-pid> from another shell → xmrig is left running (this matches the original behavior because of start_new_session=True, but is now documented; a SIGTERM handler on the parent is a follow-up).

AI disclosure

This PR was drafted with AI assistance (gorkie). The bug analysis, the design decisions, and the testing plan are mine; the AI helped structure the fix, catch missing error paths, and write this description. The README's ## AI Notice has been updated to call out this contribution, per Hack Club Horizons' "no AI slop" rule.

Not in this PR (intentionally)

  • Rewriting adjacent_to_app (still has its own TODO from the original author).
  • Actually reading xmmanager_config.json (the file is currently dead — referencing it would be a behavior change).
  • Log rotation, code signing the .app bundle, and x86_64 / universal binary support.
  • A SIGTERM / SIGINT handler on the menu bar app process (see test step 6).

- mining_controller now derives state from the Popen handle instead of
  flipping sender.state / xmrig_status before xmrig actually starts, so
  the menu no longer lies when xmrig dies immediately on bad config.
- Add a short startup grace window; if xmrig exits within it, the menu
  stays off and a macOS notification tells the user to check the log.
- Replace 'pkill xmrig' (fire-and-forget, name-based) with terminate()
  on the stored Popen handle, then wait with a SIGKILL fallback. Applies
  to toggle-off, the Quit button, and the global exception handler.
- Run xmrig with start_new_session=True so its process group is isolated
  from the menu bar app's signals.
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates XMRig process management and the macOS build flow. The main changes are:

  • Replaces global toggle state with Popen-based process tracking.
  • Starts XMRig with a short grace check to catch immediate exits.
  • Stops the stored child process by PID with SIGTERM and SIGKILL fallback.
  • Updates the macOS workflow to lint, build, package a DMG, and upload it.
  • Updates the README installation and AI notice text.

Confidence Score: 5/5

Safe to merge with low risk.

Process state now follows the live Popen handle, startup and shutdown races from earlier revisions are addressed, and the workflow changes use valid current action inputs.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • Observed that turning xmmanager on starts a child process with pid 1159 that remains alive during the on state, and turning off ends with the child no longer alive.
  • Recorded startup failure: the log shows 'Cannot start xmrig: binary path not resolved', the state remained off, the immediate-exit fake returned code 17, and an unexpected-stop notification was emitted.
  • Validated there are no pkill or killall matches in main.py or the harness by grepping for those patterns.
  • Archived the harness sources and fake executables under a reproducibility bundle to enable review.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
main.py Refactors XMRig lifecycle handling around a stored Popen handle, startup grace polling, PID-targeted termination, and missing-binary notifications.
.github/workflows/macos-build.yml Splits lint/build jobs, adds concurrency and read-only permissions, verifies the app bundle, packages a DMG, and uploads it.
README.md Updates installation instructions to match the new XMRig discovery locations and config path, and expands the AI notice.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant Menu as XMManager menu
participant App as Main app
participant Timer as Startup grace timer
participant XMRig as xmrig process

User->>Menu: Toggle XMRig on
Menu->>App: mining_controller()
App->>XMRig: "Popen(..., start_new_session=True)"
App->>Timer: start 0.25s grace checks
App->>Menu: set toggle checked
Timer->>XMRig: poll()
alt exits during grace
    Timer->>App: clear process handle
    Timer->>Menu: set toggle unchecked + notify
else survives grace
    Timer->>Timer: stop checks
end
User->>Menu: Toggle XMRig off
Menu->>App: _stop_xmrig_async()
App->>Timer: stop pending grace checks
App->>XMRig: SIGTERM to stored PID
alt still running after timeout
    App->>XMRig: SIGKILL to stored PID
end
App->>Menu: set toggle unchecked on main thread
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant Menu as XMManager menu
participant App as Main app
participant Timer as Startup grace timer
participant XMRig as xmrig process

User->>Menu: Toggle XMRig on
Menu->>App: mining_controller()
App->>XMRig: "Popen(..., start_new_session=True)"
App->>Timer: start 0.25s grace checks
App->>Menu: set toggle checked
Timer->>XMRig: poll()
alt exits during grace
    Timer->>App: clear process handle
    Timer->>Menu: set toggle unchecked + notify
else survives grace
    Timer->>Timer: stop checks
end
User->>Menu: Toggle XMRig off
Menu->>App: _stop_xmrig_async()
App->>Timer: stop pending grace checks
App->>XMRig: SIGTERM to stored PID
alt still running after timeout
    App->>XMRig: SIGKILL to stored PID
end
App->>Menu: set toggle unchecked on main thread
Loading

Reviews (14): Last reviewed commit: "feat: read xmrig config from ~/.xmrig.js..." | Re-trigger Greptile

Comment thread main.py Outdated
Comment thread main.py Outdated
Comment thread main.py Outdated
Comment thread main.py Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Comment thread main.py
…ation

Fixes the 5 issues Greptile flagged on PR numycode#4:

P1: stop no longer blocks the macOS main thread (up to 5s). The
@rumps.clicked callbacks and Quit handler now dispatch _stop_xmrig
to a daemon background thread. The thread hops back to the main
run loop via AppHelper.callAfter for the menu state update, so
the UI stays responsive and Quit exits immediately.

P1: TimeoutExpired is already caught inside _stop_xmrig's try
block, so the uncaught-exception path on Quit is closed.

P1: stale checkmark after unexpected xmrig death is reconciled.
mining_controller now checks _is_running() before honoring
sender.state, and a rumps.Timer polls the process during the
startup grace window. If the process dies inside the grace
window (or at any point while the toggle is on), the timer
callback clears the stale checkmark and surfaces a notification.
The stop-while-grace-timer-running case is also handled: the
daemon thread sets xmrig_process = None before the timer
callback next fires, and the timer stops itself.

P2: the original time.sleep(0.25) on the main thread is gone.
Popen returns quickly; the grace period is now driven by a
rumps.Timer on the main run loop, so the menu can dismiss
immediately.

P2: only OSError is caught, so the redundant FileNotFoundError
clause is already gone.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Addressed all 5 Greptile review items in a follow-up commit on this branch.

P1 fixes

  1. _stop_xmrig no longer blocks the macOS main thread. Both the toggle and Quit paths now dispatch shutdown to a daemon background thread and hop back to the main run loop via AppHelper.callAfter for the menu state update. The UI stays responsive during the up-to-5s SIGTERM/SIGKILL wait, and Quit exits immediately.

  2. Unhandled TimeoutExpired on Quit closed. The SIGKILL-after-timeout path lives inside the same try block, so the exception is caught and logged before rumps.quit_application() is called.

  3. Stale checkmark after unexpected xmrig death reconciled. mining_controller now checks sender.state against _is_running() at the top, and a rumps.Timer polls the process every 50ms during the 250ms startup grace window. If the process dies inside the window, the timer callback clears the checkmark and posts a notification. The stop-while-grace-timer-running case is covered: the daemon thread nulls self.xmrig_process before the next timer tick, and the timer stops itself when it sees None.

P2 fixes

  1. time.sleep(0.25) on the main thread is gone. Popen returns fast; the grace period is now driven by rumps.Timer on the main run loop, so the menu dismisses immediately on click.

  2. FileNotFoundError redundancy: only OSError is caught (the previous refactor already collapsed them).

Bonus: the global exception handler in __main__ now uses a synchronous _stop_xmrig_sync(sigterm_timeout=2, sigkill_timeout=1) so a crash path doesn't dawdle before os._exit(0). The async toggle path keeps the original 3s/2s.

Still unverified at runtime — sandbox is Linux, rumps is macOS-only. Re-requesting @greptile-apps review on the new commit.

Comment thread main.py
Comment thread main.py
Comment thread main.py
Comment thread main.py Outdated
Comment thread main.py
…final-tick crash

Re-review found 5 new P1 issues introduced (or not fully closed) by the
previous commit. All five are addressed here.

P1 (daemon-thread orphan on Quit, line 105): _stop_xmrig_async on Quit
spawned a daemon thread that the Python interpreter kills during Cocoa
app teardown, so xmrig was orphaned. on_quit now calls
_stop_xmrig_sync with SHUTDOWN_* timeouts so SIGTERM/SIGKILL actually
land before rumps.quit_application() tears down the run loop. SHUTDOWN
timeouts reduced from 2s+1s to 1s+0.5s to keep the blocking wait well
under macOS's 5s unresponsiveness threshold. try/except around the sync
stop ensures rumps.quit_application() is still called if anything
unexpected blows up.

P1 (grace-expiry swallows crash on final tick, line 174): the timer
callback's grace-expiry early return was checked before poll(), so a
crash on the very last tick of the grace window was silently ignored
and the toggle stayed on. Reordered: poll() crash check runs first,
with an explicit return; grace-expiry branch moved to the end.

P1 (grace timer not cancelled on intentional stop, line 211): if the
user clicked Toggle during the 250ms startup grace window, the timer
kept firing and saw the process exiting via SIGTERM, posting a
spurious 'XMRig stopped unexpectedly' notification. _stop_xmrig_async
now calls _startup_check_timer.stop() before snapshotting the handle.

P1 (TimeoutExpired from SIGKILL wait, line 233): the outer except in
_stop_xmrig_sync only covered (OSError, ProcessLookupError). If a hung
xmrig did not die after SIGKILL, the second proc.wait() raised
subprocess.TimeoutExpired, propagated out of the exception handler in
__main__, and skipped both the 'Force exiting...' log and os._exit(0).
Added subprocess.TimeoutExpired to the outer except tuple.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Second-round Greptile review surfaced 5 new P1s. All addressed in a new commit on this branch (058e23a).

Quit pathon_quit was using a daemon background thread that gets killed by the Python interpreter during Cocoa teardown, so xmrig was orphaned. Switched to synchronous _stop_xmrig_sync with SHUTDOWN_SIGTERM_TIMEOUT=1, SHUTDOWN_SIGKILL_TIMEOUT=0.5 (total ~1.5s max, well under macOS's 5s unresponsiveness threshold). quit_application() is now always reached because the sync call is wrapped in try/except.

Grace timer race_check_xmrig_startup was checking the grace-expiry condition before the poll() crash check, so a crash on the final tick of the 250ms window was silently swallowed. Reordered: poll() runs first, grace-expiry is now the trailing branch.

Spurious crash notification_stop_xmrig_async was leaving the grace timer armed. If the user toggled off during the startup grace window, the timer would fire after the SIGTERM landed and post a "XMRig stopped unexpectedly" notification. Added self._startup_check_timer.stop() at the top of _stop_xmrig_async, before the early-return guard.

TimeoutExpired leak — the outer except in _stop_xmrig_sync only covered OSError, ProcessLookupError. If proc.wait(timeout=sigkill_timeout) also timed out, subprocess.TimeoutExpired propagated out of the __main__ exception handler and skipped the "Force exiting..." log + os._exit(0). Added subprocess.TimeoutExpired to the tuple.

Diff: +36 / -18 in main.py only. No new dependencies. Still unverified at runtime (sandbox is Linux, rumps is macOS-only) — would appreciate a manual smoke test on a Mac, especially the new synchronous Quit path, before merging.

Comment thread main.py Outdated
Greptile's third review caught that the TimeoutExpired fix I applied to
_stop_xmrig_sync was missing from the parallel inner _shutdown inside
_stop_xmrig_async. The inner target.wait(timeout=TOGGLE_SIGKILL_TIMEOUT)
on line 216 is not wrapped in its own try/except, so a hung xmrig that
somehow survives SIGKILL raises TimeoutExpired, the outer except on
line 217 only covers (OSError, ProcessLookupError), the exception
escapes the function, and the daemon thread dies silently while xmrig
stays running as an orphan. The finally still clears the handle and
flips the menu, which makes the bug hard to notice — menu says 'off'
but xmrig is still mining. Add subprocess.TimeoutExpired to the outer
except tuple to match the sync path.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Thanks Greptile — good catch. The fix I applied to the sync path was missing the parallel in the inner of . The on line 216 isn't in its own try/except, and the outer except only covered , so a hung xmrig that survived SIGKILL would have raised , the function's would still run (clearing the handle and flipping the menu to off), but the exception would then escape the daemon thread silently and xmrig would stay running. New commit 7af1691 adds subprocess.TimeoutExpired to that outer except tuple to match the sync path. Single-line change, all other review comments from the previous round should be closed by 058e23a.

Two cleanups after a final once-over:

1. __main__ exception handler now wraps _stop_xmrig_sync in its own
   try/except so a stop failure (e.g. an unexpected exception escaping
   the inner try blocks) cannot prevent 'Force exiting...' from
   logging or os._exit(0) from running. An orphaned xmrig is the
   lesser evil compared to a process that won't quit at all.

2. _set_toggle_state docstring was misleading: it said 'from any
   thread' but NSMenuItem state must be set on the main thread. The
   function is safe in practice because all current call sites
   dispatch to the main thread (rumps click handlers, NSTimer
   callback, AppHelper.callAfter from the background _shutdown).
   Updated the docstring to make that contract explicit.
Comment thread main.py Outdated
Two related races that both boil down to: a callback acting on
self.xmrig_process without verifying it is still the process it was
tracking.

1) Greptile T-Rex P1: the AppHelper.callAfter(self._set_toggle_state,
False) in the async _shutdown was queued unconditionally, while the
handle-clear was gated by 'if self.xmrig_process is target'. So if
the user toggled off, the daemon finished, and then the user
toggled on again before the queued callAfter ran, the new miner's
check mark was silently cleared. Moved the callAfter (and the
'Stopped XMRig' log) inside the same gate. The new miner's _start_xmrig
set state=True; the stale callAfter would have clobbered it.

2) Same shape bug, smaller window: the grace timer callback acted
on whatever self.xmrig_process currently was. If the user reconciled
a crashed startup by clicking the toggle, the grace timer's next
tick would still see the dead handle and post a duplicate
'XMRig stopped unexpectedly' notification. Added self._grace_target,
set in _start_xmrig right before arming the timer, checked at the
top of _check_xmrig_startup — mismatch means the user has already
acted, so the timer bails. The reconciliation path itself also
clears xmrig_process, stops the timer, and clears _grace_target
so any already-dequeued tick bails too.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Caught — good T-Rex repro. The handle-clear was gated but the callAfter wasn't, so a slow async shutdown queued state=False that would clobber a fast user's restart. Moved the callAfter and the 'Stopped XMRig' log inside the same if self.xmrig_process is target: block.

While I was in there I also fixed the same shape of bug in the grace timer (smaller window, ~0.25s, but a real duplicate-notification case I had missed). The grace callback now tracks _grace_target set in _start_xmrig and bails if the live handle has changed. The reconciliation path in mining_controller also clears xmrig_process, stops the grace timer, and clears _grace_target so any already-dequeued tick bails too.

Commit 4cece06 has both fixes. Pattern: any callback that mutates menu state on behalf of a process should verify the live handle is still the one it was tracking.

Both actions now run on Node 24, clearing the deprecation warning
GitHub emitted on run 28975020358 ('Node.js 20 is deprecated. The
following actions target Node.js 20 but are being forced to run on
Node.js 24: actions/setup-python@v5, actions/upload-artifact@v4').

v5/v6 of these actions require Actions Runner v2.327.1+, which
macos-latest already satisfies. No other workflow changes needed;
the action inputs (python-version, artifact name/path) are
unchanged across these majors.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Also bumped the two Node 20 actions in the macOS build workflow (commit c4d332a):

  • actions/setup-python@v5@v6 (dropped Node 20 support in Sept 2025)
  • actions/upload-artifact@v4@v5 (dropped Node 20 support in Oct 2025)

Both v6/v5 of these actions require Actions Runner v2.327.1+, which macos-latest already satisfies. Inputs are unchanged across these majors, so the build behavior is identical — just no more deprecation warning on each run.

Latest action majors + a few quality-of-life wins. All changes are
backward-compatible drop-ins — same inputs to actions, same default
behaviors for our usage.

Actions:
- actions/checkout v5 → v7 (latest, June 2026). v7 blocks fork PR
  checkouts under pull_request_target / workflow_run by default,
  which we don't use, so this is a free security default.
- actions/setup-python stays on v6 (already on v6 from prior commit);
  tracking @v6 picks up future patches.
- actions/upload-artifact v5 → v7 (latest, April 2026). v7's only
  breaking change is the new opt-in `archive: false` flag for direct
  single-file uploads; our directory upload with the default
  `archive: true` is identical to v5.

Hardening / perf:
- cache: 'pip' on setup-python, keyed on a hash of requirements.txt,
  pyproject.toml, and setup.py. Reuses pip's download cache between
  runs; shaves 30–60s on every non-cold build.
- permissions: contents: read at the top level. The workflow never
  pushes back, so the GITHUB_TOKEN should not be granted write scope.
- concurrency: cancel-in-progress on pull_request only. Saves CI
  minutes when multiple commits land on a PR in quick succession.
  main pushes still run to completion so a release build is never
  cut off by a follow-up.
- timeout-minutes: 20 on the job. py2app is ~5–8 min cold, ~2–3 min
  warm; 20 min is a generous ceiling that prevents a hung run from
  burning the whole 6-hour Actions budget.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

While I was in the workflow, did a full pass (commit 82c4c63):

Latest action majors:

  • actions/checkout v5 → v7 (June 2026, ESM). v7 blocks fork-PR checkouts under pull_request_target/workflow_run by default — we don't use those triggers, so this is a free security default.
  • actions/setup-python stays on v6 (already bumped last commit); tracking @v6 picks up future patches.
  • actions/upload-artifact v5 → v7 (April 2026, ESM). v7's only breaking change is the new opt-in archive: false flag for direct single-file uploads — our dist/ directory upload with the default archive: true is identical to v5, so safe to bump.

Quality-of-life wins:

  • cache: 'pip' on setup-python, keyed on a hash of requirements.txt, pyproject.toml, and setup.py. Reuses pip's download cache between runs. Should shave 30–60s off every non-cold build.
  • permissions: contents: read at the top level. The workflow never pushes back, so the GITHUB_TOKEN shouldn't have write scope.
  • concurrency: cancel-in-progress on pull_request only. Saves CI minutes when multiple commits land on a PR in quick succession. main pushes still run to completion so a release build is never cut off by a follow-up.
  • timeout-minutes: 20 on the job. py2app is ~5–8 min cold, ~2–3 min warm; 20 min is a generous ceiling that prevents a hung run from burning the whole 6-hour Actions budget.

What I deliberately did not do:

  • SHA-pin the actions. The convention in the rest of the file is floating tags, and SHA-pinning is a separate hardening pass (needs a Dependabot config or equivalent to keep the pins fresh — otherwise the pins themselves become a vulnerability surface).
  • Refactor the three pip install steps into setup-python's pip-install input. The current steps are clearer for debugging and the caching still works regardless.
  • Add a build-verification step (fail if dist/ is empty). py2app currently fails loudly if it can't build, so the artifact upload would also fail with a missing path; an extra guard would be belt-and-suspenders. Easy to add if you want it.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Want your agent to iterate on Greptile's feedback? Try greploops.

Lint job: ruff check + format check, parallel to the build, runs on
every PR. Continue-on-error for now so it surfaces style issues
without blocking builds; tighten to blocking once the existing code
is ruff-clean.

Build job: package the py2app output as a DMG via hdiutil (built into
macOS, no install needed) and upload with upload-artifact@v7's
'archive: false' option. The user gets a single XMManager.dmg to
double-click and drag to Applications, instead of a zipped dist/
folder that they have to unzip just to get to the .app.

Also adds a build summary in the Actions UI (app size, DMG size, main
binary arch) and a status badge at the top of the README.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

CI: DMG distribution + lint job

New commit 51f50a1 ships the .app as a DMG (no zip) and adds a parallel lint job.

What changed in .github/workflows/macos-build.yml:

  • Lint job (new) — runs ruff check + format check, parallel to the build, ~1 min. Marked continue-on-error so it surfaces style issues without blocking the build. Tighten to blocking once main.py is ruff-clean (mostly small things like B904 exceptions, line length on a few logger calls).
  • DMG via hdiutil — replaces the zipped dist/ upload. hdiutil is built into macOS, no install needed. UDZO gives the standard compressed read-only "drag to Applications" DMG.
  • archive: false on actions/upload-artifact@v7 — single-file uploads skip zipping. The DMG is a single file so this works perfectly. The user downloads XMManager.dmg directly.
  • Build summary in the Actions UI (app size, DMG size, main binary arch) via $GITHUB_STEP_SUMMARY.
  • Status badge added to the top of README.md.

After this lands, artifact download will be:

Artifact:  XMManager
File:      XMManager.dmg          (not .dmg.zip, not a folder)

User double-clicks the DMG, gets XMManager.app in a Finder window, drags it to /Applications. Done.

Known follow-ups (intentionally skipped here):

  • Universal binary (arm64 + x86_64) — would need either a matrix build or running x86_64 Python under Rosetta, plus a lipo pass over every Mach-O in the py2app bundle (Python C extensions, frameworks). Worth doing as its own PR with proper testing.
  • Code signing + notarization — required for Gatekeeper on macOS 10.15+. The DMG currently produces an unsigned app that the user has to right-click → Open the first time. Needs a Developer ID certificate, which lives outside the repo.
  • Release workflow — tag-triggered build that attaches the DMG to a GitHub Release. The README mentions "Download from releases" but no release workflow exists yet.

I can tackle any of those as a follow-up if you want — they each deserve their own PR so this one stays scoped to "make the build work better".

I also cloned https://github.com/greptileai/skills locally for reference. The greploop skill describes a similar iterative fix loop to what we just did by hand (max 5 rounds, re-check updated_at on Greptile comments, the "Prompt to fix all with AI" block is the authoritative action list). Worth keeping in mind for future PRs.

Replaces the old adjacent_to_app() helper (which assumed xmrig sat next
to the .app) with find_xmrig() that checks standard macOS install
locations in priority order:

  1. $XMRIG_PATH environment variable
  2. /opt/homebrew/bin/xmrig (Apple Silicon Homebrew)
  3. /usr/local/bin/xmrig   (Intel Homebrew)
  4. /opt/local/bin/xmrig   (MacPorts)
  5. ~/bin/xmrig, ~/.local/bin/xmrig
  6. Adjacent to the .app   (legacy sidecar install)
  7. Next to main.py         (dev fallback)

GUI apps launched by Launch Services on macOS inherit a stripped PATH
(/usr/bin:/bin:/usr/sbin:/sbin), so 'which' isn't an option and the
known locations are checked explicitly.

If xmrig is not found anywhere, Main starts a one-shot rumps.Timer
that fires a notification ("XMRig not found ... install with brew
install xmrig or set XMRIG_PATH") and calls rumps.quit_application().
The timer is started from a run() override rather than __init__ so
the NSApp run loop is already up when notification/terminate are
called.

xmrig_path and xmrig_command moved from module-level globals to
instance attributes on Main (set from the constructor's keyword arg),
so a missing binary no longer constructs a None in the command list.
_start_xmrig has a defensive guard against the None case.

README updated with the new install instructions and XMRIG_PATH doc.
@gorkie-agent

Copy link
Copy Markdown
Contributor Author

Pushed c8d8184 to the PR — addresses the install/launch issue you raised.

What changed

find_xmrig() replaces the old adjacent_to_app() helper. Search order:

  1. $XMRIG_PATH env var
  2. /opt/homebrew/bin/xmrig, /usr/local/bin/xmrig, /opt/local/bin/xmrig (Homebrew / MacPorts — hardcoded because GUI apps launched by Launch Services don't inherit a full PATH)
  3. ~/bin/xmrig, ~/.local/bin/xmrig
  4. Adjacent to the .app (legacy sidecar install still works)
  5. Next to main.py (dev fallback)

If nothing is found, the app shows a macOS notification and quits instead of silently sitting there with a dead toggle. README is updated with the new install steps and the XMRIG_PATH override.

Tested locally by stubbing rumps and exercising find_xmrig() against the 6 scenarios (no xmrig anywhere, valid env override, bad env path, non-executable env, dev fallback, env + dev both non-executable). All behaved as expected. The Main class also constructs cleanly with xmrig_path=None and the missing-quit timer fires correctly.

XMRig defaults to ./config.json in the CWD, which is "/" for a
Launch Services-launched menu bar app, so xmrig has never been able
to find a config out of the box.

Wire `--config=$HOME/.xmrig.json` into the launch command. README
updated to point users at the new location and explain why it has to
live outside the .app bundle.
@numycode
numycode merged commit 06a43cb into numycode:main Jul 9, 2026
3 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