Close conflict lifecycle gaps

Author: mchwang

agents/process-group.ts

@@ -60,6 +60,8 @@ const DRAIN_LIMIT_MS = 10_000;
 // Once the group is gone, how long to wait for stdout and stderr to reach end of file. Only a process outside the group
 // (one that called setsid but kept the inherited pipe) can hold them open after that, and it must not hold this call.
 const STDIO_CLOSE_MS = 1_000;
+/** Worst-case default SIGTERM grace plus post-exit group and pipe settlement. */
+export const DEFAULT_PROCESS_SETTLEMENT_MS = DEFAULT_GRACE_MS + DRAIN_LIMIT_MS + STDIO_CLOSE_MS;
 const pause = (ms: number) => new Promise<void>(resolve => setTimeout(resolve, ms));
 const notStarted = (message: string): DockerOutcome =>
   ({ status: null, stdout: '', stderr: '', error: Object.assign(new Error(message), { code: NOT_STARTED }) });

docs/implementation/pre-merge-rebase.md

@@ -20,7 +20,7 @@ After replay settles and before its first result-ref write, the rebaser supplies
 
 For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, one killable helper batches the index scan and hashes every non-conflict changed path against the same pinned `HEAD`; separate bounded helpers hash every index-only gitlink entry and the byte-exact rebase control directory without following links. These helpers run inside the operation deadline instead of blocking the server event loop or spawning once per path. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, refuses any change to the remaining rebase operation, stages only literal NUL-delimited path data on stdin, then rechecks the gitlink index so an allowed file cannot become an embedded repository. It also refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite.
 
-Before any clone or Docker operation, the production adapter records a unique child attempt plus distinct storage and network allocation IDs in `conflict`. It clones the pinned pre-conflict `HEAD`, copies only the exact conflict leaves without following ancestor links, safely recreating missing plain parents for nested modify/delete conflicts, and allocates ordinary bounded task storage. The path set is capped at 32 KiB before ownership is claimed, keeping the provider prompt below the operating system's single-argument limit. Every individual Git or Docker client—validation, execution, and cleanup included—moves the durable marker through `spawning`, its exact process group, and settled or retained ownership; a retained client prevents every later subprocess from starting. Each child subprocess budget leaves 13 seconds for group and pipe settlement, and every asynchronous child stage shares the parent's rebase-work boundary. If a stage cannot settle by that boundary, the resolver returns with durable ownership retained instead of consuming the parent's 30-second cleanup reserve. A specialized pre-launch tree check allows the expected dirty conflict paths but rejects repository metadata, commits, outside paths, unsafe types, links, populated gitlinks, and unmountable gitlink layouts. The ordinary `fix` profile then runs with no command capability. Its host-side input snapshot is created beneath the durably known conflict staging directory, so retaining the parent workspace also retains every reported directory resource for restart cleanup. On success, the adapter audits the complete manifest, exports only the exact approved leaves with a 32 MiB raw-content budget, and copies them back without exposing the host worktree or shared Git metadata to the agent. Storage and staging cleanup complete before the marker clears; an unreleased resource keeps the marker and parent workspace durable. Startup recovery removes agent/network leftovers through lane D, aborts the parent worktree (including profile snapshots), removes an exactly matching child storage handle, and only then clears child and parent ownership.
+Before any clone or Docker operation, the production adapter records a unique child attempt plus distinct storage and network allocation IDs in `conflict`. It clones the pinned pre-conflict `HEAD`, copies only the exact conflict leaves without following ancestor links, safely recreating missing plain parents for nested modify/delete conflicts, and allocates ordinary bounded task storage. The path set is capped at 32 KiB before ownership is claimed, keeping the provider prompt below the operating system's single-argument limit. Every individual Git or Docker client—validation, execution, and cleanup included—moves the durable marker through `spawning`, its exact process group, and settled or retained ownership; a retained client prevents every later subprocess from starting. Each child subprocess budget leaves 17 seconds for the default 5-second stop grace, 10-second group drain, 1-second pipe drain, and durable settlement, and every asynchronous child stage shares the parent's rebase-work boundary. If a stage cannot settle by that boundary, the resolver returns with durable ownership retained instead of consuming the parent's 30-second cleanup reserve. A specialized pre-launch tree check allows the expected dirty conflict paths but rejects repository metadata, commits, outside paths, unsafe types, links, populated gitlinks, and unmountable gitlink layouts. The ordinary `fix` profile then runs with no command capability. Its host-side input snapshot is created beneath the durably known conflict staging directory, so retaining the parent workspace also retains every reported directory resource for restart cleanup. On success, the adapter audits the complete manifest, exports only the exact approved leaves with a 32 MiB raw-content budget, and copies them back without exposing the host worktree or shared Git metadata to the agent. Storage and staging cleanup complete before the marker clears; an unreleased resource keeps the marker and parent workspace durable. Startup recovery removes agent/network leftovers through lane D, aborts the parent worktree (including profile snapshots), removes an exactly matching child storage handle, and only then clears child and parent ownership.
 
 The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added.
 

runner/rebase-conflict.ts

@@ -9,7 +9,7 @@ import { checkConflictTree, inspectTaskChanges, MAXIMUM_DECLARED_LINKS, MAXIMUM_
   type TaskChangeManifest, type TaskTreeCheck } from '../agents/container/changes.ts';
 import { exportTaskPaths, prepareTaskFilesystemsAsync, removeTaskFilesystemsAsync,
   type ExportedTaskPath, type PreparationOptions, type TaskFilesystems, type TaskStorageLimits } from '../agents/container/storage.ts';
-import type { ProcessGroup } from '../agents/process-group.ts';
+import { DEFAULT_PROCESS_SETTLEMENT_MS, type ProcessGroup } from '../agents/process-group.ts';
 import { runTrackedProcess, type ProcessGroupLifecycle } from '../agents/tracked-docker.ts';
 import type { PlanIdentity } from '../core/identity.ts';
 import { createTaskCloneAsync } from '../git/clone.ts';
@@ -20,8 +20,8 @@ import type { Store } from './store.ts';
 
 export const MAX_CONFLICT_SNAPSHOT_BYTES = 32 * 1024 * 1024;
 export const MAX_CONFLICT_PATH_BYTES = 32 * 1024;
-// A timed-out process may need 1 s of graceful termination, 10 s to drain descendants and 1 s to close held pipes.
-export const CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS = 13_000;
+// Reserve the tracked process's complete default stop/drain/pipe budget, plus one second for durable settlement writes.
+export const CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS = DEFAULT_PROCESS_SETTLEMENT_MS + 1_000;
 const SCHEMA = '{"$schema":"https://json-schema.org/draft/2020-12/schema","title":"codeboost conflict resolution summary","type":"string"}\n';
 
 interface ResolverDeps {
@@ -193,6 +193,8 @@ export function createForeignConflictResolver(options: ForeignConflictResolverOp
     assertConflictPathSet(input.files);
     if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(input.baseHead)) throw new Error('Conflict base must be a full commit ID.');
     if (!Number.isSafeInteger(input.deadline) || input.deadline <= Date.now()) throw new Error('Conflict deadline has passed.');
+    // A pre-cancelled request owns nothing: check immediately before the first durable child claim.
+    input.signal?.throwIfAborted();
     const childAttemptId = randomUUID(), allocationId = randomUUID(), networkAllocationId = randomUUID();
     options.store.beginRebaseConflict(options.planKey, input.attemptId,
       { attemptId: childAttemptId, allocationId, networkAllocationId, source: input.commit });

runner/store.ts

@@ -811,6 +811,8 @@ export class Store {
       const task = this.#task(planKey), marker = task.rebase_in_progress === null ? null : decode<RebaseMarker>(task.rebase_in_progress);
       if (marker?.attemptId !== attemptId || marker.conflict?.attemptId !== conflictAttemptId)
         throw new GuardRefusal('This conflict child no longer owns the rebase.');
+      if (marker.processGroup !== null)
+        throw new GuardRefusal('The conflict child process has not settled.');
       if (this.#run('UPDATE tasks SET rebase_in_progress=? WHERE plan_key=? AND rebase_in_progress=?',
         encode({ ...marker, conflict: null }), planKey, task.rebase_in_progress as string).changes !== 1)
         throw new GuardRefusal('This conflict child no longer owns the rebase.');

test/runner-lifecycle-store.test.ts

@@ -588,6 +588,10 @@ describe('pre-merge rebase ownership', () => {
     expect(() => store.prepareRebaseResult(planKey, marker.attemptId, oid(4), [oid(4)], [oid(2)])).toThrow(/child has not settled/);
     expect(store.abortRebase(planKey, marker.attemptId)).toBe(false);
     expect(() => store.clearRebaseConflict(planKey, marker.attemptId, randomUUID())).toThrow(/no longer owns/);
+    store.setRebaseProcessGroup(planKey, marker.attemptId, null, 'spawning');
+    expect(() => store.clearRebaseConflict(planKey, marker.attemptId, child)).toThrow(/process has not settled/);
+    expect(store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: { attemptId: child }, processGroup: 'spawning' });
+    store.setRebaseProcessGroup(planKey, marker.attemptId, 'spawning', null);
     store.clearRebaseConflict(planKey, marker.attemptId, child);
     expect(store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: null });
     expect(store.abortRebase(planKey, marker.attemptId)).toBe(true);

test/runner-rebase-conflict.test.ts

@@ -8,6 +8,7 @@ import type { AgentAdapterRequest } from '../agents/adapters/types.ts';
 import type { TaskChangeManifest, TaskTreeCheck } from '../agents/container/changes.ts';
 import type { ExportedTaskPath, TaskFilesystems } from '../agents/container/storage.ts';
 import type { ProcessGroupLifecycle } from '../agents/tracked-docker.ts';
+import { DEFAULT_PROCESS_SETTLEMENT_MS } from '../agents/process-group.ts';
 import type { Plan, PlanContext } from '../core/plan.ts';
 import type { AsyncCloneOptions } from '../git/clone.ts';
 import { CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS, copyConflictPaths, createForeignConflictResolver,
@@ -94,6 +95,22 @@ const input = (f: ReturnType<typeof fixture>, signal?: AbortSignal): RebaseConfl
   commit: oid(2), baseHead: oid(2), files: ['conflict.txt'], repository: f.repository, deadline: Date.now() + 60_000, signal });
 
 describe('production foreign conflict resolver', () => {
+  it('reserves the complete default tracked-process settlement budget plus its durable-write margin', () => {
+    expect(CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS).toBe(DEFAULT_PROCESS_SETTLEMENT_MS + 1_000);
+    expect(DEFAULT_PROCESS_SETTLEMENT_MS).toBe(16_000);
+  });
+
+  it('does not claim a conflict child for an already-aborted request', async () => {
+    const f = fixture(), x = deps(f), reason = new Error('cancelled before conflict admission');
+    const resolve = createForeignConflictResolver({ store: f.store, identity, planKey: f.planKey,
+      repository: { path: join(f.root, 'bare.git') } as RunnerRepository, runnerOwner: 'a'.repeat(32),
+      image: vi.fn(() => 'sha256:' + 'b'.repeat(64)), token: 'secret',
+      limits: { workBytes: 1, workInodes: 1, metadataBytes: 1, metadataInodes: 1 }, deps: x.d as never });
+    await expect(resolve(input(f, AbortSignal.abort(reason)))).rejects.toBe(reason);
+    expect(x.events).toEqual([]);
+    expect(f.store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: null, processGroup: null });
+  });
+
   it('bounds the complete host snapshot before changing the destination', () => {
     const root = mkdtempSync(join(tmpdir(), 'codeboost-conflict-copy-')); roots.push(root);
     const source = join(root, 'source'), destination = join(root, 'destination');