Repository navigation
openclaw-tps-mail: obligation records are never deleted — define a retention policy for terminal records #401
Description
The reply-obligation store added in #398 (<mailDir>/<agent>/.obligations/<inboundId>.json) never deletes a record. Every inbound leaves one small JSON file (~600 B) forever, including those in terminal states (acked, failed). Both reviews of #398 flagged it; it is deliberately out of scope there.
Decide
- What may be deleted: terminal records only (
acked,failed). Neverpending,postedoryielded: restart recovery reads those. - When: a terminal record older than N days, swept at startup recovery (the one place that already walks the store), N configurable.
- What must survive: whatever the replay guard needs. Today a replayed inbound finds its existing record and does not reopen the obligation. Once that record is swept, a replay of the same inbound id would open a fresh obligation. Say whether that is acceptable after N days, or whether the dedupe must come from elsewhere (the maildir's
cur/record, say).
Acceptance
- A terminal record older than N days is removed at startup, and a younger one is kept.
- A non-terminal record is never removed at any age.
- The replay behaviour after a sweep is pinned by a test, whichever answer is chosen.
Activity
tps-flint commentedon Sep 23, 2026
Resolved by #405 (merged 64a3071). Each acceptance line maps to a test in
plugins/openclaw-tps-mail/test/obligation-retention.test.ts:plugins.entries["openclaw-tps-mail"].config.obligationRetentionDays: (d), (d2); N <= 0 disables: (f).pending/posted/yielded) never removed at any age: (b).ackedrecord lets the same inbound id open a fresh obligation. Pinned by (e).Round 2 (from the CodeRabbit threads) added: unparseable-but-present
lastTransitionAtis not aged byinboundTimestamp(g); non-ENOENT store read errors are logged (h); malformed shapes are counted unreadable (i); a terminal record whosecur/record is still unresolved is held, which closes the crash window between theackedtransition and theackedAtpatch inackObligation(j).On the hold's bound: the reviewers differed. Kern proposed a retention age on the hold itself. Sherlock traced that recovery either stamps
ackedAtor quarantines thecur/record todlq/, and both release the hold. Decision: no hold-age now. A hold that persists means acur/record survived recovery unresolved, which is a recovery defect and would be fixed there. The signal is the sweep's startup lineheld N for unresolved cur/ recovery. N staying non-zero across restarts is the trigger to open that issue.