fix(platform): unify org identity — drop orgSlug from public action surface#1734
Merged
Conversation
…urface
Non-`default` org users hit `ORG_FORBIDDEN` on the chat page (and most
dashboard surfaces) because ~30 frontend callsites hardcoded
`orgSlug: 'default'` when calling Convex actions. Root cause: actions
took both `organizationId` (Better Auth doc id) and `orgSlug`, but the
frontend had no clean path to the current org's slug, so developers
substituted the literal.
Make `organizationId` the single public identity. Slugs are an
implementation detail (auth check + filesystem paths) resolved
server-side from the doc id via existing `resolveOrgSlug(ctx, id)`:
- Drop `orgSlug` from public action signatures in providers/agents/
integrations/threads/moderation/governance.
- Remove the `args.orgSlug ?? 'default'` dev fallback from 4 internal
actions (resolveModelData / resolveModelByTag /
getAll(Configured)ModelIds) — required `organizationId` instead.
- Helper functions (`resolveLanguageModel*`, `resolveTtsModel`,
`analyzeImage`, `processAttachments`, ...) and ~15 callers updated
to forward `organizationId`; pre-resolved-slug call patterns folded
back into the internal action.
- Rename frontend hooks `useListProviders` / `useListAgents` /
`useIntegrations` to take `organizationId`. Thread the prop through
`IntegrationPanel`, `useIntegrationManage`,
`TopAgents{,Feedback}Table`, `MessageEditor`.
- Fix `useChatAgents` (the param was prefixed `_organizationId` and
the body hardcoded `'default'`).
- Open code-execution endpoint `/api/v1/models` now resolves the
caller's active org (same path the chat completion endpoint uses)
instead of relying on the removed fallback.
Schema, RLS, Better Auth integration, and the orgSlug-keyed on-disk
layout under `providers/<slug>/`, `agents/<slug>/`, etc. are unchanged
— slugs remain the human-readable filesystem key.
Also fixes a swallowed-error in `delegation.tsx` (empty catch → logs
the failure) along the way.
Contributor
📝 WalkthroughWalkthroughThis PR systematically refactors the platform to use Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/platform/convex/agents/file_actions.ts (1)
459-469:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't fail the action after the delete already committed.
cleanupAgentBindingnow runs after the file and history directory are removed. If that mutation throws, callers get an error even though the destructive part already succeeded, and the binding stays stale until a retry. Handle the cleanup failure explicitly here or move the failure boundary before the filesystem delete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/convex/agents/file_actions.ts` around lines 459 - 469, The deletion commits before calling ctx.runMutation(internal.agents.mutations.cleanupAgentBinding) so if that mutation throws we return an error even though filePath and historyDir were already removed; change the control flow to either call cleanupAgentBinding before unlink(filePath)/rm(historyDir) or (preferably) wrap the ctx.runMutation(...) call in a try/catch that handles failures explicitly (log the error and swallow or enqueue a retry) so the overall action does not fail after the destructive deletes; reference unlink, rm, ctx.runMutation, internal.agents.mutations.cleanupAgentBinding, filePath, historyDir, args.organizationId and args.agentName when making the change.services/platform/convex/openai_compat/http_actions.ts (1)
693-722: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winExtract the auth + organization-resolution block into a shared helper.
modelsListHandlernow duplicates the same request-authentication andx-organization-slugresolution flow used earlier inchatCompletionsHandler. In tenant-scoped endpoints this kind of drift is easy to miss, and the next header or error-mapping tweak can fix one route while leaving the other behind.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/convex/openai_compat/http_actions.ts` around lines 693 - 722, Extract the duplicated auth + organization-resolution logic into a shared helper (e.g., create a function like authenticateAndResolveOrg) and replace the blocks in modelsListHandler and chatCompletionsHandler with calls to it; the helper should call authenticateRequest, read the x-organization-slug header (handling both 'x-organization-slug' and 'X-Organization-Slug'), run ctx.runQuery(internal.openai_compat.internal_queries.resolveUserOrganization, { userId, orgSlug }), map AuthError to openAIErrorResponse with the same parameters used today, and map resolution errors to openAIErrorResponse preserving the existing error message behaviour so both handlers reuse identical error mapping and header handling.services/platform/convex/lib/agent_chat/internal_actions.ts (1)
330-347: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUpdate the nearby orgSlug comment to match the new resolution flow.
The code here now resolves models by
organizationId, but the comment still saysorgSlugis shared across model lookup. That drift makes this multi-tenant path harder to debug the next time resolver behavior changes.As per coding guidelines, "Comments should explain why, rarely what; well-named identifiers should be self-documenting".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/convex/lib/agent_chat/internal_actions.ts` around lines 330 - 347, The comment above the model resolution block is outdated (mentions orgSlug) and should be updated to reflect that model resolution now uses organizationId; edit the comment near parseModelRef/currentModelId and the calls to resolveLanguageModelById and resolveLanguageModelWithFallback to state that organizationId is resolved once in the outer Promise.all and is shared across model lookup, delegation, and workflows so multi-tenant deployments read each org's own provider/API-key files — keep it brief and explain why (clarify multi-tenant resolution uses organizationId), not how.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@services/platform/app/features/settings/integrations/hooks/use-integration-manage.ts`:
- Around line 636-640: The install gating and OAuth URL generation currently use
integration.organizationId instead of the organizationId passed into the hook,
which can skip installs or generate reauth URLs for the wrong org; update the
checks and calls to use the incoming organizationId parameter consistently
(e.g., replace uses of integration.organizationId in the credentialId === slug
conditional and in the installFn({ slug, organizationId }) invocation and the
similar blocks around installFn and OAuth URL construction at the other
occurrences) so all install gating, installFn calls, and OAuth URL args rely on
the provided organizationId rather than integration.organizationId.
In `@services/platform/convex/agents/file_actions.ts`:
- Around line 69-79: The readAgent action currently only verifies
authComponent.getAuthUser(ctx) and then calls
resolveOrgSlug(args.organizationId) which is only an ID→slug lookup; insert an
explicit org membership/role check (use the existing guard requireOrgMembership
or equivalent) immediately after obtaining authUser and before calling
resolveOrgSlug/readAgentFile so the authenticated user is authorized for
args.organizationId; apply the same pattern for other tenant-scoped handlers in
this file (e.g., any write/delete actions) by calling requireOrgMembership(ctx,
authUser, args.organizationId) (or the project’s requireOrgMembership signature)
to enforce membership/role authorization.
In `@services/platform/convex/providers/file_actions.ts`:
- Around line 601-603: The provider fallback depends on filesystem order; update
loadAllProviders() to sort the provider file list (e.g., sort filenames returned
by readdir()) before loading/parsing so provider order is deterministic, then
use that sorted list wherever handlers call loadAllProviders() (e.g., the
handler using resolveOrgSlug and the other occurrence around lines 762-764) to
ensure unqualified model or tag-only lookups pick the same provider across
machines.
- Around line 352-353: The module mixes orgSlug and organizationId causing
frontend coupling; migrate the remaining public provider actions (readProvider,
saveProvider, deleteProvider, getAllProviderConfigs, the provider fetch/test
actions, and secret-management actions) to accept args.organizationId instead of
args.orgSlug, update any internal lookups/queries that currently use orgSlug to
use organizationId (or resolve slug→id centrally before use), and adjust call
sites to pass organizationId; ensure param validation schemas are updated and
any references to orgSlug in these function signatures and downstream DB/service
calls are replaced so the public provider API is consistently using
organizationId like listProviders.
In
`@services/platform/convex/workflow_engine/helpers/nodes/llm/utils/resolve_chat_model.ts`:
- Around line 18-22: The exported function resolveChatModel currently lacks an
explicit return type; update its signature to annotate the return type (e.g.,
resolveChatModel(ctx: ActionCtx, config: LLMNodeConfig, organizationId: string):
Promise<YourReturnType>) so it matches repository standards for exported APIs.
Inspect usages of resolveChatModel and the types it constructs/returns to pick
the correct exported type (replace YourReturnType with the actual type or a
union), import that type if necessary, and ensure the signature uses the
explicit Promise<...> form if it returns asynchronously.
---
Outside diff comments:
In `@services/platform/convex/agents/file_actions.ts`:
- Around line 459-469: The deletion commits before calling
ctx.runMutation(internal.agents.mutations.cleanupAgentBinding) so if that
mutation throws we return an error even though filePath and historyDir were
already removed; change the control flow to either call cleanupAgentBinding
before unlink(filePath)/rm(historyDir) or (preferably) wrap the
ctx.runMutation(...) call in a try/catch that handles failures explicitly (log
the error and swallow or enqueue a retry) so the overall action does not fail
after the destructive deletes; reference unlink, rm, ctx.runMutation,
internal.agents.mutations.cleanupAgentBinding, filePath, historyDir,
args.organizationId and args.agentName when making the change.
In `@services/platform/convex/lib/agent_chat/internal_actions.ts`:
- Around line 330-347: The comment above the model resolution block is outdated
(mentions orgSlug) and should be updated to reflect that model resolution now
uses organizationId; edit the comment near parseModelRef/currentModelId and the
calls to resolveLanguageModelById and resolveLanguageModelWithFallback to state
that organizationId is resolved once in the outer Promise.all and is shared
across model lookup, delegation, and workflows so multi-tenant deployments read
each org's own provider/API-key files — keep it brief and explain why (clarify
multi-tenant resolution uses organizationId), not how.
In `@services/platform/convex/openai_compat/http_actions.ts`:
- Around line 693-722: Extract the duplicated auth + organization-resolution
logic into a shared helper (e.g., create a function like
authenticateAndResolveOrg) and replace the blocks in modelsListHandler and
chatCompletionsHandler with calls to it; the helper should call
authenticateRequest, read the x-organization-slug header (handling both
'x-organization-slug' and 'X-Organization-Slug'), run
ctx.runQuery(internal.openai_compat.internal_queries.resolveUserOrganization, {
userId, orgSlug }), map AuthError to openAIErrorResponse with the same
parameters used today, and map resolution errors to openAIErrorResponse
preserving the existing error message behaviour so both handlers reuse identical
error mapping and header handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 567e00e8-67b3-4f69-a69f-356e897527e5
📒 Files selected for processing (78)
services/platform/app/features/agents/components/agent-create-dialog.tsxservices/platform/app/features/agents/components/agent-delete-dialog.tsxservices/platform/app/features/agents/components/agent-navigation.tsxservices/platform/app/features/agents/components/agent-row-actions.tsxservices/platform/app/features/agents/components/agents-action-menu.tsxservices/platform/app/features/agents/hooks/mutations.tsservices/platform/app/features/agents/hooks/queries.tsservices/platform/app/features/analytics/feedback/feedback-metrics-page.tsxservices/platform/app/features/analytics/feedback/top-agents-feedback-table.tsxservices/platform/app/features/analytics/usage/top-agents-table.tsxservices/platform/app/features/analytics/usage/usage-metrics-page.tsxservices/platform/app/features/automations/triggers/components/schedule-create-dialog.tsxservices/platform/app/features/chat/components/arena/arena-model-selector.tsxservices/platform/app/features/chat/components/chat-interface.tsxservices/platform/app/features/chat/components/model-selector.tsxservices/platform/app/features/chat/components/shared-chat-view.tsxservices/platform/app/features/chat/hooks/queries.tsservices/platform/app/features/chat/hooks/use-composer-capabilities.tsservices/platform/app/features/conversations/components/conversation-panel.tsxservices/platform/app/features/conversations/components/message-editor.test.tsxservices/platform/app/features/conversations/components/message-editor.tsxservices/platform/app/features/conversations/components/message-editor/types.tsxservices/platform/app/features/settings/governance/components/default-model-editor.tsxservices/platform/app/features/settings/governance/components/model-access-editor.tsxservices/platform/app/features/settings/governance/components/moderation-provider-config.tsxservices/platform/app/features/settings/integrations/components/integration-panel.tsxservices/platform/app/features/settings/integrations/components/integration-upload/integration-upload-dialog.tsxservices/platform/app/features/settings/integrations/components/integrations.tsxservices/platform/app/features/settings/integrations/hooks/queries.tsservices/platform/app/features/settings/integrations/hooks/use-integration-manage.tsservices/platform/app/features/settings/providers/hooks/queries.tsservices/platform/app/routes/dashboard/$id/agents/$agentId/conversation-starters.tsxservices/platform/app/routes/dashboard/$id/agents/$agentId/delegation.tsxservices/platform/app/routes/dashboard/$id/agents/$agentId/index.tsxservices/platform/app/routes/dashboard/$id/agents/$agentId/instructions.tsxservices/platform/convex/agent_tools/files/helpers/analyze_image.tsservices/platform/convex/agent_tools/files/helpers/analyze_image_by_url.tsservices/platform/convex/agent_tools/files/image_tool.tsservices/platform/convex/agent_tools/files/internal_actions.tsservices/platform/convex/agent_tools/human_input/actions.tsservices/platform/convex/agent_tools/integrations/trigger_completion_action.tsservices/platform/convex/agent_tools/location/actions.tsservices/platform/convex/agent_tools/workflows/trigger_completion_action.test.tsservices/platform/convex/agent_tools/workflows/trigger_completion_action.tsservices/platform/convex/agents/file_actions.test.tsservices/platform/convex/agents/file_actions.tsservices/platform/convex/agents/image_generation/run_image_generation.tsservices/platform/convex/agents/translate_fields.tsservices/platform/convex/agents/webhooks/internal_actions.tsservices/platform/convex/conversations/actions.tsservices/platform/convex/file_metadata/transcribe_audio.tsservices/platform/convex/governance/moderation_provider/internal_actions.tsservices/platform/convex/governance/moderation_provider/test_action.tsservices/platform/convex/governance/sanitize.tsservices/platform/convex/integrations/file_actions.tsservices/platform/convex/lib/agent_chat/internal_actions.tsservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/lib/agent_response/generate_response.tsservices/platform/convex/lib/attachments/process_attachments.test.tsservices/platform/convex/lib/attachments/process_attachments.tsservices/platform/convex/lib/summarization/internal_actions.tsservices/platform/convex/openai_compat/http_actions.tsservices/platform/convex/openai_compat/internal_actions.tsservices/platform/convex/prompts/actions.tsservices/platform/convex/prompts/generate_title.tsservices/platform/convex/providers/failover.test.tsservices/platform/convex/providers/failover.tsservices/platform/convex/providers/file_actions.tsservices/platform/convex/providers/resolve_model.tsservices/platform/convex/providers/resolve_tts_model.test.tsservices/platform/convex/threads/edit_and_branch.tsservices/platform/convex/threads/fork_and_chat.tsservices/platform/convex/threads/generate_thread_title.tsservices/platform/convex/tts/synthesize.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/execute_llm_node.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/resolve_chat_model.test.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/resolve_chat_model.tsservices/platform/convex/workflows/triggers/actions.ts
💤 Files with no reviewable changes (15)
- services/platform/convex/agent_tools/human_input/actions.ts
- services/platform/app/features/settings/governance/components/moderation-provider-config.tsx
- services/platform/convex/agent_tools/workflows/trigger_completion_action.test.ts
- services/platform/convex/agents/webhooks/internal_actions.ts
- services/platform/convex/governance/moderation_provider/internal_actions.ts
- services/platform/convex/governance/moderation_provider/test_action.ts
- services/platform/convex/threads/fork_and_chat.ts
- services/platform/app/features/chat/components/shared-chat-view.tsx
- services/platform/convex/agent_tools/location/actions.ts
- services/platform/convex/agent_tools/workflows/trigger_completion_action.ts
- services/platform/convex/lib/agent_chat/start_agent_chat.ts
- services/platform/convex/threads/edit_and_branch.ts
- services/platform/convex/lib/agent_response/generate_response.ts
- services/platform/convex/agent_tools/integrations/trigger_completion_action.ts
- services/platform/convex/governance/sanitize.ts
Comment on lines
636
to
640
| if (credentialId === slug && slug && integration.organizationId) { | ||
| const installResult = await installFn({ | ||
| orgSlug: 'default', | ||
| slug, | ||
| organizationId: integration.organizationId, | ||
| organizationId, | ||
| }); |
Contributor
There was a problem hiding this comment.
Use the passed organizationId consistently for install gating and OAuth URL generation.
These paths still depend on integration.organizationId for install preconditions and OAuth URL args. If that field is absent/stale, install is skipped or reauth is requested against the wrong org.
🔧 Proposed fix
- if (credentialId === slug && slug && integration.organizationId) {
+ if (credentialId === slug && slug) {
const installResult = await installFn({
slug,
organizationId,
});
credentialId = installResult.credentialId;
}
@@
- if (credentialId === slug && slug && integration.organizationId) {
+ if (credentialId === slug && slug) {
const installResult = await installFn({
slug,
organizationId,
});
credentialId = installResult.credentialId;
}
@@
- if (credentialId === slug && slug && integration.organizationId) {
+ if (credentialId === slug && slug) {
const installResult = await installFn({
slug,
organizationId,
});
credentialId = installResult.credentialId;
}
@@
const authUrl = await generateOAuth2Url({
credentialId: toId<'integrationCredentials'>(credentialId),
- organizationId: integration.organizationId ?? '',
+ organizationId,
});Also applies to: 753-757, 809-820
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/settings/integrations/hooks/use-integration-manage.ts`
around lines 636 - 640, The install gating and OAuth URL generation currently
use integration.organizationId instead of the organizationId passed into the
hook, which can skip installs or generate reauth URLs for the wrong org; update
the checks and calls to use the incoming organizationId parameter consistently
(e.g., replace uses of integration.organizationId in the credentialId === slug
conditional and in the installFn({ slug, organizationId }) invocation and the
similar blocks around installFn and OAuth URL construction at the other
occurrences) so all install gating, installFn calls, and OAuth URL args rely on
the provided organizationId rather than integration.organizationId.
Comment on lines
69
to
+79
| export const readAgent = action({ | ||
| args: { | ||
| orgSlug: v.string(), | ||
| organizationId: v.string(), | ||
| agentName: v.string(), | ||
| }, | ||
| returns: v.any(), | ||
| handler: async (ctx, args): Promise<AgentReadResult> => { | ||
| const authUser = await authComponent.getAuthUser(ctx); | ||
| if (!authUser) throw new Error('Unauthenticated'); | ||
| return readAgentFile(args.orgSlug, args.agentName); | ||
| const orgSlug = await resolveOrgSlug(ctx, args.organizationId); | ||
| return readAgentFile(orgSlug, args.agentName); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== resolveOrgSlug implementation ==="
fd 'resolve_org_slug.ts' services/platform/convex --exec sed -n '1,220p' {}
echo
echo "=== auth/access checks in agent file actions ==="
rg -n -C2 "resolveOrgSlug\\(ctx, args\\.organizationId\\)|requireOrgMembership\\(|requireDeveloperSettingsAccess\\(|getAuthUser\\(" \
services/platform/convex/agents/file_actions.tsRepository: tale-project/tale
Length of output: 5171
Fix cross-tenant access control: resolveOrgSlug() is only an ID→slug lookup (no membership/role gate).
resolveOrgSlug() just fetches the organization and returns its slug; it doesn’t validate that the authenticated user belongs to (or has access to) args.organizationId. In services/platform/convex/agents/file_actions.ts, the public actions only check authComponent.getAuthUser(ctx) and then proceed with resolveOrgSlug(ctx, args.organizationId) for tenant-scoped reads/writes—so another org’s organizationId can be supplied without an authorization check.
Add an explicit org membership/role authorization before resolving the slug / accessing tenant-scoped files (e.g., the existing requireOrgMembership(...)/equivalent guard used elsewhere in the codebase).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/agents/file_actions.ts` around lines 69 - 79, The
readAgent action currently only verifies authComponent.getAuthUser(ctx) and then
calls resolveOrgSlug(args.organizationId) which is only an ID→slug lookup;
insert an explicit org membership/role check (use the existing guard
requireOrgMembership or equivalent) immediately after obtaining authUser and
before calling resolveOrgSlug/readAgentFile so the authenticated user is
authorized for args.organizationId; apply the same pattern for other
tenant-scoped handlers in this file (e.g., any write/delete actions) by calling
requireOrgMembership(ctx, authUser, args.organizationId) (or the project’s
requireOrgMembership signature) to enforce membership/role authorization.
Comment on lines
352
to
+353
| export const listProviders = action({ | ||
| args: { orgSlug: v.string() }, | ||
| args: { organizationId: v.string() }, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Finish migrating the remaining public provider actions off orgSlug.
listProviders now takes organizationId, but this module still exposes readProvider, saveProvider, deleteProvider, getAllProviderConfigs, the fetch/test actions, and secret-management actions on orgSlug. That keeps the frontend coupled to slugs for settings flows and leaves the public provider API in a mixed state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/providers/file_actions.ts` around lines 352 - 353,
The module mixes orgSlug and organizationId causing frontend coupling; migrate
the remaining public provider actions (readProvider, saveProvider,
deleteProvider, getAllProviderConfigs, the provider fetch/test actions, and
secret-management actions) to accept args.organizationId instead of
args.orgSlug, update any internal lookups/queries that currently use orgSlug to
use organizationId (or resolve slug→id centrally before use), and adjust call
sites to pass organizationId; ensure param validation schemas are updated and
any references to orgSlug in these function signatures and downstream DB/service
calls are replaced so the public provider API is consistently using
organizationId like listProviders.
Comment on lines
+601
to
603
| handler: async (ctx, args) => { | ||
| const orgSlug = await resolveOrgSlug(ctx, args.organizationId); | ||
| const providers = await loadAllProviders(orgSlug); |
Contributor
There was a problem hiding this comment.
Make provider fallback selection deterministic.
Both resolver paths still depend on the order returned by loadAllProviders(). Because that order comes from readdir(), an unqualified model id or tag-only lookup can silently pick a different provider across machines or after file rewrites. Sort provider files once before loading so the implicit fallback is stable.
Possible fix in loadAllProviders()
const jsonFiles = entries.filter(
(e) =>
e.endsWith('.json') && !e.startsWith('.') && !e.endsWith('.secrets.json'),
);
+ jsonFiles.sort((a, b) => a.localeCompare(b));Also applies to: 762-764
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/providers/file_actions.ts` around lines 601 - 603,
The provider fallback depends on filesystem order; update loadAllProviders() to
sort the provider file list (e.g., sort filenames returned by readdir()) before
loading/parsing so provider order is deterministic, then use that sorted list
wherever handlers call loadAllProviders() (e.g., the handler using
resolveOrgSlug and the other occurrence around lines 762-764) to ensure
unqualified model or tag-only lookups pick the same provider across machines.
Comment on lines
18
to
22
| export async function resolveChatModel( | ||
| ctx: ActionCtx, | ||
| config: LLMNodeConfig, | ||
| orgSlug: string, | ||
| organizationId: string, | ||
| ) { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add an explicit return type on exported resolveChatModel.
This exported API currently relies on inferred return typing; please annotate it explicitly to match the repository typing standard.
Proposed diff
export async function resolveChatModel(
ctx: ActionCtx,
config: LLMNodeConfig,
organizationId: string,
-) {
+): Promise<Awaited<ReturnType<typeof resolveLanguageModelWithFallback>>> {As per coding guidelines, **/*.{ts,tsx}: "Use implicit typing where inference is obvious; annotate public APIs, exported functions, and anywhere the inferred type would be confusing".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/convex/workflow_engine/helpers/nodes/llm/utils/resolve_chat_model.ts`
around lines 18 - 22, The exported function resolveChatModel currently lacks an
explicit return type; update its signature to annotate the return type (e.g.,
resolveChatModel(ctx: ActionCtx, config: LLMNodeConfig, organizationId: string):
Promise<YourReturnType>) so it matches repository standards for exported APIs.
Inspect usages of resolveChatModel and the types it constructs/returns to pick
the correct exported type (replace YourReturnType with the actual type or a
union), import that type if necessary, and ensure the signature uses the
explicit Promise<...> form if it returns asynchronously.
…ty review Multi-agent review of 41f355f surfaced four classes of regression and new vulnerabilities the unification commit left exposed. This addresses all CRITICAL/HIGH/MEDIUM findings. CRITICAL — four frontend regressions: renamed hooks were still being passed `orgSlug` (or the literal `'default'`) where backend now expects the Better Auth doc id, breaking every non-default org's dashboard at `resolveOrgSlug`. Fixed in `$agentId.tsx`, `agents-table.tsx`, `settings/integrations.tsx`, `providers-table.tsx`. HIGH — cross-tenant authz gap. New helper `requireOrgMembershipById` (`convex/lib/auth/require_org_membership.ts`) folds auth + slug-resolve + membership check into one call, throwing stable ConvexError codes (`UNAUTHENTICATED` / `ORG_NOT_FOUND` / `ORG_FORBIDDEN`). Applied to 22 public actions in `agents/file_actions.ts`, `integrations/file_actions .ts`, `threads/{edit_and_branch,fork_and_chat}.ts`, and the three newly-org-aware actions `improveMessage`, `generateCronExpression`, `translateAgentFields` — all of which previously only checked `getAuthUser` and trusted whatever `organizationId` the client sent. HIGH — `/v1/models` X-Organization-Slug leak. `resolveUserOrganization` now membership-checks the header so an API key for org A can't enumerate org B's model catalog. Returns the canonical org slug rather than the header echo, and `modelsListHandler` maps non-membership errors to 403. MEDIUM: - Threads: `editAndBranch` / `forkAndChat` derive `organizationId` from source-thread metadata server-side and assert client-supplied arg matches. `create_branch_thread` now persists `organizationId` on the new branch's threadMetadata row (was previously dropped, orphaning branches from cross-tenant filtering). - `/v1/models`: multi-org users without `X-Organization-Slug` now fall back to `lastActiveOrganizationId` before throwing. New `openai:models` rate-limit bucket (120 rpm) and IP check before auth, mirroring chat-completions. - Provider migration completed: the 9 remaining public actions in `providers/file_actions.ts` (`readProvider`, `saveProvider`, `deleteProvider`, `getAllProviderConfigs`, `fetchProviderModels`, `fetchConfiguredProviderModels`, `testProviderConnection`, `saveProviderSecret`, `hasProviderSecret`) now take `organizationId`. New `requireOrgMembershipById` / `requireDeveloperSettingsAccessById` wrappers in `providers/auth.ts`. Frontend hooks (`queries.ts`/`mutations.ts`) and every component consumer migrated; resolves the stale-cache mismatch where invalidations keyed by slug never refreshed queries keyed by id. - `getAllConfiguredModelIds` logs `readdir`, invalid-name, and `readProviderFile` failures instead of silently returning `[]` — consumers (notably `saveAgent`) can no longer mistake "providers malformed" for "no providers configured". Tests: - New `convex/organizations/resolve_org_slug.test.ts`, `convex/lib/auth/require_org_membership.test.ts`, `convex/openai_compat/internal_queries.test.ts` covering happy path, every throw branch, membership rejection on the header path, lastActive fallback, and no-memberships. - `convex/agents/file_actions.test.ts` updated to mock the new helper; unauthenticated cases assert the new ConvexError shape. `bun run check` clean: 267 test files, 70,752 tests, 0 lint warnings, 0 typecheck errors.
Multi-agent review of the unified-org-identity branch (42 sub-agents
across two rounds) surfaced one HIGH regression introduced by the
migration plus three MEDIUM issues. All four addressed here; bun run
check stays clean (267 test files, 70,752 tests, 0 lint warnings, 0
typecheck errors).
HIGH — SSE file-event invalidations broken.
`use-file-events.ts` was invalidating React Query caches with the slug
from the SSE payload (`['config', type, orgSlug]`), but the cache-key
factory now keys by organizationId. External JSON edits (CLI, git
pull, operator hand-edits) silently stopped refreshing the dashboard.
Fix: client-side slug→id translation via
`useUserOrganizationsWithDetails` — keep a Map in a ref so the SSE
listener doesn't re-subscribe on every membership refresh. Events for
orgs the user isn't a member of are skipped silently. Also rename the
`configKeys` parameter from `orgSlug` to `organizationId` so the bug
can't recur from a stale parameter name.
MEDIUM — /v1/chat/completions error-code asymmetry.
`modelsListHandler` already mapped `Not a member` to 403; the chat
endpoint mapped everything to 400, leaving a higher-volume
cross-tenant probe surface. Extracted shared `mapResolveOrgError`
helper used by both handlers.
MEDIUM — frontend ORG_FORBIDDEN / ORG_NOT_FOUND / UNAUTHENTICATED
toasts. The new auth helper throws stable ConvexError codes, but the
provider-area toasts rendered the generic "Failed to create/save…"
string for membership-revocation, so users had no way to tell a real
operation failure from a stale-org-id one. Added
`dispatchOrgAccessError(err, tAccessDenied)` reusing the existing
top-level `accessDenied` namespace (workspaceNotFound, noMembership,
title), plus a new `unauthenticated` key in en/de/fr (de-CH override
not needed — ß-free). Wired into all 7 provider toast sites
(add-panel, detail-drawer ×4, edit-panel, providers-table). Also
fixes a bare `catch {}` at `providers-table.tsx` that violated
CLAUDE.md's no-empty-catch rule.
MEDIUM — provider silent `[]` catches. The unification commit fixed
`getAllConfiguredModelIds` to distinguish ENOENT from real readdir
failures but left the same pattern in three siblings (`listProviders`,
`getAllProviderConfigs`, `getAllModelIds`). Applied the same fix.
`getAllModelIds` additionally preserves the `NoProviderAvailableError
.{reason, details[]}` context that `loadAllProviders` deliberately
attaches.
Out of scope (filed as follow-ups, see plan): forkAndChat write
ordering (LOW), fork_own_thread missing organizationId (pre-existing
LOW), dead useInstallIntegration hook, and the slug-enumeration
oracle hardening for both compat endpoints.
…erge Resolve semantic conflict introduced by merging main (#1732 added the loader using orgSlug) into the org-identity unification branch: switch useReadProvider + ProviderConfigProvider calls to organizationId.
yannickmonney
added a commit
that referenced
this pull request
May 24, 2026
…gSlug from transcribe_dictation - jsdom doesn't ship ResizeObserver; MobileBottomNav uses it for nav height CSS var, which broke the dashboard layout test after the mobile-shell merge. - transcribe_dictation.ts still passed orgSlug to resolveTranscriptionModel after main #1734 unified on organizationId for the public action surface.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
defaultorg users hitORG_FORBIDDENon the chat page (and most dashboard surfaces) because ~30 frontend callsites hardcodedorgSlug: 'default'when calling Convex actions. Reported ondemo.tale.dev/dashboard/{orgDocId}/chat.organizationId(Better Auth doc id) andorgSlug, but the frontend had no clean path to the current org's slug, so developers substituted the literal — only worked when the user happened to also be a member ofdefault.organizationIdthe single public identity. Slugs become an implementation detail (auth check + filesystem paths), resolved server-side via the existingresolveOrgSlug(ctx, id).What changes
orgSlugfrom public action signatures across providers / agents / integrations / threads / moderation / governance.args.orgSlug ?? 'default'dev fallback from 4 internal actions (resolveModelData/resolveModelByTag/getAllModelIds/getAllConfiguredModelIds) —organizationIdis now required.resolveLanguageModel*,resolveTtsModel,analyzeImage,processAttachments, …) and ~15 callers updated to forwardorganizationIdinstead of pre-resolved slugs.useListProviders/useListAgents/useIntegrationsto takeorganizationId. Thread the prop throughIntegrationPanel,useIntegrationManage,TopAgents{,Feedback}Table,MessageEditor.useChatAgents(param was_organizationId— silently ignored — and body hardcoded'default')./api/v1/models(OpenAI-compat) now resolves the caller's active org (same path the chat completion endpoint already uses) instead of leaning on the removed fallback.delegation.tsxhad an emptycatch {}(violates the no-empty-catch rule) — now logs the failure.What does NOT change
providers/<slug>/,agents/<slug>/,integrations/@<slug>/, etc. — slugs remain the human-readable key on disk.Test plan
bun run check(oxfmt + lint + typecheck + 73122 tests across 264 files for@tale/platform, plus crawler/workspace) — greendefaultorg account ondemo.tale.dev:/dashboard/{orgDocId}/chat— noORG_FORBIDDEN; model selector loads; edit-and-branch works/dashboard/{orgDocId}/agents— list / create / duplicate / delete; instructions / delegation / conversation-starters / history tabs/dashboard/{orgDocId}/settings/governance— default model, model access, moderation provider test/dashboard/{orgDocId}/settings/integrations— panel open, upload package, test connection, OAuth2 reauth, uninstall/dashboard/{orgDocId}/analytics/usageand/analytics/feedback— top-agents tables show display namesdefaultmember and another org member — verify nothing got worseGET /api/v1/modelsreturns the caller's org models (notdefault's)Summary by CodeRabbit