Ship production AI crypto workspace - #2
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (72)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds bounded project workspaces, sandbox execution, AI patching, Stripe billing, team collaboration, Drops Bot webhook lifecycle APIs, runtime security controls, updated Studio UI flows, and expanded documentation and test coverage. ChangesStudio platform expansion
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/api/dropsbot/events/route.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: expand is not a function app/api/dropsbot/webhooks/route.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: expand is not a function db/dropsbot-webhooks.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: expand is not a function
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b4bac75f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| idempotencyKey: `drops-checkout-${createHash("sha256") | ||
| .update(`${input.accountIdentity}:${options.config.priceId}`, "utf8") | ||
| .digest("hex")}`, |
There was a problem hiding this comment.
When a member cancels or abandons Checkout and retries for the same configured Price, this deterministic account-and-Price key makes Stripe replay the original Checkout Session rather than create a usable new one for the key's retention window. An expired or otherwise unusable first session can therefore prevent the member from subscribing; retain idempotency for one request/retry sequence, but include a server-issued checkout-attempt identifier so later user attempts can create fresh sessions.
Useful? React with 👍 / 👎.
| // Keep a zero differing-pixel budget while ignoring one-channel Chromium | ||
| // antialias jitter on the rounded active rail border. | ||
| threshold: 0.01, |
There was a problem hiding this comment.
With maxDiffPixels: 0, setting threshold: 0.01 still makes every pixel whose color distance is at or below 0.01 count as unchanged, so small regressions across an arbitrary number of pixels can now pass this Studio visual test. Keep the threshold at zero and address the rendering instability instead of weakening the immutable baseline comparison.
AGENTS.md reference: AGENTS.md:L34-L37
Useful? React with 👍 / 👎.
| await expect(page).toHaveScreenshot(`${story}.png`, { | ||
| fullPage: false, | ||
| maxDiffPixels: 50, |
There was a problem hiding this comment.
Each Storybook screenshot call now overrides the configuration's maxDiffPixels: 0 with an allowance of 50, permitting real visual changes in every story and viewport to pass CI. Remove this override and preserve the exact approved-baseline comparison rather than masking differences.
AGENTS.md reference: AGENTS.md:L34-L37
Useful? React with 👍 / 👎.
| ? { | ||
| platformDailyBuilds: 100, | ||
| privateProjects: 500, | ||
| teamWorkspaces: 10, |
There was a problem hiding this comment.
A verified Pro account is told it can store 500 private projects, and the new account panel displays this value, but db/member-projects.ts still hard-codes MEMBER_PROJECT_LIMIT = 50 for parsing and every upsert while /api/projects also returns that fixed limit. Consequently a paying Pro user is rejected on project 51 despite the entitlement response promising 500; derive the storage/API limit from the verified billing tier or advertise the actually enforced limit.
Useful? React with 👍 / 👎.
| const body = await teamRequestBody(request, 8 * 1_024); | ||
| await enforceTeamRateLimit(account.identity, "team-workspace-invite-accept"); | ||
| const result = await acceptTeamWorkspaceInvite({ |
There was a problem hiding this comment.
If an owner creates an invite while Pro and the subscription later expires before that capability does, this route still adds the recipient to the workspace because invite acceptance is the only team mutation that never calls proTeamEntitlements. The newly accepted member can then read the owner's private shared projects even though the owner now has zero team/collaborator entitlement; verify the encoded owner identity's current Pro status before mutating membership.
Useful? React with 👍 / 👎.
| if (result.status === "exists") { | ||
| throw new DropsBotWebhookResponseError(409, { | ||
| code: "DROPSBOT_CALLBACK_EXISTS", | ||
| error: "This project already has a Drops Bot callback. Its secret URL is never re-disclosed.", | ||
| }); |
There was a problem hiding this comment.
If the user refreshes before copying the one-time URL, loses it before registering it, or suspects it leaked, every subsequent creation attempt permanently returns this conflict. A repo-wide check of app/api/dropsbot and db/dropsbot-webhooks.ts finds no DELETE, revoke, or rotation path, so that project can never obtain a usable callback again; add an owner-authorized, consented rotation/revocation operation that invalidates the old capability.
Useful? React with 👍 / 👎.
| if (workspace.invites.length >= MAX_INVITES_PER_WORKSPACE) { | ||
| throw new TeamWorkspaceValidationError("Team workspace invite limit reached."); |
There was a problem hiding this comment.
Accepted and expired invites remain in workspace.invites forever, and there is no invite deletion or revocation path, yet this check counts all of them toward the 100-invite ceiling. A long-lived team that periodically invites collaborators will eventually be permanently unable to issue another capability even when it has no pending invites; prune terminal entries or count only currently pending, unexpired invitations.
Useful? React with 👍 / 👎.
| } | ||
| const origin = safeOrigin(input.origin); | ||
| const returnPath = safeReturnPath(options.config.portalReturnPath); | ||
| let account = await options.repository.readAccount(input.accountIdentity); |
There was a problem hiding this comment.
When DROPS_ACCOUNT_COOKIE_SECRET is rotated, resolveStudioAccount derives a different account.identity for the same OpenRouter subject, so this lookup can no longer find the existing Stripe customer/subscription row keyed by the old HMAC. The paid user is then shown as Member and cannot open their existing customer portal, while Stripe webhooks continue updating the orphaned old identity; persist billing ownership under a stable provider-subject identifier or provide an explicit identity migration independent of the cookie-signing key.
Useful? React with 👍 / 👎.
| if (message.slug !== projectSlug) return; | ||
| if (message.type === "drops-studio-data-request") { void handleData(); return; } | ||
| if (message.type === "drops-studio-product-hunt-request") { void handleHunt(message); return; } |
There was a problem hiding this comment.
For a project with sourceEditedAt, publishing supplies htmlOverride, whose embedded projectSpec.slug remains the local project slug, while the public shell is initialized with the newly generated collision-resistant publish slug. This equality check then discards every data, Product Hunt, and external-action message from that edited runtime, leaving refreshes unanswered and handoffs unusable after publication; rewrite or recompile the embedded slug before storing the override, or bind the shell to a separate verified runtime identifier.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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 `@app/api/telegram/account/send-code/route.ts`:
- Around line 31-32: Update the catch block in the Telegram account send-code
route to log the caught error server-side with appropriate context, then return
only a fixed generic failure message through telegramAccountJson instead of
exposing error.message. Preserve the existing 422 response status.
In `@components/project-studio.tsx`:
- Around line 973-1004: Update the workspace run digest evidence flow around
createWorkspaceRunDigest and the workspaceRunDigestEvidence comparison so
verification is keyed to stable workspace revision or digest inputs rather than
project object identity. Preserve verified receipt evidence across unrelated
project object updates, while still invalidating it when the relevant workspace
files, task, or receipt inputs change; avoid redundant digest recomputation.
In `@components/studio-account-team-panel.tsx`:
- Around line 1076-1080: Update the optimistic revision badge condition near the
optimisticRevision display so it requires an actual optimisticRevision and
selectedWorkspace before comparing their workspace IDs. Preserve the existing
badge rendering for matching, non-null values while preventing the branch from
being entered when either value is absent.
In `@DESIGN.md`:
- Around line 102-115: Update the CSS policy in DESIGN.md to explicitly cover
project-studio.chrome.css and project-studio.workspace.css, including their
intended legacy bounds and the existing 48 KiB cap. Alternatively, adjust the
release guardrail to enforce the same policy across these imported stylesheets,
ensuring the documented rules match the shipped imports.
In `@docs/ACCESS_TIERS.md`:
- Around line 114-122: Update the “External provider setup” section to replace
“the signed account secret” with the actual environment variable name used by
the implementation and documented in docs/INTEGRATIONS.md, keeping the
surrounding prerequisite list unchanged.
In `@e2e/proofs/all-presets-completion.spec.ts`:
- Around line 608-613: Update the assertion after publishFromStudio in the
storedProject flow to use expect.poll, repeatedly fetching storedProject(page,
project.id) until quality.readyToPublish, criticalFailures, and
runtimeSmoke.mode match the expected values. Preserve the existing quality
expectations while allowing later persistence writes to complete.
In `@lib/member-project-cloud.ts`:
- Around line 268-276: Separate the cheap structural validation in
validatedWorkspace/sanitizeMemberProjectDraft from compileWorkspaceRuntime and
assertPublishedArtifactSafe, running compilation and artifact-safety checks only
on ingress or via memoization keyed by projectId and revision; update
db/team-workspaces.ts lines 625-660 to use the cheaper read validation during
membership fan-out and mutateEnvelope retries, while preserving full checks for
newly submitted data.
In `@lib/project-workspace.ts`:
- Around line 849-852: Update the reconciliation logic around rootTasks and
validateTasks so root tasks beyond PROJECT_WORKSPACE_ROOT_TASK_LIMIT are
explicitly rejected with a deterministic validation error instead of being
truncated by slice. Preserve the existing limit for valid input and keep usedIds
generation unchanged after validation.
In `@lib/public-project-shell.ts`:
- Around line 71-82: The inline approvedExternalUrl allowlist in the
public-project shell duplicates approvedPreviewExternalUrl from the shared
runtime-external-link module and has diverged on protocol handling. Replace the
inline rules with a generated or serialized predicate sourced from
approvedPreviewExternalUrl, preserving the shared module as the single source of
truth for published apps.
In `@storybook-e2e/visual.spec.ts`:
- Around line 49-66: Update the background-image URL collection in the visual
asset preload flow to skip any URL whose value starts with the data: scheme
before adding it to imageUrls. Preserve preloading for regular external or
relative URLs and keep the existing error handling unchanged.
In `@tests/dropsbot-webhook.test.mjs`:
- Around line 103-118: In the “Drops Bot receiver clears stale provider evidence
after account expiry” test, validate that both source markers used by indexOf
are present before calling source.slice. Add explicit assertions for the 401
branch marker and the subsequent response.ok marker, matching the
marker-presence pattern used later in the test file, while preserving the
existing evidence assertions.
In `@tests/project-export.test.mjs`:
- Around line 96-100: Fix the negative assertion in the project export test
around files["tests/smoke.mjs"] so it excludes the specific dropstab/fallback
provider claim rather than the impossible literal pipe-separated string. Keep
the existing positive assertion requiring data-provider-evidence="unverified"
and ensure the exclusion targets only the unwanted provider marker.
In `@tests/rendered-html.test.mjs`:
- Around line 65-71: The rendered HTML assertion for drops-studio:dropsbot and
account-connected is too narrow because .* does not span newlines; update the
assertion in the rendered HTML test to use a cross-line pattern such as [\s\S]*
for file-wide matching, or remove it if the adjacent marker assertion fully
covers the intended constraint.
- Around line 5-9: Update the test in rendered-html.test.mjs so the missing
.next/server/app/index.html precondition is handled explicitly: either invoke
the production build before running the test through the unit-test entrypoint,
or detect a missing artifact and skip with a clear message. Ensure npm run
test:unit succeeds on a clean checkout without an unhandled file-read failure.
In `@tests/studio-account-team-panel.test.mjs`:
- Around line 52-62: Make the source-extraction assertions fail loudly when the
createInvite/acceptInvite or replaceWorkspace anchors are missing: validate each
indexOf result before slicing and assert the expected ordering, rather than
allowing slice to produce a truncated or empty string. Apply the same
guarded-anchor approach to the analogous extraction around lines 112-115, while
preserving the existing receipt-order assertions.
In `@tests/ui-css-policy.test.mjs`:
- Around line 163-170: Update the fourColumnRules extraction in the UI CSS
policy test to inspect the complete .project-studio-layout rule body, including
declarations after nested blocks, instead of using a regex that stops at the
first closing brace. Implement balanced-brace scanning or an equivalent
nesting-aware parser, then continue applying countGridTracks to every
grid-template-columns declaration.
In `@tests/workspace-ai-patch.test.mjs`:
- Around line 691-705: Update the test around runnableWorkspace and the
build-task mutation to first assert that a task with id "build" exists, then
construct unsafe with the modified args. Keep the existing assert.throws
validation unchanged so the test cannot silently pass against an unchanged
workspace.
- Around line 786-790: Update the assertion using
workspaceAiPatchJsonSchema.required so it sorts a copied array rather than
mutating the exported schema’s shared required array. Preserve the expected
values and assertion behavior while leaving workspaceAiPatchJsonSchema.required
unchanged for provider callers and later tests.
- Around line 1-19: Update the project’s Node.js minimum requirement to 22.15.0
or newer so registerHooks is available at runtime; apply this requirement
consistently for tests/workspace-ai-patch.test.mjs (anchor, lines 1-19) and
tests/runtime-preview-security.test.mjs (sibling, lines 5-9), without changing
either test’s hook usage.
In `@tests/workspace-ai-route.test.mjs`:
- Around line 1-4: Resolve the Node runtime compatibility issue in
tests/workspace-ai-route.test.mjs by either raising the package.json Node engine
minimum to 22.15.0 or replacing the registerHooks import with an API supported
by the existing minimum version. Ensure the test remains functional on the
declared engine floor.
---
Nitpick comments:
In `@app/api/access/route.ts`:
- Around line 21-33: Replace the hardcoded "member-ai-plan" namespace and
24-hour window in the readRequestLimitState call with fundedQuota.namespace and
fundedQuota.windowMs. Keep the existing memberLimit, readiness gating, and
unavailable fallback unchanged so the status check follows
resolveFundedBuildQuota’s policy.
In `@app/api/billing/checkout/route.ts`:
- Around line 30-44: Extract the duplicated HEADERS constant and sameOrigin
function into a shared helper module, preserving their current behavior and
exports. In app/api/billing/checkout/route.ts lines 30-44 and
app/api/billing/portal/route.ts lines 24-38, remove the local definitions and
import the shared symbols instead.
In `@app/api/teams/`[workspaceId]/invites/route.ts:
- Around line 37-38: Run enforceTeamRateLimit before proTeamEntitlements in
every affected handler: app/api/teams/[workspaceId]/invites/route.ts lines 37-38
using "team-workspace-invite"; app/api/teams/[workspaceId]/members/route.ts
lines 33-34 using "team-workspace-member-role";
app/api/teams/[workspaceId]/projects/route.ts lines 38-39 using
"team-workspace-project-write"; and app/api/teams/[workspaceId]/route.ts lines
66-67 using "team-workspace-update". Keep each call’s existing arguments and
move only the entitlement lookup after the rate-limit check.
- Line 49: Validate body.role as "editor" or "viewer" before invoking the domain
operations. In app/api/teams/[workspaceId]/invites/route.ts (49-49), return HTTP
400 for invalid roles before createTeamWorkspaceInvite and pass the narrowed
value; apply the same guard in app/api/teams/[workspaceId]/members/route.ts
(41-41) before changeTeamMemberRole, removing the unsafe never cast.
In `@app/api/teams/route.ts`:
- Around line 36-54: Replace the local json, account, and sameOrigin
implementations in the teams route with the shared teamJson, teamAccount, and
requireTeamSameOrigin helpers from lib/team-api.ts, updating imports and call
sites as needed while preserving the existing route behavior.
In `@app/api/workspace/patch/route.ts`:
- Around line 404-433: In the catch handler around
WorkspaceAiPatchConflictError, remove the redundant
WorkspaceAiPatchValidationError check and consolidate the Error handling into
the intended single fallback response. Ensure the final fallback remains
reachable for non-Error throws and preserve the existing response payload,
status, and quota-cookie behavior.
In `@app/api/workspace/run/route.ts`:
- Around line 41-108: The helpers json, withQuotaCookies, sameOrigin,
RequestBodyResult, and requestBody are duplicated between the workspace run and
patch routes; extract them into a shared workspace route-boundary module and
update both routes to import and reuse the shared implementations. Preserve the
existing CSRF, bounded-body, quota-cookie, and no-store behavior, and extend the
shared json helper to accept optional extra headers so the run route’s inline
NextResponse.json calls can use it.
In `@app/styles/project-studio.runtime.css`:
- Around line 39-43: Merge the intended color value into the existing
`.publish-dialog header p` rule in the rewritten CSS block, replacing the
earlier color declaration with the override value, then remove the standalone
duplicate `.publish-dialog header p` rule.
In `@components/drops-studio.tsx`:
- Around line 1218-1230: In the Drops Bot opening flow, replace the about:blank
window creation and subsequent opener/location manipulation with the existing
window.open pattern used by handlePreviewAction: open the static
https://t.me/Drops URL in a new tab with noopener,noreferrer, retain the
blocked-popup check and existing toast/connection state behavior.
In `@components/project-studio.tsx`:
- Around line 981-990: Extract the shared run-digest input construction from the
digest effect and runWorkspaceTask into a single reusable builder, preserving
the existing files and task fields including the 15-second timeout default.
Update both call sites to use this builder so changes to the digest input shape
or timeout remain synchronized.
- Line 137: Remove the meaningless SourceFile type alias and update its usages
to use string directly, unless the implementation already supports introducing a
branded workspace path type. Preserve the existing behavior while eliminating
the misleading unconstrained alias.
- Around line 850-876: Update the drops-studio-data-request branch in the event
handler to return immediately after scheduling its fetch and response handling.
Preserve the existing success and catch behavior, and prevent execution from
falling through to the subsequent if chains.
In `@components/project-workspace-dialog.stories.tsx`:
- Around line 133-134: Update the story’s default draft initialization to locate
the workspace file matching activePath and use its content, rather than relying
on workspace.files[1]. Preserve the existing activePath value and ensure the
draft remains synchronized if fixture file ordering changes.
In `@components/project-workspace-dialog.tsx`:
- Around line 483-492: Update the preview link rendered in the
receipt.previewUrl branch of the project workspace dialog to include noopener
explicitly in its rel attribute, while preserving the existing noreferrer
behavior and link labels.
- Around line 177-188: Export the canonical REQUIRED_FILES set from
lib/project-workspace.ts and update the requiredPath calculation in the project
workspace dialog to reuse it instead of defining a duplicate Set. Preserve the
existing activePath membership check so Delete-button behavior remains aligned
with workspace validation.
In `@components/studio-account-team-panel.tsx`:
- Line 259: Replace message-text heuristics with explicit message state in the
team panel: track each team/share message as a `{ tone: "info" | "error", text:
string }` pair, update all setters and consumers to preserve the intended tone,
and render styling from `tone` instead of `teamMessageIsError` or
`shareMessage.includes("failed")`. Apply the same change to both referenced
message flows.
In `@components/telegram-channel-wizard.tsx`:
- Around line 301-323: The copy-command fallback in the button handler should
use a local inline hint near the button instead of the top-level error
state/banner; preserve the manual-copy command text for unavailable or failed
clipboard access and clear the hint on successful copy. In the “Create another”
handler, also reset profileCommandCopied along with the other wizard state.
In `@db/billing.ts`:
- Around line 229-254: The mutateBlob implementation serializes all billing
writes through one global blob and exhausts retries under concurrent webhook
delivery. Update the production billing storage path to prefer D1 instead of
this whole-state blob approach, or shard blob storage by account so independent
mutations do not share BLOB_PATH; additionally, record a metric when mutateBlob
reaches its precondition-retry limit before throwing
BillingStorageUnavailableError.
In `@db/team-workspaces.ts`:
- Around line 798-802: Update the invite lookup in the mutateEnvelope callback
to compare item.capabilityHash and capabilityHash using the existing
constant-time hashesMatch approach from dropsbot-webhooks.ts, while preserving
the current workspace, invite, and role validation behavior.
In `@e2e/contracts/member-project-cloud.spec.ts`:
- Around line 110-141: Hoist the private runtime field denylist to a shared
constant outside assertNoPrivateRuntimeFields so it is not rebuilt for each
object key. Update the validation in assertSafeProjectWrite to report the
offending normalized key and its object path when a prohibited field is found,
while preserving recursive traversal of arrays and nested objects.
In `@e2e/contracts/ui-standards.spec.ts`:
- Around line 217-222: Update the test “1920px Project Studio keeps Director in
the left context surface” to avoid depending on the hardcoded “chromium-1440”
project name; gate it with a dedicated marker/project or explicitly fail fast
when the required project is unavailable, while preserving the 1920px viewport
override.
In `@e2e/proofs/all-presets-completion.spec.ts`:
- Around line 461-473: Replace the process.env.CI condition in the download
assertion flow with a documented capability or browser-channel check that
identifies environments where Playwright native download events are supported.
Keep the native suggestedFilename() and failure() assertions enabled whenever
that capability is available, and retain the byte-level Blob validation fallback
for unsupported Chromium environments.
In `@e2e/proofs/director-flow.spec.ts`:
- Around line 235-237: In the preview flow, replace the direct Preview button
click with the existing showPreviewOnMobile() helper so its iframe visibility
assertion runs before checking runtimeReady. Keep the subsequent Browser
telemetry assertion unchanged.
In `@lib/dropsbot-webhook.ts`:
- Around line 197-266: Replace the duplicated body-size and UTF-8 handling in
readDropsBotWebhookBody and parseDropsBotWebhookPayload with the shared
readBoundedRequestBody and decodeUtf8Body helpers from
lib/http-request-boundary.ts. Remove the local Content-Length validation,
streaming reader, and TextDecoder logic while preserving the existing
DropsBotWebhookValidationError behavior and JSON parsing/redaction flow.
In `@lib/project-workspace.ts`:
- Around line 339-358: Update packageManifest so its engines.node declaration
requires Node 24, matching the validated node24 runtime contract and README;
leave the rest of the generated manifest unchanged.
- Around line 744-747: Replace the hardcoded upper bound 16 in the workspace
task validation condition with the existing PROJECT_WORKSPACE_TASK_LIMIT
constant used by reconcileProjectWorkspaceTasks, while preserving the current
validation and issue message behavior.
In `@lib/runtime-srcdoc-security.ts`:
- Line 91: Move the findHtmlOpeningTag import to the top import section of the
file and remove its current trailing declaration near the usage site; leave all
runtime logic unchanged.
In `@lib/telegram-account-request.ts`:
- Around line 59-132: Replace the local requireJsonContentType,
declaredBodyLength, and readBoundedBody implementations with the shared
hasJsonMediaType and readBoundedRequestBody helpers from
lib/http-request-boundary.ts. In readTelegramAccountJson, decode the bounded
bytes with decodeUtf8Body and map RequestBodyBoundaryError with reason
"too-large" to 413 and all other boundary errors to 400, preserving the existing
TelegramAccountRequestError behavior.
In `@lib/workspace-ai-provider.ts`:
- Around line 186-188: Update the gateway call near timeoutSignal() to use the
shared PROVIDER_TIMEOUT_MS budget instead of the hardcoded 18_000 value,
ensuring both direct-provider and platform fetch paths honor the named timeout
constant.
- Around line 244-278: The fallback loop around PLATFORM_WORKSPACE_MODELS
currently discards all attempt failures. Capture a redacted diagnostic record
for each failed model attempt, containing only the model and safe error class
information, while keeping prompts, tokens, and provider error text out of logs
or caller-visible responses; retain the existing generic
WorkspaceAiProviderResponseError message and fallback order.
In `@lib/workspace-sandbox.ts`:
- Around line 513-518: Update packageContract to use the existing
SandboxPackageContract interface as its return type instead of repeating the
inline object shape. Preserve the current fields and behavior unchanged.
- Around line 940-945: Extend the retained-preview flow controlled by
keepPreview to persist sandbox.name or the provider session ID with the preview
receipt. Use that identifier to stop the microVM when the workspace revision
changes or the tab closes, while preserving the existing 300-second timeout as a
fallback. Keep the preview URL and successful readiness behavior unchanged.
In `@scripts/check-ui-guardrails.mjs`:
- Around line 190-192: Update the success message in the guardrail-check flow to
interpolate the existing MAX_MANUAL_STYLESHEET_BYTES constant instead of
hardcoding “48 KiB,” converting it to the same KiB representation used by the
message so it stays accurate when the limit changes.
- Around line 117-131: Update collectUiArchitectureViolations to define and use
a named constant for the 4096-byte globals.css budget, and narrow the try/catch
to only the readFile(globalsPath) operation. Preserve the missing-file handling
for that read while removing the outer catch so later ENOENT errors are not
misreported as a missing stylesheet.
In `@tests/billing.test.mjs`:
- Around line 23-36: Remove .catch(() => null) from all six required imports in
tests/billing.test.mjs lines 23-36 and from the required imports in
tests/workspace-sandbox.test.mjs lines 23-31, allowing module load failures to
throw their original errors. Remove the now-redundant api() and
assert.ok(routeModule, …) guards in the workspace sandbox tests while preserving
the remaining test behavior.
In `@tests/member-project-cloud.test.mjs`:
- Around line 203-220: Update the rejection assertion in the mutation loop to
require the public MemberProjectValidationError type as well as the message
pattern, and destructure MemberProjectValidationError alongside
MEMBER_PROJECT_BODY_LIMIT_BYTES before using it in assert.throws. Keep the
existing mutation-specific message matching intact.
In `@tests/public-project-shell.test.mjs`:
- Around line 68-79: Replace the source-text regex assertions in the relevant
test with behavioral tests of the generated bridge, especially evaluating
approvedExternalUrl against allowed and hostile inputs. Follow the
runtime-external-link.test.mjs approach so implementation formatting, quoting,
or condition ordering changes do not break the tests.
- Around line 5-10: Remove the error-swallowing import fallbacks across
tests/public-project-shell.test.mjs:5-10,
tests/workspace-ai-entitlement.test.mjs:5-13, and
tests/workspace-ai-provider.test.mjs:4-11. Import each module directly so
compile and resolution failures preserve their original stack traces; in
public-project-shell.test.mjs remove the shellModule assertion guards, and in
the entitlement and provider suites remove the api() guard helpers.
- Around line 5-10: Remove the silent catch handlers from the top-level imports
of public-project-shell.ts and artifact-csp.ts so module load or compile errors
propagate with their original details. Preserve the existing shellModule
assertion and destructuring in the test while ensuring missing-module failures
remain distinguishable from other import failures.
In `@tests/published-quality-evidence.test.mjs`:
- Around line 76-114: Extend the “rejects stale or future-dated publish
evidence” test using acceptPublishedQuality to assert the exact freshness
boundaries are accepted: 11:55:00.000Z and 12:00:30.000Z, including the nested
runtimeSmoke checkedAt values through evidenceAt. Keep the existing just-outside
rejection cases unchanged.
In `@tests/runtime-preview-security.test.mjs`:
- Around line 53-70: The test assertion for CSP insertion in the “CSP insertion
respects quoted greater-than characters in head attributes” case is coupled to
an exact newline. Replace that regex with assertions that the preserved
data-marker attribute exists and that the CSP meta tag appears after the opening
head tag, while retaining the existing policy-before-runtime ordering checks.
- Around line 86-138: Replace the source-text assertions in “Studio keeps srcdoc
popups disabled and treats iframe smoke as browser-only telemetry” with
behavior-based checks. Exercise the relevant Studio/compiler paths and assert
observable runtime HTML, returned sandbox attributes, external-navigation
behavior, and publish payload handling through symbols such as runtimeSrcDoc,
openTab, publish, and acceptPublishedQuality. Keep coverage for popup blocking,
isolated fullscreen HTML, browser-only telemetry, and authoritative publish
quality without depending on formatting, names, or source proximity.
In `@tests/server-release-quality.test.mjs`:
- Line 129: Update the HTML assertion in the release-quality test to avoid
requiring lang=en to appear immediately after the opening html tag. Match
lang=en independently while retaining the separate exact-once assertion for
data-provider-evidence, so attribute ordering does not affect the test.
In `@tests/storybook-static-server.test.mjs`:
- Around line 5-13: Replace the source-text assertions in the static Storybook
server test with behavioral integration coverage: start the server on an
ephemeral port, request an existing file, and verify its response completes
correctly before shutdown. Also request a missing or unreadable file and assert
that the server returns the expected error status, without depending on
implementation-specific identifiers or source formatting.
In `@tests/studio-account-team-panel.test.mjs`:
- Line 100: Update the assertion in the studio account team panel test to avoid
requiring exactly two occurrences of the full ordered class string. Instead,
verify the sizing tokens are present at least once or validate the equivalent
44px minimum height for each relevant control, while preserving the test’s
intent.
In `@tests/team-workspaces.test.mjs`:
- Around line 1107-1109: Ensure the test setup that assigns
globalThis.__DROPS_STUDIO_LOCAL_RATE_LIMITS__ restores or clears that global
after the test completes, using t.after/finally or an appropriate reset helper
alongside resetLocalTeamWorkspaceStateForTests. Keep the rate-limit state scoped
to this test and prevent leakage into subsequent tests.
- Around line 543-551: Expand the test fixture around the owners setup so it
contains more distinct owners than the concurrency limit, such as six or more.
Preserve the per-owner single-fetch assertions and ensure the concurrent read
tracking exercises the existing maxActiveOwnerReads <= 4 bound, allowing
unbounded loading to fail.
- Around line 22-42: Update the module-loading setup around teamModule,
teamApiModule, teamStoreModule, and the route module imports to capture each
import failure in an importErrors map keyed by its specifier instead of
swallowing it with catch(() => null). Include
importErrors.get(specifier)?.message in the corresponding assert.ok failure
messages so initialization errors are surfaced while preserving the existence
checks.
In `@tests/telegram-account-request-boundary.test.mjs`:
- Around line 24-40: Update the route-loading setup around routes to remove the
redundant Promise.all wrapper while preserving the existing route imports; use a
direct array of awaited imports, or retain explicit static specifiers if
required by registerHooks or bundling. In request, remove the unnecessary
backslash escapes from the JSON template literal so it produces the same valid
body.
In `@tests/telegram-export-receipt.test.mjs`:
- Around line 80-84: Update the fetchImplementation stub to detect when calls
has reached or exceeded providerResults.length before reading the fixture, and
throw immediately for an unexpected provider call; preserve the existing
successful response and call-count progression for queued results.
- Around line 10-28: Replace the marker-based slicing and Function re-evaluation
in the test helper with a direct import of the exported Telegram template or a
buildTelegramHandlerSource() helper from lib/project-export.ts. Update the
production module to expose that stable symbol, then construct the handler
source through it without relying on publicDataFunction ordering or dynamic
eval.
In `@tests/ui-css-policy.test.mjs`:
- Around line 219-224: Update the assertions in the runtime CSS checks of
tests/ui-css-policy.test.mjs so height and width requirements are matched
independently within each rule, rather than enforcing declaration order.
Preserve the existing selectors and 44px values while applying the same
order-independent approach to all affected button rules.
In `@tests/workspace-ai-patch.test.mjs`:
- Around line 21-33: Remove the catch handler from the patchModule import in
tests/workspace-ai-patch.test.mjs so import failures propagate with their
original errors and stack traces. Update api() to use the direct imported module
without the null assertion or generic “workspace AI patch module must exist”
check, matching the behavior of the sibling imports.
- Around line 792-824: In the two parseWorkspaceAiPatch assertions, replace the
broad message regexes with checks targeting the specific validation failure for
baseRevision being below the minimum and language being unsupported. Prefer
exact issue details or assert the first Zod issue code and relevant path so
unrelated schema errors cannot satisfy either test.
- Around line 149-150: Update the assertions for result.workspace.tasks and
result.workspace.runtime in the workspace AI patch test to compare values rather
than reference identity, using deep-equality assertions while preserving the
expected unchanged contents.
In `@tests/workspace-ai-provider.test.mjs`:
- Around line 177-245: Extend the provider-key leakage checks in the Anthropic
test around generateWorkspaceAiPatch and the OpenAI GPT-5.6 Sol test to assert
that JSON.stringify(result) does not match /request-only-test-key/. Keep the
existing provider-specific request and result assertions unchanged, mirroring
the OpenRouter no-key-leak assertion for both direct providers.
- Line 94: Update the schema comparison assertion in the provider test to use
deep structural equality instead of reference equality, so equivalent schemas
pass even when separately instantiated. Keep the existing calls[0].schema and
calls[1].schema comparison unchanged otherwise.
In `@tests/workspace-ai-route.test.mjs`:
- Around line 22-36: Update the route module initialization in
workspace-ai-route.test.mjs to import app/api/workspace/patch/route.ts directly
without catching and replacing import failures with null. Remove the routeModule
assertion in api() that only supports the swallowed-error path, while preserving
the module access behavior for successful imports.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ef5481ac-51c3-447e-ad7b-750c63b34a36
⛔ Files ignored due to path filters (21)
docs/design/current-home-actual.pngis excluded by!**/*.pngdocs/design/current-studio-actual.pngis excluded by!**/*.pnge2e/visual/home.spec.ts-snapshots/home-builder-chromium-1024-linux.pngis excluded by!**/*.pnge2e/visual/home.spec.ts-snapshots/home-builder-chromium-1440-linux.pngis excluded by!**/*.pnge2e/visual/home.spec.ts-snapshots/studio-crypto-game-chromium-1024-linux.pngis excluded by!**/*.pnge2e/visual/home.spec.ts-snapshots/studio-crypto-game-chromium-1440-linux.pngis excluded by!**/*.pnge2e/visual/home.spec.ts-snapshots/studio-crypto-game-chromium-390-linux.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.jsonpublic/brand/drops-bot-avatar.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/action-engine-desktop-chromium-1440-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/alpha-channel-disconnected-chromium-1024-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/alpha-channel-disconnected-chromium-1440-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/alpha-channel-disconnected-chromium-390-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/crypto-game-desktop-chromium-1024-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/crypto-game-desktop-chromium-1440-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-data-error-chromium-1024-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-data-error-chromium-1440-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-data-error-chromium-390-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-populated-chromium-1024-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-populated-chromium-1440-linux.pngis excluded by!**/*.pngstorybook-e2e/visual.spec.ts-snapshots/morning-alpha-populated-chromium-390-linux.pngis excluded by!**/*.png
📒 Files selected for processing (130)
.env.example.github/workflows/ui-quality.ymlDESIGN.mdREADME.mdapp/api/access/route.tsapp/api/agent/plan/route.tsapp/api/billing/checkout/route.tsapp/api/billing/portal/route.tsapp/api/billing/status/route.tsapp/api/billing/webhook/route.tsapp/api/dropsbot/events/route.tsapp/api/dropsbot/webhooks/[connectionId]/[capability]/route.tsapp/api/dropsbot/webhooks/route.tsapp/api/teams/[workspaceId]/invites/route.tsapp/api/teams/[workspaceId]/members/route.tsapp/api/teams/[workspaceId]/projects/route.tsapp/api/teams/[workspaceId]/route.tsapp/api/teams/invites/accept/route.tsapp/api/teams/route.tsapp/api/telegram/account/create-channel/route.tsapp/api/telegram/account/send-code/route.tsapp/api/telegram/account/sign-in/route.tsapp/api/telegram/account/status/route.tsapp/api/workspace/patch/route.tsapp/api/workspace/run/route.tsapp/layout.tsxapp/p/[slug]/route.tsapp/styles/project-studio.chrome.cssapp/styles/project-studio.runtime.cssapp/styles/project-studio.wide.cssapp/styles/project-studio.workspace.csscomponents/drops-studio-dialogs.tsxcomponents/drops-studio.tsxcomponents/dropsbot-webhook-connection.tsxcomponents/preview-canvas.tsxcomponents/project-studio.tsxcomponents/project-workspace-dialog.stories.tsxcomponents/project-workspace-dialog.tsxcomponents/studio-account-team-panel.tsxcomponents/telegram-channel-wizard.tsxcomponents/ui/toast.tsxdb/billing.tsdb/dropsbot-webhooks.tsdb/team-workspaces.tsdocs/ACCESS_TIERS.mddocs/BRAND_ASSETS.mddocs/COMPETITIVE-BENCHMARK.mddocs/INTEGRATIONS.mddocs/PRODUCT-CONTRACT.mde2e/contracts/home-builder-p1.spec.tse2e/contracts/member-access.spec.tse2e/contracts/member-project-cloud.spec.tse2e/contracts/release-boundaries.spec.tse2e/contracts/ui-standards.spec.tse2e/interactions/project-dialog-keyboard.spec.tse2e/proofs/all-presets-completion.spec.tse2e/proofs/director-flow.spec.tse2e/proofs/published-products.spec.tse2e/visual/home.spec.tslib/access-tier.tslib/artifact-csp.tslib/billing.tslib/dropsbot-webhook.tslib/html-opening-tag.tslib/http-request-boundary.tslib/member-project-cloud.tslib/member-project-sync-client.tslib/product-blueprint.tslib/product-reality.tslib/project-compiler.tslib/project-export.tslib/project-history.tslib/project-quality.tslib/project-types.tslib/project-workspace.tslib/public-project-shell.tslib/published-quality-evidence.tslib/runtime-active-content.tslib/runtime-external-link.tslib/runtime-srcdoc-security.tslib/server-release-quality.tslib/source-workspace.tslib/team-api.tslib/team-workspaces.tslib/telegram-account-request.tslib/workspace-ai-entitlement.tslib/workspace-ai-patch.tslib/workspace-ai-provider.tslib/workspace-run-digest.tslib/workspace-run-receipt.tslib/workspace-sandbox.tspackage.jsonplaywright.storybook.config.tsscripts/check-ui-guardrails.mjsscripts/serve-storybook-static.mjsstorybook-e2e/visual.spec.tstests/access-tier.test.mjstests/artifact-csp.test.mjstests/billing.test.mjstests/dropsbot-webhook.test.mjstests/generated-products.test.mjstests/html-opening-tag.test.mjstests/http-request-boundary.test.mjstests/member-project-cloud.test.mjstests/member-project-sync-client.test.mjstests/platform-security-p1.test.mjstests/product-reality.test.mjstests/project-export.test.mjstests/project-history.test.mjstests/project-provider-evidence.test.mjstests/project-quality.test.mjstests/project-workspace.test.mjstests/public-project-shell.test.mjstests/published-quality-evidence.test.mjstests/rendered-html.test.mjstests/runtime-external-link.test.mjstests/runtime-preview-security.test.mjstests/server-release-quality.test.mjstests/source-workspace.test.mjstests/storybook-static-server.test.mjstests/studio-account-team-panel.test.mjstests/team-workspaces.test.mjstests/telegram-account-request-boundary.test.mjstests/telegram-export-receipt.test.mjstests/ui-css-policy.test.mjstests/workspace-ai-entitlement.test.mjstests/workspace-ai-patch.test.mjstests/workspace-ai-provider.test.mjstests/workspace-ai-route.test.mjstests/workspace-sandbox.test.mjs
💤 Files with no reviewable changes (1)
- app/styles/project-studio.wide.css
| } catch (error) { | ||
| return NextResponse.json({ error: error instanceof Error ? error.message : "Telegram sign-in could not start." }, { status: 422, headers: { "cache-control": "no-store, max-age=0" } }); | ||
| return telegramAccountJson({ error: error instanceof Error ? error.message : "Telegram sign-in could not start." }, 422); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Don't return raw upstream error text to the client.
The sibling create-channel route was hardened to log the error and return a fixed message; this handler still surfaces error.message from the MTProto call, which can carry internal detail (session/API state, phone value) and gives no server-side trace.
🔒️ Proposed change
} catch (error) {
- return telegramAccountJson({ error: error instanceof Error ? error.message : "Telegram sign-in could not start." }, 422);
+ console.error("Telegram sign-in could not start.", error);
+ return telegramAccountJson({
+ error: "Telegram could not start sign-in. Check the phone number and try again.",
+ }, 422);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| return NextResponse.json({ error: error instanceof Error ? error.message : "Telegram sign-in could not start." }, { status: 422, headers: { "cache-control": "no-store, max-age=0" } }); | |
| return telegramAccountJson({ error: error instanceof Error ? error.message : "Telegram sign-in could not start." }, 422); | |
| } catch (error) { | |
| console.error("Telegram sign-in could not start.", error); | |
| return telegramAccountJson({ | |
| error: "Telegram could not start sign-in. Check the phone number and try again.", | |
| }, 422); |
🤖 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 `@app/api/telegram/account/send-code/route.ts` around lines 31 - 32, Update the
catch block in the Telegram account send-code route to log the caught error
server-side with appropriate context, then return only a fixed generic failure
message through telegramAccountJson instead of exposing error.message. Preserve
the existing 422 response status.
| useEffect(() => { | ||
| let cancelled = false; | ||
| if (!project || !workspaceRunReceipt) return; | ||
| const workspace = project.workspace ?? materializeProjectWorkspace(project); | ||
| const task = workspace.tasks.find( | ||
| (candidate) => candidate.id === workspaceRunReceipt.task, | ||
| ); | ||
| if (!task) return; | ||
| void createWorkspaceRunDigest({ | ||
| files: workspace.files, | ||
| task: { | ||
| id: task.id, | ||
| argv: [task.command, ...task.args], | ||
| cwd: task.cwd ?? ".", | ||
| timeoutMs: 15_000, | ||
| previewPort: task.port, | ||
| }, | ||
| }) | ||
| .then((digest) => { | ||
| if (!cancelled) { | ||
| setWorkspaceRunDigestEvidence({ | ||
| project, | ||
| receipt: workspaceRunReceipt, | ||
| digest, | ||
| }); | ||
| } | ||
| }) | ||
| .catch(() => undefined); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [project, workspaceRunReceipt]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Receipt verification is keyed to project object identity, so unrelated state updates transiently downgrade a verified receipt.
workspaceRunDigestEvidence.project === project (Line 4410) fails whenever any other edit produces a new project object (chat message, spec tweak), which flips currentWorkspaceDigest to null and shows "Historical receipt" until the async digest effect re-runs. Keying the evidence on the workspace revision/digest inputs instead of object identity removes the flicker and the redundant recomputation.
Also applies to: 4409-4414
🤖 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 `@components/project-studio.tsx` around lines 973 - 1004, Update the workspace
run digest evidence flow around createWorkspaceRunDigest and the
workspaceRunDigestEvidence comparison so verification is keyed to stable
workspace revision or digest inputs rather than project object identity.
Preserve verified receipt evidence across unrelated project object updates,
while still invalidating it when the relevant workspace files, task, or receipt
inputs change; avoid redundant digest recomputation.
| {optimisticRevision?.workspaceId === selectedWorkspace?.id ? ( | ||
| <Badge variant="secondary"> | ||
| Optimistic {optimisticRevision.workspaceRevision}/{optimisticRevision.projectRevision} | ||
| </Badge> | ||
| ) : null} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Null dereference when there is no selected workspace and no optimistic revision.
With no teams loaded, optimisticRevision is null and selectedWorkspace is null, so optimisticRevision?.workspaceId === selectedWorkspace?.id compares undefined === undefined → true, and the branch then reads optimisticRevision.workspaceRevision and throws. This section renders unconditionally (the fallback copy is only at Line 1143), so a signed-in account with zero teams crashes the panel.
🐛 Proposed fix
- {optimisticRevision?.workspaceId === selectedWorkspace?.id ? (
+ {optimisticRevision
+ && selectedWorkspace
+ && optimisticRevision.workspaceId === selectedWorkspace.id ? (
<Badge variant="secondary">
Optimistic {optimisticRevision.workspaceRevision}/{optimisticRevision.projectRevision}
</Badge>
) : null}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {optimisticRevision?.workspaceId === selectedWorkspace?.id ? ( | |
| <Badge variant="secondary"> | |
| Optimistic {optimisticRevision.workspaceRevision}/{optimisticRevision.projectRevision} | |
| </Badge> | |
| ) : null} | |
| {optimisticRevision | |
| && selectedWorkspace | |
| && optimisticRevision.workspaceId === selectedWorkspace.id ? ( | |
| <Badge variant="secondary"> | |
| Optimistic {optimisticRevision.workspaceRevision}/{optimisticRevision.projectRevision} | |
| </Badge> | |
| ) : null} |
🤖 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 `@components/studio-account-team-panel.tsx` around lines 1076 - 1080, Update
the optimistic revision badge condition near the optimisticRevision display so
it requires an actual optimisticRevision and selectedWorkspace before comparing
their workspace IDs. Preserve the existing badge rendering for matching,
non-null values while preventing the branch from being entered when either value
is absent.
| `app/globals.css` is an import-only manifest and must remain below 4 KiB. New | ||
| product surfaces belong in local React components using Tailwind CSS v4 and the | ||
| shared Base UI primitives; do not add another monolithic hand-written CSS file. | ||
| The release guardrail rejects any source declaration below 12 px and any manual | ||
| style block added back to `globals.css`. It also rejects any individual manual | ||
| stylesheet above 48 KiB so a new monolithic CSS surface cannot silently return. | ||
|
|
||
| `app/styles/project-studio.runtime.css` is a bounded legacy exception for the | ||
| existing Studio runtime canvas and portalled publish dialog. Targeted | ||
| accessibility and regression fixes may update those existing selectors, but no | ||
| new product surface or component may be added there; new UI stays in local | ||
| Tailwind CSS v4 and Base UI components. The file remains subject to the 48 KiB | ||
| stylesheet cap and is migrated only in reviewed, browser-tested slices. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e css . app/styles --exec ls -l
rg -n "project-studio\.(chrome|workspace|runtime)\.css" -g '!**/node_modules/**'
rg -n "48 ?KiB|49152|4 ?KiB|4096" scripts/check-ui-guardrails.mjs tests/ui-css-policy.test.mjsRepository: svg8bit/drops-studio
Length of output: 1778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== DESIGN.md mentions ==\n'
rg -n "project-studio\.(chrome|workspace|runtime)\.css|globals\.css|48 ?KiB|4 ?KiB|manual style block|guardrail|exception" DESIGN.md app/styles scripts tests || true
printf '\n== Style imports / references ==\n'
rg -n "project-studio\.(chrome|workspace|runtime)\.css" app scripts tests DESIGN.md || true
printf '\n== Guardrail / policy files ==\n'
fd -e mjs -e js -e ts -e md . scripts tests app | sed -n '1,120p'Repository: svg8bit/drops-studio
Length of output: 6246
Document project-studio.chrome.css and project-studio.workspace.css in the CSS policy. DESIGN.md only grants an exception to project-studio.runtime.css, but app/studio/project-studio.css also imports project-studio.chrome.css and project-studio.workspace.css. Add those files here with their intended bounds, or update the guardrail so the policy matches the shipped CSS surface.
🧰 Tools
🪛 LanguageTool
[style] ~107-~107: This is not the usual sequence for adjectives that have no special emphasis.
Context: ...ual manual stylesheet above 48 KiB so a new monolithic CSS surface cannot silently return. `a...
(EN_ADJ_ORDER)
🤖 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 `@DESIGN.md` around lines 102 - 115, Update the CSS policy in DESIGN.md to
explicitly cover project-studio.chrome.css and project-studio.workspace.css,
including their intended legacy bounds and the existing 48 KiB cap.
Alternatively, adjust the release guardrail to enforce the same policy across
these imported stylesheets, ensuring the documented rules match the shipped
imports.
| ### External provider setup | ||
|
|
||
| No billing product, checkout, subscription webhook or paid entitlement store | ||
| is configured. A Pro badge or higher quota must not be enabled until those | ||
| provider-confirmed states exist. At minimum this needs Stripe (or another | ||
| billing provider) credentials, product/price identifiers, webhook verification, | ||
| an entitlement table and cancellation/refund handling. | ||
| The implementation includes billing and team collaboration, but a deployment | ||
| must configure `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, | ||
| `STRIPE_PRO_PRICE_ID`, `DROPS_TEAM_INVITE_SECRET`, the signed account secret and | ||
| durable D1 or private Blob storage. Stripe Customer Portal must be enabled and | ||
| the webhook endpoint must receive the supported subscription lifecycle events. | ||
| Until those external prerequisites exist, the UI and routes report billing and | ||
| teams as unavailable; they do not display an active Pro tier. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Name the "signed account secret" variable.
Every other prerequisite here is a concrete env var; this one leaves the deployer guessing. Use the actual variable name (as done in docs/INTEGRATIONS.md).
📝 Suggested wording
-`STRIPE_PRO_PRICE_ID`, `DROPS_TEAM_INVITE_SECRET`, the signed account secret and
-durable D1 or private Blob storage.
+`STRIPE_PRO_PRICE_ID`, `DROPS_TEAM_INVITE_SECRET`, `<DROPS_ACCOUNT_COOKIE_SECRET>`
+and durable D1 or private Blob storage.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### External provider setup | |
| No billing product, checkout, subscription webhook or paid entitlement store | |
| is configured. A Pro badge or higher quota must not be enabled until those | |
| provider-confirmed states exist. At minimum this needs Stripe (or another | |
| billing provider) credentials, product/price identifiers, webhook verification, | |
| an entitlement table and cancellation/refund handling. | |
| The implementation includes billing and team collaboration, but a deployment | |
| must configure `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, | |
| `STRIPE_PRO_PRICE_ID`, `DROPS_TEAM_INVITE_SECRET`, the signed account secret and | |
| durable D1 or private Blob storage. Stripe Customer Portal must be enabled and | |
| the webhook endpoint must receive the supported subscription lifecycle events. | |
| Until those external prerequisites exist, the UI and routes report billing and | |
| teams as unavailable; they do not display an active Pro tier. | |
| ### External provider setup | |
| The implementation includes billing and team collaboration, but a deployment | |
| must configure `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, | |
| `STRIPE_PRO_PRICE_ID`, `DROPS_TEAM_INVITE_SECRET`, `<DROPS_ACCOUNT_COOKIE_SECRET>` | |
| and durable D1 or private Blob storage. Stripe Customer Portal must be enabled and | |
| the webhook endpoint must receive the supported subscription lifecycle events. | |
| Until those external prerequisites exist, the UI and routes report billing and | |
| teams as unavailable; they do not display an active Pro tier. |
🤖 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 `@docs/ACCESS_TIERS.md` around lines 114 - 122, Update the “External provider
setup” section to replace “the signed account secret” with the actual
environment variable name used by the implementation and documented in
docs/INTEGRATIONS.md, keeping the surrounding prerequisite list unchanged.
| const fourColumnRules = styleSources.flatMap(({ name, source }) => | ||
| [...source.matchAll(/\.project-studio-layout[^\{]*\{([^}]*)\}/g)].flatMap( | ||
| (rule) => | ||
| [...rule[1].matchAll(/grid-template-columns\s*:\s*([^;]+)(?:;|$)/g)] | ||
| .filter((declaration) => countGridTracks(declaration[1]) > 3) | ||
| .map((declaration) => `${name}: ${declaration[1].trim()}`) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Rule-body capture stops at the first }, so nested CSS can smuggle a fourth column past this guardrail. \.project-studio-layout[^\{]*\{([^}]*)\} truncates at the first closing brace; with native nesting (supported by the Lightning CSS pipeline in Tailwind v4), a grid-template-columns declared after a nested block inside .project-studio-layout is never inspected. Match balanced bodies (or scan declarations within the block by brace counting) so the check can't be bypassed.
🤖 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 `@tests/ui-css-policy.test.mjs` around lines 163 - 170, Update the
fourColumnRules extraction in the UI CSS policy test to inspect the complete
.project-studio-layout rule body, including declarations after nested blocks,
instead of using a regex that stops at the first closing brace. Implement
balanced-brace scanning or an equivalent nesting-aware parser, then continue
applying countGridTracks to every grid-template-columns declaration.
| import assert from "node:assert/strict"; | ||
| import { registerHooks } from "node:module"; | ||
| import test from "node:test"; | ||
|
|
||
| const projectRoot = new URL("../", import.meta.url); | ||
|
|
||
| registerHooks({ | ||
| resolve(specifier, context, nextResolve) { | ||
| if (!specifier.startsWith("@/")) return nextResolve(specifier, context); | ||
| const projectPath = specifier.slice(2); | ||
| return { | ||
| shortCircuit: true, | ||
| url: new URL( | ||
| projectPath.endsWith(".ts") ? projectPath : `${projectPath}.ts`, | ||
| projectRoot, | ||
| ).href, | ||
| }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '"engines"|"test"|"node"|--experimental-strip-types|--experimental-transform-types' -A4 package.json
fd -t f -e yml -e yaml . .github/workflows --exec rg -n 'node-version|setup-node' {}Repository: svg8bit/drops-studio
Length of output: 754
🌐 Web query:
Node.js 22.13 registerHooks node:module type stripping .ts imports support version
💡 Result:
In Node.js 22.13.0, support for TypeScript was moved to active development [1][2]. While Node.js 22.13 introduced the module.stripTypeScriptTypes() API for manually stripping types [1][2], built-in support for executing.ts files via type stripping (using the --experimental-strip-types flag) was available in earlier versions [3][4]. Regarding your query: 1. module.stripTypeScriptTypes: This API was added in Node.js 22.13.0 [1][2]. It allows for the programmatic removal of type annotations from TypeScript code [5][6]. 2. Registering Hooks: The module.register() method is a standard feature for customizing module resolution and loading [5][7]. It can be used in conjunction with type stripping; however,.ts imports in Node.js require specific handling. 3..ts Imports Support: Node.js does not natively support resolving.ts file extensions in import specifiers (e.g., import { fn } from './file.ts') without additional configuration or custom hooks [8][9]. Even with type stripping enabled, Node.js generally expects standard module resolution, meaning import paths typically require the.js extension (or no extension, depending on module resolution settings) [8][9]. To support imports that include the.ts extension, you would need to implement a custom loader hook using module.register() to intercept and resolve these specifiers [5][7]. In summary, while Node.js 22.13 enhanced TypeScript support [1][2], it does not automatically resolve.ts import extensions. You must use module.register() to create custom hooks if you need to support that specific import behavior [5][6].
Citations:
- 1: https://github.com/nodejs/node/releases/tag/v22.13.0
- 2: https://nodejs.org/en/blog/release/v22.13.0
- 3: https://nodejs.org/download/release/v22.12.0/docs/api/typescript.html
- 4: https://nodejs.org/docs/v22.12.0/api/typescript.html
- 5: https://nodejs.org/docs/v22.14.0/api/module.html
- 6: https://nodejs.org/download/release/v22.14.0/docs/api/module.html
- 7: https://nodejs.org/download/release/v22.6.0/docs/api/module.html
- 8: https://nodejs.org/docs/latest-v22.x/api/typescript.html
- 9: https://nodejs.org/docs/v22.16.0/api/typescript.html
🌐 Web query:
Node.js registerHooks introduced in which version node:module API
💡 Result:
The module.registerHooks() method in the node:module API was introduced in Node.js version 23.5.0 and version 22.15.0 [1][2]. This API provides a way to register synchronous module customization hooks that run directly in the same thread as the modules being loaded [1][3]. It was implemented to offer a simpler and more performant alternative to the earlier module.register() method, which uses asynchronous hooks running on a separate loader thread and has inherent complexities, such as difficulties with CommonJS module customization and inter-thread communication overhead [1][4]. As a result of the introduction of module.registerHooks(), the older module.register() method has been deprecated [1][5].
Citations:
- 1: http://nodejs.org/docs/latest/api/module.html
- 2: https://nodejs.org/api/module.html
- 3: nodejs/node@e859646
- 4: implement module.registerHooks() to run synchronous module customization hooks in thread nodejs/node#55698
- 5: module: doc-deprecate
module.register()nodejs/node#62395
🌐 Web query:
site:nodejs.org registerHooks node:module Node 22.13 availability
💡 Result:
The module.registerHooks() method is not available in Node.js v22.13.0 [1] or v22.13.1 [2]. Documentation for node:module in these versions does not include registerHooks(). The module.registerHooks() API was introduced in later versions of Node.js, specifically appearing in the documentation for Node.js v22.15.0 [3]. It allows for the registration of synchronous hook functions to customize module resolution and loading behavior, running directly on the main thread rather than a separate loader thread [3][4]. In contrast, the asynchronous module.register() API is available in earlier versions of Node.js, including the v22.x series [5][6].
Citations:
- 1: https://nodejs.org/en/download/archive/v22.13.0
- 2: https://nodejs.org/download/release/v22.13.1/docs/api/index.html
- 3: https://nodejs.org/docs/latest-v22.x/api/module.html
- 4: https://nodejs.org/docs/latest-v24.x/api/module.html
- 5: https://nodejs.org/docs/v22.14.0/api/module.html
- 6: https://nodejs.org/download/release/v22.5.1/docs/api/module.html
🌐 Web query:
"registerHooks" "Node.js" "v22" "module" "registerHooks" "introduced"
💡 Result:
The module.registerHooks function was introduced to Node.js in version 22.15.0 [1]. It provides a mechanism to register synchronous module customization hooks (such as resolve and load) that execute directly within the same thread as the modules being loaded [2][3]. This differs from the earlier module.register API, which uses asynchronous hooks running in a separate loader thread, often incurring higher overhead and presenting challenges related to inter-thread communication and debugging [2][3]. Key characteristics of module.registerHooks include: - Synchronous Execution: It accepts synchronous hook functions, making it easier to debug and avoiding many of the deadlock issues associated with the asynchronous module.register hooks [2][3]. - Performance: Because it avoids inter-thread communication, it is generally recommended as the preferred method for customizing module resolution and loading [2]. - Compatibility: It was specifically designed to be more compatible with existing CommonJS monkey-patching workflows, helping bridge the transition to ESM [3]. - Implementation: When used, it returns an object that allows for deregistering the hooks, ensuring they do not persist for the entire lifetime of the process if not desired [2]. While module.registerHooks is the recommended modern approach, it is distinct from the older module.register method; they are not direct drop-in replacements for one another because one expects synchronous hook definitions and the other expects asynchronous ones [1]. [2][3][1]
Citations:
- 1: Node 26 - DeprecationWarning: module.register() is deprecated. getsentry/sentry-javascript#20810
- 2: https://nodejs.org/api/module.html
- 3: implement module.registerHooks() to run synchronous module customization hooks in thread nodejs/node#55698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'node=%s\n' "$(node -v)"
node - <<'JS'
const m = require('node:module');
console.log('registerHooks' in m ? 'registerHooks=yes' : 'registerHooks=no');
console.log('register' in m ? 'register=yes' : 'register=no');
JS
rg -n '22\.13|24|registerHooks|strip-types|TypeScript' README.md package.json .github/workflows tests lib -g '!**/node_modules/**'Repository: svg8bit/drops-studio
Length of output: 50376
Raise the Node minimum to 22.15+ or drop registerHooks. registerHooks() only lands in Node 22.15, so >=22.13.0 still admits versions where tests/workspace-ai-patch.test.mjs and tests/runtime-preview-security.test.mjs fail at startup.
📍 Affects 2 files
tests/workspace-ai-patch.test.mjs#L1-L19(this comment)tests/runtime-preview-security.test.mjs#L5-L9
🤖 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 `@tests/workspace-ai-patch.test.mjs` around lines 1 - 19, Update the project’s
Node.js minimum requirement to 22.15.0 or newer so registerHooks is available at
runtime; apply this requirement consistently for
tests/workspace-ai-patch.test.mjs (anchor, lines 1-19) and
tests/runtime-preview-security.test.mjs (sibling, lines 5-9), without changing
either test’s hook usage.
| test("canonical validation includes the sandbox package and declared-task boundary", () => { | ||
| const { assertRunnableWorkspaceAiRevision } = api(); | ||
| const current = runnableWorkspace(); | ||
| const unsafe = { | ||
| ...current, | ||
| tasks: current.tasks.map((task) => | ||
| task.id === "build" ? { ...task, args: ["install"] } : task, | ||
| ), | ||
| }; | ||
|
|
||
| assert.throws( | ||
| () => assertRunnableWorkspaceAiRevision(unsafe), | ||
| /cannot install packages directly/i, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The build task mapping can silently no-op.
If runnableWorkspace() ever stops emitting a task with id === "build", unsafe becomes an unchanged copy and the failure message ("cannot install packages directly") gives no hint about why. Assert the task exists first.
🛡️ Proposed fix
const current = runnableWorkspace();
+ assert.ok(
+ current.tasks.some((task) => task.id === "build"),
+ "runnable workspace must declare a build task",
+ );
const unsafe = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("canonical validation includes the sandbox package and declared-task boundary", () => { | |
| const { assertRunnableWorkspaceAiRevision } = api(); | |
| const current = runnableWorkspace(); | |
| const unsafe = { | |
| ...current, | |
| tasks: current.tasks.map((task) => | |
| task.id === "build" ? { ...task, args: ["install"] } : task, | |
| ), | |
| }; | |
| assert.throws( | |
| () => assertRunnableWorkspaceAiRevision(unsafe), | |
| /cannot install packages directly/i, | |
| ); | |
| }); | |
| test("canonical validation includes the sandbox package and declared-task boundary", () => { | |
| const { assertRunnableWorkspaceAiRevision } = api(); | |
| const current = runnableWorkspace(); | |
| assert.ok( | |
| current.tasks.some((task) => task.id === "build"), | |
| "runnable workspace must declare a build task", | |
| ); | |
| const unsafe = { | |
| ...current, | |
| tasks: current.tasks.map((task) => | |
| task.id === "build" ? { ...task, args: ["install"] } : task, | |
| ), | |
| }; | |
| assert.throws( | |
| () => assertRunnableWorkspaceAiRevision(unsafe), | |
| /cannot install packages directly/i, | |
| ); | |
| }); |
🤖 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 `@tests/workspace-ai-patch.test.mjs` around lines 691 - 705, Update the test
around runnableWorkspace and the build-task mutation to first assert that a task
with id "build" exists, then construct unsafe with the modified args. Keep the
existing assert.throws validation unchanged so the test cannot silently pass
against an unchanged workspace.
| assert.deepEqual(workspaceAiPatchJsonSchema.required.sort(), [ | ||
| "baseRevision", | ||
| "operations", | ||
| "summary", | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
.sort() mutates the exported schema in place.
workspaceAiPatchJsonSchema.required is shared module state (see lib/workspace-ai-patch.ts Lines 230-240); sorting it here reorders the array that provider structured-output callers and any later test in this process observe. Copy before sorting.
🐛 Proposed fix
- assert.deepEqual(workspaceAiPatchJsonSchema.required.sort(), [
+ assert.deepEqual([...workspaceAiPatchJsonSchema.required].sort(), [
"baseRevision",
"operations",
"summary",
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert.deepEqual(workspaceAiPatchJsonSchema.required.sort(), [ | |
| "baseRevision", | |
| "operations", | |
| "summary", | |
| ]); | |
| assert.deepEqual([...workspaceAiPatchJsonSchema.required].sort(), [ | |
| "baseRevision", | |
| "operations", | |
| "summary", | |
| ]); |
🤖 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 `@tests/workspace-ai-patch.test.mjs` around lines 786 - 790, Update the
assertion using workspaceAiPatchJsonSchema.required so it sorts a copied array
rather than mutating the exported schema’s shared required array. Preserve the
expected values and assertion behavior while leaving
workspaceAiPatchJsonSchema.required unchanged for provider callers and later
tests.
| import assert from "node:assert/strict"; | ||
| import { registerHooks } from "node:module"; | ||
| import test from "node:test"; | ||
| import { NextRequest } from "next/server.js"; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '"engines"' -A4 package.json
fd -H -t f '.nvmrc|.node-version|.tool-versions' --exec cat
rg -n "node-version" -C2 .github/workflowsRepository: svg8bit/drops-studio
Length of output: 615
🌐 Web query:
Which Node.js version added module.registerHooks?
💡 Result:
The module.registerHooks method was added to Node.js in versions 23.5.0 and 22.15.0 [1][2]. It allows for the registration of synchronous module customization hooks that run on the same thread as the modules being loaded [3][4].
Citations:
- 1: http://nodejs.org/docs/latest/api/module.html
- 2: https://nodejs.org/uk/blog/release/v22.15.0
- 3: implement module.registerHooks() to run synchronous module customization hooks in thread nodejs/node#55698
- 4: https://nodejs.org/en/blog/release/v23.5.0
Bump the Node engine floor or drop registerHooks
tests/workspace-ai-route.test.mjs imports registerHooks, which needs Node 22.15+; package.json still allows 22.13.0, so this file will fail on the declared minimum runtime. Raise the engines floor or avoid this API.
🤖 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 `@tests/workspace-ai-route.test.mjs` around lines 1 - 4, Resolve the Node
runtime compatibility issue in tests/workspace-ai-route.test.mjs by either
raising the package.json Node engine minimum to 22.15.0 or replacing the
registerHooks import with an API supported by the existing minimum version.
Ensure the test remains functional on the declared engine floor.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
db/team-workspaces.ts (1)
754-762: 🧹 Nitpick | 🔵 TrivialConsider also pruning long-accepted invites, not just expired ones.
Pruning here only removes invites past
expiresAt; accepted invites that haven't hit their original expiry persist inworkspace.invitesindefinitely. Over a very long-lived workspace with heavy invite churn this could slowly approachMAX_STORED_INVITES_PER_WORKSPACE(5000), at which pointstoredWorkspacewould start rejecting reads for that workspace entirely. Given the collaborator cap tops out around 100, this is a distant edge case, but worth a retention strategy (e.g., prune accepted invites after some grace period) as the product scales.🤖 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 `@db/team-workspaces.ts` around lines 754 - 762, Extend the invite-pruning logic in the workspace invite flow around workspace.invites and pendingInviteCount to remove accepted invites after an appropriate retention grace period, while continuing to remove expired invites. Preserve unaccepted, non-expired invites and ensure pending invite counting and MAX_PENDING_INVITES_PER_WORKSPACE validation remain unchanged.tests/dropsbot-webhook.test.mjs (1)
136-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese assertions test the component's source text, not its behavior.
/method === "PUT" \? "rotate" : "revoke"/and/mutateCallback\("PUT"\)/pin exact source formatting incomponents/dropsbot-webhook-connection.tsx; a rename, prettier pass, or extracted helper breaks them with no behavior change. The user-visible copy assertions are defensible; the implementation-shape ones aren't. Prefer covering rotate/revoke through the Storybook interaction tests (which already exist for this cohort) and keep the source scan limited to the consent copy.🤖 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 `@tests/dropsbot-webhook.test.mjs` around lines 136 - 140, Update the assertions in the Drops Bot webhook source-scan test to remove implementation-shape checks for the ternary expression and mutateCallback calls. Keep the user-visible consent-copy assertions, and rely on the existing Storybook interaction tests to cover rotate and revoke behavior.
🤖 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 `@app/api/dropsbot/webhooks/route.ts`:
- Around line 67-86: The requireSameOrigin helper should validate the request
Origin against both request.nextUrl.origin and the browser-visible origin
derived from Host plus X-Forwarded-Proto, matching the publish route’s existing
proxy/CDN handling. Reuse the established origin-derivation logic and accept
either origin while preserving the current 403 responses for missing, malformed,
or mismatched origins.
In `@app/api/teams/invites/accept/route.ts`:
- Around line 35-48: Update the invite acceptance flow around
proTeamEntitlements and acceptTeamWorkspaceInvite so entitlement lookup failures
do not block valid invite acceptance. Wrap
proTeamEntitlements(invite.ownerIdentity) in the established try/catch fallback
pattern from privateProjectLimit, use a safe default collaborator cap when
billing is unavailable or the owner is no longer Pro, and pass that cap as
maxCollaborators while preserving the existing invite verification, rate
limiting, and acceptance flow.
In `@components/dropsbot-webhook-connection.tsx`:
- Around line 186-193: Update the PUT success path in the callback rotation
handler to preserve the existing evidence when payload.callbackEvidence is
omitted, rather than assigning null. Keep the returned evidence when present so
the manage controls remain available after rotation.
In `@db/dropsbot-webhooks.ts`:
- Around line 369-389: Update recordD1CallbackEvidence and acceptInState so
last-event evidence only advances when the incoming event is newer than the
stored event, preserving the existing timestamp/hash pair for older or duplicate
deliveries. Apply the same guarded max-based semantics in the D1 duplicate
branch that calls recordD1CallbackEvidence, keeping it consistent with the
accepted-path latest-event query and ensuring timestamp and content hash always
describe the same event.
In `@lib/access-tier.ts`:
- Around line 195-202: Update accountIdentity and its callers to support
dual-read migration: derive the new rotation-independent identity while also
accepting the prior storage identity when reading, and backfill records to the
new key after a successful legacy lookup. Use a separate identity pepper for the
new derivation rather than an unkeyed digest, and preserve new writes under only
the new identity.
---
Nitpick comments:
In `@db/team-workspaces.ts`:
- Around line 754-762: Extend the invite-pruning logic in the workspace invite
flow around workspace.invites and pendingInviteCount to remove accepted invites
after an appropriate retention grace period, while continuing to remove expired
invites. Preserve unaccepted, non-expired invites and ensure pending invite
counting and MAX_PENDING_INVITES_PER_WORKSPACE validation remain unchanged.
In `@tests/dropsbot-webhook.test.mjs`:
- Around line 136-140: Update the assertions in the Drops Bot webhook
source-scan test to remove implementation-shape checks for the ternary
expression and mutateCallback calls. Keep the user-visible consent-copy
assertions, and rely on the existing Storybook interaction tests to cover rotate
and revoke behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d06f6a4-07e2-4e56-8ffa-d56eddbf6179
📒 Files selected for processing (21)
app/api/dropsbot/webhooks/route.tsapp/api/projects/publish/route.tsapp/api/projects/route.tsapp/api/teams/invites/accept/route.tscomponents/dropsbot-webhook-connection.tsxdb/dropsbot-webhooks.tsdb/member-projects.tsdb/team-workspaces.tslib/access-tier.tslib/billing.tslib/source-workspace.tsplaywright.config.tsplaywright.storybook.config.tsstorybook-e2e/visual.spec.tstests/access-tier.test.mjstests/billing.test.mjstests/dropsbot-webhook.test.mjstests/member-project-cloud.test.mjstests/publish-lifecycle.test.mjstests/source-workspace.test.mjstests/team-workspaces.test.mjs
💤 Files with no reviewable changes (1)
- storybook-e2e/visual.spec.ts
| const capability = String(body.capability ?? ""); | ||
| const invite = verifyTeamInviteCapability(capability, secret); | ||
| if (!invite) { | ||
| throw new TeamWorkspaceValidationError("Team invite is invalid or expired."); | ||
| } | ||
| const entitlements = await proTeamEntitlements(invite.ownerIdentity); | ||
| await enforceTeamRateLimit(account.identity, "team-workspace-invite-accept"); | ||
| const result = await acceptTeamWorkspaceInvite({ | ||
| capability, | ||
| memberIdentity: account.identity, | ||
| consent: body.consent === true, | ||
| secret, | ||
| maxCollaborators: entitlements.collaboratorsPerWorkspace, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Hard-gating invite acceptance on the owner's live Pro status defeats the DB layer's graceful degradation.
proTeamEntitlements(invite.ownerIdentity) throws a 403 (PRO_REQUIRED) if the workspace owner isn't currently on an active Pro subscription, and a 503 if billing storage is unavailable — both unhandled here. This means:
- An invitee can no longer accept an otherwise-valid, unexpired invite if the owner's subscription has lapsed since the invite was created, even though
acceptTeamWorkspaceInviteindb/team-workspaces.tswas specifically designed to gracefully clamp viaMath.min(invite.maxCollaborators, input.maxCollaborators ?? invite.maxCollaborators)rather than hard-block. - A transient billing-storage read failure now blocks all invite acceptance platform-wide, since this call sits ahead of the actual accept logic.
app/api/projects/route.ts's privateProjectLimit uses the same billing primitives but wraps them in a try/catch that fails closed to a default limit instead of throwing — the same pattern should be applied here so a missing/lapsed entitlement degrades the collaborator cap rather than blocking acceptance entirely.
🛡️ Proposed direction
- const entitlements = await proTeamEntitlements(invite.ownerIdentity);
+ const entitlements = await proTeamEntitlements(invite.ownerIdentity).catch(() => null);
await enforceTeamRateLimit(account.identity, "team-workspace-invite-accept");
const result = await acceptTeamWorkspaceInvite({
capability,
memberIdentity: account.identity,
consent: body.consent === true,
secret,
- maxCollaborators: entitlements.collaboratorsPerWorkspace,
+ maxCollaborators: entitlements?.collaboratorsPerWorkspace,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const capability = String(body.capability ?? ""); | |
| const invite = verifyTeamInviteCapability(capability, secret); | |
| if (!invite) { | |
| throw new TeamWorkspaceValidationError("Team invite is invalid or expired."); | |
| } | |
| const entitlements = await proTeamEntitlements(invite.ownerIdentity); | |
| await enforceTeamRateLimit(account.identity, "team-workspace-invite-accept"); | |
| const result = await acceptTeamWorkspaceInvite({ | |
| capability, | |
| memberIdentity: account.identity, | |
| consent: body.consent === true, | |
| secret, | |
| maxCollaborators: entitlements.collaboratorsPerWorkspace, | |
| }); | |
| const capability = String(body.capability ?? ""); | |
| const invite = verifyTeamInviteCapability(capability, secret); | |
| if (!invite) { | |
| throw new TeamWorkspaceValidationError("Team invite is invalid or expired."); | |
| } | |
| const entitlements = await proTeamEntitlements(invite.ownerIdentity).catch(() => null); | |
| await enforceTeamRateLimit(account.identity, "team-workspace-invite-accept"); | |
| const result = await acceptTeamWorkspaceInvite({ | |
| capability, | |
| memberIdentity: account.identity, | |
| consent: body.consent === true, | |
| secret, | |
| maxCollaborators: entitlements?.collaboratorsPerWorkspace, | |
| }); |
🤖 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 `@app/api/teams/invites/accept/route.ts` around lines 35 - 48, Update the
invite acceptance flow around proTeamEntitlements and acceptTeamWorkspaceInvite
so entitlement lookup failures do not block valid invite acceptance. Wrap
proTeamEntitlements(invite.ownerIdentity) in the established try/catch fallback
pattern from privateProjectLimit, use a safe default collaborator cap when
billing is unavailable or the owner is no longer Pro, and pass that cap as
maxCollaborators while preserving the existing invite verification, rate
limiting, and acceptance flow.
| if (method === "PUT") { | ||
| if (!payload.callbackUrl) { | ||
| throw new Error("The rotated one-time callback URL was not returned.") | ||
| } | ||
| setCallbackUrl(payload.callbackUrl) | ||
| setEvidence(payload.callbackEvidence ?? null) | ||
| setMessage("Copy the replacement URL now and update @drops. The previous secret URL no longer works.") | ||
| onToast("Drops Bot callback secret rotated") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
A rotate response without callbackEvidence leaves the panel with no controls.
setEvidence(payload.callbackEvidence ?? null) clears evidence while canCreate stays false, so if the PUT response omits callbackEvidence the manage block unmounts and the create block never appears — the user is stuck until a manual refresh. Keep the previous evidence (or re-sync) instead of nulling it on the success path.
♻️ Proposed fix
setCallbackUrl(payload.callbackUrl)
- setEvidence(payload.callbackEvidence ?? null)
+ setEvidence((current) => payload.callbackEvidence ?? current)
setMessage("Copy the replacement URL now and update `@drops`. The previous secret URL no longer works.")
onToast("Drops Bot callback secret rotated")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (method === "PUT") { | |
| if (!payload.callbackUrl) { | |
| throw new Error("The rotated one-time callback URL was not returned.") | |
| } | |
| setCallbackUrl(payload.callbackUrl) | |
| setEvidence(payload.callbackEvidence ?? null) | |
| setMessage("Copy the replacement URL now and update @drops. The previous secret URL no longer works.") | |
| onToast("Drops Bot callback secret rotated") | |
| if (method === "PUT") { | |
| if (!payload.callbackUrl) { | |
| throw new Error("The rotated one-time callback URL was not returned.") | |
| } | |
| setCallbackUrl(payload.callbackUrl) | |
| setEvidence((current) => payload.callbackEvidence ?? current) | |
| setMessage("Copy the replacement URL now and update `@drops`. The previous secret URL no longer works.") | |
| onToast("Drops Bot callback secret rotated") |
🤖 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 `@components/dropsbot-webhook-connection.tsx` around lines 186 - 193, Update
the PUT success path in the callback rotation handler to preserve the existing
evidence when payload.callbackEvidence is omitted, rather than assigning null.
Keep the returned evidence when present so the manage controls remain available
after rotation.
| async function recordD1CallbackEvidence( | ||
| db: D1Database, | ||
| connectionId: string, | ||
| capabilityHash: string, | ||
| event: Pick<DropsBotWebhookEvent, "receivedAt" | "contentHash">, | ||
| ): Promise<DropsBotCallbackEvidence | null> { | ||
| const updated = await db.prepare( | ||
| `UPDATE dropsbot_webhook_connections | ||
| SET callback_received_at = COALESCE(callback_received_at, ?), | ||
| last_event_received_at = ?, last_event_content_hash = ? | ||
| WHERE id = ? AND capability_hash = ?`, | ||
| ).bind( | ||
| event.receivedAt, | ||
| event.receivedAt, | ||
| event.contentHash, | ||
| connectionId, | ||
| capabilityHash, | ||
| ).run(); | ||
| if (Number(updated.meta?.changes ?? 0) < 1) return null; | ||
| return latestD1CallbackEvidence(db, connectionId); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Webhook "last event" evidence can regress on out-of-order/duplicate deliveries — inconsistent across backends.
Three places now update lastEventReceivedAt/lastEventContentHash (or the D1 equivalents) whenever a duplicate event is received:
recordD1CallbackEvidence(369-389) unconditionally setslast_event_received_at/last_event_content_hashto the incoming (duplicate) event's own values.acceptInState(549-551) does the same in-memory, for both the "accepted" and "duplicate" outcomes.- The D1 "duplicate" branch (798-811) calls
recordD1CallbackEvidencewith the incoming event's data.
By contrast, the D1 accepted path's UPDATE (~838-845) correctly derives these fields via ORDER BY received_at DESC LIMIT 1 across all stored events — a true "latest event" query.
If a duplicate delivery of an older event arrives after a genuinely newer, different event has already been recorded, the three paths above will overwrite last_event_content_hash back to the stale duplicate's hash while bumping last_event_received_at to "now" — an internally inconsistent state that misrepresents the Drops Bot integration's actual latest activity in the UI. This is also a cross-backend inconsistency: D1 accepted-path is correct, but D1 duplicate-path and both in-memory paths are not.
🛡️ Proposed direction (in-memory path)
- connection.callbackReceivedAt ??= input.event.receivedAt;
- connection.lastEventReceivedAt = input.event.receivedAt;
- connection.lastEventContentHash = input.event.contentHash;
+ connection.callbackReceivedAt ??= input.event.receivedAt;
+ if (!connection.lastEventReceivedAt || input.event.receivedAt >= connection.lastEventReceivedAt) {
+ connection.lastEventReceivedAt = input.event.receivedAt;
+ connection.lastEventContentHash = input.event.contentHash;
+ }The D1 recordD1CallbackEvidence update would need an equivalent guard (e.g. last_event_received_at = MAX(COALESCE(last_event_received_at, ?), ?) with a matching conditional for the content hash) so all three code paths agree with the accepted-path's max-based semantics.
Also applies to: 537-567, 798-811
🤖 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 `@db/dropsbot-webhooks.ts` around lines 369 - 389, Update
recordD1CallbackEvidence and acceptInState so last-event evidence only advances
when the incoming event is newer than the stored event, preserving the existing
timestamp/hash pair for older or duplicate deliveries. Apply the same guarded
max-based semantics in the D1 duplicate branch that calls
recordD1CallbackEvidence, keeping it consistent with the accepted-path
latest-event query and ensuring timestamp and content hash always describe the
same event.
| function accountIdentity(provider: StudioAccount["provider"], subject: string): string { | ||
| // Storage ownership must survive independent cookie-signing key rotation. | ||
| // The provider subject is authenticated by the signed cookie before this | ||
| // pseudonymous, provider-scoped storage key is accepted. | ||
| return createHash("sha256") | ||
| .update(`drops-studio-account:v1:${provider}:${subject}`, "utf8") | ||
| .digest("hex"); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any migration/dual-read handling for the identity change.
rg -n -e 'accountIdentity' -e 'identity' --iglob '*migrat*' --iglob '*backfill*' .
rg -n 'drops-studio-account:v1' .Repository: svg8bit/drops-studio
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== access-tier slice =="
sed -n '1,280p' lib/access-tier.ts
echo
echo "== keyed/unkeyed hash usage =="
rg -n 'createHmac\("sha256"|createHash\("sha256"|accountIdentity\(' lib . --glob '!**/node_modules/**'
echo
echo "== identity storage paths =="
rg -n 'member-projects/|billing|team-workspace|Drops Bot|drops-studio-account:v1' lib .Repository: svg8bit/drops-studio
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lines 180-240 of lib/access-tier.ts =="
nl -ba lib/access-tier.ts | sed -n '180,240p'
echo
echo "== repository search for identity persistence hooks =="
rg -n 'accountIdentity|drops-studio-account:v1|member-projects/|billing|team-workspace|Drops Bot' .Repository: svg8bit/drops-studio
Length of output: 240
Add a dual-read/backfill for accountIdentity lib/access-tier.ts:195-202 This changes the storage key for existing Studio accounts, so records written under the previous identity will become unreachable unless both identities are accepted during a migration. If rotation independence is the goal, use a separate identity pepper instead of an unkeyed digest.
🤖 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 `@lib/access-tier.ts` around lines 195 - 202, Update accountIdentity and its
callers to support dual-read migration: derive the new rotation-independent
identity while also accepting the prior storage identity when reading, and
backfill records to the new key after a successful legacy lookup. Use a separate
identity pepper for the new derivation rather than an unkeyed digest, and
preserve new writes under only the new identity.
1 participant
Ships the production Drops Studio workspace: multi-file AI editing and sandbox execution, honest DropsTab and Drops Bot boundaries, Telegram MTProto automation, secure publishing and ZIP export, billing and team collaboration, plus the permanent single-left-surface Project Studio architecture. Validation: 372 unit tests, full 177-test Playwright release suite with 36 intentional skips and focused visual rerun, 44 Storybook interaction tests, 42 Storybook visual tests, Lighthouse, Next and vinext builds, and two CodeRabbit review passes.
Summary by CodeRabbit