fix(gastown): production fixes, settings cleanup, and E2E testing#2034
fix(gastown): production fixes, settings cleanup, and E2E testing#2034
Conversation
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)N/A Files Reviewed (4 files)
Reviewed by gpt-5.4-20260305 · 545,342 tokens |
5e0407e to
d4dcf65
Compare
…CF Access pattern The debug/beads/:beadId endpoint should be accessible in production (behind CF Access), matching the pattern of debug/status and other debug endpoints. The dev-only guard was incorrectly blocking production debugging.
…code_review is off When refineryCodeReview is disabled, only skip Rules 5-6 if all open MR beads have a pr_url. MR beads without pr_url (direct merge strategy) still need the refinery to merge, even when code review is disabled.
…les, add convoy merge mode setting - refinery.code_review: whether the refinery reviews PRs at all (gates + diff) - refinery.code_review_comments: whether the refinery posts GitHub review comments (only relevant when code_review is on). When off, the refinery uses internal rework requests instead of GitHub comments. - convoy_merge_mode: town-level default for new convoys — 'review-then-land' (single landing PR) or 'review-and-merge' (individual PR per bead). - Settings UI: conditional comments toggle under code review, convoy merge mode selector with review-then-land / review-and-merge buttons.
…ays create PRs
Settings are now:
1. Refinery reviews (bool) — toggle refinery on/off entirely
2. Merge strategy (direct/pr) — already existed
3. Review mode (rework/comments) — how refinery communicates findings
4. Auto-resolve PR feedback (bool) — auto-dispatch polecat for comments/CI
5. Auto-merge + delay — auto-merge when PR is green
- Replaced code_review_comments bool with review_mode enum ('rework'|'comments')
- Polecats always create PRs when merge_strategy is 'pr' (removed
review-then-land convoy exception — even intermediate beads get PRs)
- UI: review mode is a button selector, shown only when code review is on
- Convoy merge mode selector already added in previous commit
… transition, cleanup P0: poll_pr now calls checkPRFeedback once and reuses the result for both auto-resolve and auto-merge, halving GitHub API consumption. P1: Rule 7 now emits transition_agent to idle alongside stop+unhook, eliminating the 1-tick inconsistent working+unhooked state. P1: Removed duplicate metadata column from MR beads SQL query. P1: Clarified the --approve prohibition in the refinery prompt to explain it's a GitHub limitation (bot can't approve own PRs). P2: PR comments review mode button is disabled when merge strategy is not 'pr', with an explanatory tooltip.
…lse, add debug config endpoints Bug fix: When code_review=false, the reconciler now transitions ALL open MR beads to in_progress (not just those with pr_url). This prevents the refinery from being dispatched during the timing gap between MR bead creation and pr_url population. MR beads without pr_url will be caught by the orphan timeout. Also adds debug GET/PATCH /debug/towns/:townId/config endpoints (dev only) for reading/updating town config without auth during local testing. Updates e2e-pr-feedback-testing.md with 3 test scenarios, pre-flight checklist, debug config endpoint docs, and known bugs.
…des, convoy target branch, direct-merge gate Thread 1: Always pass existingPrUrl to refinery prompt regardless of review_mode. The review mode now controls HOW findings are communicated (comments vs gt_request_changes) via the reviewMode param, not WHETHER the refinery knows the PR exists. Thread 2: Polecat prompt now resolves the convoy feature branch for review-then-land intermediate beads, so PRs target the convoy branch instead of main. Thread 3: Only fast-track MR beads with pr_url when code_review=false. Direct-merge MR beads (no pr_url) still dispatch the refinery for the merge operation via Rules 5-6.
The rework flow for existing PRs was using gt_mail_send which only delivers mail — it doesn't create the blocking rework bead needed to dispatch a polecat for fixes. Now uses gt_request_changes which correctly creates the rework bead and hands the MR back.
1183058 to
34d98d6
Compare
| app.patch('/debug/towns/:townId/config', async c => { | ||
| if (c.env.ENVIRONMENT !== 'development') return c.json({ error: 'dev only' }, 403); | ||
| const townId = c.req.param('townId'); | ||
| const parsed = TownConfigUpdateSchema.safeParse(await c.req.json()); |
There was a problem hiding this comment.
WARNING: Malformed JSON still bypasses the new validation
safeParse(await c.req.json()) only runs after the request body has already been parsed. If a caller sends syntactically invalid JSON, c.req.json() throws before Zod sees it, so this debug endpoint still returns the worker's generic 500 instead of the 400 this validation path is trying to provide. Reusing parseJsonBody() here, like the authenticated config handler does, would keep bad debug requests from surfacing as server errors.
repoRoot was one level up (../), but after the move from cloudflare-gastown/ to services/gastown/ it needs to be two levels up (../../) to reach the workspace root.
| const [refineryGates, setRefineryGates] = useState<string[]>([]); | ||
| const [autoMerge, setAutoMerge] = useState(true); | ||
| const [refineryCodeReview, setRefineryCodeReview] = useState(true); | ||
| const [reviewMode, setReviewMode] = useState<'rework' | 'comments'>('rework'); |
There was a problem hiding this comment.
One day we'll migrate to https://react-hook-form.com/ and not have a bajillion useState's
Summary
Production debugging, settings model cleanup, and E2E test infrastructure from the gastown-staging stabilization effort. Fixes several issues discovered during production monitoring of town
5f5fda7f.Production fixes
code_review=false— When disabled, the reconciler now transitions ALL open MR beads toin_progress(not just those withpr_url), preventing the refinery from sneaking through the timing gap between MR bead creation andpr_urlpopulation.rig_idvalues (confirmed viadebugGetBead).completeReviewWithResultonly closes the source bead when it's inin_reviewstate, preventing stale MR beads from incorrectly closing active source beads.code_review=falsebut MR beads have nopr_url(direct merge), the refinery is still dispatched for the merge operation.Settings model cleanup
review_modeenum replacescode_review_commentsboolean —'rework'(internal gt_request_changes) or'comments'(GitHub review comments on PR)'review-then-land'or'review-and-merge'merge_strategy: 'pr'(removed review-then-land convoy exception)Code quality (from sub-agent review)
poll_prnow callscheckPRFeedbackonce and reuses for both auto-resolve and auto-merge (halves GitHub API consumption)transition_agentto idle alongside stop+unhook (eliminates 1-tick inconsistent state)metadatacolumn from MR beads SQL query--approveprohibition in refinery prompt (GitHub limitation)Debug infrastructure
GET /debug/towns/:townId/beads/:beadId— Full bead inspection (review_metadata, dependencies)GET/PATCH /debug/towns/:townId/config— Read/update town config in devVerification
pnpm typecheck— passes (full repo)pnpm -r --filter cloudflare-gastown test— 181 tests pass5f5fda7f— reconciler 0 violations after deploy, refinery dispatching correctlyVisual Changes
Reviewer Notes
review_modeenum replaces the short-livedcode_review_commentsboolean (which was only in the previous commit batch)convoy_merge_modetown config field usesconvoy_prefix to namespace it vs the per-convoymerge_modecolumncode_review=falsegate now skips Rules 5-6 unconditionally — MR beads withoutpr_urlare handled by orphan timeout rather than dispatching a refinery the user disabled