Skip to content

fix(omo-senpi): journal mailbox persistence - #8968

Closed
code-yeongyu wants to merge 4 commits into
devfrom
fix/win-ci-mailbox-caps
Closed

code-yeongyu wants to merge 4 commits into
devfrom
fix/win-ci-mailbox-caps

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

The ordered-delivery mailbox no longer rewrites and fsyncs its complete pending queue for every accepted message. It now records durable journal updates whose enqueue cost scales with the new message, repairs torn tails, rolls back failed appends, compacts the journal with bounded snapshots, and migrates existing mailbox.json state.

The Windows cap-and-restart test keeps the count, byte, overflow, FIFO, and restart contracts with injected limits, and the prior 15-second timeout override is removed.

Root cause

mailbox.ts previously serialized the full queue and called the atomic writer on every enqueue. The failing test performed 128 count-cap writes plus five growing 200 KiB writes, so Windows filesystem latency amplified quadratic total bytes written into a 16.6849-second test timeout.

There is no Windows rename retry/backoff loop in this path. The atomic writer performs one EPERM/EACCES unlink+rename fallback; EBUSY throws. The measured wait was repeated full-state persistence, not hidden retries.

Fix

  • Append one fsynced journal event per enqueue using a write-capable r+ handle and explicit end offset.
  • Truncate back to the original file size if an append fails.
  • Atomically repair a torn final line before the next append.
  • Compact drained or long journals into snapshots while preserving the next sequence.
  • Migrate legacy mailbox.json snapshots on first open.
  • Preserve non-Error host pushback handling so retryable values remain queued.
  • Regenerate the shipped Senpi extension bundle.

Evidence

Persistence benchmark

Twenty trials, real mailbox path:

Case Before After
128 tiny messages, median total 209.548 ms 16.681 ms
Five 200 KiB messages, median total 31.368 ms 1.145 ms
Fixed 200 KiB per-write medians 1.164 -> 8.957 ms 0.155-0.200 ms

This shows the previous enqueue cost grew with existing queue bytes; the fixed path stays tied to the new event.

Local checks

  • Mailbox stress: 450 pass / 0 fail.
  • Final mailbox + persistence tests: 13 pass / 0 fail.
  • Thread component suite after the final rebase: 196 pass / 0 fail.
  • tsgo --noEmit -p packages/omo-senpi/tsconfig.json: pass.
  • TypeScript strict rule checker: no violations.
  • Rebuilt live thread QA: 6/6 scenarios exit 0, failed_scenarios=0, two documented environment/upstream checks skipped.
  • Adversarial code review: initial torn-tail and non-Error-rejection blockers reproduced and fixed; final verdict APPROVE, no high/medium findings.

The extra local test:senpi aggregate was attempted with Bun 1.4.2. Its bundle-size gate passed, but other simultaneous worktree builds on the shared host caused existing 60-second minifier tests to time out. Focused package checks above are green; PR CI is the clean-runner aggregate gate.

Local evidence hashes:

e8caa9d80020714ce8597eb5da452f4d16d23c2d448809abcc6eead498d0a2e6  persistence-benchmark.md
019325fe6fd01ab2de49f93db895aa1cc7194fc73c3eadf6cc7590256daa5ee3  code-review.md
c63b840d378215a9bc0b85e2416c79b2c4954f85289bcfb2cfd9ff499d9bd4d2  live cli-surface
41007356c013c0a27dbad2716b89c32e193e37c1b952a7f4e15357fbcd4e4e60  live plugin-surface

Windows proof

Three independent focused windows-latest soaks ran the fixed mailbox.test.ts 10 iterations each on final soak head c8b885b1b7 (the rebased PR head plus the throwaway paths-target workflow commit):

PR CI on head fbe8613fac completed with 23 success, 6 intentional skips, 1 neutral external reviewer, 0 failures, and 0 pending checks. The full matrix included both windows-latest test shards and senpi-compatibility (windows-latest).

Risks and residuals

  • The new journal format is internal but persisted; legacy migration and crash recovery are covered by tests.
  • Windows r+ fsync behavior is the platform-specific risk and is gated by three focused Windows soaks plus PR CI.
  • Snapshot compaction still uses the shared atomic-write helper; its existing Windows unlink+rename fallback is unchanged.

Fixes #8948
Refs #8324


Summary by cubic

Switches the ordered-delivery mailbox from rewriting and fsyncing its full pending queue on every write to appending single durable journal events, so persistence cost scales with the new message instead of the queue size. This fixes the Windows cap-and-restart test that timed out at 16.7 seconds and removes the 15-second timeout override.

  • Appends one fsynced journal event per enqueue or removal, with truncation rollback if an append fails.
  • Repairs a torn final journal record on the next open and compacts drained or long journals into snapshots that preserve sequence numbers.
  • Migrates existing mailbox.json snapshots to the new mailbox.jsonl journal on first open.
  • Preserves retryable non-Error host pushback so those messages stay queued.
  • Adds persistence, migration, and recovery tests and regenerates the shipped Senpi extension bundle.

Written for commit 3059961. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added the omo-senpi Changes under packages/omo-senpi label Sep 27, 2026
@code-yeongyu
code-yeongyu force-pushed the fix/win-ci-mailbox-caps branch from eace86e to fbe8613 Compare September 27, 2026 14:24
@code-yeongyu
code-yeongyu marked this pull request as ready for review September 27, 2026 15:19
@code-yeongyu
code-yeongyu force-pushed the fix/win-ci-mailbox-caps branch from fbe8613 to 47198da Compare September 27, 2026 17:42
@code-yeongyu

Copy link
Copy Markdown
Owner Author

Superseded by #8961, which lands this fix together with the other win-ci fixes as one merge (all commits of this branch kept, bundles regenerated once over the merged sources). The evidence in this PR (RED/GREEN, focused Windows soaks) still applies; the issue is closed by #8961's Fixes line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

omo-senpi Changes under packages/omo-senpi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(omo-senpi): avoid whole-queue mailbox rewrites

1 participant