Add queued markdown imports#365
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces the client-side import flow with a backend import job pipeline, adds a new Data settings page and import history UI, and rewires CMS/API code to shared sanitize and queue modules. It also adds worker parsing, cleanup jobs, and the new ChangesBackend-driven import pipeline
Sequence Diagram(s)sequenceDiagram
participant User
participant ImportModal as ImportModal
participant ImportRoute as /api/data/import
participant R2 as R2 Storage
participant DB
participant TaskQueue
participant Worker as runImport
User->>ImportModal: choose file or URL
ImportModal->>ImportRoute: POST upload request
ImportRoute->>R2: store upload
ImportRoute->>DB: create import job
ImportRoute->>TaskQueue: enqueue import.process
TaskQueue->>Worker: deliver job
Worker->>R2: read upload
Worker->>DB: write import records and final status
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
marble-jobs | 655bdbc | Commit Preview URL Branch Preview URL |
Jun 27 2026, 09:42 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/jobs/src/lib/import.ts (1)
116-132: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAtomically claim queued import jobs before processing.
The current entrypoint can process jobs already marked
processing, and the queued → processing transition is not conditional. Duplicate queue deliveries can therefore run Lines 194-225 twice for the same job and create duplicate posts/import items.Proposed claim guard
- if (job.status !== "queued" && job.status !== "processing") { + if (job.status !== "queued") { return; } - if (job.status === "queued") { - await db.importJob.update({ - where: { id: job.id }, - data: { status: "processing", startedAt: job.startedAt ?? new Date() }, - }); + const claim = await db.importJob.updateMany({ + where: { id: job.id, status: "queued" }, + data: { status: "processing", startedAt: job.startedAt ?? new Date() }, + }); + + if (claim.count === 0) { + return; }🤖 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 `@apps/jobs/src/lib/import.ts` around lines 116 - 132, Atomically claim import jobs in runImport before any work starts, instead of processing existing processing jobs directly. Update the queued-to-processing transition in runImport so it only succeeds when the job is still queued (use a conditional update/claim pattern on db.importJob.update or a helper around getImportJob), and only continue into the import flow after that claim succeeds. Make the job status check in runImport refuse already-processing jobs unless they were just claimed by this invocation, so duplicate deliveries cannot execute the later import/post creation logic twice.
🧹 Nitpick comments (5)
apps/cms/src/app/api/data/import/route.ts (1)
99-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
console.errorin this route.Please route this through the project's normal logging utility instead. As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Don't use console.🤖 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 `@apps/cms/src/app/api/data/import/route.ts` at line 99, The import route is logging with console.error, which violates the project’s no-console guideline. Update the error handling in the route handler for the import upload path to use the project’s standard logging utility instead of console.error, preserving the same error context and message while routing it through the existing logger used elsewhere in the app.Source: Coding guidelines
apps/jobs/src/scheduled/imports.ts (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
console.*in the new cleanup task.Use the project logging abstraction or add an explicit lint exception if worker cleanup intentionally logs directly. As per coding guidelines,
**/*.{ts,tsx,js,jsx}: "Don't use console."Also applies to: 117-123
🤖 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 `@apps/jobs/src/scheduled/imports.ts` at line 28, The new cleanup task is logging directly with console.error, which violates the project logging guideline. Update the cleanup flow in imports.ts to use the existing logging abstraction used by the worker/job code, or add an explicit lint exception only if direct console logging is truly intentional. Make the change in the cleanup task logic around the import deletion path so the log still includes the import id and error details without using console.*.Source: Coding guidelines
apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/data/page.tsx (1)
9-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unnecessary
asyncmodifier.
Pagedoes not await anything, so this adds an avoidable promise boundary and violates the repo rule for async functions. As per coding guidelines, "Make sure async functions actually use await".Suggested fix
-async function Page() { +function Page() { return <PageClient />; }🤖 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 `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/settings/data/page.tsx around lines 9 - 11, The Page component is marked async even though it does not await anything, so remove the unnecessary async modifier from Page and keep it as a normal component that returns PageClient directly. Use the Page function in the settings data page as the target for this cleanup to comply with the repo rule that async functions must actually use await.Source: Coding guidelines
apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsx (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repo-standard icon package here.
This change introduces Hugeicons for the import action, but the repository guidelines require Phosphor icons for UI icons. Please switch this trigger to a Phosphor icon instead. As per coding guidelines, "Always use the Phosphor Icons package for icons".
Also applies to: 133-142
🤖 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 `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/posts/page-client.tsx around lines 3 - 4, The import/action trigger in the posts page is using Hugeicons instead of the repo-standard Phosphor icons. Update the icon imports in page-client.tsx and the related icon usage in the import action component/trigger (including the code around the referenced import action block) to use a Phosphor icon component instead, and remove the Hugeicons-specific imports and JSX so the UI follows the repository icon guideline.Source: Coding guidelines
apps/cms/src/components/settings/fields/import.tsx (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repo-standard icon package here.
This new settings card uses Hugeicons, but the repository guidelines require Phosphor icons for UI iconography. Please swap this to a Phosphor equivalent before merge. As per coding guidelines, "Always use the Phosphor Icons package for icons".
Also applies to: 115-119
🤖 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 `@apps/cms/src/components/settings/fields/import.tsx` around lines 3 - 5, The settings field icon import is using Hugeicons instead of the repository-standard Phosphor package. Update the icon usage in the import settings component by replacing FileImportIcon and HugeiconsIcon with the appropriate Phosphor icon components, and adjust the JSX in the related settings card to use the new icon API consistently. Keep the Button usage unchanged and ensure all iconography in this component follows the same Phosphor pattern.Source: Coding guidelines
🤖 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 `@apps/cms/src/app/api/data/import/route.ts`:
- Around line 88-123: The import route currently uploads to R2 before creating
the import job, so a failure in db.importJob.create() can leave an orphaned
imports/... object behind. Update the handler in route.ts to track the uploaded
object key and wrap the db.importJob.create() call in error handling that
deletes the R2 object when job creation fails; keep the cleanup tied to the
existing uploadKey/jobData flow so the file upload and database record stay
consistent.
- Around line 111-115: The import route is still enqueuing URL-based jobs via
the `jobData` branch that sets `source: "url"` and `sourceUrl`, but the current
worker contract only supports file/R2 imports. Update the `route.ts` import
handler to reject URL imports for now by returning an error response instead of
creating that `jobData` shape, and keep the existing file-backed path intact for
the worker to consume.
In `@apps/cms/src/app/api/posts/route.ts`:
- Around line 11-14: The post creation flow is waiting on dashboard event
delivery, which keeps a slow or half-open internal events request on the
mutation critical path. Update the shared helper in "`@/lib/queues/events`" around
emitDashboardEvent/logDashboardEventError to use a non-blocking delivery
strategy or a bounded timeout, or move the dispatch so the route handler returns
immediately after the transaction commits. Keep the post mutation path in the
route handler independent of the event call so a successful write is not delayed
by dashboard event delivery.
In `@apps/cms/src/components/settings/fields/import.tsx`:
- Around line 80-101: The useQuery in import.tsx currently treats fetch failures
as missing data, so the empty-state UI can mask real outages. Update the imports
view logic around useQuery, latestJobs, and the empty-state rendering to branch
on the query error/loading state instead of relying only on data being
undefined. Add an explicit error state with a retry action for failed requests,
while keeping the existing imports list and polling behavior for successful
responses.
- Around line 18-24: The import status handling in the `status` union and
related polling/spinner checks is missing the backend’s `discovering` state, so
the job can appear idle too early. Update the local status type in the import
component and the active-state logic used by the polling/rendering paths
(including the checks around the import request flow) to treat `discovering` the
same as the other in-progress states. Use the `status` union and the
polling/spinner conditions in this component to locate and adjust all affected
branches.
In `@apps/cms/src/utils/import.ts`:
- Around line 88-124: The import request parser in importRequestSource currently
assumes request.formData() always succeeds, which can turn malformed multipart
or unsupported bodies into a 500. Wrap the request.formData() call in error
handling and return a validation-style error response when parsing fails, before
the existing file/url checks and importRequestSourceSchema validation run.
In `@apps/jobs/src/scheduled/imports.ts`:
- Around line 74-82: The stale-job cleanup in imports.ts can overwrite a job
that finished after selection because the current db.importJob.update call only
filters by id. Update the logic in the scheduled import cleanup flow around the
job status mutation to use a guarded db.importJob.updateMany (or equivalent
conditional update) that повторяет the stale/status predicate from the earlier
findMany selection before setting terminal fields like status, failedAt,
errorMessage, and uploadKey. Then only apply the failure mutation when the
conditional update actually matches the still-stale job.
In `@apps/jobs/src/utils/import-content.ts`:
- Around line 152-155: Update isMdxExportStart() in import-content.ts so it also
recognizes MDX re-export statements, not just declarations and export default.
Extend the regex to catch forms like export { ... } from ... and export * from
..., so stripMdxModuleSyntax() removes these before Markdown parsing. Keep the
change localized to the export-detection helper used by stripMdxModuleSyntax().
- Around line 246-270: The ZIP import logic in parseMarkdownZipImport currently
checks MAX_ZIP_EXTRACTED_BYTES only after unzipSync(bytes) has already
materialized all entries, so oversized archives can inflate memory before the
guard runs. Move the limit enforcement into unzipSync via its filter hook, using
each entry’s originalSize to stop processing once the cumulative extracted size
would exceed the cap, or switch parseMarkdownZipImport to the streaming Unzip
API if needed; keep the existing file-count and markdown-file checks intact.
In `@apps/jobs/src/utils/import-records.ts`:
- Around line 30-73: The slug generation in getUniquePostSlug and
getUniqueAuthorSlug is still vulnerable to races because it checks uniqueness
before the eventual create/upsert in importMarkdownFile and getImportAuthor.
Move slug resolution to the write path or retry on unique-constraint failure so
tx.post.create and db.author.upsert cannot lose to a concurrent writer; keep the
helper names as the central location for this logic and ensure the final
persisted slug is chosen atomically.
In `@packages/events/src/queue.ts`:
- Around line 28-30: The shared queue contract in TaskMessage has been narrowed
too early, and tasks.ts now treats old import.create messages as unknown. Keep
backward-compatible handling for the legacy import.create variant in the task
consumer while the queue drains, or add an explicit migration/purge step before
removing it; make sure TaskMessage and the consumer logic in tasks.ts stay
aligned until no old messages remain.
In `@packages/utils/src/functions/sanitize.ts`:
- Line 68: The input sanitization rule is still stripping the disabled state
from kept checkbox inputs, so imported task-list checkboxes become interactive
after sanitizeHtml() runs. Update the allowed input attributes in sanitize.ts so
the sanitizeHtml/exclusiveFilter path preserves disabled for checkbox inputs
while still allowing type and checked, and make sure any related attribute list
entries referenced by the comment are updated consistently.
---
Outside diff comments:
In `@apps/jobs/src/lib/import.ts`:
- Around line 116-132: Atomically claim import jobs in runImport before any work
starts, instead of processing existing processing jobs directly. Update the
queued-to-processing transition in runImport so it only succeeds when the job is
still queued (use a conditional update/claim pattern on db.importJob.update or a
helper around getImportJob), and only continue into the import flow after that
claim succeeds. Make the job status check in runImport refuse already-processing
jobs unless they were just claimed by this invocation, so duplicate deliveries
cannot execute the later import/post creation logic twice.
---
Nitpick comments:
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/posts/page-client.tsx:
- Around line 3-4: The import/action trigger in the posts page is using
Hugeicons instead of the repo-standard Phosphor icons. Update the icon imports
in page-client.tsx and the related icon usage in the import action
component/trigger (including the code around the referenced import action block)
to use a Phosphor icon component instead, and remove the Hugeicons-specific
imports and JSX so the UI follows the repository icon guideline.
In `@apps/cms/src/app/`(main)/[workspace]/(dashboard)/settings/data/page.tsx:
- Around line 9-11: The Page component is marked async even though it does not
await anything, so remove the unnecessary async modifier from Page and keep it
as a normal component that returns PageClient directly. Use the Page function in
the settings data page as the target for this cleanup to comply with the repo
rule that async functions must actually use await.
In `@apps/cms/src/app/api/data/import/route.ts`:
- Line 99: The import route is logging with console.error, which violates the
project’s no-console guideline. Update the error handling in the route handler
for the import upload path to use the project’s standard logging utility instead
of console.error, preserving the same error context and message while routing it
through the existing logger used elsewhere in the app.
In `@apps/cms/src/components/settings/fields/import.tsx`:
- Around line 3-5: The settings field icon import is using Hugeicons instead of
the repository-standard Phosphor package. Update the icon usage in the import
settings component by replacing FileImportIcon and HugeiconsIcon with the
appropriate Phosphor icon components, and adjust the JSX in the related settings
card to use the new icon API consistently. Keep the Button usage unchanged and
ensure all iconography in this component follows the same Phosphor pattern.
In `@apps/jobs/src/scheduled/imports.ts`:
- Line 28: The new cleanup task is logging directly with console.error, which
violates the project logging guideline. Update the cleanup flow in imports.ts to
use the existing logging abstraction used by the worker/job code, or add an
explicit lint exception only if direct console logging is truly intentional.
Make the change in the cleanup task logic around the import deletion path so the
log still includes the import id and error details without using console.*.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2d25954-714c-4a4f-84b3-c49779c8f16e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (56)
.gitignoreapps/api/package.jsonapps/api/src/lib/fields.tsapps/api/src/lib/sanitize.tsapps/api/src/routes/posts.tsapps/api/src/validations/misc.tsapps/cms/package.jsonapps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/data/page-client.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/data/page.tsxapps/cms/src/app/(main)/[workspace]/(dashboard)/settings/general/page-client.tsxapps/cms/src/app/api/authors/[id]/route.tsapps/cms/src/app/api/authors/route.tsapps/cms/src/app/api/categories/[id]/route.tsapps/cms/src/app/api/categories/route.tsapps/cms/src/app/api/data/export/[id]/download/route.tsapps/cms/src/app/api/data/export/route.tsapps/cms/src/app/api/data/import/route.tsapps/cms/src/app/api/media/route.tsapps/cms/src/app/api/posts/[id]/fields/route.tsapps/cms/src/app/api/posts/[id]/route.tsapps/cms/src/app/api/posts/import/route.tsapps/cms/src/app/api/posts/route.tsapps/cms/src/app/api/tags/[id]/route.tsapps/cms/src/app/api/tags/route.tsapps/cms/src/app/api/upload/complete/route.tsapps/cms/src/components/layout/page-header.tsxapps/cms/src/components/nav/nav-settings.tsxapps/cms/src/components/posts/data-view.tsxapps/cms/src/components/posts/import-modal.tsxapps/cms/src/components/settings/fields/export.tsxapps/cms/src/components/settings/fields/import.tsxapps/cms/src/components/shared/dropzone.tsxapps/cms/src/lib/queries/keys.tsapps/cms/src/lib/queues/events.tsapps/cms/src/lib/queues/tasks.tsapps/cms/src/lib/validations/import.tsapps/cms/src/utils/import.tsapps/jobs/package.jsonapps/jobs/src/consumers/dlq.tsapps/jobs/src/consumers/tasks.tsapps/jobs/src/lib/constants.tsapps/jobs/src/lib/export.tsapps/jobs/src/lib/import.tsapps/jobs/src/scheduled/cleanup.tsapps/jobs/src/scheduled/imports.tsapps/jobs/src/types/env.tsapps/jobs/src/types/import.tsapps/jobs/src/utils/import-content.tsapps/jobs/src/utils/import-records.tsapps/jobs/tsconfig.jsonapps/jobs/worker-configuration.d.tspackages/events/src/queue.tspackages/parser/package.jsonpackages/utils/package.jsonpackages/utils/src/functions/sanitize.ts
💤 Files with no reviewable changes (7)
- apps/api/package.json
- apps/cms/src/app/api/posts/import/route.ts
- apps/api/src/lib/sanitize.ts
- apps/cms/src/app/(main)/[workspace]/(dashboard)/settings/general/page-client.tsx
- apps/jobs/src/consumers/dlq.ts
- apps/cms/package.json
- apps/api/src/validations/misc.ts
| import { | ||
| emitDashboardEvent, | ||
| logDashboardEventError, | ||
| } from "@/lib/events/dispatch"; | ||
| import { getDashboardPosts } from "@/lib/queries/dashboard/posts"; | ||
| } from "@/lib/queues/events"; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep dashboard event delivery off the mutation critical path.
@/lib/queues/events posts to ${MARBLE_API_URL}/internal/events with fetch() and no timeout, and this handler awaits it after the transaction commits. If the internal API is slow or half-open, post creation can hang even though the write already succeeded; fix this once in the shared helper or dispatch it after the response.
🤖 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 `@apps/cms/src/app/api/posts/route.ts` around lines 11 - 14, The post creation
flow is waiting on dashboard event delivery, which keeps a slow or half-open
internal events request on the mutation critical path. Update the shared helper
in "`@/lib/queues/events`" around emitDashboardEvent/logDashboardEventError to use
a non-blocking delivery strategy or a bounded timeout, or move the dispatch so
the route handler returns immediately after the transaction commits. Keep the
post mutation path in the route handler independent of the event call so a
successful write is not delayed by dashboard event delivery.
| export async function getUniquePostSlug( | ||
| db: DbClient, | ||
| workspaceId: string, | ||
| preferredSlug: string | ||
| ) { | ||
| const baseSlug = preferredSlug || "post"; | ||
| let slug = baseSlug; | ||
| let suffix = 1; | ||
|
|
||
| while ( | ||
| await db.post.findUnique({ | ||
| where: { workspaceId_slug: { workspaceId, slug } }, | ||
| select: { id: true }, | ||
| }) | ||
| ) { | ||
| suffix += 1; | ||
| slug = `${baseSlug}-${suffix}`; | ||
| } | ||
|
|
||
| return slug; | ||
| } | ||
|
|
||
| /** Finds a unique author slug within the workspace by appending a numeric suffix. */ | ||
| export async function getUniqueAuthorSlug( | ||
| db: DbClient, | ||
| workspaceId: string, | ||
| preferredSlug: string | ||
| ) { | ||
| const baseSlug = preferredSlug || "imported-author"; | ||
| let slug = baseSlug; | ||
| let suffix = 1; | ||
|
|
||
| while ( | ||
| await db.author.findUnique({ | ||
| where: { workspaceId_slug: { workspaceId, slug } }, | ||
| select: { id: true }, | ||
| }) | ||
| ) { | ||
| suffix += 1; | ||
| slug = `${baseSlug}-${suffix}`; | ||
| } | ||
|
|
||
| return slug; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
These slug helpers are still race-prone.
Both functions do a read-then-write uniqueness loop outside the eventual create. importMarkdownFile() calls getUniquePostSlug() before tx.post.create(...), and getImportAuthor() computes an author slug before db.author.upsert(...), so a concurrent writer can still claim the same slug between the check and the write. The result is intermittent unique-constraint failures and failed imports under parallel jobs.
🤖 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 `@apps/jobs/src/utils/import-records.ts` around lines 30 - 73, The slug
generation in getUniquePostSlug and getUniqueAuthorSlug is still vulnerable to
races because it checks uniqueness before the eventual create/upsert in
importMarkdownFile and getImportAuthor. Move slug resolution to the write path
or retry on unique-constraint failure so tx.post.create and db.author.upsert
cannot lose to a concurrent writer; keep the helper names as the central
location for this logic and ensure the final persisted slug is chosen
atomically.
| export type TaskMessage = | ||
| | { type: "export.process"; jobId: string } | ||
| | { type: "import.process"; jobId: string } | ||
| | { type: "import.create"; jobId: string }; | ||
| | { type: "import.process"; jobId: string }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep a temporary compatibility path for import.create.
This narrows the shared queue contract immediately, but apps/jobs/src/consumers/tasks.ts now throws on any unknown task type. Any already-enqueued import.create message during rollout will be retried/DLQ'd instead of finishing the import. Keep the old variant handling until the queue is drained, or explicitly migrate/purge outstanding messages before deploy.
🤖 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/events/src/queue.ts` around lines 28 - 30, The shared queue contract
in TaskMessage has been narrowed too early, and tasks.ts now treats old
import.create messages as unknown. Keep backward-compatible handling for the
legacy import.create variant in the task consumer while the queue drains, or add
an explicit migration/purge step before removing it; make sure TaskMessage and
the consumer logic in tasks.ts stay aligned until no old messages remain.
| div: ["data-twitter", "data-src", "data-youtube-video"], | ||
| span: ["style", "data-color"], | ||
| iframe: ["src", "allowfullscreen", "style", "width", "height"], | ||
| input: ["type", "checked"], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve disabled on allowed checkboxes.
exclusiveFilter now keeps checkbox inputs, but allowedAttributes.input still strips disabled. That turns imported task-list checkboxes into live controls once sanitizeHtml() runs in apps/jobs/src/lib/import.ts, so stored post content no longer matches the Markdown source.
Suggested fix
- input: ["type", "checked"],
+ input: ["type", "checked", "disabled"],Also applies to: 104-105
🤖 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/utils/src/functions/sanitize.ts` at line 68, The input sanitization
rule is still stripping the disabled state from kept checkbox inputs, so
imported task-list checkboxes become interactive after sanitizeHtml() runs.
Update the allowed input attributes in sanitize.ts so the
sanitizeHtml/exclusiveFilter path preserves disabled for checkbox inputs while
still allowing type and checked, and make sure any related attribute list
entries referenced by the comment are updated consistently.
![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
marble/apps/jobs/src/lib/import.ts
Lines 124 to 128 in 796bd3c
Make import retries idempotent
When a queue delivery is retried after the worker has already created some posts but before the final importJob.update completes, the job remains processing; this guard explicitly allows processing jobs to run again. Since importMarkdownFile always inserts new ImportItems/posts and only suffixes slugs, the retry imports the same files a second time instead of resuming or skipping existing items.
ℹ️ 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".
|
|
||
| /** Extracts supported Markdown files from a zip archive in deterministic order. */ | ||
| function parseMarkdownZipImport(bytes: Uint8Array) { | ||
| const entries = unzipSync(bytes); |
There was a problem hiding this comment.
For ZIP imports, unzipSync(bytes) inflates every entry before the .md/.mdx filter and before MAX_ZIP_EXTRACTED_BYTES is checked. An otherwise accepted 4 MB zip can contain a huge unsupported or hidden entry, so the Worker does all decompression and can run out of CPU/memory before the safeguards run; cap total uncompressed size during extraction or use streaming/lazy entry processing.
Useful? React with 👍 / 👎.
| if (activeWorkspace?.id) { | ||
| await queryClient.invalidateQueries({ | ||
| queryKey: QUERY_KEYS.POSTS(workspaceId), | ||
| queryKey: QUERY_KEYS.IMPORTS(activeWorkspace.id), | ||
| }); |
There was a problem hiding this comment.
When imports are launched from the posts page, that page's React Query entry stays fresh for one hour (apps/cms/src/app/(main)/[workspace]/(dashboard)/posts/page-client.tsx:64-69). This success handler only invalidates IMPORTS, so after the worker creates drafts and the user follows “View posts”, the old list/empty state can be reused until refresh or stale time; invalidate the posts query when the import is started or when the polled job completes.
Useful? React with 👍 / 👎.
| categoryId, | ||
| publishedAt: now, | ||
| workspaceId: job.workspaceId, | ||
| authors: { | ||
| connect: [{ id: authorId }], |
There was a problem hiding this comment.
For Markdown with category, tags, or author frontmatter, parseMarkdownImport records those values on the ImportItem, but the draft post created here always uses the fallback Uncategorized category and connects only the job creator, with no tag relation. Those imported drafts lose the metadata the import flow parsed, so ZIPs/single files with frontmatter don't round-trip taxonomy/author data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/jobs/src/scheduled/imports.ts (1)
109-113: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBase terminal-job retention on terminal timestamps, not
createdAt.A job that stays active longer than the retention window will match this query as soon as it flips to
failedorcompleted, so the same cleanup run can delete it with effectively no terminal retention. Filter terminal jobs byfailedAt/completedAtinstead.Suggested fix
const oldJobs = await db.importJob.findMany({ where: { - createdAt: { lt: retentionCutoff }, - status: { in: ["completed", "failed"] }, + OR: [ + { status: "completed", completedAt: { lt: retentionCutoff } }, + { status: "failed", failedAt: { lt: retentionCutoff } }, + ], },🤖 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 `@apps/jobs/src/scheduled/imports.ts` around lines 109 - 113, The retention query in the import cleanup path is using createdAt in the oldJobs lookup, which can delete jobs immediately after they become terminal. Update the filtering in the scheduled import cleanup logic to base terminal-job retention on the actual terminal timestamps in db.importJob.findMany, using failedAt for failed jobs and completedAt for completed jobs instead of createdAt, while keeping the existing status filter and retentionCutoff behavior.
🧹 Nitpick comments (3)
apps/cms/src/app/api/data/import/route.ts (1)
94-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared logger instead of
console.error.These new failure paths log raw request/SDK errors directly from the route handler. Please send them through the app's structured/redacted logging path rather than
console.*. As per coding guidelines, "Don't use console".Also applies to: 146-152
🤖 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 `@apps/cms/src/app/api/data/import/route.ts` around lines 94 - 95, The import route is logging raw failures with console.error instead of the app’s structured logger, so replace these error paths in the route handler with the shared logging utility used by the CMS app. Update the failure handling in the import upload flow and the other affected failure block referenced by the same route to call the existing logger with redacted/structured error context rather than console.*. Use the route handler and the import/upload helper functions in this file to locate the affected spots.Source: Coding guidelines
apps/jobs/src/scheduled/imports.ts (2)
71-106: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThis cleanup batch is fully serialized.
Each DB mutation and storage delete waits for the previous job, so one slow call stretches the whole batch. Consider
Promise.allSettledwith a small concurrency limit here. As per coding guidelines, "Don't use await inside loops".Also applies to: 123-137
🤖 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 `@apps/jobs/src/scheduled/imports.ts` around lines 71 - 106, The cleanup loop in the scheduled imports job is fully serialized because `staleJobs` is processed with sequential awaits inside the `imports` flow, which makes one slow DB or storage call block the whole batch. Refactor the per-job work in this section to run with bounded concurrency instead of awaiting inside the loop, using a small limit and `Promise.allSettled`-style handling while preserving the current `db.importJob.updateMany` and `deleteImportUpload` behavior and the existing `staleCount`/`uploadKey` updates.Source: Coding guidelines
140-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared logger instead of
console.log.Please route these cleanup summaries through the app logger rather than
console.*. As per coding guidelines, "Don't use console".🤖 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 `@apps/jobs/src/scheduled/imports.ts` around lines 140 - 145, The cleanup summary messages in the import job cleanup flow are still using console.log, which violates the shared logging guideline. Update the logging in the imports cleanup logic to route both the stale-count and deleted-count summaries through the app’s shared logger instead of console, using the existing logger available in the imports.ts cleanup path and the related cleanup function that emits these messages.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@apps/jobs/src/scheduled/imports.ts`:
- Around line 109-113: The retention query in the import cleanup path is using
createdAt in the oldJobs lookup, which can delete jobs immediately after they
become terminal. Update the filtering in the scheduled import cleanup logic to
base terminal-job retention on the actual terminal timestamps in
db.importJob.findMany, using failedAt for failed jobs and completedAt for
completed jobs instead of createdAt, while keeping the existing status filter
and retentionCutoff behavior.
---
Nitpick comments:
In `@apps/cms/src/app/api/data/import/route.ts`:
- Around line 94-95: The import route is logging raw failures with console.error
instead of the app’s structured logger, so replace these error paths in the
route handler with the shared logging utility used by the CMS app. Update the
failure handling in the import upload flow and the other affected failure block
referenced by the same route to call the existing logger with
redacted/structured error context rather than console.*. Use the route handler
and the import/upload helper functions in this file to locate the affected
spots.
In `@apps/jobs/src/scheduled/imports.ts`:
- Around line 71-106: The cleanup loop in the scheduled imports job is fully
serialized because `staleJobs` is processed with sequential awaits inside the
`imports` flow, which makes one slow DB or storage call block the whole batch.
Refactor the per-job work in this section to run with bounded concurrency
instead of awaiting inside the loop, using a small limit and
`Promise.allSettled`-style handling while preserving the current
`db.importJob.updateMany` and `deleteImportUpload` behavior and the existing
`staleCount`/`uploadKey` updates.
- Around line 140-145: The cleanup summary messages in the import job cleanup
flow are still using console.log, which violates the shared logging guideline.
Update the logging in the imports cleanup logic to route both the stale-count
and deleted-count summaries through the app’s shared logger instead of console,
using the existing logger available in the imports.ts cleanup path and the
related cleanup function that emits these messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 786293d5-19d7-4604-aeb1-51ab4d10729d
📒 Files selected for processing (7)
apps/cms/src/app/api/data/import/route.tsapps/cms/src/components/settings/fields/import.tsxapps/cms/src/utils/import.tsapps/jobs/src/lib/import.tsapps/jobs/src/scheduled/imports.tsapps/jobs/src/utils/import-content.tsapps/jobs/src/utils/import-records.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/cms/src/components/settings/fields/import.tsx
- apps/cms/src/utils/import.ts
- apps/jobs/src/utils/import-content.ts
- apps/jobs/src/lib/import.ts
1 participant
Description
Adds file-based data import and export flows for Marble workspaces, centered on queued Markdown imports. The CMS settings page can now upload Markdown files or ZIP archives, create import jobs, and show recent import/export activity while the jobs worker parses supported files and creates posts asynchronously.
Motivation and Context
Workspace data imports need to handle larger uploads without blocking the CMS request lifecycle. Moving Markdown processing into queued jobs gives users a clearer import flow, lets ZIP archives create one post per supported file, and keeps import/export status visible from settings.
How to Test
MARBLE_API_URL=http://localhost:8787and matchingSYSTEM_SECRET.Screenshots (if applicable)
N/A
Video Demo (if applicable)
N/A
Types of Changes
Summary by CodeRabbit
<input>elements).