docs(data-drains): describe the resolved feature, not the bare flag - #7475
Conversation
Two things were wrong in the self-hosted flag paragraph. The first sentence attributed the page and the endpoints to `DATA_DRAINS_ENABLED` alone, which the next sentence then contradicted. `enterpriseFeatureEnabled` resolves `explicit ?? (ENTERPRISE_ENABLED || legacy default)`, and the legacy default for data drains is `false` — so the feature is on with the variable unset whenever `ENTERPRISE_ENABLED` is set, and an explicit `false` always wins. The paragraph now leads with that resolution. The rationale given for the public twin was also wrong. Inside a workspace `WorkspaceHostProvider` seeds the deployment shape from the server-resolved host context, `useDeploymentShape()` returns that seeded value, and the settings sidebar gates `data-drains` on `features.dataDrains` from it — so the page follows the server regardless of the twin, and "set it so the browser matches the server" misdescribed what the browser reads. The twin still backs `browserFallbackShape()` where nothing has seeded the shape, and it is what `sim-setup doctor` compares, so the advice to set it stands with the real reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
`isEnterpriseEnabled` is `isTruthy(env.ENTERPRISE_ENABLED)`, so `ENTERPRISE_ENABLED=false` is set and still resolves off. "Is set" would have implied an unset `DATA_DRAINS_ENABLED` turns drains on there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Greptile SummaryCorrects the self-hosted Data Drains documentation to describe effective feature resolution rather than attributing behavior solely to the feature-specific flag.
Confidence Score: 5/5The documentation-only change appears safe to merge. The corrected statements align with the implementation’s entitlement resolution, workspace deployment-shape seeding, client fallback, access gating, dispatcher behavior, and setup-doctor comparison.
|
| Filename | Overview |
|---|---|
| apps/docs/content/docs/platform/enterprise/data-drains.mdx | The revised guidance accurately reflects feature-flag precedence, server-seeded workspace behavior, browser fallback behavior, and setup diagnostics. |
Reviews (2): Last reviewed commit: "docs(data-drains): say the master switch..." | Re-trigger Greptile
Summary
Fixes the Data drains self-hosted flag paragraph, raised on the v0.8.22 release PR (#7473). The finding holds up on both points.
The bare flag is not what gates the feature.
enterpriseFeatureEnabledresolvesexplicit ?? (ENTERPRISE_ENABLED || legacyDefault), andENTERPRISE_FEATURE_LEGACY_DEFAULTS.dataDrainsisfalse— so withDATA_DRAINS_ENABLEDunset andENTERPRISE_ENABLEDset, data drains are on. The first sentence credited the bare variable and the second sentence then contradicted it. The paragraph now leads with the resolution, including that an explicitfalsealways wins.The reason given for the public twin was wrong. Inside a workspace,
WorkspaceHostProviderseeds the deployment shape from the server-resolved host context,useDeploymentShape()returns that seeded value, and the settings sidebar gates'data-drains': features.dataDrainsfrom it (components/settings/navigation.ts;settings/navigation.tssays so in a comment: "the sidebar applies deployment and entitlement visibility from the host context"). So the page follows the server whether or not the twin is set, and "set it so the browser's configuration matches the server's" misdescribed what the browser reads.The advice to set it stands, with the accurate reason: it backs
browserFallbackShape()wherever nothing has seeded the shape, and it is whatsim-setup doctorcompares (packages/sim-setup/src/twins.ts).One qualification to the original finding: it proposed treating the twin as "doctor consistency only", which understates it — the twin is still the client fallback outside a seeded workspace, so the page says both roles rather than dropping one.
Type of Change
Testing
bun run lint,bun run check:audits(45), andbun run docs-manifest:checkall pass. Every claim was traced to source before rewriting:enterprise-entitlements.ts,env-flags.ts,deployment-shape.ts,workspace-host-provider.tsx,components/settings/navigation.ts, andsim-setup/src/twins.ts.Checklist
🤖 Generated with Claude Code