Fix agent event log compaction race

[trydis]

Summary by CodeRabbit

• Bug Fixes

  • Enhanced thread safety for event logging operations to prevent race conditions during concurrent access.

• New Features

  • Added configurable parameters for log compaction thresholds and pre-compaction callbacks.

• Tests

  • Expanded test coverage with new test cases validating log compaction behavior and concurrent operation scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces advisory locking mechanisms for agent event log operations in the Shellraiser infrastructure. The changes add configurable log compaction with callback hooks and serialize concurrent access to event logs using sidecar lock files to prevent race conditions during compaction and appends.

Changes

Cohort / File(s)	Summary
Core Lock Mechanism Sources/Shellraiser/Infrastructure/Agents/AgentCompletionEventMonitor.swift, Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift	Introduces advisory lock file mechanism for serializing log compaction and appends. AgentCompletionEventMonitor now accepts configurable compactionThresholdBytes and optional beforeCompactionAttempt callback; uses sidecar lock file to guard compaction. AgentRuntimeBridge adds eventLogLockURL property and creates lock file during runtime preparation; wraps log writes with lockf to serialize access.
Test Coverage Tests/ShellraiserTests/AgentCompletionEventMonitorTests.swift, Tests/ShellraiserTests/AgentRuntimeBridgeTests.swift	Adds test cases validating compaction behavior including oversized log truncation, offset reset, and race condition handling where new appends during compaction window prevent truncation. AgentRuntimeBridgeTests updated to expect lock file creation and lockf usage in generated helper content.

Possibly related PRs

• Add workspace busy badge indicator #15: Modifies the same components (AgentCompletionEventMonitor and AgentRuntimeBridge) in the agent event pipeline, indicating potential coordination or dependency between these changes.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix agent event log compaction race' directly and clearly describes the main change—addressing a race condition in agent event log compaction through lock mechanisms and configurable behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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 file-based-ipc-has-race-conditions

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

🧹 Nitpick comments (1)

Sources/Shellraiser/Infrastructure/Agents/AgentCompletionEventMonitor.swift (1)

143-157: Good race condition handling with re-validation under lock.

The pattern of re-checking trailingFragment, readOffset, and currentFileSize after acquiring the lock correctly handles the race window where new data could arrive between the initial check and lock acquisition.

Minor observation: Line 144 declares let currentFileSize = currentFileSize() which shadows the method name. This works correctly but could be slightly clearer with a distinct name like lockedFileSize.

♻️ Optional: Rename to avoid shadowing

 try withLogLock {
-    let currentFileSize = currentFileSize()
+    let lockedFileSize = currentFileSize()
     guard trailingFragment.isEmpty else { return }
-    guard readOffset == currentFileSize else { return }
-    guard currentFileSize >= compactionThresholdBytes else { return }
+    guard readOffset == lockedFileSize else { return }
+    guard lockedFileSize >= compactionThresholdBytes else { return }
     
     // ... truncation logic ...
     
-    CompletionDebugLogger.log("compacted completion log bytes=\(currentFileSize)")
+    CompletionDebugLogger.log("compacted completion log bytes=\(lockedFileSize)")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/Shellraiser/Infrastructure/Agents/AgentCompletionEventMonitor.swift`
around lines 143 - 157, The local constant currentFileSize declared as `let
currentFileSize = currentFileSize()` shadows the method name and may be
confusing; rename the local to something like `lockedFileSize` and update all
uses inside the lock (the guards comparing with `readOffset` and
`compactionThresholdBytes`, and the `CompletionDebugLogger.log` message) so the
code calls the `currentFileSize()` method only for its value and then uses
`lockedFileSize` for comparisons and logging; keep the surrounding logic (checks
of `trailingFragment`, `readOffset`, `compactionThresholdBytes`,
`FileHandle(forWritingTo: logURL)`, truncate and `readOffset = 0`) unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Sources/Shellraiser/Infrastructure/Agents/AgentCompletionEventMonitor.swift`:
- Around line 143-157: The local constant currentFileSize declared as `let
currentFileSize = currentFileSize()` shadows the method name and may be
confusing; rename the local to something like `lockedFileSize` and update all
uses inside the lock (the guards comparing with `readOffset` and
`compactionThresholdBytes`, and the `CompletionDebugLogger.log` message) so the
code calls the `currentFileSize()` method only for its value and then uses
`lockedFileSize` for comparisons and logging; keep the surrounding logic (checks
of `trailingFragment`, `readOffset`, `compactionThresholdBytes`,
`FileHandle(forWritingTo: logURL)`, truncate and `readOffset = 0`) unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b4fc459e-3f46-43f1-9c6f-e8ff07907bc4

📥 Commits

Reviewing files that changed from the base of the PR and between 9f5a36f and f172d77.

📒 Files selected for processing (4)

• Sources/Shellraiser/Infrastructure/Agents/AgentCompletionEventMonitor.swift
• Sources/Shellraiser/Infrastructure/Agents/AgentRuntimeBridge.swift
• Tests/ShellraiserTests/AgentCompletionEventMonitorTests.swift
• Tests/ShellraiserTests/AgentRuntimeBridgeTests.swift