Migrate API and CLI services to sennoric.com - #87
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR moves web, API, OAuth, email, monitoring, client, and documentation URLs to Sennoric domains. It also adds legacy-domain redirects and secure, single-use session handoffs for authenticated migration. ChangesDomain migration and cutover
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LegacyAPI
participant MigrationCodeTable
participant NewAPI
participant Browser
LegacyAPI->>MigrationCodeTable: create short-lived migration code
LegacyAPI->>Browser: redirect with signed handoff
Browser->>NewAPI: submit signed handoff
NewAPI->>MigrationCodeTable: atomically consume unredeemed code
NewAPI->>Browser: set session cookie and redirect
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
api-proxy-cf/src/index.js (1)
375-405: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winApply migration 041 before deploying these routes.
041_domain_migration_codes.sqldefines the required primary key, nullableredeemed_at, and integer timestamps used withDate.now(). Add tests for expired handoffs and concurrent redemption; current tests cover only sequential replay.🤖 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 `@api-proxy-cf/src/index.js` around lines 375 - 405, Apply migration 041_domain_migration_codes.sql before deploying the domain-migration routes, ensuring domain_migration_codes has the required primary key, nullable redeemed_at, and integer timestamp columns compatible with Date.now(). Extend tests around the domain-migration accept handler to cover expired handoffs and concurrent redemption in addition to sequential replay.api-proxy-cf/src/status.js (1)
77-77: 🩺 Stability & Availability | 🔵 TrivialVerify Resend authorization for the new sender.
Confirm that
sennoric.comis verified in Resend and that the required SPF and DKIM records are active. Otherwise Resend can reject alerts after the incident is written to D1.🤖 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 `@api-proxy-cf/src/status.js` at line 77, Verify that the sennoric.com domain and status@sennoric.com sender are authorized in Resend, with active SPF and DKIM records, before relying on the sender in the alert delivery flow.api-proxy-cf/test/desktop-auth.test.mjs (1)
254-255: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a concurrent acceptance test.
The handler already uses one conditional
UPDATEwithredeemed_at IS NULLand accepts only one changed row. The current test covers only sequential replay. Send two acceptance requests withPromise.alland assert one302response and one400response.🤖 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 `@api-proxy-cf/test/desktop-auth.test.mjs` around lines 254 - 255, Add a concurrent acceptance case around the existing replay test by issuing two requests to acceptUrl.href simultaneously with Promise.all. Assert that the resulting statuses contain exactly one 302 redirect and one 400 rejection, preserving the existing sequential replay coverage.
🤖 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 `@api-proxy-cf/migrations/041_domain_migration_codes.sql`:
- Around line 1-13: Update the account deletion flow to explicitly delete rows
from domain_migration_codes for the target user before deleting the
corresponding users record, while preserving the existing expiry purge and
redemption behavior.
In `@api-proxy-cf/test/desktop-auth.test.mjs`:
- Around line 251-252: Update the cookie assertions in the accepted-auth
response test to also require the Secure attribute, alongside the existing
Domain and HttpOnly checks. Keep the current set-cookie header source and
assertions unchanged otherwise.
- Around line 48-54: Add the user_id foreign-key constraint to the
domain_migration_codes table definition, referencing the users table
consistently with migration 041. Explicitly enable SQLite foreign-key
enforcement for this test database connection before exercising the schema,
while preserving the existing column definitions and test behavior.
In `@api-proxy-cf/test/migrations.test.mjs`:
- Around line 32-45: Rename the test around the visible assertions in the
domain_migration_codes setup so it describes insertion, user association, and
the initial null redeemed_at state; do not retain “single-use” or handoff claims
unless the test is expanded to perform redemption and verify replay rejection.
---
Nitpick comments:
In `@api-proxy-cf/src/index.js`:
- Around line 375-405: Apply migration 041_domain_migration_codes.sql before
deploying the domain-migration routes, ensuring domain_migration_codes has the
required primary key, nullable redeemed_at, and integer timestamp columns
compatible with Date.now(). Extend tests around the domain-migration accept
handler to cover expired handoffs and concurrent redemption in addition to
sequential replay.
In `@api-proxy-cf/src/status.js`:
- Line 77: Verify that the sennoric.com domain and status@sennoric.com sender
are authorized in Resend, with active SPF and DKIM records, before relying on
the sender in the alert delivery flow.
In `@api-proxy-cf/test/desktop-auth.test.mjs`:
- Around line 254-255: Add a concurrent acceptance case around the existing
replay test by issuing two requests to acceptUrl.href simultaneously with
Promise.all. Assert that the resulting statuses contain exactly one 302 redirect
and one 400 rejection, preserving the existing sequential replay coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ddf3ab6-1c42-4107-b9fb-a90e22811cbf
📒 Files selected for processing (24)
AGENTS.mdREADME.mdapi-proxy-cf/migrations/041_domain_migration_codes.sqlapi-proxy-cf/src/chatGeneration.jsapi-proxy-cf/src/index.jsapi-proxy-cf/src/status.jsapi-proxy-cf/src/webOrigins.jsapi-proxy-cf/test/billing.test.mjsapi-proxy-cf/test/chat-generation.test.mjsapi-proxy-cf/test/desktop-auth.test.mjsapi-proxy-cf/test/desktop-integrations.test.mjsapi-proxy-cf/test/migrations.test.mjsapi-proxy-cf/test/sandbox-route.test.mjsapi-proxy-cf/test/status.test.mjsapi-proxy-cf/wrangler.tomlmodels/Lumen/IPYNBs (COLAB RUNS)/In Use/lumen-1.3-sft.ipynbsrc/agent/agent.jssrc/agent/mcp-marketplace.jssrc/agent/models.jssrc/agent/tools.jssrc/bridge.jssrc/config.jssrc/tui/App.jsxtest/cloudArtifactTool.test.js
1 participant
Migrates backend, CLI, email, model, and status URLs to sennoric.com. Adds a signed 60-second single-use cookie handoff, preserves the legacy API during cutover, and routes the old website hostname through the Worker. Includes D1 migration 041 and coverage for replay prevention and redirect behavior.\n\nValidated: 345 CLI tests and 239 Worker tests pass. The D1 migration and Worker routes are already deployed.
Summary by CodeRabbit
New Features
Documentation
Tests