Storage doctor, ingress payload elimination, and Storage tab health surface#853
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
arul28
changed the title
|
@copilot review but do not make fixes |
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds policy-driven storage maintenance across desktop and CLI surfaces, including database retention, ingress bounds, maintenance journaling, runtime health reporting, safe cleanup, diagnostics UI, analytics, IPC, and tests. ChangesStorage maintenance workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@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: abb1a7be6b
ℹ️ 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".
| cursor, received_at | ||
| from automation_ingress_events | ||
| where project_id = ? and source = ? and event_key = ? |
There was a problem hiding this comment.
When a provider replays an event after the new 7-day retention window and no other ingress insert has happened first, this lookup still finds the expired row and returns it as a duplicate, so the event remains suppressed indefinitely and the age prune never runs. Prune before this query, or exclude/delete rows with received_at older than the retention cutoff during the dedupe check.
Useful? React with 👍 / 👎.
| let safeReclaimableBytes = compressibleBytes; | ||
| for (const items of categoryItems.values()) { | ||
| for (const item of items) { | ||
| if (item.safety === "safe_to_remove") safeReclaimableBytes += item.bytes; | ||
| } |
There was a problem hiding this comment.
When reclaimable space is mainly in prunable database tables, extras.safeReclaimableBytes stays at zero because it only sums compressible history and safe_to_remove filesystem items. The renderer uses this value to show the primary “Clean up safely” action while buildSafeCleanupPlan includes prunable dbBreakdown rows, so database-only cleanup loses the main CTA and mixed cleanup underestimates what the doctor will reclaim.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f881f0597
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e6665cc9c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e1dd10454
ℹ️ 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".
| where rowid in ( | ||
| select rowid | ||
| from automation_ingress_events | ||
| where project_id = ? and status != 'dispatched' |
There was a problem hiding this comment.
When an automation matches and runRule later throws, dispatchIngressTrigger changes the ingress row from dispatched to failed; before this retention change, that row still deduped webhook redelivery. With more than 2,000 non-dispatched rows for a project, this cap can delete a failed delivery, so a retry with the same event_key is inserted and executes the automation again, potentially duplicating side effects from actions that already ran before the failure. Exempt statuses that have already been dispatched, such as failed, from both the write-time and doctor caps.
Useful? React with 👍 / 👎.
| <StorageCleanupDialog | ||
| open={safeOpen} | ||
| title="Clean up safely" | ||
| intro="ADE will reclaim space it can rebuild or no longer needs. Your chats, projects, active lanes, and backups stay untouched." |
There was a problem hiding this comment.
When runMaintenanceNow is available, this dialog runs the full storage doctor, whose steps include fs.recovery_backups via collectObsoleteBackups; in projects with old verified recovery backups, confirming this action can delete those older backups. The intro promises that backups stay untouched, so users can approve a cleanup under a false data-retention guarantee. Either adjust the copy to say the newest backup is kept or exclude backup reaping from this user-facing action.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
apps/ade-cli/src/cli.test.ts (1)
1554-1562: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd execution-path coverage for
storage maintenance
apps/ade-cli/src/cli.test.ts:1554-1642only covers plan construction and text formatting. Add headless CLI coverage and the desktop socket-backed RPC path, including CTO-role rejection and CTO success.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/cli.test.ts` around lines 1554 - 1562, Add execution-path tests alongside the existing storage maintenance plan assertions in the relevant CLI test suite: cover headless CLI invocation and the desktop socket-backed RPC flow for storage maintenance, including rejection for a non-CTO role and successful execution for a CTO role. Reuse the existing test helpers, RPC setup, and assertions established elsewhere in the suite rather than testing only buildCliPlan, expectExecutePlan, or inferFormatter.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/automations/automationService.ts`:
- Around line 1171-1174: Remove the early return after the hasPayloads query in
the retention cleanup flow. Guard only the payload-nullification loop with
hasPayloads, while allowing the subsequent ingress pruning and review/PR
retention logic to run on every database, including fresh databases without
legacy payloads.
In `@apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts`:
- Around line 1239-1240: Update the timed action flow around callError and
recordSlowAction so serialized runtime action failures with a fast `{ ok: false
}` response are counted in runtime health. Validate the response envelope inside
the timed try, or record the failure before throwing, ensuring both RPC throws
and unsuccessful responses reach recordSlowAction.
In `@apps/desktop/src/main/services/state/kvDb.ts`:
- Around line 3500-3503: Update the desktop and CLI openKvDb initialization
paths to provide the live sync-peer-state callback through
OpenKvDbOptions.hasSyncPeers. Reuse each runtime’s existing peer
connectivity/state source, ensuring unpaired projects return false and active
peers return true; do not rely on the default callback.
- Around line 3787-3805: The cutoff calculations in pruneReviewArtifacts and
prunePrSnapshots use inline retention durations that can diverge from the shared
policy. Replace both literals with the corresponding shared retention constants
already used by dbMaintenanceApi.ts and the storage ledger, preserving the
existing date and deletion logic.
In `@apps/desktop/src/main/services/storage/storageDbBreakdown.ts`:
- Around line 69-75: Update deriveSyncBookkeepingAction so it returns
“compactable” only when lastCompact has kind === "compact", no error, and no
skippedReason; return “compaction_pending” for failed, unsupported, skipped, or
otherwise non-successful compactions.
In `@apps/desktop/src/main/services/storage/storageInsightsService.ts`:
- Around line 479-482: Update the candidate validation and collection logic in
the staging scan, including the paths around the shown loop and the
corresponding sections near the other referenced ranges, to reject any temp path
that overlaps either projectRoot or adeHome in either direction. Reuse a shared
containment/overlap predicate so candidates containing or contained by either
protected root are excluded both during scanning and before being advertised as
removable, rather than checking only exact path equality.
- Around line 621-645: Update safeReclaimableBytes in the snapshot-building flow
to include only bytes the validated cleanup plan can actually reclaim. Replace
the raw compressibleBytes, broad dbBreakdown prunable totals, and unrestricted
build_release/caches item sum with conservative eligible-row and compression
estimates tied to validated cleanup paths; omit any category without a
defensible savings estimate, especially app-update staging outside
layout.cacheDir.
In `@apps/desktop/src/main/services/storage/storageMaintenanceJournal.ts`:
- Around line 13-20: Strengthen isMaintenanceReport to validate the complete
MaintenanceRunReport shape before returning true: validate each entry in
report.actions, including required action fields such as ledgerId, validate
trigger values against the supported trigger shape, and require a numeric
dbSizeBytes. Return false for malformed or incomplete reports so downstream
deriveSyncBookkeepingAction cannot dereference invalid actions.
In `@apps/desktop/src/renderer/components/app/TopBar.tsx`:
- Around line 293-296: In the button rendered by the TopBar component, remove
the explicit role="status" attribute while preserving the existing aria-label
and native button semantics.
In
`@apps/desktop/src/renderer/components/settings/storage/StorageDiagnostics.tsx`:
- Around line 126-155: Update the health-chip condition around
appResourcePressureLevel and the usageReady/usage render guard so unavailable
resource data is not labeled “Healthy.” Show the chip only when usage contains
at least one valid pressure signal or an explicitly successful sample, while
preserving the existing health calculation and styling for valid data.
In `@apps/desktop/src/renderer/components/settings/storage/storageView.test.ts`:
- Around line 238-279: Add obsolete and newest recovery-backup fixtures to the
buildSafeCleanupPlan test data, then extend the plan assertions to verify
obsolete backups are listed as removable while the newest backup is excluded and
remains protected. Keep the existing category, database, and filesystem cleanup
assertions unchanged.
In `@apps/desktop/src/renderer/components/settings/storage/storageView.ts`:
- Around line 521-580: Update buildSafeCleanupPlan in
apps/desktop/src/renderer/components/settings/storage/storageView.ts: include
safe_to_remove recovery backups in the cleanup groups and description, while
explicitly retaining the newest backup. In
apps/desktop/src/renderer/components/settings/StorageSection.tsx lines 922-930,
replace the claim that backups remain untouched with accurate retention copy.
Extend apps/desktop/src/renderer/components/settings/storage/storageView.test.ts
lines 238-279 to cover obsolete-versus-newest backup planning, and
apps/desktop/src/renderer/components/settings/StorageSection.test.tsx lines
352-365 to assert backup removal is disclosed before confirmation.
In `@apps/desktop/src/renderer/components/settings/StorageSection.test.tsx`:
- Around line 367-381: Add a regression test alongside the existing “shows the
database breakdown and runs maintenance from an inline action” case that
configures runMaintenanceNow to return a report with an action error and zero
reclaimed bytes, triggers the inline compact action, and asserts the UI does not
display the “storage is already tidy” state.
- Around line 352-365: Update the safe-cleanup test around installAdeMock and
the cleanup confirmation dialog to seed an obsolete backup in the maintenance
plan. Assert that the dialog discloses the obsolete backup may be removed and
explicitly indicates the newest backup is retained, while preserving the
existing doctor invocation and reclaimed-space assertions.
In `@apps/desktop/src/renderer/components/settings/StorageSection.tsx`:
- Around line 736-750: Update the maintenance result handling to inspect action
errors before reporting success: in
apps/desktop/src/renderer/components/settings/StorageSection.tsx lines 736-750
and 768-784, show an error-aware status instead of deriving success only from
reclaimed bytes; in
apps/desktop/src/renderer/components/settings/storage/StorageCleanupDialog.tsx
lines 434-447, avoid the “already tidy” headline when actions failed; in
apps/desktop/src/renderer/components/settings/StorageSection.test.tsx lines
367-381, add a zero-byte partial-failure regression covering the inline
maintenance flow.
---
Nitpick comments:
In `@apps/ade-cli/src/cli.test.ts`:
- Around line 1554-1562: Add execution-path tests alongside the existing storage
maintenance plan assertions in the relevant CLI test suite: cover headless CLI
invocation and the desktop socket-backed RPC flow for storage maintenance,
including rejection for a non-CTO role and successful execution for a CTO role.
Reuse the existing test helpers, RPC setup, and assertions established elsewhere
in the suite rather than testing only buildCliPlan, expectExecutePlan, or
inferFormatter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3caaeb01-4b6a-40d2-9b7c-5b769e3f449b
⛔ Files ignored due to path filters (6)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/automations/README.mdis excluded by!docs/**docs/features/automations/triggers-and-actions.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/storage-and-recovery/README.mdis excluded by!docs/**docs/logging.mdis excluded by!docs/**
📒 Files selected for processing (39)
apps/ade-cli/README.mdapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/analytics/productAnalyticsPolicy.tsapps/desktop/src/main/services/analytics/productAnalyticsService.test.tsapps/desktop/src/main/services/automations/automationService.test.tsapps/desktop/src/main/services/automations/automationService.tsapps/desktop/src/main/services/ios/iosSimulatorService.test.tsapps/desktop/src/main/services/ios/iosSimulatorService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/state/dbMaintenanceApi.tsapps/desktop/src/main/services/state/kvDb.rebuildRecovery.test.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/storage/historyCompression.tsapps/desktop/src/main/services/storage/storageDbBreakdown.tsapps/desktop/src/main/services/storage/storageInsightsService.test.tsapps/desktop/src/main/services/storage/storageInsightsService.tsapps/desktop/src/main/services/storage/storageLedger.tsapps/desktop/src/main/services/storage/storageMaintenanceJournal.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/SettingsPage.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/settings/StorageSection.test.tsxapps/desktop/src/renderer/components/settings/StorageSection.tsxapps/desktop/src/renderer/components/settings/storage/StorageCleanupDialog.tsxapps/desktop/src/renderer/components/settings/storage/StorageDiagnostics.tsxapps/desktop/src/renderer/components/settings/storage/StorageMaintenanceJournal.tsxapps/desktop/src/renderer/components/settings/storage/storageUiConstants.tsapps/desktop/src/renderer/components/settings/storage/storageView.test.tsapps/desktop/src/renderer/components/settings/storage/storageView.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/storage.ts
…ge tab health surface Root-caused the 2026-07-17 daemon wedge: unbounded data accumulation (450MB of never-read webhook payloads — 32,023 of 32,026 events ignored), DELETE-mode journaling, and zero automatic maintenance. - automations: never persist raw webhook payloads (verified never read); write-time retention on ingress events (7d / 2,000 per project); one-time boot reclaim nulls legacy payloads, prunes stale review artifacts + PR snapshots (rehearsed on the real 614MB DB: -> 146MB in ~1.6s) - kvDb: WAL + synchronous=NORMAL on rw opens; DbMaintenanceApi on the handle (ingress/artifact/snapshot pruning, peer-gated crsql compaction, fragmentation-aware VACUUM that activates incremental auto_vacuum) - storage doctor: existing daemon sweep extended into a per-step-isolated maintenance pass (transcript compression 30d->14d, auto-reap of os.tmpdir + .ade/tmp staging, obsolete recovery backups, DerivedData), 30-run journal, storage ledger with CI coverage test, runMaintenanceNow wired through action-domain/IPC/preload, 24h slow-action counter via getRuntimeHealth, storage_doctor PostHog taxonomy - Storage tab: SettingsSectionShell header, one-click safe cleanup with itemized preview, policy chips, DB internal breakdown replacing the "Protected" black box, diagnostics strip (DB trend, daemon memory, slow responses, last cleanup), maintenance journal, load pill now deep-links to settings diagnostics Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…opy, resilient reclaim, structural extractions - doctor never reaps the newest recovery backup (keepLatest:1 honored); copy no longer claims backups are never touched - sync-bookkeeping breakdown defaults to "waiting" until a compact run proves peer-free; tooltip no longer promises automatic reclaim - one-time ingress reclaim is non-fatal on error (disk-full machines can still open projects); dispatched events exempt from the count cap so replays can't re-run automations under webhook storms - vacuumIfFragmented prefers bounded incremental passes once auto_vacuum is active; maintenance journal writes atomically (tmp+rename) - .ade/tmp staging mapped in the renderer cleanup targets; scan skips symlinked staging like the reaper does - retention windows hoisted to dbMaintenanceApi (single source of truth); db-breakdown + journal helpers and diagnostics/journal components extracted to their own modules (1k-line rule) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orage overhaul - fold storageLedger.test.ts into storageInsightsService.test.ts (folder test budget), suite green - docs: storage-and-recovery/automations/settings READMEs + ARCHITECTURE rewritten to current reality (doctor sweep, ledger, maintenance hooks, WAL, diagnostics strip, new IPC channels) - ade-cli: typed `ade storage maintenance` subcommand + formatter + README inventory (generic actions path already resolved runMaintenanceNow) - iOS + TUI parity verified not-applicable (no surface / already reachable) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4e1dd10 to
1068ce7
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1068ce77f4
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts (1)
1239-1240: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCount unsuccessful response envelopes in runtime health.
A fast
{ ok: false }result is converted to an error after thisfinally, so it is not sampled. Validate the envelope inside the timedtry, or record that failure before throwing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts` around lines 1239 - 1240, Update the timed call flow around recordSlowAction so unsuccessful response envelopes such as { ok: false } are counted in runtime health even when the call completes quickly. Validate the response envelope inside the timed try block, or record the failure before the post-finally throw, while preserving existing error handling and slow-action sampling.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/automations/automationService.test.ts (1)
2590-2626: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert that reclamation actually uses multiple bounded chunks.
The test only checks the final state, so replacing the chunked UPDATE with one unbounded UPDATE would still pass. Instrument
db.runand assert the payload-clearing statement executes multiple times for these 2,205 rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/automations/automationService.test.ts` around lines 2590 - 2626, Update the test around createIngressService to instrument db.run, identify payload-clearing UPDATE executions, and assert that the statement runs multiple times when reclaiming 2,205 rows. Keep the existing final-state assertions and verify the calls reflect bounded chunk processing rather than a single unbounded update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/storage/storageMaintenanceJournal.ts`:
- Around line 20-35: Update isNullableString and its validation in
isMaintenanceAction so missing skippedReason or error fields are rejected while
explicitly provided null or string values remain valid. Distinguish an absent
property from a property whose value is undefined, and require both action
fields to be present before accepting the maintenance action.
In `@apps/desktop/src/renderer/components/settings/StorageSection.test.tsx`:
- Around line 352-356: Update the test using installAdeMock to provide an
explicit extras snapshot with safeReclaimableBytes set to 0, while preserving
the existing assertion that the safe-cleanup primary button is hidden. Ensure
the test exercises the zero-reclaimable-space case rather than the
missing-legacy-data case.
---
Duplicate comments:
In `@apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts`:
- Around line 1239-1240: Update the timed call flow around recordSlowAction so
unsuccessful response envelopes such as { ok: false } are counted in runtime
health even when the call completes quickly. Validate the response envelope
inside the timed try block, or record the failure before the post-finally throw,
while preserving existing error handling and slow-action sampling.
---
Nitpick comments:
In `@apps/desktop/src/main/services/automations/automationService.test.ts`:
- Around line 2590-2626: Update the test around createIngressService to
instrument db.run, identify payload-clearing UPDATE executions, and assert that
the statement runs multiple times when reclaiming 2,205 rows. Keep the existing
final-state assertions and verify the calls reflect bounded chunk processing
rather than a single unbounded update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0bfbb27c-1382-4fae-9d09-20e5f7492991
⛔ Files ignored due to path filters (6)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/automations/README.mdis excluded by!docs/**docs/features/automations/triggers-and-actions.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/storage-and-recovery/README.mdis excluded by!docs/**docs/logging.mdis excluded by!docs/**
📒 Files selected for processing (39)
apps/ade-cli/README.mdapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/analytics/productAnalyticsPolicy.tsapps/desktop/src/main/services/analytics/productAnalyticsService.test.tsapps/desktop/src/main/services/automations/automationService.test.tsapps/desktop/src/main/services/automations/automationService.tsapps/desktop/src/main/services/ios/iosSimulatorService.test.tsapps/desktop/src/main/services/ios/iosSimulatorService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/state/dbMaintenanceApi.tsapps/desktop/src/main/services/state/kvDb.rebuildRecovery.test.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/storage/historyCompression.tsapps/desktop/src/main/services/storage/storageDbBreakdown.tsapps/desktop/src/main/services/storage/storageInsightsService.test.tsapps/desktop/src/main/services/storage/storageInsightsService.tsapps/desktop/src/main/services/storage/storageLedger.tsapps/desktop/src/main/services/storage/storageMaintenanceJournal.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/SettingsPage.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/settings/StorageSection.test.tsxapps/desktop/src/renderer/components/settings/StorageSection.tsxapps/desktop/src/renderer/components/settings/storage/StorageCleanupDialog.tsxapps/desktop/src/renderer/components/settings/storage/StorageDiagnostics.tsxapps/desktop/src/renderer/components/settings/storage/StorageMaintenanceJournal.tsxapps/desktop/src/renderer/components/settings/storage/storageUiConstants.tsapps/desktop/src/renderer/components/settings/storage/storageView.test.tsapps/desktop/src/renderer/components/settings/storage/storageView.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/storage.ts
🚧 Files skipped from review as they are similar to previous changes (28)
- apps/desktop/src/renderer/components/settings/storage/storageUiConstants.ts
- apps/desktop/src/renderer/components/app/SettingsPage.tsx
- apps/desktop/src/preload/global.d.ts
- apps/desktop/src/preload/preload.ts
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/desktop/src/shared/ipc.ts
- apps/desktop/src/renderer/components/settings/storage/StorageDiagnostics.tsx
- apps/desktop/src/renderer/components/app/TopBar.tsx
- apps/desktop/src/main/services/state/dbMaintenanceApi.ts
- apps/desktop/src/main/services/storage/storageLedger.ts
- apps/desktop/src/main/services/storage/historyCompression.ts
- apps/desktop/src/main/services/storage/storageDbBreakdown.ts
- apps/desktop/src/main/services/analytics/productAnalyticsPolicy.ts
- apps/desktop/src/main/services/adeActions/registry.ts
- apps/desktop/src/main/services/automations/automationService.ts
- apps/desktop/src/shared/types/storage.ts
- apps/desktop/src/main/services/ios/iosSimulatorService.test.ts
- apps/desktop/src/renderer/components/settings/storage/StorageMaintenanceJournal.tsx
- apps/ade-cli/src/cli.ts
- apps/desktop/src/main/services/ios/iosSimulatorService.ts
- apps/desktop/src/main/services/state/kvDb.rebuildRecovery.test.ts
- apps/desktop/src/renderer/browserMock.ts
- apps/desktop/src/renderer/components/settings/storage/StorageCleanupDialog.tsx
- apps/desktop/src/main/services/state/kvDb.ts
- apps/desktop/src/renderer/components/settings/storage/storageView.test.ts
- apps/desktop/src/renderer/components/settings/storage/storageView.ts
- apps/desktop/src/renderer/components/settings/StorageSection.tsx
- apps/desktop/src/main/services/storage/storageInsightsService.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c84009d4d
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64ae24be16
ℹ️ 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".
1 participant
Why
On 2026-07-17 the ADE desktop app froze twice in 30 minutes — all chats unresponsive, tabs stuck, force-quit required. Root cause: the shared daemon wedged behind synchronous SQLite work on a bloated DB under memory pressure (
local_runtime.action_slowshowed every daemon call at 5–11 s, 2,200+ warnings in the incident window). Force-quitting never helped because the daemon survives the UI.Evidence from the incident machine's live DB:
automation_ingress_eventsignored, 3 dispatched. 23.5kcheck_runwebhooks in 7 days via relayoperationsreview_run_artifacts.ade/tmpjournal_modedelete— every write fsyncs+unlinks a rollback journalWhat
Ingress rework (automations): raw webhook payloads are never persisted (verified: no code path ever read the column). Write-time retention: 7 days / newest 2,000 per project, enforced in the insert transaction; dispatched events exempt from the count cap so redelivery can't re-run automations. One-time boot reclaim nulls legacy payloads (chunked), prunes stale review artifacts (30 d) and PR snapshots (60 d), and is non-fatal on error (disk-full machines still open projects).
DB core (kvDb): explicit
journal_mode=WAL+synchronous=NORMALon read-write opens. NewDbMaintenanceApion the KvDb handle: repeatable pruning hooks, fragmentation-aware VACUUM that activates incremental auto-vacuum (bounded non-blocking chunks thereafter), and cr-sqlite tombstone compaction strictly gated to zero-sync-peer projects (lineage reset is only safe when nobody has synced).Storage doctor: the existing daemon sweep (10 min post-boot + daily) extended into a per-step-isolated maintenance pass — transcript compression threshold 30 d → 14 d, auto-reap of
os.tmpdir()+.ade/tmpstaging (>7 d), obsolete recovery backups (always keeping the newest), iOS DerivedData, then the DB hooks. Every run writes a 30-entry journal (.ade/cache/storage-doctor-journal.json, atomic) and one dedupedade_feature_usedanalytics event (coarse enums + byte aggregates only; ≥20 h interval; no ceiling changes). New storage ledger declares a policy for every persistent table/directory, with a CI test that fails when a new tracked dir ships without one.Storage settings tab: full redesign — one-click "Clean up safely" with itemized preview, per-category policy chips, the database card's "Protected" black box replaced with a human-labeled internal breakdown, a diagnostics strip (DB size trend, daemon memory, slow responses 24 h via new
getRuntimeHealth, last cleanup), a collapsible maintenance journal, and the "ADE is under load" pill now deep-links tosettings?tab=storage#diagnostics.CLI parity:
ade storage maintenancetyped subcommand (+ genericade actions run storage.runMaintenanceNow). iOS/TUI verified not-applicable.Impact (rehearsed on the real incident DB)
Migration SQL replayed against a copy of the production 614 MB database: 614 MB → 146 MB in ~1.6 s total (payload wipe + prunes 0.8 s, VACUUM 0.8 s). Users get this automatically at first daemon boot after updating, then the doctor keeps steady state: the webhook table stays in the single-digit MB range forever regardless of volume.
Verification
tscclean; combined suite 331 tests green pre-quality; storage folder 46/46, CLI 272, TUI 924 baseline after parity.storage-and-recovery,automations,onboarding-and-settings,ARCHITECTURE) rewritten;validate-docs.mjsgreen (195 files); logging/analytics gate PASS.Follow-ups (deliberately out of scope)
openKvDbso cr-sqlite compaction activates for unpaired projects (UI copy is honest about the current "waiting" state); longer-term, a peer-ack-aware tombstone horizon.check_runfloods to desktops with no automation listening).🤖 Generated with Claude Code
Summary by CodeRabbit
Greptile Summary
This PR adds policy-driven storage maintenance and surfaces it across the daemon, CLI, and Storage settings UI. The main changes are:
kvDbmaintenance hooks for retention pruning, cr-sqlite compaction gating, and vacuuming.Confidence Score: 5/5
This PR appears safe to merge based on the reviewed changes.
The storage doctor steps are isolated, cleanup uses preview/validation checks, ingress pruning preserves dispatched and failed dedupe rows, and sync compaction fails closed when peer state is unavailable.
No files require special attention.
What T-Rex did
Important Files Changed
storage maintenanceCLI planning and text formatting for maintenance reports; no blocking issues found.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User as User / Storage UI / CLI participant Bridge as IPC / ADE action bridge participant Storage as storageInsightsService participant FS as Filesystem cleanup + compression participant DB as kvDb maintenance API participant Journal as Maintenance journal participant Analytics as Product analytics User->>Bridge: runMaintenanceNow Bridge->>Storage: runMaintenanceSweep(manual) Storage->>FS: compress transcripts and reap safe targets Storage->>DB: prune ingress/artifacts/snapshots Storage->>DB: compact cr-sqlite if no sync peers Storage->>DB: vacuumIfFragmented(threshold) Storage->>Journal: append bounded run report Storage->>Analytics: capture deduped maintenance_run Storage-->>Bridge: MaintenanceRunReport Bridge-->>User: updated cleanup result / diagnostics%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User as User / Storage UI / CLI participant Bridge as IPC / ADE action bridge participant Storage as storageInsightsService participant FS as Filesystem cleanup + compression participant DB as kvDb maintenance API participant Journal as Maintenance journal participant Analytics as Product analytics User->>Bridge: runMaintenanceNow Bridge->>Storage: runMaintenanceSweep(manual) Storage->>FS: compress transcripts and reap safe targets Storage->>DB: prune ingress/artifacts/snapshots Storage->>DB: compact cr-sqlite if no sync peers Storage->>DB: vacuumIfFragmented(threshold) Storage->>Journal: append bounded run report Storage->>Analytics: capture deduped maintenance_run Storage-->>Bridge: MaintenanceRunReport Bridge-->>User: updated cleanup result / diagnosticsComments Outside Diff (1)
apps/desktop/src/main/services/storage/storageInsightsService.ts, line 689-697 (link)The
recovery_backupvalidator accepts any matching backup file, but the snapshot UI now exposes aRemove…action for every recovery backup. With one backup present, confirming that action reachescleanup()and deletes the only rollback snapshot because this branch never checksisObsoleteRecoveryBackup()or the keep-latest rule.Prompt To Fix With AI
Reviews (7): Last reviewed commit: "ship: iteration 5 — refresh peer compact..." | Re-trigger Greptile