jb/harness-tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(pi-workflow): improve workflow usability and recovery

Browse Source
This commit is contained in:
jb
2026-08-26 00:20:24 -04:00
parent c291ca083c
commit ea74dbc6a7
23 changed files with 4422 additions and 913 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/pi-workflow/workspace.ts
+172 -24
View File
@@ -1,4 +1,4 @@
import { execFile, spawn } from "node:child_process";
import { execFile, spawn, type ChildProcess } from "node:child_process";
import { createHash } from "node:crypto";
import { constants } from "node:fs";
import {
@@ -13,7 +13,10 @@ import {
import * as path from "node:path";
import { promisify } from "node:util";
import { inflateSync } from "node:zlib";
import type { WorkflowWorkspace } from "./domain.ts";
import type {
SourceChangeEvidence,
WorkflowWorkspace,
} from "./domain.ts";
const execFileAsync = promisify(execFile);
@@ -470,7 +473,9 @@ export class GitStateProbe {
}
private async git(cwd: string, args: string[]) {
const result = await this.runner.run(cwd, "git", args);
const result = await this.runner.run(cwd, "git", args, {
timeoutMs: INTERNAL_GIT_TIMEOUT_MS,
});
if (result.code !== 0) {
throw new Error(`Git state probe failed: ${result.stderr.trim()}`);
}
@@ -505,6 +510,7 @@ const PACKAGE_EXEC_SUBCOMMANDS = new Map<string, Set<string>>([
["bun", new Set(["x"])],
]);
export const INTERNAL_GIT_TIMEOUT_MS = 30_000;
const DEFAULT_HOST_COMMAND_TIMEOUT_MS = 3 * 60 * 1_000;
export class ApprovedHostCommandExecutor {
@@ -745,22 +751,21 @@ export class GitDiffProvider {
async read(worktreeRoot: string, baseHead = "HEAD") {
const root = await realpath(path.resolve(worktreeRoot));
const diff = await this.runner.run(root, "git", [
"diff",
"--binary",
"--no-ext-diff",
baseHead,
"--",
]);
const diff = await this.runner.run(
root,
"git",
["diff", "--binary", "--no-ext-diff", baseHead, "--"],
{ timeoutMs: INTERNAL_GIT_TIMEOUT_MS },
);
if (diff.code !== 0) {
throw new Error(`Unable to read review diff: ${diff.stderr.trim()}`);
}
const untracked = await this.runner.run(root, "git", [
"ls-files",
"--others",
"--exclude-standard",
"-z",
]);
const untracked = await this.runner.run(
root,
"git",
["ls-files", "--others", "--exclude-standard", "-z"],
{ timeoutMs: INTERNAL_GIT_TIMEOUT_MS },
);
if (untracked.code !== 0) {
throw new Error(`Unable to list untracked review files: ${untracked.stderr.trim()}`);
}
@@ -796,6 +801,9 @@ export interface WorkspacePlan {
branch: string;
baseHead: string;
branchExists: boolean;
sourceChanges: SourceChangeEvidence[];
sourceChangesOmitted: number;
dirtySourceAllowed: boolean;
}
export interface PrepareWorkspaceOptions {
@@ -803,23 +811,57 @@ export interface PrepareWorkspaceOptions {
configuredRoot: string;
featureName: string;
runId: string;
allowDirtySource?: boolean;
}
export class WorkspaceValidationError extends Error {
readonly code:
| "not-repository"
| "dirty-source"
| "unmerged-source"
| "unignored-root"
| "existing-worktree-path"
| "git-failed";
readonly sourceChanges: SourceChangeEvidence[];
readonly sourceChangesOmitted: number;
constructor(code: WorkspaceValidationError["code"], message: string) {
constructor(
code: WorkspaceValidationError["code"],
message: string,
evidence: {
sourceChanges?: SourceChangeEvidence[];
sourceChangesOmitted?: number;
} = {},
) {
super(message);
this.name = "WorkspaceValidationError";
this.code = code;
this.sourceChanges = evidence.sourceChanges ?? [];
this.sourceChangesOmitted = evidence.sourceChangesOmitted ?? 0;
}
}
const MAX_SOURCE_CHANGE_EVIDENCE = 20;
function sourceChangeEvidence(status: string) {
const entries = Object.entries(changedPathStatuses(status)).sort(([left], [right]) =>
left.localeCompare(right),
);
const sourceChanges = entries
.slice(0, MAX_SOURCE_CHANGE_EVIDENCE)
.map(([changedPath, changedStatus]) => ({
path: changedPath.replace(/[\u0000-\u001f\u007f-\u009f]/g, "?"),
status: changedStatus,
}));
return {
sourceChanges,
sourceChangesOmitted: Math.max(0, entries.length - sourceChanges.length),
hasUnmerged: entries.some(([, changedStatus]) =>
["DD", "AU", "UD", "UA", "DU", "AA", "UU"].includes(changedStatus),
),
};
}
const PROCESS_TERMINATION_GRACE_MS = 100;
const PROCESS_TERMINATION_POLL_MS = 10;
const PROCESS_TERMINATION_TIMEOUT_MS = 1_000;
@@ -845,9 +887,29 @@ function signalProcessGroup(processId: number, signal: NodeJS.Signals) {
}
}
async function forceKillAndReap(child: ChildProcess) {
if (child.exitCode !== null || child.signalCode !== null) return;
if (process.platform !== "win32" && child.pid !== undefined) {
signalProcessGroup(child.pid, "SIGKILL");
}
child.kill("SIGKILL");
if (child.exitCode !== null || child.signalCode !== null) return;
await new Promise<void>((resolve) => {
const timer = setTimeout(resolve, PROCESS_TERMINATION_GRACE_MS);
child.once("close", () => {
clearTimeout(timer);
resolve();
});
});
}
async function terminateProcessTree(processId: number) {
if (process.platform === "win32") {
await execFileAsync("taskkill", ["/pid", String(processId), "/T", "/F"]);
await execFileAsync(
"taskkill",
["/pid", String(processId), "/T", "/F"],
{ timeout: PROCESS_TERMINATION_TIMEOUT_MS },
);
return;
}
signalProcessGroup(processId, "SIGTERM");
@@ -865,6 +927,14 @@ async function terminateProcessTree(processId: number) {
const COMMAND_OUTPUT_LIMIT_BYTES = 4 * 1024 * 1024;
export class ExecFileCommandRunner implements CommandRunner {
private readonly terminate: (processId: number) => Promise<void>;
constructor(options: {
terminateProcessTree?: (processId: number) => Promise<void>;
} = {}) {
this.terminate = options.terminateProcessTree ?? terminateProcessTree;
}
async run(
cwd: string,
executable: string,
@@ -880,7 +950,12 @@ export class ExecFileCommandRunner implements CommandRunner {
let timedOut = false;
let outputExceeded = false;
let termination: Promise<void> | undefined;
let terminationFinished = false;
let terminationFailure: string | undefined;
let timeout: NodeJS.Timeout | undefined;
let terminationDeadline: NodeJS.Timeout | undefined;
let terminationDeadlineReached = false;
let settled = false;
const child = spawn(executable, args, {
cwd,
detached: process.platform !== "win32",
@@ -889,7 +964,48 @@ export class ExecFileCommandRunner implements CommandRunner {
});
const terminate = () => {
if (!termination && child.pid !== undefined) {
termination = terminateProcessTree(child.pid);
termination = this.terminate(child.pid);
void termination.then(
() => { terminationFinished = true; },
(failure: unknown) => {
terminationFinished = true;
terminationFailure = failure instanceof Error
? failure.message
: String(failure);
},
);
terminationDeadline = setTimeout(() => {
if (settled) return;
terminationDeadlineReached = true;
const terminationError =
terminationFailure ??
(terminationFinished
? "process did not exit after process tree termination"
: "process tree termination did not finish within its deadline");
void forceKillAndReap(child).then(
() => {
if (settled) return;
settled = true;
resolve({
code: 125,
stdout: Buffer.concat(stdoutChunks).toString("utf8"),
stderr: `Host command termination failed: ${terminationError}`,
});
},
(failure: unknown) => {
if (settled) return;
settled = true;
const detail = failure instanceof Error
? failure.message
: String(failure);
resolve({
code: 125,
stdout: Buffer.concat(stdoutChunks).toString("utf8"),
stderr: `Host command termination failed: ${terminationError}; final force-kill failed: ${detail}`,
});
},
);
}, PROCESS_TERMINATION_TIMEOUT_MS + PROCESS_TERMINATION_GRACE_MS + 100);
}
};
const capture = (chunks: Buffer[], chunk: Buffer, stream: "stdout" | "stderr") => {
@@ -909,6 +1025,7 @@ export class ExecFileCommandRunner implements CommandRunner {
spawnError = error;
});
child.on("close", async (code) => {
if (settled) return;
if (timeout !== undefined) clearTimeout(timeout);
let terminationError: string | undefined;
try {
@@ -916,6 +1033,9 @@ export class ExecFileCommandRunner implements CommandRunner {
} catch (failure) {
terminationError = failure instanceof Error ? failure.message : String(failure);
}
if (settled || terminationDeadlineReached) return;
settled = true;
if (terminationDeadline !== undefined) clearTimeout(terminationDeadline);
const stdout = Buffer.concat(stdoutChunks).toString("utf8");
const stderr = Buffer.concat(stderrChunks).toString("utf8");
if (timedOut) {
@@ -980,8 +1100,9 @@ export class GitWorkspaceManager {
const status = await this.git(sourceRoot, [
"status",
"--porcelain",
"--untracked-files=normal",
"--porcelain=v1",
"-z",
"--untracked-files=all",
]);
if (status.code !== 0) {
throw new WorkspaceValidationError(
@@ -989,10 +1110,19 @@ export class GitWorkspaceManager {
`Unable to inspect source state: ${status.stderr.trim()}`,
);
}
if (status.stdout.trim().length > 0) {
const evidence = sourceChangeEvidence(status.stdout);
if (evidence.hasUnmerged) {
throw new WorkspaceValidationError(
"unmerged-source",
"Source checkout has unmerged paths; resolve conflicts before creating a workflow worktree",
evidence,
);
}
if (evidence.sourceChanges.length > 0 && !options.allowDirtySource) {
throw new WorkspaceValidationError(
"dirty-source",
"Source checkout must be clean before creating a workflow worktree",
"Source checkout has changes that are not included in the workflow worktree created from HEAD",
evidence,
);
}
@@ -1058,6 +1188,10 @@ export class GitWorkspaceManager {
branch,
baseHead: head.stdout.trim(),
branchExists: branchCheck.code === 0,
sourceChanges: evidence.sourceChanges,
sourceChangesOmitted: evidence.sourceChangesOmitted,
dirtySourceAllowed:
evidence.sourceChanges.length > 0 && options.allowDirtySource === true,
};
}
@@ -1075,6 +1209,9 @@ export class GitWorkspaceManager {
root: plan.worktreePath,
branch: plan.branch,
baseHead: plan.baseHead,
sourceChanges: structuredClone(plan.sourceChanges),
sourceChangesOmitted: plan.sourceChangesOmitted,
dirtySourceAllowed: plan.dirtySourceAllowed,
error: `Unable to inspect partial branch creation: ${branch.stderr.trim()}`,
};
}
@@ -1098,6 +1235,9 @@ export class GitWorkspaceManager {
root: plan.worktreePath,
branch: plan.branch,
baseHead: plan.baseHead,
sourceChanges: structuredClone(plan.sourceChanges),
sourceChangesOmitted: plan.sourceChangesOmitted,
dirtySourceAllowed: plan.dirtySourceAllowed,
error: `Existing partial branch ${plan.branch} does not match persisted base HEAD ${plan.baseHead}`,
};
}
@@ -1111,6 +1251,9 @@ export class GitWorkspaceManager {
root: plan.worktreePath,
branch: plan.branch,
baseHead: plan.baseHead,
sourceChanges: structuredClone(plan.sourceChanges),
sourceChangesOmitted: plan.sourceChangesOmitted,
dirtySourceAllowed: plan.dirtySourceAllowed,
error: `Git worktree creation returned ${result.code}: ${result.stderr.trim()}`,
});
}
@@ -1120,6 +1263,9 @@ export class GitWorkspaceManager {
root: plan.worktreePath,
branch: plan.branch,
baseHead: plan.baseHead,
sourceChanges: structuredClone(plan.sourceChanges),
sourceChangesOmitted: plan.sourceChangesOmitted,
dirtySourceAllowed: plan.dirtySourceAllowed,
};
}
@@ -1240,6 +1386,8 @@ export class GitWorkspaceManager {
}
private git(cwd: string, args: string[]) {
return this.runner.run(cwd, "git", args);
return this.runner.run(cwd, "git", args, {
timeoutMs: INTERNAL_GIT_TIMEOUT_MS,
});
}
}