Clean up extension credentials on uninstall

[henrypark133]

Summary

• delete stored credentials when WASM tools, WASM channels, and MCP servers are uninstalled
• preserve shared secrets until the last installed extension referencing them is removed
• add E2E coverage for tool, channel, shared Google OAuth, and MCP uninstall cleanup flows

Testing

• cargo fmt --all
• cargo clippy --all --benches --tests --examples --all-features -- -D warnings
• cargo clippy --all --benches --tests --examples -- -D warnings
• cargo clippy --all --benches --tests --examples --no-default-features --features libsql -- -D warnings
• cargo test test_remove_mcp_server_deletes_stored_secrets --lib
• tests/e2e/.venv/bin/pytest tests/e2e/scenarios/test_extension_uninstall_cleanup.py -q

[gemini-code-assist]

Code Review

This pull request introduces a secret cleanup mechanism to ensure that secrets and their associated companion secrets (e.g., refresh tokens, scopes) are deleted when an extension is uninstalled, provided they are no longer referenced by other extensions. The implementation includes a SecretCleanupPlan to track dependencies and logic within the ExtensionManager to verify references across WASM tools, channels, and MCP servers before deletion. Comprehensive unit and E2E tests have been added to validate that unique secrets are removed and shared secrets are preserved until the final referencing extension is gone. I have no feedback to provide.

[Copilot]

Pull request overview

Adds uninstall-time secret cleanup for extensions (WASM tools, WASM channels, MCP servers) while preserving shared credentials until the last referencing extension is removed, and extends E2E coverage to validate the behavior against the libSQL secrets table.

Changes:

• Implement secret cleanup planning + best-effort deletion during ExtensionManager::remove() for WASM tools/channels and MCP servers.
• Add E2E scenarios and a dedicated isolated E2E server fixture to validate secret deletion/preservation flows.
• Document the new E2E scenario in the E2E test index.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/extensions/manager.rs	Builds a per-extension secret cleanup plan, checks whether secrets are still referenced, and deletes unreferenced secrets on uninstall; adds unit tests for cleanup behavior.
tests/e2e/conftest.py	Adds a session-scoped isolated IronClaw instance fixture for uninstall-cleanup E2E scenarios.
tests/e2e/scenarios/test_extension_uninstall_cleanup.py	New E2E tests verifying uninstall secret cleanup for WASM tools/channels, shared Google OAuth secrets, and MCP servers.
tests/e2e/CLAUDE.md	Documents the new uninstall cleanup E2E scenario and fixture.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In collect_secret_cleanup_plan for WasmTool, the loop over Self::tool_secret_names(&cap) already inserts auth.secret_name (see tool_secret_names). The subsequent plan.add_base_secret(&auth.secret_name) is redundant; consider removing it to keep the cleanup-plan logic single-sourced (while still adding the companion secrets).

Suggested change

plan.add_base_secret(&auth.secret_name);

[zmanian]

Review: Clean up extension credentials on uninstall

What was done well

• Correct ordering: The cleanup plan is collected BEFORE files are deleted, then the reference scan runs AFTER deletion, so the just-uninstalled extension's secrets correctly fall out of the "still referenced" set. This is the right approach.
• Shared secret safety: The collect_referenced_secret_names scan prevents premature deletion of secrets shared across extensions (e.g., Google OAuth token used by both gmail and google-drive). The test test_remove_wasm_tool_keeps_shared_secrets_until_last_extension exercises this directly.
• Fail-safe on uncertainty: When capabilities files for other installed tools are missing or unreadable, collect_referenced_secret_names returns an error, and cleanup_uninstalled_extension_secrets bails out entirely, keeping all secrets. This is the conservative choice -- tested by test_remove_wasm_tool_keeps_secrets_when_other_tool_capabilities_missing.
• Best-effort deletion: delete_secret_best_effort logs warnings rather than failing the uninstall if a secret deletion fails. Correct tradeoff -- the uninstall should succeed even if cleanup is partial.
• No .unwrap() or .expect() in production code. All instances are in tests.
• Companion secrets covered: OAuth refresh tokens and scopes companions are properly tracked via SecretCleanupPlan::companion_secrets, with dedicated helpers oauth_refresh_secret_name and oauth_scopes_secret_name.
• ChannelRelay: Correctly returns an empty SecretCleanupPlan since relay secret cleanup (relay:<name>:oauth_state, relay:<name>:stream_token) is already handled inline in the staging branch's ChannelRelay removal arm.
• Thorough test coverage: 6 new unit tests plus 4 E2E scenarios covering WASM tool, WASM channel, shared Google OAuth, and MCP server cleanup flows.

Suggestions (nice to have)

1. SecretCleanupPlan companion secrets for non-base secrets: The companion_secrets HashMap is keyed by base secret name, and companions are only deleted when their base secret is being deleted (not referenced elsewhere). If a companion secret name somehow also appears as a base secret in a different extension, it would get double-checked -- not harmful, but worth noting.

2. collect_referenced_secret_names does not track companion secrets: The reference scan only collects base secret names (via tool_secret_names, channel_secret_names, mcp_server_secret_names). Companion secrets (refresh tokens, scopes) are not added to the referenced set. This means if Tool A and Tool B share an OAuth provider, and Tool A is removed, the companion secrets (refresh token, scopes) would only be protected by their base secret still being referenced. This is correct in practice since companions are only deleted when the base is not referenced, but adding companion secret names to the reference set would be a more explicit safety net.

3. tracing::warn! structured fields: In cleanup_uninstalled_extension_secrets, the error field is passed without % formatting (error, instead of error = %error). This works because String implements Value, but using error = %error would be consistent with delete_secret_best_effort which uses error = %error.

Overall this is clean, well-tested, and handles the edge cases correctly. LGTM.