Repository navigation
fix(adoption): search every ancestor for an adoption and word doctor's adopted generated-files warning - #441
Merged
Merged
Conversation
…s adopted generated-files warning The adoption selector stopped after 64 directories and then refused the command as an unsupported adopted command, so `hack ps`, `up`, `down`, `logs` and `open` from a deep directory with no project exited with E_NATIVE_PROJECT_UNSUPPORTED. The selector now walks to the nearest project root or the filesystem root, as ordinary discovery does. Returning "no adoption" at a bound instead is unsafe: ordinary discovery reads an active adoption as native input, so `up` from deep inside an adopted project would reach native startup. For an adopted project, doctor's generated files warning now says that --fix leaves the adoption's .hack/.gitignore unchanged instead of promising to ensure it. Refs HACK-1380 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ted-selector-depth
…elector walks every ancestor After merging next (#432), the adoption selector returns null for a deep start directory with no adoption, so doctor's E_NATIVE_PROJECT_UNSUPPORTED catch was dead code that could hide a future refusal. Remove it, update the doctor helper's comments, the HACK-1374 deep-start test (the selector now resolves null) and the doctor paragraph of the adoption reference, which still described the 64-directory bound. Refs HACK-1380 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unit G62 for HACK-1380.
Root cause
selectLegacyComposeAdoptedRoot(src/lib/native-compose-adoption-command.ts) looked for an adoption in at most 64 directories. If it found no project root in that range, it calledunsupported(). Every caller let that error through:tryLegacyComposeAdoptedCommand(ps, up, down, logs, restart, exec throughtryNativeComposeCommand),tryNativeComposeOpen, and doctor'sfindDoctorAdoptedProject. So from a directory more than 64 levels deep with no project, these commands exited 1 with the misleadingE_NATIVE_PROJECT_UNSUPPORTED: Adopted Compose supports retained-container ….generated fileswarning (checkTrackedGeneratedFilesAtinsrc/commands/doctor.ts) said--fixwould "ensure the committed .hack/.gitignore". For adopted projects,--fixonly untracks the files and leaves the adoption's.gitignoreunchanged.Fix
discoverProjectInputs) walks. A start directory with no project now gets the same missing-project error at any depth. Unsafe adoption state still throws the marker's closedE_CONFIG_INVALID. The selector now has an explicitPromise<string | null>return type, and its doc comment states the invariant.… Run 'hack doctor --fix' to untrack them (files stay on disk). It leaves the adopted project's .hack/.gitignore unchanged, so keep these files out of later commits.This matches the existing--fixplan line and the fix note. The legacy text is unchanged.docs/reference/native-compose-adoption.md.Why there is no "search limit reached" result
The task suggested a distinct "search limit reached" result that callers would treat as "no adopted root". I built and probed that version and found it unsafe. Ordinary discovery reads an active adoption as native input (
inspectProjectInputsAtRoot). So if the selector misses an adoption more than 64 levels up, the command falls into the native authored path. With that version,hack up --detachfrom 70 levels inside an active adoption reached native startup and failed only because the fixture's compiler was missing (Native configuration compiler is unavailable). With the unbounded walk, the same command stays on the adopted path. The old limit protected nothing:dirnameof an absolute path always reaches/, and ordinary discovery already walks every ancestor without a limit.Tests
tests/native-compose-adoption-search-depth.test.ts(4 tests, start directories 70 levels deep):ps,up --detach,down,logs --no-followandopengive exactly the same exit code and output as from a 2-level directory, includeNo .hack/ (or legacy .dev/) found, and never includeE_NATIVE_PROJECT_UNSUPPORTED. The selector andfindDoctorAdoptedProjectboth return null.pslists the retained container (db\texited), the same as from a shallow directory.up --detachfrom deep gives the same result as from shallow.psanddowngive the same closedE_CONFIG_INVALIDas from a shallow directory, with no canary in the output.E_CONFIG_INVALIDrefusal with no canary or path, andpsfrom the CLI gives the same refusal.tests/doctor-adopted-generated-files.test.ts:doctor --jsonin an adopted Git checkout with a tracked.hack/.env.state.jsongives the adopted warning text exactly, with no "ensure the committed".privateCliEnv(private HOME and HACK_HOME, isolated Docker client config,HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1, daemon disabled). The adopted cases use only the synthetic retained source-bind engine. Git runs withGIT_CONFIG_GLOBAL=/dev/nullandGIT_CONFIG_NOSYSTEM=1.Mutation proof
depth < 64plusunsupported()): all 4 depth tests fail withE_NATIVE_PROJECT_UNSUPPORTED.depth < 64plusreturn null): the no-project test passes, and the active, pending and unsafe tests fail. With the in-process assertions removed, the CLI assertions alone still fail on activeps(Native Compose state is unsafe or changed) and on activeup --detach(Native configuration compiler is unavailable, meaning it reached native startup). The pending CLI assertions fail on a different refusal text.Evidence
mise x bun@1.4.2).bun run --cwd packages/cli typecheckpasses with 0 errors. Rootbun run checkpasses (privacy-check: ok; the 34 Biome warnings are not in the changed files). Biome is clean on the changed files.gitleaks diron agit log -p origin/next..HEADexport): no leaks.cfc3886e: hosted CI run 39888208360 passed (all 16 checks), but on a merge ref built before fix(doctor): bound the adopted-project search and use the ok-skip wording; test shared host stop waits #432 landed, so it does not cover next as it is now. Superseded by round 2 below.Gaps and coordination
a start directory deeper than the adoption search bound…expectsselectLegacyComposeAdoptedRoot({ cwd: deep })to reject withE_NATIVE_PROJECT_UNSUPPORTED. With this PR the call resolves to null.isAdoptionSearchBoundRefusalcatch becomes dead code. That is harmless, but it could also hide a real futureE_NATIVE_PROJECT_UNSUPPORTED.resolves.toBeNull(), drop the bound sentence, and preferably remove the catch. Its other assertions (findDoctorAdoptedProject null, preflight null, global-only fix plan) still hold.src/lib/doctor-adopted-project.tsortests/doctor-adopted-project.test.ts, so there is no textual conflict there. Both PRs edit the doctor paragraph indocs/reference/native-compose-adoption.md, so a nearby-lines conflict there is possible.hack upwith no project printsERROR No .hack/ (or legacy .dev/) foundwith noE_PROJECT_NOT_FOUNDcode, while ps, down, logs and open include the code. This happens at every depth and was there before this PR. It is reported as a separate finding.Round 2: merged next after #432
#432 (HACK-1374) merged into next as
bc825421after this PR's base. A verifier built next7d9c89f8plus this PR and found that #432's testa start directory deeper than the adoption search bound gets ordinary discovery instead of a throwfailed: it expected the selector to reject withE_NATIVE_PROJECT_UNSUPPORTED, and the selector now resolves to null. The docs and doctor's code comments also still described the 64-directory bound.Changes (
0f747d26, on top of a merge of next7d9c89f8):src/lib/doctor-adopted-project.ts: removed theisAdoptionSearchBoundRefusalcatch infindDoctorAdoptedProject(dead code now, and it would hide a futureE_NATIVE_PROJECT_UNSUPPORTED), the now-unusedHackCliErrorimport, and the 64-ancestor wording in the doc comments. Doctor calls the selector directly; unsafe adoption state still propagatesE_CONFIG_INVALID.tests/doctor-adopted-project.test.ts: fix(doctor): bound the adopted-project search and use the ok-skip wording; test shared host stop waits #432's deep-start test now expectsselectLegacyComposeAdoptedRoot({ cwd: deep })to resolve tonull. Its doctor assertions are kept unchanged:findDoctorAdoptedProjectandpreflightDoctorProjectreturn null and the fix plan is the global step only. So fix(doctor): bound the adopted-project search and use the ok-skip wording; test shared host stop waits #432's behaviour (a deep start directory gets ordinary discovery, not a throw) is kept.docs/reference/native-compose-adoption.md: replaced "The adoption search stops after 64 parent directories…" in the doctor paragraph with: doctor uses the same adoption search as project commands, so from a directory with no adoption above it, however deep, doctor uses ordinary project discovery. The doc now has no 64-directory bound left.Evidence at
0f747d26(Bun 1.4.2, private HOME/HACK_HOME/DOCKER_CONFIG,DOCKER_HOSTat a socket that does not exist,HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1,HACK_DAEMON_DISABLE=1):tests/doctor-adopted-project.test.ts,tests/native-compose-adoption-search-depth.test.ts,tests/doctor-adopted-generated-files.test.ts: 10 pass, 0 fail (host load average about 130).depth < 64plusunsupported()), fix(doctor): bound the adopted-project search and use the ok-skip wording; test shared host stop waits #432's deep-start test fails withE_NATIVE_PROJECT_UNSUPPORTED. With the fix it passes.bun run --cwd packages/cli typecheck: exit 0. Rootbun run check: exit 0 (privacy-check: ok). Biome clean on the 6 touched source and test files.gitleaks diron agit log -p origin/next..HEADexport): no leaks.testjob at0f747d26): pending: hosted CI at0f747d26is running; this line is updated when it finishes.Refs HACK-1380
🤖 Generated with Claude Code