Repository navigation
feat(review-fix): confirm fix sessions before starting them - #8499
Conversation
The review fix link now opens a confirmation page on the Kilo origin, and the session is prepared from that page's form submission. The submission's origin is validated before the review is read and before a session is prepared. The per-actor session start cap moves out of the REVIEW.md conversion route into lib/code-reviews/core/session-start-rate-limit, and the fix route applies it after the review read proves access. Each flow keeps its own bucket; a capped reader lands back on the confirmation page with an explanation.
iscekic
added
drain
![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 three prior suggestions are fixed at HEAD Assumptions: Files Reviewed (7 files)
Previous Review Summary (commit d561419)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d561419)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe confirmation gate, origin check, and per-actor session cap are correctly ordered and access-gated; the remaining findings are hardening and copy accuracy around the new POST origin check and the confirmation page. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
Reviewed by deepseek-flash-latest · Input: 38K · Output: 3.4K · Cached: 248.2K Review guidance: REVIEW.md from base branch |
|
bot: The review fix link renders this confirmation page on the local dev stack, and the session starts from its button.
|
- Accept only `same-origin` from `Sec-Fetch-Site` when `Origin` is absent. - Explain a rejected submission on the confirmation page instead of returning the reader there unchanged. - Word the credits sentence for organization reviews.
|
bot: The confirmation page after the copy update, on the local dev stack.
|
|
bot: The later capture (02) supersedes the earlier screenshot (01): it shows the current confirmation page copy. |
iscekic
added
the
intent:ok
iscekic
added
merge:blocked:insufficient-e2e
|
Changes requested: merge |
iscekic
added
merge:human
|
bot: Merged Route tests rerun on the merged tree ( |
Evidence
Evidence: demonstrated — a local dev-stack rerun at this head exercised the confirmation step, an accepted submission that ran the server-side review-comment selection, and the per-actor cap; session rows and Redis counters were read back in the running database and cache.
What changes
apps/web/src/app/cloud-agent-fork/review/[reviewId]/route.ts:GETrenders the confirmation step for the review, andPOSTprepares the fix session only for a same-origin submission of that page's form. The session start runs after the review read proves access, after the per-actor session start cap, and after the server-side review-comment selection. Both responses stay on the app origin (relativeLocation,no-store).apps/web/src/app/cloud-agent-fork/review/[reviewId]/confirm/page.tsx: the confirmation page. It requires a session (sign-in returns to this page), explains what the session will do, and posts the form to the route.apps/web/src/lib/code-reviews/core/session-start-rate-limit.ts: the Redis fixed-window session start cap extracted fromreview-md/route.ts. That route keeps its key and limits (review-md-conversion, 30/hour); fix sessions use areview-fix-sessionbucket withREVIEW_FIX_SESSION_RATE_LIMIT/REVIEW_FIX_SESSION_RATE_WINDOW_SECONDS(30/hour).route.test.tscovers the confirmation step, the app-origin requirement, the cap (including that it runs only after access is proven and before comment selection), the sign-in callback path, and the selected-comment prompt on both personal and organization starts.confirm/page.test.tscovers the invalid id redirect, the sign-in callback, the form target, and the capped message.This branch was merged with
main, so the fix session usesselectFixReviewCommentsForReviewand builds its prompt from the resolved comments exactly as the sibling fix-review flow does.Verification
Rerun on a fresh work VM at this head (
f48527155e): dev stack with Postgres and Redis,nextjson :3000, thecloud-agent-nextworker on :8794, one seeded fake account plus a code review row owned by it, a linked GitHub app integration, and a local stand-in for the provider API the selection reads (installation token, review comments, review threads, collaborator permission). Each request was made over HTTP with a real signed-in session cookie.302->/cloud-agent-fork/review/<id>/confirm; no session row302-> the same confirmation path; no session row200, headingStart a fix session, submit buttonStart fix session, form action/cloud-agent-fork/review/<id>307->/users/sign_in?callbackPath=%2Fcloud-agent-fork%2Freview%2F<id>%2Fconfirm303->.../confirm?error=invalid_origin; no session row303->.../confirm?error=invalid_origin; no session row302->/users/sign_in?callbackPath=...; no session row302->/code-reviews?error=access_denied; no session row, no counter for the review owner, no provider request303->/cloud/chat?sessionId=ses_...; one new session rowautoInitiateset303->.../confirm?error=rate_limited; no session row and no provider request; the counter advanced onceYou have started several fix sessions in the last hour. Try again later.302->/code-reviews?error=invalid_conversion_requestreview-md-conversioncounter, which is separate from the fix-session counter302and307->/code-reviews?error=invalid_review_idUnit checks:
pnpm exec jest --runInBand src/app/cloud-agent-forkinapps/webpasses (3 suites, 40 tests); the changed files pass the root oxlint config.No screenshots were published: this VM's agent reads pages as text, so I could not inspect image pixels or metadata before publishing. The rendered text above was read from the live DOM.
Production prerequisites
None. The cap reuses the existing Redis client and adds no environment variable, secret, binding, migration, or external configuration.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Uniform confirmation page (Recommended)
Confidence: 0.55 · Precedent: Speed and least work: replace old paths instead of keeping them alongside; session ad6a3f589364 "alongside? why not instead?"
Answer: New in-app page (Recommended)
Confidence: 0.70 · Precedent: UX and design quality: hold all UI work to the best-in-class standard within the design system; session df491946927a accepted "Improve within the design system"
Answer: Shared helper, per-flow buckets (Recommended)
Confidence: 0.60 · Precedent: Engineering conventions: extract shared code into an internal library; session 5d8113f51c48 "extract any common code to an internal library"