Conversation
… banner (#472) - Gates trigger automatically (non-blocking) when execution status is 'completed' - useRef guard prevents duplicate runs across re-renders - CompletionBanner completed state shows gate summary below action buttons: - Running: "Running quality gates..." - All passed: "✓ All N gates passed" - Some failed: "⚠ N/M gates passed · View full report →" (links to /review) - Error: "Gate check unavailable · View in Review →" - Extend CompletionBannerProps with gateResult, gateRunning, gateError fields
WalkthroughAutomatically run gate checks once a task execution completes and surface a preliminary gate summary (running state, pass/fail counts, or error) in the CompletionBanner via a new GateSummary component and added gate-related props. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as ExecutionPage
participant Monitor as TaskMonitor
participant API as GatesAPI
participant Banner as CompletionBanner
participant Summary as GateSummary
Monitor->>Page: completionStatus = "completed"
Page->>Page: if not hasRunGatesRef -> set hasRunGatesRef
Page->>API: gatesApi.run(workspacePath)
activate API
Page->>Page: set gateRunning = true
API-->>Page: return gateResult / error
deactivate API
Page->>Page: set gateResult, gateRunning = false, gateError?
Page->>Banner: render with gateResult, gateRunning, gateError
Banner->>Summary: render summary (running / passed counts / link to /review)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Review — PR #494: Auto-run gates on execution completionOverall this is a well-scoped, clean implementation. The Bug: Wrong color class on gate error stateIn // line 199 — should NOT be green-700 for an error state
<p className="mt-2 text-xs text-green-700 dark:text-green-300">
Gate check unavailable · ...It should be Logic inconsistency:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/app/execution/`[taskId]/page.tsx:
- Around line 197-203: The gateError branch currently renders the unavailable
state using green "success" styling; update the JSX returned when gateError is
truthy (the conditional handling around gateError in the page component) to use
warning or destructive classes instead of "text-green-700 dark:text-green-300"
(for example switch to a yellow/orange or red text class) so the
unavailable/error state uses non-success styling and visually indicates a
problem; keep the message and Link component unchanged.
- Around line 54-62: The effect that runs gates waits for a state update so the
first render after monitor.completionStatus becomes 'completed' shows a blank
GateSummary; change GateSummary to compute the "pending" state from
monitor.completionStatus plus existing gateResult/gateError (i.e., treat pending
= monitor.completionStatus === 'completed' && !gateResult && !gateError) instead
of relying solely on gateRunning, and keep the useEffect as the trigger that
sets hasRunGatesRef and calls gatesApi.run(workspacePath) to populate
setGateResult/setGateError; update any conditional rendering in GateSummary to
use this derived pending condition so the UI shows the pending state immediately
on first completed render.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f0e2b23-620c-4361-ba35-11547a687872
📒 Files selected for processing (2)
web-ui/src/app/execution/[taskId]/page.tsxweb-ui/src/types/index.ts
Code ReviewThe overall approach is sound — Critical (confirmed bugs)1. Wrong color on "gate running" state ( The 2. SKIPPED gates cause false-negative fraction display ( const passed = gateResult.checks.filter((c) => c.status === 'PASSED').length;
Important3. If const showGatePending = monitor.completionStatus === 'completed' && !!workspacePath && !gateResult && !gateError;4. Error silently swallowed in catch ( .catch(() => setGateError(true))The error object is discarded entirely. This makes debugging impossible in DevTools. At minimum: .catch((err) => {
console.error('Gates auto-run failed:', err);
setGateError(true);
})Nice to Have
Block on: Issues 1 and 2. Recommend fixing: Issues 3 and 4. Issues 5–8 are optional polish. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/app/execution/`[taskId]/page.tsx:
- Around line 244-248: The success banner text still tells users to manually
"Run PROOF9 gates", which is stale; update the JSX paragraph that renders
"Execution complete{durationText && ` in ${durationText}`}. Run PROOF9 gates to
verify quality before shipping." (the <p> element containing that string and the
durationText interpolation) to reflect that gates run automatically—e.g., change
to "Execution complete{durationText && ` in ${durationText}`}. PROOF9 gates have
been run automatically to verify quality." or similar concise wording so the
banner no longer implies a manual action.
- Around line 26-30: When calling gatesApi.run() (the code path that updates
gateResult via setGateResult, gateError via setGateError, and gateRunning via
setGateRunning and uses hasRunGatesRef) wrap the async completion handlers with
a local cancelled flag: set cancelled = false before starting the promise, and
in the effect cleanup set cancelled = true; after the gatesApi.run() promise
resolves or rejects, check if cancelled is true before calling setGateResult,
setGateError, or setGateRunning to avoid updating state for a stale taskId; if
the API supports AbortSignal prefer passing an abort token and aborting it in
the cleanup.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d58bd446-fc66-4b1e-83a0-e4d0b66e95d3
📒 Files selected for processing (1)
web-ui/src/app/execution/[taskId]/page.tsx
| // Gate auto-run state | ||
| const [gateResult, setGateResult] = useState<GateResult | null>(null); | ||
| const [gateRunning, setGateRunning] = useState(false); | ||
| const [gateError, setGateError] = useState(false); | ||
| const hasRunGatesRef = useRef(false); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Next.js App Router, when navigating between two /execution/[taskId]routes handled by the same clientpage.tsx, is local component state preserved across taskId changes by default, or is the page remounted?
💡 Result:
In the Next.js App Router, navigating from /execution/1 to /execution/2 (i.e., changing the dynamic path param in the same [taskId]/page.tsx) will unmount and remount the page subtree, so local component state is not preserved by default. (github.com)
Citations:
🏁 Script executed:
cat -n web-ui/src/app/execution/[taskId]/page.tsx | head -100Repository: frankbria/codeframe
Length of output: 4593
🏁 Script executed:
find web-ui/src/app/execution -name "layout.tsx" -o -name "layout.ts"Repository: frankbria/codeframe
Length of output: 45
🏁 Script executed:
cat -n web-ui/src/hooks/useExecutionMonitor.tsRepository: frankbria/codeframe
Length of output: 7865
Handle stale promise completions from gatesApi.run() when navigating between tasks.
The gatesApi.run() call has no cleanup function, so if you navigate away (changing taskId) before the promise resolves, the old promise will still call setGateResult(), setGateError(), and setGateRunning() on the unmounted component. Add a cleanup function with a cancelled flag to prevent stale state updates.
💡 Suggested fix
// Auto-run gates non-blocking when execution completes
useEffect(() => {
if (monitor.completionStatus !== 'completed' || !workspacePath || hasRunGatesRef.current) return;
hasRunGatesRef.current = true;
setGateRunning(true);
+ let cancelled = false;
gatesApi.run(workspacePath)
- .then(setGateResult)
- .catch(() => setGateError(true))
- .finally(() => setGateRunning(false));
+ .then((result) => {
+ if (!cancelled) setGateResult(result);
+ })
+ .catch(() => {
+ if (!cancelled) setGateError(true);
+ })
+ .finally(() => {
+ if (!cancelled) setGateRunning(false);
+ });
+
+ return () => {
+ cancelled = true;
+ };
}, [monitor.completionStatus, workspacePath]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/src/app/execution/`[taskId]/page.tsx around lines 26 - 30, When
calling gatesApi.run() (the code path that updates gateResult via setGateResult,
gateError via setGateError, and gateRunning via setGateRunning and uses
hasRunGatesRef) wrap the async completion handlers with a local cancelled flag:
set cancelled = false before starting the promise, and in the effect cleanup set
cancelled = true; after the gatesApi.run() promise resolves or rejects, check if
cancelled is true before calling setGateResult, setGateError, or setGateRunning
to avoid updating state for a stale taskId; if the API supports AbortSignal
prefer passing an abort token and aborting it in the cleanup.
| <div role="alert" className="rounded-lg border border-green-200 bg-green-50 px-4 py-3 dark:border-green-900 dark:bg-green-950/30"> | ||
| <div className="flex items-center justify-between"> | ||
| <p className="text-sm font-medium text-green-800 dark:text-green-200"> | ||
| Execution complete{durationText && ` in ${durationText}`}. Run PROOF9 gates to verify quality before shipping. | ||
| </p> |
There was a problem hiding this comment.
The completed copy still implies a manual gate run.
This state now auto-runs gates in the background, so “Run PROOF9 gates” is stale and conflicts with the summary rendered underneath.
✏️ Suggested wording
- Execution complete{durationText && ` in ${durationText}`}. Run PROOF9 gates to verify quality before shipping.
+ Execution complete{durationText && ` in ${durationText}`}. Review the PROOF9 gate summary before shipping.📝 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.
| <div role="alert" className="rounded-lg border border-green-200 bg-green-50 px-4 py-3 dark:border-green-900 dark:bg-green-950/30"> | |
| <div className="flex items-center justify-between"> | |
| <p className="text-sm font-medium text-green-800 dark:text-green-200"> | |
| Execution complete{durationText && ` in ${durationText}`}. Run PROOF9 gates to verify quality before shipping. | |
| </p> | |
| <div role="alert" className="rounded-lg border border-green-200 bg-green-50 px-4 py-3 dark:border-green-900 dark:bg-green-950/30"> | |
| <div className="flex items-center justify-between"> | |
| <p className="text-sm font-medium text-green-800 dark:text-green-200"> | |
| Execution complete{durationText && ` in ${durationText}`}. Review the PROOF9 gate summary before shipping. | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web-ui/src/app/execution/`[taskId]/page.tsx around lines 244 - 248, The
success banner text still tells users to manually "Run PROOF9 gates", which is
stale; update the JSX paragraph that renders "Execution complete{durationText &&
` in ${durationText}`}. Run PROOF9 gates to verify quality before shipping."
(the <p> element containing that string and the durationText interpolation) to
reflect that gates run automatically—e.g., change to "Execution
complete{durationText && ` in ${durationText}`}. PROOF9 gates have been run
automatically to verify quality." or similar concise wording so the banner no
longer implies a manual action.
1 participant
Summary
useEffectwhen execution status becomescompleteduseRefguard ensures gates run exactly once per completion — not on re-rendersGateSummarysub-component renders below action buttons in the completed state banner:/review)CompletionBannerPropsextended withgateResult?,gateRunning?,gateError?Closes #472
Test plan
npm run buildclean)Summary by CodeRabbit