Deslop round 2: real bug fixes — env binding stripping, D1 param limit, silent catches, missing 401 handling - #672
Merged
Merged
Conversation
…hs, redirectTo validation, timestamp helper, alarm state, bounded cache
…ndling on admin POSTs, surface incomplete 2FA setup
…in error, derive cookie domain, clean shutdown exit code
…rrow legacy stable_user_id fallbacks, log failed email rejection audit writes
…eport, connect-oauth posts), guard 2FA cancel after 401, stale-response guard for invites loader
… lists, add shared chunk helper
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds a route-load latch for account client routes, introduces 401-based login redirects in several client actions, migrates worker handlers from ChangesClient Route Load Latch & 401 Redirects
Worker AppEnv → Env Migration
Shared Utilities & Consumers
E2E Seed SQL & Auth Flow
MCP Search/Execute Refactor
Worker Resilience Fixes
Dev Tooling
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kentcdodds
marked this pull request as ready for review
July 8, 2026 09:41
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
🔎 Preview deployed: https://kody-pr-672.kody-a99.workers.dev Worker: Mocks:
|
…memory hydration IN list, guard stored-JSON parses (email, jobs, memory), stop orphaning R2 blobs on delete, reuse shared chunkArray
| try { | ||
| const href = readCurrentRouterHref(handle) | ||
| const search = new URL(href, 'http://localhost').search | ||
| lastLoadedHref = href |
Contributor
There was a problem hiding this comment.
401 load leaves retry loop
Low Severity
New 401 handlers redirect to login and return without resetting in-flight mutation state. actionState stays 'acting' and reportState stays 'submitting', so buttons stay disabled if the redirect is slow or blocked.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 90a82d1. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/worker/src/email/repo.ts (1)
905-923: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDelete the raw MIME blob before removing the row.
This still deletes the DB row before a best-effort R2 delete. If
input.blobs.delete(rawMimeKey)fails, the row is gone and the raw MIME key is no longer retryable, orphaning sensitive email content. This contradicts the retention policy used inpackages/worker/src/email/system-email.ts:264-285.Proposed fix
- await input.db - .prepare(`DELETE FROM email_messages WHERE id = ?`) - .bind(input.messageId) - .run() if (rawMimeKey != null && input.blobs) { - await input.blobs.delete(rawMimeKey).catch((error: unknown) => { - console.warn('email-raw-mime-blob-delete-failed', rawMimeKey, error) - }) + await input.blobs.delete(rawMimeKey) } + await input.db + .prepare(`DELETE FROM email_messages WHERE id = ?`) + .bind(input.messageId) + .run()🤖 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 `@packages/worker/src/email/repo.ts` around lines 905 - 923, Delete the raw MIME blob before removing the `email_messages` row in the delete flow inside `deleteEmailMessage` (the block that reads `raw_mime_key` and calls `input.blobs.delete`). Reorder the operations so the R2 delete happens first and only proceed with the SQL `DELETE FROM email_messages` after the blob removal succeeds; keep the row lookup/guard around `input.blobs` and preserve the best-effort warning log for blob delete failures so the delete remains retryable if the blob removal errors.packages/worker/client/routes/account-package-invocation-tokens.tsx (1)
399-408: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIgnore failures from stale token loads before updating the current view.
The success path drops results after the URL changes, but the catch path still sets error UI and
markFailed(href)for the old token route.🐛 Proposed fix
} catch (error) { if (signal.aborted) return if (loadStartedAtMutationVersion !== mutationVersion) return + if (href !== readCurrentRouterHref(handle)) return status = 'error' message = error instanceof Error ? error.message : 'Unable to load package invocation tokens.'🤖 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 `@packages/worker/client/routes/account-package-invocation-tokens.tsx` around lines 399 - 408, The stale-load guard in the token loading catch path is incomplete, so old route failures still update the current error UI and call loadLatch.markFailed(href). Update the error handling in the token loader to mirror the success-path check by bailing out before setting status/message or marking failed when the loadStartedAtMutationVersion no longer matches mutationVersion, and keep the existing signal.aborted guard in the same flow.
🧹 Nitpick comments (1)
packages/worker/src/package-runtime/package-service.ts (1)
397-406: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winclearAlarm error propagation in finalizeServiceRun path gets effectively swallowed.
The change is correct — if
deleteAlarm()fails, the in-memory state (lines 400-402) is not cleared, keeping the snapshot accurate. However, whenclearAlarm()is called fromfinalizeServiceRun(line 475) withinrunServiceInBackground, a thrown error falls into the catch block (line 558), which callsfinalizeServiceRunagain — but that call early-returns becausecurrentRunIdwas already nulled (line 453). The error is lost in this path.This is a pre-existing architectural concern (not introduced by this change — previously the error was swallowed by
.catch()and the in-memory state was incorrectly cleared). The new behavior is strictly better. Consider adding aconsole.warnor structured log whenclearAlarm()throws so the stale-alarm condition is at least observable.🤖 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 `@packages/worker/src/package-runtime/package-service.ts` around lines 397 - 406, The clearAlarm failure path in finalizeServiceRun is still effectively silent when runServiceInBackground catches the error and retries finalizeServiceRun, so add an explicit warning/log at the clearAlarm call site or inside clearAlarm itself to surface deleteAlarm failures. Use the package service methods clearAlarm, finalizeServiceRun, and runServiceInBackground to place the log where the error is first thrown, and include enough context to identify the stale-alarm condition without changing the existing error propagation 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 `@packages/worker/client/route-load-latch.ts`:
- Around line 21-23: The route load latch currently only records failures in
markFailed() and leaves lastLoadedHref stale, so a failed refresh can keep a
route incorrectly marked as already loaded. Update route-load-latch.ts so the
failure path invalidates the loaded href state as well, and add a regression
test covering markLoaded('/a'), markFailed('/a'), navigating to '/b', then back
to '/a' where needsLoad() should return true. Use the existing needsLoad(),
markLoaded(), and markFailed() flow to verify the latch resets correctly after a
failed refresh.
In `@packages/worker/src/mcp/tools/execute.ts`:
- Around line 179-199: The outer error handling in execute() is classifying
every thrown failure as a platform sandbox failure, but bundling the
user-provided code in runModuleWithRegistry can throw and should be treated as a
user-code error. Update the execute tool flow so bundling failures are converted
into the existing result.error path, or explicitly detect bundling-related
throws before the outer catch, and keep sandboxError false only for true
platform/setup failures. Use the execute() wrapper, runModuleWithRegistry, and
the result.error handling path to locate the fix.
- Around line 311-329: The success event is being emitted before response
formatting in execute(), so a later failure in limitExecutionResultValue() can
leave a false success log. Move the logMcpEvent call in execute() to after the
response payload has been fully formatted, ideally after
limitExecutionResultValue() and raw content extraction succeed, so the success
outcome is only recorded once the return data is ready.
In `@tools/mcp-test-support.ts`:
- Around line 478-480: The readiness check currently only respects the global
deadline before calling fetch, so a stalled response can hang past
defaultWaitTimeoutMs. Update the readiness helper that performs await
fetch(input.url) and input.isReady(response) to bound each fetch attempt with
its own timeout or abort signal derived from the remaining deadline, and make
sure the per-attempt timeout is enforced even after the TCP connection is
established.
In `@tools/seed-sql.ts`:
- Around line 30-32: The seed SQL generated by the SQL builder in
tools/seed-sql.ts is missing stable_user_id, so update the insert logic in the
seed helper that builds the users VALUES statement to include the derived stable
user ID alongside username, email, password_hash, and email_verified_at. Make
sure the same deterministic value used by the signup path is included in the
seed output so tool/E2E fixtures match the production schema and no longer
depend on the legacy fallback.
In `@tsconfig-tools.json`:
- Line 40: Remove the inline comment from the JSON in tsconfig-tools.json so the
file remains valid for Biome parsing and static analysis. Update the JSON entry
directly where the comment appears, keeping the surrounding tsconfig-tools
configuration intact and preserving the referenced modules/settings without any
comment text.
---
Outside diff comments:
In `@packages/worker/client/routes/account-package-invocation-tokens.tsx`:
- Around line 399-408: The stale-load guard in the token loading catch path is
incomplete, so old route failures still update the current error UI and call
loadLatch.markFailed(href). Update the error handling in the token loader to
mirror the success-path check by bailing out before setting status/message or
marking failed when the loadStartedAtMutationVersion no longer matches
mutationVersion, and keep the existing signal.aborted guard in the same flow.
In `@packages/worker/src/email/repo.ts`:
- Around line 905-923: Delete the raw MIME blob before removing the
`email_messages` row in the delete flow inside `deleteEmailMessage` (the block
that reads `raw_mime_key` and calls `input.blobs.delete`). Reorder the
operations so the R2 delete happens first and only proceed with the SQL `DELETE
FROM email_messages` after the blob removal succeeds; keep the row lookup/guard
around `input.blobs` and preserve the best-effort warning log for blob delete
failures so the delete remains retryable if the blob removal errors.
---
Nitpick comments:
In `@packages/worker/src/package-runtime/package-service.ts`:
- Around line 397-406: The clearAlarm failure path in finalizeServiceRun is
still effectively silent when runServiceInBackground catches the error and
retries finalizeServiceRun, so add an explicit warning/log at the clearAlarm
call site or inside clearAlarm itself to surface deleteAlarm failures. Use the
package service methods clearAlarm, finalizeServiceRun, and
runServiceInBackground to place the log where the error is first thrown, and
include enough context to identify the stale-alarm condition without changing
the existing error propagation 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: 1a4ab74a-6590-4c92-8481-a23428284f04
📒 Files selected for processing (70)
cli.tse2e/auth-test-user.tse2e/community-frames.spec.tse2e/d1-utils.tse2e/playwright-utils.tse2e/smoke.spec.tse2e/ssr-hydration.spec.tspackages/shared/src/chunk.node.test.tspackages/shared/src/chunk.tspackages/shared/src/date-keys.tspackages/shared/src/tags-json.tspackages/worker/client/route-load-latch.node.test.tspackages/worker/client/route-load-latch.tspackages/worker/client/routes/account-integrations.tsxpackages/worker/client/routes/account-mcp-servers.tsxpackages/worker/client/routes/account-package-invocation-tokens.tsxpackages/worker/client/routes/account-passkeys.tsxpackages/worker/client/routes/account-remote-connectors.tsxpackages/worker/client/routes/account-two-factor.tsxpackages/worker/client/routes/account.tsxpackages/worker/client/routes/admin-community-reports.tsxpackages/worker/client/routes/admin-invites.tsxpackages/worker/client/routes/community-detail.tsxpackages/worker/client/routes/connect-oauth.tsxpackages/worker/src/app/admin-system-email-data.tspackages/worker/src/app/admin-usage-data.tspackages/worker/src/app/admin-user-creation.tspackages/worker/src/app/admin-users-data.tspackages/worker/src/app/auth-redirect.tspackages/worker/src/app/email-change.tspackages/worker/src/app/email-verification.tspackages/worker/src/app/env.tspackages/worker/src/app/handler.tspackages/worker/src/app/handlers/account-email-change.tspackages/worker/src/app/handlers/account-passkeys.tspackages/worker/src/app/handlers/account-profile.tspackages/worker/src/app/handlers/account-resend-verification.tspackages/worker/src/app/handlers/account-two-factor.tspackages/worker/src/app/handlers/auth-page.tspackages/worker/src/app/handlers/auth.tspackages/worker/src/app/handlers/password-reset.tspackages/worker/src/app/handlers/verify.tspackages/worker/src/app/handlers/webauthn.tspackages/worker/src/app/query-params.tspackages/worker/src/app/request-auth-cache.tspackages/worker/src/app/router.tspackages/worker/src/app/username.tspackages/worker/src/community/repo.tspackages/worker/src/community/snapshot.tspackages/worker/src/email/inbound.tspackages/worker/src/email/platform-address.tspackages/worker/src/email/repo.tspackages/worker/src/entitlements/service.tspackages/worker/src/jobs/repo.tspackages/worker/src/mcp-auth-user-context.tspackages/worker/src/mcp/capabilities/meta/search.tspackages/worker/src/mcp/memory/json-string-array.tspackages/worker/src/mcp/memory/repo.tspackages/worker/src/mcp/run-kody-registry.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/package-registry/repo.tspackages/worker/src/package-registry/service.tspackages/worker/src/package-runtime/package-service.tstools/ci/sync-worker-secrets.tstools/mcp-test-support.tstools/seed-sql.tstools/seed-test-data.tstsconfig-tools.jsonwrangler-env.ts
Comment on lines
311
to
+329
| logMcpEvent({ | ||
| category: 'mcp', | ||
| tool: 'execute', | ||
| toolName: 'execute', | ||
| outcome: 'failure', | ||
| outcome: 'success', | ||
| durationMs, | ||
| baseUrl, | ||
| hasUser, | ||
| registeredCapabilityCount, | ||
| sandboxError: true, | ||
| errorName, | ||
| errorMessage, | ||
| cause: result.error, | ||
| sandboxError: false, | ||
| context: activeStorageId ? { storageId: activeStorageId } : undefined, | ||
| }) | ||
| const limitedResult = limitExecutionResultValue( | ||
| result.result, | ||
| responseLimit ?? defaultExecutionResponseLimitBytes, | ||
| ) | ||
| const rawContent = limitedResult.truncated | ||
| ? null | ||
| : extractRawContent(limitedResult.value) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the success log still precedes result limiting/formatting.
sed -n '300,335p' packages/worker/src/mcp/tools/execute.ts
sed -n '945,993p' packages/worker/src/mcp/executor.tsRepository: kentcdodds/kody
Length of output: 2580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== execute.ts around the success log =="
sed -n '300,360p' packages/worker/src/mcp/tools/execute.ts
echo
echo "== executor.ts around response formatting helpers =="
rg -n "function (limitExecutionResultValue|extractRawContent|formatLimitedExecutionOutput)|const (limitExecutionResultValue|extractRawContent|formatLimitedExecutionOutput)" packages/worker/src/mcp/executor.ts packages/worker/src/mcp/tools/execute.ts
sed -n '1,260p' packages/worker/src/mcp/executor.tsRepository: kentcdodds/kody
Length of output: 9256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== execute.ts try/catch around success path =="
sed -n '240,380p' packages/worker/src/mcp/tools/execute.ts
echo
echo "== executor.ts helper bodies =="
sed -n '920,1045p' packages/worker/src/mcp/executor.tsRepository: kentcdodds/kody
Length of output: 6780
Move the success log after response formatting.
limitExecutionResultValue() can throw on non-serializable results (for example, circular data or BigInt), so emitting outcome: 'success' before that step can produce a success event followed by a failure path. Format the response first, then log success once the return payload is ready.
🤖 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 `@packages/worker/src/mcp/tools/execute.ts` around lines 311 - 329, The success
event is being emitted before response formatting in execute(), so a later
failure in limitExecutionResultValue() can leave a false success log. Move the
logMcpEvent call in execute() to after the response payload has been fully
formatted, ideally after limitExecutionResultValue() and raw content extraction
succeed, so the success outcome is only recorded once the return data is ready.
| "./tools/**/*.ts", | ||
| "./packages/shared/src/**/*.ts" | ||
| "./packages/shared/src/**/*.ts", | ||
| // Worker app modules imported by the seeding tools. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the inline comment from this JSON file.
Biome reports this line as a parse error, so tsconfig-tools.json currently fails the configured static analysis.
Proposed fix
- // Worker app modules imported by the seeding tools.
"./packages/worker/src/app/username.ts",📝 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.
Suggested change
| // Worker app modules imported by the seeding tools. |
🧰 Tools
🪛 Biome (2.5.1)
[error] 40-40: Expected an array, an object, or a literal but instead found '// Worker app modules imported by the seeding tools.'.
(parse)
[error] 40-40: End of file expected
(parse)
🤖 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 `@tsconfig-tools.json` at line 40, Remove the inline comment from the JSON in
tsconfig-tools.json so the file remains valid for Biome parsing and static
analysis. Update the JSON entry directly where the comment appears, keeping the
surrounding tsconfig-tools configuration intact and preserving the referenced
modules/settings without any comment text.
Source: Linters/SAST tools
…ing throws as sandbox errors, bound readiness fetch attempts, seed stable_user_id
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 73de898. Configure here.
| needsStaleRefresh) && | ||
| typeof document !== 'undefined' | ||
| ) { | ||
| status !== 'loading' && !loadLatch.isLoadedFor(currentHref) |
Contributor
There was a problem hiding this comment.
Unused variable left behind after latch refactor
Low Severity
isRefreshingForLocationChange is computed but never read. All six other routes refactored in this PR dropped this intermediate variable entirely and rely solely on loadLatch.needsLoad(...). This route accidentally kept the now-dead assignment, adding confusion about whether it participates in the load decision.
Reviewed by Cursor Bugbot for commit 73de898. Configure here.
This was referenced Jul 10, 2026
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Round two of the AI-slop audit (follow-up to #668). This round is mostly behavior fixes rather than pure deduplication: silent failure paths, an env-schema bug that stripped Cloudflare bindings, a D1 bound-parameter limit bug on admin pages, and a repeated failed-load retry bug across seven client routes. A final round-4 sweep (after merging
mainwith #669) added fixes in memory search, email storage, and jobs.High-severity fixes
packages/worker/src/app/env.ts):getEnvreturnedparseSafe(EnvSchema, env).value, which drops any binding not listed in the schema — includingREPO_SESSION,OAUTH_PROVIDER,REMOTE_CONNECTOR_SESSION, andASSETS. Account deletion was silently failing to purge repo sessions and revoke OAuth grants because handlers sawundefinedbindings behindas unknown as Envcasts.getEnvnow validates in place and returns the originalEnv;router.tsand ~10 handler factories drop theirAppEnv → Envcasts and takeEnvdirectly.too many SQL variableson full admin pages:pageSize=100madeloadCurrentMonthRollupsbind 101 parameters (100 user ids + month), over D1's 100-parameter cap, so/admin/usage.json?pageSize=100returned 500. New@kody-internal/shared/chunk.ts(chunkArray+maxD1BoundParameters) chunks theIN (...)lists here and inloadRolesByUserIds.listMemoriesByIds, which binds 103–104 parameters (userId+ ids + statuses) — semantic matches on older memories would fail the whole search. Now chunked with headroom for the fixed bindings.stable_user_id(admin-user-creation.ts): the follow-upUPDATE ... .catch(() => undefined)meant a user could be created with no stable id and no error. TheINSERTnow setsstable_user_iddirectly.lastLoadedHrefwas assigned before the fetch, so a failed load latched the URL and revisiting the route never retried. New#client/route-load-latch.ts(with unit tests) marks a URL loaded only on success; applied toaccount,account-integrations,account-mcp-servers,account-remote-connectors,account-package-invocation-tokens,account-two-factor,account-passkeys. Per Bugbot's review: a one-shot stale-refresh signal (same-path reload whose loader failed) now overrides the failure latch instead of being blocked by it.Silent-failure and error-handling fixes
stable_user_idfallbacks inentitlements/service.tsandemail/platform-address.tscaught all D1 errors; now they only fall back on the known missing-column error and rethrow everything else.email/inbound.tsaudit-trail writes used.catch(() => undefined)seven times; failures are now logged (warnRejectionAuditWriteFailed).email/repo.ts(round 4):deleteEmailMessageByIdswallowed D1 read failures with.catch(() => null)before deleting the row — a transient error would orphan the raw-MIME R2 blob forever. The read now propagates so the delete aborts and can retry.package-service.tsno longer clears alarm state in memory whendeleteAlarm()fails.mcp/tools/execute.tsgets an outer error boundary so setup failures return a structured MCP error instead of an unhandled rejection;run-kody-registry.tswarns instead of silently hiding missing MCP servers.community/snapshot.tstreats bad KV snapshots as a cache miss;tags_jsonparsing centralized in@kody-internal/shared/tags-json.ts; (round 4)email/repo.tsaddress/header columns,jobs/repo.tsschedule/params/history columns, and memorytags_json/source_uris_jsonall degrade to safe defaults instead of throwing on one bad row.mcp/tools/search.tsvalue-entity detail no longer reports false "not found" from a stale in-memory snapshot; falls back to a directgetValue.Auth/session correctness
handlers/verify.tsforwarded rawredirectTo(open-redirect shape); now normalized via the sharednormalizeRedirectTo./loginredirects added: passkey register/delete, community report submit, admin invites/community-reports mutations, and all sixsecrets.jsonPOSTs inconnect-oauth.tsx; 2FA cancel no longer continues after a 401.updated_atwrites inaccount-profile.ts/password-reset.tsnow use sharedutcSqliteTimestampinstead of two hand-rolled.replace('T', ' ').slice(0, 19)copies.Dedupe / tooling
displayNameFromEmail,readPagination,parseTagsJson, seed-SQL builders (tools/seed-sql.ts), consolidated readiness waits intools/mcp-test-support.ts, deduped signup/login ine2e/playwright-utils.ts(with the WebAuthnlocalhostcookie requirement now documented),failhelper deduped in CI tools, clean exit code on signal shutdown inwrangler-env.ts,community/repo.tslocalchunkValuesreplaced with sharedchunkArray.Notes for review
as unknown as DynamicCallableWorkflowStepcast inpackage-workflows.ts: removing it trips TS "type instantiation is excessively deep" viaRpc.Serializable<JsonValue>.mainafter Scale hardening: R2 email blobs, derived usage rollups, bounded cron/DO work, cheap search paths #669 landed:queryCurrentMonthRollupskeeps main's KV cachified wrapper, with chunking applied inside the fresh-value query.System recap — extends app-ui/entitlements error contracts (medium risk)
Mode: recap · Base:
main@924c6246· Head:90a82d15Classification: extends — no new primitives, but
app-ui's env handling and several error contracts change behavior (fail-closed instead of fail-silent). One new shared utility module (chunk.ts), not a system primitive.Primitives touched
app-uigetEnvreturns fullEnv(bindings no longer stripped); handlers takeEnvdirectly; 401 redirects and load-latch fixes in client routesentitlementsemailmcp-servermemoriesjobsusage-meteringrbacaccount-exportREPO_SESSION/OAUTH_PROVIDERbindingsd1-app-dbSystem map
Invariants
Per-user isolation untouched: no query, Durable Object id, or vector path changed its userId scoping. The env fix strengthens account-deletion guarantees (sessions purged, OAuth grants revoked — previously silently skipped).
Testing
npm run validate— format:check, lint, typecheck, 699 unit tests (205 files), Playwright E2E, MCP E2E all green (run after merging latestmain, including Scale hardening: R2 email blobs, derived usage rollups, bounded cron/DO work, cheap search paths #669's rollup caching, and again after the round-4 fixes; one flaky E2E run passed clean on retry and Nx flagged the task as flaky)packages/shared/src/chunk.node.test.ts,packages/worker/client/route-load-latch.node.test.ts(including the stale-refresh-vs-failure-latch case from Bugbot's review)too many SQL variablesfailure via/admin/usage.json?pageSize=100before the chunking fix; green afterSummary by CodeRabbit
New Features
IN (...)queries to respect D1 limits.Bug Fixes
Tests
Chores