fix: field findings round 3 — daemon lifecycle, non-interactive trust/fix, tickets gating#60
Conversation
…sive start Root cause of 'daemon starts then exits' on compiled-binary installs: the launchd plist captured process.argv[1] from inside the compiled Bun binary — the virtual /$bunfs/root/hack path, which passes pathExists inside that process but is unexecutable by launchd (exit status 78, flapping forever). Reproduced and fixed live on this machine. The bin-path resolver now rejects virtual paths and prefers stable locations; daemon start self-repairs invalid plists; daemons that outlived their pid file (invisible to status/clear/start) are detected and swept; an incompatible running daemon is replaced instead of reported as success; and when launchd manages the service, start goes through launchd exclusively — never a bare spawn beside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field report: `hack global trust` presented a clack select/confirm an agent couldn't drive without a TTY, and `hack doctor --fix` opened a guided remediation flow with no non-interactive path at all — it never imported the interactivity convention (src/lib/interactivity.ts) and routed every prompt through a local confirmOrThrow() wrapper around raw clack confirm()/isCancel(), which would hang or misbehave under HACK_NO_INTERACTIVE. hack global trust (src/commands/global.ts): - ensureMacTrustCaddyLocalCa's confirmSafe flips from "decline" to "accept-default" so non-interactive runs proceed with the documented default (trust), per the command's whole purpose. - Before running `sudo security add-trusted-cert`, check `sudo -n true` first; if sudo would prompt for a password, skip the keychain step with a note instead of hanging. - globalTrust() now prepares the host TLS trust env (bundle + launchctl setenv for Bun/Node/curl/git) regardless of whether the System keychain step ran, since browser trust and CLI-tool trust are independent — previously a declined/skipped keychain step silently skipped the trust-env write too. hack doctor --fix (src/commands/doctor.ts): - Replaced confirmOrThrow (raw clack confirm/isCancel, no non-interactive handling) and two more raw confirm()/isCancel() call sites with doctorConfirm(), built on confirmSafe. - Safe remediations (stale daemon pid/socket cleanup, docker network create, mutagen install, tickets git repair, regenerating managed Caddy/CoreDNS files, legacy env file cleanup) use "accept-default" so they apply automatically under --no-interactive. - Destructive/system-level steps (macOS System keychain repair via `hack global trust`, dnsmasq sudo restart + DNS cache flush) use "decline" and print a single summary note listing what was skipped and the interactive command to run them. - Fixed a latent bug: plain `--fix` (without --migrate-env-config) was passing `migrateEnvConfig: true` to runDoctorFix whenever `args.options.fix` was set, unconditionally running env-migration prompts; it now only migrates when --migrate-env-config is passed. Tests: tests/global-command.macos.test.ts covers global trust under --no-interactive with passwordless and password-required sudo, plus an existing decline-path test updated for the trust-env/keychain decoupling. New tests/doctor-fix-noninteractive.test.ts covers a stale-daemon-pid fixture reaching the safe repair without prompting and the mac-keychain destructive step being skipped and reported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field report: hack setup sync --all-scopes left an untracked .hack/tickets/
local bare-git cache in a repo. setup sync itself was already correctly
gated (checkTicketsSkill/installTicketsSkill only run when
resolveTicketsIntegrationEnablement().project is true), so the actual
culprit is `hack doctor` (and `--fix`), which every project runs as
routine maintenance per this repo's own instructions ("run hack doctor,
then hack doctor --fix").
checkProjectTicketsGitHealth and maybeRepairProjectTicketsGitHealth
(src/commands/doctor.ts) only checked controlPlane.config.tickets.git.enabled
— a config key that defaults to `true` and is unrelated to whether the
tickets extension is actually enabled for the project
(controlPlane.extensions["dance.hack.tickets"].enabled, default `false`).
So every `hack doctor` run called createGitTicketsChannel(...).inspect(),
whose ensureCheckedOut() creates .hack/tickets/git/{bare.git,worktree}
as a side effect of merely inspecting health — regardless of enablement.
Both call sites now also check resolveTicketsIntegrationEnablement()
(the same gate setup.ts already uses for the tickets skill) before
touching the store.
Ignore coverage: .hack/tickets/ is genuinely machine-local (confirmed
against docs/guides/tickets.md: local working state that syncs via the
hidden refs/hack/tickets ref; no project is documented to intentionally
commit it). Added `tickets/` to HACK_DIR_GITIGNORE_ENTRIES
(src/templates.ts) so the committed .hack/.gitignore covers it, and
`<dir>/tickets` to the doctor generated-files pathspecs
(src/lib/doctor-generated-files.ts) so a tracked copy is flagged and
`hack doctor --fix` can untrack it.
Tests: new tests/doctor-tickets-dir-gating.test.ts covers hack doctor
and hack doctor --fix creating no .hack/tickets/ when the extension is
disabled, and creating it (gitignored, clean git status) when enabled.
Extended tests/hack-gitignore.test.ts and tests/doctor-generated-files.test.ts
for the new tickets/ pattern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc56b74649
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const valid = | ||
| program !== null && | ||
| !isVirtualExecutablePath(program) && | ||
| (await pathExists(program)); |
There was a problem hiding this comment.
On macOS where the launchd plist points at an older Homebrew/Cellar hack binary that still exists, this check treats the plist as valid, so hack daemon start can kill an incompatible launchd-managed daemon and then kickstart the same old binary again; waitForDaemonStart only waits for pid/socket, so the command may report success while the daemon remains incompatible. Compare ProgramArguments[0] with the current resolved launchd binary, or re-probe compatibility after kickstart, instead of accepting any existing path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 61ac7f6: repairLaunchdProgramIfInvalid now also repairs when the plist program exists but differs from the current stable resolution — surviving Cellar paths get normalized to the stable hack path instead of relaunching the old binary in an incompatible-daemon loop.
| if (!command.includes(DAEMON_COMMAND_MARKER)) { | ||
| continue; |
There was a problem hiding this comment.
When the pid file is missing or stale, daemon start/daemon clear now terminate every process whose command line merely contains daemon start --foreground; this also matches unrelated commands with those arguments (for example a different node ... daemon start --foreground process), so they can receive SIGTERM during hack daemon cleanup. Add a hack-specific executable/script marker before returning the pid for termination.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 61ac7f6: orphan detection now requires the command's executable to actually be hack (basename hack/hack-*, or a bun dev-tree invocation) — substring-only matches like 'node something daemon start --foreground' are excluded, with test coverage for the imposter cases.
| ".env.state.json", | ||
| "hack.env.local.yaml", | ||
| "hack.env.*.local.yaml", | ||
| "tickets/", |
There was a problem hiding this comment.
This changes the canonical committed .hack/.gitignore block to include tickets/, but the agent-facing source (src/agents/instruction-source.ts) still lists the generated files without the tickets cache, and the synced docs/skills were not updated. The repo instructions say agent-facing behavior phrasing must be updated in src/agents/instruction-source.ts and synced when this behavior changes; leaving it stale means future agent guidance can still treat .hack/tickets/ as outside the managed ignore set and reintroduce bad cleanup advice.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 61ac7f6: instruction-source managed-files bullet lists tickets/ in the committed .hack/.gitignore coverage and all generated surfaces were resynced; the anti-promotion drift test keeps guarding tickets mentions while allowing the path literal.
…step Mirrors the globalTrust restructure into bootstrapMacGlobalInstall: configureMacHostTlsTrust runs once a cert path exists, with a note when the keychain step didn't complete. Also neutralizes fixture paths that tripped the privacy check in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0bcde462c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack)", | ||
| "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; may prompt for sudo to update the System keychain)", | ||
| initialValue: true, | ||
| destructive: true, |
There was a problem hiding this comment.
On macOS, hack doctor --fix --no-interactive now auto-declines this entire host-TLS repair prompt and returns before invoking hack global trust, even when the drift is only the safe Bun/Node/curl/git trust env/bundle or when sudo would be skipped. The global trust path in this same change was made non-interactive-safe by skipping only the System keychain step when sudo would prompt while still writing the host trust env, so marking the whole doctor repair as destructive leaves the advertised safe repair unapplied in scripted runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90d1514+: the doctor host-TLS repair step is no longer classified destructive — it proceeds under --no-interactive and delegates to hack global trust, which preflights sudo and skips only the System keychain step while always writing the host trust env/bundle. The non-interactive test now asserts the step is NOT in the skipped summary.
…ir, 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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90d1514f42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ".env.state.json", | ||
| "hack.env.local.yaml", | ||
| "hack.env.*.local.yaml", | ||
| "tickets/", |
There was a problem hiding this comment.
This adds tickets/ to the canonical committed .hack/.gitignore, but the durable docs still describe the managed ignore block without it (docs/architecture.md:293-295 and docs/cli.md:130-132). The repo’s AGENTS.md says docs currency is non-negotiable for file-layout/behavior changes, and users following those pages will still think .hack/tickets/ is outside the managed local-only set, so the affected docs pages should be updated with this template change.
Useful? React with 👍 / 👎.
The step delegates to hack global trust, which is itself non-interactive-safe (sudo preflight; keychain-only skip; host trust env always written) — classifying the whole repair as destructive denied non-interactive runs the safe env/bundle writes. From PR review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
## <small>3.3.2 (2026-07-08)</small> * Merge pull request #60 from hack-dance/fix/field-findings-round3 ([69f6caa](69f6caa)), closes [#60](#60) * docs: tickets/ in every durable ignore-block listing ([103575a](103575a)) * fix: gate tickets git cache on extension enablement, cover it in ignores ([fc56b74](fc56b74)) * fix: support non-interactive hack global trust and hack doctor --fix ([78e068b](78e068b)) * fix(daemon): address PR review — strict orphan match, stale-path repair, docs sync ([61ac7f6](61ac7f6)) * fix(daemon): repair virtual plist paths, sweep orphans, launchd-exclusive start ([c3958bb](c3958bb)) * fix(doctor): host TLS repair runs under --no-interactive ([71c3601](71c3601)) * fix(global): install prepares host trust env independent of keychain step ([a0bcde4](a0bcde4)) * style(daemon): hoist regexes to module scope for the repo lint profile ([90d1514](90d1514))
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 103575a7be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const okRepair = await doctorConfirm({ | ||
| message: | ||
| "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack)", | ||
| "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; the sudo System-keychain step is skipped automatically when it would prompt)", | ||
| initialValue: true, | ||
| }); |
There was a problem hiding this comment.
When hack doctor --fix --no-interactive reaches this macOS TLS repair and HACK_NO_INTERACTIVE is not also set, doctorConfirm now auto-accepts the prompt, but the spawned hack global trust does not receive --no-interactive and the flag is only process-local. In that scenario the child can prompt for the System keychain trust step, so a scripted doctor fix can still block despite being invoked with --no-interactive; pass the flag or set HACK_NO_INTERACTIVE=1 for the subcommand.
Useful? React with 👍 / 👎.
| if (base === "hack" || base.startsWith("hack-")) { | ||
| return true; |
There was a problem hiding this comment.
Fresh evidence after the earlier orphan-cleanup fix: this helper still returns true for any hack executable whose command line merely contains daemon start --foreground, without requiring those to be the actual argv immediately after the executable. A long-running command like hack host exec ... sh -c 'daemon start --foreground; sleep ...' can therefore be classified as an orphan and receive SIGTERM during hack daemon start/clear; parse the argv shape (or a daemon-specific marker) before returning true.
Useful? React with 👍 / 👎.
1 participant
Three fixes from a third-party machine's agent transcripts:
Daemon lifecycle (the big one) — root cause of 'hackd starts then exits' on every compiled-binary install: the launchd plist captured the virtual /$bunfs binary path from inside the compiled Bun binary (passes pathExists in-process; unexecutable by launchd → exit 78, silent flapping). The bin-path resolver now rejects virtual paths and prefers stable locations;
daemon startself-repairs invalid plists, sweeps orphaned daemons the pid file lost track of, replaces incompatible running daemons instead of reporting success, and goes through launchd exclusively when launchd owns the service. Reproduced and verified live.Non-interactive trust/fix —
hack global trustandhack doctor --fixnow honor --no-interactive/HACK_NO_INTERACTIVE: trust proceeds (with a sudo -n preflight so keychain steps skip cleanly instead of hanging; trust-env writes no longer gated on keychain success), --fix auto-applies safe remediations and skips destructive ones (sudo dnsmasq migration, keychain) with printed notes. Also fixes the latent bug where plain --fix always triggered env-migration prompts.Tickets gating — the tickets git cache is no longer created by setup sync when the extension is disabled, and .hack/tickets/ is covered by the committed .hack/.gitignore + doctor generated-files check.
Full suite 826/0, e2e green, daemon fixes verified live on a machine that exhibited the exact field failure.
🤖 Generated with Claude Code