Skip to content

[Draft][Epic 953][Stage III][chat-api] Deferred runtime cleanup after the export window - #384

Closed
hanakannzashi wants to merge 4 commits into
codex/issue-380-worker-retired-cloudfrom
codex/issue-381-private-chat-cleanup
Closed

[Draft][Epic 953][Stage III][chat-api] Deferred runtime cleanup after the export window#384
hanakannzashi wants to merge 4 commits into
codex/issue-380-worker-retired-cloudfrom
codex/issue-381-private-chat-cleanup

Conversation

@hanakannzashi

@hanakannzashi hanakannzashi commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Tracks #381.

Draft — Stage III only

The current head is historical only. It was built for the superseded all-410 and account-deletion-removal design. It must not be reviewed, rebased, or merged during Stage I.

This PR is retained as a Draft placeholder only. It is not part of the active #382 Stage I rollout stack, and its current diff is not authoritative for the current contract.

Future scope

After the migration/export window closes, #390, #391, and #392 remove the final stateful proxy routes, and an approved account-deletion compatibility plan is in place, rebuild this work from then-current main for #381. Revalidate all code, docs, tests, and runtime dependencies at that time.

Explicitly not current scope

  • Removing Stage I authenticated owner read views or their backing Conversation/File service/repository wiring.
  • Changing the normal DELETE /v1/users/me account-deletion workflow.
  • Schema/table deletion, retention purge, external-object cleanup, or new export behavior.
  • Changing the stateless Responses contract.

@hanakannzashi
hanakannzashi requested a review from a team as a code owner August 20, 2026 07:04
@ironloopai

ironloopai Bot commented Aug 20, 2026

Copy link
Copy Markdown

🧭 IronLoop Run · Review

This comment updates in place as the Run moves through its stages.

🟩 Final result · Completed

🟨 Queued🟦 Working🟦 Posting results🟩 Completed

Automatic trigger · attempt 1 of 3 · completed in 19m 33s

IronLoop completed the review and posted it to GitHub.

🔗 Result

Open submitted review →

Run details

Run: 88d5914c-6d91-4999-a31b-1045282708be
Base: codex/issue-380-worker-retired-cloud at 90d2ff0
Head: codex/issue-381-private-chat-cleanup at 8e85d15
Created: 2026-08-20 07:09 UTC
Updated: 2026-08-20 07:28 UTC

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Reviewed the full diff (-7646/+34) with a focus on dangling references, the account-deletion path, and the retirement contract.

No critical issues found. This is a clean mechanical removal.

Verified

  • No dangling references. Zero remaining hits for services::conversation, services::file, conversation_service, file_service, the db.*_repository() accessors, the removed DTOs (FileListResponse, ShareGroupResponse, ShareRecipientPayload, ...), the removed openapi_errors/openapi_tags consts, TrackableResource, validate_share_recipient, and validate_near_account.
  • Retirement contract intact. create_retired_stateful_router and create_optional_auth_router plus their 410 GONE coverage from feat(api): Stage I owner-only views and stateless Responses #382 are untouched, and conversations_tests.rs / files_tests.rs still assert the migration response on every retired path. The openapi.rs::omits_retired_stateful_api_paths test still guards spec exposure.
  • Account deletion unaffected. PostgresUserRepository::delete_user_account purges conversation_shares, conversation_share_group_members, conversation_share_groups, conversations, and files via raw SQL in-transaction (user_repository.rs:277-346). It never depended on the deleted repositories, so per-user data removal survives.
  • Imports still all live in routes/api.rs (Multipart, Path, stream, Read, near_api), so the trimmed use block looks correct.

Non-blocking cleanup left behind

All pub items, so -D warnings will not catch them, and they fall inside the stated "remove local tracking helpers" scope:

  • services/src/metrics/consts.rs:7-8 -- METRIC_CONVERSATION_CREATED and METRIC_FILE_UPLOADED are no longer emitted by any call site.
  • list_owned_conversation_ids now has zero callers: the trait decls (user/ports.rs:255,360), the service impl (user/service.rs:219), and the Postgres impl (user_repository.rs:714) are all unreachable.
  • AccountDeletionError::ConversationCleanupIncomplete and FileCleanupIncomplete (user/ports.rs:139,141) are now unconstructible, since nothing returns them once the repository ignores _cloud_deleted_conversation_ids / _cloud_deleted_file_ids. The match arms at routes/users.rs:192-204 are therefore dead branches.

Points to confirm

Note: I could not execute cargo clippy or cargo test in this environment, so compile verification is static (reference greps) plus the clippy and test runs reported in the description.

OK to merge once the stack lands in order. ✅

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 IronLoop review

🟢 No actionable findings

No actionable findings in the reviewed cleanup change.

Validation

  • All-targets compile — API, services, and database targets compile with all features enabled.
  • Retirement route behavior — The focused route-composition test passed.
  • OpenAPI retirement coverage — The focused test confirming retired stateful paths are omitted from OpenAPI passed.
Review details
  • Run: 88d5914c-6d91-4999-a31b-1045282708be
  • Workflow: Review
  • Attempts: 1

@hanakannzashi hanakannzashi changed the title refactor(api): remove retired stateful implementation refactor(api): Stage III deferred stateful runtime cleanup Aug 21, 2026
@hanakannzashi
hanakannzashi marked this pull request as draft August 21, 2026 11:35
@hanakannzashi

hanakannzashi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Stage III draft clarification

This draft was built on the superseded all-410 approach. It combines sharing cleanup with removal of Conversation/File runtime wiring that the Stage I owner read/export views still need, so it cannot merge as-is during Stage I.

Stage I closes all sharing routes under #389, but that does not make this broad branch safe. It remains deferred and must be reworked/rebased only after the Stage III route-removal issues (#390#392) are complete.

@hanakannzashi
hanakannzashi force-pushed the codex/issue-380-worker-retired-cloud branch 3 times, most recently from 62c5d71 to 9bf6ab0 Compare August 21, 2026 13:21
@hanakannzashi hanakannzashi changed the title refactor(api): Stage III deferred stateful runtime cleanup [Draft][Epic 953][Stage III][chat-api] Deferred runtime cleanup after the export window Aug 21, 2026
@hanakannzashi

Copy link
Copy Markdown
Contributor Author

Supersedes earlier review approvals: they applied to the superseded stack, not the current contract. Do not merge, rebase, or use this diff for Stage I. After #390#392 and confirmation of an approved account-deletion path, rebuild from then-current main and revalidate before using this PR.

@hanakannzashi

Copy link
Copy Markdown
Contributor Author

Closing this historical Stage III draft. Its current diff was built for the superseded all-410/account-deletion-removal design and must not be reused. After the export/migration window closes, the replacement path is live and verified, and account-deletion compatibility is resolved, recreate the runtime-cleanup implementation from then-current main under #381.

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.

1 participant