fix(platform): hide uninstalled app agents from assignee and mention pickers#2578
fix(platform): hide uninstalled app agents from assignee and mention pickers#2578larryro wants to merge 2 commits into
Conversation
…pickers listAgents scanned every on-disk app bundle under org/apps/, so uploaded-but- never-installed private apps leaked agents into assignee and @-mention pickers. Use appInstallations rows via listAppInstallationsInternal instead. Closes #2564 Co-authored-by: Cursor <cursoragent@cursor.com>
Desk reviewer grade — #2564Code verdict: Approve — the fix correctly addresses the reported bug. What works
Tests
Nits (non-blocking):
CI gate (blocking merge per desk rules)
Overall desk verdict: NOT READY — CHANGES REQUIRED (code is sound; CI not green). |
larryro
left a comment
There was a problem hiding this comment.
Desk reviewer grade — #2564 (rework 1/3)
Code verdict: Approve — the fix correctly addresses the reported bug. No code changes since prior review (db1a3de98).
What works
- Root cause confirmed:
listAgentsusedlistInstalledAppSlugsFromDisk, listing every bundle underorg/apps/including uploaded-but-never-installed private apps. - Fix is minimal and reuses the authoritative signal:
listAppInstallationsInternal(appInstallationsrows), already used bybuiltin_sync.ts. - Picker data path confirmed:
useActorDirectory→useListAgents→api.agents.file_actions.listAgentsfeeds assignee and @-mention pickers. - Uninstall case covered: private bundles remain on disk, but DB row deletion correctly hides agents.
- Doc comment on
listInstalledAppSlugsFromDiskclarifies disk bundles ≠ installed apps.
Tests
bunx vitest --run --project server convex/agents/file_actions.test.ts -t "listAgents app scope" → 2/2 passed (23/23 full file). Regression tests lock the listAppInstallationsInternal query swap.
Nits (non-blocking, unchanged from prior review):
issue-desk-qa/exclusion assertion is tautological with current mocks — stub a second on-disk-only bundle to make the negative case meaningful.listAppInstallationsInternaldoes not filteruninstalling: truerows (agents may linger briefly mid-uninstall; same pattern asbuiltin_sync.ts).- Commit includes
Co-authored-by:(repo discourages).
Automated review rounds
- Round 1 (adversarial): Correctness, security/boundaries, reuse, tests, blast radius — no blocking code defects.
- Round 2 (confirmation): CI failures reproduced; confirmed unrelated to four-file backend diff. Main
Buildalso failsValidate images; #2579 documents repo-wideModulesTooLargeregression onmain. - Bugbot / security-review: Could not compute branch diff in harness; manual read covered same axes. No security issues.
CI gate (blocking merge per desk rules)
| Check | Status | Notes |
|---|---|---|
| Unit, Lint, Type, Migrations, Opengrep, UI, Browser | ✅ | |
| Smoke test | ✅ | Cleared since prior review |
| Playwright (platform 1–16) | ❌ | ModulesTooLarge (42.97 MiB > 42.92 MiB) — repo-wide infra (#2579), not introduced by this PR |
| Validate images | ❌ | 39/40 — also fails on main@84384c25a |
Overall desk verdict: NOT READY — CHANGES REQUIRED — implementation is merge-ready; CI gate is not green.
Simulate an on-disk-only issue-desk-qa bundle and assert readdir is never called for it when only issue-desk is installed. Co-authored-by: Cursor <cursoragent@cursor.com>
larryro
left a comment
There was a problem hiding this comment.
Desk reviewer grade — #2564 (rework 2/3)
Code verdict: Approve — fix is correct and complete; rework 2 addresses the prior test nit.
What works
- Root cause fixed:
listAgentsno longer callslistInstalledAppSlugsFromDisk(every bundle underorg/apps/). It now querieslistAppInstallationsInternal, the authoritativeappInstallationssignal — same pattern asbuiltin_sync.ts. - Picker path confirmed:
useActorDirectory→useListAgents→api.agents.file_actions.listAgentsfeeds both the task Assignee picker and comment @-mention picker. - Uninstall case covered: private bundles remain on disk after uninstall, but DB row deletion correctly hides agents.
- Doc comment clarified:
listInstalledAppSlugsFromDisknow documents disk bundles ≠ installed apps; hub listing still uses disk scan as intended. - Rework 2 test improvement:
file_actions.test.tsnow simulates an on-disk-onlyissue-desk-qabundle and assertsreaddiris never called for it when onlyissue-deskis installed — the negative case is no longer tautological.
Tests
bunx vitest --run --project server convex/agents/file_actions.test.ts -t "listAgents app scope" → 2/2 passed.
Automated review rounds
- Round 1 (adversarial): Correctness ✓ — query swap fixes uploaded-but-uninstalled leak and post-uninstall disk retention. Security/boundaries ✓ —
requireOrgMembershipByIdstill gates the action; internal query uses the sameorganizationId. Reuse ✓ — no divergent install signal. Tests ✓ — handler + query regression. Blast radius swept ✓ —workflows/file_actions.tsdisk scan acknowledged out of scope. - Round 2 (confirmation): Bugbot/security-review harness could not compute branch diff; manual read covered same axes. CI failures reproduced and confirmed unrelated to the four-file backend diff.
- Non-blocking nits:
listAppInstallationsInternaldoes not filteruninstalling: truerows (agents may linger briefly mid-uninstall; same asbuiltin_sync.ts). Commits includeCo-authored-by:(repo discourages).
CI gate
| Check | Status | Notes |
|---|---|---|
| Unit, Lint, Type, Migrations, Opengrep, UI, Browser, Smoke test | ✅ | |
| Playwright (platform 1–16) | ❌ | ModulesTooLarge (42.97 MiB > 42.92 MiB) on Convex deploy — repo-wide infra (#2579), not introduced by this PR |
| Validate images | ❌ | sandbox-runtime 4878 MB > 4400 MB budget — repo-wide infra, unrelated to #2564 |
Overall desk verdict: READY — infra CI red only
71 sub-issues of the Improvements/Bugs/Performance umbrella epics fixed in one sweep: task-assignment liveness, automation install/readiness and schedule validation, contacts-merge fallout incl. the customerId contract phase, governance editors, server-side attachment caps on chat and conversations, SVG/mermaid sanitization, discussion agent mentions, org provisioning recovery, provider/SSO ergonomics, agent-editor validation, metrics consolidation under Settings, cold-load auth-handshake collapse, legacy-volume upgrade recovery, design/ + designs/ directory merge, prompts, docs and guides, crawler linkedom swap, E2E boot probe — plus same-class sweep siblings, absorbed PR #2578, a port of PR #2561, and all adversarial-review findings, incl. a versioned migration upgrading per-org run-assigned-task packs. Squashed by request; per-issue detail in the PR description.
|
Absorbed into #2674 (merged with a semantic conflict resolution onto the automations-renamed module: installations-query-based listing, test mocks updated) — plus the same installed-only fix applied to the workflows surface, which had the identical uploaded-bundle bug. #2674 closes #2564's parent class; recommend closing this PR. |
71 sub-issues of the Improvements/Bugs/Performance umbrella epics fixed in one sweep: task-assignment liveness, automation install/readiness and schedule validation, contacts-merge fallout incl. the customerId contract phase, governance editors, server-side attachment caps on chat and conversations, SVG/mermaid sanitization, discussion agent mentions, org provisioning recovery, provider/SSO ergonomics, agent-editor validation, metrics consolidation under Settings, cold-load auth-handshake collapse, legacy-volume upgrade recovery, design/ + designs/ directory merge, prompts, docs and guides, crawler linkedom swap, E2E boot probe — plus same-class sweep siblings, absorbed PR #2578, a port of PR #2561, and all adversarial-review findings, incl. a versioned migration upgrading per-org run-assigned-task packs. Squashed by request; per-issue detail in the PR description.
|
Closing as superseded: absorbed into #2674 (merged 2026-07-12), which also applied the same installed-only fix on the workflows surface. See Yannick's comment above. |
Summary
listAgentsenumerated app-owned agents by scanning every bundle on disk underorg/apps/, including uploaded-but-never-installed private apps.listAppInstallationsInternal(theappInstallationsDB table) so only installed apps contribute agents to the task assignee picker and comment @-mention picker.listInstalledAppSlugsFromDiskdoc comment: disk bundles ≠ installed apps.Closes #2564
Test plan
convex/agents/file_actions.test.ts—listAgentslists app agents only for installed apps; returns none when nothing is installedconvex/apps/install_mutations.test.ts—listAppInstallationsInternalreturns only slugs with anappInstallationsrowDefinition of done
file_actions.test.ts23/23)workflows/file_actions.tsstill uses disk scan for automations list (out of scope per issue)Made with Cursor