Repository navigation
fix(gitlab): report PAT connect and authentication failures accurately - #8536
Conversation
The PAT connect commits the integration and credential before it loads the repository list, so a repository-list refusal used to surface as a failed connection even though the connection was stored. The sync failure is now a warning on the successful result and the existing refresh path fills the cache later, and the settings toast shows the warning. Webhook and project access token calls classified a 401 together with a 403 as a Maintainer-role refusal. A 401 means GitLab rejected the credential, so it now throws its own error kind that webhook sync, project token creation and review preparation route to a reconnect message, while a 403 keeps the role guidance.
The document reload discarded the connect toast before it could be read. Refresh the installation query instead, so the success message with its warning stays visible and the connected view replaces the form.
iscekic
added
drain
|
bot: Verified the GitLab PAT connect result on the local dev stack with the project list refusing at the provider boundary. Observed in Chromium on the GitLab integration settings page: the PAT form validated a token, the connect action completed, and the success toast rendered with the repository-list warning as its description and stayed on screen. The PAT form was then replaced by the connected installation view, so the in-place refresh works without reloading the document.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change clears Assumptions: Files Reviewed (2 files)
Previous Review Summaries (3 snapshots, latest commit 8c441ac)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8c441ac)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit only corrects the repository-cache JSDoc, which is now accurate; the one unresolved defect is that a PAT reconnect does not clear Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 2432479)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe reconnect path still leaves Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Previous review (commit 26d47b4)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe 401/403 split and the warning-on-failed-sync flow work as described, but the existing-integration connect path can return success while leaving stale repository and auth-invalid state, and the new auth detection misses the credential broker's reconnect error. Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Reviewed by deepseek-flash-latest · Input: 30.2K · Output: 2.2K · Cached: 200.4K Review guidance: REVIEW.md from base branch |
A credential-broker reconnect refusal is the same credential problem as a provider 401, so review preparation now treats both through one predicate: the credential is unusable until the owner reconnects, and role guidance cannot fix it. A failed repository sync also drops the cached list when the connect moved to a different GitLab instance, so projects from the previous instance are not shown against the new credential.
A same-instance reconnect keeps its last list until the user refreshes it; only an empty cache is refetched on the next read.
iscekic
added
merge:blocked:kilobot-not-ready
iscekic
added
merge:human
…lure-reporting # Conflicts: # apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts
iscekic
added
merge:human
Evidence
Evidence: demonstrated — the local dev stack exercised PAT connect, repository-sync failure, and authentication reporting. A follow-up database regression reproduced stale credential invalidation after reconnect; the fixed real tRPC route reports the reconnected installation as installed even when repository sync fails.
Change
connectWithPATcommits the integration and credential before repository caching. A cache failure returns success with a warning. Same-instance reconnects preserve the last successful repository list; instance changes discard the previous instance's cache on sync failure.auth_invalid_atandauth_invalid_reasonin the credential transaction. Repository-sync success is not required to restore credential health.Verification
Initial-session evidence retained:
Follow-up verification:
auth_invalid_atremained set. After the fix, the credential persistence suite passed all 21 tests.POSTGRES_URL=<local database> NODE_ENV=test pnpm exec jest --runInBand src/lib/integrations src/lib/code-reviews: 118 suites passed; 1604 tests passed, 1 skipped.pnpm --filter web run typecheckpassed. Targeted oxlint: 0 warnings, 0 errors. Changed TypeScript files formatted with oxfmt;git diff --checkpassed.installed: false. Reconnect with a validated replacement PAT and a project-list 502 returned success with a warning. A subsequentgetInstallationreturnedinstalled: true; Postgres confirmed active status, both invalidation fields cleared, and the replacement credential authorized by the signed-in fake org owner./api/v4/personal_access_tokens/self,/api/v4/user, and/api/v4/projects. No provider writes occurred. This run used only owned fake-account fixtures and local Postgres and provider endpoints.Boundary: provider-shaped responses prove the application's behavior, not acceptance by a real GitLab instance. Project-access-token create/list/rotate/revoke remain covered by adapter/service tests, not HTTP E2E. GitHub and Bitbucket paths are unchanged. This follow-up changes backend reconnect state only; no new visual capture was taken.
Proxy answers
The owner's proxy gave these answers in the owner's place; precedents are paraphrased.
GitLabAuthenticationErrorclass. Confidence: 0.65. Precedent: Fix the class of issue while retaining 403 handling.