Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/src/handlers/clone.test.ts (1)

304-309: ⚡ Quick win

Harden env cleanup to keep tests order-independent.

process.env cleanup is distributed inside tests; if one test exits early, later tests can inherit stale forge tokens. Reset forge env vars in beforeEach/afterEach for this describe block instead of per-test deletes (Line 306-309 and per-test cleanup lines).

♻️ Suggested change

 describe('multi-forge authentication', () => {
-  afterAll(() => {
+  beforeEach(() => {
+    delete process.env.GH_TOKEN;
+    delete process.env.GITLAB_TOKEN;
+    delete process.env.GITEA_TOKEN;
+  });
+
+  afterEach(() => {
+    delete process.env.GH_TOKEN;
     delete process.env.GITLAB_TOKEN;
     delete process.env.GITEA_TOKEN;
   });
@@
-      delete process.env.GITLAB_TOKEN;
@@
-      delete process.env.GITLAB_TOKEN;
@@
-      delete process.env.GITEA_TOKEN;
@@
-      delete process.env.GITEA_TOKEN;
@@
-      delete process.env.GITLAB_TOKEN;

As per coding guidelines, "**/*.{ts,tsx,test.ts}: ... keep tests deterministic with no flaky timing or network dependence without guardrails".

Also applies to: 311-425

🤖 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 `@packages/core/src/handlers/clone.test.ts` around lines 304 - 309, The
describe block "multi-forge authentication" uses per-test env cleanup which can
leave stale GITLAB_TOKEN/GITEA_TOKEN values if a test exits early; change the
block to explicitly clear and restore those environment variables in beforeEach
and afterEach so each test runs with a deterministic env. Locate the
"describe('multi-forge authentication')" block and add a beforeEach that deletes
process.env.GITLAB_TOKEN and process.env.GITEA_TOKEN (and any other forge token
envs used by these tests) and an afterEach that also deletes or restores them to
their original values to make tests order-independent and avoid relying on
per-test deletes inside individual tests.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/src/handlers/clone.test.ts (2)

480-510: 💤 Low value

Consider beforeEach/afterEach for env var cleanup consistency.

These unit tests set and delete env vars inline within each test, which works correctly. However, using beforeEach/afterEach (like the multi-forge suite at lines 306-316) would improve consistency and ensure cleanup even if a test assertion fails before reaching the delete statement.

♻️ Optional cleanup pattern alignment

   describe('resolveForgeAuth', () => {
     const { resolveForgeAuth } = require('./clone');
+
+    beforeEach(() => {
+      delete process.env.GH_TOKEN;
+      delete process.env.GITLAB_TOKEN;
+      delete process.env.GITEA_TOKEN;
+    });
+
+    afterEach(() => {
+      delete process.env.GH_TOKEN;
+      delete process.env.GITLAB_TOKEN;
+      delete process.env.GITEA_TOKEN;
+    });
 
     test('returns GH_TOKEN for github.com', () => {
       process.env.GH_TOKEN = 'ghp_abc';
       const result = resolveForgeAuth('https://github.com/owner/repo');
       expect(result).toEqual({ token: 'ghp_abc', scheme: '' });
-      delete process.env.GH_TOKEN;
     });
 
     test('returns GITLAB_TOKEN with oauth2: scheme for gitlab.com', () => {
       process.env.GITLAB_TOKEN = 'glpat-xyz';
       const result = resolveForgeAuth('https://gitlab.com/owner/repo');
       expect(result).toEqual({ token: 'glpat-xyz', scheme: 'oauth2:' });
-      delete process.env.GITLAB_TOKEN;
     });
 
     // ... similar changes for remaining tests

🤖 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 `@packages/core/src/handlers/clone.test.ts` around lines 480 - 510, The tests
around resolveForgeAuth currently set and delete process.env variables inline;
replace those per-test inline mutations with a beforeEach that ensures relevant
env vars (GH_TOKEN, GITLAB_TOKEN) are cleared or set to known defaults and an
afterEach that restores the original process.env entries to guarantee cleanup on
failure; update the tests that currently call process.env.GH_TOKEN /
process.env.GITLAB_TOKEN and delete them to rely on the shared
beforeEach/afterEach setup so each test isolates side effects and mirrors the
multi-forge suite pattern.

---

260-262: ⚡ Quick win

Align cleanup with the new multi-forge suite pattern.

The GH_TOKEN suite sets the token in beforeEach (line 257) but cleans up in afterAll. If a test fails or if only some tests in this suite run, the env var may leak to other tests. The new multi-forge suite (lines 312-316) demonstrates the correct pattern using afterEach for cleanup, ensuring determinism even when tests are interrupted.

♻️ Align env var cleanup with beforeEach/afterEach pattern

-    afterAll(() => {
+    afterEach(() => {
       delete process.env.GH_TOKEN;
     });

🤖 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 `@packages/core/src/handlers/clone.test.ts` around lines 260 - 262, The
GH_TOKEN test suite sets process.env.GH_TOKEN in the beforeEach block but
currently removes it in afterAll, which can leak the env var; change the cleanup
to an afterEach that clears process.env.GH_TOKEN to match the multi-forge suite
pattern and ensure deterministic teardown even if tests fail or run
partially—update the suite surrounding the beforeEach that sets GH_TOKEN to use
afterEach for deletion instead of afterAll.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/src/handlers/clone.ts (1)

327-336: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Custom port on self-hosted forges is still dropped from the authenticated clone URL.

Line 330 still reads parsed.hostname, so an input like https://gitlab.example.com:8443/group/repo.git rebuilds to https://oauth2:<TOKEN>@gitlab.example.com/group/repo.git — the :8443 is lost and git clone will hit the default HTTPS port. This was flagged previously and is one of the explicit PR goals (self-hosted support with custom ports). The PR notes mention a "port-duplication regression" test, but per the current code the failure mode is port omission, not duplication.

Use parsed.host (which is hostname[:port] with no duplication when the URL doesn't carry a port) and add a regression test that asserts the port survives the rewrite for a URL like https://gitlab.example.com:8443/group/repo.git.

🔧 Proposed fix

   if (forgeToken) {
     const parsed = safeParseUrl(workingUrl);
     if (parsed) {
-      cloneUrl = `https://${authScheme}${forgeToken}@${parsed.hostname}${parsed.pathname}`;
+      cloneUrl = `https://${authScheme}${forgeToken}@${parsed.host}${parsed.pathname}`;
     } else if (!workingUrl.startsWith('http')) {
       // Bare host/path form (e.g. github.com/owner/repo)
       cloneUrl = `https://${authScheme}${forgeToken}@${workingUrl}`;
     }
     getLog().debug('clone_authenticated');
   }

🤖 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 `@packages/core/src/handlers/clone.ts` around lines 327 - 336, The
authenticated clone URL builder in clone.ts loses custom ports because it uses
parsed.hostname; change it to use parsed.host (which includes hostname:port)
when constructing cloneUrl in the block that sets cloneUrl =
`https://${authScheme}${forgeToken}@${parsed.hostname}${parsed.pathname}` so it
becomes `https://${authScheme}${forgeToken}@${parsed.host}${parsed.pathname}`;
also add a regression test exercising the code path that parses an input like
`https://gitlab.example.com:8443/group/repo.git` (using safeParseUrl/clone logic
or the function that returns cloneUrl) and assert the resulting authenticated
URL preserves `:8443`.

packages/core/src/handlers/clone.ts (1)

54-63: ⚡ Quick win

Move the resolveForgeAuth JSDoc next to the function it documents.

The docstring on lines 54-57 sits before SELF_HOSTED_FORGE (which has its own JSDoc on line 58), so IDE tooling and tsdoc/API extractors will attach the description to SELF_HOSTED_FORGE (or drop it entirely) instead of resolveForgeAuth on line 65. Relocate the block to immediately above the function.

📝 Proposed reorder

-/**
- * Resolve forge-specific authentication token and URL scheme for a repository URL.
- * Returns the token and auth scheme prefix, or empty values if no token is available.
- */
 /** Well-known self-hosted hostname label patterns → env var + scheme. */
 const SELF_HOSTED_FORGE: { label: string; envVar: string; scheme: string }[] = [
   { label: 'gitlab', envVar: 'GITLAB_TOKEN', scheme: 'oauth2:' },
   { label: 'gitea', envVar: 'GITEA_TOKEN', scheme: '' },
   { label: 'forgejo', envVar: 'GITEA_TOKEN', scheme: '' },
 ];

+/**
+ * Resolve forge-specific authentication token and URL scheme for a repository URL.
+ * Returns the token and auth scheme prefix, or empty values if no token is available.
+ */
 export function resolveForgeAuth(url: string): { token: string | undefined; scheme: 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 `@packages/core/src/handlers/clone.ts` around lines 54 - 63, The JSDoc for
resolveForgeAuth is currently placed above SELF_HOSTED_FORGE so tools will
associate it with the wrong symbol; move the comment block currently on lines
describing "Resolve forge-specific authentication token and URL scheme..." so it
appears immediately above the resolveForgeAuth function declaration (reference:
resolveForgeAuth) and leave the SELF_HOSTED_FORGE JSDoc with the
SELF_HOSTED_FORGE constant; ensure only the resolveForgeAuth description sits
directly above the resolveForgeAuth function signature so tsdoc/IDE pick it up
correctly.