Merge branch 'nwparker/term-speed-2-side-effect-cleanup' into nwparker/term-speed-2-hidden-delivery-gate

src/shared/execution-host-registry.test.ts

@@ -1,8 +1,10 @@
 import { describe, expect, it } from 'vitest'
+import { getExecutionHostLabel } from './execution-host'
 import { MIN_COMPATIBLE_RUNTIME_SERVER_VERSION, RUNTIME_PROTOCOL_VERSION } from './protocol-version'
-import { getLocalExecutionHostLabel } from './execution-host'
 import { buildExecutionHostRegistry } from './execution-host-registry'
 
+const LOCAL_HOST_LABEL = getExecutionHostLabel('local')
+
 describe('execution host registry', () => {
   it('returns only the local host for local-only state', () => {
     expect(
@@ -14,7 +16,7 @@ describe('execution host registry', () => {
       {
         id: 'local',
         kind: 'local',
-        label: getLocalExecutionHostLabel(),
+        label: LOCAL_HOST_LABEL,
         detail: 'This computer',
         health: 'local'
       }
@@ -189,7 +191,7 @@ describe('execution host registry', () => {
     })
 
     expect(hosts).toMatchObject([
-      { id: 'local', label: getLocalExecutionHostLabel() },
+      { id: 'local', label: LOCAL_HOST_LABEL },
       { id: 'ssh:repo-ssh', label: 'Derived SSH' }
     ])
   })

src/shared/execution-host.test.ts

@@ -71,4 +71,10 @@ describe('execution host identity', () => {
       'runtime:runtime-1'
     )
   })
+
+  it('labels local execution hosts by platform', () => {
+    expect(getLocalExecutionHostLabel('darwin')).toBe('Local Mac')
+    expect(getLocalExecutionHostLabel('linux')).toBe('Local Linux')
+    expect(getLocalExecutionHostLabel('win32')).toBe('Local Windows')
+  })
 })

src/shared/execution-host.ts

@@ -13,42 +13,41 @@ export type ParsedExecutionHost =
   | { kind: 'ssh'; id: `ssh:${string}`; targetId: string }
   | { kind: 'runtime'; id: `runtime:${string}`; environmentId: string }
 
+function getCurrentLocalPlatform(): NodeJS.Platform | null {
+  const globalNavigator = (globalThis as { navigator?: { userAgent?: string; platform?: string } })
+    .navigator
+  const userAgent = globalNavigator?.userAgent || globalNavigator?.platform || ''
+  if (/Windows/i.test(userAgent)) {
+    return 'win32'
+  }
+  if (/Mac/i.test(userAgent)) {
+    return 'darwin'
+  }
+  if (/Linux|X11/i.test(userAgent)) {
+    return 'linux'
+  }
+  return typeof process === 'undefined' ? null : process.platform
+}
+
+export function getLocalExecutionHostLabel(platform: NodeJS.Platform | null = null): string {
+  const localPlatform = platform ?? getCurrentLocalPlatform()
+  if (localPlatform === 'darwin') {
+    return 'Local Mac'
+  }
+  if (localPlatform === 'win32') {
+    return 'Local Windows'
+  }
+  if (localPlatform === 'linux') {
+    return 'Local Linux'
+  }
+  return 'This computer'
+}
+
 function normalizeHostPart(value: string | null | undefined): string | null {
   const trimmed = value?.trim()
   return trimmed ? trimmed : null
 }
 
-function getCurrentHostPlatform(): string {
-  if (typeof process !== 'undefined' && typeof process.platform === 'string') {
-    return process.platform
-  }
-  if (typeof navigator !== 'undefined') {
-    if (navigator.userAgent.includes('Windows')) {
-      return 'win32'
-    }
-    if (navigator.userAgent.includes('Linux')) {
-      return 'linux'
-    }
-    if (navigator.userAgent.includes('Mac')) {
-      return 'darwin'
-    }
-  }
-  return ''
-}
-
-export function getLocalExecutionHostLabel(platform = getCurrentHostPlatform()): string {
-  switch (platform) {
-    case 'darwin':
-      return 'Local Mac'
-    case 'win32':
-      return 'Local Windows'
-    case 'linux':
-      return 'Local Linux'
-    default:
-      return 'This computer'
-  }
-}
-
 export function toSshExecutionHostId(targetId: string): `ssh:${string}` {
   return `ssh:${encodeURIComponent(targetId)}`
 }

src/shared/gitlab-types.ts

@@ -63,6 +63,8 @@ export type MRInfo = {
   authorAvatarUrl?: string | null
   /** GitLab MR head SHA — pipeline status is keyed off the head commit. */
   headSha?: string
+  /** Target branch name for review-created worktree compare-base repair. */
+  baseRefName?: string
   conflictSummary?: PRConflictSummary
 }
 

src/shared/hosted-review-creation-providers.ts

@@ -0,0 +1,20 @@
+import type { HostedReviewProvider } from './hosted-review'
+
+export type HostedReviewCreationProvider = 'github' | 'gitlab' | 'azure-devops' | 'gitea'
+
+export function supportsHostedReviewCreation(
+  provider: HostedReviewProvider | null | undefined
+): provider is HostedReviewCreationProvider {
+  return (
+    provider === 'github' ||
+    provider === 'gitlab' ||
+    provider === 'azure-devops' ||
+    provider === 'gitea'
+  )
+}
+
+export function resolveHostedReviewCreationProvider(
+  provider: HostedReviewProvider | null | undefined
+): HostedReviewCreationProvider {
+  return supportsHostedReviewCreation(provider) ? provider : 'github'
+}

src/shared/hosted-review.ts

@@ -25,6 +25,8 @@ export type HostedReviewInfo = {
   mergeQueueRequired?: boolean | null
   mergeStateStatus?: string | null
   headSha?: string
+  /** Target branch name for review-created worktree compare-base repair. */
+  baseRefName?: string
   conflictSummary?: PRConflictSummary
 }
 

src/shared/pull-request-generation.test.ts

@@ -27,6 +27,19 @@ describe('buildPullRequestFieldsPrompt', () => {
     expect(prompt).toContain('Additional user prompt:')
     expect(prompt).toContain('Use conventional PR titles.')
   })
+
+  it('tells the agent to preserve existing review templates', () => {
+    const prompt = buildPullRequestFieldsPrompt(
+      {
+        ...context,
+        currentBody: '## Summary\n\n## Testing\n\n- [ ] Required checks'
+      },
+      ''
+    )
+
+    expect(prompt).toContain('preserve its headings, required sections, and checklists')
+    expect(prompt).toContain('Leave genuinely unknown template items as TODO or unchecked')
+  })
 })
 
 describe('parseGeneratedPullRequestFields', () => {

src/shared/pull-request-generation.ts

@@ -41,6 +41,8 @@ export function buildPullRequestFieldsPrompt(
     '- Keep the base branch as the current base unless the diff clearly targets a different branch.',
     '- Title: concise, specific, no trailing period.',
     '- Body: useful Markdown summary for reviewers. Include testing notes only when evidence exists.',
+    '- If Current description contains a pull request or merge request template, preserve its headings, required sections, and checklists while filling relevant sections from the branch changes.',
+    '- Leave genuinely unknown template items as TODO or unchecked instead of deleting them.',
     '- draft: true only when the changes clearly look unfinished, WIP, or unsafe to review.',
     '- Do not include labels, reviewers, code fences, prose, or any keys beyond base/title/body/draft.',
     '',

src/shared/shell-process-detection.ts

@@ -4,13 +4,20 @@
 const SHELL_NAMES = new Set(
   '|bash|zsh|sh|fish|cmd|cmd.exe|powershell|powershell.exe|pwsh|pwsh.exe|nu'.split('|')
 )
+const WINDOWS_PROCESS_EXTENSION_RE = /\.(?:exe|cmd|bat|ps1)$/i
 
 export function isShellProcess(processName: string): boolean {
   const normalized = processName
     .trim()
     .replace(/^["']|["']$/g, '')
     .toLowerCase()
+  const basename = normalized.split(/[\\/]/).pop() ?? normalized
+  // Why: Windows node-pty reports Git Bash and similar shells as `bash.exe`;
+  // those still need the same shell-safe handling as their POSIX basenames.
+  const basenameWithoutWindowsExtension = basename.replace(WINDOWS_PROCESS_EXTENSION_RE, '')
   return (
-    SHELL_NAMES.has(normalized) || SHELL_NAMES.has(normalized.split(/[\\/]/).pop() ?? normalized)
+    SHELL_NAMES.has(normalized) ||
+    SHELL_NAMES.has(basename) ||
+    SHELL_NAMES.has(basenameWithoutWindowsExtension)
   )
 }

src/shared/types.ts

@@ -494,6 +494,8 @@ export type GitPushTarget = {
 
 export type GitHubPrStartPoint = {
   baseBranch: string
+  /** Review target branch to use for Source Control compare after creating from a PR head SHA. */
+  compareBaseRef?: string
   pushTarget?: GitPushTarget
   /** Verified PR head commit. Present when checkout can be tied to a stable SHA. */
   headSha?: string
@@ -1054,6 +1056,8 @@ export type PRInfo = {
   // Keeping the head SHA in cached PR metadata lets the checks panel poll the
   // correct commit without re-querying GitHub or guessing from local branch refs.
   headSha?: string
+  /** Target branch name for PR-created worktree compare-base repair. */
+  baseRefName?: string
   prRepo?: GitHubRepositoryIdentity
   headRepo?: GitHubRepositoryIdentity
   conflictSummary?: PRConflictSummary
@@ -1894,6 +1898,8 @@ export type CreateWorktreeArgs = {
    *  Linear artifact whose title should remain readable in the sidebar. */
   displayName?: string
   baseBranch?: string
+  /** Source Control compare target when it differs from the checkout start point. */
+  compareBaseRef?: string
   /** Optional git branch to create, separate from the filesystem-safe worktree
    *  name. Used when creating from an existing branch whose local branch name
    *  legitimately contains `/` while the worktree directory must not. */