refactor(sandcastle): improve separation of concerns and API clarity

- Extract runValidation into validation.ts
- Encapsulate nonce in loop (remove from strategy interface)
- Delete dead GRACE_TIMEOUT_MS constant
- Reorganize constants into proper domain groups
- Un-export FindingSchema and extractStderr

diff --git a/.sandcastle/constants.ts b/.sandcastle/constants.ts
index 1ebfee6d3ce6a396629903f296cdf95e96bd5f56..3e0a0a78693061d7fd0c6d1ccbb764cbe102229c 100644 (file)
--- a/.sandcastle/constants.ts
+++ b/.sandcastle/constants.ts
@@ -17,6 +17,10 @@ export const AGENT_PLANNER_MODEL = 'github-copilot/claude-opus-4.6'
 
 export const AGENT_TASK_TIMEOUT_MS = 6_000_000
 
+export const COMPLETION_SIGNAL = '<promise>COMPLETE</promise>'
+
+export const MAX_PARALLEL = 5
+
 // ── Git ──────────────────────────────────────────────────────────────────────
 
 export const GIT_BASE_BRANCH = 'main'
@@ -65,25 +69,19 @@ export const GITHUB_MAX_ISSUES_FETCH = 50
 
 export const GITHUB_MAX_PRS_FETCH = 200
 
+export const MAX_TITLE_CHARS = 200
+
 // ── Validation ───────────────────────────────────────────────────────────────
 
+export const MAX_STDERR_CHARS = 500
+
 export const VALIDATION_COMMAND =
   'pnpm format && pnpm typecheck && pnpm lint && pnpm build && pnpm test'
 
 export const VALIDATION_TIMEOUT_MS = 300_000
 
-// ── Limits & Protocol ────────────────────────────────────────────────────────
-
-export const COMPLETION_SIGNAL = '<promise>COMPLETE</promise>'
+// ── Deduplication ────────────────────────────────────────────────────────────
 
 export const CONTEXT_HASH_RADIUS = 3
 
-export const GRACE_TIMEOUT_MS = 30_000
-
 export const HASH_PREFIX_LENGTH = 16
-
-export const MAX_PARALLEL = 5
-
-export const MAX_STDERR_CHARS = 500
-
-export const MAX_TITLE_CHARS = 200

diff --git a/.sandcastle/finalizer.ts b/.sandcastle/finalizer.ts
index 9d7465d1bcfa7e1cb34d395e4674fccc9fb654f3..665ae825696baa47972be7cf739c0ae1b88319f4 100644 (file)
--- a/.sandcastle/finalizer.ts
+++ b/.sandcastle/finalizer.ts
@@ -2,14 +2,7 @@ import crypto from 'node:crypto'
 
 import type { LoopResult, TaskSpec } from './types.js'
 
-import {
-  GIT_BASE_BRANCH,
-  GIT_PUSH_TIMEOUT_MS,
-  GIT_TIMEOUT_MS,
-  MAX_STDERR_CHARS,
-  VALIDATION_COMMAND,
-  VALIDATION_TIMEOUT_MS,
-} from './constants.js'
+import { GIT_BASE_BRANCH, GIT_PUSH_TIMEOUT_MS, GIT_TIMEOUT_MS } from './constants.js'
 import { execFileAsync, toErrorMessage } from './utils.js'
 
 /**
@@ -104,17 +97,6 @@ export function buildPrArgs (
   return { isDraft, prArgs }
 }
 
-/**
- * Extracts stderr from a caught error, truncated to 500 chars.
- * @param err - The caught error value.
- * @returns Stderr string or empty string if unavailable.
- */
-export function extractStderr (err: unknown): string {
-  return err instanceof Error && 'stderr' in err
-    ? String((err as { stderr: unknown }).stderr).slice(0, MAX_STDERR_CHARS)
-    : ''
-}
-
 /**
  * Pushes the branch to origin. When rebase succeeded, uses force-with-lease
  * with a rescue-branch fallback. When rebase was aborted, does a plain push.
@@ -171,29 +153,3 @@ export async function pushBranch (
     }
   }
 }
-
-/**
- * Runs the full validation suite.
- * @param cwd - Working directory (worktree path).
- * @param spec - Optional task specification (used for logging).
- * @returns `true` if validation passed, `false` otherwise.
- */
-export async function runValidation (cwd: string, spec?: TaskSpec): Promise<boolean> {
-  try {
-    await execFileAsync('sh', ['-c', VALIDATION_COMMAND], {
-      cwd,
-      maxBuffer: 8 * 1024 * 1024,
-      timeout: VALIDATION_TIMEOUT_MS,
-    })
-    return true
-  } catch (err: unknown) {
-    if (err && typeof err === 'object' && 'killed' in err && (err as { killed: boolean }).killed) {
-      const label = spec ? `#${spec.id}` : 'mid-loop'
-      console.warn(`  ${label}: Validation timed out after ${String(VALIDATION_TIMEOUT_MS)}ms.`)
-    } else if (spec) {
-      const stderr = extractStderr(err)
-      console.warn(`  #${spec.id}: Validation failed.${stderr ? `\n${stderr}` : ''}`)
-    }
-    return false
-  }
-}

diff --git a/.sandcastle/refinement-loop.ts b/.sandcastle/refinement-loop.ts
index a81f35b20a9b6b35ed65f514649e509e1ae6cb72..c11a864d3ebea88e5970193273a81ca5c410a684 100644 (file)
--- a/.sandcastle/refinement-loop.ts
+++ b/.sandcastle/refinement-loop.ts
@@ -24,9 +24,9 @@ import {
   GIT_BASE_BRANCH,
   HASH_PREFIX_LENGTH,
 } from './constants.js'
-import { runValidation } from './finalizer.js'
 import { parseFindingsSafe } from './types.js'
 import { execFileAsync } from './utils.js'
+import { runValidation } from './validation.js'
 
 /** Options for configuring the refinement loop. */
 export interface RefinementLoopOptions {
@@ -596,7 +596,7 @@ async function runCritic (
     idleTimeoutSeconds: AGENT_IDLE_TIMEOUT_S,
     maxIterations: 1,
     name: `Critic #${spec.id} R${String(round)}`,
-    promptArgs: strategy.buildCriticArgs(spec, nonce, baseBranch),
+    promptArgs: { ...strategy.buildCriticArgs(spec, baseBranch), NONCE: nonce },
     promptFile: strategy.criticPromptFile,
     signal,
   })
@@ -611,7 +611,7 @@ async function runCritic (
       idleTimeoutSeconds: AGENT_IDLE_TIMEOUT_S,
       maxIterations: 1,
       name: `Critic #${spec.id} R${String(round)} retry`,
-      promptArgs: strategy.buildCriticArgs(spec, nonce, baseBranch),
+      promptArgs: { ...strategy.buildCriticArgs(spec, baseBranch), NONCE: nonce },
       promptFile: strategy.criticPromptFile,
       signal,
     })

diff --git a/.sandcastle/strategies/implement/strategy.ts b/.sandcastle/strategies/implement/strategy.ts
index 0a3a9d138866d45b7635f288a424ad644ad2356c..94ac6d4af7efa41e832827a071d49f2d27e21134 100644 (file)
--- a/.sandcastle/strategies/implement/strategy.ts
+++ b/.sandcastle/strategies/implement/strategy.ts
@@ -1,8 +1,9 @@
 import type { FinalizationConfig, LoopStrategy } from '../../types.js'
 
 import { GIT_TIMEOUT_MS } from '../../constants.js'
-import { attemptRebase, buildPrArgs, pushBranch, runValidation } from '../../finalizer.js'
+import { attemptRebase, buildPrArgs, pushBranch } from '../../finalizer.js'
 import { execFileAsync, toErrorMessage } from '../../utils.js'
+import { runValidation } from '../../validation.js'
 
 export const implementStrategy: FinalizationConfig & LoopStrategy = {
   actorPromptFile: './.sandcastle/strategies/implement/implement-prompt.md',
@@ -15,10 +16,9 @@ export const implementStrategy: FinalizationConfig & LoopStrategy = {
     TASK_ID: spec.id,
   }),
 
-  buildCriticArgs: (spec, nonce, baseBranch) => ({
+  buildCriticArgs: (spec, baseBranch) => ({
     BASE_BRANCH: baseBranch,
     BRANCH: spec.branch,
-    NONCE: nonce,
   }),
 
   criticPromptFile: './.sandcastle/strategies/implement/critic-prompt.md',

diff --git a/.sandcastle/types.ts b/.sandcastle/types.ts
index c51deeb2c4764653fc3d24ec69e9dc6e50ffc4dc..c48b2aba3481298a2fed60b0bca595313f071463 100644 (file)
--- a/.sandcastle/types.ts
+++ b/.sandcastle/types.ts
@@ -3,7 +3,7 @@ import type * as sandcastle from '@ai-hero/sandcastle'
 import { z } from 'zod'
 
 /** Zod schema for a single critic finding. */
-export const FindingSchema = z.object({
+const FindingSchema = z.object({
   category: z.string(),
   confidence: z.enum(['HIGH', 'MEDIUM', 'LOW']),
   description: z.string(),
@@ -70,8 +70,8 @@ export type LoopStrategy = {
   actorPromptFile: string
   /** Builds promptArgs for the actor run from task spec and previous findings. */
   buildActorArgs: (spec: TaskSpec, findings: Finding[]) => Record<string, string>
-  /** Builds promptArgs for the critic run from task spec, nonce, and base branch. */
-  buildCriticArgs: (spec: TaskSpec, nonce: string, baseBranch: string) => Record<string, string>
+  /** Builds promptArgs for the critic run from task spec and base branch. */
+  buildCriticArgs: (spec: TaskSpec, baseBranch: string) => Record<string, string>
   /** Model for the critic agent. Defaults to AGENT_CRITIC_MODEL constant. */
   criticModel?: string
   /** Path to the critic prompt file. */

diff --git a/.sandcastle/validation.ts b/.sandcastle/validation.ts
new file mode 100644 (file)
index 0000000..ab49449
--- /dev/null
+++ b/.sandcastle/validation.ts
@@ -0,0 +1,41 @@
+import type { TaskSpec } from './types.js'
+
+import { MAX_STDERR_CHARS, VALIDATION_COMMAND, VALIDATION_TIMEOUT_MS } from './constants.js'
+import { execFileAsync } from './utils.js'
+
+/**
+ * Runs the full validation suite.
+ * @param cwd - Working directory (worktree path).
+ * @param spec - Optional task specification (used for logging).
+ * @returns `true` if validation passed, `false` otherwise.
+ */
+export async function runValidation (cwd: string, spec?: TaskSpec): Promise<boolean> {
+  try {
+    await execFileAsync('sh', ['-c', VALIDATION_COMMAND], {
+      cwd,
+      maxBuffer: 8 * 1024 * 1024,
+      timeout: VALIDATION_TIMEOUT_MS,
+    })
+    return true
+  } catch (err: unknown) {
+    if (err && typeof err === 'object' && 'killed' in err && (err as { killed: boolean }).killed) {
+      const label = spec ? `#${spec.id}` : 'mid-loop'
+      console.warn(`  ${label}: Validation timed out after ${String(VALIDATION_TIMEOUT_MS)}ms.`)
+    } else if (spec) {
+      const stderr = extractStderr(err)
+      console.warn(`  #${spec.id}: Validation failed.${stderr ? `\n${stderr}` : ''}`)
+    }
+    return false
+  }
+}
+
+/**
+ * Extracts stderr from a caught error, truncated to 500 chars.
+ * @param err - The caught error value.
+ * @returns Stderr string or empty string if unavailable.
+ */
+function extractStderr (err: unknown): string {
+  return err instanceof Error && 'stderr' in err
+    ? String((err as { stderr: unknown }).stderr).slice(0, MAX_STDERR_CHARS)
+    : ''
+}