[GeiserX]

Problem

Before it starts its own fence helper, the worker force-removed any container named cashpilot-fence that carried the fence label. The Desktop app runs the same helper under the same name and label. On a host that runs both, that container can be the other side's helper, still running, while the other side waits for its result. Killing it makes the other side record a failed fence and refuse its deploy.

Fix

This is the same behaviour the Desktop app has in clearHelper:

• A helper with our label that is running, or was created or exited in the last 10 seconds, belongs to another process. The worker waits for it, checking every 250 ms, and never kills it.
• Only a leftover is removed: one idle for more than 10 seconds, or still there after the helper timeout plus that grace.
• A create that loses the race for the name (409 conflict) is tried once more.
• The worker's own helper is now created and started as two steps and removed through its own container object, never by name. That means a start that fails still gets removed, and a lookup by name after the run can no longer pick up the other side's new helper. A missing helper image is pulled once, which containers.run used to do implicitly.
• A container someone else named cashpilot-fence without our label is still left alone.

Tests

TestAnotherProcesssHelper in tests/test_lan_fence.py:

• A running, just-created or just-exited helper is waited for and not removed. With the poll interval stubbed on main, the running case fails with Expected 'remove' to not have been called. Called 2 times.
• A leftover is removed.
• A helper still there past the timeout is removed.
• Only our own helper is removed afterwards.
• A 409 on create is retried once.
• A start failure removes our container by its ID.
• A missing image is pulled once.

The whole file passes: 154 tests.

Desktop needs nothing: it already waits for the other side's helper (helperBusy/clearHelper in internal/runtime/lanfence.go).

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 19 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: GeiserX/CashPilot
• Branch: fix/fence-helper-wait
• File: .coderabbit.yaml
• Commit: dfb3350
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3e8f25bc-8f7d-470c-b5d0-1792b1f66f0b

Commits

Reviewing files that changed from the base of the PR and between 4f339d9 and dfb3350.

Files selected for processing: 2

• app/lan_fence.py
• tests/test_lan_fence.py

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

[codecov]

Codecov Report

❌ Patch coverage is 87.87879% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.93%. Comparing base (4f339d9) to head (dfb3350).

Files with missing lines	Patch %	Lines
app/lan_fence.py	87.87%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #636      +/-   ##
==========================================
- Coverage   93.97%   93.93%   -0.05%     
==========================================
  Files          74       74              
  Lines       15678    15736      +58     
==========================================
+ Hits        14733    14781      +48     
- Misses        945      955      +10     

Files with missing lines	Coverage Δ
app/lan_fence.py	90.48% <87.87%> (-0.77%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[GeiserX]

Live check against a real Docker engine (Linux Docker in a VM), worker image 1.72.1, with this branch's and main's app/ mounted in turn. A second helper with the fence label runs sleep 4 under the name cashpilot-fence, then the worker runs run_helper("read"):

• main: run_helper returned in 0.7 s, and the other helper's exit code was 137. It was killed.
• this branch: run_helper returned in 14.2 s (the 4 s run, then the 10 s grace, because nothing in the check removes the other helper the way its owner would), and the other helper's exit code was 0. Its own run returned ok=True backend=nft, and no helper was left behind.

[GeiserX]

Landed in #653 (f681889), which combined this fix with the others into one release.