Skip to content

Conversation

@Gkrumbach07
Copy link
Collaborator

Problem

Custom workflows were not being activated after submission - the form dialog would close, but no network call was made to the backend API to actually load the workflow.

Root Cause

When submitting a custom workflow, the code was only calling setCustomWorkflow() which set it as "pending" but never called activateWorkflow(). Out-of-the-box workflows automatically activated, but custom ones didn't.

Solution

Modified the CustomWorkflowDialog submission handler to automatically activate the workflow after setting it as pending. This now:

  1. Sets the workflow as pending
  2. Closes the dialog
  3. Creates a workflow config object
  4. Immediately activates it (makes the POST request to /api/projects/{project}/agentic-sessions/{session}/workflow)

This matches the behavior of OOTB workflows.

Testing

  • ✅ Frontend builds successfully with npm run build
  • ✅ No linting errors
  • Ready to test on dev cluster after deployment

Changes

  • Modified components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx
    • Added automatic activation call in CustomWorkflowDialog onSubmit handler

Previously, custom workflows were only set as pending but never activated,
which meant no network call was made to the backend API. This fix ensures
custom workflows are automatically activated after submission, matching
the behavior of out-of-the-box workflows.

The fix adds an immediate call to activateWorkflow() after setCustomWorkflow(),
which triggers the POST request to /api/projects/{project}/agentic-sessions/{session}/workflow
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

Claude Code Review

Summary

This PR fixes a bug where custom workflows were not being activated after submission. The fix adds an automatic activation call after setting the workflow as pending, matching the behavior of out-of-the-box (OOTB) workflows. The change is focused, minimal, and follows the established pattern.

Issues by Severity

🚫 Blocker Issues

None identified.

🔴 Critical Issues

None identified.

🟡 Major Issues

1. Duplicate workflow object creation (lines 2038-2046)

  • Issue: The code creates a new workflow config object inline, but setCustomWorkflow at line 2035 already creates the exact same object structure (see use-workflow-management.ts:127-137).
  • Impact: This duplication means if the WorkflowConfig structure changes, both locations need updating.
  • Recommendation: Instead of creating the workflow object inline, access it from workflowManagement.pendingWorkflow after calling setCustomWorkflow:
onSubmit={(url, branch, path) => {
  workflowManagement.setCustomWorkflow(url, branch, path);
  setCustomWorkflowDialogOpen(false);
  // Automatically activate the custom workflow (same as OOTB workflows)
  // The workflow is now in pendingWorkflow after setCustomWorkflow call above
  if (workflowManagement.pendingWorkflow) {
    workflowManagement.activateWorkflow(
      workflowManagement.pendingWorkflow, 
      session?.status?.phase
    );
  }
}}

2. Execution order creates race condition risk

  • Issue: setCustomWorkflow sets state (setPendingWorkflow), then immediately activateWorkflow is called. Since setCustomWorkflow uses setPendingWorkflow (which is async state update), the pendingWorkflow might not be set when activateWorkflow runs.
  • Current mitigation: You're passing the workflow directly to activateWorkflow, which uses the workflowToActivate parameter instead of reading pendingWorkflow state. This works but creates confusion.
  • Recommendation: Follow the pattern more explicitly - either:

🔵 Minor Issues

1. Frontend standards compliance

  • Status: ✅ PASS - No any types used
  • Status: ✅ PASS - Code uses Shadcn UI components (Dialog, Button)
  • Status: ⚠️ REVIEW NEEDED - The file is 2072 lines, which violates the "components under 200 lines" guideline from frontend-development.md. However, this is a pre-existing issue not introduced by this PR.

2. Missing type safety validation

  • Issue: The inline workflow object at lines 2038-2046 doesn't have explicit type annotation
  • Impact: Minor - TypeScript infers the type, but explicit annotation improves readability
  • Recommendation: Add type annotation:
const customWorkflow: WorkflowConfig = {
  id: "custom",
  // ... rest
};

3. Comment could be more specific (line 2037)

  • Current: "Automatically activate the custom workflow (same as OOTB workflows)"
  • Better: "Automatically activate the custom workflow immediately (matches OOTB behavior in handleWorkflowChange:681)"

Positive Highlights

Follows established patterns: The fix mirrors the OOTB workflow activation pattern used in handleWorkflowChange (lines 674-683), which also calls activateWorkflow immediately after selection.

Clean and focused: The fix is minimal (11 lines) and doesn't introduce unnecessary changes or refactoring.

Proper error handling: Uses the existing workflowManagement.activateWorkflow which has built-in error handling, loading states, and queueing logic for non-Running sessions.

Testing noted: PR description mentions frontend builds successfully and no linting errors.

Good PR description: Clear problem statement, root cause analysis, and solution explanation.

Recommendations

Priority 1 (Before Merge):

  1. Address Major Issue Outcome: Reduce Refinement Time with agent System #1 - Remove duplicate workflow object creation by using pendingWorkflow state or passing workflow directly without calling setCustomWorkflow

Priority 2 (Nice to Have):

  1. Add explicit type annotation to the workflow object
  2. Improve the comment specificity to reference the matching pattern

Priority 3 (Future Refactoring):

  1. Consider extracting the page.tsx file into smaller components (currently 2072 lines, should be under 200 per guidelines)

Testing Recommendations:

  • ✅ Test custom workflow activation on a running session
  • ✅ Test custom workflow activation on a pending session (should queue)
  • ✅ Test error handling when workflow URL is invalid
  • ✅ Verify workflow appears in session spec after activation
  • ✅ Test that closing dialog without submission doesn't activate workflow

Overall Assessment

Status: ✅ APPROVE with suggestions

This is a solid bug fix that correctly addresses the issue. The major issue identified (duplicate object creation) is a code quality concern rather than a functional bug. The current implementation will work correctly due to passing the workflow directly to activateWorkflow, but cleaning up the duplication would improve maintainability.

The fix follows the repository's patterns, uses proper error handling through the existing hook, and maintains consistency with OOTB workflow activation behavior.


🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

@Gkrumbach07 Gkrumbach07 merged commit 1e3db39 into ambient-code:main Jan 14, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant