Agent cancellation support and improve agent feedback ui/ux

[JayGhiya]

No description provided.

[huly-for-github]

Connected to Huly®: UNOPL-1461

[claude]

Claude finished @JayGhiya's task —— View job

---

PR Review: Agent Cancellation Support and UI/UX Improvements

This is a substantial PR implementing two key features: workflow cancellation for agent operations and UI/UX improvements to agent feedback. I've conducted a comprehensive review across code quality, potential bugs, performance, security, and test coverage.

✅ Strengths

Excellent Architecture & Safety

• Robust cancellation logic: The cancellation_helpers.py module properly handles both temporalio.CancelledError and asyncio.CancelledError with cause chain traversal up to 8 levels deep
• Strong security guardrails: Backend enforces operation-level restrictions (only AGENTS_GENERATION/AGENT_MD_UPDATE can be cancelled) and terminal state validation
• Comprehensive error handling: API returns deterministic responses for all edge cases (not found, already terminal, non-cancellable, runtime failures)

Solid UI/UX Design

• Clean thumbs up/down interface: Replaces complex emoji selector with binary ThumbsRatingSelector using proper accessibility attributes
• Consistent design patterns: Uses existing UI components (ToggleGroup, lucide-react icons) following established conventions
• Progressive enhancement: Graceful fallback when is_cancellable is undefined (?? true)

⚠️ Areas of Concern

1. Missing Input Validation (Medium Priority)

Location: unoplat-code-confluence-query-engine/src/unoplat_code_confluence_query_engine/api/v1/endpoints/codebase_agent_rules.py:461

async def cancel_repository_agent_run(
    owner_name: str = Query(...),
    repo_name: str = Query(...), 
    repository_workflow_run_id: str = Query(...),

Issue: No input validation on query parameters. Malformed UUIDs or special characters could cause downstream errors.

Recommendation: Add Pydantic validation:

from pydantic import Field, validator

owner_name: str = Query(..., regex=r"^[a-zA-Z0-9._-]+$", min_length=1, max_length=255)
repo_name: str = Query(..., regex=r"^[a-zA-Z0-9._-]+$", min_length=1, max_length=255)  
repository_workflow_run_id: str = Query(..., regex=r"^[0-9a-f-]+$", min_length=36, max_length=36)

2. Race Condition in Frontend (Low Priority)

Location: unoplat-code-confluence-frontend/src/components/custom/GenerateAgentsDialog.tsx:262

const canCancelWorkflow =
  !!actualJob &&
  isAgentWorkflowOperation(actualJob.operation) &&
  isRunning &&
  (actualJob.is_cancellable ?? true);

Issue: Between checking isRunning and calling the cancel API, the workflow could transition to completed state.

Recommendation: The backend properly handles this with terminal state validation, so this is acceptable. Consider adding optimistic UI updates to show "cancellation pending" state.

3. Test Coverage Gaps

Missing Tests:

• No integration tests for the cancel endpoint with Temporal workflows
• Frontend cancel button interaction tests missing
• No tests for the is_parent_workflow_cancellable function logic

Existing Tests:

• ✅ Good unit tests for cancellation_helpers.py covering direct/wrapped/asyncio cancellation detection
• ✅ Proper type safety with Zod schemas in frontend

🔍 Code Quality Assessment

Backend Implementation

• Excellent: Proper async/await usage, structured error logging, clean separation of concerns
• Type Safety: Good use of type hints and Pydantic models
• Error Handling: Comprehensive exception management with proper HTTP status codes

Frontend Implementation

• React Best Practices: Proper hooks usage, memoization, and component composition
• Type Safety: Strong TypeScript usage with proper type imports and inference
• Accessibility: Good ARIA labels on interactive elements

🚀 Performance Considerations

Positive:

• Efficient cancellation detection (max 8-level traversal prevents infinite loops)
• Proper query invalidation strategy in React Query
• Minimal re-renders with appropriate memoization

Recommendations:

• Consider debouncing multiple rapid cancel clicks in UI
• Add timeout to Temporal cancellation calls

🔒 Security Review

Strong Security Posture:

• ✅ Operation-level authorization (INGESTION workflows blocked)
• ✅ Backend validation prevents UI bypass attempts
• ✅ No sensitive data exposure in error responses
• ✅ Proper async cancellation without resource leaks

📊 Test Coverage Analysis

Current Coverage: Limited but targeted

• Unit tests for core cancellation logic ✅
• Missing integration and E2E tests ⚠️

Recommended Additions:

# Test cancellation endpoint with real Temporal workflow
async def test_cancel_repository_agent_run_integration():
    # Start agent workflow, verify cancel transitions to CANCELLED status

# Test is_cancellable derivation logic
def test_is_parent_workflow_cancellable_logic():
    # Test all operation/status combinations

🎯 Final Recommendation

APPROVE with Minor Suggestions

This PR demonstrates strong engineering practices with robust error handling, security considerations, and clean architecture. The two identified issues are minor and don't block the core functionality.

Priority Actions:

1. Add input validation to the cancel endpoint (5-min fix)
2. Add integration tests for the cancel workflow (future sprint)
3. Consider optimistic UI updates for better UX

The implementation successfully delivers both agent cancellation and improved feedback UI while maintaining code quality and system integrity.

---

Reviewed Files: 65 files across backend (query-engine, ingestion), frontend, and commons
Key Changes: Agent workflow cancellation, thumbs rating UI, cancellation safety mechanisms

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9640f562c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add DB migration before writing CANCELLED workflow states

This change extends the ORM check constraint to include CANCELLED, but there is no accompanying schema migration for existing databases; startup uses SQLBase.metadata.create_all (query-engine main.py), which does not alter existing check constraints. As soon as cancellation paths persist JobStatus.CANCELLED, upgraded environments with the old status_check/codebase_status_check constraints will reject the update at the DB layer, leaving runs in incorrect states and making cancel appear broken.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Keep parent repository_workflow_id immutable on status updates

Blindly rewriting existing.repository_workflow_id from each envelope can corrupt parent workflow identity because codebase failure cascade updates currently send ParentWorkflowDbActivityEnvelope with the child workflow ID (agent_workflow_interceptor.py in _handle_codebase_workflow). With this assignment, a parent run row can be rewritten to a child Temporal ID, so lifecycle actions keyed off repository_workflow_id can target the wrong workflow and stale incorrect IDs may persist if parent finalization does not complete.

Useful? React with 👍 / 👎.