Repository navigation
Conversation
Biome flags an illegal return statement outside of a function in
workflows/df12-build.js: the worker-pool orchestration ran as flat
module-scope code terminating in a bare return {...}. The repository's
own workflow-parse gate wraps the body in a synthetic async function so
it tolerates the top-level return, but Biome in CI rightly rejects it.
Enclose the pool state, control loop, end-of-run flush, and result
assembly in a single async function workflowMain(), so its return {...}
is legal, and drive it from module scope with a bare await
workflowMain() (a valid top-level await in this ESM module that already
exports meta). The configuration, schemas, prompt builders, and the
runTask/runAudit/runTriage helpers stay at module scope and are reached
through closure, so they remain reusable.
The result contract is unchanged: all nine fields (base, maxParallel,
processed, results, audits, remediationTriage, pendingProposals,
halted, summary) and their semantics are preserved byte-for-byte; the
only edits are the enclosing function, its indentation, and the
trailing invocation.
Closes #6
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @pandalump, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
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:
Summary
WalkthroughEncapsulate the existing worker-pool orchestration in ChangesWorkflow entrypoint
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (17 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b24480e8c
ℹ️ 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".
| (halted ? ` | halted: ${halted}` : ' | clean stop (no more unblocked tasks).'), | ||
| } | ||
|
|
||
| await workflowMain() |
There was a problem hiding this comment.
When the Claude Code launcher executes this body inside its async wrapper, this expression awaits workflowMain() but discards its returned object, so every completed run resolves to undefined instead of exposing processed, results, halted, summary, and the other documented result fields. This is reproducible even with an empty mocked frontier using the wrapper mirrored by make workflow-parse; use return await workflowMain() so the result propagates to the caller.
AGENTS.md reference: AGENTS.md:L6-L8
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@workflows/df12-build.js`:
- Around line 709-717: Move the self-contained mutex factory out of
workflowMain() to module scope alongside the other helper functions, preserving
its promise-chain serialization and queue recovery behavior. Leave
workflowMain() responsible only for creating mergeLock by calling mutex().
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a115cc73-9513-4681-9ea4-6298ac73f8cb
📒 Files selected for processing (1)
workflows/df12-build.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/nixie(auto-detected)
| function mutex() { | ||
| let tail = Promise.resolve() | ||
| return (fn) => { | ||
| const result = tail.then(() => fn()) | ||
| tail = result.then(() => {}, () => {}) // keep the queue alive regardless of outcome | ||
| return result | ||
| } | ||
| } | ||
| } | ||
| const mergeLock = mutex() | ||
|
|
||
| let selectSeq = 0 | ||
| async function doSelect(excludeIds) { | ||
| phase('Select') | ||
| return await agent(selectPrompt(excludeIds), { phase: 'Select', label: `select#${++selectSeq}`, schema: SELECTION_SCHEMA }) | ||
| } | ||
| const mergeLock = mutex() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Move mutex() to module scope.
mutex() closes over nothing inside workflowMain(). It is a self-contained factory. Nesting it adds depth to an already large entrypoint and hides its reusability. Keep workflowMain() focused on orchestration state.
♻️ Proposed refactor
Declare the factory at module scope, next to the other helpers:
// Minimal async mutex: serialize callers through a promise chain. Used as a
// merge queue so only one task rebases + squash-merges + pushes BASE at a time.
function mutex() {
let tail = Promise.resolve()
return (fn) => {
const result = tail.then(() => fn())
tail = result.then(() => {}, () => {}) // keep the queue alive regardless of outcome
return result
}
}Then reduce the entrypoint to the call:
- // Minimal async mutex: serialize callers through a promise chain. Used as a
- // merge queue so only one task rebases + squash-merges + pushes BASE at a time.
- function mutex() {
- let tail = Promise.resolve()
- return (fn) => {
- const result = tail.then(() => fn())
- tail = result.then(() => {}, () => {}) // keep the queue alive regardless of outcome
- return result
- }
- }
const mergeLock = mutex()As per coding guidelines: "prefer clarity over cleverness, small single-responsibility functions".
🤖 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 `@workflows/df12-build.js` around lines 709 - 717, Move the self-contained
mutex factory out of workflowMain() to module scope alongside the other helper
functions, preserving its promise-chain serialization and queue recovery
behavior. Leave workflowMain() responsible only for creating mergeLock by
calling mutex().
Source: Coding guidelines
2 participants
Summary
Fixes the Biome
Illegal return statement outside of a functiondiagnostic inthe hand-maintained Claude Code workflow
workflows/df12-build.js.The worker-pool orchestration previously ran as flat module-scope code
terminating in a bare
return {…}at module scope. The repository's ownmake workflow-parsegate wraps the body in a synthetic async function so ittolerates the top-level
return, but Biome (CI) rightly rejects it.This change encloses the pool state, control loop, end-of-run flush, and result
assembly in a single
async function workflowMain()(whosereturn {…}is nowlegal) and drives it from module scope with a bare
await workflowMain()— avalid top-level
awaitin this ESM module (it already exportsmeta), and legalunder the launcher's async-function wrap. The configuration constants, schemas,
prompt builders, and the
runTask/runAudit/runTriagehelpers stay at modulescope and are reached through closure.
The result contract is preserved byte-for-byte: all nine fields (
base,maxParallel,processed,results,audits,remediationTriage,pendingProposals,halted,summary) and their semantics are unchanged. Awhitespace-ignoring diff (
git diff -w) shows only the enclosing function andthe trailing invocation were added.
This is the Claude Code workflow only; the generated ODW sibling
(
workflows/df12-build-odw.js) legitimately ends withreturn await workflowMain()and is unaffected.Validation
make workflow-parse:workflows/df12-build.js: wrapped JavaScript parses.returnstatement remains at column 0 anywhere in the file.Closes #6