-
Notifications
You must be signed in to change notification settings - Fork 24
Description
Hardening follow-ups from the PR #544 code review. These are warnings and suggestions from the bot reviewer that are valid improvements but not blocking for the initial merge.
Items
1. Fire-and-forget dispatchAgent in slingBead should .catch() log errors
File: cloudflare-gastown/src/dos/Town.do.ts:601
void this.dispatchAgent(hookedAgent, bead) silently swallows synchronous errors. Wrap in .catch(err => console.error(...)) so failures are visible.
2. Validate SDK session creation response shape
File: cloudflare-gastown/container/src/process-manager.ts:301
sessionResult.data ?? sessionResult + 'id' in session is fragile. Add a Zod schema for the session creation response to fail fast if the SDK changes.
3. Verify registerAgent stores rig_id on the agent record
File: cloudflare-gastown/src/handlers/rig-agents.handler.ts:48
Handler passes { ...parsed.data, rig_id: params.rigId } but RegisterAgentInput has rig_id as optional. Verify the agents.ts implementation stores it correctly and add a test.
4. Thread-safe currentTownConfig in control-server
File: cloudflare-gastown/container/src/control-server.ts:30
Module-level mutable state currentTownConfig is overwritten by each request. Single-tenant container makes this safe in practice, but consider passing config through Hono context per-request instead.
5. Emit bead event on failed escalation notification
File: cloudflare-gastown/src/dos/Town.do.ts:1087
routeEscalation fire-and-forgets sendMayorMessage. If mayor notification fails, the escalation is created but nobody knows it was raised. Emit a bead event for dashboard visibility.
6. Atomic rig creation + git credential write
File: src/routers/gastown-router.ts:175
createRig writes git credentials after rig creation. If updateTownConfig fails, the rig exists without git credentials. Consider making this atomic or adding retry/rollback.
7. Ensure /api/gastown/git-credentials route exists
File: cloudflare-gastown/container/src/agent-runner.ts:240
resolveGitCredentialsIfMissing calls /api/gastown/git-credentials on the Next.js server but the route handler may not exist. Verify it exists and test the credential resolution flow end-to-end.
8. Return 403 (not 400) for cross-town access attempts
File: cloudflare-gastown/src/middleware/auth.middleware.ts:88
getTownId() returns null on JWT/route townId mismatch, callers return 400 "Missing townId". Should return 403 to properly signal authorization failure.