feat: add gitlab#49
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughThis PR adds multi-provider VCS support by introducing a GitLab adapter and wiring a provider-agnostic VCS abstraction across env validation, adapter construction, sandbox operations, and workflows. It includes new runtime dependency, tests for GitLab flows, and URL/credential normalization utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (startup)
participant Env as getVcsConfig()
participant Factory as createVCS()
participant Adapter as VCS Adapter
participant RemoteAPI as GitHub/GitLab API
App->>Env: read VCS_KIND + provider envs
Env-->>App: validated VcsConfig
App->>Factory: createVCS(vcsConfig)
Factory-->>Adapter: instantiate GitHubAdapter or GitLabAdapter
App->>Adapter: push/createPR/getChecks...
Adapter->>RemoteAPI: call REST API (commits/branches/MRs/pipelines)
RemoteAPI-->>Adapter: responses / errors
Adapter-->>App: domain-shaped results or FatalError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/sandbox/poll-agent.test.ts (1)
34-40: Add a GitLab-mode test case for URL/auth behavior.This mock only exercises GitHub. Since this PR introduces multi-provider VCS, add one test variant with
kind: "gitlab"to verify push flow uses GitLab auth semantics (especiallyoauth2auth user and custom host handling).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sandbox/poll-agent.test.ts` around lines 34 - 40, Add a second test variant in poll-agent.test.ts that exercises GitLab semantics by modifying the existing getVcsConfig mock to include a case where kind: "gitlab", host is a GitLab URL, and token/auth fields reflect OAuth2 (e.g., auth user "oauth2" or equivalent) so the push flow uses GitLab auth handling; update the test that exercises the push flow (the test invoking getVcsConfig) to run against both the GitHub mock and the new GitLab mock and assert that the push step uses the GitLab-specific auth user and host behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 19-24: Update the .env.example to avoid duplicate VCS_KIND entries
by removing or commenting out the existing VCS_KIND=github line (the existing
VCS_KIND variable) before presenting the GitLab block (GITLAB_TOKEN,
GITLAB_PROJECT_ID, GITLAB_BASE_BRANCH, GITLAB_HOST) so the example never
contains two active VCS_KIND keys; alternatively present the GitLab block as a
clear replacement snippet that instructs the user to swap the VCS_KIND value
rather than uncommenting a second VCS_KIND.
In `@src/adapters/vcs/gitlab.ts`:
- Around line 18-20: The GitLab diff note position currently only models
new_path/new_line; update the GitLabNotePosition interface to also include
optional old_path?: string and old_line?: number, and then change the code that
computes comment anchors (the logic that reads new_path/new_line — the
note-to-anchor mapping where you previously used new_path/new_line) to fall back
to old_path/old_line when new_* are undefined (i.e., use new_path || old_path
and new_line ?? old_line) so comments on deleted lines get proper anchors.
In `@src/lib/create-vcs.ts`:
- Around line 16-19: The current destructuring of vcs.repoPath into const
[owner, repo] = vcs.repoPath.split("/") allows extra segments like
"owner/repo/extra"; update the parsing in create-vcs.ts to first split
vcs.repoPath and assert that the result has exactly two parts before assigning
owner and repo (rejecting or throwing a clear error otherwise), referencing the
existing vcs.repoPath, owner, and repo identifiers when implementing the
stricter validation and error message.
In `@src/sandbox/manager.ts`:
- Around line 29-43: The authUrl currently hardcodes "https://" which can
mismatch cloneUrl when config.host includes a different scheme; update
buildVcsUrls to preserve the original scheme: derive the scheme from the
normalized host (e.g. const scheme = host.match(/^https?:\/\//)?.[0] ??
"https://") and use that scheme when constructing authUrl (instead of the fixed
"https://"), keeping the existing hostNoScheme, authUser, and repoPath
interpolation so cloneUrl and authUrl use the same scheme and host.
In `@src/workflows/agent.ts`:
- Around line 52-55: The code passes vcs.repoPath straight into SandboxManager
URL construction which can be a numeric GitLab project ID and produce invalid
clone/push URLs; in the function that builds/initializes SandboxManager
(referencing SandboxManager and the vcs object, specifically vcs.repoPath), add
a validation/resolution step: if vcs.host is a GitLab instance and vcs.repoPath
is purely numeric, either resolve the numeric project ID to its
namespace/project path via the GitLab API before using it, or fail-fast by
throwing/logging a clear error asking for a URL-safe namespace/project path;
ensure the constructed URL logic uses the validated path and surface a helpful
error if resolution fails.
---
Nitpick comments:
In `@src/sandbox/poll-agent.test.ts`:
- Around line 34-40: Add a second test variant in poll-agent.test.ts that
exercises GitLab semantics by modifying the existing getVcsConfig mock to
include a case where kind: "gitlab", host is a GitLab URL, and token/auth fields
reflect OAuth2 (e.g., auth user "oauth2" or equivalent) so the push flow uses
GitLab auth handling; update the test that exercises the push flow (the test
invoking getVcsConfig) to run against both the GitHub mock and the new GitLab
mock and assert that the push step uses the GitLab-specific auth user and host
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a319a4c1-6eaf-4edb-baa9-cbb6a1b79714
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.env.exampleenv.test.tsenv.tspackage.jsonsrc/adapters/vcs/github.tssrc/adapters/vcs/gitlab.test.tssrc/adapters/vcs/gitlab.tssrc/adapters/vcs/types.tssrc/lib/adapters.tssrc/lib/create-vcs.tssrc/lib/step-adapters.tssrc/sandbox/manager.test.tssrc/sandbox/manager.tssrc/sandbox/poll-agent.test.tssrc/sandbox/poll-agent.tssrc/workflows/agent.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/sandbox/manager.ts (1)
29-50: Scheme preservation fix looks correct, but consider validating host format.The scheme extraction at lines 38-41 properly addresses the previous review concern about hardcoding
https://. However, ifhostlacks a scheme (e.g.,"github.com"instead of"https://github.com"), the function produces inconsistent URLs:
cloneUrl→"github.com/owner/repo.git"(no scheme)authUrl→"https://oauth2:token@github.com/owner/repo.git"(with scheme)Based on the test and
.env.example, hosts are expected to include schemes, but this isn't enforced.💡 Optional: Add validation for scheme presence
export function buildVcsUrls(config: { kind: "github" | "gitlab"; token: string; repoPath: string; host: string; }) { // Strip trailing slash for consistent URL joining. const host = config.host.replace(/\/+$/, ""); + if (!/^https?:\/\//.test(host)) { + throw new Error(`VCS host must include scheme (http:// or https://), got: ${host}`); + } // Preserve the scheme from the configured host so cloneUrl and authUrl agree🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sandbox/manager.ts` around lines 29 - 50, The buildVcsUrls function can produce a cloneUrl without a scheme when config.host lacks one, causing cloneUrl and authUrl to disagree; update buildVcsUrls to validate/normalize the host by ensuring a scheme is present (e.g., if host has no /^https?:\/\//, prepend the default scheme before computing host, scheme, hostNoScheme) so that cloneUrl and authUrl are constructed consistently; specifically adjust the logic around the host, scheme, and hostNoScheme variables used to form cloneUrl and authUrl (and surface a clear error or throw if config.host is malformed) so callers of buildVcsUrls always get URLs with matching schemes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/sandbox/manager.ts`:
- Around line 29-50: The buildVcsUrls function can produce a cloneUrl without a
scheme when config.host lacks one, causing cloneUrl and authUrl to disagree;
update buildVcsUrls to validate/normalize the host by ensuring a scheme is
present (e.g., if host has no /^https?:\/\//, prepend the default scheme before
computing host, scheme, hostNoScheme) so that cloneUrl and authUrl are
constructed consistently; specifically adjust the logic around the host, scheme,
and hostNoScheme variables used to form cloneUrl and authUrl (and surface a
clear error or throw if config.host is malformed) so callers of buildVcsUrls
always get URLs with matching schemes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b23c9e2-520f-44a6-9064-45c6ee34da7b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.env.examplesrc/adapters/vcs/gitlab.tssrc/lib/create-vcs.tssrc/sandbox/manager.tssrc/sandbox/poll-agent.test.tssrc/workflows/agent.ts
✅ Files skipped from review due to trivial changes (2)
- src/workflows/agent.ts
- src/adapters/vcs/gitlab.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/sandbox/poll-agent.test.ts
- src/lib/create-vcs.ts
* fix: workflow timout * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * Aiw 16 cicd info (#38) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * feat: remove console.log * fix: ci --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> Co-authored-by: Kacper <89151689+kasin-it@users.noreply.github.com> * fix: push issue * fix: branch creation * feat: implement new agent flow (#43) * feat: prevent gitignoing memory * feat: add token usage * docs: add security observability spec for Arthur Engine integration Defines 6 threat categories (prompt injection, data exfiltration, secrets leakage, PII, code safety, behavioral anomalies), pipeline integration gates, response severity model, and observability streams for the AWS on-prem deployment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add debug mode section to security observability spec On-demand deep inspection for suspicious agent runs — full prompt/response logging, tool call traces, network packet capture (tcpdump), per-tool-call file system diffs, and env dumps. Triggered manually or auto-escalated from medium+ findings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * dcos: add security & observability spec * feat: add vsc adapter plan * fix: cc oauth * feat: remove redundant model export * feat: add webhooks * feat: add better error handling * fix: webhook token * chore: trigger redeploy * wip * wip * feat: remove logs * feat: remove desination column logic * feat: add gitlab (#49) * feat: add gitlab * chore: regenerate pnpm lock * feat: review improvements * feat: harden poll logic (#52) * feat: harden poll logic * fix: mock env in dispatch * fix: ghost workflow logs * feat: implement IssueTrackerNotFoundError handling in JiraAdapter and related tests * feat: add cancel notification * fix: sandbox cancelation * fix: clarification flow * feat: tune resaerch prompt * feat: attachemnt support (#50) * feat: attachemnt support * fix: attachement fetch * fix: ci fail * feat: enhance JiraAdapter and dispatch logic - Added tests for sanitizing malformed attachment sizes in JiraAdapter. - Improved handling of cross-origin requests by omitting Authorization headers. - Enhanced redirect handling in JiraAdapter to support multiple redirects. - Updated dispatchTicket function to paginate sandbox list and handle capacity checks more robustly. - Added pagination support in sandbox list responses to improve efficiency. * fix: ci * feat: enhance JiraAdapter attachment handling and improve error management - Added test to omit contentUrl when Jira does not provide attachment content. - Improved attachment handling in JiraAdapter to ensure contentUrl is undefined when absent. - Enhanced error handling for missing Location headers during redirects in attachment downloads. - Updated fetchAttachmentsWithRetry to skip attachments without content URLs and log appropriate messages. * feat: add logs to dispatch * fix: dispatch timeout * feat: add new e2e * feat: go back to 3 phase flow * feat: new tests * fix: remove fake err * feat: add clarification e2e * feat: add failure e2e tests * feat: add dispatch e2e --------- Co-authored-by: ai-workflow-blazity <ai-workflow@blazity.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: workflow timout * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * Aiw 16 cicd info (#38) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * feat: remove console.log * fix: ci --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> Co-authored-by: Kacper <89151689+kasin-it@users.noreply.github.com> * fix: push issue * fix: branch creation * feat: implement new agent flow (#43) * feat: prevent gitignoing memory * feat: add token usage * docs: add security observability spec for Arthur Engine integration Defines 6 threat categories (prompt injection, data exfiltration, secrets leakage, PII, code safety, behavioral anomalies), pipeline integration gates, response severity model, and observability streams for the AWS on-prem deployment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add debug mode section to security observability spec On-demand deep inspection for suspicious agent runs — full prompt/response logging, tool call traces, network packet capture (tcpdump), per-tool-call file system diffs, and env dumps. Triggered manually or auto-escalated from medium+ findings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * dcos: add security & observability spec * feat: add vsc adapter plan * fix: cc oauth * feat: remove redundant model export * feat: add webhooks * feat: add better error handling * fix: webhook token * chore: trigger redeploy * wip * wip * feat: remove logs * feat: remove desination column logic * feat: add gitlab (#49) * feat: add gitlab * chore: regenerate pnpm lock * feat: review improvements * feat: harden poll logic (#52) * feat: harden poll logic * fix: mock env in dispatch * fix: ghost workflow logs * feat: implement IssueTrackerNotFoundError handling in JiraAdapter and related tests * feat: add cancel notification * fix: sandbox cancelation * fix: clarification flow * feat: tune resaerch prompt * feat: attachemnt support (#50) * feat: attachemnt support * fix: attachement fetch * fix: ci fail * feat: enhance JiraAdapter and dispatch logic - Added tests for sanitizing malformed attachment sizes in JiraAdapter. - Improved handling of cross-origin requests by omitting Authorization headers. - Enhanced redirect handling in JiraAdapter to support multiple redirects. - Updated dispatchTicket function to paginate sandbox list and handle capacity checks more robustly. - Added pagination support in sandbox list responses to improve efficiency. * fix: ci * feat: enhance JiraAdapter attachment handling and improve error management - Added test to omit contentUrl when Jira does not provide attachment content. - Improved attachment handling in JiraAdapter to ensure contentUrl is undefined when absent. - Enhanced error handling for missing Location headers during redirects in attachment downloads. - Updated fetchAttachmentsWithRetry to skip attachments without content URLs and log appropriate messages. * feat: add logs to dispatch * fix: dispatch timeout * feat: add new e2e * feat: go back to 3 phase flow * feat: new tests * fix: remove fake err * feat: add clarification e2e * feat: add failure e2e tests * feat: add dispatch e2e * feat: update capacity check * feat: update capacity check * fix: at capacity error * fix: us13 cleanup * fix: stale run * fix: us03 prompt * fix: follow pr comments * fix: us7 and us8 * feat: optimize sandbox delete * fix: concurrent tests flow * feat: better multi ticket dispatch * feat: disable review * feat: new actions for e2e * fix: gh actions * feat: add oidc token to e2e * fix: us15 e2e * fix: us11 * feat: new capacity check * feat: remove tier1 * feat: remove duplicates * fix: ci/cd order * feat: new ci * fix; duplicate ci --------- Co-authored-by: ai-workflow-blazity <ai-workflow@blazity.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: workflow timout * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * Aiw 16 cicd info (#38) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * Aiw 25 safeguard (#37) * feat: add redis safeguard * fix: json parse --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> * feat: push inside the sandbox (#40) * feat: add cicd check * feat: ci/cd fail check * feat: remove console log * feat: remove console.log * fix: ci --------- Co-authored-by: kasin-it <kacperosinilo@gmail.com> Co-authored-by: Kacper <89151689+kasin-it@users.noreply.github.com> * fix: push issue * fix: branch creation * feat: implement new agent flow (#43) * feat: prevent gitignoing memory * feat: add token usage * docs: add security observability spec for Arthur Engine integration Defines 6 threat categories (prompt injection, data exfiltration, secrets leakage, PII, code safety, behavioral anomalies), pipeline integration gates, response severity model, and observability streams for the AWS on-prem deployment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add debug mode section to security observability spec On-demand deep inspection for suspicious agent runs — full prompt/response logging, tool call traces, network packet capture (tcpdump), per-tool-call file system diffs, and env dumps. Triggered manually or auto-escalated from medium+ findings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * dcos: add security & observability spec * feat: add vsc adapter plan * fix: cc oauth * feat: remove redundant model export * feat: add webhooks * feat: add better error handling * fix: webhook token * chore: trigger redeploy * wip * wip * feat: remove logs * feat: remove desination column logic * feat: add gitlab (#49) * feat: add gitlab * chore: regenerate pnpm lock * feat: review improvements * feat: harden poll logic (#52) * feat: harden poll logic * fix: mock env in dispatch * fix: ghost workflow logs * feat: implement IssueTrackerNotFoundError handling in JiraAdapter and related tests * feat: add cancel notification * fix: sandbox cancelation * fix: clarification flow * feat: tune resaerch prompt * feat: attachemnt support (#50) * feat: attachemnt support * fix: attachement fetch * fix: ci fail * feat: enhance JiraAdapter and dispatch logic - Added tests for sanitizing malformed attachment sizes in JiraAdapter. - Improved handling of cross-origin requests by omitting Authorization headers. - Enhanced redirect handling in JiraAdapter to support multiple redirects. - Updated dispatchTicket function to paginate sandbox list and handle capacity checks more robustly. - Added pagination support in sandbox list responses to improve efficiency. * fix: ci * feat: enhance JiraAdapter attachment handling and improve error management - Added test to omit contentUrl when Jira does not provide attachment content. - Improved attachment handling in JiraAdapter to ensure contentUrl is undefined when absent. - Enhanced error handling for missing Location headers during redirects in attachment downloads. - Updated fetchAttachmentsWithRetry to skip attachments without content URLs and log appropriate messages. * feat: add logs to dispatch * fix: dispatch timeout * feat: add new e2e * feat: go back to 3 phase flow * feat: new tests * fix: remove fake err * feat: add clarification e2e * feat: add failure e2e tests * feat: add dispatch e2e * feat: update capacity check * feat: update capacity check * fix: at capacity error * fix: us13 cleanup * fix: stale run * fix: us03 prompt * fix: follow pr comments * fix: us7 and us8 * feat: optimize sandbox delete * fix: concurrent tests flow * feat: better multi ticket dispatch * feat: disable review * feat: new actions for e2e * fix: gh actions * feat: add oidc token to e2e * fix: us15 e2e * fix: us11 * feat: new capacity check * feat: remove tier1 * feat: remove duplicates * fix: ci/cd order * feat: new ci * fix; duplicate ci * fix: e2e on pr --------- Co-authored-by: ai-workflow-blazity <ai-workflow@blazity.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests