Skip to content

Commit 61ac7f6

Browse files
roodboiclaude
andcommitted
fix(daemon): address PR review — strict orphan match, stale-path repair, docs sync
Orphan detection now requires a hack executable (basename hack/hack-* or a bun dev-tree invocation) — a bare substring match could have terminated unrelated processes. Plist repair also normalizes programs that still exist but differ from the current stable resolution (surviving homebrew Cellar paths would relaunch the old binary in an incompatible-daemon loop). Instruction source lists the tickets/ ignore entry; surfaces resynced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a0bcde4 commit 61ac7f6

9 files changed

Lines changed: 46 additions & 8 deletions

File tree

.codex/skills/hack-cli/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ Use `hack` as the primary interface for local-first development.
6565
- Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose.<branch>.override.yml`.
6666
- Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).
6767
- Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.
68-
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
68+
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
6969

7070
## Linked git worktrees
7171

.cursor/rules/hack.mdc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is
2929
- Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose.<branch>.override.yml`.
3030
- Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).
3131
- Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.
32-
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
32+
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
3333

3434
## Linked git worktrees
3535

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ Project files (managed vs generated):
257257
- Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose.<branch>.override.yml`.
258258
- Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).
259259
- Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.
260-
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
260+
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
261261

262262
Linked git worktrees:
263263
- Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ Project files (managed vs generated):
127127
- Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose.<branch>.override.yml`.
128128
- Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).
129129
- Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.
130-
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
130+
- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).
131131

132132
Linked git worktrees:
133133
- Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments.

src/agents/instruction-source.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export const INSTRUCTION_SECTIONS: readonly InstructionSection[] = [
113113
"Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose.<branch>.override.yml`.",
114114
"Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).",
115115
"Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.",
116-
"Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).",
116+
"Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).",
117117
],
118118
},
119119
{

src/daemon/launchd.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,23 @@ export async function repairLaunchdProgramIfInvalid(opts: {
249249
return "not-installed";
250250
}
251251
const program = extractLaunchdProgramPath({ plistText });
252-
const valid =
252+
const invocation = await resolveHackInvocation();
253+
const resolved = await resolveLaunchdHackBinPath({ invocation });
254+
const exists =
253255
program !== null &&
254256
!isVirtualExecutablePath(program) &&
255257
(await pathExists(program));
258+
// A program that still exists can still be stale: a versioned homebrew
259+
// Cellar path survives until brew cleanup, so kickstart would relaunch
260+
// the OLD binary forever (incompatible-daemon loop). Normalize to the
261+
// current stable resolution whenever it differs.
262+
const valid = exists && (resolved === null || program === resolved);
256263
if (valid) {
257264
return "ok";
258265
}
266+
if (!exists && resolved === null) {
267+
return "failed";
268+
}
259269

260270
const result = await installLaunchdService({
261271
paths: opts.paths,

src/daemon/process.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ export async function findOrphanDaemonProcesses(opts: {
101101
if (!command.includes(DAEMON_COMMAND_MARKER)) {
102102
continue;
103103
}
104+
if (!isHackDaemonCommand({ command })) {
105+
continue;
106+
}
104107
if (pid === opts.trackedPid || pid === process.pid) {
105108
continue;
106109
}
@@ -109,6 +112,26 @@ export async function findOrphanDaemonProcesses(opts: {
109112
return orphans;
110113
}
111114

115+
/**
116+
* True when the process command line is actually a hack daemon: the
117+
* executable's basename must be `hack`/`hack-*` (or a `bun .../hack`
118+
* dev-tree invocation). A bare substring match on the daemon arguments
119+
* would also kill unrelated processes that merely mention them.
120+
*/
121+
function isHackDaemonCommand(opts: { readonly command: string }): boolean {
122+
const firstToken = opts.command.trim().split(/\s+/)[0] ?? "";
123+
const base = firstToken.split("/").pop()?.toLowerCase() ?? "";
124+
if (base === "hack" || base.startsWith("hack-")) {
125+
return true;
126+
}
127+
if (base === "bun") {
128+
const secondToken = opts.command.trim().split(/\s+/)[1] ?? "";
129+
const secondBase = secondToken.split("/").pop()?.toLowerCase() ?? "";
130+
return secondBase === "index.ts" || secondBase.startsWith("hack");
131+
}
132+
return false;
133+
}
134+
112135
async function listProcessTable(): Promise<readonly string[]> {
113136
const proc = Bun.spawn(["ps", "-axo", "pid=,command="], {
114137
stdout: "pipe",

tests/agent-instruction-source.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ test("tickets appear at most once, only as an optional extension", () => {
103103
for (const [surface, rendered] of Object.entries(RENDERED_SURFACES)) {
104104
const ticketLines = rendered
105105
.split("\n")
106+
// The gitignore pattern literal `tickets/` (managed-files section) is
107+
// a filename, not tickets promotion — the thing this test guards.
108+
.map((line) => line.replaceAll("`tickets/`", ""))
106109
.filter((line) => /ticket/i.test(line));
107110
expect(
108111
ticketLines.length,

tests/daemon-orphan.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,18 @@ test("all daemon processes are orphans when no pid is tracked", async () => {
2525
expect(orphans).toEqual([123, 456]);
2626
});
2727

28-
test("ignores its own pid and requires the exact daemon marker", async () => {
28+
test("ignores its own pid, non-hack executables, and near-miss commands", async () => {
2929
const orphans = await findOrphanDaemonProcesses({
3030
trackedPid: null,
3131
psLines: [
3232
` ${process.pid} hack daemon start --foreground`,
3333
" 789 tail -f daemon-start-foreground.log",
3434
" 790 node something daemon start --foreground",
35+
" 791 /tmp/hack-repo/bin/hack-dev daemon start --foreground",
36+
" 792 bun /tmp/hack-repo/index.ts daemon start --foreground",
3537
],
3638
});
37-
expect(orphans).toEqual([790]);
39+
expect(orphans).toEqual([791, 792]);
3840
});
3941

4042
import {

0 commit comments

Comments
 (0)