diff --git a/.codex/skills/hack-cli/SKILL.md b/.codex/skills/hack-cli/SKILL.md index abe2e5dd..acae552d 100644 --- a/.codex/skills/hack-cli/SKILL.md +++ b/.codex/skills/hack-cli/SKILL.md @@ -70,7 +70,7 @@ Use `hack` as the primary interface for local-first development. ## Linked git worktrees - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. -- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; opt out with config `worktree.auto_branch=false` or an explicit `--branch`. +- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; a detached linked worktree requires an explicit `--branch`, unless config `worktree.auto_branch=false` explicitly opts into the base instance. - `hack doctor` flags divergent secret keys and dev_host collisions across checkouts. ## Advanced networking (extra_hosts + local proxies/tunnels) diff --git a/.cursor/rules/hack.mdc b/.cursor/rules/hack.mdc index acf4b028..23a2377b 100644 --- a/.cursor/rules/hack.mdc +++ b/.cursor/rules/hack.mdc @@ -34,7 +34,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is ## Linked git worktrees - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. -- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; opt out with config `worktree.auto_branch=false` or an explicit `--branch`. +- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; a detached linked worktree requires an explicit `--branch`, unless config `worktree.auto_branch=false` explicitly opts into the base instance. - `hack doctor` flags divergent secret keys and dev_host collisions across checkouts. ## Standard workflow diff --git a/AGENTS.md b/AGENTS.md index dc531c4e..1cbb89c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -261,7 +261,7 @@ Project files (managed vs generated): Linked git worktrees: - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. -- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; opt out with config `worktree.auto_branch=false` or an explicit `--branch`. +- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; a detached linked worktree requires an explicit `--branch`, unless config `worktree.auto_branch=false` explicitly opts into the base instance. - `hack doctor` flags divergent secret keys and dev_host collisions across checkouts. Advanced networking (extra_hosts + local proxies/tunnels): diff --git a/CLAUDE.md b/CLAUDE.md index 7a17fb43..9e77597a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,7 +131,7 @@ Project files (managed vs generated): Linked git worktrees: - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. -- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; opt out with config `worktree.auto_branch=false` or an explicit `--branch`. +- `hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; a detached linked worktree requires an explicit `--branch`, unless config `worktree.auto_branch=false` explicitly opts into the base instance. - `hack doctor` flags divergent secret keys and dev_host collisions across checkouts. Advanced networking (extra_hosts + local proxies/tunnels): diff --git a/docs/architecture.md b/docs/architecture.md index 3a88a05f..df76fad3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -122,6 +122,16 @@ Projects can run host-side hooks around `hack up/down` and start managed host pr local proxies/tunnels). Processes are started inside a mux session (tmux or zellij) so they have a stable home and can be torn down on `hack down`. +Lifecycle state is checkout-local at `.hack/.internal/lifecycle/state.json`. For tmux processes, the +saved process group belongs to the wrapped user command rather than its mux pane shell. Each compose +instance has its own entry, so starting a branch instance does not replace the base instance's entry +in the same checkout. Cleanup reconciles the saved process group with the live process table; if the group +leader exited while members remain, `hack doctor` reports the orphan. `hack down` terminates the +persisted leaderless group and descendant process groups only while a matching lifecycle mux session +still proves ownership; without that session, cleanup stays non-destructive because the numeric PGID +may have been reused. A live group leader without its saved pane PID is likewise not trusted as +lifecycle ownership. + For fixed-port helpers such as SSM/database/search tunnels, lifecycle config can also declare a `singleton` listener set. This lets Hack reuse an already-running equivalent helper or fail fast on partial conflicts instead of launching a competing duplicate supervisor. The intent is to reduce local @@ -267,6 +277,10 @@ the primary checkout through the shared git common dir (see "Project env + secre `hack doctor` checks for divergent secret keys and `dev_host` collisions across checkouts sharing the same repo. +If the linked worktree is detached, Hack refuses to silently target the base compose project because +there is no branch name from which to derive an isolated instance. Pass `--branch ` to select +one, or set `worktree.auto_branch=false` to opt into the base instance explicitly. + ## Files and directories - `~/.hack/` (default; override the whole root with `HACK_HOME`) diff --git a/docs/cli.md b/docs/cli.md index b910caa6..0ec25067 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -94,6 +94,10 @@ branch when no `--branch` is passed (`worktree.auto_branch`), so two checkouts n same hostnames. A one-line notice is printed to stderr when the default kicks in, so captured stdout stays clean. +A detached linked worktree has no branch name to derive, so these commands fail instead of silently +targeting the base instance. Pass `--branch ` to select an isolated instance, or set +`worktree.auto_branch` to `false` only when intentionally opting into the base instance. + Opt out: - pass `--branch ` explicitly (always wins), or diff --git a/docs/core.md b/docs/core.md index a4d9791b..2c9be846 100644 --- a/docs/core.md +++ b/docs/core.md @@ -31,7 +31,9 @@ alive. You do not need gateway, remote nodes, or extension setup to use this pat `HACK_NO_INTERACTIVE=1`) so scripted/agent runs never block on prompts. - In a linked git worktree, `hack up` defaults to a branch instance named after the worktree's git branch (`worktree.auto_branch`), and the secret key is inherited automatically from the primary - checkout via the shared git common dir. + checkout via the shared git common dir. A detached linked worktree has no branch name to derive; + pass `--branch `, or set `worktree.auto_branch=false` when intentionally targeting the base + instance. ## When to leave core diff --git a/docs/lifecycle.md b/docs/lifecycle.md index b50683b6..ed6cefa1 100644 --- a/docs/lifecycle.md +++ b/docs/lifecycle.md @@ -212,8 +212,8 @@ Lifecycle session name: Notes: - If no mux backend is available, lifecycle process startup fails with an actionable error. - Teardown is implemented by killing the lifecycle session; anything running inside that session will be stopped. -- For tmux-backed lifecycle sessions, Hack also persists pane PID and process-group metadata to - `.hack/.internal/lifecycle/state.json`, with per-hook/process output logged to +- For tmux-backed lifecycle sessions, Hack also persists the pane PID and the wrapped command's actual + process-group metadata to `.hack/.internal/lifecycle/state.json`, with per-hook/process output logged to `.hack/.internal/lifecycle/*.log`. If tmux pane state disappears before teardown, `hack down` still uses that persisted metadata to clean up any live lifecycle process groups instead of leaving orphaned host processes behind. - `hack doctor` reports stale lifecycle state when the persisted lifecycle entry no longer has a live mux session and points operators to `hack down` so cleanup and state removal happen through the supported path. `hack doctor --fix` does not tear down lifecycle sessions itself — use `hack down` for that. diff --git a/src/agents/instruction-source.ts b/src/agents/instruction-source.ts index f59b899c..67fb2a8d 100644 --- a/src/agents/instruction-source.ts +++ b/src/agents/instruction-source.ts @@ -122,7 +122,7 @@ export const INSTRUCTION_SECTIONS: readonly InstructionSection[] = [ surfaces: ALL_SURFACES, bullets: [ "Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments.", - "`hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; opt out with config `worktree.auto_branch=false` or an explicit `--branch`.", + "`hack up` in a linked worktree defaults to a branch instance named after the worktree's git branch; a detached linked worktree requires an explicit `--branch`, unless config `worktree.auto_branch=false` explicitly opts into the base instance.", "`hack doctor` flags divergent secret keys and dev_host collisions across checkouts.", ], }, diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 6196e2a4..8ef9c566 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1845,7 +1845,10 @@ async function checkProjectLifecycleHygiene({ const inspection = await inspectProjectLifecycleHygiene({ projectDir: ctx.projectDir, }); - if (inspection.staleEntries.length === 0) { + if ( + inspection.staleEntries.length === 0 && + inspection.orphanedProcessGroups.length === 0 + ) { return { name: "lifecycle hygiene", status: "ok", @@ -1854,7 +1857,7 @@ async function checkProjectLifecycleHygiene({ }; } - const orphanedGroups = new Set(); + const orphanedGroups = new Set(inspection.orphanedProcessGroups); for (const entry of inspection.staleEntries) { for (const group of entry.liveProcessGroups) { orphanedGroups.add(group); @@ -1862,7 +1865,11 @@ async function checkProjectLifecycleHygiene({ } const details = [ - `${inspection.staleEntries.length} stale lifecycle state entr${inspection.staleEntries.length === 1 ? "y" : "ies"}`, + ...(inspection.staleEntries.length > 0 + ? [ + `${inspection.staleEntries.length} stale lifecycle state entr${inspection.staleEntries.length === 1 ? "y" : "ies"}`, + ] + : []), ...(orphanedGroups.size > 0 ? [ `${orphanedGroups.size} orphaned lifecycle process group${orphanedGroups.size === 1 ? "" : "s"}`, diff --git a/src/commands/project.ts b/src/commands/project.ts index d4f2761a..79f84f0c 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -1,3 +1,4 @@ +import { rm } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { autocompleteMultiselect, @@ -210,6 +211,9 @@ const LABELS_LINE_PATTERN = /^(\s*)labels:\s*$/; /** Regex to extract leading whitespace (indentation). */ const INDENT_PATTERN = /^(\s*)/; +const LIFECYCLE_COMMAND_PID_WAIT_ATTEMPTS = 50; +const LIFECYCLE_COMMAND_PID_WAIT_INTERVAL_MS = 20; + /** Regex to match caddy label line in YAML. */ const CADDY_LABEL_PATTERN = /^(\s*)caddy:\s*(.*)$/; @@ -594,6 +598,11 @@ async function resolveEffectiveBranchForCommand(opts: { projectRoot: opts.project.projectRoot, autoBranchEnabled: resolveWorktreeAutoBranch(cfg), }); + if (resolved.source === "detached-worktree") { + throw new CliUsageError( + "Detached linked worktree cannot auto-select an isolated instance. Pass --branch , or set worktree.auto_branch=false to target the base instance explicitly." + ); + } if (resolved.source === "worktree" && resolved.branch) { const message = `Linked worktree detected → using branch instance "${resolved.branch}" (override with --branch , disable with worktree.auto_branch=false)`; if (opts.noticeToStderr === true) { @@ -702,6 +711,13 @@ function emitLifecycleResult(opts: { return opts.exitCode; } +function buildLifecycleJsonErrorResult(opts: { readonly error: unknown }) { + if (opts.error instanceof CliUsageError) { + return errorResult({ code: "E_USAGE", message: opts.error.message }); + } + return errorResultFromUnknown({ error: opts.error }); +} + /** * Envelope for lifecycle actions that were routed to a remote node — the * remote runner owns per-service detail, so the payload reports the routing @@ -1969,13 +1985,16 @@ async function startLifecycleProcess(opts: { projectDir: opts.projectDir, composeProject: opts.composeProject, }); - const wrappedCommand = wrapLifecyclePersistentCommand({ - command: opts.process.command, - logPath, - serviceName: opts.process.name, - }); - if (opts.backend === "tmux") { + const commandPidPath = `${logPath}.${windowName}.pid`; + await ensureDir(dirname(commandPidPath)); + await rm(commandPidPath, { force: true }); + const wrappedCommand = wrapLifecyclePersistentCommand({ + command: opts.process.command, + commandPidPath, + logPath, + serviceName: opts.process.name, + }); const result = await exec( [ "tmux", @@ -1993,6 +2012,7 @@ async function startLifecycleProcess(opts: { { stdin: "ignore" } ); if (result.exitCode !== 0) { + await rm(commandPidPath, { force: true }); throw new Error( `Failed to start lifecycle process "${opts.process.name}": ${result.stderr.trim()}` ); @@ -2002,10 +2022,17 @@ async function startLifecycleProcess(opts: { windowName, }); const panePid = panePids[0]; - const processGroupId = - panePid !== undefined - ? await readProcessGroupIdForPid({ pid: panePid }) - : null; + let processGroupId: number | null = null; + try { + processGroupId = await waitForLifecycleCommandProcessGroupId({ + commandPidPath, + }); + } finally { + await rm(commandPidPath, { force: true }); + } + if (!processGroupId && panePid !== undefined) { + processGroupId = await readProcessGroupIdForPid({ pid: panePid }); + } await appendLifecycleLogRecord({ projectDir: opts.projectDir, composeProject: opts.composeProject, @@ -2025,6 +2052,12 @@ async function startLifecycleProcess(opts: { }; } + const wrappedCommand = wrapLifecyclePersistentCommand({ + command: opts.process.command, + commandPidPath: null, + logPath, + serviceName: opts.process.name, + }); const result = await exec( ["zellij", "run", "--", "sh", "-c", wrappedCommand], { @@ -2184,6 +2217,29 @@ async function readProcessGroupIdForPid(opts: { return Number.isInteger(parsed) && parsed > 0 ? parsed : null; } +async function waitForLifecycleCommandProcessGroupId(opts: { + readonly commandPidPath: string; +}): Promise { + for ( + let attempt = 0; + attempt < LIFECYCLE_COMMAND_PID_WAIT_ATTEMPTS; + attempt += 1 + ) { + const commandPidRaw = await readTextFile(opts.commandPidPath); + const commandPid = Number.parseInt(commandPidRaw?.trim() ?? "", 10); + if (Number.isInteger(commandPid) && commandPid > 1) { + const processGroupId = await readProcessGroupIdForPid({ + pid: commandPid, + }); + if (processGroupId) { + return processGroupId; + } + } + await Bun.sleep(LIFECYCLE_COMMAND_PID_WAIT_INTERVAL_MS); + } + return null; +} + async function resolveLifecycleProcessGroupIds(opts: { readonly sessionName: string; readonly lifecycleEntry: LifecycleStateEntry | null; @@ -2262,16 +2318,22 @@ function resolveLifecycleCommandServiceName(opts: { export function wrapLifecyclePersistentCommand(opts: { readonly command: string; + readonly commandPidPath: string | null; readonly logPath: string; readonly serviceName: string; }): string { const logPath = shellSingleQuote(opts.logPath); const service = shellSingleQuote(opts.serviceName); const command = shellSingleQuote(opts.command); + const commandPidPath = + opts.commandPidPath === null ? null : shellSingleQuote(opts.commandPidPath); return [ `HACK_LIFECYCLE_LOG=${logPath}`, `HACK_LIFECYCLE_SERVICE=${service}`, `HACK_LIFECYCLE_COMMAND=${command}`, + ...(commandPidPath + ? [`HACK_LIFECYCLE_COMMAND_PID_FILE=${commandPidPath}`] + : []), `fifo="$(mktemp -u "\${TMPDIR:-/tmp}/hack-lifecycle.XXXXXX")"`, 'mkfifo "$fifo"', "cleanup_lifecycle() {", @@ -2289,6 +2351,7 @@ export function wrapLifecyclePersistentCommand(opts: { ` if [ -n "\${reader_pid:-}" ]; then`, ' wait "$reader_pid" 2>/dev/null || true', " fi", + ...(commandPidPath ? [' rm -f "$HACK_LIFECYCLE_COMMAND_PID_FILE"'] : []), ' rm -f "$fifo"', "}", 'trap "cleanup_lifecycle; exit 130" INT TERM HUP', @@ -2299,11 +2362,21 @@ export function wrapLifecyclePersistentCommand(opts: { ' done < "$fifo" ) &', "reader_pid=$!", "if command -v python3 >/dev/null 2>&1; then", - ' python3 -c \'import os, sys; os.setsid(); os.execvp("sh", ["sh", "-c", sys.argv[1]])\' "$HACK_LIFECYCLE_COMMAND" >"$fifo" 2>&1 &', + ...(commandPidPath + ? [ + ' python3 -c \'import os, sys; os.setsid(); pid_file = open(sys.argv[2], "w"); pid_file.write(str(os.getpid())); pid_file.close(); os.execvp("sh", ["sh", "-c", sys.argv[1]])\' "$HACK_LIFECYCLE_COMMAND" "$HACK_LIFECYCLE_COMMAND_PID_FILE" >"$fifo" 2>&1 &', + ] + : [ + ' python3 -c \'import os, sys; os.setsid(); os.execvp("sh", ["sh", "-c", sys.argv[1]])\' "$HACK_LIFECYCLE_COMMAND" >"$fifo" 2>&1 &', + ]), + " cmd_pid=$!", "else", ' sh -c "$HACK_LIFECYCLE_COMMAND" >"$fifo" 2>&1 &', + " cmd_pid=$!", + ...(commandPidPath + ? [' printf "%s\\n" "$cmd_pid" > "$HACK_LIFECYCLE_COMMAND_PID_FILE"'] + : []), "fi", - "cmd_pid=$!", 'wait "$cmd_pid"', "cmd_status=$?", 'cmd_pid=""', @@ -5237,7 +5310,7 @@ async function handleUp({ return await runUpCommand({ ctx, args, json: true, startedAtMs }); } catch (error: unknown) { return emitLifecycleResult({ - result: errorResultFromUnknown({ error }), + result: buildLifecycleJsonErrorResult({ error }), exitCode: 1, }); } @@ -5562,7 +5635,7 @@ async function handleDown({ }); } return emitLifecycleResult({ - result: errorResultFromUnknown({ error }), + result: buildLifecycleJsonErrorResult({ error }), exitCode: 1, }); } @@ -5960,7 +6033,7 @@ async function handleRestart({ }); } return emitLifecycleResult({ - result: errorResultFromUnknown({ error }), + result: buildLifecycleJsonErrorResult({ error }), exitCode: 1, }); } diff --git a/src/lib/branches.ts b/src/lib/branches.ts index 4b4a5b62..1a858db3 100644 --- a/src/lib/branches.ts +++ b/src/lib/branches.ts @@ -160,7 +160,11 @@ export async function touchBranchUsage(opts: { return { updated: true, created, path: read.path }; } -export type EffectiveBranchSource = "explicit" | "worktree" | "none"; +export type EffectiveBranchSource = + | "explicit" + | "worktree" + | "detached-worktree" + | "none"; export type EffectiveBranchResolution = { /** Branch instance slug to target, or null for the base instance. */ @@ -178,8 +182,10 @@ export type EffectiveBranchResolution = { * - otherwise, in a LINKED git worktree with `autoBranchEnabled` (config * `worktree.auto_branch`, default true), default to the sanitized current * git branch name (`source: "worktree"`) - * - primary checkouts, non-git paths, detached HEAD, and opted-out projects - * resolve to the base instance (`branch: null`, `source: "none"`) + * - a detached linked worktree is reported separately so callers can refuse + * an implicit base instance instead of colliding with the primary checkout + * - primary checkouts, non-git paths, and opted-out projects resolve to the + * base instance (`branch: null`, `source: "none"`) */ export async function resolveEffectiveBranch(opts: { readonly explicitBranch: string | null; @@ -206,7 +212,11 @@ export async function resolveEffectiveBranch(opts: { repoRoot: opts.projectRoot, }); if (!gitBranch) { - return { branch: null, source: "none", gitBranch: null }; + return { + branch: null, + source: "detached-worktree", + gitBranch: null, + }; } const slug = sanitizeBranchSlug(gitBranch); diff --git a/src/lib/project-lifecycle-hygiene.ts b/src/lib/project-lifecycle-hygiene.ts index f64a3ca5..1b2a65a6 100644 --- a/src/lib/project-lifecycle-hygiene.ts +++ b/src/lib/project-lifecycle-hygiene.ts @@ -7,6 +7,7 @@ import { readLifecycleState } from "./lifecycle-runtime.ts"; import { type ProcessSnapshotRow, readProcessSnapshot, + resolveLeaderlessPersistedLifecycleProcessGroupIds, resolvePersistedLifecycleProcessGroupIds, } from "./project-lifecycle-processes.ts"; @@ -17,6 +18,7 @@ export type StaleLifecycleStateEntry = { export type LifecycleHygieneInspection = { readonly staleEntries: readonly StaleLifecycleStateEntry[]; + readonly orphanedProcessGroups: readonly number[]; }; /** Inspect persisted lifecycle state for entries whose mux session is gone. */ @@ -25,7 +27,7 @@ export async function inspectProjectLifecycleHygiene(opts: { }): Promise { const entries = await readLifecycleState({ projectDir: opts.projectDir }); if (entries.length === 0) { - return { staleEntries: [] }; + return { staleEntries: [], orphanedProcessGroups: [] }; } const [sessionsByBackend, snapshot] = await Promise.all([ @@ -50,11 +52,20 @@ export function inspectLifecycleStateEntries(opts: { readonly snapshot: readonly ProcessSnapshotRow[]; }): LifecycleHygieneInspection { const staleEntries: StaleLifecycleStateEntry[] = []; + const orphanedProcessGroups = new Set(); for (const entry of opts.entries) { const sessions = opts.sessionsByBackend.get(entry.backend); const sessionPresent = sessions?.has(entry.sessionName) ?? false; if (sessionPresent) { + const leaderlessGroups = + resolveLeaderlessPersistedLifecycleProcessGroupIds({ + lifecycleEntry: entry, + snapshot: opts.snapshot, + }); + for (const processGroupId of leaderlessGroups) { + orphanedProcessGroups.add(processGroupId); + } continue; } @@ -67,7 +78,12 @@ export function inspectLifecycleStateEntries(opts: { }); } - return { staleEntries }; + return { + staleEntries, + orphanedProcessGroups: [...orphanedProcessGroups].sort( + (left, right) => left - right + ), + }; } async function listSessionsByBackend(): Promise< diff --git a/src/lib/project-lifecycle-processes.ts b/src/lib/project-lifecycle-processes.ts index a5473fd2..38045976 100644 --- a/src/lib/project-lifecycle-processes.ts +++ b/src/lib/project-lifecycle-processes.ts @@ -103,10 +103,55 @@ export function resolveLifecycleProcessGroupIdsForTmuxState(opts: { } } - return collectDescendantProcessGroupIds({ + const discoveredGroups = collectDescendantProcessGroupIds({ snapshot: opts.snapshot, rootPids: [...rootPids], }); + const leaderlessGroups = resolveLeaderlessPersistedLifecycleProcessGroupIds({ + lifecycleEntry: opts.lifecycleEntry, + snapshot: opts.snapshot, + }); + return [...new Set([...discoveredGroups, ...leaderlessGroups])].sort( + (left, right) => left - right + ); +} + +/** Recover persisted groups whose original leader is gone but members remain. */ +export function resolveLeaderlessPersistedLifecycleProcessGroupIds(opts: { + readonly lifecycleEntry: LifecycleStateEntry | null; + readonly snapshot: readonly ProcessSnapshotRow[]; +}): number[] { + const groups = new Set(); + + for (const processInfo of opts.lifecycleEntry?.processes ?? []) { + const processGroupId = processInfo.processGroupId; + if (!(processGroupId && processGroupId > 1)) { + continue; + } + const persistedPaneStillExists = + processInfo.panePid !== undefined && + opts.snapshot.some((row) => row.pid === processInfo.panePid); + const groupLeaderStillExists = opts.snapshot.some( + (row) => row.pid === processGroupId + ); + const groupMemberPids = opts.snapshot + .filter((row) => row.processGroupId === processGroupId) + .map((row) => row.pid); + if ( + !(persistedPaneStillExists || groupLeaderStillExists) && + groupMemberPids.length > 0 + ) { + const descendantGroups = collectDescendantProcessGroupIds({ + snapshot: opts.snapshot, + rootPids: groupMemberPids, + }); + for (const descendantGroup of descendantGroups) { + groups.add(descendantGroup); + } + } + } + + return [...groups].sort((left, right) => left - right); } /** Recover live lifecycle process groups from persisted metadata when mux panes are gone. */ @@ -121,7 +166,7 @@ export function resolvePersistedLifecycleProcessGroupIds(opts: { }); } -/** Only trust persisted lifecycle pane metadata when a matching mux session was observed live. */ +/** Only trust persisted lifecycle metadata when a matching mux session was observed live. */ export function resolveLifecycleStopProcessGroupIds(opts: { readonly matchedLiveSession: boolean; readonly lifecycleEntry: LifecycleStateEntry | null; diff --git a/tests/agent-instruction-source.test.ts b/tests/agent-instruction-source.test.ts index bc9b7ffc..dfddb77e 100644 --- a/tests/agent-instruction-source.test.ts +++ b/tests/agent-instruction-source.test.ts @@ -138,6 +138,7 @@ test("linked worktree guidance is present in every surface", () => { for (const rendered of Object.values(RENDERED_SURFACES)) { expect(rendered).toContain("Linked git worktrees"); expect(rendered).toContain("HACK_ENV_SECRET_KEY"); + expect(rendered).toContain("detached linked worktree"); expect(rendered).toContain("worktree.auto_branch=false"); } }); diff --git a/tests/branches-effective-branch.test.ts b/tests/branches-effective-branch.test.ts index 1c7b3ccb..03887783 100644 --- a/tests/branches-effective-branch.test.ts +++ b/tests/branches-effective-branch.test.ts @@ -107,7 +107,7 @@ test("primary checkout resolves to the base instance", async () => { expect(resolved.branch).toBeNull(); }); -test("detached HEAD in a linked worktree resolves to the base instance", async () => { +test("detached HEAD in a linked worktree refuses an implicit base instance", async () => { const fixture = await createFixture({ branch: "feature/detach-me" }); await runGit(["checkout", "--detach"], fixture.linkedRoot); @@ -117,6 +117,20 @@ test("detached HEAD in a linked worktree resolves to the base instance", async ( autoBranchEnabled: true, }); + expect(resolved.source).toBe("detached-worktree"); + expect(resolved.branch).toBeNull(); +}); + +test("worktree.auto_branch=false explicitly allows the base instance from detached HEAD", async () => { + const fixture = await createFixture({ branch: "feature/detached-opt-out" }); + await runGit(["checkout", "--detach"], fixture.linkedRoot); + + const resolved = await resolveEffectiveBranch({ + explicitBranch: null, + projectRoot: fixture.linkedRoot, + autoBranchEnabled: false, + }); + expect(resolved.source).toBe("none"); expect(resolved.branch).toBeNull(); }); diff --git a/tests/e2e/scenarios/lifecycle-host-process.ts b/tests/e2e/scenarios/lifecycle-host-process.ts index 32200f14..1ed279a0 100644 --- a/tests/e2e/scenarios/lifecycle-host-process.ts +++ b/tests/e2e/scenarios/lifecycle-host-process.ts @@ -1,22 +1,33 @@ import { join } from "node:path"; import { createMonorepoFixture } from "../fixture.ts"; -import { expect, expectExit, type Scenario } from "../harness.ts"; +import { expect, expectExit, runCommand, type Scenario } from "../harness.ts"; import { downBestEffort, requireDockerPreconditions } from "./docker-shared.ts"; const UP_TIMEOUT_MS = 420_000; +type LifecycleStateFile = { + readonly entries?: readonly { + readonly processes?: readonly { + readonly name?: string; + readonly panePid?: number; + readonly processGroupId?: number; + }[]; + }[]; +}; + /** * Lifecycle host hooks run as part of `hack up` (there is no standalone * lifecycle start/stop command today), so this scenario lives in the docker * tier: `hack up --detach` must execute the configured lifecycle.up.before * hook on the host (observed via a marker file) before starting services, - * and record lifecycle runtime state under .hack/.internal/lifecycle/. + * record lifecycle runtime state under .hack/.internal/lifecycle/, and retain + * the wrapped command's actual process group for teardown. */ export const lifecycleHostProcessScenario: Scenario = { name: "lifecycle-host-process", tier: "docker", - summary: "lifecycle.up.before host hook runs during hack up", + summary: "lifecycle hooks and command-group teardown run during up/down", run: async (ctx) => { await requireDockerPreconditions({ ctx }); @@ -24,7 +35,10 @@ export const lifecycleHostProcessScenario: Scenario = { const fixture = await createMonorepoFixture({ parentDir: ctx.tempRoot, withHackConfig: true, - lifecycle: { upBeforeMarkerFile: markerFile }, + lifecycle: { + persistentProcess: true, + upBeforeMarkerFile: markerFile, + }, }); try { @@ -54,12 +68,57 @@ export const lifecycleHostProcessScenario: Scenario = { const stateFile = Bun.file( join(fixture.hackDir, ".internal", "lifecycle", "state.json") ); - const hasState = await stateFile.exists(); - ctx.log( - hasState - ? "lifecycle state recorded at .hack/.internal/lifecycle/state.json" - : "note: no lifecycle state.json written (no persistent processes configured)" + expect({ + that: await stateFile.exists(), + message: "persistent lifecycle process did not write lifecycle state", + result: up, + }); + const state = (await stateFile.json()) as LifecycleStateFile; + const sleeper = state.entries?.[0]?.processes?.find( + (process) => process.name === "e2e-sleeper" ); + const panePid = sleeper?.panePid ?? 0; + const processGroupId = sleeper?.processGroupId ?? 0; + expect({ + that: + Number.isInteger(panePid) && + panePid > 1 && + Number.isInteger(processGroupId) && + processGroupId > 1 && + processGroupId !== panePid, + message: + "lifecycle state should persist the wrapped command group, not the tmux pane group", + result: up, + }); + + const groupLeader = await runCommand({ + argv: ["ps", "-o", "pgid=", "-p", String(processGroupId)], + cwd: fixture.root, + }); + expectExit({ + result: groupLeader, + codes: [0], + message: "persisted lifecycle command group leader should be live", + }); + expect({ + that: Number.parseInt(groupLeader.stdout.trim(), 10) === processGroupId, + message: + "persisted lifecycle PGID does not match the live command group", + result: groupLeader, + }); + + await downBestEffort({ ctx, fixture }); + let commandGroupStillExists = true; + try { + process.kill(-processGroupId, 0); + } catch { + commandGroupStillExists = false; + } + expect({ + that: !commandGroupStillExists, + message: "hack down left the lifecycle command process group alive", + result: up, + }); } finally { await downBestEffort({ ctx, fixture }); } diff --git a/tests/e2e/scenarios/worktree-branch-default.ts b/tests/e2e/scenarios/worktree-branch-default.ts index a20195a1..00813168 100644 --- a/tests/e2e/scenarios/worktree-branch-default.ts +++ b/tests/e2e/scenarios/worktree-branch-default.ts @@ -3,6 +3,7 @@ import { expect, expectExit, extractJsonObject, + runCommand, type Scenario, } from "../harness.ts"; @@ -96,5 +97,52 @@ export const worktreeBranchDefaultScenario: Scenario = { message: `explicit --branch should win over the worktree default, got "${optOutUrl}"`, result: optOut, }); + + const detach = await runCommand({ + argv: ["git", "checkout", "--detach"], + cwd: worktreePath, + }); + expectExit({ + result: detach, + codes: [0], + message: "failed to detach the linked worktree fixture", + }); + + const detachedOpen = await ctx.cli({ + args: ["open", "--json"], + cwd: worktreePath, + }); + expectExit({ + result: detachedOpen, + codes: [1], + message: "detached linked worktree must refuse an implicit base instance", + }); + expect({ + that: + detachedOpen.combined.includes("Detached linked worktree") && + detachedOpen.combined.includes("--branch "), + message: + "detached worktree failure must explain the safe explicit override", + result: detachedOpen, + }); + + const detachedExplicit = await ctx.cli({ + args: ["open", "--json", "--branch", "detached-review"], + cwd: worktreePath, + }); + expectExit({ + result: detachedExplicit, + codes: [0], + message: + "explicit branch should remain available from detached worktrees", + }); + const detachedExplicitUrl = + extractJsonObject({ text: detachedExplicit.stdout })?.url ?? + ""; + expect({ + that: detachedExplicitUrl.includes("detached-review"), + message: `explicit detached branch URL should carry the requested slug, got "${detachedExplicitUrl}"`, + result: detachedExplicit, + }); }, }; diff --git a/tests/lifecycle-json.test.ts b/tests/lifecycle-json.test.ts index 64a89f2b..1e6b2d97 100644 --- a/tests/lifecycle-json.test.ts +++ b/tests/lifecycle-json.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -116,6 +116,78 @@ test("usage errors under --json emit an E_USAGE envelope instead of help text", expect(result.stdout).not.toContain("Usage:"); }); +for (const action of ["up", "down", "restart"] as const) { + test(`${action} --json preserves E_USAGE for a detached linked worktree`, async () => { + const worktreeRoot = await createDetachedWorktreeFixture(); + const result = await runCliWithCapturedOutput([ + action, + "--json", + "--path", + worktreeRoot, + ]); + + expect(result.exitCode).toBe(1); + const parsed = JSON.parse(result.stdout) as { + ok: boolean; + error?: { code: string; message: string }; + }; + expect(parsed.ok).toBe(false); + expect(parsed.error?.code).toBe("E_USAGE"); + expect(parsed.error?.message).toContain("Detached linked worktree"); + expect(parsed.error?.message).toContain("--branch "); + }); +} + +async function createDetachedWorktreeFixture(): Promise { + if (!tempDir) { + throw new Error("Missing temp directory"); + } + + const primaryRoot = join(tempDir, "repo-primary"); + const hackDir = join(primaryRoot, ".hack"); + await mkdir(hackDir, { recursive: true }); + await writeFile( + join(hackDir, "hack.config.json"), + `${JSON.stringify({ name: "json-worktree", dev_host: "json-worktree.hack" }, null, 2)}\n` + ); + await writeFile( + join(hackDir, "docker-compose.yml"), + ["services:", " api:", " image: alpine:3.20", ""].join("\n") + ); + await writeFile(join(primaryRoot, "README.md"), "# fixture\n"); + + runGit({ cwd: primaryRoot, args: ["init", "-b", "main"] }); + runGit({ + cwd: primaryRoot, + args: ["config", "user.email", "test@example.com"], + }); + runGit({ cwd: primaryRoot, args: ["config", "user.name", "Test User"] }); + runGit({ cwd: primaryRoot, args: ["add", "."] }); + runGit({ cwd: primaryRoot, args: ["commit", "-m", "init"] }); + + const worktreeRoot = join(tempDir, "repo-worktree"); + runGit({ + cwd: primaryRoot, + args: ["worktree", "add", "-b", "feature/json", worktreeRoot], + }); + runGit({ cwd: worktreeRoot, args: ["checkout", "--detach"] }); + return worktreeRoot; +} + +function runGit(opts: { + readonly cwd: string; + readonly args: readonly string[]; +}): void { + const result = Bun.spawnSync(["git", "-C", opts.cwd, ...opts.args], { + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + if (result.exitCode !== 0) { + throw new Error(Buffer.from(result.stderr).toString("utf8")); + } +} + async function runCliWithCapturedOutput( args: readonly string[] ): Promise { diff --git a/tests/project-lifecycle-hygiene.test.ts b/tests/project-lifecycle-hygiene.test.ts index 4cd98bf4..8793515c 100644 --- a/tests/project-lifecycle-hygiene.test.ts +++ b/tests/project-lifecycle-hygiene.test.ts @@ -29,6 +29,7 @@ test("inspectLifecycleStateEntries ignores lifecycle entries with a live mux ses }); expect(inspection.staleEntries).toEqual([]); + expect(inspection.orphanedProcessGroups).toEqual([]); }); test("inspectLifecycleStateEntries reports stale lifecycle entries and orphaned groups", () => { @@ -47,4 +48,20 @@ test("inspectLifecycleStateEntries reports stale lifecycle entries and orphaned liveProcessGroups: [500, 501], }, ]); + expect(inspection.orphanedProcessGroups).toEqual([]); +}); + +test("inspectLifecycleStateEntries reports leaderless groups behind a live mux session", () => { + const inspection = inspectLifecycleStateEntries({ + entries: [staleEntry], + sessionsByBackend: new Map([["tmux", new Set([staleEntry.sessionName])]]), + snapshot: [ + { pid: 501, ppid: 1, processGroupId: 500 }, + { pid: 502, ppid: 501, processGroupId: 500 }, + { pid: 503, ppid: 502, processGroupId: 503 }, + ], + }); + + expect(inspection.staleEntries).toEqual([]); + expect(inspection.orphanedProcessGroups).toEqual([500, 503]); }); diff --git a/tests/project-lifecycle-processes.test.ts b/tests/project-lifecycle-processes.test.ts index 9aa410a0..92b209eb 100644 --- a/tests/project-lifecycle-processes.test.ts +++ b/tests/project-lifecycle-processes.test.ts @@ -179,12 +179,45 @@ test("resolveLifecycleProcessGroupIdsForTmuxState ignores recycled process group ], }, panePidsByWindow: new Map([["proxy", []]]), - snapshot: [{ pid: 700, ppid: 1, processGroupId: 500 }], + snapshot: [ + { pid: 500, ppid: 1, processGroupId: 500 }, + { pid: 700, ppid: 500, processGroupId: 500 }, + ], }); expect(groups).toEqual([]); }); +test("resolveLifecycleProcessGroupIdsForTmuxState recovers a persisted leaderless group and descendants", () => { + const groups = resolveLifecycleProcessGroupIdsForTmuxState({ + lifecycleEntry: { + composeProject: "event-agent", + projectName: "event-agent", + branch: "feature-cleanup", + sessionName: "event-agent--lifecycle-feature-cleanup", + backend: "tmux", + updatedAt: "2026-04-01T14:00:00.000Z", + processes: [ + { + name: "proxy", + windowName: "proxy", + logPath: "/tmp/event-agent.log", + panePid: 500, + processGroupId: 500, + }, + ], + }, + panePidsByWindow: new Map([["proxy", []]]), + snapshot: [ + { pid: 501, ppid: 1, processGroupId: 500 }, + { pid: 502, ppid: 501, processGroupId: 500 }, + { pid: 503, ppid: 502, processGroupId: 503 }, + ], + }); + + expect(groups).toEqual([500, 503]); +}); + test("resolvePersistedLifecycleProcessGroupIds recovers live groups without a mux session", () => { const groups = resolvePersistedLifecycleProcessGroupIds({ lifecycleEntry: { @@ -242,9 +275,71 @@ test("resolveLifecycleStopProcessGroupIds skips persisted cleanup without a live expect(groups).toEqual([]); }); +test("resolveLifecycleStopProcessGroupIds skips a leaderless persisted group without a mux session", () => { + const groups = resolveLifecycleStopProcessGroupIds({ + matchedLiveSession: false, + lifecycleEntry: { + composeProject: "event-agent", + projectName: "event-agent", + branch: "feature-cleanup", + sessionName: "event-agent--lifecycle-feature-cleanup", + backend: "tmux", + updatedAt: "2026-04-01T14:00:00.000Z", + processes: [ + { + name: "proxy", + windowName: "proxy", + logPath: "/tmp/event-agent.log", + panePid: 500, + processGroupId: 500, + }, + ], + }, + snapshot: [ + { pid: 501, ppid: 1, processGroupId: 500 }, + { pid: 502, ppid: 501, processGroupId: 500 }, + { pid: 503, ppid: 502, processGroupId: 503 }, + ], + }); + + expect(groups).toEqual([]); +}); + +test("resolveLifecycleStopProcessGroupIds recovers a leaderless persisted group and descendants with a live mux session", () => { + const groups = resolveLifecycleStopProcessGroupIds({ + matchedLiveSession: true, + lifecycleEntry: { + composeProject: "event-agent", + projectName: "event-agent", + branch: "feature-cleanup", + sessionName: "event-agent--lifecycle-feature-cleanup", + backend: "tmux", + updatedAt: "2026-04-01T14:00:00.000Z", + processes: [ + { + name: "proxy", + windowName: "proxy", + logPath: "/tmp/event-agent.log", + panePid: 500, + processGroupId: 500, + }, + ], + }, + snapshot: [ + { pid: 501, ppid: 1, processGroupId: 500 }, + { pid: 502, ppid: 501, processGroupId: 500 }, + { pid: 503, ppid: 502, processGroupId: 503 }, + { pid: 504, ppid: 503, processGroupId: 504 }, + ], + }); + + expect(groups).toEqual([500, 503, 504]); +}); + test("wrapLifecyclePersistentCommand uses external kill for process-group cleanup", () => { const script = wrapLifecyclePersistentCommand({ command: "bun run proxy", + commandPidPath: "/tmp/event-agent.pid", logPath: "/tmp/event-agent.log", serviceName: "proxy", }); @@ -259,12 +354,18 @@ test("wrapLifecyclePersistentCommand uses external kill for process-group cleanu test("wrapLifecyclePersistentCommand avoids login-shell execution", () => { const script = wrapLifecyclePersistentCommand({ command: "bun run proxy", + commandPidPath: "/tmp/event-agent.pid", logPath: "/tmp/event-agent.log", serviceName: "proxy", }); expect(script).toContain('os.execvp("sh", ["sh", "-c", sys.argv[1]])'); expect(script).toContain('sh -c "$HACK_LIFECYCLE_COMMAND" >"$fifo" 2>&1 &'); + expect(script).toContain("os.setsid()"); + expect(script).toContain("pid_file.write(str(os.getpid()))"); + expect(script).toContain( + 'printf "%s\\n" "$cmd_pid" > "$HACK_LIFECYCLE_COMMAND_PID_FILE"' + ); expect(script).not.toContain('os.execvp("sh", ["sh", "-lc", sys.argv[1]])'); expect(script).not.toContain( 'sh -lc "$HACK_LIFECYCLE_COMMAND" >"$fifo" 2>&1 &' diff --git a/tests/run-exec-branch-default.test.ts b/tests/run-exec-branch-default.test.ts index ee51aa0f..1dd106b8 100644 --- a/tests/run-exec-branch-default.test.ts +++ b/tests/run-exec-branch-default.test.ts @@ -180,6 +180,25 @@ test("hack exec in a linked worktree targets the branch compose project by defau expect(combined).toContain('branch instance "feature-run-default"'); }); +test("hack run in a detached linked worktree refuses to target the base compose project", async () => { + const fixture = await createWorktreeFixture(); + runGit({ cwd: fixture.worktreeRoot, args: ["checkout", "--detach"] }); + + const result = await runCliWithCapturedOutput([ + "run", + "--path", + fixture.worktreeRoot, + "api", + "echo", + "hi", + ]); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("Detached linked worktree"); + expect(result.stderr).toContain("--branch "); + expect(await Bun.file(fixture.dockerLogPath).exists()).toBe(false); +}); + async function runCliWithCapturedOutput( args: readonly string[] ): Promise {