feat(gastown): configurable merge strategy (direct/PR)#701
Merged
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)No additional issues found outside the diff. Files Reviewed (75 files)Infrastructure & Build:
Core Backend (Worker):
tRPC Migration:
Auth & Security:
Platform PR/MR:
Container Agent:
Frontend (tRPC migration + merge strategy UI):
Deleted:
Tests:
|
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
5 times, most recently
from
3846180 to
b3274c2
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
2 times, most recently
from
091068c to
b01e42a
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
b01e42a to
8d28245
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
8d28245 to
69680fe
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
4 times, most recently
from
f5a6c64 to
de8771d
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
2 times, most recently
from
7324fca to
58324af
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
58324af to
3fdb05f
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
2 times, most recently
from
facacdc to
7bc9749
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
7bc9749 to
ce705d3
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
ce705d3 to
d10c0e8
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
d10c0e8 to
b899c21
Compare
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
b899c21 to
b80f29f
Compare
pandemicsyn
approved these changes
Mar 5, 2026
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
3 times, most recently
from
db8deff to
5dd6a1a
Compare
… review bead fixes - Add configurable merge strategy (direct/PR) for rigs and towns - Refinery agents now handle merging (direct) or PR creation (gh/glab CLI) themselves - Strategy-aware refinery system prompt with explicit merge/PR instructions - Fix review beads missing PR link: write pr_url to beads.metadata on submission - Fix PR beads not closing: validate pr_url is a real PR, add polling diagnostics - Add small_model to TownConfig for configurable lightweight model - Use resolveModel()/resolveSmallModel() throughout instead of hardcoded model strings - All kilo config sub-agent roles derive model from town config - Add PR link display to BeadPanel.tsx drawer - Polecat prompt: clarify not to pass git push convenience URLs as pr_url
- Add kiloAuthMiddleware: validates Kilo user JWTs (NEXTAUTH_SECRET) via verifyKiloToken from @kilocode/worker-utils - Apply kiloAuthMiddleware to user-facing routes (/api/users/*, /api/towns/*/config, /api/towns/*/container/*, /api/towns/*/convoys/*, /api/towns/*/escalations/*, /api/towns/*/mayor/*) - Remove global CF Access middleware from all routes - Replace CF Access validation in WebSocket upgrade handler with Kilo JWT validation - Update gastown-client.ts to send Bearer token (via generateInternalServiceToken) instead of CF-Access-Client-Id/Secret headers - Add NEXTAUTH_SECRET binding to wrangler.jsonc secrets store - Add NEXTAUTH_SECRET to Env type in worker-configuration.d.ts - Keep existing agent JWT auth (GASTOWN_SESSION_TOKEN) for container→worker routes - Health and dashboard endpoints remain unauthenticated
…C proxy The gastown frontend now calls the Cloudflare Worker's tRPC endpoint directly instead of proxying through the Next.js backend. This removes a network hop and lets the worker handle auth end-to-end. Key changes: - Add /api/gastown/token endpoint that mints short-lived JWTs with isAdmin and apiTokenPepper claims for browser→worker auth - Create standalone gastown tRPC client (src/lib/gastown/trpc.ts) with token management, GastownTRPCProvider, and gastownWsUrl helper - Add .output() schemas to all worker tRPC procedures for type-safe generated declarations - Add GIT_TOKEN_SERVICE binding to worker for git credential refresh (replaces server-side refreshTownGitCredentials) - Add configureRig/addRig calls to worker's createRig procedure - Add setTownId calls to ensure TownDO uses correct UUID for containers - Add CORS for /trpc/* routes and always-on kiloAuthMiddleware - Worker requireAdmin now checks JWT claims instead of DB lookup - WebSocket URLs returned as relative paths (frontend constructs full URL) - Update ~25 frontend files: useTRPC → useGastownTRPC - Remove gastown-router.ts, gastown-client.ts, and root-router registration
…robustness - Remove console.log(secret) that leaked NEXTAUTH_SECRET to stdout - Replace raw error logging with sanitized message in kilo-auth middleware - Add verifyRigOwnership check to deleteRig tRPC procedure - Use TownConfigUpdateSchema instead of z.record for updateTownConfig input - Validate GitLab host in checkPRStatus before sending PRIVATE-TOKEN - Handle empty model strings in kiloModel with fallback to kilo/auto - Replace 'as' cast with Zod validation in gastown trpc.ts fetchToken - Add warning log for unrecognized PR URL formats in checkPRStatus - Handle HTTP 422 (duplicate PR) in createGitHubPR by fetching existing PR - Add console.warn for label application failures instead of silent catch - Emit pr_creation_failed event when refinery provides invalid pr_url - Add git status instruction to refinery prompt per reviewer feedback - Use MergeStrategy type import in refinery prompt instead of inline union - Strip workspace: references from dependencies (not just devDependencies) - Remove unused TOKEN_EXPIRY import to fix lint error
- Add SSRF protection to createGitLabMR: validate instanceUrl host against gitlab.com or configured instance URL before sending PRIVATE-TOKEN - Log HTTP error status codes in checkPRStatus for both GitHub and GitLab API responses instead of silently returning null - Add TownConfigUpdateSchema tests verifying no phantom defaults are injected on empty input
jrf0110
force-pushed
the
473-configurable-merge-strategy
branch
from
4440eb8 to
0bae000
Compare
This was referenced Mar 5, 2026
| // The browser constructs the full ws(s):// URL using the known GASTOWN_URL. | ||
|
|
||
| export function gastownWsUrl(relativePath: string): string { | ||
| const base = new URL(GASTOWN_URL); |
Contributor
There was a problem hiding this comment.
[WARNING]: new URL(GASTOWN_URL) throws TypeError when NEXT_PUBLIC_GASTOWN_URL is not configured
GASTOWN_URL defaults to '' (empty string) in src/lib/constants.ts:48 when the env var is unset. new URL('') throws TypeError: Invalid URL, crashing every gastownWsUrl() call.
Similarly, gastownTrpcUrl on line 71 would evaluate to /trpc (a relative path), which may not work with httpBatchLink that typically expects an absolute URL.
Consider guarding against an empty GASTOWN_URL:
export function gastownWsUrl(relativePath: string): string {
if (!GASTOWN_URL) throw new Error('NEXT_PUBLIC_GASTOWN_URL is not configured');
const base = new URL(GASTOWN_URL);
...
}Or validate early at module scope so the error surfaces at startup rather than on the first WS connection attempt.
| townId: z.string().uuid(), | ||
| message: z.string().min(1), | ||
| model: z.string().default('anthropic/claude-sonnet-4.6'), | ||
| rigId: z.string().uuid().optional(), |
Contributor
There was a problem hiding this comment.
[SUGGESTION]: Unused input field — rigId is declared but never read
rigId is accepted in the sendMessage input schema but is not passed to townStub.sendMayorMessage(input.message, input.model). If it's intended for future use, consider removing it now and adding it when the mayor actually uses it. Dead schema fields create confusion about what the procedure actually depends on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #473 — Configurable Merge Strategy, a sub-issue of #204.
Video walkthrough: https://storage.j0.hn/gastown-merge-strategy-1.mp4
merge_strategyconfig field (direct|pr) at the town and rig level, with rig-level overrides inheriting from the town defaultpr, the Refinery creates a GitHub PR or GitLab MR instead of pushing directly to the default branch, enabling human review before agent work landsChanges by area
Schema & Config
MergeStrategyZod enum andmerge_strategyfield onTownConfigSchema(defaults todirect)merge_strategyoverride onRigConfigresolveMergeStrategy()helper: rig override → town default →directPR/MR Creation (
platform-pr.util.ts— new)parseGitUrl()— extracts platform/owner/repo from HTTPS and SSH git URLscreateGitHubPR()/createGitLabMR()— REST API calls with labelsbuildPRBody()— Markdown body with quality gate results and agent attributionReview Queue & Town DO
setReviewPrUrl(),markReviewInReview(),listPendingPRReviews()on review queueexecuteMergeStrategy()dispatches to direct merge or PR creationtriggerPRCreation()creates the PR and tracks the URL on the MR beadpollPendingPRs()+checkPRStatus()poll GitHub/GitLab APIs for PR statepr_createdandpr_creation_failedbead event typesRefinery Prompt
mergeStrategyparam; whenpr, instructs the refinery to push the branch and callgt_done(TownDO creates the PR) instead of merging itselfDashboard UI
BeadDetailDrawerDev Infrastructure
192.168.65.254) instead ofhost.docker.internalwhich doesn't work on wrangler'sworkerd-network0.0.0.0so containers can reach the workercatalog:references in Dockerfiles for bun compatibilityresolveJWTSecretTests — 23 unit tests covering merge strategy resolution, git URL parsing, and PR body generation
How to test
merge_strategy: "pr"in town settingsCloses #473