Dogfood Workflow for Intent sync#951
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces durable TanStack Workflow orchestration for intent package discovery and processing. It replaces two separate Netlify scheduled functions with a unified workflow-driven system: database schema and runtime persistence for workflows, npm and GitHub package discovery/filtering, intent version processing with skill extraction, admin UI to view workflow runs, and comprehensive integration tests. ChangesIntent Sync Workflow Implementation
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🧹 Nitpick comments (1)
src/utils/intent-sync.server.ts (1)
258-259: 💤 Low valueRedundant database write:
markPackageVerifiedduplicates the upsert.
upsertIntentPackagewithverified: truealready sets bothverifiedandlastSyncedAton conflict. The subsequentmarkPackageVerifiedcall performs the same update.♻️ Suggested fix
await upsertIntentPackage({ name: packageJson.name, verified: true }) - await markPackageVerified(packageJson.name) const packument = await fetchPackument(packageJson.name)🤖 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 `@src/utils/intent-sync.server.ts` around lines 258 - 259, The second database write is redundant: upsertIntentPackage({ name: packageJson.name, verified: true }) already sets verified and lastSyncedAt on conflict, so remove the subsequent markPackageVerified(packageJson.name) call (or guard it behind a meaningful conditional) to avoid duplicate updates; locate the calls to upsertIntentPackage and markPackageVerified and delete or skip the markPackageVerified invocation for packageJson.name.
🤖 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 `@netlify/functions/workflow-sweep-background.ts`:
- Around line 10-19: The current handler exported via
createNetlifyWorkflowSweepHandler is both a scheduled function (config.schedule
= WORKFLOW_RUNTIME_SWEEP_CRON) and uses the -background deployment model, which
Netlify doesn’t support; split responsibilities: create one scheduled function
(remove the -background suffix from its export/name) that calls or enqueues the
sweep (use createNetlifyWorkflowSweepConfig with schedule =
WORKFLOW_RUNTIME_SWEEP_CRON and export a normal function that triggers the job),
and create a separate background function (keep the -background suffix) that
implements the long-running work using createNetlifyWorkflowSweepHandler with
appropriate maxDurationMs/maxTimers; alternatively, if you prefer a single
function, remove the -background suffix and lower maxDurationMs to the
scheduled-function limit so the scheduled function can run directly. Ensure
references to createNetlifyWorkflowSweepHandler,
createNetlifyWorkflowSweepConfig, and WORKFLOW_RUNTIME_SWEEP_CRON are updated
accordingly.
In `@src/utils/intent-admin.server.ts`:
- Around line 85-99: The current code calls workflowExecutionStore.listRuns
twice with limit: 5 (for INTENT_DISCOVER_WORKFLOW_ID and
INTENT_PROCESS_WORKFLOW_ID), but then merges and slices to 10, which can drop
newer runs; update the per-workflow fetch to request up to the final desired cap
(e.g., limit: 10) so each call to
workflowExecutionStore.listRuns(INTENT_DISCOVER_WORKFLOW_ID,
INTENT_PROCESS_WORKFLOW_ID) can return up to 10 entries before merging, then
keep the existing .flat().sort(...).slice(0, 10) logic to produce the true 10
most-recent runs.
---
Nitpick comments:
In `@src/utils/intent-sync.server.ts`:
- Around line 258-259: The second database write is redundant:
upsertIntentPackage({ name: packageJson.name, verified: true }) already sets
verified and lastSyncedAt on conflict, so remove the subsequent
markPackageVerified(packageJson.name) call (or guard it behind a meaningful
conditional) to avoid duplicate updates; locate the calls to upsertIntentPackage
and markPackageVerified and delete or skip the markPackageVerified invocation
for packageJson.name.
🪄 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
Run ID: 9b70ed18-5e2a-4881-89ff-49dfec4bd23f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.pnpmfile.cjsdocs/internal/workflow-adapter-research.mddocs/internal/workflow-intent-sync.mddrizzle/migrations/0000_workflow_run_store.sqlnetlify/functions/sync-intent-discover-background.tsnetlify/functions/sync-intent-process-background.tsnetlify/functions/workflow-sweep-background.tspackage.jsonscripts/workflow.tssrc/db/schema.tssrc/routes/admin/intent.tsxsrc/utils/intent-admin.functions.tssrc/utils/intent-admin.server.tssrc/utils/intent-db.server.tssrc/utils/intent-sync.server.tssrc/utils/intent-workflows.server.tssrc/utils/workflow-registrations.server.tssrc/utils/workflow-runtime.server.tstests/intent-workflow.test.ts
💤 Files with no reviewable changes (2)
- netlify/functions/sync-intent-process-background.ts
- netlify/functions/sync-intent-discover-background.ts
| export default createNetlifyWorkflowSweepHandler({ | ||
| runtime: workflowRuntime, | ||
| maxDurationMs: 25_000, | ||
| maxScheduledRuns: 10, | ||
| maxTimers: 10, | ||
| }) | ||
|
|
||
| export const config = createNetlifyWorkflowSweepConfig({ | ||
| schedule: WORKFLOW_RUNTIME_SWEEP_CRON, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Netlify support a scheduled function that is also a background function (-background suffix) in 2026?
💡 Result:
As of May 2026, Netlify does not support a single function that is simultaneously a scheduled function and a background function. These are distinct types of serverless functions with different invocation models and limitations [1][2]. Scheduled functions are invoked by Netlify on a regular, cron-like schedule and are intended for automated, periodic tasks [1][3]. They are designed to run for a limited duration (typically 30 seconds) and do not support direct URL invocation or payload processing [1]. Background functions (defined by the -background suffix in the filename) are designed for long-running, asynchronous tasks—supporting execution times of up to 15 minutes—and are typically triggered by client-side HTTP requests [2]. Because these two configurations serve different purposes and utilize different invocation patterns, they cannot be combined into a single file or deployment construct [1][2]. Users needing both scheduling and long-running capabilities typically achieve this by having a scheduled function trigger a separate background function or a workflow process [1][4]. Attempts to apply both configurations to the same function—such as naming a file with a -background suffix while also including a schedule configuration—are not natively supported and will not function as expected [5].
Citations:
- 1: https://docs.netlify.com/build/functions/scheduled-functions/
- 2: https://docs.netlify.com/build/functions/background-functions/
- 3: https://netli.fyi/blog/netlify-scheduled-functions-cloud-cron-jobs
- 4: https://github.com/netlify/next-scheduled-bg-function-migration
- 5: https://answers.netlify.com/t/scheduled-background-function-is-being-deployed-but-not-executing/120731
Fix -background + config.schedule usage for the workflow sweep handler
Netlify doesn’t support deploying a single function that is both scheduled (config.schedule) and a background function (-background suffix) (as of May 2026). This means the sweep likely won’t run as intended (e.g., the schedule may be ignored or invocation won’t match the background model). Split this into a scheduled function that triggers/enqueues the sweep, and run the long-running work in a separate background function (or remove -background and align maxDurationMs to the scheduled-function limit).
🤖 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 `@netlify/functions/workflow-sweep-background.ts` around lines 10 - 19, The
current handler exported via createNetlifyWorkflowSweepHandler is both a
scheduled function (config.schedule = WORKFLOW_RUNTIME_SWEEP_CRON) and uses the
-background deployment model, which Netlify doesn’t support; split
responsibilities: create one scheduled function (remove the -background suffix
from its export/name) that calls or enqueues the sweep (use
createNetlifyWorkflowSweepConfig with schedule = WORKFLOW_RUNTIME_SWEEP_CRON and
export a normal function that triggers the job), and create a separate
background function (keep the -background suffix) that implements the
long-running work using createNetlifyWorkflowSweepHandler with appropriate
maxDurationMs/maxTimers; alternatively, if you prefer a single function, remove
the -background suffix and lower maxDurationMs to the scheduled-function limit
so the scheduled function can run directly. Ensure references to
createNetlifyWorkflowSweepHandler, createNetlifyWorkflowSweepConfig, and
WORKFLOW_RUNTIME_SWEEP_CRON are updated accordingly.
| const runs = await Promise.all([ | ||
| workflowExecutionStore.listRuns({ | ||
| workflowId: INTENT_DISCOVER_WORKFLOW_ID, | ||
| limit: 5, | ||
| }), | ||
| workflowExecutionStore.listRuns({ | ||
| workflowId: INTENT_PROCESS_WORKFLOW_ID, | ||
| limit: 5, | ||
| }), | ||
| ]) | ||
|
|
||
| return runs | ||
| .flat() | ||
| .sort((a, b) => b.updatedAt - a.updatedAt) | ||
| .slice(0, 10) |
There was a problem hiding this comment.
Per-workflow cap of 5 can yield an inaccurate "10 most recent" list.
Each query is capped at limit: 5, but the merged result is sliced to 10. If one workflow has more than 5 recent runs that are all newer than the other workflow's runs, those extra recent runs are dropped before sorting, so the top-10 won't reflect the true most-recent runs across both. Fetch up to the final cap from each side.
🔧 Proposed fix
const runs = await Promise.all([
workflowExecutionStore.listRuns({
workflowId: INTENT_DISCOVER_WORKFLOW_ID,
- limit: 5,
+ limit: 10,
}),
workflowExecutionStore.listRuns({
workflowId: INTENT_PROCESS_WORKFLOW_ID,
- limit: 5,
+ limit: 10,
}),
])📝 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 runs = await Promise.all([ | |
| workflowExecutionStore.listRuns({ | |
| workflowId: INTENT_DISCOVER_WORKFLOW_ID, | |
| limit: 5, | |
| }), | |
| workflowExecutionStore.listRuns({ | |
| workflowId: INTENT_PROCESS_WORKFLOW_ID, | |
| limit: 5, | |
| }), | |
| ]) | |
| return runs | |
| .flat() | |
| .sort((a, b) => b.updatedAt - a.updatedAt) | |
| .slice(0, 10) | |
| const runs = await Promise.all([ | |
| workflowExecutionStore.listRuns({ | |
| workflowId: INTENT_DISCOVER_WORKFLOW_ID, | |
| limit: 10, | |
| }), | |
| workflowExecutionStore.listRuns({ | |
| workflowId: INTENT_PROCESS_WORKFLOW_ID, | |
| limit: 10, | |
| }), | |
| ]) | |
| return runs | |
| .flat() | |
| .sort((a, b) => b.updatedAt - a.updatedAt) | |
| .slice(0, 10) |
🤖 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 `@src/utils/intent-admin.server.ts` around lines 85 - 99, The current code
calls workflowExecutionStore.listRuns twice with limit: 5 (for
INTENT_DISCOVER_WORKFLOW_ID and INTENT_PROCESS_WORKFLOW_ID), but then merges and
slices to 10, which can drop newer runs; update the per-workflow fetch to
request up to the final desired cap (e.g., limit: 10) so each call to
workflowExecutionStore.listRuns(INTENT_DISCOVER_WORKFLOW_ID,
INTENT_PROCESS_WORKFLOW_ID) can return up to 10 entries before merging, then
keep the existing .flat().sort(...).slice(0, 10) logic to produce the true 10
most-recent runs.
tannerlinsley
force-pushed
the
split/workflow-intent-sync-poc
branch
from
224037b to
60021d7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
netlify/functions/workflow-sweep-background.ts (1)
12-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
config.scheduleon a-backgroundfunction is still a deployment-model conflict.Line 13 schedules this function, but this file is still a background function (
workflow-sweep-background.ts). Netlify treats scheduled and background functions as distinct invocation models, so this combo is likely to misfire in production. Split into: (1) scheduled function trigger, (2) background worker.As of May 2026, does Netlify officially support a single function that is BOTH a scheduled function (export const config.schedule) and a background function (filename ends with -background)?🤖 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 `@netlify/functions/workflow-sweep-background.ts` around lines 12 - 14, The file workflow-sweep-background.ts is both named as a background worker and exports config.schedule, which Netlify treats as conflicting invocation models; fix by splitting responsibilities: remove export const config.schedule from workflow-sweep-background.ts so it remains a pure background function (keep its background handler like handler or background-compatible export) and create a new scheduled function (e.g., workflow-sweep-scheduler.ts) that exports export const config = { schedule: '*/5 * * * *' } and invokes the background worker (via Netlify background invocation, HTTP call, or your queue mechanism). Ensure workflow-sweep-background.ts retains only background logic and the new scheduler file contains only the schedule config and the trigger call.
🤖 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.
Duplicate comments:
In `@netlify/functions/workflow-sweep-background.ts`:
- Around line 12-14: The file workflow-sweep-background.ts is both named as a
background worker and exports config.schedule, which Netlify treats as
conflicting invocation models; fix by splitting responsibilities: remove export
const config.schedule from workflow-sweep-background.ts so it remains a pure
background function (keep its background handler like handler or
background-compatible export) and create a new scheduled function (e.g.,
workflow-sweep-scheduler.ts) that exports export const config = { schedule: '*/5
* * * *' } and invokes the background worker (via Netlify background invocation,
HTTP call, or your queue mechanism). Ensure workflow-sweep-background.ts retains
only background logic and the new scheduler file contains only the schedule
config and the trigger call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6356d64-065e-4cd3-a41b-2b27a77e013b
📒 Files selected for processing (3)
docs/internal/workflow-intent-sync.mdnetlify/functions/workflow-sweep-background.tssrc/utils/workflow-runtime.server.ts
💤 Files with no reviewable changes (1)
- src/utils/workflow-runtime.server.ts
✅ Files skipped from review due to trivial changes (1)
- docs/internal/workflow-intent-sync.md
tannerlinsley
changed the title
1 participant
Summary
Validation
pnpm testnode --import tsx --test tests/intent-workflow.test.tspnpm exec dotenv -e /Users/tannerlinsley/GitHub/tanstack.com/.env.local -- sh -c 'exec node node_modules/.pnpm/cross-env@7.0.3/node_modules/cross-env/src/bin/cross-env.js INTENT_WORKFLOW_DB_TESTS=1 node --import tsx --test tests/intent-workflow.test.ts'pnpm workflow list-workflowsNotes
workspace:*internal deps, so.pnpmfile.cjsrewrites those manifests until packages are republished.drizzle/migrationsis normally ignored; this PR force-adds the workflow runtime migration.Summary by CodeRabbit
New Features
Refactor
Tests
Chores