fix(shared): release relay install locks on cancellation

[yashranaway]

What Changed

The relay client installer registers lock cleanup as part of each successful exclusive acquisition. The installation owns that cleanup through its scope, while contention waits remain interruptible.

Why

Cancellation after the lock file was created but before the old cleanup handler was installed could strand the lock. A subsequent installation then failed as locked until stale-lock recovery became eligible.

Testing

• The controlled acquisition-cancellation regression leaves a lock behind on upstream and removes it with the fix.
• Eight focused relay client tests pass, including cancellation during download, cancellation while another installer owns the lock, checksum failure cleanup, and an immediate successful retry.
• Shared-package typecheck, scoped lint, and formatting pass.
• No tunnel configuration, frontend changes, browser verification, or live network operations.

Model: GPT-6 Astra
Harness: T3 code

Note

Release relay install locks on cancellation in cloudflared.installWithProgress

• Lock cleanup moves from an installer-level finalizer into an acquire-release resource inside cloudflared.acquireInstallLock, so the lock file is force-removed when the scope exits — including interruption or cancellation.
• Failed acquisition attempts caused by an existing lock owned by another installer do not register cleanup, so that pre-existing lock is left untouched.
• cloudflared.installWithProgress now wraps installUnlocked in an explicit scoped effect so the acquire-release lock resource is finalized for the public install operation.
• Adds tests covering cancellation during a contended lock, during download, and racing with lock acquisition, plus an assertion that an invalid checksum leaves no lock behind.
• Risk: installUnlocked no longer unconditionally removes the lock file from its own finalizer; any caller that invokes installUnlocked outside of a managed scope will not get automatic lock cleanup.

^{Macroscope summarized 03235d8.}

[macroscopeapp]

Approvability

Verdict: Approved at c8e346b

Macroscope's review found this PR approvable — This focused fix changes only relay-install lock cleanup, ensuring acquired locks are released on cancellation without disturbing locks owned by competing installers. Targeted regression tests cover the affected lifecycle paths, and no product defaults, schemas, infrastructure, or static-analysis suppressions are changed.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: e2e34d25-6edf-42a4-95c4-7eea44a16f85

📥 Commits

Reviewing files that changed from the base of the PR and between 36bd024 and c8e346b.

📒 Files selected for processing (2)

• packages/shared/src/relayClient.test.ts
• packages/shared/src/relayClient.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The relay client now removes install locks through scoped resource finalization. Tests cover contended installs, cancellation during download and lock acquisition, and checksum failure.

Changes

Install lock cleanup

Layer / File(s)	Summary
Scoped install-lock lifecycle packages/shared/src/relayClient.ts	Lock acquisition registers lock removal with Effect.acquireRelease. installUnlocked no longer performs separate cleanup. installWithProgress runs the installation with Effect.scoped.
Cancellation and failure validation packages/shared/src/relayClient.test.ts	Tests check lock preservation during contention and lock removal after download cancellation, interrupted lock acquisition, and checksum mismatch.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to c8e34

Cancellation now releases an acquired installation lock without removing another installer’s lock. The change is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to c8e34

The change closes a cancellation gap without expanding installation authority or weakening executable validation. Successful acquisitions own their cleanup, while interrupted contention leaves the other installer’s lock intact.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed state is the lock associated with a managed relay executable. Installations targeting that same managed path share the contention domain. The reviewed changes add neither a new caller nor additional filesystem, credential, or execution authority.

Trust Boundaries and Controls

• observed — Exclusive creation remains the acquisition control. AlreadyExists becomes a failed acquisition without registering cleanup, and the contention-cancellation test asserts that the foreign lock’s contents remain intact.

Resilience and Maintainability Implications

• observed — Lock ownership remains path-based rather than token-based, stale recovery remains mtime-based, and release suppresses filesystem removal errors. These limitations predate the PR: scoped release improves cancellation handling but does not guarantee removal after filesystem failure or abrupt process termination.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the problem, implementation, affected behavior, and focused verification results. It does not provide the required Scope and approval information, such as a linked issue, main…	Add a Scope and approval section. Include the triaged issue or explicit maintainer approval with a link, or explain why this focused fix qualifies as an obvious bug exemption. Rename or align the existing sections with Problem, Change, Scop…

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: releasing relay install locks when cancellation occurs.
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.

Full details: Description check

Explanation

The description explains the problem, implementation, affected behavior, and focused verification results. It does not provide the required Scope and approval information, such as a linked issue, maintainer approval, or an explanation that this focused fix qualifies as an obvious bug exemption. It also uses alternative headings instead of the template headings.

Resolution

Add a Scope and approval section. Include the triaged issue or explicit maintainer approval with a link, or explain why this focused fix qualifies as an obvious bug exemption. Rename or align the existing sections with Problem, Change, Scope and approval, and Verification where practical.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

Dismissing prior approval to re-evaluate 03235d8

Dismissing prior approval to re-evaluate c8e346b

[juliusmarminge]

Rebased onto main after #17275 and squashed into one commit (c8e346b). Your three cancellation tests and the lock check on the checksum test are ported onto main's test file. They pass, and "releases a lock acquired while installation is cancelled" fails without the fix.