fix(TaskQueue): append groups instead of replacing on re-entrant enqueue

[Ryan Zhu (underthestars-zhy)]

Summary

enqueuePriorityMounts / enqueuePriorityUnmounts replaced pendingMountGroups (and unconditionally kicked off the next group) on every call. That is only correct when the queue is idle. A second enqueue that lands while an earlier priority sequence is still draining — which happens routinely, since tick() can fire again before the previous wave's groups finish — corrupted the in-flight queue.

This PR makes both methods re-entrant: groups are appended, never replaced, and a new group is started only when the queue is idle. An already-draining sequence picks up the newly appended groups through the existing completion chain (mountTaskCompleted → _startNextMountGroup).

The bug

// before
pendingMountGroups = filteredGroups.map { ($0, reconciler, payloadStore) }  // replace
_startNextMountGroup()                                                       // always start

Two distinct failure modes when a second call lands mid-drain:

1. Stranded pending work / leaked placeholders. Each enqueue first registers every identity as an in-flight placeholder in tasks, then queues its groups. Replacing pendingMountGroups dropped any not-yet-started groups from the earlier sequence — but their placeholders stayed in tasks forever. Those nodes never mounted, and inFlightIdentities() never drained, so the engine believed work was still running indefinitely.

2. Broken priority sequentiality. The unconditional _startNextMountGroup() overwrote mountGroupRemaining and launched a fresh group concurrently with the group already running, violating the "priority groups execute sequentially" contract and corrupting the remaining-count bookkeeping that drives the completion chain.

The fix

// after
pendingMountGroups.append(contentsOf: filteredGroups.map { ($0, reconciler, payloadStore) })
if mountGroupRemaining == 0 {
    _startNextMountGroup()
}

• Append, don't replace — earlier pending groups survive, so their placeholders always get consumed.
• Start only when idle — a running sequence keeps ownership of its current group; the appended groups flow in via mountTaskCompleted → _startNextMountGroup once the running group finishes.
• Identical change for the unmount path.

Identity dedup is unchanged: work already in-flight (placeholdered by an earlier group) is still filtered out — re-convergence remains loop()'s job, not the queue's.

Changes

File	Change
Sources/Astrolabe/Engine/TaskQueue.swift	enqueuePriorityMounts / enqueuePriorityUnmounts append groups and start the next one only when *GroupRemaining == 0. Doc comments spell out the append/idle-start re-entrancy contract.
Tests/AstrolabeTests/AstrolabeTests.swift	New regression suite: a MountGate rendezvous holds a node mid-mount so a second enqueue lands while the first sequence is still draining. Covers stranded-pending + leaked-placeholder (mount and unmount) and sequential execution of mid-flight enqueues.

Test plan

⚠️ Could not run the suite locally this session: .build/repositories is owned by root (leftover from a prior sudo swift run), so SwiftPM can't fetch packages. Needs CI or a local sudo rm -rf .build/repositories first.

• swift test --filter priorityMount / --filter priorityUnmount — the three new tests pass
• swift test — full suite green
• Each new test fails against the pre-fix replace/unconditional-start code (confirms it's a real regression guard)

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR makes TaskQueue priority enqueue methods re-entrant. Instead of replacing pending mount/unmount groups and starting immediately, new groups are now appended to existing lists and started only when the queue is idle; otherwise they drain via the existing completion chain. Comprehensive concurrency tests verify no work strands when re-entrant calls arrive mid-flight.

Changes

Re-entrant priority enqueuing

Layer / File(s)	Summary
Mount/unmount re-entrancy implementation Sources/Astrolabe/Engine/TaskQueue.swift	enqueuePriorityMounts and enqueuePriorityUnmounts now append newly filtered groups into pending lists instead of replacing them, calling _startNextMountGroup()/_startNextUnmountGroup() only when mountGroupRemaining == 0 or unmountGroupRemaining == 0 (queue idle). Documentation expanded to describe append/idle-start contract, behavior when calls arrive mid-flight, and identity deduplication via placeholders.
Concurrency test utilities Tests/AstrolabeTests/AstrolabeTests.swift	MountGate using locks and CheckedContinuation for deterministic mid-flight pausing, IdentityBox for thread-safe execution tracking, GatedNode reconcilable wrapper that invokes mount/unmount hooks and optionally waits on the gate, plus helpers (leaf, work, waitFor polling).
Mount and unmount re-entrancy regression tests Tests/AstrolabeTests/AstrolabeTests.swift	Three test cases verify no pending work strands when priority enqueues arrive mid-flight: priorityMountSecondEnqueueDoesNotStrandPending (all identities mount, placeholders clear), priorityMountSecondEnqueueRunsGroupsSequentially (second identity waits for first gate), and priorityUnmountSecondEnqueueDoesNotStrandPending (symmetric unmount guarantee).

Sequence Diagram

sequenceDiagram
  participant Caller1 as First enqueue
  participant Caller2 as Second enqueue
  participant Queue as TaskQueue
  participant Groups as pendingGroups
  participant Counter as Counter (idle?)
  participant Start as _startNext...
  
  Caller1->>Queue: enqueuePriority(groups)
  Queue->>Counter: check if idle (== 0)
  Counter-->>Queue: idle=true
  Queue->>Groups: append groups
  Queue->>Start: _startNextMountGroup()
  
  Note over Start: Draining, counter > 0
  
  Caller2->>Queue: enqueuePriority(groups)
  Queue->>Counter: check if idle (== 0)
  Counter-->>Queue: idle=false
  Queue->>Groups: append groups
  Note over Queue: Skip _startNext, already draining
  
  Start->>Start: complete current group
  Start->>Counter: decrement, check idle
  Counter-->>Start: idle=false (more pending)
  Start->>Start: _startNextMountGroup() via completion
  
  Note over Start: Continue draining appended groups

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The implementation change is straightforward (append instead of replace, conditional start), but the concurrent test infrastructure and three interrelated regression cases require careful understanding of the deterministic gating mechanism and assertions around race conditions and pending work guarantees.

---

🐰 A queue that yields its place,
Lets newcomers append, not race—
Mid-flight, no strand,
Groups drain as they're planned,
Reentrancy wins with good grace! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically describes the main fix: making enqueue methods re-entrant by appending groups instead of replacing them, which aligns perfectly with the core problem and solution.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/taskqueue-reentrant-enqueue

---

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

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.