# Scratchpad: Stale Code Quick Wins

## Key Discoveries

### Media package exists for a reason that no longer applies

The `@alga-psa/media` package was created to break the cycle `documents -> users -> media -> documents`. This cycle was resolved and removed from the known-cycles baseline. The media package's dynamic import workarounds (`getStorageServiceAsync()`, `deleteDocumentAsync()`, `getDocumentTypeIdAsync()`) are no longer needed -- the canonical `documents/lib/entityImageService.ts` uses direct imports to `@alga-psa/storage/StorageService` and internal `documentActions`.

### Three copies of avatarUtils exist

1. `@alga-psa/formatting/avatarUtils.ts` (228 LOC) -- uses `getImageUrlInternal` from `./imageUrl.ts`
2. `@alga-psa/user-composition/lib/avatarUtils.ts` (~95 LOC) -- has its own `getImageUrlInternalLite` inline
3. `@alga-psa/media/lib/avatarUtils.ts` (87 LOC) -- uses dynamic import from documents

This PR deletes #3. Consolidating #1 and #2 is a separate future task.

### `getTeamAvatarUrl` only exists in media

The formatting package's `EntityType` already includes `'team'`, and `getEntityImageUrl` works for any entity type. But there's no `getTeamAvatarUrl` convenience function. Adding one is a trivial 4-line wrapper.

### documents version of EntityImageService is the canonical one

The `@alga-psa/documents` version:
- Uses direct `StorageService` import (not dynamic)
- Uses direct `deleteDocument`/`getDocumentTypeId` calls
- Has `isActionPermissionError` handling
- Already used by 4 other packages (`clients`, `client-portal`, `tenancy`, documents-internal)

The media version:
- Uses dynamic imports to break cycles (no longer needed)
- Less robust error handling (no permission error check)
- Only used by 3 callers (users x2, teams x1)

### Cross-package violations trade-off

Adding `users -> documents` creates 2 new lint warnings. But:
- 4 other packages already do this exact import
- We delete an entire package (8 files) in exchange
- `teams -> documents` creates NO warning (teams not in VERTICAL_PACKAGES)

### Test file reads source files via fs.readFileSync

`server/src/test/teams-v2-improvements.test.ts` does string-content assertions on source files:
```typescript
const mediaAvatarUtils = read('packages/media/src/lib/avatarUtils.ts');
```
This must be updated to point at the formatting file, and assertions T085/T089 need adjustment.

### Avatar Bug 1: Client portal user avatars on MSP user list

Root cause chain:
1. Client portal user uploads avatar via `uploadContactAvatar(user.contact_id, formData)` in `clientUserActions.ts:289`
2. This stores `entity_type='contact'`, `entity_id=contact_id` in `document_associations`
3. MSP UserList fetches via `getUserAvatarUrlAction(user.user_id)` which queries `entity_type='user'`, `entity_id=user_id`
4. No match → null → no avatar displayed

Fix: Branch on `user.user_type` in `UserList.tsx` avatar fetch loop. Use `getContactAvatarUrlAction(user.contact_id)` for client users.

### Avatar Bug 2: Contact avatar '?' fallback

Root cause chain:
1. `ContactAvatarUpload.tsx:38` passes `entityName=""` to `EntityImageUpload`
2. `EntityImageUpload` renders `UserAvatar` with `userName=""`
3. `ContactAvatar.getContactInitials` returns `'?'` when `!name` (line 12 of `ContactAvatar.tsx`)

Fix: Add `contactName: string` prop to `ContactAvatarUpload`, pass it as `entityName`. Update 2 callers (`ContactDetails.tsx`, `ContactDetailsEdit.tsx`) to pass the contact's `full_name`.

## Build Command

Use instead of `npm run build`:
```bash
NODE_OPTIONS=--max-old-space-size=32768 npx nx run-many -t build --maxParallel=4
```

## File Inventory

### Files to delete (10 total)
- `server/src/lib/posthog.ts` (7 LOC)
- `server/src/lib/actions/tenant-secret-actions.ts` (222 LOC)
- `packages/media/src/index.ts` (3 LOC)
- `packages/media/src/lib/avatarUtils.ts` (87 LOC)
- `packages/media/src/lib/documentsHelpers.ts` (32 LOC)
- `packages/media/src/services/EntityImageService.ts` (296 LOC)
- `packages/media/package.json`
- `packages/media/project.json`
- `packages/media/tsconfig.json`
- `packages/media/tsup.config.ts`

### Files to edit (15 total)
- `server/src/components/settings/secrets/SecretsManagement.tsx` (import path)
- `server/src/components/settings/secrets/SecretDialog.tsx` (import path)
- `packages/formatting/src/avatarUtils.ts` (add getTeamAvatarUrl)
- `packages/users/src/services/UserService.ts` (import paths)
- `packages/users/src/actions/user-actions/userActions.ts` (import path)
- `packages/users/package.json` (dependencies)
- `packages/teams/src/actions/team-actions/avatarActions.ts` (import paths)
- `packages/teams/package.json` (dependencies)
- `server/next.config.mjs` (remove media alias)
- `server/tsconfig.json` (remove media paths)
- `ee/server/tsconfig.json` (remove media paths)
- `services/workflow-worker/Dockerfile` (remove media workspace)
- `server/src/test/teams-v2-improvements.test.ts` (update file paths + assertions)
- `server/src/components/settings/general/UserList.tsx` (avatar fetch branch)
- `packages/clients/src/components/contacts/ContactAvatarUpload.tsx` (add contactName prop)
- `packages/clients/src/components/contacts/ContactDetails.tsx` (pass contactName)
- `packages/clients/src/components/contacts/ContactDetailsEdit.tsx` (pass contactName)

## Execution Order

1. Task 1: Delete posthog.ts (independent, no deps)
2. Task 2: Redirect tenant-secret-actions callers, delete server copy (independent)
3. Task 3: Media deletion (depends on nothing, but do in sub-steps):
   a. Add `getTeamAvatarUrl` to formatting
   b. Update 3 caller files (import paths)
   c. Update 2 package.json files (dependencies)
   d. Update test file
   e. Update config files (next.config, tsconfig x2, Dockerfile)
   f. Delete `packages/media/` directory
   g. `npm install` to update lock file
4. Task 4: Fix UserList.tsx avatar fetch for client portal users
5. Task 5: Fix ContactAvatarUpload.tsx and its 2 callers
6. Build verification

## Gotchas

- The `teams-v2-improvements.test.ts` test does string matching on file content -- it will ENOENT if media files are deleted before the test path is updated.
- `server/next.config.mjs` has media referenced in 3 separate places (aliases object, transpilePackages array, resolve.alias). All 3 must be removed.
- `package-lock.json` will have stale entries for `@alga-psa/media` until `npm install` is run.
- The Dockerfile `--workspace=@alga-psa/media` line is in a `COPY` instruction context -- verify the line can be cleanly removed without breaking the multi-line command.
- T102 in `teams-v2-improvements.test.ts` checks that `@alga-psa/media` is in teams package.json deps -- must update to check for `@alga-psa/documents` and `@alga-psa/formatting` instead.

## Progress Log
- F001: Deleted `server/src/lib/posthog.ts` after validating the deprecated wrapper path had no remaining imports.
- F002: Pointed `SecretsManagement.tsx` at `@alga-psa/tenancy/actions` so it uses the canonical tenant secret actions barrel.
- F003: Updated `SecretDialog.tsx` to import secret CRUD helpers from the canonical `@alga-psa/tenancy/actions` barrel.
- F004: Deleted `server/src/lib/actions/tenant-secret-actions.ts` once both settings callers had been redirected to `@alga-psa/tenancy/actions`.
- F005: Added `getTeamAvatarUrl` to `packages/formatting/src/avatarUtils.ts` as the canonical wrapper over `getEntityImageUrl('team', ...)`.
- F006: Repointed `packages/users/src/services/UserService.ts` to import avatar URL lookup from formatting and image mutations from documents.
- F007: Updated `packages/users/src/actions/user-actions/userActions.ts` to consume entity image mutations from `@alga-psa/documents`.
- F008: Split `packages/teams/src/actions/team-actions/avatarActions.ts` imports across canonical documents and formatting packages.
- F009: Replaced the stale `@alga-psa/media` dependency in `packages/users/package.json` with canonical `documents` and `formatting` package dependencies.
- F010: Replaced the teams package dependency on `@alga-psa/media` with the canonical documents and formatting packages.
- F011: Removed all `@alga-psa/media` alias and transpilation wiring from `server/next.config.mjs`.
- F012: Removed the `@alga-psa/media` path mappings from `server/tsconfig.json`.
- F013: Removed the stale `@alga-psa/media` path mappings from `ee/server/tsconfig.json`.
- F014: Dropped the deleted `@alga-psa/media` workspace from `services/workflow-worker/Dockerfile`.
- F015: Updated `server/src/test/teams-v2-improvements.test.ts` to read formatting avatar utilities and assert the new canonical package dependencies.
- F016: Deleted `packages/media/` entirely after migrating its three callers, config references, and the source-reading regression test.
- F017: Ran `npm install` to refresh `package-lock.json` after removing `@alga-psa/media` and adding canonical package dependencies.
- F018: Imported `getContactAvatarUrlAction` into `UserList.tsx` to support client-user avatar lookup via contact records.
- F019: Updated the `UserList.tsx` avatar lookup loop to fetch contact avatars for client users, preserve user avatars for internal users, and fall back to `null` when a client user lacks `contact_id`.
- F020: Added a required `contactName` prop to `ContactAvatarUploadProps` and plumbed it into the component signature.
- F021: Replaced the hardcoded empty `entityName` in `ContactAvatarUpload` with the new `contactName` prop so initials render correctly.
- F022: Updated `ContactDetails.tsx` to pass `editedContact.full_name` into `ContactAvatarUpload`.
- F023: Updated `ContactDetailsEdit.tsx` to pass `contact.full_name` into `ContactAvatarUpload`.
- Discovery: the first full build after F023 failed because `server/src/app/api/secrets/route.ts` and `server/src/app/api/secrets/[name]/route.ts` still imported the deleted server duplicate. Added follow-up items `F023A`/`F023B` and `T041A`/`T041B` so the plan matches the real remaining work.
- F023A: Repointed `server/src/app/api/secrets/route.ts` to `@alga-psa/tenancy/actions` to remove the hidden stale import found by the build.
- F023B: Repointed `server/src/app/api/secrets/[name]/route.ts` to `@alga-psa/tenancy/actions` so the build no longer looks for the deleted server duplicate.
- F024: Re-ran `NODE_OPTIONS=--max-old-space-size=32768 npx nx run-many -t build --maxParallel=4` after fixing the hidden secrets API imports; the full build completed successfully.
- Discovery: added `T042A` because the PRD explicitly requires `server/src/test/teams-v2-improvements.test.ts` to pass after the media cleanup.
- T001: Confirmed the deprecated `@/lib/posthog` wrapper path has zero remaining imports anywhere outside docs and lockfiles.
- T002: Verified `server/src/lib/posthog.ts` is absent on disk after F001.
- T003: Verified `SecretsManagement.tsx` imports tenant secret helpers from `@alga-psa/tenancy/actions`.
- T004: Verified `SecretDialog.tsx` imports its secret helpers from `@alga-psa/tenancy/actions`.
- T005: Verified the deleted server-side `tenant-secret-actions.ts` duplicate is no longer present.
- T006: Confirmed `server/src/` no longer references `tenant-secret-actions` outside excluded test/docs paths.
- T007: Corrected the outdated checklist wording and verified the tenancy barrel re-exports the exact secret helpers now used by the settings UI and API routes.
- Discovery: added `T042A` because the PRD explicitly requires `server/src/test/teams-v2-improvements.test.ts` to pass after the media cleanup.
- T001: Confirmed the deprecated @/lib/posthog wrapper path has zero remaining imports anywhere outside docs and lockfiles.
- T002: Verified server/src/lib/posthog.ts is absent on disk after F001.
- T003: Verified SecretsManagement.tsx imports tenant secret helpers from @alga-psa/tenancy/actions.
- T004: Verified SecretDialog.tsx imports its secret helpers from @alga-psa/tenancy/actions.
- T005: Verified the deleted server-side tenant-secret-actions.ts duplicate is no longer present.
- T006: Confirmed server/src no longer references tenant-secret-actions outside excluded test/docs paths.
- T007: Corrected the outdated checklist wording and verified the tenancy barrel re-exports the exact secret helpers now used by the settings UI and API routes.
- T008: Verified getTeamAvatarUrl exists in formatting and delegates to getEntityImageUrl('team', teamId, tenant).
- T009: Confirmed getTeamAvatarUrl is exported directly from packages/formatting/src/avatarUtils.ts.
- T010: Verified UserService.ts imports getUserAvatarUrl from formatting instead of the deleted media package.
- T011: Verified UserService.ts imports uploadEntityImage and deleteEntityImage from @alga-psa/documents.
- T012: Verified userActions.ts imports uploadEntityImage and deleteEntityImage from @alga-psa/documents.
- T013: Verified teams avatarActions.ts imports uploadEntityImage and deleteEntityImage from @alga-psa/documents.
- T014: Verified teams avatarActions.ts imports getTeamAvatarUrl from formatting instead of media.
- T015: Verified packages/users/package.json no longer lists @alga-psa/media in dependencies.
- T016: Verified packages/users/package.json includes canonical documents and formatting dependencies.
- T017: Verified packages/teams/package.json no longer lists @alga-psa/media in dependencies.
- T018: Verified packages/teams/package.json includes canonical documents and formatting dependencies.
- T019: Confirmed server/next.config.mjs no longer contains any @alga-psa/media alias or transpile references.
- T020: Confirmed server/tsconfig.json no longer contains @alga-psa/media path mappings.
- T021: Confirmed ee/server/tsconfig.json no longer contains @alga-psa/media path mappings.
- T022: Confirmed the workflow-worker Dockerfile no longer references the deleted @alga-psa/media workspace.
- T023: Verified teams-v2-improvements.test.ts now reads packages/formatting/src/avatarUtils.ts instead of the deleted media file.
- T024: Verified the T085 assertion now checks formatting avatar utils for the team EntityType entry.
- T025: Verified the T089 assertion now checks formatting avatar utils for getTeamAvatarUrl.
- T026: Verified the entire packages/media directory has been removed from disk.
- T027: Confirmed there are no remaining @alga-psa/media references outside docs, plan artifacts, lockfiles, and AI metadata.
- T028: Re-ran npm install successfully after the dependency cleanup; see /tmp/t028-npm-install.log for the command output from this validation run.
- T029: Re-ran the full Nx build successfully; output captured in /tmp/t029-build.log.
- T030: Generated a fresh Nx graph and confirmed scripts/check-circular-deps.mjs reports no new cycles against .github/known-cycles.json.
- T031: Checked all modified source/config files and confirmed none are shim-only re-export files.
- T032: Verified packages/documents/src/lib/entityImageService.ts exports EntityType, uploadEntityImage, and deleteEntityImage for migrated callers.
- T033: Verified packages/formatting/src/avatarUtils.ts exports both getUserAvatarUrl and getTeamAvatarUrl.
- T034: Verified UserList.tsx imports getContactAvatarUrlAction alongside getUserAvatarUrlAction from @alga-psa/user-composition/actions.
- T035: Verified the avatar fetch loop branches on user.user_type === 'client' and uses getContactAvatarUrlAction(user.contact_id, user.tenant).
- T036: Verified the avatar fetch loop still uses getUserAvatarUrlAction(user.user_id, user.tenant) for non-client users.
- T037: Verified client users without contact_id fall back to a null avatar instead of querying the wrong entity type.
- T038: Verified ContactAvatarUploadProps includes a required contactName: string prop.
- T039: Verified ContactAvatarUpload passes contactName through as EntityImageUpload.entityName.
- T040: Verified ContactDetails.tsx passes editedContact.full_name to ContactAvatarUpload.
- T041: Verified ContactDetailsEdit.tsx passes contact.full_name to ContactAvatarUpload.
- T041A: Verified server/src/app/api/secrets/route.ts imports tenant secret actions from @alga-psa/tenancy/actions.
- T041B: Verified server/src/app/api/secrets/[name]/route.ts imports tenant secret actions from @alga-psa/tenancy/actions.
- T042A: Updated `server/src/test/teams-v2-improvements.test.ts` to read the existing `packages/user-composition/src/lib/avatarUtils.ts` source and to assert the current TeamDetails/UserManagement structure before rerunning the test successfully.
- T042: Reused the successful full-build validation from T029, which ran after all planned feature work was complete.