Skip to content

Conversation

@ferruhcihan
Copy link
Collaborator

@ferruhcihan ferruhcihan commented Apr 1, 2025

@dennisvankekem dennisvankekem self-assigned this Apr 2, 2025
@ferruhcihan ferruhcihan requested a review from j-zimnowoda April 9, 2025 11:33
@github-actions
Copy link

github-actions bot commented Apr 10, 2025

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements
50.52% (+0.2% 🔼)
2249/4452
🔴 Branches
34.9% (+0.22% 🔼)
356/1020
🔴 Functions
43.2% (+0.58% 🔼)
416/963
🔴 Lines
50.87% (+0.14% 🔼)
2112/4152
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟡
... / repoBranches.ts
66.67% 100% 50% 66.67%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴 otomi-stack.ts
21.12% (-0.04% 🔻)
17.2% (-0.21% 🔻)
18.12% (+0.22% 🔼)
21.33% (-0.14% 🔻)
🟢
... / codeRepoUtils.ts
94.12% (-1.12% 🔻)
83.72% (+1.9% 🔼)
100%
96.26% (-2.42% 🔻)

Test suite run success

289 tests passing in 12 suites.

Report generated by 🧪jest coverage report action from 5e4703f

if (allRepoUrls.includes(data.spec.repositoryUrl)) throw new AlreadyExists()
if (!data.spec.private) unset(data.spec, 'secret')
if (data.spec.gitService === 'gitea') unset(data.spec, 'private')
const codeRepo = this.repoService.getTeamConfigService(teamId).createCodeRepo(data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the try block should start from here. Then you do not need a complicated catch statement and throw AlreadyExists exception.


return await getPublicRepoBranches(repoUrl)
} catch (error) {
return []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No error log?

let sshPrivateKey = '',
username = '',
accessToken = ''

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by defining here isPrivate you can solely rely on the variable to make decisions instead of relying on secretName and then on isPrivate

@j-zimnowoda j-zimnowoda requested a review from Copilot April 10, 2025 15:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/openapi/api.yaml: Language not supported
Comments suppressed due to low confidence (1)

src/utils/codeRepoUtils.ts:84

  • The normalizeSSHKey function does not appear to throw an error when given an invalid SSH key format even though tests expect an exception. Consider adding a check to throw an error (e.g., 'Invalid SSH Key format') when the key does not include the required header and footer.
export function normalizeSSHKey(sshPrivateKey) {

let url = repoUrl

if (url.startsWith('git@')) {
const normalizedKey: string = sshPrivateKey ? normalizeSSHKey(sshPrivateKey) : ''
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the repo URL starts with 'git@', if no SSH private key is provided (resulting in an empty normalized key), the function silently bypasses SSH authentication configuration. Consider explicitly handling the case where sshPrivateKey is missing by throwing an error or providing a meaningful fallback.

Suggested change
const normalizedKey: string = sshPrivateKey ? normalizeSSHKey(sshPrivateKey) : ''
if (!sshPrivateKey) {
throw new Error('SSH private key is required for SSH-based repository URLs (git@).')
}
const normalizedKey: string = normalizeSSHKey(sshPrivateKey)

Copilot uses AI. Check for mistakes.
@j-zimnowoda j-zimnowoda self-requested a review April 10, 2025 15:32
@j-zimnowoda j-zimnowoda self-assigned this Apr 10, 2025
@ferruhcihan ferruhcihan requested a review from Copilot April 11, 2025 12:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • src/openapi/api.yaml: Language not supported


if (!repoUrl) return ['HEAD']

if (isPrivate) return await getPrivateRepoBranches(repoUrl, sshPrivateKey, username, accessToken)
Copy link

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For private repositories, secret values (sshPrivateKey, username, accessToken) are not being retrieved prior to calling getPrivateRepoBranches, which may lead to authentication failures. Consider fetching these secret values (using getSecretValues) when secretName is present.

Copilot uses AI. Check for mistakes.
@dennisvankekem dennisvankekem merged commit a075c4f into main Apr 11, 2025
3 checks passed
@dennisvankekem dennisvankekem deleted the APL-535 branch April 11, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants