feat: dynamic-workflow 来了

[claude-code-best]

Summary by CodeRabbit

Release Notes

• New Features

  • Added a real-time /workflows monitoring panel with phase/agent tracking and keyboard controls.
  • Introduced an interactive /effort selection panel with animated ripple effects.
  • Added /ultracode skill for workflow orchestration guidance.
  • Enabled deterministic JavaScript workflow scripting with persistence across restarts.

• Improvements

  • Redefined workflow orchestration behavior with improved agent routing, concurrency limits, and run/phase state handling.

• Changes

  • Removed an extra reminder appended by the file reader tool.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
ccb-863780bf	🟢 Ready	View Preview	Jun 14, 2026, 8:04 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a standalone workflow engine package, rewires workflow tool/command integration to use it, and adds workflow progress, persistence, panel, and documentation updates. It also adds an EffortPanel UI with ripple rendering, plus keybinding and command changes around /effort.

Changes

Workflow engine and workflow UI migration

Layer / File(s)	Summary
Engine contracts and package surface packages/workflow-engine/src/{types,ports,agentAdapter,constants,index}.ts, packages/workflow-engine/src/tool/{schema,constants}.ts, packages/workflow-engine/package.json, packages/builtin-tools/src/index.ts, packages/workflow-engine/src/__tests__/*	Adds the workflow-engine package API, ports, types, constants, adapter registry, schema/export surface, and migration re-exports with contract tests.
Execution runtime and engine helpers packages/workflow-engine/src/engine/*, packages/workflow-engine/src/__tests__/*, packages/workflow-engine/examples/registry-demo.ts	Implements deterministic script parsing, journaling, concurrency, budgets, hooks, named workflow loading, and runWorkflow, with runtime and resume tests.
Workflow tool entrypoints and examples packages/workflow-engine/src/tool/{WorkflowTool,persistInline}.ts, src/workflow/namedWorkflowCommands.ts, src/commands.ts, packages/workflow-engine/examples/{smoke.ts,research-report/*}, packages/workflow-engine/src/__tests__/WorkflowTool.test.ts	Adds the new workflow tool, inline script persistence, named workflow command discovery, and runnable examples for direct workflow execution.
Host wiring, backend, and service src/workflow/{hostHandle,notifications,...}, src/workflow/backends/claudeCodeBackend.ts, src/tasks/LocalWorkflowTask/*, src/tools.ts, src/constants/tools.ts, src/main.tsx, src/components/permissions/PermissionRequest.tsx, src/workflow/__tests__/*	Integrates the engine into the host runtime through the Claude backend, workflow service behavior, notifications, task error state, and updated workflow imports.
/workflows panel and persistence-related state src/workflow/panel/*, src/commands/workflows/index.ts, src/components/tasks/BackgroundTasksDialog.tsx, src/workflow/__tests__/*	Adds the local-JSX workflows panel, selector and rendering helpers, progress-store and persistence-oriented tests, and removes the old workflow detail dialog path.
Workflow docs and plans docs/features/workflow-scripts.md, docs/superpowers/{specs,plans,reviews}/...workflow...md, biome.json	Rewrites workflow documentation around the new engine and panel architecture, adds workflow design/plan/review documents, and ignores workflow script paths in Biome VCS includes.

Effort panel UI

Layer / File(s)	Summary
Panel state and keybindings src/components/EffortPanel/effortPanelState.ts, src/keybindings/{schema,defaultBindings}.ts, src/utils/effort.ts, src/components/EffortPanel/__tests__/effortPanelState.test.ts	Adds EffortPanel cursor and confirm-state helpers, new keybinding actions/defaults, and clarifies that ultracode is not an effort value.
Panel rendering and animation src/components/EffortPanel/{EffortPanel,rippleAnimation,useRippleFrame}.ts*, src/components/EffortPanel/__tests__/*	Adds the EffortPanel component, ripple animation helpers, animation timing hook, and tests for rendering and deterministic ripple utilities.
/effort command integration and docs src/commands/effort/effort.tsx, docs/superpowers/{plans,specs}/2026-06-14-effort-panel*.md	Mounts the panel for no-argument /effort usage and adds implementation/design documents for the effort panel behavior.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐇 I found a workflow burrow bright,
with phases hopping left to right.
An effort slider starts to gleam,
while ripples color rabbit dreams.
I tap, resume, and softly cheer—
new tunnels bloom from code to here.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/workflow-squashed

[coderabbitai]

Actionable comments posted: 14

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/keybindings/schema.ts (1)

12-32: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing 'EffortPanel' context in KEYBINDING_CONTEXTS.

The defaultBindings.ts file (line 331) defines keybindings for the 'EffortPanel' context, but this context is not included in the KEYBINDING_CONTEXTS enum array. This will cause validation failures when the keybinding schema is applied.

🔧 Proposed fix

 export const KEYBINDING_CONTEXTS = [
   'Global',
   'Chat',
   'Autocomplete',
   'Confirmation',
   'Help',
   'Transcript',
   'HistorySearch',
   'Task',
   'ThemePicker',
   'Settings',
   'Tabs',
   // New contexts for keybindings migration
   'Attachments',
   'Footer',
   'MessageSelector',
   'DiffDialog',
   'ModelPicker',
   'Select',
   'Plugin',
+  'EffortPanel',
 ] as const

🤖 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/keybindings/schema.ts` around lines 12 - 32, The KEYBINDING_CONTEXTS
array in schema.ts is missing the 'EffortPanel' context that is referenced in
defaultBindings.ts, which will cause validation failures. Add 'EffortPanel' to
the KEYBINDING_CONTEXTS array as a new entry within the list of context strings,
placing it in an appropriate position alongside the other context names like
'Attachments', 'Footer', 'MessageSelector', etc.

src/components/EffortPanel/__tests__/effortPanelState.test.ts (1)

1-164: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Test descriptions must be in English.

The test logic is comprehensive and correct, but all describe() and test() strings use Chinese. According to coding guidelines, test descriptions must be in English.

As per coding guidelines, files matching **/*.test.ts must use describe() + test() pattern with English descriptions.

📝 Example English translations

-  test('PANEL_POSITIONS 顺序为 low → ultracode', () => {
+  test('PANEL_POSITIONS order is low → ultracode', () => {
     expect(PANEL_POSITIONS).toEqual([
       'low',
       'medium',
       'high',
       'xhigh',
       'max',
       'ultracode',
     ])
   })

-  test('moveLeft 在 low 处保持 low', () => {
+  test('moveLeft clamps at low', () => {
     expect(moveLeft('low')).toBe('low')
   })

-  test('moveLeft 正常左移', () => {
+  test('moveLeft moves left normally', () => {
     expect(moveLeft('high')).toBe('medium')
     expect(moveLeft('ultracode')).toBe('max')
   })

Apply similar translations to all test descriptions throughout the file.

🤖 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/components/EffortPanel/__tests__/effortPanelState.test.ts` around lines 1
- 164, All test descriptions in the file are currently written in Chinese, but
per coding guidelines, test descriptions must be in English. Translate all
Chinese strings in the describe() and test() function calls to English
throughout the entire file. This includes the two describe blocks
('effortPanelState' and 'computeConfirmOutcome') and all individual test() calls
that currently have Chinese descriptions like 'PANEL_POSITIONS 顺序为 low →
ultracode', 'moveLeft 在 low 处保持 low', etc. Replace each with clear, concise
English descriptions that convey the same test intent and behavior being
validated.

Source: Coding guidelines

packages/workflow-engine/src/__tests__/concurrency.test.ts (1)

9-120: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Group these test cases under a describe(...) suite.

The file uses only top-level test(...); please wrap related cases in describe('Semaphore') / describe('clampMaxConcurrency') blocks to match the repository test structure convention.

As per coding guidelines, **/*.test.ts: “use describe() + test() pattern with English descriptions”.

🤖 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/workflow-engine/src/__tests__/concurrency.test.ts` around lines 9 -
120, Group the individual test cases into describe blocks to follow the
repository convention. Wrap the Semaphore-related tests (the tests for acquire
with concurrency limits, FIFO ordering, and abort signal handling) into a
describe('Semaphore') block, and wrap the clampMaxConcurrency test along with
the maxConcurrency test into a describe('clampMaxConcurrency') block. This
organizes the test file structure and makes it consistent with the repository's
testing guidelines that require using the describe() and test() pattern with
English descriptions.

Source: Coding guidelines

🟡 Minor comments (17)

src/components/EffortPanel/__tests__/EffortPanel.test.tsx-14-24 (1)

14-24: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use describe() + English behavior names for this test file.

Line 14 and Line 18 are standalone test() cases with non-English descriptions. Wrap them under describe('EffortPanel') and use English behavior text to match repository test conventions.

As per coding guidelines: “Test file naming convention: use describe('functionName') with test('behavior description') in English.”

🤖 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/components/EffortPanel/__tests__/EffortPanel.test.tsx` around lines 14 -
24, The test file has standalone test cases with non-English (Chinese)
descriptions that need to be organized according to repository conventions. Wrap
both test cases (the ones checking if EffortPanel is a valid function and the
one checking if it accepts props correctly) inside a describe('EffortPanel')
block, and replace the Chinese test descriptions with English behavior
descriptions. For example, change 'EffortPanel 是有效 React 组件' to 'should be a
valid React component' and 'EffortPanel 接受 props 并返回 React element（不挂载）' to
'should accept props and return a valid React element without mounting' or
similar English descriptions that clearly describe the behavior being tested.

Source: Coding guidelines

docs/superpowers/plans/2026-06-14-effort-panel-basic.md-535-537 (1)

535-537: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix MD028: remove the blank line inside this blockquote.

Line 536 breaks the blockquote flow between Line 535 and Line 537. Keep the blockquote contiguous (or use > on the separator line) so markdownlint passes.

🤖 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 `@docs/superpowers/plans/2026-06-14-effort-panel-basic.md` around lines 535 -
537, The blockquote spanning the warning symbol (⚠️) text and the Step 3.3 text
is broken by a blank line in the middle that lacks the `>` prefix required for
blockquote continuation. Remove the blank line between the two blockquote
segments, or alternatively add a `>` marker on the blank line to maintain
blockquote contiguity and satisfy markdownlint MD028 requirements.

Source: Linters/SAST tools

src/components/EffortPanel/__tests__/rippleAnimation.test.ts-17-501 (1)

17-501: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Rename test behavior descriptions to English throughout this suite.

Most test() descriptions in this file are not in English (starting at Line 17 onward). Please convert behavior strings to English to align with the test convention.

As per coding guidelines: “Test file naming convention: use describe('functionName') with test('behavior description') in English.”

🤖 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/components/EffortPanel/__tests__/rippleAnimation.test.ts` around lines 17
- 501, Convert all test descriptions from Chinese/mixed language to English
throughout the entire test file. This includes all test cases in the describe
blocks for intensityToColor, rotateHue, getHueShiftAtTime, computeRippleCells,
applyOverlaysToCells, cellsToSegments, fadeColor, and fadeCells. Replace Chinese
text and arrows (→) with clear English descriptions that accurately describe the
test behavior and expected outcomes. Ensure each test string passed to the
test() function is a complete English sentence describing what is being tested.

Source: Coding guidelines

packages/workflow-engine/src/engine/namedWorkflows.ts-42-45 (1)

42-45: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Deduplicate workflow names across extensions in listing output.

If both foo.ts and foo.js exist, this currently returns foo twice, which can create duplicate command/list entries.

🐛 Proposed fix

-  return files
-    .filter(f => isScriptExt(parse(f).ext))
-    .map(f => parse(f).name)
-    .sort()
+  return [...new Set(
+    files
+      .filter(f => isScriptExt(parse(f).ext))
+      .map(f => parse(f).name),
+  )].sort()

🤖 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/workflow-engine/src/engine/namedWorkflows.ts` around lines 42 - 45,
The workflow listing in namedWorkflows.ts returns duplicate names when multiple
files with the same name but different extensions exist (e.g., both foo.ts and
foo.js). After mapping file names with parse(f).name and before sorting, add a
deduplication step to ensure each workflow name appears only once in the
returned array. This can be accomplished by converting the array to a Set to
eliminate duplicates, then converting back to a sorted array, or by filtering to
keep only the first occurrence of each name using indexOf.

packages/workflow-engine/src/__tests__/ports.test.ts-17-49 (1)

17-49: ⚠️ Potential issue | 🟡 Minor

Add satisfies WorkflowPorts to enforce the contract at compile time, and fix the runAgentToResult signature

The ports object claims to validate the WorkflowPorts contract (per the comment), but without a satisfies annotation, TypeScript does not enforce it. The current noop function has signature (): void but AgentRunner.runAgentToResult requires (params: AgentRunParams, host: HostHandle): Promise<AgentRunResult>. This mismatch goes undetected.

Suggested fix

-import { createHostHandle, isHostHandle, unwrapHostHandle } from '../ports.js'
+import {
+  createHostHandle,
+  isHostHandle,
+  unwrapHostHandle,
+  type WorkflowPorts,
+} from '../ports.js'
@@
 test('ports object satisfies the minimal shape', () => {
   // compile-time shape validation: the assignment below passing means the ports contract is self-consistent
   const noop = (): void => {}
   const ports = {
     agentRunner: {
-      runAgentToResult: noop,
+      runAgentToResult: async () => ({
+        kind: 'ok',
+        output: 'x',
+        usage: { outputTokens: 1 },
+      }),
     },
     progressEmitter: { emit: noop },
     taskRegistrar: {
       register: () => ({
         runId: 'run-1',
         signal: new AbortController().signal,
       }),
       complete: noop,
       fail: noop,
       kill: noop,
       pendingAction: () => null,
     },
     journalStore: {
       read: async () => [],
       append: async () => {},
       truncate: async () => {},
     },
     permissionGate: { isAborted: () => false },
     logger: { debug: noop, event: noop },
     hostFactory: () => ({
       handle: createHostHandle(null),
       cwd: '/tmp',
       budgetTotal: null,
       toolUseId: 'tu-1',
     }),
-  }
+  } satisfies WorkflowPorts

🤖 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/workflow-engine/src/__tests__/ports.test.ts` around lines 17 - 49,
The ports object in the test lacks compile-time type validation even though the
comment indicates it should validate the WorkflowPorts contract. Add a
`satisfies WorkflowPorts` type annotation to the ports object declaration to
enforce the contract at compile time. Additionally, the agentRunner mock has an
incorrect signature for runAgentToResult—replace the generic noop function with
a proper mock that matches the expected signature (accepting params and host
arguments and returning a Promise). This will catch signature mismatches like
this at compile time rather than allowing them to pass silently.

src/workflow/panel/PhaseSidebar.tsx-50-50 (1)

50-50: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use a unique row key; row.title can collide.

If multiple phases share the same title, React reconciliation can misrender updates.

Suggested fix

-          <Box key={row.title} backgroundColor={highlighted ? 'selectionBg' : undefined} justifyContent="space-between">
+          <Box key={`${row.title}-${i}`} backgroundColor={highlighted ? 'selectionBg' : undefined} justifyContent="space-between">

🤖 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/workflow/panel/PhaseSidebar.tsx` at line 50, The Box component in
PhaseSidebar.tsx uses row.title as its React key, which can collide if multiple
phases share the same title, causing incorrect reconciliation and rendering.
Replace the key prop on the Box element with a unique identifier from the row
object (such as row.id or another unique field) to ensure each row is distinctly
identified by React and prevent misrendering when updates occur.

packages/workflow-engine/src/__tests__/index.test.ts-4-89 (1)

4-89: ⚠️ Potential issue | 🟡 Minor

Both test suites must wrap tests under describe() blocks per coding guidelines.

• packages/workflow-engine/src/__tests__/index.test.ts: wrap the 7 export tests under describe('index exports', ...).
• src/workflow/__tests__/service.test.ts: wrap the 19 service tests under describe('workflow service', ...).

The guideline requires: describe() + test() pattern with English descriptions for all **/*.test.ts files.

🤖 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/workflow-engine/src/__tests__/index.test.ts` around lines 4 - 89,
The test files do not follow the required describe()/test() pattern per coding
guidelines. In packages/workflow-engine/src/__tests__/index.test.ts (lines
4-89), wrap all 7 individual test() calls (covering engine core API exports,
ports/host API exports, persistence/structured output/named workflow/progress
API exports, concurrency/budget/error classes exports, tool descriptor and input
schema exports, engine constant values stability, and createWorkflowTool
descriptor shape) under a single describe('index exports', ...) block. In
src/workflow/__tests__/service.test.ts (lines 183-594), wrap all 19 service
test() calls under a single describe('workflow service', ...) block. Both files
must follow the describe() + test() pattern with English descriptions as
required by the coding guidelines.

Source: Coding guidelines

docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md-75-91 (1)

75-91: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language identifiers to fenced Markdown blocks (MD040).
Markdown lint warnings indicate unlabeled fenced code blocks.

• docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md#L75-L91: change fence to a labeled one (e.g., ```text).
• docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md#L95-L102: change fence to a labeled one (e.g., ```text).
• docs/superpowers/specs/2026-06-13-workflow-run-state-persistence-design.md#L41-L49: add language label (e.g., ```text).
• docs/superpowers/specs/2026-06-13-workflow-run-state-persistence-design.md#L57-L63: add language label (e.g., ```text).
• docs/superpowers/specs/2026-06-13-workflow-run-state-persistence-design.md#L68-L72: add language label (e.g., ```text).

🤖 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 `@docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md` around lines 75
- 91, Several fenced code blocks in the Markdown files are missing language
identifiers, which violates the MD040 Markdown lint rule. Add language labels to
the triple backticks for unlabeled code blocks. Specifically, in
docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md at lines 75-91 and
lines 95-102, change the opening fence from triple backticks with no label to
```text or an appropriate language identifier. Similarly, in
docs/superpowers/specs/2026-06-13-workflow-run-state-persistence-design.md at
lines 41-49, lines 57-63, and lines 68-72, add a language label (e.g., ```text)
to each unlabeled fenced code block.

Source: Linters/SAST tools

docs/superpowers/specs/2026-06-13-workflow-tui-ultracode-design.md-80-80 (1)

80-80: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add fence languages for the two code blocks to satisfy markdown lint.

Line 80 and Line 195 use unlabeled fenced blocks. Add a language tag (e.g., text/ts) to satisfy MD040 and keep docs lint-clean.

Also applies to: 195-195

🤖 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 `@docs/superpowers/specs/2026-06-13-workflow-tui-ultracode-design.md` at line
80, Two markdown code fences in
docs/superpowers/specs/2026-06-13-workflow-tui-ultracode-design.md lack language
tags, violating the MD040 linter rule. At line 80, add an appropriate language
identifier (such as text or ts) after the opening triple backticks of the fenced
code block. Similarly, at line 195, add a language identifier to the opening
fence of that code block as well. This ensures both code blocks are properly
labeled and the documentation passes markdown linting checks.

Source: Linters/SAST tools

packages/workflow-engine/src/__tests__/journal.test.ts-10-113 (1)

10-113: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Test structure is inconsistent with the required describe() + test() pattern. Both files define only top-level test(...); wrap related tests in describe(...) blocks for consistent suite organization.

• packages/workflow-engine/src/__tests__/journal.test.ts#L10-L113: group tests into describe('agentCallKey') and describe('createFileJournalStore') blocks.
• packages/workflow-engine/src/__tests__/persistInline.test.ts#L8-L41: wrap persistence scenarios under describe('persistInlineScript') and keep existing English test(...) descriptions.
  As per coding guidelines, **/*.test.ts requires: “use describe() + test() pattern with English descriptions”.

🤖 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/workflow-engine/src/__tests__/journal.test.ts` around lines 10 -
113, The tests in both files violate the coding guidelines requiring describe()
+ test() pattern with English descriptions. In
packages/workflow-engine/src/__tests__/journal.test.ts (lines 10-113),
reorganize the top-level test() calls by wrapping agentCallKey-related tests
(the test functions for agentCallKey stable, agentCallKey varies with prompt,
agentCallKey ignores display-only fields, agentCallKey varies with schema,
agentCallKey varies with model, agentCallKey stable across params field order)
into a describe('agentCallKey') block, and wrap FileJournalStore-related tests
(FileJournalStore append → read preserves order truncate clears,
FileJournalStore read sorts by seq, FileJournalStore read for non-existent run)
into a describe('createFileJournalStore') block. In
packages/workflow-engine/src/__tests__/persistInline.test.ts (lines 8-41), wrap
all persistence scenario tests under a describe('persistInlineScript') block
while keeping the existing English test() descriptions unchanged.

Source: Coding guidelines

src/workflow/namedWorkflowCommands.ts-6-7 (1)

6-7: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use src/* alias imports in src/ modules.

Line 6-7 currently uses relative imports; this violates the repo import-path contract for src code.

Proposed fix

-import type { Command } from '../types/command.js'
-import { getProjectRoot } from '../bootstrap/state.js'
+import type { Command } from 'src/types/command.js'
+import { getProjectRoot } from 'src/bootstrap/state.js'

As per coding guidelines, src/**/*.{ts,tsx} requires: “Import paths must use the src/* alias (e.g., import { ... } from 'src/utils/...')”.

🤖 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/workflow/namedWorkflowCommands.ts` around lines 6 - 7, The import
statements in src/workflow/namedWorkflowCommands.ts at lines 6-7 are using
relative import paths (..) instead of the required src/* alias. Replace the
relative imports for Command from ../types/command.js and getProjectRoot from
../bootstrap/state.js with their equivalent src/* alias paths
(src/types/command.js and src/bootstrap/state.js respectively) to comply with
the repo's import-path contract for src modules.

Source: Coding guidelines

packages/workflow-engine/examples/smoke.ts-66-77 (1)

66-77: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

withRetry currently performs one extra attempt (off-by-one).

Line 71 allows 5 total attempts when default is 4; this does not match the intended cap.

Proposed fix

-async function withRetry<T>(fn: () => Promise<T>, retries = 4): Promise<T> {
-  for (let attempt = 0; ; attempt++) {
+async function withRetry<T>(fn: () => Promise<T>, maxAttempts = 4): Promise<T> {
+  for (let attempt = 1; ; attempt++) {
@@
-      if (!isRetryable(e) || attempt >= retries) throw e
-      const wait = Math.min(500 * 2 ** attempt, 8000)
+      if (!isRetryable(e) || attempt >= maxAttempts) throw e
+      const wait = Math.min(500 * 2 ** (attempt - 1), 8000)

🤖 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/workflow-engine/examples/smoke.ts` around lines 66 - 77, The
`withRetry` function has an off-by-one error in its retry limit check. The
condition `attempt >= retries` allows one extra attempt beyond the intended
limit. With the default value of `retries = 4`, the function performs 5 total
attempts instead of 4. Fix this by changing the condition from `attempt >=
retries` to `attempt >= retries - 1` so that exactly 4 attempts occur before the
error is thrown when retries defaults to 4.

packages/workflow-engine/src/__tests__/context.test.ts-57-60 (1)

57-60: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid double-releasing semaphore permits in test cleanup.

At Line 57 / Line 75 you release one permit to unblock pending acquires, then at Line 59 / Line 77 the same releaser can be invoked again via the cleanup loop. That over-credits permits and can mask semaphore accounting bugs.

Proposed fix

-  releases1[0]!() // release one, the fourth should be woken up
+  const firstRelease = releases1.shift()
+  firstRelease?.() // release one, the fourth should be woken up
   releases1.push(await pending)
   for (const rel of releases1) rel()
...
-  releases2[0]!()
+  const secondRelease = releases2.shift()
+  secondRelease?.()
   releases2.push(await pending2)
   for (const rel of releases2) rel()

Also applies to: 75-77

🤖 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/workflow-engine/src/__tests__/context.test.ts` around lines 57 - 60,
The test is double-releasing semaphore permits which can mask accounting bugs.
At the first site (lines 57-60 in
packages/workflow-engine/src/__tests__/context.test.ts), you explicitly call
releases1[0]() to release one permit, then at the cleanup loop (lines 75-77),
the same releaser is invoked again through the for loop over all releases1
elements. To fix this, after the explicit release call releases1[0](), remove
that element from the releases1 array (using splice or slice) so it is not
released again in the subsequent cleanup loop. Apply the same fix at the second
affected site (lines 75-77) to ensure permits are never released more than once.

src/skills/bundled/__tests__/ultracode.test.ts-85-96 (1)

85-96: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard USER_TYPE restoration with try/finally to keep test isolation.

If this test fails before the restore block runs, process.env.USER_TYPE stays mutated and can cascade failures into other suites.

Proposed fix

   const previousUserType = process.env.USER_TYPE
-  delete process.env.USER_TYPE
-  clearBundledSkills()
-  registerUltracodeSkill()
-
-  const skills = getBundledSkills()
-  expect(skills.some(s => s.name === 'ultracode')).toBe(true)
-
-  // Restore so we never mutate the process env for other test files.
-  if (previousUserType === undefined) delete process.env.USER_TYPE
-  else process.env.USER_TYPE = previousUserType
+  try {
+    delete process.env.USER_TYPE
+    clearBundledSkills()
+    registerUltracodeSkill()
+
+    const skills = getBundledSkills()
+    expect(skills.some(s => s.name === 'ultracode')).toBe(true)
+  } finally {
+    // Restore so we never mutate the process env for other test files.
+    if (previousUserType === undefined) delete process.env.USER_TYPE
+    else process.env.USER_TYPE = previousUserType
+  }

🤖 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/skills/bundled/__tests__/ultracode.test.ts` around lines 85 - 96, The
test does not guarantee that process.env.USER_TYPE is restored if an error
occurs before the restoration block, which can cause test isolation issues. Wrap
the section that saves previousUserType, deletes process.env.USER_TYPE, calls
clearBundledSkills(), registerUltracodeSkill(), and getBundledSkills(), in a
try/finally block. Move the existing restoration logic (the if/else that checks
previousUserType) into the finally block so that process.env.USER_TYPE is always
restored to its original state, regardless of whether the test passes or fails.

src/tasks/LocalWorkflowTask/__tests__/LocalWorkflowTask.test.ts-65-66 (1)

65-66: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use English test descriptions for .test.ts files.

Line 65 and Line 78 test descriptions are not in English, which breaks the test naming convention used across the suite.

Suggested adjustment

-test('保存 error 字符串到 state（供 BackgroundTasksDialog 显示失败原因）', () => {
+test('stores error string in state for BackgroundTasksDialog failure display', () => {
@@
-test('不传 error 时 state.error 保持 undefined（向后兼容现有调用）', () => {
+test('keeps state.error undefined when error is omitted (backward compatible)', () => {

As per coding guidelines, **/*.test.ts must use describe() + test() with English descriptions.

Also applies to: 78-79

🤖 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/tasks/LocalWorkflowTask/__tests__/LocalWorkflowTask.test.ts` around lines
65 - 66, Replace the Chinese test descriptions with English equivalents in the
LocalWorkflowTask.test.ts file. At line 65, the test description '保存 error 字符串到
state（供 BackgroundTasksDialog 显示失败原因）' must be translated to English. Similarly,
at line 78-79, any Chinese test descriptions must be converted to English.
Ensure all test() and describe() blocks throughout the file use only English
descriptions to maintain consistency with the coding guidelines for test files.

Source: Coding guidelines

src/workflow/notifications.ts-47-62 (1)

47-62: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prune prevStatus entries for runs that no longer exist.

The map keeps statuses for removed run IDs indefinitely, which can grow unbounded during long-lived sessions.

Suggested fix

   const unsubscribe = service.subscribe(() => {
     const runs = service.listRuns()
+    const currentIds = new Set(runs.map(r => r.runId))
+    for (const runId of prevStatus.keys()) {
+      if (!currentIds.has(runId)) prevStatus.delete(runId)
+    }
     for (const run of runs) {

🤖 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/workflow/notifications.ts` around lines 47 - 62, The prevStatus map in
the subscription callback is never cleaned up, causing entries for deleted runs
to accumulate indefinitely. After processing all runs from service.listRuns() in
the loop, add logic to remove entries from prevStatus that correspond to run IDs
no longer present in the current list. Identify which run IDs exist in
prevStatus but are not in the runs array, and delete those entries from the map
to prevent unbounded memory growth.

src/workflow/panel/WorkflowsPanel.tsx-130-134 (1)

130-134: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize prevTab fallback when activeRunId is not found.

When findIndex returns -1, current math picks runs.length - 2, which is inconsistent with expected wrap behavior.

Suggested fix

   const prevTab = (): void => {
     if (runs.length === 0) return;
     const idx = runs.findIndex(r => r.runId === activeRunId);
-    const next = runs[(idx - 1 + runs.length) % runs.length]!;
+    const base = idx === -1 ? 0 : idx;
+    const next = runs[(base - 1 + runs.length) % runs.length]!;
     switchTab(next.runId);
   };

🤖 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/workflow/panel/WorkflowsPanel.tsx` around lines 130 - 134, The prevTab
function has an issue with the fallback behavior when activeRunId is not found
in the runs array. When findIndex returns -1, the calculation (idx - 1 +
runs.length) % runs.length incorrectly results in runs.length - 2 instead of
wrapping properly. Fix this by normalizing the idx value when it equals -1 to a
valid starting position (such as runs.length) before performing the modulo
arithmetic, ensuring consistent and predictable wrap-around behavior for the
previous tab navigation.

🧹 Nitpick comments (7)

src/components/EffortPanel/effortPanelState.ts (1)

1-127: 💤 Low value

Consider using English for inline comments.

The logic and type safety in this module are excellent. However, the inline comments are in Chinese (lines 4-5, 28, etc.), which is unusual for the codebase. While not explicitly forbidden by the coding guidelines, using English would improve maintainability for international contributors.

🤖 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/components/EffortPanel/effortPanelState.ts` around lines 1 - 127, Replace
all Chinese inline comments in the effortPanelState.ts file with English
equivalents to improve maintainability for international contributors.
Specifically, convert the comments above the PanelPosition type definition
(describing cursor position and ultracode), the comment above
isNonUltracodePosition function (describing position validation), the comment
above normalizeToPanelPosition function (describing normalization logic), and
any other Chinese comments throughout the module to clear English descriptions
that preserve the original meaning and context.

docs/superpowers/specs/2026-06-14-effort-panel-design.md (1)

37-37: 💤 Low value

Add language specifiers to fenced code blocks.

The markdown linter flags multiple fenced code blocks without language specifiers (lines 37, 73, 84, 100, 111, 135, 161, 180, 222, 275, 308). Adding language identifiers improves syntax highlighting and readability.

💡 Example fixes

For pseudo-code or plain text blocks showing UI output, use:

-```
+```text
 Effort
 
        Faster                                                          Smarter

For actual code blocks, use the appropriate language (e.g., typescript, javascript, bash).

Also applies to: 73-73, 84-84, 100-100, 111-111, 135-135, 161-161, 180-180, 222-222, 275-275, 308-308

🤖 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 `@docs/superpowers/specs/2026-06-14-effort-panel-design.md` at line 37,
Multiple fenced code blocks in the markdown file lack language specifiers, which
the markdown linter flags as issues. Add appropriate language identifiers to
each fenced code block at lines 37, 73, 84, 100, 111, 135, 161, 180, 222, 275,
and 308. For blocks showing plain text or UI output, use the "text" language
specifier. For blocks containing actual code, use the appropriate language
identifier (such as typescript, javascript, bash, etc.). Update each opening
triple backtick from ``` to ```<language> with the correct language identifier.

Source: Linters/SAST tools

packages/workflow-engine/src/tool/schema.ts (1)

1-1: ⚡ Quick win

Derive maxConcurrency from shared constants to prevent contract drift.

max(16) duplicates the engine cap declared in constants.ts. If that cap changes, schema validation and prompts can silently diverge.

♻️ Proposed fix

 import { z } from 'zod/v4'
+import { MAX_CONCURRENCY_CAP } from '../constants.js'
@@
   maxConcurrency: z
     .number()
     .int()
     .min(1)
-    .max(16)
+    .max(MAX_CONCURRENCY_CAP)
     .optional()
     .describe(
-      'Concurrency cap for agent(). Defaults to 3 (max 16). When the workflow contains heavy parallel/pipeline fan-out, you may confirm the desired concurrency with the user via AskUserQuestion before launching.',
+      `Concurrency cap for agent(). Defaults to 3 (max ${MAX_CONCURRENCY_CAP}). When the workflow contains heavy parallel/pipeline fan-out, you may confirm the desired concurrency with the user via AskUserQuestion before launching.`,
     ),

Also applies to: 32-40

🤖 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/workflow-engine/src/tool/schema.ts` at line 1, The schema file
contains a hardcoded max(16) value for maxConcurrency that duplicates the engine
cap defined in constants.ts, causing potential contract drift if the cap changes
in one place but not the other. Import the maxConcurrency constant from
constants.ts and replace all hardcoded max(16) occurrences in the schema
definition with the imported constant. This applies to multiple locations in the
file where maxConcurrency validation is defined, ensuring the schema always
stays in sync with the authoritative constant value.

packages/workflow-engine/examples/research-report/run.ts (1)

68-87: ⚡ Quick win

Replace direct unknown casts with a type guard in error helpers.

isRetryable and errSummary force-cast unknown; narrowing once via a guard keeps strict-mode intent and avoids unsafe assumptions.

As per coding guidelines, "**/*.{ts,tsx}: Use type guards (type narrowing) with union types instead of forced type casting."

🤖 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/workflow-engine/examples/research-report/run.ts` around lines 68 -
87, Both the `isRetryable` and `errSummary` functions use unsafe forced type
casting with the `as` operator to cast the unknown parameter to specific object
shapes. Replace these force-casts with proper type guards that safely narrow the
type by checking property existence and types using `typeof` checks and property
validation before accessing error properties like `status`, `name`, `message`,
and `error.type`. This maintains strict-mode type safety while preserving the
intended logic for both error handling functions.

Source: Coding guidelines

packages/workflow-engine/src/__tests__/schema.test.ts (1)

4-62: ⚡ Quick win

Use the required describe() + test() suite structure consistently in .test.ts files.
All four suites currently use top-level test() only; wrap related cases in describe('moduleName', ...) to match repository test conventions.

• packages/workflow-engine/src/__tests__/schema.test.ts#L4-L62: wrap current schema tests in a single describe('workflowInputSchema', ...).
• packages/workflow-engine/src/__tests__/types.test.ts#L4-L52: wrap branch/round-trip tests in describe('AgentRunResult/JournalEntry', ...).
• packages/workflow-engine/src/__tests__/structuredOutput.test.ts#L14-L40: wrap validator tests in describe('validateAgainstSchema', ...).
• packages/workflow-engine/src/__tests__/integration.test.ts#L97-L282: group integration scenarios under describe('runWorkflow integration', ...).

As per coding guidelines, "**/*.test.ts: ... use describe() + test() pattern with English descriptions".

🤖 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/workflow-engine/src/__tests__/schema.test.ts` around lines 4 - 62,
The test files currently use top-level test() calls without describe() grouping
blocks. This needs to be fixed across four files to match repository
conventions. In packages/workflow-engine/src/__tests__/schema.test.ts (lines
4-62), wrap all the existing test() calls in a describe('workflowInputSchema',
...) block. In packages/workflow-engine/src/__tests__/types.test.ts (lines
4-52), wrap all branch/round-trip test() calls in a
describe('AgentRunResult/JournalEntry', ...) block. In
packages/workflow-engine/src/__tests__/structuredOutput.test.ts (lines 14-40),
wrap the validateAgainstSchema test() calls in a
describe('validateAgainstSchema', ...) block. In
packages/workflow-engine/src/__tests__/integration.test.ts (lines 97-282), wrap
the integration scenario test() calls in a describe('runWorkflow integration',
...) block. Each describe() should contain all related test() cases for that
module, maintaining the existing test logic unchanged.

Source: Coding guidelines

src/workflow/__tests__/claudeCodeBackend.test.ts (1)

116-387: Wrap tests in describe() blocks and stop re-mocking the same module within test cases.

This file violates two test structure guidelines:

1. Missing describe() grouping: All 22 tests are top-level test() calls without describe() blocks. Guideline requires describe('functionName') + test('behavior...') grouping (e.g., group claudeCodeBackend.run tests, extractStructuredOutput tests, etc.).

2. Re-mocking same module in tests: Lines 180, 200, 226, and 244 re-mock @claude-code-best/builtin-tools/tools/AgentTool/runAgent.js within individual test functions. Guideline prohibits mocking the same module twice. Use a single setup mock or restore the default with a helper between tests instead.

🤖 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/workflow/__tests__/claudeCodeBackend.test.ts` around lines 116 - 387,
Organize all 22 top-level test() calls into logical describe() blocks based on
functionality: group tests for claudeCodeBackend.run() behavior (including
worktree isolation, error handling, and abort scenarios), tests for
resolveAgentDefinition(), tests for mapWorkflowModel(), and tests for
extractStructuredOutput(). For the re-mocking issue at the four locations where
`@claude-code-best/builtin-tools/tools/AgentTool/runAgent.js` is mocked (in the
abort pre-abort test, the AbortError test, the registerAgentAbort test, and
comments mentioning restoration), establish a single mock setup or use a
beforeEach/afterEach pattern to reset the mock between test groups instead of
re-mocking within individual tests. This consolidation ensures the module is
mocked only once at the appropriate scope rather than multiple times across
different test functions.

Source: Coding guidelines

src/workflow/__tests__/persistence.test.ts (1)

36-199: ⚡ Quick win

Standardize these test files to describe(...) + test(...) structure.

All three files currently define top-level tests without a describe block; align them with the repository test structure contract for consistency and discoverability.

• src/workflow/__tests__/persistence.test.ts#L36-L199: wrap related persistence cases in a module/function-level describe(...).
• src/workflow/__tests__/selectors.test.ts#L26-L82: group selector tests under a describe('selectors') (or similar) block.
• src/workflow/__tests__/WorkflowsPanel.test.tsx#L14-L197: group panel/helper tests into one or more describe(...) blocks by subject.

As per coding guidelines, **/*.test.ts should use "describe() + test() pattern with English descriptions" and **/__tests__/**/*.test.{ts,tsx} should use "describe('functionName') with test('behavior description') in English."

🤖 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/workflow/__tests__/persistence.test.ts` around lines 36 - 199, Wrap all
test cases in each file with `describe()` blocks to follow the repository test
structure pattern. In src/workflow/__tests__/persistence.test.ts (lines 36-199),
wrap all the existing test cases in a single describe block (e.g.,
describe('persistence') or describe('writeRunState and readRunState')). In
src/workflow/__tests__/selectors.test.ts (lines 26-82), wrap all test cases in
describe('selectors'). In src/workflow/__tests__/WorkflowsPanel.test.tsx (lines
14-197), group the panel and helper tests into one or more describe blocks
organized by subject (e.g., describe('WorkflowsPanel') or similar based on test
content). Each describe block should contain all the existing test() calls as
children, maintaining the same test descriptions and implementation logic.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e28d12a2-6468-4557-bb60-ec3863a1d625

📥 Commits

Reviewing files that changed from the base of the PR and between 3e3e1de and e637b4f.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (127)

• biome.json
• docs/features/workflow-scripts.md
• docs/superpowers/plans/2026-06-12-workflow-engine.md
• docs/superpowers/plans/2026-06-13-workflow-panel-redesign.md
• docs/superpowers/plans/2026-06-13-workflow-run-state-persistence.md
• docs/superpowers/plans/2026-06-13-workflow-tui-ultracode.md
• docs/superpowers/plans/2026-06-14-effort-panel-basic.md
• docs/superpowers/reviews/2026-06-13-workflow-engine-commit-0768d4dc-review.md
• docs/superpowers/specs/2026-06-12-workflow-engine-design.md
• docs/superpowers/specs/2026-06-13-workflow-panel-redesign.md
• docs/superpowers/specs/2026-06-13-workflow-run-state-persistence-design.md
• docs/superpowers/specs/2026-06-13-workflow-tui-ultracode-design.md
• docs/superpowers/specs/2026-06-14-effort-panel-design.md
• packages/builtin-tools/src/index.ts
• packages/builtin-tools/src/tools/WorkflowTool/WorkflowTool.ts
• packages/builtin-tools/src/tools/WorkflowTool/__tests__/WorkflowTool.test.ts
• packages/builtin-tools/src/tools/WorkflowTool/bundled/index.ts
• packages/builtin-tools/src/tools/WorkflowTool/constants.ts
• packages/builtin-tools/src/tools/WorkflowTool/createWorkflowCommand.ts
• packages/workflow-engine/examples/registry-demo.ts
• packages/workflow-engine/examples/research-report/README.md
• packages/workflow-engine/examples/research-report/research-report.workflow.mjs
• packages/workflow-engine/examples/research-report/run.ts
• packages/workflow-engine/examples/smoke.ts
• packages/workflow-engine/package.json
• packages/workflow-engine/src/__tests__/WorkflowTool.test.ts
• packages/workflow-engine/src/__tests__/agentAdapter.test.ts
• packages/workflow-engine/src/__tests__/agentId.test.ts
• packages/workflow-engine/src/__tests__/budget.test.ts
• packages/workflow-engine/src/__tests__/concurrency.test.ts
• packages/workflow-engine/src/__tests__/context.test.ts
• packages/workflow-engine/src/__tests__/errors.test.ts
• packages/workflow-engine/src/__tests__/events.test.ts
• packages/workflow-engine/src/__tests__/hooks.test.ts
• packages/workflow-engine/src/__tests__/index.test.ts
• packages/workflow-engine/src/__tests__/integration.test.ts
• packages/workflow-engine/src/__tests__/journal.test.ts
• packages/workflow-engine/src/__tests__/namedWorkflows.test.ts
• packages/workflow-engine/src/__tests__/paths.test.ts
• packages/workflow-engine/src/__tests__/persistInline.test.ts
• packages/workflow-engine/src/__tests__/ports.test.ts
• packages/workflow-engine/src/__tests__/runWorkflow.test.ts
• packages/workflow-engine/src/__tests__/schema.test.ts
• packages/workflow-engine/src/__tests__/script.test.ts
• packages/workflow-engine/src/__tests__/structuredOutput.test.ts
• packages/workflow-engine/src/__tests__/types.test.ts
• packages/workflow-engine/src/agentAdapter.ts
• packages/workflow-engine/src/constants.ts
• packages/workflow-engine/src/engine/budget.ts
• packages/workflow-engine/src/engine/concurrency.ts
• packages/workflow-engine/src/engine/context.ts
• packages/workflow-engine/src/engine/errors.ts
• packages/workflow-engine/src/engine/hooks.ts
• packages/workflow-engine/src/engine/journal.ts
• packages/workflow-engine/src/engine/namedWorkflows.ts
• packages/workflow-engine/src/engine/paths.ts
• packages/workflow-engine/src/engine/runWorkflow.ts
• packages/workflow-engine/src/engine/script.ts
• packages/workflow-engine/src/engine/structuredOutput.ts
• packages/workflow-engine/src/index.ts
• packages/workflow-engine/src/ports.ts
• packages/workflow-engine/src/progress/events.ts
• packages/workflow-engine/src/tool/WorkflowTool.ts
• packages/workflow-engine/src/tool/constants.ts
• packages/workflow-engine/src/tool/persistInline.ts
• packages/workflow-engine/src/tool/schema.ts
• packages/workflow-engine/src/types.ts
• packages/workflow-engine/tsconfig.json
• src/commands.ts
• src/commands/effort/effort.tsx
• src/commands/workflows/index.ts
• src/components/EffortPanel/EffortPanel.tsx
• src/components/EffortPanel/__tests__/EffortPanel.test.tsx
• src/components/EffortPanel/__tests__/effortPanelState.test.ts
• src/components/EffortPanel/__tests__/rippleAnimation.test.ts
• src/components/EffortPanel/effortPanelState.ts
• src/components/EffortPanel/rippleAnimation.ts
• src/components/EffortPanel/useRippleFrame.ts
• src/components/permissions/PermissionRequest.tsx
• src/components/tasks/BackgroundTasksDialog.tsx
• src/components/tasks/WorkflowDetailDialog.tsx
• src/constants/tools.ts
• src/keybindings/defaultBindings.ts
• src/keybindings/schema.ts
• src/main.tsx
• src/skills/bundled/__tests__/ultracode.test.ts
• src/skills/bundled/index.ts
• src/skills/bundled/ultracode.ts
• src/tasks/LocalWorkflowTask/LocalWorkflowTask.ts
• src/tasks/LocalWorkflowTask/__tests__/LocalWorkflowTask.test.ts
• src/tools.ts
• src/utils/effort.ts
• src/utils/permissions/classifierDecision.ts
• src/utils/worktree.ts
• src/workflow/WorkflowPermissionRequest.tsx
• src/workflow/__tests__/WorkflowsPanel.test.tsx
• src/workflow/__tests__/claudeCodeBackend.test.ts
• src/workflow/__tests__/notifications.test.ts
• src/workflow/__tests__/persistence.test.ts
• src/workflow/__tests__/ports.test.ts
• src/workflow/__tests__/progressBus.test.ts
• src/workflow/__tests__/progressStore.test.ts
• src/workflow/__tests__/runStatePersistence.test.ts
• src/workflow/__tests__/selectors.test.ts
• src/workflow/__tests__/service.test.ts
• src/workflow/__tests__/status.test.ts
• src/workflow/__tests__/useWorkflowKeyboard.test.ts
• src/workflow/backends/claudeCodeBackend.ts
• src/workflow/hostHandle.ts
• src/workflow/namedWorkflowCommands.ts
• src/workflow/notifications.ts
• src/workflow/panel/AgentList.tsx
• src/workflow/panel/PhaseSidebar.tsx
• src/workflow/panel/TabsBar.tsx
• src/workflow/panel/WorkflowsPanel.tsx
• src/workflow/panel/panelCall.tsx
• src/workflow/panel/selectors.ts
• src/workflow/panel/status.ts
• src/workflow/panel/useWorkflowKeyboard.ts
• src/workflow/persistence.ts
• src/workflow/ports.ts
• src/workflow/progress/bus.ts
• src/workflow/progress/store.ts
• src/workflow/registry.ts
• src/workflow/service.ts
• src/workflow/wiring.ts
• tsconfig.json

💤 Files with no reviewable changes (6)

• packages/builtin-tools/src/tools/WorkflowTool/constants.ts
• src/components/tasks/WorkflowDetailDialog.tsx
• packages/builtin-tools/src/tools/WorkflowTool/bundled/index.ts
• packages/builtin-tools/src/tools/WorkflowTool/tests/WorkflowTool.test.ts
• packages/builtin-tools/src/tools/WorkflowTool/WorkflowTool.ts
• packages/builtin-tools/src/tools/WorkflowTool/createWorkflowCommand.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace fixed sleep waits with condition-based polling to prevent flaky tests.

These assertions depend on setTimeout(50) finishing background work in time. On slower CI, this intermittently races and causes non-deterministic failures.

Proposed pattern

+async function waitFor(
+  predicate: () => boolean,
+  timeoutMs = 2000,
+  stepMs = 10,
+): Promise<void> {
+  const start = Date.now()
+  while (!predicate()) {
+    if (Date.now() - start > timeoutMs) {
+      throw new Error('Timed out waiting for condition')
+    }
+    await new Promise(r => setTimeout(r, stepMs))
+  }
+}
...
-    await new Promise(r => {
-      setTimeout(r, 50)
-    })
+    await waitFor(() => runStatus.get('run-x') === 'completed')

Also applies to: 158-160, 203-205, 223-225, 315-317, 370-372, 427-429, 520-522

🤖 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/workflow-engine/src/__tests__/WorkflowTool.test.ts` around lines 68
- 70, Replace all fixed setTimeout-based waits with condition-based polling to
eliminate race conditions in the WorkflowTool.test.ts file. At lines 68-70,
158-160, 203-205, 223-225, 315-317, 370-372, 427-429, and 520-522, replace each
instance of await new Promise(r => { setTimeout(r, 50) }) with a polling loop
that repeatedly checks for the expected condition (such as async operations
completing, state changes, or side effects manifesting) until it succeeds or
times out. This ensures tests wait for actual completion of background work
rather than a fixed duration, making tests deterministic and reliable on both
fast and slow CI environments.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate budget caps at construction to prevent invalid cap states.

total is accepted as any number, so NaN/negative/non-finite values create broken enforcement (remaining() can be NaN, or cap is instantly exhausted). Normalize or reject invalid totals at construction.

Proposed fix

 export class Budget {
   private spentTokens = 0
+  readonly total: number | null
 
-  constructor(readonly total: number | null) {}
+  constructor(total: number | null) {
+    if (total !== null && (!Number.isFinite(total) || total < 0)) {
+      throw new TypeError(
+        'budget.total must be null or a finite non-negative number',
+      )
+    }
+    this.total = total
+  }

Also applies to: 21-25, 31-34

🤖 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/workflow-engine/src/engine/budget.ts` at line 15, Add validation
logic in the constructor of the Budget class to reject or normalize invalid
values for the total parameter. Before assigning the total value, check if it is
NaN, negative, or non-finite (using Number.isFinite()), and either throw an
error for invalid inputs or normalize them to appropriate defaults. This
validation at construction time in the constructor method will prevent
downstream issues in methods that depend on the total field, such as the
remaining() method and other budget enforcement logic that use this value.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the returned release callback idempotent.

Calling release twice currently returns extra permits and can violate the max-concurrency bound. Guard the callback so each acquire can release at most once.

Proposed fix

 export class Semaphore {
@@
   async acquire(signal?: AbortSignal): Promise<() => void> {
@@
     if (this.available > 0) {
       this.available -= 1
-      return () => this.release()
+      let released = false
+      return () => {
+        if (released) return
+        released = true
+        this.release()
+      }
     }
@@
       const wake = () => {
         signal?.removeEventListener('abort', onAbort)
-        resolve(() => this.release())
+        let released = false
+        resolve(() => {
+          if (released) return
+          released = true
+          this.release()
+        })
       }

Also applies to: 35-38, 48-55

🤖 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/workflow-engine/src/engine/concurrency.ts` around lines 21 - 29,
Make the release callback returned by the Semaphore class idempotent across all
affected sites. In the acquire method (anchor location at lines 21-29 showing
the synchronous return case), wrap the release callback returned at line 27 with
a guard that tracks whether it has already been called, ensuring it can only
release the permit once. Apply the same idempotent guard pattern to the sibling
locations at lines 35-38 (inside the Promise resolve path) and lines 48-55 (in
another method that returns release callbacks). Use a local flag variable within
each closure to track invocation state and prevent multiple releases.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Block path traversal on runId before any fs operation.

Line 25, Line 43, and Line 47 compose paths from raw runId; ../ or path separators can escape runsDir, and truncate() can recursively delete arbitrary directories.

Proposed fix

-import { join } from 'node:path'
+import { join, resolve, sep } from 'node:path'
@@
 export function createFileJournalStore(runsDir: string): JournalStore {
-  const pathOf = (runId: string) => join(runsDir, runId, 'journal.jsonl')
+  const runDirOf = (runId: string) => {
+    if (!/^[A-Za-z0-9._-]+$/.test(runId)) {
+      throw new Error(`Invalid runId: ${runId}`)
+    }
+    const base = resolve(runsDir)
+    const dir = resolve(base, runId)
+    if (dir !== base && !dir.startsWith(base + sep)) {
+      throw new Error(`Invalid runId: ${runId}`)
+    }
+    return dir
+  }
+  const pathOf = (runId: string) => join(runDirOf(runId), 'journal.jsonl')
@@
-      await mkdir(join(runsDir, runId), { recursive: true })
+      await mkdir(runDirOf(runId), { recursive: true })
       await appendFile(pathOf(runId), JSON.stringify(entry) + '\n', 'utf-8')
     },
     async truncate(runId) {
-      await rm(join(runsDir, runId), { recursive: true, force: true })
+      await rm(runDirOf(runId), { recursive: true, force: true })
     },

🤖 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/workflow-engine/src/engine/journal.ts` around lines 25 - 47, The
pathOf function and filesystem operations in the read, append, and truncate
methods use the runId parameter directly to construct file paths without
validating it first, creating a path traversal vulnerability where malicious
runId values containing "../" or path separators could escape the runsDir
directory. Add validation at the beginning of the read, append, and truncate
methods (or create a helper function) to ensure runId only contains safe
characters and does not include path traversal sequences like "../" or path
separators, rejecting or sanitizing any invalid runId before it is used in the
pathOf function or any filesystem operation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not treat all read failures as “no journal”.

Line 38 catches parse/permission/I/O corruption errors and returns [], which silently hides broken journal state and forces replay.

Proposed fix

-      } catch {
-        return []
+      } catch (error) {
+        if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+          return []
+        }
+        throw error
       }

🤖 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/workflow-engine/src/engine/journal.ts` around lines 28 - 40, The
catch block in the read method is too broad and silently treats all errors
(including JSON parsing failures, permission issues, and I/O corruption) as if
the journal file doesn't exist, returning an empty array. Instead, differentiate
between error types within the catch block: only return an empty array for "file
not found" errors (which legitimately means there is no journal), and re-throw
or handle other errors (like JSON parse errors or permission errors) so they
propagate to the caller and alert that the journal state is corrupted rather
than forcing a silent replay.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
import re, collections, pathlib
p = pathlib.Path("src/workflow/__tests__/claudeCodeBackend.test.ts")
s = p.read_text()
mods = re.findall(r"mock\.module\(\s*'([^']+)'", s)
counts = collections.Counter(mods)
for mod, n in sorted(counts.items(), key=lambda x: (-x[1], x[0])):
    if n > 1:
        print(f"{n}x {mod}")
PY

Repository: claude-code-best/claude-code

Length of output: 135

---

Consolidate repeated mocks for runAgent.js into a single module mock with per-test behavior.

The module @claude-code-best/builtin-tools/tools/AgentTool/runAgent.js is mocked 5 times across this test file (lines 6–16, 180–188, 200–213, 226–236, 244–254). Since mock.module is process-global in Bun, this creates order-coupled test outcomes. Per coding guidelines, do not mock the same module twice. Define the mock once and reset mutable stub state in beforeEach for per-test variations.

🤖 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/workflow/__tests__/claudeCodeBackend.test.ts` around lines 6 - 16, The
module `@claude-code-best/builtin-tools/tools/AgentTool/runAgent.js` is mocked
multiple times across the test file (at lines 6–16, 180–188, 200–213, 226–236,
and 244–254), which creates order-coupled test outcomes since mock.module is
process-global. Remove the duplicate mock.module calls for runAgent.js and keep
only one definition at the top of the file. Then in a beforeEach hook, reset the
mutable state of the runAgent mock (such as the generator function's yielded
values) before each test to allow per-test variations without re-mocking the
module. This ensures each test can configure the mock's behavior independently
while adhering to the guideline of not mocking the same module twice.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace fixed sleeps with deterministic wait to avoid flaky tests.

Line 38, Line 68, Line 93, Line 135, Line 160, and Line 172 rely on setTimeout(50). Under slower CI I/O, this can intermittently fail even when behavior is correct.

Suggested adjustment

+async function waitFor<T>(
+  read: () => Promise<T>,
+  done: (value: T) => boolean,
+  timeoutMs = 1000,
+  stepMs = 20,
+): Promise<T> {
+  const start = Date.now()
+  while (Date.now() - start < timeoutMs) {
+    const value = await read()
+    if (done(value)) return value
+    await new Promise(r => setTimeout(r, stepMs))
+  }
+  throw new Error('Timed out waiting for persisted run state')
+}
@@
-    await new Promise(r => setTimeout(r, 50))
-    const got = await readRunState(dir, 'rW')
+    const got = await waitFor(
+      () => readRunState(dir, 'rW'),
+      value => value !== null,
+    )

Also applies to: 68-68, 93-93, 135-135, 160-160, 172-172

🤖 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/workflow/__tests__/runStatePersistence.test.ts` at line 38, Replace all
fixed timeout sleeps with deterministic waits to eliminate test flakiness. In
src/workflow/__tests__/runStatePersistence.test.ts at lines 38, 68, 93, 135,
160, and 172, replace the `await new Promise(r => setTimeout(r, 50))` pattern
with a polling mechanism or condition-based wait that checks for a specific
state change or callback completion instead of relying on an arbitrary 50ms
timeout. This ensures tests pass reliably regardless of CI environment I/O
performance.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

settle() uses a fixed delay and makes async tests timing-flaky.

A hardcoded 60ms sleep can race detached workflow completion under slower CI; switch to condition-based polling with timeout.

Suggested pattern

-async function settle(): Promise<void> {
-  await new Promise(r => setTimeout(r, 60))
-}
+async function waitFor(
+  predicate: () => boolean,
+  timeoutMs = 2000,
+  intervalMs = 20,
+): Promise<void> {
+  const start = Date.now();
+  while (!predicate()) {
+    if (Date.now() - start > timeoutMs) {
+      throw new Error('Timed out waiting for condition');
+    }
+    await new Promise(r => setTimeout(r, intervalMs));
+  }
+}

🤖 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/workflow/__tests__/service.test.ts` around lines 179 - 181, The settle()
function uses a hardcoded 60ms setTimeout which creates timing-dependent
flakiness in tests, particularly in slower CI environments where workflow
completion may race against the fixed delay. Replace this fixed delay with a
condition-based polling approach that repeatedly checks for a specific condition
(such as workflow completion status or callback invocation) with a maximum
timeout. This ensures the test waits only as long as necessary and completes
promptly when the condition is satisfied, rather than always waiting the full
60ms.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Escape XML-special characters before interpolating run fields into notification XML.

run.workflowName and run.error are interpolated raw into the payload, so <, &, or " can corrupt the message body or inject unintended tags.

Suggested fix

+function escapeXml(value: string): string {
+  return value
+    .replaceAll('&', '&')
+    .replaceAll('<', '&lt;')
+    .replaceAll('>', '&gt;')
+    .replaceAll('"', '&quot;')
+    .replaceAll("'", '&apos;')
+}
+
 function buildMessage(run: RunProgress): string {
@@
-  const errorSuffix =
-    run.status === 'failed' && run.error ? `: ${run.error}` : ''
-  const summary = `Workflow "${run.workflowName}" ${statusText}${errorSuffix}`
+  const errorSuffix =
+    run.status === 'failed' && run.error ? `: ${String(run.error)}` : ''
+  const summary = `Workflow "${run.workflowName}" ${statusText}${errorSuffix}`
+  const safeRunId = escapeXml(run.runId)
+  const safeStatus = escapeXml(run.status)
+  const safeSummary = escapeXml(summary)
@@
-<${TASK_ID_TAG}>${run.runId}</${TASK_ID_TAG}>
+<${TASK_ID_TAG}>${safeRunId}</${TASK_ID_TAG}>
 <${TASK_TYPE_TAG}>${WORKFLOW_TASK_TYPE}</${TASK_TYPE_TAG}>
-<${STATUS_TAG}>${run.status}</${STATUS_TAG}>
-<${SUMMARY_TAG}>${summary}</${SUMMARY_TAG}>
+<${STATUS_TAG}>${safeStatus}</${STATUS_TAG}>
+<${SUMMARY_TAG}>${safeSummary}</${SUMMARY_TAG}>
 </${TASK_NOTIFICATION_TAG}>`
 }

📝 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.

Suggested change

	function buildMessage(run: RunProgress): string {
	const statusText =
	run.status === 'completed'
	? 'completed successfully'
	: run.status === 'failed'
	? 'failed'
	: 'was stopped'
	const errorSuffix =
	run.status === 'failed' && run.error ? `: ${run.error}` : ''
	const summary = `Workflow "${run.workflowName}" ${statusText}${errorSuffix}`
	return `<${TASK_NOTIFICATION_TAG}>
	<${TASK_ID_TAG}>${run.runId}</${TASK_ID_TAG}>
	<${TASK_TYPE_TAG}>${WORKFLOW_TASK_TYPE}</${TASK_TYPE_TAG}>
	<${STATUS_TAG}>${run.status}</${STATUS_TAG}>
	<${SUMMARY_TAG}>${summary}</${SUMMARY_TAG}>
	</${TASK_NOTIFICATION_TAG}>`
	function escapeXml(value: string): string {
	return value
	.replaceAll('&', '&amp;')
	.replaceAll('<', '&lt;')
	.replaceAll('>', '&gt;')
	.replaceAll('"', '&quot;')
	.replaceAll("'", '&apos;')
	}
	function buildMessage(run: RunProgress): string {
	const statusText =
	run.status === 'completed'
	? 'completed successfully'
	: run.status === 'failed'
	? 'failed'
	: 'was stopped'
	const errorSuffix =
	run.status === 'failed' && run.error ? `: ${String(run.error)}` : ''
	const summary = `Workflow "${run.workflowName}" ${statusText}${errorSuffix}`
	const safeRunId = escapeXml(run.runId)
	const safeStatus = escapeXml(run.status)
	const safeSummary = escapeXml(summary)
	return `<${TASK_NOTIFICATION_TAG}>
	<${TASK_ID_TAG}>${safeRunId}</${TASK_ID_TAG}>
	<${TASK_TYPE_TAG}>${WORKFLOW_TASK_TYPE}</${TASK_TYPE_TAG}>
	<${STATUS_TAG}>${safeStatus}</${STATUS_TAG}>
	<${SUMMARY_TAG}>${safeSummary}</${SUMMARY_TAG}>
	</${TASK_NOTIFICATION_TAG}>`
	}

🤖 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/workflow/notifications.ts` around lines 71 - 87, The buildMessage
function interpolates run.workflowName and run.error directly into XML without
escaping special characters, which can corrupt the message or enable XML
injection. Create a helper function to escape XML-special characters
(specifically &, <, >, ", and ') and apply this escaping to both
run.workflowName when building the summary string and to run.error when building
the errorSuffix before they are interpolated into the XML payload.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Snapshot the agent target when opening confirm mode to prevent killing the wrong agent.

The selected agent is re-derived at confirm time from visibleAgents[clampedAgent]. If agent ordering/filtering changes while the dialog is open, confirmYes can kill a different agent than the one the user intended.

Suggested fix

-const [confirmKill, setConfirmKill] = useState<null | 'agent' | 'workflow'>(null);
+const [confirmKill, setConfirmKill] = useState<
+  | null
+  | { kind: 'workflow' }
+  | { kind: 'agent'; runId: string; agentId: number; label: string }
+>(null);
@@
-      setConfirmKill('agent');
+      setConfirmKill({
+        kind: 'agent',
+        runId: focused.runId,
+        agentId: agent.id,
+        label: agent.label ?? `agent-${agent.id}`,
+      });
@@
-      setConfirmKill('workflow');
+      setConfirmKill({ kind: 'workflow' });
@@
-      if (confirmKill === 'workflow' && focused) {
+      if (confirmKill?.kind === 'workflow' && focused) {
@@
-      } else if (confirmKill === 'agent' && focused) {
-        const agent = visibleAgents[clampedAgent];
-        if (agent) svc.killAgent(focused.runId, agent.id);
+      } else if (confirmKill?.kind === 'agent' && focused) {
+        if (confirmKill.runId === focused.runId) {
+          svc.killAgent(focused.runId, confirmKill.agentId);
+        }
       }
@@
-            confirmKill === 'workflow'
+            confirmKill?.kind === 'workflow'
               ? `Kill workflow "${focused?.workflowName ?? ''}"?`
-              : `Kill agent "${visibleAgents[clampedAgent]?.label ?? ''}"?`
+              : `Kill agent "${confirmKill?.kind === 'agent' ? confirmKill.label : ''}"?`

Also applies to: 193-196, 268-269

🤖 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/workflow/panel/WorkflowsPanel.tsx` around lines 150 - 157, Snapshot the
selected agent when opening the confirm dialog to prevent killing the wrong
agent if the agent list changes. In the killAgent function, capture the actual
agent object from visibleAgents at the time of setting setConfirmKill, and store
it in state so that the confirmYes function can use the saved agent instead of
re-deriving it from visibleAgents. Apply the same pattern to all other similar
confirmation flows that re-derive their targets (like the operations around line
193-196 and 268-269), ensuring the target entity is always captured at decision
time rather than at confirmation time.

[coderabbitai]

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 @.github/workflows/ci.yml:
- Around line 50-58: The Codecov upload step in the CI workflow is currently
commented out, but this breaks the PR patch-coverage enforcement policy defined
in codecov.yml. Uncomment the entire "Upload coverage to Codecov" step
(including the name, if condition, uses directive with the specific version pin
codecov/codecov-action@75cd11691c0faa626561e295848008c8a7dddffe, and all
configuration parameters like fail_ci_if_error, files, disable_search, and
token) to restore coverage policy enforcement for pull requests.

🪄 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: 93b537b1-f64d-4afd-aa68-cc2508296678

📥 Commits

Reviewing files that changed from the base of the PR and between e637b4f and 6252a1b.

📒 Files selected for processing (4)

• .github/workflows/ci.yml
• packages/builtin-tools/src/tools/FileReadTool/FileReadTool.ts
• packages/workflow-engine/src/engine/script.ts
• src/workflow/__tests__/claudeCodeBackend.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/workflow/tests/claudeCodeBackend.test.ts
• packages/workflow-engine/src/engine/script.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Re-enable Codecov upload to preserve PR patch-coverage enforcement.

Commenting out this step breaks the coverage policy contract: codecov.yml (Lines 1–11) requires PR patch coverage status (target 100%), but no report is uploaded anymore.

Suggested fix

-      # codecov 坏了，老是失败，先注释掉
-      # - name: Upload coverage to Codecov
-      #   if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
-      #   uses: codecov/codecov-action@75cd11691c0faa626561e295848008c8a7dddffe # v5, 2026-04-25
-      #   with:
-      #     fail_ci_if_error: true
-      #     files: ./coverage/lcov.info
-      #     disable_search: true
-      #     token: ${{ secrets.CODECOV_TOKEN }}
+      - name: Upload coverage to Codecov
+        if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
+        uses: codecov/codecov-action@75cd11691c0faa626561e295848008c8a7dddffe # v5, 2026-04-25
+        with:
+          fail_ci_if_error: false
+          files: ./coverage/lcov.info
+          disable_search: true
+          token: ${{ secrets.CODECOV_TOKEN }}

🤖 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 @.github/workflows/ci.yml around lines 50 - 58, The Codecov upload step in
the CI workflow is currently commented out, but this breaks the PR
patch-coverage enforcement policy defined in codecov.yml. Uncomment the entire
"Upload coverage to Codecov" step (including the name, if condition, uses
directive with the specific version pin
codecov/codecov-action@75cd11691c0faa626561e295848008c8a7dddffe, and all
configuration parameters like fail_ci_if_error, files, disable_search, and
token) to restore coverage policy enforcement for pull requests.