feat: Quality plugin — Task QA tab, preview servers, tests, and suggested cases#2127
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughIntroduces the ChangesQuality plugin MVP
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TaskDetailModal
participant PluginSlot
participant QualityQaTab
participant QualityRoutes
participant QualityStore
TaskDetailModal->>PluginSlot: pass task and project context
PluginSlot->>QualityQaTab: render task QA slot
QualityQaTab->>QualityRoutes: request runs, preview, or suggestions
QualityRoutes->>QualityStore: read or persist project-scoped data
QualityRoutes-->>QualityQaTab: return status, reports, or suggested cases
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR adds a bundled Quality plugin for task-level and project-level QA. The main changes are:
Confidence Score: 5/5No new blocking issue qualifies for this follow-up review.
Important Files Changed
Reviews (14): Last reviewed commit: "fix(FN-2127): gate Quality and harden pr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
plugins/fusion-plugin-quality/src/store/quality-store.ts (1)
79-81: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a strict type guard instead of a broad type assertion.
Casting any string to
QualityPresetIdbypasses TypeScript's safety guarantees and could allow invalid strings to enter the domain if the database contains corrupted or outdated data. Please consider using the existingisQualityPresetIdhelper to ensure strict validation.🛡️ Proposed fix to use the strict type guard
Add the following import at the top of the file:
import { isQualityPresetId } from "../runner/command-presets.js";Then, apply this diff:
- if (Array.isArray(parsed)) { - steps = parsed.filter((s): s is QualityPresetId => typeof s === "string"); - } + if (Array.isArray(parsed)) { + steps = parsed.filter(isQualityPresetId); + }🤖 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 `@plugins/fusion-plugin-quality/src/store/quality-store.ts` around lines 79 - 81, Update the parsed-array filtering in the quality store to use the existing isQualityPresetId type guard from command-presets instead of accepting every string as QualityPresetId. Add the required import and preserve only values validated by that helper.plugins/fusion-plugin-quality/manifest.json (1)
7-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSync
manifest.jsonproperties withindex.ts.The
manifest.jsondefinitions are missing thedescriptionfor the dashboard view and theorderfor the UI slot, both of which are defined in the runtimeindex.tsplugin definition. Keeping these in sync ensures consistent behavior whether the host reads the static manifest or the runtime module.📝 Proposed fix to synchronize properties
"dashboardViews": [ { "viewId": "quality", "label": "Quality", "componentPath": "./dashboard-view", "icon": "ShieldCheck", "placement": "primary", - "order": 32 + "order": 32, + "description": "Project quality: runs, plans, and CI" } ], "uiSlots": [ { "slotId": "task-detail-tab", "label": "QA", "icon": "FlaskConical", - "componentPath": "./qa-tab" + "componentPath": "./qa-tab", + "order": 20 } ]🤖 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 `@plugins/fusion-plugin-quality/manifest.json` around lines 7 - 24, Update the manifest entries for the dashboard view with viewId "quality" and the UI slot with slotId "task-detail-tab" to match the corresponding runtime definitions in index.ts: add the dashboard view’s description and the UI slot’s order, preserving all existing properties.
🤖 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/plugin-sdk-core-runtime-shim.ts`:
- Around line 18-20: Update the bundled shim’s spawn implementation associated
with SuperviseSpawnOptions to forward killGraceMs and maxLifetimeMs into the
underlying spawn supervision logic, preserving the enforcement provided by
`@fusion/core`. If the shim cannot implement either option, remove the unsupported
fields from the SuperviseSpawnOptions public interface instead of exposing
ineffective configuration.
In `@plugins/fusion-plugin-quality/src/__tests__/command-presets.test.ts`:
- Around line 59-63: Add Windows absolute-path regression expectations to the
“rejects unsafe path tokens” test for isSafeFilePathToken, covering drive-letter
paths such as C:/outside/x.test.ts and environment-variable paths such as
%TEMP%/x.test.ts; both must be rejected while the existing safe relative-path
case remains accepted.
In `@plugins/fusion-plugin-quality/src/dashboard-view.tsx`:
- Around line 23-28: Update fetchRuns to throw an error when the response is not
OK instead of returning an empty array, allowing refresh()’s existing catch
handling to distinguish failed requests from genuinely empty run histories.
Preserve the current JSON parsing and data.runs fallback for successful
responses.
In `@plugins/fusion-plugin-quality/src/preview/preview-sessions.ts`:
- Around line 153-165: Update the stop method’s supervised-process shutdown flow
so the child reference remains available until the SIGTERM/SIGKILL cleanup
completes, rather than clearing session.supervised immediately. Ensure the later
close handling cannot overwrite the intentional stopped state with failed, while
preserving the existing graceful-then-forced termination timing.
In `@plugins/fusion-plugin-quality/src/runner/command-presets.ts`:
- Around line 39-45: Constrain file-scoped paths to projectRoot before
constructing the vitest command: update isSafeFilePathToken and the
command-building flow in command-presets.ts (lines 39-45 and 65-80) to reject
Windows drive-qualified and %VAR%-style paths, or switch the invocation to argv
with shell disabled. Add coverage for Windows absolute and environment-expansion
inputs in plugins/fusion-plugin-quality/src/__tests__/command-presets.test.ts
lines 59-63.
In `@plugins/fusion-plugin-quality/src/runner/command-runner.ts`:
- Around line 43-48: Quality run cancellation must terminate the supervised
child and remain terminal: in
plugins/fusion-plugin-quality/src/runner/command-runner.ts lines 43-48, expose
or register the supervised process by project/run ID; in lines 94-104, preserve
cancellation during finalization instead of overwriting it; in
plugins/fusion-plugin-quality/src/routes/create-routes.ts lines 187-209, retain
and clean up the run cancellation handle; and in lines 214-235, signal the
process group through that handle before marking the run cancelled.
---
Nitpick comments:
In `@plugins/fusion-plugin-quality/manifest.json`:
- Around line 7-24: Update the manifest entries for the dashboard view with
viewId "quality" and the UI slot with slotId "task-detail-tab" to match the
corresponding runtime definitions in index.ts: add the dashboard view’s
description and the UI slot’s order, preserving all existing properties.
In `@plugins/fusion-plugin-quality/src/store/quality-store.ts`:
- Around line 79-81: Update the parsed-array filtering in the quality store to
use the existing isQualityPresetId type guard from command-presets instead of
accepting every string as QualityPresetId. Add the required import and preserve
only values validated by that helper.
🪄 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 Plus
Run ID: 8739c560-716f-4270-bd26-0809a336c1db
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
.changeset/quality-plugin-mvp.mdCONCEPTS.mddocs/plans/2026-07-14-001-feat-quality-plugin-plan.mdpackages/cli/src/plugin-sdk-core-runtime-shim.tspackages/cli/src/plugins/staged-bundled-plugin-ids.tspackages/cli/tsup.config.tspackages/core/src/plugins/bundled-plugin-install.tspackages/dashboard/app/components/PluginManager.tsxpackages/dashboard/app/components/PluginSlot.tsxpackages/dashboard/app/components/TaskDetailModal.tsxpackages/dashboard/app/plugins/pluginSlotRegistry.tsxpackages/dashboard/app/plugins/registerBundledPluginViews.tspackages/dashboard/package.jsonpackages/dashboard/src/routes.tspackages/dashboard/vite.config.tspackages/dashboard/vitest.config.tsplugins/fusion-plugin-quality/README.mdplugins/fusion-plugin-quality/manifest.jsonplugins/fusion-plugin-quality/package.jsonplugins/fusion-plugin-quality/src/__tests__/command-presets.test.tsplugins/fusion-plugin-quality/src/__tests__/heuristic-cases.test.tsplugins/fusion-plugin-quality/src/__tests__/manifest.test.tsplugins/fusion-plugin-quality/src/__tests__/quality-store.test.tsplugins/fusion-plugin-quality/src/dashboard-view.tsxplugins/fusion-plugin-quality/src/index.tsplugins/fusion-plugin-quality/src/preview/preview-sessions.tsplugins/fusion-plugin-quality/src/qa-tab.tsxplugins/fusion-plugin-quality/src/quality-schema.tsplugins/fusion-plugin-quality/src/routes/create-routes.tsplugins/fusion-plugin-quality/src/runner/command-presets.tsplugins/fusion-plugin-quality/src/runner/command-runner.tsplugins/fusion-plugin-quality/src/settings.tsplugins/fusion-plugin-quality/src/store/quality-store.tsplugins/fusion-plugin-quality/src/store/quality-types.tsplugins/fusion-plugin-quality/src/suggestions/heuristic-cases.tsplugins/fusion-plugin-quality/tsconfig.jsonplugins/fusion-plugin-quality/vitest.config.tspnpm-workspace.yaml
Collaborator
Author
CodeRabbit review follow-upInline findings (6) from the earlier review body were already addressed on this branch ( Nitpicks (2) just landed in the latest commit:
Greptile follow-up remains at confidence 5/5 with no blocking issues. Plugin unit suite: 17 passing. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
plugins/fusion-plugin-quality/src/runner/command-runner.ts (2)
59-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnreference trailing
SIGKILLtimers.Both sites schedule a 2-second fallback timer to forcefully kill the process. Unreferencing these timers ensures they do not artificially keep the Node.js event loop alive if the server or test runner attempts a graceful shutdown before they fire.
plugins/fusion-plugin-quality/src/runner/command-runner.ts#L59-L61: Append.unref()to thesetTimeoutcall.plugins/fusion-plugin-quality/src/runner/command-runner.ts#L112-L114: Append.unref()to thesetTimeoutcall.🤖 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 `@plugins/fusion-plugin-quality/src/runner/command-runner.ts` around lines 59 - 61, Update both trailing SIGKILL fallback timers in plugins/fusion-plugin-quality/src/runner/command-runner.ts at lines 59-61 and 112-114 to unreference their setTimeout handles with unref(), so they do not keep the Node.js event loop alive during graceful shutdown.
101-106: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSet UTF-8 encoding on standard streams to prevent multibyte character corruption.
When standard streams emit raw
Bufferchunks, converting them to strings individually viaString(chunk)can corrupt multibyte characters (like emojis or international text) if they happen to be split across a chunk boundary. CallingsetEncoding("utf8")instructs the stream to safely buffer incomplete characters.♻️ Proposed refactor
const child = supervised.child; + child.stdout?.setEncoding("utf8"); + child.stderr?.setEncoding("utf8"); - child.stdout?.on("data", (chunk: Buffer | string) => { + child.stdout?.on("data", (chunk: string) => { stdout = truncate(stdout + String(chunk), opts.logTruncateKb); }); - child.stderr?.on("data", (chunk: Buffer | string) => { + child.stderr?.on("data", (chunk: string) => { stderr = truncate(stderr + String(chunk), opts.logTruncateKb); });🤖 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 `@plugins/fusion-plugin-quality/src/runner/command-runner.ts` around lines 101 - 106, Update the child process stream setup near the stdout and stderr data handlers to call setEncoding("utf8") on both standard streams before registering their listeners. Keep the existing truncation and accumulation logic in the stdout and stderr handlers unchanged.
🤖 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/plugin-sdk-core-runtime-shim.ts`:
- Around line 97-101: Update both SIGKILL escalation timers in kill() at
packages/cli/src/plugin-sdk-core-runtime-shim.ts:97-101 and the lifetimeTimer
block at packages/cli/src/plugin-sdk-core-runtime-shim.ts:77-80 to invoke
unref?.() on the returned timeout, allowing the CLI to exit without waiting for
killGraceMs while preserving the existing escalation behavior.
- Around line 84-88: Attach a no-op "error" listener to the spawned child
alongside the existing close handler in the child process lifecycle setup, so
spawn failures are safely absorbed while the existing close callback continues
resolving waitExit and cleaning up the lifetime timer.
---
Nitpick comments:
In `@plugins/fusion-plugin-quality/src/runner/command-runner.ts`:
- Around line 59-61: Update both trailing SIGKILL fallback timers in
plugins/fusion-plugin-quality/src/runner/command-runner.ts at lines 59-61 and
112-114 to unreference their setTimeout handles with unref(), so they do not
keep the Node.js event loop alive during graceful shutdown.
- Around line 101-106: Update the child process stream setup near the stdout and
stderr data handlers to call setEncoding("utf8") on both standard streams before
registering their listeners. Keep the existing truncation and accumulation logic
in the stdout and stderr handlers unchanged.
🪄 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 Plus
Run ID: e51814ad-1ffb-4f89-aabc-1d96636ce4e4
📒 Files selected for processing (11)
packages/cli/src/plugin-sdk-core-runtime-shim.tsplugins/fusion-plugin-quality/manifest.jsonplugins/fusion-plugin-quality/src/__tests__/cancel-and-plans.test.tsplugins/fusion-plugin-quality/src/__tests__/command-presets.test.tsplugins/fusion-plugin-quality/src/dashboard-view.tsxplugins/fusion-plugin-quality/src/preview/preview-sessions.tsplugins/fusion-plugin-quality/src/qa-tab.tsxplugins/fusion-plugin-quality/src/routes/create-routes.tsplugins/fusion-plugin-quality/src/runner/command-presets.tsplugins/fusion-plugin-quality/src/runner/command-runner.tsplugins/fusion-plugin-quality/src/store/quality-store.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- plugins/fusion-plugin-quality/manifest.json
- plugins/fusion-plugin-quality/src/tests/command-presets.test.ts
- plugins/fusion-plugin-quality/src/preview/preview-sessions.ts
- plugins/fusion-plugin-quality/src/runner/command-presets.ts
- plugins/fusion-plugin-quality/src/store/quality-store.ts
- plugins/fusion-plugin-quality/src/routes/create-routes.ts
- plugins/fusion-plugin-quality/src/qa-tab.tsx
gsxdsm
force-pushed
the
feature/qa-workflow
branch
from
July 15, 2026 19:50
7a917bb to
e9f59b0
Compare
gsxdsm
force-pushed
the
feature/qa-workflow
branch
from
July 15, 2026 20:26
9d690c1 to
a02854f
Compare
Ship bundled fusion-plugin-quality for task-scoped QA: preview servers, allowlisted test runs with reports, screenshots CTA, suggested cases, and a project Quality hub. Host injects task context into task-detail plugin tabs; superviseSpawn is re-exported on the plugin packaging shim.
Kill supervised children on cancel and preserve cancelled status; use server default preview script; reject unknown plan steps; isolate QA section fetches with allSettled; prune finished preview sessions.
Enforce shim killGraceMs/maxLifetimeMs; harden file-scoped path tokens; surface hub fetch errors; wait for preview process exit on stop.
Validate plan steps with isQualityPresetId when reading from store, and sync dashboard view description / UI slot order in manifest.json.
Ship bundled fusion-plugin-quality for task-scoped QA: preview servers, allowlisted test runs with reports, screenshots CTA, suggested cases, and a project Quality hub. Host injects task context into task-detail plugin tabs; superviseSpawn is re-exported on the plugin packaging shim.
Fusion-Task-Id: FN-2127
Fusion-Task-Id: FN-2139
gsxdsm
force-pushed
the
feature/qa-workflow
branch
from
July 15, 2026 21:01
a02854f to
ae161ab
Compare
Fusion-Task-Id: FN-2139
Fusion-Task-Id: FN-2127
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/plans/2026-07-14-001-feat-quality-plugin-plan.md`:
- Line 133: Clarify the concurrency model in the plan’s concurrency statement
and the “one active project run” wording: project-root hub runs must use the
project:{projectId} scope, while task-scoped runs must use task:{taskId},
allowing runs for different tasks in the same project concurrently. Add a test
covering concurrent runs across distinct tasks.
In `@packages/cli/src/__tests__/plugin-sdk-core-runtime-shim.test.ts`:
- Around line 38-57: Extend the test for superviseSpawn’s SIGKILL escalation
timers to emit the child’s close event before invoking the captured escalation
callback, then assert that the child receives no SIGKILL after exit. Preserve
the existing timer-unref assertions and cover both explicit-kill and
max-lifetime termination paths if those are exposed by the test setup.
In `@packages/cli/src/plugin-sdk-core-runtime-shim.mjs`:
- Around line 48-51: Make the maximum-lifetime escalation settlement-aware in
packages/cli/src/plugin-sdk-core-runtime-shim.mjs around lines 48-51 by
preventing the SIGKILL callback from calling killProcess after the child has
settled, or by clearing the escalation timer on close. Update
packages/cli/src/__tests__/plugin-sdk-core-runtime-shim.test.ts around lines
38-57 to emit close, invoke the escalation callback, and assert that no kill
occurs.
🪄 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 Plus
Run ID: a76fde6d-c625-4d3a-850d-60b4dcb06454
📒 Files selected for processing (15)
.changeset/friendly-shims-fix.md.changeset/quality-plugin-mvp.md.changeset/safe-plugin-spawn.mdCONCEPTS.mddocs/plans/2026-07-14-001-feat-quality-plugin-plan.mdpackages/cli/src/__tests__/plugin-sdk-core-runtime-shim.test.tspackages/cli/src/__tests__/plugin-sdk-export.test.tspackages/cli/src/plugin-sdk-core-runtime-shim.mjspackages/cli/src/plugins/staged-bundled-plugin-ids.tspackages/cli/tsup.config.tspackages/core/src/__tests__/postgres/plugin-schema-hook.test.tspackages/core/src/plugins/bundled-plugin-install.tspackages/dashboard/app/components/PluginManager.tsxpackages/dashboard/app/components/PluginSlot.tsxpackages/dashboard/app/components/TaskDetailModal.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- .changeset/quality-plugin-mvp.md
- packages/cli/src/plugins/staged-bundled-plugin-ids.ts
- packages/core/src/plugins/bundled-plugin-install.ts
- packages/cli/tsup.config.ts
- packages/dashboard/app/components/PluginSlot.tsx
- CONCEPTS.md
- packages/dashboard/app/components/TaskDetailModal.tsx
|
|
||
| 7. **cwd policy: hub = project root; task tab = task worktree if present, otherwise block with clear CTA (optional advanced “run on root”).** Rationale: silent root fallback produces false greens for unbuilt task branches. | ||
|
|
||
| 8. **Concurrency: at most one active project-scoped command run and one active run per task; 409 when busy.** Rationale: avoid thrashing shared resources; cancel reaps process groups. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clarify project versus task concurrency scope.
Line 458’s “one active project run” conflicts with Line 133 and the concurrency table at Lines 226–228. If every task-tab run is treated as a project run, QA runs for different tasks will be incorrectly serialized. Specify that project-root hub runs use project:{projectId}, while task-scoped runs use task:{taskId}, and add a cross-task concurrency test.
Also applies to: 458-458
🤖 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/plans/2026-07-14-001-feat-quality-plugin-plan.md` at line 133, Clarify
the concurrency model in the plan’s concurrency statement and the “one active
project run” wording: project-root hub runs must use the project:{projectId}
scope, while task-scoped runs must use task:{taskId}, allowing runs for
different tasks in the same project concurrently. Add a test covering concurrent
runs across distinct tasks.
gsxdsm
force-pushed
the
feature/qa-workflow
branch
from
July 16, 2026 00:55
1c111e3 to
83d80e1
Compare
Fusion-Task-Id: FN-2127
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugins/fusion-plugin-quality/src/routes/create-routes.ts (2)
155-166: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard against falsy returns from
getTask. Ifctx.taskStore.getTask(taskId)returnsundefinedornullinstead of throwing an error when a task is not found, accessing its properties (liketask.worktreeortask.prompt) will cause an unhandledTypeError(HTTP 500) rather than the intended HTTP 404.
plugins/fusion-plugin-quality/src/routes/create-routes.ts#L155-L166: Addif (!task) httpError(404, "Task not found");immediately after thetry/catchblock.plugins/fusion-plugin-quality/src/routes/create-routes.ts#L306-L316: Add the same guard immediately after thetry/catchblock.plugins/fusion-plugin-quality/src/routes/create-routes.ts#L360-L365: Add the same guard immediately after thetry/catchblock.🤖 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 `@plugins/fusion-plugin-quality/src/routes/create-routes.ts` around lines 155 - 166, Add a falsy-task guard immediately after each getTask try/catch in plugins/fusion-plugin-quality/src/routes/create-routes.ts at lines 155-166, 306-316, and 360-365, returning httpError(404, "Task not found") when task is undefined or null before any property access.
224-235: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPrevent unhandled promise rejections in the detached execution chain.
If
store.pruneRunsthrows an error, the chain jumps to the.catchblock and incorrectly marks a successful run as a failure. More importantly, ifstore.updateRunthrows an error within the.catchblock (e.g., due to a transient database lock), it causes an unhandled promise rejection which will crash the Node.js process.🛡️ Proposed fix to isolate failures
.then(() => { - store.pruneRuns(projectId, getRunRetentionCount(ctx.settings as Record<string, unknown>)); + try { + store.pruneRuns(projectId, getRunRetentionCount(ctx.settings as Record<string, unknown>)); + } catch (err) { + ctx.logger?.warn?.(`Prune runs failed: ${err instanceof Error ? err.message : String(err)}`); + } }) .catch((err) => { ctx.logger?.warn?.( `Quality run ${run.id} failed: ${err instanceof Error ? err.message : String(err)}`, ); - store.updateRun(projectId, run.id, { - status: "error", - errorMessage: err instanceof Error ? err.message : String(err), - finishedAt: new Date().toISOString(), - }); + try { + store.updateRun(projectId, run.id, { + status: "error", + errorMessage: err instanceof Error ? err.message : String(err), + finishedAt: new Date().toISOString(), + }); + } catch (updateErr) { + ctx.logger?.error?.(`Failed to update run status: ${updateErr instanceof Error ? updateErr.message : String(updateErr)}`); + } });🤖 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 `@plugins/fusion-plugin-quality/src/routes/create-routes.ts` around lines 224 - 235, Update the detached execution chain around store.pruneRuns and its catch handler so pruning failures do not mark an otherwise successful run as failed, and failures from store.updateRun are contained rather than becoming unhandled rejections. Separate or locally handle pruneRuns errors, and add a guarded error path around the failure-status update while preserving the existing logging and run-state behavior.plugins/fusion-plugin-quality/src/preview/preview-sessions.ts (1)
159-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent
closehandler from overwriting intentionalstoppedstate.When the session is intentionally stopped, the child process is killed via
SIGTERM, resulting in anullexit code. The current logic incorrectly treats anullcode as a failure and blindly overwrites the intentional"stopped"status with"failed". Ensure the status is only set to failed if it wasn't already marked as stopped, and correctly handle the signal string.🐛 Proposed fix to respect intentional stops and handle signals
- supervised.child.on("close", (code) => { - session.status = code === 0 ? "stopped" : "failed"; - session.stoppedAt = new Date().toISOString(); - if (code && code !== 0) { - session.errorMessage = `Exited with code ${code}`; - } + supervised.child.on("close", (code, signal) => { + if (session.status !== "stopped") { + session.status = code === 0 ? "stopped" : "failed"; + if (code !== null && code !== 0) { + session.errorMessage = `Exited with code ${code}`; + } else if (signal) { + session.errorMessage = `Killed by signal ${signal}`; + } + } + session.stoppedAt = new Date().toISOString(); session.supervised = undefined; pruneTerminalPreviewSessions(sessions); });🤖 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 `@plugins/fusion-plugin-quality/src/preview/preview-sessions.ts` around lines 159 - 167, Update the `supervised.child.on("close")` handler to preserve an existing `"stopped"` session status when termination was intentional. Only assign `"failed"` for nonzero exits when the status is not already `"stopped"`, and handle signal-based termination using the close callback’s signal value rather than treating a `null` exit code as a failure.
🤖 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.
Outside diff comments:
In `@plugins/fusion-plugin-quality/src/preview/preview-sessions.ts`:
- Around line 159-167: Update the `supervised.child.on("close")` handler to
preserve an existing `"stopped"` session status when termination was
intentional. Only assign `"failed"` for nonzero exits when the status is not
already `"stopped"`, and handle signal-based termination using the close
callback’s signal value rather than treating a `null` exit code as a failure.
In `@plugins/fusion-plugin-quality/src/routes/create-routes.ts`:
- Around line 155-166: Add a falsy-task guard immediately after each getTask
try/catch in plugins/fusion-plugin-quality/src/routes/create-routes.ts at lines
155-166, 306-316, and 360-365, returning httpError(404, "Task not found") when
task is undefined or null before any property access.
- Around line 224-235: Update the detached execution chain around
store.pruneRuns and its catch handler so pruning failures do not mark an
otherwise successful run as failed, and failures from store.updateRun are
contained rather than becoming unhandled rejections. Separate or locally handle
pruneRuns errors, and add a guarded error path around the failure-status update
while preserving the existing logging and run-state behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f545e5f9-3957-480c-8662-b76a141725e7
📒 Files selected for processing (6)
packages/cli/src/__tests__/plugin-sdk-core-runtime-shim.test.tspackages/cli/src/plugin-sdk-core-runtime-shim.mjspackages/dashboard/app/components/SettingsModal.tsxplugins/fusion-plugin-quality/src/__tests__/preview-sessions.test.tsplugins/fusion-plugin-quality/src/preview/preview-sessions.tsplugins/fusion-plugin-quality/src/routes/create-routes.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/tests/plugin-sdk-core-runtime-shim.test.ts
- packages/cli/src/plugin-sdk-core-runtime-shim.mjs
gsxdsm
added a commit
that referenced
this pull request
Jul 16, 2026
Rebase onto main after #2127: keep only unmerged Quality plugin deltas — await async settings for the experimental gate, disposable QA worktrees for done tasks, and ViewHeader/layout parity with other dashboard views.
5 tasks
gsxdsm
added a commit
that referenced
this pull request
Jul 16, 2026
…out (#2164) ## Summary Follow-up on **#2127** (Quality plugin already on `main`). This PR only lands the remaining Quality deltas that were not merged: - **Experimental gate fix** — `TaskStore.getSettings()` is async; the gate now awaits merged settings so enabling `experimentalFeatures.qualityPlugin` actually works, and status-bearing errors return structured `{ status, body }` instead of collapsing to hard failures - **Done-task QA worktrees** — when a task has no live worktree (typical after land), preview/task runs create a disposable checkout under `.fusion/quality-qa/` at the task branch or merge commit so processes run the **done task’s code**, not project root - **Hub layout** — shared `ViewHeader` + dashboard spacing/typography so Quality matches Insights / Compound Engineering / Goals Scoped to `plugins/fusion-plugin-quality/**` only (rebased onto current `main`; duplicate plugin-landing commits dropped). ## Test plan - [ ] Enable **Settings → Experimental → Quality Plugin**, restart if routes were cold - [ ] Quality hub: header matches other views; refresh + presets work - [ ] Done task → QA tab → Start preview uses QA worktree at branch/merge commit (not project root) - [ ] Active task with live worktree still uses that worktree - [ ] Flag off: clear experimental-disabled error (not generic empty failure) - [ ] `pnpm --filter @fusion-plugin-examples/quality test` (32 tests)
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a bundled Quality plugin (
fusion-plugin-quality) that makes task QA easier and more visual:taskId, worktree,projectId) so plugin tabs can scope correctlysuperviseSpawnre-exported on the plugin packaging shim for published pluginsdocs/plans/2026-07-14-001-feat-quality-plugin-plan.mdDesign constraints
Test plan
pnpm --filter @fusion-plugin-examples/quality test(15 tests)Residual / follow-up (same plan, later units)
Summary by CodeRabbit
New Features
Documentation
Bug Fixes