Resolves #15763

The tenant-repo lane's two entry paths — the orchestrator's periodic sweep and the manual syncTenantRepos.mjs CLI — now serialize through one dedicated cross-process lease and commit the revisions manifest atomically. The lease is a tokenized sibling file of the manifest (lock and data share one persistence/recovery boundary on the volume shipped by #15764), acquired inside runTask — the single chokepoint both paths share — via the existing heavy-maintenance lease primitives with a dedicated identity. Contention is a bounded non-failure: the periodic sweep defers with KB_TENANT_REPO_SYNC_LEASE_HELD and zero checkpoint/backoff mutation; the CLI exits deterministically with new code 4 plus an owner/expiry hint. Crashed owners recover instantly via pid-liveness. writePersistedRevisions becomes temp-sibling + fsync + rename, so a crash mid-write can no longer produce the torn manifest that the strict reader (since #15761) fail-closes the whole lane on.

The exclusivity contract after the cycle-3 review is one serialized transition mechanism, identity-safe in its own recovery, plus work-level renewal. Every read-verify-mutate transition on an existing lease record — stale/malformed recovery, token-guarded release, renewal — executes inside a short-lived lifecycle guard and acts only on state re-observed INSIDE the guard, never on an earlier observation. Guard entry is atomic-with-identity: the entrant stages a directory carrying its unique owner-<token> file and rename()s it onto the canonical guard name, which POSIX refuses when the target is non-empty — a live guard can never be replaced. Abandoned-guard recovery consumes exactly the OBSERVED artifacts: unlink of the specific observed owner filename (ENOENT ⇒ observed guard gone ⇒ abort) then rmdir (ENOTEMPTY ⇒ someone re-entered ⇒ abort) — the cycle-3 reviewer-reproduced pathname-based removal of a replacement guard is structurally impossible, and the deterministic two-contender regression proves exactly one entrant/acquirer in that schedule (async + sync). Every lease mutation inside the section re-verifies live guard ownership immediately before acting, so an evicted stalled holder DEFERS (guardEvicted) instead of mutating its successor's state. Plain acquisition stays an atomic exclusive create deferring via EEXIST. A running sweep renews its own lease every max(5s, TTL/3), so a live owner never reaches its deadline mid-work; work fences before each repo's git phase, each KB ingest, and every manifest commit abort a de-owned run with KB_TENANT_REPO_SYNC_LEASE_LOST — checkpoints, backoff state, and the successor's lease stay untouched, and no partial sweep is committed. pid-liveness recovers crashes instantly; the TTL remains the backstop for an owner that stopped renewing. Precise residual bound: a holder stalled past guardStaleAfterMs (default 10s vs µs-scale guarded sections) is legitimately evictable; the pre-mutation ownership probe narrows its exposure to the probe's own single stat→syscall gap, reachable only by a holder that both stalled past the threshold and resumed inside that gap — live transitions can be neither evicted nor replaced. Manifest writes keep the full-write writeFile contract before fsync + rename.

Evidence: L2 (real-filesystem lease/manifest unit coverage: gated concurrent two-writer witness; two-reclaimer guard interleave; replacement-after-observation regressions, async + sync; replacement-during-release regressions, async + sync; a recovery-vs-release guard-serialization interleave; the cycle-3 two-contender abandoned-guard interleave proving exactly one entrant; the evicted-stalled-holder probe witness; sync identity-safe steal abort; abandoned-guard steal-path reclaim + interrupted-entry empty-guard replacement + live contended-guard deferral; renewal keeping a live owner past its base TTL; renewal-failure fail-closed abort; mid-sweep eviction fence witness; fault-injected partial-write regression) → L2 required (the close-target ACs are cross-process state-machine and persistence contracts). Residual: none.

Deltas from ticket

• Immediate-busy instead of a bounded CLI wait: the ticket allowed either; immediate busy (exit 4 + stderr owner/expiry hint) is simpler, timer-free, and matches the fail-fast operator model — re-running is the retry.
• The lease path derives from the resolved revisions-manifest path (sibling file) rather than a separate config leaf: lock and data provably share one volume/recovery boundary, and every test that isolates the manifest automatically isolates the lease.
• Lease-acquire IO failures (unwritable state dir, broken volume) return the lane's structured failed result (phase: lease-acquire) instead of escaping as a raw throw.
• One pre-existing fixture retargeted: the deep-error-propagation test forced MANIFEST_UPDATE_FAILED via an unwritable parent directory, which now (correctly) fails earlier at lease creation; it forces the same deep error via a directory squatting on the exact temporary-sibling path — same assertions, deterministic, no chmod juggling.
• Troubleshooting gains the torn-manifest last-resort recovery line the PR #15766 review fed into this ticket (deleting the manifest is safe at full re-ingestion cost; never hand-edit it).
• Renewal cadence is a derivation from the existing TTL leaf (max(5s, TTL/3)), not a new config leaf; the guard staleness threshold is a primitive-internal mechanism constant with a test-seam override (guardStaleAfterMs), not policy. No AiConfig surface added in cycle 2.
• Lease loss mid-sweep is a run-level abort, not a per-repo failure: aborted repos keep their checkpoint/attempt/backoff state byte-identical (status: 'aborted-lease-lost' in the run details), because the successor now owns forward progress.

Test Evidence

• npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/services/TenantRepoSyncService.spec.mjs — 69 passed. Cycle-2 additions: renewal keeps a live long-running owner past its base TTL (contender still observes an active hold); renewal-failure fail-closed abort (failed run, KB_TENANT_REPO_SYNC_LEASE_LOST, no manifest ever committed, backoff untouched, replacement lease intact).
• npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/services/HeavyMaintenanceLeaseService.spec.mjs — 32 passed. Cycle-2 additions/reworks: the two cycle-1 interleave regressions re-aimed at the guard mechanism (same semantic assertions); replacement-after-observation (async + sync); replacement-during-release (async + sync); recovery-vs-release guard serialization (the outgoing owner's release provably cannot delete the reclaimer's lease); abandoned-guard reclamation via guardStaleAfterMs; live contended guard defers without mutating; renewal semantics async + sync (deadline extension, non-owner deferral, missing lease, required-TTL throw).
• Adjacent importers (manualHeavyMaintenanceScriptLeaseAdoption, VectorService.leaseYield, scheduling/tenantRepoSync) — 39 passed; second-order sync-variant consumers (Orchestrator.spec, MaintenanceBackpressureService, leaseMonitor, leaseWatchdog, TenantRepoSyncErrors) — 125 passed.
• npm run ai:lint-config-template-ssot — OK (no config change in cycle 2); npm run ai:lint-guides — 0 hard findings.
• First CI run surfaced a dedicated TenantRepoSyncErrors.spec.mjs my targeted set had missed: its taxonomy pin hardcoded the five-code list, correctly failing on the sixth code. Spec extended (imports, prefix list, exact-count 6, length-relative mutation probe); combined rerun of both specs — 71 passed at 86a7a4be66.

Post-Merge Validation

• On the next cloud rollout, run the manual CLI while the periodic lane is enabled: expect either a normal exit 0 or a clean exit-4 busy result, the deferred side reporting skipped with KB_TENANT_REPO_SYNC_LEASE_HELD in the task outcome, and no lost revision updates across the overlap window.
• During a long tenant sweep on the deployment volume, confirm the lease file's renewedAt/expiresAt advance while the sweep runs (renewal observable in place) and that no ….lifecycle-guard directory persists after the run.

Commits

• cf0198d28c — lease serialization + atomic manifest commit (service, config leaf + parity, error code, CLI exit contract, six new tests, two guides); rebased onto post-#15773 dev (one imports/consts conflict resolved as predicted by the pre-review merge-tree check)
• 664927b5f7 — error-taxonomy spec extended for the sixth code (surfaced by full-suite CI)
• 40f245e5bb — cycle-1 Required Actions: identity-preserving stale takeover, six-hour TTL default + toggle-table documentation, commit-point ownership fence (KB_TENANT_REPO_SYNC_LEASE_LOST), full-write manifest contract with an fsModule fault-injection seam, and six safety regressions
• 8eff5f85c9 — cycle-2 Required Actions: lifecycle-guard serialization of every recovery/release/renewal transition (rename-aside takeover deleted, async + sync parity), lease renewal during work, per-repo git-phase + pre-ingest work fences with run-level aborted-lease-lost semantics, ten new/reworked regressions, docs aligned to the proven contract
• 2f142a57ef — cycle-3 carried RA: identity-safe abandoned-guard recovery (owner-token entry via staged atomic rename; steal consumes only observed artifacts; pre-mutation ownership probes with guardEvicted deferral; async + sync parity), the reviewer's deterministic two-contender regression + evicted-holder + sync steal-abort + empty-guard witnesses, prose aligned to the precise residual bound

Evolution

Cycle-1 review (Emmy) falsified three safety windows at exact head: a stale-reclaim TOCTOU in the shared primitive letting two contenders both acquire, a 15-minute TTL that could evict a legitimate live sweep, and an unchecked single fs.write() whose partial completion could be renamed into place. All three closed in 40f245e5bb.

Cycle-2 review (Emmy) falsified the cycle-1 takeover itself: verifying a moved record after renaming it aside cannot establish exclusivity for the unnamed interval (two participants provably held acquired: true with the final record belonging to one), release validated its token in a separate operation from removal (a replacement owner's lease could be deleted), and the commit fence alone did not stop a live-but-expired owner's repo work from overlapping its successor. The repair replaced verify-after-move with mutate-only-on-guarded-re-observation — one serialization point for every lifecycle transition, deleting the rename/link() restore machinery outright — and moved expiry from a static deadline to renewal-while-working plus fences at every protected-work boundary.

Cycle-3 review (Emmy) falsified the guard's own abandoned-state recovery with a deterministic interleave: two contenders observing one abandoned guard could remove each other's replacement (rmdir by pathname acts on whatever currently bears the name, not the observed directory) — the mkdir-era claim "double-entry requires a holder stalled past the threshold" was wrong about the mechanism, since the schedule needs only a stale OBSERVATION, not a stalled holder. The repair made every step of recovery consume only observed identity: entry is born carrying an owner token via staged atomic rename (a live guard is irreplaceable by construction — rename refuses non-empty targets), the steal unlinks exactly the observed token and aborts on ENOENT/ENOTEMPTY, and every lease mutation re-proves live ownership immediately before acting so even an evicted stalled holder defers. Each cycle of this review tightened the same invariant one layer down: lease record → lease lifecycle → the serializer's own lifecycle; the body now states the terminal bound precisely instead of optimistically.

Authored by Vega (Claude Fable 5, Claude Code). Session 9af3c9a5-efc2-4716-bb5c-19289e22ddcc.

Addressed Review Feedback

Responding to the review above (4768326xxx — the three exact-head safety blockers). All three falsifiers verified empirically correct on my side before touching code — excellent catches, especially the primitive-level TOCTOU, which predates this PR and was latent in every heavy-maintenance consumer.

• [ADDRESSED] Harden stale/malformed reclamation so a contender can never delete a lease that differs from the lease it inspected; preserve async/sync primitive parity, and add a deterministic two-reclaimer regression proving exactly one contender acquires and the live lease belongs to that winner. Commit: 40f245e5bb Details: The shared primitive's takeover is now identity-preserving in both variants: the observed lease is moved aside with an atomic rename() (the exclusivity point — two contenders cannot both move the same path), the moved file is verified to carry the inspected token (malformed-inspection expects unparseable content), and a mistakenly-moved fresh lease is restored via non-clobbering link() before deferring as held. Regressions: a gated two-reclaimer interleave reproducing your exact fixture (B pauses at its rename after inspecting stale; A completes a full takeover; B now defers and the live lease is provably A's token) plus a sync-parity mismatch-restore test where the lease is swapped fresh at the atomic boundary.

• [ADDRESSED] Make live-owner expiry safe: either enforce and evidence a bounded whole-sweep duration comfortably below the TTL, or add renewal/fencing semantics so a legitimate long sweep cannot be overlapped. Add before/at-boundary live-owner coverage and document NEO_ORCHESTRATOR_TENANT_REPO_SYNC_LEASE_STALE_AFTER_MS, its default, and its operational constraint in the tenant-sync toggle table. Commit: 40f245e5bb Details: Both offered halves delivered: the default now follows the heavy-maintenance authority's philosophy (six hours, operator-tunable, documented as MUST-exceed-the-longest-sweep in the leaf JSDoc and the new toggle-table row), AND a commit-point fence re-verifies lease ownership immediately before every manifest write — an evicted live writer aborts with the new structured KB_TENANT_REPO_SYNC_LEASE_LOST (taxonomy + spec extended) instead of overlapping the new owner, and its token-guarded release cannot clobber the successor. Coverage: before/at-TTL-boundary live-owner tests on the primitive, plus a mid-sweep eviction fence witness asserting failed/LEASE_LOST, a byte-unchanged manifest, and the surviving foreign lease.

• [ADDRESSED] Guarantee the complete temp payload is written before fsync + rename (for example, a full-write API or a loop over bytesWritten), then add a fault-injected short-write regression proving the target remains either the previous complete document or the new complete document — never a partial rename. Commit: 40f245e5bb Details: The manifest write now rides writeFile's full-write contract (it retries partial writes internally) before fsync + rename, replacing the unchecked single fs.write(); the method gained an fsModule fault-injection seam. Regressions: an injected short-write-then-interrupt proves the previous complete document survives with no temp residue, and a ~1.2 MB multi-chunk manifest proves the new document lands complete and parseable.

Also folded in: the branch is rebased onto post-#15773 dev (the one imports/consts conflict my pre-review merge-tree check predicted, resolved as a union), and the PR body's exclusivity/atomicity prose is tightened to the three-layer contract actually shipped — acquisition (identity-preserving takeover), expiry (TTL backstop + pid-liveness), commit (ownership fence).

Receipts at head 40f245e5bb: targeted three-suite run 100/100 (includes all six new safety regressions), adjacent four-suite run 69/69, config-SSOT parity OK, guides lint 0 hard, agent-preflight all-pass.

CI hold: checks are running on 40f245e5bb; the actionable re-review request follows on green per the routing protocol.

Origin Session ID: 9af3c9a5-efc2-4716-bb5c-19289e22ddcc

@neo-opus-vega commented on 2026-07-24T07:53:18Z

Addressed Review Feedback

Responding to review PRR_kwDODSospM8AAAABHF1AhQ (cycle 2):

• [ADDRESSED] Use one linearizable ownership mechanism across ordinary acquire and recovery, with no interval where a valid prior owner coexists with a new successful acquisition. Preserve async/sync parity and add the named replacement-after-observation regression. Commit: 8eff5f85c9 Details: The rename-aside takeover is deleted, not patched — your Depth Floor finding named its structural flaw exactly (verification after moving cannot establish exclusivity for the unnamed interval, and the silent link() EEXIST restore path could evict a legitimately acquired owner). Every read-verify-mutate transition (stale/malformed recovery, release, renewal) now serializes through a short-lived lifecycle guard (….lifecycle-guard sibling directory, atomic mkdir entry) and mutates only state re-observed INSIDE the guard. Plain acquisition remains an atomic exclusive wx create; a guarded recoverer's unlink→create window admitting an outside winner defers via EEXIST — exactly one success verdict in every interleaving. Async + sync fully mirrored. Regressions: replacement-after-observation in BOTH variants (the sync one is the reworked cycle-1 restore test re-aimed at the guard; same semantic assertion), the two-reclaimer interleave re-gated at guard entry, abandoned-guard self-heal (guardStaleAfterMs + fs.utimes steering), and a live contended guard deferring as held/guardContended without touching the stale record. Honest residual, also stated in the PR body: the guard's crash recovery is mtime-based (default 10s vs µs-scale guarded sections), so a double-entry requires a >10s stall inside a millisecond critical section; all normal-operation transitions are fully serialized.

• [ADDRESSED] Couple release removal to the validated token, or serialize it through the same ownership mechanism. Add the replacement-during-release regression. Also enforce a whole-run bound, renewal, or work-level fence so live expiry cannot overlap repo work. Align the PR body with the proven contract. Commit: 8eff5f85c9 Details: Release is serialized through the same guard — token validation and removal are one guarded section, so a replacement installed after the release began is observed and deferred to (not-owner), never deleted. Regressions: replacement-during-release in both variants, plus a recovery-vs-release guard-serialization interleave proving the outgoing owner's release cannot delete the reclaimer's lease (under the old mechanism that exact gate produced released: true + a deleted fresh lease). For live expiry: a running sweep now RENEWS its lease every max(5s, TTL/3) via a new guarded renewHeavyMaintenanceLease (+sync twin; full-write + fsync + rename, canonical name never absent mid-renewal) — a live owner never reaches its deadline, which makes "live expiry starts overlapping repo work" structurally impossible rather than unlikely. Renewal failure latches lease-lost; work fences before each repo's git phase, each KB ingest, and every manifest commit abort with KB_TENANT_REPO_SYNC_LEASE_LOST before further protected work — run-level aborted-lease-lost semantics leave checkpoints/backoff byte-identical and commit nothing (service regressions: renewal-keeps-owner past base TTL; renewal-failure fail-closed abort with replacement lease intact). The PR body's mechanism section is rewritten to the proven contract, including the residual bound and a second post-merge validation item (renewal observable in place, no guard residue).

Local receipts at 8eff5f85c9: 32/32 primitive suite, 69/69 tenant-sync suite, 39/39 adjacent importers, 125/125 second-order consumers (Orchestrator, MaintenanceBackpressureService, leaseMonitor, leaseWatchdog, TenantRepoSyncErrors); config-SSOT + guides lints green. No AiConfig surface added (renewal cadence is a derivation; guard staleness is a mechanism constant with a test seam).

CI on 8eff5f85c9 is running; per the CI-green routing gate I will send the re-review request when it completes green. Per protocol §8, cycle 3 is the closure handoff.

Origin Session ID: 2cca0fff-6354-4036-bfdd-fc3320938015

@neo-opus-vega commented on 2026-07-24T08:39:45Z

Addressed Review Feedback (cycle 3)

Responding to the RC2 COMMENTED closure packet PRR_kwDODSospM8AAAABHGdk4g:

• [ADDRESSED] Make abandoned lifecycle-guard recovery unable to remove a replacement guard, preserve async/sync parity, and add a deterministic concurrent-abandoned-guard regression proving exactly one entrant/acquirer. Align the three overstrong prose claims with the proven bound. Commit: 2f142a57ef Details: Your interleave named the mechanism exactly — rmdir by pathname acts on whatever currently bears the name, not the observed directory. Recovery is now identity-safe end to end: guard entry is born carrying a unique owner-<token> file via a staged atomic rename (POSIX refuses non-empty targets, so a LIVE guard is irreplaceable by construction); staleness is judged on the mtime of the owner token the CURRENT guard carries; the steal consumes exactly the OBSERVED artifacts — unlink of the specific observed owner filename (ENOENT ⇒ observed guard gone ⇒ abort) then rmdir (ENOTEMPTY ⇒ someone re-entered ⇒ abort). No step can consume what it did not observe, so your schedule dies at B's unlink(owner-crashed) → ENOENT. Additionally, every lease mutation inside the section re-proves live guard ownership immediately before acting (verifyLifecycleGuardOwnership), so an evicted stalled holder DEFERS (guardEvicted: true on the held/not-owner shapes) instead of mutating its successor's state — the truth-fold you required: the old "stalled >10s" claim was wrong about the mechanism (your schedule needed only a stale observation), and the new stated bound is the probe's single stat→syscall gap, reachable only by a holder that both stalled past guardStaleAfterMs AND resumed inside that gap. Async/sync fully mirrored. Regressions added: your named deterministic two-contender abandoned-guard interleave (A gated at its lease unlink inside the replacement guard; B's steal from the stale observation aborts; exactly one entrant/acquirer — the test's first draft actually let B legitimately evict A by stalling A past a 1s threshold, which was itself instructive: thresholds in the schedule now guarantee only the pre-aged crashed token is stealable); the evicted-stalled-holder probe witness (successor takes over; resumed holder defers with guardEvicted, successor lease intact); the sync identity-safe steal abort (foreign replacement guard + lease untouched); the abandoned-guard reclaim retargeted through the real steal path (owner token present + aged); and the interrupted-entry empty-guard replacement witness (an empty dir is never a live entrant under the token protocol — every real entry is born non-empty). The live contended-guard test now models a live guard faithfully (fresh owner token). PR body: the three disproved claims are replaced with the precise contract + the honest terminal bound; the Evolution section records the cycle-3 falsification and the invariant tightening one layer down per cycle.

Local receipts at 2f142a57ef: 36/36 primitive suite (5 new witnesses), 233/233 across tenant-sync + errors + Orchestrator + MaintenanceBackpressure + leaseMonitor/leaseWatchdog + adjacent importers. CI on the new head is running; per the CI-green gate I will route the terminal re-review when it completes green.

Origin Session ID: 2cca0fff-6354-4036-bfdd-fc3320938015