fix(engine): tighten Layer 3 — pass safety constraint into AI prompt + harden Layer 2 restore
Self-review of the recovery cascade surfaced three issues; this commit addresses all of them. 1. AI didn't actually receive the safety constraint under Layer 3. The previous commit logged the safety preamble to the task log via `store.logEntry`, but the merge agent doesn't read task log entries as prompt context — so the AI was running blind. The "no silent re-introduction of main's deletions" guarantee was therefore relying *entirely* on the deterministic verification gate (test + build), which is correct as a backstop but doesn't help the AI produce a correct first attempt. Fixed by threading `preMergeRebaseFallthrough` through `MergeAttemptParams` → `executeMergeAttempt` → `runAiAgentForCommit` → `MergePromptParams` → `buildMergePrompt`, where it now injects an explicit "⚠️ Pre-merge rebase recovery exhausted" preamble at the top of the user prompt with three concrete rules: - Prefer main's deletion when branch re-adds removed lines - Prefer main's version on ambiguous hunks - Call `fn_report_build_failure` rather than commit a regression Also includes the original rebase failure message (truncated) so the AI has diagnostic context. The truncated-context retry path also forwards the preamble — it's the safety constraint, not bulk context, so we keep it even when stripping diff stat / commit log to fit the window. 2. Layer 2's branch-restore could fail with "uncommitted changes". When a cherry-pick midway through Layer 2's replay fails, the worktree is in a half-applied state with conflicts in the index. The previous restore did `git checkout <branch>` (no -f) followed by `git reset --hard <originalSha>`. The plain checkout would refuse with "would overwrite local changes" if there were unmerged paths, preventing the reset from running and leaving the branch at the half-replayed tip. Fixed by reordering: hard-reset to the captured original SHA first (this clears index/working tree of any cherry-pick state), then `git checkout -f <branch>` to ensure HEAD points at the named branch, then a final hard-reset to the original SHA as belt-and-suspenders. Worst case the worktree is at the original branch tip — never worse than where Layer 2 started. 3. Pre-existing unrelated lint error blocking workspace lint. `packages/dashboard/src/server.ts` had an unused `resolve` import from `node:path` left behind by a recent refactor that extracted `PACKAGE_VERSION` into its own file. The user explicitly asked to clean it up so workspace lint passes. One-line drop. Tests + checks: - Engine: 2886 / 2886 pass (added safety preamble didn't break any existing prompt-content assertions) - Core: 3120 / 3120 pass - Workspace lint: clean - Engine typecheck: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import express, { type Router } from "express";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { join, dirname, resolve } from "node:path";
|
||||
import { join, dirname } from "node:path";
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { createSecureServer as createHttp2SecureServer, type Http2SecureServer } from "node:http2";
|
||||
|
||||
@@ -2831,7 +2831,13 @@ export async function aiMergeTask(
|
||||
}
|
||||
const restoreOriginalBranch = async () => {
|
||||
if (!originalBranchSha) return;
|
||||
await execAsync(`git checkout "${branch}"`, { cwd: worktreePath }).catch(
|
||||
// Hard-reset clears any in-progress cherry-pick / merge state
|
||||
// and resets the index, so the subsequent forced checkout has
|
||||
// no conflicting unmerged paths to refuse on.
|
||||
await execAsync(`git reset --hard "${originalBranchSha}"`, {
|
||||
cwd: worktreePath,
|
||||
}).catch(() => undefined);
|
||||
await execAsync(`git checkout -f "${branch}"`, { cwd: worktreePath }).catch(
|
||||
() => undefined,
|
||||
);
|
||||
await execAsync(`git reset --hard "${originalBranchSha}"`, {
|
||||
@@ -3072,6 +3078,7 @@ export async function aiMergeTask(
|
||||
buildCommand: effectiveBuildCommand,
|
||||
testSource: effectiveTestSource,
|
||||
buildSource: effectiveBuildSource,
|
||||
preMergeRebaseFallthrough,
|
||||
}, aiTracker);
|
||||
|
||||
if (success) {
|
||||
@@ -3826,6 +3833,12 @@ interface MergeAttemptParams {
|
||||
testSource?: "explicit" | "inferred";
|
||||
/** Source of the build command: 'explicit' from settings or 'inferred' (future use) */
|
||||
buildSource?: "explicit" | "inferred";
|
||||
/** Set when the pre-merge rebase recovery cascade (Layers 1–2) failed and
|
||||
* the merge proceeds under smart-prefer-main fall-through. The AI prompt
|
||||
* uses this to inject the safety preamble; the merge cascade uses it to
|
||||
* suppress the unsafe `-X ours` Attempt 3. Carries the original rebase
|
||||
* failure message for diagnostic context. */
|
||||
preMergeRebaseFallthrough?: string;
|
||||
}
|
||||
|
||||
/** Mutable flags carried through the merge cascade. */
|
||||
@@ -4104,6 +4117,7 @@ async function executeMergeAttempt(
|
||||
testCommand,
|
||||
buildCommand,
|
||||
sourceIssueRef,
|
||||
preMergeRebaseFallthrough: params.preMergeRebaseFallthrough,
|
||||
});
|
||||
|
||||
// Handle build failure
|
||||
@@ -4302,6 +4316,10 @@ interface AiAgentParams {
|
||||
options: MergerOptions;
|
||||
testCommand?: string;
|
||||
buildCommand?: string;
|
||||
/** Forwarded from MergeAttemptParams; injects the safety preamble into
|
||||
* the merge prompt when the pre-merge rebase recovery cascade fell
|
||||
* through. See MergePromptParams.preMergeRebaseFallthrough for details. */
|
||||
preMergeRebaseFallthrough?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -4340,6 +4358,7 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
options,
|
||||
testCommand,
|
||||
buildCommand,
|
||||
preMergeRebaseFallthrough,
|
||||
} = params;
|
||||
|
||||
const settings = await store.getSettings();
|
||||
@@ -4468,6 +4487,7 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
buildCommand,
|
||||
authorArg,
|
||||
sourceIssueRef,
|
||||
preMergeRebaseFallthrough,
|
||||
});
|
||||
|
||||
// Attempt prompting with fresh session (first attempt).
|
||||
@@ -4495,7 +4515,10 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
mergerLog.warn(`${taskId}: context limit hit after auto-compaction — retrying with minimal merge prompt`);
|
||||
await store.logEntry(taskId, "Context limit reached during merge after auto-compaction — retrying with reduced prompt");
|
||||
|
||||
// Build minimal prompt: omit diff stat, use placeholder for commit log
|
||||
// Build minimal prompt: omit diff stat, use placeholder for commit log.
|
||||
// The fall-through preamble is preserved (it's the safety constraint,
|
||||
// not bulk context) so the AI's truncated retry still knows main's
|
||||
// deletions are authoritative.
|
||||
const truncatedPrompt = buildMergePrompt({
|
||||
taskId,
|
||||
branch,
|
||||
@@ -4507,6 +4530,7 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
buildCommand,
|
||||
authorArg,
|
||||
sourceIssueRef,
|
||||
preMergeRebaseFallthrough,
|
||||
});
|
||||
|
||||
try {
|
||||
@@ -4605,23 +4629,62 @@ interface MergePromptParams {
|
||||
testCommand?: string;
|
||||
buildCommand?: string;
|
||||
authorArg?: string;
|
||||
/** When set, the pre-merge rebase aborted under smart-prefer-main and the
|
||||
* surgical/patch-id recovery layers couldn't unblock it. The prompt
|
||||
* injects an explicit safety preamble so the AI knows main's deletions
|
||||
* are authoritative and to prefer main on ambiguous hunks. The
|
||||
* deterministic post-merge verification (test + build) is the safety
|
||||
* gate; this preamble gives the AI a fighting chance to do the right
|
||||
* thing on its first try. */
|
||||
preMergeRebaseFallthrough?: string;
|
||||
}
|
||||
|
||||
export function buildMergePrompt(params: MergePromptParams): string {
|
||||
const { taskId, branch, commitLog, diffStat, hasConflicts, simplifiedContext, sourceIssueRef, testCommand, buildCommand, authorArg } = params;
|
||||
const { taskId, branch, commitLog, diffStat, hasConflicts, simplifiedContext, sourceIssueRef, testCommand, buildCommand, authorArg, preMergeRebaseFallthrough } = params;
|
||||
|
||||
// Apply truncation to prevent context overflow for large branches/diffs
|
||||
const truncatedCommitLog = truncateWithEllipsis(commitLog, MERGE_COMMIT_LOG_MAX_CHARS);
|
||||
const truncatedDiffStat = truncateWithEllipsis(diffStat, MERGE_DIFF_STAT_MAX_CHARS);
|
||||
|
||||
const parts = [
|
||||
const parts: string[] = [];
|
||||
|
||||
// When pre-merge rebase recovery layers (1+2) couldn't reconcile this
|
||||
// branch with main, this AI invocation is the final automated arbiter.
|
||||
// Give it the context and the safety constraint up front — verification
|
||||
// (test + build) is what enforces the constraint, but the AI should still
|
||||
// know what's expected so its first attempt has a real chance.
|
||||
if (preMergeRebaseFallthrough) {
|
||||
parts.push(
|
||||
"## ⚠️ Pre-merge rebase recovery exhausted — you are the final arbiter",
|
||||
"",
|
||||
"The pre-merge rebase against main aborted, and the surgical (Layer 1) and",
|
||||
"patch-id (Layer 2) recovery layers could not reconcile the branch. You are",
|
||||
"running under `smart-prefer-main` strategy, which means:",
|
||||
"",
|
||||
"**SAFETY CONSTRAINT — main's deletions are authoritative.**",
|
||||
"- If a hunk shows main has deleted lines that the branch re-adds, prefer",
|
||||
" main's deletion. Branch-only re-additions are likely orphan content from",
|
||||
" a squash-merged dependency and must NOT be re-introduced.",
|
||||
"- If a hunk is genuinely ambiguous, prefer main's version.",
|
||||
"- The merge result MUST pass `pnpm test` and `pnpm build`. If you can't",
|
||||
" produce a result that does, call `fn_report_build_failure` with concrete",
|
||||
" output rather than committing a regression.",
|
||||
"",
|
||||
`Original rebase failure for context: ${preMergeRebaseFallthrough.slice(0, 800)}`,
|
||||
"",
|
||||
"---",
|
||||
"",
|
||||
);
|
||||
}
|
||||
|
||||
parts.push(
|
||||
`Finalize the merge of branch \`${branch}\` for task ${taskId}.`,
|
||||
"",
|
||||
"## Branch commits",
|
||||
"```",
|
||||
truncatedCommitLog,
|
||||
"```",
|
||||
];
|
||||
);
|
||||
|
||||
if (!simplifiedContext) {
|
||||
parts.push(
|
||||
|
||||
Reference in New Issue
Block a user