Repository navigation
Conversation
A manual Code Reviewer job authorizes the pasted project against the connection's cached repository scope before it resolves the connection's credential: the project has to be one of that connection's repositories, and the pasted instance has to be the connected one. The review row records the authorized repository identity (canonical full path and provider id) and the integration it belongs to. Classified provider refusals map to tRPC errors through one shared helper used by the provider-review router, the branch listing and the manual job, and the manual job's provider read failures answer classified client errors.
iscekic
added
drain
The branch listing and the manual Code Reviewer job each built the same GitLab review owner by hand. Expose one gitlabReviewOwnerFor helper in the review layer and use it in both, so the organization-owner-plus-acting-user pair has a single definition.
Pin the manual job's provider read refusals: a refused merge request read answers NOT_FOUND and a refused credential answers the reconnect state, both without creating a review or dispatching one.
One preparation attempt runs the sandbox create, the wrapper's first connect and hello, the snapshot restore with its clone fallback, the Kilo runtime start and the session import in sequence. Each step keeps its own bound, so the attempt deadline is derived from their sum instead of a fixed 12 minutes: a shorter deadline fails a slow but valid preparation as preparation_timeout while every step stayed inside its own budget. The step bounds, a permanent create failure and the reconnect window still fail a stuck preparation earlier.
…wn steps" This reverts commit 155b6f4.
![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 follow-up pins the authorized cached repository id in the manual GitLab review row and makes the out-of-scope refusal name the actionable repository-refresh remedy; both previous findings are resolved. Assumptions: Files Reviewed (2 files)
Previous Review Summary (commit 70fd47f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 70fd47f)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe GitLab authorization scoping change is sound, but manual reviews now hard-fail as Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (7 files)
Reviewed by deepseek-flash-latest · Input: 63.5K · Output: 4.6K · Cached: 209.5K Review guidance: REVIEW.md from base branch |
…sitory list The repository list is the project's authorization scope, so the refusal now tells the user to refresh the integration's repository list. The manual job test fixture keeps the provider's project ids different from the cached repository id, so the assertion pins which id the review row stores.
iscekic
added
the
intent:ok
|
bot: End-to-end run on the work VM through
Stub paths observed: Head e1b4967. Responses are stub-shaped; no real GitLab instance was contacted, and GitHub/Bitbucket provider paths were not re-run. |
iscekic
added
merge:human
…eview-scope # Conflicts: # apps/web/src/routers/provider-review-router.ts
iscekic
added
merge:human
Evidence
Evidence: demonstrated — the manual Code Reviewer GitLab scope boundary was exercised end to end on this VM's local dev stack (real Next.js server, Postgres, tRPC and dev fake sign-in for an owned fixture account), with the GitLab API and the git-token-service answered by one local forge stub at the network boundary. Driven through the organization and personal manual Code Reviewer procedures at head
917f480.A manual GitLab job requires the pasted merge request to name a project in the connection's repository list, on the connected instance. Observed for each case, with the stub's request log read before and after every call:
group/sub/repo, merge request 7): HTTP 200{ outputMode: 'provider' }. The stub saw onePOST /internal/gitlab/credentials, oneGET /api/v4/projects/group%2Fsub%2Frepo/merge_requests/7carrying the bearer token, and then, during session preparation, onePOST /api/v4/projects/123/access_tokensfor the connection's repository id. Postgres holds one review row: repogroup/sub/repo, merge request 7, platformgitlab,platform_project_id123, the seeded connection id, triggermanual, output modeprovider.group/other): HTTP 404NOT_FOUND, naming the refresh-the-repository-list remedy. The stub saw no credential request and no provider request, and Postgres gained no review row and no attempt row for that project.NOT_FOUND, no credential request, no provider request.NOT_FOUND, after one credential request and one merge request read. Both merge request URL refusals use the same client answer.PRECONDITION_FAILED, no credential request, no provider request.Provider boundary: the GitLab instance is the connection's configured default, rewritten to the local stub by a server-side fetch preload; the token broker is redirected by env to the same stub. No real GitLab instance was contacted, so a real provider accepting these requests stays unproven. Review execution stays best-effort (the worker URL points at the stub, and the created review row settles as
failedafter dispatch). GitHub and Bitbucket provider paths are untouched by this change and were not re-run.Unit runs at this head:
manual-code-review-jobs,gitlab-authorizationandprovider-branch-listing(65 tests) pass.Change
authorizeProject, the boundary the GitLab branch listing already uses: the project must be one of the connection's cached repositories (case-insensitive full path) and the pasted origin must be the connected instance. A refusal isNOT_FOUND.GitLabProjectAccesscarries the resolved integration and repository ids, andgitlabReviewOwnerForis the single definition of the review owner shared by the branch listing and the manual job.provider-review/error-mapping) used by the provider-review router, the branch listing and the manual job, so a refused merge request read answers a classified client error.manual-code-review-jobs.test.tsandgitlab-authorization.test.tscover a listed project, an unlisted project refused before any credential or provider call, a URL on another instance, a missing connection precondition, a refused merge request read, a refused credential, and the organization credential actor.Production prerequisites
None.
Proxy answers
Answer: NOT_FOUND
Confidence: 0.90
Answer: Keep it
Confidence: 0.70
Answer: Yes, map them
Confidence: 0.80
Answer: Shared export, migrate the others too
Confidence: 0.50
Answer: Authorized cache id first
Confidence: 0.50
Answer: Push a small real follow-up commit
Confidence: 0.80
Answer: One more real commit to re-trigger
Confidence: 0.80
Answer: Do not report blocked. Treat a fleet-wide preparation timeout as a workflow bug and fix its class: find why review environment preparation times out and raise the timeout when it cuts real work short, then re-trigger the review on this pull request. Merging main into the branch is a legitimate new head. Check progress every 10–15 minutes until the pull request is human ready, and keep the real code unchanged.
Confidence: 0.60
Answer: Connection cache only (current)
Confidence: 0.75
Answer: Second PR for the worker fix, then merge main
Confidence: 0.72
Answer: Not merged, and do not wait on me. I merge by hand later. Leave fix(cloud-agent-next): budget a preparation attempt for its own steps #8819 ready for review, assigned to me, as its own PR. Remove the worker change from fix(code-reviews): scope manual GitLab merge request sources #8799 so it holds only the GitLab authorization fix and its tests, and keep it marked merge-by-human. Continue the fix(code-reviews): scope manual GitLab merge request sources #8799 loop now: retrigger the kilobot review, and if it times out, treat that as a workflow bug to fix, not a stop. Address each comment, keep CI green, and post the e2e proof. Merge main into fix(code-reviews): scope manual GitLab merge request sources #8799 only after fix(cloud-agent-next): budget a preparation attempt for its own steps #8819 lands, or if a conflict needs it.
Confidence: 0.50
Answer: End handled now
Confidence: 0.62