feat(workflow): add fn_workflow_step_resume operator escape hatch for stuck pending merge-review steps - #3339
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThis change adds the operator-only ChangesWorkflow step resume
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant fn_workflow_step_resume
participant TaskStore
participant RunAudit
Operator->>fn_workflow_step_resume: Provide task ID, step ID, and reason
fn_workflow_step_resume->>TaskStore: Call resumeWorkflowStep
TaskStore->>TaskStore: Validate and mark pending step failed
TaskStore->>RunAudit: Record task:resume-step
TaskStore-->>fn_workflow_step_resume: Return updated task or error
fn_workflow_step_resume-->>Operator: Return structured result
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/core/src/merge/task-merge.ts (1)
576-591: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove
findPendingPreMergeStepif no production call site exists.
findPendingPreMergeStep,resumeWorkflowStep, andfn_workflow_step_resumeare documented as operator-only and not exposed to executor/reviewer/triage agent surfaces, but there is no active production call site for this helper. If the helper is only tested as a unit, remove it frompackages/core/src/index.tsto avoid exposing it as part of the public core API.🤖 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/core/src/merge/task-merge.ts` around lines 576 - 591, Remove findPendingPreMergeStep and its export from packages/core/src/index.ts if it has no active production callers, retaining only necessary test references. Do not alter resumeWorkflowStep or fn_workflow_step_resume, which are unrelated context symbols.
🤖 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 `@packages/cli/src/extension.ts`:
- Around line 2580-2632: The withheld-tool guard is missing from
fn_workflow_step_resume, allowing agent sessions to invoke this operator-only
tool. Add "fn_workflow_step_resume" to WITHHELD_FROM_AGENT_EXTENSION_TOOLS, call
denyWithheldToolForAgentPrincipal first in its execute method following
fn_task_bypass_review, and prefix the surrounding comment with the required
FNXC:<Area> <timestamp>: format.
In `@packages/core/src/store.ts`:
- Around line 1723-1727: Update resumeWorkflowStep to resolve the task’s
workflow IR and derive the review lane via
resolveReviewColumns/declaresAnyLifecycleTrait and the in-progress lane via
resolveNodeOverrideLanes or columnsWithFlag(ir, "countsTowardWip"), then
validate task.column against those resolved lanes instead of hardcoded literals.
Preserve the existing error behavior when the task matches neither resolved
lane.
- Around line 1746-1754: Update the resumed record construction in
resumeWorkflowStep to explicitly remove leaseOwner and leaseNodeId after
spreading target, alongside the existing terminal-state cleanup. Preserve the
resulting failed record’s other fields while ensuring no lease fields remain
when a pending step is resumed.
- Around line 1732-1743: The resumeWorkflowStep validation currently permits
pending post-merge results. Update the target lookup or validation in
resumeWorkflowStep so only pending pre-merge entries are accepted, while
preserving the existing errors for missing or non-pending workflow steps.
---
Nitpick comments:
In `@packages/core/src/merge/task-merge.ts`:
- Around line 576-591: Remove findPendingPreMergeStep and its export from
packages/core/src/index.ts if it has no active production callers, retaining
only necessary test references. Do not alter resumeWorkflowStep or
fn_workflow_step_resume, which are unrelated context symbols.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a4c9e9aa-a7ff-4755-849f-16abddeeeec5
📒 Files selected for processing (10)
.changeset/stas-032-resume-workflow-step.mdpackages/cli/src/__tests__/extension.test.tspackages/cli/src/extension.tspackages/core/src/__tests__/store-resume-step.test.tspackages/core/src/__tests__/task-merge-bypass.test.tspackages/core/src/index.tspackages/core/src/merge/task-merge.tspackages/core/src/store.tspackages/core/src/types/workflow/workflow-steps.tspackages/engine/src/util/run-audit.ts
…wStep operator escape hatch Operator-only CLI/pi-extension escape hatch for in-review/in-progress tasks with a workflow step permanently stuck in 'pending' status (Runfusion/ Fusion#1946 dispatched verdict callback never received). Transitions the stuck pending step to 'failed' with resume audit metadata so the existing fn_task_bypass_review escape hatch can clear the merge blocker. - Add resume audit fields to WorkflowStepResult (resumedBy/resumedAt/ resumeReason/resumedFromStatus) - Add findPendingPreMergeStep helper to summarize the stuck-pending state - Add TaskStore.resumeWorkflowStep primitive (eligibility-gated, lock-held, run-audit-logged via 'task:resume-step') - Register fn_workflow_step_resume operator tool on the CLI/pi surface only - Add engine 'task:resume-step' run-audit mutation type - Add PG-backed store-resume-step tests + findPendingPreMergeStep unit tests
- Withhold fn_workflow_step_resume from agent sessions (WITHHELD_FROM_AGENT_EXTENSION_TOOLS + denyWithheldToolForAgentPrincipal guard in execute), FNXC comment on registration. - Resolve review/WIP lanes from the task's workflow IR (resolveWorkflowIrForTask + resolveReviewColumns / declaresAnyLifecycleTrait + countsTowardWip) instead of hardcoded 'in-review'/'in-progress' literals. - Restrict resumable steps to pending PRE-MERGE steps only; findPendingPreMergeStep now has a production caller (named in the not-found error). - Clear leaseOwner/leaseNodeId on the terminal 'failed' resumed result. - Tests: post-merge rejection, lease cleanup, updated lane/not-found error regexes.
bad3e23 to
67ab7b3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Summary
Adds an operator-only escape hatch for a card stranded
in-review(orin-progress) with a workflow step permanently stuck inpendingstatus — the leading real-world cause being a dispatched prompt node (e.g.code-review) whose verdict callback was never received (see #1946). Transitions the stuckpendingpre-merge step tostatus: "failed"with resume audit metadata, so the existingfn_task_bypass_reviewescape hatch can then clear the merge blocker.What changed
WorkflowStepResultgains resume audit fields:resumedBy,resumedAt,resumeReason,resumedFromStatus. They are pure audit trail and do not participate in merge-blocking (getTaskMergeBlocker).findPendingPreMergeStep(new helper, exported from@fusion/core) summarizes the stuck-pending pre-merge state for operator tooling. Ignores post-merge steps; returns the newest pending pre-merge result.TaskStore.resumeWorkflowStep(id, { stepId, reason, actor })— the store primitive (eligibility-gated: task must bein-review/in-progress, not paused; step must exist and bepending; a mandatory non-blankreasonandstepIdare required). Runs underwithTaskLock, writes the resume as a terminalfailedresult, appends a task-log breadcrumb, and emits the newtask:resume-steprun-audit event.fn_workflow_step_resume— new CLI/pi-extension tool registered only on the operator surface (deliberately not wired into executor/reviewer/triage agent tool lists). Accepts{ id, stepId, reason }; the actor defaults tocli-operator.task:resume-stepDatabaseMutationTypemember.Why
A prompt-node verdict callback can be lost (dispatched prompt never receives a verdict), leaving the step
pendingforever. Previously the only recourse wasfn_task_bypass_review, which requires a terminal failed pre-merge step to clear the blocker — a permanentlypendingstep could not be bypassed. This PR bridges that gap: resume (pending → failed) then bypass (failed merge-blocker cleared).Verification
@fusion/core,@fusion/engine,@runfusion/fusionall clean.task-merge-bypass.test.ts: 15/15 pass (incl. 5 newfindPendingPreMergeStepcases).store-resume-step.test.ts(new, PG-backed): 9/9 pass — eligibility gating, resume rewrite + audit fields, run-audit event, non-pending/non-found/blank-argument rejection, in-progress column support, property preservation.extension.test.ts: 75/75 pass (expected-tool registration includes the new tool).Files
packages/core/src/types/workflow/workflow-steps.tspackages/core/src/merge/task-merge.tspackages/core/src/store.tspackages/core/src/index.tspackages/core/src/__tests__/store-resume-step.test.ts(new)packages/core/src/__tests__/task-merge-bypass.test.tspackages/engine/src/util/run-audit.tspackages/cli/src/extension.tspackages/cli/src/__tests__/extension.test.ts.changeset/stas-032-resume-workflow-step.md(minor, feature)Summary by CodeRabbit