Uh oh!
There was an error while loading. Please reload this page.
[codex] Add Workflows Engineer dashboard role - #298
Merged
Conversation
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThis PR introduces the Workflows Engineer Discord role as a Steering Committee peer with dry-run access for admin-only sync and rerun operations. Changes span role definition and hierarchy refactoring, dashboard permission computation, dry-run authorization helpers wired into endpoints, frontend UI updates to handle dry-run responses, cross-service integration, and comprehensive test coverage. ChangesWorkflows Engineer Role with Dry-Run Access
Sequence Diagram(s)sequenceDiagram
participant Discord User
participant SessionInit as Session<br/>Initialization
participant PermLogic as Permission<br/>Logic
participant Endpoint as Dashboard<br/>Endpoint
participant DryRunHelper
participant Frontend
Discord User->>SessionInit: Workflows Engineer role
SessionInit->>PermLogic: has_workflows_engineer_role check
PermLogic->>PermLogic: Conditionally grant dry-run permissions
PermLogic->>Endpoint: Return scoped session
Frontend->>Endpoint: POST sync/rerun with canUse permission
Endpoint->>DryRunHelper: Check dry-run vs. write access
alt has write permission
DryRunHelper->>Endpoint: Return can_write=True
Endpoint->>Endpoint: Enqueue job
Endpoint->>Frontend: Return ok toast
else has only dry-run permission
DryRunHelper->>Endpoint: Return dry_run=True
Endpoint->>Frontend: Return would_enqueue description + warning toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e6b3d9df25
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Discord Workflows Engineer role that behaves as a Steering Committee peer for normal dashboard writes, while granting additional admin-like read access (jobs/audit) and dry-run access for sensitive write operations (job reruns and people/project syncs). It also updates the dashboard UI to surface dry-run responses and refreshes documentation/assets/tests accordingly.
Changes:
- Add
Workflows Engineerto role/permission evaluation across the API backend, Discord bot utilities, and shared policy code. - Introduce dry-run permissions and handlers for dashboard write endpoints (rerun/sync), and display dry-run feedback in the dashboard UI.
- Update tests and documentation, and rebuild dashboard static assets/manifest.
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_role_decorators.py | Adds hierarchy tests ensuring Workflows Engineer is a Steering Committee peer but not Admin. |
| tests/unit/test_backend_api.py | Adds permission and dry-run behavior tests for Workflows Engineer dashboard sessions. |
| packages/shared/src/five08/job_match.py | Prevents auto-suggest/apply for the new role. |
| packages/shared/src/five08/agent/policy.py | Treats workflows engineer as an engineer role for agent scopes. |
| ENVIRONMENT.md | Documents the new role’s intended permission model. |
| ARCHITECTURE.md | Documents dashboard permission behavior including dry-run semantics. |
| apps/discord_bot/src/five08/discord_bot/utils/role_decorators.py | Updates Discord command role hierarchy logic to include Workflows Engineer. |
| apps/discord_bot/src/five08/discord_bot/cogs/crm.py | Protects Workflows Engineer from automatic role apply operations. |
| apps/discord_bot/src/five08/discord_bot/cogs/agent.py | Allows workflows engineer to see engineer-level agent capabilities. |
| apps/api/src/five08/backend/static/dashboard/index.html | Updates static asset references to rebuilt dashboard bundles. |
| apps/api/src/five08/backend/static/dashboard/assets/index-DW09PDM3.css | Adds rebuilt CSS bundle. |
| apps/api/src/five08/backend/static/dashboard/assets/index-D9GwxYhO.css | Removes prior CSS bundle. |
| apps/api/src/five08/backend/static/dashboard/.vite/manifest.json | Updates manifest to point at rebuilt asset filenames. |
| apps/api/src/five08/backend/auth.py | Adds Workflows Engineer permission sets, role checks, and sensitive-permission handling. |
| apps/api/src/five08/backend/api.py | Adds dry-run session helper + dry-run implementations for rerun/sync endpoints. |
| apps/api/README.md | Documents new role behavior and the dry-run exception model. |
| apps/admin_dashboard/src/main.tsx | Adds dry-run UI handling (warning toast, enabling certain actions via *:dry_run). |
Files not reviewed (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-DW09PDM3.css: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
michaelmwuforce-pushed
the
michaelmwu/workflows-engineer-role
branch
from
May 20, 2026 07:24
e6b3d9d to
c3b377fCompareThere was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/discord_bot/src/five08/discord_bot/utils/role_decorators.py (1)
121-127: 💤 Low value
role_levelsis duplicated in two functions.The
role_levelsmapping is defined identically in bothcheck_user_roles_with_hierarchyandget_user_hierarchy_level. Consider extracting it to a module-level constant for consistency and easier maintenance, similar to_ROLE_LEVELSinauth.py.♻️ Proposed refactor
+# Role hierarchy levels for access control+ROLE_LEVELS = {+ "Member": 0,+ "Steering Committee": 1,+ "Workflows Engineer": 1,+ "Admin": 2,+ "Owner": 3,+}++ def check_user_roles_with_hierarchy( user_roles: List[discord.Role], required_roles: List[str] ) -> bool: ... - role_levels = {- "Member": 0,- "Steering Committee": 1,- "Workflows Engineer": 1,- "Admin": 2,- "Owner": 3,- } ... - if role_name in role_levels:- user_highest_level = max(user_highest_level, role_levels[role_name])+ if role_name in ROLE_LEVELS:+ user_highest_level = max(user_highest_level, ROLE_LEVELS[role_name])Apply similar changes to
get_user_hierarchy_level.Also applies to: 160-166
🤖 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 `@apps/discord_bot/src/five08/discord_bot/utils/role_decorators.py` around lines 121 - 127, The duplicate role_levels mapping used in check_user_roles_with_hierarchy and get_user_hierarchy_level should be extracted to a single module-level constant (e.g., _ROLE_LEVELS) so both functions reference the same source of truth; create the constant at top of the file, move the mapping there, and replace the local role_levels references in check_user_roles_with_hierarchy and get_user_hierarchy_level with the new _ROLE_LEVELS name to ensure consistency and easier maintenance.apps/api/src/five08/backend/auth.py (1)
28-36: 💤 Low valueRemove unused
ROLE_HIERARCHYtuple.The
ROLE_HIERARCHYtuple at line 28 is not referenced anywhere in the codebase and has been superseded by the_ROLE_LEVELSmapping for hierarchy evaluation. Removing it will eliminate unnecessary duplication and clarify that_ROLE_LEVELSis the authoritative hierarchy source.🤖 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 `@apps/api/src/five08/backend/auth.py` around lines 28 - 36, Remove the unused ROLE_HIERARCHY tuple declaration and keep _ROLE_LEVELS as the single source of truth for role hierarchy; delete the line defining ROLE_HIERARCHY and ensure any references are not present elsewhere (verify by searching for ROLE_HIERARCHY), leaving WORKFLOWS_ENGINEER_ROLE and _ROLE_LEVELS intact and run tests/lint to confirm no breakage.apps/admin_dashboard/src/main.tsx (1)
1529-1543: 💤 Low valueConsider extracting the dry-run response type.
The response type
{ job_id?: string; dry_run?: boolean; would_enqueue?: { job_type?: string } }is duplicated acrosssyncProjects,rerunJob, andsyncPeople. Extracting to a shared type alias would improve maintainability.typeDryRunResponse={job_id?: stringdry_run?: booleanwould_enqueue?: {job_type?: string}}🤖 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 `@apps/admin_dashboard/src/main.tsx` around lines 1529 - 1543, Extract the duplicated response shape into a shared type alias (e.g., DryRunResponse) and replace the inline generic object type used in requestJson calls; update the places that use the same shape (notably the requestJson call inside the function handling "syncPeople" where payload is declared, and the similar usages in syncProjects and rerunJob) to requestJson<DryRunResponse>(...) so they all reference the new DryRunResponse type; add the type near related request helpers or a shared types file and update imports if necessary.
🤖 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 `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 79-81: The user-facing blocked-role message is hardcoded and now
out-of-sync with the protection set; instead build that message dynamically from
the _DISCORD_ROLES_PROTECTED_FROM_APPLY frozenset so it always reflects current
protected roles. Locate where the "blocked-role" / blocked-role message is
constructed (the user-facing string shown when apply is rejected) and replace
the hardcoded list with a formatted join of
sorted(_DISCORD_ROLES_PROTECTED_FROM_APPLY) (e.g., comma-separated with final
separator) to preserve deterministic ordering and readable punctuation; ensure
you reference _DISCORD_ROLES_PROTECTED_FROM_APPLY and keep the rest of the error
wording intact.
---
Nitpick comments:
In `@apps/admin_dashboard/src/main.tsx`:
- Around line 1529-1543: Extract the duplicated response shape into a shared
type alias (e.g., DryRunResponse) and replace the inline generic object type
used in requestJson calls; update the places that use the same shape (notably
the requestJson call inside the function handling "syncPeople" where payload is
declared, and the similar usages in syncProjects and rerunJob) to
requestJson<DryRunResponse>(...) so they all reference the new DryRunResponse
type; add the type near related request helpers or a shared types file and
update imports if necessary.
In `@apps/api/src/five08/backend/auth.py`:
- Around line 28-36: Remove the unused ROLE_HIERARCHY tuple declaration and keep
_ROLE_LEVELS as the single source of truth for role hierarchy; delete the line
defining ROLE_HIERARCHY and ensure any references are not present elsewhere
(verify by searching for ROLE_HIERARCHY), leaving WORKFLOWS_ENGINEER_ROLE and
_ROLE_LEVELS intact and run tests/lint to confirm no breakage.
In `@apps/discord_bot/src/five08/discord_bot/utils/role_decorators.py`:
- Around line 121-127: The duplicate role_levels mapping used in
check_user_roles_with_hierarchy and get_user_hierarchy_level should be extracted
to a single module-level constant (e.g., _ROLE_LEVELS) so both functions
reference the same source of truth; create the constant at top of the file, move
the mapping there, and replace the local role_levels references in
check_user_roles_with_hierarchy and get_user_hierarchy_level with the new
_ROLE_LEVELS name to ensure consistency and easier maintenance.
🪄 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
Run ID: c92b7a16-b14c-47d5-88d4-d36d2ce4799a
📒 Files selected for processing (19)
ARCHITECTURE.mdENVIRONMENT.mdapps/admin_dashboard/src/main.tsxapps/api/README.mdapps/api/src/five08/backend/api.pyapps/api/src/five08/backend/auth.pyapps/api/src/five08/backend/static/dashboard/.vite/manifest.jsonapps/api/src/five08/backend/static/dashboard/assets/index-B8EWkBmI.jsapps/api/src/five08/backend/static/dashboard/assets/index-CPTdpcwK.jsapps/api/src/five08/backend/static/dashboard/assets/index-D9GwxYhO.cssapps/api/src/five08/backend/static/dashboard/assets/index-DW09PDM3.cssapps/api/src/five08/backend/static/dashboard/index.htmlapps/discord_bot/src/five08/discord_bot/cogs/agent.pyapps/discord_bot/src/five08/discord_bot/cogs/crm.pyapps/discord_bot/src/five08/discord_bot/utils/role_decorators.pypackages/shared/src/five08/agent/policy.pypackages/shared/src/five08/job_match.pytests/unit/test_backend_api.pytests/unit/test_role_decorators.py
💤 Files with no reviewable changes (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-D9GwxYhO.css
Uh oh!
There was an error while loading. Please reload this page.
michaelmwuforce-pushed
the
michaelmwu/workflows-engineer-role
branch
from
May 20, 2026 08:36
c3b377f to
6e8c23eCompareThere was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-DW09PDM3.css: Language not supported
Comments suppressed due to low confidence (1)
tests/unit/test_backend_api.py:5169
- These new Workflows Engineer dry-run endpoint tests also run under the default test environment, which bypasses the production SSO gating. Add a production-environment variant (e.g.,
monkeypatch.setattr(api.settings, "environment", "production")) to ensure Workflows Engineer still receives dry-run access whenid_tokenis empty in production.
def test_dashboard_rerun_job_workflows_engineer_is_dry_run(
client: TestClient,
) -> None:
session = api.AuthSession(
subject="workflows-1",
email="workflows@508.dev",
display_name="Workflows Engineer",
groups=["Workflows Engineer"],
is_admin=False,
id_token="",
expires_at=4_102_444_800,
actor_provider=api.ActorProvider.DISCORD.value,
)
source_job = Mock(
id="job-old-1",
type="sync_people_from_crm_job",
max_attempts=8,
payload={"args": [], "kwargs": {"reason": "manual"}},
)
with (
patch(
"five08.backend.api._current_session",
new_callable=AsyncMock,
return_value=("session-1", session),
),
patch("five08.backend.api.get_job", return_value=source_job),
patch("five08.backend.api._rerun_job", new_callable=AsyncMock) as mock_rerun,
patch("five08.backend.api.insert_audit_event") as mock_insert,
):
response = client.post("/dashboard/api/jobs/job-old-1/rerun")
Uh oh!
There was an error while loading. Please reload this page.
michaelmwuforce-pushed
the
michaelmwu/workflows-engineer-role
branch
from
May 20, 2026 09:27
6e8c23e to
b612bbaCompareUh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Workflows Engineerrole as a Steering Committee peer for normal dashboard writes.Validation
bun run checkbun run build./scripts/lint.sh./scripts/test.sh(1492 passed, 20 skipped)Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests