Conversation
WalkthroughThis pull request introduces a new React component, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant JAS as JobAgentSection
participant DGC as DeploymentJobAgentGithubConfig
participant API as TRPC API
participant GB as GitHub Router
U->>JAS: Selects job agent configuration
JAS->>DGC: Render GitHub config (if type = GithubApp)
DGC->>API: Fetch GitHub agent & repos via TRPC query
API->>GB: Process GitHub request with agent ID
GB-->>API: Return agent details and repositories
API-->>DGC: Provide data
DGC->>U: Display GitHub configuration UI
sequenceDiagram
participant API as TRPC API
participant GR as GitHub Router
participant GRF as getRepos Function
participant Org as listForOrg
participant UserRepo as listForUser
API->>GR: Request repositories (includes owner and type)
GR->>GRF: Call getRepos(page, history, token, owner, type)
alt type == "organization"
GRF->>Org: Fetch org repositories
Org-->>GRF: Return repositories
else type == "user"
GRF->>UserRepo: Fetch user repositories
UserRepo-->>GRF: Return repositories
end
GRF-->>GR: Return repository list
GR-->>API: Deliver updated repository list
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/api/src/router/job.ts (1)
566-576: Add defensive check for installationId in agent config.The code assumes
agent.config.installationIdexists and can be converted to a number. Consider adding a type check or validation to handle cases where the config structure might be different than expected.+ if (!agent.config?.installationId) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "GitHub app configuration is missing installation ID", + }); + } + const ghEntity = await ctx.db .select() .from(schema.githubEntity) .where( eq( schema.githubEntity.installationId, Number(agent.config.installationId), ), ) .then(takeFirstOrNull);apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx (1)
61-92: Well-implemented conditional rendering for job agent configsThe new approach of conditionally rendering different configuration components based on the job agent type is clean and maintainable. Consider adding a default case to handle unknown agent types.
{selectedJobAgent?.type.startsWith("exec-") && ( <JobAgentScriptConfig type={ selectedJobAgent.type.startsWith(JobAgentType.ExecWindows) ? "powershell" : "shell" } {...field} /> )} + {selectedJobAgent && + !selectedJobAgent.type.startsWith("exec-") && + selectedJobAgent.type !== JobAgentType.KubernetesJob && + selectedJobAgent.type !== JobAgentType.GithubApp && ( + <span className="text-sm text-muted-foreground"> + Configuration not available for this agent type + </span> + )}apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/DeploymentJobAgentGithubConfig.tsx (3)
34-44: Add error handling for failed data fetchingWhile loading states are handled, there's no explicit error handling for failed queries. Consider adding error states to inform users when API calls fail.
const { data: githubAgent, isLoading: isGithubAgentLoading } = api.job.agent.github.byId.useQuery(jobAgentId); const { data: repos, isLoading: isReposLoading } = api.github.entities.repos.list.useQuery( { installationId: githubAgent?.ghEntity.installationId ?? 0, owner: githubAgent?.ghEntity.slug ?? "", workspaceId: githubAgent?.workspaceId ?? "", }, { enabled: githubAgent != null }, ); + + // Add error handling + if (repos === undefined && !isReposLoading) { + return ( + <div className="flex w-96 items-center justify-center gap-2 text-red-500"> + <span>Failed to load repositories. Please try again.</span> + </div> + ); + }
189-199: Add validation for the Git reference fieldConsider adding basic validation for the Git reference input to prevent invalid inputs that could cause workflow failures.
<div className="flex flex-col gap-2"> <Label className="font-medium">Git reference</Label> <Input placeholder="(uses repositories default if not set)" value={value.ref ?? ""} + pattern="^[a-zA-Z0-9_\-./]+$" + title="Valid Git references: branch names, tag names, or commit hashes" onChange={(e) => { const ref = e.target.value === "" ? null : e.target.value; onChange({ ...value, ref }); }} /> + <span className="text-xs text-muted-foreground"> + Enter branch name, tag name, or commit hash + </span> </div>
110-113: Consider clearing related fields when repo changesWhen a user selects a new repository, the workflow selection may become invalid. Consider resetting the workflowId when a new repo is selected.
onSelect={(currentValue) => { - onChange({ ...value, repo: currentValue }); + onChange({ ...value, repo: currentValue, workflowId: undefined }); setRepoOpen(false); + setWorkflowOpen(true); // Optionally open workflow selector immediately }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/DeploymentJobAgentGithubConfig.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx(3 hunks)packages/api/src/router/github.ts(4 hunks)packages/api/src/router/job.ts(4 hunks)packages/validators/src/auth/index.ts(1 hunks)packages/validators/src/jobs/agents/index.ts(1 hunks)packages/validators/src/jobs/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/validators/src/auth/index.tspackages/validators/src/jobs/index.tspackages/validators/src/jobs/agents/index.tspackages/api/src/router/github.tsapps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/DeploymentJobAgentGithubConfig.tsxpackages/api/src/router/job.tsapps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (13)
packages/validators/src/jobs/agents/index.ts (1)
1-5: Enum looks good for job agent types.The enum definition is clear and follows best practices with descriptive string literals.
packages/validators/src/auth/index.ts (1)
33-33: Permission addition for job agent retrieval.The new
JobAgentGetpermission correctly follows the established naming convention and is properly placed in the enum structure.packages/validators/src/jobs/index.ts (1)
2-2: New module export for job agents.This export statement correctly makes the job agent types available from the main jobs module.
packages/api/src/router/job.ts (2)
538-586: GitHub router implementation looks good with proper error handling.The new router implementation correctly implements the necessary error handling and data fetching logic for GitHub job agents.
560-565: Verify agent type check is robust for all case scenarios.The current check verifies if the agent type matches
String(JobAgentType.GithubApp). This approach works but consider using a direct comparison against the enum value for better type safety.- if (agent.type !== String(JobAgentType.GithubApp)) + if (agent.type !== JobAgentType.GithubApp)packages/api/src/router/github.ts (4)
128-128: Good addition of type parameterAdding the type parameter to distinguish between organizations and users enhances the flexibility of the
getReposfunction.
130-149: Improved repository fetching with conditional logicThe implementation correctly handles both organization and user repositories by conditionally calling the appropriate GitHub API method.
192-209: Enhanced error handling for entity retrievalGood improvement in error handling by explicitly checking if the entity exists and throwing a proper error with a meaningful message.
226-226: Valid usage of entity typeYou're correctly passing the entity type to the getRepos function, ensuring that the right repositories are fetched.
apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx (2)
12-18: Good modularization of job agent configurationsThe updated imports reflect a better separation of concerns by using specialized components for different job agent types.
46-46: Improved spacing for better readabilityChanging from
space-y-3tospace-y-6provides more vertical space between elements, improving the form's readability.apps/webservice/src/app/[workspaceSlug]/(appv2)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/DeploymentJobAgentGithubConfig.tsx (2)
26-30: Well-structured component propsThe component props are well-defined and provide a clean interface for integration with the parent component.
102-132: Good handling of loading and empty statesThe component properly handles loading states and empty data scenarios, providing clear feedback to the user.
Summary by CodeRabbit
New Features
Chores