Skip to content

[AI-1391] Daemon sequenced-command settlement (B2-b, daemon half) - #347

Merged
realtonyyoung merged 27 commits into
mainfrom
worktree-ai-1391-b2b
Jul 22, 2026
Merged

realtonyyoung merged 27 commits into
mainfrom
worktree-ai-1391-b2b

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

What & why

Part of AI-1391 B2-b — the bilateral sequenced daemon-command settlement protocol (parent AI-1313 §5.5/§6/§7/§8). This is the daemon (producer) half; the paired kcap-server PR consumes the wire. Rollout is daemon-first: everything here is advertised-but-inert — the SupportsSequencedCommands capability is true on DaemonConnect, but no behaviour changes until the server PR lands and starts sending sequenced commands. All wire changes are additive (an old server ignores unknown fields/methods), so either merge order is safe.

Spec: docs/superpowers/specs/2026-07-17-ai1391-sequenced-settlement-design.md. Plan (in this PR): docs/superpowers/plans/2026-07-22-ai1391-b2b-daemon-sequenced-settlement.md.

Scope (daemon producer side)

  • Additive wire DTOs (Cli.Core/Models.cs) — StopAgentV2 / CommandAck / CommandRejected / AckProcessedPrefix / AckResolvedCandidates / RequestStatusReport; ResolvedStartupCandidate / UnresolvedStartupCandidate / StartupDiscovery; and additive fields on LaunchAgentCommand (Epoch/Seq/CommandId) + DaemonConnect/DaemonStatusReport (watermark + startup-completeness + SupportsSequencedCommands). Enums pinned with [JsonStringEnumMemberName] and zero = safe default.
  • SequencedCommandProcessor — two sequenced lanes (Seq'd LaunchAgentCommand + StopAgentV2) executed strictly serially per epoch; exact-next acceptance is one atomic op under lock; LastProcessedSeq is the contiguous terminal-processed prefix; duplicates answered with CommandAck (never re-executed); wrong_next / duplicate_collision / backpressure / stale_epoch / internal_error rejections; AckProcessedPrefix retires identity-cache entries (never evicts an unacked entry). Un-Seq'd commands stay on the legacy unsequenced lane (old-server compat) and never advance the watermark.
  • ResolvedCandidatesLedger — durable positive-death-evidence outbox in the daemon state dir (atomic temp+rename, monotonic Generation, append-before-source-delete, (AgentId, OldEpoch) reconcile key). Four hooks feed it: OrphanReaper record-pass, quarantine drain, StopAgent PID-record fallback, and the env-marker recordless resolution matrix. Pruned by a validated AckResolvedCandidates.
  • Coverage boot-chain attestation — CoverageJournal (single-atomic-write genesis, cumulative_covered fold) + DaemonLock.PriorInstanceId chain-check → the fail-closed, sticky-false RecordlessSurvivorsImpossible flag advertised on connect.
  • Per-platform startup discovery — StartupReapComplete + StartupDiscovery (marker-scan state) + UnresolvedStartupCandidates; the daemon heal-barrier serves RequestStatusReport and emits CommandRejected per reason.

Deferred (noted seams, not in this PR)

  • Wait-at-capacity DaemonLaunchQueue (FIFO ticket state machine) — the spec frames it as a separate layer; an OnLaunchRequeue seam is provided.

Marker-path safety (review follow-up)

OrphanReaper.EmitAndClear now corroborates the reaped pid AND prior epoch (r.Pid == c.Pid && r.DaemonEpoch == c.OldEpoch) before trusting/deleting a durable record on the marker path — the env agentId is untrusted for role authority, so a prior-epoch descendant that inherited a still-live leader's env can no longer emit a false trusted-flow death proof for (or delete the record of) the live leader. Non-corroborating matches fall back to the recordless null-flow path with no record delete.

Testing

Capacitor.Cli.Tests.Unit full suite green — total 3663, failed 0, 2 skipped (a gated live-ACP E2E). New coverage across the processor, ledger + 4 hooks, coverage journal, DaemonLock prior-instance chain, heal-barrier report, marker-candidate resolution matrix, startup completeness, and the wire DTOs. Tests use isolated dummy processes only — no real daemon, no live flows. The Linux-only marker-scan matrix is exercised under Linux CI.

Compatibility

Fully additive; SupportsSequencedCommands is the single gate. An old server ignores the new fields/methods and the daemon behaves exactly as before. No projection or read-model changes.

🤖 Generated with Claude Code

realtonyyoung and others added 20 commits July 22, 2026 14:22
…w-round-1 applied

Implementation plan for the daemon (kcap-cli) half of B2-b: 17 TDD tasks (wire DTOs,
sequenced lanes+watermark, resolved-candidates ledger + 4 crash-consistent hooks,
coverage-journal/boot-chain attestation, per-platform StartupReapComplete, daemon
heal-barrier). Authored + adversarially reviewed + revised (genesis atomicity, compile
defects, full crash-injection matrix, synthesized-item monotonicity, racing-liveness).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…artup discovery)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ks, RequestStatusReport)

StopAgentV2 + CommandAck/CommandRejected (+ their reason/state/outcome/liveness enums),
AckProcessedPrefix, AckResolvedCandidates (+ ResolvedCandidateAck), RequestStatusReport,
and the additive LaunchAgentCommand Epoch?/Seq?/CommandId? sequencing fields. Registered
on CapacitorJsonContext, snake_case, enum tokens pinned with safe zero-defaults; raw-JSON
round-trip asserts. All additive. Phase B2-b (sequenced-settlement design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Open the per-name lock file FileAccess.ReadWrite and, immediately after
inspecting the leftover PID file (while holding the exclusive flock, so the
prior holder is provably gone), read the previous holder's InstanceId from the
lock-file content BEFORE truncating and rewriting our own. Exposed as the new
DaemonLock.PriorInstanceId (null on a genuinely fresh lock / unreadable
content; read failures fail-closed to null).

Unlike the PID file (deleted on clean shutdown), the lock file's InstanceId is
the persistent per-boot nonce every shipped version rewrites at boot and never
deletes, so it witnesses the immediately-preceding boot even one by an unaware
binary. Phase B2-b (sequenced-settlement design) uses it as the coverage
boot-chain's chain-check. Additive and unused until later tasks consume it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…testation)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ssible

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pend-before-delete)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…es ledger

Phase B2-b (sequenced-settlement design): wire the real ResolvedCandidatesLedger
into AgentOrchestrator and add the two remaining confirmed-gone hooks, both
append-before-delete and idempotent on the source-stable (AgentId, OldEpoch) key:

- Hook B (quarantine drain, RetryQuarantineOnceAsync): emit (AgentId, _daemonEpoch,
  flow...) before deleting each drained entry's PID record. RetryAllAsync now returns
  IReadOnlyList<Entry> so the drain has the flow identity.
- Hook C (StopAgent fallback, TryStopByPidRecordAsync): emit (agentId,
  record.DaemonEpoch, record.flow...) from the trusted record before the delete.

Also wires the Task 7 OrphanReaper record-pass callback (Hook A) to the ledger and
adds ResolvedLedgerSnapshotForTest / QuarantineForTest seams. Shipped teardown/
quarantine/orphan-reap semantics are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… into the ledger

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rune

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vedStartupCandidates

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ontiguous watermark, synthesized-error item)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…collisions

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re (no unacked eviction)

Backpressure guard in the accept path: once the per-epoch identity cache reaches its
bound, further exact-next accepts are rejected with CommandRejectedReason.Backpressure
rather than evicting an unacked entry. AckPrefix retires cache entries through a VALIDATED
AckProcessedPrefix (current epoch, not over-ahead of LastProcessedSeq, strictly monotonic);
stale-epoch / over-ahead / regressing acks are ignored without eviction. 2 tests. Phase B2-b.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y/semantic rejections

HandleNonNextLocked now emits CommandRejected{wrong_next} for a gap/too-low Seq (never
accepted out of order; the server transport resyncs) — accept path + watermark untouched.
Execution-time terminal rejections (daemon_capacity/semantic) already flow via the lane's
LaunchRejected+RejectReason mapping and advance the watermark as a terminal item. 2 tests. Phase B2-b.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lity, RequestStatusReport

Phase B2-b (sequenced-settlement design): own an epoch-scoped SequencedCommandProcessor
in AgentOrchestrator; route Seq'd LaunchAgentCommand/StopAgentV2 through its serial lane
(the execute closure returns a CommandOutcome — LaunchRejected+daemon_capacity/semantic
where the shipped launch would reject, LaunchExecuted/LaunchFailedCleaned/StopExecuted
otherwise), keeping un-Seq'd commands on the legacy unsequenced lane. Add ReadLiveness
(confirmed-death precedence over _agents ∪ _quarantine), advertise Epoch/HighestAcceptedSeq/
LastProcessedSeq counters on BuildStatusReport + the enriched DaemonConnect payload, set
SupportsSequencedCommands=true, and serve StopAgentV2/AckProcessedPrefix/RequestStatusReport
plus the one-way CommandAck/CommandRejected sends.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r advances watermark

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OrphanReaper.EmitAndClear looked up a durable record by env-derived agentId
alone. A prior-epoch descendant that inherited a still-live leader X's
KCAP_AGENT_ID env triple, reaped under a different pid, would match X's leader
record — emitting a false trusted-flow death proof for the LIVE leader and
deleting X's record (stranding it). The lookup was also unscoped by epoch.

Now corroborate the reaped pid AND the prior epoch (r.Pid == c.Pid &&
r.DaemonEpoch == c.OldEpoch) before trusting/deleting the record; a
non-corroborating record falls back to the recordless null-flow path with no
record delete. The legitimate identity_unavailable case (record genuinely IS
the reaped pid) is unaffected. Regression test added.

Phase B2-b (sequenced-settlement design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…donly, SafeInvoke

- Fix 2: OrphanReaper.CurrentDiscovery is a multi-field struct written on the
  reap thread and read on the report/connect thread; guard get/set with a
  dedicated gate so the read can't tear (sweep is single-writer, so no wider
  critical section is needed).
- Fix 3: advertise the DaemonConnect epoch from the orchestrator's own per-boot
  _daemonEpoch via a GetDaemonEpoch seam (the single source the processor is
  scoped to), falling back to _config.DaemonEpoch when unwired — removes the
  test-divergence footgun; prod behaviour unchanged (DaemonRunner pins config).
- Fix 4: mark _resolvedLedger / _markerCandidates / _processor readonly.
- Fix 5: route the RequestStatusReport receive through SafeInvoke like the other
  command handlers.
- Fix 6: drop the dead report/expected locals in the startup-discovery test.

Phase B2-b (sequenced-settlement design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 22, 2026

Copy link
Copy Markdown

AI-1391

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

AI-1391 B2-b: daemon-side sequenced-command settlement (producer half)

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Adds the daemon (producer) half of sequenced-command settlement, gated by
 SupportsSequencedCommands.
• Introduces additive wire DTOs and report/connect fields; old servers safely ignore them.
• Implements serial per-epoch sequenced processing and durable death-evidence reporting with crash
 safety.
Diagram

graph TD
  Server["kcap-server (peer)"] -->|"StopAgentV2 / AckProcessedPrefix"| Conn["ServerConnection"] --> Orch["AgentOrchestrator"]
  Orch --> Proc["SequencedCommandProcessor"] --> Conn
  Orch --> Reaper["OrphanReaper"] --> Ledger[("ResolvedCandidatesLedger")]
  Orch --> Quarantine["AgentKillQuarantine"] --> Ledger
  Orch --> Journal[("CoverageJournal")]
  Ledger --> Conn -->|"DaemonConnect / StatusReport"| Server

  subgraph Legend
    direction LR
    _svc(["Service"]) ~~~ _db[("Durable store")] ~~~ _ext["External peer"]
  end
Loading
High-Level Assessment

The additive, capability-gated rollout (SupportsSequencedCommands advertised but behavior inert) is the safest approach for a bilateral protocol change and avoids lockstep deployment. The implementation reuses proven patterns in this repo (atomic temp+rename durable stores; delegate-injected, unit-testable services) and matches the spec’s constraints (exact-next acceptance, deduped replays, contiguous watermark, append-before-delete crash consistency).

Files changed (23) +5000 / -50

Enhancement (11) +1210 / -44
Models.csAdd additive sequenced-settlement wire DTOs and fields +205/-3

Add additive sequenced-settlement wire DTOs and fields

• Adds new additive DTOs/enums for sequenced settlement (StopAgentV2, CommandAck/Rejected, AckProcessedPrefix, AckResolvedCandidates, startup discovery/candidates, RequestStatusReport) and appends optional sequencing/heal-barrier fields to LaunchAgentCommand, DaemonConnect, and DaemonStatusReport. Enums pin wire tokens with safe zero defaults.

src/Capacitor.Cli.Core/Models.cs

DaemonConfig.csAdd RecordlessSurvivorsImpossible config flag +13/-1

Add RecordlessSurvivorsImpossible config flag

• Adds a config flag carrying the durable coverage boot-chain verdict so it can be advertised on DaemonConnect and used in startup completeness decisions.

src/Capacitor.Cli.Daemon/DaemonConfig.cs

DaemonLock.csExpose PriorInstanceId from lockfile content +33/-5

Expose PriorInstanceId from lockfile content

• Changes lock acquisition to read the previous holder’s InstanceId from the lock file before overwriting it, exposing PriorInstanceId for boot-chain attestation.

src/Capacitor.Cli.Daemon/DaemonLock.cs

DaemonRunner.csPin daemon epoch and record coverage journal before services +15/-0

Pin daemon epoch and record coverage journal before services

• Pins DaemonEpoch before building services and records the CoverageJournal fold (using InstanceId + PriorInstanceId) before any Connect/spawn, fail-closed to false.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

AgentOrchestrator.csIntegrate sequenced processor, ledger hooks, and completeness reporting +272/-16

Integrate sequenced processor, ledger hooks, and completeness reporting

• Instantiates SequencedCommandProcessor, ResolvedCandidatesLedger, and MarkerCandidateStore; wires new server events; routes fully-sequenced LaunchAgentCommand and StopAgentV2 through the serial lane while keeping legacy commands unsequenced. Adds startup completeness rollup, discovery reporting, liveness read for duplicate acks, ledger hook emission (append-before-delete), and multiple test seams.

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

CoverageJournal.csAdd durable boot-chain attestation journal +82/-0

Add durable boot-chain attestation journal

• Introduces CoverageJournal with a single atomic JSON write that folds 'cumulative_covered' via chain-checking lock InstanceIds. Implements fail-closed and sticky-false semantics to prevent laundering through unaware boots.

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

MarkerCandidateStore.csAdd durable marker-candidate source persistence +53/-0

Add durable marker-candidate source persistence

• Adds MarkerCandidateStore for recordless survivor candidates with hashed filenames and atomic temp+rename writes, supporting crash-consistent marker-based resolution.

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

OrphanReaper.csAdd marker-candidate reconciliation + startup discovery surfaces +169/-18

Add marker-candidate reconciliation + startup discovery surfaces

• Adds marker-candidate reconciliation prior to the record pass, an asymmetric resolution matrix (dead/kill-confirmed/spare), CurrentDiscovery status updates, and BlockedCandidates reporting for startup completeness. Emits resolved evidence via injected sinks before deleting sources.

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

ResolvedCandidatesLedger.csAdd durable resolved-candidates outbox ledger +82/-0

Add durable resolved-candidates outbox ledger

• Adds a durable, atomic persisted ledger keyed by (AgentId, OldEpoch) with monotonic Generation, ordered snapshots for reporting, and sparse per-entry pruning via AckResolvedCandidates.

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

SequencedCommandProcessor.csAdd per-epoch serial sequenced command processor +187/-0

Add per-epoch serial sequenced command processor

• Implements exact-next acceptance, dedupe-by-(Seq,CommandId), contiguous terminal watermark advancement, and backpressure via bounded identity cache. Supports retirement via validated AckProcessedPrefix and emits CommandAck/CommandRejected one-way to the server.

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

ServerConnection.csAdd sequenced settlement hub events and enriched connect payload +99/-1

Add sequenced settlement hub events and enriched connect payload

• Adds server→daemon receive seams (StopAgentV2, AckProcessedPrefix, RequestStatusReport, AckResolvedCandidates) and daemon→server one-way sends (CommandAck/CommandRejected). Enriches DaemonConnect with epoch/watermark/startup-completeness/ledger/capability fields while remaining wire-compatible.

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

Refactor (1) +8 / -6
AgentKillQuarantine.csReturn drained quarantine entries with flow identity +8/-6

Return drained quarantine entries with flow identity

• Adjusts RetryAllAsync to return drained Entry objects rather than agent ids so callers can ledger-append trusted flow identity before PID-record deletion.

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

Tests (10) +1007 / -0
CoverageJournalTests.csAdd CoverageJournal boot-chain coverage tests +89/-0

Add CoverageJournal boot-chain coverage tests

• Tests genesis seeding, chain continuity, downgrade-sandwich detection, sticky-false propagation, corrupt state handling, and crash-before-rename behavior.

test/Capacitor.Cli.Tests.Unit/Daemon/CoverageJournalTests.cs

DaemonLockPriorInstanceIdTests.csAdd tests for PriorInstanceId behavior +37/-0

Add tests for PriorInstanceId behavior

• Verifies PriorInstanceId is null for a fresh slot and equals the previous boot’s InstanceId on re-acquire.

test/Capacitor.Cli.Tests.Unit/Daemon/DaemonLockPriorInstanceIdTests.cs

HealBarrierReportTests.csAdd sequencing/heal-barrier report and liveness-race tests +158/-0

Add sequencing/heal-barrier report and liveness-race tests

• Validates report advertises epoch/watermark counters, ReadLiveness ordering prevents transient false-dead, sequenced capacity rejection emits daemon_capacity and advances watermark, StopAgentV2 advances watermark and removes confirmed-dead ids, and legacy unsequenced launches do not advance the watermark.

test/Capacitor.Cli.Tests.Unit/Daemon/HealBarrierReportTests.cs

LedgerHookOrchestratorTests.csAdd orchestrator-level ledger hook coverage +39/-0

Add orchestrator-level ledger hook coverage

• Adds unit tests exercising resolved-candidates ledger behavior as wired through AgentOrchestrator and ServerConnection seams.

test/Capacitor.Cli.Tests.Unit/Daemon/LedgerHookOrchestratorTests.cs

LedgerHookTests.csAdd crash-consistency tests for ledger feed hooks +178/-0

Add crash-consistency tests for ledger feed hooks

• Covers record-pass, quarantine-drain, and StopAgent fallback hooks including crash-before-append and crash-between-append-and-delete reconciliation ensuring idempotent single-emission.

test/Capacitor.Cli.Tests.Unit/Daemon/LedgerHookTests.cs

MarkerCandidateResolutionTests.csAdd tests for marker-candidate resolution matrix +179/-0

Add tests for marker-candidate resolution matrix

• Adds tests validating the marker-candidate spare/kill/resolve matrix and corroboration rules that prevent trusting mutable env data for flow identity.

test/Capacitor.Cli.Tests.Unit/Daemon/MarkerCandidateResolutionTests.cs

ResolvedCandidatesLedgerTests.csAdd resolved ledger persistence and pruning tests +46/-0

Add resolved ledger persistence and pruning tests

• Tests upsert idempotency, snapshot ordering, ack pruning, and persistence semantics for ResolvedCandidatesLedger.

test/Capacitor.Cli.Tests.Unit/Daemon/ResolvedCandidatesLedgerTests.cs

SequencedCommandProcessorTests.csAdd SequencedCommandProcessor correctness tests +133/-0

Add SequencedCommandProcessor correctness tests

• Tests serial execution, out-of-order rejection, fault→internal_error, synthesized terminal monotonicity under shutdown races, duplicate ack behavior, backpressure reopening via AckProcessedPrefix, and stale/regressing ack handling.

test/Capacitor.Cli.Tests.Unit/Daemon/SequencedCommandProcessorTests.cs

SequencedSettlementWireTests.csAdd wire token pinning and DTO round-trip tests +81/-0

Add wire token pinning and DTO round-trip tests

• Pins enum wire tokens and asserts snake_case JSON round-trips for new DTOs and additive fields, including conservative zero defaults (e.g., MarkerScanState.Pending).

test/Capacitor.Cli.Tests.Unit/Daemon/SequencedSettlementWireTests.cs

StartupCompletenessTests.csAdd startup-completeness signal tests +67/-0

Add startup-completeness signal tests

• Tests RecordlessSurvivorsImpossible surfacing, per-platform StartupDiscovery behavior, pending marker candidates blocking completion, and resolved-candidate ack pruning via report/connect surfaces.

test/Capacitor.Cli.Tests.Unit/Daemon/StartupCompletenessTests.cs

Documentation (1) +2775 / -0
2026-07-22-ai1391-b2b-daemon-sequenced-settlement.mdAdd B2-b daemon implementation plan (17 TDD tasks) +2775/-0

Add B2-b daemon implementation plan (17 TDD tasks)

• Adds the detailed daemon-side implementation plan for B2-b, including adversarial-review revisions around atomicity, crash injection, and monotonicity hazards.

docs/superpowers/plans/2026-07-22-ai1391-b2b-daemon-sequenced-settlement.md

@qodo-code-review

qodo-code-review Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Coverage genesis misclassified on error ✓ Resolved 🐞 Bug ≡ Correctness
Description
CoverageJournal treats priorLockInstanceId == null as proof of a fresh genesis slot, but
DaemonLock sets PriorInstanceId to null on any read failure, conflating “empty file” with
“unreadable file”. This can incorrectly compute RecordlessSurvivorsImpossible=true in cases that
should fail-closed to false.
Code

src/Capacitor.Cli.Daemon/Services/CoverageJournal.cs[R47-54]

+            if (!journalExists) {
+                // Genesis is the ONLY seed-true base case: the journal file is absent (we are about to
+                // atomically initialize it) AND the captured prior lock shows no pre-existing InstanceId
+                // (a genuine first-ever boot for this name). A previously-used dir whose journal is gone
+                // but whose prior lock DOES carry an InstanceId is un-journaled history that re-pointing/
+                // deleting the dir cannot launder ⇒ false.
+                var genesis = priorLockInstanceId is null;
+                covered = genesis && thisEpochContained;
Evidence
CoverageJournal’s genesis logic is priorLockInstanceId is null, but DaemonLock’s read path
explicitly sets priorInstanceId = null in its catch-all error handler. That means read/parsing
failures can be misinterpreted as a clean genesis slot.

src/Capacitor.Cli.Daemon/Services/CoverageJournal.cs[41-60]
src/Capacitor.Cli.Daemon/DaemonLock.cs[114-127]

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

### Issue description
`CoverageJournal.RecordBoot` uses `priorLockInstanceId is null` as the genesis predicate. However, `DaemonLock.TryAcquire` assigns `PriorInstanceId = null` both when the lock file is truly empty and when reading/parsing fails. That breaks the journal’s “fail-closed” guarantee by potentially treating an unreadable prior instance id as a clean genesis.

### Issue Context
This affects the computed `DaemonConfig.RecordlessSurvivorsImpossible` advertised on connect/status. The design intent (per comments) is fail-closed: any missing/corrupt/unwritable state should evaluate to false.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/DaemonLock.cs[114-127]
- src/Capacitor.Cli.Daemon/Services/CoverageJournal.cs[41-60]

### Suggested fix
- Represent the prior-lock read as a 3-state value: {Empty, ReadOk(value), ReadFailed}.
 - Option A: add a `bool PriorInstanceIdReadOk` (or `PriorInstanceIdState`) to `DaemonLock`.
 - Option B: change `PriorInstanceId` type to something like `string?` plus a separate `bool priorKnown`.
- In `CoverageJournal.RecordBoot`, require an explicit “empty lock content” signal for genesis; if the read failed/unknown, force `covered = false` (fail-closed).
- Keep the chain-check (`priorLockInstanceId == tail.InstanceId`) unchanged when the read succeeded.

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


2. Lock InstanceId read breaks build ✓ Resolved 🐞 Bug ≡ Correctness
Description
DaemonLock.TryAcquire allocates new byte[stream.Length] where stream.Length is a long, which
does not compile and also enables unbounded allocation if the lock file is corrupted/oversized. This
blocks the build and makes startup vulnerable to memory pressure when reading the lock file.
Code

src/Capacitor.Cli.Daemon/DaemonLock.cs[R119-123]

+            if (stream.Length > 0) {
+                stream.Position = 0;
+                var buf = new byte[stream.Length];
+                var n = stream.Read(buf, 0, buf.Length);
+                priorInstanceId = System.Text.Encoding.UTF8.GetString(buf, 0, n)
Evidence
The new code reads the lock file by allocating a byte[] sized by FileStream.Length, which is a long
and cannot be used as an array length in C#. It also reads the whole file into memory even though
the file is expected to be a single short line.

src/Capacitor.Cli.Daemon/DaemonLock.cs[114-127]

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

### Issue description
`DaemonLock.TryAcquire` currently reads the previous lock-holder instance id by allocating a byte buffer sized to `stream.Length` (a `long`). This is a compile-time error in C# (array length must be `int`-compatible), and even after fixing the type it would still be risky to allocate based on an untrusted on-disk length.

### Issue Context
The lock file is expected to contain a single short line (`InstanceId + "\n"`). The implementation should read only the first line (bounded) and avoid large allocations. This prior instance id is also used downstream by `CoverageJournal.RecordBoot`, so this read should be robust.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/DaemonLock.cs[114-127]

### Suggested fix
- Replace the buffer allocation with a bounded first-line read (e.g., `StreamReader.ReadLine()` with `leaveOpen:true`, or manual small-buffer scanning until `\n` with a max length).
- Ensure you handle partial reads correctly (don’t assume a single `Read` fills the buffer).
- Keep `PriorInstanceId` as `null` only for a genuinely empty file; treat read errors distinctly (see related CoverageJournal finding).

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



Remediation recommended

3. Ledger persistence exceptions unhandled ✓ Resolved 🐞 Bug ☼ Reliability
Description
ResolvedCandidatesLedger mutates in-memory state and then calls Persist() with no exception
handling, so disk I/O failures can throw into orchestrator control flow and potentially leave
memory/disk out of sync. Additionally, the SignalR AckResolvedCandidates receive handler is
registered without SafeInvoke/try-catch, so exceptions during ack processing are not contained at
the hub boundary.
Code

src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs[R76-81]

+    void Persist() {
+        var tmp = _path + ".tmp-" + Guid.NewGuid().ToString("N")[..8];
+        File.WriteAllText(tmp, JsonSerializer.Serialize(
+            new Persisted(_nextGeneration, [.. _entries.Values]), LedgerJsonCtx.Default.Persisted));
+        File.Move(tmp, _path, overwrite: true); // atomic same-directory rename
+    }
Evidence
The ledger’s Upsert/Ack mutate state then call Persist(), which performs WriteAllText/Move without
handling exceptions. The orchestrator directly calls ledger.Ack from its ack handler, and the hub
registration invokes that handler inline without SafeInvoke, so any thrown exception can propagate
out of the invocation callback.

src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs[31-40]
src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs[76-81]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[614-617]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[206-217]

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

### Issue description
`ResolvedCandidatesLedger.Upsert` and `Ack` both modify `_entries` / `_nextGeneration` and then call `Persist()` which performs file writes/move without try/catch. If any filesystem operation fails (disk full, permission, transient IO), the exception will propagate to callers, and the ledger can become inconsistent (memory already changed, disk not updated).

Also, `ServerConnection` registers the `AckResolvedCandidates` hub method without `SafeInvoke`, so exceptions from the orchestrator ack path aren’t contained/logged consistently at the hub boundary.

### Issue Context
The ledger is used in multiple “confirmed gone” hooks and is also pruned via a server→daemon Ack. These paths should not be able to destabilize the daemon when persistence fails; they should either fail closed (keep entries and retry) or degrade gracefully while logging.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs[31-55]
- src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs[76-81]
- src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[206-217]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[614-617]

### Suggested fix
- Make persistence failure-safe:
 - Compute the new state first, persist it successfully, then swap/commit it in-memory; or
 - Wrap `Persist()` in try/catch and, on failure, revert in-memory mutations (or don’t apply them until persist succeeds).
- Ensure `AckResolvedCandidates` handling cannot throw out of the SignalR dispatcher:
 - Wrap `OnAckResolvedCandidates?.Invoke(ack)` in `SafeInvoke(...)` or a local try/catch that logs and returns `Task.CompletedTask`.
- Consider logging at warning level with enough context (count of entries, ack generations) for diagnosis.

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


4. Verbose B2-b comment blocks ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
New/modified comment blocks are lengthy and include extensive design-rationale that makes the code
harder to scan and maintain. This conflicts with the guideline to keep comments minimal and prefer
self-explanatory code.
Code

src/Capacitor.Cli.Daemon/Services/SequencedCommandProcessor.cs[R15-21]

+/// <summary>
+/// Phase B2-b (sequenced-settlement design §4.2.2; parent §5.5): the daemon's two-lane sequenced
+/// command handler. Exactly two command types are sequenced (Seq'd LaunchAgentCommand + StopAgentV2),
+/// executed strictly serially per epoch. Acceptance (bump HighestAcceptedSeq + cache entry + enqueue)
+/// is one atomic operation under <c>_lock</c>; LastProcessedSeq is the contiguous terminal prefix
+/// (advances only on a terminal outcome). Self-contained + delegate-injected so it is unit-testable
+/// with no live orchestrator (mirrors OrphanReaper/AgentKillQuarantine).
Evidence
PR Compliance ID 6 requires concise, necessary comments. The added XML/doc comments in
SequencedCommandProcessor and DaemonConfig are long, design-heavy explanations that could be
shortened and/or moved to the referenced design docs.

CLAUDE.md: Keep Comments Minimal; Prefer Self-Explanatory Code
src/Capacitor.Cli.Daemon/Services/SequencedCommandProcessor.cs[15-21]
src/Capacitor.Cli.Daemon/DaemonConfig.cs[37-44]

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

## Issue description
Several new/modified comments are overly verbose (multi-line design rationales and protocol narration). This reduces readability; prefer shorter summaries and rely on the spec/plan docs for detailed rationale.

## Issue Context
Compliance rule requires keeping comments minimal and favoring self-explanatory code.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/SequencedCommandProcessor.cs[15-21]
- src/Capacitor.Cli.Daemon/DaemonConfig.cs[37-44]

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


Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Daemon/Services/SequencedCommandProcessor.cs
Comment thread src/Capacitor.Cli.Daemon/DaemonLock.cs
Comment thread src/Capacitor.Cli.Daemon/Services/CoverageJournal.cs
Comment thread src/Capacitor.Cli.Daemon/Services/ResolvedCandidatesLedger.cs Outdated
realtonyyoung and others added 6 commits July 22, 2026 14:51
…nt §5.5)

OrphanReaper.BlockedCandidates() only listed IdentityUnavailable records and
the macOS legacy-live case. A Present prior-epoch record the record pass
spared (a transient ambiguous identity read on Linux, or a record-pass fault)
stayed on disk yet unlisted, so ComputeStartupReapComplete() could return true
while a prior-epoch process may still be alive — the paired server would treat
the omission as proof of death and launch a duplicate.

Add a third arm: any other prior-epoch record still on disk is unresolved
(confirmed-dead records are deleted at reap time), so it blocks as
identity_unresolvable. A record whose process dies between passes blocks only
until the next reap tick deletes it — a false-incomplete only delays relaunch,
never mints a duplicate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A capacity/semantic launch rejection is cached as LaunchRejected with its
RejectReason, but the processed-duplicate CommandAck was built without
RejectionReason. A retransmitted duplicate of a rejected launch therefore
couldn't distinguish daemon_capacity (requeue) from semantic (fail) — breaking
the per-reason ticket semantics for exactly the lost-rejection case the
identity cache exists to answer.

Pass the cached reason on the processed ack, serialized through the same
CapacitorJsonContext the SignalR hub uses for CommandRejected.Reason, so the
ack's RejectionReason string equals that enum's [JsonStringEnumMemberName]
snake_case wire token (daemon_capacity / semantic).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HandleLaunchAgent routed to the processor only when Epoch/Seq/CommandId were
ALL present; any partial tuple fell through to the unwatermarked legacy lane,
where a malformed/truncated capable-server command's retry could be re-accepted
on the sequenced lane and double-create the generation (violating
at-most-once-per-generation).

Split routing three ways: none present -> legacy lane (old server); all three
present -> sequenced lane; anything in between -> fail closed with a
LaunchFailed, never the legacy lane. The watermark is untouched and nothing
spawns.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rite (§5.5)

Upsert mutated _entries and _nextGeneration in memory BEFORE Persist(). If
Persist threw, memory led disk: the next sweep's Upsert returned the
unpersisted in-memory entry via the idempotent short-circuit WITHOUT
persisting, and the caller then deleted the durable source — a crash could
then lose both (violating append-before-delete).

Persist the prospective state (post-increment high-water) BEFORE committing
the counter, and roll back the in-memory entry on a write failure. Refactor
Persist() into PersistState(nextGeneration); Ack keeps the plain Persist()
(its memory-behind-disk direction is benign — re-loads + re-acks idempotently).

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

The report/connect payloads carried the resolved-candidates array but not the
daemon-lifetime monotonic high-water, so once sparse acks prune entries the
server lost the generation frontier.

Add an optional long? HighestResolutionGeneration to DaemonStatusReport and
DaemonConnect (additive, snake_case), a ledger high-water property
(_nextGeneration - 1; persists across prunes/restarts), and wire it into
BuildStatusReport plus a new GetHighestResolutionGeneration ServerConnection
seam on the connect payload (mirroring the GetResolvedStartupCandidates
pattern). DaemonConnect already advertises ResolvedStartupCandidates, so the
high-water goes on connect too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The env-marker scan discovers a recordless prior-epoch survivor, persists a
durable marker-candidate source, then kills it. The per-PID try/catch logged and
continued on ANY failure, then the pass unconditionally published
MarkerScanState.Complete. If the source WRITE fails (disk-full / permission /
I/O), the survivor is neither recorded (invisible to BlockedCandidates) nor
killed — yet the scan reported Complete, so with the spared-record completion fix
StartupReapComplete could go true beside a live prior-epoch survivor and the
paired server would launch a duplicate.

Split the write from the resolution: a WRITE failure sets captureFailed and skips
the kill (never resolve without a durable source), leaving discovery Failed for
the pass (retried next heartbeat, last-successful-scan time preserved). A failure
AFTER a successful write is unchanged — the pending_marker source blocks via
BlockedCandidates and the next boot re-derives + retries the kill.

Linux-gated regression test: a MarkerCandidateStore whose state dir is a file
(every Write throws) + a live prior-epoch survivor -> discovery stays Failed and
the survivor is not killed.

Phase B2-b (sequenced-settlement design §5.5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s (§4.2.3)

DaemonLock returned null PriorInstanceId for BOTH a genuinely-empty lock file
(a real first-ever/genesis slot) and a read failure on a non-empty file, and
CoverageJournal.RecordBoot treated `priorLockInstanceId is null` as genesis-
eligible. So an absent journal + a corrupt/unreadable non-empty lock file wrongly
attested RecordlessSurvivorsImpossible=true — violating the boot-chain's
fail-closed invariant (any missing/corrupt state ⇒ false).

- DaemonLock now exposes `PriorLockIndeterminate`: true iff the lock file was
  non-empty but yielded no id (a read fault OR blank/garbage). A genuinely empty
  file stays null + not-indeterminate (genesis-eligible).
- RecordBoot takes `priorLockReadFailed`; genesis is now
  `!priorLockReadFailed && priorLockInstanceId is null`, so an indeterminate
  prior fails closed instead of being mistaken for genesis.
- The lock-file read is now bounded (`new byte[(int)Math.Min(stream.Length,
  4096)]`) so a corrupt/oversized file can't drive an unbounded allocation (the
  content is a 32-char GUID + newline). ('new byte[long]' already compiled — the
  reviewer's 'breaks build' note was a false positive; this addresses the valid
  unbounded-allocation half.)

Tests: DaemonLock indeterminate flag (non-empty-unreadable ⇒ indeterminate;
fresh ⇒ not) + RecordBoot fail-closed on read-failed prior vs genesis on a
genuinely empty prior.

Phase B2-b (sequenced-settlement design §4.2.3).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit f15c152 into main Jul 22, 2026
10 of 11 checks passed
@realtonyyoung
realtonyyoung deleted the worktree-ai-1391-b2b branch July 22, 2026 19:47
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.

1 participant