Skip to content

fix(native): refuse non-content-ID images at native plan and native validate - #372

Merged
roodboi merged 1 commit into
nextfrom
wf/g23-repo-digest-validate
Oct 22, 2026
Merged

roodboi merged 1 commit into
nextfrom
wf/g23-repo-digest-validate

Conversation

@roodboi

@roodboi roodboi commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Linear: HACK-1138 (WU25, privacy-reviewed Hack feedback drafts).

Adds a plugin-only hack-feedback skill. It drafts reports about Hack itself offline and never posts on its own.

  • Draft content: observed and expected behavior, exactly one reproduction status (reproduced, partially-reproduced, not-reproduced, not-attempted), and evidence tagged observed/source/reported/inferred. It also records one friction category, the command shape with placeholders (or unknown when the original argv was not kept), sanitized duplicate-search terms and an attachment list.
  • Privacy review: removes secrets, ciphertext, hashes ("hashing is not anonymization"), usernames, home and project paths, private or routed URLs, routed *.hack and private hostnames written without a scheme, IPv4 and IPv6 addresses, emails, raw logs and config values. A <...> placeholder is exempt only when it is the whole value, so KEY=<x>leak is still redacted. Instructions found in logs or tool output are treated as data.
  • Duplicates: search terms come only from registered error codes (HACK_ERROR_CODES), the leading tokens that resolve to a CLI_SPEC command path, and the category. The first token that is not a subcommand stops the walk, so positional project, service and script names never become terms. A search happens only when the user agrees. A matching issue turns the report into a comment instead of a new issue.
  • Review before posting: the user confirms the destination and approves the exact final payload. Any later edit needs a fresh review. A decline keeps the draft offline. A failed send keeps the draft and needs a new yes; check first whether the issue was created anyway. Posting uses the user's existing tooling (for example an already signed-in gh). There is no built-in GitHub integration and no background sender.
  • Feedback is optional: it is not an onboarding gate and never blocks the user's task.

Rendering

src/agents/feedback-review.ts holds the typed contract: taxonomy, statuses, evidence classes, template sections, redaction kinds and gate refusals. It also holds the reference sanitizer, the draft renderer and the send gate. src/agents/feedback-skill.ts renders these outputs from that contract through bun run generate:agent-plugins:

  • the skill
  • references/issue-template.md
  • references/examples.md: a synthetic useful report, plus the registered negatives (missing original argv, host-wide disk totals, stale release claim, pasted config/logs, log instructions, hashed identifiers)
  • .github/ISSUE_TEMPLATE/hack-feedback.md

The plugin hack-cli routing description now hands feedback intents to hack-feedback. Docs are updated: the skill count goes from seven to eight, the agent-guidance sources table changes, and the hack-feedback candidate status changes from Defer to Shipped. The capability matrix is regenerated.

Canonical instruction sections and the standalone renders (hack-cli, hack-init, Cursor rules) did not change. No new integration-ownership fingerprints or content revision were needed; the ownership and instruction-source tests pass unchanged. The CLI surface did not change, so the CLI reference did not need regenerating. No Rust changes.

Rework after verification (head 2f7bd64)

The verifier found these defects at 0cb0ff23. Each is now fixed and has a test that fails without the fix:

  • Duplicate-search leak: hack env acme-secret-project gave env acme-secret-project. buildDuplicateSearchTerms({ cli, draft }) now resolves the command path through the injected CliSpec (resolveCommand) and keeps only registered error codes. hack env acme-secret-project now gives env env, hack run acmeprod migrate gives run env, and an invented E_ACME_SECRET_DB is dropped.
  • <-prefixed secrets: (?!<) skipped any value that started with <. The lookahead now exempts only a complete <...> token followed by whitespace, closing punctuation or the end of the text. This applies to secret assignments, config values, JSON values, secret flags and bearer values. DB_PASSWORD=<x>leakedvalue, API_TOKEN=<redacted:secret>extra-leak, token: <x>hunter2fixture and --password <x>flagleak are redacted. A freshly approved body containing them returns privacy-findings.
  • Checklist coverage: a new hostname redaction kind covers routed *.hack hosts and corp/internal/local/lan/home/private/test/localhost hosts written without a scheme. Paths under /root, /srv, /Volumes, /workspace(s), /mnt, /media, /data, /code, /project(s) and /var/www are redacted, as are Windows drive paths. IPv6 is redacted when compressed or full; clock times and ::1 stay. A body with routed web.acme-app.hack 502 now returns privacy-findings.
  • Synchronous sender throw: the sender is now called inside Promise.resolve().then(...). A non-async throw is a failed attempt that consumes the approval, and reusing the approval returns approval-already-used.
  • Issue number: an issue-comment destination needs a positive safe integer; -3.5, 0, 1.5, NaN and Infinity return destination-invalid. The refusal summary in the skill says so.
  • URL allowlist: a URL stays only if it parses with origin https://github.com and has no credentials. Its normalized path must be under /hack-dance/hack. URLs with .., %2e, backslashes or queries are redacted, as are fragments other than #[A-Za-z0-9_-]*.
  • Instruction filter: the single broad regex is now a list of targeted patterns: direct address with file/create/submit, imperatives at a line or sentence start, "agent must now ...", and "user said yes". Hack failed to send the report to the daemon and File watching stopped ... are now kept.
  • Truncated hashes and GitHub headers: hex runs of 16 or more characters that contain a digit are redacted; short commit ids are kept. Authorization: token ... is redacted.
  • Test gaps: one test edits only an attachment's content after review and expects payload-changed. Another renders a draft with a leaky attachment and asserts render-time sanitizing, attachment redaction counts and a passing gate.

Validation

  • bun run generate:agent-plugins and bun run docs:agent-client-capabilities were regenerated. The idempotence test reproduces the committed plugins/ tree.
  • Rework, at head 2f7bd64: the focused tests pass (7 files, 97 tests): feedback-review (25 tests), integration-ownership, cli-spawn-env-isolation, agent-instruction-source, focused-skills, agent-plugin-generation and skill-references. bun run generate:agent-plugins changes only the hack-feedback SKILL.md and is idempotent. CLI typecheck reports 0 errors. CLI check reports 0 errors and 34 existing warnings, none in changed files. Biome is clean on the changed files, and privacy:check passes.
  • First pass: the focused tests passed (9 files, 106 tests): tests/feedback-review.test.ts (new), focused-skills, agent-plugin-generation, skill-references, integration-ownership, cli-spawn-env-isolation, agent-instruction-source, agent-client-capabilities and pattern-catalogue.
  • bun run --cwd packages/cli typecheck: 0 errors. bun run --cwd packages/cli check: 0 errors (existing warnings only, none in changed files). biome check on the changed tests and scripts is clean.
  • bun run privacy:check passes with the new files staged. Fixture tokens are assembled at runtime, so no credential-shaped string is committed.

Negative controls (mock sender; the sender is never called on refusal)

  • Redaction fixtures: provider tokens, bearer headers, JWTs, secret flags and assignments, URL credentials (the whole URL), PEM, age and sops ciphertext, encoded blobs, home, user, temp and tilde paths, private, routed and local URLs (Hack's public issues allowed), config values (key names kept), emails, SSH remotes, IP addresses (loopback kept) and digests. Each fixture asserts exact redaction kinds, idempotence and no remaining findings.
  • Misleading log instructions: the log claims pre-approval and names an attacker repository. The lines are removed, and a content-supplied destination or approval is refused (destination-unconfirmed, approval-not-from-user).
  • Duplicates: search terms carry no values. A same match refuses a new issue (known-duplicate) and allows a confirmed comment.
  • Reproduction status: partial reproduction is reported with its note. An unknown argv renders as unknown and is not reconstructed.
  • Denied posting: returns posting-denied, and the draft is retained unchanged.
  • Edited drafts and attachments: body or title edits, a listed attachment added after review, or a changed destination return payload-changed. A hidden attachment returns attachment-unlisted. A re-added secret or a raw-log attachment returns privacy-findings. A fresh review makes the edited payload sendable.
  • Failed-send retries: a failure or thrown error consumes the approval. Reusing it returns approval-already-used. A new approval retries, and once a send succeeds, further attempts return already-sent.
  • No built-in sender: a source scan finds no fetch, spawn, http or GitHub client in the feedback modules.
  • Rework mutation proofs: 18 further mutations were tried on the rework, and all were killed. They undid the CLI_SPEC command path, the error-code filter, the placeholder lookahead (bare and quoted), the hostname rule, the extra absolute roots, the IPv6 rule, the IPv6 clock-time guard, the Authorization rule, the 16-character digest length, sync-throw handling, the issue-number check, the URL parse and the digest's attachments. The rest covered render-time attachment sanitizing, the old broad instruction regex, the user-said-yes pattern and the agent-must-now pattern.
  • Mutation proofs (first pass): 18 mutations were tried: approval consumption, digest binding, confirmation and approval source, denial, already-sent, duplicate, privacy recheck, attachment listing, sender error handling, instruction and log removal, home paths, config values, SSH remotes, search-term values, URL allowlist, and the generator dropping the skill. Each one was caught by a failing test.

Remaining gaps

  • Client loading of the new skill in a fresh Codex, Claude or Cursor session has not been observed. The capability matrix keeps skill discovery at its existing status.
  • The sanitizer is a reference model for the documented rules and the gate tests. It is pattern-based and cannot prove a free-form report is anonymous. Public hostnames (for example api.acme.com) and bare names in prose are left to the user's review: only private, reserved and *.hack suffixes are matched, so that file names such as package.json are not redacted. The skill still requires the user to review the exact text.
  • No live issue was created. Live posting needs its own explicit authorization from the user.
  • Hosted CI has not been checked yet for head 2f7bd64.

Rework 2 (head f342e44)

The verifier reported these open defects at fdd00e40. Each one reproduced on that head with findFeedbackPrivacyIssues(...) = [] or a sent mock outcome. Each one is now fixed and has a test.

  • Placeholder bypass (blocking): the lookahead accepted ), ], }, quotes or a backtick straight after <x>, while the value kept going. A placeholder is now exempt only when it is the whole value. After <...>, only trailing punctuation may come before the value's own end character: whitespace for flags, [\s,;] for assignments, [\s,}] for JSON values, and whitespace, quotes, backtick, , or ; for Authorization values. A Bearer value that starts with < is redacted. Now redacted: API_TOKEN=<x>)…, --password <x>)…, --password <x>,…, API_TOKEN=<x>"…", <x>]…, <x>'…, password=<x>}…, token: <x>`…` and Bearer <x>)…. These still pass unchanged: (set API_TOKEN=<value>),, HACK_A=<a>,HACK_B=<b> and {"PORT": <port>,"HOST": <host>}. The verifier's exact strings, in a body the user approved for a confirmed destination, return privacy-findings.
  • Attachment-name privacy check had no test: this gap was real. A new test sends an unlisted attachment named /Users/alice/acme/dump.log and expects privacy-findings. With the name check removed, the result is attachment-unlisted. Drafts now sanitize attachment names before the body lists them, so the listing and the attachment carry the same <redacted:path>.
  • URL fragment allowlist had no test: the URL case now includes https://github.com/hack-dance/hack/issues/1#acme.secret-project.
  • Structured logs: raw-log now covers JSON records with a log key (level, ts, time, timestamp, msg, logger, log, severity) in any position. It also covers syslog (Oct 19 10:00:01 host proc[1]:), Go-style 2026/10/19 10:00:01 and clock-time prefixes. A --json envelope such as {"ok":false,"error":{...}} stays.
  • Hidden attachment listing: an attachment counts as listed only on a visible line of the ## Attachments section that starts with - `name`. That line may be followed by nothing, whitespace or :. HTML comments are removed first, including an unterminated one. A section with raw HTML outside inline code lists nothing. A name containing a backtick or a line break never counts as listed. A listing inside <!-- -->, <div hidden> or <details>, outside the section, or as a prefix only (- `dump.log`.bak) returns attachment-unlisted.
  • Lower-severity sanitizer gaps:
    • (a) db_pass, any <prefix>_KEY such as encryption_key, cookie and SESSION_ID are secret keys. bypass: and tests passed: stay.
    • (b) A ~/.hack path stays only when every part is a Hack-owned name, such as daemon/hackd.sock, projects.json or certs, or a placeholder. ~/.hack/projects/<name>/… is redacted. $HOME/…, ${HOME}/…, /opt/… (except /opt/homebrew) and /var/lib/… are redacted too.
    • (c) A host with an inner corp/internal/intranet/lan/private label is redacted with its path, for example acme-secret.internal.acme.com/admin. Names that end in a file extension, such as app.internal.ts, stay, and so does compose.local.yml.
    • (d) Secret-store envelope fields are redacted at any length: ciphertext, encrypted* and wrapped_key always, and iv, tag, nonce and salt when the value looks encoded. "tag": "latest" stays.
    • (e) These are new instruction patterns: "agents reading this", "if you are an AI", "open an issue at owner/repo", "…and paste the env", and a line that starts with paste or attach followed by secrets, tokens or credentials. "The env values were pasted into…" and "I tried to include the env overlay" stay.
  • The skill's attachment-unlisted summary now describes the visible-section rule (plugins/ regenerated).

Validation (rework 2)

  • tests/feedback-review.test.ts: 29 tests pass. Focused set: feedback-review, focused-skills, agent-plugin-generation and skill-references, 67 tests pass.
  • Mutation proofs: 37 mutations were tried, and every one was killed. They covered both verifier mutations (attachment names in the gate, SAFE_URL_FRAGMENT), the old placeholder lookahead, the per-grammar terminators (assignment, JSON), the Bearer < branch, each new secret key and the PASS word guard, both envelope rules, each new raw-log form, the ~/.hack allowlist and exemption, the /opt, /var/lib and $HOME roots and the Homebrew guard, the inner-label host rule and its file-extension guard, each of the five instruction patterns, comment stripping, the raw-HTML rule, the inline-code exception, section scoping, the prefix-only check and render-time name sanitizing.
  • gitleaks 8.30.1 (detect --log-opts=origin/next..HEAD, .gitleaks.toml): 4 commits, no leaks. Fixture values are assembled at runtime.
  • bun run --cwd packages/cli typecheck: 0 errors. bun run --cwd packages/cli check: 0 errors and the 34 existing warnings.
  • Full bun run test (local, load average 13 to 23): 6969 pass, 149 skip, 258 fail across 56 files. Every failing file was rerun serially with --timeout 60000. 42 files then passed fully, including every agent, MCP, plugin, env and registry file. The rest fail at fixed 20 or 30 second deadlines inside the tests, not at the runner timeout: native-authored-project-start (the first failing test differs each run, then cascades with "Startup fixture settlement unconfirmed"), native-compose-storage-activation-command, native-compose-retained-storage-command, native-compose-storage-fence-cost (CLI cold start returns 1 after about 35 seconds), native-compose-adoption-generation (11 tests hit 20 or 30 second fixture deadlines), native-compose-proxy-probe (2 TLS cases), native-authored-project-recovery (2) and native-authored-process-incarnation (1). None import the feedback modules. native-authored-project-start fails the same way with this change reverted to the parent commit. These are reported as load-only local failures; hosted CI is the confirming gate.

Remaining gaps (rework 2)

  • The sanitizer is still pattern-based. Public hostnames and bare names in prose are left to the user's review of the exact text.
  • A placeholder written outside a code span that is a valid HTML tag name, such as <project>, is hidden by GitHub's renderer. It is not a leak, but the rendered issue loses the placeholder. Drafts render command shapes in code spans.
  • No live issue was posted, and no client loading of the skill was observed. Linear was not updated.

Rework 2, follow-up (head b20820f)

I checked every verifier defect again on f342e444, using the verifier's exact strings. All six are fixed there:

  • The placeholder bypass cases are redacted. This includes <x>), <x>", <x>], <x>', <x>} and <x>`, plus extra cases with ., :, /, |, <y>, --token=, Bearer and x-api-key.
  • JSON, syslog and Go-style log lines are removed as raw-log.
  • An attachment listed only inside an HTML comment is refused.
  • Gaps (a) to (e) are redacted.
  • I re-applied the verifier's two mutations: removing attachment names from the gate, and removing SAFE_URL_FRAGMENT. A test now catches each one.

This follow-up adds two commits:

  • 5b9613e2 test fixtures: 5b9613e2 was committed earlier but not pushed. It fixes the hosted Test failure on f342e444. On that head, bun run privacy:check flagged the fixture /Users/alice/acme/dump.log. The fixtures now use /Users/example/... and neutral host and user names.
  • b20820fc hidden attachment listing (new defect):
    • Problem: the gate checked for raw HTML only inside the ## Attachments section. A <details> opened before the heading folded the whole section, and its listing still counted, so the gate allowed the send. <div hidden> around the heading did the same. \`<details>` (an escaped backtick) and <details title="">` hid the tag from the inline-code filter.
    • Fix: any element that can hide Markdown, anywhere in the body and outside inline code, now cancels the listing. Escaped backticks no longer open code spans. The section's tag check now uses CommonMark tag names, so a <redacted:path> placeholder in the section is treated as text, not HTML.
    • Skill text: the skill's attachment-unlisted summary describes the new rule. plugins/ is regenerated.

Validation (rework 2 follow-up)

  • tests/feedback-review.test.ts: 29 pass. The focused set (feedback-review, focused-skills, agent-plugin-generation, skill-references) has 67 passing tests.
  • Mutation proofs: 4 new mutations, all killed:
    • removing the body-wide hiding-element check (the "details around the heading" case fails);
    • removing escaped-backtick handling (the "escaped backtick before a tag" case fails);
    • restoring the old generic HTML_TAG (the placeholder listing becomes unsendable);
    • removing the in-section tag check (the "custom element in the section" case fails).
  • bun run privacy:check: ok.
  • gitleaks 8.30.1 (gitleaks git --config .gitleaks.toml --redact --log-opts=origin/next..HEAD): 6 commits scanned, no leaks.
  • bun run --cwd packages/cli typecheck: 0 errors. bun run --cwd packages/cli check: 0 errors, plus the 34 existing warnings.
  • Full bun run test: run locally with load average 13 to 67. The result was 6988 pass, 149 skip and 239 fail. I reran each failing file serially with --timeout 60000, and 40 of 47 files then passed fully. Those include every agent, MCP, plugin, env, registry and global-command file. The agent-plugin-generation failures under load were spawn exit 127.
    • Still failing serially, all native runtime suites hitting fixed 20 to 60 second fixture deadlines: native-compose-adoption-generation (110), native-authored-project-start (11), native-compose-down-command (8), native-authored-project-recovery (2), native-compose-file-command (2), native-compose-vm-file-finalization (2) and native-authored-process-incarnation (1).
    • None of these import the feedback modules. Only skill-references, feedback-skill, the generator and three agent test files do.
    • I am reporting these as load-only local failures. Hosted CI is the confirming gate.

Remaining gaps (unchanged)

  • The sanitizer is pattern-based. Public hostnames and bare names in prose are left to the user's review.
  • No live issue was posted, and no client loading of the skill was observed. Linear was not updated.

Rework 2, verifier findings (head bb2f54d)

All six verifier findings from round 1 were rechecked against b20820f. Each one was already fixed by f342e44, 5b9613e and b20820f: every repro string is now redacted or refused, and a mutation shows each fix is tested. This round adds ec2e068, which was a local commit that had not been pushed, and bb2f54d, which closes new gaps of the same kinds.

  • Send gate, code spans (bb2f54d): the gate removed inline code line by line with a plain backtick match. Markdown that GitHub renders as HTML could pass as code. A <details> hidden that way folded the Attachments listing, and the gate still allowed the send. The GitHub Markdown API confirmed the folding for each case:

    • a backslash inside a code span;
    • backtick runs of different lengths;
    • a code span that closes on the next line;
    • an HTML block (<hr>, <x-note>);
    • a tag attribute, autolink or HTML comment that takes the backtick.

    Code spans are now paired the CommonMark way. Inline code hides nothing when any line is ambiguous, when an HTML block or table delimiter row is present, or when a comment holds a backtick.

  • Sanitizer (bb2f54d):

    • A quoted secret flag value is redacted whole. Before, --password "a b c" left b c.
    • A bare value runs through , unless another assignment follows, so PASSWORD=a,b and HOSTS=a,b no longer leak b, while A=<a>,B=<b> stays.
    • pwd and camel-case names such as authKey and signingKey are secret keys.
    • The value arguments of hack env add|set <key> <value> and hack config set <key> <value> are redacted.
  • HTML declarations (ec2e068): <? and <! outside inline code cancel the listing.

Validation (head bb2f54d)

  • tests/feedback-review.test.ts: 29 pass. It adds 10 refused listing cases, 2 sendable ones (a harmless comment, and certain code spans before the heading), and 2 redaction fixture groups.

  • Mutation proofs: 15 new mutations, all killed. They cover:

    • the comment-backtick guard, the HTML-block rule, the table rule, exact run-length pairing, escape handling, the attribute-tag check, the autolink-backtick check, and the body-wide (not per-line) fallback;
    • comment stripping, which previously survived a mutation;
    • the value-argument rule, comma handling, quoted flags, camel-case keys, pwd, and the A=<a>,B=<b> placeholder terminator.

    Earlier mutations were rechecked and are still killed: attachment-name privacy, the fragment allowlist, the placeholder trailer, JSON log lines, the ~/.hack allowlist, inner private labels and the ciphertext field.

  • gitleaks 8.30.1 (gitleaks git --config .gitleaks.toml --redact --log-opts=origin/next..HEAD): 8 commits scanned, no leaks.

  • bun run --cwd packages/cli typecheck: 0 errors. bun run --cwd packages/cli check: 0 errors, plus the 34 existing warnings. Biome is clean on the changed files.

  • Full bun run test: run locally with load average 13 to 42. The result was 6950 pass, 149 skip and 277 fail. I reran every failing file serially with --timeout 60000.

    • Every non-native file passed, including all agent, MCP, plugin-generation, env and registry files.
    • These native runtime suites still fail: native-compose-adoption-generation (109), native-authored-project-start (42), native-compose-retained-storage-command (5), native-compose-storage-activation-command (5), native-compose-file-command (5), native-compose-vm-file-owner (3), native-compose-proxy-probe (2), native-authored-project-recovery (2) and native-authored-process-incarnation (1).
    • In native-authored-project-start, one runtime-stage failure marks the fixture unconfirmed, and the rest of the file fails after it. The first failing test passes when run alone.
    • None of these suites import the feedback modules. I am reporting them as load or environment failures on this host. Hosted CI is the confirming gate.

Remaining gaps (rework 2, verifier findings)

  • The sanitizer is still pattern-based:
    • Short flags such as -p <value> and -p<value> are not redacted, because -p often means a port.
    • Bare auth: <value> is not redacted.
    • A redacted bare value takes its closing backtick or parenthesis with it, which can leave inline code unbalanced in the draft.
  • The gate is conservative. Unrelated raw HTML, a fence, or a stray backtick turns off the inline-code exemption, so a listing next to `<script>` in code may be refused.
  • No live issue was posted, and no client loading of the skill was observed. Linear was not updated.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

…alidate

The bounded native authored consumer runs only `sha256:` image content IDs.
A `repository@sha256:` digest (or a tag) passed `graph native plan`, so
`hack up` reserved its startup intent and source before `graph native serve`
refused `native_graph_admission` during lowering.

Native selection now applies the lowering's image check and returns the same
closed `native_graph_admission` refusal. `plan`, `frontend-plan`, `run` and
`serve` refuse before any state, and authored `up` stops at its `native-plan`
stage with nothing reserved. With `HACK_RUNTIME_BACKEND=native`,
`config validate` reports the same code and message. Without it, validation
stays backend-neutral because Compose runs tags and digests.

Refs HACK-1300

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 21, 2026

Copy link
Copy Markdown

HACK-1300

@roodboi
roodboi marked this pull request as ready for review October 22, 2026 10:19
@roodboi
roodboi merged commit de2bd45 into next Oct 22, 2026
17 checks passed
@roodboi
roodboi deleted the wf/g23-repo-digest-validate branch October 22, 2026 10:19
roodboi added a commit that referenced this pull request Oct 23, 2026
)

## Root cause

A candidate run could start the stable hackd in four ways. Each runs whatever hackd the host
resolves, not the running CLI.

1. `requestDaemonJson` autostarts hackd by default when it runs in an interactive TTY:    - on
macOS with the default Hack home, through the per-user launchd plist, which points at the
stable binary;    - otherwise through `startDaemonViaCli`, where `resolveHackInvocation` picks
the first `hack` on      PATH, which is the stable CLI.

So a candidate command such as `HACK_RUNTIME_BACKEND=compose hack-next ps --json` (the daemon
query in `src/commands/project.ts`) could start the stable hackd. 2. `hack global up` runs `hack
daemon start` through `resolveHackInvocation` whenever    `daemon.autoStart` is on (the
default). This step ran even when the process was not interactive,    and it ignored both
`HACK_CANDIDATE_ENTRYPOINT` and `HACK_DAEMON_AUTOSTART_DISABLE`. It is    reachable from `hack-
next global up`, from `global install`, and from the interactive    `hack-next up` prompt that
offers to start global services (`maybePromptToStartGlobal`). 3. On macOS, `hack global install`
offers to run `hack daemon install`    (`maybeOfferMacHackdLaunchdInstall`), and a non-
interactive run accepts the offer. That command    also runs through `resolveHackInvocation`, so
`hack-next global install` installed the stable    hackd launchd service and started the stable
hackd through it. This check also ignored both    variables. 4. `hack doctor --fix` repairs
hackd (`maybeRepairHackd`): it restarts an incompatible hackd, clears    stale pid/socket files
and starts hackd, each through `hack daemon ...` on the first `hack` on    PATH. A non-
interactive run accepts every step. So `hack-v5 doctor --fix --no-interactive` with    a stale
daemon pid ran the stable `hack daemon clear` and `hack daemon start` without asking.    This
also ignored both variables, and the earlier docs wrongly said it would ask.

On the Compose backend that stable hackd then runs beside the candidate on the same `~/.hack`.
`scripts/hack-v5.sh` set no guard.

## Fix

- `src/daemon/client.ts`: new `resolveDaemonAutostartAccess`. It denies autostart when
  `HACK_DAEMON_AUTOSTART_DISABLE=1` (new, documented opt-out) or when
  `HACK_CANDIDATE_ENTRYPOINT` is
  set to any non-empty value. `hack-next` already sets that marker to `hack-next`, and the
  bundled
  launcher sets it to `hack-v5`. `ensureDaemonReady` checks it first, so a denied run reads no
  config
  and touches neither launchd nor PATH. Callers already fall back to direct Docker inspection,
  as for
  a non-interactive run. An explicit `hack daemon start` still works.
- `src/commands/global.ts` (rework): the `daemon.autoStart` step of `global up` moved into
  `ensureHackdForGlobalUp`. It checks `resolveDaemonAutostartAccess` first. When the run is
  denied,
  it logs `Skipping hackd start: <reason>`, reads no config and spawns nothing. Caddy and
  logging
  still start. The `global install` and `hack up` prompt paths go through `globalUp`, so the
  same
  check covers that step there.
- `src/commands/global.ts` (second rework, `5ddf03ee`): `maybeOfferMacHackdLaunchdInstall`
  checks
  `resolveDaemonAutostartAccess` first. A denied run logs `Skipping hackd launchd setup:
  <reason>`
  before it reads launchd state or prompts, and it runs no `hack daemon install`. The function
  is
  exported for tests.
- `src/commands/doctor.ts` (third rework, `ea48b508`): `maybeRepairHackd` checks
  `resolveDaemonAutostartAccess` after it reads the daemon status and before it prompts or runs
  anything, in interactive and non-interactive mode alike. A denied run skips the restart, clear
  and start steps and prints one `doctor fix` note: `Skipped hackd repair (hackd is <status>):
  <reason>` and `To repair it yourself, run: <command>`. The command is `hack daemon restart`
  for an
  incompatible hackd, `hack daemon clear && hack daemon start` for a stale one, and
  `hack daemon start` otherwise. An allowed run prompts and repairs as before. The
  `DAEMON_AUTOSTART_DISABLE_ENV` doc now lists this path as covered.
- `scripts/hack-v5.sh`: exports `HACK_DAEMON_AUTOSTART_DISABLE=1` for both backends. An
  inherited
  `HACK_DAEMON_AUTOSTART_DISABLE=0` cannot turn autostart back on.
- `docs/guides/candidate-install.md`: run stable `hack doctor` or `hack doctor --fix` **before**
  `hack daemon stop`, because v4.2.0 `doctor --fix` (`maybeRepairHackd`) offers to start hackd.
  When `pgrep -fl "hack daemon start"` still lists a process, run stable `hack daemon uninstall`
  (or `launchctl bootout "gui/$(id -u)/dance.hack.hackd"`), then `hack daemon stop` again, and
  run
  `hack daemon install` afterwards to restore the service. Third rework: `candidate-install.md`,
  `native-candidate.md` and `architecture.md` say that newer candidate builds do not start the
  stable hackd on their own, including from a candidate `global up`, `global install` or
  `doctor --fix`; that a candidate `doctor --fix` skips its hackd repair without asking, in a
  terminal or not, and prints the reason and the command to run; that this command runs the
  stable
  CLI; and that older bundles still can start hackd. They also correct the stable v4.2.0
  behaviour:
  stable `doctor --fix` asks before it starts hackd only in a terminal and starts it without
  asking
  in a non-interactive run, so the guide says not to run it again after `hack daemon stop`.
  `native-candidate.md` and `v5-dogfooding.md` follow the same order. `docs/architecture.md`
  documents the opt-out and every skipped step.
- `src/backends/native-authored-project-start.ts`: the HACK-1272 comment named
  `native_graph_admission` for a non-content-ID image as a refusal before reservation. #372
  (HACK-1300) makes plan refuse that image first. The comment now names reachable examples: a
  `native_graph_canceled` cancel, a source or review that changed after planning, and an
  unreachable engine.
- `src/lib/native-compose-ownership.ts`: the `waitNativeComposeStartupOwned` doc now lists both
  shapes that trigger the `settled` check. The second one, which the old doc left out, is a
  `running` or `restarting` member whose `ep-` placeholder first appears in the recheck scan
  (`transient` also counts recheck placeholders).

The branch also merges `origin/next` three times: at `616407a5`, at `4e0bfa55` in the second
rework, and at `2da56da0` in the third rework (merge commit `3e507ab8`). No merge had conflicts.

## Tests

`tests/launchd-isolation.test.ts` uses its existing isolation: darwin preload, a private HOME
and HACK_HOME, and a stub `launchctl` plus a stub `hack` first on PATH. The driver exits if the
stub is not first. Nothing reaches the real launchd, HOME or the Docker socket.

- A table test for `resolveDaemonAutostartAccess`: empty or blank marker, `0`/`true` values,
  trimmed
  `1`, and the precedence between the marker and the opt-out.
- The new `autostart` mode of `tests/fixtures/launchd-isolation-driver.ts` reports TTYs, so the
  interactive gate is open, and makes one default `requestDaemonJson` call. Six cases:
  - Denials: `hack-next` with the default home, `hack-next` with an isolated home, the `hack-v5`
    launcher env, and the opt-out alone. Each must record no `launchctl` call, no `hack` call,
  no
    socket file, and an unchanged plist.
  - Positive controls: a stable run with the default home records `launchctl print` and
    `kickstart`. A stable run with an isolated home records `hack daemon start`.
  - Third rework: during the request the driver wraps `Bun.file`, `Bun.which`, `Bun.spawn` and
    `Bun.spawnSync` and prints what they saw. Each denial case must show no read of the global
    config path, no PATH lookup and no spawn. Both positive controls must show the config read
  and
    the `launchctl` or `hack` spawn, so the recorder is proven to see them.
- `tests/doctor-fix-noninteractive.test.ts` (third rework): the existing harness (mocked shell
  `run`/`exec`, temp HOME, `isMac` true, a stale daemon pid) runs
  `doctor --fix --no-interactive` four times: the `hack-v5` env
  (`HACK_CANDIDATE_ENTRYPOINT=hack-v5`
  plus `HACK_DAEMON_AUTOSTART_DISABLE=1`, as `scripts/hack-v5.sh` sets them),
  `HACK_CANDIDATE_ENTRYPOINT=hack-next`, the opt-out alone, and a positive control with neither.
  Each denial must record no `daemon` command and print the skip note with `(hackd is stale)`,
  the
  reason, and `To repair it yourself, run: hack daemon clear && hack daemon start`. The positive
  control must record exactly `daemon clear` then `daemon start` and no skip note.
- `tests/native-candidate-launcher.test.ts` asserts that the launcher exports the opt-out on
  both
  backends, even when the caller passes `HACK_DAEMON_AUTOSTART_DISABLE=0`.
- `tests/global-command.test.ts` (rework): `global up` under `HACK_CANDIDATE_ENTRYPOINT=hack-v5`
  and under `HACK_DAEMON_AUTOSTART_DISABLE=1`. It uses the file's existing shell mock and a temp
  HOME. It asserts no `daemon` command, exactly one skip line that names the reason, no
  "Ensuring hackd is running" line, and that Caddy and logging still come up. The existing
  positive
  test still asserts that a stable `global up` runs `hack daemon start`. `beforeEach` clears
  both
  variables, and `afterEach` restores them.
- `tests/global-command.test.ts` (second rework): the macOS `global install` launchd offer is
  run
  directly and non-interactively (`HACK_NO_INTERACTIVE=1`, so the offer is accepted). HOME is
  the
  temp dir, so no plist exists and no `launchctl` runs, and `run` is mocked. The positive
  control
  records exactly one `hack daemon install` call. Under `HACK_CANDIDATE_ENTRYPOINT=hack-next`
  and
  under `HACK_DAEMON_AUTOSTART_DISABLE=1`, no `daemon` command is recorded and exactly one skip
  line
  names the reason.

Mutation proof (each mutation reverted afterwards):

| Mutation | Failing tests |
| --- | --- |
| Remove the guard in `ensureDaemonReady` | the 4 denial cases |
| Drop the candidate-marker check | the table test + both `hack-next` cases |
| Drop the opt-out check | the table test + the opt-out case |
| Remove the export from `hack-v5.sh` | all 4 launcher tests |
| Remove the guard in `ensureHackdForGlobalUp` | both new `global up` cases (6 pass, 2 fail) |
| Remove the guard in `maybeOfferMacHackdLaunchdInstall` (`5ddf03ee`) | both `global install` denial cases (9 pass, 2 fail) |
| Always skip the `global install` launchd offer (`5ddf03ee`) | the `global install` positive control (10 pass, 1 fail) |
| Remove the guard in `ensureHackdForGlobalUp`, rerun at `5ddf03ee` | both `global up` denial cases (9 pass, 2 fail) |
| Disable the guard in `maybeRepairHackd` (`ea48b508`) | the 3 `doctor --fix` denial cases (11 pass, 3 fail); each records `daemon clear` and `daemon start`, the reported defect |
| Move the `ensureDaemonReady` guard after `canPrompt()` and `readControlPlaneConfig` (`ea48b508`) | the 4 `requestDaemonJson` denial cases (10 pass, 4 fail), on the config-read assertion |

End-to-end probe (rework): `bun index.ts global up` with a private HOME and HACK_HOME, stub
`docker`, `hack`, `launchctl`, `brew` and `sudo` first on a minimal PATH, and the platform
preloaded as linux. The env was `HACK_CANDIDATE_ENTRYPOINT=hack-v5
HACK_DAEMON_AUTOSTART_DISABLE=1 HACK_RUNTIME_BACKEND=compose HACK_NO_INTERACTIVE=1`. In the
review run at `b21ec543`, the stub `hack` recorded `daemon start`. With the fix, the stub `hack`
recorded nothing and the output shows `Skipping hackd start: hackd autostart is disabled by
HACK_DAEMON_AUTOSTART_DISABLE=1.`

## Evidence

- Bun 1.4.2 (`mise x bun@1.4.2`).
- Local (this Mac), at `ea48b508` (the earlier head merged with `origin/next` `2da56da0`, plus
  the
  `doctor --fix` fix): `bun run --cwd packages/cli typecheck` exited 0. Root `bun run check`
  exited 0 with `privacy-check: ok` (its 34 Biome warnings are existing complexity warnings in
  other files). Biome on the 5 TS files changed in this rework is clean.
- Focused tests at `ea48b508`, run with `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`:
  `doctor-fix-noninteractive`, `launchd-isolation`, `daemon-autostart-bounded`, `daemon-client`,
  `global-command`, `native-candidate-launcher` and `doctor-fix-adopted-project`: 54 pass, 0
  fail.
- No test reached the real launchd, HOME, Docker socket or keychain: `launchd-isolation` uses
  its
  stub `launchctl`/`hack` and private HOME, and the `doctor --fix` cases use the mocked shell
  and a
  temp HOME.
- gitleaks 8.30.1 (`gitleaks detect --log-opts=origin/next..HEAD`): 4 commits scanned, no leaks.
- Earlier reworks: hosted CI run 39877694201 on `5ddf03ee` passed all 16 checks. A verifier
  confirmed brief items 1-5 for `requestDaemonJson`, `global up` and `global install`, and found
  the
  `doctor --fix` defect fixed here.
- Hosted CI run 39882998921 on `ea48b508`: all 16 checks passed, including `test` (the full
  suite), `docker-e2e` shards 1-4, Candidate core (macOS and Ubuntu), Config compiler x4,
  Runtime state models and `secret-scan`.

## Gaps

- No local full suite. Hosted CI `test` is the full-suite signal.
- The `global install` launchd offer is tested by calling the function directly, not through
  `runCli(["global", "install"])` with `isMac` mocked true. The macOS install bootstrap around
  it
  (DNS, trust, sudoers) is not exercised.
- The interactive `doctor --fix` path has no test of its own. The guard runs before the first
  prompt and does not depend on the mode, and the non-interactive tests prove it.
- No real-launchd or installed-bundle check. The guards are proven against the stub `launchctl`,
  the stub `hack` and the mocked shell only. No `hack-next` bundle was built or installed.
- Older candidate bundles do not have these guards. The guide still requires stopping stable
  hackd
  for every bundle.
- Moving the `ensureDaemonReady` guard after `canPrompt()` alone is not detected. `canPrompt()`
  only reads TTY and env state, so that order has no observable effect.
- The gateway and supervisor `daemon start` calls are explicit, unsupported flows and are
  unchanged.

Refs HACK-1368

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant