Skip to content

TUI: extract ProjectMailStore lifecycle ownership#649

Open
TZZheng wants to merge 1 commit into
Lingtai-AI:mainfrom
TZZheng:codex/issue-645-pr2-project-mail-store
Open

TUI: extract ProjectMailStore lifecycle ownership#649
TZZheng wants to merge 1 commit into
Lingtai-AI:mainfrom
TZZheng:codex/issue-645-pr2-project-mail-store

Conversation

@TZZheng

@TZZheng TZZheng commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • move project-wide mailbox cache, accepted snapshots, refresh arbitration, polling, and human-location publication out of MailModel into one root-owned ProjectMailStore
  • bind every refresh to store, project, activation, source version, and current MailModel generation before root state or UI state can be published
  • preserve authoritative initial rebuilds behind in-flight steady work and behind an initial launched by a replaced generation
  • serialize uncancellable physical mailbox scan/rebuild bodies process-wide so home and visited projects never scan concurrently
  • make cross-project visit and Mail-view tick lifetimes explicit, with stale ticks/results rejected and restored home snapshots withheld until a fresh initial is accepted
  • remove duplicate location-update launchers, adapt affected tests, and update the touched anatomy contracts

Part of #645. This PR does not close the issue. It is a reviewable lifecycle/ownership change; it does not add the broader Agent rail or thread experience.

Exact reviewed candidate:

  • HEAD: 76ab358434ada1f15ee43dfb2acd4816a3026c5a
  • parent/base: d0167c35d696654899678e5a5498df04ebab6492
  • full binary patch SHA-256 (git diff --binary HEAD^ HEAD): ea3214317980cef2d801a9e6f63c4ba6e32a2bbff655f3d6c42a6c5adc30392d
  • commit shape: 24 files, 1276 insertions, 278 deletions
  • commit subject: refactor(tui): extract project mail store lifecycle

The source worktree was clean when these coordinates and the review-report hashes were recomputed. Drafting this body performed no push, PR operation, issue operation, or merge action; merge is not authorized by this artifact.

Behavior and scope

One project-lifetime owner

App now owns the active ProjectMailStore beside MailModel, plus an optional suspended home store during a cross-project visit. For one project/human-directory identity, replacing target MailModels reuses the same store. The store owns:

  • exactly one private fs.MailCache
  • the accepted immutable ProjectMailSnapshot sequence and version
  • one logical in-flight refresh slot
  • one invalidatable project-mail polling chain
  • refresh scanner and human-location updater dependencies
  • activation/version state needed to reject delayed work

MailModel remains the target/session/presentation consumer. It retains the accepted snapshot reference, bounded SessionCache reconstruction and persistence state, history paging/count state, compose input, and rendering state. It no longer owns a project mailbox cache, scanner, refresh slot, or project-mail polling loop. Its 250 ms pulseTick is presentation animation, not mailbox polling. The separate app-level auto-refresh ticker still serves the kanban and is explicitly not a mailbox scan path.

Identity and generation gates

Every installed MailModel receives a monotonically increasing target generation. A physical refresh captures the store ID, canonical project identity, activation, source version, and launching Mail generation. Acceptance proceeds in this order:

  1. the active store, project, activation, and source version must match;
  2. an exact physical completion may release only its matching store slot;
  3. the result's Mail generation must still be current;
  4. only then may root cache/version/snapshot state change, the payload reach MailModel.Update, and a location update be scheduled.

An unrelated stale result cannot release the active slot. An exact completion from a replaced Mail generation releases its old slot but publishes no root cache, snapshot, UI state, or location side effect. MailModel also retains its own generation checks for accepted session rebuilds, persistence continuations, older-page loads, counts, and UI-local async results.

Initial versus steady arbitration

A steady refresh updates the detached mailbox cache and target/status state. An authoritative initial additionally rebuilds the bounded session cache and is the only refresh that clears initialLoading.

The one store-local slot distinguishes three cases:

  • duplicate steady work coalesces;
  • an initial requested behind steady work sets initialRefreshPending and starts after that exact slot completes;
  • an initial for replacement generation B requested behind generation A's initial is also pending, because refreshGeneration proves A's rebuild cannot satisfy B.

Same-generation duplicate initial requests still coalesce. When old work completes, App.Update starts the pending initial from the live current Mail model, so a second replacement cannot resurrect an older generation. This is correctness arbitration, not a general work queue.

Physical scan serialization and updater ownership

Bubble Tea commands already launched cannot be cancelled. A data-less process-wide mutex therefore wraps the complete physical MailCache.Refresh plus initial session-rebuild body. A visited-project scan may wait for an already-running suspended-home scan, but the scanner bodies cannot overlap. The mutex owns no project data, snapshot, tick, or updater state.

ProjectMailStore is also the sole production owner that injects fs.UpdateHumanLocation. The old startup launcher in tui/main.go and the recipe-substitution launcher are removed. Location work is created only after a current-generation accepted refresh, then re-checks a shared atomic active/activation/version token immediately before the filesystem side effect. Delayed work becomes a no-op after suspension or after a newer accepted store version.

View, tick, and visit lifecycle

  • Leaving Mail invalidates the store tick chain even if its tea.Every command has already fired; a late tick cannot re-arm itself.
  • Re-entering Mail starts at most one current chain and requests initial or steady work according to the model's loading state.
  • Delayed refresh requests are accepted only for the current Mail generation while the store is active and Mail is the current view.
  • Unknown or unavailable navigation returns the untouched original App; it does not pause a live Mail chain.
  • Entering a visited project suspends and retains the home store, installs a distinct visited store, and makes the visited owner active.
  • Returning suspends the visited store and reactivates the same retained home store, but deliberately withholds its old accepted snapshot. A return directly to Mail requires a fresh authoritative initial; a return to Projects leaves home active-but-paused, and a later Projects → Mail transition requests that initial.
  • Store/project/activation/source/generation checks reject completions from the wrong side of any handoff. The process-wide physical gate prevents still-running old and new scan bodies from overlapping.

Test-first evidence

The committed 16-test lifecycle contract grew through real behavioral RED stages. Each recorded test package compiled and reached the intended assertion; the failures below were not syntax, import, toolchain, or dependency failures.

1. Original ownership and lifecycle RED on the exact base

red.md (SHA-256 d1f43f831c0fcd441cef196e67181fe821be8ae205571740794660e8117ef3e2) records:

go test ./internal/tui -run 'Test(ProjectMailStoreOwnsTheOnlyGlobalCache|ShortMailOtherMailCycleRejectsLateTick|RepeatedMarkdownCloseDoesNotRestartMailTick|VisitStoreIdentityRejectsWrongProjectRefresh|HomeVisitedBackHasOneActiveStore)$' -count=1

It exited 1 on five intended gaps: MailModel still owned the project cache; late ticks re-armed after short Mail/view cycles and repeated markdown-close delivery; a same-generation wrong-project refresh was accepted; and App had no explicit active/suspended store lifecycle.

2. Parent edge and visit contracts before their corrections

Artifact SHA-256 Recorded command and intended RED
parent-edge-red.md a71dab5f15d0b6f8fafd5e695c96ac49c378ffd5dfeb4ea6f8ae47def82a4770 `go test ./internal/tui -run 'Test(DelayedRefreshRequestOutsideMailCannotRestartStore
parent-visit-red.md b2b74b01d8e13bea40f5052c38fdbf646c041c812a6ebebe6da31a86f387393a `go test ./internal/tui -run 'Test(HomeVisitedBackHasOneActiveStore
parent-final-red.md f2418323336b59ad4b1f5d9191b01633486c7520135912c696f4689eacfdeb9c `go test ./internal/tui -run 'Test(VisitReturnToProjectsThenMailRequiresInitialRefresh

Chronology note: parent-visit-red.md truthfully records the exact base with the earlier refresh-coalescing correction still uncommitted; it is not represented as a pristine-base run.

3. First independent-review BLOCK and test-first correction

At old candidate 10c63ed74ba74441e17572e0c0bf3c331fa6dc19, four tests were added before production/anatomy repair. review-block-red.md (SHA-256 fad18cbe86be73fc97fe685cb4bba0409f216d1f17a3a8e8a3c31dd656b6444f) records:

go test ./internal/tui -run 'Test(SupersededMailGenerationCannotInstallRootRefresh|ProjectMailStoresNeverScanConcurrently|HumanLocationUpdateRevalidatesAtExecution|MainDoesNotOwnHumanLocationUpdate)$' -count=1

The run exited 1 at the intended assertions: old-generation work installed root version/snapshot state, home and visited scanners reached max active=2, delayed location work ran after activation/version supersession, and tui/main.go remained a second location updater. The correction introduced generation-before-publication, the process-wide physical gate, runtime side-effect revalidation, sole updater ownership, and matching anatomy repairs.

4. Second independent-review BLOCK and final correction

At old candidate 4ac8d26d558ecdff9c89464df4bc618e75d437d9, the first correction still tracked only whether in-flight work was initial, not which generation launched it. The test-only RED added TestSupersededInitialRefreshQueuesReplacementInitial before production/documentation changes.

review-second-block-red.md (SHA-256 3c3ea8b60bfdcacd32e6c77424960e7b400b960f11f5a0bbf2d9b4f22b3f239a) records:

go test ./internal/tui -run '^TestSupersededInitialRefreshQueuesReplacementInitial$' -count=1 -v

It exited 1 at the exact intended assertion:

replacement generation lost its authoritative initial behind the old-generation initial

The minimum fix added refreshGeneration, used it in initial arbitration, and cleared it with the exact physical slot. That prevents a replacement generation from remaining in initialLoading indefinitely.

Historical PASS reports on either old HEAD are superseded and do not count toward the final gate.

Validation

Parent validation is bound to exact final HEAD 76ab358434ada1f15ee43dfb2acd4816a3026c5a, parent d0167c35d696654899678e5a5498df04ebab6492, and binary patch SHA-256 ea3214317980cef2d801a9e6f63c4ba6e32a2bbff655f3d6c42a6c5adc30392d. The final parent report is review-second-block-fix-validation.md, SHA-256 4df98427a3daf31e7fc9135c0a7556fd39ea7824fbb29cf161ce9470ba4c1093.

Exact-head parent gate Result
all 16 dedicated ProjectMailStore / visit contracts PASS, normal 1.417s
same 16 contracts with -race PASS, 3.150s
go test ./internal/tui -count=1 PASS, 8.273s
unfiltered go test ./... -count=1 PASS; internal/headless 85.910s, internal/tui 11.884s
go vet ./... PASS
gofmt -l over every changed Go file PASS; no output
git diff --check HEAD^ HEAD PASS
repository all-ANATOMY citation checker PASS; 452 citations, 0 errors
executable ownership topology PASS: one production mailbox scan execution, one production location-updater injection, and no MailModel cache/slot/tick/updater owner
HEAD / parent / binary patch / source cleanliness exact values above; clean

The final incremental old-initial/new-initial correction did not rerun the complete repository-wide race suite. It did rerun all 16 adjacent lifecycle contracts under -race, and they are green. The earlier causal record remains race-attribution.md, SHA-256 9397a59ed9bed00615ff3992df0d13af545e34d94e76793862a1c66e18157064: historical full-race candidate runs emitted no race-detector report but had portal PID-startup timeouts (reproduced on exact clean base) and one load-sensitive clear timeout (not reproduced by focused candidate/base comparison). Those runs are documented, not relabeled as PASS.

This distinguishes three facts: the parent exact-head non-race repository suite passed; the changed lifecycle contract union passed under race; the historical unfiltered repository race command is not claimed green.

Independent exact-head review

Only the fresh reports below count. Both independently bound themselves to the final HEAD, parent, patch hash, 24-file scope, and clean start/end state, and both end exactly VERDICT: PASS.

  • Claude — PASS
    Report: reviews/claude-direct-76ab3584.md
    SHA-256: ad62930586fa358ee3821365965adb5c8337b971c10314da7708f0ae1e797d25
    Claude re-derived the ownership and generation ordering, ran all 16 contracts normally and with -race, passed unfiltered internal/tui, vet, formatting, diff, and anatomy checks, and found no exact-head defect. Its full go test ./... -count=1 environment observed six failures in the root, internal/inventory, and internal/migrate; the same six failures reproduced identically on a clean exact-base checkout in that environment, outside this diff. They are not treated as product failures, and Claude did not claim universal full-suite green.

  • Codex — PASS
    Report: reviews/codex-direct-76ab3584.md
    SHA-256: ee9b9edbcf08fb0bc7daa61fdc195cc76388b52cabe9e82031cb26138dd46af9
    Codex hand-traced the final arbitration, physical gate, updater token, Mail/UI boundary, visit lifecycle, and anatomy; its static diff/format/anatomy/ownership/binding checks passed and it found no exact-head defect. Its read-only sandbox denied every Go test/vet attempt before compilation because the configured external Go build cache could not create its shard directory (operation not permitted). That is an explicitly reported review-environment limitation, not a candidate failure; executable coverage comes from the exact-head parent and Claude runs above.

The older review rounds are useful defect history only. Their PASS verdicts are non-transferable after each correction.

Cleanup and bridge ledger

  • MailModel no longer contains the project fs.MailCache, project refresh slot, mailbox polling chain, or location-updater owner.
  • Same-project target/model replacement goes through installMailModel and preserves the matching store; mismatched active stores are suspended before replacement.
  • The startup and recipe-substitution UpdateHumanLocation launchers are removed. Production has one store-owned injection after generation-gated acceptance.
  • The app-level auto-refresh ticker and the store mail ticker have separate message types and responsibilities; auto-refresh does not scan mail.
  • Obsolete duplicate visit/cache/tick ownership fields are gone; affected tests use production-routed store acceptance helpers rather than reproducing arbitration logic.
  • tui/internal/tui/ANATOMY.md documents the new owner, generation/publication order, physical gate, visit lifecycle, updater token, writes, and ephemeral state. Shifted main.go citations in tui/ANATOMY.md and tui/internal/config/ANATOMY.md were also corrected.
  • One explicit temporary bridge remains: visitReturnState preserves App-owned back-navigation and UI model state until issue TUI: add a persistent per-project Agent switcher with isolated one-to-one chats #645's planned PR 7 installs bounded ThreadState. PR 7 owns its removal; it must not be deferred to PR 8. The bridge owns no mailbox cache, scanner, refresh slot, updater, or tick, although the retained MailModel may reference accepted snapshot/session-cache presentation state. There is no second store adapter or hidden mailbox owner.

Risks and non-goals

  • Physical scan commands remain uncancellable once launched. The process-wide mutex trades overlap for possible head-of-line waiting when one project scan is slow; stale work may still consume time, but it cannot publish and cannot overlap another physical scan.
  • This PR does not add a general cancellation API, scheduler, arbitrary queue, or cross-subsystem coalescing. Coalescing is intentionally limited to the store's one in-flight slot, same-generation duplicate suppression, and one pending-authoritative-initial correctness bit.
  • The atomic runtime gate is side-effect protection across copied App values; it does not make project data process-global. Project cache/snapshot/tick ownership stays in each store.
  • Returning from a visit intentionally shows loading rather than republishing a suspended snapshot. That is the correctness barrier, not a promise that background work was cancelled.
  • This PR does not add the visible Agent rail, Agent selection, ordinary per-Agent threads, strict direct-mail Main projection, unread UI, new layout geometry, or broader kernel/mail schema changes.
  • No persistence-format or data migration is introduced by this lifecycle extraction.
  • Parent non-race repository PASS, focused race PASS, Claude's base-reproduced environment failures, and Codex's build-cache denial are all reported separately; this body does not claim every command passed in every environment.
  • No merge action is included or authorized.

Rollback

Revert the exact commit as one unit to restore the former MailModel-owned mailbox refresh/tick lifecycle and the prior visit behavior. There is no data migration to reverse. Do not selectively revert only the sole-updater removals or only the generation gate: that would recreate duplicate side effects or permit stale root publication without restoring a coherent former owner. After rollback, rerun the TUI suite and re-audit UpdateHumanLocation, MailCache.Refresh, and anatomy ownership as a single boundary.

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