Skip to content

fix(admin): cascade profiles on user delete#1224

Open
ymote wants to merge 1 commit into
mainfrom
fix/user-delete-cascades-profiles-422
Open

fix(admin): cascade profiles on user delete#1224
ymote wants to merge 1 commit into
mainfrom
fix/user-delete-cascades-profiles-422

Conversation

@ymote

@ymote ymote commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Cascade admin user deletion through the matching parent profile and sub-account profiles.
  • Stop matching profile processes when available and remove profile data directories.
  • Preserve orphaned profile data when the user record is missing.

Closes #422

Current-main refresh

  • Refreshed onto origin/main 4adbe05fff212cb85d1f0a6d6225da0713446016.
  • Head: 6974f2e35ec4b0b97e4a16b6fcf91db5403b41ff.
  • Isolated checkout: /private/tmp/octos-1224-current.kUxGqg/octos.
  • Environment: Darwin arm64; rustc 1.95.0; cargo 1.95.0.

Validation

  • git diff --check origin/main...HEAD passed.
  • rustfmt --check crates/octos-cli/src/api/user_admin.rs passed.
  • cargo fmt --all -- --check passed.
  • typos crates/octos-cli/src/api/user_admin.rs passed.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-4adbe-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo test -p octos-cli --features api delete_user_ -- --nocapture passed: 2/2 focused cascade tests.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-4adbe-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo test -p octos-cli --features api user_admin::tests -- --nocapture passed: 5/5 user-admin tests.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-4adbe-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo check -p octos-cli --features api --all-targets passed with existing unrelated warning noise.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-4adbe-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo clippy -p octos-cli --features api --all-targets --no-deps exited 0 with existing unrelated clippy warning debt outside this one-file slice.
  • git ls-files --others --exclude-standard was empty in the isolated checkout.

CI / merge status

  • GitHub CI run 26795290895 is green on refreshed head 6974f2e35ec4b0b97e4a16b6fcf91db5403b41ff.
  • Attempt 1 had an unrelated test-octos-cli flake: cli_agent_adapter::tests::times_out_and_kills_child hit Text file busy (os error 26) in job 78990191714.
  • Reran failed jobs; attempt 2 test-octos-cli job 78990936926 passed.
  • Merge remains branch-protection blocked by required review (REVIEW_REQUIRED).

@ymote

ymote commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Closure validation for #422 on PR head 17a045d8f1271b9998b28013aea37cf1d52348bf.

Environment:

  • Temp clone: /private/tmp/octos-pr1315-review.qf59zp/octos
  • Base: current origin/main at 4bb169b409f21cdd42ba31b63d3b3a868877454e
  • macOS 26.2 (25C56), arm64
  • rustc 1.95.0 (59807616e 2026-04-14)
  • cargo 1.95.0 (f2d3ce0bd 2026-03-21)

Commands and results:

  • git diff --check origin/main...HEAD -> passed
  • cargo fmt --all -- --check -> passed
  • CARGO_TARGET_DIR=/private/tmp/octos-pr1315-review.qf59zp/target CARGO_INCREMENTAL=0 cargo test -p octos-cli --features api api::user_admin::tests -- --nocapture -> passed: 5 tests, including cascade cleanup and missing-user preservation
  • CARGO_TARGET_DIR=/private/tmp/octos-pr1315-review.qf59zp/target CARGO_INCREMENTAL=0 cargo test -p octos-cli --features api delete_user_ -- --nocapture -> passed: 2 tests
  • CARGO_TARGET_DIR=/private/tmp/octos-pr1315-review.qf59zp/target CARGO_INCREMENTAL=0 cargo clippy -p octos-cli --features api --all-targets --no-deps -> exit 0; existing warnings remain outside this slice
  • Merge rehearsal: git checkout -B codex/merge-pr1224 origin/main; git merge --no-commit --no-ff codex/review-pr-1224; reran focused delete_user_ tests -> passed: 2 tests; git merge --abort
  • git status --short --branch after rehearsal -> clean on codex/review-pr-1224
  • git ls-files --others --exclude-standard -> no untracked files

Acceptance check:

  • delete_user now deletes sub-account profiles with parent_id == user_id, deletes the matching parent profile if present, removes profile data directories, and attempts to stop profile processes.
  • Regression coverage verifies profile records and data dirs are removed for parent and sub-account profiles, and verifies a missing user does not delete a matching orphan profile.

Remaining gap:

  • GitHub reports this PR as MERGEABLE but BLOCKED / REVIEW_REQUIRED, so it needs an approving review before branch protection will allow merge.

@ymote ymote requested a review from bobdingAI May 28, 2026 11:34
@ymote

ymote commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Current-main validation refresh for #1224 (Closes #422):

  • Current origin/main: 33a18b208e59f8e524b7ab93707348f02f7cbb84
  • PR fix(admin): cascade profiles on user delete #1224 head rehearsed: 17a045d8f1271b9998b28013aea37cf1d52348bf
  • Local rehearsal merge: ac3d1fe3f1e2d267529bc6914ebbe11125c47c81
  • Environment: ignored current-main clone under target/; Cargo target reused at /private/tmp/octos-1228-current-f900-target; root checkout untouched.

Commands passed:

  • git diff --check origin/main...HEAD
  • rustfmt --edition 2024 --check crates/octos-cli/src/api/user_admin.rs
  • CARGO_TARGET_DIR=/private/tmp/octos-1228-current-f900-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo test -p octos-cli --features api --lib delete_user_ -- --nocapture -> 2/2 focused delete-user tests passed
  • CARGO_TARGET_DIR=/private/tmp/octos-1228-current-f900-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo test -p octos-cli --features api --lib user_admin::tests -- --nocapture -> 5/5 user_admin tests passed
  • CARGO_TARGET_DIR=/private/tmp/octos-1228-current-f900-target CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 cargo clippy -p octos-cli --features api --all-targets --no-deps -> exited 0 with existing warnings outside this slice
  • git ls-files --others --exclude-standard -> empty

Acceptance coverage:

  • delete_user_cascades_sub_account_profiles_and_data_dirs proves deleting a user removes the user, parent profile, sub-account profile, and both profile data directories.
  • delete_user_missing_user_does_not_delete_matching_profile proves orphaned profile data is preserved when the user record is missing.

Result: #1224 still cleanly applies to current main and satisfies #422's cascade-delete acceptance. Remaining gap: branch protection still reports reviewDecision=REVIEW_REQUIRED; no merge attempted.

@ymote

ymote commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Current-main closure-path evidence for #422.

Commits checked:

Environment/artifacts:

  • Temp worktree: /private/tmp/octos-1224-current-t6rIiN
  • Cargo target: /private/tmp/octos-1224-current-1fb4-target
  • Clippy target: /private/tmp/octos-1224-current-1fb4-clippy-target
  • Root checkout was not used.
  • No generated artifacts were added to the repo; git ls-files --others --exclude-standard is empty in the rehearsal worktree.

Acceptance checked:

  • Existing user deletion cascades through the matching parent profile and sub-account profiles.
  • Matching profile data dirs are removed.
  • Missing user returns 404 and preserves a same-named orphaned profile/data dir.

Commands/results:

  • git merge --no-edit pr/1224 onto current origin/main: clean.
  • git diff --check origin/main...HEAD: passed.
  • rustfmt --edition 2024 --check crates/octos-cli/src/api/user_admin.rs: passed.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-1fb4-target CARGO_INCREMENTAL=0 cargo test -p octos-cli --features api delete_user_ -- --nocapture: passed, 2/2; existing warnings emitted.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-1fb4-target CARGO_INCREMENTAL=0 cargo test -p octos-cli --features api user_admin::tests -- --nocapture: passed, 5/5; existing warnings emitted.
  • CARGO_TARGET_DIR=/private/tmp/octos-1224-current-1fb4-clippy-target CARGO_INCREMENTAL=0 cargo clippy -p octos-cli --features api --all-targets --no-deps: exited 0; existing warnings emitted in unrelated files.

Result:

@ymote

ymote commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Current-main validation for #422 on 2026-05-29.

Validated PR head 17a045d8f1271b9998b28013aea37cf1d52348bf by cleanly merging it onto current origin/main 1fb4c88e020242d82cee245b331798ed6dc551e7 in /private/tmp/octos-1224-current-7U2pBS; local rehearsal merge commit was 16de497867aeb024d045bfce162fe54b4059b7db.

Acceptance evidence:

  • delete_user now verifies the user exists before profile cleanup, avoiding deletion of matching orphan profiles for missing users.
  • User deletion enumerates sub-account profiles with parent_id == user_id, deletes each sub-account profile, and then deletes the parent profile when present.
  • Profile cleanup stops the profile process when a process manager exists, removes the profile data directory, and deletes the profile record.
  • Tests cover cascading parent/sub-account profile deletion plus data-dir removal, and the missing-user case that must not delete a matching profile.

Commands run:

  • git diff --check origin/main...HEAD passed.
  • rustfmt --check crates/octos-cli/src/api/user_admin.rs passed.
  • cargo test -p octos-cli --features api api::user_admin::tests -- --nocapture passed: 5 passed, with existing warnings in unrelated API test code.
  • cargo check -p octos-cli --features api passed.
  • cargo clippy -p octos-cli --features api --lib --no-deps passed with existing warnings.

Environment: rustc 1.95.0 (59807616e 2026-04-14), cargo 1.95.0 (f2d3ce0bd 2026-03-21), Darwin 25.5.0 arm64. Build/test artifacts were confined to /private/tmp/octos-1224-current-1fb4-test-target; git ls-files --others --exclude-standard was empty after validation.

Remaining gap: cargo fmt --all -- --check still fails on current-main formatting drift outside this PR diff in crates/octos-agent/src/agent/loop_runner.rs, crates/octos-agent/src/loop_detect.rs, and crates/octos-cli/src/api/ui_protocol_ledger.rs. GitHub reports this PR as MERGEABLE but branch protection is BLOCKED because review is still required, so I did not merge it.

@ymote ymote force-pushed the fix/user-delete-cascades-profiles-422 branch from 17a045d to 9e3d876 Compare June 1, 2026 12:25
@ymote ymote force-pushed the fix/user-delete-cascades-profiles-422 branch from 9e3d876 to 6974f2e Compare June 2, 2026 02:50
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.

Delete user does not cascade-delete sub-account profiles

1 participant