Feature/add developer roles by valentinuuiuiu · Pull Request #770 · coleam00/Archon

valentinuuiuiu · 2025-10-09T05:49:00Z

Pull Request

Summary

Changes Made

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Performance improvement
Code refactoring

Affected Services

Testing

All existing tests pass
Added new tests for new functionality
Manually tested affected user flows
Docker builds succeed for all services

Test Evidence

# Example: python -m pytest tests/
# Example: cd archon-ui-main && npm run test

Checklist

My code follows the service architecture patterns
If using an AI coding assistant, I used the CLAUDE.md rules
I have added tests that prove my fix/feature works
All new and existing tests pass locally
My changes generate no new warnings
I have updated relevant documentation
I have verified no regressions in existing features

Breaking Changes

Additional Notes

Summary by CodeRabbit

Bug Fixes
- Prevents assigning tasks to unsupported or mistyped assignees. Validation is now strict, ensuring only recognized options are accepted during task creation and editing, improving data integrity and consistency across UI and API.
New Features
- Error messages for invalid assignees are clearer and now list the allowed options, guiding users to make valid selections and reducing retry cycles.

This commit introduces a predefined list of developer roles to the `TaskService`, enabling users to assign tasks to specific teams. The `validate_assignee` method in `task_service.py` has been updated to check against a new `VALID_ASSIGNEES` list, which now includes: - DevOps - Frontend - Backend - API Agent - MCPP Agent This change enhances task management by allowing for more granular assignments, aligning with the user's request to create a developer team structure.

coderabbitai · 2025-10-09T05:49:35Z

Walkthrough

Introduces a new class attribute VALID_ASSIGNEES in TaskService and tightens validate_assignee to require membership in this set, changing validation logic and error messaging to enumerate allowed assignees.

Changes

Cohort / File(s)	Summary
Task assignee validation `python/src/server/services/projects/task_service.py`	Added class attribute `VALID_ASSIGNEES`. Updated `validate_assignee` to check membership in `VALID_ASSIGNEES` and return an error listing allowed values; previously allowed any non-empty string. No new methods or signature changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Caller
    participant TS as TaskService
    rect rgba(230,245,255,0.6)
    note right of TS: New stricter validation
    C->>TS: validate_assignee(assignee)
    TS->>TS: Check assignee in VALID_ASSIGNEES
    alt Assignee allowed
        TS-->>C: return OK
    else Assignee invalid
        TS-->>C: return error (message lists allowed values)
    end
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tap my paw, approve the list,
VALID_ASSIGNEES can’t be missed.
No more strings that go astray,
Only names that pass the way.
Hippity-hop, the checks are tight—
Tasks assigned exactly right! 🐇✅

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description is just the untouched template with blank sections and placeholders, providing no actual summary, change details, testing information, or checklist updates, so it fails to document the intended work.	Please fill out each section of the PR template with a concise summary of the feature, a list of the actual changes made, the type of change, affected services, testing steps and evidence, completed checklist items, and any breaking changes or additional notes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title “Feature/add developer roles” does reference the new restriction on valid task assignees (roles) but does not clearly summarize that the change adds a VALID_ASSIGNEES list and stricter validation in TaskService, making it somewhat broad but still related to the feature introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a580fdf and 09c7dd1.

📒 Files selected for processing (1)

python/src/server/services/projects/task_service.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

python/src/server/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/src/server/**/*.py: Never accept corrupted data in batch/continuation paths: skip failed items entirely (e.g., do not store zero embeddings, null FKs, or malformed JSON)
Use specific exception types; avoid catching bare Exception
Preserve full stack traces in logs (use exc_info=True with Python logging)
Never return None to indicate failure; raise an exception with details
Authentication/authorization failures must halt the operation and be clearly surfaced
Service startup, missing configuration, database connection, or critical dependency failures should crash fast with clear errors
During crawling/batch/background tasks and WebSocket events, continue processing other items but log failures with context
Include context (operation intent, relevant IDs/URLs/data) in error messages
Pydantic should raise on data corruption or validation errors; do not accept invalid inputs

Files:

python/src/server/services/projects/task_service.py

python/src/server/services/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/src/server/services/**/*.py: For batch operations, report both success count and a detailed failure list
On external API calls, retry with exponential backoff and fail with a clear message after retries
Place business logic in service layer modules (API Route → Service → Database pattern)

Files:

python/src/server/services/projects/task_service.py

python/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting (errors, warnings, unused imports)

Files:

python/src/server/services/projects/task_service.py

python/src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use MyPy for type checking to ensure type safety

python/src/**/*.py: On service startup, missing configuration, DB connection failures, auth/authorization failures, critical dependency outages, or invalid/corrupting data: fail fast and bubble errors
For batch processing, background tasks, WebSocket events, optional features, and external API calls: continue processing but log errors (with retries/backoff for APIs)
Never accept or persist corrupted data; skip failed items entirely (e.g., zero embeddings, null FKs, malformed JSON)
Error messages must include operation context, IDs/URLs, use specific exception types, preserve full stack traces (logging with exc_info=True), and avoid returning None/null—raise exceptions instead; for batches report success counts and detailed failures
Backend code targets Python 3.12 and adheres to a 120 character line length
Use Ruff for linting (errors, warnings, unused imports) in backend code
Use Mypy for static type checking in backend code

Files:

python/src/server/services/projects/task_service.py

🔇 Additional comments (2)

python/src/server/services/projects/task_service.py (2)
49-56: Implementation looks good, but coordinate with data migration.

The validation logic is correct and follows the same pattern as validate_status. The error message clearly enumerates allowed assignees.

However, ensure this change is coordinated with the data migration identified in the previous comment. Consider:

Adding a feature flag to enable strict validation after migration

Logging warnings for invalid assignees before enforcing validation

Providing a clear migration path for existing tasks

25-34: Verify existing task assignees in the database against VALID_ASSIGNEES
This change restricts assignees to a fixed list and may break updates on existing tasks whose assignees aren’t in VALID_ASSIGNEES. Run a SQL query such as:
SELECT assignee, COUNT(*) 
FROM archon_tasks 
GROUP BY assignee 
ORDER BY COUNT(*) DESC;
and map or migrate any non-conforming values before merging.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Wirasm · 2025-11-24T09:13:04Z

The assignee field accepts a string by design to allow the agent to asign any subagent it creates.

thank you for the contribution, but not something we will add right now

google-labs-jules Bot and others added 2 commits October 9, 2025 05:15

Merge branch 'coleam00:main' into feature/add-developer-roles

09c7dd1

Wirasm closed this Nov 24, 2025

Wirasm mentioned this pull request Apr 8, 2026

feat: DAG loop-back nodes — conditional retry and loop-within-DAG #972

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add developer roles#770

Feature/add developer roles#770
valentinuuiuiu wants to merge 2 commits into
coleam00:mainfrom
valentinuuiuiu:feature/add-developer-roles

valentinuuiuiu commented Oct 9, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2025 •

edited

Loading

Uh oh!

Wirasm commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

valentinuuiuiu commented Oct 9, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Summary

Changes Made

Type of Change

Affected Services

Testing

Test Evidence

Checklist

Breaking Changes

Additional Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Wirasm commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

valentinuuiuiu commented Oct 9, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 9, 2025 •

edited

Loading