Skip to content

feat(cli): add dream purge command and data-aware disable messages#797

Open
yasinBursali wants to merge 2 commits intoLight-Heart-Labs:mainfrom
yasinBursali:feat/data-lifecycle-cli
Open

feat(cli): add dream purge command and data-aware disable messages#797
yasinBursali wants to merge 2 commits intoLight-Heart-Labs:mainfrom
yasinBursali:feat/data-lifecycle-cli

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

What

  • Add dream purge <service> command to permanently delete extension data with type-to-confirm safety
  • Update dream disable to show preserved data size and hint about purge
  • Register purge in command dispatch and help text

Why

  • Users have no way to reclaim disk space from disabled extensions without manual rm -rf
  • Root-owned files from Docker containers make manual cleanup fail without sudo
  • dream disable gives no indication of data persistence or how to clean up
  • Explicit data lifecycle: disable (preserves data) → purge (deletes data)

How

  • cmd_purge(): Validates service_id against registry (prevents path traversal), blocks core services, requires disabled state, shows size via du -sh, requires typing service name to confirm
  • Root-owned files: Tries rm -rf first, falls back to docker run --rm alpine sh -c 'rm -rf contents' for Docker-created root-owned files, suggests sudo as last resort
  • cmd_disable() update: Checks for data/$service_id directory, shows size if present, hints about dream purge

Three Pillars Impact

  • Install Reliability: No impact — purge is a post-install user action
  • Broad Compatibility: Uses only POSIX tools (du -sh, read -p, rm -rf). Docker fallback works on Linux and WSL2. All three platforms supported
  • Extension Coherence: Strengthens data lifecycle by making purge an explicit, confirmed operation that integrates with the existing enable/disable workflow

New Files

None.

Modified Files

  • dream-server/dream-clicmd_purge(), cmd_disable() message, dispatch entry, help text

Testing

Automated

  • ShellCheck: PASS (exit 0 on extracted function)
  • Existing tests: 55 passed, 0 failed (no regression)
  • Security review: SERVICE_IDS validation prevents path traversal

Manual — All Platforms

  • dream purge (no args) → usage error
  • dream purge nonexistent → "Unknown service"
  • dream purge dashboard → "Cannot purge core service data"
  • dream purge n8n (enabled) → "still enabled"
  • dream disable n8n → shows data size hint
  • dream purge n8n → shows size, prompts, type name → success
  • Wrong confirmation text → cancelled
  • dream purge n8n (already purged) → "No data directory found"
  • dream help → lists purge command

Manual — Linux-specific

  • Root-owned files in data dir → Docker fallback succeeds
  • Docker not running → suggests sudo rm -rf

Review

  • CG: ✅ APPROVED (after Docker fallback fix — rm-rf logic corrected, EBUSY warning resolved)
  • Compatibility: PASS (all tools portable across BSD/GNU)
  • Security: PASS (registry validation, no confirmation bypass, core service guard)

Platform Impact

  • macOS (Apple Silicon): Fully supported
  • Linux: Fully supported — Docker fallback for root-owned files
  • Windows (WSL2): Fully supported — Docker Desktop fallback

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audit: APPROVE

Well-implemented CLI command with good safety measures: service ID validated against SERVICE_IDS array (not raw user input in paths), core service guard, disabled-state check, type-to-confirm requirement. Docker fallback for root-owned files is a nice UX touch.

Minor notes (non-blocking):

  • du -sh can be slow on large directories and will block the terminal — acceptable for a CLI command
  • The first rm -rf might partially delete before failing, leaving inconsistent state — the Docker fallback handles remaining files gracefully

Pairs with #812 (dashboard purge). Both deliver one feature together but can merge independently.

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revised Audit: REQUEST CHANGES — missing running-container check

Withdrawing my earlier approval after deeper review.

Bug (MEDIUM): Only checks compose file state, not actual container state
cmd_purge checks if compose.yaml exists (enabled). But if docker compose stop failed silently during dream disable (the disable command uses 2>/dev/null || true), the container could still be running. dream purge passes the check and rm -rfs data under a live container.

Fix: Add a container running check:

local _container="${SERVICE_CONTAINERS[$service_id]:-dream-$service_id}"
if docker ps --format '{{.Names}}' 2>/dev/null | grep -q "^${_container}$"; then
    error "$service_id container is still running. Run 'dream stop $service_id' first."
fi

Everything else confirmed solid after line-by-line review:

  • Service ID validation via sr_resolve + SERVICE_IDS array match — no path traversal possible ✓
  • Docker fallback volume mount constrained to validated data dir — no escape ✓
  • Type-to-confirm is case-sensitive exact match — no bypass ✓
  • du -sh does not follow symlinks by default ✓
  • Core service guard is dynamic from manifests ✓
  • All paths properly double-quoted (including rm -rf and Docker mount) ✓
  • Error recovery: partial deletion falls through to Docker fallback, then to user message with sudo suggestion ✓

@yasinBursali
Copy link
Copy Markdown
Contributor Author

Addressing review feedback

Bug (Missing running-container check) — Fixed:
Added container running check in cmd_purge() between the enabled-state check and the data directory check:

local _container="${SERVICE_CONTAINERS[$service_id]:-dream-$service_id}"
if docker ps --format '{{.Names}}' 2>/dev/null | grep -q "^${_container}$"; then
    error "$service_id container is still running. Run 'dream stop $service_id' first."
fi
  • Uses SERVICE_CONTAINERS associative array (populated by sr_load) with dream-$service_id fallback — matches existing patterns in service-registry.sh and extension-runtime-check.sh
  • docker ps failing silently (daemon down) correctly passes the check — no containers can be running
  • grep -q with ^...$ anchors prevents partial name matches

Lightheartdevs
Lightheartdevs previously approved these changes Apr 6, 2026
Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-audit: APPROVE — fix verified

Running container check added: docker ps --format '{{.Names}}' | grep -q "^${_container}$" before purge. Prevents data deletion under a live container. CI all green.

@Lightheartdevs
Copy link
Copy Markdown
Collaborator

Note: PR needs rebase after recent merges. This PR only touches dream-cli which is unaffected by the Rust rewrite, but GitHub shows merge conflicts — likely from #790 or #795 that just merged. Please rebase on latest main.

yasinBursali and others added 2 commits April 6, 2026 16:52
Add cmd_purge() for permanent service data deletion with type-to-confirm
safety gate. Handles root-owned Docker files via Alpine container fallback.

Update cmd_disable() to show preserved data size and hint about purge.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@yasinBursali
Copy link
Copy Markdown
Contributor Author

Rebased on latest main. Conflict was in dream-cli where upstream replaced rm -f .compose-flags with _regenerate_compose_flags — resolved by using the new function while keeping the data-aware disable messages and cmd_purge command.

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.

2 participants