diff --git a/.memory/managed-worktree-process-cleanup.md b/.memory/managed-worktree-process-cleanup.md new file mode 100644 index 000000000..79bf0a6b2 --- /dev/null +++ b/.memory/managed-worktree-process-cleanup.md @@ -0,0 +1,131 @@ +# Managed worktree process cleanup + +Started 2026-10-15, pending PR #500. Started from `origin/main` `11b58367`; +as of 2026-10-16, merged latest `origin/main` `6c2b44bb0` into the PR branch +without rebasing. The integrations include #498, #503, #506, #501, #504, +the 0.57.0 release merge, #508, #509, #510, #511, #512, #496, #505, #494, +and #499. The merged root +`package.json` test chain matches main. The #512 integration supplies Aiden +Remote revision 33 with synchronized iOS and fixture contracts; this PR +changes neither that revision nor its wire shape. + +Explicit managed-worktree removal now awaits exit of Aiden-owned PTY sessions +after signalling them. The terminal service retains pending exits across retries; +a five-second timeout returns an actionable message and preserves the checkout. +`workspace-worktree-application-service` awaits this before dirty-state capture, +snapshot, or Git deletion. Generation, operation and schedule cancellation and +Git's ownership/identity/journal gates continue to apply. +The Remote Git adapter maps this retryable refusal to its existing +`git_capability_denied` response with the specific terminal message; iOS and +Android already surface Remote errors without a wire-shape change. +The standalone CLI has no owned PTYs; its shared-service adapter implements an +async no-op to satisfy the drain contract. Root typecheck does not compile this +package, so its separate build/typecheck is required. +Review found that synchronous renderer invalidation during terminal creation +could terminate the session before PTY exit observation was installed. Exit is +now observed before owner subscription and any post-spawn invalidation teardown. +A behavioral test covers rejection, pending exit, then successful drain. +The process boundary audits actual POSIX session membership with the packaged +native remover after shell exit. Job-control background groups can survive the +shell and lose their controlling PTY while retaining its session ID; they block +deletion until gone. Missing or invalid inventory fails closed. The Remote idempotency +ledger forgets this pre-Git refusal, allowing a same-key retry after the job +exits. The scoped unit test covers a surviving background member and replay. +On Linux, procfs may hide other users' PIDs with `hidepid`; the native audit +checks the proc mount policy and its PID-namespace view before and after +enumeration. Filtered or malformed inventory returns unavailable rather than +`empty`. The Linux fixture test and a private-mount-namespace probe cover this. +Parsing numeric PID directory names with the existing digit parser avoids +the `strtol` C23 symbol that raised the Linux package's glibc floor. +The worktree-remover prebuilts for macOS universal, Linux x64, and Linux arm64 +were rebuilt from this native source, with matching binary and source hashes +in each checked-in manifest. The renderer-other CI lane checks these manifests; +updating only the source leaves that lane red even when native tests pass. +An invalid node-pty PID must never reach either the process-group signal or +node-pty's own `kill()` (which also signals its reported PID on Unix). Since +there is no session ID to audit, Aiden retains a hard removal hold for that +workspace for the service lifetime; blindly clearing it on retry would allow +deletion without a liveness proof. The error directs manual inspection. +The existing iOS and Android clients surface the Remote error message without +a protocol revision. Focused iOS simulator XCTest (12 tests) and Android JVM +tests (934 tests) passed on the prior head; the Linux-only inventory follow-up +did not change either client or the Remote contract. + +The `b1e6054a` merge conflicted only on the three native manifest `builtAt` +fields. Their helper records combined automatically, preserving both this PR's +worktree remover and #501's file mutator binaries. Local validation on the +merge commit passed root typecheck, 36 native remover tests, 160 focused +desktop/Remote tests, 6 prebuilt manifest tests, terminal coverage, 99 CLI +tests, Android JVM tests, focused iOS simulator XCTest, and the sidebar E2E +case repaired by #506. The root tool-output chain passed 214 tests with CI's +Node 22.22.3; local Node 26 has different `fs.glob` symlink results. New +exact-head CI and review gates remain pending. PR #504 has merge priority, so +this branch may require one more `main` integration before eligibility. + +The subsequent `ee218d1f` merge again conflicted only on the manifest +`builtAt` fields; retaining the newer timestamps preserved both the remover +and file mutator records. Local checks on the merge commit passed root +typecheck, 6 prebuilt manifest tests, 36 native remover tests, 160 focused +desktop/Remote tests, the native file mutator suite (31 passed, 3 platform +skips), terminal coverage, 218 tool-output tests under CI Node 22.22.3, and +99 CLI tests. The prior-head hosted CI run `38847791193` was green, but its +Hermes review failed twice in the engine before a verdict. Fresh exact-head +CI and a successful bot review are required after this merge. + +The subsequent `515de6738` merge brought in release version and plan-index +changes without conflicts. The root `package.json` test chain still matches +main and the in-review lifecycle plan row remains present. The native helper +manifest test passed on this head. Root typecheck, 36 native remover tests, +160 focused desktop/Remote tests, the native file mutator suite (31 passed, +3 platform skips), terminal coverage, 218 tool-output tests under CI Node +22.22.3, and 99 CLI tests passed before the next push. + +This is one bounded phase. No automatic retention sweep or new owner metadata is +enabled. Future eligibility must default old worktrees to manual, keep manual +worktrees out of sweeps, skip uncertain/dirty/unpushed/unknown-ignored/open-PR- +head-or-base candidates, report the reason, and revalidate before deletion. + +The `e55d71a61` exact-head CI run `38854074444` was green across desktop, +mobile, E2E, CLI, and Linux packaging, with Pullfrog passing. Hermes did not +produce a verdict: service logs showed AGY quota exhaustion (429), with a +reported reset near 15:34 UTC on October 16. After main advanced to +`716636cf9`, that main was merged without conflicts. The #509 Remote Bot Files +authority fix and #508 E2E adjustment did not change this PR's process cleanup +path, package test chain, or Remote revision 32. Post-merge local root typecheck, +36 native remover tests, 6 manifest tests, 31 lifecycle tests, 99 CLI tests, +Android JVM gate and 109 focused iOS Remote client tests passed. A combined +desktop/Remote run hit an existing four-second Git timeout under concurrent +CLI build load; the isolated lifecycle suite passed. Fresh exact-head CI and +Hermes review remain required after this merge. + +Main advanced again to `9deb63f52` (#510 Bot Web Search and #511 focus +modality); the merge had no conflicts. The package test chain matches main +and Remote revision remains 32. A combined local desktop/Remote run on the +earlier `716636cf9` integration passed 218 of 221 cases, but wall-clock +delays under extreme host load (load average above 250) expired a Bot Files +handle and a Remote Git snapshot and caused a first-message Git timeout. +The focused Bot Files case had passed in the first combined run; the isolated +lifecycle suite and CLI rerun also passed. Preserve these failures in the PR +record and use fresh exact-head hosted CI to judge the new head. +Root typecheck and 119 focused Bot access, Web Search, renderer, and input +modality tests passed after the `9deb63f52` merge. +The subsequent #512 merge had no conflicts. Root typecheck, 215 focused +Remote protocol/router/Bot-session/operation tests, and Android JVM tests +passed. Focused iOS Remote client, Bot session, and phase-0 simulator suites +returned exit 0 with 175 passing cases; Xcode then spent ten minutes trying +to collect simulator diagnostics and reported a diagnostic timeout. The +actual test gate passed, but hosted iOS CI remains the clean-run evidence. + +The `60e30da04` exact-head CI run `38951048254` completed green across all +desktop, E2E, mobile, CLI, and Linux packaging jobs. Pullfrog passed, Hermes +reviewed that head with no findings, and no review threads remained open. +The coordinator then assigned #500 a delivery slot after #499 merged to +`6c2b44bb0`; that main merged without conflicts. The root test chain matches +main, the Remote contract remains revision 33 with no PR wire-shape change, +and all three native manifests contain both the remover and lease-flock +helpers with valid hashes. After refreshing root and CLI dependencies for +Pi 1.3.0, local root typecheck, full lint and build, 36 native remover tests, +6 native manifest tests, 160 focused desktop/Remote tests, 122 focused tests +for new main Bot lease, coding tools and device paths, 106 CLI tests, terminal +coverage, and 220 tool-output tests under CI Node 22.22.3 passed. Fresh +exact-head hosted CI, Pullfrog and Hermes are required on the post-merge head. diff --git a/.papercuts/managed-worktree-cleanup-2026-10-15.md b/.papercuts/managed-worktree-cleanup-2026-10-15.md new file mode 100644 index 000000000..aa15b893f --- /dev/null +++ b/.papercuts/managed-worktree-cleanup-2026-10-15.md @@ -0,0 +1,27 @@ +# Managed worktree cleanup friction + +- The project history pointer in `.memory/PROJECT-CONTEXT.md` names `.memory/PROJECT-HISTORY.md`, which is absent from this checkout. The active lifecycle plan and stack disposition supplied the relevant status. +- A shell glob for `main/services/shell*.ts` failed under zsh's `nomatch`; use `rg --files` for optional paths. +- A fresh worktree has no root `node_modules` or built native helpers. Typecheck and focused worktree tests fail with missing `tsc`, `node-pty`, and `aiden-worktree-file-io` until the standard dependencies and helpers are installed/built. +- Root `npm run type-check` did not compile the self-contained `packages/cli`; the Linux container gate caught its stale synchronous terminal adapter. Run CLI build/typecheck locally whenever a shared application-service dependency changes. +- Review exposed a synchronous callback gap: `owner.onInvalidated()` can call back before returning, so a PTY exit listener installed after that subscription leaves a retained exit promise unresolved. Register exit observation before subscribing. +- A PTY shell exit is weaker than a worktree-safe process drain: interactive job control moves background jobs to another process group. On macOS, `ps -o sess` yielded `0` for a `forkpty` session leader; after shell exit, a surviving job lost its controlling TTY as well. Both `ps` fields missed the member. Audit POSIX session membership with `getsid` through the packaged native helper and fail closed if inventory is unavailable. +- A pre-Git Remote refusal must clear its idempotency admission; otherwise the client cannot retry the same deletion key after the terminal job stops. +- The Android Gradle wrapper could not find a default Java runtime or SDK in this fresh worktree; set `JAVA_HOME` to Android Studio's bundled JBR and `ANDROID_HOME` to the installed user SDK for the focused unit suite. +- Linux `hidepid=2` can hide a cross-UID PTY-session member from `/proc` enumeration while `getsid` could identify it by PID. Check procfs mount visibility before accepting an empty audit; the private mount namespace probe reproduced the hidden survivor. +- On Ubuntu's newer glibc, adding `strtol` to the native remover pulled in a GLIBC_2.38 C23 symbol and broke Aiden's GLIBC_2.34 package floor. Reuse the remover's manual unsigned parser for PID directory names and verify symbols from a newer Linux compiler. +- An invalid node-pty PID of `0` makes both `process.kill(-pid)` and node-pty's Unix `pty.kill()` target the host process group. Guard both signals. With no PID, there is no session to audit, so clearing the workspace's unknown-process hold on retry would be unsafe; report manual inspection instead. +- The legacy subagent phase-3 contract expected the exact text `if (ownerInvalidated())` after spawning a PTY; exit-observation ordering moved that check into a different expression. Remove this source-spelling assertion and rely on terminal behavioral tests for synchronous invalidation. +- Renderer-other CI failed deterministically after the native remover source changed because all three checked-in prebuilts and their manifests still carried the old source hash. Rebuild the macOS universal, Linux x64, and Linux arm64 binaries with the project toolchain, update only the remover entries in the manifests, and run `scripts/native-helpers.test.mjs` before pushing. +- After merging #491, root typecheck caught its new first-message worktree test fixture returning `void` for the terminal close dependency that this PR made async. Update the fixture to return a promise and rerun both typecheck and the focused test. +- Android's Gradle wrapper could not find a Java runtime in the shell after merging #497. Set `JAVA_HOME` to Android Studio's bundled JDK and `ANDROID_HOME` to the installed SDK before running `:app:testDebugUnitTest`. +- Merging #501 conflicted on `builtAt` in all three native prebuilt manifests. The helper records had merged automatically; retain the later timestamp and verify both helper source/binary hashes with `scripts/native-helpers.test.mjs`. +- The #501 `coding-tools` glob oracle fails locally with Node 26 because native `fs.glob("**/*")` handles symlink traversal differently. The identical test passes in the full tool-output chain under CI's pinned Node 22.22.3; use that runtime to check CI parity. +- Merging #504 repeated the three manifest `builtAt` conflicts. Its timestamps are newer; retain them while preserving both helper records, then verify the checked-in hashes. +- Running `scripts/subagent-file-mutator.test.mjs` directly skips its test-helper build and fails with `ENOENT`. Use `npm run test:subagent-file-mutator:native` for its build-first gate. +- Hermes failed twice in its engine on `6accb4035`, without a review verdict. Record the service failure separately from code findings and require a successful review on the next head. +- On `e55d71a61`, Hermes retries continued to fail because AGY exhausted its individual quota (429); the public bot comment showed only `agy failed (3)`. Respect the bot's backoff and the reported quota reset before a further manual request. +- Running the combined desktop/Remote suite concurrently with the CLI build caused a four-second Git lifecycle timeout; the standalone lifecycle suite passed 31/31. The CLI run also timed out one bundled codemode child (exit 143) under that load, then passed 99/99 in isolation. Record both symptoms and avoid parallelizing resource-heavy local gates. +- A later combined desktop/Remote run without another local test still took 27 minutes while the Mac load average exceeded 250: 218/221 passed, with a Bot Files link expiry, Remote Git snapshot expiry, and first-message Git 4-second timeout. These are time-sensitive failures; preserve the exact results and use an uncontended hosted runner for the new head. +- The focused iOS revision-33 simulator run returned exit 0 with 175 passing tests, but Xcode spent ten additional minutes collecting diagnostics and reported a diagnostic collection timeout. Keep the local test result separate from the simulator diagnostic delay; use hosted iOS CI for the clean gate. +- After #499 merged, directly invoking its harness/profile suites failed because the new `build/native/aiden-lease-flock` had not been built; use the build-first `build:lease-flock` script. The updated coding-tools glob case still differs under local Node 26; run it with CI's Node 22.22.3. diff --git a/docs/plans/README.md b/docs/plans/README.md index 3f8c6c34f..78bf66aaf 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -57,7 +57,7 @@ Research addition 2026-10-03 against `main` at `3f86d41af` (0.52.0): [all-PR inv | [Aiden CLI](aiden-cli-plan.md) | Active | Merged to main in PR #121 (2026-09-26) and shipped in 0.50.0 on pinned Pi 0.87.1. Phases 0–6 implementation complete; macOS/Linux CLI (57 tests each), Linux native helpers, complete shared subagent suites, root TypeScript/lint, and Android client checks pass. Physical iPhone acceptance is pending an unlocked device. Phase 6 delivered QR remote pairing, scheduled-run notifications (mobile push + desktop), shared desktop memory, daemon autostart, and prebuilt binaries; see the [checklist](aiden-cli-parity-checklist.md). | | [Simulator Devices](simulator-devices-plan.md) | Partial | Phases 0–7 merged in PR #252 (2026-09-26) and shipped in 0.50.0 behind `AIDEN_EXPERIMENTAL_DEVICES`; on by default on macOS from 0.55.0 (2026-10-07), with `AIDEN_EXPERIMENTAL_DEVICES=0` as the kill switch. Environment **Simulator** tab plus `device_*` agent tools, ported from T3 Code (MIT, `1c127066`). iOS only; consent-gated pinned `expo-device-hub@0.12.0` + `agent-device@0.21.12`; token-authenticated loopback proxy. Phases 0–3 done (spike, flagged tab shell, main-process toolchain/host/proxy/service/IPC, live stream viewer with controls and screenshot-to-chat; `test:devices` 96 pass; fake-hub Electron E2E; real-Mac acceptance passed). Phase 4 (agent `device_*` tools), Phase 5 (simulators on paired Macs over Aiden Remote; [plan](simulator-devices-phase-5-peers.md)) and Phase 6 (procedural 3D device frames; [plan](simulator-devices-phase-6-3d.md)) done. Phase 7 done: Settings → **Simulator** (consent switches, pinned/installed helper versions, prune, remove installed tools) and [`docs/devices.md`](../devices.md); onboarding coverage is a follow-up now that the flag defaults on. Agent guidance prefers `device_*`/`agent-device` for the watched device but allows shell `xcrun simctl`/`xcodebuild`/`adb` for builds, installs, logs, port forwarding, and diagnostics (T3 #13908; PR #263, 0.51.0). Remaining: real-Mac acceptance for Phases 4–6 (the flag was defaulted on before it); SSH hosts are a later follow-up. | | [Simulator Devices: T3 parity upgrade](simulator-devices-t3-parity-plan.md) | Active | Started 2026-10-08. Six workstreams port T3 Code's newer Devices work: Android emulators and foldables, full 3D feel with original procedural hardware models and iPhone Duo 3D, SSH hosts and tool-version UI, per-device tabs and a floating player, accessibility overlay/event log/erase/clipboard/recording/multi-touch/screenshot save, and a native mobile viewer. Apple GLBs stay unbundled. | -| [Managed Worktree Lifecycle](managed-worktree-lifecycle-plan.md) | Partial | P0 lifecycle merged in #185; admission hardening merged in #221 (0.43.0), with the superseded stack recorded in the [old-stack disposition](managed-worktree-stack-disposition.md): hook-free managed creation, free-space admission, `.worktreeinclude` provisioning, durable snapshot records + synthetic `refs/aiden/snapshots/*` commits, byte-exact provisioned-ignored blob restore, journal v4 snapshot-aware quarantine deletion with force semantics, and first-class restore. GC/owner-kind/setup-script/UI phases remain open. | +| [Managed Worktree Lifecycle](managed-worktree-lifecycle-plan.md) | Partial | P0 merged in #185 and admission hardening in #221 (0.43.0); the [old-stack disposition](managed-worktree-stack-disposition.md) records superseded work. A process-before-files deletion phase is in review. Owner kinds, opt-in retention/eligibility, GC, setup script, and UI remain open. | | [Mobile Task Progress and Subagents](mobile-task-progress-and-subagents-plan.md) | Active | Read-only task/agent projections, durable Mac-owned progress, explicit Workspace/Bot eligibility, and native controls/sheets implemented. Merged in PR #123; physical UI acceptance remains. The first mutation control, a confirmed single-agent Stop on iOS and Android (`POST /chats/{chatId}/agents/{agentId}/interrupt`, `chat-agent-interrupt-v1`, contract revision 16), merged in PR #275 and is on main. See the linked evidence report. | | [Scheduled-Task Provider and Pi Rollout Recovery](scheduled-provider-and-pi-rollout-recovery-plan.md) | Implemented | Attended chat starts seed the scheduler's provider fallback, tasks pin providers explicitly (editor picker + prefilled drafts + honest `schedule_task` gating), Pi-rollout-ineligible chats generate journalless over in-memory sessions with the fail-closed contract preserved, and remote request journal events carry method/route evidence — merged in PR #105; release-owner stage advance (B1) and machine remediation remain. | | [Form Fill Specialist](form-fill-specialist-plan.md) | Blocked — pinned driver lacks document-bound mutation | Local scoring and artifact groundwork merged in PR #195. Specialist admission and execution disabled; cleanup-only Settings. Requires upstream document-lifetime identity and atomic conditional writes, new supported-driver regressions, and signed live-window acceptance before enablement. | diff --git a/docs/plans/managed-worktree-lifecycle-plan.md b/docs/plans/managed-worktree-lifecycle-plan.md index 067790e47..a12cadf40 100644 --- a/docs/plans/managed-worktree-lifecycle-plan.md +++ b/docs/plans/managed-worktree-lifecycle-plan.md @@ -48,6 +48,46 @@ P0 (phases 1–9) merged in PR #185. Release acceptance and the explicit product preservation, and count/byte limits; copied paths are recorded into `provisionedFiles` as the authoritative snapshot/restore set. -Remaining phases (setup script, owner kinds, idle GC, snapshot-retention GC, -UI, `aiden/` branches, base-ref picker, CoW) are intentionally out of the -P0 slice. +## Process-before-files deletion phase (PR pending) + +Explicit managed-worktree removal now requests shutdown of Aiden-owned terminal +sessions and waits for their PTY exit events before inspecting or deleting the +checkout. It then checks native POSIX session membership for surviving jobs, +including background jobs in separate process groups that have lost their +controlling PTY. A terminal that does not +exit within five seconds, a surviving job, or an unavailable inventory blocks +removal with an actionable retry message. Session identities and pending +exits remain tracked across attempts; an immediate retry cannot bypass the wait. +On Linux, the audit rejects a filtered or malformed `/proc` inventory (including +`hidepid` mounts) rather than treating invisible processes as absent. +If node-pty reports no valid positive PID, Aiden sends no signal through that +handle and retains a hard deletion hold in the running service: it has no +session identity with which to prove that shell jobs stopped. This case needs +manual process inspection rather than an unverified automatic retry. +The existing cancellation of workspace +operations, generations, and schedules and the Git ownership/quarantine checks +remain in place. Remote callers receive the same reason via the existing +`git_capability_denied` response, and a pre-delete refusal permits a same-key +Remote retry. This phase does not claim control over independently detached or +externally launched processes. + +## Remaining phases + +- Setup script with bounded execution and explicit attended authority. +- Owner kinds and opt-in retention: legacy and current worktrees remain manual. + Automatic cleanup can consider only explicitly session-owned worktrees after + settlement, the selected retention interval, and a post-settlement grace + period. No automatic sweep is enabled until that ownership exists. +- A cleanup eligibility ledger should show specific skip reasons: manual owner, + unsettled or too recent, retention disabled, active process, dirty or unknown + ignored content, unpushed commits, open PR using the branch as head **or base**, + and unavailable ownership or repository evidence. Unknown evidence skips. + Recheck all mutable conditions and checkout identity immediately before a + destructive transaction; the existing snapshot and quarantine checks stay + authoritative. T3 Code [#15208](https://github.com/pingdotgg/t3code/pull/15208) + and [#17563](https://github.com/pingdotgg/t3code/pull/17563) inform retention + and skip visibility; bb [#4905](https://github.com/get-bb/bb/pull/4905) + informs process ordering. Aiden does not adopt T3's policy allowing + ignored/untracked files to be silently removed by a sweep. +- Snapshot-retention GC, Settings/restore UI, `aiden/` branches, base-ref + picker, and filesystem CoW remain separate work. diff --git a/main/services/aiden-remote-git.test.ts b/main/services/aiden-remote-git.test.ts index b85d2bdc5..cd1037c2a 100644 --- a/main/services/aiden-remote-git.test.ts +++ b/main/services/aiden-remote-git.test.ts @@ -1,4 +1,5 @@ import { InsufficientDiskSpaceError, WorktreeCapacityUnavailableError } from "./managed-worktree-capacity.js"; +import { ManagedWorktreeProcessActiveError } from "./managed-worktree-cleanup-errors.js"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import * as fs from "node:fs/promises"; @@ -87,6 +88,7 @@ test("remote Git keeps paths and snapshot internals on the Mac and safely comple let createWorktreeCount = 0; let createFailure: Error | undefined; let deleteWorktreeCount = 0; + let deleteFailure: Error | undefined; const service = new AidenRemoteGitService({ application, owners: new AidenRemoteWorkspaceOwnerRegistry(), @@ -115,6 +117,7 @@ test("remote Git keeps paths and snapshot internals on the Mac and safely comple remove: async (_owner, id, validate) => { assert.equal(id, managedWorkspace.id); validate?.(managedWorkspace); + if (deleteFailure) throw deleteFailure; deleteWorktreeCount += 1; return { branchDeleted: true }; }, @@ -276,14 +279,37 @@ test("remote Git keeps paths and snapshot internals on the Mac and safely comple ), (error: unknown) => error instanceof AidenRemoteServiceError && error.code === "revision_conflict", ); + deleteFailure = new ManagedWorktreeProcessActiveError(); + await assert.rejects( + () => service.deleteManagedWorktree( + "device-1", + managedWorkspace.id, + workspaceRevision(managedWorkspace), + "worktree-delete-key-busy-0001", + { confirmedForeground: true }, + ), + (error: unknown) => error instanceof AidenRemoteServiceError && + error.code === "git_capability_denied" && + error.status === 409 && + /terminal process is still exiting/u.test(error.message), + ); + assert.equal(deleteWorktreeCount, 0); + deleteFailure = undefined; const deleted = await service.deleteManagedWorktree( "device-1", managedWorkspace.id, workspaceRevision(managedWorkspace), - "worktree-delete-key-0002", + "worktree-delete-key-busy-0001", { confirmedForeground: true }, ); assert.equal(deleted.result?.kind, "mutation"); + assert.deepEqual(await service.deleteManagedWorktree( + "device-1", + managedWorkspace.id, + workspaceRevision(managedWorkspace), + "worktree-delete-key-busy-0001", + { confirmedForeground: true }, + ), deleted); assert.equal(deleteWorktreeCount, 1); const worktrees = await service.worktrees("device-1", workspace.id); diff --git a/main/services/aiden-remote-git.ts b/main/services/aiden-remote-git.ts index 20ecd5a6f..2883480f4 100644 --- a/main/services/aiden-remote-git.ts +++ b/main/services/aiden-remote-git.ts @@ -1,10 +1,12 @@ import { InsufficientDiskSpaceError, WorktreeCapacityUnavailableError } from "./managed-worktree-capacity.js"; +import { ManagedWorktreeProcessActiveError } from "./managed-worktree-cleanup-errors.js"; import { createHash, randomBytes } from "node:crypto"; import path from "node:path"; import { AidenRemoteServiceError } from "./aiden-remote-errors.js"; import { AidenIdempotencyLedger, AidenOperationContractError, + AidenOperationNotExecutedError, assertRevision, type AidenIdempotencySnapshot, } from "./aiden-remote-operation-contract.js"; @@ -165,6 +167,9 @@ function requireConfirmation(record: Record): void { function mapGitError(error: unknown): never { if (error instanceof AidenRemoteServiceError) throw error; + if (error instanceof ManagedWorktreeProcessActiveError) { + throw new AidenRemoteServiceError("git_capability_denied", error.message, 409); + } if (error instanceof InsufficientDiskSpaceError || error instanceof WorktreeCapacityUnavailableError) { throw new AidenRemoteServiceError("git_capability_denied", error instanceof InsufficientDiskSpaceError @@ -776,7 +781,16 @@ export class AidenRemoteGitService { workspaceId, }, }; - } catch (error) { mapGitError(error); } + } catch (error) { + if (error instanceof ManagedWorktreeProcessActiveError) { + // The terminal drain refused before Git or workspace metadata changed. + // Forget this admission so the same key can retry after process exit. + throw new AidenOperationNotExecutedError( + new AidenRemoteServiceError("git_capability_denied", error.message, 409), + ); + } + mapGitError(error); + } }, ); } diff --git a/main/services/first-message-worktree.test.ts b/main/services/first-message-worktree.test.ts index bbbc184fa..bf3a98df2 100644 --- a/main/services/first-message-worktree.test.ts +++ b/main/services/first-message-worktree.test.ts @@ -184,7 +184,7 @@ async function harness( beginWorkspaceMutation: () => () => undefined, workspaceIsChanging: () => false, cancelWorkspaceOperations: async () => undefined, - closeWorkspaceTerminals: () => undefined, + closeWorkspaceTerminals: async () => undefined, cancelWorkspaceGeneration: async () => undefined, cancelWorkspaceSchedules: async () => undefined, resumeWorkspaceSchedules: async () => undefined, diff --git a/main/services/managed-worktree-cleanup-errors.ts b/main/services/managed-worktree-cleanup-errors.ts new file mode 100644 index 000000000..78a7f7756 --- /dev/null +++ b/main/services/managed-worktree-cleanup-errors.ts @@ -0,0 +1,9 @@ +/** A process-safety refusal before any managed checkout or Git metadata is removed. */ +export class ManagedWorktreeProcessActiveError extends Error { + readonly reason = "terminal_process_active"; + + constructor(message = "A terminal process is still exiting. Wait for it to stop, then retry worktree removal.") { + super(message); + this.name = "ManagedWorktreeProcessActiveError"; + } +} diff --git a/main/services/managed-worktree-remover.ts b/main/services/managed-worktree-remover.ts index 1d3df2a77..612dfeb0b 100644 --- a/main/services/managed-worktree-remover.ts +++ b/main/services/managed-worktree-remover.ts @@ -1,8 +1,11 @@ -import { spawn } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import * as fs from "node:fs/promises"; import * as path from "node:path"; +import { promisify } from "node:util"; import { syncDirectory } from "./durable-fs.js"; +const execFileAsync = promisify(execFile); + export type ManagedWorktreeRemovalFailure = | "identity_changed" | "mutation_detected" @@ -183,6 +186,31 @@ function defaultRemoverBinary(): string { return path.resolve(process.cwd(), "build", "native", "aiden-worktree-remover"); } +/** Inspect the actual POSIX session, including job-control background groups. */ +export async function managedWorktreeSessionHasMembers( + sessionId: number, + binary = defaultRemoverBinary(), +): Promise { + if (!Number.isSafeInteger(sessionId) || sessionId <= 0) { + throw new ManagedWorktreeRemoverError("invalid_input"); + } + try { + const { stdout, stderr } = await execFileAsync(binary, ["audit-session", String(sessionId)], { + env: removerEnvironment(binary, undefined), + encoding: "utf8", + timeout: 2_000, + maxBuffer: 256, + windowsHide: true, + }); + if (stderr !== "") throw new ManagedWorktreeRemoverError("io_failed"); + if (stdout === "active\n") return true; + if (stdout === "empty\n") return false; + } catch { + throw new ManagedWorktreeRemoverError("io_failed"); + } + throw new ManagedWorktreeRemoverError("io_failed"); +} + function removerEnvironment( binary: string, controls: ManagedWorktreeRemoverTestControls | undefined, diff --git a/main/services/subagents/subagent-phase3-contract.test.ts b/main/services/subagents/subagent-phase3-contract.test.ts index b57b1cf59..93be22dde 100644 --- a/main/services/subagents/subagent-phase3-contract.test.ts +++ b/main/services/subagents/subagent-phase3-contract.test.ts @@ -520,11 +520,12 @@ test("managed worktree deletion and terminal creation share workspace mutation a const revalidate = terminalService.indexOf("await revalidateAccess?.()"); const finalAbortCheck = terminalService.indexOf("if (ownerInvalidated())", revalidate); const spawn = terminalService.indexOf("const { pty,", finalAbortCheck); - const postSpawnCheck = terminalService.indexOf("if (ownerInvalidated())", spawn); assert.ok(revalidate >= 0); assert.ok(finalAbortCheck > revalidate); assert.ok(spawn > finalAbortCheck); - assert.ok(postSpawnCheck > spawn); + // Post-spawn invalidation is covered by terminal.test.ts with a live owner + // callback; this source-spelling check became stale when teardown was moved + // after PTY exit observation. }); test("managed worktree cleanup is root-bound, resumable, and packaged as signed native code", async () => { diff --git a/main/services/terminal.test.ts b/main/services/terminal.test.ts index 1dd4b748d..7ba7989da 100644 --- a/main/services/terminal.test.ts +++ b/main/services/terminal.test.ts @@ -46,14 +46,14 @@ function ownerState(documentId = "document-1", id = 42) { }; } -function fakePty() { +function fakePty(pid = 999_999) { let killed = false; let dataListener: ((data: string) => void) | undefined; let exitListener: ((event: { exitCode: number; signal?: number }) => void) | undefined; const writes: string[] = []; const resizes: Array<{ cols: number; rows: number }> = []; const pty = { - pid: 999_999, + pid, kill: () => { killed = true; }, @@ -82,6 +82,31 @@ function fakePty() { }; } +test("invalid PTY identities never signal a process group or bypass the removal hold", async () => { + for (const pid of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) { + const owner = ownerState(); + const child = fakePty(pid); + const signaled: number[] = []; + const service = new TerminalService({ + prepareSpawnHelper: async () => undefined, + spawnPty: (() => child.pty) as typeof spawn, + signalProcess: (target) => { signaled.push(target); }, + }); + await assert.rejects( + service.create("managed-workspace", "/tmp", owner.owner), + /terminal process identity could not be verified/u, + ); + assert.deepEqual(signaled, []); + assert.equal(child.killed(), false, "node-pty kill also signals its reported PID"); + for (let attempt = 0; attempt < 2; attempt += 1) { + await assert.rejects( + service.closeForWorkspaceAndWait("managed-workspace", 10), + /cannot safely remove it in this session/u, + ); + } + } +}); + test("renderer destruction during terminal revalidation prevents spawn", async () => { const owner = ownerState(); const revalidation = deferred(); @@ -126,6 +151,36 @@ test("renderer destruction at the synchronous spawn boundary kills the new PTY", ); assert.equal(child.killed(), true); assert.throws(() => service.workspaceId("missing", owner.owner), /unavailable/u); + child.emitExit(0); + await service.closeForWorkspaceAndWait("workspace-1", 10); +}); + +test("synchronous owner invalidation during terminal subscription still permits a later drain", async () => { + const stableOwner = ownerState(); + const child = fakePty(); + const owner: RendererDocumentOwner = { + ...stableOwner.owner, + onInvalidated: (listener) => { + listener(); + return () => undefined; + }, + }; + const service = new TerminalService({ + prepareSpawnHelper: async () => undefined, + spawnPty: (() => child.pty) as typeof spawn, + }); + + await assert.rejects( + service.create("workspace-1", "/tmp", owner), + /renderer document changed before the terminal could start/u, + ); + assert.equal(child.killed(), true); + await assert.rejects( + service.closeForWorkspaceAndWait("workspace-1", 10), + /terminal process is still exiting/u, + ); + child.emitExit(0); + await service.closeForWorkspaceAndWait("workspace-1", 10); }); test("renderer reload invalidates a pending terminal before helper preparation can spawn", async () => { @@ -780,6 +835,75 @@ test("terminal teardown is scoped by workspace and renderer web contents", async }); }); +test("managed worktree removal waits for owned terminal exit and reports a stuck shell", async () => { + const owner = ownerState(); + const child = fakePty(); + const service = new TerminalService({ + prepareSpawnHelper: async () => undefined, + spawnPty: (() => child.pty) as typeof spawn, + }); + await service.create("managed-workspace", "/tmp", owner.owner); + + let settled = false; + const closing = service.closeForWorkspaceAndWait("managed-workspace", 100) + .then(() => { settled = true; }); + assert.equal(child.killed(), true); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(settled, false, "signalling the PTY is not proof that it exited"); + child.emitExit(0); + await closing; + assert.equal(settled, true); + + const stuck = fakePty(); + const stuckService = new TerminalService({ + prepareSpawnHelper: async () => undefined, + spawnPty: (() => stuck.pty) as typeof spawn, + }); + await stuckService.create("managed-workspace", "/tmp", owner.owner); + await assert.rejects( + stuckService.closeForWorkspaceAndWait("managed-workspace", 10), + /terminal process is still exiting/u, + ); + await assert.rejects( + stuckService.closeForWorkspaceAndWait("managed-workspace", 10), + /terminal process is still exiting/u, + ); + stuck.emitExit(0); + await stuckService.closeForWorkspaceAndWait("managed-workspace", 10); +}); + +test("managed worktree removal refuses a surviving job in the PTY session after shell exit", async () => { + const owner = ownerState(); + const child = fakePty(); + let sessionHasSurvivor = true; + let inventoryAvailable = false; + const service = new TerminalService({ + prepareSpawnHelper: async () => undefined, + spawnPty: (() => child.pty) as typeof spawn, + // The job has a different PID and process group but retains the shell's + // POSIX session ID after the shell and controlling PTY disappear. + sessionHasMembers: async (sessionId) => { + assert.equal(sessionId, child.pty.pid); + if (!inventoryAvailable) throw new Error("process inventory unavailable"); + return sessionHasSurvivor; + }, + }); + await service.create("managed-workspace", "/tmp", owner.owner); + child.emitExit(0); + + await assert.rejects( + service.closeForWorkspaceAndWait("managed-workspace", 10), + /could not verify terminal jobs/u, + ); + inventoryAvailable = true; + await assert.rejects( + service.closeForWorkspaceAndWait("managed-workspace", 10), + /terminal job is still running/u, + ); + sessionHasSurvivor = false; + await service.closeForWorkspaceAndWait("managed-workspace", 10); +}); + test("terminal enforces the per-document session cap and history initialization order", async () => { const owner = ownerState(); const service = new TerminalService({ diff --git a/main/services/terminal.ts b/main/services/terminal.ts index 87ee4e2a4..7706ada41 100644 --- a/main/services/terminal.ts +++ b/main/services/terminal.ts @@ -10,6 +10,8 @@ import { spawn, type IPty } from "node-pty"; import type { RendererDocumentOwner } from "./renderer-document-owner.js"; import { resolveNodePtySpawnHelperPaths } from "./terminal-spawn-helper.js"; import { agentCommandEnvironment } from "./agent-command-environment.js"; +import { ManagedWorktreeProcessActiveError } from "./managed-worktree-cleanup-errors.js"; +import { managedWorktreeSessionHasMembers } from "./managed-worktree-remover.js"; const MAX_INPUT_CHARS = 64_000; const MAX_BUFFER_CHARS = 200_000; @@ -44,6 +46,8 @@ export interface TerminalSnapshot { interface TerminalSession extends TerminalSessionInfo { pty: IPty; + exited: Promise; + settleExit: () => void; ownerWebContentsId: number; ownerDocumentId: string; owner: RendererDocumentOwner; @@ -78,6 +82,10 @@ export interface TerminalServiceOptions { * app restart. Defaults to none (in-memory only) for tests and legacy paths. */ historyStore?: TerminalHistoryStoreLike; + /** POSIX session liveness seam for deletion tests. */ + sessionHasMembers?: (sessionId: number) => Promise; + /** Test seam for process-group signaling. */ + signalProcess?: (pid: number, signal: NodeJS.Signals) => void; } /** @@ -231,6 +239,9 @@ export class TerminalService { this.outputObserver = observer; } private readonly sessions = new Map(); + private readonly pendingWorkspaceExits = new Map>>(); + private readonly workspaceSessionIds = new Map>(); + private readonly unverifiableWorkspaces = new Set(); private readonly webContentsEpochs = new Map(); private spawnHelperReady: Promise | undefined; private historyStore: TerminalHistoryStoreLike | undefined; @@ -320,10 +331,25 @@ export class TerminalService { env: { ...agentCommandEnvironment(), TERM: "xterm-256color" } as Record, }, ); - if (ownerInvalidated()) { + if (!Number.isSafeInteger(pty.pid) || pty.pid <= 0) { + // There is no session identity to audit, and node-pty's kill() also + // signals its reported PID. With 0, either signal would hit our own + // process group. Keep a hard removal hold; a retry has no new proof. + this.unverifiableWorkspaces.add(workspaceId); this.terminatePty(pty); - throw new Error("The workspace changed before the terminal could start."); + throw new Error("The terminal process identity could not be verified."); } + let sessionIds = this.workspaceSessionIds.get(workspaceId); + if (!sessionIds) { + sessionIds = new Set(); + this.workspaceSessionIds.set(workspaceId, sessionIds); + } + // forkpty makes its shell the POSIX session leader. Job-control background + // groups retain that session ID even when the shell and PTY have exited. + sessionIds.add(pty.pid); + const invalidatedAtSpawnBoundary = ownerInvalidated(); + let settleExit: () => void = () => undefined; + const exited = new Promise((resolve) => { settleExit = resolve; }); const session: TerminalSession = { id, workspaceId, @@ -331,6 +357,8 @@ export class TerminalService { resolvedShell, preferredShellSkipped, pty, + exited, + settleExit, ownerWebContentsId: owner.id, ownerDocumentId: owner.documentId, owner, @@ -341,20 +369,24 @@ export class TerminalService { pendingOutput: "", }; this.sessions.set(id, session); - const removeOwnerInvalidation = owner.onInvalidated(() => { + // Observe exit before any owner callback can synchronously terminate the + // session. A retained pending exit must always have a way to settle. + pty.onExit(({ exitCode, signal }) => { + session.settleExit(); const current = this.sessions.get(id); - if (current === session) this.terminate(id, session); + if (current !== session) return; + // Deliver the final output before the exit notification. + this.flushOutput(id, current); + this.sessions.delete(id); + current.removeOwnerInvalidation(); + // Flush the final chunk before the session goes away. + void this.historyStore?.flush(workspaceId); + try { + owner.send("terminal:exit", { sessionId: id, exitCode, signal }); + } catch { + // The exact renderer document already disappeared. + } }); - session.removeOwnerInvalidation = removeOwnerInvalidation; - if (!this.sessions.has(id)) { - removeOwnerInvalidation(); - throw new Error("The renderer document changed before the terminal could start."); - } - if (ownerInvalidated()) { - this.terminate(id, session); - throw new Error("The renderer document changed before the terminal could start."); - } - pty.onData((data) => { const current = this.sessions.get(id); if (!current) return; @@ -377,21 +409,23 @@ export class TerminalService { current.outputTimer.unref?.(); } }); - pty.onExit(({ exitCode, signal }) => { + + let invalidatedDuringSubscription = false; + const removeOwnerInvalidation = owner.onInvalidated(() => { + invalidatedDuringSubscription = true; const current = this.sessions.get(id); - if (current !== session) return; - // Deliver the final output before the exit notification. - this.flushOutput(id, current); - this.sessions.delete(id); - current.removeOwnerInvalidation(); - // Flush the final chunk before the session goes away. - void this.historyStore?.flush(workspaceId); - try { - owner.send("terminal:exit", { sessionId: id, exitCode, signal }); - } catch { - // The exact renderer document already disappeared. - } + if (current === session) this.terminate(id, session); }); + session.removeOwnerInvalidation = removeOwnerInvalidation; + if (invalidatedDuringSubscription || ownerInvalidated()) { + if (this.sessions.get(id) === session) this.terminate(id, session); + removeOwnerInvalidation(); + throw new Error(invalidatedAtSpawnBoundary + ? "The workspace changed before the terminal could start." + : "The renderer document changed before the terminal could start."); + } + // A shell can exit while the owner subscription is being installed. + if (this.sessions.get(id) !== session) removeOwnerInvalidation(); return { id, workspaceId, cwd, resolvedShell, preferredShellSkipped }; } @@ -441,6 +475,55 @@ export class TerminalService { } } + /** Stop Aiden-owned shells and wait for their exit before removing a workspace. */ + async closeForWorkspaceAndWait(workspaceId: string, timeoutMs = 5_000): Promise { + const sessions = [...this.sessions].filter(([, session]) => session.workspaceId === workspaceId); + for (const [id, session] of sessions) this.terminate(id, session); + const pending = [...(this.pendingWorkspaceExits.get(workspaceId) ?? [])]; + if (this.unverifiableWorkspaces.has(workspaceId)) { + throw new ManagedWorktreeProcessActiveError( + "A terminal started without a verifiable process identity. Keep this worktree for manual inspection; Aiden cannot safely remove it in this session.", + ); + } + let timeout: ReturnType | undefined; + if (pending.length > 0) { + try { + await Promise.race([ + Promise.all(pending), + new Promise((_, reject) => { + timeout = setTimeout(() => reject(new ManagedWorktreeProcessActiveError()), timeoutMs); + }), + ]); + } finally { + if (timeout) clearTimeout(timeout); + } + } + const sessionIds = [...(this.workspaceSessionIds.get(workspaceId) ?? [])]; + if (sessionIds.length === 0) return; + for (const sessionId of sessionIds) { + let active: boolean; + try { + active = await (this.options.sessionHasMembers ?? managedWorktreeSessionHasMembers)(sessionId); + } catch { + throw new ManagedWorktreeProcessActiveError( + "Aiden could not verify terminal jobs. Keep this worktree and try again after checking its processes.", + ); + } + if (active) { + throw new ManagedWorktreeProcessActiveError( + "A terminal job is still running. Stop it, then retry worktree removal.", + ); + } + } + const currentIds = this.workspaceSessionIds.get(workspaceId); + if (currentIds && (currentIds.size !== sessionIds.length || sessionIds.some((id) => !currentIds.has(id)))) { + throw new ManagedWorktreeProcessActiveError( + "Terminal processes changed during cleanup. Retry worktree removal.", + ); + } + this.workspaceSessionIds.delete(workspaceId); + } + workspaceId(id: string, owner: RendererDocumentOwner): string { return this.getOwned(id, owner).workspaceId; } @@ -481,6 +564,16 @@ export class TerminalService { session.pendingOutput = ""; this.sessions.delete(id); session.removeOwnerInvalidation(); + let pending = this.pendingWorkspaceExits.get(session.workspaceId); + if (!pending) { + pending = new Set(); + this.pendingWorkspaceExits.set(session.workspaceId, pending); + } + pending.add(session.exited); + void session.exited.then(() => { + pending.delete(session.exited); + if (pending.size === 0) this.pendingWorkspaceExits.delete(session.workspaceId); + }); this.terminatePty(session.pty); // Best-effort flush so the last output chunk is on disk before the session // is torn down (window close, workspace switch, quit). Never block on it. @@ -497,12 +590,14 @@ export class TerminalService { } private terminatePty(pty: IPty): void { + const pid = pty.pid; + if (!Number.isSafeInteger(pid) || pid <= 0) return; // A PTY shell is normally its own process group. Signalling the group // cleans up ordinary foreground/background descendants as well as the // shell; deliberately detached processes retain normal Unix semantics. if (process.platform !== "win32") { try { - process.kill(-pty.pid, "SIGHUP"); + (this.options.signalProcess ?? process.kill)(-pid, "SIGHUP"); } catch { // The process may have exited between lookup and termination. } diff --git a/main/services/workspace-worktree-application-service-main.ts b/main/services/workspace-worktree-application-service-main.ts index 4f3c9a7e7..7a4435b03 100644 --- a/main/services/workspace-worktree-application-service-main.ts +++ b/main/services/workspace-worktree-application-service-main.ts @@ -123,7 +123,7 @@ export const workspaceWorktreeApplicationService = createWorkspaceWorktreeApplic workspaceIsChanging: (workspaceId) => workspaceMutationGate.isChanging(workspaceId), cancelWorkspaceOperations: (workspaceId, exceptSignal) => workspaceOperationRegistry.cancelAndSettle(workspaceId, { exceptSignal }), - closeWorkspaceTerminals: (workspaceId) => terminalService.closeForWorkspace(workspaceId), + closeWorkspaceTerminals: (workspaceId) => terminalService.closeForWorkspaceAndWait(workspaceId), cancelWorkspaceGeneration: (workspaceId) => llmClient.cancelWorkspaceAndSettle(workspaceId), cancelWorkspaceSchedules: (workspaceId) => scheduleService.cancelWorkspace(workspaceId), resumeWorkspaceSchedules: (workspaceId) => scheduleService.resumeWorkspace(workspaceId), diff --git a/main/services/workspace-worktree-application-service.test.ts b/main/services/workspace-worktree-application-service.test.ts index d1695d592..8ec474a62 100644 --- a/main/services/workspace-worktree-application-service.test.ts +++ b/main/services/workspace-worktree-application-service.test.ts @@ -23,6 +23,8 @@ test("shared managed-worktree workflow preserves creation rollback gates and des updatedAt: 2, }; let managed: Workspace | undefined; + let releaseTerminals: () => void = () => undefined; + let terminalDrain: Promise = Promise.resolve(); const signal = new AbortController().signal; const service = createWorkspaceWorktreeApplicationService({ environment: { @@ -114,7 +116,10 @@ test("shared managed-worktree workflow preserves creation rollback gates and des }, workspaceIsChanging: () => false, cancelWorkspaceOperations: async () => { events.push("cancel-operations"); }, - closeWorkspaceTerminals: () => { events.push("close-terminals"); }, + closeWorkspaceTerminals: async () => { + events.push("close-terminals"); + await terminalDrain; + }, cancelWorkspaceGeneration: async () => { events.push("cancel-generation"); }, cancelWorkspaceSchedules: async () => { events.push("cancel-schedules"); }, resumeWorkspaceSchedules: async () => { events.push("resume-schedules"); }, @@ -135,9 +140,19 @@ test("shared managed-worktree workflow preserves creation rollback gates and des ]); events.length = 0; - const removed = await service.remove(owner, created.id, (workspace) => { + terminalDrain = new Promise((resolve) => { releaseTerminals = resolve; }); + const removal = service.remove(owner, created.id, (workspace) => { events.push(`validate:${workspace.id}`); }); + await new Promise((resolve) => setImmediate(resolve)); + assert.deepEqual(events, [ + "validate:workspace-managed", + "begin-mutation", + "cancel-operations", + "close-terminals", + ], "Git deletion must wait for terminal exit"); + releaseTerminals(); + const removed = await removal; assert.equal(removed.branchDeleted, true); assert.deepEqual(events, [ "validate:workspace-managed", @@ -287,7 +302,7 @@ test("dirty removal snapshots before deletion, admits Git objects on the object beginWorkspaceMutation: () => () => undefined, workspaceIsChanging: () => false, cancelWorkspaceOperations: async () => undefined, - closeWorkspaceTerminals: () => undefined, + closeWorkspaceTerminals: async () => undefined, cancelWorkspaceGeneration: async () => undefined, cancelWorkspaceSchedules: async () => undefined, resumeWorkspaceSchedules: async () => undefined, diff --git a/main/services/workspace-worktree-application-service.ts b/main/services/workspace-worktree-application-service.ts index bc2843c71..46b25b152 100644 --- a/main/services/workspace-worktree-application-service.ts +++ b/main/services/workspace-worktree-application-service.ts @@ -140,7 +140,7 @@ export interface WorkspaceWorktreeApplicationDependencies { beginWorkspaceMutation(workspaceId: string): () => void; workspaceIsChanging(workspaceId: string): boolean; cancelWorkspaceOperations(workspaceId: string, exceptSignal: AbortSignal): Promise; - closeWorkspaceTerminals(workspaceId: string): void; + closeWorkspaceTerminals(workspaceId: string): Promise; cancelWorkspaceGeneration(workspaceId: string): Promise; cancelWorkspaceSchedules(workspaceId: string): Promise; resumeWorkspaceSchedules(workspaceId: string): Promise; @@ -464,7 +464,7 @@ export function createWorkspaceWorktreeApplicationService( ), }, async ({ keepPaused }) => { - dependencies.closeWorkspaceTerminals(workspaceId); + await dependencies.closeWorkspaceTerminals(workspaceId); await dependencies.cancelWorkspaceGeneration(workspaceId); await dependencies.cancelWorkspaceSchedules(workspaceId); diff --git a/native/worktree-remover/main.c b/native/worktree-remover/main.c index a6152910f..28948cd96 100644 --- a/native/worktree-remover/main.c +++ b/native/worktree-remover/main.c @@ -14,6 +14,9 @@ #include #include #include +#ifdef __APPLE__ +#include +#endif #define EXIT_IDENTITY_CHANGED 20 #define EXIT_MUTATION_DETECTED 21 @@ -33,6 +36,7 @@ #define MANIFEST_MAGIC_LENGTH 26 #define SHA256_HEX_LENGTH 64 #define MAX_CAPTURE_ATTEMPTS 32 +#define MAX_PROCESS_PIDS 262144 typedef struct EntryList EntryList; @@ -1419,6 +1423,122 @@ static void print_error(int code) { fputs("invalid_input\n", stderr); } +static int audit_pid_session(pid_t pid, pid_t expected_session) { + if (pid <= 0) return 0; + errno = 0; + pid_t actual_session = getsid(pid); + if (actual_session < 0) return errno == ESRCH ? 0 : -1; + return actual_session == expected_session ? 1 : 0; +} + +#ifndef __APPLE__ +static int proc_mount_is_complete(void) { + const char *mounts_path = "/proc/self/mounts"; +#ifdef AIDEN_REMOVER_TESTING + const char *test_mounts = getenv("AIDEN_REMOVER_TEST_MOUNTS"); + if (test_mounts != NULL && test_mounts[0] != '\0') mounts_path = test_mounts; +#endif + FILE *mounts = fopen(mounts_path, "r"); + if (!mounts) return 0; + int found = 0; + int complete = 1; + char line[8192]; + while (fgets(line, sizeof(line), mounts) != NULL) { + if (strchr(line, '\n') == NULL) { complete = 0; break; } + char *save = NULL; + char *source = strtok_r(line, " \t\n", &save); + char *target = strtok_r(NULL, " \t\n", &save); + char *type = strtok_r(NULL, " \t\n", &save); + char *options = strtok_r(NULL, " \t\n", &save); + char *dump = strtok_r(NULL, " \t\n", &save); + char *pass = strtok_r(NULL, " \t\n", &save); + if (!source || !target || !type || !options || !dump || !pass || + strtok_r(NULL, " \t\n", &save) != NULL) { complete = 0; break; } + if (strcmp(target, "/proc") != 0) continue; + found = 1; + if (strcmp(type, "proc") != 0) { complete = 0; break; } + char *option_save = NULL; + for (char *option = strtok_r(options, ",", &option_save); + option != NULL; option = strtok_r(NULL, ",", &option_save)) { + if (strncmp(option, "hidepid=", 8) == 0 && + strcmp(option + 8, "0") != 0 && + strcmp(option + 8, "off") != 0) { + complete = 0; + break; + } + if (strncmp(option, "pidns=", 6) == 0) { + complete = 0; + break; + } + } + if (!complete) break; + } + if (ferror(mounts)) complete = 0; + if (fclose(mounts) != 0) complete = 0; + if (!found || !complete) return 0; + // A procfs mounted for another PID namespace cannot enumerate our session. + char self_pid[32]; + ssize_t length = readlink("/proc/self", self_pid, sizeof(self_pid) - 1); + if (length <= 0 || length >= (ssize_t)sizeof(self_pid)) return 0; + self_pid[length] = '\0'; + uint64_t visible_pid = 0; + return parse_uint64(self_pid, &visible_pid) && visible_pid == (uint64_t)getpid(); +} +#endif + +static int audit_session_command(int argc, char **argv) { + uint64_t requested = 0; + if (argc != 3 || !parse_uint64(argv[2], &requested) || requested == 0 || + requested > INT_MAX) return EXIT_INVALID_INPUT; + pid_t expected_session = (pid_t)requested; + int found = 0; +#ifdef __APPLE__ + int needed = proc_listpids(PROC_ALL_PIDS, 0, NULL, 0); + if (needed <= 0 || needed > (int)(MAX_PROCESS_PIDS * sizeof(pid_t))) + return EXIT_IO_FAILED; + size_t capacity = (size_t)needed + 4096U * sizeof(pid_t); + if (capacity > MAX_PROCESS_PIDS * sizeof(pid_t)) return EXIT_IO_FAILED; + pid_t *pids = calloc(capacity / sizeof(pid_t), sizeof(pid_t)); + if (!pids) return EXIT_IO_FAILED; + int bytes = proc_listpids(PROC_ALL_PIDS, 0, pids, (int)capacity); + if (bytes <= 0 || (size_t)bytes >= capacity) { + free(pids); + return EXIT_IO_FAILED; + } + for (int index = 0; index < bytes / (int)sizeof(pid_t); index++) { + int observed = audit_pid_session(pids[index], expected_session); + if (observed < 0) { free(pids); return EXIT_IO_FAILED; } + if (observed > 0) { found = 1; break; } + } + free(pids); +#else + if (!proc_mount_is_complete()) return EXIT_IO_FAILED; + DIR *processes = opendir("/proc"); + if (!processes) return EXIT_IO_FAILED; + struct dirent *entry; + size_t count = 0; + for (;;) { + errno = 0; + entry = readdir(processes); + if (entry == NULL) { + if (errno != 0) { closedir(processes); return EXIT_IO_FAILED; } + break; + } + if (++count > MAX_PROCESS_PIDS) { closedir(processes); return EXIT_IO_FAILED; } + uint64_t value = 0; + if (!parse_uint64(entry->d_name, &value) || value == 0 || + value > INT_MAX) continue; + int observed = audit_pid_session((pid_t)value, expected_session); + if (observed < 0) { closedir(processes); return EXIT_IO_FAILED; } + if (observed > 0) { found = 1; break; } + } + if (closedir(processes) != 0) return EXIT_IO_FAILED; + if (!proc_mount_is_complete()) return EXIT_IO_FAILED; +#endif + if (fputs(found ? "active\n" : "empty\n", stdout) == EOF) return EXIT_IO_FAILED; + return 0; +} + static int finalize_manifest_command(int argc, char **argv) { if (argc != 8 || strcmp(argv[2], "--parent") != 0 || strcmp(argv[4], "--token") != 0 || strcmp(argv[6], "--digest") != 0 || @@ -1448,6 +1568,8 @@ static int finalize_manifest_command(int argc, char **argv) { } int main(int argc, char **argv) { + if (argc > 1 && strcmp(argv[1], "audit-session") == 0) + return audit_session_command(argc, argv); if (argc > 1 && strcmp(argv[1], "finalize-manifest") == 0) return finalize_manifest_command(argc, argv); if (argc != 12 || strcmp(argv[1], "remove") != 0 || diff --git a/packages/cli/src/remote-worktrees.ts b/packages/cli/src/remote-worktrees.ts index 927ce7569..ff09fe8b3 100644 --- a/packages/cli/src/remote-worktrees.ts +++ b/packages/cli/src/remote-worktrees.ts @@ -91,7 +91,8 @@ return createWorkspaceWorktreeApplicationService({ workspaceIsChanging: (workspaceId) => workspace.mutationGate.isChanging(workspaceId), cancelWorkspaceOperations: (workspaceId, exceptSignal) => workspace.operationRegistry.cancelAndSettle(workspaceId, { exceptSignal }), - closeWorkspaceTerminals: () => {}, + // The headless CLI has no renderer-owned PTYs to drain. + closeWorkspaceTerminals: async () => {}, cancelWorkspaceGeneration: (workspaceId) => cancelGeneration(workspaceId), cancelWorkspaceSchedules: (workspaceId) => scheduler.service.cancelWorkspace(workspaceId), resumeWorkspaceSchedules: (workspaceId) => scheduler.service.resumeWorkspace(workspaceId), diff --git a/prebuilt/native/darwin-universal/aiden-worktree-remover b/prebuilt/native/darwin-universal/aiden-worktree-remover index bcffeb460..460d7ed70 100755 Binary files a/prebuilt/native/darwin-universal/aiden-worktree-remover and b/prebuilt/native/darwin-universal/aiden-worktree-remover differ diff --git a/prebuilt/native/darwin-universal/manifest.json b/prebuilt/native/darwin-universal/manifest.json index c28189a61..b50cab278 100644 --- a/prebuilt/native/darwin-universal/manifest.json +++ b/prebuilt/native/darwin-universal/manifest.json @@ -3,8 +3,8 @@ "builtAt": "2026-10-16T03:49:05.251Z", "helpers": { "worktree-remover": { - "sha256": "72f46591d67614b8125feb58a0d8ac11e794ad83ae917076bb135a49396cb7be", - "source": "2d0dae7552acd391ea02189cbdcc016f04790032116e2a2c82cf15e6985178ca" + "sha256": "0425f2b33785009177a40bab2524f8db840fc7aa5be6da25d59401360b136fda", + "source": "bae1c9565939f0bbab15a5562685d17a41156733a0703ad0836e4b0294a45e48" }, "lease-flock": { "sha256": "a9dd128955b87813b235edf08e8fe23e3e307ed2300cf78f09ffb3523a8352be", diff --git a/prebuilt/native/linux-arm64/aiden-worktree-remover b/prebuilt/native/linux-arm64/aiden-worktree-remover index 79ccc1881..5ede508a7 100755 Binary files a/prebuilt/native/linux-arm64/aiden-worktree-remover and b/prebuilt/native/linux-arm64/aiden-worktree-remover differ diff --git a/prebuilt/native/linux-arm64/manifest.json b/prebuilt/native/linux-arm64/manifest.json index c45fb2180..2895273fd 100644 --- a/prebuilt/native/linux-arm64/manifest.json +++ b/prebuilt/native/linux-arm64/manifest.json @@ -3,8 +3,8 @@ "builtAt": "2026-10-16T03:49:21.713Z", "helpers": { "worktree-remover": { - "sha256": "1832482e431808d4fad7d2ce9f27b554caca716ffc44cdc43679976f367142e4", - "source": "2d0dae7552acd391ea02189cbdcc016f04790032116e2a2c82cf15e6985178ca" + "sha256": "90b3ae30a8ba9efcaf8977d3be3d4a8a08b5dbd31b172e8ad8e62c6ad283baa2", + "source": "bae1c9565939f0bbab15a5562685d17a41156733a0703ad0836e4b0294a45e48" }, "lease-flock": { "sha256": "c3d953b03ae58a28a714ac80576d070b07d52b1e5f02b30a2a7525675660bba1", diff --git a/prebuilt/native/linux-x64/aiden-worktree-remover b/prebuilt/native/linux-x64/aiden-worktree-remover index 184efe5f0..e1bdf6a60 100755 Binary files a/prebuilt/native/linux-x64/aiden-worktree-remover and b/prebuilt/native/linux-x64/aiden-worktree-remover differ diff --git a/prebuilt/native/linux-x64/manifest.json b/prebuilt/native/linux-x64/manifest.json index d8c40e1e3..1b519a2aa 100644 --- a/prebuilt/native/linux-x64/manifest.json +++ b/prebuilt/native/linux-x64/manifest.json @@ -3,8 +3,8 @@ "builtAt": "2026-10-16T03:49:16.518Z", "helpers": { "worktree-remover": { - "sha256": "0a26da641b1f74f1af4c40dce7d2405eff8771a705eb888a8d6366aac917d95a", - "source": "2d0dae7552acd391ea02189cbdcc016f04790032116e2a2c82cf15e6985178ca" + "sha256": "2164f941716de1ac9a8f0fd3e37c8c9524150d4e4e126c764919e1ca7f0543e8", + "source": "bae1c9565939f0bbab15a5562685d17a41156733a0703ad0836e4b0294a45e48" }, "lease-flock": { "sha256": "604c1d624a183a956c1bfdc5783a860000b183072518df9e53523472071ab770", diff --git a/scripts/worktree-remover.test.mjs b/scripts/worktree-remover.test.mjs index a5eef4a01..a63800ba8 100644 --- a/scripts/worktree-remover.test.mjs +++ b/scripts/worktree-remover.test.mjs @@ -1,7 +1,7 @@ /* global Buffer, process, setTimeout */ import assert from "node:assert/strict"; -import { spawn } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import { createHash } from "node:crypto"; import { access, @@ -20,10 +20,82 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const binary = path.join(repositoryRoot, "build", "native", "aiden-worktree-remover-test"); const productionBinary = path.join(repositoryRoot, "build", "native", "aiden-worktree-remover"); +const execFileAsync = promisify(execFile); + +test("native session audit sees a surviving member and clears after exit", async (t) => { + if (process.platform !== "darwin" && process.platform !== "linux") return; + const child = spawn("/bin/sleep", ["10"], { detached: true, stdio: "ignore" }); + t.after(() => { try { child.kill("SIGKILL"); } catch { /* Already reaped. */ } }); + assert.ok(Number.isSafeInteger(child.pid) && child.pid > 0); + const active = await execFileAsync(binary, ["audit-session", String(child.pid)], { encoding: "utf8" }); + assert.equal(active.stdout, "active\n"); + child.kill("SIGKILL"); + await new Promise((resolve) => child.once("exit", resolve)); + const empty = await execFileAsync(binary, ["audit-session", String(child.pid)], { encoding: "utf8" }); + assert.equal(empty.stdout, "empty\n"); +}); + +test("Linux session audit refuses a hidden process inventory instead of reporting empty", async (t) => { + if (process.platform !== "linux") return; + const absentSession = "2147483647"; + const root = await mkdtemp(path.join(os.tmpdir(), "aiden-proc-audit-")); + t.after(() => rm(root, { recursive: true, force: true })); + const mounts = path.join(root, "mounts"); + const hiddenOrIncomplete = [ + ...["2", "invisible", "1", "ptraceable"].map((hidepid) => + `proc /proc proc rw,relatime,hidepid=${hidepid} 0 0\n`), + "tmpfs /proc tmpfs rw 0 0\n", + "proc /other proc rw 0 0\n", + "proc /proc proc rw,pidns=/proc/1/ns/pid 0 0\n", + "proc /proc proc rw 0\n", + ]; + for (const fixture of hiddenOrIncomplete) { + await writeFile(mounts, fixture); + await assert.rejects( + execFileAsync(binary, ["audit-session", absentSession], { + encoding: "utf8", + env: { AIDEN_REMOVER_TEST_MOUNTS: mounts }, + }), + (error) => error.code === 22 && error.stdout === "", + ); + } + const member = spawn("/bin/sleep", ["10"], { detached: true, stdio: "ignore" }); + t.after(() => { try { member.kill("SIGKILL"); } catch { /* Already reaped. */ } }); + assert.ok(Number.isSafeInteger(member.pid) && member.pid > 0); + await writeFile(mounts, "proc /proc proc rw,relatime,hidepid=2 0 0\n"); + await assert.rejects( + execFileAsync(binary, ["audit-session", String(member.pid)], { + encoding: "utf8", + env: { AIDEN_REMOVER_TEST_MOUNTS: mounts }, + }), + (error) => error.code === 22 && error.stdout === "", + ); + member.kill("SIGKILL"); + await new Promise((resolve) => member.once("exit", resolve)); + for (const fixture of [ + "proc /proc proc rw,relatime,hidepid=off 0 0\n", + "proc /proc proc rw,relatime,hidepid=0 0 0\n", + "proc /proc proc rw,relatime 0 0\n", + ]) { + await writeFile(mounts, fixture); + const complete = await execFileAsync(binary, ["audit-session", absentSession], { + encoding: "utf8", + env: { AIDEN_REMOVER_TEST_MOUNTS: mounts }, + }); + assert.equal(complete.stdout, "empty\n"); + } + await writeFile(mounts, "proc /proc proc rw,relatime,hidepid=2 0 0\n"); + const production = await execFileAsync(productionBinary, ["audit-session", absentSession], { + encoding: "utf8", + env: { AIDEN_REMOVER_TEST_MOUNTS: mounts }, + }); + assert.equal(production.stdout, "empty\n", "production ignores fixture controls"); +}); async function fixture(t, name) { const root = await mkdtemp(path.join(os.tmpdir(), "aiden-remover-test-"));