Harden edit_transaction type repair, read-block diagnostics, and orchestrator spawn guidance

agents/__tests__/quality-prompt-snapshot.test.ts

@@ -13,6 +13,7 @@ import {
   gitDisciplineSection,
   qualitySection,
   securityReviewSection,
+  specialistRoutingSection,
 } from '../base2/quality-prompt-section'
 
 /**
@@ -88,6 +89,49 @@ describe('shared craftsmanship prompt sections', () => {
     expect(gitDisciplineSection).toContain('git_branch')
   })
 
+  test('gitDisciplineSection tells the orchestrator to pass owned_paths in git-committer params', () => {
+    // git-committer requires params.owned_paths; a prompt-only or empty-params
+    // spawn fails outright. Guard that the guidance names the required key so
+    // the orchestrator supplies it instead of relying on the prose prompt.
+    expect(gitDisciplineSection).toContain('owned_paths')
+    expect(gitDisciplineSection).toContain('in params')
+  })
+
+  test('gitDisciplineSection warns that an empty/prompt-only git-committer spawn fails the spawn', () => {
+    // Regression guard for the observed failure: spawning git-committer with
+    // empty params ({}) fails with "Missing required: owned_paths". The
+    // guidance must mark owned_paths REQUIRED and explain that omitting it via
+    // an empty or prompt-only spawn fails outright, so the orchestrator does
+    // not rely on the prose prompt alone and hit the same spawn rejection.
+    expect(gitDisciplineSection).toContain('REQUIRED')
+    expect(gitDisciplineSection).toContain('required field')
+    expect(gitDisciplineSection).toContain('empty or prompt-only spawn')
+    expect(gitDisciplineSection).toContain('fails the spawn')
+  })
+
+  test('gitDisciplineSection names the literal owned_paths key and the gate-block on early git-committer spawns', () => {
+    // Two observed failures this guidance targets: (1) passing a wrong key
+    // name (filePaths) instead of the literal owned_paths, and (2) attempting
+    // the git-committer spawn before the gate passed. Guard that the guidance
+    // names the exact key and the runtime gate-block message.
+    expect(gitDisciplineSection).toContain('literally `owned_paths`')
+    expect(gitDisciplineSection).toContain('filePaths')
+    expect(gitDisciplineSection).toContain('Missing required: owned_paths')
+    expect(gitDisciplineSection).toContain(
+      'Spawning git-committer is not available yet',
+    )
+  })
+
+  test('gateAwarenessSection contains the required gate-awareness topics (not byte-frozen)', () => {
+    // gateAwarenessSection is advisory guidance that may evolve; only assert
+    // topic coverage so future tightening does not silently drop the
+    // don't-double-spawn-code-reviewer guidance.
+    expect(gateAwarenessSection).toContain('# Automated Validation & Review Gate')
+    expect(gateAwarenessSection).toContain('code-reviewer')
+    expect(gateAwarenessSection).toContain('validation hooks')
+    expect(gateAwarenessSection).toContain('before finalization')
+  })
+
   test('securityReviewSection contains the required security-review topics (not byte-frozen)', () => {
     // securityReviewSection is advisory guidance that may evolve; only assert
     // topic coverage so future tightening does not silently drop a rule.
@@ -102,6 +146,17 @@ describe('shared craftsmanship prompt sections', () => {
     expect(securityReviewSection).toContain('read-only')
   })
 
+  test('specialistRoutingSection names the exact snapshot param contract for reviewer-family specialists', () => {
+    // Reviewer-family specialists require params.snapshot_id from
+    // get_change_review_bundle, while security-reviewer requires
+    // changed_files + snapshot_fingerprint. Guard that the guidance names both
+    // contracts so a spawn does not fail on the wrong/missing snapshot key.
+    expect(specialistRoutingSection).toContain('snapshot_id')
+    expect(specialistRoutingSection).toContain('get_change_review_bundle')
+    expect(specialistRoutingSection).toContain('changed_files')
+    expect(specialistRoutingSection).toContain('snapshot_fingerprint')
+  })
+
   test('all three consumers interpolate shared sections and leave conditional sections gated', () => {
     // Frontend and language guidance are runtime placeholders so unrelated
     // repos do not receive prompt pollution.
@@ -136,4 +191,29 @@ describe('shared craftsmanship prompt sections', () => {
     expect(editor.instructionsPrompt).toContain(PLACEHOLDER.LANGUAGE_PROFILE)
     expect(editor.instructionsPrompt).not.toContain(frontendSection)
   })
+
+  test('base2 system prompt routes ripgrep-style search through code-searcher', () => {
+    // The root orchestrator is not granted code_search/find_files_matching_content;
+    // its prompt must tell it to spawn code-searcher instead of calling them
+    // directly (otherwise the runtime rejects the call). Guard the semantic
+    // content without freezing the exact wording.
+    const base2 = createBase2('default')
+
+    expect(base2.systemPrompt).toContain('code-searcher')
+    expect(base2.systemPrompt).toContain('code_search')
+    expect(base2.systemPrompt).toContain('find_files_matching_content')
+    expect(base2.systemPrompt).toContain('not granted to you as root')
+  })
+
+  test('base2 system prompt names required spawn params for code-searcher and basher', () => {
+    // Regression guard for observed spawn failures: code-searcher requires
+    // params.searchQueries and basher requires params.command. The prompt
+    // must name both required keys so the orchestrator supplies them in
+    // params instead of relying on the prose prompt and hitting a spawn
+    // rejection.
+    const base2 = createBase2('default')
+
+    expect(base2.systemPrompt).toContain('params.searchQueries')
+    expect(base2.systemPrompt).toContain('params.command')
+  })
 })

agents/base2/base2.ts

@@ -208,7 +208,7 @@ ${
     ? '- **Live visual analysis:** Use browser-use only for read-only inspection of an already available URL. Do not start dev servers or request browser interactions in plan mode.'
     : '- **Live visual verification:** Visual verification extends beyond web apps. Image artifacts from 3D renders (e.g. Blender frames), image/video exports, generated diagrams, and charts must be inspected with read_image, not inferred from text logs alone. The workflow is: render/export -> poll the background job to completion -> read_image the emitted artifacts -> assess the result -> make a targeted edit -> re-render. Polling (check_job/check_background_agent/read_logs) is only the bridge to artifact inspection; do not re-poll a finished or unchanging job indefinitely. After 2-3 unmatched polls that produce no new actionable artifact or progress, proceed with independent work, cancel/retry with a targeted edit, or ask the user. For web app visual checks specifically, start any long-running dev server through a BACKGROUND basher, keep its returned jobId, use check_job to wait for readiness, then spawn browser-use for screenshots/navigation/interaction.'
 }
-- **Prefer dedicated harness tools over shell fallbacks:** Repository status is injected automatically by the runtime; do not spawn basher merely to run git status. Use read_files/read_outline/read_subtree/glob/list_directory/query_index for file and codebase inspection instead of shelling out to cat/ls/find/grep. Use basher for commands that do not have a dedicated tool, such as tests, builds, package scripts, and one-off project CLIs. Never embed a multi-KB file body or heredoc (\`<<'EOF' ... EOF\`) inside \`basher.params.command\`; the transport truncates large payloads and the JSON normalizer intentionally fails closed on truncated input. Author files with \`write_file\`/\`edit_transaction\` and run them via a short basher command instead.
+- **Prefer dedicated harness tools over shell fallbacks:** Repository status is injected automatically by the runtime; do not spawn basher merely to run git status. Use read_files/read_outline/read_subtree/glob/list_directory/query_index for file and codebase inspection instead of shelling out to cat/ls/find/grep. Use basher for commands that do not have a dedicated tool, such as tests, builds, package scripts, and one-off project CLIs. Never embed a multi-KB file body or heredoc (\`<<'EOF' ... EOF\`) inside \`basher.params.command\`; the transport truncates large payloads and the JSON normalizer intentionally fails closed on truncated input. Author files with \`write_file\`/\`edit_transaction\` and run them via a short basher command instead. For ripgrep-style content search, spawn the code-searcher agent (and file-picker for fuzzy file discovery): \`code_search\`/\`find_files_matching_content\` are registered runtime tools but are intentionally not granted to you as root, so calling them directly is rejected. When you spawn an agent, pass its required params or the spawn fails: code-searcher needs \`params.searchQueries\` (an array of { pattern } objects) and basher needs \`params.command\` (a shell string); put these in \`params\`, not only in the prose prompt.
 
 # Code Editing Mandates
 
@@ -704,9 +704,6 @@ ${specialistRoutingSection}
       const initialGitStatusDirtyFiles = extractGitStatusFiles(
         (initialGitStatus as any)?.toolResult,
       )
-      const initialGitStatusLineMap = extractGitStatusLineMap(
-        (initialGitStatus as any)?.toolResult,
-      )
       const changedFiles = new Set<string>(activeWorkState.changedFiles)
       const pendingGateFiles = new Set<string>(activeWorkState.pendingGateFiles)
       let editsHappened =
@@ -730,7 +727,6 @@ ${specialistRoutingSection}
         activeWorkState.gatePassedFingerprint &&
         !hasFreshGateFingerprintForPendingFiles(
           activeWorkState.gatePassedPendingFiles,
-          initialGitStatusLineMap,
           activeWorkState.gatePassedValidationSummary ||
             activeWorkState.lastValidationSummary ||
             'No configured file-change hooks ran.',
@@ -884,9 +880,6 @@ ${specialistRoutingSection}
         const gitStatusFiles = extractGitStatusFiles(
           (currentGitStatus as any)?.toolResult,
         )
-        const currentGitStatusLineMap = extractGitStatusLineMap(
-          (currentGitStatus as any)?.toolResult,
-        )
         // Prune pending gate files that were previously observed as dirty in
         // git status but are no longer present (i.e., they were committed).
         // Without this, committed files stay in the pending set forever,
@@ -1972,7 +1965,6 @@ ${specialistRoutingSection}
           conversationGatePass &&
           hasFreshGateFingerprintForPendingFiles(
             currentPendingGateFiles,
-            currentGitStatusLineMap,
             conversationValidationSummary,
           )
         ) {
@@ -2034,10 +2026,7 @@ ${specialistRoutingSection}
         if (
           runValidationGate &&
           editsHappened &&
-          hasDurableGatePassForPendingFiles(
-            currentPendingGateFiles,
-            currentGitStatusLineMap,
-          )
+          hasDurableGatePassForPendingFiles(currentPendingGateFiles)
         ) {
           const durableReviewerVerdict =
             reviewerFinalizationVerdictFromDurablePass()
@@ -2837,7 +2826,17 @@ ${specialistRoutingSection}
           reviewerFinalizationVerdict = 'NON_BLOCKING'
           activeWorkState.currentPhase = 'awaiting_review'
           activeWorkState.nextRequiredAction = ''
-          activeWorkState.staticReviewerJobId = undefined
+          if (activeWorkState.staticReviewerJobId) {
+            yield {
+              toolName: 'check_background_agent',
+              input: {
+                jobId: activeWorkState.staticReviewerJobId,
+                cancel: true,
+              },
+              includeToolCall: false,
+            } as any
+            activeWorkState.staticReviewerJobId = undefined
+          }
           const reviewerSkipReason =
             reviewablePendingFiles.length === 0
               ? 'reviewer skip: no reviewable source files'
@@ -4771,7 +4770,6 @@ ${specialistRoutingSection}
 
       function hasFreshGateFingerprintForPendingFiles(
         files: string[],
-        currentStatusLines: Map<string, string>,
         validationSummary: string,
       ): boolean {
         if (files.length === 0) return false
@@ -4791,14 +4789,10 @@ ${specialistRoutingSection}
         return recorded === currentFingerprint
       }
 
-      function hasDurableGatePassForPendingFiles(
-        files: string[],
-        currentStatusLines: Map<string, string>,
-      ): boolean {
+      function hasDurableGatePassForPendingFiles(files: string[]): boolean {
         if (!reviewerFinalizationVerdictFromDurablePass()) return false
         return hasFreshGateFingerprintForPendingFiles(
           files,
-          currentStatusLines,
           activeWorkState.gatePassedValidationSummary ||
             activeWorkState.lastValidationSummary ||
             'No configured file-change hooks ran.',
@@ -5314,41 +5308,6 @@ ${specialistRoutingSection}
         return normalizeGateFileList([...files])
       }
 
-      /**
-       * Extracts a map of normalized-file -> raw git status line for the given
-       * pending files from a git_status tool result. Lines for other files are
-       * ignored. Used to build a durable fingerprint for the gate pass so we
-       * only reuse a durable pass when the working tree state for those files
-       * still matches.
-       */
-      function extractGitStatusLineMap(
-        toolResult: unknown,
-      ): Map<string, string> {
-        const map = new Map<string, string>()
-        if (!Array.isArray(toolResult)) return map
-        for (const part of toolResult) {
-          const value =
-            part && (part as any).type === 'json'
-              ? (part as any).value
-              : undefined
-          const status =
-            value && typeof value === 'object'
-              ? (value as Record<string, unknown>).status
-              : undefined
-          if (typeof status !== 'string') continue
-          for (const rawLine of status.split('\n')) {
-            const file = parseGitStatusLine(rawLine)
-            if (!file) continue
-            const normalized = normalizeGateFilePath(file)
-            if (!normalized) continue
-            // Keep the raw line (without trailing whitespace) so XY status bits
-            // are part of the fingerprint.
-            map.set(normalized, rawLine.replace(/\s+$/, ''))
-          }
-        }
-        return map
-      }
-
       /**
        * Build a durable gate fingerprint from the normalized pending files,
        * their current git status lines (if known), per-file working-tree

agents/base2/quality-prompt-section.ts

@@ -137,7 +137,7 @@ Use specialists when repository evidence or the requested outcome crosses one of
 
 Gather the exact source and snapshot evidence before spawning. Advisory specialists inform the plan; reviewer specialists can block their scoped risk dimension. They complement rather than replace targeted validation and the final code-reviewer gate.
 
-Post-edit reviewer-family specialists are routed automatically by the orchestrator's gate. Do not manually re-spawn them after edits, after compaction, or merely because set_output is unavailable; wait for the runtime-owned gate result. Manual specialist calls are for pre-edit advisory work or an explicit user request.`
+Post-edit reviewer-family specialists are routed automatically by the orchestrator's gate. Do not manually re-spawn them after edits, after compaction, or merely because set_output is unavailable; wait for the runtime-owned gate result. Manual specialist calls are for pre-edit advisory work or an explicit user request. When you do spawn one, pass its exact params contract: reviewer-family specialists (product-reviewer, performance-specialist, reliability-reviewer, migration-reviewer, compatibility-reviewer, accessibility-reviewer, ux-visual-reviewer, dependency-reviewer, evaluator) require params.snapshot_id set to the exact fingerprint returned by get_change_review_bundle. security-reviewer is the exception: it requires params.changed_files plus params.snapshot_fingerprint and does not accept snapshot_id. Spawning with the wrong or missing snapshot key fails the spawn.`
 
 /**
  * Git-discipline section: orchestrator-level guidance for git workflows.
@@ -155,7 +155,8 @@ export const gitDisciplineSection = `# Git Discipline
 
 When the user asks to commit, stage, branch, or push changes, delegate the full git workflow to the \`git-committer\` agent rather than running raw \`git\` commands yourself. Pass exact task-owned paths whenever known. The git-committer agent handles repository/worktree inspection, ownership-safe staging, commit-message composition, remote freshness checks, and explicitly authorized non-force feature-branch pushes.
 
-- **Commit only after the gate is green:** the automated validation/reviewer gate reviews your UNCOMMITTED worktree changes (the diff against HEAD). Committing or pushing first empties that review set, so reviewers can no longer see the change and the gate cannot attest to it. Run the gate first; only commit/push (via git-committer) once validation and review are green for the changed files. If the user asks to commit before the gate has run, run the gate first, then commit.
+- **Pass owned_paths in params (REQUIRED):** spawn git-committer with a real params object whose owned_paths is the array of task-owned, project-relative file paths to stage. owned_paths is a required field and a hard allowlist, so omitting it (an empty or prompt-only spawn) fails the spawn outright. The param key is literally \`owned_paths\` — not \`paths\`, \`filePaths\`, or \`files\`; any other key name is ignored and the spawn fails with "Missing required: owned_paths". Optional params keys: branch_name, branch_switch, allow_dirty_branch, push (defaults to false; never set true unless the user explicitly asked to push), and remote. Put these in params, not only in the prose prompt.
+- **Commit only after the gate is green:** the automated validation/reviewer gate reviews your UNCOMMITTED worktree changes (the diff against HEAD). Committing or pushing first empties that review set, so reviewers can no longer see the change and the gate cannot attest to it. Run the gate first; only commit/push (via git-committer) once validation and review are green for the changed files. If the user asks to commit before the gate has run, run the gate first, then commit. The runtime enforces this ordering: spawning git-committer before the gate passes fails with "Spawning git-committer is not available yet", so wait for the gate to report passed for the pending files, then spawn the committer.
 - **Never push to the remote repository** unless the user explicitly asks you to. Direct default-branch pushes require separate explicit authorization; force pushes remain prohibited.
 - **Never alter git config** (no \`git config user.name/email\`, no \`--global\` flags).
 - **Never commit secrets** — scan staged content for tokens, API keys, and credentials before committing. The git-committer agent does this automatically.

common/src/tools/params/__tests__/coerce-to-array.test.ts

@@ -764,6 +764,36 @@ describe('normalizeTransactionEditList', () => {
     ).toEqual([explicit, contentOnly, conflicting])
   })
 
+  it('overrides an invalid-but-unambiguous edit type from shape', () => {
+    expect(
+      normalizeTransactionEditList([
+        {
+          path: 'a.ts',
+          type: 'edit',
+          replacements: [{ oldString: 'a', newString: 'b' }],
+        },
+      ]),
+    ).toMatchObject([{ type: 'str_replace' }])
+  })
+
+  it('canonicalizes case/separator variants of a valid edit type', () => {
+    expect(
+      normalizeTransactionEditList([
+        {
+          path: 'a.ts',
+          type: 'Str-Replace',
+          replacements: [{ oldString: 'a', newString: 'b' }],
+        },
+      ]),
+    ).toMatchObject([{ type: 'str_replace' }])
+  })
+
+  it('leaves an invalid type with an ambiguous shape unchanged', () => {
+    expect(
+      normalizeTransactionEditList([{ path: 'a.ts', type: 'bogus', content: 'x' }]),
+    ).toEqual([{ path: 'a.ts', type: 'bogus', content: 'x' }])
+  })
+
   it('repairs a JSON-stringified transaction array', () => {
     expect(
       normalizeTransactionEditList(