Fence new daemon work while a rename retires it

[realtonyyoung]

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

What & why

A rename boots out the old daemon, killing whatever it runs. Settings' idle check was a stale snapshot, so a launch admitted between it and the bootout was killed — including work still in the consent prompt or worktree setup, local kcap agent start spawns and eval runs. The daemon now has one admission fence: server launches, local spawns and eval prepares admit through it, and a rename acquires it only when nothing is in flight and the daemon is idle, under the same lock. The CLI holds it over the control socket from before leftover-marker recovery, commits it just before bootout, and refuses with agents_active, fence_unsupported or fence_unavailable (exit 30) otherwise. Settings restores the saved name on those, and the lane no longer marks the old id retired after a refusal that changed nothing — which also blocked a retry after target_occupied.

Where to look

A committed fence is written to disk before the daemon answers, survives the CLI's connection, and is inherited by a restarted process under the old name; only abort on the open connection or a 2-minute lease lifts it. A CLI that dies between commit and bootout can leave an idle daemon closed to new work for that lease — deliberate, since a broken connection is indistinguishable from one mid-bootout. A running daemon older than the fence fails closed (fence_unsupported): restart it, then rename. The design is in docs/superpowers/specs/.

Verification

New daemon tests drive admission versus acquire, commit/abort/close/lease, marker inheritance and unreadable markers, and refused launches, spawns and prepares over a real socket. ServiceVerify retire tests cover each refusal before anything changes; a mutant that skips the fence fails 7 of them. App tests cover name restore and the lane retry.

🤖 Generated with Claude Code

[linear-code]

AI-2704

[realtonyyoung]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Fence daemon work before retiring a service during rename

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Prevent renames from retiring a daemon that has admitted agents or evaluations.
• Persist committed fences across daemon restarts and refuse unsafe retirements before bootout.
• Restore the saved name after untouched refusals and allow subsequent rename attempts.

Diagram

sequenceDiagram
    participant Work as Work entries
    participant Fence as Admission fence
    participant Settings as Settings
    participant Lane as Mutation lane
    participant CLI as Service verify
    participant IPC as Control socket
    participant Marker as Retiring marker
    participant Launchd as Service manager
    Work->>Fence: Admit new work
    Settings->>Lane: Request rename
    Lane->>CLI: Install with retire
    CLI->>Launchd: Check old process
    CLI->>IPC: Acquire fence
    IPC->>Fence: Check idle and hold
    Fence-->>IPC: Held or busy
    IPC-->>CLI: Fence acknowledgment
    CLI->>IPC: Commit fence
    IPC->>Fence: Commit hold
    Fence->>Marker: Persist commit
    IPC-->>CLI: Commit acknowledgment
    CLI->>Launchd: Boot out old service
    CLI-->>Lane: Result or refusal
    Lane-->>Settings: Rename outcome

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Queue rename until daemon is idle

• ➕ Could complete a rename without asking the user to retry.
• ➖ Changes the rename interaction and still requires atomic admission blocking, cancellation handling, and restart-safe coordination.

Recommendation: Keep the connection-held, persisted fence and fail-closed refusal. It directly closes the admission-to-bootout race while preserving the existing service transaction and explicit retry behavior; queuing would add lifecycle complexity without removing the need for a fence.

Files changed (42) +1518 / -62

Enhancement (14) +319 / -6

DaemonStore.csLocate the retiring marker +4/-0

Locate the retiring marker

• Adds a per-daemon state path for the persistent fence commit marker.

src/Capacitor.Cli.Core/DaemonStore.cs

AdmissionFenceAcquireResult.csRepresent fence acquisition results +4/-0

Represent fence acquisition results

• Pairs an acquisition outcome with its optional live fence session and diagnostic detail.

src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceAcquireResult.cs

AdmissionFenceClient.csImplement the CLI fence socket client +129/-0

Implement the CLI fence socket client

• Checks the daemon's advertised capability, acquires a connection-held fence, and supports commit and abort with bounded exchanges.

src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceClient.cs

AdmissionFenceIpc.csDefine fence wire payloads and tokens +33/-0

Define fence wire payloads and tokens

• Introduces acquire and acknowledgment DTOs, protocol constants, and snake-case JSON serialization metadata.

src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceIpc.cs

AdmissionFenceOutcome.csClassify fence acquisition outcomes +11/-0

Classify fence acquisition outcomes

• Distinguishes acquired, busy, unsupported, and unavailable daemons for retirement decisions.

src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceOutcome.cs

FrameCodec.csEncode and decode fence frames +6/-2

Encode and decode fence frames

• Adds the fence request and acknowledgment frame types to the UTF-8 text codec.

src/Capacitor.Cli.Core/LocalIpc/FrameCodec.cs

FrameType.csAllocate fence protocol frame types +6/-0

Allocate fence protocol frame types

• Defines acquire, commit, abort, and acknowledgment frame identifiers.

src/Capacitor.Cli.Core/LocalIpc/FrameType.cs

IAdmissionFenceSession.csExpose a connection-held fence session +14/-0

Expose a connection-held fence session

• Defines PID inspection and commit, abort, and disposal operations used by service retirement.

src/Capacitor.Cli.Core/LocalIpc/IAdmissionFenceSession.cs

LocalFrame.csConstruct fence JSON frames +2/-0

Construct fence JSON frames

• Adds a helper for carrying serialized fence payloads in local frames.

src/Capacitor.Cli.Core/LocalIpc/LocalFrame.cs

DaemonRunner.csShare one daemon admission fence +3/-0

Share one daemon admission fence

• Registers the fence and its socket handler as singletons, using the daemon's marker path and instance identity.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

AdmissionFenceIpc.csServe connection-scoped fence requests +91/-0

Serve connection-scoped fence requests

• Validates acquisition identity and daemon idleness, then handles commit and abort on the acquiring socket. An uncommitted hold ends with the connection.

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs

LocalControlCapabilities.csAdvertise fence protocol support +3/-2

Advertise fence protocol support

• Adds the fence/1 capability so CLI clients can distinguish supported daemons from older running processes.

src/Capacitor.Cli.Daemon/Services/LocalControlCapabilities.cs

LocalControlServer.csRoute fence acquisition over the control socket +3/-2

Route fence acquisition over the control socket

• Passes acquire frames to the new long-lived fence handler and includes the frame in the supported-request diagnostic.

src/Capacitor.Cli.Daemon/Services/LocalControlServer.cs

RetiringMarker.csSerialize persisted fence commits +10/-0

Serialize persisted fence commits

• Defines the instance ID and commit timestamp stored for daemon restart inheritance.

src/Capacitor.Cli.Daemon/Services/RetiringMarker.cs

Bug fix (9) +368 / -26

KcapCli.csAllow more time for fenced renames +3/-2

Allow more time for fenced renames

• Extends the app's rename CLI timeout from 100 to 120 seconds to accommodate fence exchanges and abort.

src/Capacitor.App/Services/KcapCli.cs

DaemonMutationLane.csKeep untouched service IDs eligible for retry +2/-1

Keep untouched service IDs eligible for retry

• Avoids recording the old ID as retired when a rename refusal changed neither service.

src/Capacitor.App/Services/Mutation/DaemonMutationLane.cs

RenameRefusal.csCentralize untouched rename refusals +20/-0

Centralize untouched rename refusals

• Identifies refusal reasons that leave both services unchanged and supplies corresponding user-facing messages.

src/Capacitor.App/Services/RenameRefusal.cs

SettingsViewModel.csRestore the name after fence refusals +2/-4

Restore the name after fence refusals

• Restores the previous saved name and keeps editing open when active work, unsupported fencing, or unavailable fencing prevents retirement.

src/Capacitor.App/ViewModels/SettingsViewModel.cs

AdmissionFence.csMake admission and idle acquisition atomic +200/-0

Make admission and idle acquisition atomic

• Tracks in-flight work and held or committed fences under one lock. Persists commits, inherits live markers on startup, and releases committed fences after a two-minute lease or explicit abort.

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs

AgentOrchestrator.LocalIpc.csFence local agent spawns +10/-0

Fence local agent spawns

• Requires admission before starting a local spawn and returns an error frame when the daemon is retiring.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs

AgentOrchestrator.csFence server agent launches +23/-1

Fence server agent launches

• Shares the daemon admission fence and admits launches before consent or setup work. Rejects launches reaching a fenced daemon as semantic failures.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

EvalRunner.csFence evaluation preparation +12/-1

Fence evaluation preparation

• Requires admission before preparing an evaluation and refuses new preparations while a rename fence stands.

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs

ServiceVerify.csFence the old daemon before retirement +96/-17

Fence the old daemon before retirement

• Locks and checks the old service, acquires a PID-matched fence before target recovery and checks, and commits it before bootout. Refuses unsafe states with specific reason tokens and aborts an unacknowledged commit.

src/Capacitor.Cli/Services/ServiceVerify.cs

Tests (18) +685 / -30

DaemonMutationLaneTests.csVerify retries after untouched refusals +12/-4

Verify retries after untouched refusals

• Checks that target and fence refusals do not retire the old ID or block a queued mutation, while outcomes that may change state still do.

test/Capacitor.App.Tests.Unit/DaemonMutationLaneTests.cs

KcapCliTests.csCheck the expanded rename timeout +1/-1

Check the expanded rename timeout

• Updates the expected CLI timeout for a rename to 120 seconds.

test/Capacitor.App.Tests.Unit/KcapCliTests.cs

SettingsViewModelTests.csVerify name restoration after fence refusals +4/-1

Verify name restoration after fence refusals

• Extends refusal cases to check the saved name and user message for active work and unsupported or unavailable fencing.

test/Capacitor.App.Tests.Unit/SettingsViewModelTests.cs

AdmissionFenceIpcTests.csExercise fence protocol over a real socket +283/-0

Exercise fence protocol over a real socket

• Tests acquisition identity, busy and malformed replies, connection release, persisted commit, abort, capability discovery, client exchanges, and refused launches and spawns.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceIpcTests.cs

AdmissionFenceTests.csTest fence admission and persistence +183/-0

Test fence admission and persistence

• Covers in-flight and idle exclusion, held and committed lifetimes, abort, lease expiry, marker-write failure, restart inheritance, and unreadable markers.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs

AgentOrchestratorLocalAttachTests.csWire fences into local attach test servers +3/-3

Wire fences into local attach test servers

• Supplies the new fence socket handler to existing real-socket test setups.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorLocalAttachTests.cs

ConsentRulesPutV2Tests.csWire a fence into consent-rule socket tests +1/-1

Wire a fence into consent-rule socket tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ConsentRulesPutV2Tests.cs

DaemonSettingsIpcTests.csWire a fence into daemon-settings socket tests +1/-1

Wire a fence into daemon-settings socket tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/DaemonSettingsIpcTests.cs

DaemonStatusIpcTests.csWire a fence into daemon-status socket tests +1/-1

Wire a fence into daemon-status socket tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/DaemonStatusIpcTests.cs

EvalContextCacheTests.csTest fenced evaluation preparation +28/-1

Test fenced evaluation preparation

• Supplies an admission fence to evaluation fixtures and checks that fenced prepares fail without creating runs while cached preparations keep acquisition busy.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/EvalContextCacheTests.cs

EvalRunnerEvidenceTests.csSupply a fence to evidence-runner fixtures +2/-1

Supply a fence to evidence-runner fixtures

• Updates evaluation runner construction to provide its required admission fence.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/EvalRunnerEvidenceTests.cs

LaunchConsentIpcTests.csWire a fence into consent socket tests +1/-1

Wire a fence into consent socket tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LaunchConsentIpcTests.cs

LocalControlCapabilitiesTests.csCheck the advertised fence capability +1/-1

Check the advertised fence capability

• Adds fence/1 to the exact expected control capability list.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LocalControlCapabilitiesTests.cs

LocalControlHelloTests.csCheck fence discovery in hello replies +4/-4

Check fence discovery in hello replies

• Wires the fence handler into socket fixtures and expects fence/1 in hello capability lists.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LocalControlHelloTests.cs

LocalControlOpsV2PutTests.csWire a fence into control-ops tests +1/-1

Wire a fence into control-ops tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LocalControlOpsV2PutTests.cs

LocalControlProbeTests.csWire a fence into control-probe tests +1/-1

Wire a fence into control-probe tests

• Updates the control-server fixture with the required fence handler.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LocalControlProbeTests.cs

TestFences.csShare test fence-handler construction +10/-0

Share test fence-handler construction

• Builds a control handler around the orchestrator's own fence for socket test fixtures.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/TestFences.cs

ServiceVerifyRetireTests.csTest fenced service retirement and refusals +148/-8

Test fenced service retirement and refusals

• Adds a controllable fence session to verify acquisition and commit ordering, untouched refusal paths, PID mismatch, abort after an unanswered commit, and retirement of an unloaded service.

test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyRetireTests.cs

Documentation (1) +146 / -0

2026-10-01-rename-admission-fence-design.mdSpecify the rename admission fence +146/-0

Specify the rename admission fence

• Documents the race, work admission points, socket protocol, persisted lease, CLI ordering, app behavior, and test plan.

docs/superpowers/specs/2026-10-01-rename-admission-fence-design.md

[qodo-code-review]

Code Review by Qodo

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

1. Active evaluations can be killed by rename ✓ Resolved

Description

AdmissionFenceIpc.HandleAcquireAsync treats an empty evaluation cache as idle, even though
cancelling a run removes its cache entry before an already-running question or finalize phase
finishes. If cancellation overlaps a legacy evaluation phase, that phase continues without a per-run
cancellation token while the rename can acquire the fence and boot out its daemon; evidence phases
can likewise remain active under a lease after cache removal.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32]

+        if (fence.TryAcquire(() => orchestrator.EffectiveCount > 0 || evalCache.Count > 0, out var hold) is AdmissionFence.AcquireResult.Busy) {

Relevance

●● Moderate

The race is plausible and reliability-relevant, but evidence is weaker because cache lifetime is the
documented busy mechanism.

PR-#616
PR-#734

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new acquire predicate checks only cache membership. Cache removal immediately reduces Count,
whereas legacy phases retain their context and run with only the daemon shutdown token; evidence
leases can also survive removal.

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[30-35]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[139-166]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[350-361]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[39-59]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[117-145]

Agent prompt

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

## Issue description
A cancelled evaluation leaves the cache immediately, but its question or finalize phase can still be running. The rename fence can therefore mistake active evaluation work for an idle daemon.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[30-35]
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[139-166]
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[350-361]
- src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[39-59]

## Recommended Fix
Track active question and finalize phases independently of cache membership, and include that count in the fence's busy decision. Keep each phase counted until it has actually returned, including after cancellation or cache removal, and test an acquire while a cancelled phase remains in flight.

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

---

2. Repeated commits can prolong a rename fence ✗ Dismissed

Description

Hold.Commit() permits another commit on an already-committed connection, and CommitLocked
rewrites the marker with a fresh timestamp each time. If a client retries commits on that
connection, each retry moves the two-minute expiry forward, so the daemon can keep refusing new work
beyond the intended lease.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R192-195]

+        /// <summary>Undoes this hold, committed or not; false when it is no longer this hold's, or a
+        /// committed marker could not be deleted.</summary>
+        public bool Abort() {
+            lock (fence._gate) return fence.AbortLocked(this);

Relevance

●●● Strong

Concrete lease-extension bug in retryable commit handling; accepted race/reliability fixes show the
team addresses these hazards.

PR-#1021
PR-#1031

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The IPC handler accepts commit frames in a loop, and each invokes the hold's commit method.
CommitLocked writes a new marker and assigns the current time to _committedAt, while FencedLocked
calculates expiry from that mutable value.

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[53-63]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[97-103]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[79-90]

Agent prompt

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

## Issue description
Repeated commit frames on one fence connection refresh the persisted commit timestamp, allowing the two-minute lease to be extended indefinitely.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[79-103]
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[185-200]
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[53-63]

## Recommended Fix
Make a second commit on the same hold idempotent without rewriting the marker or changing its original timestamp, and reject it once the original lease has expired. Add a test that sends a second commit before expiry and verifies admission reopens at the original deadline.

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

---

3. Fence tests bypass temp-file helpers ✓ Resolved

Description

AdmissionFenceTests creates directories and writes a marker beneath Tmp with direct filesystem
calls instead of its CreateDir and CreateFile helpers. Both the unwritable-marker and
unreadable-marker tests construct temporary state this way, leaving later test changes to maintain
two path-creation patterns.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[R132-133]

+        Directory.CreateDirectory(Tmp.PathTo("state"));
+        Directory.CreateDirectory(MarkerPath); // a directory where the marker file belongs

Relevance

●●● Strong

Recent test precedents accept replacing direct temporary filesystem operations with TempDir helpers.

PR-#803
PR-#590

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new tests derive MarkerPath from the injected TempDir but use Directory.CreateDirectory and
File.WriteAllTextAsync beneath it. The checklist specifically flags direct directory creation and
file writing where TempDir offers equivalent helpers.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[13-17]
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[130-133]
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[171-176]

Agent prompt

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

## Issue description
Two fence tests create temporary state beneath the injected TempDir using direct filesystem calls.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[130-134]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[171-177]

## Recommended Fix
Use `Tmp.CreateDir` for the state directory and marker-path directory, and `Tmp.CreateFile` for the malformed marker content.

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

---

4. Rename refusals are missing from README ✓ Resolved

Description

PrepareRetireAsync adds agents_active, fence_unsupported, and fence_unavailable refusals to
the --retire command without updating README.md. The README documents that command and its
existing refusal reasons, so users consulting it cannot distinguish these new outcomes.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R1086-1089]

+        var fence = await _acquireFence(retireId, CancellationToken.None);
+        switch (fence.Outcome) {
+            case AdmissionFenceOutcome.Busy:        return Refuse("agents_active");
+            case AdmissionFenceOutcome.Unsupported: return Refuse("fence_unsupported");

Relevance

●●● Strong

Recent same-day precedent accepts README updates for new user-visible CLI behavior and refusal
cases.

PR-#1268
PR-#403
PR-#191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed code adds user-visible refusal reasons, while the README section covering --retire
lists only the earlier reasons. The checklist requires that section to stay in sync with changed CLI
behavior.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Services/ServiceVerify.cs[1082-1096]
README.md[1091-1091]

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` command now has fence-related refusal reasons that README.md does not explain.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[1082-1096]
- README.md[1091-1091]

## Recommended Fix
Update the README's `--retire` documentation with the new refusal reasons and what users should do when they occur.

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

---

View medium (4)

5. A failed marker deletion blocks new work ✓ Resolved

Description

AdmissionFence.AbortLocked clears the in-memory committed state and acknowledges the abort even
when DeleteMarker fails to remove the persisted marker. If the daemon restarts before the marker's
lease expires, ReadMarker restores the committed fence and refuses new work despite the CLI having
aborted the rename.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R97-99]

+        if (_committedAt is null) return;
+        _committedAt = null;
+        DeleteMarker();

Relevance

●●● Strong

Acknowledging abort while marker deletion failed can persist a stale fence across restart;
fail-closed cleanup concerns are accepted.

PR-#347
PR-#1021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Abort clears _committedAt before calling a deletion helper that catches and logs failures without
reporting them. The IPC handler then sends an aborted acknowledgement, while construction of a
replacement fence reads any remaining live marker and starts fenced.

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[94-100]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[155-157]
src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[60-64]

Agent prompt

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

## Issue description
An abort can acknowledge success while its retiring marker remains on disk. A subsequent daemon process reads that marker and resumes refusing work until the lease expires.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[94-100]
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[60-64]

## Recommended Fix
Make marker deletion success explicit. Do not report an aborted, open fence when the committed marker could not be removed; return a failure acknowledgement and retain a consistent fenced state until deletion succeeds or the lease expires. Add a restart test with deletion failure.

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

---

6. A constructor comment narrates old code ✓ Resolved

Description

The new admissionFence parameter comment calls other callers pre-existing construction sites,
describing when those callers were written rather than just their current behavior. A later reader
has to interpret that historical qualifier to understand which callers receive a private fence.

Code

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[R759-760]

+            // Null in every pre-existing construction site — those get a private fence over this
+            // daemon's own marker path. DaemonRunner passes the singleton the control socket fences.

Relevance

●●● Strong

Recent review history consistently accepts removing historical or process-oriented narration from
comments.

PR-#731
PR-#878
PR-#879

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added comment uses a historical qualifier about construction sites. The checklist disallows
change-history narration in code comments.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[759-765]

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 new constructor comment describes call-site history instead of stating the current fallback behavior.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[759-765]

## Recommended Fix
Describe what happens when the optional fence is null and how the daemon supplies its shared fence, without referring to when callers were created.

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

---

7. Renaming back quickly leaves the daemon refusing work ✓ Resolved

Description

ReadMarker starts any new process under the name fenced whenever committed_at is within the
lease, and it ignores the marker's instance_id. Nothing deletes retiring.json after a successful
bootout, so a rename back to the old name, or a manual start under it, within two minutes leaves a
daemon that refuses every launch with no connection able to abort the fence.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R119-122]

+        if (_time.GetUtcNow() < committedAt + CommitLease) {
+            LogStartingFenced(committedAt);
+            return committedAt;
+        }

Relevance

●● Moderate

Marker inheritance is deliberate, but successful bootout cleanup semantics are ambiguous without a
closely matching precedent.

PR-#1268
PR-#1021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The marker survives the bootout, and ReadMarker checks only the timestamp. The constructor sets
_committedAt from the marker, so a new process starts fenced.

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[34-40]
src/Capacitor.Cli/Services/ServiceVerify.cs[1117-1124]

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 retiring marker is never removed after a successful rename. A new daemon that starts under the old name within the lease inherits the fence and refuses all work.
## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[1109-1124]
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
## Recommended Fix
After `WaitForStopConfirmedAsync` succeeds in `RunRetireAsync`, delete `store.RetiringMarkerPath(retireId)`, ignoring IO errors.

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

---

8. Retry reports active agents on an idle daemon ✓ Resolved

Description

TryAcquire returns Busy when FencedLocked() is true, which includes a committed fence still
inside its lease, and the CLI maps that to agents_active. After a commit whose answer was lost and
could not be aborted, an immediate retry tells the user to wait for agents that do not exist, for up
to two minutes.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R62-65]

+            if (FencedLocked() || _inFlight > 0 || isBusy()) {
+                hold = null;
+                return AcquireResult.Busy;
+            }

Evidence

FencedLocked() returns true while a committed lease is live, so TryAcquire reports Busy. The
IPC handler sends that as busy, and ServiceVerify maps busy to agents_active.

src/Capacitor.Cli/Services/ServiceVerify.cs[1086-1089]
src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32-35]

Agent prompt

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

## Issue description
When the daemon is already fenced, `TryAcquire` returns `Busy`. The CLI then reports `agents_active` instead of `fence_unavailable`, which gives the user the wrong message.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[60-69]
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32-35]
- src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceClient.cs[39-41]
## Recommended Fix
1. Add `AcquireResult.Fenced` and return it when `FencedLocked()` is true.
2. Have `AdmissionFenceIpc` reply with a new `fenced` reason for that result.
3. Have `AdmissionFenceClient` map the `fenced` reason to `Unavailable`.

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

---

9. Rename timeout leaves almost no headroom ✓ Resolved

Description

The 120s RenameTimeout counts only two 3s fence exchanges and an abort. It omits the new 5s
old-label query and the two 3s connect timeouts in AdmissionFenceClient, so the worst case reaches
about 118s.

Code

src/Capacitor.App/Services/KcapCli.cs[R70-72]

+    // Retiring spends up to another 20s forward budget, 10s lock wait, 5s target probe and the old
+    // daemon's fence (two 3s exchanges, plus an abort) before installation.
+    static readonly TimeSpan RenameTimeout = TimeSpan.FromSeconds(120);

Relevance

●●● Strong

Timeout arithmetic is a deterministic reliability bug; recent budget-related fixes are accepted.

PR-#1268
PR-#154

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PrepareRetireAsync calls manager.Query(retireId, TargetProbeWait) (5s). AcquireAsync opens two
connections, each with its own 3s connect timeout and 3s exchange. Commit and abort add 3s each.

src/Capacitor.Cli/Services/ServiceVerify.cs[1081-1086]
PR-#1268

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 rename timeout comment and value omit the old-label query and the connect timeouts. The worst-case CLI run leaves only about 2s of headroom.
## Fix Focus Areas
- src/Capacitor.App/Services/KcapCli.cs[70-72]
- src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceClient.cs[8-47]
## Recommended Fix
List every term in the comment and raise `RenameTimeout` to about 135s, or bound the whole acquire step with a single shared deadline.

ⓘ 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

  Explored: repo: kurrent-io/kcap-server (sha: e5f057a4) — View relationship

Review mode: ⚖️ Balanced: This push changes concurrency and lifecycle-fencing behavior across daemon admission, IPC, eval execution, and rename retirement paths, creating genuine cross-cutting correctness and outage risk but not enough independent new logic to warrant 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 763eca7

Results up to commit edf5d65 🧠 Deep

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

1. Active evaluations can be killed by rename ✓ Resolved

Description

AdmissionFenceIpc.HandleAcquireAsync treats an empty evaluation cache as idle, even though
cancelling a run removes its cache entry before an already-running question or finalize phase
finishes. If cancellation overlaps a legacy evaluation phase, that phase continues without a per-run
cancellation token while the rename can acquire the fence and boot out its daemon; evidence phases
can likewise remain active under a lease after cache removal.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32]

+        if (fence.TryAcquire(() => orchestrator.EffectiveCount > 0 || evalCache.Count > 0, out var hold) is AdmissionFence.AcquireResult.Busy) {

Relevance

●● Moderate

The race is plausible and reliability-relevant, but evidence is weaker because cache lifetime is the
documented busy mechanism.

PR-#616
PR-#734

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new acquire predicate checks only cache membership. Cache removal immediately reduces Count,
whereas legacy phases retain their context and run with only the daemon shutdown token; evidence
leases can also survive removal.

src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[30-35]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[139-166]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[350-361]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[39-59]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[117-145]

Agent prompt

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

## Issue description
A cancelled evaluation leaves the cache immediately, but its question or finalize phase can still be running. The rename fence can therefore mistake active evaluation work for an idle daemon.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[30-35]
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[139-166]
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[350-361]
- src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[39-59]

## Recommended Fix
Track active question and finalize phases independently of cache membership, and include that count in the fence's busy decision. Keep each phase counted until it has actually returned, including after cancellation or cache removal, and test an acquire while a cancelled phase remains in flight.

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

---

2. Fence tests bypass temp-file helpers ✓ Resolved

Description

AdmissionFenceTests creates directories and writes a marker beneath Tmp with direct filesystem
calls instead of its CreateDir and CreateFile helpers. Both the unwritable-marker and
unreadable-marker tests construct temporary state this way, leaving later test changes to maintain
two path-creation patterns.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[R132-133]

+        Directory.CreateDirectory(Tmp.PathTo("state"));
+        Directory.CreateDirectory(MarkerPath); // a directory where the marker file belongs

Relevance

●●● Strong

Recent test precedents accept replacing direct temporary filesystem operations with TempDir helpers.

PR-#803
PR-#590

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new tests derive MarkerPath from the injected TempDir but use Directory.CreateDirectory and
File.WriteAllTextAsync beneath it. The checklist specifically flags direct directory creation and
file writing where TempDir offers equivalent helpers.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[13-17]
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[130-133]
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[171-176]

Agent prompt

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

## Issue description
Two fence tests create temporary state beneath the injected TempDir using direct filesystem calls.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[130-134]
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AdmissionFenceTests.cs[171-177]

## Recommended Fix
Use `Tmp.CreateDir` for the state directory and marker-path directory, and `Tmp.CreateFile` for the malformed marker content.

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

---

3. Rename refusals are missing from README ✓ Resolved

Description

PrepareRetireAsync adds agents_active, fence_unsupported, and fence_unavailable refusals to
the --retire command without updating README.md. The README documents that command and its
existing refusal reasons, so users consulting it cannot distinguish these new outcomes.

Code

src/Capacitor.Cli/Services/ServiceVerify.cs[R1086-1089]

+        var fence = await _acquireFence(retireId, CancellationToken.None);
+        switch (fence.Outcome) {
+            case AdmissionFenceOutcome.Busy:        return Refuse("agents_active");
+            case AdmissionFenceOutcome.Unsupported: return Refuse("fence_unsupported");

Relevance

●●● Strong

Recent same-day precedent accepts README updates for new user-visible CLI behavior and refusal
cases.

PR-#1268
PR-#403
PR-#191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed code adds user-visible refusal reasons, while the README section covering --retire
lists only the earlier reasons. The checklist requires that section to stay in sync with changed CLI
behavior.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Services/ServiceVerify.cs[1082-1096]
README.md[1091-1091]

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` command now has fence-related refusal reasons that README.md does not explain.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[1082-1096]
- README.md[1091-1091]

## Recommended Fix
Update the README's `--retire` documentation with the new refusal reasons and what users should do when they occur.

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

---

4. A failed marker deletion blocks new work ✓ Resolved

Description

AdmissionFence.AbortLocked clears the in-memory committed state and acknowledges the abort even
when DeleteMarker fails to remove the persisted marker. If the daemon restarts before the marker's
lease expires, ReadMarker restores the committed fence and refuses new work despite the CLI having
aborted the rename.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R97-99]

+        if (_committedAt is null) return;
+        _committedAt = null;
+        DeleteMarker();

Relevance

●●● Strong

Acknowledging abort while marker deletion failed can persist a stale fence across restart;
fail-closed cleanup concerns are accepted.

PR-#347
PR-#1021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Abort clears _committedAt before calling a deletion helper that catches and logs failures without
reporting them. The IPC handler then sends an aborted acknowledgement, while construction of a
replacement fence reads any remaining live marker and starts fenced.

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[94-100]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[155-157]
src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[60-64]

Agent prompt

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

## Issue description
An abort can acknowledge success while its retiring marker remains on disk. A subsequent daemon process reads that marker and resumes refusing work until the lease expires.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[94-100]
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[60-64]

## Recommended Fix
Make marker deletion success explicit. Do not report an aborted, open fence when the committed marker could not be removed; return a failure acknowledgement and retain a consistent fenced state until deletion succeeds or the lease expires. Add a restart test with deletion failure.

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

---

View medium (3)

5. A constructor comment narrates old code ✓ Resolved

Description

The new admissionFence parameter comment calls other callers pre-existing construction sites,
describing when those callers were written rather than just their current behavior. A later reader
has to interpret that historical qualifier to understand which callers receive a private fence.

Code

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[R759-760]

+            // Null in every pre-existing construction site — those get a private fence over this
+            // daemon's own marker path. DaemonRunner passes the singleton the control socket fences.

Relevance

●●● Strong

Recent review history consistently accepts removing historical or process-oriented narration from
comments.

PR-#731
PR-#878
PR-#879

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added comment uses a historical qualifier about construction sites. The checklist disallows
change-history narration in code comments.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[759-765]

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 new constructor comment describes call-site history instead of stating the current fallback behavior.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[759-765]

## Recommended Fix
Describe what happens when the optional fence is null and how the daemon supplies its shared fence, without referring to when callers were created.

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

---

6. Renaming back quickly leaves the daemon refusing work ✓ Resolved

Description

ReadMarker starts any new process under the name fenced whenever committed_at is within the
lease, and it ignores the marker's instance_id. Nothing deletes retiring.json after a successful
bootout, so a rename back to the old name, or a manual start under it, within two minutes leaves a
daemon that refuses every launch with no connection able to abort the fence.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R119-122]

+        if (_time.GetUtcNow() < committedAt + CommitLease) {
+            LogStartingFenced(committedAt);
+            return committedAt;
+        }

Relevance

●● Moderate

Marker inheritance is deliberate, but successful bootout cleanup semantics are ambiguous without a
closely matching precedent.

PR-#1268
PR-#1021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The marker survives the bootout, and ReadMarker checks only the timestamp. The constructor sets
_committedAt from the marker, so a new process starts fenced.

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[34-40]
src/Capacitor.Cli/Services/ServiceVerify.cs[1117-1124]

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 retiring marker is never removed after a successful rename. A new daemon that starts under the old name within the lease inherits the fence and refuses all work.
## Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceVerify.cs[1109-1124]
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[107-125]
## Recommended Fix
After `WaitForStopConfirmedAsync` succeeds in `RunRetireAsync`, delete `store.RetiringMarkerPath(retireId)`, ignoring IO errors.

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

---

7. Retry reports active agents on an idle daemon ✓ Resolved

Description

TryAcquire returns Busy when FencedLocked() is true, which includes a committed fence still
inside its lease, and the CLI maps that to agents_active. After a commit whose answer was lost and
could not be aborted, an immediate retry tells the user to wait for agents that do not exist, for up
to two minutes.

Code

src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[R62-65]

+            if (FencedLocked() || _inFlight > 0 || isBusy()) {
+                hold = null;
+                return AcquireResult.Busy;
+            }

Evidence

FencedLocked() returns true while a committed lease is live, so TryAcquire reports Busy. The
IPC handler sends that as busy, and ServiceVerify maps busy to agents_active.

src/Capacitor.Cli/Services/ServiceVerify.cs[1086-1089]
src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32-35]

Agent prompt

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

## Issue description
When the daemon is already fenced, `TryAcquire` returns `Busy`. The CLI then reports `agents_active` instead of `fence_unavailable`, which gives the user the wrong message.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AdmissionFence.cs[60-69]
- src/Capacitor.Cli.Daemon/Services/AdmissionFenceIpc.cs[32-35]
- src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceClient.cs[39-41]
## Recommended Fix
1. Add `AcquireResult.Fenced` and return it when `FencedLocked()` is true.
2. Have `AdmissionFenceIpc` reply with a new `fenced` reason for that result.
3. Have `AdmissionFenceClient` map the `fenced` reason to `Unavailable`.

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

---

8. Rename timeout leaves almost no headroom ✓ Resolved

Description

The 120s RenameTimeout counts only two 3s fence exchanges and an abort. It omits the new 5s
old-label query and the two 3s connect timeouts in AdmissionFenceClient, so the worst case reaches
about 118s.

Code

src/Capacitor.App/Services/KcapCli.cs[R70-72]

+    // Retiring spends up to another 20s forward budget, 10s lock wait, 5s target probe and the old
+    // daemon's fence (two 3s exchanges, plus an abort) before installation.
+    static readonly TimeSpan RenameTimeout = TimeSpan.FromSeconds(120);

Relevance

●●● Strong

Timeout arithmetic is a deterministic reliability bug; recent budget-related fixes are accepted.

PR-#1268
PR-#154

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PrepareRetireAsync calls manager.Query(retireId, TargetProbeWait) (5s). AcquireAsync opens two
connections, each with its own 3s connect timeout and 3s exchange. Commit and abort add 3s each.

src/Capacitor.Cli/Services/ServiceVerify.cs[1081-1086]
PR-#1268

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 rename timeout comment and value omit the old-label query and the connect timeouts. The worst-case CLI run leaves only about 2s of headroom.
## Fix Focus Areas
- src/Capacitor.App/Services/KcapCli.cs[70-72]
- src/Capacitor.Cli.Core/LocalIpc/AdmissionFenceClient.cs[8-47]
## Recommended Fix
List every term in the comment and raise `RenameTimeout` to about 135s, or bound the whole acquire step with a single shared deadline.

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

---

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 3356e75