Skip to content

feat(platform): wire feature flags resolver and add governance tab#1375

Merged
larryro merged 5 commits into
mainfrom
feat/issue-1360-feature-flags
Apr 11, 2026
Merged

feat(platform): wire feature flags resolver and add governance tab#1375
larryro merged 5 commits into
mainfrom
feat/issue-1360-feature-flags

Conversation

@yannickmonney
Copy link
Copy Markdown
Contributor

@yannickmonney yannickmonney commented Apr 11, 2026

Summary

  • Wire the existing resolveFeatureFlags() into the chat pipeline (startAgentChat) so governance feature flag rules are enforced server-side before agent generation starts
  • Add featureFlagsConfigSchema validation to the upsertPolicy mutation (rejects invalid payloads)
  • Enforce web search disable by removing web from convexToolNames and setting webSearchMode to 'off'
  • Block file uploads with a user-visible assistant message when fileUpload is false for the user
  • Forward maxContextTokens through the scheduler args to generateAgentResponse, overriding maxHistoryTokens in all three buildStructuredContext calls
  • Build FeatureFlagsEditor admin UI component (modeled after BudgetEditor) with scope/target rules table, add/edit dialog with toggle switches, and number input for max context tokens
  • Add "Feature controls" tab to the governance settings page, migrating existing hardcoded tab labels to i18n keys under governance.tabs.*
  • Add all translation keys to en.json and de.json

Test plan

  • Unit tests for resolveFeatureFlags priority chain (user > team > role > default), partial rules, disabled policy, empty rules
  • Existing startAgentChat tests updated with mocks for new dependencies
  • FeatureFlagsEditor component tests: empty state, rules table, add button, enabled toggle, loading state
  • Accessibility tests (checkAccessibility()) for empty state and rules table
  • Storybook story for FeatureFlagsEditor
  • All server tests pass (bun run --filter @tale/platform test --project server)
  • All client tests pass (bun run --filter @tale/platform test --project client)
  • Lint passes (bun run --filter @tale/platform lint)
  • Typecheck passes (bun run --filter @tale/platform typecheck)

Closes #1360

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Feature Controls governance tab to manage feature flags (web search, file uploads, code execution) with organization-wide and scope-specific rules.
    • Feature flag rules are now enforced when agents generate responses, including context token limits.
  • Tests

    • Added comprehensive test coverage for feature flag enforcement and editor functionality.
  • Documentation

    • Added UI translations for feature flag management interface.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

This PR introduces per-user, per-team, and per-role feature flag governance enforcement for an organization. It adds a React component (FeatureFlagsEditor) to manage governance policies for web search, code execution, and file uploads alongside context token limits. A backend function (resolveFeatureFlags) determines which rules apply to a user, and enforcement is integrated into agent chat initialization to restrict capabilities based on the resolved policy. The changes include validation schemas, comprehensive test coverage, and localized UI strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main changes: wiring the feature flags resolver and adding a governance tab to the settings page.
Linked Issues check ✅ Passed All coding requirements from issue #1360 are met: per-user feature controls for web search, code execution, and context length limiting are fully implemented with proper enforcement.
Out of Scope Changes check ✅ Passed All changes directly support the feature flags implementation and governance UI; no unrelated out-of-scope modifications were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-1360-feature-flags

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Wire the existing resolveFeatureFlags() into the chat pipeline so
governance feature flag rules are enforced before agent generation.
Add validation for feature_flags config in upsertPolicy, pass
maxContextTokens through to context builder, and build a
FeatureFlagsEditor admin UI on the governance settings page.

- Call resolveFeatureFlags() in startAgentChat after budget check
- Enforce webSearch disable (remove web tool, set mode off)
- Block file uploads with assistant message when fileUpload is false
- Forward maxContextTokens through scheduler to generate_response
- Override maxHistoryTokens in all buildStructuredContext calls
- Add featureFlagsConfigSchema validation in upsertPolicy mutation
- Build FeatureFlagsEditor with scope/target rules table and dialog
- Add Feature Controls tab to governance page with i18n tab labels
- Add translation keys to en.json and de.json
- Add unit tests for resolveFeatureFlags priority chain
- Add component tests with accessibility checks
- Add Storybook story for FeatureFlagsEditor

Closes #1360
Add tests verifying governance feature flag enforcement:
- maxContextTokens forwarded to scheduled generation args
- web search disabled removes web tool and sets mode off
- file upload disabled blocks request with assistant message
- file upload disabled allows request without attachments
Replace all hardcoded English strings in feature-flags-editor with i18n
translation keys (scope/role labels, search placeholders, aria-labels,
submit text, actions header). Use formatNumber() instead of
toLocaleString(). Show loading skeleton with aria-busy instead of
returning null. Normalize maxContextTokens once before reuse in
generate_response. Remove redundant object spread in start_agent_chat.
@larryro larryro force-pushed the feat/issue-1360-feature-flags branch from eccf802 to 917e25f Compare April 11, 2026 06:54
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.test.tsx`:
- Around line 63-66: Replace the unsafe "as never" casts on
mockedUseGovernancePolicy fixtures with a typed mock that preserves the hook's
return shape: create a small typed factory or use TypeScript's "satisfies" on
the mock objects so they conform to the actual useGovernancePolicy return type
(e.g., the shape used by mockedUseGovernancePolicy in
feature-flags-editor.test.tsx) and update all instances (lines matching the
other fixtures at 78-93, 116-119, 135-150) to use that factory or "satisfies"
expression so the tests will error if the hook contract drifts.

In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`:
- Around line 255-275: Replace the hardcoded placeholder string in the Input
component for maxContextTokens with a translated string: add a new translation
key (e.g., "featureFlags.maxContextTokensPlaceholder" or
"maxContextTokensPlaceholder") to the locale JSON and use the i18n function
t(...) in the Input's placeholder prop in the FeatureFlagsEditor component (the
Input where value={draft.maxContextTokens ?? ''} and onChange updates
draft.maxContextTokens). Ensure the key matches your translations naming
convention and update any tests or snapshots if needed.
- Around line 505-547: The table rows currently use the array index as the React
key in rules.map which can break reconciliation; generate a stable client-side
ID for each rule when rules are created/loaded (e.g., assign rule._clientId =
crypto.randomUUID() in the functions that initialize or add rules) and use
rule._clientId as the key in rules.map; ensure functions referenced here
(rules.map rendering, the rule-creation handler where new rules are pushed, and
any loader that populates initial rules) preserve that _clientId and strip it
only when sending payloads to the backend (or omit it in save logic), keeping
openEditDialog and removeRule working with index or find-by-_clientId as needed.

In `@services/platform/app/routes/dashboard/`$id/settings/governance.tsx:
- Around line 61-69: Replace the hardcoded label 'Retention' with the
translation hook (e.g. t('governance.tabs.retention')) so the tab uses localized
text; update the object with value: 'retention' (the same one that renders
<RetentionEditor organizationId={organizationId} />) to set label:
t('governance.tabs.retention') and ensure the translation key exists in the
locale files.

In `@services/platform/convex/governance/__tests__/feature_enforcement.test.ts`:
- Around line 17-25: The test fixture createMockCtx currently uses the unsafe
assertion "as never"; replace this with TypeScript's "satisfies" to preserve
type safety by ensuring the mock shape matches the real context type. Update
createMockCtx to return the object with "satisfies <ContextType>" where
<ContextType> is the actual context/interface used by the code under test (or
define an expectedCtxShape type reflecting db.query and auth.getUserIdentity),
removing "as never" so the compiler validates the mock structure while keeping
proper inferred types.

In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts`:
- Around line 269-285: The file-upload feature-flag check
(featureFlags.fileUpload) runs after buildMessageWithAttachments() and
saveMessage(), causing forbidden attachments to be persisted; move the guard
that inspects attachments and featureFlags.fileUpload to run before calling
buildMessageWithAttachments() and before any saveMessage() call so rejected
uploads are never written. Specifically, relocate the block that references
featureFlags.fileUpload, attachments, saveMessage, threadMeta, threadId,
messageAlreadyExists, and streamId to an earlier point in start_agent_chat where
attachments are first available (before buildMessageWithAttachments and before
the initial saveMessage), and keep the same return behavior and threadMeta
generationStatus update when rejecting.
- Around line 250-289: Call resolveFeatureFlags with the user's role (pass the
`role` variable) so role-scoped rules are evaluated, and then enforce the
returned codeExecution flag into `enforcedConfig` (alongside the existing
webSearch handling): when `featureFlags.codeExecution` is false, set the
appropriate code-execution toggle in `enforcedConfig` (e.g., `codeExecutionMode:
'off'` or the boolean field your agent uses) and remove any code-execution tools
from `enforcedConfig.convexToolNames` (similar to how 'web' is filtered out);
update the block around `resolveFeatureFlags`, `enforcedConfig`, and
`agentConfig` to apply these changes so governance actually disables code
execution for this chat path.

In `@services/platform/messages/de.json`:
- Around line 3588-3612: The JSON adds governance.tabs and
governance.featureFlags blocks that are being overwritten by later duplicate
keys; merge the new keys into the existing governance.tabs and
governance.featureFlags objects instead of adding duplicate top-level objects so
translations aren't lost — update the German locale to merge keys like
tabs.featureControls into governance.tabs and featureFlags.fileUpload,
featureFlags.enabled, featureFlags.scope, featureFlags.target,
featureFlags.noRules, featureFlags.maxContextTokensHint, featureFlags.saved, and
featureFlags.saveFailed into governance.featureFlags (ensure you preserve any
existing keys in those objects and only add/replace the missing/new entries).

In `@services/platform/messages/en.json`:
- Around line 3641-3646: The JSON contains two governance.tabs objects causing
the later one to override the first and drop governance.tabs.featureControls;
fix by merging the unique key "pii" from the second governance.tabs into the
first governance.tabs object (which already includes systemPrompt, budgets,
featureControls, usage) and then remove the duplicate governance.tabs entry so
only the merged governance.tabs remains.
🪄 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: fa26d370-d083-452a-bcd9-04523607a725

📥 Commits

Reviewing files that changed from the base of the PR and between 3c4fe57 and 917e25f.

📒 Files selected for processing (14)
  • services/platform/app/features/settings/governance/components/feature-flags-editor.stories.tsx
  • services/platform/app/features/settings/governance/components/feature-flags-editor.test.tsx
  • services/platform/app/features/settings/governance/components/feature-flags-editor.tsx
  • services/platform/app/routes/dashboard/$id/settings/governance.tsx
  • services/platform/convex/governance/__tests__/feature_enforcement.test.ts
  • services/platform/convex/governance/mutations.ts
  • services/platform/convex/lib/agent_chat/__tests__/start_agent_chat.test.ts
  • services/platform/convex/lib/agent_chat/internal_actions.ts
  • services/platform/convex/lib/agent_chat/start_agent_chat.ts
  • services/platform/convex/lib/agent_chat/types.ts
  • services/platform/convex/lib/agent_response/generate_response.ts
  • services/platform/convex/lib/agent_response/types.ts
  • services/platform/messages/de.json
  • services/platform/messages/en.json

Comment on lines +63 to +66
mockedUseGovernancePolicy.mockReturnValue({
data: null,
isLoading: false,
} as never);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Prefer typed mock fixtures over as never.

These casts erase the useGovernancePolicy contract, so the tests will still compile if the hook return shape drifts. A small typed factory or satisfies keeps the mocks honest without losing inference. Based on learnings, "In TypeScript test fixtures ... prefer the 'satisfies' operator over 'as' type assertions for config objects and test data."

Also applies to: 78-93, 116-119, 135-150

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.test.tsx`
around lines 63 - 66, Replace the unsafe "as never" casts on
mockedUseGovernancePolicy fixtures with a typed mock that preserves the hook's
return shape: create a small typed factory or use TypeScript's "satisfies" on
the mock objects so they conform to the actual useGovernancePolicy return type
(e.g., the shape used by mockedUseGovernancePolicy in
feature-flags-editor.test.tsx) and update all instances (lines matching the
other fixtures at 78-93, 116-119, 135-150) to use that factory or "satisfies"
expression so the tests will error if the hook contract drifts.

Comment on lines +255 to +275
<div>
<Input
label={t('featureFlags.maxContextTokens')}
type="number"
value={draft.maxContextTokens ?? ''}
onChange={(e) =>
updateDraft({
maxContextTokens: e.target.value
? Number(e.target.value)
: undefined,
})
}
disabled={cannotManage}
size="sm"
placeholder="e.g. 50000"
min={0}
/>
<Text className="text-muted-foreground mt-1 text-xs">
{t('featureFlags.maxContextTokensHint')}
</Text>
</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded placeholder text should use translation key.

Line 269 has a hardcoded placeholder "e.g. 50000" which should be internationalized per coding guidelines.

🌐 Proposed fix

Add a translation key in en.json:

"maxContextTokensPlaceholder": "e.g. 50000"

Then use it in the component:

               placeholder="e.g. 50000"
+              placeholder={t('featureFlags.maxContextTokensPlaceholder')}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`
around lines 255 - 275, Replace the hardcoded placeholder string in the Input
component for maxContextTokens with a translated string: add a new translation
key (e.g., "featureFlags.maxContextTokensPlaceholder" or
"maxContextTokensPlaceholder") to the locale JSON and use the i18n function
t(...) in the Input's placeholder prop in the FeatureFlagsEditor component (the
Input where value={draft.maxContextTokens ?? ''} and onChange updates
draft.maxContextTokens). Ensure the key matches your translations naming
convention and update any tests or snapshots if needed.

Comment on lines +505 to +547
<tbody>
{rules.map((rule, index) => (
<tr key={index} className="border-border border-b">
<td className="px-3 py-2 capitalize">{rule.scope}</td>
<td className="px-3 py-2">{resolveTarget(rule)}</td>
<td className="px-3 py-2 text-center">
{rule.webSearch === false ? '\u2718' : '\u2714'}
</td>
<td className="px-3 py-2 text-center">
{rule.codeExecution === false ? '\u2718' : '\u2714'}
</td>
<td className="px-3 py-2 text-center">
{rule.fileUpload === false ? '\u2718' : '\u2714'}
</td>
<td className="px-3 py-2 text-right">
{rule.maxContextTokens != null
? formatNumber(rule.maxContextTokens)
: '\u2014'}
</td>
<td className="px-3 py-2 text-right">
<HStack gap={1} justify="end">
<Button
variant="ghost"
size="icon"
onClick={() => openEditDialog(index)}
disabled={cannotManage}
aria-label={`${t('featureFlags.editRule')} ${index + 1}`}
>
<Pencil className="size-4" />
</Button>
<Button
variant="ghost"
size="icon"
onClick={() => removeRule(index)}
disabled={cannotManage}
aria-label={`${t('featureFlags.deleteRule')} ${index + 1}`}
>
<Trash2 className="size-4" />
</Button>
</HStack>
</td>
</tr>
))}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider generating unique IDs for rules to improve React reconciliation.

Using array index as the React key (Line 507) works for the current implementation but can cause subtle bugs if rules are ever reordered or if list operations become more complex. Since rules lack a natural unique identifier, consider generating a client-side ID (e.g., using crypto.randomUUID()) when creating rules.

♻️ Example approach
 function emptyRule(): FeatureFlagRule {
   return {
+    _clientId: crypto.randomUUID(),
     scope: 'default',
     webSearch: true,
     codeExecution: true,
     fileUpload: true,
   };
 }

Then use rule._clientId as the key. The _clientId would be stripped before saving to the backend.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`
around lines 505 - 547, The table rows currently use the array index as the
React key in rules.map which can break reconciliation; generate a stable
client-side ID for each rule when rules are created/loaded (e.g., assign
rule._clientId = crypto.randomUUID() in the functions that initialize or add
rules) and use rule._clientId as the key in rules.map; ensure functions
referenced here (rules.map rendering, the rule-creation handler where new rules
are pushed, and any loader that populates initial rules) preserve that _clientId
and strip it only when sending payloads to the backend (or omit it in save
logic), keeping openEditDialog and removeRule working with index or
find-by-_clientId as needed.

Comment on lines 61 to +69
{
value: 'retention',
label: 'Retention',
content: <RetentionEditor organizationId={organizationId} />,
},
{
value: 'feature-controls',
label: t('tabs.featureControls'),
content: <FeatureFlagsEditor organizationId={organizationId} />,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Localize the retention tab label.

'Retention' is still hardcoded while the surrounding tabs were moved to governance.tabs.*, so this tab will remain English in localized UIs. As per coding guidelines, "Do NOT hardcode text, use the translation hooks/functions instead for user-facing UI".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/routes/dashboard/`$id/settings/governance.tsx around
lines 61 - 69, Replace the hardcoded label 'Retention' with the translation hook
(e.g. t('governance.tabs.retention')) so the tab uses localized text; update the
object with value: 'retention' (the same one that renders <RetentionEditor
organizationId={organizationId} />) to set label: t('governance.tabs.retention')
and ensure the translation key exists in the locale files.

Comment on lines +17 to +25
function createMockCtx() {
return {
db: {
query: vi.fn(),
},
auth: {
getUserIdentity: vi.fn(),
},
} as never;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether unsafe cast escape hatches remain in this test.
# Expected after fix: no output.
rg -nP --type=ts '\bas\s+never\b' services/platform/convex/governance/__tests__/feature_enforcement.test.ts

Repository: tale-project/tale

Length of output: 77


Replace as never with satisfies in test fixture (Line 25).

Use satisfies instead of as never to maintain type safety in mock objects. For example:

Suggested change
function createMockCtx() {
  return {
    db: {
      query: vi.fn(),
    },
    auth: {
      getUserIdentity: vi.fn(),
    },
  } satisfies typeof expectedCtxShape; // Define expectedCtxShape based on actual context interface

Type assertions like as bypass compile-time validation; satisfies verifies the object structure matches the type while preserving proper typing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/governance/__tests__/feature_enforcement.test.ts`
around lines 17 - 25, The test fixture createMockCtx currently uses the unsafe
assertion "as never"; replace this with TypeScript's "satisfies" to preserve
type safety by ensuring the mock shape matches the real context type. Update
createMockCtx to return the object with "satisfies <ContextType>" where
<ContextType> is the actual context/interface used by the code under test (or
define an expectedCtxShape type reflecting db.query and auth.getUserIdentity),
removing "as never" so the compiler validates the mock structure while keeping
proper inferred types.

Comment on lines +250 to +289
if (userId) {
const userTeamIds = await getUserTeamIds(ctx, userId);
const featureFlags = await resolveFeatureFlags(
ctx,
organizationId,
userId,
userTeamIds,
);

if (!featureFlags.webSearch) {
enforcedConfig = {
...agentConfig,
webSearchMode: 'off',
convexToolNames: (agentConfig.convexToolNames ?? []).filter(
(t) => t !== 'web',
),
};
}

if (!featureFlags.fileUpload && attachments && attachments.length > 0) {
await saveMessage(ctx, components.agent, {
threadId,
message: {
role: 'assistant',
content:
'File uploads are disabled for your account by organization policy. Please contact your administrator.',
},
});
if (threadMeta) {
await ctx.db.patch(threadMeta._id, {
generationStatus: 'idle' as const,
updatedAt: Date.now(),
});
}
return { messageAlreadyExists, streamId };
}

if (featureFlags.maxContextTokens != null) {
governanceMaxContextTokens = featureFlags.maxContextTokens;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Role-based and code-execution rules are still no-ops here.

resolveFeatureFlags() supports a role parameter, but this call omits it, so role-scoped rules can never match. The returned codeExecution flag is also never applied to enforcedConfig, leaving that governance control ineffective on this chat path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts` around lines 250
- 289, Call resolveFeatureFlags with the user's role (pass the `role` variable)
so role-scoped rules are evaluated, and then enforce the returned codeExecution
flag into `enforcedConfig` (alongside the existing webSearch handling): when
`featureFlags.codeExecution` is false, set the appropriate code-execution toggle
in `enforcedConfig` (e.g., `codeExecutionMode: 'off'` or the boolean field your
agent uses) and remove any code-execution tools from
`enforcedConfig.convexToolNames` (similar to how 'web' is filtered out); update
the block around `resolveFeatureFlags`, `enforcedConfig`, and `agentConfig` to
apply these changes so governance actually disables code execution for this chat
path.

Comment on lines +269 to +285
if (!featureFlags.fileUpload && attachments && attachments.length > 0) {
await saveMessage(ctx, components.agent, {
threadId,
message: {
role: 'assistant',
content:
'File uploads are disabled for your account by organization policy. Please contact your administrator.',
},
});
if (threadMeta) {
await ctx.db.patch(threadMeta._id, {
generationStatus: 'idle' as const,
updatedAt: Date.now(),
});
}
return { messageAlreadyExists, streamId };
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reject blocked uploads before persisting the user message.

This guard runs after buildMessageWithAttachments() and saveMessage() at Lines 179-198, so a forbidden upload is still written into thread history with attachment URLs/file IDs before the assistant warning is sent. Move the fileUpload check ahead of attachment markdown generation and message persistence.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts` around lines 269
- 285, The file-upload feature-flag check (featureFlags.fileUpload) runs after
buildMessageWithAttachments() and saveMessage(), causing forbidden attachments
to be persisted; move the guard that inspects attachments and
featureFlags.fileUpload to run before calling buildMessageWithAttachments() and
before any saveMessage() call so rejected uploads are never written.
Specifically, relocate the block that references featureFlags.fileUpload,
attachments, saveMessage, threadMeta, threadId, messageAlreadyExists, and
streamId to an earlier point in start_agent_chat where attachments are first
available (before buildMessageWithAttachments and before the initial
saveMessage), and keep the same return behavior and threadMeta generationStatus
update when rejecting.

Comment thread services/platform/messages/de.json Outdated
Comment on lines +3588 to +3612
"tabs": {
"systemPrompt": "System-Prompt",
"budgets": "Budgets",
"featureControls": "Feature-Steuerung",
"usage": "Nutzung"
},
"featureFlags": {
"title": "Feature-Steuerung",
"description": "Funktionen pro Benutzer, Team oder Rolle aktivieren oder deaktivieren.",
"webSearch": "Websuche",
"codeExecution": "Code-Ausführung",
"fileUpload": "Datei-Upload",
"maxContextTokens": "Max. Kontext-Tokens",
"maxContextTokensHint": "Maximale Kontext-Tokens für KI-Antworten. Leer lassen für unbegrenzt.",
"enabled": "Feature-Steuerung aktivieren",
"scope": "Geltungsbereich",
"target": "Ziel",
"allUsers": "Alle Benutzer",
"addRule": "Regel hinzufügen",
"editRule": "Regel bearbeiten",
"deleteRule": "Regel löschen",
"noRules": "Keine Feature-Regeln konfiguriert. Füge eine Regel hinzu, um Funktionen pro Benutzer, Team oder Rolle zu steuern.",
"saved": "Feature-Steuerung gespeichert",
"saveFailed": "Feature-Steuerung konnte nicht gespeichert werden"
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Merge these into the existing governance.tabs and governance.featureFlags objects.

JSON keeps the last duplicate key, so the later tabs block at Line 3717 and featureFlags block at Line 3667 overwrite this entire addition. In de, that drops tabs.featureControls plus most of the new feature-flag strings (fileUpload, enabled, scope, target, noRules, etc.), so the new governance UI will render missing translations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/messages/de.json` around lines 3588 - 3612, The JSON adds
governance.tabs and governance.featureFlags blocks that are being overwritten by
later duplicate keys; merge the new keys into the existing governance.tabs and
governance.featureFlags objects instead of adding duplicate top-level objects so
translations aren't lost — update the German locale to merge keys like
tabs.featureControls into governance.tabs and featureFlags.fileUpload,
featureFlags.enabled, featureFlags.scope, featureFlags.target,
featureFlags.noRules, featureFlags.maxContextTokensHint, featureFlags.saved, and
featureFlags.saveFailed into governance.featureFlags (ensure you preserve any
existing keys in those objects and only add/replace the missing/new entries).

Comment thread services/platform/messages/en.json Outdated
Comment on lines +3641 to +3646
"tabs": {
"systemPrompt": "System prompt",
"budgets": "Budgets",
"featureControls": "Feature controls",
"usage": "Usage"
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Duplicate governance.tabs object will cause missing translation key.

There are two governance.tabs objects in this JSON file:

  • Lines 3641-3646: Contains systemPrompt, budgets, featureControls, usage
  • Lines 3783-3788: Contains systemPrompt, budgets, usage, pii

JSON parsers will use the last occurrence, so governance.tabs.featureControls will be undefined at runtime, breaking the new Feature Controls tab UI.

🐛 Proposed fix: merge the two objects

Remove the duplicate at lines 3783-3788 and merge its unique key (pii) into the first definition:

     "tabs": {
       "systemPrompt": "System prompt",
       "budgets": "Budgets",
       "featureControls": "Feature controls",
-      "usage": "Usage"
+      "usage": "Usage",
+      "pii": "PII protection"
     },

Then delete lines 3783-3788 entirely.

Also applies to: 3783-3788

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/messages/en.json` around lines 3641 - 3646, The JSON
contains two governance.tabs objects causing the later one to override the first
and drop governance.tabs.featureControls; fix by merging the unique key "pii"
from the second governance.tabs into the first governance.tabs object (which
already includes systemPrompt, budgets, featureControls, usage) and then remove
the duplicate governance.tabs entry so only the merged governance.tabs remains.

larryro added 2 commits April 11, 2026 15:02
Remove duplicate governance.tabs and governance.featureFlags entries,
merging feature controls tab label into the canonical tabs section.
…load when disabled

Add getMyFeatureFlags query that resolves the current user's feature
flags server-side (reactive via Convex subscriptions). Use it in
ChatInterface to hide the file upload button, drop zone, and paste
handler when fileUpload is disabled by governance policy. Also disable
the Confirm button in the rule edit dialog until the user makes a change.
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.

Feature enable/disable is not supported per user

2 participants