Skip to content

TUI: establish direct-mail persistence contracts#648

Open
TZZheng wants to merge 1 commit into
Lingtai-AI:mainfrom
TZZheng:codex/issue-645-pr1-foundation
Open

TUI: establish direct-mail persistence contracts#648
TZZheng wants to merge 1 commit into
Lingtai-AI:mainfrom
TZZheng:codex/issue-645-pr1-foundation

Conversation

@TZZheng

@TZZheng TZZheng commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add one list-first endpoint normalizer and strict human↔Agent direct-mail predicate for scalar, list, multi-To, and CC cases
  • add a versioned, atomic per-project unread-boundary store keyed by canonical Agent directory plus address fingerprint
  • make SessionCache disk ownership explicit with zero-safe NoPersist and sole-writer MainAggregateWriter roles enforced inside both write primitives
  • remove the superseded topology recipient parser, update all current Main cache construction sites, and document the ownership contracts in package anatomy

Part of #645 (the first independent root PR in the issue's implementation DAG). This is a behavior-preserving semantic foundation: it does not add the Agent rail, switch the Main viewport to strict direct mail, wire ordinary Agent threads, change the kernel mail schema, or alter legacy mailbox labels.

Behavior and scope

Direct-mail semantics

NormalizeMailEndpoints produces one trimmed, order-preserving, deduplicated endpoint list from scalar strings, []string, and JSON-decoded lists. IsDirectMail accepts only these two relationships:

  • human → target, with the target in To
  • target → human, with the human in To

CC never establishes direct-thread membership. The topology builder reuses only the shared normalizer, eliminating its duplicate parser without coupling display behavior to the direct-thread predicate. Literal duplicate To entries now normalize to one endpoint rather than double-counting the same topology edge.

Durable unread boundary

The new version-1 store lives at .tui-asset/rail-last-seen.json and records the maximum accepted timestamp plus all IDs observed at that same timestamp. It:

  • uses canonical Agent directory plus address fingerprint for identity
  • writes atomically through the existing filesystem primitive
  • treats missing, corrupt, and unsupported-version state as an all-read baseline at the accepted snapshot
  • preserves boundaries across nickname-only changes
  • re-baselines on address change, disappearance, or directory reuse
  • counts only incoming direct Agent→human mail
  • requires SyncTargets to accept the current identity before MarkSeen
  • surfaces non-missing state-read failures instead of treating them as missing state

This PR exposes the foundation API only; later rail/lifecycle work owns accepted-snapshot wiring.

Session persistence ownership

SessionPersistenceRole has a fail-closed zero value, NoPersist, and an explicit MainAggregateWriter role. Authorization is checked inside both rewriteFile and append, so rebuild, explicit persist, rewrite, and steady refresh cannot bypass it. A short history with complete=true is still never authorized to write.

All current MailModel cache construction sites explicitly use MainAggregateWriter, so the existing aggregate human/logs/session.jsonl behavior and visible Main chat remain unchanged in this PR.

Test-first evidence

The original contract test was written and run against base d0167c35d696654899678e5a5498df04ebab6492 before implementation:

go test ./internal/fs -run '^TestDetachedSessionCacheSteadyRefreshRemainsMemoryOnly$' -count=1

It exited 1 after compiling and exercising the real steady-refresh path because the memory-only cache created human/logs/session.jsonl. This was the intended missing-authorization failure, not a syntax, import, dependency, environment, or unrelated flaky-test failure. The contemporaneous RED record has SHA-256 78aab5ba00a5084309aff12e81d52cf2727d69159cc4938f839ab356c170b79b.

Parent review then added edge-contract tests before the corresponding production correction:

go test ./internal/fs -run 'TestRailUnread(UnsupportedVersion|ReadError|NicknameRename|MarkSeen)' -count=1

That run exited 1 with the two intended failures: non-missing read errors lacked read context, and MarkSeen accepted an identity that had not been synchronized. Its RED record has SHA-256 ed41b8b14a8b4f985b6fcf17515bde07a5040bf99cdf0fc0b59d3f28c21c12af.

Validation

Exact candidate: 05e73f17477f830b93af2033046ee19902dd3fcb

# focused PR1 contracts
go test ./internal/fs -run 'Test(NormalizeMailEndpoints|IsDirectMail|RailUnread|DetachedSessionCache|NoPersistSessionCache|MainAggregateWriter)' -count=1

# complete changed package and TUI module
go test ./internal/fs -count=1
go test ./... -count=1

# static and patch gates
go vet ./...
git diff --check

# complete race-enabled module except one proven pre-existing timing flake
go test -race ./... -skip '^TestPortalURLTimeoutKillsChild$'

All listed commands passed. The repository-wide anatomy checker also passed with 447 citations and 0 missing or out-of-range targets; changed citations were inspected semantically.

The unfiltered race suite passed every package changed by this PR and failed only untouched TestPortalURLTimeoutKillsChild, whose fake child has a 400 ms startup deadline. The same assertion reproduced on a clean detached canonical base 5 times in 50 race-stressed runs; the candidate reproduced it 2 times in 10. No portal code or test is changed here.

Independent review of this exact final HEAD:

  • Claude: PASS, no blockers; independently verified the RED chronology and ran focused/full tests, build, vet, formatting/diff, anatomy, and bounded race checks
  • Codex (gpt-5.6-sol, high reasoning): PASS, no code or contract blockers; its read-only sandbox could not create Go temporary compilation directories, a limitation stated in its report and covered by the exact parent and Claude executable validation above

A prior old-HEAD Codex review caught one inaccurate anatomy sentence about unconditional rewrite behavior. The sentence was corrected to describe the role-gated rewrite, the full parent gates were rerun, the commit was amended, and both final reviewers freshly reviewed the new exact HEAD; no old-HEAD verdict was carried forward.

Cleanup and bridge ledger

  • duplicate resolveRecipients parsing is removed
  • every NewSessionCache call supplies an explicit persistence role
  • both disk primitives enforce the role; complete remains only a window/completeness fact
  • legacy label-formatting paths remain intentionally unchanged
  • no duplicate writer, store adapter, compatibility branch, or ownerless TODO remains
  • bridge ledger: empty

Risks and non-goals

The unread store consumes accepted snapshots supplied by its future owner; it intentionally performs no mailbox scan and owns no timer in this PR. Later lifecycle work must call SyncTargets with the accepted project target set and must not install rejected or wrong-project snapshots.

No visible rail, target switching, strict-Main projection, or ordinary-thread cache is introduced here.

Rollback

Before downstream consumers exist, reverting this commit removes the foundation as one unit and restores the former implicit persistence behavior. Do not roll back NoPersist alone after ordinary Agent thread caches begin depending on it.

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