Skip to content

refactor: split run creation into composable record and dispatch phases#7168

Merged
lancy merged 1 commit intomainfrom
feature/issue-7164-split-create-run
Mar 31, 2026
Merged

refactor: split run creation into composable record and dispatch phases#7168
lancy merged 1 commit intomainfrom
feature/issue-7164-split-create-run

Conversation

@lancy
Copy link
Copy Markdown
Contributor

@lancy lancy commented Mar 30, 2026

Summary

  • Extract createRunRecord() from the first half of createRun() (compose loading, authorization, validation, DB transaction)
  • Export buildAndDispatchRun() which was previously a private function
  • Rewrite createRun() as a thin composition of the two with identical behavior
  • Add new exports to index.ts: createRunRecord, buildAndDispatchRun, CreateRunRecordResult

This enables zero-run-service to insert Zero-specific logic (token generation, secret resolution) between record creation and dispatch in future iterations.

Closes #7164
Parent Epic: #7152

Test plan

  • All 9 existing test files pass unchanged (136/136 tests)
  • Lint clean (0 warnings, 0 errors)
  • Type-check clean (no errors in changed files)
  • Knip clean (no unused exports)
  • Prettier formatted
  • CI pipeline passes

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
turbo/apps/web/src/lib/run/run-service.ts 95.83% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 30, 2026

Code Review: PR #7168 (Round 1) — LGTM 🎉

All P0 and P1 issues have been resolved.

Summary

This is a clean, well-executed mechanical refactor that splits createRun() into two composable phases:

  • createRunRecord() — handles steps 1-5 (compose loading, authorization, validation, DB transaction)
  • buildAndDispatchRun() — handles steps 6-10 (callbacks, token generation, context building, dispatch)
  • createRun() — rewritten as a thin composition of the two with identical behavior

Key observations:

  • Behavioral equivalence verified — the refactored createRun() produces identical results to the original
  • The enqueueRun(params) call correctly drops the redundant { ...params, orgSlug, orgId } spread since CreateRunParams already carries both fields
  • JSDoc correction from "per-user advisory lock" to "per-org advisory lock" is accurate (SQL uses hashtext(${orgId}))
  • CreateRunRecordResult interface exposes only the fields needed by downstream buildAndDispatchRun(), with run narrowed to { id, createdAt }
  • No any types, lint suppressions, defensive patterns, or dead code detected
  • All 136 existing tests pass unchanged (expected for a behavioral-preserving refactor)

Verdict: LGTM ✅

No critical or high-priority issues remaining. This PR is ready for merge.


Completed after 1 round(s) of automated review-fix loop

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 30, 2026

⚡ Lighthouse — Web

Category Score
🟠 Performance 69
🟢 Accessibility 96
🟠 Best Practices 79
🟢 SEO 92

Tested URL: https://pr-7168-www.vm6.ai/ · Full report

@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 30, 2026

Acceptance Report: #7164 — Split createRun into composable phases

Epic: #7152 — Extract Zero platform layer from generic run infrastructure
PR: #7168
Reviewer: Tech Lead (via /epic-review)


1. Requirements (Definition of Done)

# Requirement Status Notes
1 createRunRecord() exported, contains compose loading + auth + transaction ✅ PASS Clean extraction with CreateRunRecordResult interface
2 buildAndDispatchRun() exported ✅ PASS Changed from private to export
3 createRun() is composition of the two ✅ PASS Thin wrapper: try createRunRecord → catch ConcurrentRunLimit → enqueueRun, then buildAndDispatchRun
4 All existing tests pass without modification ✅ PASS 136 tests pass per agent report
5 No new exports unused (knip clean) ✅ PASS

2. Code Review

Dimension Status Findings
Correctness enqueueRun(params) is safe — startRun() always provides orgSlug/orgId in params. ConcurrentRunLimit handling preserved.
Project Standards Clean TypeScript, proper interface, good JSDoc
Consistency Follows existing patterns

3. Merge Conflict Note

This PR branches from main which still has the zeroRuns INSERT inside the transaction. PR #7162 removes it. Whichever merges second will need a rebase — the conflict is straightforward (just delete the zeroRuns lines from createRunRecord if #7162 merges first).

Verdict

ACCEPTED

Clean structural refactoring. createRunRecord() + buildAndDispatchRun() are now composable — this is the foundation for the next iterations (zero token extraction, secret resolution migration). No behavior change.

@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Mar 30, 2026
@lancy lancy force-pushed the feature/issue-7164-split-create-run branch 2 times, most recently from 8770d4f to e564aec Compare March 30, 2026 06:58
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy force-pushed the feature/issue-7164-split-create-run branch 3 times, most recently from 5e47ea9 to 14770a7 Compare March 30, 2026 10:37
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@e7h4n e7h4n removed this pull request from the merge queue due to a manual request Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
@lancy lancy removed this pull request from the merge queue due to a manual request Mar 30, 2026
@lancy lancy added this pull request to the merge queue Mar 30, 2026
Extract the record-creation phase (compose loading, authorization, validation,
DB transaction) into a standalone createRunRecord() function and export
buildAndDispatchRun() so that zero-run-service can insert Zero-specific logic
between record creation and dispatch in future iterations.

createRun() is now a thin composition of the two with identical behavior.

Closes #7164

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@e7h4n e7h4n removed this pull request from the merge queue due to a manual request Mar 30, 2026
@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 31, 2026

Acceptance Report: #7164 — Split createRun into composable phases

Epic: #7152 — Extract Zero Platform Layer from Generic Run Infrastructure
PR: #7168
Reviewer: Tech Lead (via /epic-review)


1. Requirements (Definition of Done)

# Requirement Status Notes
1 createRunRecord() is exported and contains compose loading + authorization + DB transaction Lines 1131-1207. Contains steps 1-5: loadCompose, authorize, validate, advisory lock + checks + INSERT
2 buildAndDispatchRun() is exported Line 726: changed from async function to export async function
3 createRun() is a composition of the two, with identical behavior Lines 1209-1240. Catches ConcurrentRunLimitErrorenqueueRun(), otherwise delegates to both functions
4 All existing tests pass without modification No test files changed. All CI test suites pass (test-web 1-4, test-cli, test-app, test-other)
5 Lint, type-check, format pass lint-eslint, lint-types, lint-format, lint-knip all pass
6 No new exports are unused (knip clean) createRunRecord, buildAndDispatchRun, CreateRunRecordResult exported via index.ts, knip passes

2. Code Review

Dimension Status Findings
Correctness Pure structural refactoring. createRunRecord returns all metadata needed by buildAndDispatchRun. createRun composes them identically to the original monolith. enqueueRun(params) is safe — orgId and orgSlug are already set by startRun() caller.
Project Standards No any types, no eslint-disable, no defensive try-catch. Clean JSDoc on all three functions.
Consistency Follows existing patterns. CreateRunRecordResult interface is well-structured.
Security No changes to authorization flow — canAccessCompose check preserved in createRunRecord.
Performance No additional DB queries or overhead. Same transaction structure.

3. Architectural Alignment

Check Status Notes
Enables future zero-run-service insertion createRunRecord() returns runId, composeContent — zero-run-service can call this, inject token/secrets, then call buildAndDispatchRun()
ConcurrentRunLimitError handling correct Propagated from createRunRecord, caught by createRun for enqueue fallback. Future: zero-run-service can catch it instead.
dispatchQueuedRun unaffected Separate function, already called buildAndDispatchRun internally. Not changed by this PR.
startRun path unchanged startRun()createRun() — composition is transparent to callers

4. Independence

Check Status
Compiles standalone
No forward dependencies
All CI checks pass ✅ (all green, including rebased on latest main)

5. Issues Found

Must Fix

None.

Should Fix

None.

Nits

  1. Comment at line 724 still references dispatchQueuedRun ("Used by both createRun and dispatchQueuedRun") — technically now used by createRun which calls buildAndDispatchRun, and dispatchQueuedRun calls it directly. Minor, non-blocking.

Verdict

ACCEPTED

Clean structural refactoring that splits createRun() into createRunRecord() + buildAndDispatchRun() without any behavior change. All 6 Definition of Done items pass. All CI checks green. Enables the next iteration (#7152 — move injectZeroToken to zero-run-service).

@lancy lancy added this pull request to the merge queue Mar 31, 2026
@lancy lancy removed this pull request from the merge queue due to a manual request Mar 31, 2026
@lancy lancy force-pushed the feature/issue-7164-split-create-run branch from 14770a7 to 484751f Compare March 31, 2026 02:25
@lancy lancy added this pull request to the merge queue Mar 31, 2026
Merged via the queue into main with commit 022a53e Mar 31, 2026
57 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in VM0 Kanban Mar 31, 2026
@github-actions github-actions Bot mentioned this pull request Mar 31, 2026
@github-actions github-actions Bot deleted the feature/issue-7164-split-create-run branch March 31, 2026 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

refactor: split createRun into composable phases (record creation + dispatch)

1 participant