feat(storage): bounded retention for agent trace events - #1514
Conversation
Single-owner retention service for agent_events: only tool_call/trace rows older than the configured cutoff (default 30 days, configurable, opt-out) are ever deletable, in per-batch transactions with a state_meta cadence marker and cross-process lease; compaction runs only after a checkpoint, free-space, and margin preflight, with WAL checkpoint after VACUUM so bytes actually return to disk. Ships a partial index migration, controller daily task, vibe data retention CLI (status/dry-run + --run), and en+zh i18n.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2386ecf70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b622fcdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…g hardening - Automatic retention never VACUUMs the live controller database; compaction is owned by the manual vibe data retention --run path (2x-copy free-space preflight, post-VACUUM checkpoint result checked and deferred when busy). - Lease release deletes only the row carrying this runner's token; cadence is rechecked under the lease so overlapping startups do not double-run. - Controller reads the window from persisted V2Config (not the compat shim), fails closed on malformed opt-out values, checks before the first sleep, and shutdown joins the single-worker executor. - plan() measures UTF-8 bytes via CAST(.. AS BLOB); CLI failure text is i18n'd.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d25b6a1ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d25b6a1ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…d malformed windows
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 046b67595f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cquisition - Migration 0057 rewrites offset/fractional agent_events timestamps to whole-second Z so the retention cutoff's lexical comparison is chronological for released rows (a fractional row one second NEWER than the cutoff otherwise sorts before it and is wrongly deleted). - try_acquire_lease takes the SQLite writer lock before its read-decide-write sequence (BEGIN IMMEDIATE prelude), removing the deferred-transaction race where a losing process could delete the winner's valid lease. - vibe data parent help now distinguishes read-only query from the mutating retention --run maintenance subcommand.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3935622ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… renew leases - vibe data retention --run refuses (exit 1) when V2Config.load() returned recovery defaults and no explicit --days window is given; busy runs also exit nonzero so automation retries. - The lease renews between batches and around compaction; ownership loss stops the run instead of deleting beside a replacement runner. - Deferred-compaction reason codes map to localized explanations.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3750e991f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tention lease-loss The re-added canonicalization test had its body duplicated inline (insert ran twice, second hit the UNIQUE constraint); also pin lease-loss at the run_retention level where the renewal loop lives.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c169b58263
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Any malformed persisted retention window (boolean, non-int, <1) refuses --run like recovered config; lease_lost exits nonzero for automation and renders through data.retention.leaseLost (en+zh).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fd7e29237
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t-out display maybe_compact renews the lease from a heartbeat thread for the duration of VACUUM; an ownership loss mid-compaction reports lease_lost_during_compaction and run_once surfaces lease_lost. The status view treats a malformed agent_events_trace_retention_enabled as disabled/recovery (fail closed) like the controller.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7ef3dc666
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…very to retention The heartbeat's normal exit no longer reads as lease_lost (a flag now marks actual renewal failure), so successful VACUUMs checkpoint and report vacuumed; release_lease deletes with an atomic JSON token predicate; recovery fail-closed triggers only on recovery-default warnings, leaving unrelated migration warnings (legacy Model Hub) with retention active; the status view reports recovered policies as disabled.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d6d2af881
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ic renewal - Fail-closed now also matches the 'Recovered invalid config section' warning shape (a replaced runtime section), in both controller and CLI; CLI load-failure and recovery report enabled=False. - The heartbeat waits on a stop Event so VACUUM completion wakes it immediately (no post-VACUUM join stall). - renew_lease is a single token-conditional UPDATE (no read-then-write, no busy-snapshot upgrade path); contested compaction after completed deletion reports ok_with_contested_compaction instead of lease_lost.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6611214187
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Section-recovery fail-closed matches only a replaced 'runtime' section (recovered platforms/model_hub leave the valid policy active); CLI compaction is now opt-in via --compact because VACUUM stalls live-service writes past their busy timeout — the default --run deletes rows only.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f26d76caba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…round VACUUM lock Malformed windows now also report enabled=false in the status view; the compaction heartbeat treats VACUUM-held writer-lock OperationalErrors as retry-later (ownership loss only on token replacement), with renewals before and after the VACUUM window.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebd83ef77d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-replacement flag A recovered/malformed policy stays disabled in the status view (valid-looking defaults no longer overwrite it); --compact without --run now performs the run instead of silently doing nothing; definitive lease replacement during compaction flows through a shared flag so the heartbeat never mistakes it for a VACUUM lock conflict; config comments document the default-on first- pass upgrade behavior and point at the user docs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af077282c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
--days below the service minimum refuses (exit 1) instead of silently clamping to one day; the retention subcommand's help/descriptions render through vibe/i18n (en+zh) with the configured CLI language.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a5ac16959
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ndow The vibe data parent help/description render through i18n (en+zh); --days help interpolates the actual configured window instead of a hardcoded 30.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4869bdeff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| help=i18n_t( | ||
| "data.retention.helpDays", | ||
| _retention_help_lang, | ||
| current=_configured_trace_retention_days(_retention_help_lang), |
There was a problem hiding this comment.
Because build_parser() evaluates this interpolation before parsing the requested command, every CLI invocation—including unrelated commands such as vibe stop, vibe status, and top-level help—now calls V2Config.load(). That loader is not read-only: it creates data directories, backs up recovered configurations, and can persist Model Hub migrations, so merely constructing help for the retention option can modify user state. Read the retention value directly from the JSON as _configured_cli_language() does, or defer the full load until the retention command actually runs.
Useful? React with 👍 / 👎.
| finally: | ||
| executor.shutdown(wait=True) |
There was a problem hiding this comment.
When the controller is stopped while a retention pass is processing a large pre-upgrade backlog, cancelling the asyncio task does not stop the executor worker, and run_retention() has no overall batch limit or cancellation signal. This shutdown(wait=True) then runs on the event-loop thread and blocks it until the entire database sweep finishes; the five-second timeout in cleanup_sync() only releases its calling thread, while the controller loop remains stuck and subsequent shutdown work cannot run. Signal cancellation to the worker and check it between committed batches before joining the executor.
Useful? React with 👍 / 👎.
| agent_events_trace_retention_enabled: bool = True | ||
| agent_events_trace_retention_days: int = 30 |
There was a problem hiding this comment.
When an API client saves agent_events_trace_retention_enabled as "false", or saves agent_events_trace_retention_days as a string, boolean, zero, or negative number, the dataclass annotations do not validate these values and V2Config.from_payload() accepts and persists them. The save therefore reports success, but the controller subsequently treats the malformed policy as disabled, so the requested retention setting never takes effect and trace storage can grow without bound. Reject invalid types and require a non-boolean integer of at least one at the configuration write boundary.
Useful? React with 👍 / 👎.
1 participant
What
Bounded retention for internal agent trace events (lane B of avibe#1506):
agent_eventsrows withevent_type='tool_call'andvisibility='trace'older than a configurable window (default 30 days) are now deleted by a storage-owned daily maintenance pass, with a safe physical-compaction step and a manual CLI. This bounds~/.avibe/state/vibe.sqlitegrowth (one active installation: 362k rows / ~1.27 GiB, 99.8% old tool_call traces).Change
storage/agent_events_retention.py— the single owner ofagent_eventsdeletion policy:event_type='tool_call' AND visibility='trace' AND created_at < cutoff), shared by planning, execution, and tests. Messages, deliveries, session/run records, Vault audit, non-trace events, and newer traces are preserved by construction.run_retention()deletes in small batches, one transaction per batch (WAL lets concurrent writers proceed between batches);plan()is the read-only dry-run (candidate count + logical payload bytes).state_metamarker (agent_events_trace_retention.last_run) throttles to at most one run per day; a lease row (…lease) serializes concurrent runners (controller task vs CLI) across processes — loser sees{"status": "busy"}.maybe_compact): below-threshold skip → WAL checkpoint (defer if busy) → free-space preflight (defer when free < db + wal + 256 MiB margin) → VACUUM + post-VACUUM checkpoint so bytes actually return to the OS. Deferred compaction is a reported state, never an error, and never runs on a nearly-full volume.20260817_0056— partial indexix_agent_events_trace_retention ON agent_events(created_at) WHERE event_type='tool_call' and visibility='trace'(byte-compatible predicate instorage/models.py); idempotent against template-created schema.tests/test_sqlite_state_migration.pyHEAD_REVISION bumped.config/v2_config.py+vibe/api.py—runtime.agent_events_trace_retention_enabled(default true; explicit opt-out) andruntime.agent_events_trace_retention_days(default 30, clamped ≥1 at use). Both keys are carried into_payloadand the API deep-merge base so an unrelated settings save cannot silently revert a user's opt-out.core/controller.py— background loop checking hourly; actual work at most daily via the marker; reads the config each cycle so opt-out applies without restart; runs viaasyncio.to_threadoutside request paths; cancelled on shutdown.vibe/cli.py—vibe data retention [--run] [--days N] [--no-compact] [--json]: default shows status/plan (candidates, bytes, last run, compaction outlook);--runforces one pass now (bypasses the cadence gate, still respects the lease). Output i18n'd (en + zh,data.retention.*).Capability / scenarios
Capability: storage lifecycle for internal trace events. No scenario catalog covers storage retention (model_hub covers routing/turn contracts); no applicable scenario ID.
Evidence
tests/test_agent_events_retention.py— 9 tests: predicate-only deletion (boundary row at cutoff kept), messages untouched, per-batch concurrent appends preserved, plan counts/bytes, idempotency + marker cadence (24h), lease exclusion, compaction defers on injected low disk, compaction vacuums + shrinks the real file (post-VACUUM checkpoint), day clamping.tests/test_sqlite_state_migration.py— 93 passed with the new head (single-head check, upgrade/downgrade cycle).test_config_write_transaction.py,test_api_save_config_merge.py,test_api_runtime_refresh.py76 passed) and agent_events consumers (test_agent_activity_service.py,test_message_mirror.py,test_session_fork.py193 total passed).ruff checkon all changed files — clean.vibe data retentionthenvibe data retention --runand observe row/byte reduction plus compaction result.Non-goals (per issue)
Show Runtime archive cache cleanup is lane A (PR #1509); no deletion of user-visible messages, attachments, Vault data; no external disk or service restart required.
Dependencies
None (independent of #1509; only shares the issue). avibe-docs retention documentation ships with this PR's merge per the plan doc.
Known-by-design ledger
busyand retries later.VACUUM_MIN_RECLAIM_BYTES(64 MiB) intentionally skips compaction for small gains so a daily pass does not rewrite a large database for pocket change; rows are still deleted and reported.