-
Notifications
You must be signed in to change notification settings - Fork 9
Feat/ai workflows -> dev #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sync with develop
…gger Gitea workflow trigger
Sync up with dev
eslint: | ||
specifier: ^9.18.0 | ||
version: 9.21.0 | ||
version: 9.21.0([email protected]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version format for eslint
seems incorrect. It appears to have an unexpected addition ([email protected])
. Please verify if this is intentional or a formatting error.
eslint-config-prettier: | ||
specifier: ^10.0.1 | ||
version: 10.0.1([email protected]) | ||
version: 10.0.1([email protected]([email protected])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version format for eslint-config-prettier
includes ([email protected]([email protected]))
, which seems inconsistent with typical versioning. Please confirm if this is correct or if it needs adjustment.
eslint-plugin-prettier: | ||
specifier: ^5.2.2 | ||
version: 5.2.3(@types/[email protected])([email protected]([email protected]))([email protected])([email protected]) | ||
version: 5.2.3(@types/[email protected])([email protected]([email protected]([email protected])))([email protected]([email protected]))([email protected]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version format for eslint-plugin-prettier
includes multiple nested dependencies with ([email protected])
. Ensure this is the intended format and verify if jiti
is a necessary addition here.
resolution: {integrity: sha512-j6vWzfrGVfyXxge+O0x5sh6cvxAog0a/4Rdd2K36zCMV5eJ+/+tOAngRO8cODMNWbVRdVlmGZQL2YS3yR8bIUA==} | ||
engines: {node: '>= 0.4'} | ||
|
||
[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of [email protected]
should be verified to ensure it is necessary for the AI Workflows feature. If it is not required, consider removing it to avoid unnecessary dependencies.
optional: true | ||
|
||
'@eslint-community/[email protected]([email protected])': | ||
'@eslint-community/[email protected]([email protected]([email protected]))': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of ([email protected])
to the @eslint-community/eslint-utils
dependency may require verification to ensure compatibility and correctness. Please confirm that this change is intentional and that [email protected]
is indeed a necessary dependency for this package.
constructor() { | ||
this.giteaClient = new Api({ | ||
baseURL: | ||
process.env.GITEA_BASE_URL || 'https://git.topcoder-dev.com/api/v1', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a configuration management library or service to manage environment variables more securely and flexibly.
baseURL: | ||
process.env.GITEA_BASE_URL || 'https://git.topcoder-dev.com/api/v1', | ||
headers: { | ||
Authorization: `Bearer ${process.env.GITEA_TOKEN}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a configuration management library or service to manage environment variables more securely and flexibly.
process.env.GITEA_SUBMISSION_REVIEW_NEW_REPO_DEF_BRANCH || | ||
'develop', | ||
name: challengeId, | ||
private: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making the repository private by default for security reasons, unless there's a specific reason for it to be public.
ResourceApiService, | ||
EventBusService, | ||
MemberService, | ||
SubmissionBaseService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider checking if SubmissionBaseService
is used elsewhere in the module or application. If it's not used, it might be unnecessary to include it here.
EventBusService, | ||
MemberService, | ||
SubmissionBaseService, | ||
GiteaService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that GiteaService
is correctly implemented and integrated within the application. Verify its dependencies and usage.
MemberService, | ||
SubmissionBaseService, | ||
GiteaService, | ||
SubmissionScanCompleteOrchestrator, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that SubmissionScanCompleteOrchestrator
is properly configured and that its lifecycle is managed correctly within the module.
try { | ||
const submission: SubmissionResponseDto = | ||
await this.submissionBaseService.getSubmissionById(submissionId); | ||
this.logger.log(`Submission details: ${JSON.stringify(submission)}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more structured logging approach instead of JSON.stringify(submission)
to avoid logging potentially sensitive information.
await this.challengeApiService.getChallengeDetail( | ||
submission.challengeId!, | ||
); | ||
this.logger.log(`Challenge details: ${JSON.stringify(challenge)}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more structured logging approach instead of JSON.stringify(challenge)
to avoid logging potentially sensitive information.
challenge.id, | ||
); | ||
} catch (error) { | ||
const errorMessage = `Error processing workflow: ${workflow.workflowId}. Error: ${error.message}.`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable error
is used here, but it is not declared in the catch block. Consider using catch (error: any)
to ensure TypeScript recognizes the error object.
import { BaseEventHandler } from '../base-event.handler'; | ||
import { KafkaHandlerRegistry } from '../kafka-handler.registry'; | ||
import { LoggerService } from '../../global/logger.service'; | ||
import { SubmissionScanCompleteOrchestrator } from '../../global/submission-scan-complete.orchestrator'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider checking if the SubmissionScanCompleteOrchestrator
import is used in the code. If it's not used, it might be unnecessary and could be removed to keep the code clean.
this.logger.log('=== Submission Scan Complete Event ==='); | ||
this.logger.log('Topic:', this.topic); | ||
this.logger.log('Payload:', JSON.stringify(message, null, 2)); | ||
this.logger.log('Topic: ' + this.topic); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition process.env.DISPATCH_AI_REVIEW_WORKFLOWS !== 'true'
might be better expressed as process.env.DISPATCH_AI_REVIEW_WORKFLOWS !== 'true'
to avoid potential issues with type coercion.
return; | ||
} | ||
|
||
if (!message.isInfected) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a null or undefined check for message.isInfected
to ensure that the property exists and is a boolean before proceeding with the logic.
|
||
if (!message.isInfected) { | ||
// delegate to orchestrator for further processing | ||
await this.orchestrator.orchestrateScanComplete(message.submissionId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that message.submissionId
is validated or sanitized before using it in the orchestrateScanComplete
method to prevent potential security issues.
Merge AI Workflows code base in dev to prepare for team work.