Refuse a daemon rename onto an installed service by realtonyyoung · Pull Request #1268 · kurrent-io/kcap

realtonyyoung · 2026-10-01T13:45:07Z

AI-2703 (Linear-only; there is no GitHub issue)

What & why

Settings probes only the target name's socket, so a stopped-but-installed service under the new name looked free, and install --replace --retire then cleared that unit — possibly another profile's background service — after retiring the old one. Under the target's transaction lock, before the old service is touched, the rename now queries the target: an installed unit (loaded or stopped) refuses with retire_reason=target_occupied, an unknown state with target_unknown (exit 30). A live owner stays verify_contended. The app restores the saved name on those two reasons and says nothing changed.

Where to look

A unit that appears under the new name while the old one is being retired (a writer ignoring the lock) is refused as target_claimed rather than cleared; the old service is already retired by then, so the app keeps its existing attention message with the uninstall hint.

Verification

ServiceVerify suites pass (112), including new cases for a stopped target, an unknown target and a target claimed mid-retire; each new case failed before the change. A free target still renames. SettingsViewModel tests (39) cover the restored name and message for both refusal reasons.

🤖 Generated with Claude Code

A stopped service has no socket, so only its unit shows the name is taken; the rename now checks it under the target lock before retiring the old service. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

realtonyyoung · 2026-10-01T13:45:17Z

/agentic_review

linear-code · 2026-10-01T13:45:18Z

AI-2703

qodo-code-review · 2026-10-01T13:45:44Z

PR Summary by Qodo

Refuse daemon renames when the target service is installed

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Refuse renames to installed or unqueryable target services before retiring the old service.
• Preserve a target unit claimed during retirement instead of clearing it.
• Restore the saved name and explain when a rename was refused without changing either service.

Diagram

graph TD
  S["Settings rename"] --> Q["Probe target"] --> F{"Target free?"}
  F -- "No or unknown" --> R["Restore saved name"]
  F -- "Yes" --> O["Retire old unit"] --> C{"Target still free?"}
  C -- "No" --> X["Refuse claimed unit"]
  C -- "Yes" --> I["Install new unit"]

High-Level Assessment

Keep the target-unit check before retirement and the second guard before replacement. A socket-only check misses stopped services, while checking solely during replacement would leave the old service retired before discovering an existing target.

Files changed (4) +111 / -3

Bug fix (2) +27 / -3

SettingsViewModel.csRestore the saved name after an early target refusal +7/-0

Restore the saved name after an early target refusal
• Handles occupied and unknown target outcomes by restoring the previous saved name and showing a specific message that nothing changed. Returns without requesting an app relaunch.
src/Capacitor.App/ViewModels/SettingsViewModel.cs

ServiceVerify.csGuard both sides of target-service retirement +20/-3

Guard both sides of target-service retirement

• Queries the target under its transaction lock and refuses installed or unknown units before retiring the old service. If a unit appears during retirement, replacement refuses to clear it and reports target_claimed.

src/Capacitor.Cli/Services/ServiceVerify.cs

Tests (2) +84 / -0

SettingsViewModelTests.csTest saved-name restoration for target refusals +22/-0

Test saved-name restoration for target refusals
• Covers occupied and unknown outcomes, checking that the original name remains saved, no relaunch occurs, and the UI explains the refusal.
test/Capacitor.App.Tests.Unit/SettingsViewModelTests.cs

ServiceVerifyRetireTests.csTest stopped, unknown, and mid-retirement targets +62/-0

Test stopped, unknown, and mid-retirement targets
• Extends the fake service manager to represent each target state. Verifies that early refusals leave the old service untouched and that a target appearing during retirement is neither cleared nor overwritten.
test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyRetireTests.cs

qodo-code-review · 2026-10-01T13:47:49Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Late refusal docs hide a saved marker~~ ✓ Resolved

Description

The expanded RetireRefused documentation says nothing is written for the new id, but the new
target_claimed branch returns after InstallVerifiedAsync writes a captured transaction marker
for that id. When another unit claims the target during retirement, a later reader could mistake the
refusal for one that leaves no target-side transaction state.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R75-77]

+    /// or its state is unknown. Nothing is written for the new id — its own
+    /// entry-time leftover-marker recovery already ran, as on every install. The old unit is untouched
+    /// unless the new id was taken while it was being retired. The stderr line

Relevance

●●● Strong
The comment makes a demonstrably false behavioral claim about marker writes on the target-claimed
refusal path.
PR-#656
PR-#473

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The updated comment includes a refusal path that writes a target marker before returning; the
comment's unqualified assertion about writes is therefore no longer true.
Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.Cli/Services/ServiceVerify.cs[73-78]
src/Capacitor.Cli/Services/ServiceVerify.cs[839-843]
src/Capacitor.Cli/Services/ServiceVerify.cs[1094-1097]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The refusal documentation now covers `target_claimed`, but its claim that nothing is written for the new id is inaccurate for that path.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[73-78]
- src/Capacitor.Cli/Services/ServiceVerify.cs[839-843]
- src/Capacitor.Cli/Services/ServiceVerify.cs[1094-1097]

## Recommended Fix
Distinguish the early refusals from `target_claimed`: no new unit is written, but the late refusal can leave a captured transaction marker for recovery.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Rename docs omit stopped collisions~~ ✓ Resolved

Description

InstallVerifiedAsync now refuses --retire when the new name has an installed unit or an unknown
state, but the README's daemon command documentation describes only live-owner contention under the
new name. Operators using install --replace --verify --retire cannot learn from that section why a
stopped target or uncertain probe returns a refusal instead of proceeding.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R799-801]

+            var target = manager.Query(serviceId, _forwardBudget);
+            if (target.Probe == LabelProbe.Unknown) return RetireRefusal("target_unknown");
+            if (target.Probe == LabelProbe.Loaded || target.UnitPresent) {

Relevance

●●● Strong
The PR adds user-facing refusal outcomes, so README documentation should explain stopped and unknown
target behavior.
PR-#719

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed CLI path adds target-state refusals, while the README section covering --retire
describes live contention but not these new outcomes.
Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Services/ServiceVerify.cs[799-806]
README.md[1068-1090]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The documented `--retire` behavior omits the new refusal cases for installed and unknown targets.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[799-806]
- README.md[1068-1090]

## Recommended Fix
Update the README daemon command section to describe both refusal reasons, their exit behavior, and the distinction from live-owner contention.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: The push changes runtime rename/install verification behavior and timeout budgeting in a service-management path with meaningful state, race, and rollback risks, but the logic is localized rather than dense enough for extended review.

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit e15473c

Results up to commit d30e632 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Rename docs omit stopped collisions~~ ✓ Resolved

Description

InstallVerifiedAsync now refuses --retire when the new name has an installed unit or an unknown
state, but the README's daemon command documentation describes only live-owner contention under the
new name. Operators using install --replace --verify --retire cannot learn from that section why a
stopped target or uncertain probe returns a refusal instead of proceeding.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R799-801]

+            var target = manager.Query(serviceId, _forwardBudget);
+            if (target.Probe == LabelProbe.Unknown) return RetireRefusal("target_unknown");
+            if (target.Probe == LabelProbe.Loaded || target.UnitPresent) {

Relevance

●●● Strong
The PR adds user-facing refusal outcomes, so README documentation should explain stopped and unknown
target behavior.
PR-#719

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed CLI path adds target-state refusals, while the README section covering --retire
describes live contention but not these new outcomes.
Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Services/ServiceVerify.cs[799-806]
README.md[1068-1090]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The documented `--retire` behavior omits the new refusal cases for installed and unknown targets.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[799-806]
- README.md[1068-1090]

## Recommended Fix
Update the README daemon command section to describe both refusal reasons, their exit behavior, and the distinction from live-owner contention.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Late refusal docs hide a saved marker~~ ✓ Resolved

Description

The expanded RetireRefused documentation says nothing is written for the new id, but the new
target_claimed branch returns after InstallVerifiedAsync writes a captured transaction marker
for that id. When another unit claims the target during retirement, a later reader could mistake the
refusal for one that leaves no target-side transaction state.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R75-77]

+    /// or its state is unknown. Nothing is written for the new id — its own
+    /// entry-time leftover-marker recovery already ran, as on every install. The old unit is untouched
+    /// unless the new id was taken while it was being retired. The stderr line

Relevance

●●● Strong
The comment makes a demonstrably false behavioral claim about marker writes on the target-claimed
refusal path.
PR-#656
PR-#473

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The updated comment includes a refusal path that writes a target marker before returning; the
comment's unqualified assertion about writes is therefore no longer true.
Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.Cli/Services/ServiceVerify.cs[73-78]
src/Capacitor.Cli/Services/ServiceVerify.cs[839-843]
src/Capacitor.Cli/Services/ServiceVerify.cs[1094-1097]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The refusal documentation now covers `target_claimed`, but its claim that nothing is written for the new id is inaccurate for that path.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[73-78]
- src/Capacitor.Cli/Services/ServiceVerify.cs[839-843]
- src/Capacitor.Cli/Services/ServiceVerify.cs[1094-1097]

## Recommended Fix
Distinguish the early refusals from `target_claimed`: no new unit is written, but the late refusal can leave a captured transaction marker for recovery.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-10-01T13:47:50Z

+    /// or its state is unknown. Nothing is written for the new id — its own
+    /// entry-time leftover-marker recovery already ran, as on every install. The old unit is untouched
+    /// unless the new id was taken while it was being retired. The stderr line


Resolved ✓

~~2. Late refusal docs hide a saved marker~~ 📘 Rule violation ⚙ Maintainability

The expanded RetireRefused documentation says nothing is written for the new id, but the new target_claimed branch returns after InstallVerifiedAsync writes a captured transaction marker for that id. When another unit claims the target during retirement, a later reader could mistake the refusal for one that leaves no target-side transaction state.

Agent Prompt

## Issue description The refusal documentation now covers `target_claimed`, but its claim that nothing is written for the new id is inaccurate for that path. ## Fix Focus Areas - src/Capacitor.Cli/Services/ServiceVerify.cs[73-78] - src/Capacitor.Cli/Services/ServiceVerify.cs[839-843] - src/Capacitor.Cli/Services/ServiceVerify.cs[1094-1097] ## Recommended Fix Distinguish the early refusals from `target_claimed`: no new unit is written, but the late refusal can leave a captured transaction marker for recovery.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

realtonyyoung · 2026-10-01T14:18:56Z

/agentic_review

qodo-code-review · 2026-10-01T14:20:22Z

Code review by qodo was updated up to the latest commit e15473c

Refuse a daemon rename onto an installed service

d30e632

A stopped service has no socket, so only its unit shows the name is taken; the rename now checks it under the target lock before retiring the old service. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qodo-code-review Bot reviewed Oct 1, 2026

View reviewed changes

realtonyyoung and others added 2 commits October 1, 2026 09:50

Bound the rename target probe inside the app's rename timeout

8aa4f0a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Document the rename refusals for an occupied target

e15473c

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

realtonyyoung merged commit e6f597d into main Oct 1, 2026
8 checks passed

realtonyyoung deleted the tonyyoung/ai-2703-refuse-colliding-rename branch October 1, 2026 15:12

This was referenced Oct 1, 2026

Fence new daemon work while a rename retires it #1271

Merged

Never save repos.json over a copy that could not be read #1277

Merged

This was referenced Oct 16, 2026

Add the service adapter for the local server unit #1532

Merged

Delete the daemon state directory on kcap uninstall #1581

Merged

qodo-code-review Bot mentioned this pull request Oct 27, 2026

[AI-3993]: Added Systemd and snap support #1723

Merged

Navigation Menu

Repository navigation

Refuse a daemon rename onto an installed service - #1268

Refuse a daemon rename onto an installed service#1268
realtonyyoung merged 3 commits into
mainfrom
tonyyoung/ai-2703-refuse-colliding-rename

realtonyyoung commented Oct 1, 2026

Uh oh!

realtonyyoung commented Oct 1, 2026

Uh oh!

linear-code Bot commented Oct 1, 2026

Uh oh!

qodo-code-review Bot commented Oct 1, 2026

Uh oh!

qodo-code-review Bot commented Oct 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

qodo-code-review Bot Oct 1, 2026 •

edited

Loading

Uh oh!

realtonyyoung commented Oct 1, 2026

Uh oh!

qodo-code-review Bot commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

realtonyyoung commented Oct 1, 2026

What & why

Where to look

Verification

Uh oh!

realtonyyoung commented Oct 1, 2026

Uh oh!

linear-code Bot commented Oct 1, 2026

Uh oh!

qodo-code-review Bot commented Oct 1, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

qodo-code-review Bot Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

realtonyyoung commented Oct 1, 2026

Uh oh!

qodo-code-review Bot commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-code-review Bot commented Oct 1, 2026 •

edited

Loading

qodo-code-review Bot Oct 1, 2026 •

edited

Loading