Skip to content

fix(snapshot): finish queued writes without blocking quit (CC-196) - #344

Merged
jaywedgeworth22 merged 4 commits into
mainfrom
instinct/codecaps-cc196-snapshot-drain
Oct 31, 2026
Merged

jaywedgeworth22 merged 4 commits into
mainfrom
instinct/codecaps-cc196-snapshot-drain

Conversation

@jaywedgeworth22

@jaywedgeworth22 jaywedgeworth22 commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Summary

Ensures queued snapshot writes finish during app quit without blocking termination.

On applicationShouldTerminate, the app now stops sampling, asynchronously flushes pending snapshot writes with a 2-second deadline, then confirms termination, instead of quitting immediately and dropping queued I/O.

What changed

  • Graceful quit: Termination is deferred (terminateLater) until flushSnapshotWrites() completes or times out, keeping the main actor free for queued completions.
  • Bounded drain: Added SnapshotWriteDrain helper that waits for the in-flight write chain with a timeout. Timeout unblocks quit without cancelling the underlying disk write.
  • Ordered disable: Disabling local state now enqueues snapshot removal through the write handoff queue instead of deleting directly, preserving write order.
  • Fresh widget order: Widget snapshot completion now reads platformOrder at completion time instead of using a value captured at enqueue time.
  • Tests: Added SnapshotWriteDrainTests covering empty queue, main-actor completion ordering, real queued file write + flush, timeout behavior, and termination/disable integration checks.

Ensures queued snapshot writes finish during app quit without blocking termination (CC-196).

What changed:

  • App termination now defers quit reply until pending snapshot writes drain, allowing the app to exit promptly once the flush completes or times out instead of hanging.
  • Snapshot write queue is closed when the flush starts — previously queued writes still complete in order, while any late writes enqueued after termination begins are ignored.
  • Repeated terminate requests reuse the same flush operation instead of starting a new one.
  • Added test coverage verifying queue closure preserves prior write ordering and that late completions are dropped, and updated termination integration checks for the new defer-and-reply behavior.

Summary

Ensures queued snapshot writes are finished during app quit without blocking termination, and that a restarted monitor starts a new drain lifecycle (CC-196).

What changed

  • App termination flow (AppDelegate.swift): After awaiting the in-flight snapshot flush, clears the flush-task tracking before replying toApplicationShouldTerminate: true, allowing quit to proceed as terminateLater once queued writes complete.
  • Monitor resume (MonitorModel.swift): start() reopens snapshot-write admission by resetting snapshotWritesClosedForTermination to false, so writes enqueued after a restart are accepted and waited on by the next drain while remaining ordered behind any unfinished I/O.

Tests

  • Added testRestartReopensAdmissionAndNextDrainWaitsForNewWrites to verify stop → drain → start reopens admission and a resumed write completes on the next drain.
  • Updated termination integration assertions to cover the flush-await → clear-task → reply sequence.

Summary

Ensures app quit finishes queued snapshot writes without blocking, then allows termination to proceed.

What Changed

  • Deferred quit for snapshot drain: Termination now returns terminateLater, stops new work, asynchronously waits for queued snapshot writes to flush, then replies to allow quit.
  • Single terminal drain per process: Repeated termination requests share one stop/flush operation while each request still gets its own allow-to-terminate reply. Stale or late callbacks cannot start a new generation of work.
  • Termination is terminal: Once snapshot writes are closed for termination, restarting the monitor does not reopen admission and newly enqueued writes are not run, preventing old filesystem writes from racing newer snapshots.
  • Tests updated: Replaced resume/reopen coverage with terminal-lifecycle coverage, added coverage for shared drain across repeated termination callbacks, and updated integration checks for the new drain path.

@linear-code

linear-code Bot commented Oct 31, 2026

Copy link
Copy Markdown

CC-196

@kody-ai

This comment has been minimized.

Comment thread Sources/CodeCaps/AppDelegate.swift Resolved
Comment thread Sources/CodeCaps/MonitorModel.swift Resolved

@kody-ai kody-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swu-echo

swu-echo Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

@kody start-review

Final-code review requested for exact head 578c90a. Addresses both previous findings: one shared flush task with a reply per quit callback; termination closes queue admission before awaiting without clearing the ordered chain. Actual Mac compile plus 10 focused tests pass (0 failures), including post-close enqueue rejection and source integration coverage. No merge, runtime install or deployment performed.

@kody-ai

This comment has been minimized.

Comment thread Sources/CodeCaps/AppDelegate.swift Resolved
Comment thread Sources/CodeCaps/MonitorModel.swift Resolved
@swu-echo

swu-echo Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

@kody start-review

Final-code review requested at exact 97f8835. Follow-on cancellation/restart findings fixed: clear retained flush task before replying and reopen admission in MonitorModel.start, retaining old chain ordering. Actual Mac compile and 11 focused snapshot/termination/dedup tests pass. No runtime installation or deployment performed.

@kody-ai

This comment has been minimized.

Comment thread Sources/CodeCaps/AppDelegate.swift Resolved
Comment thread Sources/CodeCaps/MonitorModel.swift Resolved

@kody-ai kody-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found critical issues please review the requested changes

@swu-echo

swu-echo Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

@kody start-review

Final-code review requested for exact 5c86684. Owner-aligned terminal-stop lifecycle, not cancelled-quit recovery. Behavioral repeated callbacks share one retained drain forever; no stale task clearing/newer generation exists. MonitorModel.start cannot reopen after termination. No unsafe reset of potentially active disk I/O. Actual Mac compile and 12 focused tests pass, 0 failures. See unresolved discussion4433420643 for lifecycle evidence and disputed recovery assumption. No merge, runtime install or deployment.

@kody-ai

kody-ai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Kody Review Complete

Great news! 🎉
No issues were found that match your current review configurations.

Keep up the excellent work! 🚀

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

​

@jaywedgeworth22
jaywedgeworth22 enabled auto-merge (squash) October 31, 2026 22:26
@jaywedgeworth22
jaywedgeworth22 merged commit 5bec160 into main Oct 31, 2026
22 checks passed
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