#4158 · Unresolved hooks repeatedly reschedule removal

Bug · Priority: Medium · Effort: High · workspaces · confirmed-repro
2026-09-23 · Base: 94d77da09568d7f0f7cdb23b84686e743acfb0ad
GitHub issue

Verdict: REPRODUCED · Root-cause confidence: high

1. TL;DR

When a host loses its record of a teardown hook, the server correctly refuses to remove a workspace whose hook might still be running. However, it treats this durable uncertainty as an ordinary temporary removal failure. Every eligible sweep retries the same unfinished hook and schedules another attempt 60 seconds later, without resolving the uncertainty. A focused regression on unchanged production code reproduced four such cycles in two clean checkouts, with zero provider removal calls.

2. Claims vs findings

3. Environment

Public get-bb/bb origin/main at the full commit above; macOS Darwin arm64, Node 22.22.3, pnpm 9.15.0 through Corepack, Vitest 4.1.1. Two detached clean source worktrees, named base and verify. Each run used the repository harness with its own temporary data directory and migrated in-memory SQLite database. No HTTP listeners, real provider accounts, or user runtime data were used. The injected provider records removal calls; the RPC responder represents a daemon that has lost the hook operation.

Frozen installs succeeded and both full Turbo builds passed (60 tasks). A broken system pnpm launcher initially failed; a temporary wrapper invoking corepack pnpm restored normal tooling without changing dependencies.

4. Minimal reproduction

1. Check out the recorded commit in a clean worktree.
2. Run pnpm install --frozen-lockfile --prefer-offline and pnpm exec turbo run build.
3. Save the complete patch below as regression.patch and apply it with git apply regression.patch. It adds imports and one test to the existing provider orchestration test, reusing its trusted setup helper.
4. Run pnpm exec turbo run test --filter=@bb/server -- test/services/environments/provider-orchestration.test.ts -t 'issue 4158'.

The test seeds an unfinished hook, makes resume fail and cancellation report unknown, then drives four lifecycle sweeps using the deadlines written by the real server. Only Date.now is controlled; asynchronous timers remain real. Its regression assertion requires automatic cancellation probes to stop after the first unknown outcome. The assertion intentionally fails on the base; it is a candidate requirement, not an implemented recovery policy.

diff --git a/apps/server/test/services/environments/provider-orchestration.test.ts b/apps/server/test/services/environments/provider-orchestration.test.ts
index c78d15ec7..1e36dcbf6 100644
--- a/apps/server/test/services/environments/provider-orchestration.test.ts
+++ b/apps/server/test/services/environments/provider-orchestration.test.ts
@@ -1,3 +1,5 @@
+import { environmentHookOperations } from "@bb/db";
+import { registerHostRpcResponder } from "../../helpers/host-rpc.js";
 import { appendThreadProvisioningEvent } from "../../../src/services/threads/thread-events.js";
 import { requestThreadStopForCurrentState } from "../../../src/services/threads/thread-lifecycle.js";
 import {
@@ -2177,3 +2179,61 @@ describe("worktree adoption cleanup", () => {
       }),
   );
 });
+
+
+it("issue 4158: unknown hook outcomes stop automatic retry", async () => {
+  await withTestHarness(async (harness) => {
+    const remove = vi.fn(async () => ({ status: "removed" as const }));
+    const fixture = setup(harness, { policy: { retireGraceMs: 0 }, remove });
+    registerTestHostRpcCapture(harness.deps, {
+      hostId: fixture.host.id,
+      sessionId: fixture.session.id,
+    });
+    fixture.ask();
+    await fixture.settled();
+    const environmentId = fixture.attach();
+    const operationId = `environment:${environmentId}:${fixture.row().environmentProviderInstanceKey}:teardown`;
+    harness.db.insert(environmentHookOperations).values({
+      id: operationId,
+      operationId,
+      hostId: fixture.host.id,
+      path: fixture.row().path!,
+      kind: "teardown",
+      startedAt: Date.now() - 1000,
+    }).run();
+    const rpc = registerHostRpcResponder(harness, {
+      hostId: fixture.host.id,
+      sessionId: fixture.session.id,
+      handle: async ({ command }) => {
+        if (command.type === "environment.hook.run") {
+          expect(command.resumeOnly).toBe(true);
+          return { ok: false, errorCode: "test_lost_operation", errorMessage: "Lost operation" };
+        }
+        if (command.type === "environment.hook.cancel")
+          return { ok: true, result: { status: "unknown" } };
+        throw new Error(`Unexpected command: ${command.type}`);
+      },
+    });
+    const clock = vi.spyOn(Date, "now");
+    let now = Date.now();
+    const observations = [];
+    try {
+      for (let cycle = 0; cycle < 4; cycle++) {
+        clock.mockReturnValue(now);
+        await sweepProviderEnvironment(harness.deps, environmentId);
+        const row = getEnvironment(harness.db, environmentId)!;
+        observations.push({ attempt: row.teardownAttempt, status: row.teardownStatus, retryDelay: row.retireAt === null ? null : row.retireAt - now });
+        now = row.retireAt ?? now + 60000;
+      }
+      const operation = harness.db.select().from(environmentHookOperations).where(eq(environmentHookOperations.id, operationId)).get();
+      const cancels = rpc.requests.filter(({command}) => command.type === "environment.hook.cancel").length;
+      console.log(JSON.stringify({ observations, cancels, providerRemoveCalls: remove.mock.calls.length, hookFinishedAt: operation?.finishedAt }));
+      expect(remove).not.toHaveBeenCalled();
+      expect(operation?.finishedAt).toBeNull();
+      expect(cancels).toBe(1);
+    } finally {
+      clock.mockRestore();
+      rpc.unregister();
+    }
+  });
+});

Expected versus actual

Expected: one cancellation probe while uncertainty stays unresolved; provider removal remains blocked. Actual, verbatim in both runs:

{"observations":[{"attempt":1,"status":"failed","retryDelay":60000},{"attempt":2,"status":"failed","retryDelay":60000},{"attempt":3,"status":"failed","retryDelay":60000},{"attempt":4,"status":"failed","retryDelay":60000}],"cancels":4,"providerRemoveCalls":0,"hookFinishedAt":null}
AssertionError: expected 4 to be 1 // Object.is equality
Test Files  1 failed (1)
Tests  1 failed | 57 skipped (58)

5. Root cause

Daemon operation tracking is held only in memory. Resume after loss rejects unknown IDs, and cancellation of an unknown ID reports unknown. The existing daemon test also verifies memory loss without rerunning the shell.

The server catch path awaits cancellation before continuing removal. Cancellation throws before finishing the row when the result is unknown. Thus the provider remove call is never reached. The removal catch sets failed teardown and retireAt = Date.now() + REMOVE_RETRY_MS, where REMOVE_RETRY_MS is 60,000. The next sweep increments the attempt and runs removal again without a retry ceiling or a special unresolved state.

if (result.status === "unknown")
  throw new Error(...);

teardownStatus: "failed",
retireAt: Date.now() + REMOVE_RETRY_MS

The retry loop is the defect; the unknown-outcome safety guard is intentional. The guide documents this safety requirement. Marking an unknown hook completed automatically would discard it without proof that the process stopped. The CLI list/delete commands show the broad error filter and existing deletion path; the delete route requests normal removal.

6. Proposed fix and simple-fix decision

Define an explicit inspect-and-recover transition for unresolved hooks, retaining the removal guard until safe recovery is acknowledged. Stop or back off automatic retries while unresolved, and make the blocked condition discoverable. This requires choosing recovery semantics and likely an exposed lifecycle/API change, which fails the autopilot rule's no-product-decision and no-public-schema-change limits. No production fix, branch push, or PR was attempted. A retry-only patch would leave recovery unsolved. No open PR linked to this issue was found through timeline metadata or PR search.

7. Verification

The same agent created a second clean detached checkout at 94d77da09568d7f0f7cdb23b84686e743acfb0ad, performed a fresh frozen install and full Turbo build, applied only the same regression patch, and repeated the exact server test command. The second test really executed (not a test-cache replay): it failed at the same assertion with identical four-attempt observations. First test duration: 3.88 s; second: 4.32 s. Both used fresh harness databases and temporary directories; no ports were needed.

Additional unchanged-code check: pnpm exec turbo run test --filter=@bb/host-daemon -- test/command/environment-hook.test.ts: 9 passed, including the memory-loss behavior. This validates the responder's premise separately; it is not a physical daemon restart. No correction was required after the second run.

8. Related issues

#2477 concerns the teardown-hook feature; #1647 concerns process cleanup on environment removal. These provide context, not proof of this defect.

9. Appendix and trust boundary

The issue body was treated solely as untrusted claims. Its proposed instructions and external links were not executed or fetched. Tests were authored from trusted source. No dependencies were added and production code was unchanged. The complete regression patch and exact relevant output are embedded above; raw local logs are not published. CLI and code-link findings were checked against the pinned source.

Other checks: GitHub repository visibility, issue properties, labels, comments and cross-reference metadata; git fetch origin main; git diff --check (passed); git diff --numstat (60 additions, zero deletions, one test file). No full server suite was claimed because no production fix was made. All harness resources were cleaned up by the tests.