fix(gastown): address second round of PR review comments

jrf0110 · jrf0110 · commit 306a61e731e9 · 2026-04-01T14:44:09.000-05:00
Comment: expired kilocode_token can't self-refresh
- Use jose's jwtVerify with a large clockTolerance instead of
  verifyKiloToken so expired but validly-signed tokens are accepted.
  The alarm is the recovery path for expired tokens; rejecting on exp
  would leave the town permanently stuck.

Comment: eviction snapshot --no-verify bypasses pre-push hooks
- Removed --no-verify from the git push in the eviction drain path.
  Repo-level pre-push hooks now run normally.

Comment: force-save commit missing git author identity
- Pass GIT_AUTHOR_NAME/EMAIL and GIT_COMMITTER_NAME/EMAIL from the
  agent's startupEnv into the Bun.spawn env, falling back to
  GASTOWN_GIT_AUTHOR_* from process.env, then to 'Gastown'.

Comment: stale triage restart interrupts unrelated work
- Guard the RESTART/RESTART_WITH_BACKOFF path with an agentStillOnBead
  check before stopping the agent or resetting its status to idle.
  Matches the existing CLOSE_BEAD / REASSIGN_BEAD guard pattern.

Comment: failed review messages disappear from needs-attention
- extractFailureMessage now falls back to metadata.message (from
  review_completed/pr_creation_failed events) when the structured
  failure_reason.message path yields nothing.
diff --git a/cloudflare-gastown/container/src/process-manager.ts b/cloudflare-gastown/container/src/process-manager.ts
@@ -1137,7 +1137,7 @@ export async function drainAll(): Promise<void> {
       const hasOrigin = (await remoteCheck.exited) === 0;
 
       const gitCmd = hasOrigin
-        ? "git add -A && git commit --allow-empty -m 'WIP: container eviction save' && git push --set-upstream origin HEAD --no-verify"
+        ? "git add -A && git commit --allow-empty -m 'WIP: container eviction save' && git push --set-upstream origin HEAD"
         : "git add -A && git commit --allow-empty -m 'WIP: container eviction save'";
 
       if (!hasOrigin) {
@@ -1146,10 +1146,24 @@ export async function drainAll(): Promise<void> {
         );
       }
 
+      // Use the agent's startup env for git author/committer identity.
+      // The control-server's process.env may not have GIT_AUTHOR_NAME set,
+      // but the agent's startupEnv (captured at spawn time) does.
+      const gitEnv: Record<string, string | undefined> = { ...process.env };
+      const authorName =
+        agent.startupEnv?.GIT_AUTHOR_NAME ?? process.env.GASTOWN_GIT_AUTHOR_NAME ?? 'Gastown';
+      const authorEmail =
+        agent.startupEnv?.GIT_AUTHOR_EMAIL ?? process.env.GASTOWN_GIT_AUTHOR_EMAIL ?? 'gastown@kilo.ai';
+      gitEnv.GIT_AUTHOR_NAME = authorName;
+      gitEnv.GIT_COMMITTER_NAME = authorName;
+      gitEnv.GIT_AUTHOR_EMAIL = authorEmail;
+      gitEnv.GIT_COMMITTER_EMAIL = authorEmail;
+
       const proc = Bun.spawn(['bash', '-c', gitCmd], {
         cwd: agent.workdir,
         stdout: 'pipe',
         stderr: 'pipe',
+        env: gitEnv,
       });
       const exitCode = await proc.exited;
       const stdout = await new Response(proc.stdout).text();
diff --git a/cloudflare-gastown/src/dos/Town.do.ts b/cloudflare-gastown/src/dos/Town.do.ts
@@ -57,7 +57,8 @@ import { query } from '../util/query.util';
 import { getAgentDOStub } from './Agent.do';
 import { getTownContainerStub } from './TownContainer.do';
 
-import { verifyKiloToken } from '@kilocode/worker-utils';
+import { verifyKiloToken, kiloTokenPayload } from '@kilocode/worker-utils';
+import { jwtVerify } from 'jose';
 import { generateKiloApiToken } from '../util/kilo-token.util';
 import { resolveSecret } from '../util/secret.util';
 import { writeEvent, type GastownEventData } from '../util/analytics.util';
@@ -1679,18 +1680,24 @@ export class TownDO extends DurableObject<Env> {
       switch (action) {
         case 'RESTART':
         case 'RESTART_WITH_BACKOFF': {
-          // Stop the agent in the container, reset to idle so the
-          // scheduler picks it up again on the next alarm cycle.
-          if (targetAgent?.status === 'working' || targetAgent?.status === 'stalled') {
-            dispatch.stopAgentInContainer(this.env, this.townId, targetAgentId).catch(() => {});
-          }
           if (targetAgent) {
             // Use the bead captured in the triage snapshot (not the agent's
             // current hook, which may have changed since the triage request
             // was created). Fall back to current hook for backward compat.
             const restartBeadId =
               snapshotHookedBeadId ?? targetAgent.current_hook_bead_id;
 
+            // Only stop the agent if it's still working on the snapshot bead.
+            // If it has moved on, stopping it would abort unrelated work.
+            const agentStillOnBead =
+              restartBeadId && targetAgent.current_hook_bead_id === restartBeadId;
+            if (
+              agentStillOnBead &&
+              (targetAgent.status === 'working' || targetAgent.status === 'stalled')
+            ) {
+              dispatch.stopAgentInContainer(this.env, this.townId, targetAgentId).catch(() => {});
+            }
+
             // Check if the hooked bead has exhausted its dispatch cap.
             // If so, fail it immediately instead of letting the reconciler
             // re-dispatch indefinitely (#1653).
@@ -1706,25 +1713,28 @@ export class TownDO extends DurableObject<Env> {
                 break;
               }
             }
-            // RESTART clears last_activity_at so the scheduler picks it
-            // up immediately. RESTART_WITH_BACKOFF sets it to now() so
-            // the dispatch cooldown (DISPATCH_COOLDOWN_MS) delays the
-            // next attempt, preventing immediate restart of crash loops.
-            const activityAt = action === 'RESTART_WITH_BACKOFF' ? now() : null;
-            query(
-              this.sql,
-              /* sql */ `
-                UPDATE ${agent_metadata}
-                SET ${agent_metadata.columns.status} = 'idle',
-                    ${agent_metadata.columns.last_activity_at} = ?
-                WHERE ${agent_metadata.bead_id} = ?
-              `,
-              [activityAt, targetAgentId]
-            );
-            // Also stamp the bead's last_dispatch_attempt_at so the
-            // reconciler's exponential backoff gate fires correctly.
-            // Without this, the backoff variant allows immediate
-            // redispatch once last_dispatch_attempt_at ages out.
+            // Only reset agent state if it's still on the snapshot bead.
+            // If it moved on, let it continue its current work.
+            if (agentStillOnBead) {
+              // RESTART clears last_activity_at so the scheduler picks it
+              // up immediately. RESTART_WITH_BACKOFF sets it to now() so
+              // the dispatch cooldown (DISPATCH_COOLDOWN_MS) delays the
+              // next attempt, preventing immediate restart of crash loops.
+              const activityAt = action === 'RESTART_WITH_BACKOFF' ? now() : null;
+              query(
+                this.sql,
+                /* sql */ `
+                  UPDATE ${agent_metadata}
+                  SET ${agent_metadata.columns.status} = 'idle',
+                      ${agent_metadata.columns.last_activity_at} = ?
+                  WHERE ${agent_metadata.bead_id} = ?
+                `,
+                [activityAt, targetAgentId]
+              );
+            }
+            // Stamp the bead's last_dispatch_attempt_at regardless — even
+            // if the agent moved on, the backoff gate should still fire
+            // on the snapshot bead to prevent immediate redispatch.
             if (action === 'RESTART_WITH_BACKOFF' && restartBeadId) {
               query(
                 this.sql,
@@ -3540,9 +3550,24 @@ export class TownDO extends DurableObject<Env> {
 
     // Verify the existing token's signature before trusting its claims.
     // This prevents a forged token from being re-signed with real credentials.
+    // Use a very large clockTolerance so that already-expired (but validly
+    // signed) tokens are still accepted — this alarm is the recovery path
+    // for expired tokens, so rejecting them on exp would leave the town
+    // permanently stuck if it missed the 7-day refresh window.
     let payload: { kiloUserId: string; apiTokenPepper?: string | null; exp?: number };
     try {
-      payload = await verifyKiloToken(token, secret);
+      const TEN_YEARS_SECONDS = 10 * 365 * 24 * 60 * 60;
+      const { payload: raw } = await jwtVerify(
+        token,
+        new TextEncoder().encode(secret),
+        { algorithms: ['HS256'], clockTolerance: TEN_YEARS_SECONDS }
+      );
+      const parsed = kiloTokenPayload.safeParse(raw);
+      if (!parsed.success) {
+        logger.warn('refreshKilocodeTokenIfExpiring: token payload failed schema validation');
+        return;
+      }
+      payload = parsed.data;
     } catch {
       // Signature invalid or token malformed — don't remint from untrusted claims.
       logger.warn('refreshKilocodeTokenIfExpiring: existing token failed signature verification');
diff --git a/cloudflare-gastown/src/dos/town/review-queue.ts b/cloudflare-gastown/src/dos/town/review-queue.ts
@@ -37,17 +37,26 @@ function now(): string {
   return new Date().toISOString();
 }
 
-/** Extract the human-readable failure message from a bead event's metadata. */
+/**
+ * Extract the human-readable failure message from a bead event's metadata.
+ *
+ * Two sources:
+ *  - status_changed events store it at `metadata.failure_reason.message`
+ *  - review_completed / pr_creation_failed events store it at `metadata.message`
+ */
 function extractFailureMessage(
   status: string,
   metadata: Record<string, unknown> | null | undefined
 ): string | null {
   if (status !== 'failed' || !metadata) return null;
+  // Structured failure_reason (from status_changed events via updateBeadStatus)
   const fr = metadata.failure_reason;
   if (typeof fr === 'object' && fr !== null && 'message' in fr) {
     const msg = (fr as Record<string, unknown>).message;
     if (typeof msg === 'string') return msg;
   }
+  // Top-level message (from review_completed / pr_creation_failed events)
+  if (typeof metadata.message === 'string') return metadata.message;
   return null;
 }
 
