Fix execute-task loop detection: adaptive reconciliation instead of hard stop (#342)

* Initial plan

* Add adaptive reconciliation, concrete remediation steps, and xxHash32 JS fallback

- auto.ts: final reconciliation pass before halting on loop detection
  (runs skipExecuteTask to write blocker artifacts and advance pipeline)
- auto.ts: adaptive retry on prevCount>=2: write stub summary when still
  missing after two agent sessions, so the agent has a recovery context
- auto.ts: buildLoopRemediationSteps() helper with concrete manual steps
  when automatic reconciliation cannot resolve the unit
- xxhash/index.ts: pure-JS xxHash32 fallback when native addon does not
  export xxHash32 (prevents 'native.xxHash32 is not a function' crash)
- idle-recovery.test.ts: tests for buildLoopRemediationSteps and loop-recovery
  path in skipExecuteTask

Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com>

* Address code review feedback

- xxhash/index.ts: extract accumulate() helper to remove duplicate lane
  processing lines in the 16-byte loop
- auto.ts: replace magic number 2 with named constant STUB_RECOVERY_THRESHOLD
- idle-recovery.test.ts: add comment explaining the 3 (== MAX_UNIT_DISPATCHES)
  used in the loop-recovery test

Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com>

* Address PR review: verify post-state, fix stub wording, fix checkbox syntax, add fallback test

- auto.ts (MAX branch): check verifyExpectedArtifact() after skipExecuteTask()
  before advancing; annotate both conditions explaining what each validates
- auto.ts (self-repair branch): guard advance with verifyExpectedArtifact() so
  regex mismatches in the slice plan don't silently re-dispatch
- auto.ts (stub-recovery): write stub summary directly with "PARTIAL RECOVERY"
  wording instead of using skipExecuteTask (which would claim "exhausted N attempts")
- auto.ts (remediation): fix checkbox example to "- [x] **${tid}:" (no trailing **)
- xxhash/index.ts: export xxHash32Fallback so CI can test JS path independently
- native/src/index.ts: re-export xxHash32Fallback
- xxhash.test.mjs: add "xxHash32Fallback (pure-JS path)" describe block that
  validates the JS implementation against the reference on all test cases

Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com>

* chore: retrigger CI

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com>
Co-authored-by: CI Trigger <noreply@github.com>
This commit is contained in:
Copilot 2026-03-14 07:26:10 -06:00 committed by GitHub
parent 9317816aa2
commit ce7bde14f3
5 changed files with 333 additions and 10 deletions

View file

@ -1,6 +1,6 @@
import { describe, it } from "node:test";
import assert from "node:assert/strict";
import { xxHash32 } from "@gsd/native/xxhash";
import { xxHash32, xxHash32Fallback } from "@gsd/native/xxhash";
/**
* Reference values computed from the pure-JS xxHash32 implementation
@ -84,3 +84,39 @@ describe("xxHash32 native vs JS compatibility", () => {
});
}
});
describe("xxHash32Fallback (pure-JS path)", () => {
// These tests exercise the JS fallback directly, validating the path that
// runs when the native addon loads but does not export xxHash32.
const testCases = [
["empty string, seed 0", "", 0],
["short string, seed 0", "hello", 0],
["short string, seed 42", "hello", 42],
["medium string, seed 0", "hello world!", 0],
["long string (>16 bytes)", "abcdefghijklmnopqrstuvwxyz", 0],
["whitespace only", " ", 0],
["punctuation", "{}();", 0],
["unicode", "\u{4e16}\u{754c}\u{1f600}", 0],
["empty with nonzero seed", "", 7],
["typical code line", " const x = 42;", 0],
["typical code line with seed", " const x = 42;", 3],
];
for (const [label, input, seed] of testCases) {
it(`JS fallback matches JS reference: ${label}`, () => {
const expected = jsXxHash32(input, seed);
const actual = xxHash32Fallback(input, seed);
assert.equal(
actual,
expected,
`Fallback mismatch for "${input}" seed=${seed}: fallback=${actual.toString(16)} reference=${expected.toString(16)}`
);
});
}
it("JS fallback produces same result as xxHash32", () => {
const input = " const x = hashline;";
const seed = 0;
assert.equal(xxHash32Fallback(input, seed), xxHash32(input, seed));
});
});

View file

@ -91,7 +91,7 @@ export type {
export { parseImage, ImageFormat, SamplingFilter } from "./image/index.js";
export type { NativeImageHandle } from "./image/index.js";
export { xxHash32 } from "./xxhash/index.js";
export { xxHash32, xxHash32Fallback } from "./xxhash/index.js";
export { ttsrCompileRules, ttsrCheckBuffer, ttsrFreeRules } from "./ttsr/index.js";
export type { TtsrHandle, TtsrRuleInput } from "./ttsr/index.js";

View file

@ -1,18 +1,98 @@
/**
* Native xxHash32 Rust implementation via napi-rs.
* Native xxHash32 Rust implementation via napi-rs, with a pure-JS fallback.
*
* Hashes the UTF-8 representation of the input string with the given seed.
*/
import { native } from "../native.js";
// ─── Pure-JS xxHash32 fallback ────────────────────────────────────────────────
// Used when the native addon is present but does not export xxHash32
// (e.g. an older build or an unsupported platform variant).
const PRIME32_1 = 0x9e3779b1;
const PRIME32_2 = 0x85ebca77;
const PRIME32_3 = 0xc2b2ae3d;
const PRIME32_4 = 0x27d4eb2f;
const PRIME32_5 = 0x165667b1;
function rotl32(v: number, n: number): number {
return ((v << n) | (v >>> (32 - n))) >>> 0;
}
/** Single xxHash32 lane accumulation step (processes one 32-bit word). */
function accumulate(v: number, lane: number): number {
return Math.imul(rotl32((v + Math.imul(lane, PRIME32_2)) >>> 0, 13), PRIME32_1) >>> 0;
}
function xxHash32JS(input: string, seed: number): number {
const data = Buffer.from(input, "utf-8");
const len = data.length;
let h32: number;
let i = 0;
if (len >= 16) {
let v1 = (seed + PRIME32_1 + PRIME32_2) >>> 0;
let v2 = (seed + PRIME32_2) >>> 0;
let v3 = seed >>> 0;
let v4 = (seed - PRIME32_1) >>> 0;
while (i <= len - 16) {
v1 = accumulate(v1, data.readUInt32LE(i)); i += 4;
v2 = accumulate(v2, data.readUInt32LE(i)); i += 4;
v3 = accumulate(v3, data.readUInt32LE(i)); i += 4;
v4 = accumulate(v4, data.readUInt32LE(i)); i += 4;
}
h32 = (rotl32(v1, 1) + rotl32(v2, 7) + rotl32(v3, 12) + rotl32(v4, 18)) >>> 0;
} else {
h32 = (seed + PRIME32_5) >>> 0;
}
h32 = (h32 + len) >>> 0;
while (i <= len - 4) {
h32 = Math.imul(rotl32((h32 + Math.imul(data.readUInt32LE(i), PRIME32_3)) >>> 0, 17), PRIME32_4) >>> 0;
i += 4;
}
while (i < len) {
h32 = Math.imul(rotl32((h32 + Math.imul(data[i]!, PRIME32_5)) >>> 0, 11), PRIME32_1) >>> 0;
i++;
}
h32 = Math.imul(h32 ^ (h32 >>> 15), PRIME32_2) >>> 0;
h32 = Math.imul(h32 ^ (h32 >>> 13), PRIME32_3) >>> 0;
h32 = (h32 ^ (h32 >>> 16)) >>> 0;
return h32;
}
/**
* Pure-JS xxHash32 implementation. Exposed for testing to allow CI to validate
* the fallback path independently of native addon availability.
*/
export function xxHash32Fallback(input: string, seed: number): number {
return xxHash32JS(input, seed);
}
// Resolve once at module load: prefer native, fall back to JS.
const _xxHash32Impl: (input: string, seed: number) => number =
typeof native.xxHash32 === "function"
? (input, seed) => native.xxHash32(input, seed)
: xxHash32JS;
/**
* Compute xxHash32 of a UTF-8 string.
*
* Uses the native Rust implementation when available; falls back to a
* pure-JS implementation if the loaded native addon does not export
* `xxHash32` (e.g. an older build).
*
* @param input The string to hash (encoded as UTF-8 internally).
* @param seed 32-bit seed value.
* @returns 32-bit unsigned hash.
*/
export function xxHash32(input: string, seed: number): number {
return native.xxHash32(input, seed);
return _xxHash32Impl(input, seed);
}

View file

@ -135,6 +135,8 @@ let gitService: GitServiceImpl | null = null;
/** Track total dispatches per unit to detect stuck loops (catches A→B→A→B patterns) */
const unitDispatchCount = new Map<string, number>();
const MAX_UNIT_DISPATCHES = 3;
/** Retry index at which a stub summary placeholder is written when the summary is still absent. */
const STUB_RECOVERY_THRESHOLD = 2;
/** Tracks recovery attempt count per unit for backoff and diagnostics. */
const unitRecoveryCount = new Map<string, number>();
@ -1498,24 +1500,54 @@ async function dispatchNextUnit(
}
saveActivityLog(ctx, basePath, unitType, unitId);
// Final reconciliation pass for execute-task: write any missing durable
// artifacts (summary placeholder + [x] checkbox) so the pipeline can
// advance instead of stopping. This is the last resort before halting.
if (unitType === "execute-task") {
const [mid, sid, tid] = unitId.split("/");
if (mid && sid && tid) {
const status = await inspectExecuteTaskDurability(basePath, unitId);
if (status) {
const reconciled = skipExecuteTask(basePath, mid, sid, tid, status, "loop-recovery", prevCount);
// reconciled: skipExecuteTask attempted to write missing artifacts.
// verifyExpectedArtifact: confirms physical artifacts (summary + [x]) now exist on disk.
// Both must pass before we clear the dispatch counter and advance.
if (reconciled && verifyExpectedArtifact(unitType, unitId, basePath)) {
ctx.ui.notify(
`Loop recovery: ${unitId} reconciled after ${prevCount + 1} dispatches — blocker artifacts written, pipeline advancing.\n Review ${status.summaryPath} and replace the placeholder with real work.`,
"warning",
);
unitDispatchCount.delete(dispatchKey);
await new Promise(r => setImmediate(r));
await dispatchNextUnit(ctx, pi);
return;
}
}
}
}
const expected = diagnoseExpectedArtifact(unitType, unitId, basePath);
const remediation = buildLoopRemediationSteps(unitType, unitId, basePath);
await stopAuto(ctx, pi);
ctx.ui.notify(
`Loop detected: ${unitType} ${unitId} dispatched ${prevCount + 1} times total. Expected artifact not found.${expected ? `\n Expected: ${expected}` : ""}\n Check branch state and .gsd/ artifacts.`,
`Loop detected: ${unitType} ${unitId} dispatched ${prevCount + 1} times total. Expected artifact not found.${expected ? `\n Expected: ${expected}` : ""}${remediation ? `\n\n Remediation steps:\n${remediation}` : "\n Check branch state and .gsd/ artifacts."}`,
"error",
);
return;
}
unitDispatchCount.set(dispatchKey, prevCount + 1);
if (prevCount > 0) {
// Self-repair: if summary exists but checkbox not marked, fix it and re-derive
// Adaptive self-repair: each retry attempts a different remediation step.
if (unitType === "execute-task") {
const status = await inspectExecuteTaskDurability(basePath, unitId);
if (status?.summaryExists && !status.taskChecked) {
const [mid, sid, tid] = unitId.split("/");
if (mid && sid && tid) {
const [mid, sid, tid] = unitId.split("/");
if (status && mid && sid && tid) {
if (status.summaryExists && !status.taskChecked) {
// Retry 1+: summary exists but checkbox not marked — mark [x] and advance.
const repaired = skipExecuteTask(basePath, mid, sid, tid, status, "self-repair", 0);
if (repaired) {
// repaired: skipExecuteTask updated metadata (returned early-true even if regex missed).
// verifyExpectedArtifact: confirms the physical artifact (summary + [x]) now exists.
if (repaired && verifyExpectedArtifact(unitType, unitId, basePath)) {
ctx.ui.notify(
`Self-repaired ${unitId}: summary existed but checkbox was unmarked. Marked [x] and advancing.`,
"warning",
@ -1525,6 +1557,32 @@ async function dispatchNextUnit(
await dispatchNextUnit(ctx, pi);
return;
}
} else if (prevCount >= STUB_RECOVERY_THRESHOLD && !status.summaryExists) {
// Retry STUB_RECOVERY_THRESHOLD+: summary still missing after multiple attempts.
// Write a minimal stub summary so the next agent session has a recovery artifact
// to overwrite, rather than starting from scratch again.
const tasksDir = resolveTasksDir(basePath, mid, sid);
const sDir = resolveSlicePath(basePath, mid, sid);
const targetDir = tasksDir ?? (sDir ? join(sDir, "tasks") : null);
if (targetDir) {
if (!existsSync(targetDir)) mkdirSync(targetDir, { recursive: true });
const summaryPath = join(targetDir, buildTaskFileName(tid, "SUMMARY"));
if (!existsSync(summaryPath)) {
const stubContent = [
`# PARTIAL RECOVERY — attempt ${prevCount + 1} of ${MAX_UNIT_DISPATCHES}`,
``,
`Task \`${tid}\` in slice \`${sid}\` (milestone \`${mid}\`) has not yet produced a real summary.`,
`This placeholder was written by auto-mode after ${prevCount} dispatch attempts.`,
``,
`The next agent session will retry this task. Replace this file with real work when done.`,
].join("\n");
writeFileSync(summaryPath, stubContent, "utf-8");
ctx.ui.notify(
`Stub recovery (attempt ${prevCount + 1}/${MAX_UNIT_DISPATCHES}): ${unitId} stub summary placeholder written. Retrying with recovery context.`,
"warning",
);
}
}
}
}
}
@ -3178,3 +3236,51 @@ function diagnoseExpectedArtifact(unitType: string, unitId: string, base: string
return null;
}
}
/**
* Build concrete, manual remediation steps for a loop-detected unit failure.
* These are shown when automatic reconciliation is not possible.
*/
export function buildLoopRemediationSteps(unitType: string, unitId: string, base: string): string | null {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const tid = parts[2];
switch (unitType) {
case "execute-task": {
if (!mid || !sid || !tid) break;
const planRel = relSliceFile(base, mid, sid, "PLAN");
const summaryRel = relTaskFile(base, mid, sid, tid, "SUMMARY");
return [
` 1. Write ${summaryRel} (even a partial summary is sufficient to unblock the pipeline)`,
` 2. Mark ${tid} [x] in ${planRel}: change "- [ ] **${tid}:" → "- [x] **${tid}:"`,
` 3. Run \`gsd doctor\` to reconcile .gsd/ state`,
` 4. Resume auto-mode — it will pick up from the next task`,
].join("\n");
}
case "plan-slice":
case "research-slice": {
if (!mid || !sid) break;
const artifactRel = unitType === "plan-slice"
? relSliceFile(base, mid, sid, "PLAN")
: relSliceFile(base, mid, sid, "RESEARCH");
return [
` 1. Write ${artifactRel} manually (or with the LLM in interactive mode)`,
` 2. Run \`gsd doctor\` to reconcile .gsd/ state`,
` 3. Resume auto-mode`,
].join("\n");
}
case "complete-slice": {
if (!mid || !sid) break;
return [
` 1. Write the slice summary and UAT file for ${sid} in ${relSlicePath(base, mid, sid)}`,
` 2. Mark ${sid} [x] in ${relMilestoneFile(base, mid, "ROADMAP")}`,
` 3. Run \`gsd doctor\` to reconcile .gsd/ state`,
` 4. Resume auto-mode`,
].join("\n");
}
default:
break;
}
return null;
}

View file

@ -7,6 +7,7 @@ import {
writeBlockerPlaceholder,
skipExecuteTask,
verifyExpectedArtifact,
buildLoopRemediationSteps,
} from "../auto.ts";
let passed = 0;
@ -490,6 +491,106 @@ const ROADMAP_COMPLETE = `# M001: Test Milestone
}
}
// ═══ buildLoopRemediationSteps ═══════════════════════════════════════════════
{
console.log("\n=== buildLoopRemediationSteps: execute-task returns concrete steps ===");
const base = mkdtempSync(join(tmpdir(), "gsd-loop-remediation-test-"));
try {
mkdirSync(join(base, ".gsd", "milestones", "M002", "slices", "S03", "tasks"), { recursive: true });
const result = buildLoopRemediationSteps("execute-task", "M002/S03/T01", base);
assert(result !== null, "should return remediation steps");
assert(result!.includes("T01-SUMMARY.md"), "steps mention the summary file");
assert(result!.includes("S03-PLAN.md"), "steps mention the slice plan");
assert(result!.includes("T01"), "steps mention the task ID");
assert(result!.includes("gsd doctor"), "steps include gsd doctor command");
// Exact slice plan checkbox syntax (no trailing **)
assert(result!.includes('"- [x] **T01:"'), "steps show exact checkbox syntax without trailing **");
} finally {
rmSync(base, { recursive: true, force: true });
}
}
{
console.log("\n=== buildLoopRemediationSteps: plan-slice returns concrete steps ===");
const base = mkdtempSync(join(tmpdir(), "gsd-loop-remediation-test-"));
try {
mkdirSync(join(base, ".gsd", "milestones", "M001", "slices", "S01"), { recursive: true });
const result = buildLoopRemediationSteps("plan-slice", "M001/S01", base);
assert(result !== null, "should return remediation steps for plan-slice");
assert(result!.includes("S01-PLAN.md"), "steps mention the slice plan file");
assert(result!.includes("gsd doctor"), "steps include gsd doctor command");
} finally {
rmSync(base, { recursive: true, force: true });
}
}
{
console.log("\n=== buildLoopRemediationSteps: research-slice returns concrete steps ===");
const base = mkdtempSync(join(tmpdir(), "gsd-loop-remediation-test-"));
try {
mkdirSync(join(base, ".gsd", "milestones", "M001", "slices", "S01"), { recursive: true });
const result = buildLoopRemediationSteps("research-slice", "M001/S01", base);
assert(result !== null, "should return remediation steps for research-slice");
assert(result!.includes("S01-RESEARCH.md"), "steps mention the slice research file");
assert(result!.includes("gsd doctor"), "steps include gsd doctor command");
} finally {
rmSync(base, { recursive: true, force: true });
}
}
{
console.log("\n=== buildLoopRemediationSteps: unknown type returns null ===");
const base = mkdtempSync(join(tmpdir(), "gsd-loop-remediation-test-"));
try {
const result = buildLoopRemediationSteps("unknown-type", "M001/S01", base);
assertEq(result, null, "unknown type returns null");
} finally {
rmSync(base, { recursive: true, force: true });
}
}
{
console.log("\n=== skipExecuteTask: loop-recovery writes blocker when both summary and checkbox missing ===");
const base = mkdtempSync(join(tmpdir(), "gsd-loop-recovery-test-"));
try {
mkdirSync(join(base, ".gsd", "milestones", "M002", "slices", "S03", "tasks"), { recursive: true });
const planPath = join(base, ".gsd", "milestones", "M002", "slices", "S03", "S03-PLAN.md");
writeFileSync(planPath, [
"# S03: Harden guided session",
"",
"## Tasks",
"",
"- [ ] **T01: Harden contract usage** `est:30m`",
" Harden guided session contract usage in desktop flow.",
].join("\n"), "utf-8");
const result = skipExecuteTask(
base, "M002", "S03", "T01",
{ summaryExists: false, taskChecked: false },
"loop-recovery",
// 3 == MAX_UNIT_DISPATCHES: represents the prevCount when the final
// reconciliation path runs (loop detected, reconciling before halting).
3,
);
assert(result === true, "loop-recovery should succeed");
// Blocker summary written
const summaryPath = join(base, ".gsd", "milestones", "M002", "slices", "S03", "tasks", "T01-SUMMARY.md");
assert(existsSync(summaryPath), "blocker summary should be written");
const summaryContent = readFileSync(summaryPath, "utf-8");
assert(summaryContent.includes("BLOCKER"), "summary should be a blocker placeholder");
assert(summaryContent.includes("loop-recovery"), "summary should mention the recovery reason");
// Checkbox marked
const planContent = readFileSync(planPath, "utf-8");
assert(planContent.includes("- [x] **T01:"), "T01 checkbox should be marked [x] after loop-recovery");
} finally {
rmSync(base, { recursive: true, force: true });
}
}
// ═════════════════════════════════════════════════════════════════════════════
// Results
// ═════════════════════════════════════════════════════════════════════════════